fix: fallbfix: fallback for streaming tool_call args lost when proxy omits index fieldck for streaming tool_call args lost when proxy omits inde…#6662
Conversation
…x field Some OpenAI-compatible proxies (e.g. Gemini via continue proxy) return streaming tool_call chunks without the required 'index' field, causing the openai SDK's ChatCompletionStreamState to reject them with: "Expected list delta entry to have an 'index' key" This results in tool_call arguments being None after stream completion, leading to AttributeError crashes in tool execution. Changes: - Add _StreamToolCallFallback class that manually accumulates tool_call data from rejected stream chunks and restores args after parsing - Add None guard for func_tool_args in tool_loop_agent_runner - Add try/except around json.loads in _parse_openai_completion for robustness against malformed arguments strings Fixes tool_call failures when using Gemini models through OpenAI- compatible proxy services with streaming enabled.
dosubot
bot
added
the
size:M
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue where streaming tool call arguments are lost when using OpenAI-compatible proxy services that omit the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
dosubot
bot
added
the
area:provider
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
_StreamToolCallFallback.collect_from_chunkhelper only looks atchunk.choices[0], so any multi-choice streaming responses will lose fallback data for non-zero choices; consider iterating over allchunk.choicesinstead. - In
_StreamToolCallFallback.apply_to, you unconditionally setresponse.role = "tool"when rebuilding from fallback; verify whether the expected role for messages containingtool_callsshould remain"assistant", as switching the role may break downstream assumptions about assistant vs tool messages. - The
logger.infocalls in the fallback path (e.g. "Stream fallback: restored args...") may be too noisy for normal operation; consider downgrading todebugor gating them behind a verbosity flag while still keeping the warning whenhandle_chunkfails.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `_StreamToolCallFallback.collect_from_chunk` helper only looks at `chunk.choices[0]`, so any multi-choice streaming responses will lose fallback data for non-zero choices; consider iterating over all `chunk.choices` instead.
- In `_StreamToolCallFallback.apply_to`, you unconditionally set `response.role = "tool"` when rebuilding from fallback; verify whether the expected role for messages containing `tool_calls` should remain `"assistant"`, as switching the role may break downstream assumptions about assistant vs tool messages.
- The `logger.info` calls in the fallback path (e.g. "Stream fallback: restored args...") may be too noisy for normal operation; consider downgrading to `debug` or gating them behind a verbosity flag while still keeping the warning when `handle_chunk` fails.
## Individual Comments
### Comment 1
<location path="astrbot/core/provider/sources/openai_source.py" line_range="56" />
<code_context>
+ if not delta or not hasattr(delta, "tool_calls") or not delta.tool_calls:
+ return
+ for tc in delta.tool_calls:
+ idx = getattr(tc, "index", None) or 0
+ if idx not in self._calls:
+ self._calls[idx] = {"id": "", "name": "", "arguments": ""}
</code_context>
<issue_to_address>
**issue (bug_risk):** Potential collision when multiple tool_calls share a falsy index value
`getattr(tc, "index", None) or 0` collapses missing indices and an explicit `index=0` into the same key. If multiple tool calls share `index=0` (or some are missing and some are `0`), their state will be merged and reconstruction will be wrong. Consider preserving `tc.index` when present and using a distinct fallback (e.g., a separate counter or sentinel key) only when the index is actually `None`.
</issue_to_address>
### Comment 2
<location path="astrbot/core/provider/sources/openai_source.py" line_range="97" />
<code_context>
+ response.tools_call_args.append(parsed)
+ response.tools_call_name.append(fb["name"])
+ response.tools_call_ids.append(fb["id"] or f"fallback_{idx}")
+ response.role = "tool"
+ logger.info(f"Stream fallback: rebuilt tool_call {fb['name']}({parsed})")
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Rewriting the final response role to "tool" may be semantically incorrect
This fallback path sets `response.role = "tool"` even though no actual tool result is present. Downstream code usually expects the *requesting* message to be `"assistant"` and only actual tool outputs to be `"tool"`, so this change may break logic that branches on role. Consider keeping the original role here, or only switching to `"tool"` if you’ve confirmed all downstream consumers expect the triggering message to be marked that way.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if not delta or not hasattr(delta, "tool_calls") or not delta.tool_calls: | ||
| return | ||
| for tc in delta.tool_calls: | ||
| idx = getattr(tc, "index", None) or 0 |
There was a problem hiding this comment.
issue (bug_risk): Potential collision when multiple tool_calls share a falsy index value
getattr(tc, "index", None) or 0 collapses missing indices and an explicit index=0 into the same key. If multiple tool calls share index=0 (or some are missing and some are 0), their state will be merged and reconstruction will be wrong. Consider preserving tc.index when present and using a distinct fallback (e.g., a separate counter or sentinel key) only when the index is actually None.
| response.tools_call_args.append(parsed) | ||
| response.tools_call_name.append(fb["name"]) | ||
| response.tools_call_ids.append(fb["id"] or f"fallback_{idx}") | ||
| response.role = "tool" |
There was a problem hiding this comment.
issue (bug_risk): Rewriting the final response role to "tool" may be semantically incorrect
This fallback path sets response.role = "tool" even though no actual tool result is present. Downstream code usually expects the requesting message to be "assistant" and only actual tool outputs to be "tool", so this change may break logic that branches on role. Consider keeping the original role here, or only switching to "tool" if you’ve confirmed all downstream consumers expect the triggering message to be marked that way.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a robust fallback mechanism to handle malformed streaming tool call chunks from non-compliant OpenAI-compatible proxies, which can omit the index field. The core of the fix is the new _StreamToolCallFallback class that manually reconstructs tool call data from rejected stream chunks. The changes also include safeguards in the agent runner and argument parsing logic to prevent crashes. The approach is solid and directly addresses the issue. I've included a couple of minor suggestions to improve code clarity and consistency.
| response.tools_call_args = response.tools_call_args or [] | ||
| response.tools_call_name = response.tools_call_name or [] | ||
| response.tools_call_ids = response.tools_call_ids or [] | ||
| response.tools_call_args.append(parsed) | ||
| response.tools_call_name.append(fb["name"]) | ||
| response.tools_call_ids.append(fb["id"] or f"fallback_{idx}") |
There was a problem hiding this comment.
The LLMResponse dataclass initializes tools_call_args, tools_call_name, and tools_call_ids with default_factory=list, meaning they will be empty lists by default, not None. The assignments using or [] are therefore redundant and can be removed for cleaner code.
| response.tools_call_args = response.tools_call_args or [] | |
| response.tools_call_name = response.tools_call_name or [] | |
| response.tools_call_ids = response.tools_call_ids or [] | |
| response.tools_call_args.append(parsed) | |
| response.tools_call_name.append(fb["name"]) | |
| response.tools_call_ids.append(fb["id"] or f"fallback_{idx}") | |
| response.tools_call_args.append(parsed) | |
| response.tools_call_name.append(fb["name"]) | |
| response.tools_call_ids.append(fb["id"] or f"fallback_{idx}") |
| args = json.loads(tool_call.function.arguments) | ||
| try: | ||
| args = json.loads(tool_call.function.arguments) | ||
| except (json.JSONDecodeError, TypeError): |
There was a problem hiding this comment.
For consistency with the new _StreamToolCallFallback._parse_args method, consider also catching ValueError here. This can improve robustness as json.loads can raise ValueError or its subclasses for various parsing issues.
| except (json.JSONDecodeError, TypeError): | |
| except (json.JSONDecodeError, TypeError, ValueError): |
1 participant
Fixes #6661
Motivation
When using Gemini models through OpenAI-compatible proxy services, streaming tool_call chunks are missing the required
indexfield. The openai SDK'sChatCompletionStreamState.handle_chunk()rejects these chunks, silently droppingtool_call arguments. This causes
AttributeError: 'NoneType' object has no attribute 'items'crashes in toolexecution.
Modifications / 改动点
Files modified:
astrbot/core/provider/sources/openai_source.py_StreamToolCallFallbackclass: manually accumulates tool_call data from rejected stream chunks and restoresargs after parsing
try/exceptaroundjson.loadsin_parse_openai_completionfor robustnessastrbot/core/agent/runners/tool_loop_agent_runner.pyNoneguard forfunc_tool_argsto prevent crashThis is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
Before fix: tool_call args are None, crashes with AttributeError
[WARN] Saving chunk state error: Expected list delta entry to have an index key
[INFO] 使用工具：memory_save，参数：None
AttributeError: 'NoneType' object has no attribute 'items'
After fix: fallback restores args from rejected chunks
[INFO] Stream fallback: restored args for memory_save
[INFO] 使用工具：memory_save，参数：{'content': '用户喜欢吃火锅', 'tags': 'preference,food'}
Zero impact on standard OpenAI/DeepSeek providers — fallback only activates when
handle_chunk()throws.Checklist / 检查清单
indexfield (e.g. Gemini) #6661Summary by Sourcery
Add a streaming tool call fallback for OpenAI-compatible proxies that omit tool_call indices and harden tool-call argument handling to prevent crashes.
Bug Fixes:
Enhancements: