Code Review

Code Review

Staged pipeline: test → scope → intent → rules → analysis → verification → report. Finds bugs through state enumeration, not pattern matching. Catches regressions through git history, not guessing. Kills false positives through intent-anchored verification.

Scope argument: "$ARGUMENTS"

Important

• False positives are the enemy. Every finding must survive a verification pass with type-stratified confidence thresholds. Findings below threshold are discarded.
• This skill finds BUGS, not style issues.
• Read CLAUDE.md, REVIEW.md, and all @-imported rules files BEFORE launching review agents.
• Never invent data. If you need to reference a function's output or API response, read the actual source — do not estimate or fabricate.
• This skill reviews code, not plans. For plan review use /plan-review.

Instructions

Step 0: Test Baseline

Auto-detect and run the project's test suite and linter/analyzer before reviewing any code. This gives agents factual ground truth about whether the code under review is broken.

Detection order (stop at first match):

1. pubspec.yaml exists → run flutter analyze then flutter test
2. package.json exists with a scripts.test key that is NOT the default npm placeholder (echo "Error: no test specified" && exit 1) → run npm test (or yarn test if yarn.lock present, or bun test if bun.lockb present)
3. Cargo.toml exists → run cargo clippy then cargo test
4. go.mod exists → run go vet ./... then go test ./...
5. pyproject.toml or setup.py exists → run pytest
6. Check REVIEW.md for a test_command: directive → run that command
7. No runner found → skip with note "No test runner detected — skipping baseline"

Capture output and exit codes.

• If tests FAIL: record the failure output. This becomes mandatory CRITICAL context injected into every agent in Step 4. The test failure itself is a finding — include it in the report.
• If tests PASS: record Tests: PASS for the report header.
• If linter/analyzer has errors (not warnings): record as context for agents.

Do NOT abort the review if tests fail — the diff may contain the fix.

Step 1: Determine Review Scope

If $ARGUMENTS is a PR number or "PR":

• Run gh pr diff to get the changes
• Run gh pr view to get the PR description and title
• Extract the PR description as raw text for Step 2

If $ARGUMENTS is a file path or glob:

• Use that as the scope
• Extract the most recent commit message touching those files via git log -1 -- <file>

If $ARGUMENTS is empty:

• Run git diff for unstaged changes
• If empty, run git diff --cached for staged changes
• If empty, run git diff HEAD~1 for the most recent commit
• If all empty, ask the user what to review
• Extract the commit message via git log -1 --format=%B

State what you found: "Reviewing N changed files: [list]."

Step 2: Intent Extraction

Launch a single agent (not parallel) to determine what the change is trying to accomplish.

Input: the diff from Step 1 + the commit message or PR description.

Agent prompt:

Analyze this diff and its commit message / PR description. Answer:

1. Intent: What is this change trying to accomplish? State the goal in one paragraph.
2. Scope: List the files being modified and their roles.
3. Constraints: List any constraints mentioned in the commit message, PR description, or comments in the diff (e.g., "must not change public API", "backwards compatible").
4. Confidence: Rate your confidence in understanding the intent:
   • clear — the commit message explains the goal and the diff matches
   • ambiguous — the diff does multiple things or the message is vague
   • missing — no commit message, or the message doesn't match the diff

Return a structured response with these four fields.

Store the intent artifact. It is passed to every subsequent agent and to the verification step. If confidence is missing, all downstream findings receive a -15 confidence penalty (they cannot be as confidently judged without knowing the author's goal).

Step 3: Load Project Rules

Read CLAUDE.md in the project root. If it contains @-imports (lines like @~/.claude/rules/foo.md), read each imported file. Collect all explicit rules into a rules set.

Read REVIEW.md in the project root if it exists. REVIEW.md contains review-specific rules and causal bug patterns ("when you see X, check for Y because Z"). These are injected as highest-priority context to all agents — they take precedence over default behavior.

Identify which CLAUDE.md files exist in directories whose files were changed — these are also relevant and should be loaded.

This step is mandatory. Skipping this produces opinions, not compliance checks.

Step 4: Domain Analysis

Launch 3 agents in parallel. Each agent receives:

• The diff from Step 1
• The test baseline results from Step 0
• The intent artifact from Step 2
• The project rules from Step 3

Each agent returns a list of findings. Each finding must include: file path, line number(s), description, category, and the evidence (specific code, state combination, or historical pattern that supports the finding).

Agent A — State Enumeration Bug Detection

You detect bugs by systematically tracking every possible value of every variable at every program point. You do not pattern-match against bug categories. You trace state.

For each changed function or method in the diff, execute the following phases IN ORDER. Show your work at each phase. Do not skip to conclusions.

Phase 1 — State Inventory

Build a state table for the function:

"Possible Values" means the ACTUAL set, not just the type:

• bool: {true, false}
• String? / string | null: {null, empty, non-empty}
• List<T> / T[]: {null, empty, single-element, many}
• Controller/resource: {uninitialized, initialized, disposed/closed}
• Promise/Future state: {pending, resolved, rejected}

"Lifecycle": when created, when it can become invalid, who owns disposal.

Include: all locals, all fields/properties touched, all parameters, any global/singleton state, any async state (Futures, Streams, Promises, pending callbacks), external state (DB, network, file system).

Phase 2 — Transition Tracing

For each execution path through the function:

1. ENTRY STATE: what combinations of variable values can exist at entry?
2. Walk each branch. At each decision point, fork:
   • Condition-true path: what does the state table look like now?
   • Condition-false path: what does the state table look like now?
3. At each WRITE: is the new value consistent with what other variables expect?
4. At each AWAIT / ASYNC BOUNDARY: any state read before the await may have changed after it. Re-examine the state table post-await.
5. At EXIT: does the final state satisfy the caller's expectations?

For each path, check:

• Can this path be reached? (dead code)
• Is state ever partially updated? (field A changed, dependent field B not yet updated)
• Is there a window where invariants don't hold? (between two writes)

Phase 3 — Invariant Checking

State invariants that must hold for this function:

ENTRY (preconditions):

• Is the object/widget still alive (mounted, not disposed)?
• Are required fields non-null / initialized?
• Are resources in the expected state?

MAINTAINED (during execution):

• If state A and state B must be consistent, is there a window where they aren't?
• If a resource is held, is it held across the entire critical section?
• Across async boundaries, is re-validation performed?

EXIT (postconditions):

• Are all acquired resources released on every path (including error paths)?
• Is the return value in the expected range/type on every path?
• Are listeners/subscriptions properly managed?

CROSS-FUNCTION:

• If function A must be called before function B, is that enforced?
• Does this function's output match its callers' expectations?

For each invariant: does every traced path maintain it?

Output rules:

• Do NOT report style issues, naming preferences, or simplification opportunities.
• Only report bugs traceable to a specific state violation through a specific path.
• Each finding must reference: the state variable(s), the path, and the invariant violated.
• If Phases 1-3 found no violations, report "CLEAN — no state-based bugs found."

Agent B — Project Rules + Compliance

• Audit the changes against every rule in the loaded CLAUDE.md rules set
• For each violation, quote the specific rule and its source file
• Focus on rules that constrain the CODE itself (naming, patterns, forbidden practices), not rules about workflow or process
• If REVIEW.md exists and contains causal bug patterns ("when you see X, check for Y because Z"), actively check for Y in the diff — these are the highest-priority checks
• Note: CLAUDE.md is guidance for Claude writing code — not all rules apply during review

Agent C — History + Regression Detection

This agent holds both git history context AND prior PR patterns in a single context, enabling cross-cutting findings that isolated agents would miss.

Git history analysis:

• Run git log --oneline -20 -- <file> and git blame on the modified files
• Check: was this code recently fixed for the same kind of issue?
• Check: does the blame show this area has been a repeated source of bugs?
• Check: are the changes reverting or contradicting recent intentional fixes?

Prior PR comment replay:

• Find previous PRs that touched the same files: gh pr list --state merged --search "file:path" or git log --oneline --follow -- <file> then gh pr list to find associated PRs
• Read comments on those PRs (use gh api repos/{owner}/{repo}/pulls/{number}/comments)
• Check if any prior review comments apply to the current changes — is the author making the same mistake that was flagged before?

Regression detection:

• Cross-reference: does this diff re-introduce a pattern that was previously fixed (visible in git log) AND flagged in a prior PR comment? That's a high-confidence regression.
• Are historically buggy files being modified without corresponding test changes?

If gh CLI is not available, skip the PR comment analysis and note it in the output. If no prior PRs touched these files, return only git history findings (or empty if none).

Step 5: Verification

Launch a single sequential agent that receives ALL findings from Step 4 plus the intent artifact from Step 2. This agent verifies each finding and assigns a confidence score.

For each finding, the verifier checks:

1. Intent relevance: "Does this finding affect the stated intent from Step 2?"
   • If irrelevant to the change's goal → cap at 50
2. Traceability: "Can I trace this to a specific line, path, or state combination?"
   • If untraceable or speculative → cap at 50
3. Scope: "Is this pre-existing (on lines the user didn't modify) or introduced by this change?"
   • If pre-existing → cap at 50

Confidence rubric (pass verbatim to the verifier):

• 0: False positive. Does not stand up to scrutiny, or is a pre-existing issue unrelated to the change.
• 25: Might be real, but could be a false positive. Unable to verify from the diff.
• 50: Verified as real, but irrelevant to the stated intent, or rarely hit in practice.
• 75: Real issue that will be hit in practice. Important and will directly impact functionality, OR directly violates a CLAUDE.md / REVIEW.md rule.
• 100: Definitely real, directly impacts the stated intent, evidence confirms it. Test failure from Step 0 that correlates with the diff is automatic 100.

Type-stratified thresholds — findings below these are discarded:

These are always false positives — discard regardless of score:

• Things a linter, typechecker, or compiler would catch (imports, types, formatting)
• Pedantic nitpicks a senior engineer wouldn't flag
• General code quality concerns (test coverage, documentation) unless required in CLAUDE.md
• Issues silenced in code (lint ignore comments, type: ignore)
• Style preferences, naming opinions, simplification suggestions
• Runtime-state claims the verifier cannot trace from the diff — if verification would require reading code outside the diff and the verifier can't do so in 1-2 reads, the finding is speculative

Cap at 50 (never escalate above) when:

• The only evidence is a deleted comment or removed block from a prior fix commit. Removals of prior-fix code are often revised tradeoffs. Report at 50 as "UNCERTAIN — verify on-device before fixing," never higher. Only escalates if reproduced on a running build.
• Claims that depend on controller/animation resting values without reading the underlying Tween/animation definition
• "Missing dismiss/cleanup" claims when the parent controller manages lifecycle

Step 6: Report

CODE REVIEW — [N files reviewed, 3 agents dispatched, tests: PASS/FAIL/SKIPPED]

CRITICAL (confidence 90-100) — [count]
  * [file:line] [description]
    Source: [Agent A|B|C] | Type: [state-bug | compliance | regression]
    Path: [execution path or state combination that triggers this]
    Fix: [specific change]

IMPORTANT (confidence 65-89) — [count]
  * [file:line] [description]
    Source: [Agent A|B|C] | Type: [state-bug | compliance | regression]
    Fix: [specific change]

CLEAN AREAS: [agents that returned no findings above threshold]

SUMMARY: [1-sentence assessment anchored to the stated intent from Step 2]

If no findings survived verification: "No issues found. Reviewed [N] files against stated intent: [one-line intent summary]. Checked for state-based bugs, project rule compliance, and regressions. Tests: [PASS/FAIL/SKIPPED]."

Step 7: Apply Fixes

For CRITICAL findings (90+): apply fixes immediately unless the user said "report only."

For IMPORTANT findings (65-89): offer to apply: "Want me to apply these fixes?"

Never apply fixes to code outside the reviewed scope.

Error Handling

1. No changes found: "No changed files found. Provide a file path, PR number, or run after making changes. Example: /code-review src/bot.py or /code-review 42"
2. Scope argument doesn't exist: "File $ARGUMENTS not found. Check the path."
3. CLAUDE.md not found: Run Agents A and C only. Note: "No CLAUDE.md found — skipping compliance check (Agent B)."
4. REVIEW.md not found: Normal — most projects won't have one. No note needed.
5. No prior PRs for Agent C: Agent C reports only git history findings. Note in report: "No prior PR comments found for these files."
6. gh CLI not available: Agent C skips PR comment analysis. Note in report.
7. Binary or generated files in diff: Skip silently. Note excluded files in summary.
8. Test runner not detected: Skip Step 0 with "Tests: SKIPPED (no runner detected)."
9. Tests timeout or error: Record as test infrastructure issue, not a code finding. Note in report header and proceed with review.

Examples

Example 1: State bug caught via enumeration

Input: /code-review Process: git diff shows changes to an auth guard. Step 0 runs tests (pass). Step 2 extracts intent: "Fix null dereference when anonymous session expires." Agent A builds the state table, traces the session token through the guard, and finds that ref.read(authProvider) is used post-await without re-read — the session can change across the async boundary. Verifier scores 85 (state bug, traceable path, relevant to intent). Reports as IMPORTANT.

Example 2: Regression caught via merged history agent

Input: /code-review 42 Process: Fetches PR #42 diff. Agent C finds via git blame that line 47 was fixed for an off-by-one last month. A prior PR comment on PR #38 says "watch the boundary condition here." The current diff reintroduces the same pattern. Cross-referencing history + prior comment gives high confidence. Verifier scores 90. Reports as CRITICAL, auto-fixes.

Example 3: Clean review

Input: /code-review Process: Step 0 tests pass. 3 agents review 2 changed files. No findings survive verification. Output: "No issues found. Reviewed 2 files against stated intent: 'Add retry logic to API client.' Checked for state-based bugs, project rule compliance, and regressions. Tests: PASS."