batch request logging #1090
batch request logging #1090
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughCentralizes Bifrost request construction and handling (including streaming), adds file and batch request/response types and raw-body accessors, changes provider BatchList/FileList signatures to single-key, adjusts provider stubs, updates transports/routes/OTEL spans, docs, UI labels, and bumps versions/dependencies. Changes
sequenceDiagram
autonumber
participant Client
participant HTTP as Transport_HTTP
participant Bifrost
participant Plugins
participant Provider
Client->>HTTP: POST/GET (files or batches)
HTTP->>Bifrost: getBifrostRequest(raw)
Bifrost->>Bifrost: validateRequest (IsModelRequired)
Bifrost->>Plugins: pre-request hooks (mutate/annotate)
Plugins-->>Bifrost: mutated request
alt streaming
Bifrost->>Provider: handleStreamRequest(bifrostReq)
Provider-->>Bifrost: stream chunks / errors
else non-streaming
Bifrost->>Provider: handleRequest(bifrostReq)
Provider-->>Bifrost: response / error
end
Bifrost->>Plugins: post-request hooks (enrich)
Plugins-->>Bifrost: enriched response
Bifrost->>HTTP: unified response / structured error
HTTP->>Client: HTTP response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (86)
Comment |
🧪 Test Suite AvailableThis PR can be tested by a repository admin. |
akshaydeo
force-pushed
the
12-15-batch_request_logging
branch
from
6f9d9b4 to
cf707b2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
transports/bifrost-http/handlers/middlewares.go (1)
84-91: Add tests for Content-Type requirement and consider explicit error handling.This middleware introduces a breaking change: requests with JSON bodies but no
Content-Type: application/jsonheader will silently have their body replaced with"{}". While this is safer than previously attempting to unmarshal any non-empty body, it could mask client errors. Consider either:
- Adding tests to verify the Content-Type requirement behavior
- Adding explicit error logging or rejection when JSON is expected but Content-Type is missing
- Documenting this requirement for API clients
The middleware is applied only to inference routes; verify all callers set the appropriate Content-Type header.
core/providers/nebius/nebius.go (1)
121-145: Guard against nil Params when readingai_project_id
ChatCompletionaccessesrequest.Params.ExtraParams["ai_project_id"]without checking whetherrequest.Paramsis nil. IfParamsis an optional field inBifrostChatRequest(e.g., omitted in JSON) this will panic. Consider:if request.Params != nil { if rawID, ok := request.Params.ExtraParams["ai_project_id"]; ok && rawID != nil { // append query param... } }to keep the behavior safe for callers that don’t populate
params.
🧹 Nitpick comments (21)
core/providers/bedrock/batch.go (1)
335-338: Avoid hard‑coded"dummy-model"; prefer fixing model‑validation or using a typed sentinelUsing
Model: "dummy-model"purely to bypass validation is brittle: it can leak into logs/metrics and may start affecting behavior if any future code paths key offModelfor batch list flows. Prefer one of:
- Adjust the central
IsModelRequired/validation logic soBifrostBatchListRequestdoesn’t require a model, or- Introduce a well‑named constant/sentinel (e.g.,
bedrockBatchListPlaceholderModel) and add tests guaranteeing it’s never consumed by provider routing/model lookup.This keeps the Bedrock adapter from relying on a magic string while preserving the current behavior.
tests/integrations/config.json (1)
138-138: Enabling SQLite log_store for integrations is good; consider artifact handlingTurning on the
sqlitelog_storefor integration tests (./logs.db) is a nice way to exercise the new logging path. The only follow‑ups I’d consider:
- Ensure
logs.dbis in.gitignore(or otherwise cleaned up) so local test runs don’t create untracked artifacts.- If tests are ever parallelized, think about whether a shared
./logs.dbis acceptable or if you’d prefer a per‑run / temp directory location.Also applies to: 152-156
plugins/logging/version (1)
1-1: Ensure logging plugin version is bumped consistently across the stackThe logging plugin version is set to
1.3.49inplugins/logging/version, buttransports/go.modstill requiresv1.3.48. If this is a stacked PR where the version bump in transports is staged for a later commit, that's fine—but verify that all consuming modules (at minimumtransports) will be updated to1.3.49in this stack so you don't end up with mixed versions at runtime.core/utils.go (1)
52-52: Consider improving readability of the long condition.The single-line condition checking 11 request types is difficult to read and maintain. Consider using a switch statement or a set lookup for better readability.
-func IsModelRequired(reqType schemas.RequestType) bool { - return reqType == schemas.TextCompletionRequest || reqType == schemas.TextCompletionStreamRequest || reqType == schemas.ChatCompletionRequest || reqType == schemas.ChatCompletionStreamRequest || reqType == schemas.ResponsesRequest || reqType == schemas.ResponsesStreamRequest || reqType == schemas.SpeechRequest || reqType == schemas.SpeechStreamRequest || reqType == schemas.TranscriptionRequest || reqType == schemas.TranscriptionStreamRequest || reqType == schemas.EmbeddingRequest -} +// modelRequiredTypes contains request types that require a model field. +var modelRequiredTypes = map[schemas.RequestType]struct{}{ + schemas.TextCompletionRequest: {}, + schemas.TextCompletionStreamRequest: {}, + schemas.ChatCompletionRequest: {}, + schemas.ChatCompletionStreamRequest: {}, + schemas.ResponsesRequest: {}, + schemas.ResponsesStreamRequest: {}, + schemas.SpeechRequest: {}, + schemas.SpeechStreamRequest: {}, + schemas.TranscriptionRequest: {}, + schemas.TranscriptionStreamRequest: {}, + schemas.EmbeddingRequest: {}, +} + +func IsModelRequired(reqType schemas.RequestType) bool { + _, ok := modelRequiredTypes[reqType] + return ok +}docs/integrations/bedrock-sdk/files-and-batch.mdx (2)
868-868: Consider removing assertion from documentation example.The
assertstatement in documentation examples may confuse users who copy-paste the code, as assertion failures provide poor error messages. Consider using explicit error handling or a simple print statement instead.-assert job_arn in job_arns, f"Job {job_arn} should be in list" +if job_arn not in job_arns: + print(f"Warning: Job {job_arn} not found in list") +else: + print(f" Verified job is in list") -print(f" Verified job is in list")
1070-1072: Incomplete polling logic in Gemini example.The Gemini end-to-end example references "same polling logic as above" but doesn't include the actual implementation. Consider either including the complete polling code or extracting it into a reusable helper function shown earlier in the document.
docs/integrations/openai-sdk/files-and-batch.mdx (1)
429-430: Step numbering mismatch in workflow comments.The print statement says "Step 1" but the comment above says "Step 2". This pattern appears in multiple workflows (lines 438-439, 448-449, etc.).
# Step 2: Upload file (uses OpenAI's native file storage) -print("Step 1: Uploading batch input file...") +print("Step 2: Uploading batch input file...")transports/bifrost-http/integrations/bedrock_test.go (1)
191-191: Assertion message doesn't match field name.The assertion tests
routes[i].BatchRequestConverterbut the message still references the old name "BatchCreateRequestConverter".- assert.NotNil(t, routes[i].BatchRequestConverter, "batch route %d BatchCreateRequestConverter should not be nil", i) + assert.NotNil(t, routes[i].BatchRequestConverter, "batch route %d BatchRequestConverter should not be nil", i)tests/integrations/tests/test_bedrock.py (2)
1503-1503: Remove unnecessary f-string prefix.This f-string has no placeholders and should be a regular string.
- print(f"stopping the job") + print("stopping the job")
1499-1500: Consider using logging instead of print statements.These debug print statements are useful for development but consider either removing them before merge or converting to proper logging with appropriate log levels for production visibility.
Also applies to: 1557-1560
core/schemas/batch.go (1)
111-127: Clarify optional vs required semantics for the newModelfields
BifrostBatchCreateRequest.Modelis taggedjson:"model,omitempty"and documented as an optional routing hint, but the newModelfields onBifrostBatchListRequest,BifrostBatchRetrieveRequest,BifrostBatchCancelRequest, andBifrostBatchResultsRequestare taggedjson:"model"withoutomitempty. At the same time, some converters (e.g., Anthropic batch list/retrieve/cancel/results) currently do not populateModel, so it will stay as"".If
Modelis intended to be optional and primarily for logging/routing hints on these operations as well, consider either:
- Adding
omitemptyto these tags for consistency with create, or- Making
Modela pointer (*string) to clearly distinguish “unset” from “empty string”.Alternatively, if
Modelis now required for these operations, it would be good to enforce or document that at call‑sites and in docs, and ensure all integrations set it.Also applies to: 143-151, 191-199, 213-224
core/schemas/files.go (1)
52-67: Define and enforce the intended contract forModelon file requestsYou’ve introduced
Model string "json:\"model\""on all file request types (upload/list/retrieve/delete/content), but current integrations like the Anthropic file routes only populateProvider/IDs and leaveModelat its zero value.If
Modelis only a best‑effort routing/logging hint for file APIs (which are typically account‑scoped, not model‑scoped), you may want to:
- Add
omitemptyto these tags for consistency with other optional fields, or- Make
Modela pointer and treatnilas “no model hint”.If instead
Modelis now required for correct key selection or logging, then the Anthropic (and other) FileRequest converters should be updated to set it, or you should add validation to catch missing models early.Also applies to: 108-126, 138-150, 171-182, 193-204
transports/bifrost-http/integrations/router.go (1)
80-88: Batch routing integration inGenericRouterlooks correct; consider guarding against dual batch+file configsThe new
BatchRequestConverterplumbs batch handling cleanly intoRouteConfigandcreateHandler:
- Route typing via
isBatchRoute := route.BatchRequestConverter != nilcorrectly excludes batch routes from the inferenceRequestConverterrequirement.- The batch branch in
createHandlerruns after parsing +PreCallback, usesBatchRequestConverter, and wraps the whole block withdefer cancel(), matching the non‑streaming pattern used for file routes.handleBatchRequestthen fans out to the appropriate Bifrost batch method and response converter.One minor hardening you might consider: add a startup validation that a route doesn’t configure both
BatchRequestConverterandFileRequestConverterat the same time, since currently batch would win silently. Not a blocker, just a future‑proofing improvement.Also applies to: 100-105, 263-295, 336-340, 435-448
transports/bifrost-http/integrations/anthropic.go (1)
283-355: Anthropic batch converters correctly adoptBatchRequestConverter; small robustness nitsThe Anthropic batch routes now plug into the new
BatchRequestConvertercleanly:
- Create: builds a
BatchRequestwith aBifrostBatchCreateRequestthat:
- Derives
providerfromBifrostContextKey("batch_provider").- For non‑Anthropic providers, enforces a per‑request
modeland requires all models to match, populatingCreateRequest.Model.- For OpenAI, also enforces a common
urland setsCreateRequest.Endpoint, which is a nice validation improvement.- List/retrieve/cancel/results: wrap the Anthropic batch types into the corresponding Bifrost batch request structs, using
bifrostContextKeyProviderfrom thePreCallbackand handling Gemini ID rewrite.Two minor nits you may want to consider:
Provider extraction symmetry –
BatchListRequestuses a safe type assertion with anokcheck, but retrieve/cancel/results doprovider := (*ctx).Value(bifrostContextKeyProvider).(schemas.ModelProvider)without checking. Today thePreCallbackguarantees this, but mirroring theokcheck would avoid panics if routes are ever reused or callbacks changed.New
Modelfield usage – The list/retrieve/cancel/results wrappers don’t setModelon the Bifrost requests. IfModelstays optional and only a logging hint, that’s fine; if core code starts relying on it, you may want to either populate it here when you can infer it or explicitly treat emptyModelas “unknown” in downstream logic.Overall, the BatchRequest wiring and validation look solid.
Also applies to: 359-382, 399-422, 435-458, 471-494
core/providers/openai/openai.go (1)
2245-2286: Single-keyFileListimplementation and URL construction look goodThe refactor to a single
keyparameter, use ofbuildRequestURLwithurl.Values, and conditional Authorization header fromkey.Valueall look correct and consistent with other OpenAI methods. Behavior remains essentially the same as the old multi-key version for the first key.If you want symmetry with
BatchList, you could consider returning a configuration error whenkey.Valueis empty instead of relying on a 401 from OpenAI, provided you don't need keyless flows for Files.core/bifrost.go (2)
872-921: Batch/File public helpers look consistent; BatchCreate’s provider lookup is now redundantThe new Batch* and File* request methods correctly wrap the typed requests into a pooled
BifrostRequestand delegate tohandleRequest, matching how other APIs are routed. The explicitgetProviderByKeyinBatchCreateRequestis now redundant (the provider/queue will be resolved insidetryRequest/getProviderQueue) and could be dropped to avoid double initialization work, but that’s optional and not correctness‑critical.Also applies to: 923-1087, 1089-1300
2244-2256: Stream validation now sets HTTP 400 only for streaming pathAdding
StatusCode = fasthttp.StatusBadRequestonvalidateRequestfailure for streaming requests is reasonable and improves signaling to HTTP handlers. If you want fully consistent behavior, consider mirroring the same status‑code assignment inhandleRequest’s non‑streaming path as well; otherwise this asymmetry is acceptable.core/providers/gemini/gemini.go (2)
1456-1623: Consider relaxing Gemini BatchList’s hard key.Value check
BatchListnow returns a configuration error whenkey.Value == "", while the rest of the Gemini surface (chat, responses, etc.) happily runs with an empty key as long as auth is provided viaExtraHeadersor a keyless/custom provider config. If you intend to support keyless/custom Gemini configs (e.g.,CustomProviderConfig.IsKeyLessorBifrostContextKeySkipKeySelection), this guard will turn otherwise valid setups into configuration errors. It may be safer to rely on the existing key‑selection logic in bifrost and only set the header whenkey.Valueis non‑empty, letting the backend auth layer decide.Also applies to: 1625-1739
2239-2315: FileList key validation and pagination behavior
FileListmirrors the BatchList key check (key.Value == "") and returnsNewConfigurationError("no keys provided", ...). Two minor points:
- Same concern as BatchList: a hard requirement on
key.Valuemay block keyless/custom auth flows even though headers can carry credentials.- The error text still refers to “keys” plural, but the method now takes a single
schemas.Key; consider rewording to “key is required” if you keep the check.Setting
AfterfromNextPageTokenis a good fit for the Bifrost pagination shape.core/schemas/bifrost.go (1)
160-180: Extend SetProvider/SetModel/SetFallbacks/SetRawRequestBody to cover batch/file requests
BifrostRequestnow carries File* and Batch* request pointers, andGetRequestFieldscan return their provider/model, but the helper setters (SetProvider,SetModel,SetFallbacks,SetRawRequestBody) still only update the original text/chat/responses/embedding/speech/transcription variants. Any plugins or middleware that call these generically will not affect batch/file requests today. If you expect plugins to be able to rewrite provider/model or attach raw bodies for these new types, consider extending the switch statements accordingly.Also applies to: 222-288
core/providers/nebius/nebius.go (1)
17-23: sendBackRawRequest is never initialized from config
NebiusProvidernow has asendBackRawRequestfield and you threadShouldSendBackRawRequest(ctx, provider.sendBackRawRequest)into all handlers, butNewNebiusProvideronly setssendBackRawResponse. If you want parity with other providers (OpenAI, OpenRouter, etc.), you probably also want:sendBackRawRequest: config.SendBackRawRequest,in the constructor.
Also applies to: 48-53
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
plugins/governance/go.sumis excluded by!**/*.sumplugins/logging/go.sumis excluded by!**/*.sumplugins/maxim/go.sumis excluded by!**/*.sumplugins/otel/go.sumis excluded by!**/*.sumplugins/semanticcache/go.sumis excluded by!**/*.sumplugins/telemetry/go.sumis excluded by!**/*.sumtransports/go.sumis excluded by!**/*.sum
📒 Files selected for processing (82)
core/bifrost.go(14 hunks)core/changelog.md(1 hunks)core/providers/anthropic/anthropic.go(3 hunks)core/providers/anthropic/batch.go(1 hunks)core/providers/azure/azure.go(3 hunks)core/providers/bedrock/batch.go(1 hunks)core/providers/bedrock/bedrock.go(5 hunks)core/providers/cerebras/cerebras.go(2 hunks)core/providers/cohere/cohere.go(2 hunks)core/providers/elevenlabs/elevenlabs.go(2 hunks)core/providers/gemini/gemini.go(3 hunks)core/providers/groq/groq.go(2 hunks)core/providers/mistral/batch.go(0 hunks)core/providers/mistral/files.go(0 hunks)core/providers/mistral/mistral.go(1 hunks)core/providers/nebius/nebius.go(8 hunks)core/providers/ollama/ollama.go(2 hunks)core/providers/openai/openai.go(5 hunks)core/providers/openrouter/openrouter.go(2 hunks)core/providers/parasail/batch.go(0 hunks)core/providers/parasail/files.go(0 hunks)core/providers/parasail/parasail.go(1 hunks)core/providers/perplexity/perplexity.go(2 hunks)core/providers/sgl/batch.go(0 hunks)core/providers/sgl/files.go(0 hunks)core/providers/sgl/sgl.go(1 hunks)core/providers/utils/utils.go(1 hunks)core/providers/vertex/vertex.go(2 hunks)core/schemas/batch.go(4 hunks)core/schemas/bifrost.go(4 hunks)core/schemas/chatcompletions.go(1 hunks)core/schemas/files.go(5 hunks)core/schemas/provider.go(2 hunks)core/utils.go(2 hunks)core/version(1 hunks)docs/docs.json(2 hunks)docs/integrations/anthropic-sdk/files-and-batch.mdx(1 hunks)docs/integrations/anthropic-sdk/overview.mdx(3 hunks)docs/integrations/bedrock-sdk/files-and-batch.mdx(1 hunks)docs/integrations/bedrock-sdk/overview.mdx(3 hunks)docs/integrations/genai-sdk/files-and-batch.mdx(1 hunks)docs/integrations/genai-sdk/overview.mdx(3 hunks)docs/integrations/openai-sdk/files-and-batch.mdx(1 hunks)docs/integrations/openai-sdk/overview.mdx(3 hunks)framework/changelog.md(1 hunks)framework/configstore/migrations.go(1 hunks)framework/version(1 hunks)plugins/governance/changelog.md(1 hunks)plugins/governance/go.mod(1 hunks)plugins/governance/version(1 hunks)plugins/jsonparser/changelog.md(1 hunks)plugins/jsonparser/version(1 hunks)plugins/logging/changelog.md(1 hunks)plugins/logging/go.mod(1 hunks)plugins/logging/main.go(1 hunks)plugins/logging/version(1 hunks)plugins/maxim/changelog.md(1 hunks)plugins/maxim/go.mod(1 hunks)plugins/maxim/version(1 hunks)plugins/mocker/changelog.md(1 hunks)plugins/mocker/version(1 hunks)plugins/otel/changelog.md(1 hunks)plugins/otel/go.mod(1 hunks)plugins/otel/version(1 hunks)plugins/semanticcache/changelog.md(1 hunks)plugins/semanticcache/go.mod(1 hunks)plugins/semanticcache/version(1 hunks)plugins/telemetry/changelog.md(1 hunks)plugins/telemetry/go.mod(1 hunks)plugins/telemetry/version(1 hunks)tests/integrations/config.json(2 hunks)tests/integrations/tests/test_bedrock.py(3 hunks)transports/bifrost-http/handlers/inference.go(0 hunks)transports/bifrost-http/handlers/middlewares.go(1 hunks)transports/bifrost-http/integrations/anthropic.go(5 hunks)transports/bifrost-http/integrations/bedrock.go(4 hunks)transports/bifrost-http/integrations/bedrock_test.go(1 hunks)transports/bifrost-http/integrations/openai.go(4 hunks)transports/bifrost-http/integrations/router.go(4 hunks)transports/changelog.md(1 hunks)transports/go.mod(1 hunks)transports/version(1 hunks)
💤 Files with no reviewable changes (7)
- transports/bifrost-http/handlers/inference.go
- core/providers/mistral/batch.go
- core/providers/parasail/files.go
- core/providers/mistral/files.go
- core/providers/parasail/batch.go
- core/providers/sgl/batch.go
- core/providers/sgl/files.go
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
plugins/governance/changelog.mdplugins/logging/changelog.mdplugins/jsonparser/changelog.mdcore/schemas/chatcompletions.gocore/schemas/provider.gocore/versionplugins/otel/go.modtransports/bifrost-http/integrations/bedrock_test.gotransports/go.moddocs/integrations/bedrock-sdk/files-and-batch.mdxtransports/versionplugins/semanticcache/changelog.mdcore/utils.godocs/integrations/genai-sdk/files-and-batch.mdxcore/providers/ollama/ollama.goplugins/governance/go.modcore/schemas/batch.gocore/providers/elevenlabs/elevenlabs.gocore/providers/cohere/cohere.goframework/configstore/migrations.godocs/docs.jsoncore/providers/bedrock/batch.goplugins/maxim/changelog.mdcore/schemas/files.goplugins/logging/versiontransports/bifrost-http/handlers/middlewares.godocs/integrations/anthropic-sdk/overview.mdxdocs/integrations/openai-sdk/overview.mdxplugins/semanticcache/go.moddocs/integrations/openai-sdk/files-and-batch.mdxframework/versioncore/providers/anthropic/batch.gocore/providers/utils/utils.gocore/providers/perplexity/perplexity.goplugins/telemetry/go.modplugins/maxim/go.modplugins/mocker/changelog.mdframework/changelog.mdplugins/telemetry/versiondocs/integrations/genai-sdk/overview.mdxplugins/semanticcache/versionplugins/otel/versioncore/providers/vertex/vertex.gocore/providers/azure/azure.gotransports/bifrost-http/integrations/router.goplugins/maxim/versioncore/providers/openai/openai.goplugins/logging/go.modcore/providers/groq/groq.gotransports/bifrost-http/integrations/anthropic.gocore/providers/gemini/gemini.goplugins/otel/changelog.mddocs/integrations/bedrock-sdk/overview.mdxtransports/bifrost-http/integrations/openai.gocore/providers/bedrock/bedrock.gotests/integrations/config.jsoncore/providers/anthropic/anthropic.goplugins/jsonparser/versionplugins/logging/main.gocore/bifrost.gotests/integrations/tests/test_bedrock.pytransports/changelog.mdplugins/governance/versioncore/changelog.mdcore/providers/cerebras/cerebras.gotransports/bifrost-http/integrations/bedrock.godocs/integrations/anthropic-sdk/files-and-batch.mdxcore/providers/parasail/parasail.goplugins/mocker/versioncore/schemas/bifrost.gocore/providers/sgl/sgl.gocore/providers/nebius/nebius.gocore/providers/mistral/mistral.goplugins/telemetry/changelog.mdcore/providers/openrouter/openrouter.go
🧠 Learnings (5)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
core/schemas/chatcompletions.gocore/schemas/provider.gotransports/bifrost-http/integrations/bedrock_test.gocore/utils.gocore/providers/ollama/ollama.gocore/schemas/batch.gocore/providers/elevenlabs/elevenlabs.gocore/providers/cohere/cohere.goframework/configstore/migrations.gocore/providers/bedrock/batch.gocore/schemas/files.gotransports/bifrost-http/handlers/middlewares.gocore/providers/anthropic/batch.gocore/providers/utils/utils.gocore/providers/perplexity/perplexity.gocore/providers/vertex/vertex.gocore/providers/azure/azure.gotransports/bifrost-http/integrations/router.gocore/providers/openai/openai.gocore/providers/groq/groq.gotransports/bifrost-http/integrations/anthropic.gocore/providers/gemini/gemini.gotransports/bifrost-http/integrations/openai.gocore/providers/bedrock/bedrock.gocore/providers/anthropic/anthropic.goplugins/logging/main.gocore/bifrost.gocore/providers/cerebras/cerebras.gotransports/bifrost-http/integrations/bedrock.gocore/providers/parasail/parasail.gocore/schemas/bifrost.gocore/providers/sgl/sgl.gocore/providers/nebius/nebius.gocore/providers/mistral/mistral.gocore/providers/openrouter/openrouter.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/integrations/bedrock_test.gotransports/bifrost-http/handlers/middlewares.gotransports/bifrost-http/integrations/router.gotransports/bifrost-http/integrations/anthropic.gotransports/bifrost-http/integrations/openai.gotransports/bifrost-http/integrations/bedrock.go
📚 Learning: 2025-12-11T11:58:25.307Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: core/providers/openai/responses.go:42-84
Timestamp: 2025-12-11T11:58:25.307Z
Learning: In core/providers/openai/responses.go (and related OpenAI response handling), document and enforce the API format constraint: if ResponsesReasoning != nil and the response contains content blocks, all content blocks should be treated as reasoning blocks by default. Implement type guards or parsing logic accordingly, and add unit tests to verify that when ResponsesReasoning is non-nil, content blocks are labeled as reasoning blocks. Include clear comments in the code explaining the rationale and ensure downstream consumers rely on this behavior.
Applied to files:
core/providers/openai/openai.go
📚 Learning: 2025-12-14T14:43:30.902Z
Learnt from: Radheshg04
Repo: maximhq/bifrost PR: 980
File: core/providers/openai/images.go:10-22
Timestamp: 2025-12-14T14:43:30.902Z
Learning: Enforce the OpenAI image generation SSE event type values across the OpenAI image flow in the repository: use "image_generation.partial_image" for partial chunks, "image_generation.completed" for the final result, and "error" for errors. Apply this consistently in schemas, constants, tests, accumulator routing, and UI code within core/providers/openai (and related Go files) to ensure uniform event typing and avoid mismatches.
Applied to files:
core/providers/openai/openai.go
📚 Learning: 2025-12-13T13:03:38.364Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 955
File: transports/bifrost-http/integrations/bedrock.go:682-697
Timestamp: 2025-12-13T13:03:38.364Z
Learning: In transports/bifrost-http/integrations/bedrock.go, implement FileRetrieveRequest.FileID and FileDeleteRequest.FileID to use the ETag from the If-Match header as the provider-specific file ID (e.g., OpenAI file-xxx, Gemini files/yyy), rather than deriving the ID from the S3 URI. The StorageConfig should supply the S3 bucket/prefix context, enabling cross-provider routing while keeping S3 as the backing store. This change should be accompanied by unit tests that verify the FileID source is the If-Match header, update any relevant request/response models, and add in-code comments to clarify the separation of provider file IDs from S3 paths.
Applied to files:
transports/bifrost-http/integrations/bedrock.go
🧬 Code graph analysis (30)
core/schemas/provider.go (3)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
transports/bifrost-http/integrations/bedrock_test.go (1)
transports/bifrost-http/integrations/router.go (1)
BatchRequestConverter(101-101)
core/utils.go (1)
core/schemas/bifrost.go (12)
RequestType(85-85)TextCompletionRequest(89-89)TextCompletionStreamRequest(90-90)ChatCompletionRequest(91-91)ChatCompletionStreamRequest(92-92)ResponsesRequest(93-93)ResponsesStreamRequest(94-94)SpeechRequest(96-96)SpeechStreamRequest(97-97)TranscriptionRequest(98-98)TranscriptionStreamRequest(99-99)EmbeddingRequest(95-95)
core/providers/ollama/ollama.go (3)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
core/schemas/batch.go (1)
core/schemas/models.go (1)
Model(109-129)
core/providers/elevenlabs/elevenlabs.go (4)
core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (1)
BifrostError(432-441)ui/lib/types/logs.ts (1)
BifrostError(226-232)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
core/providers/cohere/cohere.go (4)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (1)
BifrostError(432-441)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
framework/configstore/migrations.go (1)
framework/configstore/tables/modelpricing.go (2)
TableModelPricing(4-41)TableModelPricing(44-44)
core/providers/bedrock/batch.go (1)
core/schemas/models.go (1)
Model(109-129)
core/schemas/files.go (1)
core/schemas/models.go (1)
Model(109-129)
core/providers/utils/utils.go (2)
core/utils.go (1)
Ptr(56-58)core/schemas/utils.go (1)
Ptr(16-18)
core/providers/perplexity/perplexity.go (2)
core/schemas/account.go (1)
Key(8-18)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
core/providers/vertex/vertex.go (4)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (1)
BifrostError(432-441)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
core/providers/azure/azure.go (2)
core/schemas/account.go (2)
Key(8-18)AzureKeyConfig(22-26)core/schemas/files.go (1)
BifrostFileListRequest(109-126)
core/providers/openai/openai.go (1)
core/providers/utils/utils.go (3)
SetExtraHeaders(179-209)CheckOperationAllowed(473-485)NewConfigurationError(550-560)
core/providers/groq/groq.go (3)
core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (1)
BifrostError(432-441)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
transports/bifrost-http/integrations/anthropic.go (1)
transports/bifrost-http/integrations/router.go (2)
BatchRequestConverter(101-101)BatchRequest(81-88)
core/providers/gemini/gemini.go (5)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/providers/utils/utils.go (2)
CheckOperationAllowed(473-485)NewConfigurationError(550-560)transports/bifrost-http/handlers/inference.go (1)
BatchListRequest(302-307)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
transports/bifrost-http/integrations/openai.go (1)
transports/bifrost-http/integrations/router.go (2)
BatchRequestConverter(101-101)BatchRequest(81-88)
core/providers/bedrock/bedrock.go (4)
core/providers/utils/utils.go (1)
CheckOperationAllowed(473-485)core/schemas/account.go (1)
Key(8-18)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)
core/providers/anthropic/anthropic.go (4)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (4)
BifrostError(432-441)Anthropic(37-37)BatchListRequest(101-101)FileListRequest(106-106)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
plugins/logging/main.go (3)
ui/lib/types/logs.ts (1)
TranscriptionInput(15-17)core/schemas/bifrost.go (1)
TranscriptionRequest(98-98)transports/bifrost-http/handlers/inference.go (1)
TranscriptionRequest(285-289)
core/providers/cerebras/cerebras.go (3)
core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)core/schemas/bifrost.go (1)
BifrostError(432-441)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)
transports/bifrost-http/integrations/bedrock.go (1)
transports/bifrost-http/integrations/router.go (2)
BatchRequestConverter(101-101)BatchRequest(81-88)
core/providers/parasail/parasail.go (3)
core/schemas/bifrost.go (11)
BifrostError(432-441)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)FileContentRequest(109-109)BatchCreateRequest(100-100)BatchListRequest(101-101)BatchRetrieveRequest(102-102)BatchCancelRequest(103-103)BatchResultsRequest(104-104)ui/lib/types/logs.ts (1)
BifrostError(226-232)core/providers/utils/utils.go (1)
NewUnsupportedOperationError(455-467)
core/schemas/bifrost.go (4)
core/schemas/files.go (10)
BifrostFileUploadRequest(53-67)BifrostFileListRequest(109-126)BifrostFileRetrieveRequest(139-150)BifrostFileDeleteRequest(172-182)BifrostFileContentRequest(194-204)BifrostFileUploadResponse(90-106)BifrostFileListResponse(129-136)BifrostFileRetrieveResponse(153-169)BifrostFileDeleteResponse(185-191)BifrostFileContentResponse(207-213)core/schemas/batch.go (10)
BifrostBatchCreateRequest(65-82)BifrostBatchListRequest(112-127)BifrostBatchRetrieveRequest(144-151)BifrostBatchCancelRequest(192-199)BifrostBatchResultsRequest(214-224)BifrostBatchCreateResponse(85-109)BifrostBatchListResponse(130-141)BifrostBatchRetrieveResponse(154-189)BifrostBatchCancelResponse(202-211)BifrostBatchResultsResponse(258-267)core/schemas/provider.go (1)
Provider(312-359)core/schemas/models.go (1)
Model(109-129)
core/providers/sgl/sgl.go (5)
core/schemas/files.go (10)
BifrostFileUploadRequest(53-67)BifrostFileUploadResponse(90-106)BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)BifrostFileRetrieveRequest(139-150)BifrostFileRetrieveResponse(153-169)BifrostFileDeleteRequest(172-182)BifrostFileDeleteResponse(185-191)BifrostFileContentRequest(194-204)BifrostFileContentResponse(207-213)core/schemas/bifrost.go (11)
BifrostError(432-441)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)FileContentRequest(109-109)BatchCreateRequest(100-100)BatchListRequest(101-101)BatchRetrieveRequest(102-102)BatchCancelRequest(103-103)BatchResultsRequest(104-104)core/providers/utils/utils.go (1)
NewUnsupportedOperationError(455-467)core/schemas/batch.go (10)
BifrostBatchCreateRequest(65-82)BifrostBatchCreateResponse(85-109)BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)BifrostBatchRetrieveRequest(144-151)BifrostBatchRetrieveResponse(154-189)BifrostBatchCancelRequest(192-199)BifrostBatchCancelResponse(202-211)BifrostBatchResultsRequest(214-224)BifrostBatchResultsResponse(258-267)transports/bifrost-http/handlers/inference.go (2)
BatchCreateRequest(292-299)BatchListRequest(302-307)
core/providers/nebius/nebius.go (1)
core/providers/utils/utils.go (2)
ShouldSendBackRawRequest(607-612)NewUnsupportedOperationError(455-467)
core/providers/mistral/mistral.go (2)
core/schemas/bifrost.go (2)
BifrostError(432-441)BatchCreateRequest(100-100)core/providers/utils/utils.go (1)
NewUnsupportedOperationError(455-467)
core/providers/openrouter/openrouter.go (2)
core/schemas/bifrost.go (5)
BifrostError(432-441)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)core/providers/utils/utils.go (1)
NewUnsupportedOperationError(455-467)
🪛 Ruff (0.14.8)
tests/integrations/tests/test_bedrock.py
1503-1503: f-string without any placeholders
Remove extraneous f prefix
(F541)
akshaydeo
force-pushed
the
12-15-batch_request_logging
branch
from
cf707b2 to
dfbc9b8
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
ui/lib/constants/logs.ts (1)
29-41: Critical: RequestTypes array missing new batch and file operation types.The
RequestTypesarray doesn't include the new batch and file operation types (batch_create, batch_list, batch_retrieve, batch_cancel, batch_results, file_upload, file_list, file_retrieve, file_delete, file_content) that were added toRequestTypeLabelsandRequestTypeColors. This breaks type safety if aRequestTypetype is derived from this array (similar to howProviderNameis derived fromKnownProvidersNamesat line 23).Apply this diff to add the missing types:
export const RequestTypes = [ "text_completion", "text_completion_stream", "chat_completion", "chat_completion_stream", "responses", "responses_stream", "embedding", "speech", "speech_stream", "transcription", "transcription_stream", + "batch_create", + "batch_list", + "batch_retrieve", + "batch_cancel", + "batch_results", + "file_upload", + "file_list", + "file_retrieve", + "file_delete", + "file_content", ] as const;core/providers/nebius/nebius.go (1)
48-53: AddsendBackRawRequestinitialization inNewNebiusProvider.The new
sendBackRawRequestfield is missing from the provider initialization at lines 48-53. It exists inProviderConfigand is initialized by all other providers, but Nebius leaves it at its zero value (false), making the feature non-configurable:return &NebiusProvider{ logger: logger, client: client, networkConfig: config.NetworkConfig, + sendBackRawRequest: config.SendBackRawRequest, sendBackRawResponse: config.SendBackRawResponse, }, nilcore/providers/anthropic/batch.go (3)
53-53: Addomitemptyto prevent empty string serialization.The
ExpiresAtfield lacks theomitemptyJSON tag, causing it to serialize as"expires_at": ""when unset (empty string zero value). Per Anthropic's Batch API spec,expires_atmust be a valid RFC-3339 datetime, not an empty string.Apply this diff:
- ExpiresAt string `json:"expires_at"` + ExpiresAt string `json:"expires_at,omitempty"`This will prevent the field from appearing in the JSON output when it's an empty string, addressing the root cause of serialization issues in lines 260-262 and 301-303.
301-303: Same empty string serialization issue as lines 260-262.When
resp.ExpiresAtis nil,result.ExpiresAtremains as an empty string and will serialize as"expires_at": ""because the field on line 53 lacksomitempty. This creates an invalid Anthropic API response.Adding
omitemptyto line 53 (see separate comment) will resolve this for all conversion functions.
133-141: Inconsistent ExpiresAt handling compared to ToBifrostBatchRetrieveResponse.
ToBifrostBatchCreateResponseunconditionally assignsExpiresAteven whenparseAnthropicTimestampreturns 0 (line 133 → 141). This means if Anthropic returns an emptyexpires_atstring, Bifrost will serialize"expires_at": 0(Unix epoch).In contrast,
ToBifrostBatchRetrieveResponse(lines 192-195) only assignsExpiresAtwhen the parsed value is> 0, correctly omitting the field when invalid.Apply this diff to align the behavior:
- 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, ExtraFields: schemas.BifrostResponseExtraFields{ RequestType: schemas.BatchCreateRequest, Provider: providerName, Latency: latency.Milliseconds(), }, } + + expiresAt := parseAnthropicTimestamp(r.ExpiresAt) + if expiresAt > 0 { + resp.ExpiresAt = &expiresAt + }
♻️ Duplicate comments (4)
core/providers/anthropic/batch.go (1)
260-262: Empty string serialization issue persists despite nil-check fix.The nil-check added in this PR prevents the panic that would occur from dereferencing a nil pointer. However, when
resp.ExpiresAtis nil,result.ExpiresAtremains as an empty string (the zero value for string type). SinceAnthropicBatchResponse.ExpiresAt(line 53) lacks theomitemptyJSON tag, this serializes as"expires_at": ""in the API response, which is invalid per Anthropic's spec.Either add
omitemptyto line 53 (recommended—see separate comment) or compute a default value (created_at + 24 hours) whenresp.ExpiresAtis nil.transports/changelog.md (1)
1-6: Remove stray>in changelog entryLine 6 still has a trailing
>which was flagged previously; it should be a plain sentence.-- fix: proper cost compute for gemini models (>200k and <200k token costs are now considered>) +- fix: proper cost compute for gemini models (>200k and <200k token costs are now considered)core/providers/azure/azure.go (1)
945-947: FileList/BatchList still block bearer‑token–only auth; align with other Azure file/batch methods
FileListandBatchListnow immediately error whenkey.Value == "", even thoughsetAzureAuthalready supports bearer tokens viaAzureAuthorizationTokenKey(same pattern used byFileUpload,FileRetrieve,FileDelete,FileContent,BatchRetrieve,BatchCancel, etc.). This makes these two endpoints inconsistent and prevents valid bearer‑token–only usages.Recommend only failing when both the API key and bearer token are absent:
func (provider *AzureProvider) FileList(ctx context.Context, key schemas.Key, request *schemas.BifrostFileListRequest) (*schemas.BifrostFileListResponse, *schemas.BifrostError) { - if key.Value == "" { - return nil, providerUtils.NewConfigurationError("no keys provided", provider.GetProviderKey()) - } + if key.Value == "" { + if _, ok := ctx.Value(AzureAuthorizationTokenKey).(string); !ok { + return nil, providerUtils.NewConfigurationError("no keys provided", provider.GetProviderKey()) + } + } ... } func (provider *AzureProvider) BatchList(ctx context.Context, key schemas.Key, request *schemas.BifrostBatchListRequest) (*schemas.BifrostBatchListResponse, *schemas.BifrostError) { - if key.Value == "" { - return nil, providerUtils.NewConfigurationError("no keys provided", provider.GetProviderKey()) - } + if key.Value == "" { + if _, ok := ctx.Value(AzureAuthorizationTokenKey).(string); !ok { + return nil, providerUtils.NewConfigurationError("no keys provided", provider.GetProviderKey()) + } + } ... }This brings FileList/BatchList in line with the rest of the Azure file/batch surface while still guarding against missing credentials.
Also applies to: 1401-1403
core/providers/bedrock/bedrock.go (1)
1423-1561: Fix Bedrock FileList auth guard and BedrockKeyConfig nil handling (panic + incorrect rejection)
FileListcurrently has two concrete problems:
- It gates on
key.Value:if key.Value == "" { return nil, providerUtils.NewConfigurationError("no keys provided", providerName) }But S3 access here is authenticated via
key.BedrockKeyConfigAWS credentials, not the bearerValue. In the normal Bedrock patternValueis empty and onlyBedrockKeyConfigis set, so this incorrectly rejects valid configs and breaks callers likeBatchResults, which pass a Bedrock key withoutValue.
- It dereferences
key.BedrockKeyConfigwithout a nil check:region := DefaultBedrockRegion if key.BedrockKeyConfig.Region != nil { region = *key.BedrockKeyConfig.Region } ... if err := signAWSRequest(ctx, httpReq, key.BedrockKeyConfig.AccessKey, key.BedrockKeyConfig.SecretKey, key.BedrockKeyConfig.SessionToken, region, "s3", providerName); err != nil { ... }If
BedrockKeyConfigis nil (misconfigured key), this will panic.Recommend aligning with other Bedrock methods (Embedding, BatchCreate, BatchList, FileRetrieve, FileDelete, FileContent) by requiring
BedrockKeyConfigand using a localconfigvariable, while removing thekey.Valuegate:func (provider *BedrockProvider) FileList(ctx context.Context, key schemas.Key, request *schemas.BifrostFileListRequest) (*schemas.BifrostFileListResponse, *schemas.BifrostError) { @@ - providerName := provider.GetProviderKey() - - if key.Value == "" { - return nil, providerUtils.NewConfigurationError("no keys provided", providerName) - } + providerName := provider.GetProviderKey() + if key.BedrockKeyConfig == nil { + return nil, providerUtils.NewConfigurationError("bedrock key config is not provided", providerName) + } @@ - region := DefaultBedrockRegion - if key.BedrockKeyConfig.Region != nil { - region = *key.BedrockKeyConfig.Region - } + config := key.BedrockKeyConfig + region := DefaultBedrockRegion + if config.Region != nil { + region = *config.Region + } @@ - // Sign request for S3 - if err := signAWSRequest(ctx, httpReq, key.BedrockKeyConfig.AccessKey, key.BedrockKeyConfig.SecretKey, key.BedrockKeyConfig.SessionToken, region, "s3", providerName); err != nil { + // Sign request for S3 + if err := signAWSRequest(ctx, httpReq, config.AccessKey, config.SecretKey, config.SessionToken, region, "s3", providerName); err != nil { return nil, err }This removes the panic risk and ensures IAM‑style Bedrock configs (empty
Value, populatedBedrockKeyConfig) work correctly, including whenBatchResultsiterates result files viaFileList.
🧹 Nitpick comments (11)
core/providers/sgl/sgl.go (1)
223-271: Unsupported file/batch stubs are correctly wired; consider adding TODOs for future implementationThe new File* and Batch* methods all have the right signatures, map to the correct
schemas.RequestTypevalues, and consistently delegate toproviderUtils.NewUnsupportedOperationErrorwithprovider.GetProviderKey(). This keeps SGL aligned with the expanded provider surface while clearly marking these operations as unsupported.If you expect SGL to gain file/batch support later, you might optionally add
// TODOmarkers on these methods (or a shared comment block) so they’re easy to find across the provider stack when implementing real behavior.core/providers/parasail/parasail.go (1)
185-233: Unsupported File/Batch stubs look correct and consistent**The new File* and Batch* methods cleanly implement the provider interface by fast‑failing with
NewUnsupportedOperationErrorusing the correctRequestTypeconstants andGetProviderKey(), matching the existing unsupported operations pattern in this provider and ensuring consistent logging/extra fields downstream. If you find yourself adding more such stubs across providers, you might consider a small helper or shared mixin to reduce repetition, but it’s not necessary for this PR.ui/lib/constants/logs.ts (1)
119-119: Remove whitespace from empty line.Line 119 contains only whitespace characters. Empty lines should be completely empty for consistency.
tests/integrations/tests/test_bedrock.py (1)
982-987: Clarify skip messages for Bedrock-only testsThe gating logic is correct (run these tests only for
provider == "bedrock"), but the skip reasons are misleading:
- Line 985–987:
pytest.skip("Bedrock does not support file upload")whenprovider != "bedrock".- Line 1182–1185:
pytest.skip("Bedrock does not support file content download")whenprovider != "bedrock".These will print a message blaming Bedrock when the test is actually skipping non‑Bedrock providers.
Consider rewording to make the condition and reason consistent, e.g.:
- if provider != "bedrock": - pytest.skip("Bedrock does not support file upload") + if provider != "bedrock": + pytest.skip(f"File upload via this path is only supported for provider 'bedrock', got '{provider}'")and similarly for the file-content test.
Also applies to: 1182-1185
tests/integrations/config.json (1)
151-157: Consider isolating the sqlite logs DB path for testsEnabling
logs_storewithtype: "sqlite"at./logs.dbwill drop a DB file in the repo root for integration runs. If that’s undesirable, consider moving this under a test-specific temp/fixtures directory or making the path configurable via env to avoid working-tree noise.transports/bifrost-http/integrations/anthropic.go (1)
290-345: BatchRequestConverter wiring is solid; consider defensive provider lookupThe batch converters correctly:
- Derive
providerfrom the bifrost context/header (with Anthropic default),- Enforce a single
modelacross requests for non-Anthropic providers,- Add an OpenAI
Endpointfrom a commonurl, and- Handle Gemini ID format quirks.
One improvement: in
BatchListyou already guard provider extraction with anokcheck;BatchRetrieve,BatchCancel, andBatchResultsuse a direct type assertion and would panic if the context were ever missing or misconfigured. Mirroring the safe pattern (or factoring out a smallgetProviderFromContexthelper that returns a typed error) would make these routes more robust without changing behavior in normal flows.Also applies to: 365-381, 407-422, 443-458, 479-494
core/providers/bedrock/batch.go (1)
333-339: DummyModelavoids validation but is a bit of a hackSetting
Model: "dummy-model"here is a pragmatic way to satisfy validation forBifrostBatchListRequest, and it’s clearly documented as unused. Longer term, it’d be cleaner to relax or specialize validation for list‑type requests (soModelcan be optional there) instead of injecting a sentinel value that could leak into logs/metrics.core/providers/vertex/vertex.go (1)
1467-1470: Vertex BatchList/FileList correctly updated to single-key interfaceThese methods now match the new provider interface and still return
unsupported_operation, so runtime behavior remains unchanged. If you touch this again, consider renaming thekeysparameter inBatchListtokey(or_) for clarity, but it’s not functionally important.Also applies to: 1493-1496
core/bifrost.go (1)
3121-3139: resetBifrostRequest correctly clears new request fields but helper setters remain text/chat‑onlyResetting all file and batch request pointers (plus
ListModelsRequest) ensures pooledBifrostRequestobjects don’t leak state between calls, which is essential for the new centralization. If you expect plugins or helpers to manipulate providers/models for file and batch requests too, consider extendingSetProvider,SetModel,SetFallbacks, andSetRawRequestBodyto handle the new request variants; right now they are effectively no‑ops for those types.core/providers/gemini/gemini.go (1)
2239-2248: Gemini FileList: pagination cursor wiring is good; error message could be clearerUsing
Limit/Afterto drivepageSize/pageTokenand exposingNextPageTokenviaBifrostFileListResponse.Aftergives clients a clean pagination story. The only nit is the configuration error text “no keys provided” even though the method takes a singlekey; changing the message to “key is required” would better reflect the current signature.Also applies to: 2256-2267, 2277-2289, 2296-2315
core/schemas/bifrost.go (1)
160-180: Schema wiring for batch and file request/response types is consistent and completeAdding the batch/file request pointers to
BifrostRequest, updatingGetRequestFieldsto return the correct Provider/Model for each, and extendingBifrostResponse/GetExtraFieldswith the matching response types gives the core enough information to route, log, and decorate these operations uniformly. Functionally this looks sound; the only follow‑up worth considering is updating theBifrostRequestcomment to mention the new request variants so the documentation matches the actual union.Also applies to: 183-220, 293-313, 315-355
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
plugins/governance/go.sumis excluded by!**/*.sumplugins/logging/go.sumis excluded by!**/*.sumplugins/maxim/go.sumis excluded by!**/*.sumplugins/otel/go.sumis excluded by!**/*.sumplugins/semanticcache/go.sumis excluded by!**/*.sumplugins/telemetry/go.sumis excluded by!**/*.sumtransports/go.sumis excluded by!**/*.sum
📒 Files selected for processing (84)
core/bifrost.go(14 hunks)core/changelog.md(1 hunks)core/providers/anthropic/anthropic.go(3 hunks)core/providers/anthropic/batch.go(1 hunks)core/providers/azure/azure.go(3 hunks)core/providers/bedrock/batch.go(1 hunks)core/providers/bedrock/bedrock.go(5 hunks)core/providers/cerebras/cerebras.go(2 hunks)core/providers/cohere/cohere.go(2 hunks)core/providers/elevenlabs/elevenlabs.go(2 hunks)core/providers/gemini/gemini.go(3 hunks)core/providers/groq/groq.go(2 hunks)core/providers/mistral/batch.go(0 hunks)core/providers/mistral/files.go(0 hunks)core/providers/mistral/mistral.go(1 hunks)core/providers/nebius/nebius.go(8 hunks)core/providers/ollama/ollama.go(2 hunks)core/providers/openai/openai.go(5 hunks)core/providers/openrouter/openrouter.go(2 hunks)core/providers/parasail/batch.go(0 hunks)core/providers/parasail/files.go(0 hunks)core/providers/parasail/parasail.go(1 hunks)core/providers/perplexity/perplexity.go(2 hunks)core/providers/sgl/batch.go(0 hunks)core/providers/sgl/files.go(0 hunks)core/providers/sgl/sgl.go(1 hunks)core/providers/utils/utils.go(1 hunks)core/providers/vertex/vertex.go(2 hunks)core/schemas/batch.go(4 hunks)core/schemas/bifrost.go(4 hunks)core/schemas/chatcompletions.go(1 hunks)core/schemas/files.go(5 hunks)core/schemas/provider.go(2 hunks)core/utils.go(2 hunks)core/version(1 hunks)docs/docs.json(2 hunks)docs/integrations/anthropic-sdk/files-and-batch.mdx(1 hunks)docs/integrations/anthropic-sdk/overview.mdx(3 hunks)docs/integrations/bedrock-sdk/files-and-batch.mdx(1 hunks)docs/integrations/bedrock-sdk/overview.mdx(3 hunks)docs/integrations/genai-sdk/files-and-batch.mdx(1 hunks)docs/integrations/genai-sdk/overview.mdx(3 hunks)docs/integrations/openai-sdk/files-and-batch.mdx(1 hunks)docs/integrations/openai-sdk/overview.mdx(3 hunks)framework/changelog.md(1 hunks)framework/configstore/migrations.go(1 hunks)framework/version(1 hunks)plugins/governance/changelog.md(1 hunks)plugins/governance/go.mod(1 hunks)plugins/governance/version(1 hunks)plugins/jsonparser/changelog.md(1 hunks)plugins/jsonparser/version(1 hunks)plugins/logging/changelog.md(1 hunks)plugins/logging/go.mod(1 hunks)plugins/logging/main.go(1 hunks)plugins/logging/version(1 hunks)plugins/maxim/changelog.md(1 hunks)plugins/maxim/go.mod(1 hunks)plugins/maxim/version(1 hunks)plugins/mocker/changelog.md(1 hunks)plugins/mocker/version(1 hunks)plugins/otel/changelog.md(1 hunks)plugins/otel/go.mod(1 hunks)plugins/otel/version(1 hunks)plugins/semanticcache/changelog.md(1 hunks)plugins/semanticcache/go.mod(1 hunks)plugins/semanticcache/version(1 hunks)plugins/telemetry/changelog.md(1 hunks)plugins/telemetry/go.mod(1 hunks)plugins/telemetry/version(1 hunks)tests/integrations/config.json(4 hunks)tests/integrations/tests/test_anthropic.py(1 hunks)tests/integrations/tests/test_bedrock.py(3 hunks)transports/bifrost-http/handlers/inference.go(0 hunks)transports/bifrost-http/handlers/middlewares.go(1 hunks)transports/bifrost-http/integrations/anthropic.go(6 hunks)transports/bifrost-http/integrations/bedrock.go(4 hunks)transports/bifrost-http/integrations/bedrock_test.go(1 hunks)transports/bifrost-http/integrations/openai.go(4 hunks)transports/bifrost-http/integrations/router.go(4 hunks)transports/changelog.md(1 hunks)transports/go.mod(1 hunks)transports/version(1 hunks)ui/lib/constants/logs.ts(3 hunks)
💤 Files with no reviewable changes (7)
- core/providers/sgl/batch.go
- core/providers/mistral/files.go
- core/providers/mistral/batch.go
- core/providers/parasail/files.go
- core/providers/parasail/batch.go
- transports/bifrost-http/handlers/inference.go
- core/providers/sgl/files.go
✅ Files skipped from review due to trivial changes (4)
- plugins/telemetry/changelog.md
- tests/integrations/tests/test_anthropic.py
- plugins/semanticcache/changelog.md
- plugins/mocker/changelog.md
🚧 Files skipped from review as they are similar to previous changes (39)
- plugins/telemetry/version
- framework/version
- plugins/maxim/version
- plugins/semanticcache/version
- plugins/jsonparser/version
- core/utils.go
- plugins/maxim/go.mod
- plugins/jsonparser/changelog.md
- core/providers/ollama/ollama.go
- plugins/maxim/changelog.md
- docs/docs.json
- core/schemas/files.go
- docs/integrations/genai-sdk/overview.mdx
- plugins/governance/changelog.md
- plugins/logging/changelog.md
- core/version
- core/schemas/chatcompletions.go
- core/providers/utils/utils.go
- transports/bifrost-http/integrations/bedrock_test.go
- framework/changelog.md
- plugins/logging/go.mod
- plugins/governance/go.mod
- transports/bifrost-http/integrations/openai.go
- plugins/telemetry/go.mod
- transports/go.mod
- transports/version
- core/providers/perplexity/perplexity.go
- plugins/logging/main.go
- docs/integrations/bedrock-sdk/overview.mdx
- docs/integrations/anthropic-sdk/overview.mdx
- plugins/governance/version
- transports/bifrost-http/handlers/middlewares.go
- transports/bifrost-http/integrations/bedrock.go
- core/providers/cerebras/cerebras.go
- framework/configstore/migrations.go
- docs/integrations/bedrock-sdk/files-and-batch.mdx
- core/providers/mistral/mistral.go
- core/changelog.md
- plugins/logging/version
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
plugins/otel/changelog.mdplugins/otel/go.modcore/schemas/batch.gocore/providers/bedrock/batch.goplugins/mocker/versioncore/providers/vertex/vertex.gocore/providers/elevenlabs/elevenlabs.gocore/providers/anthropic/batch.gocore/schemas/provider.gocore/providers/gemini/gemini.godocs/integrations/genai-sdk/files-and-batch.mdxtransports/bifrost-http/integrations/router.goui/lib/constants/logs.tsplugins/otel/versiondocs/integrations/openai-sdk/files-and-batch.mdxcore/providers/groq/groq.godocs/integrations/openai-sdk/overview.mdxdocs/integrations/anthropic-sdk/files-and-batch.mdxtests/integrations/config.jsoncore/schemas/bifrost.gocore/providers/cohere/cohere.gocore/providers/openai/openai.gocore/providers/anthropic/anthropic.gotransports/bifrost-http/integrations/anthropic.gocore/bifrost.gocore/providers/sgl/sgl.gocore/providers/bedrock/bedrock.gotransports/changelog.mdplugins/semanticcache/go.modcore/providers/openrouter/openrouter.gocore/providers/parasail/parasail.gocore/providers/nebius/nebius.gocore/providers/azure/azure.gotests/integrations/tests/test_bedrock.py
🧠 Learnings (4)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
core/schemas/batch.gocore/providers/bedrock/batch.gocore/providers/vertex/vertex.gocore/providers/elevenlabs/elevenlabs.gocore/providers/anthropic/batch.gocore/schemas/provider.gocore/providers/gemini/gemini.gotransports/bifrost-http/integrations/router.gocore/providers/groq/groq.gocore/schemas/bifrost.gocore/providers/cohere/cohere.gocore/providers/openai/openai.gocore/providers/anthropic/anthropic.gotransports/bifrost-http/integrations/anthropic.gocore/bifrost.gocore/providers/sgl/sgl.gocore/providers/bedrock/bedrock.gocore/providers/openrouter/openrouter.gocore/providers/parasail/parasail.gocore/providers/nebius/nebius.gocore/providers/azure/azure.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/integrations/router.gotransports/bifrost-http/integrations/anthropic.go
📚 Learning: 2025-12-11T11:58:25.307Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: core/providers/openai/responses.go:42-84
Timestamp: 2025-12-11T11:58:25.307Z
Learning: In core/providers/openai/responses.go (and related OpenAI response handling), document and enforce the API format constraint: if ResponsesReasoning != nil and the response contains content blocks, all content blocks should be treated as reasoning blocks by default. Implement type guards or parsing logic accordingly, and add unit tests to verify that when ResponsesReasoning is non-nil, content blocks are labeled as reasoning blocks. Include clear comments in the code explaining the rationale and ensure downstream consumers rely on this behavior.
Applied to files:
core/providers/openai/openai.go
📚 Learning: 2025-12-14T14:43:30.902Z
Learnt from: Radheshg04
Repo: maximhq/bifrost PR: 980
File: core/providers/openai/images.go:10-22
Timestamp: 2025-12-14T14:43:30.902Z
Learning: Enforce the OpenAI image generation SSE event type values across the OpenAI image flow in the repository: use "image_generation.partial_image" for partial chunks, "image_generation.completed" for the final result, and "error" for errors. Apply this consistently in schemas, constants, tests, accumulator routing, and UI code within core/providers/openai (and related Go files) to ensure uniform event typing and avoid mismatches.
Applied to files:
core/providers/openai/openai.go
🧬 Code graph analysis (16)
core/schemas/batch.go (1)
core/schemas/models.go (1)
Model(109-129)
core/providers/vertex/vertex.go (4)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (1)
BifrostError(432-441)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
core/providers/elevenlabs/elevenlabs.go (5)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (1)
BifrostError(432-441)ui/lib/types/logs.ts (1)
BifrostError(226-232)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
core/schemas/provider.go (4)
core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (1)
BifrostError(432-441)ui/lib/types/logs.ts (1)
BifrostError(226-232)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
core/providers/gemini/gemini.go (2)
core/schemas/batch.go (1)
BifrostBatchListRequest(112-127)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
core/providers/groq/groq.go (5)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (1)
BifrostError(432-441)ui/lib/types/logs.ts (1)
BifrostError(226-232)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
core/schemas/bifrost.go (3)
core/schemas/files.go (10)
BifrostFileUploadRequest(53-67)BifrostFileListRequest(109-126)BifrostFileRetrieveRequest(139-150)BifrostFileDeleteRequest(172-182)BifrostFileContentRequest(194-204)BifrostFileUploadResponse(90-106)BifrostFileListResponse(129-136)BifrostFileRetrieveResponse(153-169)BifrostFileDeleteResponse(185-191)BifrostFileContentResponse(207-213)core/schemas/batch.go (10)
BifrostBatchCreateRequest(65-82)BifrostBatchListRequest(112-127)BifrostBatchRetrieveRequest(144-151)BifrostBatchCancelRequest(192-199)BifrostBatchResultsRequest(214-224)BifrostBatchCreateResponse(85-109)BifrostBatchListResponse(130-141)BifrostBatchRetrieveResponse(154-189)BifrostBatchCancelResponse(202-211)BifrostBatchResultsResponse(258-267)core/schemas/provider.go (1)
Provider(312-359)
core/providers/cohere/cohere.go (4)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (1)
BifrostBatchListRequest(112-127)core/schemas/bifrost.go (1)
BifrostError(432-441)core/schemas/files.go (1)
BifrostFileListRequest(109-126)
core/providers/openai/openai.go (4)
core/schemas/account.go (1)
Key(8-18)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)core/providers/utils/utils.go (3)
SetExtraHeaders(179-209)CheckOperationAllowed(473-485)NewConfigurationError(550-560)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)
core/providers/anthropic/anthropic.go (4)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/providers/utils/utils.go (2)
CheckOperationAllowed(473-485)NewConfigurationError(550-560)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
core/bifrost.go (3)
core/schemas/bifrost.go (18)
RequestType(85-85)BatchCreateRequest(100-100)BatchListRequest(101-101)BatchRetrieveRequest(102-102)BatchCancelRequest(103-103)BatchResultsRequest(104-104)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)FileContentRequest(109-109)BifrostRequest(160-180)ListModelsRequest(88-88)TextCompletionRequest(89-89)ResponsesRequest(93-93)EmbeddingRequest(95-95)SpeechRequest(96-96)TranscriptionRequest(98-98)core/utils.go (1)
Ptr(56-58)core/schemas/utils.go (1)
Ptr(16-18)
core/providers/sgl/sgl.go (4)
core/schemas/files.go (5)
BifrostFileUploadRequest(53-67)BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)BifrostFileRetrieveRequest(139-150)BifrostFileRetrieveResponse(153-169)core/schemas/bifrost.go (11)
BifrostError(432-441)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)FileContentRequest(109-109)BatchCreateRequest(100-100)BatchListRequest(101-101)BatchRetrieveRequest(102-102)BatchCancelRequest(103-103)BatchResultsRequest(104-104)core/providers/utils/utils.go (1)
NewUnsupportedOperationError(455-467)core/schemas/batch.go (3)
BifrostBatchCreateRequest(65-82)BifrostBatchCreateResponse(85-109)BifrostBatchListRequest(112-127)
core/providers/openrouter/openrouter.go (3)
core/schemas/bifrost.go (5)
BifrostError(432-441)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)core/schemas/files.go (4)
BifrostFileUploadRequest(53-67)BifrostFileListRequest(109-126)BifrostFileContentRequest(194-204)BifrostFileContentResponse(207-213)core/providers/utils/utils.go (1)
NewUnsupportedOperationError(455-467)
core/providers/parasail/parasail.go (3)
core/schemas/bifrost.go (11)
BifrostError(432-441)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)FileContentRequest(109-109)BatchCreateRequest(100-100)BatchListRequest(101-101)BatchRetrieveRequest(102-102)BatchCancelRequest(103-103)BatchResultsRequest(104-104)ui/lib/types/logs.ts (1)
BifrostError(226-232)core/providers/utils/utils.go (1)
NewUnsupportedOperationError(455-467)
core/providers/nebius/nebius.go (1)
core/providers/utils/utils.go (2)
ShouldSendBackRawRequest(607-612)NewUnsupportedOperationError(455-467)
core/providers/azure/azure.go (3)
core/schemas/account.go (2)
Key(8-18)AzureKeyConfig(22-26)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)
🪛 Ruff (0.14.8)
tests/integrations/tests/test_bedrock.py
1503-1503: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (27)
plugins/otel/version (1)
1-1: Version bump looks goodSimple version update only; no behavioral impact within this file.
plugins/semanticcache/go.mod (1)
87-87: Good catch: security updates in weaviate v1.33.4.Weaviate versions <= 1.33.3 are impacted by CVE-2025-67818 and CVE-2025-67819, so this patch-level bump is worthwhile. The change is consistent with other modules in the PR.
plugins/otel/go.mod (1)
83-83: Consistent update with semanticcache module.Same patch-level bump as the other modules; aligns with security improvements.
core/providers/nebius/nebius.go (2)
71-72: LGTM on raw request flag propagation.The
ShouldSendBackRawRequestcalls are consistently wired into all OpenAI-compatible request handlers, correctly passing the context and provider-level default.Also applies to: 89-90, 111-112, 140-141, 165-166, 213-214
238-286: LGTM on unsupported operation stubs.All batch and file operation stubs correctly follow the established pattern, using
providerUtils.NewUnsupportedOperationErrorwith appropriate request types. The use of blank identifiers for unused parameters is idiomatic.plugins/mocker/version (1)
1-1: Version bump looks fine
plugins/mocker/versioncorrectly advances to1.3.48; nothing else to change.tests/integrations/config.json (2)
40-43: Vertex/Bedrock key config renames look correctUsing
project_id/regionundervertex_key_configand explicitly wiring Bedrockregionfromenv.AWS_REGIONaligns with the newer key schema and should keep provider setup consistent.Also applies to: 135-139
158-170: Direct keys disabled in test config – ensure no tests assume they’re on
client.allow_direct_keys: falseis a safer default, but any integration tests or examples that rely on direct key passthrough must not use this config (or must override it). Just confirm test coverage matches this stance.transports/bifrost-http/integrations/anthropic.go (1)
518-519: Provider propagation via bifrostContextKeyProvider is consistentStoring
schemas.ModelProviderunderbifrostContextKeyProviderinextractAnthropicBatchCreateParamslines up with how the batch converters now read provider state and matches the pattern used in list/retrieve/cancel/results and file helpers.core/providers/openrouter/openrouter.go (1)
291-293: OpenRouter batch/file stubs correctly follow the new single‑key interfaceUpdating
BatchListand all File* methods to accept a singleschemas.Keyand to consistently returnNewUnsupportedOperationErrorkeeps OpenRouter aligned with the provider interface while clearly signalling unsupported operations, with structured error codes for logging.Also applies to: 311-333
docs/integrations/openai-sdk/overview.mdx (1)
2-5: Docs updates nicely surface Files/Batch and related SDK guidesThe updated frontmatter, explicit pointer to Quickstart for enabling direct keys, and the new Supported Features + Next Steps bullets (including Files and Batch API, Anthropic/GenAI SDKs, configuration, and core features) make this page a solid entry point into the broader docs set and line up with the new batch/file capabilities.
Also applies to: 212-213, 374-385
core/providers/cohere/cohere.go (1)
849-852: Cohere BatchList/FileList single-key signatures look correctThese stubs now match the single-key provider interface while preserving the existing
unsupported_operationbehavior; no further changes needed here.Also applies to: 874-877
docs/integrations/anthropic-sdk/files-and-batch.mdx (1)
1-618: Anthropic Files & Batch docs are coherent and aligned with the gateway designThe page accurately reflects the cross‑provider routing model (
x-model-providerheader), provider limitations (e.g., Bedrock/file‑based batches), and the inline vs file‑based split across providers. Code samples and workflows are consistent with the described behavior.docs/integrations/openai-sdk/files-and-batch.mdx (1)
1-673: OpenAI Files & Batch docs match the multi‑provider gateway behaviorThe docs correctly describe using
extra_body/extra_queryfor provider selection, per‑provider storage/batch specifics (OpenAI/Gemini native storage, Bedrock S3, Anthropic inline), and end‑to‑end workflows. Nothing blocking from a correctness perspective.core/providers/groq/groq.go (1)
257-260: Groq BatchList/FileList stubs align with the new provider interfaceSignatures are updated to accept a single
schemas.Keyand the methods continue to surfaceunsupported_operation, which is consistent with the rest of the Groq provider surface.Also applies to: 283-285
core/providers/anthropic/anthropic.go (2)
1113-1205: Anthropic BatchList single‑key refactor looks correctKey validation, URL/query construction, and error handling are consistent with the rest of the provider; no issues spotted.
1586-1685: Anthropic FileList URL building and auth handling look soundThe single‑key signature,
key.Valueguard, query parameter assembly, and headers (includinganthropic-beta) are wired correctly; behavior aligns with other Anthropic operations.core/providers/bedrock/bedrock.go (3)
1293-1420: Bedrock FileUpload gating and S3 upload flow look goodOperation gating, BedrockKeyConfig validation, S3 key construction, SigV4 signing, and response mapping are consistent with other Bedrock S3 helpers; no issues spotted here.
2053-2171: Bedrock BatchList single‑key implementation is consistent and safeBatch listing correctly requires a non‑nil
BedrockKeyConfig, builds the jobs URL withmaxResults/nextToken, signs via SigV4, and maps summaries intoBifrostBatchListResponse; no additional issues noted.
2475-2588: Bedrock BatchResults flow is solid but relies on the FileList fix aboveThe pagination over
FileListplus per‑fileFileContentparsing and JSONL aggregation are structured well; onceFileListstops gating onkey.Valueand safely usesBedrockKeyConfig, this end‑to‑end results path should behave correctly.core/providers/openai/openai.go (3)
2245-2347: OpenAI FileList single‑key refactor and query handling look goodRequest URL construction, query parameters (purpose/limit/after/order), and optional Authorization header from
key.Valueare all wired correctly; response mapping toBifrostFileListResponseis consistent with existing patterns.
2647-2740: Confirm stricter key requirement for OpenAI BatchList
BatchListnow returns a configuration error whenkey.Value == ""and otherwise uses that value for the Authorization header. This is stricter than other OpenAI methods (e.g.,FileList, chat/embedding) that allow emptykey.Valueand rely onnetworkConfig.ExtraHeaders/context for auth (useful for proxies/gateways).If you don’t support keyless/proxy setups for batch listing, this is fine; otherwise consider relaxing the guard (or checking for an existing Authorization header instead of only
key.Value).
2481-2540: OpenAI FileContent content type fallback remains correctUsing the response
Content-Typeheader with a safe fallback toapplication/octet-streamand copying the body into a fresh slice is appropriate; no changes needed here.core/bifrost.go (3)
872-921: Batch and file public methods now correctly route through the shared Bifrost pipelineAll Batch* and File* request methods now construct a pooled
BifrostRequest, setRequestType, attach the concrete request, and delegate tohandleRequest, aligning them with the rest of the API (text/chat/responses/etc.). This ensures consistent queuing, retries, plugins, and error shaping for these operations and looks correct givenhandleProviderRequest’s new cases.Also applies to: 923-954, 956-995, 997-1036, 1038-1087, 1089-1300
2248-2256: Setting StatusCode on validation errors for streaming requests is a good additionPopulating
StatusCodeon validation failures inhandleStreamRequestmakes the resultingBifrostErrorunambiguous for HTTP transports (e.g., mapping to a 400 instead of a generic 500) while keeping existing extra fields behavior intact.
2823-2937: New handleProviderRequest cases for batch and file operations are wired correctlyThe added switch cases for all file and batch
RequestTypes invoke the matching provider methods and populate the correspondingBifrostResponsefields, with consistent error-short‑circuit behavior and a safe default for unsupported types. This cleanly centralizes provider dispatch for the new APIs.core/providers/gemini/gemini.go (1)
1627-1636: Gemini BatchList: single-key API and pagination behavior look correctThe move to a single
Keyparameter with an explicit non-empty check, plus building/batcheswithpageSizeandpageToken, matches the updated provider interface. Treating 404/405 as “listing not available” and returning an empty list (with properExtraFieldsand latency) is a reasonable compatibility choice.Also applies to: 1644-1694, 1701-1739
akshaydeo
force-pushed
the
12-15-batch_request_logging
branch
from
dfbc9b8 to
aeb87c5
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
transports/bifrost-http/integrations/bedrock_test.go (1)
186-193: Align assertion message with renamedBatchRequestConverterfieldYou’re correctly asserting on
routes[i].BatchRequestConverter, but the failure message still mentionsBatchCreateRequestConverter, which can be misleading when debugging.Consider updating the message:
-assert.NotNil(t, routes[i].BatchRequestConverter, "batch route %d BatchCreateRequestConverter should not be nil", i) +assert.NotNil(t, routes[i].BatchRequestConverter, "batch route %d BatchRequestConverter should not be nil", i)tests/integrations/tests/test_bedrock.py (1)
1181-1186: Similarly clarify skip reason for file content download testSame issue for
test_15_file_content: behavior is Bedrock‑only, message reads as if Bedrock lacks support.Suggested tweak:
- if provider != "bedrock": - pytest.skip("Bedrock does not support file content download") + if provider != "bedrock": + pytest.skip(f"File content download test is only implemented for the bedrock provider (got {provider})")core/providers/nebius/nebius.go (1)
17-54: NebiusProvider does not propagate config.SendBackRawRequest into sendBackRawRequest
sendBackRawRequestis introduced on the struct and then fed intoShouldSendBackRawRequest(...), butNewNebiusProvidernever initializes it fromconfig.SendBackRawRequest. As a result, Nebius will never emit raw requests unless the context explicitly sets the override, which is inconsistent with other providers and the config flag’s meaning.Suggest wiring it up in the constructor:
func NewNebiusProvider(config *schemas.ProviderConfig, logger schemas.Logger) (*NebiusProvider, error) { @@ - return &NebiusProvider{ - logger: logger, - client: client, - networkConfig: config.NetworkConfig, - sendBackRawResponse: config.SendBackRawResponse, - }, nil + return &NebiusProvider{ + logger: logger, + client: client, + networkConfig: config.NetworkConfig, + sendBackRawRequest: config.SendBackRawRequest, + sendBackRawResponse: config.SendBackRawResponse, + }, nil }
♻️ Duplicate comments (4)
tests/integrations/tests/test_bedrock.py (1)
1493-1504: Fix unnecessary f‑string to satisfy Ruff F541 (duplicate of prior review)Static analysis (Ruff F541) still flags
print(f"stopping the job")as an f‑string without placeholders. Only the second print needs changing; thecreate_responseprint is fine.- print(f"create_response: {create_response}") - - print(f"stopping the job") + print(f"create_response: {create_response}") + + print("stopping the job")docs/docs.json (1)
74-112: Add GenAI SDKfiles-and-batchpage to navigation (duplicate of prior comment)OpenAI, Anthropic, and Bedrock SDK groups all list both
overviewandfiles-and-batch, but the GenAI SDK group only includes the overview page. Sincedocs/integrations/genai-sdk/files-and-batch.mdxexists, omitting it frompagesmakes the sidebar inconsistent and hides the page from navigation.Consider:
{ "group": "GenAI SDK", "icon": "g", "pages": [ - "integrations/genai-sdk/overview" + "integrations/genai-sdk/overview", + "integrations/genai-sdk/files-and-batch" ] },core/providers/bedrock/bedrock.go (1)
1422-1485: BedrockFileListstill risks nil‑pointer onBedrockKeyConfig(panic) and should guard it
FileListnow uses S3 directly and signs requests with:if key.BedrockKeyConfig.Region != nil { region = *key.BedrockKeyConfig.Region } ... if err := signAWSRequest(ctx, httpReq, key.BedrockKeyConfig.AccessKey, key.BedrockKeyConfig.SecretKey, key.BedrockKeyConfig.SessionToken, region, "s3", providerName); ...If
key.BedrockKeyConfigis nil (misconfigured key, tests, or future call sites), this will panic instead of returning a configuration error. Other Bedrock methods (FileUpload,FileRetrieve,FileDelete,FileContent, batch APIs) already guardBedrockKeyConfigfirst.Recommend aligning
FileListwith that pattern:func (provider *BedrockProvider) FileList(ctx context.Context, key schemas.Key, request *schemas.BifrostFileListRequest) (*schemas.BifrostFileListResponse, *schemas.BifrostError) { @@ - providerName := provider.GetProviderKey() + providerName := provider.GetProviderKey() + + if key.BedrockKeyConfig == nil { + return nil, providerUtils.NewConfigurationError("bedrock key config is not provided", providerName) + } + config := key.BedrockKeyConfig @@ - region := DefaultBedrockRegion - if key.BedrockKeyConfig.Region != nil { - region = *key.BedrockKeyConfig.Region - } + region := DefaultBedrockRegion + if config.Region != nil { + region = *config.Region + } @@ - // Sign request for S3 - if err := signAWSRequest(ctx, httpReq, key.BedrockKeyConfig.AccessKey, key.BedrockKeyConfig.SecretKey, key.BedrockKeyConfig.SessionToken, region, "s3", providerName); err != nil { + // Sign request for S3 + if err := signAWSRequest(ctx, httpReq, config.AccessKey, config.SecretKey, config.SessionToken, region, "s3", providerName); err != nil { return nil, err }This prevents panics and matches the configuration handling elsewhere in the Bedrock provider.
core/providers/azure/azure.go (1)
946-982: Past review concern may be outdated.A previous review flagged that FileList should support bearer-token auth when
key.Value == "", but the current implementation usesvalidateKeyConfigForFiles(lines 821-829) which only checks endpoint configuration and does not validatekey.Valueat all. ThesetAzureAuthmethod (line 982) handles bearer-token authentication from context, so bearer tokens should work correctly without additional validation logic.
🧹 Nitpick comments (12)
transports/bifrost-http/handlers/middlewares.go (1)
84-91: Content-Type check may be too strict for JSON variants.The exact match
"application/json"will fail for requests with charset suffixes like"application/json; charset=utf-8". Consider usingstrings.HasPrefixor checking if the content type contains"application/json".- if len(bodyBytes) > 0 && string(ctx.Request.Header.Peek("Content-Type")) == "application/json" { + contentType := string(ctx.Request.Header.Peek("Content-Type")) + if len(bodyBytes) > 0 && (contentType == "application/json" || strings.HasPrefix(contentType, "application/json;")) {core/providers/bedrock/batch.go (1)
333-339: Avoid hard‑coded “dummy-model” sentinel in batch list requestsUsing a magic string for
Modelavoids validation errors but may leak confusing values into logs/metrics or future model-based logic. Consider one of:
- Relax validation to allow empty
Modelfor list‑style operations; or- Use a well‑named constant (e.g.,
schemas.BatchListSentinelModel) so downstream code can reliably treat it as “no concrete model”.This keeps the behavior while making the intent explicit and easier to maintain.
core/schemas/batch.go (1)
114-114: Consider addingomitemptyfor JSON tag consistency.The newly added
Modelfields lackomitemptyin their JSON tags, whileBifrostBatchCreateRequest.Model(line 67) usesjson:"model,omitempty". This inconsistency means empty strings will be serialized for List/Retrieve/Cancel/Results requests but omitted for Create requests.If the Model field is optional for these operations (as suggested by the "Model hint for routing" comment in
BifrostBatchCreateRequest), consider addingomitempty:type BifrostBatchListRequest struct { Provider ModelProvider `json:"provider"` - Model string `json:"model"` + Model string `json:"model,omitempty"`Also applies to: 146-146, 194-194, 216-216
docs/integrations/openai-sdk/files-and-batch.mdx (1)
425-462: Minor step numbering inconsistency.The comments describe four steps (Step 1: Create content, Step 2: Upload, Step 3: Create batch, Step 4: Poll), but the print statements number them as Steps 1-3. This pattern appears in other workflow examples too.
Consider aligning the numbering:
# Step 1: Create OpenAI JSONL content ... -# Step 2: Upload file (uses OpenAI's native file storage) -print("Step 1: Uploading batch input file...") +# Step 2: Upload file +print("Step 2: Uploading batch input file...") ... -# Step 3: Create batch -print("Step 2: Creating batch job...") +# Step 3: Create batch +print("Step 3: Creating batch job...")core/providers/anthropic/anthropic.go (1)
1112-1152: AnthropicBatchListsingle‑key refactor looks correct; update stale commentThe switch to
key schemas.Keyand directx-api-keyusage is consistent with other Anthropic methods and the request URL construction looks sound. The inline comment// Use first key if availableis now misleading and can be dropped or reworded since only a single key is accepted.core/providers/vertex/vertex.go (1)
1467-1470: Nit: parameter namekeysis singular type
BatchListnow takes a singleschemas.Keybut the parameter name iskeys, which is slightly confusing. Consider renaming tokeyfor consistency with other providers and withFileList.core/providers/bedrock/bedrock.go (1)
1294-1299: FileUpload: gating + logging addition is fine; consider consistent logger usageThe early
CheckOperationAllowedguard and extra error log are reasonable and won’t change behavior. For consistency with other Bedrock logs, you may want to avoid%sformatting and instead log the error as a structured field, e.g.Error("file upload operation not allowed", "error", err).core/providers/openai/openai.go (2)
2245-2286: OpenAI FileList single-key + pagination flow looks good; consider including model in error contextThe move to a single
schemas.Keyand query-string construction viaurl.Valuesis correct and matches the new request shape. As a small improvement for observability, you could passrequest.ModelintoParseOpenAIErrorinstead of""so file-list errors carry model context in logs/extra fields.
2647-2735: OpenAI BatchList single-key implementation is sound; optional improvement for ParseOpenAIError model fieldThe BatchList refactor to single-key with
limit/afterquery params is consistent with OpenAI’s batch API and the shared schema. Similar to FileList, you may want to passrequest.Modelinstead of""intoParseOpenAIErrorso batch-list failures are tagged with the model in error extra fields.core/providers/gemini/gemini.go (2)
1627-1735: Gemini BatchList behavior and empty-list fallback look correct; consider enriching error metadataBatch listing with
pageSize/pageTokenand the 404/405 → empty-list fallback are well thought out and match the schema. For better diagnostics, you could also populateModel: request.Modelin theRequestMetadatapassed toparseGeminiErrorso errors from this endpoint carry the model hint.
2235-2339: Gemini FileList pagination mapping is correct; optional: add model to error contextThe FileList implementation correctly maps
Limit/After↔pageSize/pageTokenand surfacesNextPageTokenviaHasMoreandAfter. As with BatchList, you might want to includeModel: request.Modelin theRequestMetadatayou pass toparseGeminiErrorto get slightly richer logging when the list call fails.core/schemas/bifrost.go (1)
160-220: New File/Batch request wiring in BifrostRequest/GetRequestFields is consistent; consider updating setters too**The additions of
FileUploadRequest,FileListRequest,FileRetrieveRequest,FileDeleteRequest,FileContentRequestand the variousBatch*Requestfields, plus the corresponding cases inGetRequestFields, look coherent and align with the new schemas.If you expect generic code to call
SetProvider/SetModel/SetFallbacksonBifrostRequest, you may want to extend those switches to cover the new request types at some point so they benefit from the same normalization path, even if there are no fallbacks today.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
plugins/governance/go.sumis excluded by!**/*.sumplugins/logging/go.sumis excluded by!**/*.sumplugins/maxim/go.sumis excluded by!**/*.sumplugins/otel/go.sumis excluded by!**/*.sumplugins/semanticcache/go.sumis excluded by!**/*.sumplugins/telemetry/go.sumis excluded by!**/*.sumtransports/go.sumis excluded by!**/*.sum
📒 Files selected for processing (85)
core/bifrost.go(14 hunks)core/changelog.md(1 hunks)core/providers/anthropic/anthropic.go(3 hunks)core/providers/anthropic/batch.go(1 hunks)core/providers/azure/azure.go(3 hunks)core/providers/bedrock/batch.go(1 hunks)core/providers/bedrock/bedrock.go(5 hunks)core/providers/cerebras/cerebras.go(2 hunks)core/providers/cohere/cohere.go(2 hunks)core/providers/elevenlabs/elevenlabs.go(2 hunks)core/providers/gemini/gemini.go(3 hunks)core/providers/groq/groq.go(2 hunks)core/providers/mistral/batch.go(0 hunks)core/providers/mistral/files.go(0 hunks)core/providers/mistral/mistral.go(1 hunks)core/providers/nebius/nebius.go(8 hunks)core/providers/ollama/ollama.go(2 hunks)core/providers/openai/openai.go(5 hunks)core/providers/openrouter/openrouter.go(2 hunks)core/providers/parasail/batch.go(0 hunks)core/providers/parasail/files.go(0 hunks)core/providers/parasail/parasail.go(1 hunks)core/providers/perplexity/perplexity.go(2 hunks)core/providers/sgl/batch.go(0 hunks)core/providers/sgl/files.go(0 hunks)core/providers/sgl/sgl.go(1 hunks)core/providers/utils/utils.go(1 hunks)core/providers/vertex/vertex.go(2 hunks)core/schemas/batch.go(4 hunks)core/schemas/bifrost.go(4 hunks)core/schemas/chatcompletions.go(1 hunks)core/schemas/files.go(5 hunks)core/schemas/provider.go(2 hunks)core/utils.go(2 hunks)core/version(1 hunks)docs/docs.json(2 hunks)docs/integrations/anthropic-sdk/files-and-batch.mdx(1 hunks)docs/integrations/anthropic-sdk/overview.mdx(3 hunks)docs/integrations/bedrock-sdk/files-and-batch.mdx(1 hunks)docs/integrations/bedrock-sdk/overview.mdx(3 hunks)docs/integrations/genai-sdk/files-and-batch.mdx(1 hunks)docs/integrations/genai-sdk/overview.mdx(3 hunks)docs/integrations/openai-sdk/files-and-batch.mdx(1 hunks)docs/integrations/openai-sdk/overview.mdx(3 hunks)framework/changelog.md(1 hunks)framework/configstore/migrations.go(1 hunks)framework/version(1 hunks)plugins/governance/changelog.md(1 hunks)plugins/governance/go.mod(1 hunks)plugins/governance/version(1 hunks)plugins/jsonparser/changelog.md(1 hunks)plugins/jsonparser/version(1 hunks)plugins/logging/changelog.md(1 hunks)plugins/logging/go.mod(1 hunks)plugins/logging/main.go(1 hunks)plugins/logging/version(1 hunks)plugins/maxim/changelog.md(1 hunks)plugins/maxim/go.mod(1 hunks)plugins/maxim/version(1 hunks)plugins/mocker/changelog.md(1 hunks)plugins/mocker/version(1 hunks)plugins/otel/changelog.md(1 hunks)plugins/otel/converter.go(3 hunks)plugins/otel/go.mod(1 hunks)plugins/otel/version(1 hunks)plugins/semanticcache/changelog.md(1 hunks)plugins/semanticcache/go.mod(1 hunks)plugins/semanticcache/version(1 hunks)plugins/telemetry/changelog.md(1 hunks)plugins/telemetry/go.mod(1 hunks)plugins/telemetry/version(1 hunks)tests/integrations/config.json(4 hunks)tests/integrations/tests/test_anthropic.py(1 hunks)tests/integrations/tests/test_bedrock.py(3 hunks)transports/bifrost-http/handlers/inference.go(0 hunks)transports/bifrost-http/handlers/middlewares.go(1 hunks)transports/bifrost-http/integrations/anthropic.go(6 hunks)transports/bifrost-http/integrations/bedrock.go(4 hunks)transports/bifrost-http/integrations/bedrock_test.go(1 hunks)transports/bifrost-http/integrations/openai.go(4 hunks)transports/bifrost-http/integrations/router.go(4 hunks)transports/changelog.md(1 hunks)transports/go.mod(1 hunks)transports/version(1 hunks)ui/lib/constants/logs.ts(3 hunks)
💤 Files with no reviewable changes (7)
- transports/bifrost-http/handlers/inference.go
- core/providers/mistral/files.go
- core/providers/mistral/batch.go
- core/providers/parasail/files.go
- core/providers/sgl/files.go
- core/providers/parasail/batch.go
- core/providers/sgl/batch.go
✅ Files skipped from review due to trivial changes (3)
- plugins/jsonparser/changelog.md
- core/version
- transports/changelog.md
🚧 Files skipped from review as they are similar to previous changes (35)
- plugins/maxim/go.mod
- plugins/telemetry/version
- plugins/semanticcache/go.mod
- plugins/semanticcache/version
- plugins/semanticcache/changelog.md
- plugins/jsonparser/version
- plugins/governance/version
- plugins/otel/go.mod
- plugins/mocker/changelog.md
- plugins/governance/changelog.md
- core/utils.go
- framework/configstore/migrations.go
- core/schemas/provider.go
- docs/integrations/bedrock-sdk/files-and-batch.mdx
- core/providers/cerebras/cerebras.go
- transports/bifrost-http/integrations/bedrock.go
- transports/version
- core/providers/ollama/ollama.go
- tests/integrations/tests/test_anthropic.py
- plugins/maxim/changelog.md
- docs/integrations/genai-sdk/overview.mdx
- core/providers/elevenlabs/elevenlabs.go
- docs/integrations/anthropic-sdk/overview.mdx
- plugins/telemetry/go.mod
- transports/bifrost-http/integrations/anthropic.go
- plugins/mocker/version
- framework/version
- core/providers/cohere/cohere.go
- plugins/otel/version
- plugins/logging/changelog.md
- core/schemas/files.go
- tests/integrations/config.json
- plugins/logging/main.go
- plugins/maxim/version
- ui/lib/constants/logs.ts
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
transports/bifrost-http/integrations/router.gocore/providers/anthropic/batch.godocs/integrations/openai-sdk/files-and-batch.mdxplugins/governance/go.moddocs/integrations/openai-sdk/overview.mdxdocs/integrations/anthropic-sdk/files-and-batch.mdxtransports/bifrost-http/handlers/middlewares.gocore/providers/utils/utils.gocore/schemas/batch.gocore/providers/anthropic/anthropic.gocore/bifrost.gocore/providers/perplexity/perplexity.gocore/providers/vertex/vertex.gotransports/bifrost-http/integrations/openai.goplugins/otel/converter.goplugins/telemetry/changelog.mdcore/providers/azure/azure.gocore/providers/openai/openai.gocore/providers/groq/groq.gotransports/go.modtests/integrations/tests/test_bedrock.pytransports/bifrost-http/integrations/bedrock_test.gocore/schemas/chatcompletions.goplugins/logging/versionplugins/otel/changelog.mdcore/providers/bedrock/bedrock.godocs/docs.jsonframework/changelog.mdplugins/logging/go.moddocs/integrations/bedrock-sdk/overview.mdxdocs/integrations/genai-sdk/files-and-batch.mdxcore/providers/gemini/gemini.gocore/providers/parasail/parasail.gocore/providers/mistral/mistral.gocore/providers/sgl/sgl.gocore/providers/nebius/nebius.gocore/providers/bedrock/batch.gocore/changelog.mdcore/providers/openrouter/openrouter.gocore/schemas/bifrost.go
🧠 Learnings (4)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
transports/bifrost-http/integrations/router.gocore/providers/anthropic/batch.gotransports/bifrost-http/handlers/middlewares.gocore/providers/utils/utils.gocore/schemas/batch.gocore/providers/anthropic/anthropic.gocore/bifrost.gocore/providers/perplexity/perplexity.gocore/providers/vertex/vertex.gotransports/bifrost-http/integrations/openai.goplugins/otel/converter.gocore/providers/azure/azure.gocore/providers/openai/openai.gocore/providers/groq/groq.gotransports/bifrost-http/integrations/bedrock_test.gocore/schemas/chatcompletions.gocore/providers/bedrock/bedrock.gocore/providers/gemini/gemini.gocore/providers/parasail/parasail.gocore/providers/mistral/mistral.gocore/providers/sgl/sgl.gocore/providers/nebius/nebius.gocore/providers/bedrock/batch.gocore/providers/openrouter/openrouter.gocore/schemas/bifrost.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/integrations/router.gotransports/bifrost-http/handlers/middlewares.gotransports/bifrost-http/integrations/openai.gotransports/bifrost-http/integrations/bedrock_test.go
📚 Learning: 2025-12-11T11:58:25.307Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: core/providers/openai/responses.go:42-84
Timestamp: 2025-12-11T11:58:25.307Z
Learning: In core/providers/openai/responses.go (and related OpenAI response handling), document and enforce the API format constraint: if ResponsesReasoning != nil and the response contains content blocks, all content blocks should be treated as reasoning blocks by default. Implement type guards or parsing logic accordingly, and add unit tests to verify that when ResponsesReasoning is non-nil, content blocks are labeled as reasoning blocks. Include clear comments in the code explaining the rationale and ensure downstream consumers rely on this behavior.
Applied to files:
core/providers/openai/openai.go
📚 Learning: 2025-12-14T14:43:30.902Z
Learnt from: Radheshg04
Repo: maximhq/bifrost PR: 980
File: core/providers/openai/images.go:10-22
Timestamp: 2025-12-14T14:43:30.902Z
Learning: Enforce the OpenAI image generation SSE event type values across the OpenAI image flow in the repository: use "image_generation.partial_image" for partial chunks, "image_generation.completed" for the final result, and "error" for errors. Apply this consistently in schemas, constants, tests, accumulator routing, and UI code within core/providers/openai (and related Go files) to ensure uniform event typing and avoid mismatches.
Applied to files:
core/providers/openai/openai.go
🧬 Code graph analysis (18)
core/providers/utils/utils.go (2)
core/utils.go (1)
Ptr(56-58)core/schemas/utils.go (1)
Ptr(16-18)
core/schemas/batch.go (1)
core/schemas/models.go (1)
Model(109-129)
core/providers/anthropic/anthropic.go (5)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (3)
BifrostError(432-441)Anthropic(37-37)FileListRequest(106-106)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)core/providers/utils/utils.go (1)
CheckOperationAllowed(473-485)
core/bifrost.go (3)
core/schemas/bifrost.go (18)
RequestType(85-85)BatchCreateRequest(100-100)BatchListRequest(101-101)BatchRetrieveRequest(102-102)BatchCancelRequest(103-103)BatchResultsRequest(104-104)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)FileContentRequest(109-109)BifrostRequest(160-180)ListModelsRequest(88-88)TextCompletionRequest(89-89)ResponsesRequest(93-93)EmbeddingRequest(95-95)SpeechRequest(96-96)TranscriptionRequest(98-98)core/utils.go (1)
Ptr(56-58)core/schemas/utils.go (1)
Ptr(16-18)
transports/bifrost-http/integrations/openai.go (1)
transports/bifrost-http/integrations/router.go (2)
BatchRequestConverter(101-101)BatchRequest(81-88)
plugins/otel/converter.go (3)
core/schemas/files.go (5)
BifrostFileUploadRequest(53-67)BifrostFileListRequest(109-126)BifrostFileRetrieveRequest(139-150)BifrostFileDeleteRequest(172-182)BifrostFileContentRequest(194-204)core/schemas/batch.go (5)
BifrostBatchCreateRequest(65-82)BifrostBatchListRequest(112-127)BifrostBatchRetrieveRequest(144-151)BifrostBatchCancelRequest(192-199)BifrostBatchResultsRequest(214-224)core/schemas/bifrost.go (10)
BatchCreateRequest(100-100)BatchListRequest(101-101)BatchRetrieveRequest(102-102)BatchCancelRequest(103-103)BatchResultsRequest(104-104)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)FileContentRequest(109-109)
core/providers/azure/azure.go (3)
core/schemas/account.go (2)
Key(8-18)AzureKeyConfig(22-26)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)
core/providers/openai/openai.go (4)
core/schemas/account.go (1)
Key(8-18)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)core/providers/utils/utils.go (1)
SetExtraHeaders(179-209)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)
core/providers/groq/groq.go (3)
core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (1)
BifrostError(432-441)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
transports/bifrost-http/integrations/bedrock_test.go (1)
transports/bifrost-http/integrations/router.go (1)
BatchRequestConverter(101-101)
core/providers/bedrock/bedrock.go (6)
core/providers/utils/utils.go (1)
CheckOperationAllowed(473-485)core/schemas/bifrost.go (2)
Bedrock(38-38)BifrostError(432-441)core/schemas/account.go (1)
Key(8-18)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)ui/lib/types/logs.ts (1)
BifrostError(226-232)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)
core/providers/gemini/gemini.go (5)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (3)
BifrostError(432-441)Gemini(48-48)FileListRequest(106-106)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)core/providers/utils/utils.go (1)
CheckOperationAllowed(473-485)
core/providers/parasail/parasail.go (2)
core/schemas/bifrost.go (11)
BifrostError(432-441)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)FileContentRequest(109-109)BatchCreateRequest(100-100)BatchListRequest(101-101)BatchRetrieveRequest(102-102)BatchCancelRequest(103-103)BatchResultsRequest(104-104)core/providers/utils/utils.go (1)
NewUnsupportedOperationError(455-467)
core/providers/sgl/sgl.go (3)
core/schemas/files.go (10)
BifrostFileUploadRequest(53-67)BifrostFileUploadResponse(90-106)BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)BifrostFileRetrieveRequest(139-150)BifrostFileRetrieveResponse(153-169)BifrostFileDeleteRequest(172-182)BifrostFileDeleteResponse(185-191)BifrostFileContentRequest(194-204)BifrostFileContentResponse(207-213)core/schemas/bifrost.go (11)
BifrostError(432-441)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)FileContentRequest(109-109)BatchCreateRequest(100-100)BatchListRequest(101-101)BatchRetrieveRequest(102-102)BatchCancelRequest(103-103)BatchResultsRequest(104-104)core/providers/utils/utils.go (1)
NewUnsupportedOperationError(455-467)
core/providers/nebius/nebius.go (3)
core/providers/utils/utils.go (2)
ShouldSendBackRawRequest(607-612)NewUnsupportedOperationError(455-467)core/schemas/batch.go (3)
BifrostBatchCreateRequest(65-82)BifrostBatchCreateResponse(85-109)BifrostBatchListRequest(112-127)core/schemas/bifrost.go (11)
BifrostError(432-441)BatchCreateRequest(100-100)BatchListRequest(101-101)BatchRetrieveRequest(102-102)BatchCancelRequest(103-103)BatchResultsRequest(104-104)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)FileContentRequest(109-109)
core/providers/bedrock/batch.go (1)
core/schemas/models.go (1)
Model(109-129)
core/providers/openrouter/openrouter.go (4)
core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (2)
BifrostError(432-441)FileUploadRequest(105-105)core/schemas/files.go (2)
BifrostFileUploadRequest(53-67)BifrostFileUploadResponse(90-106)core/providers/utils/utils.go (1)
NewUnsupportedOperationError(455-467)
core/schemas/bifrost.go (2)
core/schemas/files.go (10)
BifrostFileUploadRequest(53-67)BifrostFileListRequest(109-126)BifrostFileRetrieveRequest(139-150)BifrostFileDeleteRequest(172-182)BifrostFileContentRequest(194-204)BifrostFileUploadResponse(90-106)BifrostFileListResponse(129-136)BifrostFileRetrieveResponse(153-169)BifrostFileDeleteResponse(185-191)BifrostFileContentResponse(207-213)core/schemas/batch.go (10)
BifrostBatchCreateRequest(65-82)BifrostBatchListRequest(112-127)BifrostBatchRetrieveRequest(144-151)BifrostBatchCancelRequest(192-199)BifrostBatchResultsRequest(214-224)BifrostBatchCreateResponse(85-109)BifrostBatchListResponse(130-141)BifrostBatchRetrieveResponse(154-189)BifrostBatchCancelResponse(202-211)BifrostBatchResultsResponse(258-267)
🪛 Ruff (0.14.8)
tests/integrations/tests/test_bedrock.py
1503-1503: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (47)
plugins/logging/version (1)
1-1: LGTM! Version bump is appropriate.The patch-level version increment (1.3.48 → 1.3.49) is appropriate for adding logging support for batch and file requests without introducing breaking changes.
core/schemas/chatcompletions.go (1)
21-24: LGTM!The getter method is straightforward and correctly exposes the
RawRequestBodyfor logging purposes, aligning with the PR's objective to add logging support for batch and file requests.plugins/telemetry/changelog.md (1)
1-2: LGTM!The changelog entries now properly document both the feature addition (batch and file request logging) and the dependency upgrades. This addresses the previous review feedback.
core/providers/anthropic/batch.go (1)
260-266: LGTM - Fallback addresses the empty string issue.The nil-check with a 24-hour fallback from
time.Now()correctly prevents the empty string serialization issue flagged in the previous review. The defensive comment is appropriate.docs/integrations/openai-sdk/overview.mdx (1)
380-385: LGTM!The Next Steps section appropriately includes the new Files and Batch API documentation link, aligning with the PR's feature additions.
core/providers/utils/utils.go (1)
455-467: LGTM!Adding the explicit
Codefield enriches the error payload for unsupported operations, which improves telemetry tagging and client-side error handling. The use ofschemas.Ptris consistent with the existing import.docs/integrations/genai-sdk/files-and-batch.mdx (1)
1-981: Comprehensive documentation for Files and Batch API.The documentation is well-structured with clear cross-provider routing examples, appropriate warnings for Bedrock limitations, and complete end-to-end workflows. The code examples follow consistent patterns and include proper cleanup handling.
docs/integrations/anthropic-sdk/files-and-batch.mdx (1)
1-618: Well-structured documentation for Anthropic SDK Files and Batch API.The documentation properly uses Anthropic's
beta.filesandbeta.messages.batchesnamespaces, includes cross-provider routing examples, and clearly documents Bedrock limitations.plugins/otel/converter.go (3)
370-550: LGTM - Consistent parameter extraction for file/batch operations.The new helper functions follow the established patterns in the file, properly handling optional fields with nil checks and extracting ExtraParams where applicable.
581-610: LGTM - Span name mapping is well-organized.The new request types are correctly mapped to descriptive span names following the
gen_ai.{category}.{operation}naming convention used by OpenTelemetry semantic conventions for GenAI.
875-1039: LGTM - Comprehensive response field extraction.The response handling properly extracts relevant fields for observability, including IDs, statuses, timestamps, and counts. The nil checks prevent panics on optional fields.
plugins/otel/changelog.md (1)
1-2: Changelog entry now accurately reflects feature + version bumpThe entry now clearly calls out the new batch/file logging feature alongside the core/framework bumps and matches the PR’s scope.
core/providers/parasail/parasail.go (1)
185-233: Unsupported file/batch stubs are consistent with existing provider behaviorThese methods correctly surface
unsupported_operationerrors usingNewUnsupportedOperationErrorand match the existing pattern for other unsupported Parasail operations.core/changelog.md (1)
1-5: Core changelog entries align with implemented featuresThe new feature/fix lines (batch/files support, Nebius provider, structured output, streaming fixes) match the code changes and are clearly described.
docs/integrations/bedrock-sdk/overview.mdx (1)
2-5: Bedrock SDK overview + navigation updates look consistent
- Generic “Overview” title and
bookicon are fine within the “Bedrock SDK” group.- “Learn more” now points to
../../quickstart/READMEand Next Steps links (Files and Batch API, What is an integration?, Configuration, Core Features) correctly reference the new files-and-batch page and existing docs via normalized relative paths.These docs changes align with the new Bedrock Files/Batch support.
Also applies to: 175-176, 222-225
tests/integrations/tests/test_bedrock.py (1)
1556-1561: Debug logging in batch e2e test is reasonableThe added
print(f"getting the bedrock client for provider {provider}")is helpful for diagnosing cross‑provider issues intest_20_batch_e2eand is low‑risk for CI logs.transports/bifrost-http/integrations/openai.go (1)
462-462: Consistent field rename applied across all batch routes.The rename from
BatchCreateRequestConvertertoBatchRequestConverteris correctly applied to all four batch endpoints (Create, List, Retrieve, Cancel). This better reflects that the converter handles all batch request types, not just create operations.Also applies to: 571-571, 617-617, 668-668
transports/bifrost-http/integrations/router.go (2)
100-101: Type and field rename correctly applied.The
BatchRequestConvertertype definition and its usage inRouteConfigare correctly updated. The rename better reflects that this converter handles all batch request types (Create, List, Retrieve, Cancel, Results), not just create operations.Also applies to: 272-272
435-437: Batch request handling correctly updated.The handler logic properly checks for
BatchRequestConverterpresence and invokes it to transform requests. The conditional check at line 435 correctly identifies batch routes.core/providers/perplexity/perplexity.go (1)
261-262: Method signatures updated to single-key pattern.The
BatchListandFileListmethod signatures are correctly updated from slice-based keys to single key, aligning with the broader provider interface changes across the codebase. Since these methods return unsupported operation errors for Perplexity, there's no functional change.Also applies to: 286-287
docs/integrations/openai-sdk/files-and-batch.mdx (2)
359-372: Verify Bedrock list batches parameters.The example shows
role_arninextra_queryfor listing batches. Typically,role_arnis required for batch creation operations but may not be needed for listing. Please verify this matches the Bedrock implementation.response = client.batches.list( limit=10, extra_query={ "provider": "bedrock", "model": "anthropic.claude-3-sonnet-20240229-v1:0", # Required for bedrock "role_arn": "arn:aws:iam::123456789:role/BedrockBatchRole", # Is this actually required for listing? } )
1-673: Comprehensive documentation for multi-provider Files and Batch API.This is a well-structured guide covering cross-provider file and batch operations with clear examples for OpenAI, Bedrock, Anthropic, and Gemini. The provider-specific notes table (lines 654-659) is particularly helpful for understanding the differences between providers.
core/providers/anthropic/anthropic.go (1)
1582-1605: AnthropicFileListURL/query handling is consistent with schemaThe new single‑key signature and
buildRequestURLusage, along withlimitandafter_idquery parameters, matchBifrostFileListRequestand Anthropic’s pagination model. No functional issues here.core/providers/vertex/vertex.go (1)
1493-1496: VertexFileListsingle‑key signature matches refactor pattern
FileListnow takes a singleschemas.Keyand still returns an unsupported‑operation error, which is consistent with the rest of the provider. No functional issues here.core/providers/groq/groq.go (2)
257-260: GroqBatchListsingle‑key signature aligns with provider interfaceThe updated signature to
(_ context.Context, _ schemas.Key, _ *schemas.BifrostBatchListRequest)keeps the method unsupported while matching the new single‑key interface. Looks good.
282-285: GroqFileListsingle‑key signature aligns with provider interfaceSame as
BatchList: signature matches the new single‑key contract, behavior unchanged as unsupported. No issues.core/providers/bedrock/bedrock.go (2)
2048-2064: BedrockBatchListsingle‑key refactor is consistent and safe
BatchListnow accepts a singleschemas.Key, validatesBedrockKeyConfigup front, and builds the request URL/SignatureV4 auth exactly as before. This aligns with the global single‑key refactor while preserving behavior.
2495-2519: BedrockBatchResultscorrectly reusesFileListfor paging S3 outputThe updated
BatchResultsloop now callsFileList(ctx, key, ...)with a single key and pages viaAfter/HasMore, reusing the S3 listing logic. This is a good consolidation; onceFileListhas a BedrockKeyConfig nil‑guard, this path is robust.To be safe, please run the Bedrock batch happy‑path tests (including a batch with multiple output shards) to confirm pagination via
Aftercontinues to work end‑to‑end after this refactor.core/bifrost.go (6)
912-921: LGTM!The refactor to use centralized
handleRequestis consistent with the broader pattern applied to batch and file operations. The logic correctly constructs the bifrostRequest, sets the request type, and delegates to the unified handler.
945-954: LGTM!All four batch methods (BatchList, BatchRetrieve, BatchCancel, BatchResults) follow the same centralized request handling pattern introduced for BatchCreate. The refactor is consistent and correct.
Also applies to: 986-995, 1027-1036, 1078-1087
1129-1138: LGTM!All five file methods (FileUpload, FileList, FileRetrieve, FileDelete, FileContent) correctly adopt the centralized request handling pattern. The refactor maintains consistency across all file operations.
Also applies to: 1168-1177, 1209-1218, 1250-1259, 1291-1300
2254-2254: LGTM!Setting
StatusCodetofasthttp.StatusBadRequestfor validation errors in streaming paths is appropriate and aligns with the centralized error handling improvements in this PR.
2862-2921: LGTM!The extensions to
handleProviderRequestproperly integrate all file and batch operations into the unified request lifecycle. Each case correctly invokes the provider method and populates the appropriate response field.
3122-3139: LGTM!All file and batch request fields are properly reset in
resetBifrostRequest, ensuring clean reuse of pooledBifrostRequestobjects and preventing state leakage between requests.core/providers/azure/azure.go (3)
944-944: LGTM!The signature change from
[]schemas.Keytoschemas.Keyaligns with the broader provider normalization effort mentioned in the PR summary, which standardizes FileList/BatchList to single-key signatures across multiple providers.
1397-1397: LGTM!The signature change from
[]schemas.Keytoschemas.Keyis consistent with the provider normalization effort and matches the FileList signature change.
965-973: LGTM!The URL construction in both methods correctly handles query parameters. FileList uses conditional encoding (line 971-973) that only appends "?" if parameters exist, while BatchList always appends "?" (line 1427) since
api-versionis always present. Both approaches are functionally correct.Also applies to: 1417-1427
core/schemas/bifrost.go (1)
292-355: GetExtraFields extension for File/Batch responses looks correct**Routing
GetExtraFieldsthrough the newFile*andBatch*response types is straightforward and matches the rest of the design; this will let logging/metrics code consumeExtraFieldsuniformly across the new operations.core/providers/nebius/nebius.go (1)
238-286: Nebius Batch/File unsupported-operation stubs are consistent with the shared pattern**The new
Batch*andFile*methods all clearly indicate that these operations are unsupported by Nebius viaNewUnsupportedOperationError, matching the cross-provider convention and giving callers a predictable error shape.core/providers/sgl/sgl.go (1)
223-271: SGL File/Batch unsupported-operation stubs align with provider interface expectations**The added File* and Batch* methods correctly expose the new API surface while returning
NewUnsupportedOperationErrorfor SGL, which keeps behavior explicit and consistent with how other unsupported operations are handled.core/providers/openrouter/openrouter.go (2)
291-291: LGTM! Signature normalized to single-key pattern.The BatchList signature change from
[]schemas.Keytoschemas.Keyaligns with the stack-wide normalization mentioned in the AI summary, where all providers are being updated to use single-key signatures for BatchList and FileList operations.
311-331: LGTM! Idiomatic parameter naming for unsupported operations.The file operation methods now use
_for all parameters, clearly signaling that these inputs are intentionally ignored since these operations are not supported by OpenRouter. This is idiomatic Go practice for unused parameters.core/providers/mistral/mistral.go (2)
260-283: LGTM! Batch operation stubs properly implemented.The batch operation methods (BatchCreate, BatchList, BatchRetrieve, BatchCancel, BatchResults) are correctly implemented as unsupported operation stubs. All methods use the single-key signature pattern consistent with the stack-wide normalization, and appropriately return unsupported operation errors since Mistral doesn't support these operations.
285-308: LGTM! File operation stubs properly implemented.The file operation methods (FileUpload, FileList, FileRetrieve, FileDelete, FileContent) are correctly implemented as unsupported operation stubs. All methods follow the consistent pattern of using ignored parameters (
_) and returning unsupported operation errors, appropriate since Mistral doesn't support these file operations.plugins/governance/go.mod (1)
87-87: Consistent with transports/go.mod update.The weaviate dependency version bump mirrors the change in transports/go.mod. Assuming the parent file's verification passes, this change is consistent with the ecosystem-wide update.
plugins/logging/go.mod (1)
85-85: Consistent coordinated update.All three modules (transports, governance, logging) are updated to weaviate v1.33.4 in lock-step, indicating a coordinated ecosystem-wide refresh.
transports/go.mod (1)
109-109: Remove verification requirement—weaviate v1.33.4 dependency update is unrelated to batch/file features.The patch update to v1.33.4 is a "Backup path Fix" with no connection to the batch and file request functionality this PR implements. The batch/file request features are handled entirely within Bifrost's own code (transports/bifrost-http/integrations/router.go); they do not depend on improvements from this weaviate version bump.
Likely an incorrect or invalid review comment.
akshaydeo
force-pushed
the
12-15-batch_request_logging
branch
2 times, most recently
from
68ab3ee to
dca1b2e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
transports/bifrost-http/handlers/middlewares.go (1)
115-121: Critical: Non-JSON request bodies are corrupted.The body is always marshaled and set back on the request (lines 115-121), even when the original request was not JSON. For non-JSON requests,
requestBodyremains an empty map (initialized at line 82), so these requests will have their bodies replaced with{}. This breaks file upload requests that usemultipart/form-dataor other non-JSON content types—a core feature mentioned in the PR objectives.Apply this diff to only marshal and update the body when it was originally JSON:
} } - // Marshal the body back to JSON - updatedBody, err := json.Marshal(requestBody) - if err != nil { - SendError(ctx, fasthttp.StatusInternalServerError, fmt.Sprintf("TransportInterceptor: Failed to marshal request body: %v", err)) - return - } - ctx.Request.SetBody(updatedBody) + // Marshal the body back to JSON only if it was originally JSON + if len(bodyBytes) > 0 && strings.HasPrefix(string(ctx.Request.Header.Peek("Content-Type")), "application/json") { + updatedBody, err := json.Marshal(requestBody) + if err != nil { + SendError(ctx, fasthttp.StatusInternalServerError, fmt.Sprintf("TransportInterceptor: Failed to marshal request body: %v", err)) + return + } + ctx.Request.SetBody(updatedBody) + } // Remove headers that were present originally but removed by pluginscore/providers/nebius/nebius.go (1)
48-53: InitializesendBackRawRequestfrom config.The
sendBackRawRequestfield is never initialized in the constructor, so it will always default tofalse. This breaks the feature, as users cannot configure it.Apply this diff:
return &NebiusProvider{ logger: logger, client: client, networkConfig: config.NetworkConfig, + sendBackRawRequest: config.SendBackRawRequest, sendBackRawResponse: config.SendBackRawResponse, }, niltests/integrations/tests/test_bedrock.py (1)
1181-1182: Clarify skip reason: message is inverted.The skip message says "Bedrock does not support file content download" but the condition skips when the provider is not bedrock. This is misleading—the intent appears to be restricting the test to Bedrock only.
if provider != "bedrock": - pytest.skip("Bedrock does not support file content download") + pytest.skip(f"File content download test is only implemented for Bedrock (got {provider})")ui/lib/constants/logs.ts (1)
29-41: Add batch and file operation types to theRequestTypesarray for filter consistency.The new batch and file operation types are defined in
RequestTypeLabelsandRequestTypeColorsbut missing from theRequestTypesarray (lines 29-41). SinceRequestTypesis used to populate the filter dropdown in the logs view, users cannot filter by these types. Add all 10 new types to maintain consistency and enable filtering.core/providers/bedrock/bedrock.go (1)
2478-2519: BatchResults integration with FileList/FileContent is correct once FileList nil-guard is fixed
BatchResultsnow:
- Uses
BatchRetrieveto locate the output S3 URI.- Pages through that prefix via
FileList(ctx, key, ...)and then downloads JSONL out files viaFileContentas needed, with a directFileContentfallback if listing fails.Functionally this is sound and aligns with the S3-based batch semantics, but it inherits the
FileListBedrockKeyConfig nil-deref risk noted above. OnceFileListguardsBedrockKeyConfigproperly, this flow should be robust.Also applies to: 2549-2577
♻️ Duplicate comments (2)
tests/integrations/tests/test_bedrock.py (1)
1500-1500: Fix unnecessary f-string (Ruff F541).
print(f"stopping the job")is an f-string without placeholders; it should be a plain string.- print(f"stopping the job") + print("stopping the job")core/providers/bedrock/bedrock.go (1)
1423-1463: Fix BedrockFileListBedrockKeyConfig nil handling (panic risk, affects BatchResults)
FileListnow runs on a singleschemas.Keybut still dereferenceskey.BedrockKeyConfigwithout a nil check:
if key.BedrockKeyConfig.Region != nil { ... }signAWSRequest(..., key.BedrockKeyConfig.AccessKey, ...)If
BedrockKeyConfigis nil (e.g., IAM-based config without explicit key config),FileListwill panic. This was already called out in a previous review and is still present in the updated single-key flow.BatchResultsnow depends onFileList, so the impact is broader.You’re already guarding
BedrockKeyConfigin other Bedrock methods (Embedding,FileRetrieve,FileDelete,FileContent,BatchList, etc.).FileListshould follow the same pattern.func (provider *BedrockProvider) FileList(ctx context.Context, key schemas.Key, request *schemas.BifrostFileListRequest) (*schemas.BifrostFileListResponse, *schemas.BifrostError) { if err := providerUtils.CheckOperationAllowed(schemas.Bedrock, provider.customProviderConfig, schemas.FileListRequest); err != nil { return nil, err } providerName := provider.GetProviderKey() + + // Ensure Bedrock-specific key config is present before using it + if key.BedrockKeyConfig == nil { + return nil, providerUtils.NewConfigurationError("bedrock key config is not provided", providerName) + } + + config := key.BedrockKeyConfig // Get S3 bucket from storage config or extra params @@ - region := DefaultBedrockRegion - if key.BedrockKeyConfig.Region != nil { - region = *key.BedrockKeyConfig.Region - } + region := DefaultBedrockRegion + if config.Region != nil { + region = *config.Region + } @@ - requestURL := fmt.Sprintf("https://%s.s3.%s.amazonaws.com/?%s", bucketName, region, params.Encode()) + requestURL := fmt.Sprintf("https://%s.s3.%s.amazonaws.com/?%s", bucketName, region, params.Encode()) @@ - if err := signAWSRequest(ctx, httpReq, key.BedrockKeyConfig.AccessKey, key.BedrockKeyConfig.SecretKey, key.BedrockKeyConfig.SessionToken, region, "s3", providerName); err != nil { + if err := signAWSRequest(ctx, httpReq, config.AccessKey, config.SecretKey, config.SessionToken, region, "s3", providerName); err != nil { return nil, err }This will eliminate the panic and keep FileList’s behavior consistent with the rest of the Bedrock S3 helpers used by
BatchResults.Also applies to: 1475-1485, 1515-1537, 2500-2519
🧹 Nitpick comments (12)
core/providers/mistral/mistral.go (1)
260-308: Unsupported batch/file stubs look correct and consistentThese new methods correctly surface
unsupported_operationerrors with the appropriateRequestTypevalues and provider key, matching the existing pattern used for other unsupported Mistral operations in this file. The signatures also align with the single‑key provider shape used elsewhere in the stack.One small optional nit: other unsupported methods here keep named parameters (
ctx,key,request) instead of_—you could mirror that for consistency, but it’s not functionally important.framework/configstore/tables/modelpricing.go (1)
31-34: Consider applying consistent GORM specifications across all nullable float pricing fields.This new file has two different patterns for nullable float fields: lines 31-34 and 37-40 (200k token and cache/batch pricing) include explicit
type:float(10,6)andcolumn:specifications, while lines 22-28 (128k token pricing) and earlier fields only havegorm:"default:null".If the explicit type and column mappings are required for database consistency, they should be applied uniformly across all nullable pricing fields. If they're only needed for the newer fields, add a comment explaining the difference.
core/schemas/batch.go (1)
111-127: Model field additions to batch list/retrieve/cancel/results—consider semantics and JSON tagsThe new
Model string \json:"model"`fields onBifrostBatchListRequest,BifrostBatchRetrieveRequest,BifrostBatchCancelRequest, andBifrostBatchResultsRequestmakemodeleffectively required in the JSON shape. That’s likely what you want for OpenAI/Gemini/Anthropic, but some providers (e.g., Bedrock) don’t have a meaningful model concept for these operations, which is whyToBifrostBatchListRequestinjects"dummy-model"`.It may be worth either (a) explicitly documenting that
modelis a routing hint that can be ignored by some providers, and/or (b) switching these fields tojson:"model,omitempty"and relying on central validation (e.g.,IsModelRequired) to enforce presence where truly necessary, so provider adapters don’t need sentinel values.Also applies to: 143-151, 191-199, 213-224
core/providers/bedrock/batch.go (1)
331-339: Use a named sentinel for Bedrock dummy model instead of a raw stringIn
ToBifrostBatchListRequest,"dummy-model"is injected purely to satisfy model validation and is never used downstream. To avoid any chance of this being confused with a real model ID later, consider promoting it to a package-levelconstwith an obviously sentinel-style value (e.g.,bedrockDummyBatchModelID = "__bedrock_batch_list_dummy__"), plus a brief comment explaining why it exists.docs/integrations/genai-sdk/files-and-batch.mdx (1)
908-915: Clarifyendedbatch state in Anthropic workflow vs job-state tableThe Anthropic workflow’s
terminal_statesincludes the literal"ended", while the “Batch Job States” table documents onlyJOB_STATE_*values. To avoid confusion, consider either mapping Anthropic’sendedinto one of the genericJOB_STATE_*states (and checking only those), or adding a short note/table entry explaining that Anthropic may surface anendedstate and how it relates to the generic job states.ui/lib/constants/logs.ts (1)
119-119: Minor: Remove extra blank line.There's an extra blank line that could be cleaned up for consistency.
file_content: "File Content", - - + } as const;core/bifrost.go (2)
872-921: Centralizing batch/file public APIs throughhandleRequestlooks correct; minor unused provider lookupAll batch and file request entrypoints now consistently construct a pooled
BifrostRequestand delegate tohandleRequest, so plugins, retries, fallbacks, and logging apply uniformly. Validation remains local to each method, so behavior is preserved while routing is unified.Only nit:
BatchCreateRequeststill resolvesprovider := bifrost.getProviderByKey(req.Provider)but then never uses it after the switch tohandleRequest, so that lookup can be dropped to avoid redundant work and confusion.- provider := bifrost.getProviderByKey(req.Provider) - if provider == nil { - return nil, &schemas.BifrostError{ - IsBifrostError: false, - Error: &schemas.ErrorField{ - Message: "provider not found for batch create request", - }, - } - } - bifrostReq := bifrost.getBifrostRequest() bifrostReq.RequestType = schemas.BatchCreateRequest bifrostReq.BatchCreateRequest = reqAlso applies to: 923-954, 956-995, 997-1036, 1038-1087, 1089-1138, 1140-1177, 1179-1218, 1220-1259, 1261-1300
2862-2922: Provider dispatch and request pooling correctly extended for batch/file operationsAdding explicit
File*andBatch*branches inhandleProviderRequestand clearing the corresponding pointers inresetBifrostRequestkeeps the worker dispatch and pooling logic consistent as new request types are introduced. This ensures reusedBifrostRequestinstances don’t leak prior batch/file state between calls.Just keep in mind that whenever new request types are added, both this switch and
resetBifrostRequestmust be updated together.Also applies to: 3121-3139
transports/bifrost-http/integrations/router.go (2)
100-105: Batch routing viaBatchRequestConverteris wired correctlyRenaming and centralizing batch conversion as
BatchRequestConverterand using it to:
- Classify routes as batch (
isBatchRoute := route.BatchRequestConverter != nil),- Short-circuit
createHandlerintohandleBatchRequest,keeps the batch API clearly separated from inference and file routes without changing semantics. The
BatchRequestwrapper remains the single entrypoint into the variousBatch*Bifrost methods, which makes future batch operations and logging easier to extend.Also applies to: 266-295, 337-340, 435-448, 696-825
989-1053: Streaming refinements (SSE vs eventstream, cancellation,[DONE]semantics) look solidKey streaming changes here are coherent and match the stated architecture:
handleStreamingRequestnow relies on a cancellable context fromConvertToBifrostContextand callscancel()on write errors or early failures, so upstream streams are terminated promptly when clients disconnect.- When
StreamConfigis nil, you now both cancel the context and drain the stream channel in a goroutine to avoid worker leaks.handleStreaming:
- Cleanly distinguishes Bedrock AWS eventstream (
eventstream.Encoder+BedrockStreamEvent.ToEncodedEvents) from SSE-based providers.- Supports both object-based converters (JSON →
data: {…}\n\n) and string-based converters for fully custom SSE payloads.- Restricts
[DONE]to classic SSE flows (OpenAI chat/text) and disables it for:
- Anthropic and
/responsespaths, which use typed SSE events,- GenAI and Bedrock routes, which either use their own formats or eventstream.
This should improve correctness around termination semantics and resource cleanup for all streaming APIs.
Also applies to: 1105-1139, 1165-1193, 1216-1233, 1286-1335
core/providers/azure/azure.go (1)
965-973: Minor: Conditional query-string check is redundant.Since
api-versionis always set (line 967),values.Encode()will never return an empty string, making the check on line 971 redundant. However, this is defensive coding and doesn't cause any issues—just a micro-optimization opportunity.core/schemas/bifrost.go (1)
170-179: File and batch request types integrated correctly.The new request fields and
GetRequestFieldscases follow the established pattern. All return(provider, model, nil)since file/batch operations don't support fallbacks, which is appropriate.Minor: Line 196 has trailing whitespace after the
returnstatement, but this is a pedantic formatting detail.Also applies to: 197-216
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
plugins/governance/go.sumis excluded by!**/*.sumplugins/logging/go.sumis excluded by!**/*.sumplugins/maxim/go.sumis excluded by!**/*.sumplugins/otel/go.sumis excluded by!**/*.sumplugins/semanticcache/go.sumis excluded by!**/*.sumplugins/telemetry/go.sumis excluded by!**/*.sumtransports/go.sumis excluded by!**/*.sum
📒 Files selected for processing (86)
core/bifrost.go(14 hunks)core/changelog.md(1 hunks)core/providers/anthropic/anthropic.go(3 hunks)core/providers/anthropic/batch.go(1 hunks)core/providers/azure/azure.go(3 hunks)core/providers/bedrock/batch.go(1 hunks)core/providers/bedrock/bedrock.go(5 hunks)core/providers/cerebras/cerebras.go(2 hunks)core/providers/cohere/cohere.go(2 hunks)core/providers/elevenlabs/elevenlabs.go(2 hunks)core/providers/gemini/gemini.go(3 hunks)core/providers/groq/groq.go(2 hunks)core/providers/mistral/batch.go(0 hunks)core/providers/mistral/files.go(0 hunks)core/providers/mistral/mistral.go(1 hunks)core/providers/nebius/nebius.go(8 hunks)core/providers/ollama/ollama.go(2 hunks)core/providers/openai/openai.go(5 hunks)core/providers/openrouter/openrouter.go(2 hunks)core/providers/parasail/batch.go(0 hunks)core/providers/parasail/files.go(0 hunks)core/providers/parasail/parasail.go(1 hunks)core/providers/perplexity/perplexity.go(2 hunks)core/providers/sgl/batch.go(0 hunks)core/providers/sgl/files.go(0 hunks)core/providers/sgl/sgl.go(1 hunks)core/providers/utils/utils.go(1 hunks)core/providers/vertex/vertex.go(2 hunks)core/schemas/batch.go(4 hunks)core/schemas/bifrost.go(4 hunks)core/schemas/chatcompletions.go(1 hunks)core/schemas/files.go(5 hunks)core/schemas/provider.go(2 hunks)core/utils.go(2 hunks)core/version(1 hunks)docs/docs.json(2 hunks)docs/integrations/anthropic-sdk/files-and-batch.mdx(1 hunks)docs/integrations/anthropic-sdk/overview.mdx(3 hunks)docs/integrations/bedrock-sdk/files-and-batch.mdx(1 hunks)docs/integrations/bedrock-sdk/overview.mdx(3 hunks)docs/integrations/genai-sdk/files-and-batch.mdx(1 hunks)docs/integrations/genai-sdk/overview.mdx(3 hunks)docs/integrations/openai-sdk/files-and-batch.mdx(1 hunks)docs/integrations/openai-sdk/overview.mdx(3 hunks)framework/changelog.md(1 hunks)framework/configstore/migrations.go(2 hunks)framework/configstore/tables/modelpricing.go(1 hunks)framework/version(1 hunks)plugins/governance/changelog.md(1 hunks)plugins/governance/go.mod(1 hunks)plugins/governance/version(1 hunks)plugins/jsonparser/changelog.md(1 hunks)plugins/jsonparser/version(1 hunks)plugins/logging/changelog.md(1 hunks)plugins/logging/go.mod(1 hunks)plugins/logging/main.go(1 hunks)plugins/logging/version(1 hunks)plugins/maxim/changelog.md(1 hunks)plugins/maxim/go.mod(1 hunks)plugins/maxim/version(1 hunks)plugins/mocker/changelog.md(1 hunks)plugins/mocker/version(1 hunks)plugins/otel/changelog.md(1 hunks)plugins/otel/converter.go(3 hunks)plugins/otel/go.mod(1 hunks)plugins/otel/version(1 hunks)plugins/semanticcache/changelog.md(1 hunks)plugins/semanticcache/go.mod(1 hunks)plugins/semanticcache/version(1 hunks)plugins/telemetry/changelog.md(1 hunks)plugins/telemetry/go.mod(1 hunks)plugins/telemetry/version(1 hunks)tests/integrations/config.json(4 hunks)tests/integrations/tests/test_anthropic.py(1 hunks)tests/integrations/tests/test_bedrock.py(2 hunks)transports/bifrost-http/handlers/inference.go(0 hunks)transports/bifrost-http/handlers/middlewares.go(1 hunks)transports/bifrost-http/integrations/anthropic.go(6 hunks)transports/bifrost-http/integrations/bedrock.go(4 hunks)transports/bifrost-http/integrations/bedrock_test.go(1 hunks)transports/bifrost-http/integrations/openai.go(4 hunks)transports/bifrost-http/integrations/router.go(4 hunks)transports/changelog.md(1 hunks)transports/go.mod(1 hunks)transports/version(1 hunks)ui/lib/constants/logs.ts(3 hunks)
💤 Files with no reviewable changes (7)
- core/providers/parasail/files.go
- transports/bifrost-http/handlers/inference.go
- core/providers/mistral/files.go
- core/providers/parasail/batch.go
- core/providers/mistral/batch.go
- core/providers/sgl/files.go
- core/providers/sgl/batch.go
✅ Files skipped from review due to trivial changes (1)
- plugins/jsonparser/version
🚧 Files skipped from review as they are similar to previous changes (37)
- plugins/semanticcache/go.mod
- transports/bifrost-http/integrations/bedrock_test.go
- plugins/telemetry/changelog.md
- docs/integrations/genai-sdk/overview.mdx
- plugins/jsonparser/changelog.md
- docs/integrations/bedrock-sdk/files-and-batch.mdx
- plugins/mocker/version
- docs/docs.json
- plugins/governance/version
- docs/integrations/anthropic-sdk/overview.mdx
- plugins/mocker/changelog.md
- plugins/maxim/version
- plugins/telemetry/version
- plugins/governance/go.mod
- transports/version
- core/utils.go
- core/providers/anthropic/batch.go
- core/providers/utils/utils.go
- core/providers/openai/openai.go
- plugins/otel/go.mod
- docs/integrations/openai-sdk/files-and-batch.mdx
- tests/integrations/config.json
- framework/configstore/migrations.go
- docs/integrations/openai-sdk/overview.mdx
- core/version
- plugins/otel/changelog.md
- tests/integrations/tests/test_anthropic.py
- core/providers/ollama/ollama.go
- plugins/otel/version
- core/providers/sgl/sgl.go
- plugins/logging/version
- framework/changelog.md
- plugins/maxim/changelog.md
- plugins/logging/go.mod
- plugins/semanticcache/changelog.md
- plugins/logging/changelog.md
- transports/bifrost-http/integrations/anthropic.go
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
framework/versioncore/schemas/provider.goplugins/governance/changelog.mddocs/integrations/bedrock-sdk/overview.mdxplugins/logging/main.gocore/providers/elevenlabs/elevenlabs.goplugins/otel/converter.gocore/providers/bedrock/batch.goplugins/telemetry/go.modtransports/go.modcore/providers/cohere/cohere.godocs/integrations/anthropic-sdk/files-and-batch.mdxcore/schemas/files.gocore/bifrost.gocore/providers/gemini/gemini.gocore/changelog.mdtransports/bifrost-http/integrations/router.gocore/providers/anthropic/anthropic.gocore/providers/perplexity/perplexity.gocore/providers/groq/groq.gocore/providers/azure/azure.goplugins/maxim/go.modtransports/bifrost-http/handlers/middlewares.gocore/schemas/batch.godocs/integrations/genai-sdk/files-and-batch.mdxcore/providers/cerebras/cerebras.gotransports/bifrost-http/integrations/bedrock.gocore/providers/vertex/vertex.gocore/schemas/bifrost.gocore/providers/openrouter/openrouter.gocore/providers/nebius/nebius.goframework/configstore/tables/modelpricing.goplugins/semanticcache/versioncore/providers/parasail/parasail.gotransports/bifrost-http/integrations/openai.gocore/schemas/chatcompletions.gocore/providers/mistral/mistral.gocore/providers/bedrock/bedrock.goui/lib/constants/logs.tstransports/changelog.mdtests/integrations/tests/test_bedrock.py
🧠 Learnings (3)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
core/schemas/provider.goplugins/logging/main.gocore/providers/elevenlabs/elevenlabs.goplugins/otel/converter.gocore/providers/bedrock/batch.gocore/providers/cohere/cohere.gocore/schemas/files.gocore/bifrost.gocore/providers/gemini/gemini.gotransports/bifrost-http/integrations/router.gocore/providers/anthropic/anthropic.gocore/providers/perplexity/perplexity.gocore/providers/groq/groq.gocore/providers/azure/azure.gotransports/bifrost-http/handlers/middlewares.gocore/schemas/batch.gocore/providers/cerebras/cerebras.gotransports/bifrost-http/integrations/bedrock.gocore/providers/vertex/vertex.gocore/schemas/bifrost.gocore/providers/openrouter/openrouter.gocore/providers/nebius/nebius.goframework/configstore/tables/modelpricing.gocore/providers/parasail/parasail.gotransports/bifrost-http/integrations/openai.gocore/schemas/chatcompletions.gocore/providers/mistral/mistral.gocore/providers/bedrock/bedrock.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/integrations/router.gotransports/bifrost-http/handlers/middlewares.gotransports/bifrost-http/integrations/bedrock.gotransports/bifrost-http/integrations/openai.go
📚 Learning: 2025-12-13T13:03:38.364Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 955
File: transports/bifrost-http/integrations/bedrock.go:682-697
Timestamp: 2025-12-13T13:03:38.364Z
Learning: In transports/bifrost-http/integrations/bedrock.go, implement FileRetrieveRequest.FileID and FileDeleteRequest.FileID to use the ETag from the If-Match header as the provider-specific file ID (e.g., OpenAI file-xxx, Gemini files/yyy), rather than deriving the ID from the S3 URI. The StorageConfig should supply the S3 bucket/prefix context, enabling cross-provider routing while keeping S3 as the backing store. This change should be accompanied by unit tests that verify the FileID source is the If-Match header, update any relevant request/response models, and add in-code comments to clarify the separation of provider file IDs from S3 paths.
Applied to files:
transports/bifrost-http/integrations/bedrock.go
🧬 Code graph analysis (20)
core/schemas/provider.go (4)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (1)
BifrostError(432-441)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
plugins/logging/main.go (3)
ui/lib/types/logs.ts (1)
TranscriptionInput(15-17)core/schemas/bifrost.go (1)
TranscriptionRequest(98-98)transports/bifrost-http/handlers/inference.go (1)
TranscriptionRequest(285-289)
core/providers/elevenlabs/elevenlabs.go (4)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (1)
BifrostError(432-441)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
plugins/otel/converter.go (2)
core/schemas/files.go (5)
BifrostFileUploadRequest(53-67)BifrostFileListRequest(109-126)BifrostFileRetrieveRequest(139-150)BifrostFileDeleteRequest(172-182)BifrostFileContentRequest(194-204)core/schemas/batch.go (5)
BifrostBatchCreateRequest(65-82)BifrostBatchListRequest(112-127)BifrostBatchRetrieveRequest(144-151)BifrostBatchCancelRequest(192-199)BifrostBatchResultsRequest(214-224)
core/providers/bedrock/batch.go (1)
core/schemas/models.go (1)
Model(109-129)
core/providers/cohere/cohere.go (3)
core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (1)
BifrostError(432-441)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
core/schemas/files.go (1)
core/schemas/models.go (1)
Model(109-129)
core/providers/gemini/gemini.go (5)
core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (3)
BifrostError(432-441)Gemini(48-48)FileListRequest(106-106)ui/lib/types/logs.ts (1)
BifrostError(226-232)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)core/providers/utils/utils.go (1)
CheckOperationAllowed(473-485)
core/providers/anthropic/anthropic.go (3)
core/schemas/batch.go (1)
BifrostBatchListRequest(112-127)core/schemas/bifrost.go (2)
Anthropic(37-37)FileListRequest(106-106)core/schemas/files.go (1)
BifrostFileListRequest(109-126)
core/providers/perplexity/perplexity.go (4)
core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (1)
BifrostError(432-441)ui/lib/types/logs.ts (1)
BifrostError(226-232)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
core/providers/groq/groq.go (3)
core/schemas/bifrost.go (1)
BifrostError(432-441)ui/lib/types/logs.ts (1)
BifrostError(226-232)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
core/providers/azure/azure.go (1)
core/schemas/account.go (2)
Key(8-18)AzureKeyConfig(22-26)
core/providers/cerebras/cerebras.go (2)
core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)
transports/bifrost-http/integrations/bedrock.go (1)
transports/bifrost-http/integrations/router.go (2)
BatchRequestConverter(101-101)BatchRequest(81-88)
core/providers/vertex/vertex.go (3)
core/schemas/batch.go (2)
BifrostBatchListRequest(112-127)BifrostBatchListResponse(130-141)core/schemas/bifrost.go (1)
BifrostError(432-441)core/schemas/files.go (2)
BifrostFileListRequest(109-126)BifrostFileListResponse(129-136)
core/schemas/bifrost.go (3)
core/schemas/files.go (10)
BifrostFileUploadRequest(53-67)BifrostFileListRequest(109-126)BifrostFileRetrieveRequest(139-150)BifrostFileDeleteRequest(172-182)BifrostFileContentRequest(194-204)BifrostFileUploadResponse(90-106)BifrostFileListResponse(129-136)BifrostFileRetrieveResponse(153-169)BifrostFileDeleteResponse(185-191)BifrostFileContentResponse(207-213)core/schemas/batch.go (10)
BifrostBatchCreateRequest(65-82)BifrostBatchListRequest(112-127)BifrostBatchRetrieveRequest(144-151)BifrostBatchCancelRequest(192-199)BifrostBatchResultsRequest(214-224)BifrostBatchCreateResponse(85-109)BifrostBatchListResponse(130-141)BifrostBatchRetrieveResponse(154-189)BifrostBatchCancelResponse(202-211)BifrostBatchResultsResponse(258-267)core/schemas/provider.go (1)
Provider(312-359)
core/providers/nebius/nebius.go (2)
core/providers/utils/utils.go (2)
ShouldSendBackRawRequest(607-612)NewUnsupportedOperationError(455-467)core/schemas/bifrost.go (11)
BifrostError(432-441)BatchCreateRequest(100-100)BatchListRequest(101-101)BatchRetrieveRequest(102-102)BatchCancelRequest(103-103)BatchResultsRequest(104-104)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)FileContentRequest(109-109)
core/providers/parasail/parasail.go (5)
core/schemas/files.go (1)
BifrostFileUploadRequest(53-67)core/schemas/bifrost.go (7)
BifrostError(432-441)FileUploadRequest(105-105)BatchCreateRequest(100-100)BatchListRequest(101-101)BatchRetrieveRequest(102-102)BatchCancelRequest(103-103)BatchResultsRequest(104-104)core/providers/utils/utils.go (1)
NewUnsupportedOperationError(455-467)core/schemas/batch.go (1)
BifrostBatchCreateRequest(65-82)transports/bifrost-http/handlers/inference.go (2)
BatchCreateRequest(292-299)BatchListRequest(302-307)
transports/bifrost-http/integrations/openai.go (1)
transports/bifrost-http/integrations/router.go (2)
BatchRequestConverter(101-101)BatchRequest(81-88)
core/providers/mistral/mistral.go (3)
core/schemas/bifrost.go (11)
BifrostError(432-441)BatchCreateRequest(100-100)BatchListRequest(101-101)BatchRetrieveRequest(102-102)BatchCancelRequest(103-103)BatchResultsRequest(104-104)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)FileContentRequest(109-109)ui/lib/types/logs.ts (1)
BifrostError(226-232)core/providers/utils/utils.go (1)
NewUnsupportedOperationError(455-467)
🪛 Ruff (0.14.8)
tests/integrations/tests/test_bedrock.py
1500-1500: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (55)
core/changelog.md (1)
1-5: Changelog entries look good—previous typo has been fixed.All entries are well-formatted, grammatically correct, and align with the PR objectives. The "structured output support" on line 3 correctly addresses the typo flagged in the previous review. The entries clearly document the batch/files API support, new Nebius provider, and streaming improvements.
transports/go.mod (1)
109-109: Clarify the purpose of the weaviate dependency version bump.The update from v1.33.1 to v1.33.4 is a patch version bump of an indirect dependency. While patch versions are typically backward compatible, the PR description (focused on batch request logging) doesn't explicitly explain why this weaviate upgrade is necessary or related to the feature changes. This same update appears across multiple modules in the PR.
- Confirm whether this weaviate v1.33.4 bump is intentional and required for the batch/file request logging feature, or if it's an accidental dependency resolution.
- If intentional, document the rationale in the PR or commit message for future maintainers.
plugins/telemetry/go.mod (1)
91-91: Verify consistency and rationale for coordinated weaviate dependency updates.Line 91 shows the same weaviate patch version bump (v1.33.1 → v1.33.4) observed in other modules. The coordinated nature of this change across
transports/go.mod,plugins/telemetry/go.mod, and (per AI summary) multiple other modules suggests intentionality, but the PR description does not document why this upgrade is necessary for the batch request logging feature.
- Ensure that this coordinated weaviate upgrade across all affected modules has been tested and doesn't introduce any incompatibilities.
- If this update is unrelated to the batch/file request logging feature, consider separating it into a dedicated dependency-bump PR for clarity and reviewability.
core/schemas/chatcompletions.go (1)
21-24: Getter implementation forGetRawRequestBodylooks correctThe method cleanly exposes the existing
RawRequestBodyfield via an exported getter and is consistent with the rest of the schema types. No issues from a correctness or API-shape perspective.plugins/maxim/go.mod (1)
87-87: Weaviate v1.33.4 is a valid and stable release with no known breaking changes. The patch-level update from v1.33.1 to v1.33.4 for this indirect dependency is safe to proceed with.docs/integrations/bedrock-sdk/overview.mdx (2)
222-222: Verify new files-and-batch documentation file exists.The "Files and Batch API" link references
./files-and-batch, which aligns with the PR's goal of adding file and batch request documentation. Ensure this file has been created and is properly structured before merging.
2-2: Documentation title and icon changes are appropriate.The change from "AWS Bedrock Integration" to "Overview" and the icon change from "b" to "book" align with making this page a generic overview. This supports the broader documentation reorganization mentioned in the PR.
Also applies to: 4-4
plugins/otel/converter.go (5)
370-453: LGTM! File request parameter extraction is well-implemented.The five file operation parameter extraction functions follow the established pattern and properly handle optional fields with appropriate nil/empty checks.
455-550: LGTM! Batch request parameter extraction is well-implemented.The five batch operation parameter extraction functions properly handle all request fields including pagination parameters, metadata, and provider-specific extra parameters.
581-610: LGTM! Span creation for batch and file operations is correctly integrated.The span names follow a logical naming convention (e.g., "gen_ai.batch.create", "gen_ai.file.upload") and all new request types are properly handled.
875-989: LGTM! Batch response handling is comprehensive.The implementation properly extracts all relevant fields from batch responses including IDs, statuses, timestamps, and request counts, with appropriate nil checks for optional fields.
990-1039: LGTM! File response handling is well-implemented.The implementation properly extracts file metadata and metrics. Note that file content is appropriately represented as byte length rather than actual content, which is good for logging and security.
core/providers/nebius/nebius.go (3)
21-21: Field addition looks good.The
sendBackRawRequestfield follows the same pattern assendBackRawResponseand is appropriately documented.
71-71: Consistent propagation ofsendBackRawRequestparameter.The addition of
providerUtils.ShouldSendBackRawRequest(ctx, provider.sendBackRawRequest)is consistent across all methods that delegate to OpenAI handlers. The utility function correctly allows context-based overrides while falling back to the provider's default configuration.Also applies to: 89-89, 111-111, 140-140, 165-165, 213-213
238-286: Well-structured stub implementations for batch and file operations.The ten stub methods correctly return unsupported operation errors, following the established pattern used for Speech and Transcription operations. The use of blank identifiers for unused parameters is idiomatic Go, and all methods properly satisfy the provider interface.
framework/configstore/tables/modelpricing.go (1)
37-40: Explicit tags support batch/cache pricing features.The addition of explicit column types and names for these batch and cache pricing fields aligns with the PR's objective of adding batch request logging support. The
float(10,6)precision provides 6 decimal places, which should accommodate typical token pricing (e.g., $0.000001 per token up to $9999.999999).Note: If extremely small fractional pricing becomes necessary in the future (< $0.000001), consider whether
float(10,6)provides adequate precision or if a higher precision type or decimal-based approach would be preferred. For current use cases, this should be sufficient.framework/version (1)
1-1: Version bump to 1.1.48 is consistentSingle-line framework version bump; no behavioral impact. Looks consistent with the governance changelog entry.
plugins/logging/main.go (1)
265-268: TranscriptionInput logging aligns with other request typesAssigning
req.TranscriptionRequest.InputintoInitialLogData.TranscriptionInputmirrors the existingSpeechInputhandling and ensures transcription requests have their inputs captured in logs. No issues.plugins/semanticcache/version (1)
1-1: Semantic cache plugin version bump looks goodPatch-level version increment only; no runtime behavior changes implied here.
plugins/governance/changelog.md (1)
1-1: Changelog entry correctly reflects core/framework bumpsThe note about upgrading core to 1.2.38 and framework to 1.1.48 matches the version files in this stack. No further changes needed.
core/schemas/provider.go (1)
311-358: Single-key BatchList/FileList signature is coherent with the rest of ProviderSwitching
BatchListandFileListto accept a singleKeyinstead of[]Keymatches all other non-list-model operations onProvider, which already use a single key. This simplifies implementations and keeps the interface consistent; any multi-key retry/fallback logic can now cleanly live at a higher layer. No issues from this file.core/schemas/files.go (1)
55-55: LGTM! Model field additions are consistent across all file request structs.The
Modelfield is uniformly added to all five file-related request types (BifrostFileUploadRequest,BifrostFileListRequest,BifrostFileRetrieveRequest,BifrostFileDeleteRequest,BifrostFileContentRequest), aligning with the parallel pattern in batch requests. This enables model-scoped operations for file API calls.Also applies to: 111-111, 141-141, 174-174, 196-196
core/providers/parasail/parasail.go (1)
185-233: LGTM! Unsupported operation stubs are correctly implemented.All ten file and batch operation stubs follow the established pattern in this provider:
- Blank identifiers for unused parameters
- Return
nilandNewUnsupportedOperationErrorwith the appropriate request type- Consistent with existing unsupported methods (e.g.,
TextCompletion,Embedding,Speech)This ensures the Parasail provider correctly surfaces unsupported operations with standardized error responses.
tests/integrations/tests/test_bedrock.py (1)
1496-1497: Debug print statements added for test visibility.These prints provide useful runtime visibility for batch job creation and status tracking during test execution. Consider using
loggingmodule for more structured output in production tests, but acceptable for integration test debugging.Also applies to: 1554-1556
transports/changelog.md (1)
1-6: LGTM! Changelog entries are well-documented.The changelog accurately reflects the PR's changes:
- Batch API support for multiple providers (OpenAI, Anthropic, Google Gemini, Bedrock)
- New Nebius provider support
- Related fixes for structured output and cost computation
The entries follow conventional commit format and provide good context for users.
transports/bifrost-http/integrations/openai.go (4)
462-483: LGTM! Field rename aligns with unified BatchRequest pattern.The
BatchRequestConverterfield correctly populates theBatchRequeststruct withType: schemas.BatchCreateRequestandCreateRequest, matching the unified batch handling pattern defined inrouter.go.
571-582: LGTM! List batch converter correctly implemented.Properly sets
Type: schemas.BatchListRequestwithListRequestfield, consistent with the unified batch request pattern.
617-637: LGTM! Retrieve batch converter correctly implemented.Provider-specific transformations (Gemini path format, Bedrock base64 decoding) are preserved while using the unified
BatchRequestConverterfield.
668-688: LGTM! Cancel batch converter correctly implemented.Consistent with the unified pattern, setting
Type: schemas.BatchCancelRequestwithCancelRequestfield.core/providers/perplexity/perplexity.go (2)
261-263: LGTM! Signature updated to single-key pattern.The
BatchListmethod signature now accepts a singleschemas.Keyinstead of a slice, aligning with the centralized request handling pattern across all providers.
286-288: LGTM! FileList signature updated consistently.Matches the single-key pattern applied uniformly across provider implementations.
core/providers/cerebras/cerebras.go (2)
220-222: LGTM! FileList signature aligned with single-key pattern.Consistent with the provider interface updates across the codebase.
245-247: LGTM! BatchList signature aligned with single-key pattern.Matches the uniform approach applied to all provider implementations.
transports/bifrost-http/integrations/bedrock.go (4)
204-271: LGTM! Batch create converter correctly uses unified field name.The converter properly handles cross-provider routing (Bedrock native vs OpenAI/Gemini via tags) while using the renamed
BatchRequestConverterfield.
303-313: LGTM! Batch list converter aligned with unified pattern.Correctly extracts provider from context and populates
BatchRequestwithBatchListRequesttype.
345-355: LGTM! Batch retrieve converter correctly implemented.Properly converts Bedrock-specific request to unified
BatchRequestformat.
387-397: LGTM! Batch cancel converter correctly implemented.Consistent use of
BatchRequestConverterfield with proper type setting.core/providers/elevenlabs/elevenlabs.go (2)
721-723: LGTM! BatchList signature aligned with single-key pattern.Consistent with the provider interface updates applied uniformly across all providers.
746-748: LGTM! FileList signature aligned with single-key pattern.Completes the uniform single-key pattern across the Elevenlabs provider implementation.
core/providers/vertex/vertex.go (2)
1467-1470: Single-key BatchList stub matches new provider interfaceThe updated
BatchListsignature taking a singleschemas.Keyis consistent with the centralized single-key provider interface, and the unsupported-operation error preserves previous behavior. No changes needed.
1493-1496: FileList remains correctly stubbed as unsupported
FileListnow accepts a single key and request while still returning anunsupported_operationerror. This aligns with the current lack of Vertex Files API support and the updated interface.core/providers/anthropic/anthropic.go (2)
1112-1202: BatchList single-key implementation and pagination look correct
- Uses
CheckOperationAllowedand a singleschemas.Keyconsistently.- Correctly builds
/v1/messages/batchesURL withlimit,before_id, andafter_idquery params.- Auth via
x-api-keyfromkey.Valueandanthropic-versionheader matches other Anthropic calls.- Response handling (
HandleProviderResponse→BifrostBatchListResponse) and latency/RawResponse wiring align with existing patterns.No issues from a correctness or interface standpoint.
1581-1678: FileList single-key implementation and query handling are consistent
- Switch to
FileList(ctx context.Context, key schemas.Key, request *schemas.BifrostFileListRequest)matches the new provider interface.- Builds
/v1/filesURL with optionallimitandafter_id(fromrequest.After), which matches Anthropic Files API pagination.- Uses
x-api-key,anthropic-version, andAnthropicFilesAPIBetaHeaderheaders consistently withFileUpload/FileRetrieve.- Response conversion to
schemas.BifrostFileListResponse(includingHasMore,Data, and ExtraFields) follows established patterns and respects raw request/response flags.Looks correct and in line with the rest of the Anthropic provider.
docs/integrations/anthropic-sdk/files-and-batch.mdx (1)
1-618: Docs align well with new Files & Batch capabilities and routing modelThe page clearly documents Anthropic SDK usage for Files/Batch, including
x-model-providerrouting, Bedrock limitations, and provider-specific examples. The workflows and examples are consistent with the single-key batch/file APIs and current provider support exposed in this PR.core/providers/cohere/cohere.go (1)
849-877: Cohere BatchList/FileList stubs correctly updated to single-key interfaceBoth
BatchListandFileListnow accept a singleschemas.Keyand request while still returningunsupported_operationerrors. This keeps Cohere’s behavior explicit (no batch/file support) and aligns its interface with the centralized single-key provider contract.core/bifrost.go (1)
2248-2256: Good: streaming validation failures now surface as HTTP 400
handleStreamRequestnow attachesStatusCode = fasthttp.StatusBadRequestonvalidateRequestfailures, which will let the HTTP transport map malformed streaming requests to a proper 400 instead of a generic error.Also applies to: 37-38
core/providers/groq/groq.go (1)
257-260: Groq batch/file signatures correctly aligned to single-key provider interfaceUpdating
BatchListandFileListto accept a singleschemas.Keymatches the new provider contract while preserving the existing “unsupported operation” behavior. No further changes needed here.Also applies to: 283-285
core/providers/bedrock/bedrock.go (2)
1293-1307: FileUpload logging / config-guard looks good
FileUploadnow logs a clearer message whenCheckOperationAllowedfails (only whenerr.Erroris non-nil) and still validatesBedrockKeyConfigbefore touching S3, which keeps error reporting sane and avoids nil derefs for upload.Also applies to: 1371-1374
2049-2087: Bedrock BatchList single-key refactor is consistent with other methods
BatchListnow:
- Accepts a single
schemas.Keyand validatesBedrockKeyConfigpresence.- Builds the regional URL and signs via
signAWSRequestusing that key.- Returns a
BifrostBatchListResponsewith pagination fields and metadata.This matches the single-key design used elsewhere and keeps error handling aligned with other Bedrock batch operations.
core/providers/azure/azure.go (2)
944-948: Past auth issue resolved; FileList now consistent with other file methods.The previous review flagged that FileList rejected requests when
key.Value == ""even if a bearer token was present in context. This change resolves that by:
- Removing the explicit
key.Value == ""check- Using
validateKeyConfigForFiles(validates Azure config only)- Calling
setAzureAuth(line 982) which handles bothkey.Valueand bearer-token auth from contextThis pattern now matches FileUpload, FileRetrieve, and other file methods, allowing bearer-token auth to work correctly.
1397-1401: BatchList signature change aligns with single-key pattern and resolves auth issue.Like FileList, BatchList now accepts a single
key schemas.Keyinstead of a slice, and usesvalidateKeyConfigForFiles+setAzureAuth(line 1436) to support both api-key and bearer-token authentication. This resolves the auth inconsistency flagged in the previous review.core/providers/gemini/gemini.go (2)
1670-1684: Empty list fallback for unsupported batch listing is appropriate.Returning an empty list (instead of an error) when the Gemini API returns 404 or 405 is a reasonable graceful degradation. The debug log on line 1674 makes the behavior transparent.
2305-2307: LGTM: Pagination token correctly mapped to After field.The
Afterfield is correctly populated with theNextPageTokenwhen present, enabling pagination continuation per theBifrostFileListResponseschema.core/schemas/bifrost.go (1)
303-312: Response types and GetExtraFields extended consistently.All new response fields are pointers (consistent with existing types), and
GetExtraFieldscorrectly returns theExtraFieldsfrom each new response type.Also applies to: 335-354
core/providers/openrouter/openrouter.go (1)
291-291: Signature changes and unused-parameter pattern applied correctly.The
BatchListsignature change to a singlekeyparameter aligns with the broader API migration. Using_for unused parameters in stub methods (FileUpload, FileList, etc.) is idiomatic Go and clearly signals these parameters are intentionally ignored since the methods return unsupported-operation errors.Also applies to: 311-332
akshaydeo
force-pushed
the
12-15-batch_request_logging
branch
from
dca1b2e to
3c90b3a
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
transports/bifrost-http/handlers/middlewares.go (1)
82-91: Non‑JSON request bodies are now being overwritten with{}by the interceptorWith the new guard:
requestBody := make(map[string]any) bodyBytes := ctx.Request.Body() if len(bodyBytes) > 0 && strings.HasPrefix(string(ctx.Request.Header.Peek("Content-Type")), "application/json") { if err := json.Unmarshal(bodyBytes, &requestBody); err != nil { logger.Warn(fmt.Sprintf("[transportInterceptor]: Failed to unmarshal request body: %v, skipping interceptor", err)) next(ctx) return } }and the unconditional marshal:
updatedBody, err := json.Marshal(requestBody) ... ctx.Request.SetBody(updatedBody)non‑JSON requests with a non‑empty body (e.g.
multipart/form-datafile uploads) now:
- Skip JSON unmarshalling (condition is false),
- Still go through the plugin loop with an empty
requestBodymap,- And then have their original body replaced by
{}(or modified JSON) viaSetBody.Previously, non‑JSON bodies caused
json.Unmarshalto fail, and the middleware immediately callednext(ctx)and returned, without running plugins or rewriting the body. So this change silently corrupts non‑JSON payloads, which is particularly risky for the new Files / batch APIs.I’d recommend restoring the old “JSON‑only interception” semantics and explicitly bypassing interception for non‑JSON payloads, while still tightening the Content‑Type check:
- requestBody := make(map[string]any) - bodyBytes := ctx.Request.Body() - if len(bodyBytes) > 0 && strings.HasPrefix(string(ctx.Request.Header.Peek("Content-Type")), "application/json") { - if err := json.Unmarshal(bodyBytes, &requestBody); err != nil { - // If body is not valid JSON, log warning and continue without interception - logger.Warn(fmt.Sprintf("[transportInterceptor]: Failed to unmarshal request body: %v, skipping interceptor", err)) - next(ctx) - return - } - } + requestBody := make(map[string]any) + bodyBytes := ctx.Request.Body() + contentType := strings.ToLower(strings.TrimSpace(string(ctx.Request.Header.Peek("Content-Type")))) + if len(bodyBytes) > 0 { + if strings.HasPrefix(contentType, "application/json") { + if err := json.Unmarshal(bodyBytes, &requestBody); err != nil { + // If body is not valid JSON, log warning and continue without interception + logger.Warn(fmt.Sprintf("[transportInterceptor]: Failed to unmarshal request body: %v, skipping interceptor", err)) + next(ctx) + return + } + } else { + // Non‑JSON payloads should bypass governance interception to avoid corrupting the body + next(ctx) + return + } + } @@ - // Marshal the body back to JSON - updatedBody, err := json.Marshal(requestBody) + // Marshal the body back to JSON (only reached for empty bodies or JSON requests) + updatedBody, err := json.Marshal(requestBody) if err != nil { SendError(ctx, fasthttp.StatusInternalServerError, fmt.Sprintf("TransportInterceptor: Failed to marshal request body: %v", err)) return } ctx.Request.SetBody(updatedBody)This:
- Keeps interception and body rewriting restricted to JSON requests (and empty bodies), preserving previous behavior.
- Prevents file/multipart and other non‑JSON transports from being broken.
- Normalizes
Content-Type(trim + lower‑case) so JSON detection is a bit more robust while still using the simpleHasPrefixcheck.Also applies to: 115-121
core/providers/gemini/gemini.go (1)
1627-1735: Guard against missingMetadatainBatchListto avoid potential panicsThe overall
BatchListimplementation (URL construction, pagination params, 404/405 → empty list behaviour) looks consistent with the rest of the Gemini provider and Bifrost batch schemas.One thing to harden: in the loop you assume
batch.Metadatais non‑nil:for _, batch := range geminiResp.Operations { data = append(data, schemas.BifrostBatchRetrieveResponse{ ID: extractBatchIDFromName(batch.Name), Status: ToBifrostBatchStatus(batch.Metadata.State), CreatedAt: parseGeminiTimestamp(batch.Metadata.CreateTime), // ... }) }If the API ever returns an entry without
metadata(or with a partial object), this will panic. InBatchCreateyou already explicitly validategeminiResp.Metadata/BatchStatsand fail fast instead of dereferencing blindly; mirroring that pattern here would make this code more robust.Consider something like:
for _, batch := range geminiResp.Operations { - data = append(data, schemas.BifrostBatchRetrieveResponse{ - ID: extractBatchIDFromName(batch.Name), - Object: "batch", - Status: ToBifrostBatchStatus(batch.Metadata.State), - CreatedAt: parseGeminiTimestamp(batch.Metadata.CreateTime), - OperationName: &batch.Name, - ExtraFields: schemas.BifrostResponseExtraFields{RequestType: schemas.BatchListRequest, Provider: providerName}, - }) + if batch.Metadata == nil { + return nil, providerUtils.NewBifrostOperationError("gemini batch list response missing metadata", nil, providerName) + } + + data = append(data, schemas.BifrostBatchRetrieveResponse{ + ID: extractBatchIDFromName(batch.Name), + Object: "batch", + Status: ToBifrostBatchStatus(batch.Metadata.State), + CreatedAt: parseGeminiTimestamp(batch.Metadata.CreateTime), + OperationName: &batch.Name, + ExtraFields: schemas.BifrostResponseExtraFields{ + RequestType: schemas.BatchListRequest, + Provider: providerName, + }, + }) }This keeps behaviour aligned with
BatchCreateand removes a class of potential crashes coming from provider response shape changes or partial results.tests/integrations/tests/test_bedrock.py (1)
1181-1183: Clarifypytest.skipmessage for non-Bedrock providersHere the test runs only for
provider == "bedrock", but the skip message says “Bedrock does not support file content download”, which reads as the opposite of what’s happening.Consider rewording to make the reason explicit:
- if provider != "bedrock": - pytest.skip("Bedrock does not support file content download") + if provider != "bedrock": + pytest.skip( + f"File content download is only implemented for the bedrock provider (got {provider})" + )
♻️ Duplicate comments (4)
core/providers/bedrock/bedrock.go (1)
1423-1485: AddBedrockKeyConfignil-guard inFileListto avoid panic
FileListdereferenceskey.BedrockKeyConfigforRegionand when callingsignAWSRequestwithout checking it for nil. If a Bedrock key is configured withoutBedrockKeyConfig(or a direct key is injected via context), this will panic instead of returning a structured configuration error. Other Bedrock file/batch methods (FileUpload,FileRetrieve,FileDelete,FileContent,BatchList,BatchRetrieve) all guardBedrockKeyConfigfirst.Align
FileListwith those methods:func (provider *BedrockProvider) FileList(ctx context.Context, key schemas.Key, request *schemas.BifrostFileListRequest) (*schemas.BifrostFileListResponse, *schemas.BifrostError) { if err := providerUtils.CheckOperationAllowed(schemas.Bedrock, provider.customProviderConfig, schemas.FileListRequest); err != nil { return nil, err } providerName := provider.GetProviderKey() + + if key.BedrockKeyConfig == nil { + return nil, providerUtils.NewConfigurationError("bedrock key config is not provided", providerName) + } + + config := key.BedrockKeyConfig @@ - region := DefaultBedrockRegion - if key.BedrockKeyConfig.Region != nil { - region = *key.BedrockKeyConfig.Region - } + region := DefaultBedrockRegion + if config.Region != nil { + region = *config.Region + } @@ - if err := signAWSRequest(ctx, httpReq, key.BedrockKeyConfig.AccessKey, key.BedrockKeyConfig.SecretKey, key.BedrockKeyConfig.SessionToken, region, "s3", providerName); err != nil { + if err := signAWSRequest(ctx, httpReq, config.AccessKey, config.SecretKey, config.SessionToken, region, "s3", providerName); err != nil { return nil, err }This keeps the new single-key flow while removing the panic risk and matching the pattern used elsewhere in the Bedrock provider.
tests/integrations/tests/test_bedrock.py (1)
1492-1501: Fix unnecessary f-string in log message (Ruff F541)
print(f"stopping the job")is an f-string with no placeholders and is flagged by Ruff F541. Use a plain string literal instead.- print(f"stopping the job") + print("stopping the job")docs/docs.json (1)
103-107: Add GenAI SDK files-and-batch page to navigationThe GenAI SDK group still only lists
integrations/genai-sdk/overview, so the existingintegrations/genai-sdk/files-and-batchpage is not reachable via the sidebar.Update the pages array to include it:
- { - "group": "GenAI SDK", - "icon": "g", - "pages": [ - "integrations/genai-sdk/overview" - ] - }, + { + "group": "GenAI SDK", + "icon": "g", + "pages": [ + "integrations/genai-sdk/overview", + "integrations/genai-sdk/files-and-batch" + ] + },docs/integrations/bedrock-sdk/files-and-batch.mdx (1)
1160-1162: Fix broken “Configuration” link targetThe
Configurationlink still points to../../quickstart/gateway/configuration, which doesn’t exist. It should target the provider configuration page.-- **[Configuration](../../quickstart/gateway/configuration)** - Bifrost setup and configuration +- **[Configuration](../../quickstart/gateway/provider-configuration)** - Bifrost setup and configuration
🧹 Nitpick comments (8)
core/schemas/chatcompletions.go (1)
21-24: Getter is fine; consider nil‑safety (and optional immutability) for robustnessThis getter is straightforward and matches the RawRequestBody field usage pattern elsewhere in the stack, so behavior looks good. As a small hardening:
- For an exported method on a pointer receiver, a cheap nil‑guard avoids panics if a nil
*BifrostChatRequestever leaks to callers.- The method exposes the underlying slice, so callers can mutate
RawRequestBody. If you ever want to guarantee immutability from the outside, you could instead return a copy.Concrete tweak for nil‑safety:
-// GetRawRequestBody returns the raw request body -func (r *BifrostChatRequest) GetRawRequestBody() []byte { - return r.RawRequestBody -} +// GetRawRequestBody returns the raw request body. +func (r *BifrostChatRequest) GetRawRequestBody() []byte { + if r == nil { + return nil + } + return r.RawRequestBody +}transports/bifrost-http/integrations/router.go (1)
187-191: Doc comment for ResponsesStreamResponseConverter is stale vs. its signatureThe comment still describes returning “the list of event types and the list of streaming formats”, but the type now returns a single
stringandinterface{}. This can confuse implementers of new integrations.Consider tightening the comment to describe the actual behavior:
-// It takes a BifrostResponsesStreamResponse and returns the list of event types and the list of streaming formats expected by the specific integration. (so that one bifrost response chunk can be converted to multiple streaming chunks by the specific integration) -// The list of event types and streaming formats are returned in the same order as the bifrost response chunks. +// It takes a BifrostResponsesStreamResponse and returns the event type and the streaming payload +// expected by the specific integration for that chunk. +// If an implementation needs to emit multiple SSE events for a single Bifrost chunk, it can encode +// them into the returned payload (for example as a pre‑formatted SSE string).plugins/mocker/changelog.md (1)
1-1: Consider adding feature context for consistency.While the version bump is clear, the
plugins/otel/changelog.mdincludes a feature description line. For consistency across plugin changelogs, consider adding the same feature context here if the mocker plugin is affected by the batch and file request logging changes:+- feat: add batch and file request logging support; refactor centralized request handling - chore: upgrades core to 1.2.38 and framework to 1.2.48If the mocker plugin is unaffected by these features and this is purely a dependency update, the current entry is acceptable.
core/utils.go (1)
50-53: Consider splitting the long boolean expression for readability.The
IsModelRequiredfunction logic is correct, but line 52 is quite long with 11 boolean conditions. For improved maintainability, consider using a switch statement or map lookup:// IsModelRequired returns true if the request type requires a model func IsModelRequired(reqType schemas.RequestType) bool { - return reqType == schemas.TextCompletionRequest || reqType == schemas.TextCompletionStreamRequest || reqType == schemas.ChatCompletionRequest || reqType == schemas.ChatCompletionStreamRequest || reqType == schemas.ResponsesRequest || reqType == schemas.ResponsesStreamRequest || reqType == schemas.SpeechRequest || reqType == schemas.SpeechStreamRequest || reqType == schemas.TranscriptionRequest || reqType == schemas.TranscriptionStreamRequest || reqType == schemas.EmbeddingRequest + switch reqType { + case schemas.TextCompletionRequest, + schemas.TextCompletionStreamRequest, + schemas.ChatCompletionRequest, + schemas.ChatCompletionStreamRequest, + schemas.ResponsesRequest, + schemas.ResponsesStreamRequest, + schemas.SpeechRequest, + schemas.SpeechStreamRequest, + schemas.TranscriptionRequest, + schemas.TranscriptionStreamRequest, + schemas.EmbeddingRequest: + return true + default: + return false + } }core/bifrost.go (2)
2823-2922: Centralized provider dispatch now covers file and batch operationsThe extended
handleProviderRequestswitch correctly forwards all new request types (File*, Batch*) to the corresponding provider methods and stores the result in the matching*Responsefield. This keeps the dispatch logic in one place and ensures retries/plugins apply uniformly.If you later add more request types, consider grouping related cases (e.g., comments or helper functions per “family”) to keep this switch maintainable as the surface grows.
3121-3139: Resetting new File/Batch fields inresetBifrostRequestavoids pool contamination**Clearing
ListModelsRequestand all File*/Batch* request pointers before returning aBifrostRequestto the pool ensures no stale sub-requests leak between calls, which is especially important now that more public APIs share the same pooled struct.If
BifrostRequestgains more fields over time, consider a small helper on the struct itself (e.g.,Reset()) to keep the reset logic co-located with its definition inschemasand reduce drift.core/providers/bedrock/batch.go (1)
335-338: RevisitModel: "dummy-model"placeholder for batch listUsing a hard-coded
"dummy-model"avoids validation errors but will still flow into places that treatmodelas meaningful (e.g., request metadata/telemetry). That can make traces and logs harder to interpret.Consider instead:
- Allowing
Modelto be empty for list operations in validation, or- Supplying a more descriptive synthetic value (e.g.
"bedrock-batch-list"), or- Threading through a real hint if one exists upstream.
core/schemas/files.go (1)
148-152: Optional: Consider consistent field ordering.
FileIDappears afterRawRequestBodyinBifrostFileRetrieveRequestbut before it inBifrostFileDeleteRequestandBifrostFileContentRequest. While this doesn't affect functionality, aligning the order (e.g., consistently placingFileIDbeforeRawRequestBody) would improve code consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
plugins/governance/go.sumis excluded by!**/*.sumplugins/logging/go.sumis excluded by!**/*.sumplugins/maxim/go.sumis excluded by!**/*.sumplugins/otel/go.sumis excluded by!**/*.sumplugins/semanticcache/go.sumis excluded by!**/*.sumplugins/telemetry/go.sumis excluded by!**/*.sumtransports/go.sumis excluded by!**/*.sum
📒 Files selected for processing (86)
core/bifrost.go(14 hunks)core/changelog.md(1 hunks)core/providers/anthropic/anthropic.go(3 hunks)core/providers/anthropic/batch.go(1 hunks)core/providers/azure/azure.go(3 hunks)core/providers/bedrock/batch.go(1 hunks)core/providers/bedrock/bedrock.go(5 hunks)core/providers/cerebras/cerebras.go(2 hunks)core/providers/cohere/cohere.go(2 hunks)core/providers/elevenlabs/elevenlabs.go(2 hunks)core/providers/gemini/gemini.go(3 hunks)core/providers/groq/groq.go(2 hunks)core/providers/mistral/batch.go(0 hunks)core/providers/mistral/files.go(0 hunks)core/providers/mistral/mistral.go(1 hunks)core/providers/nebius/nebius.go(8 hunks)core/providers/ollama/ollama.go(2 hunks)core/providers/openai/openai.go(5 hunks)core/providers/openrouter/openrouter.go(2 hunks)core/providers/parasail/batch.go(0 hunks)core/providers/parasail/files.go(0 hunks)core/providers/parasail/parasail.go(1 hunks)core/providers/perplexity/perplexity.go(2 hunks)core/providers/sgl/batch.go(0 hunks)core/providers/sgl/files.go(0 hunks)core/providers/sgl/sgl.go(1 hunks)core/providers/utils/utils.go(1 hunks)core/providers/vertex/vertex.go(2 hunks)core/schemas/batch.go(6 hunks)core/schemas/bifrost.go(4 hunks)core/schemas/chatcompletions.go(1 hunks)core/schemas/files.go(7 hunks)core/schemas/provider.go(2 hunks)core/utils.go(2 hunks)core/version(1 hunks)docs/docs.json(2 hunks)docs/integrations/anthropic-sdk/files-and-batch.mdx(1 hunks)docs/integrations/anthropic-sdk/overview.mdx(3 hunks)docs/integrations/bedrock-sdk/files-and-batch.mdx(1 hunks)docs/integrations/bedrock-sdk/overview.mdx(3 hunks)docs/integrations/genai-sdk/files-and-batch.mdx(1 hunks)docs/integrations/genai-sdk/overview.mdx(3 hunks)docs/integrations/openai-sdk/files-and-batch.mdx(1 hunks)docs/integrations/openai-sdk/overview.mdx(3 hunks)framework/changelog.md(1 hunks)framework/configstore/migrations.go(2 hunks)framework/configstore/tables/modelpricing.go(1 hunks)framework/version(1 hunks)plugins/governance/changelog.md(1 hunks)plugins/governance/go.mod(1 hunks)plugins/governance/version(1 hunks)plugins/jsonparser/changelog.md(1 hunks)plugins/jsonparser/version(1 hunks)plugins/logging/changelog.md(1 hunks)plugins/logging/go.mod(1 hunks)plugins/logging/main.go(1 hunks)plugins/logging/version(1 hunks)plugins/maxim/changelog.md(1 hunks)plugins/maxim/go.mod(1 hunks)plugins/maxim/version(1 hunks)plugins/mocker/changelog.md(1 hunks)plugins/mocker/version(1 hunks)plugins/otel/changelog.md(1 hunks)plugins/otel/converter.go(3 hunks)plugins/otel/go.mod(1 hunks)plugins/otel/version(1 hunks)plugins/semanticcache/changelog.md(1 hunks)plugins/semanticcache/go.mod(1 hunks)plugins/semanticcache/version(1 hunks)plugins/telemetry/changelog.md(1 hunks)plugins/telemetry/go.mod(1 hunks)plugins/telemetry/version(1 hunks)tests/integrations/config.json(4 hunks)tests/integrations/tests/test_anthropic.py(1 hunks)tests/integrations/tests/test_bedrock.py(2 hunks)transports/bifrost-http/handlers/inference.go(0 hunks)transports/bifrost-http/handlers/middlewares.go(1 hunks)transports/bifrost-http/integrations/anthropic.go(6 hunks)transports/bifrost-http/integrations/bedrock.go(4 hunks)transports/bifrost-http/integrations/bedrock_test.go(1 hunks)transports/bifrost-http/integrations/openai.go(4 hunks)transports/bifrost-http/integrations/router.go(4 hunks)transports/changelog.md(1 hunks)transports/go.mod(1 hunks)transports/version(1 hunks)ui/lib/constants/logs.ts(3 hunks)
💤 Files with no reviewable changes (7)
- core/providers/mistral/batch.go
- core/providers/mistral/files.go
- core/providers/parasail/batch.go
- transports/bifrost-http/handlers/inference.go
- core/providers/parasail/files.go
- core/providers/sgl/batch.go
- core/providers/sgl/files.go
✅ Files skipped from review due to trivial changes (6)
- plugins/maxim/version
- plugins/logging/version
- transports/changelog.md
- plugins/semanticcache/version
- framework/changelog.md
- plugins/governance/version
🚧 Files skipped from review as they are similar to previous changes (33)
- transports/version
- plugins/telemetry/go.mod
- plugins/logging/main.go
- core/providers/elevenlabs/elevenlabs.go
- docs/integrations/anthropic-sdk/overview.mdx
- transports/go.mod
- docs/integrations/openai-sdk/overview.mdx
- framework/version
- tests/integrations/tests/test_anthropic.py
- plugins/telemetry/changelog.md
- core/providers/utils/utils.go
- plugins/jsonparser/changelog.md
- core/providers/vertex/vertex.go
- plugins/logging/changelog.md
- plugins/telemetry/version
- transports/bifrost-http/integrations/openai.go
- plugins/mocker/version
- core/providers/cohere/cohere.go
- tests/integrations/config.json
- plugins/jsonparser/version
- docs/integrations/bedrock-sdk/overview.mdx
- plugins/otel/version
- core/providers/anthropic/batch.go
- transports/bifrost-http/integrations/bedrock_test.go
- plugins/semanticcache/changelog.md
- transports/bifrost-http/integrations/anthropic.go
- core/schemas/batch.go
- docs/integrations/genai-sdk/overview.mdx
- plugins/maxim/changelog.md
- ui/lib/constants/logs.ts
- core/providers/cerebras/cerebras.go
- docs/integrations/openai-sdk/files-and-batch.mdx
- plugins/governance/changelog.md
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
plugins/maxim/go.modplugins/semanticcache/go.modcore/providers/groq/groq.gocore/providers/ollama/ollama.gotransports/bifrost-http/handlers/middlewares.goplugins/otel/go.moddocs/docs.jsoncore/schemas/chatcompletions.godocs/integrations/bedrock-sdk/files-and-batch.mdxplugins/otel/converter.gocore/schemas/provider.gotransports/bifrost-http/integrations/bedrock.gotransports/bifrost-http/integrations/router.goframework/configstore/migrations.goplugins/logging/go.modcore/providers/bedrock/bedrock.gocore/providers/openai/openai.gocore/providers/azure/azure.goplugins/otel/changelog.mdcore/providers/bedrock/batch.gocore/versionframework/configstore/tables/modelpricing.gocore/providers/anthropic/anthropic.godocs/integrations/genai-sdk/files-and-batch.mdxtests/integrations/tests/test_bedrock.pycore/bifrost.gocore/utils.gocore/providers/openrouter/openrouter.gocore/schemas/files.goplugins/governance/go.modcore/changelog.mdcore/providers/gemini/gemini.gocore/providers/nebius/nebius.gocore/providers/sgl/sgl.gocore/schemas/bifrost.gocore/providers/perplexity/perplexity.goplugins/mocker/changelog.mdcore/providers/parasail/parasail.godocs/integrations/anthropic-sdk/files-and-batch.mdxcore/providers/mistral/mistral.go
🧠 Learnings (5)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
core/providers/groq/groq.gocore/providers/ollama/ollama.gotransports/bifrost-http/handlers/middlewares.gocore/schemas/chatcompletions.goplugins/otel/converter.gocore/schemas/provider.gotransports/bifrost-http/integrations/bedrock.gotransports/bifrost-http/integrations/router.goframework/configstore/migrations.gocore/providers/bedrock/bedrock.gocore/providers/openai/openai.gocore/providers/azure/azure.gocore/providers/bedrock/batch.goframework/configstore/tables/modelpricing.gocore/providers/anthropic/anthropic.gocore/bifrost.gocore/utils.gocore/providers/openrouter/openrouter.gocore/schemas/files.gocore/providers/gemini/gemini.gocore/providers/nebius/nebius.gocore/providers/sgl/sgl.gocore/schemas/bifrost.gocore/providers/perplexity/perplexity.gocore/providers/parasail/parasail.gocore/providers/mistral/mistral.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/handlers/middlewares.gotransports/bifrost-http/integrations/bedrock.gotransports/bifrost-http/integrations/router.go
📚 Learning: 2025-12-13T13:03:38.364Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 955
File: transports/bifrost-http/integrations/bedrock.go:682-697
Timestamp: 2025-12-13T13:03:38.364Z
Learning: In transports/bifrost-http/integrations/bedrock.go, implement FileRetrieveRequest.FileID and FileDeleteRequest.FileID to use the ETag from the If-Match header as the provider-specific file ID (e.g., OpenAI file-xxx, Gemini files/yyy), rather than deriving the ID from the S3 URI. The StorageConfig should supply the S3 bucket/prefix context, enabling cross-provider routing while keeping S3 as the backing store. This change should be accompanied by unit tests that verify the FileID source is the If-Match header, update any relevant request/response models, and add in-code comments to clarify the separation of provider file IDs from S3 paths.
Applied to files:
transports/bifrost-http/integrations/bedrock.go
📚 Learning: 2025-12-11T11:58:25.307Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: core/providers/openai/responses.go:42-84
Timestamp: 2025-12-11T11:58:25.307Z
Learning: In core/providers/openai/responses.go (and related OpenAI response handling), document and enforce the API format constraint: if ResponsesReasoning != nil and the response contains content blocks, all content blocks should be treated as reasoning blocks by default. Implement type guards or parsing logic accordingly, and add unit tests to verify that when ResponsesReasoning is non-nil, content blocks are labeled as reasoning blocks. Include clear comments in the code explaining the rationale and ensure downstream consumers rely on this behavior.
Applied to files:
core/providers/openai/openai.go
📚 Learning: 2025-12-14T14:43:30.902Z
Learnt from: Radheshg04
Repo: maximhq/bifrost PR: 980
File: core/providers/openai/images.go:10-22
Timestamp: 2025-12-14T14:43:30.902Z
Learning: Enforce the OpenAI image generation SSE event type values across the OpenAI image flow in the repository: use "image_generation.partial_image" for partial chunks, "image_generation.completed" for the final result, and "error" for errors. Apply this consistently in schemas, constants, tests, accumulator routing, and UI code within core/providers/openai (and related Go files) to ensure uniform event typing and avoid mismatches.
Applied to files:
core/providers/openai/openai.go
🧬 Code graph analysis (20)
core/providers/groq/groq.go (4)
core/schemas/batch.go (2)
BifrostBatchListRequest(118-133)BifrostBatchListResponse(136-147)core/schemas/bifrost.go (1)
BifrostError(432-441)ui/lib/types/logs.ts (1)
BifrostError(226-232)core/schemas/files.go (2)
BifrostFileListRequest(109-128)BifrostFileListResponse(136-143)
core/providers/ollama/ollama.go (3)
core/schemas/batch.go (2)
BifrostBatchListRequest(118-133)BifrostBatchListResponse(136-147)core/schemas/bifrost.go (1)
BifrostError(432-441)core/schemas/files.go (2)
BifrostFileListRequest(109-128)BifrostFileListResponse(136-143)
plugins/otel/converter.go (2)
core/schemas/files.go (5)
BifrostFileUploadRequest(53-67)BifrostFileListRequest(109-128)BifrostFileRetrieveRequest(146-159)BifrostFileDeleteRequest(186-198)BifrostFileContentRequest(215-227)core/schemas/batch.go (5)
BifrostBatchCreateRequest(65-83)BifrostBatchListRequest(118-133)BifrostBatchRetrieveRequest(150-159)BifrostBatchCancelRequest(205-214)BifrostBatchResultsRequest(234-246)
core/schemas/provider.go (3)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (1)
BifrostBatchListRequest(118-133)core/schemas/files.go (1)
BifrostFileListRequest(109-128)
transports/bifrost-http/integrations/bedrock.go (1)
transports/bifrost-http/integrations/router.go (2)
BatchRequestConverter(101-101)BatchRequest(81-88)
framework/configstore/migrations.go (2)
framework/migrator/migrator.go (3)
New(131-149)DefaultOptions(100-106)Migration(62-69)framework/configstore/tables/modelpricing.go (2)
TableModelPricing(4-41)TableModelPricing(44-44)
core/providers/bedrock/bedrock.go (5)
core/providers/utils/utils.go (1)
CheckOperationAllowed(473-485)core/schemas/bifrost.go (2)
Bedrock(38-38)BifrostError(432-441)core/schemas/account.go (1)
Key(8-18)core/schemas/files.go (2)
BifrostFileListRequest(109-128)BifrostFileListResponse(136-143)core/schemas/batch.go (2)
BifrostBatchListRequest(118-133)BifrostBatchListResponse(136-147)
core/providers/openai/openai.go (4)
core/schemas/account.go (1)
Key(8-18)core/schemas/files.go (2)
BifrostFileListRequest(109-128)BifrostFileListResponse(136-143)core/providers/utils/utils.go (1)
SetExtraHeaders(179-209)core/schemas/batch.go (2)
BifrostBatchListRequest(118-133)BifrostBatchListResponse(136-147)
core/providers/azure/azure.go (3)
core/schemas/account.go (2)
Key(8-18)AzureKeyConfig(22-26)core/schemas/files.go (1)
BifrostFileListRequest(109-128)core/schemas/batch.go (2)
BifrostBatchListRequest(118-133)BifrostBatchListResponse(136-147)
core/providers/bedrock/batch.go (1)
core/schemas/models.go (1)
Model(109-129)
core/providers/anthropic/anthropic.go (4)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(118-133)BifrostBatchListResponse(136-147)core/schemas/files.go (2)
BifrostFileListRequest(109-128)BifrostFileListResponse(136-143)core/providers/utils/utils.go (1)
CheckOperationAllowed(473-485)
core/bifrost.go (5)
core/schemas/bifrost.go (18)
RequestType(85-85)BatchCreateRequest(100-100)BatchListRequest(101-101)BatchRetrieveRequest(102-102)BatchCancelRequest(103-103)BatchResultsRequest(104-104)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)FileContentRequest(109-109)BifrostRequest(160-180)ListModelsRequest(88-88)TextCompletionRequest(89-89)ResponsesRequest(93-93)EmbeddingRequest(95-95)SpeechRequest(96-96)TranscriptionRequest(98-98)ui/lib/types/config.ts (1)
RequestType(124-146)transports/bifrost-http/handlers/inference.go (7)
BatchCreateRequest(292-299)BatchListRequest(302-307)ChatRequest(172-176)ResponsesRequest(266-270)EmbeddingRequest(273-277)SpeechRequest(279-283)TranscriptionRequest(285-289)core/utils.go (1)
Ptr(56-58)core/schemas/utils.go (1)
Ptr(16-18)
core/utils.go (2)
core/schemas/bifrost.go (12)
RequestType(85-85)TextCompletionRequest(89-89)TextCompletionStreamRequest(90-90)ChatCompletionRequest(91-91)ChatCompletionStreamRequest(92-92)ResponsesRequest(93-93)ResponsesStreamRequest(94-94)SpeechRequest(96-96)SpeechStreamRequest(97-97)TranscriptionRequest(98-98)TranscriptionStreamRequest(99-99)EmbeddingRequest(95-95)ui/lib/types/config.ts (1)
RequestType(124-146)
core/providers/openrouter/openrouter.go (3)
core/schemas/batch.go (2)
BifrostBatchListRequest(118-133)BifrostBatchListResponse(136-147)core/schemas/bifrost.go (5)
BifrostError(432-441)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)core/providers/utils/utils.go (1)
NewUnsupportedOperationError(455-467)
core/schemas/files.go (1)
core/schemas/models.go (1)
Model(109-129)
core/providers/gemini/gemini.go (4)
core/schemas/batch.go (2)
BifrostBatchListRequest(118-133)BifrostBatchListResponse(136-147)core/schemas/bifrost.go (3)
BifrostError(432-441)Gemini(48-48)FileListRequest(106-106)core/schemas/files.go (2)
BifrostFileListRequest(109-128)BifrostFileListResponse(136-143)core/providers/utils/utils.go (1)
CheckOperationAllowed(473-485)
core/providers/nebius/nebius.go (2)
core/providers/utils/utils.go (2)
ShouldSendBackRawRequest(607-612)NewUnsupportedOperationError(455-467)core/schemas/bifrost.go (11)
BifrostError(432-441)BatchCreateRequest(100-100)BatchListRequest(101-101)BatchRetrieveRequest(102-102)BatchCancelRequest(103-103)BatchResultsRequest(104-104)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)FileContentRequest(109-109)
core/providers/sgl/sgl.go (3)
core/schemas/files.go (1)
BifrostFileUploadRequest(53-67)core/schemas/bifrost.go (11)
BifrostError(432-441)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)FileContentRequest(109-109)BatchCreateRequest(100-100)BatchListRequest(101-101)BatchRetrieveRequest(102-102)BatchCancelRequest(103-103)BatchResultsRequest(104-104)core/providers/utils/utils.go (1)
NewUnsupportedOperationError(455-467)
core/providers/perplexity/perplexity.go (4)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(118-133)BifrostBatchListResponse(136-147)core/schemas/bifrost.go (1)
BifrostError(432-441)core/schemas/files.go (2)
BifrostFileListRequest(109-128)BifrostFileListResponse(136-143)
core/providers/parasail/parasail.go (4)
core/schemas/files.go (1)
BifrostFileUploadRequest(53-67)core/schemas/bifrost.go (7)
BifrostError(432-441)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)FileContentRequest(109-109)BatchCreateRequest(100-100)ui/lib/types/logs.ts (1)
BifrostError(226-232)core/providers/utils/utils.go (1)
NewUnsupportedOperationError(455-467)
🪛 Ruff (0.14.8)
tests/integrations/tests/test_bedrock.py
1500-1500: f-string without any placeholders
Remove extraneous f prefix
(F541)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
tests/integrations/tests/test_bedrock.py (1)
1491-1503: Remove unnecessary f-string to satisfy Ruff F541
print(f"stopping the job")is an f-string with no placeholders and is flagged by Ruff (F541). It should be a plain string literal.- print(f"create_response: {create_response}") - - print(f"stopping the job") + print(f"create_response: {create_response}") + + print("stopping the job")This keeps the helpful debug output while eliminating the lint warning.
transports/bifrost-http/integrations/anthropic.go (3)
407-421: Missing nil check for provider type assertion.The type assertion
(*ctx).Value(bifrostContextKeyProvider).(schemas.ModelProvider)does not check theokreturn value. If the context key is missing or contains a different type, this will panic.- if retrieveReq, ok := req.(*anthropic.AnthropicBatchRetrieveRequest); ok { - provider := (*ctx).Value(bifrostContextKeyProvider).(schemas.ModelProvider) + if retrieveReq, ok := req.(*anthropic.AnthropicBatchRetrieveRequest); ok { + provider, ok := (*ctx).Value(bifrostContextKeyProvider).(schemas.ModelProvider) + if !ok { + return nil, errors.New("provider not found in context") + }
443-456: Missing nil check for provider type assertion.Same issue as the retrieve request - the type assertion should check the
okvalue to avoid potential panics.- if cancelReq, ok := req.(*anthropic.AnthropicBatchCancelRequest); ok { - provider := (*ctx).Value(bifrostContextKeyProvider).(schemas.ModelProvider) + if cancelReq, ok := req.(*anthropic.AnthropicBatchCancelRequest); ok { + provider, ok := (*ctx).Value(bifrostContextKeyProvider).(schemas.ModelProvider) + if !ok { + return nil, errors.New("provider not found in context") + }
479-493: Missing nil check for provider type assertion.Same issue as above - the type assertion on line 481 should check the
okvalue.- if resultsReq, ok := req.(*anthropic.AnthropicBatchResultsRequest); ok { - provider := (*ctx).Value(bifrostContextKeyProvider).(schemas.ModelProvider) + if resultsReq, ok := req.(*anthropic.AnthropicBatchResultsRequest); ok { + provider, ok := (*ctx).Value(bifrostContextKeyProvider).(schemas.ModelProvider) + if !ok { + return nil, errors.New("provider not found in context") + }
♻️ Duplicate comments (7)
plugins/logging/go.mod (1)
85-85: Duplicate concern: Weaviate version bump consistency across modules.This file shows the identical Weaviate v1.33.1 → v1.33.4 update as observed in
transports/go.modandplugins/telemetry/go.mod. The feedback from the transports file applies here as well.plugins/telemetry/go.mod (1)
91-91: Duplicate concern: Weaviate version bump consistency across modules.This file shows the same Weaviate v1.33.1 → v1.33.4 update as the other go.mod files in this PR. Please address the verification request made for
transports/go.modat line 109 to confirm stability and necessity across all affected modules.ui/lib/constants/logs.ts (1)
147-158: Optional: consider distinct colors for batch/file request types (duplicate of earlier feedback).Several of the new
batch_*andfile_*entries reuse colors already assigned to other request types, which can make log views less visually distinguishable when many types appear together. If clearer visual separation is desired, consider assigning unique bg/text color pairs for these new types.docs/integrations/bedrock-sdk/overview.mdx (1)
175-225: Quickstart link path likely still incorrect (points too high in tree)From
docs/integrations/bedrock-sdk/overview.mdx, the link target../../../quickstartwill resolve to aquickstartdirectory at the repo root, whereas prior structure and tooling output indicatequickstartlives underdocs/quickstart/. This will result in a broken link.Consider updating both occurrences to the docs-relative path, e.g.:
- > **Learn more:** See [Quickstart Configuration](../../../quickstart) for enabling direct API key usage. + > **Learn more:** See [Quickstart Configuration](../../quickstart/) for enabling direct API key usage. ... - - **[Configuration](../../../quickstart)** - Bedrock provider setup and API key management + - **[Configuration](../../quickstart/)** - Bedrock provider setup and API key managementPlease adjust to the actual quickstart location if it has moved.
tests/integrations/tests/test_bedrock.py (1)
1178-1183: Fix misleading skip reason when gating file content test to BedrockHere the test runs only when
provider == "bedrock", but the skip message says “Bedrock does not support file content download”, which reads as the opposite of what’s happening.Consider clarifying:
- if provider != "bedrock": - pytest.skip("Bedrock does not support file content download") + if provider != "bedrock": + pytest.skip(f"File content download test runs only for the bedrock provider (got {provider})")This matches the behavior and makes the skip reason clear in test output.
docs/docs.json (1)
103-108: GenAI SDK files-and-batch page missing from navigation.The GenAI SDK group's
pagesarray at lines 103-108 only includes "integrations/genai-sdk/overview" but is missing "integrations/genai-sdk/files-and-batch", which exists in this PR. This creates an inconsistent navigation structure compared to OpenAI, Anthropic, and Bedrock SDK groups.Apply this diff to add the missing page:
{ "group": "GenAI SDK", "icon": "g", "pages": [ - "integrations/genai-sdk/overview" + "integrations/genai-sdk/overview", + "integrations/genai-sdk/files-and-batch" ] },core/providers/bedrock/bedrock.go (1)
1422-1485: Still missing BedrockKeyConfig nil-guard in FileList (panic risk when signing S3)
FileListdereferenceskey.BedrockKeyConfigforRegionand when callingsignAWSRequestwithout first checking it for nil, unlikeFileUpload,FileRetrieve,FileDelete,FileContent,BatchList, etc. If a Bedrock key is misconfigured withoutBedrockKeyConfig, this will panic instead of returning a structured configuration error, and it also reintroduces the risk previously flagged on this method.Recommend aligning with the other helpers by explicitly requiring
BedrockKeyConfigand using a localconfigvariable:func (provider *BedrockProvider) FileList(ctx context.Context, key schemas.Key, request *schemas.BifrostFileListRequest) (*schemas.BifrostFileListResponse, *schemas.BifrostError) { if err := providerUtils.CheckOperationAllowed(schemas.Bedrock, provider.customProviderConfig, schemas.FileListRequest); err != nil { return nil, err } providerName := provider.GetProviderKey() + + if key.BedrockKeyConfig == nil { + return nil, providerUtils.NewConfigurationError("bedrock key config is not provided", providerName) + } + + config := key.BedrockKeyConfig @@ - region := DefaultBedrockRegion - if key.BedrockKeyConfig.Region != nil { - region = *key.BedrockKeyConfig.Region - } + region := DefaultBedrockRegion + if config.Region != nil { + region = *config.Region + } @@ - // Sign request for S3 - if err := signAWSRequest(ctx, httpReq, key.BedrockKeyConfig.AccessKey, key.BedrockKeyConfig.SecretKey, key.BedrockKeyConfig.SessionToken, region, "s3", providerName); err != nil { + // Sign request for S3 + if err := signAWSRequest(ctx, httpReq, config.AccessKey, config.SecretKey, config.SessionToken, region, "s3", providerName); err != nil { return nil, err }This avoids the panic and makes
FileListconsistent with the rest of the Bedrock S3 helpers and with howBatchResultscalls into it.
🧹 Nitpick comments (8)
plugins/maxim/changelog.md (1)
1-1: Enhance changelog entry to reflect actual features added.The current entry only documents version upgrades but omits the primary features being added. Consider mentioning batch and file request logging support, as indicated in the PR objectives.
Consider updating the entry to:
- chore: upgrades core to 1.2.38 and framework to 1.1.48 + feat: add batch and file request logging support + chore: upgrades core to 1.2.38 and framework to 1.1.48Alternatively, if you prefer a single-line summary:
- chore: upgrades core to 1.2.38 and framework to 1.1.48 + feat: add batch and file request logging; upgrade core to 1.2.38, framework to 1.1.48This gives users a clearer understanding of what functionality was added in this release.
ui/lib/constants/logs.ts (1)
106-119: Confirm whetherRequestTypesshould include new batch/file types.You’ve added labels for
batch_*andfile_*request types here; if any UI (filters, dropdowns, legends) is driven off theRequestTypesarray above, consider adding these keys there as well so they’re available everywhere those enums are used. If those UIs rely solely on the raw log keys and this map, then this is fine as-is.plugins/otel/converter.go (1)
370-550: Batch/file request param helpers look solid; consider loggingNextCursorand watching ExtraParams cardinalityThe new helpers for file and batch requests are consistent with existing patterns (nil/empty checks, key naming, use of
kv*helpers), so behavior looks correct.Two small optional enhancements:
BifrostBatchListRequesthas aNextCursorfield incore/schemas/batch.gobut it isn’t logged here; if you rely on it for diagnosing pagination flows (e.g., Gemini), consider adding agen_ai.batch.next_cursorattribute alongside the other cursors.MetadataandExtraParamsare stringified wholesale into single attributes; if those maps can grow large or contain highly-cardinal keys/values, you may want to eventually whitelist or prefix them to avoid OTEL cardinality / volume issues, even though this matches the existing request helpers.transports/bifrost-http/integrations/bedrock_test.go (1)
186-193: Align assertion message with renamedBatchRequestConverterfieldThe assertion now correctly checks
routes[i].BatchRequestConverter, but the message still refers toBatchCreateRequestConverter, which may confuse future readers.- assert.NotNil(t, routes[i].BatchRequestConverter, "batch route %d BatchCreateRequestConverter should not be nil", i) + assert.NotNil(t, routes[i].BatchRequestConverter, "batch route %d BatchRequestConverter should not be nil", i)core/providers/vertex/vertex.go (1)
1468-1470: Parameter namekeysis inconsistent with the singular typeschemas.Key.The parameter was renamed from a slice to a single key, but the parameter name remains plural. Consider renaming to
keyfor consistency with the type and other method signatures in this file (e.g.,BatchCreate,BatchRetrieve).-func (provider *VertexProvider) BatchList(ctx context.Context, keys schemas.Key, request *schemas.BifrostBatchListRequest) (*schemas.BifrostBatchListResponse, *schemas.BifrostError) { +func (provider *VertexProvider) BatchList(ctx context.Context, key schemas.Key, request *schemas.BifrostBatchListRequest) (*schemas.BifrostBatchListResponse, *schemas.BifrostError) {core/bifrost.go (1)
3193-3269: Key selection for model-less operations with Deployments may be overly strict
selectKeyFromProviderForModelfilters keys for Azure/Bedrock/Vertex by checkingDeployments[model]when the base provider type matches. For request types that don’t carry a model (e.g., File*/Batch* APIs wheremodelfromGetRequestFields()is typically""), this means:
- Any key with a non-empty
Deploymentsmap but no entry for""will be treated as unsupported.- Only keys with empty
Deployments(and usually emptyModels) will be eligible for these operations.If the intent is to require separate “non-model” keys for file/batch operations, this is fine. If instead you expect the same Bedrock/Azure/Vertex keys (with Deployments configured for model-based calls) to work for File*/Batch* too, you may want to relax the deployment check when
model == ""or whenrequestTypeis a non-model operation.core/providers/gemini/gemini.go (1)
1940-1943: Remove unusedretrieveReqvariable.The
retrieveReqvariable is declared but only used to accessBatchID, which is already available asrequest.BatchID. This is dead code.Apply this diff:
- // First, retrieve the batch to get its results - retrieveReq := &schemas.BifrostBatchRetrieveRequest{ - BatchID: request.BatchID, - } - // We need to get the full batch response with results, so make the API call directlyAnd update line 1995:
- fmt.Sprintf("batch %s is still processing (state: %s), results not yet available", retrieveReq.BatchID, geminiResp.Metadata.State), + fmt.Sprintf("batch %s is still processing (state: %s), results not yet available", request.BatchID, geminiResp.Metadata.State),core/schemas/bifrost.go (1)
222-236: SetProvider, SetModel, SetFallbacks, and SetRawRequestBody should be extended for consistency.
GetRequestFieldsnow handles all 16 request types including file/batch operations (lines 197-216), butSetProvider,SetModel,SetFallbacks, andSetRawRequestBodyonly cover the 6 traditional request types. File and batch request types have Provider and Model fields defined but are missing corresponding case statements. While file/batch requests currently set these fields at construction time rather than through these setters, completing all four methods would prevent future maintenance issues and ensure the setter methods work uniformly across all request types.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
plugins/governance/go.sumis excluded by!**/*.sumplugins/logging/go.sumis excluded by!**/*.sumplugins/maxim/go.sumis excluded by!**/*.sumplugins/otel/go.sumis excluded by!**/*.sumplugins/semanticcache/go.sumis excluded by!**/*.sumplugins/telemetry/go.sumis excluded by!**/*.sumtransports/go.sumis excluded by!**/*.sum
📒 Files selected for processing (86)
core/bifrost.go(14 hunks)core/changelog.md(1 hunks)core/providers/anthropic/anthropic.go(3 hunks)core/providers/anthropic/batch.go(1 hunks)core/providers/azure/azure.go(3 hunks)core/providers/bedrock/batch.go(1 hunks)core/providers/bedrock/bedrock.go(5 hunks)core/providers/cerebras/cerebras.go(2 hunks)core/providers/cohere/cohere.go(2 hunks)core/providers/elevenlabs/elevenlabs.go(2 hunks)core/providers/gemini/gemini.go(3 hunks)core/providers/groq/groq.go(2 hunks)core/providers/mistral/batch.go(0 hunks)core/providers/mistral/files.go(0 hunks)core/providers/mistral/mistral.go(1 hunks)core/providers/nebius/nebius.go(8 hunks)core/providers/ollama/ollama.go(2 hunks)core/providers/openai/openai.go(5 hunks)core/providers/openrouter/openrouter.go(2 hunks)core/providers/parasail/batch.go(0 hunks)core/providers/parasail/files.go(0 hunks)core/providers/parasail/parasail.go(1 hunks)core/providers/perplexity/perplexity.go(2 hunks)core/providers/sgl/batch.go(0 hunks)core/providers/sgl/files.go(0 hunks)core/providers/sgl/sgl.go(1 hunks)core/providers/utils/utils.go(1 hunks)core/providers/vertex/vertex.go(2 hunks)core/schemas/batch.go(6 hunks)core/schemas/bifrost.go(4 hunks)core/schemas/chatcompletions.go(1 hunks)core/schemas/files.go(7 hunks)core/schemas/provider.go(2 hunks)core/utils.go(2 hunks)core/version(1 hunks)docs/docs.json(2 hunks)docs/integrations/anthropic-sdk/files-and-batch.mdx(1 hunks)docs/integrations/anthropic-sdk/overview.mdx(3 hunks)docs/integrations/bedrock-sdk/files-and-batch.mdx(1 hunks)docs/integrations/bedrock-sdk/overview.mdx(3 hunks)docs/integrations/genai-sdk/files-and-batch.mdx(1 hunks)docs/integrations/genai-sdk/overview.mdx(3 hunks)docs/integrations/openai-sdk/files-and-batch.mdx(1 hunks)docs/integrations/openai-sdk/overview.mdx(3 hunks)framework/changelog.md(1 hunks)framework/configstore/migrations.go(2 hunks)framework/configstore/tables/modelpricing.go(1 hunks)framework/version(1 hunks)plugins/governance/changelog.md(1 hunks)plugins/governance/go.mod(1 hunks)plugins/governance/version(1 hunks)plugins/jsonparser/changelog.md(1 hunks)plugins/jsonparser/version(1 hunks)plugins/logging/changelog.md(1 hunks)plugins/logging/go.mod(1 hunks)plugins/logging/main.go(1 hunks)plugins/logging/version(1 hunks)plugins/maxim/changelog.md(1 hunks)plugins/maxim/go.mod(1 hunks)plugins/maxim/version(1 hunks)plugins/mocker/changelog.md(1 hunks)plugins/mocker/version(1 hunks)plugins/otel/changelog.md(1 hunks)plugins/otel/converter.go(3 hunks)plugins/otel/go.mod(1 hunks)plugins/otel/version(1 hunks)plugins/semanticcache/changelog.md(1 hunks)plugins/semanticcache/go.mod(1 hunks)plugins/semanticcache/version(1 hunks)plugins/telemetry/changelog.md(1 hunks)plugins/telemetry/go.mod(1 hunks)plugins/telemetry/version(1 hunks)tests/integrations/config.json(4 hunks)tests/integrations/tests/test_anthropic.py(1 hunks)tests/integrations/tests/test_bedrock.py(2 hunks)transports/bifrost-http/handlers/inference.go(0 hunks)transports/bifrost-http/handlers/middlewares.go(1 hunks)transports/bifrost-http/integrations/anthropic.go(6 hunks)transports/bifrost-http/integrations/bedrock.go(4 hunks)transports/bifrost-http/integrations/bedrock_test.go(1 hunks)transports/bifrost-http/integrations/openai.go(4 hunks)transports/bifrost-http/integrations/router.go(4 hunks)transports/changelog.md(1 hunks)transports/go.mod(1 hunks)transports/version(1 hunks)ui/lib/constants/logs.ts(3 hunks)
💤 Files with no reviewable changes (7)
- core/providers/parasail/files.go
- core/providers/mistral/batch.go
- core/providers/mistral/files.go
- core/providers/sgl/files.go
- core/providers/parasail/batch.go
- transports/bifrost-http/handlers/inference.go
- core/providers/sgl/batch.go
✅ Files skipped from review due to trivial changes (7)
- core/version
- docs/integrations/bedrock-sdk/files-and-batch.mdx
- framework/changelog.md
- plugins/logging/version
- plugins/telemetry/version
- plugins/semanticcache/changelog.md
- transports/version
🚧 Files skipped from review as they are similar to previous changes (34)
- plugins/mocker/changelog.md
- core/changelog.md
- plugins/logging/main.go
- core/providers/utils/utils.go
- plugins/logging/changelog.md
- plugins/jsonparser/changelog.md
- docs/integrations/anthropic-sdk/overview.mdx
- plugins/otel/changelog.md
- framework/configstore/migrations.go
- core/utils.go
- plugins/maxim/go.mod
- plugins/otel/go.mod
- plugins/governance/go.mod
- plugins/mocker/version
- docs/integrations/genai-sdk/overview.mdx
- transports/bifrost-http/integrations/bedrock.go
- tests/integrations/config.json
- core/providers/azure/azure.go
- framework/configstore/tables/modelpricing.go
- plugins/maxim/version
- plugins/governance/version
- core/providers/groq/groq.go
- plugins/jsonparser/version
- plugins/semanticcache/go.mod
- core/providers/perplexity/perplexity.go
- framework/version
- plugins/governance/changelog.md
- tests/integrations/tests/test_anthropic.py
- plugins/otel/version
- core/schemas/provider.go
- core/providers/sgl/sgl.go
- plugins/semanticcache/version
- transports/bifrost-http/handlers/middlewares.go
- core/providers/mistral/mistral.go
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
plugins/maxim/changelog.mdtransports/go.modplugins/logging/go.modcore/providers/openai/openai.gocore/bifrost.godocs/docs.jsondocs/integrations/openai-sdk/overview.mdxcore/providers/ollama/ollama.gocore/providers/bedrock/batch.gotransports/changelog.mdcore/providers/anthropic/batch.godocs/integrations/genai-sdk/files-and-batch.mdxcore/providers/cerebras/cerebras.gocore/providers/vertex/vertex.godocs/integrations/anthropic-sdk/files-and-batch.mdxtransports/bifrost-http/integrations/bedrock_test.gocore/providers/elevenlabs/elevenlabs.goui/lib/constants/logs.tscore/providers/cohere/cohere.goplugins/otel/converter.gocore/schemas/batch.godocs/integrations/openai-sdk/files-and-batch.mdxcore/providers/parasail/parasail.gocore/providers/bedrock/bedrock.gocore/providers/openrouter/openrouter.gocore/schemas/files.goplugins/telemetry/go.modplugins/telemetry/changelog.mdcore/providers/gemini/gemini.gotransports/bifrost-http/integrations/router.gotransports/bifrost-http/integrations/anthropic.gocore/schemas/chatcompletions.gotransports/bifrost-http/integrations/openai.gocore/schemas/bifrost.gocore/providers/anthropic/anthropic.godocs/integrations/bedrock-sdk/overview.mdxcore/providers/nebius/nebius.gotests/integrations/tests/test_bedrock.py
🧠 Learnings (4)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
core/providers/openai/openai.gocore/bifrost.gocore/providers/ollama/ollama.gocore/providers/bedrock/batch.gocore/providers/anthropic/batch.gocore/providers/cerebras/cerebras.gocore/providers/vertex/vertex.gotransports/bifrost-http/integrations/bedrock_test.gocore/providers/elevenlabs/elevenlabs.gocore/providers/cohere/cohere.goplugins/otel/converter.gocore/schemas/batch.gocore/providers/parasail/parasail.gocore/providers/bedrock/bedrock.gocore/providers/openrouter/openrouter.gocore/schemas/files.gocore/providers/gemini/gemini.gotransports/bifrost-http/integrations/router.gotransports/bifrost-http/integrations/anthropic.gocore/schemas/chatcompletions.gotransports/bifrost-http/integrations/openai.gocore/schemas/bifrost.gocore/providers/anthropic/anthropic.gocore/providers/nebius/nebius.go
📚 Learning: 2025-12-11T11:58:25.307Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: core/providers/openai/responses.go:42-84
Timestamp: 2025-12-11T11:58:25.307Z
Learning: In core/providers/openai/responses.go (and related OpenAI response handling), document and enforce the API format constraint: if ResponsesReasoning != nil and the response contains content blocks, all content blocks should be treated as reasoning blocks by default. Implement type guards or parsing logic accordingly, and add unit tests to verify that when ResponsesReasoning is non-nil, content blocks are labeled as reasoning blocks. Include clear comments in the code explaining the rationale and ensure downstream consumers rely on this behavior.
Applied to files:
core/providers/openai/openai.go
📚 Learning: 2025-12-14T14:43:30.902Z
Learnt from: Radheshg04
Repo: maximhq/bifrost PR: 980
File: core/providers/openai/images.go:10-22
Timestamp: 2025-12-14T14:43:30.902Z
Learning: Enforce the OpenAI image generation SSE event type values across the OpenAI image flow in the repository: use "image_generation.partial_image" for partial chunks, "image_generation.completed" for the final result, and "error" for errors. Apply this consistently in schemas, constants, tests, accumulator routing, and UI code within core/providers/openai (and related Go files) to ensure uniform event typing and avoid mismatches.
Applied to files:
core/providers/openai/openai.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/integrations/bedrock_test.gotransports/bifrost-http/integrations/router.gotransports/bifrost-http/integrations/anthropic.gotransports/bifrost-http/integrations/openai.go
🧬 Code graph analysis (19)
core/providers/openai/openai.go (4)
core/schemas/account.go (1)
Key(8-18)core/schemas/files.go (2)
BifrostFileListRequest(109-128)BifrostFileListResponse(136-143)core/providers/utils/utils.go (1)
SetExtraHeaders(179-209)core/schemas/batch.go (2)
BifrostBatchListRequest(118-133)BifrostBatchListResponse(136-147)
core/providers/ollama/ollama.go (3)
core/schemas/batch.go (2)
BifrostBatchListRequest(118-133)BifrostBatchListResponse(136-147)core/schemas/bifrost.go (1)
BifrostError(432-441)core/schemas/files.go (2)
BifrostFileListRequest(109-128)BifrostFileListResponse(136-143)
core/providers/bedrock/batch.go (1)
core/schemas/models.go (1)
Model(109-129)
core/providers/cerebras/cerebras.go (4)
core/schemas/files.go (2)
BifrostFileListRequest(109-128)BifrostFileListResponse(136-143)core/schemas/bifrost.go (1)
BifrostError(432-441)ui/lib/types/logs.ts (1)
BifrostError(226-232)core/schemas/batch.go (2)
BifrostBatchListRequest(118-133)BifrostBatchListResponse(136-147)
core/providers/vertex/vertex.go (3)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(118-133)BifrostBatchListResponse(136-147)core/schemas/files.go (2)
BifrostFileListRequest(109-128)BifrostFileListResponse(136-143)
transports/bifrost-http/integrations/bedrock_test.go (1)
transports/bifrost-http/integrations/router.go (1)
BatchRequestConverter(101-101)
core/providers/elevenlabs/elevenlabs.go (2)
core/schemas/account.go (1)
Key(8-18)core/schemas/files.go (2)
BifrostFileListRequest(109-128)BifrostFileListResponse(136-143)
core/providers/cohere/cohere.go (3)
core/schemas/batch.go (1)
BifrostBatchListRequest(118-133)core/schemas/bifrost.go (1)
BifrostError(432-441)core/schemas/files.go (1)
BifrostFileListRequest(109-128)
plugins/otel/converter.go (3)
core/schemas/files.go (5)
BifrostFileUploadRequest(53-67)BifrostFileListRequest(109-128)BifrostFileRetrieveRequest(146-159)BifrostFileDeleteRequest(186-198)BifrostFileContentRequest(215-227)core/schemas/batch.go (5)
BifrostBatchCreateRequest(65-83)BifrostBatchListRequest(118-133)BifrostBatchRetrieveRequest(150-159)BifrostBatchCancelRequest(205-214)BifrostBatchResultsRequest(234-246)transports/bifrost-http/handlers/inference.go (2)
BatchCreateRequest(292-299)BatchListRequest(302-307)
core/schemas/batch.go (1)
core/schemas/models.go (1)
Model(109-129)
core/providers/parasail/parasail.go (4)
core/schemas/bifrost.go (11)
BifrostError(432-441)FileUploadRequest(105-105)FileListRequest(106-106)FileRetrieveRequest(107-107)FileDeleteRequest(108-108)FileContentRequest(109-109)BatchCreateRequest(100-100)BatchListRequest(101-101)BatchRetrieveRequest(102-102)BatchCancelRequest(103-103)BatchResultsRequest(104-104)ui/lib/types/logs.ts (1)
BifrostError(226-232)core/providers/utils/utils.go (1)
NewUnsupportedOperationError(455-467)transports/bifrost-http/handlers/inference.go (2)
BatchCreateRequest(292-299)BatchListRequest(302-307)
core/providers/bedrock/bedrock.go (5)
core/providers/utils/utils.go (1)
CheckOperationAllowed(473-485)core/schemas/bifrost.go (2)
Bedrock(38-38)BifrostError(432-441)core/schemas/account.go (1)
Key(8-18)core/schemas/files.go (2)
BifrostFileListRequest(109-128)BifrostFileListResponse(136-143)core/schemas/batch.go (2)
BifrostBatchListRequest(118-133)BifrostBatchListResponse(136-147)
core/schemas/files.go (1)
core/schemas/models.go (1)
Model(109-129)
core/providers/gemini/gemini.go (3)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(118-133)BifrostBatchListResponse(136-147)core/schemas/files.go (2)
BifrostFileListRequest(109-128)BifrostFileListResponse(136-143)
transports/bifrost-http/integrations/anthropic.go (3)
transports/bifrost-http/integrations/router.go (2)
BatchRequestConverter(101-101)BatchRequest(81-88)core/providers/anthropic/batch.go (1)
AnthropicBatchCreateRequest(20-22)core/schemas/bifrost.go (2)
ModelProvider(32-32)Anthropic(37-37)
transports/bifrost-http/integrations/openai.go (1)
transports/bifrost-http/integrations/router.go (2)
BatchRequestConverter(101-101)BatchRequest(81-88)
core/schemas/bifrost.go (2)
core/schemas/files.go (10)
BifrostFileUploadRequest(53-67)BifrostFileListRequest(109-128)BifrostFileRetrieveRequest(146-159)BifrostFileDeleteRequest(186-198)BifrostFileContentRequest(215-227)BifrostFileUploadResponse(90-106)BifrostFileListResponse(136-143)BifrostFileRetrieveResponse(167-183)BifrostFileDeleteResponse(206-212)BifrostFileContentResponse(235-241)core/schemas/batch.go (10)
BifrostBatchCreateRequest(65-83)BifrostBatchListRequest(118-133)BifrostBatchRetrieveRequest(150-159)BifrostBatchCancelRequest(205-214)BifrostBatchResultsRequest(234-246)BifrostBatchCreateResponse(91-115)BifrostBatchListResponse(136-147)BifrostBatchRetrieveResponse(167-202)BifrostBatchCancelResponse(222-231)BifrostBatchResultsResponse(285-294)
core/providers/anthropic/anthropic.go (5)
core/schemas/account.go (1)
Key(8-18)core/schemas/batch.go (2)
BifrostBatchListRequest(118-133)BifrostBatchListResponse(136-147)core/schemas/bifrost.go (3)
BifrostError(432-441)Anthropic(37-37)FileListRequest(106-106)core/schemas/files.go (2)
BifrostFileListRequest(109-128)BifrostFileListResponse(136-143)core/providers/utils/utils.go (1)
CheckOperationAllowed(473-485)
core/providers/nebius/nebius.go (3)
core/providers/utils/utils.go (2)
ShouldSendBackRawRequest(607-612)NewUnsupportedOperationError(455-467)core/schemas/batch.go (1)
BifrostBatchCreateRequest(65-83)core/schemas/bifrost.go (2)
BifrostError(432-441)BatchCreateRequest(100-100)
🪛 Ruff (0.14.8)
tests/integrations/tests/test_bedrock.py
1500-1500: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (62)
plugins/telemetry/changelog.md (1)
1-2: Changelog entries properly address previous feedback.The additions now clearly document both the feature (batch and file request logging) and dependency upgrades, following Conventional Commits conventions. The feature-focused entry resolves the earlier flag about omitted functionality.
transports/changelog.md (1)
1-6: Changelog entries are well-formatted and align with PR objectives.The entries correctly document the new batch API support, provider additions, and fixes. Line 6 now reads correctly (the extra closing bracket from the past review appears to have been resolved).
One minor note: line 2 uses a JSX
<Badge>component in the changelog—verify this is intentional if the changelog is processed as markdown or as rendered documentation. If the changelog is served as plain markdown, consider removing the JSX and using standard markdown formatting instead (e.g.,(Beta)in plain text or a markdown table).transports/go.mod (1)
109-109: The review comment appears to analyze a change that is not evident in the repository state. There are no modifiedgo.modfiles detected in this PR, andtransports/go.modshows v1.33.4 as the current state without evidence of modification. Additionally, v1.33.4 has no known breaking changes. If a Weaviate version update was made, provide the actual diff; otherwise, this comment should be removed.Likely an incorrect or invalid review comment.
ui/lib/constants/logs.ts (1)
60-60: Nebius provider label addition looks good.The new
nebiuslabel is consistent with the other provider labels and matches the expected naming style.plugins/otel/converter.go (2)
581-610: createResourceSpan wiring for batch/file types is correct and consistentThe new
RequestTypecases set clear span names (gen_ai.batch.*,gen_ai.file.*) and delegate to the dedicated helpers, matching how text/chat/etc. are handled. AssumingBifrostRequestguarantees the corresponding sub-structs are non-nil for eachRequestType, this is safe and should produce well-structured spans.
875-1040: Batch/file response telemetry is comprehensive and follows existing conventionsThe added response branches capture the key batch/file fields (IDs, status, endpoints, timestamps, counts, bytes, storage backend, cursors) and use the same
gen_ai.*naming scheme as the existing text/chat/embedding telemetry. The nil/zero guards look appropriate; there are no obvious correctness or panic risks here.core/providers/anthropic/batch.go (1)
252-266: ExpiresAt fallback avoids invalid empty-string valuesThe new conditional + 24h fallback ensures
expires_atis always a valid RFC3339 timestamp instead of an empty string, resolving the earlier API-compatibility concern. Looks good.core/schemas/chatcompletions.go (1)
21-24: Raw body accessor for chat requests is consistent
GetRawRequestBodycleanly exposesRawRequestBodyand matches the pattern used for batch/file request types. No issues.docs/integrations/bedrock-sdk/overview.mdx (1)
2-5: Front-matter updates look goodRenaming the page to "Overview" and switching the icon to
"book"align with the content and general docs conventions. No issues here.transports/bifrost-http/integrations/openai.go (1)
450-556: BatchRequestConverter wiring and provider-specific ID handling look correctThe new batch route wiring cleanly switches to
BatchRequestConverterwith:
- Correct
BatchRequestconstruction per operation (Type+ specific request field).- Provider defaults to OpenAI when unset, with overrides from query/body as appropriate.
- Symmetric ID transformations:
- Gemini:
"files-"/"batches-"↔"files/"/"batches/".- Bedrock: base64 decode inbound
BatchID/InputFileID, base64 encode on responses.The Bedrock/Anthropic
PreCallbacklogic (extra params and inlinerequestsextraction) is defensive and only activates on matching providers. Overall this aligns with the centralBatchRequestConvertercontract inrouter.go.Also applies to: 571-582, 605-654, 657-703
core/providers/ollama/ollama.go (1)
231-259: Ollama Batch/File list signatures updated to single-key without behavior changeSwitching
BatchListandFileListto accept a singleschemas.Keyaligns this provider with the new single-key interface while preserving the existing “unsupported operation” behavior. Looks good.core/providers/cerebras/cerebras.go (1)
219-222: Cerebras FileList/BatchList moved to single-key form consistentlyThe updated
FileListandBatchListsignatures now accept a singleschemas.Key, matching the rest of the provider interface, while still returning “unsupported operation” errors. No issues.Also applies to: 245-247
core/providers/vertex/vertex.go (1)
1494-1496: LGTM!The
FileListsignature update to use a singleschemas.Keyis consistent with the PR-wide pattern. The underscore parameter naming is appropriate since the key is unused in this stub.transports/bifrost-http/integrations/anthropic.go (4)
290-298: LGTM - Batch request converter updated correctly.The rename from
BatchCreateRequestConvertertoBatchRequestConverteraligns with the generalized batch handling approach. The converter correctly extracts the provider from context and handles both Anthropic and non-Anthropic providers.
365-380: LGTM - BatchList converter correctly uses new pattern.The batch list request converter properly retrieves the provider from context and constructs the
BifrostBatchListRequestwith appropriate pagination parameters.
518-519: LGTM - Context key storage for batch create.The provider extraction and context storage pattern is correctly implemented here.
296-298:bifrostContextKeyProvideris defined as a package variable attransports/bifrost-http/integrations/utils.go:16(var bifrostContextKeyProvider = schemas.BifrostContextKey("provider")). Since both files are in the same package, the context key is properly accessible and no changes are needed.Likely an incorrect or invalid review comment.
core/providers/cohere/cohere.go (2)
850-852: LGTM!The
BatchListsignature update to use a singleschemas.Keyis consistent with the PR-wide pattern. The underscore parameter naming is appropriate for this stub method.
875-877: LGTM!The
FileListsignature update follows the same pattern asBatchListand other providers in this PR.transports/bifrost-http/integrations/router.go (4)
100-101: LGTM - Type rename is appropriate.Renaming
BatchCreateRequestConvertertoBatchRequestConverteraccurately reflects that this converter now handles all batch operations (create, list, retrieve, cancel, results), not just batch creation.
272-272: LGTM - RouteConfig field updated consistently.The field in
RouteConfigstruct is correctly updated to use the newBatchRequestConvertername.
337-339: LGTM - Route detection logic updated.The batch route detection correctly uses the renamed
BatchRequestConverterfield.
435-447: LGTM - Handler batch request processing updated.The handler correctly uses the renamed
BatchRequestConverterto convert and process batch requests.core/schemas/batch.go (6)
68-68: LGTM - RawRequestBody field added for batch create.The
RawRequestBodyfield withjson:"-"tag is correctly added to preserve the original request payload without affecting JSON serialization.
85-88: LGTM - Accessor method follows Go conventions.The
GetRawRequestBody()accessor provides a clean interface for retrieving the raw request body.
120-120: LGTM - Model field added for routing.Adding the
Modelfield toBifrostBatchListRequestenables model-based routing, consistent with other request types.
152-164: LGTM - Model and RawRequestBody fields with accessor.The additions to
BifrostBatchRetrieveRequestfollow the same pattern asBifrostBatchCreateRequest, maintaining consistency across the batch API.
207-219: LGTM - Consistent pattern for BifrostBatchCancelRequest.The
Model,RawRequestBodyfields, andGetRawRequestBody()accessor follow the established pattern.
236-251: LGTM - BifrostBatchResultsRequest updated consistently.The additions complete the pattern across all batch request types that have request bodies.
core/providers/nebius/nebius.go (2)
71-71: LGTM!The raw-request propagation via
providerUtils.ShouldSendBackRawRequestis correctly added to all OpenAI-compatible handlers, following the established pattern.Also applies to: 89-89, 111-111, 140-140, 165-165, 213-213
238-286: LGTM!The batch and file operation stubs are correctly implemented as not-supported operations, using the single-key pattern and returning appropriate errors via
NewUnsupportedOperationError.core/schemas/files.go (2)
55-55: LGTM!The
Modelfield additions are consistent across all file request structs, aligning with the broader PR pattern of including model hints for routing. Note that in some provider flows (e.g., Bedrock batch.go line 337), this field may be populated with placeholder values when not actively used.Also applies to: 111-111, 148-148, 188-188, 217-217
113-113: LGTM!The
RawRequestBodyfields andGetRawRequestBody()accessor methods are correctly implemented, following the established pattern from other request types likeBifrostChatRequest. Thejson:"-"tag appropriately prevents serialization.Also applies to: 130-133, 150-150, 161-164, 191-191, 200-203, 220-220, 229-232
core/providers/bedrock/batch.go (1)
335-337: LGTM!The
Modelfield is correctly initialized to "dummy-model" as a placeholder to satisfy validation requirements. The inline comments clearly document that this value is not used in provider flows, which is an acceptable pattern for bridging API requirements.docs/integrations/openai-sdk/overview.mdx (2)
2-2: LGTM!The title change to "Overview" and icon change to "book" appropriately reflect the restructuring of SDK documentation into grouped pages.
Also applies to: 4-4
212-212: LGTM!The path corrections and Next Steps updates are consistent with the new SDK grouping structure. The "Files and Batch API" entry correctly references the new documentation page added in this PR.
Also applies to: 380-385
docs/integrations/genai-sdk/files-and-batch.mdx (3)
1-16: LGTM!The documentation overview clearly explains cross-provider routing via the
x-model-providerheader and appropriately warns about Bedrock limitations.
413-421: Variable naming is now consistent.The batch creation examples now correctly define
uploaded_fileviaclient.files.upload(...)before usinguploaded_file.namein the batch creation call. The variable naming inconsistency flagged in previous reviews has been resolved.Also applies to: 462-470
733-961: LGTM!The end-to-end workflows are comprehensive and consistent across providers. The provider notes table clearly documents capability differences, and the Next Steps section appropriately guides users to related documentation.
Also applies to: 1000-1022
core/providers/elevenlabs/elevenlabs.go (1)
721-721: LGTM!The signature updates for
BatchListandFileListfrom[]schemas.Keytoschemas.Keyalign with the PR-wide single-key pattern adopted across multiple providers. The not-supported error handling remains appropriate.Also applies to: 746-746
docs/integrations/openai-sdk/files-and-batch.mdx (3)
1-29: LGTM!The documentation clearly explains cross-provider routing via
extra_body(POST) andextra_query(GET) parameters. The base client setup is straightforward and appropriately marked as Beta.
32-405: LGTM!The Files API and Batch API sections comprehensively document all operations with clear provider-specific examples. Configuration requirements (S3 for Bedrock, inline requests for Anthropic) are well-explained with appropriate warnings.
409-673: LGTM!The end-to-end workflows provide complete examples for each provider, demonstrating the full batch lifecycle. The provider notes table effectively summarizes capability differences, and the Next Steps section appropriately guides users to related resources.
core/providers/anthropic/anthropic.go (2)
1113-1153: LGTM: Single-key signature correctly applied.The migration from
keys []schemas.Keytokey schemas.Keyis clean and consistent. The authorization header correctly useskey.Value, and URL construction follows the expected pattern.
1582-1618: LGTM: FileList correctly migrated to single-key pattern.The method signature and implementation are consistent with the BatchList changes. URL construction and authorization logic are correct.
docs/integrations/anthropic-sdk/files-and-batch.mdx (5)
8-85: LGTM: Clear documentation of cross-provider routing.The overview and client setup sections effectively explain the cross-provider routing pattern. Bedrock limitations are appropriately highlighted with warning tags.
89-232: LGTM: Comprehensive Files API documentation.The Files API examples are well-organized with provider-specific tabs. Code samples demonstrate proper usage patterns for upload, list, delete, and download operations.
235-469: LGTM: Well-documented Batch API workflows.The Batch API section clearly demonstrates inline requests pattern and covers all operations. Provider-specific model selections are appropriate, and Bedrock limitations are clearly communicated.
473-599: LGTM: Practical end-to-end workflow examples.The workflow examples demonstrate realistic usage patterns including polling, verification, and results retrieval. The examples provide valuable guidance for implementing batch processing.
603-618: LGTM: Clear provider capability summary.The provider notes table effectively summarizes capabilities across providers. The consistent guidance on Bedrock limitations throughout the document helps prevent confusion.
core/providers/openai/openai.go (2)
2245-2286: LGTM: FileList correctly refactored to single-key pattern.The implementation is clean and consistent with the Anthropic provider changes. URL construction properly handles optional query parameters, and authorization logic is correct.
2647-2683: LGTM: BatchList migration complete and correct.The single-key refactor is implemented consistently. URL construction and authorization logic align with the FileList method and cross-provider patterns.
core/providers/bedrock/bedrock.go (2)
1294-1299: FileUpload gating + logging is safeThe added
CheckOperationAllowedcall with a nil-safe log onerr.Errorintegrates cleanly and avoids panics; no functional concerns here.
2049-2167: Single-key BatchList implementation and signing look correct
BatchListnow:
- Validates
BedrockKeyConfigpresence.- Derives region from that config.
- Builds query params for
maxResultsandnextToken.- Signs with
signAWSRequestusing the same bedrock service/region pattern as other Bedrock calls.- Maps Bedrock job summaries into
BifrostBatchListResponsewithHasMore/LastIDand metadata.This is consistent with the rest of the Bedrock provider and the new single-key pattern; no issues from a correctness or resiliency standpoint.
core/bifrost.go (2)
872-921: Centralized routing for batch/file APIs and provider dispatch is consistentAll Batch* and File* public methods now follow the same pattern as the existing text/chat/embedding APIs:
- They validate basic shape (nil req, required Provider/IDs).
- Acquire a pooled
BifrostRequest, setRequestTypeand the corresponding typed request pointer.- Delegate to
handleRequest/handleStreamRequest, which handles validation, fallbacks, plugins, queuing, and retries.handleProviderRequesthas matching cases for each newRequestTypethat call the provider’sFile*/Batch*methods and populate the corresponding field onBifrostResponse.resetBifrostRequestnow clears ListModels and all File*/Batch* fields, so pooling is safe.This wiring looks correct and keeps the lifecycle symmetrical with existing operations; no functional issues spotted.
Also applies to: 945-955, 986-996, 1027-1036, 1078-1087, 1090-1138, 1141-1177, 1180-1218, 1221-1260, 1263-1300, 2823-2937, 3121-3139
2248-2255: Explicit 400 status for invalid streaming requestsSetting
StatusCode = schemas.Ptr(fasthttp.StatusBadRequest)whenvalidateRequestfails inhandleStreamRequestgives HTTP handlers a clear signal that these are client-side validation errors (as opposed to provider/transport failures). The new fasthttp import is used appropriately and doesn’t introduce other coupling.Also applies to: 37-38
core/providers/gemini/gemini.go (2)
1627-1627: LGTM! Signature change aligns with single-key pattern.The
BatchListmethod signature change from[]schemas.Keytoschemas.Keyis consistent with the broader PR refactor that simplifies request construction across all providers.
2235-2307: LGTM! FileList properly handles pagination.The signature change to single key and the new pagination handling (lines 2305-2307) correctly propagate Gemini's
NextPageTokento the Bifrost response'sAfterfield, enabling proper cursor-based pagination for clients.core/providers/parasail/parasail.go (1)
185-233: LGTM! Explicit unsupported operation stubs.The new File and Batch operation stubs follow Go conventions (underscore for unused parameters) and provide clear unsupported operation errors. This aligns with the pattern used across other providers in this PR and ensures a consistent API surface.
core/schemas/bifrost.go (2)
170-179: LGTM! BifrostRequest extended with file/batch request types.The new request fields follow the established pattern and are properly typed. The switch-case in
GetRequestFieldscorrectly returnsnilfor fallbacks since file/batch operations don't support fallback semantics.
303-354: LGTM! Response fields and GetExtraFields properly extended.The new response fields and their corresponding cases in
GetExtraFieldsfollow the established pattern. Each new response type correctly returns a pointer to itsExtraFields.core/providers/openrouter/openrouter.go (1)
291-333: LGTM! Signature updates and unsupported operation stubs.The
BatchListsignature change to single key and the file operation stubs with underscore parameters are consistent with the pattern applied across other providers in this PR. The unsupported operation errors provide clear feedback to callers.
akshaydeo
force-pushed
the
12-15-batch_request_logging
branch
from
3c90b3a to
6d792cd
Compare
akshaydeo
force-pushed
the
12-15-batch_request_logging
branch
from
6d792cd to
d4fc1ad
Compare
Summary
Adds logging support for batch and file requests
Changes
Type of change
Affected areas
How to test
Describe the steps to validate this change. Include commands and expected outcomes.
If adding new configs or environment variables, document them here.
Screenshots/Recordings
If UI changes, add before/after screenshots or short clips.
Breaking changes
If yes, describe impact and migration instructions.
Related issues
Link related issues and discussions. Example: Closes #123
Security considerations
Note any security implications (auth, secrets, PII, sandboxing, etc.).
Checklist
docs/contributing/README.mdand followed the guidelines