2_review

Review the current implementation changes and produce a written review report.

IMPORTANT: For best results, run this skill in a fresh session (/clear first) so the review is unbiased by the implementation context.

Use a subagent to do the file investigation so the reading does not consume the main session's context. The subagent should return its findings and you write the report from those findings.

Steps:

1. Read CLAUDE.md for project context and conventions
2. If $ARGUMENTS is provided, treat it as a spec filename and read .claude/specs/<name>.md to understand what was intended
3. Phase awareness: check if .claude/specs/<name>-phases.md exists. If it does:
   • Read it and identify the current phase (the phase with status in-progress, or the latest done phase)
   • Scope all review lenses to the current phase's requirements and validation criteria only
   • Requirements belonging to later phases (status: pending) are explicitly out of scope — do not flag them as missing in Correctness, PM, or Spec validation
   • State at the top of the review report: "Reviewing Phase N of M — later-phase items are out of scope"
   • On the final phase (no pending phases remain), review against the full spec to catch cross-phase integration issues
4. Read all files in .claude/context/ if the directory exists — for additional project context when evaluating correctness
5. Detect review mode — check if .claude/reviews/<name>-review.md already exists from a previous cycle:
   • If no previous review exists → full review mode (steps 6-8 below)
   • If a previous review exists → delta review mode (step 9 below)

Full Review Mode (first review cycle)

1. Identify what changed: use the project's version control system (e.g. git diff main or git diff HEAD~1); if not in a VCS repo, ask the user which files to review
2. Read each changed file in full
3. Evaluate against these criteria (multi-role review — address each lens):
   • Correctness: does it match the spec requirements? Are there logic bugs or missing cases?
   • Code quality: does it follow existing patterns? Is anything over-engineered or unnecessarily complex?
   • Security: any injection risks, exposed secrets, or missing input validation at system boundaries? OWASP top-10 considerations?
   • Tests / QA: are the relevant cases covered? Are there obvious edge-case gaps? Any regression risk to existing behavior? Are component-level rendering tests present (not just pure-function tests)?
   • UX / Minimal impact: does the change touch only what's necessary? Any scope creep? Are any fixes patches hiding a root-cause problem that should be fixed properly? For components displaying computed numbers, check for floating-point precision issues (use .toFixed() or Math.round()). For components with reactive state (useEffect + Zustand/Redux), check that dismissals, resets, and one-time actions cannot be re-triggered by unrelated state changes.
   • PM: does the change deliver business value? Does it align with the stated goal in the spec? Is anything built that wasn't asked for?
   • DevOps: any CI/CD implications? Environment variables, build config, or deployment steps affected? Any observability gaps (missing logs, metrics)?
   • Spec validation: check "Validation criteria" in the spec if present — can each criterion be confirmed from the diff? List each criterion and whether it is met or unmet.
   • Spec completeness: walk every requirement, validation criterion, and test case listed in the spec. For each, mark ✅ implemented / ❌ missing / ⚠️ partially implemented. A missing requirement (❌) is automatically critical severity. A partial implementation (⚠️) is major. Present the full checklist in the Issues section grouped under "Spec completeness gaps". This lens catches the most common error class: code that passes quality review but silently omits spec items.