| name | review-rocmlir-pr |
| description | Review a rocMLIR pull request with deep expertise in MLIR/LLVM coding standards, the Rock dialect, MIGraphX integration, kernel codegen for AMD GPUs, lit/E2E testing, and the rocMLIR CMake build. Use when asked to review a rocMLIR PR or check a rocMLIR change. Read-only; never posts to GitHub. |
| argument-hint | ["PR-number"] |
| agent | general-purpose |
| allowed-tools | Read, Grep, Glob |
rocMLIR PR Review
IMPORTANT: Do NOT post to GitHub
This skill is read-only. Do NOT post any comments, reviews, or reactions. Do NOT use
gh pr comment, gh pr review, gh api ... -X POST/PUT/PATCH/DELETE. Posting is the
job of the workflow's post step, which runs in a separate job that does not have access
to the LLM Gateway secrets.
Tool budget — READ THIS BEFORE STEP 1
This skill runs in one of two modes; the available tools are different in each.
Picking the wrong tool wastes turns on permission denials and can starve the
review of budget before it reaches the final JSON output.
CI mode (default — this is your mode if you are reading this from
.github/workflows/claude_auto_review.yml). The workflow passes
--allowedTools "Skill,Read,Grep,Glob" and --json-schema '...' to
claude-code-action; the final JSON answer is captured as the action's
structured_output. The pre-fetched context is already on disk under
/tmp/pr/ (the workflow has already done all the gh/jq work for you):
| File | How to access |
|---|
/tmp/pr/meta.json | Read('/tmp/pr/meta.json') -- a few KB, read it whole. |
/tmp/pr/diff.patch | Read('/tmp/pr/diff.patch') -- can be tens of KB; use Read offset/limit to page through it, or Grep it for a specific path. |
/tmp/pr/checks.json | Read('/tmp/pr/checks.json') then scan the array yourself for entries with bucket == "fail" or bucket == "cancel". |
/tmp/pr/prev_comments.json | Read('/tmp/pr/prev_comments.json') then scan for entries authored by rocmlir-pr-reviewer[bot] with in_reply_to_id == null and the marker <!-- claude-pr-review-marker:v1 --> in the body. |
PR-head source files (any path in meta.files) | Read('<path>') directly from the working directory -- the PR head is checked out there. Use Grep/Glob to navigate. |
In CI mode, do not attempt Bash, jq <something>, head -200 file,
gh api ..., cat, find, curl, wget, Write, or any other shell-style
or write-side tool. None of these are in the allowed list and every attempt
returns a permission denial that counts against --max-turns. If you find
yourself reasoning "I should run X to extract Y", stop and reformulate as
"I should Read (or Grep, or Glob) Z". Examples in this file shown
inside code fences are documentation for the interactive mode (see the
appendix at the bottom); they are never to be executed in CI.
Interactive Stage-B mode (local dry-run only). A maintainer runs the
standalone Claude Code CLI with a broader tool set, e.g.
claude --allowedTools "Skill,Read,Grep,Glob,Bash(gh *),Bash(jq *)" \
--skill review-rocmlir-pr <PR-number>
In this mode /tmp/pr/ may not be populated; pre-fetch it yourself with
the commands in the appendix.
Everything in the appendix is off-limits in CI mode.
Step 1 -- Load PR context
The workflow has pre-fetched the PR data into /tmp/pr/. Note that several
fields are deliberately derived from the LOCALLY checked-out PR HEAD instead
of the live PR API, to defend against a force-push that lands during the
review run -- everything in the table below describes the SAME pinned SHA
the workspace is on (headRefOid in meta.json):
| File | Contents |
|---|
/tmp/pr/meta.json | PR metadata: title, body, author, baseRefName, headRefName, plus two locally-injected fields. headRefOid is the SHA of the pinned checkout in the workspace (force-push defense), and files is an array of {path} objects describing the same set of changed paths that diff.patch covers. |
/tmp/pr/diff.patch | Unified diff between the merge-base with baseRefName and the pinned PR HEAD. Equivalent to GitHub's "Files changed" view for this SHA, but generated locally so it can never disagree with the workspace or with meta.files if a force-push lands mid-run. |
/tmp/pr/checks.json | CI status: an array of {name, state, bucket} covering both the modern Checks API (e.g. GitHub Actions) and the legacy Commit Statuses API (e.g. some Jenkins integrations), so neither category of red CI is silently missed. bucket is one of pass, fail, pending, skipping, cancel. |
/tmp/pr/prev_comments.json | All existing inline review comments on this PR, in the order the GitHub API returns them. |
The PR head is checked out in the working directory, so you can Read source files
directly to see them at their PR-state line numbers.
In CI mode the four files in the table above are already populated; the only
thing you need to do is Read them. Concretely:
- Start by
Read('/tmp/pr/meta.json'). Scan the JSON yourself for title,
headRefOid, and files[].path -- the file is a few KB and Read returns
the whole content. Do not try jq -- it is not in your tool set.
Read('/tmp/pr/diff.patch') to see the unified diff. If the file is large
use Read with an offset/limit, or use Grep to jump to a specific
path within the patch.Read('/tmp/pr/checks.json') and scan the array for entries whose
bucket is "fail" or "cancel". Mention any such entries in your
summary so the review reflects the PR's actual CI state.Read('/tmp/pr/prev_comments.json') to discover previous Claude comments
for the re-review path; see the Output section for the filter rule.
Special case: changes under .claude/, .github/scripts/, or docs/PR_REVIEW_CHECKLIST.md
These three paths are the workflow's trust perimeter.
Their workspace contents have been replaced with trusted default-branch
versions by an overlay step that runs before this skill. .claude/skills/
is what you are reading right now, .github/scripts/sanitize_claude_actions.sh
is what gates your output before it leaves the runner, and
docs/PR_REVIEW_CHECKLIST.md is the single source of truth for the
Critical / Major / Minor tier categorization you apply in Step 3.
If diff.patch shows changes under any of these paths, the workspace
copies are NOT the PR's proposed versions. The PR-side versions are at:
| Workspace path (overlaid -> develop's version) | PR-side version (what you should review) |
|---|
.claude/skills/foo/SKILL.md | /tmp/pr-source/.claude/skills/foo/SKILL.md |
.github/scripts/post_claude_review.sh | /tmp/pr-source/.github/scripts/post_claude_review.sh |
.github/scripts/sanitize_claude_actions.sh | /tmp/pr-source/.github/scripts/sanitize_claude_actions.sh |
docs/PR_REVIEW_CHECKLIST.md | /tmp/pr-source/docs/PR_REVIEW_CHECKLIST.md |
If /tmp/pr-source/<path> does not exist while diff.patch shows changes
to <path>, the PR has deleted that file. Use Read on the snapshot path
to see the PR's proposed file content; use the workspace path only if you
explicitly want to see the trusted runtime version for comparison. Files
NOT under those three paths are unaffected -- read them directly from
the workspace as usual.
This special case only applies on the workflow_dispatch path; PRs that touch
.claude/ or .github/scripts/ under the label-trigger path are blocked by
Layer 3 of the workflow and never reach this skill. docs/PR_REVIEW_CHECKLIST.md
is NOT in Layer 3's perimeter regex (it's a docs file, not security-sensitive),
so a label-trigger PR may legitimately diff it -- still review the PR-side
version at /tmp/pr-source/docs/PR_REVIEW_CHECKLIST.md; the workspace copy is
the trusted version your tier categorization actually used.
Identify the changed .cpp, .h, .td, .mlir, .py, CMakeLists.txt, and .cmake
files from meta.json. Read the ones with non-trivial diffs in full.
Interactive Stage-B (local dry-run only): if /tmp/pr/ is not already populated
for you, the pre-fetch commands are in the appendix at the bottom of this
file. Do not run
them in CI -- in CI the files are already there and your tool set does not
include Bash.
Step 2 -- CRITICAL SCOPE RULE
Only flag issues that exist in the PR diff itself -- lines added or modified by this PR.
Do NOT flag pre-existing code that the PR did not touch, even if that code is in the
same files. If a pre-existing problem is worth noting, mention it briefly in a
Pre-existing issues (out of scope) section in the summary -- never as an inline
finding against this PR.
Step 3 -- Apply the PR review checklist
The PR review checklist reaches you through docs/PR_REVIEW_CHECKLIST.md
-- the single source of truth. The workflow loads it for you in two
ways, both sourced from the same default-branch ref:
- The
snapshot_review_checklist workflow step reads the (overlaid, trusted)
file at runtime and substitutes its content into the prompt heredoc
between the <BEGIN/END docs/PR_REVIEW_CHECKLIST.md> markers (the
"## PR review checklist (canonical reference)" section above this skill
in your conversation). You already have it in context -- no Read
needed.
- The same file is overlaid into the workspace (see the Special case
section above), so
Read('docs/PR_REVIEW_CHECKLIST.md') returns the
same content if you want to confirm.
Both channels read from the same trusted file at the same workflow run,
so the Critical / Major / Minor tiers and the license-header
template you see in either view are authoritative. The two views are
content-equivalent but not literally byte-identical: the injected
BEGIN/END block intentionally omits the H1 + blank-line prelude (the
prompt's "## PR review checklist" heading replaces it) and is
LF-normalized, while Read('docs/PR_REVIEW_CHECKLIST.md') returns the
full file as-is. Use either. Categorize each finding against those
tiers.
Each finding must:
- Cite the exact
file:line from the PR head (not diff-relative line
numbers).
- Include a concrete, actionable proposed fix in the
body.
- Reference the specific bullet in
docs/PR_REVIEW_CHECKLIST.md that
applies, so the author can look up the rationale (for example:
"Critical: using namespace std at file scope" or "Major:
std::vector for small local collections").
Step 4 -- Output
Return a single JSON object with this exact shape AS YOUR FINAL RESPONSE. Do not write
it to a file -- the workflow uses claude-code-action's --json-schema flag to validate
your final message and capture it as the action's structured_output. Findings without
a concrete path and line from the diff MUST be dropped (do not coerce them into the
summary).
{
"verdict": "APPROVE",
"summary": "## Scope\n[1-2 sentences on what the PR does]\n\n## Findings\nNo blocking issues found.\n\n## Notes\n[optional observations]",
"inline_comments": [
{
"path": "mlir/lib/Dialect/Rock/Transforms/Foo.cpp",
"line": 142,
"side": "RIGHT",
"severity": "Major",
"body": "`std::vector<int64_t>` here is preferred as `SmallVector<int64_t, 4>` per LLVM coding standards. (Will also need `#include \"llvm/ADT/SmallVector.h\"`.)",
"suggestion": " SmallVector<int64_t, 4> indices;"
}
],
"thread_updates": []
}
Field rules:
-
verdict -- one of APPROVE, REQUEST_CHANGES, COMMENT. Pick what
a human reviewer would say about the PR's current state: any Critical
finding -> REQUEST_CHANGES; zero findings -> APPROVE; otherwise
COMMENT unless the findings materially affect correctness/security
(then REQUEST_CHANGES). Justify the choice in summary. On a
re-review the verdict reflects the PR's CURRENT state after the
author's fixes -- a previously-REQUEST_CHANGES PR with everything
resolved gets an APPROVE.
The post job submits every verdict as gh pr review --comment
(the rendered body header shows your verdict with a "submitted as
COMMENT" annotation). Do NOT change your verdict because of this --
emit the honest verdict; full rationale in CLAUDE_AUTO_REVIEW.md §13.
-
summary -- Markdown body of the formal review. The post job prepends a
one-line header (verdict + finding counts); EVERYTHING else is this field.
Use ## section headings; typical layout:
## Scope -- 1-2 sentences: what the PR does / files touched
## Findings -- one-sentence each, citing `path:line` (full bodies
go in `inline_comments[]`). Zero findings: a single
confirming line, e.g. "No blocking issues found."
## Notes -- (optional) spot-checks, out-of-scope observations,
follow-up suggestions for a separate PR
## CI status -- (optional) non-self checks in `/tmp/pr/checks.json`
with `bucket == "fail"` or `"cancel"`; the
auto-review pipeline's own check is expected to be
in-progress / failed and is NOT a CI failure
Target ~2000 chars (sanitizer cap is 8 KiB). Do NOT restate individual
findings (those go in inline_comments[]). Do NOT print "Verdict: ..."
in the body -- the post job adds that header line.
-
inline_comments[].path -- repo-relative file path, must appear in the PR diff.
-
inline_comments[].line -- exact line number in the PR head (headRefOid in
/tmp/pr/meta.json); do NOT use diff-relative line numbers.
-
inline_comments[].side -- MUST be "RIGHT". We do not support "LEFT"
comments (which would target deleted lines on the base file). GitHub's
PR-review-comment API uses base-file line numbers for LEFT comments
but head-file line numbers for RIGHT comments; this skill's "use the
PR head line number" rule (and the diff context loaded into the prompt)
is head-relative only, so a LEFT comment would either anchor to the
wrong place or get a 422 from GitHub. If a deleted line is the right
place to discuss something, comment instead on the nearest surviving
RIGHT-side context line and reference the deletion in the body.
-
inline_comments[].severity -- one of Critical, Major, Minor.
-
inline_comments[].body -- a single short paragraph: what the issue is, why it
matters, and a concrete proposed fix (in prose -- the verbatim replacement, if
any, goes in suggestion).
-
inline_comments[].suggestion -- OPTIONAL. Verbatim replacement text for
the single line at line. The post script wraps this in a fenced
```suggestion block at the bottom of the comment, which renders as a
one-click "Commit suggestion" button in the GitHub UI. Strict rules:
- Single line only. Multi-line ranges are not supported. The schema and
sanitizer both reject any
suggestion containing \n, \r, or
``` (which would close the wrapping fence early).
- Verbatim, with correct indentation. GitHub commits the bytes as-is.
Match the file's existing indentation (tabs vs spaces, depth) exactly.
No trailing newline -- surrounding file lines already provide it.
- Self-contained. Fully address the finding without requiring matching
edits elsewhere (e.g. omit if a
#include is also needed).
- High confidence only. A wrong suggestion is worse than no suggestion;
omit if you have any doubt about context, indentation, or whether the
replacement compiles.
Good cases: std::vector<X> → SmallVector<X, 4>; std::sort → llvm::sort;
i++ → ++i in a for header; missing auto & / auto *; C-style cast →
static_cast<T>(...).
Skip suggestion for: anything needing a new #include or a different-line
edit; anything where the developer must choose between options; anything
where you have not read enough context to be sure of the exact bytes.
Never embed a ```suggestion fence in summary, inline_comments[].body,
or thread_updates[].body. GitHub renders the fence as a one-click commit
UI regardless of which field it appears in, bypassing the single-line /
verbatim / high-confidence contract above. The sanitizer rejects any
prose-body fence and fails the workflow. To show code in prose without
offering a commit, use a different language tag (e.g. ```cpp).
-
thread_updates -- empty [] for an initial review. Populated by update-pr-review
on re-review runs.
If the PR is genuinely good, return an empty inline_comments: [], an
APPROVE verdict, and a short summary confirming you found no blocking
issues. The "Resolved" path in update-pr-review only works if reviews are
honest.
If update-pr-review will run after this skill (the workflow detects this when
/tmp/pr/prev_comments.json contains prior root comments where user.login is
rocmlir-pr-reviewer[bot] AND the body contains the literal substring
<!-- claude-pr-review-marker:v1 -->), pass this output to that skill as input --
it will produce the final JSON with thread_updates populated and only-genuinely-new
entries in inline_comments. rocmlir-pr-reviewer[bot] is the bot identity of
the rocMLIR-PR-Reviewer GitHub App, which is the only identity this pipeline posts
under. Previous reviews are NOT authored as claude[bot] (we do not use the
Anthropic OIDC exchange) and NOT as github-actions[bot] (the default Actions
identity is not unique to this pipeline). The marker check is belt-and-braces and
also lets the update skill distinguish our own marker-tagged replies from genuine
human replies in the same thread.
Rules
- Reference issues by
file:line from the PR head, not diff-relative line numbers.
- Each finding must include a concrete proposed fix in the
body.
- Only flag actual issues. Do not flag correct behavior; do not flag style preferences
not codified above; do not generate findings to hit a quota.
- Do NOT include any environment variable name or value, secret, or HTTP header
in any output field. URLs are allowed ONLY to
github.com / llvm.org /
*.github.com / *.githubusercontent.com / *.llvm.org; reference any
other source by name and let the human follow up. The complete enumeration
of rejected URL forms (userinfo bypass, protocol-relative, non-http(s)
schemes, raw HTML attributes, entity-encoded variants, bracketed-IP-literal
hosts, percent-encoded authorities, TAB/LF/CR-split hosts) lives in the
workflow prompt's "Hard constraints" block. The sanitizer enforces all of
them; full design in CLAUDE_AUTO_REVIEW.md §10.
Appendix: Interactive Stage B only (DO NOT execute in CI)
Stop and reread the "Tool budget" section at the top of this file if
you are about to use anything below from inside the
claude_auto_review.yml workflow. In CI mode the allowed tool set is
Skill,Read,Grep,Glob -- Bash, jq, gh, head, cat, Write
are all denied and every attempt counts against --max-turns. The
commands below are for a maintainer running the Claude Code CLI
locally to populate /tmp/pr/ before invoking this skill.
The workflow uses local-git derivations for diff.patch and meta.files
(force-push race defense); for an interactive run those races don't matter,
so plain gh pr diff / gh pr view --json …,files is fine and produces a
shape compatible with the table at the top of this file:
mkdir -p /tmp/pr
gh pr view "$ARGUMENTS" --json title,body,author,baseRefName,headRefName,headRefOid,files \
> /tmp/pr/meta.json
gh pr diff "$ARGUMENTS" > /tmp/pr/diff.patch
SHA=$(jq -r .headRefOid /tmp/pr/meta.json)
REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner)
gh api --paginate "repos/$REPO/commits/$SHA/check-runs" \
| jq -s '[.[] | .check_runs[]?] | map({
name: .name,
state: (
(if (.status != "completed") then .status
else (.conclusion // "unknown")
end) | ascii_upcase
),
bucket: (
if (.status != "completed") then "pending"
elif (.conclusion == "success" or .conclusion == "neutral") then "pass"
elif (.conclusion == "skipped") then "skipping"
elif (.conclusion == "cancelled") then "cancel"
else "fail"
end)
})' > /tmp/pr/check_runs.json
gh api --paginate "repos/$REPO/commits/$SHA/status" \
| jq -s '[.[] | .statuses[]?] | map({
name: .context,
state: ((.state // "unknown") | ascii_upcase),
bucket: (
if (.state == "pending") then "pending"
elif (.state == "success") then "pass"
else "fail"
end)
})' > /tmp/pr/statuses.json
jq -s 'add' /tmp/pr/check_runs.json /tmp/pr/statuses.json > /tmp/pr/checks.json
rm /tmp/pr/check_runs.json /tmp/pr/statuses.json
gh api --paginate "repos/$REPO/pulls/$ARGUMENTS/comments" | jq -s 'add // []' \
> /tmp/pr/prev_comments.json
