From 8d591fe3c74f795eebf0322e87a7ba0f03b4b332 Mon Sep 17 00:00:00 2001 From: brooklyn! Date: Tue, 28 Apr 2026 15:47:50 -0700 Subject: [PATCH] fix(tui): prefer raw text over Rich-rendered ANSI in TUI message display (#17111) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `turnController.recordMessageComplete` and `recordMessageDelta` both prioritised `payload.rendered` over `payload.text`. `payload.rendered` is the Rich-Console output `tui_gateway` builds for terminals that can't render markdown themselves; the TUI already renders markdown via ``. Two real bugs follow: 1. **Final answer garbled when `display.final_response_markdown: render` is set** (#16391). Raw ANSI escape sequences pass through into the React tree and the user sees overlapping coloured text instead of their answer. 2. **Streaming silently drops content.** Per-delta `rendered` is an *incremental* Rich fragment. The previous code did `this.bufRef = rendered ?? this.bufRef + text`, which on every tick replaced the whole accumulated buffer with the latest mid-sequence ANSI fragment. Long replies arrived truncated and looked half-painted — easy to miss as "model is being terse" instead of a client bug. Fix: * `recordMessageComplete` now prefers `payload.text`, falling back to `payload.rendered` only when the gateway elected not to send any. * `recordMessageDelta` always accumulates `text`; `rendered` is ignored on the streaming path entirely (Ink does its own markdown render via `` / `streamingMarkdown.tsx`). Tests: * `prefers raw text over Rich-rendered ANSI on message.complete` — the assistant message reflects raw markdown, not ANSI. * `falls back to payload.rendered when text is missing` — preserves the legacy "no `text`, only ANSI" path used by some adapters. * `always accumulates raw text in message.delta and ignores rendered` — pre-fix code would have made this assertion fail because each delta overwrote the buffer. Validation: `npm run type-check` clean, `npm test --run` 392/392 pass. --- .../createGatewayEventHandler.test.ts | 42 +++++++++++++++++++ ui-tui/src/app/turnController.ts | 17 ++++++-- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts index ffda1055a0..f2326dc5ac 100644 --- a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts +++ b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts @@ -314,6 +314,48 @@ describe('createGatewayEventHandler', () => { expect(messages.some(m => m.includes('FileNotFoundError'))).toBe(true) }) + it('prefers raw text over Rich-rendered ANSI on message.complete (#16391)', () => { + const appended: Msg[] = [] + const onEvent = createGatewayEventHandler(buildCtx(appended)) + const raw = 'Hermes here.\n\nLine two.' + // Rich-rendered ANSI (`final_response_markdown: render`) used to win, + // which left visible escape codes in Ink output. Raw text must win. + const rendered = '\u001b[33mHermes here.\u001b[0m\n\n\u001b[2mLine two.\u001b[0m' + + onEvent({ payload: { rendered, text: raw }, type: 'message.complete' } as any) + + const assistant = appended.find(msg => msg.role === 'assistant') + expect(assistant?.text).toBe(raw) + expect(assistant?.text).not.toContain('\u001b[') + }) + + it('falls back to payload.rendered when text is missing on message.complete', () => { + const appended: Msg[] = [] + const onEvent = createGatewayEventHandler(buildCtx(appended)) + const rendered = 'fallback when gateway omitted text' + + onEvent({ payload: { rendered }, type: 'message.complete' } as any) + + const assistant = appended.find(msg => msg.role === 'assistant') + expect(assistant?.text).toBe(rendered) + }) + + it('always accumulates raw text in message.delta and ignores `rendered` (#16391)', () => { + const appended: Msg[] = [] + const onEvent = createGatewayEventHandler(buildCtx(appended)) + + // Stream of partial text deltas; each delta carries an incremental + // Rich-ANSI fragment. Pre-fix code would replace the whole bufRef + // with the latest fragment, dropping prior text. + onEvent({ payload: { rendered: '\u001b[33mFi\u001b[0m', text: 'Fi' }, type: 'message.delta' } as any) + onEvent({ payload: { rendered: '\u001b[33mrst.\u001b[0m', text: 'rst.' }, type: 'message.delta' } as any) + onEvent({ payload: { text: ' second.' }, type: 'message.delta' } as any) + onEvent({ payload: {}, type: 'message.complete' } as any) + + const assistant = appended.find(msg => msg.role === 'assistant') + expect(assistant?.text).toBe('First. second.') + }) + it('anchors inline_diff as its own segment where the edit happened', () => { const appended: Msg[] = [] const onEvent = createGatewayEventHandler(buildCtx(appended)) diff --git a/ui-tui/src/app/turnController.ts b/ui-tui/src/app/turnController.ts index dbd5e1faf0..b9e0aa04c1 100644 --- a/ui-tui/src/app/turnController.ts +++ b/ui-tui/src/app/turnController.ts @@ -431,7 +431,13 @@ class TurnController { recordMessageComplete(payload: { rendered?: string; reasoning?: string; text?: string }) { this.closeReasoningSegment() - const rawText = (payload.rendered ?? payload.text ?? this.bufRef).trimStart() + // Ink renders markdown via ; the gateway's Rich-rendered ANSI + // (`payload.rendered`) is for terminals that can't. Prioritising + // `rendered` here garbles output whenever a user opts into + // `display.final_response_markdown: render` because raw ANSI escapes + // pass through into the React tree. Prefer raw text and fall back + // only when the gateway elected not to send any (#16391). + const rawText = (payload.text ?? payload.rendered ?? this.bufRef).trimStart() const split = splitReasoning(rawText) const finalText = finalTail(split.text, this.segmentMessages) const existingReasoning = this.reasoningText.trim() || String(payload.reasoning ?? '').trim() @@ -516,7 +522,7 @@ class TurnController { return { finalMessages, finalText, wasInterrupted } } - recordMessageDelta({ rendered, text }: { rendered?: string; text?: string }) { + recordMessageDelta({ text }: { rendered?: string; text?: string }) { if (this.interrupted || !text) { return } @@ -524,7 +530,12 @@ class TurnController { this.pruneTransient() this.endReasoningPhase() - this.bufRef = rendered ?? this.bufRef + text + // Always accumulate the raw text delta. The pre-#16391 path replaced + // the entire buffer with `rendered` (an *incremental* Rich ANSI + // fragment), which on every tick discarded everything streamed so far + // — visible as overlapping coloured text and lost prose under + // `display.final_response_markdown: render`. + this.bufRef += text if (getUiState().streaming) { this.scheduleStreaming()