From b1b3069cd13271eb8c11b057975fe1069270c0d2 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Wed, 24 Jun 2026 11:55:40 -0300 Subject: [PATCH] fix(translator): regroup parallel tool results adjacent to their assistant (#4714) (#4882) Integrated into release/v3.8.36 (fixes #4714) --- .../translator/request/claude-to-openai.ts | 93 ++++++++++---- ...-claude-to-openai-orphan-tool-4385.test.ts | 115 +++++++++++++++++- 2 files changed, 179 insertions(+), 29 deletions(-) diff --git a/open-sse/translator/request/claude-to-openai.ts b/open-sse/translator/request/claude-to-openai.ts index 013fd77a4d..b6ff556316 100644 --- a/open-sse/translator/request/claude-to-openai.ts +++ b/open-sse/translator/request/claude-to-openai.ts @@ -55,7 +55,9 @@ function convertClaudeServerWebSearchTool(tool: JsonRecord): JsonRecord { return { type: "web_search", ...(Object.keys(filters).length > 0 ? { filters } : {}), - ...(tool.user_location && typeof tool.user_location === "object" && !Array.isArray(tool.user_location) + ...(tool.user_location && + typeof tool.user_location === "object" && + !Array.isArray(tool.user_location) ? { user_location: tool.user_location } : {}), }; @@ -133,32 +135,15 @@ export function claudeToOpenAIRequest(model, body, stream, credentials: unknown } } - // Fix missing tool responses - OpenAI requires every tool_call to have a response - fixMissingToolResponses(result.messages); + // #4714 / #4385: re-group every role:"tool" message immediately after the + // assistant turn whose tool_calls issued it (dropping genuine orphans), then + // fill placeholders for any tool_call left unanswered. + result.messages = regroupToolMessages(result.messages); - // #4385: drop orphan tool results — a role:"tool" message whose tool_call_id has no - // matching assistant.tool_calls (e.g. history truncation / compression removed the - // assistant turn that issued the call but kept the tool_result). OpenAI-compatible - // upstreams reject these with 502 "Messages with role 'tool' must be a response to a - // preceding message with 'tool_calls'". Mirrors the filter already applied on the - // Responses->Chat path in openai-responses.ts (#2893). Run after fixMissingToolResponses - // so the inserted "[No response received]" placeholders (which DO match a tool_call) - // are kept, and only genuinely orphaned results are removed. - const assistantToolCallIds = new Set(); - for (const msg of result.messages) { - const calls = (msg as JsonRecord).tool_calls; - if (Array.isArray(calls)) { - for (const tc of calls as { id?: string }[]) { - if (tc.id) assistantToolCallIds.add(String(tc.id)); - } - } - } - result.messages = result.messages.filter((msg) => { - if ((msg as JsonRecord).role === "tool") { - return assistantToolCallIds.has(String((msg as JsonRecord).tool_call_id ?? "")); - } - return true; - }); + // Fix missing tool responses - OpenAI requires every tool_call to have a response. + // Runs after regrouping so real results are already adjacent and only a truly + // unanswered tool_call receives a "[No response received]" placeholder. + fixMissingToolResponses(result.messages); const useNativeResponsesWebSearch = shouldUseNativeResponsesWebSearch(credentials); @@ -223,6 +208,62 @@ export function claudeToOpenAIRequest(model, body, stream, credentials: unknown return result; } +// #4714: Re-group tool result messages so every role:"tool" message sits +// immediately after the assistant message whose tool_calls issued it, in +// tool_calls order. Claude Code can issue parallel tool_use in one assistant turn +// but have their tool_result blocks arrive across SEPARATE user turns with +// interleaved text; the naive conversion then left a role:"tool" stranded after a +// user message, which OpenAI-compatible upstreams reject with 400 "Messages with +// role 'tool' must be a response to a preceding message with 'tool_calls'". +// Tool messages whose tool_call_id matches no assistant.tool_calls are dropped here +// (supersedes the standalone #4385 orphan filter — same drop behavior, plus ordering). +function regroupToolMessages(messages: JsonRecord[]): JsonRecord[] { + // tool_call_id -> index of the assistant message that issued it (first wins). + const callIdToAssistant = new Map(); + messages.forEach((msg, idx) => { + if (msg.role === "assistant" && Array.isArray(msg.tool_calls)) { + for (const tc of msg.tool_calls as { id?: string }[]) { + if (tc.id && !callIdToAssistant.has(String(tc.id))) { + callIdToAssistant.set(String(tc.id), idx); + } + } + } + }); + + // Collect tool messages per assistant index, keyed by tool_call_id (first result wins). + const toolsByAssistant = new Map>(); + for (const msg of messages) { + if (msg.role !== "tool") continue; + const callId = String(msg.tool_call_id ?? ""); + const assistantIdx = callIdToAssistant.get(callId); + if (assistantIdx === undefined) continue; // orphan -> drop + let group = toolsByAssistant.get(assistantIdx); + if (!group) { + group = new Map(); + toolsByAssistant.set(assistantIdx, group); + } + if (!group.has(callId)) group.set(callId, msg); + } + + // Rebuild: keep non-tool messages in order; attach each assistant's tool results + // immediately after it, ordered by the assistant's own tool_calls sequence. + const out: JsonRecord[] = []; + messages.forEach((msg, idx) => { + if (msg.role === "tool") return; // moved into its assistant's group + out.push(msg); + if (msg.role === "assistant" && Array.isArray(msg.tool_calls)) { + const group = toolsByAssistant.get(idx); + if (group) { + for (const tc of msg.tool_calls as { id?: string }[]) { + const tool = tc.id ? group.get(String(tc.id)) : undefined; + if (tool) out.push(tool); + } + } + } + }); + return out; +} + // Fix missing tool responses - add empty responses for tool_calls without responses function fixMissingToolResponses(messages) { for (let i = 0; i < messages.length; i++) { diff --git a/tests/unit/translator-claude-to-openai-orphan-tool-4385.test.ts b/tests/unit/translator-claude-to-openai-orphan-tool-4385.test.ts index 67ac2857af..0f7a492eef 100644 --- a/tests/unit/translator-claude-to-openai-orphan-tool-4385.test.ts +++ b/tests/unit/translator-claude-to-openai-orphan-tool-4385.test.ts @@ -10,9 +10,8 @@ import assert from "node:assert/strict"; // the assistant turn but kept the tool_result). OpenAI-compatible upstreams reject it. // This mirrors the orphan filter already on the Responses->Chat path (#2893). -const { claudeToOpenAIRequest } = await import( - "../../open-sse/translator/request/claude-to-openai.ts" -); +const { claudeToOpenAIRequest } = + await import("../../open-sse/translator/request/claude-to-openai.ts"); type Msg = { role: string; tool_call_id?: string; tool_calls?: { id?: string }[] }; @@ -92,3 +91,113 @@ test("#4385 keeps valid tool_results and drops orphans in the same user turn", ( assert.equal(toolMsgs.length, 1); assert.equal(toolMsgs[0].tool_call_id, "tu_valid"); }); + +// #4714: regression (v3.8.26+) — Claude Code issues parallel tool_use in one +// assistant turn but their tool_result blocks arrive across SEPARATE user turns +// with interleaved text. The translator emitted each role:"tool" message in the +// position of its originating user turn, so a later tool_result ended up AFTER a +// user message instead of adjacent to its assistant.tool_calls. The #4385 orphan +// filter kept it (the id DID match an assistant tool_call) but never re-ordered +// it, so OpenAI-compatible upstreams (deepseek, etc.) still rejected the request +// with 400 "Messages with role 'tool' must be a response to a preceding message +// with 'tool_calls'". fixMissingToolResponses also injected a bogus +// "[No response received]" placeholder for the not-yet-adjacent tool_call. + +// Helper: every role:"tool" must be immediately preceded by an assistant carrying +// the matching tool_call id (or another tool message in the same group). +function assertToolOrdering(messages: Msg[]) { + let activeIds: Set | null = null; + for (const m of messages) { + if (m.role === "assistant" && Array.isArray(m.tool_calls)) { + activeIds = new Set(m.tool_calls.map((t) => String(t.id))); + } else if (m.role === "tool") { + assert.ok( + activeIds && activeIds.has(String(m.tool_call_id)), + `role:'tool' ${m.tool_call_id} is not preceded by a matching assistant tool_calls` + ); + } else { + activeIds = null; + } + } +} + +test("#4714 regroups parallel tool_results split across user turns next to their assistant", () => { + const result = claudeToOpenAIRequest( + "deepseek/deepseek-v4-flash-free", + { + messages: [ + { role: "user", content: "do two things" }, + { + role: "assistant", + content: [ + { type: "text", text: "calling tools" }, + { type: "tool_use", id: "callA", name: "toolA", input: { x: 1 } }, + { type: "tool_use", id: "callB", name: "toolB", input: { y: 2 } }, + ], + }, + { + role: "user", + content: [ + { type: "tool_result", tool_use_id: "callA", content: "result A" }, + { type: "text", text: "and here's more context" }, + ], + }, + { + role: "user", + content: [{ type: "tool_result", tool_use_id: "callB", content: "result B" }], + }, + ], + }, + false + ); + + const messages = result.messages as Msg[]; + // No upstream-rejecting ordering violations. + assertToolOrdering(messages); + + // Both real tool results survive, adjacent to the assistant, in tool_calls order. + const assistantIdx = messages.findIndex( + (m) => m.role === "assistant" && Array.isArray(m.tool_calls) + ); + assert.equal((messages[assistantIdx + 1] as Msg).tool_call_id, "callA"); + assert.equal((messages[assistantIdx + 2] as Msg).tool_call_id, "callB"); + + // The real "result B" must NOT have been replaced by a placeholder. + const toolB = messages.find((m) => m.role === "tool" && m.tool_call_id === "callB") as + | (Msg & { content?: string }) + | undefined; + assert.ok(toolB); + assert.notEqual(toolB.content, "[No response received]"); + // Exactly one tool message per call id (no duplicate placeholder + real result). + assert.equal(messages.filter((m) => m.role === "tool" && m.tool_call_id === "callB").length, 1); +}); + +test("#4714 still inserts a placeholder for a genuinely unanswered parallel tool_call", () => { + const result = claudeToOpenAIRequest( + "deepseek/deepseek-v4-flash-free", + { + messages: [ + { + role: "assistant", + content: [ + { type: "tool_use", id: "ansA", name: "toolA", input: {} }, + { type: "tool_use", id: "noAns", name: "toolB", input: {} }, + ], + }, + { + role: "user", + content: [{ type: "tool_result", tool_use_id: "ansA", content: "ok" }], + }, + ], + }, + false + ); + + const messages = result.messages as Msg[]; + assertToolOrdering(messages); + const placeholder = messages.find((m) => m.role === "tool" && m.tool_call_id === "noAns") as + | (Msg & { content?: string }) + | undefined; + assert.ok(placeholder, "missing tool_call must still get a placeholder response"); + assert.equal(placeholder.content, "[No response received]"); +});