From e0a64cf2bc4535eb330c22857260a7856c1e8749 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Wed, 23 Sep 2026 16:27:18 +0000 Subject: [PATCH] Keep unfinished link destinations out of rich streaming previews (#47589) ## Why An unfinished Markdown link can display its destination across multiple preview rows, then collapse when the closing parenthesis arrives. ## What changed Keep the link label visible while withholding the opening parenthesis and destination until the link closes. Scan incoming text incrementally, accounting for nested parentheses, escapes, angle-bracket destinations, titles, and inline code. Apply this holdback only to rich prose previews; newline commitment and finalization still render the original source. ## Testing Add regression tests for character-by-character input, long Unicode destinations, resizing, raw-mode switching, and flushing unfinished links on newline or completion, plus a snapshot of the label-only preview. GitOrigin-RevId: 9cea680a93dfcd0aea21f0fc12006b27400d4dab --- .../src/streaming/controller_preview_tests.rs | 158 ++++++++++++++++++ codex-rs/tui/src/streaming/prose_preview.rs | 102 ++++++++++- ...review_tests__unfinished_link_preview.snap | 8 + 3 files changed, 267 insertions(+), 1 deletion(-) create mode 100644 codex-rs/tui/src/streaming/snapshots/codex_tui__streaming__controller__preview_tests__unfinished_link_preview.snap diff --git a/codex-rs/tui/src/streaming/controller_preview_tests.rs b/codex-rs/tui/src/streaming/controller_preview_tests.rs index 19d4820f94..bad0220331 100644 --- a/codex-rs/tui/src/streaming/controller_preview_tests.rs +++ b/codex-rs/tui/src/streaming/controller_preview_tests.rs @@ -2,6 +2,164 @@ use super::*; use crate::terminal_hyperlinks::visible_lines; use pretty_assertions::assert_eq; +#[test] +fn unfinished_link_destinations_do_not_expand_the_preview() { + let cwd = std::env::temp_dir(); + for destination in [ + "https://example.com/a/very/long/path/to/the/guide", + "https://example.com/a_(nested_(path))", + r"https://example.com/escaped\)parenthesis", + "", + " ", + "https://example.com/guide \"a title with ) parentheses\"", + "https://example.com/guide (x < y)", + ] { + let mut controller = StreamController::new(Some(24), &cwd, HistoryRenderMode::Rich); + controller.push("See [documentation]"); + let label = controller.current_tail_lines(); + // Split at every character, including the opening delimiter and escapes. + for character in format!("({destination}").chars() { + controller.push(&character.to_string()); + assert_eq!(controller.current_tail_lines(), label); + } + controller.push(") and continue."); + assert_eq!( + controller.current_tail_lines(), + render_source( + &format!("See [documentation]({destination}) and continue."), + Some(24), + &cwd, + HistoryRenderMode::Rich, + /*inline_visualization_context*/ None, + ), + ); + } + // The preview's byte limit must not drop the opener and expose a long URL tail. + let mut controller = StreamController::new(Some(24), &cwd, HistoryRenderMode::Rich); + controller.push("See [documentation]"); + let label = controller.current_tail_lines(); + controller.push(&format!("(https://example.com/{}", "界".repeat(3000))); + controller.push("more-path"); + assert_eq!(controller.current_tail_lines(), label); +} + +#[test] +fn unfinished_links_flush_on_newline_or_completion() { + let cwd = std::env::temp_dir(); + let source = "See [documentation](https://example.com/a/very/long/path"; + for ending in ["", "\n"] { + let mut controller = StreamController::new(Some(24), &cwd, HistoryRenderMode::Rich); + controller.push(source); + insta::assert_debug_snapshot!( + "unfinished_link_preview", + visible_lines(controller.current_tail_lines()), + ); + controller.set_width(Some(32)); + assert_eq!( + visible_lines(controller.current_tail_lines()), + vec![Line::from("See [documentation]")], + ); + controller.push(ending); + let (queued, _) = controller.on_commit_tick_batch(usize::MAX); + let (remaining, finalized_source) = controller.finalize(); + let displayed = queued + .into_iter() + .chain(remaining) + .flat_map(|cell| cell.display_lines(/*width*/ 34)) + .collect::>(); + let expected = history_cell::AgentMessageCell::new_hyperlink_lines( + render_source( + source, + Some(32), + &cwd, + HistoryRenderMode::Rich, + /*inline_visualization_context*/ None, + ), + /*is_first_line*/ true, + ); + assert_eq!(displayed, expected.display_lines(/*width*/ 34)); + assert_eq!(finalized_source, Some(format!("{source}\n"))); + } +} + +#[test] +fn link_preview_respects_code_escapes_and_raw_mode() { + let cwd = std::env::temp_dir(); + for source in [ + r"See \[label](https://example.com/unfinished", + r"See [label\](https://example.com/unfinished", + "See `[label](https://example.com/unfinished`", + "See ``[la`bel](https://example.com/unfinished``", + "See [label](https://example.com/complete) and prose", + ] { + let mut controller = StreamController::new(Some(24), &cwd, HistoryRenderMode::Rich); + for character in source.chars() { + controller.push(&character.to_string()); + } + assert_eq!( + controller.current_tail_lines(), + render_source( + source, + Some(24), + &cwd, + HistoryRenderMode::Rich, + /*inline_visualization_context*/ None, + ), + "{source}", + ); + } + let mut controller = StreamController::new(Some(24), &cwd, HistoryRenderMode::Rich); + controller.push("See `[code](literal` then [label](https://example.com/unfinished"); + assert_eq!( + controller.current_tail_lines(), + render_source( + "See `[code](literal` then [label]", + Some(24), + &cwd, + HistoryRenderMode::Rich, + /*inline_visualization_context*/ None, + ), + ); + + let source = "See [label](https://example.com/unfinished"; + let mut controller = StreamController::new(Some(24), &cwd, HistoryRenderMode::Raw); + controller.push(source); + assert_eq!( + controller.current_tail_lines(), + render_source( + source, + Some(24), + &cwd, + HistoryRenderMode::Raw, + /*inline_visualization_context*/ None, + ), + ); +} + +#[test] +fn link_holdback_stays_rich_only_when_a_pipe_freezes_the_preview() { + let cwd = std::env::temp_dir(); + let source = "See [label](https://example.com"; + let mut controller = StreamController::new(Some(40), &cwd, HistoryRenderMode::Rich); + controller.push(source); + let label = controller.current_tail_lines(); + controller.push("|suffix)"); + assert_eq!(controller.current_tail_lines(), label); + controller.set_render_mode(HistoryRenderMode::Raw); + assert_eq!( + controller.current_tail_lines(), + render_source( + source, + Some(40), + &cwd, + HistoryRenderMode::Raw, + /*inline_visualization_context*/ None, + ), + ); + controller.set_render_mode(HistoryRenderMode::Rich); + assert_eq!(controller.current_tail_lines(), label); +} + #[test] fn source_only_changes_refresh_the_preview_and_active_tail() { let cwd = std::env::temp_dir(); diff --git a/codex-rs/tui/src/streaming/prose_preview.rs b/codex-rs/tui/src/streaming/prose_preview.rs index b8b4cdc1cf..ba464101ab 100644 --- a/codex-rs/tui/src/streaming/prose_preview.rs +++ b/codex-rs/tui/src/streaming/prose_preview.rs @@ -1,6 +1,7 @@ //! Bounded, disposable previews of unterminated prose. Preview lines never enter scrollback; //! newline commitment and finalization render the original source independently. Math previews //! show wrapped source until closure, without interpreting TeX punctuation as Markdown. +//! Rich prose withholds unfinished link destinations so closing a link cannot collapse URL rows. use super::render::render_source; use crate::history_cell::HistoryRenderMode; @@ -23,7 +24,9 @@ pub(super) struct ProsePreview { pub(super) lines: Vec, scanned_len: usize, safe_len: usize, + rich_safe_len: usize, has_pipe: bool, + link: LinkPreview, } impl ProsePreview { @@ -37,6 +40,7 @@ impl ProsePreview { ) -> bool { // Only scan newly arrived bytes, including on very long single-line responses. self.has_pipe |= source[self.scanned_len..].contains('|'); + self.link.scan(source, self.scanned_len); self.scanned_len = source.len(); // Indented and quoted lines may belong to nested code blocks. Keep their // existing newline holdback instead of guessing at the missing block context. @@ -48,9 +52,18 @@ impl ProsePreview { || matches!(source, "`" | "``" | "~" | "~~")) { self.safe_len = source.len(); + self.rich_safe_len = source.len(); + } + // Keep Raw/Math's structural bound independent of Rich-only link holdback. + if let Some((start, _)) = self.link.destination { + self.rich_safe_len = self.rich_safe_len.min(start); } // Retain the last safe text when tokens reveal structure, but still reflow it. - let source = &source[..self.safe_len]; + let source = &source[..if matches!(mode, PreviewMode::Prose(HistoryRenderMode::Rich)) { + self.rich_safe_len + } else { + self.safe_len + }]; let start = source.ceil_char_boundary(source.len().saturating_sub(MAX_PREVIEW_BYTES)); let mut lines = match mode { PreviewMode::Prose(render_mode) => render_source( @@ -75,3 +88,90 @@ impl ProsePreview { true } } + +/// Scans append-only source without buffering or rescanning a growing destination. +/// Keep the label visible; the opening `(` and everything after it wait for closure. +#[derive(Default)] +struct LinkPreview { + brackets: usize, + closed_label: bool, + destination: Option<(usize, usize)>, + delimiter: Option, + whitespace: bool, + escaped: bool, + code_ticks: usize, + pending_ticks: usize, +} + +impl LinkPreview { + fn scan(&mut self, source: &str, start: usize) { + for (index, byte) in source.bytes().enumerate().skip(start) { + if self.destination.is_none() { + if byte == b'`' && (self.code_ticks > 0 || !self.escaped) { + self.pending_ticks += 1; + self.closed_label = false; + continue; + } + if self.pending_ticks > 0 { + if self.code_ticks == 0 { + self.code_ticks = self.pending_ticks; + } else if self.code_ticks == self.pending_ticks { + self.code_ticks = 0; + } + self.pending_ticks = 0; + } + if self.code_ticks > 0 { + continue; + } + } + if self.escaped { + self.escaped = false; + self.closed_label = false; + self.whitespace = false; + continue; + } + if byte == b'\\' { + self.escaped = true; + self.closed_label = false; + continue; + } + if let Some((destination_start, depth)) = self.destination.as_mut() { + if let Some(delimiter) = self.delimiter { + if byte == delimiter { + self.delimiter = None; + } + } else if byte == b'<' + && *depth == 1 + && (index == *destination_start + 1 || self.whitespace) + { + self.delimiter = Some(b'>'); + } else if matches!(byte, b'\'' | b'"') && self.whitespace && *depth == 1 { + self.delimiter = Some(byte); + } else if byte == b'(' { + *depth += 1; + } else if byte == b')' { + *depth -= 1; + if *depth == 0 { + self.destination = None; + } + } + self.whitespace = byte.is_ascii_whitespace(); + } else { + match byte { + b'[' => self.brackets += 1, + b']' if self.brackets > 0 => { + self.brackets -= 1; + self.closed_label = true; + continue; + } + b'(' if self.closed_label => { + self.destination = Some((index, 1)); + self.whitespace = false; + } + _ => {} + } + self.closed_label = false; + } + } + } +} diff --git a/codex-rs/tui/src/streaming/snapshots/codex_tui__streaming__controller__preview_tests__unfinished_link_preview.snap b/codex-rs/tui/src/streaming/snapshots/codex_tui__streaming__controller__preview_tests__unfinished_link_preview.snap new file mode 100644 index 0000000000..4895b40acf --- /dev/null +++ b/codex-rs/tui/src/streaming/snapshots/codex_tui__streaming__controller__preview_tests__unfinished_link_preview.snap @@ -0,0 +1,8 @@ +--- +source: tui/src/streaming/controller_preview_tests.rs +assertion_line: 52 +expression: visible_lines(controller.current_tail_lines()) +--- +[ + Line::from("See [documentation]"), +]