From 664fdd2d6e83d1e1502154d792acd6a085b9dbcf Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Fri, 5 Jun 2026 20:02:14 -0300 Subject: [PATCH] fix(tui): preserve streamed tails across remaps --- ...t_toggle_refreshes_active_stream_tail.snap | 5 + .../src/chatwidget/tests/status_and_layout.rs | 37 +++ codex-rs/tui/src/chatwidget/turn_runtime.rs | 6 +- codex-rs/tui/src/streaming/controller.rs | 277 +++++++++++++++++- ...r_streamed_list_continuation_snapshot.snap | 88 ++++++ 5 files changed, 401 insertions(+), 12 deletions(-) create mode 100644 codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__status_and_layout__raw_output_toggle_refreshes_active_stream_tail.snap create mode 100644 codex-rs/tui/src/streaming/snapshots/codex_tui__streaming__controller__tests__plan_controller_streamed_list_continuation_snapshot.snap diff --git a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__status_and_layout__raw_output_toggle_refreshes_active_stream_tail.snap b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__status_and_layout__raw_output_toggle_refreshes_active_stream_tail.snap new file mode 100644 index 0000000000..348b6e13de --- /dev/null +++ b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__status_and_layout__raw_output_toggle_refreshes_active_stream_tail.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/chatwidget/tests/status_and_layout.rs +expression: raw_tail +--- + **tail remains visible** 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 f713b3bd6c..aa925e9a43 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs @@ -1483,6 +1483,42 @@ async fn finalize_turn_persists_held_plan_stream_tail() { assert!(chat.plan_stream_controller.is_none()); } +#[tokio::test] +async fn finalize_turn_persists_structural_plan_tail_as_history_cell() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let cwd = chat.config.cwd.to_path_buf(); + + let mut controller = crate::streaming::controller::PlanStreamController::new( + Some(/*width*/ 80), + cwd.as_path(), + HistoryRenderMode::Rich, + ); + controller.push("| Step | Owner |\n"); + controller.push("| --- | --- |\n"); + controller.push("| Preserve held table tail | Agent |\n"); + assert!( + controller.requires_final_scrollback_reflow(), + "expected table plan tail to require consolidation reflow", + ); + chat.plan_stream_controller = Some(controller); + + while rx.try_recv().is_ok() {} + + chat.finalize_turn(); + + let inserted = drain_insert_history(&mut rx); + let rendered = inserted + .iter() + .map(|cell| lines_to_single_string(cell)) + .collect::>() + .join("\n"); + assert!( + rendered.contains("Preserve held table tail"), + "expected structural plan tail to be persisted directly, got {rendered:?}", + ); + assert!(chat.plan_stream_controller.is_none()); +} + #[tokio::test] async fn raw_output_toggle_refreshes_active_stream_tail() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; @@ -1519,6 +1555,7 @@ async fn raw_output_toggle_refreshes_active_stream_tail() { raw_tail.contains("**tail remains visible**"), "expected active tail to refresh after raw toggle, got {raw_tail:?}", ); + insta::assert_snapshot!(raw_tail); } #[tokio::test] diff --git a/codex-rs/tui/src/chatwidget/turn_runtime.rs b/codex-rs/tui/src/chatwidget/turn_runtime.rs index 0366de6b0d..bf1a085c8a 100644 --- a/codex-rs/tui/src/chatwidget/turn_runtime.rs +++ b/codex-rs/tui/src/chatwidget/turn_runtime.rs @@ -117,10 +117,9 @@ impl ChatWidget { // If a stream is currently active, finalize it. self.flush_answer_stream_with_separator(); if let Some(mut controller) = self.plan_stream_controller.take() { - let requires_scrollback_reflow = controller.requires_final_scrollback_reflow(); self.clear_active_stream_tail(); let (cell, source) = controller.finalize(); - if !requires_scrollback_reflow && let Some(cell) = cell { + if let Some(cell) = cell { self.add_boxed_history(cell); } if let Some(source) = source { @@ -296,10 +295,9 @@ impl ChatWidget { pub(super) fn finalize_turn(&mut self) { self.flush_answer_stream_with_separator(); if let Some(mut controller) = self.plan_stream_controller.take() { - let requires_scrollback_reflow = controller.requires_final_scrollback_reflow(); self.clear_active_stream_tail(); let (cell, source) = controller.finalize(); - if !requires_scrollback_reflow && let Some(cell) = cell { + if let Some(cell) = cell { self.add_boxed_history(cell); } if let Some(source) = source { diff --git a/codex-rs/tui/src/streaming/controller.rs b/codex-rs/tui/src/streaming/controller.rs index e7f891c9de..44d45146f8 100644 --- a/codex-rs/tui/src/streaming/controller.rs +++ b/codex-rs/tui/src/streaming/controller.rs @@ -123,6 +123,7 @@ struct StablePrefixLenCache { struct PartialSourceLine { source_start: usize, source_end: usize, + emitted_source_end: usize, emitted_prefix: Vec, } @@ -284,7 +285,9 @@ impl StreamCore { let had_pending_queue = self.state.queued_len() > 0; let had_structural_tail = self.requires_final_scrollback_reflow(); let partial_line = if had_pending_queue { - self.partial_source_line_after_rendered_len(self.emitted_stable_len) + self.synthetic_queue_partial + .clone() + .or_else(|| self.partial_source_line_after_rendered_len(self.emitted_stable_len)) } else { None }; @@ -359,7 +362,9 @@ impl StreamCore { let had_pending_queue = self.state.queued_len() > 0; let had_structural_tail = self.requires_final_scrollback_reflow(); let partial_line = if had_pending_queue { - self.partial_source_line_after_rendered_len(self.emitted_stable_len) + self.synthetic_queue_partial + .clone() + .or_else(|| self.partial_source_line_after_rendered_len(self.emitted_stable_len)) } else { None }; @@ -470,10 +475,16 @@ impl StreamCore { self.clear_stable_queue(); self.emitted_stable_len = source_line_start_len; - let mut queued = trim_rendered_prefix( - self.rendered_lines[source_line_start_len..source_line_end_len].to_vec(), - &partial_line.emitted_prefix, - ); + let mut queued = if self.render_mode == HistoryRenderMode::Raw { + plain_hyperlink_lines(raw_lines_from_source( + &self.raw_source[partial_line.emitted_source_end..partial_line.source_end], + )) + } else { + trim_rendered_prefix( + self.rendered_lines[source_line_start_len..source_line_end_len].to_vec(), + &partial_line.emitted_prefix, + ) + }; let synthetic_len = queued.len(); if source_line_end_len < target_stable_len { queued.extend(self.rendered_lines[source_line_end_len..target_stable_len].to_vec()); @@ -513,10 +524,26 @@ impl StreamCore { let mut ordinary_lines = step.len(); if self.synthetic_queue_remaining > 0 { let synthetic_lines = ordinary_lines.min(self.synthetic_queue_remaining); + let mut emitted_prefix = None; if let Some(partial_line) = &mut self.synthetic_queue_partial { partial_line .emitted_prefix .extend(step[..synthetic_lines].to_vec()); + emitted_prefix = Some(( + partial_line.source_start, + partial_line.source_end, + partial_line.emitted_prefix.clone(), + )); + } + if let Some((source_start, source_end, emitted_prefix)) = emitted_prefix { + let emitted_source_end = self.source_boundary_after_rendered_prefix( + source_start, + source_end, + &emitted_prefix, + ); + if let Some(partial_line) = &mut self.synthetic_queue_partial { + partial_line.emitted_source_end = emitted_source_end; + } } ordinary_lines -= synthetic_lines; self.synthetic_queue_remaining -= synthetic_lines; @@ -664,11 +691,18 @@ impl StreamCore { if rendered_len <= source_start_rendered_len { return None; } + let emitted_prefix = + self.rendered_lines[source_start_rendered_len..rendered_len].to_vec(); + let emitted_source_end = self.source_boundary_after_rendered_prefix( + source_start, + source_end, + &emitted_prefix, + ); return Some(PartialSourceLine { source_start, source_end, - emitted_prefix: self.rendered_lines[source_start_rendered_len..rendered_len] - .to_vec(), + emitted_source_end, + emitted_prefix, }); } source_start = source_end; @@ -689,6 +723,54 @@ impl StreamCore { boundaries } + fn source_boundary_after_rendered_prefix( + &self, + source_start: usize, + source_end: usize, + emitted_prefix: &[HyperlinkLine], + ) -> usize { + if emitted_prefix.is_empty() { + return source_start; + } + let emitted_text = emitted_prefix + .iter() + .map(hyperlink_line_text) + .collect::>(); + let emitted_visible_text = emitted_text.join(" "); + if let Some(boundary) = visible_text_source_boundary( + &self.raw_source[source_start..source_end], + &emitted_visible_text, + ) { + return source_start + boundary; + } + + let mut boundaries = Vec::new(); + boundaries.push(source_start); + boundaries.extend( + self.raw_source[source_start..source_end] + .char_indices() + .map(|(idx, ch)| source_start + idx + ch.len_utf8()), + ); + + for boundary in boundaries { + let rendered = self.render_source(&self.raw_source[..boundary]); + if rendered.len() < emitted_prefix.len() { + continue; + } + let rendered_text = rendered[..emitted_prefix.len()] + .iter() + .map(hyperlink_line_text) + .collect::>(); + if rendered_text == emitted_text { + return boundary; + } + } + + self.source_boundary_after_rendered_len(emitted_prefix.len()) + .min(source_end) + .max(source_start) + } + fn default_tail_budget_lines(&mut self) -> usize { if self.default_tail_lines == 0 || self.rendered_lines.is_empty() { return 0; @@ -716,6 +798,83 @@ impl StreamCore { } } +fn visible_text_source_boundary(source: &str, visible_prefix: &str) -> Option { + if visible_prefix.is_empty() { + return Some(0); + } + + let mut visible_chars = visible_prefix.chars().peekable(); + let mut iter = source.char_indices().peekable(); + while let Some((idx, ch)) = iter.next() { + if visible_chars.peek().is_none() { + return Some(idx); + } + + if ch == '[' { + let mut label_end = None; + let mut label_chars = Vec::new(); + for (label_idx, label_ch) in iter.by_ref() { + if label_ch == ']' { + label_end = Some(label_idx + label_ch.len_utf8()); + break; + } + label_chars.push((label_idx, label_ch)); + } + let label_end = label_end?; + for (label_idx, label_ch) in label_chars { + match visible_chars.peek().copied() { + Some(expected) if expected == label_ch => { + visible_chars.next(); + if visible_chars.peek().is_none() { + return Some( + skip_link_destination(source, label_end) + .unwrap_or(label_idx + label_ch.len_utf8()), + ); + } + } + _ => return None, + } + } + if let Some(destination_end) = skip_link_destination(source, label_end) { + while let Some((next_idx, _)) = iter.peek().copied() { + if next_idx < destination_end { + iter.next(); + } else { + break; + } + } + } + continue; + } + + if matches!(ch, '*' | '_' | '`') { + continue; + } + + match visible_chars.peek().copied() { + Some(expected) if expected == ch => { + visible_chars.next(); + if visible_chars.peek().is_none() { + return Some(idx + ch.len_utf8()); + } + } + _ => return None, + } + } + + visible_chars.peek().is_none().then_some(source.len()) +} + +fn skip_link_destination(source: &str, label_end: usize) -> Option { + let after_label = source.get(label_end..)?; + if !after_label.starts_with('(') { + return None; + } + after_label + .find(')') + .map(|destination_end| label_end + destination_end + ')'.len_utf8()) +} + fn trim_rendered_prefix( lines: Vec, emitted_prefix: &[HyperlinkLine], @@ -1225,6 +1384,31 @@ mod tests { snapshots } + fn collect_visible_plan_stream_snapshots( + deltas: &[&str], + width: Option, + ) -> Vec> { + let mut ctrl = plan_stream_controller(width); + let mut committed_lines = Vec::new(); + let mut snapshots = Vec::new(); + for delta in deltas { + ctrl.push(delta); + while let (Some(cell), idle) = ctrl.on_commit_tick() { + committed_lines.extend(cell.transcript_lines(u16::MAX)); + if idle { + break; + } + } + + let mut visible = lines_to_plain_strings(&committed_lines); + visible.extend(hyperlink_lines_to_plain_strings( + &ctrl.current_tail_display_lines(), + )); + snapshots.push(visible); + } + snapshots + } + fn format_stream_snapshots(snapshots: &[Vec]) -> String { snapshots .iter() @@ -1414,6 +1598,16 @@ mod tests { ); } + #[test] + fn plan_controller_streamed_list_continuation_snapshot() { + let snapshots = collect_visible_plan_stream_snapshots( + HAPPY_PATH_DUPLICATE_REPRO_DELTAS, + Some(/*width*/ 80), + ); + + insta::assert_snapshot!(format_stream_snapshots(&snapshots)); + } + #[test] fn controller_live_tail_rerenders_table_tail_after_resize() { let mut ctrl = stream_controller(Some(96)); @@ -2542,6 +2736,41 @@ mod tests { ); } + #[test] + fn controller_set_width_reuses_synthetic_partial_across_repeated_resize() { + let mut ctrl = stream_controller(Some(/*width*/ 18)); + ctrl.push("alpha beta gamma delta epsilon zeta eta theta iota kappa lambda mu\n"); + ctrl.push("tail line\n"); + + let (first_emit, idle) = ctrl.on_commit_tick(); + let first_emit = first_emit + .expect("expected first wrapped line emission") + .transcript_lines(u16::MAX); + assert!(!idle, "expected remaining wrapped content after one tick"); + + ctrl.set_width(Some(/*width*/ 80)); + assert!( + ctrl.queued_lines() > 0, + "expected synthetic queue after first resize" + ); + ctrl.set_width(Some(/*width*/ 24)); + + let (cell, _source) = ctrl.finalize(); + let remaining = cell + .map(|c| lines_to_plain_strings(&c.transcript_lines(u16::MAX))) + .unwrap_or_default(); + let first_emit = lines_to_plain_strings(&first_emit).join("\n"); + let joined = remaining.join("\n"); + assert!( + !joined.contains("alpha beta"), + "finalize must not replay the already-visible prefix after repeated resize; emitted before resize: {first_emit:?}, finalized after resize: {joined:?}", + ); + assert!( + joined.contains("lambda mu") && joined.contains("tail line"), + "repeated resize must preserve un-emitted suffix and tail; emitted before resize: {first_emit:?}, finalized after resize: {joined:?}", + ); + } + #[test] fn controller_set_render_mode_partial_wrapped_emit_preserves_remaining_content() { let mut ctrl = stream_controller(Some(/*width*/ 18)); @@ -2606,6 +2835,38 @@ mod tests { ); } + #[test] + fn controller_set_render_mode_raw_suffix_starts_after_rich_link_prefix() { + let mut ctrl = stream_controller(Some(/*width*/ 10)); + ctrl.push( + "[alpha beta](https://example.com) gamma delta epsilon zeta eta theta iota kappa lambda mu\n", + ); + ctrl.push("tail line\n"); + + let (first_emit, idle) = ctrl.on_commit_tick(); + let first_emit = first_emit + .expect("expected first rich wrapped line emission") + .transcript_lines(u16::MAX); + assert!(!idle, "expected remaining rich content after one tick"); + + ctrl.set_render_mode(HistoryRenderMode::Raw); + + let (cell, _source) = ctrl.finalize(); + let remaining = cell + .map(|c| lines_to_plain_strings(&c.transcript_lines(u16::MAX))) + .unwrap_or_default(); + let first_emit = lines_to_plain_strings(&first_emit).join("\n"); + let joined = remaining.join("\n"); + assert!( + !joined.contains("https://example.com") && !joined.contains("](https://"), + "raw suffix must not start inside already-visible link markup; emitted before toggle: {first_emit:?}, finalized after toggle: {joined:?}", + ); + assert!( + joined.contains("gamma") && joined.contains("tail line"), + "raw suffix should preserve un-emitted source after the rendered link label; emitted before toggle: {first_emit:?}, finalized after toggle: {joined:?}", + ); + } + #[test] fn controller_clear_queue_advances_synthetic_accounting_before_finalize() { let mut ctrl = stream_controller(Some(/*width*/ 18)); diff --git a/codex-rs/tui/src/streaming/snapshots/codex_tui__streaming__controller__tests__plan_controller_streamed_list_continuation_snapshot.snap b/codex-rs/tui/src/streaming/snapshots/codex_tui__streaming__controller__tests__plan_controller_streamed_list_continuation_snapshot.snap new file mode 100644 index 0000000000..6d9e9bb0b9 --- /dev/null +++ b/codex-rs/tui/src/streaming/snapshots/codex_tui__streaming__controller__tests__plan_controller_streamed_list_continuation_snapshot.snap @@ -0,0 +1,88 @@ +--- +source: tui/src/streaming/controller.rs +expression: format_stream_snapshots(&snapshots) +--- +after delta 0: +• Proposed Plan + + + Happy Path +--- +after delta 1: +• Proposed Plan + + + Happy Path +--- +after delta 2: +• Proposed Plan + + + Happy Path + + 1. In the second terminal, make the smoke home non-writable: +--- +after delta 3: +• Proposed Plan + + + Happy Path + + 1. In the second terminal, make the smoke home non-writable: + +--- +after delta 4: +• Proposed Plan + + + Happy Path + + 1. In the second terminal, make the smoke home non-writable: + + chmod a-w $CODEX_SMOKE_HOME +--- +after delta 5: +• Proposed Plan + + + Happy Path + + 1. In the second terminal, make the smoke home non-writable: + + chmod a-w $CODEX_SMOKE_HOME +--- +after delta 6: +• Proposed Plan + + + Happy Path + + 1. In the second terminal, make the smoke home non-writable: + + chmod a-w $CODEX_SMOKE_HOME + Expected: the running TUI stays open. +--- +after delta 7: +• Proposed Plan + + + Happy Path + + 1. In the second terminal, make the smoke home non-writable: + + chmod a-w $CODEX_SMOKE_HOME + Expected: the running TUI stays open. +--- +after delta 8: +• Proposed Plan + + + Happy Path + + 1. In the second terminal, make the smoke home non-writable: + + chmod a-w $CODEX_SMOKE_HOME + + Expected: the running TUI stays open. + + 2. In the TUI, type /model and press Enter.