diff --git a/crates/goose/src/providers/formats/openai.rs b/crates/goose/src/providers/formats/openai.rs index c696cb8e34..28420888a0 100644 --- a/crates/goose/src/providers/formats/openai.rs +++ b/crates/goose/src/providers/formats/openai.rs @@ -126,7 +126,7 @@ pub fn format_messages(messages: &[Message], image_format: &ImageFormat) -> Vec< let mut output = Vec::new(); let mut content_array = Vec::new(); - let mut text_array = Vec::new(); + let mut has_non_text_content = false; let mut reasoning_text = String::new(); for content in &message.content { @@ -136,16 +136,17 @@ pub fn format_messages(messages: &[Message], image_format: &ImageFormat) -> Vec< if message.role == Role::User { if let Some(image_path) = detect_image_path(&text.text) { if let Ok(image) = load_image_file(image_path) { + has_non_text_content = true; content_array.push(json!({"type": "text", "text": text.text})); content_array.push(convert_image(&image, image_format)); } else { - text_array.push(text.text.clone()); + content_array.push(json!({"type": "text", "text": text.text})); } } else { - text_array.push(text.text.clone()); + content_array.push(json!({"type": "text", "text": text.text})); } } else { - text_array.push(text.text.clone()); + content_array.push(json!({"type": "text", "text": text.text})); } } } @@ -259,6 +260,7 @@ pub fn format_messages(messages: &[Message], image_format: &ImageFormat) -> Vec< MessageContent::ActionRequired(_) => {} MessageContent::Image(image) => { if message.role == Role::User { + has_non_text_content = true; content_array.push(convert_image(image, image_format)); } else { content_array.push(json!({ @@ -304,9 +306,15 @@ pub fn format_messages(messages: &[Message], image_format: &ImageFormat) -> Vec< } if !content_array.is_empty() { - converted["content"] = json!(content_array); - } else if !text_array.is_empty() { - converted["content"] = json!(text_array.join("\n")); + if has_non_text_content { + converted["content"] = json!(content_array); + } else { + let texts: Vec = content_array + .iter() + .filter_map(|v| v["text"].as_str().map(|s| s.to_string())) + .collect(); + converted["content"] = json!(texts.join("\n")); + } } // Some strict OpenAI-compatible providers require "content" to be present @@ -1210,6 +1218,40 @@ mod tests { Ok(()) } + #[test] + fn test_format_messages_with_text_and_image_preserves_order() { + // Text before image: order should be [text, image] + let msg_text_first = Message::user() + .with_text("Describe this image") + .with_image("aW1hZ2VkYXRh", "image/png"); + + let spec = format_messages(&[msg_text_first], &ImageFormat::OpenAi); + assert_eq!(spec.len(), 1); + assert_eq!(spec[0]["role"], "user"); + + let content = spec[0]["content"] + .as_array() + .expect("content should be an array"); + assert_eq!(content.len(), 2, "expected text + image entries"); + assert_eq!(content[0]["type"], "text"); + assert_eq!(content[0]["text"], "Describe this image"); + assert_eq!(content[1]["type"], "image_url"); + + // Image before text: order should be [image, text] + let msg_image_first = Message::user() + .with_image("aW1hZ2VkYXRh", "image/png") + .with_text("What do you see?"); + + let spec2 = format_messages(&[msg_image_first], &ImageFormat::OpenAi); + let content2 = spec2[0]["content"] + .as_array() + .expect("content should be an array"); + assert_eq!(content2.len(), 2, "expected image + text entries"); + assert_eq!(content2[0]["type"], "image_url"); + assert_eq!(content2[1]["type"], "text"); + assert_eq!(content2[1]["text"], "What do you see?"); + } + #[test] fn test_response_to_message_text() -> anyhow::Result<()> { let response = json!({