From 8a37410e7ed3bca12935f22a9a94091b02c361c6 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Tue, 28 Apr 2026 13:10:04 -0300 Subject: [PATCH 1/3] feat(tui): add streaming markdown table support --- codex-rs/tui/src/app.rs | 1 + .../src/app/agent_message_consolidation.rs | 80 + codex-rs/tui/src/app/event_dispatch.rs | 36 +- codex-rs/tui/src/app_event.rs | 12 + codex-rs/tui/src/chatwidget.rs | 115 +- .../src/chatwidget/tests/status_and_layout.rs | 145 ++ codex-rs/tui/src/history_cell.rs | 83 +- codex-rs/tui/src/lib.rs | 1 + codex-rs/tui/src/markdown.rs | 380 ++++- codex-rs/tui/src/markdown_render.rs | 1321 +++++++++++++- codex-rs/tui/src/markdown_render_tests.rs | 97 ++ codex-rs/tui/src/markdown_stream.rs | 37 + ...ll__tests__raw_mode_toggle_transcript.snap | 16 +- ...sts__markdown_render_complex_snapshot.snap | 9 +- codex-rs/tui/src/streaming/controller.rs | 1520 +++++++++++++++-- codex-rs/tui/src/streaming/mod.rs | 1 + codex-rs/tui/src/streaming/table_holdback.rs | 210 +++ codex-rs/tui/src/table_detect.rs | 479 ++++++ 18 files changed, 4300 insertions(+), 243 deletions(-) create mode 100644 codex-rs/tui/src/app/agent_message_consolidation.rs create mode 100644 codex-rs/tui/src/streaming/table_holdback.rs create mode 100644 codex-rs/tui/src/table_detect.rs diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 4cd3d62cbd3e..8aefbd0f47fe 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -180,6 +180,7 @@ use tokio::sync::mpsc::unbounded_channel; use tokio::task::JoinHandle; use toml::Value as TomlValue; use uuid::Uuid; +mod agent_message_consolidation; mod agent_navigation; mod app_server_event_targets; mod app_server_events; diff --git a/codex-rs/tui/src/app/agent_message_consolidation.rs b/codex-rs/tui/src/app/agent_message_consolidation.rs new file mode 100644 index 000000000000..5e7991529099 --- /dev/null +++ b/codex-rs/tui/src/app/agent_message_consolidation.rs @@ -0,0 +1,80 @@ +use std::path::PathBuf; +use std::sync::Arc; + +use color_eyre::eyre::Result; + +use super::App; +use super::resize_reflow::trailing_run_start; +use crate::app_event::ConsolidationScrollbackReflow; +use crate::history_cell; +use crate::history_cell::HistoryCell; +use crate::pager_overlay::Overlay; +use crate::tui; + +impl App { + pub(super) fn handle_consolidate_agent_message( + &mut self, + tui: &mut tui::Tui, + source: String, + cwd: PathBuf, + scrollback_reflow: ConsolidationScrollbackReflow, + deferred_history_cell: Option>, + ) -> Result<()> { + if let Some(cell) = deferred_history_cell { + let cell: Arc = cell.into(); + if let Some(Overlay::Transcript(t)) = &mut self.overlay { + t.insert_cell(cell.clone()); + } + self.transcript_cells.push(cell); + } + + // Walk backward to find the contiguous run of streaming AgentMessageCells that + // belong to the just-finalized stream. + let end = self.transcript_cells.len(); + tracing::debug!( + "ConsolidateAgentMessage: transcript_cells.len()={end}, source_len={}", + source.len() + ); + let start = trailing_run_start::(&self.transcript_cells); + if start < end { + tracing::debug!( + "ConsolidateAgentMessage: replacing cells [{start}..{end}] with AgentMarkdownCell" + ); + let consolidated: Arc = + Arc::new(history_cell::AgentMarkdownCell::new(source, &cwd)); + self.transcript_cells + .splice(start..end, std::iter::once(consolidated.clone())); + + if let Some(Overlay::Transcript(t)) = &mut self.overlay { + t.consolidate_cells(start..end, consolidated.clone()); + tui.frame_requester().schedule_frame(); + } + + self.finish_agent_message_consolidation(tui, scrollback_reflow)?; + } else { + tracing::debug!( + "ConsolidateAgentMessage: no cells to consolidate(start={start}, end={end})", + ); + self.maybe_finish_stream_reflow(tui)?; + } + + Ok(()) + } + + fn finish_agent_message_consolidation( + &mut self, + tui: &mut tui::Tui, + scrollback_reflow: ConsolidationScrollbackReflow, + ) -> Result<()> { + match scrollback_reflow { + ConsolidationScrollbackReflow::IfResizeReflowRan => { + self.maybe_finish_stream_reflow(tui)?; + } + ConsolidationScrollbackReflow::Required => { + self.finish_required_stream_reflow(tui)?; + } + } + + Ok(()) + } +} diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 565ee6ba1fb9..af89021d666e 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -217,29 +217,19 @@ impl App { AppEvent::EndInitialHistoryReplayBuffer => { self.finish_initial_history_replay_buffer(tui); } - AppEvent::ConsolidateAgentMessage { source, cwd } => { - if !self.terminal_resize_reflow_enabled() { - self.transcript_reflow.clear(); - return Ok(AppRunControl::Continue); - } - let end = self.transcript_cells.len(); - let start = - trailing_run_start::(&self.transcript_cells); - if start < end { - let consolidated: Arc = - Arc::new(history_cell::AgentMarkdownCell::new(source, &cwd)); - self.transcript_cells - .splice(start..end, std::iter::once(consolidated.clone())); - - if let Some(Overlay::Transcript(t)) = &mut self.overlay { - t.consolidate_cells(start..end, consolidated.clone()); - tui.frame_requester().schedule_frame(); - } - - self.maybe_finish_stream_reflow(tui)?; - } else { - self.maybe_finish_stream_reflow(tui)?; - } + AppEvent::ConsolidateAgentMessage { + source, + cwd, + scrollback_reflow, + deferred_history_cell, + } => { + self.handle_consolidate_agent_message( + tui, + source, + cwd, + scrollback_reflow, + deferred_history_cell, + )?; } AppEvent::ConsolidateProposedPlan(source) => { if !self.terminal_resize_reflow_enabled() { diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 664ddf12d8fc..8519ea4ea66c 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -85,6 +85,12 @@ impl RealtimeAudioDeviceKind { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ConsolidationScrollbackReflow { + IfResizeReflowRan, + Required, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[cfg_attr(not(target_os = "windows"), allow(dead_code))] pub(crate) enum WindowsSandboxEnableMode { @@ -518,9 +524,15 @@ pub(crate) enum AppEvent { /// finalization. The `App` handler walks backward through `transcript_cells` /// to find the `AgentMessageCell` run and splices in the consolidated cell. /// The `cwd` keeps local file-link display stable across the final re-render. + /// `scrollback_reflow` lets table-tail finalization force the already-emitted + /// terminal scrollback to be rebuilt from the consolidated source-backed cell. + /// `deferred_history_cell` lets callers add the final stream tail to the + /// transcript without first writing its provisional render to scrollback. ConsolidateAgentMessage { source: String, cwd: PathBuf, + scrollback_reflow: ConsolidationScrollbackReflow, + deferred_history_cell: Option>, }, /// Replace the contiguous run of streaming `ProposedPlanStreamCell`s at the diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 159c65f011e4..c6fcfd829c36 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1547,16 +1547,30 @@ impl ChatWidget { fn flush_answer_stream_with_separator(&mut self) { let had_stream_controller = self.stream_controller.is_some(); if let Some(mut controller) = self.stream_controller.take() { + let scrollback_reflow = if controller.has_live_tail() { + crate::app_event::ConsolidationScrollbackReflow::Required + } else { + crate::app_event::ConsolidationScrollbackReflow::IfResizeReflowRan + }; + self.clear_active_stream_tail(); let (cell, source) = controller.finalize(); - if let Some(cell) = cell { - self.add_boxed_history(cell); - } + let deferred_history_cell = + if scrollback_reflow == crate::app_event::ConsolidationScrollbackReflow::Required { + cell + } else { + if let Some(cell) = cell { + self.add_boxed_history(cell); + } + None + }; // Consolidate the run of streaming AgentMessageCells into a single AgentMarkdownCell // that can re-render from source on resize. if let Some(source) = source { self.app_event_tx.send(AppEvent::ConsolidateAgentMessage { source, cwd: self.config.cwd.to_path_buf(), + scrollback_reflow, + deferred_history_cell, }); } } @@ -2115,11 +2129,10 @@ impl ChatWidget { self.transcript.plan_delta_buffer.clear(); } self.transcript.plan_delta_buffer.push_str(&delta); - // Before streaming plan content, flush any active exec cell group. - self.flush_unified_exec_wait_streak(); - self.flush_active_cell(); - if self.plan_stream_controller.is_none() { + // Before starting a plan stream, flush any active exec cell group. + self.flush_unified_exec_wait_streak(); + self.flush_active_cell(); self.plan_stream_controller = Some(PlanStreamController::new( self.current_stream_width(/*reserved_cols*/ 4), &self.config.cwd, @@ -2132,6 +2145,7 @@ impl ChatWidget { self.app_event_tx.send(AppEvent::StartCommitAnimation); self.run_catch_up_commit_tick(); } + self.sync_active_stream_tail(); self.request_redraw(); } @@ -2154,7 +2168,14 @@ impl ChatWidget { self.transcript.saw_plan_item_this_turn = true; let (finalized_streamed_cell, consolidated_plan_source) = if let Some(mut controller) = self.plan_stream_controller.take() { - controller.finalize() + let had_live_tail = controller.has_live_tail(); + self.clear_active_stream_tail(); + let (cell, source) = controller.finalize(); + if had_live_tail { + (None, source) + } else { + (cell, source) + } } else { (None, None) }; @@ -2286,8 +2307,10 @@ impl ChatWidget { // If a stream is currently active, finalize it. self.flush_answer_stream_with_separator(); if let Some(mut controller) = self.plan_stream_controller.take() { + let had_live_tail = controller.has_live_tail(); + self.clear_active_stream_tail(); let (cell, source) = controller.finalize(); - if let Some(cell) = cell { + if !had_live_tail && let Some(cell) = cell { self.add_boxed_history(cell); } if let Some(source) = source { @@ -2772,6 +2795,9 @@ impl ChatWidget { /// This does not clear MCP startup tracking, because MCP startup can overlap with turn cleanup /// and should continue to drive the bottom-pane running indicator while it is in progress. fn finalize_turn(&mut self) { + // Drop preview-only stream tail content on any termination path before + // failed-cell finalization, so transient tail cells are never persisted. + self.clear_active_stream_tail(); // Ensure any spinner is replaced by a red ✗ and flushed into history. self.finalize_active_cell_as_failed(); // Turn-scoped hook rows are transient live state; once the turn is over, @@ -4093,6 +4119,7 @@ impl ChatWidget { self.bottom_pane.hide_status_indicator(); self.add_boxed_history(cell); } + self.sync_active_stream_tail(); if outcome.has_controller && outcome.all_idle { self.maybe_restore_status_indicator_after_stream_idle(); @@ -4137,11 +4164,10 @@ impl ChatWidget { #[inline] fn handle_streaming_delta(&mut self, delta: String) { - // Before streaming agent content, flush any active exec cell group. - self.flush_unified_exec_wait_streak(); - self.flush_active_cell(); - if self.stream_controller.is_none() { + // Before starting an agent stream, flush any active exec cell group. + self.flush_unified_exec_wait_streak(); + self.flush_active_cell(); // If the previous turn inserted non-stream history (exec output, patch status, MCP // calls), render a separator before starting the next streamed assistant message. if self.transcript.needs_final_message_separator && self.transcript.had_work_activity { @@ -4165,6 +4191,7 @@ impl ChatWidget { self.app_event_tx.send(AppEvent::StartCommitAnimation); self.run_catch_up_commit_tick(); } + self.sync_active_stream_tail(); self.request_redraw(); } @@ -5322,12 +5349,70 @@ impl ChatWidget { if !keep_placeholder_header_active && !cell.display_lines(u16::MAX).is_empty() { // Only break exec grouping if the cell renders visible lines. - self.flush_active_cell(); + if !self.has_active_stream_tail() { + self.flush_active_cell(); + } self.transcript.needs_final_message_separator = true; } self.app_event_tx.send(AppEvent::InsertHistoryCell(cell)); } + fn active_cell_is_stream_tail(&self) -> bool { + self.transcript.active_cell.as_ref().is_some_and(|cell| { + cell.as_any().is::() + || cell.as_any().is::() + }) + } + + fn has_active_stream_tail(&self) -> bool { + (self.stream_controller.is_some() || self.plan_stream_controller.is_some()) + && self.active_cell_is_stream_tail() + } + + fn sync_active_stream_tail(&mut self) { + if let Some(controller) = self.stream_controller.as_ref() { + let tail_lines = controller.current_tail_lines(); + if tail_lines.is_empty() { + self.clear_active_stream_tail(); + return; + } + + self.bottom_pane.hide_status_indicator(); + self.transcript.active_cell = + Some(Box::new(history_cell::StreamingAgentTailCell::new( + tail_lines, + controller.tail_starts_stream(), + ))); + self.bump_active_cell_revision(); + return; + } + + if let Some(controller) = self.plan_stream_controller.as_ref() { + let tail_lines = controller.current_tail_display_lines(); + if tail_lines.is_empty() { + self.clear_active_stream_tail(); + return; + } + + self.bottom_pane.hide_status_indicator(); + self.transcript.active_cell = Some(Box::new(history_cell::StreamingPlanTailCell::new( + tail_lines, + !controller.tail_starts_stream(), + ))); + self.bump_active_cell_revision(); + return; + } + + self.clear_active_stream_tail(); + } + + fn clear_active_stream_tail(&mut self) { + if self.active_cell_is_stream_tail() { + self.transcript.active_cell = None; + self.bump_active_cell_revision(); + } + } + fn queue_user_message(&mut self, user_message: UserMessage) { self.queue_user_message_with_options(user_message, QueuedInputAction::Plain); } @@ -9723,6 +9808,7 @@ impl ChatWidget { if let Some(controller) = self.plan_stream_controller.as_mut() { controller.set_width(plan_stream_width); } + self.sync_active_stream_tail(); if !had_rendered_width { self.request_redraw(); } @@ -9918,6 +10004,7 @@ impl ChatWidget { if let Some(controller) = self.plan_stream_controller.as_mut() { controller.clear_queue(); } + self.clear_active_stream_tail(); self.request_redraw(); } } diff --git a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs index 00972ff8450f..a1b2cf423d8c 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs @@ -237,6 +237,151 @@ async fn raw_output_mode_can_change_without_inserting_notice() { ); } +#[tokio::test] +async fn flush_answer_stream_keeps_default_reflow_for_plain_text_tail() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let cwd = chat.config.cwd.to_path_buf(); + + let mut controller = crate::streaming::controller::StreamController::new( + Some(80), + cwd.as_path(), + HistoryRenderMode::Rich, + ); + assert!(controller.push("plain response line\n")); + chat.stream_controller = Some(controller); + + while rx.try_recv().is_ok() {} + + chat.flush_answer_stream_with_separator(); + + let mut saw_consolidate = false; + let mut saw_insert_history = false; + while let Ok(event) = rx.try_recv() { + match event { + AppEvent::InsertHistoryCell(_) => saw_insert_history = true, + AppEvent::ConsolidateAgentMessage { + scrollback_reflow, + deferred_history_cell, + .. + } => { + saw_consolidate = true; + assert_eq!( + scrollback_reflow, + crate::app_event::ConsolidationScrollbackReflow::IfResizeReflowRan + ); + assert!(deferred_history_cell.is_none()); + } + _ => {} + } + } + + assert!( + saw_consolidate, + "expected stream finalization to consolidate" + ); + assert!( + saw_insert_history, + "plain text should still insert history before consolidation" + ); +} + +#[tokio::test] +async fn flush_answer_stream_requests_scrollback_reflow_for_live_table_tail() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let cwd = chat.config.cwd.to_path_buf(); + + let mut controller = crate::streaming::controller::StreamController::new( + Some(80), + cwd.as_path(), + HistoryRenderMode::Rich, + ); + controller.push("| Name | Notes |\n"); + controller.push("| --- | --- |\n"); + controller.push("| alpha | tail held until final table render |\n"); + assert!( + controller.has_live_tail(), + "expected table holdback to leave a live tail for this regression", + ); + chat.stream_controller = Some(controller); + + while rx.try_recv().is_ok() {} + + chat.flush_answer_stream_with_separator(); + + let mut saw_consolidate = false; + let mut saw_insert_history = false; + while let Ok(event) = rx.try_recv() { + match event { + AppEvent::InsertHistoryCell(_) => saw_insert_history = true, + AppEvent::ConsolidateAgentMessage { + scrollback_reflow, + deferred_history_cell, + .. + } => { + saw_consolidate = true; + assert_eq!( + scrollback_reflow, + crate::app_event::ConsolidationScrollbackReflow::Required + ); + assert!( + deferred_history_cell.is_some(), + "live table tail should be staged for consolidation", + ); + } + _ => {} + } + } + + assert!( + saw_consolidate, + "expected stream finalization to consolidate" + ); + assert!( + !saw_insert_history, + "live table tail should not be inserted before canonical reflow" + ); +} + +#[tokio::test] +async fn completed_plan_table_tail_skips_provisional_history_insert() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let cwd = chat.config.cwd.to_path_buf(); + + let mut controller = crate::streaming::controller::PlanStreamController::new( + Some(80), + cwd.as_path(), + HistoryRenderMode::Rich, + ); + controller.push("| Step | Owner |\n"); + controller.push("| --- | --- |\n"); + controller.push("| Verify | Codex |\n"); + assert!( + controller.has_live_tail(), + "expected plan table holdback to leave a live tail", + ); + chat.plan_stream_controller = Some(controller); + chat.transcript.plan_delta_buffer = + "| Step | Owner |\n| --- | --- |\n| Verify | Codex |\n".to_string(); + + while rx.try_recv().is_ok() {} + + chat.on_plan_item_completed(String::new()); + + let mut saw_source_backed_plan = false; + let mut saw_stream_plan = false; + while let Ok(event) = rx.try_recv() { + if let AppEvent::InsertHistoryCell(cell) = event { + saw_source_backed_plan |= cell.as_any().is::(); + saw_stream_plan |= cell.as_any().is::(); + } + } + + assert!(saw_source_backed_plan, "expected source-backed plan insert"); + assert!( + !saw_stream_plan, + "live plan table tail should not be inserted provisionally" + ); +} #[tokio::test] async fn helpers_are_available_and_do_not_panic() { let (tx_raw, _rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 9920bf63a828..a7b54158e2c9 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -578,7 +578,8 @@ impl HistoryCell for AgentMessageCell { /// After a stream finalizes, the `ConsolidateAgentMessage` handler in `App` /// replaces the contiguous run of `AgentMessageCell`s with a single /// `AgentMarkdownCell`. On terminal resize, `display_lines(width)` re-renders -/// from source via `append_markdown`. +/// from source via `append_markdown_agent`, producing correctly-sized tables +/// with box-drawing borders. /// /// The cell snapshots `cwd` at construction so local file-link display remains aligned with the /// session that produced the message. Reusing the current process cwd during reflow would make old @@ -614,7 +615,7 @@ impl HistoryCell for AgentMarkdownCell { let mut lines: Vec> = Vec::new(); // Re-render markdown from source at the current width. Reserve 2 columns for the "• " / // " " prefix prepended below. - crate::markdown::append_markdown( + crate::markdown::append_markdown_agent_with_cwd( &self.markdown_source, Some(wrap_width), Some(self.cwd.as_path()), @@ -628,6 +629,84 @@ impl HistoryCell for AgentMarkdownCell { } } +/// Transient active-cell representation of the mutable tail of an agent stream. +/// +/// During streaming, lines that have not yet been committed to scrollback because they belong to +/// an in-progress table are displayed via this cell in the `active_cell` slot. It is replaced on +/// every delta and cleared when the stream finalizes. +#[derive(Debug)] +pub(crate) struct StreamingAgentTailCell { + lines: Vec>, + is_first_line: bool, +} + +impl StreamingAgentTailCell { + pub(crate) fn new(lines: Vec>, is_first_line: bool) -> Self { + Self { + lines, + is_first_line, + } + } +} + +impl HistoryCell for StreamingAgentTailCell { + fn display_lines(&self, _width: u16) -> Vec> { + // Tail lines are already rendered at the controller's current stream width. + // Re-wrapping them here can split table borders and produce malformed in-flight rows. + prefix_lines( + self.lines.clone(), + if self.is_first_line { + "• ".dim() + } else { + " ".into() + }, + " ".into(), + ) + } + + fn raw_lines(&self) -> Vec> { + plain_lines(self.display_lines(u16::MAX)) + } + + fn is_stream_continuation(&self) -> bool { + !self.is_first_line + } +} + +/// Transient active-cell representation of the mutable tail of a proposed-plan stream. +/// +/// The controller prepares the full styled plan lines because plan tails need the same header, +/// padding, and background treatment as committed `ProposedPlanStreamCell`s while remaining +/// preview-only during streaming. +#[derive(Debug)] +pub(crate) struct StreamingPlanTailCell { + lines: Vec>, + is_stream_continuation: bool, +} + +impl StreamingPlanTailCell { + pub(crate) fn new(lines: Vec>, is_stream_continuation: bool) -> Self { + Self { + lines, + is_stream_continuation, + } + } +} + +impl HistoryCell for StreamingPlanTailCell { + fn display_lines(&self, _width: u16) -> Vec> { + self.lines.clone() + } + + fn raw_lines(&self) -> Vec> { + plain_lines(self.lines.clone()) + } + + fn is_stream_continuation(&self) -> bool { + self.is_stream_continuation + } +} + #[derive(Debug)] pub(crate) struct PlainHistoryCell { lines: Vec>, diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 0e507dd78357..ba7460ca4723 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -251,6 +251,7 @@ mod voice { mod wrapping; +mod table_detect; #[cfg(test)] pub(crate) mod test_backend; #[cfg(test)] diff --git a/codex-rs/tui/src/markdown.rs b/codex-rs/tui/src/markdown.rs index e29246e76dd5..6d0daf89b2c4 100644 --- a/codex-rs/tui/src/markdown.rs +++ b/codex-rs/tui/src/markdown.rs @@ -1,7 +1,32 @@ +//! Markdown-to-ratatui rendering entry points. +//! +//! This module provides the public API surface that the rest of the TUI uses +//! to turn markdown source into `Vec>`. Two variants exist: +//! +//! - [`append_markdown`] -- general-purpose, used for plan blocks and history +//! cells that already hold pre-processed markdown (no fence unwrapping). +//! - [`append_markdown_agent`] -- for agent responses. Runs +//! [`unwrap_markdown_fences`] first so that `` ```md ``/`` ```markdown `` +//! fences containing tables are stripped and `pulldown-cmark` sees raw +//! table syntax instead of fenced code. +//! +//! ## Why fence unwrapping exists +//! +//! LLM agents frequently wrap tables in `` ```markdown `` fences, treating +//! them as code. Without unwrapping, `pulldown-cmark` parses those lines +//! as a fenced code block and renders them as monospace code rather than a +//! structured table. The unwrapper is intentionally conservative: it +//! buffers the entire fence body before deciding, only unwraps fences whose +//! info string is `md` or `markdown` AND whose body contains a +//! header+delimiter pair, and degrades gracefully on unclosed fences. use ratatui::text::Line; +use std::borrow::Cow; +use std::ops::Range; use std::path::Path; -/// Render markdown into `lines` while resolving local file-link display relative to `cwd`. +use crate::table_detect; + +/// Render markdown source to styled ratatui lines and append them to `lines`. /// /// Callers that already know the session working directory should pass it here so streamed and /// non-streamed rendering show the same relative path text even if the process cwd differs. @@ -19,6 +44,253 @@ pub(crate) fn append_markdown( crate::render::line_utils::push_owned_lines(&rendered.lines, lines); } +/// Render an agent message to styled ratatui lines. +/// +/// Before rendering, the source is passed through [`unwrap_markdown_fences`] so that tables +/// wrapped in `` ```md `` fences are rendered as native tables rather than code blocks. +/// Non-markdown fences (e.g. `rust`, `sh`) are left +/// intact. +#[cfg(test)] +pub(crate) fn append_markdown_agent( + markdown_source: &str, + width: Option, + lines: &mut Vec>, +) { + append_markdown_agent_with_cwd(markdown_source, width, /*cwd*/ None, lines); +} + +/// Render an agent message while resolving local file links relative to `cwd`. +pub(crate) fn append_markdown_agent_with_cwd( + markdown_source: &str, + width: Option, + cwd: Option<&Path>, + lines: &mut Vec>, +) { + let normalized = unwrap_markdown_fences(markdown_source); + let rendered = + crate::markdown_render::render_markdown_text_with_width_and_cwd(&normalized, width, cwd); + crate::render::line_utils::push_owned_lines(&rendered.lines, lines); +} + +/// Strip `` ```md ``/`` ```markdown `` fences that contain tables, emitting their content as bare +/// markdown so `pulldown-cmark` parses the tables natively. +/// +/// Fences whose info string is not `md` or `markdown` are passed through unchanged. Markdown +/// fences that do *not* contain a table (detected by checking for a header row + delimiter row) +/// are also passed through so that non-table markdown inside a fence still renders as a code +/// block. +/// +/// The fence unwrapping is intentionally conservative: it buffers the entire fence body before +/// deciding, and an unclosed fence at end-of-input is re-emitted with its opening line so partial +/// streams degrade to code display. +fn unwrap_markdown_fences<'a>(markdown_source: &'a str) -> Cow<'a, str> { + // Zero-copy fast path: most messages contain no fences at all. + if !markdown_source.contains("```") && !markdown_source.contains("~~~") { + return Cow::Borrowed(markdown_source); + } + + #[derive(Clone, Copy)] + struct Fence { + marker: u8, + len: usize, + is_blockquoted: bool, + } + + // Strip a trailing newline and up to 3 leading spaces, returning the + // trimmed slice. Returns `None` when the line has 4+ leading spaces + // (which makes it an indented code line per CommonMark). + fn strip_line_indent(line: &str) -> Option<&str> { + let without_newline = line.strip_suffix('\n').unwrap_or(line); + let mut byte_idx = 0usize; + let mut column = 0usize; + for b in without_newline.as_bytes() { + match b { + b' ' => { + byte_idx += 1; + column += 1; + } + b'\t' => { + byte_idx += 1; + column += 4; + } + _ => break, + } + if column >= 4 { + return None; + } + } + Some(&without_newline[byte_idx..]) + } + + // Parse an opening fence line, returning the fence metadata and whether + // the fence info string indicates markdown content. + fn parse_open_fence(line: &str) -> Option<(Fence, bool)> { + let trimmed = strip_line_indent(line)?; + let is_blockquoted = trimmed.trim_start().starts_with('>'); + let fence_scan_text = table_detect::strip_blockquote_prefix(trimmed); + let (marker, len) = table_detect::parse_fence_marker(fence_scan_text)?; + let is_markdown = table_detect::is_markdown_fence_info(fence_scan_text, len); + Some(( + Fence { + marker: marker as u8, + len, + is_blockquoted, + }, + is_markdown, + )) + } + + fn is_close_fence(line: &str, fence: Fence) -> bool { + let Some(trimmed) = strip_line_indent(line) else { + return false; + }; + let fence_scan_text = if fence.is_blockquoted { + if !trimmed.trim_start().starts_with('>') { + return false; + } + table_detect::strip_blockquote_prefix(trimmed) + } else { + trimmed + }; + if let Some((marker, len)) = table_detect::parse_fence_marker(fence_scan_text) { + marker as u8 == fence.marker + && len >= fence.len + && fence_scan_text[len..].trim().is_empty() + } else { + false + } + } + + fn markdown_fence_contains_table(content: &str, is_blockquoted_fence: bool) -> bool { + let mut previous_line: Option<&str> = None; + for line in content.lines() { + let text = if is_blockquoted_fence { + table_detect::strip_blockquote_prefix(line) + } else { + line + }; + let trimmed = text.trim(); + if trimmed.is_empty() { + previous_line = None; + continue; + } + + if let Some(previous) = previous_line + && table_detect::is_table_header_line(previous) + && !table_detect::is_table_delimiter_line(previous) + && table_detect::is_table_delimiter_line(trimmed) + { + return true; + } + + previous_line = Some(trimmed); + } + false + } + + fn content_from_ranges(source: &str, ranges: &[Range]) -> String { + let total_len: usize = ranges.iter().map(ExactSizeIterator::len).sum(); + let mut content = String::with_capacity(total_len); + for range in ranges { + content.push_str(&source[range.start..range.end]); + } + content + } + + struct MarkdownCandidateData { + fence: Fence, + opening_range: Range, + content_ranges: Vec>, + } + + // Box the large variant to keep ActiveFence small (~pointer-sized). + enum ActiveFence { + Passthrough(Fence), + MarkdownCandidate(Box), + } + + let mut out = String::with_capacity(markdown_source.len()); + let mut active_fence: Option = None; + let mut source_offset = 0usize; + + let mut push_source_range = |range: Range| { + if !range.is_empty() { + out.push_str(&markdown_source[range]); + } + }; + + for line in markdown_source.split_inclusive('\n') { + let line_start = source_offset; + source_offset += line.len(); + let line_range = line_start..source_offset; + + if let Some(active) = active_fence.take() { + match active { + ActiveFence::Passthrough(fence) => { + push_source_range(line_range); + if !is_close_fence(line, fence) { + active_fence = Some(ActiveFence::Passthrough(fence)); + } + } + ActiveFence::MarkdownCandidate(mut data) => { + if is_close_fence(line, data.fence) { + if markdown_fence_contains_table( + &content_from_ranges(markdown_source, &data.content_ranges), + data.fence.is_blockquoted, + ) { + for range in data.content_ranges { + push_source_range(range); + } + } else { + push_source_range(data.opening_range); + for range in data.content_ranges { + push_source_range(range); + } + push_source_range(line_range); + } + } else { + data.content_ranges.push(line_range); + active_fence = Some(ActiveFence::MarkdownCandidate(data)); + } + } + } + continue; + } + + if let Some((fence, is_markdown)) = parse_open_fence(line) { + if is_markdown { + active_fence = Some(ActiveFence::MarkdownCandidate(Box::new( + MarkdownCandidateData { + fence, + opening_range: line_range, + content_ranges: Vec::new(), + }, + ))); + } else { + push_source_range(line_range); + active_fence = Some(ActiveFence::Passthrough(fence)); + } + continue; + } + + push_source_range(line_range); + } + + if let Some(active) = active_fence { + match active { + ActiveFence::Passthrough(_) => {} + ActiveFence::MarkdownCandidate(data) => { + push_source_range(data.opening_range); + for range in data.content_ranges { + push_source_range(range); + } + } + } + } + + Cow::Owned(out) +} + #[cfg(test)] mod tests { use super::*; @@ -118,4 +390,110 @@ mod tests { "did not expect a split into ['1.', 'Tight item']; got: {lines:?}" ); } + + #[test] + fn append_markdown_agent_unwraps_markdown_fences_for_table_rendering() { + let src = "```markdown\n| A | B |\n|---|---|\n| 1 | 2 |\n```\n"; + let mut out = Vec::new(); + append_markdown_agent(src, /*width*/ None, &mut out); + let rendered = lines_to_strings(&out); + assert!(rendered.iter().any(|line| line.contains("┌"))); + assert!(rendered.iter().any(|line| line.contains("│ 1 │ 2 │"))); + } + + #[test] + fn append_markdown_agent_unwraps_markdown_fences_for_no_outer_table_rendering() { + let src = "```md\nCol A | Col B | Col C\n--- | --- | ---\nx | y | z\n10 | 20 | 30\n```\n"; + let mut out = Vec::new(); + append_markdown_agent(src, /*width*/ None, &mut out); + let rendered = lines_to_strings(&out); + assert!(rendered.iter().any(|line| line.contains("┌"))); + assert!( + rendered + .iter() + .any(|line| line.contains("│ Col A │ Col B │ Col C │")) + ); + assert!( + !rendered + .iter() + .any(|line| line.trim() == "Col A | Col B | Col C") + ); + } + + #[test] + fn append_markdown_agent_unwraps_markdown_fences_for_two_column_no_outer_table() { + let src = "```md\nA | B\n--- | ---\nleft | right\n```\n"; + let mut out = Vec::new(); + append_markdown_agent(src, /*width*/ None, &mut out); + let rendered = lines_to_strings(&out); + assert!(rendered.iter().any(|line| line.contains("┌"))); + assert!(rendered.iter().any(|line| line.contains("│ A"))); + assert!(!rendered.iter().any(|line| line.trim() == "A | B")); + } + + #[test] + fn append_markdown_agent_unwraps_markdown_fences_for_single_column_table() { + let src = "```md\n| Only |\n|---|\n| value |\n```\n"; + let mut out = Vec::new(); + append_markdown_agent(src, /*width*/ None, &mut out); + let rendered = lines_to_strings(&out); + assert!(rendered.iter().any(|line| line.contains("┌"))); + assert!(!rendered.iter().any(|line| line.trim() == "| Only |")); + } + + #[test] + fn append_markdown_agent_keeps_non_markdown_fences_as_code() { + let src = "```rust\n| A | B |\n|---|---|\n| 1 | 2 |\n```\n"; + let mut out = Vec::new(); + append_markdown_agent(src, /*width*/ None, &mut out); + let rendered = lines_to_strings(&out); + assert_eq!( + rendered, + vec![ + "| A | B |".to_string(), + "|---|---|".to_string(), + "| 1 | 2 |".to_string(), + ] + ); + } + + #[test] + fn append_markdown_agent_unwraps_blockquoted_markdown_fence_table() { + let src = "> ```markdown\n> | A | B |\n> |---|---|\n> | 1 | 2 |\n> ```\n"; + let rendered = unwrap_markdown_fences(src); + assert!( + !rendered.contains("```"), + "expected markdown fence markers to be removed: {rendered:?}" + ); + } + + #[test] + fn append_markdown_agent_keeps_non_blockquoted_markdown_fence_with_blockquote_table_example() { + let src = "```markdown\n> | A | B |\n> |---|---|\n> | 1 | 2 |\n```\n"; + let normalized = unwrap_markdown_fences(src); + assert_eq!(normalized, src); + } + + #[test] + fn append_markdown_agent_keeps_markdown_fence_when_content_is_not_table() { + let src = "```markdown\n**bold**\n```\n"; + let mut out = Vec::new(); + append_markdown_agent(src, /*width*/ None, &mut out); + let rendered = lines_to_strings(&out); + assert_eq!(rendered, vec!["**bold**".to_string()]); + } + + #[test] + fn unwrap_markdown_fences_repro_keeps_fence_without_header_delimiter_pair() { + let src = "```markdown\n| A | B |\nnot a delimiter row\n| --- | --- |\n# Heading\n```\n"; + let normalized = unwrap_markdown_fences(src); + assert_eq!(normalized, src); + } + + #[test] + fn append_markdown_agent_keeps_markdown_fence_with_blank_line_between_header_and_delimiter() { + let src = "```markdown\n| A | B |\n\n|---|---|\n| 1 | 2 |\n```\n"; + let rendered = unwrap_markdown_fences(src); + assert_eq!(rendered, src); + } } diff --git a/codex-rs/tui/src/markdown_render.rs b/codex-rs/tui/src/markdown_render.rs index 3dfef1d9d8ce..3d7d8208c428 100644 --- a/codex-rs/tui/src/markdown_render.rs +++ b/codex-rs/tui/src/markdown_render.rs @@ -1,16 +1,50 @@ -//! Markdown rendering for the TUI transcript. +//! Low-level markdown event renderer for the TUI transcript. +//! +//! This module consumes `pulldown-cmark` events and emits styled `ratatui` +//! lines, including table layout, width-aware wrapping, and local file-link +//! display. It is the final rendering stage used by higher-level helpers in +//! `markdown.rs`. //! //! This renderer intentionally treats local file links differently from normal web links. For //! local paths, the displayed text comes from the destination, not the markdown label, so //! transcripts show the real file target (including normalized location suffixes) and can shorten //! absolute paths relative to a known working directory. +//! +//! ## Table rendering pipeline +//! +//! When the parser emits `Tag::Table` .. `TagEnd::Table`, the writer +//! accumulates header and body rows into a `TableState`, then hands it to +//! `render_table_lines` which runs this pipeline: +//! +//! 1. **Filter spillover rows** -- heuristic extraction of rows that are +//! artifacts of pulldown-cmark's lenient parsing. +//! 2. **Normalize column counts** -- pad or truncate so every row matches the +//! alignment count. +//! 3. **Compute column widths** -- allocate widths with Narrative/Structured +//! priority and iterative shrinking. +//! 4. **Render box grid** -- Unicode borders (`┌───┬───┐`) or fallback to pipe +//! format when the minimum cannot fit. +//! 5. **Append spillover** -- extracted spillover rows rendered as plain text +//! after the table. +//! +//! ## Width allocation +//! +//! Columns are classified as Narrative (long prose, >= 4 avg words or >= 28 +//! avg char width) or Structured (short tokens). The shrink loop removes +//! one character at a time, preferring Narrative columns, until the total +//! fits the available width. A guard cost penalises shrinking below a +//! column's header token width. When even 3-char-wide columns cannot fit, +//! the table falls back to pipe-delimited format. use crate::render::highlight::highlight_code_to_lines; use crate::render::line_utils::line_to_static; +use crate::render::line_utils::push_owned_lines; use crate::wrapping::RtOptions; use crate::wrapping::adaptive_wrap_line; +use crate::wrapping::word_wrap_line; use codex_utils_string::normalize_markdown_hash_location_suffix; use dirs::home_dir; +use pulldown_cmark::Alignment; use pulldown_cmark::CodeBlockKind; use pulldown_cmark::CowStr; use pulldown_cmark::Event; @@ -20,13 +54,16 @@ use pulldown_cmark::Parser; use pulldown_cmark::Tag; use pulldown_cmark::TagEnd; use ratatui::style::Style; +use ratatui::style::Stylize; use ratatui::text::Line; use ratatui::text::Span; use ratatui::text::Text; use regex_lite::Regex; +use std::ops::Range; use std::path::Path; use std::path::PathBuf; use std::sync::LazyLock; +use unicode_width::UnicodeWidthStr; use url::Url; struct MarkdownStyles { @@ -48,8 +85,6 @@ struct MarkdownStyles { impl Default for MarkdownStyles { fn default() -> Self { - use ratatui::style::Stylize; - Self { h1: Style::new().bold().underlined(), h2: Style::new().bold(), @@ -86,11 +121,155 @@ impl IndentContext { } } +/// Styled content of a single cell in the table being parsed. +/// +/// A cell can contain multiple lines (hard breaks inside the cell) and rich inline spans (bold, +/// code, links). The `plain_text()` projection is used for column-width measurement; the styled +/// `lines` are used for final rendering. +#[derive(Clone, Debug, Default)] +struct TableCell { + lines: Vec>, +} + +// TableCell mutators inlined — called per-span during table event parsing. +impl TableCell { + #[inline] + fn ensure_line(&mut self) { + if self.lines.is_empty() { + self.lines.push(Line::default()); + } + } + + #[inline] + fn push_span(&mut self, span: Span<'static>) { + self.ensure_line(); + if let Some(line) = self.lines.last_mut() { + line.push_span(span); + } + } + + #[inline] + fn hard_break(&mut self) { + self.lines.push(Line::default()); + } + + fn plain_text(&self) -> String { + use std::fmt::Write; + let mut buf = String::new(); + for (i, line) in self.lines.iter().enumerate() { + if i > 0 { + buf.push(' '); + } + for span in &line.spans { + let _ = write!(buf, "{}", span.content); + } + } + buf + } +} + +/// Accumulates pulldown-cmark table events into a structured representation. +/// +/// `TableState` is created on `Tag::Table` and consumed on `TagEnd::Table`. Between those events, +/// the Writer delegates cell content (text, code, html, breaks) into the `current_cell`, which is +/// flushed into `current_row` on `TagEnd::TableCell`, then into `header`/`rows` on row/head end +/// events. +#[derive(Debug)] +struct TableBodyRow { + cells: Vec, + has_table_pipe_syntax: bool, +} + +#[derive(Debug)] +struct TableState { + alignments: Vec, + header: Option>, + rows: Vec, + current_row: Option>, + current_row_has_table_pipe_syntax: bool, + current_cell: Option, + in_header: bool, +} + +impl TableState { + fn new(alignments: Vec) -> Self { + Self { + alignments, + header: None, + rows: Vec::new(), + current_row: None, + current_row_has_table_pipe_syntax: false, + current_cell: None, + in_header: false, + } + } +} + +/// Rendered table output split by wrapping behavior. +/// +/// `table_lines` are either prewrapped grid rows (box rendering) or pipe +/// fallback rows that should still pass through normal wrapping. +/// `spillover_lines` are prose rows extracted from parser artifacts and should +/// be routed through normal wrapping. +struct RenderedTableLines { + table_lines: Vec>, + table_lines_prewrapped: bool, + spillover_lines: Vec>, +} + +/// Classification of a table column for width-allocation priority. +/// +/// Narrative columns (long prose, many words per cell) are shrunk first when the table exceeds +/// available width. Structured columns (short tokens like dates, status words, numbers) are +/// preserved as long as possible to keep their content on a single line. +/// +/// The heuristic is simple: >= 4 average words per cell OR >= 28 average character width → +/// Narrative. Everything else → Structured. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum TableColumnKind { + /// Long-form prose content (>= 4 avg words/cell or >= 28 avg char width). + Narrative, + /// Short, token-like content that should resist wrapping. + Structured, +} + +/// Per-column statistics used to drive the width-allocation algorithm. +/// +/// Collected in a single pass over the header and body rows before any +/// shrinking decisions are made. +#[derive(Clone, Debug)] +struct TableColumnMetrics { + /// Widest cell content (display width) across header and all body rows. + max_width: usize, + /// Display width of the longest whitespace-delimited token in the header. + header_token_width: usize, + /// Display width of the longest whitespace-delimited token across body rows. + body_token_width: usize, + /// Average number of whitespace-delimited words per non-empty body cell. + avg_words_per_cell: f64, + /// Average display width of non-empty body cells. + avg_cell_width: f64, + /// Classification derived from `avg_words_per_cell` and `avg_cell_width`. + kind: TableColumnKind, +} + +/// Render markdown with default wrapping behavior. +/// +/// Use this when the caller does not have a concrete render width yet (for +/// example, snapshot tests or contexts that intentionally defer wrapping). If +/// a viewport width is known, prefer [`render_markdown_text_with_width`] so +/// table fallback and line wrapping decisions match the visible terminal. pub fn render_markdown_text(input: &str) -> Text<'static> { render_markdown_text_with_width(input, /*width*/ None) } -/// Render markdown using the current process working directory for local file-link display. +/// Render markdown constrained to a known terminal width. +/// +/// The renderer preserves table structure when possible and falls back to +/// pipe-table output when a box table cannot fit the available width. Passing +/// `None` keeps intrinsic line widths and disables width-driven wrapping in the +/// markdown writer. Local file links render relative to the current process +/// working directory. pub(crate) fn render_markdown_text_with_width(input: &str, width: Option) -> Text<'static> { let cwd = std::env::current_dir().ok(); render_markdown_text_with_width_and_cwd(input, width, cwd.as_deref()) @@ -108,8 +287,9 @@ pub(crate) fn render_markdown_text_with_width_and_cwd( ) -> 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, cwd); + options.insert(Options::ENABLE_TABLES); + let parser = Parser::new_ext(input, options).into_offset_iter(); + let mut w = Writer::new(input, parser, width, cwd); w.run(); w.text } @@ -144,10 +324,17 @@ static HASH_LOCATION_SUFFIX_RE: LazyLock = Err(error) => panic!("invalid hash location regex: {error}"), }); +/// Stateful pulldown-cmark event consumer that builds styled `ratatui` output. +/// +/// Tracks inline style nesting, indent/blockquote context, list numbering, +/// and an optional `TableState` for accumulating table events. The +/// `wrap_width` field enables width-aware line wrapping and table column +/// allocation; when `None`, lines keep their intrinsic width. struct Writer<'a, I> where - I: Iterator>, + I: Iterator, Range)>, { + input: &'a str, iter: I, text: Text<'static>, styles: MarkdownStyles, @@ -172,14 +359,16 @@ where current_subsequent_indent: Vec>, current_line_style: Style, current_line_in_code_block: bool, + table_state: Option, } impl<'a, I> Writer<'a, I> where - I: Iterator>, + I: Iterator, Range)>, { - fn new(iter: I, wrap_width: Option, cwd: Option<&Path>) -> Self { + fn new(input: &'a str, iter: I, wrap_width: Option, cwd: Option<&Path>) -> Self { Self { + input, iter, text: Text::default(), styles: MarkdownStyles::default(), @@ -204,20 +393,21 @@ where current_subsequent_indent: Vec::new(), current_line_style: Style::default(), current_line_in_code_block: false, + table_state: None, } } fn run(&mut self) { - while let Some(ev) = self.iter.next() { - self.handle_event(ev); + while let Some((ev, range)) = self.iter.next() { + self.handle_event(ev, range); } self.flush_current_line(); } - fn handle_event(&mut self, event: Event<'a>) { + fn handle_event(&mut self, event: Event<'a>, range: Range) { self.prepare_for_event(&event); match event { - Event::Start(tag) => self.start_tag(tag), + Event::Start(tag) => self.start_tag(tag, range), Event::End(tag) => self.end_tag(tag), Event::Text(text) => self.text(text), Event::Code(code) => self.code(code), @@ -255,7 +445,7 @@ where self.push_line(Line::default()); } - fn start_tag(&mut self, tag: Tag<'a>) { + fn start_tag(&mut self, tag: Tag<'a>, range: Range) { match tag { Tag::Paragraph => self.start_paragraph(), Tag::Heading { level, .. } => self.start_heading(level), @@ -277,12 +467,12 @@ where Tag::Strong => self.push_inline_style(self.styles.strong), Tag::Strikethrough => self.push_inline_style(self.styles.strikethrough), Tag::Link { dest_url, .. } => self.push_link(dest_url.to_string()), + Tag::Table(alignments) => self.start_table(alignments), + Tag::TableHead => self.start_table_head(), + Tag::TableRow => self.start_table_row(range), + Tag::TableCell => self.start_table_cell(), Tag::HtmlBlock | Tag::FootnoteDefinition(_) - | Tag::Table(_) - | Tag::TableHead - | Tag::TableRow - | Tag::TableCell | Tag::Image { .. } | Tag::MetadataBlock(_) => {} } @@ -306,18 +496,21 @@ where } TagEnd::Emphasis | TagEnd::Strong | TagEnd::Strikethrough => self.pop_inline_style(), TagEnd::Link => self.pop_link(), + TagEnd::Table => self.end_table(), + TagEnd::TableHead => self.end_table_head(), + TagEnd::TableRow => self.end_table_row(), + TagEnd::TableCell => self.end_table_cell(), TagEnd::HtmlBlock | TagEnd::FootnoteDefinition - | TagEnd::Table - | TagEnd::TableHead - | TagEnd::TableRow - | TagEnd::TableCell | TagEnd::Image | TagEnd::MetadataBlock(_) => {} } } fn start_paragraph(&mut self) { + if self.in_table_cell() { + return; + } if self.needs_newline { self.push_blank_line(); } @@ -327,12 +520,18 @@ where } fn end_paragraph(&mut self) { + if self.in_table_cell() { + return; + } self.needs_newline = true; self.in_paragraph = false; self.pending_marker_line = false; } fn start_heading(&mut self, level: HeadingLevel) { + if self.in_table_cell() { + return; + } if self.needs_newline { self.push_line(Line::default()); self.needs_newline = false; @@ -352,11 +551,17 @@ where } fn end_heading(&mut self) { + if self.in_table_cell() { + return; + } self.needs_newline = true; self.pop_inline_style(); } fn start_blockquote(&mut self) { + if self.in_table_cell() { + return; + } if self.needs_newline { self.push_blank_line(); self.needs_newline = false; @@ -369,6 +574,9 @@ where } fn end_blockquote(&mut self) { + if self.in_table_cell() { + return; + } self.indent_stack.pop(); self.needs_newline = true; } @@ -378,6 +586,11 @@ where return; } self.line_ends_with_local_link_target = false; + if self.in_table_cell() { + self.push_text_to_table_cell(&text); + return; + } + if self.pending_marker_line { self.push_line(Line::default()); } @@ -431,6 +644,11 @@ where return; } self.line_ends_with_local_link_target = false; + if self.in_table_cell() { + self.push_span_to_table_cell(Span::from(code.into_string()).style(self.styles.code)); + return; + } + if self.pending_marker_line { self.push_line(Line::default()); self.pending_marker_line = false; @@ -444,6 +662,19 @@ where return; } self.line_ends_with_local_link_target = false; + if self.in_table_cell() { + let style = self.inline_styles.last().copied().unwrap_or_default(); + for (i, line) in html.lines().enumerate() { + if i > 0 { + self.push_table_cell_hard_break(); + } + self.push_span_to_table_cell(Span::styled(line.to_string(), style)); + } + if !inline { + self.push_table_cell_hard_break(); + } + return; + } self.pending_marker_line = false; for (i, line) in html.lines().enumerate() { if self.needs_newline { @@ -464,6 +695,10 @@ where return; } self.line_ends_with_local_link_target = false; + if self.in_table_cell() { + self.push_table_cell_hard_break(); + return; + } self.push_line(Line::default()); } @@ -471,6 +706,11 @@ where if self.suppressing_local_link_label() { return; } + if self.in_table_cell() { + let style = self.inline_styles.last().copied().unwrap_or_default(); + self.push_span_to_table_cell(Span::styled(" ".to_string(), style)); + return; + } if self.line_ends_with_local_link_target { self.pending_local_link_soft_break = true; self.line_ends_with_local_link_target = false; @@ -593,6 +833,722 @@ where self.indent_stack.pop(); } + fn start_table(&mut self, alignments: Vec) { + self.flush_current_line(); + if self.needs_newline { + self.push_blank_line(); + self.needs_newline = false; + } + self.table_state = Some(TableState::new(alignments)); + } + + fn end_table(&mut self) { + let Some(table_state) = self.table_state.take() else { + return; + }; + + let RenderedTableLines { + table_lines, + table_lines_prewrapped, + spillover_lines, + } = self.render_table_lines(table_state); + let mut pending_marker_line = self.pending_marker_line; + for line in table_lines { + if table_lines_prewrapped { + self.push_prewrapped_line(line, pending_marker_line); + } else { + self.push_line(line); + self.flush_current_line(); + } + pending_marker_line = false; + } + self.pending_marker_line = false; + for spillover_line in spillover_lines { + self.push_line(spillover_line); + self.flush_current_line(); + } + self.needs_newline = true; + } + + fn start_table_head(&mut self) { + if let Some(table_state) = self.table_state.as_mut() { + table_state.in_header = true; + table_state.current_row = Some(Vec::new()); + } + } + + fn end_table_head(&mut self) { + let Some(table_state) = self.table_state.as_mut() else { + return; + }; + if let Some(current_cell) = table_state.current_cell.take() { + table_state + .current_row + .get_or_insert_with(Vec::new) + .push(current_cell); + } + if let Some(row) = table_state.current_row.take() { + table_state.header = Some(row); + } + table_state.in_header = false; + } + + fn start_table_row(&mut self, source_range: Range) { + let has_table_pipe_syntax = self.has_table_row_boundary_pipe(source_range); + if let Some(table_state) = self.table_state.as_mut() { + table_state.current_row = Some(Vec::new()); + table_state.current_row_has_table_pipe_syntax = has_table_pipe_syntax; + } + } + + fn has_table_row_boundary_pipe(&self, source_range: Range) -> bool { + let Some(source) = self.input.get(source_range) else { + return false; + }; + let source = source.trim(); + source.starts_with('|') || source.ends_with('|') + } + + fn end_table_row(&mut self) { + let Some(table_state) = self.table_state.as_mut() else { + return; + }; + + if let Some(current_cell) = table_state.current_cell.take() { + table_state + .current_row + .get_or_insert_with(Vec::new) + .push(current_cell); + } + + let Some(row) = table_state.current_row.take() else { + return; + }; + + if table_state.in_header { + table_state.header = Some(row); + } else { + table_state.rows.push(TableBodyRow { + cells: row, + has_table_pipe_syntax: table_state.current_row_has_table_pipe_syntax, + }); + } + table_state.current_row_has_table_pipe_syntax = false; + } + + fn start_table_cell(&mut self) { + if let Some(table_state) = self.table_state.as_mut() { + table_state.current_cell = Some(TableCell::default()); + } + } + + fn end_table_cell(&mut self) { + let Some(table_state) = self.table_state.as_mut() else { + return; + }; + + if let Some(cell) = table_state.current_cell.take() { + table_state + .current_row + .get_or_insert_with(Vec::new) + .push(cell); + } + } + + fn in_table_cell(&self) -> bool { + self.table_state + .as_ref() + .and_then(|table_state| table_state.current_cell.as_ref()) + .is_some() + } + + fn push_span_to_table_cell(&mut self, span: Span<'static>) { + if let Some(table_state) = self.table_state.as_mut() + && let Some(cell) = table_state.current_cell.as_mut() + { + cell.push_span(span); + } + } + + fn push_table_cell_hard_break(&mut self) { + if let Some(table_state) = self.table_state.as_mut() + && let Some(cell) = table_state.current_cell.as_mut() + { + cell.hard_break(); + } + } + + fn push_text_to_table_cell(&mut self, text: &str) { + let style = self.inline_styles.last().copied().unwrap_or_default(); + for (i, line) in text.lines().enumerate() { + if i > 0 { + self.push_table_cell_hard_break(); + } + self.push_span_to_table_cell(Span::styled(line.to_string(), style)); + } + } + + /// Convert a completed `TableState` into styled `Line`s with Unicode + /// box-drawing borders. + /// + /// Pipeline: filter spillover rows -> normalize column counts -> compute + /// column widths -> render box grid (or fall back to pipe format if the + /// minimum column widths exceed available terminal width). Spillover rows + /// are appended as plain text after the table grid. + /// + /// Falls back to `render_table_pipe_fallback` (raw `| A | B |` format) + /// when `compute_column_widths` returns `None` (terminal too narrow for + /// even 3-char-wide columns). + fn render_table_lines(&self, mut table_state: TableState) -> RenderedTableLines { + let column_count = table_state.alignments.len(); + if column_count == 0 { + return RenderedTableLines { + table_lines: Vec::new(), + table_lines_prewrapped: true, + spillover_lines: Vec::new(), + }; + } + + let mut spillover_rows: Vec = Vec::with_capacity(4); + let mut rows: Vec> = Vec::with_capacity(table_state.rows.len()); + for (row_idx, row) in table_state.rows.iter().enumerate() { + let next_row = table_state.rows.get(row_idx + 1); + // pulldown-cmark accepts body rows without pipes, which can turn a following paragraph + // into a one-cell table row. For multi-column tables, treat those as spillover text + // rendered after the table. + if column_count > 1 && Self::is_spillover_row(row, next_row) { + if let Some(cell) = row.cells.first().cloned() { + spillover_rows.push(cell); + } + } else { + rows.push(row.cells.clone()); + } + } + + let mut header = table_state + .header + .take() + .unwrap_or_else(|| vec![TableCell::default(); column_count]); + Self::normalize_row(&mut header, column_count); + for row in &mut rows { + Self::normalize_row(row, column_count); + } + + let available_width = self.available_table_width(column_count); + let widths = + self.compute_column_widths(&header, &rows, &table_state.alignments, available_width); + let spillover_lines: Vec> = spillover_rows + .into_iter() + .flat_map(|spillover| spillover.lines) + .collect(); + + let Some(column_widths) = widths else { + return RenderedTableLines { + table_lines: self.render_table_pipe_fallback( + &header, + &rows, + &table_state.alignments, + ), + table_lines_prewrapped: false, + spillover_lines, + }; + }; + + let border_style = Style::new().dim(); + let mut out = Vec::with_capacity(3 + rows.len() * 2); + out.push(self.render_border_line('┌', '┬', '┐', &column_widths, border_style)); + out.extend(self.render_table_row( + &header, + &column_widths, + &table_state.alignments, + border_style, + )); + out.push(self.render_border_line('├', '┼', '┤', &column_widths, border_style)); + for row in &rows { + out.extend(self.render_table_row( + row, + &column_widths, + &table_state.alignments, + border_style, + )); + } + out.push(self.render_border_line('└', '┴', '┘', &column_widths, border_style)); + RenderedTableLines { + table_lines: out, + table_lines_prewrapped: true, + spillover_lines, + } + } + + fn normalize_row(row: &mut Vec, column_count: usize) { + row.truncate(column_count); + row.resize(column_count, TableCell::default()); + } + + /// subtracts the space eaten by border characters + fn available_table_width(&self, column_count: usize) -> Option { + self.wrap_width.map(|wrap_width| { + let prefix_width = + Self::spans_display_width(&self.prefix_spans(self.pending_marker_line)); + let reserved = prefix_width + 1 + (column_count * 3); + wrap_width.saturating_sub(reserved) + }) + } + + /// Allocate column widths for box-drawing table rendering. + /// + /// Each column starts at its natural (max cell content) width, then columns + /// are iteratively shrunk one character at a time until the total fits within + /// `available_width`. Narrative columns (long prose) are shrunk before + /// Structured columns (short tokens). Returns `None` when even the minimum + /// width (3 chars per column) cannot fit. + fn compute_column_widths( + &self, + header: &[TableCell], + rows: &[Vec], + alignments: &[Alignment], + available_width: Option, + ) -> Option> { + let min_column_width = 3usize; + let metrics = Self::collect_table_column_metrics(header, rows, alignments.len()); + let mut widths: Vec = metrics + .iter() + .map(|col| col.max_width.max(min_column_width)) + .collect(); + + let Some(max_width) = available_width else { + return Some(widths); + }; + let minimum_total = alignments.len() * min_column_width; + if max_width < minimum_total { + return None; + } + + let mut floors: Vec = metrics + .iter() + .map(|col| Self::preferred_column_floor(col, min_column_width)) + .collect(); + let mut floor_total: usize = floors.iter().sum(); + if floor_total > max_width { + // Relax preferred floors (starting with narrative columns) until we can satisfy the + // width budget. We still keep hard minimums. + while floor_total > max_width { + let Some((idx, _)) = floors + .iter() + .enumerate() + .filter(|(_, floor)| **floor > min_column_width) + .min_by_key(|(idx, floor)| { + let kind_priority = match metrics[*idx].kind { + TableColumnKind::Narrative => 0, + TableColumnKind::Structured => 1, + }; + (kind_priority, *floor) + }) + else { + break; + }; + + floors[idx] -= 1; + floor_total -= 1; + } + } + + let mut total_width: usize = widths.iter().sum(); + + while total_width > max_width { + let Some(idx) = Self::next_column_to_shrink(&widths, &floors, &metrics) else { + break; + }; + widths[idx] -= 1; + total_width -= 1; + } + + if total_width > max_width { + return None; + } + + Some(widths) + } + + fn collect_table_column_metrics( + header: &[TableCell], + rows: &[Vec], + column_count: usize, + ) -> Vec { + let mut metrics = Vec::with_capacity(column_count); + for column in 0..column_count { + let header_cell = &header[column]; + let header_plain = header_cell.plain_text(); + let header_token_width = Self::longest_token_width(&header_plain); + let mut max_width = Self::cell_display_width(header_cell); + let mut body_token_width = 0usize; + let mut total_words = 0usize; + let mut total_cells = 0usize; + let mut total_cell_width = 0usize; + + for row in rows { + let cell = &row[column]; + max_width = max_width.max(Self::cell_display_width(cell)); + let plain = cell.plain_text(); + body_token_width = body_token_width.max(Self::longest_token_width(&plain)); + let word_count = plain.split_whitespace().count(); + if word_count > 0 { + total_words += word_count; + total_cells += 1; + total_cell_width += plain.width(); + } + } + + let avg_words_per_cell = if total_cells == 0 { + header_plain.split_whitespace().count() as f64 + } else { + total_words as f64 / total_cells as f64 + }; + let avg_cell_width = if total_cells == 0 { + header_plain.width() as f64 + } else { + total_cell_width as f64 / total_cells as f64 + }; + let kind = if body_token_width >= 20 && avg_words_per_cell <= 2.0 { + // URL-like/token-heavy columns should resist collapse even if their + // average cell width is high. + TableColumnKind::Structured + } else if avg_words_per_cell >= 4.0 || avg_cell_width >= 28.0 { + TableColumnKind::Narrative + } else { + TableColumnKind::Structured + }; + + metrics.push(TableColumnMetrics { + max_width, + header_token_width, + body_token_width, + avg_words_per_cell, + avg_cell_width, + kind, + }); + } + + metrics + } + + /// Compute the preferred minimum width for a column before the shrink loop + /// starts reducing it further. + /// + /// Narrative columns floor at the header's longest token (capped at 10). + /// Structured columns floor at the larger of the header and body token widths + /// (body capped at 16). The result is clamped to `[min_column_width, max_width]`. + fn preferred_column_floor(metrics: &TableColumnMetrics, min_column_width: usize) -> usize { + let token_target = match metrics.kind { + TableColumnKind::Narrative => metrics.header_token_width.min(10), + TableColumnKind::Structured => metrics + .header_token_width + .max(metrics.body_token_width.min(16)), + }; + token_target.max(min_column_width).min(metrics.max_width) + } + + /// Pick the next column to shrink by one character during width allocation. + /// + /// Priority: Narrative columns are shrunk before Structured. Within the same + /// kind, the column with the most slack above its floor is chosen. A guard + /// cost is added when the width would fall below the header's longest token + /// (to avoid truncating column headers). + fn next_column_to_shrink( + widths: &[usize], + floors: &[usize], + metrics: &[TableColumnMetrics], + ) -> Option { + widths + .iter() + .enumerate() + .filter(|(idx, width)| **width > floors[*idx]) + .min_by_key(|(idx, width)| { + let slack = width.saturating_sub(floors[*idx]); + let kind_cost = match metrics[*idx].kind { + TableColumnKind::Narrative => 0i32, + TableColumnKind::Structured => 2i32, + }; + let header_guard = if **width <= metrics[*idx].header_token_width { + 3i32 + } else { + 0i32 + }; + let density_guard = if metrics[*idx].avg_words_per_cell >= 4.0 + || metrics[*idx].avg_cell_width >= 24.0 + { + 0i32 + } else { + 1i32 + }; + ( + kind_cost + header_guard + density_guard, + usize::MAX.saturating_sub(slack), + ) + }) + .map(|(idx, _)| idx) + } + + fn render_border_line( + &self, + left: char, + sep: char, + right: char, + column_widths: &[usize], + style: Style, + ) -> Line<'static> { + let mut spans = Vec::with_capacity(column_widths.len() * 2 + 1); + spans.push(Span::styled(String::from(left), style)); + for (idx, width) in column_widths.iter().enumerate() { + spans.push(Span::styled("─".repeat(*width + 2), style)); + if idx + 1 == column_widths.len() { + spans.push(Span::styled(String::from(right), style)); + } else { + spans.push(Span::styled(String::from(sep), style)); + } + } + Line::from(spans) + } + + fn render_table_row( + &self, + row: &[TableCell], + column_widths: &[usize], + alignments: &[Alignment], + border_style: Style, + ) -> Vec> { + let wrapped_cells: Vec>> = row + .iter() + .zip(column_widths) + .map(|(cell, width)| self.wrap_cell(cell, *width)) + .collect(); + let row_height = wrapped_cells.iter().map(Vec::len).max().unwrap_or(1); + + let mut out = Vec::with_capacity(row_height); + for row_line in 0..row_height { + let mut spans = Vec::new(); + spans.push(Span::styled("│", border_style)); + for (column, width) in column_widths.iter().enumerate() { + spans.push(Span::raw(" ")); + let line = wrapped_cells[column] + .get(row_line) + .cloned() + .unwrap_or_default(); + let line_width = Self::line_display_width(&line); + let remaining = width.saturating_sub(line_width); + let (left_padding, right_padding) = match alignments[column] { + Alignment::Left | Alignment::None => (0, remaining), + Alignment::Center => (remaining / 2, remaining - (remaining / 2)), + Alignment::Right => (remaining, 0), + }; + if left_padding > 0 { + spans.push(Span::raw(" ".repeat(left_padding))); + } + spans.extend(line.spans); + if right_padding > 0 { + spans.push(Span::raw(" ".repeat(right_padding))); + } + spans.push(Span::raw(" ")); + spans.push(Span::styled("│", border_style)); + } + out.push(Line::from(spans)); + } + out + } + + /// Render the table as raw pipe-delimited lines (`| A | B |`). + /// + /// Used when `compute_column_widths` returns `None` (terminal too narrow + /// for even 3-char-wide columns). Pipe characters inside cell content are + /// escaped as `\|` so downstream parsers keep cell boundaries intact. + fn render_table_pipe_fallback( + &self, + header: &[TableCell], + rows: &[Vec], + alignments: &[Alignment], + ) -> Vec> { + let mut out = Vec::new(); + out.push(Line::from(Self::row_to_pipe_string(header))); + out.push(Line::from(Self::alignments_to_pipe_delimiter(alignments))); + out.extend( + rows.iter() + .map(|row| Line::from(Self::row_to_pipe_string(row))), + ); + out + } + + fn row_to_pipe_string(row: &[TableCell]) -> String { + let mut out = String::new(); + out.push('|'); + for cell in row { + out.push(' '); + // Preserve literal `|` inside cell text in markdown fallback mode so + // downstream markdown parsers keep the cell content intact. + out.push_str(&cell.plain_text().replace('|', "\\|")); + out.push(' '); + out.push('|'); + } + out + } + + fn alignments_to_pipe_delimiter(alignments: &[Alignment]) -> String { + let mut out = String::new(); + out.push('|'); + for alignment in alignments { + let segment = match alignment { + Alignment::Left => ":---", + Alignment::Center => ":---:", + Alignment::Right => "---:", + Alignment::None => "---", + }; + out.push_str(segment); + out.push('|'); + } + out + } + + /// Wrap a single table cell's content to `width`, preserving rich inline + /// styling (bold, code, links) across wrapped lines. + /// + /// Each logical line within the cell (separated by hard breaks) is wrapped + /// independently. Empty cells produce a single blank line so the row grid + /// stays aligned. + fn wrap_cell(&self, cell: &TableCell, width: usize) -> Vec> { + if cell.lines.is_empty() { + return vec![Line::default()]; + } + let mut wrapped = Vec::new(); + for source_line in &cell.lines { + let rendered = word_wrap_line(source_line, RtOptions::new(width.max(1))); + if rendered.is_empty() { + wrapped.push(Line::default()); + } else { + push_owned_lines(&rendered, &mut wrapped); + }; + } + if wrapped.is_empty() { + wrapped.push(Line::default()); + } + wrapped + } + + /// Detect rows that are artifacts of pulldown-cmark's lenient table parsing. + /// + /// pulldown-cmark accepts body rows without leading pipes, which can absorb a + /// trailing paragraph as a single-cell row in a multi-column table. These + /// "spillover" rows are extracted and rendered as plain text after the table + /// grid so they don't appear as malformed table content. + /// + /// Heuristic: a row is spillover if its only non-empty cell is the first one + /// AND (a single-cell row lacked table pipe syntax, the content looks like + /// HTML, it's a label line followed by HTML content, or a trailing + /// HTML-intro label line). + fn is_spillover_row(row: &TableBodyRow, next_row: Option<&TableBodyRow>) -> bool { + let Some(first_text) = Self::first_non_empty_only_text(&row.cells) else { + return false; + }; + + if row.cells.len() == 1 && !row.has_table_pipe_syntax { + return true; + } + + if Self::looks_like_html_content(&first_text) { + return true; + } + + // Keep common intro + html-block spillover together: + // "HTML block:" followed by "
". + if first_text.trim_end().ends_with(':') { + if next_row + .and_then(|row| Self::first_non_empty_only_text(&row.cells)) + .is_some_and(|text| Self::looks_like_html_content(&text)) + { + return true; + } + + // pulldown can end the table before the corresponding HTML block line. + // In that case, treat trailing HTML-intro labels (e.g., "HTML block:") + // as spillover while keeping explicit sparse labels in real tables. + if next_row.is_none() && Self::looks_like_html_label_line(&first_text) { + return true; + } + } + + false + } + + fn first_non_empty_only_text(row: &[TableCell]) -> Option { + let first = row.first()?.plain_text(); + if first.trim().is_empty() { + return None; + } + let rest_empty = row[1..] + .iter() + .all(|cell| cell.plain_text().trim().is_empty()); + rest_empty.then_some(first) + } + + fn looks_like_html_content(text: &str) -> bool { + let bytes = text.as_bytes(); + for (idx, &byte) in bytes.iter().enumerate() { + if byte != b'<' { + continue; + } + + let mut tag_start = idx + 1; + if tag_start < bytes.len() && (bytes[tag_start] == b'/' || bytes[tag_start] == b'!') { + tag_start += 1; + } + + if bytes.get(tag_start).is_some_and(u8::is_ascii_alphabetic) + && bytes + .get(tag_start + 1..) + .is_some_and(|suffix| suffix.contains(&b'>')) + { + return true; + } + } + false + } + + fn looks_like_html_label_line(text: &str) -> bool { + let trimmed = text.trim(); + if !trimmed.ends_with(':') { + return false; + } + let prefix = trimmed.trim_end_matches(':').trim(); + prefix + .split_whitespace() + .any(|word| word.eq_ignore_ascii_case("html")) + } + + // Width-measurement helpers inlined — called per-cell during table column + // width computation, which runs on every re-render. + + #[inline] + fn spans_display_width(spans: &[Span<'_>]) -> usize { + spans.iter().map(|span| span.content.width()).sum() + } + + #[inline] + fn line_display_width(line: &Line<'_>) -> usize { + Self::spans_display_width(&line.spans) + } + + #[inline] + fn cell_display_width(cell: &TableCell) -> usize { + cell.lines + .iter() + .map(Self::line_display_width) + .max() + .unwrap_or(0) + } + + #[inline] + fn longest_token_width(text: &str) -> usize { + text.split_whitespace().map(str::width).max().unwrap_or(0) + } + fn push_inline_style(&mut self, style: Style) { let current = self.inline_styles.last().copied().unwrap_or_default(); let merged = current.patch(style); @@ -619,13 +1575,19 @@ where fn pop_link(&mut self) { if let Some(link) = self.link.take() { if link.show_destination { - self.push_span(" (".into()); - self.push_span(Span::styled(link.destination, self.styles.link)); - self.push_span(")".into()); - } else if let Some(local_target_display) = link.local_target_display { - if self.pending_marker_line { - self.push_line(Line::default()); + // Link destinations are rendered as " (url)" suffixes. When parsing table cells, + // append the suffix into the active cell buffer rather than the outer paragraph + // line to avoid detached url lines. + if self.in_table_cell() { + self.push_span_to_table_cell(" (".into()); + self.push_span_to_table_cell(Span::styled(link.destination, self.styles.link)); + self.push_span_to_table_cell(")".into()); + } else { + self.push_span(" (".into()); + self.push_span(Span::styled(link.destination, self.styles.link)); + self.push_span(")".into()); } + } else if let Some(local_target_display) = link.local_target_display { // Local file links are rendered as code-like path text so the transcript shows the // resolved target instead of arbitrary caller-provided label text. let style = self @@ -634,8 +1596,16 @@ where .copied() .unwrap_or_default() .patch(self.styles.code); - self.push_span(Span::styled(local_target_display, style)); - self.line_ends_with_local_link_target = true; + let span = Span::styled(local_target_display, style); + if self.in_table_cell() { + self.push_span_to_table_cell(span); + } else { + if self.pending_marker_line { + self.push_line(Line::default()); + } + self.push_span(span); + self.line_ends_with_local_link_target = true; + } } } } @@ -659,13 +1629,13 @@ where .subsequent_indent(self.current_subsequent_indent.clone().into()); for wrapped in adaptive_wrap_line(&line, opts) { let owned = line_to_static(&wrapped).style(style); - self.text.lines.push(owned); + self.push_output_line(owned); } } else { let mut spans = self.current_initial_indent.clone(); let mut line = line; spans.append(&mut line.spans); - self.text.lines.push(Line::from_iter(spans).style(style)); + self.push_output_line(Line::from_iter(spans).style(style)); } self.current_initial_indent.clear(); self.current_subsequent_indent.clear(); @@ -674,12 +1644,36 @@ where } } + /// Push a line that has already been laid out at the correct width, skipping + /// word wrapping. + /// + /// Table lines are pre-formatted with exact column widths and box-drawing + /// borders. Passing them through `word_wrap_line` would break the grid at + /// arbitrary positions. This method prepends the indent/blockquote prefix + /// and pushes directly to `self.text.lines`. + fn is_blockquote_active(&self) -> bool { + self.indent_stack + .iter() + .any(|ctx| ctx.prefix.iter().any(|p| p.content.contains('>'))) + } + + fn push_prewrapped_line(&mut self, line: Line<'static>, pending_marker_line: bool) { + self.flush_current_line(); + let blockquote_active = self.is_blockquote_active(); + let style = if blockquote_active { + self.styles.blockquote.patch(line.style) + } else { + line.style + }; + + let mut spans = self.prefix_spans(pending_marker_line); + spans.extend(line.spans); + self.push_output_line(Line::from(spans).style(style)); + } + fn push_line(&mut self, line: Line<'static>) { self.flush_current_line(); - let blockquote_active = self - .indent_stack - .iter() - .any(|ctx| ctx.prefix.iter().any(|s| s.content.contains('>'))); + let blockquote_active = self.is_blockquote_active(); let style = if blockquote_active { self.styles.blockquote } else { @@ -708,13 +1702,17 @@ where fn push_blank_line(&mut self) { self.flush_current_line(); if self.indent_stack.iter().all(|ctx| ctx.is_list) { - self.text.lines.push(Line::default()); + self.push_output_line(Line::default()); } else { self.push_line(Line::default()); self.flush_current_line(); } } + fn push_output_line(&mut self, line: Line<'static>) { + self.text.lines.push(line); + } + fn prefix_spans(&self, pending_marker_line: bool) -> Vec> { let mut prefix: Vec> = Vec::new(); let last_marker_index = if pending_marker_line { @@ -1156,4 +2154,253 @@ mod tests { "CRLF code block should not produce extra blank lines: {lines:?}" ); } + + #[test] + fn wrap_cell_preserves_hard_break_lines() { + let mut cell = TableCell::default(); + cell.push_span("first line".into()); + cell.hard_break(); + cell.push_span("second line".into()); + + let writer = W::new("", std::iter::empty(), Some(80), /*cwd*/ None); + let wrapped = writer.wrap_cell(&cell, /*width*/ 40); + let rendered = wrapped + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.clone()) + .collect::() + }) + .collect::>(); + + assert_eq!( + rendered, + vec!["first line".to_string(), "second line".to_string()] + ); + } + + // --------------------------------------------------------------- + // Type alias for calling private associated functions on Writer. + // --------------------------------------------------------------- + type W<'a> = Writer<'a, std::iter::Empty<(Event<'a>, Range)>>; + + /// Build a single-line `TableCell` from plain text. + fn make_cell(text: &str) -> TableCell { + let mut cell = TableCell::default(); + cell.push_span(Span::raw(text.to_string())); + cell + } + + fn make_body_row(cells: Vec, has_table_pipe_syntax: bool) -> TableBodyRow { + TableBodyRow { + cells, + has_table_pipe_syntax, + } + } + + // ===== Column-metrics unit tests ===== + + #[test] + fn column_classification_narrative_by_word_count() { + // Col 0: short tokens (1-2 words each) → Structured + // Col 1: prose (≥4 words per cell) → Narrative + let header = vec![make_cell("ID"), make_cell("Description")]; + let rows = vec![ + vec![make_cell("1"), make_cell("a long description of the item")], + vec![make_cell("2"), make_cell("another verbose body cell here")], + ]; + let metrics = W::collect_table_column_metrics(&header, &rows, /*column_count*/ 2); + assert_eq!(metrics[0].kind, TableColumnKind::Structured); + assert_eq!(metrics[1].kind, TableColumnKind::Narrative); + } + + #[test] + fn column_classification_structured_by_url_like_token() { + // Col with short word count but ≥28 avg char width and long tokens → Structured + // (URL-like/token-heavy columns resist collapse) + let header = vec![make_cell("URL")]; + let rows = vec![ + vec![make_cell("https://example.com/very/long/path")], + vec![make_cell("https://another.example.org/deep")], + ]; + let metrics = W::collect_table_column_metrics(&header, &rows, /*column_count*/ 1); + assert!(metrics[0].avg_cell_width >= 28.0); + assert_eq!(metrics[0].kind, TableColumnKind::Structured); + } + + #[test] + fn column_classification_structured_all_short() { + // Both columns short tokens → both Structured + let header = vec![make_cell("Status"), make_cell("Count")]; + let rows = vec![ + vec![make_cell("ok"), make_cell("42")], + vec![make_cell("err"), make_cell("7")], + ]; + let metrics = W::collect_table_column_metrics(&header, &rows, /*column_count*/ 2); + assert_eq!(metrics[0].kind, TableColumnKind::Structured); + assert_eq!(metrics[1].kind, TableColumnKind::Structured); + } + + #[test] + fn preferred_floor_narrative_caps_header_at_10() { + // Narrative col: header_token_width 15 → floors at 10 + let m = TableColumnMetrics { + max_width: 40, + header_token_width: 15, + body_token_width: 8, + avg_words_per_cell: 5.0, + avg_cell_width: 30.0, + kind: TableColumnKind::Narrative, + }; + assert_eq!(W::preferred_column_floor(&m, /*min_column_width*/ 3), 10); + + // Narrative col: header_token_width 6 → floors at 6 (below cap) + let m2 = TableColumnMetrics { + max_width: 40, + header_token_width: 6, + body_token_width: 8, + avg_words_per_cell: 5.0, + avg_cell_width: 30.0, + kind: TableColumnKind::Narrative, + }; + assert_eq!(W::preferred_column_floor(&m2, /*min_column_width*/ 3), 6); + } + + #[test] + fn preferred_floor_structured_uses_body_token() { + // Structured: max(header_token_width, body_token_width.min(16)) + let m = TableColumnMetrics { + max_width: 30, + header_token_width: 5, + body_token_width: 12, + avg_words_per_cell: 1.0, + avg_cell_width: 10.0, + kind: TableColumnKind::Structured, + }; + // max(5, min(12, 16)) = max(5, 12) = 12 + assert_eq!(W::preferred_column_floor(&m, /*min_column_width*/ 3), 12); + + // Body token exceeds 16 cap → capped at 16, then max with header + let m2 = TableColumnMetrics { + max_width: 30, + header_token_width: 5, + body_token_width: 20, + avg_words_per_cell: 1.0, + avg_cell_width: 10.0, + kind: TableColumnKind::Structured, + }; + // max(5, min(20, 16)) = max(5, 16) = 16 + assert_eq!(W::preferred_column_floor(&m2, /*min_column_width*/ 3), 16); + } + + #[test] + fn next_column_to_shrink_prefers_narrative() { + // Two columns: Narrative (col 0) and Structured (col 1), both with slack. + // Narrative should be shrunk first. + let widths = [20usize, 20]; + let floors = [8usize, 8]; + let metrics = [ + TableColumnMetrics { + max_width: 30, + header_token_width: 8, + body_token_width: 6, + avg_words_per_cell: 5.0, + avg_cell_width: 30.0, + kind: TableColumnKind::Narrative, + }, + TableColumnMetrics { + max_width: 30, + header_token_width: 8, + body_token_width: 6, + avg_words_per_cell: 1.0, + avg_cell_width: 10.0, + kind: TableColumnKind::Structured, + }, + ]; + let idx = W::next_column_to_shrink(&widths, &floors, &metrics); + assert_eq!(idx, Some(0), "Narrative column should be shrunk first"); + } + + // ===== Spillover-detection unit tests ===== + + #[test] + fn spillover_detects_single_cell_row() { + let row = make_body_row( + vec![make_cell("some trailing text")], + /*has_table_pipe_syntax*/ false, + ); + assert!(W::is_spillover_row(&row, /*next_row*/ None)); + } + + #[test] + fn spillover_keeps_single_cell_row_with_table_pipe_syntax() { + let row = make_body_row( + vec![make_cell("some sparse value")], + /*has_table_pipe_syntax*/ true, + ); + assert!(!W::is_spillover_row(&row, /*next_row*/ None)); + } + + #[test] + fn spillover_detects_html_content() { + // 3-cell row where only cell 0 has HTML content + let row = make_body_row( + vec![ + make_cell("
content
"), + make_cell(""), + make_cell(""), + ], + /*has_table_pipe_syntax*/ false, + ); + assert!(W::is_spillover_row(&row, /*next_row*/ None)); + } + + #[test] + fn spillover_detects_label_followed_by_html() { + // cell 0 = "HTML block:" and next_row cell 0 = "
x
" + let row = make_body_row( + vec![make_cell("HTML block:"), make_cell(""), make_cell("")], + /*has_table_pipe_syntax*/ false, + ); + let next = make_body_row( + vec![make_cell("
x
"), make_cell(""), make_cell("")], + /*has_table_pipe_syntax*/ false, + ); + assert!(W::is_spillover_row(&row, Some(&next))); + } + + #[test] + fn spillover_detects_trailing_html_label() { + // "HTML block:" with no next_row → trailing HTML label spillover + let row = make_body_row( + vec![make_cell("HTML block:"), make_cell(""), make_cell("")], + /*has_table_pipe_syntax*/ false, + ); + assert!(W::is_spillover_row(&row, /*next_row*/ None)); + } + + #[test] + fn spillover_keeps_normal_multi_cell_row() { + // 3 cells all non-empty → not spillover + let row = make_body_row( + vec![make_cell("one"), make_cell("two"), make_cell("three")], + /*has_table_pipe_syntax*/ true, + ); + assert!(!W::is_spillover_row(&row, /*next_row*/ None)); + } + + #[test] + fn spillover_keeps_label_when_next_is_not_html() { + // cell 0 = "Status:" and next_row cell 0 = "ok" → not spillover (not HTML) + let row = make_body_row( + vec![make_cell("Status:"), make_cell(""), make_cell("")], + /*has_table_pipe_syntax*/ true, + ); + let next = make_body_row( + vec![make_cell("ok"), make_cell(""), make_cell("")], + /*has_table_pipe_syntax*/ true, + ); + assert!(!W::is_spillover_row(&row, Some(&next))); + } } diff --git a/codex-rs/tui/src/markdown_render_tests.rs b/codex-rs/tui/src/markdown_render_tests.rs index f14d93304b09..4970008d25f4 100644 --- a/codex-rs/tui/src/markdown_render_tests.rs +++ b/codex-rs/tui/src/markdown_render_tests.rs @@ -1428,3 +1428,100 @@ fn code_block_preserves_trailing_blank_lines() { "trailing blank line inside code fence was lost: {content:?}" ); } + +#[test] +fn table_renders_unicode_box() { + let md = "| A | B |\n|---|---|\n| 1 | 2 |\n"; + let text = render_markdown_text(md); + let lines: Vec = text + .lines + .iter() + .map(|line| line.spans.iter().map(|span| span.content.clone()).collect()) + .collect(); + + assert_eq!( + lines, + vec![ + "┌─────┬─────┐".to_string(), + "│ A │ B │".to_string(), + "├─────┼─────┤".to_string(), + "│ 1 │ 2 │".to_string(), + "└─────┴─────┘".to_string(), + ] + ); +} + +#[test] +fn table_alignment_respects_markers() { + let md = "| Left | Center | Right |\n|:-----|:------:|------:|\n| a | b | c |\n"; + let text = render_markdown_text(md); + let lines: Vec = text + .lines + .iter() + .map(|line| line.spans.iter().map(|span| span.content.clone()).collect()) + .collect(); + + assert_eq!(lines[1], "│ Left │ Center │ Right │"); + assert_eq!(lines[3], "│ a │ b │ c │"); +} + +#[test] +fn table_wraps_cell_content_when_width_is_narrow() { + let md = "| Key | Description |\n| --- | --- |\n| -v | Enable very verbose logging output for debugging |\n"; + let text = crate::markdown_render::render_markdown_text_with_width(md, Some(30)); + let lines: Vec = text + .lines + .iter() + .map(|line| line.spans.iter().map(|span| span.content.clone()).collect()) + .collect(); + + assert!(lines[0].starts_with('┌') && lines[0].ends_with('┐')); + assert!( + lines + .iter() + .any(|line| line.contains("Enable very verbose")) + && lines.iter().any(|line| line.contains("logging output")), + "expected wrapped row content: {lines:?}" + ); +} + +#[test] +fn table_inside_blockquote_has_quote_prefix() { + let md = "> | A | B |\n> |---|---|\n> | 1 | 2 |\n"; + let text = render_markdown_text(md); + let lines: Vec = text + .lines + .iter() + .map(|line| line.spans.iter().map(|span| span.content.clone()).collect()) + .collect(); + + assert!(lines.iter().all(|line| line.starts_with("> "))); + assert!(lines.iter().any(|line| line.contains("┌─────┬─────┐"))); +} + +#[test] +fn escaped_pipes_render_in_table_cells() { + let md = "| Col |\n| --- |\n| a \\| b |\n"; + let text = render_markdown_text(md); + let lines: Vec = text + .lines + .iter() + .map(|line| line.spans.iter().map(|span| span.content.clone()).collect()) + .collect(); + + assert!(lines.iter().any(|line| line.contains("a | b"))); +} + +#[test] +fn table_falls_back_to_pipe_rendering_if_it_cannot_fit() { + let md = "| c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 | c9 | c10 |\n|---|---|---|---|---|---|---|---|---|---|\n| 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 |\n"; + let text = crate::markdown_render::render_markdown_text_with_width(md, Some(20)); + let lines: Vec = text + .lines + .iter() + .map(|line| line.spans.iter().map(|span| span.content.clone()).collect()) + .collect(); + + assert!(lines.first().is_some_and(|line| line.starts_with('|'))); + assert!(!lines.iter().any(|line| line.contains('┌'))); +} diff --git a/codex-rs/tui/src/markdown_stream.rs b/codex-rs/tui/src/markdown_stream.rs index 311ea202c489..66b5cab121f7 100644 --- a/codex-rs/tui/src/markdown_stream.rs +++ b/codex-rs/tui/src/markdown_stream.rs @@ -848,4 +848,41 @@ mod tests { async fn table_like_lines_inside_fenced_code_are_not_held() { assert_streamed_equals_full(&["```\n", "| a | b |\n", "```\n"]).await; } + + #[tokio::test] + async fn collector_source_chunks_round_trip_into_agent_fence_unwrapping() { + let deltas = [ + "```md\n", + "| A | B |\n", + "|---|---|\n", + "| 1 | 2 |\n", + "```\n", + ]; + let mut collector = + super::MarkdownStreamCollector::new(/*width*/ None, &super::test_cwd()); + let mut raw_source = String::new(); + + for delta in deltas { + collector.push_delta(delta); + if delta.contains('\n') + && let Some(chunk) = collector.commit_complete_source() + { + raw_source.push_str(&chunk); + } + } + raw_source.push_str(&collector.finalize_and_drain_source()); + + let mut rendered = Vec::new(); + crate::markdown::append_markdown_agent(&raw_source, /*width*/ None, &mut rendered); + let rendered_strs = lines_to_plain_strings(&rendered); + + assert!( + rendered_strs.iter().any(|line| line.contains('┌')), + "expected markdown-fenced table to render as boxed table: {rendered_strs:?}" + ); + assert!( + !rendered_strs.iter().any(|line| line.trim() == "| A | B |"), + "did not expect raw table header after markdown-fence unwrapping: {rendered_strs:?}" + ); + } } diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__raw_mode_toggle_transcript.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__raw_mode_toggle_transcript.snap index 5f18eb209d9f..1d89a4bd6a09 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__raw_mode_toggle_transcript.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__raw_mode_toggle_transcript.snap @@ -10,9 +10,11 @@ rich before: • - first item - second item - | Col | Value | - | --- | --- | - | code | x = 1 | + ┌──────┬───────┐ + │ Col │ Value │ + ├──────┼───────┤ + │ code │ x = 1 │ + └──────┴───────┘ copy me • Called @@ -46,9 +48,11 @@ rich after: • - first item - second item - | Col | Value | - | --- | --- | - | code | x = 1 | + ┌──────┬───────┐ + │ Col │ Value │ + ├──────┼───────┤ + │ code │ x = 1 │ + └──────┴───────┘ copy me • Called diff --git a/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_complex_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_complex_snapshot.snap index cc752dd66d41..34cbd1c63e84 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_complex_snapshot.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_complex_snapshot.snap @@ -28,9 +28,12 @@ Image: alt text ——— Table below (alignment test): -| Left | Center | Right | -|:-----|:------:|------:| -| a | b | c | + +┌──────┬────────┬───────┐ +│ Left │ Center │ Right │ +├──────┼────────┼───────┤ +│ a │ b │ c │ +└──────┴────────┴───────┘ Inline HTML: sup and sub. HTML block:
inline block
diff --git a/codex-rs/tui/src/streaming/controller.rs b/codex-rs/tui/src/streaming/controller.rs index 5d903c91ccca..37d064837080 100644 --- a/codex-rs/tui/src/streaming/controller.rs +++ b/codex-rs/tui/src/streaming/controller.rs @@ -1,20 +1,45 @@ -//! Streams markdown deltas while retaining source for later transcript reflow. +//! Two-region streaming controllers for agent messages and proposed plans. //! -//! Streaming has two outputs with different lifetimes. The live viewport needs incremental -//! `HistoryCell`s so the user sees progress, while finalized transcript history needs raw markdown -//! source so it can be rendered again after a terminal resize. These controllers keep those outputs -//! tied together: newline-complete source is rendered into queued live cells, and finalization -//! returns the accumulated source to the app for consolidation. +//! Each stream partitions rendered markdown into a *stable region* (committed +//! to scrollback via the animation queue in `StreamState`) and a *tail region* +//! (mutable, displayed in the active-cell slot as a transient stream-tail cell). //! -//! Width changes are handled by re-rendering from source and rebuilding only the not-yet-emitted -//! queue. Already emitted rows stay emitted until the app-level transcript reflow rebuilds the full -//! scrollback from finalized cells. +//! `StreamCore` owns the shared bookkeeping: source accumulation, re-rendering, +//! stable/tail partitioning, commit-animation queue management, and terminal +//! resize handling. `StreamController` and `PlanStreamController` are thin +//! wrappers that add only their `emit()` styling and finalize return types. +//! +//! ## Table holdback +//! +//! Table rendering is inherently non-incremental: adding a new row can change +//! every column's width and reshape all prior rows. The holdback mechanism +//! (`table_holdback_state`) detects pipe-table patterns (header + delimiter +//! pair) in the accumulated source and keeps content from the table header +//! onward as mutable tail until the stream finalizes. Holdback is enabled for +//! agent and proposed-plan streams. Lines in `Outside` and `Markdown` fence +//! contexts are scanned; lines inside non-markdown fences are skipped. +//! +//! ## Resize handling +//! +//! On terminal width change, `StreamCore::set_width` re-renders at the new +//! width and rebuilds the queued stable region from the current emitted line +//! count. This intentionally avoids byte-level remap complexity while the +//! stream is active; finalized content is canonicalized by transcript +//! consolidation into source-backed markdown cells. +//! +//! ## Invariants +//! +//! - `emitted_stable_len <= enqueued_stable_len <= rendered_lines.len()`. +//! - `raw_source` is append-only until `reset()`; never modified mid-stream. +//! - Tail starts exactly at `enqueued_stable_len`. +//! - During confirmed table streaming, only lines from the table header onward +//! are forced into tail; pre-table lines may remain stable. use crate::history_cell::HistoryCell; use crate::history_cell::HistoryRenderMode; use crate::history_cell::raw_lines_from_source; use crate::history_cell::{self}; -use crate::markdown::append_markdown; +use crate::markdown::append_markdown_agent_with_cwd; use crate::render::line_utils::prefix_lines; use crate::style::proposed_plan_style; use ratatui::prelude::Stylize; @@ -25,23 +50,49 @@ use std::time::Duration; use std::time::Instant; use super::StreamState; +use super::table_holdback::TableHoldbackScanner; +use super::table_holdback::TableHoldbackState; +#[cfg(test)] +use super::table_holdback::table_holdback_state; -/// Shared source-retaining stream state for assistant and plan output. +// --------------------------------------------------------------------------- +// StreamCore — shared bookkeeping for both stream controllers +// --------------------------------------------------------------------------- + +/// Shared state and logic for the two-region streaming model. +/// +/// Both [`StreamController`] (agent messages) and [`PlanStreamController`] +/// (proposed plans) delegate their core bookkeeping here: source +/// accumulation, re-rendering, stable/tail partitioning, commit-animation +/// queue management, and terminal resize handling. /// -/// `raw_source` is the markdown source that has crossed a newline boundary and can be rendered -/// deterministically. `rendered_lines` is the current-width render of that source. `enqueued_len` -/// tracks how much of that render has been offered to the commit queue, while `emitted_len` tracks -/// how much has actually reached history cells. Keeping those counters separate lets width changes -/// rebuild pending output without duplicating lines that are already visible. +/// The wrapping controllers add only their own `emit()` styling and +/// finalize return types. struct StreamCore { state: StreamState, + /// Current rendering width (columns available for markdown content). width: Option, + /// Accumulated raw markdown source for the current stream. raw_source: String, + /// Full re-render of `raw_source` at `width`. Rebuilt on every committed delta. rendered_lines: Vec>, - enqueued_len: usize, - emitted_len: usize, + /// Lines enqueued into the commit-animation queue. + enqueued_stable_len: usize, + /// Lines actually emitted to scrollback. + emitted_stable_len: usize, + /// Session cwd used to keep local file-link display stable during stream re-renders. cwd: PathBuf, render_mode: HistoryRenderMode, + /// Cached rendered line count for prefix-before-table keyed by source start and width. + stable_prefix_len_cache: Option, + /// Incremental holdback scanner state for append-only source updates. + holdback_scanner: TableHoldbackScanner, +} + +struct StablePrefixLenCache { + source_start: usize, + width: Option, + stable_prefix_len: usize, } impl StreamCore { @@ -51,101 +102,171 @@ impl StreamCore { width, raw_source: String::with_capacity(1024), rendered_lines: Vec::with_capacity(64), - enqueued_len: 0, - emitted_len: 0, + enqueued_stable_len: 0, + emitted_stable_len: 0, cwd: cwd.to_path_buf(), render_mode, + stable_prefix_len_cache: None, + holdback_scanner: TableHoldbackScanner::new(), } } + /// Push a delta; if it contains a newline, commit completed lines and enqueue newly-stable + /// lines. Returns `true` if new lines were enqueued. fn push_delta(&mut self, delta: &str) -> bool { if !delta.is_empty() { self.state.has_seen_delta = true; } self.state.collector.push_delta(delta); + let mut enqueued = false; if delta.contains('\n') && let Some(committed_source) = self.state.collector.commit_complete_source() { self.raw_source.push_str(&committed_source); - self.recompute_render(); - return self.sync_queue_to_render(); + self.holdback_scanner.push_source_chunk(&committed_source); + self.recompute_streaming_render(); + enqueued = self.sync_stable_queue(); } - - false + enqueued } + /// Drain the collector, re-render, and return lines not yet emitted. + /// Does NOT reset state - the caller must call `reset()` afterward. fn finalize_remaining(&mut self) -> Vec> { let remainder_source = self.state.collector.finalize_and_drain_source(); if !remainder_source.is_empty() { self.raw_source.push_str(&remainder_source); + self.holdback_scanner.push_source_chunk(&remainder_source); } - let rendered = self.render_source(&self.raw_source); - if self.emitted_len >= rendered.len() { + if self.emitted_stable_len >= rendered.len() { Vec::new() } else { - rendered[self.emitted_len..].to_vec() + rendered[self.emitted_stable_len..].to_vec() } } + /// Step animation: dequeue one line, update the emitted count. fn tick(&mut self) -> Vec> { let step = self.state.step(); - self.emitted_len += step.len(); + self.emitted_stable_len += step.len(); step } + /// Batch drain: dequeue up to `max_lines`, update the emitted count. fn tick_batch(&mut self, max_lines: usize) -> Vec> { if max_lines == 0 { return Vec::new(); } let step = self.state.drain_n(max_lines); - self.emitted_len += step.len(); + if step.is_empty() { + return step; + } + self.emitted_stable_len += step.len(); step } + // Trivial StreamCore accessors inlined — called on every animation tick + // and render frame during active streaming. + + #[inline] + fn is_idle(&self) -> bool { + self.state.is_idle() + } + + #[inline] fn queued_lines(&self) -> usize { self.state.queued_len() } + #[inline] fn oldest_queued_age(&self, now: Instant) -> Option { self.state.oldest_queued_age(now) } - fn is_idle(&self) -> bool { - self.state.is_idle() + /// Lines that belong to the mutable tail, not yet queued for stable commit. + /// + /// The tail starts at `enqueued_stable_len` -- everything from that offset to + /// the end of `rendered_lines` is displayed live in the active-cell slot. + #[inline] + fn current_tail_lines(&self) -> Vec> { + let start = self.enqueued_stable_len.min(self.rendered_lines.len()); + self.rendered_lines[start..].to_vec() + } + + #[inline] + fn has_tail(&self) -> bool { + self.enqueued_stable_len < self.rendered_lines.len() } + /// Update rendering width and rebuild queued stable lines for the new layout. + /// + /// Re-renders once at the new width and rebuilds queue state from the + /// current emitted line count. fn set_width(&mut self, width: Option) { if self.width == width { return; } - let had_pending_queue = self.state.queued_len() > 0; + let had_live_tail = self.has_tail(); self.width = width; self.state.collector.set_width(width); if self.raw_source.is_empty() { return; } - self.recompute_render(); - self.emitted_len = self.emitted_len.min(self.rendered_lines.len()); + self.recompute_streaming_render(); + self.emitted_stable_len = self.emitted_stable_len.min(self.rendered_lines.len()); if had_pending_queue - && self.emitted_len == self.rendered_lines.len() - && self.emitted_len > 0 + && self.emitted_stable_len == self.rendered_lines.len() + && self.emitted_stable_len > 0 { // If wrapped remainder compresses into fewer lines at the new width, // keep at least one line un-emitted so pre-resize pending content is // not skipped permanently. - self.emitted_len -= 1; + self.emitted_stable_len -= 1; } - self.state.clear_queue(); - if self.emitted_len > 0 && !had_pending_queue { - self.enqueued_len = self.rendered_lines.len(); + if self.emitted_stable_len > 0 && !had_pending_queue && !had_live_tail { + // Avoid replaying already-emitted content after resize when no + // stable lines were waiting in the queue and there was no mutable + // tail to preserve. + self.enqueued_stable_len = self.rendered_lines.len(); return; } - self.rebuild_queue_from_render(); + self.rebuild_stable_queue_from_render(); + } + + /// Clear all accumulated state for current stream. + fn reset(&mut self) { + self.state.clear(); + self.raw_source.clear(); + self.rendered_lines.clear(); + self.enqueued_stable_len = 0; + self.emitted_stable_len = 0; + self.stable_prefix_len_cache = None; + self.holdback_scanner.reset(); + } + + fn render_source(&self, source: &str) -> Vec> { + match self.render_mode { + HistoryRenderMode::Rich => { + let mut rendered = Vec::new(); + append_markdown_agent_with_cwd( + source, + self.width, + Some(self.cwd.as_path()), + &mut rendered, + ); + rendered + } + HistoryRenderMode::Raw => raw_lines_from_source(source), + } + } + + fn recompute_streaming_render(&mut self) { + self.rendered_lines = self.render_source(&self.raw_source); } fn set_render_mode(&mut self, render_mode: HistoryRenderMode) { @@ -154,99 +275,165 @@ impl StreamCore { } let had_pending_queue = self.state.queued_len() > 0; + let had_live_tail = self.has_tail(); self.render_mode = render_mode; if self.raw_source.is_empty() { return; } - self.recompute_render(); - self.emitted_len = self.emitted_len.min(self.rendered_lines.len()); + self.recompute_streaming_render(); + self.emitted_stable_len = self.emitted_stable_len.min(self.rendered_lines.len()); + if had_pending_queue + && self.emitted_stable_len == self.rendered_lines.len() + && self.emitted_stable_len > 0 + { + self.emitted_stable_len -= 1; + } self.state.clear_queue(); - if self.emitted_len > 0 && !had_pending_queue { - self.enqueued_len = self.rendered_lines.len(); + if self.emitted_stable_len > 0 && !had_pending_queue && !had_live_tail { + self.enqueued_stable_len = self.rendered_lines.len(); return; } - self.rebuild_queue_from_render(); - } - - fn clear_queue(&mut self) { - self.state.clear_queue(); - self.enqueued_len = self.emitted_len; - } - - fn reset(&mut self) { - self.state.clear(); - self.raw_source.clear(); - self.rendered_lines.clear(); - self.enqueued_len = 0; - self.emitted_len = 0; + self.rebuild_stable_queue_from_render(); } - fn recompute_render(&mut self) { - self.rendered_lines = self.render_source(&self.raw_source); + /// Compute how many rendered lines should be in the stable region. + fn compute_target_stable_len(&mut self) -> usize { + let tail_budget = self.active_tail_budget_lines(); + self.rendered_lines + .len() + .saturating_sub(tail_budget) + .max(self.emitted_stable_len) } - fn render_source(&self, source: &str) -> Vec> { - match self.render_mode { - HistoryRenderMode::Rich => { - let mut rendered = Vec::new(); - append_markdown(source, self.width, Some(self.cwd.as_path()), &mut rendered); - rendered + /// Advance `enqueued_stable_len` toward the target stable boundary and enqueue any + /// newly-stable lines. Returns `true` if new lines were enqueued. + fn sync_stable_queue(&mut self) -> bool { + let target_stable_len = self.compute_target_stable_len(); + + // A structural rewrite moved the stable boundary backward into enqueue-but-unemitted + // lines. Rebuild queue from the latest snapshot. + if target_stable_len < self.enqueued_stable_len { + self.state.clear_queue(); + if self.emitted_stable_len < target_stable_len { + self.state.enqueue( + self.rendered_lines[self.emitted_stable_len..target_stable_len].to_vec(), + ); } - HistoryRenderMode::Raw => raw_lines_from_source(source), - } - } - - /// Append newly rendered lines to the live queue without replaying already queued rows. - /// - /// Width changes can make the rendered line count smaller than the previous queue boundary; in - /// that case the only safe option is rebuilding the queue from `emitted_len`, because slicing - /// from the stale `enqueued_len` would skip pending source. - fn sync_queue_to_render(&mut self) -> bool { - let target_len = self.rendered_lines.len().max(self.emitted_len); - if target_len < self.enqueued_len { - self.rebuild_queue_from_render(); + self.enqueued_stable_len = target_stable_len; return self.state.queued_len() > 0; } - if target_len == self.enqueued_len { + if target_stable_len == self.enqueued_stable_len { return false; } self.state - .enqueue(self.rendered_lines[self.enqueued_len..target_len].to_vec()); - self.enqueued_len = target_len; + .enqueue(self.rendered_lines[self.enqueued_stable_len..target_stable_len].to_vec()); + self.enqueued_stable_len = target_stable_len; true } - /// Rebuild the pending live queue from the current render and current emitted position. - /// - /// This is used when resize invalidates queued wrapping. It must never enqueue rows before - /// `emitted_len`, because those rows have already been inserted into terminal history. - fn rebuild_queue_from_render(&mut self) { + /// Rebuild the stable queue from the current render snapshot. Used after `set_width()` where + /// the old queue is stale. + fn rebuild_stable_queue_from_render(&mut self) { + let target_stable_len = self.compute_target_stable_len(); self.state.clear_queue(); - let target_len = self.rendered_lines.len().max(self.emitted_len); - if self.emitted_len < target_len { + if self.emitted_stable_len < target_stable_len { self.state - .enqueue(self.rendered_lines[self.emitted_len..target_len].to_vec()); + .enqueue(self.rendered_lines[self.emitted_stable_len..target_stable_len].to_vec()); + } + self.enqueued_stable_len = target_stable_len; + } + + /// How many rendered lines to withhold as mutable tail. + /// + /// When a table is detected (`Confirmed` or `PendingHeader`), the entire + /// table region is held as tail because adding a row can reshape table + /// column widths. For `PendingHeader`, only content from the speculative + /// header line onward is kept mutable so earlier prose can continue + /// streaming. When no table is detected, everything flows directly to + /// stable. This is the core decision point for the holdback mechanism. + fn active_tail_budget_lines(&mut self) -> usize { + if self.render_mode == HistoryRenderMode::Raw { + return 0; + } + let scan_start = Instant::now(); + let holdback_state = self.holdback_scanner.state(); + let tail_budget = match holdback_state { + TableHoldbackState::Confirmed { table_start: start } + | TableHoldbackState::PendingHeader { + header_start: start, + } => self.tail_budget_from_source_start(start), + TableHoldbackState::None => 0, + }; + tracing::trace!( + state = ?holdback_state, + tail_budget, + elapsed_us = scan_start.elapsed().as_micros(), + "table holdback decision", + ); + tail_budget + } + + fn tail_budget_from_source_start(&mut self, source_start: usize) -> usize { + if source_start == 0 { + return self.rendered_lines.len(); } - self.enqueued_len = target_len; + let source_start = source_start.min(self.raw_source.len()); + let stable_prefix_len = self.stable_prefix_len_for_source_start(source_start); + self.rendered_lines.len().saturating_sub(stable_prefix_len) + } + + fn stable_prefix_len_for_source_start(&mut self, source_start: usize) -> usize { + if let Some(cache) = &self.stable_prefix_len_cache + && cache.source_start == source_start + && cache.width == self.width + { + tracing::trace!( + source_start, + width = ?self.width, + stable_prefix_len = cache.stable_prefix_len, + "table holdback stable-prefix cache hit", + ); + return cache.stable_prefix_len; + } + + let render_start = Instant::now(); + let mut stable_prefix_render = Vec::new(); + append_markdown_agent_with_cwd( + &self.raw_source[..source_start.min(self.raw_source.len())], + self.width, + Some(self.cwd.as_path()), + &mut stable_prefix_render, + ); + let stable_prefix_len = stable_prefix_render.len(); + tracing::trace!( + source_start, + width = ?self.width, + stable_prefix_len, + elapsed_us = render_start.elapsed().as_micros(), + "table holdback stable-prefix render", + ); + self.stable_prefix_len_cache = Some(StablePrefixLenCache { + source_start, + width: self.width, + stable_prefix_len, + }); + stable_prefix_len } } -/// Controls newline-gated streaming for assistant messages. +/// Controller for streaming agent message content with table-aware holdback. /// -/// The controller emits transient `AgentMessageCell`s for live display and returns raw markdown -/// source on `finalize` so the app can replace those transient cells with a source-backed -/// `AgentMarkdownCell`. Callers should use `set_width` on terminal resize; rebuilding the queue -/// from already emitted cells would duplicate output instead of preserving the stream position. +/// Wraps [`StreamCore`] and adds `AgentMessageCell` emission styling. pub(crate) struct StreamController { core: StreamCore, header_emitted: bool, } impl StreamController { - /// Create a stream controller that renders markdown relative to the given width and cwd. + /// Create a controller whose markdown renderer shortens local file links relative to `cwd`. /// /// `width` is the content width available to markdown rendering, not necessarily the full /// terminal width. Passing a stale width after resize will keep queued live output wrapped for @@ -258,19 +445,12 @@ impl StreamController { } } - /// Push a raw model delta and return whether it produced queued complete lines. - /// - /// Deltas are committed only through newline boundaries. A `false` return can still mean source - /// was buffered; it only means no newly renderable complete line is ready for live emission. pub(crate) fn push(&mut self, delta: &str) -> bool { self.core.push_delta(delta) } - /// Finish the stream and return the final transient cell plus accumulated markdown source. - /// - /// The source is `None` only when the stream never accumulated content. Callers that discard the - /// returned source cannot later consolidate the transcript into a width-sensitive finalized - /// cell. + /// Finalize the active stream. Returns the final cell (if any remaining lines) and the raw + /// markdown source for consolidation. pub(crate) fn finalize(&mut self) -> (Option>, Option) { let remaining = self.core.finalize_remaining(); if self.core.raw_source.is_empty() { @@ -278,6 +458,7 @@ impl StreamController { return (None, None); } + // Move ownership — source is consumed before reset() clears it. let source = std::mem::take(&mut self.core.raw_source); let out = self.emit(remaining); self.core.reset(); @@ -297,6 +478,10 @@ impl StreamController { (self.emit(step), self.core.is_idle()) } + // Thin StreamController accessors inlined — one-liner delegates called + // on every render frame and animation tick. + + #[inline] pub(crate) fn queued_lines(&self) -> usize { self.core.queued_lines() } @@ -305,8 +490,24 @@ impl StreamController { self.core.oldest_queued_age(now) } + #[inline] + pub(crate) fn current_tail_lines(&self) -> Vec> { + self.core.current_tail_lines() + } + + #[inline] + pub(crate) fn tail_starts_stream(&self) -> bool { + !self.header_emitted && self.core.enqueued_stable_len == 0 + } + + #[inline] + pub(crate) fn has_live_tail(&self) -> bool { + self.core.has_tail() + } + pub(crate) fn clear_queue(&mut self) { - self.core.clear_queue(); + self.core.state.clear_queue(); + self.core.enqueued_stable_len = self.core.emitted_stable_len; } pub(crate) fn set_width(&mut self, width: Option) { @@ -328,12 +529,14 @@ impl StreamController { }))) } } +// --------------------------------------------------------------------------- +// PlanStreamController — proposed plan streams +// --------------------------------------------------------------------------- -/// Controls newline-gated streaming for proposed plan markdown. +/// Controller that streams proposed plan markdown into a styled plan block. /// -/// This follows the same source-retention contract as `StreamController`, but wraps emitted lines -/// in the proposed-plan header, padding, and style. Finalization must return source for -/// `ProposedPlanCell`; otherwise a resized finalized plan would keep the transient stream shape. +/// Wraps [`StreamCore`] and adds plan-specific header, indentation, and +/// background styling. pub(crate) struct PlanStreamController { core: StreamCore, header_emitted: bool, @@ -341,7 +544,8 @@ pub(crate) struct PlanStreamController { } impl PlanStreamController { - /// Create a proposed-plan stream controller that renders markdown relative to the given cwd. + /// Create a plan-stream controller whose markdown renderer shortens local file links relative + /// to `cwd`. /// /// The width has the same meaning as in `StreamController`: it is the markdown body width, and /// callers must update it when the terminal width changes. @@ -353,18 +557,12 @@ impl PlanStreamController { } } - /// Push a raw proposed-plan delta and return whether it produced queued complete lines. - /// - /// Source may be buffered even when this returns `false`; callers should continue ticking only - /// when queued lines exist. pub(crate) fn push(&mut self, delta: &str) -> bool { self.core.push_delta(delta) } - /// Finish the plan stream and return the final transient cell plus accumulated markdown source. - /// - /// The returned source is consumed by app-level consolidation to create the source-backed - /// `ProposedPlanCell` used for later resize reflow. + /// Finalize the active stream. Returns the final cell (if any remaining + /// lines) plus raw markdown source for consolidation. pub(crate) fn finalize(&mut self) -> (Option>, Option) { let remaining = self.core.finalize_remaining(); if self.core.raw_source.is_empty() { @@ -372,6 +570,7 @@ impl PlanStreamController { return (None, None); } + // Move ownership — source is consumed before reset() clears it. let source = std::mem::take(&mut self.core.raw_source); let out = self.emit(remaining, /*include_bottom_padding*/ true); self.core.reset(); @@ -397,16 +596,41 @@ impl PlanStreamController { ) } + #[inline] pub(crate) fn queued_lines(&self) -> usize { self.core.queued_lines() } + #[inline] + pub(crate) fn has_live_tail(&self) -> bool { + self.core.has_tail() + } + + #[inline] + pub(crate) fn current_tail_lines(&self) -> Vec> { + self.core.current_tail_lines() + } + + #[inline] + pub(crate) fn tail_starts_stream(&self) -> bool { + !self.header_emitted && self.core.enqueued_stable_len == 0 + } + + pub(crate) fn current_tail_display_lines(&self) -> Vec> { + let lines = self.current_tail_lines(); + if lines.is_empty() { + return Vec::new(); + } + self.render_display_lines(lines, /*include_bottom_padding*/ false) + } + pub(crate) fn oldest_queued_age(&self, now: Instant) -> Option { self.core.oldest_queued_age(now) } pub(crate) fn clear_queue(&mut self) { - self.core.clear_queue(); + self.core.state.clear_queue(); + self.core.enqueued_stable_len = self.core.emitted_stable_len; } pub(crate) fn set_width(&mut self, width: Option) { @@ -426,18 +650,31 @@ impl PlanStreamController { return None; } - let mut out_lines: Vec> = Vec::with_capacity(4); let is_stream_continuation = self.header_emitted; + let out_lines = self.render_display_lines(lines, include_bottom_padding); + self.header_emitted = true; + self.top_padding_emitted = true; + + Some(Box::new(history_cell::new_proposed_plan_stream( + out_lines, + is_stream_continuation, + ))) + } + + fn render_display_lines( + &self, + lines: Vec>, + include_bottom_padding: bool, + ) -> Vec> { + let mut out_lines: Vec> = Vec::with_capacity(4); if !self.header_emitted { out_lines.push(vec!["• ".dim(), "Proposed Plan".bold()].into()); out_lines.push(Line::from(" ")); - self.header_emitted = true; } let mut plan_lines: Vec> = Vec::with_capacity(4); if !self.top_padding_emitted { plan_lines.push(Line::from(" ")); - self.top_padding_emitted = true; } plan_lines.extend(lines); if include_bottom_padding { @@ -450,11 +687,7 @@ impl PlanStreamController { .map(|line| line.style(plan_style)) .collect::>(); out_lines.extend(plan_lines); - - Some(Box::new(history_cell::new_proposed_plan_stream( - out_lines, - is_stream_continuation, - ))) + out_lines } } @@ -462,8 +695,11 @@ impl PlanStreamController { mod tests { use super::*; use pretty_assertions::assert_eq; + use std::path::PathBuf; fn test_cwd() -> PathBuf { + // These tests only need a stable absolute cwd; using temp_dir() avoids baking Unix- or + // Windows-specific root semantics into the fixtures. std::env::temp_dir() } @@ -475,14 +711,15 @@ mod tests { PlanStreamController::new(width, &test_cwd(), HistoryRenderMode::Rich) } - fn lines_to_plain_strings(lines: &[Line<'_>]) -> Vec { + fn lines_to_plain_strings(lines: &[ratatui::text::Line<'_>]) -> Vec { lines .iter() - .map(|line| { - line.spans + .map(|l| { + l.spans .iter() - .map(|span| span.content.clone()) - .collect::() + .map(|s| s.content.clone()) + .collect::>() + .join("") }) .collect() } @@ -490,8 +727,8 @@ mod tests { fn collect_streamed_lines(deltas: &[&str], width: Option) -> Vec { let mut ctrl = stream_controller(width); let mut lines = Vec::new(); - for delta in deltas { - ctrl.push(delta); + for d in deltas { + ctrl.push(d); while let (Some(cell), idle) = ctrl.on_commit_tick() { lines.extend(cell.transcript_lines(u16::MAX)); if idle { @@ -504,15 +741,15 @@ mod tests { } lines_to_plain_strings(&lines) .into_iter() - .map(|line| line.chars().skip(2).collect::()) + .map(|s| s.chars().skip(2).collect::()) .collect() } fn collect_plan_streamed_lines(deltas: &[&str], width: Option) -> Vec { let mut ctrl = plan_stream_controller(width); let mut lines = Vec::new(); - for delta in deltas { - ctrl.push(delta); + for d in deltas { + ctrl.push(d); while let (Some(cell), idle) = ctrl.on_commit_tick() { lines.extend(cell.transcript_lines(u16::MAX)); if idle { @@ -584,37 +821,1006 @@ mod tests { } #[test] - fn controller_finalize_returns_raw_source_for_consolidation() { + fn controller_has_live_tail_reflects_tail_presence() { + let mut ctrl = stream_controller(Some(80)); + assert!(!ctrl.has_live_tail()); + + ctrl.core.rendered_lines = vec![Line::from("tail line")]; + ctrl.core.enqueued_stable_len = 0; + assert!(ctrl.has_live_tail()); + + ctrl.core.enqueued_stable_len = 1; + assert!(!ctrl.has_live_tail()); + } + + #[test] + fn plan_controller_has_live_tail_reflects_tail_presence() { + let mut ctrl = plan_stream_controller(Some(80)); + assert!(!ctrl.has_live_tail()); + + ctrl.core.rendered_lines = vec![Line::from("tail line")]; + ctrl.core.enqueued_stable_len = 0; + assert!(ctrl.has_live_tail()); + + ctrl.core.enqueued_stable_len = 1; + assert!(!ctrl.has_live_tail()); + } + + #[test] + fn controller_live_tail_keeps_uncommitted_table_cell_newline_gated() { + let mut ctrl = stream_controller(Some(80)); + ctrl.push("| A | B |\n"); + ctrl.push("| --- | --- |\n"); + ctrl.push("| partial"); + + let tail = lines_to_plain_strings(&ctrl.current_tail_lines()).join("\n"); + assert!( + !tail.contains("partial"), + "expected live tail to remain newline-gated: {tail:?}", + ); + } + + #[test] + fn controller_live_tail_requires_table_holdback_state() { + let mut ctrl = stream_controller(Some(80)); + ctrl.push("plain text without newline"); + + assert!( + ctrl.current_tail_lines().is_empty(), + "expected no live tail outside table holdback state", + ); + assert!(!ctrl.has_live_tail()); + } + + #[test] + fn controller_live_tail_rerenders_table_tail_after_resize() { + let mut ctrl = stream_controller(Some(96)); + ctrl.push("| # | Feature | Details | Link |\n"); + ctrl.push("| --- | --- | --- | --- |\n"); + ctrl.push( + "| 1 | RESIZE_REPRO_SENTINEL | long wrapped content that should be reflowed | https://example.com/resize |\n", + ); + + for width in [48, 104, 56] { + ctrl.set_width(Some(width)); + let tail = lines_to_plain_strings(&ctrl.current_tail_lines()); + + let mut expected = Vec::new(); + crate::markdown::append_markdown_agent( + &ctrl.core.raw_source, + Some(width), + &mut expected, + ); + let expected = lines_to_plain_strings(&expected); + + assert_eq!( + tail, expected, + "expected live table tail to be rerendered at width {width}", + ); + } + } + + #[test] + fn controller_set_width_partial_drain_no_lost_lines() { + let mut ctrl = stream_controller(Some(40)); + ctrl.push("AAAA BBBB CCCC DDDD EEEE FFFF GGGG HHHH IIII JJJJ\n"); + ctrl.push("second line\n"); + + let (cell, idle) = ctrl.on_commit_tick(); + assert!(cell.is_some(), "expected 1 emitted line"); + assert!(!idle, "queue should still have lines"); + let remaining_before = ctrl.queued_lines(); + assert!(remaining_before > 0, "should have queued lines left"); + + ctrl.set_width(Some(20)); + + let (cell, source) = ctrl.finalize(); + let final_lines = cell + .map(|c| lines_to_plain_strings(&c.transcript_lines(u16::MAX))) + .unwrap_or_default(); + + assert!( + final_lines.iter().any(|l| l.contains("second line")), + "un-emitted 'second line' was lost after resize; got: {final_lines:?}", + ); + assert!(source.is_some(), "expected source from finalize"); + } + + #[test] + fn controller_set_width_partial_drain_keeps_pending_queue() { + let mut ctrl = stream_controller(Some(40)); + ctrl.push("AAAA BBBB CCCC DDDD EEEE FFFF GGGG HHHH IIII JJJJ\n"); + ctrl.push("second line\n"); + + let (cell, idle) = ctrl.on_commit_tick(); + assert!(cell.is_some(), "expected 1 emitted line"); + assert!(!idle, "queue should still have lines"); + assert!(ctrl.queued_lines() > 0, "expected pending queued lines"); + + ctrl.set_width(Some(20)); + + assert!( + ctrl.queued_lines() > 0, + "resize must preserve pending queued lines" + ); + + let mut drained = Vec::new(); + for _ in 0..64 { + let (cell, is_idle) = ctrl.on_commit_tick(); + if let Some(cell) = cell { + drained.extend(lines_to_plain_strings(&cell.transcript_lines(u16::MAX))); + } + if is_idle { + break; + } + } + + assert!( + drained.iter().any(|l| l.contains("second line")), + "pending lines should continue draining after resize; got {drained:?}", + ); + } + + #[test] + fn controller_set_width_preserves_in_flight_tail() { + let mut ctrl = stream_controller(Some(80)); + ctrl.push("tail without newline"); + ctrl.set_width(Some(24)); + + let (cell, _source) = ctrl.finalize(); + let rendered = lines_to_plain_strings( + &cell + .expect("expected finalized tail") + .transcript_lines(u16::MAX), + ); + + assert_eq!(rendered, vec!["• tail without newline".to_string()]); + } + + #[test] + fn controller_set_width_preserves_table_tail_when_queue_is_empty() { let mut ctrl = stream_controller(Some(80)); - assert!(ctrl.push("hello\n")); - let (_cell, source) = ctrl.finalize(); - assert_eq!(source, Some("hello\n".to_string())); + ctrl.push("intro line\n"); + + let (_cell, idle) = ctrl.on_commit_tick(); + assert!(idle, "intro line should fully drain"); + assert_eq!(ctrl.queued_lines(), 0, "expected empty queue before table"); + + ctrl.push("| A | B |\n"); + assert_eq!( + ctrl.queued_lines(), + 0, + "pending table header should remain mutable tail, not queued", + ); + assert!(ctrl.has_live_tail(), "expected live tail before resize"); + + ctrl.set_width(Some(24)); + + let tail_after = lines_to_plain_strings(&ctrl.current_tail_lines()); + assert!( + !tail_after.is_empty(), + "resize must keep mutable tail when queue is empty", + ); + let joined = tail_after.join(" "); + assert!( + joined.contains('A') && joined.contains('B'), + "expected table header content to remain in tail after resize: {tail_after:?}", + ); } #[test] - fn plan_controller_finalize_returns_raw_source_for_consolidation() { + fn plan_controller_set_width_preserves_in_flight_tail() { let mut ctrl = plan_stream_controller(Some(80)); - assert!(ctrl.push("- step\n")); - let (_cell, source) = ctrl.finalize(); - assert_eq!(source, Some("- step\n".to_string())); + ctrl.push("1. Item without newline"); + ctrl.set_width(Some(24)); + + let rendered = lines_to_plain_strings( + &(ctrl + .finalize() + .0 + .expect("expected finalized tail") + .transcript_lines(u16::MAX)), + ); + + assert!( + rendered + .iter() + .any(|line| line.contains("Item without newline")), + "expected finalized plan content after resize, got {rendered:?}", + ); + } + + #[test] + fn plan_controller_holds_table_header_as_live_tail() { + let mut ctrl = plan_stream_controller(Some(80)); + assert!(ctrl.push("Intro\n")); + let (_cell, idle) = ctrl.on_commit_tick_batch(usize::MAX); + assert!(idle, "intro line should fully drain"); + + assert!(!ctrl.push("| Step | Owner |\n")); + assert!( + ctrl.has_live_tail(), + "expected plan table header to be held" + ); } #[test] - fn simple_lines_stream_in_order() { - let actual = collect_streamed_lines(&["hello\n", "world\n"], Some(80)); - assert_eq!(actual, vec!["hello".to_string(), "world".to_string()]); + fn controller_loose_vs_tight_with_commit_ticks_matches_full() { + let mut ctrl = stream_controller(/*width*/ None); + let mut lines = Vec::new(); + + let deltas = vec![ + "\n\n", + "Loose", + " vs", + ".", + " tight", + " list", + " items", + ":\n", + "1", + ".", + " Tight", + " item", + "\n", + "2", + ".", + " Another", + " tight", + " item", + "\n\n", + "1", + ".", + " Loose", + " item", + " with", + " its", + " own", + " paragraph", + ".\n\n", + " ", + " This", + " paragraph", + " belongs", + " to", + " the", + " same", + " list", + " item", + ".\n\n", + "2", + ".", + " Second", + " loose", + " item", + " with", + " a", + " nested", + " list", + " after", + " a", + " blank", + " line", + ".\n\n", + " ", + " -", + " Nested", + " bullet", + " under", + " a", + " loose", + " item", + "\n", + " ", + " -", + " Another", + " nested", + " bullet", + "\n\n", + ]; + + for d in deltas.iter() { + ctrl.push(d); + while let (Some(cell), idle) = ctrl.on_commit_tick() { + lines.extend(cell.transcript_lines(u16::MAX)); + if idle { + break; + } + } + } + if let (Some(cell), _source) = ctrl.finalize() { + lines.extend(cell.transcript_lines(u16::MAX)); + } + + let streamed: Vec<_> = lines_to_plain_strings(&lines) + .into_iter() + .map(|s| s.chars().skip(2).collect::()) + .collect(); + + let source: String = deltas.iter().copied().collect(); + let mut rendered: Vec> = Vec::new(); + crate::markdown::append_markdown_agent(&source, /*width*/ None, &mut rendered); + let rendered_strs = lines_to_plain_strings(&rendered); + + assert_eq!(streamed, rendered_strs); + + let expected = vec![ + "Loose vs. tight list items:".to_string(), + "".to_string(), + "1. Tight item".to_string(), + "2. Another tight item".to_string(), + "3. Loose item with its own paragraph.".to_string(), + "".to_string(), + " This paragraph belongs to the same list item.".to_string(), + "4. Second loose item with a nested list after a blank line.".to_string(), + " - Nested bullet under a loose item".to_string(), + " - Another nested bullet".to_string(), + ]; + assert_eq!( + streamed, expected, + "expected exact rendered lines for loose/tight section" + ); + } + + #[test] + fn controller_streamed_table_matches_full_render_widths() { + let deltas = vec![ + "| Key | Description |\n", + "| --- | --- |\n", + "| -v | Enable very verbose logging output for debugging |\n", + "\n", + ]; + + let streamed = collect_streamed_lines(&deltas, Some(80)); + + let source: String = deltas.iter().copied().collect(); + let mut rendered = Vec::new(); + crate::markdown::append_markdown_agent(&source, /*width*/ Some(80), &mut rendered); + let expected = lines_to_plain_strings(&rendered); + + assert_eq!(streamed, expected); + } + + #[test] + fn controller_holds_blockquoted_table_tail_until_stable() { + let deltas = vec![ + "> | A | B |\n", + "> | --- | --- |\n", + "> | longvalue | ok |\n", + "\n", + ]; + + let streamed = collect_streamed_lines(&deltas, Some(80)); + + let source: String = deltas.iter().copied().collect(); + let mut rendered = Vec::new(); + crate::markdown::append_markdown_agent(&source, /*width*/ Some(80), &mut rendered); + let expected = lines_to_plain_strings(&rendered); + + assert_eq!(streamed, expected); + } + + #[test] + fn controller_keeps_pre_table_lines_queued_when_table_is_confirmed() { + let mut ctrl = stream_controller(Some(80)); + + ctrl.push("Intro line before table.\n"); + assert_eq!(ctrl.queued_lines(), 1); + + ctrl.push("| Key | Value |\n"); + ctrl.push("| --- | --- |\n"); + assert_eq!( + ctrl.queued_lines(), + 1, + "pre-table line should remain queued after table confirmation", + ); + + let (cell, idle) = ctrl.on_commit_tick(); + let committed = cell + .map(|cell| lines_to_plain_strings(&cell.transcript_lines(u16::MAX))) + .unwrap_or_default(); + assert!( + committed + .iter() + .any(|line| line.contains("Intro line before table.")), + "expected pre-table line to commit independently: {committed:?}", + ); + assert!(idle, "only pre-table content should have been queued"); + } + + #[test] + fn controller_set_width_during_confirmed_table_stream_matches_finalize_render() { + let mut ctrl = stream_controller(Some(120)); + let deltas = [ + "| Key | Description |\n", + "| --- | --- |\n", + "| one | value that should wrap after resize |\n", + ]; + for delta in deltas { + ctrl.push(delta); + } + assert_eq!( + ctrl.queued_lines(), + 0, + "confirmed table should remain mutable" + ); + + ctrl.set_width(Some(32)); + + let (cell, source) = ctrl.finalize(); + let source = source.expect("expected finalized source"); + let streamed = lines_to_plain_strings( + &cell + .expect("expected finalized table") + .transcript_lines(u16::MAX), + ) + .into_iter() + .map(|line| line.chars().skip(2).collect::()) + .collect::>(); + + let mut rendered = Vec::new(); + crate::markdown::append_markdown_agent(&source, /*width*/ Some(32), &mut rendered); + let expected = lines_to_plain_strings(&rendered); + assert_eq!(streamed, expected); + } + + #[test] + fn controller_does_not_hold_back_pipe_prose_without_table_delimiter() { + let mut ctrl = stream_controller(Some(80)); + + ctrl.push("status | owner | note\n"); + let (_first_commit, first_idle) = ctrl.on_commit_tick(); + assert!(first_idle); + + ctrl.push("next line\n"); + let (second_commit, _second_idle) = ctrl.on_commit_tick(); + assert!( + second_commit.is_some(), + "expected prose lines to be released once no table delimiter follows" + ); + } + + #[test] + fn controller_does_not_stall_repeated_pipe_prose_paragraphs() { + let mut ctrl = stream_controller(Some(80)); + + ctrl.push("alpha | beta\n\n"); + let (_first_commit, first_idle) = ctrl.on_commit_tick(); + assert!(first_idle); + + ctrl.push("gamma | delta\n\n"); + let (second_commit, _second_idle) = ctrl.on_commit_tick(); + let second_lines = second_commit + .map(|cell| lines_to_plain_strings(&cell.transcript_lines(u16::MAX))) + .unwrap_or_default(); + + assert!( + second_lines + .iter() + .any(|line| line.contains("alpha | beta")), + "expected the first pipe-prose paragraph to stream before finalize; got {second_lines:?}", + ); + } + + #[test] + fn controller_handles_table_immediately_after_heading() { + let deltas = vec![ + "### 1) Basic table\n", + "| Name | Role | Status |\n", + "|---|---|---|\n", + "| Alice | Admin | Active |\n", + "| Bob | Editor | Pending |\n", + "\n", + ]; + + let streamed = collect_streamed_lines(&deltas, Some(100)); + + let source: String = deltas.iter().copied().collect(); + let mut rendered = Vec::new(); + crate::markdown::append_markdown_agent(&source, /*width*/ Some(100), &mut rendered); + let expected = lines_to_plain_strings(&rendered); + + assert_eq!(streamed, expected); + } + + #[test] + fn controller_renders_unicode_for_multi_table_response_shape() { + let source = "Absolutely. Here are several different Markdown table patterns you can use for rendering tests.\n\n| Name | Role | + Location |\n|-------|-----------|----------|\n| Ava | Engineer | NYC |\n| Malik | Designer | Berlin |\n| Priya | PM | Remote + |\n\n| Item | Qty | Price | In Stock |\n|:------------|----:|------:|:--------:|\n| Keyboard | 2 | 49.99 | Yes |\n| Mouse | 10 + | 19.50 | Yes |\n| Monitor | 1 | 219.0 | No |\n\n| Field | Example | Notes + |\n|---------------|----------------------------------|--------------------------|\n| Escaped pipe | `foo \\| bar` | Should stay + in one cell |\n| Inline code | `let x = value;` | Monospace inline content |\n| Link | [OpenAI](https://openai.com) | + Standard markdown link |\n"; + + let chunked = source + .split_inclusive('\n') + .map(ToString::to_string) + .collect::>(); + let deltas = chunked.iter().map(String::as_str).collect::>(); + let streamed = collect_streamed_lines(&deltas, Some(120)); + assert!( + streamed.iter().any(|line| line.contains('┌')), + "expected unicode table border in streamed output: {streamed:?}" + ); + } + + #[test] + fn controller_renders_unicode_for_no_outer_pipes_table_shape() { + let source = "### 1) Basic\n\n| Name | Role | Active |\n|---|---|---|\n| Alice | Engineer | Yes |\n| Bob | Designer | No |\n\n### 2) No outer + pipes\n\nCol A | Col B | Col C\n--- | --- | ---\nx | y | z\n10 | 20 | 30\n\n### 3) Another table\n\n| Key | Value |\n|---|---|\n| a | b |\n"; + + let chunked = source + .split_inclusive('\n') + .map(ToString::to_string) + .collect::>(); + let deltas = chunked.iter().map(String::as_str).collect::>(); + let streamed = collect_streamed_lines(&deltas, Some(100)); + + let mut rendered = Vec::new(); + crate::markdown::append_markdown_agent(source, /*width*/ Some(100), &mut rendered); + let expected = lines_to_plain_strings(&rendered); + + assert_eq!(streamed, expected); + let has_raw_no_outer_header = streamed + .iter() + .any(|line| line.trim() == "Col A | Col B | Col C"); + assert!( + !has_raw_no_outer_header, + "no-outer-pipes header should not remain raw in final streamed output: {streamed:?}" + ); + } + + #[test] + fn controller_stabilizes_first_no_outer_pipes_table_in_response() { + let deltas = vec![ + "### No outer pipes first\n\n", + "Col A | Col B | Col C\n", + "--- | --- | ---\n", + "x | y | z\n", + "10 | 20 | 30\n", + "\n", + "After table paragraph.\n", + ]; + let streamed = collect_streamed_lines(&deltas, Some(100)); + + let source: String = deltas.iter().copied().collect(); + let mut rendered = Vec::new(); + crate::markdown::append_markdown_agent(&source, /*width*/ Some(100), &mut rendered); + let expected = lines_to_plain_strings(&rendered); + + assert_eq!(streamed, expected); + assert!( + streamed.iter().any(|line| line.contains('┌')), + "expected unicode table border for no-outer-pipes streaming: {streamed:?}" + ); + assert!( + !streamed + .iter() + .any(|line| line.trim() == "Col A | Col B | Col C"), + "did not expect raw no-outer-pipes header in final streamed output: {streamed:?}" + ); + } + + #[test] + fn controller_stabilizes_two_column_no_outer_table_in_response() { + let deltas = vec![ + "A | B\n", + "--- | ---\n", + "left | right\n", + "\n", + "After table paragraph.\n", + ]; + let streamed = collect_streamed_lines(&deltas, Some(80)); + + let source: String = deltas.iter().copied().collect(); + let mut rendered = Vec::new(); + crate::markdown::append_markdown_agent(&source, /*width*/ Some(80), &mut rendered); + let expected = lines_to_plain_strings(&rendered); + + assert_eq!(streamed, expected); + assert!( + streamed.iter().any(|line| line.contains('┌')), + "expected unicode table border for two-column no-outer table: {streamed:?}" + ); + assert!( + !streamed.iter().any(|line| line.trim() == "A | B"), + "did not expect raw two-column no-outer header in final streamed output: {streamed:?}" + ); + } + + #[test] + fn controller_converts_no_outer_table_between_preboxed_sections() { + let source = " ┌───────┬──────────┬────────┐\n │ Name │ Role │ Active │\n ├───────┼──────────┼────────┤\n │ Alice │ Engineer │ Yes │\n │ Bob │ Designer │ No │\n │ Cara │ PM │ Yes │\n └───────┴──────────┴────────┘\n\n ### 3) No outer pipes\n\n Col A | Col B | Col C\n --- | --- | ---\n x | y | z\n 10 | 20 | 30\n\n ┌─────────────────┬────────┬────────────────────────┐\n │ Example │ Output │ Notes │\n ├─────────────────┼────────┼────────────────────────┤\n │ a | b │ `a │ b` │\n │ npm run test │ ok │ Inline code formatting │\n │ SELECT * FROM t │ 3 rows │ SQL snippet │\n └─────────────────┴────────┴────────────────────────┘\n"; + + let deltas = source + .split_inclusive('\n') + .map(ToString::to_string) + .collect::>(); + let streamed = collect_streamed_lines( + &deltas.iter().map(String::as_str).collect::>(), + Some(100), + ); + + let has_raw_no_outer_header = streamed + .iter() + .any(|line| line.trim() == "Col A | Col B | Col C"); + assert!( + !has_raw_no_outer_header, + "no-outer table header remained raw in streamed output: {streamed:?}" + ); + assert!( + streamed + .iter() + .any(|line| line.contains("┌───────┬───────┬───────┐")), + "expected converted no-outer table border in streamed output: {streamed:?}" + ); + } + + #[test] + fn controller_keeps_markdown_fenced_tables_mutable_until_finalize() { + let source = "```md\n| A | B |\n|---|---|\n| 1 | 2 |\n```\n"; + let deltas = vec![ + "```md\n", + "| A | B |\n", + "|---|---|\n", + "| 1 | 2 |\n", + "```\n", + ]; + let streamed = collect_streamed_lines(&deltas, Some(80)); + + let mut rendered = Vec::new(); + crate::markdown::append_markdown_agent(source, /*width*/ Some(80), &mut rendered); + let expected = lines_to_plain_strings(&rendered); + + assert_eq!(streamed, expected); + assert!( + streamed.iter().any(|line| line.contains('┌')), + "expected unicode table border in streamed output: {streamed:?}" + ); + assert!( + !streamed.iter().any(|line| line.trim() == "| A | B |"), + "did not expect raw table header line after finalize: {streamed:?}" + ); + } + + #[test] + fn controller_keeps_markdown_fenced_no_outer_tables_mutable_until_finalize() { + let source = + "```md\nCol A | Col B | Col C\n--- | --- | ---\nx | y | z\n10 | 20 | 30\n```\n"; + let deltas = vec![ + "```md\n", + "Col A | Col B | Col C\n", + "--- | --- | ---\n", + "x | y | z\n", + "10 | 20 | 30\n", + "```\n", + ]; + let streamed = collect_streamed_lines(&deltas, Some(100)); + + let mut rendered = Vec::new(); + crate::markdown::append_markdown_agent(source, /*width*/ Some(100), &mut rendered); + let expected = lines_to_plain_strings(&rendered); + + assert_eq!(streamed, expected); + assert!( + streamed.iter().any(|line| line.contains('┌')), + "expected unicode table border in streamed output: {streamed:?}" + ); + assert!( + !streamed + .iter() + .any(|line| line.trim() == "Col A | Col B | Col C"), + "did not expect raw no-outer-pipes header line after finalize: {streamed:?}" + ); + } + + #[test] + fn controller_live_view_matches_render_during_interleaved_table_streaming() { + let source = "Project updates are easier to scan when narrative and structured data alternate.\n\n| Focus Area | Owner | Priority | Status |\n|---|---|---|---|\n| Authentication cleanup | Maya | High | 80% |\n| CLI error messages | Jordan | Medium | 55% |\n| Docs refresh | Lee | Low | 30% |\n\nThe first checkpoint shows progress, but we still have open risks.\n\n| Task | Command / Artifact | Due | State |\n|---|---|---|---|\n| Run unit tests | `cargo test -p codex-core` | Today | ✅ |\n| Snapshot review | `cargo insta pending-snapshots -p codex-tui` | Today | ⏳ |\n| Changelog draft | Release template (https://replacechangelog.com/) | Tomorrow | 📝 |\n\nFinal sign-off criteria are summarized below.\n"; + let width = Some(72usize); + let mut ctrl = stream_controller(width); + let mut emitted_lines: Vec> = Vec::new(); + + for delta in source.split_inclusive('\n') { + ctrl.push(delta); + loop { + let (cell, idle) = ctrl.on_commit_tick(); + if let Some(cell) = cell { + emitted_lines.extend(cell.transcript_lines(u16::MAX).into_iter().map(|line| { + let plain: String = line + .spans + .iter() + .map(|s| s.content.clone()) + .collect::>() + .join(""); + Line::from(plain.chars().skip(2).collect::()) + })); + } + if idle { + break; + } + } + + let mut visible = emitted_lines.clone(); + visible.extend(ctrl.current_tail_lines()); + let visible_plain = lines_to_plain_strings(&visible); + + let mut expected = Vec::new(); + crate::markdown::append_markdown_agent( + &ctrl.core.raw_source, + /*width*/ width, + &mut expected, + ); + let expected_plain = lines_to_plain_strings(&expected); + + assert_eq!( + visible_plain, expected_plain, + "live view diverged after delta: {delta:?}" + ); + } + } + + #[test] + fn controller_keeps_non_markdown_fenced_tables_as_code() { + let source = "```sh\n| A | B |\n|---|---|\n| 1 | 2 |\n```\n"; + let deltas = vec![ + "```sh\n", + "| A | B |\n", + "|---|---|\n", + "| 1 | 2 |\n", + "```\n", + ]; + let streamed = collect_streamed_lines(&deltas, Some(80)); + + let mut rendered = Vec::new(); + crate::markdown::append_markdown_agent(source, /*width*/ Some(80), &mut rendered); + let expected = lines_to_plain_strings(&rendered); + + assert_eq!(streamed, expected); + assert!( + streamed.iter().any(|line| line.trim() == "| A | B |"), + "expected code-fenced pipe line to remain raw: {streamed:?}" + ); + assert!( + !streamed.iter().any(|line| line.contains('┌')), + "did not expect unicode table border for non-markdown fence: {streamed:?}" + ); + } + + #[test] + fn plan_controller_streamed_table_matches_final_render() { + let deltas = vec![ + "## Build plan\n\n", + "| Step | Owner |\n", + "|---|---|\n", + "| Write tests | Agent |\n", + "| Verify output | User |\n", + "\n", + ]; + let streamed = collect_plan_streamed_lines(&deltas, Some(80)); + + let source: String = deltas.iter().copied().collect(); + let baseline = collect_plan_streamed_lines(&[source.as_str()], Some(80)); + + assert_eq!(streamed, baseline); + assert!( + streamed + .iter() + .any(|line| line.contains('│') || line.contains('└') || line.contains('┌')), + "expected unicode table box drawing chars in plan streamed output: {streamed:?}" + ); + assert!( + !streamed + .iter() + .any(|line| line.trim() == "| Step | Owner |"), + "did not expect raw table header line in plan output: {streamed:?}" + ); + } + + #[test] + fn plan_controller_streamed_markdown_fenced_table_matches_final_render() { + let deltas = vec![ + "## Build plan\n\n", + "```md\n", + "| Step | Owner |\n", + "|---|---|\n", + "| Write tests | Agent |\n", + "| Verify output | User |\n", + "```\n", + "\n", + ]; + let streamed = collect_plan_streamed_lines(&deltas, Some(80)); + + let source: String = deltas.iter().copied().collect(); + let baseline = collect_plan_streamed_lines(&[source.as_str()], Some(80)); + + assert_eq!(streamed, baseline); + assert!( + streamed + .iter() + .any(|line| line.contains('│') || line.contains('└') || line.contains('┌')), + "expected unicode table box drawing chars in fenced plan output: {streamed:?}" + ); + assert!( + !streamed + .iter() + .any(|line| line.trim() == "| Step | Owner |"), + "did not expect raw table header line in fenced plan output: {streamed:?}" + ); + } + + #[test] + fn table_holdback_state_detects_header_plus_delimiter() { + let source = "| Key | Description |\n| --- | --- |\n"; + assert!(matches!( + table_holdback_state(source), + TableHoldbackState::Confirmed { .. } + )); + } + + #[test] + fn table_holdback_state_detects_single_column_header_plus_delimiter() { + let source = "| Only |\n| --- |\n"; + assert!(matches!( + table_holdback_state(source), + TableHoldbackState::Confirmed { .. } + )); } #[test] - fn plan_lines_stream_in_order() { - let actual = collect_plan_streamed_lines(&["- one\n", "- two\n"], Some(80)); + fn table_holdback_state_ignores_table_like_lines_inside_unclosed_long_fence() { + let source = "````sh\n```cmd\n| Key | Description |\n| --- | --- |\n````\n"; assert!( - actual.iter().any(|line| line.contains("Proposed Plan")), - "expected plan header in streamed plan: {actual:?}", + matches!(table_holdback_state(source), TableHoldbackState::None), + "table holdback should ignore pipe lines inside an open non-markdown fence", ); + } + + #[test] + fn table_holdback_state_treats_indented_fence_text_as_plain_content() { + let source = " ```sh\n| Key | Description |\n| --- | --- |\n"; + assert!( + matches!( + table_holdback_state(source), + TableHoldbackState::Confirmed { .. } + ), + "indented fence-like text should not open a fence and should not block table detection", + ); + } + + #[test] + fn table_holdback_state_ignores_table_like_lines_inside_blockquoted_other_fence() { + let source = "> ```sh\n> | Key | Value |\n> | --- | --- |\n> ```\n"; + assert!( + matches!(table_holdback_state(source), TableHoldbackState::None), + "table holdback should ignore pipe lines inside non-markdown blockquoted fences", + ); + } + + #[test] + fn incremental_holdback_matches_stateless_scan_per_chunk() { + let chunks = [ + "status | owner\n", + "\n", + "> ```sh\n", + "> | A | B |\n", + "> | --- | --- |\n", + "> ```\n", + "> | Key | Value |\n", + "> | --- | --- |\n", + ]; + + let mut scanner = TableHoldbackScanner::new(); + let mut source = String::new(); + for chunk in chunks { + source.push_str(chunk); + scanner.push_source_chunk(chunk); + assert_eq!( + scanner.state(), + table_holdback_state(&source), + "scanner mismatch after chunk: {chunk:?}\nsource:\n{source}", + ); + } + } + + #[test] + fn incremental_holdback_detects_header_delimiter_across_chunk_boundary() { + let mut scanner = TableHoldbackScanner::new(); + scanner.push_source_chunk("| A | B |\n"); + assert_eq!( + scanner.state(), + TableHoldbackState::PendingHeader { header_start: 0 } + ); + scanner.push_source_chunk("| --- | --- |\n"); + assert_eq!( + scanner.state(), + TableHoldbackState::Confirmed { table_start: 0 } + ); + } + + #[test] + fn controller_set_width_after_first_line_emit_does_not_requeue_first_line() { + let mut ctrl = stream_controller(Some(120)); + ctrl.push( + "FIRSTTOKEN contains enough words to wrap once the width is reduced dramatically.\n", + ); + ctrl.push("second line remains pending\n"); + + let (first_emit, _) = ctrl.on_commit_tick(); + assert!(first_emit.is_some(), "expected first line emission"); + + ctrl.set_width(Some(20)); + + let (cell, _source) = ctrl.finalize(); + let remaining = cell + .map(|cell| lines_to_plain_strings(&cell.transcript_lines(u16::MAX))) + .unwrap_or_default() + .into_iter() + .map(|line| line.chars().skip(2).collect::()) + .collect::>(); + assert!( + !remaining.iter().any(|line| line.contains("FIRSTTOKEN")), + "first line should not be re-queued after resize: {remaining:?}", + ); + assert!( + remaining.iter().any(|line| line.contains("second line")), + "expected pending second line after resize: {remaining:?}", + ); + } + + #[test] + fn controller_set_width_partial_wrapped_emit_preserves_remaining_content() { + let mut ctrl = stream_controller(Some(20)); + ctrl.push("The quick brown fox jumps over the lazy dog near the riverbank.\n"); + ctrl.push("tail line\n"); + + let (first_emit, idle) = ctrl.on_commit_tick(); + assert!(first_emit.is_some(), "expected first wrapped line emission"); + assert!(!idle, "expected remaining queued content after one tick"); + assert!( + ctrl.queued_lines() > 0, + "expected non-empty queue before resize" + ); + + ctrl.set_width(Some(120)); + + let (cell, _source) = ctrl.finalize(); + let remaining = cell + .map(|c| lines_to_plain_strings(&c.transcript_lines(u16::MAX))) + .unwrap_or_default() + .into_iter() + .map(|line| line.chars().skip(2).collect::()) + .collect::>(); + assert!( + remaining.iter().any(|line| line.contains("tail line")), + "un-emitted content should remain after resize remap: {remaining:?}", + ); + } + + #[test] + fn controller_set_width_partial_wrapped_emit_keeps_wrapped_remainder() { + let mut ctrl = stream_controller(Some(18)); + ctrl.push("alpha beta gamma delta epsilon zeta eta theta iota kappa lambda mu\n"); + + let (first_emit, idle) = ctrl.on_commit_tick(); + assert!(first_emit.is_some(), "expected first wrapped line emission"); + assert!(!idle, "expected remaining wrapped content after one tick"); + assert!( + ctrl.queued_lines() > 0, + "expected queued wrapped remainder before resize" + ); + + ctrl.set_width(Some(80)); + + let (cell, _source) = ctrl.finalize(); + let remaining = cell + .map(|c| lines_to_plain_strings(&c.transcript_lines(u16::MAX))) + .unwrap_or_default(); + let joined = remaining.join(" "); assert!( - actual.iter().any(|line| line.contains("one")), - "expected plan body in streamed plan: {actual:?}", + joined.contains("kappa") || joined.contains("lambda") || joined.contains("mu"), + "wrapped remainder from partially emitted source line was lost after resize: {remaining:?}", ); } } diff --git a/codex-rs/tui/src/streaming/mod.rs b/codex-rs/tui/src/streaming/mod.rs index ddbac2e4c547..009a5eef6ea1 100644 --- a/codex-rs/tui/src/streaming/mod.rs +++ b/codex-rs/tui/src/streaming/mod.rs @@ -20,6 +20,7 @@ use crate::markdown_stream::MarkdownStreamCollector; pub(crate) mod chunking; pub(crate) mod commit_tick; pub(crate) mod controller; +mod table_holdback; struct QueuedLine { line: Line<'static>, diff --git a/codex-rs/tui/src/streaming/table_holdback.rs b/codex-rs/tui/src/streaming/table_holdback.rs new file mode 100644 index 000000000000..ba8c06386ee5 --- /dev/null +++ b/codex-rs/tui/src/streaming/table_holdback.rs @@ -0,0 +1,210 @@ +//! Pipe-table holdback scanner for source-backed agent streams. +//! +//! Agent streams with markdown tables keep the active table as mutable tail so +//! adding a row can reflow earlier table rows instead of committing a stale +//! render to scrollback. + +use std::time::Instant; + +use crate::table_detect::FenceKind; +use crate::table_detect::FenceTracker; +use crate::table_detect::is_table_delimiter_line; +use crate::table_detect::is_table_header_line; +use crate::table_detect::parse_table_segments; +use crate::table_detect::strip_blockquote_prefix; + +/// Result of scanning accumulated raw source for pipe-table patterns. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(super) enum TableHoldbackState { + /// No table detected -- all rendered lines can flow into the stable queue. + None, + /// The last non-blank line looks like a table header row but no delimiter + /// row has followed yet. Hold back in case the next delta is a delimiter. + PendingHeader { header_start: usize }, + /// A header + delimiter pair was found -- the source contains a confirmed + /// table. Content from the table header onward stays mutable. + Confirmed { table_start: usize }, +} + +#[derive(Clone, Copy)] +struct PreviousLineState { + source_start: usize, + fence_kind: FenceKind, + is_header: bool, +} + +/// Incremental scanner for table holdback state on append-only source streams. +pub(super) struct TableHoldbackScanner { + source_offset: usize, + fence_tracker: FenceTracker, + previous_line: Option, + pending_header_start: Option, + confirmed_table_start: Option, +} + +impl TableHoldbackScanner { + pub(super) fn new() -> Self { + Self { + source_offset: 0, + fence_tracker: FenceTracker::new(), + previous_line: None, + pending_header_start: None, + confirmed_table_start: None, + } + } + + pub(super) fn reset(&mut self) { + *self = Self::new(); + } + + pub(super) fn state(&self) -> TableHoldbackState { + if let Some(table_start) = self.confirmed_table_start { + TableHoldbackState::Confirmed { table_start } + } else if let Some(header_start) = self.pending_header_start { + TableHoldbackState::PendingHeader { header_start } + } else { + TableHoldbackState::None + } + } + + pub(super) fn push_source_chunk(&mut self, source_chunk: &str) { + if source_chunk.is_empty() { + return; + } + + let scan_start = Instant::now(); + let mut lines = 0usize; + for source_line in source_chunk.split_inclusive('\n') { + lines += 1; + self.push_line(source_line); + } + tracing::trace!( + bytes = source_chunk.len(), + lines, + state = ?self.state(), + elapsed_us = scan_start.elapsed().as_micros(), + "table holdback incremental scan", + ); + } + + fn push_line(&mut self, source_line: &str) { + let line = source_line.strip_suffix('\n').unwrap_or(source_line); + let source_start = self.source_offset; + let fence_kind = self.fence_tracker.kind(); + + let candidate_text = if fence_kind == FenceKind::Other { + None + } else { + table_candidate_text(line) + }; + let is_header = candidate_text.is_some_and(is_table_header_line); + let is_delimiter = candidate_text.is_some_and(is_table_delimiter_line); + + if self.confirmed_table_start.is_none() + && let Some(previous_line) = self.previous_line + && previous_line.fence_kind != FenceKind::Other + && fence_kind != FenceKind::Other + && previous_line.is_header + && is_delimiter + { + self.confirmed_table_start = Some(previous_line.source_start); + self.pending_header_start = None; + } + + if self.confirmed_table_start.is_none() && !line.trim().is_empty() { + if fence_kind != FenceKind::Other && is_header { + self.pending_header_start = Some(source_start); + } else { + self.pending_header_start = None; + } + } + + self.previous_line = Some(PreviousLineState { + source_start, + fence_kind, + is_header, + }); + + self.fence_tracker.advance(line); + self.source_offset = self.source_offset.saturating_add(source_line.len()); + } +} + +/// Strip blockquote prefixes and return the trimmed text if it contains +/// pipe-table segments, or `None` otherwise. +fn table_candidate_text(line: &str) -> Option<&str> { + let stripped = strip_blockquote_prefix(line).trim(); + parse_table_segments(stripped).map(|_| stripped) +} + +/// A source line annotated with whether it falls inside a fenced code block. +#[cfg(test)] +struct ParsedLine<'a> { + text: &'a str, + fence_context: FenceKind, + source_start: usize, +} + +/// Parse source into lines tagged with fenced-code context for table scanning. +#[cfg(test)] +fn parse_lines_with_fence_state(source: &str) -> Vec> { + let mut tracker = FenceTracker::new(); + let mut lines = Vec::new(); + let mut source_start = 0usize; + + for raw_line in source.split('\n') { + lines.push(ParsedLine { + text: raw_line, + fence_context: tracker.kind(), + source_start, + }); + + tracker.advance(raw_line); + source_start = source_start + .saturating_add(raw_line.len()) + .saturating_add(1); + } + + lines +} + +/// Scan `source` for pipe-table patterns outside of non-markdown fenced code +/// blocks. +#[cfg(test)] +pub(super) fn table_holdback_state(source: &str) -> TableHoldbackState { + let lines = parse_lines_with_fence_state(source); + for pair in lines.windows(2) { + let [header_line, delimiter_line] = pair else { + continue; + }; + if header_line.fence_context == FenceKind::Other + || delimiter_line.fence_context == FenceKind::Other + { + continue; + } + + let Some(header_text) = table_candidate_text(header_line.text) else { + continue; + }; + let Some(delimiter_text) = table_candidate_text(delimiter_line.text) else { + continue; + }; + + if is_table_header_line(header_text) && is_table_delimiter_line(delimiter_text) { + return TableHoldbackState::Confirmed { + table_start: header_line.source_start, + }; + } + } + + let pending_header = lines.iter().rev().find(|line| !line.text.trim().is_empty()); + if let Some(line) = pending_header + && line.fence_context != FenceKind::Other + && table_candidate_text(line.text).is_some_and(is_table_header_line) + { + return TableHoldbackState::PendingHeader { + header_start: line.source_start, + }; + } + TableHoldbackState::None +} diff --git a/codex-rs/tui/src/table_detect.rs b/codex-rs/tui/src/table_detect.rs new file mode 100644 index 000000000000..8ca0d8540ab7 --- /dev/null +++ b/codex-rs/tui/src/table_detect.rs @@ -0,0 +1,479 @@ +//! Canonical pipe-table structure detection and fenced-code-block tracking for +//! raw markdown source. +//! +//! Both the streaming controller (`streaming/controller.rs`) and the +//! markdown-fence unwrapper (`markdown.rs`) need to identify pipe-table +//! structure and fenced code blocks in raw markdown source. This module +//! provides the canonical implementations so fixes only need to happen in one +//! place. +//! +//! ## Concepts +//! +//! A GFM pipe table is a sequence of lines where: +//! - A **header line** contains pipe-separated segments with at least one +//! non-empty cell. +//! - A **delimiter line** immediately follows the header and contains only +//! alignment markers (`---`, `:---`, `---:`, `:---:`), each with at least +//! three dashes. +//! - **Body rows** follow the delimiter. +//! +//! A **fenced code block** starts with 3+ backticks or tildes and ends with a +//! matching close marker. [`FenceTracker`] classifies each line as +//! [`FenceKind::Outside`], [`FenceKind::Markdown`], or [`FenceKind::Other`] +//! so callers can skip pipe characters that appear inside non-markdown fences. +//! +//! The table functions operate on single lines and do not maintain cross-line +//! state. Callers (the streaming controller and fence unwrapper) are +//! responsible for pairing consecutive lines to confirm a table. + +/// Split a pipe-delimited line into trimmed segments. +/// +/// Returns `None` if the line is empty or has no unescaped separator marker. +/// Leading/trailing pipes are stripped before splitting. +pub(crate) fn parse_table_segments(line: &str) -> Option> { + let trimmed = line.trim(); + if trimmed.is_empty() { + return None; + } + + let has_outer_pipe = trimmed.starts_with('|') || trimmed.ends_with('|'); + let content = trimmed.strip_prefix('|').unwrap_or(trimmed); + let content = content.strip_suffix('|').unwrap_or(content); + let raw_segments = split_unescaped_pipe(content); + if !has_outer_pipe && raw_segments.len() <= 1 { + return None; + } + + let segments: Vec<&str> = raw_segments.into_iter().map(str::trim).collect(); + (!segments.is_empty()).then_some(segments) +} + +/// Split `content` on unescaped `|` characters. +/// +/// A pipe preceded by `\` is treated as literal text, not a column separator. +/// The backslash remains in the segment (this is structure detection, not +/// rendering). +fn split_unescaped_pipe(content: &str) -> Vec<&str> { + let mut segments = Vec::with_capacity(8); + let mut start = 0; + let bytes = content.as_bytes(); + let mut i = 0; + while i < bytes.len() { + if bytes[i] == b'\\' { + // Skip the escaped character. + i += 2; + } else if bytes[i] == b'|' { + segments.push(&content[start..i]); + start = i + 1; + i += 1; + } else { + i += 1; + } + } + segments.push(&content[start..]); + segments +} + +// Small table-detection helpers inlined for the streaming hot path — they are +// called on every source line during incremental holdback scanning. + +/// Whether `line` looks like a table header row (has pipe-separated +/// segments with at least one non-empty cell). +#[inline] +pub(crate) fn is_table_header_line(line: &str) -> bool { + parse_table_segments(line).is_some_and(|segments| segments.iter().any(|s| !s.is_empty())) +} + +/// Whether a single segment matches the `---`, `:---`, `---:`, or `:---:` +/// alignment-colon syntax used in markdown table delimiter rows. +#[inline] +fn is_table_delimiter_segment(segment: &str) -> bool { + let trimmed = segment.trim(); + if trimmed.is_empty() { + return false; + } + let without_leading = trimmed.strip_prefix(':').unwrap_or(trimmed); + let without_ends = without_leading.strip_suffix(':').unwrap_or(without_leading); + without_ends.len() >= 3 && without_ends.chars().all(|c| c == '-') +} + +/// Whether `line` is a valid table delimiter row (every segment passes +/// [`is_table_delimiter_segment`]). +#[inline] +pub(crate) fn is_table_delimiter_line(line: &str) -> bool { + parse_table_segments(line) + .is_some_and(|segments| segments.into_iter().all(is_table_delimiter_segment)) +} + +// --------------------------------------------------------------------------- +// Fenced code block tracking +// --------------------------------------------------------------------------- + +/// Where a source line sits relative to fenced code blocks. +/// +/// Table holdback only applies to lines that are `Outside` or inside a +/// `Markdown` fence. Lines inside `Other` fences (e.g. `sh`, `rust`) are +/// ignored by the table scanner because their pipe characters are code, not +/// table syntax. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum FenceKind { + /// Not inside any fenced code block. + Outside, + /// Inside a `` ```md `` or `` ```markdown `` fence. + Markdown, + /// Inside a fence with a non-markdown info string. + Other, +} + +/// Incremental tracker for fenced-code-block open/close transitions. +/// +/// Feed lines one at a time via [`advance`](Self::advance); query the current +/// context with [`kind`](Self::kind). The tracker handles leading-whitespace +/// limits (>3 spaces → not a fence), blockquote prefix stripping, and +/// backtick/tilde marker matching. +pub(crate) struct FenceTracker { + state: Option<(char, usize, FenceKind)>, +} + +impl FenceTracker { + #[inline] + pub(crate) fn new() -> Self { + Self { state: None } + } + + /// Process one raw source line and update fence state. + /// + /// Lines with >3 leading spaces are ignored (indented code blocks, not + /// fences). Blockquote prefixes (`>`) are stripped before scanning. + pub(crate) fn advance(&mut self, raw_line: &str) { + let leading_spaces = raw_line + .as_bytes() + .iter() + .take_while(|byte| **byte == b' ') + .count(); + if leading_spaces > 3 { + return; + } + + let trimmed = &raw_line[leading_spaces..]; + let fence_scan_text = strip_blockquote_prefix(trimmed); + if let Some((marker, len)) = parse_fence_marker(fence_scan_text) { + if let Some((open_char, open_len, _)) = self.state { + // Close the current fence if the marker matches. + if marker == open_char + && len >= open_len + && fence_scan_text[len..].trim().is_empty() + { + self.state = None; + } + } else { + // Opening a new fence. + let kind = if is_markdown_fence_info(fence_scan_text, len) { + FenceKind::Markdown + } else { + FenceKind::Other + }; + self.state = Some((marker, len, kind)); + } + } + } + + /// Current fence context for the most-recently-advanced line. + #[inline] + pub(crate) fn kind(&self) -> FenceKind { + self.state.map_or(FenceKind::Outside, |(_, _, k)| k) + } +} + +/// Return fence marker character and run length for a potential fence line. +/// +/// Recognises backtick and tilde fences with a minimum run of 3. +/// The input should already have leading whitespace and blockquote prefixes +/// stripped. +#[inline] +pub(crate) fn parse_fence_marker(line: &str) -> Option<(char, usize)> { + let first = line.as_bytes().first().copied()?; + if first != b'`' && first != b'~' { + return None; + } + let len = line.bytes().take_while(|&b| b == first).count(); + if len < 3 { + return None; + } + Some((first as char, len)) +} + +/// Whether the info string after a fence marker indicates markdown content. +/// +/// Matches `md` and `markdown` (case-insensitive). +#[inline] +pub(crate) fn is_markdown_fence_info(trimmed_line: &str, marker_len: usize) -> bool { + let info = trimmed_line[marker_len..] + .split_whitespace() + .next() + .unwrap_or_default(); + info.eq_ignore_ascii_case("md") || info.eq_ignore_ascii_case("markdown") +} + +/// Peel all leading `>` blockquote markers from a line. +/// +/// Tables can appear inside blockquotes (`> | A | B |`), so the holdback +/// scanner must strip these markers before checking for table syntax. +#[inline] +pub(crate) fn strip_blockquote_prefix(line: &str) -> &str { + let mut rest = line.trim_start(); + loop { + let Some(stripped) = rest.strip_prefix('>') else { + return rest; + }; + rest = stripped.strip_prefix(' ').unwrap_or(stripped).trim_start(); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_table_segments_basic() { + assert_eq!( + parse_table_segments("| A | B | C |"), + Some(vec!["A", "B", "C"]) + ); + } + + #[test] + fn parse_table_segments_no_outer_pipes() { + assert_eq!(parse_table_segments("A | B | C"), Some(vec!["A", "B", "C"])); + } + + #[test] + fn parse_table_segments_no_leading_pipe() { + assert_eq!( + parse_table_segments("A | B | C |"), + Some(vec!["A", "B", "C"]) + ); + } + + #[test] + fn parse_table_segments_no_trailing_pipe() { + assert_eq!( + parse_table_segments("| A | B | C"), + Some(vec!["A", "B", "C"]) + ); + } + + #[test] + fn parse_table_segments_single_segment_is_allowed() { + assert_eq!(parse_table_segments("| only |"), Some(vec!["only"])); + } + + #[test] + fn parse_table_segments_without_pipe_returns_none() { + assert_eq!(parse_table_segments("just text"), None); + } + + #[test] + fn parse_table_segments_empty_returns_none() { + assert_eq!(parse_table_segments(""), None); + assert_eq!(parse_table_segments(" "), None); + } + + #[test] + fn parse_table_segments_escaped_pipe() { + // Escaped pipe should NOT split — stays inside the segment. + assert_eq!( + parse_table_segments(r"| A \| B | C |"), + Some(vec![r"A \| B", "C"]) + ); + } + + #[test] + fn is_table_delimiter_segment_valid() { + assert!(is_table_delimiter_segment("---")); + assert!(is_table_delimiter_segment(":---")); + assert!(is_table_delimiter_segment("---:")); + assert!(is_table_delimiter_segment(":---:")); + assert!(is_table_delimiter_segment(":-------:")); + } + + #[test] + fn is_table_delimiter_segment_invalid() { + assert!(!is_table_delimiter_segment("")); + assert!(!is_table_delimiter_segment("--")); + assert!(!is_table_delimiter_segment("abc")); + assert!(!is_table_delimiter_segment(":--")); + } + + #[test] + fn is_table_delimiter_line_valid() { + assert!(is_table_delimiter_line("| --- | --- |")); + assert!(is_table_delimiter_line("|:---:|---:|")); + assert!(is_table_delimiter_line("--- | --- | ---")); + } + + #[test] + fn is_table_delimiter_line_invalid() { + assert!(!is_table_delimiter_line("| A | B |")); + assert!(!is_table_delimiter_line("| -- | -- |")); + } + + #[test] + fn is_table_header_line_valid() { + assert!(is_table_header_line("| A | B |")); + assert!(is_table_header_line("Name | Value")); + } + + #[test] + fn is_table_header_line_all_empty_segments() { + assert!(!is_table_header_line("| | |")); + } + + // ----------------------------------------------------------------------- + // FenceTracker tests + // ----------------------------------------------------------------------- + + #[test] + fn fence_tracker_outside_by_default() { + let tracker = FenceTracker::new(); + assert_eq!(tracker.kind(), FenceKind::Outside); + } + + #[test] + fn fence_tracker_opens_and_closes_backtick_fence() { + let mut tracker = FenceTracker::new(); + tracker.advance("```rust"); + assert_eq!(tracker.kind(), FenceKind::Other); + + tracker.advance("let x = 1;"); + assert_eq!(tracker.kind(), FenceKind::Other); + + tracker.advance("```"); + assert_eq!(tracker.kind(), FenceKind::Outside); + } + + #[test] + fn fence_tracker_opens_and_closes_tilde_fence() { + let mut tracker = FenceTracker::new(); + tracker.advance("~~~python"); + assert_eq!(tracker.kind(), FenceKind::Other); + tracker.advance("~~~"); + assert_eq!(tracker.kind(), FenceKind::Outside); + } + + #[test] + fn fence_tracker_markdown_fence() { + let mut tracker = FenceTracker::new(); + tracker.advance("```md"); + assert_eq!(tracker.kind(), FenceKind::Markdown); + tracker.advance("| A | B |"); + assert_eq!(tracker.kind(), FenceKind::Markdown); + tracker.advance("```"); + assert_eq!(tracker.kind(), FenceKind::Outside); + } + + #[test] + fn fence_tracker_markdown_case_insensitive() { + let mut tracker = FenceTracker::new(); + tracker.advance("```Markdown"); + assert_eq!(tracker.kind(), FenceKind::Markdown); + tracker.advance("```"); + assert_eq!(tracker.kind(), FenceKind::Outside); + } + + #[test] + fn fence_tracker_nested_shorter_marker_does_not_close() { + let mut tracker = FenceTracker::new(); + tracker.advance("````sh"); + assert_eq!(tracker.kind(), FenceKind::Other); + // Shorter marker inside should not close. + tracker.advance("```"); + assert_eq!(tracker.kind(), FenceKind::Other); + // Matching length closes. + tracker.advance("````"); + assert_eq!(tracker.kind(), FenceKind::Outside); + } + + #[test] + fn fence_tracker_mismatched_char_does_not_close() { + let mut tracker = FenceTracker::new(); + tracker.advance("```sh"); + assert_eq!(tracker.kind(), FenceKind::Other); + // Tilde marker should not close a backtick fence. + tracker.advance("~~~"); + assert_eq!(tracker.kind(), FenceKind::Other); + tracker.advance("```"); + assert_eq!(tracker.kind(), FenceKind::Outside); + } + + #[test] + fn fence_tracker_indented_4_spaces_ignored() { + let mut tracker = FenceTracker::new(); + tracker.advance(" ```sh"); + assert_eq!(tracker.kind(), FenceKind::Outside); + } + + #[test] + fn fence_tracker_blockquote_prefix_stripped() { + let mut tracker = FenceTracker::new(); + tracker.advance("> ```sh"); + assert_eq!(tracker.kind(), FenceKind::Other); + tracker.advance("> ```"); + assert_eq!(tracker.kind(), FenceKind::Outside); + } + + #[test] + fn fence_tracker_close_with_trailing_content_does_not_close() { + let mut tracker = FenceTracker::new(); + tracker.advance("```sh"); + assert_eq!(tracker.kind(), FenceKind::Other); + // Trailing content prevents closing. + tracker.advance("``` extra"); + assert_eq!(tracker.kind(), FenceKind::Other); + tracker.advance("```"); + assert_eq!(tracker.kind(), FenceKind::Outside); + } + + // ----------------------------------------------------------------------- + // Fence helper function tests + // ----------------------------------------------------------------------- + + #[test] + fn parse_fence_marker_backtick() { + assert_eq!(parse_fence_marker("```rust"), Some(('`', 3))); + assert_eq!(parse_fence_marker("````"), Some(('`', 4))); + } + + #[test] + fn parse_fence_marker_tilde() { + assert_eq!(parse_fence_marker("~~~python"), Some(('~', 3))); + } + + #[test] + fn parse_fence_marker_too_short() { + assert_eq!(parse_fence_marker("``"), None); + assert_eq!(parse_fence_marker("~~"), None); + } + + #[test] + fn parse_fence_marker_not_fence() { + assert_eq!(parse_fence_marker("hello"), None); + assert_eq!(parse_fence_marker(""), None); + } + + #[test] + fn is_markdown_fence_info_basic() { + assert!(is_markdown_fence_info("```md", /*marker_len*/ 3)); + assert!(is_markdown_fence_info("```markdown", /*marker_len*/ 3)); + assert!(is_markdown_fence_info("```MD", /*marker_len*/ 3)); + assert!(!is_markdown_fence_info("```rust", /*marker_len*/ 3)); + assert!(!is_markdown_fence_info("```", /*marker_len*/ 3)); + } + + #[test] + fn strip_blockquote_prefix_basic() { + assert_eq!(strip_blockquote_prefix("> hello"), "hello"); + assert_eq!(strip_blockquote_prefix("> > nested"), "nested"); + assert_eq!(strip_blockquote_prefix("no prefix"), "no prefix"); + } +} From 0f6607eb10f5b41ccc46967a932fc7faa79f2186 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Sun, 10 May 2026 16:06:57 -0300 Subject: [PATCH 2/3] docs(tui): clarify markdown table streaming invariants --- .../src/app/agent_message_consolidation.rs | 12 +++++ codex-rs/tui/src/streaming/controller.rs | 53 ++++++++++++++++--- codex-rs/tui/src/streaming/table_holdback.rs | 32 +++++++++++ codex-rs/tui/src/table_detect.rs | 9 ++++ 4 files changed, 98 insertions(+), 8 deletions(-) diff --git a/codex-rs/tui/src/app/agent_message_consolidation.rs b/codex-rs/tui/src/app/agent_message_consolidation.rs index 5e7991529099..31ba087e39ef 100644 --- a/codex-rs/tui/src/app/agent_message_consolidation.rs +++ b/codex-rs/tui/src/app/agent_message_consolidation.rs @@ -1,3 +1,12 @@ +//! Transcript consolidation for finalized streaming agent messages. +//! +//! During streaming, the chat widget emits transient `AgentMessageCell`s so it +//! can animate stable lines into scrollback while keeping the active mutable +//! tail in the bottom pane. Once the answer finishes, the app replaces that +//! trailing run with a single source-backed `AgentMarkdownCell`. This makes the +//! transcript the canonical owner of the raw markdown source used for future +//! resize re-renders. + use std::path::PathBuf; use std::sync::Arc; @@ -20,6 +29,9 @@ impl App { scrollback_reflow: ConsolidationScrollbackReflow, deferred_history_cell: Option>, ) -> Result<()> { + // Some finalize paths must preserve a last provisional stream cell long + // enough for queue ordering, then fold it into the canonical + // source-backed cell during consolidation. if let Some(cell) = deferred_history_cell { let cell: Arc = cell.into(); if let Some(Overlay::Transcript(t)) = &mut self.overlay { diff --git a/codex-rs/tui/src/streaming/controller.rs b/codex-rs/tui/src/streaming/controller.rs index 37d064837080..0427af24c295 100644 --- a/codex-rs/tui/src/streaming/controller.rs +++ b/codex-rs/tui/src/streaming/controller.rs @@ -90,8 +90,14 @@ struct StreamCore { } struct StablePrefixLenCache { + /// Byte offset of the candidate table/header start in `raw_source`. source_start: usize, + /// Width that produced `stable_prefix_len`. width: Option, + /// Rendered line count for `raw_source[..source_start]` at `width`. + /// + /// The streaming controller uses this to avoid repeatedly re-rendering the + /// same stable prefix while a live table tail is still mutating. stable_prefix_len: usize, } @@ -111,8 +117,13 @@ impl StreamCore { } } - /// Push a delta; if it contains a newline, commit completed lines and enqueue newly-stable - /// lines. Returns `true` if new lines were enqueued. + /// Push a streaming delta and enqueue any newly-stable rendered lines. + /// + /// Only newline-terminated source is committed into `raw_source`. This is + /// important for tables because an unterminated partial row must stay out + /// of both the stable queue and the live tail until its structure is + /// unambiguous; otherwise the user can briefly see malformed columns that + /// immediately disappear on the next delta. fn push_delta(&mut self, delta: &str) -> bool { if !delta.is_empty() { self.state.has_seen_delta = true; @@ -131,8 +142,13 @@ impl StreamCore { enqueued } - /// Drain the collector, re-render, and return lines not yet emitted. - /// Does NOT reset state - the caller must call `reset()` afterward. + /// Drain the collector, render the final source snapshot, and return lines not yet emitted. + /// + /// This intentionally re-renders from the full raw source instead of + /// trying to stitch together queued stable lines and the current tail. The + /// final render is the canonical transcript representation used for + /// consolidation, so callers that skip `reset()` can accidentally replay a + /// finished stream into the next answer. fn finalize_remaining(&mut self) -> Vec> { let remainder_source = self.state.collector.finalize_and_drain_source(); if !remainder_source.is_empty() { @@ -187,8 +203,11 @@ impl StreamCore { /// Lines that belong to the mutable tail, not yet queued for stable commit. /// - /// The tail starts at `enqueued_stable_len` -- everything from that offset to - /// the end of `rendered_lines` is displayed live in the active-cell slot. + /// The tail starts at `enqueued_stable_len`, so this returns the portion + /// of the current render snapshot that is still allowed to change without + /// violating scrollback ordering. If callers were to derive the tail from + /// `emitted_stable_len` instead, queued-but-not-yet-emitted lines could + /// reappear in the active cell and duplicate content on screen. #[inline] fn current_tail_lines(&self) -> Vec> { let start = self.enqueued_stable_len.min(self.rendered_lines.len()); @@ -204,6 +223,11 @@ impl StreamCore { /// /// Re-renders once at the new width and rebuilds queue state from the /// current emitted line count. + /// + /// Resize is the point where source-backed rendering matters most: + /// previously emitted prose must stay in scrollback order, while any live + /// table tail is free to reshape at the new width. This method preserves + /// that split without attempting byte-for-byte line remapping. fn set_width(&mut self, width: Option) { if self.width == width { return; @@ -334,8 +358,11 @@ impl StreamCore { true } - /// Rebuild the stable queue from the current render snapshot. Used after `set_width()` where - /// the old queue is stale. + /// Rebuild the stable queue from the current render snapshot. + /// + /// This is used after `set_width()`, where any queued lines were computed + /// against the old width and can no longer be trusted to line up with the + /// current render. fn rebuild_stable_queue_from_render(&mut self) { let target_stable_len = self.compute_target_stable_len(); self.state.clear_queue(); @@ -376,6 +403,11 @@ impl StreamCore { tail_budget } + /// Convert a raw-source boundary into the number of rendered tail lines. + /// + /// The important contract here is that the holdback scanner reasons in + /// byte offsets while the queue operates in rendered lines. This helper is + /// the only place where those coordinate systems are bridged. fn tail_budget_from_source_start(&mut self, source_start: usize) -> usize { if source_start == 0 { return self.rendered_lines.len(); @@ -385,6 +417,11 @@ impl StreamCore { self.rendered_lines.len().saturating_sub(stable_prefix_len) } + /// Render the stable prefix before `source_start` and return its line count. + /// + /// This value is cached because dense table streams can call into this path + /// for every committed line while the header/delimiter/body are still + /// arriving incrementally. fn stable_prefix_len_for_source_start(&mut self, source_start: usize) -> usize { if let Some(cache) = &self.stable_prefix_len_cache && cache.source_start == source_start diff --git a/codex-rs/tui/src/streaming/table_holdback.rs b/codex-rs/tui/src/streaming/table_holdback.rs index ba8c06386ee5..d208b60bd84d 100644 --- a/codex-rs/tui/src/streaming/table_holdback.rs +++ b/codex-rs/tui/src/streaming/table_holdback.rs @@ -3,6 +3,11 @@ //! Agent streams with markdown tables keep the active table as mutable tail so //! adding a row can reflow earlier table rows instead of committing a stale //! render to scrollback. +//! +//! The scanner is intentionally conservative: it only looks for enough +//! structure to decide where the mutable tail should start. It does not try to +//! validate an entire table or predict final layout. Rendering remains the job +//! of the markdown renderer. use std::time::Instant; @@ -26,6 +31,10 @@ pub(super) enum TableHoldbackState { Confirmed { table_start: usize }, } +/// Facts remembered about the previous committed source line. +/// +/// The scanner only needs one-line lookbehind because a table is confirmed by +/// a header row immediately followed by a delimiter row. #[derive(Clone, Copy)] struct PreviousLineState { source_start: usize, @@ -34,6 +43,11 @@ struct PreviousLineState { } /// Incremental scanner for table holdback state on append-only source streams. +/// +/// `push_source_chunk` must receive source in the same order it will be +/// appended to the stream. The scanner stores byte offsets into that logical +/// source buffer, so feeding chunks out of order would make later tail +/// boundaries point at the wrong rendered region. pub(super) struct TableHoldbackScanner { source_offset: usize, fence_tracker: FenceTracker, @@ -57,6 +71,13 @@ impl TableHoldbackScanner { *self = Self::new(); } + /// Return the current holdback decision for the committed source prefix. + /// + /// `PendingHeader` means the newest non-blank line looks like a table + /// header but the delimiter row has not arrived yet, so callers should + /// optimistically keep that region mutable. `Confirmed` means the header + /// and delimiter pair has been seen and all subsequent body rows remain in + /// the live tail until finalization. pub(super) fn state(&self) -> TableHoldbackState { if let Some(table_start) = self.confirmed_table_start { TableHoldbackState::Confirmed { table_start } @@ -67,6 +88,13 @@ impl TableHoldbackScanner { } } + /// Advance the scanner with newly committed source. + /// + /// Chunks are expected to contain only source that is now safe to commit + /// into `raw_source`, typically newline-terminated lines from the + /// streaming collector. Partial rows are intentionally excluded so the + /// scanner never treats an unfinished table row as a stable structural + /// signal. pub(super) fn push_source_chunk(&mut self, source_chunk: &str) { if source_chunk.is_empty() { return; @@ -87,6 +115,7 @@ impl TableHoldbackScanner { ); } + /// Fold one committed source line into the scanner state machine. fn push_line(&mut self, source_line: &str) { let line = source_line.strip_suffix('\n').unwrap_or(source_line); let source_start = self.source_offset; @@ -132,6 +161,9 @@ impl TableHoldbackScanner { /// Strip blockquote prefixes and return the trimmed text if it contains /// pipe-table segments, or `None` otherwise. +/// +/// Table holdback treats quoted tables as real tables, but it still requires a +/// pipe-table shape after the quote markers are removed. fn table_candidate_text(line: &str) -> Option<&str> { let stripped = strip_blockquote_prefix(line).trim(); parse_table_segments(stripped).map(|_| stripped) diff --git a/codex-rs/tui/src/table_detect.rs b/codex-rs/tui/src/table_detect.rs index 8ca0d8540ab7..39ca35a8d098 100644 --- a/codex-rs/tui/src/table_detect.rs +++ b/codex-rs/tui/src/table_detect.rs @@ -30,6 +30,11 @@ /// /// Returns `None` if the line is empty or has no unescaped separator marker. /// Leading/trailing pipes are stripped before splitting. +/// +/// This is intentionally a structural parser, not a renderer. It preserves +/// escaped pipes inside the returned segments because callers only care about +/// whether the line can participate in a table, not how the cell text should +/// finally be displayed. pub(crate) fn parse_table_segments(line: &str) -> Option> { let trimmed = line.trim(); if trimmed.is_empty() { @@ -131,6 +136,10 @@ pub(crate) enum FenceKind { /// context with [`kind`](Self::kind). The tracker handles leading-whitespace /// limits (>3 spaces → not a fence), blockquote prefix stripping, and /// backtick/tilde marker matching. +/// +/// The tracker reports the fence context that applies to the current line +/// before that line mutates the state. Callers rely on that when deciding +/// whether the current raw line can open or continue a table. pub(crate) struct FenceTracker { state: Option<(char, usize, FenceKind)>, } From 9d7a002469dc21eefbca64930b1015544ed3d494 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Sun, 10 May 2026 17:22:11 -0300 Subject: [PATCH 3/3] fix(tui): render plan markdown tables --- .../src/chatwidget/tests/status_and_layout.rs | 11 +++- codex-rs/tui/src/history_cell.rs | 54 ++++++++++++++++++- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs index a1b2cf423d8c..2f4aa30b3bc5 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs @@ -369,19 +369,28 @@ async fn completed_plan_table_tail_skips_provisional_history_insert() { let mut saw_source_backed_plan = false; let mut saw_stream_plan = false; + let mut rendered_plan = String::new(); while let Ok(event) = rx.try_recv() { if let AppEvent::InsertHistoryCell(cell) = event { - saw_source_backed_plan |= cell.as_any().is::(); + if cell.as_any().is::() { + saw_source_backed_plan = true; + rendered_plan = lines_to_single_string(&cell.display_lines(/*width*/ 80)); + } saw_stream_plan |= cell.as_any().is::(); } } assert!(saw_source_backed_plan, "expected source-backed plan insert"); + assert!( + rendered_plan.contains('│') || rendered_plan.contains('┌'), + "expected completed plan table to render as a boxed table, got: {rendered_plan:?}" + ); assert!( !saw_stream_plan, "live plan table tail should not be inserted provisionally" ); } + #[tokio::test] async fn helpers_are_available_and_do_not_panic() { let (tx_raw, _rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index a7b54158e2c9..4521fe9a7b45 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -22,6 +22,7 @@ use crate::exec_command::strip_bash_lc_and_escape; use crate::legacy_core::config::Config; use crate::live_wrap::take_prefix_by_width; use crate::markdown::append_markdown; +use crate::markdown::append_markdown_agent_with_cwd; use crate::motion::MotionMode; use crate::motion::ReducedMotionIndicator; use crate::motion::activity_indicator; @@ -3094,7 +3095,7 @@ impl HistoryCell for ProposedPlanCell { let plan_style = proposed_plan_style(); let wrap_width = width.saturating_sub(4).max(1) as usize; let mut body: Vec> = Vec::new(); - append_markdown( + append_markdown_agent_with_cwd( &self.plan_markdown, Some(wrap_width), Some(self.cwd.as_path()), @@ -3778,6 +3779,57 @@ mod tests { assert_unstyled_lines(&plan_lines); } + #[test] + fn proposed_plan_cell_renders_markdown_table() { + let plan = new_proposed_plan( + "## Plan\n\n| Step | Owner |\n| --- | --- |\n| Verify | Codex |\n".to_string(), + &test_cwd(), + ); + + let rendered = render_lines(&plan.display_lines(/*width*/ 80)); + + assert!( + rendered + .iter() + .any(|line| line.contains('│') || line.contains('┌')), + "expected boxed table in proposed plan output: {rendered:?}" + ); + assert!( + !rendered + .iter() + .any(|line| line.trim() == "| Step | Owner |"), + "did not expect raw table header in rich proposed plan output: {rendered:?}" + ); + + let raw = render_lines(&plan.raw_lines()); + assert!( + raw.iter().any(|line| line == "| Step | Owner |"), + "expected raw mode to preserve table markdown source: {raw:?}" + ); + } + + #[test] + fn proposed_plan_cell_unwraps_markdown_fenced_table() { + let plan = new_proposed_plan( + "## Plan\n\n```markdown\n| Step | Owner |\n| --- | --- |\n| Verify | Codex |\n```\n" + .to_string(), + &test_cwd(), + ); + + let rendered = render_lines(&plan.display_lines(/*width*/ 80)); + + assert!( + rendered + .iter() + .any(|line| line.contains('│') || line.contains('┌')), + "expected boxed table for markdown-fenced proposed plan output: {rendered:?}" + ); + assert!( + !rendered.iter().any(|line| line.trim() == "```markdown"), + "did not expect markdown fence to render as code block: {rendered:?}" + ); + } + #[test] fn structured_tool_cell_renders_raw_plain_text_without_prefix_or_style() { let invocation = McpInvocation {