From 5cce507582c49eb01a9c5e01d993c88c9972bf92 Mon Sep 17 00:00:00 2001 From: roboomp Date: Mon, 22 Jun 2026 10:03:44 +0000 Subject: [PATCH] fix(agent): size snapcompact maxFrames by the live model window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Snapcompact's bundled MAX_FRAMES_DEFAULT (80) × FRAME_TOKEN_ESTIMATE (5024) ≈ 402k tokens worth of frames. AgentSession was calling snapcompact.compact() with no maxFrames override, so the post-render projection inside #runAuto Compaction / compact() always overflowed the budget on any sub-1M-token window (Claude Sonnet 4.5's 200k = 170k usable, the 80-frame projection alone clears that 2.4×), looping the 'snapcompact could not bring the context under the limit — using an LLM summary instead' warning on every threshold tick. AgentSession.#computeSnapcompactMaxFrames now sizes the frame cap from the resolved budget — (window − reserve − non-message − kept-recent − summary-text reserve) / FRAME_TOKEN_ESTIMATE, clamped to MAX_FRAMES_DEFAULT — and threads it into snapcompact.compact() in both the auto-compaction and manual /compact paths. When the kept-recent slice already exceeds the budget, snapcompact is skipped outright instead of running just to be rejected: the projection guard remains as a defensive check. Fixes #3247 --- packages/coding-agent/CHANGELOG.md | 4 + .../coding-agent/src/session/agent-session.ts | 125 ++++++++--- .../agent-session-snapcompact-budget.test.ts | 198 ++++++++++++++++++ 3 files changed, 297 insertions(+), 30 deletions(-) create mode 100644 packages/coding-agent/test/agent-session-snapcompact-budget.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 2e70c2bd9..6aa2c35a1 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed snapcompact auto-compaction looping the "snapcompact could not bring the context under the limit — using an LLM summary instead" warning on every threshold tick for sub-1M-token models (Claude Sonnet 4.5, GPT-5.x, Gemini 2.x). `snapcompact.compact()` was called with no `maxFrames` override, so it defaulted to `MAX_FRAMES_DEFAULT = 80`; the projection in `AgentSession` charges `FRAME_TOKEN_ESTIMATE = 5024` per frame block (the conservative high-res Anthropic ceiling), making 80 × 5024 ≈ 402k frame-token projections that always overflow a 200k budget. `AgentSession.#computeSnapcompactMaxFrames` now sizes the frame cap from the live `(window − reserve − non-message − kept-recent − summary-text reserve)` envelope before invoking snapcompact in both the auto-compaction and manual `/compact` paths, and skips snapcompact outright when the kept-recent slice alone already exceeds the budget (with a clearer notice explaining the cause). ([#3247](https://github.com/can1357/oh-my-pi/issues/3247)) + ## [16.1.14] - 2026-06-22 ### Added diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index b25b30507..5f9e2c874 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -44,6 +44,7 @@ import { CompactionCancelledError, type CompactionPreparation, type CompactionResult, + type CompactionSettings, calculateContextTokens, calculatePromptTokens, collectEntriesForBranchSummary, @@ -7797,31 +7798,45 @@ export class AgentSession { let tokensBefore: number; let details: unknown; - // Snapcompact runs locally first; if its frame archive plus the kept - // history still overflows the model window, fall back to an LLM summary - // (far cheaper than ~FRAME_TOKEN_ESTIMATE per frame). + // Snapcompact runs locally first. The frame cap is sized from the live + // model window via #computeSnapcompactMaxFrames so the post-render context + // fits without the warning loop (issue #3247). Zero-frame budget → skip + // snapcompact and take the summarizer path immediately. let snapcompactResult: snapcompact.CompactionResult | undefined; if (snapcompactReady) { - snapcompactResult = await snapcompact.compact(preparation, { - convertToLlm, - model: this.model, - shape: snapcompact.resolveShape(this.model, this.settings.get("snapcompact.shape")), - }); - const ctxWindow = this.model?.contextWindow ?? 0; - const budget = - ctxWindow > 0 - ? ctxWindow - effectiveReserveTokens(ctxWindow, effectiveSettings) - : Number.POSITIVE_INFINITY; - if (this.#projectSnapcompactContextTokens(preparation, snapcompactResult) > budget) { - logger.warn("Snapcompact still overflows the window; falling back to an LLM summary", { + const maxFrames = this.#computeSnapcompactMaxFrames(preparation, effectiveSettings); + if (maxFrames < 1) { + logger.warn("Snapcompact skipped: kept history alone exceeds the context budget", { model: this.model?.id, }); this.emitNotice( "warning", - "snapcompact could not bring the context under the limit — using an LLM summary instead", + "snapcompact: kept history alone exceeds the context budget — using an LLM summary instead", "compaction", ); - snapcompactResult = undefined; + } else { + snapcompactResult = await snapcompact.compact(preparation, { + convertToLlm, + model: this.model, + shape: snapcompact.resolveShape(this.model, this.settings.get("snapcompact.shape")), + maxFrames, + }); + const ctxWindow = this.model?.contextWindow ?? 0; + const budget = + ctxWindow > 0 + ? ctxWindow - effectiveReserveTokens(ctxWindow, effectiveSettings) + : Number.POSITIVE_INFINITY; + if (this.#projectSnapcompactContextTokens(preparation, snapcompactResult) > budget) { + logger.warn("Snapcompact still overflows the window after frame-budget sizing; falling back", { + model: this.model?.id, + }); + this.emitNotice( + "warning", + "snapcompact could not bring the context under the limit — using an LLM summary instead", + "compaction", + ); + snapcompactResult = undefined; + } } } @@ -9407,6 +9422,40 @@ export class AgentSession { return { kind: "needsLlm", hookContext, hookPrompt, preserveData }; } + /** + * Cap on snapcompact frames the post-compaction context can carry without + * busting the model window. Mirrors the per-frame token charge used by the + * projection ({@link snapcompact.FRAME_TOKEN_ESTIMATE}, the conservative + * high-res Anthropic ceiling), so picking `maxFrames` from this helper makes + * {@link #projectSnapcompactContextTokens} succeed by construction. + * + * Returns `0` when the kept-recent slice plus the non-message overhead + * already eats the entire budget — at that point snapcompact cannot fit a + * single frame and the caller MUST skip it instead of running just to + * reject the result and re-emit the "could not bring the context under the + * limit" warning every threshold tick. Without this cap, the bundled + * `MAX_FRAMES_DEFAULT = 80` × 5024 tokens = ~402k frame-token projection + * always overflows any sub-1M-token window (issue #3247). + */ + #computeSnapcompactMaxFrames(preparation: CompactionPreparation, settings: CompactionSettings): number { + const ctxWindow = this.model?.contextWindow ?? 0; + if (ctxWindow <= 0) return snapcompact.MAX_FRAMES_DEFAULT; + const reserve = effectiveReserveTokens(ctxWindow, settings); + let nonFrameTokens = computeNonMessageTokens(this); + for (const message of preparation.recentMessages) { + nonFrameTokens += estimateTokens(message); + } + // Headroom for the summary-message lead-in plus the verbatim text edges + // snapcompact pins around the imaged middle. Sized for the typical + // snapcompact summary (~2k tokens) plus one HQ-capacity text edge on + // each side; conservative, so a tighter post-render run cannot drift + // past the projection check below. + const SUMMARY_TEXT_RESERVE = 4000; + const frameBudget = ctxWindow - reserve - nonFrameTokens - SUMMARY_TEXT_RESERVE; + if (frameBudget < snapcompact.FRAME_TOKEN_ESTIMATE) return 0; + return Math.min(Math.floor(frameBudget / snapcompact.FRAME_TOKEN_ESTIMATE), snapcompact.MAX_FRAMES_DEFAULT); + } + /** * Project the post-compaction context size of a snapcompact result: kept * recent messages + the summary message with its re-attached frames + the @@ -9652,24 +9701,20 @@ export class AgentSession { let tokensBefore: number; let details: unknown; - // Snapcompact runs locally first; if its frame archive plus the kept - // history still overflows the model window (frames default to - // MAX_FRAMES_DEFAULT and cost ~FRAME_TOKEN_ESTIMATE each), an LLM - // summary is far cheaper — downgrade to context-full and take the - // summarizer path. + // Snapcompact runs locally first. The post-compaction context = kept-recent + // + a summary message carrying the imaged archive at FRAME_TOKEN_ESTIMATE + // per frame; #computeSnapcompactMaxFrames sizes the frame cap from the + // live window so we don't run snapcompact just to overflow and fall back + // every threshold tick. Kept-recent already over budget → skip snapcompact + // outright (a single frame won't fit). Otherwise the projection below is + // only a defensive guard for summary-text drift. let snapcompactResult: snapcompact.CompactionResult | undefined; if (action === "snapcompact" && compactionPrep.kind !== "fromHook") { const text = snapcompact.serializeConversation( convertToLlm(preparation.messagesToSummarize.concat(preparation.turnPrefixMessages)), ); const renderScan = snapcompact.scanRenderability(text); - if (renderScan.isSafe) { - snapcompactResult = await snapcompact.compact(preparation, { - convertToLlm, - model: this.model, - shape: snapcompact.resolveShape(this.model, this.settings.get("snapcompact.shape")), - }); - } else { + if (!renderScan.isSafe) { logger.warn("Snapcompact disabled: high non-ASCII rate detected; falling back to an LLM summary", { model: this.model?.id, unrenderableRatio: renderScan.unrenderableRatio, @@ -9680,6 +9725,26 @@ export class AgentSession { "compaction", ); action = "context-full"; + } else { + const maxFrames = this.#computeSnapcompactMaxFrames(preparation, compactionSettings); + if (maxFrames < 1) { + logger.warn("Snapcompact skipped: kept history alone exceeds the context budget", { + model: this.model?.id, + }); + this.emitNotice( + "warning", + "snapcompact: kept history alone exceeds the context budget — using an LLM summary instead", + "compaction", + ); + action = "context-full"; + } else { + snapcompactResult = await snapcompact.compact(preparation, { + convertToLlm, + model: this.model, + shape: snapcompact.resolveShape(this.model, this.settings.get("snapcompact.shape")), + maxFrames, + }); + } } if (snapcompactResult) { @@ -9690,7 +9755,7 @@ export class AgentSession { : Number.POSITIVE_INFINITY; const projected = this.#projectSnapcompactContextTokens(preparation, snapcompactResult); if (projected > budget) { - logger.warn("Snapcompact still overflows the window; falling back to an LLM summary", { + logger.warn("Snapcompact still overflows the window after frame-budget sizing; falling back", { model: this.model?.id, projected, budget, diff --git a/packages/coding-agent/test/agent-session-snapcompact-budget.test.ts b/packages/coding-agent/test/agent-session-snapcompact-budget.test.ts new file mode 100644 index 000000000..dba0a415a --- /dev/null +++ b/packages/coding-agent/test/agent-session-snapcompact-budget.test.ts @@ -0,0 +1,198 @@ +/** + * Regression test for issue #3247. + * + * Snapcompact's bundled `MAX_FRAMES_DEFAULT = 80` × `FRAME_TOKEN_ESTIMATE = 5024` + * ≈ 402k tokens worth of frames. On any sub-1M-token window (e.g. Claude + * Sonnet 4.5's 200k), passing the default cap to `snapcompact.compact()` made + * the post-render projection in `AgentSession` always overflow the budget, + * emit the "snapcompact could not bring the context under the limit" warning + * on every threshold tick, and downgrade to an LLM summary. The fix sizes the + * `maxFrames` cap from the live model window (window − reserve − non-message + * overhead − kept-recent − summary-text reserve) before calling + * `snapcompact.compact()`. + * + * The contract this test defends: for a 200k-window vision model with sane + * kept-recent traffic, AgentSession MUST pass a budget-sized `maxFrames` + * (smaller than `MAX_FRAMES_DEFAULT`, and with `maxFrames × FRAME_TOKEN_ESTIMATE` + * inside the resolved budget) so the projection accepts the snapcompact + * result instead of falling back to the LLM summarizer. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; +import * as path from "node:path"; +import { Agent } from "@oh-my-pi/pi-agent-core"; +import { effectiveReserveTokens } from "@oh-my-pi/pi-agent-core/compaction"; +import { getBundledModel } from "@oh-my-pi/pi-catalog/models"; +import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session"; +import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; +import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; +import { TempDir } from "@oh-my-pi/pi-utils"; +import * as snapcompact from "@oh-my-pi/snapcompact"; + +describe("AgentSession snapcompact frame-budget sizing", () => { + let tempDir: TempDir; + let session: AgentSession; + let sessionManager: SessionManager; + let authStorage: AuthStorage; + let modelRegistry: ModelRegistry; + + beforeEach(async () => { + tempDir = TempDir.createSync("@pi-snapcompact-budget-"); + + authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db")); + authStorage.setRuntimeApiKey("anthropic", "test-key"); + modelRegistry = new ModelRegistry(authStorage); + sessionManager = SessionManager.create(tempDir.path(), tempDir.path()); + + const model = getBundledModel("anthropic", "claude-sonnet-4-5"); + if (!model) throw new Error("Expected bundled claude-sonnet-4-5 model"); + // Sanity: the contract only holds for vision models with a window + // genuinely smaller than the snapcompact upper bound. If the bundled + // catalog ever raises Sonnet's window past 1M, this test no longer + // covers the failure mode the fix targets. + expect(model.input).toContain("image"); + expect(model.contextWindow).toBeLessThan(1_000_000); + + const agent = new Agent({ + initialState: { model, systemPrompt: ["Test"], tools: [], messages: [] }, + }); + + // Seed a representative long-running session: many turn-pairs with + // substantial filler so prepareCompaction() splits the branch into + // "discard + summarize" (oldest) vs "kept-recent" (newest). + const filler = "the quick brown fox jumps over the lazy dog. ".repeat(64); + for (let i = 0; i < 64; i++) { + sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: `turn ${i}: ${filler}` }], + timestamp: Date.now() - (64 - i) * 1000, + }); + sessionManager.appendMessage({ + role: "assistant", + content: [{ type: "text", text: `reply ${i}: ${filler}` }], + api: "anthropic-messages", + provider: "anthropic", + model: "claude-sonnet-4-5", + stopReason: "stop", + usage: { + input: 1000, + output: 1000, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 2000, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + timestamp: Date.now() - (64 - i) * 1000 + 100, + }); + } + + session = new AgentSession({ + agent, + sessionManager, + settings: Settings.isolated({ + "compaction.strategy": "snapcompact", + "compaction.autoContinue": false, + // Force a small kept-recent window so the seeded conversation + // definitely splits into discard + kept and prepareCompaction() + // returns a non-empty preparation. + "compaction.keepRecentTokens": 4000, + }), + modelRegistry, + }); + }); + + afterEach(async () => { + try { + await session?.dispose(); + } finally { + authStorage?.close(); + await tempDir?.remove(); + vi.restoreAllMocks(); + } + }); + + it("passes a window-sized maxFrames to snapcompact.compact() on sub-1M-token models", async () => { + // Capture the options snapcompact.compact() is invoked with, and short- + // circuit it so the projection downstream evaluates against a known + // empty-frame archive (which fits any budget). The contract is about + // what the caller asks for, not what snapcompact then chooses to emit. + const model = session.model; + if (!model) throw new Error("Expected model to be set on session"); + const ctxWindow = model.contextWindow ?? 0; + expect(ctxWindow).toBeGreaterThan(0); + + const branchEntries = sessionManager.getBranch(); + const firstKeptEntry = branchEntries[branchEntries.length - 1]; + if (!firstKeptEntry?.id) throw new Error("Expected branch entry with id"); + + const compactSpy = vi.spyOn(snapcompact, "compact").mockResolvedValue({ + summary: "stubbed snapcompact", + shortSummary: "stub", + firstKeptEntryId: firstKeptEntry.id, + tokensBefore: 100_000, + details: { readFiles: [], modifiedFiles: [] }, + preserveData: { + snapcompact: { frames: [], totalChars: 0, truncatedChars: 0 }, + }, + }); + + await session.compact(undefined, { mode: "snapcompact" }); + + expect(compactSpy).toHaveBeenCalledTimes(1); + const opts = compactSpy.mock.calls[0]?.[1]; + expect(opts).toBeDefined(); + const maxFrames = opts?.maxFrames; + expect(maxFrames).toBeDefined(); + expect(maxFrames).toBeLessThan(snapcompact.MAX_FRAMES_DEFAULT); + expect(maxFrames).toBeGreaterThan(0); + + // The chosen cap MUST keep the projected frame budget inside the + // resolved (window − reserve) envelope — otherwise the projection + // guard would reject and loop back to the LLM summary every tick. + const reserve = effectiveReserveTokens(ctxWindow, { + enabled: true, + reserveTokens: 16384, + keepRecentTokens: 4000, + }); + const budget = ctxWindow - reserve; + expect((maxFrames ?? 0) * snapcompact.FRAME_TOKEN_ESTIMATE).toBeLessThan(budget); + }); + + it("skips snapcompact entirely when kept-recent already exceeds the budget", async () => { + // Append one synthetic message large enough to overflow the model window + // on its own (kept by findCutPoint since keepRecentTokens=4000 falls + // well short of it). Snapcompact CANNOT fit even a single frame; the + // session MUST skip it instead of running and emitting "could not bring + // the context under the limit" every tick. + const model = session.model; + if (!model) throw new Error("Expected model"); + const ctxWindow = model.contextWindow ?? 0; + const huge = "a".repeat(ctxWindow * 4); + sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: huge }], + timestamp: Date.now(), + }); + + const compactSpy = vi.spyOn(snapcompact, "compact"); + const notices: { level: string; message: string }[] = []; + session.subscribe(event => { + if (event.type === "notice") { + notices.push({ level: event.level, message: event.message }); + } + }); + + await expect(session.compact(undefined, { mode: "snapcompact" })).rejects.toThrow(); + + // snapcompact.compact() MUST NOT be invoked when the budget cannot + // fit even one frame — running it just to reject the result and + // re-emit the warning is the exact loop issue #3247 reports. + expect(compactSpy).not.toHaveBeenCalled(); + // The user-facing notice MUST explain the kept-history overflow rather + // than the misleading "could not bring the context under the limit" + // (which implied snapcompact had run and produced an oversized result). + expect(notices.some(n => n.level === "warning" && n.message.includes("kept history"))).toBe(true); + }); +});