diff --git a/packages/db/src/queries/benchmarks.test.ts b/packages/db/src/queries/benchmarks.test.ts new file mode 100644 index 00000000..3e32a780 --- /dev/null +++ b/packages/db/src/queries/benchmarks.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from 'vitest'; + +import type { DbClient } from '../connection.js'; +import { getAllBenchmarksForHistory, getLatestBenchmarks } from './benchmarks.js'; + +/** + * A {@link DbClient} stand-in that records every SQL template it is handed and + * resolves to an empty result set. Lets us assert on the *generated SQL* without + * a live database — in particular that the read path never names the optional + * `workers` column directly. + * + * Joining the template's static segments with a ` ? ` placeholder reconstructs + * the literal SQL (interpolated values like model keys / dates become `?`), + * which is all we need to substring-match the column selection. + */ +function makeRecordingSql(): { sql: DbClient; sqlText: () => string } { + const queries: string[] = []; + const sql = ((strings: TemplateStringsArray, ..._values: unknown[]) => { + queries.push(strings.join(' ? ')); + return Promise.resolve([]); + }) as DbClient; + return { sql, sqlText: () => queries.join('\n') }; +} + +/** + * Regression guard for the #405 fallout: the read queries must surface `workers` + * via `to_jsonb(row) -> 'workers'`, NOT a bare `br.workers` / `lb.workers`. A bare + * column reference fails to plan ("column \"workers\" does not exist") when migration + * 006 hasn't been applied, which 500s every cache-miss request to /api/v1/benchmarks. + * The to_jsonb form returns null for the absent column and lets the endpoint keep + * serving everything else. + */ +describe('benchmark read queries — workers column tolerance', () => { + it('getLatestBenchmarks (no-date / materialized-view branch) does not reference lb.workers directly', async () => { + const { sql, sqlText } = makeRecordingSql(); + await getLatestBenchmarks(sql, 'dsr1'); + const text = sqlText(); + expect(text).toContain("to_jsonb(lb) -> 'workers'"); + expect(text).not.toMatch(/\blb\.workers\b/u); + }); + + it('getLatestBenchmarks (date-filtered / base-table branch) does not reference br.workers directly', async () => { + const { sql, sqlText } = makeRecordingSql(); + await getLatestBenchmarks(sql, 'dsr1', '2026-01-01'); + const text = sqlText(); + expect(text).toContain("to_jsonb(br) -> 'workers'"); + expect(text).not.toMatch(/\bbr\.workers\b/u); + }); + + it('getAllBenchmarksForHistory does not reference br.workers directly', async () => { + const { sql, sqlText } = makeRecordingSql(); + await getAllBenchmarksForHistory(sql, 'dsr1', 1024, 1024); + const text = sqlText(); + expect(text).toContain("to_jsonb(br) -> 'workers'"); + expect(text).not.toMatch(/\bbr\.workers\b/u); + }); +}); diff --git a/packages/db/src/queries/benchmarks.ts b/packages/db/src/queries/benchmarks.ts index a2167e95..b3a6e96f 100644 --- a/packages/db/src/queries/benchmarks.ts +++ b/packages/db/src/queries/benchmarks.ts @@ -90,7 +90,11 @@ export async function getLatestBenchmarks( br.conc, br.image, br.metrics, - br.workers, + -- Read workers via to_jsonb(row)->'workers' rather than a bare column ref so the + -- query still plans (and returns null) if migration 006 has not been applied yet. + -- A bare column reference throws "column does not exist" at parse time and 500s the + -- whole endpoint. See migration 006_benchmark_results_workers.sql. + to_jsonb(br) -> 'workers' AS workers, br.date::text, CASE WHEN wr.html_url IS NOT NULL THEN wr.html_url || '/attempts/' || wr.run_attempt ELSE NULL END AS run_url FROM benchmark_results br @@ -129,7 +133,8 @@ export async function getLatestBenchmarks( lb.conc, lb.image, lb.metrics, - lb.workers, + -- to_jsonb(row)->'workers' guard: tolerate a pre-migration-006 view (see date branch above). + to_jsonb(lb) -> 'workers' AS workers, lb.date::text, CASE WHEN wr.html_url IS NOT NULL THEN wr.html_url || '/attempts/' || wr.run_attempt ELSE NULL END AS run_url FROM latest_benchmarks lb @@ -176,7 +181,8 @@ export async function getAllBenchmarksForHistory( br.osl, br.conc, br.metrics - '{std_ttft,std_tpot,std_e2el,std_intvty,std_itl,mean_ttft,mean_tpot,mean_e2el,mean_intvty,mean_itl}'::text[] as metrics, - br.workers, + -- to_jsonb(row)->'workers' guard: tolerate a pre-migration-006 base table (see getLatestBenchmarks). + to_jsonb(br) -> 'workers' AS workers, br.date::text, CASE WHEN wr.html_url IS NOT NULL THEN wr.html_url || '/attempts/' || wr.run_attempt ELSE NULL END AS run_url FROM configs c