feat: add Mistral provider and enhance ToolChoice handling#83
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis update introduces support for the Mistral AI provider across the codebase, including provider implementation, configuration, and testing. It also refactors the Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Suite
participant Bifrost as Bifrost
participant MistralProvider as MistralProvider
participant MistralAPI as Mistral API
Test->>Bifrost: Initialize with Mistral config
Bifrost->>MistralProvider: Create provider instance
Test->>Bifrost: Send ChatCompletion request
Bifrost->>MistralProvider: ChatCompletion(...)
MistralProvider->>MistralAPI: POST /v1/chat/completions
MistralAPI-->>MistralProvider: Response
MistralProvider-->>Bifrost: BifrostResponse
Bifrost-->>Test: Test result
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate Unit Tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (1)
core/providers/anthropic.go (1)
518-535:⚠️ Potential issueRegression: sending raw string
tool_choicecontradicts proven requirementLong-term feedback (see learning note 2025-06-04) shows Anthropic rejects
"tool_choice": "auto"and requires the object form{"type":"auto"}.
Re-introducing the string path here revives the earlier bug.Suggest: always convert any provided string to the object representation instead of passing it through.
- if params.ToolChoice.ToolChoiceStr != nil { - preparedParams["tool_choice"] = *params.ToolChoice.ToolChoiceStr + if params.ToolChoice.ToolChoiceStr != nil { + preparedParams["tool_choice"] = map[string]interface{}{ + "type": *params.ToolChoice.ToolChoiceStr, + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
core/bifrost.go(1 hunks)core/providers/anthropic.go(1 hunks)core/providers/cohere.go(1 hunks)core/providers/mistral.go(1 hunks)core/schemas/bifrost.go(2 hunks)core/schemas/meta/azure.go(1 hunks)core/schemas/meta/bedrock.go(1 hunks)core/schemas/meta/vertex.go(1 hunks)core/tests/account.go(3 hunks)core/tests/e2e_tool_test.go(1 hunks)core/tests/mistral_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
core/providers/anthropic.go (1)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:483-494
Timestamp: 2025-06-04T09:36:27.956Z
Learning: In the Anthropic provider (core/providers/anthropic.go), the user has confirmed through practical experience that the tool_choice parameter should always use an object format with a "type" field (e.g., {"type": "auto"}, {"type": "tool", "name": "function_name"}), even though the official documentation examples sometimes show "auto" as a direct string. The current implementation correctly handles all tool choice types with the object format.
🧬 Code Graph Analysis (6)
core/providers/cohere.go (1)
core/schemas/bifrost.go (2)
ToolChoice(149-152)ToolChoiceStruct(143-146)
core/tests/mistral_test.go (2)
core/tests/tests.go (2)
TestConfig(34-46)SetupAllRequests(347-391)core/schemas/bifrost.go (3)
Mistral(45-45)Fallback(73-76)Anthropic(41-41)
core/tests/e2e_tool_test.go (1)
core/schemas/bifrost.go (4)
ToolChoiceStruct(143-146)ToolChoiceTypeFunction(132-132)Function(107-111)ToolChoiceFunction(138-140)
core/bifrost.go (2)
core/schemas/bifrost.go (1)
Mistral(45-45)core/providers/mistral.go (1)
NewMistralProvider(59-87)
core/providers/anthropic.go (1)
core/schemas/bifrost.go (4)
ToolChoice(149-152)ToolChoiceStruct(143-146)ToolChoiceTypeFunction(132-132)Function(107-111)
core/tests/account.go (3)
core/schemas/bifrost.go (8)
ModelProvider(36-36)OpenAI(39-39)Anthropic(41-41)Bedrock(42-42)Cohere(43-43)Azure(40-40)Vertex(44-44)Mistral(45-45)core/schemas/account.go (1)
Key(6-10)core/schemas/provider.go (4)
ProviderConfig(108-115)NetworkConfig(29-36)DefaultRequestTimeoutInSeconds(13-13)ConcurrencyAndBufferSize(72-75)
🔇 Additional comments (7)
core/schemas/meta/bedrock.go (1)
46-51: Compact noop methods look fineNo functional impact; consolidation keeps behaviour unchanged.
core/schemas/meta/azure.go (1)
33-40: Consistent noop stubs – OKKeeps parity with other providers.
core/schemas/bifrost.go (1)
45-46: Constant addition LGTMRemember to update docs / env validation lists.
core/bifrost.go (1)
141-143: Provider wired correctlyNew branch matches existing pattern; no concerns.
core/tests/e2e_tool_test.go (1)
44-49: Test update matches new schemaStructure aligns with refactored
ToolChoice; good.core/tests/mistral_test.go (1)
19-33: Model identifiers contain typos & unverified capabilities
"ministral-3b-2410"– appears to be misspelled (mistral-3b-...ormixtral-8x7b-...?).- The test enables
SetupImageandSetupBaseImage, but PR description notes image handling is added only for tests; confirm the provider really supports vision – Mistral’s public API currently doesn’t.These will cause flaky / failing tests.
core/tests/account.go (1)
92-99: Model list contains probable typosSame issue as in the test file –
"ministral-3b-2410"looks wrong.
Validate all three model IDs with Mistral’s dashboard; wrong IDs will make every call fail and mask real provider issues.
There was a problem hiding this comment.
Bug: Cohere API Tool Choice Handling Inconsistency
The Cohere provider's tool_choice handling is inconsistent and breaks API compatibility. ToolChoiceStr values are no longer uppercased, while ToolChoiceStruct values are now sent as an object {"type": "VALUE"} instead of a simple uppercase string. This deviates from the previous behavior of always sending an uppercase string, potentially causing Cohere API requests to fail.
core/providers/cohere.go#L327-L335
bifrost/core/providers/cohere.go
Lines 327 to 335 in c62001a
BugBot free trial expires on June 17, 2025
You have used $0.00 of your $50.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
c62001a to
7a2e3ac
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (4)
core/providers/azure.go (1)
114-124:⚠️ Potential issueCompile-time panic:
for range <int>is illegal
config.ConcurrencyAndBufferSize.Concurrencyis anint.
for range <int>does not compile (cannot range over <int>). All pool pre-warming loops in this file will therefore fail to build.-// Pre-warm response pools -for range config.ConcurrencyAndBufferSize.Concurrency { +// Pre-warm response pools +for i := 0; i < config.ConcurrencyAndBufferSize.Concurrency; i++ { azureChatResponsePool.Put(&AzureChatResponse{}) azureTextCompletionResponsePool.Put(&AzureTextResponse{}) bifrostResponsePool.Put(&schemas.BifrostResponse{}) }Apply the same pattern everywhere this idiom occurs.
Without this fix, the provider will not build at all.core/providers/openai.go (1)
79-88:⚠️ Potential issueSame illegal
rangeoverint→ build breaksIdentical to the Azure provider: the pool warm-up loop uses
for range <int>and will not compile.-// Pre-warm response pools -for range config.ConcurrencyAndBufferSize.Concurrency { +// Pre-warm response pools +for i := 0; i < config.ConcurrencyAndBufferSize.Concurrency; i++ { openAIResponsePool.Put(&OpenAIResponse{}) bifrostResponsePool.Put(&schemas.BifrostResponse{}) }Fix this here and in any other providers that copied the pattern.
core/providers/cohere.go (1)
115-119:⚠️ Potential issueCompile-time error: cannot
rangeover anint
for range config.ConcurrencyAndBufferSize.Concurrency { … }does not compile –rangeexpects an iterable.- for range config.ConcurrencyAndBufferSize.Concurrency { + for i := 0; i < config.ConcurrencyAndBufferSize.Concurrency; i++ {core/providers/anthropic.go (1)
140-145:⚠️ Potential issueSame compile-time bug as Cohere provider
- for range config.ConcurrencyAndBufferSize.Concurrency { + for i := 0; i < config.ConcurrencyAndBufferSize.Concurrency; i++ {
♻️ Duplicate comments (2)
core/providers/mistral.go (2)
158-160: Returned object is released before caller can use it (use-after-put)
defer releaseBifrostResponse(result)runs before the function returns, so the pointer handed to the caller is immediately put back in the pool. This causes data races and memory corruption.Allocate normally (or let the caller release):
- result := acquireBifrostResponse() - defer releaseBifrostResponse(result) + result := &schemas.BifrostResponse{}
168-170:result.Choicesaliases pooled slice – will corrupt after pool releaseAssigning the slice header copies only the pointer; once
responseis returned to the pool the underlying array may be reused.- result.Choices = response.Choices + result.Choices = append( + make([]schemas.BifrostResponseChoice, 0, len(response.Choices)), + response.Choices..., + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
core/bifrost.go(1 hunks)core/providers/anthropic.go(2 hunks)core/providers/azure.go(1 hunks)core/providers/cohere.go(2 hunks)core/providers/mistral.go(1 hunks)core/providers/openai.go(1 hunks)core/schemas/bifrost.go(2 hunks)core/schemas/meta/azure.go(1 hunks)core/schemas/meta/bedrock.go(1 hunks)core/schemas/meta/vertex.go(1 hunks)core/tests/account.go(3 hunks)core/tests/e2e_tool_test.go(1 hunks)core/tests/mistral_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
core/tests/e2e_tool_test.go (1)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/cohere.go:327-335
Timestamp: 2025-06-15T13:46:24.841Z
Learning: For Cohere v1 API in core/providers/cohere.go, the tool_choice parameter formatting uses uppercase strings for the "type" field (e.g., "AUTO", "TOOL") and follows a different structure than initially assumed. The current implementation with strings.ToUpper() for the type field is correct for the v1 API.
core/providers/cohere.go (3)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/cohere.go:327-335
Timestamp: 2025-06-15T13:46:24.841Z
Learning: For Cohere v1 API in core/providers/cohere.go, the tool_choice parameter formatting uses uppercase strings for the "type" field (e.g., "AUTO", "TOOL") and follows a different structure than initially assumed. The current implementation with strings.ToUpper() for the type field is correct for the v1 API.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/cohere.go:327-335
Timestamp: 2025-06-15T13:46:24.841Z
Learning: Cohere v1 API tool_choice parameter accepts only uppercase string values: "REQUIRED" and "NONE". Unlike other providers, it doesn't use structured objects with "type" and "name" fields. The current implementation in core/providers/cohere.go correctly uses strings.ToUpper() to convert ToolChoiceStruct.Type to uppercase format as expected by the API.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:483-494
Timestamp: 2025-06-04T09:36:27.956Z
Learning: In the Anthropic provider (core/providers/anthropic.go), the user has confirmed through practical experience that the tool_choice parameter should always use an object format with a "type" field (e.g., {"type": "auto"}, {"type": "tool", "name": "function_name"}), even though the official documentation examples sometimes show "auto" as a direct string. The current implementation correctly handles all tool choice types with the object format.
core/providers/mistral.go (2)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:96-101
Timestamp: 2025-06-15T13:50:41.379Z
Learning: In the Bifrost project, when a provider doesn't support a specific operation (like text completion), the IsBifrostError flag should be set to false to mark it as a provider-level error rather than a Bifrost framework error. This is intentional design for unsupported operations.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#83
File: core/providers/mistral.go:68-72
Timestamp: 2025-06-15T13:52:45.677Z
Learning: Go 1.22+ supports `for range N` syntax where N is an integer, which iterates from 0 to N-1. This is valid Go syntax and does not require conversion to an indexed loop.
🧬 Code Graph Analysis (8)
core/tests/e2e_tool_test.go (1)
core/schemas/bifrost.go (4)
ToolChoiceStruct(143-146)ToolChoiceTypeFunction(132-132)Function(107-111)ToolChoiceFunction(138-140)
core/providers/openai.go (1)
core/schemas/provider.go (1)
ConcurrencyAndBufferSize(72-75)
core/bifrost.go (2)
core/schemas/bifrost.go (1)
Mistral(45-45)core/providers/mistral.go (1)
NewMistralProvider(59-87)
core/providers/azure.go (1)
core/schemas/provider.go (1)
ConcurrencyAndBufferSize(72-75)
core/tests/mistral_test.go (2)
core/tests/tests.go (2)
TestConfig(34-46)SetupAllRequests(347-391)core/schemas/bifrost.go (3)
Mistral(45-45)Fallback(73-76)Anthropic(41-41)
core/providers/cohere.go (2)
core/schemas/provider.go (1)
ConcurrencyAndBufferSize(72-75)core/schemas/bifrost.go (2)
ToolChoice(149-152)ToolChoiceStruct(143-146)
core/providers/anthropic.go (2)
core/schemas/provider.go (1)
ConcurrencyAndBufferSize(72-75)core/schemas/bifrost.go (4)
ToolChoice(149-152)ToolChoiceStruct(143-146)ToolChoiceTypeFunction(132-132)Function(107-111)
core/tests/account.go (3)
core/schemas/bifrost.go (8)
ModelProvider(36-36)OpenAI(39-39)Anthropic(41-41)Bedrock(42-42)Cohere(43-43)Azure(40-40)Vertex(44-44)Mistral(45-45)core/schemas/account.go (1)
Key(6-10)core/schemas/provider.go (5)
ProviderConfig(108-115)NetworkConfig(29-36)DefaultNetworkConfig(39-44)ConcurrencyAndBufferSize(72-75)DefaultConcurrencyAndBufferSize(78-81)
🔇 Additional comments (6)
core/bifrost.go (1)
141-143: Mistral provider wired-in correctlyThe switch now handles
schemas.Mistral, returning a properly initialised provider andnilerror. Interface contract remains intact.No issues spotted.
core/schemas/meta/bedrock.go (1)
46-51: Nice consolidation of unused Bedrock meta accessorsGrouping the always-
nilhelpers in one block reduces noise while keeping the API surface unchanged.core/schemas/meta/vertex.go (1)
32-39: Alphabetical stub block looks goodThe unused-method block is now alphabetically ordered and consistent with other meta files.
No further action needed.core/schemas/meta/azure.go (1)
33-40: Alphabetical ordering fixedSame positive note here – the nil stubs are ordered and tidy.
LGTM.core/tests/mistral_test.go (1)
19-33: Model names & image flags may be wrong – please verify
- The strings
"ministral-3b-2410"and"pixtral-12b-latest"look like typos of"mistral-…" / "mixtral-…"etc.SetupImage/SetupBaseImageare enabled, but (as far as this PR shows) the Mistral provider doesn’t advertise image endpoints – tests will inevitably fail if the API rejects them.Would you confirm the correct model identifiers and whether image-generation should really be exercised for Mistral?
core/providers/cohere.go (1)
327-334:Details
❌ Incorrect review comment
tool_choicemap is incomplete and may break Cohere v1When the struct form is used:
- It always returns
{ "type": "<UPPERCASE>" }, omitting the mandatory"name"key for the TOOL/ FUNCTION case.- The value for function should be
"TOOL"per Cohere docs, not"FUNCTION".- requestBody["tool_choice"] = map[string]interface{}{ - "type": strings.ToUpper(string(params.ToolChoice.ToolChoiceStruct.Type)), - } + choiceType := strings.ToUpper(string(params.ToolChoice.ToolChoiceStruct.Type)) + // Map local "function" → API "TOOL" + if choiceType == "FUNCTION" { + choiceType = "TOOL" + } + tc := map[string]interface{}{"type": choiceType} + if choiceType == "TOOL" && params.ToolChoice.ToolChoiceStruct.Function.Name != "" { + tc["name"] = params.ToolChoice.ToolChoiceStruct.Function.Name + } + requestBody["tool_choice"] = tcPlease re-check against the latest Cohere v1 spec – happy to adjust once confirmed.
🏁 Script executed:
#!/bin/bash # Display lines around the tool_choice handling in cohere provider sed -n '320,360p' core/providers/cohere.goLength of output: 1201
Ignore struct form for
tool_choicein Cohere v1Cohere’s v1 chat API only accepts uppercase string values for
tool_choice(e.g."REQUIRED","NONE"). It does not support an object with"type"/"name"fields, so the suggested diff is not applicable.• File needing attention:
core/providers/cohere.go
– Remove or ignore theelse if params.ToolChoice.ToolChoiceStruct != nil { … }branch.Likely an incorrect or invalid review comment.
Merge activity
|
# Add Mistral Provider Support This PR adds support for the Mistral AI provider to Bifrost. The implementation includes: - New `MistralProvider` with chat completion capabilities - Added Mistral to the list of supported model providers - Enhanced `ToolChoice` to support both string and struct representations - Updated Anthropic and Cohere providers to work with the new `ToolChoice` structure - Added Mistral tests with support for tool calls and image handling - Refactored provider meta config files to use more concise function declarations The Mistral implementation follows the same pattern as other providers, using connection pooling and efficient memory management. It supports models like `mistral-large-2411`, `ministral-3b-2410`, and `pixtral-12b-latest`.

Add Mistral Provider Support
This PR adds support for the Mistral AI provider to Bifrost. The implementation includes:
MistralProviderwith chat completion capabilitiesToolChoiceto support both string and struct representationsToolChoicestructureThe Mistral implementation follows the same pattern as other providers, using connection pooling and efficient memory management. It supports models like
mistral-large-2411,ministral-3b-2410, andpixtral-12b-latest.