Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 77 additions & 1 deletion crates/goose/src/providers/formats/databricks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ fn format_messages(messages: &[Message], image_format: &ImageFormat) -> Vec<Data
let mut content_array = Vec::new();
let mut has_tool_calls = false;
let mut has_multiple_content = false;
// Deferred so all tool-role messages stay consecutive (required by Claude via Databricks).
let mut pending_image_messages: Vec<DatabricksMessage> = Vec::new();

for content in &message.content {
match content {
Expand Down Expand Up @@ -188,7 +190,13 @@ fn format_messages(messages: &[Message], image_format: &ImageFormat) -> Vec<Data
}
}
MessageContent::ToolResponse(response) => {
result.extend(format_tool_response(response, image_format));
for msg in format_tool_response(response, image_format) {
if msg.role == "user" {
pending_image_messages.push(msg);
Comment on lines +194 to +195
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve image-to-tool association when reordering responses

Deferring every user image message until after all tool responses breaks the implicit pairing between a tool result and its image payload. In mixed parallel results (e.g., tool A has an image, tool B is text-only), the image from A is emitted after B, and these image messages carry no tool_call_id, so the model can misattribute which tool produced which image. This is a behavior regression from the prior adjacent ordering and can lead to incorrect reasoning about tool outputs.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've prepared a pull request into this branch so the approach I took can be reviewed first msteve123#1

@DOsinga Did you want to take a look?

} else {
result.push(msg);
}
}
}
MessageContent::Image(image) => {
content_array.push(convert_image(image, image_format));
Expand All @@ -210,6 +218,8 @@ fn format_messages(messages: &[Message], image_format: &ImageFormat) -> Vec<Data
}
}

result.extend(pending_image_messages);

if !content_array.is_empty() {
converted.content = if content_array.len() == 1
&& !has_multiple_content
Expand Down Expand Up @@ -1616,4 +1626,70 @@ mod tests {

Ok(())
}

#[test]
fn test_parallel_tool_responses_with_images_are_consecutive() -> anyhow::Result<()> {
// Regression: #7449 — parallel tool calls with images must keep tool messages consecutive.
let messages = vec![
Message::assistant()
.with_tool_request("id1", Ok(CallToolRequestParams::new("tool_a")))
.with_tool_request("id2", Ok(CallToolRequestParams::new("tool_b"))),
Message::user()
.with_tool_response(
"id1",
Ok(CallToolResult::success(vec![Content::image(
"base64data1".to_string(),
"image/png".to_string(),
)])),
)
.with_tool_response(
"id2",
Ok(CallToolResult::success(vec![Content::image(
"base64data2".to_string(),
"image/png".to_string(),
)])),
),
];

let as_value =
serde_json::to_value(format_messages(&messages, &ImageFormat::OpenAi)).unwrap();
let spec = as_value.as_array().unwrap();
let roles: Vec<&str> = spec.iter().map(|m| m["role"].as_str().unwrap()).collect();

// Without the fix this was ["assistant", "tool", "user", "tool", "user"].
assert_eq!(roles, vec!["assistant", "tool", "tool", "user", "user"]);

Ok(())
}

#[test]
fn test_mixed_tool_responses_image_and_text_ordering() -> anyhow::Result<()> {
// Mixed case: only one tool response has an image.
let messages = vec![
Message::assistant()
.with_tool_request("id1", Ok(CallToolRequestParams::new("tool_a")))
.with_tool_request("id2", Ok(CallToolRequestParams::new("tool_b"))),
Message::user()
.with_tool_response(
"id1",
Ok(CallToolResult::success(vec![Content::text("text result")])),
)
.with_tool_response(
"id2",
Ok(CallToolResult::success(vec![Content::image(
"base64data".to_string(),
"image/png".to_string(),
)])),
),
];

let as_value =
serde_json::to_value(format_messages(&messages, &ImageFormat::OpenAi)).unwrap();
let spec = as_value.as_array().unwrap();
let roles: Vec<&str> = spec.iter().map(|m| m["role"].as_str().unwrap()).collect();

assert_eq!(roles, vec!["assistant", "tool", "tool", "user"]);

Ok(())
}
}