Playbook: Reviewing AI PRs

Objective

Merge agent-authored changes only when intent, structure, and proof all hold — using a fixed three-pass review that catches silent assumptions, architecture erosion, and false-green tests. Speed comes from a checklist, not from skimming the diff once.

When to use

Every non-trivial PR primarily authored by Claude Code / Cursor / Copilot agent mode. Mechanical renames can use a shortened pass 3 only. For writing the tests under review, see Testing AI Code.

Inputs required

• PR diff + linked spec (or explicit "no behavior change" claim).
• Plan / allowlist from the implementing session (if it exists).
• Local ability to run make check and one manual acceptance path.
• CLAUDE.md never-dos and architecture map.

Step-by-step process

Pass 1 — Intent and spec

Question: Did we build the right thing, and does the durable record match?

Check	Fail if
Spec linked and read	Behavior PR with no spec and no justification
Acceptance criteria ↔ diff	Criterion unmet or untested
Non-goals	Scope expanded ("while here" features)
Spec sync	Behavior changed, spec untouched — see Keeping Specs Alive
Silent product decisions	New status codes, error taxonomy, rounding, authz rules not in spec

Reviewer prompt (optional second agent):

You are reviewing PR <url or diff>. Read specs/<path> and the diff.
Pass 1 only: intent.
List: unmet acceptance criteria, silent decisions, spec drift, scope creep.
Do not comment on style. Do not rewrite code.

Pass 2 — Design and boundaries

Question: Did we put code in the right place without eroding the architecture?

Check	Fail if
File allowlist	Drive-by files unrelated to the feature
Navigability	New utils.ts, *-v2/, duplicated helpers — Navigability
Layering	Domain importing HTTP, lib importing features
Dependencies	New package without approval
Abstractions	Speculative repository/DI/factory layers
Compatibility	Breaking API/JSON/schema without migration plan

Reject phrase that should be common: "Split the cleanup into another PR."

Pass 3 — Evidence and tests

Question: Would we know if this broke?

Check	Fail if
CI green	Missing or no-op gates
Pasted local evidence	"Tests pass" with no output on risky changes
Regression / acceptance tests	Bug fix without failing-first story
Mock-only as sole proof	Boundary change covered only by mocks
Characterization	Refactor weakened expectations
You ran one check	Reviewer never executed anything

Spot-run the highest-risk test or manual path yourself. Agents hallucinate green; your shell does not. See Hallucination and False Confidence.

Severity → action

Finding	Action
Silent product decision	Changes requested; amend spec first
Drive-by refactor	Revert those files; re-approve narrowed diff
Weak tests	Block merge on payment/auth/data paths; otherwise conditional
Spec drift	Require spec patch in-PR
Wrong design, right intent	Reject; new plan mode session — don't nitpick into shape

Human review points

You are the merge authority. A second agent can draft pass 1–2 findings; you still own pass 3 spot-checks and the approve button. Builder/reviewer split: don't use the same polluted session that authored the PR as the sole reviewer (Multi-Agent Patterns).

Expected artifacts

Review comments mapped to pass 1/2/3 · required spec amendments · evidence of at least one human-run check on non-trivial PRs · approve/reject decision.

Common failures

Symptom	Root cause	Early detection	Fix
LGTM on first skim	Single-pass review	No checklist	Enforce three passes
"Fix nits" on a wrong design	Politeness over rejection	Pass 2 failed	Reject for replan
Merged, spec lies	Pass 1 skipped	Post-merge agent reverts behavior	Spec sync required in CI culture / template
CI green, bug in prod	Pass 3 shallow	No human spot-run	Add regression; tighten review bar
Reviewer agent rubber-stamps	Same context as builder	No split	Fresh reviewer session

Recovery strategy

If a bad agent PR already merged: treat as incident — revert or forward-fix with Recovery, then amend the review checklist that missed it. If review is stuck in nit loops: declare pass 2 failed and demand a new implementation plan instead of drive-by comments.

Acceptance criteria

• [ ] Pass 1: spec/intent clear; drift addressed
• [ ] Pass 2: boundaries/deps/scope acceptable
• [ ] Pass 3: evidence + meaningful tests; human spot-check done for risky changes
• [ ] No merge on silent product decisions
• [ ] Drive-by refactors removed or split out

Related

• Testing AI Code — how tests should have been written.
• Control Techniques — what the authoring session should have constrained.
• Architecture Erosion — pass 2 failure mode.
• Shallow Implementation — pass 1/3 failure mode.
• Verification Checklists — extra copy-paste lists.
• Responsibility Split — why merge stays human.