diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 2f799074..665ec4a3 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixed +- Fixed interactive compaction UI updates so `ctx.compact()` rebuilds the chat through unified compaction events, manual compaction no longer duplicates the summary block, and the `trigger-compact` example only fires when context usage crosses its threshold ([#2617](https://github.com/badlogic/pi-mono/issues/2617)) - Fixed auto-compaction overflow recovery for Ollama models when the backend returns explicit `prompt too long; exceeded max context length ...` errors instead of silently truncating input ([#2626](https://github.com/badlogic/pi-mono/issues/2626)) - Fixed built-in tool overrides that reuse built-in parameter schemas to still honor custom `renderCall` and `renderResult` renderers in the interactive TUI, restoring the `minimal-mode` example ([#2595](https://github.com/badlogic/pi-mono/issues/2595)) diff --git a/packages/coding-agent/examples/extensions/trigger-compact.ts b/packages/coding-agent/examples/extensions/trigger-compact.ts index e947318c..cd0b5995 100644 --- a/packages/coding-agent/examples/extensions/trigger-compact.ts +++ b/packages/coding-agent/examples/extensions/trigger-compact.ts @@ -3,6 +3,8 @@ import type { ExtensionAPI, ExtensionContext } from "@mariozechner/pi-coding-age const COMPACT_THRESHOLD_TOKENS = 100_000; export default function (pi: ExtensionAPI) { + let previousTokens: number | null | undefined; + const triggerCompaction = (ctx: ExtensionContext, customInstructions?: string) => { if (ctx.hasUI) { ctx.ui.notify("Compaction started", "info"); @@ -24,7 +26,15 @@ export default function (pi: ExtensionAPI) { pi.on("turn_end", (_event, ctx) => { const usage = ctx.getContextUsage(); - if (!usage || usage.tokens === null || usage.tokens <= COMPACT_THRESHOLD_TOKENS) { + const currentTokens = usage?.tokens ?? null; + if (currentTokens === null) { + return; + } + + const crossedThreshold = + previousTokens !== undefined && previousTokens !== null && previousTokens <= COMPACT_THRESHOLD_TOKENS; + previousTokens = currentTokens; + if (!crossedThreshold || currentTokens <= COMPACT_THRESHOLD_TOKENS) { return; } triggerCompaction(ctx); diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 157da913..384d5621 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -112,9 +112,10 @@ export function parseSkillBlock(text: string): ParsedSkillBlock | null { /** Session-specific events that extend the core AgentEvent */ export type AgentSessionEvent = | AgentEvent - | { type: "auto_compaction_start"; reason: "threshold" | "overflow" } + | { type: "compaction_start"; reason: "manual" | "threshold" | "overflow" } | { - type: "auto_compaction_end"; + type: "compaction_end"; + reason: "manual" | "threshold" | "overflow"; result: CompactionResult | undefined; aborted: boolean; willRetry: boolean; @@ -1627,6 +1628,7 @@ export class AgentSession { this._disconnectFromAgent(); await this.abort(); this._compactionAbortController = new AbortController(); + this._emit({ type: "compaction_start", reason: "manual" }); try { if (!this.model) { @@ -1719,12 +1721,32 @@ export class AgentSession { }); } - return { + const compactionResult = { summary, firstKeptEntryId, tokensBefore, details, }; + this._emit({ + type: "compaction_end", + reason: "manual", + result: compactionResult, + aborted: false, + willRetry: false, + }); + return compactionResult; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + const aborted = message === "Compaction cancelled" || (error instanceof Error && error.name === "AbortError"); + this._emit({ + type: "compaction_end", + reason: "manual", + result: undefined, + aborted, + willRetry: false, + errorMessage: aborted ? undefined : `Compaction failed: ${message}`, + }); + throw error; } finally { this._compactionAbortController = undefined; this._reconnectToAgent(); @@ -1787,7 +1809,8 @@ export class AgentSession { if (sameModel && isContextOverflow(assistantMessage, contextWindow)) { if (this._overflowRecoveryAttempted) { this._emit({ - type: "auto_compaction_end", + type: "compaction_end", + reason: "overflow", result: undefined, aborted: false, willRetry: false, @@ -1842,18 +1865,30 @@ export class AgentSession { private async _runAutoCompaction(reason: "overflow" | "threshold", willRetry: boolean): Promise { const settings = this.settingsManager.getCompactionSettings(); - this._emit({ type: "auto_compaction_start", reason }); + this._emit({ type: "compaction_start", reason }); this._autoCompactionAbortController = new AbortController(); try { if (!this.model) { - this._emit({ type: "auto_compaction_end", result: undefined, aborted: false, willRetry: false }); + this._emit({ + type: "compaction_end", + reason, + result: undefined, + aborted: false, + willRetry: false, + }); return; } const authResult = await this._modelRegistry.getApiKeyAndHeaders(this.model); if (!authResult.ok || !authResult.apiKey) { - this._emit({ type: "auto_compaction_end", result: undefined, aborted: false, willRetry: false }); + this._emit({ + type: "compaction_end", + reason, + result: undefined, + aborted: false, + willRetry: false, + }); return; } const { apiKey, headers } = authResult; @@ -1862,7 +1897,13 @@ export class AgentSession { const preparation = prepareCompaction(pathEntries, settings); if (!preparation) { - this._emit({ type: "auto_compaction_end", result: undefined, aborted: false, willRetry: false }); + this._emit({ + type: "compaction_end", + reason, + result: undefined, + aborted: false, + willRetry: false, + }); return; } @@ -1879,7 +1920,13 @@ export class AgentSession { })) as SessionBeforeCompactResult | undefined; if (extensionResult?.cancel) { - this._emit({ type: "auto_compaction_end", result: undefined, aborted: true, willRetry: false }); + this._emit({ + type: "compaction_end", + reason, + result: undefined, + aborted: true, + willRetry: false, + }); return; } @@ -1917,7 +1964,13 @@ export class AgentSession { } if (this._autoCompactionAbortController.signal.aborted) { - this._emit({ type: "auto_compaction_end", result: undefined, aborted: true, willRetry: false }); + this._emit({ + type: "compaction_end", + reason, + result: undefined, + aborted: true, + willRetry: false, + }); return; } @@ -1945,7 +1998,7 @@ export class AgentSession { tokensBefore, details, }; - this._emit({ type: "auto_compaction_end", result, aborted: false, willRetry }); + this._emit({ type: "compaction_end", reason, result, aborted: false, willRetry }); if (willRetry) { const messages = this.agent.state.messages; @@ -1967,7 +2020,8 @@ export class AgentSession { } catch (error) { const errorMessage = error instanceof Error ? error.message : "compaction failed"; this._emit({ - type: "auto_compaction_end", + type: "compaction_end", + reason, result: undefined, aborted: false, willRetry: false, diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index be24a144..dc0ed3df 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -57,7 +57,7 @@ import type { } from "../../core/extensions/index.js"; import { FooterDataProvider, type ReadonlyFooterDataProvider } from "../../core/footer-data-provider.js"; import { type AppKeybinding, KeybindingsManager } from "../../core/keybindings.js"; -import { createCompactionSummaryMessage } from "../../core/messages.js"; + import { findExactModelReferenceMatch, resolveModelScope } from "../../core/model-resolver.js"; import { DefaultPackageManager } from "../../core/package-manager.js"; import type { ResourceDiagnostic } from "../../core/resource-loader.js"; @@ -1290,10 +1290,8 @@ export class InteractiveMode { compact: (options) => { void (async () => { try { - const result = await this.executeCompaction(options?.customInstructions, false); - if (result) { - options?.onComplete?.(result); - } + const result = await this.session.compact(options?.customInstructions); + options?.onComplete?.(result); } catch (error) { const err = error instanceof Error ? error : new Error(String(error)); options?.onError?.(err); @@ -2398,58 +2396,56 @@ export class InteractiveMode { this.ui.requestRender(); break; - case "auto_compaction_start": { + case "compaction_start": { // Keep editor active; submissions are queued during compaction. - // Set up escape to abort auto-compaction this.autoCompactionEscapeHandler = this.defaultEditor.onEscape; this.defaultEditor.onEscape = () => { this.session.abortCompaction(); }; - // Show compacting indicator with reason this.statusContainer.clear(); - const reasonText = event.reason === "overflow" ? "Context overflow detected, " : ""; + const cancelHint = `(${keyText("app.interrupt")} to cancel)`; + const label = + event.reason === "manual" + ? `Compacting context... ${cancelHint}` + : `${event.reason === "overflow" ? "Context overflow detected, " : ""}Auto-compacting... ${cancelHint}`; this.autoCompactionLoader = new Loader( this.ui, (spinner) => theme.fg("accent", spinner), (text) => theme.fg("muted", text), - `${reasonText}Auto-compacting... (${keyText("app.interrupt")} to cancel)`, + label, ); this.statusContainer.addChild(this.autoCompactionLoader); this.ui.requestRender(); break; } - case "auto_compaction_end": { - // Restore escape handler + case "compaction_end": { if (this.autoCompactionEscapeHandler) { this.defaultEditor.onEscape = this.autoCompactionEscapeHandler; this.autoCompactionEscapeHandler = undefined; } - // Stop loader if (this.autoCompactionLoader) { this.autoCompactionLoader.stop(); this.autoCompactionLoader = undefined; this.statusContainer.clear(); } - // Handle result if (event.aborted) { - this.showStatus("Auto-compaction cancelled"); + if (event.reason === "manual") { + this.showError("Compaction cancelled"); + } else { + this.showStatus("Auto-compaction cancelled"); + } } else if (event.result) { - // Rebuild chat to show compacted state this.chatContainer.clear(); this.rebuildChatFromMessages(); - // Add compaction component at bottom so user sees it without scrolling - this.addMessageToChat({ - role: "compactionSummary", - tokensBefore: event.result.tokensBefore, - summary: event.result.summary, - timestamp: Date.now(), - }); this.footer.invalidate(); } else if (event.errorMessage) { - // Compaction failed (e.g., quota exceeded, API error) - this.chatContainer.addChild(new Spacer(1)); - this.chatContainer.addChild(new Text(theme.fg("error", event.errorMessage), 1, 0)); + if (event.reason === "manual") { + this.showError(event.errorMessage); + } else { + this.chatContainer.addChild(new Spacer(1)); + this.chatContainer.addChild(new Text(theme.fg("error", event.errorMessage), 1, 0)); + } } void this.flushCompactionQueue({ willRetry: event.willRetry }); this.ui.requestRender(); @@ -4573,63 +4569,21 @@ export class InteractiveMode { return; } - await this.executeCompaction(customInstructions, false); + await this.executeCompaction(customInstructions); } - private async executeCompaction(customInstructions?: string, isAuto = false): Promise { - // Stop loading animation + private async executeCompaction(customInstructions?: string): Promise { if (this.loadingAnimation) { this.loadingAnimation.stop(); this.loadingAnimation = undefined; } this.statusContainer.clear(); - // Set up escape handler during compaction - const originalOnEscape = this.defaultEditor.onEscape; - this.defaultEditor.onEscape = () => { - this.session.abortCompaction(); - }; - - // Show compacting status - this.chatContainer.addChild(new Spacer(1)); - const cancelHint = `(${keyText("app.interrupt")} to cancel)`; - const label = isAuto ? `Auto-compacting context... ${cancelHint}` : `Compacting context... ${cancelHint}`; - const compactingLoader = new Loader( - this.ui, - (spinner) => theme.fg("accent", spinner), - (text) => theme.fg("muted", text), - label, - ); - this.statusContainer.addChild(compactingLoader); - this.ui.requestRender(); - - let result: CompactionResult | undefined; - try { - result = await this.session.compact(customInstructions); - - // Rebuild UI - this.rebuildChatFromMessages(); - - // Add compaction component at bottom so user sees it without scrolling - const msg = createCompactionSummaryMessage(result.summary, result.tokensBefore, new Date().toISOString()); - this.addMessageToChat(msg); - - this.footer.invalidate(); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - if (message === "Compaction cancelled" || (error instanceof Error && error.name === "AbortError")) { - this.showError("Compaction cancelled"); - } else { - this.showError(`Compaction failed: ${message}`); - } - } finally { - compactingLoader.stop(); - this.statusContainer.clear(); - this.defaultEditor.onEscape = originalOnEscape; + return await this.session.compact(customInstructions); + } catch { + return undefined; } - void this.flushCompactionQueue({ willRetry: false }); - return result; } stop(): void { diff --git a/packages/coding-agent/test/agent-session-auto-compaction-queue.test.ts b/packages/coding-agent/test/agent-session-auto-compaction-queue.test.ts index ab8c9f00..f24f60ce 100644 --- a/packages/coding-agent/test/agent-session-auto-compaction-queue.test.ts +++ b/packages/coding-agent/test/agent-session-auto-compaction-queue.test.ts @@ -153,10 +153,10 @@ describe("AgentSession auto-compaction queue resume", () => { ) .mockResolvedValue(); - const events: Array<{ type: string; errorMessage?: string }> = []; + const events: Array<{ type: string; reason: string; errorMessage?: string }> = []; session.subscribe((event) => { - if (event.type === "auto_compaction_end") { - events.push({ type: event.type, errorMessage: event.errorMessage }); + if (event.type === "compaction_end") { + events.push({ type: event.type, reason: event.reason, errorMessage: event.errorMessage }); } }); @@ -171,7 +171,8 @@ describe("AgentSession auto-compaction queue resume", () => { expect(runAutoCompactionSpy).toHaveBeenCalledTimes(1); expect(events).toContainEqual({ - type: "auto_compaction_end", + type: "compaction_end", + reason: "overflow", errorMessage: "Context overflow recovery failed after one compact-and-retry attempt. Try reducing context or switching to a larger-context model.", }); diff --git a/packages/coding-agent/test/agent-session-compaction.test.ts b/packages/coding-agent/test/agent-session-compaction.test.ts index b4873bdb..cfa62ed7 100644 --- a/packages/coding-agent/test/agent-session-compaction.test.ts +++ b/packages/coding-agent/test/agent-session-compaction.test.ts @@ -181,7 +181,7 @@ describe.skipIf(!API_KEY)("AgentSession compaction e2e", () => { expect(compactionEntries.length).toBe(1); }, 120000); - it("should emit correct events during auto-compaction", async () => { + it("should emit compaction events during manual compaction", async () => { createSession(); // Build some history @@ -191,12 +191,15 @@ describe.skipIf(!API_KEY)("AgentSession compaction e2e", () => { // Manually trigger compaction and check events await session.compact(); - // Check that no auto_compaction events were emitted for manual compaction - const autoCompactionEvents = events.filter( - (e) => e.type === "auto_compaction_start" || e.type === "auto_compaction_end", - ); - // Manual compaction doesn't emit auto_compaction events - expect(autoCompactionEvents.length).toBe(0); + const compactionEvents = events.filter((e) => e.type === "compaction_start" || e.type === "compaction_end"); + expect(compactionEvents).toHaveLength(2); + expect(compactionEvents[0]).toEqual({ type: "compaction_start", reason: "manual" }); + expect(compactionEvents[1]).toMatchObject({ + type: "compaction_end", + reason: "manual", + aborted: false, + willRetry: false, + }); // Regular events should have been emitted const messageEndEvents = events.filter((e) => e.type === "message_end"); diff --git a/packages/coding-agent/test/interactive-mode-compaction.test.ts b/packages/coding-agent/test/interactive-mode-compaction.test.ts new file mode 100644 index 00000000..16d80c64 --- /dev/null +++ b/packages/coding-agent/test/interactive-mode-compaction.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, test, vi } from "vitest"; +import { InteractiveMode } from "../src/modes/interactive/interactive-mode.js"; + +describe("InteractiveMode compaction events", () => { + test("rebuilds chat without appending a duplicate compaction summary", async () => { + const fakeThis = { + isInitialized: true, + footer: { invalidate: vi.fn() }, + autoCompactionEscapeHandler: undefined as (() => void) | undefined, + autoCompactionLoader: undefined, + defaultEditor: {}, + statusContainer: { clear: vi.fn() }, + chatContainer: { clear: vi.fn() }, + rebuildChatFromMessages: vi.fn(), + addMessageToChat: vi.fn(), + showError: vi.fn(), + showStatus: vi.fn(), + flushCompactionQueue: vi.fn().mockResolvedValue(undefined), + ui: { requestRender: vi.fn() }, + }; + + const handleEvent = Reflect.get(InteractiveMode.prototype, "handleEvent") as ( + this: typeof fakeThis, + event: { + type: "compaction_end"; + reason: "manual" | "threshold" | "overflow"; + result: { tokensBefore: number; summary: string } | undefined; + aborted: boolean; + willRetry: boolean; + errorMessage?: string; + }, + ) => Promise; + + await handleEvent.call(fakeThis, { + type: "compaction_end", + reason: "manual", + result: { + tokensBefore: 123, + summary: "summary", + }, + aborted: false, + willRetry: false, + }); + + expect(fakeThis.chatContainer.clear).toHaveBeenCalledTimes(1); + expect(fakeThis.rebuildChatFromMessages).toHaveBeenCalledTimes(1); + expect(fakeThis.addMessageToChat).not.toHaveBeenCalled(); + expect(fakeThis.flushCompactionQueue).toHaveBeenCalledWith({ willRetry: false }); + }); +}); diff --git a/packages/coding-agent/test/trigger-compact-extension.test.ts b/packages/coding-agent/test/trigger-compact-extension.test.ts new file mode 100644 index 00000000..c65288e4 --- /dev/null +++ b/packages/coding-agent/test/trigger-compact-extension.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, test, vi } from "vitest"; +import triggerCompactExtension from "../examples/extensions/trigger-compact.js"; +import type { ExtensionAPI, ExtensionCommandContext, ExtensionContext } from "../src/core/extensions/index.js"; + +function createContext(tokens: number | null, compact = vi.fn()): ExtensionContext { + return { + hasUI: false, + ui: {} as ExtensionContext["ui"], + cwd: process.cwd(), + sessionManager: {} as ExtensionContext["sessionManager"], + modelRegistry: {} as ExtensionContext["modelRegistry"], + model: undefined, + isIdle: () => true, + abort: vi.fn(), + hasPendingMessages: () => false, + shutdown: vi.fn(), + getContextUsage: () => ({ tokens, contextWindow: 200_000, percent: tokens === null ? null : tokens / 2000 }), + compact, + getSystemPrompt: () => "", + }; +} + +describe("trigger-compact example extension", () => { + test("only auto-compacts when context usage crosses the threshold", () => { + let turnEndHandler: + | ((event: { type: "turn_end" }, ctx: ExtensionContext | ExtensionCommandContext) => void) + | undefined; + + const api = { + on: (event: string, handler: (event: { type: "turn_end" }, ctx: ExtensionContext) => void) => { + if (event === "turn_end") { + turnEndHandler = handler; + } + }, + registerCommand: vi.fn(), + } as unknown as ExtensionAPI; + + triggerCompactExtension(api); + expect(turnEndHandler).toBeDefined(); + + const compact = vi.fn(); + const event = { type: "turn_end" } as const; + + turnEndHandler?.(event, createContext(110_000, compact)); + expect(compact).not.toHaveBeenCalled(); + + turnEndHandler?.(event, createContext(120_000, compact)); + expect(compact).not.toHaveBeenCalled(); + + turnEndHandler?.(event, createContext(95_000, compact)); + expect(compact).not.toHaveBeenCalled(); + + turnEndHandler?.(event, createContext(105_000, compact)); + expect(compact).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/mom/src/agent.ts b/packages/mom/src/agent.ts index 12a0e7c7..88d2d187 100644 --- a/packages/mom/src/agent.ts +++ b/packages/mom/src/agent.ts @@ -600,15 +600,14 @@ function createRunner(sandboxConfig: SandboxConfig, channelId: string, channelDi queue.enqueueMessage(text, "thread", "response thread", false); } } - } else if (event.type === "auto_compaction_start") { - log.logInfo(`Auto-compaction started (reason: ${(event as any).reason})`); + } else if (event.type === "compaction_start") { + log.logInfo(`Compaction started (reason: ${event.reason})`); queue.enqueue(() => ctx.respond("_Compacting context..._", false), "compaction start"); - } else if (event.type === "auto_compaction_end") { - const compEvent = event as any; - if (compEvent.result) { - log.logInfo(`Auto-compaction complete: ${compEvent.result.tokensBefore} tokens compacted`); - } else if (compEvent.aborted) { - log.logInfo("Auto-compaction aborted"); + } else if (event.type === "compaction_end") { + if (event.result) { + log.logInfo(`Compaction complete: ${event.result.tokensBefore} tokens compacted`); + } else if (event.aborted) { + log.logInfo("Compaction aborted"); } } else if (event.type === "auto_retry_start") { const retryEvent = event as any;