From 0b4754c5e3fddf38ea53150dcc8df72d0afd3330 Mon Sep 17 00:00:00 2001 From: Josh Owens Date: Wed, 30 Sep 2026 20:15:50 +0000 Subject: [PATCH 1/2] fix: fail regression when proposed produces the artifact less often Pass rates exclude no-artifact runs, so a skill change that stops the agent writing the artifact could pass a regression assertion: proposed at 1/1 plus 1 no-artifact did not count as a regression from 2/2. A regression assertion now also fails when proposed's share of runs that produced the artifact is below baseline's. Co-Authored-By: Claude Opus 5.5 --- README.md | 8 ++++++-- SPEC.md | 8 ++++++-- src/cli.ts | 6 ++++-- src/engine/compare.ts | 13 +++++++++++-- test/grade-file.test.ts | 14 ++++++++++++-- 5 files changed, 39 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 9e51ee6..85c7ced 100644 --- a/README.md +++ b/README.md @@ -322,7 +322,8 @@ Useful overrides: `compare` exits non-zero when assertions fail. For target scenarios, baseline must not fully pass and proposed must improve the pass rate. For regression -scenarios, proposed must not fall below baseline. +scenarios, proposed must not fall below baseline, and must not produce the +artifact in fewer of its runs than baseline. Reading results: @@ -341,7 +342,10 @@ Reading results: pass rate and counted on their own (`1/1 pass (100%), 1 no-artifact`), with a `NOTE` naming each arm's count. An arm with no graded run at all fails a target or regression assertion, since it has no pass rate to - compare. + compare. A regression assertion also fails when proposed produces the + artifact in a smaller share of its runs than baseline: a skill change that + stops the agent writing the file is a regression even if the runs that do + write it all pass. ### Raw per-run results diff --git a/SPEC.md b/SPEC.md index a448330..893e18c 100644 --- a/SPEC.md +++ b/SPEC.md @@ -177,6 +177,8 @@ Target assertions: Regression assertions: 1. proposed must not fall below baseline pass rate +2. proposed must not produce the artifact in a smaller share of its runs than + baseline (see **No artifact**) Every case also carries a two-tailed Fisher exact p for its pass/fail table; the summary labels deltas with p > 0.05 as explainable by sampling noise. @@ -289,8 +291,10 @@ runs (`totalRuns - noArtifact`), and Fisher's p uses the graded counts. Summaries print `4/4 pass (100%), 1 no-artifact` and list each such run. `compare` adds a `NOTE` with both arms' no-artifact counts whenever either is non-zero, and fails target and regression assertions for an arm with no -graded run at all, which has no pass rate to compare. An arm that produces -the artifact less often is noted, not failed. +graded run at all, which has no pass rate to compare. A regression +assertion also fails when proposed produces the artifact in a smaller share +of its runs than baseline, since excluding those runs from the pass rate +would otherwise hide a change that stops the agent writing the artifact. LLM judges are implemented, WITH mandatory calibration, for judgments that cannot be expressed as local checks (semantic and style rubrics that regex diff --git a/src/cli.ts b/src/cli.ts index f317cf0..12ff8b6 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -675,11 +675,13 @@ function compareUsage(): string { " a run whose grader reports no artifact (exit 77) is neither a pass nor a", " failure: pass rates cover graded runs only and the summary line counts it", " (\"1/1 pass (100%), 1 no-artifact\"). An arm with no graded run fails its", - " target or regression assertion.", + " target or regression assertion, and a regression assertion fails when", + " proposed produces the artifact in fewer of its runs than baseline.", "", "assertions:", " target baseline must not fully pass; proposed must beat baseline pass rate", - " regression proposed must not fall below baseline pass rate", + " regression proposed must not fall below baseline pass rate, nor produce the", + " artifact in fewer of its runs than baseline", " compare no assertion — reports both arms' pass rates and the delta", " (for model-vs-model diffs with no directional claim)", "", diff --git a/src/engine/compare.ts b/src/engine/compare.ts index f647480..3d44cfb 100644 --- a/src/engine/compare.ts +++ b/src/engine/compare.ts @@ -729,10 +729,19 @@ function evaluateAssertions( return failures; } + const failures = []; if (proposed.passRate < baseline.passRate) { - return ["proposed regressed below baseline pass rate"]; + failures.push("proposed regressed below baseline pass rate"); } - return []; + // Pass rates exclude no-artifact runs, so a change that stops the agent + // writing the artifact would otherwise hide behind a steady pass rate. + if (gradedRuns(proposed) / proposed.totalRuns < gradedRuns(baseline) / baseline.totalRuns) { + failures.push( + `proposed produced the artifact in ${gradedRuns(proposed)}/${proposed.totalRuns} runs, ` + + `less often than baseline (${gradedRuns(baseline)}/${baseline.totalRuns})`, + ); + } + return failures; } function inferMode(evalCase: EvalCaseConfig): "text" | "artifact" { diff --git a/test/grade-file.test.ts b/test/grade-file.test.ts index 9a30f3e..203dc70 100644 --- a/test/grade-file.test.ts +++ b/test/grade-file.test.ts @@ -385,13 +385,23 @@ test("compare excludes no-artifact runs from pass rates, notes them, and refuses const summary = await runCompare({ config: regression, runners: { baseline: shared, proposed: shared } }); const caseSummary = summary.cases[0]!; expect(caseSummary.proposed).toMatchObject({ passes: 1, noArtifact: 1, passRate: 1 }); - // 1/1 is not a regression from 2/2; a no-artifact run is not a failure. - expect(caseSummary.assertions).toEqual([]); + // 1/1 against 2/2 is no pass-rate regression, but proposed wrote the + // artifact in 1 of 2 runs against baseline's 2 of 2, and that is one. + expect(caseSummary.assertions).toEqual([ + "proposed produced the artifact in 1/2 runs, less often than baseline (2/2)", + ]); const text = formatCompareSummary(summary); expect(text).toContain("proposed: 1/1 pass (100%), 1 no-artifact"); expect(text).toContain("NOTE: pass rates exclude runs with no artifact (baseline 0/2, proposed 1/2)"); expect(text).toContain("proposed run 2 no artifact:"); + // Proposed writing the artifact as often as baseline, or more often, is fine. + const flipped = await runCompare({ + config: regression, + runners: { baseline: artifactRunner([PASSING_PLAN, undefined]), proposed: artifactRunner([PASSING_PLAN, PASSING_PLAN]) }, + }); + expect(flipped.cases[0]!.assertions).toEqual([]); + // An arm that never produced the artifact has no pass rate to compare. const empty = artifactRunner([undefined]); const none = await runCompare({ config: regression, runners: { baseline: shared, proposed: empty } }); From 0088aaa2f6d13586d8aa07e95d71978f8fabc510 Mon Sep 17 00:00:00 2001 From: Josh Owens Date: Wed, 30 Sep 2026 21:39:56 +0000 Subject: [PATCH 2/2] fix: validate grader files that print while loading Scenario load parsed all of `promptdiff grade --list` stdout as JSON, so a grader file with a top-level console.log failed to load with a bare SyntaxError. Parse the last line, where the listing always lands, and report unparseable output with an excerpt. Co-Authored-By: Claude Opus 5.5 --- src/engine/grader-file.ts | 11 ++++++++++- test/grade-file.test.ts | 9 +++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/engine/grader-file.ts b/src/engine/grader-file.ts index 5fa790e..7ffa18d 100644 --- a/src/engine/grader-file.ts +++ b/src/engine/grader-file.ts @@ -141,7 +141,16 @@ export function assertGraderFileHasName(file: string, name: string, label: strin const reason = proc.stderr.toString().trim() || `exit ${proc.exitCode}`; throw new Error(`${label}.file: cannot load ${file}: ${reason}`); } - const names = JSON.parse(proc.stdout.toString()) as string[]; + // The listing is the last line: anything the grader file prints while it + // loads comes before it. + const stdout = proc.stdout.toString(); + const listing = stdout.trimEnd().split("\n").at(-1) ?? ""; + let names: string[]; + try { + names = JSON.parse(listing) as string[]; + } catch { + throw new Error(`${label}.file: cannot load ${file}: unparseable grader listing: ${JSON.stringify(stdout.slice(0, 200))}`); + } if (!names.includes(name)) { throw new Error(`${label}.name: ${file} has no grader named ${JSON.stringify(name)} (has: ${names.join(", ")})`); } diff --git a/test/grade-file.test.ts b/test/grade-file.test.ts index 203dc70..82eca7f 100644 --- a/test/grade-file.test.ts +++ b/test/grade-file.test.ts @@ -189,6 +189,15 @@ test("inherited Object.prototype names are not graders", async () => { expect(() => loadCompareConfig(path, {}, { singleArm: true })).toThrow(/has no grader named "constructor"/); }); +test("a grader file that prints while loading still validates at scenario load", () => { + const dir = join(root, "noisy"); + writeGraderFile(dir, `console.log("loading graders");\n${GRADER_SOURCE}`); + const path = writeScenario(dir, { file: "./grade.eval.ts", name: "covers-8" }); + expect(() => loadCompareConfig(path, {}, { singleArm: true })).not.toThrow(); + const missing = writeScenario(dir, { file: "./grade.eval.ts", name: "nope" }); + expect(() => loadCompareConfig(missing, {}, { singleArm: true })).toThrow(/has no grader named "nope" \(has: covers-8/); +}); + test("a file grader that hangs is killed at timeoutMs and fails as a timeout, not no-artifact", async () => { const dir = join(root, "timeout"); const file = writeGraderFile(