From b14356d1d3980aa4dc5c0d01e163b9581bbad967 Mon Sep 17 00:00:00 2001 From: Agus Zubiaga Date: Tue, 22 Apr 2025 00:41:47 -0300 Subject: [PATCH] agent: Do not add `` placeholder (#29194) Our provider code in `language_models` filters out messages for which `LanguageModelRequestMessage::contents_empty` returns `false`. This doesn't seem wrong by itself, but `contents_empty` was returning `false` for messages whose first segment didn't contain non-whitespace text even if they contained other non-empty segments. This caused requests to fail when a message with a tool call didn't contain any preceding text. Release Notes: - N/A --- crates/agent/src/thread.rs | 9 +++------ crates/agent/src/tool_use.rs | 22 ---------------------- crates/language_model/src/request.rs | 27 ++++++++++----------------- 3 files changed, 13 insertions(+), 45 deletions(-) diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index 81b6b3c980a0bf33cf6c879aa4e41f2b55669a22..99dab5f93988de04a5d51755dd11b929ef4251d2 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -38,7 +38,7 @@ use crate::thread_store::{ SerializedMessage, SerializedMessageSegment, SerializedThread, SerializedToolResult, SerializedToolUse, SharedProjectContext, }; -use crate::tool_use::{PendingToolUse, ToolUse, ToolUseMetadata, ToolUseState, USING_TOOL_MARKER}; +use crate::tool_use::{PendingToolUse, ToolUse, ToolUseMetadata, ToolUseState}; #[derive( Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Serialize, Deserialize, JsonSchema, @@ -167,12 +167,9 @@ pub enum MessageSegment { impl MessageSegment { 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, + Self::Text(text) => text.is_empty(), + Self::Thinking { text, .. } => text.is_empty(), Self::RedactedThinking(_) => false, } } diff --git a/crates/agent/src/tool_use.rs b/crates/agent/src/tool_use.rs index 4fb49a2d16f2087d61011e4c2962f0c46816b674..33472773ee0af27e231c9ff52a8f7f7a89ef9db1 100644 --- a/crates/agent/src/tool_use.rs +++ b/crates/agent/src/tool_use.rs @@ -27,8 +27,6 @@ pub struct ToolUse { pub needs_confirmation: bool, } -pub const USING_TOOL_MARKER: &str = ""; - pub struct ToolUseState { tools: Entity, tool_uses_by_assistant_message: HashMap>, @@ -452,28 +450,8 @@ 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 diff --git a/crates/language_model/src/request.rs b/crates/language_model/src/request.rs index 0f1e97af5a776b337089ab56c54611e5aebe525a..44fb82b88a51f1951bb5ea15bc57900a58119fae 100644 --- a/crates/language_model/src/request.rs +++ b/crates/language_model/src/request.rs @@ -221,23 +221,16 @@ impl LanguageModelRequestMessage { } pub fn contents_empty(&self) -> bool { - self.content.is_empty() - || self - .content - .first() - .map(|content| match content { - MessageContent::Text(text) => text.chars().all(|c| c.is_whitespace()), - MessageContent::Thinking { text, .. } => { - text.chars().all(|c| c.is_whitespace()) - } - MessageContent::ToolResult(tool_result) => { - tool_result.content.chars().all(|c| c.is_whitespace()) - } - MessageContent::RedactedThinking(_) - | MessageContent::ToolUse(_) - | MessageContent::Image(_) => true, - }) - .unwrap_or(false) + self.content.iter().all(|content| match content { + MessageContent::Text(text) => text.chars().all(|c| c.is_whitespace()), + MessageContent::Thinking { text, .. } => text.chars().all(|c| c.is_whitespace()), + MessageContent::ToolResult(tool_result) => { + tool_result.content.chars().all(|c| c.is_whitespace()) + } + MessageContent::RedactedThinking(_) + | MessageContent::ToolUse(_) + | MessageContent::Image(_) => false, + }) } }