Stub superseded read_file results during compaction (CL-4374)

src/session/compactor.ts

@@ -228,6 +228,99 @@ export function compactorNoOpFloor(keepRecentTurns: number): number {
 // Minimum anchor score for a turn to be pulled forward past the summary boundary.
 const ANCHOR_SCORE_THRESHOLD = 5;
 
+// Tool names whose results are path-keyed for re-read dedup during compaction.
+const READ_TOOLS = new Set(["read_file"]);
+
+// Call-id index for stub rendering (name + path). Not path-keyed — that is
+// buildPathToReads below.
+type ToolCallInfo = {
+  name: string;
+  pathArg?: string;
+};
+
+type PathRead = {
+  callId: string;
+  /** Monotonic order across the turn list; higher = later in the session. */
+  order: number;
+  isError: boolean;
+};
+
+function pathArgFromArguments(raw: unknown): string | undefined {
+  let args: unknown = raw ?? {};
+  if (typeof args === "string") {
+    try {
+      args = JSON.parse(args) as unknown;
+    } catch {
+      return undefined;
+    }
+  }
+  if (args === null || typeof args !== "object" || Array.isArray(args)) return undefined;
+  const path = (args as Record<string, unknown>)["path"];
+  return typeof path === "string" && path.length > 0 ? path : undefined;
+}
+
+// callId → tool name/path for readable stubs. Inverse of path-to-reads.
+function buildCallIndex(turns: readonly ConversationTurn[]): Map<string, ToolCallInfo> {
+  const index = new Map<string, ToolCallInfo>();
+  for (const turn of turns) {
+    for (const block of turn.content) {
+      if (block.type !== "tool_call") continue;
+      const info: ToolCallInfo = { name: block.name };
+      const path = pathArgFromArguments(block.arguments);
+      if (path !== undefined) info.pathArg = path;
+      index.set(block.id, info);
+    }
+  }
+  return index;
+}
+
+/**
+ * Path → every read_file result that targeted it, in session order.
+ * Groups repeated reads so older successful results can be stubbed when a
+ * later read of the same path survives compaction.
+ */
+function buildPathToReads(
+  turns: readonly ConversationTurn[],
+  callIndex: ReadonlyMap<string, ToolCallInfo>,
+): Map<string, PathRead[]> {
+  const pathToReads = new Map<string, PathRead[]>();
+  let order = 0;
+  for (const turn of turns) {
+    for (const block of turn.content) {
+      if (block.type !== "tool_result") continue;
+      const info = callIndex.get(block.callId);
+      if (info === undefined || !READ_TOOLS.has(info.name) || info.pathArg === undefined) continue;
+      const entry: PathRead = {
+        callId: block.callId,
+        order: order++,
+        isError: block.isError === true,
+      };
+      const list = pathToReads.get(info.pathArg);
+      if (list === undefined) pathToReads.set(info.pathArg, [entry]);
+      else list.push(entry);
+    }
+  }
+  return pathToReads;
+}
+
+/**
+ * Call ids of successful read_file results that are superseded by a later
+ * successful read of the same path. Error results never appear here — they
+ * stay verbatim so the model still sees the failure.
+ */
+function supersededReadCallIds(pathToReads: ReadonlyMap<string, PathRead[]>): Set<string> {
+  const superseded = new Set<string>();
+  for (const reads of pathToReads.values()) {
+    const successes = reads.filter((r) => !r.isError);
+    if (successes.length < 2) continue;
+    // Newest success (highest order) stays whole; every earlier success stubs.
+    for (let i = 0; i < successes.length - 1; i++) {
+      superseded.add(successes[i]!.callId);
+    }
+  }
+  return superseded;
+}
+
 // Locate the turn index of each tool_call and its matching tool_result. In this
 // runtime a call lives on one turn and its result on the following turn, so the
 // two halves of a pair can straddle a keep/summarize boundary.
@@ -277,6 +370,43 @@ function resultContentSize(block: Extract<ConversationTurn["content"][number], {
   return block.content.reduce((sum, c) => sum + (c.type === "text" ? c.text.length : 0), 0);
 }
 
+function buildResultStub(
+  block: Extract<ConversationTurn["content"][number], { type: "tool_result" }>,
+  callIndex: ReadonlyMap<string, ToolCallInfo>,
+): string {
+  const info = callIndex.get(block.callId);
+  const name = info?.name ?? "tool_result";
+  const size = resultContentSize(block);
+  if (info?.pathArg !== undefined) {
+    const path = info.pathArg;
+    const spillHint =
+      path.startsWith("tool-output://")
+        ? " Re-read with read_file offset/limit or grep on that URI."
+        : "";
+    return `[${name} ${path} — ${size} chars omitted from context; source unchanged.${spillHint}]`;
+  }
+  return `[${name} — ${size} chars, omitted]`;
+}
+
+// Hollow out superseded successful read_file results; leave everything else.
+// Errors and the newest successful read of each path stay whole.
+function stubSupersededReads(
+  turn: ConversationTurn,
+  superseded: ReadonlySet<string>,
+  callIndex: ReadonlyMap<string, ToolCallInfo>,
+): ConversationTurn {
+  if (superseded.size === 0) return turn;
+  let changed = false;
+  const content = turn.content.map((block): ConversationTurn["content"][number] => {
+    if (block.type !== "tool_result" || !superseded.has(block.callId)) return block;
+    // Defensive: errors never enter the superseded set, but keep them whole.
+    if (block.isError === true) return block;
+    changed = true;
+    return { ...block, content: [{ type: "text", text: buildResultStub(block, callIndex) }] };
+  });
+  return changed ? { ...turn, content } : turn;
+}
+
 // True when a turn carries no tool_call/tool_result blocks.
 function isPlainTextTurn(turn: ConversationTurn): boolean {
   return !turn.content.some((b) => b.type === "tool_call" || b.type === "tool_result");
@@ -364,7 +494,7 @@ export function createPruningCompactor(
 
   return {
     name: "pruning-compactor",
-    version: "1.2.0",
+    version: "1.3.0",
     async apply(
       turns: ConversationTurn[],
       _ctx: StrategyContext,
@@ -390,6 +520,13 @@ export function createPruningCompactor(
         };
       }
 
+      // callId → name/path for stubs; path → ordered reads for re-read dedup.
+      // Only older successful reads of a path re-read later are stubbed — not a
+      // blanket strip of every kept tool_result (see CL-5595 / CL-4374).
+      const callIndex = buildCallIndex(aged.turns);
+      const pathToReads = buildPathToReads(aged.turns, callIndex);
+      const supersededReads = supersededReadCallIds(pathToReads);
+
       const keepCount = Math.min(cfg.keepRecentTurns, aged.turns.length - 1);
       const keepFrom = aged.turns.length - keepCount;
       const recentTurns = aged.turns.slice(keepFrom);
@@ -446,17 +583,18 @@ export function createPruningCompactor(
         timestamp: olderTurns[olderTurns.length - 1]?.timestamp ?? Date.now(),
       };
 
-      // Anchors and recent turns are exactly what compaction chose to keep —
-      // pulling a turn forward and then hollowing out its tool_result defeats
-      // the reason it was kept. Only summarizedTurns lose their content, and
-      // they lose it wholesale (folded into `summary` above), not stubbed
-      // in place. Anchors are already image-aged (outside the recent window).
-      // Recent turns keep live base64 so a just-pasted screenshot still
-      // reaches the model.
+      // Anchors and recent turns stay contentful except for path-dedup: when the
+      // same file was read successfully more than once, older results become a
+      // one-line stub and the newest stays whole. Error results are never
+      // stubbed. SummarizedTurns lose content wholesale via the summary above.
+      // Anchors are already image-aged (outside the recent window). Recent turns
+      // keep live base64 so a just-pasted screenshot still reaches the model.
+      const process = (t: ConversationTurn): ConversationTurn =>
+        stubSupersededReads(t, supersededReads, callIndex);
       const output = coalesceAdjacentTextTurns([
         summaryTurn,
-        ...anchorTurns,
-        ...recentTurns,
+        ...anchorTurns.map(process),
+        ...recentTurns.map(process),
       ]);
 
       return {
@@ -476,6 +614,7 @@ export function createPruningCompactor(
             recentTurnCount: recentTurns.length,
             summaryLength: summary.length,
             agedImageCount: aged.agedImageCount,
+            supersededReadCount: supersededReads.size,
           },
         },
         ...(aged.blobs.length > 0 ? { blobs: aged.blobs } : {}),

tests/unit/compactor-pairing.test.ts

@@ -98,3 +98,98 @@ describe("pruning compactor preserves tool_call/tool_result pairing", () => {
     expect(tokens).toBeGreaterThan(20000);
   });
 });
+
+// CL-4374: when the same path is read more than once and both results survive
+// compaction (recent window / anchors), older successful reads become one-line
+// stubs; the newest successful read stays whole; error results stay whole.
+function assistantRead(id: string, path: string): ConversationTurn {
+  return {
+    role: "assistant",
+    content: [{ type: "tool_call", id, name: "read_file", arguments: { path } }],
+    timestamp: 1,
+  };
+}
+
+function userReadResult(callId: string, body: string, isError = false): ConversationTurn {
+  return {
+    role: "user",
+    content: [
+      {
+        type: "tool_result",
+        callId,
+        content: [{ type: "text", text: body }],
+        ...(isError ? { isError: true } : {}),
+      },
+    ],
+    timestamp: 1,
+  };
+}
+
+function resultText(output: ConversationTurn[], callId: string): string | undefined {
+  for (const turn of output) {
+    for (const block of turn.content) {
+      if (block.type === "tool_result" && block.callId === callId) {
+        return block.content
+          .filter((c): c is { type: "text"; text: string } => c.type === "text")
+          .map((c) => c.text)
+          .join("");
+      }
+    }
+  }
+  return undefined;
+}
+
+describe("pruning compactor stubs superseded file reads (CL-4374)", () => {
+  test("stubs an older successful read of a path re-read later; newest stays whole", async () => {
+    const oldBody = "OLD_CONTENT_" + "a".repeat(200);
+    const newBody = "NEW_CONTENT_" + "b".repeat(200);
+    // Both read pairs sit in the recent window so neither is folded into the
+    // summary — only the path-dedup pass should hollow the older result.
+    const turns: ConversationTurn[] = [
+      userText("start"),
+      userText("a"),
+      userText("b"),
+      userText("c"),
+      assistantRead("r1", "src/hot.ts"),
+      userReadResult("r1", oldBody),
+      assistantRead("r2", "src/hot.ts"),
+      userReadResult("r2", newBody),
+      userText("d"),
+      userText("e"),
+    ];
+    const compactor = createPruningCompactor({ keepRecentTurns: 6, maxAnchorTurns: 2 });
+    const { output } = await compactor.apply(turns, {} as never);
+
+    const older = resultText(output, "r1");
+    const newer = resultText(output, "r2");
+    expect(older).toBeDefined();
+    expect(newer).toBe(newBody);
+    expect(older).not.toBe(oldBody);
+    expect(older).toMatch(/read_file/);
+    expect(older).toMatch(/src\/hot\.ts/);
+    expect(older).toMatch(/omitted|chars/);
+    expect(older!.length).toBeLessThan(oldBody.length);
+  });
+
+  test("preserves error read results verbatim even when a later success supersedes the path", async () => {
+    const errBody = "Error: ENOENT no such file " + "e".repeat(80);
+    const okBody = "OK_CONTENT_" + "c".repeat(200);
+    const turns: ConversationTurn[] = [
+      userText("start"),
+      userText("a"),
+      userText("b"),
+      userText("c"),
+      assistantRead("r1", "src/hot.ts"),
+      userReadResult("r1", errBody, true),
+      assistantRead("r2", "src/hot.ts"),
+      userReadResult("r2", okBody),
+      userText("d"),
+      userText("e"),
+    ];
+    const compactor = createPruningCompactor({ keepRecentTurns: 6, maxAnchorTurns: 2 });
+    const { output } = await compactor.apply(turns, {} as never);
+
+    expect(resultText(output, "r1")).toBe(errBody);
+    expect(resultText(output, "r2")).toBe(okBody);
+  });
+});