fix(core): route streaming tool call chunks by protocol index to prevent null tool names#2205
fix(core): route streaming tool call chunks by protocol index to prevent null tool names#2205chickenlj wants to merge 3 commits into
Conversation
…ent null tool names In streaming mode, argument fragments were merged into tool calls purely via lastToolCallKey heuristics because the parser discarded the tool_calls[].index of each delta. When a model or gateway never sends function.name (or sends it after argument fragments), the accumulated tool call ended up with a null name, was persisted in memory, and then skipped with "ToolUseBlock has null id or name" on every subsequent request, stalling the ReAct loop. Propagate the protocol-level index through block metadata and let the accumulator use it as the primary routing key, so all chunks of the same tool call converge on one builder regardless of id/name presence. Tool calls whose name never arrived are now dropped with a warning at build time instead of polluting memory. Blocks without the index keep the legacy routing behavior.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR fixes a streaming-mode tool-call accumulation bug where OpenAI-compatible providers may omit function.name (and sometimes send it late), previously causing ToolUseBlock instances with null names to be persisted and subsequently skipped, stalling ReAct execution. It also standardizes LF line endings across platforms to avoid Spotless CRLF/LF mismatches.
Changes:
- Propagates protocol
tool_calls[].indexthroughOpenAIResponseParserintoToolUseBlockmetadata and uses it inToolCallsAccumulatoras the primary routing key for streaming tool-call deltas. - Prevents persisting unexecutable tool calls by dropping accumulated builders whose tool name never arrived, and strips the routing metadata from final accumulated blocks.
- Adds tests covering standard streaming, late-arriving names, missing names, interleaved parallel calls, and legacy behavior without stream index; enforces LF line endings via Spotless +
.gitattributes.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Configure Spotless to use UNIX line endings for consistent formatting across platforms. |
| agentscope-distribution/agentscope-bom/pom.xml | Same Spotless line-ending normalization for the BOM module. |
| agentscope-dependencies-bom/pom.xml | Same Spotless line-ending normalization for the dependencies BOM module. |
| agentscope-core/src/main/java/io/agentscope/core/message/ToolUseBlock.java | Introduce METADATA_STREAM_INDEX for internal streaming routing. |
| agentscope-core/src/main/java/io/agentscope/core/formatter/openai/OpenAIResponseParser.java | Attach stream index metadata to tool-call blocks and fragments during chunk parsing. |
| agentscope-core/src/main/java/io/agentscope/core/agent/accumulator/ToolCallsAccumulator.java | Route streaming tool-call chunks by stream index; drop nameless calls; strip routing metadata. |
| agentscope-core/src/test/java/io/agentscope/core/formatter/openai/OpenAIResponseParserTest.java | Add coverage asserting stream-index metadata propagation on named chunks and fragments. |
| agentscope-core/src/test/java/io/agentscope/core/agent/accumulator/ToolCallsAccumulatorTest.java | Add coverage for stream-index routing, late names, nameless call dropping, and legacy behavior. |
| .gitattributes | Enforce LF for text files and mark binaries to avoid line-ending normalization. |
| // 0. Prefer the protocol-level stream index when the parser provided one | ||
| Integer streamIndex = getStreamIndex(block); | ||
| if (streamIndex != null) { | ||
| String key = streamIndexToKey.get(streamIndex); | ||
| if (key == null) { | ||
| // First chunk for this index: prefer the real tool ID as key, | ||
| // otherwise derive a stable key from the index itself | ||
| if (block.getId() != null && !block.getId().isEmpty()) { | ||
| key = block.getId(); | ||
| } else { | ||
| key = "stream_index:" + streamIndex; | ||
| } | ||
| streamIndexToKey.put(streamIndex, key); | ||
| } | ||
| lastToolCallKey = key; | ||
| return key; | ||
| } |
| log.warn( | ||
| "Dropping accumulated tool call without a name (id={}," | ||
| + " arguments={}). The model or gateway did not send" | ||
| + " function.name in any streaming chunk.", | ||
| builder.toolId, | ||
| builder.rawContent); | ||
| return false; |
AgentScopeJavaBot
left a comment
AgentScopeJavaBot
left a comment
There was a problem hiding this comment.
🤖 AI Review
This PR fixes a real streaming bug (issue #1237): when OpenAI-compatible streaming responses omit function.name from every chunk, ToolCallsAccumulator previously produced a ToolUseBlock with a null name, stalling the agent. The fix has three coordinated parts: (1) OpenAIResponseParser now propagates tool_calls[].index into ToolUseBlock metadata, (2) ToolCallsAccumulator uses the protocol stream index as the primary routing key for chunk merging, and (3) buildAllToolCalls() defensively drops builders whose name never arrived. The core fix is well-architected, correctly solves the described problem, and includes thorough unit tests (9 new tests covering index routing, parallel calls, fragment merging, and nameless-call dropping). Note: the PR also bundles unrelated changes (.gitattributes, BOM pom entries, Claw controller WorkspaceManager fixes) that should ideally be in separate commits/PRs.
(inline comments could not be attached — line numbers fell outside PR hunks. See archived report.)
AgentScopeJavaBot
left a comment
AgentScopeJavaBot
left a comment
There was a problem hiding this comment.
🤖 AI Review
This PR fixes a real streaming bug (issue #1237): when OpenAI-compatible streaming responses omit function.name from every chunk, ToolCallsAccumulator previously produced a ToolUseBlock with a null name, stalling the agent. The fix has three coordinated parts: (1) OpenAIResponseParser now propagates tool_calls[].index into ToolUseBlock metadata, (2) ToolCallsAccumulator uses the protocol stream index as the primary routing key for chunk merging, and (3) buildAllToolCalls() defensively drops builders whose name never arrived. The core fix is well-architected, correctly solves the described problem, and includes thorough unit tests (9 new tests covering index routing, parallel calls, fragment merging, and nameless-call dropping). Note: the PR also bundles unrelated changes (.gitattributes, BOM pom entries, Claw controller WorkspaceManager fixes) that should ideally be in separate commits/PRs.
(inline comments could not be attached — line numbers fell outside PR hunks. See archived report.)
AgentScopeJavaBot
added
bug
Fixes #1237
Summary
In streaming mode,
OpenAIResponseParserdiscarded the protocol-leveltool_calls[].indexof each delta, soToolCallsAccumulatorhad to merge argument fragments purely vialastToolCallKeyorder heuristics. When a model or gateway never sendsfunction.namein any chunk (observed with Qwen3-14b behind an OpenAI-compatible gateway), or sends it after argument fragments, the accumulated tool call ended up with anullname. That block was persisted into memory, and every subsequent request loggedToolUseBlock has null id or name, skipping, leaving the ReAct agent stuck in a retry loop until token exhaustion. This mirrors the design of the Python implementation, which keys streaming tool call accumulation bytool_call.indexand therefore does not exhibit this bug.OpenAIResponseParsernow propagatestool_calls[].indexon both named chunks and fragments via block metadata (ToolUseBlock.METADATA_STREAM_INDEX). Fragments still carry no fabricated id (an earlier attempt, fix(ToolCallsAccumulator): fallback to 'unknown_tool' for null tool names and use stable synthetic IDs for streaming tool calls #1238, fabricated ids from the index, which split one tool call into two builders whenever the first chunk carried a real id).ToolCallsAccumulatoruses the stream index as the primary routing key, so all chunks of the same tool call converge on one builder regardless of id/name presence. This also fixes late-arriving names and interleaved parallel tool call deltas. Blocks without the index (other providers' parsers) keep the legacy routing path unchanged.buildAllToolCalls()drops builders whose name never arrived, with a diagnostic warning, instead of emitting an unexecutable block with a null name into memory. The internal routing metadata is stripped from the final block.Test plan
function.name): call dropped with warning, no null-name block emittedagentscope-coresuite: 3821 tests, 0 failures