agent2: always finalize diffs from the edit tool
There were cases before where we would exit after creating the diff and wouldn't finalize it
This commit is contained in:
Ben Brandt
parent
64b14ef848
commit
afd2c3aa76
1 changed files with 137 additions and 132 deletions
|
|
@ -272,159 +272,164 @@ impl AgentTool for EditFileTool {
|
||||||
.await?;
|
.await?;
|
||||||
|
|
||||||
let diff = cx.new(|cx| Diff::new(buffer.clone(), cx))?;
|
let diff = cx.new(|cx| Diff::new(buffer.clone(), cx))?;
|
||||||
event_stream.update_diff(diff.clone());
|
let result = async {
|
||||||
|
event_stream.update_diff(diff.clone());
|
||||||
|
|
||||||
let old_snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot())?;
|
let old_snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot())?;
|
||||||
let old_text = cx
|
let old_text = cx
|
||||||
.background_spawn({
|
.background_spawn({
|
||||||
let old_snapshot = old_snapshot.clone();
|
let old_snapshot = old_snapshot.clone();
|
||||||
async move { Arc::new(old_snapshot.text()) }
|
async move { Arc::new(old_snapshot.text()) }
|
||||||
})
|
})
|
||||||
.await;
|
.await;
|
||||||
|
|
||||||
|
|
||||||
let (output, mut events) = if matches!(input.mode, EditFileMode::Edit) {
|
let (output, mut events) = if matches!(input.mode, EditFileMode::Edit) {
|
||||||
edit_agent.edit(
|
edit_agent.edit(
|
||||||
buffer.clone(),
|
buffer.clone(),
|
||||||
input.display_description.clone(),
|
input.display_description.clone(),
|
||||||
&request,
|
&request,
|
||||||
cx,
|
cx,
|
||||||
)
|
)
|
||||||
} else {
|
} else {
|
||||||
edit_agent.overwrite(
|
edit_agent.overwrite(
|
||||||
buffer.clone(),
|
buffer.clone(),
|
||||||
input.display_description.clone(),
|
input.display_description.clone(),
|
||||||
&request,
|
&request,
|
||||||
cx,
|
cx,
|
||||||
)
|
)
|
||||||
};
|
};
|
||||||
|
|
||||||
let mut hallucinated_old_text = false;
|
let mut hallucinated_old_text = false;
|
||||||
let mut ambiguous_ranges = Vec::new();
|
let mut ambiguous_ranges = Vec::new();
|
||||||
let mut emitted_location = false;
|
let mut emitted_location = false;
|
||||||
while let Some(event) = events.next().await {
|
while let Some(event) = events.next().await {
|
||||||
match event {
|
match event {
|
||||||
EditAgentOutputEvent::Edited(range) => {
|
EditAgentOutputEvent::Edited(range) => {
|
||||||
if !emitted_location {
|
if !emitted_location {
|
||||||
let line = buffer.update(cx, |buffer, _cx| {
|
let line = buffer.update(cx, |buffer, _cx| {
|
||||||
range.start.to_point(&buffer.snapshot()).row
|
range.start.to_point(&buffer.snapshot()).row
|
||||||
}).ok();
|
}).ok();
|
||||||
if let Some(abs_path) = abs_path.clone() {
|
if let Some(abs_path) = abs_path.clone() {
|
||||||
event_stream.update_fields(ToolCallUpdateFields {
|
event_stream.update_fields(ToolCallUpdateFields {
|
||||||
locations: Some(vec![ToolCallLocation { path: abs_path, line }]),
|
locations: Some(vec![ToolCallLocation { path: abs_path, line }]),
|
||||||
..Default::default()
|
..Default::default()
|
||||||
});
|
});
|
||||||
|
}
|
||||||
|
emitted_location = true;
|
||||||
}
|
}
|
||||||
emitted_location = true;
|
},
|
||||||
|
EditAgentOutputEvent::UnresolvedEditRange => hallucinated_old_text = true,
|
||||||
|
EditAgentOutputEvent::AmbiguousEditRange(ranges) => ambiguous_ranges = ranges,
|
||||||
|
EditAgentOutputEvent::ResolvingEditRange(range) => {
|
||||||
|
diff.update(cx, |card, cx| card.reveal_range(range.clone(), cx))?;
|
||||||
|
// if !emitted_location {
|
||||||
|
// let line = buffer.update(cx, |buffer, _cx| {
|
||||||
|
// range.start.to_point(&buffer.snapshot()).row
|
||||||
|
// }).ok();
|
||||||
|
// if let Some(abs_path) = abs_path.clone() {
|
||||||
|
// event_stream.update_fields(ToolCallUpdateFields {
|
||||||
|
// locations: Some(vec![ToolCallLocation { path: abs_path, line }]),
|
||||||
|
// ..Default::default()
|
||||||
|
// });
|
||||||
|
// }
|
||||||
|
// }
|
||||||
}
|
}
|
||||||
},
|
|
||||||
EditAgentOutputEvent::UnresolvedEditRange => hallucinated_old_text = true,
|
|
||||||
EditAgentOutputEvent::AmbiguousEditRange(ranges) => ambiguous_ranges = ranges,
|
|
||||||
EditAgentOutputEvent::ResolvingEditRange(range) => {
|
|
||||||
diff.update(cx, |card, cx| card.reveal_range(range.clone(), cx))?;
|
|
||||||
// if !emitted_location {
|
|
||||||
// let line = buffer.update(cx, |buffer, _cx| {
|
|
||||||
// range.start.to_point(&buffer.snapshot()).row
|
|
||||||
// }).ok();
|
|
||||||
// if let Some(abs_path) = abs_path.clone() {
|
|
||||||
// event_stream.update_fields(ToolCallUpdateFields {
|
|
||||||
// locations: Some(vec![ToolCallLocation { path: abs_path, line }]),
|
|
||||||
// ..Default::default()
|
|
||||||
// });
|
|
||||||
// }
|
|
||||||
// }
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
// If format_on_save is enabled, format the buffer
|
// If format_on_save is enabled, format the buffer
|
||||||
let format_on_save_enabled = buffer
|
let format_on_save_enabled = buffer
|
||||||
.read_with(cx, |buffer, cx| {
|
.read_with(cx, |buffer, cx| {
|
||||||
let settings = language_settings::language_settings(
|
let settings = language_settings::language_settings(
|
||||||
buffer.language().map(|l| l.name()),
|
buffer.language().map(|l| l.name()),
|
||||||
buffer.file(),
|
buffer.file(),
|
||||||
cx,
|
cx,
|
||||||
);
|
);
|
||||||
settings.format_on_save != FormatOnSave::Off
|
settings.format_on_save != FormatOnSave::Off
|
||||||
})
|
})
|
||||||
.unwrap_or(false);
|
.unwrap_or(false);
|
||||||
|
|
||||||
let edit_agent_output = output.await?;
|
let edit_agent_output = output.await?;
|
||||||
|
|
||||||
|
if format_on_save_enabled {
|
||||||
|
action_log.update(cx, |log, cx| {
|
||||||
|
log.buffer_edited(buffer.clone(), cx);
|
||||||
|
})?;
|
||||||
|
|
||||||
|
let format_task = project.update(cx, |project, cx| {
|
||||||
|
project.format(
|
||||||
|
HashSet::from_iter([buffer.clone()]),
|
||||||
|
LspFormatTarget::Buffers,
|
||||||
|
false, // Don't push to history since the tool did it.
|
||||||
|
FormatTrigger::Save,
|
||||||
|
cx,
|
||||||
|
)
|
||||||
|
})?;
|
||||||
|
format_task.await.log_err();
|
||||||
|
}
|
||||||
|
|
||||||
|
project
|
||||||
|
.update(cx, |project, cx| project.save_buffer(buffer.clone(), cx))?
|
||||||
|
.await?;
|
||||||
|
|
||||||
if format_on_save_enabled {
|
|
||||||
action_log.update(cx, |log, cx| {
|
action_log.update(cx, |log, cx| {
|
||||||
log.buffer_edited(buffer.clone(), cx);
|
log.buffer_edited(buffer.clone(), cx);
|
||||||
})?;
|
})?;
|
||||||
|
|
||||||
let format_task = project.update(cx, |project, cx| {
|
let new_snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot())?;
|
||||||
project.format(
|
let (new_text, unified_diff) = cx
|
||||||
HashSet::from_iter([buffer.clone()]),
|
.background_spawn({
|
||||||
LspFormatTarget::Buffers,
|
let new_snapshot = new_snapshot.clone();
|
||||||
false, // Don't push to history since the tool did it.
|
let old_text = old_text.clone();
|
||||||
FormatTrigger::Save,
|
async move {
|
||||||
cx,
|
let new_text = new_snapshot.text();
|
||||||
)
|
let diff = language::unified_diff(&old_text, &new_text);
|
||||||
})?;
|
(new_text, diff)
|
||||||
format_task.await.log_err();
|
}
|
||||||
}
|
})
|
||||||
|
.await;
|
||||||
|
|
||||||
project
|
let input_path = input.path.display();
|
||||||
.update(cx, |project, cx| project.save_buffer(buffer.clone(), cx))?
|
if unified_diff.is_empty() {
|
||||||
.await?;
|
anyhow::ensure!(
|
||||||
|
!hallucinated_old_text,
|
||||||
|
formatdoc! {"
|
||||||
|
Some edits were produced but none of them could be applied.
|
||||||
|
Read the relevant sections of {input_path} again so that
|
||||||
|
I can perform the requested edits.
|
||||||
|
"}
|
||||||
|
);
|
||||||
|
anyhow::ensure!(
|
||||||
|
ambiguous_ranges.is_empty(),
|
||||||
|
{
|
||||||
|
let line_numbers = ambiguous_ranges
|
||||||
|
.iter()
|
||||||
|
.map(|range| range.start.to_string())
|
||||||
|
.collect::<Vec<_>>()
|
||||||
|
.join(", ");
|
||||||
|
formatdoc! {"
|
||||||
|
<old_text> matches more than one position in the file (lines: {line_numbers}). Read the
|
||||||
|
relevant sections of {input_path} again and extend <old_text> so
|
||||||
|
that I can perform the requested edits.
|
||||||
|
"}
|
||||||
|
}
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
action_log.update(cx, |log, cx| {
|
Ok(EditFileToolOutput {
|
||||||
log.buffer_edited(buffer.clone(), cx);
|
input_path: input.path,
|
||||||
})?;
|
new_text,
|
||||||
|
old_text,
|
||||||
let new_snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot())?;
|
diff: unified_diff,
|
||||||
let (new_text, unified_diff) = cx
|
edit_agent_output,
|
||||||
.background_spawn({
|
|
||||||
let new_snapshot = new_snapshot.clone();
|
|
||||||
let old_text = old_text.clone();
|
|
||||||
async move {
|
|
||||||
let new_text = new_snapshot.text();
|
|
||||||
let diff = language::unified_diff(&old_text, &new_text);
|
|
||||||
(new_text, diff)
|
|
||||||
}
|
|
||||||
})
|
})
|
||||||
.await;
|
}.await;
|
||||||
|
|
||||||
|
// Always finalize the diff, regardless of whether the operation succeeded or failed
|
||||||
diff.update(cx, |diff, cx| diff.finalize(cx)).ok();
|
diff.update(cx, |diff, cx| diff.finalize(cx)).ok();
|
||||||
|
|
||||||
let input_path = input.path.display();
|
result
|
||||||
if unified_diff.is_empty() {
|
|
||||||
anyhow::ensure!(
|
|
||||||
!hallucinated_old_text,
|
|
||||||
formatdoc! {"
|
|
||||||
Some edits were produced but none of them could be applied.
|
|
||||||
Read the relevant sections of {input_path} again so that
|
|
||||||
I can perform the requested edits.
|
|
||||||
"}
|
|
||||||
);
|
|
||||||
anyhow::ensure!(
|
|
||||||
ambiguous_ranges.is_empty(),
|
|
||||||
{
|
|
||||||
let line_numbers = ambiguous_ranges
|
|
||||||
.iter()
|
|
||||||
.map(|range| range.start.to_string())
|
|
||||||
.collect::<Vec<_>>()
|
|
||||||
.join(", ");
|
|
||||||
formatdoc! {"
|
|
||||||
<old_text> matches more than one position in the file (lines: {line_numbers}). Read the
|
|
||||||
relevant sections of {input_path} again and extend <old_text> so
|
|
||||||
that I can perform the requested edits.
|
|
||||||
"}
|
|
||||||
}
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
Ok(EditFileToolOutput {
|
|
||||||
input_path: input.path,
|
|
||||||
new_text,
|
|
||||||
old_text,
|
|
||||||
diff: unified_diff,
|
|
||||||
edit_agent_output,
|
|
||||||
})
|
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue