From f46671b14aa3bc37d4ee9a67c06385cb9ec8e2d3 Mon Sep 17 00:00:00 2001 From: Ian MacLeod Date: Fri, 4 Sep 2026 01:35:17 +0000 Subject: [PATCH] Render assistant file citations as local links (#42650) ## What changed - Convert `codex-file-citation` directives in assistant Markdown into local-file links while preserving paths with Markdown-significant characters, Unicode, Windows separators, and location suffixes. - Apply citation rendering consistently to streaming output, finalized messages and plans, and resume-picker transcript previews. - Keep directives literal in code, HTML, existing links, reference definitions, escaped text, and generic Markdown rendering. - Bound repeated parsing work for malformed directive candidates without limiting valid citations. ## Testing - Add coverage for citation parsing and rendering across finalized, streaming, and resume-preview output, including malformed input and path edge cases. GitOrigin-RevId: 5a0bfb3fc7ac21b7e8846fd170a349fb2d2d5e85 --- codex-rs/tui/src/assistant_directives.rs | 43 +++- .../tui/src/assistant_directives_tests.rs | 50 +++++ .../tui/src/history_cell/messages_tests.rs | 16 ++ codex-rs/tui/src/history_cell/plans_tests.rs | 16 ++ codex-rs/tui/src/markdown.rs | 14 +- codex-rs/tui/src/markdown_render.rs | 2 + .../tui/src/markdown_render/file_citations.rs | 198 +++++++++++++++++ .../markdown_render/file_citations_tests.rs | 202 ++++++++++++++++++ codex-rs/tui/src/markdown_render/streaming.rs | 6 +- codex-rs/tui/src/resume_picker.rs | 26 ++- ...tests__resume_picker_expanded_session.snap | 2 +- codex-rs/tui/src/streaming/render_tests.rs | 20 ++ ...er__tests__incremental_file_citations.snap | 62 ++++++ 13 files changed, 635 insertions(+), 22 deletions(-) create mode 100644 codex-rs/tui/src/markdown_render/file_citations.rs create mode 100644 codex-rs/tui/src/markdown_render/file_citations_tests.rs create mode 100644 codex-rs/tui/src/streaming/snapshots/codex_tui__streaming__render__tests__incremental_file_citations.snap diff --git a/codex-rs/tui/src/assistant_directives.rs b/codex-rs/tui/src/assistant_directives.rs index f88306448a..9e89bf0e02 100644 --- a/codex-rs/tui/src/assistant_directives.rs +++ b/codex-rs/tui/src/assistant_directives.rs @@ -6,6 +6,7 @@ //! can contain quotes and braces within its quoted value. Consumers choose //! the quote-escaping rules and interpret the attributes; this module also //! retains the exact directive source, excluding any trailing Markdown. +//! Scanners can share a byte-work budget across unsuccessful parse attempts. use std::borrow::Cow; use std::collections::BTreeMap; @@ -30,13 +31,30 @@ pub(crate) fn parse_assistant_directive( source: &str, escaping: QuoteEscaping, ) -> Option> { + let mut remaining = usize::MAX; + parse_assistant_directive_with_budget(source, escaping, &mut remaining) +} + +/// Parse with a shared byte-scanning budget for callers that retry at multiple offsets. +/// +/// Charge inspected source, not the whole supplied suffix. A scan may finish its current token +/// before exhausting the budget; subsequent attempts return immediately. This bounds repeated +/// malformed candidates without imposing a fixed size or count limit on valid directives. +pub(crate) fn parse_assistant_directive_with_budget<'source>( + source: &'source str, + escaping: QuoteEscaping, + remaining: &mut usize, +) -> Option> { + spend_scan_budget(remaining, /*scanned*/ 1)?; // `::git-create-pr{...}` starts with a one-to-three-colon marker and a name; // require `{` immediately after the name so `::git-create-pr prose` is not parsed. let rest = source.trim_start_matches(':'); + spend_scan_budget(remaining, source.len() - rest.len())?; if !(1..=3).contains(&(source.len() - rest.len())) { return None; } let name_len = rest.bytes().take_while(|byte| is_name_byte(*byte)).count(); + spend_scan_budget(remaining, name_len + 1)?; let (name, suffix) = rest.split_at(name_len); let mut rest = suffix.strip_prefix('{')?; if !name.as_bytes().first().is_some_and(u8::is_ascii_alphabetic) { @@ -45,7 +63,7 @@ pub(crate) fn parse_assistant_directive( let mut attributes = BTreeMap::new(); loop { - rest = rest.trim_start_matches([' ', '\t']); + rest = trim_attribute_space(rest, remaining)?; if let Some(suffix) = rest.strip_prefix('}') { // In `::git-push{cwd="/repo"} done`, retain the directive but not ` done`. return Some(AssistantDirective { @@ -57,13 +75,14 @@ pub(crate) fn parse_assistant_directive( // For `cwd = "/repo" isDraft=true}`, split off `cwd` and leave the next // attribute for the next iteration after consuming this value. let key_len = rest.bytes().take_while(|byte| is_name_byte(*byte)).count(); + spend_scan_budget(remaining, key_len + 1)?; let (key, suffix) = rest.split_at(key_len); // Reject malformed keys and duplicates before scanning a potentially long value. if key.is_empty() || attributes.contains_key(key) { return None; } - let value = suffix.trim_start_matches([' ', '\t']).strip_prefix('=')?; - rest = value.trim_start_matches([' ', '\t']); + let value = trim_attribute_space(suffix, remaining)?.strip_prefix('=')?; + rest = trim_attribute_space(value, remaining)?; let value = if let Some(delimiter @ (b'"' | b'\'')) = rest.as_bytes().first().copied() { // In `body="Keep \"x}\" literal."`, only the matching unescaped quote // ends the value, not the embedded `}`. Single-quoted values work too. @@ -71,6 +90,7 @@ pub(crate) fn parse_assistant_directive( let mut characters = quoted.char_indices().peekable(); let end = loop { let (index, character) = characters.next()?; + spend_scan_budget(remaining, character.len_utf8())?; match character { c if c == char::from(delimiter) => break index, '\n' | '\r' => return None, @@ -82,6 +102,7 @@ pub(crate) fn parse_assistant_directive( // Backslash mode consumes `\"` as a literal quote. Literal // mode instead lets the quote close `cwd="/repo\"`. characters.next(); + spend_scan_budget(remaining, /*scanned*/ 1)?; } _ => {} } @@ -99,6 +120,7 @@ pub(crate) fn parse_assistant_directive( let end = rest .find([' ', '\t', '}', '\n', '\r']) .unwrap_or(rest.len()); + spend_scan_budget(remaining, end + 1)?; if end == 0 { return None; } @@ -110,6 +132,21 @@ pub(crate) fn parse_assistant_directive( } } +fn trim_attribute_space<'source>( + source: &'source str, + remaining: &mut usize, +) -> Option<&'source str> { + let rest = source.trim_start_matches([' ', '\t']); + spend_scan_budget(remaining, source.len() - rest.len() + 1)?; + Some(rest) +} + +fn spend_scan_budget(remaining: &mut usize, scanned: usize) -> Option<()> { + let available = *remaining; + *remaining = remaining.saturating_sub(scanned); + (scanned <= available).then_some(()) +} + fn is_name_byte(byte: u8) -> bool { byte.is_ascii_alphanumeric() || matches!(byte, b'_' | b'-') } diff --git a/codex-rs/tui/src/assistant_directives_tests.rs b/codex-rs/tui/src/assistant_directives_tests.rs index 8592f1e6c9..2babca6c7d 100644 --- a/codex-rs/tui/src/assistant_directives_tests.rs +++ b/codex-rs/tui/src/assistant_directives_tests.rs @@ -3,6 +3,7 @@ use super::AssistantDirective; use super::QuoteEscaping; use super::parse_assistant_directive; +use super::parse_assistant_directive_with_budget; use pretty_assertions::assert_eq; use std::borrow::Cow; use std::collections::BTreeMap; @@ -84,3 +85,52 @@ fn rejects_ambiguous_or_incomplete_directives() { ); } } + +#[test] +fn malformed_retries_exhaust_the_shared_scan_budget() { + let source = format!( + "codex-file-citation {} x bad}}", + ":a{k=".repeat(/*n*/ 16_000) + ); + let mut remaining = source.len() * 4; + let mut attempts = 0; + for (offset, _) in source.match_indices(':') { + if remaining == 0 { + break; + } + assert_eq!( + parse_assistant_directive_with_budget( + &source[offset..], + QuoteEscaping::Literal, + &mut remaining, + ), + None, + ); + attempts += 1; + } + assert!(attempts <= 8, "retried {attempts} long malformed values"); + assert_eq!(remaining, 0); +} + +#[test] +fn scan_budget_counts_inspected_values_not_the_unread_suffix() { + let tail = "x".repeat(/*n*/ 16_000); + let raw = ":artifact{path=report.xlsx}"; + let source = format!("{raw}{tail}"); + let mut remaining = 64; + assert_eq!( + parse_assistant_directive_with_budget(&source, QuoteEscaping::Literal, &mut remaining), + parse_assistant_directive(raw, QuoteEscaping::Literal), + ); + for source in [ + format!(":artifact{{path=\"{tail}"), + format!(":artifact{{path={tail}"), + ] { + let mut remaining = 64; + assert_eq!( + parse_assistant_directive_with_budget(&source, QuoteEscaping::Literal, &mut remaining), + None, + ); + assert_eq!(remaining, 0); + } +} diff --git a/codex-rs/tui/src/history_cell/messages_tests.rs b/codex-rs/tui/src/history_cell/messages_tests.rs index c031a21147..0288915899 100644 --- a/codex-rs/tui/src/history_cell/messages_tests.rs +++ b/codex-rs/tui/src/history_cell/messages_tests.rs @@ -92,6 +92,22 @@ fn finalized_markdown_reuses_lines_primed_by_transcript_height() { ); } +#[test] +fn finalized_assistant_file_citation_renders_as_local_path_snapshot() { + let cwd = std::env::temp_dir(); + let output = cwd.join("Quarterly Report.xlsx").display().to_string(); + let cell = AgentMarkdownCell::new( + format!( + r#"Generated :codex-file-citation{{artifact_kind="workbook" path="{output}" purpose="output"}}."# + ), + &cwd, + ); + + let rendered = ratatui::text::Text::from(cell.display_lines(/*width*/ 80)); + + insta::assert_snapshot!(rendered, @"• Generated Quarterly Report.xlsx."); +} + #[test] fn finalized_markdown_cache_misses_when_width_or_render_style_changes() { let cell = AgentMarkdownCell::new("finalized **markdown**".to_string(), Path::new("/tmp")); diff --git a/codex-rs/tui/src/history_cell/plans_tests.rs b/codex-rs/tui/src/history_cell/plans_tests.rs index 82fa3eed9d..b2ab08a6e2 100644 --- a/codex-rs/tui/src/history_cell/plans_tests.rs +++ b/codex-rs/tui/src/history_cell/plans_tests.rs @@ -20,3 +20,19 @@ fn finalized_plan_reuses_lines_primed_by_transcript_height() { vec![Line::from("cached")] ); } + +#[test] +fn finalized_plan_file_citation_renders_as_local_path_snapshot() { + let cwd = std::env::temp_dir(); + let output = cwd.join("Quarterly Report.xlsx").display().to_string(); + let plan = new_proposed_plan( + format!( + "- :codex-file-citation{{path=\"{output}\" purpose=\"output\" artifact_kind=\"workbook\"}}\n" + ), + &cwd, + ); + + let rendered = ratatui::text::Text::from(plan.display_lines(/*width*/ 80)); + + insta::assert_snapshot!(rendered, @"• Proposed Plan\n \n \n - Quarterly Report.xlsx"); +} diff --git a/codex-rs/tui/src/markdown.rs b/codex-rs/tui/src/markdown.rs index a6c562bb25..8c6ce25b2f 100644 --- a/codex-rs/tui/src/markdown.rs +++ b/codex-rs/tui/src/markdown.rs @@ -98,13 +98,13 @@ pub(crate) fn render_markdown_agent_with_links_cwd_and_visualizations( rewritten.trusted_file_links.contains_key(destination) || crate::markdown_render::hide_web_link_destination(destination) }; - let mut lines = - crate::markdown_render::render_markdown_lines_with_width_cwd_and_hidden_link_destinations( - &normalized, - width, - cwd, - &is_hidden_link_destination, - ); + let mut lines = crate::markdown_render::render_streaming_markdown_lines_with_width_and_cwd( + &normalized, + width, + cwd, + &is_hidden_link_destination, + ) + .lines; for hyperlink in lines.iter_mut().flat_map(|line| &mut line.hyperlinks) { if let Some(link) = rewritten.trusted_file_links.get(&hyperlink.destination) { hyperlink.retarget_to_trusted_file(&link.destination); diff --git a/codex-rs/tui/src/markdown_render.rs b/codex-rs/tui/src/markdown_render.rs index 39e885da60..a9f33b1291 100644 --- a/codex-rs/tui/src/markdown_render.rs +++ b/codex-rs/tui/src/markdown_render.rs @@ -68,11 +68,13 @@ use std::ops::Range; use std::path::Path; use std::path::PathBuf; +mod file_citations; mod local_links; mod streaming; mod table_key_value; mod web_links; +use file_citations::FileCitations; use local_links::is_local_path_like_link; use local_links::render_local_link_target; use local_links::should_render_local_link_label; diff --git a/codex-rs/tui/src/markdown_render/file_citations.rs b/codex-rs/tui/src/markdown_render/file_citations.rs new file mode 100644 index 0000000000..f14ceb3134 --- /dev/null +++ b/codex-rs/tui/src/markdown_render/file_citations.rs @@ -0,0 +1,198 @@ +//! Prepare file citations once, then feed them to the ordinary Markdown link renderer. + +use super::local_links::extract_colon_location_suffix; +use super::local_links::is_local_path_like_link; +use crate::assistant_directives::AssistantDirective; +use crate::assistant_directives::QuoteEscaping; +use crate::assistant_directives::parse_assistant_directive_with_budget; +use itertools::Either; +use pulldown_cmark::Event; +use pulldown_cmark::LinkType; +use pulldown_cmark::Options; +use pulldown_cmark::Parser; +use pulldown_cmark::Tag; +use pulldown_cmark::TagEnd; +use std::borrow::Cow; +use std::ops::Range; +use std::path::Path; + +/// Offset-preserving Markdown plus the original, fully parsed citation metadata. +pub(super) struct FileCitations<'a> { + input: &'a str, + pub(super) markdown: Cow<'a, str>, + citations: Vec<(Range, AssistantDirective<'a>)>, +} + +impl<'a> FileCitations<'a> { + pub(super) fn new(input: &'a str, options: Options) -> Self { + let mut prepared = Self { + input, + markdown: Cow::Borrowed(input), + citations: Vec::new(), + }; + if !input.contains("codex-file-citation") { + return prepared; + } + + let parser = Parser::new_ext(input, options); + let mut literal_ranges: Vec<_> = parser + .reference_definitions() + .iter() + .map(|(_, definition)| definition.span.clone()) + .collect(); + literal_ranges.extend(parser.into_offset_iter().filter_map(|(event, range)| { + matches!( + event, + Event::Code(_) + | Event::Html(_) + | Event::InlineHtml(_) + | Event::Start(Tag::CodeBlock(_) | Tag::Link { .. } | Tag::Image { .. }) + ) + .then_some(range) + })); + // Reference definitions arrive separately; visit all literal ranges in source order. + literal_ranges.sort_unstable_by_key(|range| range.start); + let mut literal_ranges = literal_ranges.into_iter().peekable(); + + let mut directive_end = 0; + // Share the allowance across offsets and quote modes: malformed retries must stay linear. + let mut scan_budget = input.len().saturating_mul(/*rhs*/ 4); + for (start, _) in input.match_indices(':') { + while literal_ranges.next_if(|range| range.end <= start).is_some() {} + if start < directive_end + || input[..start].ends_with(':') + || literal_ranges + .peek() + .is_some_and(|range| range.contains(&start)) + { + continue; + } + let source = &input[start..]; + // Citations prefer literal quoting; other directives prefer escaped quotes. + let escaping = if source + .trim_start_matches(':') + .starts_with("codex-file-citation{") + { + [QuoteEscaping::Literal, QuoteEscaping::Backslash] + } else { + [QuoteEscaping::Backslash, QuoteEscaping::Literal] + }; + let Some(directive) = escaping.into_iter().find_map(|escaping| { + parse_assistant_directive_with_budget(source, escaping, &mut scan_budget) + }) else { + continue; + }; + let end = start + directive.raw.len(); + directive_end = end; + if input[..start] + .bytes() + .rev() + .take_while(|byte| *byte == b'\\') + .count() + % 2 + != 0 + || directive.name != "codex-file-citation" + || directive + .attributes + .get("path") + .is_none_or(|path| path.is_empty()) + { + continue; + } + // Mask the interior without moving offsets or changing Markdown delimiter flanking. + let markdown = prepared.markdown.to_mut(); + markdown.replace_range(start + 1..end - 1, &"x".repeat(end - start - 2)); + prepared.citations.push((start..end, directive)); + } + prepared + } + + /// Adapt before `DecodedTextMerge`, while plain text still has exact source offsets. + pub(super) fn events<'s>( + &'s self, + parser: Parser<'s>, + cwd: Option<&'s Path>, + ) -> impl Iterator, Range)> { + let mut citations = self.citations.iter().peekable(); + parser.into_offset_iter().flat_map(move |(event, range)| { + while citations + .next_if(|(span, _)| span.end <= range.start) + .is_some() + {} + let Event::Text(text) = event else { + return Either::Left(std::iter::once((event, range))); + }; + if citations + .peek() + .is_none_or(|(span, _)| span.start >= range.end) + { + return Either::Left(std::iter::once((Event::Text(text), range))); + } + // Never apply source offsets to entity-decoded text or a partial citation. + if text.as_ref() != &self.markdown[range.clone()] + || citations + .peek() + .is_some_and(|(span, _)| span.start < range.start || span.end > range.end) + { + let text = self.input.get(range.clone()).map_or(text, Into::into); + return Either::Left(std::iter::once((Event::Text(text), range))); + } + + let mut events = Vec::new(); + let mut offset = range.start; + while let Some((span, directive)) = citations.next_if(|(span, _)| span.end <= range.end) + { + if offset < span.start { + events.push(( + Event::Text(self.markdown[offset..span.start].into()), + offset..span.start, + )); + } + let path = directive.attributes["path"].as_ref(); + let destination = if is_local_path_like_link(path) { + path.to_string() + } else { + cwd.map_or_else( + || format!("./{path}"), + |cwd| cwd.join(path).to_string_lossy().into_owned(), + ) + }; + // Citation paths are literal; the existing link renderer decodes destinations. + let mut destination = destination + .replace('%', "%25") + .replace('#', "%23") + .replace('?', "%3F"); + if let Some(suffix) = extract_colon_location_suffix(&destination) { + let suffix_start = destination.len() - suffix.len(); + destination.replace_range(suffix_start.., &suffix.replace(':', "%3A")); + } + // Citations have no descriptive label; compare the same encoded path on both sides. + events.extend([ + ( + Event::Start(Tag::Link { + link_type: LinkType::Inline, + dest_url: destination.clone().into(), + title: "".into(), + id: "".into(), + }), + span.clone(), + ), + (Event::Text(destination.into()), span.clone()), + (Event::End(TagEnd::Link), span.clone()), + ]); + offset = span.end; + } + if offset < range.end { + events.push(( + Event::Text(self.markdown[offset..range.end].into()), + offset..range.end, + )); + } + Either::Right(events.into_iter()) + }) + } +} + +#[cfg(test)] +#[path = "file_citations_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/markdown_render/file_citations_tests.rs b/codex-rs/tui/src/markdown_render/file_citations_tests.rs new file mode 100644 index 0000000000..54cebcfc6b --- /dev/null +++ b/codex-rs/tui/src/markdown_render/file_citations_tests.rs @@ -0,0 +1,202 @@ +//! Citation rendering, literal Markdown boundaries, and filesystem-path compatibility. + +use crate::markdown::render_markdown_agent_with_links_and_cwd; +use crate::markdown_render::render_markdown_lines_with_width_and_cwd; +use itertools::Itertools; +use pretty_assertions::assert_eq; +use std::path::Path; + +fn rendered_text(markdown: &str, cwd: Option<&Path>) -> String { + render_markdown_agent_with_links_and_cwd(markdown, /*width*/ None, cwd) + .into_iter() + .map(|line| line.line.to_string()) + .join("\n") +} + +#[test] +fn file_citation_paths_preserve_markdown_significant_characters() { + for path in [ + "/tmp/a*b*.txt", + "/tmp/a`b`.txt", + "/tmp/a.txt", + "/tmp/report#L10", + "/tmp/report:10", + "/tmp/report%20final.xlsx", + "/tmp/report?final.xlsx", + ] { + let markdown = format!(":codex-file-citation{{path=\"{path}\"}}"); + assert_eq!(rendered_text(&markdown, /*cwd*/ None), path); + } +} + +#[test] +fn file_url_citation_preserves_literal_query_delimiters_snapshot() { + let cwd = std::env::temp_dir(); + let directory = url::Url::from_directory_path(&cwd).unwrap(); + for (cwd, directory) in [ + (cwd.as_path(), directory.as_str()), + (Path::new("C:/repo"), "file:///C:/repo/"), + (Path::new("C:/repo"), "file://localhost/C:/repo/"), + ] { + let markdown = format!(":codex-file-citation{{path=\"{directory}report?final.xlsx\"}}"); + insta::allow_duplicates! { + insta::assert_snapshot!(rendered_text(&markdown, Some(cwd)), @"report?final.xlsx"); + } + } +} + +#[test] +fn file_citations_inside_code_existing_links_and_html_remain_literal() { + let citation = r#":codex-file-citation{path="/tmp/report.xlsx" purpose="output"}"#; + + for markdown in [ + format!("`{citation}`"), + format!("```text\n{citation}\n```\n"), + ] { + assert_eq!(rendered_text(&markdown, /*cwd*/ None), citation); + } + for markdown in [ + format!(""), + format!(""), + ] { + assert_eq!(rendered_text(&markdown, /*cwd*/ None), markdown); + } + assert_eq!( + rendered_text( + &format!("[{citation}](https://example.com)"), + /*cwd*/ None + ), + format!("{citation} (https://example.com)"), + ); +} + +#[test] +fn file_citations_accept_unquoted_paths_and_trailing_windows_separators() { + for (citation, expected) in [ + ( + r#":codex-file-citation{path="C:\Users\me\.codex\report.xlsx"}"#, + "C:/Users/me/.codex/report.xlsx", + ), + ( + ":codex-file-citation{path=/tmp/a*b*.txt purpose=output}", + "/tmp/a*b*.txt", + ), + ( + r#":codex-file-citation{path="C:\repo\" purpose="output" sheet="Q1=Actual"}"#, + "C:/repo/", + ), + ( + r#":codex-file-citation{path="/tmp/a\"b" label="team's \"report\""}"#, + "/tmp/a\"b", + ), + ] { + assert_eq!(rendered_text(citation, /*cwd*/ None), expected); + } +} + +#[test] +fn file_citations_preserve_escaped_nested_and_reference_directives() { + let citation = ":codex-file-citation{path=/tmp/report.xlsx}"; + + assert_eq!( + rendered_text(&format!(r"\{citation}"), /*cwd*/ None), + citation, + ); + assert_eq!( + rendered_text(&format!(r"\{citation} and {citation}"), /*cwd*/ None), + format!("{citation} and /tmp/report.xlsx"), + ); + + for literal in [ + format!(r#":unsupported{{value="{citation}"}}"#), + format!(":::{citation}"), + format!("codex-file-citation {}", ":x{v=".repeat(16_000)), + format!("codex-file-citation {}}}", ":x{v=a v=".repeat(16_000)), + format!( + "codex-file-citation {} x bad}}", + ":a{k=".repeat(/*n*/ 16_000) + ), + ] { + assert_eq!(rendered_text(&literal, /*cwd*/ None), literal); + } + + // Normal Markdown escaping still applies, but the nested citation is not rendered. + let literal = format!(r#":unsupported{{path="C:\repo\" value="{citation}"}}"#); + let rendered = rendered_text(&literal, /*cwd*/ None); + insta::assert_snapshot!(rendered, @r#":unsupported{path="C:\repo" value=":codex-file-citation{path=/tmp/report.xlsx}"}"#); + + assert_eq!( + rendered_text( + &format!("`{citation}`\n\n[report][file]\n\n[file]: {citation}\n\n{citation}"), + /*cwd*/ None, + ), + format!("{citation}\n\nreport ({citation})\n\n/tmp/report.xlsx"), + ); +} + +#[test] +fn multiple_file_citations_render_without_interpreting_encoded_source() { + let cwd = std::env::temp_dir(); + let second = cwd.join("second.xlsx"); + let markdown = format!( + r#":codex-file-citation{{path="ignored.xlsx"}} :codex-file-citation{{artifact_kind="workbook" path="reports/final%20report.xlsx" purpose="output" sheet="Dashboard" range="A1:D8"}} and ::codex-file-citation{{path="{}"}}"#, + second.display(), + ); + + assert_eq!( + rendered_text(&markdown, Some(&cwd)), + r#":codex-file-citation{path="ignored.xlsx"} reports/final%20report.xlsx and second.xlsx"#, + ); + + // A work budget must not become a fixed count limit on legitimate citations. + let many = [":codex-file-citation{path=/tmp/report.xlsx}"; 512].join(" "); + assert_eq!( + rendered_text(&many, /*cwd*/ None), + ["/tmp/report.xlsx"; 512].join(" "), + ); +} + +#[test] +fn file_citations_preserve_adjacent_entities_and_escaped_punctuation() { + let cwd = std::env::temp_dir(); + for (markdown, expected) in [ + ( + r#"Préface &:codex-file-citation{path="first&.txt"}🙂:codex-file-citation{path="second.txt"}<fin>"#, + "Préface &first&.txt🙂second.txt", + ), + ( + r#"\[:codex-file-citation{path="first.txt"}\]\*:codex-file-citation{path="second.txt"}\!"#, + "[first.txt]*second.txt!", + ), + ] { + assert_eq!(rendered_text(markdown, Some(&cwd)), expected); + } +} + +#[test] +fn file_citation_after_local_link_soft_break_starts_a_new_line_snapshot() { + let cwd = std::env::temp_dir(); + let markdown = format!( + "[first](<{}>)\n:codex-file-citation{{path=\"second.txt\"}}", + cwd.join("first.txt").display(), + ); + let rendered = rendered_text(&markdown, Some(&cwd)); + + insta::assert_snapshot!(rendered, @r" + first (first.txt) + second.txt + "); +} + +#[test] +fn generic_markdown_keeps_assistant_directives_literal() { + let citation = r#":codex-file-citation{path="/tmp/report.xlsx"}"#; + + assert_eq!( + render_markdown_lines_with_width_and_cwd(citation, /*width*/ None, /*cwd*/ None) + .into_iter() + .map(|line| line.line.to_string()) + .join("\n"), + citation, + ); +} diff --git a/codex-rs/tui/src/markdown_render/streaming.rs b/codex-rs/tui/src/markdown_render/streaming.rs index 535774ff72..66c55f9252 100644 --- a/codex-rs/tui/src/markdown_render/streaming.rs +++ b/codex-rs/tui/src/markdown_render/streaming.rs @@ -5,6 +5,7 @@ use super::DecodedTextMerge; use super::Event; +use super::FileCitations; use super::HyperlinkLine; use super::Options; use super::Parser; @@ -38,10 +39,11 @@ pub(crate) fn render_streaming_markdown_lines_with_width_and_cwd( let mut options = Options::empty(); options.insert(Options::ENABLE_STRIKETHROUGH); options.insert(Options::ENABLE_TABLES); - let parser = Parser::new_ext(input, options); + let citations = FileCitations::new(input, options); + let parser = Parser::new_ext(&citations.markdown, options); let has_reference_link_definition = parser.reference_definitions().iter().next().is_some(); let parser = TopLevelBlockTracker { - iter: DecodedTextMerge::new(parser.into_offset_iter()), + iter: DecodedTextMerge::new(citations.events(parser, cwd)), depth: 0, block_count: 0, last_start: 0, diff --git a/codex-rs/tui/src/resume_picker.rs b/codex-rs/tui/src/resume_picker.rs index 7a2e912475..255998ff66 100644 --- a/codex-rs/tui/src/resume_picker.rs +++ b/codex-rs/tui/src/resume_picker.rs @@ -17,7 +17,7 @@ use crate::keymap::RuntimeChordKeymap; use crate::keymap::RuntimeKeymap; use crate::legacy_core::config::Config; use crate::legacy_core::config::edit::ConfigEditsBuilder; -use crate::markdown::append_markdown; +use crate::markdown_render::render_streaming_markdown_lines_with_width_and_cwd as render_assistant; use crate::pager_overlay::Overlay; use crate::session_resume::resolve_session_thread_id; use crate::status::format_directory_display; @@ -3192,7 +3192,7 @@ fn render_transcript_preview_lines( .into(), ], Some(TranscriptPreviewState::Loaded(lines)) => { - render_conversation_preview_lines(lines, width) + render_conversation_preview_lines(lines, width, row.cwd.as_deref()) } None => Vec::new(), }; @@ -3242,6 +3242,7 @@ fn render_expanded_session_details( fn render_conversation_preview_lines( lines: &[TranscriptPreviewLine], width: u16, + cwd: Option<&Path>, ) -> Vec> { if lines.is_empty() { return vec![ @@ -3255,7 +3256,7 @@ fn render_conversation_preview_lines( let mut rendered = Vec::new(); for line in lines { - rendered.extend(render_transcript_content_lines(line, width)); + rendered.extend(render_transcript_content_lines(line, width, cwd)); } let rendered_len = rendered.len(); rendered @@ -3272,7 +3273,11 @@ fn render_conversation_preview_lines( .collect() } -fn render_transcript_content_lines(line: &TranscriptPreviewLine, width: u16) -> Vec> { +fn render_transcript_content_lines( + line: &TranscriptPreviewLine, + width: u16, + cwd: Option<&Path>, +) -> Vec> { let content_width = width.saturating_sub(4) as usize; let lines = match line.speaker { TranscriptPreviewSpeaker::User => vec![conversation_content_line( @@ -3280,10 +3285,11 @@ fn render_transcript_content_lines(line: &TranscriptPreviewLine, width: u16) -> conversation_user_style(), )], TranscriptPreviewSpeaker::Assistant => { - let mut lines = Vec::new(); - append_markdown( - &line.text, /*width*/ None, /*cwd*/ None, &mut lines, - ); + let mut lines = render_assistant(&line.text, /*width*/ None, cwd, &|_| false) + .lines + .into_iter() + .map(|line| line.line) + .collect::>(); for line in &mut lines { *line = conversation_content_line(line.clone(), conversation_assistant_style()); } @@ -5482,7 +5488,9 @@ session_picker_view = "dense" }, TranscriptPreviewLine { speaker: TranscriptPreviewSpeaker::Assistant, - text: String::from("Here are the *last* few lines."), + text: String::from( + r#"Here are the *last* lines: [docs](https://example.com) :codex-file-citation{path="/tmp/codex/report.xlsx"}."#, + ), }, ]), ); diff --git a/codex-rs/tui/src/snapshots/codex_tui__resume_picker__tests__resume_picker_expanded_session.snap b/codex-rs/tui/src/snapshots/codex_tui__resume_picker__tests__resume_picker_expanded_session.snap index 23470e5a6f..9a6332b45b 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__resume_picker__tests__resume_picker_expanded_session.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__resume_picker__tests__resume_picker_expanded_session.snap @@ -11,4 +11,4 @@ expression: rendered │ │ Conversation: │ Show me the recent transcript - └ Here are the last few lines. + └ Here are the last lines: docs (https://example.com) report.xlsx. diff --git a/codex-rs/tui/src/streaming/render_tests.rs b/codex-rs/tui/src/streaming/render_tests.rs index 550215325c..44ad11e55d 100644 --- a/codex-rs/tui/src/streaming/render_tests.rs +++ b/codex-rs/tui/src/streaming/render_tests.rs @@ -109,6 +109,26 @@ fn incremental_render_keeps_final_block_mutable_and_matches_full_render() { assert_debug_snapshot!("incremental_render_representative_stream", render.lines); } +#[test] +fn incremental_file_citations_preserve_metadata_unicode_and_markdown() { + let cwd = test_cwd(); + let rendered_cases = [ + ( + "Quarterly Report.xlsx", + "- :codex-file-citation{artifact_kind=\"workbook\" ", + ), + ("Résumé *final* ✨.xlsx", "- :codex-file-citation{"), + ] + .map(|(filename, prefix)| { + let tail = format!("path=\"{}\"}}\n", cwd.join(filename).display()); + let chunks = ["# Output\n\n", prefix, &tail, "\n", "Continue.\n"]; + let (_, render) = assert_rich_stream_matches_full_render(&chunks, Some(80)); + + render.lines + }); + assert_debug_snapshot!("incremental_file_citations", rendered_cases); +} + #[test] fn growing_single_top_level_blocks_render_and_scan_in_one_pass() { let streams: &[&[&str]] = &[ diff --git a/codex-rs/tui/src/streaming/snapshots/codex_tui__streaming__render__tests__incremental_file_citations.snap b/codex-rs/tui/src/streaming/snapshots/codex_tui__streaming__render__tests__incremental_file_citations.snap new file mode 100644 index 0000000000..46f1f00bfb --- /dev/null +++ b/codex-rs/tui/src/streaming/snapshots/codex_tui__streaming__render__tests__incremental_file_citations.snap @@ -0,0 +1,62 @@ +--- +source: tui/src/streaming/render_tests.rs +expression: rendered_cases +--- +[ + [ + HyperlinkLine { + line: Line::from_iter([ + Span::from("# ").bold().underlined(), + Span::from("Output").bold().underlined(), + ]), + hyperlinks: [], + }, + HyperlinkLine { + line: Line::default(), + hyperlinks: [], + }, + HyperlinkLine { + line: Line::from_iter([ + Span::from("- "), + Span::from("Quarterly Report.xlsx").cyan(), + ]), + hyperlinks: [], + }, + HyperlinkLine { + line: Line::default(), + hyperlinks: [], + }, + HyperlinkLine { + line: Line::from("Continue."), + hyperlinks: [], + }, + ], + [ + HyperlinkLine { + line: Line::from_iter([ + Span::from("# ").bold().underlined(), + Span::from("Output").bold().underlined(), + ]), + hyperlinks: [], + }, + HyperlinkLine { + line: Line::default(), + hyperlinks: [], + }, + HyperlinkLine { + line: Line::from_iter([ + Span::from("- "), + Span::from("Résumé *final* ✨.xlsx").cyan(), + ]), + hyperlinks: [], + }, + HyperlinkLine { + line: Line::default(), + hyperlinks: [], + }, + HyperlinkLine { + line: Line::from("Continue."), + hyperlinks: [], + }, + ], +]