fix(compaction): previous summary is duplicated when a later split degrades (#109828)

Yigtwxx · steipete · web-flow · commit 12e7f91047e6 · 2026-07-17T17:50:22.000+01:00
* fix(compaction): stop duplicating the previous summary when a later split degrades

summarizeInStages stamped a merged result as generic-fallback whenever ANY
split degraded. compaction-safeguard reads that kind to decide whether to
restore the previous summary, on the documented assumption that "a generic
fallback means redistillation never happened".

That assumption only holds for chunk 0. summarizeViaLLM prepends the previous
summary as the first message, so it always lands in the oldest chunk. When
that chunk summarizes fine but a later one degrades, the merge already carries
the redistilled summary — and the safeguard prepends it a second time. Every
subsequent compaction that has any degraded split re-adds it again, so a PR
that exists to stop token burn compounds it instead.

The kind now reports whether the oldest split degraded, which is exactly the
question the only consumer asks. The coarse "any chunk" flag is gone rather
than kept alongside the precise one, so there is a single source of truth.
Behavior for a degraded oldest split is unchanged: the previous summary is
genuinely lost there and restoring it is still correct.

* test(compaction): pin previous-summary restoration boundary

---------

Co-authored-by: Peter Steinberger &lt;steipete@gmail.com&gt;
diff --git a/src/agents/agent-hooks/compaction-safeguard.test.ts b/src/agents/agent-hooks/compaction-safeguard.test.ts
@@ -2021,6 +2021,7 @@ describe("compaction-safeguard recent-turn preservation", () => {
     const messages = requireArray(call.messages);
     expect(JSON.stringify(messages[0])).toContain("<previous-compaction-summary>");
     expect(JSON.stringify(messages[0])).toContain("Old duplicated section");
+    expect(result.compaction?.summary).not.toContain("Old duplicated section");
   });
 
   it("preserves the prior summary when staged summarization returns a generic fallback", async () => {
diff --git a/src/agents/compaction.circuit-breaker.test.ts b/src/agents/compaction.circuit-breaker.test.ts
@@ -79,4 +79,25 @@ describe("compaction staged fallback circuit breaker", () => {
     });
     expect(agentSessionMocks.generateSummary).toHaveBeenCalledTimes(4);
   });
+
+  it("reports a summary when only a later split degraded and the oldest one survived", async () => {
+    agentSessionMocks.generateSummary
+      .mockResolvedValueOnce("oldest summary")
+      .mockRejectedValueOnce(new Error("fetch failed"))
+      .mockResolvedValueOnce("newest summary")
+      .mockResolvedValueOnce("merged: oldest summary + newest summary");
+
+    // The oldest split carries whatever context the caller needs redistilled, and it
+    // made it into the merge. Reporting a fallback here makes callers re-add it.
+    await expect(summarize()).resolves.toEqual({
+      kind: "summary",
+      text: "merged: oldest summary + newest summary",
+    });
+    expect(agentSessionMocks.generateSummary).toHaveBeenCalledTimes(4);
+    expect(agentSessionMocks.generateSummary.mock.calls[3]?.[0]).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({ content: expect.stringContaining("oldest summary") }),
+      ]),
+    );
+  });
 });
diff --git a/src/agents/compaction.ts b/src/agents/compaction.ts
@@ -413,7 +413,9 @@ export async function summarizeInStages(params: {
 
   const partialSummaries: string[] = [];
   let consecutiveGenericFallbacks = 0;
-  let usedGenericFallback = false;
+  // Caller-owned leading context lives in the oldest split. Only losing that
+  // split requires restoration; later fallback placeholders remain in the merge.
+  let oldestChunkDegraded = false;
   for (const [index, chunk] of plan.chunks.entries()) {
     const result = await summarizeWithFallbackResult({
       ...params,
@@ -422,7 +424,9 @@ export async function summarizeInStages(params: {
     });
     consecutiveGenericFallbacks =
       result.kind === "generic-fallback" ? consecutiveGenericFallbacks + 1 : 0;
-    usedGenericFallback ||= result.kind === "generic-fallback";
+    if (index === 0) {
+      oldestChunkDegraded = result.kind === "generic-fallback";
+    }
 
     // Keep one placeholder to mark the missing split, but stop before repeated
     // placeholders trigger more split requests or a doomed merge request.
@@ -445,7 +449,7 @@ export async function summarizeInStages(params: {
       throw new Error("Compaction summary plan produced no summary");
     }
     return {
-      kind: usedGenericFallback ? "generic-fallback" : "summary",
+      kind: oldestChunkDegraded ? "generic-fallback" : "summary",
       text: summary,
     };
   }
@@ -486,7 +490,7 @@ export async function summarizeInStages(params: {
     messages: summaryMessages,
     customInstructions: mergeInstructions,
   });
-  return usedGenericFallback && mergedResult.kind === "summary"
+  return oldestChunkDegraded && mergedResult.kind === "summary"
     ? { kind: "generic-fallback", text: mergedResult.text }
     : mergedResult;
 }
