fix: passthrough budgets - #3941
Conversation
|
tejas ghatte seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds provider passthrough usage extraction (Anthropic, OpenAI, Gemini/Vertex, Azure), a shared SSE streaming helper, schema and streaming-accumulator fields for passthrough path/usage, wires usage into pricing/logging/governance, and adds comprehensive extractor tests. ChangesPassthrough Usage Extraction & Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Confidence Score: 5/5Safe to merge. The phantom-billing concern from prior review threads is addressed with 2xx guards in all providers. The streaming refactor is consistent and the accumulator chain is correct. All provider streaming paths use the shared StreamPassthrough helper and are structurally correct. The 2xx guard is applied before usage extraction in every non-streaming provider. The two findings are coverage gaps in the new feature (streaming TTS billing, and image-edit/variation type precision) rather than regressions in existing behaviour. core/providers/openai/passthrough_usage.go — streaming TTS usage path; framework/modelcatalog/pricing.go — image request type coercion in inferPassthroughRequestType. Important Files Changed
Reviews (7): Last reviewed commit: "fix: passthrough budgets" | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/providers/anthropic/anthropic.go`:
- Around line 2725-2732: PassthroughStream currently accumulates the entire
stream into accBody while reading from bodyStream.Read (used later to extract
PassthroughUsage), which can OOM on very large responses; modify the
accumulation loop in the PassthroughStream logic to enforce a configurable
maxAccumulationBytes (e.g., 100MB) — continue streaming to consumers but stop
appending to accBody once the threshold is reached, set PassthroughUsage to nil
(or mark usage extraction skipped) when the threshold is exceeded, and emit a
debug/processLogger message indicating usage extraction was skipped due to size;
reference the accumulation variables accBody, bodyStream.Read, and the
PassthroughUsage extraction path when making the change.
- Around line 2615-2617: The non-streaming Passthrough response is missing
RawRequest in its ExtraFields while PassthroughStream includes RawRequest in
extraFields; to make observability consistent, add RawRequest: req.Body to the
non-streaming ExtraFields (where PassthroughPath and PassthroughUsage are set)
so both Passthrough and PassthroughStream include the same RawRequest field;
update the block that builds ExtraFields for the non-streaming path (the code
that sets PassthroughPath and PassthroughUsage and calls
ExtractAnthropicPassthroughUsage) to also include RawRequest with the same key
used in PassthroughStream.
In `@core/providers/azure/azure.go`:
- Line 3558: The PassthroughUsage assignment currently always calls
openai.ExtractOpenAIPassthroughUsage(req.Path, req.Body, body) which misses
Anthropic/other upstream usages; update the dispatch logic in the code that sets
PassthroughUsage (where PassthroughUsage is assigned) to select the extractor
based on the incoming upstream API path/host (e.g., inspect req.Path or upstream
identifier): call anthropic.ExtractAnthropicPassthroughUsage(...) for
/anthropic/* routes and openai.ExtractOpenAIPassthroughUsage(...) for OpenAI
routes, and fall back to a generic/no-op extractor if unknown so budgets/logging
are correct; ensure you reference the existing req.Path, req.Body and body
variables and keep the same return shape for PassthroughUsage.
- Around line 3658-3665: The loop reading from bodyStream and appending every
chunk into accBody (accBody = append(accBody, chunk...)) causes full buffering
of passthrough streams; change it to only accumulate when the response is a
known usage-bearing JSON/SSE path (e.g., check Content-Type, an
isUsagePath/isSSE flag, or request metadata) or enforce a small hard cap
(introduce maxAccumulateBytes) and stop appending once accBody exceeds that cap;
keep streaming all bytes onward regardless, and ensure the reader loop still
copies/forwards buf[:n] to downstream while avoiding the extra memory copy for
large binary/audio/video passthroughs.
- Around line 3636-3640: The code currently always populates
extraFields.RawRequest with the full request body
(schemas.BifrostResponseExtraFields.RawRequest), leaking sensitive data; update
the construction of extraFields to only set RawRequest when the feature flag
(e.g., sendBackRawRequest or the module's raw capture boolean) is true—otherwise
leave RawRequest empty/nil or omit the field—so modify the extraFields
assignment near extraFields := ... to conditionally include RawRequest based on
that flag.
In `@core/providers/gemini/gemini.go`:
- Around line 4254-4263: The loop currently appends every chunk into accBody
(created above) causing unbounded memory growth; change the logic around accBody
(and its use with providerUtils.StreamTerminalDetector/process path) so
accumulation is only performed for routes that require billing passthrough data
(the billable passthrough predicate you have for Gemini requests) or, if
accumulation is needed generally, replace accBody with a bounded tail buffer
(fixed-size ring/byte-slice) that keeps only the last N bytes sufficient for the
usage extractor; update the loop that reads from bodyStream and the code that
reads accBody for usage (and references to providerUtils.StreamTerminalDetector,
providerUtils.ProcessAndSendResponse, and schemas.BifrostResponse) so they read
from the gated/bounded buffer instead of an unbounded accBody, and ensure
accBody is not allocated/appended to for non-billable paths.
In `@core/providers/gemini/passthrough_usage.go`:
- Around line 280-289: The code currently only sets
u.ImageUsage.OutputTokensDetails.NImages when it's zero, which preserves the
originally requested sampleCount and leads to overbilling when resp.Predictions
is shorter; change the logic in the block that inspects
providerUtils.LastSSEOrBody -> GeminiImagenResponse so that after unmarshalling
you always set u.ImageUsage.OutputTokensDetails.NImages = len(resp.Predictions)
(ensuring OutputTokensDetails is allocated first) rather than conditionally only
when NImages == 0, thereby using the actual delivered prediction count to
override the requested sampleCount.
In `@core/providers/openai/openai.go`:
- Around line 7096-7104: The code in openai.go accumulates the entire stream
into accBody inside the bodyStream.Read loop (used alongside
providerUtils.ProcessAndSendResponse) which can grow unbounded; change accBody
to a bounded tail buffer (e.g., fixed-size ring/byte-slice kept at max N bytes)
and on each read append only up to the buffer cap by dropping oldest bytes, so
streaming forwarding via providerUtils.ProcessAndSendResponse remains unchanged
but memory stays bounded; apply the same bounded-buffer logic where accBody is
used again (the other occurrence referenced near the second block around
ProcessAndSendResponse) and ensure any final passthrough usage extraction reads
from the bounded tail buffer rather than the full ever-growing accBody.
- Around line 7074-7078: extraFields currently unconditionally sets
schemas.BifrostResponseExtraFields.RawRequest from req.Body; change this so
RawRequest is only populated when the configured "send-back" flag is enabled.
Update the code that constructs extraFields (the extraFields variable /
schemas.BifrostResponseExtraFields) to check the send-back flag (e.g., a request
or provider config like req.Config.SendBackRawRequest or the provider-level
sendBackRawRequest boolean) and set RawRequest = req.Body only if that flag is
true, otherwise leave RawRequest empty/nil; ensure other fields
(ProviderResponseHeaders, PassthroughPath) remain unchanged.
In `@core/providers/openai/passthrough_usage.go`:
- Around line 69-76: The multipart handling in the block using
multipart.NewReader(...)/mr.ReadForm(32 << 20) reads a form but never calls
form.RemoveAll(), which can leave temp files behind; update the code around
multipart.NewReader and the ReadForm call to ensure form.RemoveAll() is called
in all paths (defer immediately after a successful ReadForm or call RemoveAll()
before every return) while preserving the existing parsing of
form.Value["seconds"] and assignment to secs.
In `@core/providers/utils/passthrough.go`:
- Around line 17-25: LastSSEOrBody currently falls back to returning the raw
body and LastSSEDataLine only matches "data: " (with space), so valid SSE lines
like "data:" are missed and whole SSE envelopes (including "[DONE]") get
returned as non-JSON. Update LastSSEDataLine to accept "data:" with optional
whitespace after the colon and to ignore empty data lines and sentinel lines
like "[DONE]"; then change LastSSEOrBody so if LastSSEDataLine returns nil but
the body contains SSE markers (e.g., "data:"/"event:"), it returns nil instead
of the raw body to avoid passing raw SSE envelopes downstream. Ensure references
to LastSSEOrBody and LastSSEDataLine are used to locate and update the logic.
In `@framework/modelcatalog/pricing.go`:
- Around line 1368-1371: inferPassthroughRequestType currently collapses any
passthrough image request to schemas.ImageGenerationRequest when su.ImageUsage
is set, and detectPassthroughRequestType never handles "/images/variations",
causing edits/variations to be billed as generations; update
inferPassthroughRequestType to check path suffixes for "/images/edits" and
"/images/variations" before falling back to generation and return
schemas.ImageEditRequest or schemas.ImageVariationRequest accordingly, and add a
corresponding case for "/images/variations" in detectPassthroughRequestType so
variation requests are recognized and mapped to the correct schema (also apply
the same change in the analogous block around lines 1416-1429).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d4f945ea-d4ec-4054-9ec1-d713335b4b47
📒 Files selected for processing (18)
core/bifrost.gocore/providers/anthropic/anthropic.gocore/providers/anthropic/passthrough_usage.gocore/providers/azure/azure.gocore/providers/gemini/gemini.gocore/providers/gemini/passthrough_usage.gocore/providers/openai/openai.gocore/providers/openai/passthrough_usage.gocore/providers/utils/passthrough.gocore/providers/utils/utils.gocore/schemas/bifrost.gocore/schemas/passthrough.goframework/modelcatalog/pricing.goframework/streaming/passthrough.goframework/streaming/types.goplugins/governance/main.goplugins/logging/main.goplugins/logging/operations.go
💤 Files with no reviewable changes (1)
- core/providers/utils/utils.go
0f0c461 to
cefaa51
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (5)
framework/modelcatalog/pricing.go (1)
1352-1430:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftImage edit and variation request types not preserved in passthrough pricing.
detectPassthroughRequestTypedoes not recognize/images/variations(line 1370 checks/images/editsbut variations is missing).inferPassthroughRequestType(lines 1416-1418) collapses allImageUsagetoschemas.ImageGenerationRequest, losing edit/variation-specific pricing rows.This is a duplicate of the existing review comment on lines 1368-1371 and 1416-1429 from past reviews. The fix suggested in that comment remains valid: add
/images/variationscase todetectPassthroughRequestType, and modifyinferPassthroughRequestTypeto check the detected type whenImageUsageis present and return it if it's one of the image request types.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@framework/modelcatalog/pricing.go` around lines 1352 - 1430, detectPassthroughRequestType is missing "/images/variations" and inferPassthroughRequestType collapses all ImageUsage to ImageGenerationRequest; add a case for "/images/variations" in detectPassthroughRequestType (alongside "/images/edits") returning the correct image-specific schemas.RequestType, and update inferPassthroughRequestType so when su.ImageUsage != nil it calls detectPassthroughRequestType(provider, path) and returns the detected type if it is one of the image request types (e.g., ImageGenerationRequest, ImageEditRequest, ImageVariationRequest) instead of unconditionally returning ImageGenerationRequest.core/providers/azure/azure.go (2)
3636-3640:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard streamed
RawRequestbehind the raw-capture flag.This now copies the full passthrough request body into every streamed chunk even when raw capture is disabled, which leaks request payloads by default and inflates the stream payload.
Proposed fix
extraFields := schemas.BifrostResponseExtraFields{ ProviderResponseHeaders: headers, PassthroughPath: req.Path, - RawRequest: req.Body, } + if providerUtils.ShouldSendBackRawRequest(ctx, provider.sendBackRawRequest) { + extraFields.RawRequest = req.Body + }As per coding guidelines, "do not log secrets or sensitive request/response bodies by default."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/azure/azure.go` around lines 3636 - 3640, The code is populating extraFields := schemas.BifrostResponseExtraFields{... RawRequest: req.Body} unconditionally which copies the full passthrough body into every streamed chunk; guard setting the RawRequest field behind the raw-capture feature flag so RawRequest is only populated when raw capture is enabled (e.g., check the service/config flag or request context like enableRawCapture or opts.RawCapture) and otherwise leave RawRequest nil/empty; modify the extraFields construction to conditionally assign RawRequest (or set it after creating extraFields) so only ProviderResponseHeaders, PassthroughPath are always set and RawRequest is set only when the raw-capture flag is true.
3658-3665:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't buffer arbitrary passthrough streams in full.
accBody = append(accBody, chunk...)retains the entire upstream stream in memory just to compute final usage. On large audio/video/binary passthroughs, that doubles memory usage and defeats the point of streaming. Gate accumulation to usage-bearing JSON/SSE responses and cap it.Proposed fix
- var accBody []byte + const maxPassthroughUsageBytes = 1 << 20 // 1 MiB + contentType := strings.ToLower(headers["content-type"]) + shouldAccumulateUsage := strings.Contains(contentType, "json") || strings.Contains(contentType, "text/event-stream") + var accBody []byte buf := make([]byte, 4096) for { n, readErr := bodyStream.Read(buf) if n > 0 { chunk := make([]byte, n) copy(chunk, buf[:n]) - accBody = append(accBody, chunk...) + if shouldAccumulateUsage && len(accBody) < maxPassthroughUsageBytes { + remaining := maxPassthroughUsageBytes - len(accBody) + if remaining > len(chunk) { + remaining = len(chunk) + } + accBody = append(accBody, chunk[:remaining]...) + } providerUtils.ProcessAndSendResponse(ctx, postHookRunner, &schemas.BifrostResponse{ PassthroughResponse: &schemas.BifrostPassthroughResponse{ StatusCode: statusCode, @@ if readErr == io.EOF { ctx.SetValue(schemas.BifrostContextKeyStreamEndIndicator, true) extraFields.Latency = time.Since(startTime).Milliseconds() + var usage *schemas.BifrostPassthroughUsage + if shouldAccumulateUsage { + usage = extractAzurePassthroughUsage(req.Path, req.Body, accBody, req.Model) + } providerUtils.ProcessAndSendResponse(ctx, postHookRunner, &schemas.BifrostResponse{ PassthroughResponse: &schemas.BifrostPassthroughResponse{ StatusCode: statusCode, Headers: headers, ExtraFields: extraFields, - PassthroughUsage: extractAzurePassthroughUsage(req.Path, req.Body, accBody, req.Model), + PassthroughUsage: usage, }, }, ch, postHookSpanFinalizer) returnAlso applies to: 3675-3683
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/azure/azure.go` around lines 3658 - 3665, The code currently accumulates the entire passthrough stream into accBody by repeatedly appending chunks read from bodyStream (see accBody, bodyStream, Read), which can exhaust memory for large binary/audio/video payloads; change this to only buffer response bytes when the response is a usage-bearing JSON/SSE payload or otherwise required: inspect the response Content-Type (or an SSE/JSON flag) and only append chunks to accBody in that case, otherwise stream through without buffering (e.g., copy directly to the downstream writer or io.Discard). Also introduce a hard cap (e.g., maxAccBytes) and stop buffering once reached (and mark it truncated) to avoid unbounded growth; apply the same change to the similar block around lines 3675-3683. Ensure all reads still forward data to the consumer so streaming behavior is preserved.core/providers/openai/openai.go (2)
7074-7078:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate passthrough
RawRequestbehind send-back config.Line 7077 always sets
RawRequest, which can leak request payloads even when raw-request return is disabled.🔧 Suggested fix
extraFields := schemas.BifrostResponseExtraFields{ ProviderResponseHeaders: headers, PassthroughPath: req.Path, - RawRequest: req.Body, } + if providerUtils.ShouldSendBackRawRequest(ctx, provider.sendBackRawRequest) { + extraFields.RawRequest = append([]byte(nil), req.Body...) + }As per coding guidelines "Apply Go security practices: do not log secrets or sensitive request/response bodies by default."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/openai/openai.go` around lines 7074 - 7078, The code always assigns RawRequest into extraFields (schemas.BifrostResponseExtraFields) which can leak sensitive payloads; change the construction so RawRequest is only included when the "send-back raw request" configuration is enabled (check the existing send-back flag/setting used elsewhere in this package, e.g., a sendBackRawRequest or similar field on the request/context/config). Concretely, build ProviderResponseHeaders and PassthroughPath unconditionally, and conditionally set RawRequest on extraFields only when that send-back flag is true (or omit the field otherwise), referencing the extraFields variable and the RawRequest field on schemas.BifrostResponseExtraFields to implement the guard. Ensure you follow the same config/flag lookup pattern used elsewhere in this file to remain consistent.
7096-7104:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBound
accBodygrowth in passthrough streaming.Line 7103 appends every chunk into
accBodywithout a limit. Long streams can cause memory pressure/OOM before final usage extraction at Line 7121.🔧 Suggested fix (bounded tail buffer)
- var accBody []byte + const usageTailLimit = 256 * 1024 + var accBody []byte buf := make([]byte, 4096) for { n, readErr := bodyStream.Read(buf) if n > 0 { chunk := make([]byte, n) copy(chunk, buf[:n]) accBody = append(accBody, chunk...) + if len(accBody) > usageTailLimit { + accBody = append([]byte(nil), accBody[len(accBody)-usageTailLimit:]...) + } providerUtils.ProcessAndSendResponse(ctx, postHookRunner, &schemas.BifrostResponse{As per coding guidelines "Apply standard Go review practices: ... bounded goroutines/channels ...".
Also applies to: 7118-7121
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/openai/openai.go` around lines 7096 - 7104, The loop reading from bodyStream currently appends every chunk into accBody (used later for extraction), which can grow unbounded and cause OOM; change this to maintain a bounded tail buffer: replace the unbounded append logic around accBody in the bodyStream.Read loop with a fixed-capacity buffer (e.g., a circular/ring or sliding-window byte buffer limited to a configurable max bytes) that retains only the last N bytes while still calling providerUtils.ProcessAndSendResponse(&schemas.BifrostResponse{...}) for each chunk; ensure the final extraction code that reads from accBody (the code after the loop) reads from this bounded buffer instead of the unbounded accBody so memory usage stays bounded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/providers/anthropic/anthropic.go`:
- Around line 2703-2707: The code currently populates extraFields :=
schemas.BifrostResponseExtraFields{ RawRequest: req.Body, ... } and reuses
extraFields for every streaming chunk; move RawRequest out of the per-chunk
extraFields and only attach it to the final chunk (where PassthroughUsage is
set) to match the ChatCompletionStream pattern and avoid per-chunk payload
bloat—update the streaming loop that emits intermediate chunks to omit
RawRequest (keep ProviderResponseHeaders and PassthroughPath) and only set
RawRequest on the final emission alongside PassthroughUsage, unless you
intentionally want full-request observability for every chunk (in which case add
a clear comment explaining that decision).
In `@core/providers/anthropic/passthrough_usage.go`:
- Around line 14-153: buildAnthropicPassthroughUsage currently casts
*au.ServiceTier directly to schemas.BifrostServiceTier which is wrong for
Anthropic values; update buildAnthropicPassthroughUsage to, when au.ServiceTier
!= nil, call MapAnthropicServiceTierToBifrost(*au.ServiceTier) to get the
correct schemas.BifrostServiceTier value and assign its pointer to u.ServiceTier
(instead of the direct cast) so service_tier is mapped to OpenAI-compatible
values; locate this change in the buildAnthropicPassthroughUsage function where
u.ServiceTier is set.
In `@core/providers/gemini/gemini.go`:
- Around line 4233-4237: The code always sets
schemas.BifrostResponseExtraFields.RawRequest = req.Body which leaks raw request
bodies into streamed passthrough chunks; change construction of extraFields so
RawRequest is only populated when the existing opt-in flag is enabled (e.g.,
check the project’s config flag such as
EnableRawRequestEcho/AllowRawRequestPassthrough) — otherwise leave RawRequest
nil/empty; ensure all places that create or copy extraFields (the extraFields
variable used for streamed passthroughs and any post-hook logging) use this
conditional construction so req.Body is never attached by default.
In `@core/providers/openai/passthrough_usage.go`:
- Around line 46-47: ExtractOpenAIPassthroughUsage currently routes any path
containing "/video" to extractOAIVideoUsage which can misclassify non-billable
GET/DELETE video routes; update the routing so that extractOAIVideoUsage is only
invoked for billable POST endpoints by checking the HTTP method and matching
specific billable path patterns (e.g., POST /v1/videos, POST
/v1/videos/{id}/remix, POST /v1/videos/edits, POST /v1/videos/extensions) before
calling extractOAIVideoUsage; references: modify the switch/case in
ExtractOpenAIPassthroughUsage and use extractOAIVideoUsage and
openAIVideoDefaultSeconds unchanged, ensuring non-POST video routes fall through
to the non-video handlers.
- Around line 246-264: The code currently only attempts sonic.Unmarshal(reqBody,
&OpenAIImageGenerationRequest) so multipart/form-data bodies (used by
/v1/images/edits and /v1/images/variations) won’t populate size/quality/n;
update the logic to detect and handle multipart before the JSON unmarshal: if
the incoming Content-Type is multipart/form-data, parse reqBody as a multipart
form (extract form values "size", "quality", and "n" and convert types) and set
u.ImageSize, u.ImageQuality and u.ImageUsage.OutputTokensDetails.NImages
accordingly (creating u.ImageUsage and OutputTokensDetails if nil), otherwise
fall back to sonic.Unmarshal into OpenAIImageGenerationRequest and use
req.Size/req.Quality/req.N as before.
---
Duplicate comments:
In `@core/providers/azure/azure.go`:
- Around line 3636-3640: The code is populating extraFields :=
schemas.BifrostResponseExtraFields{... RawRequest: req.Body} unconditionally
which copies the full passthrough body into every streamed chunk; guard setting
the RawRequest field behind the raw-capture feature flag so RawRequest is only
populated when raw capture is enabled (e.g., check the service/config flag or
request context like enableRawCapture or opts.RawCapture) and otherwise leave
RawRequest nil/empty; modify the extraFields construction to conditionally
assign RawRequest (or set it after creating extraFields) so only
ProviderResponseHeaders, PassthroughPath are always set and RawRequest is set
only when the raw-capture flag is true.
- Around line 3658-3665: The code currently accumulates the entire passthrough
stream into accBody by repeatedly appending chunks read from bodyStream (see
accBody, bodyStream, Read), which can exhaust memory for large
binary/audio/video payloads; change this to only buffer response bytes when the
response is a usage-bearing JSON/SSE payload or otherwise required: inspect the
response Content-Type (or an SSE/JSON flag) and only append chunks to accBody in
that case, otherwise stream through without buffering (e.g., copy directly to
the downstream writer or io.Discard). Also introduce a hard cap (e.g.,
maxAccBytes) and stop buffering once reached (and mark it truncated) to avoid
unbounded growth; apply the same change to the similar block around lines
3675-3683. Ensure all reads still forward data to the consumer so streaming
behavior is preserved.
In `@core/providers/openai/openai.go`:
- Around line 7074-7078: The code always assigns RawRequest into extraFields
(schemas.BifrostResponseExtraFields) which can leak sensitive payloads; change
the construction so RawRequest is only included when the "send-back raw request"
configuration is enabled (check the existing send-back flag/setting used
elsewhere in this package, e.g., a sendBackRawRequest or similar field on the
request/context/config). Concretely, build ProviderResponseHeaders and
PassthroughPath unconditionally, and conditionally set RawRequest on extraFields
only when that send-back flag is true (or omit the field otherwise), referencing
the extraFields variable and the RawRequest field on
schemas.BifrostResponseExtraFields to implement the guard. Ensure you follow the
same config/flag lookup pattern used elsewhere in this file to remain
consistent.
- Around line 7096-7104: The loop reading from bodyStream currently appends
every chunk into accBody (used later for extraction), which can grow unbounded
and cause OOM; change this to maintain a bounded tail buffer: replace the
unbounded append logic around accBody in the bodyStream.Read loop with a
fixed-capacity buffer (e.g., a circular/ring or sliding-window byte buffer
limited to a configurable max bytes) that retains only the last N bytes while
still calling
providerUtils.ProcessAndSendResponse(&schemas.BifrostResponse{...}) for each
chunk; ensure the final extraction code that reads from accBody (the code after
the loop) reads from this bounded buffer instead of the unbounded accBody so
memory usage stays bounded.
In `@framework/modelcatalog/pricing.go`:
- Around line 1352-1430: detectPassthroughRequestType is missing
"/images/variations" and inferPassthroughRequestType collapses all ImageUsage to
ImageGenerationRequest; add a case for "/images/variations" in
detectPassthroughRequestType (alongside "/images/edits") returning the correct
image-specific schemas.RequestType, and update inferPassthroughRequestType so
when su.ImageUsage != nil it calls detectPassthroughRequestType(provider, path)
and returns the detected type if it is one of the image request types (e.g.,
ImageGenerationRequest, ImageEditRequest, ImageVariationRequest) instead of
unconditionally returning ImageGenerationRequest.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1476b4f8-60db-43fe-aca5-908d2ce84ea6
📒 Files selected for processing (18)
core/bifrost.gocore/providers/anthropic/anthropic.gocore/providers/anthropic/passthrough_usage.gocore/providers/azure/azure.gocore/providers/gemini/gemini.gocore/providers/gemini/passthrough_usage.gocore/providers/openai/openai.gocore/providers/openai/passthrough_usage.gocore/providers/utils/passthrough.gocore/providers/utils/utils.gocore/schemas/bifrost.gocore/schemas/passthrough.goframework/modelcatalog/pricing.goframework/streaming/passthrough.goframework/streaming/types.goplugins/governance/main.goplugins/logging/main.goplugins/logging/operations.go
💤 Files with no reviewable changes (1)
- core/providers/utils/utils.go
cefaa51 to
82bf99d
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
core/providers/azure/azure.go (2)
3677-3677:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate
RawRequestassignment onsendBackRawRequestflag.
extraFields.RawRequest = req.Bodyis set unconditionally at EOF, leaking the full request body even when raw capture is disabled.Suggested fix
if readErr == io.EOF { ctx.SetValue(schemas.BifrostContextKeyStreamEndIndicator, true) extraFields.Latency = time.Since(startTime).Milliseconds() - extraFields.RawRequest = req.Body + if providerUtils.ShouldSendBackRawRequest(ctx, provider.sendBackRawRequest) { + extraFields.RawRequest = req.Body + } providerUtils.ProcessAndSendResponse(ctx, postHookRunner, &schemas.BifrostResponse{As per coding guidelines, "do not log secrets or sensitive request/response bodies by default."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/azure/azure.go` at line 3677, The code unconditionally assigns req.Body to extraFields.RawRequest, leaking sensitive data; update the logic in the sendBackRawRequest handling (where extraFields.RawRequest is set) to only assign req.Body when the sendBackRawRequest flag is true (otherwise leave extraFields.RawRequest unset or nil/empty), i.e., guard the assignment with the sendBackRawRequest condition in the same function/block that builds extraFields so RawRequest is only populated when raw capture is explicitly enabled.
3657-3664:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftUnbounded buffering of streamed bytes for usage extraction.
accBody = append(accBody, chunk...)copies every byte, doubling memory for large audio/video/binary passthrough streams. Consider either:
- Gating accumulation to known usage-bearing paths (JSON/SSE chat endpoints), or
- Enforcing a size cap and skipping usage extraction once exceeded.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/azure/azure.go` around lines 3657 - 3664, The loop that reads from bodyStream and appends into accBody (accBody = append(accBody, chunk...)) can grow unbounded and double memory for large streams; change it to only accumulate when the request is a usage-bearing path (e.g., JSON chat/SSE) or enforce a hard cap: introduce a maxUsageBytes constant and a boolean like collectingUsage; while reading from bodyStream (the for { n, readErr := bodyStream.Read(buf) ... } loop) append only up to maxUsageBytes and, once reached, stop growing accBody (set collectingUsage=false or mark truncated) to skip further usage extraction; also avoid the extra per-chunk copy by appending buf[:n] when within the cap. Ensure any downstream code that reads accBody handles the truncated flag appropriately.core/providers/openai/openai.go (2)
7115-7115:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate
RawRequeston the configured send-back flag.Line 7115 sets
extraFields.RawRequestunconditionally, which can expose request payloads even when raw-request capture is disabled.🔧 Suggested fix
- extraFields.RawRequest = req.Body + if sendBackRawRequest { + extraFields.RawRequest = append([]byte(nil), req.Body...) + }As per coding guidelines "Apply Go security practices: do not log secrets or sensitive request/response bodies by default."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/openai/openai.go` at line 7115, The code currently sets extraFields.RawRequest = req.Body unconditionally; change it to only set extraFields.RawRequest when the configured "send-back raw request" flag is enabled (e.g., check the relevant config/option such as sendBackRawRequests or cfg.SendBackRawRequests) and otherwise leave extraFields.RawRequest unset (or nil); ensure you reference extraFields.RawRequest and req.Body and perform a nil-safe check of the flag before assigning so raw request payloads are not captured unless explicitly enabled.
7095-7095:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBound
accBodygrowth during passthrough streaming usage extraction.Lines 7095/7102 append every chunk to
accBodywithout a limit; long streams can drive avoidable memory growth before the final usage parse at Lines 7118-7121.🔧 Suggested fix (bounded tail buffer)
- var accBody []byte + const usageTailLimit = 256 * 1024 // keep only tail needed for final usage extraction + var accBody []byte buf := make([]byte, 4096) for { n, readErr := bodyStream.Read(buf) if n > 0 { chunk := make([]byte, n) copy(chunk, buf[:n]) - accBody = append(accBody, chunk...) + accBody = append(accBody, chunk...) + if len(accBody) > usageTailLimit { + accBody = append([]byte(nil), accBody[len(accBody)-usageTailLimit:]...) + }As per coding guidelines "Apply standard Go review practices ... bounded resource usage" and "core/** ... concurrency safety ...".
Also applies to: 7102-7102, 7118-7121
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/openai/openai.go` at line 7095, The accBody slice is unboundedly appended to during passthrough streaming usage extraction, risking memory growth; change the accumulation to a bounded tail buffer (e.g., keep only the last N bytes) when appending chunks to accBody so it never grows beyond a fixed cap, and then use that bounded buffer for the final usage parse (the code that appends to accBody at the chunk-handling site and reads/parse it at the final usage parse site around the accBody variable and the final parse logic). Implement the cap by truncating/rotating the slice when its length would exceed MAX_TAIL_BYTES (or by preallocating a fixed-size ring buffer), ensuring behaviour is identical for extracting the usage info while preventing unbounded memory growth.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/providers/anthropic/anthropic.go`:
- Around line 2744-2750: The code unconditionally sets extraFields.RawRequest
(in the response assembly around ProcessAndSendResponse) which can expose
sensitive payloads; change it to only assign extraFields.RawRequest when the
sendBackRawRequest flag is true (i.e., wrap the assignment in an if
sendBackRawRequest { extraFields.RawRequest = req.Body }), leaving other fields
and ExtractAnthropicPassthroughUsage intact so raw-request echoing is disabled
by default.
In `@core/providers/gemini/gemini.go`:
- Around line 4254-4262: The code accumulates raw bytes into accBody from
bodyStream (using providerUtils.StreamTerminalDetector) and passes them to
ExtractGeminiPassthroughUsage without decompressing, so gzip-encoded SSE will
produce nil usage; fix by running providerUtils.DecompressStreamBody on accBody
(or on the accumulated buffer) before calling ExtractGeminiPassthroughUsage,
ensuring you handle/decorate any returned error and use the decompressed bytes
for usage extraction; apply the same change to the analogous block that covers
the other occurrence (lines for the second loop handling the stream).
In `@core/providers/gemini/passthrough_usage.go`:
- Around line 22-55: The extractor currently always applies request-derived
fallbacks (e.g., extractGeminiVeoUsage returning VideoSeconds,
extractGeminiImagenUsage falling back to sampleCount) even for upstream 4xx/5xx
passthroughs; modify ExtractGeminiPassthroughUsage to determine upstream success
(e.g., parse a status code or success flag from accBody) and thread that
upstreamSucceeded boolean (or statusCode int) into downstream helpers (change
signatures of extractGeminiVeoUsage, extractGeminiImagenUsage,
extractGeminiGenerateContentUsage, extractGeminiPredictUsage, etc.), then update
those functions to suppress request-only fallbacks unless upstreamSucceeded is
true so billing-only occurs when the upstream actually produced media or
succeeded.
In `@framework/streaming/passthrough.go`:
- Around line 91-93: Update the comment that incorrectly references
"StreamUsage" to instead reference the actual source field
passthroughUsage.LLMUsage and clarify that we're populating TokenUsage from
passthroughUsage.LLMUsage so applyStreamingOutputToEntry sets
entry.TokenUsageParsed via the standard streaming token path; keep the rest of
the explanatory text intact and ensure the symbols passthroughUsage.LLMUsage,
applyStreamingOutputToEntry, and entry.TokenUsageParsed are mentioned for
clarity.
---
Duplicate comments:
In `@core/providers/azure/azure.go`:
- Line 3677: The code unconditionally assigns req.Body to
extraFields.RawRequest, leaking sensitive data; update the logic in the
sendBackRawRequest handling (where extraFields.RawRequest is set) to only assign
req.Body when the sendBackRawRequest flag is true (otherwise leave
extraFields.RawRequest unset or nil/empty), i.e., guard the assignment with the
sendBackRawRequest condition in the same function/block that builds extraFields
so RawRequest is only populated when raw capture is explicitly enabled.
- Around line 3657-3664: The loop that reads from bodyStream and appends into
accBody (accBody = append(accBody, chunk...)) can grow unbounded and double
memory for large streams; change it to only accumulate when the request is a
usage-bearing path (e.g., JSON chat/SSE) or enforce a hard cap: introduce a
maxUsageBytes constant and a boolean like collectingUsage; while reading from
bodyStream (the for { n, readErr := bodyStream.Read(buf) ... } loop) append only
up to maxUsageBytes and, once reached, stop growing accBody (set
collectingUsage=false or mark truncated) to skip further usage extraction; also
avoid the extra per-chunk copy by appending buf[:n] when within the cap. Ensure
any downstream code that reads accBody handles the truncated flag appropriately.
In `@core/providers/openai/openai.go`:
- Line 7115: The code currently sets extraFields.RawRequest = req.Body
unconditionally; change it to only set extraFields.RawRequest when the
configured "send-back raw request" flag is enabled (e.g., check the relevant
config/option such as sendBackRawRequests or cfg.SendBackRawRequests) and
otherwise leave extraFields.RawRequest unset (or nil); ensure you reference
extraFields.RawRequest and req.Body and perform a nil-safe check of the flag
before assigning so raw request payloads are not captured unless explicitly
enabled.
- Line 7095: The accBody slice is unboundedly appended to during passthrough
streaming usage extraction, risking memory growth; change the accumulation to a
bounded tail buffer (e.g., keep only the last N bytes) when appending chunks to
accBody so it never grows beyond a fixed cap, and then use that bounded buffer
for the final usage parse (the code that appends to accBody at the
chunk-handling site and reads/parse it at the final usage parse site around the
accBody variable and the final parse logic). Implement the cap by
truncating/rotating the slice when its length would exceed MAX_TAIL_BYTES (or by
preallocating a fixed-size ring buffer), ensuring behaviour is identical for
extracting the usage info while preventing unbounded memory growth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 68fdddd2-0aac-46ef-9133-37ad91df4d81
📒 Files selected for processing (19)
core/bifrost.gocore/providers/anthropic/anthropic.gocore/providers/anthropic/passthrough_usage.gocore/providers/azure/azure.gocore/providers/gemini/gemini.gocore/providers/gemini/passthrough_usage.gocore/providers/openai/openai.gocore/providers/openai/passthrough_usage.gocore/providers/utils/passthrough.gocore/providers/utils/utils.gocore/providers/vertex/vertex.gocore/schemas/bifrost.gocore/schemas/passthrough.goframework/modelcatalog/pricing.goframework/streaming/passthrough.goframework/streaming/types.goplugins/governance/main.goplugins/logging/main.goplugins/logging/operations.go
💤 Files with no reviewable changes (1)
- core/providers/utils/utils.go
82bf99d to
07c7575
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (4)
core/providers/azure/azure.go (1)
3633-3657:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse path-first extractor dispatch for Azure passthrough usage.
Line 3634 and Line 3706 route Anthropic/OpenAI extraction only from
model, but this request shape can be model-ambiguous whilepathis definitive (/anthropic/...). That can silently drop Anthropic usage and skew budgets/logging.💡 Proposed fix
- var anthropicUsage *anthropic.AnthropicPassthroughStreamUsage - if schemas.IsAnthropicModel(req.Model) { + isAnthropicRoute := strings.HasPrefix(req.Path, "/anthropic/") || schemas.IsAnthropicModel(req.Model) + + var anthropicUsage *anthropic.AnthropicPassthroughStreamUsage + if isAnthropicRoute { anthropicUsage = &anthropic.AnthropicPassthroughStreamUsage{} } @@ - HasUsage: func(event []byte) bool { - if anthropicUsage != nil { + HasUsage: func(event []byte) bool { + if isAnthropicRoute { return anthropic.HasAnthropicPassthroughUsage(event) } return openai.HasOpenAIPassthroughUsage(event) }, Observe: func(event []byte) *schemas.BifrostPassthroughUsage { - if anthropicUsage != nil { + if isAnthropicRoute { return anthropicUsage.ObserveEvent(event) } return openai.ExtractOpenAIPassthroughUsage(req.Path, req.Body, event) }, @@ func extractAzurePassthroughUsage(path string, reqBody, accBody []byte, model string) *schemas.BifrostPassthroughUsage { - if schemas.IsAnthropicModel(model) { + if strings.HasPrefix(path, "/anthropic/") || schemas.IsAnthropicModel(model) { return anthropic.ExtractAnthropicPassthroughUsage(path, reqBody, accBody) } return openai.ExtractOpenAIPassthroughUsage(path, reqBody, accBody) }As per coding guidelines, "For provider changes, verify converters remain pure, OpenAI helper changes account for delegated providers."
Also applies to: 3705-3709
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/azure/azure.go` around lines 3633 - 3657, The passthrough usage dispatch currently relies only on schemas.IsAnthropicModel(req.Model) (anthropicUsage variable) which can misclassify requests; change the logic in the providerUtils.PassthroughStreamParams callbacks (HasUsage and Observe) to prefer path-based detection first (e.g., inspect req.Path for Anthropic vs OpenAI endpoints such as "/anthropic/") and only fall back to model-based checks; update references in this block (anthropicUsage, HasUsage, Observe) so HasUsage calls anthropic.HasAnthropicPassthroughUsage when the path indicates Anthropic and otherwise calls openai.HasOpenAIPassthroughUsage, and similarly have Observe call anthropicUsage.ObserveEvent / openai.ExtractOpenAIPassthroughUsage based on path-first dispatch.core/providers/openai/passthrough_usage.go (2)
247-268:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle multipart image request fields before the JSON fallback.
/v1/images/variationsis documented with form parts like-F image,-F n, and-F size, and/v1/images/editsexamples also use-Fuploads. This block onlysonic.Unmarshals JSON, so multipart edits/variations will drop request-derived pricing inputs such asn,size, andquality, which can undercount or misprice the request. Parse multipart form values before falling back to JSON. (developers.openai.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/openai/passthrough_usage.go` around lines 247 - 268, This code only attempts JSON unmarshalling (sonic.Unmarshal into OpenAIImageGenerationRequest) and therefore ignores multipart/form-data fields used by image endpoints; update the logic to first detect and parse multipart form values (e.g., check request.Form / multipart form parts) for fields "n", "size", and "quality" and populate u.ImageSize, u.ImageQuality and u.ImageUsage.OutputTokensDetails.NImages accordingly before falling back to the JSON path that uses sonic.Unmarshal and OpenAIImageGenerationRequest; ensure you still create u.ImageUsage and u.ImageUsage.OutputTokensDetails when setting NImages, and only use the JSON branch if no multipart values are present.
46-47:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThread the HTTP method into video usage dispatch.
Because this helper only sees
path,strings.Contains(path, "/video")collapses billable create/edit/extend/remix calls together with free list/retrieve/delete/content endpoints under/videos. Those non-billable responses will still fall intoextractOAIVideoUsageand get a defaultVideoSeconds=4. The extractor contract needs the request method so only the POST generation routes are billed. (developers.openai.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/openai/passthrough_usage.go` around lines 46 - 47, The video branch must consider the HTTP method so only billable generation routes are handled: change the dispatch condition in the switch to include the request method (e.g., check req.Method == "POST" along with strings.Contains(path, "/video")), and update the call to extractOAIVideoUsage to pass the method (add a method parameter to extractOAIVideoUsage). Then modify extractOAIVideoUsage to return zero/empty usage for non-billable methods and only compute VideoSeconds for POST generation endpoints (avoid defaulting to VideoSeconds=4 for all /videos routes). Ensure you update the function signature and all call sites to use the new method parameter (reference: extractOAIVideoUsage).core/providers/openai/openai.go (1)
7068-7084:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate streaming passthrough
RawRequestbehind the send-back flag.Line 7075 always forwards
req.BodyintoStreamPassthrough, so passthrough streams can expose request payloads even when raw-request send-back is disabled.As per coding guidelines "Apply Go security practices: do not log secrets or sensitive request/response bodies by default."🔧 Suggested fix
- // Forward raw chunks to the client and extract usage incrementally per SSE event — - return providerUtils.StreamPassthrough( + // Forward raw chunks to the client and extract usage incrementally per SSE event — + sendBackRawRequest := providerUtils.ShouldSendBackRawRequest(ctx, provider.sendBackRawRequest) + params := providerUtils.PassthroughStreamParams{ + StatusCode: resp.StatusCode(), + Headers: headers, + Path: req.Path, + CancellationBody: providerUtils.PassthroughJSONBody(fasthttpReq, req.Body), + StartTime: startTime, + Logger: provider.logger, + HasUsage: HasOpenAIPassthroughUsage, + Observe: func(event []byte) *schemas.BifrostPassthroughUsage { + return ExtractOpenAIPassthroughUsage(req.Path, req.Body, event) + }, + } + if sendBackRawRequest { + params.RawRequest = append([]byte(nil), req.Body...) + } + return providerUtils.StreamPassthrough( ctx, postHookRunner, postHookSpanFinalizer, resp, rawBodyStream, - providerUtils.PassthroughStreamParams{ - StatusCode: resp.StatusCode(), - Headers: headers, - Path: req.Path, - RawRequest: req.Body, - CancellationBody: providerUtils.PassthroughJSONBody(fasthttpReq, req.Body), - StartTime: startTime, - Logger: provider.logger, - HasUsage: HasOpenAIPassthroughUsage, - Observe: func(event []byte) *schemas.BifrostPassthroughUsage { - return ExtractOpenAIPassthroughUsage(req.Path, req.Body, event) - }, - }, + params, ), nil🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/openai/openai.go` around lines 7068 - 7084, The code currently always forwards req.Body into providerUtils.StreamPassthrough via PassthroughStreamParams.RawRequest and CancellationBody; gate these fields behind the raw-request "send-back" flag (the config flag controlling whether raw requests are returned). In the StreamPassthrough call (and when constructing providerUtils.PassthroughJSONBody), check the send-back flag and only set RawRequest and CancellationBody when the flag is true; otherwise set them to nil/empty so request payloads are not forwarded. Keep all other params (StatusCode, Headers, HasUsage, Observe, etc.) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/bifrost.go`:
- Line 6451: Guard against a nil passthroughResponse after calling
provider.Passthrough to avoid a panic: check the returned variable
passthroughResponse for nil before reading or assigning passthroughResponse.Path
(the assignment to req.BifrostRequest.PassthroughRequest.Path should only happen
when passthroughResponse != nil). Update the worker path where
provider.Passthrough(...) is invoked to explicitly handle the (nil, nil) case by
returning an error or setting a safe default, and keep the existing error branch
intact so dereferences of passthroughResponse are only performed when the
response is non-nil.
In `@core/providers/azure/passthrough_usage_test.go`:
- Around line 9-27: Replace the two subtests in TestExtractAzurePassthroughUsage
with a table-driven loop: define a slice of cases (fields: name, path, body
[]byte, model string, wantPrompt, wantCompletion, wantTotal) and iterate over
them calling t.Run(case.name, func(t *testing.T){...}); inside each iteration
call extractAzurePassthroughUsage with the case.path, nil, case.body, case.model
and assert u and u.LLMUsage are non-nil and that PromptTokens, CompletionTokens
and TotalTokens equal case.wantPrompt, case.wantCompletion and case.wantTotal;
keep the original failure message format (e.g., t.Fatalf("... = %+v", u)) and
retain TestExtractAzurePassthroughUsage and extractAzurePassthroughUsage
references so the routing matrix is extensible without duplicating assertions.
In `@core/providers/openai/passthrough_usage_test.go`:
- Around line 144-205: The image/video test suite is asserting multipart form
parsing that ExtractOpenAIPassthroughUsage does not implement (it only
JSON-unmarshals reqBody) and lacks a JSON-body test for video seconds; update
tests to stop relying on multipart for images/edits and add a regression case
that calls ExtractOpenAIPassthroughUsage("/v1/videos", []byte(`{"seconds":6}`),
nil) and asserts u.VideoSeconds != nil && *u.VideoSeconds == 6 (reference
TestExtractOpenAIPassthroughUsage_* and the ExtractOpenAIPassthroughUsage
function), and remove or avoid multipartBody-based expectations for image/video
tests.
In `@core/providers/openai/passthrough_usage.go`:
- Around line 67-85: extractOAIVideoUsage only checks multipart bodies and
leaves secs at openAIVideoDefaultSeconds, so JSON payloads with a top-level
"seconds" are ignored; update extractOAIVideoUsage to first attempt to parse
reqBody as JSON (unmarshal into a small struct or map and read "seconds"),
validating >0 and using that value, and only if JSON parsing doesn't yield a
valid seconds fallback to the existing multipart logic (multipart.NewReader /
mr.ReadForm / form.Value["seconds"]); ensure you still default to
openAIVideoDefaultSeconds when no valid seconds is found and preserve
form.RemoveAll cleanup.
In `@core/providers/utils/passthrough_stream.go`:
- Around line 60-63: The emitted chunks reuse params.Headers (assigned into
schemas.BifrostResponseExtraFields.ProviderResponseHeaders and
PassthroughResponse), causing a shared map that can be mutated by downstream
readers; before each emission (every place where extraFields is built and where
PassthroughResponse is constructed—see uses around the assignments to
schemas.BifrostResponseExtraFields and any PassthroughResponse emissions at the
noted blocks), create a shallow copy of params.Headers (e.g., allocate a new map
and copy each key->slice value) and use that copy for ProviderResponseHeaders
and for the PassthroughResponse so each published chunk has its own snapshot and
avoids races/aliasing.
In `@core/providers/vertex/vertex.go`:
- Around line 3042-3043: The code currently unconditionally copies req.Body into
the passthrough metadata (fields PassthroughPath and RawRequest) in both unary
and streaming paths; update the logic to call ShouldSendBackRawRequest(...) and
only set RawRequest (and any passthrough body fields) when that function returns
true. Locate the places that assign PassthroughPath and RawRequest (around the
unary handler and the streaming handler—referring to the variables req.Body,
PassthroughPath, RawRequest and the ShouldSendBackRawRequest function) and guard
the RawRequest assignment (and any copying of req.Body into passthrough
metadata) behind that conditional, leaving other metadata like PassthroughPath
unchanged if desired.
- Line 3045: The PassthroughUsage field is always set via
gemini.ExtractGeminiPassthroughUsage, which misses Anthropic and
OpenAI-compatible Vertex proxy responses; update the Vertex passthrough response
construction inside VertexProvider.Passthrough* to route by normalized req.Path
and publisher (or routed API family) and call the appropriate extractor: use the
Anthropic extractor for Anthropic-publisher routes, the OpenAI extractor for
openapi/... or OpenAI-compatible paths, then fall back to
gemini.ExtractGeminiPassthroughUsage if neither matches; ensure you call the
correct extractor functions (e.g., ExtractAnthropicPassthroughUsage,
ExtractOpenAIPassthroughUsage, ExtractGeminiPassthroughUsage), preserve existing
response/error metadata, and keep converters pure and streaming/fasthttp
resource handling unchanged.
---
Duplicate comments:
In `@core/providers/azure/azure.go`:
- Around line 3633-3657: The passthrough usage dispatch currently relies only on
schemas.IsAnthropicModel(req.Model) (anthropicUsage variable) which can
misclassify requests; change the logic in the
providerUtils.PassthroughStreamParams callbacks (HasUsage and Observe) to prefer
path-based detection first (e.g., inspect req.Path for Anthropic vs OpenAI
endpoints such as "/anthropic/") and only fall back to model-based checks;
update references in this block (anthropicUsage, HasUsage, Observe) so HasUsage
calls anthropic.HasAnthropicPassthroughUsage when the path indicates Anthropic
and otherwise calls openai.HasOpenAIPassthroughUsage, and similarly have Observe
call anthropicUsage.ObserveEvent / openai.ExtractOpenAIPassthroughUsage based on
path-first dispatch.
In `@core/providers/openai/openai.go`:
- Around line 7068-7084: The code currently always forwards req.Body into
providerUtils.StreamPassthrough via PassthroughStreamParams.RawRequest and
CancellationBody; gate these fields behind the raw-request "send-back" flag (the
config flag controlling whether raw requests are returned). In the
StreamPassthrough call (and when constructing
providerUtils.PassthroughJSONBody), check the send-back flag and only set
RawRequest and CancellationBody when the flag is true; otherwise set them to
nil/empty so request payloads are not forwarded. Keep all other params
(StatusCode, Headers, HasUsage, Observe, etc.) unchanged.
In `@core/providers/openai/passthrough_usage.go`:
- Around line 247-268: This code only attempts JSON unmarshalling
(sonic.Unmarshal into OpenAIImageGenerationRequest) and therefore ignores
multipart/form-data fields used by image endpoints; update the logic to first
detect and parse multipart form values (e.g., check request.Form / multipart
form parts) for fields "n", "size", and "quality" and populate u.ImageSize,
u.ImageQuality and u.ImageUsage.OutputTokensDetails.NImages accordingly before
falling back to the JSON path that uses sonic.Unmarshal and
OpenAIImageGenerationRequest; ensure you still create u.ImageUsage and
u.ImageUsage.OutputTokensDetails when setting NImages, and only use the JSON
branch if no multipart values are present.
- Around line 46-47: The video branch must consider the HTTP method so only
billable generation routes are handled: change the dispatch condition in the
switch to include the request method (e.g., check req.Method == "POST" along
with strings.Contains(path, "/video")), and update the call to
extractOAIVideoUsage to pass the method (add a method parameter to
extractOAIVideoUsage). Then modify extractOAIVideoUsage to return zero/empty
usage for non-billable methods and only compute VideoSeconds for POST generation
endpoints (avoid defaulting to VideoSeconds=4 for all /videos routes). Ensure
you update the function signature and all call sites to use the new method
parameter (reference: extractOAIVideoUsage).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c22ebb38-062c-4c12-a351-8f107d390ae8
📒 Files selected for processing (23)
core/bifrost.gocore/providers/anthropic/anthropic.gocore/providers/anthropic/passthrough_usage.gocore/providers/anthropic/passthrough_usage_test.gocore/providers/azure/azure.gocore/providers/azure/passthrough_usage_test.gocore/providers/gemini/gemini.gocore/providers/gemini/passthrough_usage.gocore/providers/gemini/passthrough_usage_test.gocore/providers/openai/openai.gocore/providers/openai/passthrough_usage.gocore/providers/openai/passthrough_usage_test.gocore/providers/utils/passthrough_stream.gocore/providers/utils/utils.gocore/providers/vertex/vertex.gocore/schemas/bifrost.gocore/schemas/passthrough.goframework/modelcatalog/pricing.goframework/streaming/passthrough.goframework/streaming/types.goplugins/governance/main.goplugins/logging/main.goplugins/logging/operations.go
💤 Files with no reviewable changes (1)
- core/providers/utils/utils.go
07c7575 to
1c23068
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 (1)
core/providers/openai/openai.go (1)
6938-6941:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRoute passthrough URLs through the OpenAI URL helper.
Both passthrough paths still concatenate
BaseURL + "/v1" + pathdirectly. That bypasses the OpenAI normalization used for delegated/ChatGPTOAuth providers, so these requests can hit the wrong upstream route even though the new usage extraction is wired correctly.🔧 Minimal fix
- url := provider.networkConfig.BaseURL + "/v1" + path + url := provider.buildFullURL("/v1" + path)Based on learnings "In core/providers/openai, when ChatGPTOAuth is enabled, ensure all OpenAI request routes to any
/v1/...path build their URLs usingOpenAIProvider.buildRequestURL(...)orOpenAIProvider.buildFullURL(...)... For Passthrough and PassthroughStream specifically, callbuildFullURL("/v1"+path)so the same normalization logic runs."Also applies to: 7008-7011
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/openai/openai.go` around lines 6938 - 6941, The passthrough handlers are building target URLs by concatenating provider.networkConfig.BaseURL + "/v1" + path, which bypasses the OpenAI normalization used by delegated/ChatGPTOAuth; replace those concatenations in the Passthrough and PassthroughStream call sites with the OpenAI provider URL helper (e.g., call OpenAIProvider.buildFullURL("/v1"+path) or buildRequestURL("/v1"+path) instead of manual string concat) so delegated providers are normalized correctly—update both occurrences (the block around the current BaseURL + "/v1" + path and the similar one at ~7008-7011) to use the buildFullURL/buildRequestURL helper on the OpenAI provider instance.
♻️ Duplicate comments (4)
core/providers/vertex/vertex.go (2)
3047-3048:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard passthrough raw request fields behind raw-request opt-in.
RawRequest(and streaming cancellation payload built fromreq.Body) is attached unconditionally, which can leak prompt/tool payloads even when raw echo is disabled.Suggested fix
+sendRawRequest := providerUtils.ShouldSendBackRawRequest(ctx, provider.sendBackRawRequest) +var rawRequest []byte +if sendRawRequest && len(req.Body) > 0 { + rawRequest = append([]byte(nil), req.Body...) +} ... ExtraFields: schemas.BifrostResponseExtraFields{ Latency: latency.Milliseconds(), ProviderResponseHeaders: headers, PassthroughPath: req.Path, - RawRequest: req.Body, + RawRequest: rawRequest, }, ... providerUtils.PassthroughStreamParams{ StatusCode: resp.StatusCode(), Headers: headers, Path: req.Path, - RawRequest: req.Body, - CancellationBody: providerUtils.PassthroughJSONBody(fasthttpReq, req.Body), + RawRequest: rawRequest, + CancellationBody: providerUtils.PassthroughJSONBody(fasthttpReq, rawRequest),As per coding guidelines, "Apply Go security practices: do not log secrets or sensitive request/response bodies by default".
Also applies to: 3197-3199
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/vertex/vertex.go` around lines 3047 - 3048, The code currently always attaches RawRequest and reads req.Body to build streaming cancellation payload (around the struct fields PassthroughPath and RawRequest), which can leak sensitive data; change it to only set RawRequest and to read req.Body when a clear opt-in flag is enabled (e.g., opts.EchoRawRequest or a function allowRawRequestEcho()), leaving PassthroughPath unchanged, and ensure the cancellation-payload construction that consumes req.Body is similarly guarded; apply the same conditional guard to the other occurrence that sets RawRequest/reads req.Body so req.Body is never read or stored unless the explicit raw-request opt-in is true.
3035-3038:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRoute passthrough usage extraction by routed API family, not Gemini-only.
Both unary and streaming paths currently use Gemini extractors for all Vertex passthrough routes. Anthropic publisher routes and OpenAI-compatible
openapi/...routes will miss usage extraction, causing passthrough cost/budget/logging undercount.Suggested fix
+func extractVertexPassthroughUsage(path string, reqBody, body []byte) *schemas.BifrostPassthroughUsage { + p := strings.ToLower(path) + switch { + case strings.Contains(p, "publishers/anthropic") || strings.HasPrefix(p, "/v1/messages") || strings.HasPrefix(p, "/v1/complete"): + return anthropic.ExtractAnthropicPassthroughUsage(path, reqBody, body) + case strings.Contains(p, "/openapi/") || strings.Contains(p, "/chat/completions") || strings.Contains(p, "/responses"): + return openai.ExtractOpenAIPassthroughUsage(path, reqBody, body) + default: + return gemini.ExtractGeminiPassthroughUsage(path, reqBody, body) + } +} ... - passthroughUsage = gemini.ExtractGeminiPassthroughUsage(req.Path, req.Body, body) + passthroughUsage = extractVertexPassthroughUsage(req.Path, req.Body, body) ... - HasUsage: gemini.HasGeminiPassthroughUsage, + HasUsage: func(event []byte) bool { + p := strings.ToLower(req.Path) + if strings.Contains(p, "publishers/anthropic") || strings.HasPrefix(p, "/v1/messages") || strings.HasPrefix(p, "/v1/complete") { + return anthropic.HasAnthropicPassthroughUsage(event) + } + if strings.Contains(p, "/openapi/") || strings.Contains(p, "/chat/completions") || strings.Contains(p, "/responses") { + return openai.HasOpenAIPassthroughUsage(event) + } + return gemini.HasGeminiPassthroughUsage(event) + }, Observe: func(event []byte) *schemas.BifrostPassthroughUsage { - return gemini.ExtractGeminiPassthroughUsage(req.Path, req.Body, event) + return extractVertexPassthroughUsage(req.Path, req.Body, event) },As per coding guidelines, "For provider changes, verify converters remain pure, OpenAI helper changes account for delegated providers, streaming paths use the streaming client, fasthttp requests/responses are acquired and released correctly, and response/error metadata is preserved."
Also applies to: 3202-3205
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/vertex/vertex.go` around lines 3035 - 3038, The code currently always calls gemini.ExtractGeminiPassthroughUsage (assigning passthroughUsage) which misses Anthropic and OpenAI-compatible passthroughs; change the extraction to dispatch based on the routed API family or request path (e.g., inspect req.RoutedAPIFamily or req.Path) and call the appropriate extractor (e.g., gemini.ExtractGeminiPassthroughUsage, anthropic.ExtractAnthropicPassthroughUsage, openai.ExtractOpenAIPassthroughUsage or similar functions), making the same change in both the unary path (where passthroughUsage is assigned) and the streaming path counterpart so all passthrough routes get correct usage extraction and downstream logging/billing remains accurate.core/bifrost.go (1)
6451-6454:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTreat nil passthrough payload as an error, not a successful response.
This avoids the panic, but
(passthroughResponse == nil, bifrostError == nil)still propagates as success here, which can skip fallback/error flow and mark post-hooks as successful incorrectly. Return aBifrostErrorimmediately when passthrough response is nil.Suggested fix
case schemas.PassthroughRequest: passthroughResponse, bifrostError := provider.Passthrough(req.Context, key, req.BifrostRequest.PassthroughRequest) if bifrostError != nil { return nil, bifrostError } - if passthroughResponse != nil { - passthroughResponse.Path = req.BifrostRequest.PassthroughRequest.Path - } + if passthroughResponse == nil { + return nil, &schemas.BifrostError{ + IsBifrostError: false, + Error: &schemas.ErrorField{ + Message: "provider returned nil passthrough response", + }, + ExtraFields: schemas.BifrostErrorExtraFields{ + RequestType: req.RequestType, + Provider: provider.GetProviderKey(), + OriginalModelRequested: req.BifrostRequest.PassthroughRequest.Model, + ResolvedModelUsed: req.BifrostRequest.PassthroughRequest.Model, + }, + } + } + passthroughResponse.Path = req.BifrostRequest.PassthroughRequest.Path response.PassthroughResponse = passthroughResponseAs per coding guidelines,
core/**changes should maintain explicit error handling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/bifrost.go` around lines 6451 - 6454, The code currently treats a nil passthroughResponse as success; update the handling around passthroughResponse (the block that sets passthroughResponse.Path and response.PassthroughResponse) to detect passthroughResponse == nil and immediately return or populate a BifrostError (e.g., construct a BifrostError with a clear message and assign it to the error return/bifrostError variable) instead of proceeding; ensure you do not set response.PassthroughResponse when nil and that callers observing (passthroughResponse == nil, bifrostError == nil) no longer occur so fallback/error flow and post-hook status are correctly triggered.core/providers/openai/passthrough_usage.go (1)
46-50:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTighten video billing route matching to known billable POST endpoints.
Line 46 currently matches any POST path containing
"/video", so non-billable/unknown POST paths can still get defaultVideoSecondsand be costed. Restrict this to explicit billable create/remix/edit/extend routes before callingextractOAIVideoUsage.Proposed fix
@@ - case strings.Contains(path, "/video"): - if strings.EqualFold(method, "POST") { + case strings.EqualFold(method, "POST") && isBillableOAIVideoPath(path): return extractOAIVideoUsage(reqBody) - } - return nil + case strings.Contains(path, "/video"): + return nil @@ } + +func isBillableOAIVideoPath(path string) bool { + return path == "/v1/videos" || + strings.HasSuffix(path, "/videos/edits") || + strings.HasSuffix(path, "/videos/extensions") || + strings.HasSuffix(path, "/remix") +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/openai/passthrough_usage.go` around lines 46 - 50, The current route-matching block that calls extractOAIVideoUsage when strings.Contains(path, "/video") and method is POST is too broad; change it to only call extractOAIVideoUsage for explicit billable POST endpoints (e.g., paths that match known create/remix/edit/extend patterns) by replacing the generic contains check with a whitelist of allowed video POST routes (check path against exact or regex patterns for create/remix/edit/extend) and only invoke extractOAIVideoUsage when the path matches one of those patterns; leave other POST /video requests returning nil.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/providers/anthropic/anthropic.go`:
- Around line 2615-2617: The passthrough usage extraction calls are using
req.Path verbatim instead of the normalized path, so query params in Path can
prevent correct endpoint matching; update both extraction call sites (the
non-streaming PassthroughUsage assignment that calls
ExtractAnthropicPassthroughUsage and the streaming counterpart around lines
2696-2720) to pass the normalized strippedPath variable instead of req.Path (and
ensure strippedPath is computed before those calls), so both streaming and
non-streaming requests use the normalized path for matching/logging.
In `@core/providers/azure/azure.go`:
- Around line 3705-3709: extractAzurePassthroughUsage currently chooses
extractor only by schemas.IsAnthropicModel(model), which can misroute delegated
Anthropic passthroughs; update extractAzurePassthroughUsage to be route-aware:
if the incoming path/method match Anthropic passthrough routes (inspect path and
method patterns used elsewhere for Anthropic routing) call
anthropic.ExtractAnthropicPassthroughUsage, otherwise fall back to model-based
detection and then to openai.ExtractOpenAIPassthroughUsage; keep references to
schemas.IsAnthropicModel, anthropic.ExtractAnthropicPassthroughUsage, and
openai.ExtractOpenAIPassthroughUsage so the function prefers explicit Anthropic
routes before relying on model heuristics.
In `@core/schemas/passthrough.go`:
- Line 48: The comment for the PassthroughUsage field is stale: update the field
comment on PassthroughUsage (type BifrostPassthroughUsage) to remove the claim
that it is nil for non-streaming; instead state that it is populated by the
provider on final streaming chunk and may also be set for non-streaming
passthrough implementations (e.g., providers like the Azure provider set it for
non-streaming), so downstream implementers/tests should not assume nil for
non-streaming.
---
Outside diff comments:
In `@core/providers/openai/openai.go`:
- Around line 6938-6941: The passthrough handlers are building target URLs by
concatenating provider.networkConfig.BaseURL + "/v1" + path, which bypasses the
OpenAI normalization used by delegated/ChatGPTOAuth; replace those
concatenations in the Passthrough and PassthroughStream call sites with the
OpenAI provider URL helper (e.g., call OpenAIProvider.buildFullURL("/v1"+path)
or buildRequestURL("/v1"+path) instead of manual string concat) so delegated
providers are normalized correctly—update both occurrences (the block around the
current BaseURL + "/v1" + path and the similar one at ~7008-7011) to use the
buildFullURL/buildRequestURL helper on the OpenAI provider instance.
---
Duplicate comments:
In `@core/bifrost.go`:
- Around line 6451-6454: The code currently treats a nil passthroughResponse as
success; update the handling around passthroughResponse (the block that sets
passthroughResponse.Path and response.PassthroughResponse) to detect
passthroughResponse == nil and immediately return or populate a BifrostError
(e.g., construct a BifrostError with a clear message and assign it to the error
return/bifrostError variable) instead of proceeding; ensure you do not set
response.PassthroughResponse when nil and that callers observing
(passthroughResponse == nil, bifrostError == nil) no longer occur so
fallback/error flow and post-hook status are correctly triggered.
In `@core/providers/openai/passthrough_usage.go`:
- Around line 46-50: The current route-matching block that calls
extractOAIVideoUsage when strings.Contains(path, "/video") and method is POST is
too broad; change it to only call extractOAIVideoUsage for explicit billable
POST endpoints (e.g., paths that match known create/remix/edit/extend patterns)
by replacing the generic contains check with a whitelist of allowed video POST
routes (check path against exact or regex patterns for create/remix/edit/extend)
and only invoke extractOAIVideoUsage when the path matches one of those
patterns; leave other POST /video requests returning nil.
In `@core/providers/vertex/vertex.go`:
- Around line 3047-3048: The code currently always attaches RawRequest and reads
req.Body to build streaming cancellation payload (around the struct fields
PassthroughPath and RawRequest), which can leak sensitive data; change it to
only set RawRequest and to read req.Body when a clear opt-in flag is enabled
(e.g., opts.EchoRawRequest or a function allowRawRequestEcho()), leaving
PassthroughPath unchanged, and ensure the cancellation-payload construction that
consumes req.Body is similarly guarded; apply the same conditional guard to the
other occurrence that sets RawRequest/reads req.Body so req.Body is never read
or stored unless the explicit raw-request opt-in is true.
- Around line 3035-3038: The code currently always calls
gemini.ExtractGeminiPassthroughUsage (assigning passthroughUsage) which misses
Anthropic and OpenAI-compatible passthroughs; change the extraction to dispatch
based on the routed API family or request path (e.g., inspect
req.RoutedAPIFamily or req.Path) and call the appropriate extractor (e.g.,
gemini.ExtractGeminiPassthroughUsage,
anthropic.ExtractAnthropicPassthroughUsage, openai.ExtractOpenAIPassthroughUsage
or similar functions), making the same change in both the unary path (where
passthroughUsage is assigned) and the streaming path counterpart so all
passthrough routes get correct usage extraction and downstream logging/billing
remains accurate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7ae563c6-9bf2-480a-9f41-c946b72962e1
📒 Files selected for processing (23)
core/bifrost.gocore/providers/anthropic/anthropic.gocore/providers/anthropic/passthrough_usage.gocore/providers/anthropic/passthrough_usage_test.gocore/providers/azure/azure.gocore/providers/azure/passthrough_usage_test.gocore/providers/gemini/gemini.gocore/providers/gemini/passthrough_usage.gocore/providers/gemini/passthrough_usage_test.gocore/providers/openai/openai.gocore/providers/openai/passthrough_usage.gocore/providers/openai/passthrough_usage_test.gocore/providers/utils/passthrough_stream.gocore/providers/utils/utils.gocore/providers/vertex/vertex.gocore/schemas/bifrost.gocore/schemas/passthrough.goframework/modelcatalog/pricing.goframework/streaming/passthrough.goframework/streaming/types.goplugins/governance/main.goplugins/logging/main.goplugins/logging/operations.go
💤 Files with no reviewable changes (1)
- core/providers/utils/utils.go
1c23068 to
c17a81c
Compare
c17a81c to
fd22dae
Compare
Merge activity
|
## Summary Passthrough requests previously had no usage extraction, meaning cost calculation and token logging were silently skipped for all provider passthrough endpoints. This PR adds per-provider usage extraction for both streaming and non-streaming passthrough responses across OpenAI, Azure, Anthropic, and Gemini, and wires the extracted usage into the pricing, logging, and governance plugins. ## Changes - **New `BifrostPassthroughUsage` schema** added to `schemas/passthrough.go` carrying LLM tokens, image counts, audio chars/seconds, video seconds, and container identifiers — covering every billable endpoint type. - **`PassthroughPath` field** added to `BifrostResponseExtraFields` and `BifrostPassthroughResponse` so the path is available downstream without re-parsing the original request. - **Provider-level usage extractors** introduced as new files: - `core/providers/openai/passthrough_usage.go` — handles chat/completions, responses API, embeddings, speech (TTS), transcription/translation, image generation/edit/variation, video generation, and container creation. - `core/providers/anthropic/passthrough_usage.go` — handles `/messages` (SSE and non-streaming) and legacy `/complete`, including cache token details. - `core/providers/gemini/passthrough_usage.go` — handles `:generateContent`/`:streamGenerateContent` (text, audio, image output modalities), embeddings, Imagen (`:predict`), Veo (`:predictLongRunning`), and the Interactions API. - **Streaming accumulation** updated across all four providers to accumulate the full response body (`accBody`) and call the usage extractor on the final EOF chunk, attaching `PassthroughUsage` to the terminal response. - **`core/providers/utils/passthrough.go`** added with shared SSE parsing helpers (`ScanSSEDataLines`, `LastSSEDataLine`, `LastSSEOrBody`) used by all extractors. - **Pricing integration** (`framework/modelcatalog/pricing.go`): `extractCostInput` now checks `PassthroughResponse.PassthroughUsage` first; `inferPassthroughRequestType` maps usage fields and path to the correct `RequestType`; `passthroughUsageToCostInput` converts the usage struct into the existing `costInput` shape so all existing compute functions apply without modification. - **Logging plugin** (`plugins/logging/main.go`, `operations.go`): passthrough token usage is now applied to log entries via `applyNonStreamingOutputToEntry`, and streaming passthrough cost is computed in `PostLLMHook` when `PassthroughUsage` is present. The `Model` field is now forwarded in `PassthroughLogParams`. - **Governance plugin** (`plugins/governance/main.go`): token usage is read from `PassthroughUsage.LLMUsage` for passthrough responses; `HasUsageData` now also triggers when `cost > 0` so non-token-based billing (images, audio, video) is tracked correctly. - **`content-type` removed** from the provider response header filter list so it is forwarded to callers. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/... ./framework/... ./plugins/... ``` To validate end-to-end: 1. Send a passthrough request to `/v1/chat/completions`, `/v1/images/generations`, `/v1/audio/speech`, and a streaming `/v1/responses` endpoint via each supported provider. 2. Confirm that the log entry for each request contains a non-zero `cost` and populated `token_usage_parsed` (or the appropriate usage field for non-token endpoints). 3. For streaming passthrough, confirm that the final accumulated response includes `PassthroughUsage` and that cost appears in the governance usage tracker. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations No new auth surfaces. The `content-type` header is now forwarded from providers to callers, which was previously suppressed — callers should be aware the response content type now reflects the provider's actual content type. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Enhanced passthrough tracking: upstream request path is surfaced and detailed usage metrics (tokens, images, audio, video, container identifiers) are captured and returned for passthrough requests. * Streaming passthroughs now reliably forward raw chunks, observe incremental usage, and emit final usage on completion. * **Improvements** * Pricing and logging now use passthrough usage to improve cost calculation and reporting. * **Tests** * Added comprehensive tests for passthrough usage extraction and streaming across providers. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Passthrough requests previously had no usage extraction, meaning cost calculation and token logging were silently skipped for all provider passthrough endpoints. This PR adds per-provider usage extraction for both streaming and non-streaming passthrough responses across OpenAI, Azure, Anthropic, and Gemini, and wires the extracted usage into the pricing, logging, and governance plugins. ## Changes - **New `BifrostPassthroughUsage` schema** added to `schemas/passthrough.go` carrying LLM tokens, image counts, audio chars/seconds, video seconds, and container identifiers — covering every billable endpoint type. - **`PassthroughPath` field** added to `BifrostResponseExtraFields` and `BifrostPassthroughResponse` so the path is available downstream without re-parsing the original request. - **Provider-level usage extractors** introduced as new files: - `core/providers/openai/passthrough_usage.go` — handles chat/completions, responses API, embeddings, speech (TTS), transcription/translation, image generation/edit/variation, video generation, and container creation. - `core/providers/anthropic/passthrough_usage.go` — handles `/messages` (SSE and non-streaming) and legacy `/complete`, including cache token details. - `core/providers/gemini/passthrough_usage.go` — handles `:generateContent`/`:streamGenerateContent` (text, audio, image output modalities), embeddings, Imagen (`:predict`), Veo (`:predictLongRunning`), and the Interactions API. - **Streaming accumulation** updated across all four providers to accumulate the full response body (`accBody`) and call the usage extractor on the final EOF chunk, attaching `PassthroughUsage` to the terminal response. - **`core/providers/utils/passthrough.go`** added with shared SSE parsing helpers (`ScanSSEDataLines`, `LastSSEDataLine`, `LastSSEOrBody`) used by all extractors. - **Pricing integration** (`framework/modelcatalog/pricing.go`): `extractCostInput` now checks `PassthroughResponse.PassthroughUsage` first; `inferPassthroughRequestType` maps usage fields and path to the correct `RequestType`; `passthroughUsageToCostInput` converts the usage struct into the existing `costInput` shape so all existing compute functions apply without modification. - **Logging plugin** (`plugins/logging/main.go`, `operations.go`): passthrough token usage is now applied to log entries via `applyNonStreamingOutputToEntry`, and streaming passthrough cost is computed in `PostLLMHook` when `PassthroughUsage` is present. The `Model` field is now forwarded in `PassthroughLogParams`. - **Governance plugin** (`plugins/governance/main.go`): token usage is read from `PassthroughUsage.LLMUsage` for passthrough responses; `HasUsageData` now also triggers when `cost > 0` so non-token-based billing (images, audio, video) is tracked correctly. - **`content-type` removed** from the provider response header filter list so it is forwarded to callers. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/... ./framework/... ./plugins/... ``` To validate end-to-end: 1. Send a passthrough request to `/v1/chat/completions`, `/v1/images/generations`, `/v1/audio/speech`, and a streaming `/v1/responses` endpoint via each supported provider. 2. Confirm that the log entry for each request contains a non-zero `cost` and populated `token_usage_parsed` (or the appropriate usage field for non-token endpoints). 3. For streaming passthrough, confirm that the final accumulated response includes `PassthroughUsage` and that cost appears in the governance usage tracker. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations No new auth surfaces. The `content-type` header is now forwarded from providers to callers, which was previously suppressed — callers should be aware the response content type now reflects the provider's actual content type. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Enhanced passthrough tracking: upstream request path is surfaced and detailed usage metrics (tokens, images, audio, video, container identifiers) are captured and returned for passthrough requests. * Streaming passthroughs now reliably forward raw chunks, observe incremental usage, and emit final usage on completion. * **Improvements** * Pricing and logging now use passthrough usage to improve cost calculation and reporting. * **Tests** * Added comprehensive tests for passthrough usage extraction and streaming across providers. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary This PR bumps the Go toolchain version from `1.26.3` to `1.26.4` across all modules and CI workflows, and cuts a new release (`core` v1.5.17, `framework` v1.3.17, `transports` v1.5.9, `plugins/compat` v0.1.16, `plugins/governance` v1.5.17, and associated plugin versions) incorporating a large batch of features and fixes accumulated since the previous release. ## Changes - **Go 1.26.4** — Updated `go-version` in all GitHub Actions workflows (`e2e-tests`, `helm-release`, `pr-tests`, `release-cli`, `release-pipeline`, `snyk`) and all `go.mod` files (core, framework, transports, cli, all plugins, examples, and test modules). - **Core (v1.5.17)** — OpenAI compaction support, multi-customer logs and usage tracking, multiple team/business unit support, `request_headers` wildcard pattern capture for OTel and Maxim plugins, xAI `x_search` tool, fetch URL validation with SSRF hardening, `file://` pricing URL scheme, virtual key provider fan-out filtering, and a broad set of fixes including Anthropic prompt cache key, empty thinking block stripping, OpenAI stream usage event cleanup, Gemini numeric schema constraints, stale connection retries, Azure Claude diagnostic strip, and passthrough budget handling. - **Framework (v1.3.17)** — Scope-aware budgets and limits wired from model configs, provider-level governance, multiple customer budget support with `calendar_aligned` windows, paginated virtual key fetch, `config.json` source-of-truth flow, FTS index cap reduction, sync worker drift fix, cascade deletes for model configs, and high-scale virtual key flow improvements. - **Transports (v1.5.9)** — Full changelog covering all of the above plus UI improvements (log navigation, customer detail sheet, `BudgetDisplay` component, inline loading shell, materialized view alias), SCIM provisioning fields, Helm/config schema additions (`roles`, `per_user_oauth`), client IP resolution from forwarded headers, and dependency upgrades (`recharts` to 3.8.1, `golang.org/x` CVE remediation). - **Plugins** — `governance` v1.5.17 adds team budget/rate-limit exporters, ghost node reconciliation fix, and VK double usage counting fix; `logging` v1.5.17 adds wildcard header capture and file attachment rendering; `otel` v1.2.17 adds `disable_content_logging` and multiple collectors support; `maxim` v1.6.17 adds `request_headers` wildcard capture; `compat` v0.1.16 fixes `max_tokens` preservation during param filtering. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Verify Go version go version # should report go1.26.4 # Run core tests cd core && go test ./... # Run framework tests cd framework && go test ./... # Run transports tests cd transports && go test ./... # Run plugin tests cd plugins/governance && go test ./... cd plugins/logging && go test ./... cd plugins/otel && go test ./... # UI cd ui pnpm i pnpm build pnpm test ``` ## Breaking changes - [ ] Yes - [x] No ## Related issues #4053, #4066, #4041, #4012, #3976, #3947, #3991, #4045, #3957, #3938, #3937, #3939, #3981, #3998, #3997, #4092, #4091, #4079, #4080, #4086, #3929, #3994, #4028, #3970, #3919, #3861, #3664, #3999, #4088, #4070, #4051, #4043, #4057, #4023, #3941, #3955, #4024, #3956, #3967, #3925, #3992, #3900 ## Security considerations - Fetch URL validation hardened against SSRF by tightening IP checks for private networks and link-local addresses (#4092, #3947, #3991). - Transitive `golang.org/x` dependencies (crypto, net, sys, text) bumped to address Docker Scout CVEs (#3900). ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * OpenAI compaction, multi-customer/team logstore support, request-header wildcard capture, enhanced governance (provider-level & scope-aware limits), disable-content-logging option, support for multiple OpenTelemetry collectors, SSRF hardening and URL validation. * **Chores** * Bumped Go toolchain across modules and updated component/plugin version releases. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Passthrough requests previously had no usage extraction, meaning cost calculation and token logging were silently skipped for all provider passthrough endpoints. This PR adds per-provider usage extraction for both streaming and non-streaming passthrough responses across OpenAI, Azure, Anthropic, and Gemini, and wires the extracted usage into the pricing, logging, and governance plugins. ## Changes - **New `BifrostPassthroughUsage` schema** added to `schemas/passthrough.go` carrying LLM tokens, image counts, audio chars/seconds, video seconds, and container identifiers — covering every billable endpoint type. - **`PassthroughPath` field** added to `BifrostResponseExtraFields` and `BifrostPassthroughResponse` so the path is available downstream without re-parsing the original request. - **Provider-level usage extractors** introduced as new files: - `core/providers/openai/passthrough_usage.go` — handles chat/completions, responses API, embeddings, speech (TTS), transcription/translation, image generation/edit/variation, video generation, and container creation. - `core/providers/anthropic/passthrough_usage.go` — handles `/messages` (SSE and non-streaming) and legacy `/complete`, including cache token details. - `core/providers/gemini/passthrough_usage.go` — handles `:generateContent`/`:streamGenerateContent` (text, audio, image output modalities), embeddings, Imagen (`:predict`), Veo (`:predictLongRunning`), and the Interactions API. - **Streaming accumulation** updated across all four providers to accumulate the full response body (`accBody`) and call the usage extractor on the final EOF chunk, attaching `PassthroughUsage` to the terminal response. - **`core/providers/utils/passthrough.go`** added with shared SSE parsing helpers (`ScanSSEDataLines`, `LastSSEDataLine`, `LastSSEOrBody`) used by all extractors. - **Pricing integration** (`framework/modelcatalog/pricing.go`): `extractCostInput` now checks `PassthroughResponse.PassthroughUsage` first; `inferPassthroughRequestType` maps usage fields and path to the correct `RequestType`; `passthroughUsageToCostInput` converts the usage struct into the existing `costInput` shape so all existing compute functions apply without modification. - **Logging plugin** (`plugins/logging/main.go`, `operations.go`): passthrough token usage is now applied to log entries via `applyNonStreamingOutputToEntry`, and streaming passthrough cost is computed in `PostLLMHook` when `PassthroughUsage` is present. The `Model` field is now forwarded in `PassthroughLogParams`. - **Governance plugin** (`plugins/governance/main.go`): token usage is read from `PassthroughUsage.LLMUsage` for passthrough responses; `HasUsageData` now also triggers when `cost > 0` so non-token-based billing (images, audio, video) is tracked correctly. - **`content-type` removed** from the provider response header filter list so it is forwarded to callers. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/... ./framework/... ./plugins/... ``` To validate end-to-end: 1. Send a passthrough request to `/v1/chat/completions`, `/v1/images/generations`, `/v1/audio/speech`, and a streaming `/v1/responses` endpoint via each supported provider. 2. Confirm that the log entry for each request contains a non-zero `cost` and populated `token_usage_parsed` (or the appropriate usage field for non-token endpoints). 3. For streaming passthrough, confirm that the final accumulated response includes `PassthroughUsage` and that cost appears in the governance usage tracker. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations No new auth surfaces. The `content-type` header is now forwarded from providers to callers, which was previously suppressed — callers should be aware the response content type now reflects the provider's actual content type. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Enhanced passthrough tracking: upstream request path is surfaced and detailed usage metrics (tokens, images, audio, video, container identifiers) are captured and returned for passthrough requests. * Streaming passthroughs now reliably forward raw chunks, observe incremental usage, and emit final usage on completion. * **Improvements** * Pricing and logging now use passthrough usage to improve cost calculation and reporting. * **Tests** * Added comprehensive tests for passthrough usage extraction and streaming across providers. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## ✨ Features - **OpenAI Compaction** — Added OpenAI conversation compaction support across core, framework, logging, and the API surface (#4053) - **Multi-Customer & Org Hierarchy** — Logs and usage tracking now support multiple customers, teams, and business units, including business unit CRUD, team assignment, and governance endpoints in the OpenAPI spec (#4066, #4041, #4082) - **Provider-Level Governance** — Budgets & limits are now scope-aware and can be applied at the virtual-key top level and per provider, wired from the model configs table, with UI filters for scope and providers (#3938, #3937, #3939, #3981, #3962) - **Customer Budgets** — Customers support multiple budgets and `calendar_aligned` budget windows (#3998, #3997) - **Virtual Key Attribution & Controls** — Added a `created_by` user attribution column and a `blacklisted_models` column for virtual key provider configs (#3672, #3653) - **Request Header Capture** — OTel and Maxim observability plugins capture `request_headers` by pattern, with wildcard support (e.g. `x-custom-*`); logging gained the same wildcard header capture (#4012, #3958) - **OTel Content Controls & Collectors** — New `disable_content_logging` option drops message/tool content from exported spans, plus support for multiple OTel collectors (#4064, #3894) - **xAI x_search** — Added xAI `x_search` tool support (#3976) - **URL Validation** — Added fetch URL validation with private-network configuration and link-local blocking (#3947, #3991) - **File Scheme Pricing URLs** — Pricing source URLs now accept the `file://` scheme for air-gapped and self-hosted deployments (#4045) - **Paginated Virtual Keys** — Virtual key fetching is paginated to handle deployments with very large numbers of keys (#3957) - **Client IP Resolution** — Resolve client IP from `X-Forwarded-For`/`X-Real-IP` headers - **SCIM Provisioning** — Added `attributeType`/`attributeValue` SCIM provisioning fields - **Helm/Config Schema** — Added `roles` RBAC governance config and `per_user_oauth` MCP auth to the Helm chart and config schema (#4004, #4009) - **Log Navigation UI** — Added a "View logs" menu item to customer, team, and virtual key tables, clickable links in log detail views, a customer detail sheet, and a reusable `BudgetDisplay` component (#4073, #4054, #4026, #4055) - **Faster First Paint** — Added an inline loading shell to `#root` before React mounts (#4063) - **Materialized View Alias** — Added an `alias` column to the materialized view with filter support (#4078) ## 🐞 Fixed - **Fetch URL IP Checks** — Hardened fetch URL IP checks against SSRF (#4092) - **Mantle Model Matching** — Broadened Mantle model matching to all `gpt` variants (#4091) - **Empty Thinking Blocks** — Strip thinking blocks when the signature is empty (#4079) - **OpenAI Stream Usage** — Removed usage from the `responses.created` event in the OpenAI stream (#4080) - **Prompt Cache Key** — Set the prompt cache key from the Anthropic integration (#4086) - **Upstream Failure Status** — Map upstream connection failures to 502 instead of 400 (#3929) (thanks [@chris-colinsky](https://github.com/chris-colinsky)!) - **Gemini Schema Constraints** — Accept numeric schema integer constraints for Gemini (#3994) (thanks [@yanhao98](https://github.com/yanhao98)!) - **Files Provider Param** — Accept the `?provider=` query param on `GET /v1/files` (#3971) (thanks [@alexef](https://github.com/alexef)!) - **Optional Batch Model** — Made the `model` field optional on `POST /v1/batches` (#3973) (thanks [@alexef](https://github.com/alexef)!) - **Helm Azure Config** — Added missing `azure_key_config` fields to the Helm schema (#3996) (thanks [@axelray-dev](https://github.com/axelray-dev)!) - **Text Completion Chunk Model** — Added the missing `Model` field to `TextCompletionChunkResponse` (#3970) (thanks [@kuishou68](https://github.com/kuishou68)!) - **MCP Inline stdio Env** — MCP stdio server configs accept inline environment variable assignments (#3861) (thanks [@Shushmitaaaa](https://github.com/Shushmitaaaa)!) - **Orphaned Tool Results** — Orphaned tool results in the OpenAI to Anthropic conversion flow are no longer rejected by the Anthropic API (#3919) - **Node Usage Reconciliation** — Added a monotonic `inc_number` log cursor so node usage reconciliation does not skip late async log writes (#3664) - **Bedrock Output Assessments** — Corrected the type of `outputAssessments` in Bedrock responses (#4028) - **Model Pool Pricing Reloads** — Preserve non-pricing model pool entries across pricing reloads (#3999) - **Ghost Node Reconciliation** — Replicate the VK hierarchy flow for ghost node reconciliation (#4088) - **VK Double Usage Counting** — Fixed double usage counting when creating a virtual key (#4070) - **Model Config Lifecycle** — Cascade deletes for model configs and removal of stale in-memory model configs (#4051, #4043) - **FTS Index Cap** — Reduced the FTS index `left()` cap from 800k to 250k chars to stay within the tsvector limit (#4057) - **Sync Worker Drift** — Reduced the sync worker ticker period to 5m to prevent threshold drift (#4023) - **Passthrough** — Fixed passthrough budgets, gated passthrough models per VK, model extraction for Azure passthrough, and restricted fallbacks/provider selection to the VK boundary (#3941, #3988, #3983, #3924) - **Provider Response Headers** — Strip provider response headers and add a content-type filter (#3955, #4024) - **Stream Handling** — Drain non-SSE stream readers and retry stale connections (#3956, #3967) - **Azure Claude** — Strip Azure diagnostic property for Claude models (#3925) - **Compat max_tokens** — Preserve chat `max_tokens` during param filtering (#3992) - **Raw Request Flag** — Removed the raw request flag from providers that don't support it (#4058) - **UI Fixes** — Standardized page container layout, virtual key model configs UI, and dashboard chart tooltips (#4046, #4052, #4044) ## 🔧 Maintenance - **Dependency Upgrades** — Bumped transitive `golang.org/x` dependencies (crypto, net, sys, text) for Docker Scout CVE remediation and `recharts` to 3.8.1; cascaded version bumps across all modules (#3900, #4003)

Summary
Passthrough requests previously had no usage extraction, meaning cost calculation and token logging were silently skipped for all provider passthrough endpoints. This PR adds per-provider usage extraction for both streaming and non-streaming passthrough responses across OpenAI, Azure, Anthropic, and Gemini, and wires the extracted usage into the pricing, logging, and governance plugins.
Changes
BifrostPassthroughUsageschema added toschemas/passthrough.gocarrying LLM tokens, image counts, audio chars/seconds, video seconds, and container identifiers — covering every billable endpoint type.PassthroughPathfield added toBifrostResponseExtraFieldsandBifrostPassthroughResponseso the path is available downstream without re-parsing the original request.core/providers/openai/passthrough_usage.go— handles chat/completions, responses API, embeddings, speech (TTS), transcription/translation, image generation/edit/variation, video generation, and container creation.core/providers/anthropic/passthrough_usage.go— handles/messages(SSE and non-streaming) and legacy/complete, including cache token details.core/providers/gemini/passthrough_usage.go— handles:generateContent/:streamGenerateContent(text, audio, image output modalities), embeddings, Imagen (:predict), Veo (:predictLongRunning), and the Interactions API.accBody) and call the usage extractor on the final EOF chunk, attachingPassthroughUsageto the terminal response.core/providers/utils/passthrough.goadded with shared SSE parsing helpers (ScanSSEDataLines,LastSSEDataLine,LastSSEOrBody) used by all extractors.framework/modelcatalog/pricing.go):extractCostInputnow checksPassthroughResponse.PassthroughUsagefirst;inferPassthroughRequestTypemaps usage fields and path to the correctRequestType;passthroughUsageToCostInputconverts the usage struct into the existingcostInputshape so all existing compute functions apply without modification.plugins/logging/main.go,operations.go): passthrough token usage is now applied to log entries viaapplyNonStreamingOutputToEntry, and streaming passthrough cost is computed inPostLLMHookwhenPassthroughUsageis present. TheModelfield is now forwarded inPassthroughLogParams.plugins/governance/main.go): token usage is read fromPassthroughUsage.LLMUsagefor passthrough responses;HasUsageDatanow also triggers whencost > 0so non-token-based billing (images, audio, video) is tracked correctly.content-typeremoved from the provider response header filter list so it is forwarded to callers.Type of change
Affected areas
How to test
go test ./core/... ./framework/... ./plugins/...To validate end-to-end:
/v1/chat/completions,/v1/images/generations,/v1/audio/speech, and a streaming/v1/responsesendpoint via each supported provider.costand populatedtoken_usage_parsed(or the appropriate usage field for non-token endpoints).PassthroughUsageand that cost appears in the governance usage tracker.Breaking changes
Related issues
Security considerations
No new auth surfaces. The
content-typeheader is now forwarded from providers to callers, which was previously suppressed — callers should be aware the response content type now reflects the provider's actual content type.Checklist
docs/contributing/README.mdand followed the guidelinesSummary by CodeRabbit
New Features
Improvements
Tests