Multi-Pass PR Code Review
This skill performs 3 iterative review passes over a pull request diff with escalating focus: general defects, cross-file interactions, then absent behavior. After all passes, a final summary deduplicates, validates, assigns severity, and filters to Critical/High/Medium. The output includes clickable deep links to the relevant code on GitHub so the developer or reviewer can easily navigate to each finding.
PR and Diff Resolution
- Run
gh pr view --json number,baseRefName,state,isDraft. This single call determines whether a PR exists and, if so, its state. - If a PR exists:
- If the PR is closed or a draft, inform the user and stop.
- Otherwise, use the PR's
baseRefNameas the base for the diff.
- If no PR exists (the command fails):
- Use
mainas the default base branch (the repo's mainline; feature work anddevelopare PR'd into it). - Inform the user: "No PR found. Diffing against
main."
- Use
- Get the diff via
git diff <base>...HEAD. - Resolve the full HEAD SHA via
git rev-parse HEAD. - Resolve the repository owner/name via
gh repo view --json owner,name(fall back to parsinggit remote get-url originifghis unavailable). Store the owner, repo name, full SHA, and PR number (ornullwhen no PR exists) for constructing deep links in the output.
Context Gathering
Before starting the review passes, gather context to understand the intent behind the changes:
- Branch and commits: Read the current branch name via
git branch --show-currentand the commit subjects in range viagit log --oneline <base>..HEADto understand the stated intent. - Full file reads: Do NOT read full files upfront. Instead, during each pass, if a diff hunk lacks sufficient surrounding context to assess a potential defect, read the full file at that point using the Read tool.
Design Review (before passes)
After gathering context and the diff, but before starting the defect passes, evaluate the overall design of the change:
- Scope: Does the PR implement more or less than its stated intent (branch name, commit subjects)? Flag scope creep or missing pieces.
- Placement: Do the changes live in the correct layer? This repo is a
PowerShell wrapper: build/orchestration logic belongs in
*.ps1at the repo root orexamples/; model configuration belongs inpresets/*.ini; no original C/C++/Python source belongs here, and nothing undervendor/llama.cpp/is expected to persist (each rebuild wipes it). - Complexity: Is any part of the change more complex than necessary? Flag over-engineering, unnecessary abstractions, or functionality that is not required by the change's intent.
Record design-level findings separately under ### Design in the output,
before the severity tables. Design findings do not go through the severity
classification; they are qualitative observations for the author.
Workflow and User Communication
Use the TodoWrite tool to create and track these steps:
- Resolve PR, base branch, and gather context
- Design review
- Pass 1
- Pass 2
- Pass 3
- Build final summary (deduplicate, re-examine, severity, filter); internally only
- Print the report in the terminal
Mark each todo as in_progress when starting and completed when done.
After each pass, tell the user in one line how many new defects were found
(e.g., "Pass 1 complete; found 6 defects.").
After Pass 3, build the final summary internally; do not print it yet. Once the summary is complete, print the report body in the conversation as the turn's final output, following the Report Output section below. Nothing (no recap, no "Let me know if…", no "Review complete" line, no follow-up commentary) may be printed after the report. The report is the terminal output of the turn; any follow-up question opens a new turn.
Review Passes
Three passes over the full diff, each with a different focus. All passes review the full diff. Findings are recorded without severity: just File, Line(s), and Description.
Only flag defects in lines that are added or modified in this PR. Do not flag issues in unchanged context lines, even if they appear in diff hunks.
Skip any finding that:
- Matches a finding from a previous pass (same file and overlapping line range).
- Describes the same logical issue as an existing finding but references a different location (e.g., a function definition vs. its call site). Two findings about the same root cause count as one; keep the one closest to the root cause.
Pass 1: General scan Review the diff. Report all defects: bugs, logic errors, security issues, bad practices, missing validation, incorrect error handling. Also check the project-specific concerns listed in the Project-Specific Review Checklist section below. Only flag defects in lines that are added or modified in this PR.
Pass 2: What was missed Review the diff again, assuming defects were missed on the first pass. Focus on interactions between changed files, subtle logic errors, and implicit assumptions in the code. Only flag defects in lines that are added or modified in this PR.
Pass 3: What the code does NOT do Assume there are still undiscovered defects. Focus on what is absent: missing error handling, missing edge cases, missing input validation, missing null checks, race conditions, resource leaks, and incorrect assumptions about state. Only flag defects in lines that are added or modified in this PR.
Track findings internally across passes (in conversation context). The format for each finding is: File, Line(s), Description.
False Positive Exclusion List
Do NOT flag any of the following:
- Pre-existing issues not introduced in this PR's changes.
- Code that appears to be a bug but is actually correct.
- Pedantic nitpicks that a senior engineer would not flag.
- General code quality concerns unless explicitly required in AGENTS.md.
- Issues explicitly silenced in code (e.g., via a lint ignore comment).
- Pure code style or formatting preferences.
- Potential issues that depend on specific inputs or runtime state.
- Findings inside
vendor/(the submodule is wiped and re-checked-out each rebuild; only the documented idempotent CMakeLists shim is sanctioned).
Project-Specific Review Checklist
These are architectural and safety concerns specific to this PowerShell wrapper around llama.cpp; there is no linter to catch them. Check for these during all passes, in addition to general defect scanning. They are NOT style issues; they are correctness and safety rules. Each maps to a rule in AGENTS.md ("Non-obvious behavior", "Presets", "Changelog style").
- Ephemeral submodule edits: Changes under
vendor/llama.cpp/that expect to persist. Eachrebuild_llama.cpp.ps1resets the submodule toorigin/masterand re-checks-out; the only sanctioned edit is the idempotent OpenBLASCMakeLists.txtshim the build re-applies. - Submodule "cleanup": Removing
.gitmodulesignore = dirty, or resetting thevendor/llama.cpppointer casually. The dirty state is by design. - PowerShell argument passing: Values that can contain spaces routed through
an
Invoke-Expression-built command string or the-additionalArgumentswhitespace-split/re-pair parser (they will not survive); unquoted space-bearing paths inllama-server/pythoninvocations. - CWD-relative path assumptions:
chat-template-fileandread_file()resolve against the launch CWD, not the INI directory. Scripts that shell out to vendored Python (gguf_dump.py,speed_bench.py) must guard the path against upstream relocation. - Build-detection invariants: CUDA is selected only when both
nvidia-smiandnvccare present; do not drop-DCMAKE_ASM_COMPILER=ml64(CMP0194 ASM breakage); do not defeat the SMT-aware--parallelcap or the running-build-tree-process abort. - Python requirements layering: Edits to
requirements_override.txtthat break the documentedtorch(cu126),transformers,numpy<2.3, ortiktokenpins. - Preset INI semantics:
mmproj-offload = truesilently OOMing CLIP warmup on a saturated GPU; dropping a requiredchat-template-filepin (it replaces the GGUF-embedded template, not redundant withjinja = true); stalespec-typeflag names. - CHANGELOG style: Entries that are not one physical line, omit the
[Component] <verb> <thing>form, or carry rationale/paths/line numbers.
Final Summary
After Pass 3:
- Deduplicate: Two findings are duplicates if they reference the same file and overlapping line ranges, or if they describe the same root cause at different locations. Keep the more detailed description.
- Re-examine: For each remaining finding, verify it against the full file context. If the file was not already read during a pass, read it now using the Read tool. Remove any finding that cannot be confirmed after re-examination.
- Confidence filter: Remove any finding where confidence is below 80% that it is a real defect. When in doubt, exclude.
- Assign severity using these definitions:
- Critical: Data loss, security breach, corruption of state, a build that destroys a working tree, or a crash on a common path.
- High: Incorrect behavior under normal usage, unhandled error paths that will be hit, significant logic flaw.
- Medium: Bad practice that could lead to bugs, missing validation for unlikely but possible inputs, minor logic issue.
- Filter: Keep only Critical, High, and Medium.
- Print: Print the deduped, severity-filtered findings directly in the
terminal, following the Report Output section below. When the filtered
set is empty, print a single line
No Critical/High/Medium findings.in place of the severity tables; the metadata table and Design section still print normally. When the set is non-empty, print one table per severity level (omitting empty levels), following the link format rules in the Link Format section below.
No data is written to GitHub. The developer or reviewer uses the output to manually create PR comments.
Link Format
This section defines how links appear in the output tables. Follow these rules exactly.
Columns: File, Description. The File column contains a markdown link. The
link label is the file path relative to the repository root with a leading
/, plus the line range in :{start}-{end} format (e.g.,
/examples/speed-bench.ps1:41-43). The link URL is the full GitHub
URL constructed as described below.
Line range: Include 1 line of context before and after the finding. A
finding on line 42 links to lines 41-43. A finding spanning lines 42-45
links to lines 41-46. The label always uses :{start}-{end} regardless of
whether the URL uses L or R anchors.
SHA-256 hash for PR links: Compute the SHA-256 hex digest of each unique
file path via Node.js (cross-platform):
node -e "process.stdout.write(require('crypto').createHash('sha256').update('{path}').digest('hex'))".
When a PR exists: Use PR deep links. The URL format is
https://github.com/{owner}/{repo}/pull/{pr-number}/files#diff-{sha256}R{start}-R{end}.
When no PR exists (fallback): Use blob links with the full HEAD SHA:
https://github.com/{owner}/{repo}/blob/{full-sha}/{path}#L{start}-L{end}.
Note: blob links use L (not R) for line anchors.
Correct: markdown links with file path labels.
| File | Description |
| --------------------------------------------------------------------------------------------------------------- | ---------------------------- |
| [/examples/speed-bench.ps1:41-43](https://github.com/owner/repo/pull/42/files#diff-a1b2c3d4e5f6a7b8c9d0R41-R43) | Unquoted path breaks on space |
| [/examples/speed-bench.ps1:41-43](https://github.com/owner/repo/blob/4a7c9e1f/examples/speed-bench.ps1#L41-L43) | Unquoted path breaks on space |
Wrong: do NOT use any of these formats.
| https://github.com/owner/repo/pull/42/files#diff-a1b2c3d4e5f6a7b8c9d0R41-R43 | ... |
| [https://github.com/...](https://github.com/...) | ... |
| [speed-bench.ps1:41-43](https://github.com/...) | ... |
The first is wrong because it uses a raw URL instead of a markdown link. The
second is wrong because the label is a URL instead of a file path. The third
is wrong because it uses a filename only; the label must be the full path
from the repository root with a leading /.
Report Output
The report is printed directly in the terminal as the turn's final output; no
files are written and no trailer follows it. Print the body in this fixed
order: (1) the title (# PR Review then ## <branch> (vs <base>)),
(2) the metadata table, (3) the ### Design section, (4) the
severity tables (or the empty-findings line). Nothing may be printed after the
report.
All tables (the metadata table and every severity table) must have every cell (header, separator, body) padded with spaces (or dashes for the separator row) so all cells in the same column occupy the same character width between pipes. This keeps the report scannable in the monospace TUI.
The metadata block is a 2-column GFM table with explicit Field / Value
headers. Severity glyphs (🔴 🟠 🟡) keep the Findings row scannable without
custom styling.
Metadata-row formatting rules:
- Branch: link both branches to their GitHub tree URLs:
https://github.com/<owner>/<repo>/tree/<branch-name>. Multi-segment branch names (e.g.feature/foo) work without encoding because thetree/path accepts literal slashes; percent-encode any branch name that contains spaces or other reserved characters. - HEAD: use the 7-char short SHA in both the link label and the URL
(
/commit/<head7>). The short SHA keeps the row narrow enough that the OpenCode TUI does not wrap the Field column. GitHub redirects/commit/<short>to the full commit, so the link resolves identically. - PR: the PR number linked to its GitHub URL when one exists
(
[#42](https://github.com/<owner>/<repo>/pull/42)),noneotherwise. - Diff: plain text, no colour, no emoji. Format:
<files> files · +<additions> · −<deletions>, with middle dots (·, U+00B7) as separators. Use the Unicode minus sign (U+2212,−) for the deletions count, not the ASCII hyphen-minus, for typographic correctness.
When the severity-filtered set is empty, print No Critical/High/Medium findings. in place of the severity tables; the metadata table and Design
section still print.
Full report template:
# PR Review
## develop (vs main)
| Field | Value |
| --------- | ---------------------------------------------------------------------------- |
| Branch | [`develop`](https://github.com/owner/repo/tree/develop) → [`main`](https://github.com/owner/repo/tree/main) |
| HEAD | [`head7`](https://github.com/owner/repo/commit/head7) |
| PR | [#42](https://github.com/owner/repo/pull/42) (or `none` when no PR is open) |
| Generated | YYYY-MM-DD HH:MM |
| Diff | 6 files · +390 · −0 |
| Findings | 🔴 0 Critical · 🟠 1 High · 🟡 6 Medium |
### Design
- <Qualitative observation about scope, placement, or complexity>
### Critical
| File | Description |
| ------------------------------------------------------------------------------------------------------------- | -------------------------- |
| [/rebuild_llama.cpp.ps1:41-43](https://github.com/owner/repo/pull/42/files#diff-a1b2c3d4e5f6a7b8c9d0R41-R43) | Destroys tree on running proc |
### High
| File | Description |
| ---------------------------------------------------------------------------------------------------------- | ------------------------- |
| [/examples/server.ps1:14-19](https://github.com/owner/repo/pull/42/files#diff-f6a7b8c9d0a1b2c3d4e5R14-R19) | Unhandled startup failure |
Constraints
- Do NOT modify source code.
- Do NOT post comments, reviews, or any data to GitHub. This skill is advisory with respect to GitHub; it never writes there.
- This skill is read-only: do NOT write any files. The report is printed in
the terminal only. Do not touch
.claude/, the repo root, orvendor/. - Do NOT report pure style or formatting preferences.
- Do NOT report issues in generated files, lock files, or changelog entries.
- Do NOT report findings inside
vendor/. - File cells in output tables must use markdown link syntax with the file path as label; see the Link Format section.
Source: countzero/windows_llama.cpp — distributed by TomeVault.