From 94758acc80dc7e09003da55b461ca7b431be0ad3 Mon Sep 17 00:00:00 2001 From: CarlesUIPath Date: Mon, 27 Jul 2026 18:23:23 +0100 Subject: [PATCH 1/3] fix(evalboard): average per-task metrics across repeats in the run grid The run page task-grid collapsed a task's replicates to a single representative row and displayed that one run's metrics verbatim, so the Cost column (and score/duration/turns/tokens) showed the representative replicate's value instead of the average over the repeats. Add collapseReplicates() to lib/status.ts: it keeps the representative only for categorical fields (status pill, ?r=NN detail link, tags/skill/ model) and averages the quantitative columns across all replicates. With repeats disabled it's a no-op. Round the now-fractional turns display to at most 2 decimals (dropping trailing zeros) in fmtTurnsCount. Co-Authored-By: Claude Opus 4.8 (1M context) --- evalboard/app/runs/[id]/task-grid.tsx | 36 ++++------- evalboard/lib/__tests__/status.test.ts | 82 ++++++++++++++++++++++++++ evalboard/lib/__tests__/turns.test.ts | 6 ++ evalboard/lib/status.ts | 73 +++++++++++++++++++++++ evalboard/lib/turns.ts | 5 +- 5 files changed, 176 insertions(+), 26 deletions(-) create mode 100644 evalboard/lib/__tests__/status.test.ts diff --git a/evalboard/app/runs/[id]/task-grid.tsx b/evalboard/app/runs/[id]/task-grid.tsx index 20fc07b5..eb73d061 100644 --- a/evalboard/app/runs/[id]/task-grid.tsx +++ b/evalboard/app/runs/[id]/task-grid.tsx @@ -14,7 +14,11 @@ import { MaturePill, StatusPill, } from "@/lib/pills"; -import { isPassStatus, perTaskPassCounts, statusSortRank } from "@/lib/status"; +import { + collapseReplicates, + perTaskPassCounts, + statusSortRank, +} from "@/lib/status"; import { displayedTurns, fmtTurnsCount, @@ -506,30 +510,12 @@ export function TaskGrid({ // 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. - const collapsed = useMemo(() => { - const byTask = new Map(); - for (const t of tasks) { - const cur = byTask.get(t.taskId); - if (!cur) { - byTask.set(t.taskId, 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); - } else if ((t.replicateIndex ?? 0) < (cur.replicateIndex ?? 0)) { - byTask.set(t.taskId, t); - } - } - return [...byTask.values()]; - }, [tasks]); + // selectable on the task page. The status pill and detail link come from a + // representative replicate (a passing one when any passed, else the + // lowest-index one), while the quantitative columns (score, duration, cost, + // turns, tokens) are averaged across all repeats — see collapseReplicates. + // Collapse BEFORE sorting so a metric-sorted view still shows one row per task. + const collapsed = useMemo(() => collapseReplicates(tasks), [tasks]); const sorted = useMemo(() => { const arr = [...collapsed]; diff --git a/evalboard/lib/__tests__/status.test.ts b/evalboard/lib/__tests__/status.test.ts new file mode 100644 index 00000000..c0534f33 --- /dev/null +++ b/evalboard/lib/__tests__/status.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, test } from "vitest"; +import { collapseReplicates } from "../status"; +import type { TaskResultSummary } from "../runs"; + +function row( + taskId: string, + extra: Partial = {}, +): TaskResultSummary { + return { + taskId, + replicateIndex: 0, + status: "SUCCESS", + weightedScore: 1.0, + durationSeconds: 1.0, + totalCostUsd: 0.1, + actualCommands: 1, + totalTurns: null, + expectedTurns: null, + hasFinalReply: false, + inputTokens: null, + outputTokens: null, + cacheCreationTokens: null, + cacheReadTokens: null, + model: null, + tags: [], + skill: null, + matureSkipped: false, + ...extra, + }; +} + +describe("collapseReplicates", () => { + test("averages the numeric columns across a task's replicates", () => { + const [collapsed] = collapseReplicates([ + row("t", { replicateIndex: 0, totalCostUsd: 0.1, durationSeconds: 10, weightedScore: 0.9, actualCommands: 4 }), + row("t", { replicateIndex: 1, totalCostUsd: 0.2, durationSeconds: 20, weightedScore: 0.6, actualCommands: 8 }), + row("t", { replicateIndex: 2, totalCostUsd: 0.3, durationSeconds: 30, weightedScore: 0.3, actualCommands: 6 }), + ]); + // Cost is the MEAN of the three repeats — not the first run's 0.1. + expect(collapsed.totalCostUsd).toBeCloseTo(0.2, 10); + expect(collapsed.durationSeconds).toBeCloseTo(20, 10); + expect(collapsed.weightedScore).toBeCloseTo(0.6, 10); + expect(collapsed.actualCommands).toBeCloseTo(6, 10); + }); + + test("keeps categorical fields from the representative (passing replicate wins, then lowest index)", () => { + const [collapsed] = collapseReplicates([ + row("t", { replicateIndex: 0, status: "FAILURE" }), + row("t", { replicateIndex: 1, status: "SUCCESS" }), + ]); + // Representative is the passing replicate (index 1): status + detail link. + expect(collapsed.status).toBe("SUCCESS"); + expect(collapsed.replicateIndex).toBe(1); + }); + + test("single replicate passes through unchanged (repeats disabled)", () => { + const only = row("t", { replicateIndex: 0, totalCostUsd: 0.42, actualCommands: 3 }); + const [collapsed] = collapseReplicates([only]); + expect(collapsed.totalCostUsd).toBe(0.42); + expect(collapsed.actualCommands).toBe(3); + expect(collapsed.status).toBe("SUCCESS"); + }); + + test("averages over non-null values; all-null stays null", () => { + const [collapsed] = collapseReplicates([ + row("t", { replicateIndex: 0, totalCostUsd: 0.1, outputTokens: null }), + row("t", { replicateIndex: 1, totalCostUsd: null, outputTokens: null }), + ]); + // 0.1 averaged over the single non-null value; all-null column → null. + expect(collapsed.totalCostUsd).toBeCloseTo(0.1, 10); + expect(collapsed.outputTokens).toBeNull(); + }); + + test("one row per task, preserving first-seen order", () => { + const out = collapseReplicates([ + row("b", { replicateIndex: 0 }), + row("a", { replicateIndex: 0 }), + row("b", { replicateIndex: 1 }), + ]); + expect(out.map((r) => r.taskId)).toEqual(["b", "a"]); + }); +}); diff --git a/evalboard/lib/__tests__/turns.test.ts b/evalboard/lib/__tests__/turns.test.ts index 017e4f28..5397d554 100644 --- a/evalboard/lib/__tests__/turns.test.ts +++ b/evalboard/lib/__tests__/turns.test.ts @@ -133,4 +133,10 @@ describe("fmtTurnsCount", () => { test("renders zero as 0 (not em dash)", () => { expect(fmtTurnsCount(0)).toBe("0"); }); + + test("caps a fractional average at 2 decimals, dropping trailing zeros", () => { + expect(fmtTurnsCount(15.3333333333)).toBe("15.33"); + expect(fmtTurnsCount(6.5)).toBe("6.5"); + expect(fmtTurnsCount(6.999)).toBe("7"); // rounds to a whole count + }); }); diff --git a/evalboard/lib/status.ts b/evalboard/lib/status.ts index efff8aff..89ae28b1 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 type { TaskResultSummary } from "./runs"; + export type StatusCategory = "passed" | "failed" | "error" | "unknown"; export function statusCategory(status: string | null): StatusCategory { @@ -37,6 +39,77 @@ export function perTaskPassCounts< return m; } +// Mean of the non-null values, or null when every value is null (so the cell +// renders "—" instead of a misleading 0). The averaging primitive behind the +// replicate collapse below. +function meanOrNull(values: readonly (number | null)[]): number | null { + let sum = 0; + let n = 0; + for (const v of values) { + if (v != null) { + sum += v; + n += 1; + } + } + return n ? sum / n : null; +} + +// Collapse per-replicate rows to one row per task for the run grid. Repeated +// runs of a task share a taskId, so this returns a single row per task whose: +// - categorical fields (status, replicateIndex/detail link, tags, skill, +// model, expected_turns, mature flag) come from a REPRESENTATIVE replicate — +// a passing one when any passed, else the lowest-index one — so the status +// pill and the "open detail" link both describe one real run; and +// - quantitative columns (score, duration, cost, turns via actualCommands, +// tokens) are the MEAN across ALL replicates, so the grid reflects the whole +// repeat set rather than just the representative run. (Previously every +// column was the representative's own value, so e.g. cost showed a single +// run's price instead of the average over the repeats.) +// First-seen task order is preserved. With repeats disabled (one replicate per +// task) each mean is that single value, so the output is byte-identical. +export function collapseReplicates( + rows: readonly TaskResultSummary[], +): TaskResultSummary[] { + const groups = new Map(); + for (const t of rows) { + const g = groups.get(t.taskId); + if (g) g.push(t); + else groups.set(t.taskId, [t]); + } + const out: TaskResultSummary[] = []; + for (const group of groups.values()) { + // Representative for the categorical fields: a passing replicate wins + // over a non-passing one; ties break to the lowest replicateIndex. + let rep = group[0]; + for (const t of group) { + const repPass = isPassStatus(rep.status); + const tPass = isPassStatus(t.status); + if (repPass !== tPass) { + if (tPass) rep = t; + } else if ((t.replicateIndex ?? 0) < (rep.replicateIndex ?? 0)) { + rep = t; + } + } + out.push({ + ...rep, + weightedScore: meanOrNull(group.map((t) => t.weightedScore)), + durationSeconds: meanOrNull(group.map((t) => t.durationSeconds)), + totalCostUsd: meanOrNull(group.map((t) => t.totalCostUsd)), + // Turns render from displayedTurns(actualCommands, hasFinalReply); + // averaging the command count carries the average into that column. + actualCommands: meanOrNull(group.map((t) => t.actualCommands)), + totalTurns: meanOrNull(group.map((t) => t.totalTurns)), + inputTokens: meanOrNull(group.map((t) => t.inputTokens)), + outputTokens: meanOrNull(group.map((t) => t.outputTokens)), + cacheCreationTokens: meanOrNull( + group.map((t) => t.cacheCreationTokens), + ), + cacheReadTokens: meanOrNull(group.map((t) => t.cacheReadTokens)), + }); + } + return out; +} + // Default table sort: failures and errors first, unknowns next, passes last. export function statusSortRank(status: string | null): number { const c = statusCategory(status); diff --git a/evalboard/lib/turns.ts b/evalboard/lib/turns.ts index 1eabd030..c4e831f7 100644 --- a/evalboard/lib/turns.ts +++ b/evalboard/lib/turns.ts @@ -73,7 +73,10 @@ export function turnsCellClasses(tint: TurnTint): string { } export function fmtTurnsCount(n: number | null): string { - return n == null ? "—" : `${n}`; + // Collapsed replicate rows carry an averaged (fractional) turn count, e.g. + // 15.333…; cap at 2 decimals and drop trailing zeros so whole counts still + // render as "7" (not "7.00") and a half stays "6.5". + return n == null ? "—" : `${Number(n.toFixed(2))}`; } // Fail a task's turn-budget check once its visible turns exceed the budget by From 54163033760d269e7c8a5b52586000d4ac0d6838 Mon Sep 17 00:00:00 2001 From: CarlesUIPath Date: Thu, 30 Jul 2026 14:27:38 +0100 Subject: [PATCH 2/3] fix(evalboard): surface per-model rows and the model name for multi-model runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dashboard had no concept of an experiment *variant*, so A/B (multi-model) runs were mis-rendered on two surfaces: - Grid: each model produces a task_result sharing the task_id, so the replicate collapse (keyed on task_id alone) folded every model into one row with metrics averaged ACROSS models — the distinct models weren't shown at all. - Detail: the per-task content path was hardcoded to a `default/` subdir, which doesn't exist for a named variant (`kimi-k3/…`), so opening any multi-model task 404'd. Make the variant a first-class dimension end to end: - Data model: capture run.json `variant_id` as TaskResultSummary.variant (null on legacy rows → treated as "default"). - Paths: thread a sanitized `variant` (default "default") through taskContentBase, ensureTaskDir, readTaskDetail (now also matches the row on variant), readTaskReplicates, readLogTail, readConversationLog, collectTaskFiles, resolveSafePath, and the download API. - Grid: collapse + pass-counts key on (taskId, variant) via a new taskGroupKey, so each model keeps its own row and its own metrics. Add a Model column shown only when a run has >1 distinct model, and carry the variant on detail links (?v=). - Detail page: parse ?v=, thread it to every reader, preserve it on the replicate selector + download link, and show a model chip in the header so a task always names the LLM it ran on (single-model runs included). Single-config runs carry variant "default", so their grid, links and paths are byte-for-behavior unchanged (verified: no Model column, no ?v=, model chip still shown). Tests: variant grouping / taskGroupKey collision-safety / perTaskPassCounts, grid Model-column + ?v= link rendering, toTaskRow mapping. Co-Authored-By: Claude Opus 4.8 (1M context) --- evalboard/app/api/download/route.ts | 10 +- evalboard/app/runs/[id]/[...task]/page.tsx | 54 +++++++--- .../[id]/__tests__/run-view.render.test.tsx | 1 + .../app/runs/[id]/__tests__/run-view.test.ts | 1 + .../runs/[id]/__tests__/task-grid.test.tsx | 68 ++++++++++++ evalboard/app/runs/[id]/task-grid.tsx | 79 +++++++++++--- evalboard/lib/__tests__/runs.test.ts | 15 +++ evalboard/lib/__tests__/status.test.ts | 69 +++++++++++- evalboard/lib/blob.ts | 15 ++- evalboard/lib/runs.ts | 101 +++++++++++++----- evalboard/lib/status.ts | 52 ++++++--- 11 files changed, 389 insertions(+), 76 deletions(-) diff --git a/evalboard/app/api/download/route.ts b/evalboard/app/api/download/route.ts index 87b13e79..083631ee 100644 --- a/evalboard/app/api/download/route.ts +++ b/evalboard/app/api/download/route.ts @@ -6,20 +6,24 @@ 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=] → just that task's folder +// (//, variant default "default") +// ?run= → the entire run folder (run.json + every task dir) // minus the usual scaffolding noise. In blob mode the collect* helpers fetch // the needed blobs first, so this mirrors what the page would load. export async function GET(req: Request) { const url = new URL(req.url); const runId = url.searchParams.get("run"); const taskId = url.searchParams.get("task"); + // Which A/B variant's copy of the task to zip. Absent → "default" (the + // single-config subdir), so single-model download links are unchanged. + const variant = url.searchParams.get("v") ?? undefined; if (!runId) { return new NextResponse("missing run", { status: 400 }); } const files = taskId - ? await collectTaskFiles(runId, taskId) + ? await collectTaskFiles(runId, taskId, variant) : await collectRunFiles(runId); if (!files) { return new NextResponse("not found", { status: 404 }); diff --git a/evalboard/app/runs/[id]/[...task]/page.tsx b/evalboard/app/runs/[id]/[...task]/page.tsx index 4676d04d..c0db59b9 100644 --- a/evalboard/app/runs/[id]/[...task]/page.tsx +++ b/evalboard/app/runs/[id]/[...task]/page.tsx @@ -33,10 +33,10 @@ export default async function TaskPage({ searchParams, }: { params: Promise<{ id: string; task: string[] }>; - searchParams: Promise<{ r?: string }>; + searchParams: Promise<{ r?: string; v?: string }>; }) { const { id, task: taskSegments } = await params; - const { r } = await searchParams; + const { r, v } = await searchParams; const taskId = taskSegments.join("/"); // Replicate index from ?r=NN — repeated runs of one task share this task // path, so the query param is what selects which replicate's / dir to @@ -45,28 +45,41 @@ 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); + // Variant (model) from ?v=NAME — in a multi-model (A/B) run several variants + // share this task path, so ?v selects which model's / subdir to + // open. Absent → "default" (the single-config subdir); the readers sanitize + // and fall back to "default" for anything unsafe or unknown. + const variant = v ?? "default"; + const task = await readTaskDetail(id, taskId, replicate, variant); 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); + // Replicate indices available for this task/variant — drives the run + // selector below. [0] (or fewer) for a non-repeated task, so it self-hides. + const replicates = await readTaskReplicates(id, taskId, variant); - // variant is always "default" here; the replicate selects the / dir. + // The replicate selects the / dir within the variant's subtree. // readTaskReview returns null for older runs that predate the review feature. const review = await readTaskReview( id, - "default", + variant, taskId, replicateDirName(replicate), ); - const log = await readLogTail(id, taskId, replicate); + const log = await readLogTail(id, taskId, replicate, variant); const conversation = parseConversation( - await readConversationLog(id, taskId, replicate), + await readConversationLog(id, taskId, replicate, variant), ); const { flowDebug } = task; + // Preserve ?v= on in-page links (replicate selector, download) so switching + // replicates or downloading stays on the SAME model. "default" is the + // implicit fallback, so it's omitted to keep single-config URLs clean. + const variantParam = + variant && variant !== "default" + ? `&v=${encodeURIComponent(variant)}` + : ""; + return (
+ {showModel && ( + + {t.model ?? "—"} + + )} {t.matureSkipped ? ( @@ -819,7 +854,7 @@ export function TaskGrid({ ); return (
@@ -829,11 +864,13 @@ 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(taskGroupKey(t)) ?? 1 + } + replicatePassCount={ + replicatePassCounts.get( + taskGroupKey(t), + ) ?? 0 } - replicatePassCount={ - replicatePassCounts.get(t.taskId) ?? 0 - } /> {t.matureSkipped ? ( @@ -843,6 +880,14 @@ export function TaskGrid({ )}
+ {showModel && t.model && ( +
+ {t.model} +
+ )} { const row = toTaskRow({ task_id: "x", expected_turns: null }); expect(row.expectedTurns).toBeNull(); }); + + test("maps variant_id and model_used (multi-model row)", () => { + const row = toTaskRow({ + task_id: "x", + variant_id: "kimi-k3", + model_used: "moonshotai/kimi-k3", + }); + expect(row.variant).toBe("kimi-k3"); + expect(row.model).toBe("moonshotai/kimi-k3"); + }); + + test("legacy row without variant_id yields null variant", () => { + const row = toTaskRow({ task_id: "x" }); + expect(row.variant).toBeNull(); + }); }); describe("aggregateSubAgentUsage", () => { diff --git a/evalboard/lib/__tests__/status.test.ts b/evalboard/lib/__tests__/status.test.ts index c0534f33..9239a24c 100644 --- a/evalboard/lib/__tests__/status.test.ts +++ b/evalboard/lib/__tests__/status.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "vitest"; -import { collapseReplicates } from "../status"; +import { collapseReplicates, perTaskPassCounts, taskGroupKey } from "../status"; import type { TaskResultSummary } from "../runs"; function row( @@ -22,6 +22,7 @@ function row( cacheCreationTokens: null, cacheReadTokens: null, model: null, + variant: null, tags: [], skill: null, matureSkipped: false, @@ -79,4 +80,70 @@ describe("collapseReplicates", () => { ]); expect(out.map((r) => r.taskId)).toEqual(["b", "a"]); }); + + test("keeps a row PER MODEL — variants sharing a taskId are not merged", () => { + // A multi-model (A/B) run: three variants ran the SAME task. They share + // the taskId but are distinct models, so they must stay three rows with + // their OWN metrics — never averaged into one. + const out = collapseReplicates([ + row("t", { variant: "kimi-k3", model: "moonshotai/kimi-k3", totalCostUsd: 0.1 }), + row("t", { variant: "glm-5-2", model: "z-ai/glm-5.2", totalCostUsd: 0.2 }), + row("t", { variant: "deepseek-v4-pro", model: "deepseek/deepseek-v4-pro", totalCostUsd: 0.3 }), + ]); + expect(out).toHaveLength(3); + expect(out.map((r) => r.variant)).toEqual([ + "kimi-k3", + "glm-5-2", + "deepseek-v4-pro", + ]); + // Each row keeps its own cost — no cross-model averaging. + expect(out.map((r) => r.totalCostUsd)).toEqual([0.1, 0.2, 0.3]); + }); + + test("still averages replicates WITHIN one variant of a multi-model run", () => { + const out = collapseReplicates([ + row("t", { variant: "a", replicateIndex: 0, totalCostUsd: 0.1 }), + row("t", { variant: "a", replicateIndex: 1, totalCostUsd: 0.3 }), + row("t", { variant: "b", replicateIndex: 0, totalCostUsd: 1.0 }), + ]); + expect(out).toHaveLength(2); + const a = out.find((r) => r.variant === "a")!; + const b = out.find((r) => r.variant === "b")!; + expect(a.totalCostUsd).toBeCloseTo(0.2, 10); // mean of a's two replicates + expect(b.totalCostUsd).toBeCloseTo(1.0, 10); + }); +}); + +describe("taskGroupKey", () => { + test("a null variant collapses to the same key as an explicit 'default'", () => { + expect(taskGroupKey({ taskId: "t", variant: null })).toBe( + taskGroupKey({ taskId: "t", variant: "default" }), + ); + }); + + test("different variants of the same task yield different keys", () => { + expect(taskGroupKey({ taskId: "t", variant: "kimi-k3" })).not.toBe( + taskGroupKey({ taskId: "t", variant: "glm-5-2" }), + ); + }); + + test("the separator cannot be forged from task-id/variant collisions", () => { + // "ab"+"c" must not equal "a"+"bc" — the control-char separator can't + // appear in an id, so no two distinct (task, variant) pairs collide. + expect(taskGroupKey({ taskId: "ab", variant: "c" })).not.toBe( + taskGroupKey({ taskId: "a", variant: "bc" }), + ); + }); +}); + +describe("perTaskPassCounts", () => { + test("counts each variant of a shared task separately", () => { + const m = perTaskPassCounts([ + row("t", { variant: "a", status: "SUCCESS" }), + row("t", { variant: "b", status: "FAILURE" }), + ]); + expect(m.size).toBe(2); + expect(m.get(taskGroupKey({ taskId: "t", variant: "a" }))).toBe(1); + expect(m.get(taskGroupKey({ taskId: "t", variant: "b" }))).toBe(0); + }); }); diff --git a/evalboard/lib/blob.ts b/evalboard/lib/blob.ts index da5a1e5e..e4c656eb 100644 --- a/evalboard/lib/blob.ts +++ b/evalboard/lib/blob.ts @@ -270,11 +270,16 @@ export async function ensureTaskDir( runId: string, taskId: string, destRoot: string, + // Experiment variant subdir. "default" for single-config runs; a model name + // (e.g. "kimi-k3") in A/B runs. Validated as a path segment so it can't + // escape the run prefix. + variant = "default", ): Promise { assertValidId(runId, "runId"); assertValidTaskId(taskId, "taskId"); + assertValidId(variant, "variant"); if (LOCAL_RUNS_DIR) return; - return dedupe(`task:${runId}/${taskId}`, async () => { + return dedupe(`task:${runId}/${variant}/${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. @@ -284,13 +289,13 @@ export async function ensureTaskDir( const c = await getContainer(); 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. const prefix = activation - ? `${runId}/activation/default/${taskId}/` - : `${runId}/default/${taskId}/`; + ? `${runId}/activation/${variant}/${taskId}/` + : `${runId}/${variant}/${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 c1d3f824..b5cd76e2 100644 --- a/evalboard/lib/runs.ts +++ b/evalboard/lib/runs.ts @@ -87,6 +87,13 @@ export interface TaskResultSummary { // Model the task ran on (run.json `model_used`). Used to price token // buckets as USD for the Tokens↔USD column toggle. Null on legacy runs. model: string | null; + // Experiment variant this row belongs to (run.json `variant_id`, e.g. + // "default" for a single-config run, or a model name like "kimi-k3" in an + // A/B run). Variant rows share a taskId but live in distinct on-disk subdirs + // (///), so this is what tells sibling models apart in the + // grid and selects the right content dir on the detail page. Null on legacy + // runs that predate the field — treated as "default" when building paths. + variant: string | null; tags: string[]; // Derived primary group. See deriveSkill below for the resolution chain // (new runs use task_path; older runs fall back to a tag heuristic). @@ -352,6 +359,11 @@ interface RawTaskResult { // Model the task ran on (e.g. "claude-sonnet-4-6"). Used to price token // buckets as USD. Absent on legacy runs. model_used?: string | null; + // Experiment variant that produced this row. In an A/B (multi-model) run + // each variant contributes a row sharing the task_id but differing here and + // in model_used, and its artifacts live under ///. + // Absent → single-config run whose content dir is the literal "default". + variant_id?: string | null; // Per-task agent config; `type` is the harness (coder-eval AgentKind, e.g. // "claude-code" | "codex" | "antigravity"). Used to derive the run's harness // when the run-level RunConfig stamp is absent (direct coder-eval / legacy runs). @@ -585,13 +597,31 @@ function isActivationTaskId(taskId: string): boolean { return taskId.startsWith("skill-activation/"); } +// The on-disk subdir a single-config run writes its tasks under. Multi-model +// (A/B) runs replace this with the variant id (e.g. "kimi-k3"); the fallback +// keeps legacy / single-config runs — whose rows carry no variant — resolving. +const DEFAULT_VARIANT = "default"; + +// Sanitize a variant into a safe single path segment. A variant id is reflected +// into a filesystem path and a blob prefix, so anything that isn't a plain id +// (null on legacy rows, or a would-be traversal) collapses to "default". +function variantSegment(variant: string | null | undefined): string { + return variant && isValidId(variant) ? variant : DEFAULT_VARIANT; +} + // Filesystem base for a task's content (before the optional `00` replicate dir): -// activation cases under /activation/default/, skills tasks under -// /default/. -function taskContentBase(runId: string, taskId: string): string { +// activation cases under /activation//, skills tasks under +// //. `variant` defaults to "default" — the subdir a +// single-config run uses and the safe fallback for legacy rows. +function taskContentBase( + runId: string, + taskId: string, + variant: string | null = DEFAULT_VARIANT, +): string { + const v = variantSegment(variant); return isActivationTaskId(taskId) - ? path.join(RUNS_DIR, runId, "activation", "default", taskId) - : path.join(RUNS_DIR, runId, "default", taskId); + ? path.join(RUNS_DIR, runId, "activation", v, taskId) + : path.join(RUNS_DIR, runId, v, taskId); } // Resolve the skill (primary grouping axis) for a task. Two-stage fallback: @@ -637,6 +667,7 @@ export function toTaskRow(t: RawTaskResult): TaskResultSummary { cacheCreationTokens: t.cache_creation_input_tokens ?? null, cacheReadTokens: t.cache_read_input_tokens ?? null, model: t.model_used ?? null, + variant: t.variant_id ?? null, tags, skill: deriveSkill(t.task_path, tags), matureSkipped: t.mature_skipped ?? false, @@ -1776,12 +1807,18 @@ async function resolveTaskContentDir( export async function readTaskReplicates( runId: string, taskId: string, + variant: string | null = DEFAULT_VARIANT, ): Promise { + const v = variantSegment(variant); const data = isActivationTaskId(taskId) ? await readActivationRunJson(runId) : await readRunJson(runId); + // Scope to the selected variant so the run selector on a multi-model task + // lists only THIS model's replicates, not every variant's. const indices = (data?.task_results ?? []) - .filter((t) => t.task_id === taskId) + .filter( + (t) => t.task_id === taskId && variantSegment(t.variant_id) === v, + ) .map((t) => t.replicate_index ?? 0); return [...new Set(indices)].sort((a, b) => a - b); } @@ -1790,8 +1827,10 @@ export async function readTaskDetail( runId: string, taskId: string, replicate = 0, + variant: string | null = DEFAULT_VARIANT, ): Promise { - await ensureTaskDir(runId, taskId, RUNS_DIR); + const v = variantSegment(variant); + await ensureTaskDir(runId, taskId, RUNS_DIR, v); // 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 @@ -1799,11 +1838,16 @@ export async function readTaskDetail( const data = isActivationTaskId(taskId) ? await readActivationRunJson(runId) : await readRunJson(runId); - // 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. + // Repeated runs share a task_id, so match on (task_id, replicate_index). In + // a multi-model run several variants ALSO share the task_id at replicate 0, + // so filter on the variant too (rows carrying a variant_id) — otherwise + // every model would resolve to the first variant's row. Legacy rows carry + // neither field (null variant / null replicate_index) → treated as + // ("default", 0), so an old single-result run still resolves. const matches = (data?.task_results ?? []).filter( - (t) => t.task_id === taskId, + (t) => + t.task_id === taskId && + variantSegment(t.variant_id) === v, ); const rawTask = matches.find((t) => (t.replicate_index ?? 0) === replicate) ?? @@ -1811,7 +1855,7 @@ export async function readTaskDetail( if (!rawTask) return null; const row = toTaskRow(rawTask); - const taskDir = taskContentBase(runId, taskId); + const taskDir = taskContentBase(runId, taskId, v); const contentDir = await resolveTaskContentDir(taskDir, replicate); const task = await readJson<{ final_status?: string; @@ -2046,10 +2090,12 @@ export async function readLogTail( runId: string, taskId: string, replicate = 0, + variant: string | null = DEFAULT_VARIANT, maxBytes = 200_000, ): Promise { - await ensureTaskDir(runId, taskId, RUNS_DIR); - const taskDir = taskContentBase(runId, taskId); + const v = variantSegment(variant); + await ensureTaskDir(runId, taskId, RUNS_DIR, v); + const taskDir = taskContentBase(runId, taskId, v); const contentDir = await resolveTaskContentDir(taskDir, replicate); const logPath = path.join(contentDir, "task.log"); const raw = await fs.readFile(logPath, "utf-8").catch(() => ""); @@ -2071,10 +2117,12 @@ export async function readConversationLog( runId: string, taskId: string, replicate = 0, + variant: string | null = DEFAULT_VARIANT, maxBytes = 200_000, ): Promise { - await ensureTaskDir(runId, taskId, RUNS_DIR); - const taskDir = taskContentBase(runId, taskId); + const v = variantSegment(variant); + await ensureTaskDir(runId, taskId, RUNS_DIR, v); + const taskDir = taskContentBase(runId, taskId, v); const contentDir = await resolveTaskContentDir(taskDir, replicate); const logPath = path.join(contentDir, "conversation.log"); const raw = await fs.readFile(logPath, "utf-8").catch(() => ""); @@ -2122,10 +2170,12 @@ export function parseConversation(raw: string): ConversationTurn[] { export async function collectTaskFiles( runId: string, taskId: string, + variant: string | null = DEFAULT_VARIANT, ): Promise<{ relPath: string; abs: string }[] | null> { if (!isValidId(runId) || !isValidTaskId(taskId)) return null; - await ensureTaskDir(runId, taskId, RUNS_DIR); - const taskDir = taskContentBase(runId, taskId); + const v = variantSegment(variant); + await ensureTaskDir(runId, taskId, RUNS_DIR, v); + const taskDir = taskContentBase(runId, taskId, v); const refs = await walkArtifacts(taskDir); if (refs.length === 0) return null; return refs.map((r) => ({ relPath: r.relPath, abs: path.join(taskDir, r.relPath) })); @@ -2151,13 +2201,16 @@ export async function resolveSafePath( relPath: string, ): Promise { if (!isValidId(runId)) return null; - // 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 task subdir in relPath as + // `//artifacts/...` — the variant is "default" for a + // single-config run and a model name (e.g. "kimi-k3") in an A/B run. + // Extract both so the narrow fetch hits the right blobs without pulling the + // whole run. Anything that isn't that two-segment task shape (run-level + // files, or the nested activation layout) falls back to the run summary. const parts = relPath.split("/"); - if (parts[0] === "default" && parts[1]) { - if (!isValidId(parts[1])) return null; - await ensureTaskDir(runId, parts[1], RUNS_DIR); + if (parts.length >= 2 && parts[0] !== "activation" && parts[1]) { + if (!isValidId(parts[0]) || !isValidId(parts[1])) return null; + await ensureTaskDir(runId, parts[1], RUNS_DIR, parts[0]); } else { await ensureRunSummary(runId, RUNS_DIR); } diff --git a/evalboard/lib/status.ts b/evalboard/lib/status.ts index 89ae28b1..e984410e 100644 --- a/evalboard/lib/status.ts +++ b/evalboard/lib/status.ts @@ -25,16 +25,36 @@ 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 +// Separator used to join a task id and its variant into one map key. A control +// character (unit separator) that can never appear in a task id or variant id, +// so the two segments can never collide. +const KEY_SEP = "\u001f"; + +// Grouping key for the "one row per task" collapse. A task's repeated runs +// (replicates) share it, so they fold together; but in a multi-model (A/B) run +// several variants ALSO share a taskId — they must stay DISTINCT rows, one per +// model — so the variant is part of the key. Single-config runs carry variant +// "default" (or null on legacy rows), so the key is effectively the taskId and +// behavior is unchanged. +export function taskGroupKey(t: { + taskId: string; + variant?: string | null; +}): string { + return `${t.taskId}${KEY_SEP}${t.variant ?? "default"}`; +} + +// Roll per-replicate rows up per (task, variant): key -> number of replicates +// that passed. Repeated runs share a key, 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. Keyed by +// taskGroupKey so a multi-model run counts each model's attempt separately. export function perTaskPassCounts< - T extends { taskId: string; status: string | null }, + T extends { taskId: string; status: string | null; variant?: 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 = taskGroupKey(r); + m.set(k, (m.get(k) ?? 0) + (isPassStatus(r.status) ? 1 : 0)); } return m; } @@ -54,27 +74,31 @@ function meanOrNull(values: readonly (number | null)[]): number | null { return n ? sum / n : null; } -// Collapse per-replicate rows to one row per task for the run grid. Repeated -// runs of a task share a taskId, so this returns a single row per task whose: +// Collapse per-replicate rows to one row per (task, variant) for the run grid. +// Repeated runs of a task share a taskId, so they fold together; a multi-model +// run keeps one row PER MODEL (the variant is part of the group key), so each +// model's metrics stay separate instead of being averaged across models. Each +// collapsed row's: // - categorical fields (status, replicateIndex/detail link, tags, skill, -// model, expected_turns, mature flag) come from a REPRESENTATIVE replicate — -// a passing one when any passed, else the lowest-index one — so the status -// pill and the "open detail" link both describe one real run; and +// model, variant, expected_turns, mature flag) come from a REPRESENTATIVE +// replicate — a passing one when any passed, else the lowest-index one — so +// the status pill and the "open detail" link both describe one real run; and // - quantitative columns (score, duration, cost, turns via actualCommands, // tokens) are the MEAN across ALL replicates, so the grid reflects the whole // repeat set rather than just the representative run. (Previously every // column was the representative's own value, so e.g. cost showed a single // run's price instead of the average over the repeats.) -// First-seen task order is preserved. With repeats disabled (one replicate per -// task) each mean is that single value, so the output is byte-identical. +// First-seen group order is preserved. With repeats disabled (one replicate per +// task/variant) each mean is that single value, so the output is byte-identical. export function collapseReplicates( rows: readonly TaskResultSummary[], ): TaskResultSummary[] { const groups = new Map(); for (const t of rows) { - const g = groups.get(t.taskId); + const key = taskGroupKey(t); + const g = groups.get(key); if (g) g.push(t); - else groups.set(t.taskId, [t]); + else groups.set(key, [t]); } const out: TaskResultSummary[] = []; for (const group of groups.values()) { From b5a4f02d3add296833fd3ff799806350b1a3c2aa Mon Sep 17 00:00:00 2001 From: CarlesUIPath Date: Thu, 30 Jul 2026 14:39:05 +0100 Subject: [PATCH 3/3] test(evalboard): cover variant path resolution; harden resolveSafePath traversal Add lib/__tests__/variant-paths.test.ts exercising the variant-aware readers against a temp local runs dir (the collect.test.ts env-stub + fresh-import harness): readTaskDetail picks the right model's row (not the first variant), returns null with no ?v on a run that has no "default" subdir, and sanitizes an unsafe ?v to "default"; readTaskReplicates / readLogTail / collectTaskFiles are scoped to the selected variant; resolveSafePath resolves a non-default variant artifact and rejects traversal. That last test caught a regression: the broadened resolveSafePath handed a "../.." segment to ensureTaskDir, which THROWS (isValidId admits dots), so the /api/file route would 500 instead of 403. Restrict the narrow-fetch prefetch to genuinely safe segments (reject "."/".."); traversal now falls through to the realpath containment check and returns null as before. Co-Authored-By: Claude Opus 4.8 (1M context) --- evalboard/lib/__tests__/variant-paths.test.ts | 151 ++++++++++++++++++ evalboard/lib/runs.ts | 13 +- 2 files changed, 160 insertions(+), 4 deletions(-) create mode 100644 evalboard/lib/__tests__/variant-paths.test.ts diff --git a/evalboard/lib/__tests__/variant-paths.test.ts b/evalboard/lib/__tests__/variant-paths.test.ts new file mode 100644 index 00000000..984356cf --- /dev/null +++ b/evalboard/lib/__tests__/variant-paths.test.ts @@ -0,0 +1,151 @@ +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"; + +// The variant-aware readers (readTaskDetail / readTaskReplicates / readLogTail / +// collectTaskFiles / resolveSafePath) all resolve a task's content under +// ////. RUNS_DIR is baked from EVALBOARD_LOCAL_RUNS_DIR +// at *import* time, so — like collect.test.ts — each test stubs the env to a +// throwaway runs dir and dynamically imports a fresh module copy so RUNS_DIR +// picks it up. LOCAL mode makes ensureTaskDir a no-op, so these exercise the +// pure on-disk path resolution deterministically. +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 RUN = "2026-01-01_00-00-00"; +const TASK = "demo-task"; + +// A multi-model (A/B) run: two variants ran the SAME task. They share the +// task_id at replicate 0 and differ only by variant_id / model_used — exactly +// the shape that used to collapse into one row and 404 on the detail page. +beforeEach(async () => { + tmp = await fs.mkdtemp(path.join(os.tmpdir(), "evalboard-variant-")); + await write( + `${RUN}/run.json`, + JSON.stringify({ + run_id: "x", + task_results: [ + { + task_id: TASK, + variant_id: "kimi-k3", + model_used: "moonshotai/kimi-k3", + replicate_index: 0, + status: "SUCCESS", + }, + { + task_id: TASK, + variant_id: "glm-5-2", + model_used: "z-ai/glm-5.2", + replicate_index: 0, + status: "FAILURE", + }, + // A second replicate of ONE variant, to prove replicate listing + // is scoped to the selected variant. + { + task_id: TASK, + variant_id: "kimi-k3", + model_used: "moonshotai/kimi-k3", + replicate_index: 1, + status: "SUCCESS", + }, + ], + }), + ); + await write(`${RUN}/kimi-k3/${TASK}/00/task.json`, "{}"); + await write(`${RUN}/kimi-k3/${TASK}/00/task.log`, "kimi log"); + await write(`${RUN}/kimi-k3/${TASK}/01/task.json`, "{}"); + await write(`${RUN}/glm-5-2/${TASK}/00/task.json`, "{}"); + await write(`${RUN}/glm-5-2/${TASK}/00/task.log`, "glm log"); + await write(`${RUN}/glm-5-2/${TASK}/00/artifacts/out.txt`, "glm artifact"); +}); + +afterEach(async () => { + vi.unstubAllEnvs(); + await fs.rm(tmp, { recursive: true, force: true }); +}); + +describe("readTaskDetail — variant selects the model's own row + content", () => { + test("?v selects the matching model (not just the first variant)", async () => { + const { readTaskDetail } = await loadRuns(); + const kimi = await readTaskDetail(RUN, TASK, 0, "kimi-k3"); + const glm = await readTaskDetail(RUN, TASK, 0, "glm-5-2"); + expect(kimi?.model).toBe("moonshotai/kimi-k3"); + expect(kimi?.variant).toBe("kimi-k3"); + expect(kimi?.status).toBe("SUCCESS"); + // The SECOND variant resolves to its OWN row — before the fix every + // variant collapsed onto the first one at replicate 0. + expect(glm?.model).toBe("z-ai/glm-5.2"); + expect(glm?.status).toBe("FAILURE"); + }); + + test("a run with no 'default' variant returns null when ?v is omitted", async () => { + // Mirrors the live behavior: the grid only ever links multi-model rows + // with ?v=, and there is no default/ subdir to fall back to. + const { readTaskDetail } = await loadRuns(); + expect(await readTaskDetail(RUN, TASK, 0)).toBeNull(); + }); + + test("an unsafe variant is sanitized to 'default' (no path escape)", async () => { + const { readTaskDetail } = await loadRuns(); + // "../glm-5-2" is not a valid id → falls back to "default", which has + // no row here → null. It must NOT traverse into the glm-5-2 subtree. + expect(await readTaskDetail(RUN, TASK, 0, "../glm-5-2")).toBeNull(); + }); +}); + +describe("readTaskReplicates — scoped to the selected variant", () => { + test("lists only the chosen model's replicates", async () => { + const { readTaskReplicates } = await loadRuns(); + expect(await readTaskReplicates(RUN, TASK, "kimi-k3")).toEqual([0, 1]); + expect(await readTaskReplicates(RUN, TASK, "glm-5-2")).toEqual([0]); + }); +}); + +describe("readLogTail — reads the variant's log", () => { + test("each variant's task.log is read from its own subdir", async () => { + const { readLogTail } = await loadRuns(); + expect(await readLogTail(RUN, TASK, 0, "kimi-k3")).toBe("kimi log"); + expect(await readLogTail(RUN, TASK, 0, "glm-5-2")).toBe("glm log"); + }); +}); + +describe("collectTaskFiles — zips the variant's folder", () => { + test("collects files under the requested variant only", async () => { + const { collectTaskFiles } = await loadRuns(); + const files = await collectTaskFiles(RUN, TASK, "glm-5-2"); + const rels = files?.map((f) => f.relPath).sort(); + expect(rels).toEqual(["00/artifacts/out.txt", "00/task.json", "00/task.log"]); + for (const f of files ?? []) { + expect(f.abs).toContain(path.join("glm-5-2", TASK)); + } + }); +}); + +describe("resolveSafePath — variant-prefixed artifact URLs", () => { + test("resolves a non-default variant artifact path", async () => { + const { resolveSafePath } = await loadRuns(); + const abs = await resolveSafePath( + RUN, + `glm-5-2/${TASK}/00/artifacts/out.txt`, + ); + expect(abs).not.toBeNull(); + expect(abs).toContain(path.join(RUN, "glm-5-2", TASK)); + }); + + test("still rejects traversal outside the run dir", async () => { + const { resolveSafePath } = await loadRuns(); + expect(await resolveSafePath(RUN, "../../etc/passwd")).toBeNull(); + }); +}); diff --git a/evalboard/lib/runs.ts b/evalboard/lib/runs.ts index b5cd76e2..c7be66ec 100644 --- a/evalboard/lib/runs.ts +++ b/evalboard/lib/runs.ts @@ -2205,11 +2205,16 @@ export async function resolveSafePath( // `//artifacts/...` — the variant is "default" for a // single-config run and a model name (e.g. "kimi-k3") in an A/B run. // Extract both so the narrow fetch hits the right blobs without pulling the - // whole run. Anything that isn't that two-segment task shape (run-level - // files, or the nested activation layout) falls back to the run summary. + // whole run. This is only a prefetch optimization; the realpath containment + // check below is the actual security boundary, so an input that isn't that + // clean two-segment task shape (run-level files, the nested activation + // layout, OR any traversal like "../..") just falls back to the run summary + // and lets the containment check reject it — we must never hand a "."/".." + // segment to ensureTaskDir, which throws on it (isValidId admits dots). const parts = relPath.split("/"); - if (parts.length >= 2 && parts[0] !== "activation" && parts[1]) { - if (!isValidId(parts[0]) || !isValidId(parts[1])) return null; + const safeSeg = (s: string | undefined): s is string => + !!s && isValidId(s) && s !== "." && s !== ".."; + if (parts[0] !== "activation" && safeSeg(parts[0]) && safeSeg(parts[1])) { await ensureTaskDir(runId, parts[1], RUNS_DIR, parts[0]); } else { await ensureRunSummary(runId, RUNS_DIR);