Skip to content

fix(ToolCallsAccumulator): fallback to 'unknown_tool' for null tool names and use stable synthetic IDs for streaming tool calls #1238

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.

Sign up for GitHub

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

Closed
chickenlj wants to merge 1 commit into from
Closed

fix(ToolCallsAccumulator): fallback to 'unknown_tool' for null tool names and use stable synthetic IDs for streaming tool calls #1238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion agentscope-core/src/main/java/io/agentscope/core/agent/accumulator/ToolCallsAccumulator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
*/
public class ToolCallsAccumulator implements ContentAccumulator<ToolUseBlock> {

private static final String UNKNOWN_TOOL_NAME = "unknown_tool";

// Map to support multiple parallel tool calls
// Key: tool identifier (ID, name, or index)
private final Map<String, ToolCallBuilder> builders = new LinkedHashMap<>();
Expand Down Expand Up @@ -103,7 +105,7 @@ ToolUseBlock build() {

return ToolUseBlock.builder()
.id(toolId != null ? toolId : generateId())
.name(name)
.name(name != null ? name : UNKNOWN_TOOL_NAME)
.input(finalArgs)
.content(rawContentStr.isEmpty() ? "{}" : rawContentStr)
.metadata(metadata.isEmpty() ? null : metadata)
Expand Down
13 changes: 10 additions & 3 deletions agentscope-core/src/main/java/io/agentscope/core/formatter/openai/OpenAIResponseParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,8 +461,15 @@ protected ChatResponse parseChunkResponse(OpenAIResponse response, Instant start
}
}

if (toolCallId == null) {
toolCallId = "streaming_" + System.currentTimeMillis();
if (toolCallId == null || toolCallId.isEmpty()) {
// Use index as stable synthetic id so chunks from the same
// tool call can be consistently merged across stream
// updates.
if (toolIndex != null) {
toolCallId = "streaming_idx_" + toolIndex;
} else {
toolCallId = "streaming_" + System.currentTimeMillis();
}
Comment on lines +468 to +472

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
}
if (toolName == null) {
toolName = "";
Expand Down Expand Up @@ -538,7 +545,7 @@ protected ChatResponse parseChunkResponse(OpenAIResponse response, Instant start

contentBlocks.add(
ToolUseBlock.builder()
.id("")
.id(toolCallId)
.name(FRAGMENT_PLACEHOLDER)
.input(new HashMap<>())
.content(arguments)
Expand Down
29 changes: 29 additions & 0 deletions ...ope-core/src/test/java/io/agentscope/core/agent/accumulator/ToolCallsAccumulatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,4 +323,33 @@ void testMultipleParallelToolCallsWithStreamingChunks() {
List<ToolUseBlock> allCalls = accumulator.getAllAccumulatedToolCalls();
assertEquals(2, allCalls.size());
}

@Test
@DisplayName("Should fallback to non-null name when only fragments are received")
void testOnlyFragmentsFallbackName() {
ToolUseBlock fragment1 =
ToolUseBlock.builder()
.id("streaming_idx_0")
.name("__fragment__")
.content("{\"city\":")
.build();
ToolUseBlock fragment2 =
ToolUseBlock.builder()
.id("streaming_idx_0")
.name("__fragment__")
.content("\"Beijing\"}")
.build();

accumulator.add(fragment1);
accumulator.add(fragment2);

List<ToolUseBlock> result = accumulator.buildAllToolCalls();
assertEquals(1, result.size());
ToolUseBlock toolCall = result.get(0);

assertNotNull(toolCall.getName());
assertEquals("unknown_tool", toolCall.getName());
assertEquals("{\"city\":\"Beijing\"}", toolCall.getContent());
assertEquals("Beijing", toolCall.getInput().get("city"));
}
}
72 changes: 72 additions & 0 deletions ...cope-core/src/test/java/io/agentscope/core/formatter/openai/OpenAIResponseParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,78 @@ void testStreamingToolCallFragment() {
assertEquals(OpenAIResponseParser.FRAGMENT_PLACEHOLDER, toolBlock.getName());
}

@Test
@DisplayName("Should use stable synthetic id for streaming tool calls by index")
void testStreamingToolCallUsesStableSyntheticIdByIndex() {
// First chunk: fragment only (no id, no name), has index
OpenAIResponse firstChunk = new OpenAIResponse();
firstChunk.setObject("chat.completion.chunk");

OpenAIFunction firstFunction = new OpenAIFunction();
firstFunction.setName(null);
firstFunction.setArguments("{\"city\":");

OpenAIToolCall firstToolCall = new OpenAIToolCall();
firstToolCall.setId(null);
firstToolCall.setIndex(0);
firstToolCall.setType("function");
firstToolCall.setFunction(firstFunction);

OpenAIMessage firstDelta = new OpenAIMessage();
firstDelta.setToolCalls(List.of(firstToolCall));
firstDelta.setRole("assistant");

OpenAIChoice firstChoice = new OpenAIChoice();
firstChoice.setDelta(firstDelta);
firstChoice.setIndex(0);
firstChunk.setChoices(List.of(firstChoice));

ChatResponse firstResult = parser.parseResponse(firstChunk, startTime);
ToolUseBlock firstBlock =
firstResult.getContent().stream()
.filter(block -> block instanceof ToolUseBlock)
.map(block -> (ToolUseBlock) block)
.findFirst()
.orElse(null);
assertNotNull(firstBlock);
assertEquals("streaming_idx_0", firstBlock.getId());
assertEquals(OpenAIResponseParser.FRAGMENT_PLACEHOLDER, firstBlock.getName());

// Second chunk: same index, now with tool name, still no id
OpenAIResponse secondChunk = new OpenAIResponse();
secondChunk.setObject("chat.completion.chunk");

OpenAIFunction secondFunction = new OpenAIFunction();
secondFunction.setName("get_weather");
secondFunction.setArguments("\"Beijing\"}");

OpenAIToolCall secondToolCall = new OpenAIToolCall();
secondToolCall.setId(null);
secondToolCall.setIndex(0);
secondToolCall.setType("function");
secondToolCall.setFunction(secondFunction);

OpenAIMessage secondDelta = new OpenAIMessage();
secondDelta.setToolCalls(List.of(secondToolCall));
secondDelta.setRole("assistant");

OpenAIChoice secondChoice = new OpenAIChoice();
secondChoice.setDelta(secondDelta);
secondChoice.setIndex(0);
secondChunk.setChoices(List.of(secondChoice));

ChatResponse secondResult = parser.parseResponse(secondChunk, startTime);
ToolUseBlock secondBlock =
secondResult.getContent().stream()
.filter(block -> block instanceof ToolUseBlock)
.map(block -> (ToolUseBlock) block)
.findFirst()
.orElse(null);
assertNotNull(secondBlock);
assertEquals("streaming_idx_0", secondBlock.getId());
assertEquals("get_weather", secondBlock.getName());
}

@Test
@DisplayName("Should parse chunk with reasoning content")
void testChunkWithReasoningContent() {
Expand Down
Loading