From 13039b7bdff8460461798067ece8f65181063d89 Mon Sep 17 00:00:00 2001 From: roboomp Date: Tue, 21 Jul 2026 20:25:35 +0000 Subject: [PATCH] fix(tui): reused settled components after compaction Preserved settled user and assistant component instances during compaction-only transcript rebuilds so warmed Markdown and layout caches remain valid. Covered both manual and automatic compaction paths with focused regression tests. Fixes #6033 --- packages/coding-agent/CHANGELOG.md | 2 + .../modes/controllers/command-controller.ts | 2 +- .../src/modes/controllers/event-controller.ts | 5 +- .../src/modes/interactive-mode.ts | 14 ++-- packages/coding-agent/src/modes/types.ts | 12 +++- .../src/modes/utils/ui-helpers.ts | 60 +++++++++++------ .../test/compaction-lifecycle.test.ts | 5 +- .../test/compaction-transcript-reuse.test.ts | 66 +++++++++++++++++++ 8 files changed, 135 insertions(+), 31 deletions(-) create mode 100644 packages/coding-agent/test/compaction-transcript-reuse.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 6f32dc885..b8b0d2f9a 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -24,6 +24,8 @@ ### Fixed +- Fixed post-compaction transcript rebuilds blocking the main thread by reusing settled message components and their rendered layout caches ([#6033](https://github.com/can1357/oh-my-pi/issues/6033)). + - Fixed isolated `task` subagents mutating the parent git checkout and stacking parallel task branches by detaching the git directory. - Fixed Windows compatibility issues, including `launch start` daemons opening visible console windows, startup crashes when running from a drive root, and command errors in the `hub` tool with non-POSIX shells. - Fixed Windows stdio MCP servers launched through `.cmd`/`.bat` shims failing with `Transport closed` by escaping arguments properly. diff --git a/packages/coding-agent/src/modes/controllers/command-controller.ts b/packages/coding-agent/src/modes/controllers/command-controller.ts index 6d21be4de..730406dd0 100644 --- a/packages/coding-agent/src/modes/controllers/command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/command-controller.ts @@ -1271,7 +1271,7 @@ export class CommandController { compactingLoader.stop(); this.ctx.statusContainer.disposeChildren(); - this.ctx.rebuildChatFromMessages(); + this.ctx.rebuildChatFromMessages({ reuseSettledComponents: true }); this.ctx.statusLine.invalidate(); // Same as the auto-compaction rebuild: a collapsed transcript is an diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index 48af882fe..99b3ee169 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -915,6 +915,9 @@ export class EventController { createUsageRowBlock(event.message.usage, event.message.duration, event.message.ttft), ); } + if (displayMessage === event.message) { + this.ctx.transcriptMessageComponents.set(event.message, this.ctx.streamingComponent); + } this.ctx.streamingComponent = undefined; this.ctx.streamingMessage = undefined; // Pin a turn-ending provider error (e.g. Anthropic content-filter block) @@ -1311,7 +1314,7 @@ export class EventController { } } else if (event.result) { this.ctx.lastAssistantUsage = undefined; - this.ctx.rebuildChatFromMessages(); + this.ctx.rebuildChatFromMessages({ reuseSettledComponents: true }); this.ctx.statusLine.invalidate(); // When history collapses behind the summary divider, the frame // shrinks far below the committed row count; without clearing, the diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 9d3a2a59a..a11d60f6d 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -488,6 +488,7 @@ export class InteractiveMode implements InteractiveModeContext { proseOnlyThinking = true; compactionQueuedMessages: CompactionQueuedMessage[] = []; pendingTools = new Map(); + transcriptMessageComponents = new WeakMap(); pendingBashComponents: BashExecutionComponent[] = []; bashComponent: BashExecutionComponent | undefined = undefined; pendingPythonComponents: EvalExecutionComponent[] = []; @@ -1657,7 +1658,7 @@ export class InteractiveMode implements InteractiveModeContext { if (options.requestRender !== false) this.ui.requestRender(); } - rebuildChatFromMessages(): void { + rebuildChatFromMessages(options: { reuseSettledComponents?: boolean } = {}): void { // Mid-stream rebuilds (e.g. `/shake`, theme/setting changes that touch the // transcript) replay only committed `state.messages`. The agent's in-flight // `streamMessage` and its still-pending tool calls live OUTSIDE @@ -1690,7 +1691,7 @@ export class InteractiveMode implements InteractiveModeContext { const context = this.viewSession.buildTranscriptSessionContext({ collapseCompactedHistory: settings.get("display.collapseCompacted"), }); - this.renderSessionContext(context); + this.renderSessionContext(context, { reuseSettledComponents: options.reuseSettledComponents }); for (const child of liveComponents) { this.chatContainer.addChild(child); } @@ -3902,6 +3903,7 @@ export class InteractiveMode implements InteractiveModeContext { } resetTranscript(): void { + this.transcriptMessageComponents = new WeakMap(); this.chatContainer.dispose(); this.chatContainer.clear(); } @@ -4127,14 +4129,18 @@ export class InteractiveMode implements InteractiveModeContext { addMessageToChat( message: AgentMessage, - options?: { populateHistory?: boolean; imageLinks?: readonly (string | undefined)[] }, + options?: { + populateHistory?: boolean; + imageLinks?: readonly (string | undefined)[]; + reuseSettledComponent?: boolean; + }, ): Component[] { return this.#uiHelpers.addMessageToChat(message, options); } renderSessionContext( sessionContext: SessionContext, - options?: { updateFooter?: boolean; populateHistory?: boolean }, + options?: { updateFooter?: boolean; populateHistory?: boolean; reuseSettledComponents?: boolean }, ): void { for (const message of sessionContext.messages) { this.noteDisplayableThinkingContent(message); diff --git a/packages/coding-agent/src/modes/types.ts b/packages/coding-agent/src/modes/types.ts index 57ed56bb3..b4c9dfa0f 100644 --- a/packages/coding-agent/src/modes/types.ts +++ b/packages/coding-agent/src/modes/types.ts @@ -176,6 +176,8 @@ export interface InteractiveModeContext { noteDisplayableThinkingContent(message: AgentMessage): boolean; proseOnlyThinking: boolean; compactionQueuedMessages: CompactionQueuedMessage[]; + /** Settled user/assistant components reusable across post-compaction transcript rebuilds. */ + transcriptMessageComponents: WeakMap; pendingTools: Map; pendingBashComponents: BashExecutionComponent[]; bashComponent: BashExecutionComponent | undefined; @@ -302,11 +304,15 @@ export interface InteractiveModeContext { isKnownSlashCommand(text: string): boolean; addMessageToChat( message: AgentMessage, - options?: { populateHistory?: boolean; imageLinks?: readonly (string | undefined)[] }, + options?: { + populateHistory?: boolean; + imageLinks?: readonly (string | undefined)[]; + reuseSettledComponent?: boolean; + }, ): Component[]; renderSessionContext( sessionContext: SessionContext, - options?: { updateFooter?: boolean; populateHistory?: boolean }, + options?: { updateFooter?: boolean; populateHistory?: boolean; reuseSettledComponents?: boolean }, ): void; renderInitialMessages(options?: { preserveExistingChat?: boolean; clearTerminalHistory?: boolean }): void; getUserMessageText(message: Message): string; @@ -315,7 +321,7 @@ export interface InteractiveModeContext { /** Refresh the running-subagents status badge from the active local or collab registry. */ syncRunningSubagentBadge(): void; updateEditorBorderColor(): void; - rebuildChatFromMessages(): void; + rebuildChatFromMessages(options?: { reuseSettledComponents?: boolean }): void; setTodos(todos: TodoItem[] | TodoPhase[]): void; reloadTodos(): Promise; toggleTodoExpansion(): void; diff --git a/packages/coding-agent/src/modes/utils/ui-helpers.ts b/packages/coding-agent/src/modes/utils/ui-helpers.ts index b12467485..384028e38 100644 --- a/packages/coding-agent/src/modes/utils/ui-helpers.ts +++ b/packages/coding-agent/src/modes/utils/ui-helpers.ts @@ -66,6 +66,17 @@ type QueuedMessages = { steering: string[]; followUp: string[]; }; +type AddMessageOptions = { + populateHistory?: boolean; + imageLinks?: readonly (string | undefined)[]; + reuseSettledComponent?: boolean; +}; + +type RenderSessionContextOptions = { + updateFooter?: boolean; + populateHistory?: boolean; + reuseSettledComponents?: boolean; +}; function imageLinksForMessage( message: Extract, @@ -118,10 +129,7 @@ export class UiHelpers { this.ctx.lastStatusText = text; } - addMessageToChat( - message: AgentMessage, - options?: { populateHistory?: boolean; imageLinks?: readonly (string | undefined)[] }, - ): Component[] { + addMessageToChat(message: AgentMessage, options?: AddMessageOptions): Component[] { switch (message.role) { case "bashExecution": { const component = new BashExecutionComponent(message.command, this.ctx.ui, message.excludeFromContext); @@ -233,13 +241,22 @@ export class UiHelpers { const textContent = this.ctx.getUserMessageText(message); if (textContent) { const isSynthetic = message.role === "developer" ? true : (message.synthetic ?? false); - const imageLinks = - options?.imageLinks ?? - imageLinksForMessage( - message, - this.ctx.viewSession.sessionManager.putBlobSync.bind(this.ctx.viewSession.sessionManager), - ); - const userComponent = new UserMessageComponent(textContent, isSynthetic, imageLinks); + const cached = options?.reuseSettledComponent + ? this.ctx.transcriptMessageComponents.get(message) + : undefined; + let userComponent: UserMessageComponent; + if (cached instanceof UserMessageComponent) { + userComponent = cached; + } else { + const imageLinks = + options?.imageLinks ?? + imageLinksForMessage( + message, + this.ctx.viewSession.sessionManager.putBlobSync.bind(this.ctx.viewSession.sessionManager), + ); + userComponent = new UserMessageComponent(textContent, isSynthetic, imageLinks); + this.ctx.transcriptMessageComponents.set(message, userComponent); + } this.ctx.chatContainer.addChild(userComponent); if (options?.populateHistory && message.role === "user" && !isSynthetic) { this.ctx.editor.addToHistory(textContent); @@ -248,10 +265,16 @@ export class UiHelpers { break; } case "assistant": { - const assistantComponent = createAssistantMessageComponent( - this.ctx, - splitAssistantMessageToolTimeline(message).beforeTools, - ); + const cached = options?.reuseSettledComponent + ? this.ctx.transcriptMessageComponents.get(message) + : undefined; + const assistantComponent = + cached instanceof AssistantMessageComponent + ? cached + : createAssistantMessageComponent(this.ctx, splitAssistantMessageToolTimeline(message).beforeTools); + if (cached !== assistantComponent) { + this.ctx.transcriptMessageComponents.set(message, assistantComponent); + } this.ctx.chatContainer.addChild(assistantComponent); break; } @@ -272,10 +295,7 @@ export class UiHelpers { * @param options.updateFooter Update footer state * @param options.populateHistory Add user messages to editor history */ - renderSessionContext( - sessionContext: SessionContext, - options: { updateFooter?: boolean; populateHistory?: boolean } = {}, - ): void { + renderSessionContext(sessionContext: SessionContext, options: RenderSessionContextOptions = {}): void { // Preserved: message_start handler owns this lifecycle (see #783) this.ctx.pendingTools.clear(); // Reseed the cache-invalidation baseline: this rebuild re-derives every @@ -358,7 +378,7 @@ export class UiHelpers { // Assistant messages need special handling for tool calls if (message.role === "assistant") { const timeline = splitAssistantMessageToolTimeline(message); - this.ctx.addMessageToChat(message); + this.ctx.addMessageToChat(message, { reuseSettledComponent: options.reuseSettledComponents }); const lastChild = this.ctx.chatContainer.children[this.ctx.chatContainer.children.length - 1]; const assistantComponent = lastChild instanceof AssistantMessageComponent ? lastChild : undefined; if (assistantComponent) { diff --git a/packages/coding-agent/test/compaction-lifecycle.test.ts b/packages/coding-agent/test/compaction-lifecycle.test.ts index 8658a5e8d..7c3fe3d36 100644 --- a/packages/coding-agent/test/compaction-lifecycle.test.ts +++ b/packages/coding-agent/test/compaction-lifecycle.test.ts @@ -116,8 +116,9 @@ describe("executeCompaction UI lifecycle", () => { expect(outcome).toBe("ok"); // Status container is empty once compaction resolves. expect(statusContainer.children).toHaveLength(0); - // Proof the success branch ran (rebuild happens only on the ok path). - expect(rebuildChatFromMessages).toHaveBeenCalledTimes(1); + // Post-compaction rebuilds preserve settled components so their warmed + // Markdown/layout caches survive the transcript reset. + expect(rebuildChatFromMessages).toHaveBeenCalledWith({ reuseSettledComponents: true }); // The loader was drained BEFORE the transcript rebuild, not only by the // finally that runs afterward: the status container was already empty at // the instant rebuildChatFromMessages ran (1 leaked loader without the fix). diff --git a/packages/coding-agent/test/compaction-transcript-reuse.test.ts b/packages/coding-agent/test/compaction-transcript-reuse.test.ts new file mode 100644 index 000000000..508a07644 --- /dev/null +++ b/packages/coding-agent/test/compaction-transcript-reuse.test.ts @@ -0,0 +1,66 @@ +import { beforeAll, describe, expect, it, vi } from "bun:test"; +import type { AgentMessage } from "@oh-my-pi/pi-agent-core"; +import type { Message } from "@oh-my-pi/pi-ai"; +import { TranscriptContainer } from "@oh-my-pi/pi-coding-agent/modes/components/transcript-container"; +import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types"; +import { UiHelpers } from "@oh-my-pi/pi-coding-agent/modes/utils/ui-helpers"; + +function buildContext(): InteractiveModeContext { + const chatContainer = new TranscriptContainer(); + return { + chatContainer, + transcriptMessageComponents: new WeakMap(), + getUserMessageText: (message: Message) => + message.role === "user" && typeof message.content === "string" ? message.content : "", + viewSession: { + extensionRunner: undefined, + sessionManager: { putBlobSync: () => "unused" }, + }, + ui: { requestRender: vi.fn(), imageBudget: undefined }, + settings: { get: vi.fn(() => false) }, + effectiveHideThinkingBlock: false, + proseOnlyThinking: true, + editor: { addToHistory: vi.fn() }, + } as unknown as InteractiveModeContext; +} + +beforeAll(async () => { + await initTheme(false); +}); + +describe("post-compaction transcript reuse", () => { + it("retains settled user and assistant components across a rebuild", () => { + const ctx = buildContext(); + const helpers = new UiHelpers(ctx); + const messages: AgentMessage[] = [ + { role: "user", content: "large settled user turn", timestamp: Date.now() }, + { + role: "assistant", + content: [{ type: "text", text: "## Large settled assistant turn\n\n```ts\nconst retained = true;\n```" }], + api: "anthropic-messages", + provider: "anthropic", + model: "test", + usage: { + input: 1, + output: 1, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 2, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop", + timestamp: Date.now(), + }, + ]; + + for (const message of messages) helpers.addMessageToChat(message); + const settledComponents = ctx.chatContainer.children; + ctx.chatContainer.clear(); + for (const message of messages) { + helpers.addMessageToChat(message, { reuseSettledComponent: true }); + } + + expect(ctx.chatContainer.children).toEqual(settledComponents); + }); +});