fix(proxy): preserve hedge loser Codex priority billing#1255
Conversation
📝 WalkthroughWalkthrough在流式 hedge 的输家计费路径中新增并传播 requestedServiceTier 字段：forwarder 在 commitWinner 时回填，response-handler 将其纳入 billingContext，用于 Codex 优先级决策，并在 trackCostToRedis 中使用覆盖值计算与记录成本；新增单测覆盖该逻辑分支。 变更详情Hedge Loser 计费中的请求服务等级传播
评估代码审查工作量🎯 3 (中等) | ⏱️ ~20 分钟 可能相关的 PRs
建议审阅者
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for capturing and preserving the requested service tier (requestedServiceTier) in the billing snapshot of hedge loser attempts. This ensures that when finalizing billing for a hedge loser, the correct service tier is used for resolving Codex priority billing decisions. Unit tests have been added to verify this behavior. The review feedback suggests a more robust check for requestedServiceTier in the options object of resolveCodexPriorityBillingDecision to avoid inconsistent behavior when the property is explicitly set to undefined.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const requestedServiceTier = | ||
| options && "requestedServiceTier" in options | ||
| ? (options.requestedServiceTier ?? null) | ||
| : getRequestedCodexServiceTier(session, provider); |
There was a problem hiding this comment.
Using the "requestedServiceTier" in options check can lead to inconsistent behavior in TypeScript/JavaScript depending on whether the property is omitted or explicitly set to undefined (e.g., { provider } vs { provider, requestedServiceTier: undefined }).
A more robust and idiomatic pattern is to check if the property is not undefined. This consistently falls back to getRequestedCodexServiceTier when the value is not provided or is undefined, while correctly preserving explicit string or null values.
| const requestedServiceTier = | |
| options && "requestedServiceTier" in options | |
| ? (options.requestedServiceTier ?? null) | |
| : getRequestedCodexServiceTier(session, provider); | |
| const requestedServiceTier = | |
| options?.requestedServiceTier !== undefined | |
| ? options.requestedServiceTier | |
| : getRequestedCodexServiceTier(session, provider); |
github-actions
Bot
added
size/S
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/app/v1/_lib/proxy/response-handler.ts`:
- Around line 3851-3855: The Redis billing call still reads provider and
multiplier from the mutated loserSession, causing Redis to be billed with the
winner's context; extract the billing fields from loserSession immediately after
determining the initial loser (e.g. provider, session.getContext1mApplied(),
session.getGroupCostMultiplier(), and requestedServiceTier if present) and pass
that explicit billing context into trackCostToRedis instead of letting
trackCostToRedis read from loserSession, and ensure
resolveCodexPriorityBillingDecision continues to receive
provider/requestedServiceTier as before; update calls to trackCostToRedis(...)
to accept and use this extracted loserBillingContext so Redis accounting remains
decoupled from the shared session mutations.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c17a0a27-7031-4d09-9099-1f51840b9423
📒 Files selected for processing (3)
src/app/v1/_lib/proxy/forwarder.tssrc/app/v1/_lib/proxy/response-handler.tstests/unit/proxy/response-handler-hedge-loser-priority.test.ts
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This is a well-targeted bug fix that captures the initial service_tier from the loser's request before syncWinningAttemptSession overwrites the shared session, then threads it through resolveCodexPriorityBillingDecision for accurate priority billing. The change is backward-compatible, properly typed, and includes a focused regression test.
PR Size: M
- Lines changed: 221 (214 additions, 7 deletions; 187 of which are the new test file)
- Files changed: 3
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Review Coverage
- Logic and correctness - Correct capture-before-sync pattern; backward-compatible optional parameter; all 4 existing call sites of
resolveCodexPriorityBillingDecisionverified - Security (OWASP Top 10) - No user input handling; internal billing path only
- Error handling - Consistent with surrounding code; no new try/catch blocks introduced
- Type safety -
requestedServiceTier: string | nullappropriately typed in bothbillingSnapshotandbillingContext; optionaloptionsparameter preserves existing signatures - Documentation accuracy - Existing comments accurately describe the pre-sync capture pattern
- Test coverage - Regression test covers the exact scenario (initial Codex priority loser after winner session sync); verifies priority pricing rates are applied (cost=400 from input_cost_per_token_priority=2, output_cost_per_token_priority=20 with 100/10 tokens)
- Code clarity - Focused change with clear intent
Automated review by Claude AI
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
2 participants
Summary
service_tierbefore the shared session is synced to the winning attemptpriorityloser after winner-session sync pollutionRoot cause
finalizeHedgeLoserBillingalready accepts a pre-sync billing context so the initial provider loser is priced with its own model and multipliers. However, Codex priority billing still calledresolveCodexPriorityBillingDecision(loserSession, ...), which readsservice_tierfrom the session aftersyncWinningAttemptSessionmay have overwritten it with the winner request. In a hedge race where the initial Codex attempt requestedprioritybut the winner did not, the loser could be under-billed at default-tier rates.Related PRs:
Changes
Core Changes
forwarder.ts: CapturerequestedServiceTierfrom the loser's request message intobillingSnapshotbefore the winner session can overwrite itresponse-handler.ts: ThreadproviderandrequestedServiceTierfrombillingContextthroughresolveCodexPriorityBillingDecisionso priority billing uses the loser's own tier, not the winner'sresponse-handler.ts: ExtendgetRequestedCodexServiceTierto accept an optional provider overrideSupporting Changes
StreamingHedgeAttempt.billingSnapshottype: AddrequestedServiceTier: string | nullfieldfinalizeHedgeLoserBillingparams: AddrequestedServiceTiertobillingContexttypeTesting
Automated Tests
tests/unit/proxy/response-handler-hedge-loser-priority.test.ts(187 lines) — verifies that an initial Codex loser withpriorityservice tier is correctly billed at priority rates after winner session syncValidation
bun run typecheckbun run test tests/unit/proxy/response-handler-hedge-loser-priority.test.ts tests/unit/proxy/proxy-forwarder-hedge-first-byte.test.ts tests/integration/billing-model-source.test.tsbunx biome check src/app/v1/_lib/proxy/response-handler.ts src/app/v1/_lib/proxy/forwarder.ts tests/unit/proxy/response-handler-hedge-loser-priority.test.tsDescription enhanced by Claude AI
Greptile Summary
This PR fixes an under-billing bug in the Codex hedge-loser path where
syncWinningAttemptSessioncould overwrite the shared session'sservice_tierbeforeresolveCodexPriorityBillingDecisionread it, causing an initial Codex loser that requestedpriorityto be billed at default rates.forwarder.ts: CapturesrequestedServiceTierfrom the loser's request message intobillingSnapshotbefore the winner session sync, extending the existing snapshot mechanism that already preserved model and cost-multiplier state.response-handler.ts: Threadsproviderand the capturedrequestedServiceTierthroughresolveCodexPriorityBillingDecisionandtrackCostToRedisso both the DB cost record and Redis rate-limit counters use the loser's pre-sync tier, not the winner's.requestedServiceTier: \"priority\"in its billing snapshot is billed at priority rates even after the shared session has been polluted with the winner's default-tier request.Confidence Score: 5/5
Safe to merge — the change is a narrow, well-scoped billing snapshot extension with no mutations to the hot path and a dedicated regression test.
The fix captures
requestedServiceTierinto the billing snapshot before session-sync can overwrite it, then consistently threads both the provider and the captured tier through cost calculation and Redis tracking. Shadow-session losers preserve existing behavior, and the tests verify the arithmetic end-to-end.No files require special attention.
Important Files Changed
requestedServiceTierto the billing snapshot captured beforesyncWinningAttemptSession; snapshot logic and the type definition are correctly extended in lockstep.providerandrequestedServiceTierthroughresolveCodexPriorityBillingDecisionandtrackCostToRedis; both the DB cost and Redis counters now consistently use the loser's pre-sync billing parameters.Sequence Diagram
sequenceDiagram participant FW as forwarder.ts (commitWinner) participant SS as syncWinningAttemptSession participant FHL as finalizeHedgeLoserBilling participant RCPBD as resolveCodexPriorityBillingDecision participant TCR as trackCostToRedis FW->>FW: Capture billingSnapshot FW->>SS: syncWinningAttemptSession(session, winner.session) Note over SS: Overwrites session.request.message with winner's tier FW->>FHL: "finalizeHedgeLoserBilling({billingContext: billingSnapshot})" FHL->>RCPBD: "resolveCodexPriorityBillingDecision(loserSession, actualTier, {provider, requestedServiceTier})" Note over RCPBD: Uses captured tier instead of polluted session value RCPBD-->>FHL: "{effectivePriority: true}" FHL->>TCR: "trackCostToRedis(..., {provider, context1mApplied, groupCostMultiplier})" Note over TCR: Redis counters use loser's cost parametersReviews (2): Last reviewed commit: "fix(proxy): track hedge loser Redis cost..." | Re-trigger Greptile