@@ -148,22 +148,36 @@ 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)>,
}
+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 DiffResult {
- BadSearch(BadSearch),
Diff(language::Diff),
+ SearchError(SearchError),
}
#[derive(Debug)]
-enum BadSearch {
+enum SearchError {
NoMatch {
file_path: String,
search: String,
@@ -242,9 +256,7 @@ impl EditToolRequest {
let mut request = Self {
parser: EditActionParser::new(),
- 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,
@@ -263,6 +275,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)
@@ -313,18 +331,45 @@ 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(())
+ }
+
+ 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);
+ }
+ }
+ }
+
+ 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);
+ }
+ }
}
async fn replace_diff(
@@ -338,152 +383,166 @@ impl EditToolRequest {
.file()
.map_or(false, |file| file.disk_state().exists());
- return Ok(DiffResult::BadSearch(BadSearch::EmptyBuffer {
+ let error = SearchError::EmptyBuffer {
file_path: file_path.display().to_string(),
exists,
search: old,
- }));
+ };
+
+ return Ok(DiffResult::SearchError(error));
}
- let result =
+ 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) = result else {
- return anyhow::Ok(DiffResult::BadSearch(BadSearch::NoMatch {
+ let Some(diff) = replace_result else {
+ let error = SearchError::NoMatch {
search: old,
file_path: file_path.display().to_string(),
- }));
+ };
+
+ return Ok(DiffResult::SearchError(error));
};
- anyhow::Ok(DiffResult::Diff(diff))
+ 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?;
- }
-
- self.action_log
- .update(cx, |log, cx| log.buffer_edited(self.changed_buffers, cx))
- .log_err();
-
- let errors = self.parser.errors();
+ 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))
+ }
}
}
}