Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 5 additions & 0 deletions .changeset/fix-toolcall-dropped-leading-deltas.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"zoo-code": patch
---

Fix streaming tool-call arguments being dropped when argument deltas arrive before the tool-call id, which caused spurious "missing required parameter" errors (affects LiteLLM, OpenAI-compatible, and DeepSeek providers).
39 changes: 39 additions & 0 deletions prs/fix-toolcall-dropped-leading-deltas.md
Comment thread
awschmeder marked this conversation as resolved.
Outdated
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
### Related GitHub Issue

Closes: #695

### Description

When a provider streams a tool call whose first delta(s) arrive *before* the tool-call `id` is known, those leading argument bytes are silently discarded by `NativeToolCallParser.processRawChunk`. This causes downstream "missing required parameter" errors even when the model supplied the data.

This PR fixes the issue by centralizing the tracking of streaming tool calls in `NativeToolCallParser`. The `rawChunkTracker` is now initialized on the first sight of a stream `index`, independent of whether an `id` is present. All `arguments` deltas are buffered until both `id` and `name` are known, ensuring no data loss during streaming reassembly.

### Test Procedure

1. Ran the newly added unit test in `src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts` which verifies that leading argument bytes arriving before the `id` are correctly preserved and finalized.
2. Verified that existing provider tests in the same test file pass.

### Pre-Submission Checklist

- [x] **Issue Linked**: This PR is linked to an approved GitHub Issue.
- [x] **Scope**: My changes are focused on the linked issue (one major feature/fix per PR).
- [x] **Self-Review**: I have performed a thorough self-review of my code.
- [x] **Testing**: New and/or updated tests have been added to cover my changes.
- [x] **Documentation Impact**: I have considered if my changes require documentation updates.
- [x] **Contribution Guidelines**: I have read and agree to the [Contributor Guidelines](/CONTRIBUTING.md).

### Screenshots / Videos

N/A

### Documentation Updates

- [x] No documentation updates are required.

### Additional Notes

N/A

### Get in Touch

@awschmeder
47 changes: 27 additions & 20 deletions src/core/assistant-message/NativeToolCallParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export class NativeToolCallParser {
private static rawChunkTracker = new Map<
number,
{
id: string
id?: string
name: string
hasStarted: boolean
deltaBuffer: string[]
Expand Down Expand Up @@ -105,10 +105,11 @@ export class NativeToolCallParser {
const events: ToolCallStreamEvent[] = []
const { index, id, name, arguments: args } = chunk

// Create the tracker on first sight of this index, independent of whether
// an id has arrived yet. Keying the lifecycle by index (not id) ensures any
// `arguments` that stream before the id is known are buffered rather than dropped.
let tracked = this.rawChunkTracker.get(index)

// Initialize new tool call tracking when we receive an id
if (id && !tracked) {
if (!tracked) {
tracked = {
id,
name: name || "",
Expand All @@ -118,38 +119,39 @@ export class NativeToolCallParser {
this.rawChunkTracker.set(index, tracked)
}

if (!tracked) {
return events
// Record id and name as they arrive (they may come in separate chunks).
if (id) {
tracked.id = id
}

// Update name if present in chunk and not yet set
if (name) {
tracked.name = name
Comment thread
awschmeder marked this conversation as resolved.
}

// Emit start event when we have the name
if (!tracked.hasStarted && tracked.name) {
// Emit start event only once both id and name are known. Using a local
// non-null id keeps emitted events typed as id: string.
if (!tracked.hasStarted && tracked.id && tracked.name) {
const startedId = tracked.id
events.push({
type: "tool_call_start",
id: tracked.id,
id: startedId,
name: tracked.name,
})
tracked.hasStarted = true

// Flush buffered deltas
// Flush buffered deltas accumulated during the pre-start window.
for (const bufferedDelta of tracked.deltaBuffer) {
events.push({
type: "tool_call_delta",
id: tracked.id,
id: startedId,
delta: bufferedDelta,
})
}
tracked.deltaBuffer = []
}

// Emit delta event for argument chunks
// Emit delta event for argument chunks, buffering until start is emitted.
if (args) {
if (tracked.hasStarted) {
if (tracked.hasStarted && tracked.id) {
events.push({
type: "tool_call_delta",
id: tracked.id,
Expand All @@ -172,10 +174,15 @@ export class NativeToolCallParser {

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 an end for trackers that actually started. A tracker that
// never received an id/name (malformed stream) must not emit a phantom
// end; since start requires an id, hasStarted implies tracked.id is set.
if (tracked.hasStarted && tracked.id) {
Comment thread
awschmeder marked this conversation as resolved.
events.push({
type: "tool_call_end",
id: tracked.id,
})
}
}
}

Expand All @@ -191,7 +198,7 @@ export class NativeToolCallParser {

if (this.rawChunkTracker.size > 0) {
for (const [, tracked] of this.rawChunkTracker.entries()) {
if (tracked.hasStarted) {
if (tracked.hasStarted && tracked.id) {
events.push({
type: "tool_call_end",
id: tracked.id,
Expand Down
144 changes: 143 additions & 1 deletion src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { NativeToolCallParser } from "../NativeToolCallParser"
import { NativeToolCallParser, type ToolCallStreamEvent } from "../NativeToolCallParser"

describe("NativeToolCallParser", () => {
beforeEach(() => {
Expand Down Expand Up @@ -343,4 +343,146 @@ describe("NativeToolCallParser", () => {
})
})
})

describe("processRawChunk streaming reassembly", () => {
// Mirror the sequencing Task.ts performs: feed each raw chunk through
// processRawChunk, drive startStreamingToolCall on tool_call_start, feed
// tool_call_delta into processStreamingChunk, and finalize at the end.
// Returns the ordered event types/ids plus the finalized tool uses by id.
const drive = (
rawChunks: Array<{ index: number; id?: string; name?: string; arguments?: string }>,
finishReason: string | null = "tool_calls",
) => {
const events: ToolCallStreamEvent[] = []

const handleEvent = (event: ToolCallStreamEvent) => {
events.push(event)
if (event.type === "tool_call_start") {
NativeToolCallParser.startStreamingToolCall(event.id, event.name)
} else if (event.type === "tool_call_delta") {
NativeToolCallParser.processStreamingChunk(event.id, event.delta)
}
}

for (const chunk of rawChunks) {
for (const event of NativeToolCallParser.processRawChunk(chunk)) {
handleEvent(event)
}
}

// Task.ts emits ends via processFinishReason on finish_reason: "tool_calls".
// Use clearRawChunkState (not finalizeRawChunks) for cleanup so we don't
// double-count the end events both paths would produce.
for (const event of NativeToolCallParser.processFinishReason(finishReason)) {
handleEvent(event)
}
NativeToolCallParser.clearRawChunkState()

const finalized = new Map<string, ReturnType<typeof NativeToolCallParser.finalizeStreamingToolCall>>()
const startIds = events.filter((e) => e.type === "tool_call_start").map((e) => e.id)
for (const id of startIds) {
finalized.set(id, NativeToolCallParser.finalizeStreamingToolCall(id))
}

return { events, finalized }
}

it("preserves leading argument bytes that arrive before the id", () => {
// First chunk carries arguments but NO id; id+name arrive later, then more args.
const fullArgs = JSON.stringify({ path: "src/leading.ts", mode: "slice" })
const firstHalf = fullArgs.slice(0, 10)
const secondHalf = fullArgs.slice(10)

const { events, finalized } = drive([
{ index: 0, arguments: firstHalf },
{ index: 0, id: "call_late_id", name: "read_file" },
{ index: 0, arguments: secondHalf },
])

// Exactly one start, in the right order, with the late id.
const starts = events.filter((e) => e.type === "tool_call_start")
expect(starts).toHaveLength(1)
expect(starts[0].id).toBe("call_late_id")

// The finalized arguments must contain the complete, uncorrupted payload.
const result = finalized.get("call_late_id")
expect(result).not.toBeNull()
expect(result?.type).toBe("tool_use")
if (result?.type === "tool_use") {
const nativeArgs = result.nativeArgs as { path: string; mode?: string }
expect(nativeArgs.path).toBe("src/leading.ts")
expect(nativeArgs.mode).toBe("slice")
}
})

it("handles id and name arriving in separate chunks (issue #218)", () => {
Comment thread
awschmeder marked this conversation as resolved.
const fullArgs = JSON.stringify({ path: "src/split.ts" })

const { events, finalized } = drive([
{ index: 0, id: "call_split" },
{ index: 0, name: "read_file" },
{ index: 0, arguments: fullArgs },
])

const starts = events.filter((e) => e.type === "tool_call_start")
expect(starts).toHaveLength(1)
expect(starts[0].id).toBe("call_split")

const result = finalized.get("call_split")
expect(result?.type).toBe("tool_use")
if (result?.type === "tool_use") {
const nativeArgs = result.nativeArgs as { path: string }
expect(nativeArgs.path).toBe("src/split.ts")
}
})

it("keeps two parallel tool calls on distinct indices isolated", () => {
const argsA = JSON.stringify({ path: "src/a.ts" })
const argsB = JSON.stringify({ path: "src/b.ts" })

const { events, finalized } = drive([
{ index: 0, arguments: argsA.slice(0, 8) },
{ index: 1, arguments: argsB.slice(0, 8) },
{ index: 0, id: "call_a", name: "read_file" },
{ index: 1, id: "call_b", name: "read_file" },
{ index: 0, arguments: argsA.slice(8) },
{ index: 1, arguments: argsB.slice(8) },
])

const starts = events.filter((e) => e.type === "tool_call_start")
expect(starts).toHaveLength(2)

const resultA = finalized.get("call_a")
const resultB = finalized.get("call_b")
if (resultA?.type === "tool_use") {
Comment thread
awschmeder marked this conversation as resolved.
expect((resultA.nativeArgs as { path: string }).path).toBe("src/a.ts")
}
if (resultB?.type === "tool_use") {
expect((resultB.nativeArgs as { path: string }).path).toBe("src/b.ts")
}
})

it("emits the same event sequence for the single-chunk-with-id flow (regression guard)", () => {
const fullArgs = JSON.stringify({ path: "src/single.ts" })

const { events, finalized } = drive([
{ index: 0, id: "call_single", name: "read_file", arguments: fullArgs },
])

expect(events.map((e) => e.type)).toEqual(["tool_call_start", "tool_call_delta", "tool_call_end"])
expect(events.every((e) => e.id === "call_single")).toBe(true)

const result = finalized.get("call_single")
if (result?.type === "tool_use") {
Comment thread
awschmeder marked this conversation as resolved.
expect((result.nativeArgs as { path: string }).path).toBe("src/single.ts")
}
})

it("does not emit a phantom tool_call_end for a tracker that never received an id", () => {
const { events } = drive([{ index: 0, arguments: '{"path":"orphan.ts"}' }])

expect(events.filter((e) => e.type === "tool_call_start")).toHaveLength(0)
expect(events.filter((e) => e.type === "tool_call_end")).toHaveLength(0)
})
})
})
4 changes: 2 additions & 2 deletions src/core/prompts/tools/native-tools/ask_followup_question.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const ASK_FOLLOWUP_QUESTION_DESCRIPTION = `Ask the user a question to gather add

Parameters:
- question: (required) A clear, specific question addressing the information needed
- follow_up: (required) A list of 2-4 suggested answers. Suggestions must be complete, actionable answers without placeholders. Optionally include mode to switch modes (code/architect/etc.)
- follow_up: (required) An array of 1-4 suggested answers. Always provide this as an array, even when there is only one suggestion. Each suggestion must be a complete, actionable answer without placeholders. Suggestions optionally include mode to switch modes (code/architect/etc.)

Example: Asking for file path
{ "question": "What is the path to the frontend-config.json file?", "follow_up": [{ "text": "./src/frontend-config.json", "mode": null }, { "text": "./config/frontend-config.json", "mode": null }, { "text": "./frontend-config.json", "mode": null }] }
Expand All @@ -14,7 +14,7 @@ Example: Asking with mode switch

const QUESTION_PARAMETER_DESCRIPTION = `Clear, specific question that captures the missing information you need`

const FOLLOW_UP_PARAMETER_DESCRIPTION = `Required list of 2-4 suggested responses; each suggestion must be a complete, actionable answer and may include a mode switch`
const FOLLOW_UP_PARAMETER_DESCRIPTION = `Required array of 1-4 suggested responses, always an array even for a single suggestion; each suggestion must be a complete, actionable answer and may include a mode switch`
Comment thread
awschmeder marked this conversation as resolved.
Outdated

const FOLLOW_UP_TEXT_DESCRIPTION = `Suggested answer the user can pick`

Expand Down
Loading