From d8554c92d09db8ffa5f4ddb139edf4758db26f5d Mon Sep 17 00:00:00 2001 From: Hugo Lopes <4919100+HugoLopes45@users.noreply.github.com> Date: Thu, 23 Jul 2026 14:03:12 +0200 Subject: [PATCH] fix(compaction): charge the kept tail in the rescue budget and badge the active entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups (Codex on #6362, round 5): - #computeSnapcompactRescueMaxFrames now subtracts the kept tail AFTER the archive (plus the existing fixed-context reserves) so the budget mirrors what #compactionCreatedHeadroom will measure, and returns 0 when not even one frame fits — the rescue bails instead of appending a rebuild that can never create headroom (and would wedge prepareCompaction behind its last-entry guard once elide fixes the real tail). - Dead-end warnings now stamp the branch's LATEST compaction entry: the post-pass path no longer badges the entry the rescue just superseded, and the no-preparation path badges the rebuilt entry when the rescue appended without creating headroom. Claude-Session: https://claude.ai/code/session_014rh4JyWFkxgMhgFaEf8VBY --- .../coding-agent/src/session/agent-session.ts | 90 +++++++++++-------- ...session-snapcompact-frame-dead-end.test.ts | 38 ++++++++ 2 files changed, 89 insertions(+), 39 deletions(-) diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 300343bd4..a163abedf 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -13854,9 +13854,13 @@ export class AgentSession { * {@link #computeSnapcompactMaxFrames} sizes against — a rebuilt archive * must land back under the maintenance trigger, or the next settle * re-enters the same dead-end. Cap reserve mirrors - * #computeSnapcompactMaxFrames (text edges + summary template). + * #computeSnapcompactMaxFrames (text edges + summary template), and + * `keptTailTokens` charges the kept entries AFTER the archive so the + * budget mirrors what #compactionCreatedHeadroom will actually measure. + * Returns 0 when not even one frame fits that budget — the rebuild could + * never create headroom, so the caller must not append it. */ - #computeSnapcompactRescueMaxFrames(settings: CompactionSettings): number { + #computeSnapcompactRescueMaxFrames(settings: CompactionSettings, keptTailTokens: number): number { const ctxWindow = this.model?.contextWindow ?? 0; if (ctxWindow <= 0) return Math.min(snapcompact.MAX_FRAMES_DEFAULT, snapcompact.maxFramesForDataBudget()); const thresholdTokens = resolveThresholdTokens(ctxWindow, settings); @@ -13866,18 +13870,15 @@ export class AgentSession { const edgeCap = snapcompact.geometry(shape).capacity; const textEdgeTokens = Math.ceil((2 * edgeCap * 1.15) / 4); const SUMMARY_TEMPLATE_TOKENS = 2000; - const frameBudget = recoveryBandTokens - baseTokens - textEdgeTokens - SUMMARY_TEMPLATE_TOKENS; - if (frameBudget < snapcompact.FRAME_TOKEN_ESTIMATE) return 1; + const frameBudget = recoveryBandTokens - baseTokens - keptTailTokens - textEdgeTokens - SUMMARY_TEMPLATE_TOKENS; + if (frameBudget < snapcompact.FRAME_TOKEN_ESTIMATE) return 0; // Same hard caps as #computeSnapcompactMaxFrames: a threshold-derived // count above the per-request payload budget would "shrink" a huge // archive to a frame count the rebuilt prompt can never attach anyway. - return Math.max( - 1, - Math.min( - Math.floor(frameBudget / snapcompact.FRAME_TOKEN_ESTIMATE), - snapcompact.MAX_FRAMES_DEFAULT, - snapcompact.maxFramesForDataBudget(), - ), + return Math.min( + Math.floor(frameBudget / snapcompact.FRAME_TOKEN_ESTIMATE), + snapcompact.MAX_FRAMES_DEFAULT, + snapcompact.maxFramesForDataBudget(), ); } @@ -13913,30 +13914,27 @@ export class AgentSession { const staleEntry = getLatestCompactionEntry(branchEntries); if (!staleEntry) return undefined; // Only rescue when the archive is the actual source of the overflow. - // If the kept tail AFTER it is itself over the recovery band (e.g. a - // huge kept tool result), rebuilding the archive would append the - // replacement compaction at the leaf — turning the branch tail into a - // compaction entry, which prepareCompaction's last-entry guard can - // never summarize past even after an elide shrinks the real culprit. - // Bail and let the elide/image tiers handle that tail instead. - const ctxWindow = this.model.contextWindow ?? 0; - if (ctxWindow > 0) { - const tailBar = Math.floor(resolveThresholdTokens(ctxWindow, settings) * COMPACTION_RECOVERY_BAND); - let tailTokens = 0; - for (let i = branchEntries.length - 1; i >= 0; i--) { - const entry = branchEntries[i]; - if (entry.id === staleEntry.id) break; - const message = (entry as { message?: AgentMessage }).message; - if (message) tailTokens += estimateTokens(message); - } - if (tailTokens > tailBar) return undefined; + // The frame budget below charges the kept tail AFTER the archive plus + // the fixed context, mirroring what #compactionCreatedHeadroom will + // measure. When not even one frame fits (e.g. a huge kept tool result + // dominates), rebuilding would append the replacement compaction at + // the leaf — turning the branch tail into a compaction entry, which + // prepareCompaction's last-entry guard can never summarize past even + // after an elide shrinks the real culprit. Bail and let the + // elide/image tiers handle that tail instead. + let keptTailTokens = 0; + for (let i = branchEntries.length - 1; i >= 0; i--) { + const entry = branchEntries[i]; + if (entry.id === staleEntry.id) break; + const message = (entry as { message?: AgentMessage }).message; + if (message) keptTailTokens += estimateTokens(message); } const archive = snapcompact.getPreservedArchive(staleEntry.preserveData); if (!archive || archive.frames.length <= 1) return undefined; const archiveText = snapcompact.archiveSourceText(archive); if (!archiveText) return undefined; - const maxFrames = this.#computeSnapcompactRescueMaxFrames(settings); - if (maxFrames >= archive.frames.length) return undefined; + const maxFrames = this.#computeSnapcompactRescueMaxFrames(settings, keptTailTokens); + if (maxFrames < 1 || maxFrames >= archive.frames.length) return undefined; const staleDetails = staleEntry.details as snapcompact.CompactionDetails | undefined; const fileOps = snapcompact.createFileOps(); @@ -14298,11 +14296,19 @@ export class AgentSession { continuationScheduled = true; } if (noProgressDeadEnd) { - this.emitNotice( - "warning", - compactionDeadEndWarning("shrink it (e.g. clear large tool output)"), - "compaction", - ); + const deadEndWarning = compactionDeadEndWarning("shrink it (e.g. clear large tool output)"); + this.emitNotice("warning", deadEndWarning, "compaction"); + // A rescue that appended a rebuilt archive without creating + // headroom must carry the dead-end badge on the entry the + // transcript actually shows (the rebuilt one), or the pause + // loses its explanation once the notice scrolls away. + if (frameRescueResult) { + const stampEntry = getLatestCompactionEntry(this.sessionManager.getBranch()); + if (stampEntry) { + stampEntry.warning = deadEndWarning; + await this.sessionManager.rewriteEntries(); + } + } } // A rescue that offloaded content but still could not produce a // preparation rewrote the branch; flag it so the overflow-recovery @@ -14723,12 +14729,18 @@ export class AgentSession { } const deadEndWarning = noProgressDeadEnd ? compactionDeadEndWarning("clear large tool output") : undefined; - if (deadEndWarning && savedCompactionEntry) { + if (deadEndWarning) { // Stamp the divider: the compaction bar badges the dead-end and // carries the full warning in its ctrl+o detail, so the pause - // stays explained even after the notice row scrolls away. - savedCompactionEntry.warning = deadEndWarning; - await this.sessionManager.rewriteEntries(); + // stays explained even after the notice row scrolls away. Stamp + // the branch's LATEST compaction entry — a frame rescue may have + // superseded `savedCompactionEntry` with a rebuilt one, and the + // collapsed transcript badges only the active entry. + const stampEntry = getLatestCompactionEntry(this.sessionManager.getBranch()) ?? savedCompactionEntry; + if (stampEntry) { + stampEntry.warning = deadEndWarning; + await this.sessionManager.rewriteEntries(); + } } await this.#emitSessionEvent({ type: "auto_compaction_end", action, result, aborted: false, willRetry }); diff --git a/packages/coding-agent/test/agent-session-snapcompact-frame-dead-end.test.ts b/packages/coding-agent/test/agent-session-snapcompact-frame-dead-end.test.ts index 9d2b6d53e..e9af615c4 100644 --- a/packages/coding-agent/test/agent-session-snapcompact-frame-dead-end.test.ts +++ b/packages/coding-agent/test/agent-session-snapcompact-frame-dead-end.test.ts @@ -373,6 +373,44 @@ describe("AgentSession snapcompact frame dead-end rescue", () => { const noProgress = notices.filter(n => n.source === NOTICE_SOURCE && n.message.includes(NO_PROGRESS_FRAGMENT)); expect(noProgress.length).toBe(1); expect(noProgress[0].level).toBe("warning"); + // The dead-end badge must live on the ACTIVE (rebuilt) entry — the + // collapsed transcript only shows the latest compaction divider. + const compactions = sessionManager.getBranch().filter(e => e.type === "compaction") as CompactionEntry[]; + const active = compactions.at(-1); + expect(snapcompact.getPreservedArchive(active?.preserveData)?.frames.length).toBe(4); + expect(active?.warning).toContain(NO_PROGRESS_FRAGMENT); + }); + + it("bails when the kept tail plus fixed context leaves no frame budget", async () => { + // Codex review on #6362 (round 5): a tail just under the recovery band + // still cannot coexist with the fixed context + a minimum rebuilt + // archive. The budget now charges the kept tail like + // #compactionCreatedHeadroom does, so the rescue must bail instead of + // appending a rebuild that can never create headroom. + await createSession({ frameCount: SEEDED_FRAME_COUNT }); + // ~40k estimated tokens: under the 48k band, but over band − edges/template. + sessionManager.appendMessage({ + role: "toolResult", + toolCallId: "call-mid", + toolName: "bash", + content: [{ type: "text", text: "y".repeat(160_000) }], + isError: false, + timestamp: Date.now(), + }); + vi.spyOn(compactionModule, "prepareCompaction").mockReturnValue(undefined); + vi.spyOn(session.agent, "prompt").mockResolvedValue(undefined as never); + vi.spyOn(session.agent, "continue").mockResolvedValue(); + vi.spyOn(session, "getContextUsage").mockReturnValue({ tokens: 190000, contextWindow: 200000, percent: 95 }); + const shakeSpy = vi + .spyOn(session, "shake") + .mockResolvedValue({ mode: "elide", toolResultsDropped: 0, blocksDropped: 0, tokensFreed: 0 }); + const compactSpy = vi.spyOn(snapcompact, "compact"); + + await triggerMaintenance(); + + expect(compactSpy).not.toHaveBeenCalled(); + expect(shakeSpy).toHaveBeenCalledWith("elide", expect.anything()); + expect(sessionManager.getBranch().at(-1)?.type).not.toBe("compaction"); }); it("leaves an oversized non-archive tail to the elide tiers instead of rescuing the archive", async () => {