fix: respect provider region limits - #1909
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughCollapse multi-region provider candidates to a single best region per provider, apply max_tokens-based eligibility against provider maxOutput across routing and fallbacks, sync expanded per-region provider sets with IAM filtering, and enforce max_tokens validation prior to requests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Gateway
participant ProviderSelection
participant Metrics
Client->>Gateway: POST /chat (includes max_tokens)
Gateway->>ProviderSelection: Resolve modelInfo, expand regional mappings
ProviderSelection->>ProviderSelection: Filter candidates by maxOutput >= max_tokens
Gateway->>Metrics: Fetch provider metrics/pricing
ProviderSelection->>ProviderSelection: Collapse providers to best region per provider (rgba(0,128,0,0.5))
ProviderSelection->>ProviderSelection: Select cheapest provider from collapsed set
Gateway->>Client: Route request with usedProvider/usedModel and routingMetadata
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: e4f834527d
ℹ️ 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".
| let expandedIamFilteredModelProviders = iamAllowedProviders | ||
| ? expandedActiveModelProviders.filter((p) => | ||
| iamAllowedProviders.includes(p.providerId), | ||
| ) | ||
| : expandedActiveModelProviders; |
There was a problem hiding this comment.
expandedIamFilteredModelProviders is built from expandedActiveModelProviders, but that expanded list is created before the credits/hybrid region-limit filters run. Later routing paths use this expanded IAM list for provider selection, which reintroduces non-default regions that were intentionally excluded by filterRegionsByAvailableKeys/hybrid filtering. In credits or hybrid projects without region-specific keys, this can select an endpoint region the configured token cannot use, causing avoidable routing failures.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Updates gateway routing to better respect provider region constraints by preferring concrete regional mappings over synthetic/root mappings and by filtering out providers that can’t satisfy a request’s max_tokens.
Changes:
- Add routing logic to collapse multiple regional candidates down to the best region per provider and avoid scoring/choosing synthetic root region mappings.
- Extend provider eligibility filtering to exclude mappings whose
maxOutputis below the request’smax_tokens. - Update/extend fallback routing tests to cover synthetic root region mappings and maxOutput-based exclusion.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/gateway/src/fallback.spec.ts | Adds routing tests for ignoring synthetic/root region mappings and for maxOutput-based provider exclusion; adjusts routing metadata assertions. |
| apps/gateway/src/chat/tools/resolve-provider-context.ts | Changes behavior from silently capping max_tokens to throwing a 400 when exceeding provider mapping maxOutput. |
| apps/gateway/src/chat/chat.ts | Adds per-provider “best region” collapsing and maxTokens eligibility filtering; updates routing to use expanded regional providers in more paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let expandedIamFilteredModelProviders = iamAllowedProviders | ||
| ? expandedActiveModelProviders.filter((p) => | ||
| iamAllowedProviders.includes(p.providerId), | ||
| ) | ||
| : expandedActiveModelProviders; |
There was a problem hiding this comment.
expandedIamFilteredModelProviders is derived from expandedActiveModelProviders computed before requested-region filtering and before credits/hybrid region filtering (filterRegionsByAvailableKeys/filterHybridRegions). Later routing paths use this expanded list (e.g. in low-uptime fallback and normal routing), which can re-introduce regions that were intentionally filtered out and potentially route to a region without an available key or outside the explicitly requested region. Consider deriving the expanded+IAM-filtered list from the already-filtered modelInfo.providers (and applying the same project-mode region filtering to the expanded list), so routing cannot pick an ineligible region.
| let expandedIamFilteredModelProviders = iamAllowedProviders | |
| ? expandedActiveModelProviders.filter((p) => | |
| iamAllowedProviders.includes(p.providerId), | |
| ) | |
| : expandedActiveModelProviders; | |
| // Ensure the expanded IAM-filtered providers cannot reintroduce regions | |
| // that were filtered out from modelInfo.providers (e.g. due to region | |
| // constraints or missing keys) by deriving them directly from the | |
| // IAM-filtered model providers. | |
| let expandedIamFilteredModelProviders = iamFilteredModelProviders; |
| id: "glm-4-6-alibaba-cn-beijing-max-tokens", | ||
| modelId: "glm-4.6", | ||
| providerId: "alibaba", | ||
| modelName: "glm-4.6:cn-beijing", | ||
| region: "cn-beijing", | ||
| streaming: true, | ||
| }, |
There was a problem hiding this comment.
This test asserts routing excludes providers when max_tokens exceeds a provider mapping’s maxOutput, but none of the inserted modelProviderMapping rows set maxOutput (so it will be NULL/undefined and the new filtering won’t exclude anything). To make this test meaningful and deterministic, set a concrete maxOutput for the provider you expect to be excluded (e.g. Alibaba < 20000) and (optionally) ensure at least one other candidate has maxOutput >= 20000 so the request can succeed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/gateway/src/chat/chat.ts`:
- Around line 971-975: The code recreates expandedIamFilteredModelProviders from
expandedActiveModelProviders, which reintroduces providers that were pruned
earlier by region filtering on modelInfo.providers; instead, rebuild
expandedIamFilteredModelProviders from the already-region-filtered provider set
(the variable used earlier where modelInfo.providers was pruned) when
iamAllowedProviders is present, i.e. replace the use of
expandedActiveModelProviders with the previously filtered provider list so IAM
filtering cannot reintroduce disallowed regions; reference
expandedIamFilteredModelProviders, iamAllowedProviders,
expandedActiveModelProviders, and modelInfo.providers to locate and correct the
assignment.
- Around line 301-307: The current pre-filter uses the raw request field
options.maxTokens against provider.maxOutput which can differ from the effective
max_tokens after prepareRequestBody(); change the checks in the blocks that
compare options.maxTokens to provider.maxOutput (including the other occurrence
noted) to first obtain the normalized/effective maxTokens by calling or
mimicking prepareRequestBody() (or running the same normalization logic) for the
candidate provider and request, then compare provider.maxOutput to that
effective value so deterministic failures are filtered out before
prepareRequestBody() is invoked.
- Around line 1101-1111: Auto-routing currently only applies regional filtering
in credits mode, allowing api-keys/hybrid to pick regions that conflict with a
provider key's DB-lock; update the candidate provider selection so the DB-key
region lock is applied regardless of project.mode by deriving the key-locked
region via resolveRegionFromProviderKey(providerKey) (or similar) and filtering
the expanded mappings (expandAllProviderRegions /
preferConcreteRegionalMappings) with filterRegionsByAvailableKeys (or an added
filter) before populating usedRegion, ensuring usedRegion cannot be set to a
region outside the provider key's configured region.
In `@apps/gateway/src/fallback.spec.ts`:
- Around line 1123-1137: The test's log lookup can miss the entry because the
request used model "zai/glm-4.6" while the find checks for requestedModel ===
"glm-4.6"; update the lookup in the test (the logs/find block that uses
requestedModel) to either match the full prefixed model string ("zai/glm-4.6")
or normalize both sides (e.g., strip provider prefix) so the correct log is
found, replace the weak response assertion expect(res.status).not.toBe(400) with
a strict expect(res.status).toBe(200) to ensure success, and add a positive
assertion for the chosen provider (e.g., expect(log?.usedProvider).toBe("zai")
or the expected provider) instead of only negative checks to guarantee the
intended routing behavior; adjust references to waitForLogs, logs, log,
requestedModel, res, and usedProvider accordingly.
- Around line 1068-1094: The test inserts into modelProviderMapping but omits
the maxOutput field, so the routing filter in chat.ts (which checks maxOutput
!== undefined) never runs; update the inserted records for ids
"glm-4-6-alibaba-cn-beijing-max-tokens", "glm-4-6-zai-root-max-tokens", and
"glm-4-6-novita-root-max-tokens" to include explicit maxOutput values (set
alibaba's maxOutput to a value below 20000 and the others to >=20000 as needed)
so the test exercises the maxOutput-based exclusion logic used in chat.ts.
🪄 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: 47952dcb-b833-4f3d-b72c-cd9a1faf8e7e
📒 Files selected for processing (3)
apps/gateway/src/chat/chat.tsapps/gateway/src/chat/tools/resolve-provider-context.tsapps/gateway/src/fallback.spec.ts
| if ( | ||
| options.maxTokens !== undefined && | ||
| provider.maxOutput !== undefined && | ||
| options.maxTokens > provider.maxOutput | ||
| ) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Filter on the effective max_tokens value.
prepareRequestBody() can raise or inject max_tokens per provider, so these raw checks still keep candidates that will deterministically fail the later post-prepareRequestBody 400 validation. Please base both sites on the normalized/effective value instead of the raw request field.
Also applies to: 1184-1190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/gateway/src/chat/chat.ts` around lines 301 - 307, The current pre-filter
uses the raw request field options.maxTokens against provider.maxOutput which
can differ from the effective max_tokens after prepareRequestBody(); change the
checks in the blocks that compare options.maxTokens to provider.maxOutput
(including the other occurrence noted) to first obtain the normalized/effective
maxTokens by calling or mimicking prepareRequestBody() (or running the same
normalization logic) for the candidate provider and request, then compare
provider.maxOutput to that effective value so deterministic failures are
filtered out before prepareRequestBody() is invoked.
| expect(res.status).not.toBe(400); | ||
|
|
||
| const logs = await waitForLogs(1); | ||
| const log = | ||
| logs.find((entry) => entry.requestedModel === "glm-4.6") ?? logs.at(-1); | ||
| expect(log).toBeTruthy(); | ||
| expect(log?.usedProvider).not.toBe("alibaba"); | ||
| expect(log?.usedModel).not.toBe("alibaba/glm-4.6:cn-beijing"); | ||
| expect(log?.routingMetadata?.providerScores).not.toContainEqual( | ||
| expect.objectContaining({ | ||
| providerId: "alibaba", | ||
| region: "cn-beijing", | ||
| }), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Log lookup may not find the correct entry due to model prefix mismatch.
The request uses model: "zai/glm-4.6" (line 1117), but the log lookup searches for requestedModel === "glm-4.6" (line 1127). If requestedModel includes the provider prefix, this find will fail and silently fall back to logs.at(-1), potentially masking test failures.
Additionally:
- Line 1123:
expect(res.status).not.toBe(400)is a weak assertion — consider assertingtoBe(200)to verify the request actually succeeded. - Lines 1129-1130: Only negative assertions about alibaba being excluded. Consider adding a positive assertion about which provider is expected to be used (e.g.,
zaiornovita).
Suggested improvements
- expect(res.status).not.toBe(400);
+ expect(res.status).toBe(200);
const logs = await waitForLogs(1);
const log =
- logs.find((entry) => entry.requestedModel === "glm-4.6") ?? logs.at(-1);
+ logs.find((entry) => entry.requestedModel === "zai/glm-4.6") ?? logs.at(-1);
expect(log).toBeTruthy();
+ // Add positive assertion for expected provider
+ expect(log?.usedProvider).toBe("zai"); // or "novita" depending on routing logic
expect(log?.usedProvider).not.toBe("alibaba");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect(res.status).not.toBe(400); | |
| const logs = await waitForLogs(1); | |
| const log = | |
| logs.find((entry) => entry.requestedModel === "glm-4.6") ?? logs.at(-1); | |
| expect(log).toBeTruthy(); | |
| expect(log?.usedProvider).not.toBe("alibaba"); | |
| expect(log?.usedModel).not.toBe("alibaba/glm-4.6:cn-beijing"); | |
| expect(log?.routingMetadata?.providerScores).not.toContainEqual( | |
| expect.objectContaining({ | |
| providerId: "alibaba", | |
| region: "cn-beijing", | |
| }), | |
| ); | |
| }); | |
| expect(res.status).toBe(200); | |
| const logs = await waitForLogs(1); | |
| const log = | |
| logs.find((entry) => entry.requestedModel === "zai/glm-4.6") ?? logs.at(-1); | |
| expect(log).toBeTruthy(); | |
| // Add positive assertion for expected provider | |
| expect(log?.usedProvider).toBe("zai"); // or "novita" depending on routing logic | |
| expect(log?.usedProvider).not.toBe("alibaba"); | |
| expect(log?.usedModel).not.toBe("alibaba/glm-4.6:cn-beijing"); | |
| expect(log?.routingMetadata?.providerScores).not.toContainEqual( | |
| expect.objectContaining({ | |
| providerId: "alibaba", | |
| region: "cn-beijing", | |
| }), | |
| ); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/gateway/src/fallback.spec.ts` around lines 1123 - 1137, The test's log
lookup can miss the entry because the request used model "zai/glm-4.6" while the
find checks for requestedModel === "glm-4.6"; update the lookup in the test (the
logs/find block that uses requestedModel) to either match the full prefixed
model string ("zai/glm-4.6") or normalize both sides (e.g., strip provider
prefix) so the correct log is found, replace the weak response assertion
expect(res.status).not.toBe(400) with a strict expect(res.status).toBe(200) to
ensure success, and add a positive assertion for the chosen provider (e.g.,
expect(log?.usedProvider).toBe("zai") or the expected provider) instead of only
negative checks to guarantee the intended routing behavior; adjust references to
waitForLogs, logs, log, requestedModel, res, and usedProvider accordingly.
|
Follow-up update pushed in This fixes a second routing hole in Validation run after the patch:
Note: the fresh build initially failed on a corrupted generated file at |
|
Added follow-up test fix in The
That makes the test actually exercise the Validation:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e5a984a9d
ℹ️ 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".
| project.mode === "credits" | ||
| ? filterRegionsByAvailableKeys( |
There was a problem hiding this comment.
The auto-routing candidate setup only applies filterRegionsByAvailableKeys(...) when project.mode === "credits", while the non-credits branch expands all regions unconditionally. In api-keys and hybrid modes this bypasses the region restrictions enforced elsewhere (DB-key locked regions and hybrid env-key limits), so auto routing can choose a provider region the project is not configured to use, leading to wrong-region routing or avoidable auth failures for region-bound credentials.
Useful? React with 👍 / 👎.
| return true; | ||
| }); | ||
| const availableModelProviders = filterEligibleModelProviders( | ||
| preferConcreteRegionalMappings(expandedIamFilteredModelProviders), |
There was a problem hiding this comment.
Low-uptime fallback now derives alternatives from expandedIamFilteredModelProviders, which comes from routingExpandedModelProviders and is not narrowed by the earlier requestedRegion filter. When a caller explicitly requests provider/model:region, fallback can therefore select a different region once uptime drops below threshold, violating the explicit region constraint and potentially sending traffic outside the requested geography.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/gateway/src/chat/chat.ts (3)
249-259:⚠️ Potential issue | 🟠 MajorUse the effective
max_tokensvalue for eligibility, not the raw request field.Lines 301-307 and 1191-1197 still compare the user-supplied
max_tokensbeforeprepareRequestBody()applies provider-specific normalization/defaults. That leaves candidates in the pool that will deterministically hit the later effective-max_tokens400 instead of being filtered out up front, so routing can fail even when another provider would have been valid.Also applies to: 301-307, 1191-1197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/gateway/src/chat/chat.ts` around lines 249 - 259, filterEligibleModelProviders currently uses the raw request max_tokens to decide provider eligibility; instead call or reuse the normalization that prepareRequestBody performs to compute the effective max_tokens and use that when filtering so providers that will end up with a lower default (e.g., 400) are excluded up front. Update filterEligibleModelProviders (and the code paths referenced around prepareRequestBody usage) to obtain the prepared request or the provider-specific effectiveMaxTokens (by invoking prepareRequestBody or its utility) and compare that effective value against ProviderModelMapping limits rather than the original user-supplied max_tokens.
686-692:⚠️ Potential issue | 🔴 CriticalKeep explicit
:regionrequests in sync with the expanded IAM/routing pool.Line 686 seeds
routingExpandedModelProvidersfrom the full expanded set, but therequestedRegionbranch only narrowsmodelInfo.providers. By Lines 978-982,expandedIamFilteredModelProvidersis rebuilt from that stale expanded list, so later fallback/routing paths can reintroduce regions outside the caller's explicit region constraint.Possible fix
if (requestedRegion) { const regionProviders = expandedActiveModelProviders.filter( (p) => p.region === requestedRegion, ); + routingExpandedModelProviders = routingExpandedModelProviders.filter( + (p) => p.region === requestedRegion, + ); modelInfo = { ...modelInfo, providers: regionProviders, };Also applies to: 978-982
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/gateway/src/chat/chat.ts` around lines 686 - 692, The code seeds routingExpandedModelProviders from expandedActiveModelProviders but only narrows modelInfo.providers when requestedRegion is present, causing expandedIamFilteredModelProviders (rebuilt later in expandedIamFilteredModelProviders) to be computed from a stale full list and reintroduce disallowed regions; update the logic so that when requestedRegion is set (and when useExpandedRoutingProviders is true) you also filter routingExpandedModelProviders to the requestedRegion (the same way you narrow modelInfo.providers), and ensure any place that rebuilds expandedIamFilteredModelProviders uses this filtered routingExpandedModelProviders instead of the original expandedActiveModelProviders (adjust references in the blocks that set routingExpandedModelProviders, modelInfo, and expandedIamFilteredModelProviders).
1108-1118:⚠️ Potential issue | 🔴 CriticalAuto-routing still bypasses region locks outside credits mode.
This block rebuilds candidates from raw
modelDef.providersand only appliesfilterRegionsByAvailableKeys()in credits mode. Inhybrid, env-backed providers can still surface non-default regions without region-specific keys, and inapi-keys/hybrida provider with a DB-locked region can still be auto-routed to a different region beforeusedRegionis fixed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/gateway/src/chat/chat.ts` around lines 1108 - 1118, The candidateProviders rebuild currently only applies filterRegionsByAvailableKeys when project.mode === "credits", which allows non-allowed regions to be surfaced in api-keys/hybrid and lets auto-routing pick a different region before usedRegion is fixed; update the logic around preferConcreteRegionalMappings/expandAllProviderRegions so filterRegionsByAvailableKeys is always applied (use modelDef.providers -> expandAllProviderRegions -> filterRegionsByAvailableKeys -> preferConcreteRegionalMappings) and ensure the auto-route/selection code that sets usedRegion rejects providers whose region is DB-locked (respect any existing region lock and only consider providers in that locked region) so usedRegion cannot be changed by auto-routing in api-keys or hybrid modes (refer to candidateProviders, preferConcreteRegionalMappings, filterRegionsByAvailableKeys, expandAllProviderRegions, modelDef.providers, project.mode, and usedRegion).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/gateway/src/chat/chat.ts`:
- Around line 1516-1526: The uptime-improvement gate is applied after
collapseProvidersToBestRegionPerProvider which can drop a provider because its
chosen region is lower uptime; instead, filter candidate regions/providers by
uptime > currentUptime before collapsing so each provider's best-region
selection only considers regions that meet the uptime gate. Update the flow to
run the uptime filter against availableModelProviders (or pass an options flag
into collapseProvidersToBestRegionPerProvider to perform the filter internally)
so collapseProvidersToBestRegionPerProvider returns only providers whose best
region already satisfies the uptime > currentUptime check, then compute
betterUptimeProviders from that result.
---
Duplicate comments:
In `@apps/gateway/src/chat/chat.ts`:
- Around line 249-259: filterEligibleModelProviders currently uses the raw
request max_tokens to decide provider eligibility; instead call or reuse the
normalization that prepareRequestBody performs to compute the effective
max_tokens and use that when filtering so providers that will end up with a
lower default (e.g., 400) are excluded up front. Update
filterEligibleModelProviders (and the code paths referenced around
prepareRequestBody usage) to obtain the prepared request or the
provider-specific effectiveMaxTokens (by invoking prepareRequestBody or its
utility) and compare that effective value against ProviderModelMapping limits
rather than the original user-supplied max_tokens.
- Around line 686-692: The code seeds routingExpandedModelProviders from
expandedActiveModelProviders but only narrows modelInfo.providers when
requestedRegion is present, causing expandedIamFilteredModelProviders (rebuilt
later in expandedIamFilteredModelProviders) to be computed from a stale full
list and reintroduce disallowed regions; update the logic so that when
requestedRegion is set (and when useExpandedRoutingProviders is true) you also
filter routingExpandedModelProviders to the requestedRegion (the same way you
narrow modelInfo.providers), and ensure any place that rebuilds
expandedIamFilteredModelProviders uses this filtered
routingExpandedModelProviders instead of the original
expandedActiveModelProviders (adjust references in the blocks that set
routingExpandedModelProviders, modelInfo, and
expandedIamFilteredModelProviders).
- Around line 1108-1118: The candidateProviders rebuild currently only applies
filterRegionsByAvailableKeys when project.mode === "credits", which allows
non-allowed regions to be surfaced in api-keys/hybrid and lets auto-routing pick
a different region before usedRegion is fixed; update the logic around
preferConcreteRegionalMappings/expandAllProviderRegions so
filterRegionsByAvailableKeys is always applied (use modelDef.providers ->
expandAllProviderRegions -> filterRegionsByAvailableKeys ->
preferConcreteRegionalMappings) and ensure the auto-route/selection code that
sets usedRegion rejects providers whose region is DB-locked (respect any
existing region lock and only consider providers in that locked region) so
usedRegion cannot be changed by auto-routing in api-keys or hybrid modes (refer
to candidateProviders, preferConcreteRegionalMappings,
filterRegionsByAvailableKeys, expandAllProviderRegions, modelDef.providers,
project.mode, and usedRegion).
🪄 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: 7d96ee5a-0d98-4ed1-85c1-9b3e4e4ee51e
📒 Files selected for processing (1)
apps/gateway/src/chat/chat.ts
| const providerAgnosticCandidates = | ||
| collapseProvidersToBestRegionPerProvider( | ||
| availableModelProviders, | ||
| modelWithPricing, | ||
| { metricsMap: allMetricsMap, isStreaming: stream }, | ||
| ); | ||
|
|
||
| // Filter to only providers with better uptime than the original | ||
| // to avoid falling back to worse providers | ||
| const betterUptimeProviders = availableModelProviders.filter( | ||
| const betterUptimeProviders = providerAgnosticCandidates.filter( | ||
| (p) => { |
There was a problem hiding this comment.
Apply the uptime-improvement gate before collapsing per provider.
Here the code picks one region per provider first and only then checks uptime > currentUptime. If the scoring helper chooses a cheaper but less healthy region, that provider is dropped entirely even when another region for the same provider would have satisfied the fallback rule.
Possible fix
- const providerAgnosticCandidates =
- collapseProvidersToBestRegionPerProvider(
- availableModelProviders,
- modelWithPricing,
- { metricsMap: allMetricsMap, isStreaming: stream },
- );
-
- const betterUptimeProviders = providerAgnosticCandidates.filter(
- (p) => {
+ const betterUptimeRegionalCandidates =
+ availableModelProviders.filter((p) => {
const providerMetrics = allMetricsMap.get(
metricsKey(modelWithPricing.id, p.providerId, p.region),
);
- // If no metrics, assume the provider is healthy (100% uptime)
- // If has metrics, only include if uptime is better than original
return (
!providerMetrics ||
(providerMetrics.uptime ?? 100) > currentUptime
);
- },
- );
+ });
+ const betterUptimeProviders =
+ collapseProvidersToBestRegionPerProvider(
+ betterUptimeRegionalCandidates,
+ modelWithPricing,
+ { metricsMap: allMetricsMap, isStreaming: stream },
+ );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/gateway/src/chat/chat.ts` around lines 1516 - 1526, The
uptime-improvement gate is applied after
collapseProvidersToBestRegionPerProvider which can drop a provider because its
chosen region is lower uptime; instead, filter candidate regions/providers by
uptime > currentUptime before collapsing so each provider's best-region
selection only considers regions that meet the uptime gate. Update the flow to
run the uptime filter against availableModelProviders (or pass an options flag
into collapseProvidersToBestRegionPerProvider to perform the filter internally)
so collapseProvidersToBestRegionPerProvider returns only providers whose best
region already satisfies the uptime > currentUptime check, then compute
betterUptimeProviders from that result.
Summary by CodeRabbit
Bug Fixes
Improvements
Tests