diff --git a/evalboard/README.md b/evalboard/README.md index 20dcefab..fde55a10 100644 --- a/evalboard/README.md +++ b/evalboard/README.md @@ -57,7 +57,53 @@ show up in the index — empty shells and the `latest` symlink are filtered out. `` is the same string the eval framework writes to `task_results[].task_id` (e.g., `skill-flow-calculator`) and equals the -subdir name under `/default/`. +subdir name under `//`. + +## Variants (A/B runs) + +A coder_eval experiment that declares `variants:` runs every task once per arm +and writes each arm to its own subtree (`////`), +stamping `variant_id` on every `run.json` row. An experiment with no `variants:` +still writes one arm, named `default` — which is why a single-arm run and a +multi-arm run have the same shape on disk. + +Evalboard reads that directly: + +- The run page's **Pass rate tile reports one entry per arm** (side by side, so + the tile keeps the height of the single-arm version and the tiles beside it are + not stretched), and reports no pooled rate at all. A blended rate would average configurations that were + deliberately made to differ (and would move when the arms are merely + reordered), so on a variant run it is not a number anyone wants. The tile + states the observed `spread` between arms and nothing more. +- **Total cost and Time keep their pooled totals** and carry the per-arm split on + their sub-line, in place of p50/p90. A run's spend is a real operational + number however many arms produced it, in a way a run's pass rate is not. +- The task grid gains a **Variant** column and keeps one row per (task, arm). + Replicates of one arm still collapse into a single row; **arms never collapse + into each other** — that difference is the measurement. The default ordering + ranks a *task* by its worst arm rather than ranking each row on its own, so + failures still sort to the top while a task's arms stay adjacent. Ranking rows + independently splits exactly the pairs worth reading: a task whose arms + disagree ends up with one row at the top of the grid and the other at the + bottom. +- Task detail is addressed by `?v=` alongside `?r=`, so + each arm opens its own transcript, log, criteria and artifacts. A link that + names no arm resolves to the task's first — `default` on an ordinary run, the + leading arm on an experiment run, whose arms are all named and which therefore + has no `default` subtree for a bare link to land in. A link naming an arm the + run does not have is a 404, not a fallback: the point of addressing an arm is + that you get that arm or nothing. + +Every one of those is inert on a run without variants: the column is dropped, the +tiles keep their existing pooled numbers and percentiles, and no link carries +`?v=`. Cross-run comparison (two separate run ids) is a different feature and is +not what this does. + +Statistics are deliberately not computed here. Whether a gap between arms is +real is a question about variance, and coder_eval already answers it in the +experiment report it writes beside `run.json` (`experiment.md` — win rates, +per-task comparison, most divergent tasks, paired comparison with Welch t-test +and bootstrap CIs). ## Sources @@ -104,9 +150,10 @@ Two invariants worth preserving if you add a source: - `/api/file?run=&path=[&src=]` serves `.flow`, `.uipx`, etc. with path-traversal guard (`resolveSafePath`). -- `/api/download?run=[&task=][&src=]` streams a zip of a task - folder (with `task`) or the whole run (without). Files are gathered by `collectTaskFiles` - / `collectRunFiles`, which reuse the `walkArtifacts` noise filter, and zipped - by `lib/zip.ts` (a dependency-free DEFLATE writer). +- `/api/download?run=[&task=][&v=][&src=]` streams a zip + of a task folder (with `task`, from the named arm) or the whole run (without). + Files are gathered by `collectTaskFiles` / `collectRunFiles`, which reuse the + `walkArtifacts` noise filter, and zipped by `lib/zip.ts` (a dependency-free + DEFLATE writer). - Pass rows render green (`bg-green-50 text-green-700`), failures render red (`bg-red-50 text-red-700`), on a white background. diff --git a/evalboard/app/api/download/route.ts b/evalboard/app/api/download/route.ts index 2ef88d42..5aa7a509 100644 --- a/evalboard/app/api/download/route.ts +++ b/evalboard/app/api/download/route.ts @@ -1,14 +1,15 @@ import { promises as fs } from "node:fs"; import { NextResponse } from "next/server"; import { collectRunFiles, collectTaskFiles } from "@/lib/runs"; +import { DEFAULT_VARIANT_ID, isValidVariantId } from "@/lib/variants"; import { sourceById } from "@/lib/sources"; import { createZip, type ZipEntry } from "@/lib/zip"; export const dynamic = "force-dynamic"; // Bundle a task folder, or a whole run, into a zip download. -// ?run=&task= → just that task's folder (default//) -// ?run= → the entire run folder (run.json + every task dir) +// ?run=&task=[&v=] → that task's folder (//) +// ?run= → the entire run folder (run.json + every task dir) // minus the usual scaffolding noise, from the container named by ?src (the // skills nightly when absent). In blob mode the collect* helpers fetch the // needed blobs first, so this mirrors what the page would load. @@ -16,21 +17,30 @@ export async function GET(req: Request) { const url = new URL(req.url); const runId = url.searchParams.get("run"); const taskId = url.searchParams.get("task"); + // Experiment arm. Absent / malformed → the single arm a non-experiment run + // writes, so pre-variant download URLs resolve unchanged. + const v = url.searchParams.get("v"); + const variantId = isValidVariantId(v) ? v : DEFAULT_VARIANT_ID; const source = sourceById(url.searchParams.get("src")); if (!runId) { return new NextResponse("missing run", { status: 400 }); } const files = taskId - ? await collectTaskFiles(runId, taskId, source) + ? await collectTaskFiles(runId, taskId, source, variantId) : await collectRunFiles(runId, source); if (!files) { return new NextResponse("not found", { status: 404 }); } // Top-level folder inside the archive: the task id for a task download, - // the run id for a whole-run download. - const root = taskId ?? runId; + // the run id for a whole-run download. A non-default arm is named too, so + // two arms of the same task don't produce two identically-named zips. + const root = taskId + ? variantId === DEFAULT_VARIANT_ID + ? taskId + : `${taskId}__${variantId}` + : runId; const entries: ZipEntry[] = []; for (const f of files) { const data = await fs.readFile(f.abs).catch(() => null); diff --git a/evalboard/app/runs/[id]/[...task]/page.tsx b/evalboard/app/runs/[id]/[...task]/page.tsx index 0bf02fa2..36e2cb03 100644 --- a/evalboard/app/runs/[id]/[...task]/page.tsx +++ b/evalboard/app/runs/[id]/[...task]/page.tsx @@ -7,7 +7,9 @@ import { readTaskDetail, readTaskReplicates, replicateDirName, + resolveVariantId, } from "@/lib/runs"; +import { DEFAULT_VARIANT_ID, isValidVariantId } from "@/lib/variants"; import { readTaskReview } from "@/lib/reviews"; import { sourceById } from "@/lib/sources"; import { scalarParam, withSource } from "@/app/_lib/source-param"; @@ -37,10 +39,10 @@ export default async function TaskPage({ searchParams, }: { params: Promise<{ id: string; task: string[] }>; - searchParams: Promise<{ r?: string; src?: string | string[] }>; + searchParams: Promise<{ r?: string; v?: string; src?: string | string[] }>; }) { const { id, task: taskSegments } = await params; - const { r, src } = await searchParams; + const { r, v, src } = await searchParams; // Which container this run lives in. Unknown/absent coerces to the skills // nightly, so every URL that predates the Scribe tab keeps resolving as-is. const source = sourceById(scalarParam(src)); @@ -52,28 +54,63 @@ export default async function TaskPage({ const parsedR = Number(r); const replicate = r != null && Number.isInteger(parsedR) && parsedR >= 0 ? parsedR : 0; - const task = await readTaskDetail(id, taskId, replicate, source); + // Experiment arm from ?v=NAME. A run with variants stores each arm's content + // under its own // subtree and repeats the task id once per + // arm, so without this both arms would resolve to the same transcript. An + // absent or malformed value names no arm, and resolveVariantId picks the + // task's first — `default` on an ordinary run, so every pre-variant URL + // resolves exactly as it did, and the leading arm on an experiment run, + // whose arms are named and would otherwise leave a bare link with nothing + // to match. + const variantId = await resolveVariantId( + id, + taskId, + isValidVariantId(v) ? v : null, + source, + ); + const task = await readTaskDetail(id, taskId, replicate, source, variantId); if (!task) notFound(); // Replicate indices available for this task — drives the run selector below. // [0] (or fewer) for a non-repeated task, so the selector self-hides. - const replicates = await readTaskReplicates(id, taskId, source); + const replicates = await readTaskReplicates(id, taskId, source, variantId); - // variant is always "default" here; the replicate selects the / dir. // readTaskReview returns null for older runs that predate the review feature. const review = await readTaskReview( id, - "default", + variantId, taskId, replicateDirName(replicate), source, ); - const log = await readLogTail(id, taskId, replicate, undefined, source); + const log = await readLogTail( + id, + taskId, + replicate, + undefined, + source, + variantId, + ); const conversation = parseConversation( - await readConversationLog(id, taskId, replicate, undefined, source), + await readConversationLog( + id, + taskId, + replicate, + undefined, + source, + variantId, + ), ); const { flowDebug } = task; + // Only a multi-variant run names its arm in a URL. Omitting the param on a + // default-arm run keeps every link byte-identical to what it was before + // variants were addressable. + const variantQuery = + variantId === DEFAULT_VARIANT_ID + ? "" + : `&v=${encodeURIComponent(variantId)}`; + const showVariant = variantId !== DEFAULT_VARIANT_ID; return (
@@ -86,6 +123,14 @@ export default async function TaskPage({ / {taskId} + {showVariant && ( + <> + / + + {variantId} + + + )} {replicates.length > 1 && ( <> / @@ -112,7 +157,7 @@ export default async function TaskPage({ {taskId} · run {id} + {showVariant && ` · variant ${variantId}`} {replicates.length > 1 && ` · replicate ${replicate}`} {/* Component SHAs point at internal tooling; internal-only. diff --git a/evalboard/app/runs/[id]/__tests__/run-view.render.test.tsx b/evalboard/app/runs/[id]/__tests__/run-view.render.test.tsx index 4b12f178..4b4ed252 100644 --- a/evalboard/app/runs/[id]/__tests__/run-view.render.test.tsx +++ b/evalboard/app/runs/[id]/__tests__/run-view.render.test.tsx @@ -18,6 +18,7 @@ function row( ): TaskResultSummary { return { taskId, + variantId: null, replicateIndex: null, status: "SUCCESS", weightedScore: 1.0, @@ -83,3 +84,55 @@ describe("RunView — Pass-rate / Failed tiles for repeated runs", () => { expect(screen.queryByText(/replicate runs/)).toBeNull(); }); }); + +describe("RunView — multi-variant runs replace the pooled pass rate", () => { + // Arm A passes everything, arm B fails everything. The pooled rate would be + // 50%, which describes neither configuration; the whole point of the tile + // change is that this number no longer appears anywhere on the page. + const AB = [ + row("X", { variantId: "A", status: "SUCCESS", totalCostUsd: 0.1, durationSeconds: 1 }), + row("Y", { variantId: "A", status: "SUCCESS", totalCostUsd: 0.1, durationSeconds: 1 }), + row("X", { variantId: "B", status: "FAILURE", totalCostUsd: 0.3, durationSeconds: 5 }), + row("Y", { variantId: "B", status: "FAILURE", totalCostUsd: 0.3, durationSeconds: 5 }), + ]; + + test("each arm gets its own rate and the blended rate is gone", () => { + render(); + + expect(screen.getByText("100%")).toBeInTheDocument(); + expect(screen.getByText("0%")).toBeInTheDocument(); + // The blended 50% must not be rendered at all. + expect(screen.queryByText("50%")).toBeNull(); + expect(screen.getByText(/2 arms · spread 100 pts/)).toBeInTheDocument(); + }); + + test("spend and elapsed time keep their pooled total, split by arm on the sub-line", () => { + render(); + + // Pooled totals stay: a run's cost is real however many arms produced it. + expect(screen.getByText("$0.80")).toBeInTheDocument(); + expect(screen.getByText("12s")).toBeInTheDocument(); + // ...with the per-arm split replacing p50/p90, which would describe a + // pooled population that does not exist. + expect(screen.getByText("A $0.20 · B $0.60")).toBeInTheDocument(); + expect(screen.getByText("A 2s · B 10s")).toBeInTheDocument(); + expect(screen.queryByText(/p50/)).toBeNull(); + }); + + test("a single-arm run is untouched: pooled rate and p50/p90 as before", () => { + render( + , + ); + expect(screen.getByText("50%")).toBeInTheDocument(); + expect(screen.queryByText(/arms · spread/)).toBeNull(); + // Both the cost and time tiles carry one, hence getAll. + expect(screen.getAllByText(/p50/).length).toBe(2); + }); +}); diff --git a/evalboard/app/runs/[id]/__tests__/run-view.test.ts b/evalboard/app/runs/[id]/__tests__/run-view.test.ts index 22a266bd..3b1c45ae 100644 --- a/evalboard/app/runs/[id]/__tests__/run-view.test.ts +++ b/evalboard/app/runs/[id]/__tests__/run-view.test.ts @@ -1,6 +1,10 @@ import { describe, expect, test } from "vitest"; import type { TaskResultSummary } from "@/lib/runs"; -import { computeRunMetrics } from "../run-view"; +import { + computeRunMetrics, + computeVariantMetrics, + variantSub, +} from "../run-view"; function row( taskId: string, @@ -8,6 +12,7 @@ function row( ): TaskResultSummary { return { taskId, + variantId: null, replicateIndex: null, status: "SUCCESS", weightedScore: 1.0, @@ -114,3 +119,82 @@ describe("computeRunMetrics — per-task pass rate across replicates", () => { expect(m.taskFailed).toBe(m.failedTotal); }); }); + +describe("computeVariantMetrics", () => { + // Every ordinary run: nothing to compare, so the strip stays hidden and the + // headline tiles are the whole story, exactly as before. + test("a run with fewer than two arms reports nothing", () => { + expect(computeVariantMetrics([row("A"), row("B")])).toEqual([]); + expect( + computeVariantMetrics([ + row("A", { variantId: "only" }), + row("B", { variantId: "only" }), + ]), + ).toEqual([]); + }); + + // The whole point: a blended 50% would hide that one arm passed everything + // and the other failed everything. + test("each arm is scored on its own rows", () => { + const rows = computeVariantMetrics([ + row("A", { variantId: "live-v1", status: "SUCCESS" }), + row("B", { variantId: "live-v1", status: "SUCCESS" }), + row("A", { variantId: "preview-v2", status: "FAILURE" }), + row("B", { variantId: "preview-v2", status: "FAILURE" }), + ]); + expect(rows.map((r) => r.variantId)).toEqual(["live-v1", "preview-v2"]); + expect(rows[0].metrics.pct).toBe(100); + expect(rows[1].metrics.pct).toBe(0); + expect(rows[0].metrics.taskTotal).toBe(2); + expect(rows[1].metrics.taskTotal).toBe(2); + }); + + // Cost and duration must not be pooled either — an arm that is cheaper is a + // finding, and pooling would erase it. + test("cost and duration are per arm, not pooled", () => { + const rows = computeVariantMetrics([ + row("A", { variantId: "a", totalCostUsd: 1, durationSeconds: 10 }), + row("A", { variantId: "b", totalCostUsd: 3, durationSeconds: 30 }), + ]); + expect(rows[0].metrics.cost).toBeCloseTo(1, 10); + expect(rows[1].metrics.cost).toBeCloseTo(3, 10); + expect(rows[0].metrics.duration).toBeCloseTo(10, 10); + expect(rows[1].metrics.duration).toBeCloseTo(30, 10); + }); + + // Rows with no variant_id belong to the default arm, so a run that mixes + // stamped and unstamped rows still resolves to two arms, not three. + test("unstamped rows fall into the default arm", () => { + const rows = computeVariantMetrics([ + row("A", { variantId: null }), + row("A", { variantId: "other" }), + ]); + expect(rows.map((r) => r.variantId)).toEqual(["default", "other"]); + }); +}); + +describe("variantSub", () => { + const rows = (...specs: [string, number | null][]) => + specs.map(([variantId, cost]) => ({ + variantId, + metrics: { cost } as never as import("../run-view").RunMetrics, + })); + + test("joins the per-arm values in arm order", () => { + expect( + variantSub(rows(["A", 0.2], ["B", 0.6]), (m) => + m.cost != null ? `$${m.cost.toFixed(2)}` : null, + ), + ).toBe("A $0.20 · B $0.60"); + }); + + // An arm with nothing to report is dropped rather than rendered as a dash, + // so a partially-priced run shows what it knows. + test("arms with no value are omitted", () => { + expect( + variantSub(rows(["A", null], ["B", 0.6]), (m) => + m.cost != null ? `$${m.cost.toFixed(2)}` : null, + ), + ).toBe("B $0.60"); + }); +}); diff --git a/evalboard/app/runs/[id]/__tests__/source-hrefs.test.tsx b/evalboard/app/runs/[id]/__tests__/source-hrefs.test.tsx index c35143bd..0434c3cb 100644 --- a/evalboard/app/runs/[id]/__tests__/source-hrefs.test.tsx +++ b/evalboard/app/runs/[id]/__tests__/source-hrefs.test.tsx @@ -10,6 +10,7 @@ function row( ): TaskResultSummary { return { taskId, + variantId: null, replicateIndex: null, status: "SUCCESS", weightedScore: 1.0, diff --git a/evalboard/app/runs/[id]/__tests__/task-grid.test.tsx b/evalboard/app/runs/[id]/__tests__/task-grid.test.tsx index 965d9724..c840715f 100644 --- a/evalboard/app/runs/[id]/__tests__/task-grid.test.tsx +++ b/evalboard/app/runs/[id]/__tests__/task-grid.test.tsx @@ -11,6 +11,7 @@ function row( ): TaskResultSummary { return { taskId, + variantId: null, replicateIndex: null, status: "SUCCESS", weightedScore: 1.0, @@ -472,3 +473,125 @@ describe("TaskGrid — replicates", () => { ).toBe("1/2✓"); }); }); + +describe("TaskGrid — variants", () => { + // A run with no variants must render exactly as it always has: no extra + // column, no extra chip, no ?v= on any link. + test("an ordinary run gains no variant column and no ?v= link", () => { + render( + , + ); + const table = screen.getByRole("table"); + expect( + within(table).queryByRole("columnheader", { name: /variant/i }), + ).toBeNull(); + const link = within(table).getByRole("link", { name: /alpha/i }); + expect(link.getAttribute("href")).not.toContain("v="); + }); + + test("a two-arm run shows the column and one row per arm", () => { + render( + , + ); + const table = screen.getByRole("table"); + expect( + within(table).getByRole("columnheader", { name: /variant/i }), + ).toBeTruthy(); + // Both arms survive: the collapse groups replicates, never arms. + expect(within(table).getAllByRole("link", { name: /alpha/i })).toHaveLength( + 2, + ); + expect(within(table).getByText("live-v1")).toBeTruthy(); + expect(within(table).getByText("preview-v2")).toBeTruthy(); + }); + + // Without ?v= both rows would open the same transcript, which is the exact + // failure this change exists to fix. + test("each arm's row links to its own arm", () => { + render( + , + ); + const table = screen.getByRole("table"); + const hrefs = within(table) + .getAllByRole("link", { name: /alpha/i }) + .map((a) => a.getAttribute("href") ?? ""); + expect(hrefs.some((h) => h.includes("v=live-v1"))).toBe(true); + expect(hrefs.some((h) => h.includes("v=preview-v2"))).toBe(true); + }); +}); + +describe("TaskGrid — default ordering keeps a task's arms together", () => { + // The case that motivated the rule: one task's arms DISAGREE. Ranking rows + // independently sends the failing arm to the top and the passing arm to the + // bottom, which is precisely the comparison the run was made to show. + test("a task whose arms disagree still renders its two rows adjacent", () => { + render( + , + ); + const order = screen + .getAllByRole("row") + .slice(1) + .map((tr) => tr.textContent ?? ""); + + // alpha's arms are rows 0 and 1: the failing task sorts first, and its + // passing arm comes with it rather than being ranked to the bottom. + expect(order[0]).toMatch(/alpha/i); + expect(order[1]).toMatch(/alpha/i); + expect(order[2]).toMatch(/beta/i); + expect(order[3]).toMatch(/beta/i); + }); + + test("a run without variants keeps failures-first, then task id", () => { + render( + , + ); + const order = screen + .getAllByRole("row") + .slice(1) + .map((tr) => tr.textContent ?? ""); + expect(order[0]).toMatch(/mmm/i); + expect(order[1]).toMatch(/aaa/i); + expect(order[2]).toMatch(/zzz/i); + }); +}); diff --git a/evalboard/app/runs/[id]/run-view.tsx b/evalboard/app/runs/[id]/run-view.tsx index f397113d..a5171fe5 100644 --- a/evalboard/app/runs/[id]/run-view.tsx +++ b/evalboard/app/runs/[id]/run-view.tsx @@ -7,6 +7,11 @@ import type { ReviewIndexEntry } from "@/lib/reviews-types"; import { fmtDuration, humanizeTaskId } from "@/lib/format"; import { passBarClass, passClass } from "@/lib/pass-rate"; import { perTaskPassCounts, statusCategory } from "@/lib/status"; +import { + DEFAULT_VARIANT_ID, + taskVariantKey, + variantsOf, +} from "@/lib/variants"; import { taskCarriesRepoTag } from "@/lib/tags"; import { ChipLegend } from "@/app/_overview/tag-rail"; import { CollapsibleRail } from "@/app/_components/collapsible-rail"; @@ -113,6 +118,23 @@ export function computeRunMetrics(tasks: TaskResultSummary[]): RunMetrics { }; } +// Per-arm rollup for a run that declares `variants:`. Reuses computeRunMetrics so +// an arm's numbers come from exactly the same code as a single-arm run's. +export function computeVariantMetrics( + tasks: TaskResultSummary[], +): { variantId: string; metrics: RunMetrics }[] { + const ids = variantsOf(tasks); + if (ids.length < 2) return []; + return ids.map((variantId) => ({ + variantId, + metrics: computeRunMetrics( + tasks.filter( + (t) => (t.variantId ?? DEFAULT_VARIANT_ID) === variantId, + ), + ), + })); +} + function parseTagsParam(raw: string | null): string[] { if (!raw) return []; return raw @@ -151,6 +173,75 @@ function Metric({ ); } +// Replaces the pooled number inside the Pass rate tile: a blended rate averages +// configurations that were deliberately made to differ, and moves when the arms +// are merely reordered. Spend and time keep their pooled totals instead, since a +// run's cost is true however many arms produced it. +// +// Arms sit side by side because the tile is already double width; stacking them +// made it the tallest thing in the row and stretched its neighbours. +// +// No significance test on purpose — that lives in experiment.md. "spread" states +// the observed gap and claims nothing about it. + +// Per-TASK rate, matching the single-arm tile's rule (a task passes if any +// replicate passed). Equals the per-row rate on a run without repeats. +const variantRate = (m: RunMetrics) => + m.taskTotal ? (m.taskPassed / m.taskTotal) * 100 : 0; + +function PassRateByVariant({ + rows, +}: { + rows: { variantId: string; metrics: RunMetrics }[]; +}) { + return ( +
+ {rows.map(({ variantId, metrics: m }) => { + const pct = variantRate(m); + // null when the arm ran nothing → neutral, not a measured 0%. + const tone = m.taskTotal > 0 ? pct : null; + return ( +
+
+ + {variantId} + + + {pct.toFixed(0)}% + + + {m.taskPassed} / {m.taskTotal} + +
+
+
+
+
+ ); + })} +
+ ); +} + +// The same quantity split by arm, for a pooled tile's sub-line. Arms missing the +// value are dropped rather than rendered as a dash. +export function variantSub( + rows: { variantId: string; metrics: RunMetrics }[], + pick: (m: RunMetrics) => string | null, +): string | undefined { + const parts: string[] = []; + for (const { variantId, metrics } of rows) { + const v = pick(metrics); + if (v != null) parts.push(`${variantId} ${v}`); + } + return parts.length ? parts.join(" · ") : undefined; +} + export function RunView({ runId, tasks, @@ -327,20 +418,34 @@ export function RunView({ // the run actually did — distinct from the grid below, which collapses // replicates to one row per task. The count label spells out both numbers. const metrics = useMemo(() => computeRunMetrics(filtered), [filtered]); + // Empty on every ordinary run (fewer than two arms), which is what keeps the + // comparison strip out of the way until a run actually has something to + // compare. + const variantMetrics = useMemo( + () => computeVariantMetrics(filtered), + [filtered], + ); // The run has repeated tasks iff the per-task and per-replicate totals // differ. When true, the Pass-rate and Failed tiles switch to per-task units // (with the per-replicate figures shown as a sub-line) so they never mix. const hasRepeats = metrics.taskTotal !== metrics.total; + const hasVariants = variantMetrics.length > 0; + const variantRates = variantMetrics.map((r) => variantRate(r.metrics)); - // The grid collapses replicates to one row per task, so the count beside the - // "Tasks" header must report distinct tasks (not execution rows) to match it; - // when a run has replicates we also surface the execution count. + // The grid collapses replicates to one row per (task, arm), so the count + // beside the "Tasks" header must count the same thing to match it; when a run + // has replicates we also surface the execution count. + // + // Keying on the arm as well as the task is what keeps this honest on a + // multi-variant run: those rows are NOT collapsed in the grid, so counting + // distinct task ids would print "6 tasks" above twelve visible rows and then + // mislabel the other six as replicate executions. const taskCount = useMemo( - () => new Set(tasks.map((t) => t.taskId)).size, + () => new Set(tasks.map(taskVariantKey)).size, [tasks], ); const filteredTaskCount = useMemo( - () => new Set(filtered.map((t) => t.taskId)).size, + () => new Set(filtered.map(taskVariantKey)).size, [filtered], ); const hasReplicates = taskCount !== tasks.length; @@ -395,8 +500,20 @@ export function RunView({ · filtered )} + {hasVariants && ( + + {variantMetrics.length} arms · spread{" "} + {( + Math.max(...variantRates) - + Math.min(...variantRates) + ).toFixed(0)}{" "} + pts + + )}
- {(() => { + {hasVariants ? ( + + ) : (() => { // With repeats, the headline is the per-TASK rate — a // task counts as passed if any replicate passed — and the // raw per-replicate rate moves to a sub-line. Single-shot @@ -488,19 +605,33 @@ export function RunView({ : "—" } sub={ - metrics.costP50 != null && metrics.costP90 != null - ? `p50 $${metrics.costP50.toFixed(2)} · p90 $${metrics.costP90.toFixed(2)}` - : undefined + // p50/p90 across pooled arms would describe a population + // that does not exist. + hasVariants + ? variantSub(variantMetrics, (m) => + m.cost != null + ? `$${m.cost.toFixed(2)}` + : null, + ) + : metrics.costP50 != null && metrics.costP90 != null + ? `p50 $${metrics.costP50.toFixed(2)} · p90 $${metrics.costP90.toFixed(2)}` + : undefined } /> + m.duration != null + ? fmtDuration(m.duration) + : null, + ) + : metrics.durationP50 != null && + metrics.durationP90 != null + ? `p50 ${fmtDuration(metrics.durationP50)} · p90 ${fmtDuration(metrics.durationP90)}` + : undefined } />
diff --git a/evalboard/app/runs/[id]/task-grid.tsx b/evalboard/app/runs/[id]/task-grid.tsx index 390fdff2..de97a5e4 100644 --- a/evalboard/app/runs/[id]/task-grid.tsx +++ b/evalboard/app/runs/[id]/task-grid.tsx @@ -15,6 +15,7 @@ import { StatusPill, } from "@/lib/pills"; import { isPassStatus, perTaskPassCounts, statusSortRank } from "@/lib/status"; +import { DEFAULT_VARIANT_ID, taskVariantKey, variantsOf } from "@/lib/variants"; import { displayedTurns, fmtTurnsCount, @@ -36,6 +37,7 @@ import { TOKEN_COLUMN_HELP } from "@/app/_components/col-help"; type SortKey = | "task" + | "variant" | "status" | "score" | "duration" @@ -54,6 +56,7 @@ const COLUMN_HELP: Partial> = { turns: "Visible turns: one per tool call plus one for the final reply. Tinted against the task's hand-written expected_turns budget (yellow past 1.25×, red past 1.5×); untinted when the task declares none.", vsExp: "Duration ÷ the time this task is expected to need. The expected time is derived per task, per harness by the eval runner (its fastest passing run, or p10 once there are ten) and stamped into the run — never hand-written. Past 2× counts as slow; a task its harness has never passed shows —.", cost: "Total billed cost for this task, reported by the SDK (summed across turns).", + variant: "Experiment arm this row was produced by. A run declaring `variants:` executes every task once per arm and keeps each arm's output in its own subtree, so the same task appears once per arm and the two rows are separate measurements — never collapsed together.", }; // A mature task that was skipped this run has no detail page in THIS run, but it @@ -218,10 +221,18 @@ function TaskIdCell({ // For a collapsed replicate row, link to the SAME replicate the row's // status/score/cost describe (the representative), not implicitly to // replicate 0 — so clicking a green "Passed" row lands on the passing run. + // + // On a multi-variant run the arm is part of the row's identity, so it has to + // travel in the link too; without ?v= both arms of a task would open the + // same (first-matching) transcript. A default-arm row omits the param, so + // every link on an ordinary run is unchanged. + const variant = t.variantId ?? DEFAULT_VARIANT_ID; + const params = new URLSearchParams(); + if (replicateCount > 1) params.set("r", String(t.replicateIndex ?? 0)); + if (variant !== DEFAULT_VARIANT_ID) params.set("v", variant); + const qs = params.toString(); const href = withSource( - replicateCount > 1 - ? `/runs/${runId}/${t.taskId}?r=${t.replicateIndex ?? 0}` - : `/runs/${runId}/${t.taskId}`, + `/runs/${runId}/${t.taskId}${qs ? `?${qs}` : ""}`, sourceId, ); return ( @@ -239,6 +250,16 @@ function TaskIdCell({ ); } +// One neutral style, not a per-arm colour: the id is already the signal, and it +// leaves colour in this column meaning pass/fail (the replicate badge). +function VariantChip({ variantId }: { variantId: string }) { + return ( + + {variantId} + + ); +} + // Colour tier for the k/N ✓ replicate badge: all passed → green, some → amber, // none → red. Kept as a named helper so the all/some/none intent is explicit // and the JSX stays flat. @@ -285,8 +306,20 @@ const DEFAULT_DIR: Record = { output: "desc", cw: "desc", cr: "desc", + variant: "asc", }; +// Final tiebreak for both sort paths — without the variant leg, two rows of one +// task have no defined order and reshuffle between renders. +function byTaskThenVariant(a: TaskResultSummary, b: TaskResultSummary): number { + return ( + a.taskId.localeCompare(b.taskId) || + (a.variantId ?? DEFAULT_VARIANT_ID).localeCompare( + b.variantId ?? DEFAULT_VARIANT_ID, + ) + ); +} + function compare( a: TaskResultSummary, b: TaskResultSummary, @@ -295,6 +328,10 @@ function compare( switch (key) { case "task": return a.taskId.localeCompare(b.taskId); + case "variant": + return (a.variantId ?? DEFAULT_VARIANT_ID).localeCompare( + b.variantId ?? DEFAULT_VARIANT_ID, + ); case "status": return statusSortRank(a.status) - statusSortRank(b.status); case "score": @@ -346,6 +383,8 @@ const COLUMNS: Array<{ align?: "right"; }> = [ { key: "task", header: "Task" }, + // Only rendered when the run has more than one arm — see visibleColumns. + { key: "variant", header: "Variant" }, { key: "status", header: "Status" }, { key: "score", header: "Score", align: "right" }, { key: "duration", header: "Duration", align: "right" }, @@ -504,12 +543,19 @@ export function TaskGrid({ // the token detail is one click away via the toolbar toggle. const [showTokens, setShowTokens] = useState(false); - // How many rows share each taskId — i.e. the replicate count for that task. - // Drives whether a row shows its replicate badge + ?r link (only when >1, so - // single-run tasks aren't cluttered with a "#0"). + // Experiment arms present in this run. One entry (or none) on an ordinary + // run, in which case every variant affordance below stays hidden and the + // grid renders exactly as it did before variants were readable. + const variantIds = useMemo(() => variantsOf(tasks), [tasks]); + const hasVariants = variantIds.length > 1; + + // How many rows share each (variant, task) — i.e. the replicate count for + // that arm of that task. Drives whether a row shows its replicate badge + + // ?r link (only when >1, so single-run tasks aren't cluttered with a "#0"). const replicateCounts = useMemo(() => { const m = new Map(); - for (const t of tasks) m.set(t.taskId, (m.get(t.taskId) ?? 0) + 1); + for (const t of tasks) + m.set(taskVariantKey(t), (m.get(taskVariantKey(t)) ?? 0) + 1); return m; }, [tasks]); @@ -519,28 +565,34 @@ export function TaskGrid({ // page's pass-rate tile all apply the same "any replicate passed" rule. const replicatePassCounts = useMemo(() => perTaskPassCounts(tasks), [tasks]); - // Collapse replicates to one row per task: repeated runs share a taskId, so - // the grid shows a single entry with a k/N ✓ badge; the per-run detail is - // selectable on the task page. The representative is chosen so its status, - // score, cost, duration AND detail link all describe the SAME run: prefer a - // passing replicate when any passed (else the lowest-index one), breaking - // ties by lowest replicateIndex for stability. Pick BEFORE sorting so a - // metric-sorted view still shows one row per task. + // Collapse replicates to one row per (variant, task): repeated runs share a + // taskId, so the grid shows a single entry with a k/N ✓ badge; the per-run + // detail is selectable on the task page. The representative is chosen so its + // status, score, cost, duration AND detail link all describe the SAME run: + // prefer a passing replicate when any passed (else the lowest-index one), + // breaking ties by lowest replicateIndex for stability. Pick BEFORE sorting + // so a metric-sorted view still shows one row per task. + // + // Replicates collapse; ARMS DO NOT. Two variants of one task are separate + // measurements of separate configurations — collapsing them would let a pass + // in one arm hide a failure in the other, which is the whole signal an A/B + // run exists to show. const collapsed = useMemo(() => { const byTask = new Map(); for (const t of tasks) { - const cur = byTask.get(t.taskId); + const key = taskVariantKey(t); + const cur = byTask.get(key); if (!cur) { - byTask.set(t.taskId, t); + byTask.set(key, t); continue; } const curPass = isPassStatus(cur.status); const tPass = isPassStatus(t.status); if (curPass !== tPass) { // A passing replicate always wins over a non-passing one. - if (tPass) byTask.set(t.taskId, t); + if (tPass) byTask.set(key, t); } else if ((t.replicateIndex ?? 0) < (cur.replicateIndex ?? 0)) { - byTask.set(t.taskId, t); + byTask.set(key, t); } } return [...byTask.values()]; @@ -552,14 +604,25 @@ export function TaskGrid({ arr.sort((a, b) => { const c = compare(a, b, sort.key); if (c !== 0) return sort.dir === "asc" ? c : -c; - return a.taskId.localeCompare(b.taskId); + return byTaskThenVariant(a, b); }); } else { - // Default: failures first, then by task id. + // Failures first, then task id — but ranked on the TASK by its worst + // arm. Ranking rows independently splits exactly the pairs worth + // reading: a task whose arms disagree gets one row at the top of the + // grid and the other at the bottom. Unchanged without variants, where + // a task's single row IS its worst arm. + const worstByTask = new Map(); + for (const t of arr) { + const r = statusSortRank(t.status); + const cur = worstByTask.get(t.taskId); + if (cur === undefined || r < cur) worstByTask.set(t.taskId, r); + } arr.sort( (a, b) => - statusSortRank(a.status) - statusSortRank(b.status) || - a.taskId.localeCompare(b.taskId), + (worstByTask.get(a.taskId) ?? 0) - + (worstByTask.get(b.taskId) ?? 0) || + byTaskThenVariant(a, b), ); } return arr; @@ -573,8 +636,13 @@ export function TaskGrid({ ); }; + // The Variant column only carries information on a run that actually has more + // than one arm; on every ordinary run it would be a column of identical + // "default" cells, so it is dropped and the grid renders as it always has. const visibleColumns = COLUMNS.filter( - (c) => showTokens || !TOKEN_KEYS.has(c.key), + (c) => + (showTokens || !TOKEN_KEYS.has(c.key)) && + (hasVariants || c.key !== "variant"), ); return ( @@ -671,7 +739,7 @@ export function TaskGrid({ ); return ( @@ -682,10 +750,10 @@ export function TaskGrid({ className="text-gray-900 hover:text-studio-blue font-semibold" matureSourceRuns={matureSourceRuns} replicateCount={ - replicateCounts.get(t.taskId) ?? 1 + replicateCounts.get(taskVariantKey(t)) ?? 1 } replicatePassCount={ - replicatePassCounts.get(t.taskId) ?? 0 + replicatePassCounts.get(taskVariantKey(t)) ?? 0 } sourceId={sourceId} /> @@ -699,6 +767,15 @@ export function TaskGrid({ /> + {hasVariants && ( + + + + )} {t.matureSkipped ? ( @@ -824,7 +901,7 @@ export function TaskGrid({ ); return (
@@ -834,14 +911,22 @@ export function TaskGrid({ className="min-w-0 break-words font-semibold text-gray-900 hover:text-studio-blue" matureSourceRuns={matureSourceRuns} replicateCount={ - replicateCounts.get(t.taskId) ?? 1 + replicateCounts.get(taskVariantKey(t)) ?? 1 } replicatePassCount={ - replicatePassCounts.get(t.taskId) ?? 0 + replicatePassCounts.get(taskVariantKey(t)) ?? 0 } sourceId={sourceId} /> - + + {hasVariants && ( + + )} {t.matureSkipped ? ( ) : ( diff --git a/evalboard/lib/__tests__/variant-reads.test.ts b/evalboard/lib/__tests__/variant-reads.test.ts new file mode 100644 index 00000000..bc40cce6 --- /dev/null +++ b/evalboard/lib/__tests__/variant-reads.test.ts @@ -0,0 +1,231 @@ +import { promises as fs } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; + +// Same harness as collect.test.ts: the readers resolve RUNS_DIR from +// EVALBOARD_LOCAL_RUNS_DIR at *import* time, so each test stubs the env to a +// throwaway runs dir and then dynamically imports a fresh module copy. +let tmp: string; + +async function write(rel: string, body: string): Promise { + const abs = path.join(tmp, rel); + await fs.mkdir(path.dirname(abs), { recursive: true }); + await fs.writeFile(abs, body); +} + +async function loadRuns() { + vi.resetModules(); + vi.stubEnv("EVALBOARD_LOCAL_RUNS_DIR", tmp); + return import("../runs"); +} + +const TASK = "demo-task"; + +// A two-arm run, exactly as coder_eval's experiment layer lays it out: +// run.json rows stamped with variant_id, content under ////. +const AB_RUN = "2026-01-02_00-00-00"; +// A run predating variants: no variant_id on the rows, content under +// /default///. This is the backward-compatibility fixture. +const LEGACY_RUN = "2026-01-01_00-00-00"; + +beforeEach(async () => { + tmp = await fs.mkdtemp(path.join(os.tmpdir(), "evalboard-variants-")); + + await write( + `${AB_RUN}/run.json`, + JSON.stringify({ + task_results: [ + { + task_id: TASK, + variant_id: "live-v1", + replicate_index: 0, + status: "SUCCESS", + weighted_score: 1, + }, + { + task_id: TASK, + variant_id: "preview-v2", + replicate_index: 0, + status: "FAILURE", + weighted_score: 0, + }, + // A second replicate in one arm only, so replicate enumeration + // must not leak across arms. + { + task_id: TASK, + variant_id: "preview-v2", + replicate_index: 1, + status: "SUCCESS", + weighted_score: 1, + }, + ], + }), + ); + await write( + `${AB_RUN}/live-v1/${TASK}/00/task.json`, + JSON.stringify({ final_status: "SUCCESS" }), + ); + await write(`${AB_RUN}/live-v1/${TASK}/00/task.log`, "log from live-v1"); + await write( + `${AB_RUN}/preview-v2/${TASK}/00/task.json`, + JSON.stringify({ final_status: "FAILURE" }), + ); + await write( + `${AB_RUN}/preview-v2/${TASK}/00/task.log`, + "log from preview-v2", + ); + await write( + `${AB_RUN}/preview-v2/${TASK}/01/task.json`, + JSON.stringify({ final_status: "SUCCESS" }), + ); + await write(`${AB_RUN}/preview-v2/${TASK}/01/task.log`, "log from replicate 1"); + + await write( + `${LEGACY_RUN}/run.json`, + JSON.stringify({ + task_results: [{ task_id: TASK, status: "SUCCESS", weighted_score: 1 }], + }), + ); + await write( + `${LEGACY_RUN}/default/${TASK}/00/task.json`, + JSON.stringify({ final_status: "SUCCESS" }), + ); + await write(`${LEGACY_RUN}/default/${TASK}/00/task.log`, "legacy log"); +}); + +afterEach(async () => { + vi.unstubAllEnvs(); + await fs.rm(tmp, { recursive: true, force: true }); +}); + +describe("backward compatibility", () => { + // The hard requirement: a run with no variant_id anywhere must resolve + // exactly as it did before, with no caller passing a variant. + test("a legacy run resolves with no variant argument", async () => { + const { readTaskDetail, readLogTail, readTaskReplicates } = + await loadRuns(); + const detail = await readTaskDetail(LEGACY_RUN, TASK); + expect(detail?.status).toBe("SUCCESS"); + expect(await readLogTail(LEGACY_RUN, TASK)).toBe("legacy log"); + expect(await readTaskReplicates(LEGACY_RUN, TASK)).toEqual([0]); + }); + + test("a legacy row carries a null variantId, not a fabricated one", async () => { + const { toTaskRow } = await loadRuns(); + expect(toTaskRow({ task_id: TASK }).variantId).toBeNull(); + }); +}); + +describe("multi-variant reads", () => { + // Without variant-aware row matching both arms resolve to the first matching + // row, so the failing arm would render the passing arm's result. + test("each arm resolves to its own row", async () => { + const { readTaskDetail } = await loadRuns(); + const a = await readTaskDetail(AB_RUN, TASK, 0, undefined, "live-v1"); + const b = await readTaskDetail(AB_RUN, TASK, 0, undefined, "preview-v2"); + expect(a?.status).toBe("SUCCESS"); + expect(b?.status).toBe("FAILURE"); + expect(a?.variantId).toBe("live-v1"); + expect(b?.variantId).toBe("preview-v2"); + }); + + // And its own content: the row is only half the resolution, the path is the + // other half. + test("each arm resolves to its own content directory", async () => { + const { readLogTail } = await loadRuns(); + expect( + await readLogTail(AB_RUN, TASK, 0, undefined, undefined, "live-v1"), + ).toBe("log from live-v1"); + expect( + await readLogTail( + AB_RUN, + TASK, + 0, + undefined, + undefined, + "preview-v2", + ), + ).toBe("log from preview-v2"); + }); + + test("replicate enumeration is scoped to the arm", async () => { + const { readTaskReplicates } = await loadRuns(); + expect( + await readTaskReplicates(AB_RUN, TASK, undefined, "live-v1"), + ).toEqual([0]); + expect( + await readTaskReplicates(AB_RUN, TASK, undefined, "preview-v2"), + ).toEqual([0, 1]); + }); + + // An arm that isn't in the run must 404 rather than silently fall back to + // another arm's result. + test("an unknown arm yields no detail", async () => { + const { readTaskDetail } = await loadRuns(); + expect( + await readTaskDetail(AB_RUN, TASK, 0, undefined, "no-such-arm"), + ).toBeNull(); + }); +}); + +describe("resolving an unnamed arm", () => { + // Every experiment but experiments/default.yaml names its own arms, so an + // experiment run has no `default` row. Trends, the watchlist and every + // pre-variant bookmark link to /runs// with no arm, which would + // otherwise match nothing and 404 — a regression from the pre-variant page. + test("a variant-less URL resolves to the task's first arm", async () => { + const { resolveVariantId, readTaskDetail } = await loadRuns(); + const arm = await resolveVariantId(AB_RUN, TASK, null); + expect(arm).toBe("live-v1"); + expect((await readTaskDetail(AB_RUN, TASK, 0, undefined, arm))?.status).toBe( + "SUCCESS", + ); + }); + + test("a run whose arm is the default one still resolves to it", async () => { + const { resolveVariantId } = await loadRuns(); + expect(await resolveVariantId(LEGACY_RUN, TASK, null)).toBe("default"); + }); + + // Resolution only fills in an arm nobody asked for. A URL naming an arm the + // run does not have must keep 404ing rather than land on a different one. + test("an explicitly named arm is returned untouched", async () => { + const { resolveVariantId } = await loadRuns(); + expect(await resolveVariantId(AB_RUN, TASK, "no-such-arm")).toBe( + "no-such-arm", + ); + }); + + test("an unknown task keeps the default arm", async () => { + const { resolveVariantId } = await loadRuns(); + expect(await resolveVariantId(AB_RUN, "no-such-task", null)).toBe( + "default", + ); + }); +}); + +describe("collectTaskFiles under variants", () => { + test("zips the named arm's folder", async () => { + const { collectTaskFiles } = await loadRuns(); + const files = await collectTaskFiles( + AB_RUN, + TASK, + undefined, + "preview-v2", + ); + expect(files?.map((f) => f.relPath).sort()).toEqual([ + "00/task.json", + "00/task.log", + "01/task.json", + "01/task.log", + ]); + }); + + test("rejects a variant id that could escape the run dir", async () => { + const { collectTaskFiles } = await loadRuns(); + expect( + await collectTaskFiles(AB_RUN, TASK, undefined, ".."), + ).toBeNull(); + }); +}); diff --git a/evalboard/lib/__tests__/variants.test.ts b/evalboard/lib/__tests__/variants.test.ts new file mode 100644 index 00000000..d6163b8c --- /dev/null +++ b/evalboard/lib/__tests__/variants.test.ts @@ -0,0 +1,129 @@ +import { describe, expect, test } from "vitest"; +import { + DEFAULT_VARIANT_ID, + isValidVariantId, + taskVariantKey, + variantsOf, +} from "../variants"; +import { perTaskPassCounts } from "../status"; + +describe("isValidVariantId", () => { + test("accepts the ids coder_eval writes", () => { + expect(isValidVariantId(DEFAULT_VARIANT_ID)).toBe(true); + expect(isValidVariantId("live-v1")).toBe(true); + expect(isValidVariantId("preview_v2")).toBe(true); + expect(isValidVariantId("sonnet-4.6")).toBe(true); + }); + + // The id reaches a filesystem path AND a blob prefix off untyped run.json. + // Stricter than the task-id rule: an internal slash is never legitimate, + // since dataset expansion nests the TASK id, never the variant. + test("rejects anything that could escape the run directory", () => { + expect(isValidVariantId("")).toBe(false); + expect(isValidVariantId(".")).toBe(false); + expect(isValidVariantId("..")).toBe(false); + expect(isValidVariantId("../etc/passwd")).toBe(false); + expect(isValidVariantId("a/b")).toBe(false); + expect(isValidVariantId("a\\b")).toBe(false); + expect(isValidVariantId("/absolute")).toBe(false); + expect(isValidVariantId("C:\\windows")).toBe(false); + }); + + test("rejects non-strings and over-long ids", () => { + expect(isValidVariantId(null)).toBe(false); + expect(isValidVariantId(undefined)).toBe(false); + expect(isValidVariantId(42)).toBe(false); + expect(isValidVariantId("a".repeat(128))).toBe(false); + }); +}); + +describe("taskVariantKey", () => { + test("a row with no variant reads as the default arm", () => { + expect(taskVariantKey({ taskId: "t" })).toBe(taskVariantKey({ + taskId: "t", + variantId: DEFAULT_VARIANT_ID, + })); + expect(taskVariantKey({ taskId: "t", variantId: null })).toBe( + taskVariantKey({ taskId: "t", variantId: DEFAULT_VARIANT_ID }), + ); + }); + + test("two arms of one task get distinct keys", () => { + expect(taskVariantKey({ taskId: "t", variantId: "a" })).not.toBe( + taskVariantKey({ taskId: "t", variantId: "b" }), + ); + }); + + // The separator must not be forgeable from either side, or an arm could be + // made to collide with a different (arm, task) pair. + test("cannot be forged by an id containing the separator", () => { + expect(taskVariantKey({ taskId: "b c", variantId: "a" })).not.toBe( + taskVariantKey({ taskId: "c", variantId: "a b" }), + ); + }); +}); + +describe("variantsOf", () => { + test("an ordinary run reports a single arm", () => { + expect(variantsOf([{ variantId: null }, {}])).toEqual([ + DEFAULT_VARIANT_ID, + ]); + }); + + test("arms come back sorted and deduplicated", () => { + expect( + variantsOf([ + { variantId: "preview-v2" }, + { variantId: "live-v1" }, + { variantId: "preview-v2" }, + ]), + ).toEqual(["live-v1", "preview-v2"]); + }); +}); + +describe("perTaskPassCounts under variants", () => { + // The regression gate: a run with no variants must roll up exactly as it did + // when the key was the task id alone. + test("single-arm run rolls up one entry per task", () => { + const counts = perTaskPassCounts([ + { taskId: "t1", status: "SUCCESS" }, + { taskId: "t1", status: "FAILURE" }, + { taskId: "t2", status: "SUCCESS" }, + ]); + expect(counts.size).toBe(2); + expect(counts.get(taskVariantKey({ taskId: "t1" }))).toBe(1); + expect(counts.get(taskVariantKey({ taskId: "t2" }))).toBe(1); + }); + + // The point of the change: one arm passing must not make the other arm's + // failure disappear from the rollup. + test("arms are counted separately, so a pass cannot mask a failure", () => { + const counts = perTaskPassCounts([ + { taskId: "t1", variantId: "live-v1", status: "SUCCESS" }, + { taskId: "t1", variantId: "preview-v2", status: "FAILURE" }, + ]); + expect(counts.size).toBe(2); + expect( + counts.get(taskVariantKey({ taskId: "t1", variantId: "live-v1" })), + ).toBe(1); + expect( + counts.get( + taskVariantKey({ taskId: "t1", variantId: "preview-v2" }), + ), + ).toBe(0); + }); + + test("replicates still collapse within one arm", () => { + const counts = perTaskPassCounts([ + { taskId: "t1", variantId: "a", status: "SUCCESS" }, + { taskId: "t1", variantId: "a", status: "SUCCESS" }, + { taskId: "t1", variantId: "b", status: "FAILURE" }, + ]); + expect(counts.get(taskVariantKey({ taskId: "t1", variantId: "a" }))).toBe( + 2, + ); + expect(counts.get(taskVariantKey({ taskId: "t1", variantId: "b" }))).toBe( + 0, + ); + }); +}); diff --git a/evalboard/lib/blob.ts b/evalboard/lib/blob.ts index 7fd12b64..041454f5 100644 --- a/evalboard/lib/blob.ts +++ b/evalboard/lib/blob.ts @@ -2,6 +2,7 @@ import { promises as fs } from "node:fs"; import path from "node:path"; import { randomBytes } from "node:crypto"; import type { BlobServiceClient, ContainerClient } from "@azure/storage-blob"; +import { DEFAULT_VARIANT_ID, isValidVariantId } from "./variants"; const ACCOUNT = "coderevaltests"; @@ -44,6 +45,12 @@ function assertValidId(id: string, label: string): void { } } +function assertValidVariantId(id: string, label: string): void { + if (!isValidVariantId(id)) { + throw new Error(`Invalid ${label}: ${JSON.stringify(id)}`); + } +} + function assertValidTaskId(id: string, label: string): void { if (!isValidTaskId(id)) { throw new Error(`Invalid ${label}: ${JSON.stringify(id)}`); @@ -322,11 +329,15 @@ export async function ensureTaskDir( runId: string, taskId: string, destRoot: string, + // Which arm of a multi-variant run to fetch. Defaults to the single arm a + // non-experiment run writes, so every existing call site is unchanged. + variantId: string = DEFAULT_VARIANT_ID, ): Promise { assertValidId(runId, "runId"); assertValidTaskId(taskId, "taskId"); + assertValidVariantId(variantId, "variantId"); if (LOCAL_RUNS_DIR) return; - return dedupe(`task:${container}:${runId}/${taskId}`, async () => { + return dedupe(`task:${container}:${runId}/${variantId}/${taskId}`, async () => { // Activation cases live in the nested sub-run (/activation/...), // so their row + per-case dir come from there; skills tasks from the // top-level run. Fetch the matching run.json for the row lookup. @@ -337,13 +348,16 @@ export async function ensureTaskDir( const c = await getContainer(container); const ops: Promise[] = []; // `listBlobsFlat` recurses, so both the flat legacy layout - // (`default//task.json`) and the nested replicate layout - // (`default//00/task.json`) download unchanged — the prefix + // (`//task.json`) and the nested replicate layout + // (`//00/task.json`) download unchanged — the prefix // scope is the task subtree either way. `resolveTaskContentDir` in // runs.ts then picks the right shape at render time. + // + // The activation sub-run is single-variant by construction (it is a + // nested run of its own), so it keeps the literal `default` segment. const prefix = activation - ? `${runId}/activation/default/${taskId}/` - : `${runId}/default/${taskId}/`; + ? `${runId}/activation/${DEFAULT_VARIANT_ID}/${taskId}/` + : `${runId}/${variantId}/${taskId}/`; for await (const blob of c.listBlobsFlat({ prefix })) { // Agent sandboxes that run Python leave a `.venv/` tree (hundreds // of files, tens of MB) under the task dir. No UI page reads it, diff --git a/evalboard/lib/runs.ts b/evalboard/lib/runs.ts index 0126e110..ad31a97d 100644 --- a/evalboard/lib/runs.ts +++ b/evalboard/lib/runs.ts @@ -13,6 +13,7 @@ import { listRunIdsLocal, listRunIdsRemote, } from "./blob"; +import { DEFAULT_VARIANT_ID, isValidVariantId } from "./variants"; import { DEFAULT_SOURCE, runsDirFor, type Source } from "./sources"; import { DELIVERABLE_KINDS, DELIVERABLE_NAMES } from "./artifact-kinds"; import { messageCostUsd } from "./pricing"; @@ -75,6 +76,13 @@ export interface RunSummary { export interface TaskResultSummary { taskId: string; + // Experiment arm this row belongs to. A run with variants writes one row per + // (task, variant, replicate) and stores each arm's content under its own + // // subtree, so this — not taskId alone — is what tells + // two arms of the same task apart. Null on runs whose run.json predates the + // field; every reader falls back to DEFAULT_VARIANT_ID, which is the segment + // a single-arm run writes anyway. + variantId: string | null; // Replicate index of this row, or null when repeats are disabled / on legacy // runs. Repeated runs share taskId, so this disambiguates sibling rows and is // carried in the per-task link (?r=NN) so each replicate opens its own detail. @@ -387,6 +395,10 @@ export function aggregateSubAgentUsage( interface RawTaskResult { task_id?: string; + // Experiment arm that produced this row (the sub-dir). Written by + // reports_experiment.py on every run; absent on runs that predate it, which + // read as DEFAULT_VARIANT_ID. + variant_id?: string | null; // Replicate index of this row (the // sub-dir). Repeated // runs of one task share a task_id; this is what tells the rows apart. Null // on runs that didn't track replicates (repeats disabled / legacy run.json). @@ -713,16 +725,24 @@ function isActivationTaskId(taskId: string): boolean { // Filesystem base for a task's content (before the optional `00` replicate dir): // activation cases under /activation/default/, skills tasks under -// /default/. +// //. +// +// `variantId` is the experiment arm — coder_eval's path_utils.build_task_run_dir +// writes //// for every run, using "default" +// when the experiment declares no variants. Defaulting here therefore reproduces +// the old hardcoded path exactly for every single-arm run. function taskContentBase( runId: string, taskId: string, + variantId: string = DEFAULT_VARIANT_ID, source: Source = DEFAULT_SOURCE, ): string { const dir = runsDirFor(RUNS_DIR, source); + // The activation sub-run is a nested single-variant run, so its cases always + // sit under `default` regardless of the outer run's arms. return isActivationTaskId(taskId) - ? path.join(dir, runId, "activation", "default", taskId) - : path.join(dir, runId, "default", taskId); + ? path.join(dir, runId, "activation", DEFAULT_VARIANT_ID, taskId) + : path.join(dir, runId, variantId, taskId); } // Resolve the skill (primary grouping axis) for a task. Two-stage fallback: @@ -754,6 +774,7 @@ export function toTaskRow(t: RawTaskResult): TaskResultSummary { const tags = t.tags ?? []; return { taskId: t.task_id ?? "", + variantId: t.variant_id ?? null, replicateIndex: t.replicate_index ?? null, status: t.status ?? null, weightedScore: t.weighted_score ?? null, @@ -2009,6 +2030,49 @@ async function resolveTaskContentDir( } } +// Row matcher shared by every per-task reader: a row belongs to (taskId, +// variantId) iff both match, with a missing variant_id reading as the default +// arm. Keeping it in one place is what stops the two arms of a multi-variant run +// from resolving to each other's transcript. +function rowMatches( + t: RawTaskResult, + taskId: string, + variantId: string, +): boolean { + return ( + t.task_id === taskId && (t.variant_id ?? DEFAULT_VARIANT_ID) === variantId + ); +} + +// The arm a task URL resolves to when it names none. Only `experiments/ +// default.yaml` calls its arm `default`; every other experiment names its own +// (`sonnet`/`opus`, `baseline`, `e2e`/`smoke`), so an experiment run has no +// `default` row at all and a bare /runs// would match nothing and +// 404. Trends, the watchlist and every pre-variant bookmark still emit exactly +// that form, so an unnamed arm resolves to the task's first arm in the order +// the grid lists them. An explicitly named arm is returned untouched — a URL +// asking for an arm the run does not have still 404s downstream rather than +// quietly rendering a different one. +export async function resolveVariantId( + runId: string, + taskId: string, + requested: string | null, + source: Source = DEFAULT_SOURCE, +): Promise { + if (requested != null) return requested; + const data = isActivationTaskId(taskId) + ? await readActivationRunJson(runId, source) + : await readRunJson(runId, source); + const arms = new Set(); + for (const t of data?.task_results ?? []) { + if (t.task_id === taskId) arms.add(t.variant_id ?? DEFAULT_VARIANT_ID); + } + // No rows (unknown task, unreadable run.json) keeps the default arm, so the + // caller still 404s the way it did before variants were addressable. + if (arms.size === 0 || arms.has(DEFAULT_VARIANT_ID)) return DEFAULT_VARIANT_ID; + return [...arms].sort()[0]; +} + // Replicate indices present for a task in this run, ascending (e.g. [0, 1, 2] // for a task run 3×). Drives the task page's run selector. A non-repeated or // legacy run yields [0] (rows carry no replicate_index → treated as 0); an @@ -2017,12 +2081,13 @@ export async function readTaskReplicates( runId: string, taskId: string, source: Source = DEFAULT_SOURCE, + variantId: string = DEFAULT_VARIANT_ID, ): Promise { const data = isActivationTaskId(taskId) ? await readActivationRunJson(runId, source) : await readRunJson(runId, source); const indices = (data?.task_results ?? []) - .filter((t) => t.task_id === taskId) + .filter((t) => rowMatches(t, taskId, variantId)) .map((t) => t.replicate_index ?? 0); return [...new Set(indices)].sort((a, b) => a - b); } @@ -2032,9 +2097,10 @@ export async function readTaskDetail( taskId: string, replicate = 0, source: Source = DEFAULT_SOURCE, + variantId: string = DEFAULT_VARIANT_ID, ): Promise { const dir = runsDirFor(RUNS_DIR, source); - await ensureTaskDir(source.container, runId, taskId, dir); + await ensureTaskDir(source.container, runId, taskId, dir, variantId); // Activation cases live in the nested activation sub-run; skills tasks in the // top-level run. Read the row from whichever run.json owns this task so the @@ -2042,11 +2108,13 @@ export async function readTaskDetail( const data = isActivationTaskId(taskId) ? await readActivationRunJson(runId, source) : await readRunJson(runId, source); - // Repeated runs share a task_id, so match on (task_id, replicate_index). - // Legacy rows carry no replicate_index (null) → treated as replicate 0, so - // an old single-result run still resolves at replicate 0. - const matches = (data?.task_results ?? []).filter( - (t) => t.task_id === taskId, + // A run with variants repeats a task_id once per arm and a repeated run + // repeats it once per replicate, so the row is only identified by all three + // of (task_id, variant_id, replicate_index). Legacy rows carry neither + // variant_id nor replicate_index (null) → treated as the default arm at + // replicate 0, so an old single-result run still resolves. + const matches = (data?.task_results ?? []).filter((t) => + rowMatches(t, taskId, variantId), ); const rawTask = matches.find((t) => (t.replicate_index ?? 0) === replicate) ?? @@ -2054,7 +2122,7 @@ export async function readTaskDetail( if (!rawTask) return null; const row = toTaskRow(rawTask); - const taskDir = taskContentBase(runId, taskId, source); + const taskDir = taskContentBase(runId, taskId, variantId, source); const contentDir = await resolveTaskContentDir(taskDir, replicate); const task = await readJson<{ final_status?: string; @@ -2286,14 +2354,16 @@ export async function readLogTail( replicate = 0, maxBytes = 200_000, source: Source = DEFAULT_SOURCE, + variantId: string = DEFAULT_VARIANT_ID, ): Promise { await ensureTaskDir( source.container, runId, taskId, runsDirFor(RUNS_DIR, source), + variantId, ); - const taskDir = taskContentBase(runId, taskId, source); + const taskDir = taskContentBase(runId, taskId, variantId, source); const contentDir = await resolveTaskContentDir(taskDir, replicate); const logPath = path.join(contentDir, "task.log"); const raw = await fs.readFile(logPath, "utf-8").catch(() => ""); @@ -2317,14 +2387,16 @@ export async function readConversationLog( replicate = 0, maxBytes = 200_000, source: Source = DEFAULT_SOURCE, + variantId: string = DEFAULT_VARIANT_ID, ): Promise { await ensureTaskDir( source.container, runId, taskId, runsDirFor(RUNS_DIR, source), + variantId, ); - const taskDir = taskContentBase(runId, taskId, source); + const taskDir = taskContentBase(runId, taskId, variantId, source); const contentDir = await resolveTaskContentDir(taskDir, replicate); const logPath = path.join(contentDir, "conversation.log"); const raw = await fs.readFile(logPath, "utf-8").catch(() => ""); @@ -2363,7 +2435,7 @@ export function parseConversation(raw: string): ConversationTurn[] { return turns; } -// Collect every file under a task's folder (`default//`) for the +// Collect every file under a task's folder (`//`) for the // download-as-zip button on the task page. Reuses walkArtifacts so the same // noise filter (`.venv`, `node_modules`, `*.pyc`, lockfiles, secrets) and // symlink skip that drive the Artifacts list also shape the zip — plus @@ -2373,15 +2445,18 @@ export async function collectTaskFiles( runId: string, taskId: string, source: Source = DEFAULT_SOURCE, + variantId: string = DEFAULT_VARIANT_ID, ): Promise<{ relPath: string; abs: string }[] | null> { if (!isValidId(runId) || !isValidTaskId(taskId)) return null; + if (!isValidVariantId(variantId)) return null; await ensureTaskDir( source.container, runId, taskId, runsDirFor(RUNS_DIR, source), + variantId, ); - const taskDir = taskContentBase(runId, taskId, source); + const taskDir = taskContentBase(runId, taskId, variantId, source); const refs = await walkArtifacts(taskDir); if (refs.length === 0) return null; return refs.map((r) => ({ relPath: r.relPath, abs: path.join(taskDir, r.relPath) })); @@ -2411,13 +2486,16 @@ export async function resolveSafePath( ): Promise { if (!isValidId(runId)) return null; const dir = runsDirFor(RUNS_DIR, source); - // Artifact URLs embed the task subdir in relPath - // (`default//artifacts/...`) — extract it so the narrow fetch - // hits the right blobs without pulling the whole run. + // Artifact URLs embed the variant + task subdirs in relPath + // (`//artifacts/...`) — extract them so the narrow + // fetch hits the right blobs without pulling the whole run. `activation` is + // excluded because its prefix nests one level deeper + // (`activation/default//…`), so parts[0..1] are not (variant, task) + // there; that case falls through to the run-summary fetch as it always did. const parts = relPath.split("/"); - if (parts[0] === "default" && parts[1]) { + if (parts[0] !== "activation" && isValidVariantId(parts[0]) && parts[1]) { if (!isValidId(parts[1])) return null; - await ensureTaskDir(source.container, runId, parts[1], dir); + await ensureTaskDir(source.container, runId, parts[1], dir, parts[0]); } else { await ensureRunSummary(source.container, runId, dir); } diff --git a/evalboard/lib/status.ts b/evalboard/lib/status.ts index efff8aff..f2b02520 100644 --- a/evalboard/lib/status.ts +++ b/evalboard/lib/status.ts @@ -8,6 +8,8 @@ // (e.g. StatusPill) also handles flow execution statuses like "Completed" // and "Faulted" and uses its own logic. +import { taskVariantKey } from "./variants"; + export type StatusCategory = "passed" | "failed" | "error" | "unknown"; export function statusCategory(status: string | null): StatusCategory { @@ -23,16 +25,18 @@ export function isPassStatus(status: string | null): boolean { return statusCategory(status) === "passed"; } -// Roll per-replicate rows up per task: taskId -> number of replicates that -// passed. Repeated runs share a taskId, so this is the one place the "any +// Roll per-replicate rows up per (variant, task): key -> number of replicates +// that passed. Repeated runs share a taskId, so this is the one place the "any // replicate passed" aggregation lives — consumed by the run-page pass-rate -// tile AND the grid badge / collapse so they can never disagree. +// tile AND the grid badge / collapse so they can never disagree. Key the lookup +// with taskVariantKey. export function perTaskPassCounts< - T extends { taskId: string; status: string | null }, + T extends { taskId: string; variantId?: string | null; status: string | null }, >(rows: readonly T[]): Map { const m = new Map(); for (const r of rows) { - m.set(r.taskId, (m.get(r.taskId) ?? 0) + (isPassStatus(r.status) ? 1 : 0)); + const k = taskVariantKey(r); + m.set(k, (m.get(k) ?? 0) + (isPassStatus(r.status) ? 1 : 0)); } return m; } diff --git a/evalboard/lib/variants.ts b/evalboard/lib/variants.ts new file mode 100644 index 00000000..b41763ac --- /dev/null +++ b/evalboard/lib/variants.ts @@ -0,0 +1,52 @@ +// Variant (experiment arm) semantics, in one dependency-free module. +// +// coder_eval writes each arm to its own subtree — //// — +// and stamps `variant_id` on every run.json row. An experiment declaring no +// variants still writes one arm named DEFAULT_VARIANT_ID, so single-arm and +// multi-arm runs have the same shape on disk. +// +// Imports nothing on purpose: the grid and run view are client components and +// cannot reach into lib/blob.ts (node:fs, the Azure SDK) for these. + +export const DEFAULT_VARIANT_ID = "default"; + +// A variant id is exactly ONE path segment, so it is held to a stricter rule +// than a task id, which may nest (`/` from dataset expansion). +// Mirrors coder_eval's reports_junit._is_safe_component. Restated rather than +// imported from lib/blob.ts to keep this module free of node built-ins. +const VARIANT_ID_RE = /^[\w.-]+$/; + +export function isValidVariantId(id: unknown): id is string { + return ( + typeof id === "string" && + id.length > 0 && + id.length < 128 && + VARIANT_ID_RE.test(id) && + id !== "." && + id !== ".." + ); +} + +// A grid row's identity is the (variant, task) pair, not the task id: folding +// arms together would let one arm's pass mask the other's failure. +// +// Length-prefixed rather than joined on a separator, because both ids arrive +// straight off untyped run.json — "neither can contain the separator" would be +// an assumption about data this function never sees. +export function taskVariantKey(row: { + taskId: string; + variantId?: string | null; +}): string { + const v = row.variantId ?? DEFAULT_VARIANT_ID; + return `${v.length}:${v}/${row.taskId}`; +} + +// The distinct arms in a set of rows, sorted for stable rendering. Length <= 1 +// is what every variant affordance in the UI hides behind. +export function variantsOf( + rows: readonly { variantId?: string | null }[], +): string[] { + const s = new Set(); + for (const r of rows) s.add(r.variantId ?? DEFAULT_VARIANT_ID); + return [...s].sort(); +}