fix(compaction): gate frame-rescue success on real headroom and cap rescue frames
Review follow-ups (Codex on #6362): - The !preparation frame rescue now counts as complete only when the rebuild actually created headroom; otherwise the elide/image tiers still run and the no-progress warning stays — a frame-count shrink alone must not suppress it when the oversized tail is a kept message/tool result the archive rescue cannot touch. - #computeSnapcompactRescueMaxFrames now applies the same MAX_FRAMES_DEFAULT / maxFramesForDataBudget caps as #computeSnapcompactMaxFrames, so a threshold-derived count can never exceed what the rebuilt prompt can attach. Claude-Session: https://claude.ai/code/session_014rh4JyWFkxgMhgFaEf8VBY
This commit is contained in:
@@ -13868,7 +13868,17 @@ export class AgentSession {
|
||||
const SUMMARY_TEMPLATE_TOKENS = 2000;
|
||||
const frameBudget = recoveryBandTokens - baseTokens - textEdgeTokens - SUMMARY_TEMPLATE_TOKENS;
|
||||
if (frameBudget < snapcompact.FRAME_TOKEN_ESTIMATE) return 1;
|
||||
return Math.max(1, Math.floor(frameBudget / snapcompact.FRAME_TOKEN_ESTIMATE));
|
||||
// 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(),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -14178,15 +14188,18 @@ export class AgentSession {
|
||||
// strategy pass (it tried and found nothing); skip entirely on the
|
||||
// idle timer (it re-checks usage on its own cadence).
|
||||
let rescueRewroteHistory = false;
|
||||
// A trailing snapcompact CompactionEntry is invisible to both rescue
|
||||
// tiers below (they only inspect message entries) and to
|
||||
// prepareCompaction itself (last-entry-is-compaction guard), so a
|
||||
// frame archive billed past the threshold dead-ends here on every
|
||||
// resume. Rebuild it at a threshold-derived frame budget first; when
|
||||
// that lands, the branch is already fully compacted — there is
|
||||
// nothing left to summarize, so skip the elide/image tiers (provably
|
||||
// no-ops on this shape) and the no-progress warning entirely.
|
||||
// A snapcompact CompactionEntry is invisible to both rescue tiers
|
||||
// below (they only inspect message entries) and to prepareCompaction
|
||||
// itself (last-entry-is-compaction guard), so a frame archive billed
|
||||
// past the threshold dead-ends here on every resume. Rebuild it at a
|
||||
// threshold-derived frame budget first — but treat that as complete
|
||||
// only when it actually created headroom: the latest archive may not
|
||||
// be the oversized tail (e.g. a huge kept tool result after it), and
|
||||
// declaring victory on a mere frame-count shrink would skip the
|
||||
// elide/image tiers that can still reach that tail and suppress a
|
||||
// warning the user should see.
|
||||
let frameOverflowRescued = false;
|
||||
let frameRescueCreatedHeadroom = false;
|
||||
if (reason !== "idle") {
|
||||
frameOverflowRescued = await this.#rescueSnapcompactFrameOverflow(
|
||||
pathEntriesForCompaction,
|
||||
@@ -14196,7 +14209,9 @@ export class AgentSession {
|
||||
if (frameOverflowRescued) {
|
||||
rescueRewroteHistory = true;
|
||||
pathEntriesForCompaction = this.sessionManager.getBranch();
|
||||
} else {
|
||||
frameRescueCreatedHeadroom = this.#compactionCreatedHeadroom();
|
||||
}
|
||||
if (!frameRescueCreatedHeadroom) {
|
||||
await this.#rescueCompactionDeadEnd(autoCompactionSignal, {
|
||||
skipElide: fallbackFromShake,
|
||||
hasProgress: () => {
|
||||
@@ -14223,12 +14238,12 @@ export class AgentSession {
|
||||
willRetry: false,
|
||||
skipped: true,
|
||||
});
|
||||
const noProgressDeadEnd = reason !== "idle" && !frameOverflowRescued;
|
||||
const noProgressDeadEnd = reason !== "idle" && !frameRescueCreatedHeadroom;
|
||||
let continuationScheduled = false;
|
||||
if (frameOverflowRescued) {
|
||||
if (frameRescueCreatedHeadroom) {
|
||||
continuationScheduled = this.#scheduleCompactionContinuation({
|
||||
generation,
|
||||
autoContinue: shouldAutoContinue && this.#compactionCreatedHeadroom(),
|
||||
autoContinue: shouldAutoContinue,
|
||||
terminalTextAnswer,
|
||||
suppressContinuation,
|
||||
});
|
||||
|
||||
@@ -228,17 +228,27 @@ describe("AgentSession snapcompact frame dead-end rescue", () => {
|
||||
await createSession({ frameCount: SEEDED_FRAME_COUNT });
|
||||
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 });
|
||||
// Over the band until the rescue rebuilds the archive, then well under —
|
||||
// the rescue only counts as complete when it creates real headroom.
|
||||
let rebuiltArchiveApplied = false;
|
||||
vi.spyOn(session, "getContextUsage").mockImplementation(() =>
|
||||
rebuiltArchiveApplied
|
||||
? { tokens: 30000, contextWindow: 200000, percent: 15 }
|
||||
: { 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").mockResolvedValue({
|
||||
summary: "Rebuilt archive at a smaller frame budget.",
|
||||
shortSummary: "rebuilt snapcompact archive",
|
||||
firstKeptEntryId: (sessionManager.getBranch()[0] as { id: string }).id,
|
||||
tokensBefore: 150_000,
|
||||
details: { readFiles: ["src/a.ts"], modifiedFiles: ["src/b.ts"] },
|
||||
preserveData: makeArchivePreserveData(4),
|
||||
const compactSpy = vi.spyOn(snapcompact, "compact").mockImplementation(async () => {
|
||||
rebuiltArchiveApplied = true;
|
||||
return {
|
||||
summary: "Rebuilt archive at a smaller frame budget.",
|
||||
shortSummary: "rebuilt snapcompact archive",
|
||||
firstKeptEntryId: (sessionManager.getBranch()[0] as { id: string }).id,
|
||||
tokensBefore: 150_000,
|
||||
details: { readFiles: ["src/a.ts"], modifiedFiles: ["src/b.ts"] },
|
||||
preserveData: makeArchivePreserveData(4),
|
||||
};
|
||||
});
|
||||
|
||||
const notices = collectNotices();
|
||||
@@ -315,6 +325,37 @@ describe("AgentSession snapcompact frame dead-end rescue", () => {
|
||||
expect(recovery.length).toBe(1);
|
||||
});
|
||||
|
||||
it("still runs the elide tiers and warns when the frame rebuild frees too little", async () => {
|
||||
// Codex review on #6362: the latest archive may not be the oversized
|
||||
// tail (e.g. a huge kept tool result sits after it). A frame-count
|
||||
// shrink alone must NOT count as success — the elide/image tiers still
|
||||
// get their shot at the real tail, and the no-progress warning stays.
|
||||
await createSession({ frameCount: SEEDED_FRAME_COUNT });
|
||||
vi.spyOn(session.agent, "prompt").mockResolvedValue(undefined as never);
|
||||
vi.spyOn(session.agent, "continue").mockResolvedValue();
|
||||
// Usage stays over the band even after the rebuild.
|
||||
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 });
|
||||
vi.spyOn(snapcompact, "compact").mockResolvedValue({
|
||||
summary: "Rebuilt archive at a smaller frame budget.",
|
||||
shortSummary: "rebuilt snapcompact archive",
|
||||
firstKeptEntryId: (sessionManager.getBranch()[0] as { id: string }).id,
|
||||
tokensBefore: 150_000,
|
||||
details: { readFiles: [], modifiedFiles: [] },
|
||||
preserveData: makeArchivePreserveData(4),
|
||||
});
|
||||
|
||||
const notices = collectNotices();
|
||||
await triggerMaintenance();
|
||||
|
||||
expect(shakeSpy).toHaveBeenCalledWith("elide", expect.anything());
|
||||
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");
|
||||
});
|
||||
|
||||
it("still warns once when the trailing archive is already at the minimum frame count", async () => {
|
||||
await createSession({ frameCount: 1 });
|
||||
const promptSpy = vi.spyOn(session.agent, "prompt").mockResolvedValue(undefined as never);
|
||||
|
||||
Reference in New Issue
Block a user