diff --git a/.changeset/review-eval-case-selection.md b/.changeset/review-eval-case-selection.md new file mode 100644 index 00000000..ee272d1c --- /dev/null +++ b/.changeset/review-eval-case-selection.md @@ -0,0 +1,5 @@ +--- +"review": patch +--- + +Fail-fast case selection in the live A/B runner. An explicit `--cases` list is now an exact selection: it bypasses `--smoke-only` (which scopes unscoped runs only), preserves the requested order, runs duplicate ids once, and throws before any model spend when an id matches no live case. Previously the smoke scope filtered the corpus before the case filter, so a dispatch naming a non-smoke case silently dropped it: the 2026-07-10 anchor-snap powered run named two cases and the paid report covered one without saying so, and a typo'd case id would shrink a measurement the same way. The workflow needs no change; it already passes both flags and the case list now wins. diff --git a/workflows/review/eval/README.md b/workflows/review/eval/README.md index 26c17276..71deeabe 100644 --- a/workflows/review/eval/README.md +++ b/workflows/review/eval/README.md @@ -28,8 +28,8 @@ pnpm test --run workflows/review/eval/ ```sh pnpm dlx tsx workflows/review/eval/live-ab.ts \ [--base-ref ] # baseline review.md source (default: merge-base with origin/main) - [--cases ] # subset of live cases - [--smoke-only] # only live cases also tagged smoke (the per-PR default) + [--cases ] # EXACT selection: bypasses --smoke-only; unknown ids fail before spend + [--smoke-only] # only live cases also tagged smoke (the per-PR default; ignored under --cases) [--repeats ] # n runs per arm in one invocation; pooled pass-rate report [--force-arms] # run identical arms anyway (wobble control / noise floor) [--max-usd ] # hard budget across all arm-runs (default 40) diff --git a/workflows/review/eval/live-ab.test.ts b/workflows/review/eval/live-ab.test.ts index a8a1f21d..8a8187ca 100644 --- a/workflows/review/eval/live-ab.test.ts +++ b/workflows/review/eval/live-ab.test.ts @@ -10,6 +10,7 @@ import { renderMultiMarkdownReport, retryGateFlips, runArm, + selectCases, type AbReport, type ArmProduce, type ArmRunReport, @@ -125,6 +126,52 @@ const produceMiss: ArmProduce = async () => ({ ], }); +describe("selectCases", () => { + const corpus = () => [ + liveCase("smoke-a", {tags: ["live", "smoke"]}), + liveCase("smoke-b", {tags: ["live", "smoke"]}), + liveCase("holdout-c", {tags: ["live", "holdout"]}), + ]; + + it("scopes unscoped runs by the smoke tag, or not at all", () => { + expect( + selectCases(corpus(), {smokeOnly: true}).map((c) => c.id), + ).toEqual(["smoke-a", "smoke-b"]); + expect( + selectCases(corpus(), {smokeOnly: false}).map((c) => c.id), + ).toEqual(["smoke-a", "smoke-b", "holdout-c"]); + }); + + it("treats an explicit case list as exact: it bypasses the smoke scope", () => { + // The 2026-07-10 footgun: a powered dispatch named a non-smoke case + // and the smoke scope silently dropped it from a paid measurement. + expect( + selectCases(corpus(), { + smokeOnly: true, + caseFilter: ["smoke-a", "holdout-c"], + }).map((c) => c.id), + ).toEqual(["smoke-a", "holdout-c"]); + }); + + it("preserves requested order and runs duplicate ids once", () => { + expect( + selectCases(corpus(), { + smokeOnly: false, + caseFilter: ["holdout-c", "smoke-a", "holdout-c"], + }).map((c) => c.id), + ).toEqual(["holdout-c", "smoke-a"]); + }); + + it("fails before any spend on an id that matches no live case", () => { + expect(() => + selectCases(corpus(), { + smokeOnly: false, + caseFilter: ["smoke-a", "not-a-case"], + }), + ).toThrow(/not in the live corpus: not-a-case/); + }); +}); + describe("runArm", () => { it("scores cases, accounts cost, and reports agent failures", async () => { const report = await runArm( diff --git a/workflows/review/eval/live-ab.ts b/workflows/review/eval/live-ab.ts index dfaaa0c2..0ba66a19 100644 --- a/workflows/review/eval/live-ab.ts +++ b/workflows/review/eval/live-ab.ts @@ -18,9 +18,13 @@ * pnpm dlx tsx workflows/review/eval/live-ab.ts * [--base-ref ] baseline review.md source (default: merge-base * of HEAD and origin/main) - * [--cases ] subset of live cases (default: every live case) + * [--cases ] EXACT selection of live cases: bypasses + * --smoke-only, and an id matching no live case + * fails the run before any model spend * [--smoke-only] only live cases also tagged smoke (the per-PR - * default in CI; a full-eval label lifts it) + * default in CI; a full-eval label lifts it); + * scopes unscoped runs only — ignored when + * --cases names the selection * [--max-usd ] total hard budget across both arms (default 40) * [--no-judge] skip judge quality scoring * [--no-match-arbiter] deterministic spec matching only (skip the @@ -107,6 +111,50 @@ export type { MultiAbReport, } from "./live-ab-report"; +/* -------------------------------------------------------------------------- */ +/* Case selection */ +/* -------------------------------------------------------------------------- */ + +/** + * Select the cases a run scores. An explicit case list is an EXACT + * selection: it bypasses the smoke scope (naming a non-smoke case in + * `--cases` selects it; the smoke tag scopes only unscoped runs) and throws + * on any id that matches no live case. A typo'd or non-live id must fail + * the dispatch BEFORE any model spend — the alternative already happened: + * the 2026-07-10 anchor-snap powered run named two cases, the smoke scope + * silently dropped the non-smoke one, and the paid report covered half the + * measurement without saying so. Requested order is preserved; duplicate + * ids run once. + */ +export const selectCases = ( + allLive: CorpusCase[], + options: {smokeOnly: boolean; caseFilter?: string[]}, +): CorpusCase[] => { + const {caseFilter} = options; + if (caseFilter !== undefined) { + const byId = new Map(allLive.map((c) => [c.id, c])); + const unknown = caseFilter.filter((id) => !byId.has(id)); + if (unknown.length > 0) { + throw new Error( + `--cases: not in the live corpus: ${unknown.join(", ")} ` + + `(live case ids: ${allLive.map((c) => c.id).join(", ")})`, + ); + } + const seen = new Set(); + return caseFilter.flatMap((id) => { + if (seen.has(id)) { + return []; + } + seen.add(id); + const found = byId.get(id); + return found === undefined ? [] : [found]; + }); + } + return options.smokeOnly + ? allLive.filter((c) => c.tags.includes(SMOKE_TAG)) + : [...allLive]; +}; + /* -------------------------------------------------------------------------- */ /* One arm */ /* -------------------------------------------------------------------------- */ @@ -408,7 +456,10 @@ const main = async (): Promise => { const outPath = argValue("--out") ?? "out/live-ab-report.json"; const stageRoot = argValue("--stage-root") ?? mkdtempSync(`${tmpdir()}/review-ab-`); - const caseFilter = argValue("--cases")?.split(","); + const caseFilter = argValue("--cases") + ?.split(",") + .map((id) => id.trim()) + .filter((id) => id !== ""); const withJudge = !process.argv.includes("--no-judge"); const reviewMdPath = "workflows/review/review.md"; @@ -452,15 +503,10 @@ const main = async (): Promise => { return; } - const allCases = loadLiveCorpus().filter( - (c) => - !process.argv.includes("--smoke-only") || - c.tags.includes(SMOKE_TAG), - ); - const cases = - caseFilter === undefined - ? allCases - : allCases.filter((c) => caseFilter.includes(c.id)); + const cases = selectCases(loadLiveCorpus(), { + smokeOnly: process.argv.includes("--smoke-only"), + ...(caseFilter !== undefined ? {caseFilter} : {}), + }); if (cases.length === 0) { throw new Error("no live cases selected"); }