Repository navigation
Add weighted multi-model routing - #107
Conversation
- Preserve legacy single-upstream rules while supporting ordered or weighted route candidates with independent enable switches. - Automatically group cloud catalog variants only when region, declared cost and capability metadata match; saved manual routes take precedence. - Add route failover, management metadata, WebUI editing, documentation and regression coverage. Tests: - for s in tests/test_*.py; do uv run python -B "$s"; done - cd web && vp check - git diff --check
Reviewer's GuideIntroduces backward-compatible multi-model routing with ordered or weighted candidates, independent route enablement, conservative automatic cloud grouping, route-level inference failover, management/WebUI support, and bilingual documentation. Sequence diagram for weighted route selection and failoversequenceDiagram
participant Client
participant Gateway
participant Policy as model_policy
participant UpstreamA
participant UpstreamB
Client->>Gateway: POST chat request
Gateway->>Policy: resolve(public_model)
Policy-->>Gateway: route_candidates
Gateway->>Policy: route_candidates(public_model)
Policy-->>Gateway: weighted primary plus ordered fallbacks
Gateway->>UpstreamA: Inference request
alt Primary route succeeds
UpstreamA-->>Gateway: Response or stream
Gateway-->>Client: Response or stream
else Primary route fails
Gateway->>Gateway: _route_retry_attempt
Gateway->>Policy: bind_route(next upstream)
Gateway->>UpstreamB: Inference retry
UpstreamB-->>Gateway: Response or stream
Gateway-->>Client: Response or stream
end
Entity relationship diagram for model routeserDiagram
MODEL_RULE ||--o{ MODEL_ROUTE : contains
MODEL_RULE {
string public_id
string upstream_id
boolean enabled
}
MODEL_ROUTE {
string upstream_id
float weight
boolean enabled
string source
}
Flow diagram for automatic cloud route groupingflowchart TD
CatalogVariants[Cloud catalog variants] --> Signature[_catalog_model_signature]
Signature --> Match{Region and capability signature match}
Match -->|yes, multiple variants| Group[_build_catalog_route_groups]
Match -->|no| Separate[Keep variants separate]
Group --> AutoRoute[Automatic cloud routes]
Manual[Saved manual routes] --> Override{Manual routes exist}
AutoRoute --> Override
Override -->|yes| Manual
Override -->|no| AutoRoute
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 18 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="converter.py" line_range="4009-4012" />
<code_context>
- body["model"] = model_policy.resolve(CONFIG, body.get("model", "auto"))
+ public_model = body.get("model", "auto")
+ body["model"] = model_policy.resolve(CONFIG, public_model)
+ route_candidates = model_policy.route_candidates(CONFIG, public_model)
+ if len(route_candidates) > 1:
+ body["_route_candidates"] = route_candidates
+ body["_route_index"] = 0
guard_model(body["model"], region=region, resolved=True)
messages = body.get("messages")
</code_context>
<issue_to_address>
**Model restrictions are bypassed**
When a caller supplies `_route_candidates` containing a disallowed model and the initial upstream attempt fails, `_next_route_body` reads the caller-supplied `_route_candidates`, and `_route_retry_attempt` passes the next model directly to `_route_chat` without resolving it or checking the API key’s model allowlist. The caller can therefore make a restricted key invoke a disallowed model after the initial upstream attempt fails.
Remove caller-supplied internal routing fields before normalization, and validate every retry candidate against the request’s resolved policy and API-key model restrictions.
</issue_to_address>
### Comment 2
<location path="converter.py" line_range="5398" />
<code_context>
await _close_stream(stream) # Release the failed upstream connection.
release_credential(cred)
recovered = observe_failure_seq() # Recover only this failure sequence.
+ next_body, attempt = await run_in_threadpool(_route_retry_attempt, payload, canonical, rid)
+ if attempt is not None:
+ canonical = next_body
</code_context>
<issue_to_address>
**Unsafe retries duplicate inference**
When an upstream failure is filtered or `_failover_safe` considers it unsafe to replay, and another route is configured, `_stream_plan` and `_routed_fetch` call `_route_retry_attempt` before checking `_failover_safe`, so a filtered request or one already accepted upstream is submitted to another model. The caller can receive a different completion or incur duplicate processing and charges.
Check `_failover_safe` before attempting another model route.
Also at `converter.py:5454-5461`.
</issue_to_address>
### Comment 3
<location path="app/model_policy.py" line_range="200" />
<code_context>
if name in (embeddings.MODEL_ID, embeddings.PUBLIC_MODEL_ID):
return embeddings.MODEL_ID
+ aliases = config.get("model_route_aliases") or {}
+ if name in aliases and name not in snapshot(config).get("models", {}):
+ return aliases[name]
data = snapshot(config)
</code_context>
<issue_to_address>
**Catalog aliases bypass grouped routing**
When a caller requests a noncanonical catalog variant assigned to a canonical route group, `resolve` derives the source from saved policies instead of applying the automatic alias mapping, so the variant becomes its own route candidate and skips the canonical group’s weighted selection and failover. When the canonical model is disabled, the alias can also remain callable through its default enabled rule.
Resolve automatic aliases to their canonical source before building route candidates, while preserving explicit model rules.
Also at `app/model_policy.py:262-270`, `converter.py:2310-2311`.
</issue_to_address>
### Comment 4
<location path="converter.py" line_range="4010" />
<code_context>
+ public_model = body.get("model", "auto")
+ body["model"] = model_policy.resolve(CONFIG, public_model)
+ route_candidates = model_policy.route_candidates(CONFIG, public_model)
+ if len(route_candidates) > 1:
+ body["_route_candidates"] = route_candidates
+ body["_route_index"] = 0
</code_context>
<issue_to_address>
**Usable fallback routes are skipped**
When the initially selected route has no eligible credential or fails capability preflight while a later route is usable, `_route_chat` selects credentials before the retry handling in `_stream_plan` and `_routed_fetch`; when that selection or preflight raises, the request exits without trying later candidates, and the caller receives an error despite an available route.
Handle credential-selection and preflight failures by trying the remaining enabled route candidates before returning an error.
Also at `converter.py:5374-5377`, `converter.py:5398-5403`, `converter.py:5454-5460`.
</issue_to_address>
### Comment 5
<location path="converter.py" line_range="4012" />
<code_context>
+ route_candidates = model_policy.route_candidates(CONFIG, public_model)
+ if len(route_candidates) > 1:
+ body["_route_candidates"] = route_candidates
+ body["_route_index"] = 0
guard_model(body["model"], region=region, resolved=True)
messages = body.get("messages")
</code_context>
<issue_to_address>
**Internal routing fields reach upstream**
When a request has more than one enabled route, `_prepare_chat_body` adds `_route_candidates` and `_route_index` to the request body, and `_route_chat` carries those fields through to the upstream request without removing them. Upstreams receive gateway-only fields and can reject otherwise valid chat requests.
Keep route retry state outside the upstream body, or strip these internal fields before sending the request.
</issue_to_address>
### Comment 6
<location path="app/control_store.py" line_range="90-91" />
<code_context>
+ weighted = True
+ normalized_routes.append({"upstream_id": route_id, "weight": weight,
+ "enabled": enabled, "source": source_kind})
+ if weighted and not any((route["weight"] or 0) > 0 for route in normalized_routes):
+ raise ValueError("加权路由至少需要一条正权重路线")
+ clean["routes"] = normalized_routes
if type(clean["custom"]) is not bool or (clean["custom"] and clean["keep_original"]):
</code_context>
<issue_to_address>
**Enabled routes can become unavailable**
When a weighted route list has a positive-weight disabled entry and only zero-weight enabled entries, `validate_model` counts the disabled route’s positive weight, so the rule passes validation; `_route_candidates_for_rule` then removes that route and returns no candidates because the enabled routes total zero, leaving the model unavailable.
Require at least one enabled route to have positive weight when validating a weighted route list.
Also at `app/model_policy.py:146-147`, `web/src/pages/Models.tsx:86`.
</issue_to_address>
### Comment 7
<location path="app/model_policy.py" line_range="155-156" />
<code_context>
+ cursor = 0
+ selected = 0
+ for index, weight in enumerate(weights):
+ cursor += int(round(weight * 1000))
+ if offset < cursor:
+ selected = index
+ break
</code_context>
<issue_to_address>
**Weighted routing ignores configured proportions**
When fractional weights are configured, including small positive weights or equal weights such as 0.5 and 0.5, `_route_candidates_for_rule` rounds weights to thousandths and advances a shared counter in 1000-unit steps, so offsets can repeatedly select the same route and rounding can select a zero-weight route. Requests for other models also change this model’s counter phase.
Select routes using unrounded cumulative weights and a per-model selection cycle that honors the configured proportions.
Also at `app/model_policy.py:148-151`.
</issue_to_address>
### Comment 8
<location path="app/model_policy.py" line_range="336" />
<code_context>
upstream = rule["upstream_id"]
- item = known.get(upstream)
+ route_names = route_ids(config, source, policy)
+ item = next((known.get(route) for route in route_names if known.get(route) is not None), None)
if not rule["enabled"] or item is None:
continue
</code_context>
<issue_to_address>
**Usable fallback models appear unavailable**
When the primary upstream has no eligible account but another enabled route is servable, `public_details` and the admin preview or inventory check credentials and metadata against only `rule['upstream_id']`, so they omit the model or report it unavailable despite a usable route; published pricing can also describe the wrong upstream.
Evaluate route availability and published metadata across the configured routes instead of only the primary upstream.
Also at `app/gateway_management.py:378-380`.
</issue_to_address>
### Comment 9
<location path="web/src/pages/Models.tsx" line_range="292" />
<code_context>
revision,
public_id: rule.public_id,
upstream_id: rule.upstream_id,
+ ...(rule.routes?.length ? { routes: rule.routes } : {}),
enabled: rule.enabled,
keep_original: rule.keep_original,
</code_context>
<issue_to_address>
**Automatic route groups become stale**
When an administrator saves an edit to a model with automatically grouped routes and a later catalog refresh changes that group, `ModelEditor` submits the displayed cloud routes as a saved rule, and saved routes take precedence over automatic groups, so future catalog additions or removals do not update the model’s routes.
Keep automatically generated routes distinguishable from manual routes and omit them from persisted edits unless the operator explicitly configures them.
Also at `app/gateway_management.py:378-379`.
</issue_to_address>
### Comment 10
<location path="converter.py" line_range="2307-2310" />
<code_context>
+ if len(ordered) < 2:
+ continue
+ canonical = ordered[0]
+ routes[canonical] = [{"upstream_id": model_id, "weight": None,
+ "enabled": True, "source": "cloud"} for model_id in ordered]
+ for model_id in ordered[1:]:
+ aliases[model_id] = canonical
+ return routes, aliases
+
</code_context>
<issue_to_address>
**Catalog groups overwrite each other**
When the same model ID is the first ID in multiple signature groups, including groups from different regions, `_build_catalog_route_groups` stores each group under its canonical model ID, so a later group replaces the earlier route list while aliases from both groups still point to that ID; requests can then use the wrong variants.
Keep distinct signature and region groups separate when storing route groups and resolving their aliases.
</issue_to_address>
### Comment 11
<location path="converter.py" line_range="4009-4011" />
<code_context>
- body["model"] = model_policy.resolve(CONFIG, body.get("model", "auto"))
+ public_model = body.get("model", "auto")
+ body["model"] = model_policy.resolve(CONFIG, public_model)
+ route_candidates = model_policy.route_candidates(CONFIG, public_model)
+ if len(route_candidates) > 1:
+ body["_route_candidates"] = route_candidates
+ body["_route_index"] = 0
guard_model(body["model"], region=region, resolved=True)
</code_context>
<issue_to_address>
**Non-chat endpoints skip route dispatch**
When a multi-route rule is used for an embeddings model, `_prepare_chat_body` adds route candidates only to chat-style requests, while endpoints such as embeddings resolve the model and call their inference path directly; a manual multi-route rule on those endpoints uses only the first upstream and never performs route selection or route-level failover.
Either implement route selection and fallback in the other inference handlers or reject routes for model types those handlers do not support.
</issue_to_address>
### Comment 12
<location path="web/src/pages/Models.tsx" line_range="70-71" />
<code_context>
return "对外 ID 不能包含空白字符";
if (!validId(rule.public_id)) return "对外 ID 只允许 1–160 位字母、数字及 _ . : / @ -";
if (!validId(rule.upstream_id ?? rule.id)) return "上游 ID 不能为空,且须为有效模型标识";
+ if (rule.routes) {
+ if (!rule.routes.length) return "路由列表不能为空,删除全部候选请停用模型";
+ const routeIds = new Set<string>();
+ let hasWeight = false;
</code_context>
<issue_to_address>
**Legacy model rules cannot be saved**
When an existing model has no routes configured, `ModelEditor` initializes missing routes to `[]`, and JavaScript treats that array as truthy. `validateRule` therefore rejects every legacy rule with an empty routes list, preventing both preview and save in the model editor.
Only reject an empty route list when the user has explicitly enabled route-list editing, or keep absent routes as `undefined` in editor state.
Also at `web/src/pages/Models.tsx:243-246`.
</issue_to_address>
### Comment 13
<location path="app/model_policy.py" line_range="199-200" />
<code_context>
return "auto"
if name in (embeddings.MODEL_ID, embeddings.PUBLIC_MODEL_ID):
return embeddings.MODEL_ID
+ aliases = config.get("model_route_aliases") or {}
+ if name in aliases and name not in snapshot(config).get("models", {}):
+ return aliases[name]
data = snapshot(config)
</code_context>
<issue_to_address>
**Automatic aliases shadow configured names**
When a catalog refresh adds an automatic alias equal to a persisted rule’s public ID, while that ID is not itself a rule key, `source_for()` returns the automatic alias mapping before checking persisted rules’ `public_id` values, and its guard checks only whether the name is a rule key. A configured public model ID matching a catalog alias therefore resolves to the automatic group’s source instead of its owning rule, so callers see the wrong model policy and routes.
Resolve persisted public IDs before applying automatic aliases, or exclude all persisted public IDs from the automatic alias map.
</issue_to_address>
### Comment 14
<location path="app/model_policy.py" line_range="336" />
<code_context>
upstream = rule["upstream_id"]
- item = known.get(upstream)
+ route_names = route_ids(config, source, policy)
+ item = next((known.get(route) for route in route_names if known.get(route) is not None), None)
if not rule["enabled"] or item is None:
continue
</code_context>
<issue_to_address>
**Model metadata misstates route capabilities**
When a manual route list contains upstreams with different capabilities or declared prices, the published metadata is still derived from the single `upstream` model, while manual routes can name models with different capabilities or prices. Clients therefore receive capability and pricing details that do not describe all routes the gateway may select, and can make requests that fail on the selected route.
Publish metadata that accurately describes the route set, or reject route combinations whose capabilities and pricing cannot be represented consistently.
Also at `app/model_policy.py:415`.
</issue_to_address>
### Comment 15
<location path="web/src/pages/Models.tsx" line_range="70-87" />
<code_context>
return "对外 ID 不能包含空白字符";
if (!validId(rule.public_id)) return "对外 ID 只允许 1–160 位字母、数字及 _ . : / @ -";
if (!validId(rule.upstream_id ?? rule.id)) return "上游 ID 不能为空,且须为有效模型标识";
+ if (rule.routes) {
+ if (!rule.routes.length) return "路由列表不能为空,删除全部候选请停用模型";
+ const routeIds = new Set<string>();
+ let hasWeight = false;
+ let hasPositive = false;
+ for (const route of rule.routes) {
+ if (!validId(route.upstream_id) || routeIds.has(route.upstream_id))
+ return "路由上游 ID 必须非空且不重复";
+ routeIds.add(route.upstream_id);
+ if (route.weight !== null) {
+ hasWeight = true;
+ if (!Number.isFinite(route.weight) || route.weight < 0 || route.weight > 1000)
+ return "路由权重必须是 0–1000 的有限数字";
+ if (route.weight > 0) hasPositive = true;
+ }
+ }
+ if (hasWeight && !hasPositive) return "加权路由至少需要一条正权重路线";
</code_context>
<issue_to_address>
**Route controls ignore selected language**
When the WebUI language is set to English, when a user selects English, the new route validation messages and controls in `Models.tsx` still render literal Chinese because they do not use `useT` or translated message keys. English-language users therefore see untranslated text for the new routing feature.
Add typed Chinese and English message keys in `web/src/i18n.tsx` and use `useT` for the new route labels and validation messages.
Also at `web/src/pages/Models.tsx:376-478`.
</issue_to_address>
### Comment 16
<location path="tests/test_weighted_routes.py" line_range="45-46" />
<code_context>
+ {"upstream_id": "second", "weight": 1},
+ ]}
+ seen = [model_policy.route_candidates(config(rule), "aggregate")[0] for _ in range(40)]
+ self.assertGreater(seen.count("first"), seen.count("second"))
+ self.assertEqual(set(seen), {"first", "second"})
+
+ def test_all_disabled_routes_are_unavailable(self):
</code_context>
<issue_to_address>
**Incorrect weight ratios pass**
When the selector returns `first` 39 times and `second` once across these 40 calls, `test_weighted_routes_select_a_weighted_first_candidate` accepts 39 selections of `first` and 1 of `second`: `assertGreater` and the set assertion both pass, despite the configured 3:1 weights.
Assert the expected deterministic counts, or use a tolerance that rejects materially skewed ratios.
</issue_to_address>
### Comment 17
<location path="tests/test_weighted_routes.py" line_range="61-63" />
<code_context>
+ {"upstream_id": "first", "source": "builtin", "enabled": False},
+ {"upstream_id": "second", "source": "cloud", "weight": 2},
+ ]}, known_models=["first", "second"])
+ self.assertEqual([item["upstream_id"] for item in rule["routes"]], ["first", "second"])
+ self.assertFalse(rule["routes"][0]["enabled"])
+ self.assertEqual(rule["routes"][1]["weight"], 2.0)
+
+ def test_validation_rejects_zero_weight_only_and_duplicate_routes(self):
</code_context>
<issue_to_address>
**Route sources go unchecked**
When validation returns incorrect or missing `source` values for the supplied routes, `test_validation_preserves_route_order_and_source` checks route IDs, one enabled flag, and one weight, but never checks either `source`; validation can discard or alter the supplied sources and the test still passes.
Assert that the normalized routes retain the expected `builtin` and `cloud` source values.
</issue_to_address>
### Comment 18
<location path="tests/test_weighted_routes.py" line_range="30" />
<code_context>
def validate_model(source, rule, models=None, known_models=(), *, legacy_scopes=False):
_identifier(source, "模型规则 ID")
</code_context>
<issue_to_address>
**Configured legacy routing goes unchecked**
When a configured legacy rule's `upstream_id` differs from its public model ID and route lookup ignores that rule, `test_legacy_rule_keeps_one_upstream_route` checks only the default `plain`→`plain` result, so it still passes if `route_candidates` ignores configured legacy rules and always returns the public model ID.
Pass a store containing a legacy rule whose `upstream_id` differs from its public model ID, then assert that ID is returned.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 18 findings to address first, and a routing mistake can send request contents to a different configured upstream model, and those requests cannot be recalled after reverting the change. The impact is limited to requests routed to the unintended upstream, but any disclosure has already occurred.
Blocking findings: converter.py:4012, converter.py:5398, app/model_policy.py:200, converter.py:4010, converter.py:4012, and 13 more
| upstream = rule["upstream_id"] | ||
| item = known.get(upstream) | ||
| route_names = route_ids(config, source, policy) | ||
| item = next((known.get(route) for route in route_names if known.get(route) is not None), None) |
There was a problem hiding this comment.
🟡 Medium · Model metadata misstates route capabilities
When a manual route list contains upstreams with different capabilities or declared prices, the published metadata is still derived from the single upstream model, while manual routes can name models with different capabilities or prices. Clients therefore receive capability and pricing details that do not describe all routes the gateway may select, and can make requests that fail on the selected route.
Publish metadata that accurately describes the route set, or reject route combinations whose capabilities and pricing cannot be represented consistently.
Also at app/model_policy.py:415.
Prompt for AI agents
In `app/model_policy.py` at line 336:
**Model metadata misstates route capabilities**
When a manual route list contains upstreams with different capabilities or declared prices, the published metadata is still derived from the single `upstream` model, while manual routes can name models with different capabilities or prices. Clients therefore receive capability and pricing details that do not describe all routes the gateway may select, and can make requests that fail on the selected route.
Publish metadata that accurately describes the route set, or reject route combinations whose capabilities and pricing cannot be represented consistently.
Also at `app/model_policy.py:415`.Sourcery withdrew this approval because the latest commits introduced blocking findings.
Summary
Tests
for s in tests/test_*.py; do uv run python -B "$s"; done— passed.cd web && vp check— formatting, lint and type checks passed.git diff --check— passed.Summary by Sourcery
Enable weighted and ordered multi-model routing with automatic compatible-variant grouping, route failover, management controls, and backward-compatible legacy rules.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: