Skip to content

fix: billing on failed responses stream requests anthropic and bedrock - #4842

Merged
Pratham-Mishra04 merged 1 commit into
devfrom
07-02-fix_billing_on_failed_responses_stream_requests_anthropic_and_bedrock
Jul 2, 2026
Merged

fix: billing on failed responses stream requests anthropic and bedrock#4842
Pratham-Mishra04 merged 1 commit into
devfrom
07-02-fix_billing_on_failed_responses_stream_requests_anthropic_and_bedrock

Conversation

@TejasGhatte

@TejasGhatte TejasGhatte commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

When a streaming Responses API request is cancelled or times out mid-stream, usage tokens reported by intermediate events (e.g. message_start, message_delta for Anthropic, or early Bedrock Converse events) were being discarded. This meant callers could not be billed for provider-reported usage that arrived before the stream was interrupted. This PR fixes that by maintaining a mirrored BifrostLLMUsage struct that is updated in lockstep with the in-flight ResponsesResponseUsage and registered on the context so cancellation/timeout paths can retrieve and apply it.

Changes

  • Extracted accumulateAnthropicResponsesUsage and accumulateBedrockResponsesUsage helper functions that simultaneously update both the streaming ResponsesResponseUsage and a parallel BifrostLLMUsage (including all cache token detail fields: read, write, 5m, and 1h TTL buckets).
  • In both HandleAnthropicResponsesStream and BedrockProvider.ResponsesStream, a billedUsage pointer is now registered on the context via BifrostContextKeyStreamAccumulatedUsage at stream start, so any mid-stream interruption can access already-accumulated usage.
  • A deferred closure normalises cached token counts into billedUsage (via normalizeCachedUsage) when the context is cancelled, ensuring prompt token totals are correct even for partial streams.
  • The inline usage-accumulation blocks in both streaming handlers are replaced with calls to the new helper functions, removing the duplication.
  • Tests are added for both accumulateAnthropicResponsesUsage and accumulateBedrockResponsesUsage, verifying that all token fields (including cache read/write details and TTL buckets) are mirrored correctly and that post-normalization totals are accurate.

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

go test ./core/providers/anthropic/... ./core/providers/bedrock/...

Specifically, the new test cases TestAccumulateAnthropicResponsesUsage_MirrorsCacheIntoBilledUsage and TestAccumulateBedrockResponsesUsage_MirrorsCacheIntoBilledUsage validate that:

  1. All token fields (input, output, total, cached read, cached write, 5m/1h TTL buckets) are mirrored from upstream usage into BifrostLLMUsage.
  2. After normalizeCachedUsage is applied, prompt token totals correctly include cached token contributions.

To validate the cancellation path end-to-end, initiate a Responses streaming request and cancel the context mid-stream; the usage reported on the context key BifrostContextKeyStreamAccumulatedUsage should reflect tokens seen up to the point of cancellation.

Screenshots/Recordings

N/A

Breaking changes

  • No

Related issues

N/A

Security considerations

No auth, secrets, PII, or sandboxing implications. Usage data is internal billing metadata only.

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

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 39 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c31ea511-74e1-4bd0-ae56-154cd5b84e16

📥 Commits

Reviewing files that changed from the base of the PR and between 64745de and 38db0ce.

📒 Files selected for processing (4)
  • core/providers/anthropic/anthropic.go
  • core/providers/anthropic/cancelbilling_test.go
  • core/providers/bedrock/bedrock.go
  • core/providers/bedrock/cancelbilling_test.go
📝 Walkthrough

Walkthrough

Anthropic and Bedrock Responses streaming handlers now maintain a separate billedUsage accumulator alongside stream usage, via new helper functions accumulateAnthropicResponsesUsage and accumulateBedrockResponsesUsage. This billedUsage is registered in context for mid-stream cancellation/timeout billing and normalized once cached tokens are folded in. New unit tests validate cache mirroring and normalization.

Changes

Billed usage accumulation for Responses streaming

Layer / File(s) Summary
Anthropic billed usage accumulation
core/providers/anthropic/anthropic.go, core/providers/anthropic/cancelbilling_test.go
Adds accumulateAnthropicResponsesUsage helper, wires a billedUsage accumulator into HandleAnthropicResponsesStream registered under BifrostContextKeyStreamAccumulatedUsage with a normalizeBilledUsage closure invoked on cancellation/timeout, replaces inline event-loop accumulation, and adds a unit test verifying cache token mirroring and normalization arithmetic.
Bedrock billed usage accumulation
core/providers/bedrock/bedrock.go, core/providers/bedrock/cancelbilling_test.go
Adds accumulateBedrockResponsesUsage helper that updates stream usage and billed usage including cached-write TTL detail, wires billedUsage into ResponsesStream for context registration and normalization on cancellation/timeout, replaces inline per-event accumulation, and adds a unit test verifying cache token mirroring and normalization.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Stream as Responses Stream Handler
  participant Accum as Usage Accumulator
  participant Ctx as Stream Context
  participant Norm as normalizeCachedUsage

  Stream->>Ctx: register billedUsage accumulator
  loop each streamed event
    Stream->>Accum: accumulate usage and billedUsage from event
    Accum-->>Stream: updated counters and cache details
  end
  alt normal completion
    Stream->>Norm: normalize billedUsage
  else cancellation or timeout
    Stream->>Norm: normalize billedUsage (deferred)
  end
  Norm-->>Ctx: folded cached tokens into prompt/total for billing
Loading

Suggested reviewers: akshaydeo, danpiths

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR does not implement Files API support or POST /v1/files; it changes stream billing logic instead. [#123] Implement the Files API upload endpoint and provider support described in #123, or relink the PR to the correct issue.
Out of Scope Changes check ⚠️ Warning The billing changes are unrelated to the linked Files API support objective and appear outside the scope of #123. Remove the billing refactor from this PR or update the linked issue to match the actual Files API work.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly reflects the main change: fixing billing for interrupted Anthropic and Bedrock streaming requests.
Description check ✅ Passed The description follows the template well and covers summary, changes, testing, type, affected areas, and checklist items.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 07-02-fix_billing_on_failed_responses_stream_requests_anthropic_and_bedrock

Comment @coderabbitai help to get the list of available commands.

@CLAassistant

CLAassistant commented Jul 2, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@TejasGhatte
TejasGhatte marked this pull request as ready for review July 2, 2026 07:32

TejasGhatte commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai
coderabbitai Bot requested review from akshaydeo and danpiths July 2, 2026 07:33
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 2, 2026
@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

Safe to merge — the change is a targeted billing fix confined to the Responses API streaming paths for Anthropic and Bedrock, with no changes to the unary or chat-completion paths.

The accumulate helpers faithfully reproduce the existing max-taking logic while adding a mirrored BifrostLLMUsage write. The deferred normalization runs via LIFO before HandleStreamCancellation reads billedUsage (both in the same goroutine), so there is no data race. Bedrock's normalizeCachedUsage intentionally omits TotalTokens because Bedrock reports a provider-inclusive total; Anthropic's version adjusts both, and the tests validate both post-normalization invariants. No existing caller contract is broken.

No files require special attention. The test files omit the 5m TTL branch (already noted in a prior review thread) but this does not affect production correctness.

Important Files Changed

Filename Overview
core/providers/anthropic/anthropic.go Adds accumulateAnthropicResponsesUsage helper and wires billedUsage registration + deferred normalization into HandleAnthropicResponsesStream; logic and defer ordering are correct.
core/providers/bedrock/bedrock.go Adds accumulateBedrockResponsesUsage helper and mirrors the same billedUsage + deferred normalization pattern into ResponsesStream; intentionally omits TotalTokens adjustment in normalizeCachedUsage because Bedrock reports the total inclusive of cache tokens.
core/providers/anthropic/cancelbilling_test.go New test covers the 1h TTL cache path and normalization totals; the 5m TTL branch (Ephemeral5mInputTokens) is not exercised (already noted in a prior review thread).
core/providers/bedrock/cancelbilling_test.go New test covers 1h TTL and validates normalization; same 5m TTL gap as the Anthropic test file.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant G as Goroutine (stream loop)
    participant C as Context
    participant D as Defers (LIFO)
    participant H as HandleStreamCancellation
    participant B as attachBilledUsageFromContext

    G->>C: SetValue(BifrostContextKeyStreamAccumulatedUsage, billedUsage)
    loop Per usage event
        G->>G: accumulateAnthropicResponsesUsage / accumulateBedrockResponsesUsage
        Note over G: updates both usage & billedUsage
    end
    Note over G: ctx cancelled → return

    D->>D: "defer 7 — normalizeBilledUsage (if ctx.Err() != nil)"
    D->>D: defer 6 — stopCancellation
    D->>D: defer 5 — stopIdleTimeout
    D->>D: defer 4 — releaseGzip / resp.Body.Close
    D->>D: defer 3 — ReleaseStreamingResponse
    D->>H: "defer 2 — HandleStreamCancellation (ctx.Err() == Canceled)"
    H->>B: attachBilledUsageFromContext(ctx, cancelErr)
    B->>C: Value(BifrostContextKeyStreamAccumulatedUsage) → normalized billedUsage
    B->>B: copy billedUsage → cancelErr.BilledUsage
    D->>D: defer 1 — EnsureStreamFinalizerCalled
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant G as Goroutine (stream loop)
    participant C as Context
    participant D as Defers (LIFO)
    participant H as HandleStreamCancellation
    participant B as attachBilledUsageFromContext

    G->>C: SetValue(BifrostContextKeyStreamAccumulatedUsage, billedUsage)
    loop Per usage event
        G->>G: accumulateAnthropicResponsesUsage / accumulateBedrockResponsesUsage
        Note over G: updates both usage & billedUsage
    end
    Note over G: ctx cancelled → return

    D->>D: "defer 7 — normalizeBilledUsage (if ctx.Err() != nil)"
    D->>D: defer 6 — stopCancellation
    D->>D: defer 5 — stopIdleTimeout
    D->>D: defer 4 — releaseGzip / resp.Body.Close
    D->>D: defer 3 — ReleaseStreamingResponse
    D->>H: "defer 2 — HandleStreamCancellation (ctx.Err() == Canceled)"
    H->>B: attachBilledUsageFromContext(ctx, cancelErr)
    B->>C: Value(BifrostContextKeyStreamAccumulatedUsage) → normalized billedUsage
    B->>B: copy billedUsage → cancelErr.BilledUsage
    D->>D: defer 1 — EnsureStreamFinalizerCalled
Loading

Reviews (2): Last reviewed commit: "fix: billing on failed responses stream ..." | Re-trigger Greptile

Comment thread core/providers/anthropic/cancelbilling_test.go

Pratham-Mishra04 commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Merge activity

  • Jul 2, 1:44 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 2, 1:56 PM UTC: Graphite rebased this pull request as part of a merge.
  • Jul 2, 1:57 PM UTC: @Pratham-Mishra04 merged this pull request with Graphite.

@Pratham-Mishra04
Pratham-Mishra04 changed the base branch from 06-30-fix_gemini_openai_through_signature_compatibility to graphite-base/4842 July 2, 2026 13:52
@Pratham-Mishra04
Pratham-Mishra04 changed the base branch from graphite-base/4842 to dev July 2, 2026 13:55
@Pratham-Mishra04
Pratham-Mishra04 dismissed coderabbitai[bot]’s stale review July 2, 2026 13:55

The base branch was changed.

@Pratham-Mishra04
Pratham-Mishra04 force-pushed the 07-02-fix_billing_on_failed_responses_stream_requests_anthropic_and_bedrock branch from 64745de to 38db0ce Compare July 2, 2026 13:55
@Pratham-Mishra04
Pratham-Mishra04 merged commit e1198e5 into dev Jul 2, 2026
13 of 15 checks passed
@Pratham-Mishra04
Pratham-Mishra04 deleted the 07-02-fix_billing_on_failed_responses_stream_requests_anthropic_and_bedrock branch July 2, 2026 13:57
yangtuooc added a commit to yangtuooc/bifrost that referenced this pull request Jul 2, 2026
* upstream/dev:
  feat(mcp): add per-MCP-server tool execution timeout (maximhq#4472)
  fix: billing on failed responses stream requests anthropic and bedrock (maximhq#4842)
  fix: gemini openai through signature compatibility (maximhq#4810)
  fix: cancelled state in logs (maximhq#4831)
  fix: perplexity responses api compatibility (maximhq#4813)
  docs: clarify two-layer token refresh behavior and disabled-client refresh token expiry (maximhq#4849)
  fix: skip background token refresh for disabled/unconfigured MCP clients and guarantee non-nil logger in sync workers (maximhq#4848)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants