Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

agent: Handle tool use without text (#28030)

Browse source 
### Context 

The Anthropic API fails if a request message contains a tool use and no
`Text` segments or it only contains empty `Text` segments. These are
cases that the model itself produces, but the API doesn't support
sending them back.

#27917 fixed this by appending "Using tool..." in the thread's message,
but this causes the actual conversation to include it, so it would
appear in the UI (we would actually display a gap because we never
rendered its markdown, but "Using tool..." would show up when the thread
was restored).

### Solution

We'll now only append this placeholder when we build the request, so the
API still sees it, but the UI/Thread doesn't.

Another issue we found is that the model starts mimicking these
placeholders in later tool uses which is undesirable. So unfortunately,
we had to add logic to filter them out.

Release Notes:

- agent: Improved rendering of tool uses without text

---------

Co-authored-by: Bennet <bennet@zed.dev>
This commit is contained in:
Agus Zubiaga 2025-04-03 14:22:59 -03:00 • committed by GitHub
parent ece4a1cd7c
commit ed3722023e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 270 additions and 237 deletions
Download patch file Download diff file Expand all files Collapse all files

438
crates/agent/src/active_thread.rs
View file

@ -1099,36 +1099,47 @@ impl ActiveThread {
.into_any_element(), .into_any_element(),
}; };
let message_content = v_flex() let message_is_empty = message.should_display_content();
.gap_1p5() let has_content = !message_is_empty || !context.is_empty();
.child(
if let Some(edit_message_editor) = edit_message_editor.clone() { let message_content =
div() has_content.then(|| {
.key_context("EditMessageEditor") v_flex()
.on_action(cx.listener(Self::cancel_editing_message)) .gap_1p5()
.on_action(cx.listener(Self::confirm_editing_message)) .when(!message_is_empty, |parent| {
.min_h_6() parent.child(
.child(edit_message_editor) if let Some(edit_message_editor) = edit_message_editor.clone() {
} else { div()
div() .key_context("EditMessageEditor")
.min_h_6() .on_action(cx.listener(Self::cancel_editing_message))
.text_ui(cx) .on_action(cx.listener(Self::confirm_editing_message))
.child(self.render_message_content( .min_h_6()
message_id, .child(edit_message_editor)
rendered_message, .into_any()
has_tool_uses, } else {
cx, div()
)) .min_h_6()
}, .text_ui(cx)
) .child(self.render_message_content(
.when(!context.is_empty(), |parent| { message_id,
parent.child( rendered_message,
h_flex() has_tool_uses,
.flex_wrap() cx,
.gap_1() ))
.children(context.into_iter().map(|context| { .into_any()
let context_id = context.id(); },
ContextPill::added(AddedContext::new(context, cx), false, false, None) )
})
.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({ .on_click(Rc::new(cx.listener({
let workspace = workspace.clone(); let workspace = workspace.clone();
let context_store = context_store.clone(); let context_store = context_store.clone();
@ -1145,8 +1156,9 @@ impl ActiveThread {
} }
} }
}))) })))
})), }),
) ))
})
}); });
let styled_message = match message.role { 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() Role::Assistant => v_flex()
.id(("message-container", ix)) .id(("message-container", ix))
@ -1270,7 +1282,9 @@ impl ActiveThread {
.pr_4() .pr_4()
.border_l_1() .border_l_1()
.border_color(cx.theme().colors().border_variant) .border_color(cx.theme().colors().border_variant)
.child(message_content) .children(message_content)
.gap_2p5()
.pb_2p5()
.when(!tool_uses.is_empty(), |parent| { .when(!tool_uses.is_empty(), |parent| {
parent.child( parent.child(
v_flex().children( v_flex().children(
@ -1284,7 +1298,7 @@ impl ActiveThread {
v_flex() v_flex()
.bg(colors.editor_background) .bg(colors.editor_background)
.rounded_sm() .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| { div().map(|element| {
if !tool_use.needs_confirmation { if !tool_use.needs_confirmation {
element.py_2p5().child( element.child(
v_flex() v_flex()
.child( .child(
h_flex() h_flex()
@ -1888,145 +1902,164 @@ impl ActiveThread {
}), }),
) )
} else { } else {
element.py_2().child( v_flex()
v_flex() .rounded_lg()
.rounded_lg() .border_1()
.border_1() .border_color(self.tool_card_border_color(cx))
.border_color(self.tool_card_border_color(cx)) .overflow_hidden()
.overflow_hidden() .child(
.child( h_flex()
h_flex() .group("disclosure-header")
.group("disclosure-header") .relative()
.relative() .justify_between()
.justify_between() .py_1()
.py_1() .map(|element| {
.map(|element| { if is_status_finished {
if is_status_finished { element.pl_2().pr_0p5()
element.pl_2().pr_0p5() } else {
} else { element.px_2()
element.px_2() }
} })
}) .bg(self.tool_card_header_bg(cx))
.bg(self.tool_card_header_bg(cx)) .map(|element| {
.map(|element| { if is_open {
if is_open { element.border_b_1().rounded_t_md()
element.border_b_1().rounded_t_md() } else if needs_confirmation {
} else if needs_confirmation { element.rounded_t_md()
element.rounded_t_md() } else {
} else { element.rounded_md()
element.rounded_md() }
} })
}) .border_color(self.tool_card_border_color(cx))
.border_color(self.tool_card_border_color(cx)) .child(
.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(
h_flex() h_flex()
.py_1() .id("tool-label-container")
.pl_2() .gap_1p5()
.pr_1() .max_w_full()
.gap_1() .overflow_x_scroll()
.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( .child(
h_flex() Icon::new(tool_use.icon)
.gap_0p5() .size(IconSize::XSmall)
.child({ .color(Color::Muted),
let tool_id = tool_use.id.clone(); )
Button::new( .child(
"always-allow-tool-action", h_flex().pr_8().text_ui_sm(cx).children(
"Always Allow", 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) .on_click(cx.listener(
.icon_position(IconPosition::Start) move |this, event, window, cx| {
.icon_size(IconSize::Small) if let Some(fs) = fs.clone() {
.icon_color(Color::Success) update_settings_file::<AssistantSettings>(
.tooltip(move |window, cx| { fs.clone(),
Tooltip::with_meta( cx,
"Never ask for permission", |settings, _| {
None, settings.set_always_allow_tool_actions(true);
"Restore the original behavior in your Agent Panel settings", },
);
}
this.handle_allow_tool(
tool_id.clone(),
event,
window, window,
cx, 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( .on_click(cx.listener(
move |this, event, window, cx| { move |this, event, window, cx| {
if let Some(fs) = fs.clone() {
update_settings_file::<AssistantSettings>(
fs.clone(),
cx,
|settings, _| {
settings.set_always_allow_tool_actions(true);
},
);
}
this.handle_allow_tool( this.handle_allow_tool(
tool_id.clone(), tool_id.clone(),
event, event,
@ -2035,52 +2068,31 @@ impl ActiveThread {
) )
}, },
)) ))
}) })
.child(ui::Divider::vertical()) .child({
.child({ let tool_id = tool_use.id.clone();
let tool_id = tool_use.id.clone(); let tool_name: Arc<str> = tool_use.name.into();
Button::new("allow-tool-action", "Allow") Button::new("deny-tool", "Deny")
.label_size(LabelSize::Small) .label_size(LabelSize::Small)
.icon(IconName::Check) .icon(IconName::Close)
.icon_position(IconPosition::Start) .icon_position(IconPosition::Start)
.icon_size(IconSize::Small) .icon_size(IconSize::Small)
.icon_color(Color::Success) .icon_color(Color::Error)
.on_click(cx.listener( .on_click(cx.listener(
move |this, event, window, cx| { move |this, event, window, cx| {
this.handle_allow_tool( this.handle_deny_tool(
tool_id.clone(), tool_id.clone(),
event, tool_name.clone(),
window, event,
cx, window,
) cx,
}, )
)) },
}) ))
.child({ }),
let tool_id = tool_use.id.clone(); ),
let tool_name: Arc<str> = 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,
)
},
))
}),
),
)
}),
)
} }
}) })
} }

47
crates/agent/src/thread.rs
View file

@ -35,7 +35,7 @@ use crate::thread_store::{
SerializedMessage, SerializedMessageSegment, SerializedThread, SerializedToolResult, SerializedMessage, SerializedMessageSegment, SerializedThread, SerializedToolResult,
SerializedToolUse, SerializedToolUse,
}; };
use crate::tool_use::{PendingToolUse, ToolUse, ToolUseState}; use crate::tool_use::{PendingToolUse, ToolUse, ToolUseState, USING_TOOL_MARKER};
#[derive(Debug, Clone, Copy)] #[derive(Debug, Clone, Copy)]
pub enum RequestKind { pub enum RequestKind {
@ -85,6 +85,12 @@ pub struct Message {
} }
impl 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) { pub fn push_thinking(&mut self, text: &str) {
if let Some(MessageSegment::Thinking(segment)) = self.segments.last_mut() { if let Some(MessageSegment::Thinking(segment)) = self.segments.last_mut() {
segment.push_str(text); segment.push_str(text);
@ -131,6 +137,16 @@ impl MessageSegment {
Self::Thinking(text) => text, 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)] #[derive(Debug, Clone, Serialize, Deserialize)]
@ -1083,32 +1099,15 @@ impl Thread {
} }
} }
LanguageModelCompletionEvent::ToolUse(tool_use) => { LanguageModelCompletionEvent::ToolUse(tool_use) => {
let last_assistant_message = thread let last_assistant_message_id = thread
.messages .messages
.iter_mut() .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( thread.tool_use.request_tool_use(
last_assistant_message_id, last_assistant_message_id,
tool_use, tool_use,

22
crates/agent/src/tool_use.rs
View file

@ -43,6 +43,8 @@ pub struct ToolUseState {
pending_tool_uses_by_id: HashMap<LanguageModelToolUseId, PendingToolUse>, pending_tool_uses_by_id: HashMap<LanguageModelToolUseId, PendingToolUse>,
} }
pub const USING_TOOL_MARKER: &str = "<using_tool>";
impl ToolUseState { impl ToolUseState {
pub fn new(tools: Arc<ToolWorkingSet>) -> Self { pub fn new(tools: Arc<ToolWorkingSet>) -> Self {
Self { Self {
@ -357,8 +359,28 @@ impl ToolUseState {
request_message: &mut LanguageModelRequestMessage, request_message: &mut LanguageModelRequestMessage,
) { ) {
if let Some(tool_uses) = self.tool_uses_by_assistant_message.get(&message_id) { 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 { for tool_use in tool_uses {
if self.tool_results.contains_key(&tool_use.id) { 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 // Do not send tool uses until they are completed
request_message request_message
.content .content