diff --git a/src/benchmarks/terminal-bench/ori-parquet.test.ts b/src/benchmarks/terminal-bench/ori-parquet.test.ts index 8d256775..a69efe52 100644 --- a/src/benchmarks/terminal-bench/ori-parquet.test.ts +++ b/src/benchmarks/terminal-bench/ori-parquet.test.ts @@ -21,6 +21,7 @@ import { import type { Sample } from "../../harness/core"; import { initialTaskState } from "../../harness/core"; import { Solver } from "../../harness/solver"; +import { assertSuccess } from "../../internal/testing"; import { asyncBufferFromBytes, readResultRows, @@ -175,7 +176,8 @@ async function runOriSampleToParquetRow() { const rows = await readResultRows( asyncBufferFromBytes(new Uint8Array(buffer)) ); - const row = rows[0]; + assertSuccess(rows); + const row = rows.success[0]; if (row === undefined) { throw new Error("parquet contained no rows"); } diff --git a/src/benchmarks/toolcall-schema-fuzz/cli/report.ts b/src/benchmarks/toolcall-schema-fuzz/cli/report.ts index a948c673..d4dc258b 100644 --- a/src/benchmarks/toolcall-schema-fuzz/cli/report.ts +++ b/src/benchmarks/toolcall-schema-fuzz/cli/report.ts @@ -1,6 +1,12 @@ import { readFile } from "node:fs/promises"; -import { asyncBufferFromBytes, readResultRows } from "../../../results/parquet"; +import * as Result from "effect/Result"; + +import { + asyncBufferFromBytes, + parquetErrorMessage, + readResultRows, +} from "../../../results/parquet"; import { stabilityReport } from "../stability"; const paths = process.argv.slice(2); @@ -9,11 +15,17 @@ if (paths.length === 0) { process.exit(1); } -const rowLists = await Promise.all( - paths.map(async (path) => - readResultRows(asyncBufferFromBytes(await readFile(path))) +const rowLists = Result.all( + await Promise.all( + paths.map(async (path) => + readResultRows(asyncBufferFromBytes(await readFile(path))) + ) ) ); +if (Result.isFailure(rowLists)) { + process.stderr.write(`${parquetErrorMessage(rowLists.failure)}\n`); + process.exit(1); +} process.stdout.write( - `${JSON.stringify(stabilityReport(rowLists.flat()), null, 2)}\n` + `${JSON.stringify(stabilityReport(rowLists.success.flat()), null, 2)}\n` ); diff --git a/src/results/parquet-stream-writer.test.ts b/src/results/parquet-stream-writer.test.ts index b09cabca..095ea34d 100644 --- a/src/results/parquet-stream-writer.test.ts +++ b/src/results/parquet-stream-writer.test.ts @@ -3,12 +3,14 @@ import { describe, expect, it } from "bun:test"; import { ByteWriter } from "hyparquet-writer"; import { ScoreValue } from "../harness/core"; +import { assertSuccess } from "../internal/testing"; import { asyncBufferFromBytes, mergeResultFilesToParquet, readResultRows, runResultToParquet, } from "./parquet"; +import type { BenchmarkResultRow } from "./parquet-schema"; import { parquetStreamWriter } from "./parquet-stream-writer"; const META = { @@ -18,7 +20,9 @@ const META = { createdAt: "2026-01-01T00:00:00.000Z", } as const; -function resultRows(sampleCount: number): ReturnType { +async function resultRows( + sampleCount: number +): Promise { const sampleScores = Array.from({ length: sampleCount }, (_, index) => ({ sampleId: `s${index}`, epoch: 0, @@ -48,7 +52,9 @@ function resultRows(sampleCount: number): ReturnType { }, meta: META, }); - return readResultRows(asyncBufferFromBytes(bytes)); + const rows = await readResultRows(asyncBufferFromBytes(bytes)); + assertSuccess(rows); + return rows.success; } describe("parquetStreamWriter", () => { diff --git a/src/results/parquet.test.ts b/src/results/parquet.test.ts index 6b74a2a2..68fd67f8 100644 --- a/src/results/parquet.test.ts +++ b/src/results/parquet.test.ts @@ -3,7 +3,7 @@ import assert from "node:assert"; import type { AsyncBuffer } from "hyparquet"; import { parquetMetadata } from "hyparquet"; -import { ByteWriter } from "hyparquet-writer"; +import { ByteWriter, parquetWriteBuffer } from "hyparquet-writer"; import type { ModelMessage, ResponseItem } from "../harness/core"; import { MessageRole, ScoreValue } from "../harness/core"; @@ -15,6 +15,8 @@ import { AtifTrajectorySchema } from "./atif-schema"; import type { MergeResultFilesInput, MergedResultFiles } from "./parquet"; import { mergeResultFilesToParquet, + asyncBufferFromBytes, + parquetErrorMessage, readResultRows, runResultToParquet, rowScoreToNumber, @@ -27,6 +29,7 @@ import { BenchmarkResultRowSchema, ScorerTrajectorySchema, } from "./parquet-schema"; +import { parquetStreamWriter } from "./parquet-stream-writer"; const USAGE = { inputTokens: 100, @@ -103,9 +106,11 @@ function bufferToArrayBuffer(buf: Buffer): ArrayBuffer { return ab; } -function readRows(buf: Buffer): Promise { +async function readRows(buf: Buffer): Promise { const file = asyncBufferFromArrayBuffer(bufferToArrayBuffer(buf)); - return readResultRows(file); + const rows = await readResultRows(file); + assertSuccess(rows); + return rows.success; } async function mergeToBytes( @@ -119,7 +124,8 @@ async function mergeToBytes( writer, ...(runLevelScores !== undefined && { runLevelScores }), }); - return { bytes: Buffer.from(writer.getBuffer()), merged }; + assertSuccess(merged); + return { bytes: Buffer.from(writer.getBuffer()), merged: merged.success }; } async function mergedRowsOf( @@ -1203,18 +1209,18 @@ describe("mergeResultFilesToParquet", () => { ); let calls = 0; - await expect( - mergeResultFilesToParquet({ - files: [ - () => { - calls += 1; - return Promise.resolve(calls === 1 ? rows : rows.slice(1)); - }, - ], - meta: { task: META.task, model: META.model, createdAt: META.createdAt }, - writer: new ByteWriter(), - }) - ).rejects.toThrow("Result file 0 changed between merge passes"); + const merged = await mergeResultFilesToParquet({ + files: [ + () => { + calls += 1; + return Promise.resolve(calls === 1 ? rows : rows.slice(1)); + }, + ], + meta: { task: META.task, model: META.model, createdAt: META.createdAt }, + writer: new ByteWriter(), + }); + assertFailure(merged); + expect(merged.failure).toEqual({ kind: "result-file-changed", index: 0 }); }); it("rejects a file whose scores change between passes without changing length", async () => { const rows = await readRows( @@ -1229,18 +1235,18 @@ describe("mergeResultFilesToParquet", () => { })); let calls = 0; - await expect( - mergeResultFilesToParquet({ - files: [ - () => { - calls += 1; - return Promise.resolve(calls === 1 ? rows : flipped); - }, - ], - meta: { task: META.task, model: META.model, createdAt: META.createdAt }, - writer: new ByteWriter(), - }) - ).rejects.toThrow("Result file 0 changed between merge passes"); + const merged = await mergeResultFilesToParquet({ + files: [ + () => { + calls += 1; + return Promise.resolve(calls === 1 ? rows : flipped); + }, + ], + meta: { task: META.task, model: META.model, createdAt: META.createdAt }, + writer: new ByteWriter(), + }); + assertFailure(merged); + expect(merged.failure).toEqual({ kind: "result-file-changed", index: 0 }); }); it("rejects a file whose run-level usage changes between passes", async () => { const rows = await readRows( @@ -1252,18 +1258,128 @@ describe("mergeResultFilesToParquet", () => { })); let calls = 0; - await expect( - mergeResultFilesToParquet({ - files: [ - () => { - calls += 1; - return Promise.resolve(calls === 1 ? rows : changed); - }, - ], - meta: { task: META.task, model: META.model, createdAt: META.createdAt }, - writer: new ByteWriter(), - }) - ).rejects.toThrow("Result file 0 changed between merge passes"); + const merged = await mergeResultFilesToParquet({ + files: [ + () => { + calls += 1; + return Promise.resolve(calls === 1 ? rows : changed); + }, + ], + meta: { task: META.task, model: META.model, createdAt: META.createdAt }, + writer: new ByteWriter(), + }); + assertFailure(merged); + expect(merged.failure).toEqual({ kind: "result-file-changed", index: 0 }); + }); +}); + +describe("ParquetError", () => { + const meta = { + task: META.task, + model: META.model, + createdAt: META.createdAt, + }; + const scored: MergeResultFilesInput["runLevelScores"] = () => []; + + it("returns result-file-read-failed when a reader rejects", async () => { + const cause = new Error("disk gone"); + const merged = await mergeResultFilesToParquet({ + files: [() => Promise.reject(cause)], + meta, + writer: new ByteWriter(), + }); + assertFailure(merged); + expect(merged.failure).toEqual({ + kind: "result-file-read-failed", + index: 0, + cause, + }); + expect(parquetErrorMessage(merged.failure)).toBe("disk gone"); + }); + it("returns parquet-write-failed when the writer sink rejects", async () => { + const rows = await readRows( + runResultToParquet({ result: RESULT, meta: META }) + ); + const cause = new Error("upload closed"); + const merged = await mergeResultFilesToParquet({ + files: [() => Promise.resolve(rows)], + meta, + writer: parquetStreamWriter(() => Promise.reject(cause), 16), + }); + assertFailure(merged); + expect(merged.failure).toEqual({ kind: "parquet-write-failed", cause }); + expect(parquetErrorMessage(merged.failure)).toBe("upload closed"); + }); + it("returns parquet-decode-failed for bytes that are not parquet", async () => { + const read = await readResultRows( + asyncBufferFromBytes(new TextEncoder().encode('{"trial_results":[]}')) + ); + assertFailure(read); + expect(read.failure.kind).toBe("parquet-decode-failed"); + expect(parquetErrorMessage(read.failure)).toBe( + "parquet file invalid (footer != PAR1)" + ); + }); + it("returns invalid-result-row for parquet without the result columns", async () => { + const bytes = parquetWriteBuffer({ + columnData: [{ name: "sample_id", data: ["s0"] }], + }); + const read = await readResultRows( + asyncBufferFromBytes(new Uint8Array(bytes)) + ); + assertFailure(read); + expect(read.failure.kind).toBe("invalid-result-row"); + expect(parquetErrorMessage(read.failure)).toStartWith( + "Invalid benchmark result row: " + ); + }); + it("returns invalid-result-column-json when a scored JSON column is not JSON", async () => { + const rows = await readRows( + runResultToParquet({ result: RESULT, meta: META }) + ); + const broken = rows.map((row) => ({ ...row, metadata: "nope" })); + const merged = await mergeResultFilesToParquet({ + files: [() => Promise.resolve(broken)], + meta, + writer: new ByteWriter(), + runLevelScores: scored, + }); + assertFailure(merged); + expect(merged.failure.kind).toBe("invalid-result-column-json"); + expect(parquetErrorMessage(merged.failure)).toStartWith("JSON Parse error"); + }); + it("returns invalid-result-column when a scored JSON column fails its schema", async () => { + const rows = await readRows( + runResultToParquet({ result: RESULT, meta: META }) + ); + const broken = rows.map((row) => ({ + ...row, + scorer_trajectory: '{"kind":"unknown"}', + })); + const merged = await mergeResultFilesToParquet({ + files: [() => Promise.resolve(broken)], + meta, + writer: new ByteWriter(), + runLevelScores: scored, + }); + assertFailure(merged); + expect(merged.failure.kind).toBe("invalid-result-column"); + expect(parquetErrorMessage(merged.failure)).toStartWith( + "Invalid result column: " + ); + }); + it("formats result-file-changed with the file index", () => { + expect(parquetErrorMessage({ kind: "result-file-changed", index: 3 })).toBe( + "Result file 3 changed between merge passes" + ); + }); + it("formats a non-Error cause the way unknownErrorToString does", () => { + expect( + parquetErrorMessage({ kind: "parquet-decode-failed", cause: "boom" }) + ).toBe("boom"); + expect( + parquetErrorMessage({ kind: "parquet-write-failed", cause: { code: 7 } }) + ).toBe('{"code":7}'); }); }); diff --git a/src/results/parquet.ts b/src/results/parquet.ts index 1a59c65a..752fd49d 100644 --- a/src/results/parquet.ts +++ b/src/results/parquet.ts @@ -16,7 +16,10 @@ import { ScoreValue } from "../harness/core"; import type { AggregateMetrics, SampleScore } from "../harness/metric"; import { aggregateScores } from "../harness/metric"; import type { RunResult } from "../harness/run"; +import { unknownErrorToString } from "../internal/errors"; import { definedValues } from "../internal/guards"; +import type { AsyncResult } from "../internal/result"; +import { tryPromiseResult } from "../internal/result"; import { firstZodIssueMessage, parseSchema, z } from "../internal/zod"; import { citationToPojo, @@ -166,6 +169,19 @@ export interface MergedResultFiles { readonly benchmarkConfig: string | null; } +export type ParquetError = + | { + readonly kind: "result-file-read-failed"; + readonly index: number; + readonly cause: unknown; + } + | { readonly kind: "result-file-changed"; readonly index: number } + | { readonly kind: "parquet-write-failed"; readonly cause: unknown } + | { readonly kind: "parquet-decode-failed"; readonly cause: unknown } + | { readonly kind: "invalid-result-row"; readonly issue: string } + | { readonly kind: "invalid-result-column-json"; readonly cause: unknown } + | { readonly kind: "invalid-result-column"; readonly issue: string }; + type MergedRunColumns = Pick< BenchmarkResultRow, | "format_version" @@ -214,15 +230,19 @@ const RESULT_SCHEMA = schemaFromColumnData({ export async function mergeResultFilesToParquet( input: MergeResultFilesInput -): Promise { +): AsyncResult { const { files, meta, writer } = input; const createdAt = meta.createdAt ?? formatIso(nowUnsafe()); - const scan = await scanResultFiles( + const scanned = await scanResultFiles( files, input.runLevelScores === undefined - ? rowsToSampleScores + ? (rows) => Result.succeed(rowsToSampleScores(rows)) : rowsToScoredSamples ); + if (Result.isFailure(scanned)) { + return Result.fail(scanned.failure); + } + const scan = scanned.success; const extraScores = input.runLevelScores?.({ metrics: aggregateScores(scan.sampleScores), usage: scan.usage, @@ -245,7 +265,11 @@ export async function mergeResultFilesToParquet( if (expectedRows === 0) { continue; } - const rows = await readFile(); + const read = await readResultFile(readFile, index); + if (Result.isFailure(read)) { + return Result.fail(read.failure); + } + const rows = read.success; const unchanged = rows.length === expectedRows && runFingerprint(rows[0]) === scan.runFingerprints[index] && @@ -253,20 +277,20 @@ export async function mergeResultFilesToParquet( sameSampleScore(row, scan.sampleScores[sampleOffset + rowIndex]) ); if (!unchanged) { - throw new Error(`Result file ${index} changed between merge passes`); + return Result.fail({ kind: "result-file-changed", index }); } sampleOffset += expectedRows; const mergedRows = rows.map((row) => ({ ...row, ...runColumns })); - await parquetWriter.write({ - columnData: COLUMN_SPECS.map((spec) => ({ - name: spec.name, - data: mergedRows.map((row) => row[spec.name] ?? null), - })), - rowGroupSize: MERGE_ROW_GROUP_ROWS, - }); + const written = await writeParquetRows(parquetWriter, mergedRows); + if (Result.isFailure(written)) { + return Result.fail(written.failure); + } } - await parquetWriter.finish(); - return { + const finished = await tryParquetWrite(() => parquetWriter.finish()); + if (Result.isFailure(finished)) { + return Result.fail(finished.failure); + } + return Result.succeed({ summary: scan.first === undefined ? null @@ -276,13 +300,75 @@ export async function mergeResultFilesToParquet( ...definedValues({ primaryScore: scan.primaryScore }), }), benchmarkConfig: runColumns.benchmark_config ?? null, - }; + }); +} + +export function parquetErrorMessage(error: ParquetError): string { + switch (error.kind) { + case "result-file-read-failed": + case "parquet-write-failed": + case "parquet-decode-failed": + case "invalid-result-column-json": { + return unknownErrorToString(error.cause); + } + case "result-file-changed": { + return `Result file ${error.index} changed between merge passes`; + } + case "invalid-result-row": { + return `Invalid benchmark result row: ${error.issue}`; + } + case "invalid-result-column": { + return `Invalid result column: ${error.issue}`; + } + default: { + return error satisfies never; + } + } +} + +async function readResultFile( + readFile: ResultFileReader, + index: number +): AsyncResult { + return Result.mapError(await tryPromiseResult(readFile), (cause) => ({ + kind: "result-file-read-failed", + index, + cause, + })); +} + +function writeParquetRows( + parquetWriter: ParquetWriter, + mergedRows: readonly BenchmarkResultRow[] +): AsyncResult { + return tryParquetWrite(() => + parquetWriter.write({ + columnData: COLUMN_SPECS.map((spec) => ({ + name: spec.name, + data: mergedRows.map((row) => row[spec.name] ?? null), + })), + rowGroupSize: MERGE_ROW_GROUP_ROWS, + }) + ); +} + +async function tryParquetWrite( + step: () => void | Promise +): AsyncResult { + return Result.mapError( + await tryPromiseResult(async () => { + await step(); + }), + (cause) => ({ kind: "parquet-write-failed", cause }) + ); } async function scanResultFiles( files: readonly ResultFileReader[], - toSampleScores: (rows: readonly BenchmarkResultRow[]) => SampleScore[] -): Promise { + toSampleScores: ( + rows: readonly BenchmarkResultRow[] + ) => Result.Result +): AsyncResult { const rowCounts: number[] = []; const runFingerprints: (string | undefined)[] = []; const sampleScores: SampleScore[] = []; @@ -298,8 +384,12 @@ async function scanResultFiles( let primaryScoreValue = 0; let primaryScoreWeight = 0; let hasPrimaryScore = false; - for (const readFile of files) { - const rows = await readFile(); + for (const [index, readFile] of files.entries()) { + const read = await readResultFile(readFile, index); + if (Result.isFailure(read)) { + return Result.fail(read.failure); + } + const rows = read.success; rowCounts.push(rows.length); runFingerprints.push(runFingerprint(rows[0])); const [row] = rows; @@ -311,7 +401,11 @@ async function scanResultFiles( temperature: row.temperature, benchmark_config: row.benchmark_config, }; - for (const sampleScore of toSampleScores(rows)) { + const fileSampleScores = toSampleScores(rows); + if (Result.isFailure(fileSampleScores)) { + return Result.fail(fileSampleScores.failure); + } + for (const sampleScore of fileSampleScores.success) { sampleScores.push(sampleScore); } const parsedPrimaryScore = parsePrimaryScore(row.primary_score); @@ -329,7 +423,7 @@ async function scanResultFiles( usage.totalCost += row.total_cost; usage.generationTimeMs += row.generation_time_ms; } - return { + return Result.succeed({ rowCounts, runFingerprints, sampleScores, @@ -342,7 +436,7 @@ async function scanResultFiles( weight: primaryScoreWeight, } : undefined, - }; + }); } function runFingerprint( @@ -574,8 +668,7 @@ function cellValue(name: ColumnName, ctx: RowContext, s: SampleScore): unknown { return s.metadata !== undefined ? JSON.stringify(s.metadata) : null; } default: { - name satisfies never; - throw new Error(`Unhandled column: ${name}`); + return name satisfies never; } } } @@ -627,18 +720,24 @@ export function rowScoreToNumber(scoreValue: string): number { return scoreValue === ScoreValue.Correct ? 1 : 0; } -export function readResultRows( +export async function readResultRows( file: AsyncBuffer -): Promise { - return parquetReadObjects({ file }).then((decoded) => { - const result = parseSchema(z.array(BenchmarkResultRowSchema), decoded); - if (Result.isFailure(result)) { - throw new Error( - `Invalid benchmark result row: ${firstZodIssueMessage(result.failure)}` - ); - } - return result.success; - }); +): AsyncResult { + const decoded = Result.mapError( + await tryPromiseResult(() => parquetReadObjects({ file })), + (cause): ParquetError => ({ kind: "parquet-decode-failed", cause }) + ); + if (Result.isFailure(decoded)) { + return Result.fail(decoded.failure); + } + const rows = parseSchema(z.array(BenchmarkResultRowSchema), decoded.success); + if (Result.isFailure(rows)) { + return Result.fail({ + kind: "invalid-result-row", + issue: firstZodIssueMessage(rows.failure), + }); + } + return Result.succeed(rows.success); } export function asyncBufferFromBytes(bytes: Uint8Array): AsyncBuffer { @@ -710,8 +809,28 @@ function rowsToSampleScores( function rowsToScoredSamples( rows: readonly BenchmarkResultRow[] -): SampleScore[] { - return rows.map((row) => +): Result.Result { + return Result.all(rows.map(rowToScoredSample)); +} + +function rowToScoredSample( + row: BenchmarkResultRow +): Result.Result { + const trajectory = parseJsonColumn( + ScorerTrajectorySchema, + row.scorer_trajectory + ); + if (Result.isFailure(trajectory)) { + return Result.fail(trajectory.failure); + } + const metadata = parseJsonColumn( + z.record(z.string(), z.unknown()), + row.metadata + ); + if (Result.isFailure(metadata)) { + return Result.fail(metadata.failure); + } + return Result.succeed( definedValues({ sampleId: row.sample_id, epoch: row.epoch, @@ -719,15 +838,9 @@ function rowsToScoredSamples( value: rowScoreValue(row.score_value), answer: row.answer, explanation: row.explanation ?? "", - trajectory: parseJsonColumn( - ScorerTrajectorySchema, - row.scorer_trajectory - ), + trajectory: trajectory.success, }), - metadata: parseJsonColumn( - z.record(z.string(), z.unknown()), - row.metadata - ), + metadata: metadata.success, }) ); } @@ -735,17 +848,28 @@ function rowsToScoredSamples( function parseJsonColumn( schema: z.ZodType, raw: string | null | undefined -): T | undefined { +): Result.Result { if (raw === null || raw === undefined) { - return undefined; + return Result.succeed(undefined); } - const parsed = parseSchema(schema, JSON.parse(raw)); + const json = Result.try({ + try: (): unknown => JSON.parse(raw), + catch: (cause): ParquetError => ({ + kind: "invalid-result-column-json", + cause, + }), + }); + if (Result.isFailure(json)) { + return Result.fail(json.failure); + } + const parsed = parseSchema(schema, json.success); if (Result.isFailure(parsed)) { - throw new Error( - `Invalid result column: ${firstZodIssueMessage(parsed.failure)}` - ); + return Result.fail({ + kind: "invalid-result-column", + issue: firstZodIssueMessage(parsed.failure), + }); } - return parsed.success; + return Result.succeed(parsed.success); } function extraScoresToJson(