fix(ToolCallsAccumulator): fallback to 'unknown_tool' for null tool names and use stable synthetic IDs for streaming tool calls#1238
Conversation
…ames and use stable synthetic IDs for streaming tool calls
There was a problem hiding this comment.
Pull request overview
This PR addresses a streaming-mode tool-calling bug (Issue #1237) where tool-call fragments could end up persisted with a null tool name and/or unstable identifiers, causing downstream conversion to skip ToolUseBlocks and breaking subsequent agent iterations.
Changes:
- Generate stable synthetic tool call IDs during OpenAI-like streaming using
toolIndex(e.g.,streaming_idx_0) when provider IDs are missing. - Propagate the resolved tool call ID into fragment
ToolUseBlocks (instead of emitting fragments with an empty id). - Add a null-name fallback in
ToolCallsAccumulator(unknown_tool) and extend test coverage for the streaming edge cases.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| agentscope-core/src/main/java/io/agentscope/core/formatter/openai/OpenAIResponseParser.java | Uses stable synthetic IDs (by tool index) and assigns IDs to fragment ToolUseBlocks to enable consistent merging across chunks. |
| agentscope-core/src/main/java/io/agentscope/core/agent/accumulator/ToolCallsAccumulator.java | Ensures built ToolUseBlocks always have a non-null name by falling back to unknown_tool. |
| agentscope-core/src/test/java/io/agentscope/core/formatter/openai/OpenAIResponseParserTest.java | Adds a test asserting stable synthetic IDs across streaming chunks by index. |
| agentscope-core/src/test/java/io/agentscope/core/agent/accumulator/ToolCallsAccumulatorTest.java | Adds a test ensuring fragment-only streams still produce a ToolUseBlock with a non-null name. |
| if (toolIndex != null) { | ||
| toolCallId = "streaming_idx_" + toolIndex; | ||
| } else { | ||
| toolCallId = "streaming_" + System.currentTimeMillis(); | ||
| } |
There was a problem hiding this comment.
When toolCallId is missing and toolIndex is also null, the fallback "streaming_" + System.currentTimeMillis() will generate a different synthetic ID for each chunk. That makes it impossible for downstream accumulators to reliably merge multiple chunks belonging to the same tool call (and can lead to duplicated/fragmented tool calls). Consider using a deterministic/stable fallback in this branch (e.g., reuse a per-parser last-seen synthetic ID for fragments, or derive one from choice index + position-in-toolCalls list) so repeated chunks map to the same tool call.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Closing due to inactivity. Feel free to reopen when ready. |

Fixes #1237