@@ -145,22 +145,30 @@ impl Tool for EditFilesTool {
struct EditToolRequest {
parser: EditActionParser,
- output: String,
- changed_buffers: HashSet<Entity<language::Buffer>>,
- bad_searches: Vec<BadSearch>,
+ editor_response: EditorResponse,
project: Entity<Project>,
action_log: Entity<ActionLog>,
tool_log: Option<(Entity<EditToolLog>, EditToolRequestId)>,
}
-#[derive(Debug)]
-enum DiffResult {
- BadSearch(BadSearch),
- Diff(language::Diff),
+enum EditorResponse {
+ /// The editor model hasn't produced any actions yet.
+ /// If we don't have any by the end, we'll return its message to the architect model.
+ Message(String),
+ /// The editor model produced at least one action.
+ Actions {
+ applied: Vec<AppliedAction>,
+ search_errors: Vec<SearchError>,
+ },
+}
+
+struct AppliedAction {
+ source: String,
+ buffer: Entity<language::Buffer>,
}
#[derive(Debug)]
-enum BadSearch {
+enum SearchError {
NoMatch {
file_path: String,
search: String,
@@ -226,10 +234,7 @@ impl EditToolRequest {
let mut request = Self {
parser: EditActionParser::new(),
- // we start with the success header so we don't need to shift the output in the common case
- output: Self::SUCCESS_OUTPUT_HEADER.to_string(),
- changed_buffers: HashSet::default(),
- bad_searches: Vec::new(),
+ editor_response: EditorResponse::Message(String::with_capacity(256)),
action_log,
project,
tool_log,
@@ -246,6 +251,12 @@ impl EditToolRequest {
async fn process_response_chunk(&mut self, chunk: &str, cx: &mut AsyncApp) -> Result<()> {
let new_actions = self.parser.parse_chunk(chunk);
+ if let EditorResponse::Message(ref mut message) = self.editor_response {
+ if new_actions.is_empty() {
+ message.push_str(chunk);
+ }
+ }
+
if let Some((ref log, req_id)) = self.tool_log {
log.update(cx, |log, cx| {
log.push_editor_response_chunk(req_id, chunk, &new_actions, cx)
@@ -276,6 +287,11 @@ impl EditToolRequest {
.update(cx, |project, cx| project.open_buffer(project_path, cx))?
.await?;
+ enum DiffResult {
+ Diff(language::Diff),
+ SearchError(SearchError),
+ }
+
let result = match action {
EditAction::Replace {
old,
@@ -285,7 +301,39 @@ impl EditToolRequest {
let snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot())?;
cx.background_executor()
- .spawn(Self::replace_diff(old, new, file_path, snapshot))
+ .spawn(async move {
+ if snapshot.is_empty() {
+ let exists = snapshot
+ .file()
+ .map_or(false, |file| file.disk_state().exists());
+
+ let error = SearchError::EmptyBuffer {
+ file_path: file_path.display().to_string(),
+ exists,
+ search: old,
+ };
+
+ return anyhow::Ok(DiffResult::SearchError(error));
+ }
+
+ let replace_result =
+ // Try to match exactly
+ replace_exact(&old, &new, &snapshot)
+ .await
+ // If that fails, try being flexible about indentation
+ .or_else(|| replace_with_flexible_indent(&old, &new, &snapshot));
+
+ let Some(diff) = replace_result else {
+ let error = SearchError::NoMatch {
+ search: old,
+ file_path: file_path.display().to_string(),
+ };
+
+ return Ok(DiffResult::SearchError(error));
+ };
+
+ Ok(DiffResult::Diff(diff))
+ })
.await
}
EditAction::Write { content, .. } => Ok(DiffResult::Diff(
@@ -296,177 +344,179 @@ impl EditToolRequest {
}?;
match result {
- DiffResult::BadSearch(invalid_replace) => {
- self.bad_searches.push(invalid_replace);
+ DiffResult::SearchError(error) => {
+ self.push_search_error(error);
}
DiffResult::Diff(diff) => {
let _clock = buffer.update(cx, |buffer, cx| buffer.apply_diff(diff, cx))?;
- write!(&mut self.output, "\n\n{}", source)?;
- self.changed_buffers.insert(buffer);
+ self.push_applied_action(AppliedAction { source, buffer });
}
}
- Ok(())
+ anyhow::Ok(())
}
- async fn replace_diff(
- old: String,
- new: String,
- file_path: std::path::PathBuf,
- snapshot: language::BufferSnapshot,
- ) -> 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,
- }));
+ fn push_search_error(&mut self, error: SearchError) {
+ match &mut self.editor_response {
+ EditorResponse::Message(_) => {
+ self.editor_response = EditorResponse::Actions {
+ applied: Vec::new(),
+ search_errors: vec![error],
+ };
+ }
+ EditorResponse::Actions { search_errors, .. } => {
+ search_errors.push(error);
+ }
}
-
- let result =
- // Try to match exactly
- replace_exact(&old, &new, &snapshot)
- .await
- // If that fails, try being flexible about indentation
- .or_else(|| replace_with_flexible_indent(&old, &new, &snapshot));
-
- let Some(diff) = result else {
- return anyhow::Ok(DiffResult::BadSearch(BadSearch::NoMatch {
- search: old,
- file_path: file_path.display().to_string(),
- }));
- };
-
- anyhow::Ok(DiffResult::Diff(diff))
}
- const SUCCESS_OUTPUT_HEADER: &str = "Successfully applied. Here's a list of changes:";
- const ERROR_OUTPUT_HEADER_NO_EDITS: &str = "I couldn't apply any edits!";
- const ERROR_OUTPUT_HEADER_WITH_EDITS: &str =
- "Errors occurred. First, here's a list of the edits we managed to apply:";
-
- async fn finalize(self, cx: &mut AsyncApp) -> Result<String> {
- let changed_buffer_count = self.changed_buffers.len();
-
- // Save each buffer once at the end
- for buffer in &self.changed_buffers {
- self.project
- .update(cx, |project, cx| project.save_buffer(buffer.clone(), cx))?
- .await?;
+ fn push_applied_action(&mut self, action: AppliedAction) {
+ match &mut self.editor_response {
+ EditorResponse::Message(_) => {
+ self.editor_response = EditorResponse::Actions {
+ applied: vec![action],
+ search_errors: Vec::new(),
+ };
+ }
+ EditorResponse::Actions { applied, .. } => {
+ applied.push(action);
+ }
}
+ }
- self.action_log
- .update(cx, |log, cx| log.buffer_edited(self.changed_buffers, cx))
- .log_err();
-
- let errors = self.parser.errors();
+ async fn finalize(self, cx: &mut AsyncApp) -> Result<String> {
+ match self.editor_response {
+ EditorResponse::Message(message) => Err(anyhow!(
+ "No edits were applied! You might need to provide more context.\n\n{}",
+ message
+ )),
+ EditorResponse::Actions {
+ applied,
+ search_errors,
+ } => {
+ let mut output = String::with_capacity(1024);
+
+ let parse_errors = self.parser.errors();
+ let has_errors = !search_errors.is_empty() || !parse_errors.is_empty();
+
+ if has_errors {
+ let error_count = search_errors.len() + parse_errors.len();
+
+ if applied.is_empty() {
+ writeln!(
+ &mut output,
+ "{} errors occurred! No edits were applied.",
+ error_count,
+ )?;
+ } else {
+ writeln!(
+ &mut output,
+ "{} errors occurred, but {} edits were correctly applied.",
+ error_count,
+ applied.len(),
+ )?;
+
+ writeln!(
+ &mut output,
+ "# {} SEARCH/REPLACE block(s) applied:\n\nDo not re-send these since they are already applied!\n",
+ applied.len()
+ )?;
+ }
+ } else {
+ write!(
+ &mut output,
+ "Successfully applied! Here's a list of applied edits:"
+ )?;
+ }
- if errors.is_empty() && self.bad_searches.is_empty() {
- if changed_buffer_count == 0 {
- return Err(anyhow!(
- "The instructions didn't lead to any changes. You might need to consult the file contents first."
- ));
- }
+ let mut changed_buffers = HashSet::default();
- Ok(self.output)
- } else {
- let mut output = self.output;
+ for action in applied {
+ changed_buffers.insert(action.buffer);
+ write!(&mut output, "\n\n{}", action.source)?;
+ }
- if output.is_empty() {
- output.replace_range(
- 0..Self::SUCCESS_OUTPUT_HEADER.len(),
- Self::ERROR_OUTPUT_HEADER_NO_EDITS,
- );
- } else {
- output.replace_range(
- 0..Self::SUCCESS_OUTPUT_HEADER.len(),
- Self::ERROR_OUTPUT_HEADER_WITH_EDITS,
- );
- }
+ for buffer in &changed_buffers {
+ self.project
+ .update(cx, |project, cx| project.save_buffer(buffer.clone(), cx))?
+ .await?;
+ }
- if !self.bad_searches.is_empty() {
- writeln!(
- &mut output,
- "\n\n# {} SEARCH/REPLACE block(s) failed to match:\n",
- self.bad_searches.len()
- )?;
-
- for bad_search in self.bad_searches {
- match bad_search {
- BadSearch::NoMatch { file_path, search } => {
- writeln!(
- &mut output,
- "## No exact match in: `{}`\n```\n{}\n```\n",
- 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,
- )?;
+ self.action_log
+ .update(cx, |log, cx| log.buffer_edited(changed_buffers.clone(), cx))
+ .log_err();
+
+ if !search_errors.is_empty() {
+ writeln!(
+ &mut output,
+ "\n\n## {} SEARCH/REPLACE block(s) failed to match:\n",
+ search_errors.len()
+ )?;
+
+ for error in search_errors {
+ match error {
+ SearchError::NoMatch { file_path, search } => {
+ writeln!(
+ &mut output,
+ "### No exact match in: `{}`\n```\n{}\n```\n",
+ file_path, search,
+ )?;
+ }
+ SearchError::EmptyBuffer {
+ file_path,
+ exists: true,
+ search,
+ } => {
+ writeln!(
+ &mut output,
+ "### No match because `{}` is empty:\n```\n{}\n```\n",
+ file_path, search,
+ )?;
+ }
+ SearchError::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,
- "The SEARCH section must exactly match an existing block of lines including all white \
- space, comments, indentation, docstrings, etc."
- )?;
- }
+ write!(&mut output,
+ "The SEARCH section must exactly match an existing block of lines including all white \
+ space, comments, indentation, docstrings, etc."
+ )?;
+ }
- if !errors.is_empty() {
- writeln!(
- &mut output,
- "\n\n# {} SEARCH/REPLACE blocks failed to parse:",
- errors.len()
- )?;
+ if !parse_errors.is_empty() {
+ writeln!(
+ &mut output,
+ "\n\n## {} SEARCH/REPLACE blocks failed to parse:",
+ parse_errors.len()
+ )?;
- for error in errors {
- writeln!(&mut output, "- {}", error)?;
+ for error in parse_errors {
+ writeln!(&mut output, "- {}", error)?;
+ }
}
- }
- if changed_buffer_count > 0 {
- writeln!(
- &mut output,
- "\n\nThe other SEARCH/REPLACE blocks were applied successfully. Do not re-send them!",
- )?;
- }
+ if has_errors {
+ writeln!(&mut output,
+ "\n\nYou can fix errors by running the tool again. You can include instructions, \
+ but errors are part of the conversation so you don't need to repeat them.",
+ )?;
- writeln!(
- &mut output,
- "{}You can fix errors by running the tool again. You can include instructions, \
- but errors are part of the conversation so you don't need to repeat them.",
- if changed_buffer_count == 0 {
- "\n\n"
+ Err(anyhow!(output))
} else {
- ""
+ Ok(output)
}
- )?;
-
- Err(anyhow!(output))
+ }
}
}
}