Skip to content

fix: apply mergeToolResultText and normalize tool IDs for Mistral/Devstral models in OpenAI provider #10904

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

Closed
roomote-v0 wants to merge 4 commits into from
Closed

fix: apply mergeToolResultText and normalize tool IDs for Mistral/Devstral models in OpenAI provider #10904

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9162633
fix: apply mergeToolResultText and normalize tool IDs for Mistral/Dev…
roomote Jan 22, 2026
367ec5f
test: ensure Mistral tests always assert tool message existence
roomote Jan 22, 2026
914cc4e
test: strengthen Mistral test to verify tool message contains merged …
roomote Jan 22, 2026
0d212ef
test: remove if conditional and add test to verify Mistral no-user-af…
roomote Jan 23, 2026
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
152 changes: 152 additions & 0 deletions src/api/providers/__tests__/openai.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1139,6 +1139,158 @@ describe("OpenAiHandler", () => {
)
})
})

describe("Mistral/Devstral Family Models", () => {
const systemPrompt = "You are a helpful assistant."
const messagesWithToolResult: Anthropic.Messages.MessageParam[] = [
{
role: "user",
content: [{ type: "text", text: "Hello!" }],
},
{
role: "assistant",
content: [
{
type: "tool_use",
id: "call_test_123456789",
name: "read_file",
input: { path: "test.ts" },
},
],
},
{
role: "user",
content: [
{
type: "tool_result",
tool_use_id: "call_test_123456789",
content: "File content here",
},
{
type: "text",
text: "<environment_details>Details here</environment_details>",
},
],
},
]

it("should detect Mistral models and apply mergeToolResultText", async () => {
const mistralHandler = new OpenAiHandler({
...mockOptions,
openAiModelId: "mistral-large-latest",
})

const stream = mistralHandler.createMessage(systemPrompt, messagesWithToolResult)
for await (const _chunk of stream) {
// Consume the stream
}

expect(mockCreate).toHaveBeenCalled()
const callArgs = mockCreate.mock.calls[0][0]

// Find the messages - should NOT have a user message after tool message
// because mergeToolResultText should merge text into the tool message
const messages = callArgs.messages
const toolMessageIndex = messages.findIndex((m: any) => m.role === "tool")

// Assert tool message exists - test setup should always produce a tool message
expect(toolMessageIndex).not.toBe(-1)

// The message after tool should be the next user message from a new request,
// not a user message with environment_details (which should be merged)
const nextMessage = messages[toolMessageIndex + 1]
// If there's a next message, it should not be a user message containing environment_details
if (nextMessage && nextMessage.role === "user") {
Copy link
Copy Markdown

@CullenShane CullenShane Jan 22, 2026

Choose a reason for hiding this comment

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

@roomote After a tool message, according to mistral_common, the next message must be an assistant message or another tool message, not a user message. There must never be a user message following a tool message for mistral models.

Please review this code snippet that validates the messages on the llm server:
https://github.com/mistralai/mistral-common/blob/c2c643a44e661b71a1eb99cdf47cd4ed979c5ac7/src/mistral_common/protocol/instruct/validator.py#L248

    def _validate_message_order(self, messages: list[UATS]) -> None:
        """
        Validates the order of the messages, for example user -> assistant -> user -> assistant -> ...
        """
        previous_role = None
        for message in messages:
            current_role = message.role

            if previous_role is not None:
                if previous_role == Roles.system:
                    expected_roles = {Roles.user, Roles.assistant, Roles.system}
                elif previous_role == Roles.user:
                    expected_roles = {Roles.assistant, Roles.system, Roles.user}
                elif previous_role == Roles.assistant:
                    expected_roles = {Roles.assistant, Roles.user, Roles.tool}
                elif previous_role == Roles.tool:
                    expected_roles = {Roles.assistant, Roles.tool}

                if current_role not in expected_roles:
                    raise InvalidMessageStructureException(
                        f"Unexpected role '{current_role}' after role '{previous_role}'"
                    )

            previous_role = current_role

Copy link
Copy Markdown
Contributor Author

@roomote-v0 roomote-v0 Bot Jan 22, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Fixaroo Clock   See task on Roo Cloud

Reviewed and verified the implementation against the Mistral constraint. The mergeToolResultText: true option correctly prevents the forbidden tool -> user message sequence by appending text content into the tool message. Strengthened the test to explicitly verify this behavior.

View commit | Revert commit

const content =
typeof nextMessage.content === "string" ? nextMessage.content : JSON.stringify(nextMessage.content)
expect(content).not.toContain("environment_details")
}
})

it("should detect Devstral models and apply mergeToolResultText", async () => {
const devstralHandler = new OpenAiHandler({
...mockOptions,
openAiModelId: "devstral-small-2",
})

const stream = devstralHandler.createMessage(systemPrompt, messagesWithToolResult)
for await (const _chunk of stream) {
// Consume the stream
}

expect(mockCreate).toHaveBeenCalled()
const callArgs = mockCreate.mock.calls[0][0]

// Verify the model ID was passed correctly
expect(callArgs.model).toBe("devstral-small-2")
})

it("should normalize tool call IDs to 9-char alphanumeric for Mistral models", async () => {
const mistralHandler = new OpenAiHandler({
...mockOptions,
openAiModelId: "mistral-medium",
})

const stream = mistralHandler.createMessage(systemPrompt, messagesWithToolResult)
for await (const _chunk of stream) {
// Consume the stream
}

expect(mockCreate).toHaveBeenCalled()
const callArgs = mockCreate.mock.calls[0][0]

// Find the tool message and verify the tool_call_id is normalized
const toolMessage = callArgs.messages.find((m: any) => m.role === "tool")
// Assert tool message exists - test setup should always produce a tool message
expect(toolMessage).toBeDefined()
// The ID should be normalized to 9 alphanumeric characters
expect(toolMessage.tool_call_id).toMatch(/^[a-zA-Z0-9]{9}$/)
})

it("should NOT apply Mistral-specific handling for non-Mistral models", async () => {
const gpt4Handler = new OpenAiHandler({
...mockOptions,
openAiModelId: "gpt-4-turbo",
})

const stream = gpt4Handler.createMessage(systemPrompt, messagesWithToolResult)
for await (const _chunk of stream) {
// Consume the stream
}

expect(mockCreate).toHaveBeenCalled()
const callArgs = mockCreate.mock.calls[0][0]

// For non-Mistral models, tool_call_id should retain original format
const toolMessage = callArgs.messages.find((m: any) => m.role === "tool")
// Assert tool message exists - test setup should always produce a tool message
expect(toolMessage).toBeDefined()
// The original ID format should be preserved (not normalized)
expect(toolMessage.tool_call_id).toBe("call_test_123456789")
})

it("should handle case-insensitive model detection", async () => {
const mixedCaseHandler = new OpenAiHandler({
...mockOptions,
openAiModelId: "Mistral-Large-LATEST",
})

const stream = mixedCaseHandler.createMessage(systemPrompt, messagesWithToolResult)
for await (const _chunk of stream) {
// Consume the stream
}

expect(mockCreate).toHaveBeenCalled()
const callArgs = mockCreate.mock.calls[0][0]

// Verify model detection worked despite mixed case
const toolMessage = callArgs.messages.find((m: any) => m.role === "tool")
// Assert tool message exists - test setup should always produce a tool message
expect(toolMessage).toBeDefined()
// The ID should be normalized (indicating Mistral detection worked)
expect(toolMessage.tool_call_id).toMatch(/^[a-zA-Z0-9]{9}$/)
})
})
})

describe("getOpenAiModels", () => {
Expand Down
47 changes: 42 additions & 5 deletions src/api/providers/openai.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import type { ApiHandlerOptions } from "../../shared/api"

import { TagMatcher } from "../../utils/tag-matcher"

import { convertToOpenAiMessages } from "../transform/openai-format"
import { convertToOpenAiMessages, ConvertToOpenAiMessagesOptions } from "../transform/openai-format"
import { normalizeMistralToolCallId } from "../transform/mistral-format"
import { convertToR1Format } from "../transform/r1-format"
import { ApiStream, ApiStreamUsageChunk } from "../transform/stream"
import { getModelParams } from "../transform/model-params"
Expand Down Expand Up @@ -91,6 +92,9 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl
const isAzureAiInference = this._isAzureAiInference(modelUrl)
const deepseekReasoner = modelId.includes("deepseek-reasoner") || enabledR1Format

// Mistral/Devstral models require strict tool message ordering and normalized tool call IDs
const mistralConversionOptions = this._getMistralConversionOptions(modelId)

if (modelId.includes("o1") || modelId.includes("o3") || modelId.includes("o4")) {
yield* this.handleO3FamilyMessage(modelId, systemPrompt, messages, metadata)
return
Expand Down Expand Up @@ -121,7 +125,7 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl
}
}

convertedMessages = [systemMessage, ...convertToOpenAiMessages(messages)]
convertedMessages = [systemMessage, ...convertToOpenAiMessages(messages, mistralConversionOptions)]

if (modelInfo.supportsPromptCache) {
// Note: the following logic is copied from openrouter:
Expand Down Expand Up @@ -225,7 +229,7 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl
model: modelId,
messages: deepseekReasoner
? convertToR1Format([{ role: "user", content: systemPrompt }, ...messages])
: [systemMessage, ...convertToOpenAiMessages(messages)],
: [systemMessage, ...convertToOpenAiMessages(messages, mistralConversionOptions)],
// Tools are always present (minimum ALWAYS_AVAILABLE_TOOLS)
tools: this.convertToolsForOpenAI(metadata?.tools),
tool_choice: metadata?.tool_choice,
Expand Down Expand Up @@ -329,6 +333,9 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl
const modelInfo = this.getModel().info
const methodIsAzureAiInference = this._isAzureAiInference(this.options.openAiBaseUrl)

// Mistral/Devstral models require strict tool message ordering and normalized tool call IDs
const mistralConversionOptions = this._getMistralConversionOptions(modelId)

if (this.options.openAiStreamingEnabled ?? true) {
const isGrokXAI = this._isGrokXAI(this.options.openAiBaseUrl)

Expand All @@ -339,7 +346,7 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl
role: "developer",
content: `Formatting re-enabled\n${systemPrompt}`,
},
...convertToOpenAiMessages(messages),
...convertToOpenAiMessages(messages, mistralConversionOptions),
],
stream: true,
...(isGrokXAI ? {} : { stream_options: { include_usage: true } }),
Expand Down Expand Up @@ -375,7 +382,7 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl
role: "developer",
content: `Formatting re-enabled\n${systemPrompt}`,
},
...convertToOpenAiMessages(messages),
...convertToOpenAiMessages(messages, mistralConversionOptions),
],
reasoning_effort: modelInfo.reasoningEffort as "low" | "medium" | "high" | undefined,
temperature: undefined,
Expand Down Expand Up @@ -508,6 +515,36 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl
return urlHost.endsWith(".services.ai.azure.com")
}

/**
* Checks if the model is part of the Mistral/Devstral family.
* Mistral models require strict message ordering (no user message after tool message)
* and have specific tool call ID format requirements (9-char alphanumeric).
* @param modelId - The model identifier to check
* @returns true if the model is a Mistral/Devstral family model
*/
private _isMistralFamily(modelId: string): boolean {
const modelIdLower = modelId.toLowerCase()
return modelIdLower.includes("mistral") || modelIdLower.includes("devstral")
}

/**
* Gets the conversion options for Mistral/Devstral models.
* When the model is in the Mistral family, returns options to:
* 1. Merge text content after tool results into the last tool message (prevents user-after-tool error)
* 2. Normalize tool call IDs to 9-char alphanumeric format (Mistral's strict requirement)
* @param modelId - The model identifier
* @returns Conversion options for convertToOpenAiMessages, or undefined for non-Mistral models
*/
private _getMistralConversionOptions(modelId: string): ConvertToOpenAiMessagesOptions | undefined {
if (this._isMistralFamily(modelId)) {
return {
mergeToolResultText: true,
normalizeToolCallId: normalizeMistralToolCallId,
}
}
return undefined
}

/**
* Adds max_completion_tokens to the request body if needed based on provider configuration
* Note: max_tokens is deprecated in favor of max_completion_tokens as per OpenAI documentation
Expand Down
Loading