From 0af5049341acfb03e6ce74016202ae839066b79e Mon Sep 17 00:00:00 2001 From: thinmintdev Date: Mon, 10 Aug 2026 04:13:00 -0400 Subject: [PATCH] fix(bench): decode-history graph plotted prefill runs as decode points MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /v1/history filtered on model_id and lane only, but a model normally has pp (prefill) records alongside its tg (decode) ones under the same model_id and lane — and a pp record carries no decode figure at all. The run drawer's "decode history" sparkline was therefore plotting a series that was partly prefill runs. Measured against real CT105 data: MiniCPM5-1B returns 8 points for model+lane, only 4 of which belong on the graph. Different config_labels are the same apples-to-oranges problem — two configurations of a model, not successive measurements of one. The endpoint gains optional `kind` and `config` filters and the drawer pins both: `kind=tg`, and `config` to the row's own variant. Filters stay optional so the CLI-shaped ?cell_key= form is unaffected. Filtering by display dimensions rather than by cell_key is deliberate even though a cell is exactly "one comparable series": cell_key is content-addressed over engine/image provenance, so an unrelated runner-image bump between sweeps forks the key and shatters one continuous history into several one-point series. hal0's own dashboard moved off cell_key for this reason. cell_key remains available as an explicit filter. Missed in #63 — the endpoint's own tests seeded one record per model, so no test had a pp row to leak. Both filters are now covered. Co-Authored-By: Claude Opus 5 --- src/scripts/bench-island.ts | 33 +++++++++------ workers/bench-api/src/reads.ts | 31 +++++++++++++- workers/bench-api/test/reads.test.ts | 61 ++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 14 deletions(-) diff --git a/src/scripts/bench-island.ts b/src/scripts/bench-island.ts index 3539716..e1c759c 100644 --- a/src/scripts/bench-island.ts +++ b/src/scripts/bench-island.ts @@ -913,7 +913,7 @@ function init() { void resolveBundleLink(model.runId, model.cellKey); } if (model.mode === 'api' && model.identity.lane) { - void fetchRunHistory(id, model.identity.lane); + void fetchRunHistory(id, model.identity.lane, model.identity.variant); } if (opts.pushState !== false) { @@ -979,20 +979,29 @@ function init() { } } - // Decode-history graph: fetches the proposed /v1/history?model=&lane= - // endpoint (adapter-only today, see HISTORY_API_BASE's header comment) - // and, only on a fully successful + non-empty response, inserts the graph - // section right after #run-drawer-identity. Any failure — network error, - // non-2xx (including a 404 on a production API that doesn't have this - // route yet), non-JSON body, or a shape that normalizes to zero usable - // points — degrades silently: no section, no skeleton, no error text, per - // the same "the snapshot/existing content stays, nothing looks broken" - // principle the rest of this file follows for optional enhancements. - async function fetchRunHistory(modelId: string, lane: string) { + // Decode-history graph: fetches /v1/history and, only on a fully successful + // + non-empty response, inserts the graph section right after + // #run-drawer-identity. Any failure — network error, non-2xx (including a + // 404 on a deploy predating the route), non-JSON body, or a shape that + // normalizes to zero usable points — degrades silently: no section, no + // skeleton, no error text, per the same "the snapshot/existing content + // stays, nothing looks broken" principle the rest of this file follows for + // optional enhancements. + // + // The query pins the DISPLAY DIMENSIONS, not just model+lane. A model + // usually has pp (prefill) records alongside its tg (decode) ones under the + // same model_id and lane, and a pp record carries no decode figure at all — + // asking only by model+lane returns a "decode history" that is half prefill + // runs. `config` is pinned to the row's own variant for the same reason: + // two config_labels are different configurations, not successive + // measurements of one. + async function fetchRunHistory(modelId: string, lane: string, variant: string | null) { const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS); try { - const url = `${HISTORY_API_BASE}?model=${encodeURIComponent(modelId)}&lane=${encodeURIComponent(lane)}`; + const query = new URLSearchParams({ model: modelId, lane, kind: DEFAULT_WORKLOAD }); + if (variant) query.set('config', variant); + const url = `${HISTORY_API_BASE}?${query.toString()}`; const res = await fetch(url, { signal: controller.signal }); if (!res.ok) return; const json = await res.json(); diff --git a/workers/bench-api/src/reads.ts b/workers/bench-api/src/reads.ts index 9ea4239..09a1f5f 100644 --- a/workers/bench-api/src/reads.ts +++ b/workers/bench-api/src/reads.ts @@ -375,8 +375,25 @@ interface HistoryPointRow { * * Filter contract mirrors CT105's own /api/benchmarks/history — `cell_key` or * `model` is required (an unfiltered "history of everything" is a table scan - * with no caller), `lane` narrows further. The site calls it as - * ?model=&lane=; the CLI-shaped ?cell_key= form is supported for parity. + * with no caller); `lane`, `kind` and `config` narrow further. The site calls + * it as ?model=&lane=&kind=tg&config=…; the CLI-shaped ?cell_key= form is + * supported for parity. + * + * `kind` and `config` matter more than they look. A model typically has both + * pp (prefill) and tg (decode) records under the same model_id and lane, and + * a pp record has no decode_ts_med at all. Without narrowing, a "decode + * history" series is half prefill runs — measured against real data, an + * 8-point response where only 4 points belonged on the graph. Different + * config_labels are the same apples-to-oranges problem: they are different + * configurations of the model, not successive measurements of one. + * + * Filtering is by these DISPLAY DIMENSIONS rather than by cell_key even + * though a cell is exactly "one comparable series". A cell_key is + * content-addressed over engine/image provenance, so an unrelated runner-image + * bump between two sweeps forks the key and shatters one continuous history + * into several one-point series. hal0's own dashboard moved off cell_key for + * this reason. cell_key remains available as an explicit filter for callers + * that genuinely want that one identity. * * Ordered oldest-first because that is plot order. The LIMIT therefore has to * be applied to the NEWEST rows and reversed in JS, or a model with more than @@ -389,6 +406,8 @@ export async function historyHandler(req: Request, env: Env): Promise const cellKey = url.searchParams.get("cell_key"); const model = url.searchParams.get("model"); const lane = url.searchParams.get("lane"); + const kind = url.searchParams.get("kind"); + const config = url.searchParams.get("config"); if (!cellKey && !model) { return publicErrors(req, ["cell_key or model is required"], 400); @@ -408,6 +427,14 @@ export async function historyHandler(req: Request, env: Env): Promise conditions.push("lane = ?"); params.push(lane); } + if (kind) { + conditions.push("kind = ?"); + params.push(kind); + } + if (config) { + conditions.push("config_label = ?"); + params.push(config); + } const { results } = await env.DB.prepare( `SELECT measured_at AS ts, decode_ts_med, prefill_ts_med, lane diff --git a/workers/bench-api/test/reads.test.ts b/workers/bench-api/test/reads.test.ts index fe77b00..41ed8bc 100644 --- a/workers/bench-api/test/reads.test.ts +++ b/workers/bench-api/test/reads.test.ts @@ -388,6 +388,67 @@ describe("GET /v1/history", () => { expect(body.points[0].lane).toBe("vulkan_radv"); }); + // A pp record has no decode figure at all, so mixing kinds turns a "decode + // history" into a half-empty series. Measured against real CT105 data: 8 + // points back, only 4 of which belonged on the graph. + it("filters by workload kind so prefill runs stay out of a decode series", async () => { + const n = ++seq; + const modelId = `kind-model-${n}`; + await insertRecord({ + cellKey: "sha256:" + hex(0xa1000 + n), + runId: `run-tg-${n}`, + bundleId: fx.pubBundle, + status: "published", + modelId, + kind: "tg", + }); + await insertRecord({ + cellKey: "sha256:" + hex(0xa2000 + n), + runId: `run-pp-${n}`, + bundleId: fx.pubBundle, + status: "published", + modelId, + kind: "pp", + }); + + const all = await (await SELF.fetch(`https://api.hal0.dev/v1/history?model=${modelId}`)).json<{ + points: unknown[]; + }>(); + expect(all.points).toHaveLength(2); + + const tg = await ( + await SELF.fetch(`https://api.hal0.dev/v1/history?model=${modelId}&kind=tg`) + ).json<{ points: unknown[] }>(); + expect(tg.points).toHaveLength(1); + }); + + it("filters by config so two configurations aren't plotted as one series", async () => { + const n = ++seq; + const modelId = `config-model-${n}`; + await insertRecord({ + cellKey: "sha256:" + hex(0xb1000 + n), + runId: `run-default-${n}`, + bundleId: fx.pubBundle, + status: "published", + modelId, + configLabel: "default", + }); + await insertRecord({ + cellKey: "sha256:" + hex(0xb2000 + n), + runId: `run-tuned-${n}`, + bundleId: fx.pubBundle, + status: "published", + modelId, + configLabel: "tuned", + }); + + const res = await SELF.fetch( + `https://api.hal0.dev/v1/history?model=${modelId}&config=default`, + ); + const body = await res.json<{ points: unknown[] }>(); + expect(body.points).toHaveLength(1); + }); + // normalizeHistoryPoints on the site reads decode_ts_med/prefill_ts_med/ts, // so the column aliasing has to survive. it("emits points in the shape the site's normalizeHistoryPoints expects", async () => {