review: make the live eval a measurement tool (repeat aggregation, powered runs, drift watch)#252
review: make the live eval a measurement tool (repeat aggregation, powered runs, drift watch)#252jwbron wants to merge 13 commits into
Conversation
…powered runs, drift watch, sql-index case fix The eval instrument's percentage deltas sit below the one-case noise floor; this lands the memo's measurement items. eval/aggregate.ts pools N report artifacts into per-case pass rates with Wilson intervals and pooled rows (reproduces the 07-10 cumulative numbers exactly); live-ab.ts --repeats runs one powered dispatch with a strict-majority adversarial gate; the weekly review-eval-drift.yml workflow runs full corpus x3 on main and publishes the aggregate; identical-arm pools render noise-floor bands as data. incident-sql-missing-index diagnosed against all 28 recorded arm-runs: the reviewer found the missing index every single time; the 8/16 catch rate was the spec accepting only the migration-file anchor while the reviewer anchored the same blocking finding at the hot query (11 runs) or had it provenance- dropped on a mis-anchor (2 runs). Specs gain altLocations; the case accepts the query-site anchor and its residual misses now classify found-but-dropped (the anchor-snap defect class), not recall.
🦋 Changeset detectedLatest commit: e48bc52 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 |
Review live A/BNo reviewable delta: review.md is byte-identical in both arms (baseline |
khan-actions-bot
requested review from
a team,
kevinb-khan and
somewhatabstract
and removed request for
a team
…rbiter on the pinned Haiku snapshot Fills the MatchFallback seam in live-match.ts: when a spec stays unmatched after the deterministic pass and a posted candidate shares its file, a pinned claude-haiku-4-5-20251001 answers one yes/no question (same defect, same root cause?), biased to no since a false yes inflates recall. All seam guard rails inherited: capped per case, same-file only, matches recorded via 'fallback' for audit. API failures degrade to non-matches through onError; both arms share the matcher so the A/B delta stays unbiased. On by default in live-ab.ts; --no-match-arbiter restores deterministic-only matching. This targets the 54-69% 'unmatched posted' readings, implausibly high as true noise.
…nderers into live-ab-report.ts CI's max-lines (1000) caught live-ab.ts at 1011 after the repeats and arbiter work; this worktree cannot run eslint locally (dot-dir ignore). The report types (ArmRunReport, AbReport, MultiAbReport, gate types) and both markdown renderers move to a leaf module with no runner dependency; live-ab.ts re-exports them so the import surface is unchanged.
…t after every repeat A multi-repeat run carries tens of dollars of spend but wrote its artifact only at the end, so a crash or cancellation on repeat n forfeited repeats 1..n-1 (the exact dies-with-nothing-emitted failure mode the plan forbids; the workflow's always() artifact upload had nothing to grab). Each completed repeat now overwrites the out path with the accumulated partial payload, which aggregate.ts already pools via its repeats-field handling.
…or in the report footer Bought per the memo's item 6: run 29069228968 (--force-arms --repeats 3, full 14-case corpus, $58.71 under the $60 clamp) gives 6 arm-samples of one review.md. Measured bands: must-catch recall 54-86%, verdict agreement 75-100%, noise 50-60%, judge quality 0.82-0.86. Every single-run report now renders these as data with provenance, replacing prose guesswork; a delta whose arms both sit inside a band is wobble, and the footer points at --repeats for smaller effects. Pre-arbiter measurement; the weekly drift run re-measures with the arbiter active.
…budget to $85 The noise-floor dispatch showed the live corpus is now 14 cases (the #251 golden re-review set), and the $60 default trimmed case-runs on both arms (38 and 36 of 42; $58.71 spent at the clamp). $85 covers 6 full arm-runs at the measured ~$10/arm-run with landing headroom, so the weekly aggregate never carries budget-skip asymmetry.
|
Post-review additions on this branch since the opening description:
Not landed, deliberately: baseline-arm reuse. Actions cache entries are unreadable across sibling/child PR runs, which is exactly the stacked topology carrying the 44% duplicate spend; the memo (rev 3) records the analysis and three candidate alternatives. |
…, and noise floor in the changeset
…port opens a visibility PR workflows/review/eval/README.md is the guide an agent or engineer needs to run the eval system cold: the three tiers, every live-ab flag and CI entry point, powered-run recipes with real costs, corpus-growth rules (target the 20-80% band), report-reading guidance anchored on the measured noise floor, model pins, and the harness's historical limits (nothing before the 2026-07-08 structured-agent architecture runs under it). The weekly drift run now also opens a PR committing the report markdown and compact aggregate under .github/review-eval/drift/ (changeset-gate-excluded, outside the A/B path filter): a job summary nobody opens is not a drift watch, and merged report PRs accumulate an in-repo time series.
…rom the main README
Main vs stack tip: the cumulative measurement, and how to read itPooled from the three 2026-07-10 full-corpus dispatches (29065968954 / 29065976210 / 29065977160; baseline
Verdict: recall-neutral, which is the designed outcome. Batch two is process and output-quality work (accountability, rendering, re-review machinery); the A/B's job for that class of change is to prove nothing silently regressed across a heavy prompt rework, and it did: the candidate arm has zero true misses. All movement sits inside the measured noise floor (recall wobbles 54-86% on byte-identical arms; see the report footer this PR adds). What the run actually taught us:
Reading future reports: the next drift runs will show recall and noise 'improving' because this PR fixed the ruler (altLocations) and de-noises matching (arbiter), not because the reviewer changed. Compare like with like via the review.md shas and the noise-floor provenance in each report. |
…-floor statistics, rolling repeat budgets Findings from an adversarial pass over the instrument's own claims: Reports now stamp their ruler (matcher configuration + a content hash of the loaded corpus); the aggregate prints it, warns when a pool mixes rulers, and the drift series stays interpretable across instrument upgrades like the arbiter default or corpus growth, which move every rate without the reviewer changing. Noise-floor bands gain SD (min/max are extreme-value statistics that only widen as samples accumulate; mean +/- sd is the stable band) and a loud case-asymmetry warning: the 2026-07-10 measurement had budget skips, so its published v1 bands fold case-mix variance in on top of wobble; the footer constants and provenance string now say so. Repeat budgets roll: each arm-run's slice is the remaining budget over the remaining arm-runs, so cheap early arms donate headroom forward instead of stranding it (fixed slices caused the 38/36-of-42 skip asymmetry that contaminated the noise-floor run). README gains a statistical-honesty section: clustered-interval optimism, the repeats-gate relaxation, and the arbiter's uncalibrated refuse bias.
|
One more commit (2369b86) from an adversarial pass over the instrument's own claims:
Known-open items live in the tuning memo rather than this stack: arbiter false-positive calibration, the gate-policy decision, a post-merge validation dispatch of the drift PR flow, and a clean re-measurement of the noise floor at the $85 budget. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| * prose. The weekly drift run re-measures these bands (with the arbiter | ||
| * active); update the constants when they move materially. | ||
| */ | ||
| export const MEASURED_NOISE_FLOOR = { |
There was a problem hiding this comment.
thought (non-blocking): MEASURED_NOISE_FLOOR's provenance says "pre-arbiter", but this PR makes the Haiku match arbiter the default matcher, so every single-run report footer prices arbiter-era deltas against pre-arbiter wobble bands until someone hand-edits the constant. Consider warning when the renderer's provenance.matcher differs from the noise floor's ruler, or sourcing the bands from the latest committed drift aggregate.
| # the memo's answer: the FULL live corpus, 3 repeats per arm, both arms | ||
| # pinned to main's review.md (`--force-arms` past the identity | ||
| # short-circuit), aggregated into per-case pass rates with binomial | ||
| # intervals. Week-over-week comparison happens across artifacts; within one |
There was a problem hiding this comment.
suggestion (non-blocking): The drift watch emits weekly snapshots but nothing computes the week-over-week comparison — the header notes it "happens across artifacts" (i.e. by a human), so a slow recall regression on main would sail under the within-run adversarial gate. aggregate.ts already accepts run ids, so feeding the current run plus the last N committed drift JSONs through it would render prior-week bands next to this week's and make the watch self-comparing.
| STAMP="$(date -u +%Y-%m-%d)" | ||
| DIR=".github/review-eval/drift" | ||
| BRANCH="drift-report/${STAMP}-${{ github.run_id }}" | ||
| mkdir -p "$DIR" |
There was a problem hiding this comment.
note (non-blocking): This workflow grants contents: write + pull-requests: write on a schedule trigger and opens a PR on every run (if: always()). The interpolations are all repo-controlled, so there is no injection surface — flagging only for visibility that the write-scope and the per-run branch/PR accumulation are intended.
| // Fallback: only candidates sharing a spec file, in posted order. | ||
| for (const candidate of posted) { | ||
| if (claimed.has(candidate.id) || candidate.path !== spec.path) { | ||
| if (claimed.has(candidate.id) || !onSpecFile(candidate, spec)) { |
There was a problem hiding this comment.
suggestion (non-blocking): onSpecFile's new altLocations awareness (this fallback gate and the drop classifier at line 254) is only exercised via anchorAgrees; every matchCase test uses a candidate on the primary spec path. A spec whose real finding lands on the alt file only would be scored a true miss rather than a fallback/found-but-dropped catch, and that path is unverified. Consider a matchCase test with an alt-file candidate.
…array, not the scalar count
The per-repeat checkpoint wrote {repeats: <n>} where the recovery path
in aggregate.ts expects {repeats: [reports]}; a crash mid-run therefore
forfeited every completed repeat, which is exactly what the checkpoint
exists to prevent. Write the reports array, matching the final artifact
shape, and lock the checkpoint-to-extractSamples contract with a test.
This comment has been minimized.
This comment has been minimized.
| let armRunsDone = 0; | ||
| let spentUsd = 0; | ||
| const nextArmBudget = (): number => | ||
| Math.max(0, maxUsd - spentUsd) / (totalArmRuns - armRunsDone); |
There was a problem hiding this comment.
note (non-blocking): Judge and arbiter spend are excluded from --max-usd and scale with --repeats; gate-retry spend is also untracked. The rolling budget only accounts for arm-producer cost (spentUsd += report.usd), and nextArmBudget divides only the remaining maxUsd - spentUsd. Judge cost (liveJudgeModel, one paid call per finding per arm) and the Haiku arbiter cost (haikuMatchArbiter) never flow into spentUsd, so --repeats n multiplies that untracked spend across 2*n arm-runs. Gate-retry cost is likewise untracked (though it runs only on the single-run path). Consider folding these into spentUsd, or documenting that --max-usd bounds arm-producer spend only.
…; pin github-script to v8 Two review nitpicks from the stack: live-match.ts imported two types from ./corpus/loader in separate statements where the rest of the eval dir merges them, and the sticky-comment step pinned github-script v7 while node-ci.yml and release.yml pin v8 (ed597411). Align both.
Review Guidancegithub-actions (7 files)
Common patterns4 files: altLocations support — LiveDefectSpec gains an altLocations field (type + parser in live.ts, matcher in live-match.ts), validated in loader.test.ts and used in the smoke case fixture. 2 files: Report types and renderers extracted from live-ab.ts into the new live-ab-report.ts leaf module; live-ab.ts re-exports them to preserve the import surface. 2 files: One-line re-export propagation — LiveSpecLocation added to the loader barrel and the altLocations field added to the smoke case.json. Excluded from review (6 files)Not individually reviewed — generated, formatting-only, or fully explained by a common pattern above:
|
| - name: Post the sticky PR comment | ||
| if: always() && github.event_name == 'pull_request' | ||
| uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7 | ||
| uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 |
There was a problem hiding this comment.
note (non-blocking): This v8 SHA matches the pin already used by node-ci.yml and release.yml, so the alignment is correct. Heads up that .github/workflows/review-rereview-sweep.yml:79 is now the last workflow still on the v7 SHA — a trivial follow-up would bring the repo to a single pinned version.
1 participant
Lands the tuning memo's rev-2 measurement items (memo: "Dialing in the evals", rev 2). The per-PR A/B stays a tripwire; this adds the machinery that turns repeats into resolution.
What's here
Repeat-aggregation report (
eval/aggregate.ts, memo item 3). Deterministic core + thin CLI: pools Nlive-ab-report.jsonartifacts (local paths orghrun ids) into per-case pass rates per arm with 95% Wilson intervals, pooled recall/verdict/noise rows, and true-miss vs found-but-dropped classification per gate bucket. Warns when a pool mixes review.md shas. Validated against the three 07-10 cumulative dispatches (29065968954 / 29065976210 / 29065977160): reproduces the memo's hand-computed numbers exactly (baseline 21/24 specs, 28/30 verdicts; candidate 22/24, 28/30).--repeats <n>inlive-ab.ts(memo sizing section). One dispatch runs the selected cases n times per arm (budget split per arm-run, fresh staging per repeat) and reports through the aggregation core instead of single-run percentages; this is the ~$29 targeted-repeats powered run (--cases <two anchor-fragile cases> --repeats 10). The adversarial gate is decided by strict majority across repeats; the single-run best-of-three retry is skipped since the repeat structure buys the same flake evidence. The A/B workflow exposescases/repeats/force_armsdispatch inputs.Scheduled drift watch (
review-eval-drift.yml). Weekly cron: full live corpus x3 repeats, both arms pinned to main's review.md (--force-armspast the identity short-circuit), aggregate published to job summary + artifact. Identical arms mean 6 arm-samples of the same prompt per run, so every drift run also refreshes the noise-floor bands, which the aggregate renders as data (memo item 6).incident-sql-missing-index diagnosed and fixed (memo item 2). Read every recorded transcript in the wave + cumulative artifacts (28 arm-runs): the reviewer found the missing index in 28/28. The 8/16 catch rate was the spec accepting only the migration-file anchor while the reviewer anchored the same blocking finding at the hot query in
src/models/order.ts(11 "misses") or had it provenance-dropped on an out-of-file mis-anchor (2, the adversarial-injection-approve pathology). Neither mechanism alternates nor lens routing were at fault; the case measured anchor-site preference. Specs gainaltLocations(validated like the primary location; matcher, fallback filter, and drop classification all honor them) and the case accepts the query-site window. Replayed against all 28 recorded runs: 26 caught, 2 found-but-dropped, 0 true misses; the residual failure class is exactly the anchor-snap production defect, now correctly bucketed.Notes
pnpm test --run(879 tests) andpnpm typecheckpass; eslint can't run in this worktree (dot-dir ignore), prettier applied.