fix(tui): prefer raw text over Rich-rendered ANSI in TUI message display (#17111)
`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 `<Md>`. 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 `<Md>` / `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.
This commit is contained in:
parent
15ef11a8b8
commit
8d591fe3c7
@ -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))
|
||||
|
||||
@ -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 <Md>; 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()
|
||||
|
||||
Loading…
Reference in New Issue
Block a user