fix(provider): add missing index field to streaming tool_call deltas (#6661)#6692
fix(provider): add missing index field to streaming tool_call deltas (#6661)#6692Yaohua-Leo wants to merge 2 commits intoAstrBotDevs:masterfrom
Conversation
- Fix AstrBotDevs#6661: Streaming tool_call arguments lost when OpenAI-compatible proxy omits index field - Gemini and some proxies (e.g. Continue) don't include index field in tool_call deltas - Add default index=0 when missing to prevent ChatCompletionStreamState.handle_chunk() from rejecting chunks Fixes AstrBotDevs#6661
dosubot
bot
added
the
size:XS
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 bug affecting the streaming of 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
|
![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 1 issue, and left some high level feedback:
- The fix currently only inspects
chunk.choices[0]; if a provider ever sends tool_call deltas on multiple choices in a single chunk, those additional choices will still be dropped—consider iterating over allchunk.choicesinstead of just the first. - You always set a missing
tool_call.indexto0, which might conflict with multiple tool calls in the same choice; it may be safer to derive the index from the tool_calls list position or preserve any existing ordering assumptions inhandle_chunk().
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The fix currently only inspects `chunk.choices[0]`; if a provider ever sends tool_call deltas on multiple choices in a single chunk, those additional choices will still be dropped—consider iterating over all `chunk.choices` instead of just the first.
- You always set a missing `tool_call.index` to `0`, which might conflict with multiple tool calls in the same choice; it may be safer to derive the index from the tool_calls list position or preserve any existing ordering assumptions in `handle_chunk()`.
## Individual Comments
### Comment 1
<location path="astrbot/core/provider/sources/openai_source.py" line_range="310-315" />
<code_context>
state = ChatCompletionStreamState()
async for chunk in stream:
+ # Fix for #6661: Add missing 'index' field to tool_call deltas
+ # Gemini and some OpenAI-compatible proxies omit this field
+ if chunk.choices:
+ choice = chunk.choices[0]
+ if choice.delta and choice.delta.tool_calls:
+ for tc in choice.delta.tool_calls:
+ if not hasattr(tc, "index") or tc.index is None:
+ tc.index = 0
</code_context>
<issue_to_address>
**issue (bug_risk):** Setting all missing tool_call indices to 0 could break multi-tool-call streams.
This change sets `tc.index = 0` for every tool_call delta missing an index. If a choice yields multiple tool_calls, they’ll all share index 0, which can break any logic that uses indices to track or merge tool_calls across chunks.
If multiple tool_calls per choice are possible, consider deriving the index from position (e.g. `enumerate(choice.delta.tool_calls)`) or inferring it inside `ChatCompletionStreamState`, where you can use previously seen tool_calls for that choice. Otherwise, you risk incorrect tool_call reconstruction while only masking the missing-field issue.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
dosubot
bot
added
the
area:provider
![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 adds a workaround for OpenAI-compatible providers that omit the index field in streaming tool call deltas. The change correctly patches the chunk before it's processed. However, the current implementation has a flaw where it assigns a hardcoded index=0, which can cause issues if multiple tool calls are streamed concurrently. I've left a comment with details on this potential bug and a suggestion for a more robust implementation. I've also added a minor suggestion to improve code style.
| if choice.delta and choice.delta.tool_calls: | ||
| for tc in choice.delta.tool_calls: | ||
| if not hasattr(tc, "index") or tc.index is None: | ||
| tc.index = 0 |
There was a problem hiding this comment.
Hardcoding tc.index = 0 can lead to incorrect behavior if the streaming provider sends multiple tool calls concurrently without an index. All tool calls would be assigned to the same index, causing them to be merged incorrectly by ChatCompletionStreamState. This could result in lost tool calls or malformed arguments.
A more robust solution would be to assign a unique index to each tool call. This can be achieved by maintaining a state within _query_stream that maps tool call IDs to indices. When a new tool call ID is seen, assign it the next available index.
If you can confirm that the targeted OpenAI-compatible proxies will only ever stream one tool call at a time, then this fix is sufficient. Otherwise, I strongly recommend implementing the stateful indexing logic to prevent bugs with multiple tool calls.
| choice = chunk.choices[0] | ||
| if choice.delta and choice.delta.tool_calls: | ||
| for tc in choice.delta.tool_calls: | ||
| if not hasattr(tc, "index") or tc.index is None: |
There was a problem hiding this comment.
This condition can be written more concisely and safely using getattr.
| if not hasattr(tc, "index") or tc.index is None: | |
| if getattr(tc, "index", None) is None: |
- Use enumerate() to assign correct index based on list position - Iterate over all choices (not just the first) for completeness - Addresses review feedback from sourcery-ai and gemini-code-assist
|
Hi, I've addressed the review feedback from @sourcery-ai and @gemini-code-assist. The fix now uses enumerate() for correct index assignment and iterates over all choices. Please take another look when you have time. Thanks! |
There was a problem hiding this comment.
Code Review
This pull request introduces a fix to handle streaming tool call deltas from OpenAI-compatible proxies that omit the index field. The change correctly patches the incoming chunks by adding a sequential index to tool calls where it's missing, preventing arguments from being dropped. The implementation is straightforward and effectively addresses the issue. I have one suggestion to improve the code's readability and maintainability by using a more idiomatic Python pattern.
| for choice in chunk.choices: | ||
| if choice.delta and choice.delta.tool_calls: | ||
| for idx, tc in enumerate(choice.delta.tool_calls): | ||
| if not hasattr(tc, "index") or tc.index is None: |
There was a problem hiding this comment.
For improved readability and maintainability, consider using getattr with a default value to check for the missing or None index. This is a more concise and idiomatic Python pattern for this type of check.
| if not hasattr(tc, "index") or tc.index is None: | |
| if getattr(tc, "index", None) is None: |
1 participant
Summary
Fixes #6661 - Streaming tool_call arguments lost when OpenAI-compatible proxy omits
indexfield (e.g. Gemini)Changes
indexfield to tool_call deltas before passing toChatCompletionStreamState.handle_chunk()indexfieldhandle_chunk()to reject the chunk and silently drop tool_call argumentsTesting
✅ Docker sandbox test passed
Summary by Sourcery
Bug Fixes: