Repository navigation
docs: add agent guidance to reduce verbosity - #3337
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| @@ -1,3 +1,3 @@ | |||
| # Instructions | |||
|
|
|||
| ## Code review | |||
There was a problem hiding this comment.
these instructions were copied over from iceberg-cpp, and can use some tuning
| under the License. | ||
| --> | ||
|
|
||
| # Apache Iceberg Rust — Agent Instructions |
There was a problem hiding this comment.
we only had "Security Model" in this file before
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The API-growth check can incorrectly classify legitimate downstream APIs as crate-internal.
Review effort: Balanced
Findings: 1
What changed in this PR
Adds repository guidance to reduce verbose agent-generated code, documentation, reviews, and PR descriptions.
Changes:
- Defines concise commenting, documentation, visibility, and diff-scope rules.
- Extends Copilot review guidance to flag unnecessary churn and API growth.
| File | Description |
|---|---|
AGENTS.md |
Adds coding-agent and PR guidance. |
.github/copilot-instructions.md |
Adds targeted review checks. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| - API growth: New `pub` items with no use outside the crate (see | ||
| `public-api.txt`). Suggest `pub(crate)`. |
|
I'm still new at this coding agent / review agent thing, so not sure if this is the best. PTAL! cc @blackmwk @CTTY @laskoviymishka @mbutrovich @dannycjones @comphead |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| explicitly instruct them to "Please check and address all review | ||
| comments in this PR." | ||
|
|
||
| Also flag these, once per PR with the locations: |
There was a problem hiding this comment.
note that these items that should be flagged
comphead
left a comment
There was a problem hiding this comment.
Thanks @kevinjqliu looks good to me, my agent skill also includes below, maybe useful in this PR:
- Reuse before adding: for every new object, method, helper, abstraction, etc., explicitly check whether an existing one can be reused, extended, or generalized.
- Deduplicate tests: identify tests introduced by the PR that duplicate existing coverage and remove/merge them rather than increasing test count without adding coverage
- Remove PR-created dead code: detect unused methods, structs, imports, fields, helpers, branches, feature flags, and test utilities introduced by the PR
- Avoid abstraction for abstraction's sake: reuse/generalize only when it makes the code simpler; don't introduce a new abstraction merely to eliminate a few lines
|
I like these, thanks @comphead! I’ll add them to the pr. What are your thoughts on agent skill vs AGENTS.md? Do you think these changes are helpful in AGENTS.md? |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
AGENTS.md makes more sense to me as this is project wise skill for any agent. The good followup would also be to have a kinda precommit hooks to perform code analysis and fix it. Also run tests, to avoid spending ASF CI resources on partially ready PRs like https://github.com/apache/datafusion/blob/main/AGENTS.md#before-committing |
|
I'd also recommend adding PR review skills to the repo so you can set expectations for reviews. Feel free to take inspiration from those in Comet |
https://github.com/apache/datafusion-comet/tree/main/.ai/skills |
dannycjones
left a comment
There was a problem hiding this comment.
Thanks Kevin, I think these are good improvements. Good to encode in AGENTS.md rather than continuing to argue with it 😄
AGENTS.md makes sense as a good general steer for the LLM. I'd hestitate to add skills for now except for very specific tasks (like code review which Andy suggested just now).
I'm wondering if we can do this, and just point Copilot at it so people can use whichever model and harness with the same markdown. |
laskoviymishka
left a comment
There was a problem hiding this comment.
Thanks! This will make our life a bit better <3
🌬️ ⛵
|
I'm taking a pass through this now. |
mbutrovich
left a comment
There was a problem hiding this comment.
Thanks @kevinjqliu, this will save reviewers a lot of back and forth with agent-written PRs. The comments suggest a few additions drawn from feedback that comes up repeatedly in reviews here.
Co-authored-by: Matt Butrovich <mbutrovich@users.noreply.github.com>
blackmwk
left a comment
There was a problem hiding this comment.
Thanks @kevinjqliu for this pr, and @mbutrovich for review!

Which issue does this PR close?
What changes are included in this PR?
I find my agent not behaving the way I'd like to create PRs, write code, and documentation... Hopefully these rules are a helpful nudge towards the right direction. 😄
Coding agents tend to be verbose: comments that restate the code, docs explaining review history, tests narrating asserts, unrelated renames, new
pubitems nobody needs, and PR descriptions that restate the diff.AGENTS.md: add guidance for coding agents. Default to no comment, keep docs to what callers need, keep diffs to the change, reuse before adding, use the narrowest visibility, keep PR descriptions short..github/copilot-instructions.md: have Copilot review flag comment churn, unrelated edits, duplication, API growth, and verbose PR descriptions instead of skipping them as trivialities.Follows the Java repo's comment guidance (apache/iceberg AGENTS.md comment guidance, rewrite).
Are these changes tested?
Docs only.
AI Disclosure
Implemented with GitHub Copilot.