From b0f192ec2ece9e01b794e0ae3d45f4495b9afb7a Mon Sep 17 00:00:00 2001 From: Oleksiy Syvokon Date: Fri, 20 Jun 2025 16:56:49 +0300 Subject: [PATCH] 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 --- .../src/prompts/stale_files_prompt_header.txt | 3 - crates/agent/src/thread.rs | 158 +----------------- 2 files changed, 1 insertion(+), 160 deletions(-) delete mode 100644 crates/agent/src/prompts/stale_files_prompt_header.txt diff --git a/crates/agent/src/prompts/stale_files_prompt_header.txt b/crates/agent/src/prompts/stale_files_prompt_header.txt deleted file mode 100644 index f743e239c8..0000000000 --- a/crates/agent/src/prompts/stale_files_prompt_header.txt +++ /dev/null @@ -1,3 +0,0 @@ -[The following is an auto-generated notification; do not reply] - -These files have changed since the last read: diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index 87c70b19d3..edc0ea1152 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -1,4 +1,3 @@ -use std::fmt::Write as _; use std::io::Write; use std::ops::Range; use std::sync::Arc; @@ -37,6 +36,7 @@ use settings::Settings; use thiserror::Error; use ui::Window; use util::{ResultExt as _, post_inc}; + use uuid::Uuid; use zed_llm_client::{CompletionIntent, CompletionRequestStatus}; @@ -1389,8 +1389,6 @@ impl Thread { request.messages[message_ix_to_cache].cache = true; } - self.attach_tracked_files_state(&mut request.messages, cx); - request.tools = available_tools; request.mode = if model.supports_max_mode() { Some(self.completion_mode.into()) @@ -1453,60 +1451,6 @@ impl Thread { request } - fn attach_tracked_files_state( - &self, - messages: &mut Vec, - 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( &mut self, 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] async fn test_storing_profile_setting_per_thread(cx: &mut TestAppContext) { init_test_settings(cx);