Review a PR

A reviewer finds what matters, says it, and stays silent elsewhere. Prove nothing; assert what you found.

Setup

1. Read CLAUDE.md, docs/engineering/ (principles.md, coding-standards.md, coding-agents.md, relevant specialized standards), and the docs/patterns/ files for the concerns this PR touches.
2. Read the linked issue (gh issue view <n>) and the full diff (gh pr diff <n>).
3. Check out the branch (gh pr checkout <n>), seed the worktree (copy .env if present, then uv venv --python 3.12 && make install), run make check. It ends by naming the docs-site pages this diff affects: open every page the PR left unchanged and confirm it is still true. No CI job does this; it is where stale prose is caught (#409 shipped five stale references past a ticked box).
4. Run uv run scripts/quality_delta.py and PR_BODY="$(gh pr view <n> --json body -q .body)" uv run scripts/concept_budget.py. Hold the PR to its own ## Expected delta and ## Concept budget, which ship-pr Stage 1 wrote into the draft body before the first source edit and the design gate already passed. Unexplained overrun beyond roughly 2x runs the overrun pass (references/overrun.md); the number itself is never the finding. A body carrying no estimate is a contribution that never ran Stage 1: report the measurement and skip the trigger.
5. Sibling comparison: for each changed or new module, pick 2-3 canonical siblings from uv run scripts/repomap.py (same package, same responsibility class), read them, and verify the PR's own ## Analog claim against the code.

Phases

Work in order. Correctness before craft: a wrong design makes naming irrelevant. Copy this checklist into your own response and check off each phase as you complete it:

Review progress:
- [ ] 1. Claims: what does this PR assert, and what must be true
- [ ] 2. Attack: go after those claims (references/attack.md)
- [ ] 3. Craft: patterns, naming, tests as spec (references/craft.md)
- [ ] 4. Scope: should this be two PRs
- [ ] 5. Verdict: post it (references/templates.md)
- [ ] 6. Harvest: usually nothing (references/craft.md)

Harvest

The default is to write nothing. Most reviews produce neither output below. docs/patterns/ is 106 lines total across four files; that shortness is what makes it worth reading, and a reviewer appending once per PR would double it in a week. Pattern entries are not scored by volume: the measure is whether a later review cites one, never how many exist. Full bar and rationale: references/craft.md.

• A docs/patterns/ entry, only when all four of references/craft.md's tests pass: it recurred, it generalizes past this module, it is a real cited good/bad pair, and it is not already covered by the four existing files. Extending an existing file beats creating one. You never commit it: open an issue containing the finished entry text and the file it belongs in, and a dev agent lands it. A reviewer that commits has reviewed nothing, and taste belongs under review like any other change.
• A harness note, only when the same dev-agent behavior (not output) has appeared in two PRs. Establish that with uv run scripts/finding_tags.py, which counts the tag: lines your own findings and every earlier review's findings carry, not with recall: across sessions a reviewer has no memory, and before the tags existed this bar was reachable only by luck. File the note the same way as a DEFER (finding: <gap> issue) but name the behavior, not a proposed rule. Not a new mechanical guard: guards caught 0 of 8 v5 findings, and three of those findings were themselves guard proposals.

Tagging is not optional, and it is the one thing Harvest always does. Every BLOCK and DISCUSS carries a tag: line naming the dev-agent behavior (references/templates.md, Template A). Run uv run scripts/finding_tags.py before you write findings and reuse a tag from the histogram whenever one fits: a tag invented per finding can never reach a count, and a count is the only thing tags are for. Nothing is written to the tree; the comment you post is the record.

Anything that surfaces but fails its bar is a NIT, or it is nothing. It is never a pattern.

Finding classes

BLOCK / DISCUSS / DEFER / NIT replace ERROR / WARNING / NOTE. Each is defined by what happens next, not by severity. Worked examples: references/classify.md.

• A major feature that leaves one of Q1-Q7 (docs/engineering/principles.md) unanswered is a DISCUSS, and what would settle it is the answer. Q4 is the one the project fails most: a PR that moves work to the caller and does not say so has failed it whatever the diff looks like.
• DISCUSS resolves when the author answers, not when the author agrees. Disagreement after an answer goes to the user; neither side rules.
• A DISCUSS must state what would settle it. A shrug is not a DISCUSS.
• A public API finding classifies on what you are asserting, never on what the fix touches. A choice among several valid shapes is a DISCUSS, where a reviewer holds no more authority than the author. A demonstrated broken contract is a BLOCK even when the fix changes a signature.
• A NIT needs no reply. Collect NITs in one list at the bottom of the verdict, never inline.
• A cause the overrun pass names carries no class of its own and classifies by the rules above like any other finding, the public-API rule included. Whatever the class, the fix is the design: splitting a PR whose size came from a missing concept moves the bloat rather than removing it.
• Cost is folded into the class, not a separate axis. Real, expensive, and not a regression is a DEFER; introduced by this PR is a BLOCK regardless of cost.
• DEFER is not a rejection and does not require present actionability: a finding that only informs a later bug fix or feature is still worth filing. NIT means small and optional now; DEFER means real and later.
• DEFER is where the promotion loop lives: file finding: <gap> with the finding label for a real out-of-scope defect, citing evidence. Never file a DEFER proposing a new mechanical guard; guards caught 0 of 8 v5 findings.

Report format

Every comment and the verdict body follow fixed templates: references/templates.md. Do not improvise structure, length, or style; it is all decided there, including the exact marker line.

Delivering the review

• Reviews are COMMENTED only. Approve and request-changes are unavailable when the agent authenticates as the PR author.
• Inline, line-anchored comments (BLOCK, DISCUSS, DEFER; NITs never inline) in Template A form: gh api repos/{owner}/{repo}/pulls/<n>/comments -f body=... -f commit_id=$(gh pr view <n> --json headRefOid -q .headRefOid) -f path=<file> -F line=<line> -f side=RIGHT
• Post the verdict as a real review in Template B form: gh pr review <n> --comment --body-file <file>. Pass requires zero BLOCK and every DISCUSS answered.
• Post measured against declared with the verdict, one line, so the author sees the delta against the estimate Stage 1 wrote. The raw script output stays in the return to the orchestrator.
• The review is only delivered once it is on the PR. A local pass with nothing posted is incomplete.
• A push that changes the code needs a new review: nothing enforces that mechanically, so it is on whoever merges.
• File DEFER and harness-note issues per the finding-class table above.

Return to the orchestrator

Template C form (references/templates.md): verdict, make check result, counts per finding class, links to the posted review and any issues filed, then the evidence, uncapped, raw, no prose (mutation results, measured delta vs the PR's declarations, sibling-comparison notes, probe limits, any docs/patterns/ commit).