Repository navigation
Conversation
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughBare model routing now evaluates expanded regional candidates before using the single-provider shortcut. Managed-credential tests cover default-region fallback, cheapest eligible region, provider-key pinning, and provider selection. ChangesBare model regional routing
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
A bare model id (no provider prefix, no `:region` suffix) for a model whose only provider is regional - e.g. qwen3.7-plus, qwen3.7-flash and the other latest Alibaba models - keeps `modelInfo.providers` un-expanded: one Alibaba entry whose region is undefined, with the concrete regions still inside its `regions` array. The single-provider shortcut then pinned `usedRegion = undefined` and the request fell through to the provider's default region, silently skipping the cheapest-region routing that the `provider/model` spelling gets. Gate the shortcut on the expanded candidate count: a single provider with several regions now flows through the normal routing branch, where `collapseProvidersToBestRegionPerProvider` prices the regional variants against each other and resolves a concrete, credentialed region. Genuinely single-candidate models (no `regions` array) keep the shortcut, unchanged. Also corrects bare ids ignoring provider-key region pins (e.g. `alibaba_region: cn-beijing`), which the shortcut previously bypassed and the routing branch honors. Tests: managed-credentials spec gains bare-id routing cases - cheapest region selection (singapore + eu-frankfurt credentials), default-region- only fallback, BYOK provider-key region pin (alibaba_region: cn-beijing), and multi-provider routing (alibaba+novita+deepinfra catalog with only novita credentialed). The theopenco#3528 default-region regression test now uses qwen-omni-turbo (no regional variants).
5571d58 to
95466d9
Compare
|
@coderabbitai review |
|
|
Thanks for digging into this — the underlying observation is real and it's now tracked as #3530, but this PR's framing and two of its claims don't hold up, so I'm closing it in favour of #3540, which lands the same one-line gate with a corrected description and a test that's verified red-before/green-after. I checked all of this against a running gateway on The 🔴 URGENT claim does not reproduce. Bare Alibaba ids work fine on coding plans:
#3528 already covers the region-less case: a mapping with no region is judged on the provider's default region, so the singapore credential serves it. Nobody is locked out, and there is nothing to hotfix. The provider-key region pin was never ignored. With a BYOK key carrying The one configuration that genuinely cannot route isn't fixed here. If the deployment has managed Alibaba credentials but none for the default region (I seeded an What is real is exactly what #3530 describes: bare ids skip cheapest-region selection. Measured on the same request with |
|
@steebchen as long it's getting fixed quickly, I am good. |
🔴 URGENT
Since the Alibaba EU-Frankfurt region update (#3525), all standard-routing (bare model id) requests to Alibaba models are broken for coding/DevPass plan users.
qwen3.7-flash,qwen3.7-plus,qwen3-vl-flash,qwen3.6-flashand the other latest Alibaba models are Alibaba-only — there is no fallback provider, and on coding plans neitheralibaba/prefix (403) nor:regionsuffix (400) is allowed. These users have no workaround; they cannot route to Alibaba models at all until this ships. Please prioritize review + release.Problem
After the Alibaba EU-Frankfurt region update (#3525) and the region-less managed-credentials fix (#3528), bare Alibaba model ids (
qwen3.7-flash,qwen3.7-plus, and other latest Alibaba models — noalibaba/prefix, no:regionsuffix) still skip region routing entirely.A bare id makes
useExpandedRoutingProvidersfalse, so the catalogue keeps the un-expanded provider mapping (one Alibaba entry,region: undefined). The single-provider shortcut inchat.ts(iamFilteredModelProviders.length === 1) then pinsusedRegion = undefined, and the request falls through to the provider's default region. The region-expanded, credential-filtered list that the multi-provider branch prices over is never consulted.On coding/DevPass plans there is no workaround: provider prefixes are 403 ("Direct provider routing is not available on coding plans") and
:regionsuffixes are 400. So standard routing (bare ids) is the only path those users have — and it cannot resolve Alibaba's per-region routing.Fix
Gate the single-provider shortcut on the expanded candidate count instead of the provider-level count:
if (iamFilteredModelProviders.length === 1)→if (expandedIamFilteredModelProviders.length === 1)(shortcut site)A single provider with several regions (Alibaba with singapore/eu-frankfurt/us-virginia/cn-beijing) now flows through the normal routing branch, where
collapseProvidersToBestRegionPerProviderprices regions against each other and resolves a concrete, credentialed region. Genuinely single-candidate models (noregionsarray) keep the shortcut, unchanged.Also corrects: bare ids + a provider-key region pin (e.g.
alibaba_region: cn-beijing) now honor the pin, since the routing branch appliesbuildProviderLockedRegions; the shortcut previously ignored it.Impact
Checked for the shared regional providers: google-vertex, aws-mantle, aws-bedrock (shared credentials across regions), azure (region-scoped, filtered at the same pre-routing step). service_tier narrowing updates both candidate lists in lockstep. Session stickiness is preserved on both paths.
Tests
apps/gateway/src/chat/managed-credentials.spec.ts(21/21 pass):qwen3.7-pluswith singapore + eu-frankfurt credentials routes to eu-frankfurt (cheapest), asserting upstream host + credentialqwen3.7-pluswith only the default-region (singapore) credential still resolves to it via the routing branchqwen3.7-pluswith a BYOK key pinned viaoptions: { alibaba_region: "cn-beijing" }routes to cn-beijing — the pin the shortcut previously bypassed is honoredqwen3-vl-235b-a22b-instruct(alibaba + novita + deepinfra catalog) with only a novita credential routes to novitaqwen-omni-turbo(no regional variants).pnpm build— passes (17/17 tasks).pnpm test:unit— 0 failures acrossapps/gateway; the only repo-wide failures areapps/apiStripe/wallet/webhook specs that require a Postgres test DB + external services not provisioned in this environment (pre-existing, unrelated to this change).Tested against upstream/main @ e8b84aa.
Summary by CodeRabbit
Bug Fixes
Tests