From d0fb6120d9583fd46b17aed9d2b9a5b08e302f7e Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 20 Aug 2025 18:39:46 +0200 Subject: [PATCH 1/8] Fix scrollbar flicker when streaming agent2 response (#36606) This was caused by calling `list_state.splice` on updated entries. We don't need to splice the entry, as we'll recompute its measurements automatically when we render it. Release Notes: - N/A --- crates/agent_ui/src/acp/thread_view.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/agent_ui/src/acp/thread_view.rs b/crates/agent_ui/src/acp/thread_view.rs index 9bb5953eafedc47712339197ecc791ab473f8034..87fe133bbaf77abff38e26f7e2fb14e269a35795 100644 --- a/crates/agent_ui/src/acp/thread_view.rs +++ b/crates/agent_ui/src/acp/thread_view.rs @@ -793,7 +793,6 @@ impl AcpThreadView { self.entry_view_state.update(cx, |view_state, cx| { view_state.sync_entry(*index, thread, window, cx) }); - self.list_state.splice(*index..index + 1, 1); } AcpThreadEvent::EntriesRemoved(range) => { self.entry_view_state From 8334cdb35805ca00c574daa623f62dc1867adb67 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Wed, 20 Aug 2025 19:10:43 +0200 Subject: [PATCH 2/8] agent2: Port feedback (#36603) Release Notes: - N/A --------- Co-authored-by: Ben Brandt --- crates/acp_thread/src/connection.rs | 17 ++ crates/agent/src/thread.rs | 53 ----- crates/agent2/src/agent.rs | 25 +++ crates/agent_ui/src/acp/thread_view.rs | 283 ++++++++++++++++++++++++- crates/agent_ui/src/active_thread.rs | 1 - 5 files changed, 321 insertions(+), 58 deletions(-) diff --git a/crates/acp_thread/src/connection.rs b/crates/acp_thread/src/connection.rs index 8cae975ce553b185f2a9c7d4d69d568e9c28673a..dc1a41c81eb0dee6fd62318e568e6f3fa2e10eac 100644 --- a/crates/acp_thread/src/connection.rs +++ b/crates/acp_thread/src/connection.rs @@ -64,6 +64,10 @@ pub trait AgentConnection { None } + fn telemetry(&self) -> Option> { + None + } + fn into_any(self: Rc) -> Rc; } @@ -81,6 +85,19 @@ pub trait AgentSessionResume { fn run(&self, cx: &mut App) -> Task>; } +pub trait AgentTelemetry { + /// The name of the agent used for telemetry. + fn agent_name(&self) -> String; + + /// A representation of the current thread state that can be serialized for + /// storage with telemetry events. + fn thread_data( + &self, + session_id: &acp::SessionId, + cx: &mut App, + ) -> Task>; +} + #[derive(Debug)] pub struct AuthRequired { pub description: Option, diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index a584fba88169e8b385678643db66b26820428c30..7b70fde56ab1e7acb6705aeace82f142dc28a9f3 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -387,7 +387,6 @@ pub struct Thread { cumulative_token_usage: TokenUsage, exceeded_window_error: Option, tool_use_limit_reached: bool, - feedback: Option, retry_state: Option, message_feedback: HashMap, last_received_chunk_at: Option, @@ -487,7 +486,6 @@ impl Thread { cumulative_token_usage: TokenUsage::default(), exceeded_window_error: None, tool_use_limit_reached: false, - feedback: None, retry_state: None, message_feedback: HashMap::default(), last_error_context: None, @@ -612,7 +610,6 @@ impl Thread { cumulative_token_usage: serialized.cumulative_token_usage, exceeded_window_error: None, tool_use_limit_reached: serialized.tool_use_limit_reached, - feedback: None, message_feedback: HashMap::default(), last_error_context: None, last_received_chunk_at: None, @@ -2787,10 +2784,6 @@ impl Thread { cx.emit(ThreadEvent::CancelEditing); } - pub fn feedback(&self) -> Option { - self.feedback - } - pub fn message_feedback(&self, message_id: MessageId) -> Option { self.message_feedback.get(&message_id).copied() } @@ -2852,52 +2845,6 @@ impl Thread { }) } - pub fn report_feedback( - &mut self, - feedback: ThreadFeedback, - cx: &mut Context, - ) -> Task> { - let last_assistant_message_id = self - .messages - .iter() - .rev() - .find(|msg| msg.role == Role::Assistant) - .map(|msg| msg.id); - - if let Some(message_id) = last_assistant_message_id { - self.report_message_feedback(message_id, feedback, cx) - } else { - let final_project_snapshot = Self::project_snapshot(self.project.clone(), cx); - let serialized_thread = self.serialize(cx); - let thread_id = self.id().clone(); - let client = self.project.read(cx).client(); - self.feedback = Some(feedback); - cx.notify(); - - cx.background_spawn(async move { - let final_project_snapshot = final_project_snapshot.await; - let serialized_thread = serialized_thread.await?; - let thread_data = serde_json::to_value(serialized_thread) - .unwrap_or_else(|_| serde_json::Value::Null); - - let rating = match feedback { - ThreadFeedback::Positive => "positive", - ThreadFeedback::Negative => "negative", - }; - telemetry::event!( - "Assistant Thread Rated", - rating, - thread_id, - thread_data, - final_project_snapshot - ); - client.telemetry().flush_events().await; - - Ok(()) - }) - } - } - /// Create a snapshot of the current project state including git information and unsaved buffers. fn project_snapshot( project: Entity, diff --git a/crates/agent2/src/agent.rs b/crates/agent2/src/agent.rs index 1fa307511fd35f55a9aef9cc82c59b1c8e8430b5..2f5f15399ee6c0805a8ce179a2f308a49f479c4d 100644 --- a/crates/agent2/src/agent.rs +++ b/crates/agent2/src/agent.rs @@ -948,11 +948,36 @@ impl acp_thread::AgentConnection for NativeAgentConnection { }) } + fn telemetry(&self) -> Option> { + Some(Rc::new(self.clone()) as Rc) + } + fn into_any(self: Rc) -> Rc { self } } +impl acp_thread::AgentTelemetry for NativeAgentConnection { + fn agent_name(&self) -> String { + "Zed".into() + } + + fn thread_data( + &self, + session_id: &acp::SessionId, + cx: &mut App, + ) -> Task> { + let Some(session) = self.0.read(cx).sessions.get(session_id) else { + return Task::ready(Err(anyhow!("Session not found"))); + }; + + let task = session.thread.read(cx).to_db(cx); + cx.background_spawn(async move { + serde_json::to_value(task.await).context("Failed to serialize thread") + }) + } +} + struct NativeAgentSessionEditor { thread: Entity, acp_thread: WeakEntity, diff --git a/crates/agent_ui/src/acp/thread_view.rs b/crates/agent_ui/src/acp/thread_view.rs index 87fe133bbaf77abff38e26f7e2fb14e269a35795..4ce55cce5657c4eee84953f84108e1326469f1c9 100644 --- a/crates/agent_ui/src/acp/thread_view.rs +++ b/crates/agent_ui/src/acp/thread_view.rs @@ -65,6 +65,12 @@ const RESPONSE_PADDING_X: Pixels = px(19.); pub const MIN_EDITOR_LINES: usize = 4; pub const MAX_EDITOR_LINES: usize = 8; +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +enum ThreadFeedback { + Positive, + Negative, +} + enum ThreadError { PaymentRequired, ModelRequestLimitReached(cloud_llm_client::Plan), @@ -106,6 +112,128 @@ impl ProfileProvider for Entity { } } +#[derive(Default)] +struct ThreadFeedbackState { + feedback: Option, + comments_editor: Option>, +} + +impl ThreadFeedbackState { + pub fn submit( + &mut self, + thread: Entity, + feedback: ThreadFeedback, + window: &mut Window, + cx: &mut App, + ) { + let Some(telemetry) = thread.read(cx).connection().telemetry() else { + return; + }; + + if self.feedback == Some(feedback) { + return; + } + + self.feedback = Some(feedback); + match feedback { + ThreadFeedback::Positive => { + self.comments_editor = None; + } + ThreadFeedback::Negative => { + self.comments_editor = Some(Self::build_feedback_comments_editor(window, cx)); + } + } + let session_id = thread.read(cx).session_id().clone(); + let agent_name = telemetry.agent_name(); + let task = telemetry.thread_data(&session_id, cx); + let rating = match feedback { + ThreadFeedback::Positive => "positive", + ThreadFeedback::Negative => "negative", + }; + cx.background_spawn(async move { + let thread = task.await?; + telemetry::event!( + "Agent Thread Rated", + session_id = session_id, + rating = rating, + agent = agent_name, + thread = thread + ); + anyhow::Ok(()) + }) + .detach_and_log_err(cx); + } + + pub fn submit_comments(&mut self, thread: Entity, cx: &mut App) { + let Some(telemetry) = thread.read(cx).connection().telemetry() else { + return; + }; + + let Some(comments) = self + .comments_editor + .as_ref() + .map(|editor| editor.read(cx).text(cx)) + .filter(|text| !text.trim().is_empty()) + else { + return; + }; + + self.comments_editor.take(); + + let session_id = thread.read(cx).session_id().clone(); + let agent_name = telemetry.agent_name(); + let task = telemetry.thread_data(&session_id, cx); + cx.background_spawn(async move { + let thread = task.await?; + telemetry::event!( + "Agent Thread Feedback Comments", + session_id = session_id, + comments = comments, + agent = agent_name, + thread = thread + ); + anyhow::Ok(()) + }) + .detach_and_log_err(cx); + } + + pub fn clear(&mut self) { + *self = Self::default() + } + + pub fn dismiss_comments(&mut self) { + self.comments_editor.take(); + } + + fn build_feedback_comments_editor(window: &mut Window, cx: &mut App) -> Entity { + let buffer = cx.new(|cx| { + let empty_string = String::new(); + MultiBuffer::singleton(cx.new(|cx| Buffer::local(empty_string, cx)), cx) + }); + + let editor = cx.new(|cx| { + let mut editor = Editor::new( + editor::EditorMode::AutoHeight { + min_lines: 1, + max_lines: Some(4), + }, + buffer, + None, + window, + cx, + ); + editor.set_placeholder_text( + "What went wrong? Share your feedback so we can improve.", + cx, + ); + editor + }); + + editor.read(cx).focus_handle(cx).focus(window); + editor + } +} + pub struct AcpThreadView { agent: Rc, workspace: WeakEntity, @@ -120,6 +248,7 @@ pub struct AcpThreadView { notification_subscriptions: HashMap, Vec>, thread_retry_status: Option, thread_error: Option, + thread_feedback: ThreadFeedbackState, list_state: ListState, scrollbar_state: ScrollbarState, auth_task: Option>, @@ -218,6 +347,7 @@ impl AcpThreadView { scrollbar_state: ScrollbarState::new(list_state).parent_entity(&cx.entity()), thread_retry_status: None, thread_error: None, + thread_feedback: Default::default(), auth_task: None, expanded_tool_calls: HashSet::default(), expanded_thinking_blocks: HashSet::default(), @@ -615,6 +745,7 @@ impl AcpThreadView { ) { self.thread_error.take(); self.editing_message.take(); + self.thread_feedback.clear(); let Some(thread) = self.thread().cloned() else { return; @@ -1087,6 +1218,12 @@ impl AcpThreadView { .w_full() .child(primary) .child(self.render_thread_controls(cx)) + .when_some( + self.thread_feedback.comments_editor.clone(), + |this, editor| { + this.child(Self::render_feedback_feedback_editor(editor, window, cx)) + }, + ) .into_any_element() } else { primary @@ -3556,7 +3693,9 @@ impl AcpThreadView { this.scroll_to_top(cx); })); - h_flex() + let mut container = h_flex() + .id("thread-controls-container") + .group("thread-controls-container") .w_full() .mr_1() .pb_2() @@ -3564,9 +3703,145 @@ impl AcpThreadView { .opacity(0.4) .hover(|style| style.opacity(1.)) .flex_wrap() - .justify_end() - .child(open_as_markdown) - .child(scroll_to_top) + .justify_end(); + + if AgentSettings::get_global(cx).enable_feedback { + let feedback = self.thread_feedback.feedback; + container = container.child( + div().visible_on_hover("thread-controls-container").child( + Label::new( + match feedback { + Some(ThreadFeedback::Positive) => "Thanks for your feedback!", + Some(ThreadFeedback::Negative) => "We appreciate your feedback and will use it to improve.", + None => "Rating the thread sends all of your current conversation to the Zed team.", + } + ) + .color(Color::Muted) + .size(LabelSize::XSmall) + .truncate(), + ), + ).child( + h_flex() + .child( + IconButton::new("feedback-thumbs-up", IconName::ThumbsUp) + .shape(ui::IconButtonShape::Square) + .icon_size(IconSize::Small) + .icon_color(match feedback { + Some(ThreadFeedback::Positive) => Color::Accent, + _ => Color::Ignored, + }) + .tooltip(Tooltip::text("Helpful Response")) + .on_click(cx.listener(move |this, _, window, cx| { + this.handle_feedback_click( + ThreadFeedback::Positive, + window, + cx, + ); + })), + ) + .child( + IconButton::new("feedback-thumbs-down", IconName::ThumbsDown) + .shape(ui::IconButtonShape::Square) + .icon_size(IconSize::Small) + .icon_color(match feedback { + Some(ThreadFeedback::Negative) => Color::Accent, + _ => Color::Ignored, + }) + .tooltip(Tooltip::text("Not Helpful")) + .on_click(cx.listener(move |this, _, window, cx| { + this.handle_feedback_click( + ThreadFeedback::Negative, + window, + cx, + ); + })), + ) + ) + } + + container.child(open_as_markdown).child(scroll_to_top) + } + + fn render_feedback_feedback_editor( + editor: Entity, + window: &mut Window, + cx: &Context, + ) -> Div { + let focus_handle = editor.focus_handle(cx); + v_flex() + .key_context("AgentFeedbackMessageEditor") + .on_action(cx.listener(move |this, _: &menu::Cancel, _, cx| { + this.thread_feedback.dismiss_comments(); + cx.notify(); + })) + .on_action(cx.listener(move |this, _: &menu::Confirm, _window, cx| { + this.submit_feedback_message(cx); + })) + .mb_2() + .mx_4() + .p_2() + .rounded_md() + .border_1() + .border_color(cx.theme().colors().border) + .bg(cx.theme().colors().editor_background) + .child(editor) + .child( + h_flex() + .gap_1() + .justify_end() + .child( + Button::new("dismiss-feedback-message", "Cancel") + .label_size(LabelSize::Small) + .key_binding( + KeyBinding::for_action_in(&menu::Cancel, &focus_handle, window, cx) + .map(|kb| kb.size(rems_from_px(10.))), + ) + .on_click(cx.listener(move |this, _, _window, cx| { + this.thread_feedback.dismiss_comments(); + cx.notify(); + })), + ) + .child( + Button::new("submit-feedback-message", "Share Feedback") + .style(ButtonStyle::Tinted(ui::TintColor::Accent)) + .label_size(LabelSize::Small) + .key_binding( + KeyBinding::for_action_in( + &menu::Confirm, + &focus_handle, + window, + cx, + ) + .map(|kb| kb.size(rems_from_px(10.))), + ) + .on_click(cx.listener(move |this, _, _window, cx| { + this.submit_feedback_message(cx); + })), + ), + ) + } + + fn handle_feedback_click( + &mut self, + feedback: ThreadFeedback, + window: &mut Window, + cx: &mut Context, + ) { + let Some(thread) = self.thread().cloned() else { + return; + }; + + self.thread_feedback.submit(thread, feedback, window, cx); + cx.notify(); + } + + fn submit_feedback_message(&mut self, cx: &mut Context) { + let Some(thread) = self.thread().cloned() else { + return; + }; + + self.thread_feedback.submit_comments(thread, cx); + cx.notify(); } fn render_vertical_scrollbar(&self, cx: &mut Context) -> Stateful
{ diff --git a/crates/agent_ui/src/active_thread.rs b/crates/agent_ui/src/active_thread.rs index e214986b82b973905307c23319144f4544c36c9c..2cad9132950787b9e5404e638275fb92147db5a7 100644 --- a/crates/agent_ui/src/active_thread.rs +++ b/crates/agent_ui/src/active_thread.rs @@ -2349,7 +2349,6 @@ impl ActiveThread { this.submit_feedback_message(message_id, cx); cx.notify(); })) - .on_action(cx.listener(Self::confirm_editing_message)) .mb_2() .mx_4() .p_2() From 41e28a71855c9e5595d3764423e56517e5315931 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Wed, 20 Aug 2025 14:01:18 -0400 Subject: [PATCH 3/8] Add tracked buffers for agent2 mentions (#36608) Release Notes: - N/A --- crates/agent_ui/src/acp/message_editor.rs | 151 ++++++++++++++-------- crates/agent_ui/src/acp/thread_view.rs | 13 +- 2 files changed, 107 insertions(+), 57 deletions(-) diff --git a/crates/agent_ui/src/acp/message_editor.rs b/crates/agent_ui/src/acp/message_editor.rs index a50e33dc312d6a2892a55065a929c8c068d0a55c..ccd33c9247ce1b686bbea41e70dcde0039b49df2 100644 --- a/crates/agent_ui/src/acp/message_editor.rs +++ b/crates/agent_ui/src/acp/message_editor.rs @@ -34,7 +34,7 @@ use settings::Settings; use std::{ cell::Cell, ffi::OsStr, - fmt::{Display, Write}, + fmt::Write, ops::Range, path::{Path, PathBuf}, rc::Rc, @@ -391,30 +391,33 @@ impl MessageEditor { let rope = buffer .read_with(cx, |buffer, _cx| buffer.as_rope().clone()) .log_err()?; - Some(rope) + Some((rope, buffer)) }); cx.background_spawn(async move { - let rope = rope_task.await?; - Some((rel_path, full_path, rope.to_string())) + let (rope, buffer) = rope_task.await?; + Some((rel_path, full_path, rope.to_string(), buffer)) }) })) })?; let contents = cx .background_spawn(async move { - let contents = descendants_future.await.into_iter().flatten(); - contents.collect() + let (contents, tracked_buffers) = descendants_future + .await + .into_iter() + .flatten() + .map(|(rel_path, full_path, rope, buffer)| { + ((rel_path, full_path, rope), buffer) + }) + .unzip(); + (render_directory_contents(contents), tracked_buffers) }) .await; anyhow::Ok(contents) }); let task = cx - .spawn(async move |_, _| { - task.await - .map(|contents| DirectoryContents(contents).to_string()) - .map_err(|e| e.to_string()) - }) + .spawn(async move |_, _| task.await.map_err(|e| e.to_string())) .shared(); self.mention_set @@ -663,7 +666,7 @@ impl MessageEditor { &self, window: &mut Window, cx: &mut Context, - ) -> Task>> { + ) -> Task, Vec>)>> { let contents = self.mention_set .contents(&self.project, self.prompt_store.as_ref(), window, cx); @@ -672,6 +675,7 @@ impl MessageEditor { cx.spawn(async move |_, cx| { let contents = contents.await?; + let mut all_tracked_buffers = Vec::new(); editor.update(cx, |editor, cx| { let mut ix = 0; @@ -702,7 +706,12 @@ impl MessageEditor { chunks.push(chunk); } let chunk = match mention { - Mention::Text { uri, content } => { + Mention::Text { + uri, + content, + tracked_buffers, + } => { + all_tracked_buffers.extend(tracked_buffers.iter().cloned()); acp::ContentBlock::Resource(acp::EmbeddedResource { annotations: None, resource: acp::EmbeddedResourceResource::TextResourceContents( @@ -745,7 +754,7 @@ impl MessageEditor { } }); - chunks + (chunks, all_tracked_buffers) }) }) } @@ -1043,7 +1052,7 @@ impl MessageEditor { .add_fetch_result(url, Task::ready(Ok(text)).shared()); } MentionUri::Directory { abs_path } => { - let task = Task::ready(Ok(text)).shared(); + let task = Task::ready(Ok((text, Vec::new()))).shared(); self.mention_set.directories.insert(abs_path, task); } MentionUri::File { .. } @@ -1153,16 +1162,13 @@ impl MessageEditor { } } -struct DirectoryContents(Arc<[(Arc, PathBuf, String)]>); - -impl Display for DirectoryContents { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - for (_relative_path, full_path, content) in self.0.iter() { - let fence = codeblock_fence_for_path(Some(full_path), None); - write!(f, "\n{fence}\n{content}\n```")?; - } - Ok(()) +fn render_directory_contents(entries: Vec<(Arc, PathBuf, String)>) -> String { + let mut output = String::new(); + for (_relative_path, full_path, content) in entries { + let fence = codeblock_fence_for_path(Some(&full_path), None); + write!(output, "\n{fence}\n{content}\n```").unwrap(); } + output } impl Focusable for MessageEditor { @@ -1328,7 +1334,11 @@ impl Render for ImageHover { #[derive(Debug, Eq, PartialEq)] pub enum Mention { - Text { uri: MentionUri, content: String }, + Text { + uri: MentionUri, + content: String, + tracked_buffers: Vec>, + }, Image(MentionImage), } @@ -1346,7 +1356,7 @@ pub struct MentionSet { images: HashMap>>>, thread_summaries: HashMap>>>, text_thread_summaries: HashMap>>>, - directories: HashMap>>>, + directories: HashMap>), String>>>>, } impl MentionSet { @@ -1382,6 +1392,7 @@ impl MentionSet { self.fetch_results.clear(); self.thread_summaries.clear(); self.text_thread_summaries.clear(); + self.directories.clear(); self.uri_by_crease_id .drain() .map(|(id, _)| id) @@ -1424,7 +1435,14 @@ impl MentionSet { let buffer = buffer_task?.await?; let content = buffer.read_with(cx, |buffer, _cx| buffer.text())?; - anyhow::Ok((crease_id, Mention::Text { uri, content })) + anyhow::Ok(( + crease_id, + Mention::Text { + uri, + content, + tracked_buffers: vec![buffer], + }, + )) }) } MentionUri::Directory { abs_path } => { @@ -1433,11 +1451,14 @@ impl MentionSet { }; let uri = uri.clone(); cx.spawn(async move |_| { + let (content, tracked_buffers) = + content.await.map_err(|e| anyhow::anyhow!("{e}"))?; Ok(( crease_id, Mention::Text { uri, - content: content.await.map_err(|e| anyhow::anyhow!("{e}"))?, + content, + tracked_buffers, }, )) }) @@ -1473,7 +1494,14 @@ impl MentionSet { .collect() })?; - anyhow::Ok((crease_id, Mention::Text { uri, content })) + anyhow::Ok(( + crease_id, + Mention::Text { + uri, + content, + tracked_buffers: vec![buffer], + }, + )) }) } MentionUri::Thread { id, .. } => { @@ -1490,6 +1518,7 @@ impl MentionSet { .await .map_err(|e| anyhow::anyhow!("{e}"))? .to_string(), + tracked_buffers: Vec::new(), }, )) }) @@ -1505,6 +1534,7 @@ impl MentionSet { Mention::Text { uri, content: content.await.map_err(|e| anyhow::anyhow!("{e}"))?, + tracked_buffers: Vec::new(), }, )) }) @@ -1518,7 +1548,14 @@ impl MentionSet { cx.spawn(async move |_| { // TODO: report load errors instead of just logging let text = text_task.await?; - anyhow::Ok((crease_id, Mention::Text { uri, content: text })) + anyhow::Ok(( + crease_id, + Mention::Text { + uri, + content: text, + tracked_buffers: Vec::new(), + }, + )) }) } MentionUri::Fetch { url } => { @@ -1532,6 +1569,7 @@ impl MentionSet { Mention::Text { uri, content: content.await.map_err(|e| anyhow::anyhow!("{e}"))?, + tracked_buffers: Vec::new(), }, )) }) @@ -1703,6 +1741,7 @@ impl Addon for MessageEditorAddon { mod tests { use std::{ops::Range, path::Path, sync::Arc}; + use acp_thread::MentionUri; use agent_client_protocol as acp; use agent2::HistoryStore; use assistant_context::ContextStore; @@ -1815,7 +1854,7 @@ mod tests { editor.backspace(&Default::default(), window, cx); }); - let content = message_editor + let (content, _) = message_editor .update_in(cx, |message_editor, window, cx| { message_editor.contents(window, cx) }) @@ -2046,13 +2085,13 @@ mod tests { .into_values() .collect::>(); - pretty_assertions::assert_eq!( - contents, - [Mention::Text { - content: "1".into(), - uri: url_one.parse().unwrap() - }] - ); + { + let [Mention::Text { content, uri, .. }] = contents.as_slice() else { + panic!("Unexpected mentions"); + }; + pretty_assertions::assert_eq!(content, "1"); + pretty_assertions::assert_eq!(uri, &url_one.parse::().unwrap()); + } cx.simulate_input(" "); @@ -2098,15 +2137,15 @@ mod tests { .into_values() .collect::>(); - assert_eq!(contents.len(), 2); let url_eight = uri!("file:///dir/b/eight.txt"); - pretty_assertions::assert_eq!( - contents[1], - Mention::Text { - content: "8".to_string(), - uri: url_eight.parse().unwrap(), - } - ); + + { + let [_, Mention::Text { content, uri, .. }] = contents.as_slice() else { + panic!("Unexpected mentions"); + }; + pretty_assertions::assert_eq!(content, "8"); + pretty_assertions::assert_eq!(uri, &url_eight.parse::().unwrap()); + } editor.update(&mut cx, |editor, cx| { assert_eq!( @@ -2208,14 +2247,18 @@ mod tests { .into_values() .collect::>(); - assert_eq!(contents.len(), 3); - pretty_assertions::assert_eq!( - contents[2], - Mention::Text { - content: "1".into(), - uri: format!("{url_one}?symbol=MySymbol#L1:1").parse().unwrap(), - } - ); + { + let [_, _, Mention::Text { content, uri, .. }] = contents.as_slice() else { + panic!("Unexpected mentions"); + }; + pretty_assertions::assert_eq!(content, "1"); + pretty_assertions::assert_eq!( + uri, + &format!("{url_one}?symbol=MySymbol#L1:1") + .parse::() + .unwrap() + ); + } cx.run_until_parked(); diff --git a/crates/agent_ui/src/acp/thread_view.rs b/crates/agent_ui/src/acp/thread_view.rs index 4ce55cce5657c4eee84953f84108e1326469f1c9..14f9cacd154f4bcd127a8a3a2714313421bf7b49 100644 --- a/crates/agent_ui/src/acp/thread_view.rs +++ b/crates/agent_ui/src/acp/thread_view.rs @@ -739,7 +739,7 @@ impl AcpThreadView { fn send_impl( &mut self, - contents: Task>>, + contents: Task, Vec>)>>, window: &mut Window, cx: &mut Context, ) { @@ -751,7 +751,7 @@ impl AcpThreadView { return; }; let task = cx.spawn_in(window, async move |this, cx| { - let contents = contents.await?; + let (contents, tracked_buffers) = contents.await?; if contents.is_empty() { return Ok(()); @@ -764,7 +764,14 @@ impl AcpThreadView { message_editor.clear(window, cx); }); })?; - let send = thread.update(cx, |thread, cx| thread.send(contents, cx))?; + let send = thread.update(cx, |thread, cx| { + thread.action_log().update(cx, |action_log, cx| { + for buffer in tracked_buffers { + action_log.buffer_read(buffer, cx) + } + }); + thread.send(contents, cx) + })?; send.await }); From ec8106d1dbe8937a0b0cf7c9250b1491c22c1338 Mon Sep 17 00:00:00 2001 From: Umesh Yadav <23421535+imumesh18@users.noreply.github.com> Date: Wed, 20 Aug 2025 23:44:30 +0530 Subject: [PATCH 4/8] Fix `clippy::println_empty_string`, `clippy::while_let_on_iterator`, `clippy::while_let_on_iterator` lint style violations (#36613) Related: #36577 Release Notes: - N/A --- Cargo.toml | 3 +++ crates/agent/src/context.rs | 6 +++--- crates/agent2/src/thread.rs | 2 +- crates/buffer_diff/src/buffer_diff.rs | 4 ++-- crates/editor/src/display_map/block_map.rs | 4 ++-- crates/editor/src/editor.rs | 4 ++-- crates/editor/src/indent_guides.rs | 4 ++-- crates/editor/src/items.rs | 4 ++-- crates/eval/src/eval.rs | 2 +- crates/eval/src/instance.rs | 4 ++-- crates/git/src/repository.rs | 2 +- crates/language/src/text_diff.rs | 2 +- crates/multi_buffer/src/multi_buffer_tests.rs | 4 ++-- crates/project/src/git_store/git_traversal.rs | 4 ++-- crates/project/src/lsp_store.rs | 4 ++-- crates/settings/src/settings_json.rs | 2 +- crates/tab_switcher/src/tab_switcher.rs | 2 +- crates/terminal_view/src/terminal_view.rs | 4 ++-- crates/vim/src/command.rs | 2 +- crates/vim/src/normal/increment.rs | 4 ++-- 20 files changed, 35 insertions(+), 32 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a2de4aaaed97e011a57180c0fa22ef9a460960ef..6218e8dbb9d77cb096746a69eaf110ed37ce7777 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -904,6 +904,7 @@ ok_expect = "warn" owned_cow = "warn" print_literal = "warn" print_with_newline = "warn" +println_empty_string = "warn" ptr_eq = "warn" question_mark = "warn" redundant_closure = "warn" @@ -924,7 +925,9 @@ unneeded_struct_pattern = "warn" unsafe_removed_from_name = "warn" unused_unit = "warn" unusual_byte_groupings = "warn" +while_let_on_iterator = "warn" write_literal = "warn" +write_with_newline = "warn" writeln_empty_string = "warn" wrong_self_convention = "warn" zero_ptr = "warn" diff --git a/crates/agent/src/context.rs b/crates/agent/src/context.rs index 9bb8fc0eaef2126687f5e3277016469801af682c..a94a933d864d204037b3d575cbdc4d85870aeddb 100644 --- a/crates/agent/src/context.rs +++ b/crates/agent/src/context.rs @@ -362,7 +362,7 @@ impl Display for DirectoryContext { let mut is_first = true; for descendant in &self.descendants { if !is_first { - write!(f, "\n")?; + writeln!(f)?; } else { is_first = false; } @@ -650,7 +650,7 @@ impl TextThreadContextHandle { impl Display for TextThreadContext { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { // TODO: escape title? - write!(f, "\n", self.title)?; + writeln!(f, "", self.title)?; write!(f, "{}", self.text.trim())?; write!(f, "\n") } @@ -716,7 +716,7 @@ impl RulesContextHandle { impl Display for RulesContext { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { if let Some(title) = &self.title { - write!(f, "Rules title: {}\n", title)?; + writeln!(f, "Rules title: {}", title)?; } let code_block = MarkdownCodeBlock { tag: "", diff --git a/crates/agent2/src/thread.rs b/crates/agent2/src/thread.rs index f407ee7de5483d75862f5f57647daad8e57f865c..01c9ab03ba42fe661e148ba2ae1406d2e003d851 100644 --- a/crates/agent2/src/thread.rs +++ b/crates/agent2/src/thread.rs @@ -163,7 +163,7 @@ impl UserMessage { if !content.is_empty() { let _ = write!(&mut markdown, "{}\n\n{}\n", uri.as_link(), content); } else { - let _ = write!(&mut markdown, "{}\n", uri.as_link()); + let _ = writeln!(&mut markdown, "{}", uri.as_link()); } } } diff --git a/crates/buffer_diff/src/buffer_diff.rs b/crates/buffer_diff/src/buffer_diff.rs index bef0c5cfc3093126362bcf468b9b22e3319781c1..10b59d0ba20ee72537406dc4645f8565df6361fc 100644 --- a/crates/buffer_diff/src/buffer_diff.rs +++ b/crates/buffer_diff/src/buffer_diff.rs @@ -2024,8 +2024,8 @@ mod tests { fn gen_working_copy(rng: &mut StdRng, head: &str) -> String { let mut old_lines = { let mut old_lines = Vec::new(); - let mut old_lines_iter = head.lines(); - while let Some(line) = old_lines_iter.next() { + let old_lines_iter = head.lines(); + for line in old_lines_iter { assert!(!line.ends_with("\n")); old_lines.push(line.to_owned()); } diff --git a/crates/editor/src/display_map/block_map.rs b/crates/editor/src/display_map/block_map.rs index 1e0cdc34ac6824bfc3500fdfc2b3a0905ac21dbe..e32a4e45dbfb5d929adf980fc97f338e1b445518 100644 --- a/crates/editor/src/display_map/block_map.rs +++ b/crates/editor/src/display_map/block_map.rs @@ -3183,9 +3183,9 @@ mod tests { // so we special case row 0 to assume a leading '\n'. // // Linehood is the birthright of strings. - let mut input_text_lines = input_text.split('\n').enumerate().peekable(); + let input_text_lines = input_text.split('\n').enumerate().peekable(); let mut block_row = 0; - while let Some((wrap_row, input_line)) = input_text_lines.next() { + for (wrap_row, input_line) in input_text_lines { let wrap_row = wrap_row as u32; let multibuffer_row = wraps_snapshot .to_point(WrapPoint::new(wrap_row, 0), Bias::Left) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 2136d5f4b363c056669f807e6990aa4ffc7ef670..45a90b843b6d020e1db7b4937bcd851cb9e8e58a 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -11021,7 +11021,7 @@ impl Editor { let mut col = 0; let mut changed = false; - while let Some(ch) = chars.next() { + for ch in chars.by_ref() { match ch { ' ' => { reindented_line.push(' '); @@ -11077,7 +11077,7 @@ impl Editor { let mut first_non_indent_char = None; let mut changed = false; - while let Some(ch) = chars.next() { + for ch in chars.by_ref() { match ch { ' ' => { // Keep track of spaces. Append \t when we reach tab_size diff --git a/crates/editor/src/indent_guides.rs b/crates/editor/src/indent_guides.rs index a1de2b604bd51cdae7efaaf19492ade911d6156c..23717eeb158cea0f01e6a4efca6d2ff14a8fa824 100644 --- a/crates/editor/src/indent_guides.rs +++ b/crates/editor/src/indent_guides.rs @@ -164,8 +164,8 @@ pub fn indent_guides_in_range( let end_anchor = snapshot.buffer_snapshot.anchor_after(end_offset); let mut fold_ranges = Vec::>::new(); - let mut folds = snapshot.folds_in_range(start_offset..end_offset).peekable(); - while let Some(fold) = folds.next() { + let folds = snapshot.folds_in_range(start_offset..end_offset).peekable(); + for fold in folds { let start = fold.range.start.to_point(&snapshot.buffer_snapshot); let end = fold.range.end.to_point(&snapshot.buffer_snapshot); if let Some(last_range) = fold_ranges.last_mut() diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 62889c638fdcd1db43e83f3d7b55a80c8e99d996..afc5767de010d67bdbe3e6fd21e1ffcfe840b801 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -103,9 +103,9 @@ impl FollowableItem for Editor { multibuffer = MultiBuffer::new(project.read(cx).capability()); let mut sorted_excerpts = state.excerpts.clone(); sorted_excerpts.sort_by_key(|e| e.id); - let mut sorted_excerpts = sorted_excerpts.into_iter().peekable(); + let sorted_excerpts = sorted_excerpts.into_iter().peekable(); - while let Some(excerpt) = sorted_excerpts.next() { + for excerpt in sorted_excerpts { let Ok(buffer_id) = BufferId::new(excerpt.buffer_id) else { continue; }; diff --git a/crates/eval/src/eval.rs b/crates/eval/src/eval.rs index c5a072eea15d8176e7141072f4ead9724b4fd61a..9e0504abca479483b4e5f49c41eec1f6ba3834f1 100644 --- a/crates/eval/src/eval.rs +++ b/crates/eval/src/eval.rs @@ -706,7 +706,7 @@ fn print_report( println!("Average thread score: {average_thread_score}%"); } - println!(""); + println!(); print_h2("CUMULATIVE TOOL METRICS"); println!("{}", cumulative_tool_metrics); diff --git a/crates/eval/src/instance.rs b/crates/eval/src/instance.rs index 074cb121d3b588e3c82735de65dc3178a6eacc80..c6e4e0b6ec683b63b90920861f3cd023069666e6 100644 --- a/crates/eval/src/instance.rs +++ b/crates/eval/src/instance.rs @@ -913,9 +913,9 @@ impl RequestMarkdown { for tool in &request.tools { write!(&mut tools, "# {}\n\n", tool.name).unwrap(); write!(&mut tools, "{}\n\n", tool.description).unwrap(); - write!( + writeln!( &mut tools, - "{}\n", + "{}", MarkdownCodeBlock { tag: "json", text: &format!("{:#}", tool.input_schema) diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 9c125d2c4726fb369274d782137d384c8dfb4c0c..fd12dafa98575b5d8b0190d6ed4e894ac61e8165 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -916,7 +916,7 @@ impl GitRepository for RealGitRepository { .context("no stdin for git cat-file subprocess")?; let mut stdin = BufWriter::new(stdin); for rev in &revs { - write!(&mut stdin, "{rev}\n")?; + writeln!(&mut stdin, "{rev}")?; } stdin.flush()?; drop(stdin); diff --git a/crates/language/src/text_diff.rs b/crates/language/src/text_diff.rs index cb2242a6b1a04ce23ab2e232f1235f3680a33aa5..11d8a070d213852f0a98078f2ed8c76c9cced47b 100644 --- a/crates/language/src/text_diff.rs +++ b/crates/language/src/text_diff.rs @@ -186,7 +186,7 @@ fn tokenize(text: &str, language_scope: Option) -> impl Iterator< let mut prev = None; let mut start_ix = 0; iter::from_fn(move || { - while let Some((ix, c)) = chars.next() { + for (ix, c) in chars.by_ref() { let mut token = None; let kind = classifier.kind(c); if let Some((prev_char, prev_kind)) = prev diff --git a/crates/multi_buffer/src/multi_buffer_tests.rs b/crates/multi_buffer/src/multi_buffer_tests.rs index 598ee0f9cba32fa13760e14092a423e56860aabe..61b4b0520f23ed50b3b36374710b52c78c37080f 100644 --- a/crates/multi_buffer/src/multi_buffer_tests.rs +++ b/crates/multi_buffer/src/multi_buffer_tests.rs @@ -2250,11 +2250,11 @@ impl ReferenceMultibuffer { let base_buffer = diff.base_text(); let mut offset = buffer_range.start; - let mut hunks = diff + let hunks = diff .hunks_intersecting_range(excerpt.range.clone(), buffer, cx) .peekable(); - while let Some(hunk) = hunks.next() { + for hunk in hunks { // Ignore hunks that are outside the excerpt range. let mut hunk_range = hunk.buffer_range.to_offset(buffer); diff --git a/crates/project/src/git_store/git_traversal.rs b/crates/project/src/git_store/git_traversal.rs index 9eadaeac824756bd3b128ef3e0118ceef1c05680..eee492e482daf746c60836cab172f84b2834b468 100644 --- a/crates/project/src/git_store/git_traversal.rs +++ b/crates/project/src/git_store/git_traversal.rs @@ -42,8 +42,8 @@ impl<'a> GitTraversal<'a> { // other_repo/ // .git/ // our_query.txt - let mut query = path.ancestors(); - while let Some(query) = query.next() { + let query = path.ancestors(); + for query in query { let (_, snapshot) = self .repo_root_to_snapshot .range(Path::new("")..=query) diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 1b461178972b6cf9182c3da2a4fcba4595986eb9..0b58009f37204fa3383bfc73683826aa3b8d7fb3 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -13149,10 +13149,10 @@ fn ensure_uniform_list_compatible_label(label: &mut CodeLabel) { let mut offset_map = vec![0; label.text.len() + 1]; let mut last_char_was_space = false; let mut new_idx = 0; - let mut chars = label.text.char_indices().fuse(); + let chars = label.text.char_indices().fuse(); let mut newlines_removed = false; - while let Some((idx, c)) = chars.next() { + for (idx, c) in chars { offset_map[idx] = new_idx; match c { diff --git a/crates/settings/src/settings_json.rs b/crates/settings/src/settings_json.rs index 8080ec8d5f56b8f82a460dd7edcf0c14cd98c9e9..f112ec811d2828350d41eeab63161c8e345d4d77 100644 --- a/crates/settings/src/settings_json.rs +++ b/crates/settings/src/settings_json.rs @@ -209,7 +209,7 @@ fn replace_value_in_json_text( if ch == ',' { removal_end = existing_value_range.end + offset + 1; // Also consume whitespace after the comma - while let Some((_, next_ch)) = chars.next() { + for (_, next_ch) in chars.by_ref() { if next_ch.is_whitespace() { removal_end += next_ch.len_utf8(); } else { diff --git a/crates/tab_switcher/src/tab_switcher.rs b/crates/tab_switcher/src/tab_switcher.rs index 12af124ec78eb4c2b1bf6915131024d34ee64c93..655b8a2e8f4542fff0a08506e8c635cbd0d7c317 100644 --- a/crates/tab_switcher/src/tab_switcher.rs +++ b/crates/tab_switcher/src/tab_switcher.rs @@ -307,7 +307,7 @@ impl TabSwitcherDelegate { (Reverse(history.get(&item.item.item_id())), item.item_index) ) } - eprintln!(""); + eprintln!(); all_items .sort_by_key(|tab| (Reverse(history.get(&tab.item.item_id())), tab.item_index)); all_items diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 0c16e3fb9d6b39dad0ca9bd083724c36161db742..5b4d32714097a4c0bb24ebe5c9dcf72acf7f5ebe 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -1397,8 +1397,8 @@ fn possible_open_target( let found_entry = worktree .update(cx, |worktree, _| { let worktree_root = worktree.abs_path(); - let mut traversal = worktree.traverse_from_path(true, true, false, "".as_ref()); - while let Some(entry) = traversal.next() { + let traversal = worktree.traverse_from_path(true, true, false, "".as_ref()); + for entry in traversal { if let Some(path_in_worktree) = worktree_paths_to_check .iter() .find(|path_to_check| entry.path.ends_with(&path_to_check.path)) diff --git a/crates/vim/src/command.rs b/crates/vim/src/command.rs index 79d18a85e97c22ec82e7daef2ba7c50a7d3c0990..b57c916db988f683cef6bae15ec562392a488be6 100644 --- a/crates/vim/src/command.rs +++ b/crates/vim/src/command.rs @@ -1492,7 +1492,7 @@ impl OnMatchingLines { let mut search = String::new(); let mut escaped = false; - while let Some(c) = chars.next() { + for c in chars.by_ref() { if escaped { escaped = false; // unescape escaped parens diff --git a/crates/vim/src/normal/increment.rs b/crates/vim/src/normal/increment.rs index 115aef1dabd16826e18cf5a39182f0eec3e670e0..1d2a4e9b6180c8130f2126053e5f54694a6d4081 100644 --- a/crates/vim/src/normal/increment.rs +++ b/crates/vim/src/normal/increment.rs @@ -274,9 +274,9 @@ fn find_boolean(snapshot: &MultiBufferSnapshot, start: Point) -> Option<(Range

Date: Wed, 20 Aug 2025 14:43:29 -0400 Subject: [PATCH 5/8] Remove special case for singleton buffers from `MultiBufferSnapshot::anchor_at` (#36524) This may be responsible for a panic that we've been seeing with increased frequency in agent2 threads. Release Notes: - N/A Co-authored-by: Conrad Irwin --- crates/editor/src/editor.rs | 8 +-- crates/multi_buffer/src/multi_buffer.rs | 66 +++++++++++++++---------- 2 files changed, 43 insertions(+), 31 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 45a90b843b6d020e1db7b4937bcd851cb9e8e58a..25fddf5cf1d44f947e7c4128e82158787cdd28f2 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -4876,11 +4876,7 @@ impl Editor { cx: &mut Context, ) -> bool { let position = self.selections.newest_anchor().head(); - let multibuffer = self.buffer.read(cx); - let Some(buffer) = position - .buffer_id - .and_then(|buffer_id| multibuffer.buffer(buffer_id)) - else { + let Some(buffer) = self.buffer.read(cx).buffer_for_anchor(position, cx) else { return false; }; @@ -5844,7 +5840,7 @@ impl Editor { multibuffer_anchor.start.to_offset(&snapshot) ..multibuffer_anchor.end.to_offset(&snapshot) }; - if newest_anchor.head().buffer_id != Some(buffer.remote_id()) { + if snapshot.buffer_id_for_anchor(newest_anchor.head()) != Some(buffer.remote_id()) { return None; } diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 60e9c14c34437bf79ad5eaa0be60972d95ddff58..f73014a6ff161d8944741d679ab53a61da035975 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -2196,6 +2196,15 @@ impl MultiBuffer { }) } + pub fn buffer_for_anchor(&self, anchor: Anchor, cx: &App) -> Option> { + if let Some(buffer_id) = anchor.buffer_id { + self.buffer(buffer_id) + } else { + let (_, buffer, _) = self.excerpt_containing(anchor, cx)?; + Some(buffer) + } + } + // If point is at the end of the buffer, the last excerpt is returned pub fn point_to_buffer_offset( &self, @@ -5228,15 +5237,6 @@ impl MultiBufferSnapshot { excerpt_offset += ExcerptOffset::new(offset_in_transform); }; - if let Some((excerpt_id, buffer_id, buffer)) = self.as_singleton() { - return Anchor { - buffer_id: Some(buffer_id), - excerpt_id: *excerpt_id, - text_anchor: buffer.anchor_at(excerpt_offset.value, bias), - diff_base_anchor, - }; - } - let mut excerpts = self .excerpts .cursor::>>(&()); @@ -5260,10 +5260,17 @@ impl MultiBufferSnapshot { text_anchor, diff_base_anchor, } - } else if excerpt_offset.is_zero() && bias == Bias::Left { - Anchor::min() } else { - Anchor::max() + let mut anchor = if excerpt_offset.is_zero() && bias == Bias::Left { + Anchor::min() + } else { + Anchor::max() + }; + // TODO this is a hack, remove it + if let Some((excerpt_id, _, _)) = self.as_singleton() { + anchor.excerpt_id = *excerpt_id; + } + anchor } } @@ -6305,6 +6312,14 @@ impl MultiBufferSnapshot { }) } + pub fn buffer_id_for_anchor(&self, anchor: Anchor) -> Option { + if let Some(id) = anchor.buffer_id { + return Some(id); + } + let excerpt = self.excerpt_containing(anchor..anchor)?; + Some(excerpt.buffer_id()) + } + pub fn selections_in_range<'a>( &'a self, range: &'a Range, @@ -6983,19 +6998,20 @@ impl Excerpt { } fn contains(&self, anchor: &Anchor) -> bool { - Some(self.buffer_id) == anchor.buffer_id - && self - .range - .context - .start - .cmp(&anchor.text_anchor, &self.buffer) - .is_le() - && self - .range - .context - .end - .cmp(&anchor.text_anchor, &self.buffer) - .is_ge() + anchor.buffer_id == None + || anchor.buffer_id == Some(self.buffer_id) + && self + .range + .context + .start + .cmp(&anchor.text_anchor, &self.buffer) + .is_le() + && self + .range + .context + .end + .cmp(&anchor.text_anchor, &self.buffer) + .is_ge() } /// The [`Excerpt`]'s start offset in its [`Buffer`] From 74ce543d8b16c33fb418db668ae403909eed4c2e Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Wed, 20 Aug 2025 20:45:40 +0200 Subject: [PATCH 6/8] clippy: println_empty_string & non_minimal_cfg (#36614) - **clippy: Fix println-empty-string** - **clippy: non-minimal-cfg** Related to #36577 Release Notes: - N/A --- Cargo.toml | 1 + crates/agent2/src/thread.rs | 2 +- crates/gpui/src/taffy.rs | 1 - .../tests/derive_inspector_reflection.rs | 14 +------------- crates/tab_switcher/src/tab_switcher.rs | 1 - 5 files changed, 3 insertions(+), 16 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6218e8dbb9d77cb096746a69eaf110ed37ce7777..dcf07b707912e3323c165d5613f221818b1d78a5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -900,6 +900,7 @@ needless_parens_on_range_literals = "warn" needless_pub_self = "warn" needless_return = "warn" needless_return_with_question_mark = "warn" +non_minimal_cfg = "warn" ok_expect = "warn" owned_cow = "warn" print_literal = "warn" diff --git a/crates/agent2/src/thread.rs b/crates/agent2/src/thread.rs index 01c9ab03ba42fe661e148ba2ae1406d2e003d851..62174fd3b475cf14f9259d0a18b6a2685dba6407 100644 --- a/crates/agent2/src/thread.rs +++ b/crates/agent2/src/thread.rs @@ -161,7 +161,7 @@ impl UserMessage { } UserMessageContent::Mention { uri, content } => { if !content.is_empty() { - let _ = write!(&mut markdown, "{}\n\n{}\n", uri.as_link(), content); + let _ = writeln!(&mut markdown, "{}\n\n{}", uri.as_link(), content); } else { let _ = writeln!(&mut markdown, "{}", uri.as_link()); } diff --git a/crates/gpui/src/taffy.rs b/crates/gpui/src/taffy.rs index f198bb771849a0c617d3f4b4a1cf0e5ceda475f5..58386ad1f5031e1427baad05c4db075df1b2d761 100644 --- a/crates/gpui/src/taffy.rs +++ b/crates/gpui/src/taffy.rs @@ -164,7 +164,6 @@ impl TaffyLayoutEngine { // for (a, b) in self.get_edges(id)? { // println!("N{} --> N{}", u64::from(a), u64::from(b)); // } - // println!(""); // if !self.computed_layouts.insert(id) { diff --git a/crates/gpui_macros/tests/derive_inspector_reflection.rs b/crates/gpui_macros/tests/derive_inspector_reflection.rs index aab44a70ce58380f172756a21e69fb19e3eddad5..a0adcb7801e55d7272191a1e4e831b2c9c6b115c 100644 --- a/crates/gpui_macros/tests/derive_inspector_reflection.rs +++ b/crates/gpui_macros/tests/derive_inspector_reflection.rs @@ -34,13 +34,6 @@ trait Transform: Clone { /// Adds one to the value fn add_one(self) -> Self; - - /// cfg attributes are respected - #[cfg(all())] - fn cfg_included(self) -> Self; - - #[cfg(any())] - fn cfg_omitted(self) -> Self; } #[derive(Debug, Clone, PartialEq)] @@ -70,10 +63,6 @@ impl Transform for Number { fn add_one(self) -> Self { Number(self.0 + 1) } - - fn cfg_included(self) -> Self { - Number(self.0) - } } #[test] @@ -83,14 +72,13 @@ fn test_derive_inspector_reflection() { // Get all methods that match the pattern fn(self) -> Self or fn(mut self) -> Self let methods = methods::(); - assert_eq!(methods.len(), 6); + assert_eq!(methods.len(), 5); let method_names: Vec<_> = methods.iter().map(|m| m.name).collect(); assert!(method_names.contains(&"double")); assert!(method_names.contains(&"triple")); assert!(method_names.contains(&"increment")); assert!(method_names.contains(&"quadruple")); assert!(method_names.contains(&"add_one")); - assert!(method_names.contains(&"cfg_included")); // Invoke methods by name let num = Number(5); diff --git a/crates/tab_switcher/src/tab_switcher.rs b/crates/tab_switcher/src/tab_switcher.rs index 655b8a2e8f4542fff0a08506e8c635cbd0d7c317..11e32523b4935f464dba81471a5673549c088eda 100644 --- a/crates/tab_switcher/src/tab_switcher.rs +++ b/crates/tab_switcher/src/tab_switcher.rs @@ -307,7 +307,6 @@ impl TabSwitcherDelegate { (Reverse(history.get(&item.item.item_id())), item.item_index) ) } - eprintln!(); all_items .sort_by_key(|tab| (Reverse(history.get(&tab.item.item_id())), tab.item_index)); all_items From 2813073d7b642bc40c6a2f4188dec8445f9688ae Mon Sep 17 00:00:00 2001 From: Agus Zubiaga Date: Wed, 20 Aug 2025 16:04:10 -0300 Subject: [PATCH 7/8] message editor: Only allow types of content the agent can handle (#36616) Uses the new [`acp::PromptCapabilities`](https://github.com/zed-industries/agent-client-protocol/blob/a39b7f635d67528f0a4e05e086ab283b9fc5cb93/rust/agent.rs#L194-L215) to disable non-file mentions and images for agents that don't support them. Release Notes: - N/A --- Cargo.lock | 4 +- Cargo.toml | 2 +- crates/acp_thread/src/acp_thread.rs | 8 ++ crates/acp_thread/src/connection.rs | 10 ++ crates/agent2/src/agent.rs | 8 ++ crates/agent_servers/src/acp/v0.rs | 8 ++ crates/agent_servers/src/acp/v1.rs | 6 + crates/agent_servers/src/claude.rs | 8 ++ .../agent_ui/src/acp/completion_provider.rs | 124 ++++++++++++------ crates/agent_ui/src/acp/message_editor.rs | 93 +++++++++++-- crates/agent_ui/src/acp/thread_view.rs | 13 ++ 11 files changed, 234 insertions(+), 50 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 342bb1058fb7883d4960de8c6099d2c7f719299b..70b8f630f772e804144dba52a647d24e453d2bd1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -171,9 +171,9 @@ dependencies = [ [[package]] name = "agent-client-protocol" -version = "0.0.26" +version = "0.0.28" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "160971bb53ca0b2e70ebc857c21e24eb448745f1396371015f4c59e9a9e51ed0" +checksum = "4c887e795097665ab95119580534e7cc1335b59e1a7fec296943e534b970f4ed" dependencies = [ "anyhow", "futures 0.3.31", diff --git a/Cargo.toml b/Cargo.toml index dcf07b707912e3323c165d5613f221818b1d78a5..436d4a7f5c2b065621cfe7b15351847699c5b182 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -423,7 +423,7 @@ zlog_settings = { path = "crates/zlog_settings" } # agentic-coding-protocol = "0.0.10" -agent-client-protocol = "0.0.26" +agent-client-protocol = "0.0.28" aho-corasick = "1.1" alacritty_terminal = { git = "https://github.com/zed-industries/alacritty.git", branch = "add-hush-login-flag" } any_vec = "0.14" diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index a1f9b32eba6a92687c676e8f60beba2e9c0453e9..9833e1957c589f99c2051fef4b59c699f9c249f7 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -2598,6 +2598,14 @@ mod tests { } } + fn prompt_capabilities(&self) -> acp::PromptCapabilities { + acp::PromptCapabilities { + image: true, + audio: true, + embedded_context: true, + } + } + fn cancel(&self, session_id: &acp::SessionId, cx: &mut App) { let sessions = self.sessions.lock(); let thread = sessions.get(session_id).unwrap().clone(); diff --git a/crates/acp_thread/src/connection.rs b/crates/acp_thread/src/connection.rs index dc1a41c81eb0dee6fd62318e568e6f3fa2e10eac..791b16141762dc941729042063c1e5923f35b346 100644 --- a/crates/acp_thread/src/connection.rs +++ b/crates/acp_thread/src/connection.rs @@ -38,6 +38,8 @@ pub trait AgentConnection { cx: &mut App, ) -> Task>; + fn prompt_capabilities(&self) -> acp::PromptCapabilities; + fn resume( &self, _session_id: &acp::SessionId, @@ -334,6 +336,14 @@ mod test_support { Task::ready(Ok(thread)) } + fn prompt_capabilities(&self) -> acp::PromptCapabilities { + acp::PromptCapabilities { + image: true, + audio: true, + embedded_context: true, + } + } + fn authenticate( &self, _method_id: acp::AuthMethodId, diff --git a/crates/agent2/src/agent.rs b/crates/agent2/src/agent.rs index 2f5f15399ee6c0805a8ce179a2f308a49f479c4d..c15048ad8c53943ef57b04ebfe90bb9a43c307f3 100644 --- a/crates/agent2/src/agent.rs +++ b/crates/agent2/src/agent.rs @@ -913,6 +913,14 @@ impl acp_thread::AgentConnection for NativeAgentConnection { }) } + fn prompt_capabilities(&self) -> acp::PromptCapabilities { + acp::PromptCapabilities { + image: true, + audio: false, + embedded_context: true, + } + } + fn resume( &self, session_id: &acp::SessionId, diff --git a/crates/agent_servers/src/acp/v0.rs b/crates/agent_servers/src/acp/v0.rs index 30643dd00516a9a10391c70b20d984fa47738f95..be960489293f6db935d5d4cba0160ed807079c64 100644 --- a/crates/agent_servers/src/acp/v0.rs +++ b/crates/agent_servers/src/acp/v0.rs @@ -498,6 +498,14 @@ impl AgentConnection for AcpConnection { }) } + fn prompt_capabilities(&self) -> acp::PromptCapabilities { + acp::PromptCapabilities { + image: false, + audio: false, + embedded_context: false, + } + } + fn cancel(&self, _session_id: &acp::SessionId, cx: &mut App) { let task = self .connection diff --git a/crates/agent_servers/src/acp/v1.rs b/crates/agent_servers/src/acp/v1.rs index e0e92f29ba0c55ac5e7c256f9cfc29f96d68d16b..2e70a5f37a439aab03a0c14b945e7e304544d72a 100644 --- a/crates/agent_servers/src/acp/v1.rs +++ b/crates/agent_servers/src/acp/v1.rs @@ -21,6 +21,7 @@ pub struct AcpConnection { connection: Rc, sessions: Rc>>, auth_methods: Vec, + prompt_capabilities: acp::PromptCapabilities, _io_task: Task>, } @@ -119,6 +120,7 @@ impl AcpConnection { connection: connection.into(), server_name, sessions, + prompt_capabilities: response.agent_capabilities.prompt_capabilities, _io_task: io_task, }) } @@ -206,6 +208,10 @@ impl AgentConnection for AcpConnection { }) } + fn prompt_capabilities(&self) -> acp::PromptCapabilities { + self.prompt_capabilities + } + fn cancel(&self, session_id: &acp::SessionId, cx: &mut App) { let conn = self.connection.clone(); let params = acp::CancelNotification { diff --git a/crates/agent_servers/src/claude.rs b/crates/agent_servers/src/claude.rs index 6b9732b468f334a31b35a14847908d2bad2192d6..8d93557e1ca28531ad86df3767ed936aeed50b45 100644 --- a/crates/agent_servers/src/claude.rs +++ b/crates/agent_servers/src/claude.rs @@ -319,6 +319,14 @@ impl AgentConnection for ClaudeAgentConnection { cx.foreground_executor().spawn(async move { end_rx.await? }) } + fn prompt_capabilities(&self) -> acp::PromptCapabilities { + acp::PromptCapabilities { + image: true, + audio: false, + embedded_context: true, + } + } + fn cancel(&self, session_id: &acp::SessionId, _cx: &mut App) { let sessions = self.sessions.borrow(); let Some(session) = sessions.get(session_id) else { diff --git a/crates/agent_ui/src/acp/completion_provider.rs b/crates/agent_ui/src/acp/completion_provider.rs index d90520d26acf0a8ce14605dc43a12a72816fd133..bf0a3f7a5ab520b01af302820fa3b29b894041bf 100644 --- a/crates/agent_ui/src/acp/completion_provider.rs +++ b/crates/agent_ui/src/acp/completion_provider.rs @@ -1,8 +1,11 @@ +use std::cell::Cell; use std::ops::Range; +use std::rc::Rc; use std::sync::Arc; use std::sync::atomic::AtomicBool; use acp_thread::MentionUri; +use agent_client_protocol as acp; use agent2::{HistoryEntry, HistoryStore}; use anyhow::Result; use editor::{CompletionProvider, Editor, ExcerptId}; @@ -63,6 +66,7 @@ pub struct ContextPickerCompletionProvider { workspace: WeakEntity, history_store: Entity, prompt_store: Option>, + prompt_capabilities: Rc>, } impl ContextPickerCompletionProvider { @@ -71,12 +75,14 @@ impl ContextPickerCompletionProvider { workspace: WeakEntity, history_store: Entity, prompt_store: Option>, + prompt_capabilities: Rc>, ) -> Self { Self { message_editor, workspace, history_store, prompt_store, + prompt_capabilities, } } @@ -544,17 +550,19 @@ impl ContextPickerCompletionProvider { }), ); - const RECENT_COUNT: usize = 2; - let threads = self - .history_store - .read(cx) - .recently_opened_entries(cx) - .into_iter() - .filter(|thread| !mentions.contains(&thread.mention_uri())) - .take(RECENT_COUNT) - .collect::>(); - - recent.extend(threads.into_iter().map(Match::RecentThread)); + if self.prompt_capabilities.get().embedded_context { + const RECENT_COUNT: usize = 2; + let threads = self + .history_store + .read(cx) + .recently_opened_entries(cx) + .into_iter() + .filter(|thread| !mentions.contains(&thread.mention_uri())) + .take(RECENT_COUNT) + .collect::>(); + + recent.extend(threads.into_iter().map(Match::RecentThread)); + } recent } @@ -564,11 +572,17 @@ impl ContextPickerCompletionProvider { workspace: &Entity, cx: &mut App, ) -> Vec { - let mut entries = vec![ - ContextPickerEntry::Mode(ContextPickerMode::File), - ContextPickerEntry::Mode(ContextPickerMode::Symbol), - ContextPickerEntry::Mode(ContextPickerMode::Thread), - ]; + let embedded_context = self.prompt_capabilities.get().embedded_context; + let mut entries = if embedded_context { + vec![ + ContextPickerEntry::Mode(ContextPickerMode::File), + ContextPickerEntry::Mode(ContextPickerMode::Symbol), + ContextPickerEntry::Mode(ContextPickerMode::Thread), + ] + } else { + // File is always available, but we don't need a mode entry + vec![] + }; let has_selection = workspace .read(cx) @@ -583,11 +597,13 @@ impl ContextPickerCompletionProvider { )); } - if self.prompt_store.is_some() { - entries.push(ContextPickerEntry::Mode(ContextPickerMode::Rules)); - } + if embedded_context { + if self.prompt_store.is_some() { + entries.push(ContextPickerEntry::Mode(ContextPickerMode::Rules)); + } - entries.push(ContextPickerEntry::Mode(ContextPickerMode::Fetch)); + entries.push(ContextPickerEntry::Mode(ContextPickerMode::Fetch)); + } entries } @@ -625,7 +641,11 @@ impl CompletionProvider for ContextPickerCompletionProvider { let offset_to_line = buffer.point_to_offset(line_start); let mut lines = buffer.text_for_range(line_start..position).lines(); let line = lines.next()?; - MentionCompletion::try_parse(line, offset_to_line) + MentionCompletion::try_parse( + self.prompt_capabilities.get().embedded_context, + line, + offset_to_line, + ) }); let Some(state) = state else { return Task::ready(Ok(Vec::new())); @@ -745,12 +765,16 @@ impl CompletionProvider for ContextPickerCompletionProvider { let offset_to_line = buffer.point_to_offset(line_start); let mut lines = buffer.text_for_range(line_start..position).lines(); if let Some(line) = lines.next() { - MentionCompletion::try_parse(line, offset_to_line) - .map(|completion| { - completion.source_range.start <= offset_to_line + position.column as usize - && completion.source_range.end >= offset_to_line + position.column as usize - }) - .unwrap_or(false) + MentionCompletion::try_parse( + self.prompt_capabilities.get().embedded_context, + line, + offset_to_line, + ) + .map(|completion| { + completion.source_range.start <= offset_to_line + position.column as usize + && completion.source_range.end >= offset_to_line + position.column as usize + }) + .unwrap_or(false) } else { false } @@ -841,7 +865,7 @@ struct MentionCompletion { } impl MentionCompletion { - fn try_parse(line: &str, offset_to_line: usize) -> Option { + fn try_parse(allow_non_file_mentions: bool, line: &str, offset_to_line: usize) -> Option { let last_mention_start = line.rfind('@')?; if last_mention_start >= line.len() { return Some(Self::default()); @@ -865,7 +889,9 @@ impl MentionCompletion { if let Some(mode_text) = parts.next() { end += mode_text.len(); - if let Some(parsed_mode) = ContextPickerMode::try_from(mode_text).ok() { + if let Some(parsed_mode) = ContextPickerMode::try_from(mode_text).ok() + && (allow_non_file_mentions || matches!(parsed_mode, ContextPickerMode::File)) + { mode = Some(parsed_mode); } else { argument = Some(mode_text.to_string()); @@ -898,10 +924,10 @@ mod tests { #[test] fn test_mention_completion_parse() { - assert_eq!(MentionCompletion::try_parse("Lorem Ipsum", 0), None); + assert_eq!(MentionCompletion::try_parse(true, "Lorem Ipsum", 0), None); assert_eq!( - MentionCompletion::try_parse("Lorem @", 0), + MentionCompletion::try_parse(true, "Lorem @", 0), Some(MentionCompletion { source_range: 6..7, mode: None, @@ -910,7 +936,7 @@ mod tests { ); assert_eq!( - MentionCompletion::try_parse("Lorem @file", 0), + MentionCompletion::try_parse(true, "Lorem @file", 0), Some(MentionCompletion { source_range: 6..11, mode: Some(ContextPickerMode::File), @@ -919,7 +945,7 @@ mod tests { ); assert_eq!( - MentionCompletion::try_parse("Lorem @file ", 0), + MentionCompletion::try_parse(true, "Lorem @file ", 0), Some(MentionCompletion { source_range: 6..12, mode: Some(ContextPickerMode::File), @@ -928,7 +954,7 @@ mod tests { ); assert_eq!( - MentionCompletion::try_parse("Lorem @file main.rs", 0), + MentionCompletion::try_parse(true, "Lorem @file main.rs", 0), Some(MentionCompletion { source_range: 6..19, mode: Some(ContextPickerMode::File), @@ -937,7 +963,7 @@ mod tests { ); assert_eq!( - MentionCompletion::try_parse("Lorem @file main.rs ", 0), + MentionCompletion::try_parse(true, "Lorem @file main.rs ", 0), Some(MentionCompletion { source_range: 6..19, mode: Some(ContextPickerMode::File), @@ -946,7 +972,7 @@ mod tests { ); assert_eq!( - MentionCompletion::try_parse("Lorem @file main.rs Ipsum", 0), + MentionCompletion::try_parse(true, "Lorem @file main.rs Ipsum", 0), Some(MentionCompletion { source_range: 6..19, mode: Some(ContextPickerMode::File), @@ -955,7 +981,7 @@ mod tests { ); assert_eq!( - MentionCompletion::try_parse("Lorem @main", 0), + MentionCompletion::try_parse(true, "Lorem @main", 0), Some(MentionCompletion { source_range: 6..11, mode: None, @@ -963,6 +989,28 @@ mod tests { }) ); - assert_eq!(MentionCompletion::try_parse("test@", 0), None); + assert_eq!(MentionCompletion::try_parse(true, "test@", 0), None); + + // Allowed non-file mentions + + assert_eq!( + MentionCompletion::try_parse(true, "Lorem @symbol main", 0), + Some(MentionCompletion { + source_range: 6..18, + mode: Some(ContextPickerMode::Symbol), + argument: Some("main".to_string()), + }) + ); + + // Disallowed non-file mentions + + assert_eq!( + MentionCompletion::try_parse(false, "Lorem @symbol main", 0), + Some(MentionCompletion { + source_range: 6..18, + mode: None, + argument: Some("main".to_string()), + }) + ); } } diff --git a/crates/agent_ui/src/acp/message_editor.rs b/crates/agent_ui/src/acp/message_editor.rs index ccd33c9247ce1b686bbea41e70dcde0039b49df2..5eab1a4e2db5a970c271b6b74a23e6da91bf5362 100644 --- a/crates/agent_ui/src/acp/message_editor.rs +++ b/crates/agent_ui/src/acp/message_editor.rs @@ -51,7 +51,10 @@ use ui::{ }; use url::Url; use util::ResultExt; -use workspace::{Workspace, notifications::NotifyResultExt as _}; +use workspace::{ + Toast, Workspace, + notifications::{NotificationId, NotifyResultExt as _}, +}; use zed_actions::agent::Chat; const PARSE_SLASH_COMMAND_DEBOUNCE: Duration = Duration::from_millis(50); @@ -64,6 +67,7 @@ pub struct MessageEditor { history_store: Entity, prompt_store: Option>, prevent_slash_commands: bool, + prompt_capabilities: Rc>, _subscriptions: Vec, _parse_slash_command_task: Task<()>, } @@ -96,11 +100,13 @@ impl MessageEditor { }, None, ); + let prompt_capabilities = Rc::new(Cell::new(acp::PromptCapabilities::default())); let completion_provider = ContextPickerCompletionProvider::new( cx.weak_entity(), workspace.clone(), history_store.clone(), prompt_store.clone(), + prompt_capabilities.clone(), ); let semantics_provider = Rc::new(SlashCommandSemanticsProvider { range: Cell::new(None), @@ -158,6 +164,7 @@ impl MessageEditor { history_store, prompt_store, prevent_slash_commands, + prompt_capabilities, _subscriptions: subscriptions, _parse_slash_command_task: Task::ready(()), } @@ -193,6 +200,10 @@ impl MessageEditor { .detach(); } + pub fn set_prompt_capabilities(&mut self, capabilities: acp::PromptCapabilities) { + self.prompt_capabilities.set(capabilities); + } + #[cfg(test)] pub(crate) fn editor(&self) -> &Entity { &self.editor @@ -230,7 +241,7 @@ impl MessageEditor { let Some((excerpt_id, _, _)) = snapshot.buffer_snapshot.as_singleton() else { return Task::ready(()); }; - let Some(anchor) = snapshot + let Some(start_anchor) = snapshot .buffer_snapshot .anchor_in_excerpt(*excerpt_id, start) else { @@ -244,6 +255,33 @@ impl MessageEditor { .unwrap_or_default(); if Img::extensions().contains(&extension) && !extension.contains("svg") { + if !self.prompt_capabilities.get().image { + struct ImagesNotAllowed; + + let end_anchor = snapshot.buffer_snapshot.anchor_before( + start_anchor.to_offset(&snapshot.buffer_snapshot) + content_len + 1, + ); + + self.editor.update(cx, |editor, cx| { + // Remove mention + editor.edit([((start_anchor..end_anchor), "")], cx); + }); + + self.workspace + .update(cx, |workspace, cx| { + workspace.show_toast( + Toast::new( + NotificationId::unique::(), + "This agent does not support images yet", + ) + .autohide(), + cx, + ); + }) + .ok(); + return Task::ready(()); + } + let project = self.project.clone(); let Some(project_path) = project .read(cx) @@ -277,7 +315,7 @@ impl MessageEditor { }; return self.confirm_mention_for_image( crease_id, - anchor, + start_anchor, Some(abs_path.clone()), image, window, @@ -301,17 +339,22 @@ impl MessageEditor { match mention_uri { MentionUri::Fetch { url } => { - self.confirm_mention_for_fetch(crease_id, anchor, url, window, cx) + self.confirm_mention_for_fetch(crease_id, start_anchor, url, window, cx) } MentionUri::Directory { abs_path } => { - self.confirm_mention_for_directory(crease_id, anchor, abs_path, window, cx) + self.confirm_mention_for_directory(crease_id, start_anchor, abs_path, window, cx) } MentionUri::Thread { id, name } => { - self.confirm_mention_for_thread(crease_id, anchor, id, name, window, cx) - } - MentionUri::TextThread { path, name } => { - self.confirm_mention_for_text_thread(crease_id, anchor, path, name, window, cx) + self.confirm_mention_for_thread(crease_id, start_anchor, id, name, window, cx) } + MentionUri::TextThread { path, name } => self.confirm_mention_for_text_thread( + crease_id, + start_anchor, + path, + name, + window, + cx, + ), MentionUri::File { .. } | MentionUri::Symbol { .. } | MentionUri::Rule { .. } @@ -778,6 +821,10 @@ impl MessageEditor { } fn paste(&mut self, _: &Paste, window: &mut Window, cx: &mut Context) { + if !self.prompt_capabilities.get().image { + return; + } + let images = cx .read_from_clipboard() .map(|item| { @@ -2009,6 +2056,34 @@ mod tests { (message_editor, editor) }); + cx.simulate_input("Lorem @"); + + editor.update_in(&mut cx, |editor, window, cx| { + assert_eq!(editor.text(cx), "Lorem @"); + assert!(editor.has_visible_completions_menu()); + + // Only files since we have default capabilities + assert_eq!( + current_completion_labels(editor), + &[ + "eight.txt dir/b/", + "seven.txt dir/b/", + "six.txt dir/b/", + "five.txt dir/b/", + ] + ); + editor.set_text("", window, cx); + }); + + message_editor.update(&mut cx, |editor, _cx| { + // Enable all prompt capabilities + editor.set_prompt_capabilities(acp::PromptCapabilities { + image: true, + audio: true, + embedded_context: true, + }); + }); + cx.simulate_input("Lorem "); editor.update(&mut cx, |editor, cx| { diff --git a/crates/agent_ui/src/acp/thread_view.rs b/crates/agent_ui/src/acp/thread_view.rs index 14f9cacd154f4bcd127a8a3a2714313421bf7b49..81a56165c81e7ad794f288d54da04e0ffb2a8ea4 100644 --- a/crates/agent_ui/src/acp/thread_view.rs +++ b/crates/agent_ui/src/acp/thread_view.rs @@ -492,6 +492,11 @@ impl AcpThreadView { }) }); + this.message_editor.update(cx, |message_editor, _cx| { + message_editor + .set_prompt_capabilities(connection.prompt_capabilities()); + }); + cx.notify(); } Err(err) => { @@ -4762,6 +4767,14 @@ pub(crate) mod tests { &[] } + fn prompt_capabilities(&self) -> acp::PromptCapabilities { + acp::PromptCapabilities { + image: true, + audio: true, + embedded_context: true, + } + } + fn authenticate( &self, _method_id: acp::AuthMethodId, From b0bef3a9a279e98abc499d4e2f7850ff7f5959ea Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Wed, 20 Aug 2025 21:17:07 +0200 Subject: [PATCH 8/8] agent2: Clean up tool descriptions (#36619) schemars was passing along the newlines from the doc comments. This should make these closer to the markdown file versions we had in the old agent. Release Notes: - N/A --- crates/agent2/src/tools/copy_path_tool.rs | 15 ++++----------- .../agent2/src/tools/create_directory_tool.rs | 7 ++----- crates/agent2/src/tools/delete_path_tool.rs | 3 +-- crates/agent2/src/tools/edit_file_tool.rs | 19 ++++++------------- crates/agent2/src/tools/find_path_tool.rs | 1 - crates/agent2/src/tools/grep_tool.rs | 3 +-- .../agent2/src/tools/list_directory_tool.rs | 6 ++---- crates/agent2/src/tools/move_path_tool.rs | 9 +++------ crates/agent2/src/tools/open_tool.rs | 10 +++------- crates/agent2/src/tools/read_file_tool.rs | 5 +---- crates/agent2/src/tools/thinking_tool.rs | 3 +-- crates/agent2/src/tools/web_search_tool.rs | 2 +- 12 files changed, 25 insertions(+), 58 deletions(-) diff --git a/crates/agent2/src/tools/copy_path_tool.rs b/crates/agent2/src/tools/copy_path_tool.rs index f973b86990af76ea923d548f95a4f05b4cd32c18..4b40a9842f69cb87977ae948c4f858a2540b2eb1 100644 --- a/crates/agent2/src/tools/copy_path_tool.rs +++ b/crates/agent2/src/tools/copy_path_tool.rs @@ -8,16 +8,11 @@ use serde::{Deserialize, Serialize}; use std::sync::Arc; use util::markdown::MarkdownInlineCode; -/// Copies a file or directory in the project, and returns confirmation that the -/// copy succeeded. -/// +/// Copies a file or directory in the project, and returns confirmation that the copy succeeded. /// Directory contents will be copied recursively (like `cp -r`). /// -/// This tool should be used when it's desirable to create a copy of a file or -/// directory without modifying the original. It's much more efficient than -/// doing this by separately reading and then writing the file or directory's -/// contents, so this tool should be preferred over that approach whenever -/// copying is the goal. +/// This tool should be used when it's desirable to create a copy of a file or directory without modifying the original. +/// It's much more efficient than doing this by separately reading and then writing the file or directory's contents, so this tool should be preferred over that approach whenever copying is the goal. #[derive(Debug, Serialize, Deserialize, JsonSchema)] pub struct CopyPathToolInput { /// The source path of the file or directory to copy. @@ -33,12 +28,10 @@ pub struct CopyPathToolInput { /// You can copy the first file by providing a source_path of "directory1/a/something.txt" /// pub source_path: String, - /// The destination path where the file or directory should be copied to. /// /// - /// To copy "directory1/a/something.txt" to "directory2/b/copy.txt", - /// provide a destination_path of "directory2/b/copy.txt" + /// To copy "directory1/a/something.txt" to "directory2/b/copy.txt", provide a destination_path of "directory2/b/copy.txt" /// pub destination_path: String, } diff --git a/crates/agent2/src/tools/create_directory_tool.rs b/crates/agent2/src/tools/create_directory_tool.rs index c173c5ae67512813b610552c2001dc16ceb38212..7720eb3595d475560b90cc890396b8f6b27600b1 100644 --- a/crates/agent2/src/tools/create_directory_tool.rs +++ b/crates/agent2/src/tools/create_directory_tool.rs @@ -9,12 +9,9 @@ use util::markdown::MarkdownInlineCode; use crate::{AgentTool, ToolCallEventStream}; -/// Creates a new directory at the specified path within the project. Returns -/// confirmation that the directory was created. +/// Creates a new directory at the specified path within the project. Returns confirmation that the directory was created. /// -/// This tool creates a directory and all necessary parent directories (similar -/// to `mkdir -p`). It should be used whenever you need to create new -/// directories within the project. +/// This tool creates a directory and all necessary parent directories (similar to `mkdir -p`). It should be used whenever you need to create new directories within the project. #[derive(Debug, Serialize, Deserialize, JsonSchema)] pub struct CreateDirectoryToolInput { /// The path of the new directory. diff --git a/crates/agent2/src/tools/delete_path_tool.rs b/crates/agent2/src/tools/delete_path_tool.rs index e013b3a3e755cf6662718d620264cb1e38fa5417..c281f1b5b69e2f3cbc3282a17298e9002f3b7c52 100644 --- a/crates/agent2/src/tools/delete_path_tool.rs +++ b/crates/agent2/src/tools/delete_path_tool.rs @@ -9,8 +9,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::sync::Arc; -/// Deletes the file or directory (and the directory's contents, recursively) at -/// the specified path in the project, and returns confirmation of the deletion. +/// Deletes the file or directory (and the directory's contents, recursively) at the specified path in the project, and returns confirmation of the deletion. #[derive(Debug, Serialize, Deserialize, JsonSchema)] pub struct DeletePathToolInput { /// The path of the file or directory to delete. diff --git a/crates/agent2/src/tools/edit_file_tool.rs b/crates/agent2/src/tools/edit_file_tool.rs index 24fedda4eb57d36db9e8769a107a947fed998a05..f89cace9a84e1f9a877daf9a60503b8d78c8c336 100644 --- a/crates/agent2/src/tools/edit_file_tool.rs +++ b/crates/agent2/src/tools/edit_file_tool.rs @@ -34,25 +34,21 @@ const DEFAULT_UI_TEXT: &str = "Editing file"; /// - Use the `list_directory` tool to verify the parent directory exists and is the correct location #[derive(Debug, Serialize, Deserialize, JsonSchema)] pub struct EditFileToolInput { - /// A one-line, user-friendly markdown description of the edit. This will be - /// shown in the UI and also passed to another model to perform the edit. + /// A one-line, user-friendly markdown description of the edit. This will be shown in the UI and also passed to another model to perform the edit. /// - /// Be terse, but also descriptive in what you want to achieve with this - /// edit. Avoid generic instructions. + /// Be terse, but also descriptive in what you want to achieve with this edit. Avoid generic instructions. /// /// NEVER mention the file path in this description. /// /// Fix API endpoint URLs /// Update copyright year in `page_footer` /// - /// Make sure to include this field before all the others in the input object - /// so that we can display it immediately. + /// Make sure to include this field before all the others in the input object so that we can display it immediately. pub display_description: String, /// The full path of the file to create or modify in the project. /// - /// WARNING: When specifying which file path need changing, you MUST - /// start each path with one of the project's root directories. + /// WARNING: When specifying which file path need changing, you MUST start each path with one of the project's root directories. /// /// The following examples assume we have two root directories in the project: /// - /a/b/backend @@ -61,22 +57,19 @@ pub struct EditFileToolInput { /// /// `backend/src/main.rs` /// - /// Notice how the file path starts with `backend`. Without that, the path - /// would be ambiguous and the call would fail! + /// Notice how the file path starts with `backend`. Without that, the path would be ambiguous and the call would fail! /// /// /// /// `frontend/db.js` /// pub path: PathBuf, - /// The mode of operation on the file. Possible values: /// - 'edit': Make granular edits to an existing file. /// - 'create': Create a new file if it doesn't exist. /// - 'overwrite': Replace the entire contents of an existing file. /// - /// When a file already exists or you just created it, prefer editing - /// it as opposed to recreating it from scratch. + /// When a file already exists or you just created it, prefer editing it as opposed to recreating it from scratch. pub mode: EditFileMode, } diff --git a/crates/agent2/src/tools/find_path_tool.rs b/crates/agent2/src/tools/find_path_tool.rs index deccf37ab71109fadd1d394ee4ee15000c74f2e5..9e11ca6a37d92d6c189e542611fbc396128e6a15 100644 --- a/crates/agent2/src/tools/find_path_tool.rs +++ b/crates/agent2/src/tools/find_path_tool.rs @@ -31,7 +31,6 @@ pub struct FindPathToolInput { /// You can get back the first two paths by providing a glob of "*thing*.txt" /// pub glob: String, - /// Optional starting position for paginated results (0-based). /// When not provided, starts from the beginning. #[serde(default)] diff --git a/crates/agent2/src/tools/grep_tool.rs b/crates/agent2/src/tools/grep_tool.rs index 265c26926d816a00a414755ea1193eb22d1c915f..955dae723558c3b8b3324109c18e9448215d66a3 100644 --- a/crates/agent2/src/tools/grep_tool.rs +++ b/crates/agent2/src/tools/grep_tool.rs @@ -27,8 +27,7 @@ use util::paths::PathMatcher; /// - DO NOT use HTML entities solely to escape characters in the tool parameters. #[derive(Debug, Serialize, Deserialize, JsonSchema)] pub struct GrepToolInput { - /// A regex pattern to search for in the entire project. Note that the regex - /// will be parsed by the Rust `regex` crate. + /// A regex pattern to search for in the entire project. Note that the regex will be parsed by the Rust `regex` crate. /// /// Do NOT specify a path here! This will only be matched against the code **content**. pub regex: String, diff --git a/crates/agent2/src/tools/list_directory_tool.rs b/crates/agent2/src/tools/list_directory_tool.rs index 61f21d8f95117f0b0a8efccf7481874037af365c..31575a92e44aa66558175b078b6c8e6087a67653 100644 --- a/crates/agent2/src/tools/list_directory_tool.rs +++ b/crates/agent2/src/tools/list_directory_tool.rs @@ -10,14 +10,12 @@ use std::fmt::Write; use std::{path::Path, sync::Arc}; use util::markdown::MarkdownInlineCode; -/// Lists files and directories in a given path. Prefer the `grep` or -/// `find_path` tools when searching the codebase. +/// Lists files and directories in a given path. Prefer the `grep` or `find_path` tools when searching the codebase. #[derive(Debug, Serialize, Deserialize, JsonSchema)] pub struct ListDirectoryToolInput { /// The fully-qualified path of the directory to list in the project. /// - /// This path should never be absolute, and the first component - /// of the path should always be a root directory in a project. + /// This path should never be absolute, and the first component of the path should always be a root directory in a project. /// /// /// If the project has the following root directories: diff --git a/crates/agent2/src/tools/move_path_tool.rs b/crates/agent2/src/tools/move_path_tool.rs index f8d5d0d176e5cd53d1f563385797596048c9a87e..2a173a4404fc344051d238c6ba377e63ec2d9acc 100644 --- a/crates/agent2/src/tools/move_path_tool.rs +++ b/crates/agent2/src/tools/move_path_tool.rs @@ -8,14 +8,11 @@ use serde::{Deserialize, Serialize}; use std::{path::Path, sync::Arc}; use util::markdown::MarkdownInlineCode; -/// Moves or rename a file or directory in the project, and returns confirmation -/// that the move succeeded. +/// Moves or rename a file or directory in the project, and returns confirmation that the move succeeded. /// -/// If the source and destination directories are the same, but the filename is -/// different, this performs a rename. Otherwise, it performs a move. +/// If the source and destination directories are the same, but the filename is different, this performs a rename. Otherwise, it performs a move. /// -/// This tool should be used when it's desirable to move or rename a file or -/// directory without changing its contents at all. +/// This tool should be used when it's desirable to move or rename a file or directory without changing its contents at all. #[derive(Debug, Serialize, Deserialize, JsonSchema)] pub struct MovePathToolInput { /// The source path of the file or directory to move/rename. diff --git a/crates/agent2/src/tools/open_tool.rs b/crates/agent2/src/tools/open_tool.rs index 36420560c1832d40496a95c69505ab8eb9cbb2c6..c20369c2d80cadc99f60c8335b5ecdbef763bf5f 100644 --- a/crates/agent2/src/tools/open_tool.rs +++ b/crates/agent2/src/tools/open_tool.rs @@ -8,19 +8,15 @@ use serde::{Deserialize, Serialize}; use std::{path::PathBuf, sync::Arc}; use util::markdown::MarkdownEscaped; -/// This tool opens a file or URL with the default application associated with -/// it on the user's operating system: +/// This tool opens a file or URL with the default application associated with it on the user's operating system: /// /// - On macOS, it's equivalent to the `open` command /// - On Windows, it's equivalent to `start` /// - On Linux, it uses something like `xdg-open`, `gio open`, `gnome-open`, `kde-open`, `wslview` as appropriate /// -/// For example, it can open a web browser with a URL, open a PDF file with the -/// default PDF viewer, etc. +/// For example, it can open a web browser with a URL, open a PDF file with the default PDF viewer, etc. /// -/// You MUST ONLY use this tool when the user has explicitly requested opening -/// something. You MUST NEVER assume that the user would like for you to use -/// this tool. +/// You MUST ONLY use this tool when the user has explicitly requested opening something. You MUST NEVER assume that the user would like for you to use this tool. #[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] pub struct OpenToolInput { /// The path or URL to open with the default application. diff --git a/crates/agent2/src/tools/read_file_tool.rs b/crates/agent2/src/tools/read_file_tool.rs index f37dff4f47adfa65ca8145dd88d591467f60956a..11a57506fb6454ad3c527e68f43089f5b80216e1 100644 --- a/crates/agent2/src/tools/read_file_tool.rs +++ b/crates/agent2/src/tools/read_file_tool.rs @@ -21,8 +21,7 @@ use crate::{AgentTool, ToolCallEventStream}; pub struct ReadFileToolInput { /// The relative path of the file to read. /// - /// This path should never be absolute, and the first component - /// of the path should always be a root directory in a project. + /// This path should never be absolute, and the first component of the path should always be a root directory in a project. /// /// /// If the project has the following root directories: @@ -34,11 +33,9 @@ pub struct ReadFileToolInput { /// If you want to access `file.txt` in `directory2`, you should use the path `directory2/file.txt`. /// pub path: String, - /// Optional line number to start reading on (1-based index) #[serde(default)] pub start_line: Option, - /// Optional line number to end reading on (1-based index, inclusive) #[serde(default)] pub end_line: Option, diff --git a/crates/agent2/src/tools/thinking_tool.rs b/crates/agent2/src/tools/thinking_tool.rs index 43647bb468d808b978a1b5176539a3167c5065f6..c5e94511621768868cd56f165b4f50c903874e0d 100644 --- a/crates/agent2/src/tools/thinking_tool.rs +++ b/crates/agent2/src/tools/thinking_tool.rs @@ -11,8 +11,7 @@ use crate::{AgentTool, ToolCallEventStream}; /// Use this tool when you need to work through complex problems, develop strategies, or outline approaches before taking action. #[derive(Debug, Serialize, Deserialize, JsonSchema)] pub struct ThinkingToolInput { - /// Content to think about. This should be a description of what to think about or - /// a problem to solve. + /// Content to think about. This should be a description of what to think about or a problem to solve. content: String, } diff --git a/crates/agent2/src/tools/web_search_tool.rs b/crates/agent2/src/tools/web_search_tool.rs index d71a128bfe4f70a95aa71d776b76bd4f5426800a..ffcd4ad3becf0417aa6175808614c71abdd95e8e 100644 --- a/crates/agent2/src/tools/web_search_tool.rs +++ b/crates/agent2/src/tools/web_search_tool.rs @@ -14,7 +14,7 @@ use ui::prelude::*; use web_search::WebSearchRegistry; /// Search the web for information using your query. -/// Use this when you need real-time information, facts, or data that might not be in your training. \ +/// Use this when you need real-time information, facts, or data that might not be in your training. /// Results will include snippets and links from relevant web pages. #[derive(Debug, Serialize, Deserialize, JsonSchema)] pub struct WebSearchToolInput {