agent: Less disruptive changed file notification (#31693)
When the user edits one of the tracked files, we used to notify the agent by inserting a user message at the end of the thread. This was causing a few problems: - The agent would stop doing its work and start reading changed files - The agent would write something like, "Thank you for letting me know about these changed files." This fix contains two parts: 1. Changing the prompt to indicate this is a service message 2. Moving the message higher in the conversation thread This works, but it slightly hurts caching. We may consider making these notification messages stick in history, trading context tokens count for the cache. This might be related to #30906 Release Notes: - N/A --------- Co-authored-by: Marshall Bowers <git@maxdeviant.com>
This commit is contained in:
parent
92addb005a
commit
6df4c537b9
6 changed files with 140 additions and 30 deletions
|
@ -1 +1,3 @@
|
|||
These files changed since last read:
|
||||
[The following is an auto-generated notification; do not reply]
|
||||
|
||||
These files have changed since the last read:
|
||||
|
|
|
@ -1389,7 +1389,7 @@ impl Thread {
|
|||
request.messages[message_ix_to_cache].cache = true;
|
||||
}
|
||||
|
||||
self.attached_tracked_files_state(&mut request.messages, cx);
|
||||
self.attach_tracked_files_state(&mut request.messages, cx);
|
||||
|
||||
request.tools = available_tools;
|
||||
request.mode = if model.supports_max_mode() {
|
||||
|
@ -1453,43 +1453,57 @@ impl Thread {
|
|||
request
|
||||
}
|
||||
|
||||
fn attached_tracked_files_state(
|
||||
fn attach_tracked_files_state(
|
||||
&self,
|
||||
messages: &mut Vec<LanguageModelRequestMessage>,
|
||||
cx: &App,
|
||||
) {
|
||||
const STALE_FILES_HEADER: &str = include_str!("./prompts/stale_files_prompt_header.txt");
|
||||
|
||||
let mut stale_message = String::new();
|
||||
let mut stale_files = String::new();
|
||||
|
||||
let action_log = self.action_log.read(cx);
|
||||
|
||||
for stale_file in action_log.stale_buffers(cx) {
|
||||
let Some(file) = stale_file.read(cx).file() else {
|
||||
continue;
|
||||
};
|
||||
|
||||
if stale_message.is_empty() {
|
||||
write!(&mut stale_message, "{}\n", STALE_FILES_HEADER.trim()).ok();
|
||||
if let Some(file) = stale_file.read(cx).file() {
|
||||
writeln!(&mut stale_files, "- {}", file.path().display()).ok();
|
||||
}
|
||||
|
||||
writeln!(&mut stale_message, "- {}", file.path().display()).ok();
|
||||
}
|
||||
|
||||
let mut content = Vec::with_capacity(2);
|
||||
|
||||
if !stale_message.is_empty() {
|
||||
content.push(stale_message.into());
|
||||
if stale_files.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
if !content.is_empty() {
|
||||
let context_message = LanguageModelRequestMessage {
|
||||
role: Role::User,
|
||||
content,
|
||||
cache: false,
|
||||
};
|
||||
// NOTE: Changes to this prompt require a symmetric update in the LLM Worker
|
||||
const STALE_FILES_HEADER: &str = include_str!("./prompts/stale_files_prompt_header.txt");
|
||||
let content = MessageContent::Text(
|
||||
format!("{STALE_FILES_HEADER}{stale_files}").replace("\r\n", "\n"),
|
||||
);
|
||||
|
||||
messages.push(context_message);
|
||||
// Insert our message before the last Assistant message.
|
||||
// Inserting it to the tail distracts the agent too much
|
||||
let insert_position = messages
|
||||
.iter()
|
||||
.enumerate()
|
||||
.rfind(|(_, message)| message.role == Role::Assistant)
|
||||
.map_or(messages.len(), |(i, _)| i);
|
||||
|
||||
let request_message = LanguageModelRequestMessage {
|
||||
role: Role::User,
|
||||
content: vec![content],
|
||||
cache: false,
|
||||
};
|
||||
|
||||
messages.insert(insert_position, request_message);
|
||||
|
||||
// It makes no sense to cache messages after this one because
|
||||
// the cache is invalidated when this message is gone.
|
||||
// Move the cache marker before this message.
|
||||
let has_cached_messages_after = messages
|
||||
.iter()
|
||||
.skip(insert_position + 1)
|
||||
.any(|message| message.cache);
|
||||
|
||||
if has_cached_messages_after {
|
||||
messages[insert_position - 1].cache = true;
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -3295,12 +3309,24 @@ fn main() {{
|
|||
assert_eq!(last_message.role, Role::User);
|
||||
|
||||
// Check the exact content of the message
|
||||
let expected_content = "These files changed since last read:\n- code.rs\n";
|
||||
let expected_content = "[The following is an auto-generated notification; do not reply]
|
||||
|
||||
These files have changed since the last read:
|
||||
- code.rs
|
||||
";
|
||||
assert_eq!(
|
||||
last_message.string_contents(),
|
||||
expected_content,
|
||||
"Last message should be exactly the stale buffer notification"
|
||||
);
|
||||
|
||||
// The message before the notification should be cached
|
||||
let index = new_request.messages.len() - 2;
|
||||
let previous_message = new_request.messages.get(index).unwrap();
|
||||
assert!(
|
||||
previous_message.cache,
|
||||
"Message before the stale buffer notification should be cached"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
|
|
|
@ -456,18 +456,18 @@ impl ActionLog {
|
|||
})?
|
||||
}
|
||||
|
||||
/// Track a buffer as read, so we can notify the model about user edits.
|
||||
/// Track a buffer as read by agent, so we can notify the model about user edits.
|
||||
pub fn buffer_read(&mut self, buffer: Entity<Buffer>, cx: &mut Context<Self>) {
|
||||
self.track_buffer_internal(buffer, false, cx);
|
||||
}
|
||||
|
||||
/// Mark a buffer as edited, so we can refresh it in the context
|
||||
/// Mark a buffer as created by agent, so we can refresh it in the context
|
||||
pub fn buffer_created(&mut self, buffer: Entity<Buffer>, cx: &mut Context<Self>) {
|
||||
self.edited_since_project_diagnostics_check = true;
|
||||
self.track_buffer_internal(buffer.clone(), true, cx);
|
||||
}
|
||||
|
||||
/// Mark a buffer as edited, so we can refresh it in the context
|
||||
/// Mark a buffer as edited by agent, so we can refresh it in the context
|
||||
pub fn buffer_edited(&mut self, buffer: Entity<Buffer>, cx: &mut Context<Self>) {
|
||||
self.edited_since_project_diagnostics_check = true;
|
||||
|
||||
|
|
74
crates/eval/src/examples/file_change_notification.rs
Normal file
74
crates/eval/src/examples/file_change_notification.rs
Normal file
|
@ -0,0 +1,74 @@
|
|||
use agent_settings::AgentProfileId;
|
||||
use anyhow::Result;
|
||||
use async_trait::async_trait;
|
||||
|
||||
use crate::example::{Example, ExampleContext, ExampleMetadata, JudgeAssertion};
|
||||
|
||||
pub struct FileChangeNotificationExample;
|
||||
|
||||
#[async_trait(?Send)]
|
||||
impl Example for FileChangeNotificationExample {
|
||||
fn meta(&self) -> ExampleMetadata {
|
||||
ExampleMetadata {
|
||||
name: "file_change_notification".to_string(),
|
||||
url: "https://github.com/octocat/hello-world".to_string(),
|
||||
revision: "7fd1a60b01f91b314f59955a4e4d4e80d8edf11d".to_string(),
|
||||
language_server: None,
|
||||
max_assertions: Some(1),
|
||||
profile_id: AgentProfileId::default(),
|
||||
existing_thread_json: None,
|
||||
max_turns: Some(3),
|
||||
}
|
||||
}
|
||||
|
||||
async fn conversation(&self, cx: &mut ExampleContext) -> Result<()> {
|
||||
// Track README so that the model gets notified of its changes
|
||||
let project_path = cx.agent_thread().read_with(cx, |thread, cx| {
|
||||
thread
|
||||
.project()
|
||||
.read(cx)
|
||||
.find_project_path("README", cx)
|
||||
.expect("README file should exist in this repo")
|
||||
})?;
|
||||
|
||||
let buffer = {
|
||||
cx.agent_thread()
|
||||
.update(cx, |thread, cx| {
|
||||
thread
|
||||
.project()
|
||||
.update(cx, |project, cx| project.open_buffer(project_path, cx))
|
||||
})?
|
||||
.await?
|
||||
};
|
||||
|
||||
cx.agent_thread().update(cx, |thread, cx| {
|
||||
thread.action_log().update(cx, |action_log, cx| {
|
||||
action_log.buffer_read(buffer.clone(), cx);
|
||||
});
|
||||
})?;
|
||||
|
||||
// Start conversation (specific message is not important)
|
||||
cx.push_user_message("Find all files in this repo");
|
||||
cx.run_turn().await?;
|
||||
|
||||
// Edit the README buffer - the model should get a notification on next turn
|
||||
buffer.update(cx, |buffer, cx| {
|
||||
buffer.edit([(0..buffer.len(), "Surprise!")], None, cx);
|
||||
})?;
|
||||
|
||||
// Run for some more turns.
|
||||
// The model shouldn't thank us for letting it know about the file change.
|
||||
cx.run_turns(3).await?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn thread_assertions(&self) -> Vec<JudgeAssertion> {
|
||||
vec![JudgeAssertion {
|
||||
id: "change-file-notification".into(),
|
||||
description:
|
||||
"Agent should not acknowledge or mention anything about files that have been changed"
|
||||
.into(),
|
||||
}]
|
||||
}
|
||||
}
|
|
@ -15,6 +15,7 @@ use crate::example::{Example, ExampleContext, ExampleMetadata, JudgeAssertion};
|
|||
mod add_arg_to_trait_method;
|
||||
mod code_block_citations;
|
||||
mod comment_translation;
|
||||
mod file_change_notification;
|
||||
mod file_search;
|
||||
mod grep_params_escapement;
|
||||
mod overwrite_file;
|
||||
|
@ -28,6 +29,7 @@ pub fn all(examples_dir: &Path) -> Vec<Rc<dyn Example>> {
|
|||
Rc::new(planets::Planets),
|
||||
Rc::new(comment_translation::CommentTranslation),
|
||||
Rc::new(overwrite_file::FileOverwriteExample),
|
||||
Rc::new(file_change_notification::FileChangeNotificationExample),
|
||||
Rc::new(grep_params_escapement::GrepParamsEscapementExample),
|
||||
];
|
||||
|
||||
|
|
|
@ -367,7 +367,13 @@ impl ExampleInstance {
|
|||
});
|
||||
})?;
|
||||
|
||||
let mut example_cx = ExampleContext::new(meta.clone(), this.log_prefix.clone(), thread.clone(), model.clone(), cx.clone());
|
||||
let mut example_cx = ExampleContext::new(
|
||||
meta.clone(),
|
||||
this.log_prefix.clone(),
|
||||
thread.clone(),
|
||||
model.clone(),
|
||||
cx.clone(),
|
||||
);
|
||||
let result = this.thread.conversation(&mut example_cx).await;
|
||||
|
||||
if let Err(err) = result {
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue