fix(router): route unlisted models to wildcard external workers under IGW (#1871)#1896
fix(router): route unlisted models to wildcard external workers under IGW (#1871)#1896slin1237 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughRouter manager adds a fallback in ChangesIGW Wildcard External Model Routing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Test
participant Gateway
participant RouterManager
participant WorkerRegistry
participant OpenAIProxyRouter
Test->>Gateway: start(worker_urls, igw_mode=True)
Gateway->>RouterManager: initialize IGW routing
Test->>RouterManager: select_router_for_request(model)
RouterManager->>WorkerRegistry: get_by_model(model)
WorkerRegistry-->>RouterManager: no indexed workers
RouterManager->>WorkerRegistry: scan External workers supporting_model(model)
WorkerRegistry-->>RouterManager: wildcard external worker
RouterManager->>OpenAIProxyRouter: route request
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request enables support for mixed self-hosted and external workers under the Inference Gateway (IGW), resolving issue #1871. When a requested model is not explicitly indexed, the gateway now falls back to wildcard external workers that support the model instead of defaulting to the local router. The changes include updates to the gateway test infrastructure, a new integration test, and a unit test in Rust. The review feedback suggests optimizing the worker lookup in RouterManager by using get_workers_filtered instead of get_all() to avoid cloning all registered workers on the hot path.
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.
| self.worker_registry | ||
| .get_all() | ||
| .into_iter() | ||
| .filter(|w| { | ||
| matches!(w.metadata().spec.runtime_type, RuntimeType::External) | ||
| && w.supports_model(model) | ||
| }) | ||
| .collect() |
There was a problem hiding this comment.
Using get_all() on the worker registry clones the Arc<dyn Worker> for every single registered worker, which can be quite expensive on the hot path of request routing. Since we only care about external workers here, we can use get_workers_filtered to only retrieve and clone the external workers, significantly improving efficiency.
self.worker_registry
.get_workers_filtered(None, None, None, Some(RuntimeType::External), false)
.into_iter()
.filter(|w| w.supports_model(model))
.collect()There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21bb672a3e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| .filter(|w| { | ||
| matches!(w.metadata().spec.runtime_type, RuntimeType::External) | ||
| && w.supports_model(model) | ||
| }) |
There was a problem hiding this comment.
Infer provider before wildcard external fallback
In an IGW BYOK setup with more than one keyless external provider (for example --worker-urls https://api.openai.com https://api.anthropic.com), this fallback collects every wildcard external worker because supports_model() is true for all of them. Those wildcard workers have no default provider until discovery, and the downstream provider filter treats all-None providers as a single-provider set, so a gpt-* request can be selected onto the Anthropic worker and forward the caller's OpenAI bearer there. Please preserve/infer the provider from the worker URL or filter the fallback candidates by inferred provider/model before returning them.
Useful? React with 👍 / 👎.
… IGW In IGW mode, `select_router_for_request` resolves a request's candidate workers via the registry's per-model index (`get_by_model`), which only holds workers' explicitly-listed models. A wildcard external worker (e.g. `--worker-urls https://api.openai.com` with no key) isn't indexed, so an unlisted model like `gpt-4o-mini` matched no worker and fell back to the default local router — returning `model_not_found` instead of dispatching to the OpenAI proxy router (discussion #1871). When the index lookup is empty, fall back to external workers that support the model, so the request dispatches to the provider proxy router (which forwards the caller's bearer / BYOK and strips SGLang-only fields). Indexed (explicit) workers still win, so a local model never leaks to a wildcard. Also lets the e2e harness start `--enable-igw` together with `--worker-urls`. Refs #1871 Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
21bb672 to
f3b6068
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3b6068e08
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| matches!(w.metadata().spec.runtime_type, RuntimeType::External) | ||
| && w.supports_model(model) |
There was a problem hiding this comment.
Guard wildcard fallback from local startup misses
When IGW is started with both local workers and a keyless external wildcard, startup only submits InitializeWorkersFromConfig in the background (server.rs) and the job queue registers workers asynchronously, so the external worker can be present before the local model is indexed. In that window, a request for the local model sees an empty model index and this predicate matches the wildcard because supports_model() is true for any model, sending the request through the external proxy with the caller's auth instead of treating the local worker as not ready. Please restrict the fallback by inferred provider/model family or otherwise avoid using wildcard externals for local-looking models while configured local workers are still registering.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e_test/infra/gateway.py (1)
45-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocstring doesn't mention the new mixed IGW + worker_urls startup path.
Worth a line noting
start(igw_mode=True, worker_urls=[...])is now supported for registering self-hosted + external workers at launch, since that's the mode this PR adds and tests.📝 Suggested docstring update
Four startup modes: - Regular: start(worker_urls=[...], model_path="...") - PD: start(prefill_workers=[...], decode_workers=[...]) - - IGW: start(igw_mode=True), then add_worker(url) dynamically + - IGW: start(igw_mode=True), then add_worker(url) dynamically, or + start(igw_mode=True, worker_urls=[...]) to register workers at launch - Cloud: start(cloud_backend="openai"|"xai"|"anthropic")🤖 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 `@e2e_test/infra/gateway.py` around lines 45 - 49, Update the startup mode docstring in the gateway helper to document the new mixed IGW launch path. In the `start(...)` docstring, add a line for `start(igw_mode=True, worker_urls=[...])` to show that self-hosted and external workers can be registered at launch, alongside the existing `start`, PD, IGW, and Cloud examples.
🤖 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 `@model_gateway/src/routers/router_manager.rs`:
- Around line 326-341: The fallback in `RouterManager::get_target_workers` (the
`indexed.is_empty()` branch) is doing a full `worker_registry.get_all()` scan
and clone on every unindexed-model request, which is too expensive on the hot
path. Replace this linear search with a small secondary cache/index for wildcard
`RuntimeType::External` workers, and update it when workers are registered or
deregistered so the request path can look up matching external workers without
scanning the full registry.
---
Outside diff comments:
In `@e2e_test/infra/gateway.py`:
- Around line 45-49: Update the startup mode docstring in the gateway helper to
document the new mixed IGW launch path. In the `start(...)` docstring, add a
line for `start(igw_mode=True, worker_urls=[...])` to show that self-hosted and
external workers can be registered at launch, alongside the existing `start`,
PD, IGW, and Cloud examples.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: f04790be-938d-4292-9f87-2f4ebde8b1e4
📒 Files selected for processing (3)
e2e_test/infra/gateway.pye2e_test/router/test_worker_api.pymodel_gateway/src/routers/router_manager.rs
| let indexed = self.worker_registry.get_by_model(model).to_vec(); | ||
| if indexed.is_empty() { | ||
| // Wildcard external workers (e.g. `--worker-urls https://api.openai.com` | ||
| // with no key) accept any model but aren't in the per-model index, so | ||
| // `get_by_model` misses them. Fall back to external workers that support | ||
| // the model, so the request dispatches to the provider proxy router | ||
| // instead of the default local router. Indexed (explicit) workers win, | ||
| // so a local model never falls through to a wildcard. (#1871) | ||
| self.worker_registry | ||
| .get_all() | ||
| .into_iter() | ||
| .filter(|w| { | ||
| matches!(w.metadata().spec.runtime_type, RuntimeType::External) | ||
| && w.supports_model(model) | ||
| }) | ||
| .collect() |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift
Full-registry scan on every unindexed-model request.
When get_by_model misses, this scans and clones every worker in the registry (get_all()) on the hot request path for each unlisted model — this happens for every gpt-*-style call, not just at startup. At small worker counts this is fine, but it doesn't scale with registry size. Consider maintaining a small secondary index/cache of wildcard RuntimeType::External workers (updated on register/deregister) instead of a linear scan per request.
🤖 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 `@model_gateway/src/routers/router_manager.rs` around lines 326 - 341, The
fallback in `RouterManager::get_target_workers` (the `indexed.is_empty()`
branch) is doing a full `worker_registry.get_all()` scan and clone on every
unindexed-model request, which is too expensive on the hot path. Replace this
linear search with a small secondary cache/index for wildcard
`RuntimeType::External` workers, and update it when workers are registered or
deregistered so the request path can look up matching external workers without
scanning the full registry.
Description
Problem
In IGW mode, starting with
--enable-igw --worker-urls <local-sglang> https://api.openai.com(no key) registers OpenAI as a wildcard external worker, but agpt-*request returned404 model_not_found— it was never dispatched to the OpenAI proxy router (discussion #1871).Root cause:
select_router_for_requestresolves a request's candidate workers via the registry's per-model index (get_by_model), which only contains each worker's explicitly-listed models. A wildcard external worker has none, so for an unlisted model the lookup is empty → dispatch falls back to the default (local) router instead of the external→provider-router branch inselect_router_for_workers.Solution
When the per-model index is empty, fall back to external workers that support the model, so the request dispatches to the OpenAI/Anthropic/Gemini proxy router — which forwards the caller's bearer (BYOK) and strips SGLang-only fields. Indexed (explicit) workers still win, so a locally-served model never routes to a wildcard.
First of a few focused PRs from discussion #1871. Follow-ups: warn + auto-enable IGW when
--worker-urlsincludes an external provider; stop--backend openaifrom classifying self-hosted workers as external.Changes
model_gateway/.../routers/router_manager.rs: wildcard-external fallback inselect_router_for_request, plus a unit test asserting dispatch by router identity (Arc::ptr_eq).e2e_test/infra/gateway.py: allow--enable-igwtogether with--worker-urls.e2e_test/router/test_worker_api.py: gated e2e for the mixed--enable-igw --worker-urlspath.Test Plan
Unit:
gpt-4o-mini(served only by the wildcard external worker) → OpenAI router; a locally-served model → regular router (never the wildcard).e2e —
TestIGWWorkerUrlsMixedExternal(gated onOPENAI_API_KEY,@gpu(1)@engine(sglang)): starts--enable-igw --worker-urls <local-sglang> https://api.openai.com, asserts the OpenAI URL registers asexternal, and agpt-4o-minichat carrying the caller'sAuthorization: Bearer(BYOK) returns 200.Checklist
cargo +nightly fmtpassescargo clippy -- -D warningsclean on the changed crateSummary by CodeRabbit
Bug Fixes
Tests