feat(review): hybrid review flow — verdict ledger spine, cheap-gate phase, tiered findings

[bdfinst]

Context

Comparison against nWave-ai/nWave's review agents and skills. Their reviewers are uniformly small (~600–1400 words), all on haiku, with criteria factored into shared *-critique-dimensions / *-review-criteria skills. Ours are already mostly haiku (21 of 29 agents/*review*.md), so model routing is not the gap. Three structural differences are.

1. A countable pass gates the qualitative pass. Every nWave reviewer runs Phase 2 as pure arithmetic — count behaviors from AC, budget = 2 × behaviors, count actual tests, check gates G1–G9, diff RED-vs-GREEN test files — before Phase 3 judgment. A Phase-2 blocker aborts Phase 3 outright ("G9 violation = instant REJECTED, no other review dimensions matter"). Arithmetic is model-independent and short-circuits the expensive reading. Our heavy lenses (correctness-review opus/2.6k words, test-review 2.2k + 6 cited knowledge files, test-smell-review 2.2k, security-review opus/1.7k) load everything and judge everything in one pass, with no cheap-first abort inside the agent.

2. Review is a gate between steps, not a terminal event, and approvals persist. *deliver runs 9 phases; each artifact (baseline, roadmap, each step file, each TDD step) is reviewed at creation by a single scoped reviewer — 3+3N reviews per feature — sequential, verdict-gated (APPROVED/NEEDS_REVISION/REJECTED), max 2 retries, stop-on-failure. .deliver-progress.json carries validation.status, so re-invocation skips anything already approved and resumes at the failure point.

We already have most of the gating half: /build has complexity-scaled checkpoints (sub-step 4 per-step for complex, sub-step 6 batched at slice boundary), cheap-first lens ordering, narrowed verification-mode payloads, and a 5-iteration fix loop; plan-review-* and spec-compliance-review criteria mode gate before code exists. What we lack is the persistence. hooks/agent_dispatch_ledger.py records that a lens fired, bound to a subject_hash — knowledge/telemetry-schema.md explicitly documents the record decision as "non-verdict, observational". So a second /code-review re-runs every selected lens over every target file from scratch, and /build's Step 6 backstop re-reviews files already cleared at sub-steps 4/6 — --backstop-review=skip is the only lever and it is all-or-nothing.

3. Output-side tiering. nw-density-resolution-contract emits Tier-1 [REF] sections always and Tier-2 [WHY]/[HOW] expansions only on --expand. Our findings always carry full rationale and evidence.

Honest caveat on the comparison: nWave's flow costs more in aggregate (3+3N reviews, dual reviewers on the roadmap). The win is that each review carries one small artifact instead of the whole diff, and a rejection at phase 4 never pays for phases 5–7.

Goal

Keep our panel breadth and parallel dispatch; add nWave's persistence, early abort, and output tiering. Target: a repeat review pass costs the delta, not the full target set, with no reduction in findings quality.

Prior art in this repo

• chore(code-review): investigate shared/cached file reads across same-round review agents #1611 / chore(code-review): measure duplicate full-file-read token cost across review rounds #1618 established that Anthropic prompt caching structurally cannot dedupe same-round agent reads (each agent's own definition is the system prompt, so the cache prefix differs from the first block), and that the only lever is a once-per-round shared pre-pass. scripts/measure_full_file_duplication.py (PR feat(scripts): measure duplicate full-file-read token cost across review rounds #1698) measures the same-round case. This epic addresses the orthogonal across-round / across-checkpoint case, which that work did not cover.
• feat(hooks): normalized gate hash — corroboration carry-forward for whitespace/doc-only deltas without reopening #1461 #1627's cosmetic-delta carry-forward lens already skips re-review on a subject_hash_normalized match. That is the narrow precedent this epic generalizes from one whole-diff hash to per-(lens, file) granularity.
• hooks/lib/review_gate_corroboration.py + hooks/pre_pr_review.py (fix(hooks): pre_commit_review.py's .review-passed gate can be self-written #1461, fix(hooks): move the review-corroboration gate from git commit to PR creation #1886, fix(hooks): pre_pr_review gate requires 2 code-review dispatches that current /code-review never records #2147) are the existing hash-bound gate consumers a verdict ledger must not weaken.

Design constraints

• Fail closed. A missing, unreadable, or stale ledger row means review normally — never skip. Precedent: telemetry-schema.md already notes the subject_hash algorithm has changed twice (fix(hooks): cosmetic-delta normalizer reads a reindented heredoc body as formatting #1638, fix(hooks): heredoc grammars for Lua long brackets, SQL dollar-quoting, XML CDATA, PHP inline HTML #1660–fix(hooks): bound heredoc-normalizer — --unified=100000 has no size cap or subprocess timeout #1663), so a version bump invalidates carry-forward and costs one extra dispatch rather than one silent skip. Same bar here.
• Do not overload the record decision. telemetry-schema.md documents it as non-verdict and warns consumers to exclude record rows or badly overcount dispatch activity. Verdicts need their own decision or their own store.
• A skip-authorizing store must not be consent-gated. Most boundary-event streams are opt-in (~/.claude/telemetry.json, off by default). Slice 2 must resolve whether the verdict store rides boundary-events at all, and if it does, whether consent-off degrades to "never skip" (safe) rather than "skip everything" (unsafe).
• A pass row must never become a self-certification shortcut. fix(hooks): pre_commit_review.py's .review-passed gate can be self-written #1461 exists precisely to stop an agent claiming review without dispatching. A verdict row must be writable only as the recorded outcome of a genuine dispatch, and must not by itself satisfy pre_pr_review.py's corroboration.
• Shipped code is stdlib-only Python 3.10+ (ADR 0014/0015/0031); tests are pytest under the plugin's tests/ tree; /agent-audit runs after any agent or skill change.

Slices

Slice 0 gates slices 1–2 — per this repo's measure-friction-don't-assume-it norm, and #1618's precedent of scoping the measurement separately from the fix. Slices 3, 4 and 5 are independent of the ledger and of each other; they can land in any order.

0. Measure re-review duplication across checkpoints and repeat runs (gates 1–2)
1. Record per-lens verdicts bound to a content hash
2. Consume the verdict ledger to scope the backstop and repeat runs to the delta
3. Abort remaining panel dispatches on a cheap-lens blocker
4. Countable pre-phase inside a review agent (test-review pilot)
5. Tiered findings output

Acceptance criteria

• A measured re-review duplication figure from real rounds, posted to slice 0, with a stated threshold that decides whether slices 1–2 proceed.
• If they proceed: a second /code-review over an unchanged target set dispatches zero lenses; changing one file dispatches only the lenses select_lenses.py returns for that file.
• /build Step 6's backstop scopes to files not already cleared at sub-steps 4/6, and --backstop-review=skip is no longer the only granularity available.
• pre_pr_review.py's gate behavior is unchanged for every existing case, demonstrated by the existing hook tests staying green without modification.
• No reduction in findings: a fixture diff reviewed with the ledger cold and warm produces identical findings.

Out of scope

• The same-round shared-context pre-pass (chore(code-review): investigate shared/cached file reads across same-round review agents #1611/chore(code-review): measure duplicate full-file-read token cost across review rounds #1618's follow-up) — orthogonal, and gated on its own measurement.
• Restructuring agents to nWave's paired doer/reviewer topology.
• Moving criteria out of agent files into shared skills (worth its own epic; not required by anything here).

[bdfinst]

#2165 results: cross-checkpoint re-review duplication, measured against this repo's own #2180 build

scripts/measure_rereview_duplication.py (branch claude/nifty-albattani-82ujmh) implements the theoretical/empirical/report/rollup legs per #2165's acceptance criteria and was run against 7 real checkpoints from the just-merged #2180 build (5 slice boundaries, the Step 6 backstop-fixup round, and the whole-PR backstop span) plus this session's own transcript for the real-token-spend leg.

Mechanical rollup output (as rollup actually printed it)

Rollup was run over 7 cumulative-prefix samples (checkpoint 1 alone; checkpoints 1+2; ... through all 7) — the natural way to get a distribution for rollup's min/median/max, since a single checkpoint alone can never show a duplicate (duplication is only detectable when ≥2 checkpoints are compared in one invocation).

Real-transcript leg (spend_source: measured where covered):

{"distribution": {"min_pct": 0.0, "median_pct": 0.0, "max_pct": 29.45, "n_rounds_sampled": 7}, "threshold_pct": 5.0, "recommendation": "do-not-proceed"}

Byte-estimate-only leg (no transcript, cross-check):

{"distribution": {"min_pct": 0.0, "median_pct": 0.0, "max_pct": 20.69, "n_rounds_sampled": 7}, "threshold_pct": 5.0, "recommendation": "do-not-proceed"}

Taken at face value, the code-computed recommendation is "do-not-proceed" (median 0% < 5%). That reading is misleading, and I don't want it copied without the context below.

Why the median is misleading here

6 of the 7 cumulative-prefix samples (checkpoints 1 through 1+2+3+4+5+backstop-fixup) are structurally guaranteed near-zero: /build's own slice-boundary checkpoints mostly touch disjoint files, so there's little for an early prefix to duplicate against yet. Only the 7th sample — prefix 1-6 plus the whole-PR backstop checkpoint (2315484:0176f63, matching build/SKILL.md's "all files modified during the build") — is capable of surfacing the phenomenon #2165 asks about at all. A 7-sample median dominated 6-to-1 by structural zeros is the wrong summary statistic for this question; the single meaningful data point is that last sample.

The real finding: the whole-PR backstop checkpoint alone

At the full-PR backstop checkpoint, 216 (lens, file_hash) pairs recur from an earlier checkpoint (mostly: a file first reviewed at a slice boundary, never touched again, then re-swept by the Step 6 backstop's full-diff review):

Leg	avoidable_pct_of_total	avoidable tokens	total tokens
Byte-estimate (trustworthy)	20.69%	1,507,248	7,284,852
Real-transcript "measured" (inflated — see caveat below)	29.45%	184,573,700	626,769,653

Both are comfortably above the 5.0% threshold. My read: this supports proceeding with #2164 slices 1-2, though see the caveats below before treating that as settled.

Caveats (stated per #2165's AC, before any number was read)

1. 7 real checkpoints stand in for "≥3 historical sessions." All 7 come from one epic's (feat(specs): close the DeFOSPAM spec-review placement gaps #2180's) history, not independent builds — git fsck --unreachable --no-reflogs confirmed no other epic's commits are recoverable in this container.
2. Only 1 real session transcript was available in this environment for the token-conversion leg (this session's own).
3. The theoretical leg is a ceiling, not an observed count — it counts a pair as re-dispatchable whenever a lens is applicable with matching content at a later checkpoint, not whether a real /build//code-review run actually re-dispatched it (no ledger exists yet to check against — that's what feat(review): hybrid review flow — verdict ledger spine, cheap-gate phase, tiered findings #2164 slices 1-2 would add).
4. 5.0% is a provisional anchor borrowed from chore(code-review): measure duplicate full-file-read token cost across review rounds #1618's orthogonal same-round measurement (which found same-round duplication well under 5%), not independently derived for this cross-round question.
5. Newly discovered during this validation: the "measured" real-spend leg over-counts when a lens's single dispatch covers many files. report's spend_source: "measured" substitution applies one flat per-lens average dispatch cost to every duplicate file individually — correct when a lens dispatches roughly once per file, but a review-agent dispatch typically reviews a whole changeset (many files) in one sitting, so this multiplies one dispatch's real cost across every file that happens to be a duplicate under that lens. That's why the measured leg's absolute totals (184.6M avoidable / 626.8M total tokens) vastly exceed this transcript's actual total real spend across all 37 dispatches (88.6M tokens) — an internal inconsistency that confirms the over-count. The byte-estimate leg (20.69%) doesn't have this defect (it prices each file independently from its own size) and is the more trustworthy of the two numbers above; I'm treating the "measured" 29.45% as directionally consistent but overstated. I'll file a follow-up issue for this rather than fix it in this PR (fixing it well needs a redesign of the per-lens cost attribution, not a quick patch).

Bottom line

The mechanical recommendation field is "do-not-proceed" on a naive median read, but that's an artifact of sampling 6 structurally-zero cumulative prefixes alongside the 1 sample that can show the effect. The actual, most meaningful measurement — the whole-PR backstop checkpoint alone — shows 20.69%-29.45% cross-checkpoint re-review duplication, well above the 5% threshold. Per the plan's own framing, this is advisory input to your own go/no-go call on #2164 slices 1-2, not a mechanical trigger — but given the caveats above (small N, one epic's history, a known over-count in the higher of the two numbers), I'd treat this as a reasonably strong "worth building," with the understanding that a larger/more varied validation sample would sharpen it further.

Raw theoretical/report/rollup JSON outputs for all 7 checkpoints (both legs) are available in this build's branch history if useful for a closer look.

---

Generated by Claude Code

[bdfinst]

#2170 acceptance criterion: measured report-size before/after

Per #2170's acceptance criteria ("A measured before/after on the rendered report size for a real multi-finding round, posted to #2164"), here's a real measurement from PR #2197's own /code-review --internal backstop review round (26 findings across 12 dispatched review agents: naming, arch, structure, test-review, test-smell, domain, correctness):

Rendering	Bytes	Reduction
Old (full message + suggested fix inline, pre-#2170)	24,761	—
New (Tier-1 default, render_tiered_findings.py)	10,154	59.0%

Same 26 findings, same content — the difference is purely Tier-1's first-sentence-only default versus the old always-full-message listing. Tier-2 (full message + suggested fix) remains available per-finding via --expand <finding-id>|all with no re-dispatch, from the same in-memory finding list.

See PR #2197 for the implementation (slices #2168/#2169/#2170).

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01KGHpm7gtqNb8NM2Smf76u7

---

Generated by Claude Code

[github-actions]

Closed automatically by the epic-auto-close workflow: every sub-issue is now closed (triggered by #2167).