From 2b238a3123cdc338364c3d11a1ad2dc157ad0934 Mon Sep 17 00:00:00 2001 From: Josh Owens Date: Fri, 18 Sep 2026 20:51:42 +0000 Subject: [PATCH 1/8] feat: per-turn transcripts (--transcript-out) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The aggregate `usage` on a run's result object can bound whether an injected context block was served from cache, but it cannot show how context grew turn by turn. `--no-session-persistence` means no transcript is written either, so there was no per-turn record anywhere. `--transcript-out ` (compare and measure) runs claude with `--output-format stream-json --verbose` and writes each run's event stream as `__.stream.jsonl`, one line per event as it arrives. Runners emit lines through a new `onStreamEvent` sink and never learn what a scenario is: the engine opens the file, names it, and closes it in a `finally`, so `buildClaudeArgs` stays pure and naming lives next to raw persistence. Three paths assumed stdout was one parseable JSON object and now work off a terminal `result` event decoded while reading: the success parse, the `error_max_turns` recovery, and `describeClaudeFailure`'s budget-abort decode. A stream killed mid-flight has no terminal event and throws, keeping the lines it printed — for a cost investigation a partial stream is evidence. `RunnerCapabilities` gains `streamEvents` so `--transcript-out --runner openai` fails before any paid run instead of writing empty files. Closes #39 Co-Authored-By: Claude Opus 5 --- README.md | 26 ++++++++ SPEC.md | 6 +- src/cli.ts | 21 +++++- src/engine/compare.ts | 93 ++++++++++++++++++++++---- src/runner/claude-p.ts | 102 +++++++++++++++++++++++++--- src/runner/openai-compat.ts | 2 +- src/types.ts | 12 ++++ test/cache.test.ts | 34 +++++++++- test/compare.test.ts | 100 +++++++++++++++++++++++++-- test/delivery.test.ts | 2 +- test/json-grader.test.ts | 2 +- test/judge.test.ts | 8 +-- test/measure.test.ts | 2 +- test/openai-runner.test.ts | 4 +- test/receipt.test.ts | 2 +- test/render.test.ts | 2 +- test/runner-support.test.ts | 4 +- test/runner.test.ts | 130 ++++++++++++++++++++++++++++++++++++ 18 files changed, 506 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 99bb34b..bd4eaa9 100644 --- a/README.md +++ b/README.md @@ -343,6 +343,32 @@ categories, not the blend. Files are written per run, so a compare that dies midway still leaves the completed runs' records. Cache-served baseline arms ran in an earlier invocation and write nothing. +### Per-turn transcripts + +The raw result is still an aggregate: one `usage` total for the whole run. It +can tell you a prompt's uncached input stayed small; it cannot tell you how +context grew between turn 2 and turn 3, or which turn paid for the cache +write. `--transcript-out ` (on `compare` and `measure`, claude-p only) +runs claude with `--output-format stream-json --verbose` and writes the full +event stream as `__.stream.jsonl`, one JSON event per line: + +```bash +promptdiff compare --scenario ./scenario.json --transcript-out ./transcripts + +# per-turn input growth, cache reads, and cache writes for one run +jq -r 'select(.type == "assistant") | .message.usage + | [.input_tokens, .cache_read_input_tokens, .cache_creation_input_tokens] | @tsv' \ + transcripts/injected-context_proposed_1.stream.jsonl +``` + +Lines are written as they arrive, so a run killed by the timeout or a budget +abort keeps everything it printed up to the kill — for a cost investigation a +truncated stream is the evidence. The runner still passes +`--no-session-persistence`: capture replaces the transcript without writing +anything to `~/.claude/projects`. Stream files are large (every tool result +and every message block), so the flag is off by default and best pointed at a +throwaway directory. Cache-served baseline arms write nothing here either. + ### Run history `--report ndjson --report-out ./runs.ndjson` appends one record per scenario diff --git a/SPEC.md b/SPEC.md index 5243769..b74d0e5 100644 --- a/SPEC.md +++ b/SPEC.md @@ -194,7 +194,11 @@ state. `--raw-out ` (compare and measure) additionally writes each run's full runner result JSON — token usage categories, modelUsage, subtype — one file per completed run, for token-level analysis that the digested summaries cannot support; files land as runs finish, so a partial invocation still -leaves its completed records. +leaves its completed records. `--transcript-out ` (compare and +measure, streamEvents runners only) additionally captures the runner's +per-event stream as NDJSON, one `__.stream.jsonl` per run +written event by event — per-turn usage and tool calls, which the aggregate +result object cannot carry. A run killed mid-stream keeps its partial file. `compare` scenarios assert nothing: they exist to report both arms' pass rates and the delta, for comparisons (typically model-vs-model) where neither diff --git a/src/cli.ts b/src/cli.ts index 25522c4..b1f5966 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -70,6 +70,7 @@ const compareSpecs: FlagSpecs = { "report-out": { arity: "one" }, receipts: { arity: "one" }, "raw-out": { arity: "one" }, + "transcript-out": { arity: "one" }, cache: { arity: "none" }, "cache-dir": { arity: "one" }, }; @@ -241,6 +242,7 @@ const measureSpecs: FlagSpecs = { "keep-sandbox": { arity: "none" }, receipts: { arity: "one" }, "raw-out": { arity: "one" }, + "transcript-out": { arity: "one" }, }; async function cmdMeasure(argv: string[]): Promise { @@ -272,12 +274,14 @@ async function cmdMeasure(argv: string[]): Promise { }; const rawOutDir = args.one("raw-out"); + const transcriptOutDir = args.one("transcript-out"); const config = loadCompareConfig(scenario, overrides, { singleArm: true }); const summary = await runMeasure({ config, runner: armRunner(config.arms.baseline, config), onProgress: (message) => console.error(`[promptdiff] ${message}`), rawOut: rawOutDir === undefined ? undefined : { dir: rawOutDir }, + transcriptOut: transcriptOutDir === undefined ? undefined : { dir: transcriptOutDir }, }); const receiptsDir = args.one("receipts"); @@ -369,6 +373,7 @@ async function cmdCompare(argv: string[]): Promise { }; const rawOutDir = args.one("raw-out"); + const transcriptOutDir = args.one("transcript-out"); const config = loadCompareConfig(scenario, overrides); const summary = await runCompare({ config, @@ -379,6 +384,7 @@ async function cmdCompare(argv: string[]): Promise { onProgress: (message) => console.error(`[promptdiff] ${message}`), cache, rawOut: rawOutDir === undefined ? undefined : { dir: rawOutDir }, + transcriptOut: transcriptOutDir === undefined ? undefined : { dir: transcriptOutDir }, }); // History is appended before the exit code is decided — failed comparisons @@ -545,7 +551,8 @@ function compareUsage(): string { " --mode --tools ", " --timeout-ms --max-budget-usd --max-turns ", " --report ndjson --report-out ", - " --raw-out --cache [--cache-dir ]", + " --raw-out --transcript-out ", + " --cache [--cache-dir ]", "", "turn cap:", " --max-turns (or scenario \"maxTurns\", per-case override allowed) caps", @@ -558,6 +565,14 @@ function compareUsage(): string { " run — the token-level record that summaries and reports digest away.", " Cache-served baseline arms ran earlier and write nothing here.", "", + "transcripts:", + " --transcript-out runs claude with --output-format stream-json and", + " writes the full per-event NDJSON as __.stream.jsonl,", + " one line per event as it arrives — per-turn usage (cache reads, cache", + " creation, context growth) and tool calls, which the aggregate result", + " cannot show. Files are large; a run killed by the timeout keeps the", + " partial stream. claude-p only; cache-served arms write nothing.", + "", "caching:", " --cache reuses recorded baseline-arm results (default dir .promptdiff/cache)", " when nothing that could change the outcome changed: rendered prompts, skill", @@ -664,7 +679,7 @@ function measureUsage(): string { " --mode --tools ", " --sandbox --seed --keep-sandbox", " --timeout-ms --max-budget-usd --max-turns ", - " --receipts --raw-out ", + " --receipts --raw-out --transcript-out ", "", "--receipts writes one .receipt.json per scenario with", "per-file prompt hashes and the measured rates (verdict \"measured\").", @@ -672,6 +687,8 @@ function measureUsage(): string { "--max-turns caps agentic turns per run (claude-p); a capped run is", "scored as a failure without grading. --raw-out writes each run's", "full runner result JSON (token usage included) as _measure_.json.", + "--transcript-out additionally captures the per-event stream-json", + "NDJSON as _measure_.stream.jsonl (claude-p only, large).", "", "Exit code is 0 whenever the runs complete — a measurement has no pass/fail.", ].join("\n"); diff --git a/src/engine/compare.ts b/src/engine/compare.ts index 4c8ac3e..8e2696d 100644 --- a/src/engine/compare.ts +++ b/src/engine/compare.ts @@ -1,5 +1,5 @@ import { createHash } from "node:crypto"; -import { mkdirSync, writeFileSync } from "node:fs"; +import { closeSync, mkdirSync, openSync, writeFileSync, writeSync } from "node:fs"; import { join } from "node:path"; import { assembleSystemPrompt } from "../prompt"; import type { Runner, RunnerRunOptions, RunResult } from "../types"; @@ -68,14 +68,21 @@ export interface CompareRunOptions { * nothing here. */ rawOut?: { dir: string }; + /** + * Opt-in per-run transcript capture: the runner's full event stream is + * written to this directory as NDJSON while the run is in flight. Needs a + * streamEvents runner. Cache-served baseline arms write nothing here. + */ + transcriptOut?: { dir: string }; } export async function runCompare(options: CompareRunOptions): Promise { - const { config, runners, onProgress, cache, rawOut } = options; + const { config, runners, onProgress, cache, rawOut, transcriptOut } = options; // Validate each arm against its own runner — a mixed comparison fails on the // violating arm before either arm's paid runs. - validateRunnerSupport(config, runners.baseline); - validateRunnerSupport(config, runners.proposed); + const transcripts = transcriptOut !== undefined; + validateRunnerSupport(config, runners.baseline, { transcripts }); + validateRunnerSupport(config, runners.proposed, { transcripts }); assertJudgeGradersCalibrated(config); // Install delivery keeps skill text out of the system prompt entirely — the // arms differ only by which skill directory lands in the sandbox registry. @@ -89,8 +96,8 @@ export async function runCompare(options: CompareRunOptions): Promise void; /** Opt-in per-run raw persistence; see CompareRunOptions.rawOut. */ rawOut?: { dir: string }; + /** Opt-in per-run transcript capture; see CompareRunOptions.transcriptOut. */ + transcriptOut?: { dir: string }; } /** @@ -297,8 +315,8 @@ export interface MeasureRunOptions { * nonsense verdicts from sampling noise. */ export async function runMeasure(options: MeasureRunOptions): Promise { - const { config, runner, onProgress, rawOut } = options; - validateRunnerSupport(config, runner); + const { config, runner, onProgress, rawOut, transcriptOut } = options; + validateRunnerSupport(config, runner, { transcripts: transcriptOut !== undefined }); assertJudgeGradersCalibrated(config); const inline = config.delivery !== "install"; const basePrompt = assembleSystemPrompt(config.agent, inline ? config.baselineSkills : []); @@ -315,7 +333,7 @@ export async function runMeasure(options: MeasureRunOptions): Promise void, rawOut?: { dir: string }, + transcriptOut?: { dir: string }, ): Promise { if (cache === undefined) { - return runArm(config, evalCase, "baseline", systemPrompt, runner, onProgress, "baseline", rawOut); + return runArm(config, evalCase, "baseline", systemPrompt, runner, onProgress, "baseline", rawOut, transcriptOut); } const key = buildCacheKey({ systemPrompt, @@ -421,9 +440,12 @@ async function runBaselineArm( if (rawOut !== undefined) { onProgress?.(" baseline: cache hit — no raw results to write"); } + if (transcriptOut !== undefined) { + onProgress?.(" baseline: cache hit — no transcript to write"); + } return { ...hit, cached: true }; } - const summary = await runArm(config, evalCase, "baseline", systemPrompt, runner, onProgress, "baseline", rawOut); + const summary = await runArm(config, evalCase, "baseline", systemPrompt, runner, onProgress, "baseline", rawOut, transcriptOut); storeCachedArm(cache.dir, key, summary); return summary; } @@ -437,6 +459,7 @@ async function runArm( onProgress?: (message: string) => void, label: string = arm, rawOut?: { dir: string }, + transcriptOut?: { dir: string }, ): Promise { const runs = evalCase.runs ?? config.runs; const summaries: ArmRunSummary[] = []; @@ -450,6 +473,11 @@ async function runArm( keep: config.keepSandbox, }); + // Opened before the run so a timeout kill still leaves the lines the run + // did print — a partial stream is the evidence, not garbage to discard. + const transcript = + transcriptOut === undefined ? undefined : openTranscript(transcriptOut.dir, evalCase.name, label, index + 1); + try { onProgress?.(` ${label} run ${index + 1}/${runs}`); if (config.delivery === "install") { @@ -461,7 +489,9 @@ async function runArm( } } } - const run = await runner.run(buildRunnerOptions(config, evalCase, arm, systemPrompt, sandbox.dir)); + const run = await runner.run( + buildRunnerOptions(config, evalCase, arm, systemPrompt, sandbox.dir, transcript?.write), + ); if (rawOut !== undefined) { writeRawResult(rawOut.dir, evalCase.name, label, index + 1, run.raw); } @@ -478,6 +508,7 @@ async function runArm( }); summaries.push(toArmRunSummary(index + 1, run, grade, sandbox.dir)); } finally { + transcript?.close(); sandbox.cleanup(); } } @@ -500,6 +531,7 @@ function buildRunnerOptions( arm: "baseline" | "proposed", systemPrompt: string, cwd: string, + onStreamEvent?: (line: string) => void, ): RunnerRunOptions { return { systemPrompt, @@ -514,6 +546,7 @@ function buildRunnerOptions( timeoutMs: config.timeoutMs, maxBudgetUsd: config.maxBudgetUsd, maxTurns: evalCase.maxTurns ?? config.maxTurns, + ...(onStreamEvent === undefined ? {} : { onStreamEvent }), }; } @@ -525,8 +558,40 @@ function buildRunnerOptions( */ function writeRawResult(dir: string, caseName: string, label: string, runNumber: number, raw: unknown): void { mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, `${runFileStem(caseName, label, runNumber)}.json`), JSON.stringify(raw, null, 2) + "\n", "utf8"); +} + +interface TranscriptSink { + /** Appends one NDJSON event line; a no-op once the file is closed. */ + write: (line: string) => void; + close: () => void; +} + +/** + * One NDJSON file per run, written event by event. The runner emits lines and + * never learns what a scenario is; naming and the keep-the-partial policy live + * here, next to raw persistence, so both answer to one convention. + */ +function openTranscript(dir: string, caseName: string, label: string, runNumber: number): TranscriptSink { + mkdirSync(dir, { recursive: true }); + const fd = openSync(join(dir, `${runFileStem(caseName, label, runNumber)}.stream.jsonl`), "w"); + let open = true; + return { + write(line: string): void { + if (!open) return; + writeSync(fd, line.endsWith("\n") ? line : `${line}\n`); + }, + close(): void { + if (!open) return; + open = false; + closeSync(fd); + }, + }; +} + +function runFileStem(caseName: string, label: string, runNumber: number): string { const safeCase = caseName.toLowerCase().replace(/[^a-z0-9_-]+/g, "-").replace(/^-+|-+$/g, "") || "scenario"; - writeFileSync(join(dir, `${safeCase}_${label}_${runNumber}.json`), JSON.stringify(raw, null, 2) + "\n", "utf8"); + return `${safeCase}_${label}_${runNumber}`; } function effectiveTools(config: CompareConfig, evalCase: EvalCaseConfig): string { diff --git a/src/runner/claude-p.ts b/src/runner/claude-p.ts index a8f5ae6..baeb515 100644 --- a/src/runner/claude-p.ts +++ b/src/runner/claude-p.ts @@ -4,6 +4,7 @@ import { join } from "node:path"; import type { RunResult, Runner, RunnerRunOptions } from "../types"; interface ClaudeJsonResult { + type?: unknown; result?: unknown; subtype?: unknown; total_cost_usd?: unknown; @@ -29,12 +30,19 @@ export function buildClaudeArgs(options: RunnerRunOptions & { systemPromptFile: ? ["--tools", options.tools] : ["--tools", options.tools, "--allowedTools", options.tools]; + // A transcript sink needs the per-event stream; the terminal event of that + // stream is the same result object --output-format json prints on its own. + // stream-json is only valid with --verbose under -p. + const outputArgs = + options.onStreamEvent === undefined + ? ["--output-format", "json"] + : ["--output-format", "stream-json", "--verbose"]; + const args = [ "-p", options.userPrompt, ...systemPromptArgs, - "--output-format", - "json", + ...outputArgs, "--model", options.model, ...toolArgs, @@ -59,7 +67,7 @@ export function buildClaudeArgs(options: RunnerRunOptions & { systemPromptFile: export class ClaudePrintRunner implements Runner { readonly name = "claude-p"; - readonly capabilities = { sandboxTools: true, skillRegistry: true, images: false }; + readonly capabilities = { sandboxTools: true, skillRegistry: true, images: false, streamEvents: true }; constructor(private readonly claudeBin = "claude") {} @@ -83,8 +91,9 @@ export class ClaudePrintRunner implements Runner { setTimeout(() => proc.kill("SIGKILL"), 2_000); }, options.timeoutMs); - const [stdout, stderr, code] = await Promise.all([ - new Response(proc.stdout).text(), + const sink = options.onStreamEvent; + const [out, stderr, code] = await Promise.all([ + sink === undefined ? readWholeStdout(proc.stdout) : consumeStreamJson(proc.stdout, sink), new Response(proc.stderr).text(), proc.exited, ]); @@ -97,19 +106,30 @@ export class ClaudePrintRunner implements Runner { // A turn-cap stop is a measured outcome, not a crash: claude may exit // non-zero with the full result JSON on stdout. Return it so the engine // can score the run as a failure instead of aborting the comparison. - const capped = tryParseClaudeJson(stdout); + const capped = out.result ?? tryParseClaudeJson(out.text); if (capped?.subtype === "error_max_turns") { return normalizeClaudeResult(capped); } - throw new Error(describeClaudeFailure(code, stdout, stderr, options.maxBudgetUsd)); + throw new Error(describeClaudeFailure(code, out.text, stderr, options.maxBudgetUsd, out.result)); + } + + if (sink !== undefined) { + // A stream killed mid-flight (SIGTERM, a crashed CLI) ends without its + // terminal event, so "the last line is the result" is not safe. The + // lines already written stay on disk as evidence; the run itself has + // no outcome to score. + if (out.result === undefined) { + throw new Error(`claude produced no terminal result event${tailForMessage(out.text)}`); + } + return normalizeClaudeResult(out.result); } let parsed: ClaudeJsonResult; try { - parsed = JSON.parse(stdout) as ClaudeJsonResult; + parsed = JSON.parse(out.text) as ClaudeJsonResult; } catch (error) { const reason = error instanceof Error ? error.message : String(error); - throw new Error(`claude returned invalid JSON: ${reason}\n${stdout.slice(0, 1_500)}`); + throw new Error(`claude returned invalid JSON: ${reason}\n${out.text.slice(0, 1_500)}`); } return normalizeClaudeResult(parsed); @@ -129,8 +149,10 @@ export function describeClaudeFailure( stdout: string, stderr: string, maxBudgetUsd: number, + /** Terminal result event, already decoded — stream-json stdout will not parse whole. */ + streamResult?: ClaudeJsonResult, ): string { - const parsed = tryParseClaudeJson(stdout); + const parsed = streamResult ?? tryParseClaudeJson(stdout); if (parsed && typeof parsed === "object") { if (parsed.subtype === "error_max_budget_usd") { @@ -146,6 +168,66 @@ export function describeClaudeFailure( return `claude exited ${code}: ${detail.slice(0, 1_500)}`; } +/** Bytes of stdout kept for diagnostics when the stream is too big to hold. */ +const STDOUT_TAIL_CHARS = 4_000; + +interface ClaudeStdout { + /** stdout text for error messages: all of it in json mode, a tail in stream mode. */ + text: string; + /** Terminal `result` event; only stream mode decodes one while reading. */ + result?: ClaudeJsonResult; +} + +async function readWholeStdout(stdout: ReadableStream): Promise { + return { text: await new Response(stdout).text() }; +} + +/** + * Reads NDJSON as it arrives, hands each line to the sink, and keeps only the + * terminal `result` event. Buffering the whole verbose stream to parse it + * afterwards is the memory cost transcript capture exists to avoid, and a + * killed run would lose every line it had already printed. + */ +async function consumeStreamJson( + stdout: ReadableStream, + onStreamEvent: (line: string) => void, +): Promise { + const decoder = new TextDecoder(); + let pending = ""; + let tail = ""; + let result: ClaudeJsonResult | undefined; + + const handleLine = (line: string): void => { + if (line.trim().length === 0) return; + onStreamEvent(line); + tail = (tail + line + "\n").slice(-STDOUT_TAIL_CHARS); + const event = tryParseClaudeJson(line); + // Subagents and compaction can emit result-shaped events mid-stream; the + // terminal one is last, so the last wins. + if (event?.type === "result") result = event; + }; + + for await (const chunk of stdout) { + pending += decoder.decode(chunk, { stream: true }); + let newline = pending.indexOf("\n"); + while (newline !== -1) { + handleLine(pending.slice(0, newline)); + pending = pending.slice(newline + 1); + newline = pending.indexOf("\n"); + } + } + pending += decoder.decode(); + // A kill mid-line leaves an unterminated fragment. It is not parseable, but + // for a token-economics investigation a truncated event is still evidence. + if (pending.length > 0) handleLine(pending); + + return { text: tail, result }; +} + +function tailForMessage(text: string): string { + return text.trim().length === 0 ? "" : ` (last output: ${text.slice(-500)})`; +} + function tryParseClaudeJson(stdout: string): ClaudeJsonResult | undefined { try { return JSON.parse(stdout) as ClaudeJsonResult; diff --git a/src/runner/openai-compat.ts b/src/runner/openai-compat.ts index c69059b..514b39f 100644 --- a/src/runner/openai-compat.ts +++ b/src/runner/openai-compat.ts @@ -94,7 +94,7 @@ export function buildChatRequest(options: RunnerRunOptions): ChatRequest { */ export class OpenAiCompatRunner implements Runner { readonly name = "openai"; - readonly capabilities = { sandboxTools: false, skillRegistry: false, images: true }; + readonly capabilities = { sandboxTools: false, skillRegistry: false, images: true, streamEvents: false }; private readonly baseUrl: string; private readonly apiKey: string | undefined; diff --git a/src/types.ts b/src/types.ts index 298e880..facbb19 100644 --- a/src/types.ts +++ b/src/types.ts @@ -37,6 +37,11 @@ export interface RunnerCapabilities { * required for scenarios with `images`. */ images: boolean; + /** + * The runner can emit its provider's per-event stream to `onStreamEvent` — + * required for transcript capture. + */ + streamEvents: boolean; } export interface RunnerRunOptions { @@ -64,6 +69,13 @@ export interface RunnerRunOptions { maxBudgetUsd: number; /** Turn cap for agentic runners; completion runners are single-turn and ignore it. */ maxTurns?: number; + /** + * Per-event sink for transcript capture: the runner emits one NDJSON line + * per provider event and never learns where the lines land. Naming, the + * partial-write-on-failure policy, and closing the file are the engine's. + * Runners without a streaming mode leave it unused. + */ + onStreamEvent?: (line: string) => void; } export interface Runner { diff --git a/test/cache.test.ts b/test/cache.test.ts index 5416e9b..302bd38 100644 --- a/test/cache.test.ts +++ b/test/cache.test.ts @@ -14,7 +14,7 @@ function makeCountingRunner(): CountingRunner { const counts = { baseline: 0, proposed: 0 }; return { name: "mock", - capabilities: { sandboxTools: true, skillRegistry: true, images: false }, + capabilities: { sandboxTools: true, skillRegistry: true, images: false, streamEvents: false }, counts, async run(options: RunnerRunOptions) { if (options.systemPrompt.includes("PROPOSED")) { @@ -191,3 +191,35 @@ test("compare rejects --cache-dir without --cache before doing any work", () => expect(result.exitCode).toBe(2); expect(result.stderr.toString()).toContain("--cache-dir requires --cache"); }); + +test("a cached baseline arm says it has no transcript to write instead of writing an empty one", async () => { + const fixture = makeFixture(); + try { + const cache = { dir: fixture.cacheDir }; + const transcriptOut = { dir: join(fixture.dir, "transcripts") }; + // The counting runner stands in for claude-p here: capture demands a + // streamEvents runner, which the engine checks before any run. + const streamingRunner = (): CountingRunner => { + const runner = makeCountingRunner(); + return { ...runner, capabilities: { ...runner.capabilities, streamEvents: true } }; + }; + const first = streamingRunner(); + await runCompare({ config: fixture.config, runners: { baseline: first, proposed: first }, cache, transcriptOut }); + + const progress: string[] = []; + const second = streamingRunner(); + await runCompare({ + config: fixture.config, + runners: { baseline: second, proposed: second }, + cache, + transcriptOut, + onProgress: (message) => progress.push(message), + }); + + // The arm ran in an earlier invocation; capture cannot reach back for it. + expect(second.counts.baseline).toBe(0); + expect(progress).toContain(" baseline: cache hit — no transcript to write"); + } finally { + rmSync(fixture.dir, { recursive: true, force: true }); + } +}); diff --git a/test/compare.test.ts b/test/compare.test.ts index 1696192..83cdbb5 100644 --- a/test/compare.test.ts +++ b/test/compare.test.ts @@ -18,7 +18,7 @@ test("runCompare verifies target improvement and regression preservation", async const runner: Runner = { name: "mock", - capabilities: { sandboxTools: true, skillRegistry: true, images: false }, + capabilities: { sandboxTools: true, skillRegistry: true, images: false, streamEvents: false }, async run(options: RunnerRunOptions) { const isProposed = options.systemPrompt.includes("PROPOSED"); const isRegression = options.userPrompt.includes("regression"); @@ -93,7 +93,7 @@ test("runCompare routes each arm to its own runner and model, and compare kind n const makeRunner = (name: string, seen: string[], output: string): Runner => ({ name, - capabilities: { sandboxTools: false, skillRegistry: false, images: false }, + capabilities: { sandboxTools: false, skillRegistry: false, images: false, streamEvents: false }, async run(options: RunnerRunOptions) { seen.push(options.model); return { output, costUsd: 0, turns: 1, durationMs: 10, models: [options.model], raw: {} }; @@ -258,7 +258,7 @@ test("a run that exhausted its turn cap fails without grading its partial output // Output would satisfy the grader — the cap must fail the run anyway. const runner: Runner = { name: "mock", - capabilities: { sandboxTools: true, skillRegistry: true, images: false }, + capabilities: { sandboxTools: true, skillRegistry: true, images: false, streamEvents: false }, async run(options: RunnerRunOptions) { expect(options.maxTurns).toBe(7); const capped = options.systemPrompt.includes("BASELINE"); @@ -289,7 +289,7 @@ test("rawOut persists each run's full runner result for both compare arms and fo const usage = { input_tokens: 100, cache_read_input_tokens: 900, output_tokens: 50 }; const runner: Runner = { name: "mock", - capabilities: { sandboxTools: true, skillRegistry: true, images: false }, + capabilities: { sandboxTools: true, skillRegistry: true, images: false, streamEvents: false }, async run(options: RunnerRunOptions) { const arm = options.systemPrompt.includes("BASELINE") ? "baseline" : "proposed"; return { output: "ok", costUsd: 0.1, turns: 1, durationMs: 10, models: ["sonnet"], raw: { arm, usage } }; @@ -317,3 +317,95 @@ test("rawOut persists each run's full runner result for both compare arms and fo rmSync(fixture.dir, { recursive: true, force: true }); } }); + +test("transcriptOut captures each run's event stream, keeping the partial when a run throws", async () => { + const fixture = fixtureDir(); + try { + const streamingRunner = (fail: boolean): Runner => ({ + name: "mock-stream", + capabilities: { sandboxTools: true, skillRegistry: true, images: false, streamEvents: true }, + async run(options: RunnerRunOptions) { + const arm = options.systemPrompt.includes("BASELINE") ? "baseline" : "proposed"; + options.onStreamEvent?.(JSON.stringify({ type: "system", arm })); + options.onStreamEvent?.(JSON.stringify({ type: "assistant", usage: { cache_read_input_tokens: 900 } })); + if (fail) throw new Error("killed mid-run"); + options.onStreamEvent?.(JSON.stringify({ type: "result", result: "ok" })); + return { output: "ok", costUsd: 0.1, turns: 1, durationMs: 10, models: ["sonnet"], raw: {} }; + }, + }); + + const config = cappedConfig(fixture); + const dir = join(fixture.dir, "transcripts"); + const runner = streamingRunner(false); + await runCompare({ config, runners: { baseline: runner, proposed: runner }, transcriptOut: { dir } }); + + // Same naming convention as raw results, one NDJSON line per event. + const baselineLines = readFileSync(join(dir, "case-one_baseline_1.stream.jsonl"), "utf8").trim().split("\n"); + expect(baselineLines).toHaveLength(3); + expect(JSON.parse(baselineLines[0] ?? "{}").arm).toBe("baseline"); + expect(JSON.parse(baselineLines[1] ?? "{}").usage.cache_read_input_tokens).toBe(900); + expect(existsSync(join(dir, "case-one_proposed_1.stream.jsonl"))).toBe(true); + + const measureDir = join(fixture.dir, "transcripts-measure"); + await runMeasure({ config, runner, transcriptOut: { dir: measureDir } }); + expect(existsSync(join(measureDir, "case-one_measure_1.stream.jsonl"))).toBe(true); + + // A run that dies mid-stream keeps what it printed — that partial record is + // what a timeout or a budget abort leaves to investigate. + const failedDir = join(fixture.dir, "transcripts-failed"); + await expect( + runMeasure({ config, runner: streamingRunner(true), transcriptOut: { dir: failedDir } }), + ).rejects.toThrow("killed mid-run"); + expect(readFileSync(join(failedDir, "case-one_measure_1.stream.jsonl"), "utf8").trim().split("\n")).toHaveLength(2); + + // Without the flag nothing is captured and no sink reaches the runner. + let sinkSeen = true; + await runMeasure({ + config, + runner: { + name: "mock", + capabilities: { sandboxTools: true, skillRegistry: true, images: false, streamEvents: true }, + async run(options: RunnerRunOptions) { + sinkSeen = options.onStreamEvent !== undefined; + return { output: "ok", costUsd: 0, turns: 1, durationMs: 1, models: [], raw: {} }; + }, + }, + }); + expect(sinkSeen).toBe(false); + } finally { + rmSync(fixture.dir, { recursive: true, force: true }); + } +}); + +test("transcriptOut is refused before any paid run when the runner cannot stream events", async () => { + const fixture = fixtureDir(); + try { + let ran = false; + const runner: Runner = { + name: "openai", + capabilities: { sandboxTools: true, skillRegistry: true, images: false, streamEvents: false }, + async run() { + ran = true; + return { output: "ok", costUsd: 0.1, turns: 1, durationMs: 10, models: [], raw: {} }; + }, + }; + + const config = cappedConfig(fixture); + // Empty transcript files would misrepresent a runner that simply cannot + // produce one, so this fails loudly instead. + await expect( + runCompare({ + config, + runners: { baseline: runner, proposed: runner }, + transcriptOut: { dir: join(fixture.dir, "transcripts") }, + }), + ).rejects.toThrow('runner "openai" does not'); + expect(ran).toBe(false); + + // The same config without the flag still runs on that runner. + const summary = await runCompare({ config, runners: { baseline: runner, proposed: runner } }); + expect(summary.cases[0]?.baseline.passes).toBe(1); + } finally { + rmSync(fixture.dir, { recursive: true, force: true }); + } +}); diff --git a/test/delivery.test.ts b/test/delivery.test.ts index 4f72d80..9260320 100644 --- a/test/delivery.test.ts +++ b/test/delivery.test.ts @@ -28,7 +28,7 @@ test("install delivery puts arm skills in each sandbox registry and keeps them o const seenPrompts: string[] = []; const runner: Runner = { name: "mock", - capabilities: { sandboxTools: true, skillRegistry: true, images: false }, + capabilities: { sandboxTools: true, skillRegistry: true, images: false, streamEvents: false }, async run(options: RunnerRunOptions) { seenPrompts.push(options.systemPrompt); expect(options.systemPromptMode).toBe("append"); diff --git a/test/json-grader.test.ts b/test/json-grader.test.ts index 4b0222e..0ca5824 100644 --- a/test/json-grader.test.ts +++ b/test/json-grader.test.ts @@ -209,7 +209,7 @@ test("runCompare grades reasoning prose + trailing JSON with a json grader on a const toolsSeen: string[] = []; const runner: Runner = { name: "openai", - capabilities: { sandboxTools: false, skillRegistry: false, images: false }, + capabilities: { sandboxTools: false, skillRegistry: false, images: false, streamEvents: false }, async run(options: RunnerRunOptions) { toolsSeen.push(options.tools); const isProposed = options.systemPrompt.includes("PROPOSED"); diff --git a/test/judge.test.ts b/test/judge.test.ts index ab7f01a..f64278a 100644 --- a/test/judge.test.ts +++ b/test/judge.test.ts @@ -113,7 +113,7 @@ function mockJudgeFactory(calls: RunnerRunOptions[], factoryArgs: unknown[][] = factoryArgs.push([name, options]); const runner: Runner = { name, - capabilities: { sandboxTools: false, skillRegistry: false, images: false }, + capabilities: { sandboxTools: false, skillRegistry: false, images: false, streamEvents: false }, async run(runOptions: RunnerRunOptions) { calls.push(runOptions); const output = runOptions.userPrompt.includes("GARBLE") @@ -177,7 +177,7 @@ async function expectGateRefusal(tree: JudgeTree, pattern: RegExp): Promise ({ name: "mock", - capabilities: { sandboxTools: true, skillRegistry: true, images: false }, + capabilities: { sandboxTools: true, skillRegistry: true, images: false, streamEvents: false }, async run() { return { output, costUsd: 0.1, turns: 1, durationMs: 5, models: ["sonnet"], raw: {} }; }, @@ -299,7 +299,7 @@ test("a judge transport failure fails the graded run with the error in the messa try { setJudgeRunnerFactory(() => ({ name: "claude-p", - capabilities: { sandboxTools: false, skillRegistry: false, images: false }, + capabilities: { sandboxTools: false, skillRegistry: false, images: false, streamEvents: false }, async run(): Promise { throw new Error("endpoint unreachable"); }, diff --git a/test/measure.test.ts b/test/measure.test.ts index 08a56ef..59db4ce 100644 --- a/test/measure.test.ts +++ b/test/measure.test.ts @@ -11,7 +11,7 @@ function mockRunner(outputs: string[]): { runner: Runner; seen: RunnerRunOptions let call = 0; const runner: Runner = { name: "mock", - capabilities: { sandboxTools: true, skillRegistry: true, images: false }, + capabilities: { sandboxTools: true, skillRegistry: true, images: false, streamEvents: false }, async run(options: RunnerRunOptions) { seen.push(options); const output = outputs[call % outputs.length]; diff --git a/test/openai-runner.test.ts b/test/openai-runner.test.ts index 1aa01c0..aead0c0 100644 --- a/test/openai-runner.test.ts +++ b/test/openai-runner.test.ts @@ -101,11 +101,11 @@ test("OpenAiCompatRunner surfaces HTTP errors and malformed completions", async test("createRunner builds runners with the right capabilities", () => { const claude = createRunner("claude-p"); expect(claude.name).toBe("claude-p"); - expect(claude.capabilities).toEqual({ sandboxTools: true, skillRegistry: true, images: false }); + expect(claude.capabilities).toEqual({ sandboxTools: true, skillRegistry: true, images: false, streamEvents: true }); const openai = createRunner("openai", { baseUrl: "http://localhost:8080/v1" }); expect(openai.name).toBe("openai"); - expect(openai.capabilities).toEqual({ sandboxTools: false, skillRegistry: false, images: true }); + expect(openai.capabilities).toEqual({ sandboxTools: false, skillRegistry: false, images: true, streamEvents: false }); }); test("buildChatRequest attaches images as data-URI content parts before the text", () => { diff --git a/test/receipt.test.ts b/test/receipt.test.ts index 59d4430..3a1f836 100644 --- a/test/receipt.test.ts +++ b/test/receipt.test.ts @@ -15,7 +15,7 @@ function sha256(content: string): string { function mockRunner(decide: (options: RunnerRunOptions) => string): Runner { return { name: "mock", - capabilities: { sandboxTools: true, skillRegistry: true, images: false }, + capabilities: { sandboxTools: true, skillRegistry: true, images: false, streamEvents: false }, async run(options: RunnerRunOptions) { return { output: decide(options), costUsd: 0.1, turns: 1, durationMs: 5, models: ["m"], raw: {} }; }, diff --git a/test/render.test.ts b/test/render.test.ts index 5be1c6a..47a86c8 100644 --- a/test/render.test.ts +++ b/test/render.test.ts @@ -141,7 +141,7 @@ test("runCompare renders per-case vars into both arms and fails unbound before a const seenPrompts: Array<{ system: string; user: string }> = []; const runner: Runner = { name: "mock", - capabilities: { sandboxTools: true, skillRegistry: true, images: false }, + capabilities: { sandboxTools: true, skillRegistry: true, images: false, streamEvents: false }, async run(options: RunnerRunOptions) { seenPrompts.push({ system: options.systemPrompt, user: options.userPrompt }); return { output: "ok", costUsd: 0, turns: 1, durationMs: 1, models: ["m"], raw: {} }; diff --git a/test/runner-support.test.ts b/test/runner-support.test.ts index 49381d4..75c931e 100644 --- a/test/runner-support.test.ts +++ b/test/runner-support.test.ts @@ -5,7 +5,7 @@ import type { Runner } from "../src/types"; const textOnlyRunner: Runner = { name: "openai", - capabilities: { sandboxTools: false, skillRegistry: false, images: true }, + capabilities: { sandboxTools: false, skillRegistry: false, images: true, streamEvents: false }, async run() { throw new Error("unused"); }, @@ -56,7 +56,7 @@ test("install delivery is rejected on a runner without a skill registry", () => test("mixed arms validate per arm: the openai arm rejects command graders, text graders pass both", () => { const toolRunner: Runner = { name: "claude-p", - capabilities: { sandboxTools: true, skillRegistry: true, images: false }, + capabilities: { sandboxTools: true, skillRegistry: true, images: false, streamEvents: false }, async run() { throw new Error("unused"); }, diff --git a/test/runner.test.ts b/test/runner.test.ts index 261085c..f8fb226 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -236,3 +236,133 @@ test("non-zero exits without a turn-cap subtype still throw", async () => { rmSync(dir, { recursive: true, force: true }); } }); + +test("buildClaudeArgs switches to stream-json only when a transcript sink is attached", () => { + const base = { + systemPrompt: "system", + systemPromptFile: "/tmp/system.md", + userPrompt: "do work", + model: "sonnet", + cwd: "/tmp/sandbox", + addDirs: [], + tools: "", + timeoutMs: 1_000, + maxBudgetUsd: 0.25, + }; + + expect(buildClaudeArgs(base)).toContain("json"); + expect(buildClaudeArgs(base)).not.toContain("stream-json"); + expect(buildClaudeArgs(base)).not.toContain("--verbose"); + + const streaming = buildClaudeArgs({ ...base, onStreamEvent: () => {} }); + const i = streaming.indexOf("--output-format"); + expect(streaming[i + 1]).toBe("stream-json"); + // stream-json is rejected under -p without --verbose. + expect(streaming).toContain("--verbose"); +}); + +// Captured shape of a stream-json run: NDJSON events, terminal `result` last. +const STREAM_EVENTS = [ + { type: "system", subtype: "init", session_id: "s-1" }, + { type: "assistant", message: { usage: { input_tokens: 12, cache_read_input_tokens: 9_000 } } }, + { type: "assistant", message: { usage: { input_tokens: 40, cache_read_input_tokens: 9_400 } } }, + { + type: "result", + subtype: "success", + result: "done", + num_turns: 2, + total_cost_usd: 0.03, + duration_ms: 4_200, + modelUsage: { "claude-sonnet-5": { costUSD: 0.03 } }, + }, +]; + +function fakeClaude(dir: string, body: string): string { + const bin = join(dir, "fake-claude"); + writeFileSync(bin, `#!/bin/sh\n${body}\n`, { mode: 0o755 }); + return bin; +} + +const streamRunOptions = (cwd: string, onStreamEvent: (line: string) => void) => ({ + systemPrompt: "system", + userPrompt: "do work", + model: "sonnet", + cwd, + addDirs: [] as string[], + tools: "", + timeoutMs: 10_000, + maxBudgetUsd: 1, + onStreamEvent, +}); + +test("stream mode feeds every event to the sink and scores the terminal result event", async () => { + const dir = mkdtempSync(join(tmpdir(), "promptdiff-runner-test-")); + try { + const echoes = STREAM_EVENTS.map((event) => `echo '${JSON.stringify(event)}'`).join("\n"); + const runner = new ClaudePrintRunner(fakeClaude(dir, echoes)); + + const lines: string[] = []; + const result = await runner.run(streamRunOptions(dir, (line) => lines.push(line))); + + // Per-turn usage is the whole point: the aggregate result cannot show how + // context grew between turn 1 and turn 2. + expect(lines).toHaveLength(4); + expect(JSON.parse(lines[1] ?? "{}").message.usage.cache_read_input_tokens).toBe(9_000); + expect(JSON.parse(lines[2] ?? "{}").message.usage.cache_read_input_tokens).toBe(9_400); + + expect(result.output).toBe("done"); + expect(result.turns).toBe(2); + expect(result.costUsd).toBeCloseTo(0.03); + expect(result.models).toEqual(["claude-sonnet-5"]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("a truncated stream keeps its lines but has no outcome to score", async () => { + const dir = mkdtempSync(join(tmpdir(), "promptdiff-runner-test-")); + try { + // Two whole events then a killed mid-line write: no terminal result event. + const body = [ + `echo '${JSON.stringify(STREAM_EVENTS[0])}'`, + `echo '${JSON.stringify(STREAM_EVENTS[1])}'`, + `printf '%s' '{"type":"assistant","mess'`, + ].join("\n"); + const runner = new ClaudePrintRunner(fakeClaude(dir, body)); + + const lines: string[] = []; + await expect(runner.run(streamRunOptions(dir, (line) => lines.push(line)))).rejects.toThrow( + "no terminal result event", + ); + + // "Last line is the result" is not safe, but the partial stream is still + // the evidence a token-economics run was after. + expect(lines).toHaveLength(3); + expect(lines[2]).toBe('{"type":"assistant","mess'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("stream mode decodes turn caps and budget aborts out of NDJSON", async () => { + const dir = mkdtempSync(join(tmpdir(), "promptdiff-runner-test-")); + try { + const capped = fakeClaude( + dir, + [`echo '${JSON.stringify(STREAM_EVENTS[0])}'`, `echo '${MAX_TURNS_STDOUT}'`, "exit 1"].join("\n"), + ); + const cappedResult = await new ClaudePrintRunner(capped).run(streamRunOptions(dir, () => {})); + expect(cappedResult.exhaustedTurns).toBe(true); + expect(cappedResult.turns).toBe(25); + + const broke = fakeClaude( + dir, + [`echo '${JSON.stringify(STREAM_EVENTS[0])}'`, `echo '${BUDGET_ABORT_STDOUT}'`, "exit 1"].join("\n"), + ); + // Whole-stdout JSON.parse fails on NDJSON, which would have made a budget + // abort read as a bare exit code again. + await expect(new ClaudePrintRunner(broke).run(streamRunOptions(dir, () => {}))).rejects.toThrow("max budget"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); From 95f1d6145abe9121856af326f23610cfd28281dc Mon Sep 17 00:00:00 2001 From: Josh Owens Date: Fri, 18 Sep 2026 21:06:21 +0000 Subject: [PATCH 2/8] fix: address PR #43 review feedback (pass 1) - Fix: an unwritable --transcript-out dir no longer orphans a sandbox. runCompare/runMeasure prepare the dir once, before scenario 1 prepares anything, and runArm opens the per-run file inside the try so the existing finally closes the descriptor and cleans the sandbox on a failed open too. - Test: a transcript dir pointed at a regular file rejects before any run, with no sandbox left behind. - Test: pin the exit-code gating that keeps a mid-stream result-shaped event from being scored as a killed run's outcome, for both a non-zero exit and a signal death. - Comment: consumeStreamJson no longer asserts mid-stream result events as fact; it states why last-wins is safe. Addresses review comments from queso and github-actions[bot]. Co-Authored-By: Claude Opus 5 --- src/engine/compare.ts | 17 ++++++++++--- src/runner/claude-p.ts | 7 ++++-- test/compare.test.ts | 34 +++++++++++++++++++++++++ test/runner.test.ts | 57 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 6 deletions(-) diff --git a/src/engine/compare.ts b/src/engine/compare.ts index 8e2696d..d18b0c1 100644 --- a/src/engine/compare.ts +++ b/src/engine/compare.ts @@ -84,6 +84,10 @@ export async function runCompare(options: CompareRunOptions): Promise { + const fixture = fixtureDir(); + try { + let ran = false; + const runner: Runner = { + name: "mock-stream", + capabilities: { sandboxTools: true, skillRegistry: true, images: false, streamEvents: true }, + async run() { + ran = true; + return { output: "ok", costUsd: 0.1, turns: 1, durationMs: 10, models: [], raw: {} }; + }, + }; + + const config = cappedConfig(fixture); + // A regular file where the transcript dir belongs makes mkdirSync throw — + // that must surface before scenario 1 ever prepares a sandbox. + const blockedPath = join(fixture.dir, "blocked-transcripts"); + writeFileSync(blockedPath, "not a directory", "utf8"); + + await expect( + runCompare({ + config, + runners: { baseline: runner, proposed: runner }, + transcriptOut: { dir: blockedPath }, + }), + ).rejects.toThrow(blockedPath); + expect(ran).toBe(false); + // Nothing was prepared, so nothing was orphaned. + expect(existsSync(config.sandboxRoot)).toBe(false); + } finally { + rmSync(fixture.dir, { recursive: true, force: true }); + } +}); diff --git a/test/runner.test.ts b/test/runner.test.ts index f8fb226..fee4671 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -344,6 +344,63 @@ test("a truncated stream keeps its lines but has no outcome to score", async () } }); +// A result-shaped event that is NOT the real terminal one, the kind a +// subagent or compaction can emit mid-stream. Its "result" text is +// distinctive so a test would notice if it were ever scored as the outcome. +const MID_STREAM_RESULT_EVENT = { + type: "result", + subtype: "success", + result: "mid-stream, not the real outcome", + num_turns: 1, + total_cost_usd: 0.01, +}; + +test("a mid-stream result event is not scored as the outcome when the run then exits non-zero", async () => { + const dir = mkdtempSync(join(tmpdir(), "promptdiff-runner-test-")); + try { + // A result event fires mid-stream, then the process dies (exit 1) without + // ever emitting the real terminal event. A result event is only trusted + // as terminal when the process also exited cleanly, so this must reject + // rather than return MID_STREAM_RESULT_EVENT as a scored success. + const body = [ + `echo '${JSON.stringify(STREAM_EVENTS[0])}'`, + `echo '${JSON.stringify(MID_STREAM_RESULT_EVENT)}'`, + "exit 1", + ].join("\n"); + const runner = new ClaudePrintRunner(fakeClaude(dir, body)); + + const lines: string[] = []; + await expect(runner.run(streamRunOptions(dir, (line) => lines.push(line)))).rejects.toThrow(/claude exited 1/); + + // The partial stream is still evidence even though the run is scored as a failure. + expect(lines).toHaveLength(2); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("a mid-stream result event is not scored as the outcome when the run is then killed by a signal", async () => { + const dir = mkdtempSync(join(tmpdir(), "promptdiff-runner-test-")); + try { + // Same scenario, but the process dies from a signal (as the timeout path's + // SIGTERM/SIGKILL would deliver) instead of a self-chosen exit code. A + // signaled process does not exit 0, so this must reject too. + const body = [ + `echo '${JSON.stringify(STREAM_EVENTS[0])}'`, + `echo '${JSON.stringify(MID_STREAM_RESULT_EVENT)}'`, + "kill -TERM $$", + ].join("\n"); + const runner = new ClaudePrintRunner(fakeClaude(dir, body)); + + const lines: string[] = []; + await expect(runner.run(streamRunOptions(dir, (line) => lines.push(line)))).rejects.toThrow(/claude exited/); + + expect(lines).toHaveLength(2); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + test("stream mode decodes turn caps and budget aborts out of NDJSON", async () => { const dir = mkdtempSync(join(tmpdir(), "promptdiff-runner-test-")); try { From fd92d2d703f61e64746ca392da6eb9e6d51b8f34 Mon Sep 17 00:00:00 2001 From: Josh Owens Date: Fri, 18 Sep 2026 21:17:35 +0000 Subject: [PATCH 3/8] fix: write transcript lines by byte offset (PR #43 pass 2) - Fix: the transcript sink loops until every byte of a line is written. writeSync may legally return a short count, which would truncate an NDJSON event and corrupt the record these files exist to preserve. - Test: multi-byte event content and a 250k-character line round-trip through a real compare run. The loop works on a Buffer and byte offsets. Slicing the string by the returned count, as the review suggested, misaligns on any multi-byte character and would corrupt more than it fixed. Addresses a review comment from github-actions[bot]. Co-Authored-By: Claude Opus 5 --- src/engine/compare.ts | 11 +++++++- test/compare.test.ts | 60 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/engine/compare.ts b/src/engine/compare.ts index d18b0c1..e98094c 100644 --- a/src/engine/compare.ts +++ b/src/engine/compare.ts @@ -588,7 +588,16 @@ function openTranscript(dir: string, caseName: string, label: string, runNumber: return { write(line: string): void { if (!open) return; - writeSync(fd, line.endsWith("\n") ? line : `${line}\n`); + const payload = Buffer.from(line.endsWith("\n") ? line : `${line}\n`, "utf8"); + // writeSync may legally write fewer bytes than asked (a signal, a full + // disk) — a short write would truncate an NDJSON line and corrupt the + // evidence these files exist to preserve. Loop on byte offsets, not + // string indices: slicing the string by the returned count would + // misalign on any multi-byte UTF-8 character. + let written = 0; + while (written < payload.length) { + written += writeSync(fd, payload, written, payload.length - written); + } }, close(): void { if (!open) return; diff --git a/test/compare.test.ts b/test/compare.test.ts index ecf90a6..e3c53dd 100644 --- a/test/compare.test.ts +++ b/test/compare.test.ts @@ -443,3 +443,63 @@ test("an unwritable transcriptOut dir is refused before any paid run and leaves rmSync(fixture.dir, { recursive: true, force: true }); } }); + +test("transcriptOut round-trips multi-byte event content exactly", async () => { + const fixture = fixtureDir(); + try { + // CJK, an emoji, and accented Latin each encode to a different number of + // UTF-8 bytes per character. This pins the encoding end to end: the sink + // writes bytes, and what lands on disk has to parse back identically. + const multiByte = "こんにちは 🎉 café"; + const runner: Runner = { + name: "mock-stream", + capabilities: { sandboxTools: true, skillRegistry: true, images: false, streamEvents: true }, + async run(options: RunnerRunOptions) { + options.onStreamEvent?.(JSON.stringify({ type: "assistant", text: multiByte })); + return { output: "ok", costUsd: 0.1, turns: 1, durationMs: 10, models: ["sonnet"], raw: {} }; + }, + }; + + const config = cappedConfig(fixture); + const dir = join(fixture.dir, "transcripts-multibyte"); + await runCompare({ config, runners: { baseline: runner, proposed: runner }, transcriptOut: { dir } }); + + const lines = readFileSync(join(dir, "case-one_baseline_1.stream.jsonl"), "utf8").trim().split("\n"); + expect(lines).toHaveLength(1); + expect(JSON.parse(lines[0] ?? "{}").text).toBe(multiByte); + } finally { + rmSync(fixture.dir, { recursive: true, force: true }); + } +}); + +test("transcriptOut writes a large event line whole", async () => { + const fixture = fixtureDir(); + try { + // A quarter-megabyte line, well past any real event, so the write path is + // exercised at a size where a short write is at least possible. On a + // regular file one writeSync still completes it here, so this is + // round-trip coverage, not proof that the loop handles a short count. + const big = "x".repeat(250_000); + const runner: Runner = { + name: "mock-stream", + capabilities: { sandboxTools: true, skillRegistry: true, images: false, streamEvents: true }, + async run(options: RunnerRunOptions) { + options.onStreamEvent?.(JSON.stringify({ type: "assistant", text: big })); + return { output: "ok", costUsd: 0.1, turns: 1, durationMs: 10, models: ["sonnet"], raw: {} }; + }, + }; + + const config = cappedConfig(fixture); + const dir = join(fixture.dir, "transcripts-large"); + await runCompare({ config, runners: { baseline: runner, proposed: runner }, transcriptOut: { dir } }); + + const raw = readFileSync(join(dir, "case-one_baseline_1.stream.jsonl"), "utf8"); + const lines = raw.trim().split("\n"); + expect(lines).toHaveLength(1); + const line = lines[0] ?? ""; + expect(Buffer.byteLength(line, "utf8")).toBe(line.length); // ASCII payload: bytes == chars, sanity-checks the assertion below + expect(JSON.parse(line).text).toBe(big); + } finally { + rmSync(fixture.dir, { recursive: true, force: true }); + } +}); From 09811bfe5f0f893a23c342865a4e98c07cfc0029 Mon Sep 17 00:00:00 2001 From: Josh Owens Date: Fri, 18 Sep 2026 21:28:26 +0000 Subject: [PATCH 4/8] perf: scan only new bytes for NDJSON line breaks (PR #43 pass 3) - Fix: consumeStreamJson searches each chunk from the first unscanned character instead of rescanning the whole buffer. A single multi-megabyte event line (a large tool result) previously rescanned everything already accumulated on every chunk. - Test: a 2,000,000-character event line, which arrives across 11 stdout chunks here, is read whole, with a trailing line after it to catch a merged newline boundary. - Test: the last result event wins when the run exits cleanly, covering the success side of the gating the previous pass pinned on the failure side. Addresses review comments from github-actions[bot]. Co-Authored-By: Claude Opus 5 --- src/runner/claude-p.ts | 7 +++- test/runner.test.ts | 72 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/src/runner/claude-p.ts b/src/runner/claude-p.ts index 5103525..8f21142 100644 --- a/src/runner/claude-p.ts +++ b/src/runner/claude-p.ts @@ -211,8 +211,13 @@ async function consumeStreamJson( }; for await (const chunk of stdout) { + // `pending` up to this length was already scanned for "\n" with none + // found (that's why the previous iteration's while loop exited) — only + // the newly appended bytes need to be searched. Once a line is sliced + // off below, the remaining buffer is unscanned from its own start 0. + const scanned = pending.length; pending += decoder.decode(chunk, { stream: true }); - let newline = pending.indexOf("\n"); + let newline = pending.indexOf("\n", scanned); while (newline !== -1) { handleLine(pending.slice(0, newline)); pending = pending.slice(newline + 1); diff --git a/test/runner.test.ts b/test/runner.test.ts index fee4671..0a812b9 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -401,6 +401,32 @@ test("a mid-stream result event is not scored as the outcome when the run is the } }); +test("the last result event wins when the run exits cleanly", async () => { + const dir = mkdtempSync(join(tmpdir(), "promptdiff-runner-test-")); + try { + // A result event fires mid-stream, then the real terminal result event + // follows, and the process exits 0. The mid-stream event must not win + // just because it arrived first — only the last one is the outcome. + const body = [ + `echo '${JSON.stringify(STREAM_EVENTS[0])}'`, + `echo '${JSON.stringify(MID_STREAM_RESULT_EVENT)}'`, + `echo '${JSON.stringify(STREAM_EVENTS[3])}'`, + ].join("\n"); + const runner = new ClaudePrintRunner(fakeClaude(dir, body)); + + const lines: string[] = []; + const result = await runner.run(streamRunOptions(dir, (line) => lines.push(line))); + + expect(lines).toHaveLength(3); + expect(result.output).toBe("done"); + expect(result.turns).toBe(2); + expect(result.costUsd).toBeCloseTo(0.03); + expect(result.output).not.toBe(MID_STREAM_RESULT_EVENT.result); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + test("stream mode decodes turn caps and budget aborts out of NDJSON", async () => { const dir = mkdtempSync(join(tmpdir(), "promptdiff-runner-test-")); try { @@ -423,3 +449,49 @@ test("stream mode decodes turn caps and budget aborts out of NDJSON", async () = rmSync(dir, { recursive: true, force: true }); } }); + +test("a single event line spanning many stdout chunks is still read whole", async () => { + const dir = mkdtempSync(join(tmpdir(), "promptdiff-runner-test-")); + try { + // A pipe hands back far less than 2MB per read, so a field this large + // splits the NDJSON line across many stdout chunks (11 here) — the case + // the per-chunk scan-offset arithmetic has to get right. Built with + // head/tr rather than an embedded literal: a 2,000,000-character + // argument to echo risks blowing past ARG_MAX. + const bigFieldChars = 2_000_000; + const body = [ + `echo '${JSON.stringify(STREAM_EVENTS[0])}'`, + `printf '%s' '{"type":"assistant","big":"'`, + `head -c ${bigFieldChars} /dev/zero | tr '\\0' 'a'`, + `printf '"}\\n'`, + `echo '${JSON.stringify(STREAM_EVENTS[3])}'`, + // A short line after the giant one: if a late chunk happens to land + // two newlines at once (the giant line's close plus this line's own), + // a stale scan offset would merge them into one garbled line instead + // of missing them outright — this line is what would expose that. + `echo '${JSON.stringify(STREAM_EVENTS[1])}'`, + ].join("\n"); + const runner = new ClaudePrintRunner(fakeClaude(dir, body)); + + const lines: string[] = []; + const result = await runner.run(streamRunOptions(dir, (line) => lines.push(line))); + + // Exactly the four lines the script printed: init, the giant line, the + // terminal result, and the trailing line — a dropped, duplicated, or + // merged newline boundary would show up here as the wrong count. + expect(lines).toHaveLength(4); + + const big = JSON.parse(lines[1] ?? "{}") as { big: string }; + expect(big.big).toHaveLength(bigFieldChars); + expect(big.big).toBe("a".repeat(bigFieldChars)); + + const trailing = JSON.parse(lines[3] ?? "{}") as { message: { usage: { cache_read_input_tokens: number } } }; + expect(trailing.message.usage.cache_read_input_tokens).toBe(9_000); + + expect(result.output).toBe("done"); + expect(result.turns).toBe(2); + expect(result.costUsd).toBeCloseTo(0.03); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); From 0f4f6cd9672e3cb9eba28f01a82c14c2cb13516f Mon Sep 17 00:00:00 2001 From: Josh Owens Date: Fri, 18 Sep 2026 21:38:48 +0000 Subject: [PATCH 5/8] fix: score a turn cap only from a terminal event; bound the write loop (PR #43 pass 4) - Fix: the non-zero-exit path takes the error_max_turns early return only when the result event was the last line the stream produced. A result-shaped event with more stream after it was previously scored as exhaustedTurns, which turns a crash into a data point in a pass rate instead of aborting. describeClaudeFailure still uses any decoded event, since a message is not a score. - Fix: the transcript write loop throws on a zero-byte write instead of spinning. An unbounded hang inside the runner's event callback is worse than a throw the existing finally can clean up after. - Test: a mid-stream turn-cap event followed by more stream and a crash rejects rather than scoring. Verified failing before the fix. No test for the zero-byte write: forcing it needs OS-level fault injection, and mock.module on node:fs corrupts or hangs the suite in Bun 1.3.11. Addresses review comments from github-actions[bot]. Co-Authored-By: Claude Opus 5 --- src/engine/compare.ts | 8 +++++++- src/runner/claude-p.ts | 37 +++++++++++++++++++++++++++++-------- test/runner.test.ts | 25 +++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 9 deletions(-) diff --git a/src/engine/compare.ts b/src/engine/compare.ts index e98094c..c3e26da 100644 --- a/src/engine/compare.ts +++ b/src/engine/compare.ts @@ -596,7 +596,13 @@ function openTranscript(dir: string, caseName: string, label: string, runNumber: // misalign on any multi-byte UTF-8 character. let written = 0; while (written < payload.length) { - written += writeSync(fd, payload, written, payload.length - written); + const n = writeSync(fd, payload, written, payload.length - written); + // A zero-byte write is legal and makes no progress — looping on it + // would hang the run inside the runner's event callback. Throwing + // bounds it: the caller's finally still closes the transcript and + // cleans the sandbox, and the lines already written stay on disk. + if (n <= 0) throw new Error(`transcript write stalled at ${written}/${payload.length} bytes`); + written += n; } }, close(): void { diff --git a/src/runner/claude-p.ts b/src/runner/claude-p.ts index 8f21142..c8b54a9 100644 --- a/src/runner/claude-p.ts +++ b/src/runner/claude-p.ts @@ -106,10 +106,16 @@ export class ClaudePrintRunner implements Runner { // A turn-cap stop is a measured outcome, not a crash: claude may exit // non-zero with the full result JSON on stdout. Return it so the engine // can score the run as a failure instead of aborting the comparison. - const capped = out.result ?? tryParseClaudeJson(out.text); + // Only a terminal event counts — a result-shaped event with more stream + // after it says nothing about how the run ended, and scoring it would + // turn a crash into a data point. + const terminal = out.resultIsTerminal ? out.result : undefined; + const capped = terminal ?? tryParseClaudeJson(out.text); if (capped?.subtype === "error_max_turns") { return normalizeClaudeResult(capped); } + // A non-terminal event still beats a bare exit code in an error message, + // and a message is not a score — so this one keeps using out.result. throw new Error(describeClaudeFailure(code, out.text, stderr, options.maxBudgetUsd, out.result)); } @@ -176,10 +182,18 @@ interface ClaudeStdout { text: string; /** Terminal `result` event; only stream mode decodes one while reading. */ result?: ClaudeJsonResult; + /** + * Whether `result` was the last line the stream produced, rather than a + * result-shaped event with more stream (or a truncated fragment) after it. + * Only a terminal event may be scored as a run's outcome. + */ + resultIsTerminal: boolean; } async function readWholeStdout(stdout: ReadableStream): Promise { - return { text: await new Response(stdout).text() }; + // json mode decodes no event stream, so it has no terminal result event; + // its parse runs on the whole buffered text instead. + return { text: await new Response(stdout).text(), resultIsTerminal: false }; } /** @@ -196,6 +210,7 @@ async function consumeStreamJson( let pending = ""; let tail = ""; let result: ClaudeJsonResult | undefined; + let resultIsTerminal = false; const handleLine = (line: string): void => { if (line.trim().length === 0) return; @@ -203,11 +218,17 @@ async function consumeStreamJson( tail = (tail + line + "\n").slice(-STDOUT_TAIL_CHARS); const event = tryParseClaudeJson(line); // The last result event wins when more than one appears. A subagent or - // compaction could in principle emit a result-shaped event mid-stream, - // and it still cannot be scored as the run's outcome — the success path - // trusts out.result only after a clean exit, and a killed or crashed run - // exits non-zero into the failure path. - if (event?.type === "result") result = event; + // compaction could in principle emit one mid-stream, so callers get told + // whether the stream ended on it: the success path trusts out.result only + // after a clean exit, and the failure path only when it was terminal. + // Any later line — another event, or a truncated trailing fragment — + // demotes it. + if (event?.type === "result") { + result = event; + resultIsTerminal = true; + } else { + resultIsTerminal = false; + } }; for await (const chunk of stdout) { @@ -229,7 +250,7 @@ async function consumeStreamJson( // for a token-economics investigation a truncated event is still evidence. if (pending.length > 0) handleLine(pending); - return { text: tail, result }; + return { text: tail, result, resultIsTerminal }; } function tailForMessage(text: string): string { diff --git a/test/runner.test.ts b/test/runner.test.ts index 0a812b9..9c09e2c 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -450,6 +450,31 @@ test("stream mode decodes turn caps and budget aborts out of NDJSON", async () = } }); +test("a mid-stream turn-cap event followed by more stream and a crash is not scored as exhaustedTurns", async () => { + const dir = mkdtempSync(join(tmpdir(), "promptdiff-runner-test-")); + try { + // MAX_TURNS_STDOUT fires mid-stream (not as the last line), another event + // follows it, and then the process exits non-zero without ever emitting a + // real terminal result. The run must be reported as a failure — scoring it + // as exhaustedTurns would corrupt the measurement by turning a crash into + // a counted, "completed" data point. + const body = [ + `echo '${JSON.stringify(STREAM_EVENTS[0])}'`, + `echo '${MAX_TURNS_STDOUT}'`, + `echo '${JSON.stringify(STREAM_EVENTS[1])}'`, + "exit 1", + ].join("\n"); + const runner = new ClaudePrintRunner(fakeClaude(dir, body)); + + const lines: string[] = []; + await expect(runner.run(streamRunOptions(dir, (line) => lines.push(line)))).rejects.toThrow(/claude exited 1/); + + expect(lines).toHaveLength(3); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + test("a single event line spanning many stdout chunks is still read whole", async () => { const dir = mkdtempSync(join(tmpdir(), "promptdiff-runner-test-")); try { From 3eb4e84ed387450aadc4d9c08b66563ab476fa72 Mon Sep 17 00:00:00 2001 From: Josh Owens Date: Fri, 18 Sep 2026 21:49:07 +0000 Subject: [PATCH 6/8] fix: refuse to score a non-terminal result event on a clean exit (PR #43 pass 5) The failure path already required a terminal event before scoring a turn cap; the success path scored any decoded result event. A mid-stream result event followed by more stream and then exit 0 was returned as the run's outcome, which is the corruption the terminal check exists to prevent. The two causes throw separate messages. A missing result event and a result event that was not last need different fixes, and this guard is only worth failing loudly for if the message says which one happened. Trade this makes: any event claude emits after its terminal result now fails the run instead of being ignored. That is deliberate. Silently scoring the wrong event corrupts a measurement invisibly, while this fails immediately with the stream tail in the message. - Test: a mid-stream result event, more stream, then exit 0 rejects. Verified it resolved with the mid-stream output before the change. - The large-line chunk test now emits its boundary-check line before the terminal event rather than after. That line exists to catch a merged newline after the giant line, which works either side of the result event. Addresses a review comment from github-actions[bot]. Co-Authored-By: Claude Opus 5 --- src/runner/claude-p.ts | 15 ++++++++++++--- test/runner.test.ts | 39 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/runner/claude-p.ts b/src/runner/claude-p.ts index c8b54a9..628c651 100644 --- a/src/runner/claude-p.ts +++ b/src/runner/claude-p.ts @@ -121,12 +121,21 @@ export class ClaudePrintRunner implements Runner { if (sink !== undefined) { // A stream killed mid-flight (SIGTERM, a crashed CLI) ends without its - // terminal event, so "the last line is the result" is not safe. The - // lines already written stay on disk as evidence; the run itself has - // no outcome to score. + // terminal event, so "the last line is the result" is not safe. Neither + // is a result event that fired mid-stream and was then followed by more + // output, even on a clean exit. Either way there is no outcome to score, + // and the lines already written stay on disk as evidence. The two causes + // get separate messages: a run that hard-fails here is only worth failing + // loudly if whoever reads the message can tell which one happened. if (out.result === undefined) { throw new Error(`claude produced no terminal result event${tailForMessage(out.text)}`); } + if (!out.resultIsTerminal) { + throw new Error( + `claude emitted a result event before the end of the stream — refusing to score a ` + + `non-terminal event${tailForMessage(out.text)}`, + ); + } return normalizeClaudeResult(out.result); } diff --git a/test/runner.test.ts b/test/runner.test.ts index 9c09e2c..00c9a08 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -427,6 +427,36 @@ test("the last result event wins when the run exits cleanly", async () => { } }); +test("a mid-stream result event is not scored as the outcome when the run then exits cleanly with no real terminal event", async () => { + const dir = mkdtempSync(join(tmpdir(), "promptdiff-runner-test-")); + try { + // A result event fires mid-stream, another event follows it, and then the + // process exits 0 without ever emitting a real terminal result event. + // Claude exiting 0 normally means the run finished cleanly on its result + // event, so this combination is self-contradictory — it must reject + // rather than score MID_STREAM_RESULT_EVENT as the outcome. + const body = [ + `echo '${JSON.stringify(STREAM_EVENTS[0])}'`, + `echo '${JSON.stringify(MID_STREAM_RESULT_EVENT)}'`, + `echo '${JSON.stringify(STREAM_EVENTS[1])}'`, + ].join("\n"); + const runner = new ClaudePrintRunner(fakeClaude(dir, body)); + + const lines: string[] = []; + // A distinct message from the truncated-stream case: a result event was + // produced here, it just was not last, and someone reading the failure + // should not go looking for a missing event. + await expect(runner.run(streamRunOptions(dir, (line) => lines.push(line)))).rejects.toThrow( + /result event before the end of the stream/, + ); + + // The printed lines are still evidence even though the run has no outcome to score. + expect(lines).toHaveLength(3); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + test("stream mode decodes turn caps and budget aborts out of NDJSON", async () => { const dir = mkdtempSync(join(tmpdir(), "promptdiff-runner-test-")); try { @@ -489,12 +519,15 @@ test("a single event line spanning many stdout chunks is still read whole", asyn `printf '%s' '{"type":"assistant","big":"'`, `head -c ${bigFieldChars} /dev/zero | tr '\\0' 'a'`, `printf '"}\\n'`, - `echo '${JSON.stringify(STREAM_EVENTS[3])}'`, // A short line after the giant one: if a late chunk happens to land // two newlines at once (the giant line's close plus this line's own), // a stale scan offset would merge them into one garbled line instead // of missing them outright — this line is what would expose that. + // It comes before the terminal result event (not after): a result + // event only counts as the run's outcome when it is the last line the + // stream produced, so this line has to sit ahead of it, not behind it. `echo '${JSON.stringify(STREAM_EVENTS[1])}'`, + `echo '${JSON.stringify(STREAM_EVENTS[3])}'`, ].join("\n"); const runner = new ClaudePrintRunner(fakeClaude(dir, body)); @@ -502,7 +535,7 @@ test("a single event line spanning many stdout chunks is still read whole", asyn const result = await runner.run(streamRunOptions(dir, (line) => lines.push(line))); // Exactly the four lines the script printed: init, the giant line, the - // terminal result, and the trailing line — a dropped, duplicated, or + // short line, and the terminal result — a dropped, duplicated, or // merged newline boundary would show up here as the wrong count. expect(lines).toHaveLength(4); @@ -510,7 +543,7 @@ test("a single event line spanning many stdout chunks is still read whole", asyn expect(big.big).toHaveLength(bigFieldChars); expect(big.big).toBe("a".repeat(bigFieldChars)); - const trailing = JSON.parse(lines[3] ?? "{}") as { message: { usage: { cache_read_input_tokens: number } } }; + const trailing = JSON.parse(lines[2] ?? "{}") as { message: { usage: { cache_read_input_tokens: number } } }; expect(trailing.message.usage.cache_read_input_tokens).toBe(9_000); expect(result.output).toBe("done"); From e1080f7437959e0c8db10cad04fb43a83b8230c9 Mon Sep 17 00:00:00 2001 From: Josh Owens Date: Fri, 18 Sep 2026 21:58:48 +0000 Subject: [PATCH 7/8] test: pin that a cached arm writes no transcript; explain the capability check (pass 6) - Test: the cached-baseline test now asserts the invariant its name promises. It checked the progress message and the run count, never that no file was written, so a regression that opened an empty .stream.jsonl for a cache-served arm would still have passed. Each run now gets its own transcript dir, so "no baseline file exists" is directly assertable while the proposed arm's file proves capture was live. Verified by inducing the regression. - The streamEvents rejection now says the check runs before any cache lookup. Skipping the demand for an arm that will be fully cache-served means computing cache keys inside validation, which couples two things this file keeps apart; the fail-fast stays, and now explains itself. Addresses review comments from github-actions[bot]. Co-Authored-By: Claude Opus 5 --- src/engine/compare.ts | 5 ++++- test/cache.test.ts | 25 ++++++++++++++++++++++--- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/engine/compare.ts b/src/engine/compare.ts index c3e26da..bec1851 100644 --- a/src/engine/compare.ts +++ b/src/engine/compare.ts @@ -258,9 +258,12 @@ export function validateRunnerSupport( runner: Runner, demands: { transcripts?: boolean } = {}, ): void { + // Deliberately ahead of any cache lookup: knowing an arm would be fully + // cache-served means computing its key, which validation has no business doing. if (demands.transcripts && !runner.capabilities.streamEvents) { throw new Error( - `transcript capture needs a runner that emits a per-event stream; runner "${runner.name}" does not (use claude-p)`, + `transcript capture needs a runner that emits a per-event stream; runner "${runner.name}" does not (use claude-p). ` + + `The check runs before any cache lookup, so it applies even to an arm that would be served from cache.`, ); } if (config.delivery === "install" && !runner.capabilities.skillRegistry) { diff --git a/test/cache.test.ts b/test/cache.test.ts index 302bd38..93f68f8 100644 --- a/test/cache.test.ts +++ b/test/cache.test.ts @@ -196,7 +196,6 @@ test("a cached baseline arm says it has no transcript to write instead of writin const fixture = makeFixture(); try { const cache = { dir: fixture.cacheDir }; - const transcriptOut = { dir: join(fixture.dir, "transcripts") }; // The counting runner stands in for claude-p here: capture demands a // streamEvents runner, which the engine checks before any run. const streamingRunner = (): CountingRunner => { @@ -204,21 +203,41 @@ test("a cached baseline arm says it has no transcript to write instead of writin return { ...runner, capabilities: { ...runner.capabilities, streamEvents: true } }; }; const first = streamingRunner(); - await runCompare({ config: fixture.config, runners: { baseline: first, proposed: first }, cache, transcriptOut }); + const firstTranscriptOut = { dir: join(fixture.dir, "transcripts-1") }; + await runCompare({ + config: fixture.config, + runners: { baseline: first, proposed: first }, + cache, + transcriptOut: firstTranscriptOut, + }); + // Sanity check on the harness itself: a genuine (cache-miss) baseline run + // does open transcript files, so the second run's directory is a fair test. + expect(readdirSync(firstTranscriptOut.dir).some((name) => name.includes("_baseline_"))).toBe(true); + // A fresh directory per run makes "no baseline file was created" directly + // assertable, rather than inferring it from mtimes on a shared directory + // that the proposed arm (never cached) also writes into on every run. const progress: string[] = []; const second = streamingRunner(); + const secondTranscriptOut = { dir: join(fixture.dir, "transcripts-2") }; await runCompare({ config: fixture.config, runners: { baseline: second, proposed: second }, cache, - transcriptOut, + transcriptOut: secondTranscriptOut, onProgress: (message) => progress.push(message), }); // The arm ran in an earlier invocation; capture cannot reach back for it. expect(second.counts.baseline).toBe(0); expect(progress).toContain(" baseline: cache hit — no transcript to write"); + // The actual invariant the test name promises: a cache-served baseline + // opens no transcript file at all, empty or otherwise. The proposed arm + // (not cached) still writes its own, which also proves transcript capture + // was live for this run and not just skipped wholesale. + const secondRunFiles = readdirSync(secondTranscriptOut.dir); + expect(secondRunFiles.some((name) => name.includes("_baseline_"))).toBe(false); + expect(secondRunFiles.some((name) => name.includes("_proposed_"))).toBe(true); } finally { rmSync(fixture.dir, { recursive: true, force: true }); } From bbca38f03f0e54bfff1aed5720d119b022160695 Mon Sep 17 00:00:00 2001 From: Josh Owens Date: Fri, 18 Sep 2026 22:05:03 +0000 Subject: [PATCH 8/8] docs: record the measured cost of the NDJSON line accumulator Third review question on this loop. The string accumulation reads as quadratic and measures linear, so the number belongs next to the code: a 16MB single line costs ~3ms, a 4MB one under 1ms. Co-Authored-By: Claude Opus 5 --- src/runner/claude-p.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/runner/claude-p.ts b/src/runner/claude-p.ts index 628c651..f03bc24 100644 --- a/src/runner/claude-p.ts +++ b/src/runner/claude-p.ts @@ -245,6 +245,11 @@ async function consumeStreamJson( // found (that's why the previous iteration's while loop exited) — only // the newly appended bytes need to be searched. Once a line is sliced // off below, the remaining buffer is unscanned from its own start 0. + // Accumulating into a string looks quadratic and measures linear: the + // engine ropes the `+=` instead of copying, and this offset keeps the + // scan off old bytes. A 16MB single line costs ~3ms, a 4MB one under 1ms, + // so a byte-buffer rewrite would add UTF-8 boundary handling here for no + // gain worth having. const scanned = pending.length; pending += decoder.decode(chunk, { stream: true }); let newline = pending.indexOf("\n", scanned);