Skip to content

Commit c92dbea

Browse files
authored
tui2: stop baking streaming wraps; reflow agent markdown (#8761)
Background Streaming assistant prose in tui2 was being rendered with viewport-width wrapping during streaming, then stored in history cells as already split `Line`s. Those width-derived breaks became indistinguishable from hard newlines, so the transcript could not "un-split" on resize. This also degraded copy/paste, since soft wraps looked like hard breaks. What changed - Introduce width-agnostic `MarkdownLogicalLine` output in `tui2/src/markdown_render.rs`, preserving markdown wrap semantics: initial/subsequent indents, per-line style, and a preformatted flag. - Update the streaming collector (`tui2/src/markdown_stream.rs`) to emit logical lines (newline-gated) and remove any captured viewport width. - Update streaming orchestration (`tui2/src/streaming/*`) to queue and emit logical lines, producing `AgentMessageCell::new_logical(...)`. - Make `AgentMessageCell` store logical lines and wrap at render time in `HistoryCell::transcript_lines_with_joiners(width)`, emitting joiners so copy/paste can join soft-wrap continuations correctly. Overlay deferral When an overlay is active, defer *cells* (not rendered `Vec<Line>`) and render them at overlay close time. This avoids baking width-derived wraps based on a stale width. Tests + docs - Add resize/reflow regression tests + snapshots for streamed agent output. - Expand module/API docs for the new logical-line streaming pipeline and clarify joiner semantics. - Align scrollback-related docs/comments with current tui2 behavior (main draw loop does not flush queued "history lines" to the terminal). More details See `codex-rs/tui2/docs/streaming_wrapping_design.md` for the full problem statement and solution approach, and `codex-rs/tui2/docs/tui_viewport_and_history.md` for viewport vs printed output behavior.
1 parent 771f1ca commit c92dbea

13 files changed

+885
-342
lines changed
Lines changed: 168 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -1,85 +1,169 @@
1-
# Streaming Markdown Wrapping & Animation – TUI2 Notes
2-
3-
This document mirrors the original `tui/streaming_wrapping_design.md` and
4-
captures how the same concerns apply to the new `tui2` crate. It exists so that
5-
future viewport and streaming work in TUI2 can rely on the same context without
6-
having to cross‑reference the legacy TUI implementation.
7-
8-
At a high level, the design constraints are the same:
9-
10-
- Streaming agent responses are rendered incrementally, with an animation loop
11-
that reveals content over time.
12-
- Non‑streaming history cells are rendered width‑agnostically and wrapped only
13-
at display time, so they reflow correctly when the terminal is resized.
14-
- Streaming content should eventually follow the same “wrap on display” model so
15-
the transcript reflows consistently across width changes, without regressing
16-
animation or markdown semantics.
17-
18-
## 1. Where streaming is implemented in TUI2
19-
20-
TUI2 keeps the streaming pipeline conceptually aligned with the legacy TUI but
21-
in a separate crate:
22-
23-
- `tui2/src/markdown_stream.rs` implements the markdown streaming collector and
24-
animation controller for agent deltas.
25-
- `tui2/src/chatwidget.rs` integrates streamed content into the transcript via
26-
`HistoryCell` implementations.
27-
- `tui2/src/history_cell.rs` provides the concrete history cell types used by
28-
the inline transcript and overlays.
29-
- `tui2/src/wrapping.rs` contains the shared text wrapping utilities used by
30-
both streaming and non‑streaming render paths:
31-
- `RtOptions` describes viewport‑aware wrapping (width, indents, algorithm).
32-
- `word_wrap_line`, `word_wrap_lines`, and `word_wrap_lines_borrowed` provide
33-
span‑aware wrapping that preserves markdown styling and emoji width.
34-
35-
As in the original TUI, the key tension is between:
36-
37-
- **Pre‑wrapping streamed content at commit time** (simpler animation, but
38-
baked‑in splits that don’t reflow), and
39-
- **Deferring wrapping to render time** (better reflow, but requires a more
40-
sophisticated streaming cell model or recomputation on each frame).
41-
42-
## 2. Current behavior and limitations
43-
44-
TUI2 is intentionally conservative for now:
45-
46-
- Streaming responses use the same markdown streaming and wrapping utilities as
47-
the legacy TUI, with width decisions made near the streaming collector.
48-
- The transcript viewport (`App::render_transcript_cells` in
49-
`tui2/src/app.rs`) always uses `word_wrap_lines_borrowed` against the
50-
current `Rect` width, so:
51-
- Non‑streaming cells reflow naturally on resize.
52-
- Streamed cells respect whatever wrapping was applied when their lines were
53-
constructed, and may not fully “un‑wrap” if that work happened at a fixed
54-
width earlier in the pipeline.
55-
56-
This means TUI2 shares the same fundamental limitation documented in the
57-
original design note: streamed paragraphs can retain historical wrap decisions
58-
made at the time they were streamed, even if the viewport later grows wider.
59-
60-
## 3. Design directions (forward‑looking)
61-
62-
The options outlined in the legacy document apply here as well:
63-
64-
1. **Keep the current behavior but clarify tests and documentation.**
65-
- Ensure tests in `tui2/src/markdown_stream.rs`, `tui2/src/markdown_render.rs`,
66-
`tui2/src/history_cell.rs`, and `tui2/src/wrapping.rs` encode the current
67-
expectations around streaming, wrapping, and emoji / markdown styling.
68-
2. **Move towards width‑agnostic streaming cells.**
69-
- Introduce a dedicated streaming history cell that stores the raw markdown
70-
buffer and lets `HistoryCell::display_lines(width)` perform both markdown
71-
rendering and wrapping based on the current viewport width.
72-
- Keep the commit animation logic expressed in terms of “logical” positions
73-
(e.g., number of tokens or lines committed) rather than pre‑wrapped visual
74-
lines at a fixed width.
75-
3. **Hybrid “visual line count” model.**
76-
- Track committed visual lines as a scalar and re‑render the streamed prefix
77-
at the current width, revealing only the first `N` visual lines on each
78-
animation tick.
79-
80-
TUI2 does not yet implement these refactors; it intentionally stays close to
81-
the legacy behavior while the viewport work (scrolling, selection, exit
82-
transcripts) is being ported. This document exists to make that trade‑off
83-
explicit for TUI2 and to provide a natural home for any TUI2‑specific streaming
84-
wrapping notes as the design evolves.
1+
# Streaming Wrapping Reflow (tui2)
852

3+
This document describes a correctness bug in `codex-rs/tui2` and the chosen fix:
4+
while streaming assistant markdown, soft-wrap decisions were effectively persisted as hard line
5+
breaks, so resizing the viewport could not reflow prose.
6+
7+
## Goal
8+
9+
- Resizing the viewport reflows transcript prose (including streaming assistant output).
10+
- Width-derived breaks are always treated as *soft wraps* (not logical newlines).
11+
- Copy/paste continues to treat soft wraps as joinable (via joiners), and hard breaks as newlines.
12+
13+
Non-goals:
14+
15+
- Reflowing terminal scrollback that has already been printed.
16+
- Reflowing content that is intentionally treated as preformatted (e.g., code blocks, raw stdout).
17+
18+
## Background: where reflow happens in tui2
19+
20+
TUI2 renders the transcript as a list of `HistoryCell`s:
21+
22+
1. A cell stores width-agnostic content (string, diff, logical lines, etc.).
23+
2. At draw time (and on resize), `transcript_render` asks each cell for lines at the *current*
24+
width (ideally via `HistoryCell::transcript_lines_with_joiners(width)`).
25+
3. `TranscriptViewCache` caches the wrapped visual lines keyed by width; a width change triggers a
26+
rebuild.
27+
28+
This only works if cells do *not* persist width-derived wrapping inside their stored state.
29+
30+
## The bug: soft wraps became hard breaks during streaming
31+
32+
Ratatui represents multi-line content as `Vec<Line>`. If we split a paragraph into multiple `Line`s
33+
because the viewport is narrow, that split is indistinguishable from an explicit newline unless we
34+
also carry metadata describing which breaks were “soft”.
35+
36+
Streaming assistant output used to generate already-wrapped `Line`s and store them inside the
37+
history cell. Later, when the viewport became wider, the transcript renderer could not “un-split”
38+
those baked lines — they looked like hard breaks.
39+
40+
## Chosen solution (A, F1): stream logical markdown lines; wrap in the cell at render-time
41+
42+
User choice recap:
43+
44+
- **A**: Keep append-only streaming (new history cell per commit tick), but make the streamed data
45+
width-agnostic.
46+
- **F1**: Make the agent message cell responsible for wrapping-to-width so transcript-level wrapping
47+
can be a no-op for it.
48+
49+
### Key idea: separate markdown parsing from wrapping
50+
51+
We introduce a width-agnostic “logical markdown line” representation that preserves the metadata
52+
needed to wrap correctly later:
53+
54+
- `codex-rs/tui2/src/markdown_render.rs`
55+
- `MarkdownLogicalLine { content, initial_indent, subsequent_indent, line_style, is_preformatted }`
56+
- `render_markdown_logical_lines(input: &str) -> Vec<MarkdownLogicalLine>`
57+
58+
This keeps:
59+
60+
- hard breaks (paragraph/list boundaries, explicit newlines),
61+
- markdown indentation rules for wraps (list markers, nested lists, blockquotes),
62+
- preformatted runs (code blocks) stable.
63+
64+
### Updated streaming pipeline
65+
66+
- `codex-rs/tui2/src/markdown_stream.rs`
67+
- `MarkdownStreamCollector` is newline-gated (no change), but now commits
68+
`Vec<MarkdownLogicalLine>` instead of already-wrapped `Vec<Line>`.
69+
- Width is removed from the collector; wrapping is not performed during streaming.
70+
71+
- `codex-rs/tui2/src/streaming/controller.rs`
72+
- Emits `AgentMessageCell::new_logical(...)` containing logical lines.
73+
74+
- `codex-rs/tui2/src/history_cell.rs`
75+
- `AgentMessageCell` stores `Vec<MarkdownLogicalLine>`.
76+
- `HistoryCell::transcript_lines_with_joiners(width)` wraps each logical line at the current
77+
width using `word_wrap_line_with_joiners` and composes indents as:
78+
- transcript gutter prefix (`` / ` `), plus
79+
- markdown-provided initial/subsequent indents.
80+
- Preformatted logical lines are rendered without wrapping.
81+
82+
Result: on resize, the transcript cache rebuilds against the new width and the agent output reflows
83+
correctly because the stored content contains no baked soft wraps.
84+
85+
## Overlay deferral fix (D): defer cells, not rendered lines
86+
87+
When an overlay (transcript/static) is active, TUI2 is in alt screen and the normal terminal buffer
88+
is not visible. Historically, `tui2` attempted to queue “history to print” for the normal buffer by
89+
deferring *rendered lines*, which baked the then-current width.
90+
91+
User choice recap:
92+
93+
- **D**: Store deferred *cells* and render them at overlay close time.
94+
95+
Implementation:
96+
97+
- `codex-rs/tui2/src/app.rs`
98+
- `deferred_history_cells: Vec<Arc<dyn HistoryCell>>` (replaces `deferred_history_lines`).
99+
- `AppEvent::InsertHistoryCell` pushes cells into the deferral list when `overlay.is_some()`.
100+
101+
- `codex-rs/tui2/src/app_backtrack.rs`
102+
- `close_transcript_overlay` renders deferred cells at the *current* width when closing the
103+
overlay, then queues the resulting lines for the normal terminal buffer.
104+
105+
Note: as of today, `Tui::insert_history_lines` queues lines but `Tui::draw` does not flush them into
106+
the terminal (see `codex-rs/tui2/src/tui.rs`). This section is therefore best read as “behavior we
107+
want when/if scrollback printing is re-enabled”, not a guarantee that content is printed during the
108+
main TUI loop. For the current intended behavior around printing, see
109+
`codex-rs/tui2/docs/tui_viewport_and_history.md`.
110+
111+
## Tests (G2)
112+
113+
User choice recap:
114+
115+
- **G2**: Add resize reflow tests + snapshot coverage.
116+
117+
Added coverage:
118+
119+
- `codex-rs/tui2/src/history_cell.rs`
120+
- `agent_message_cell_reflows_streamed_prose_on_resize`
121+
- `agent_message_cell_reflows_streamed_prose_vt100_snapshot`
122+
123+
These assert that a streamed agent cell produces fewer visual lines at wider widths and provide
124+
snapshots showing reflow for list items and blockquotes.
125+
126+
## Audit: other `HistoryCell`s and width-baked paths
127+
128+
This section answers “what else might behave like this?” up front.
129+
130+
### History cells
131+
132+
- `AgentMessageCell` (`codex-rs/tui2/src/history_cell.rs`): **was affected**; now stores logical
133+
markdown lines and wraps at render time.
134+
- `UserHistoryCell` (`codex-rs/tui2/src/history_cell.rs`): wraps at render time from stored `String`
135+
using `word_wrap_lines_with_joiners` (reflowable).
136+
- `ReasoningSummaryCell` (`codex-rs/tui2/src/history_cell.rs`): renders from stored `String` on each
137+
call; it does call `append_markdown(..., Some(width))`, but that wrapping is recomputed per width
138+
(reflowable).
139+
- `PrefixedWrappedHistoryCell` (`codex-rs/tui2/src/history_cell.rs`): wraps at render time and
140+
returns joiners (reflowable).
141+
- `PlainHistoryCell` (`codex-rs/tui2/src/history_cell.rs`): stores `Vec<Line>` and returns it
142+
unchanged (not reflowable by design; used for already-structured/preformatted output).
143+
144+
Rule of thumb: any cell that stores already-wrapped `Vec<Line>` for prose is a candidate for the
145+
same bug; cells that store source text or logical lines and compute wrapping inside
146+
`display_lines(width)` are safe.
147+
148+
### Width-baked output outside the transcript model
149+
150+
Even with the streaming fix, some paths are inherently width-baked:
151+
152+
- Printed transcript after exit (`codex-rs/tui2/src/app.rs`): `AppExitInfo.session_lines` is rendered
153+
once using the final width and then printed; it cannot reflow afterward.
154+
- Optional scrollback insertion helper (`codex-rs/tui2/src/insert_history.rs`): once ANSI is written
155+
to the terminal, that output cannot be reflowed later. This helper is currently used for
156+
deterministic ANSI emission (`write_spans`) and tests; it is not wired into the main TUI draw
157+
loop.
158+
- Static overlays (`codex-rs/tui2/src/pager_overlay.rs`): reflow depends on whether callers provided
159+
width-agnostic input; pre-split `Vec<Line>` cannot be “un-split” within the overlay.
160+
161+
## Deferred / follow-ups
162+
163+
The fix above is sufficient to unblock correct reflow on resize. Remaining choices can be deferred:
164+
165+
- Streaming granularity: one logical line can wrap into multiple visual lines, so “commit tick”
166+
updates can appear in larger chunks than before. If this becomes a UX issue, we can add a render-
167+
time “progressive reveal” layer without reintroducing width baking.
168+
- Expand logical-line rendering to other markdown-ish cells if needed (e.g., unify `append_markdown`
169+
usage), but only if we find a concrete reflow bug beyond `AgentMessageCell`.

codex-rs/tui2/src/app.rs

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -344,10 +344,22 @@ pub(crate) struct App {
344344
transcript_copy_action: TranscriptCopyAction,
345345
transcript_scrollbar_ui: TranscriptScrollbarUi,
346346

347-
// Pager overlay state (Transcript or Static like Diff)
347+
// Pager overlay state (Transcript or Static like Diff).
348348
pub(crate) overlay: Option<Overlay>,
349-
pub(crate) deferred_history_lines: Vec<Line<'static>>,
350-
has_emitted_history_lines: bool,
349+
/// History cells received while an overlay is active.
350+
///
351+
/// While in an alt-screen overlay, the normal terminal buffer is not visible.
352+
/// Instead we queue the incoming cells here and, on overlay close, render them at the *current*
353+
/// width and queue them in one batch via `Tui::insert_history_lines`.
354+
///
355+
/// This matters for correctness if/when scrollback printing is enabled: if we deferred
356+
/// already-rendered `Vec<Line>`, we'd bake viewport-width wrapping based on the width at the
357+
/// time the cell arrived (which may differ from the width when the overlay closes).
358+
pub(crate) deferred_history_cells: Vec<Arc<dyn HistoryCell>>,
359+
/// True once at least one history cell has been inserted into terminal scrollback.
360+
///
361+
/// Used to decide whether to insert an extra blank separator line when flushing deferred cells.
362+
pub(crate) has_emitted_history_lines: bool,
351363

352364
pub(crate) enhanced_keys_supported: bool,
353365

@@ -511,7 +523,7 @@ impl App {
511523
transcript_copy_action: TranscriptCopyAction::default(),
512524
transcript_scrollbar_ui: TranscriptScrollbarUi::default(),
513525
overlay: None,
514-
deferred_history_lines: Vec::new(),
526+
deferred_history_cells: Vec::new(),
515527
has_emitted_history_lines: false,
516528
commit_anim_running: Arc::new(AtomicBool::new(false)),
517529
scroll_config,
@@ -1449,21 +1461,8 @@ impl App {
14491461
tui.frame_requester().schedule_frame();
14501462
}
14511463
self.transcript_cells.push(cell.clone());
1452-
let mut display = cell.display_lines(tui.terminal.last_known_screen_size.width);
1453-
if !display.is_empty() {
1454-
// Only insert a separating blank line for new cells that are not
1455-
// part of an ongoing stream. Streaming continuations should not
1456-
// accrue extra blank lines between chunks.
1457-
if !cell.is_stream_continuation() {
1458-
if self.has_emitted_history_lines {
1459-
display.insert(0, Line::from(""));
1460-
} else {
1461-
self.has_emitted_history_lines = true;
1462-
}
1463-
}
1464-
if self.overlay.is_some() {
1465-
self.deferred_history_lines.extend(display);
1466-
}
1464+
if self.overlay.is_some() {
1465+
self.deferred_history_cells.push(cell);
14671466
}
14681467
}
14691468
AppEvent::StartCommitAnimation => {
@@ -2135,7 +2134,7 @@ mod tests {
21352134
transcript_copy_action: TranscriptCopyAction::default(),
21362135
transcript_scrollbar_ui: TranscriptScrollbarUi::default(),
21372136
overlay: None,
2138-
deferred_history_lines: Vec::new(),
2137+
deferred_history_cells: Vec::new(),
21392138
has_emitted_history_lines: false,
21402139
enhanced_keys_supported: false,
21412140
commit_anim_running: Arc::new(AtomicBool::new(false)),
@@ -2188,7 +2187,7 @@ mod tests {
21882187
transcript_copy_action: TranscriptCopyAction::default(),
21892188
transcript_scrollbar_ui: TranscriptScrollbarUi::default(),
21902189
overlay: None,
2191-
deferred_history_lines: Vec::new(),
2190+
deferred_history_cells: Vec::new(),
21922191
has_emitted_history_lines: false,
21932192
enhanced_keys_supported: false,
21942193
commit_anim_running: Arc::new(AtomicBool::new(false)),

codex-rs/tui2/src/app_backtrack.rs

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,42 @@ impl App {
123123
}
124124

125125
/// Close transcript overlay and restore normal UI.
126+
///
127+
/// Any history emitted while the overlay was open is flushed to the normal-buffer queue here.
128+
///
129+
/// Importantly, we defer *cells* (not rendered lines) so we can render them against the current
130+
/// width on close and avoid baking width-derived wrapping based on an earlier viewport size.
131+
/// (This matters if/when scrollback printing is enabled; `Tui::insert_history_lines` currently
132+
/// queues lines without printing them during the main draw loop.)
126133
pub(crate) fn close_transcript_overlay(&mut self, tui: &mut tui::Tui) {
127134
let _ = tui.leave_alt_screen();
128135
let was_backtrack = self.backtrack.overlay_preview_active;
129-
if !self.deferred_history_lines.is_empty() {
130-
let lines = std::mem::take(&mut self.deferred_history_lines);
131-
tui.insert_history_lines(lines);
136+
if !self.deferred_history_cells.is_empty() {
137+
let cells = std::mem::take(&mut self.deferred_history_cells);
138+
let width = tui.terminal.last_known_screen_size.width;
139+
let mut lines: Vec<ratatui::text::Line<'static>> = Vec::new();
140+
for cell in cells {
141+
let mut display = cell.display_lines(width);
142+
if display.is_empty() {
143+
continue;
144+
}
145+
146+
// Only insert a separating blank line for new cells that are not part of an
147+
// ongoing stream. Streaming continuations should not accrue extra blank lines
148+
// between chunks.
149+
if !cell.is_stream_continuation() {
150+
if self.has_emitted_history_lines {
151+
display.insert(0, ratatui::text::Line::from(""));
152+
} else {
153+
self.has_emitted_history_lines = true;
154+
}
155+
}
156+
157+
lines.extend(display);
158+
}
159+
if !lines.is_empty() {
160+
tui.insert_history_lines(lines);
161+
}
132162
}
133163
self.overlay = None;
134164
self.backtrack.overlay_preview_active = false;

codex-rs/tui2/src/chatwidget.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,9 +1023,9 @@ impl ChatWidget {
10231023
self.needs_final_message_separator = false;
10241024
needs_redraw = true;
10251025
}
1026-
self.stream_controller = Some(StreamController::new(
1027-
self.last_rendered_width.get().map(|w| w.saturating_sub(2)),
1028-
));
1026+
// Streaming must not capture the current viewport width: width-derived wraps are
1027+
// applied later, at render time, so the transcript can reflow on resize.
1028+
self.stream_controller = Some(StreamController::new());
10291029
}
10301030
if let Some(controller) = self.stream_controller.as_mut()
10311031
&& controller.push(&delta)

0 commit comments

Comments
 (0)