Skip to content

Conversation

@daniel-lxs
Copy link
Member

@daniel-lxs daniel-lxs commented Dec 15, 2025

Summary

Some providers (like Mistral) require tool call IDs to be:

  • Only alphanumeric characters (a-z, A-Z, 0-9)
  • Exactly 9 characters in length

This caused errors when conversations with tool calls from one provider (e.g., OpenAI's call_xxx format) were routed to Mistral via OpenRouter.

Error Example

ApiProviderError: {"object":"error","message":"Tool call id was call_5019f900a247472bacde0b82 but must be a-z, A-Z, 0-9, with a length of 9.","type":"invalid_function_call","param":null,"code":"3280"}

Solution

  • Added normalizeToolCallId() function that strips non-alphanumeric characters and pads/truncates to exactly 9 characters
  • Added modelId option to convertToOpenAiMessages() that conditionally normalizes IDs only when the model contains 'mistral'
  • OpenRouter now passes modelId to enable normalization for Mistral models
  • Direct Mistral provider uses convertToMistralMessages() which always normalizes IDs

This scoped approach only affects Mistral models, avoiding any potential impact on the 15+ other providers using OpenAI-compatible format.

Example Transformations

  • call_5019f900a247472bacde0b82call5019f
  • toolu_01234567890abcdeftoolu0123
  • weather-123weather12

Testing

  • All 220 transform tests pass
  • All 4681 tests pass in the full test suite

Important

Adds tool call ID normalization for Mistral models in OpenRouter to ensure compatibility with strict ID requirements.

  • Behavior:
    • Adds normalizeMistralToolCallId() in mistral-format.ts to ensure tool call IDs are alphanumeric and 9 characters long.
    • Updates convertToOpenAiMessages() in openai-format.ts to optionally use normalizeToolCallId for Mistral models.
    • Modifies OpenRouterHandler in openrouter.ts to pass normalization function for Mistral models.
  • Testing:
    • Adds tests for normalizeMistralToolCallId() in mistral-format.spec.ts.
    • Updates tests in openai-format.spec.ts to cover normalization scenarios.

This description was created by Ellipsis for 58410fe. You can customize this summary. It will automatically update as commits are pushed.

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Dec 15, 2025
@roomote
Copy link
Contributor

roomote bot commented Dec 15, 2025

Rooviewer Clock   See task on Roo Cloud

Review complete. No issues found.

The latest commit correctly moves normalizeMistralToolCallId to mistral-format.ts, completing the separation of concerns refactoring. The convertToOpenAiMessages function now has no knowledge of Mistral-specific requirements, and the OpenRouterHandler properly decides when to apply normalization based on model detection.

Previous reviews

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

Some providers (like Mistral) require tool call IDs to be:
- Only alphanumeric characters (a-z, A-Z, 0-9)
- Exactly 9 characters in length

This caused errors when conversations with tool calls from one provider
(e.g., OpenAI's call_xxx format) were routed to Mistral via OpenRouter.

Solution:
- Added normalizeToolCallId() function that strips non-alphanumeric
  characters and pads/truncates to exactly 9 characters
- Added modelId option to convertToOpenAiMessages() that conditionally
  normalizes IDs only when the model contains 'mistral'
- OpenRouter now passes modelId to enable normalization for Mistral models
- Direct Mistral provider uses convertToMistralMessages() which always
  normalizes IDs

This scoped approach only affects Mistral models, avoiding any potential
impact on the 15+ other providers using OpenAI-compatible format.

Example transformations:
- call_5019f900a247472bacde0b82 → call5019f
- toolu_01234567890abcdef → toolu0123
- weather-123 → weather12
@daniel-lxs daniel-lxs force-pushed the fix/normalize-tool-call-ids branch from 87e8348 to 72eddd7 Compare December 15, 2025 19:15
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Dec 15, 2025
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Dec 15, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap Dec 15, 2025
@hannesrudolph hannesrudolph added PR - Needs Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Dec 15, 2025
Copy link
Collaborator

@cte cte left a comment

Choose a reason for hiding this comment

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

Left one nitpick.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 16, 2025
…OpenAiMessages

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.
Moves the tool call ID normalization function to mistral-format.ts with
explicit Mistral naming (normalizeMistralToolCallId) since Mistral is the
only provider that requires this specific 9-character alphanumeric ID format.

This better separates concerns by keeping provider-specific logic in the
appropriate module while keeping the generic convertToOpenAiMessages function
flexible via the optional normalizeToolCallId callback.
@cte
Copy link
Collaborator

cte commented Dec 16, 2025

Very nice!

@cte cte merged commit a7b192a into main Dec 16, 2025
10 checks passed
@cte cte deleted the fix/normalize-tool-call-ids branch December 16, 2025 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer PR - Needs Review size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants