Add a migraphx code review skill - #5111
Open
pfultz2 wants to merge 14 commits into
Open
Conversation
pfultz2
requested review from
CharlieL7,
TedThemistokleous,
kahmed10 and
shivadbhavsar
August 4, 2026 15:51
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new /migraphx-code-review Claude skill to standardize MIGraphX PR reviews across correctness, API/ABI, conventions, and test coverage, plus an optional MCP helper for interactively selecting which findings to act on. It also makes small documentation clarifications in repo guidance and the public program header.
Changes:
- Add
.claude/skills/migraphx-code-review/SKILL.md, encoding a structured multi-angle review process with optional--fix,--comment, and--selectmodes. - Add
.claude/mcp/select_findings.mjs, a zero-dependency MCP stdio server to support a checkbox-based selection flow for--select. - Clarify documentation in
AGENTS.md(template path) andsrc/include/migraphx/program.hpp(whatprogram_file_versiontracks).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/include/migraphx/program.hpp |
Clarifies that program_file_version is tied to MXR format changes, not operator changes. |
AGENTS.md |
Fixes the documented template path used by make generate. |
.claude/skills/migraphx-code-review/SKILL.md |
Introduces the new review skill definition and workflow, including --fix/--comment/--select. |
.claude/mcp/select_findings.mjs |
Adds an optional MCP server to provide checkbox selection UX for finding selection. |
| @@ -0,0 +1,799 @@ | |||
| --- | |||
| description: "Code review the changed MIGraphX code for correctness bugs, language-specific pitfalls, C/C++ API-ABI breakage, missing test coverage, and convention violations, with a verify pass that drops false positives. The quality checklist is delegated to /migraphx-simplify rather than repeated. Effort levels low through max; --comment posts inline PR comments, --fix applies every class of finding including the quality cleanups. A bare --fix or --comment after a review already ran this session applies or posts that review's findings instead of reviewing again. --select opens a checkbox picker so only the chosen findings are fixed or posted." | |||
| allowed-tools: Bash(git diff:*), Bash(git status:*), Bash(git log:*), Bash(git show:*), Bash(git blame:*), Bash(git rev-parse:*), Bash(git merge-base:*), Bash(git branch:*), Bash(git fetch:*), Bash(gh pr view:*), Bash(gh pr diff:*), Bash(gh api:*), Bash(grep:*), Bash(find:*), Read, Grep, Glob, Edit, Agent, Skill, ReportFindings, AskUserQuestion, mcp__github_inline_comment__create_inline_comment, mcp__review-picker__select_findings | |||
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #5111 +/- ##
=========================================
Coverage 93.26% 93.26%
=========================================
Files 623 623
Lines 32963 33097 +134
=========================================
+ Hits 30741 30866 +125
- Misses 2222 2231 +9
🚀 New features to boost your workflow:
|
Regressions detected 🔴 * No develop baseline was found for this PR's branch point; compared against the latest available develop run instead. |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The review standard for this repo lives in three places —
AGENTS.md, the/migraphx-simplifyquality checklist, and the expectations reviewers apply inPR comments — and only the first two are written down. Applying all three by
hand on every branch is slow and uneven. This adds a
/migraphx-code-reviewskill that encodes the standard and runs it over a branch or PR.
Technical Details
.claude/skills/migraphx-code-review/SKILL.md— the skill. It fans out oneagent per review angle (line-by-line diff scan, removed-behavior audit,
cross-file tracing, per-language pitfalls, C/C++ API-ABI, IR contracts,
conventions, test coverage, and precedent distilled from the review comments
on PRs [AIMIGRAPHX-885] Add find_concat_same_input matcher #4891–Avoid the output copy when the result is an aliased view #5108), then runs a verify pass that drops refuted candidates.
Five effort levels (
lowthroughmax) trade candidate count andprecision-vs-recall bias for cost;
lowis a single inline pass with nosubagents. Rules are read and quoted from
AGENTS.mdand/migraphx-simplifyrather than restated, so the skill can't drift fromthem.
--fixapplies findings,--commentposts them inline on a PR, and--selectfilters what either one acts on..claude/mcp/select_findings.mjs— zero-dependency MCP stdio server backing--selectwith a checkbox dialog via MCP elicitation. Not registered bydefault;
--selectfalls back to a plain multi-select prompt without it.AGENTS.md— correct themake generatetemplate path totools/include/.src/include/migraphx/program.hpp— clarify thatprogram_file_versiontracks the MXR format and is not bumped for operator changes.
Changelog Category
Add a
CHANGELOG.mdentry for any option other thanNot ApplicableFollow the LLVM AI Tool Use Policy for contributions using AI.