diff --git a/codex-rs/tui2/docs/streaming_wrapping_design.md b/codex-rs/tui2/docs/streaming_wrapping_design.md index 28937c377ff..f7af4ccccba 100644 --- a/codex-rs/tui2/docs/streaming_wrapping_design.md +++ b/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`. 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` + +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` instead of already-wrapped `Vec`. + - 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`. + - `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>` (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` and returns it + unchanged (not reflowable by design; used for already-structured/preformatted output). + +Rule of thumb: any cell that stores already-wrapped `Vec` 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` 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`. diff --git a/codex-rs/tui2/src/app.rs b/codex-rs/tui2/src/app.rs index 677d73d71ea..1f7cad57746 100644 --- a/codex-rs/tui2/src/app.rs +++ b/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, - pub(crate) deferred_history_lines: Vec>, - 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`, 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>, + /// 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)), diff --git a/codex-rs/tui2/src/app_backtrack.rs b/codex-rs/tui2/src/app_backtrack.rs index ce5dff2ed85..01cb64fe5f7 100644 --- a/codex-rs/tui2/src/app_backtrack.rs +++ b/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> = 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; diff --git a/codex-rs/tui2/src/chatwidget.rs b/codex-rs/tui2/src/chatwidget.rs index b661894a18b..ab594678719 100644 --- a/codex-rs/tui2/src/chatwidget.rs +++ b/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) diff --git a/codex-rs/tui2/src/history_cell.rs b/codex-rs/tui2/src/history_cell.rs index 7696a387528..4d96b389bcf 100644 --- a/codex-rs/tui2/src/history_cell.rs +++ b/codex-rs/tui2/src/history_cell.rs @@ -119,6 +119,19 @@ pub(crate) trait HistoryCell: std::fmt::Debug + Send + Sync + Any { /// Most cells can use the default implementation (no joiners), but cells that apply wrapping /// should override this and return joiners derived from the same wrapping operation so /// clipboard reconstruction can distinguish hard breaks from soft wraps. + /// + /// `joiner_before[i]` describes the boundary *between* `lines[i - 1]` and `lines[i]`: + /// + /// - `None` means "hard break": copy inserts a newline between the two lines. + /// - `Some(joiner)` means "soft wrap continuation": copy inserts `joiner` and continues on the + /// same logical line. + /// + /// Example (one logical line wrapped across two visual lines): + /// + /// - `lines = ["• Hello", " world"]` + /// - `joiner_before = [None, Some(\" \")]` + /// + /// Copy should produce `"Hello world"` (no hard newline). fn transcript_lines_with_joiners(&self, width: u16) -> TranscriptLinesWithJoiners { let lines = self.transcript_lines(width); TranscriptLinesWithJoiners { @@ -313,14 +326,64 @@ impl HistoryCell for ReasoningSummaryCell { #[derive(Debug)] pub(crate) struct AgentMessageCell { - lines: Vec>, + /// Width-agnostic logical markdown lines for this chunk. + /// + /// These are produced either: + /// - by streaming (`markdown_stream` → `markdown_render::render_markdown_logical_lines`), or + /// - by legacy/non-streaming callers that pass pre-rendered `Vec` via [`Self::new`]. + /// + /// Importantly, this stores *logical* lines, not already-wrapped visual lines, so the transcript + /// can reflow on resize. + logical_lines: Vec, + /// Whether this cell should render the leading transcript bullet (`• `). + /// + /// Streaming emits multiple immutable `AgentMessageCell`s per assistant message; only the first + /// chunk shows the bullet. Continuations use a two-space gutter. is_first_line: bool, } impl AgentMessageCell { + /// Construct an agent message cell from already-rendered `Line`s. + /// + /// This is primarily used by non-streaming paths. The lines are treated as already "logical" + /// lines (no additional markdown indentation metadata is available), and wrapping is still + /// performed at render time so the transcript can reflow on resize. pub(crate) fn new(lines: Vec>, is_first_line: bool) -> Self { Self { - lines, + logical_lines: lines + .into_iter() + .map(|line| { + let is_preformatted = line.style.fg == Some(ratatui::style::Color::Cyan); + let line_style = line.style; + let content = Line { + style: Style::default(), + alignment: line.alignment, + spans: line.spans, + }; + crate::markdown_render::MarkdownLogicalLine { + content, + initial_indent: Line::default(), + subsequent_indent: Line::default(), + line_style, + is_preformatted, + } + }) + .collect(), + is_first_line, + } + } + + /// Construct an agent message cell from markdown logical lines. + /// + /// This is the preferred streaming constructor: it preserves markdown indentation rules (list + /// markers, nested list continuation indent, blockquote prefix, etc.) so wrapping can be + /// performed correctly at render time for the current viewport width. + pub(crate) fn new_logical( + logical_lines: Vec, + is_first_line: bool, + ) -> Self { + Self { + logical_lines, is_first_line, } } @@ -331,43 +394,98 @@ impl HistoryCell for AgentMessageCell { self.transcript_lines_with_joiners(width).lines } + /// Render wrapped transcript lines plus soft-wrap joiners. + /// + /// This is where width-dependent wrapping happens for streaming agent output. The cell composes + /// indentation as: + /// + /// - transcript gutter (`• ` or ` `), plus + /// - markdown-provided indent/prefix spans (`initial_indent` / `subsequent_indent`) + /// + /// The wrapping algorithm returns a `joiner_before` vector so copy/paste can treat soft wraps + /// as joinable (no hard newline) while preserving exact whitespace at wrap boundaries. fn transcript_lines_with_joiners(&self, width: u16) -> TranscriptLinesWithJoiners { - use ratatui::style::Color; + if width == 0 { + return TranscriptLinesWithJoiners { + lines: Vec::new(), + joiner_before: Vec::new(), + }; + } let mut out_lines: Vec> = Vec::new(); let mut joiner_before: Vec> = Vec::new(); - let mut is_first_output_line = true; - for line in &self.lines { - let is_code_block_line = line.style.fg == Some(Color::Cyan); - let initial_indent: Line<'static> = if is_first_output_line && self.is_first_line { + // `at_cell_start` tracks whether we're about to emit the first *visual* line of this cell. + // Only the first chunk of a streamed message gets the `• ` gutter; continuations use ` `. + let mut at_cell_start = true; + for logical in &self.logical_lines { + let gutter_first_visual_line: Line<'static> = if at_cell_start && self.is_first_line { "• ".dim().into() } else { " ".into() }; - let subsequent_indent: Line<'static> = " ".into(); + let gutter_continuation: Line<'static> = " ".into(); + + // Compose the transcript gutter with markdown-provided indentation: + // + // - `gutter_*` is the transcript-level prefix (`• ` / ` `). + // - `initial_indent` / `subsequent_indent` come from markdown structure (blockquote + // prefix, list marker indentation, nested list continuation indentation, etc.). + // + // We apply these indents during wrapping so: + // - the UI renders with correct continuation indentation, and + // - soft-wrap joiners stay aligned with the exact whitespace the wrapper skipped. + let compose_indent = + |gutter: &Line<'static>, md_indent: &Line<'static>| -> Line<'static> { + let mut spans = gutter.spans.clone(); + spans.extend(md_indent.spans.iter().cloned()); + Line::from(spans) + }; - if is_code_block_line { - let mut spans = initial_indent.spans; - spans.extend(line.spans.iter().cloned()); - out_lines.push(Line::from(spans).style(line.style)); + // Preformatted lines are rendered as a single visual line (no wrapping). + // This preserves code-block whitespace and keeps code copy behavior stable. + if logical.is_preformatted { + let mut spans = gutter_first_visual_line.spans.clone(); + spans.extend(logical.initial_indent.spans.iter().cloned()); + spans.extend(logical.content.spans.iter().cloned()); + out_lines.push(Line::from(spans).style(logical.line_style)); joiner_before.push(None); - is_first_output_line = false; + at_cell_start = false; continue; } + // Prose path: wrap to current width and capture joiners. + // + // `word_wrap_line_with_joiners` guarantees: + // - `wrapped.len() == wrapped_joiners.len()` + // - `wrapped_joiners[0] == None` (first visual segment of a logical line is a hard break) + // - subsequent entries are `Some(joiner)` (soft-wrap continuations). let opts = RtOptions::new(width as usize) - .initial_indent(initial_indent) - .subsequent_indent(subsequent_indent.clone()); + .initial_indent(compose_indent( + &gutter_first_visual_line, + &logical.initial_indent, + )) + .subsequent_indent(compose_indent( + &gutter_continuation, + &logical.subsequent_indent, + )); + let (wrapped, wrapped_joiners) = - crate::wrapping::word_wrap_line_with_joiners(line, opts); - for (l, j) in wrapped.into_iter().zip(wrapped_joiners) { - out_lines.push(line_to_static(&l)); - joiner_before.push(j); - is_first_output_line = false; + crate::wrapping::word_wrap_line_with_joiners(&logical.content, opts); + for (visual, joiner) in wrapped.into_iter().zip(wrapped_joiners) { + out_lines.push(line_to_static(&visual).style(logical.line_style)); + joiner_before.push(joiner); + at_cell_start = false; } } + debug_assert_eq!(out_lines.len(), joiner_before.len()); + debug_assert!( + joiner_before + .first() + .is_none_or(std::option::Option::is_none) + ); + TranscriptLinesWithJoiners { lines: out_lines, joiner_before, @@ -1674,6 +1792,131 @@ mod tests { render_lines(&cell.transcript_lines(u16::MAX)) } + /// Remove a single leading markdown blockquote marker (`> `) from `line`. + /// + /// This is a test-only normalization helper. + /// + /// In the rendered transcript, blockquote indentation is represented as literal `> ` spans in + /// the line prefix. For wrapped blockquote prose, those prefix spans can appear on every visual + /// line (including soft-wrap continuations). When we want to compare the *logical* joined text + /// across different widths, we strip the repeated marker on continuation lines so the + /// comparison doesn't fail due to prefix duplication. + fn strip_leading_blockquote_marker(line: &str) -> String { + let mut out = String::with_capacity(line.len()); + let mut seen_non_space = false; + let mut removed = false; + let mut chars = line.chars().peekable(); + while let Some(ch) = chars.next() { + if !seen_non_space { + if ch == ' ' { + out.push(ch); + continue; + } + seen_non_space = true; + if ch == '>' && !removed { + removed = true; + if matches!(chars.peek(), Some(' ')) { + chars.next(); + } + continue; + } + } + out.push(ch); + } + out + } + + /// Normalize rendered transcript output into a width-insensitive "logical text" string. + /// + /// This is used by resize/reflow tests: + /// + /// - Joiners tell us which visual line breaks are soft wraps (`Some(joiner)`) vs hard breaks + /// (`None`). + /// - For soft-wrap continuation lines, we strip repeated blockquote markers so we can compare + /// the underlying prose independent of prefix repetition. + /// - Finally, we collapse whitespace so wrapping differences (line breaks vs spaces) do not + /// affect equality. + fn normalize_rendered_text_with_joiners(tr: &TranscriptLinesWithJoiners) -> String { + let mut rendered = render_lines(&tr.lines); + for (line, joiner) in rendered.iter_mut().zip(&tr.joiner_before) { + if joiner.is_some() { + *line = strip_leading_blockquote_marker(line); + } + } + rendered + .join("\n") + .split_whitespace() + .collect::>() + .join(" ") + } + + #[test] + fn agent_message_cell_reflows_streamed_prose_on_resize() { + let md = concat!( + "- This is a long list item that should reflow when the viewport width changes. ", + "The old streaming implementation used to bake soft wraps into hard line breaks.\n", + "> A blockquote line that is also long enough to wrap and should reflow cleanly.\n", + ); + let logical_lines = crate::markdown_stream::simulate_stream_markdown_for_tests(&[md], true); + let cell = AgentMessageCell::new_logical(logical_lines, true); + + let narrow = cell.transcript_lines_with_joiners(28); + let wide = cell.transcript_lines_with_joiners(80); + + assert!( + narrow.lines.len() > wide.lines.len(), + "expected fewer visual lines at wider width; narrow={} wide={}", + narrow.lines.len(), + wide.lines.len() + ); + assert_eq!( + normalize_rendered_text_with_joiners(&narrow), + normalize_rendered_text_with_joiners(&wide) + ); + + let snapshot = format!( + "narrow:\n{}\n\nwide:\n{}", + render_lines(&narrow.lines).join("\n"), + render_lines(&wide.lines).join("\n") + ); + insta::assert_snapshot!("agent_message_cell_reflow_on_resize", snapshot); + } + + #[test] + fn agent_message_cell_reflows_streamed_prose_vt100_snapshot() { + use crate::test_backend::VT100Backend; + + let md = concat!( + "- This is a long list item that should reflow when the viewport width changes.\n", + "> A blockquote that also reflows across widths.\n", + ); + let logical_lines = crate::markdown_stream::simulate_stream_markdown_for_tests(&[md], true); + let cell = AgentMessageCell::new_logical(logical_lines, true); + + let render = |width, height| -> String { + let backend = VT100Backend::new(width, height); + let mut terminal = ratatui::Terminal::new(backend).expect("terminal"); + terminal + .draw(|f| { + let area = f.area(); + let lines = cell.display_lines(area.width); + Paragraph::new(Text::from(lines)) + .wrap(Wrap { trim: false }) + .render(area, f.buffer_mut()); + }) + .expect("draw"); + terminal.backend().vt100().screen().contents() + }; + + let narrow = render(30, 12); + let wide = render(70, 12); + + insta::assert_snapshot!( + "agent_message_cell_reflow_on_resize_vt100", + format!("narrow:\n{narrow}\n\nwide:\n{wide}") + ); + } + #[tokio::test] async fn mcp_tools_output_masks_sensitive_values() { let mut config = test_config().await; diff --git a/codex-rs/tui2/src/insert_history.rs b/codex-rs/tui2/src/insert_history.rs index 86a32c16ce8..1b236e8eec3 100644 --- a/codex-rs/tui2/src/insert_history.rs +++ b/codex-rs/tui2/src/insert_history.rs @@ -1,9 +1,13 @@ -//! Render `ratatui` transcript lines into terminal scrollback. +//! Render `ratatui` transcript lines into terminal output (scrollback) and/or deterministic ANSI. //! //! `insert_history_lines` is responsible for inserting rendered transcript lines //! *above* the TUI viewport by emitting ANSI control sequences through the //! terminal backend writer. //! +//! Note: the current `tui2` main draw loop does not call `insert_history_lines` (see +//! `codex-rs/tui2/src/tui.rs`). This module is still used for deterministic ANSI emission via +//! `write_spans` (e.g., "print after exit" flows) and for tests. +//! //! ## Why we use crossterm style commands //! //! `write_spans` is also used by non-terminal callers (e.g. diff --git a/codex-rs/tui2/src/markdown_render.rs b/codex-rs/tui2/src/markdown_render.rs index 22a234326aa..c285ca41a45 100644 --- a/codex-rs/tui2/src/markdown_render.rs +++ b/codex-rs/tui2/src/markdown_render.rs @@ -1,3 +1,37 @@ +//! Markdown rendering for `tui2`. +//! +//! This module has two related but intentionally distinct responsibilities: +//! +//! 1. **Parse Markdown into styled text** (for display). +//! 2. **Preserve width-agnostic structure for reflow** (for streaming + resize). +//! +//! ## Why logical lines exist +//! +//! TUI2 supports viewport resize reflow and copy/paste that treats soft-wrapped prose as a single +//! logical line. If we apply wrapping while rendering and store the resulting `Vec`, those +//! width-derived breaks become indistinguishable from hard newlines and cannot be "unwrapped" when +//! the viewport gets wider. +//! +//! To avoid baking width, streaming uses [`MarkdownLogicalLine`] output: +//! +//! - `content` holds the styled spans for a single *logical* line (a hard break boundary). +//! - `initial_indent` / `subsequent_indent` encode markdown-aware indentation rules for wraps +//! (list markers, nested lists, blockquotes, etc.). +//! - `line_style` captures line-level styling (e.g., blockquote green) that must apply to all +//! wrapped segments. +//! - `is_preformatted` marks runs that should not be wrapped like prose (e.g., fenced code). +//! +//! History cells can then wrap `content` at the *current* width, applying indents appropriately and +//! returning soft-wrap joiners for correct copy/paste. +//! +//! ## Outputs +//! +//! - [`render_markdown_text_with_width`]: emits a `Text` suitable for immediate display and may +//! apply wrapping if a width is provided. +//! - [`render_markdown_logical_lines`]: emits width-agnostic logical lines (no wrapping). +//! +//! The underlying `Writer` can emit either (or both) depending on call site needs. + use crate::render::line_utils::line_to_static; use crate::wrapping::RtOptions; use crate::wrapping::word_wrap_line; @@ -14,6 +48,31 @@ use ratatui::text::Line; use ratatui::text::Span; use ratatui::text::Text; +/// A single width-agnostic markdown "logical line" plus the metadata required to wrap it later. +/// +/// A logical line is a hard-break boundary produced by markdown parsing (explicit newlines, +/// paragraph boundaries, list item boundaries, etc.). It is not a viewport-derived wrap segment. +/// +/// Wrapping is performed later (typically in `HistoryCell::transcript_lines_with_joiners(width)`), +/// where a cell can: +/// +/// - prepend a transcript gutter prefix (`• ` / ` `), +/// - prepend markdown-specific indents (`initial_indent` / `subsequent_indent`), and +/// - wrap `content` to the current width while producing joiners for copy/paste. +#[derive(Clone, Debug)] +pub(crate) struct MarkdownLogicalLine { + /// The raw content for this logical line (does not include markdown prefix/indent spans). + pub(crate) content: Line<'static>, + /// Prefix/indent spans to apply to the first visual line when wrapping. + pub(crate) initial_indent: Line<'static>, + /// Prefix/indent spans to apply to wrapped continuation lines. + pub(crate) subsequent_indent: Line<'static>, + /// Line-level style to apply to all wrapped segments. + pub(crate) line_style: Style, + /// True when this line is preformatted and should not be wrapped like prose. + pub(crate) is_preformatted: bool, +} + struct MarkdownStyles { h1: Style, h2: Style, @@ -56,8 +115,12 @@ impl Default for MarkdownStyles { #[derive(Clone, Debug)] struct IndentContext { + /// Prefix spans to apply for this nesting level (e.g., blockquote `> `, list indentation). prefix: Vec>, + /// Optional list marker spans (e.g., `- ` or `1. `) that apply only to the first visual line of + /// a list item. marker: Option>>, + /// True if this context represents a list indentation level. is_list: bool, } @@ -75,21 +138,45 @@ pub fn render_markdown_text(input: &str) -> Text<'static> { render_markdown_text_with_width(input, None) } +/// Render markdown into a ratatui `Text`, optionally wrapping to a specific width. +/// +/// This is primarily used for non-streaming rendering where storing width-derived wrapping is +/// acceptable or where the caller immediately consumes the output. pub(crate) fn render_markdown_text_with_width(input: &str, width: Option) -> Text<'static> { let mut options = Options::empty(); options.insert(Options::ENABLE_STRIKETHROUGH); let parser = Parser::new_ext(input, options); - let mut w = Writer::new(parser, width); + let mut w = Writer::new(parser, width, true, false); w.run(); w.text } +/// Render markdown into width-agnostic logical lines (no wrapping). +/// +/// This is used by streaming so that the transcript can reflow on resize: wrapping is deferred to +/// the history cell at render time. +pub(crate) fn render_markdown_logical_lines(input: &str) -> Vec { + let mut options = Options::empty(); + options.insert(Options::ENABLE_STRIKETHROUGH); + let parser = Parser::new_ext(input, options); + let mut w = Writer::new(parser, None, false, true); + w.run(); + w.logical_lines +} + +/// A markdown event sink that builds either: +/// - a wrapped `Text` (`emit_text = true`), and/or +/// - width-agnostic [`MarkdownLogicalLine`]s (`emit_logical_lines = true`). +/// +/// The writer tracks markdown structure (paragraphs, lists, blockquotes, code blocks) and builds up +/// a "current logical line". `flush_current_line` commits it to the selected output(s). struct Writer<'a, I> where I: Iterator>, { iter: I, text: Text<'static>, + logical_lines: Vec, styles: MarkdownStyles, inline_styles: Vec