fix(tui): prefer raw text over rendered ANSI in TUI message display#16392
fix(tui): prefer raw text over rendered ANSI in TUI message display#16392briandevans wants to merge 2 commits into
Conversation
TUI's Ink renderer cannot handle Rich ANSI control sequences (cursor-up \x1b[A, line-clear \x1b[K) emitted when final_response_markdown: render is active. Two sites in turnController.ts had the rendered/text priority inverted: - recordMessageComplete picked payload.rendered first, passing ANSI escape sequences to Ink which garbled them as overlapping coloured text. - recordMessageDelta replaced the accumulated text buffer with an ANSI fragment on every streaming chunk, discarding prior text and injecting a mid-sequence escape string. Fix: prefer payload.text in recordMessageComplete and always accumulate raw text in recordMessageDelta, leaving rendered unused in the TUI path. Closes NousResearch#16391 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes garbled output in the Ink-based TUI when display.final_response_markdown: render causes the gateway to include Rich-generated ANSI in payload.rendered, by ensuring the TUI prefers raw markdown (payload.text) for both complete and streaming messages.
Changes:
- Prefer
payload.textoverpayload.renderedinTurnController.recordMessageComplete. - Always accumulate streaming output from
payload.textinTurnController.recordMessageDelta(ignorerenderedfragments). - Add Vitest coverage to ensure complete/delta events prefer/accumulate raw text over ANSI-rendered output.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ui-tui/src/app/turnController.ts | Changes message buffering to prioritize raw text over ANSI rendered for TUI safety. |
| ui-tui/src/tests/createGatewayEventHandler.test.ts | Adds regression tests validating text preference and streaming accumulation behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| this.bufRef = rendered ?? this.bufRef + text | ||
| this.bufRef = this.bufRef + text |
There was a problem hiding this comment.
recordMessageDelta no longer uses rendered, but the parameter destructuring/type still includes it. Consider removing rendered from the signature to avoid implying it affects buffering/streaming behavior.
After the raw-text-preferred change, rendered is no longer read inside the function. Remove it from the destructure so the signature reflects actual behaviour and callers are not misled. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@copilot Finding addressed in commit Finding (line 519 — unused |
|
Closing — already on main as @OutThisLife's #17111 (commit 8d591fe). The merged change lands the exact same swap in Thanks @OutThisLife. |
Summary
Fixes #16391 — TUI garbles output when
final_response_markdown: renderis active.When this config key is set, the gateway sends a
payload.renderedfield containing Rich-generated ANSI output (cursor movement\x1b[A, line clear\x1b[K, colour codes). The Ink-based TUI renderer does not support these complex escape sequences butturnController.tshadrenderedtaking priority overtextin two places:recordMessageComplete(line 426):payload.rendered ?? payload.text ?? this.bufRef→ picked ANSI over raw markdown, Ink rendered it as garbled overlapping text.recordMessageDelta(line 519):rendered ?? this.bufRef + text→ replaced the accumulated streaming buffer with an incomplete ANSI fragment on every chunk, discarding all prior text.Changes
recordMessageComplete: flipped priority topayload.text ?? payload.rendered ?? this.bufRefso TUI always gets raw markdown.recordMessageDelta: removed therendered ??branch; sincetextis already guard-checked above (line 515), we always accumulate cleanly viathis.bufRef + text.Tests
Two new cases added to
createGatewayEventHandler.test.ts:message.complete prefers text over rendered to avoid ANSI garble in TUImessage.delta accumulates raw text and ignores rendered ANSI fragmentsBoth pass; no pre-existing tests broken (7 failures due to missing
dist/ink-bundle.jsbuild artifact are pre-existing onmain).Platforms affected
TUI only (
hermes --tui). CLI and gateway platforms handle Rich ANSI correctly via their terminal emulators — this change does not affect them sincerenderedis never injected into those paths.