review

Skill: review

The independent review gate. After implement has written + tested + committed the code, review looks at the whole change at once, with fresh eyes — does it actually satisfy every acceptance criterion, and is it good code? This is distinct from the per-task gate inside implement (which proves each task green): review is the cross-cutting, clean-context pass a human reviewer would do on the PR.

It reuses the shared clean-context discipline (../_shared/critic.md) and the reviewer subagent (read-only). Question phrasing per ../_shared/ask-style.md.

Review-record prose follows artifact_language (carry the language in the reviewer's dispatch prompt) — the verdict literals PASS / CHANGES REQUESTED / REVIEW_CLEAN and cited identifiers stay English → ../_shared/artifact-language.md.

Owner

Tech Lead / a reviewer who did not write the code (independence is the point).

Inputs

• <slug> — feature slug.
• Gate (hard refuse): an implemented change must exist (commits on the feature branch, or a non-empty working diff). Nothing to review → «run implement <slug> first».
• Read for the review baseline — the whole AC chain, so the trace can be checked end-to-end: docs/features/<slug>/spec.md §5 (the full AC set — the source of truth, not the diff's trailers), sad.md §6 (the sequence flows/branches each AC should appear in), data-model.md / contracts/openapi.yaml / Accepted adr/ (the contracts the code must honour), test-plan.md (the AC→test map, if a separate file), and tasks.json (which AC each task claimed).

Protocol

1. Scope the diff. Determine the change under review: git diff <base>..HEAD on the feature branch (base = the branch point), or the named changed files. Note the SDD-AC trailers — the ACs the implementation claims to satisfy.
2. Dispatch the independent reviewer. Run the reviewer agent — subagent_type: "sdd:reviewer" (read-only, clean context — it re-reads spec/contracts itself, no paraphrase; model per judgment_model; effort xhigh on L/XL via CLAUDE_CODE_EFFORT_LEVEL, per ../_shared/agent-roster.md) — over the diff along the dimensions in ./references/review-dimensions.md: stage 1 — every claimed AC genuinely satisfied and the whole §4 user-story set + §5 AC set traced end-to-end (spec → sequences §6 → data-model → api → tasks → implement): every §4 user story has ≥1 AC and a §6 flow, and every §5 AC reaches code+test — spanning every surface declared in sad.md target_surfaces (a UI AC traces to a component / e2e-through-UI test, not only a backend one) — flagging any user story or AC that dropped out anywhere in the chain, not only the ACs the diff claims via its SDD-AC trailers; stage 2 — conventions, error/edge handling, security, boundary violations, test adequacy. For a large diff, fan out one reviewer per dimension and merge.
3. Collect cited findings. Each finding cites file:line + the AC/contract it touches. Drop uncited findings (per the critic discipline). A clean review returns REVIEW_CLEAN. If the reviewer ran asynchronously and came back as an idle/completion signal with no report, pull the full report through the host's messaging channel — never accept a verdict without its text (→ ../_shared/agent-roster.md, shared-contract point 2).
4. Resolve each finding with the user via AskUserQuestion: Fix now (hand the actionable finding back to implement/the author as a follow-up task — re-enter the TDD loop for it) / Defer (record in spec §8 Open questions with owner + due) / Not an issue (the reviewer misread; record why). Never ship an unresolved stage-1 (AC) finding.
5. Write the review record. docs/features/<slug>/_review/review-<date>.md: scope (diff stat), findings with verdicts, and the gate result (PASS / CHANGES REQUESTED).
6. Verdict + next. Then emit the stage-handoff block per ../_shared/handoff.md — What I did + Review (_review/review-<date>.md) + Run next: PASS → (/clear, then /sdd:ship <slug>); CHANGES REQUESTED → /sdd:implement <slug> for the fixes (no /clear — stay in context to iterate), then re-review the changed surface.