From fceba6c79540677c2504d2c22191963b6170591a Mon Sep 17 00:00:00 2001 From: Oleksiy Syvokon Date: Mon, 16 Jun 2025 17:28:18 +0300 Subject: [PATCH] edit_file: Add diff-fenced output format (#32737) This format is enabled for Google models as they seem to prefer it. A relevant unit eval's pass rate has increased from 0.77 to 0.98. Diff-fenced format looks like this (markdown fences and a line hint are optional): ```diff <<<<<<< SEARCH line=42 ... ======= ... >>>>>>> REPLACE ``` Release Notes: - Agent: Gemini models now use the diff-fenced format when making edits --- crates/assistant_tools/src/edit_agent.rs | 52 +- .../src/edit_agent/edit_parser.rs | 567 ++++++++++++++++-- .../assistant_tools/src/edit_agent/evals.rs | 31 +- .../src/edit_agent/streaming_fuzzy_matcher.rs | 4 + crates/assistant_tools/src/edit_file_tool.rs | 12 +- .../edit_file_prompt_diff_fenced.hbs | 77 +++ ...le_prompt.hbs => edit_file_prompt_xml.hbs} | 0 script/danger/dangerfile.ts | 3 +- 8 files changed, 667 insertions(+), 79 deletions(-) create mode 100644 crates/assistant_tools/src/templates/edit_file_prompt_diff_fenced.hbs rename crates/assistant_tools/src/templates/{edit_file_prompt.hbs => edit_file_prompt_xml.hbs} (100%) diff --git a/crates/assistant_tools/src/edit_agent.rs b/crates/assistant_tools/src/edit_agent.rs index f891a769c5..c2540633f7 100644 --- a/crates/assistant_tools/src/edit_agent.rs +++ b/crates/assistant_tools/src/edit_agent.rs @@ -8,6 +8,7 @@ use crate::{Template, Templates}; use anyhow::Result; use assistant_tool::ActionLog; use create_file_parser::{CreateFileParser, CreateFileParserEvent}; +pub use edit_parser::EditFormat; use edit_parser::{EditParser, EditParserEvent, EditParserMetrics}; use futures::{ Stream, StreamExt, @@ -41,13 +42,23 @@ impl Template for CreateFilePromptTemplate { } #[derive(Serialize)] -struct EditFilePromptTemplate { +struct EditFileXmlPromptTemplate { path: Option, edit_description: String, } -impl Template for EditFilePromptTemplate { - const TEMPLATE_NAME: &'static str = "edit_file_prompt.hbs"; +impl Template for EditFileXmlPromptTemplate { + const TEMPLATE_NAME: &'static str = "edit_file_prompt_xml.hbs"; +} + +#[derive(Serialize)] +struct EditFileDiffFencedPromptTemplate { + path: Option, + edit_description: String, +} + +impl Template for EditFileDiffFencedPromptTemplate { + const TEMPLATE_NAME: &'static str = "edit_file_prompt_diff_fenced.hbs"; } #[derive(Clone, Debug, PartialEq, Eq)] @@ -70,6 +81,7 @@ pub struct EditAgent { action_log: Entity, project: Entity, templates: Arc, + edit_format: EditFormat, } impl EditAgent { @@ -78,12 +90,14 @@ impl EditAgent { project: Entity, action_log: Entity, templates: Arc, + edit_format: EditFormat, ) -> Self { EditAgent { model, project, action_log, templates, + edit_format, } } @@ -209,14 +223,23 @@ impl EditAgent { let this = self.clone(); let (events_tx, events_rx) = mpsc::unbounded(); let conversation = conversation.clone(); + let edit_format = self.edit_format; let output = cx.spawn(async move |cx| { let snapshot = buffer.read_with(cx, |buffer, _| buffer.snapshot())?; let path = cx.update(|cx| snapshot.resolve_file_path(cx, true))?; - let prompt = EditFilePromptTemplate { - path, - edit_description, - } - .render(&this.templates)?; + let prompt = match edit_format { + EditFormat::XmlTags => EditFileXmlPromptTemplate { + path, + edit_description, + } + .render(&this.templates)?, + EditFormat::DiffFenced => EditFileDiffFencedPromptTemplate { + path, + edit_description, + } + .render(&this.templates)?, + }; + let edit_chunks = this .request(conversation, CompletionIntent::EditFile, prompt, cx) .await?; @@ -236,7 +259,7 @@ impl EditAgent { self.action_log .update(cx, |log, cx| log.buffer_read(buffer.clone(), cx))?; - let (output, edit_events) = Self::parse_edit_chunks(edit_chunks, cx); + let (output, edit_events) = Self::parse_edit_chunks(edit_chunks, self.edit_format, cx); let mut edit_events = edit_events.peekable(); while let Some(edit_event) = Pin::new(&mut edit_events).peek().await { // Skip events until we're at the start of a new edit. @@ -350,6 +373,7 @@ impl EditAgent { fn parse_edit_chunks( chunks: impl 'static + Send + Stream>, + edit_format: EditFormat, cx: &mut AsyncApp, ) -> ( Task>, @@ -359,7 +383,7 @@ impl EditAgent { let output = cx.background_spawn(async move { pin_mut!(chunks); - let mut parser = EditParser::new(); + let mut parser = EditParser::new(edit_format); let mut raw_edits = String::new(); while let Some(chunk) = chunks.next().await { match chunk { @@ -1355,7 +1379,13 @@ mod tests { let project = Project::test(FakeFs::new(cx.executor()), [], cx).await; let model = Arc::new(FakeLanguageModel::default()); let action_log = cx.new(|_| ActionLog::new(project.clone())); - EditAgent::new(model, project, action_log, Templates::new()) + EditAgent::new( + model, + project, + action_log, + Templates::new(), + EditFormat::XmlTags, + ) } #[gpui::test(iterations = 10)] diff --git a/crates/assistant_tools/src/edit_agent/edit_parser.rs b/crates/assistant_tools/src/edit_agent/edit_parser.rs index f477443a5b..5ba3931c56 100644 --- a/crates/assistant_tools/src/edit_agent/edit_parser.rs +++ b/crates/assistant_tools/src/edit_agent/edit_parser.rs @@ -1,13 +1,18 @@ +use anyhow::bail; use derive_more::{Add, AddAssign}; +use language_model::LanguageModel; use regex::Regex; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use smallvec::SmallVec; -use std::{mem, ops::Range}; +use std::{mem, ops::Range, str::FromStr, sync::Arc}; const OLD_TEXT_END_TAG: &str = ""; const NEW_TEXT_END_TAG: &str = ""; const EDITS_END_TAG: &str = ""; +const SEARCH_MARKER: &str = "<<<<<<< SEARCH"; +const SEPARATOR_MARKER: &str = "======="; +const REPLACE_MARKER: &str = ">>>>>>> REPLACE"; const END_TAGS: [&str; 3] = [OLD_TEXT_END_TAG, NEW_TEXT_END_TAG, EDITS_END_TAG]; #[derive(Debug)] @@ -31,44 +36,153 @@ pub struct EditParserMetrics { pub mismatched_tags: usize, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum EditFormat { + /// XML-like tags: + /// ... + /// ... + XmlTags, + /// Diff-fenced format, in which: + /// - Text before the SEARCH marker is ignored + /// - Fences are optional + /// - Line hint is optional. + /// + /// Example: + /// + /// ```diff + /// <<<<<<< SEARCH line=42 + /// ... + /// ======= + /// ... + /// >>>>>>> REPLACE + /// ``` + DiffFenced, +} + +impl FromStr for EditFormat { + type Err = anyhow::Error; + + fn from_str(s: &str) -> anyhow::Result { + match s.to_lowercase().as_str() { + "xml_tags" | "xml" => Ok(EditFormat::XmlTags), + "diff_fenced" | "diff-fenced" | "diff" => Ok(EditFormat::DiffFenced), + _ => bail!("Unknown EditFormat: {}", s), + } + } +} + +impl EditFormat { + /// Return an optimal edit format for the language model + pub fn from_model(model: Arc) -> anyhow::Result { + if model.provider_id().0 == "google" { + Ok(EditFormat::DiffFenced) + } else { + Ok(EditFormat::XmlTags) + } + } + + /// Return an optimal edit format for the language model, + /// with the ability to override it by setting the + /// `ZED_EDIT_FORMAT` environment variable + #[allow(dead_code)] + pub fn from_env(model: Arc) -> anyhow::Result { + let default = EditFormat::from_model(model)?; + std::env::var("ZED_EDIT_FORMAT").map_or(Ok(default), |s| EditFormat::from_str(&s)) + } +} + +pub trait EditFormatParser: Send + std::fmt::Debug { + fn push(&mut self, chunk: &str) -> SmallVec<[EditParserEvent; 1]>; + fn take_metrics(&mut self) -> EditParserMetrics; +} + #[derive(Debug)] -pub struct EditParser { - state: EditParserState, +pub struct XmlEditParser { + state: XmlParserState, buffer: String, metrics: EditParserMetrics, } #[derive(Debug, PartialEq)] -enum EditParserState { +enum XmlParserState { Pending, WithinOldText { start: bool, line_hint: Option }, AfterOldText, WithinNewText { start: bool }, } -impl EditParser { +#[derive(Debug)] +pub struct DiffFencedEditParser { + state: DiffParserState, + buffer: String, + metrics: EditParserMetrics, +} + +#[derive(Debug, PartialEq)] +enum DiffParserState { + Pending, + WithinSearch { start: bool, line_hint: Option }, + WithinReplace { start: bool }, +} + +/// Main parser that delegates to format-specific parsers +pub struct EditParser { + parser: Box, +} + +impl XmlEditParser { pub fn new() -> Self { - EditParser { - state: EditParserState::Pending, + XmlEditParser { + state: XmlParserState::Pending, buffer: String::new(), metrics: EditParserMetrics::default(), } } - pub fn push(&mut self, chunk: &str) -> SmallVec<[EditParserEvent; 1]> { + fn find_end_tag(&self) -> Option> { + let (tag, start_ix) = END_TAGS + .iter() + .flat_map(|tag| Some((tag, self.buffer.find(tag)?))) + .min_by_key(|(_, ix)| *ix)?; + Some(start_ix..start_ix + tag.len()) + } + + fn ends_with_tag_prefix(&self) -> bool { + let mut end_prefixes = END_TAGS + .iter() + .flat_map(|tag| (1..tag.len()).map(move |i| &tag[..i])) + .chain(["\n"]); + end_prefixes.any(|prefix| self.buffer.ends_with(&prefix)) + } + + fn parse_line_hint(&self, tag: &str) -> Option { + use std::sync::LazyLock; + static LINE_HINT_REGEX: LazyLock = + LazyLock::new(|| Regex::new(r#"line=(?:"?)(\d+)"#).unwrap()); + + LINE_HINT_REGEX + .captures(tag) + .and_then(|caps| caps.get(1)) + .and_then(|m| m.as_str().parse::().ok()) + } +} + +impl EditFormatParser for XmlEditParser { + fn push(&mut self, chunk: &str) -> SmallVec<[EditParserEvent; 1]> { self.buffer.push_str(chunk); let mut edit_events = SmallVec::new(); loop { match &mut self.state { - EditParserState::Pending => { + XmlParserState::Pending => { if let Some(start) = self.buffer.find("') { let tag_end = start + tag_end + 1; let tag = &self.buffer[start..tag_end]; let line_hint = self.parse_line_hint(tag); self.buffer.drain(..tag_end); - self.state = EditParserState::WithinOldText { + self.state = XmlParserState::WithinOldText { start: true, line_hint, }; @@ -79,7 +193,7 @@ impl EditParser { break; } } - EditParserState::WithinOldText { start, line_hint } => { + XmlParserState::WithinOldText { start, line_hint } => { if !self.buffer.is_empty() { if *start && self.buffer.starts_with('\n') { self.buffer.remove(0); @@ -100,7 +214,7 @@ impl EditParser { } self.buffer.drain(..tag_range.end); - self.state = EditParserState::AfterOldText; + self.state = XmlParserState::AfterOldText; edit_events.push(EditParserEvent::OldTextChunk { chunk, done: true, @@ -117,15 +231,15 @@ impl EditParser { break; } } - EditParserState::AfterOldText => { + XmlParserState::AfterOldText => { if let Some(start) = self.buffer.find("") { self.buffer.drain(..start + "".len()); - self.state = EditParserState::WithinNewText { start: true }; + self.state = XmlParserState::WithinNewText { start: true }; } else { break; } } - EditParserState::WithinNewText { start } => { + XmlParserState::WithinNewText { start } => { if !self.buffer.is_empty() { if *start && self.buffer.starts_with('\n') { self.buffer.remove(0); @@ -145,7 +259,7 @@ impl EditParser { } self.buffer.drain(..tag_range.end); - self.state = EditParserState::Pending; + self.state = XmlParserState::Pending; edit_events.push(EditParserEvent::NewTextChunk { chunk, done: true }); } else { if !self.ends_with_tag_prefix() { @@ -162,34 +276,163 @@ impl EditParser { edit_events } - fn find_end_tag(&self) -> Option> { - let (tag, start_ix) = END_TAGS - .iter() - .flat_map(|tag| Some((tag, self.buffer.find(tag)?))) - .min_by_key(|(_, ix)| *ix)?; - Some(start_ix..start_ix + tag.len()) + fn take_metrics(&mut self) -> EditParserMetrics { + std::mem::take(&mut self.metrics) + } +} + +impl DiffFencedEditParser { + pub fn new() -> Self { + DiffFencedEditParser { + state: DiffParserState::Pending, + buffer: String::new(), + metrics: EditParserMetrics::default(), + } } - fn ends_with_tag_prefix(&self) -> bool { - let mut end_prefixes = END_TAGS + fn ends_with_diff_marker_prefix(&self) -> bool { + let diff_markers = [SEPARATOR_MARKER, REPLACE_MARKER]; + let mut diff_prefixes = diff_markers .iter() - .flat_map(|tag| (1..tag.len()).map(move |i| &tag[..i])) + .flat_map(|marker| (1..marker.len()).map(move |i| &marker[..i])) .chain(["\n"]); - end_prefixes.any(|prefix| self.buffer.ends_with(&prefix)) + diff_prefixes.any(|prefix| self.buffer.ends_with(&prefix)) } - fn parse_line_hint(&self, tag: &str) -> Option { - static LINE_HINT_REGEX: std::sync::LazyLock = - std::sync::LazyLock::new(|| Regex::new(r#"line=(?:"?)(\d+)"#).unwrap()); + fn parse_line_hint(&self, search_line: &str) -> Option { + use regex::Regex; + use std::sync::LazyLock; + static LINE_HINT_REGEX: LazyLock = + LazyLock::new(|| Regex::new(r#"line=(?:"?)(\d+)"#).unwrap()); LINE_HINT_REGEX - .captures(tag) + .captures(search_line) .and_then(|caps| caps.get(1)) .and_then(|m| m.as_str().parse::().ok()) } +} - pub fn finish(self) -> EditParserMetrics { - self.metrics +impl EditFormatParser for DiffFencedEditParser { + fn push(&mut self, chunk: &str) -> SmallVec<[EditParserEvent; 1]> { + self.buffer.push_str(chunk); + + let mut edit_events = SmallVec::new(); + loop { + match &mut self.state { + DiffParserState::Pending => { + if let Some(diff) = self.buffer.find(SEARCH_MARKER) { + let search_end = diff + SEARCH_MARKER.len(); + if let Some(newline_pos) = self.buffer[search_end..].find('\n') { + let search_line = &self.buffer[diff..search_end + newline_pos]; + let line_hint = self.parse_line_hint(search_line); + self.buffer.drain(..search_end + newline_pos + 1); + self.state = DiffParserState::WithinSearch { + start: true, + line_hint, + }; + } else { + break; + } + } else { + break; + } + } + DiffParserState::WithinSearch { start, line_hint } => { + if !self.buffer.is_empty() { + if *start && self.buffer.starts_with('\n') { + self.buffer.remove(0); + } + *start = false; + } + + let line_hint = *line_hint; + if let Some(separator_pos) = self.buffer.find(SEPARATOR_MARKER) { + let mut chunk = self.buffer[..separator_pos].to_string(); + if chunk.ends_with('\n') { + chunk.pop(); + } + + let separator_end = separator_pos + SEPARATOR_MARKER.len(); + if let Some(newline_pos) = self.buffer[separator_end..].find('\n') { + self.buffer.drain(..separator_end + newline_pos + 1); + self.state = DiffParserState::WithinReplace { start: true }; + edit_events.push(EditParserEvent::OldTextChunk { + chunk, + done: true, + line_hint, + }); + } else { + break; + } + } else { + if !self.ends_with_diff_marker_prefix() { + edit_events.push(EditParserEvent::OldTextChunk { + chunk: mem::take(&mut self.buffer), + done: false, + line_hint, + }); + } + break; + } + } + DiffParserState::WithinReplace { start } => { + if !self.buffer.is_empty() { + if *start && self.buffer.starts_with('\n') { + self.buffer.remove(0); + } + *start = false; + } + + if let Some(replace_pos) = self.buffer.find(REPLACE_MARKER) { + let mut chunk = self.buffer[..replace_pos].to_string(); + if chunk.ends_with('\n') { + chunk.pop(); + } + + self.buffer.drain(..replace_pos + REPLACE_MARKER.len()); + if let Some(newline_pos) = self.buffer.find('\n') { + self.buffer.drain(..newline_pos + 1); + } else { + self.buffer.clear(); + } + + self.state = DiffParserState::Pending; + edit_events.push(EditParserEvent::NewTextChunk { chunk, done: true }); + } else { + if !self.ends_with_diff_marker_prefix() { + edit_events.push(EditParserEvent::NewTextChunk { + chunk: mem::take(&mut self.buffer), + done: false, + }); + } + break; + } + } + } + } + edit_events + } + + fn take_metrics(&mut self) -> EditParserMetrics { + std::mem::take(&mut self.metrics) + } +} + +impl EditParser { + pub fn new(format: EditFormat) -> Self { + let parser: Box = match format { + EditFormat::XmlTags => Box::new(XmlEditParser::new()), + EditFormat::DiffFenced => Box::new(DiffFencedEditParser::new()), + }; + EditParser { parser } + } + + pub fn push(&mut self, chunk: &str) -> SmallVec<[EditParserEvent; 1]> { + self.parser.push(chunk) + } + + pub fn finish(mut self) -> EditParserMetrics { + self.parser.take_metrics() } } @@ -201,8 +444,8 @@ mod tests { use std::cmp; #[gpui::test(iterations = 1000)] - fn test_single_edit(mut rng: StdRng) { - let mut parser = EditParser::new(); + fn test_xml_single_edit(mut rng: StdRng) { + let mut parser = EditParser::new(EditFormat::XmlTags); assert_eq!( parse_random_chunks( "originalupdated", @@ -225,8 +468,8 @@ mod tests { } #[gpui::test(iterations = 1000)] - fn test_multiple_edits(mut rng: StdRng) { - let mut parser = EditParser::new(); + fn test_xml_multiple_edits(mut rng: StdRng) { + let mut parser = EditParser::new(EditFormat::XmlTags); assert_eq!( parse_random_chunks( indoc! {" @@ -263,8 +506,8 @@ mod tests { } #[gpui::test(iterations = 1000)] - fn test_edits_with_extra_text(mut rng: StdRng) { - let mut parser = EditParser::new(); + fn test_xml_edits_with_extra_text(mut rng: StdRng) { + let mut parser = EditParser::new(EditFormat::XmlTags); assert_eq!( parse_random_chunks( indoc! {" @@ -305,8 +548,8 @@ mod tests { } #[gpui::test(iterations = 1000)] - fn test_nested_tags(mut rng: StdRng) { - let mut parser = EditParser::new(); + fn test_xml_nested_tags(mut rng: StdRng) { + let mut parser = EditParser::new(EditFormat::XmlTags); assert_eq!( parse_random_chunks( "code with nested elementsnew content", @@ -329,8 +572,8 @@ mod tests { } #[gpui::test(iterations = 1000)] - fn test_empty_old_and_new_text(mut rng: StdRng) { - let mut parser = EditParser::new(); + fn test_xml_empty_old_and_new_text(mut rng: StdRng) { + let mut parser = EditParser::new(EditFormat::XmlTags); assert_eq!( parse_random_chunks( "", @@ -353,8 +596,8 @@ mod tests { } #[gpui::test(iterations = 100)] - fn test_multiline_content(mut rng: StdRng) { - let mut parser = EditParser::new(); + fn test_xml_multiline_content(mut rng: StdRng) { + let mut parser = EditParser::new(EditFormat::XmlTags); assert_eq!( parse_random_chunks( "line1\nline2\nline3line1\nmodified line2\nline3", @@ -377,8 +620,8 @@ mod tests { } #[gpui::test(iterations = 1000)] - fn test_mismatched_tags(mut rng: StdRng) { - let mut parser = EditParser::new(); + fn test_xml_mismatched_tags(mut rng: StdRng) { + let mut parser = EditParser::new(EditFormat::XmlTags); assert_eq!( parse_random_chunks( // Reduced from an actual Sonnet 3.7 output @@ -428,7 +671,7 @@ mod tests { } ); - let mut parser = EditParser::new(); + let mut parser = EditParser::new(EditFormat::XmlTags); assert_eq!( parse_random_chunks( // Reduced from an actual Opus 4 output @@ -459,10 +702,230 @@ mod tests { ); } + #[gpui::test(iterations = 1000)] + fn test_diff_fenced_single_edit(mut rng: StdRng) { + let mut parser = EditParser::new(EditFormat::DiffFenced); + assert_eq!( + parse_random_chunks( + indoc! {" + <<<<<<< SEARCH + original text + ======= + updated text + >>>>>>> REPLACE + "}, + &mut parser, + &mut rng + ), + vec![Edit { + old_text: "original text".to_string(), + new_text: "updated text".to_string(), + line_hint: None, + }] + ); + assert_eq!( + parser.finish(), + EditParserMetrics { + tags: 0, + mismatched_tags: 0 + } + ); + } + #[gpui::test(iterations = 100)] - fn test_line_hints(mut rng: StdRng) { + fn test_diff_fenced_with_markdown_fences(mut rng: StdRng) { + let mut parser = EditParser::new(EditFormat::DiffFenced); + assert_eq!( + parse_random_chunks( + indoc! {" + ```diff + <<<<<<< SEARCH + from flask import Flask + ======= + import math + from flask import Flask + >>>>>>> REPLACE + ``` + "}, + &mut parser, + &mut rng + ), + vec![Edit { + old_text: "from flask import Flask".to_string(), + new_text: "import math\nfrom flask import Flask".to_string(), + line_hint: None, + }] + ); + assert_eq!( + parser.finish(), + EditParserMetrics { + tags: 0, + mismatched_tags: 0 + } + ); + } + + #[gpui::test(iterations = 100)] + fn test_diff_fenced_multiple_edits(mut rng: StdRng) { + let mut parser = EditParser::new(EditFormat::DiffFenced); + assert_eq!( + parse_random_chunks( + indoc! {" + <<<<<<< SEARCH + first old + ======= + first new + >>>>>>> REPLACE + + <<<<<<< SEARCH + second old + ======= + second new + >>>>>>> REPLACE + "}, + &mut parser, + &mut rng + ), + vec![ + Edit { + old_text: "first old".to_string(), + new_text: "first new".to_string(), + line_hint: None, + }, + Edit { + old_text: "second old".to_string(), + new_text: "second new".to_string(), + line_hint: None, + }, + ] + ); + assert_eq!( + parser.finish(), + EditParserMetrics { + tags: 0, + mismatched_tags: 0 + } + ); + } + + #[gpui::test(iterations = 100)] + fn test_mixed_formats(mut rng: StdRng) { + // Test XML format parser only parses XML tags + let mut xml_parser = EditParser::new(EditFormat::XmlTags); + assert_eq!( + parse_random_chunks( + indoc! {" + xml style oldxml style new + + <<<<<<< SEARCH + diff style old + ======= + diff style new + >>>>>>> REPLACE + "}, + &mut xml_parser, + &mut rng + ), + vec![Edit { + old_text: "xml style old".to_string(), + new_text: "xml style new".to_string(), + line_hint: None, + },] + ); + assert_eq!( + xml_parser.finish(), + EditParserMetrics { + tags: 2, + mismatched_tags: 0 + } + ); + + // Test diff-fenced format parser only parses diff markers + let mut diff_parser = EditParser::new(EditFormat::DiffFenced); + assert_eq!( + parse_random_chunks( + indoc! {" + xml style oldxml style new + + <<<<<<< SEARCH + diff style old + ======= + diff style new + >>>>>>> REPLACE + "}, + &mut diff_parser, + &mut rng + ), + vec![Edit { + old_text: "diff style old".to_string(), + new_text: "diff style new".to_string(), + line_hint: None, + },] + ); + assert_eq!( + diff_parser.finish(), + EditParserMetrics { + tags: 0, + mismatched_tags: 0 + } + ); + } + + #[gpui::test(iterations = 100)] + fn test_diff_fenced_empty_sections(mut rng: StdRng) { + let mut parser = EditParser::new(EditFormat::DiffFenced); + assert_eq!( + parse_random_chunks( + indoc! {" + <<<<<<< SEARCH + ======= + >>>>>>> REPLACE + "}, + &mut parser, + &mut rng + ), + vec![Edit { + old_text: "".to_string(), + new_text: "".to_string(), + line_hint: None, + }] + ); + assert_eq!( + parser.finish(), + EditParserMetrics { + tags: 0, + mismatched_tags: 0 + } + ); + } + + #[gpui::test(iterations = 100)] + fn test_diff_fenced_with_line_hint(mut rng: StdRng) { + let mut parser = EditParser::new(EditFormat::DiffFenced); + let edits = parse_random_chunks( + indoc! {" + <<<<<<< SEARCH line=42 + original text + ======= + updated text + >>>>>>> REPLACE + "}, + &mut parser, + &mut rng, + ); + assert_eq!( + edits, + vec![Edit { + old_text: "original text".to_string(), + line_hint: Some(42), + new_text: "updated text".to_string(), + }] + ); + } + #[gpui::test(iterations = 100)] + fn test_xml_line_hints(mut rng: StdRng) { // Line hint is a single quoted line number - let mut parser = EditParser::new(); + let mut parser = EditParser::new(EditFormat::XmlTags); let edits = parse_random_chunks( r#" @@ -478,7 +941,7 @@ mod tests { assert_eq!(edits[0].new_text, "updated code"); // Line hint is a single unquoted line number - let mut parser = EditParser::new(); + let mut parser = EditParser::new(EditFormat::XmlTags); let edits = parse_random_chunks( r#" @@ -494,7 +957,7 @@ mod tests { assert_eq!(edits[0].new_text, "updated code"); // Line hint is a range - let mut parser = EditParser::new(); + let mut parser = EditParser::new(EditFormat::XmlTags); let edits = parse_random_chunks( r#" @@ -510,7 +973,7 @@ mod tests { assert_eq!(edits[0].new_text, "updated code"); // No line hint - let mut parser = EditParser::new(); + let mut parser = EditParser::new(EditFormat::XmlTags); let edits = parse_random_chunks( r#" old diff --git a/crates/assistant_tools/src/edit_agent/evals.rs b/crates/assistant_tools/src/edit_agent/evals.rs index 20be2f01b8..b5744d455a 100644 --- a/crates/assistant_tools/src/edit_agent/evals.rs +++ b/crates/assistant_tools/src/edit_agent/evals.rs @@ -41,7 +41,7 @@ fn eval_extract_handle_command_output() { // ----------------------------|---------- // claude-3.7-sonnet | 0.99 (2025-06-14) // claude-sonnet-4 | 0.97 (2025-06-14) - // gemini-2.5-pro-06-05 | 0.77 (2025-05-22) + // gemini-2.5-pro-06-05 | 0.98 (2025-06-16) // gemini-2.5-flash | 0.11 (2025-05-22) // gpt-4.1 | 1.00 (2025-05-22) @@ -59,7 +59,7 @@ fn eval_extract_handle_command_output() { let edit_description = "Extract `handle_command_output` method from `run_git_blame`."; eval( 100, - 0.7, // Taking the lower bar for Gemini + 0.95, 0.05, EvalInput::from_conversation( vec![ @@ -116,7 +116,7 @@ fn eval_delete_run_git_blame() { // ----------------------------|---------- // claude-3.7-sonnet | 1.0 (2025-06-14) // claude-sonnet-4 | 0.96 (2025-06-14) - // gemini-2.5-pro-06-05 | + // gemini-2.5-pro-06-05 | 1.0 (2025-06-16) // gemini-2.5-flash | // gpt-4.1 | let input_file_path = "root/blame.rs"; @@ -241,7 +241,7 @@ fn eval_use_wasi_sdk_in_compile_parser_to_wasm() { // // claude-3.7-sonnet | 0.96 (2025-06-14) // claude-sonnet-4 | 0.11 (2025-06-14) - // gemini-2.5-pro-preview-03-25 | 0.99 (2025-05-22) + // gemini-2.5-pro-preview-latest | 0.99 (2025-06-16) // gemini-2.5-flash-preview-04-17 | // gpt-4.1 | let input_file_path = "root/lib.rs"; @@ -366,7 +366,7 @@ fn eval_disable_cursor_blinking() { // // claude-3.7-sonnet | 0.99 (2025-06-14) // claude-sonnet-4 | 0.85 (2025-06-14) - // gemini-2.5-pro-preview-03-25 | 1.0 (2025-05-22) + // gemini-2.5-pro-preview-latest | 0.97 (2025-06-16) // gemini-2.5-flash-preview-04-17 | // gpt-4.1 | let input_file_path = "root/editor.rs"; @@ -453,12 +453,11 @@ fn eval_from_pixels_constructor() { // (e.g., at the beginning of the file), yet the evaluation may still // rate it highly. // - // Model | Pass rate - // ============================================ - // - // claude-4.0-sonnet | 0.99 - // claude-3.7-sonnet | 0.88 - // gemini-2.5-pro-preview-03-25 | 0.96 + // Model | Date | Pass rate + // ========================================================= + // claude-4.0-sonnet | 2025-06-14 | 0.99 + // claude-3.7-sonnet | 2025-06-14 | 0.88 + // gemini-2.5-pro-preview-06-05 | 2025-06-16 | 0.98 // gpt-4.1 | let input_file_path = "root/canvas.rs"; let input_file_content = include_str!("evals/fixtures/from_pixels_constructor/before.rs"); @@ -1498,8 +1497,16 @@ impl EditAgentTest { .await; let action_log = cx.new(|_| ActionLog::new(project.clone())); + let edit_format = EditFormat::from_env(agent_model.clone()).unwrap(); + Self { - agent: EditAgent::new(agent_model, project.clone(), action_log, Templates::new()), + agent: EditAgent::new( + agent_model, + project.clone(), + action_log, + Templates::new(), + edit_format, + ), project, judge_model, } diff --git a/crates/assistant_tools/src/edit_agent/streaming_fuzzy_matcher.rs b/crates/assistant_tools/src/edit_agent/streaming_fuzzy_matcher.rs index b2fc4beaeb..092bdce8b3 100644 --- a/crates/assistant_tools/src/edit_agent/streaming_fuzzy_matcher.rs +++ b/crates/assistant_tools/src/edit_agent/streaming_fuzzy_matcher.rs @@ -44,6 +44,10 @@ impl StreamingFuzzyMatcher { /// Returns `Some(range)` if a match has been found with the accumulated /// query so far, or `None` if no suitable match exists yet. pub fn push(&mut self, chunk: &str, line_hint: Option) -> Option> { + if line_hint.is_some() { + self.line_hint = line_hint; + } + // Add the chunk to our incomplete line buffer self.incomplete_line.push_str(chunk); self.line_hint = line_hint; diff --git a/crates/assistant_tools/src/edit_file_tool.rs b/crates/assistant_tools/src/edit_file_tool.rs index d3dba5cb61..fde697b00e 100644 --- a/crates/assistant_tools/src/edit_file_tool.rs +++ b/crates/assistant_tools/src/edit_file_tool.rs @@ -1,6 +1,6 @@ use crate::{ Templates, - edit_agent::{EditAgent, EditAgentOutput, EditAgentOutputEvent}, + edit_agent::{EditAgent, EditAgentOutput, EditAgentOutputEvent, EditFormat}, schema::json_schema_for, ui::{COLLAPSED_LINES, ToolOutputPreview}, }; @@ -201,8 +201,14 @@ impl Tool for EditFileTool { let card_clone = card.clone(); let action_log_clone = action_log.clone(); let task = cx.spawn(async move |cx: &mut AsyncApp| { - let edit_agent = - EditAgent::new(model, project.clone(), action_log_clone, Templates::new()); + let edit_format = EditFormat::from_model(model.clone())?; + let edit_agent = EditAgent::new( + model, + project.clone(), + action_log_clone, + Templates::new(), + edit_format, + ); let buffer = project .update(cx, |project, cx| { diff --git a/crates/assistant_tools/src/templates/edit_file_prompt_diff_fenced.hbs b/crates/assistant_tools/src/templates/edit_file_prompt_diff_fenced.hbs new file mode 100644 index 0000000000..a7db420a99 --- /dev/null +++ b/crates/assistant_tools/src/templates/edit_file_prompt_diff_fenced.hbs @@ -0,0 +1,77 @@ +You MUST respond with a series of edits to a file, using the following diff format: + +``` +<<<<<<< SEARCH line=1 +from flask import Flask +======= +import math +from flask import Flask +>>>>>>> REPLACE + +<<<<<<< SEARCH line=325 +return 0 +======= +print("Done") + +return 0 +>>>>>>> REPLACE + +``` + +# File Editing Instructions + +- Use the SEARCH/REPLACE diff format shown above +- The SEARCH section must exactly match existing file content, including indentation +- The SEARCH section must come from the actual file, not an outline +- The SEARCH section cannot be empty +- `line` should be a starting line number for the text to be replaced +- Be minimal with replacements: + - For unique lines, include only those lines + - For non-unique lines, include enough context to identify them +- Do not escape quotes, newlines, or other characters +- For multiple occurrences, repeat the same diff block for each instance +- Edits are sequential - each assumes previous edits are already applied +- Only edit the specified file + +# Example + +``` +<<<<<<< SEARCH line=3 +struct User { + name: String, + email: String, +} +======= +struct User { + name: String, + email: String, + active: bool, +} +>>>>>>> REPLACE + +<<<<<<< SEARCH line=25 + let user = User { + name: String::from("John"), + email: String::from("john@example.com"), + }; +======= + let user = User { + name: String::from("John"), + email: String::from("john@example.com"), + active: true, + }; +>>>>>>> REPLACE +``` + + +# Final instructions + +Tool calls have been disabled. You MUST respond using the SEARCH/REPLACE diff format only. + + +{{path}} + + + +{{edit_description}} + diff --git a/crates/assistant_tools/src/templates/edit_file_prompt.hbs b/crates/assistant_tools/src/templates/edit_file_prompt_xml.hbs similarity index 100% rename from crates/assistant_tools/src/templates/edit_file_prompt.hbs rename to crates/assistant_tools/src/templates/edit_file_prompt_xml.hbs diff --git a/script/danger/dangerfile.ts b/script/danger/dangerfile.ts index 9f3790b1be..3f9c80586e 100644 --- a/script/danger/dangerfile.ts +++ b/script/danger/dangerfile.ts @@ -65,7 +65,8 @@ const PROMPT_PATHS = [ "crates/agent/src/prompts/summarize_thread_detailed_prompt.txt", "crates/agent/src/prompts/summarize_thread_prompt.txt", "crates/assistant_tools/src/templates/create_file_prompt.hbs", - "crates/assistant_tools/src/templates/edit_file_prompt.hbs", + "crates/assistant_tools/src/templates/edit_file_prompt_xml.hbs", + "crates/assistant_tools/src/templates/edit_file_prompt_diff_fenced.hbs", "crates/git_ui/src/commit_message_prompt.txt", ];