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); + }); +});