Skip to content

review eval: fix floor-case ground truth, mine noise pool, mint mid-band cases - #270

Merged
jwbron merged 7 commits into
mainfrom
review-eval/corpus-discrimination
Jul 21, 2026
Merged

review eval: fix floor-case ground truth, mine noise pool, mint mid-band cases#270
jwbron merged 7 commits into
mainfrom
review-eval/corpus-discrimination

Conversation

@jwbron

@jwbron jwbron commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Improves the discrimination power of the review eval corpus, working from the 2026-07-20 drift run (PR #265, run 29724668102). Every claim below was verified by replaying that run's recorded artifacts through the deterministic matcher at $0 (--no-match-arbiter semantics); the deterministic suite passes before and after (346 -> 348 tests).

1. Floor-case triage (replay evidence)

Case / row Recorded Deterministic replay Verdict
golden-retention-lifecycle-3 (verdict) 0/6 0/6, trap false-flagged 6/6 Ground truth bug. The "documented deliberate" void pruneNotes(...) was a real defect: void discards the promise but not its rejection, so a failing prune is an unhandledRejection that can crash the process, the opposite of the comment's stated intent. The reviewer blocked it, correctly, in every sample (retention.ts:26, blocking, conf 0.7, all 6). Fix: the tree now implements the intent (.catch(() => {})), same line count, diff updated; the must-not-flag trap now also covers the swallow phrasing. Needs one live confirmation run (recorded artifacts reviewed the old tree).
trial-dedup-eventual-consistency (verdict) 0/2 2/2 after fix Ground truth bug. Expected APPROVE contradicted the case's own premise: the PR claims to stop double-submit duplicates and the eventually consistent read fails for exactly that traffic. Both samples caught the defect (spec 2/2) and blocked; flipped expected verdict to REQUEST_CHANGES, recorded fixture raised to blocking.
golden-retention-lifecycle-1:retention-unbounded-prune 0/6 0/6, no dropped-bucket rescue Genuine miss, unfair spec. No posted finding in any sample describes the mechanism, so the matcher is fine; but the facade documents no deleteMulti batch limit, so the "defect" was not evidenceable in-tree (the reviewer would have had to invent the constraint). Spec retired; the mechanism returns as mutation-retention-batch-delete-limit with the limit documented (below).

Two instrument bugs surfaced by the same replay (the sql-missing-index precedent, twice):

  • golden-retention-lifecycle-2:retention-dedup-window-untested, the corpus's only "in-band" row (3/6), was entirely Haiku-arbiter false positives: all three recorded catches were via: "fallback" claims of the overcorrected off-by-one finding at retention.ts:38, a different defect. Deterministic rate was 0/6. The bare "boundary" mechanism alternate (shown to the arbiter as ground truth) was the false-positive vector. Fixed: dedup/window-specific mechanisms plus an altLocations entry for the test file; replays an honest 3/6 deterministic, matched findings verified to be the dedup-coverage gap.
  • trial-dedup-composite-key:save-dedup-composite-key: reviewer caught it 5/5, deterministic matcher 1/5 (order-sensitive regexes); the arbiter was papering over it. Broadened alternates replay 5/5 deterministic.

2. Noise-pool mining (all 126 unmatched findings classified)

The 6-sample noise pool (55 baseline + 61 candidate after replay, 52-55% reported) decomposes into 36 (case, path, line) clusters:

Class Instances Disposition
Real defect, unannotated 43 Annotated: 4 new specs (below)
Duplicate report of an already-matched defect ~35 Stays noise (it is real author-facing chattiness; the two-comments-per-defect pattern in auth-bypass, cache-missing-key, authz, mutation-money, race-condition, money-rounding is a review-side dedup gap worth its own fix)
Re-review duplicates of KEPT prior threads 12 Stays noise by design: lifecycle-2's overcorrected off-by-one and still-vacuous test map to threads with expect: keep; fresh findings there are what re-review dup-scoring penalizes. Deliberately NOT annotated (reasons recorded in the case description)
Companion test-gap advisories, contract-hardening suggestions, design commentary ~30 Chattiness; not annotated
Injection attempt surfaced as advisory (adversarial-injection-approve handler.ts:8, 5/6) 5 Desired behavior, not a defect; left unannotated to avoid touching the hard-gate case. Candidate spec if we later want to pin "surface the injection" as recall
lifecycle-3 advisory cluster (purge test gap etc.) ~8 Advisory-grade on an expected-clean re-review case; stays noise

New specs from the "real defect" class, all replay-verified:

  • incident-sql-missing-index:dm-default-backfill (5/6): DEFAULT 'pending' backfills every fulfilled historical order into the picker queue; altLocations accepts the migration or the query anchor.
  • golden-retention-fix-push:quota-exceeded-never-true (6/6): remaining is clamped to zero then compared < 0, so the quota is never enforced; introduced by this push's own hunk.
  • golden-retention-lifecycle-1 and -2 retention-prune-unhandled-rejection (6/6 each): the void-prune crash, flagged blocking in every sample of both pushes. lifecycle-1 keeps four planted defects (the retired unbounded-prune slot is taken by this real one, fixture and all); lifecycle-3's fixed tree closes the arc.

No saturated case's existing specs, trees, or expectations were touched.

3. New mid-band cases (validation pending)

Both target the demonstrated weak class (multi-file retention/lifecycle reasoning), graded easier than the retired floor specs: the evidence chain exists in-tree but crosses files.

  • mutation-retention-batch-delete-limit: unchanged purge-user-data.ts documents the datastore's 100-key deleteMulti limit and chunks accordingly; the new prune passes an entire launch backlog to one call, the documented fire-and-forget catch swallows the rejection, and the backlog test passes only because the in-memory fake enforces no limit. Carries the same must-not-flag swallow trap as lifecycle-3.
  • mutation-retention-window-units: window constant in days (config.ts), createdAt documented as epoch milliseconds (db.ts), the since arithmetic omits the *1000, so the "30-day" window is about 43 minutes and dedup silently no-ops for the traffic it names.

Neither is smoke-tagged, so the per-PR A/B on this PR does not exercise them. Validation is done: the identical-arm calibration (run 29765059892) and the 2026-07-20 pooled A/B (runs 29783522498-29783549087) both saturated them (batch-delete-limit 2/2 then 6/6 across arms, window-units 2/2 then 5/5). Consistent with the eval README's synthetic-saturation finding, they stay live as family tripwires rather than discriminators; both case descriptions record the outcome and run ids. The pooled A/B also exposed a trap over-match (two candidate samples posted a non-blocking "log instead of an empty catch" companion advisory that scored as a false flag next to the deterministic catch): specs now support blockingOnly, set on both swallow traps, so only a blocking condemnation of the documented fire-and-forget can hit them.

4. Budget

The 2026-07-20 run spent $84.33 of its $85 cap and budget-skipped the corpus tail (composite-key 5/6 samples, dedup-eventual-consistency 2/6, erasure-suite-flag-mock 1/6), which is why the noise-floor bands carry the case-mix warning. Measured $0.82 per case-arm-run; the 23-case corpus (both new cases are live-tagged) at 3 repeats x 2 arms is 138 case-arm-runs, $113 projected. Default max_usd raised to 120. Rejected: 2 repeats ($62, halves the power axis) and per-case repeat tiering ($58, needs live-ab changes and makes every noise-floor sample a different case mix, the exact contamination the aggregate warns about).

Corpus shape, before -> after

Spec rows per drift run (recorded 2026-07-20 ruler vs this branch replayed on the same artifacts):

Band Before (recorded) After (deterministic replay)
Floor (0-20%) 3 rows: unbounded-prune 0/6, lifecycle-3 verdict 0/6, dedup-EC verdict 0/2 (+ window-untested 0/6 in truth) 1 row: erasure-suite-flag-mock verdict 0/1 (1 sample; out of scope here, same severity-calibration family as dedup-EC, worth watching next run)
In-band (20-80%) 1 illusory row (window-untested 3/6, all arbiter FPs) 1 honest row (window-untested 3/6 deterministic) + 2 new cases pending validation
Ceiling (80-100%) 15 spec rows + 16 verdict rows 21 spec rows + 18 verdict rows (4 mined specs and 2 fixture rows are deliberate: they clean the noise metric and add tripwires)

Unmatched-posted findings on the same artifacts drop 126 -> 96 (noise ~52/55% -> ~44/45%), all of it from mislabeled ground truth, none from loosening the matcher's location-AND-mechanism rule.

Note for the drift series: the corpus content hash changes, so the report's Ruler line will flag the discontinuity; week-over-week rows for untouched saturated cases remain comparable.

jwbron added 4 commits July 20, 2026 09:59
…d truth and spec matching from drift triage

Replayed the 2026-07-20 drift artifacts (run 29724668102) through the
deterministic matcher; three 0% rows and one fake in-band row traced to
ground-truth or instrument bugs, not reviewer behavior:

- golden-retention-lifecycle-3 verdict 0/6: the 'documented deliberate'
  void prune was a real defect (a rejected prune is an unhandled
  rejection that can crash the process), and the reviewer correctly
  blocked it in every sample. The tree now implements the documented
  intent (.catch(() => {})) so the expected APPROVE is fair; the
  must-not-flag trap mechanisms cover the swallow phrasing.
- trial-dedup-eventual-consistency verdict 0/2: expected APPROVE
  contradicted the case's own premise; the PR claims to stop
  double-submit duplicates and the eventually consistent read fails for
  exactly that traffic. Ground truth flipped to REQUEST_CHANGES
  (recorded finding raised to blocking); verdict replays 2/2.
- golden-retention-lifecycle-1 retention-unbounded-prune 0/6: genuine
  miss, but the facade documents no deleteMulti batch limit, so the
  defect was not evidenceable in-tree. Spec retired; the mechanism
  returns as mutation-retention-batch-delete-limit with the limit
  documented.
- golden-retention-lifecycle-2 retention-dedup-window-untested: the
  recorded 3/6 was entirely Haiku-arbiter false positives claiming the
  overcorrected off-by-one finding (the bare 'boundary' alternate).
  Mechanisms tightened to dedup/window-specific phrasing plus an
  altLocation at the test file; replays an honest 3/6 deterministic.
- trial-dedup-composite-key: order-sensitive mechanism regexes matched
  1/5 deterministic while the reviewer caught it 5/5 (arbiter rescues).
  Broadened alternates replay 5/5 deterministic.
…s mined from the drift noise pool

Classified all 126 deterministically-unmatched posted findings from the
2026-07-20 drift run. Four clear-cut real defects were being counted as
noise; each becomes a mustCatchSpec (all replay 5-6/6 deterministic):

- incident-sql-missing-index dm-default-backfill: DEFAULT 'pending'
  backfills every fulfilled historical order into the picker queue
  (flagged in every sample, anchored at the migration or the query;
  altLocations covers both).
- golden-retention-fix-push quota-exceeded-never-true: remaining is
  clamped to zero then compared < 0, so the quota is never enforced.
- golden-retention-lifecycle-1/2 retention-prune-unhandled-rejection:
  the void prune crash also exists in pushes 1 and 2 and was flagged
  blocking in every sample.

Not annotated, with reasons recorded in the case descriptions: the
lifecycle-2 overcorrected off-by-one and still-vacuous test map to KEPT
prior threads (fresh findings there are re-review duplicates by
design), and duplicate reports of already-matched defects stay noise.
…tention/lifecycle cases

The 2026-07-20 drift run showed a bimodal corpus: after the triage
fixes, every spec row sits at 100% except one honest 3/6; the 0% rows
were ground-truth bugs. Discrimination needs the 20-80% band, and the
reviewer's demonstrated weak class is multi-file retention/lifecycle
reasoning, so both new cases are graded easier than the retired floor
specs but still require cross-file inference:

- mutation-retention-batch-delete-limit: the unchanged purge module
  documents the datastore's 100-key deleteMulti limit and chunks; the
  new prune passes an entire launch backlog to one call and the
  documented fire-and-forget catch swallows the rejection. Replaces the
  retired retention-unbounded-prune spec with in-tree evidence.
- mutation-retention-window-units: config.ts holds the window in days,
  db.ts documents createdAt as epoch milliseconds, and the since math
  omits the *1000, making the 30-day dedup window about 43 minutes.

Both carry recorded fixtures for the deterministic suite and are NOT
smoke-tagged. Catch rates must be confirmed in-band by a small live
run before the weekly series leans on them; the dispatch commands and
cost caps are in the PR description.
…et so the full corpus clears

The 2026-07-20 drift run spent $84.33 of its $85 cap and budget-skipped
the tail of the corpus (composite-key, dedup-eventual-consistency,
erasure-suite-flag-mock ran 1-2 samples instead of 3), which also
contaminated the noise-floor bands with case-mix variance; the report
flags this itself. Measured cost was ~$0.82 per case-arm-run; the
21-case corpus at 3 repeats x 2 arms is 126 runs, ~$103 projected.

Raise the default max_usd to 120. Alternatives priced and rejected:
2 repeats saves ~$40 but halves the power axis and shrinks every
binomial interval's sample; 1-repeat tiering for saturated tripwires
saves ~$45 but needs live-ab changes and permanently asymmetric
noise-floor samples (the exact contamination the README warns about).
@changeset-bot

changeset-bot Bot commented Jul 20, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: e306982

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
review Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@khan-actions-bot
khan-actions-bot requested review from a team, jaredly and somewhatabstract and removed request for a team July 20, 2026 17:01
@github-actions

Copy link
Copy Markdown
Contributor

Review live A/B

No reviewable delta: review.md is byte-identical in both arms (baseline origin/main, sha 8e8da9de53bf), so the extracted prompts and the orchestrator body match and no arms were run. Pass --force-arms for a deliberate wobble control.

@github-actions

This comment has been minimized.

Comment thread .github/workflows/review-eval-drift.yml Outdated
@jwbron

jwbron commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Ran the two pending validation dispatches for this PR's minted cases (identical Opus arms via force_arms, 3 repeats, on review-eval/fable-case-calibration, run 29765059892):

Case / spec Catch rate (6 identical-arm samples)
mutation-retention-batch-delete-limit 6/6 (100%), verdict 6/6
mutation-retention-window-units 6/6 (100%), verdict 6/6

Both calibrate saturated, not mid-band. Same outcome as the three discrimination cases minted on the stacked #266 branch (removed-behavior, cross-file chain, 13-file churn needle): those read 4/4 in v1 AND 4/4 after a hardening pass that removed the stated invariants (runs 29763213774, 29764855482). The pattern across all five hand-authored synthetics from three different design philosophies is consistent: small single-defect synthetic trees saturate under the current Opus roster; the genuinely in-band and floor rows in the drift series all come from the larger golden/incident trees with real indirection.

Suggested follow-ups (no change requested on this PR; the cases are still valuable as family tripwires and the noise-metric cleanup stands):

  1. Update the two case descriptions to record the calibration outcome and reclassify them as tripwires rather than mid-band targets.
  2. The corpus's discrimination gap is structural: mid-band difficulty appears to require tree scale and indirection, not subtler planting in small trees. The next experiment worth running is a real-PR-scale case (30+ files, defect spanning subsystem boundaries) or minting directly from production incidents per the README's guidance.

The adversarial gate passed in every repeat, and the noise floor on these runs was tight (recall and verdict spread 0%).

jwbron added 2 commits July 20, 2026 22:07
…res and review fixes

- Port the drift-run regression fixtures for the dedup-window spec from
  the parallel triage branch (PR #269): the real test-file coverage
  finding must match deterministically and the arbiter's false-accepted
  off-by-one must never match.
- Add the (test|coverage).{0,80}dedup mechanism alternate; the drift
  artifact replays 4/6 deterministic (one sample phrases the gap as
  'test does not exercise ... dedup', which the order-sensitive
  alternates missed). Both pinned fixtures still hold.
- Widen the batch-delete-limit mustCatch window to cover the
  fire-and-forget .catch line: it is part of the defect's causal chain,
  and mustCatch claims run before the swallow trap, so a correct
  diagnosis anchored there scores as a catch instead of a false flag.
- Correct the drift budget sizing comment: 23 live cases (both new
  mutation cases are live-tagged), 138 case-arm-runs ~$113, and note
  the thin headroom against the budget-skewed per-run mean.
- Lead the new cases' tags with synthetic-mutation to match siblings.
…rd the minted cases' calibration outcomes

The 2026-07-20 pooled A/B (runs 29783522498-29783549087) validated the
two minted cases: both saturated (batch-delete-limit 6/6, window-units
5/5 across arms), so they stay as family tripwires per the README's
synthetic-saturation finding; descriptions now carry the outcome.

The same runs showed the swallow trap over-matching: two candidate-arm
samples posted a non-blocking 'log instead of an empty catch' companion
advisory that matched the trap's mechanism and scored as a false flag
alongside the deterministic catch. New optional spec field blockingOnly
pins a spec to blocking candidates; set on both swallow traps, so only
a finding that condemns the documented fire-and-forget as a defect can
hit them. Non-blocking mentions stay noise, which the noise metric
already counts.
@github-actions

Copy link
Copy Markdown
Contributor

Review Guidance

github-actions (2 files)
File Reason
live.ts Eval-suite spec parser; this change adds the blockingOnly field to LiveDefectSpec, so a parse bug would silently alter ground-truth loading for the whole corpus.
live-match.ts Deterministic finding-to-spec matcher; the new blockingOnly gate decides which findings match a spec, i.e. which reviewers score a catch.

Common patterns

2 files: Identical memDb/testing.ts in-memory fake added verbatim to both new synthetic-mutation fixture trees.

2 files: Identical db.ts facade type definitions added to both new synthetic-mutation fixture trees.

2 files: retention-prune-unhandled-rejection defect spec replaces the retired retention-unbounded-prune across two golden lifecycle case.json files.

2 files: blockingOnly: true added to mustNotFlagSpecs trap entries, matching the new field in live.ts.

Excluded from review (5 files)

Not individually reviewed — generated, formatting-only, or fully explained by a common pattern above:

  • .changeset/review-eval-corpus-discrimination.md — generated
  • workflows/review/eval/corpus/synthetic-mutations/mutation-retention-batch-delete-limit/tree/src/notes/db.ts — pattern-only
  • workflows/review/eval/corpus/synthetic-mutations/mutation-retention-batch-delete-limit/tree/src/notes/testing.ts — pattern-only
  • workflows/review/eval/corpus/synthetic-mutations/mutation-retention-window-units/tree/src/notes/db.ts — pattern-only
  • workflows/review/eval/corpus/synthetic-mutations/mutation-retention-window-units/tree/src/notes/testing.ts — pattern-only
Lower-confidence observations (not posted inline)
  • .github/workflows/review-eval-drift.yml:47 — Stated goal was mid-band discrimination, but both minted cases calibrated saturated (6/6, 5/5) while the budget default grows 85→120; consider tracking the still-open mid-band gap and running saturated tripwires at reduced repeats.

pr-reviewer:1.4.1 schema:2

@jwbron
jwbron merged commit 18a7a69 into main Jul 21, 2026
11 checks passed
@jwbron
jwbron deleted the review-eval/corpus-discrimination branch July 21, 2026 18:12
jwbron added a commit that referenced this pull request Jul 21, 2026
…rift budget (#267)

## What

Two follow-ups from the first scheduled drift report (#265):

1. **Identical-arm eval reports no longer read as an A/B.** When both arms ran the same review.md (only possible under `--force-arms`: the weekly drift watch and manual wobble controls), the single-run, multi-repeat, and aggregate renderers now retitle themselves "Review wobble control (identical arms)", relabel Baseline/Candidate to Arm A/Arm B, and state up front that between-arm deltas are run-to-run wobble, not a prompt effect. The aggregate additionally leads with the noise-floor bands: on an identical-arm pool the bands are the product and the per-case table is the raw material. The aggregate keys the relabel off `noiseFloor` being present, which is computed iff every pooled sample ran identical arms.

2. **A contaminated noise floor is a red run, not a footnote.** The 2026-07-20 run spent $84.33 of its $85 cap and budget-skipped cases, which the report flags as contaminating the noise-floor bands with case-mix variance; but the flag lives inside the report body, so the degradation persists silently until someone reads it. The drift job now fails when the aggregate reports `caseAsymmetry` (after the artifact upload and visibility PR, both `if: always()`), so a corpus outgrowing the budget surfaces as a red run. The budget resize itself (85 -> 120 -> 240) moved to the corpus-growth stack (#270/#266), which owns the sizing; three open PRs were editing the same lines.

## Why the relabel matters

#265 renders a "Baseline vs Candidate" per-case table with small deltas between two runs of the same prompt; that framing invites reading wobble as a result. The drift run's whole point is that within-run deltas are noise, so the report should say so in its own headline.

## Testing

- `pnpm test --run workflows/review/eval/` (new cases cover the multi-repeat, single-run, and aggregate relabels, the noise-floor-first ordering, the identical-arm drop-note prefix, and that A/B reports keep the Baseline/Candidate framing)
- `pnpm typecheck`, prettier clean on changed files

Author: jwbron

Reviewers: github-actions[bot], jwbron, jeresig, kevinb-khan

Required Reviewers:

Approved By: github-actions[bot], jeresig

Checks: ✅ 9 checks were successful, ⏭️  1 check has been skipped

Pull Request URL: #267
jwbron added a commit that referenced this pull request Jul 21, 2026
…ads (#271)

## What

Closes the one genuine reviewer blind spot from the 2026-07-20 drift triage (#265, follow-up to #269/#270): `retention-unbounded-prune` was missed in all 6 drift samples, and the reviewer's own suggested fix for an adjacent bug recommended the same unbounded-read pattern (`pageSize: "all"`), so no lens was reasoning about memory-bounded reads at all.

Two prompt changes:

1. **Correctness lens, line scan**: unbounded reads and accumulation join the named defect classes; a query/fetch/scan that materializes a result set that grows with user data (missing LIMIT, `pageSize: "all"`, loading a whole table to act on a subset, unpaginated buffering). The rule also names the expected shape (page or batch it), so the fix pattern (a bounded read that deliberately processes one batch per invocation) is not itself flagged; that matters because `golden-retention-lifecycle-2/3`'s `PRUNE_BATCH` prune is exactly that shape and must stay clean.
2. **caching-resource specialist**: a matching "No unbounded reads" review rule and an `unbounded-read-materialization` tri-state hunt. The correctness lens is the always-on catch; the specialist adds depth when routed. (Note per review: no ROUTING routes `caching-resource` today, in this repo or webapp, so the correctness-lens rule is the operative change; the specialist half is the documented home for depth once a consuming repo routes storage-heavy paths to it.)

## Measurement

Recall-affecting by design. The original acceptance recipe targeted `golden-retention-lifecycle-1:retention-unbounded-prune`, but the corpus stack (#270/#273/#266) retires that spec: the lifecycle-1 facade documents no batch limit, so under a lens-less ruler the defect was ruled not evidenceable in-tree, and the slot now tracks the void-prune unhandled rejection. This PR is exactly the lens that makes the retired row fair again (the lens supplies the norm the tree does not state), so the plan is:

1. Land the corpus stack first, then rebase this PR onto main.
2. Re-add the retired `retention-unbounded-prune` spec to `golden-retention-lifecycle-1` in this PR (spec text is in git history at #270's parent), as the measurement instrument for the lens: without it, no corpus row tracks flagging an *undocumented* unbounded read (#270's review noted the gap).
3. Run the powered recipe:

```
gh workflow run review-eval-ab.yml --ref review-unbounded-read-lens \
  -f base_ref=origin/main -f cases=golden-retention-lifecycle-1 -f repeats=10 -f max_usd=30
```

Success: candidate arm catches `retention-unbounded-prune` at a real rate (baseline is 0/6 in the drift record) with lifecycle-2/3 clean on the bounded `PRUNE_BATCH` prune (no false flags), and no new flags on `mutation-retention-batch-delete-limit`'s already-caught defect beyond the existing catch.

Author: jwbron

Reviewers: github-actions[bot], jwbron, jeresig, jaredly, somewhatabstract

Required Reviewers:

Approved By: github-actions[bot], jeresig

Checks: ⏭️  1 check has been skipped, ✅ 9 checks were successful

Pull Request URL: #271
jwbron added a commit that referenced this pull request Jul 21, 2026
…calibration finding (#273)

> Stacked on #270. Companion to #266 (the Fable model swap), split out so the model swap stays a pure, revertable diff and the corpus work is reviewable on its own.

## What

Grows the live eval corpus by seven cases in this PR's diff (nine across the stack, counting the two minted in #270) across seven defect families it previously had no coverage for, each with a verified ground-truth defect, an exact git-generated diff, and deterministically matched specs (0 anchor snaps and 0 found-but-dropped across every calibration run):

| Case | Family |
| --- | --- |
| mutation-removed-behavior-logout-cache | Removed-behavior regression (refactor drops cache invalidation) |
| mutation-crossfile-report-redaction | Cross-file inference chain (PII to a partner webhook) |
| mutation-needle-paging-rename | Needle in 13 files of mechanical churn, plus a mustNotFlagSpec precision probe on a correctly migrated call site |
| mutation-retry-nonidempotent-capture | Partial-failure / distributed semantics (retry double-charge) |
| mutation-toctou-invite-limit | Check-then-act concurrency (seat-limit race) |
| mutation-window-boundary-doublecount | Boundary composition (inclusive endpoints double-count) |
| mutation-largetree-digest-category-bypass | 29-file cross-subsystem tree; the violated invariant sits two unchanged hops from the diff |

All are tagged `live` (never `smoke`), so per-PR cost is unchanged; they run in full-corpus, powered, and weekly drift selections.

## The calibration finding (the important part)

Every case was calibrated with identical-Opus-arm `--force-arms` runs before any band claim (runs 29763213774, 29764855482, 29765059892, 29765275168, 29767404342): all nine, plus the two minted in #270, read 100% recall across ~56 identical-arm samples, including a hardening pass that removed every stated invariant and the 29-file scale test. Hand-authored synthetics do not reach the 20-80% discrimination band at any feasibly authorable size; every case that has ever calibrated in-band or at floor derives from real material. This is now documented in `eval/README.md` under "Growing the corpus", with the operational guidance: author synthetics as deliberate family tripwires, grow recall discrimination from golden human-comment cases and production incident repros, and calibrate every new case before claiming a band.

The unmatched-posted findings on these cases were audited: they are companion findings on the already-caught defect from a second angle (the known reviewer-side dedup gap #270 documented), deliberately not annotated as specs.

## Notes

- Case descriptions carry their calibration outcomes and run ids inline, so the drift series and future authors see the classification without archaeology.
- The weekly drift budget bump for the grown corpus rides in #266 (its sizing depends on the Fable-arm rates that PR introduces).

Author: jwbron

Reviewers: github-actions[bot], jwbron, jeresig, kevinb-khan

Required Reviewers:

Approved By: github-actions[bot], jeresig

Checks: ✅ 10 checks were successful

Pull Request URL: #273
jwbron added a commit that referenced this pull request Jul 21, 2026
> De-scoped 2026-07-21 per review: the swap covers `correctness-reviewer` only. Now based on main (the corpus stack #270/#273 and the drift/budget PRs have landed); the de-scoped roster has its own powered A/B below.

## What

Switches the review workflow's `correctness-reviewer` from `claude-opus-4-8` to `claude-fable-5` (effort high, unchanged). Bug-finding recall is the workflow's load-bearing metric, and both the original five-role bundle A/B and the de-scoped re-run concentrate their gain in correctness-adjacent rows.

Deliberately NOT moved, per review (jeresig) and per the measurements:

- **`claim-validator`** stays Opus at xhigh: the bundle's Fable validator did not improve the precision gate, and the de-scoped re-run confirms the noise increase never came from the validator (it reproduces without it; see below).
- **`holistic` / `completeness` / `test-adequacy`** stay Opus: the bundle A/B could not attribute any gain to them, and both known consumers `enable` them in ROUTING, so they price into every full review.
- The orchestrator, `thread-reconciler` / `skill-auditor` / `conventions`, and all specialist lenses stay Opus (the lenses deliberately: Fable's cyber safety classifiers can false-positive refuse benign security analysis, and a refused lens is a silent coverage hole). `pattern-triage` stays Sonnet 4.6; `first-principles` already ran Fable 5.

Each stay-on-Opus reason is pinned in the agent definition in `review.md`. The weekly drift budget is sized from the de-scoped roster's measured rate (~$1.08/case-arm-run): 180 case-arm-runs ~ $194/week, cap 220.

## The de-scoped measurement (powered A/B)

Nine pooled dispatches (runs 29855626692-29855643020), same shard-and-pool structure as the bundle run: 30 live cases x 3 repeats per arm; baseline = all-Opus roster, candidate = this branch. Aggregated with `eval/aggregate.ts`.

| Metric | Opus 4.8 | Fable 5 correctness-only | (five-role bundle, superseded) |
| --- | --- | --- | --- |
| Must-catch recall | 82% (101/123) | **89% (110/123)** | 83% -> 90% |
| True misses | 22 | **13** | 21 -> 12 |
| Verdict agreement | 96% | 96% | 94% -> 97% |
| Noise (unmatched posted) | 43% | 49% | 43% -> 49% |
| Judge mean quality | 0.88 | 0.86 | 0.87 -> 0.86 |
| Adversarial hard gate | — | PASSED (all shards) | PASSED |
| Cost | $72.12 | $97.19 (**+35%**) | +73% |

**The de-scope kept the entire recall gain at half the cost delta.** The improved rows are the same previously in-band/floor spec rows as the bundle run (`incident-sql-missing-index:dm-default-backfill` 1/3 -> 3/3, `trial-retention-deletion:erasure-default-limit-one` 0/3 -> 2/3, `:erasure-ignores-delete-helper` 1/3 -> 2/3, `:erasure-prune-error-swallowed` 0/3 -> 1/3, `golden-retention-fix-push:quota-exceeded-never-true` 2/3 -> 3/3, `golden-retention-lifecycle-2:retention-prune-unhandled-rejection` 2/3 -> 3/3), so the bundle's gain is now attributed: it was the correctness reviewer.

**What the re-run disambiguated.** The noise increase (43% -> 49%) reproduces without the Fable validator or whole-change reviewers: it is the Fable correctness reviewer posting more companion findings (219 posted vs 178). Likewise the one repeated verdict regression: `golden-retention-lifecycle-3` (expected-clean re-review) 3/3 -> 1/3, where two candidate samples flag the deliberate single-batch `PRUNE_BATCH` prune as blocking. Note both arms in this run predate #271 (merged since), whose correctness-lens language explicitly names the bounded-batch prune as the fix shape, not a defect; the first drift run on merged main will show whether that attenuates this. If not, the corpus-side option is a bounded-prune must-not-flag trap on lifecycle-3.

**Statistical honesty.** The pooled +7-point recall delta is z ~ 1.7 (suggestive, not conclusive at 95%; pooled intervals are optimistic per the eval README). As with the bundle run, the per-case pattern is the stronger evidence: six hard rows improved, none regressed on recall, true misses down 41%. The aggregate stamps the expected mixed-corpus-hash warning (three shard groups select different case subsets); within every shard both arms share the same ruler.

The superseded five-role bundle measurement (runs 29783522498-29783549087) is preserved in the edit history and summarized in the table's last column.

## Known risks to watch in production

- Occasional `refusal` stop on cyber-adjacent diffs hitting `correctness-reviewer` (surfaces as a missing agent result; none observed across ~380 live case-arm-runs to date). The weekly drift corpus is the soft-refusal detector: a silently refusing correctness reviewer craters must-catch recall on the security-adjacent cases.
- Author-facing chattiness: the Fable correctness reviewer posts more companion findings (noise 49% vs 43% on the eval's strict matcher). The reviewer-side dedup gap (two comments per defect) is the known follow-up that would absorb most of this.
- Over-blocking on expected-clean re-review pushes (lifecycle-3 above); watch the drift series now that #271's lens carve-out is on main.

Author: jwbron

Reviewers: github-actions[bot], jwbron, jeresig, kevinb-khan

Required Reviewers:

Approved By: github-actions[bot], jeresig

Checks: ✅ 9 checks were successful, ⏭️  1 check has been skipped

Pull Request URL: #266
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants