tui2: stop baking streaming wraps; reflow agent markdown

codex-rs/tui2/docs/streaming_wrapping_design.md

@@ -1,85 +1,169 @@
-# Streaming Markdown Wrapping & Animation – TUI2 Notes
-
-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.
-
-At a high level, the design constraints are the same:
-
-- 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.
-
-## 1. Where streaming is implemented in TUI2
-
-TUI2 keeps the streaming pipeline conceptually aligned with the legacy TUI but
-in a separate crate:
-
-- `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.
-
-As in the original TUI, the key tension is between:
-
-- **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).
-
-## 2. Current behavior and limitations
-
-TUI2 is intentionally conservative for now:
-
-- 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.
-
-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.
-
-## 3. Design directions (forward‑looking)
-
-The options outlined in the legacy document apply here as well:
-
-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.
-
-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.
+# Streaming Wrapping Reflow (tui2)
 
+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.
+
+## Goal
+
+- 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.
+
+Non-goals:
+
+- Reflowing terminal scrollback that has already been printed.
+- Reflowing content that is intentionally treated as preformatted (e.g., code blocks, raw stdout).
+
+## Background: where reflow happens in tui2
+
+TUI2 renders the transcript as a list of `HistoryCell`s:
+
+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.
+
+This only works if cells do *not* persist width-derived wrapping inside their stored state.
+
+## The bug: soft wraps became hard breaks during streaming
+
+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”.
+
+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.
+
+## Chosen solution (A, F1): stream logical markdown lines; wrap in the cell at render-time
+
+User choice recap:
+
+- **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.
+
+### 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`.

codex-rs/tui2/src/app.rs

@@ -344,10 +344,22 @@ pub(crate) struct App {
     transcript_copy_action: TranscriptCopyAction,
     transcript_scrollbar_ui: TranscriptScrollbarUi,
 
-    // Pager overlay state (Transcript or Static like Diff)
+    // Pager overlay state (Transcript or Static like Diff).
     pub(crate) overlay: Option<Overlay>,
-    pub(crate) deferred_history_lines: Vec<Line<'static>>,
-    has_emitted_history_lines: bool,
+    /// History cells received while an overlay is active.
+    ///
+    /// While in an alt-screen overlay, the normal terminal buffer is not visible.
+    /// Instead we queue the incoming cells here and, on overlay close, render them at the *current*
+    /// width and queue them in one batch via `Tui::insert_history_lines`.
+    ///
+    /// This matters for correctness if/when scrollback printing is enabled: if we deferred
+    /// already-rendered `Vec<Line>`, we'd bake viewport-width wrapping based on the width at the
+    /// time the cell arrived (which may differ from the width when the overlay closes).
+    pub(crate) deferred_history_cells: Vec<Arc<dyn HistoryCell>>,
+    /// True once at least one history cell has been inserted into terminal scrollback.
+    ///
+    /// Used to decide whether to insert an extra blank separator line when flushing deferred cells.
+    pub(crate) has_emitted_history_lines: bool,
 
     pub(crate) enhanced_keys_supported: bool,
 
@@ -511,7 +523,7 @@ impl App {
             transcript_copy_action: TranscriptCopyAction::default(),
             transcript_scrollbar_ui: TranscriptScrollbarUi::default(),
             overlay: None,
-            deferred_history_lines: Vec::new(),
+            deferred_history_cells: Vec::new(),
             has_emitted_history_lines: false,
             commit_anim_running: Arc::new(AtomicBool::new(false)),
             scroll_config,
@@ -1449,21 +1461,8 @@ impl App {
                     tui.frame_requester().schedule_frame();
                 }
                 self.transcript_cells.push(cell.clone());
-                let mut display = cell.display_lines(tui.terminal.last_known_screen_size.width);
-                if !display.is_empty() {
-                    // Only insert a separating blank line for new cells that are not
-                    // part of an ongoing stream. Streaming continuations should not
-                    // accrue extra blank lines between chunks.
-                    if !cell.is_stream_continuation() {
-                        if self.has_emitted_history_lines {
-                            display.insert(0, Line::from(""));
-                        } else {
-                            self.has_emitted_history_lines = true;
-                        }
-                    }
-                    if self.overlay.is_some() {
-                        self.deferred_history_lines.extend(display);
-                    }
+                if self.overlay.is_some() {
+                    self.deferred_history_cells.push(cell);
                 }
             }
             AppEvent::StartCommitAnimation => {
@@ -2135,7 +2134,7 @@ mod tests {
             transcript_copy_action: TranscriptCopyAction::default(),
             transcript_scrollbar_ui: TranscriptScrollbarUi::default(),
             overlay: None,
-            deferred_history_lines: Vec::new(),
+            deferred_history_cells: Vec::new(),
             has_emitted_history_lines: false,
             enhanced_keys_supported: false,
             commit_anim_running: Arc::new(AtomicBool::new(false)),
@@ -2188,7 +2187,7 @@ mod tests {
                 transcript_copy_action: TranscriptCopyAction::default(),
                 transcript_scrollbar_ui: TranscriptScrollbarUi::default(),
                 overlay: None,
-                deferred_history_lines: Vec::new(),
+                deferred_history_cells: Vec::new(),
                 has_emitted_history_lines: false,
                 enhanced_keys_supported: false,
                 commit_anim_running: Arc::new(AtomicBool::new(false)),

codex-rs/tui2/src/app_backtrack.rs

@@ -123,12 +123,42 @@ impl App {
     }
 
     /// Close transcript overlay and restore normal UI.
+    ///
+    /// Any history emitted while the overlay was open is flushed to the normal-buffer queue here.
+    ///
+    /// Importantly, we defer *cells* (not rendered lines) so we can render them against the current
+    /// width on close and avoid baking width-derived wrapping based on an earlier viewport size.
+    /// (This matters if/when scrollback printing is enabled; `Tui::insert_history_lines` currently
+    /// queues lines without printing them during the main draw loop.)
     pub(crate) fn close_transcript_overlay(&mut self, tui: &mut tui::Tui) {
         let _ = tui.leave_alt_screen();
         let was_backtrack = self.backtrack.overlay_preview_active;
-        if !self.deferred_history_lines.is_empty() {
-            let lines = std::mem::take(&mut self.deferred_history_lines);
-            tui.insert_history_lines(lines);
+        if !self.deferred_history_cells.is_empty() {
+            let cells = std::mem::take(&mut self.deferred_history_cells);
+            let width = tui.terminal.last_known_screen_size.width;
+            let mut lines: Vec<ratatui::text::Line<'static>> = Vec::new();
+            for cell in cells {
+                let mut display = cell.display_lines(width);
+                if display.is_empty() {
+                    continue;
+                }
+
+                // Only insert a separating blank line for new cells that are not part of an
+                // ongoing stream. Streaming continuations should not accrue extra blank lines
+                // between chunks.
+                if !cell.is_stream_continuation() {
+                    if self.has_emitted_history_lines {
+                        display.insert(0, ratatui::text::Line::from(""));
+                    } else {
+                        self.has_emitted_history_lines = true;
+                    }
+                }
+
+                lines.extend(display);
+            }
+            if !lines.is_empty() {
+                tui.insert_history_lines(lines);
+            }
         }
         self.overlay = None;
         self.backtrack.overlay_preview_active = false;

codex-rs/tui2/src/chatwidget.rs

@@ -1023,9 +1023,9 @@ impl ChatWidget {
                 self.needs_final_message_separator = false;
                 needs_redraw = true;
             }
-            self.stream_controller = Some(StreamController::new(
-                self.last_rendered_width.get().map(|w| w.saturating_sub(2)),
-            ));
+            // Streaming must not capture the current viewport width: width-derived wraps are
+            // applied later, at render time, so the transcript can reflow on resize.
+            self.stream_controller = Some(StreamController::new());
         }
         if let Some(controller) = self.stream_controller.as_mut()
             && controller.push(&delta)