mirror of
https://github.com/openai/codex.git
synced 2026-10-14 16:38:43 +00:00
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
This commit is contained in:
@@ -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/unbalanced)parenthesis>",
|
||||
" <https://example.com/unbalanced)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::<Vec<_>>();
|
||||
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();
|
||||
|
||||
@@ -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<HyperlinkLine>,
|
||||
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<u8>,
|
||||
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;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+8
@@ -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]"),
|
||||
]
|
||||
Reference in New Issue
Block a user