diff --git a/packages/coding-agent/src/edit/snapshot-details.ts b/packages/coding-agent/src/edit/snapshot-details.ts index 97da75a4a..f83e4c9dc 100644 --- a/packages/coding-agent/src/edit/snapshot-details.ts +++ b/packages/coding-agent/src/edit/snapshot-details.ts @@ -14,11 +14,15 @@ import type { EditToolDetails, EditToolPerFileResult } from "./renderer"; /** - * Combined `oldText` + `newText` character budget per edit-tool result. + * Combined `oldText` + `newText` character budget for a single edit-tool + * result. Applies both per-entry (one file at a time) and as an aggregate + * across `perFileResults` (so a many-small-files batch can't accumulate + * unbounded snapshot bytes — see #3787 review). * * Picked so typical code-file edits keep ACP diff visualization while - * pathological cases (large generated files, full-file rewrites) drop the raw - * snapshots before they hit the session JSONL. + * pathological cases (large generated files, full-file rewrites, or + * many-file batches) drop the raw snapshots before they hit the + * session JSONL. */ export const MAX_EDIT_SNAPSHOT_TEXT_CHARS = 32_768; @@ -32,6 +36,28 @@ function pruneSnapshot(details: T): T { return { ...rest, snapshotsPruned: true } as T; } +/** + * Walk `perFileResults` in order with a shared budget. Each per-entry payload + * is first capped individually by {@link pruneSnapshot}; if its kept bytes + * would push the running aggregate past the cap, strip and mark this entry + * too. Early entries get to keep their diff visualization; later entries in + * a large batch degrade to text-only. + */ +function capPerFileSnapshots(entries: T[]): T[] { + let remaining = MAX_EDIT_SNAPSHOT_TEXT_CHARS; + return entries.map(entry => { + const perEntry = pruneSnapshot(entry); + const kept = (perEntry.oldText?.length ?? 0) + (perEntry.newText?.length ?? 0); + if (kept === 0) return perEntry; + if (kept <= remaining) { + remaining -= kept; + return perEntry; + } + const { oldText: _old, newText: _new, ...rest } = perEntry; + return { ...rest, snapshotsPruned: true } as T; + }); +} + /** * Prune oversized `oldText` / `newText` from an edit-tool details payload, * recursing into `perFileResults` when present. Per-file overload comes first @@ -45,7 +71,7 @@ export function pruneOversizedEditSnapshots( ): EditToolDetails | EditToolPerFileResult { const pruned = pruneSnapshot(details); if ("perFileResults" in pruned && pruned.perFileResults) { - return { ...pruned, perFileResults: pruned.perFileResults.map(pruneSnapshot) }; + return { ...pruned, perFileResults: capPerFileSnapshots(pruned.perFileResults) }; } return pruned; } diff --git a/packages/coding-agent/test/edit-snapshot-details.test.ts b/packages/coding-agent/test/edit-snapshot-details.test.ts index 856357c77..7a22d42d5 100644 --- a/packages/coding-agent/test/edit-snapshot-details.test.ts +++ b/packages/coding-agent/test/edit-snapshot-details.test.ts @@ -94,6 +94,35 @@ describe("pruneOversizedEditSnapshots", () => { newText: small, }); }); + + test("caps cumulative perFileResults snapshots at the shared aggregate budget", () => { + // Each entry is individually under the per-entry budget but their sum + // busts it: walking left-to-right, the first two fit, the rest must be + // stripped so a many-small-files batch can't accumulate unbounded bytes. + const entrySize = Math.floor(MAX_EDIT_SNAPSHOT_TEXT_CHARS / 4); + const chunk = "y".repeat(entrySize); + const entries = Array.from({ length: 5 }, (_, i) => ({ + path: `/f${i}`, + diff: `d${i}`, + oldText: chunk, + newText: chunk, + })); + const result = pruneOversizedEditSnapshots({ diff: "agg", perFileResults: entries }); + + const kept = result.perFileResults!.filter(e => e.oldText !== undefined); + const pruned = result.perFileResults!.filter(e => e.snapshotsPruned === true); + expect(kept.length).toBe(2); + expect(pruned.length).toBe(3); + + // Total kept snapshot bytes never exceed the shared cap. + const totalKept = result.perFileResults!.reduce( + (acc, e) => acc + (e.oldText?.length ?? 0) + (e.newText?.length ?? 0), + 0, + ); + expect(totalKept).toBeLessThanOrEqual(MAX_EDIT_SNAPSHOT_TEXT_CHARS); + // Pruned entries keep their diff/path so the renderer still works. + expect(pruned[0]).toMatchObject({ path: "/f2", diff: "d2", snapshotsPruned: true }); + }); }); describe("executePatchSingle on oversized files", () => {