[sunwrobert]

TL;DR

mergeResultFilesToParquet, readResultRows and parseJsonColumn in src/results/parquet.ts return Result<T, ParquetError> (effect/Result) instead of throwing on expected runtime-data failures; parquetErrorMessage() reproduces every previous message byte for byte, so callers see the same text.

throw-to-result: src/results/parquet.ts (vendored in openrouter-web as packages/bench-harness/src/results/parquet.ts)
remaining (harness, git grep -E "^\s*throw new Error\(" -- 'src/**/*.ts' ':!*.test.ts' | grep -vE "// invariant:" | wc -l): 60 → 56
remaining (openrouter-web, git grep -E "^\s*throw new Error\(" -- 'packages/**/*.ts' 'services/**/*.ts' ':!*.test.ts' ':!*.integration.test.ts' | grep -vE "// invariant:" | wc -l on main): 1298 → 1298 until the subtree-pull PR lands, then 1294

Result convention: this package uses effect/Result (Result.succeed/fail/isFailure, src/internal/result.ts AsyncResult/tryPromiseResult), not @openrouter-monorepo/lib-result; the code-scan profile names lib-result, the package convention wins.

Sibling: the openrouter-web PR (subtree pull + caller updates in services/gcp-harbor-run/src/publish-kepler-run.ts, services/gcp-harbor-run/src/result-rows.ts, packages/temporal/src/activities/benchmark/import-existing-chunks.ts) opens after this merges; scripts/check-subtree-integrity.ts rejects any direct edit to the vendored copy.

What changed?

• ParquetError union, one kind per failure, carrying only the interpolated data:

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 };

• mergeResultFilesToParquet(): AsyncResult<MergedResultFiles, ParquetError>: readFile() rejections, the changed-between-passes check, and ParquetWriter.write/finish rejections become Result.fail(...); scanResultFiles and the per-row toSampleScores callback return Results and short-circuit on the first failure.
• readResultRows(): AsyncResult<readonly BenchmarkResultRow[], ParquetError>: parquetReadObjects rejections (parquet-decode-failed) and Zod row failures (invalid-result-row).
• parseJsonColumn(): Result<T | undefined, ParquetError>: JSON.parse wrapped with Result.try (invalid-result-column-json), Zod failures (invalid-result-column); rowsToScoredSamples collects per-row Results with Result.all instead of letting the first throw escape the .map.
• parquetErrorMessage(error): string: exhaustive switch producing the exact previous strings; cause kinds go through unknownErrorToString (src/internal/errors.ts), which is the same function the monorepo callers applied to the wrapped exception.
• cellValue default: return name satisfies never; instead of throw new Error("Unhandled column"). The requested // invariant: exhaustive ColumnName switch marker cannot be added in this repo: openrouter/no-comments (oxlint, tools/oxlint/no-comments.js) rejects every non-directive comment, and the function already returns unknown, so the throw is not needed.
• src/benchmarks/toolcall-schema-fuzz/cli/report.ts: branches on the Result and writes parquetErrorMessage() to stderr with exit 1.
• Tests: the three rejects.toThrow("Result file 0 changed between merge passes") cases became assertFailure + toEqual({ kind: "result-file-changed", index: 0 }) with the same inputs; one new test per kind plus parquetErrorMessage formatting tests; readRows/mergeToBytes/resultRows helpers and ori-parquet.test.ts narrow with assertSuccess.

Callers (all in this repo): src/benchmarks/toolcall-schema-fuzz/cli/report.ts, src/results/parquet.test.ts, src/results/parquet-stream-writer.test.ts, src/benchmarks/terminal-bench/ori-parquet.test.ts. Monorepo callers (sibling PR): services/gcp-harbor-run/src/publish-kepler-run.ts, services/gcp-harbor-run/src/result-rows.ts, packages/temporal/src/activities/benchmark/import-existing-chunks.ts.

purely mechanical: no

• toolcall-schema-fuzz/cli/report.ts: an unreadable results file used to end in Bun's uncaught-rejection stack dump on stderr; it now prints the one-line parquetErrorMessage() on stderr. Exit code 1 in both cases.
• A readFile() reader or a Writer sink that rejects used to reject the mergeResultFilesToParquet promise; it now resolves to Result.fail({ kind: "result-file-read-failed" | "parquet-write-failed", cause }). Monorepo callers wrapped the promise, so their output is unchanged (Kepler result merge failed: <unknownErrorToString(cause)> both before and after).
• cellValue default no longer throws (unreachable: name satisfies never).

// invariant: markers added: none (repo lint bans comments; the one candidate site was removed instead).

Prod impact: none until the openrouter-web subtree pull. Worst case after that: a ParquetError kind formats a different string than the old exception message, which parquetErrorMessage tests pin for every kind; the Kepler publish and result-rows messages are asserted unchanged in the monorepo tests (publish-kepler-run.test.ts, result-rows.test.ts).

Why?

Code-scan finding sfind-1710b4727c8d4d5096e41abab2a6114e: expected data failures were exceptions, so every caller caught generically with wrap() and stringified; the type system could not show which failures a caller handled. AGENTS.md: "Preserve the repository's strict TypeScript and Effect patterns."

How to test

bun install --frozen-lockfile
bun test src/results src/benchmarks/terminal-bench/ori-parquet.test.ts   # 83 pass
bun test                                                                 # 1837 pass, 170 files
bun run format:check && bun run check && bun run typecheck && bun run build
printf 'not parquet' > /tmp/x.parquet && bun src/benchmarks/toolcall-schema-fuzz/cli/report.ts /tmp/x.parquet; echo "exit $?"
# stderr: parquet file invalid (footer != PAR1)   exit 1

Show me

 mergeResultFilesToParquet(input)
-  scanResultFiles(files, rows => SampleScore[])        // throws from parseJsonColumn / readFile
+  scanResultFiles(files, rows => Result<SampleScore[], ParquetError>)
+    readResultFile(readFile, index)                     // result-file-read-failed
+    rowsToScoredSamples → Result.all(rows.map(rowToScoredSample))
+      parseJsonColumn → Result.try(JSON.parse)          // invalid-result-column-json
+                      → parseSchema                     // invalid-result-column
   for each file:
-    readFile(); if changed: throw new Error(`Result file ${index} changed between merge passes`)
-    parquetWriter.write(...)
+    readResultFile(...); if changed: return Result.fail({ kind: "result-file-changed", index })
+    writeParquetRows(...) → tryParquetWrite             // parquet-write-failed
-  parquetWriter.finish()
+  tryParquetWrite(() => parquetWriter.finish())
-  return MergedResultFiles
+  return Result.succeed(MergedResultFiles)

 readResultRows(file)
-  parquetReadObjects(...).then(parseSchema → throw new Error(`Invalid benchmark result row: ...`))
+  tryPromiseResult(parquetReadObjects) → parquet-decode-failed
+  parseSchema → invalid-result-row

+parquetErrorMessage(error)  // exhaustive switch, same strings as the old throws

Visualization

Artifact: https://openrouter.devinenterprise.com/api/presigned_proxy?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1c2VyX2lkIjpudWxsLCJidWNrZXRfbmFtZSI6ImRldmluYXR0YWNobWVudHMiLCJidWNrZXRfa2V5IjoiYXR0YWNobWVudHNfcHJpdmF0ZS9jbGVyay1vcmdfMmhUSzBmbWdTSk80ZncwT041T1NwOUtFZG1iL2QzMDk3OTdlLTNjM2EtNGEzNi1iYzAxLWEwZWJhZjM2OTQ0OCIsImlhdCI6MTc5MzQ4MzI5NSwib3JnX2lkIjoiY2xlcmstb3JnXzJoVEswZm1nU0pPNGZ3ME9ONU9TcDlLRWRtYiIsImZpbGVuYW1lIjoidGhyb3ctdG8tcmVzdWx0LXBhcnF1ZXQtMjAyNi0xMS0wMS5odG1sIn0.wZRRAqoF0DHrvG2jp4G4VINIfp5vOGZ0T-NJXSwWCQY

Proof matrix

Base = origin/main c6a05aa in a worktree with the head's src/results/parquet.test.ts and parquet-stream-writer.test.ts checked out; head = cc63161. Command on both: bun test src/results/parquet.test.ts src/results/parquet-stream-writer.test.ts. On base the head test file fails at module load (parquetErrorMessage is not exported there), so the base column was run with that import patched to String(e); everything else is identical test code.

Behavior	Unit	Integration	E2E
changed file between passes → result-file-changed	parquet.test.ts › "rejects a file whose row count / scores / run-level usage changes between passes" (3). Base: fail, uncaught Result file 0 changed between merge passes. Head: pass, toEqual({ kind: "result-file-changed", index: 0 })	n/a (pure in-memory module, no service boundary)	n/a (no user-facing surface; CLI covered by the stderr smoke in How to test)
reader rejects → result-file-read-failed	ParquetError › returns result-file-read-failed when a reader rejects. Base: fail, uncaught disk gone. Head: pass, message disk gone	n/a	n/a
writer sink rejects → parquet-write-failed	ParquetError › returns parquet-write-failed when the writer sink rejects. Base: fail, uncaught upload closed. Head: pass	n/a	n/a
non-parquet bytes → parquet-decode-failed	ParquetError › returns parquet-decode-failed for bytes that are not parquet. Base: fail (module load, see note). Head: pass, parquet file invalid (footer != PAR1)	n/a	n/a
missing result columns → invalid-result-row	ParquetError › returns invalid-result-row for parquet without the result columns. Base: fail (module load). Head: pass, starts with Invalid benchmark result row:	n/a	n/a
non-JSON scored column → invalid-result-column-json	ParquetError › returns invalid-result-column-json when a scored JSON column is not JSON. Base: fail, uncaught SyntaxError. Head: pass, starts with JSON Parse error	n/a	n/a
schema-failing scored column → invalid-result-column	ParquetError › returns invalid-result-column when a scored JSON column fails its schema. Base: fail, uncaught Invalid result column: .... Head: pass	n/a	n/a
message text unchanged (equivalence)	ParquetError › formats result-file-changed with the file index, formats a non-Error cause the way unknownErrorToString does; all 34 pre-existing runResultToParquet/mergeResultFilesToParquet/summarizeChunkRows round-trip tests pass on head with their original assertions (byte-identical parquet output, parquetStreamWriter byte comparison)	n/a	n/a

Unchanged-input equivalence: bun test full suite 1837 pass / 0 fail on head (1837 on main).

Thermo-nuclear fix loop

Round 1 (97e7751): three near-identical "tryPromiseResult then rebuild the failure" ternaries (readResultFile, writeParquetRows, finish) → collapsed onto Result.mapError and one tryParquetWrite(step); SampleScoresOf alias (single use) inlined; rowsToUnscoredSamples wrapper inlined at its only call site. Fixed in cc63161.

Round 2 (cc63161): no findings. File is 919 lines (was 843), no new conditionals in shared paths, no casts, no any.

Deslop-micro (fix mode) on cc63161: inventories ParquetError, parquetErrorMessage, readResultFile, writeParquetRows, tryParquetWrite, rowToScoredSample — all keep (names match the module's existing writeParquet/readResultRows vocabulary; tryParquetWrite mirrors tryPromiseResult). Invented terms removed in round 1 (SampleScoresOf, rowsToUnscoredSamples). No separate deslop commit.

Benchmark impact

None: no solver, scorer, prompt or dataset change; merged parquet bytes are unchanged (parquetStreamWriter byte-equality test).

Reviewer focus

• parquetErrorMessage strings against the removed throw new Error(...) texts.
• Result.all(rows.map(rowToScoredSample)) reports the first failing row in row order, as the old throw-in-.map did.
• Wrapping readFile() and the Writer sink: this keeps the monorepo mergeKeplerResults output identical once it drops wrap().

Checklist

• Tests cover changed behavior
• Public API or configuration changes are backward compatible, or the break is documented (return types of mergeResultFilesToParquet / readResultRows change; the only external consumer is openrouter-web, updated in the sibling PR)
• Benchmark changes document dataset provenance and licensing (n/a)
• No credentials, private results, or restricted dataset contents are included
• Documentation is updated where needed (none needed)

Link to Devin session: https://openrouter.devinenterprise.com/sessions/c4ce6b7cd05344d3bd734eb3233e6935
Open in Devin Desktop: https://openrouter.devinenterprise.com/desktop/session/c4ce6b7cd05344d3bd734eb3233e6935?variant=devin
Requested by: @sunwrobert

---

[devin-ai-integration]

I'll fix CI failures and address comments from users with write access that start with 'Devin'.

• Disable automatic comment, CI, and merge conflict monitoring

---

Original prompt from Devin Bot

Please remediate these findings together. --code-scan-step fix:sfind-1710b4727c8d4d5096e41abab2a6114e
Shared root cause: in packages/bench-harness/src/results/parquet.ts (OpenRouterTeam/openrouter-web), mergeResultFilesToParquet (~L256, result file changed between passes), readResultRows (~L636, invalid rows) and parseJsonColumn (~L744, invalid JSON column; also an unwrapped JSON.parse at ~L742) throw on expected runtime-data failures. All three production callers (services/gcp-harbor-run/src/publish-kepler-run.ts, result-rows.ts, the toolcall-schema-fuzz CLI) catch generically with wrap() and stringify the message. Return Results from these functions with the same messages, collect Results where parseJsonColumn runs inside a .map, update the callers to branch on the Result instead of wrap() so output is unchanged, and mark the exhaustive satisfies never default at ~L578 with // invariant: exhaustive ColumnName switch. Finding guidance notes this package uses effect/Result; the profile names lib-result — follow the package's existing convention and state the choice in the PR body.

@playbook:playbook-063fbf6c0b3945bf948a331c5e4f011d

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[perry-v2]

✅ Approved. The Parquet read/merge APIs now return typed Results for expected runtime-data failures, and the in-repository callers handle those Results. No actionable correctness regressions found.

Scope and evidence

Reviewed all five changed files at cc63161, against actual base c6a05aa324948d4fd66b89c2c7e8b9a3065ee01a, with two independent correctness passes. Examined the two-pass merge checks, reader and writer failure propagation, Parquet decoding and row validation, scored-column JSON/schema parsing, error-message formatting, CLI handling, and migrated test helpers. Candidate concerns about reader API composition and unchecked Results in the streaming test were independently checked and did not establish reachable defects.

Risk and boundaries

The public return types intentionally change, so consumers must unwrap Results rather than use rows or merge metadata directly. The in-repository consumers were checked; the external monorepo subtree migration was not available in this checkout and is not verified by this approval. Successful serialization and merge-consistency guards remain in place. Third-party writer internals were not available for inspection, so this review does not establish that every possible dependency exception is converted to a Result.

Verification

Static review only: no PR code, tests, installs, or builds were run. Inspected the changed round-trip/streaming assertions, changed-between-passes cases, typed error cases, and message-formatting coverage; the author's reported test results were not independently reproduced. CI was not checked.