From b0897e185ad5bad50f682c5f9bd208ce11e41a52 Mon Sep 17 00:00:00 2001 From: Jason Odoom Date: Wed, 26 Aug 2026 03:42:28 +0000 Subject: [PATCH] fix(eval): replace a re-run model's stale samples Dropping --n left the tail of a larger previous run in place, and a sample that errored then succeeded kept its .error.txt beside the new html, counting once as errored and once as scored. Only files the runner writes are removed, and only for models measured in this invocation. --- src/eval/live.ts | 29 +++++++++++++++++++++++++++- tests/eval.test.ts | 47 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/src/eval/live.ts b/src/eval/live.ts index 340d811..cba0f4c 100644 --- a/src/eval/live.ts +++ b/src/eval/live.ts @@ -1,4 +1,4 @@ -import { mkdir, writeFile, readFile, readdir } from "node:fs/promises"; +import { mkdir, writeFile, readFile, readdir, rm } from "node:fs/promises"; import { existsSync } from "node:fs"; import { join, resolve } from "node:path"; import { runnerFromSpec } from "./runners/index.js"; @@ -79,6 +79,7 @@ export async function runLiveEval(opts: LiveEvalOptions): Promise { // Keyed by canonicalId because the spec→canonical mapping is set // once per runner; populated by ModelRunnerOutput.requestId. const requestIdsByModel = new Map(); + let clearedSamples = 0; for (const spec of opts.models) { const runner = await runnerFromSpec(spec); @@ -100,6 +101,25 @@ export async function runLiveEval(opts: LiveEvalOptions): Promise { const condDir = join(modelDir, condition); await mkdir(condDir, { recursive: true }); + // Replace this cell's samples rather than writing over them. + // Without this, dropping --n leaves the tail of a larger previous + // run in place, and a sample that failed last time but succeeded + // now keeps its .error.txt beside the new .html, so the same + // sample is counted as both errored and scored. Carry-forward is + // a per-model decision: a model being re-run is being replaced. + // + // Only files this loop writes are removed. Anything else a person + // put in the directory is left alone. + const stale = (await readdir(condDir).catch(() => [])).filter((f) => + /^sample-\d+\.(html|raw\.txt|error\.txt)$/.test(f), + ); + for (const f of stale) { + await rm(join(condDir, f), { force: true }); + } + if (stale.length > 0) { + clearedSamples += stale.length; + } + const systemPrompt = condition === "compiled" ? compiled.prompts.generic : undefined; const userPrompt = [ @@ -179,6 +199,13 @@ export async function runLiveEval(opts: LiveEvalOptions): Promise { }); } + if (clearedSamples > 0) { + console.warn( + `note: removed ${clearedSamples} sample file(s) from a previous run of the ` + + `models measured now. Cells not re-run are untouched.`, + ); + } + const carriedForward = [ ...keepFromPrevious.map((m) => m.canonicalId), ...unmanifested, diff --git a/tests/eval.test.ts b/tests/eval.test.ts index aa349c9..d9d48f4 100644 --- a/tests/eval.test.ts +++ b/tests/eval.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "vitest"; -import { mkdtemp, writeFile, mkdir } from "node:fs/promises"; +import { mkdtemp, writeFile, mkdir, readdir } from "node:fs/promises"; import { join } from "node:path"; import { tmpdir } from "node:os"; import { runEval, formatEvalReport } from "../src/eval/runner.js"; @@ -208,3 +208,48 @@ describe("unmanifested sample directories", () => { expect(formatEvalReport(report)).toContain("not run in this invocation"); }); }); + +describe("stale samples from a previous run", () => { + // Re-running a model replaces its samples. Without this, dropping --n + // left the tail of a larger run in place, and a sample that errored + // last time but succeeded now kept its .error.txt beside the new + // .html, so one sample counted as both errored and scored. Models not + // re-run are untouched, which is what makes one-model-at-a-time work. + it("replaces samples for a model it re-runs and leaves others alone", async () => { + const { runLiveEval } = await import("../src/eval/live.js"); + const dir = await mkdtemp(join(tmpdir(), "ahd-stale-")); + const token = "swiss-editorial"; + const tokensDir = resolve(__dirname, "..", "tokens"); + const base = { tokensDir, token, briefPath: "briefs/landing.yml", outDir: dir }; + + await runLiveEval({ ...base, models: ["mock-swiss"], n: 3 } as never); + const rawDir = join(dir, token, "mock-swiss", "raw"); + expect((await readdir(rawDir)).filter((f) => f.endsWith(".html"))).toHaveLength(3); + + // A cell that is not re-run must survive the next invocation. + await mkdir(join(dir, token, "kept-model", "raw"), { recursive: true }); + await writeFile(join(dir, token, "kept-model", "raw", "sample-001.html"), ""); + // A stale error beside a sample that will now succeed. + await writeFile(join(rawDir, "sample-001.error.txt"), "boom"); + // Something a person left in the directory. Only files the runner + // writes may be removed, so this has to survive. + await writeFile(join(rawDir, "notes.md"), "keep me"); + + const report = await runLiveEval({ ...base, models: ["mock-swiss"], n: 1 } as never); + + const after = await readdir(rawDir); + expect(after.filter((f) => f.endsWith(".html"))).toHaveLength(1); + expect(after.filter((f) => f.endsWith(".error.txt"))).toHaveLength(0); + expect(after).toContain("notes.md"); + expect(await readdir(join(dir, token, "kept-model", "raw"))).toContain("sample-001.html"); + + // The counts the report publishes must reflect the replacement, not + // the union of two invocations. + const cell = report.cells.find( + (c) => c.model === "mock-swiss" && c.condition === "raw", + ); + expect(cell?.counts.attempted).toBe(1); + expect(cell?.counts.scored).toBe(1); + expect(cell?.counts.errored).toBe(0); + }); +});