agent: Do not send stale files notifications (#32974)
Removing it for two reasons: 1. We need a better implementation that doesn't hurt caching and doesn't distracts the agent too much (see https://github.com/zed-industries/zed/pull/32876 for more context) 2. Current insertion point of notifications doesn't play well with Claude Thinking models (see https://github.com/zed-industries/zed/issues/33000#issuecomment-2991709484) I think we should get this code back in a form of a tool. But for now, I'm dropping it to resolve recent issues. Closes #33000 Release Notes: - N/A
This commit is contained in:
parent
c9e5ff21a1
commit
b0f192ec2e
2 changed files with 1 additions and 160 deletions
|
@ -1,3 +0,0 @@
|
||||||
[The following is an auto-generated notification; do not reply]
|
|
||||||
|
|
||||||
These files have changed since the last read:
|
|
|
@ -1,4 +1,3 @@
|
||||||
use std::fmt::Write as _;
|
|
||||||
use std::io::Write;
|
use std::io::Write;
|
||||||
use std::ops::Range;
|
use std::ops::Range;
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
|
@ -37,6 +36,7 @@ use settings::Settings;
|
||||||
use thiserror::Error;
|
use thiserror::Error;
|
||||||
use ui::Window;
|
use ui::Window;
|
||||||
use util::{ResultExt as _, post_inc};
|
use util::{ResultExt as _, post_inc};
|
||||||
|
|
||||||
use uuid::Uuid;
|
use uuid::Uuid;
|
||||||
use zed_llm_client::{CompletionIntent, CompletionRequestStatus};
|
use zed_llm_client::{CompletionIntent, CompletionRequestStatus};
|
||||||
|
|
||||||
|
@ -1389,8 +1389,6 @@ impl Thread {
|
||||||
request.messages[message_ix_to_cache].cache = true;
|
request.messages[message_ix_to_cache].cache = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
self.attach_tracked_files_state(&mut request.messages, cx);
|
|
||||||
|
|
||||||
request.tools = available_tools;
|
request.tools = available_tools;
|
||||||
request.mode = if model.supports_max_mode() {
|
request.mode = if model.supports_max_mode() {
|
||||||
Some(self.completion_mode.into())
|
Some(self.completion_mode.into())
|
||||||
|
@ -1453,60 +1451,6 @@ impl Thread {
|
||||||
request
|
request
|
||||||
}
|
}
|
||||||
|
|
||||||
fn attach_tracked_files_state(
|
|
||||||
&self,
|
|
||||||
messages: &mut Vec<LanguageModelRequestMessage>,
|
|
||||||
cx: &App,
|
|
||||||
) {
|
|
||||||
let mut stale_files = String::new();
|
|
||||||
|
|
||||||
let action_log = self.action_log.read(cx);
|
|
||||||
|
|
||||||
for stale_file in action_log.stale_buffers(cx) {
|
|
||||||
if let Some(file) = stale_file.read(cx).file() {
|
|
||||||
writeln!(&mut stale_files, "- {}", file.path().display()).ok();
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if stale_files.is_empty() {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
// 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"),
|
|
||||||
);
|
|
||||||
|
|
||||||
// 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;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
pub fn stream_completion(
|
pub fn stream_completion(
|
||||||
&mut self,
|
&mut self,
|
||||||
request: LanguageModelRequest,
|
request: LanguageModelRequest,
|
||||||
|
@ -3229,106 +3173,6 @@ fn main() {{
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[gpui::test]
|
|
||||||
async fn test_stale_buffer_notification(cx: &mut TestAppContext) {
|
|
||||||
init_test_settings(cx);
|
|
||||||
|
|
||||||
let project = create_test_project(
|
|
||||||
cx,
|
|
||||||
json!({"code.rs": "fn main() {\n println!(\"Hello, world!\");\n}"}),
|
|
||||||
)
|
|
||||||
.await;
|
|
||||||
|
|
||||||
let (_workspace, _thread_store, thread, context_store, model) =
|
|
||||||
setup_test_environment(cx, project.clone()).await;
|
|
||||||
|
|
||||||
// Open buffer and add it to context
|
|
||||||
let buffer = add_file_to_context(&project, &context_store, "test/code.rs", cx)
|
|
||||||
.await
|
|
||||||
.unwrap();
|
|
||||||
|
|
||||||
let context =
|
|
||||||
context_store.read_with(cx, |store, _| store.context().next().cloned().unwrap());
|
|
||||||
let loaded_context = cx
|
|
||||||
.update(|cx| load_context(vec![context], &project, &None, cx))
|
|
||||||
.await;
|
|
||||||
|
|
||||||
// Insert user message with the buffer as context
|
|
||||||
thread.update(cx, |thread, cx| {
|
|
||||||
thread.insert_user_message("Explain this code", loaded_context, None, Vec::new(), cx)
|
|
||||||
});
|
|
||||||
|
|
||||||
// Create a request and check that it doesn't have a stale buffer warning yet
|
|
||||||
let initial_request = thread.update(cx, |thread, cx| {
|
|
||||||
thread.to_completion_request(model.clone(), CompletionIntent::UserPrompt, cx)
|
|
||||||
});
|
|
||||||
|
|
||||||
// Make sure we don't have a stale file warning yet
|
|
||||||
let has_stale_warning = initial_request.messages.iter().any(|msg| {
|
|
||||||
msg.string_contents()
|
|
||||||
.contains("These files changed since last read:")
|
|
||||||
});
|
|
||||||
assert!(
|
|
||||||
!has_stale_warning,
|
|
||||||
"Should not have stale buffer warning before buffer is modified"
|
|
||||||
);
|
|
||||||
|
|
||||||
// Modify the buffer
|
|
||||||
buffer.update(cx, |buffer, cx| {
|
|
||||||
// Find a position at the end of line 1
|
|
||||||
buffer.edit(
|
|
||||||
[(1..1, "\n println!(\"Added a new line\");\n")],
|
|
||||||
None,
|
|
||||||
cx,
|
|
||||||
);
|
|
||||||
});
|
|
||||||
|
|
||||||
// Insert another user message without context
|
|
||||||
thread.update(cx, |thread, cx| {
|
|
||||||
thread.insert_user_message(
|
|
||||||
"What does the code do now?",
|
|
||||||
ContextLoadResult::default(),
|
|
||||||
None,
|
|
||||||
Vec::new(),
|
|
||||||
cx,
|
|
||||||
)
|
|
||||||
});
|
|
||||||
|
|
||||||
// Create a new request and check for the stale buffer warning
|
|
||||||
let new_request = thread.update(cx, |thread, cx| {
|
|
||||||
thread.to_completion_request(model.clone(), CompletionIntent::UserPrompt, cx)
|
|
||||||
});
|
|
||||||
|
|
||||||
// We should have a stale file warning as the last message
|
|
||||||
let last_message = new_request
|
|
||||||
.messages
|
|
||||||
.last()
|
|
||||||
.expect("Request should have messages");
|
|
||||||
|
|
||||||
// The last message should be the stale buffer notification
|
|
||||||
assert_eq!(last_message.role, Role::User);
|
|
||||||
|
|
||||||
// Check the exact content of the message
|
|
||||||
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]
|
#[gpui::test]
|
||||||
async fn test_storing_profile_setting_per_thread(cx: &mut TestAppContext) {
|
async fn test_storing_profile_setting_per_thread(cx: &mut TestAppContext) {
|
||||||
init_test_settings(cx);
|
init_test_settings(cx);
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue