From 33eb2d83ed1a9f9ac7ca533178fa866cd4416edc Mon Sep 17 00:00:00 2001 From: MartinYe1234 <52641447+MartinYe1234@users.noreply.github.com> Date: Thu, 21 May 2026 09:16:49 -0700 Subject: [PATCH] Skip syntax highlighting for read_file outline responses (#57287) When `read_file` returns a file outline instead of full contents, the result is now wrapped in a plain fenced code block rather than tagging it with the file's path. Previously, the outline was wrapped in a fenced block tagged with the file path (e.g. `` ```crates/agent/src/tools/read_file_tool.rs ``). Because the tag contains a slash, the markdown parser routed it through `CodeBlockKind::FencedSrc`, resolved the file's language by path, and ran the language's tree-sitter parser against the outline on every paint. The outline is structural (e.g. `fn foo [L10-20]`), not actual source for the file's language, so the parse was both expensive and produced incorrect highlighting. Because GPUI rebuilds the visible element tree on every window paint, anything that triggers a repaint (cursor blink in the focused message editor, animations, the turn timer, etc.) would re-run the tree-sitter parse on the entire outline, throttling the frame rate while the tool call was expanded. This change adds an `is_outline_response` flag in `ReadFileTool::run` and, when set, passes an empty tag to `MarkdownCodeBlock` so the renderer sees `CodeBlockKind::Fenced` (no language). Plain monospace formatting is preserved; the path tag is still used for the non-outline (full file) case. Adds two regression tests: one asserting the outline path uses an untagged fenced block, and one asserting the full-file path keeps the path tag (so the next person fixing this doesn't accidentally strip the tag everywhere). Also includes a small markdown-rendering follow-up: `MarkdownElementBuilder::push_text` was recomputing the base text style (cloning `base_text_style` and walking the style stack) twice per highlighted token. For a code block with hundreds of highlight tokens, that's hundreds of redundant `TextStyle` clones per paint. The style stack does not change while runs are being attributed, so the style is now computed once outside the loop and reused. Closes AI-234 Release Notes: - Improved scrolling smoothness in the agent panel when a `read_file` tool call with a large file outline is expanded. --- crates/agent/src/tools/read_file_tool.rs | 139 ++++++++++++++++++++++- crates/markdown/src/markdown.rs | 21 ++-- 2 files changed, 146 insertions(+), 14 deletions(-) diff --git a/crates/agent/src/tools/read_file_tool.rs b/crates/agent/src/tools/read_file_tool.rs index 95301ee152..4638b17031 100644 --- a/crates/agent/src/tools/read_file_tool.rs +++ b/crates/agent/src/tools/read_file_tool.rs @@ -338,6 +338,7 @@ impl AgentTool for ReadFileTool { } let mut anchor = None; + let mut is_outline_response = false; // Check if specific line ranges are provided let result = if input.start_line.is_some() || input.end_line.is_some() { @@ -377,6 +378,8 @@ impl AgentTool for ReadFileTool { log.buffer_read(buffer.clone(), cx); }); + is_outline_response = buffer_content.is_outline; + if buffer_content.is_outline { Ok(formatdoc! {" SUCCESS: File outline retrieved. This file is too large to read all at once, so the outline below shows the file's structure with line numbers. @@ -409,11 +412,12 @@ impl AgentTool for ReadFileTool { } if let Ok(LanguageModelToolResultContent::Text(text)) = &result { let text: &str = text; - let markdown = MarkdownCodeBlock { - tag: &input.path, - text, - } - .to_string(); + // For outline responses, omit the path tag so the markdown renderer + // does not invoke tree-sitter syntax highlighting against pseudo-code + // outline text. The outline is not valid source for the file's language, + // so highlighting would be both expensive and incorrect. + let tag: &str = if is_outline_response { "" } else { &input.path }; + let markdown = MarkdownCodeBlock { tag, text }.to_string(); event_stream.update_fields(acp::ToolCallUpdateFields::new().content(vec![ acp::ToolCallContent::Content(acp::Content::new(markdown)), ])); @@ -610,6 +614,131 @@ mod test { ); } + // The outline returned for a large file is not valid source for the file's + // language, so the UI-side markdown wrapping must omit the path tag. + // Otherwise the markdown renderer routes the fenced block through + // `CodeBlockKind::FencedSrc`, resolves the file's language, and runs + // tree-sitter against pseudo-code outline text on every paint. + #[gpui::test] + async fn test_outline_response_uses_untagged_code_block(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/root"), + json!({ + "large_file.rs": (0..1000).map(|i| format!("struct Test{} {{\n a: u32,\n b: usize,\n}}", i)).collect::>().join("\n") + }), + ) + .await; + let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await; + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + language_registry.add(language::rust_lang()); + let action_log = cx.new(|_| ActionLog::new(project.clone())); + let tool = Arc::new(ReadFileTool::new(project, action_log, true)); + let (event_stream, mut rx) = ToolCallEventStream::test(); + + let result = cx + .update(|cx| { + let input = ReadFileToolInput { + path: "root/large_file.rs".into(), + start_line: None, + end_line: None, + }; + tool.clone() + .run(ToolInput::resolved(input), event_stream, cx) + }) + .await + .unwrap(); + + // Sanity-check: the file is large enough to trigger the outline branch. + assert!( + result + .to_str() + .unwrap() + .starts_with("SUCCESS: File outline retrieved."), + "expected outline response, got: {:?}", + result.to_str().unwrap() + ); + + // The first update carries the location; the second carries the + // markdown content destined for the tool-call UI. + let _location_update = rx.expect_update_fields().await; + let content_update = rx.expect_update_fields().await; + let content_blocks = content_update.content.expect("expected content update"); + let acp::ToolCallContent::Content(content) = content_blocks + .first() + .expect("expected at least one content block") + else { + panic!("expected ContentBlock, got {:?}", content_blocks.first()); + }; + let acp::ContentBlock::Text(text) = &content.content else { + panic!("expected text content block, got {:?}", content.content); + }; + + assert!( + text.text.starts_with("```\n"), + "outline response must use an untagged fenced code block; got first line: {:?}", + text.text.lines().next() + ); + assert!( + !text.text.starts_with("```root/"), + "outline response must not include the file path as a code block tag" + ); + } + + // The full-file (non-outline) response should still tag the code block + // with the file path so the markdown renderer can resolve the file's + // language for syntax highlighting. + #[gpui::test] + async fn test_full_file_response_keeps_path_tag(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/root"), + json!({ + "small_file.rs": "fn main() {}" + }), + ) + .await; + let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await; + let action_log = cx.new(|_| ActionLog::new(project.clone())); + let tool = Arc::new(ReadFileTool::new(project, action_log, true)); + let (event_stream, mut rx) = ToolCallEventStream::test(); + + cx.update(|cx| { + let input = ReadFileToolInput { + path: "root/small_file.rs".into(), + start_line: None, + end_line: None, + }; + tool.clone() + .run(ToolInput::resolved(input), event_stream, cx) + }) + .await + .unwrap(); + + let _location_update = rx.expect_update_fields().await; + let content_update = rx.expect_update_fields().await; + let content_blocks = content_update.content.expect("expected content update"); + let acp::ToolCallContent::Content(content) = content_blocks + .first() + .expect("expected at least one content block") + else { + panic!("expected ContentBlock, got {:?}", content_blocks.first()); + }; + let acp::ContentBlock::Text(text) = &content.content else { + panic!("expected text content block, got {:?}", content.content); + }; + + assert!( + text.text.starts_with("```root/small_file.rs\n"), + "full-file response must tag the code block with the file path; got first line: {:?}", + text.text.lines().next() + ); + } + // When a worktree is named "foo" and contains a subdirectory also named "foo", // read_file({"path": "foo/test.txt"}) should return the file at the worktree // root (as the tool schema promises), not the one inside the foo/ subdirectory. diff --git a/crates/markdown/src/markdown.rs b/crates/markdown/src/markdown.rs index 1f763d390b..f8ca35d722 100644 --- a/crates/markdown/src/markdown.rs +++ b/crates/markdown/src/markdown.rs @@ -2862,33 +2862,36 @@ impl MarkdownElementBuilder { self.pending_line.text.push_str(text); self.current_source_index = source_range.end; + // Compute the base text style once + let text_style = self.text_style(); + if let Some(Some(language)) = self.code_block_stack.last() { let mut offset = 0; for (range, highlight_id) in language.highlight_text(&Rope::from(text), 0..text.len()) { if range.start > offset { self.pending_line .runs - .push(self.text_style().to_run(range.start - offset)); + .push(text_style.to_run(range.start - offset)); } - let mut run_style = self.text_style(); + let run_len = range.len(); if let Some(highlight) = self.syntax_theme.get(highlight_id).cloned() { - run_style = run_style.highlight(highlight); + self.pending_line + .runs + .push(text_style.clone().highlight(highlight).to_run(run_len)); + } else { + self.pending_line.runs.push(text_style.to_run(run_len)); } - - self.pending_line.runs.push(run_style.to_run(range.len())); offset = range.end; } if offset < text.len() { self.pending_line .runs - .push(self.text_style().to_run(text.len() - offset)); + .push(text_style.to_run(text.len() - offset)); } } else { - self.pending_line - .runs - .push(self.text_style().to_run(text.len())); + self.pending_line.runs.push(text_style.to_run(text.len())); } }