review-rocmlir-triton-pr

name	review-rocmlir-triton-pr
description	Review a rocmlirTriton pull request with deep expertise in MLIR/LLVM coding standards, the Rock dialect, MIGraphX integration, the vendored Triton and Triton-pinned LLVM integration (Rock->TTIR->TTGIR->LLIR pipeline, downstream patch records, hardware-feature detection via rock::*), kernel codegen for AMD GPUs, lit/E2E testing, and the rocmlirTriton CMake build. Use when asked to review a rocmlirTriton PR or check a rocmlirTriton change. Read-only; never posts to GitHub.
argument-hint	["PR-number"]
agent	general-purpose
allowed-tools	Read, Grep, Glob

rocmlirTriton 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/commits.json	Read('/tmp/pr/commits.json') -- commit subjects, touched paths, and diffPath entries for commit-level checks.
/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-triton-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/commits.json	Commit metadata for the same pinned PR range, as an array of {sha, subject, paths, diffPath}. Use it for commit-level rules such as [EXTERNAL] subject checks. Read the referenced diffPath file only when you need the exact diff for that commit.
/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. Azure and 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 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/commits.json') to see commit subjects and touched paths. When a rule depends on the exact contents of one commit, read the listed diffPath for that commit instead of trying to run git show.
• 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/, docs/PR_REVIEW_CHECKLIST.md, or docs/bump_triton_version.md

These paths are the workflow's "trust perimeter": their workspace contents have been replaced with the trusted default-branch versions by an overlay step that runs before this skill, because their semantics are what decide whether secrets are protected at runtime and what reviewers flag as findings (.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; docs/PR_REVIEW_CHECKLIST.md is the single source of truth for the Critical / Major / Minor review tiers categorization you apply in Step 3; docs/bump_triton_version.md is the detailed Triton-bump guide you apply in Step 4).

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
docs/bump_triton_version.md	/tmp/pr-source/docs/bump_triton_version.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 these 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 and docs/bump_triton_version.md are NOT in Layer 3's perimeter regex (they are docs files, not security-sensitive), so a label-trigger PR may legitimately diff them -- still review the PR-side versions under /tmp/pr-source/docs/; the workspace copies are the trusted versions your review criteria 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 for review tiers. The Triton bump guide reaches you through docs/bump_triton_version.md. The workflow loads both for you in two ways, sourced from the same default-branch ref:

1. The snapshot_review_checklist workflow step reads the overlaid, trusted files at runtime and substitutes their content into the prompt heredoc between the <BEGIN/END docs/PR_REVIEW_CHECKLIST.md> and <BEGIN/END docs/bump_triton_version.md> markers (the canonical reference sections above this skill in your conversation). You already have them in context -- no Read needed.
2. The same files are overlaid into the workspace (see the Special case section above), so Read('docs/PR_REVIEW_CHECKLIST.md') and Read('docs/bump_triton_version.md') return the same bytes if you want to confirm.

Both channels come from the same files at the same workflow run, so they are byte-identical by construction. Categorize each finding against the Critical / Major / Minor tiers and the license-header template defined in the PR review checklist.

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 below applies the rocmlirTriton-specific sections of the same file (vendored Triton and Triton-pinned LLVM subtree updates, downstream patch records, rock::* hardware-feature detection, bridge passes, fat-library + MIGraphX, and the rocMLIR back-port check).

---

Step 4 -- Apply the rocmlirTriton-specific review rules

docs/PR_REVIEW_CHECKLIST.md (injected into the prompt above by the snapshot step and overlaid into the workspace; see Step 3) documents two rocmlirTriton-specific sections that don't fit the generic LLVM/MLIR tiers. docs/bump_triton_version.md is also injected and overlaid; apply it as the detailed source of truth when a PR imports new upstream Triton/LLVM revisions or changes downstream patch records:

• ## rocmlirTriton-specific checks -- vendored Triton and Triton-pinned LLVM subtree updates (delegating detailed steps to docs/bump_triton_version.md), downstream patch records, [EXTERNAL] commit boundaries for external/triton / external/llvm-project edits, rock::* hardware-feature detection (vs. triton::AMD::TargetInfo), bridge passes between Rock and Triton, and fat-library + downstream MIGraphX coordination. Each sub-rule documents its severity inline (Major / Minor).
• ## rocMLIR back-port check -- the path list of files shared with ROCm/rocMLIR, the rocmlirTriton-only path list, and the verdict logic for missing back-port notes (Major when none of options a/b/c is in the PR description). Compute the intersection of meta.files[].path against the path list there.

Apply both alongside Step 3. Cite the matching section heading in the finding body (for example: "rocmlirTriton-specific -- Triton subtree updates: librockcompiler_deps.cmake not regenerated") so the author can look up the rationale.

For vendored-subtree changes, always read /tmp/pr/commits.json:

• A downstream commit whose paths includes a path under external/triton/ or external/llvm-project/ must have a subject beginning with [EXTERNAL]; otherwise raise a Major finding under "rocmlirTriton-specific checks -- Vendored Triton / LLVM subtrees". Do not flag upstream import / bump commits for lacking [EXTERNAL]; those are covered by the Triton / LLVM subtree update checks.
• If the PR also changes triton-patches/*.patch or llvm-patches/*.patch, compare those patch-file contents with the relevant downstream [EXTERNAL] commit's diffPath. A patch record that does not match the corresponding [EXTERNAL] commit diff is a Major finding.

---

Step 5 -- 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 --