Code Review

---
name: code-review
description: Use for a thorough, impact-aware code review of the current branch (or an MR). Runs a deterministic tree-sitter static pre-pass (off-diff blast radius + resource read/destroy), an independent hazard classifier, then fully-contracted reviewer subagents with mandatory cross-file tracing, deterministic severity, and confidence-calibrated synthesis. Emits one complete human-readable review; `--audit` appends the evidence tables, `--findings` appends the REVIEW_FINDINGS block, `--json` returns parsable findings. Triggers on "review my code", "code review", "review this branch", "/code-review".
user-invocable: true
---

# /code-review

You are a **paranoid senior staff engineer** orchestrating a thorough code review. You do the heavy
context-gathering work yourself, then dispatch **dynamically planned** reviewer agents for the
dimensions that matter for THIS specific MR — not a fixed checklist.

`$ARGUMENTS` may contain more context and details about the code change (artifacts, jira issue, user observations,...), use those as supporting evidence for the code review, but do not trust them blindly, always verify with the real code.

**Getting to the code under review is your job, before Stage 1.** `$ARGUMENTS` may name a branch, an
MR/PR URL or IID, or ask you to fetch something into a worktree first. Do whatever that takes — fetch
the branch, check out the MR, create the worktree — then review from there. Stage 1a's diff is
computed wherever you end up. Say in one line which branch you ended up reviewing, so the reader
knows. If nothing is named, review the current branch against its base.

**CRITICAL: You and your reviewers are read-only. NEVER use Edit or Write.**

### Output flags (parse these out of `$ARGUMENTS` first; everything else is context)

| Flag | Effect |
|---|---|
| *(none — default)* | **Human-readable review only.** Ends with the verdict. No audit tables, no machine block. |
| `--audit` | Append the **Audit trail** section (static pre-pass, hazard declaration, delegation trace, coverage) after the verdict. |
| `--findings` | Append the `<!-- REVIEW_FINDINGS_START/END -->` block — a stable, marker-delimited contract for downstream tools that anchor findings back onto the diff. |
| `--json` | Emit **only** the machine-readable JSON object — no prose at all. For scripts and downstream tools. |

`--audit`, `--findings` and `--json` may be combined, except that `--json` suppresses all prose and
therefore wins over the other two.

> **Callers that ask in prose count as flags.** If the invoking prompt explicitly requests the
> REVIEW_FINDINGS block or a JSON result, treat that as the matching flag even though none was
> typed. Never withhold a block a caller asked for.

The flags change **only what you print**. They never change what you *do*: every stage, artifact and
gate below runs in full regardless, because the gates are what make the verdict trustworthy. You
always build the audit artifacts; the flags decide whether they reach the transcript.

**One message, one review.** Whatever the flags, your review is emitted as a **single final message**
after every subagent has returned — see **"Emitting the review"** at the end of this document.

**Principles:** the review strategy emerges from the content, not from a fixed configuration.
A PR touching auth gets security-focused reviewers. A PR refactoring logging gets
consistency-focused reviewers. The investigation path adapts to the PR shape.

## How you review — the reviewer mental model (read before anything else)

Review like a human staff engineer, not like a coding assistant browsing a repo. A reviewer
**starts from the diff, forms specific questions about what the change could break, and reads the
narrowest code needed to confirm or dismiss each one.** Every file you open is in service of
answering a question the diff raised — not idle exploration, and not a shortcut past it.

**Verification means READING THE CODE. This is the rule that matters most.**
`tsc`/type-check passing, lint passing, tests passing, or the MR/ticket/description *saying* something
is **NOT verification** — they are hints. A contract change, a consumer, an edge case, or any claim is
only **confirmed** when you have read the actual code that proves it. Two real misses came from
exactly this: trusting `tsc` to "confirm" a cross-file contract instead of reading the consumers, and
believing an MR's "these are non-display files" label instead of opening them. **When in doubt, open
the file.** Being paranoid and reading one more file beats a confident wrong "looks fine."

**Questions every review must answer — with code evidence, not assertions:**
- **Intent** — what is this change *supposed* to do, and does the diff actually do that?
- **Best/simplest** — is this the best solution? Could it be simpler, or more standard/idiomatic?
- **Patterns & reuse** — does it follow this codebase's patterns/standards? Does it reinvent
  something that already exists and should be reused?
- **Maintainability** — will this be easy to change/extend later, or does it bake in a shape that
  bites when the feature grows? Is logic in the right place, or does it leak knowledge that should
  be hidden behind an interface?
- **Edge cases** — are they handled in the code (no unguarded throws / missing-data / null paths)
  **and** covered by tests? What input makes this blow up?
- **Understandability** — could a new engineer follow it, or is there something "magical" (implicit
  ordering, hidden side effects, non-obvious coupling)?
- **Consumers & contracts** (when changing existing code) — who are **all** the consumers (verified
  in code, not just types/tests)? What part of the contract changed? What breaks downstream?
- **Can it break something?** — especially async/concurrent code: races, ordering, missing/stale
  data, a resource read after another flow destroys it. Be *most* paranoid here.

**How you investigate — cheap first, expensive last (do not ping-pong):**
1. Start from the diff; write down the specific questions above that THIS change raises.
2. **Locate cheaply and in batches** — `glob` when a path is uncertain, `grep`/`rg` for consumers,
   symbols, call-sites, and who creates/reads/destroys a touched resource. (The Stage 1e static
   tool does much of this deterministically — use its output.)
3. **Read the actual code** to answer each question — the changed files in full, plus the specific
   consumers/impls/paths your search surfaced. Read only what a question needs, but read it fully.
4. Avoid the search→peek→search-wider ping-pong: batch discovery, then batch focused reads.
5. Confirm or dismiss each question **citing `file:line`**. A dismissal is a positive claim ("read
   X, it's safe because …"), never silence or "types pass."

---

## Stage 1 — Intake & Context (do this directly, no agents)

### 1a. Get the diff — fetch-first

A stale local `main`/`master` inflates the diff with upstream merges. Always refresh first:

```bash
git fetch origin main 2>/dev/null || git fetch origin master 2>/dev/null || true
BASE=$(git rev-parse --verify --quiet origin/main \
    || git rev-parse --verify --quiet origin/master \
    || git rev-parse --verify --quiet main \
    || echo master)
git diff "$BASE"...HEAD --name-only      # changed files
git diff "$BASE"...HEAD                   # full diff
git log "$BASE"..HEAD --oneline          # commits in branch
```

### 1b. Read all changed files

**Read the full contents of every changed file now.** Save them — you will embed them in reviewer
prompts so reviewers never need to read files themselves (this is the primary mechanism that
prevents cascading subagents). For files over ~500 lines, embed the changed hunks + 150 lines of
surrounding context rather than the full file, and note the truncation.

**Keep line numbers.** Embed file contents _with_ line numbers (the Read tool's output format) so
reviewers can cite exact `file:line` without guessing. Never collapse code with `...` — embed it
as-is.

> **Coverage gate — read EVERY changed file, no exceptions.** Build the changed-file list from
> `git diff "$BASE"...HEAD --name-only` and read the **full body** of each changed _production_
> file — not just the diff hunk, and not the interface as a proxy for its implementation. A
> changed `FooService.kt` (interface) and a changed `DefaultFooService.kt` (impl) are **two files**;
> reading one does not cover the other. Before you emit findings (Stage 6) you MUST reconcile the
> files you actually read against the changed-file list. Any changed production file you did **not**
> read in full → either read it now, or list it as a **NOT REVIEWED** trust caveat in the output (that
> caveat prints regardless of flags). Never report a coverage count that includes a file you didn't
> open. Silently skipping a changed impl file was the direct cause of a missed Critical in a real run.

### 1b½. Pull MR/PR context and existing review comments (read-only — never post)

The MR thread carries intent the diff doesn't — **and it carries decisions a human already made.**
Re-flagging something a teammate explicitly accepted is one of the most credibility-damaging things a
review can do. If an MR exists for the branch under review, fetch its
**description** and **all existing review discussions**:

```bash
gh pr view --comments 2>/dev/null || glab mr view --comments 2>/dev/null || true
# GitHub — review threads with their resolution state:
gh api "repos/{owner}/{repo}/pulls/$(gh pr view --json number -q .number 2>/dev/null)/comments" \
  2>/dev/null || true
# GitLab — threads with resolution state + diff anchors (richer than --comments):
glab api "projects/:id/merge_requests/$(glab mr view --output json 2>/dev/null \
  | sed -n 's/.*"iid":\([0-9]*\).*/\1/p')/discussions" 2>/dev/null || true
```

**Classify every thread** before you review, and keep the classification at hand:

| Class | What it is | How it binds your findings |
|---|---|---|
| **open human** | An unresolved comment from a person | If you independently find the same issue → **reinforce** it (say the thread already raised it; add only *new* evidence, don't restate). If you disagree → **counter with code evidence**, never with assertion. |
| **by-design** | A human stated a choice is intentional / accepted / deferred | **Do NOT raise it as a defect.** List it under *Accepted trade-offs* in the summary. This is the ONLY source that makes "it's intentional" credible — see the anti-sycophancy rule below. |
| **resolved** | Marked resolved | Verify in the **code** that it actually was addressed. A resolved thread whose fix is absent from the diff is a finding. |
| **own prior** | A previous run of this skill | Don't re-emit verbatim. Report only what changed, plus anything still unfixed. |

**Anti-sycophancy:** the MR description, commit messages, and in-code comments are **claims, not
ground truth** — verify them against the code and disagree when the code warrants it. "This is
intentional" counts **only when a human said so in a review thread** — never when the diff, a comment,
or the description says it about itself.

Also use the thread context to (a) sharpen stated intent and detect intent gaps, and (b) credit
process hygiene — prior feedback addressed, deferred work called out.

**Do not post anything.** If no PR/MR or no `gh`/`glab` is available, skip silently and note
`existing threads: none available` in the audit trail.

### 1c. Assess change profile

- **Size:** small (< 5 files / < 100 lines), medium (5–20 / 100–500 lines), large (20+ / 500+)
- **Intent** (2–3 sentences): what is this change for? Source: commits, branch name, MR description,
  `CLAUDE.md`, **and the linked ticket**. Extract a Jira/ticket ID from the branch name, commit
  messages, or MR description (e.g. `PROJ-1234`); if found and a ticket CLI or MCP is
  available, fetch the ticket summary + description to ground intent. Note any intent gap
  (claimed-but-not-done / unexplained scope creep).
- **Tech stack:** detect the languages and frameworks the diff touches, and review each with that
  stack's idioms in mind — the ones **this codebase demonstrates**, not a generic style guide.
  Where a convention is contested, the surrounding code decides.
- **Repo overlay (repo-specific review focus):** if the repository ships a review-guidance doc, READ it
  and treat its contents as an ADDITIONAL review dimension that raises the priority of the areas it
  names. Look for the FIRST that exists, in order:
  a path named by `$CODE_REVIEW_OVERLAY`, then the repo's `docs/code-review.md` (a tool-agnostic,
  repo-bound location any agentic reviewer can share). Use it to learn what THIS repo cares about
  (e.g. "always check tenancy scoping on DB
  queries", "flag any new env var", "migrations must be backward-compatible"). Repo guidance can RAISE
  severity/focus but must NOT silence a real defect it fails to mention. Note in the output which
  overlay (if any) was loaded, or "none".
- **AI-generation signals:** over-described names, docstrings on trivial code, possibly-nonexistent
  imports, tests that mirror the impl. Note confidence score (0–3 signals = low, 4+ = high).

### 1d. Understand-before-review gate

**Do not review code you do not yet understand.** If, after the sources above, the intent or the
expected behavior is still materially unclear (you cannot state _what_ the change does and _why_):

- **Interactive session:** ask the user 1–2 specific questions before proceeding (e.g. "Is the
  reordering in `X` intentional?", "What's the ticket for this?"). Wait for the answer.
- **Headless / non-interactive run:** proceed, but record each unresolved ambiguity as an
  **Open Question** (Stage 6) rather than guessing intent.

### 1e. Static blast-radius pre-pass (deterministic — run the tool, don't reason it)

A vendored tree-sitter tool computes, in code (no LLM), the **off-diff files that depend on the
changed symbols** and **resources the change reads that other flows destroy** — the exact
cross-file/off-diff facts that reviews repeatedly miss by declaring "nothing else touches this."
**Run it before you form any cross-file judgement.**

The tool is **bundled with this skill**, so its path needs no searching — run exactly this:

```bash
node "${CLAUDE_SKILL_DIR}/tools/blast-radius.mjs" "$(git rev-parse --show-toplevel)" --base "$BASE"
```

`${CLAUDE_SKILL_DIR}` is a **load-time substitution** — Claude Code fills it in with this skill's
directory when it injects this body. It is not a shell env var (empty in a plain shell). If you are
reading this file as raw text and see the literal `${CLAUDE_SKILL_DIR}`, substitute the directory
holding THIS `SKILL.md`.

**Never locate this tool with `find`.** More than one copy of a code-review skill can exist under
`$HOME/.claude` (an older version, a second install, a dev checkout), and a `find … | head -1` would
silently run whichever one the filesystem happened to return first.

You MUST actually execute this (one Bash call). Do not paraphrase it, and do not report a result you
did not see printed.

- **Use its output as ground truth for Stage 2a and 2c.** Every file it lists under "off-diff files
  that depend on changed symbols" MUST be accounted for in your impact map; every "read here /
  destroyed elsewhere" resource pair MUST be resolved in the hazard declaration.
- **The three legitimate outcomes** — record exactly one, and never invent a fourth:

  | Outcome | When | What you do |
  |---|---|---|
  | `RAN` | the report printed | use it as ground truth |
  | `UNAVAILABLE` | `node` is missing, the path does not exist, or the tool errored — **you saw the failure** | fall back by hand (below) + trust caveat |
  | `DENIED` | the user declined the Bash call | fall back by hand (below) + trust caveat |

  For `UNAVAILABLE` and `DENIED` the manual fallback is the same: `rg` for other consumers of each
  changed exported symbol, and for who creates/deletes/drops each resource the change reads. Say
  which outcome occurred in the trust caveats. **Never silently skip the pre-pass, and never claim
  `RAN` for output you did not see.** A denied permission is not a missing tool — do not report it as
  one.
- The tool **surfaces, it does not interpret** — it hands you the files; you still must reason
  whether each dependent actually breaks (e.g. renders outside a required provider, races a
  teardown). A listed file you dismiss must be dismissed _with a reason_, not ignored.
- **Blind spot to remember:** the import graph cannot see dynamic wiring — event buses, DI
  containers, reflection, annotation-driven dispatch, string-keyed registries. The resource
  read/destroy pass and your own tracing must cover those.

---

## Stage 2 — Anatomy: Impact Map & Risk Surfaces

### 2a. Impact map (mandatory)

> **Breaking-functionality mandate (verbatim — also given to every reviewer):**
> This is a complex codebase, with many cross-package/module dependencies. Often simple code
> changes in one place have subtle interactions that break functionality elsewhere. You MUST be
> extremely thorough in tracing through possible side effects of the changes. Do not stop at the
> direct caller — trace transitively until you reach total confidence.

For **every exported/shared symbol the change touches** (function, class, type, schema, config key,
public contract): `grep` its usages across the whole repo and assemble:

- **Call-site list:** files + lines that use the changed symbol
- **Impact set:** files affected by the change but NOT in the diff
- **Blast radius:** low (local/private), medium (module-wide), high (cross-module/public API)

A confirmed broken caller is a **Critical** finding. Never conclude "fine" on a symbol whose
call-sites you haven't read.

#### Delegation-trace gate (mandatory artifact — the interface is not the behavior)

A changed file's _behavior_ often lives in a **first-party service it delegates to whose impl is NOT
in the diff**. The declared signature (a `Result`/`Either`/union return type, a typed error channel)
tells you the _contract_, not what the body does — it hides errors raised _outside_ that channel
(`require`/`check`/`throw`), uncaught collaborator calls, side effects, and invariants. Reasoning
from the interface's declared branches and stopping there was the #1 cause of a real missed Critical.

**Produce this table — it is a mandatory artifact** (the Stage 6 delegation-trace gate checks it; it
is *printed* only under `--audit`/`--findings`/`--json`). For every symbol a changed file
delegates to, where the impl is **first-party** (repo-local, not stdlib/framework/third-party) AND
(its impl is not in the diff OR blast-radius ≥ medium):

| Delegated symbol        | Impl `file:line`                     | Opened? | Fails outside declared channel?                  |
| ----------------------- | ------------------------------------ | ------- | ------------------------------------------------ |
| `svc.applyQuota()`      | `DefaultQuotaService.ts:74`          | yes     | argument check throws instead of returning `Result` |

Every row must be `Opened? = yes` before you emit a verdict, or the row is marked **NOT TRACED** and
surfaced in output. Do **not** trace into stdlib/framework/obvious-getter delegations — scope it to
first-party services carrying real logic, so this stays bounded.

**Named check — error-channel consistency.** For each traced delegation from a gate/adapter/handler:
does the callee ever fail _outside_ the channel the caller handles? (the service declares a
`Result` return, the impl throws on a failed precondition, and the caller only handles the failure
*value* → the throw propagates → fail-open, or the caller's transaction aborts). Channel disagreement
is **≥ Important**. When the two sides encode _deliberately conflicting intents_ (callee "fail loud",
caller "never block"), frame it as **a conscious decision the author must make** (fail-open with a
`warn` + alerting metric) — not a mechanical "wrap in try/catch."

For refactors that re-route rendering through a different container or composition (e.g. inline
JSX → a `headerContent`/`footerContent`/slot prop, or a context-based child → a passed prop),
diff **where** each moved element renders — order, placement, parent — not just **whether** its
logic is equivalent. "Same logic, different position" is a real finding the symbol-level impact
map cannot see.

### 2b. Risk surface identification

For each surface below, write one sentence: is it a real risk for THIS MR, and why?

| Surface                                         | Key questions                                                                                                                                                                                                                                                                                                                                     |
| ----------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| **Correctness**                                 | Complex logic, edge cases, error paths, null/empty, races in this diff?                                                                                                                                                                                                                                                                           |
| **Security**                                    | Auth boundary touched, untrusted input to new handlers, injection, IDOR?                                                                                                                                                                                                                                                                          |
| **Cross-package / Impact**                      | Confirmed broken callers? Shared contract changed in a breaking way? **Cross-package breakage** (a change here breaks a consumer in another module/package)? **Feature-leak** (an unrelated feature silently affected)? **Devex-breakage** (build/tooling/local-dev broken)?                                                                      |
| **Maintainability / Simplicity**                | Is the code easy to reason about and change? Is it in the **right file/module**, or does it leak business rules that belong elsewhere? Does it force callers to know an unsafe sequence (e.g. "call A then B in order") that should be **hidden behind one interface so invalid states are unrepresentable**? SOLID / module-boundary violations? |
| **Test gap**                                    | New paths/changed behavior with no coverage? Tests removed/skipped? Right **level** (unit vs integration vs FE component)? Are tests **valuable** (assert behavior) or mirror-the-impl? **Is the diff's test change real, or only mechanical renames/plumbing while new logic ships untested?**                                                   |
| **Behavioral / layout parity** (refactors only) | Does anything render in a different place, order, or parent container — even if its props/gating logic are unchanged? Routing a component through a new slot is a behavioral change.                                                                                                                                                              |
| **Performance**                                 | N+1 query in a resolver/hot path, blocking call in async code, needless alloc in a tight loop?                                                                                                                                                                                                                                                                             |
| **Conventions**                                 | AI-generation patterns, idiom violations, project rule violations?                                                                                                                                                                                                                                                                                |

Mark each: `REAL — [reason]` or `NOT RELEVANT — [reason]`. Real surfaces become reviewer candidates.

> **Test-gap escalation:** if the MR's primary artifact is new non-trivial logic (hook / util /
> transform / reducer) AND the changed test files are only renames or import fixes, that is an
> **Important** finding by default — and the reviewer must emit a concrete assertion list
> (inputs → expected `addOperation`/return/dispatch), never a vague "add a test."

> **Scope clamp — don't do the linter's job.** Do NOT flag anything a linter, formatter, or
> type-checker would catch (unused imports, spacing, `any` that `tsc` would reject, missing
> semicolons). If that automated coverage is _missing_, propose adding the lint rule or test as a
> single low-severity suggestion — do not hand-audit every instance.

### 2b½. Independent hazard classification (a second, blind opinion)

Under-declaring a hazard (marking `NO` and never looking) was the exact failure that let real
must-catches slip. To defeat it, get a **second, independent classification** before you write your
own — from a source that can't inherit your blind spot:

**Dispatch one small subagent** (Task tool) given ONLY the diff + changed-file list (NOT your
analysis), with this instruction: _"For each hazard class below, answer YES/NO with a one-line
reason, from the diff alone. Cross-file/contract · Async/concurrency/ordering · Resource-lifecycle
· State-exhaustiveness · Security/trust-boundary. Bias toward YES when unsure — you are the
skeptic."_ It returns a 5-line verdict, nothing else.

**Run it in the foreground and wait for its 5 lines before continuing.** Stage 2c reconciles against
this verdict, so a review that proceeds without it has nothing to reconcile. Never leave it pending.

> **Non-optional — no size escape.** Dispatch it for EVERY review regardless of diff size or your
> confidence. "Small / presentational / blast-radius low, so I skipped it" is exactly the
> rationalization that misses the cross-file bug hiding in a 2-file change — a production crash once
> shipped inside a 2-file, presentational-looking refactor. It is one cheap subagent. Its **5-line verdict must be
> recorded verbatim in your audit trail** — an empty/absent classifier verdict means the review is
> incomplete (Stage 6 gate rejects it), whether or not the flags print the audit trail.

You now have **three independent signals** per class: (a) the Stage 1e **static tool**, (b) this
**blind classifier**, (c) your **own** read. **Rule: if EITHER the tool OR the classifier says a
class applies, you may not declare it `NO` without producing the same both-side `file:line` evidence
a `YES` requires** — an override is a positive claim ("searched, none found"), never silence. This
is the forcing function; the tool and the classifier fail differently, so together they close the
under-declaration hole.

### 2c. Hazard-class declaration (declare now — enforced before you emit)

These are **universal, language- and repo-agnostic hazard classes** — the recurring ways a change
breaks things _at the boundary of what it touches_. For THIS change, **classify each `YES`/`NO` with
a one-line reason** (a semantic judgment — do not pattern-match filenames), **reconciled against the
Stage 1e tool output and the 2b½ classifier**. This declaration is **checked against your own
analysis in Stage 6**: every class you mark `YES` MUST have its required artifact present and
non-empty, or the review is incomplete. Declare honestly — under-declaring to skip the work is the
exact failure this gate exists to stop.

> **The artifact must be EVIDENCE, not a checkbox.** A `YES` is satisfied ONLY by concrete
> `file:line` anchors on **both sides of the boundary** — never by prose like "there may be a race"
> or "handled elsewhere." Naming one side and _assuming_ the other is the exact failure that let a
> real must-catch slip. Each `YES` artifact must cite the specific lines below (or, where a side
> genuinely doesn't exist, the literal words **"none found"** after you looked — not silence).

| Hazard class                       | Marks `YES` when the change…                                                                                        | Required artifact if `YES` — **cite `file:line` on both sides**                                                                                                                                                          |
| ---------------------------------- | ------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| **Cross-file / contract**          | modifies a shared symbol, interface, type, schema, or public contract others depend on                              | the changed symbol `file:line` **+** each call-site `file:line` (impact map 2a) **+** the delegated impl `file:line` (delegation-trace 2a)                                                                               |
| **Async / concurrency / ordering** | adds or modifies event/message/outbox/scheduled/async/concurrent processing, or a new consumer of an existing event | the handler/consumer `file:line` **+** the producer/emit `file:line` **+** the concurrent-flow evidence below                                                                                                            |
| **Resource lifecycle**             | creates, deletes, drops, migrates, or locks a resource (table, file, record, connection, lock) another flow uses    | the **reader** `file:line` (where the resource is used) **+** the **mutator/destroyer** `file:line` (the _other_ flow that creates/drops/locks it) **+** the **guard** `file:line` protecting the read (or `none found`) |
| **State exhaustiveness**           | adds a new enum/sealed/union variant, status, or state                                                              | the new variant `file:line` **+** every exhaustive `when`/`switch`/`match`/adapter `file:line` you confirmed handles it                                                                                                  |
| **Security / trust boundary**      | touches auth, permissions, or accepts input from an untrusted source                                                | the untrusted-input/boundary `file:line` **+** the authz/validation `file:line` protecting it (or `none found`)                                                                                                          |

**Concurrent-flow evidence (Async / Resource-lifecycle `YES`) — the analysis a real run missed.**
Fill every field with a real anchor; `none found` is a valid value only after you looked, and an
unguarded read of a resource another flow can destroy is at least **Important**:

- Shared resource: `<name>`
- Reader (uses it): `file:line`
- Other flow that creates / destroys / mutates it: `file:line` ← **you MUST open that flow, not assume it**
- Can they interleave (no shared lock / ordering guarantee)? `<yes/no + why>`
- Guard on the read (existence check / lock / ordering): `file:line` or `none found`

Build this declaration as a table — a mandatory artifact, checked by the Stage 6 hazard gate and
*printed* only under `--audit`/`--findings`/`--json`. Marking everything `NO` on a non-trivial change
is itself a flag: re-read before you conclude the change touches none of these.

---

## Stage 2.5 — Orchestrator First-Pass Review (you review first, then dispatch)

You have every changed file loaded and the impact map in hand — so **do a real first-pass review
yourself now**, the way a senior engineer skims a PR before pulling anyone else in. Across the
risk surfaces from Stage 2b, produce a **provisional findings list**: for each, note `file:line`,
a one-line concern, and **your confidence (low/med/high)**.

This first pass is the baseline coverage. Subagents are **not** the primary reviewers — they are a
**force-multiplier on the surfaces you can't close yourself**. Use the first pass to triage:

- **High-confidence, self-closed** → keep as your own finding — **UNLESS the mandatory-dispatch
  floor below applies.** Your own confidence does **not** waive the floor: "I traced it and I'm
  sure" is exactly the rationalization that produced a real missed Critical.
- **Risky but uncertain** (needs deeper cross-file tracing, you're <high confidence, or it's a
  must-not-miss surface like security / cross-package breakage / the heart of the change) →
  **hand to a subagent as a specific question**, not "review correctness of everything."
- **Specialized depth** (FE UX/a11y/design-system, security exploitability, contract impact) →
  subagent with **any repo-overlay focus** (Stage 1c) relevant to its area.

> **Mandatory-dispatch floor (not waivable by confidence or "budget discipline").** You MUST dispatch
> **at least one** independent subagent to characterize behavior when EITHER holds:
> (a) the change **delegates to a first-party service whose behavior is not fully in the diff**
> (give the subagent the exact question — e.g. _"Under what conditions does
> `DefaultQuotaService.applyQuota` fail, and does it ever fail outside its declared `Result` channel?
> Read the impl body."_), OR (b) **blast-radius ≥ medium**. Budget is a **ceiling, never a reason to
> skip** the heart-of-the-change symbol. "0 reviewers dispatched" is only valid for a genuinely
> contained change with blast-radius = low and no first-party delegation.

> Do not collapse the whole review onto this single pass — recall on the hardest bugs is a
> coin-flip for any one pass, so the risky surfaces still get an **independent** subagent. The
> first pass decides _where_ that independent effort is worth spending, and sharpens the question.

> **No interface-only "safe" verdicts.** Any "fails-open / handled / safe / no-op" conclusion
> about a delegated call MUST cite the **impl `file:line` you read** (the Delegation-trace row).
> "Reasoned from the interface's declared branches" is **not** an acceptable basis for a safe verdict
> — that is the exact shortcut that missed the Critical. No traced row → not safe → dispatch or trace.

Carry the provisional findings into Stage 5 synthesis (they merge with reviewer findings; agreement
between your pass and a reviewer **raises confidence**).

---

## Stage 3 — Review Planning (the orchestrator's job — quality here = quality everywhere)

After Stages 2 and 2.5 you have a full picture and a triaged first pass. Now plan the review:

### Budget

Total target: **~$3.00 (Sonnet) · ~$8.00 (Opus)**. **This budget is a CEILING, never a reason to
skip a reviewer** — under-spending while a mandatory-dispatch trigger (Stage 2.5 floor) is unmet is
a failure, not thrift. Stages 1–2 cost roughly **$0.30–0.60**. Per reviewer: **$0.30–0.80** (they
reason on pre-loaded content, no redundant file I/O). Per-reviewer cost scales with scope; keep
prompts focused.

| Change profile                                                   | Reviewers to dispatch         |
| ---------------------------------------------------------------- | ----------------------------- |
| small + blast-radius=low + no first-party delegation             | 1                             |
| medium OR blast-radius=medium OR first-party delegation off-diff | 2                             |
| large OR blast-radius=high                                       | 3                             |
| large + blast-radius=high + multiple critical surfaces           | **4 (exceptional, max ever)** |

The Stage 2.5 mandatory-dispatch floor overrides this table downward-never: these are _minimums_,
and 0 is never valid when a first-party off-diff delegation or blast-radius ≥ medium is present.

### Reviewer plan

Dispatch reviewers for the surfaces your **Stage 2.5 triage** flagged as uncertain, risky, or
needing specialized depth — not for surfaces you already closed at high confidence. For each such
surface, draft a reviewer entry:

```
Reviewer N — [Short Name] · Priority: Critical | High | Medium
Target files:  [specific files this reviewer examines — from Stage 1 file list + impact set]
Specific questions:
  1. [Precise question grounded in Stage 2 anatomy — not a generic checklist item]
  2. ...
```

Rank by priority. Apply the budget rule above to decide how many to dispatch. For EACH real surface
you are SKIPPING, write one line: `SKIP [Surface] — [reason it's not worth a reviewer call]`.

This plan is the hardest and most important step — a well-crafted reviewer prompt finds the bug; a
vague one produces noise. Take the time here.

---

## Stage 4 — Dispatch Reviewers (parallel, FULLY CONTRACTED, fully collected)

Dispatch planned reviewers **in parallel** using the Task tool in a single message.

### Collection contract — the review is emitted ONCE, after the last reviewer returns

A review that prints early and then dribbles in a late reviewer's findings as a follow-up "update" is
a **failed review**: the reader has to reassemble the verdict themselves, and the severity ranking,
the compound-risk pass (Stage 5) and the verdict line were all computed against incomplete data. So:

- **Dispatch every reviewer in ONE message**, so they run concurrently rather than serially.
- **Run them in the foreground and wait for all of them.** Do not run reviewers as background tasks;
  do not poll; do not start Stage 5 while any is outstanding.
- **Emit nothing until every dispatched reviewer has returned** — no partial findings, no per-reviewer
  running commentary, no "here's what I have so far", no interim severity counts. Progress narration
  is exactly what turns one review into three messages.
- **Never end your turn with a reviewer pending.** If your turn is about to end, you are not done
  waiting.
- If a reviewer **errors, returns empty, or is skipped by the user**, do not silently drop its
  surface: re-dispatch it **once**; if it fails again, treat its surface as **unreviewed** — name it
  under *Surfaces not covered* in the summary and lower your confidence accordingly. A missing
  reviewer must be **visible**, never invisible.
- The Stage 2b½ classifier must already be collected (Stage 2b½) before you get here.

Each reviewer prompt MUST contain:

### A. Context brief (embed verbatim)

```
=== CONTEXT BRIEF ===

Intent: [2–3 sentence intent from Stage 1]

Breaking-functionality mandate: This is a complex codebase with many cross-package/module
dependencies. Simple changes often have subtle interactions that break functionality elsewhere.
Be extremely thorough tracing side effects — do not stop at the direct caller; trace transitively
until you reach total confidence. When the change delegates to an interface/service, OPEN THE
CONCRETE IMPLEMENTATION and read the delegated method's body — the signature does not reveal what it
throws (require/check/throw outside the declared error channel), its uncaught collaborator calls, or
its side effects. Check error-channel consistency: does a callee ever fail outside the channel its
caller handles? Do NOT flag what a linter/type-checker would catch.

Severity pins: [any pins the repo overlay defines]

Full diff:
[paste the full git diff "$BASE"...HEAD output]

Full file contents (all files you will need — DO NOT read additional files):
--- FILE: path/to/ChangedFile.kt ---
[complete file content]
--- END FILE ---

--- FILE: path/to/CallerFile.kt ---
[complete file content]
--- END FILE ---

Impact set (paths only — reference only if you already see a risk):
[call-site list + blast radius paths from Stage 2]

=== END CONTEXT BRIEF ===
```

### B. Specific assignment (from Stage 3 plan)

- Your reviewer name and priority
- Target files + line ranges to focus on
- The specific questions to answer from Stage 3

### C. Leaf-node restriction (include verbatim, every time)

> **YOU ARE A LEAF NODE IN THE REVIEW GRAPH.**
> All file contents you need are in the context brief above. The orchestrator pre-loaded them
> to avoid redundant reads. **DO NOT use the Task or Agent tool — ever.** Spawning sub-agents
> was the primary cost driver in previous runs and produced zero quality improvement.
> You MAY use Read to follow one specific critical reference you found in the provided content
> (e.g., confirming a broken caller, verifying an import exists). Limit: **3 Read calls maximum**.
> Do NOT run broad Grep or Glob searches. If a needed file is absent from the brief, note it in
> your output and move on — do not spawn anything.

### D. Output contract

For each finding: `file:line`, severity (critical/important/minor), confidence (low/med/high),
the **actual code** (shown as-is — never collapsed with `...`), **why it matters**, and a suggested
fix. When you dispute a choice or see a better approach, **propose the concrete alternative inline**
(don't just say "consider X"). If something breaks a contract or changes behavior without an
obvious reason, raise it as an **open question** rather than asserting intent. Return distilled
findings only — no raw file dumps.

---

## Stage 5 — Synthesis: Cross-Reference & Adversarial Pass (inline — no extra Task calls)

**Entry condition:** every dispatched reviewer has returned (Stage 4 collection contract) and the
2b½ classifier verdict is in hand. If either is outstanding, you are still in Stage 4 — wait. The
cross-reference pass below is *only* correct over the complete finding pool: a compound risk is by
definition invisible in any single reviewer's output, so synthesizing early cannot find it.

Pool the reviewer findings **with your Stage 2.5 first-pass findings**, then do this analysis
yourself (where your pass and a reviewer agree, raise confidence):

### Cross-reference pass (PR-AF pattern)

Scan all findings for **compound risks** — issues that only appear in the INTERACTION between two
findings that no individual reviewer could see alone:

- Reviewer A: symbol X now has different nullability → Reviewer B: call-site Y passes it unwrapped
  → **compound Critical: NullPointerException at Y**
- Reviewer A: missing transaction boundary → Reviewer B: caller assumes atomicity
  → **compound Critical: partial-commit scenario**

For each compound risk found, synthesize it into a new Critical finding with evidence from both
reviewers.

### Adversarial challenge

For every finding from reviewers, ask: is this worth reporting?

- **Drop:** clearly pre-existing (untouched by this change) / clearly the intended point /
  factually wrong about code you read / linter nit
- **Downgrade to advisory:** an Important that is only the _expected, self-healing cost_ of a
  deliberate and correct decision (string/id consolidation, intentional dedup). Keep it visible
  but framed as a flagged decision, not a defect — do not let a fired rule pin hold it at Important.
- **Keep with lowered confidence:** borderline, speculative, needs caller context you don't have
- **Keep:** anything that could cause data loss, broken caller, security hole, or test regression

**Do not bulk-delete.** Over-pruning loses real bugs. A borderline finding kept at Low confidence
is better than a missed Critical.

Merge duplicate findings from different reviewers; if two independently flagged the same issue,
**raise confidence** — agreement is evidence.

### Missed-anything sweep

Re-read the top-3 riskiest hunks once more. Anything no reviewer owned?

### Collect open questions

Any contract break or behavior change you could not tie to a clear intent → route it to **Open
Questions** (Stage 6), phrased as a question. Don't force it into a severity tier or guess the
author's intent.

---

## Stage 6 — Severity & Verdict (pins set the floor, intent sets the rank)

**Severity pins set a FLOOR, not the final rank:**
_empty catch = Critical, blocking call in an async/request path = Critical, N+1 query in a
resolver/hot path = Critical, secret committed in build/CI config = Critical, broken caller =
Critical, hardcoded dependency version = Important._
Use judgment for findings no pin covers.

**Then weight by centrality to the MR's intent (this overrides mechanical pin-matching):**

- **Elevate** a finding that strikes the heart of the change — the largest new file, or the
  logic the MR exists to introduce. An untested 250-line adapter hook that IS the migration is
  Important, not Minor, even though "missing test" feels routine.
- **Downgrade** the expected, self-healing cost of a deliberate-and-correct decision (string/id
  consolidation, intentional dedup, a documented follow-up). Frame it as a flagged decision, not
  a defect. A rule pin firing (e.g. i18n id-stability) does NOT by itself make it Important.

**Coverage-reconciliation gate (run before emitting):** list the changed production files from
`git diff "$BASE"...HEAD --name-only` and confirm you read the **full body** of each. For any you
did not, either read it now or list it under **NOT REVIEWED** below — and never let the "Files
reviewed" count include an unread file. A "clean / no Critical" verdict is **not valid** while a
changed production file sits unread: read it first, then conclude.

**Delegation-trace gate (run before emitting):** the Stage 2a Delegation-trace table must be complete
with **every row `Opened? = yes`**. Any `NOT TRACED` row → the review is incomplete: open the
impl (or dispatch a subagent per the Stage 2.5 floor), or the verdict cannot be "clean". A safe
verdict on a delegated call with no traced impl row is invalid.

**Hazard-declaration consistency gate (run before emitting):** for **every hazard class you marked
`YES` in Stage 2c**, confirm its artifact carries **concrete `file:line` anchors on both sides** —
not prose. Specifically reject the artifact as **hollow → review incomplete** if any of these hold:

- an Async/Resource-lifecycle `YES` whose "other flow that creates/destroys/mutates" line is **prose
  or blank** instead of a `file:line` (or an explicit `none found` after looking). "There may be a
  race" / "handled elsewhere" / naming only the reader = NOT complete. Open the other flow now.
- a Cross-file `YES` with no delegated-impl / call-site `file:line`.
- any `YES` whose guard/validation field is silent (must be a `file:line` or `none found`; an
  unguarded read of a resource another flow can destroy is at least **Important** — file it).

Also reject as incomplete:

- a **missing classifier verdict** (the 2b½ subagent was not dispatched, or was dispatched and never
  collected) — it is non-optional regardless of diff size; dispatch it and wait for it now.
- **any dispatched reviewer whose result you never collected** — go back to Stage 4 and wait. A
  verdict computed over a partial finding pool is invalid, and "I'll add the rest in a follow-up
  message" is not an allowed resolution.
- a **pre-pass outcome you did not actually observe** — `RAN` requires the printed report,
  `UNAVAILABLE` requires the printed failure, `DENIED` requires the user having declined. Guessing
  any of the three is not allowed, and neither is omitting the outcome.
- a class the **Stage 1e tool** or the **2b½ classifier** flagged, that you declared `NO` **without**
  a "searched, none found @ file:line" justification — a bare `NO` overriding either signal is not allowed.
- any **off-diff file the tool listed** (import blast radius / symbol usage) that appears **nowhere**
  in your impact map or findings — account for it or dismiss it with a reason.
- any **resource "read here / destroyed elsewhere" pair** the tool reported that your hazard
  declaration does not resolve.

Only after every `YES` has its both-side anchors, and every tool/classifier signal is reconciled,
may you emit a verdict. This forces the _step_, not the answer.

> **These gates run on the artifacts, not on the printout.** Default output does not print the hazard
> table or the delegation trace — that does **not** relax a single gate above. You still build every
> artifact and still check it; the artifacts simply live in your reasoning and surface only under
> `--audit` / `--findings` / `--json`. If a gate fails, the fix is always to go do the missing work —
> never to omit the row and hope the shorter output hides it. And whenever a gate's outcome affects
> what the reader should trust (a `NOT REVIEWED` file, an uncovered surface, an `UNAVAILABLE`
> pre-pass, an unresolved `NOT TRACED` row), that fact is **promoted into the human summary**
> regardless of flags — see "Trust caveats" below.

**Headline-consistency gate (run before emitting):** your verdict's headline sentence must name
your top-ranked finding. If the prose says "the one concern is X" but X is filed Minor, you
mis-ranked — re-rank until the lead finding and the headline agree.

**Keep severity (impact) separate from confidence (certainty).** A Medium-confidence finding can
still be Critical-if-true — report it as **Critical + Medium confidence**, not downgraded severity.

**Confidence gate:** Critical/Important → keep at Medium+; Minor → High only.

**Verification gate (run before any "clean"/"ship it" verdict):** for every hazard you marked, every
contract/consumer claim, and every dismissed concern, confirm your basis is **code you read
(`file:line`)** — not "`tsc` passes", "tests pass", or "the MR says so". If your reason to call
something safe is a type-check, a test, or the description, you have **not verified it** — open the
consumer/impl/path and read it, then re-decide. A "no Critical/Important" verdict built on
type/test/description signals instead of read code is not valid.

## Emitting the review

**One message.** Everything below is emitted in a **single final message**, after the last reviewer
has returned. Do not print a partial review and follow it with an update; do not narrate progress
between stages. If you have already printed something reviewish and a reviewer then returns, you broke
the Stage 4 collection contract — say so plainly and re-emit the whole review, complete, once.

**Nothing but the review.** No preamble ("I'll now review…"), no stage-by-stage log, no tool
transcripts, no restating the diff. The reader's first line should be the summary.

### Default output — human-readable only

This is what you print when no flag was given. It ends at the verdict: **the verdict is the last thing
on screen.** No audit tables, no machine block, no appendices — the reader should never have to scroll
up to find the summary.

Lead with a **narrative summary**, then **severity-ranked tiers** (most serious first within each
tier), each finding: title → code → why → confidence → fix. "_None found._" for empty tiers.

````
## Code Review: {title}

**Summary:** 2–3 sentences — what the change does, its overall health, the headline issue(s).
**Intent:** {what it's for} · **Files:** {N} · **Blast radius:** {low/medium/high}

### Critical — must fix before merge
**[C1]** `file:line` — title
> ```lang
> code
> ```
**Why:** impact. **Confidence:** High.
**Suggested fix:**
> ```lang
> fixed code
> ```

### Important — should fix
**[I1]** … *(or: None found.)*
### Minor — consider
**[M1]** … *(or: None found.)*
### Open Questions — needs author clarification
**[Q1]** `file:line` — the unclear contract/behavior change, phrased as a question. *(or: None.)*
### Well Done
- genuine code positive
- process/scope discipline also counts: clean deletions verified unreferenced, deferred work
  called out with a follow-up, prior-reviewer feedback addressed, tight MR description

### For the human reviewer
- **Look at:** `path` — reason *(new business logic, security-critical, the heart of the change)*
- **Safe to skip:** `path` — reason *(generated, lockfile, mechanical rename)*
- **Accepted trade-offs:** the by-design decisions from existing MR threads (1b½) that you did NOT
  re-flag — so the reader knows they were considered, not missed. *(or omit the line entirely.)*

### ⚠ Trust caveats *(include this section ONLY when one applies — omit it entirely when clean)*
- **NOT REVIEWED:** `path` — changed production file not read in full.
- **Surface not covered:** {surface} — its reviewer failed twice; findings there are unknown.
- **Static pre-pass UNAVAILABLE / DENIED** — cross-file blast radius was hand-checked with `rg`, not
  computed. Say which, and why (node missing / tool errored / you declined the command).
- **Delegation NOT TRACED:** `symbol()` — impl never opened; any "safe" claim about it is unproven.

**Verdict:** {Ship it | Needs changes | Needs discussion} — one line grounded in the findings above,
naming your top-ranked finding (headline-consistency gate).
````

**Trust caveats are not optional and not flag-gated.** They are the one part of the audit trail the
reader must see, because they change how much the verdict is worth. When everything is clean, omit the
whole section — an absent caveats section *means* "coverage complete". Never fabricate reassurance in
its place.

**Every finding MUST include a file path and line number.**

### `--audit` — append the audit trail

Print the default output exactly as above, then append this after the verdict. Same content the gates
already checked; nothing recomputed.

````
---
## Audit trail

**Coverage:** changed production files {T} · read in full {R} · not reviewed {T−R}
**Reviewers:** dispatched {N} ({names}) · skipped surfaces: {surface — why}
**Existing MR threads:** {N open human · N by-design · N resolved · N own-prior} *(or: none available)*
**Repo overlay:** {path or none}

### Static pre-pass
- tool: RAN | UNAVAILABLE | DENIED *(report only what you actually observed)*
- off-diff dependents: {N} listed / {N} accounted-for
- resource pairs: {N} read-here/destroy-elsewhere / {N} resolved
- classifier verdict *(verbatim 5 lines from the 2b½ subagent)*:
    cross_file_contract: YES|NO — reason
    async_concurrency: YES|NO — reason
    resource_lifecycle: YES|NO — reason
    state_exhaustiveness: YES|NO — reason
    security_boundary: YES|NO — reason
- classifier vs. self: {classes where we disagreed + how resolved, or "no disagreements"}

### Delegation trace (first-party off-diff impls behind the change)
| Delegated symbol | Impl `file:line` | Opened? | Fails outside declared channel? |
|---|---|---|---|
| `symbol()` | `Impl.kt:NN` | yes | finding or "no" |
*(or: None — the change delegates to no first-party off-diff service.)*

### Hazard declaration (each YES carries both-side `file:line` evidence)
| Hazard class | Applies | Evidence (`file:line` both sides, or `none found`) |
|---|---|---|
| Cross-file / contract | YES/NO — why | changed symbol `f:L` · call-sites `f:L` · delegated impl `f:L` |
| Async / concurrency / ordering | YES/NO — why | consumer `f:L` · producer `f:L` |
| Resource lifecycle | YES/NO — why | reader `f:L` · other-flow mutator/destroyer `f:L` · guard `f:L`/`none found` |
| State exhaustiveness | YES/NO — why | new variant `f:L` · each handler/adapter `f:L` |
| Security / trust boundary | YES/NO — why | boundary/input `f:L` · authz/validation `f:L`/`none found` |
````

### `--findings` — append the REVIEW_FINDINGS block (downstream contract — do not change its shape)

Print the default output, then append this block verbatim in shape. **This is a parsed contract**, not
a display format: downstream tools regex-match the markers to anchor each finding onto the diff.
Keep the markers, the heading names, and the field labels exactly as below — a cosmetic rename here
silently breaks every consumer.

```
<!-- REVIEW_FINDINGS_START -->
## Findings

### Finding 1
- **Severity:** CRITICAL
- **File:** src/path/File.kt
- **Line:** 45
- **Title:** Short description
- **Description:** WHY it matters (grounded in the code that was read)
- **Suggested fix:**
\`\`\`lang
code
\`\`\`

### Finding 2
...

## Open Questions
- `src/path:line` — unclear contract/behavior change, phrased as a question

## Files for Human Review
- `src/path` — reason

## Files Safe to Skip
- `src/generated/**` — auto-generated

## Not Reviewed
- `src/path` — changed file not read in full *(or: None)*

## Static Pre-Pass
- tool: RAN | UNAVAILABLE | DENIED (report only what you actually observed)
- off_diff_dependents: N listed / N accounted-for
- resource_pairs: N read-here/destroy-elsewhere / N resolved
- classifier_verdict:  (verbatim 5 lines from the 2b½ subagent — REQUIRED, never "skipped")
    cross_file_contract: YES|NO — reason
    async_concurrency: YES|NO — reason
    resource_lifecycle: YES|NO — reason
    state_exhaustiveness: YES|NO — reason
    security_boundary: YES|NO — reason
- classifier_vs_self: classes where classifier/tool and my declaration disagreed (+ how resolved)

## Hazard Declaration
<!-- each YES MUST carry both-side file:line evidence (or 'none found'); a YES with prose-only evidence is INCOMPLETE -->
<!-- a NO overriding the tool/classifier MUST read 'searched, none found @ file:line', never bare NO -->
- cross_file_contract: YES|NO — reason — evidence: symbol f:L; call-sites f:L; impl f:L
- async_concurrency: YES|NO — reason — evidence: consumer f:L; producer f:L
- resource_lifecycle: YES|NO — reason — evidence: reader f:L; other-flow mutator f:L; guard f:L|none found
- state_exhaustiveness: YES|NO — reason — evidence: variant f:L; handlers f:L
- security_boundary: YES|NO — reason — evidence: boundary f:L; authz f:L|none found

## Summary Stats
- Changed production files: T · Read in full: R · Not reviewed: T−R
- Files reviewed: N
- Critical: X, Important: Y, Minor: Z, Positive: P, Open questions: Q
<!-- REVIEW_FINDINGS_END -->
```

### `--json` — machine-readable only

Emit **only** a single fenced ```json block and **no prose whatsoever** — no summary, no headings, no
closing remark. Callers parse this, so it must be valid JSON with no trailing commas and no comments.
Omit optional keys rather than emitting `null`. This is the *new* structured format; the
`--findings` block above remains the contract older consumers parse, so when a caller asks for that
one, give it that one.

```json
{
  "schema": "code-review/v1",
  "title": "short review title",
  "intent": "what the change is for",
  "base": "origin/main",
  "head": "branch-name",
  "verdict": { "decision": "ship_it | needs_changes | needs_discussion", "rationale": "one line" },
  "blast_radius": "low | medium | high",
  "findings": [
    {
      "id": "C1",
      "severity": "critical | important | minor",
      "confidence": "low | medium | high",
      "file": "src/path/File.kt",
      "line": 45,
      "title": "short description",
      "description": "why it matters, grounded in the code that was read",
      "code": "the actual code as-is",
      "suggested_fix": "fixed code",
      "language": "kotlin"
    }
  ],
  "open_questions": [{ "file": "src/path/File.kt", "line": 12, "question": "phrased as a question" }],
  "well_done": ["genuine positive"],
  "human_review": [{ "path": "src/path", "reason": "new business logic" }],
  "safe_to_skip": [{ "path": "src/generated/**", "reason": "auto-generated" }],
  "accepted_trade_offs": [{ "path": "src/path", "reason": "by-design per thread #4" }],
  "trust_caveats": [
    { "kind": "not_reviewed | surface_uncovered | prepass_unavailable | delegation_not_traced",
      "detail": "what and why" }
  ],
  "coverage": { "changed_production_files": 12, "read_in_full": 12, "not_reviewed": 0 },
  "reviewers": { "dispatched": 2, "names": ["correctness", "security"], "skipped": [] },
  "existing_threads": { "open_human": 1, "by_design": 2, "resolved": 3, "own_prior": 0 },
  "static_pre_pass": {
    "tool": "ran | unavailable | denied",
    "off_diff_dependents": { "listed": 4, "accounted_for": 4 },
    "resource_pairs": { "found": 1, "resolved": 1 },
    "classifier_verdict": {
      "cross_file_contract": { "applies": true, "reason": "…" },
      "async_concurrency": { "applies": false, "reason": "…" },
      "resource_lifecycle": { "applies": false, "reason": "…" },
      "state_exhaustiveness": { "applies": false, "reason": "…" },
      "security_boundary": { "applies": false, "reason": "…" }
    },
    "classifier_vs_self": [{ "class": "async_concurrency", "resolution": "searched, none found @ f:L" }]
  },
  "delegation_trace": [
    { "symbol": "svc.applyQuota()", "impl": "DefaultQuotaService.ts:74",
      "opened": true, "fails_outside_channel": "throws on a failed precondition instead of returning Result" }
  ],
  "hazard_declaration": {
    "cross_file_contract": { "applies": true, "reason": "…", "evidence": ["symbol f:L", "call-site f:L"] },
    "async_concurrency": { "applies": false, "reason": "…", "evidence": [] },
    "resource_lifecycle": { "applies": false, "reason": "…", "evidence": [] },
    "state_exhaustiveness": { "applies": false, "reason": "…", "evidence": [] },
    "security_boundary": { "applies": false, "reason": "…", "evidence": [] }
  },
  "stats": { "critical": 1, "important": 2, "minor": 3, "positive": 2, "open_questions": 1 }
}
```

## Important rules

- **NEVER use Edit or Write** — read-only review. Collect everything; never fix code mid-review.
- **Understand before reviewing** — if intent is unclear, ask (interactive) or log an Open Question
  (headless). Don't review what you don't understand.
- **Don't do the linter's job** — never flag lint/type-checker-catchable issues; propose adding the
  rule/test instead.
- **Never dispatch an under-briefed reviewer** — full context brief, every time.
- **Collect every subagent before you emit** — one final message, complete and final. No partial
  review, no follow-up "update", no progress narration between stages.
- **Human output by default** — audit tables under `--audit`, the REVIEW_FINDINGS block under
  `--findings` (or when a caller asks for it), JSON under `--json`. Flags change what you print, never
  what you check. Trust caveats always print.
- **All output in English.** Be specific and actionable (exact `file:line`); show code as-is.
- **Be thorough first** — don't let verification delete real bugs.
- **Local rules win** (`CLAUDE.md`, `.claude/rules/`); **acknowledge good code**; **no filler.**
- This skill does NOT post to GitHub/GitLab — it reads, and it ends at the verdict.