-
Notifications
You must be signed in to change notification settings - Fork 214
fix: add tooluseblock start #591
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
|
|
releate #588 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes an issue with the qwen3-max model where ToolUseBlock events are returned all at once at the end of streaming instead of incrementally, causing ToolCallStart events to be missing. The fix implements a backfill mechanism that caches ToolUseBlocks when they arrive during REASONING events, then emits the missing ToolCallStart and ToolCallArgs events when ToolResultBlock is processed.
Changes:
- Added caching mechanism for ToolUseBlock instances to support delayed ToolCallStart event emission
- Implemented backfill logic that checks for missing ToolCallStart events when processing ToolResult events
- Added HashMap to store ToolUseBlock instances by tool call ID for later retrieval
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| boolean hasToolUseBlock = | ||
| msg.getContent().stream().anyMatch(block -> block instanceof ToolUseBlock); |
Copilot
AI
Jan 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable hasToolUseBlock is declared but never used. This variable appears to have been introduced for debugging or future use but is not currently utilized in the logic. Consider removing it to keep the code clean.
| boolean hasToolUseBlock = | |
| msg.getContent().stream().anyMatch(block -> block instanceof ToolUseBlock); |
| void cacheToolUseBlock(ToolUseBlock toolUse) { | ||
| if (toolUse.getId() != null) { | ||
| toolUseBlocks.put(toolUse.getId(), toolUse); | ||
| } | ||
| } |
Copilot
AI
Jan 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new private method cacheToolUseBlock is missing Javadoc documentation. According to project standards, all methods should have Javadoc comments explaining their purpose, parameters, and behavior. Please add Javadoc describing that this method caches a ToolUseBlock for later retrieval during the backfill process when ToolCallStart events are missing.
| ToolUseBlock getToolUseBlock(String toolCallId) { | ||
| return toolUseBlocks.get(toolCallId); | ||
| } |
Copilot
AI
Jan 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new private method getToolUseBlock is missing Javadoc documentation. According to project standards, all methods should have Javadoc comments explaining their purpose, parameters, and return values. Please add Javadoc describing that this method retrieves a cached ToolUseBlock by its tool call ID, and returns null if not found.
| boolean hasStarted = state.hasStartedToolCall(toolCallId); | ||
| if (!hasStarted) { | ||
| ToolUseBlock toolUse = state.getToolUseBlock(toolCallId); | ||
| if (toolUse != null) { | ||
| events.add( | ||
| new AguiEvent.ToolCallStart( | ||
| state.threadId, | ||
| state.runId, | ||
| toolCallId, | ||
| toolUse.getName())); | ||
| state.startToolCall(toolCallId); | ||
|
|
||
| if (config.isEmitToolCallArgs()) { | ||
| String args = toolUse.getContent(); | ||
| if (args != null && !args.isEmpty()) { | ||
| events.add( | ||
| new AguiEvent.ToolCallArgs( | ||
| state.threadId, state.runId, toolCallId, args)); | ||
| } | ||
| } | ||
| } else { | ||
| events.add( | ||
| new AguiEvent.ToolCallStart( | ||
| state.threadId, state.runId, toolCallId, "unknown")); | ||
| state.startToolCall(toolCallId); | ||
| } | ||
| } |
Copilot
AI
Jan 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The backfill logic for ToolCallStart events (lines 208-234) introduces critical new behavior but is only covered by integration tests with a real model. Consider adding a unit test to AguiAgentAdapterTest.java that simulates the scenario where a ToolResultBlock arrives without a prior ToolCallStart event. This would verify the backfill logic works correctly without requiring external API calls.
| // Emit tool call start | ||
| String toolCallId = toolUse.getId(); | ||
| if (toolCallId == null) { | ||
| toolCallId = UUID.randomUUID().toString(); |
Copilot
AI
Jan 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a potential logic issue with caching ToolUseBlocks when the ID is null. On line 174-177, if toolUse.getId() is null, a UUID is generated for toolCallId, but the ToolUseBlock object is not updated with this ID. Then on line 178, cacheToolUseBlock(toolUse) is called, which only caches if toolUse.getId() is not null (line 346). This means that if a ToolUseBlock arrives without an ID, it won't be cached, and the backfill logic (lines 210-227) won't be able to retrieve it later. Consider either: 1) only generating a UUID after checking if toolUse.getId() is not null, or 2) modifying the cache to use the generated toolCallId instead of relying on toolUse.getId().
| toolCallId = UUID.randomUUID().toString(); | |
| toolCallId = UUID.randomUUID().toString(); | |
| // Ensure the ToolUseBlock carries the generated ID so caching and backfill work | |
| toolUse.setId(toolCallId); |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |

AgentScope-Java Version
[The version of AgentScope-Java you are working on, e.g. 1.0.7, check your pom.xml dependency version or run
mvn dependency:tree | grep agentscope-parent:pom(only mac/linux)]Description
[Please describe the background, purpose, changes made, and how to test this PR]
qwen3max:
Behavior: ToolUseBlock is not returned during the streaming phase; it is returned all at once at the last moment.
Streaming Phase: No ToolCallStart. A replacement is sent when END is detected.
点击查看测试代码
Checklist
Please check the following items before code is ready to be reviewed.
mvn spotless:applymvn test)