Skip to content

fix(chat-completions): emit tool call arguments once #363

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

Merged
lidge-jun merged 1 commit into from
Jul 24, 2026
Merged

fix(chat-completions): emit tool call arguments once #363

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
139 changes: 82 additions & 57 deletions src/chat/outbound.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,10 @@ export function responsesSseToChatCompletionsSse(
// tool call_id -> streaming index (OpenAI requires stable indices per tool call)
const toolIndexByCallId = new Map<string, number>();
const toolIndexByItemId = new Map<string, number>();
const toolCallIdByIndex = new Map<number, string>();
const toolNameByIndex = new Map<number, string>();
const toolArgumentsByIndex = new Map<number, string>();
const emittedToolIndexes = new Set<number>();
let nextToolIndex = 0;
let sseIterator: AsyncGenerator<{ event?: string; data: string }> | undefined;
const upstreamAbort = new AbortController();
Expand Down Expand Up @@ -166,8 +169,47 @@ export function responsesSseToChatCompletionsSse(
frame.choices = [{ index: 0, delta: { reasoning_content: text }, finish_reason: null }];
emit(frame);
};
const resolveFinalArguments = (candidate: unknown, streamed: string) => {
if (typeof candidate !== "string") return streamed;
// Some compatible providers send an empty final snapshot. Do not let that erase
// non-empty streamed arguments; genuinely empty calls have no buffered content.
return candidate.length > 0 || streamed.length === 0 ? candidate : streamed;
};
const emitToolCall = (toolIndex: number, callId: string, name: string, args: string) => {
if (!callId || emittedToolIndexes.has(toolIndex)) return;
emittedToolIndexes.add(toolIndex);
ensureRole();
const frame = chunkBase(id, model, created);
frame.choices = [{
index: 0,
delta: {
tool_calls: [{
index: toolIndex,
id: callId,
type: "function",
function: { name, arguments: args },
}],
},
finish_reason: null,
}];
emit(frame);
};
const flushPendingToolCalls = () => {
const pending = [...toolCallIdByIndex.entries()].sort(([a], [b]) => a - b);
for (const [toolIndex, callId] of pending) {
emitToolCall(
toolIndex,
callId,
toolNameByIndex.get(toolIndex) ?? "",
toolArgumentsByIndex.get(toolIndex) ?? "",
);
}
};
const finish = (finishReason: string, usage: unknown) => {
if (terminated) return;
// A valid completed/incomplete terminal frame may arrive without output_item.done.
// Preserve any known tool call before emitting its finish reason.
flushPendingToolCalls();
terminated = true;
ensureRole();
const frame = chunkBase(id, model, created);
Expand Down Expand Up @@ -232,24 +274,15 @@ export function responsesSseToChatCompletionsSse(
toolIndex = nextToolIndex++;
toolIndexByCallId.set(callId, toolIndex);
}
toolCallIdByIndex.set(toolIndex, callId);
if (typeof item.id === "string") toolIndexByItemId.set(item.id, toolIndex);
if (name) toolNameByIndex.set(toolIndex, name);
const frame = chunkBase(id, model, created);
frame.choices = [{
index: 0,
delta: {
tool_calls: [{
index: toolIndex,
id: callId,
type: "function",
// Always include name so clients that replace (not merge) tool_calls
// deltas never end up with function.name-less history.
function: { name, arguments: "" },
}],
},
finish_reason: null,
}];
emit(frame);
if (!toolArgumentsByIndex.has(toolIndex)) {
toolArgumentsByIndex.set(
toolIndex,
typeof item.arguments === "string" ? item.arguments : "",
);
}
Comment on lines +277 to +285

@coderabbitai coderabbitai Bot Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract a shared resolveToolIndex helper to dedupe index resolution.

response.output_item.added (Lines 272-279) and response.output_item.done (Lines 322-328) both re-implement "look up toolIndex by callId, fall back to itemId, else allocate new, then keep all three maps in sync." The two implementations are subtly different (added only checks the callId map before allocating; done checks both) which makes them easy to accidentally diverge the next time either handler is touched — exactly the kind of drift this PR's fix is designed to prevent.

♻️ Proposed refactor 
+      const resolveToolIndex = (callId: string, itemId: string | undefined): number => {
+        const existing = toolIndexByCallId.get(callId)
+          ?? (itemId ? toolIndexByItemId.get(itemId) : undefined);
+        const toolIndex = existing ?? nextToolIndex++;
+        toolIndexByCallId.set(callId, toolIndex);
+        toolCallIdByIndex.set(toolIndex, callId);
+        if (itemId) toolIndexByItemId.set(itemId, toolIndex);
+        return toolIndex;
+      };

In response.output_item.added:

-            let toolIndex = toolIndexByCallId.get(callId);
-            if (toolIndex === undefined) {
-              toolIndex = nextToolIndex++;
-              toolIndexByCallId.set(callId, toolIndex);
-            }
-            toolCallIdByIndex.set(toolIndex, callId);
-            if (typeof item.id === "string") toolIndexByItemId.set(item.id, toolIndex);
+            const toolIndex = resolveToolIndex(callId, typeof item.id === "string" ? item.id : undefined);

In response.output_item.done:

-              const itemId = typeof item.id === "string" ? item.id : undefined;
-              const existingIndex = toolIndexByCallId.get(callId)
-                ?? (itemId ? toolIndexByItemId.get(itemId) : undefined);
-              const toolIndex = existingIndex ?? nextToolIndex++;
-              toolIndexByCallId.set(callId, toolIndex);
-              toolCallIdByIndex.set(toolIndex, callId);
-              if (itemId) toolIndexByItemId.set(itemId, toolIndex);
+              const toolIndex = resolveToolIndex(callId, typeof item.id === "string" ? item.id : undefined);

Also applies to: 321-339

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/chat/outbound.ts` around lines 277 - 285, Extract a shared
resolveToolIndex helper for the response.output_item.added and
response.output_item.done handlers in outbound.ts. Centralize the lookup order
of callId, itemId, and new index allocation, and ensure the helper keeps
toolCallIdByIndex, toolIndexByItemId, and related index maps synchronized;
replace both duplicated resolution implementations with this helper while
preserving each handler’s existing argument and name updates.

break;
}
case "response.function_call_arguments.delta": {
Expand All @@ -258,23 +291,24 @@ export function responsesSseToChatCompletionsSse(
const toolIndex = (itemId ? toolIndexByItemId.get(itemId) : undefined)
?? (nextToolIndex > 0 ? nextToolIndex - 1 : 0);
ensureRole();
const knownName = toolNameByIndex.get(toolIndex) ?? "";
const fn: Rec = { arguments: data.delta };
// Re-emit name on every args delta: GitHub Copilot App / some ChatGPT clients
// replace the whole function object instead of merging, which otherwise drops name.
if (knownName) fn.name = knownName;
const frame = chunkBase(id, model, created);
frame.choices = [{
index: 0,
delta: {
tool_calls: [{
index: toolIndex,
function: fn,
}],
},
finish_reason: null,
}];
emit(frame);
toolArgumentsByIndex.set(
toolIndex,
(toolArgumentsByIndex.get(toolIndex) ?? "") + data.delta,
);
break;
}
case "response.function_call_arguments.done": {
const itemId = typeof data.item_id === "string" ? data.item_id : undefined;
const toolIndex = itemId ? toolIndexByItemId.get(itemId) : undefined;
if (toolIndex === undefined) break;
sawToolUse = true;
const name = typeof data.name === "string" ? data.name : "";
if (name) toolNameByIndex.set(toolIndex, name);
const streamedArgs = toolArgumentsByIndex.get(toolIndex) ?? "";
toolArgumentsByIndex.set(
toolIndex,
resolveFinalArguments(data.arguments, streamedArgs),
);
break;
}
case "response.output_item.done": {
Expand All @@ -284,34 +318,25 @@ export function responsesSseToChatCompletionsSse(
sawToolUse = true;
const callId = typeof item.call_id === "string" ? item.call_id : "";
const name = typeof item.name === "string" ? item.name : "";
const args = typeof item.arguments === "string" ? item.arguments : "";
if (!callId) break;
const existingIndex = toolIndexByCallId.get(callId);
const isNew = existingIndex === undefined;
const itemId = typeof item.id === "string" ? item.id : undefined;
const existingIndex = toolIndexByCallId.get(callId)
?? (itemId ? toolIndexByItemId.get(itemId) : undefined);
const toolIndex = existingIndex ?? nextToolIndex++;
if (isNew) toolIndexByCallId.set(callId, toolIndex);
if (typeof item.id === "string") toolIndexByItemId.set(item.id, toolIndex);
toolIndexByCallId.set(callId, toolIndex);
toolCallIdByIndex.set(toolIndex, callId);
if (itemId) toolIndexByItemId.set(itemId, toolIndex);
if (name) toolNameByIndex.set(toolIndex, name);
const finalName = name || toolNameByIndex.get(toolIndex) || "";
// Last-write-wins: the done-frame snapshot is authoritative final arguments.
// Always re-emit full identity (id/type/name) so replace-style clients keep function.name.
if (args.length > 0 || isNew || finalName) {
ensureRole();
const frame = chunkBase(id, model, created);
frame.choices = [{
index: 0,
delta: {
tool_calls: [{
index: toolIndex,
id: callId,
type: "function",
function: { name: finalName, arguments: args },
}],
},
finish_reason: null,
}];
emit(frame);
}
const streamedArgs = toolArgumentsByIndex.get(toolIndex) ?? "";
// A Responses stream can carry incremental/finalized argument events plus an
// authoritative output_item.done snapshot. Chat Completions tool-call fields are
// append-only deltas, so forwarding multiple representations corrupts clients that
// accumulate them. Emit one complete call here; finish() flushes any item whose
// done event was omitted, and replace-style clients still get a complete object.
const args = resolveFinalArguments(item.arguments, streamedArgs);
toolArgumentsByIndex.set(toolIndex, args);
emitToolCall(toolIndex, callId, finalName, args);
}
break;
}
Expand Down
Loading
Loading