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.
This commit is contained in:
@@ -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<string, string[]>();
|
||||
let generatedToolCallIdCounter = 0;
|
||||
|
||||
@@ -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<string, number>();
|
||||
const pendingToolResultRewrites = new Map<string, PendingToolResultRewrite[]>();
|
||||
|
||||
@@ -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<TApi extends Api>(
|
||||
messages: Message[],
|
||||
model: Model<TApi>,
|
||||
normalizeToolCallId?: (id: string, model: Model<TApi>, 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<string, string>();
|
||||
@@ -255,6 +269,8 @@ export function transformMessages<TApi extends Api>(
|
||||
}
|
||||
return msg;
|
||||
}),
|
||||
maxNormalizedToolCallIdLength,
|
||||
duplicateToolCallIdSuffixPrefix,
|
||||
);
|
||||
const realToolResultsById = new Map<string, ToolResultMessage>();
|
||||
for (const msg of transformed) {
|
||||
|
||||
@@ -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]);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user