diff --git a/Cargo.lock b/Cargo.lock index 32b079f91d6e..e1c956487860 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4795,6 +4795,7 @@ dependencies = [ "rmcp", "serde", "serde_json", + "thiserror 1.0.69", "tracing", "unicode-normalization", "utoipa 4.2.3", diff --git a/crates/goose-types/Cargo.toml b/crates/goose-types/Cargo.toml index bf0d237390ef..d0b381234f83 100644 --- a/crates/goose-types/Cargo.toml +++ b/crates/goose-types/Cargo.toml @@ -18,6 +18,7 @@ regex = { workspace = true } rmcp = { workspace = true, features = ["server"] } serde = { workspace = true } serde_json = { workspace = true } +thiserror.workspace = true tracing.workspace = true unicode-normalization = { version = "0.1.25", default-features = false, features = ["std"] } utoipa = { workspace = true } diff --git a/crates/goose-types/src/lib.rs b/crates/goose-types/src/lib.rs index 1de63fdee0a2..a08cbb468847 100644 --- a/crates/goose-types/src/lib.rs +++ b/crates/goose-types/src/lib.rs @@ -16,6 +16,7 @@ use std::fmt; use std::ops::{Add, AddAssign}; use std::str::FromStr; use std::sync::OnceLock; +use thiserror::Error; use unicode_normalization::UnicodeNormalization; use utoipa::ToSchema; use uuid::Uuid; @@ -1061,6 +1062,576 @@ impl Message { } } +#[derive(Debug, Clone, Serialize, Deserialize, ToSchema, PartialEq)] +pub struct Conversation(Vec); + +#[derive(Error, Debug)] +#[error("invalid conversation: {reason}")] +pub struct InvalidConversation { + reason: String, + conversation: Conversation, +} + +impl Conversation { + pub fn new(messages: I) -> Result + where + I: IntoIterator, + { + Self::new_unvalidated(messages).validate() + } + + pub fn new_unvalidated(messages: I) -> Self + where + I: IntoIterator, + { + Self(messages.into_iter().collect()) + } + + pub fn empty() -> Self { + Self::new_unvalidated([]) + } + + pub fn messages(&self) -> &Vec { + &self.0 + } + + pub fn push(&mut self, message: Message) { + if message.content.is_empty() && message.metadata.inference.is_some() { + if let Some(existing) = self + .0 + .iter_mut() + .rev() + .find(|m| m.role == message.role && m.is_user_visible()) + { + existing.metadata.inference = message.metadata.inference; + } + return; + } + + if let Some(last) = self + .0 + .last_mut() + .filter(|m| m.id.is_some() && m.id == message.id) + { + if message.metadata.inference.is_some() { + last.metadata.inference = message.metadata.inference.clone(); + } + match (last.content.last_mut(), message.content.last()) { + (Some(MessageContent::Text(ref mut last)), Some(MessageContent::Text(new))) + if message.content.len() == 1 => + { + last.text.push_str(&new.text); + } + ( + Some(MessageContent::Thinking(ref mut last)), + Some(MessageContent::Thinking(new)), + ) if message.content.len() == 1 + && (last.signature.is_empty() || new.signature == last.signature) => + { + // Merge cases: + // - `last` is still unsigned (block in progress) — append + // and adopt `new.signature` if it's the closing delta. + // - signatures match — same block continuing. + // An unsigned delta arriving after a signed block belongs + // to the next block (signature-at-end streams emit the + // first text of block N+1 before its signature), so the + // outer match arm falls through to push it separately. + last.thinking.push_str(&new.thinking); + if !new.signature.is_empty() { + last.signature = new.signature.clone(); + } + } + (_, _) => { + last.content.extend(message.content); + } + } + } else { + self.0.push(message); + } + } + + pub fn last(&self) -> Option<&Message> { + self.0.last() + } + + pub fn first(&self) -> Option<&Message> { + self.0.first() + } + + pub fn len(&self) -> usize { + self.0.len() + } + + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + pub fn extend(&mut self, iter: I) + where + I: IntoIterator, + { + for message in iter { + self.push(message); + } + } + + pub fn iter(&self) -> std::slice::Iter<'_, Message> { + self.0.iter() + } + + pub fn pop(&mut self) -> Option { + self.0.pop() + } + + pub fn truncate(&mut self, len: usize) { + self.0.truncate(len); + } + + pub fn clear(&mut self) { + self.0.clear(); + } + + pub fn filtered_messages(&self, filter: F) -> Vec + where + F: Fn(&MessageMetadata) -> bool, + { + self.0 + .iter() + .filter(|msg| filter(&msg.metadata)) + .cloned() + .collect() + } + + pub fn agent_visible_messages(&self) -> Vec { + self.filtered_messages(|meta| meta.agent_visible) + } + + pub fn user_visible_messages(&self) -> Vec { + self.filtered_messages(|meta| meta.user_visible) + } + + fn validate(self) -> Result { + let (_messages, issues) = fix_messages(self.0.clone()); + if !issues.is_empty() { + let reason = issues.join("\n"); + Err(InvalidConversation { + reason, + conversation: self, + }) + } else { + Ok(self) + } + } +} + +impl Default for Conversation { + fn default() -> Self { + Self::empty() + } +} + +impl IntoIterator for Conversation { + type Item = Message; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} +impl<'a> IntoIterator for &'a Conversation { + type Item = &'a Message; + type IntoIter = std::slice::Iter<'a, Message>; + + fn into_iter(self) -> Self::IntoIter { + self.0.iter() + } +} + +/// Fix a conversation that we're about to send to an LLM. So the first and last +/// messages should always be from the user. +pub fn fix_conversation(conversation: Conversation) -> (Conversation, Vec) { + let all_messages = conversation.messages(); + + // Create a shadow map: track each message as either Visible or NonVisible with its index + enum MessageSlot { + Visible(usize), // Index into agent_visible_messages + NonVisible(Message), // Non-visible messages pass through unchanged + } + + let mut agent_visible_messages = Vec::new(); + let shadow_map: Vec = all_messages + .iter() + .map(|msg| { + if msg.metadata.agent_visible { + let idx = agent_visible_messages.len(); + agent_visible_messages.push(msg.clone()); + MessageSlot::Visible(idx) + } else { + MessageSlot::NonVisible(msg.clone()) + } + }) + .collect(); + + // Fix only the agent-visible messages + let (fixed_visible, issues) = fix_messages(agent_visible_messages); + + // Reconstruct using shadow map: replace Visible slots with fixed messages + let final_messages: Vec = shadow_map + .into_iter() + .filter_map(|slot| match slot { + MessageSlot::Visible(idx) => fixed_visible.get(idx).cloned(), + MessageSlot::NonVisible(msg) => Some(msg), + }) + .collect(); + + (Conversation::new_unvalidated(final_messages), issues) +} + +fn fix_messages(messages: Vec) -> (Vec, Vec) { + [ + merge_text_content_items, + trim_assistant_text_whitespace, + remove_empty_messages, + fix_empty_tool_results, + fix_tool_calling, + merge_consecutive_messages, + fix_lead_trail, + populate_if_empty, + ] + .into_iter() + .fold( + (messages, Vec::new()), + |(msgs, mut all_issues), processor| { + let (new_msgs, issues) = processor(msgs); + all_issues.extend(issues); + (new_msgs, all_issues) + }, + ) +} + +fn merge_text_content_in_message(mut msg: Message) -> Message { + if msg.role != Role::Assistant { + return msg; + } + msg.content = msg + .content + .into_iter() + .fold(Vec::new(), |mut content, item| { + match item { + MessageContent::Text(text) => { + if let Some(MessageContent::Text(ref mut last)) = content.last_mut() { + last.text.push_str(&text.text); + } else { + content.push(MessageContent::Text(text)); + } + } + other => content.push(other), + } + content + }); + msg +} + +fn merge_text_content_items(messages: Vec) -> (Vec, Vec) { + messages.into_iter().fold( + (Vec::new(), Vec::new()), + |(mut messages, mut issues), message| { + let content_len = message.content.len(); + let message = merge_text_content_in_message(message); + if content_len != message.content.len() { + issues.push(String::from("Merged text content")) + } + messages.push(message); + (messages, issues) + }, + ) +} + +fn trim_assistant_text_whitespace(messages: Vec) -> (Vec, Vec) { + let mut issues = Vec::new(); + + let fixed_messages = messages + .into_iter() + .map(|mut message| { + if message.role == Role::Assistant { + for content in &mut message.content { + if let MessageContent::Text(text) = content { + let trimmed = text.text.trim_end(); + if trimmed.len() != text.text.len() { + issues.push( + "Trimmed trailing whitespace from assistant message".to_string(), + ); + text.text = trimmed.to_string(); + } + } + } + } + message + }) + .collect(); + + (fixed_messages, issues) +} + +fn remove_empty_messages(messages: Vec) -> (Vec, Vec) { + let mut issues = Vec::new(); + let filtered_messages = messages + .into_iter() + .filter(|msg| { + if msg + .content + .iter() + .all(|c| c.as_text().is_some_and(str::is_empty)) + { + issues.push("Removed empty message".to_string()); + false + } else { + true + } + }) + .collect(); + (filtered_messages, issues) +} + +/// Checks whether tool result content has any meaningful payload. +/// Text and resources must contain non-empty strings; images are always meaningful. +fn has_tool_result_content(content: &[Content]) -> bool { + content.iter().any(|c| { + if let Some(t) = c.as_text() { + return !t.text.is_empty(); + } + if let Some(r) = c.as_resource() { + return !extract_text_from_resource(&r.resource).is_empty(); + } + c.as_image().is_some() + }) +} + +/// Fix tool results that would be empty when formatted for LLM APIs. +/// Some APIs (like Anthropic) reject tool_result blocks with empty content. +/// This adds a placeholder message for tool results that have no extractable text. +fn fix_empty_tool_results(messages: Vec) -> (Vec, Vec) { + let mut issues = Vec::new(); + + let fixed_messages = messages + .into_iter() + .map(|mut message| { + for content in &mut message.content { + if let MessageContent::ToolResponse(ref mut tool_response) = content { + if let Ok(ref mut result) = tool_response.tool_result { + if !has_tool_result_content(&result.content) { + // Add a placeholder text content so the tool result isn't empty + result.content.push(Content::text("(empty result)")); + issues.push(format!( + "Added placeholder to empty tool result '{}'", + tool_response.id + )); + } + } + } + } + message + }) + .collect(); + + (fixed_messages, issues) +} + +fn fix_tool_calling(mut messages: Vec) -> (Vec, Vec) { + let mut issues = Vec::new(); + let mut pending_tool_requests: HashSet = HashSet::new(); + + for message in &mut messages { + let mut content_to_remove = Vec::new(); + + match message.role { + Role::User => { + for (idx, content) in message.content.iter().enumerate() { + match content { + MessageContent::ToolRequest(req) => { + content_to_remove.push(idx); + issues.push(format!( + "Removed tool request '{}' from user message", + req.id + )); + } + MessageContent::ToolConfirmationRequest(req) => { + content_to_remove.push(idx); + issues.push(format!( + "Removed tool confirmation request '{}' from user message", + req.id + )); + } + MessageContent::Thinking(_) | MessageContent::RedactedThinking(_) => { + content_to_remove.push(idx); + issues.push("Removed thinking content from user message".to_string()); + } + MessageContent::ToolResponse(resp) => { + if pending_tool_requests.contains(&resp.id) { + pending_tool_requests.remove(&resp.id); + } else { + content_to_remove.push(idx); + issues + .push(format!("Removed orphaned tool response '{}'", resp.id)); + } + } + _ => {} + } + } + } + Role::Assistant => { + for (idx, content) in message.content.iter().enumerate() { + match content { + MessageContent::ToolResponse(resp) => { + content_to_remove.push(idx); + issues.push(format!( + "Removed tool response '{}' from assistant message", + resp.id + )); + } + MessageContent::FrontendToolRequest(req) => { + content_to_remove.push(idx); + issues.push(format!( + "Removed frontend tool request '{}' from assistant message", + req.id + )); + } + MessageContent::ToolRequest(req) => { + pending_tool_requests.insert(req.id.clone()); + } + _ => {} + } + } + } + } + + for &idx in content_to_remove.iter().rev() { + message.content.remove(idx); + } + } + + for message in &mut messages { + if message.role == Role::Assistant { + let mut content_to_remove = Vec::new(); + for (idx, content) in message.content.iter().enumerate() { + if let MessageContent::ToolRequest(req) = content { + if pending_tool_requests.contains(&req.id) { + content_to_remove.push(idx); + issues.push(format!("Removed orphaned tool request '{}'", req.id)); + } + } + } + for &idx in content_to_remove.iter().rev() { + message.content.remove(idx); + } + } + } + let (messages, empty_removed) = remove_empty_messages(messages); + issues.extend(empty_removed); + (messages, issues) +} + +pub fn merge_consecutive_messages(messages: Vec) -> (Vec, Vec) { + let mut issues = Vec::new(); + let mut merged_messages: Vec = Vec::new(); + + for message in messages { + if let Some(last) = merged_messages.last_mut() { + let effective = effective_role(&message); + if effective_role(last) == effective { + last.content.extend(message.content); + issues.push(format!("Merged consecutive {} messages", effective)); + continue; + } + } + merged_messages.push(message); + } + + (merged_messages, issues) +} + +fn has_tool_response(message: &Message) -> bool { + message + .content + .iter() + .any(|content| matches!(content, MessageContent::ToolResponse(_))) +} + +pub fn effective_role(message: &Message) -> String { + if message.role == Role::User && has_tool_response(message) { + "tool".to_string() + } else { + match message.role { + Role::User => "user".to_string(), + Role::Assistant => "assistant".to_string(), + } + } +} + +fn fix_lead_trail(mut messages: Vec) -> (Vec, Vec) { + let mut issues = Vec::new(); + + if let Some(first) = messages.first() { + if first.role == Role::Assistant { + messages.remove(0); + issues.push("Removed leading assistant message".to_string()); + } + } + + if let Some(last) = messages.last() { + if last.role == Role::Assistant { + messages.pop(); + issues.push("Removed trailing assistant message".to_string()); + } + } + + (messages, issues) +} + +const PLACEHOLDER_USER_MESSAGE: &str = "Hello"; + +fn populate_if_empty(mut messages: Vec) -> (Vec, Vec) { + let mut issues = Vec::new(); + + if messages.is_empty() { + issues.push("Added placeholder user message to empty conversation".to_string()); + messages.push(Message::user().with_text(PLACEHOLDER_USER_MESSAGE)); + } + (messages, issues) +} + +pub fn debug_conversation_fix( + messages: &[Message], + fixed: &[Message], + issues: &[String], +) -> String { + let mut output = String::new(); + + output.push_str("=== CONVERSATION FIX DEBUG ===\n\n"); + + output.push_str("BEFORE:\n"); + for (i, msg) in messages.iter().enumerate() { + output.push_str(&format!(" [{}] {}\n", i, msg.debug())); + } + + output.push_str("\nISSUES FOUND:\n"); + if issues.is_empty() { + output.push_str(" (none)\n"); + } else { + for issue in issues { + output.push_str(&format!(" - {}\n", issue)); + } + } + + output.push_str("\nAFTER:\n"); + for (i, msg) in fixed.iter().enumerate() { + output.push_str(&format!(" [{}] {}\n", i, msg.debug())); + } + + output.push_str("\n==============================\n"); + output +} + impl MessageMetadata { pub fn agent_only() -> Self { Self { @@ -1848,6 +2419,73 @@ mod tests { assert!(id.starts_with("msg_")); } + #[test] + fn conversation_push_merges_same_message_text_and_thinking() { + let mut conversation = Conversation::empty(); + conversation.push(Message::assistant().with_text("Hello").with_id("turn-1")); + conversation.push(Message::assistant().with_text(" world").with_id("turn-1")); + + assert_eq!(conversation.messages().len(), 1); + assert_eq!(conversation.messages()[0].as_concat_text(), "Hello world"); + + let mut conversation = Conversation::empty(); + conversation.push( + Message::assistant() + .with_thinking("I ", "") + .with_id("turn-2"), + ); + conversation.push( + Message::assistant() + .with_thinking("should think.", "sig") + .with_id("turn-2"), + ); + + assert_eq!(conversation.messages().len(), 1); + let thinking = conversation.messages()[0].content[0].as_thinking().unwrap(); + assert_eq!(thinking.thinking, "I should think."); + assert_eq!(thinking.signature, "sig"); + } + + #[test] + fn fix_conversation_removes_invalid_tool_flow() { + let messages = vec![ + Message::assistant() + .with_text("I'll search") + .with_tool_request("search_1", Ok(CallToolRequestParams::new("search"))), + Message::user().with_tool_response( + "wrong_id", + Ok(CallToolResult::success(vec![Content::text("result")])), + ), + ]; + + let (fixed, issues) = fix_conversation(Conversation::new_unvalidated(messages)); + + assert_eq!(fixed.len(), 1); + assert_eq!(fixed.messages()[0].role, Role::User); + assert_eq!(fixed.messages()[0].as_concat_text(), "Hello"); + assert!(issues + .iter() + .any(|issue| issue == "Removed orphaned tool request 'search_1'")); + assert!(issues + .iter() + .any(|issue| issue == "Removed orphaned tool response 'wrong_id'")); + } + + #[test] + fn conversation_filters_visible_messages() { + let conversation = Conversation::new_unvalidated([ + Message::user().with_text("visible"), + Message::user().with_text("agent only").agent_only(), + Message::user().with_text("user only").user_only(), + Message::user() + .with_text("invisible") + .with_metadata(MessageMetadata::invisible()), + ]); + + assert_eq!(conversation.agent_visible_messages().len(), 2); + assert_eq!(conversation.user_visible_messages().len(), 2); + } + #[test] fn message_content_builders_create_text_image_and_thinking() { let text = MessageContent::text("hello"); diff --git a/crates/goose/src/conversation/message.rs b/crates/goose/src/conversation/message.rs index c6425b520cfa..1235bb6b881c 100644 --- a/crates/goose/src/conversation/message.rs +++ b/crates/goose/src/conversation/message.rs @@ -10,10 +10,9 @@ pub use goose_types::{ #[cfg(test)] mod tests { use crate::conversation::message::{Message, MessageContent, MessageMetadata}; - use crate::conversation::*; use rmcp::model::{ AnnotateAble, CallToolRequestParams, PromptMessage, PromptMessageContent, - PromptMessageRole, RawEmbeddedResource, RawImageContent, ResourceContents, + PromptMessageRole, RawEmbeddedResource, RawImageContent, ResourceContents, Role, }; use rmcp::model::{ErrorCode, ErrorData}; use rmcp::object; diff --git a/crates/goose/src/conversation/mod.rs b/crates/goose/src/conversation/mod.rs index 76bb31b0195e..3f55bc784e9c 100644 --- a/crates/goose/src/conversation/mod.rs +++ b/crates/goose/src/conversation/mod.rs @@ -1,582 +1,9 @@ -use crate::conversation::message::{Message, MessageContent, MessageMetadata}; -use crate::mcp_utils::extract_text_from_resource; -use rmcp::model::{Content, Role}; -use serde::{Deserialize, Serialize}; -use std::collections::HashSet; -use thiserror::Error; -use utoipa::ToSchema; - pub mod message; -#[derive(Debug, Clone, Serialize, Deserialize, ToSchema, PartialEq)] -pub struct Conversation(Vec); - -#[derive(Error, Debug)] -#[error("invalid conversation: {reason}")] -pub struct InvalidConversation { - reason: String, - conversation: Conversation, -} - -impl Conversation { - pub fn new(messages: I) -> Result - where - I: IntoIterator, - { - Self::new_unvalidated(messages).validate() - } - - pub fn new_unvalidated(messages: I) -> Self - where - I: IntoIterator, - { - Self(messages.into_iter().collect()) - } - - pub fn empty() -> Self { - Self::new_unvalidated([]) - } - - pub fn messages(&self) -> &Vec { - &self.0 - } - - pub fn push(&mut self, message: Message) { - if message.content.is_empty() && message.metadata.inference.is_some() { - if let Some(existing) = self - .0 - .iter_mut() - .rev() - .find(|m| m.role == message.role && m.is_user_visible()) - { - existing.metadata.inference = message.metadata.inference; - } - return; - } - - if let Some(last) = self - .0 - .last_mut() - .filter(|m| m.id.is_some() && m.id == message.id) - { - if message.metadata.inference.is_some() { - last.metadata.inference = message.metadata.inference.clone(); - } - match (last.content.last_mut(), message.content.last()) { - (Some(MessageContent::Text(ref mut last)), Some(MessageContent::Text(new))) - if message.content.len() == 1 => - { - last.text.push_str(&new.text); - } - ( - Some(MessageContent::Thinking(ref mut last)), - Some(MessageContent::Thinking(new)), - ) if message.content.len() == 1 - && (last.signature.is_empty() || new.signature == last.signature) => - { - // Merge cases: - // - `last` is still unsigned (block in progress) — append - // and adopt `new.signature` if it's the closing delta. - // - signatures match — same block continuing. - // An unsigned delta arriving after a signed block belongs - // to the next block (signature-at-end streams emit the - // first text of block N+1 before its signature), so the - // outer match arm falls through to push it separately. - last.thinking.push_str(&new.thinking); - if !new.signature.is_empty() { - last.signature = new.signature.clone(); - } - } - (_, _) => { - last.content.extend(message.content); - } - } - } else { - self.0.push(message); - } - } - - pub fn last(&self) -> Option<&Message> { - self.0.last() - } - - pub fn first(&self) -> Option<&Message> { - self.0.first() - } - - pub fn len(&self) -> usize { - self.0.len() - } - - pub fn is_empty(&self) -> bool { - self.0.is_empty() - } - - pub fn extend(&mut self, iter: I) - where - I: IntoIterator, - { - for message in iter { - self.push(message); - } - } - - pub fn iter(&self) -> std::slice::Iter<'_, Message> { - self.0.iter() - } - - pub fn pop(&mut self) -> Option { - self.0.pop() - } - - pub fn truncate(&mut self, len: usize) { - self.0.truncate(len); - } - - pub fn clear(&mut self) { - self.0.clear(); - } - - pub fn filtered_messages(&self, filter: F) -> Vec - where - F: Fn(&MessageMetadata) -> bool, - { - self.0 - .iter() - .filter(|msg| filter(&msg.metadata)) - .cloned() - .collect() - } - - pub fn agent_visible_messages(&self) -> Vec { - self.filtered_messages(|meta| meta.agent_visible) - } - - pub fn user_visible_messages(&self) -> Vec { - self.filtered_messages(|meta| meta.user_visible) - } - - fn validate(self) -> Result { - let (_messages, issues) = fix_messages(self.0.clone()); - if !issues.is_empty() { - let reason = issues.join("\n"); - Err(InvalidConversation { - reason, - conversation: self, - }) - } else { - Ok(self) - } - } -} - -impl Default for Conversation { - fn default() -> Self { - Self::empty() - } -} - -impl IntoIterator for Conversation { - type Item = Message; - type IntoIter = std::vec::IntoIter; - - fn into_iter(self) -> Self::IntoIter { - self.0.into_iter() - } -} -impl<'a> IntoIterator for &'a Conversation { - type Item = &'a Message; - type IntoIter = std::slice::Iter<'a, Message>; - - fn into_iter(self) -> Self::IntoIter { - self.0.iter() - } -} - -/// Fix a conversation that we're about to send to an LLM. So the first and last -/// messages should always be from the user. -pub fn fix_conversation(conversation: Conversation) -> (Conversation, Vec) { - let all_messages = conversation.messages(); - - // Create a shadow map: track each message as either Visible or NonVisible with its index - enum MessageSlot { - Visible(usize), // Index into agent_visible_messages - NonVisible(Message), // Non-visible messages pass through unchanged - } - - let mut agent_visible_messages = Vec::new(); - let shadow_map: Vec = all_messages - .iter() - .map(|msg| { - if msg.metadata.agent_visible { - let idx = agent_visible_messages.len(); - agent_visible_messages.push(msg.clone()); - MessageSlot::Visible(idx) - } else { - MessageSlot::NonVisible(msg.clone()) - } - }) - .collect(); - - // Fix only the agent-visible messages - let (fixed_visible, issues) = fix_messages(agent_visible_messages); - - // Reconstruct using shadow map: replace Visible slots with fixed messages - let final_messages: Vec = shadow_map - .into_iter() - .filter_map(|slot| match slot { - MessageSlot::Visible(idx) => fixed_visible.get(idx).cloned(), - MessageSlot::NonVisible(msg) => Some(msg), - }) - .collect(); - - (Conversation::new_unvalidated(final_messages), issues) -} - -fn fix_messages(messages: Vec) -> (Vec, Vec) { - [ - merge_text_content_items, - trim_assistant_text_whitespace, - remove_empty_messages, - fix_empty_tool_results, - fix_tool_calling, - merge_consecutive_messages, - fix_lead_trail, - populate_if_empty, - ] - .into_iter() - .fold( - (messages, Vec::new()), - |(msgs, mut all_issues), processor| { - let (new_msgs, issues) = processor(msgs); - all_issues.extend(issues); - (new_msgs, all_issues) - }, - ) -} - -fn merge_text_content_in_message(mut msg: Message) -> Message { - if msg.role != Role::Assistant { - return msg; - } - msg.content = msg - .content - .into_iter() - .fold(Vec::new(), |mut content, item| { - match item { - MessageContent::Text(text) => { - if let Some(MessageContent::Text(ref mut last)) = content.last_mut() { - last.text.push_str(&text.text); - } else { - content.push(MessageContent::Text(text)); - } - } - other => content.push(other), - } - content - }); - msg -} - -fn merge_text_content_items(messages: Vec) -> (Vec, Vec) { - messages.into_iter().fold( - (Vec::new(), Vec::new()), - |(mut messages, mut issues), message| { - let content_len = message.content.len(); - let message = merge_text_content_in_message(message); - if content_len != message.content.len() { - issues.push(String::from("Merged text content")) - } - messages.push(message); - (messages, issues) - }, - ) -} - -fn trim_assistant_text_whitespace(messages: Vec) -> (Vec, Vec) { - let mut issues = Vec::new(); - - let fixed_messages = messages - .into_iter() - .map(|mut message| { - if message.role == Role::Assistant { - for content in &mut message.content { - if let MessageContent::Text(text) = content { - let trimmed = text.text.trim_end(); - if trimmed.len() != text.text.len() { - issues.push( - "Trimmed trailing whitespace from assistant message".to_string(), - ); - text.text = trimmed.to_string(); - } - } - } - } - message - }) - .collect(); - - (fixed_messages, issues) -} - -fn remove_empty_messages(messages: Vec) -> (Vec, Vec) { - let mut issues = Vec::new(); - let filtered_messages = messages - .into_iter() - .filter(|msg| { - if msg - .content - .iter() - .all(|c| c.as_text().is_some_and(str::is_empty)) - { - issues.push("Removed empty message".to_string()); - false - } else { - true - } - }) - .collect(); - (filtered_messages, issues) -} - -/// Checks whether tool result content has any meaningful payload. -/// Text and resources must contain non-empty strings; images are always meaningful. -fn has_tool_result_content(content: &[Content]) -> bool { - content.iter().any(|c| { - if let Some(t) = c.as_text() { - return !t.text.is_empty(); - } - if let Some(r) = c.as_resource() { - return !extract_text_from_resource(&r.resource).is_empty(); - } - c.as_image().is_some() - }) -} - -/// Fix tool results that would be empty when formatted for LLM APIs. -/// Some APIs (like Anthropic) reject tool_result blocks with empty content. -/// This adds a placeholder message for tool results that have no extractable text. -fn fix_empty_tool_results(messages: Vec) -> (Vec, Vec) { - let mut issues = Vec::new(); - - let fixed_messages = messages - .into_iter() - .map(|mut message| { - for content in &mut message.content { - if let MessageContent::ToolResponse(ref mut tool_response) = content { - if let Ok(ref mut result) = tool_response.tool_result { - if !has_tool_result_content(&result.content) { - // Add a placeholder text content so the tool result isn't empty - result.content.push(Content::text("(empty result)")); - issues.push(format!( - "Added placeholder to empty tool result '{}'", - tool_response.id - )); - } - } - } - } - message - }) - .collect(); - - (fixed_messages, issues) -} - -fn fix_tool_calling(mut messages: Vec) -> (Vec, Vec) { - let mut issues = Vec::new(); - let mut pending_tool_requests: HashSet = HashSet::new(); - - for message in &mut messages { - let mut content_to_remove = Vec::new(); - - match message.role { - Role::User => { - for (idx, content) in message.content.iter().enumerate() { - match content { - MessageContent::ToolRequest(req) => { - content_to_remove.push(idx); - issues.push(format!( - "Removed tool request '{}' from user message", - req.id - )); - } - MessageContent::ToolConfirmationRequest(req) => { - content_to_remove.push(idx); - issues.push(format!( - "Removed tool confirmation request '{}' from user message", - req.id - )); - } - MessageContent::Thinking(_) | MessageContent::RedactedThinking(_) => { - content_to_remove.push(idx); - issues.push("Removed thinking content from user message".to_string()); - } - MessageContent::ToolResponse(resp) => { - if pending_tool_requests.contains(&resp.id) { - pending_tool_requests.remove(&resp.id); - } else { - content_to_remove.push(idx); - issues - .push(format!("Removed orphaned tool response '{}'", resp.id)); - } - } - _ => {} - } - } - } - Role::Assistant => { - for (idx, content) in message.content.iter().enumerate() { - match content { - MessageContent::ToolResponse(resp) => { - content_to_remove.push(idx); - issues.push(format!( - "Removed tool response '{}' from assistant message", - resp.id - )); - } - MessageContent::FrontendToolRequest(req) => { - content_to_remove.push(idx); - issues.push(format!( - "Removed frontend tool request '{}' from assistant message", - req.id - )); - } - MessageContent::ToolRequest(req) => { - pending_tool_requests.insert(req.id.clone()); - } - _ => {} - } - } - } - } - - for &idx in content_to_remove.iter().rev() { - message.content.remove(idx); - } - } - - for message in &mut messages { - if message.role == Role::Assistant { - let mut content_to_remove = Vec::new(); - for (idx, content) in message.content.iter().enumerate() { - if let MessageContent::ToolRequest(req) = content { - if pending_tool_requests.contains(&req.id) { - content_to_remove.push(idx); - issues.push(format!("Removed orphaned tool request '{}'", req.id)); - } - } - } - for &idx in content_to_remove.iter().rev() { - message.content.remove(idx); - } - } - } - let (messages, empty_removed) = remove_empty_messages(messages); - issues.extend(empty_removed); - (messages, issues) -} - -pub fn merge_consecutive_messages(messages: Vec) -> (Vec, Vec) { - let mut issues = Vec::new(); - let mut merged_messages: Vec = Vec::new(); - - for message in messages { - if let Some(last) = merged_messages.last_mut() { - let effective = effective_role(&message); - if effective_role(last) == effective { - last.content.extend(message.content); - issues.push(format!("Merged consecutive {} messages", effective)); - continue; - } - } - merged_messages.push(message); - } - - (merged_messages, issues) -} - -fn has_tool_response(message: &Message) -> bool { - message - .content - .iter() - .any(|content| matches!(content, MessageContent::ToolResponse(_))) -} - -pub fn effective_role(message: &Message) -> String { - if message.role == Role::User && has_tool_response(message) { - "tool".to_string() - } else { - match message.role { - Role::User => "user".to_string(), - Role::Assistant => "assistant".to_string(), - } - } -} - -fn fix_lead_trail(mut messages: Vec) -> (Vec, Vec) { - let mut issues = Vec::new(); - - if let Some(first) = messages.first() { - if first.role == Role::Assistant { - messages.remove(0); - issues.push("Removed leading assistant message".to_string()); - } - } - - if let Some(last) = messages.last() { - if last.role == Role::Assistant { - messages.pop(); - issues.push("Removed trailing assistant message".to_string()); - } - } - - (messages, issues) -} - -const PLACEHOLDER_USER_MESSAGE: &str = "Hello"; - -fn populate_if_empty(mut messages: Vec) -> (Vec, Vec) { - let mut issues = Vec::new(); - - if messages.is_empty() { - issues.push("Added placeholder user message to empty conversation".to_string()); - messages.push(Message::user().with_text(PLACEHOLDER_USER_MESSAGE)); - } - (messages, issues) -} - -pub fn debug_conversation_fix( - messages: &[Message], - fixed: &[Message], - issues: &[String], -) -> String { - let mut output = String::new(); - - output.push_str("=== CONVERSATION FIX DEBUG ===\n\n"); - - output.push_str("BEFORE:\n"); - for (i, msg) in messages.iter().enumerate() { - output.push_str(&format!(" [{}] {}\n", i, msg.debug())); - } - - output.push_str("\nISSUES FOUND:\n"); - if issues.is_empty() { - output.push_str(" (none)\n"); - } else { - for issue in issues { - output.push_str(&format!(" - {}\n", issue)); - } - } - - output.push_str("\nAFTER:\n"); - for (i, msg) in fixed.iter().enumerate() { - output.push_str(&format!(" [{}] {}\n", i, msg.debug())); - } - - output.push_str("\n==============================\n"); - output -} +pub use goose_types::{ + debug_conversation_fix, effective_role, fix_conversation, merge_consecutive_messages, + Conversation, InvalidConversation, +}; #[cfg(test)] mod tests {