code-review

Code Review

Two halves in order: get the change read by someone who did not write it, then answer what comes back. Steps 1–4 send it; Answering the review handles the return.

Step 1 — Run the project's own checks

Run the project's lint, type check, and test commands as that project defines them — read its package.json scripts, Makefile, or contributor guide rather than guessing a command name. A reviewer spending attention on a failure the suite already prints is attention not spent on the change.

• Done when every check the project defines has run in this working tree and you can state each one's exit status and failure count.

Red or unrunnable checks end the review here: name the command and its failure, then stop. A review dispatched over a broken tree buries that failure under review comments.

Step 2 — Compute the review range

BASE="${TASK_BASE_SHA:-$(git merge-base HEAD "${BASE_BRANCH:-main}")}"
HEAD="$(git rev-parse HEAD)"
git diff --stat "$BASE..$HEAD"

Never HEAD~1. A task carrying more than one commit loses everything but its last commit, and the reviewer then reports on a change nobody made.

Blank fields, the literal strings HEAD~1/HEAD, and "look at my recent changes" are not a range — resolve them even when asked to skip the resolution for speed.

• Done when BASE and HEAD hold resolved SHAs and git diff --stat printed a non-empty file list.

Step 3 — Dispatch a fresh reviewer

Dispatch one subagent and paste the text below into it with the four slots filled in — the text itself, because a subagent handed a file path resolves it against its own working directory. Fresh means it gets your requirements and your diff and none of your reasoning: a reviewer that already believes the change is correct reviews the belief.

You are reviewing one change. Read it once, then return both parts below.

Requirements under review:
{REQUIREMENTS}

The implementer reported:
{IMPLEMENTER_REPORT}

Read the change with:
  git diff --stat {BASE}..{HEAD}
  git diff -U10 {BASE}..{HEAD}

The implementer's report is a set of unverified claims. Check every requirement
against the code in the diff. A stated rationale ("kept it simple per YAGNI",
"deliberate for now") is itself a claim and never lowers a finding's severity.

Return exactly these two parts, in this order, each with its own verdict line.

## Part 1 — Spec compliance
Verdict: COMPLIANT | ISSUES
Classify every requirement as met, or as one of: Missing (not implemented, or
implemented so it does something else), Extra (in the code, not in the
requirements), Misunderstood (a different problem solved). A requirement that
lives outside this diff and cannot be settled from it is `Cannot verify from
diff` — say what the caller should check and stop there.

## Part 2 — Code quality
Verdict: APPROVED | NEEDS FIXES
Assess every changed function, not only the ones a requirement covers:
separation of concerns, error handling and swallowed errors, resource cleanup on
all paths, edge cases, security, duplication, test quality, file and function
size. Severity is Critical / Important / Minor by actual impact. A defect the
requirements explicitly mandated is still a finding — report it and label it
plan-mandated.

Cite file:line for every finding in both parts, state its impact, and give the
fix. Report only what you read in the diff.