diff --git a/bench/README.md b/bench/README.md index a3b69ea..8389253 100644 --- a/bench/README.md +++ b/bench/README.md @@ -451,8 +451,27 @@ detection rate, false/unrelated finding count, gate-verdict correctness (does the CLI's exit code agree with the authored must-block/advisory/clean classification), mean provider cost per case, and p95 review latency. Each metric has its own tolerance (see the exported `*_MAX_*` constants at the top -of `compare-baseline.ts`), wide enough to absorb ordinary run-to-run inference -variance but not a real behavioral, cost, or latency regression. The baseline +of `compare-baseline.ts`). + +Three of those metrics block a release: detection rate, mean cost per case, and +p95 latency. False/unrelated findings and gate-verdict correctness are reported +but never block, because one run cannot measure them precisely enough to act on. +Six runs of a single unchanged binary against this corpus, four on managed +routing and two pinned to the qualified upstream provider, spanned 4 to 7 false +findings and 12.9 percentage points of gate-verdict correctness, against +thresholds of 2 findings and 2 points. Every request goes out at temperature 0, +so this is the provider's own nondeterminism rather than sampling, and pinning +the provider did not remove it: the widest false-finding count came from a +pinned run. Detection rate over the same six runs spanned 3.5 points, which +leaves a threshold that still catches a real regression. + +Read the two reported metrics across releases rather than acting on a single +run. Making them blocking again means reducing the noise rather than tightening +the number: comparing a median across repeated runs is the direct fix, at +proportionally more cost and wall-clock per release. A gate that fails at +random is worse than no gate, because it teaches everyone to bypass it. + +The baseline also records the fixture-corpus and evaluator-source SHA-256 digests the live report already computes; a mismatch fails loudly instead of comparing metrics across an unrelated fixture set. diff --git a/bench/src/compare-baseline.test.ts b/bench/src/compare-baseline.test.ts index 4cce14d..5ab4a32 100644 --- a/bench/src/compare-baseline.test.ts +++ b/bench/src/compare-baseline.test.ts @@ -7,6 +7,7 @@ import { MEAN_COST_MAX_INCREASE_RATIO, compareMetrics, extractObservedMetrics, + formatComparisonTable, percentile, type BaselineProfile, type LiveReportForComparison, @@ -169,20 +170,49 @@ describe("compareMetrics", () => { expect(comparison.rows.find((row) => row.metric === "detection rate")?.verdict).toBe("PASS"); }); - test("fails when false/unrelated findings rise past the absolute budget", () => { + test("reports false/unrelated findings past the budget without blocking", () => { + // Six runs of one unchanged binary spanned 4 to 7 false findings against a + // budget of baseline + 2, so this threshold marks a run worth reading, not + // a release worth stopping. const observed = extractObservedMetrics(fakeReport({ falsePositives: populatedBaseline.falsePositives + FALSE_FINDINGS_MAX_INCREASE + 1, })); const comparison = compareMetrics(populatedBaseline, observed); - expect(comparison.rows.find((row) => row.metric === "false/unrelated findings")?.verdict).toBe("FAIL"); + const row = comparison.rows.find((r) => r.metric === "false/unrelated findings"); + expect(row?.verdict).toBe("FAIL"); + expect(row?.informational).toBe(true); + expect(comparison.ok).toBe(true); + }); + + test("says so in the table when a reported metric leaves its usual range", () => { + const observed = extractObservedMetrics(fakeReport({ + falsePositives: populatedBaseline.falsePositives + FALSE_FINDINGS_MAX_INCREASE + 1, + })); + const table = formatComparisonTable(compareMetrics(populatedBaseline, observed).rows); + expect(table).toContain("Outside its usual range, but not blocking"); + expect(table).toContain("false/unrelated findings"); + }); + + test("a detection-rate drop still blocks the release", () => { + // The one metric whose spread across those runs (3.5pp) stayed inside a + // threshold that can still catch a real regression. + const observed = { + ...extractObservedMetrics(fakeReport()), + detectionRate: populatedBaseline.detectionRate - (DETECTION_RATE_MAX_DROP_PP / 100) - 0.01, + }; + const comparison = compareMetrics(populatedBaseline, observed); + expect(comparison.rows.find((r) => r.metric === "detection rate")?.verdict).toBe("FAIL"); expect(comparison.ok).toBe(false); }); - test("fails when gate-verdict correctness drops more than the tolerance", () => { + test("reports a gate-verdict drop past the tolerance without blocking", () => { const baseline = { ...populatedBaseline, gateVerdictCorrectness: 1 }; const observed = { ...extractObservedMetrics(fakeReport()), gateVerdictCorrectness: 1 - (GATE_VERDICT_MAX_DROP_PP / 100) - 0.01 }; const comparison = compareMetrics(baseline, observed); - expect(comparison.rows.find((row) => row.metric === "gate verdict correctness")?.verdict).toBe("FAIL"); + const row = comparison.rows.find((r) => r.metric === "gate verdict correctness"); + expect(row?.verdict).toBe("FAIL"); + expect(row?.informational).toBe(true); + expect(comparison.ok).toBe(true); }); test("fails when mean cost rises past the ratio budget", () => { diff --git a/bench/src/compare-baseline.ts b/bench/src/compare-baseline.ts index 5c76614..ed3d416 100644 --- a/bench/src/compare-baseline.ts +++ b/bench/src/compare-baseline.ts @@ -34,16 +34,52 @@ import { z } from "zod"; * before the gate fails. */ export const DETECTION_RATE_MAX_DROP_PP = 2; -/** The false/unrelated finding count may rise by at most this many findings - * above baseline before the gate fails. An absolute count, not a rate: the - * corpus size is fixed, so a rate would just rescale the same tolerance. */ +/** The false/unrelated finding count above baseline that is reported as a + * concern. Not blocking: see MEASURED_RUN_TO_RUN_SPREAD. An absolute count, + * not a rate, since the corpus size is fixed. */ export const FALSE_FINDINGS_MAX_INCREASE = 2; /** Gate-verdict correctness (does the CLI's exit code agree with the - * authored classification: block must-block, pass everything else) may drop - * at most this many percentage points below baseline. */ + * authored classification: block must-block, pass everything else) below + * baseline that is reported as a concern. Not blocking: see + * MEASURED_RUN_TO_RUN_SPREAD. */ export const GATE_VERDICT_MAX_DROP_PP = 2; +/** + * What one run of this benchmark can and cannot decide. + * + * Six runs of a single unchanged binary against this corpus, four on + * OpenRouter managed routing and two pinned to the qualified upstream + * provider, produced: + * + * detection rate 96.5% - 100.0% (3.5pp spread) + * false/unrelated findings 4 - 7 + * gate verdict correctness 71.4% - 84.3% (12.9pp spread) + * + * Every request is issued at temperature 0, so this is not sampling noise. It + * is the provider's own nondeterminism, and pinning the upstream provider did + * not remove it: the widest false-finding count came from a pinned run. + * + * A blocking threshold has to sit outside that spread or it fails releases at + * random, and a gate that fails at random gets bypassed, which is worse than + * no gate. Detection rate is the one metric whose spread is narrow enough to + * gate on from a single run, and it is also the metric that most directly + * answers "did the reviewer find the defects". The other two are real signals + * and are still reported and worth reading across releases, but a single + * sample of either cannot separate a regression from the noise floor, so they + * do not block. + * + * Narrowing them means reducing the noise, not tightening the number: + * comparing a median across repeated runs would be the direct fix, at + * proportionally more cost and wall-clock per release. + */ +export const MEASURED_RUN_TO_RUN_SPREAD = { + runs: 6, + detectionRatePp: 3.5, + falseFindingsCount: 3, + gateVerdictPp: 12.9, +} as const; + /** Mean provider cost per case may rise at most this fraction above baseline * before the gate fails. Provider pricing and routing drift more than model * behavior does, so this tolerance is wider than the detection tolerances. */ @@ -217,6 +253,8 @@ interface MetricVerdict { observed: string; verdict: "PASS" | "FAIL"; detail?: string; + /** Reported, but never blocks a release. See MEASURED_RUN_TO_RUN_SPREAD. */ + informational?: boolean; } export interface ComparisonResult { @@ -250,7 +288,8 @@ export function compareMetrics(baseline: Extract= gateFloor ? "PASS" : "FAIL", - detail: `floor ${pct(gateFloor)} (baseline - ${GATE_VERDICT_MAX_DROP_PP}pp)`, + detail: `watch below ${pct(gateFloor)}; observed spread ${MEASURED_RUN_TO_RUN_SPREAD.gateVerdictPp}pp unchanged`, + informational: true, }); const costCeiling = baseline.meanCostUsdPerCase * (1 + MEAN_COST_MAX_INCREASE_RATIO); @@ -290,7 +330,10 @@ export function compareMetrics(baseline: Extract r.verdict === "PASS"), rows }; + return { + ok: rows.every((r) => r.informational === true || r.verdict === "PASS"), + rows, + }; } export function formatComparisonTable(rows: readonly MetricVerdict[]): string { @@ -305,11 +348,24 @@ export function formatComparisonTable(rows: readonly MetricVerdict[]): string { [pad("metric", widths.metric), pad("baseline", widths.baseline), pad("observed", widths.observed), pad("verdict", widths.verdict)].join(" "), ]; for (const row of rows) { + // An informational row reports a comparison without asserting a verdict on + // the release, so it must not render a bare PASS/FAIL that reads like one. + const verdict = row.informational === true ? "report" : row.verdict; lines.push( - [pad(row.metric, widths.metric), pad(row.baseline, widths.baseline), pad(row.observed, widths.observed), pad(row.verdict, widths.verdict)].join(" ") + + [pad(row.metric, widths.metric), pad(row.baseline, widths.baseline), pad(row.observed, widths.observed), pad(verdict, widths.verdict)].join(" ") + (row.detail ? ` (${row.detail})` : ""), ); } + const noisy = rows.filter((row) => row.informational === true && row.verdict === "FAIL"); + if (noisy.length > 0) { + lines.push( + "", + `Outside its usual range, but not blocking: ${noisy.map((row) => row.metric).join(", ")}. ` + + `One run cannot separate this from provider nondeterminism (${MEASURED_RUN_TO_RUN_SPREAD.runs} runs ` + + `of one unchanged binary spanned ${MEASURED_RUN_TO_RUN_SPREAD.gateVerdictPp}pp of gate verdict ` + + `correctness). Worth reading across several releases, not acting on alone.`, + ); + } return lines.join("\n"); }