fix: apply db discounts to routing - #2573
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR makes provider selection discount-aware and asynchronous, adds an SWR-backed effective-discount lookup, threads discount resolvers and discounted price/discount metadata through chat and video routing, updates OG image counting for discounted models, and converts affected tests to async usage. ChangesDiscounted provider selection and routing integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related 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 docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a16c98cd4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| : new Decimal(0); | ||
|
|
||
| return { | ||
| price: basePrice.times(new Decimal(1).minus(discount)), |
There was a problem hiding this comment.
When a DB discount is exactly 1 (the helper accepts it as valid), this line turns that provider's routing price into 0. In the weighted-score path with metrics, minPrice then becomes zero and the existing minPrice.gt(0) ? ... : 0 guard makes every provider's price score zero, so a 100%-discounted provider no longer gets any price advantage and can lose to a more expensive provider on uptime/latency. This affects normal gateway routing whenever metrics are present and a provider/model has a full discount; either special-case zero prices in the score calculation or avoid feeding a single zero into the ratio normalization.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates routing selection and routing metadata to consistently use effective database-driven discounts when computing “cheapest” provider choices (including chat/video fallback paths), and updates UI OG image generation to reflect discounted model counts based on API-backed mapping discounts.
Changes:
- Apply effective discounts (DB or injected resolver) before comparing provider prices, and surface the applied discount in routing metadata scores.
- Update Gateway chat/video routing to pass organization context and to use discounted prices in fallback/content-filter/rate-limit metadata paths.
- Make category OG image generation async and compute the “discounted” category count from API model mapping discounts instead of a hardcoded zero.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/actions/src/get-cheapest-from-available-providers.ts | Applies effective discounts during provider price scoring/selection and includes discount in routing metadata. |
| packages/actions/src/models.spec.ts | Updates routing tests for async selection and adds coverage for discount-applied price comparison. |
| apps/gateway/src/chat/chat.ts | Passes organization context and uses discounted prices/discount fields in routing metadata across fallback paths. |
| apps/gateway/src/videos/videos.ts | Uses discounted prices/discount fields for video routing selection metadata and low-uptime fallback metadata. |
| apps/ui/src/components/models/category-og-image.tsx | Makes category OG generation async and computes discounted model count from API-fetched mappings. |
| apps/ui/src/app/models/web-search/opengraph-image.tsx | Awaits async category OG generation. |
| apps/ui/src/app/models/vision/opengraph-image.tsx | Awaits async category OG generation. |
| apps/ui/src/app/models/video/opengraph-image.tsx | Awaits async category OG generation. |
| apps/ui/src/app/models/tools/opengraph-image.tsx | Awaits async category OG generation. |
| apps/ui/src/app/models/text/opengraph-image.tsx | Awaits async category OG generation. |
| apps/ui/src/app/models/text-to-image/opengraph-image.tsx | Awaits async category OG generation. |
| apps/ui/src/app/models/reasoning/opengraph-image.tsx | Awaits async category OG generation. |
| apps/ui/src/app/models/image-to-image/opengraph-image.tsx | Awaits async category OG generation. |
| apps/ui/src/app/models/embeddings/opengraph-image.tsx | Awaits async category OG generation. |
| apps/ui/src/app/models/discounted/opengraph-image.tsx | Awaits async category OG generation for discounted category. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const basePrice = getProviderSelectionPrice( | ||
| providerInfo, | ||
| options?.videoPricing, | ||
| ); | ||
| const discount = providerInfo | ||
| ? await getProviderSelectionDiscount(providerInfo, modelId, options) | ||
| : new Decimal(0); | ||
|
|
||
| return { | ||
| price: basePrice.times(new Decimal(1).minus(discount)), | ||
| discount, | ||
| }; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/gateway/src/chat/chat.ts (1)
2748-2758:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse discounted pricing for
originalProviderScorein fallback metadataLine 2764 and Line 2970 still build
originalProviderScorefrom rawinputPrice/outputPriceand omitdiscount, while the rest of provider scores in these paths are now discount-aware. This produces inconsistent routing metadata in rate-limit/low-uptime fallback responses.🛠️ Suggested patch
- const originalProviderPrice = originalProviderInfo - ? Number(originalProviderInfo.inputPrice ?? "0") + - Number(originalProviderInfo.outputPrice ?? "0") - : 0; + const originalProviderPricing = originalProviderInfo + ? await getDiscountedProviderSelectionPrice( + originalProviderInfo, + modelWithPricing.id, + { + organizationId: project.organizationId, + }, + ) + : null; const originalProviderScore = { providerId: requestedProvider, score: -1, - price: originalProviderPrice, + price: originalProviderPricing?.price.toNumber() ?? 0, + discount: originalProviderPricing?.discount.toNumber() ?? 0, rate_limited: true as const, };- const originalProviderPrice = originalProviderInfo - ? Number(originalProviderInfo.inputPrice ?? "0") + - Number(originalProviderInfo.outputPrice ?? "0") - : 0; + const originalProviderPricing = originalProviderInfo + ? await getDiscountedProviderSelectionPrice( + originalProviderInfo, + modelWithPricing.id, + { + organizationId: project.organizationId, + }, + ) + : null; // Create score entry for the original requested provider const originalProviderScore = { providerId: requestedProvider, score: -1, // Negative score indicates this provider was skipped due to low uptime - price: originalProviderPrice, + price: originalProviderPricing?.price.toNumber() ?? 0, + discount: originalProviderPricing?.discount.toNumber() ?? 0, uptime: currentUptime, latency: metrics.averageLatency, throughput: metrics.throughput, };Also applies to: 2764-2774, 2953-2963, 2970-2983
🤖 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 `@apps/gateway/src/chat/chat.ts` around lines 2748 - 2758, The fallback metadata builds originalProviderScore using raw inputPrice/outputPrice (e.g., where originalProviderScore is constructed in the getCheapestFromAvailableProviders fallback paths around candidatesForRouting and modelWithPricing) but omits the provider discount; update those constructions (the occurrences around originalProviderScore at the getCheapestFromAvailableProviders call sites and the later fallback blocks) to compute originalProviderScore using the discounted prices (apply the existing discount field or reuse the already-discounted price variable instead of raw inputPrice/outputPrice) so that originalProviderScore is discount-aware and consistent with other provider scores.apps/gateway/src/videos/videos.ts (1)
1691-1702:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate the routing metadata contract for
providerScores[].discount.
apps/ui/src/lib/api/v1.d.ts:2770-2807still omitsdiscountonroutingMetadata.providerScores[], so this new field is invisible to typed consumers even though the gateway now writes it here. Please update the schema/source type that generates that client shape before relying on this field downstream.Also applies to: 1780-1798
🤖 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 `@apps/gateway/src/videos/videos.ts` around lines 1691 - 1702, The gateway now writes a discount number into routingMetadata.providerScores[].discount but the generated client type for routingMetadata.providerScores omits that field; update the source schema/type generation so the providerScores entry includes discount: number (and regenerate the client types), ensuring the routingMetadata/providerScores type (and any references used by the UI client) reflect the new discount property so typed consumers can access it; update any relevant codegen inputs and re-run generation to propagate the change.
🤖 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 `@packages/actions/src/get-cheapest-from-available-providers.ts`:
- Around line 392-408: The loop in getCheapestFromAvailableProviders currently
calls getDiscountedProviderSelectionPrice (which in turn calls
getEffectiveDiscount) for each provider causing N DB reads; instead, before
mapping providers call a batched discount fetch for the (organizationId,
modelWithPricing.id) pair (or add a helper like
getEffectiveDiscountsForOrganizationModel) and return a map of
provider->discount, then change the providerPrices creation (the providers.map
using findProviderMapping, getDiscountedProviderSelectionPrice, and
providerSelectionKey) to read discounts from that in-memory map and compute
price/discount locally; apply the same batching refactor to the other occurrence
around the code referenced (the block at ~473-478) so no per-provider DB queries
occur.
- Around line 379-380: The discount check currently allows parsedDiscount === 1
which yields a resolved price of 0 and then makes minPrice === 0 force every
priceScore to 0; update the logic so fully discounted providers are handled
explicitly: either reject exact 1 by changing the validation from
parsedDiscount.lte(0) || parsedDiscount.gt(1) to treat parsedDiscount.gte(1) as
invalid (so parsedDiscount === 1 returns Decimal(0)), or keep accepting 1 but
modify the weighted scorer logic (where minPrice and priceScore are computed) to
special-case zero-priced providers so only free provider(s) receive the best
priceScore and others do not get zeroed out; apply the same change in the other
occurrence referenced (around lines 679-681). Ensure you touch symbols
parsedDiscount, minPrice, and priceScore in
get-cheapest-from-available-providers.ts.
---
Outside diff comments:
In `@apps/gateway/src/chat/chat.ts`:
- Around line 2748-2758: The fallback metadata builds originalProviderScore
using raw inputPrice/outputPrice (e.g., where originalProviderScore is
constructed in the getCheapestFromAvailableProviders fallback paths around
candidatesForRouting and modelWithPricing) but omits the provider discount;
update those constructions (the occurrences around originalProviderScore at the
getCheapestFromAvailableProviders call sites and the later fallback blocks) to
compute originalProviderScore using the discounted prices (apply the existing
discount field or reuse the already-discounted price variable instead of raw
inputPrice/outputPrice) so that originalProviderScore is discount-aware and
consistent with other provider scores.
In `@apps/gateway/src/videos/videos.ts`:
- Around line 1691-1702: The gateway now writes a discount number into
routingMetadata.providerScores[].discount but the generated client type for
routingMetadata.providerScores omits that field; update the source schema/type
generation so the providerScores entry includes discount: number (and regenerate
the client types), ensuring the routingMetadata/providerScores type (and any
references used by the UI client) reflect the new discount property so typed
consumers can access it; update any relevant codegen inputs and re-run
generation to propagate the change.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ea32d9d0-029f-45c1-b8b1-f1c807f39f1f
📒 Files selected for processing (15)
apps/gateway/src/chat/chat.tsapps/gateway/src/videos/videos.tsapps/ui/src/app/models/discounted/opengraph-image.tsxapps/ui/src/app/models/embeddings/opengraph-image.tsxapps/ui/src/app/models/image-to-image/opengraph-image.tsxapps/ui/src/app/models/reasoning/opengraph-image.tsxapps/ui/src/app/models/text-to-image/opengraph-image.tsxapps/ui/src/app/models/text/opengraph-image.tsxapps/ui/src/app/models/tools/opengraph-image.tsxapps/ui/src/app/models/video/opengraph-image.tsxapps/ui/src/app/models/vision/opengraph-image.tsxapps/ui/src/app/models/web-search/opengraph-image.tsxapps/ui/src/components/models/category-og-image.tsxpackages/actions/src/get-cheapest-from-available-providers.tspackages/actions/src/models.spec.ts
| if (parsedDiscount.lte(0) || parsedDiscount.gt(1)) { | ||
| return new Decimal(0); |
There was a problem hiding this comment.
Handle fully discounted providers explicitly in weighted scoring.
discount === 1 is accepted here, which makes the resolved price 0. In the weighted path, minPrice === 0 then forces every priceScore to 0, so routing can ignore a free provider and still pick a paid one. Either reject exact 1 here, or special-case zero-priced providers in the scorer so only the free provider(s) get the best price score.
Suggested fix
- const priceScore = minPrice.gt(0)
- ? providerScore.price.div(minPrice).minus(1)
- : new Decimal(0);
+ const priceScore = minPrice.gt(0)
+ ? providerScore.price.div(minPrice).minus(1)
+ : providerScore.price.eq(0)
+ ? new Decimal(0)
+ : new Decimal(1);Also applies to: 679-681
🤖 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 `@packages/actions/src/get-cheapest-from-available-providers.ts` around lines
379 - 380, The discount check currently allows parsedDiscount === 1 which yields
a resolved price of 0 and then makes minPrice === 0 force every priceScore to 0;
update the logic so fully discounted providers are handled explicitly: either
reject exact 1 by changing the validation from parsedDiscount.lte(0) ||
parsedDiscount.gt(1) to treat parsedDiscount.gte(1) as invalid (so
parsedDiscount === 1 returns Decimal(0)), or keep accepting 1 but modify the
weighted scorer logic (where minPrice and priceScore are computed) to
special-case zero-priced providers so only free provider(s) receive the best
priceScore and others do not get zeroed out; apply the same change in the other
occurrence referenced (around lines 679-681). Ensure you touch symbols
parsedDiscount, minPrice, and priceScore in
get-cheapest-from-available-providers.ts.
| const providerPrices = await Promise.all( | ||
| providers.map(async (provider) => { | ||
| const providerInfo = findProviderMapping( | ||
| modelWithPricing.providers, | ||
| provider, | ||
| ); | ||
| const { price, discount } = await getDiscountedProviderSelectionPrice( | ||
| providerInfo, | ||
| modelWithPricing.id, | ||
| { | ||
| ...options, | ||
| videoPricing, | ||
| }, | ||
| ); | ||
|
|
||
| return [providerSelectionKey(provider), { price, discount }] as const; | ||
| }), |
There was a problem hiding this comment.
Batch discount resolution instead of querying once per provider.
When organizationId is set, this loop fans out into one getEffectiveDiscount() call per stable provider, and getCheapestFromAvailableProviders() does that eagerly before the sticky/exploration early returns. On the gateway request path, that turns a single routing decision into N DB reads and adds avoidable latency/load as provider counts grow. Please fetch the applicable discounts once per (organizationId, modelId) and resolve precedence in-memory, or add a batched DB helper for this path.
Also applies to: 473-478
🤖 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 `@packages/actions/src/get-cheapest-from-available-providers.ts` around lines
392 - 408, The loop in getCheapestFromAvailableProviders currently calls
getDiscountedProviderSelectionPrice (which in turn calls getEffectiveDiscount)
for each provider causing N DB reads; instead, before mapping providers call a
batched discount fetch for the (organizationId, modelWithPricing.id) pair (or
add a helper like getEffectiveDiscountsForOrganizationModel) and return a map of
provider->discount, then change the providerPrices creation (the providers.map
using findProviderMapping, getDiscountedProviderSelectionPrice, and
providerSelectionKey) to read discounts from that in-memory map and compute
price/discount locally; apply the same batching refactor to the other occurrence
around the code referenced (the block at ~473-478) so no per-provider DB queries
occur.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99b5021406
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return await swrWrap( | ||
| `discount:${orgPart}:${provider}:${model}`, | ||
| [discountTableName], | ||
| async () => { |
There was a problem hiding this comment.
When Postgres is unavailable and this specific discount:{org}:{provider}:{model} SWR key has not already been primed, swrWrap rethrows the database error. Because the new routing path calls findEffectiveDiscount for every candidate provider, otherwise cache-warmed gateway requests can now fail solely because a discount lookup missed its stale mirror; the existing DB helper for discounts falls back to a zero discount on lookup errors, so this wrapper should preserve that fail-open behavior.
Useful? React with 👍 / 👎.
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 `@apps/gateway/src/chat/chat.ts`:
- Around line 1804-1936: The service_tier check is being applied too early by
mutating modelInfo.providers (the block guarded by isRequestedServiceTier and
the supportsRequestedTier filter that calls providerMatchesRequestedProvider and
mappingSupportsRequestedServiceTier), which causes auto-routing and
provider-key/region locking to validate against a synthetic candidate set;
revert removing entries from
modelInfo.providers/routingExpandedModelProviders/allModelProviders here and
instead apply
mappingSupportsRequestedServiceTier(providerMatchesRequestedProvider(...)) when
assembling each concrete candidate list (the same place the auto-routing loop
builds per-request candidateProviders and the later concrete-mapping selection
runs), i.e., remove the early filter and call the supportsRequestedTier
predicate at the point where you finalize candidate providers for
routing/selection so the tier check is enforced on final concrete mappings only.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8e93185d-f9a7-49e4-8fc6-47ddc469f80c
📒 Files selected for processing (4)
apps/gateway/src/chat/chat.tsapps/gateway/src/lib/cached-queries-swr.spec.tsapps/gateway/src/lib/cached-queries.tsapps/gateway/src/videos/videos.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/gateway/src/videos/videos.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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 `@apps/gateway/src/chat/chat.ts`:
- Around line 1804-1936: The service_tier check is being applied too early by
mutating modelInfo.providers (the block guarded by isRequestedServiceTier and
the supportsRequestedTier filter that calls providerMatchesRequestedProvider and
mappingSupportsRequestedServiceTier), which causes auto-routing and
provider-key/region locking to validate against a synthetic candidate set;
revert removing entries from
modelInfo.providers/routingExpandedModelProviders/allModelProviders here and
instead apply
mappingSupportsRequestedServiceTier(providerMatchesRequestedProvider(...)) when
assembling each concrete candidate list (the same place the auto-routing loop
builds per-request candidateProviders and the later concrete-mapping selection
runs), i.e., remove the early filter and call the supportsRequestedTier
predicate at the point where you finalize candidate providers for
routing/selection so the tier check is enforced on final concrete mappings only.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8e93185d-f9a7-49e4-8fc6-47ddc469f80c
📒 Files selected for processing (4)
apps/gateway/src/chat/chat.tsapps/gateway/src/lib/cached-queries-swr.spec.tsapps/gateway/src/lib/cached-queries.tsapps/gateway/src/videos/videos.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/gateway/src/videos/videos.ts
🛑 Comments failed to post (1)
apps/gateway/src/chat/chat.ts (1)
1804-1936:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winDefer
service_tierfiltering until the candidate set is concrete.Line 1804 prunes
modelInfo.providersbefore auto-routing, provider-key region locking, or env-key selection are resolved. That meansautorequests get validated against the synthetic pre-routing state instead of the real model/provider candidates, and direct-provider requests can lose the provider-key-locked region before the later concrete-mapping selection runs. The auto-routing loop at Lines 2460-2564 also never re-applies the tier check, so the real candidate set is being built without the constraint that triggered the early 400. Move theservice_tiercheck to the point where each concrete candidate list is assembled instead of mutatingmodelInfo.providersup front.Also applies to: 2460-2564, 2730-2865
🤖 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 `@apps/gateway/src/chat/chat.ts` around lines 1804 - 1936, The service_tier check is being applied too early by mutating modelInfo.providers (the block guarded by isRequestedServiceTier and the supportsRequestedTier filter that calls providerMatchesRequestedProvider and mappingSupportsRequestedServiceTier), which causes auto-routing and provider-key/region locking to validate against a synthetic candidate set; revert removing entries from modelInfo.providers/routingExpandedModelProviders/allModelProviders here and instead apply mappingSupportsRequestedServiceTier(providerMatchesRequestedProvider(...)) when assembling each concrete candidate list (the same place the auto-routing loop builds per-request candidateProviders and the later concrete-mapping selection runs), i.e., remove the early filter and call the supportsRequestedTier predicate at the point where you finalize candidate providers for routing/selection so the tier check is enforced on final concrete mappings only.
Summary
Validation
Notes
Summary by CodeRabbit
New Features
Improvements