mirror of
https://github.com/openai/codex.git
synced 2026-10-22 17:58:46 +00:00
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
This commit is contained in:
@@ -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<usize>, 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<Item = (Event<'s>, Range<usize>)> {
|
||||
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;
|
||||
@@ -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<b>.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!("<span title='{citation}'>"),
|
||||
format!("<!-- {citation} -->"),
|
||||
] {
|
||||
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<fin>",
|
||||
),
|
||||
(
|
||||
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,
|
||||
);
|
||||
}
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user