diff --git a/docs/data.md b/docs/data.md index 38f20e8..a333617 100644 --- a/docs/data.md +++ b/docs/data.md @@ -72,6 +72,7 @@ flowchart TB | `packages/data/src/jsonl-reader.ts` | Generic JSONL file reader (returns typed array) | | `packages/data/src/analytics.ts` | DuckDB JSONL-to-Parquet import, test run summaries, per-test queries, detail views | | `packages/data/src/run-status.ts` | SCIL and ACIL analytics: history and run detail queries over Parquet | +| `packages/data/src/parquet-files.ts` | `parquetFile()` and `hasParquet()` helpers for checking which Parquet files exist before querying | | `packages/data/src/re-eval-marker.ts` | Tracks re-evaluated run IDs via `.re-evaluated-runs.json` marker file | | `packages/data/src/scil-split.ts` | Deterministic stratified train/test splitting with seeded PRNG | | `packages/data/src/scil-prompt.ts` | Builds LLM improvement prompts from SCIL iteration results | @@ -216,6 +217,22 @@ Three main query functions join across Parquet files: - **`queryTestRunSummaries()`** — Aggregates per-test results into run-level pass/fail counts by eval - **`queryTestRunDetails()`** — Returns detailed per-test summaries, individual expectation results, grouped LLM judge criteria, and output files for a single run +#### Missing Parquet files + +Any Parquet file may be missing: the data directory may not exist yet, and `updateAllParquet()` skips any table with no source JSONL. DuckDB's `read_parquet` throws `IO Error: No files found that match the pattern` for a missing file, so every query checks with `hasParquet()` before reading: + +| Missing file | List queries (`queryTestRunSummaries`, `queryPerTest`, `queryScilHistory`, `queryAcilHistory`) | Detail queries | +|---|---|---| +| `test-run` or `test-config` | Return `[]` | `queryTestRunDetails` throws `Test run not found: ` | +| `test-results` | Return rows with `all_expectations_passed: null` | `queryTestRunDetails` returns empty `expectations` and `llmJudgeGroups` | +| `scil-iteration` / `acil-iteration` | Return `[]` | Return `iterations: []` | +| `scil-summary` / `acil-summary` | — | Throw `SCIL run not found: ` / `ACIL run not found: ` | + +Two caveats apply: + +- A partial write that leaves `test-run.parquet` without `test-config.parquet` reads as "no data", not as an error. +- The first write of each Parquet file goes straight to its final path, with no temp file and rename. An interrupted first write can leave a corrupt file that passes `hasParquet()` and still makes the query throw. + All queries filter out `infrastructure-error` status rows when the `status` column exists in the Parquet schema (backward compatibility with older data). SCIL-specific queries in `run-status.ts`: diff --git a/docs/web.md b/docs/web.md index 2ace38c..587a2c2 100644 --- a/docs/web.md +++ b/docs/web.md @@ -209,11 +209,11 @@ interface ScilSummaryRow { #### Server Startup and Asset Embedding -The server entry (`packages/web/src/server/index.ts`) uses Yargs to parse `--port` and `--data-dir` CLI arguments. It registers six API routes and three static asset routes. The client build output (`dist/client/`) is embedded using Bun's `import ... with { type: 'file' }` syntax, which resolves to `$bunfs` paths in compiled standalone executables. A SPA fallback (`/*`) serves `index.html` for all unmatched paths, enabling client-side routing. +The server entry (`packages/web/src/server/index.ts`) uses Yargs to parse `--port` and `--data-dir` CLI arguments. It registers the API routes, a `jsonErrorHandler` via `app.onError` so unexpected errors reach the client as JSON, and the static asset routes. The client build output (`dist/client/`) is embedded using Bun's `import ... with { type: 'file' }` syntax, which resolves to `$bunfs` paths in compiled standalone executables. A SPA fallback (`/*`) serves `index.html` for all unmatched paths, enabling client-side routing. #### Route Handler Pattern -All three route modules follow the same pattern: receive a Hono `Context` and a `dataDir` string, call the corresponding `@testdouble/skillwalker-data` query function, and return the result via `c.json()`. Error handling distinguishes "not found" errors (returned as 404 JSON) from unexpected errors (re-thrown). The SCIL routes additionally handle missing Parquet files by returning empty results or 404. +All three route modules follow the same pattern: receive a Hono `Context` and a `dataDir` string, call the corresponding `@testdouble/skillwalker-data` query function, and return the result via `c.json()`. Error handling distinguishes "not found" and `InvalidRunIdError` errors (returned as 404 JSON) from unexpected errors (re-thrown to `jsonErrorHandler`). Missing Parquet files need no route-level handling: the data layer returns empty results or throws "run not found". ```typescript // packages/web/src/server/routes/test-runs.ts — typical handler pattern @@ -223,7 +223,7 @@ export async function getTestRunById(c: Context, dataDir: string): Promise { expect(rows).toHaveLength(1) expect(rows[0].all_expectations_passed).toBeNull() }) - - it('throws DuckDB IO error when any parquet file is missing', async () => { - // dataDir exists but contains no parquet files - const dataDir = path.join(tmpDir, 'empty-analytics') - await mkdir(dataDir, { recursive: true }) - - await expect(queryPerTest(dataDir)).rejects.toThrow() - }) }) -// ─── queryTestRunDetails — missing parquet ──────────────────────────────────── - -describe('queryTestRunDetails (missing parquet)', () => { - it('throws DuckDB IO error when test-results.parquet is missing but test-run.parquet exists', async () => { - const outputDir = path.join(tmpDir, 'output') - const dataDir = path.join(tmpDir, 'analytics') - await mkdir(dataDir, { recursive: true }) +// ─── queries with missing parquet files ────────────────────────────────────── + +describe('queries with no data', () => { + const runId = '20260101T100001' + + for (const [label, makeDataDir] of [ + ['a data directory that does not exist', async () => path.join(tmpDir, 'missing-analytics')], + [ + 'an empty data directory', + async () => { + const dataDir = path.join(tmpDir, 'empty-analytics') + await mkdir(dataDir, { recursive: true }) + return dataDir + }, + ], + ] as const) { + describe(`given ${label}`, () => { + it('queryTestRunSummaries returns an empty list', async () => { + expect(await queryTestRunSummaries(await makeDataDir())).toEqual([]) + }) + + it('queryPerTest returns an empty list', async () => { + expect(await queryPerTest(await makeDataDir())).toEqual([]) + }) + + it('queryScilHistory returns an empty list', async () => { + expect(await queryScilHistory(await makeDataDir())).toEqual([]) + }) + + it('queryAcilHistory returns an empty list', async () => { + expect(await queryAcilHistory(await makeDataDir())).toEqual([]) + }) + + it('queryTestRunDetails rejects with not found', async () => { + await expect(queryTestRunDetails(await makeDataDir(), runId)).rejects.toThrow(`Test run not found: ${runId}`) + }) + + it('queryScilRunDetails rejects with not found', async () => { + await expect(queryScilRunDetails(await makeDataDir(), runId)).rejects.toThrow(`SCIL run not found: ${runId}`) + }) + + it('queryAcilRunDetails rejects with not found', async () => { + await expect(queryAcilRunDetails(await makeDataDir(), runId)).rejects.toThrow(`ACIL run not found: ${runId}`) + }) + }) + } +}) - // Import only test-run and test-config — skip test-results +describe('queries with partial test-run parquet files', () => { + async function importRunAndConfigOnly(outputDir: string, dataDir: string): Promise { const runDir = path.join(outputDir, '20260101T100001') await writeJsonl(path.join(runDir, 'test-config.jsonl'), [ makeConfigRecord({ testRunId: '20260101T100001', eval: 's', testName: 't' }), @@ -571,7 +604,6 @@ describe('queryTestRunDetails (missing parquet)', () => { await writeJsonl(path.join(runDir, 'test-run.jsonl'), [ makeRunResultRecord({ testRunId: '20260101T100001', eval: 's', testName: 't' }), ]) - // Manually import only test-run and test-config await importJsonlToParquet({ jsonlGlob: `${outputDir}/*/test-config.jsonl`, parquetPath: path.join(dataDir, 'test-config.parquet'), @@ -581,10 +613,99 @@ describe('queryTestRunDetails (missing parquet)', () => { parquetPath: path.join(dataDir, 'test-run.parquet'), filter: (obj) => (obj as Record).type === 'result', }) - // test-results.parquet intentionally NOT created + } + + describe('given test-run and test-config without test-results', () => { + it('queryTestRunSummaries returns the run with no passing tests', async () => { + const outputDir = path.join(tmpDir, 'output') + const dataDir = path.join(tmpDir, 'analytics') + await mkdir(dataDir, { recursive: true }) + await importRunAndConfigOnly(outputDir, dataDir) + + const runs = await queryTestRunSummaries(dataDir) + + expect(runs).toHaveLength(1) + expect(runs[0]).toMatchObject({ test_run_id: '20260101T100001', total_tests: 1, passed: 0, failed: 1 }) + }) + + it('queryPerTest returns rows with null all_expectations_passed', async () => { + const outputDir = path.join(tmpDir, 'output') + const dataDir = path.join(tmpDir, 'analytics') + await mkdir(dataDir, { recursive: true }) + await importRunAndConfigOnly(outputDir, dataDir) + + const rows = await queryPerTest(dataDir) + + expect(rows).toHaveLength(1) + expect(rows[0].all_expectations_passed).toBeNull() + }) + + it('queryTestRunDetails returns the summary with empty expectations', async () => { + const outputDir = path.join(tmpDir, 'output') + const dataDir = path.join(tmpDir, 'analytics') + await mkdir(dataDir, { recursive: true }) + await importRunAndConfigOnly(outputDir, dataDir) + + const details = await queryTestRunDetails(dataDir, '20260101T100001') + + expect(details.summary).toHaveLength(1) + expect(details.summary[0].all_expectations_passed).toBeNull() + expect(details.expectations).toEqual([]) + expect(details.llmJudgeGroups).toEqual([]) + }) + }) + + describe('given test-run without test-config', () => { + // A partial write that dropped test-config reads as "no data", not an error. + it('list queries return empty and details report not found', async () => { + const outputDir = path.join(tmpDir, 'output') + const dataDir = path.join(tmpDir, 'analytics') + await mkdir(dataDir, { recursive: true }) + await importRunAndConfigOnly(outputDir, dataDir) + await rm(path.join(dataDir, 'test-config.parquet')) + + expect(await queryTestRunSummaries(dataDir)).toEqual([]) + expect(await queryPerTest(dataDir)).toEqual([]) + await expect(queryTestRunDetails(dataDir, '20260101T100001')).rejects.toThrow( + 'Test run not found: 20260101T100001', + ) + }) + }) +}) + +describe('SCIL and ACIL details with a summary but no iterations', () => { + it('queryScilRunDetails returns the summary with empty iterations', async () => { + const outputDir = path.join(tmpDir, 'output') + const dataDir = path.join(tmpDir, 'analytics') + await writeScilRunFixture({ + outputDir, + runId: '20260101T200001', + iterations: [makeScilIterationRecord({ test_run_id: '20260101T200001' })], + }) + await rm(path.join(outputDir, '20260101T200001', 'scil-iteration.jsonl')) + await updateAllParquet({ outputDir, dataDir }) + + const details = await queryScilRunDetails(dataDir, '20260101T200001') + + expect(details.summary.test_run_id).toBe('20260101T200001') + expect(details.iterations).toEqual([]) + }) + + it('queryAcilRunDetails returns the summary with empty iterations', async () => { + const outputDir = path.join(tmpDir, 'output') + const dataDir = path.join(tmpDir, 'analytics') + await writeAcilRunFixture({ + outputDir, + runId: '20260101T200001', + iterations: [makeAcilIterationRecord({ test_run_id: '20260101T200001' })], + }) + await rm(path.join(outputDir, '20260101T200001', 'acil-iteration.jsonl')) + await updateAllParquet({ outputDir, dataDir }) + + const details = await queryAcilRunDetails(dataDir, '20260101T200001') - // Existence check passes (run is in test-run), but summary query fails on missing test-results - await expect(queryTestRunDetails(dataDir, '20260101T100001')).rejects.toThrow() + expect(details.summary.test_run_id).toBe('20260101T200001') + expect(details.iterations).toEqual([]) }) }) diff --git a/packages/data/src/analytics.ts b/packages/data/src/analytics.ts index 263238d..3f426d3 100644 --- a/packages/data/src/analytics.ts +++ b/packages/data/src/analytics.ts @@ -5,6 +5,7 @@ import os from 'node:os' import path from 'node:path' import { type DuckDBConnection, DuckDBInstance } from '@duckdb/node-api' import { withConnection } from './connection.js' +import { hasParquet } from './parquet-files.js' import type { AcilSummaryRecord, LlmJudgeCriterion, @@ -19,7 +20,20 @@ import type { } from './types.js' import { InvalidRunIdError } from './types.js' +// Stands in for the expect_summary CTE when test-results.parquet does not exist, +// so the LEFT JOIN yields NULL all_expectations_passed instead of failing. +const EMPTY_EXPECT_SUMMARY = ` + SELECT NULL::VARCHAR AS test_run_id, NULL::VARCHAR AS eval, + NULL::VARCHAR AS test_name, NULL::BOOLEAN AS all_expectations_passed + WHERE false +` + +function hasTestRunData(dataDir: string): boolean { + return hasParquet(dataDir, 'test-run') && hasParquet(dataDir, 'test-config') +} + async function infraErrorCondition(conn: DuckDBConnection, dataDir: string): Promise { + if (!hasParquet(dataDir, 'test-results')) return '' try { const cols = ( await conn.runAndReadAll( @@ -312,14 +326,19 @@ async function convertAcilSummariesToTempJsonl(outputDir: string): Promise { + if (!hasTestRunData(dataDir)) return [] return withConnection(dataDir, async (conn) => { const statusFilter = await infraErrorCondition(conn, dataDir) const sql = ` WITH expect_summary AS ( - SELECT test_run_id, eval, test_name, bool_and(passed) AS all_expectations_passed + ${ + hasParquet(dataDir, 'test-results') + ? `SELECT test_run_id, eval, test_name, bool_and(passed) AS all_expectations_passed FROM read_parquet('${dataDir}/test-results.parquet') ${statusFilter ? `WHERE ${statusFilter}` : ''} - GROUP BY test_run_id, eval, test_name + GROUP BY test_run_id, eval, test_name` + : EMPTY_EXPECT_SUMMARY + } ) SELECT r.test_run_id, @@ -358,14 +377,19 @@ function parseRunIdDate(runId: string): string { } export async function queryTestRunSummaries(dataDir: string): Promise { + if (!hasTestRunData(dataDir)) return [] return withConnection(dataDir, async (conn) => { const statusFilter = await infraErrorCondition(conn, dataDir) const sql = ` WITH expect_summary AS ( - SELECT test_run_id, eval, test_name, bool_and(passed) AS all_expectations_passed + ${ + hasParquet(dataDir, 'test-results') + ? `SELECT test_run_id, eval, test_name, bool_and(passed) AS all_expectations_passed FROM read_parquet('${dataDir}/test-results.parquet') ${statusFilter ? `WHERE ${statusFilter}` : ''} - GROUP BY test_run_id, eval, test_name + GROUP BY test_run_id, eval, test_name` + : EMPTY_EXPECT_SUMMARY + } ), per_test AS ( SELECT @@ -403,6 +427,9 @@ export async function queryTestRunSummaries(dataDir: string): Promise { validateRunId(testRunId) + if (!hasTestRunData(dataDir)) { + throw new Error(`Test run not found: ${testRunId}`) + } return withConnection(dataDir, async (conn) => { const existsRows = ( await conn.runAndReadAll( @@ -416,14 +443,19 @@ export async function queryTestRunDetails(dataDir: string, testRunId: string): P throw new Error(`Test run not found: ${testRunId}`) } + const hasResults = hasParquet(dataDir, 'test-results') const statusFilter = await infraErrorCondition(conn, dataDir) const summarySql = ` WITH expect_summary AS ( - SELECT test_run_id, test_name, bool_and(passed) AS all_expectations_passed + ${ + hasResults + ? `SELECT test_run_id, test_name, bool_and(passed) AS all_expectations_passed FROM read_parquet('${dataDir}/test-results.parquet') WHERE test_run_id = $1 ${statusFilter ? `AND ${statusFilter}` : ''} - GROUP BY test_run_id, test_name + GROUP BY test_run_id, test_name` + : EMPTY_EXPECT_SUMMARY + } ) SELECT r.test_run_id, @@ -452,22 +484,26 @@ export async function queryTestRunDetails(dataDir: string, testRunId: string): P await conn.runAndReadAll(summarySql, [testRunId]) ).getRowObjects() as unknown as (TestRunDetailRow & { result?: string })[] - const expectationsSql = ` - SELECT * - FROM read_parquet('${dataDir}/test-results.parquet') - WHERE test_run_id = $1 - ORDER BY test_name, expect_type, expect_value - ` - const allExpectations = ( - await conn.runAndReadAll(expectationsSql, [testRunId]) - ).getRowObjects() as unknown as (TestRunExpectationRow & { + type ExpectationQueryRow = TestRunExpectationRow & { confidence?: string reasoning?: string judge_model?: string judge_threshold?: number judge_score?: number rubric_file?: string - })[] + } + let allExpectations: ExpectationQueryRow[] = [] + if (hasResults) { + const expectationsSql = ` + SELECT * + FROM read_parquet('${dataDir}/test-results.parquet') + WHERE test_run_id = $1 + ORDER BY test_name, expect_type, expect_value + ` + allExpectations = ( + await conn.runAndReadAll(expectationsSql, [testRunId]) + ).getRowObjects() as unknown as ExpectationQueryRow[] + } // Build result-text lookup from summary rows const resultTextByTest = new Map() diff --git a/packages/data/src/parquet-files.ts b/packages/data/src/parquet-files.ts new file mode 100644 index 0000000..f7635a5 --- /dev/null +++ b/packages/data/src/parquet-files.ts @@ -0,0 +1,11 @@ +import { existsSync } from 'node:fs' + +export function parquetFile(dataDir: string, name: string): string { + return `${dataDir}/${name}.parquet` +} + +// Parquet files are written independently by updateAllParquet, so any of them +// may be absent — no data yet, or a table with no source JSONL. +export function hasParquet(dataDir: string, name: string): boolean { + return existsSync(parquetFile(dataDir, name)) +} diff --git a/packages/data/src/run-status.ts b/packages/data/src/run-status.ts index af9ac1b..5e71001 100644 --- a/packages/data/src/run-status.ts +++ b/packages/data/src/run-status.ts @@ -1,4 +1,5 @@ import { withConnection } from './connection.js' +import { hasParquet } from './parquet-files.js' import type { AcilHistoryRow, AcilIterationRow, @@ -36,6 +37,7 @@ function convertBigInts(val: unknown): unknown { } export async function queryScilHistory(dataDir: string): Promise { + if (!hasParquet(dataDir, 'scil-iteration')) return [] return withConnection(dataDir, async (conn) => { const sql = ` SELECT @@ -54,6 +56,9 @@ export async function queryScilHistory(dataDir: string): Promise { validateRunId(runId) + if (!hasParquet(dataDir, 'scil-summary')) { + throw new Error(`SCIL run not found: ${runId}`) + } return withConnection(dataDir, async (conn) => { const existsRows = ( await conn.runAndReadAll( @@ -74,13 +79,16 @@ export async function queryScilRunDetails(dataDir: string, runId: string): Promi ` const summaryRows = (await conn.runAndReadAll(summarySql, [runId])).getRowObjects() - const iterationsSql = ` - SELECT * REPLACE (CAST(iteration AS INTEGER) AS iteration) - FROM read_parquet('${dataDir}/scil-iteration.parquet') - WHERE test_run_id = $1 - ORDER BY iteration ASC - ` - const iterationRows = (await conn.runAndReadAll(iterationsSql, [runId])).getRowObjects() + let iterationRows: unknown[] = [] + if (hasParquet(dataDir, 'scil-iteration')) { + const iterationsSql = ` + SELECT * REPLACE (CAST(iteration AS INTEGER) AS iteration) + FROM read_parquet('${dataDir}/scil-iteration.parquet') + WHERE test_run_id = $1 + ORDER BY iteration ASC + ` + iterationRows = (await conn.runAndReadAll(iterationsSql, [runId])).getRowObjects() + } return { summary: summaryRows[0] as unknown as ScilSummaryRow, @@ -90,6 +98,7 @@ export async function queryScilRunDetails(dataDir: string, runId: string): Promi } export async function queryAcilHistory(dataDir: string): Promise { + if (!hasParquet(dataDir, 'acil-iteration')) return [] return withConnection(dataDir, async (conn) => { const sql = ` SELECT @@ -108,6 +117,9 @@ export async function queryAcilHistory(dataDir: string): Promise { validateRunId(runId) + if (!hasParquet(dataDir, 'acil-summary')) { + throw new Error(`ACIL run not found: ${runId}`) + } return withConnection(dataDir, async (conn) => { const existsRows = ( await conn.runAndReadAll( @@ -128,13 +140,16 @@ export async function queryAcilRunDetails(dataDir: string, runId: string): Promi ` const summaryRows = (await conn.runAndReadAll(summarySql, [runId])).getRowObjects() - const iterationsSql = ` - SELECT * REPLACE (CAST(iteration AS INTEGER) AS iteration) - FROM read_parquet('${dataDir}/acil-iteration.parquet') - WHERE test_run_id = $1 - ORDER BY iteration ASC - ` - const iterationRows = (await conn.runAndReadAll(iterationsSql, [runId])).getRowObjects() + let iterationRows: unknown[] = [] + if (hasParquet(dataDir, 'acil-iteration')) { + const iterationsSql = ` + SELECT * REPLACE (CAST(iteration AS INTEGER) AS iteration) + FROM read_parquet('${dataDir}/acil-iteration.parquet') + WHERE test_run_id = $1 + ORDER BY iteration ASC + ` + iterationRows = (await conn.runAndReadAll(iterationsSql, [runId])).getRowObjects() + } return { summary: summaryRows[0] as unknown as AcilSummaryRow, diff --git a/packages/web/src/client/lib/fetch-json.test.ts b/packages/web/src/client/lib/fetch-json.test.ts new file mode 100644 index 0000000..6c7cb34 --- /dev/null +++ b/packages/web/src/client/lib/fetch-json.test.ts @@ -0,0 +1,54 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { errorMessage, fetchJson } from './fetch-json.js' + +function stubFetch(response: Response): void { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(response)) +} + +afterEach(() => { + vi.unstubAllGlobals() +}) + +describe('fetchJson', () => { + it('returns the parsed body for an OK response', async () => { + stubFetch(new Response(JSON.stringify({ runs: [] }), { status: 200 })) + + await expect(fetchJson('/api/scil')).resolves.toEqual({ runs: [] }) + }) + + it('requests the given URL', async () => { + stubFetch(new Response('{}', { status: 200 })) + + await fetchJson('/api/scil') + + expect(vi.mocked(fetch)).toHaveBeenCalledWith('/api/scil') + }) + + it("throws the server's error message for a non-OK JSON response", async () => { + stubFetch(new Response(JSON.stringify({ error: 'Not found' }), { status: 404 })) + + await expect(fetchJson('/api/scil/bad')).rejects.toThrow('Not found') + }) + + it('throws the status text for a non-OK plain-text response', async () => { + stubFetch(new Response('Internal Server Error', { status: 500, statusText: 'Internal Server Error' })) + + await expect(fetchJson('/api/scil')).rejects.toThrow('Internal Server Error') + }) + + it('throws the HTTP status when there is no status text or JSON error', async () => { + stubFetch(new Response('oops', { status: 502 })) + + await expect(fetchJson('/api/scil')).rejects.toThrow('HTTP 502') + }) +}) + +describe('errorMessage', () => { + it('returns the message of an Error', () => { + expect(errorMessage(new Error('Not found'))).toBe('Not found') + }) + + it('stringifies a non-Error value', () => { + expect(errorMessage('boom')).toBe('boom') + }) +}) diff --git a/packages/web/src/client/lib/fetch-json.ts b/packages/web/src/client/lib/fetch-json.ts new file mode 100644 index 0000000..a922353 --- /dev/null +++ b/packages/web/src/client/lib/fetch-json.ts @@ -0,0 +1,15 @@ +// Fetches a JSON API response, turning any non-OK status into an Error whose +// message is the server's `error` field, or the HTTP status when the body isn't JSON. +export async function fetchJson(url: string): Promise { + const res = await fetch(url) + if (!res.ok) { + const body = (await res.json().catch(() => null)) as { error?: unknown } | null + const message = typeof body?.error === 'string' ? body.error : res.statusText || `HTTP ${res.status}` + throw new Error(message) + } + return (await res.json()) as T +} + +export function errorMessage(err: unknown): string { + return err instanceof Error ? err.message : String(err) +} diff --git a/packages/web/src/client/pages/AcilDetail.tsx b/packages/web/src/client/pages/AcilDetail.tsx index e6e8dd1..8bf0a8b 100644 --- a/packages/web/src/client/pages/AcilDetail.tsx +++ b/packages/web/src/client/pages/AcilDetail.tsx @@ -1,5 +1,6 @@ import { type JSX, useEffect, useState } from 'react' import { Link, useParams } from 'react-router-dom' +import { errorMessage, fetchJson } from '../lib/fetch-json.js' interface AcilTrainResult { testName: string @@ -139,22 +140,14 @@ export function AcilDetail(): JSX.Element { useEffect(() => { const load = async () => { try { - const res = await fetch(`/api/acil/${runId}`) - let data: { error?: string; summary?: AcilSummaryRow; iterations?: AcilIterationRow[] } - if (!res.ok) { - data = await res.json().catch(() => ({ error: res.statusText || `HTTP ${res.status}` })) - } else { - data = await res.json() - } - if (data.error) { - setError(data.error) - } else if (!data.summary || !data.iterations) { + const data = await fetchJson>(`/api/acil/${runId}`) + if (!data.summary || !data.iterations) { setError('Invalid response from server') } else { setDetails(data as AcilRunDetails) } } catch (err) { - setError(String(err)) + setError(errorMessage(err)) } finally { setLoading(false) } diff --git a/packages/web/src/client/pages/AcilHistory.tsx b/packages/web/src/client/pages/AcilHistory.tsx index 2059688..1493923 100644 --- a/packages/web/src/client/pages/AcilHistory.tsx +++ b/packages/web/src/client/pages/AcilHistory.tsx @@ -1,5 +1,6 @@ import { type JSX, useEffect, useState } from 'react' import { Link } from 'react-router-dom' +import { errorMessage, fetchJson } from '../lib/fetch-json.js' interface AcilHistoryRow { test_run_id: string @@ -14,14 +15,13 @@ export function AcilHistory(): JSX.Element { const [loading, setLoading] = useState(true) useEffect(() => { - fetch('/api/acil') - .then((res) => res.json()) + fetchJson<{ runs: AcilHistoryRow[] }>('/api/acil') .then((data) => { setRuns(data.runs) setLoading(false) }) .catch((err) => { - setError(String(err)) + setError(errorMessage(err)) setLoading(false) }) }, []) diff --git a/packages/web/src/client/pages/PerTestAnalytics.tsx b/packages/web/src/client/pages/PerTestAnalytics.tsx index 9083980..6a2df94 100644 --- a/packages/web/src/client/pages/PerTestAnalytics.tsx +++ b/packages/web/src/client/pages/PerTestAnalytics.tsx @@ -1,4 +1,5 @@ import { type JSX, useEffect, useState } from 'react' +import { errorMessage, fetchJson } from '../lib/fetch-json.js' interface PerTestRow { test_run_id: string @@ -71,14 +72,13 @@ export function PerTestAnalytics(): JSX.Element { const [error, setError] = useState(null) useEffect(() => { - fetch('/api/analytics/per-test') - .then((res) => res.json()) + fetchJson<{ rows: PerTestRow[] }>('/api/analytics/per-test') .then((data) => { setAllRows(data.rows) setLoading(false) }) .catch((err) => { - setError(String(err)) + setError(errorMessage(err)) setLoading(false) }) }, []) @@ -86,6 +86,13 @@ export function PerTestAnalytics(): JSX.Element { if (loading) return
Loading...
if (error) return
{error}
+ if (allRows.length === 0) { + return ( +
+ No test results found. Run tests with: skillwalker run-test --eval <name> +
+ ) + } // Aggregate stats const totalRuns = new Set(allRows.map((r) => r.test_run_id)).size diff --git a/packages/web/src/client/pages/ScilDetail.tsx b/packages/web/src/client/pages/ScilDetail.tsx index f5ecdd9..2e0edc0 100644 --- a/packages/web/src/client/pages/ScilDetail.tsx +++ b/packages/web/src/client/pages/ScilDetail.tsx @@ -1,5 +1,6 @@ import { type JSX, useEffect, useState } from 'react' import { Link, useParams } from 'react-router-dom' +import { errorMessage, fetchJson } from '../lib/fetch-json.js' interface ScilTrainResult { testName: string @@ -139,22 +140,14 @@ export function ScilDetail(): JSX.Element { useEffect(() => { const load = async () => { try { - const res = await fetch(`/api/scil/${runId}`) - let data: { error?: string; summary?: ScilSummaryRow; iterations?: ScilIterationRow[] } - if (!res.ok) { - data = await res.json().catch(() => ({ error: res.statusText || `HTTP ${res.status}` })) - } else { - data = await res.json() - } - if (data.error) { - setError(data.error) - } else if (!data.summary || !data.iterations) { + const data = await fetchJson>(`/api/scil/${runId}`) + if (!data.summary || !data.iterations) { setError('Invalid response from server') } else { setDetails(data as ScilRunDetails) } } catch (err) { - setError(String(err)) + setError(errorMessage(err)) } finally { setLoading(false) } diff --git a/packages/web/src/client/pages/ScilHistory.tsx b/packages/web/src/client/pages/ScilHistory.tsx index 9369da5..9b106bb 100644 --- a/packages/web/src/client/pages/ScilHistory.tsx +++ b/packages/web/src/client/pages/ScilHistory.tsx @@ -1,5 +1,6 @@ import { type JSX, useEffect, useState } from 'react' import { Link } from 'react-router-dom' +import { errorMessage, fetchJson } from '../lib/fetch-json.js' interface ScilHistoryRow { test_run_id: string @@ -14,14 +15,13 @@ export function ScilHistory(): JSX.Element { const [loading, setLoading] = useState(true) useEffect(() => { - fetch('/api/scil') - .then((res) => res.json()) + fetchJson<{ runs: ScilHistoryRow[] }>('/api/scil') .then((data) => { setRuns(data.runs) setLoading(false) }) .catch((err) => { - setError(String(err)) + setError(errorMessage(err)) setLoading(false) }) }, []) diff --git a/packages/web/src/client/pages/TestRunDetail.tsx b/packages/web/src/client/pages/TestRunDetail.tsx index 3356c91..bda35a0 100644 --- a/packages/web/src/client/pages/TestRunDetail.tsx +++ b/packages/web/src/client/pages/TestRunDetail.tsx @@ -1,6 +1,7 @@ import { marked } from 'marked' import { Fragment, type JSX, useEffect, useState } from 'react' import { Link, useParams } from 'react-router-dom' +import { errorMessage, fetchJson } from '../lib/fetch-json.js' interface TestRunDetailRow { test_run_id: string @@ -267,18 +268,13 @@ export function TestRunDetail(): JSX.Element { const [loading, setLoading] = useState(true) useEffect(() => { - fetch(`/api/test-runs/${runId}`) - .then((res) => res.json()) + fetchJson
(`/api/test-runs/${runId}`) .then((data) => { - if (data.error) { - setError(data.error) - } else { - setDetails(data) - } + setDetails(data) setLoading(false) }) .catch((err) => { - setError(String(err)) + setError(errorMessage(err)) setLoading(false) }) }, [runId]) diff --git a/packages/web/src/client/pages/TestRunHistory.tsx b/packages/web/src/client/pages/TestRunHistory.tsx index 47b2666..4318ce1 100644 --- a/packages/web/src/client/pages/TestRunHistory.tsx +++ b/packages/web/src/client/pages/TestRunHistory.tsx @@ -1,5 +1,6 @@ import { type JSX, useEffect, useState } from 'react' import { Link } from 'react-router-dom' +import { errorMessage, fetchJson } from '../lib/fetch-json.js' interface TestRunSummary { test_run_id: string @@ -16,14 +17,13 @@ export function TestRunHistory(): JSX.Element { const [loading, setLoading] = useState(true) useEffect(() => { - fetch('/api/test-runs') - .then((res) => res.json()) + fetchJson<{ runs: TestRunSummary[] }>('/api/test-runs') .then((data) => { setRuns(data.runs) setLoading(false) }) .catch((err) => { - setError(String(err)) + setError(errorMessage(err)) setLoading(false) }) }, []) diff --git a/packages/web/src/server/index.ts b/packages/web/src/server/index.ts index f064c4c..22cbe1e 100644 --- a/packages/web/src/server/index.ts +++ b/packages/web/src/server/index.ts @@ -8,6 +8,7 @@ import _indexHtml from '../../dist/client/index.html' with { type: 'file' } import indexJs from '../../dist/client/index.js' with { type: 'file' } import { getAcilHistory, getAcilRunById } from './routes/acil' import { getPerTestAnalytics } from './routes/analytics' +import { jsonErrorHandler } from './routes/error-handler' import { getScilHistory, getScilRunById } from './routes/scil' import { getTestRunById, getTestRuns } from './routes/test-runs' @@ -38,6 +39,8 @@ const dataDir = argv['data-dir'] const app = new Hono() +app.onError(jsonErrorHandler) + app.get('/api/health', (c) => c.json({ status: 'ok' })) app.get('/api/test-runs', (c) => getTestRuns(c, dataDir)) app.get('/api/test-runs/:runId', (c) => getTestRunById(c, dataDir)) diff --git a/packages/web/src/server/routes/acil.test.ts b/packages/web/src/server/routes/acil.test.ts index ba909db..2c68f7d 100644 --- a/packages/web/src/server/routes/acil.test.ts +++ b/packages/web/src/server/routes/acil.test.ts @@ -1,11 +1,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -vi.mock('@testdouble/skillwalker-data', () => ({ +vi.mock('@testdouble/skillwalker-data', async (importOriginal) => ({ + InvalidRunIdError: (await importOriginal()).InvalidRunIdError, queryAcilHistory: vi.fn(), queryAcilRunDetails: vi.fn(), })) -import { queryAcilHistory, queryAcilRunDetails } from '@testdouble/skillwalker-data' +import { InvalidRunIdError, queryAcilHistory, queryAcilRunDetails } from '@testdouble/skillwalker-data' import { getAcilHistory, getAcilRunById } from './acil.js' function makeMockContext(overrides?: { param?: Record }) { @@ -58,17 +59,6 @@ describe('getAcilHistory', () => { expect(runs).toEqual([]) }) - it('returns empty runs when parquet file does not exist', async () => { - vi.mocked(queryAcilHistory).mockRejectedValue( - new Error('IO Error: No such file or directory: acil-iteration.parquet'), - ) - const { c, jsonMock } = makeMockContext() - - await getAcilHistory(c, '/data') - - expect(jsonMock).toHaveBeenCalledWith({ runs: [] }) - }) - it('re-throws unexpected errors', async () => { vi.mocked(queryAcilHistory).mockRejectedValue(new Error('Database connection failed')) const { c } = makeMockContext() @@ -105,6 +95,15 @@ describe('getAcilRunById', () => { expect(jsonMock).toHaveBeenCalledWith({ error: 'Not found' }, 404) }) + it('returns 404 JSON when the run ID is malformed', async () => { + vi.mocked(queryAcilRunDetails).mockRejectedValue(new InvalidRunIdError('bad-id')) + const { c, jsonMock } = makeMockContext({ param: { runId: 'bad-id' } }) + + await getAcilRunById(c, '/data') + + expect(jsonMock).toHaveBeenCalledWith({ error: 'Not found' }, 404) + }) + it('defaults to empty string runId when param is missing', async () => { vi.mocked(queryAcilRunDetails).mockRejectedValue(new Error('ACIL run not found: ')) const { c, jsonMock } = makeMockContext() @@ -122,17 +121,6 @@ describe('getAcilRunById', () => { await expect(getAcilRunById(c, '/data')).rejects.toThrow('Database connection failed') }) - it('returns 404 when parquet file does not exist', async () => { - vi.mocked(queryAcilRunDetails).mockRejectedValue( - new Error('IO Error: No such file or directory: acil-summary.parquet'), - ) - const { c, jsonMock } = makeMockContext({ param: { runId: 'run-abc' } }) - - await getAcilRunById(c, '/data') - - expect(jsonMock).toHaveBeenCalledWith({ error: 'Not found' }, 404) - }) - it('re-throws non-Error throwable even if message matches "not found" pattern', async () => { vi.mocked(queryAcilRunDetails).mockRejectedValue('ACIL run not found: run-xyz') const { c, jsonMock } = makeMockContext({ param: { runId: 'run-xyz' } }) diff --git a/packages/web/src/server/routes/acil.ts b/packages/web/src/server/routes/acil.ts index c2303ff..2e3c174 100644 --- a/packages/web/src/server/routes/acil.ts +++ b/packages/web/src/server/routes/acil.ts @@ -1,16 +1,9 @@ -import { queryAcilHistory, queryAcilRunDetails } from '@testdouble/skillwalker-data' +import { InvalidRunIdError, queryAcilHistory, queryAcilRunDetails } from '@testdouble/skillwalker-data' import type { Context } from 'hono' export async function getAcilHistory(c: Context, dataDir: string): Promise { - try { - const runs = await queryAcilHistory(dataDir) - return c.json({ runs }) - } catch (err) { - if (err instanceof Error && err.message.includes('No such file or directory')) { - return c.json({ runs: [] }) - } - throw err - } + const runs = await queryAcilHistory(dataDir) + return c.json({ runs }) } export async function getAcilRunById(c: Context, dataDir: string): Promise { @@ -19,10 +12,7 @@ export async function getAcilRunById(c: Context, dataDir: string): Promise ({ data, status })) + return { c: { json: jsonMock } as any, jsonMock } +} + +beforeEach(() => { + vi.spyOn(console, 'error').mockImplementation(() => {}) +}) + +afterEach(() => { + vi.restoreAllMocks() +}) + +describe('jsonErrorHandler', () => { + it('responds with a JSON 500 error', () => { + const { c, jsonMock } = makeMockContext() + + jsonErrorHandler(new Error('boom'), c) + + expect(jsonMock).toHaveBeenCalledWith({ error: 'Internal server error' }, 500) + }) + + it('logs the original error', () => { + const { c } = makeMockContext() + const err = new Error('boom') + + jsonErrorHandler(err, c) + + expect(console.error).toHaveBeenCalledWith(err) + }) +}) diff --git a/packages/web/src/server/routes/error-handler.ts b/packages/web/src/server/routes/error-handler.ts new file mode 100644 index 0000000..1afa323 --- /dev/null +++ b/packages/web/src/server/routes/error-handler.ts @@ -0,0 +1,7 @@ +import type { Context } from 'hono' + +// Hono's default handler answers with plain text, which the client cannot parse as JSON. +export function jsonErrorHandler(err: Error, c: Context): Response { + console.error(err) + return c.json({ error: 'Internal server error' }, 500) +} diff --git a/packages/web/src/server/routes/scil.test.ts b/packages/web/src/server/routes/scil.test.ts index 7527380..8ef760b 100644 --- a/packages/web/src/server/routes/scil.test.ts +++ b/packages/web/src/server/routes/scil.test.ts @@ -1,11 +1,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -vi.mock('@testdouble/skillwalker-data', () => ({ +vi.mock('@testdouble/skillwalker-data', async (importOriginal) => ({ + InvalidRunIdError: (await importOriginal()).InvalidRunIdError, queryScilHistory: vi.fn(), queryScilRunDetails: vi.fn(), })) -import { queryScilHistory, queryScilRunDetails } from '@testdouble/skillwalker-data' +import { InvalidRunIdError, queryScilHistory, queryScilRunDetails } from '@testdouble/skillwalker-data' import { getScilHistory, getScilRunById } from './scil.js' function makeMockContext(overrides?: { param?: Record }) { @@ -58,17 +59,6 @@ describe('getScilHistory', () => { expect(runs).toEqual([]) }) - it('returns empty runs when parquet file does not exist (TP-003)', async () => { - vi.mocked(queryScilHistory).mockRejectedValue( - new Error('IO Error: No such file or directory: scil-iteration.parquet'), - ) - const { c, jsonMock } = makeMockContext() - - await getScilHistory(c, '/data') - - expect(jsonMock).toHaveBeenCalledWith({ runs: [] }) - }) - it('re-throws unexpected errors (TP-004)', async () => { vi.mocked(queryScilHistory).mockRejectedValue(new Error('Database connection failed')) const { c } = makeMockContext() @@ -105,6 +95,15 @@ describe('getScilRunById', () => { expect(jsonMock).toHaveBeenCalledWith({ error: 'Not found' }, 404) }) + it('returns 404 JSON when the run ID is malformed', async () => { + vi.mocked(queryScilRunDetails).mockRejectedValue(new InvalidRunIdError('bad-id')) + const { c, jsonMock } = makeMockContext({ param: { runId: 'bad-id' } }) + + await getScilRunById(c, '/data') + + expect(jsonMock).toHaveBeenCalledWith({ error: 'Not found' }, 404) + }) + it('defaults to empty string runId when param is missing', async () => { vi.mocked(queryScilRunDetails).mockRejectedValue(new Error('SCIL run not found: ')) const { c, jsonMock } = makeMockContext() @@ -122,17 +121,6 @@ describe('getScilRunById', () => { await expect(getScilRunById(c, '/data')).rejects.toThrow('Database connection failed') }) - it('returns 404 when parquet file does not exist (EC5)', async () => { - vi.mocked(queryScilRunDetails).mockRejectedValue( - new Error('IO Error: No such file or directory: scil-summary.parquet'), - ) - const { c, jsonMock } = makeMockContext({ param: { runId: 'run-abc' } }) - - await getScilRunById(c, '/data') - - expect(jsonMock).toHaveBeenCalledWith({ error: 'Not found' }, 404) - }) - it('re-throws non-Error throwable even if message matches "not found" pattern', async () => { vi.mocked(queryScilRunDetails).mockRejectedValue('SCIL run not found: run-xyz') const { c, jsonMock } = makeMockContext({ param: { runId: 'run-xyz' } }) diff --git a/packages/web/src/server/routes/scil.ts b/packages/web/src/server/routes/scil.ts index 8007bf0..70cf677 100644 --- a/packages/web/src/server/routes/scil.ts +++ b/packages/web/src/server/routes/scil.ts @@ -1,16 +1,9 @@ -import { queryScilHistory, queryScilRunDetails } from '@testdouble/skillwalker-data' +import { InvalidRunIdError, queryScilHistory, queryScilRunDetails } from '@testdouble/skillwalker-data' import type { Context } from 'hono' export async function getScilHistory(c: Context, dataDir: string): Promise { - try { - const runs = await queryScilHistory(dataDir) - return c.json({ runs }) - } catch (err) { - if (err instanceof Error && err.message.includes('No such file or directory')) { - return c.json({ runs: [] }) - } - throw err - } + const runs = await queryScilHistory(dataDir) + return c.json({ runs }) } export async function getScilRunById(c: Context, dataDir: string): Promise { @@ -19,10 +12,7 @@ export async function getScilRunById(c: Context, dataDir: string): Promise ({ +vi.mock('@testdouble/skillwalker-data', async (importOriginal) => ({ + InvalidRunIdError: (await importOriginal()).InvalidRunIdError, queryTestRunSummaries: vi.fn(), queryTestRunDetails: vi.fn(), })) -import { queryTestRunDetails, queryTestRunSummaries } from '@testdouble/skillwalker-data' +import { InvalidRunIdError, queryTestRunDetails, queryTestRunSummaries } from '@testdouble/skillwalker-data' import { getTestRunById, getTestRuns } from './test-runs.js' function makeMockContext(overrides?: { param?: Record }) { @@ -119,6 +120,15 @@ describe('getTestRunById', () => { expect(jsonMock).toHaveBeenCalledWith({ error: 'Not found' }, 404) }) + it('returns 404 JSON when the run ID is malformed', async () => { + vi.mocked(queryTestRunDetails).mockRejectedValue(new InvalidRunIdError('bad-id')) + const { c, jsonMock } = makeMockContext({ param: { runId: 'bad-id' } }) + + await getTestRunById(c, '/data') + + expect(jsonMock).toHaveBeenCalledWith({ error: 'Not found' }, 404) + }) + it('defaults to empty string runId when param is missing', async () => { vi.mocked(queryTestRunDetails).mockRejectedValue(new Error('Test run not found: ')) const { c, jsonMock } = makeMockContext() diff --git a/packages/web/src/server/routes/test-runs.ts b/packages/web/src/server/routes/test-runs.ts index d592934..2064a8a 100644 --- a/packages/web/src/server/routes/test-runs.ts +++ b/packages/web/src/server/routes/test-runs.ts @@ -1,4 +1,4 @@ -import { queryTestRunDetails, queryTestRunSummaries } from '@testdouble/skillwalker-data' +import { InvalidRunIdError, queryTestRunDetails, queryTestRunSummaries } from '@testdouble/skillwalker-data' import type { Context } from 'hono' export async function getTestRuns(c: Context, dataDir: string): Promise { @@ -12,7 +12,7 @@ export async function getTestRunById(c: Context, dataDir: string): Promise