Repository navigation
chore(amdsmi): use the amd-smi review agents in Copilot code review - #14718
Conversation
- Add .github/instructions/amdsmi.instructions.md, scoped to projects/amdsmi/**, so Copilot code review applies the amd-smi maintainers' review rules to amd-smi changes. Copilot code review only reads instruction files at the repository root. - Give the file to @ROCm/smi-reviewers in CODEOWNERS, like the amdsmi workflows. Signed-off-by: Adam360x <Adam.pryor@amd.com>
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
There was a problem hiding this comment.
🟡 Changes recommended
The review instructions would cause false positives by mischaracterizing ASSERT_* behavior in void lambdas and prohibiting documented partial metrics results containing N/A values.
2 open issues
What changed in this PR
This PR makes amd-smi’s review guidance available to Copilot through a repository-root instruction file, scoped to projects/amdsmi/**.
Changes:
- Adds review checks for correctness, API compatibility, concurrency, tests, and documentation, placing code checks before wording rules.
- Excludes the instructions from the Copilot cloud agent.
- Assigns ownership of the instruction file to
@ROCm/smi-reviewers.
| File | Description |
|---|---|
.github/instructions/amdsmi.instructions.md |
Adds scoped amd-smi guidance for Copilot code review. |
.github/CODEOWNERS |
Assigns the new instruction file to the amd-smi reviewers. |
Review effort: Balanced
- Replace the hand-written summary with the review agents themselves: tools/update_copilot_review_instructions.py copies the body of each .github/agents/amdsmi-review*.agent.md, unchanged, into .github/instructions/<agent>.instructions.md at the repository root, scoped to projects/amdsmi/** - Add the AMDSMI Copilot Review Instructions workflow, which fails when a copy no longer matches its agent - Give the generated files to @ROCm/smi-reviewers in CODEOWNERS Signed-off-by: Adam360x <Adam.pryor@amd.com>
- Replace the generated copies in .github/instructions/ with symlinks to the review agents, so Copilot code review always reads the current agents - Add applyTo and excludeAgent to each review agent's frontmatter, which Copilot code review reads through the symlinks - Remove the generator script and its CI check, which the symlinks make unnecessary Signed-off-by: Adam360x <Adam.pryor@amd.com>
geomin12
left a comment
There was a problem hiding this comment.
lgtm, but i would like to see amdsmi team folks review as well
|
@copilot resolve the merge conflicts in this pull request |
…smi-copilot-review-instructions # Conflicts: # .github/CODEOWNERS # projects/amdsmi/.github/agents/amdsmi-review-maintainer.agent.md Co-authored-by: adam360x <61172547+adam360x@users.noreply.github.com>
Merged |
TheRock Submodule Bump ActivityNewest first
|

Motivation
Copilot code review runs on every
developPR but never sees the amd-smi review agents (maintainer, skeptic, security, tests and the rest): it reads instruction files only from the repository root and cannot run custom agents, soprojects/amdsmi/.github/agents/is ignored.Technical Details
Copilot instructions (
.github/instructions/amdsmi-review*.instructions.md):projects/amdsmi/.github/agents/, so Copilot code review always reads the current agents. Nothing is copied.Review agents (
projects/amdsmi/.github/agents/amdsmi-review*.agent.md):applyTo: "projects/amdsmi/**", so only PRs that change amd-smi files get the agents, andexcludeAgent: "cloud-agent", which keeps them out of the Copilot cloud agent. Copilot reads both through the symlinks; VS Code doesn't use them for agent files.Code owners (
.github/CODEOWNERS):/.github/workflows/amdsmi-*.yml.Side effect: VS Code also reads root
.github/instructions/. With the whole repository open, it attaches the agents to agent-mode edits of files underprojects/amdsmi/.Issue Tracking
JIRA ID: TBD
Test Plan
developdoesn't have.Test Result
.github/instructions/files, copied ([DO NOT MERGE] docs(amdsmi): update the changelog (check L) #14719) or symlinked ([DO NOT MERGE] docs(amdsmi): update the changelog (check S1) #14750, [DO NOT MERGE] docs(amdsmi): update the changelog (check L) #14798): all 11 test words each time, so it reads every agent to the end. It ignored them inprojects/amdsmi/REVIEW.md([DO NOT MERGE] docs(amdsmi): changelog entry for the process list fix (exp D) #14703, [DO NOT MERGE] docs(amdsmi): changelog entry for the process list fix (exp D2) #14708), behind a root file pointing intoprojects/amdsmi/([DO NOT MERGE] docs(amdsmi): changelog entry for the process list fix (exp C) #14702, [DO NOT MERGE] docs(amdsmi): changelog entry for the process list fix (exp C2) #14707), and behind a root review skill pointing there ([DO NOT MERGE] docs(amdsmi): update the changelog (check S2) #14751, [DO NOT MERGE] docs(amdsmi): update the changelog (check S3) #14752).developdoesn't have ([DO NOT MERGE] docs(amdsmi): update the changelog (check F) #14800).rocm-core/rdhcline, is ondeveloptoo).Submission Checklist