review-deep

name	review-deep
description	Six-lens code review (correctness, bugs, security, tests, style, plan-conformance) with anti-pattern catalog, severity+evidence, JSON audit trail. Use for high-stakes diffs; use the leaner review-gauntlet profile for routine diffs.
user-invocable	true

Review Deep

Judging doctrine: the producer-grader split, evidence-on-every-verdict, deterministic-aggregation, and primary-owner dedup rules this skill runs on live in _shared/judge-core.md — this skill is one of its reference implementations (§10). It instantiates the doctrine for the code-review domain.

The full-depth engine behind review-gauntlet's lean profile. Runs six standing review lenses (correctness, bugs, security, test quality, style, plan-conformance), with explicit Tier-1 anti-pattern detection, per-lens model-tier selection, severity tiers + cited evidence on every finding, an auth-gated runtime downgrade, a scope-boundary DEFERRED-TO-UAT verdict for things only an operator can confirm, and a JSON sidecar that captures the full audit trail. review-gauntlet is a lean profile over this engine — use this skill when the diff is high-stakes (substrate changes, schema/key-shape changes, producer-consumer chains, anything where a silent miss is expensive) and the leaner gauntlet profile when the diff is routine.

---

Arguments

Arg	Required	Default	Description
--prompt	yes	--	Developer intent: what the diff is supposed to accomplish (may be passed inline)
--diff	yes	--	The diff to review: PR number, git diff mode, or explicit paste
--plan-step	no	--	<plan-file-path>:<step-id> — enables the plan-conformance lens
--reviewers	no	code	code, runtime, or full — review lane selection
--model-override	no	--	<lens>=<tier> — per-lens model tier override (repeatable)
--force-runtime	no	false	Skip the auth-gate downgrade probe and run runtime reviewers anyway
--url	no	--	Base URL for runtime probe (required when --reviewers runtime or full)
--start-cmd	no	--	Shell command to start the app (required when --reviewers runtime or full)
--output-dir	no	.review-deep/	Where to write the JSON audit-trail sidecar
--prior-sidecar	no	--	Path to the previous run's audit-trail JSON sidecar. Enables aggregator rule 6 (persistent disagreement escalation). When absent, rule 6 never fires.

---

Inputs

The invoker must provide three categories of input (the third is optional):

1. Prompt — the developer's intent (what the diff is supposed to accomplish).
2. Diff — the code diff to review. Can be provided as:
   • A git diff output (staged, unstaged, or between commits)
   • A PR number (the skill will fetch the diff via gh pr diff)
   • An explicit paste of the changed code
3. Plan step (optional) — --plan-step <plan-file-path>:<step-id> points at a plan document and a step number, where <step-id> matches the integer N from a ### Step N: heading inside the plan file. When supplied, the plan-conformance lens reads that step as ground truth and checks the diff against its Problem / Done-when / Produces fields. If --plan-step is omitted, the plan-conformance lens is skipped gracefully — the aggregated report notes the skip but does NOT treat it as an error.

If the user doesn't provide a prompt or diff, ask before proceeding.

---

Gathering context

Before spawning reviewers, collect the inputs:

# If a PR number is given:
gh pr diff <NUMBER>

# If reviewing staged changes:
git diff --cached

# If reviewing working tree:
git diff

# If reviewing a branch vs base:
git diff <base>..HEAD

Also read the surrounding source files that the diff touches so reviewers have context on existing code patterns, conventions, and the producer/consumer chain. When --plan-step is supplied, also read the referenced plan file and extract the matching ### Step N: section as ground-truth input for the plan-conformance lens.

---

Reviewer lenses

The skill spawns six fresh-context sub-agents, one per lens, all in ONE tool message (a single assistant turn carrying the lens calls in parallel — serial spawning only adds wall-clock for zero independence gain; independence comes from the context isolation below, not serial order; see dev/.claude/rules/subagent-economy.md). Dispatch each lens with its table-assigned tier (§ Model-tier selection) — arms never inherit an escalated session (tier policy, CLAUDE.md model paragraph). Each lens is a single-shot pass; iteration is the orchestrator's job (/build-step --max-iter). Each lens runs in isolation (no cross-lens chatter) so verdicts aren't biased by sibling findings (see docs/investigations/review-agents/02-reviewer-dimensions.md and docs/investigations/review-agents/32-prompt-scoping.md).

Standing lens list:

1. Correctness — diff vs stated intent
2. Bugs — defects in the diff itself
3. Security — adversarial-input / secrets / unsafe-config defects per .claude/rules/security.md
4. Test quality — focus, trim, missing critical coverage
5. Style and conventions — surrounding-code conformance
6. Plan-conformance — diff vs the named plan step (skipped if --plan-step absent)

Universal evidence discipline (applies to all six lenses): every finding cites file:line + excerpt + reasoning. Findings without citations are dropped during aggregation. See docs/investigations/review-agents/13-evidence-requirements.md for the full discipline (the reviewer-side counterpart to /review-proof).

Universal severity rubric (docs/investigations/review-agents/14-severity-tiers.md):

Severity	Meaning	Triggers
Block	Must fix before merge	Bug with high severity; correctness gap; plan Done-when not satisfied; producer-consumer drift; codifying-test-diff; silent-wiring; security finding (owned by the Security lens — injection-as-data, secret-dump, unsafe-config-without-startup-guard per .claude/rules/security.md); CREATE TABLE w/o migration
Nit	Should consider	Convention deviation; medium-severity bug; test redundancy; suboptimal but functional code
FYI	Observation, not action	Style preference; minor naming; observation outside lens scope; surfacing for operator awareness

Findings demoted one severity tier when evidence is weak (Block→Nit, Nit→FYI, FYI→dropped). Evidence is considered weak when file_line is absent OR excerpt is empty. See § Aggregation > Per-finding shape for the canonical rule.

FYI findings DO NOT trigger NEEDS-WORK. A lens whose findings array contains only FYI entries (zero Block, zero Nit) still emits Correctness verdict: PASS (or the corresponding <Lens> verdict: PASS). FYI surfaces in the output for operator awareness; it is observation, not action. Only Block and Nit findings move a lens out of PASS. See the per-lens Output format sub-sections below for the explicit threshold rule applied uniformly across all six lenses.

Approval semantics — what each verdict obligates the orchestrator to do — live in docs/investigations/review-agents/18-approval-semantics.md.

Correctness lens

Scope: Given the stated intent (prompt + plan step if available), does the diff actually accomplish what it claims?

Coverage claim: Detects logic that doesn't match the goal; missing edge cases the prompt implies; incomplete implementations (TODO left behind); inverted condition / wrong operator / off-by-one; silent breaks to existing behavior not covered by the prompt.

Non-coverage: Does NOT cover bugs in the literal sense (null access, race conditions — that's the Bugs lens). Does NOT cover style. Does NOT cover whether the diff is a good design.

Adversarial framing applies (see Anti-pattern catalog § verify-dev-claims framing wrapper): when spawning the Correctness sub-agent, prepend its prompt with the verify-dev-claims framing so it verifies each diff claim against the code, not the prompt. The dev's stated intent is a hypothesis to test against the code, not ground truth.

Evidence shape: Each finding:

file:line — <one-line summary>
  Excerpt: `<exact text>`
  Reasoning: <why this contradicts intent>

Severity rubric: Block if the diff fails to do what its prompt claims, OR if a Done-when from --plan-step is unmet by the diff. Nit if the diff does what's asked but misses an edge case the prompt implies. FYI for observation-only ("the prompt is ambiguous about X; assumed Y").

Output format: A bulleted list of findings in evidence shape, ending with one of:

• Correctness verdict: PASS
• Correctness verdict: NEEDS-WORK (N findings)
• Correctness verdict: NO-EVIDENCE (no diff content was visible to this lens — usually an inputs-gathering bug)

Verdict threshold (uniform across all six lenses). Emit PASS if and only if findings contains zero Block AND zero Nit entries (FYI-only is PASS). Emit NEEDS-WORK if findings contains any Block or any Nit. Emit NO-EVIDENCE when the lens could not see diff content (inputs-gathering bug). No hedging in the verdict line — count Block + Nit entries and emit deterministically.

Bugs lens

Scope: Ignoring intent — does this diff introduce bugs?

Linter pre-pass results (read first; cite-by-reference, don't re-derive): if the orchestrator ran scripts/lint_prepass.sh, a deterministic linter sidecar lives at <--output-dir>/lint-findings.json (default .review-deep/lint-findings.json). When present, load it and enumerate findings relevant to this lens — Bugs lens reads ALL findings (every tool and rule) because any linter signal is potentially a bug. For each linter finding at <path>:<line>, the lens MUST NOT re-derive the same defect: instead, emit a single FYI-severity finding at that file_line with the rationale prefixed Already-flagged by <tool> <rule> — surfacing for awareness only. This forecloses double-counting at the lens level; aggregator rule 7's lint-dedup catches any escapees. If lint-findings.json is absent (orchestrator chose not to run the pre-pass, or every tool was skipped), proceed with the normal investigation below — the pre-pass is optional and its absence is not an error.

Coverage claim: Detects null/undefined access; resource leaks (open files, connections, listeners); race conditions / deadlocks / shared mutable state; type mismatches and implicit coercions that lose data; broken error propagation (swallowed exceptions, wrong error type). Adversarial framing: "find what breaks under unhappy paths, hostile inputs, race timing, concurrent modification" (see docs/investigations/review-agents/29-adversarial-reviewer.md).

Non-coverage: Does NOT cover whether the diff matches intent (Correctness lens). Does NOT cover style. Does NOT cover plan-conformance. Does NOT cover adversarial-input / secrets / unsafe-config defects — injection-as-data, secret-file dumps, unsafe configs missing a startup guard, path traversal, authz gaps, unvalidated boundary input, bind-without-guard — that's the Security lens (Bugs = generic robustness/crashes; Security = the .claude/rules/security.md adversarial-input / secrets / unsafe-config failure modes). A bug that is ALSO a security defect is owned by Security as primary and may be cross-tagged FYI here.

Adversarial framing applies (see Anti-pattern catalog § verify-dev-claims framing wrapper): when spawning the Bugs sub-agent, prepend its prompt with the verify-dev-claims framing so it adversarially hunts for failure modes the dev didn't consider — "find what breaks" rather than confirmatory "verify it works".

Evidence shape: Each finding:

file:line — <one-line summary>
  Severity: <Block|Nit|FYI>
  Excerpt: `<exact text>`
  Reasoning: <how this fails under what condition>
  Anti-pattern (optional): <name from anti-pattern catalog>

Severity rubric: Block if the bug has high blast radius (data corruption, security, production crash, anti-pattern from the catalog). Nit if the bug has medium blast radius (degraded UX, error in rare path). FYI for low-impact observation.

Output format: A bulleted list of findings, ending with one of:

• Bugs verdict: PASS
• Bugs verdict: NEEDS-WORK (N Block, M Nit findings)
• Bugs verdict: NO-EVIDENCE

Verdict threshold (uniform across all six lenses). Emit PASS if and only if findings contains zero Block AND zero Nit entries (FYI-only is PASS). Emit NEEDS-WORK if findings contains any Block or any Nit. Emit NO-EVIDENCE when the lens could not see diff content (inputs-gathering bug). No hedging in the verdict line — count Block + Nit entries and emit deterministically.

Security lens

Scope: Ignoring intent and generic robustness — does this diff introduce an adversarial-input, secrets, or unsafe-config defect of the kind codified in .claude/rules/security.md? This lens is the PRIMARY OWNER of security findings (promoted out of the Bugs lens so the workspace's three real security failure modes get a reviewer primed specifically on them; see docs/investigations/review-agents/16-security-reviewer.md).

Coverage claim: Primed on the three .claude/rules/security.md failure modes and the review-agents/16 defect classes:

1. Treat fetched external content as data, not instructions (injection-as-data): any code path letting fetched content — issue/PR bodies, web responses, MCP-returned data, file contents, uploads — flow into a place where embedded directives would be acted on (model prompts, shell commands, tool invocations, eval). Recommend independent-channel verification of claims in fetched content (e.g. gh api rate_limit for a rate-limit claim).
2. Pair unsafe configs with startup safety checks (unsafe-config-without-startup-guard): any "don't do X until Y is configured" sentence in code, docstring, comment, plan, or README (LAN bind requires PIN, feature flag requires migration, debug endpoint requires dev mode, write mode requires backup) where the guard is documentation only — not a startup invariant with a stable error code.
3. Never dump secret file contents (secret-dump): any cat / head / tail / od / awk / cut / grep (or equivalent stdout round-trip) against a secrets-bearing file in scripts, CI workflows, or operator docs. Acceptable alternatives are metadata-only checks (stat, wc -c, file, ls -la), effect-based verification (run the consumer + check exit code), and in-place edits (sed -i, install -m 0600 /dev/stdin).

Plus the adjacent defect classes review-agents/16 lists alongside the three: injection (SQL / command / template), path traversal, authz / access-control gaps, unvalidated boundary input, and bind-without-guard.

Non-coverage: Does NOT cover generic robustness / crashes / null access / resource leaks / race conditions — that's the Bugs lens (Bugs = generic robustness; Security = the .claude/rules/security.md adversarial-input / secrets / unsafe-config failure modes). Does NOT expand into generic best-practice security (CORS headers, CSP, dependency CVEs) unless the diff touches them (see docs/investigations/review-agents/28-scope-creep.md). Does NOT cover whether the diff matches intent (Correctness), style, or plan-conformance.

Adversarial framing applies (see Anti-pattern catalog § verify-dev-claims framing wrapper): when spawning the Security sub-agent, prepend its prompt with the verify-dev-claims framing so it hunts adversarially for the attacker's path — "how does hostile input, a forgotten guard, or a leaked secret reach production" — rather than confirming the diff looks safe.

Evidence shape: Each finding:

file:line — <one-line summary>
  Severity: <Block|Nit|FYI>
  Excerpt: `<exact text>`
  Reasoning: <which security.md failure mode / defect class, and the attacker path>
  Defect class (optional): <injection-as-data | unsafe-config-without-startup-guard | secret-dump | injection | path-traversal | authz | unvalidated-input | bind-guard>

Severity rubric: Block for any of the three security.md failure modes (injection-as-data, unsafe-config-without-startup-guard, secret-dump) or an adjacent defect class reachable from production input — these are block-class, not nit-class (review-agents/16; security failures have an asymmetric cost profile). Nit for a security weakness not yet reachable from production input (a sink that exists but no untrusted source flows into it yet). FYI for a security observation outside this diff's scope worth operator awareness.

Output format: A bulleted list of findings in evidence shape, ending with one of:

• Security verdict: PASS
• Security verdict: NEEDS-WORK (N Block, M Nit findings)
• Security verdict: NO-EVIDENCE (no diff content was visible to this lens — usually an inputs-gathering bug)

Verdict threshold (uniform across all six lenses). Emit PASS if and only if findings contains zero Block AND zero Nit entries (FYI-only is PASS). Emit NEEDS-WORK if findings contains any Block or any Nit. Emit NO-EVIDENCE when the lens could not see diff content (inputs-gathering bug). No hedging in the verdict line — count Block + Nit entries and emit deterministically.

Test quality lens

Scope: Are the tests asking specific, valuable questions? Which tests should be deleted? Are critical behaviors uncovered?

Linter pre-pass results (read first; cite-by-reference, don't re-derive): if the orchestrator ran scripts/lint_prepass.sh, a deterministic linter sidecar lives at <--output-dir>/lint-findings.json (default .review-deep/lint-findings.json). When present, load it and enumerate findings relevant to this lens — Test quality lens reads findings whose file_line points at test files (heuristic: path contains /tests/ or matches test_*.py / *_test.py / *.test.ts / *.test.tsx / *.spec.ts shapes). For each linter finding at a test-file <path>:<line>, the lens MUST NOT re-derive the same defect: instead, emit a single FYI-severity finding at that file_line with the rationale prefixed Already-flagged by <tool> <rule> — surfacing for awareness only. This forecloses double-counting at the lens level; aggregator rule 7's lint-dedup catches any escapees. If lint-findings.json is absent (orchestrator chose not to run the pre-pass, or every tool was skipped), proceed with the normal investigation below — the pre-pass is optional and its absence is not an error.

Coverage claim: This lens's PRIMARY goal is to trim. Detects: tests asserting implementation details instead of behavior; duplicate tests; tautological assertions; over-mocked tests that test nothing real; tests that would pass even if the feature were broken; codifying-test-diff (assertion changes that mask new broken behavior — see docs/investigations/review-agents/06-codifying-test-diff-suspicion.md); critical behaviors with zero coverage.

Non-coverage: Does NOT request tests for trivial code. Does NOT cover whether tests pass (the gates do that). Does NOT cover style.

Evidence shape: Each finding:

file:line — <keep|delete|rewrite>
  Excerpt: `<test name and assertion>`
  Reasoning: <why>
  Anti-pattern (optional): codifying-test-diff

(For codifying-test-diff findings, cite the assertion change being suspect, not endorsing.)

Severity rubric: Block if a codifying-test-diff is detected (tests rewritten to mask broken behavior) OR if a critical Done-when behavior has zero test coverage. Nit if tests are redundant or over-mocked. FYI for observations ("this test would benefit from a property-style assertion").

Output format: Per-test verdicts (keep/delete/rewrite with one-line reason), ending with one of:

• Test quality verdict: PASS
• Test quality verdict: NEEDS-WORK (N delete, M rewrite, K Block)
• Test quality verdict: NO-EVIDENCE

Verdict threshold (uniform across all six lenses). Emit PASS if and only if findings contains zero Block AND zero Nit entries (FYI-only is PASS). Emit NEEDS-WORK if findings contains any Block or any Nit. Emit NO-EVIDENCE when the lens could not see diff content (inputs-gathering bug). No hedging in the verdict line — count Block + Nit entries and emit deterministically.

Style and conventions lens

Scope: Does this diff follow the conventions already present in the surrounding code?

Optional local-judge offload (switchboard, INERT BY DEFAULT). The Style lens is the ONLY review-deep lens cheap enough to route to a local model (tier-offload task_class review-deep-style; Switchboard Decision 9). Correctness, Bugs, Test-quality, and Plan-conformance are deep-reasoning drift-catchers (code-quality.md) and ALWAYS stay on Claude — never route them local. The Style offload is off unless switchboard offload is enabled for review-deep-style. When enabled, dispatch the Style judgment through the switchboard judge entrypoint: python -m switchboard judge --site review-deep-style --prompt-file <style-lens-prompt-file> (prints one JSON object, always exits 0). On a verdict, treat it as the Style lens's advisory result. On a defer ({"defer": true, ...}) — ALWAYS returned when offload is off, the slice is disabled, or the model is down/slow/wrong-shaped — fall back to running the Style lens on Claude per the model-tier table below. When offload is OFF (the default), the entrypoint returns a defer immediately with NO network call, so the Style lens runs on its configured Claude tier exactly as before. The local Style verdict only advises; the aggregator's pass/fail decision stays a Claude judgment.

Linter pre-pass results (read first; cite-by-reference, don't re-derive): if the orchestrator ran scripts/lint_prepass.sh, a deterministic linter sidecar lives at <--output-dir>/lint-findings.json (default .review-deep/lint-findings.json). When present, load it and enumerate findings relevant to this lens — Style lens reads style-class findings (heuristic: ruff codes matching ^E\d+$ or ^W\d+$ or the ruff style families D/I/N/Q/COM/ICN; eslint rules in the @stylistic/* namespace or named in eslint's style category). For each style-class linter finding at <path>:<line>, the lens MUST NOT re-derive the same defect: instead, emit a single FYI-severity finding at that file_line with the rationale prefixed Already-flagged by <tool> <rule> — surfacing for awareness only. This forecloses double-counting at the lens level; aggregator rule 7's lint-dedup catches any escapees. If lint-findings.json is absent (orchestrator chose not to run the pre-pass, or every tool was skipped), proceed with the normal investigation below — the pre-pass is optional and its absence is not an error.

Coverage claim: Detects naming-convention deviations from surrounding code; import ordering inconsistent with neighbors; error-handling patterns that differ (raises vs returns None); structural patterns (dataclasses vs raw dicts); comment / docstring style if the file has one. Reads the file's own surrounding code as the convention source — does NOT apply generic best practices.

Non-coverage: Does NOT flag generic best practices. Does NOT flag minor whitespace if neighbors don't enforce it. Does NOT cover correctness or bugs. Does NOT block the verdict — only Block-severity style issues do.

Evidence shape: Each finding:

file:line — <one-line summary>
  Excerpt: `<diff text>`
  Surrounding convention: `<what neighbors do>`
  Reasoning: <why this is a deviation, not just a preference>

Severity rubric: Block only if the diff breaks a convention that's enforced by tooling (linter, formatter) — i.e., the diff would fail CI. Nit for hand-readable convention deviations. FYI for minor observations.

Output format: A bulleted list of deviations, ending with one of:

• Style verdict: PASS
• Style verdict: NEEDS-WORK (N findings)
• Style verdict: NO-EVIDENCE (no diff content was visible to this lens — usually an inputs-gathering bug)

Verdict threshold (uniform across all six lenses). Emit PASS if and only if findings contains zero Block AND zero Nit entries (FYI-only is PASS). Emit NEEDS-WORK if findings contains any Block or any Nit. Emit NO-EVIDENCE when the lens could not see diff content (inputs-gathering bug). No hedging in the verdict line — count Block + Nit entries and emit deterministically.

Plan-conformance lens

Scope: Does the diff implement what the named plan step asked for? (See docs/investigations/review-agents/03-plan-conformance.md and docs/investigations/review-agents/21-requirement-coverage.md.)

Coverage claim: When --plan-step <path>:<step-id> is supplied, this lens reads the referenced plan file, locates the step in either heading-format or table-format (see Plan format detection below), extracts the step's Problem, Done when, Files, and Produces fields, and produces three per-field verdicts:

1. Done-when satisfied? Cite the file:line in the diff that satisfies each clause.
2. Files honored? Did the diff modify only the Files listed, or did it touch out-of-scope files?
3. Out-of-scope additions? Did the diff add features beyond what the step asked for (see docs/investigations/review-agents/28-scope-creep.md)?

Plan format detection: Workspace plans use two distinct formats; this lens MUST detect which one the referenced plan file uses and parse accordingly:

• Heading-format (canonical for /plan-init / /plan-feature output): scan the plan file for ^### Step <step-id>: and extract the immediately-following block of - **Problem:** / - **Done when:** / - **Files:** / - **Produces:** bullets as the step's ground-truth. Example: a plan with ### Step 5a: Author A/B fixture set followed by the four bullet fields.
• Table-format (canonical for Alpha4Gate phase-d-build-plan.md and other long-running Alpha4Gate phase plans): scan the plan file for a markdown table whose first column equals <step-id> (e.g., | D.6 | #169 | code | ... |). Extract the matched row's columns AS the step's ground-truth, AND additionally read any narrative section linked from the row's Description column (the Alpha4Gate convention is a one-line table row that points at a longer section heading later in the file via a section anchor). The lens combines the table row's column values with the linked narrative section's prose as a single ground-truth payload.
• Both formats present: if the plan file contains BOTH an ### Step <step-id>: heading AND a table row with <step-id> in the first column, the lens prefers the HEADING form (heading is canonical-authority by /plan-init convention; table is typically a summary view of the heading-form plan). Note this preference in the output's plan-format-detected field.
• Neither form locates the step: the lens emits a single Block-severity finding tagged Plan-conformance verdict: NEEDS-CLARIFICATION — could not locate step <step-id> in <plan-path> as either heading-format (### Step <step-id>:) or table-format (| <step-id> | ... |). This is treated as an inputs-gathering failure, NOT a silent skip — operator needs to verify the --plan-step argument or fix the plan file. Distinct from SKIPPED (which fires only when --plan-step is not provided at all).

Non-coverage: Does NOT cover whether the diff is bug-free (Bugs lens). Does NOT cover correctness of implementation (Correctness lens). Does NOT cover whether the plan ITSELF is good (that's /plan-review).

Evidence shape: Each finding:

<plan-file>:Step <N> — <Done-when|Files|Out-of-scope>
  Plan excerpt: `<exact text from plan step>`
  Diff evidence: `<diff line or absence>`
  Reasoning: <why this satisfies or violates the field>

Severity rubric: Block if any Done-when is unmet OR if the diff includes out-of-scope code beyond the step's stated Produces. Nit if the diff is incomplete on a non-Done-when goal. FYI for plan-vs-diff observations the operator should note.

Output format: Three per-field verdicts (Done-when / Files / Out-of-scope), each with bulleted findings + a per-field PASS/NEEDS-WORK. Final line is one of:

• Plan-conformance verdict: PASS
• Plan-conformance verdict: NEEDS-WORK (Done-when, Files, Out-of-scope counts)
• Plan-conformance verdict: NO-EVIDENCE (lens spawned but could not see diff content — inputs-gathering bug)
• Plan-conformance verdict: NEEDS-CLARIFICATION — could not locate step <step-id> in <plan-path> as either heading-format or table-format (Block-severity inputs-gathering finding — distinct from SKIPPED; operator must fix the --plan-step argument or the plan file)
• Plan-conformance verdict: SKIPPED — no --plan-step provided

Verdict threshold (uniform across all six lenses). Emit PASS if and only if findings contains zero Block AND zero Nit entries (FYI-only is PASS). Emit NEEDS-WORK if findings contains any Block or any Nit. Emit NO-EVIDENCE when the lens could not see diff content (inputs-gathering bug). No hedging in the verdict line — count Block + Nit entries and emit deterministically. Plan-conformance additionally emits SKIPPED when --plan-step is not provided at invocation (expected configuration, not a finding).

Critical: graceful skip behavior. If --plan-step is not provided at invocation, this lens is NOT spawned, NOT marked failed, and NOT treated as an error. The aggregated report (built in Step 5) notes the skip explicitly: Plan-conformance: SKIPPED (no --plan-step argument). This is not a finding — it's an expected configuration.

---

Anti-pattern catalog

The seven Tier-1 anti-patterns below are the explicit defect shapes this skill hunts for. Each has its own H3 (prefixed with anti-pattern: so the catalog is grep-able) with a fixed shape: Shape, Why it ships past unit tests, Detection prompt, Originating source, Severity, Lens routing. All seven are Block severity by definition — lower-severity defect shapes belong in the lens-specific rubrics, not here. After the seven patterns, a verify-dev-claims framing wrapper H3 captures the meta-principle behind patterns 1–3 and is referenced by the Bugs + Correctness lens prompts.

anti-pattern: silent-wiring

• Shape: A new module / function / class is added with full unit-test coverage, but never invoked from any production caller. The test passes, the gauntlet passes, and the production effect is zero forever.
• Why it ships past unit tests: Unit tests of the new module exercise it directly via import; they don't verify any production code path actually reaches it.
• Detection prompt: When the diff adds a new module, function, class, or entry point that's intended to be called from production, grep the entire codebase for the new name. If the only references are tests + the new module itself + its imports for testing, flag as Block: anti-pattern: silent-wiring. The fix is an integration test that exercises the production entry point and asserts the new component is reached end-to-end (caller-first, not callee-first).
• Originating source: .claude/rules/code-quality.md § "New components require an integration test through the production caller"; docs/investigations/review-agents/04-silent-wiring-detection.md; toybox step 15 iter 1 (developer built schedule_judge_sample and unit-tested it directly, never invoked from _do_propose — production effect would have been zero judge calls forever).
• Severity: Block.
• Lens routing: Bugs primary; Correctness secondary (the diff "fails to do what it claims" whenever the new code is never reached).

anti-pattern: producer-consumer-drift

• Shape: A producer's primary key, cache key, id format, filename format, or shape constant changes, but downstream consumers are not all updated. Tests on either endpoint mock the other and pass; production crashes on the seam.
• Why it ships past unit tests: Endpoint tests mock the OTHER endpoint; the bug lives in the relationship between producer and consumer, not in either endpoint individually.
• Detection prompt: When the diff changes the shape of a key, id, schema, filename pattern, or any value referenced from multiple call sites, grep every consumer of the old shape. If any consumer still references the pre-change shape, flag as Block: anti-pattern: producer-consumer-drift. The fix attaches a grep-results table to the PR/issue (one row per call site, verdict OK | needs fix | already handled) AND adds an integration test that exercises the full producer → consumer round trip.
• Originating source: .claude/rules/code-quality.md § "Grep all downstream consumers when changing a key/id shape"; docs/investigations/review-agents/05-producer-consumer-drift.md; Alpha4Gate Phase 4.6 Step 1 (changed SC2Env._game_id shape but missed evaluator._get_game_result(base_id); Soak-4 spent 70 minutes with 12 eval games flagged "crashed" before DB forensics found the missed caller; cost a whole Phase 4.7 plan-review-repo-sync-build-phase cycle).
• Severity: Block.
• Lens routing: Bugs primary; Correctness secondary.

anti-pattern: codifying-test-diff

• Shape: A PR changes how data is persisted, narrows a response shape, or removes a field; existing tests start failing; the developer updates the failing tests to match the new (broken) behavior; tests now pass; the regression is codified into the suite.

• Why it ships past unit tests: Test diffs that adjust assertion shape look like normal test maintenance; reviewers approve the codifying diff because the test now passes against the new behavior.

• Detection prompt: When the diff includes BOTH (a) a change to storage representation / normalization / API response shape AND (b) modifications to existing test assertions, treat the test changes as suspect, NOT endorsing. Read the original assertion vs the modified one and ask: "did the new behavior change to match the new assertion, or did the assertion change to mask new behavior?" If the latter, the default severity is Nit-suspicion: surface the finding so the reviewer or operator can adjudicate.

  Tie-breaker (escalation / downgrade):

  • Escalate to Block: anti-pattern: codifying-test-diff when the assertion change masks behavior AND the dev provides no documenting context (no plan step asks for the shape change, no changelog entry, no migration note, no commit body explaining intent).
  • Downgrade to FYI: anti-pattern: codifying-test-diff (documented) when documenting context exists AND the assertion update is internally consistent with that documented change. Surface the verdict so it lands in the audit trail; do not block on it.

  When --plan-step is supplied, the plan-conformance lens is the natural tie-breaker: if the plan step's Problem / Done-when explicitly asks for the shape change, the assertion update is endorsed by the plan (downgrade applies). When the plan is silent on the shape change, escalation applies. Pay extra attention to endpoints the frontend renders directly without intermediate code (suggestion cards, dashboards, summaries).

• Originating source: .claude/rules/code-quality.md § "Audit wire shape when storage representation changes"; docs/investigations/review-agents/06-codifying-test-diff-suspicion.md; toybox G2 silently narrowed a propose response from 5 steps to 1; the G2 dev agent updated 6 integration tests to assert len(steps) == 1; four reviewers approved the codifying test diff; caught only when the operator hit the parent UI.

• Severity: Block.

• Lens routing: Test quality primary; Bugs secondary.

anti-pattern: silent-fallthrough-in-loop

• Shape: A try/except (or try/catch) inside a for / while loop where code BELOW the try block assumes the work in the try succeeded (reads a result, advances state, saves a checkpoint, increments a counter). The catch path swallows the exception without continue or re-raise, so the success-path code runs on stale or missing data.

• Why it ships past unit tests: The exception path is rarely hit in tests; when hit, the loop appears to "keep running" which looks like resilience rather than silent corruption.

• Detection prompt: Detect try/except blocks whose lexical parent is a for or while statement (i.e., the try/except is directly inside the loop body, not inside a helper function called from the loop). The except clause logs but does not continue, break, or re-raise — and code BELOW the try block uses variables (a result, a checkpoint, a counter) that would be unbound or stale on the exception path. That's a finding. The catch path must either re-raise OR set a failed = True flag and continue BEFORE the success-path code. Flag as Block: anti-pattern: silent-fallthrough-in-loop. Scope exclusion: try/except inside helper functions called from a loop is the silent-fallthrough-in-hot-path variant — see the next catalog entry.

  Example shape that's almost always wrong:

  for item in items:
      try:
          result = expensive_op(item)
      except Exception:
          log.exception("crashed")
          # NOTHING ELSE — falls through
      # BUG: this runs even when expensive_op crashed
      save_checkpoint(result)
      advance_curriculum(result)
  

• Originating source: ~/.claude/skills/review-gauntlet/SKILL.md § "Bug Reviewer" anti-pattern 1; Alpha4Gate Phase 4.5 (found by a 4-minute smoke test, not by 682 unit tests — the most dangerous failure mode for autonomous loops is zero observable signal).

• Severity: Block.

• Lens routing: Bugs primary.

anti-pattern: silent-fallthrough-in-hot-path

• Shape: A try/except (or try/catch) inside a function that is itself called ≥100 times per session — e.g., per RL step, per HTTP request, per render frame, per game tick. The except clause logs but does not re-raise; the function returns normally on the exception path; the caller has no signal that the call failed and proceeds as if the work succeeded.
• Why it ships past unit tests: Unit tests typically call the helper once with a working input; the silent-fail pathway never fires. The hot-path call site (RL training step, request handler, render loop) is not exercised in unit-test scope, so the caller's downstream assumptions about the helper's success are never tested against a thrown-and-swallowed exception.
• Detection prompt: Identify functions whose call sites are inside a for/while loop, an RL training step, an HTTP request handler, or a per-frame render path. For each such function, check whether its body contains try/except that logs without re-raising AND where the function returns normally on the exception path. If the caller's success-path semantics depend on the function having succeeded (the caller reads a return value, advances state, or commits side effects), flag as Block: anti-pattern: silent-fallthrough-in-hot-path. The fix is either re-raise on the exception path, or return a sentinel / Result that the caller MUST inspect, with the caller updated to branch on the sentinel before its success-path code runs. Scope contrast: silent-fallthrough-in-loop covers the lexical-loop case (try/except directly inside a for/while body); this entry covers the helper-function case where the loop is elsewhere.
• Originating source: review-deep M1 smoke (Alpha4Gate Phase D.6, 2026-05-22) — hot-path function train_step() contained a try/except that logged without re-raising; the per-RL-step caller had no signal that a step had silently failed, so curriculum advance and checkpoint writes proceeded against stale state. The v1 smoke surfaced this as a Block via the Bugs lens but used the silent-fallthrough-in-loop tag, which was a scope mismatch — the try/except was not in a literal loop body. Splitting the catalog entry per v2 plan Design decision 5.
• Severity: Block.
• Lens routing: Bugs primary; Correctness secondary (caller "fails to do what it claims" when the silent-fail path fires).

anti-pattern: duplicate-shape-constants

• Shape: A literal that defines the shape of data — Discrete(N), Box(shape=(N,)), a list of column names, an action enum, a feature dimension, a schema definition — exists in two or more places. Tests mock either side and pass; producer-consumer drift inevitably follows when one site updates and the other doesn't.
• Why it ships past unit tests: Tests with mocks can't see drift; == value-equality assertions pass even after one side drifts because both started from the same literal.
• Detection prompt: When the diff introduces a literal that defines data shape, grep the diff's transitive imports for the same value or name. If it exists in two places, that's a finding. Flag as Block: anti-pattern: duplicate-shape-constants. The fix is single-source-of-truth: pick a leaf module that both sites can import from, define the constant there once, import it from both sides. Regression tests should assert object identity (is), not just value equality (==), so a future re-introduction of a duplicate copy fails CI even if the values happen to match at first.
• Originating source: .claude/rules/code-quality.md § "One source of truth for data-shape constants"; ~/.claude/skills/review-gauntlet/SKILL.md § "Bug Reviewer" anti-pattern 2; Alpha4Gate Phase 4.5 found 4 instances in one debugging session, all four invisible to 682 unit tests, all four caught by a 4-minute smoke test.
• Severity: Block.
• Lens routing: Bugs primary; Correctness secondary.

anti-pattern: create-table-without-migration

• Shape: A CREATE TABLE statement is modified (adding a column, changing a type, adding a constraint) AND the table is opened against EXISTING database files in production, but the diff has no migration code that ALTERs existing tables. SQLite's CREATE TABLE IF NOT EXISTS is a no-op for existing tables — it does NOT add new columns.
• Why it ships past unit tests: Unit tests create empty test fixtures; the bug only fires against pre-existing production databases.
• Detection prompt: When the diff modifies a SQL CREATE TABLE statement (or any persistent format with a versioned schema: pickled state, JSON files with structured shape, protobuf, etc.) AND the table is opened against EXISTING database files in production, verify the diff also adds migration code that ALTERs existing tables OR a generic migration walker that compares declared columns to live columns and ALTERs the diff. If absent, flag as Block: anti-pattern: create-table-without-migration.
• Originating source: ~/.claude/skills/review-gauntlet/SKILL.md § "Bug Reviewer" anti-pattern 3.
• Severity: Block.
• Lens routing: Bugs primary.

verify-dev-claims framing wrapper

Both the Bugs lens and the Correctness lens spawn their sub-agents with an adversarial framing preamble that overrides any tendency to trust the developer's stated intent:

Ignoring the dev's stated intent, ignoring what the prompt SAYS the diff does — does this diff actually accomplish what it claims? Verify each claim against the code, not the prompt. The dev wrote the prompt and the code; both can be wrong; the code is the ground truth, the prompt is a hypothesis to test.

The Correctness lens uses this framing to detect cases where the diff plausibly LOOKS like it implements the prompt but actually doesn't (silent breaks, missing wiring, intent-vs-action drift). The Bugs lens uses it to actively hunt for failure modes the dev probably didn't consider — adversarial "find what breaks" framing rather than confirmatory "verify it works" framing.

This pattern is the meta-principle behind anti-patterns 1 (silent-wiring), 2 (producer-consumer-drift), and 3 (codifying-test-diff): each is a class of case where the dev's claim ("the new module is reachable" / "the consumer is updated" / "the test changes were correct") doesn't survive verification against the code. See docs/investigations/review-agents/07-verify-dev-claims.md.

---

Model strategy

Each lens runs on a model tier appropriate to its judgment intensity. Fresh-context sub-agent fan-out (already the pattern in the gauntlet) ensures producer-grader split by context isolation regardless of model — the grader never shares context with the producer, so the model tier per lens is purely a cost/accuracy decision, not a "does the grader see the dev's reasoning?" decision. See docs/investigations/review-agents/01-producer-grader-split.md for the long-form rationale.

Default model-tier table

Lens	Default tier	Rationale
Style	Haiku 4.5	Narrow, cheap, well-specified
Test quality	Sonnet 4.6	Mid-stakes judgment
Plan-conformance	Sonnet 4.6	Mid-stakes; reads plan + diff
Bugs	Sonnet 4.6	Anti-pattern catalog covers most ground
Security	Sonnet 4.6	Deep-reasoning drift-catcher; asymmetric-cost defects — never Haiku
Correctness	Sonnet 4.6	Diff-vs-intent comparison

Concrete model IDs: claude-haiku-4-5-20251001 (Haiku 4.5), claude-sonnet-4-6 (Sonnet 4.6), and claude-opus-4-7 (Opus 4.7). Tier names in this skill always refer to the latest model in each family; pin a specific snapshot via --model-override if needed.

The Style row corresponds to the ### Style and conventions lens H3; for --model-override the flag name is style (the shorter form documented in the syntax below). All other lens names map kebab-case identically: Test quality → test-quality, Plan-conformance → plan-conformance, Bugs → bugs, Security → security, Correctness → correctness.

--model-override flag semantics

The --model-override flag (declared in the Arguments table at the top of this file) lets the operator swap the default tier for any single lens on a per-invocation basis.

• Syntax: --model-override <lens>=<tier> where <lens> is one of style | test-quality | plan-conformance | bugs | security | correctness and <tier> is one of haiku | sonnet | opus. Tier names map to the latest model in each family: haiku → claude-haiku-4-5-20251001, sonnet → claude-sonnet-4-6, opus → claude-opus-4-7.
• Repeatable: the flag may be passed multiple times to override multiple lenses in one invocation (e.g., --model-override bugs=opus --model-override correctness=opus).
• Effect: overrides the default tier for the named lens for THIS invocation only. The default table above is unchanged; subsequent invocations without the flag return to defaults.
• Example use cases:
  • --model-override bugs=opus — bump the Bugs lens to Opus for a high-blast-radius diff (substrate refactor, schema change touching many consumers).
  • --model-override style=sonnet — bump Style to Sonnet when a diff touches a file with intricate conventions a Haiku reviewer might miss.
  • --model-override correctness=haiku — downgrade Correctness to Haiku for a known-trivial diff (rare; primarily for cost-tuning experiments).
• Invalid input:
  • Unknown lens name → exit with an error message listing the valid lens names (style | test-quality | plan-conformance | bugs | security | correctness). Do NOT silently skip.
  • Unknown tier name → exit with an error message listing the valid tier names (haiku | sonnet | opus).
  • Malformed flag (missing = or empty side) → exit with an error and the example syntax --model-override <lens>=<tier>.

Overload-fallback behavior

When a lens's primary model returns 529 Overloaded or times out (>180s without first-token):

1. Retry once at the same tier with a 30-second backoff. The retry inherits the same 180s first-token timeout as the primary attempt — if the retry also exceeds 180s without first-token, treat it as failed and proceed to step 2.

2. If the retry also fails (529 or timeout), surface the lens as FAILED in aggregation — NOT silently skipped. The aggregated report includes <Lens> verdict: FAILED — model overloaded after 1 retry at tier <tier>. The JSON sidecar (Step 5) records overall_verdict: "FAILED" for that lens with a failure_reason: "model_overloaded" field.

   Cross-step contract: This field name (failure_reason) and its allowed values (currently "model_overloaded"; future additions documented here) are owned jointly by this section and the Step 5 aggregator's JSON schema. Step 5 MUST adopt this exact field name in lens_verdicts[] entries when overall_verdict == "FAILED". If Step 5 needs to rename the field, update this section in the same diff. Drift between this declaration and Step 5's schema is a producer-consumer-drift finding under the anti-pattern catalog.

3. Do NOT auto-fall-back to a different tier without operator instruction. Silent tier-fallback would mask cost / accuracy expectations and codify an "anything goes" review pass. The operator can re-invoke with --model-override <lens>=<other-tier> if they want to retry at a different tier.

This is the "fail loud, never silently degrade" principle from .claude/rules/code-quality.md § "Audit wire shape when storage representation changes" applied to model-tier degradation.

Per-lens overall_verdict enum (Cross-step contract): The complete set of values a lens can emit as overall_verdict is:

Value	Set by	Meaning
PASS	lens sub-agent	No findings or only minor findings; lens approves
NEEDS-WORK	lens sub-agent	Lens found Block- or Nit-severity findings the dev should address
NO-EVIDENCE	lens sub-agent	Lens could not see diff content (usually an inputs-gathering bug)
FAILED	this section (overload-fallback)	Lens's model overloaded after 1 retry — see failure_reason field
SKIPPED	Plan-conformance lens only	--plan-step was not provided; lens deliberately skipped (NOT an error)
NEEDS-CLARIFICATION	Plan-conformance lens only	--plan-step was provided but the step could not be located in either heading-format or table-format; aggregator maps to NEEDS-WORK like a Block finding (inputs-gathering failure — distinct from SKIPPED)

Step 5's aggregator MUST handle all six values explicitly — including the FAILED, SKIPPED, and NEEDS-CLARIFICATION branches, which the happy-path PASS / NEEDS-WORK / NO-EVIDENCE ladder does not cover. Mapping to the top-level aggregated_verdict.result ladder (PASS / NEEDS-WORK / DEFERRED-TO-UAT) is owned by Step 5 — NEEDS-CLARIFICATION maps to NEEDS-WORK at the aggregated level (it is a Block-severity inputs-gathering failure); this table is the source of truth for what each lens can emit as its overall_verdict.

Cost / latency note

Haiku 4.5 is roughly 5-10x cheaper and 2-3x faster per token than Sonnet 4.6 — that gap is why Style (mechanical convention-matching) defaults to Haiku while the five judgment-heavy lenses default to Sonnet. For large diffs, context-window cost dominates tier choice; prefer shrinking the diff or narrowing --reviewers over escalating tiers. Cost-modeling detail in docs/investigations/review-agents/23-context-window-cost.md and docs/investigations/review-agents/24-cost-vs-risk.md.

---

Aggregation

After all spawned lens sub-agents return, the orchestrator collects each per-lens verdict, applies the aggregator rules below, and emits both a JSON audit-trail sidecar at .review-deep/<timestamp>.json and a human-readable markdown summary. The aggregator is fully deterministic — given the same per-lens verdicts, two invocations produce byte-identical sidecars (modulo the timestamp field).

Per-lens verdict shape

Each lens sub-agent emits one LensVerdict object that the orchestrator collects into the JSON sidecar's lens_verdicts[] array. The canonical schema lives in scripts/aggregate.py (LensVerdict dataclass) with per-field docstrings; the required fields are lens_id (one of correctness | bugs | security | test-quality | style | plan-conformance for code lenses), model_tier (haiku | sonnet | opus, post---model-override resolution; stable even when the lens SKIPs so tier-mix queries don't need to handle nulls), authority and coverage_claim (one-line strings from the lens's Scope / Coverage claim sub-sections), findings (empty list when no findings — never null/absent), and overall_verdict (one of PASS | NEEDS-WORK | NO-EVIDENCE | FAILED | SKIPPED | NEEDS-CLARIFICATION per Model strategy § per-lens overall_verdict enum). The failure_reason field is present ONLY when overall_verdict == "FAILED"; current allowed value is "model_overloaded" (cross-step contract with Model strategy § Overload-fallback). A rename or new value MUST touch both scripts/aggregate.py AND this paragraph in the same diff — drift between the two sites is a producer-consumer-drift finding.

Per-finding shape

Each entry in lens_verdicts[].findings[] is a Finding object whose canonical schema lives in scripts/aggregate.py (Finding dataclass) with per-field docstrings. The required fields are severity (Block | Nit | FYI per the universal rubric at the top of Reviewer lenses), file_line (<path>:<line> for a single line or <path>:<start>-<end> for a range; findings missing file_line are dropped during aggregation per the universal evidence discipline), excerpt (the exact cited text; empty excerpts demote one severity tier), and rationale (one sentence to one paragraph referencing the lens's coverage claim, the dev's stated intent, the named plan step, or a catalog anti-pattern). The optional anti_pattern field, when present, MUST be one of the seven catalog entries (silent-wiring, producer-consumer-drift, codifying-test-diff, silent-fallthrough-in-loop, silent-fallthrough-in-hot-path, duplicate-shape-constants, create-table-without-migration) plus the reserved wire-format marker scope-boundary-deferral (NOT a defect; signals a deferral hand-off — see Scope-boundary deferral). Any other value is invalid and causes the orchestrator to drop the finding. Two optional post-aggregation annotations may appear on a finding after rule 7 runs (mutated in place; NOT raw lens output): also_flagged_by (list of sibling lens_id strings, populated by the absence-dedup path when multiple lenses clustered on the same absence) and also_flagged_by_linter (a {tool, rule} dict, populated by the lint-dedup path when a lint finding's file_line overlaps the lens finding's file_line). Both are audit-trail annotations only — a fresh-context reader of a sidecar sees them on the primary surviving finding for each cluster, not on raw lens emissions.

Aggregator rules

scripts/aggregate.py implements seven deterministic rules whose full per-rule docstrings are the canonical authority on edge cases — read aggregate.py for the source of truth. Invocation: python scripts/aggregate.py --lens-dir <dir> --output-dir <dir> [--skill-version v3] [--prior-sidecar <path>] [--lint-findings <path>]. The rule names map to functions (apply_rule_N_<desc>) and the contract summary is: rule 1 severity-dominance (Block outweighs any number of Nits; aggregated verdict tracks max severity across lenses, not finding count; per docs/investigations/review-agents/14-severity-tiers.md); rule 2 lens-owns-dimension (when two lenses flag the same file_line, the lens whose Scope covers that dimension or the catalog anti-pattern's Lens routing primary wins, others demote to FYI with rationale prefixed Demoted per rule 2: so rule 6 can detect persistent disagreement on re-run; per docs/investigations/review-agents/12-disagreement-resolution.md. Security is the registered PRIMARY OWNER of security findings: when both Bugs and Security flag the same security-class defect — defect_class/anti_pattern one of injection-as-data, unsafe-config-without-startup-guard, secret-dump (the three .claude/rules/security.md failure modes), mapped to security in aggregate.py's ANTI_PATTERN_PRIMARY — Security wins as primary and the Bugs finding demotes to FYI cross-tag, so security findings are owned by Security, not Bugs); rule 3 SKIPPED-handling (plan-conformance SKIPPED when --plan-step absent; included in lens_verdicts[] for trace completeness, does NOT downgrade aggregated_verdict.result); rule 4 FAILED-handling (model-overloaded lens maps to NEEDS-WORK with failure_reason echoed in the aggregated rationale; no silent tier fallback; operator re-invokes with --model-override <lens>=<other-tier> to retry); rule 5 NO-EVIDENCE-handling (inputs-gathering bug; maps to NEEDS-WORK so the operator notices rather than accepting a vacuous PASS); rule 6 persistent-disagreement (when --prior-sidecar <path> supplied AND a current finding matches a prior demoted finding's identity — file_line + anti_pattern + — escalate severity by one tier and prefix rationale ; without the rule never fires); rule 7 absence-of-thing dedup AND lint-finding dedup (TWO independent dedup paths; both run inside ). The absence path: when multiple lenses surface findings about the same absence — no concrete anchor in the diff per 's heuristic — dedup by and demote duplicates to ; primary lens is the catalog target or, absent an anti-pattern, the first lens to emit by correctness → bugs → security → test-quality → style → plan-conformance. The lint path: when a is supplied (via or auto-discovered at / ), any lens finding sharing a concrete with a lint finding demotes to with the rationale prefixed and an annotation for audit-trail completeness. The lint path is independent of the absence path; both run inside rule 7. Rule 7 fires BEFORE rule 6 in the pipeline so persistent-disagreement escalation applies only to the surviving primary; prevents cross-lens double-counting like the M1 smoke case where Bugs + Test quality both flagged "missing integration test" and inflated the Nit count to a noisy NEEDS-WORK.

Renames or shape changes to any rule MUST touch both scripts/aggregate.py AND this section in the same diff (producer-consumer-drift prevention per dev/.claude/rules/code-quality.md).

Aggregated-verdict output ladder

The top-level aggregated_verdict.result is one of three values, computed from the per-lens verdicts via the rules above.

Verdict	Triggers
PASS	Zero Block findings across all lenses; zero NEEDS-WORK / FAILED / NO-EVIDENCE lens verdicts; SKIPPED lenses present but do not block
NEEDS-WORK	At least one Block finding; OR any lens overall_verdict is NEEDS-WORK / FAILED / NO-EVIDENCE; OR aggregated Nit count ≥ 3 across all lenses (soft-fail threshold to surface noisy diffs)
DEFERRED-TO-UAT	The skill structurally cannot evaluate part of the diff (Step 7 — Scope-boundary deferral — defines the trigger conditions); deferral takes priority over PASS but NOT over NEEDS-WORK on the same diff

Interaction between NEEDS-WORK and DEFERRED-TO-UAT: if BOTH apply on the same diff (one part has Block findings, another part is structurally un-evaluable), the markdown summary names BOTH in the header line (# review-deep: NEEDS-WORK + DEFERRED-TO-UAT), but the JSON sidecar uses aggregated_verdict.result == "NEEDS-WORK" (the harder verdict wins for gating purposes) and the deferred items still populate deferred_uat_items[] regardless. This keeps machine-readable gating monotonic while preserving the deferral signal for the operator.

When only DEFERRED-TO-UAT applies (zero Block findings, zero FAILED/NO-EVIDENCE lenses, but Step 7's deferral fires for un-evaluable parts of the diff), aggregated_verdict.result == "DEFERRED-TO-UAT" and the markdown header reads # review-deep: DEFERRED-TO-UAT. This case is the "pure deferral" path: lenses that could run all passed; lenses that couldn't run surface as deferrals.

Audit-trail JSON sidecar schema

After aggregation, the orchestrator writes one JSON file at <--output-dir>/<timestamp>.json (default --output-dir is .review-deep/). The canonical schema lives in scripts/aggregate.py (write_sidecar / build_sidecar); reading the script is the source of truth for field semantics. The top-level keys are, in this order: timestamp (filesystem-safe ISO 8601, YYYY-MM-DDTHH-MM-SS, MUST match the filename), plan_step (string <path>:<step-id> or null), skill_version (pinned string, "v3" for v3 sidecars; lets future schema migrations identify old sidecars), invocation (echoes the resolved argument values: reviewers_flag, model_overrides, force_runtime, url, start_cmd, runtime_downgraded, runtime_downgrade_reason), lens_verdicts (a 6-element stable prefix — one per code lens, including SKIPPED entries with findings: [] — followed by 0..N runtime-lens entries at index 6+; SKIPPED lenses are present so the aggregator can rely on stable indexing), aggregated_verdict ({result, rationale}; result is one of PASS | NEEDS-WORK | DEFERRED-TO-UAT per the output-ladder rules below), model_tiers_used (a flat lens_id → tier map, separate from lens_verdicts to support quick queries without iterating findings), and deferred_uat_items (array of deferred-item dicts populated by the Scope-boundary deferral logic; empty array — not null, not absent — when no deferrals apply). The schema MUST be valid JSON (no comments). Any rename or new top-level key MUST touch both scripts/aggregate.py AND this paragraph in the same diff.

Markdown output shape

After writing the sidecar, the aggregator prints a human-readable markdown summary to stdout (suitable for paste into PR comments). The canonical renderer lives in scripts/aggregate.py (render_markdown). Structure: a header line (# review-deep: PASS / NEEDS-WORK / DEFERRED-TO-UAT / NEEDS-WORK + DEFERRED-TO-UAT), a ## Lens verdicts subsection with one line per lens (- <lens_id>: <overall_verdict> (<finding-count> findings, model: <model_tier>); SKIPPED lenses are shown explicitly so the operator always sees the full 6-lens trace; a runtime-downgrade line follows when the auth-gate probe fired), a ## Findings subsection grouped by severity (### Block, ### Nit, ### FYI) where each finding renders as - **<file_line>** (<lens_id>[, anti-pattern: <name>]): <rationale> followed by the excerpt as a fenced sub-block (severity subsections with zero findings are omitted), an optional ## Deferred to operator UAT subsection present only when deferred_uat_items[] is non-empty (rendering per the Scope-boundary deferral § Markdown rendering contract — H3 per item, covered-lenses bullet, needs-verification bullet, commands fenced powershell block), an explicit Please run M<N> next. cue line when deferrals exist (lowest M-number in the sidecar), and a final Audit-trail JSON: .review-deep/<timestamp>.json pointer. Markdown shape changes MUST touch both scripts/aggregate.py AND this paragraph in the same diff.

---

Calibration

This skill is the FIRST judge wired to the deterministic per-commit calibration gate that operationalizes _shared/judge-core.md §7's Discrimination guard. Run it as:

python .claude/skills/_shared/calibrate_judge.py --skill review-deep --mode ci

It prints PASS and exits 0 when the recorded judge snapshot is fresh, discriminating, and in agreement with the gold labels; it prints FAIL and exits 1 (fail-closed) otherwise. No LLM is invoked — every check replays recorded artifacts.

The two gold artifacts

Both live under evals/golden/ and are the calibration contract (schema source-of-truth is calibrate_judge.py's module docstring):

• verdicts.jsonl — the hand-authored GOLD labels, one JSON object per fixture (good.md → PASS; each bad_*.md → NEEDS-WORK carrying its defect_type from manifest.json as expected_block_anti_pattern provenance). All 30 golden fixtures (1 good + 29 bads) appear here.
• recorded_scores.json — the recorded judge snapshot: generated_at (freshness anchor), scores (numeric score per fixture → drives the discrimination check: good.md must strictly out-score every bad_*.md), and recorded_verdicts (the categorical verdict the judge produced when snapshotted → drives the agreement check against verdicts.jsonl). All 30 fixtures appear in both maps; a missing fixture fails the gate closed.

Operating rules

• Label-edit-separate-commit (code-review discipline, not gate-enforced). Any change to verdicts.jsonl gold labels SHOULD land in its OWN commit — never bundled with a SKILL.md or judge-logic change — so a reviewer can scrutinize the relabeling in isolation and confirm it is not masking a judge degradation. This is enforced by review practice: calibrate_judge.py inspects artifact content, not git commit history, so it cannot catch a bundled relabel on its own.
• Freshness (timestamp age only). The gate checks that recorded_scores.json's generated_at is within FRESHNESS_MAX_AGE_DAYS (180 days) — a stale (or future-dated) timestamp fails. It checks the timestamp's AGE only; it cannot tell whether the snapshot was actually re-generated. When the seed ages out, honestly re-snapshot by re-running the judge — bumping generated_at without re-running is a code-review concern the gate cannot detect.

Honest scope (what this gate does NOT catch)

The per-commit gate catches regressions in the recorded snapshot and the fixture harness: a fixture dropped from a map (fails closed), a non-discriminating recorded snapshot (good.md no longer out-scores the bad_*.md fixtures), or a mislabeled gold (recorded verdicts no longer match verdicts.jsonl). It does NOT exercise the live judge or the live aggregator (aggregate.py) — every check replays recorded numbers, so a bug introduced into the aggregator or judge logic AFTER the snapshot is invisible until recorded_scores.json is regenerated by re-running the judge. In particular it does NOT catch judge-quality drift: a judge that has silently gotten worse but whose old snapshot is still on disk will still pass. Detecting a degraded live judge needs the stochastic kappa-vs-gold sweep (re-running the judge over position-swapped pairs), which is deferred to Phase 3 — a live LLM call cannot be made flake-free, so it must be a nightly-alerting signal, not a per-commit gate (calibrate(mode="full") is the Phase-3 stub for it and currently raises NotImplementedError).

---

Flag selection

The --reviewers code|runtime|full flag picks which review lane runs against the diff. Three values are valid; the default is code.

--reviewers flag values

Value	What runs	Required deps
code (default)	The 6 code lenses (correctness, bugs, security, test-quality, style, plan-conformance)	None — diff input is sufficient
runtime	Runtime lenses (UI / Backend / Frontend per review-gauntlet's runtime model, dispatched model: sonnet per the arm pin)	--start-cmd and --url
full	All 6 code lenses + the runtime lenses	--start-cmd and --url

When --reviewers runtime is specified, the 6 code lenses do NOT run; only the runtime lenses do. When --reviewers full is specified, both fire. Per the cross-step lens_verdicts[] contract (Aggregation § Audit-trail JSON sidecar schema), the 6 code-lens entries appear at indices 0–5 always — with overall_verdict: "SKIPPED" and empty findings: [] arrays when only runtime mode runs — and runtime-lens entries (if any) appear at index 6 and onward. This keeps the stable 6-element prefix consumers can rely on regardless of which lane was selected.

Selection rule of thumb: default to code. Add runtime only when the step's Done when: includes a visual, screenshot, or HTTP-response assertion the diff alone cannot evaluate AND a fresh Playwright context can actually reach the verifiable state. Use full when the step spans backend logic and frontend behavior and both lenses pay off.

Static vs runtime authority

The two lanes have structurally different blast radii — neither one is a superset of the other. Per docs/investigations/review-agents/20-static-vs-runtime-authority.md:

Authority	Can see	Cannot see
Static (code lenses)	Diff text; surrounding source files; plan step (if --plan-step); git history; project conventions	Whether the app actually starts; whether routes 200; visual regressions; backend log output; frontend console errors; network failures
Runtime (runtime lenses)	App startup behavior; HTTP status codes; rendered HTML; screenshots; backend logs; frontend console; HAR network capture	Logic in code paths not exercised by the runtime probe; test files (unless run); plan-conformance (Static owns this); subtle correctness in untaken branches

Implication: static can't tell you whether the diff RUNS; runtime can't tell you whether the diff IS CORRECT in the parts that didn't execute. A runtime-only run on a logic-heavy diff can ship a correctness bug the screenshots happen to miss; a code-only run on a real visual diff can ship a layout break the diff text reads as fine. full mode is the union — pay for both lenses when the diff's blast radius justifies the cost. The choice is which bug surface is visible at all, not a calibration knob. See investigation 20 for the long-form rationale and the open question of whether full disagreements between a code-HIGH and a runtime-CONFIRMED should be resolved by aggregator rule 2.

Auth-gated runtime downgrade

When --reviewers runtime or --reviewers full is specified, the skill probes --url BEFORE spawning runtime reviewers. The probe is a single HTTP GET with a 10-second timeout. The probe result determines whether to downgrade per the rationale in docs/investigations/review-agents/11-auth-gated-runtime.md (toybox K17 hit: --url http://127.0.0.1:4000/child sat behind a kiosk PIN gate; four screenshots all showed "Enter parent PIN" and the reviewer cleanly PASSed the gate while zero of the diff's behavior was exercised).

Auth-gate detection (any of these triggers downgrade):

1. HTTP status 401 (Unauthorized) — explicit auth challenge.
2. HTTP status 302 (Redirect) to a Location URL containing /login, /signin, /auth, or /sso in its path (case-insensitive) — typical auth redirect pattern. Other 3xx codes (301/303/307/308) follow the same rule when the redirect target matches the path heuristic. The reason-string format is redirect_to_login:<full-Location-URL> (the URL is included verbatim as curl reports it via %{redirect_url}).
3. HTTP status 200 with login-shaped HTML — body contains, case-insensitive, an <form> element and (type="password" OR name="password" OR name="pin"). The form opener and the password input may be on different lines (real-world HTML is typically multi-line); implementations MUST use a multi-line-aware matcher (e.g., grep -iz, pcre2grep -M, or whole-body matching in a non-line-oriented language). This catches single-page apps and server-rendered pages that show a login form at HTTP 200.
4. Probe timeout (no response within the 10s budget — covers both connect-timeout and total-transaction-time exhaustion). Treated as un-evaluable. Downgrade to code-only (do NOT assume auth-gated — the lenses can't run either way, and the timeout itself is the signal).

On downgrade:

• Runtime lenses are NOT spawned.
• The aggregated report's markdown summary includes a Runtime downgrade: <reason-token> (<gloss>) line in the ## Lens verdicts subsection — where <reason-token> is the literal enum value from the JSON sidecar (status_401 / redirect_to_login:<url> / login_form_in_200_body / timeout) and <gloss> is a human-readable expansion (e.g., HTTP 401 Unauthorized, Redirected to /login, Login form detected in 200 body, Probe exceeded 10s timeout). The enum token is the cross-step contract; the gloss is presentation.
• The JSON sidecar's invocation.reviewers_flag echoes the ORIGINALLY-requested value (runtime or full) — downgrades do NOT rewrite the requested flag, so the audit trail preserves operator intent. Two new fields are set: invocation.runtime_downgraded = true AND invocation.runtime_downgrade_reason = "<reason>". The reason string is one of "status_401", "redirect_to_login:<full-Location-URL>", "login_form_in_200_body", or "timeout". (Cross-step contract with Aggregation § Audit-trail JSON sidecar schema: these two fields are part of the invocation object's REQUIRED schema and the example block above includes them.)
• For --reviewers full, the 6 code lenses still run — only the runtime lenses are dropped. aggregated_verdict.result is computed from the code-lens verdicts as usual.
• For --reviewers runtime (pure runtime mode), downgrade means runtime lenses are dropped AND the 6 code lenses do NOT run (they weren't selected). The 6-element-prefix contract is honored by emitting all 6 code-lens entries with overall_verdict: "SKIPPED" and findings: [] (skipped because they weren't selected, not because of plan-conformance's gracefully-skip rule — the SKIPPED verdict shape is the same; the rationale string differentiates: "Skipped: --reviewers runtime selected, code lenses not requested"). The aggregator emits aggregated_verdict.result == "NEEDS-WORK" with rationale citing the runtime downgrade. The operator can re-invoke with --force-runtime if the probe was a false positive, or downgrade the invocation to if static review is sufficient.

Cross-step schema parity (producer-consumer-drift prevention). The two fields above (runtime_downgraded and runtime_downgrade_reason) are owned jointly by this section AND by the Aggregation § Audit-trail JSON sidecar schema example block. The example block has been updated in the same diff that introduced this section. If a future change renames either field, update both sites in the same diff — drift between the two sites is a producer-consumer-drift finding under the anti-pattern catalog (see Anti-pattern catalog § anti-pattern: producer-consumer-drift).

--force-runtime override

--force-runtime is a boolean flag (declared in the Arguments table at the top of this file) that disables the auth-gate probe. When set:

• The probe is NOT run.
• Runtime lenses fire regardless of what --url returns.
• The JSON sidecar's invocation.force_runtime is true; invocation.runtime_downgraded stays false; invocation.runtime_downgrade_reason stays null.

--force-runtime is for two cases:

1. Known-false-positive auth-gate detection. The page renders a <form type="password"> for an unrelated reason (e.g., a search filter, a password-strength demo, an embedded credential-rotation widget) and the operator knows the runtime reviewers can still reach useful content past it.
2. The auth-gate IS the target of review. The --url legitimately serves a login page that the diff is supposed to ship — the auth UI is the feature, and runtime reviewers SHOULD exercise the gate.

Caveat: --force-runtime skipping the probe means runtime lenses may produce NO-EVIDENCE verdicts if they encounter the actual auth gate. That's by design — the operator gets explicit signal that they overrode the safety net and the lenses couldn't see useful content, rather than a silent downgrade that hides the override decision from the audit trail.

Probe script

The probe is implemented at scripts/auth_gate_probe.sh (see scripts/README.md for the full contract). Invocation pattern, run before spawning runtime lenses when --reviewers runtime|full and NOT --force-runtime:

if RUNTIME_DOWNGRADE_REASON="$(bash scripts/auth_gate_probe.sh --url "$URL")"; then
  echo "Auth-gate detected: $RUNTIME_DOWNGRADE_REASON. Downgrading to code-only."
  # Set sidecar fields: invocation.runtime_downgraded=true, invocation.runtime_downgrade_reason=$RUNTIME_DOWNGRADE_REASON
fi

Exit code 0 = downgrade-with-reason on stdout (one of status_401, redirect_to_login:<url>, login_form_in_200_body, timeout); exit code 1 = no downgrade. The 4 trigger conditions and their semantics are documented above in ### Auth-gated runtime downgrade.

---

Tools

Three mechanical helpers live under scripts/ — extracted from the SKILL.md prose so their contracts are auditable in isolation and the SKILL.md context cost stays bounded. Full per-script contracts (usage, exit codes, dependencies, output schema) live in scripts/README.md; the canonical-prose cross-links below point at the SKILL.md section that owns the helper's semantics.

• scripts/auth_gate_probe.sh — Probes --url for auth-gate conditions (HTTP 401, 3xx redirect to /login|/signin|/auth|/sso, HTTP 200 with login-shaped HTML, or probe timeout) and signals whether to downgrade runtime lenses to code-only. Canonical prose: ### Auth-gated runtime downgrade (above). Invocation: bash scripts/auth_gate_probe.sh --url <url>; exit 0 + reason token on stdout = downgrade, exit 1 = proceed.
• scripts/aggregate.py — Multi-lens verdict aggregator: reads per-lens JSON verdicts, applies aggregator rules 1-7 (severity dominance, lens-owns-dimension, SKIPPED/FAILED/NO-EVIDENCE handling, persistent-disagreement via --prior-sidecar, absence-of-thing dedup), writes the audit-trail sidecar, prints the markdown summary. Canonical prose: ## Aggregation (above). Invocation: python scripts/aggregate.py --lens-dir <dir> --output-dir <dir> [--skill-version v3] [--prior-sidecar <path>].
• scripts/lint_prepass.sh — Optional deterministic linter pre-pass. Auto-detects file extensions in git diff --name-only HEAD (or in a caller-supplied --diff-paths <comma-or-space-separated> list), runs ruff + mypy on .py files and eslint on .ts/.tsx/.js/.jsx files when each tool is both installed AND configured (mypy: mypy.ini or pyproject.toml[tool.mypy] in an ancestor of cwd up to git root; eslint: .eslintrc* likewise), and writes lint-findings.json to --output-dir with a unified {tool, rule, file_line, message} finding shape plus tools_run / tools_skipped provenance. Missing linters and missing configs are graceful skips, not failures (the script exits 0 and findings: [] is valid). Full contract: scripts/README.md § lint_prepass.sh. Invocation: bash scripts/lint_prepass.sh --output-dir <dir> [--diff-paths <comma-or-space-separated>] — invoke from the project root (cwd), not from the script's directory.