review eval: nine family-tripwire cases and the synthetic-saturation calibration finding - #273
Merged
Merged
Conversation
🦋 Changeset detectedLatest commit: 85469db The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Contributor
Review live A/BNo reviewable delta: review.md is byte-identical in both arms (baseline |
khan-actions-bot
requested review from
a team,
jeresig and
kevinb-khan
and removed request for
a team
July 20, 2026 21:27
Contributor
Review GuidanceNo Medium/High-risk files: every changed file is a dev-only eval-corpus fixture (Low) or a doc/changeset (Trivial). Common patternsThe 62 non-spec files are synthetic fixture trees for the seven eval cases — source/test scaffolding whose only role is to give the harness a tree to present, plus the seeded ground-truth defect in the mutated file of each case. They are not production code.
Excluded from review (62 files)Not individually reviewed — synthetic fixture-tree scaffolding fully explained by the common patterns above (pattern-only):
review-lib v1.4.1 · finding-schema v2 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Note: claim validation not assessed this run (claim-validator skipped under the low-risk-tier review budget; the two non-blocking findings above were not double-checked).
Note: best-practice skills not assessed this run (skill-auditor skipped under the low-risk-tier review budget).
Note: holistic review not assessed this run (skipped under the low-risk-tier review budget).
Note: test adequacy not assessed this run (skipped under the low-risk-tier review budget).
Note: first-principles review not assessed this run (skipped under the low-risk-tier review budget).
Note: conventions review not assessed this run (skipped under the low-risk-tier review budget).
jwbron
force-pushed
the
review-eval/fable-case-calibration
branch
from
July 21, 2026 05:18
b1339c9 to
df5935b
Compare
This was referenced Jul 21, 2026
Contributor
There was a problem hiding this comment.
Note: holistic not assessed this run (holistic output unavailable).
Note: completeness not assessed this run (completeness output unavailable).
Note: test-adequacy not assessed this run (test-adequacy output unavailable).
Note: first-principles not assessed this run (first-principles output unavailable).
Note: conventions not assessed this run (conventions output unavailable).
jeresig
approved these changes
Jul 21, 2026
…n cases (removed-behavior, cross-file chain, needle-in-churn)
…n cases (v1 calibrated saturated under Opus)
… cases (partial-failure retry, check-then-act race, boundary double-count)
… cases from the fable branch (false-flag tripwire + calibration outcomes)
…s case (digest path bypasses the per-category check two hops from the diff)
…tcomes and README finding
…come and README finding
…w notes on the tripwire cases - Changeset: this PR's diff adds seven cases; nine is the cross-stack count. Say both. - mutation-crossfile-report-redaction and the largetree case: unset postedCommentCount. Live samples post companion advisories (0-3 and 2-3 comments observed across the 2026-07-20 pooled A/B), so an exact count contradicts documented live behavior; mustCatch and the verdict still gate. - largetree: record the attribution caveat the review raised. The retained send.ts ordering comment all but states the missing re-check, so the saturated calibration cannot distinguish 'scale adds no difficulty' from 'the hint gave it away'.
jwbron
force-pushed
the
review-eval/fable-case-calibration
branch
from
July 21, 2026 18:13
df5935b to
85469db
Compare
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
> 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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):
All are tagged
live(neversmoke), 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-armsruns 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 ineval/README.mdunder "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