From b2e7cd263410ddf863decbc2bcff4bfc6533b0a9 Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 8 Jun 2026 01:09:47 +0200 Subject: [PATCH] fix(packages/ai): resolved tool-call duplicate IDs for OpenAI/Mistral - Adjusted deduplicateToolCallIds in transform-messages.ts to enforce max tool-call ID length. - Updated convertMessages in openai-completions.ts to pass provider-specific ID limits and suffix settings. - Added regression coverage for duplicate tool IDs on OpenAI and Mistral truncation paths. --- .../ai/src/providers/openai-completions.ts | 14 +++- .../ai/src/providers/transform-messages.ts | 28 +++++-- .../ai/test/duplicate-tool-results.test.ts | 75 +++++++++++++++++++ 3 files changed, 110 insertions(+), 7 deletions(-) diff --git a/packages/ai/src/providers/openai-completions.ts b/packages/ai/src/providers/openai-completions.ts index 4816e492b..ca8a78d33 100644 --- a/packages/ai/src/providers/openai-completions.ts +++ b/packages/ai/src/providers/openai-completions.ts @@ -1476,6 +1476,12 @@ export function convertMessages( ): ChatCompletionMessageParam[] { const params: ChatCompletionMessageParam[] = []; + const maxNormalizedToolCallIdLength = compat.requiresMistralToolIds + ? 9 + : model.provider === "openai" + ? 40 + : undefined; + const duplicateToolCallIdSuffixPrefix = compat.requiresMistralToolIds ? "dup" : undefined; const normalizeToolCallId = (id: string): string => { if (compat.requiresMistralToolIds) return normalizeMistralToolId(id, true); @@ -1492,7 +1498,13 @@ export function convertMessages( if (model.provider === "openai") return id.length > 40 ? id.slice(0, 40) : id; return id; }; - const transformedMessages = transformMessages(context.messages, model, id => normalizeToolCallId(id)); + const transformedMessages = transformMessages( + context.messages, + model, + id => normalizeToolCallId(id), + maxNormalizedToolCallIdLength, + duplicateToolCallIdSuffixPrefix, + ); const remappedToolCallIds = new Map(); let generatedToolCallIdCounter = 0; diff --git a/packages/ai/src/providers/transform-messages.ts b/packages/ai/src/providers/transform-messages.ts index 096b454e7..8d1477b27 100644 --- a/packages/ai/src/providers/transform-messages.ts +++ b/packages/ai/src/providers/transform-messages.ts @@ -28,15 +28,19 @@ const enum ToolCallStatus { */ const MAX_TOOL_CALL_ID_LENGTH = 64; -function appendDuplicateSuffix(originalId: string, suffix: string): string { - if (originalId.length + suffix.length <= MAX_TOOL_CALL_ID_LENGTH) return `${originalId}${suffix}`; - const prefixBudget = Math.max(0, MAX_TOOL_CALL_ID_LENGTH - suffix.length); +function appendDuplicateSuffix(originalId: string, suffix: string, maxLength: number): string { + if (originalId.length + suffix.length <= maxLength) return `${originalId}${suffix}`; + const prefixBudget = Math.max(0, maxLength - suffix.length); return `${originalId.slice(0, prefixBudget)}${suffix}`; } type PendingToolResultRewrite = { replacementId: string } | undefined; -function deduplicateToolCallIds(messages: Message[]): Message[] { +function deduplicateToolCallIds( + messages: Message[], + maxToolCallIdLength = MAX_TOOL_CALL_ID_LENGTH, + duplicateSuffixPrefix = "_dup", +): Message[] { const seenToolCallIds = new Map(); const pendingToolResultRewrites = new Map(); @@ -90,10 +94,18 @@ function deduplicateToolCallIds(messages: Message[]): Message[] { } let duplicateIndex = previousCount; - let replacementId = appendDuplicateSuffix(block.id, `_dup${duplicateIndex}`); + let replacementId = appendDuplicateSuffix( + block.id, + `${duplicateSuffixPrefix}${duplicateIndex}`, + maxToolCallIdLength, + ); while (seenToolCallIds.has(replacementId)) { duplicateIndex += 1; - replacementId = appendDuplicateSuffix(block.id, `_dup${duplicateIndex}`); + replacementId = appendDuplicateSuffix( + block.id, + `${duplicateSuffixPrefix}${duplicateIndex}`, + maxToolCallIdLength, + ); } seenToolCallIds.set(block.id, duplicateIndex + 1); seenToolCallIds.set(replacementId, 1); @@ -136,6 +148,8 @@ export function transformMessages( messages: Message[], model: Model, normalizeToolCallId?: (id: string, model: Model, source: AssistantMessage) => string, + maxNormalizedToolCallIdLength = MAX_TOOL_CALL_ID_LENGTH, + duplicateToolCallIdSuffixPrefix = "_dup", ): Message[] { // Build a map of original tool call IDs to normalized IDs const toolCallIdMap = new Map(); @@ -255,6 +269,8 @@ export function transformMessages( } return msg; }), + maxNormalizedToolCallIdLength, + duplicateToolCallIdSuffixPrefix, ); const realToolResultsById = new Map(); for (const msg of transformed) { diff --git a/packages/ai/test/duplicate-tool-results.test.ts b/packages/ai/test/duplicate-tool-results.test.ts index 46f34ee2c..0d8428245 100644 --- a/packages/ai/test/duplicate-tool-results.test.ts +++ b/packages/ai/test/duplicate-tool-results.test.ts @@ -1,8 +1,10 @@ import { describe, expect, it } from "bun:test"; +import { convertMessages, detectCompat } from "@oh-my-pi/pi-ai/providers/openai-completions"; import { transformMessages } from "@oh-my-pi/pi-ai/providers/transform-messages"; import type { Api, AssistantMessage, + Context, DeveloperMessage, Message, Model, @@ -10,6 +12,11 @@ import type { ToolResultMessage, UserMessage, } from "@oh-my-pi/pi-ai/types"; +import type { + ChatCompletionAssistantMessageParam, + ChatCompletionMessageParam, + ChatCompletionToolMessageParam, +} from "openai/resources/chat/completions"; /** * Regression test for: "each tool_use must have a single result. Found multiple tool_result blocks with id" @@ -491,6 +498,74 @@ describe("Duplicate Tool Results Regression", () => { { type: "text", text: "second" }, ]); }); + + it("keeps duplicate ids distinct after OpenAI completions provider caps", () => { + const assistantWireMessages = (messages: ChatCompletionMessageParam[]): ChatCompletionAssistantMessageParam[] => + messages.filter( + (message): message is ChatCompletionAssistantMessageParam => + message.role === "assistant" && Array.isArray(message.tool_calls), + ); + const toolWireIds = (messages: ChatCompletionMessageParam[]): string[] => + messages + .filter((message): message is ChatCompletionToolMessageParam => message.role === "tool") + .map(message => message.tool_call_id); + + const cases: Array<{ + model: Model<"openai-completions">; + duplicateId: string; + expectedDuplicateId: string; + }> = [ + { + model: { + api: "openai-completions", + provider: "openai", + id: "gpt-4o-mini", + name: "GPT-4o Mini", + baseUrl: "https://api.openai.com/v1", + input: ["text"], + cost: { input: 1, output: 1, cacheRead: 0, cacheWrite: 0 }, + maxTokens: 8192, + contextWindow: 128000, + reasoning: false, + }, + duplicateId: `call_${"a".repeat(35)}`, + expectedDuplicateId: `${`call_${"a".repeat(35)}`.slice(0, 35)}_dup1`, + }, + { + model: { + api: "openai-completions", + provider: "mistral", + id: "mistral-large-latest", + name: "Mistral Large", + baseUrl: "https://api.mistral.ai/v1", + input: ["text"], + cost: { input: 1, output: 1, cacheRead: 0, cacheWrite: 0 }, + maxTokens: 8192, + contextWindow: 128000, + reasoning: false, + }, + duplicateId: "ABCDEF123", + expectedDuplicateId: "ABCDEdup1", + }, + ]; + + for (const { model: providerModel, duplicateId, expectedDuplicateId } of cases) { + const messages: Message[] = [ + makeEvalAssistantMessage(duplicateId, 1), + makeEvalToolResult(duplicateId, "first", 2), + makeEvalAssistantMessage(duplicateId, 3), + makeEvalToolResult(duplicateId, "second", 4), + ]; + const context: Context = { messages }; + const wireMessages = convertMessages(providerModel, context, detectCompat(providerModel)); + const assistantIds = assistantWireMessages(wireMessages).flatMap(message => + message.tool_calls.map(toolCall => toolCall.id), + ); + + expect(assistantIds, providerModel.provider).toEqual([duplicateId, expectedDuplicateId]); + expect(toolWireIds(wireMessages), providerModel.provider).toEqual([duplicateId, expectedDuplicateId]); + } + }); }); /**