Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 15 additions & 4 deletions src/core/assistant-message/NativeToolCallParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,16 +166,27 @@ export class NativeToolCallParser {
/**
* Process stream finish reason.
* Emits end events when finish_reason is 'tool_calls'.
* Only emits events for tool calls that have been properly started (have a name).
*/
public static processFinishReason(finishReason: string | null | undefined): ToolCallStreamEvent[] {
const events: ToolCallStreamEvent[] = []

if (finishReason === "tool_calls" && this.rawChunkTracker.size > 0) {
for (const [, tracked] of this.rawChunkTracker.entries()) {
events.push({
type: "tool_call_end",
id: tracked.id,
})
// Only emit end event if the tool call was properly started
// (i.e., we received a name and emitted tool_call_start)
if (tracked.hasStarted) {
events.push({
type: "tool_call_end",
id: tracked.id,
})
} else {
// Log diagnostic warning for unstarted tool calls
console.warn(
`[NativeToolCallParser] Skipping tool_call_end for unstarted tool call: ${tracked.id} ` +
`(no name received, hasStarted=false)`,
)
}
}
}

Expand Down
118 changes: 118 additions & 0 deletions src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -343,4 +343,122 @@ describe("NativeToolCallParser", () => {
})
})
})

describe("processFinishReason", () => {
describe("hasStarted check", () => {
it("should emit tool_call_end for started tool calls", () => {
// Simulate a tool call that has been properly started
NativeToolCallParser.processRawChunk({
index: 0,
id: "call_started",
name: "read_file",
arguments: '{"path":"test.ts"}',
})

const events = NativeToolCallParser.processFinishReason("tool_calls")

expect(events).toHaveLength(1)
expect(events[0]).toEqual({
type: "tool_call_end",
id: "call_started",
})
})

it("should NOT emit tool_call_end for unstarted tool calls", () => {
// Simulate a tool call that received an ID but no name (not started)
NativeToolCallParser.processRawChunk({
index: 0,
id: "call_unstarted",
// No name provided - tool call is tracked but not started
})

const events = NativeToolCallParser.processFinishReason("tool_calls")

// Should not emit any events because the tool call wasn't started
expect(events).toHaveLength(0)
})

it("should handle mixed started and unstarted tool calls", () => {
// First tool call: properly started
NativeToolCallParser.processRawChunk({
index: 0,
id: "call_started_1",
name: "read_file",
arguments: '{"path":"test1.ts"}',
})

// Second tool call: tracked but not started (no name)
NativeToolCallParser.processRawChunk({
index: 1,
id: "call_unstarted",
// No name - won't be started
})

// Third tool call: properly started
NativeToolCallParser.processRawChunk({
index: 2,
id: "call_started_2",
name: "write_to_file",
arguments: '{"path":"output.ts"}',
})

const events = NativeToolCallParser.processFinishReason("tool_calls")

// Should only emit end events for the two started tool calls
expect(events).toHaveLength(2)
expect(events[0]).toEqual({
type: "tool_call_end",
id: "call_started_1",
})
expect(events[1]).toEqual({
type: "tool_call_end",
id: "call_started_2",
})
})

it("should not emit events when finish_reason is not tool_calls", () => {
// Set up a started tool call
NativeToolCallParser.processRawChunk({
index: 0,
id: "call_started",
name: "read_file",
arguments: '{"path":"test.ts"}',
})

// Process with different finish reason
const events = NativeToolCallParser.processFinishReason("stop")

expect(events).toHaveLength(0)
})

it("should handle tool call that receives name in a separate chunk", () => {
// First chunk: ID only
NativeToolCallParser.processRawChunk({
index: 0,
id: "call_delayed_name",
})

// At this point, tool call is tracked but not started
let events = NativeToolCallParser.processFinishReason("tool_calls")
expect(events).toHaveLength(0)

// Clear state and try again with name
NativeToolCallParser.clearRawChunkState()

// Simulate proper sequence with name
NativeToolCallParser.processRawChunk({
index: 0,
id: "call_delayed_name",
name: "read_file",
})

events = NativeToolCallParser.processFinishReason("tool_calls")
expect(events).toHaveLength(1)
expect(events[0]).toEqual({
type: "tool_call_end",
id: "call_delayed_name",
})
})
Comment on lines +434 to +461
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test doesn't actually test delayed name arrival. The clearRawChunkState() call on line 446 resets all state, so the second phase tests a completely independent scenario rather than a continuation. To properly test delayed name arrival, remove the clear and send two chunks in sequence:

Suggested change
it("should handle tool call that receives name in a separate chunk", () => {
// First chunk: ID only
NativeToolCallParser.processRawChunk({
index: 0,
id: "call_delayed_name",
})
// At this point, tool call is tracked but not started
let events = NativeToolCallParser.processFinishReason("tool_calls")
expect(events).toHaveLength(0)
// Clear state and try again with name
NativeToolCallParser.clearRawChunkState()
// Simulate proper sequence with name
NativeToolCallParser.processRawChunk({
index: 0,
id: "call_delayed_name",
name: "read_file",
})
events = NativeToolCallParser.processFinishReason("tool_calls")
expect(events).toHaveLength(1)
expect(events[0]).toEqual({
type: "tool_call_end",
id: "call_delayed_name",
})
})
it("should handle tool call that receives name in a separate chunk", () => {
// First chunk: ID only
NativeToolCallParser.processRawChunk({
index: 0,
id: "call_delayed_name",
})
// Second chunk: name arrives for the same index
NativeToolCallParser.processRawChunk({
index: 0,
name: "read_file",
})
// Now the tool call should be started and receive end event
const events = NativeToolCallParser.processFinishReason("tool_calls")
expect(events).toHaveLength(1)
expect(events[0]).toEqual({
type: "tool_call_end",
id: "call_delayed_name",
})
})

Fix it with Roo Code or mention @roomote and request a fix.

})
})
})
Loading