code-review-battery

name

code-review-battery

description

Use when reviewing code changes to dispatch parallel specialized reviewers instead of a single monolithic review — provides deeper, more precise findings across focused lenses. Invoke as: /sp-cr-battery [min-score] [--security|--no-security] [--mode=bug-fix|feature] (optional 1.0–10.0 quality threshold, default 7.0; default 9.2 in Bug Fix Review Mode). Bug Fix Mode auto-activates on hotfix/* and fix/TICKET-* branches.

Code Review Battery

Wrong skill? File-protocol review handoff → code-review. PR inline → providing-code-review. Pre-commit gate → progressive-code-review-gate. Full-repo security audit → repo-security-scan or /sp-devsec-audit. Slash commands: /sp-cr-battery [min-score] (primary; optional 1.0–10.0 quality threshold, default 7.0), /sp-deepreview (legacy).

Dispatch up to 6 specialized reviewer agents in parallel, each focused on a distinct set of review dimensions. A triage coordinator selects which reviewers to activate based on the diff, then aggregates findings into a unified report.

Why this exists: A single reviewer tries to evaluate everything simultaneously, leading to shallow coverage, inconsistent focus, and ~40% false positive rates. Specialized reviewers with focused prompts produce deeper analysis with near-zero false positives.

When to Use

• When requesting-code-review or progressive-code-review-gate triggers a review
• When you want a thorough review of staged changes, a commit range, or a PR diff
• When reviewing someone else's code
• When verification-before-completion detects the implementation→presentation transition and no valid sentinel exists for HEAD

Gate chain position 3 of 4:

Gate (order)	Self-fires when	Short-circuit if
1. debate	About to commit to a design before coding	Already ran this session
2. progressive-harsh-review	About to present a non-code deliverable	Already ran on this artifact
3. code-review-battery	About to present/commit/push code	Valid sentinel for HEAD exists
4. verification-before-completion	About to write any results-presenting response	Sentinel SHA == HEAD → skip re-dispatch

One-per-unit rule: Battery fires at most once per coherent unit of work. If a valid .code-review-cleared sentinel exists for HEAD, the gate is already satisfied — do not re-dispatch.

Procedure

Phase 0: Sentinel Check (canonical skip gate — run before dispatching anything)

This is the canonical skip gate for the one-per-unit rule. Callers (requesting-code-review, finishing-a-development-branch, progressive-code-review-gate) should run this before dispatching. If a caller does not implement Phase 0 explicitly, the agent should apply this decision manually before invoking battery.

SENTINEL="$(git rev-parse --show-toplevel 2>/dev/null || echo .)/.code-review-cleared"
cat "$SENTINEL" 2>/dev/null || echo "NO CLEARANCE"
echo "HEAD: $(git rev-parse HEAD 2>/dev/null)"
git diff --quiet && git diff --cached --quiet && echo "WORKTREE_CLEAN" || echo "WORKTREE_DIRTY"

Sentinel state	Decision
NO CLEARANCE	Run battery (proceed to Phase 1).
Sentinel SHA ≠ HEAD SHA	Run battery (battery is stale).
Sentinel valid for HEAD but WORKTREE_DIRTY	Run battery (staged/unstaged changes exist that were not reviewed).
Valid sentinel for HEAD AND WORKTREE_CLEAN	Skip. Battery already ran on the current code. Note the clearance and skip to Phase 6.
Malformed	Delete .code-review-cleared, run battery.

Phase 0.5: BugPath Mode Detection

Run immediately after the sentinel check. Detect whether this is a targeted bug fix, then set the mode before triage.

BRANCH=$(git rev-parse --abbrev-ref HEAD 2>/dev/null || echo "")
# Matches: hotfix/*, fix/<TICKET>-* where <TICKET> prefix is configured in
# .cr-battery-ticket-prefixes (one uppercase prefix per line, e.g. "PROJ").
# run-battery.sh reads that file and builds the regex at runtime.
if echo "$BRANCH" | grep -qE '^(hotfix/|fix/[A-Z]+-[0-9]+)'; then
  echo "BugPath Mode: ACTIVE (branch: $BRANCH)"
else
  echo "BugPath Mode: INACTIVE"
fi

Signal	BugPath Mode trigger
Branch prefix hotfix/*	Active
Branch prefix fix/<TICKET>-*	Active
Explicit flag --mode=bug-fix	Active
Explicit flag --mode=feature	Inactive (overrides branch detection)

When BugPath Mode is active:

• BugPath Verifier is added to the activated reviewer set (mandatory — not skippable via --skip)
• Default threshold raises from 7.0 to 9.2 unless --min-score is explicitly provided
• Path-coverage floor applies to the score (see Phase 3 scoring)
• SCOPE-SKIP re-classification: if the BugPath Verifier emits SCOPE-SKIP on a confirmed bug-fix branch (hotfix/* or fix/TICKET-*), the orchestrator must surface this as an Important finding: "BugPath Verifier SCOPE-SKIP on confirmed bug-fix branch — manual path-coverage review required." This is not an error; it is a human-review flag. The score floor is not applied, but the Important finding lowers the aggregate score by 1.5.

Output the executive summary as the absolute first element of your review — before triage, before findings, before anything else. Use the exact box format from Phase 3. This is the adoption contract: any engineer reads the first 4 lines and knows the verdict.

State the mode prominently in the executive summary and triage line:

**BugPath Mode: ACTIVE** | Branch: fix/incident-2026-1507 | Threshold: 9.2 | BugPath Verifier: mandatory

Also support --mode=bug-fix (explicit flag, same as confirmed branch prefix) and --mode=feature (explicit opt-out: disables BugPath Mode even on hotfix/* branches).

Phase 1: Triage

Analyze the diff and select reviewers:

Reviewer	Focus	Activate When
Defect Finder	Correctness, edge cases, concurrency	Any code change
Design Critic	Factoring, complexity, API design	Adds/modifies classes, functions, public APIs
Guardian	Security, blast radius, backwards compat	Any code change
Standards Enforcer	Docs, test quality, observability	Always
Performance Analyst	Performance, logging	DB, loops, caching, network I/O, or >500 LOC
AttackerPersona	Credential-flow, AI-agent boundary, ident-vs-value, cookie/session, revival re-validation, CWE tagging	Any security signal (see reference.md) or --security flag
BugPath Verifier	Root cause, fix coverage, sibling bugs, regression test	BugPath Mode active (see Phase 0.5) — mandatory, not skippable
Monolith (on-demand)	All dimensions	--all flag or manual request

Decision rules: Docs-only → Standards Enforcer only. Config-only → Guardian (+ Standards Enforcer / Defect Finder only if a metric/alarm or other dispatch signal below is present). Any code → Defect Finder + Guardian + Standards Enforcer + conditionally Design Critic and Performance Analyst.

Signal-driven dispatch (additive — a signal activates its reviewer(s); it never deactivates one already selected above). Scan the diff for these signals and route accordingly:

Diff signal	Reviewer(s)	Owns
Metric/counter/event definition, or .emit(/.inc(/publish( call	Defect Finder + Standards Enforcer	Producer Trace, metric liveness, emission symmetry
Alarm/threshold definition, or an error branch that emits a metric or feeds an alarm	Guardian + Standards Enforcer	Alarm-feeding isolation, failure-mode differentiation, multi-provider coverage
External-dependency call / SDK error classifier (429, 5xx, provider error shapes)	Guardian	Multi-provider predicate coverage, monitoring blast radius
Field set to null/0/false/reset	Defect Finder + Guardian	Consumer trace, cross-cutting regressions
Public signature / interface / message schema / shared type change	Design Critic + Guardian	API design, backward compat
Loop over I/O, DB query, cache, network call, or >500 LOC	Performance Analyst	N+1, payload bloat, blocking I/O
File rename/move/delete	Guardian (inbound-reference focus)	Broken external consumers
Test-only change	Standards Enforcer (+ Defect Finder for revert-safety)	Mock fidelity, revert-safety
Security-class signal (caller-supplied URL, dynamic SQL identifier, new MCP tool/IPC, secret read, cookie/session, _disabled/ revival)	AttackerPersona	Credential-flow, AI-agent boundary, ident-vs-value; tags + threat-model severity multiplier

When no signal and no default activates a reviewer, skip it and say why in the triage line.

Mandatory activation (not subject to triage exclusion):

• Guardian is ALWAYS activated when changes touch: retry logic, circuit breakers, rollback behavior, deployment config, feature flags, authentication/authorization, or state machine transitions.
• Design Critic is ALWAYS activated in Standard Mode when changes touch: interfaces, public APIs, contracts, message schemas, shared state types, or cross-module boundaries.

Design Critic in Bug Fix Mode (SUPPRESSED by default): In Bug Fix Mode (BugPath Verifier active), Design Critic is skipped by default and only re-activated when the diff contains API-change signals:

# API-change signals that re-activate Design Critic in Bug Fix Mode:
git diff HEAD~1 | grep -qE '^\+.*(export (default )?(class|interface|type|function|const)|public [a-zA-Z]+\()'

If any API-change signal matches: activate Design Critic and prefix the triage line with ⚠️ Design Critic re-activated on bug fix (API-change signal detected). If no signal matches: skip Design Critic and state Design Critic: SKIPPED (Bug Fix Mode — no API-change signals) in the triage line.

Overrides: --all (force all), --only=<name>, --skip=<name>, --round1-only (skip escalation), --security (force AttackerPersona on), --no-security (force AttackerPersona off), --mode=bug-fix (force Bug Fix Mode), --mode=feature (force Standard Mode). --skip=BugPath Verifier is silently ignored in Bug Fix Mode (the reviewer is mandatory).

State your triage decision before dispatching:

**Triage**: Activated: [list] | Skipped: [list] | Reason: [1-2 sentences]

Phase 2: Diff + Source Context + Dispatch

Sub-agents have NO conversation context. Pass diff + source context inline.

1. Capture diff: git diff --cached, git diff HEAD~1, or git diff main..HEAD

2. Source context for ripple analysis (#1 missed-finding cause = reviewing diff in isolation):

• Fields SET/RESET/NULLED → grep all READERS
• Symbols DEFINED (metrics, events, enums, error codes) → grep all PRODUCERS; zero producers = dead definition. Mandatory finding for metric/alarm-feeding signals; see Producer Trace steps 4-5 for indirect-producer downgrades and enum/error-code severity
• Threshold comparisons → grep all PRODUCERS of crossing values
• Stateful code → full state type + transitions
• Changed signatures → all callers
• Cross-module calls → full callee body (or signature + state-mutating/throwing/early-return branches if budget-constrained)

3. Inbound reference scan (mandatory when diff renames, moves, or deletes files):

git diff --diff-filter=RD --name-only main..HEAD   # old paths
grep -rn "old-filename" . --include="*.md" --include="*.ts" --include="*.sh"  # scan ENTIRE repo

MUST scan outside the changed directory. The #1 failure mode: scoping grep to the refactored directory, missing sibling modules that reference old paths. Hits outside the diff are mandatory CRITICAL findings — broken consumers the author didn't update. Include grep results in every reviewer's context.

4. Dispatch ALL activated reviewers simultaneously via sub-agent-code-reviewer (Augment) or Task() (Claude). Each gets: reviewer prompt + full diff + source context + inbound reference scan results.

Phase 3: Aggregate

After all reviewers return:

1. Sort findings: Critical → Important → Minor, then by file path
2. Prefix each with [Reviewer Name]
3. If 2+ reviewers flag the same location, keep both and check for convergence:
   • True convergence (promote to at least Important): reviewers reached the finding through different reasoning paths — e.g., one found it via data flow analysis, another via error handling review. The evidence snippets and rationale must differ.
   • Echo convergence (do NOT promote): reviewers cite the same evidence snippets, use near-identical phrasing, or clearly derived their finding from the same analytical path. This indicates shared context bias, not independent validation. Keep both findings at their original severity.
4. Note clean dimensions ("✅ No issues") -- and require EACH clean-dimension verdict to carry the same evidence block as a finding. A "no issues" verdict without grep-verifiable evidence is treated by the verifier as unverifiable and caps the dimension at 7.0; this is the structural anti-confabulation gate.
5. Severity normalization: Re-evaluate each finding against the shared severity definitions (provided to all reviewers). Reclassify when a reviewer's label doesn't match:
   • Critical = broken RIGHT NOW if shipped (wrong output, data loss, crash, security hole)
   • Important = breaks UNDER CONDITIONS (missing guard, incomplete fix, correctness risk)
   • Minor = works but violates standards (style, naming, missing docs/tests, observability gaps)
   • Elevate to Important when the operator-visible signal is itself wrong or missing (not merely cosmetic): a dead metric or blinded alarm feeding a dashboard/alarm — treated as live by default unless the diff proves it is unwired — OR a separately-actionable failure cause folded into a generic metric/alarm
   • If a reviewer labeled a finding Critical but it's a process/standards gap (e.g., "no tests added"), downgrade to Important or Minor. Note the reclassification: [Reclassified: Critical → Minor — missing tests are a standards gap, not a production defect]
   • True convergent findings (step 3) are promoted to at least Important. Echo convergent findings retain their original severity.
6. Triple-filter each Important/Critical finding on CX impact, complexity, and testability, then classify:

• Implement: Passes all 3 filters. Propose exact code change.
• Defer: Good finding but doesn't pass all 3. Document for future work.
• Reject: Correct observation but fix adds more complexity than it removes.

7. For each Implement finding, preserve the reviewer's Regressions Risked and Durable Check fields in the report. If multiple reviewers truly converge on the same finding (different reasoning paths), merge their regression analyses and pick the most actionable durable check.

Tightening: If total findings >10, suppress Minor findings from the report body. Still count them in the summary line. Never suppress Critical or Important. State "Tightening applied: [N] Minor findings suppressed" in the report.

Report format: Executive Summary → Header (activated/skipped reviewers) → Critical → Important → Minor (full, or "[N] Minor findings suppressed") → Clean Dimensions → Action Classification table → Durable Checks summary → Live Metrics → Summary (Findings: [N] Critical, [N] Important, [N] Minor ([N] suppressed) | Metrics: durable=[N]% or N/A, convergent-count=[N], unresolved-critical=[N]).

Executive Summary template (MANDATORY — first element of every review output):

┌────────────────────────────────────────────────────────────────────┐
│  MODE: Bug Fix Review [9.2 threshold]  |  BRANCH: fix/proj-1234  │
│  VERDICT: REJECT [6.5/10]  |  2 Critical, 1 Important, 3 Minor   │
│  ACTION: Fix 2 Critical findings. DO NOT merge.                   │
│  BugPath Coverage: INSUFFICIENT (2/4 dimensions) → cap 6.5       │
└────────────────────────────────────────────────────────────────────┘

Standard mode variant (no BugPath row):

┌─────────────────────────────────────────────────────────────────────┐
│  MODE: Standard Review [7.0 threshold]  |  BRANCH: feat/new-api   │
│  VERDICT: PASS [8.5/10]  |  0 Critical, 1 Important, 2 Minor      │
│  ACTION: Address Important finding before merge (non-blocking).    │
└─────────────────────────────────────────────────────────────────────┘

VERDICT choices: PASS, PASS_WITH_NITS, PASS_WITH_FIXES, REJECT. ACTION is a single plain-English sentence: what the author must do right now.

Metrics: Durable check rate (≥50%), convergent finding count, unresolved Critical count (target: 0). Offline: precision ≥75%, high-sev precision ≥80%, Round 2 yield ≤20%. Score (after all fix rounds): 10.0 − (Critical×2.5) − (Important×1.5) − (Minor×0.25) − (durable<50% ? 0.5 : 0), floor 0.0. Calibration: 0 findings → 10.0; 1 Important → 8.5; 1 Critical → 7.5; 2 Importants + low durable → 6.5. Extract threshold from invocation (e.g. /sp-cr-battery 8.5 → 8.5; default 7.0 or 9.2 in BugPath Mode). Score < threshold → abort Phase 6.

BugPath Mode path-coverage floor (BugPath Mode only — applied after standard formula): Extract path_coverage from the BugPath Verifier's structured output. Apply the cap: INSUFFICIENT (<3/4 dimensions verified) → cap aggregate at 6.5; PARTIAL (3/4 verified) → cap at 8.0; FULL (all 4 verified) → no cap. SCOPE-SKIP or N/A → no floor applied.

Phase 4: Escalation (Round 2)

If ANY trigger fires after Round 1, re-dispatch a focused reviewer:

Trigger	Re-run	Why
>2 state/flag findings	Defect Finder (interaction-path focus)	Systemic timing/ordering
>3 test quality issues	Standards Enforcer (mock-focused)	Shared mock infrastructure
>50 lines removed or functions deleted	Guardian (deletion focus)	Callers may depend on removed behavior
Files renamed/moved/deleted without inbound scan	Guardian (inbound-reference focus)	Broken consumers outside the changed directory
"Pre-existing" issues flagged	Defect Finder (lifecycle focus)	Deeper structural gaps
Diff adds/changes a metric or alarm definition, or an error-handling branch that emits a metric or feeds an alarm	Standards Enforcer (observability-completeness focus)	Dead definitions, Success/Failure asymmetry, undifferentiated failure modes

Re-dispatch with focused instruction (diff slice + refreshed context + trigger signal). Append under ### Round 2 Findings. Skip if --round1-only, all clean, or diff <20 lines.

Phase 5: Convergence

STOP when: unresolved Critical = 0, last 2 passes <20% new high-sev, durable ≥50%. CONTINUE if escalation trigger fires or Critical remains. ESCALATE TO HUMAN after 3 passes.

Correlated-Failure Detection

After synthesis, scan all reviewer outputs for shared blind spots:

1. Evidence overlap check: If ≥3 reviewers cite the same evidence snippets (same file + same line range) for their ONLY findings, flag ⚠️ CORRELATED EVIDENCE — reviewers may share a blind spot outside the cited region. Expand the review scope to adjacent modules.
2. Phrasing similarity check: If 2+ reviewers use near-identical phrasing for different findings (copy-paste reasoning), flag ⚠️ ECHO REASONING — findings may reflect shared analytical bias, not independent analysis. Require at least one reviewer to re-examine from a different entry point.
3. Clean-sweep suspicion: If ALL reviewers report zero findings, flag ⚠️ UNANIMOUS CLEAN — verify reviewers examined different evidence slices. Check that each reviewer's output references different source files or code paths.

Correlated-failure flags do NOT change verdicts directly — they trigger expanded scope or re-examination. The goal is to surface shared blind spots, not to manufacture findings.

Phase 6: Finalize Verdict + Write Sentinel

Prerequisite: Correlated-Failure Detection has completed and no re-examination was triggered.

Preserve the run (before sentinel write):

1. Determine the verdict from this MR's score vs threshold: PASS if score >= threshold; PASS_WITH_NITS if at or above threshold but nits flagged; REJECT or PASS_WITH_FIXES otherwise.
2. Write a JSON envelope to .cr-battery-runs/<HEAD-sha>.json (per-engineer durable record; the directory is gitignored).

Schema (run envelope wrapping Phase 3 findings):

{
  "run_timestamp": "<ISO 8601 UTC>",
  "head_sha": "<git rev-parse HEAD>",
  "verdict": "PASS | PASS_WITH_NITS | PASS_WITH_FIXES | REJECT",
  "score": 9.30,
  "rounds": 1,
  "bugpath_verdict": {
    "path_coverage": "FULL | PARTIAL | INSUFFICIENT | SCOPE-SKIP | N/A",
    "dimensions": {
      "root_cause": "VERIFIED | UNVERIFIABLE | MISSING",
      "fix_coverage": "VERIFIED | PARTIAL | MISSING",
      "sibling_scan": "VERIFIED | FOUND-N | MISSING",
      "regression_test": "VERIFIED | MISSING"
    },
    "score_cap": null
  },
  "findings" 
    
       
       
       
         
           
       
       
         
            
         
         
      
    
  
   
    
       
       
       
       
         
            
         
      
    
  

Every finding AND every clean-dimension verdict MUST carry an evidence block. verifiable: false is reserved for genuine judgment claims (race conditions, design smells) that cannot be re-executed deterministically; such claims are capped at 7.0 by the verifier, not falsified.

Expectation types: count (e.g. ">0", "==0"), exit_code (integer), match (regex applied to stdout, max 256 chars), absent (passes iff stdout has zero non-blank lines), exact (string equality after trim).

Verifier replay: tools/run-battery.sh invokes tools/verify-cr-battery-evidence.js on the freshly-written envelope when in Bug Fix Mode (mandatory) or when .cr-battery-runs/ exists in Standard Mode (graceful degrade: skipped if directory absent). The verifier re-executes every evidence.command, compares observed output to the declared expectation, and caps each (reviewer, dimension) on falsification (5.0) or unverifiable (7.0) claims. Any FALSIFIED claim aborts the sentinel write. Exit codes: 0 all-verified-or-unverifiable; 1 at least one falsification; 2 usage/IO/parse error.

tools/run-battery.sh refuses to write the sentinel if the per-HEAD JSON is missing in Bug Fix Mode. In Standard Mode, the check is gracefully degraded (skipped if .cr-battery-runs/ is absent). See docs/cr-battery/finding-lifecycle-design.md for the deferred lifecycle work.

If final verdict is PASS or PASS_WITH_NITS (all nits resolved):

# tools/run-battery.sh is the ONLY permitted way to write .code-review-cleared.
tools/run-battery.sh --verdict PASS --min-score <threshold>

❌ Never write .code-review-cleared directly with echo. Use tools/run-battery.sh.

Timing: If battery runs pre-commit, the sentinel becomes stale after commit -- re-run before push. The pre-push hook validates sentinel SHA against the pushed ref; without a valid sentinel the push is blocked.

If verdict is REJECT or PASS_WITH_FIXES: do NOT write the sentinel. Fix all Critical/Important findings, re-dispatch, then write sentinel when the re-run passes.

Gap Analysis + Error Handling

Monolith found something no specialist found → candidate pattern → candidates/. Reviewer fails → note, don't retry. Diff >3000 lines → warn, suggest chunks. Empty diff → skip.

Anti-Patterns

See reference.md for the 5 anti-patterns (all-agree, duplicates, fatigue, missing-context, over-scoping) with detection + correction columns. Moved to the companion file to keep this skill under the per-skill line budget.

Failure Modes

See reference.md for the 4 standard failure modes (no-findings, FPs-from-isolation, convergence-stuck, monolith-vs-specialist) with fixes. Moved to the companion file alongside the Anti-Patterns table.

Companion Skills

• progressive-code-review-gate: Primary consumer (dispatches this battery pre-commit)
• providing-code-review: Engineering rigor checklist (informs reviewer focus)
• inter-agent-review-protocol: File-protocol review (alternative dispatch method)
• micro-harsh-review: Per-batch review

---

Source: bordenet/superpowers-plus — distributed by TomeVault.