review-rocmlir-pr

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:

1. 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.
2. 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:

  1. 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).
  2. 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.
  3. Self-contained. Fully address the finding without requiring matching