mirror of
https://github.com/openai/codex.git
synced 2026-11-08 00:36:31 +00:00
Avoid rerendering streamed code fences (#39061)
## Why The streaming TUI rerendered the entire mutable Markdown block whenever a newline-bearing delta arrived. For long fenced code blocks, this repeatedly parsed and syntax-highlighted the growing fence. ## What changed - Detect open, top-level, language-tagged code fences and append newly completed lines directly to the existing render. - Preserve Syntect parser state across appended lines so multiline syntax and the active theme remain consistent. - Fall back to the canonical Markdown renderer for ambiguous fence syntax, theme changes, incomplete lines, and highlighting limit transitions. ## Testing Added coverage that compares streamed output with full renders across syntax styles, Unicode, links, blank lines, fence variants, theme changes, and highlighting limits. A long-fence test also verifies that the rendered prefix is retained as new lines arrive. GitOrigin-RevId: 036db2b7bc93607083b8001f8b4fa3cbb58c713e
This commit is contained in:
1 parent
80e925c28b
commit
386a7b629c
9 files changed
+583
-14
No files matched your search
@@ -0,0 +1,124 @@
|
||||
//! A conservative fast path for one open, top-level, language-tagged code fence.
|
||||
|
||||
use crate::render::highlight::MAX_HIGHLIGHT_LINE_BYTES;
|
||||
use crate::render::highlight::StreamingCodeHighlighter;
|
||||
use crate::render::highlight::syntax_theme_revision;
|
||||
use crate::terminal_hyperlinks::HyperlinkLine;
|
||||
use ratatui::text::Span;
|
||||
|
||||
/// An append-only fence whose original source is identical to pulldown-cmark's code text.
|
||||
pub(super) struct OpenCodeFence {
|
||||
marker: u8,
|
||||
marker_len: usize,
|
||||
language: String,
|
||||
content_start: usize,
|
||||
source_len: usize,
|
||||
theme_revision: u64,
|
||||
/// Initialized only if another chunk arrives, avoiding duplicate work on one-shot fences.
|
||||
highlighter: Option<StreamingCodeHighlighter>,
|
||||
}
|
||||
|
||||
impl OpenCodeFence {
|
||||
/// Recognize the final mutable top-level block after its canonical render.
|
||||
///
|
||||
/// `source` is the final block's suffix, while `source_len` includes the entire committed
|
||||
/// Markdown source and anchors the retained fence within it.
|
||||
///
|
||||
/// Indentation, CR/NUL normalization, and escaped info strings stay on the canonical path.
|
||||
/// Any line that could end the fence also stays there, including deliberately conservative
|
||||
/// false positives, so this fast path need not duplicate CommonMark's closing-fence grammar.
|
||||
pub(super) fn detect(source: &str, source_len: usize, theme_revision: u64) -> Option<Self> {
|
||||
let marker = *source.as_bytes().first()?;
|
||||
if marker != b'`' && marker != b'~' {
|
||||
return None;
|
||||
}
|
||||
let (opening, code) = source.split_once('\n')?;
|
||||
let marker_len = opening.bytes().take_while(|byte| *byte == marker).count();
|
||||
if marker_len < 3
|
||||
|| !source.ends_with('\n')
|
||||
|| theme_revision != syntax_theme_revision()
|
||||
|| source.contains(['\r', '\0'])
|
||||
{
|
||||
return None;
|
||||
}
|
||||
let info = opening[marker_len..].trim_matches([' ', '\t', '\u{b}', '\u{c}']);
|
||||
if info.contains(['&', '\\']) || (marker == b'`' && info.contains('`')) {
|
||||
return None;
|
||||
}
|
||||
let language = info
|
||||
.split([',', ' ', '\t'])
|
||||
.next()
|
||||
.filter(|language| !language.is_empty())?;
|
||||
// Retain at most one bounded language token, never the streamed code itself.
|
||||
if language.len() > MAX_HIGHLIGHT_LINE_BYTES
|
||||
|| has_possible_closing_line(code, marker, marker_len)
|
||||
{
|
||||
return None;
|
||||
}
|
||||
Some(Self {
|
||||
marker,
|
||||
marker_len,
|
||||
language: language.to_string(),
|
||||
content_start: source_len.checked_sub(code.len())?,
|
||||
source_len,
|
||||
theme_revision,
|
||||
highlighter: None,
|
||||
})
|
||||
}
|
||||
|
||||
/// Append code only while the caller's newline-committed source extends this same fence.
|
||||
///
|
||||
/// Returning `None` consumes the retained state and requires a canonical whole-fence render.
|
||||
/// Successful lines include the empty indent span expected by the canonical Markdown writer.
|
||||
pub(super) fn append(
|
||||
mut self,
|
||||
raw_source: &str,
|
||||
committed_source: &str,
|
||||
) -> Option<(Self, Vec<HyperlinkLine>)> {
|
||||
if self.source_len.checked_add(committed_source.len()) != Some(raw_source.len())
|
||||
|| !raw_source.ends_with(committed_source)
|
||||
|| committed_source.contains(['\r', '\0'])
|
||||
|| has_possible_closing_line(committed_source, self.marker, self.marker_len)
|
||||
|| self.theme_revision != syntax_theme_revision()
|
||||
{
|
||||
return None;
|
||||
}
|
||||
let highlighter = match self.highlighter.take() {
|
||||
Some(highlighter) => highlighter,
|
||||
None => StreamingCodeHighlighter::new(
|
||||
&raw_source[self.content_start..self.source_len],
|
||||
&self.language,
|
||||
self.theme_revision,
|
||||
)?,
|
||||
};
|
||||
let (highlighter, lines) = highlighter.append(committed_source)?;
|
||||
self.highlighter = Some(highlighter);
|
||||
self.source_len = raw_source.len();
|
||||
let lines = lines
|
||||
.into_iter()
|
||||
.map(|mut line| {
|
||||
// The canonical writer installs an empty indent span for top-level fences.
|
||||
line.spans.insert(/*index*/ 0, Span::default());
|
||||
HyperlinkLine::new(line)
|
||||
})
|
||||
.collect();
|
||||
Some((self, lines))
|
||||
}
|
||||
}
|
||||
|
||||
/// Conservatively detect any line that might close the fence.
|
||||
///
|
||||
/// False positives intentionally fall back to CommonMark instead of duplicating its closer rules.
|
||||
fn has_possible_closing_line(source: &str, marker: u8, marker_len: usize) -> bool {
|
||||
source.lines().any(|line| {
|
||||
line.trim_start_matches([' ', '\t'])
|
||||
.bytes()
|
||||
.take_while(|byte| *byte == marker)
|
||||
.count()
|
||||
>= marker_len
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "code_fence_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,147 @@
|
||||
use super::StreamingRender;
|
||||
use super::append;
|
||||
use super::append_rich_and_assert_matches_full;
|
||||
use super::assert_rich_stream_matches_full_render;
|
||||
use super::lines_to_plain_strings;
|
||||
use super::test_cwd;
|
||||
use crate::history_cell::HistoryRenderMode;
|
||||
use crate::render::highlight::MAX_HIGHLIGHT_LINE_BYTES;
|
||||
use insta::assert_debug_snapshot;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn growing_code_fences_preserve_styles_unicode_links_and_blank_lines() {
|
||||
let (_, render) = assert_rich_stream_matches_full_render(
|
||||
&[
|
||||
"Before [link](https://example.com).\n\n```rust,no_run\n",
|
||||
"fn main() {\n",
|
||||
" /* a multiline\n",
|
||||
" comment */\n\n",
|
||||
" println!(\"界 e\u{301} https://example.com\");\n",
|
||||
"}\n",
|
||||
"```\n\n~~~~python\n",
|
||||
"text = \"\"\"a multiline\n",
|
||||
"string\"\"\"\n",
|
||||
"~~~~\n",
|
||||
],
|
||||
Some(18),
|
||||
);
|
||||
assert!(render.open_code_fence.is_none());
|
||||
assert_debug_snapshot!(lines_to_plain_strings(&render.lines), @r#"
|
||||
[
|
||||
"Before link",
|
||||
"(https://example.com).",
|
||||
"",
|
||||
"fn main() {",
|
||||
" /* a multiline",
|
||||
" comment */",
|
||||
"",
|
||||
" println!(\"界 e\u{301} https://example.com\");",
|
||||
"}",
|
||||
"",
|
||||
"text = \"\"\"a multiline",
|
||||
"string\"\"\"",
|
||||
]
|
||||
"#);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn long_open_fence_retains_its_rendered_prefix() {
|
||||
let cwd = test_cwd();
|
||||
let mut source = String::new();
|
||||
let mut render = StreamingRender::new();
|
||||
append_rich_and_assert_matches_full(
|
||||
&mut render,
|
||||
&mut source,
|
||||
"```rust\nfn main() {\n",
|
||||
Some(80),
|
||||
&cwd,
|
||||
);
|
||||
let first_line = render.lines[0].line.spans[1].content.as_ptr();
|
||||
for index in 0..512 {
|
||||
append(
|
||||
&mut render,
|
||||
&mut source,
|
||||
&format!(" let value_{index} = {index};\n"),
|
||||
Some(80),
|
||||
&cwd,
|
||||
HistoryRenderMode::Rich,
|
||||
);
|
||||
assert!(render.open_code_fence.is_some());
|
||||
}
|
||||
assert_eq!(render.lines[0].line.spans[1].content.as_ptr(), first_line);
|
||||
append_rich_and_assert_matches_full(&mut render, &mut source, "}\n```\n", Some(80), &cwd);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ambiguous_and_normalized_fences_use_canonical_semantics() {
|
||||
let streams: &[&[&str]] = &[
|
||||
&["```rust\n", "x\n", "``", "`\n"],
|
||||
&["```rust\n", "x\n", "```\t\n", "after\n"],
|
||||
&["```rust\n", "x\n", " ```` \n", "after\n"],
|
||||
&["~~~rust\n", "x\n", "~~~\u{a0}\n", "after\n"],
|
||||
&["```rust\r\n", "let value = 'é';\r\n", "```\r\n"],
|
||||
&["```rust\n", "let value = '\0';\n", "```\n"],
|
||||
&["```rust,no_run\n", "fn main() {}\n", "```\n"],
|
||||
&[" ```rust\n", " fn main() {}\n", " ```\n"],
|
||||
&["```unknown-language\n", "first\n", "second\n", "```\n"],
|
||||
&[
|
||||
"```markdown\n| A | B |\n",
|
||||
"| --- | --- |\n",
|
||||
"| 1 | 2 |\n```\n\n```rust\nfn first() {}\n",
|
||||
"fn second() {}\n",
|
||||
"```\n",
|
||||
],
|
||||
];
|
||||
for chunks in streams {
|
||||
assert_rich_stream_matches_full_render(chunks, Some(40));
|
||||
}
|
||||
for indent in 0..=3 {
|
||||
let closing = format!("{}~~~~~\t\n", " ".repeat(indent));
|
||||
assert_rich_stream_matches_full_render(
|
||||
&["~~~~rust\n", "x\n", &closing, "after\n"],
|
||||
Some(40),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn crossing_a_highlight_limit_removes_previously_highlighted_colors() {
|
||||
let cwd = test_cwd();
|
||||
let mut source = String::new();
|
||||
let mut render = StreamingRender::new();
|
||||
for chunk in [
|
||||
"```rust\nlet first = 1;\n".to_string(),
|
||||
format!("{}\n", "x".repeat(MAX_HIGHLIGHT_LINE_BYTES + 1)),
|
||||
"let second = 2;\n".to_string(),
|
||||
"```\n".to_string(),
|
||||
] {
|
||||
append_rich_and_assert_matches_full(&mut render, &mut source, &chunk, Some(80), &cwd);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recomputing_after_resize_or_mode_change_drops_fence_state() {
|
||||
let (mut source, mut render) =
|
||||
assert_rich_stream_matches_full_render(&["```rust\nfn first() {}\n"], Some(80));
|
||||
assert!(render.open_code_fence.is_some());
|
||||
let cwd = test_cwd();
|
||||
render.recompute(
|
||||
&source,
|
||||
Some(20),
|
||||
&cwd,
|
||||
HistoryRenderMode::Rich,
|
||||
/*inline_visualization_context*/ None,
|
||||
);
|
||||
assert!(render.open_code_fence.is_none());
|
||||
append_rich_and_assert_matches_full(
|
||||
&mut render,
|
||||
&mut source,
|
||||
"fn second() {}\n",
|
||||
Some(20),
|
||||
&cwd,
|
||||
);
|
||||
assert!(render.open_code_fence.is_some());
|
||||
render.clear();
|
||||
assert!(render.open_code_fence.is_none());
|
||||
}
|
||||
@@ -0,0 +1,69 @@
|
||||
use super::OpenCodeFence;
|
||||
use super::has_possible_closing_line;
|
||||
use crate::render::highlight::MAX_HIGHLIGHT_LINE_BYTES;
|
||||
use crate::render::highlight::syntax_theme_revision;
|
||||
|
||||
#[test]
|
||||
fn detects_only_source_preserving_open_fences() {
|
||||
for source in [
|
||||
"```rust,no_run\nfn main() {}\n",
|
||||
"~~~~python title=demo\nvalue = 1\n",
|
||||
"```\u{b}rust\u{c}\nfn main() {}\n",
|
||||
"```unknown-language\ntext\n",
|
||||
] {
|
||||
let fence =
|
||||
OpenCodeFence::detect(source, source.len(), syntax_theme_revision()).expect(source);
|
||||
assert!(fence.highlighter.is_none());
|
||||
}
|
||||
for source in [
|
||||
" ```rust\ncode\n",
|
||||
"```\ncode\n",
|
||||
"```rust\r\ncode\r\n",
|
||||
"```rust\n'\0'\n",
|
||||
"```rust\ncode\n",
|
||||
"```rust\\,no_run\ncode\n",
|
||||
"```rust\npartial",
|
||||
] {
|
||||
assert!(
|
||||
OpenCodeFence::detect(source, source.len(), syntax_theme_revision()).is_none(),
|
||||
"{source:?}"
|
||||
);
|
||||
}
|
||||
let source = format!("```{}\n", "x".repeat(MAX_HIGHLIGHT_LINE_BYTES + 1));
|
||||
assert!(OpenCodeFence::detect(&source, source.len(), syntax_theme_revision()).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn every_possible_closer_returns_to_the_canonical_parser() {
|
||||
let opening = "~~~~rust\n";
|
||||
assert!(!has_possible_closing_line(
|
||||
"~~~\n```\n",
|
||||
/*marker*/ b'~',
|
||||
/*marker_len*/ 4
|
||||
));
|
||||
for line in [
|
||||
"~~~~\n",
|
||||
" ~~~~~~ \n",
|
||||
"~~~~\t\n",
|
||||
"~~~~\u{a0}\n",
|
||||
"\t~~~~\n",
|
||||
" ~~~~\n",
|
||||
"~~~~ trailing text\n",
|
||||
] {
|
||||
let source = format!("{opening}{line}");
|
||||
assert!(OpenCodeFence::detect(&source, source.len(), syntax_theme_revision()).is_none());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lazy_initialization_rejects_a_changed_theme() {
|
||||
let source = "```rust\nfn first() {}\n";
|
||||
let mut fence = OpenCodeFence::detect(source, source.len(), syntax_theme_revision()).unwrap();
|
||||
fence.theme_revision = fence.theme_revision.wrapping_sub(1);
|
||||
let appended = "fn second() {}\n";
|
||||
assert!(
|
||||
fence
|
||||
.append(&format!("{source}{appended}"), appended)
|
||||
.is_none()
|
||||
);
|
||||
}
|
||||
@@ -17,6 +17,7 @@ use std::time::Instant;
|
||||
use crate::markdown_stream::MarkdownStreamCollector;
|
||||
use crate::terminal_hyperlinks::HyperlinkLine;
|
||||
pub(crate) mod chunking;
|
||||
mod code_fence;
|
||||
pub(crate) mod commit_tick;
|
||||
pub(crate) mod controller;
|
||||
mod render;
|
||||
|
||||
@@ -3,12 +3,14 @@
|
||||
//! Completed top-level blocks are retained while the final block stays mutable, avoiding repeated
|
||||
//! rendering of the stable prefix as newline-bearing deltas arrive.
|
||||
|
||||
use super::code_fence::OpenCodeFence;
|
||||
use crate::history_cell::HistoryRenderMode;
|
||||
use crate::history_cell::raw_lines_from_source;
|
||||
use crate::inline_visualization::InlineVisualizationContext;
|
||||
use crate::inline_visualization::contains_inline_visualization;
|
||||
use crate::markdown::render_markdown_agent_with_links_cwd_and_visualizations;
|
||||
use crate::markdown::render_streaming_markdown_agent_with_links_and_cwd;
|
||||
use crate::render::highlight::syntax_theme_revision;
|
||||
use crate::terminal_hyperlinks::HyperlinkLine;
|
||||
use crate::terminal_hyperlinks::plain_hyperlink_lines;
|
||||
use ratatui::text::Line;
|
||||
@@ -28,6 +30,8 @@ pub(super) struct StreamingRender {
|
||||
has_reference_link_definition: bool,
|
||||
/// Inline visualization directives require source-wide rewriting once one is committed.
|
||||
has_inline_visualization_directive: bool,
|
||||
/// Parser state for a directly appendable, open top-level code fence.
|
||||
open_code_fence: Option<OpenCodeFence>,
|
||||
}
|
||||
|
||||
impl StreamingRender {
|
||||
@@ -38,6 +42,7 @@ impl StreamingRender {
|
||||
stable_rendered_len: 0,
|
||||
has_reference_link_definition: false,
|
||||
has_inline_visualization_directive: false,
|
||||
open_code_fence: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -47,6 +52,7 @@ impl StreamingRender {
|
||||
self.stable_rendered_len = 0;
|
||||
self.has_reference_link_definition = false;
|
||||
self.has_inline_visualization_directive = false;
|
||||
self.open_code_fence = None;
|
||||
}
|
||||
|
||||
/// Re-render the full source and reset both stable-prefix boundaries.
|
||||
@@ -61,6 +67,7 @@ impl StreamingRender {
|
||||
render_mode: HistoryRenderMode,
|
||||
inline_visualization_context: Option<&InlineVisualizationContext>,
|
||||
) {
|
||||
self.open_code_fence = None;
|
||||
self.has_inline_visualization_directive = contains_inline_visualization(source);
|
||||
self.lines = match (render_mode, inline_visualization_context) {
|
||||
(HistoryRenderMode::Rich, None) if !self.has_inline_visualization_directive => {
|
||||
@@ -130,7 +137,16 @@ impl StreamingRender {
|
||||
return;
|
||||
}
|
||||
|
||||
if let Some(fence) = self.open_code_fence.take()
|
||||
&& let Some((fence, lines)) = fence.append(raw_source, committed_source)
|
||||
{
|
||||
self.lines.extend(lines);
|
||||
self.open_code_fence = Some(fence);
|
||||
return;
|
||||
}
|
||||
|
||||
let pending_source = &raw_source[self.stable_source_len..];
|
||||
let theme_revision = syntax_theme_revision();
|
||||
let pending =
|
||||
render_streaming_markdown_agent_with_links_and_cwd(pending_source, width, Some(cwd));
|
||||
if pending.has_reference_link_definition {
|
||||
@@ -145,6 +161,13 @@ impl StreamingRender {
|
||||
return;
|
||||
}
|
||||
|
||||
let final_block_start = pending.last_top_level_block_start.unwrap_or(/*default*/ 0);
|
||||
self.open_code_fence = OpenCodeFence::detect(
|
||||
&pending_source[final_block_start..],
|
||||
raw_source.len(),
|
||||
theme_revision,
|
||||
);
|
||||
|
||||
let mut newly_stable_rendered_len = None;
|
||||
if let Some(boundary) = pending.last_top_level_block_start {
|
||||
let newly_stable_source = &pending_source[..boundary];
|
||||
|
||||
@@ -11,6 +11,9 @@ use pretty_assertions::assert_eq;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
#[path = "code_fence_render_tests.rs"]
|
||||
mod code_fence_tests;
|
||||
|
||||
fn test_cwd() -> PathBuf {
|
||||
std::env::temp_dir()
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user