fix: apply matched provider group billing multiplier#1327
Conversation
📝 WalkthroughWalkthrough新增用户分组与供应商分组的计费交集解析工具,统一供应商会话倍率计算逻辑,并将其接入并发检查及故障切换后的最终供应商路径;测试覆盖交集、通配、默认及重选供应商场景。 Changes分组倍率解析与供应商选择
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
There was a problem hiding this comment.
Code Review
This pull request refactors the provider billing group cost multiplier resolution by introducing resolveBillingProviderGroups, which intersects user groups with the selected provider's tags. The multiplier resolution logic is moved to a dedicated function resolveGroupCostMultiplierForProvider and triggered after provider selection. The feedback highlights a critical issue: because resolveGroupCostMultiplierForProvider is private, external retry mechanisms (such as in forwarder.ts) cannot access it to re-resolve the multiplier when a fallback provider is selected, which could lead to billing discrepancies. Exporting this function is recommended to resolve this issue.
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.
| return providerTags.some((tag) => groups.includes(tag)); | ||
| } | ||
|
|
||
| async function resolveGroupCostMultiplierForProvider(session: ProxySession): Promise<void> { |
There was a problem hiding this comment.
Issue Analysis:
Currently, resolveGroupCostMultiplierForProvider is a private, module-scoped function that is only called within ProxyProviderResolver.ensure.
However, when a request fails with an upstream error (such as a 5xx or timeout) during the actual forwarding phase in forwarder.ts, a cross-provider retry/fallback is typically triggered. In this scenario, forwarder.ts calls pickRandomProviderWithExclusion to select a new fallback provider and updates the session's provider.
Since forwarder.ts cannot access the private resolveGroupCostMultiplierForProvider function, the billing multiplier will not be re-resolved for the newly selected fallback provider. As a result, the final billing will incorrectly use the multiplier of the initial (failed) provider, leading to billing discrepancies (overbilling or underbilling) in multi-group environments.
Recommendation:
Export resolveGroupCostMultiplierForProvider so that forwarder.ts (or any external retry mechanism) can import and call it to update the group cost multiplier whenever the provider is switched during fallback.
| async function resolveGroupCostMultiplierForProvider(session: ProxySession): Promise<void> { | |
| export async function resolveGroupCostMultiplierForProvider(session: ProxySession): Promise<void> { |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/utils/provider-group.test.ts (1)
2-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win请使用
@/路径别名导入工具模块。本次扩展的导入仍使用
./provider-group,不符合仓库 TypeScript 导入约定。建议修改
-} from "./provider-group"; +} from "`@/lib/utils/provider-group`";As per coding guidelines,
Use path alias@/to map to ./src/ for imports。🤖 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 `@src/lib/utils/provider-group.test.ts` around lines 2 - 8, Update the imports in provider-group.test.ts to reference the provider-group utilities through the repository’s `@/` path alias instead of the relative "./provider-group" path, while preserving the existing imported symbols.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@src/lib/utils/provider-group.test.ts`:
- Around line 2-8: Update the imports in provider-group.test.ts to reference the
provider-group utilities through the repository’s `@/` path alias instead of the
relative "./provider-group" path, while preserving the existing imported
symbols.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5dc4576e-02b2-4137-ac92-a299253fc57d
📒 Files selected for processing (4)
src/app/v1/_lib/proxy/provider-selector.tssrc/lib/utils/provider-group.test.tssrc/lib/utils/provider-group.tstests/unit/proxy/provider-selector-select-provider-by-type.test.ts
Summary
Fix provider group billing multiplier resolution so billing uses the group that actually matches the selected provider, rather than the first group in the user's/key's group list.
Root cause
Provider selection filters by the intersection between the request's effective provider groups and the selected provider's
groupTag, but the billing multiplier was resolved earlier from the raw effective group string. For users/keys with multiple groups,getGroupCostMultiplier()returns the first existing group in declaration order. That can underbill when the selected provider matches a later group with a higher multiplier.Example:
cus_claude_pro,cus_grok,gpt_test,mimocus_gpt,gpt_testgpt_testmultiplier:10Before this change, billing could resolve
cus_claude_profirst and use multiplier1.Changes
resolveBillingProviderGroups()to compute the billing group intersection between provider tags and effective user/key groups.Related
Validation
Result: 16 tests passed.
Note:
bun run typecheckcurrently fails on upstream/dev due to an unrelated Langfuse type mismatch insrc/lib/langfuse/trace-proxy-request.ts(LangfuseSpan.setTraceIO), outside this PR's diff.