mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-28 13:26:04 -06:00
fix(compaction): previous summary is duplicated when a later split degrades (#109828)
* 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 <steipete@gmail.com>
This commit is contained in:
@@ -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 () => {
|
||||
|
||||
@@ -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") }),
|
||||
]),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user