assistant edit tool: Report when file is empty or doesn't exist (#27190)

Instead of just reporting a search match failure, we'll now indicate
whether the file is empty or exists to help the model recover better
from bad edits.


Release Notes:

- N/A
This commit is contained in:
Agus Zubiaga 2025-03-20 10:42:10 -03:00 committed by GitHub
parent 4421bdd12e
commit 2e8c0ff244
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -160,9 +160,16 @@ enum DiffResult {
} }
#[derive(Debug)] #[derive(Debug)]
struct BadSearch { enum BadSearch {
NoMatch {
file_path: String, file_path: String,
search: String, search: String,
},
EmptyBuffer {
file_path: String,
search: String,
exists: bool,
},
} }
impl EditToolRequest { impl EditToolRequest {
@ -309,6 +316,18 @@ impl EditToolRequest {
file_path: std::path::PathBuf, file_path: std::path::PathBuf,
snapshot: language::BufferSnapshot, snapshot: language::BufferSnapshot,
) -> Result<DiffResult> { ) -> Result<DiffResult> {
if snapshot.is_empty() {
let exists = snapshot
.file()
.map_or(false, |file| file.disk_state().exists());
return Ok(DiffResult::BadSearch(BadSearch::EmptyBuffer {
file_path: file_path.display().to_string(),
exists,
search: old,
}));
}
let result = let result =
// Try to match exactly // Try to match exactly
replace_exact(&old, &new, &snapshot) replace_exact(&old, &new, &snapshot)
@ -317,7 +336,7 @@ impl EditToolRequest {
.or_else(|| replace_with_flexible_indent(&old, &new, &snapshot)); .or_else(|| replace_with_flexible_indent(&old, &new, &snapshot));
let Some(diff) = result else { let Some(diff) = result else {
return anyhow::Ok(DiffResult::BadSearch(BadSearch { return anyhow::Ok(DiffResult::BadSearch(BadSearch::NoMatch {
search: old, search: old,
file_path: file_path.display().to_string(), file_path: file_path.display().to_string(),
})); }));
@ -377,13 +396,39 @@ impl EditToolRequest {
self.bad_searches.len() self.bad_searches.len()
)?; )?;
for replace in self.bad_searches { for bad_search in self.bad_searches {
match bad_search {
BadSearch::NoMatch { file_path, search } => {
writeln!( writeln!(
&mut output, &mut output,
"## No exact match in: {}\n```\n{}\n```\n", "## No exact match in: `{}`\n```\n{}\n```\n",
replace.file_path, replace.search, file_path, search,
)?; )?;
} }
BadSearch::EmptyBuffer {
file_path,
exists: true,
search,
} => {
writeln!(
&mut output,
"## No match because `{}` is empty:\n```\n{}\n```\n",
file_path, search,
)?;
}
BadSearch::EmptyBuffer {
file_path,
exists: false,
search,
} => {
writeln!(
&mut output,
"## No match because `{}` does not exist:\n```\n{}\n```\n",
file_path, search,
)?;
}
}
}
write!(&mut output, write!(&mut output,
"The SEARCH section must exactly match an existing block of lines including all white \ "The SEARCH section must exactly match an existing block of lines including all white \