diff --git a/crates/agent/src/active_thread.rs b/crates/agent/src/active_thread.rs index 7920b24501..6765710f77 100644 --- a/crates/agent/src/active_thread.rs +++ b/crates/agent/src/active_thread.rs @@ -1099,36 +1099,47 @@ impl ActiveThread { .into_any_element(), }; - let message_content = v_flex() - .gap_1p5() - .child( - if let Some(edit_message_editor) = edit_message_editor.clone() { - div() - .key_context("EditMessageEditor") - .on_action(cx.listener(Self::cancel_editing_message)) - .on_action(cx.listener(Self::confirm_editing_message)) - .min_h_6() - .child(edit_message_editor) - } else { - div() - .min_h_6() - .text_ui(cx) - .child(self.render_message_content( - message_id, - rendered_message, - has_tool_uses, - cx, - )) - }, - ) - .when(!context.is_empty(), |parent| { - parent.child( - h_flex() - .flex_wrap() - .gap_1() - .children(context.into_iter().map(|context| { - let context_id = context.id(); - ContextPill::added(AddedContext::new(context, cx), false, false, None) + let message_is_empty = message.should_display_content(); + let has_content = !message_is_empty || !context.is_empty(); + + let message_content = + has_content.then(|| { + v_flex() + .gap_1p5() + .when(!message_is_empty, |parent| { + parent.child( + if let Some(edit_message_editor) = edit_message_editor.clone() { + div() + .key_context("EditMessageEditor") + .on_action(cx.listener(Self::cancel_editing_message)) + .on_action(cx.listener(Self::confirm_editing_message)) + .min_h_6() + .child(edit_message_editor) + .into_any() + } else { + div() + .min_h_6() + .text_ui(cx) + .child(self.render_message_content( + message_id, + rendered_message, + has_tool_uses, + cx, + )) + .into_any() + }, + ) + }) + .when(!context.is_empty(), |parent| { + parent.child(h_flex().flex_wrap().gap_1().children( + context.into_iter().map(|context| { + let context_id = context.id(); + ContextPill::added( + AddedContext::new(context, cx), + false, + false, + None, + ) .on_click(Rc::new(cx.listener({ let workspace = workspace.clone(); let context_store = context_store.clone(); @@ -1145,8 +1156,9 @@ impl ActiveThread { } } }))) - })), - ) + }), + )) + }) }); let styled_message = match message.role { @@ -1261,7 +1273,7 @@ impl ActiveThread { ), ), ) - .child(div().p_2().child(message_content)), + .child(div().p_2().children(message_content)), ), Role::Assistant => v_flex() .id(("message-container", ix)) @@ -1270,7 +1282,9 @@ impl ActiveThread { .pr_4() .border_l_1() .border_color(cx.theme().colors().border_variant) - .child(message_content) + .children(message_content) + .gap_2p5() + .pb_2p5() .when(!tool_uses.is_empty(), |parent| { parent.child( v_flex().children( @@ -1284,7 +1298,7 @@ impl ActiveThread { v_flex() .bg(colors.editor_background) .rounded_sm() - .child(div().p_4().child(message_content)), + .child(div().p_4().children(message_content)), ), }; @@ -1816,7 +1830,7 @@ impl ActiveThread { div().map(|element| { if !tool_use.needs_confirmation { - element.py_2p5().child( + element.child( v_flex() .child( h_flex() @@ -1888,145 +1902,164 @@ impl ActiveThread { }), ) } else { - element.py_2().child( - v_flex() - .rounded_lg() - .border_1() - .border_color(self.tool_card_border_color(cx)) - .overflow_hidden() - .child( - h_flex() - .group("disclosure-header") - .relative() - .justify_between() - .py_1() - .map(|element| { - if is_status_finished { - element.pl_2().pr_0p5() - } else { - element.px_2() - } - }) - .bg(self.tool_card_header_bg(cx)) - .map(|element| { - if is_open { - element.border_b_1().rounded_t_md() - } else if needs_confirmation { - element.rounded_t_md() - } else { - element.rounded_md() - } - }) - .border_color(self.tool_card_border_color(cx)) - .child( - h_flex() - .id("tool-label-container") - .gap_1p5() - .max_w_full() - .overflow_x_scroll() - .child( - Icon::new(tool_use.icon) - .size(IconSize::XSmall) - .color(Color::Muted), - ) - .child( - h_flex().pr_8().text_ui_sm(cx).children( - self.rendered_tool_use_labels - .get(&tool_use.id) - .cloned(), - ), - ), - ) - .child( - h_flex() - .gap_1() - .child( - div().visible_on_hover("disclosure-header").child( - Disclosure::new("tool-use-disclosure", is_open) - .opened_icon(IconName::ChevronUp) - .closed_icon(IconName::ChevronDown) - .on_click(cx.listener({ - let tool_use_id = tool_use.id.clone(); - move |this, _event, _window, _cx| { - let is_open = this - .expanded_tool_uses - .entry(tool_use_id.clone()) - .or_insert(false); - - *is_open = !*is_open; - } - })), - ), - ) - .child(status_icons), - ) - .child(gradient_overlay(self.tool_card_header_bg(cx))), - ) - .map(|parent| { - if !is_open { - return parent; - } - - parent.child( - v_flex() - .bg(cx.theme().colors().editor_background) - .map(|element| { - if needs_confirmation { - element.rounded_none() - } else { - element.rounded_b_lg() - } - }) - .child(results_content), - ) - }) - .when(needs_confirmation, |this| { - this.child( + v_flex() + .rounded_lg() + .border_1() + .border_color(self.tool_card_border_color(cx)) + .overflow_hidden() + .child( + h_flex() + .group("disclosure-header") + .relative() + .justify_between() + .py_1() + .map(|element| { + if is_status_finished { + element.pl_2().pr_0p5() + } else { + element.px_2() + } + }) + .bg(self.tool_card_header_bg(cx)) + .map(|element| { + if is_open { + element.border_b_1().rounded_t_md() + } else if needs_confirmation { + element.rounded_t_md() + } else { + element.rounded_md() + } + }) + .border_color(self.tool_card_border_color(cx)) + .child( h_flex() - .py_1() - .pl_2() - .pr_1() - .gap_1() - .justify_between() - .bg(cx.theme().colors().editor_background) - .border_t_1() - .border_color(self.tool_card_border_color(cx)) - .rounded_b_lg() - .child(Label::new("Action Confirmation").color(Color::Muted).size(LabelSize::Small)) + .id("tool-label-container") + .gap_1p5() + .max_w_full() + .overflow_x_scroll() .child( - h_flex() - .gap_0p5() - .child({ - let tool_id = tool_use.id.clone(); - Button::new( - "always-allow-tool-action", - "Always Allow", + Icon::new(tool_use.icon) + .size(IconSize::XSmall) + .color(Color::Muted), + ) + .child( + h_flex().pr_8().text_ui_sm(cx).children( + self.rendered_tool_use_labels + .get(&tool_use.id) + .cloned(), + ), + ), + ) + .child( + h_flex() + .gap_1() + .child( + div().visible_on_hover("disclosure-header").child( + Disclosure::new("tool-use-disclosure", is_open) + .opened_icon(IconName::ChevronUp) + .closed_icon(IconName::ChevronDown) + .on_click(cx.listener({ + let tool_use_id = tool_use.id.clone(); + move |this, _event, _window, _cx| { + let is_open = this + .expanded_tool_uses + .entry(tool_use_id.clone()) + .or_insert(false); + + *is_open = !*is_open; + } + })), + ), + ) + .child(status_icons), + ) + .child(gradient_overlay(self.tool_card_header_bg(cx))), + ) + .map(|parent| { + if !is_open { + return parent; + } + + parent.child( + v_flex() + .bg(cx.theme().colors().editor_background) + .map(|element| { + if needs_confirmation { + element.rounded_none() + } else { + element.rounded_b_lg() + } + }) + .child(results_content), + ) + }) + .when(needs_confirmation, |this| { + this.child( + h_flex() + .py_1() + .pl_2() + .pr_1() + .gap_1() + .justify_between() + .bg(cx.theme().colors().editor_background) + .border_t_1() + .border_color(self.tool_card_border_color(cx)) + .rounded_b_lg() + .child(Label::new("Action Confirmation").color(Color::Muted).size(LabelSize::Small)) + .child( + h_flex() + .gap_0p5() + .child({ + let tool_id = tool_use.id.clone(); + Button::new( + "always-allow-tool-action", + "Always Allow", + ) + .label_size(LabelSize::Small) + .icon(IconName::CheckDouble) + .icon_position(IconPosition::Start) + .icon_size(IconSize::Small) + .icon_color(Color::Success) + .tooltip(move |window, cx| { + Tooltip::with_meta( + "Never ask for permission", + None, + "Restore the original behavior in your Agent Panel settings", + window, + cx, ) - .label_size(LabelSize::Small) - .icon(IconName::CheckDouble) - .icon_position(IconPosition::Start) - .icon_size(IconSize::Small) - .icon_color(Color::Success) - .tooltip(move |window, cx| { - Tooltip::with_meta( - "Never ask for permission", - None, - "Restore the original behavior in your Agent Panel settings", + }) + .on_click(cx.listener( + move |this, event, window, cx| { + if let Some(fs) = fs.clone() { + update_settings_file::( + fs.clone(), + cx, + |settings, _| { + settings.set_always_allow_tool_actions(true); + }, + ); + } + this.handle_allow_tool( + tool_id.clone(), + event, window, cx, ) - }) + }, + )) + }) + .child(ui::Divider::vertical()) + .child({ + let tool_id = tool_use.id.clone(); + Button::new("allow-tool-action", "Allow") + .label_size(LabelSize::Small) + .icon(IconName::Check) + .icon_position(IconPosition::Start) + .icon_size(IconSize::Small) + .icon_color(Color::Success) .on_click(cx.listener( move |this, event, window, cx| { - if let Some(fs) = fs.clone() { - update_settings_file::( - fs.clone(), - cx, - |settings, _| { - settings.set_always_allow_tool_actions(true); - }, - ); - } this.handle_allow_tool( tool_id.clone(), event, @@ -2035,52 +2068,31 @@ impl ActiveThread { ) }, )) - }) - .child(ui::Divider::vertical()) - .child({ - let tool_id = tool_use.id.clone(); - Button::new("allow-tool-action", "Allow") - .label_size(LabelSize::Small) - .icon(IconName::Check) - .icon_position(IconPosition::Start) - .icon_size(IconSize::Small) - .icon_color(Color::Success) - .on_click(cx.listener( - move |this, event, window, cx| { - this.handle_allow_tool( - tool_id.clone(), - event, - window, - cx, - ) - }, - )) - }) - .child({ - let tool_id = tool_use.id.clone(); - let tool_name: Arc = tool_use.name.into(); - Button::new("deny-tool", "Deny") - .label_size(LabelSize::Small) - .icon(IconName::Close) - .icon_position(IconPosition::Start) - .icon_size(IconSize::Small) - .icon_color(Color::Error) - .on_click(cx.listener( - move |this, event, window, cx| { - this.handle_deny_tool( - tool_id.clone(), - tool_name.clone(), - event, - window, - cx, - ) - }, - )) - }), - ), - ) - }), - ) + }) + .child({ + let tool_id = tool_use.id.clone(); + let tool_name: Arc = tool_use.name.into(); + Button::new("deny-tool", "Deny") + .label_size(LabelSize::Small) + .icon(IconName::Close) + .icon_position(IconPosition::Start) + .icon_size(IconSize::Small) + .icon_color(Color::Error) + .on_click(cx.listener( + move |this, event, window, cx| { + this.handle_deny_tool( + tool_id.clone(), + tool_name.clone(), + event, + window, + cx, + ) + }, + )) + }), + ), + ) + }) } }) } diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index 9eecd01bcc..9619cdb492 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -35,7 +35,7 @@ use crate::thread_store::{ SerializedMessage, SerializedMessageSegment, SerializedThread, SerializedToolResult, SerializedToolUse, }; -use crate::tool_use::{PendingToolUse, ToolUse, ToolUseState}; +use crate::tool_use::{PendingToolUse, ToolUse, ToolUseState, USING_TOOL_MARKER}; #[derive(Debug, Clone, Copy)] pub enum RequestKind { @@ -85,6 +85,12 @@ pub struct Message { } impl Message { + /// Returns whether the message contains any meaningful text that should be displayed + /// The model sometimes runs tool without producing any text or just a marker ([`USING_TOOL_MARKER`]) + pub fn should_display_content(&self) -> bool { + self.segments.iter().all(|segment| segment.should_display()) + } + pub fn push_thinking(&mut self, text: &str) { if let Some(MessageSegment::Thinking(segment)) = self.segments.last_mut() { segment.push_str(text); @@ -131,6 +137,16 @@ impl MessageSegment { Self::Thinking(text) => text, } } + + pub fn should_display(&self) -> bool { + // We add USING_TOOL_MARKER when making a request that includes tool uses + // without non-whitespace text around them, and this can cause the model + // to mimic the pattern, so we consider those segments not displayable. + match self { + Self::Text(text) => text.is_empty() || text.trim() == USING_TOOL_MARKER, + Self::Thinking(text) => text.is_empty() || text.trim() == USING_TOOL_MARKER, + } + } } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -1083,32 +1099,15 @@ impl Thread { } } LanguageModelCompletionEvent::ToolUse(tool_use) => { - let last_assistant_message = thread + let last_assistant_message_id = thread .messages .iter_mut() - .rfind(|message| message.role == Role::Assistant); + .rfind(|message| message.role == Role::Assistant) + .map(|message| message.id) + .unwrap_or_else(|| { + thread.insert_message(Role::Assistant, vec![], cx) + }); - let last_assistant_message_id = - if let Some(message) = last_assistant_message { - if let Some(segment) = message.segments.first_mut() { - let text = segment.text_mut(); - if text.is_empty() { - text.push_str("Using tool..."); - } - } else { - message.segments.push(MessageSegment::Text( - "Using tool...".to_string(), - )); - } - - message.id - } else { - thread.insert_message( - Role::Assistant, - vec![MessageSegment::Text("Using tool...".to_string())], - cx, - ) - }; thread.tool_use.request_tool_use( last_assistant_message_id, tool_use, diff --git a/crates/agent/src/tool_use.rs b/crates/agent/src/tool_use.rs index 602a8ab5ae..03cabb0870 100644 --- a/crates/agent/src/tool_use.rs +++ b/crates/agent/src/tool_use.rs @@ -43,6 +43,8 @@ pub struct ToolUseState { pending_tool_uses_by_id: HashMap, } +pub const USING_TOOL_MARKER: &str = ""; + impl ToolUseState { pub fn new(tools: Arc) -> Self { Self { @@ -357,8 +359,28 @@ impl ToolUseState { request_message: &mut LanguageModelRequestMessage, ) { if let Some(tool_uses) = self.tool_uses_by_assistant_message.get(&message_id) { + let mut found_tool_use = false; + for tool_use in tool_uses { if self.tool_results.contains_key(&tool_use.id) { + if !found_tool_use { + // The API fails if a message contains a tool use without any (non-whitespace) text around it + match request_message.content.last_mut() { + Some(MessageContent::Text(txt)) => { + if txt.is_empty() { + txt.push_str(USING_TOOL_MARKER); + } + } + None | Some(_) => { + request_message + .content + .push(MessageContent::Text(USING_TOOL_MARKER.into())); + } + }; + } + + found_tool_use = true; + // Do not send tool uses until they are completed request_message .content