From 688491fb5a38f3546a0ed39040c8b9828953cfa2 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Fri, 4 Sep 2026 08:05:37 -0700 Subject: [PATCH] perf(server): avoid full patches for checkpoint summaries (#9694) (cherry picked from commit c163d502dd32b993c03d8c20a5fae55b159bd8dc) --- apps/server/package.json | 1 - .../src/checkpointing/CheckpointStore.test.ts | 178 ++++++++++++++++++ .../src/checkpointing/CheckpointStore.ts | 4 +- apps/server/src/checkpointing/Diffs.test.ts | 82 ++++---- apps/server/src/checkpointing/Diffs.ts | 40 ++-- .../Layers/CheckpointReactor.test.ts | 140 ++++++++------ .../orchestration/Layers/CheckpointReactor.ts | 5 +- apps/server/src/vcs/GitVcsDriver.test.ts | 4 + apps/server/src/vcs/GitVcsDriver.ts | 7 +- apps/server/src/vcs/VcsDriver.ts | 1 + apps/server/src/vcs/VcsProcess.test.ts | 21 ++- apps/server/src/vcs/VcsProcess.ts | 3 +- docs/internals/glossary.md | 2 +- pnpm-lock.yaml | 3 - 14 files changed, 355 insertions(+), 136 deletions(-) diff --git a/apps/server/package.json b/apps/server/package.json index c7a6f60fb..246006407 100644 --- a/apps/server/package.json +++ b/apps/server/package.json @@ -29,7 +29,6 @@ "@effect/sql-sqlite-bun": "catalog:", "@ff-labs/fff-node": "0.9.4", "@opencode-ai/sdk": "^1.3.15", - "@pierre/diffs": "catalog:", "@sigstore/bundle": "4.0.0", "@sigstore/core": "3.2.1", "@sigstore/tuf": "4.0.0", diff --git a/apps/server/src/checkpointing/CheckpointStore.test.ts b/apps/server/src/checkpointing/CheckpointStore.test.ts index 2f4685898..5a9b5cc5d 100644 --- a/apps/server/src/checkpointing/CheckpointStore.test.ts +++ b/apps/server/src/checkpointing/CheckpointStore.test.ts @@ -4,6 +4,7 @@ import * as NodePath from "node:path"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { it } from "@effect/vitest"; import { ThreadId, type VcsError } from "@t3tools/contracts"; +import { HostProcessPlatform } from "@t3tools/shared/hostProcess"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; @@ -12,6 +13,7 @@ import * as Scope from "effect/Scope"; import { describe, expect } from "vite-plus/test"; import { checkpointRefForThreadTurn } from "./Utils.ts"; +import { parseTurnDiffFilesFromNumstat } from "./Diffs.ts"; import * as CheckpointStore from "./CheckpointStore.ts"; import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; @@ -250,6 +252,182 @@ it.layer(TestLayer)("CheckpointStore.layer", (it) => { expect(whitespaceIgnoredDiff).toContain("+
"); expect(whitespaceIgnoredDiff).not.toContain("-

Title

"); expect(whitespaceIgnoredDiff).not.toContain("+

Title

"); + + for (const ignoreWhitespace of [false, true]) { + const numstat = yield* checkpointStore.diffCheckpoints({ + cwd: tmp, + fromCheckpointRef, + toCheckpointRef, + ignoreWhitespace, + format: "numstat", + }); + expect(parseTurnDiffFilesFromNumstat(numstat)).toEqual([ + { + path: "Component.tsx", + additions: ignoreWhitespace ? 4 : 6, + deletions: ignoreWhitespace ? 0 : 2, + }, + ]); + } + }), + ); + }); + + describe("checkpoint file summaries", () => { + it.effect("counts changes whose full patch exceeds the output limit", () => + Effect.gen(function* () { + const tmp = yield* makeTmpDir(); + yield* initRepoWithCommit(tmp); + const checkpointStore = yield* CheckpointStore.CheckpointStore; + const threadId = ThreadId.make("large-checkpoint-summary"); + const fromCheckpointRef = checkpointRefForThreadTurn(threadId, 0); + const toCheckpointRef = checkpointRefForThreadTurn(threadId, 1); + const filePath = NodePath.join(tmp, "README.md"); + const lineCount = 20_000; + yield* writeTextFile(filePath, `${"before".repeat(50)}\n`.repeat(lineCount)); + yield* checkpointStore.captureCheckpoint({ cwd: tmp, checkpointRef: fromCheckpointRef }); + yield* writeTextFile(filePath, `${"after".repeat(60)}\n`.repeat(lineCount)); + yield* checkpointStore.captureCheckpoint({ cwd: tmp, checkpointRef: toCheckpointRef }); + + const numstat = yield* checkpointStore.diffCheckpoints({ + cwd: tmp, + fromCheckpointRef, + toCheckpointRef, + ignoreWhitespace: false, + format: "numstat", + }); + + expect(parseTurnDiffFilesFromNumstat(numstat)).toEqual([ + { path: "README.md", additions: lineCount, deletions: lineCount }, + ]); + expect(numstat.length).toBeLessThan(100); + }), + ); + + it.effect("preserves file paths and turn ranges without changing the user index", () => + Effect.gen(function* () { + const tmp = yield* makeTmpDir(); + yield* initRepoWithCommit(tmp); + yield* git(tmp, ["config", "diff.renames", "copies"]); + const fileSystem = yield* FileSystem.FileSystem; + const checkpointStore = yield* CheckpointStore.CheckpointStore; + const threadId = ThreadId.make("checkpoint-summary-paths"); + const baseline = checkpointRefForThreadTurn(threadId, 0); + const firstTurn = checkpointRefForThreadTurn(threadId, 1); + const secondTurn = checkpointRefForThreadTurn(threadId, 2); + const copiedText = Array.from({ length: 20 }, (_, index) => `copy line ${index}\n`).join( + "", + ); + const platform = yield* HostProcessPlatform; + const renamedPath = platform === "win32" ? "renamed café.txt" : "renamed\tcafé\nname.txt"; + const addedPath = platform === "win32" ? "new café.txt" : "new\tfile\n名.txt"; + for (const [path, contents] of Object.entries({ + "copy-source.txt": copiedText, + "deleted.txt": "delete me\n", + "rename-old.txt": "before\nkeep one\nkeep two\nkeep three\n", + "binary.bin": "\0before", + })) { + yield* writeTextFile(NodePath.join(tmp, path), contents); + } + yield* checkpointStore.captureCheckpoint({ cwd: tmp, checkpointRef: baseline }); + + yield* fileSystem.rename( + NodePath.join(tmp, "rename-old.txt"), + NodePath.join(tmp, renamedPath), + ); + yield* fileSystem.remove(NodePath.join(tmp, "deleted.txt")); + for (const [path, contents] of Object.entries({ + "copy-source.txt": `${copiedText}one more\n`, + "copied.txt": copiedText, + [renamedPath]: "after\nkeep one\nkeep two\nkeep three\n", + "binary.bin": "\0after", + "empty.txt": "", + [addedPath]: "first\nsecond\n", + })) { + yield* writeTextFile(NodePath.join(tmp, path), contents); + } + yield* checkpointStore.captureCheckpoint({ cwd: tmp, checkpointRef: firstTurn }); + const userIndex = yield* fileSystem.readFile(NodePath.join(tmp, ".git/index")); + const input = { + cwd: tmp, + fromCheckpointRef: baseline, + toCheckpointRef: firstTurn, + ignoreWhitespace: false, + format: "numstat" as const, + }; + const firstSummary = parseTurnDiffFilesFromNumstat( + yield* checkpointStore.diffCheckpoints(input), + ); + const expectedFiles = [ + { path: "binary.bin", additions: 0, deletions: 0 }, + { path: "copied.txt", additions: 0, deletions: 0 }, + { path: "copy-source.txt", additions: 1, deletions: 0 }, + { path: "deleted.txt", additions: 0, deletions: 1 }, + { path: "empty.txt", additions: 0, deletions: 0 }, + { path: addedPath, additions: 2, deletions: 0 }, + { path: renamedPath, additions: 1, deletions: 1 }, + ].toSorted((left, right) => left.path.localeCompare(right.path)); + expect(firstSummary).toEqual(expectedFiles); + + yield* fileSystem.remove(NodePath.join(tmp, "empty.txt")); + yield* writeTextFile(NodePath.join(tmp, "copy-source.txt"), "replacement\n"); + yield* checkpointStore.captureCheckpoint({ cwd: tmp, checkpointRef: secondTurn }); + const secondSummary = parseTurnDiffFilesFromNumstat( + yield* checkpointStore.diffCheckpoints({ + ...input, + fromCheckpointRef: firstTurn, + toCheckpointRef: secondTurn, + }), + ); + expect(secondSummary).toEqual([ + { path: "copy-source.txt", additions: 1, deletions: 21 }, + { path: "empty.txt", additions: 0, deletions: 0 }, + ]); + + const inclusiveSummary = parseTurnDiffFilesFromNumstat( + yield* checkpointStore.diffCheckpoints({ ...input, toCheckpointRef: secondTurn }), + ); + expect(inclusiveSummary).toEqual( + expectedFiles + .filter((file) => file.path !== "empty.txt") + .map((file) => + file.path === "copy-source.txt" ? { ...file, additions: 1, deletions: 20 } : file, + ), + ); + expect( + yield* checkpointStore.diffCheckpoints({ ...input, toCheckpointRef: baseline }), + ).toBe(""); + expect(yield* fileSystem.readFile(NodePath.join(tmp, ".git/index"))).toEqual(userIndex); + }), + ); + + it.effect("uses HEAD for a missing baseline only when requested", () => + Effect.gen(function* () { + const tmp = yield* makeTmpDir(); + yield* initRepoWithCommit(tmp); + const checkpointStore = yield* CheckpointStore.CheckpointStore; + const threadId = ThreadId.make("checkpoint-summary-fallback"); + const fromCheckpointRef = checkpointRefForThreadTurn(threadId, 0); + const toCheckpointRef = checkpointRefForThreadTurn(threadId, 1); + yield* writeTextFile(NodePath.join(tmp, "README.md"), "changed\n"); + yield* checkpointStore.captureCheckpoint({ cwd: tmp, checkpointRef: toCheckpointRef }); + const input = { + cwd: tmp, + fromCheckpointRef, + toCheckpointRef, + ignoreWhitespace: false, + format: "numstat" as const, + }; + + const error = yield* Effect.flip(checkpointStore.diffCheckpoints(input)); + expect(error._tag).toBe("VcsProcessExitError"); + const numstat = yield* checkpointStore.diffCheckpoints({ + ...input, + fallbackFromToHead: true, + }); + expect(parseTurnDiffFilesFromNumstat(numstat)).toEqual([ + { path: "README.md", additions: 1, deletions: 1 }, + ]); }), ); }); diff --git a/apps/server/src/checkpointing/CheckpointStore.ts b/apps/server/src/checkpointing/CheckpointStore.ts index f13aa4572..f1cc596b2 100644 --- a/apps/server/src/checkpointing/CheckpointStore.ts +++ b/apps/server/src/checkpointing/CheckpointStore.ts @@ -39,6 +39,7 @@ export interface DiffCheckpointsInput { readonly toCheckpointRef: CheckpointRef; readonly fallbackFromToHead?: boolean; readonly ignoreWhitespace: boolean; + readonly format?: "patch" | "numstat"; } export interface DeleteCheckpointRefsInput { @@ -77,8 +78,9 @@ export class CheckpointStore extends Context.Service< ) => Effect.Effect; /** - * Compute a patch diff between two checkpoint refs. + * Compute a diff between two checkpoint refs. Defaults to a full patch. * + * Numstat output has NUL-delimited paths for file summaries. * Can optionally treat a missing "from" ref as `HEAD`. */ readonly diffCheckpoints: ( diff --git a/apps/server/src/checkpointing/Diffs.test.ts b/apps/server/src/checkpointing/Diffs.test.ts index f27f5ecb8..6b7f1a875 100644 --- a/apps/server/src/checkpointing/Diffs.test.ts +++ b/apps/server/src/checkpointing/Diffs.test.ts @@ -1,68 +1,54 @@ import { describe, expect, it } from "vite-plus/test"; -import { parseTurnDiffFilesFromUnifiedDiff } from "./Diffs.ts"; +import { parseTurnDiffFilesFromNumstat } from "./Diffs.ts"; -describe("parseTurnDiffFilesFromUnifiedDiff", () => { - it("returns empty list for empty diff", () => { - expect(parseTurnDiffFilesFromUnifiedDiff("")).toEqual([]); +describe("parseTurnDiffFilesFromNumstat", () => { + it("returns an empty list when no files changed", () => { + expect(parseTurnDiffFilesFromNumstat("")).toEqual([]); }); - it("parses per-file additions and deletions", () => { - const diff = [ - "diff --git a/a.txt b/a.txt", - "index 1111111..2222222 100644", - "--- a/a.txt", - "+++ b/a.txt", - "@@ -1,2 +1,3 @@", - " one", - "-two", - "+two updated", - "+three", - "diff --git a/src/b.ts b/src/b.ts", - "index 3333333..4444444 100644", - "--- a/src/b.ts", - "+++ b/src/b.ts", - "@@ -3,2 +3,0 @@", - "-old", - "-stale", - "", - ].join("\n"); - - expect(parseTurnDiffFilesFromUnifiedDiff(diff)).toEqual([ + it("sorts files and preserves addition and deletion counts", () => { + const numstat = ["0\t2\tsrc/b.ts", "2\t1\ta.txt", ""].join("\0"); + expect(parseTurnDiffFilesFromNumstat(numstat)).toEqual([ { path: "a.txt", additions: 2, deletions: 1 }, { path: "src/b.ts", additions: 0, deletions: 2 }, ]); }); - it("parses rename-only diffs with zero line changes", () => { - const diff = [ - "diff --git a/src/old.ts b/src/new.ts", - "similarity index 100%", - "rename from src/old.ts", - "rename to src/new.ts", + it("uses destination paths for renames and copies", () => { + const numstat = [ + "0\t0\t", + "src/old.ts", + "src/new.ts", + "2\t1\t", + "src/source.ts", + "src/copied.ts", + "1\t0\tother.ts", "", - ].join("\n"); + ].join("\0"); - expect(parseTurnDiffFilesFromUnifiedDiff(diff)).toEqual([ + expect(parseTurnDiffFilesFromNumstat(numstat)).toEqual([ + { path: "other.ts", additions: 1, deletions: 0 }, + { path: "src/copied.ts", additions: 2, deletions: 1 }, { path: "src/new.ts", additions: 0, deletions: 0 }, ]); }); - it("normalizes CRLF input before parsing", () => { - const diff = [ - "diff --git a/a.txt b/a.txt", - "index 1111111..2222222 100644", - "--- a/a.txt", - "+++ b/a.txt", - "@@ -1 +1,2 @@", - "-one", - "+one updated", - "+two", - "", - ].join("\r\n"); + it("keeps binary files and empty files with zero line changes", () => { + const numstat = ["-\t-\timage.png", "0\t0\tempty.txt", ""].join("\0"); + expect(parseTurnDiffFilesFromNumstat(numstat)).toEqual([ + { path: "empty.txt", additions: 0, deletions: 0 }, + { path: "image.png", additions: 0, deletions: 0 }, + ]); + }); - expect(parseTurnDiffFilesFromUnifiedDiff(diff)).toEqual([ - { path: "a.txt", additions: 2, deletions: 1 }, + it("preserves Unicode, tabs, line endings, and spaces in paths", () => { + const path = " café\tline\r\nname.txt "; + const numstat = `3\t2\t\0old\tname\n.txt\0${path}\0`; + + expect(parseTurnDiffFilesFromNumstat(numstat)).toEqual([{ path, additions: 3, deletions: 2 }]); + expect(parseTurnDiffFilesFromNumstat(`1\t0\t${path}\0`)).toEqual([ + { path, additions: 1, deletions: 0 }, ]); }); }); diff --git a/apps/server/src/checkpointing/Diffs.ts b/apps/server/src/checkpointing/Diffs.ts index 0eeee1b6f..b79510091 100644 --- a/apps/server/src/checkpointing/Diffs.ts +++ b/apps/server/src/checkpointing/Diffs.ts @@ -1,27 +1,33 @@ -import { parsePatchFiles } from "@pierre/diffs/utils/parsePatchFiles"; - export interface TurnDiffFileSummary { readonly path: string; readonly additions: number; readonly deletions: number; } -export function parseTurnDiffFilesFromUnifiedDiff( - diff: string, -): ReadonlyArray { - const normalized = diff.replace(/\r\n/g, "\n").trim(); - if (normalized.length === 0) { - return []; - } +/** Reads Git's NUL-delimited numstat output without decoding display paths. */ +export function parseTurnDiffFilesFromNumstat(numstat: string): ReadonlyArray { + const records = numstat.split("\0"); + const files: TurnDiffFileSummary[] = []; + + for (let index = 0; index < records.length; index += 1) { + const record = records[index]!; + const counts = /^(\d+|-)\t(\d+|-)\t/.exec(record); + if (!counts) continue; - const parsedPatches = parsePatchFiles(normalized); - const files = parsedPatches.flatMap((patch) => - patch.files.map((file) => ({ - path: file.name, - additions: file.hunks.reduce((total, hunk) => total + hunk.additionLines, 0), - deletions: file.hunks.reduce((total, hunk) => total + hunk.deletionLines, 0), - })), - ); + let path = record.slice(counts[0].length); + if (path.length === 0) { + // Renames and copies use two more records: the source and destination. + path = records[index + 2] ?? ""; + index += 2; + } + if (path.length === 0) continue; + + files.push({ + path, + additions: counts[1] === "-" ? 0 : Number(counts[1]), + deletions: counts[2] === "-" ? 0 : Number(counts[2]), + }); + } return files.toSorted((left, right) => left.path.localeCompare(right.path)); } diff --git a/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts b/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts index 4e720423a..935ad3f47 100644 --- a/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts +++ b/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts @@ -524,12 +524,14 @@ describe("CheckpointReactor", () => { }; } - it("captures pre-turn baseline on turn.started and post-turn checkpoint on turn.completed", async () => { - const harness = await createHarness({ seedFilesystemCheckpoints: false }); - const createdAt = "2026-01-01T00:00:00.000Z"; + effectIt.effect("captures baseline and large turn summaries before completion receipts", () => + Effect.gen(function* () { + const harness = yield* Effect.promise(() => + createHarness({ seedFilesystemCheckpoints: false }), + ); + const createdAt = "2026-01-01T00:00:00.000Z"; - await Effect.runPromise( - harness.engine.dispatch({ + yield* harness.engine.dispatch({ type: "thread.session.set", commandId: CommandId.make("cmd-session-set-capture"), threadId: ThreadId.make("thread-1"), @@ -543,66 +545,84 @@ describe("CheckpointReactor", () => { updatedAt: createdAt, }, createdAt, - }), - ); + }); - harness.provider.emit({ - type: "turn.started", - eventId: EventId.make("evt-turn-started-1"), - provider: ProviderDriverKind.make("codex"), + harness.provider.emit({ + type: "turn.started", + eventId: EventId.make("evt-turn-started-1"), + provider: ProviderDriverKind.make("codex"), - createdAt: "2026-01-01T00:00:00.000Z", - threadId: ThreadId.make("thread-1"), - turnId: asTurnId("turn-1"), - }); - await waitForGitRefExists( - harness.cwd, - checkpointRefForThreadTurn(ThreadId.make("thread-1"), 0), - ); + createdAt: "2026-01-01T00:00:00.000Z", + threadId: ThreadId.make("thread-1"), + turnId: asTurnId("turn-1"), + }); + expect(yield* harness.nextReceipt).toMatchObject({ + type: "checkpoint.baseline.captured", + checkpointTurnCount: 0, + }); - NodeFS.writeFileSync(NodePath.join(harness.cwd, "README.md"), "v2\n", "utf8"); - harness.provider.emit({ - type: "turn.completed", - eventId: EventId.make("evt-turn-completed-1"), - provider: ProviderDriverKind.make("codex"), + NodeFS.writeFileSync(NodePath.join(harness.cwd, "README.md"), "v2\n", "utf8"); + const largeFileLineCount = 25_000; + NodeFS.writeFileSync( + NodePath.join(harness.cwd, "large.txt"), + `${"payload".repeat(64)}\n`.repeat(largeFileLineCount), + "utf8", + ); + harness.provider.emit({ + type: "turn.completed", + eventId: EventId.make("evt-turn-completed-1"), + provider: ProviderDriverKind.make("codex"), - createdAt: "2026-01-01T00:00:00.000Z", - threadId: ThreadId.make("thread-1"), - turnId: asTurnId("turn-1"), - payload: { state: "completed" }, - }); + createdAt: "2026-01-01T00:00:00.000Z", + threadId: ThreadId.make("thread-1"), + turnId: asTurnId("turn-1"), + payload: { state: "completed" }, + }); - await waitForEvent(harness.engine, (event) => event.type === "thread.turn-diff-completed"); - const thread = await waitForThread( - harness.readModel, - (entry) => entry.latestTurn?.turnId === "turn-1" && entry.checkpoints.length === 1, - ); - expect(thread.checkpoints[0]?.checkpointTurnCount).toBe(1); - expect( - (thread.checkpoints[0] as { readonly assistantMessageId: string | null } | undefined) - ?.assistantMessageId, - ).toBeNull(); - expect( - gitRefExists(harness.cwd, checkpointRefForThreadTurn(ThreadId.make("thread-1"), 0)), - ).toBe(true); - expect( - gitRefExists(harness.cwd, checkpointRefForThreadTurn(ThreadId.make("thread-1"), 1)), - ).toBe(true); - expect( - gitShowFileAtRef( - harness.cwd, - checkpointRefForThreadTurn(ThreadId.make("thread-1"), 0), - "README.md", - ), - ).toBe("v1\n"); - expect( - gitShowFileAtRef( - harness.cwd, - checkpointRefForThreadTurn(ThreadId.make("thread-1"), 1), - "README.md", - ), - ).toBe("v2\n"); - }); + expect(yield* harness.nextReceipt).toMatchObject({ + type: "checkpoint.diff.finalized", + turnId: "turn-1", + checkpointTurnCount: 1, + }); + const thread = (yield* Effect.promise(harness.readModel)).threads.find( + (entry) => entry.id === "thread-1", + ); + expect(thread?.checkpoints[0]).toMatchObject({ + checkpointTurnCount: 1, + assistantMessageId: null, + files: [ + { path: "large.txt", kind: "modified", additions: largeFileLineCount, deletions: 0 }, + { path: "README.md", kind: "modified", additions: 1, deletions: 1 }, + ], + }); + expect(yield* harness.nextReceipt).toMatchObject({ + type: "turn.processing.quiesced", + turnId: "turn-1", + checkpointTurnCount: 1, + }); + yield* Effect.promise(harness.drain); + expect( + gitRefExists(harness.cwd, checkpointRefForThreadTurn(ThreadId.make("thread-1"), 0)), + ).toBe(true); + expect( + gitRefExists(harness.cwd, checkpointRefForThreadTurn(ThreadId.make("thread-1"), 1)), + ).toBe(true); + expect( + gitShowFileAtRef( + harness.cwd, + checkpointRefForThreadTurn(ThreadId.make("thread-1"), 0), + "README.md", + ), + ).toBe("v1\n"); + expect( + gitShowFileAtRef( + harness.cwd, + checkpointRefForThreadTurn(ThreadId.make("thread-1"), 1), + "README.md", + ), + ).toBe("v2\n"); + }), + ); effectIt.effect.each(["turn.completed", "turn.aborted"] as const)( "captures every edit after a mid-turn diff update on %s", diff --git a/apps/server/src/orchestration/Layers/CheckpointReactor.ts b/apps/server/src/orchestration/Layers/CheckpointReactor.ts index 30e076370..8fb6f6568 100644 --- a/apps/server/src/orchestration/Layers/CheckpointReactor.ts +++ b/apps/server/src/orchestration/Layers/CheckpointReactor.ts @@ -22,7 +22,7 @@ import * as Stream from "effect/Stream"; import { makeDrainableWorker } from "@t3tools/shared/DrainableWorker"; import { isTemporaryWorktreeBranch } from "@t3tools/shared/git"; -import { parseTurnDiffFilesFromUnifiedDiff } from "../../checkpointing/Diffs.ts"; +import { parseTurnDiffFilesFromNumstat } from "../../checkpointing/Diffs.ts"; import { checkpointRefForThreadTurn, resolveThreadWorkspaceCwd, @@ -381,11 +381,12 @@ export const make = Effect.gen(function* () { toCheckpointRef: targetCheckpointRef, fallbackFromToHead: false, ignoreWhitespace: false, + format: "numstat", }) : Effect.succeed("") ).pipe( Effect.map((diff) => - parseTurnDiffFilesFromUnifiedDiff(diff).map((file) => ({ + parseTurnDiffFilesFromNumstat(diff).map((file) => ({ path: file.path, kind: "modified" as const, additions: file.additions, diff --git a/apps/server/src/vcs/GitVcsDriver.test.ts b/apps/server/src/vcs/GitVcsDriver.test.ts index 89f7c55d5..031055a3b 100644 --- a/apps/server/src/vcs/GitVcsDriver.test.ts +++ b/apps/server/src/vcs/GitVcsDriver.test.ts @@ -68,6 +68,7 @@ runVcsDriverContractSuite({ it.effect("GitVcsDriver forwards execute env to the VCS process", () => { let observedEnv: NodeJS.ProcessEnv | undefined; let observedAppendTruncationMarker: boolean | undefined; + let observedOutputMode: VcsProcess.VcsProcessInput["outputMode"]; return Effect.gen(function* () { const driver = yield* GitVcsDriver.makeVcsDriverShape(); @@ -80,12 +81,14 @@ it.effect("GitVcsDriver forwards execute env to the VCS process", () => { GIT_INDEX_FILE: "/tmp/t3-index", }, appendTruncationMarker: true, + outputMode: "error", }); assert.deepStrictEqual(observedEnv, { GIT_INDEX_FILE: "/tmp/t3-index", }); assert.strictEqual(observedAppendTruncationMarker, true); + assert.strictEqual(observedOutputMode, "error"); }).pipe( Effect.provide( Layer.mergeAll( @@ -95,6 +98,7 @@ it.effect("GitVcsDriver forwards execute env to the VCS process", () => { Effect.sync(() => { observedEnv = input.env; observedAppendTruncationMarker = input.appendTruncationMarker; + observedOutputMode = input.outputMode; return { exitCode: ChildProcessSpawner.ExitCode(0), stdout: "", diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts index 4c2f34400..1a677fc76 100644 --- a/apps/server/src/vcs/GitVcsDriver.ts +++ b/apps/server/src/vcs/GitVcsDriver.ts @@ -431,6 +431,7 @@ const gitCommand = ( readonly allowNonZeroExit?: boolean; readonly timeoutMs?: number; readonly maxOutputBytes?: number; + readonly outputMode?: VcsProcess.VcsProcessInput["outputMode"]; readonly appendTruncationMarker?: boolean; }, ) => @@ -447,6 +448,7 @@ const gitCommand = ( : {}), ...(options?.timeoutMs !== undefined ? { timeoutMs: options.timeoutMs } : {}), ...(options?.maxOutputBytes !== undefined ? { maxOutputBytes: options.maxOutputBytes } : {}), + ...(options?.outputMode !== undefined ? { outputMode: options.outputMode } : {}), ...(options?.appendTruncationMarker !== undefined ? { appendTruncationMarker: options.appendTruncationMarker } : {}), @@ -485,6 +487,7 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( ...(input.allowNonZeroExit !== undefined ? { allowNonZeroExit: input.allowNonZeroExit } : {}), ...(input.timeoutMs !== undefined ? { timeoutMs: input.timeoutMs } : {}), ...(input.maxOutputBytes !== undefined ? { maxOutputBytes: input.maxOutputBytes } : {}), + ...(input.outputMode !== undefined ? { outputMode: input.outputMode } : {}), ...(input.appendTruncationMarker !== undefined ? { appendTruncationMarker: input.appendTruncationMarker } : {}), @@ -838,6 +841,7 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( "checkpoint.from_ref": input.fromCheckpointRef, "checkpoint.to_ref": input.toCheckpointRef, "checkpoint.ignore_whitespace": input.ignoreWhitespace, + "checkpoint.format": input.format ?? "patch", "checkpoint.fallback_from_to_head": input.fallbackFromToHead, }); @@ -869,7 +873,7 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( cwd: input.cwd, args: [ "diff", - "--patch", + ...(input.format === "numstat" ? ["--numstat", "-z"] : ["--patch"]), "--no-color", "--no-ext-diff", "--no-textconv", @@ -880,6 +884,7 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( ], allowNonZeroExit: true, maxOutputBytes: CHECKPOINT_DIFF_MAX_OUTPUT_BYTES, + outputMode: input.format === "numstat" ? "error" : "truncate", }); if (result.exitCode !== 0) { diff --git a/apps/server/src/vcs/VcsDriver.ts b/apps/server/src/vcs/VcsDriver.ts index f2daf7935..a8680c9a5 100644 --- a/apps/server/src/vcs/VcsDriver.ts +++ b/apps/server/src/vcs/VcsDriver.ts @@ -31,6 +31,7 @@ export interface VcsDiffCheckpointsInput { readonly toCheckpointRef: CheckpointRef; readonly fallbackFromToHead?: boolean; readonly ignoreWhitespace: boolean; + readonly format?: "patch" | "numstat"; } export interface VcsDeleteCheckpointRefsInput { diff --git a/apps/server/src/vcs/VcsProcess.test.ts b/apps/server/src/vcs/VcsProcess.test.ts index 91191178c..ac0ee4428 100644 --- a/apps/server/src/vcs/VcsProcess.test.ts +++ b/apps/server/src/vcs/VcsProcess.test.ts @@ -1,5 +1,6 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; -import { describe, expect, it } from "@effect/vitest"; +import { assert, describe, expect, it } from "@effect/vitest"; +import { HostProcessWorkingDirectory } from "@t3tools/shared/hostProcess"; import * as Duration from "effect/Duration"; import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; @@ -359,6 +360,24 @@ describe("VcsProcess.run", () => { }).pipe(provideLive), ); + it.effect("fails with measured byte counts when output must not be truncated", () => + Effect.gen(function* () { + const error = yield* run({ + operation: "test.output-limit", + command: "node", + args: ["-e", "process.stdout.write('x'.repeat(2048))"], + cwd: yield* HostProcessWorkingDirectory, + maxOutputBytes: 128, + outputMode: "error", + }).pipe(Effect.flip); + + assert(error._tag === "VcsProcessOutputLimitError"); + expect(error.stream).toBe("stdout"); + expect(error.maxBytes).toBe(128); + expect(error.observedBytes).toBeGreaterThan(error.maxBytes); + }).pipe(provideLive), + ); + it.effect("fails with VcsProcessTimeoutError on timeout", () => Effect.gen(function* () { const errorFiber = yield* run({ diff --git a/apps/server/src/vcs/VcsProcess.ts b/apps/server/src/vcs/VcsProcess.ts index 69df83c42..74608e116 100644 --- a/apps/server/src/vcs/VcsProcess.ts +++ b/apps/server/src/vcs/VcsProcess.ts @@ -29,6 +29,7 @@ export interface VcsProcessInput { readonly allowNonZeroExit?: boolean; readonly timeoutMs?: number; readonly maxOutputBytes?: number; + readonly outputMode?: ProcessRunner.ProcessRunInput["outputMode"]; readonly appendTruncationMarker?: boolean; } @@ -125,7 +126,7 @@ export const make = Effect.gen(function* () { ...(input.env !== undefined ? { env: input.env } : {}), timeout: input.timeoutMs ?? DEFAULT_TIMEOUT_MS, maxOutputBytes: input.maxOutputBytes ?? DEFAULT_MAX_OUTPUT_BYTES, - outputMode: "truncate", + outputMode: input.outputMode ?? "truncate", truncatedMarker: input.appendTruncationMarker ? OUTPUT_TRUNCATED_MARKER : "", timeoutBehavior: "error", }) diff --git a/docs/internals/glossary.md b/docs/internals/glossary.md index 239c148c0..f8c22647a 100644 --- a/docs/internals/glossary.md +++ b/docs/internals/glossary.md @@ -172,7 +172,7 @@ A durable rollback state used when Pylon cannot prove that both the workspace an #### Checkpoint diff -The patch difference between two checkpoints. Query logic lives in [CheckpointDiffQuery.ts][20], diff parsing lives in [Diffs.ts][23], and finalization is coordinated by [CheckpointReactor.ts][6]. +The difference between two checkpoints. [CheckpointDiffQuery.ts][20] reads full patches on demand. [CheckpointReactor.ts][6] uses NUL-delimited Git numstat output for automatic file summaries, parsed by [Diffs.ts][23]. #### Turn diff diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 35b2e8bd7..68818fca2 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -495,9 +495,6 @@ importers: '@opencode-ai/sdk': specifier: ^1.3.15 version: 1.15.13 - '@pierre/diffs': - specifier: 'catalog:' - version: 1.3.0-beta.10(patch_hash=c2ea3a9821addf19641e88074a6d8896c62b0f7cfa762899b0e9e80ff5e8add4)(@shikijs/themes@4.3.0)(react-dom@19.2.6(react@19.2.6))(react@19.2.6) '@sigstore/bundle': specifier: 4.0.0 version: 4.0.0