04 Review

Review

Use this skill after implementation to review changes against the diff, plan, and prior learnings.

See shared pipeline instructions for model routing and pipeline behavior.

Core rules

1. Load project rules before producing findings (detailed in Workflow step 3): load ../../rules/common/code-review.md + code-smells.md, detect language from changed files, load matching rules/{lang}/ files including review-checklist.md (mark missing (fell back to common) when absent), plus rules/web/ for frontend/browser changes. Emit a Rules loaded: manifest — no manifest, no findings
2. Priority: project-level {repo-root}/rules/ overrides package defaults
3. Standards axis baseline: apply ../../rules/common/code-smells.md (Fowler smell baseline). Two binding rules: a documented repo standard overrides the baseline; every smell is a judgement call (report as "possible Feature Envy"), never a hard violation. Map severity via P0/P1/P2 — default P2, escalate when a repo doc endorses it or it harms data flow/testability.
4. Determine diff scope before selecting reviewers
5. Use review_router tool to select reviewer personas based on diff metadata
6. Read relevant plan artifact when exists
7. Run solution search (see ../references/solution-search.md):
   • Extract keywords → grep -rl "tags:.*keyword" docs/solutions/ ~/.pi/agent/docs/solutions/
   • Read frontmatter only (first 15 lines) of matches → score by severity + tag relevance
   • Fully read top 3 candidates
8. Spec axis: determine spec source via references/spec-source-detection.md (plan → brainstorm → commit issue ref → skip). Against the chosen spec, report missing requirements, scope creep (unrequested behaviour), and wrong implementation (looks done but isn't).
9. Produce structured findings using references/findings-schema.md
10. Autofixable findings: apply and re-review (max 3 iterations)

Review discipline

Code review is technical evaluation, not social performance:

• Verify before implementing any suggestion
• YAGNI check: question features nothing uses
• No performative agreement: verify before concurring
• Push back with reasoning when findings are incorrect
• Evidence before assertions: cite specific code, not principles
• Architecture axis: audit module depth and seams using ../references/module-design.md

Precision gate

Favor precision over recall. A false positive costs more trust than a missed minor issue.

• Before reporting a non-local claim (race condition, security boundary, resource leak), use file_read and code_search to confirm evidence. Do not infer from names alone.
• Stay silent when the surrounding context is unclear. A miss on ambiguous code is acceptable; a false alarm is not.
• Do not flag issues that a compiler, formatter, linter, or type checker already catches, unless the diff shows a concrete user-visible consequence those tools miss.
• Label each finding with severity. Blocking (CRITICAL/HIGH) for correctness and security; non-blocking (LOW) for style and naming.
• Apply language-specific rules from rules/{lang}/review-checklist.md — they contain precise, actionable defect patterns per language.

Handling findings

1. Read — complete all findings without reacting
2. Verify — check each against codebase reality
3. Evaluate — is it sound for THIS codebase?
4. Act — fix confirmed issues, push back on incorrect ones
5. Test — verify each fix individually, no regressions
6. Close — when findings are resolved and tests green, flip the reviewed plan's Status header to done and move it to docs/plans/archive/ (skip if no plan artifact)

Workflow

1. Load context: consume latest handoff before any broad file reads — context_handoff load or read .context/compound-engineering/handoffs/latest.md. If found, use activeFiles, artifacts.plan as starting point. If not found, proceed normally. Read CONTEXT.md if it exists at root — see ../references/domain-language.md.
2. Determine diff scope — prefer branch/base from latest handoff if present; else from explicit target; else ask user
3. Load project rules (blocking — no findings before this completes):
   • Detect language from changed files (.ts/.tsx→typescript, .py→python, .go→golang, .rs→rust, .java→java) or repo markers, merging {repo-root}/rules/language-detection.md (project-level map, same marker wins); full map in language detection. Mixed-language diffs: load per language
   • Check {repo-root}/rules/ first (overrides package defaults); load rules/common/code-review.md, code-smells.md, matching rules/{lang}/ files including review-checklist.md, rules/web/ for frontend/browser changes
   • Emit manifest before any finding: Rules loaded: language=<lang> (via <files/markers>[, project-level map]), common=<files>, lang=<files>, web=<files or N/A>
   • Same-session re-entry: if the transcript already contains a Rules loaded: manifest for the same language, do not re-read the rule files — reuse them, cite the earlier manifest, and note the skip
4. Collect stats (files, insertions, deletions) → call review_router
5. Read matching plan artifact; if absent, follow references/spec-source-detection.md to probe brainstorm and commit issue refs
6. Run solution search
7. Apply each reviewer persona from review_router
8. Merge into structured findings — include rules applied in the review summary (see references/findings-schema.md)
9. Verify each finding against codebase
10. Apply autofixes, re-run tests, re-review if needed

Optional: QA Test Mode

After code review complete, offer browser QA:

Code review done. Run browser QA?

• A) Done — stop here
• B) Browser QA — find visual/functional bugs
• C) QA + regression tests — find bugs, fix, add tests

If B or C: read references/qa-test-mode.md and execute workflow. After QA: include findings in handoff, note fix commits/test files.

Handoff

See references/handoff.md for format.

Before finishing this skill, apply the completion checklist in shared pipeline instructions.