pr-review-kit

A complete, self-contained playbook for reviewing a pull request. Hand this directory to a fresh reader — human or Claude session — along with the target PR, and they have everything needed to do a rigorous review without prior context. This file is the spine: the phases and the four verification disciplines, which is all a fresh review needs. Two sidecars carry what only some reviews reach for — ENGAGE.md for replying to existing reviewer comments, and APPENDICES.md for situational reference — so a routine review does not pay to load them.

How to use this document

You are a new reader. This kit does not assume:

• You know about other skills, agents, or memory entries in any parent repo.
• You know the project's design conventions.
• You know the PR author's review style.
• You have any prior session history.

It assumes only that you have:

• gh CLI authenticated to the relevant org.
• git with worktree support.
• File read/write access in the reader's home dir.
• (Optional) Specialty subagents available via the harness — generic instructions below cover dispatch when they exist.

Two reading modes:

• Mode A — Doing a review yourself. Read top-to-bottom; phases are sequential.
• Mode B — Coordinating multiple reviews. Skip to Phase 7 (output format) — the per-PR notes file shape is the durable artifact you'll keep.

Phase 1 — Read the PR state cold

Before forming any opinion, build a complete picture of the PR's metadata, review history, and commit history. Run these in parallel:

OWNER=<github org>
REPO=<github repo>
N=<PR number>

# Core PR metadata (title, author, size, state, mergeable)
gh pr view $N --repo $OWNER/$REPO \
  --json number,title,author,state,additions,deletions,changedFiles,baseRefName,headRefName,createdAt,updatedAt,reviewDecision,mergeable,body

# File list with per-file delta size
gh pr view $N --repo $OWNER/$REPO --json files \
  --jq '.files[] | "\(.additions)+/\(.deletions)- \(.path)"'

# All review submissions (humans, AI bots)
gh api repos/$OWNER/$REPO/pulls/$N/reviews \
  --jq '[.[] | {user: .user.login, state, submitted_at, body_preview: (.body[0:200])}]'

# All inline comments — note the in_reply_to_id; resolved threads still appear
gh api repos/$OWNER/$REPO/pulls/$N/comments \
  --jq '[.[] | {id, path, line, user: .user.login, in_reply_to_id, created_at, body_preview: (.body[0:200])}]'

# Commit history, newest first
gh api 'repos/'$OWNER'/'$REPO'/pulls/'$N'/commits?per_page=100' --paginate \
  --jq 'sort_by(.commit.author.date) | reverse | .[] | "\(.sha[0:10]) \(.commit.author.date) \(.commit.message | split("\n")[0])"'

# If state is CLOSED but mergedAt is null, find why
gh api repos/$OWNER/$REPO/issues/$N/comments \
  --jq '.[-5:] | .[] | {user: .user.login, created_at, body_preview: (.body[0:300])}'

Key thing to notice in this phase: who has reviewed, what they said, and how the PR has evolved over time. PRs by authors who run their own AI-bot self-review often have many inline comments that look "open" but are pre-merge punch lists already addressed by later commits (see Phase 4).

Staleness gate — does the PR still apply to base?

Always run this before reviewing the diff. A PR that renders a clean diff on GitHub may be unmergeable against current base if the surrounding code was refactored under it. For any PR more than a few days old, this is often the load-bearing finding — and no other phase surfaces it.

# Run git commands against the canonical remote (see Phase 2 on fork-vs-upstream).
git fetch <upstream-remote> pull/$N/head:pr-$N-review
MB=$(git merge-base pr-$N-review <upstream-remote>/<base>)
git rev-list --count $MB..<upstream-remote>/<base>   # how many commits behind base

# Drift on touched files — a changed signature means the patch will not apply.
for f in $(git diff --name-only $MB pr-$N-review); do
  diff -q <(git show <upstream-remote>/<base>:"$f" 2>/dev/null) <(git show pr-$N-review:"$f") >/dev/null || echo "diverged: $f"
done

A large commit count behind base plus signature/API drift on a touched function means the fix idea may survive a rebase but the patch does not apply — report it as a blocker distinct from code-quality findings; it is what a mergeable: UNKNOWN status usually reflects.

Phase 2 — Clean working copy via worktree

For PRs over ~500 LOC or any structural change, create a git worktree at the PR's HEAD so you can read the actual code without disturbing your main working directory.

git fetch <upstream-remote> pull/$N/head:pr-$N-review
git worktree add <path-to-new-worktree> pr-$N-review

If <upstream-remote> is unclear: git remote -v and pick the one pointing at the canonical org/repo (often upstream, not origin if you work from a fork).

Clean up when done:

git worktree remove <path-to-new-worktree>
git branch -D pr-$N-review

Phase 3 — Find the design intent

A PR is an encoding of a design decision. Without the design, you can only review syntax. Hunt for the design intent in this order:

1. The PR body itself — many authors embed design rationale, link the spec, or cite an RFC. Read it fully before assuming it's absent.
2. Linked tickets — Linear, GitHub Issues, Jira. The ticket description often contains acceptance criteria + scope decisions.
3. In-tree design docs — docs/, design/, proposals/, rfcs/. Search the repo (find . -name '*.md' | xargs grep -l 'design\|TRD\|RFC').
4. Out-of-tree design docs — Notion, Confluence, Google Docs, Slack file attachments. If the PR body cites "TRD §X" or "spec §Y" without a link, that doc is somewhere — ask the user.
5. Adjacent PRs — same author's prior or sibling PRs in the same area. Look for a "design" or "stubs" PR in the stack.

Stop here and ask the user if you can't find it. Reviewing without design intent produces shallow critique. "This code does X" is verifiable; "this code should do Y instead of X" requires knowing what Y is per the design.

Phase 4 — Verification disciplines (non-negotiable)

These four rules govern every claim you make about the PR. They're learned from prior review-session failures; each one corresponds to a specific class of error.

Rule 1 — Cite primary sources, or label inferences

When you make a claim about an RFC, spec, library behavior, or vendor API:

• Either cite the source: URL + section + quoted text. "RFC 7644 §3.5.2 says PATCH operations MUST be atomic per [link]."
• Or label the inference: "Inferred from library documentation, not verified against the RFC text."

Why this rule exists: a prior session had an agent claim "RFC 7644 doesn't specify operator precedence" — based entirely on reading three library docstrings that called the precedence rule "convention." The RFC actually does specify it (with a MUST clause in prose below the ABNF). The agent had no access to read the RFC and presented an inference as fact. The next reviewer would have inherited that wrong claim.

The generative half of Rule 1. Labeling tells you what to do with a claim you already hold. This tells you how to earn one in the first place:

Never assert third-party behavior from memory. ORM tag semantics, which plan the query planner will pick, which driver parameter actually takes effect, how a proto field lands on the wire, what a language feature does at an edge — these are the claims that feel most certain and are wrong most often, because what you remember is the common case and the finding always lives in the uncommon one.

Three things before such a claim leaves your hands:

1. Name the layer that actually controls the behavior. The annotation, the driver, the connection string, the server-side default, and the dialect each get a vote, and only one of them decides. "There is a json: tag on the field" is not a mechanism; "the encoder at marshal.go:212 reads that tag and drops the zero value" is. A type name, an annotation, or a nearby symbol is a place to look, not a cause.
2. Read that layer's official documentation — WebFetch or WebSearch the vendor's own page, not a blog post recounting it — and cite the URL plus the section. If you could not reach it, say the claim is inferred, and from what.
3. Run it when running it is cheap. A five-line scratch program, one targeted test, an EXPLAIN ANALYZE, a throwaway container: each turns a remembered claim into a quoted output. Cite the output, not your memory of it.

This applies symmetrically. The same discipline that stops you shipping a wrong finding is what lets you confirm the author's claim — "the driver does coerce this, see the vendor doc §4.2" is worth as much as a defect, and it is the half reviewers skip because being right feels like finding nothing.

Rule 2 — Walk commits forward; review-thread state ≠ code state

Before claiming "bug X is still open at HEAD" based on an inline comment showing as unresolved:

# Get the comment's created_at timestamp first
gh api repos/$OWNER/$REPO/pulls/$N/comments --jq '.[] | select(.id == <comment-id>) | .created_at'

# Scan commits AFTER that timestamp for matches against the cited file/issue label
gh api 'repos/'$OWNER'/'$REPO'/pulls/'$N'/commits?per_page=100' --paginate \
  --jq 'sort_by(.commit.author.date) | reverse | .[] | "\(.commit.author.date) \(.commit.message | split("\n")[0])"' \
  | head -30

If a 2026-05-21 commit says fix(scope): address CI-1 ... and the comment is from 2026-05-19 about CI-1, the bug is closed in code even though the thread is open in GitHub. Inline comments persist as discussion artifacts; they don't auto-resolve.

Strongest verification: read the actual current code at the cited file:line. Bug in code now → comment still valid. Bug not in code now → comment is historical.

Rule 3 — Investigate, don't ask permission to look

If a question can be answered by reading a file, running gh, or grepping the repo: do it. Don't ask the user "want me to pull X?" or "should I check Y?" — that wastes turns and signals lack of agency.

The exception: actions with side effects (posting reviews, force-pushing, sending messages, deleting work). Confirm before those. Reading and investigation are not side effects.

Also: drop verbal tics like "honest take," "my honest read," "honestly." They add nothing. If a take needs hedging, hedge specifically — "I haven't verified this against the code" — not vaguely.

Rule 4 — Verify actual artifacts, not summaries

When asked to investigate or validate something, check the actual state — code, logs, DB, the file on disk — first. Do not summarize the spec or PR description back to the user and call that investigation. The summary is what's claimed; the artifact is what's true.

Phase 5 — Dispatch agents (when the harness has them)

For PRs with significant scope or structural risk, parallel specialty agents add value over solo review. Generic shapes (your harness may name them differently):

Prompt design rules for any agent:

1. State the working directory and the exact HEAD SHA the agent should read from. Otherwise it'll read main and produce stale findings.
2. For agents claiming RFC/spec compliance: require URL + section + quoted text, or explicit "inferred from secondary sources" labeling.
3. For fresh-eyes agents: exclude the PR description from the prompt. Otherwise the agent anchors on the author's framing.
4. Tell the agent the user's intent (review-only / produce-fixes / write-tests) — agents not told this often default to writing code.

Phase 6 — Verify before relaying

Every agent claim that gets relayed to the user must be verified by you against the actual code or design doc. Agent confidence is not evidence.

Failure modes to expect:

• Misidentified targets: agent searches for "helpers" and finds the wrong helpers. Example: a discovery agent flagged a PR author as having lied about "inlining helpers" when the author had inlined a different set of helpers (toMap/toSlice) than the agent looked at (compareBool/compareString).
• Stale framing: agent reads inline comments as a punch list, doesn't walk commits forward to check fixes. Rule 2 catches this.
• Phantom citations: agent cites "RFC §X" without quoting; Rule 1 catches this.

When an agent says X, your reply should not be "the agent found X." It should be "X is verified: file.go:42 has the cited code" or "the agent claimed X but file.go:42 shows Y — drop the claim."

Phase 7 — Output format: the falsifiable matrix

The durable artifact of a review is a per-PR notes file. Format below. Each row is structured so a future reader can independently verify the claim in 30 seconds — no judgment call required, just a check.

Where the notes file goes. Resolve the destination in this order, and state which one you used when you hand the review over:

1. $HUD_ROOT/projects/<project>/prs/PR-N/review.md when HUD_ROOT resolves — the env var if set, else ~/hud if that directory exists. This is the default, and it is the HUD consolidation rule applied: a review artifact is a note, and notes live in exactly one tree — the one that is backed up, indexed, and searchable months later when you need the matrix again. The PR folder is the PR's one home: a second review of the same PR replaces review.md rather than adding a sibling, and anything else about that PR (reply drafts, follow-up notes) goes in the same folder, not in notes/.
2. /tmp/<owner>-<repo>-review/PR-N/review.md only when no HUD tree resolves. A scratch path is an honest "this is not being kept"; say so rather than letting the author assume it was filed somewhere.

Two destinations are wrong regardless of which branch you took. Never a browser's download directory — it is unbacked-up, unindexed, and invisible to every later search, so work content put there is work content lost. Never inside the repo under review, which is how a private review note becomes a commit. And never inside the agents/skills repo either: that repo holds the review machinery, not the output of running it.

Consistency across reviews is the point of having one destination rather than picking each time: future-you greps their own past matrices, and can only do that if they all landed in the same tree.

# PR #N — [title] ([ticket-id])

| Field | Value |
|---|---|
| Author | [github handle] |
| Size | +X / -Y, Z files |
| State | OPEN / CLOSED / MERGED |
| Self-review | N AI bot passes, M open inline comments |
| Linked design doc | [path or URL] |
| Notes-author | [your handle] |

## What it does (one paragraph)

[Plain-language summary. Not the PR title, not the body — your own words after reading the code.]

## Design anchor

| Source | Section | What it requires |
|---|---|---|
| [Design doc] | §X | "[quoted requirement]" |
| [RFC] | §Y | "[quoted requirement]" |
| [Threat model] | R-N | "[quoted risk]" |

## Falsifiable matrix

Each row: claim, where to verify, current verdict, who verified.

| # | Claim | Evidence location | Verdict | Verified by |
|---|---|---|---|---|
| 1 | Field X exists at proto field number Y | `path/to/file.proto` line Z | ✅ verified | [your handle], [date] |
| 2 | Code at file:line implements design requirement A | `path/to/file.go:L42-L60` | ✅ matches design | [your handle], [date] |
| 3 | RFC §X.Y requires behavior B | [RFC URL] §X.Y para 3 | ✅ design exists | [your handle], [date] |
| 4 | PR description claims P, but code does Q | PR body §P; `file.go:L80` | ❌ mismatch | [your handle], [date] |
| 5 | Agent claimed C; code at L100 confirms | `file.go:L100` | ✅ agent right | [your handle], [date] |
| 6 | Agent claimed D; code at L120 contradicts | `file.go:L120` | ❌ agent wrong | [your handle], [date] |

**Verdict vocabulary** (use these exact strings, not opinion-shaped variants):

- `✅ verified` — claim is true; evidence read.
- `✅ matches design` — code matches design doc.
- `✅ design exists` — the design doc says what's claimed; code conformance is separate.
- `❌ mismatch` — code contradicts the claim or design.
- `⚠️ deferred` — claim is for code in a different PR / future ticket; not verifiable here.
- `⚠️ unverified` — couldn't verify in this session; verification commands attached below.

## Open gaps (deferred to other PRs)

- [Gap A]: [what it is] → [which future PR or ticket]
- [Gap B]: …

## Quick verification commands

```bash
[command 1 — explicit, runnable, produces evidence for matrix row N]
[command 2 — …]

One more required section, placed between the matrix and the open gaps:

<!-- @include-begin _shared/decision-ledger.md -->
**The decision ledger — what you asked, not just what you concluded.**

The matrix records claims that survived. The dimension sweep records coverage.
Neither records the *path*: which questions the review actually turned on,
which were settled, and which were abandoned. That path is what makes a wrong
review diagnosable — a reader who disagrees with the verdict can find the
question that was mis-answered instead of re-deriving the whole review.

```markdown
## Decision ledger

| # | Question | Settled by | Decision |
|---|---|---|---|
| 1 | Does the new guard run before the signature check? | call order at `handler.go:88` vs `:104` | No — `parse()` is first. → finding 1 |
| 2 | Is the migration safe for a rolling deploy? | does old code read the new column as NULL | **Unresolved** — deploy order not visible from the diff |
| 3 | Is the retry budget bounded? | `config.go` default + caller override | Yes, 1k via config. Dropped as a candidate. |

The middle column is the whole mechanism, and it is written before the answer exists. A question-and-decision log composed after the reasoning is narration: a plausible question gets invented for a conclusion already reached, and it reads exactly like the real thing. Naming what would settle the question first is a pre-commitment — the same discipline as citing primary sources, lifted one level up from findings to the questions that produce them. If you cannot say what would settle a question, you are not ready to ask it.

Three rules make this a gate rather than a diary:

1. Every finding traces to at least one question. A finding with no question behind it arrived by pattern-match, which is the thing finding-triage exists to catch.
2. Every question terminates in a decision or an explicit Unresolved. A question left hanging is a defect in the review, the same way a blank dimension row is.
3. Count it out. State both numbers: questions asked and decisions + unresolved. They must be equal. Arithmetic is checkable by a reader who did not watch the work; prose assurance is not.

Scope it to decisions that could have gone the other way. Anything that changed what you looked at next, and anything you dropped. A question at every trivial step drowns the signal, and the rows that matter most are usually the Unresolved ones — they are the review telling a future reader exactly where it stopped seeing.

This lives in the artifact, never in the posted comment. The author wants the tag, the claim, and the file and line; comment-craft's lead-with-the-verdict rule already settles that. The reasoning path costs a local file nothing, and the posted review gets shorter once the reasoning has somewhere else to live.

Why this shape: every row is a checkable claim, not opinion. A new reviewer can pick up the file, run the commands, and confirm the verdicts independently. Reviews built this way compound: each PR's notes are reusable input for the next PR's review.

Phase 8 — Post to GitHub (only when explicitly authorized)

Default posture: do not post. Notes stay local until the user explicitly says "post X." This is non-negotiable; posting is the only side-effect category that affects shared state visibly to others.

When authorized:

# Body-only review (approve / comment / request-changes)
gh pr review $N --repo $OWNER/$REPO --approve -F /tmp/body.md
gh pr review $N --repo $OWNER/$REPO --comment -F /tmp/body.md
gh pr review $N --repo $OWNER/$REPO --request-changes -F /tmp/body.md

# Reply to a specific existing inline thread
gh api repos/$OWNER/$REPO/pulls/$N/comments -X POST \
  -f body="$(cat /tmp/reply.md)" -F in_reply_to=<comment-id>

# Atomic batch: overall body + multiple new inline comments in one submission
gh api repos/$OWNER/$REPO/pulls/$N/reviews -X POST --input /tmp/review.json
# review.json shape:
#   {commit_id, event: "COMMENT|APPROVE|REQUEST_CHANGES", body,
#    comments: [{path, line, body}]}

Do not hand-build that payload for an applyable suggestion — an inline comment whose body carries a ```suggestion fence the author commits with one click. scripts/pr-suggest.sh assembles it, and refuses (nonzero exit) when the anchor is unchanged context, which is the one mistake that ships a dead "Commit suggestion" button. It is dry-run by default. Whether a finding should be a suggestion at all is judgment, and stays in skills/pr-suggestion; the script only enforces the mechanics.

After posting, verify it landed:

gh api repos/$OWNER/$REPO/pulls/$N/reviews \
  --jq '[.[] | select(.user.login == "<your-handle>")] | .[-1] | {id, state, submitted_at}'

Phase 9 — Engage mode: responding to existing reviewer comments

When the PR already carries inline reviewer comments — yours, or on a PR you're collaborating on — drafting replies is a distinct workflow from fresh review, and it lives in full in ENGAGE.md: §9.0 the reply-tracking file (the Code-addressed / Reply-posted two-state matrix), §9.1 comment triage, §9.2 the discovery-agent prompt, §9.3 response templates, §9.4 the permalink hard-gate, §9.5 present + authorize + post, §9.6 resolve threads + re-request review, §9.7 error handling. Same default-do-not-post posture as Phase 8.

Appendices

Situational reference, moved out of the spine so a review that doesn't need it doesn't pay for it. All five sections live in APPENDICES.md:

• Appendix A — identifying the author's review pattern. AI-self-review, stacked-PR, design-doc-citing, iterative-fix and description-light authors, and what each changes about your read.
• Appendix B — how to handle CLOSED-not-merged PRs. Finding the PR that superseded this one.
• Appendix C — memory and persistence. What is worth saving across sessions when the harness has memory.
• Appendix D — when to invoke the heavier review skills. Structural (mache/modmap) review and multi-agent panel synthesis.
• Glossary. Falsifiable claim, cross-PR invariant, supersedes, walk commits forward.