From 6958b86822f444f9231e5aab9a71267ee0657eed Mon Sep 17 00:00:00 2001 From: Wolfgang Schoenberger <221313372+wolfiesch@users.noreply.github.com> Date: Fri, 26 Jun 2026 01:38:02 -0700 Subject: [PATCH] fix(coding-agent): treat at-band residual as compaction headroom The post-maintenance headroom guard returned residualTokens < triggerContextTokens as a secondary check after the recovery-band test. When stale/tool-output pruning already drove the trigger (postMaintenanceContextTokens) below the band before this pass, that strict-less comparison made a residual which merely held the line at/under the band report a false no-progress, suppressing a valid auto-continue and emitting a spurious warning even though the next turn could no longer re-trip threshold compaction. The recovery band sits strictly under the compaction threshold, so reaching it already guarantees the next turn cannot re-trip. Make the band authoritative (residual <= band) and drop the trigger argument. Adds a regression covering the sub-band trigger case. (cherry picked from commit 31cf2dc7a30c4134f7a351d61065eca76543f154) --- packages/coding-agent/CHANGELOG.md | 1 + .../coding-agent/src/session/agent-session.ts | 30 +++-- ...ion-auto-compaction-progress-guard.test.ts | 109 ++++++++++++++++++ 3 files changed, 124 insertions(+), 16 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 044c0efe6..17aa0b521 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -70,6 +70,7 @@ - Fixed the advisor entering a spam loop in which it emitted hundreds of repeated `Stop.`, `Done.`, and `No issue; continue.` `` injections, polluting the primary transcript and destabilizing the watched agent after the task was already complete. The advisor system prompt's rules ("at most one `advise` per update", "NEVER send the same advice twice") are now enforced in code by a new `AdvisorEmissionGuard` on the `enqueueAdvice` boundary in `AgentSession`: it normalizes each note (case-insensitive, punctuation-folded), drops content-free self-talk filler (`stop`/`done`/`no issue continue`/`lgtm`/etc.), dedupes by exact normalized text across the session (bounded FIFO history), and rate-limits to one accepted note per advisor model prompt cycle. Reset on advisor reset (compaction, session switch, `/new`) so a re-primed reviewer can re-raise old issues. ([#3520](https://github.com/can1357/oh-my-pi/issues/3520)) +- Fixed auto-compaction thrashing on a session whose single most-recent kept turn already exceeds the compaction threshold. `prepareCompaction` keeps that turn verbatim (`findCutPoint` never cuts at tool results), so the rewritten context stays above threshold; the context-full / snapcompact success tail scheduled the agent-authored auto-continue (and the overflow/incomplete retry) unconditionally, so the next `agent_end` re-entered `#checkCompaction` over the same oversized tail and re-fired forever. This is the residual loop left after #3247 capped snapcompact's own frame projection — once the frame cap drops below one frame, snapcompact is skipped and the context-full summarizer path still made no headroom. `#runAutoCompaction` now gates the threshold auto-continue on a post-maintenance headroom check (`#compactionCreatedHeadroom`, sharing shake's `COMPACTION_RECOVERY_BAND` hysteresis from #2275) and the overflow/incomplete retry on a separate fit check (`#compactionCreatedRetryFit`, measured after the failed turn is dropped) so a recoverable overflow that fits the window still retries; when a pass frees too little for the relevant path it pauses automatic maintenance and emits a single warning instead of looping. The post-turn threshold check also ignores an assistant's stale pre-compaction `usage` so the scheduled auto-continue cannot re-trip on the kept assistant's old high token count. The headroom check now treats any residual at or below the recovery band as progress: the band sits strictly under the compaction threshold, so a stale/tool-output prune that already pushed the trigger sub-band no longer makes a residual that merely holds the line report a false "no progress" and suppress a valid auto-continue. ## [16.1.20] - 2026-06-25 ### Fixed diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 17dbb6460..6b68aefe9 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -10138,12 +10138,14 @@ export class AgentSession { * blows the threshold cannot be reduced by compaction (findCutPoint keeps that * turn verbatim), so re-firing on the next agent_end just thrashes. We only * report progress when residual context lands at or below - * `COMPACTION_RECOVERY_BAND × threshold`. + * `COMPACTION_RECOVERY_BAND × threshold` — a band that sits strictly under the + * compaction threshold, so reaching it guarantees the next turn cannot + * re-trip threshold compaction. * * When the model/window is unknown we cannot evaluate the band, so we * optimistically allow the continuation (preserving prior behavior). */ - #compactionCreatedHeadroom(triggerContextTokens?: number): boolean { + #compactionCreatedHeadroom(): boolean { const contextWindow = this.model?.contextWindow ?? 0; if (contextWindow <= 0) return true; const compactionSettings = this.settings.getGroup("compaction"); @@ -10153,19 +10155,15 @@ export class AgentSession { ); const thresholdTokens = resolveThresholdTokens(contextWindow, compactionSettings); const recoveryBand = Math.floor(thresholdTokens * COMPACTION_RECOVERY_BAND); - // A genuine reduction past the band always counts as progress. The - // triggerContextTokens comparison is a secondary guard: if residual context - // is not meaningfully smaller than what triggered this pass, treat it as a - // no-op even when it nominally sits under the band. - if (residualTokens > recoveryBand) return false; - if ( - typeof triggerContextTokens === "number" && - Number.isFinite(triggerContextTokens) && - triggerContextTokens > 0 - ) { - return residualTokens < triggerContextTokens; - } - return true; + // Residual at/below the band is authoritative headroom: the band sits + // strictly under the compaction threshold, so the next turn cannot + // re-trip threshold compaction regardless of how little this pass shaved. + // Don't add a secondary "smaller than the trigger" guard — when stale/ + // tool-output pruning already dropped context under the band before this + // pass, the trigger is itself sub-band, and requiring a strict reduction + // would suppress a valid continuation and emit a false no-progress warning + // even though compaction left the session safe. + return residualTokens <= recoveryBand; } /** @@ -10736,7 +10734,7 @@ export class AgentSession { // landed residual context under `COMPACTION_RECOVERY_BAND × threshold`. // Re-firing on a history that still sits just over the line is the // snapcompact thrash, so require genuine headroom, not a bare fit. - if (this.#compactionCreatedHeadroom(options.triggerContextTokens)) { + if (this.#compactionCreatedHeadroom()) { this.#scheduleAutoContinuePrompt(generation); continuationScheduled = true; } else { diff --git a/packages/coding-agent/test/agent-session-auto-compaction-progress-guard.test.ts b/packages/coding-agent/test/agent-session-auto-compaction-progress-guard.test.ts index d6fe80538..8fb040d27 100644 --- a/packages/coding-agent/test/agent-session-auto-compaction-progress-guard.test.ts +++ b/packages/coding-agent/test/agent-session-auto-compaction-progress-guard.test.ts @@ -344,6 +344,115 @@ describe("AgentSession auto-compaction progress guard", () => { expect(noProgress.length).toBe(0); }); + /** + * Seed a single large `useless` tool result (plus tiny follow-up turns that + * keep its suffix inside the cache-warm window) so the per-turn maintenance + * passes free ~40k tokens before compaction runs — the same shape as the + * #3174 pruning regression. This drives `postMaintenanceContextTokens` (the + * trigger handed to the headroom guard) well below the recovery band. + */ + function seedPrunableMaintenance(now: number) { + sessionManager.appendMessage({ role: "user", content: "Investigate everything.", timestamp: now - 200 }); + const bigCallId = "call-big-useless"; + sessionManager.appendMessage({ + role: "assistant", + content: [{ type: "toolCall", id: bigCallId, name: "search", arguments: { pattern: "TODO" } }], + api: "anthropic-messages", + provider: "anthropic", + model: "claude-sonnet-4-5", + stopReason: "toolUse", + usage: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, totalTokens: 0, cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 } }, + timestamp: now - 180, + }); + sessionManager.appendMessage({ + role: "toolResult", + toolCallId: bigCallId, + toolName: "search", + content: [{ type: "text", text: "match line\n".repeat(20000) }], // ~40k+ tokens + isError: false, + useless: true, + timestamp: now - 170, + }); + for (let i = 0; i < 4; i++) { + const smallId = `call-small-${i}`; + const ts = now - 160 + i * 2; + sessionManager.appendMessage({ + role: "assistant", + content: [{ type: "toolCall", id: smallId, name: "read", arguments: { path: `note-${i}.md` } }], + api: "anthropic-messages", + provider: "anthropic", + model: "claude-sonnet-4-5", + stopReason: "toolUse", + usage: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, totalTokens: 0, cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 } }, + timestamp: ts, + }); + sessionManager.appendMessage({ + role: "toolResult", + toolCallId: smallId, + toolName: "read", + content: [{ type: "text", text: `tiny note ${i}` }], + isError: false, + timestamp: ts + 1, + }); + } + session.agent.replaceMessages(session.buildDisplaySessionContext().messages); + } + + it("auto-continues when residual sits at the recovery band but the trigger was already sub-band", async () => { + // Regression for the #3412 review: when stale/tool-output pruning already + // dropped context under the recovery band BEFORE this pass, the trigger + // (postMaintenanceContextTokens) is itself sub-band. The old guard returned + // `residual < trigger`, so a residual that merely held the line at/under the + // band — not strictly smaller than the already-safe trigger — was reported + // as no-progress and the auto-continue was suppressed with a false warning, + // even though the next turn could no longer re-trip threshold compaction. + const now = Date.now(); + // Pin the threshold so the recovery band is exact: floor(76384 * 0.8) = 61107. + session.settings.set("compaction.thresholdTokens", 76384); + session.settings.set("compaction.thresholdPercent", -1); + session.settings.set("compaction.strategy", "context-full"); + session.settings.set("compaction.dropUseless", true); + session.settings.set("compaction.supersedeReads", true); + session.settings.set("compaction.keepRecentTokens", 10000); + session.settings.set("compaction.reserveTokens", 16384); + seedPrunableMaintenance(now); + + const promptSpy = vi.spyOn(session.agent, "prompt").mockResolvedValue(undefined as never); + vi.spyOn(session.agent, "continue").mockResolvedValue(); + // Residual lands AT the band (61000 <= 61107). Maintenance pruning already + // drove the trigger below this, so the old strict-less guard would have + // suppressed; the band check proves headroom and continues. + vi.spyOn(session, "getContextUsage").mockReturnValue({ tokens: 61000, contextWindow: 200000, percent: 30.5 }); + + const notices = collectNotices(); + + const { promise: compactionDone, resolve: onCompactionDone } = Promise.withResolvers(); + session.subscribe(event => { + if (event.type === "auto_compaction_end") onCompactionDone(); + }); + + // Final turn billed above the 76384 threshold so threshold compaction fires. + const finalAssistant = { + role: "assistant" as const, + content: [{ type: "text" as const, text: "continuing." }], + api: "anthropic-messages" as const, + provider: "anthropic" as const, + model: "claude-sonnet-4-5", + stopReason: "stop" as const, + usage: { input: 5000, output: 1000, cacheRead: 85000, cacheWrite: 0, totalTokens: 91000, cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 } }, + timestamp: now, + }; + session.agent.emitExternalEvent({ type: "message_end", message: finalAssistant }); + session.agent.emitExternalEvent({ type: "agent_end", messages: [finalAssistant] }); + + await compactionDone; + await session.waitForIdle(); + + expect(promptSpy).toHaveBeenCalledTimes(1); + const noProgress = notices.filter(n => n.source === NOTICE_SOURCE && n.message.includes(NO_PROGRESS_FRAGMENT)); + expect(noProgress.length).toBe(0); + }); + it("pauses (single warning) when an overflow recovery still does not fit the window", async () => { // The genuine dead-end the retry guard must still catch: even after dropping // the failed turn the rebuilt prompt is over the window, so retrying would