diff --git a/evalboard/README.md b/evalboard/README.md index 3596d6ff..1ff33795 100644 --- a/evalboard/README.md +++ b/evalboard/README.md @@ -36,14 +36,18 @@ 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 `//`, where `` is the +experiment arm — `default` for a single-config run, or the arm name (e.g. +`opus`, `with-skill`) in an A/B run. The task page selects the arm via `?v=` +(mirroring `?r=` for replicates); a bare URL resolves the run's actual arm. ## Conventions - `/api/file?run=&path=` serves `.flow`, `.uipx`, etc. with path-traversal guard (`resolveSafePath`). -- `/api/download?run=[&task=]` streams a zip of a task folder (with - `task`) or the whole run (without). Files are gathered by `collectTaskFiles` +- `/api/download?run=[&task=][&v=]` streams a zip of a task + folder (with `task`; `v` selects the arm, default `default`) or the whole run + (without `task`). 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 diff --git a/evalboard/app/api/download/__tests__/route.test.ts b/evalboard/app/api/download/__tests__/route.test.ts new file mode 100644 index 00000000..f61fe7b7 --- /dev/null +++ b/evalboard/app/api/download/__tests__/route.test.ts @@ -0,0 +1,73 @@ +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"; + +// collectTaskFiles reads RUNS_DIR, resolved from EVALBOARD_LOCAL_RUNS_DIR at +// import time — so stub the env to a throwaway runs dir and import a fresh +// module copy (like collect.test.ts / the refresh route test). +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 loadGet() { + vi.resetModules(); + vi.stubEnv("EVALBOARD_LOCAL_RUNS_DIR", tmp); + return (await import("../route")).GET; +} + +function get(qs: string): Request { + return new Request(`http://test/api/download?${qs}`, { method: "GET" }); +} + +const RUN = "2026-01-01_00-00-00"; + +beforeEach(async () => { + tmp = await fs.mkdtemp(path.join(os.tmpdir(), "evalboard-download-")); + // A/B task: only under the glm-5-2 arm (no default/ subtree). + await write(`${RUN}/glm-5-2/ab-task/00/task.json`, "{}"); + await write(`${RUN}/glm-5-2/ab-task/00/artifacts/out.txt`, "glm out"); + // Single-config task: under default/. + await write(`${RUN}/default/solo-task/00/task.json`, "{}"); +}); + +afterEach(async () => { + vi.unstubAllEnvs(); + await fs.rm(tmp, { recursive: true, force: true }); +}); + +describe("GET /api/download — variant (?v=) wiring", () => { + test("zips the requested arm's subtree", async () => { + const GET = await loadGet(); + const res = await GET(get(`run=${RUN}&task=ab-task&v=glm-5-2`)); + expect(res.status).toBe(200); + expect(res.headers.get("Content-Type")).toBe("application/zip"); + // Non-empty archive → the arm's files were found. (Dropping the variant + // arg in the route would look in default/ab-task, which doesn't exist, + // and 404 — this assertion kills that mutation.) + expect(Number(res.headers.get("Content-Length"))).toBeGreaterThan(0); + }); + + test("a single-config task downloads with no ?v (default arm)", async () => { + const GET = await loadGet(); + const res = await GET(get(`run=${RUN}&task=solo-task`)); + expect(res.status).toBe(200); + expect(res.headers.get("Content-Type")).toBe("application/zip"); + }); + + test("an unknown arm 404s rather than zipping the wrong subtree", async () => { + const GET = await loadGet(); + const res = await GET(get(`run=${RUN}&task=ab-task&v=nope`)); + expect(res.status).toBe(404); + }); + + test("missing run -> 400", async () => { + const GET = await loadGet(); + const res = await GET(get(`task=ab-task`)); + expect(res.status).toBe(400); + }); +}); 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/api/refresh/__tests__/route.test.ts b/evalboard/app/api/refresh/__tests__/route.test.ts index cb06eb4d..c3bbf587 100644 --- a/evalboard/app/api/refresh/__tests__/route.test.ts +++ b/evalboard/app/api/refresh/__tests__/route.test.ts @@ -87,9 +87,10 @@ describe("POST /api/refresh", () => { }); test('traversal id ".." -> 400, cache root untouched', async () => { - // ".." passes isValidId (dots are word-ish) but clearRunCacheDir's - // strict-child check rejects it, so the route returns 400 and the - // cache root is never the rm target. + // isValidId now rejects "." / ".." outright, so clearRunCacheDir + // returns false and the route 400s — and even if that guard were + // loosened, its strict-child check still refuses to rm the cache + // root. Belt and suspenders; the cache root is never the rm target. const marker = path.join(cache, "keep.txt"); await fs.writeFile(marker, "x"); diff --git a/evalboard/app/runs/[id]/[...task]/page.tsx b/evalboard/app/runs/[id]/[...task]/page.tsx index 4676d04d..cbf028b5 100644 --- a/evalboard/app/runs/[id]/[...task]/page.tsx +++ b/evalboard/app/runs/[id]/[...task]/page.tsx @@ -9,6 +9,7 @@ import { replicateDirName, } from "@/lib/runs"; import { readTaskReview } from "@/lib/reviews"; +import { DEFAULT_VARIANT, firstParam, variantLinkParam } from "@/lib/variant"; import { fmtCompact, fmtRunTime, humanizeTaskId } from "@/lib/format"; import { StatusPill } from "@/lib/pills"; import { ChipButton } from "../chips"; @@ -33,40 +34,54 @@ export default async function TaskPage({ searchParams, }: { params: Promise<{ id: string; task: string[] }>; - searchParams: Promise<{ r?: string }>; + // Next yields `string | string[]` for a repeated query key; declare it + // honestly and normalize with firstParam so `?v=a&v=b` can't reach a reader + // as an array (which would throw a 500 at path.join). + searchParams: Promise<{ r?: string | string[]; v?: string | 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 // open. Absent / non-numeric / negative → replicate 0 (the single result a // non-repeated or legacy run has). - const parsedR = Number(r); + const parsedR = Number(firstParam(r)); const replicate = - r != null && Number.isInteger(parsedR) && parsedR >= 0 ? parsedR : 0; - const task = await readTaskDetail(id, taskId, replicate); + Number.isInteger(parsedR) && parsedR >= 0 ? parsedR : 0; + // Variant (arm) from ?v=NAME — in an A/B run several variants share this task + // path, so ?v selects which arm's / subdir to open. When ABSENT, + // readTaskDetail resolves the run's actual arm (single-arm tasks just work; + // this is what keeps pre-existing ?v-less deep links from 404-ing). The + // resolved arm comes back on task.variant and drives every other reader. + const task = await readTaskDetail(id, taskId, replicate, firstParam(v)); if (!task) notFound(); + const variant = task.variant ?? DEFAULT_VARIANT; - // 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 arm. "default" is the implicit + // fallback, so it's omitted to keep single-config URLs clean. + const variantParam = variantLinkParam(variant); + return (