diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index c3c29aa557..9ce01173f5 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -485,6 +485,9 @@ "steer": { "type": "boolean" }, + "stream_table_live_tail_reflow": { + "type": "boolean" + }, "tool_call_mcp_elicitation": { "type": "boolean" }, @@ -2287,6 +2290,9 @@ "steer": { "type": "boolean" }, + "stream_table_live_tail_reflow": { + "type": "boolean" + }, "tool_call_mcp_elicitation": { "type": "boolean" }, diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index 590a1ff69f..9fd0775ed2 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -182,6 +182,8 @@ pub enum Feature { TuiAppServer, /// Prevent idle system sleep while a turn is actively running. PreventIdleSleep, + /// Speculatively re-render table holdback tail with uncommitted source. + StreamTableLiveTailReflow, /// Legacy rollout flag for Responses API WebSocket transport experiments. ResponsesWebsockets, /// Legacy rollout flag for Responses API WebSocket transport v2 experiments. @@ -897,6 +899,12 @@ pub const FEATURES: &[FeatureSpec] = &[ }, default_enabled: false, }, + FeatureSpec { + id: Feature::StreamTableLiveTailReflow, + key: "stream_table_live_tail_reflow", + stage: Stage::UnderDevelopment, + default_enabled: false, + }, FeatureSpec { id: Feature::ResponsesWebsockets, key: "responses_websockets", diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 76da2ba900..066a428d6d 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -2,6 +2,7 @@ use crate::app_backtrack::BacktrackState; use crate::app_command::AppCommand; use crate::app_command::AppCommandView; use crate::app_event::AppEvent; +use crate::app_event::ConsolidationScrollbackReflow; use crate::app_event::ExitMode; use crate::app_event::FeedbackCategory; use crate::app_event::RateLimitRefreshOrigin; @@ -50,6 +51,8 @@ use crate::resume_picker::SessionSelection; use crate::resume_picker::SessionTarget; #[cfg(test)] use crate::test_support::PathBufExt; +use crate::transcript_reflow::TRANSCRIPT_REFLOW_DEBOUNCE; +use crate::transcript_reflow::TranscriptReflowState; use crate::tui; use crate::tui::TuiEvent; use crate::update_action::UpdateAction; @@ -280,7 +283,6 @@ fn guardian_approvals_mode() -> GuardianApprovalsMode { /// Smooth-mode streaming drains one line per tick, so this interval controls /// perceived typing speed for non-backlogged output. const COMMIT_ANIMATION_TICK: Duration = tui::TARGET_FRAME_INTERVAL; - #[derive(Debug, Clone)] pub struct AppExitInfo { pub token_usage: TokenUsage, @@ -488,6 +490,27 @@ fn emit_system_bwrap_warning(app_event_tx: &AppEventSender, config: &Config) { ))); } +fn trailing_run_start(transcript_cells: &[Arc]) -> usize { + let end = transcript_cells.len(); + let mut start = end; + + while start > 0 + && transcript_cells[start - 1].is_stream_continuation() + && transcript_cells[start - 1].as_any().is::() + { + start -= 1; + } + + if start > 0 + && transcript_cells[start - 1].as_any().is::() + && !transcript_cells[start - 1].is_stream_continuation() + { + start -= 1; + } + + start +} + #[derive(Debug, Clone, PartialEq, Eq)] struct SessionSummary { usage_line: Option, @@ -962,6 +985,7 @@ pub(crate) struct App { pub(crate) overlay: Option, pub(crate) deferred_history_lines: Vec>, has_emitted_history_lines: bool, + transcript_reflow: TranscriptReflowState, pub(crate) enhanced_keys_supported: bool, @@ -3274,7 +3298,9 @@ impl App { self.overlay = None; self.transcript_cells.clear(); self.deferred_history_lines.clear(); + tui.clear_pending_history_lines(); self.has_emitted_history_lines = false; + self.transcript_reflow.clear(); self.backtrack = BacktrackState::default(); self.backtrack_render_pending = false; tui.terminal.clear_scrollback()?; @@ -3282,6 +3308,206 @@ impl App { Ok(()) } + fn reset_history_emission_state(&mut self) { + self.has_emitted_history_lines = false; + self.deferred_history_lines.clear(); + } + + fn display_lines_for_history_insert( + &mut self, + cell: &dyn HistoryCell, + width: u16, + ) -> Vec> { + let mut display = cell.display_lines(width); + if !display.is_empty() && !cell.is_stream_continuation() { + if self.has_emitted_history_lines { + display.insert(0, Line::from("")); + } else { + self.has_emitted_history_lines = true; + } + } + display + } + + fn insert_history_cell_lines( + &mut self, + tui: &mut tui::Tui, + cell: &dyn HistoryCell, + width: u16, + ) { + let display = self.display_lines_for_history_insert(cell, width); + if display.is_empty() { + return; + } + if self.overlay.is_some() { + self.deferred_history_lines.extend(display); + } else { + tui.insert_history_lines(display); + } + } + + fn schedule_resize_reflow(&mut self) -> bool { + self.transcript_reflow.schedule_debounced() + } + + /// After stream consolidation, schedule a follow-up reflow if one ran mid-stream. + fn maybe_finish_stream_reflow(&mut self, tui: &mut tui::Tui) { + if self.transcript_reflow.take_ran_during_stream() { + if self.schedule_resize_reflow() { + tui.frame_requester().schedule_frame(); + } else { + tui.frame_requester() + .schedule_frame_in(TRANSCRIPT_REFLOW_DEBOUNCE); + } + } else if self.transcript_reflow.pending_is_due(Instant::now()) { + tui.frame_requester().schedule_frame(); + } + } + + fn schedule_immediate_resize_reflow(&mut self, tui: &mut tui::Tui) { + self.transcript_reflow.schedule_immediate(); + tui.frame_requester().schedule_frame(); + } + + fn finish_required_stream_reflow(&mut self, tui: &mut tui::Tui) -> Result<()> { + self.schedule_immediate_resize_reflow(tui); + self.maybe_run_resize_reflow(tui)?; + if !self.transcript_reflow.has_pending_reflow() { + self.transcript_reflow.clear_ran_during_stream(); + } + Ok(()) + } + + fn handle_draw_size_change( + &mut self, + size: ratatui::layout::Size, + last_known_screen_size: ratatui::layout::Size, + frame_requester: &tui::FrameRequester, + ) -> bool { + let width = self.transcript_reflow.note_width(size.width); + if width.changed { + self.chat_widget.on_terminal_resize(size.width); + if self.schedule_resize_reflow() { + frame_requester.schedule_frame(); + } else { + frame_requester.schedule_frame_in(TRANSCRIPT_REFLOW_DEBOUNCE); + } + } else if width.initialized { + self.chat_widget.on_terminal_resize(size.width); + } + if size != last_known_screen_size { + self.refresh_status_line(); + } + self.maybe_clear_resize_reflow_without_terminal(); + width.changed + } + + fn maybe_clear_resize_reflow_without_terminal(&mut self) { + let Some(deadline) = self.transcript_reflow.pending_until() else { + return; + }; + if Instant::now() < deadline || self.overlay.is_some() || !self.transcript_cells.is_empty() + { + return; + } + + self.transcript_reflow.clear_pending_reflow(); + self.reset_history_emission_state(); + } + + fn handle_draw_pre_render(&mut self, tui: &mut tui::Tui) -> Result<()> { + let size = tui.terminal.size()?; + let width_changed = self.handle_draw_size_change( + size, + tui.terminal.last_known_screen_size, + &tui.frame_requester(), + ); + if width_changed { + // Width-sensitive history inserts queued before this frame may be wrapped for the old + // viewport. Drop them and let resize reflow rebuild from transcript cells. + tui.clear_pending_history_lines(); + } + self.maybe_run_resize_reflow(tui)?; + Ok(()) + } + + fn maybe_run_resize_reflow(&mut self, tui: &mut tui::Tui) -> Result<()> { + let Some(deadline) = self.transcript_reflow.pending_until() else { + return Ok(()); + }; + if Instant::now() < deadline || self.overlay.is_some() { + return Ok(()); + } + + self.transcript_reflow.clear_pending_reflow(); + + // Track that a reflow happened during an active stream or while trailing + // unconsolidated AgentMessageCells are still pending consolidation so + // ConsolidateAgentMessage can schedule a follow-up reflow. + let reflow_ran_during_stream = + !self.transcript_cells.is_empty() && self.should_mark_reflow_as_stream_time(); + + self.reflow_transcript_now(tui)?; + + if reflow_ran_during_stream { + self.transcript_reflow.mark_ran_during_stream(); + } + + Ok(()) + } + + fn reflow_transcript_now(&mut self, tui: &mut tui::Tui) -> Result<()> { + // Drop any queued pre-resize/pre-consolidation inserts before rebuilding from cells. + tui.clear_pending_history_lines(); + if self.transcript_cells.is_empty() { + self.reset_history_emission_state(); + return Ok(()); + } + + if tui.is_alt_screen_active() { + tui.terminal.clear_visible_screen()?; + } else { + tui.terminal.clear_scrollback_and_visible_screen_ansi()?; + } + + self.reset_history_emission_state(); + + let width = tui.terminal.size()?.width; + // Iterate by index to avoid cloning the Vec and bumping Arc refcounts. + for i in 0..self.transcript_cells.len() { + let cell = self.transcript_cells[i].clone(); + self.insert_history_cell_lines(tui, cell.as_ref(), width); + } + + 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(()) + } + + fn should_mark_reflow_as_stream_time(&self) -> bool { + self.chat_widget.has_active_agent_stream() + || self.chat_widget.has_active_plan_stream() + || trailing_run_start::(&self.transcript_cells) + < self.transcript_cells.len() + || trailing_run_start::(&self.transcript_cells) + < self.transcript_cells.len() + } + fn reset_thread_event_state(&mut self) { self.abort_all_thread_event_listeners(); self.thread_event_channels.clear(); @@ -3807,6 +4033,7 @@ impl App { overlay: None, deferred_history_lines: Vec::new(), has_emitted_history_lines: false, + transcript_reflow: TranscriptReflowState::default(), commit_anim_running: Arc::new(AtomicBool::new(false)), status_line_invalid_items_warned: status_line_invalid_items_warned.clone(), terminal_title_invalid_items_warned: terminal_title_invalid_items_warned.clone(), @@ -3993,10 +4220,7 @@ impl App { event: TuiEvent, ) -> Result { if matches!(event, TuiEvent::Draw) { - let size = tui.terminal.size()?; - if size != tui.terminal.last_known_screen_size { - self.refresh_status_line(); - } + self.handle_draw_pre_render(tui)?; } if self.overlay.is_some() { @@ -4306,23 +4530,89 @@ impl App { tui.frame_requester().schedule_frame(); } self.transcript_cells.push(cell.clone()); - let mut display = cell.display_lines(tui.terminal.last_known_screen_size.width); - if !display.is_empty() { - // Only insert a separating blank line for new cells that are not - // part of an ongoing stream. Streaming continuations should not - // accrue extra blank lines between chunks. - if !cell.is_stream_continuation() { - if self.has_emitted_history_lines { - display.insert(0, Line::from("")); - } else { - self.has_emitted_history_lines = true; - } + self.insert_history_cell_lines( + tui, + cell.as_ref(), + tui.terminal.last_known_screen_size.width, + ); + } + AppEvent::ConsolidateAgentMessage { + source, + cwd, + scrollback_reflow, + deferred_history_cell, + } => { + 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()); } - if self.overlay.is_some() { - self.deferred_history_lines.extend(display); - } else { - tui.insert_history_lines(display); + 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())); + + // Keep the transcript overlay in sync. + 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); + } + } + AppEvent::ConsolidateProposedPlan(source) => { + let end = self.transcript_cells.len(); + let start = trailing_run_start::( + &self.transcript_cells, + ); + let consolidated: Arc = + Arc::new(history_cell::new_proposed_plan(source, &self.config.cwd)); + + if start < end { + 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_required_stream_reflow(tui)?; + } else { + self.transcript_cells.push(consolidated.clone()); + if let Some(Overlay::Transcript(t)) = &mut self.overlay { + t.insert_cell(consolidated.clone()); + tui.frame_requester().schedule_frame(); + } + self.insert_history_cell_lines( + tui, + consolidated.as_ref(), + tui.terminal.last_known_screen_size.width, + ); + + self.maybe_finish_stream_reflow(tui); } } AppEvent::ApplyThreadRollback { num_turns } => { @@ -4351,6 +4641,7 @@ impl App { } AppEvent::CommitTick => { self.chat_widget.on_commit_tick(); + self.maybe_run_resize_reflow(tui)?; } AppEvent::Exit(mode) => { return Ok(self.handle_exit_mode(app_server, mode).await); @@ -4359,7 +4650,9 @@ impl App { return Ok(AppRunControl::Exit(ExitReason::Fatal(message))); } AppEvent::CodexOp(op) => { - self.submit_active_thread_op(app_server, op.into()).await?; + let op: AppCommand = op.into(); + self.chat_widget.prepare_local_op_submission(&op); + self.submit_active_thread_op(app_server, op).await?; } AppEvent::SubmitThreadOp { thread_id, op } => { self.submit_thread_op(app_server, thread_id, op.into()) @@ -6401,6 +6694,7 @@ mod tests { use crossterm::event::KeyModifiers; use insta::assert_snapshot; use pretty_assertions::assert_eq; + use ratatui::layout::Size; use ratatui::prelude::Line; use std::path::PathBuf; use std::sync::Arc; @@ -9217,6 +9511,43 @@ guardian_approval = true assert_snapshot!("clear_ui_header_fast_status_fast_capable_models", rendered); } + #[tokio::test] + async fn resize_reflow_repro_draw_should_drain_pending_without_commit_tick() { + let mut app = make_test_app().await; + let frame_requester = crate::tui::FrameRequester::test_dummy(); + let size = Size::new(120, 40); + + app.transcript_reflow.set_last_render_width_for_test(100); + app.handle_draw_size_change(size, size, &frame_requester); + + app.transcript_reflow.set_due_for_test(); + + app.handle_draw_size_change(size, size, &frame_requester); + + assert!( + !app.transcript_reflow.has_pending_reflow(), + "resize reflow should drain on draw even when commit animation is idle", + ); + } + + #[tokio::test] + async fn resize_reflow_repro_marks_stream_time_before_consolidation() { + let mut app = make_test_app().await; + app.transcript_cells.push(Arc::new(AgentMessageCell::new( + vec![Line::from("| Key | Value |")], + false, + ))); + + assert!( + !app.chat_widget.has_active_agent_stream(), + "repro requires stream controller to be cleared before consolidate event", + ); + assert!( + app.should_mark_reflow_as_stream_time(), + "reflow in the pre-consolidation window should still be treated as stream-time", + ); + } + async fn make_test_app() -> App { let (chat_widget, app_event_tx, _rx, _op_rx) = make_chatwidget_manual_with_sender().await; let config = chat_widget.config_ref().clone(); @@ -9240,6 +9571,7 @@ guardian_approval = true overlay: None, deferred_history_lines: Vec::new(), has_emitted_history_lines: false, + transcript_reflow: TranscriptReflowState::default(), enhanced_keys_supported: false, commit_anim_running: Arc::new(AtomicBool::new(false)), status_line_invalid_items_warned: Arc::new(AtomicBool::new(false)), @@ -9295,6 +9627,7 @@ guardian_approval = true overlay: None, deferred_history_lines: Vec::new(), has_emitted_history_lines: false, + transcript_reflow: TranscriptReflowState::default(), enhanced_keys_supported: false, commit_anim_running: Arc::new(AtomicBool::new(false)), status_line_invalid_items_warned: Arc::new(AtomicBool::new(false)), diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 78a2525350..2fd4d510fc 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -64,6 +64,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 { @@ -288,6 +294,32 @@ pub(crate) enum AppEvent { InsertHistoryCell(Box), + /// Replace the contiguous run of streaming `AgentMessageCell`s at the end of + /// the transcript with a single `AgentMarkdownCell` that stores the raw + /// markdown source and re-renders from it on resize. + /// + /// Emitted by `ChatWidget::flush_answer_stream_with_separator` after stream + /// 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 + /// end of the transcript with a single source-backed `ProposedPlanCell`. + /// + /// Emitted by `ChatWidget::on_plan_item_completed` after plan stream + /// finalization. + ConsolidateProposedPlan(String), + /// Apply rollback semantics to local transcript cells. /// /// This is emitted when rollback was not initiated by the current diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index cbb698e3a2..4d346d8879 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -15,6 +15,7 @@ //! hint. The pane schedules redraws so those hints can expire even when the UI is otherwise idle. use std::path::PathBuf; +use crate::app_event::AppEvent; use crate::app_event::ConnectorsSnapshot; use crate::app_event_sender::AppEventSender; use crate::bottom_pane::pending_input_preview::PendingInputPreview; @@ -31,6 +32,7 @@ use codex_core::plugins::PluginCapabilitySummary; use codex_core::skills::model::SkillMetadata; use codex_features::Features; use codex_file_search::FileMatch; +use codex_protocol::protocol::Op; use codex_protocol::request_user_input::RequestUserInputEvent; use codex_protocol::user_input::TextElement; use crossterm::event::KeyCode; @@ -429,10 +431,13 @@ impl BottomPane { && self.is_task_running && !is_agent_command && !self.composer.popup_active() - && let Some(status) = &self.status { - // Send Op::Interrupt - status.interrupt(); + if let Some(status) = &self.status { + // Send Op::Interrupt + status.interrupt(); + } else { + self.app_event_tx.send(AppEvent::CodexOp(Op::Interrupt)); + } self.request_redraw(); return InputResult::None; } @@ -1511,6 +1516,36 @@ mod tests { assert!(rendered.contains("background terminal running · /ps to view")); } + #[test] + fn esc_interrupts_running_task_when_status_hidden() { + let (tx_raw, mut rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let mut pane = BottomPane::new(BottomPaneParams { + app_event_tx: tx, + frame_requester: FrameRequester::test_dummy(), + has_input_focus: true, + enhanced_keys_supported: false, + placeholder_text: "Ask Codex to do anything".to_string(), + disable_paste_burst: false, + animations_enabled: true, + skills: Some(Vec::new()), + }); + + pane.set_task_running(true); + pane.hide_status_indicator(); + assert!( + !pane.status_indicator_visible(), + "status indicator must be hidden for this repro" + ); + + pane.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); + + assert!( + matches!(rx.try_recv(), Ok(AppEvent::CodexOp(Op::Interrupt))), + "expected Esc to send Op::Interrupt while status is hidden" + ); + } + #[test] fn status_with_details_and_queued_messages_snapshot() { let (tx_raw, _rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 3575799b21..e59fa9a27f 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -521,6 +521,43 @@ impl RateLimitWarningState { } } +#[cfg(test)] +fn is_interrupt_droppable_stream_event(msg: &EventMsg) -> bool { + match msg { + EventMsg::AgentMessage(_) + | EventMsg::AgentMessageDelta(_) + | EventMsg::PlanDelta(_) + | EventMsg::AgentReasoning(_) + | EventMsg::AgentReasoningDelta(_) + | EventMsg::AgentReasoningRawContent(_) + | EventMsg::AgentReasoningRawContentDelta(_) + | EventMsg::AgentReasoningSectionBreak(_) + | EventMsg::AgentMessageContentDelta(_) + | EventMsg::ReasoningContentDelta(_) + | EventMsg::ReasoningRawContentDelta(_) => true, + EventMsg::ItemCompleted(event) => matches!( + &event.item, + codex_protocol::items::TurnItem::AgentMessage(_) + | codex_protocol::items::TurnItem::Plan(_) + ), + _ => false, + } +} + +fn is_interrupt_droppable_server_notification(notification: &ServerNotification) -> bool { + match notification { + ServerNotification::AgentMessageDelta(_) + | ServerNotification::PlanDelta(_) + | ServerNotification::ReasoningSummaryTextDelta(_) + | ServerNotification::ReasoningTextDelta(_) => true, + ServerNotification::ItemCompleted(notification) => matches!( + ¬ification.item, + ThreadItem::AgentMessage { .. } | ThreadItem::Plan { .. } + ), + _ => false, + } +} + pub(crate) fn get_limits_duration(windows_minutes: i64) -> String { const MINUTES_PER_HOUR: i64 = 60; const MINUTES_PER_DAY: i64 = 24 * MINUTES_PER_HOUR; @@ -811,6 +848,11 @@ pub(crate) struct ChatWidget { /// bottom pane is treated as "running" while this is populated, even if no agent turn is /// currently executing. mcp_startup_status: Option>, + /// True once an interrupt was requested for the active turn. + /// + /// While this is set, inbound stream deltas are dropped locally so a large + /// queued backlog cannot keep rendering stale content after user interrupt. + interrupt_requested_for_turn: bool, /// Expected MCP servers for the current startup round, seeded from enabled local config. mcp_startup_expected_servers: Option>, /// After startup settles, ignore stale updates until enough notifications confirm a new round. @@ -1702,23 +1744,60 @@ impl ChatWidget { } fn flush_answer_stream_with_separator(&mut self) { - if let Some(mut controller) = self.stream_controller.take() - && let Some(cell) = controller.finalize() - { - self.add_boxed_history(cell); + 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(); + 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 + }; + if let Some(cell) = deferred_history_cell.as_ref() { + debug_assert!( + cell.as_any() + .downcast_ref::() + .is_some(), + "only agent message stream tails should be deferred for consolidation", + ); + } + // 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, + }); + } else if let Some(cell) = deferred_history_cell { + self.add_boxed_history(cell); + } } self.adaptive_chunking.reset(); + if had_stream_controller && self.stream_controllers_idle() { + self.app_event_tx.send(AppEvent::StopCommitAnimation); + } } fn stream_controllers_idle(&self) -> bool { self.stream_controller .as_ref() - .map(|controller| controller.queued_lines() == 0) + .map(|controller| controller.queued_lines() == 0 && !controller.has_live_tail()) .unwrap_or(true) && self .plan_stream_controller .as_ref() - .map(|controller| controller.queued_lines() == 0) + .map(|controller| controller.queued_lines() == 0 && !controller.has_live_tail()) .unwrap_or(true) } @@ -2210,7 +2289,7 @@ impl ChatWidget { if self.plan_stream_controller.is_none() { self.plan_stream_controller = Some(PlanStreamController::new( - self.last_rendered_width.get().map(|w| w.saturating_sub(4)), + self.current_stream_width(4), &self.config.cwd, )); } @@ -2239,18 +2318,25 @@ impl ChatWidget { self.plan_delta_buffer.clear(); self.plan_item_active = false; self.saw_plan_item_this_turn = true; - let finalized_streamed_cell = + let (finalized_streamed_cell, consolidated_plan_source) = if let Some(mut controller) = self.plan_stream_controller.take() { controller.finalize() } else { - None + (None, None) }; if let Some(cell) = finalized_streamed_cell { self.add_boxed_history(cell); // TODO: Replace streamed output with the final plan item text if plan streaming is // removed or if we need to reconcile mismatches between streamed and final content. + if let Some(source) = consolidated_plan_source { + self.app_event_tx + .send(AppEvent::ConsolidateProposedPlan(source)); + } } else if !plan_text.is_empty() { self.add_to_history(history_cell::new_proposed_plan(plan_text, &self.config.cwd)); + } else if let Some(source) = consolidated_plan_source { + self.app_event_tx + .send(AppEvent::ConsolidateProposedPlan(source)); } if should_restore_after_stream { self.pending_status_indicator_restore = true; @@ -2309,6 +2395,7 @@ impl ChatWidget { self.turn_sleep_inhibitor .set_turn_running(/*turn_running*/ true); self.saw_copy_source_this_turn = false; + self.interrupt_requested_for_turn = false; self.saw_plan_update_this_turn = false; self.saw_plan_item_this_turn = false; self.last_plan_progress = None; @@ -2363,10 +2450,15 @@ impl ChatWidget { self.saw_copy_source_this_turn = false; // 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 Some(cell) = controller.finalize() - { - self.add_boxed_history(cell); + if let Some(mut controller) = self.plan_stream_controller.take() { + let (cell, source) = controller.finalize(); + if let Some(cell) = cell { + self.add_boxed_history(cell); + } + if let Some(source) = source { + self.app_event_tx + .send(AppEvent::ConsolidateProposedPlan(source)); + } } self.flush_unified_exec_wait_streak(); if !from_replay { @@ -2398,6 +2490,7 @@ impl ChatWidget { self.agent_turn_running = false; self.turn_sleep_inhibitor .set_turn_running(/*turn_running*/ false); + self.interrupt_requested_for_turn = false; self.update_task_running_state(); self.running_commands.clear(); self.suppressed_exec_calls.clear(); @@ -2752,12 +2845,16 @@ 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(); // Reset running state and clear streaming buffers. self.agent_turn_running = false; self.turn_sleep_inhibitor .set_turn_running(/*turn_running*/ false); + self.interrupt_requested_for_turn = false; self.update_task_running_state(); self.running_commands.clear(); self.suppressed_exec_calls.clear(); @@ -4139,6 +4236,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(); @@ -4183,11 +4281,11 @@ 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 streaming agent content, 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.needs_final_message_separator && self.had_work_activity { @@ -4207,8 +4305,11 @@ impl ChatWidget { self.needs_final_message_separator = false; } self.stream_controller = Some(StreamController::new( - self.last_rendered_width.get().map(|w| w.saturating_sub(2)), + self.current_stream_width(2), &self.config.cwd, + self.config + .features + .enabled(Feature::StreamTableLiveTailReflow), )); } if let Some(controller) = self.stream_controller.as_mut() @@ -4217,9 +4318,39 @@ impl ChatWidget { self.app_event_tx.send(AppEvent::StartCommitAnimation); self.run_catch_up_commit_tick(); } + self.sync_active_stream_tail(); self.request_redraw(); } + fn current_stream_width(&self, reserved_cols: usize) -> Option { + self.last_rendered_width.get().and_then(|w| { + if w == 0 { + None + } else { + // Keep a 1-column minimum for active stream controllers so they can + // continue accepting deltas on ultra-narrow layouts. + Some(crate::width::usable_content_width(w, reserved_cols).unwrap_or(1)) + } + }) + } + + pub(crate) fn on_terminal_resize(&mut self, width: u16) { + let had_rendered_width = self.last_rendered_width.get().is_some(); + self.last_rendered_width.set(Some(width as usize)); + let stream_width = self.current_stream_width(2); + let plan_stream_width = self.current_stream_width(4); + if let Some(controller) = self.stream_controller.as_mut() { + controller.set_width(stream_width); + } + 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(); + } + } + fn worked_elapsed_from(&mut self, current_elapsed: u64) -> u64 { let baseline = match self.last_separator_elapsed_secs { Some(last) if current_elapsed < last => 0, @@ -4702,6 +4833,7 @@ impl ChatWidget { mcp_startup_status: None, last_agent_markdown: None, saw_copy_source_this_turn: false, + interrupt_requested_for_turn: false, mcp_startup_expected_servers: None, mcp_startup_ignore_updates_until_next_start: false, mcp_startup_allow_terminal_only_next_round: false, @@ -4945,7 +5077,7 @@ impl ChatWidget { return; }; let should_submit_now = - self.is_session_configured() && !self.is_plan_streaming_in_tui(); + self.is_session_configured() && !self.has_active_plan_stream(); if should_submit_now { // Submitted is emitted when user submits. // Reset any reasoning header only when we are actually submitting a turn. @@ -5604,6 +5736,16 @@ impl ChatWidget { } fn flush_active_cell(&mut self) { + if self.active_cell_is_stream_tail() { + if self.stream_controller.is_some() { + return; + } + // If stream cleanup already cleared the controller, drop the transient tail instead + // of committing preview-only content to transcript history. + self.active_cell.take(); + return; + } + if let Some(active) = self.active_cell.take() { self.needs_final_message_separator = true; self.app_event_tx.send(AppEvent::InsertHistoryCell(active)); @@ -5625,12 +5767,47 @@ 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(); + let keep_stream_tail_active = + self.stream_controller.is_some() && self.active_cell_is_stream_tail(); + if !keep_stream_tail_active { + self.flush_active_cell(); + } self.needs_final_message_separator = true; } + self.app_event_tx.send(AppEvent::InsertHistoryCell(cell)); } + fn sync_active_stream_tail(&mut self) { + let Some((tail_lines, tail_starts_stream)) = + self.stream_controller.as_mut().map(|controller| { + ( + controller.current_tail_lines(), + controller.tail_starts_stream(), + ) + }) + else { + return; + }; + + if tail_lines.is_empty() { + self.clear_active_stream_tail(); + return; + } + + self.bottom_pane.hide_status_indicator(); + let tail_cell = history_cell::StreamingAgentTailCell::new(tail_lines, tail_starts_stream); + self.active_cell = Some(Box::new(tail_cell)); + self.bump_active_cell_revision(); + } + + fn clear_active_stream_tail(&mut self) { + if self.active_cell_is_stream_tail() { + self.active_cell = None; + self.bump_active_cell_revision(); + } + } + fn queue_user_message(&mut self, user_message: UserMessage) { if !self.is_session_configured() || self.bottom_pane.is_task_running() { self.queued_user_messages.push_back(user_message); @@ -6375,6 +6552,13 @@ impl ChatWidget { if !is_resume_initial_replay && !is_retry_error { self.restore_retry_status_header_if_present(); } + if !from_replay + && self.interrupt_requested_for_turn + && is_interrupt_droppable_server_notification(¬ification) + { + tracing::trace!("dropping app-server stream notification while interrupt is pending"); + return; + } match notification { ServerNotification::ThreadTokenUsageUpdated(notification) => { self.set_token_info(Some(token_usage_info_from_app_server( @@ -6895,6 +7079,15 @@ impl ChatWidget { let from_replay = replay_kind.is_some(); let is_resume_initial_replay = matches!(replay_kind, Some(ReplayKind::ResumeInitialMessages)); + + if !from_replay + && self.interrupt_requested_for_turn + && is_interrupt_droppable_stream_event(&msg) + { + tracing::trace!("dropping stream event while interrupt is pending"); + return; + } + let is_stream_error = matches!(&msg, EventMsg::StreamError(_)); if !is_resume_initial_replay && !is_stream_error { self.restore_retry_status_header_if_present(); @@ -10440,10 +10633,22 @@ impl ChatWidget { self.bottom_pane.is_task_running() || self.is_review_mode } - fn is_plan_streaming_in_tui(&self) -> bool { + /// Whether an agent message stream is active (not a plan stream). + pub(crate) fn has_active_agent_stream(&self) -> bool { + self.stream_controller.is_some() + } + + pub(crate) fn has_active_plan_stream(&self) -> bool { self.plan_stream_controller.is_some() } + /// Whether the active cell is a transient streaming tail preview. + fn active_cell_is_stream_tail(&self) -> bool { + self.active_cell + .as_ref() + .is_some_and(|cell| cell.as_any().is::()) + } + pub(crate) fn composer_is_empty(&self) -> bool { self.bottom_pane.composer_is_empty() } @@ -10472,7 +10677,7 @@ impl ChatWidget { return; } self.set_collaboration_mask(collaboration_mode); - let should_queue = self.is_plan_streaming_in_tui(); + let should_queue = self.has_active_plan_stream(); let user_message = UserMessage { text, local_images: Vec::new(), @@ -10558,6 +10763,7 @@ impl ChatWidget { T: Into, { let op: AppCommand = op.into(); + self.prepare_local_op_submission(&op); if op.is_review() && !self.bottom_pane.is_task_running() { self.bottom_pane.set_task_running(/*running*/ true); } @@ -10576,6 +10782,22 @@ impl ChatWidget { true } + pub(crate) fn prepare_local_op_submission(&mut self, op: &AppCommand) { + if matches!(op.view(), crate::app_command::AppCommandView::Interrupt) + && self.agent_turn_running + { + self.interrupt_requested_for_turn = true; + if let Some(controller) = self.stream_controller.as_mut() { + controller.clear_queue(); + } + if let Some(controller) = self.plan_stream_controller.as_mut() { + controller.clear_queue(); + } + self.clear_active_stream_tail(); + self.request_redraw(); + } + } + #[cfg(test)] fn on_list_mcp_tools(&mut self, ev: McpListToolsResponseEvent) { self.add_to_history(history_cell::new_mcp_tools_output( diff --git a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs index 1bae5d2aa8..9348103415 100644 --- a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs +++ b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs @@ -1064,6 +1064,207 @@ async fn interrupt_restores_queued_messages_into_composer() { let _ = drain_insert_history(&mut rx); } +#[tokio::test] +async fn interrupt_drops_stream_deltas_until_turn_aborted() { + let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await; + + chat.handle_codex_event(Event { + id: "turn-1".into(), + msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), + model_context_window: None, + collaboration_mode_kind: ModeKind::Default, + started_at: None, + }), + }); + + chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL)); + let mut saw_interrupt = false; + while let Ok(op) = op_rx.try_recv() { + if matches!(op, Op::Interrupt) { + saw_interrupt = true; + break; + } + } + assert!(saw_interrupt, "expected Ctrl+C to submit Op::Interrupt"); + + // Simulate stale stream backlog that arrives before TurnAborted. + chat.stream_controller = None; + chat.handle_codex_event(Event { + id: "delta-stale".into(), + msg: EventMsg::AgentMessageDelta(AgentMessageDeltaEvent { + delta: "stale row\n".to_string(), + }), + }); + assert!( + chat.stream_controller.is_none(), + "expected stale delta to be dropped while interrupt is pending", + ); + + chat.handle_codex_event(Event { + id: "abort-1".into(), + msg: EventMsg::TurnAborted(codex_protocol::protocol::TurnAbortedEvent { + turn_id: Some("turn-1".to_string()), + reason: TurnAbortReason::Interrupted, + completed_at: None, + duration_ms: None, + }), + }); + + // A subsequent turn should stream normally. + chat.handle_codex_event(Event { + id: "turn-2".into(), + msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-2".to_string(), + model_context_window: None, + collaboration_mode_kind: ModeKind::Default, + started_at: None, + }), + }); + chat.handle_codex_event(Event { + id: "delta-fresh".into(), + msg: EventMsg::AgentMessageDelta(AgentMessageDeltaEvent { + delta: "fresh row\n".to_string(), + }), + }); + assert!( + chat.stream_controller.is_some(), + "expected new-turn delta to stream after interrupt completion", + ); + + let _ = drain_insert_history(&mut rx); +} + +#[tokio::test] +async fn app_event_interrupt_prepares_local_stream_cleanup() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + let cwd = chat.config.cwd.to_path_buf(); + let mut controller = + crate::streaming::controller::StreamController::new(Some(80), cwd.as_path(), false); + assert!(controller.push("stale backlog\n")); + + chat.agent_turn_running = true; + chat.bottom_pane.set_task_running(/*running*/ true); + chat.bottom_pane.hide_status_indicator(); + chat.stream_controller = Some(controller); + chat.active_cell = Some(Box::new(crate::history_cell::StreamingAgentTailCell::new( + vec![ratatui::text::Line::from("tail")], + true, + ))); + + chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); + + let command = loop { + match rx.try_recv() { + Ok(AppEvent::CodexOp(op)) => break AppCommand::from(op), + Ok(_) => continue, + Err(err) => panic!("expected app-level interrupt event, got {err:?}"), + } + }; + chat.prepare_local_op_submission(&command); + + assert!(chat.interrupt_requested_for_turn); + assert!( + chat.active_cell.is_none(), + "interrupt cleanup should clear the active stream tail", + ); + assert_eq!( + chat.stream_controller + .as_ref() + .map(crate::streaming::controller::StreamController::queued_lines), + Some(0), + ); +} + +#[tokio::test] +async fn interrupt_remains_responsive_during_resized_table_stream() { + let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await; + + chat.last_rendered_width.set(Some(120)); + chat.handle_codex_event(Event { + id: "turn-resize".into(), + msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-resize".to_string(), + model_context_window: None, + collaboration_mode_kind: ModeKind::Default, + started_at: None, + }), + }); + + chat.handle_codex_event(Event { + id: "table-head".into(), + msg: EventMsg::AgentMessageDelta(AgentMessageDeltaEvent { + delta: + "| Row | Initiative Summary | Extended Notes | URL |\n| --- | --- | --- | --- |\n" + .to_string(), + }), + }); + + for idx in 0..40 { + chat.handle_codex_event(Event { + id: format!("table-row-{idx}"), + msg: EventMsg::AgentMessageDelta(AgentMessageDeltaEvent { + delta: format!( + "| {idx:03} | Workstream {idx:03} provides a long narrative about scope boundaries, sequencing assumptions, contingency paths, stakeholder dependencies, and quality criteria to keep complex coordination readable under pressure. | Record {idx:03} stores extended execution commentary including risk signals, approvals, rollback conditions, evidence links, and checkpoint outcomes so auditors and new contributors can understand context without reopening old threads. | https://example.com/program/workstream-{idx:03}-detailed-governance-and-delivery-context |\n", + ), + }), + }); + chat.on_terminal_resize(if idx % 2 == 0 { 72 } else { 116 }); + chat.on_commit_tick(); + let _ = drain_insert_history(&mut rx); + } + + assert!( + chat.stream_controller.is_some(), + "expected active stream during table tail stress", + ); + + chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL)); + let mut saw_interrupt = false; + while let Ok(op) = op_rx.try_recv() { + if matches!(op, Op::Interrupt) { + saw_interrupt = true; + break; + } + } + assert!(saw_interrupt, "expected Ctrl+C to submit Op::Interrupt"); + + chat.on_terminal_resize(64); + let resized_tail = { + let controller = chat + .stream_controller + .as_mut() + .expect("expected stream controller after resize"); + lines_to_single_string(&controller.current_tail_lines()) + }; + + chat.handle_codex_event(Event { + id: "table-row-stale-after-interrupt".into(), + msg: EventMsg::AgentMessageDelta(AgentMessageDeltaEvent { + delta: "| 999 | INTERRUPT_STALE_SENTINEL | INTERRUPT_STALE_SENTINEL | https://example.com/stale |\n" + .to_string(), + }), + }); + + let tail_after_stale_delta = { + let controller = chat + .stream_controller + .as_mut() + .expect("expected stream controller after stale delta"); + lines_to_single_string(&controller.current_tail_lines()) + }; + assert_eq!( + tail_after_stale_delta, resized_tail, + "expected stale table delta to be dropped while interrupt is pending", + ); + assert!( + !tail_after_stale_delta.contains("INTERRUPT_STALE_SENTINEL"), + "stale sentinel should never reach the active stream tail", + ); + + let _ = drain_insert_history(&mut rx); +} + #[tokio::test] async fn interrupt_prepends_queued_messages_before_existing_composer_text() { let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; diff --git a/codex-rs/tui/src/chatwidget/tests/helpers.rs b/codex-rs/tui/src/chatwidget/tests/helpers.rs index 0a7deee589..2319a22cda 100644 --- a/codex-rs/tui/src/chatwidget/tests/helpers.rs +++ b/codex-rs/tui/src/chatwidget/tests/helpers.rs @@ -222,6 +222,7 @@ pub(super) async fn make_chatwidget_manual( unified_exec_processes: Vec::new(), agent_turn_running: false, mcp_startup_status: None, + interrupt_requested_for_turn: false, mcp_startup_expected_servers: None, mcp_startup_ignore_updates_until_next_start: false, mcp_startup_allow_terminal_only_next_round: false, 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 b04ce74a14..55c1d6f3b2 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs @@ -112,6 +112,305 @@ async fn turn_started_uses_runtime_context_window_before_first_token_count() { "expected /status to avoid raw config context window, got: {context_line}" ); } + +#[tokio::test] +async fn current_stream_width_clamps_to_minimum_when_reserved_columns_exhaust_width() { + let (chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + + chat.last_rendered_width.set(None); + assert_eq!(chat.current_stream_width(2), None); + + chat.last_rendered_width.set(Some(2)); + assert_eq!(chat.current_stream_width(2), Some(1)); + + chat.last_rendered_width.set(Some(4)); + assert_eq!(chat.current_stream_width(4), Some(1)); + + chat.last_rendered_width.set(Some(5)); + assert_eq!(chat.current_stream_width(4), Some(1)); +} + +#[tokio::test] +async fn on_terminal_resize_initial_width_requests_redraw() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + let (draw_tx, mut draw_rx) = tokio::sync::broadcast::channel(8); + chat.frame_requester = FrameRequester::new(draw_tx); + chat.last_rendered_width.set(None); + + chat.on_terminal_resize(120); + + let draw = tokio::time::timeout(std::time::Duration::from_millis(200), draw_rx.recv()) + .await + .expect("timed out waiting for redraw request"); + assert!(draw.is_ok(), "expected redraw notification to be sent"); +} + +#[tokio::test] +async fn add_to_history_does_not_commit_transient_stream_tail_after_controller_clear() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + + chat.active_cell = Some(Box::new(crate::history_cell::StreamingAgentTailCell::new( + vec![ratatui::text::Line::from("transient table tail preview")], + true, + ))); + // Interrupt/error cleanup paths clear the controller before appending history. + chat.stream_controller = None; + + chat.add_to_history(crate::history_cell::new_error_event( + "stream interrupted".to_string(), + )); + + let mut inserted_count = 0usize; + let mut saw_transient_tail = false; + let mut saw_error = false; + while let Ok(event) = rx.try_recv() { + if let AppEvent::InsertHistoryCell(cell) = event { + inserted_count += 1; + if cell + .as_any() + .is::() + { + saw_transient_tail = true; + } + let rendered = lines_to_single_string(&cell.display_lines(80)); + if rendered.contains("stream interrupted") { + saw_error = true; + } + } + } + + assert!(saw_error, "expected error history cell to be emitted"); + assert!( + !saw_transient_tail, + "did not expect transient stream-tail cell to be committed", + ); + assert_eq!( + inserted_count, 1, + "expected only one committed history cell after cleanup" + ); +} + +#[tokio::test] +async fn on_error_does_not_persist_transient_stream_tail_during_finalize_turn() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + + chat.stream_controller = Some(crate::streaming::controller::StreamController::new( + Some(80), + chat.config.cwd.as_path(), + false, + )); + chat.active_cell = Some(Box::new(crate::history_cell::StreamingAgentTailCell::new( + vec![ratatui::text::Line::from("transient stream tail preview")], + true, + ))); + + chat.on_error("stream failed".to_string()); + + let mut saw_transient_tail = false; + let mut saw_error = false; + while let Ok(event) = rx.try_recv() { + if let AppEvent::InsertHistoryCell(cell) = event { + if cell + .as_any() + .is::() + { + saw_transient_tail = true; + } + let rendered = lines_to_single_string(&cell.display_lines(80)); + if rendered.contains("stream failed") { + saw_error = true; + } + } + } + + assert!(saw_error, "expected error history cell to be emitted"); + assert!( + !saw_transient_tail, + "did not expect transient stream-tail cell to be committed during finalize_turn", + ); +} + +#[tokio::test] +async fn flush_answer_stream_does_not_stop_animation_while_plan_stream_is_active() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + let cwd = chat.config.cwd.to_path_buf(); + + let mut plan_controller = + crate::streaming::controller::PlanStreamController::new(Some(80), cwd.as_path()); + assert!(plan_controller.push("- Step 1\n")); + assert!( + plan_controller.queued_lines() > 0, + "expected plan stream to have queued lines for this repro", + ); + chat.plan_stream_controller = Some(plan_controller); + chat.stream_controller = Some(crate::streaming::controller::StreamController::new( + Some(80), + cwd.as_path(), + false, + )); + + while rx.try_recv().is_ok() {} + + chat.flush_answer_stream_with_separator(); + + let mut saw_stop = false; + while let Ok(event) = rx.try_recv() { + if matches!(event, AppEvent::StopCommitAnimation) { + saw_stop = true; + } + } + + assert!( + !saw_stop, + "did not expect StopCommitAnimation while plan stream still has queued lines", + ); +} + +#[tokio::test] +async fn flush_answer_stream_requests_scrollback_reflow_for_live_table_tail() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + let cwd = chat.config.cwd.to_path_buf(); + + let mut controller = + crate::streaming::controller::StreamController::new(Some(80), cwd.as_path(), false); + controller.push("| Name | Notes |\n"); + controller.push("| --- | --- |\n"); + controller.push("| alpha | FINAL_DUPLICATE_SENTINEL |\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 without provisional insert", + ); + } + _ => {} + } + } + + 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 flush_answer_stream_keeps_default_reflow_for_plain_text_tail() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + let cwd = chat.config.cwd.to_path_buf(); + + let mut controller = + crate::streaming::controller::StreamController::new(Some(80), cwd.as_path(), false); + assert!(controller.push("plain response line\n")); + assert!( + !controller.has_live_tail(), + "plain completed text should not force scrollback reflow", + ); + 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(), + "plain text should keep the normal provisional insert path", + ); + } + _ => {} + } + } + + 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_does_not_stop_animation_while_plan_table_stream_is_active() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + let cwd = chat.config.cwd.to_path_buf(); + + let mut plan_controller = + crate::streaming::controller::PlanStreamController::new(Some(80), cwd.as_path()); + assert!( + plan_controller.push("| Step | Owner |\n"), + "expected table header to enqueue while plan stream is active", + ); + assert!( + plan_controller.queued_lines() > 0, + "expected queued table header lines for this repro", + ); + chat.plan_stream_controller = Some(plan_controller); + chat.stream_controller = Some(crate::streaming::controller::StreamController::new( + Some(80), + cwd.as_path(), + false, + )); + + while rx.try_recv().is_ok() {} + + chat.flush_answer_stream_with_separator(); + + let mut saw_stop = false; + while let Ok(event) = rx.try_recv() { + if matches!(event, AppEvent::StopCommitAnimation) { + saw_stop = true; + } + } + + assert!( + !saw_stop, + "did not expect StopCommitAnimation while plan table stream is still active", + ); +} + #[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 67c7e9f98b..3ad1ddac5e 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -79,6 +79,7 @@ use ratatui::style::Modifier; use ratatui::style::Style; use ratatui::style::Styled; use ratatui::style::Stylize; +use ratatui::widgets::Clear; use ratatui::widgets::Paragraph; use ratatui::widgets::Wrap; use std::any::Any; @@ -190,6 +191,9 @@ impl Renderable for Box { .saturating_sub(usize::from(area.height)); u16::try_from(overflow).unwrap_or(u16::MAX) }; + // Active-cell content can reflow dramatically during resize/stream updates. Clear the + // entire draw area first so stale glyphs from previous frames never linger. + Clear.render(area, buf); paragraph.scroll((y, 0)).render(area, buf); } fn desired_height(&self, width: u16) -> u16 { @@ -407,7 +411,7 @@ impl ReasoningSummaryCell { let mut lines: Vec> = Vec::new(); append_markdown( &self.content, - Some((width as usize).saturating_sub(2)), + crate::width::usable_content_width_u16(width, 2), Some(self.cwd.as_path()), &mut lines, ); @@ -481,6 +485,98 @@ impl HistoryCell for AgentMessageCell { } } +/// A consolidated agent message cell that stores raw markdown source and +/// re-renders from it at any width. +/// +/// 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_agent`, producing correctly-sized tables +/// with box-drawing borders. +/// +/// Uses `prefix_lines` (not `word_wrap_lines`) so table rows with box-drawing +/// characters pass through without re-wrapping. +#[derive(Debug)] +pub(crate) struct AgentMarkdownCell { + markdown_source: String, + cwd: PathBuf, +} + +impl AgentMarkdownCell { + pub(crate) fn new(markdown_source: String, cwd: &Path) -> Self { + Self { + markdown_source, + cwd: cwd.to_path_buf(), + } + } +} + +impl HistoryCell for AgentMarkdownCell { + fn display_lines(&self, width: u16) -> Vec> { + let Some(wrap_width) = crate::width::usable_content_width_u16(width, 2) else { + return prefix_lines(vec![Line::default()], "• ".dim(), " ".into()); + }; + + 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_agent_with_cwd( + &self.markdown_source, + Some(wrap_width), + Some(self.cwd.as_path()), + &mut lines, + ); + // Use prefix_lines (not word_wrap_lines) so table rows with box-drawing characters are not + // broken by word-wrapping. The markdown renderer already output to wrap_width. + prefix_lines(lines, "• ".dim(), " ".into()) + } +} + +/// 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 or are the last incomplete line) are displayed via this cell in the +/// `active_cell` slot. It is replaced on every delta and cleared when the stream finalizes. +/// +/// Unlike `AgentMessageCell`, this cell is never committed to the transcript. It exists only as a +/// live preview of content that will eventually be emitted as stable `AgentMessageCell`s or +/// consolidated into an `AgentMarkdownCell`. +#[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 table rows. + prefix_lines( + self.lines.clone(), + if self.is_first_line { + "• ".dim() + } else { + " ".into() + }, + " ".into(), + ) + } + + fn is_stream_continuation(&self) -> bool { + !self.is_first_line + } +} + #[derive(Debug)] pub(crate) struct PlainHistoryCell { lines: Vec>, @@ -2773,6 +2869,7 @@ mod tests { use crate::exec_cell::CommandOutput; use crate::exec_cell::ExecCall; use crate::exec_cell::ExecCell; + use crate::wrapping::word_wrap_lines; use codex_config::types::McpServerConfig; use codex_config::types::McpServerDisabledReason; use codex_core::config::Config; @@ -2789,6 +2886,8 @@ mod tests { use codex_protocol::protocol::SessionConfiguredEvent; use dirs::home_dir; use pretty_assertions::assert_eq; + use ratatui::buffer::Buffer; + use ratatui::layout::Rect; use serde_json::json; use std::collections::HashMap; use std::path::PathBuf; @@ -4688,4 +4787,280 @@ mod tests { ] ); } + + #[test] + fn agent_markdown_cell_renders_table_at_different_widths() { + let source = "| Name | Role |\n|------|------|\n| Alice | Engineer |\n| Bob | Designer |\n"; + let cell = AgentMarkdownCell::new(source.to_string(), &test_cwd()); + + // At width 80 the table should render with box-drawing characters. + let lines_80 = render_lines(&cell.display_lines(80)); + assert!( + lines_80.iter().any(|l| l.contains('┌')), + "expected box-drawing table at width 80: {lines_80:?}" + ); + // Verify the "• " leader is present on the first line. + assert!( + lines_80[0].starts_with("• "), + "first line should start with bullet prefix: {:?}", + lines_80[0] + ); + + // At width 40 the table should also render correctly (re-rendered from + // source, not just word-wrapped). + let lines_40 = render_lines(&cell.display_lines(40)); + assert!( + lines_40.iter().any(|l| l.contains('┌')), + "expected box-drawing table at width 40: {lines_40:?}" + ); + + // Verify table borders are intact (not broken by naive word-wrapping). + // Every line with a box char should have matching left/right borders. + for line in &lines_40 { + let trimmed = line.trim(); + if trimmed.starts_with('│') { + assert!( + trimmed.ends_with('│'), + "table row should have matching right border: {line:?}" + ); + } + } + } + + #[test] + fn agent_markdown_cell_narrow_width_shows_prefix_only() { + let source = "| Name | Role |\n|------|------|\n| Alice | Engineer |\n"; + let cell = AgentMarkdownCell::new(source.to_string(), &test_cwd()); + + let lines = render_lines(&cell.display_lines(2)); + assert_eq!(lines, vec!["• ".to_string()]); + } + + #[test] + fn wrapped_and_prefixed_cells_handle_tiny_widths() { + let user_cell = UserHistoryCell { + message: "tiny width coverage for wrapped user history".to_string(), + text_elements: Vec::new(), + local_image_paths: Vec::new(), + remote_image_urls: Vec::new(), + }; + let agent_message_cell = AgentMessageCell::new(vec!["tiny width agent line".into()], true); + let reasoning_cell = ReasoningSummaryCell::new( + "Plan".to_string(), + "Reasoning summary content for tiny widths.".to_string(), + &test_cwd(), + false, + ); + let agent_markdown_cell = + AgentMarkdownCell::new("| A | B |\n|---|---|\n| x | y |\n".to_string(), &test_cwd()); + + for width in 1..=4 { + assert!( + !user_cell.display_lines(width).is_empty(), + "user cell should render at width {width}", + ); + assert!( + !agent_message_cell.display_lines(width).is_empty(), + "agent message cell should render at width {width}", + ); + assert!( + !reasoning_cell.display_lines(width).is_empty(), + "reasoning cell should render at width {width}", + ); + assert!( + !agent_markdown_cell.display_lines(width).is_empty(), + "agent markdown cell should render at width {width}", + ); + } + } + + #[test] + fn render_clears_area_when_cell_content_shrinks() { + let area = Rect::new(0, 0, 40, 6); + let mut buf = Buffer::empty(area); + + let first: Box = Box::new(PlainHistoryCell::new(vec![ + Line::from("STALE ROW 1"), + Line::from("STALE ROW 2"), + Line::from("STALE ROW 3"), + Line::from("STALE ROW 4"), + ])); + first.render(area, &mut buf); + + let second: Box = + Box::new(PlainHistoryCell::new(vec![Line::from("fresh")])); + second.render(area, &mut buf); + + let mut rendered_rows: Vec = Vec::new(); + for y in 0..area.height { + let mut row = String::new(); + for x in 0..area.width { + row.push_str(buf.cell((x, y)).expect("cell should exist").symbol()); + } + rendered_rows.push(row); + } + + assert!( + rendered_rows.iter().all(|row| !row.contains("STALE")), + "rendered buffer should not retain stale glyphs: {rendered_rows:?}", + ); + assert!( + rendered_rows + .first() + .is_some_and(|row| row.contains("fresh")), + "expected fresh content in first row: {rendered_rows:?}", + ); + } + + #[test] + fn agent_markdown_cell_survives_insert_history_rewrap() { + let source = "\ + | Milestone | Target Date | Outcome | Extended Context | + |-----------|-------------|---------|------------------| + | Canary Rollout | 2026-01-15 | Completed | Canary remained at limited traffic longer than planned because p95 latency briefly regressed during + cold-cache periods | + | Regional Expansion | 2026-01-29 | Completed | Expansion succeeded with stable error rates, though internal analytics lagged temporarily | + "; + let cell = AgentMarkdownCell::new(source.to_string(), &test_cwd()); + let width: u16 = 80; + let lines = cell.display_lines(width); + + // Simulate what insert_history_lines does: word_wrap_lines with + // the terminal width and no indent. + let rewrapped = word_wrap_lines(&lines, width as usize); + let before = render_lines(&lines); + let after = render_lines(&rewrapped); + assert_eq!( + before, after, + "word_wrap_lines should not alter lines that already fit within width" + ); + } + + #[test] + fn agent_markdown_cell_table_fits_within_narrow_width() { + let source = "\ + | Milestone | Target Date | Outcome | Extended Context | + |-----------|-------------|---------|------------------| + | Canary Rollout | 2026-01-15 | Completed | Canary remained at limited traffic longer than planned because p95 latency briefly regressed during + cold-cache periods | + | Regional Expansion | 2026-01-29 | Completed | Expansion succeeded with stable error rates, though internal analytics lagged temporarily | + | Legacy Decommission | 2026-02-10 | In Progress | Most legacy jobs are drained, but final shutdown is blocked by one compliance export workflow | + "; + let cell = AgentMarkdownCell::new(source.to_string(), &test_cwd()); + + // Render at a narrow width (simulating terminal resize). + let narrow_width: u16 = 80; + let lines = cell.display_lines(narrow_width); + let rendered = render_lines(&lines); + + // Every rendered line must fit within the target width. + for line in &rendered { + let display_width = unicode_width::UnicodeWidthStr::width(line.as_str()); + assert!( + display_width <= narrow_width as usize, + "line exceeds width {narrow_width}: ({display_width} chars) {line:?}" + ); + } + + // Table should still have box-drawing characters. + assert!( + rendered.iter().any(|l| l.contains('┌')), + "expected box-drawing table: {rendered:?}" + ); + } + + #[test] + fn streaming_agent_tail_cell_does_not_rewrap_table_rows() { + let source = "\ +| # | Type | Example | Details | Score |\n\ +| --- | --- | --- | --- | --- |\n\ +| 5 | URL link | Rust (https://www.rust-lang.org) | external | 93 |\n\ +| 6 | GitHub link | Repo (https://github.com/openai) | external | 89 |\n"; + let mut rendered_lines = Vec::new(); + crate::markdown::append_markdown_agent(source, Some(120), &mut rendered_lines); + let cell = StreamingAgentTailCell::new(rendered_lines, true); + let lines = render_lines(&cell.display_lines(72)); + + // Ensure wrapped spillover lines were not introduced by a second wrap pass. + for line in &lines { + let content = line.chars().skip(2).collect::(); + let trimmed = content.trim(); + if trimmed.starts_with('│') { + assert!( + trimmed.ends_with('│'), + "table row should preserve right border while streaming: {line:?}", + ); + } + } + } + + /// Simulate the consolidation backward-walk logic from `App::handle_event` + /// to verify it correctly identifies and replaces `AgentMessageCell` runs. + #[test] + fn consolidation_walker_replaces_agent_message_cells() { + use std::sync::Arc; + + // Build a transcript with: [UserCell, AgentMsg(head), AgentMsg(cont), AgentMsg(cont)] + let user = Arc::new(UserHistoryCell { + message: "hello".to_string(), + text_elements: Vec::new(), + local_image_paths: Vec::new(), + remote_image_urls: Vec::new(), + }) as Arc; + let head = Arc::new(AgentMessageCell::new(vec![Line::from("line 1")], true)) + as Arc; + let cont1 = Arc::new(AgentMessageCell::new(vec![Line::from("line 2")], false)) + as Arc; + let cont2 = Arc::new(AgentMessageCell::new(vec![Line::from("line 3")], false)) + as Arc; + + let mut transcript_cells: Vec> = + vec![user.clone(), head, cont1, cont2]; + + // Run the same consolidation logic as the handler. + let source = "line 1\nline 2\nline 3\n".to_string(); + let end = transcript_cells.len(); + let mut start = end; + while start > 0 + && transcript_cells[start - 1].is_stream_continuation() + && transcript_cells[start - 1] + .as_any() + .is::() + { + start -= 1; + } + if start > 0 + && transcript_cells[start - 1] + .as_any() + .is::() + && !transcript_cells[start - 1].is_stream_continuation() + { + start -= 1; + } + + assert_eq!( + start, 1, + "should find all 3 agent cells starting at index 1" + ); + assert_eq!(end, 4); + + // Splice. + let consolidated: Arc = + Arc::new(AgentMarkdownCell::new(source, &test_cwd())); + transcript_cells.splice(start..end, std::iter::once(consolidated)); + + assert_eq!(transcript_cells.len(), 2, "should be [user, consolidated]"); + + // Verify first cell is still the user cell. + assert!( + transcript_cells[0].as_any().is::(), + "first cell should be UserHistoryCell" + ); + + // Verify second cell is AgentMarkdownCell. + assert!( + transcript_cells[1].as_any().is::(), + "second cell should be AgentMarkdownCell" + ); + } } diff --git a/codex-rs/tui/src/insert_history.rs b/codex-rs/tui/src/insert_history.rs index 76cd699e86..cff532eacf 100644 --- a/codex-rs/tui/src/insert_history.rs +++ b/codex-rs/tui/src/insert_history.rs @@ -185,8 +185,15 @@ where // fetch/restore the cursor position. insert_history_lines should be cursor-position-neutral :) queue!(writer, MoveTo(0, cursor_top))?; + let scroll_bottom = area.top().saturating_sub(1); + let mut advance_row = cursor_top; for line in &wrapped { - queue!(writer, Print("\r\n"))?; + // Explicitly anchor before each line advance to avoid terminal wrap-pending drift when + // prior content reached the right edge. + queue!(writer, MoveTo(0, advance_row), Print("\n"))?; + if advance_row < scroll_bottom { + advance_row += 1; + } write_history_line(writer, line, wrap_width)?; } @@ -821,4 +828,58 @@ mod tests { assert_eq!(term.viewport_area, Rect::new(0, 5, width, 2)); assert_eq!(term.visible_history_rows(), 1); } + + #[test] + fn vt100_exact_width_rows_keep_stable_line_progression() { + let width: u16 = 10; + let height: u16 = 7; + let backend = VT100Backend::new(width, height); + let mut term = crate::custom_terminal::Terminal::with_options(backend).expect("terminal"); + let viewport = Rect::new(0, height - 1, width, 1); + term.set_viewport_area(viewport); + + let lines = vec![ + Line::from("1234567890"), + Line::from("abcdefghij"), + Line::from("KLMNOPQRST"), + ]; + insert_history_lines(&mut term, lines).expect("insert_history_lines should succeed"); + + let screen = term.backend().vt100().screen(); + let mut non_empty_rows: Vec<(u16, String)> = Vec::new(); + for row in 0..height.saturating_sub(1) { + let row_text = (0..width) + .map(|col| { + screen + .cell(row, col) + .map(|cell| cell.contents().to_string()) + .unwrap_or_default() + }) + .collect::(); + if !row_text.trim().is_empty() { + non_empty_rows.push((row, row_text)); + } + } + + assert_eq!( + non_empty_rows.len(), + 3, + "expected exactly three populated rows, got {non_empty_rows:?}", + ); + + let expected = ["1234567890", "abcdefghij", "KLMNOPQRST"]; + for (idx, (row, row_text)) in non_empty_rows.iter().enumerate() { + assert_eq!( + row_text, expected[idx], + "unexpected row content at y={row}: {row_text:?}", + ); + } + for pair in non_empty_rows.windows(2) { + assert_eq!( + pair[1].0, + pair[0].0 + 1, + "expected contiguous row progression, got {non_empty_rows:?}", + ); + } + } } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 700d685b0f..d5674bca35 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -152,6 +152,7 @@ mod terminal_title; mod text_formatting; mod theme_picker; mod tooltips; +mod transcript_reflow; mod tui; mod ui_consts; pub(crate) mod update_action; @@ -215,9 +216,11 @@ mod voice { pub(crate) fn clear(&self) {} } } +mod width; 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 e29246e76d..3335e3eba5 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, 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, 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, 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, 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, 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, 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 ae680f5db3..e61062f9a5 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, @@ -170,14 +357,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(), @@ -200,20 +389,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), @@ -251,7 +441,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), @@ -273,12 +463,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(_) => {} } @@ -297,18 +487,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(); } @@ -318,12 +511,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; @@ -343,11 +542,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; @@ -360,6 +565,9 @@ where } fn end_blockquote(&mut self) { + if self.in_table_cell() { + return; + } self.indent_stack.pop(); self.needs_newline = true; } @@ -369,6 +577,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()); } @@ -422,6 +635,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; @@ -435,6 +653,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 { @@ -455,6 +686,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()); } @@ -462,6 +697,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; @@ -570,6 +810,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); @@ -596,13 +1552,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 @@ -611,8 +1573,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; + } } } } @@ -636,13 +1606,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(); @@ -651,12 +1621,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 { @@ -685,13 +1679,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 { @@ -1133,4 +2131,244 @@ 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), None); + let wrapped = writer.wrap_cell(&cell, 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, 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, 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, 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, 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, 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, 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, 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")], false); + assert!(W::is_spillover_row(&row, None)); + } + + #[test] + fn spillover_keeps_single_cell_row_with_table_pipe_syntax() { + let row = make_body_row(vec![make_cell("some sparse value")], true); + assert!(!W::is_spillover_row(&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(""), + ], + false, + ); + assert!(W::is_spillover_row(&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("")], + false, + ); + let next = make_body_row( + vec![make_cell("
x
"), make_cell(""), make_cell("")], + 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("")], + false, + ); + assert!(W::is_spillover_row(&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")], + true, + ); + assert!(!W::is_spillover_row(&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("")], + true, + ); + let next = make_body_row(vec![make_cell("ok"), make_cell(""), make_cell("")], 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 850d343853..d038525197 100644 --- a/codex-rs/tui/src/markdown_render_tests.rs +++ b/codex-rs/tui/src/markdown_render_tests.rs @@ -1111,23 +1111,6 @@ fn code_block_multiple_lines_inside_unordered_list() { assert_eq!(lines, vec!["- Item", "", " first", " second"]); } -#[test] -fn code_block_inside_unordered_list_item_multiple_lines() { - let md = "- Item\n\n ```\n first\n second\n ```\n"; - let text = render_markdown_text(md); - let lines: Vec = text - .lines - .iter() - .map(|l| { - l.spans - .iter() - .map(|s| s.content.clone()) - .collect::() - }) - .collect(); - assert_eq!(lines, vec!["- Item", "", " first", " second"]); -} - #[test] fn markdown_render_complex_snapshot() { let md = r#"# H1: Markdown Streaming Test @@ -1368,3 +1351,503 @@ 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_with_emoji_cells_renders_boxed_table() { + let md = "| Task | State |\n|---|---|\n| Unit tests | ✅ |\n| Release notes | 📝 |\n"; + let text = crate::markdown_render::render_markdown_text_with_width(md, Some(80)); + 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('┌')), + "expected boxed table for emoji content: {lines:?}" + ); + assert!( + !lines.iter().any(|line| line.starts_with("|:---")), + "did not expect pipe-delimiter fallback for emoji content: {lines:?}" + ); +} + +#[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('┌'))); +} + +#[test] +fn table_pipe_fallback_rows_wrap_in_narrow_width() { + let md = "| c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 | c9 | c10 |\n|---|---|---|---|---|---|---|---|---|---|\n| 111111 | 222222 | 333333 | 444444 | 555555 | 666666 | 777777 | 888888 | 999999 | 101010 |\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.len() > 3, + "expected wrapped pipe-fallback rows at narrow width, got {lines:?}" + ); +} + +#[test] +fn table_pipe_fallback_escapes_literal_pipes_in_cell_content() { + let md = "| c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 | c9 | c10 |\n|---|---|---|---|---|---|---|---|---|---|\n| keep | keep | keep | keep | keep | keep | keep | keep | a \\| b | keep |\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("\\|")), + "expected escaped pipe marker to be preserved in wrapped fallback rows: {lines:?}" + ); +} + +#[test] +fn table_link_keeps_url_suffix_inside_cell() { + let md = "| Site |\n|---|\n| [OpenAI](https://openai.com) |\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("OpenAI (https://openai.com)")), + "expected link suffix inside table cell: {lines:?}" + ); + assert!( + !lines.iter().any(|line| line.trim() == "(https://openai.com)"), + "did not expect stray url suffix line outside table: {lines:?}" + ); +} + +#[test] +fn table_does_not_absorb_trailing_html_block_label_line() { + let md = "| Left | Center | Right |\n|:-----|:------:|------:|\n| a | b | c |\nInline HTML: sup and sub.\nHTML block:\n
inline block
\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.trim() == "HTML block:"), + "expected 'HTML block:' as plain prose line: {lines:?}" + ); + assert!( + !lines.iter().any(|line| line.contains("│ HTML block:")), + "did not expect 'HTML block:' inside table grid: {lines:?}" + ); +} + +#[test] +fn table_spillover_prose_wraps_in_narrow_width() { + let long_label = "This html spillover prose line should wrap on narrow widths to avoid clipping:"; + let md = format!( + "| Left | Center | Right |\n|:-----|:------:|------:|\n| a | b | c |\n{long_label}\n
inline block
\n" + ); + let text = crate::markdown_render::render_markdown_text_with_width(&md, Some(40)); + 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("This html spillover prose")), + "expected spillover prose to be present: {lines:?}" + ); + assert!( + !lines.iter().any(|line| line.contains(long_label)), + "did not expect spillover prose to remain as one long clipped line: {lines:?}" + ); +} + +#[test] +fn table_keeps_sparse_comparison_row_inside_grid() { + let md = "| A | B | C |\n|---|---|---|\n| x < y > z | | |\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("│ x < y > z") && line.ends_with('│')), + "expected sparse comparison row to remain inside table grid: {lines:?}" + ); + assert!( + !lines.iter().any(|line| line.trim() == "x < y > z"), + "did not expect sparse comparison row to spill outside table: {lines:?}" + ); +} + +#[test] +fn table_keeps_sparse_rows_with_empty_trailing_cells() { + let md = "| A | B | C |\n|---|---|---|\n| a | | |\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") && line.ends_with('│')), + "expected sparse row to remain inside table grid: {lines:?}" + ); + assert!( + !lines.iter().any(|line| line == "a"), + "did not expect sparse row content to spill outside the table: {lines:?}" + ); +} + +#[test] +fn table_keeps_single_cell_pipe_row_inside_grid() { + let md = "| A | B |\n|---|---|\n| value |\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("│ value") && line.ends_with('│')), + "expected single-cell pipe row to remain inside table grid: {lines:?}" + ); + assert!( + !lines.iter().any(|line| line.trim() == "value"), + "did not expect single-cell pipe row to spill outside the table: {lines:?}" + ); +} + +#[test] +fn table_keeps_single_cell_row_with_leading_pipe_inside_grid() { + let md = "| A | B |\n|---|---|\n| value\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("│ value") && line.ends_with('│')), + "expected leading-pipe sparse row to remain inside table grid: {lines:?}" + ); + assert!( + !lines.iter().any(|line| line.trim() == "value"), + "did not expect leading-pipe sparse row to spill outside the table: {lines:?}" + ); +} + +#[test] +fn table_normalizes_uneven_row_column_counts() { + let md = "| A | B | C |\n|---|---|---|\n| 1 | 2 |\n| 3 | 4 | 5 | 6 |\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.starts_with('┌')) + && lines.iter().any(|line| line.starts_with('└')), + "expected normalized uneven rows to remain in boxed table output: {lines:?}" + ); + assert!( + lines + .iter() + .any(|line| line.contains("│ 1") && line.ends_with('│')), + "expected shorter row to be padded inside grid: {lines:?}" + ); + assert!( + lines + .iter() + .any(|line| line.contains("│ 3") && line.ends_with('│')), + "expected longer row to be truncated to grid width: {lines:?}" + ); +} + +#[test] +fn table_keeps_sparse_sentence_row_inside_grid() { + let md = "| A | B | C |\n|---|---|---|\n| This is done. | | |\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("│ This is done.") && line.ends_with('│')), + "expected sparse sentence row to remain inside table grid: {lines:?}" + ); + assert!( + !lines.iter().any(|line| line.trim() == "This is done."), + "did not expect sparse sentence row to spill outside table: {lines:?}" + ); +} + +#[test] +fn table_keeps_label_only_sparse_row_inside_grid() { + let md = "| A | B | C |\n|---|---|---|\n| Status: | | |\n| ok | | |\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("│ Status:") && line.ends_with('│')), + "expected label-only sparse row to remain inside table grid: {lines:?}" + ); + assert!( + !lines.iter().any(|line| line.trim() == "Status:"), + "did not expect label-only sparse row to spill outside table: {lines:?}" + ); +} + +#[test] +fn table_keeps_single_word_label_row_at_end_inside_grid() { + let md = "| A | B | C |\n|---|---|---|\n| Status: | | |\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("│ Status:") && line.ends_with('│')), + "expected single-word trailing label row to remain inside table grid: {lines:?}" + ); + assert!( + !lines.iter().any(|line| line.trim() == "Status:"), + "did not expect single-word trailing label row to spill outside table: {lines:?}" + ); +} + +#[test] +fn table_keeps_multi_word_label_row_at_end_inside_grid() { + let md = "| A | B | C |\n|---|---|---|\n| Build status: | | |\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("│ Build status:") && line.ends_with('│')), + "expected multi-word trailing label row to remain inside table grid: {lines:?}" + ); + assert!( + !lines.iter().any(|line| line.trim() == "Build status:"), + "did not expect multi-word trailing label row to spill outside table: {lines:?}" + ); +} + +#[test] +fn table_preserves_structured_leading_columns_when_last_column_is_long() { + let md = "| Milestone | Planned Date | Outcome | Retrospective Summary |\n|---|---|---|---|\n| Canary rollout | 2026-01-10 | Completed | Canary + traffic was held at 5% longer than planned due to latency regressions tied to cold cache behavior; after pre-warming and query plan hints, p95 + returned to baseline and rollout resumed safely. |\n| Full region cutover | 2026-01-24 | Completed | Cutover succeeded with no customer-visible + downtime, though internal dashboards lagged for approximately 18 minutes because ingestion workers autoscaled slower than forecast under burst load. + |\n"; + let text = crate::markdown_render::render_markdown_text_with_width(md, Some(160)); + 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("Milestone")), + "expected first structured header to remain readable: {lines:?}" + ); + assert!( + lines.iter().any(|line| line.contains("Planned Date")), + "expected date header to remain readable: {lines:?}" + ); + assert!( + lines.iter().any(|line| line.contains("2026-01-10")), + "expected date values to avoid forced mid-token wraps: {lines:?}" + ); +} + +#[test] +fn table_preserves_status_column_with_long_notes() { + let md = "| Service | Status | Notes |\n|---|---|---|\n| Auth API | Stable | Handles login and token refresh with no major incidents in the last + 30 days. |\n| Billing Worker | Monitoring | Throughput is good, but we still see occasional retry storms when upstream settlement providers return + partial failures. |\n| Search Indexer | Tuning | Performance improved after shard balancing, yet memory usage remains elevated during full rebuild + windows. |\n"; + let text = crate::markdown_render::render_markdown_text_with_width(md, Some(150)); + 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("Status")), + "expected status header to remain readable: {lines:?}" + ); + assert!( + lines.iter().any(|line| line.contains("Monitoring")), + "expected status values to avoid mid-word wraps: {lines:?}" + ); +} + +#[test] +fn table_keeps_long_body_rows_inside_grid_instead_of_spilling_raw_pipe_rows() { + let md = "| Milestone | Planned Date | Outcome | Retrospective Summary |\n|---|---|---|---|\n| Canary rollout | 2026-01-10 | Completed | Canary + traffic was held at 5% longer than planned due to latency regressions tied to cold cache behavior; after pre-warming and query plan hints, p95 + returned to baseline and rollout resumed safely. |\n| Full region cutover | 2026-01-24 | Completed | Cutover succeeded with no customer-visible + downtime, though internal dashboards lagged for approximately 18 minutes because ingestion workers autoscaled slower than forecast under burst load. + |\n| Legacy decommission | 2026-02-07 | In progress | Most workloads have been drained, but final decommission is blocked by one compliance export + task that still depends on a deprecated storage path and requires legal sign-off before removal. |\n"; + let text = crate::markdown_render::render_markdown_text_with_width(md, Some(200)); + let lines: Vec = text + .lines + .iter() + .map(|line| line.spans.iter().map(|span| span.content.clone()).collect()) + .collect(); + + assert!( + lines.iter().any(|line| line.starts_with('┌')) + && lines.iter().any(|line| line.starts_with('└')), + "expected boxed table output: {lines:?}" + ); + assert!( + lines.iter().any(|line| line.contains("│ Canary rollout")), + "expected first body row to stay inside table grid: {lines:?}" + ); + assert!( + !lines + .iter() + .any(|line| line.trim_start().starts_with("| Canary rollout |")), + "did not expect raw pipe-form body rows outside table: {lines:?}" + ); +} diff --git a/codex-rs/tui/src/markdown_stream.rs b/codex-rs/tui/src/markdown_stream.rs index 3eb37e345e..0dd0c39c50 100644 --- a/codex-rs/tui/src/markdown_stream.rs +++ b/codex-rs/tui/src/markdown_stream.rs @@ -1,55 +1,132 @@ +//! Newline-gated streaming accumulator for markdown source. +//! +//! `MarkdownStreamCollector` buffers incoming token deltas and exposes a commit boundary at each +//! newline. The stream controllers (`streaming/controller.rs`) call `commit_complete_source()` +//! after each newline-bearing delta to obtain the completed prefix for re-rendering, leaving the +//! trailing incomplete line in the buffer for the next delta. +//! +//! On finalization, `finalize_and_drain_source()` flushes whatever remains (the last line, which +//! may lack a trailing newline). + +#[cfg(test)] use ratatui::text::Line; use std::path::Path; +#[cfg(test)] use std::path::PathBuf; +#[cfg(test)] use crate::markdown; -/// Newline-gated accumulator that renders markdown and commits only fully -/// completed logical lines. +/// Newline-gated accumulator that buffers raw markdown source and commits only completed lines +/// (terminated by `\n`). +/// +/// The buffer tracks how many source bytes have already been committed via +/// `committed_source_len`, so each `commit_complete_source()` call returns only the newly +/// completed portion. This design lets the stream controller re-render the entire accumulated +/// source while only appending new content. pub(crate) struct MarkdownStreamCollector { buffer: String, + committed_source_len: usize, + #[cfg(test)] committed_line_count: usize, width: Option, + #[cfg(test)] cwd: PathBuf, } impl MarkdownStreamCollector { - /// Create a collector that renders markdown using `cwd` for local file-link display. + /// Create a collector that accumulates raw markdown deltas. /// - /// The collector snapshots `cwd` into owned state because stream commits can happen long after - /// construction. The same `cwd` should be reused for the entire stream lifecycle; mixing - /// different working directories within one stream would make the same link render with - /// different path prefixes across incremental commits. + /// `width` and `cwd` are only used by test-only rendering helpers; production stream commits + /// operate on raw source boundaries. The collector snapshots `cwd` so test rendering keeps + /// local file-link display stable across incremental commits. pub fn new(width: Option, cwd: &Path) -> Self { + #[cfg(not(test))] + let _ = cwd; + Self { buffer: String::new(), + committed_source_len: 0, + #[cfg(test)] committed_line_count: 0, width, + #[cfg(test)] cwd: cwd.to_path_buf(), } } - pub fn clear(&mut self) { - self.buffer.clear(); - self.committed_line_count = 0; + /// Update the rendering width used by test-only line-commit helpers. + pub fn set_width(&mut self, width: Option) { + self.width = width; } + /// Reset all buffered source and commit bookkeeping. + pub fn clear(&mut self) { + self.buffer.clear(); + self.committed_source_len = 0; + #[cfg(test)] + { + self.committed_line_count = 0; + } + } + + /// Append a raw streaming delta to the internal source buffer. pub fn push_delta(&mut self, delta: &str) { tracing::trace!("push_delta: {delta:?}"); self.buffer.push_str(delta); } + /// Commit newly completed raw markdown source up to the last newline. + pub fn commit_complete_source(&mut self) -> Option { + let commit_end = self.buffer.rfind('\n').map(|idx| idx + 1)?; + if commit_end <= self.committed_source_len { + return None; + } + + let out = self.buffer[self.committed_source_len..commit_end].to_string(); + self.committed_source_len = commit_end; + Some(out) + } + + /// Peek at uncommitted source content beyond the latest commit boundary. + pub fn peek_uncommitted(&self) -> &str { + &self.buffer[self.committed_source_len..] + } + + /// Finalize the stream and return any remaining raw source. + /// + /// Ensures the returned source chunk is newline-terminated when non-empty so callers can + /// safely run markdown block parsing on the final chunk. + pub fn finalize_and_drain_source(&mut self) -> String { + if self.committed_source_len >= self.buffer.len() { + self.clear(); + return String::new(); + } + + let mut out = self.buffer[self.committed_source_len..].to_string(); + if !out.ends_with('\n') { + out.push('\n'); + } + self.clear(); + out + } + /// Render the full buffer and return only the newly completed logical lines /// since the last commit. When the buffer does not end with a newline, the /// final rendered line is considered incomplete and is not emitted. + /// + /// This helper intentionally uses `append_markdown` (not + /// `append_markdown_agent`) so tests can isolate collector newline boundary + /// behavior without stream-controller holdback semantics. + #[cfg(test)] pub fn commit_complete_lines(&mut self) -> Vec> { - let source = self.buffer.clone(); - let last_newline_idx = source.rfind('\n'); - let source = if let Some(last_newline_idx) = last_newline_idx { - source[..=last_newline_idx].to_string() - } else { + let Some(commit_end) = self.buffer.rfind('\n').map(|idx| idx + 1) else { return Vec::new(); }; + if commit_end <= self.committed_source_len { + return Vec::new(); + } + let source = self.buffer[..commit_end].to_string(); let mut rendered: Vec> = Vec::new(); markdown::append_markdown(&source, self.width, Some(self.cwd.as_path()), &mut rendered); let mut complete_line_count = rendered.len(); @@ -68,25 +145,29 @@ impl MarkdownStreamCollector { let out_slice = &rendered[self.committed_line_count..complete_line_count]; let out = out_slice.to_vec(); + self.committed_source_len = commit_end; self.committed_line_count = complete_line_count; out } /// Finalize the stream: emit all remaining lines beyond the last commit. /// If the buffer does not end with a newline, a temporary one is appended - /// for rendering. Optionally unwraps ```markdown language fences in - /// non-test builds. + /// for rendering. + #[cfg(test)] pub fn finalize_and_drain(&mut self) -> Vec> { - let raw_buffer = self.buffer.clone(); - let mut source: String = raw_buffer.clone(); + let mut source = self.buffer.clone(); + if source.is_empty() { + self.clear(); + return Vec::new(); + } if !source.ends_with('\n') { source.push('\n'); - } + }; tracing::debug!( - raw_len = raw_buffer.len(), + raw_len = self.buffer.len(), source_len = source.len(), "markdown finalize (raw length: {}, rendered length: {})", - raw_buffer.len(), + self.buffer.len(), source.len() ); tracing::trace!("markdown finalize (raw source):\n---\n{source}\n---"); @@ -156,6 +237,21 @@ mod tests { assert_eq!(out.len(), 1); } + #[tokio::test] + async fn peek_uncommitted_tracks_buffer_after_commits() { + let mut c = super::MarkdownStreamCollector::new(None, &super::test_cwd()); + + c.push_delta("alpha"); + assert_eq!(c.peek_uncommitted(), "alpha"); + + c.push_delta("\n"); + assert_eq!(c.commit_complete_source(), Some("alpha\n".to_string())); + assert_eq!(c.peek_uncommitted(), ""); + + c.push_delta("beta"); + assert_eq!(c.peek_uncommitted(), "beta"); + } + #[tokio::test] async fn e2e_stream_blockquote_simple_is_green() { let out = super::simulate_stream_markdown_for_tests(&["> Hello\n"], /*finalize*/ true); @@ -416,6 +512,42 @@ mod tests { .collect() } + #[tokio::test] + async fn table_header_commits_without_holdback() { + let mut c = super::MarkdownStreamCollector::new(None, &super::test_cwd()); + c.push_delta("| A | B |\n"); + let out1 = c.commit_complete_lines(); + let out1_str = lines_to_plain_strings(&out1); + assert_eq!(out1_str, vec!["| A | B |".to_string()]); + + c.push_delta("| --- | --- |\n"); + let out = c.commit_complete_lines(); + let out_str = lines_to_plain_strings(&out); + assert!( + !out_str.is_empty(), + "expected output to continue committing after delimiter: {out_str:?}" + ); + + c.push_delta("| 1 | 2 |\n"); + let out2 = c.commit_complete_lines(); + assert!( + !out2.is_empty(), + "expected output to continue committing after body row" + ); + + c.push_delta("\n"); + let _ = c.commit_complete_lines(); + } + + #[tokio::test] + async fn pipe_text_without_table_prefix_is_not_delayed() { + let mut c = super::MarkdownStreamCollector::new(None, &super::test_cwd()); + c.push_delta("Escaped pipe in text: a | b | c\n"); + let out = c.commit_complete_lines(); + let out_str = lines_to_plain_strings(&out); + assert_eq!(out_str, vec!["Escaped pipe in text: a | b | c".to_string()]); + } + #[tokio::test] async fn lists_and_fences_commit_without_duplication() { // List case @@ -722,4 +854,45 @@ mod tests { ]) .await; } + + #[tokio::test] + 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(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, 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/pager_overlay.rs b/codex-rs/tui/src/pager_overlay.rs index bca5f1f360..e00e45c5c0 100644 --- a/codex-rs/tui/src/pager_overlay.rs +++ b/codex-rs/tui/src/pager_overlay.rs @@ -566,6 +566,49 @@ impl TranscriptOverlay { } } + /// Replace a range of committed cells with a single consolidated cell. + /// + /// Mirrors the splice performed on `App::transcript_cells` during + /// `ConsolidateAgentMessage` so the Ctrl+T overlay stays in sync with the + /// main transcript. The range is clamped defensively: cells may have been + /// inserted after the overlay opened, leaving it with fewer entries than + /// the main transcript. + pub(crate) fn consolidate_cells( + &mut self, + range: std::ops::Range, + consolidated: Arc, + ) { + let follow_bottom = self.view.is_scrolled_to_bottom(); + // Clamp the range to the overlay's cell count to avoid panic if the overlay has fewer + // cells than the main transcript (e.g. cells were inserted after the overlay has opened). + let clamped_end = range.end.min(self.cells.len()); + let clamped_start = range.start.min(clamped_end); + if clamped_start < clamped_end { + let removed = clamped_end - clamped_start; + if let Some(highlight_cell) = self.highlight_cell.as_mut() + && *highlight_cell >= clamped_start + { + if *highlight_cell < clamped_end { + *highlight_cell = clamped_start; + } else { + *highlight_cell = highlight_cell.saturating_sub(removed.saturating_sub(1)); + } + } + self.cells + .splice(clamped_start..clamped_end, std::iter::once(consolidated)); + if self + .highlight_cell + .is_some_and(|highlight_cell| highlight_cell >= self.cells.len()) + { + self.highlight_cell = None; + } + self.rebuild_renderables(); + } + if follow_bottom { + self.view.scroll_offset = usize::MAX; + } + } + /// Sync the active-cell live tail with the current width and cell state. /// /// Recomputes the tail only when the cache key changes, preserving scroll @@ -1090,6 +1133,60 @@ mod tests { assert_eq!(overlay.view.scroll_offset, 0); } + #[test] + fn transcript_overlay_consolidation_remaps_highlight_inside_range() { + let mut overlay = TranscriptOverlay::new( + (0..6) + .map(|i| { + Arc::new(TestCell { + lines: vec![Line::from(format!("line{i}"))], + }) as Arc + }) + .collect(), + ); + overlay.set_highlight_cell(Some(3)); + + overlay.consolidate_cells( + 2..5, + Arc::new(TestCell { + lines: vec![Line::from("consolidated")], + }), + ); + + assert_eq!( + overlay.highlight_cell, + Some(2), + "highlight inside consolidated range should point to replacement cell", + ); + } + + #[test] + fn transcript_overlay_consolidation_remaps_highlight_after_range() { + let mut overlay = TranscriptOverlay::new( + (0..7) + .map(|i| { + Arc::new(TestCell { + lines: vec![Line::from(format!("line{i}"))], + }) as Arc + }) + .collect(), + ); + overlay.set_highlight_cell(Some(6)); + + overlay.consolidate_cells( + 2..5, + Arc::new(TestCell { + lines: vec![Line::from("consolidated")], + }), + ); + + assert_eq!( + overlay.highlight_cell, + Some(4), + "highlight after consolidated range should shift left by removed cells", + ); + } + #[test] fn static_overlay_snapshot_basic() { // Prepare a static overlay with a few lines and a title diff --git a/codex-rs/tui/src/render/line_utils.rs b/codex-rs/tui/src/render/line_utils.rs index 175b79b2a8..54970f4486 100644 --- a/codex-rs/tui/src/render/line_utils.rs +++ b/codex-rs/tui/src/render/line_utils.rs @@ -26,6 +26,7 @@ pub fn push_owned_lines<'a>(src: &[Line<'a>], out: &mut Vec>) { /// Consider a line blank if it has no spans or only spans whose contents are /// empty or consist solely of spaces (no tabs/newlines). +#[cfg(test)] pub fn is_blank_line_spaces_only(line: &Line<'_>) -> bool { if line.spans.is_empty() { return true; 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 cc752dd66d..34cbd1c63e 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 d524e707a6..8f2f957e25 100644 --- a/codex-rs/tui/src/streaming/controller.rs +++ b/codex-rs/tui/src/streaming/controller.rs @@ -1,20 +1,527 @@ +//! Two-region streaming controllers for agent messages and proposed plans. +//! +//! 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 `StreamingAgentTailCell`). +//! +//! `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 streams and disabled for proposed-plan streams so plan output +//! continues to stream incrementally. 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::{self}; +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; use ratatui::text::Line; use std::path::Path; +use std::path::PathBuf; 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; -/// Controller that manages newline-gated streaming, header emission, and -/// commit animation across streams. -pub(crate) struct StreamController { +const SPECULATIVE_TAIL_RENDER_THROTTLE: Duration = Duration::from_millis(16); + +// --------------------------------------------------------------------------- +// 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. +/// +/// The wrapping controllers add only their own `emit()` styling and +/// finalize return types. +struct StreamCore { state: StreamState, - finishing_after_drain: bool, + /// Whether table holdback is active for this stream type. + table_holdback_mode: TableHoldbackMode, + /// Whether speculative live table-tail reflow is enabled. + live_tail_reflow_enabled: bool, + /// 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>, + /// 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, + /// Cached rendered line count for prefix-before-table keyed by source start and width. + stable_prefix_len_cache: Option, + /// Cache for speculative table-tail rendering (raw + uncommitted source). + speculative_tail_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, +} + +struct SpeculativeTailCache { + raw_source_len: usize, + uncommitted_len: usize, + enqueued_stable_len: usize, + width: Option, + lines: Vec>, + rendered_at: Instant, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum TableHoldbackMode { + Enabled, + Disabled, +} + +impl StreamCore { + fn new( + width: Option, + table_holdback_mode: TableHoldbackMode, + cwd: &Path, + live_tail_reflow_enabled: bool, + ) -> Self { + Self { + state: StreamState::new(width, cwd), + table_holdback_mode, + live_tail_reflow_enabled, + width, + raw_source: String::with_capacity(1024), + rendered_lines: Vec::with_capacity(64), + enqueued_stable_len: 0, + emitted_stable_len: 0, + cwd: cwd.to_path_buf(), + stable_prefix_len_cache: None, + speculative_tail_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.speculative_tail_cache = None; + } + 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); + if self.table_holdback_mode == TableHoldbackMode::Enabled { + self.holdback_scanner.push_source_chunk(&committed_source); + } + self.recompute_streaming_render(); + enqueued = self.sync_stable_queue(); + } + 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); + if self.table_holdback_mode == TableHoldbackMode::Enabled { + self.holdback_scanner.push_source_chunk(&remainder_source); + } + } + let mut rendered = Vec::new(); + append_markdown_agent_with_cwd( + &self.raw_source, + self.width, + Some(self.cwd.as_path()), + &mut rendered, + ); + if self.emitted_stable_len >= rendered.len() { + Vec::new() + } else { + 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_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); + 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) + } + + /// 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() + } + + fn current_tail_lines_with_partial(&mut self) -> Vec> { + if !self.can_speculatively_render_table_tail() { + self.speculative_tail_cache = None; + return self.current_tail_lines(); + } + + let raw_source_len = self.raw_source.len(); + let uncommitted_len = self.state.collector.peek_uncommitted().len(); + let enqueued_stable_len = self.enqueued_stable_len; + let width = self.width; + + if let Some(cache) = &self.speculative_tail_cache + && cache.raw_source_len == raw_source_len + && cache.uncommitted_len == uncommitted_len + && cache.enqueued_stable_len == enqueued_stable_len + && cache.width == width + { + return cache.lines.clone(); + } + + if let Some(cache) = &self.speculative_tail_cache + && cache.rendered_at.elapsed() < SPECULATIVE_TAIL_RENDER_THROTTLE + { + return cache.lines.clone(); + } + + let mut speculative_source = String::with_capacity(raw_source_len + uncommitted_len); + speculative_source.push_str(&self.raw_source); + speculative_source.push_str(self.state.collector.peek_uncommitted()); + + let mut speculative_rendered = Vec::new(); + append_markdown_agent_with_cwd( + &speculative_source, + self.width, + Some(self.cwd.as_path()), + &mut speculative_rendered, + ); + let start = enqueued_stable_len.min(speculative_rendered.len()); + let lines = speculative_rendered[start..].to_vec(); + + self.speculative_tail_cache = Some(SpeculativeTailCache { + raw_source_len, + uncommitted_len, + enqueued_stable_len, + width, + lines: lines.clone(), + rendered_at: Instant::now(), + }); + + lines + } + + #[inline] + fn has_tail(&self) -> bool { + self.enqueued_stable_len < self.rendered_lines.len() + } + + #[inline] + fn has_tail_with_partial(&self) -> bool { + if self.has_tail() { + return true; + } + + if !self.can_speculatively_render_table_tail() { + return false; + } + + if let Some(cache) = &self.speculative_tail_cache + && cache.raw_source_len == self.raw_source.len() + && cache.uncommitted_len == self.state.collector.peek_uncommitted().len() + && cache.enqueued_stable_len == self.enqueued_stable_len + && cache.width == self.width + { + return !cache.lines.is_empty(); + } + + if let Some(cache) = &self.speculative_tail_cache + && cache.rendered_at.elapsed() < SPECULATIVE_TAIL_RENDER_THROTTLE + { + return !cache.lines.is_empty(); + } + + true + } + + #[inline] + fn can_speculatively_render_table_tail(&self) -> bool { + self.live_tail_reflow_enabled + && self.table_holdback_mode == TableHoldbackMode::Enabled + && !self.state.collector.peek_uncommitted().is_empty() + && !matches!(self.holdback_scanner.state(), TableHoldbackState::None) + } + + /// 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.speculative_tail_cache = None; + self.state.collector.set_width(width); + if self.raw_source.is_empty() { + return; + } + + 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 + { + // 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_stable_len -= 1; + } + self.state.clear_queue(); + 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_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.speculative_tail_cache = None; + self.holdback_scanner.reset(); + } + + /// Re-render the full `raw_source` at current `width`. + fn recompute_streaming_render(&mut self) { + self.rendered_lines.clear(); + self.speculative_tail_cache = None; + append_markdown_agent_with_cwd( + &self.raw_source, + self.width, + Some(self.cwd.as_path()), + &mut self.rendered_lines, + ); + } + + /// 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) + } + + /// 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(), + ); + } + self.enqueued_stable_len = target_stable_len; + return self.state.queued_len() > 0; + } + + if target_stable_len == self.enqueued_stable_len { + return false; + } + + self.state + .enqueue(self.rendered_lines[self.enqueued_stable_len..target_stable_len].to_vec()); + self.enqueued_stable_len = target_stable_len; + true + } + + /// 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(); + if self.emitted_stable_len < target_stable_len { + self.state + .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.table_holdback_mode == TableHoldbackMode::Disabled { + 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(); + } + 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 + } +} + +/// Controller for streaming agent message content with table-aware holdback. +/// +/// Wraps [`StreamCore`] and adds `AgentMessageCell` emission styling. +pub(crate) struct StreamController { + core: StreamCore, header_emitted: bool, } @@ -23,81 +530,85 @@ impl StreamController { /// /// The controller snapshots the path into stream state so later commit ticks and finalization /// render against the same session cwd that was active when streaming started. - pub(crate) fn new(width: Option, cwd: &Path) -> Self { + pub(crate) fn new(width: Option, cwd: &Path, live_tail_reflow_enabled: bool) -> Self { Self { - state: StreamState::new(width, cwd), - finishing_after_drain: false, + core: StreamCore::new( + width, + TableHoldbackMode::Enabled, + cwd, + live_tail_reflow_enabled, + ), header_emitted: false, } } - /// Push a delta; if it contains a newline, commit completed lines and start animation. pub(crate) fn push(&mut self, delta: &str) -> bool { - let state = &mut self.state; - if !delta.is_empty() { - state.has_seen_delta = true; - } - state.collector.push_delta(delta); - if delta.contains('\n') { - let newly_completed = state.collector.commit_complete_lines(); - if !newly_completed.is_empty() { - state.enqueue(newly_completed); - return true; - } - } - false + self.core.push_delta(delta) } - /// Finalize the active stream. Drain and emit now. - pub(crate) fn finalize(&mut self) -> Option> { - // Finalize collector first. - let remaining = { - let state = &mut self.state; - state.collector.finalize_and_drain() - }; - // Collect all output first to avoid emitting headers when there is no content. - let mut out_lines = Vec::new(); - { - let state = &mut self.state; - if !remaining.is_empty() { - state.enqueue(remaining); - } - let step = state.drain_all(); - out_lines.extend(step); + /// 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() { + self.core.reset(); + return (None, None); } - // Cleanup - self.state.clear(); - self.finishing_after_drain = false; - self.emit(out_lines) + // 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(); + (out, Some(source)) } - /// Step animation: commit at most one queued line and handle end-of-drain cleanup. pub(crate) fn on_commit_tick(&mut self) -> (Option>, bool) { - let step = self.state.step(); - (self.emit(step), self.state.is_idle()) + let step = self.core.tick(); + (self.emit(step), self.core.is_idle()) } - /// Step animation: commit at most `max_lines` queued lines. - /// - /// This is intended for adaptive catch-up drains. Callers should keep `max_lines` bounded; a - /// very large value can collapse perceived animation into a single jump. pub(crate) fn on_commit_tick_batch( &mut self, max_lines: usize, ) -> (Option>, bool) { - let step = self.state.drain_n(max_lines.max(1)); - (self.emit(step), self.state.is_idle()) + let step = self.core.tick_batch(max_lines); + (self.emit(step), self.core.is_idle()) } - /// Returns the current number of queued lines waiting to be displayed. + // Thin StreamController accessors inlined — one-liner delegates called + // on every render frame and animation tick. + + #[inline] pub(crate) fn queued_lines(&self) -> usize { - self.state.queued_len() + self.core.queued_lines() } - /// Returns the age of the oldest queued line. pub(crate) fn oldest_queued_age(&self, now: Instant) -> Option { - self.state.oldest_queued_age(now) + self.core.oldest_queued_age(now) + } + + #[inline] + pub(crate) fn current_tail_lines(&mut self) -> Vec> { + self.core.current_tail_lines_with_partial() + } + + #[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_with_partial() + } + + pub(crate) fn clear_queue(&mut self) { + self.core.state.clear_queue(); + self.core.enqueued_stable_len = self.core.emitted_stable_len; + } + + pub(crate) fn set_width(&mut self, width: Option) { + self.core.set_width(width); } fn emit(&mut self, lines: Vec>) -> Option> { @@ -111,10 +622,16 @@ impl StreamController { }))) } } +// --------------------------------------------------------------------------- +// PlanStreamController — proposed plan streams +// --------------------------------------------------------------------------- /// Controller that streams proposed plan markdown into a styled plan block. +/// +/// Wraps [`StreamCore`] and adds plan-specific header, indentation, and +/// background styling. pub(crate) struct PlanStreamController { - state: StreamState, + core: StreamCore, header_emitted: bool, top_padding_emitted: bool, } @@ -127,81 +644,66 @@ impl PlanStreamController { /// render against the same session cwd that was active when streaming started. pub(crate) fn new(width: Option, cwd: &Path) -> Self { Self { - state: StreamState::new(width, cwd), + core: StreamCore::new(width, TableHoldbackMode::Disabled, cwd, false), header_emitted: false, top_padding_emitted: false, } } - /// Push a delta; if it contains a newline, commit completed lines and start animation. pub(crate) fn push(&mut self, delta: &str) -> bool { - let state = &mut self.state; - if !delta.is_empty() { - state.has_seen_delta = true; - } - state.collector.push_delta(delta); - if delta.contains('\n') { - let newly_completed = state.collector.commit_complete_lines(); - if !newly_completed.is_empty() { - state.enqueue(newly_completed); - return true; - } - } - false + self.core.push_delta(delta) } - /// Finalize the active stream. Drain and emit now. - pub(crate) fn finalize(&mut self) -> Option> { - let remaining = { - let state = &mut self.state; - state.collector.finalize_and_drain() - }; - let mut out_lines = Vec::new(); - { - let state = &mut self.state; - if !remaining.is_empty() { - state.enqueue(remaining); - } - let step = state.drain_all(); - out_lines.extend(step); + /// 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() { + self.core.reset(); + return (None, None); } - self.state.clear(); - self.emit(out_lines, /*include_bottom_padding*/ true) + // Move ownership — source is consumed before reset() clears it. + let source = std::mem::take(&mut self.core.raw_source); + let out = self.emit(remaining, true); + self.core.reset(); + (out, Some(source)) } - /// Step animation: commit at most one queued line and handle end-of-drain cleanup. pub(crate) fn on_commit_tick(&mut self) -> (Option>, bool) { - let step = self.state.step(); - ( - self.emit(step, /*include_bottom_padding*/ false), - self.state.is_idle(), - ) + let step = self.core.tick(); + (self.emit(step, false), self.core.is_idle()) } - /// Step animation: commit at most `max_lines` queued lines. - /// - /// This is intended for adaptive catch-up drains. Callers should keep `max_lines` bounded; a - /// very large value can collapse perceived animation into a single jump. pub(crate) fn on_commit_tick_batch( &mut self, max_lines: usize, ) -> (Option>, bool) { - let step = self.state.drain_n(max_lines.max(1)); - ( - self.emit(step, /*include_bottom_padding*/ false), - self.state.is_idle(), - ) + let step = self.core.tick_batch(max_lines); + (self.emit(step, false), self.core.is_idle()) } - /// Returns the current number of queued plan lines waiting to be displayed. + #[inline] pub(crate) fn queued_lines(&self) -> usize { - self.state.queued_len() + self.core.queued_lines() + } + + #[inline] + pub(crate) fn has_live_tail(&self) -> bool { + self.core.has_tail() } - /// Returns the age of the oldest queued plan line. pub(crate) fn oldest_queued_age(&self, now: Instant) -> Option { - self.state.oldest_queued_age(now) + self.core.oldest_queued_age(now) + } + + pub(crate) fn clear_queue(&mut self) { + self.core.state.clear_queue(); + self.core.enqueued_stable_len = self.core.emitted_stable_len; + } + + pub(crate) fn set_width(&mut self, width: Option) { + self.core.set_width(width); } fn emit( @@ -213,7 +715,7 @@ impl PlanStreamController { return None; } - let mut out_lines: Vec> = Vec::new(); + let mut out_lines: Vec> = Vec::with_capacity(4); let is_stream_continuation = self.header_emitted; if !self.header_emitted { out_lines.push(vec!["• ".dim(), "Proposed Plan".bold()].into()); @@ -221,7 +723,7 @@ impl PlanStreamController { self.header_emitted = true; } - let mut plan_lines: Vec> = Vec::new(); + let mut plan_lines: Vec> = Vec::with_capacity(4); if !self.top_padding_emitted { plan_lines.push(Line::from(" ")); self.top_padding_emitted = true; @@ -248,6 +750,7 @@ impl PlanStreamController { #[cfg(test)] mod tests { use super::*; + use pretty_assertions::assert_eq; use std::path::PathBuf; fn test_cwd() -> PathBuf { @@ -256,6 +759,21 @@ mod tests { std::env::temp_dir() } + fn stream_controller(width: Option) -> StreamController { + let cwd = test_cwd(); + StreamController::new(width, &cwd, false) + } + + fn stream_controller_with_live_tail_reflow(width: Option) -> StreamController { + let cwd = test_cwd(); + StreamController::new(width, &cwd, true) + } + + fn plan_stream_controller(width: Option) -> PlanStreamController { + let cwd = test_cwd(); + PlanStreamController::new(width, &cwd) + } + fn lines_to_plain_strings(lines: &[ratatui::text::Line<'_>]) -> Vec { lines .iter() @@ -269,12 +787,411 @@ mod tests { .collect() } - #[tokio::test] - async fn controller_loose_vs_tight_with_commit_ticks_matches_full() { - let mut ctrl = StreamController::new(/*width*/ None, &test_cwd()); + fn collect_streamed_lines(deltas: &[&str], width: Option) -> Vec { + let mut ctrl = stream_controller(width); + let mut lines = Vec::new(); + 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 { + break; + } + } + } + if let (Some(cell), _source) = ctrl.finalize() { + lines.extend(cell.transcript_lines(u16::MAX)); + } + lines_to_plain_strings(&lines) + .into_iter() + .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 d in deltas { + 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)); + } + lines_to_plain_strings(&lines) + } + + #[test] + fn controller_set_width_rebuilds_queued_lines() { + let mut ctrl = stream_controller(Some(120)); + let delta = "This is a long line that should wrap into multiple rows when resized.\n"; + assert!(ctrl.push(delta)); + assert_eq!(ctrl.queued_lines(), 1); + + ctrl.set_width(Some(24)); + let (cell, idle) = ctrl.on_commit_tick_batch(usize::MAX); + let rendered = lines_to_plain_strings( + &cell + .expect("expected resized queued lines") + .transcript_lines(u16::MAX), + ); + + assert!(idle); + assert!( + rendered.len() > 1, + "expected resized content to occupy multiple lines, got {rendered:?}", + ); + } + + #[test] + fn controller_set_width_no_duplicate_after_emit() { + let mut ctrl = stream_controller(Some(120)); + let line = + "This is a long line that definitely wraps when the terminal shrinks to 24 columns.\n"; + ctrl.push(line); + let (cell, _) = ctrl.on_commit_tick_batch(usize::MAX); + assert!(cell.is_some(), "expected emitted cell"); + assert_eq!(ctrl.queued_lines(), 0); + + ctrl.set_width(Some(24)); + + assert_eq!( + ctrl.queued_lines(), + 0, + "already-emitted content must not be re-queued after resize", + ); + } + + #[test] + fn controller_tick_batch_zero_is_noop() { + let mut ctrl = stream_controller(Some(80)); + assert!(ctrl.push("line one\n")); + assert_eq!(ctrl.queued_lines(), 1); + + let (cell, idle) = ctrl.on_commit_tick_batch(0); + assert!(cell.is_none(), "batch size 0 should not emit lines"); + assert!(!idle, "batch size 0 should not drain queued lines"); + assert_eq!( + ctrl.queued_lines(), + 1, + "queue depth should remain unchanged" + ); + } + + #[test] + 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_reflow_shows_uncommitted_table_cell_content() { + let mut ctrl = stream_controller_with_live_tail_reflow(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 speculative tail to include uncommitted content: {tail:?}", + ); + } + + #[test] + fn controller_live_tail_reflow_disabled_keeps_newline_gated_tail() { + 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 newline-gated tail when live reflow is disabled: {tail:?}", + ); + } + + #[test] + fn controller_live_tail_reflow_requires_table_holdback_state() { + let mut ctrl = stream_controller_with_live_tail_reflow(Some(80)); + ctrl.push("plain text without newline"); + + assert!( + ctrl.current_tail_lines().is_empty(), + "expected no speculative tail outside table holdback state", + ); + assert!(!ctrl.has_live_tail()); + } + + #[test] + fn controller_live_tail_reflow_throttle_reuses_stale_cache() { + let mut ctrl = stream_controller_with_live_tail_reflow(Some(80)); + ctrl.push("| A | B |\n"); + ctrl.push("| --- | --- |\n"); + ctrl.push("| fresh"); + + ctrl.core.speculative_tail_cache = Some(SpeculativeTailCache { + raw_source_len: 0, + uncommitted_len: 0, + enqueued_stable_len: 0, + width: None, + lines: vec![Line::from("stale-cache-line")], + rendered_at: Instant::now(), + }); + + let tail = lines_to_plain_strings(&ctrl.current_tail_lines()); + assert_eq!(tail, vec!["stale-cache-line".to_string()]); + } + + #[test] + fn controller_live_tail_reflow_invalidates_cache_on_push_resize_and_reset() { + let mut ctrl = stream_controller_with_live_tail_reflow(Some(80)); + ctrl.core.speculative_tail_cache = Some(SpeculativeTailCache { + raw_source_len: 0, + uncommitted_len: 0, + enqueued_stable_len: 0, + width: None, + lines: vec![Line::from("stale-cache-line")], + rendered_at: Instant::now(), + }); + + ctrl.push("x"); + assert!( + ctrl.core.speculative_tail_cache.is_none(), + "expected push to invalidate speculative cache", + ); + + ctrl.core.speculative_tail_cache = Some(SpeculativeTailCache { + raw_source_len: 0, + uncommitted_len: 0, + enqueued_stable_len: 0, + width: None, + lines: vec![Line::from("stale-cache-line")], + rendered_at: Instant::now(), + }); + ctrl.set_width(Some(40)); + assert!( + ctrl.core.speculative_tail_cache.is_none(), + "expected resize to invalidate speculative cache", + ); + + ctrl.core.speculative_tail_cache = Some(SpeculativeTailCache { + raw_source_len: 0, + uncommitted_len: 0, + enqueued_stable_len: 0, + width: None, + lines: vec![Line::from("stale-cache-line")], + rendered_at: Instant::now(), + }); + ctrl.core.reset(); + assert!( + ctrl.core.speculative_tail_cache.is_none(), + "expected reset to clear speculative cache", + ); + } + + #[test] + fn controller_live_tail_reflow_rerenders_table_tail_after_resize() { + let mut ctrl = stream_controller_with_live_tail_reflow(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)); + 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_set_width_preserves_in_flight_tail() { + let mut ctrl = plan_stream_controller(Some(80)); + 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_streams_table_header_incrementally() { + 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"), + "table header should enqueue incrementally for plan streams", + ); + } + + #[test] + fn controller_loose_vs_tight_with_commit_ticks_matches_full() { + let mut ctrl = stream_controller(/*width*/ None); let mut lines = Vec::new(); - // Exact deltas from the session log (section: Loose vs. tight list items) let deltas = vec![ "\n\n", "Loose", @@ -345,7 +1262,6 @@ mod tests { "\n\n", ]; - // Simulate streaming with a commit tick attempt after each delta. for d in deltas.iter() { ctrl.push(d); while let (Some(cell), idle) = ctrl.on_commit_tick() { @@ -355,32 +1271,22 @@ mod tests { } } } - // Finalize and flush remaining lines now. - if let Some(cell) = ctrl.finalize() { + if let (Some(cell), _source) = ctrl.finalize() { lines.extend(cell.transcript_lines(u16::MAX)); } let streamed: Vec<_> = lines_to_plain_strings(&lines) .into_iter() - // skip • and 2-space indentation .map(|s| s.chars().skip(2).collect::()) .collect(); - // Full render of the same source let source: String = deltas.iter().copied().collect(); let mut rendered: Vec> = Vec::new(); - let test_cwd = test_cwd(); - crate::markdown::append_markdown( - &source, - /*width*/ None, - Some(test_cwd.as_path()), - &mut rendered, - ); + crate::markdown::append_markdown_agent(&source, None, &mut rendered); let rendered_strs = lines_to_plain_strings(&rendered); assert_eq!(streamed, rendered_strs); - // Also assert exact expected plain strings for clarity. let expected = vec![ "Loose vs. tight list items:".to_string(), "".to_string(), @@ -398,4 +1304,662 @@ mod tests { "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, 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, 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, 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, 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, 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, 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, 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, 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, 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, &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, 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_emits_incremental_rows_before_finalize() { + 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_ne!(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 |"), + "expected incremental raw table header line in plan output: {streamed:?}" + ); + } + + #[test] + fn plan_controller_streamed_markdown_fenced_table_emits_incremental_rows_before_finalize() { + 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_ne!(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 |"), + "expected incremental 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 table_holdback_state_ignores_table_like_lines_inside_unclosed_long_fence() { + let source = "````sh\n```cmd\n| Key | Description |\n| --- | --- |\n````\n"; + assert!( + 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!( + 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 ae3b68742a..009a5eef6e 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>, @@ -70,12 +71,9 @@ impl StreamState { .map(|queued| queued.line) .collect() } - /// Drains all queued lines from the front of the queue. - pub(crate) fn drain_all(&mut self) -> Vec> { - self.queued_lines - .drain(..) - .map(|queued| queued.line) - .collect() + /// Clears queued lines while keeping collector/turn lifecycle state intact. + pub(crate) fn clear_queue(&mut self) { + self.queued_lines.clear(); } /// Returns whether no lines are queued for commit. pub(crate) fn is_idle(&self) -> bool { 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 0000000000..ba8c06386e --- /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 0000000000..cf031c777e --- /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", 3)); + assert!(is_markdown_fence_info("```markdown", 3)); + assert!(is_markdown_fence_info("```MD", 3)); + assert!(!is_markdown_fence_info("```rust", 3)); + assert!(!is_markdown_fence_info("```", 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"); + } +} diff --git a/codex-rs/tui/src/transcript_reflow.rs b/codex-rs/tui/src/transcript_reflow.rs new file mode 100644 index 0000000000..619fe990a1 --- /dev/null +++ b/codex-rs/tui/src/transcript_reflow.rs @@ -0,0 +1,84 @@ +//! State machine for transcript scrollback reflow. + +use std::time::Duration; +use std::time::Instant; + +pub(crate) const TRANSCRIPT_REFLOW_DEBOUNCE: Duration = Duration::from_millis(75); + +#[derive(Debug, Default)] +pub(crate) struct TranscriptReflowState { + last_render_width: Option, + pending_until: Option, + ran_during_stream: bool, +} + +impl TranscriptReflowState { + pub(crate) fn clear(&mut self) { + *self = Self::default(); + } + + pub(crate) fn note_width(&mut self, width: u16) -> TranscriptWidthChange { + let previous_width = self.last_render_width.replace(width); + TranscriptWidthChange { + changed: previous_width.is_some_and(|previous| previous != width), + initialized: previous_width.is_none(), + } + } + + #[cfg(test)] + pub(crate) fn set_last_render_width_for_test(&mut self, width: u16) { + self.last_render_width = Some(width); + } + + /// Schedule a debounced reflow. Returns true if the previous pending reflow was already due. + pub(crate) fn schedule_debounced(&mut self) -> bool { + let now = Instant::now(); + let due_now = self.pending_is_due(now); + self.pending_until = Some(now + TRANSCRIPT_REFLOW_DEBOUNCE); + due_now + } + + pub(crate) fn schedule_immediate(&mut self) { + self.pending_until = Some(Instant::now()); + } + + #[cfg(test)] + pub(crate) fn set_due_for_test(&mut self) { + self.pending_until = Some(Instant::now() - Duration::from_millis(1)); + } + + pub(crate) fn pending_is_due(&self, now: Instant) -> bool { + self.pending_until.is_some_and(|deadline| now >= deadline) + } + + pub(crate) fn pending_until(&self) -> Option { + self.pending_until + } + + pub(crate) fn has_pending_reflow(&self) -> bool { + self.pending_until.is_some() + } + + pub(crate) fn clear_pending_reflow(&mut self) { + self.pending_until = None; + } + + pub(crate) fn mark_ran_during_stream(&mut self) { + self.ran_during_stream = true; + } + + pub(crate) fn take_ran_during_stream(&mut self) -> bool { + let ran = self.ran_during_stream; + self.ran_during_stream = false; + ran + } + + pub(crate) fn clear_ran_during_stream(&mut self) { + self.ran_during_stream = false; + } +} + +pub(crate) struct TranscriptWidthChange { + pub(crate) changed: bool, + pub(crate) initialized: bool, +} diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 5cff8521ad..5e7f1bef25 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -498,6 +498,7 @@ impl Tui { self.frame_requester().schedule_frame(); } + /// Drop any queued history lines that have not yet been flushed to the terminal. pub fn clear_pending_history_lines(&mut self) { self.pending_history_lines.clear(); } diff --git a/codex-rs/tui/src/width.rs b/codex-rs/tui/src/width.rs new file mode 100644 index 0000000000..0a48778093 --- /dev/null +++ b/codex-rs/tui/src/width.rs @@ -0,0 +1,54 @@ +//! Width guards for transcript rendering with fixed prefix columns. +//! +//! Several rendering paths reserve a fixed number of columns for bullets, +//! gutters, or labels before laying out content. When the terminal is very +//! narrow, those reserved columns can consume the entire width, leaving zero +//! or negative space for content. +//! +//! These helpers centralise the subtraction and enforce a strict-positive +//! contract: they return `Some(n)` where `n > 0`, or `None` when no usable +//! content width remains. Callers treat `None` as "render prefix-only +//! fallback" rather than attempting wrapped rendering at zero width, which +//! would produce empty or unstable output. + +/// Returns usable content width after reserving fixed columns. +/// +/// Guarantees a strict positive width (`Some(n)` where `n > 0`) or `None` when +/// the reserved columns consume the full width. +/// +/// Treat `None` as "render prefix-only fallback". Coercing it to `0` and still +/// attempting wrapped rendering often produces empty or unstable output at very +/// narrow terminal widths. +pub(crate) fn usable_content_width(total_width: usize, reserved_cols: usize) -> Option { + total_width + .checked_sub(reserved_cols) + .filter(|remaining| *remaining > 0) +} + +/// `u16` convenience wrapper around [`usable_content_width`]. +/// +/// This keeps width math at callsites that receive terminal dimensions as +/// `u16` while preserving the same `None` contract for exhausted width. +pub(crate) fn usable_content_width_u16(total_width: u16, reserved_cols: u16) -> Option { + usable_content_width(usize::from(total_width), usize::from(reserved_cols)) +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn usable_content_width_returns_none_when_reserved_exhausts_width() { + assert_eq!(usable_content_width(0, 0), None); + assert_eq!(usable_content_width(2, 2), None); + assert_eq!(usable_content_width(3, 4), None); + assert_eq!(usable_content_width(5, 4), Some(1)); + } + + #[test] + fn usable_content_width_u16_matches_usize_variant() { + assert_eq!(usable_content_width_u16(2, 2), None); + assert_eq!(usable_content_width_u16(5, 4), Some(1)); + } +}