requesting-code-review

Requesting Code Review

Review early, review often, and never review your own work with the same context that wrote it. Dispatch a subagent with a clean brief: the diff range and the requirements, never your session history. Fresh eyes catch what your accumulated assumptions hide.

When to request

Mandatory: after each task in subagent-driven development, after completing a major feature, before merging to main. Worth it: when stuck, before a big refactor (baseline check), after fixing a gnarly bug.

Workflow

1. Pin the range.

BASE_SHA=$(git merge-base origin/main HEAD)   # or HEAD~1 for a single task
HEAD_SHA=$(git rev-parse HEAD)

1. Dispatch a general-purpose subagent with the reviewer prompt. Read references/reviewer-prompt.md and fill every placeholder: what was built, what it was supposed to do (plan, spec, or requirements), and the two SHAs. Give the reviewer requirements and code only. Do not paste your reasoning, your chat history, or your opinion of the code.
2. Triage the findings.
3. Critical (breaks, security, data loss): fix immediately, before anything else.
4. Important (correctness gaps, missing error handling): fix before proceeding to the next task.
5. Minor (naming, style, small refactors): note them, schedule them, move on.
6. Disagree with evidence. If the reviewer is wrong, push back with code, tests, or constraints, then handle the exchange per receiving-code-review. Never silently ignore a finding.
7. Re-verify. After fixes, run the tests the findings implicated. If fixes were substantial, dispatch a follow-up review of the fix range.

Good vs Bad

Bad dispatch: "Review my recent changes, here's my whole conversation so far. I think the code is pretty solid, mostly check style." The reviewer inherits your blind spots, anchors on "solid", and reviews the chat instead of the diff.

Good dispatch: "Review commits a7981ec..3df7661. Built: index verification plus repair handling four issue types. Requirements: task 2 in docs/plans/deployment-plan.md. Report Critical/Important/Minor with file:line and a verdict: ready or not." Scoped, neutral, checkable.

Verification

Run git diff --stat $BASE_SHA..$HEAD_SHA before dispatching. Expect the diff to contain exactly the work you want reviewed, no unrelated churn. If unrelated changes appear, split or restate the range, or the review is noise.

After triage, list every Critical and Important finding next to its fix commit. Expect a one-to-one mapping. Any finding without a fix or a written pushback: the review is not closed.

Red flags: stop if you hear yourself say

• "It's a simple change, review would be overkill."
• "I'll get it reviewed after I merge."
• "The reviewer flagged it Important, but I'm sure it's fine."
• "I'll fix the Critical one later, I don't want to lose momentum."
• "I'll just paste my whole session so the reviewer has context."
• "Tests pass, that's basically a review."

Each phrase trades a cheap check now for an expensive cascade later. Rephrasing the excuse does not change what it is: skipping review is skipping review, and unfixed Critical or Important findings mean the work is not done.

Footguns

• Contaminated context. Handing the reviewer your session history makes them agree with you, not review you. Fix: brief with requirements, description, and SHAs only.
• Wrong base SHA. Reviewing against HEAD~1 when the feature spans five commits reviews a fraction of the work. Fix: use git merge-base origin/main HEAD for feature-level review.
• Review theater. Dispatching a reviewer and skimming its output, then proceeding regardless. Fix: triage every finding into fixed, scheduled, or rebutted; write it down.
• Batching many tasks before one review. Issues compound across tasks and the fix touches everything. Fix: review at each task boundary.

Completion checklist

• [ ] BASE and HEAD SHAs pinned; diff verified to match the work
• [ ] Reviewer briefed from references/reviewer-prompt.md with all placeholders filled, no session history
• [ ] Every Critical and Important finding fixed or rebutted with evidence
• [ ] Minor findings recorded somewhere durable
• [ ] Tests green after fixes

Any box unchecked: not done. Fix or say so.