Skip to content

Commit ccb4e78

Browse files
committed
refactor: pass normalization function instead of modelId to convertToOpenAiMessages
Addresses PR feedback about separation of concerns. The convertToOpenAiMessages function now accepts an optional normalizeToolCallId function instead of checking for 'mistral' in the modelId. This allows callers to flexibly declare the tool call ID format without the transform module needing to know about provider-specific requirements.
1 parent 72eddd7 commit ccb4e78

File tree

3 files changed

+27
-26
lines changed

3 files changed

+27
-26
lines changed

src/api/providers/openrouter.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { NativeToolCallParser } from "../../core/assistant-message/NativeToolCal
1616

1717
import type { ApiHandlerOptions, ModelRecord } from "../../shared/api"
1818

19-
import { convertToOpenAiMessages } from "../transform/openai-format"
19+
import { convertToOpenAiMessages, normalizeToolCallId } from "../transform/openai-format"
2020
import { resolveToolProtocol } from "../../utils/resolveToolProtocol"
2121
import { TOOL_PROTOCOL } from "@roo-code/types"
2222
import { ApiStreamChunk } from "../transform/stream"
@@ -226,10 +226,11 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
226226
}
227227

228228
// Convert Anthropic messages to OpenAI format.
229-
// Pass modelId to normalize tool call IDs for Mistral compatibility
229+
// Pass normalization function for Mistral compatibility (requires 9-char alphanumeric IDs)
230+
const isMistral = modelId.toLowerCase().includes("mistral")
230231
let openAiMessages: OpenAI.Chat.ChatCompletionMessageParam[] = [
231232
{ role: "system", content: systemPrompt },
232-
...convertToOpenAiMessages(messages, { modelId }),
233+
...convertToOpenAiMessages(messages, isMistral ? { normalizeToolCallId } : undefined),
233234
]
234235

235236
// DeepSeek highly recommends using user instead of system role.

src/api/transform/__tests__/openai-format.spec.ts

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ describe("convertToOpenAiMessages", () => {
107107
})
108108
})
109109

110-
it("should handle assistant messages with tool use (no normalization without modelId)", () => {
110+
it("should handle assistant messages with tool use (no normalization without normalizeToolCallId)", () => {
111111
const anthropicMessages: Anthropic.Messages.MessageParam[] = [
112112
{
113113
role: "assistant",
@@ -134,7 +134,7 @@ describe("convertToOpenAiMessages", () => {
134134
expect(assistantMessage.content).toBe("Let me check the weather.")
135135
expect(assistantMessage.tool_calls).toHaveLength(1)
136136
expect(assistantMessage.tool_calls![0]).toEqual({
137-
id: "weather-123", // Not normalized without modelId
137+
id: "weather-123", // Not normalized without normalizeToolCallId function
138138
type: "function",
139139
function: {
140140
name: "get_weather",
@@ -143,7 +143,7 @@ describe("convertToOpenAiMessages", () => {
143143
})
144144
})
145145

146-
it("should handle user messages with tool results (no normalization without modelId)", () => {
146+
it("should handle user messages with tool results (no normalization without normalizeToolCallId)", () => {
147147
const anthropicMessages: Anthropic.Messages.MessageParam[] = [
148148
{
149149
role: "user",
@@ -162,11 +162,11 @@ describe("convertToOpenAiMessages", () => {
162162

163163
const toolMessage = openAiMessages[0] as OpenAI.Chat.ChatCompletionToolMessageParam
164164
expect(toolMessage.role).toBe("tool")
165-
expect(toolMessage.tool_call_id).toBe("weather-123") // Not normalized without modelId
165+
expect(toolMessage.tool_call_id).toBe("weather-123") // Not normalized without normalizeToolCallId function
166166
expect(toolMessage.content).toBe("Current temperature in London: 20°C")
167167
})
168168

169-
it("should normalize tool call IDs when modelId contains 'mistral'", () => {
169+
it("should normalize tool call IDs when normalizeToolCallId function is provided", () => {
170170
const anthropicMessages: Anthropic.Messages.MessageParam[] = [
171171
{
172172
role: "assistant",
@@ -191,9 +191,9 @@ describe("convertToOpenAiMessages", () => {
191191
},
192192
]
193193

194-
// With Mistral model ID - should normalize
194+
// With normalizeToolCallId function - should normalize
195195
const openAiMessages = convertToOpenAiMessages(anthropicMessages, {
196-
modelId: "mistralai/mistral-large-latest",
196+
normalizeToolCallId,
197197
})
198198

199199
const assistantMessage = openAiMessages[0] as OpenAI.Chat.ChatCompletionAssistantMessageParam
@@ -203,7 +203,7 @@ describe("convertToOpenAiMessages", () => {
203203
expect(toolMessage.tool_call_id).toBe(normalizeToolCallId("call_5019f900a247472bacde0b82"))
204204
})
205205

206-
it("should not normalize tool call IDs when modelId does not contain 'mistral'", () => {
206+
it("should not normalize tool call IDs when normalizeToolCallId function is not provided", () => {
207207
const anthropicMessages: Anthropic.Messages.MessageParam[] = [
208208
{
209209
role: "assistant",
@@ -228,8 +228,8 @@ describe("convertToOpenAiMessages", () => {
228228
},
229229
]
230230

231-
// With non-Mistral model ID - should NOT normalize
232-
const openAiMessages = convertToOpenAiMessages(anthropicMessages, { modelId: "openai/gpt-4" })
231+
// Without normalizeToolCallId function - should NOT normalize
232+
const openAiMessages = convertToOpenAiMessages(anthropicMessages, {})
233233

234234
const assistantMessage = openAiMessages[0] as OpenAI.Chat.ChatCompletionAssistantMessageParam
235235
expect(assistantMessage.tool_calls![0].id).toBe("call_5019f900a247472bacde0b82")
@@ -238,7 +238,7 @@ describe("convertToOpenAiMessages", () => {
238238
expect(toolMessage.tool_call_id).toBe("call_5019f900a247472bacde0b82")
239239
})
240240

241-
it("should be case-insensitive when checking for mistral in modelId", () => {
241+
it("should use custom normalization function when provided", () => {
242242
const anthropicMessages: Anthropic.Messages.MessageParam[] = [
243243
{
244244
role: "assistant",
@@ -253,10 +253,11 @@ describe("convertToOpenAiMessages", () => {
253253
},
254254
]
255255

256-
// Uppercase MISTRAL should still trigger normalization
257-
const openAiMessages = convertToOpenAiMessages(anthropicMessages, { modelId: "MISTRAL-7B" })
256+
// Custom normalization function that prefixes with "custom_"
257+
const customNormalizer = (id: string) => `custom_${id}`
258+
const openAiMessages = convertToOpenAiMessages(anthropicMessages, { normalizeToolCallId: customNormalizer })
258259

259260
const assistantMessage = openAiMessages[0] as OpenAI.Chat.ChatCompletionAssistantMessageParam
260-
expect(assistantMessage.tool_calls![0].id).toBe(normalizeToolCallId("toolu_123"))
261+
expect(assistantMessage.tool_calls![0].id).toBe("custom_toolu_123")
261262
})
262263
})

src/api/transform/openai-format.ts

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,11 @@ export function normalizeToolCallId(id: string): string {
3131
*/
3232
export interface ConvertToOpenAiMessagesOptions {
3333
/**
34-
* The model ID being used. If it contains "mistral", tool call IDs will be
35-
* normalized to be compatible with Mistral's strict ID requirements.
34+
* Optional function to normalize tool call IDs for providers with strict ID requirements.
35+
* When provided, this function will be applied to all tool_use IDs and tool_result tool_use_ids.
36+
* This allows callers to declare provider-specific ID format requirements.
3637
*/
37-
modelId?: string
38+
normalizeToolCallId?: (id: string) => string
3839
}
3940

4041
export function convertToOpenAiMessages(
@@ -43,8 +44,8 @@ export function convertToOpenAiMessages(
4344
): OpenAI.Chat.ChatCompletionMessageParam[] {
4445
const openAiMessages: OpenAI.Chat.ChatCompletionMessageParam[] = []
4546

46-
// Check if we need to normalize tool call IDs for Mistral compatibility
47-
const shouldNormalizeToolCallIds = options?.modelId?.toLowerCase().includes("mistral") ?? false
47+
// Use provided normalization function or identity function
48+
const normalizeId = options?.normalizeToolCallId ?? ((id: string) => id)
4849

4950
for (const anthropicMessage of anthropicMessages) {
5051
if (typeof anthropicMessage.content === "string") {
@@ -96,9 +97,7 @@ export function convertToOpenAiMessages(
9697
}
9798
openAiMessages.push({
9899
role: "tool",
99-
tool_call_id: shouldNormalizeToolCallIds
100-
? normalizeToolCallId(toolMessage.tool_use_id)
101-
: toolMessage.tool_use_id,
100+
tool_call_id: normalizeId(toolMessage.tool_use_id),
102101
content: content,
103102
})
104103
})
@@ -165,7 +164,7 @@ export function convertToOpenAiMessages(
165164

166165
// Process tool use messages
167166
let tool_calls: OpenAI.Chat.ChatCompletionMessageToolCall[] = toolMessages.map((toolMessage) => ({
168-
id: shouldNormalizeToolCallIds ? normalizeToolCallId(toolMessage.id) : toolMessage.id,
167+
id: normalizeId(toolMessage.id),
169168
type: "function",
170169
function: {
171170
name: toolMessage.name,

0 commit comments

Comments
 (0)