diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d3278878d..d5ec8fcf2 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,6 +1,9 @@ # Changelog ## [Unreleased] +### Fixed + +- Fixed streaming assistant responses leaving duplicated tail rows in WSL/Windows Terminal scrollback by enabling eager native-scrollback rebuilds while assistant text is actively streaming ([#1615](https://github.com/can1357/oh-my-pi/issues/1615)). ### Fixed diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index 603c3ebae..4c24d3c50 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -25,12 +25,14 @@ type AgentSessionEventKind = AgentSessionEvent["type"]; const IRC_MESSAGE_VISIBLE_TTL_MS = 10_000; -// Events that change which foreground tools are executing, or that reset a turn. -// The eager native-scrollback rebuild mode is recomputed only on these — other -// events (assistant text streaming, IRC, notices) leave it untouched so plain -// streaming keeps the no-yank deferral. -const TOOL_RENDER_MODE_EVENTS: Record = { +// Events that change foreground streaming state, or that reset a turn. The TUI +// eager native-scrollback rebuild mode is recomputed only on these so unrelated +// IRC/notices/status refreshes do not toggle scrollback replay policy. +const STREAM_RENDER_MODE_EVENTS: Record = { agent_start: true, + agent_end: true, + message_start: true, + message_end: true, tool_execution_start: true, tool_execution_update: true, tool_execution_end: true, @@ -46,6 +48,7 @@ export class EventController { #renderedCustomMessages = new Set(); #lastIntent: string | undefined = undefined; #backgroundToolCallIds = new Set(); + #assistantMessageStreaming = false; #readToolCallArgs = new Map>(); #readToolCallAssistantComponents = new Map(); #lastAssistantComponent: AssistantMessageComponent | undefined = undefined; @@ -169,24 +172,27 @@ export class EventController { const run = this.#handlers[event.type] as (e: AgentSessionEvent) => Promise; await run(event); - // While a foreground tool is executing, its streaming result re-renders and can - // re-lay-out rows that already scrolled into native scrollback. Let the TUI - // rebuild history on those offscreen edits (a snap to the tail is acceptable - // mid-tool) instead of deferring, which would leave stale/duplicated rows. - // Background-running tools are excluded so their late async updates — and the - // assistant text that streams alongside them — keep the no-yank deferral; - // agent_start resets the mode at every turn boundary. - if (TOOL_RENDER_MODE_EVENTS[event.type]) { + // While assistant text or a foreground tool is streaming, rows above the + // viewport can re-layout after they have already entered native scrollback + // (Markdown fences, wrapping, previews). Let the TUI rebuild history on + // those offscreen edits instead of deferring, which otherwise leaves stale + // tail rows duplicated above the live viewport. + // Background-running tools are excluded so late async updates outside the + // active foreground stream keep the no-yank deferral; agent_start resets + // the mode at every turn boundary. + if (STREAM_RENDER_MODE_EVENTS[event.type]) { this.#refreshToolRenderMode(); } } #refreshToolRenderMode(): void { - let foregroundToolActive = false; - for (const toolCallId of this.ctx.pendingTools.keys()) { - if (!this.#backgroundToolCallIds.has(toolCallId)) { - foregroundToolActive = true; - break; + let foregroundToolActive = this.#assistantMessageStreaming; + if (!foregroundToolActive) { + for (const toolCallId of this.ctx.pendingTools.keys()) { + if (!this.#backgroundToolCallIds.has(toolCallId)) { + foregroundToolActive = true; + break; + } } } this.ctx.ui.setEagerNativeScrollbackRebuild(foregroundToolActive); @@ -196,6 +202,7 @@ export class EventController { this.#lastIntent = undefined; this.#readToolCallArgs.clear(); this.#readToolCallAssistantComponents.clear(); + this.#assistantMessageStreaming = false; this.#lastAssistantComponent = undefined; if (this.ctx.retryEscapeHandler) { this.ctx.editor.onEscape = this.ctx.retryEscapeHandler; @@ -268,6 +275,7 @@ export class EventController { this.ctx.ui.requestRender(); } else if (event.message.role === "assistant") { this.#lastThinkingCount = 0; + this.#assistantMessageStreaming = true; this.#resetReadGroup(); this.ctx.streamingComponent = new AssistantMessageComponent(undefined, this.ctx.hideThinkingBlock, () => this.ctx.ui.requestRender(), @@ -414,6 +422,9 @@ export class EventController { async #handleMessageEnd(event: Extract): Promise { if (event.message.role === "user") return; + if (event.message.role === "assistant") { + this.#assistantMessageStreaming = false; + } if (this.ctx.streamingComponent && event.message.role === "assistant") { this.ctx.streamingMessage = event.message; let errorMessage: string | undefined; @@ -596,8 +607,8 @@ export class EventController { } } } - async #handleAgentEnd(_event: Extract): Promise { + this.#assistantMessageStreaming = false; if (this.ctx.loadingAnimation) { this.ctx.loadingAnimation.stop(); this.ctx.loadingAnimation = undefined; diff --git a/packages/coding-agent/test/modes/controllers/event-controller-tool-render-mode.test.ts b/packages/coding-agent/test/modes/controllers/event-controller-tool-render-mode.test.ts index 2927e9ec8..f0bb4d15c 100644 --- a/packages/coding-agent/test/modes/controllers/event-controller-tool-render-mode.test.ts +++ b/packages/coding-agent/test/modes/controllers/event-controller-tool-render-mode.test.ts @@ -1,4 +1,5 @@ -import { afterEach, describe, expect, it, vi } from "bun:test"; +import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; +import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { EventController } from "@oh-my-pi/pi-coding-agent/modes/controllers/event-controller"; import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types"; import type { AgentSessionEvent } from "@oh-my-pi/pi-coding-agent/session/agent-session"; @@ -6,11 +7,23 @@ import type { AgentSessionEvent } from "@oh-my-pi/pi-coding-agent/session/agent- function createContext() { const setEagerNativeScrollbackRebuild = vi.fn(); const pendingTools = new Map(); + const chatContainer = { addChild: vi.fn(), removeChild: vi.fn() }; const ctx = { isInitialized: true, + isBackgrounded: false, statusLine: { invalidate: vi.fn() }, updateEditorTopBorder: vi.fn(), pendingTools, + chatContainer, + hideThinkingBlock: false, + editor: { getText: vi.fn(() => "") }, + flushPendingModelSwitch: vi.fn(), + session: { + agent: { state: { messages: [] } }, + isCompacting: false, + isTtsrAbortPending: false, + retryAttempt: 0, + }, ui: { setEagerNativeScrollbackRebuild, requestRender: vi.fn() }, } as unknown as InteractiveModeContext; return { ctx, pendingTools, setEagerNativeScrollbackRebuild }; @@ -25,8 +38,32 @@ const REFRESH_TRIGGER = { partialResult: { content: [], details: {} }, } as unknown as AgentSessionEvent; +const ASSISTANT_MESSAGE = { + role: "assistant", + content: [{ type: "text", text: "" }], + api: "anthropic-messages", + provider: "anthropic", + model: "test-model", + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop", + timestamp: 0, +} as const; + describe("EventController tool render mode", () => { + beforeEach(async () => { + resetSettingsForTest(); + await Settings.init({ inMemory: true }); + }); + afterEach(() => { + resetSettingsForTest(); vi.restoreAllMocks(); }); @@ -42,4 +79,32 @@ describe("EventController tool render mode", () => { await controller.handleEvent(REFRESH_TRIGGER); expect(setEagerNativeScrollbackRebuild).toHaveBeenLastCalledWith(false); }); + + it("enables eager native scrollback rebuild while assistant text is streaming", async () => { + const { ctx, setEagerNativeScrollbackRebuild } = createContext(); + const controller = new EventController(ctx); + + await controller.handleEvent({ + type: "message_start", + message: ASSISTANT_MESSAGE, + } as unknown as AgentSessionEvent); + expect(setEagerNativeScrollbackRebuild).toHaveBeenLastCalledWith(true); + + await controller.handleEvent({ type: "message_end", message: ASSISTANT_MESSAGE } as unknown as AgentSessionEvent); + expect(setEagerNativeScrollbackRebuild).toHaveBeenLastCalledWith(false); + }); + + it("resets eager native scrollback rebuild when a stream ends without assistant message_end", async () => { + const { ctx, setEagerNativeScrollbackRebuild } = createContext(); + const controller = new EventController(ctx); + + await controller.handleEvent({ + type: "message_start", + message: ASSISTANT_MESSAGE, + } as unknown as AgentSessionEvent); + expect(setEagerNativeScrollbackRebuild).toHaveBeenLastCalledWith(true); + + await controller.handleEvent({ type: "agent_end" } as unknown as AgentSessionEvent); + expect(setEagerNativeScrollbackRebuild).toHaveBeenLastCalledWith(false); + }); });