fix: handle array content and normalize tool_calls from providers#24
Conversation
Two related response parsing fixes: 1. Content as array: Some providers (Gemini OpenAI-compat, OpenRouter) return `content` as an array of parts instead of a string. Previously this raised NoContent error, losing the entire response including any tool_calls. Now extracts text from array parts gracefully. 2. Silent tool_calls loss: When `arguments` is returned as a JSON object (Gemini) instead of a JSON-encoded string, deserialization failed silently via `.ok()`, dropping all tool call intent. Now normalizes object arguments to strings before parsing, and logs parse failures at warn level instead of swallowing them.
There was a problem hiding this comment.
Code Review
This pull request updates the LLMClient in src/utils.rs to improve compatibility with various LLM providers by handling content returned as an array of parts and normalizing tool call arguments from JSON objects or arrays into strings. Feedback suggests returning an error instead of an empty string when encountering unexpected content types to prevent silent failures and recommends using the warn! macro directly for consistency with the rest of the codebase.
| .join("") | ||
| } else { | ||
| content_val.as_str().ok_or(LLMError::NoContent)?.to_string() | ||
| "".to_string() |
There was a problem hiding this comment.
The catch-all else block now returns an empty string, which silently ignores malformed content (e.g., if the provider returns an object or a number). The previous implementation correctly errored out in these cases using LLMError::NoContent. It is better to preserve this error for truly unexpected types to help with debugging and ensure the response follows the expected schema.
| "".to_string() | |
| return Err(LLMError::NoContent); |
| match serde_json::from_value::<Vec<ToolCallRequest>>(tc_json) { | ||
| Ok(calls) => Some(calls), | ||
| Err(e) => { | ||
| log::warn!( |
|
✅ Fixes applied:
Ready for re-review. |
Summary
NoContenterror when providers (Gemini, OpenRouter) returncontentas an array of parts instead of a plain stringargumentsis returned as a JSON object instead of a JSON-encoded string (Gemini)warn-level logging when tool_calls parsing fails instead of silently droppingRoot Cause
content_val.as_str()returnsNonefor arrays →NoContenterror → entire response (including tool_calls) is lostserde_json::from_value(...).ok()silently converts parse errors toNone→ model's tool call intent is completely lost → empty responses or re-prompt loopsTest plan
cargo checkpasses