main backmerge

[akshaydeo]

No description provided.

In general, to fix inefficient regular expressions flagged for potential exponential backtracking, you either: (1) remove ambiguity inside quantified subexpressions (avoid r*/r+ where r can match the same substring in multiple ways), or (2) constrain what the repetition can match so that the engine does not have to explore many alternative paths. For URL-like patterns, this usually means making the path portion more structured and avoiding overly broad .*-style or similar constructs.

In this specific case, the problematic part is the path/query segment: ([\/\w \.-]*)* simplified to [\/\w \.-]* in the shown pattern:

/^(https?:\/\/)?((localhost|(\d{1,3}\.){3}\d{1,3})(:\d+)?|([\da-z\.-]+)\.([a-z\.]{2,6}))([\/\w \.-]*)*\/?$/
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                                   this part

We can keep the overall behavior (“simple URL with optional path”) while making the repetition less ambiguous and more structured by:

• Requiring the path to start with / when present.
• Then allowing repetitions of path segments that don’t include / (and disallowing spaces there to be more realistic and less ambiguous).
• Avoiding a trailing * that can swallow arbitrary spaces and dots indefinitely at the end.

One straightforward, safer replacement for the path part is:

(\/[^\s]*)?\/?$

Meaning “optional / followed by any number of non-whitespace characters, then an optional trailing /”. This still accepts https://example.com, https://example.com/, https://example.com/foo, http://localhost:3000/pricing.json, and similar, which matches the intended use (a URL to a datasheet). It removes the specific ambiguous, broad character class repetition and avoids nested ambiguity.

Concretely, in ui/app/workspace/config/views/pricingConfigView.tsx, on the line with the pattern.value, replace the entire regex with this safer one:

/^(https?:\/\/)?((localhost|(\d{1,3}\.){3}\d{1,3})(:\d+)?|([\da-z.-]+)\.([a-z.]{2,6}))(\/[^\s]*)?\/?$/

Note: I also removed unnecessary backslashes before - and . inside classes where they’re not needed, but their semantics remain the same. No new imports or helpers are needed; this is just an inline regex change.

---

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Added batch processing API for creating and managing asynchronous batch jobs
  • Added file management API for uploading, listing, retrieving, and deleting files
  • Added chat audio input support for voice-based interactions
  • Introduced prompt caching to optimize token usage
  • Added new HuggingFace provider with comprehensive model support

• Provider Updates

  • Extended multiple providers with batch and file operation support
  • Improved error handling and metadata propagation across providers

• Infrastructure

  • Upgraded Go version to 1.25.5
  • Enhanced test filtering with new PATTERN variable support

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Walkthrough

Comprehensive expansion of batch and file operation APIs across Bifrost providers. Introduces new HuggingFace provider with full chat, embedding, speech, and transcription support. Enhances Mistral with transcription streaming, Bedrock with S3 integration and context-aware conversions, and Anthropic/Azure/Gemini with file and batch management. Updates CI/CD for Go 1.25.5, AWS dependencies, and HuggingFace API credentials. Expands test utilities for batch, file, chat audio, and prompt caching scenarios.

Changes

Cohort / File(s)	Summary
Workflow & Build Configuration .github/workflows/pr-tests.yml, .github/workflows/release-pipeline.yml, .github/workflows/snyk.yml, .github/workflows/scripts/install-cross-compilers.sh, Makefile	Updated Go version 1.24.3→1.25.5 across workflows. Injected HUGGING_FACE_API_KEY, AWS_S3_BUCKET, AWS_BEDROCK_ROLE_ARN into release steps. Added macOS SDK upgrade (11.3→12.3) for cross-compilation. Introduced PATTERN variable to Makefile for test filtering with conditional exclusivity checks against TESTCASE.
Go Modules & Dependencies core/go.mod, core/internal/testutil/account.go	Bumped Go 1.24.3→1.25.5. Updated AWS SDK v2 components (core, config, credentials, S3, STS, SSO modules). Extended test account configuration with ChatAudioModel, ExternalTTSProvider/Model, BatchExtraParams, FileExtraParams; added HuggingFace and Nebius providers with batch/file capability flags.
Core Bifrost API Extensions core/bifrost.go	Added 5 batch methods (BatchCreate/List/Retrieve/Cancel/Results) and 5 file methods (FileUpload/List/Retrieve/Delete/Content) to Bifrost. Introduced getProviderQueue error handling, multi-key batch/file operation logic, per-key API-name filtering, provider runtime expansion for Nebius/HuggingFace.
Test Infrastructure — Batch & File core/internal/testutil/batch.go, core/internal/testutil/account.go (batch/file config)	Added 13 batch/file test runners (RunBatchCreate/List/Retrieve/Cancel/Results/Unsupported, RunFileUpload/List/Retrieve/Delete/Content/Unsupported, RunFileAndBatchIntegration). Extended TestScenarios with 13 batch/file/audio flags; expanded AllProviderConfigs with batch/file capabilities and new providers.
Test Infrastructure — Audio & Prompt Caching core/internal/testutil/chat_audio.go, core/internal/testutil/prompt_caching.go, core/internal/testutil/validation_presets.go, core/internal/testutil/utils.go	Added chat audio test runners (RunChatAudioTest, RunChatAudioStreamTest) with audio input support. Extended prompt caching with ephemeral CacheControl blocks in tools and system messages. Added ChatAudioExpectations; new helpers GetSampleAudioBase64() and CreateAudioChatMessage().
Test Infrastructure — Transcription & Streaming core/internal/testutil/transcription.go, core/internal/testutil/transcription_stream.go, core/internal/testutil/responses_stream.go, core/internal/testutil/test_retry_framework.go	Introduced ExternalTTSProvider/Model overrides for TTS. Added Fal-AI/HuggingFace path in transcription (uses pre-generated mp3 fixture). Enhanced streaming retry logging with per-attempt counters, delay info, and failure context. Increased streaming safety threshold from 100→300 chunks.
Anthropic Provider core/providers/anthropic/*.go	Implemented batch operations (BatchCreate/List/Retrieve/Cancel/Results) with full request/response lifecycle. Added Files API support (FileUpload/List/Retrieve/Delete/Content). Extended completeRequest with RequestMetadata for richer error context. Added CacheControl fields to tools and content blocks. Introduced batch/file JSONL parsing and Bifrost conversion helpers.
Azure Provider core/providers/azure/azure.go, core/providers/azure/files.go	Implemented batch operations (BatchCreate/List/Retrieve/Cancel/Results) with inline JSONL upload when file input missing. Added file operations (FileUpload/List/Retrieve/Delete/Content) with per-key validation and API-version handling. Introduced AzureFileResponse type with Bifrost conversion. Enhanced streaming paths with RequestMetadata propagation.
Bedrock Provider core/providers/bedrock/*.go (bedrock.go, batch.go, files.go, s3.go, chat.go, responses.go, utils.go, types.go)	Complete batch implementation with Bedrock-specific request/response models, status translation, JSONL result parsing. File operations leveraging S3 for storage (upload, list, retrieve, delete). Added context-aware conversions (ToBedrock* functions accept context). Extended Nova/Anthropic reasoning config handling. Introduced S3 helpers (parseS3URI, escapeS3KeyForURL, ToS3ListObjectsV2XML). Added cache-point semantics and CacheControl field support.
Cerebras, Cohere, Elevenlabs, Groq Providers core/providers/cerebras/cerebras.go, core/providers/cohere/cohere.go, core/providers/elevenlabs/elevenlabs.go, core/providers/groq/groq.go	Added stub implementations (10 methods each: BatchCreate/List/Retrieve/Cancel/Results, FileUpload/List/Retrieve/Delete/Content) returning unsupported operation errors. Extended completeRequest signatures to accept RequestMetadata for richer error context in Cohere/Elevenlabs.
Gemini Provider core/providers/gemini/*.go (gemini.go, batch.go, files.go, chat.go, responses.go, errors.go, types.go, utils.go)	Implemented batch operations with file/inline request support, per-key pagination, and manifest-based result retrieval. Added file operations (upload, list, retrieve, delete; content unsupported). Introduced extensive batch/file types (GeminiBatchCreateRequest, GeminiBatchJobResponse, GeminiFileResponse). Added thought-signature handling with base64 decoding in Part unmarshaling. Extended responses with consumption-tracking for thought signatures and multi-part content handling.
HuggingFace Provider (New) core/providers/huggingface/*.go (huggingface.go, types.go, utils.go, chat.go, responses.go, embedding.go, speech.go, transcription.go, models.go, huggingface_test.go)	Complete new provider implementation with 10+ public methods (ChatCompletion/Stream, Embedding, Speech, Transcription/Stream, ListModels, Batch*/File* stubs). Introduced model/provider routing via inferenceProvider constants and URL builders. Added model-alias caching to resolve model IDs across multiple inference providers (fal-ai, Nebius, Replicate, etc.). Implemented comprehensive type system with custom JSON marshaling for tool choice, transcription early stopping, embedding inputs.
Mistral Provider core/providers/mistral/mistral.go, core/providers/mistral/transcription.go, core/providers/mistral/types.go, core/providers/mistral/mistral_test.go, core/providers/mistral/transcription_test.go	Implemented full Transcription and TranscriptionStream with multipart form handling, SSE streaming support, segment/word timing extraction. Added multipart body builders and transcription response converters. Introduced extensive unit tests (multipart validation, mock server tests, streaming edge cases, context cancellation). Added transcription types with request/response structures and streaming event models.
Minor Updates & Fixes core/bifrost_test.go, core/chatbot_test.go, core/mcp.go, core/changelog.md	Fixed TestIsRateLimitError to use locale-aware Title casing (golang.org/x/text/cases). Simplified tool-call handling in SendMessage (len check). Fixed MCP tool schema handling to guarantee non-nil Properties and default Type to "object". Added changelog entry for gemini thought signature fix.
Provider Test Configurations core/providers/anthropic/anthropic_test.go, core/providers/azure/azure_test.go, core/providers/bedrock/bedrock_test.go, core/providers/gemini/gemini_test.go, core/providers/mistral/mistral_test.go	Extended test configs with PromptCachingModel, ChatAudioModel, SpeechSynthesisModel, TranscriptionModel. Enabled batch/file/audio scenario flags where supported. Integrated AWS credentials (S3_BUCKET, BEDROCK_ROLE_ARN) into Bedrock tests. Context propagation added to Bedrock conversion function calls.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Bifrost
    participant Provider as Provider<br/>(e.g., Anthropic)
    participant External as External Service<br/>(API / S3)
    
    rect rgb(220, 240, 255)
    Note over Client,External: Batch Operation Flow
    Client->>Bifrost: BatchCreateRequest(ctx, req)
    Bifrost->>Bifrost: Validate input, select provider
    Bifrost->>Provider: BatchCreate(ctx, key, req)
    
    alt Inline Requests
        Provider->>Provider: Convert request items to<br/>provider-specific format
        Provider->>Provider: Build request payload
    else File Input
        Provider->>External: Upload batch input file<br/>(if needed)
        External-->>Provider: File ID / URI
        Provider->>Provider: Reference file in batch create
    end
    
    Provider->>External: POST batch/create endpoint
    External-->>Provider: BatchResponse (ID, status, etc.)
    Provider->>Provider: Convert to Bifrost format<br/>w/ latency & metadata
    Provider-->>Bifrost: BifrostBatchCreateResponse
    Bifrost-->>Client: Response (with ID, status, counts)
    end
    
    rect rgb(240, 255, 240)
    Note over Client,External: File Operation Flow
    Client->>Bifrost: FileUploadRequest(ctx, req)
    Bifrost->>Provider: FileUpload(ctx, key, req)
    Provider->>Provider: Validate input, prepare<br/>multipart/file data
    
    alt Provider with Direct File API
        Provider->>External: POST files/upload<br/>(multipart)
    else Provider with S3 Backend
        Provider->>External: PUT s3://bucket/key<br/>(raw file bytes)
    end
    
    External-->>Provider: FileResponse (ID, size,<br/>timestamp, URI)
    Provider->>Provider: Enrich with latency,<br/>provider metadata
    Provider-->>Bifrost: BifrostFileUploadResponse
    Bifrost-->>Client: Response (ID, size, metadata)
    end
    
    rect rgb(255, 240, 220)
    Note over Client,External: Transcription Streaming (Mistral/HuggingFace)
    Client->>Bifrost: TranscriptionStream(ctx, req)
    Bifrost->>Provider: TranscriptionStream(ctx, req)
    Provider->>Provider: Build multipart form<br/>(audio file + params)
    Provider->>External: POST with Accept: text/event-stream
    External-->>Provider: SSE stream (delta, segment,<br/>language, done events)
    
    loop Per Event
        Provider->>Provider: Parse SSE event
        Provider->>Bifrost: Send BifrostStream<br/>(text delta / segment)
        Bifrost->>Client: Stream chunk
    end
    
    Provider-->>Bifrost: Close channel (done)
    Bifrost-->>Client: Stream complete
    end

Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Areas requiring extra attention:

• Bedrock S3 integration & context propagation (core/providers/bedrock/responses.go, core/providers/bedrock/bedrock.go, core/providers/bedrock/s3.go): Context threading through multiple conversion layers, S3 URL derivation, and manifest fetching for batch results require careful verification of ARN handling and region resolution.

• HuggingFace provider implementation (core/providers/huggingface/utils.go, core/providers/huggingface/huggingface.go): Complex model/provider routing logic, inference-provider mapping, model-alias caching, and per-provider URL construction need validation for correctness across 10+ inference providers.

• Batch/file operation consistency across providers (Anthropic, Azure, Bedrock, Gemini): Ensure per-key fallback logic, error aggregation, and response mapping are uniform where applicable; validate JSONL parsing and timestamp handling.

• Mistral transcription streaming (core/providers/mistral/transcription.go): SSE event parsing, multipart body construction, and streaming chunk assembly require careful review of edge cases (empty deltas, done events, language events).

• Gemini thought-signature handling & consumption tracking (core/providers/gemini/responses.go, core/providers/gemini/utils.go): Base64 decoding, per-part translation into multiple ResponsesMessage entries, and look-ahead consumption of reasoning messages for signature pairing are error-prone.

• Test infrastructure additions (core/internal/testutil/batch.go, core/internal/testutil/chat_audio.go): Verify conditional test execution flags align with provider capabilities and that test expectations match actual behavior across all enabled scenarios.

Possibly Related PRs

• Feature/987 mistral audio transcription support #1016: Mistral transcription implementations (types, streaming, tests) directly align with this PR's transcription additions.
• feat: huggingface provider added #1006: HuggingFace provider integration and HUGGING_FACE_API_KEY CI/workflow setup overlap with this PR's HuggingFace wiring.
• fix: prompt caching issues for OpenAI chat completions and OrderedMap implementation #952: Tool/function schema handling (OrderedMap usage, ToolFunctionParameters.Properties) shares code-level changes with this PR's conversion utilities.

Suggested Reviewers

• danpiths
• Pratham-Mishra04

---

🐰 Batch by batch, file by file,
New providers join our growing pile!
HuggingFace hops in with speed,
Transcription streams at blinding feed,
Bedrock's S3 dance so sublime! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	No description was provided by the author; the PR is submitted with only the default template and no filled-in details.	Add a summary explaining the purpose of the backmerge, key changes included, affected areas, and testing steps.
Title check	❓ Inconclusive	The title 'main backmerge' is generic and vague; it does not clearly describe the specific changes in the PR.	Replace with a descriptive title summarizing the main changes, e.g., 'Add batch and file operations, HuggingFace provider, and Go 1.25.5 upgrades'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 90.98% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch main

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 13

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

core/providers/bedrock/responses.go (2)

1432-1439: The assumption about nil-context handling is incorrect—the function will panic if ctx is nil

While ToBedrockResponsesRequest passes ctx to convertTextFormatToTool, the latter does not treat nil context defensively. At line 623 of utils.go, the function dereferences ctx without checking: (*ctx) = context.WithValue(*ctx, ...). This would cause a nil pointer panic if ctx were nil, though in practice the function receives non-nil context from its callers.

The code change itself (passing ctx down to convertTextFormatToTool for structured output formatting) is reasonable and works correctly given the current call patterns.

---

1206-1223: Guard against nil InferenceConfig/MaxTokens when deriving reasoning effort

At line 1327 in ToBifrostResponsesRequest, the code dereferences *request.InferenceConfig.MaxTokens without checking if either is nil:

effort := providerUtils.GetReasoningEffortFromBudgetTokens(maxTokens, minBudgetTokens, *request.InferenceConfig.MaxTokens)

If Bedrock returns reasoning_config in AdditionalModelRequestFields but omits InferenceConfig or MaxTokens, this will panic. The code at line 1226 guards InferenceConfig only until line 1242, while reasoning config handling starts at line 1310, well outside that guard.

Use the fallback pattern already employed at line 1481-1487:

+completionMax := DefaultCompletionMaxTokens
+if request.InferenceConfig != nil && request.InferenceConfig.MaxTokens != nil {
+    completionMax = *request.InferenceConfig.MaxTokens
+}
-effort := providerUtils.GetReasoningEffortFromBudgetTokens(maxTokens, minBudgetTokens, *request.InferenceConfig.MaxTokens)
+effort := providerUtils.GetReasoningEffortFromBudgetTokens(maxTokens, minBudgetTokens, completionMax)

This prevents crashes while maintaining backward compatibility.

🧹 Nitpick comments (42)

core/providers/elevenlabs/elevenlabs.go (1)

525-534: Enhance error metadata for consistency.

The empty response validation is valuable, but the error structure lacks ExtraFields (Provider, Model, RequestType) that other errors in this file include. This is inconsistent with the richer error context pattern applied throughout this changeset.

🔎 Suggested improvement for error consistency

 // Check for empty response
 trimmed := strings.TrimSpace(string(responseBody))
 if len(trimmed) == 0 {
   return nil, &schemas.BifrostError{
     IsBifrostError: true,
     Error: &schemas.ErrorField{
       Message: schemas.ErrProviderResponseEmpty,
     },
+    ExtraFields: &schemas.BifrostResponseExtraFields{
+      Provider:       providerName,
+      ModelRequested: request.Model,
+      RequestType:    schemas.TranscriptionRequest,
+    },
   }
 }

core/providers/huggingface/chat.go (1)

11-14: Consider documenting the (nil, nil) return semantics.

Returning (nil, nil) when input is nil or missing defers error handling to the caller. This pattern is used elsewhere in the codebase, but callers must explicitly check for a nil response even when no error is returned. Consider adding a doc comment clarifying this behavior.

core/providers/huggingface/transcription.go (1)

143-147: Timestamp extraction silently falls back to zero values.

When chunk.Timestamp has fewer than 2 elements, start and end default to 0.0. This is a reasonable fallback, but consider whether a warning log or preserving the partial timestamp would be more appropriate for debugging incomplete transcription responses.

core/internal/testutil/prompt_caching.go (1)

342-353: Consider using GetTestRetryConfigForScenario() for consistency.

Based on learnings, the convention in core/internal/testutil is to use GetTestRetryConfigForScenario() to obtain a generic retry config, then construct a typed retry config. However, this manual construction appears intentional here with specific values for the prompt caching scenario (5 attempts, 2s base delay). If this is intentional, consider adding a brief comment explaining why.

core/providers/mistral/mistral.go (1)

471-474: Handle multiple done event type variations.

The code correctly checks for both MistralTranscriptionStreamEventDone and the string literal "transcript.text.done". Consider defining the latter as a constant if it's a documented Mistral event type, or adding a comment explaining why both are checked.

🔎 Suggested improvement

+// MistralTranscriptionStreamEventTextDone handles alternate done event type
+MistralTranscriptionStreamEventTextDone MistralTranscriptionStreamEventType = "transcript.text.done"

Then use the constant in the check:

if eventType == MistralTranscriptionStreamEventDone || eventType == MistralTranscriptionStreamEventTextDone {

core/bifrost.go (1)

873-898: Minor inconsistency: Missing ExtraFields in error responses.

The BatchCreateRequest validation errors (lines 876-898) don't include ExtraFields with RequestType and Provider, unlike BatchResultsRequest (lines 1047-1073). Consider adding for consistency:

🔎 Suggested fix for consistency

 func (bifrost *Bifrost) BatchCreateRequest(ctx context.Context, req *schemas.BifrostBatchCreateRequest) (*schemas.BifrostBatchCreateResponse, *schemas.BifrostError) {
 	if req == nil {
 		return nil, &schemas.BifrostError{
 			IsBifrostError: false,
 			Error: &schemas.ErrorField{
 				Message: "batch create request is nil",
 			},
+			ExtraFields: schemas.BifrostErrorExtraFields{
+				RequestType: schemas.BatchCreateRequest,
+			},
 		}
 	}

core/internal/testutil/utils.go (1)

666-668: Redundant nil check, but harmless.

The err.Error != nil check on line 666 is redundant since the function already returns early at lines 648-654 when err.Error is nil. This defensive check doesn't cause any issues but could be simplified.

🔎 Optional simplification

 	errorCode := ""
-	if err.Error != nil && err.Error.Code != nil && *err.Error.Code != "" {
+	if err.Error.Code != nil && *err.Error.Code != "" {
 		errorCode = *err.Error.Code
 	}

core/internal/testutil/transcription_stream.go (1)

65-73: Consider extracting repeated TTS provider/model resolution logic.

This 8-line pattern for resolving speechSynthesisProvider and speechSynthesisModel with external overrides is repeated four times in this file (lines 65-73, 347-355, 442-450, 541-549). While the logic is correct, extracting it into a helper would reduce duplication.

🔎 Suggested helper function

// getTTSProviderAndModel resolves the TTS provider and model, preferring external overrides.
func getTTSProviderAndModel(testConfig ComprehensiveTestConfig) (schemas.ModelProvider, string) {
	provider := testConfig.Provider
	if testConfig.ExternalTTSProvider != "" {
		provider = testConfig.ExternalTTSProvider
	}
	
	model := testConfig.SpeechSynthesisModel
	if testConfig.ExternalTTSModel != "" {
		model = testConfig.ExternalTTSModel
	}
	
	return provider, model
}

Usage:

speechSynthesisProvider, speechSynthesisModel := getTTSProviderAndModel(testConfig)

core/providers/azure/files.go (1)

36-59: Consider adding rawRequest support for consistency.

The function handles sendBackRawResponse to optionally populate RawResponse, but RawRequest is never set. Looking at BifrostResponseExtraFields (from relevant snippets), both RawRequest and RawResponse fields exist. If the caller has access to the raw request, consider extending the signature to include it for debugging parity.

This is a minor suggestion since the current implementation works correctly for response-only scenarios.

🔎 Optional: Add rawRequest parameter

-func (r *AzureFileResponse) ToBifrostFileUploadResponse(providerName schemas.ModelProvider, latency time.Duration, sendBackRawResponse bool, rawResponse interface{}) *schemas.BifrostFileUploadResponse {
+func (r *AzureFileResponse) ToBifrostFileUploadResponse(providerName schemas.ModelProvider, latency time.Duration, sendBackRawRequest bool, rawRequest interface{}, sendBackRawResponse bool, rawResponse interface{}) *schemas.BifrostFileUploadResponse {
 	resp := &schemas.BifrostFileUploadResponse{
 		// ... existing fields ...
 	}

+	if sendBackRawRequest {
+		resp.ExtraFields.RawRequest = rawRequest
+	}
 	if sendBackRawResponse {
 		resp.ExtraFields.RawResponse = rawResponse
 	}

 	return resp
 }

core/providers/huggingface/embedding.go (1)

11-14: Returning (nil, nil) for nil input may hide errors upstream.

Returning (nil, nil) when bifrostReq is nil could silently propagate through the call chain, making it harder to diagnose issues where a nil request was unexpectedly passed. Consider returning an error instead for clearer debugging.

🔎 Optional: Return error for nil input

 func ToHuggingFaceEmbeddingRequest(bifrostReq *schemas.BifrostEmbeddingRequest) (*HuggingFaceEmbeddingRequest, error) {
 	if bifrostReq == nil {
-		return nil, nil
+		return nil, fmt.Errorf("bifrost embedding request is nil")
 	}

core/providers/bedrock/chat.go (1)

115-127: Minor: Trailing newline in accumulated reasoning text.

The reasoning text accumulation appends "\n" after each block (line 126), resulting in a trailing newline. This is typically harmless but may cause unexpected whitespace in downstream consumers.

🔎 Optional: Trim trailing newline

 	if len(reasoningDetails) > 0 {
 		if assistantMessage == nil {
 			assistantMessage = &schemas.ChatAssistantMessage{}
 		}
 		assistantMessage.ReasoningDetails = reasoningDetails
-		assistantMessage.Reasoning = schemas.Ptr(reasoningText)
+		assistantMessage.Reasoning = schemas.Ptr(strings.TrimSuffix(reasoningText, "\n"))
 	}

Note: This would require adding "strings" to imports.

core/providers/huggingface/responses.go (1)

39-42: Side effect: Input response is mutated.

Setting resp.Model directly mutates the input BifrostChatResponse. If the caller reuses this response object elsewhere, they may see unexpected model changes. Consider assigning to responsesResp.Model after conversion instead.

🔎 Optional: Avoid mutating input

 func ToBifrostResponsesResponseFromHuggingFace(resp *schemas.BifrostChatResponse, requestedModel string) (*schemas.BifrostResponsesResponse, error) {
 	if resp == nil {
 		return nil, nil
 	}

-	// Ensure model is set
-	if resp.Model == "" {
-		resp.Model = requestedModel
-	}
-
 	responsesResp := resp.ToBifrostResponsesResponse()
 	if responsesResp != nil {
+		// Ensure model is set
+		if responsesResp.Model == "" {
+			responsesResp.Model = requestedModel
+		}
 		responsesResp.ExtraFields.Provider = schemas.HuggingFace
 		responsesResp.ExtraFields.ModelRequested = requestedModel
 		responsesResp.ExtraFields.RequestType = schemas.ResponsesRequest
 	}

 	return responsesResp, nil
 }

core/providers/huggingface/speech.go (1)

9-16: Consider returning an error for nil request instead of nil, nil.

Returning nil, nil for a nil request is ambiguous - callers cannot distinguish between "no request" and "successful conversion with nil result". Other converters in the codebase (e.g., in the Anthropic files.go being reviewed) don't follow this pattern. Consider returning an error for nil input for clearer error handling.

🔎 Suggested approach

 func ToHuggingFaceSpeechRequest(request *schemas.BifrostSpeechRequest) (*HuggingFaceSpeechRequest, error) {
 	if request == nil {
-		return nil, nil
+		return nil, fmt.Errorf("speech request cannot be nil")
 	}

core/providers/gemini/batch.go (3)

16-52: Batch status and timestamp helpers look correct; consider clarifying extractBatchIDFromName semantics

ToBifrostBatchStatus and parseGeminiTimestamp are straightforward and safe (returning zero on parse failure avoids panics). extractBatchIDFromName currently returns the full resource name unchanged, which is fine if downstream expects "batches/abc123" as the ID; if callers intend to work only with the bare ID ("abc123"), consider either renaming this helper or adding a short doc comment explaining that it intentionally returns the full resource name to avoid confusion.

---

54-136: Message extraction in buildBatchRequestItems is defensive but could support more input shapes

The conversion from BatchRequestItem to GeminiBatchRequestItem is careful about multiple messages representations ([]interface{}, []map[string]interface{}, []map[string]string) and handles roles and content safely. Two minor robustness ideas:

• requestData["messages"] coming from generic JSON unmarshalling will often be []interface{}; you already handle that, but if other code injects []string or []any with non‑map elements, they’re silently dropped. If that’s a realistic input, consider logging or returning a structured error instead of just sending an empty Contents.
• Only string content is supported; if future callers pass richer content (e.g., arrays of blocks or images), this helper will quietly ignore it. A brief comment noting that this is intentionally text‑only right now would help prevent misuse.

Functionally this is fine for current text‑only batch use.

---

138-263: Batch results download/parse path is solid; minor suggestions for resilience

The download + JSONL parse flow is well-structured: correct URL rewriting, key header, error handling via MakeRequestWithContext, gunzip support, and per-line error collection via ParseJSONL. A few small improvements you might consider:

• When resultLine.Key is empty, you synthesize request-%d using len(results) before appending; that’s deterministic but depends on parse order. If you ever need stable IDs across retries or different parsers, anchoring to a line number (from ParseJSONL) would be slightly clearer.
• respBody["finish_reason"] is set to string(candidate.FinishReason) without guarding for zero values; if FinishReason is its zero value this is still safe, but you could omit the key entirely when it’s empty to keep the body minimal.
• Errors from individual lines are already accumulated in parseResult.Errors; if this function is used in batch-heavy paths, consider logging a short summary (e.g., number of parse errors) alongside the line-level warnings you already emit in the callback to aid debugging of partially-corrupt result files.

None of this blocks merging; the current implementation is correct and safe.

core/internal/testutil/test_retry_framework.go (1)

2153-2255: Responses streaming retry logging/flow improvements look correct

The added logging in WithResponsesStreamValidationRetry improves traceability without changing retry semantics:

• Start-of-attempt logs distinguish first vs subsequent attempts.
• On error or nil channel, you now log the failure (for attempts >1) plus an explicit “waiting before retry” line, while still using the same backoff (calculateRetryDelay(attempt-1, ...)).
• The continue // CRITICAL: Must continue to retry, not return in the nil responseChannel branch ensures retries are attempted rather than failing early, which aligns with the surrounding comment.

Control flow (retry conditions, delay calculation, and final-failure handling) remains intact.

Also applies to: 2316-2323

core/providers/anthropic/responses.go (2)

1344-1345: Centralized system-content conversion improves consistency; minor type nit

Wiring ConvertAnthropicMessagesToBifrostMessages through convertAnthropicSystemToBifrostMessages is a good refactor: Anthropic system content is now handled in one place and emitted as Bifrost ResponsesMessage with role=system.

One small consistency nit: in the ContentBlocks branch you currently emit blocks with Type: ResponsesOutputMessageContentTypeText, which conceptually denotes model output. Since these are input system prompts, using ResponsesInputMessageContentBlockTypeText would more accurately reflect their role and stay aligned with how you distinguish input vs output in other helpers. Behavior-wise it likely doesn’t break anything today (callers already treat both types as “text”), so this can be a low-priority clean-up.

Also applies to: 2065-2097

---

2152-2181: CacheControl propagation between Anthropic and Bifrost looks symmetric and non-breaking

You now consistently round-trip CacheControl:

• System text blocks and regular text blocks in convertAnthropicSystemToBifrostMessages, convertAnthropicContentBlocksToResponsesMessagesGrouped, and convertAnthropicContentBlocksToResponsesMessages attach block.CacheControl to ResponsesMessageContentBlock.CacheControl.
• Tool result content blocks (both function and MCP) preserve CacheControl on text content.
• convertContentBlockToAnthropic and convertBifrostContentBlocksToAnthropic mirror that back by copying CacheControl from Bifrost blocks into AnthropicContentBlock, including image blocks via toBifrostResponsesImageBlock and ConvertToAnthropicImageBlock.
• convertAnthropicToolToBifrost and convertBifrostToolToAnthropic now copy CacheControl on tools in both directions.

This keeps caching metadata intact across provider boundaries without changing behavior for callers that ignore it. The implementation is straightforward and in line with the rest of the conversions.

Also applies to: 2263-2266, 2377-2379, 2390-2395, 2623-2624, 3330-3333, 3343-3344, 3382-3383, 3082-3084, 3262-3264

core/providers/mistral/transcription.go (3)

13-41: ToMistralTranscriptionRequest is safe; consider more flexible granularities extraction

The converter correctly validates input (bifrostReq, Input, non-empty file) and maps core fields + language/prompt/response_format. Two minor robustness ideas:

• temperature extraction via SafeExtractFloat64Pointer is good; if you expect callers to also use a typed Params.Temperature field, you might additionally check that and prefer it over ExtraParams["temperature"].
• timestamp_granularities is asserted as []string; if ExtraParams ever comes from generic JSON unmarshalling it will likely be []interface{} instead. Supporting both ([]string and []interface{} with string elements) would make this more tolerant to different construction paths.

Current behavior is fine as long as those values are set programmatically as the expected types.

---

90-162: Multipart body creation is correct; guardrails are reasonable

createMistralTranscriptionMultipartBody and parseTranscriptionFormDataBodyFromRequest:

• Correctly create a file part and write the raw bytes.
• Always set a model field and add stream, language, prompt, response_format, temperature, and timestamp_granularities[] only when non-nil/present.
• Use NewBifrostOperationError to surface failures with provider context.
• Close the multipart writer exactly once inside parseTranscriptionFormDataBodyFromRequest, then use FormDataContentType() in the caller.

The only behavior change to consider later would be propagating an original filename if Bifrost ever tracks one; using a fixed "audio.mp3" is fine for now and avoids leaking user-provided names.

---

164-214: Streaming event → Bifrost mapping is straightforward; consider handling unknown events explicitly

The mapping from MistralTranscriptionStreamEvent to BifrostTranscriptionStreamResponse covers the expected cases:

• transcription.text.delta → Type=delta, with both Text and Delta populated.
• transcription.language → Type=delta with an empty Text, which is effectively a no-op payload but preserves the event boundary.
• transcription.segment → Type=delta with segment text in Text/Delta.
• transcription.done → Type=done with Usage populated from the usage struct.

Two small polish points:

• For language events, if BifrostTranscriptionStreamResponse ever grows a Language field, wiring e.Data.Language through would make that event more useful.
• For future-proofing, you might want to return nil for unrecognized e.Event values instead of an empty response struct (current default) so downstream code can easily skip unknown event types.

Functionally this is correct as written.

core/providers/bedrock/files.go (2)

59-135: XML parsing is fragile and doesn't handle edge cases.

The parseS3ListResponse function uses simple string matching for XML parsing, which has several issues:

1. No handling of CDATA sections - S3 keys can contain < or > characters which might be in CDATA.
2. No handling of XML entities - While html.UnescapeString is used for Key, Size and LastModified aren't entity-decoded.
3. Silent failure on malformed XML - Function returns nil even when XML is malformed, which could mask errors.
4. JSON fallback logic is weak - Only checks len(resp.Contents) > 0, but valid JSON with empty contents would still trigger XML parsing.

Consider using encoding/xml for proper XML parsing:

🔎 Recommended approach using encoding/xml

import "encoding/xml"

type s3ListBucketResult struct {
    XMLName               xml.Name    `xml:"ListBucketResult"`
    IsTruncated           bool        `xml:"IsTruncated"`
    NextContinuationToken string      `xml:"NextContinuationToken"`
    Contents              []s3Content `xml:"Contents"`
}

type s3Content struct {
    Key          string `xml:"Key"`
    Size         int64  `xml:"Size"`
    LastModified string `xml:"LastModified"`
}

func parseS3ListResponse(body []byte, resp *S3ListObjectsResponse) error {
    // Try JSON first
    if err := sonic.Unmarshal(body, resp); err == nil && len(resp.Contents) > 0 {
        return nil
    }
    
    // Parse XML properly
    var xmlResp s3ListBucketResult
    if err := xml.Unmarshal(body, &xmlResp); err != nil {
        return fmt.Errorf("failed to parse S3 response: %w", err)
    }
    
    resp.IsTruncated = xmlResp.IsTruncated
    resp.NextContinuationToken = xmlResp.NextContinuationToken
    
    for _, c := range xmlResp.Contents {
        t, _ := time.Parse(time.RFC3339Nano, c.LastModified)
        resp.Contents = append(resp.Contents, S3Object{
            Key:          c.Key,
            Size:         c.Size,
            LastModified: t,
        })
    }
    return nil
}

---

148-155: Hardcoded content type may be incorrect for non-JSONL files.

The ContentType is hardcoded to "application/jsonl", but the BifrostFileUploadResponse doesn't indicate the actual content type. This could be misleading for files that aren't JSONL format.

🔎 Suggested improvement

 return &BedrockFileUploadResponse{
     S3Uri:       resp.ID,
     Bucket:      bucket,
     Key:         key,
     SizeBytes:   resp.Bytes,
-    ContentType: "application/jsonl",
+    ContentType: "", // Let caller set based on actual file type, or infer from filename
     CreatedAt:   resp.CreatedAt,
 }

Alternatively, add content type to BifrostFileUploadResponse and pass it through.

core/internal/testutil/batch.go (3)

243-274: RunBatchResultsTest uses a hardcoded fake batch ID.

The test uses "test-batch-id" which will always fail. While this is noted in comments, it makes the test effectively a no-op. Consider either:

1. Skipping this test if there's no pre-existing completed batch
2. Using a config field for a known completed batch ID
3. Creating a batch and polling for completion (if feasible in test timeouts)

🔎 Alternative approach

func RunBatchResultsTest(t *testing.T, client *bifrost.Bifrost, ctx context.Context, testConfig ComprehensiveTestConfig) {
    if !testConfig.Scenarios.BatchResults {
        t.Logf("[SKIPPED] Batch Results: Not supported by provider %s", testConfig.Provider)
        return
    }

    // Skip if no pre-configured completed batch ID is available
    if testConfig.CompletedBatchID == "" {
        t.Logf("[SKIPPED] Batch Results: No completed batch ID configured for provider %s", testConfig.Provider)
        return
    }

    t.Run("BatchResults", func(t *testing.T) {
        request := &schemas.BifrostBatchResultsRequest{
            Provider: testConfig.Provider,
            BatchID:  testConfig.CompletedBatchID,
        }
        // ... actual test logic
    })
}

---

479-540: File delete test doesn't verify the file is actually gone.

After deleting the file, the test only checks response.Deleted is true, but doesn't verify the file is actually inaccessible. Consider adding a verification step.

🔎 Suggested verification

// After successful delete, verify file is gone
retrieveRequest := &schemas.BifrostFileRetrieveRequest{
    Provider: testConfig.Provider,
    FileID:   uploadResponse.ID,
}
_, retrieveErr := client.FileRetrieveRequest(ctx, retrieveRequest)
if retrieveErr == nil {
    t.Error("File should not be retrievable after deletion")
}

---

288-293: TODO comment for Azure skip should be tracked.

The TODO on line 288 indicates Azure batch support is pending. Consider creating an issue to track this.

Would you like me to help draft an issue to track enabling batch unsupported tests for Azure once it's officially supported?

core/providers/bedrock/batch.go (1)

379-395: Remove unused splitJSONL function.

The splitJSONL function is not called anywhere in the codebase. The parseBatchResultsJSONL function correctly uses providerUtils.ParseJSONL instead.

-// splitJSONL splits JSONL content into individual lines.
-func splitJSONL(data []byte) [][]byte {
-    var lines [][]byte
-    start := 0
-    for i, b := range data {
-        if b == '\n' {
-            if i > start {
-                lines = append(lines, data[start:i])
-            }
-            start = i + 1
-        }
-    }
-    if start < len(data) {
-        lines = append(lines, data[start:])
-    }
-    return lines
-}

core/providers/anthropic/anthropic.go (4)

413-431: Streaming helpers correctly pass metadata, but unused parseStreamAnthropicError is dead code

ChatCompletionStream and ResponsesStream now pass a RequestMetadata into the shared streaming helpers, and HTTP error paths call parseAnthropicError(resp, meta), which keeps error context consistent with non‑streaming flows. The helper parseStreamAnthropicError defined later is no longer used anywhere in this file, so it’s effectively dead code.

If nothing else in the package calls parseStreamAnthropicError, consider either wiring it into the streaming error path or removing it to avoid confusion.

Also applies to: 436-450, 488-492, 767-785, 790-804, 842-846

---

1033-1110: Anthropic Batch APIs look correct; consider consistent URL building and raw-response support*

The new BatchCreate, BatchList, BatchRetrieve, BatchCancel, and BatchResults implementations follow Anthropic’s endpoints and types and handle per‑key iteration and error parsing via ParseAnthropicError appropriately. Resource management for fasthttp.Request/Response in the per‑key loops is balanced.

Two follow‑ups worth considering:

• Some endpoints (e.g., BatchCancel and BatchResults) build URLs with provider.networkConfig.BaseURL + "/v1/messages/batches/..." instead of provider.buildRequestURL(...). That bypasses GetRequestPath/custom path overrides and can diverge from how other operations are routed.
• BatchList purposefully ignores rawRequest/rawResponse returned from HandleProviderResponse, even when sendBackRawResponse is true. If you expect callers to rely on raw payloads for debugging, you may want to plumb them into BifrostBatchListResponse.ExtraFields (similar to other providers).

Functionally this is fine; the above are consistency improvements.

Also applies to: 1112-1229, 1313-1526

---

1571-1576: Unused helpers (splitJSONL, parseStreamAnthropicError) can be removed or integrated

splitJSONL and parseStreamAnthropicError are defined but not invoked anywhere in this file, while JSONL parsing and streaming errors are handled via providerUtils.ParseJSONL and parseAnthropicError instead.

If these helpers are not referenced from other files in the anthropic package, it would be cleaner to drop them or replace the existing call sites to use them directly.

Also applies to: 1528-1544

---

1578-1657: Anthropic File APIs are generally correct; align URL construction with list/model calls*

The new FileUpload, FileList, FileRetrieve, FileDelete, and FileContent methods correctly implement Anthropic’s Files API semantics, including beta headers, multipart uploads, per‑key retries, and proper 200/201/204 handling.

Minor consistency nits:

• Some paths (e.g., FileDelete, FileContent) are built via provider.networkConfig.BaseURL + "/v1/files/..." instead of provider.buildRequestURL(...). Using the same helper as other operations would keep custom path overrides working uniformly.
• FileList/FileRetrieve correctly pass sendBackRawRequest/sendBackRawResponse through HandleProviderResponse and into the converter; FileList itself doesn’t surface raw payloads in ExtraFields. If raw responses from list operations are useful for debugging, you might want similar propagation there as well.

Behavior-wise this all looks fine; changes are mostly about consistency with the rest of the provider surface.

Also applies to: 1660-1783, 1785-2057

core/providers/bedrock/responses.go (2)

2458-2475: CacheControl/CachePoint round‑trip mapping looks symmetrical

The new logic:

• Adds BedrockSystemMessage{CachePoint: ...} when a system content block has CacheControl set.
• Converts BedrockSystemMessage.CachePoint back into CacheControlTypeEphemeral on the last content block of the last system message in convertBedrockSystemMessageToBifrostMessages.
• Emits BedrockContentBlock{CachePoint: ...} whenever a content block has CacheControl in convertBifrostResponsesMessageContentBlocksToBedrockContentBlocks.
• Applies the inverse in convertSingleBedrockMessageToBifrostMessages by setting CacheControl on the last content block of the last emitted Bifrost message when encountering a CachePoint.

This should maintain cache hints across Bedrock↔Bifrost conversions without changing visible content. The implementation is slightly stateful but consistent.

Also applies to: 2531-2565, 2567-2602

---

2458-2475: Minor duplication around createTextMessage and ID assignment

createTextMessage already assigns an ID when isOutputMessage is true, yet convertSingleBedrockMessageToBifrostMessages calls:

bifrostMsg := createTextMessage(block.Text, role, textBlockType, isOutputMessage)
if isOutputMessage {
    bifrostMsg.ID = schemas.Ptr("msg_" + fmt.Sprintf("%d", time.Now().UnixNano()))
}

and similarly re‑assigns IDs for structured‑output tool text. This double‑sets the ID and uses two different timestamps, which is harmless but redundant and a bit confusing.

Consider centralizing ID generation in createTextMessage (and passing isOutputMessage as today) or, alternatively, having createTextMessage never set IDs and leaving all ID creation in the callers.

Also applies to: 2567-2633

core/providers/gemini/gemini.go (2)

1629-1734: Gemini Batch APIs are functionally sound; prefer centralized path construction*

The new batchListByKey/BatchList, batchRetrieveByKey/BatchRetrieve, batchCancelByKey/BatchCancel, and batchResultsByKey/BatchResults methods correctly:

• Validate required fields (BatchID, keys).
• Iterate over keys and return on first success, logging failures.
• Use parseGeminiError with RequestMetadata so errors carry request context.
• Convert Gemini batch operations and stats into Bifrost batch responses, including status, request counts, latency, and inline vs file‑backed results.

Two consistency nits:

• URLs are built via fmt.Sprintf("%s/batches...", provider.networkConfig.BaseURL) or BaseURL + "/batches/..." instead of using providerUtils.GetPathFromContext/a shared helper like buildRequestURL. This bypasses per‑request path overrides you already use for /models/... endpoints and could surprise custom deployments.
• Several error‑handling branches assume err.Error on *schemas.BifrostError is non‑nil when logging; if any internal constructor ever leaves Error nil, those debug lines would panic.

Behavior is otherwise good; I’d recommend centralizing path construction for batch endpoints (possibly via a small buildBatchURL helper that wraps GetPathFromContext).

Also applies to: 1736-1810, 1812-1925, 1927-2022, 2046-2235

---

2237-2371: Gemini File APIs are well-shaped; align URL building with other endpoints*

The new file operations:

• FileUpload uses a multipart request with metadata + file and swaps /v1beta for /upload/v1beta when targeting the upload endpoint, which matches Gemini’s expected URL for default configs.
• fileListByKey/FileList, fileRetrieveByKey/FileRetrieve, and fileDeleteByKey/FileDelete implement per‑key retries, pagination, and status handling, mapping Gemini file metadata into Bifrost FileObject/FileResponse types and using parseGeminiError for failures.
• FileContent explicitly reports that Gemini Files API doesn’t support direct downloads and instructs callers to use the URI instead, which is accurate and better than a half‑implemented stub.

Two follow‑ups to consider:

• Like the batch endpoints, these methods build URLs directly off provider.networkConfig.BaseURL (e.g., fmt.Sprintf("%s/files", BaseURL) or BaseURL+"/"+fileID) instead of routing through GetPathFromContext or a shared helper. If you support path overrides for Gemini elsewhere, centralizing this will keep behavior consistent.
• FileUpload’s strings.Replace(BaseURL, "/v1beta", "/upload/v1beta", 1) works for the default base, but becomes fragile if the base path changes (e.g., new version or custom proxy). Wrapping this in a small helper that understands versions or reads from config would make it easier to evolve later.

Functionally the implementations look good; these are mostly about long‑term maintainability and configurability.

Also applies to: 2373-2474, 2476-2550, 2552-2664, 2666-2746, 2748-2764

core/providers/huggingface/huggingface.go (1)

136-145: Consider extracting duplicated model update logic.

The model name update logic (lines 136-145) is duplicated in the retry path (lines 161-169). Consider extracting this into a helper function to improve maintainability.

Additionally, the condition on line 136 only updates the model field for embedding requests (requestType == schemas.EmbeddingRequest). Verify that other request types (chat, transcription, speech) don't require similar model field updates in their JSON payloads.

🔎 Suggested refactor

+func updateModelInJSONBody(jsonData []byte, modelName string) ([]byte, error) {
+	var reqBody map[string]interface{}
+	if err := sonic.Unmarshal(jsonData, &reqBody); err != nil {
+		return jsonData, err
+	}
+	reqBody["model"] = modelName
+	return sonic.Marshal(reqBody)
+}
+
 func (provider *HuggingFaceProvider) completeRequestWithModelAliasCache(
 	// ... parameters
 ) ([]byte, time.Duration, *schemas.BifrostError) {
 	// ... existing code ...
 	
-	// Update the model field in the JSON body if it's not an audio request
-	updatedJSONData := jsonData
-	if !isHFInferenceAudioRequest && requestType == schemas.EmbeddingRequest {
-		// Parse, update model field, and re-encode for embedding requests
-		var reqBody map[string]interface{}
-		if err := sonic.Unmarshal(jsonData, &reqBody); err == nil {
-			reqBody["model"] = modelName
-			if newJSON, err := sonic.Marshal(reqBody); err == nil {
-				updatedJSONData = newJSON
-			}
-		}
-	}
+	updatedJSONData := jsonData
+	if !isHFInferenceAudioRequest && requestType == schemas.EmbeddingRequest {
+		if newJSON, err := updateModelInJSONBody(jsonData, modelName); err == nil {
+			updatedJSONData = newJSON
+		}
+	}
 	
 	// ... existing code ...
 	
 	if err.StatusCode != nil && *err.StatusCode == 404 {
 		// ... existing code ...
 		
-		// Update the model field in the JSON body for retry
-		if !isHFInferenceAudioRequest && requestType == schemas.EmbeddingRequest {
-			var reqBody map[string]interface{}
-			if err := sonic.Unmarshal(jsonData, &reqBody); err == nil {
-				reqBody["model"] = modelName
-				if newJSON, err := sonic.Marshal(reqBody); err == nil {
-					updatedJSONData = newJSON
-				}
-			}
-		}
+		if !isHFInferenceAudioRequest && requestType == schemas.EmbeddingRequest {
+			if newJSON, err := updateModelInJSONBody(jsonData, modelName); err == nil {
+				updatedJSONData = newJSON
+			}
+		}
 	}
 }

Also applies to: 161-169

core/internal/testutil/transcription.go (2)

63-99: Fal‑AI/HuggingFace branching and retry wiring look correct; consider extracting a shared fixture helper.

The provider/model selection and the HuggingFace fal-ai/ path (reading pre-generated mp3s) are wired correctly, and the retry configs follow the GetTestRetryConfigForScenario → typed config pattern (copying only MaxAttempts/BaseDelay/MaxDelay and using empty Conditions), which matches the existing testutil convention. Based on learnings, this is aligned.

You do repeat essentially the same runtime.Caller + filepath.Join(dir, "scenarios", "media", ...) logic in the round-trip, AdditionalAudioTests, TranscriptionAdvanced, and MultipleLanguages branches. If you want to trim duplication later, a small loadFalFixture(t, name string) []byte helper that centralizes the path construction and error handling would keep these tests tighter without changing behavior.

Also applies to: 100-177

---

151-160: Temp file naming is shared across providers; safe but slightly noisy.

Writing TTS audio to os.TempDir() with a name derived only from the subtest (roundtrip_<name>.<format>) means different providers running these tests in parallel may overwrite each other’s temp files. Since the tests never read the file back, this is harmless, but if you ever start inspecting these files post‑hoc, consider including the provider (or t.Name()) in the filename to avoid collisions.

core/providers/mistral/transcription_test.go (1)

1421-1463: Two nearly identical formatFloat64 tests can be consolidated.

TestFormatFloat64EdgeCases and TestFormatFloat64 both exercise the same formatting behavior with overlapping cases. You can keep just one of them and merge their tables to reduce maintenance overhead.

Suggested consolidation

-func TestFormatFloat64EdgeCases(t *testing.T) {
-    t.Parallel()
-    tests := []struct {
-        input    float64
-        expected string
-    }{
-        {0.0, "0"},
-        {0.5, "0.5"},
-        {1.0, "1"},
-        {1.23456, "1.23456"},
-        {-0.5, "-0.5"},
-        {0.123456789, "0.123456789"},
-        {100.0, "100"},
-        {0.001, "0.001"},
-    }
-    for _, tt := range tests {
-        result := formatFloat64(tt.input)
-        assert.Equal(t, tt.expected, result, "formatFloat64(%f)", tt.input)
-    }
-}
-
-func TestFormatFloat64(t *testing.T) {
+func TestFormatFloat64(t *testing.T) {
     t.Parallel()
     tests := []struct {
         input    float64
         expected string
     }{
         {0.0, "0"},
         {0.5, "0.5"},
         {1.0, "1"},
         {1.23456, "1.23456"},
         {-0.5, "-0.5"},
+        {0.123456789, "0.123456789"},
+        {100.0, "100"},
+        {0.001, "0.001"},
     }
     for _, tt := range tests {
         result := formatFloat64(tt.input)
         assert.Equal(t, tt.expected, result, "formatFloat64(%f)", tt.input)
     }
 }

core/providers/anthropic/batch.go (1)

131-170: Align ExpiresAt handling between create and retrieve conversions.

ToBifrostBatchCreateResponse always sets ExpiresAt to a pointer, even when parseAnthropicTimestamp(r.ExpiresAt) returns 0 (empty or invalid timestamp), while ToBifrostBatchRetrieveResponse only assigns ExpiresAt when the parsed value is > 0. For consistency with the retrieve path and to avoid treating “missing” as Unix epoch, consider guarding the create path as well:

Suggested adjustment

-	expiresAt := parseAnthropicTimestamp(r.ExpiresAt)
-	resp := &schemas.BifrostBatchCreateResponse{
+	expiresAt := parseAnthropicTimestamp(r.ExpiresAt)
+	resp := &schemas.BifrostBatchCreateResponse{
 		ID:               r.ID,
 		Object:           ToBifrostObjectType(r.Type),
 		Status:           ToBifrostBatchStatus(r.ProcessingStatus),
 		ProcessingStatus: &r.ProcessingStatus,
-		ResultsURL:       r.ResultsURL,
-		CreatedAt:        parseAnthropicTimestamp(r.CreatedAt),
-		ExpiresAt:        &expiresAt,
+		ResultsURL:       r.ResultsURL,
+		CreatedAt:        parseAnthropicTimestamp(r.CreatedAt),
 		ExtraFields: schemas.BifrostResponseExtraFields{
 			RequestType: schemas.BatchCreateRequest,
 			Provider:    providerName,
 			Latency:     latency.Milliseconds(),
 		},
 	}
+
+	if expiresAt > 0 {
+		resp.ExpiresAt = &expiresAt
+	}

core/providers/anthropic/types.go (1)

531-587: Bifrost file upload/retrieve conversions are straightforward; note the hard‑coded purpose/status.

ToBifrostFileUploadResponse and ToBifrostFileRetrieveResponse:

• Map ID, type, size, created_at (via parseAnthropicFileTimestamp), and filename.
• Hardcode Purpose to FilePurposeBatch, Status to FileStatusProcessed, and StorageBackend to FileStorageAPI.
• Attach BifrostResponseExtraFields with the appropriate file request type, provider, and latency, plus optional raw request/response.

If Anthropic later differentiates file purposes or statuses, you may want to thread that through instead of hardcoding, but for current “batch file only” usage this is acceptable.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Verify SDK URL availability—the primary source does not host macOS 12.3.

The phracker/MacOSX-SDKs repository contains SDKs compiled from various Xcode releases, covering up to MacOSX11.3.sdk. The script's primary URL references version 12.3 from this repository, which is unavailable. The joseluisq/macosx-sdks repository provides the macOS 12.3 SDK at https://github.com/joseluisq/macosx-sdks/releases/download/12.3/MacOSX12.3.sdk.tar.xz, as do alternative sources. Ensure your download script has appropriate fallback handling or update to use an available source. The SDK upgrade from 11.3 to 12.3 may also affect your project's minimum supported macOS version for built binaries.

🤖 Prompt for AI Agents

.github/workflows/scripts/install-cross-compilers.sh lines 30-31: the script
points to a nonexistent SDK URL in the phracker repo for MacOSX12.3.sdk; replace
that URL with a known working mirror (for example joseluisq's release URL) or
add fallback logic that attempts the primary URL then retries one or more
alternate URLs (e.g., joseluisq/macosx-sdks) and exits with a clear error if all
attempts fail; also ensure any change documents that switching to SDK 12.3 may
raise the minimum supported macOS deployment target so update build flags or CI
variables accordingly.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Duplicate "Setup Python" step detected.

There are two consecutive "Setup Python (for tests tooling)" steps (lines 87-95 and 92-99). The second step at lines 92-99 includes cache configuration while the first at 87-91 does not. This appears to be an unintended duplication—the first step can likely be removed.

🔎 Proposed fix: Remove duplicate step

-      - name: Setup Python (for tests tooling)
-        uses: actions/setup-python@v5
-        with:
-          python-version: "3.11"
-
       - name: Setup Python (for tests tooling)
         uses: actions/setup-python@v5
         with:
           python-version: "3.11"
           cache: "pip"
           cache-dependency-path: |
             tests/integrations/requirements.txt
             tests/governance/requirements.txt

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	- name: Setup Python (for tests tooling)
	uses: actions/setup-python@v5
	with:
	python-version: '3.11'
	python-version: "3.11"
	- name: Setup Python (for tests tooling)
	uses: actions/setup-python@v5
	with:
	python-version: '3.11'
	cache: 'pip'
	python-version: "3.11"
	cache: "pip"
	cache-dependency-path: |
	tests/integrations/requirements.txt
	tests/governance/requirements.txt
	- name: Setup Python (for tests tooling)
	uses: actions/setup-python@v5
	with:
	python-version: "3.11"
	cache: "pip"
	cache-dependency-path: |
	tests/integrations/requirements.txt
	tests/governance/requirements.txt

🤖 Prompt for AI Agents

.github/workflows/snyk.yml around lines 87 to 100: there are two consecutive
"Setup Python (for tests tooling)" steps; remove the first duplicate (lines
~87-91) so only the second step with the cache and cache-dependency-path
remains, ensuring no other steps rely on the removed stub; keep the
setup-python@v5 usage with python-version "3.11" and cache settings in the
remaining step.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check Nebius presence in the account.go file
echo "=== Checking for all Nebius mentions in account.go ==="
grep -n "Nebius" core/internal/testutil/account.go | head -20

echo ""
echo "=== Checking line count to understand file structure ==="
wc -l core/internal/testutil/account.go

echo ""
echo "=== Looking at AllProviderConfigs definition ==="
grep -n "AllProviderConfigs" core/internal/testutil/account.go

Repository: maximhq/bifrost

Length of output: 463

---

🏁 Script executed:

#!/bin/bash
# Read the AllProviderConfigs variable to see what providers are included
echo "=== Reading AllProviderConfigs structure ==="
sed -n '621,750p' core/internal/testutil/account.go

Repository: maximhq/bifrost

Length of output: 5652

---

🏁 Script executed:

#!/bin/bash
# Continue reading to see all provider configs
echo "=== Reading more of AllProviderConfigs ==="
sed -n '750,1055p' core/internal/testutil/account.go | head -100

Repository: maximhq/bifrost

Length of output: 3910

---

🏁 Script executed:

#!/bin/bash
# Search for all provider entries in AllProviderConfigs
echo "=== All Provider entries in AllProviderConfigs ==="
grep -n "Provider:" core/internal/testutil/account.go | grep -A 1 "622:" | head -50

echo ""
echo "=== Check if Nebius appears in AllProviderConfigs section ==="
sed -n '621,1055p' core/internal/testutil/account.go | grep -c "schemas.Nebius"

echo ""
echo "=== List all providers in AllProviderConfigs ==="
sed -n '621,1055p' core/internal/testutil/account.go | grep "Provider:" | sed 's/.*Provider: *//' | sed 's/[,].*//'

Repository: maximhq/bifrost

Length of output: 634

---

🏁 Script executed:

#!/bin/bash
# Verify Nebius is a valid provider in schemas
echo "=== Check if Nebius exists in schemas ==="
fd -t f -e go . | xargs grep -l "Nebius" | grep -i schema | head -5

echo ""
echo "=== Check the Nebius case in GetConfigForProvider ==="
sed -n '603,620p' core/internal/testutil/account.go

Repository: maximhq/bifrost

Length of output: 681

---

🏁 Script executed:

#!/bin/bash
# Check what GetConfiguredProviders includes
echo "=== Checking GetConfiguredProviders function ==="
sed -n '110,130p' core/internal/testutil/account.go

echo ""
echo "=== Check if there's a pattern - are ALL providers in GetConfiguredProviders in AllProviderConfigs? ==="
echo "Providers in GetConfiguredProviders:"
sed -n '110,130p' core/internal/testutil/account.go | grep "schemas\." | sed 's/.*schemas\./schemas\./' | sed 's/[,].*//'

Repository: maximhq/bifrost

Length of output: 1006

---

🏁 Script executed:

#!/bin/bash
# Check if other providers from GetConfiguredProviders are in AllProviderConfigs
echo "=== Checking for Perplexity, Cerebras in AllProviderConfigs ==="
sed -n '621,1055p' core/internal/testutil/account.go | grep -E "Perplexity|Cerebras" | wc -l

echo ""
echo "=== Get complete list of providers from GetConfiguredProviders ==="
sed -n '110,125p' core/internal/testutil/account.go

echo ""
echo "=== Check the context - what is AllProviderConfigs used for? ==="
grep -B 5 -A 5 "AllProviderConfigs" core/internal/testutil/account.go | head -30

Repository: maximhq/bifrost

Length of output: 1157

---

🏁 Script executed:

#!/bin/bash
# Let's check how many providers are in each function
echo "=== Summary of provider presence ==="
echo ""
echo "Providers in GetConfiguredProviders:"
sed -n '110,125p' core/internal/testutil/account.go | grep "schemas\." | grep -v "//" | wc -l

echo ""
echo "Providers in AllProviderConfigs:"
sed -n '621,1055p' core/internal/testutil/account.go | grep "Provider:" | wc -l

echo ""
echo "=== Check stack - are there related PRs? ==="
git log --oneline -20 | head -20

Repository: maximhq/bifrost

Length of output: 299

---

🏁 Script executed:

#!/bin/bash
# Verify the claim more directly - check case statements
echo "=== Check how many case statements for providers in GetKeysForProvider ==="
sed -n '130,600p' core/internal/testutil/account.go | grep -c "case schemas\."

echo ""
echo "=== Check case statements in GetConfigForProvider ==="
sed -n '600,620p' core/internal/testutil/account.go | grep -c "case schemas\."

echo ""
echo "=== List all case statements in GetConfigForProvider ==="
sed -n '550,620p' core/internal/testutil/account.go | grep "case schemas\."

Repository: maximhq/bifrost

Length of output: 372

---

🏁 Script executed:

#!/bin/bash
# Check the complete list of case statements in GetKeysForProvider to understand all providers
echo "=== All case statements in GetKeysForProvider ==="
sed -n '130,600p' core/internal/testutil/account.go | grep "case schemas\." | sed 's/.*case //' | sed 's/:.*//' | sort -u

echo ""
echo "=== Comparing: Providers in GetConfiguredProviders vs AllProviderConfigs ==="
echo "GetConfiguredProviders providers:"
sed -n '110,125p' core/internal/testutil/account.go | grep -oE "schemas\.[A-Za-z]+" | sort -u

echo ""
echo "AllProviderConfigs providers:"
sed -n '621,1055p' core/internal/testutil/account.go | grep -oE "schemas\.[A-Za-z]+" | sort -u | head -20

Repository: maximhq/bifrost

Length of output: 903

---

🏁 Script executed:

#!/bin/bash
# Check how AllProviderConfigs is used in the codebase
echo "=== Usage of AllProviderConfigs ==="
fd -t f -e go . | xargs grep -l "AllProviderConfigs" | head -10

echo ""
echo "=== How is AllProviderConfigs used? ==="
fd -t f -e go . | xargs grep -B 2 -A 2 "AllProviderConfigs" | head -50

Repository: maximhq/bifrost

Length of output: 540

---

Nebius provider missing from AllProviderConfigs, along with other newly added providers.

Nebius is added to GetConfiguredProviders() (line 115), GetKeysForProvider() (lines 336-344), and GetConfigForProvider() (lines 603-615), but is not included in AllProviderConfigs (starting line 621). This is part of a broader pattern: several providers (Perplexity, Cerebras, Elevenlabs, Parasail, SGL) also lack entries in AllProviderConfigs despite being in other functions.

AllProviderConfigs currently contains comprehensive test scenarios for only a subset of providers. If Nebius should have test scenario coverage, add a corresponding entry. Otherwise, update the variable comment from "all providers" to clarify that it only covers providers with comprehensive test scenarios defined.

🤖 Prompt for AI Agents

In core/internal/testutil/account.go around lines 621–625 (AllProviderConfigs)
the Nebius provider — along with Perplexity, Cerebras, Elevenlabs, Parasail, and
SGL — is missing from AllProviderConfigs even though it was added to
GetConfiguredProviders(), GetKeysForProvider() (lines 336–344), and
GetConfigForProvider() (lines 603–615); either add a corresponding test scenario
entry for Nebius (and the other newly added providers) following the existing
AllProviderConfigs entry pattern (provide a minimal config object keyed by
schemas.Nebius with representative fields used by tests, e.g., env-based key,
default models, weights, and flags) or, if you intentionally do not want
coverage, change the AllProviderConfigs comment to clearly state it only
contains providers with comprehensive test scenarios rather than “all
providers.” Ensure keys/matching enums align exactly with schemas constants and
run tests.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Add nil check to prevent potential nil pointer dereference.

The function dereferences resp without checking if it's nil first. This could cause a panic if called with a nil response.

🔎 Proposed fix

 func ToAnthropicFileUploadResponse(resp *schemas.BifrostFileUploadResponse) *AnthropicFileResponse {
+	if resp == nil {
+		return nil
+	}
 	return &AnthropicFileResponse{
 		ID:        resp.ID,

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	func ToAnthropicFileUploadResponse(resp *schemas.BifrostFileUploadResponse) *AnthropicFileResponse {
	return &AnthropicFileResponse{
	ID: resp.ID,
	Type: resp.Object,
	Filename: resp.Filename,
	MimeType: "",
	SizeBytes: resp.Bytes,
	CreatedAt: formatAnthropicFileTimestamp(resp.CreatedAt),
	}
	}
	func ToAnthropicFileUploadResponse(resp *schemas.BifrostFileUploadResponse) *AnthropicFileResponse {
	if resp == nil {
	return nil
	}
	return &AnthropicFileResponse{
	ID: resp.ID,
	Type: resp.Object,
	Filename: resp.Filename,
	MimeType: "",
	SizeBytes: resp.Bytes,
	CreatedAt: formatAnthropicFileTimestamp(resp.CreatedAt),
	}
	}

🤖 Prompt for AI Agents

In core/providers/anthropic/files.go around lines 10 to 19, the function
ToAnthropicFileUploadResponse dereferences resp without a nil check; add a guard
at the top that returns nil if resp == nil to avoid a nil pointer dereference,
then proceed to construct and return the AnthropicFileResponse as before (no
other behavior changes).

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Add nil check to prevent potential nil pointer dereference.

Similar to the upload response converter, this function should check for nil input before dereferencing.

🔎 Proposed fix

 func ToAnthropicFileListResponse(resp *schemas.BifrostFileListResponse) *AnthropicFileListResponse {
+	if resp == nil {
+		return nil
+	}
 	data := make([]AnthropicFileResponse, len(resp.Data))

🤖 Prompt for AI Agents

In core/providers/anthropic/files.go around lines 22 to 39, the converter
dereferences resp and resp.Data without nil checks which can cause a nil pointer
panic; add a guard at the top: if resp == nil return nil, and ensure resp.Data
is handled safely (e.g., if resp.Data == nil initialize data as an empty slice
or set length 0) before iterating, then proceed to build and return the
AnthropicFileListResponse using resp.HasMore.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

XML output is vulnerable to injection if bucket, prefix, or key contain special characters.

The ToS3ListObjectsV2XML function uses fmt.Sprintf to build XML, but doesn't escape special XML characters (<, >, &, ', "). This could produce malformed XML or enable XML injection.

🔎 Proposed fix using proper XML escaping

+import "encoding/xml"
+
+// escapeXML escapes special XML characters
+func escapeXML(s string) string {
+    var buf strings.Builder
+    xml.EscapeText(&buf, []byte(s))
+    return buf.String()
+}
+
 func ToS3ListObjectsV2XML(resp *schemas.BifrostFileListResponse, bucket, prefix string, maxKeys int) []byte {
     // ...
-    sb.WriteString(fmt.Sprintf("<Name>%s</Name>", bucket))
-    sb.WriteString(fmt.Sprintf("<Prefix>%s</Prefix>", prefix))
+    sb.WriteString(fmt.Sprintf("<Name>%s</Name>", escapeXML(bucket)))
+    sb.WriteString(fmt.Sprintf("<Prefix>%s</Prefix>", escapeXML(prefix)))
     // ... and similarly for Key, NextContinuationToken

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In core/providers/bedrock/files.go around lines 226-263, the XML is constructed
with fmt.Sprintf using user-controlled values (bucket, prefix, key,
NextContinuationToken) and is vulnerable to XML injection; instead of raw
formatting, escape all text nodes before writing them (bucket, prefix, key from
parseS3URI(f.ID), and resp.After token) using encoding/xml's escaping helpers
(e.g., xml.EscapeText) or equivalent, writing escaped bytes into the
strings.Builder so special characters (& < > ' ") are encoded; leave numeric and
known-safe fields (Size, IsTruncated, StorageClass, LastModified formatted
timestamps) as-is but ensure any string-derived value is escaped before
appending.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Same XML injection vulnerability in error XML output.

ToS3ErrorXML also uses fmt.Sprintf without escaping, which is vulnerable to XML injection if code, message, resource, or requestID contain special characters like < or &.

🔎 Proposed fix

 func ToS3ErrorXML(code, message, resource, requestID string) []byte {
     var sb strings.Builder
     sb.WriteString(`<?xml version="1.0" encoding="UTF-8"?>`)
     sb.WriteString("<Error>")
-    sb.WriteString(fmt.Sprintf("<Code>%s</Code>", code))
-    sb.WriteString(fmt.Sprintf("<Message>%s</Message>", message))
-    sb.WriteString(fmt.Sprintf("<Resource>%s</Resource>", resource))
-    sb.WriteString(fmt.Sprintf("<RequestId>%s</RequestId>", requestID))
+    sb.WriteString(fmt.Sprintf("<Code>%s</Code>", escapeXML(code)))
+    sb.WriteString(fmt.Sprintf("<Message>%s</Message>", escapeXML(message)))
+    sb.WriteString(fmt.Sprintf("<Resource>%s</Resource>", escapeXML(resource)))
+    sb.WriteString(fmt.Sprintf("<RequestId>%s</RequestId>", escapeXML(requestID)))
     sb.WriteString("</Error>")
     return []byte(sb.String())
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In core/providers/bedrock/files.go around lines 265 to 276, the ToS3ErrorXML
function constructs XML by interpolating unescaped values (code, message,
resource, requestID) which allows XML injection; update the function to escape
XML special characters for each input (e.g., use xml.EscapeString from
encoding/xml or html.EscapeString) before inserting into the XML builder so that
<, >, &, ' and " are encoded, and keep returning the assembled []byte of the
safe string.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Typo in function documentation comment.

Line 613 has "structured outpute" instead of "structured output".

🔎 Proposed fix

-// convertTextFormatToTool converts a text config to a Bedrock tool for structured outpute
+// convertTextFormatToTool converts a text config to a Bedrock tool for structured output

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// convertTextFormatToTool converts a text config to a Bedrock tool for structured outpute
	func convertTextFormatToTool(ctx *context.Context, textConfig *schemas.ResponsesTextConfig) *BedrockTool {
	if textConfig == nil || textConfig.Format == nil {
	return nil
	}
	format := textConfig.Format
	if format.Type != "json_schema" {
	return nil
	}
	toolName := "json_response"
	if format.Name != nil && strings.TrimSpace(*format.Name) != "" {
	toolName = strings.TrimSpace(*format.Name)
	}
	description := "Returns structured JSON output"
	if format.JSONSchema.Description != nil {
	description = *format.JSONSchema.Description
	}
	toolName = fmt.Sprintf("bf_so_%s", toolName)
	(*ctx) = context.WithValue(*ctx, schemas.BifrostContextKeyStructuredOutputToolName, toolName)
	var schemaObj any
	if format.JSONSchema != nil {
	schemaObj = *format.JSONSchema
	} else {
	return nil // Schema is required for Bedrock tooling
	}
	return &BedrockTool{
	ToolSpec: &BedrockToolSpec{
	Name: toolName,
	Description: schemas.Ptr(description),
	InputSchema: BedrockToolInputSchema{
	JSON: schemaObj,
	},
	},
	}
	}
	// convertTextFormatToTool converts a text config to a Bedrock tool for structured output
	func convertTextFormatToTool(ctx *context.Context, textConfig *schemas.ResponsesTextConfig) *BedrockTool {
	if textConfig == nil || textConfig.Format == nil {
	return nil
	}
	format := textConfig.Format
	if format.Type != "json_schema" {
	return nil
	}
	toolName := "json_response"
	if format.Name != nil && strings.TrimSpace(*format.Name) != "" {
	toolName = strings.TrimSpace(*format.Name)
	}
	description := "Returns structured JSON output"
	if format.JSONSchema.Description != nil {
	description = *format.JSONSchema.Description
	}
	toolName = fmt.Sprintf("bf_so_%s", toolName)
	(*ctx) = context.WithValue(*ctx, schemas.BifrostContextKeyStructuredOutputToolName, toolName)
	var schemaObj any
	if format.JSONSchema != nil {
	schemaObj = *format.JSONSchema
	} else {
	return nil // Schema is required for Bedrock tooling
	}
	return &BedrockTool{
	ToolSpec: &BedrockToolSpec{
	Name: toolName,
	Description: schemas.Ptr(description),
	InputSchema: BedrockToolInputSchema{
	JSON: schemaObj,
	},
	},
	}
	}

🤖 Prompt for AI Agents

In core/providers/bedrock/utils.go around lines 613 to 653, the function doc
comment for convertTextFormatToTool contains a typo ("structured outpute");
update the comment to read "structured output" (i.e., remove the trailing 'e')
so the documentation is correct and professional.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for ResponsesTextConfig struct definition and JSONSchema field type
rg "type ResponsesTextConfig struct" -A 30 core/providers/bedrock/

Repository: maximhq/bifrost

Length of output: 41

---

🏁 Script executed:

#!/bin/bash
# First, let's see what files exist in the bedrock directory
ls -la core/providers/bedrock/

# Search for ResponsesTextConfig in the codebase
rg "ResponsesTextConfig" core/providers/bedrock/ -B 5 -A 10

Repository: maximhq/bifrost

Length of output: 1884

---

🏁 Script executed:

#!/bin/bash
# View the specific lines mentioned in the review comment
sed -n '635,645p' core/providers/bedrock/utils.go

# Also search for where ResponsesTextConfig is defined (likely in schemas package)
find . -name "*.go" -type f | xargs grep -l "type ResponsesTextConfig struct" 2>/dev/null | head -5

Repository: maximhq/bifrost

Length of output: 386

---

🏁 Script executed:

#!/bin/bash
# Find ResponsesTextConfig struct definition and show JSONSchema field type
rg "type ResponsesTextConfig" -A 20 core/schemas/responses.go

# Also view how schemaObj is used after line 642 in utils.go
sed -n '642,670p' core/providers/bedrock/utils.go

Repository: maximhq/bifrost

Length of output: 2633

---

🏁 Script executed:

#!/bin/bash
# Find BedrockToolInputSchema definition
rg "type BedrockToolInputSchema" -A 5 core/providers/bedrock/

# Also check what type InputSchema.JSON expects
rg "InputSchema.*JSON" core/providers/bedrock/ -B 2 -A 2

Repository: maximhq/bifrost

Length of output: 1599

---

🏁 Script executed:

#!/bin/bash
# Check if there are any tests or examples showing expected usage of InputSchema.JSON
rg "InputSchema.*JSON" -B 5 -A 5 core/providers/bedrock/ | head -40

# Also look for how ResponsesTextConfigFormatJSONSchema.Schema is used elsewhere
rg "\.Schema" core/providers/bedrock/ -B 2 -A 2 | grep -i json -A 3 -B 3

Repository: maximhq/bifrost

Length of output: 2146

---

🏁 Script executed:

#!/bin/bash
# Check what format Bedrock expects for InputSchema
# Search for Bedrock API references or documentation
rg "InputSchema" -B 3 -A 3 core/providers/bedrock/bedrock.go | head -50

# Also check if there are tests that show expected structure
rg "InputSchema.*JSON" core/providers/bedrock/bedrock_test.go -B 5 -A 5

Repository: maximhq/bifrost

Length of output: 41

---

🏁 Script executed:

#!/bin/bash
# Let's look at the full convertTextFormatToTool function to understand the complete flow
sed -n '600,700p' core/providers/bedrock/utils.go

Repository: maximhq/bifrost

Length of output: 2902

---

🏁 Script executed:

#!/bin/bash
# Search for how ResponsesTextConfigFormatJSONSchema is used elsewhere
rg "ResponsesTextConfigFormatJSONSchema" core/ -B 3 -A 3

# Check what Bedrock API documentation or comments say about InputSchema format
rg "InputSchema" core/providers/bedrock/ -B 2 -A 2 | grep -i "json\|schema" -B 2 -A 2

Repository: maximhq/bifrost

Length of output: 7370

---

🏁 Script executed:

#!/bin/bash
# Look at how Gemini provider handles the same struct to understand the correct pattern
sed -n '1,50p' core/providers/gemini/utils.go | grep -A 30 "buildJSONSchemaFromMap"

# Also check Gemini responses to see how it reconstructs the schema
rg "reconstructSchemaFromJSONSchema" core/providers/gemini/responses.go -A 15

Repository: maximhq/bifrost

Length of output: 897

---

🏁 Script executed:

#!/bin/bash
# Check how the schemaObj is ultimately used when sent to Bedrock
rg "InputSchema.*JSON" core/providers/bedrock/bedrock.go -B 5 -A 5

# Also check Anthropic's implementation for comparison
rg "InputSchema" core/providers/anthropic/ -B 3 -A 3 | head -40

Repository: maximhq/bifrost

Length of output: 2302

---

🏁 Script executed:

#!/bin/bash
# Check if there's a test case for convertTextFormatToTool
rg "convertTextFormatToTool" core/providers/bedrock/ -B 2 -A 10

# Check what happens when the struct is serialized by comparing to the expected format
rg "ResponsesTextConfigFormatJSONSchema" core/providers/bedrock/bedrock_test.go -B 5 -A 10

Repository: maximhq/bifrost

Length of output: 1831

---

🏁 Script executed:

#!/bin/bash
# Let's see how the struct is actually serialized by checking JSON marshaling
rg "MarshalJSON" core/schemas/responses.go -A 20 | grep -A 20 "ResponsesTextConfigFormatJSONSchema"

# Also check if there's any test that validates the JSON output for this struct
rg "ResponsesTextConfigFormatJSONSchema" core/schemas/ -B 2 -A 2

Repository: maximhq/bifrost

Length of output: 969

---

🏁 Script executed:

#!/bin/bash
# Let's check if there are any comments or documentation about Bedrock's expected format
rg "Bedrock" core/providers/bedrock/types.go -B 2 -A 5 | grep -i "schema\|json" -B 2 -A 2

# Check the full ResponsesTextConfigFormatJSONSchema struct definition
sed -n '1,100p' core/schemas/responses.go | grep -A 25 "type ResponsesTextConfigFormatJSONSchema"

Repository: maximhq/bifrost

Length of output: 26633

---

Pass the actual schema definition to Bedrock, not the wrapper struct.

When format.JSONSchema != nil, extract the schema from the struct's Schema field (*format.JSONSchema.Schema) rather than dereferencing the entire struct. If using individual fields, reconstruct the schema map like the Gemini provider does (combining Type, Properties, Required, etc.) instead of passing the wrapper struct.

	var schemaObj any
	if format.JSONSchema != nil {
		if format.JSONSchema.Schema != nil {
			schemaObj = *format.JSONSchema.Schema
		} else {
			// Build schema map from individual fields like Gemini provider
			schemaObj = buildSchemaFromFields(format.JSONSchema)
		}
	} else {
		return nil // Schema is required for Bedrock tooling
	}

🤖 Prompt for AI Agents

In core/providers/bedrock/utils.go around lines 637-642, the code currently
dereferences the entire format.JSONSchema wrapper and passes that to Bedrock;
instead, when format.JSONSchema != nil extract the actual schema payload by
using format.JSONSchema.Schema (i.e., use *format.JSONSchema.Schema) and assign
that to schemaObj; if Schema is nil reconstruct a schema map from the wrapper's
individual fields (Type, Properties, Required, etc.) similar to the Gemini
provider and assign that map to schemaObj; keep the early return when
format.JSONSchema is nil.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Find the file and examine the context around lines 526-538
cd /tmp
find . -name "mistral.go" -type f 2>/dev/null | head -1

Repository: maximhq/bifrost

Length of output: 41

---

🏁 Script executed:

# Look for the mistral.go file
find . -name "mistral.go" 2>/dev/null | grep -E "(core/providers|mistral)" | head -5

Repository: maximhq/bifrost

Length of output: 94

---

🏁 Script executed:

# Search for BifrostError definition
rg "type BifrostError" -A 10 --type go 2>/dev/null | head -40

Repository: maximhq/bifrost

Length of output: 1270

---

🏁 Script executed:

# Look at the PR context - find git history
git log --oneline -20 2>/dev/null | head -20

Repository: maximhq/bifrost

Length of output: 139

---

🏁 Script executed:

# Check if this is part of a larger PR stack
git branch -a 2>/dev/null | head -20

Repository: maximhq/bifrost

Length of output: 174

---

🏁 Script executed:

# Find files in core/providers/mistral
fd mistral.go 2>/dev/null

Repository: maximhq/bifrost

Length of output: 92

---

🏁 Script executed:

# List contents of core/providers directory
ls -la core/providers/ 2>/dev/null || ls -la 2>/dev/null | head -20

Repository: maximhq/bifrost

Length of output: 1252

---

🏁 Script executed:

# Find and examine the mistral.go file
find . -path "*/core/providers/mistral/mistral.go" -type f 2>/dev/null | head -1 | xargs wc -l

Repository: maximhq/bifrost

Length of output: 98

---

🏁 Script executed:

# List the providers directory structure
find . -path "*/core/providers/*" -type d 2>/dev/null | head -20

Repository: maximhq/bifrost

Length of output: 533

---

🏁 Script executed:

# Search for BifrostError definition
find . -type f -name "*.go" | xargs grep -l "type BifrostError" 2>/dev/null | head -5

Repository: maximhq/bifrost

Length of output: 84

---

🏁 Script executed:

# Search for the specific code pattern mentioned in the review
rg "bifrostErr.Error.Message" -A 5 -B 5 --type go 2>/dev/null | head -50

Repository: maximhq/bifrost

Length of output: 3147

---

🏁 Script executed:

# Examine the mistral.go file around lines 526-538
sed -n '510,550p' ./core/providers/mistral/mistral.go

Repository: maximhq/bifrost

Length of output: 1342

---

🏁 Script executed:

# Look for similar error handling patterns in other providers
rg "sonic.Unmarshal.*BifrostError" -A 5 -B 5 --type go

Repository: maximhq/bifrost

Length of output: 41

---

🏁 Script executed:

# Check if there's a type field check elsewhere for Mistral errors
rg "bifrostErr.Type" -B 3 -A 3 --type go | head -40

Repository: maximhq/bifrost

Length of output: 2146

---

🏁 Script executed:

# Search for how other providers handle streaming error detection
rg "ProcessAndSendBifrostError" --type go | head -20

Repository: maximhq/bifrost

Length of output: 2728

---

🏁 Script executed:

# Check how other providers handle similar streaming error detection
sed -n '1,100p' ./core/providers/openai/openai.go | grep -A 10 "Unmarshal.*BifrostError"

Repository: maximhq/bifrost

Length of output: 41

---

🏁 Script executed:

# Look for the MistralTranscriptionStreamData struct to understand what a normal event looks like
rg "type MistralTranscriptionStreamData" -A 10 --type go

Repository: maximhq/bifrost

Length of output: 760

---

🏁 Script executed:

# Check if there's a Type field being used in error detection elsewhere
rg "\.Type.*error" -B 2 -A 2 --type go | head -30

Repository: maximhq/bifrost

Length of output: 1612

---

🏁 Script executed:

# Look at how Bedrock handles similar stream error detection
sed -n '1,600p' ./core/providers/bedrock/bedrock.go | grep -A 15 "ProcessAndSendBifrostError"

Repository: maximhq/bifrost

Length of output: 41

---

🏁 Script executed:

# Search for any Mistral API documentation or error handling comments
rg "mistral.*error\|Mistral.*error" -i --type go -A 3 -B 3 | head -50

Repository: maximhq/bifrost

Length of output: 41

---

🏁 Script executed:

# Check if there are any tests or examples showing Mistral error responses
find . -name "*mistral*test*" -o -name "*mistral*example*" | head -10

Repository: maximhq/bifrost

Length of output: 99

---

🏁 Script executed:

# Look at the complete MistralTranscriptionStreamData struct
sed -n '/type MistralTranscriptionStreamData/,/^}/p' ./core/providers/mistral/types.go

Repository: maximhq/bifrost

Length of output: 528

---

🏁 Script executed:

# Check if Type field is used elsewhere in error detection patterns
rg "bifrostErr.Type\s*!=" -B 5 -A 5 --type go | head -40

Repository: maximhq/bifrost

Length of output: 1306

---

🏁 Script executed:

# Look for how openai/bedrock handle streaming error detection to compare patterns
rg -A 20 "streaming.*error\|stream.*error" -i --type go core/providers/openai/ core/providers/bedrock/ | head -60

Repository: maximhq/bifrost

Length of output: 41

---

🏁 Script executed:

# Examine the mistral test file for error response examples
cat ./core/providers/mistral/mistral_test.go | head -100

Repository: maximhq/bifrost

Length of output: 1858

---

🏁 Script executed:

# Search for error response examples in mistral tests
rg "error\|Error" ./core/providers/mistral/mistral_test.go -B 2 -A 5 | head -80

Repository: maximhq/bifrost

Length of output: 41

---

🏁 Script executed:

# Check if there's any Mistral API documentation or comments about error format
sed -n '1,50p' ./core/providers/mistral/mistral.go

Repository: maximhq/bifrost

Length of output: 1971

---

🏁 Script executed:

# Look for the handleMistralTranscriptionStream function signature and full context
sed -n '500,560p' ./core/providers/mistral/mistral.go

Repository: maximhq/bifrost

Length of output: 1944

---

🏁 Script executed:

# Check how the eventType parameter is used - does it distinguish error events?
rg "eventType" ./core/providers/mistral/mistral.go -B 2 -A 2 | head -40

Repository: maximhq/bifrost

Length of output: 1120

---

🏁 Script executed:

# Check if there are actual Mistral error examples in comments or documentation
rg "mistral.*error\|error.*mistral" -i ./core/providers/mistral/ -B 3 -A 3

Repository: maximhq/bifrost

Length of output: 41

---

🏁 Script executed:

# Look at how the streaming is initiated - check request/response headers
sed -n '380,450p' ./core/providers/mistral/mistral.go

Repository: maximhq/bifrost

Length of output: 2368

---

🏁 Script executed:

# Check if Mistral has an error event type constant defined
rg "transcription\|Transcription" ./core/providers/mistral/types.go -B 2 -A 2

Repository: maximhq/bifrost

Length of output: 41

---

🏁 Script executed:

# Look for any comments about error handling in the streaming function
sed -n '500,560p' ./core/providers/mistral/mistral.go | grep -i "error\|check\|note"

Repository: maximhq/bifrost

Length of output: 535

---

🏁 Script executed:

# Check if there's an "error" event type constant or pattern in Mistral types
cat ./core/providers/mistral/types.go | grep -i "event\|type.*string"

Repository: maximhq/bifrost

Length of output: 1445

---

🏁 Script executed:

# Look at the full types file to understand all event types
wc -l ./core/providers/mistral/types.go

Repository: maximhq/bifrost

Length of output: 96

---

🏁 Script executed:

# Check if eventType parameter is passed but never used for error detection
sed -n '500,580p' ./core/providers/mistral/mistral.go | grep -n "eventType"

Repository: maximhq/bifrost

Length of output: 237

---

🏁 Script executed:

# Check how OpenAI handles similar streaming error detection for comparison
sed -n '1,100p' ./core/providers/openai/openai.go | head -50

Repository: maximhq/bifrost

Length of output: 2009

---

🏁 Script executed:

# Search for any error event type patterns in the codebase
rg "error.*event\|event.*error" -i --type go | head -20

Repository: maximhq/bifrost

Length of output: 41

---

🏁 Script executed:

# Check if there's a Type field being set on BifrostError in Mistral error handling
rg "bifrostErr.Type\|bifrostErr.IsBifrostError" ./core/providers/mistral/ --type go

Repository: maximhq/bifrost

Length of output: 41

---

🏁 Script executed:

# Look at how other providers detect streaming errors - check if they use event type
rg "eventType.*error\|event.*Type.*error" -i ./core/providers/ --type go | head -20

Repository: maximhq/bifrost

Length of output: 41

---

🏁 Script executed:

# Check if there's any documentation or inline comments about Mistral error format
sed -n '1,150p' ./core/providers/mistral/types.go

Repository: maximhq/bifrost

Length of output: 6756

---

🏁 Script executed:

# Check if there are any other streaming providers that have a better error detection pattern
rg -A 10 "processTranscriptionStreamEvent\|processStreamEvent.*error" ./core/providers/bedrock/ ./core/providers/anthropic/ --type go 2>/dev/null | head -50

Repository: maximhq/bifrost

Length of output: 41

---

🏁 Script executed:

# Look for any usage of eventType in error checking across providers
rg "eventType.*error\|event.*Type\|Type.*!=" ./core/providers/ --type go | grep -v test | head -20

Repository: maximhq/bifrost

Length of output: 41

---

Validate against known event types before checking for error conditions.

The current error detection attempts to unmarshal all stream events into BifrostError and only checks if Error.Message is populated. This is fragile—if any event data coincidentally contains an error field with a non-empty message, it will be misclassified as an error. Since eventType is already available and Mistral defines exactly four event types (transcription.language, transcription.segment, transcription.text.delta, transcription.done), validate that the event type is recognized before attempting error unmarshaling. Alternatively, check the Type or IsBifrostError fields if Mistral populates them for error responses.

🤖 Prompt for AI Agents

In core/providers/mistral/mistral.go around lines 526 to 538, the code currently
unmarshals every incoming stream event into BifrostError and treats any
non-empty Error.Message as an error; instead first validate the event type
against the known Mistral event types (transcription.language,
transcription.segment, transcription.text.delta, transcription.done) or check a
Type/IsBifrostError field before attempting to unmarshal into BifrostError, and
only when the event is not a recognized normal transcription event proceed to
unmarshal and handle it as a BifrostError; update the logic to gate the
sonic.Unmarshal call behind that event-type check so normal events with
incidental "error" fields are not misclassified.