Review Work

Review Work — Automated Code Review

Review uncommitted code changes using Claude sub-agents as independent reviewers. The invoking agent (you) triages the diff and decides who reviews; reviewers self-prime via /prime, verify API/library claims via Context7, and report against progress.md intent.

This skill is run by an AI, not by a human — use judgment about the change you just made. Don't apply a fixed rubric mechanically. Zero external dependencies: reviewers are Claude sub-agents.

Process

Step 1: Capture the Diff (+ Tests)

Run these and save the output:

git diff --stat HEAD

git diff HEAD

git diff HEAD captures uncommitted work — the normal pre-commit flow. If the work was already committed (e.g. direct-to-main), review the last commit instead: git diff HEAD~1 HEAD (or git show HEAD).

If the project has a test command configured and relevant source changed, run it and capture the output:

# Use whatever test/build command is appropriate for this project's stack.
# e.g. npm test · pytest · cargo test · go test ./... · the test_command in
# hooks/config/pipeline.json if set.

Test/build failures are the #1 finding for every reviewer — include the failure output in each reviewer's prompt verbatim.

Step 2: Triage

Look at the changed file paths and produce two lists.

Impacted modules — by default the whole project is one scope (one project per repo). Only split into modules/components when the diff clearly spans distinct top-level areas (e.g. api/ vs web/, backend/ vs frontend/). If it's all one area, that's a single scope — don't manufacture splits.

Risk surfaces — flag the presence of any of these generic surfaces. For each one that fires, inject the matching focus-area line into reviewer prompts in Step 4:

Omit the focus-areas section entirely if no surfaces fire.

Step 3: Decide Reviewers — Judgment Rubric

Skip the skill entirely when the change is genuinely trivial:

• Comment-only / formatting / typo-only
• Single-line fix with no logic or contract effect
• Pure doc edits with no code references

If you skip, tell the user once: "Change is trivial, skipping review." Then continue.

For non-trivial changes, scale reviewers to the diff:

• Under ~50 lines changed → one reviewer with the combined Bug Hunter + Rules Auditor checklist (Step 4, single-reviewer template).
• ~50+ lines, or 2+ modules → parallel specialists: a Bug Hunter (correctness + security) and a Rules Auditor (project rules + tests). When the diff splits into distinct modules, give each specialist the scope note for the modules it covers.

Add an OPTIONAL Architect reviewer when ANY of these is true:

• Changes span 2+ modules/components
• New abstraction or API contract introduced (not just a config tweak)
• A refactor / migration is left unfinished, or files were renamed/moved
• The diff shows scope creep — more than the active task called for

Most changes need 1 reviewer. Larger or multi-module changes get 2. The Architect appears only when the change is design-significant — don't pre-spawn it.

Step 4: Spawn Reviewers (Parallel, Single Message)

Use the Agent tool with subagent_type: "Explore" (read-only). Send all reviewers in a single message so they run in parallel. Inline the role — no custom sub-agent files needed.

Every reviewer prompt includes these shared blocks. Define them once, paste into each template:

## Required reading (self-prime)
Before reviewing, run /prime — read .claude/commands/prime.md and follow its
file-loading instructions to load this project's core docs (spec,
project-structure, progress). Skip the acknowledgement step — load the files,
then review.

## The diff
{full `git diff HEAD` output}

## Test results
{Step 1 test/build output, or "n/a — no testable files in this diff"}

## Focus areas flagged by triage
{relevant lines from the Step 2 catalogue; omit this section if none fired}

## Mandatory verification (Context7)
If you flag a finding about an API signature, library usage, deprecation, or
SDK version behavior, you MUST first call the Context7 query-docs tool to
verify it. If that tool isn't directly callable, load it via ToolSearch first
(`select:mcp__context7__query-docs`) — don't skip verification just because the
tool wasn't preloaded. Tag every finding:
- [verified]   — Context7 confirmed the issue.
- [unverified] — you couldn't or didn't check. AUTO-DISCARDED by the judge.
                 Don't bother reporting these.
- [n/a]        — finding is not an API/library claim (most bugs and rules).

## Intent verification (required, output FIRST)
Before your findings, output exactly one line:

    INTENT: [yes | partial | no | n/a] — <one-line reason referencing progress.md>

- yes     — diff fulfills the active task in docs/ai-context/progress.md.
- partial — fulfills part of it, or fulfills it but adds unrelated changes (scope creep).
- no      — diff doesn't match anything in progress.md's active scope.
- n/a     — there is no progress.md, or no active task to verify against.

## Output format
INTENT line first, then one finding per line:

    [high|medium|low] [verified|unverified|n/a] path/to/file:line — Description. Reason: <why this is a problem>.

Check ONLY for real issues. Don't nitpick style, naming, or formatting unless
it causes a bug. If a category is clean, omit it. Don't invent issues to seem
thorough — only report what you can point to in the diff.

Template: Single Reviewer (small diffs)

You are a code reviewer for an uncommitted-diff code review. Cover both
correctness/security AND project-rule/test compliance.

{shared blocks}

## Checklist
**BUGS** — Logic errors, null/undefined handling, off-by-one, race conditions,
async/await mistakes, state-machine violations, wrong return types, unreachable
code, missing error handling, incorrect boolean logic.

**SECURITY** — Secrets or PII logged or exposed, missing input validation at
system boundaries, internals leaked in error messages, hardcoded secrets,
injection vulnerabilities, broken access checks.

**PROJECT RULES** — Violations of the loaded CLAUDE.md and ai-context docs:
architecture decisions, coding conventions, wrong storage/transport layer, any
documented project-specific constraint.

**TESTS** — If this touches shared modules or critical paths, do corresponding
tests exist? Are assertions structural rather than exact-string matches?

Template: Bug Hunter (correctness + security)

You are the Bug Hunter for an uncommitted-diff code review. Your ONLY job is
logic errors and security vulnerabilities. Ignore style, naming, and project
rules — the Rules Auditor handles those.

{shared blocks}

## Checklist
**BUGS** — Logic errors, null/undefined handling, off-by-one, race conditions,
async/await mistakes, state-machine violations, wrong return types, unreachable
code, missing error handling, incorrect boolean logic.

**SECURITY** — Secrets or PII logged or exposed, missing input validation at
system boundaries, internals leaked in error messages, hardcoded secrets,
injection vulnerabilities, unsafe deserialization, broken access checks.

Template: Rules Auditor (project rules + test coverage)

You are the Rules Auditor for an uncommitted-diff code review. Your ONLY job is
compliance with this project's rules and test coverage. Ignore general
correctness and security — the Bug Hunter handles those.

{shared blocks}

## Checklist
**PROJECT RULES** — Violations of the loaded CLAUDE.md and ai-context docs:
architecture decisions, coding conventions, wrong storage/transport layer, any
documented project-specific constraint.

**TESTS** — If this touches shared modules or critical paths, do corresponding
tests exist? Are assertions structural rather than exact-string matches?

Template: Architect (optional — design-significant changes)

You are the Architect reviewer for an uncommitted-diff code review. You look at
the diff AS A WHOLE — design coherence, structural soundness, invariants. You do
NOT report line-level bugs or style; the other reviewers handle that.

{shared blocks}

## What to check
- **Premature abstraction** — a new abstraction wrapping one caller, or where a
  few inline lines would have been clearer.
- **Half-finished migrations** — files renamed inconsistently, removed code
  still referenced, dual code paths left after a rewrite.
- **Cross-file invariants** — type renames, signature/contract changes: are all
  call sites updated?
- **Cross-module impact** — when a shared module changes, do its consumers still
  hold conceptually? Are public APIs preserved, or the break noted?
- **Dead code** — branches, parameters, or files no longer reachable.
- **Scope creep** — does the diff do more than progress.md's active task called
  for? Refactor mixed into feature work?

Architect findings tend to be MEDIUM/HIGH because they're structural. Be
precise — point to specific files and behaviors, not vibes.

Step 5: Judge Findings

Combine output from all reviewers and evaluate each finding. Reviewers have fresh eyes but lack your conversation context — they don't know WHY you made certain choices.

Auto-discard unconditionally:

• Findings tagged [unverified] about API/library/SDK claims. Context7 is mandatory — no verification, no finding.

For everything else:

Lead with INTENT if any reviewer reported partial or no — that's the headline, not the line findings. Code can be locally clean but solving the wrong problem.

Step 6: Output to User

## Code Review Results

Reviewers: <list, e.g. "Bug Hunter + Rules Auditor (parallel)" or "single reviewer">
Modules touched: <list, or "whole project">
Tests: <pass | fail | n/a>
**Intent: <yes | partial | no | n/a>** — <one-line reason>

### Blockers
- [high] file:line — <description>. **Action:** Fixed | Rejected (reason) | Noted

### Mediums
- [med] file:line — <description>. **Action:** …

### Lows
<N findings — expand if you want details>

If everything is clean: a single line — "No blockers. N low-severity items (expand if interested). Intent: ."

Important Rules

1. Never skip review for non-trivial work. Self-review is not review.
2. Trivial means trivial. Comments, formatting, typos, no-logic-effect single-line fixes. Anything that changes behavior is non-trivial.
3. Never blindly accept findings. Reviewers can hallucinate file paths, misread logic, or flag intentional choices. You're the judge.
4. Auto-discard [unverified] API/library findings. Context7 is mandatory — no verification, no finding. Don't relax this.
5. Lead with INTENT. A diff that's clean but off-target is worse than a diff with fixable bugs.
6. Reviewers are read-only. Use subagent_type: "Explore". They never edit code — only the judge (you) applies fixes.
7. Test failures dominate. If tests failed, that's finding #1; everything else is secondary.
8. Don't pre-spawn the Architect. Use the rubric — most changes don't need it.
9. Spawn in parallel. Multiple reviewers → single message with multiple Agent calls.