Merge remote-tracking branch 'origin/farm/5636bafd/dedupe-streaming-assistant-tail'
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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<string, true> = {
|
||||
// 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<string, true> = {
|
||||
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<string>();
|
||||
#lastIntent: string | undefined = undefined;
|
||||
#backgroundToolCallIds = new Set<string>();
|
||||
#assistantMessageStreaming = false;
|
||||
#readToolCallArgs = new Map<string, Record<string, unknown>>();
|
||||
#readToolCallAssistantComponents = new Map<string, AssistantMessageComponent>();
|
||||
#lastAssistantComponent: AssistantMessageComponent | undefined = undefined;
|
||||
@@ -169,24 +172,27 @@ export class EventController {
|
||||
|
||||
const run = this.#handlers[event.type] as (e: AgentSessionEvent) => Promise<void>;
|
||||
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<AgentSessionEvent, { type: "message_end" }>): Promise<void> {
|
||||
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<AgentSessionEvent, { type: "agent_end" }>): Promise<void> {
|
||||
this.#assistantMessageStreaming = false;
|
||||
if (this.ctx.loadingAnimation) {
|
||||
this.ctx.loadingAnimation.stop();
|
||||
this.ctx.loadingAnimation = undefined;
|
||||
|
||||
+66
-1
@@ -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<string, unknown>();
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user