mirror of
https://github.com/openai/codex.git
synced 2026-04-29 02:41:12 +03:00
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.
This commit is contained in:
@@ -1,85 +1,169 @@
|
||||
# Streaming Markdown Wrapping & Animation – TUI2 Notes
|
||||
# Streaming Wrapping Reflow (tui2)
|
||||
|
||||
This document mirrors the original `tui/streaming_wrapping_design.md` and
|
||||
captures how the same concerns apply to the new `tui2` crate. It exists so that
|
||||
future viewport and streaming work in TUI2 can rely on the same context without
|
||||
having to cross‑reference the legacy TUI implementation.
|
||||
This document describes a correctness bug in `codex-rs/tui2` and the chosen fix:
|
||||
while streaming assistant markdown, soft-wrap decisions were effectively persisted as hard line
|
||||
breaks, so resizing the viewport could not reflow prose.
|
||||
|
||||
At a high level, the design constraints are the same:
|
||||
## Goal
|
||||
|
||||
- Streaming agent responses are rendered incrementally, with an animation loop
|
||||
that reveals content over time.
|
||||
- Non‑streaming history cells are rendered width‑agnostically and wrapped only
|
||||
at display time, so they reflow correctly when the terminal is resized.
|
||||
- Streaming content should eventually follow the same “wrap on display” model so
|
||||
the transcript reflows consistently across width changes, without regressing
|
||||
animation or markdown semantics.
|
||||
- Resizing the viewport reflows transcript prose (including streaming assistant output).
|
||||
- Width-derived breaks are always treated as *soft wraps* (not logical newlines).
|
||||
- Copy/paste continues to treat soft wraps as joinable (via joiners), and hard breaks as newlines.
|
||||
|
||||
## 1. Where streaming is implemented in TUI2
|
||||
Non-goals:
|
||||
|
||||
TUI2 keeps the streaming pipeline conceptually aligned with the legacy TUI but
|
||||
in a separate crate:
|
||||
- Reflowing terminal scrollback that has already been printed.
|
||||
- Reflowing content that is intentionally treated as preformatted (e.g., code blocks, raw stdout).
|
||||
|
||||
- `tui2/src/markdown_stream.rs` implements the markdown streaming collector and
|
||||
animation controller for agent deltas.
|
||||
- `tui2/src/chatwidget.rs` integrates streamed content into the transcript via
|
||||
`HistoryCell` implementations.
|
||||
- `tui2/src/history_cell.rs` provides the concrete history cell types used by
|
||||
the inline transcript and overlays.
|
||||
- `tui2/src/wrapping.rs` contains the shared text wrapping utilities used by
|
||||
both streaming and non‑streaming render paths:
|
||||
- `RtOptions` describes viewport‑aware wrapping (width, indents, algorithm).
|
||||
- `word_wrap_line`, `word_wrap_lines`, and `word_wrap_lines_borrowed` provide
|
||||
span‑aware wrapping that preserves markdown styling and emoji width.
|
||||
## Background: where reflow happens in tui2
|
||||
|
||||
As in the original TUI, the key tension is between:
|
||||
TUI2 renders the transcript as a list of `HistoryCell`s:
|
||||
|
||||
- **Pre‑wrapping streamed content at commit time** (simpler animation, but
|
||||
baked‑in splits that don’t reflow), and
|
||||
- **Deferring wrapping to render time** (better reflow, but requires a more
|
||||
sophisticated streaming cell model or recomputation on each frame).
|
||||
1. A cell stores width-agnostic content (string, diff, logical lines, etc.).
|
||||
2. At draw time (and on resize), `transcript_render` asks each cell for lines at the *current*
|
||||
width (ideally via `HistoryCell::transcript_lines_with_joiners(width)`).
|
||||
3. `TranscriptViewCache` caches the wrapped visual lines keyed by width; a width change triggers a
|
||||
rebuild.
|
||||
|
||||
## 2. Current behavior and limitations
|
||||
This only works if cells do *not* persist width-derived wrapping inside their stored state.
|
||||
|
||||
TUI2 is intentionally conservative for now:
|
||||
## The bug: soft wraps became hard breaks during streaming
|
||||
|
||||
- Streaming responses use the same markdown streaming and wrapping utilities as
|
||||
the legacy TUI, with width decisions made near the streaming collector.
|
||||
- The transcript viewport (`App::render_transcript_cells` in
|
||||
`tui2/src/app.rs`) always uses `word_wrap_lines_borrowed` against the
|
||||
current `Rect` width, so:
|
||||
- Non‑streaming cells reflow naturally on resize.
|
||||
- Streamed cells respect whatever wrapping was applied when their lines were
|
||||
constructed, and may not fully “un‑wrap” if that work happened at a fixed
|
||||
width earlier in the pipeline.
|
||||
Ratatui represents multi-line content as `Vec<Line>`. If we split a paragraph into multiple `Line`s
|
||||
because the viewport is narrow, that split is indistinguishable from an explicit newline unless we
|
||||
also carry metadata describing which breaks were “soft”.
|
||||
|
||||
This means TUI2 shares the same fundamental limitation documented in the
|
||||
original design note: streamed paragraphs can retain historical wrap decisions
|
||||
made at the time they were streamed, even if the viewport later grows wider.
|
||||
Streaming assistant output used to generate already-wrapped `Line`s and store them inside the
|
||||
history cell. Later, when the viewport became wider, the transcript renderer could not “un-split”
|
||||
those baked lines — they looked like hard breaks.
|
||||
|
||||
## 3. Design directions (forward‑looking)
|
||||
## Chosen solution (A, F1): stream logical markdown lines; wrap in the cell at render-time
|
||||
|
||||
The options outlined in the legacy document apply here as well:
|
||||
User choice recap:
|
||||
|
||||
1. **Keep the current behavior but clarify tests and documentation.**
|
||||
- Ensure tests in `tui2/src/markdown_stream.rs`, `tui2/src/markdown_render.rs`,
|
||||
`tui2/src/history_cell.rs`, and `tui2/src/wrapping.rs` encode the current
|
||||
expectations around streaming, wrapping, and emoji / markdown styling.
|
||||
2. **Move towards width‑agnostic streaming cells.**
|
||||
- Introduce a dedicated streaming history cell that stores the raw markdown
|
||||
buffer and lets `HistoryCell::display_lines(width)` perform both markdown
|
||||
rendering and wrapping based on the current viewport width.
|
||||
- Keep the commit animation logic expressed in terms of “logical” positions
|
||||
(e.g., number of tokens or lines committed) rather than pre‑wrapped visual
|
||||
lines at a fixed width.
|
||||
3. **Hybrid “visual line count” model.**
|
||||
- Track committed visual lines as a scalar and re‑render the streamed prefix
|
||||
at the current width, revealing only the first `N` visual lines on each
|
||||
animation tick.
|
||||
- **A**: Keep append-only streaming (new history cell per commit tick), but make the streamed data
|
||||
width-agnostic.
|
||||
- **F1**: Make the agent message cell responsible for wrapping-to-width so transcript-level wrapping
|
||||
can be a no-op for it.
|
||||
|
||||
TUI2 does not yet implement these refactors; it intentionally stays close to
|
||||
the legacy behavior while the viewport work (scrolling, selection, exit
|
||||
transcripts) is being ported. This document exists to make that trade‑off
|
||||
explicit for TUI2 and to provide a natural home for any TUI2‑specific streaming
|
||||
wrapping notes as the design evolves.
|
||||
### Key idea: separate markdown parsing from wrapping
|
||||
|
||||
We introduce a width-agnostic “logical markdown line” representation that preserves the metadata
|
||||
needed to wrap correctly later:
|
||||
|
||||
- `codex-rs/tui2/src/markdown_render.rs`
|
||||
- `MarkdownLogicalLine { content, initial_indent, subsequent_indent, line_style, is_preformatted }`
|
||||
- `render_markdown_logical_lines(input: &str) -> Vec<MarkdownLogicalLine>`
|
||||
|
||||
This keeps:
|
||||
|
||||
- hard breaks (paragraph/list boundaries, explicit newlines),
|
||||
- markdown indentation rules for wraps (list markers, nested lists, blockquotes),
|
||||
- preformatted runs (code blocks) stable.
|
||||
|
||||
### Updated streaming pipeline
|
||||
|
||||
- `codex-rs/tui2/src/markdown_stream.rs`
|
||||
- `MarkdownStreamCollector` is newline-gated (no change), but now commits
|
||||
`Vec<MarkdownLogicalLine>` instead of already-wrapped `Vec<Line>`.
|
||||
- Width is removed from the collector; wrapping is not performed during streaming.
|
||||
|
||||
- `codex-rs/tui2/src/streaming/controller.rs`
|
||||
- Emits `AgentMessageCell::new_logical(...)` containing logical lines.
|
||||
|
||||
- `codex-rs/tui2/src/history_cell.rs`
|
||||
- `AgentMessageCell` stores `Vec<MarkdownLogicalLine>`.
|
||||
- `HistoryCell::transcript_lines_with_joiners(width)` wraps each logical line at the current
|
||||
width using `word_wrap_line_with_joiners` and composes indents as:
|
||||
- transcript gutter prefix (`• ` / ` `), plus
|
||||
- markdown-provided initial/subsequent indents.
|
||||
- Preformatted logical lines are rendered without wrapping.
|
||||
|
||||
Result: on resize, the transcript cache rebuilds against the new width and the agent output reflows
|
||||
correctly because the stored content contains no baked soft wraps.
|
||||
|
||||
## Overlay deferral fix (D): defer cells, not rendered lines
|
||||
|
||||
When an overlay (transcript/static) is active, TUI2 is in alt screen and the normal terminal buffer
|
||||
is not visible. Historically, `tui2` attempted to queue “history to print” for the normal buffer by
|
||||
deferring *rendered lines*, which baked the then-current width.
|
||||
|
||||
User choice recap:
|
||||
|
||||
- **D**: Store deferred *cells* and render them at overlay close time.
|
||||
|
||||
Implementation:
|
||||
|
||||
- `codex-rs/tui2/src/app.rs`
|
||||
- `deferred_history_cells: Vec<Arc<dyn HistoryCell>>` (replaces `deferred_history_lines`).
|
||||
- `AppEvent::InsertHistoryCell` pushes cells into the deferral list when `overlay.is_some()`.
|
||||
|
||||
- `codex-rs/tui2/src/app_backtrack.rs`
|
||||
- `close_transcript_overlay` renders deferred cells at the *current* width when closing the
|
||||
overlay, then queues the resulting lines for the normal terminal buffer.
|
||||
|
||||
Note: as of today, `Tui::insert_history_lines` queues lines but `Tui::draw` does not flush them into
|
||||
the terminal (see `codex-rs/tui2/src/tui.rs`). This section is therefore best read as “behavior we
|
||||
want when/if scrollback printing is re-enabled”, not a guarantee that content is printed during the
|
||||
main TUI loop. For the current intended behavior around printing, see
|
||||
`codex-rs/tui2/docs/tui_viewport_and_history.md`.
|
||||
|
||||
## Tests (G2)
|
||||
|
||||
User choice recap:
|
||||
|
||||
- **G2**: Add resize reflow tests + snapshot coverage.
|
||||
|
||||
Added coverage:
|
||||
|
||||
- `codex-rs/tui2/src/history_cell.rs`
|
||||
- `agent_message_cell_reflows_streamed_prose_on_resize`
|
||||
- `agent_message_cell_reflows_streamed_prose_vt100_snapshot`
|
||||
|
||||
These assert that a streamed agent cell produces fewer visual lines at wider widths and provide
|
||||
snapshots showing reflow for list items and blockquotes.
|
||||
|
||||
## Audit: other `HistoryCell`s and width-baked paths
|
||||
|
||||
This section answers “what else might behave like this?” up front.
|
||||
|
||||
### History cells
|
||||
|
||||
- `AgentMessageCell` (`codex-rs/tui2/src/history_cell.rs`): **was affected**; now stores logical
|
||||
markdown lines and wraps at render time.
|
||||
- `UserHistoryCell` (`codex-rs/tui2/src/history_cell.rs`): wraps at render time from stored `String`
|
||||
using `word_wrap_lines_with_joiners` (reflowable).
|
||||
- `ReasoningSummaryCell` (`codex-rs/tui2/src/history_cell.rs`): renders from stored `String` on each
|
||||
call; it does call `append_markdown(..., Some(width))`, but that wrapping is recomputed per width
|
||||
(reflowable).
|
||||
- `PrefixedWrappedHistoryCell` (`codex-rs/tui2/src/history_cell.rs`): wraps at render time and
|
||||
returns joiners (reflowable).
|
||||
- `PlainHistoryCell` (`codex-rs/tui2/src/history_cell.rs`): stores `Vec<Line>` and returns it
|
||||
unchanged (not reflowable by design; used for already-structured/preformatted output).
|
||||
|
||||
Rule of thumb: any cell that stores already-wrapped `Vec<Line>` for prose is a candidate for the
|
||||
same bug; cells that store source text or logical lines and compute wrapping inside
|
||||
`display_lines(width)` are safe.
|
||||
|
||||
### Width-baked output outside the transcript model
|
||||
|
||||
Even with the streaming fix, some paths are inherently width-baked:
|
||||
|
||||
- Printed transcript after exit (`codex-rs/tui2/src/app.rs`): `AppExitInfo.session_lines` is rendered
|
||||
once using the final width and then printed; it cannot reflow afterward.
|
||||
- Optional scrollback insertion helper (`codex-rs/tui2/src/insert_history.rs`): once ANSI is written
|
||||
to the terminal, that output cannot be reflowed later. This helper is currently used for
|
||||
deterministic ANSI emission (`write_spans`) and tests; it is not wired into the main TUI draw
|
||||
loop.
|
||||
- Static overlays (`codex-rs/tui2/src/pager_overlay.rs`): reflow depends on whether callers provided
|
||||
width-agnostic input; pre-split `Vec<Line>` cannot be “un-split” within the overlay.
|
||||
|
||||
## Deferred / follow-ups
|
||||
|
||||
The fix above is sufficient to unblock correct reflow on resize. Remaining choices can be deferred:
|
||||
|
||||
- Streaming granularity: one logical line can wrap into multiple visual lines, so “commit tick”
|
||||
updates can appear in larger chunks than before. If this becomes a UX issue, we can add a render-
|
||||
time “progressive reveal” layer without reintroducing width baking.
|
||||
- Expand logical-line rendering to other markdown-ish cells if needed (e.g., unify `append_markdown`
|
||||
usage), but only if we find a concrete reflow bug beyond `AgentMessageCell`.
|
||||
|
||||
Reference in New Issue
Block a user