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
65 changes: 63 additions & 2 deletions crates/goose-providers/src/formats/openai.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1004,8 +1004,9 @@ where
let mut tool_call_data: ToolCallData = HashMap::new();

if let Some(tool_calls) = &chunk.choices[0].delta.tool_calls {
for tool_call in tool_calls {
if let (Some(index), Some(id), Some(name)) = (tool_call.index, &tool_call.id, &tool_call.function.name) {
for (position, tool_call) in tool_calls.iter().enumerate() {
if let (Some(id), Some(name)) = (&tool_call.id, &tool_call.function.name) {
let index = tool_call.index.unwrap_or(position as i32);

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 Handle indexless continuation chunks

When an indexless provider streams a tool call over multiple SSE chunks, this fallback only assigns a synthetic index to the first delta. The continuation loop below still ignores any delta_tool_calls without index, so the later argument fragments are dropped and Goose emits the tool request with empty or truncated parameters; this is the common streaming shape where the first chunk carries id/name and later chunks carry function.arguments.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this is not wrong, but also we don't have any examples of providers doing this, so i think we leave support for this or similar until we have a real world use case

tool_call_data.insert(index, (id.clone(), name.clone(), tool_call.function.arguments.clone(), tool_call.extra.clone()));
}
}
Expand Down Expand Up @@ -3652,6 +3653,66 @@ data: [DONE]"#;
Ok(())
}

#[tokio::test]
async fn test_streaming_tool_call_without_tool_call_index() -> anyhow::Result<()> {
let response_lines = concat!(
"data: {\"id\":\"x\",\"object\":\"chat.completion.chunk\",\"model\":\"m\",\"choices\":[{\"index\":0,\"delta\":{\"role\":\"assistant\",\"tool_calls\":[{\"id\":\"functions.get_weather:0\",\"type\":\"function\",\"function\":{\"name\":\"get_weather\",\"arguments\":\"{\\\"city\\\": \\\"Paris\\\"}\"}}]},\"finish_reason\":\"tool_calls\"}]}\n",
"data: [DONE]"
);
let lines: Vec<String> = response_lines.lines().map(|s| s.to_string()).collect();
let response_stream = tokio_stream::iter(lines.into_iter().map(Ok));
let mut messages = std::pin::pin!(response_to_streaming_message(response_stream));

let mut tool_calls = Vec::new();
while let Some(result) = messages.next().await {
let (message, _usage) = result?;
if let Some(msg) = message {
for content in &msg.content {
if let MessageContent::ToolRequest(request) = content {
let tool_call = request.tool_call.as_ref().expect("tool call should parse");
tool_calls.push((tool_call.name.to_string(), tool_call.arguments.clone()));
}
}
}
}

assert_eq!(tool_calls.len(), 1);
assert_eq!(tool_calls[0].0, "get_weather");
assert_eq!(tool_calls[0].1, Some(object!({"city": "Paris"})));
Ok(())
}

#[tokio::test]
async fn test_streaming_multiple_tool_calls_without_tool_call_index() -> anyhow::Result<()> {
let response_lines = concat!(
"data: {\"id\":\"x\",\"object\":\"chat.completion.chunk\",\"model\":\"m\",\"choices\":[{\"index\":0,\"delta\":{\"role\":\"assistant\",\"tool_calls\":[{\"id\":\"functions.get_weather:0\",\"type\":\"function\",\"function\":{\"name\":\"get_weather\",\"arguments\":\"{\\\"city\\\": \\\"Paris\\\"}\"}},{\"id\":\"functions.get_weather:1\",\"type\":\"function\",\"function\":{\"name\":\"get_weather\",\"arguments\":\"{\\\"city\\\": \\\"Tokyo\\\"}\"}}]},\"finish_reason\":\"tool_calls\"}]}\n",
"data: [DONE]"
);
let lines: Vec<String> = response_lines.lines().map(|s| s.to_string()).collect();
let response_stream = tokio_stream::iter(lines.into_iter().map(Ok));
let mut messages = std::pin::pin!(response_to_streaming_message(response_stream));

let mut tool_calls = Vec::new();
while let Some(result) = messages.next().await {
let (message, _usage) = result?;
if let Some(msg) = message {
for content in &msg.content {
if let MessageContent::ToolRequest(request) = content {
let tool_call = request.tool_call.as_ref().expect("tool call should parse");
tool_calls.push((tool_call.name.to_string(), tool_call.arguments.clone()));
}
}
}
}

assert_eq!(tool_calls.len(), 2);
assert_eq!(tool_calls[0].0, "get_weather");
assert_eq!(tool_calls[0].1, Some(object!({"city": "Paris"})));
assert_eq!(tool_calls[1].0, "get_weather");
assert_eq!(tool_calls[1].1, Some(object!({"city": "Tokyo"})));
Ok(())
}

// Streaming counterpart: both fields in one delta must parse and yield
// thinking content, not fail with "duplicate field `reasoning_content`".
#[tokio::test]
Expand Down
Loading