Conversation
The common/HTTP worker-selection path (`WorkerSelector`) scanned the whole worker map on every request via `get_workers_filtered(None, ...)` then filtered by `supports_model`, making candidate selection O(total fleet). That `None` was deliberate: wildcard workers (those advertising no specific models) accept any model_id but are not indexed under a per-model key, so a naive switch to `get_by_model(model)` would drop them and regress servable requests into `model_not_found`. Add a bounded, wildcard-safe lookup and route the hot path through it: - registry: `get_candidates_for_model(model)` returns the per-model index slice unioned with a tracked set of wildcard workers, deduped by URL. Wildcards are tracked in a `DashSet<WorkerId>` maintained under the per-worker mutation lock in register / replace / remove, keyed on `has_models_discovered()`. Cost is O(per-model + wildcards); wildcards are few. A fast path skips the dedup allocation entirely when no wildcards exist. - common/worker_selection: `get_candidates` / `any_worker_supports_model` use the bounded lookup when the model is known (mirroring the gRPC `UNKNOWN_MODEL_ID` guard), keeping every existing filter (worker_type/connection_mode/runtime_type/availability/provider/ supports_model). Fallback: if the bounded set is empty they fall back to the original full scan, so behavior is never worse (e.g. alias-only matches, or a brief index lag after registration never produce a spurious `model_not_found`). - admin: the `?model`-filtered `list_workers` path uses the same bounded lookup (per-model index + wildcards) instead of `get_all()` + filter; the unfiltered listing stays O(workers), which is inherent for admin. Tests: wildcard worker is selected for an arbitrary model via the bounded path (the core regression); per-model bounding returns the right set and excludes other models; empty-bounded-set fallback (alias-only) still resolves; wildcard membership updates on replace; dedup when a wildcard is also model-indexed. Part of #1692. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
📝 WalkthroughWalkthrough
ChangesBounded Worker Candidate Lookup
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
👋 The PR description doesn't fully follow
Please update the PR description so reviewers have the context they need. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request optimizes worker selection and listing by introducing a bounded, wildcard-safe per-model lookup (get_candidates_for_model) to avoid scanning the entire fleet. Wildcard workers are tracked separately in a DashSet and unioned with model-indexed workers. The feedback identifies a performance optimization in get_candidates_for_model on the hot path: utilizing a HashSet<&str> instead of HashSet<String> to eliminate heap allocations, and collecting wildcard workers into a temporary vector first to avoid holding DashSet shard locks during lookups.
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.
| pub fn get_candidates_for_model(&self, model_id: &str) -> Vec<Arc<dyn Worker>> { | ||
| let indexed = self.get_by_model(model_id); | ||
|
|
||
| // Fast path: no wildcard workers, so the per-model slice is the full | ||
| // candidate set. Avoids the dedup HashSet allocation entirely. | ||
| if self.wildcard_workers.is_empty() { | ||
| return indexed.to_vec(); | ||
| } | ||
|
|
||
| let mut seen: HashSet<String> = HashSet::with_capacity(indexed.len()); | ||
| let mut candidates: Vec<Arc<dyn Worker>> = Vec::with_capacity(indexed.len()); | ||
| for worker in indexed.iter() { | ||
| if seen.insert(worker.url().to_string()) { | ||
| candidates.push(Arc::clone(worker)); | ||
| } | ||
| } | ||
| for entry in self.wildcard_workers.iter() { | ||
| if let Some(worker) = self.get(entry.key()) { | ||
| // A wildcard worker may also be model-indexed (e.g. under a | ||
| // `model_id` label that equals `model_id`); dedup by URL. | ||
| if seen.insert(worker.url().to_string()) { | ||
| candidates.push(worker); | ||
| } | ||
| } | ||
| } | ||
| candidates | ||
| } |
There was a problem hiding this comment.
On the hot path of every request, get_candidates_for_model is called to retrieve candidate workers. The current implementation allocates a new String via worker.url().to_string() for every single candidate worker and wildcard worker to perform deduplication in a HashSet<String>.
We can completely eliminate these heap allocations by using a HashSet<&str> instead. To satisfy the borrow checker, we can first collect the wildcard workers into a temporary Vec to extend their lifetime to the end of the function. This also has the added benefit of avoiding holding the DashSet shard locks on wildcard_workers while performing the self.get() lookups, preventing potential lock contention.
pub fn get_candidates_for_model(&self, model_id: &str) -> Vec<Arc<dyn Worker>> {
let indexed = self.get_by_model(model_id);
// Fast path: no wildcard workers, so the per-model slice is the full
// candidate set. Avoids the dedup HashSet allocation entirely.
if self.wildcard_workers.is_empty() {
return indexed.to_vec();
}
// Collect wildcard workers first to extend their lifetime, allowing us to
// use a zero-allocation HashSet<&str> for deduplication. This also avoids
// holding DashSet shard locks while performing the `self.get()` lookups.
let wildcards: Vec<Arc<dyn Worker>> = self
.wildcard_workers
.iter()
.filter_map(|entry| self.get(entry.key()))
.collect();
let mut seen: HashSet<&str> = HashSet::with_capacity(indexed.len() + wildcards.len());
let mut candidates: Vec<Arc<dyn Worker>> = Vec::with_capacity(indexed.len() + wildcards.len());
for worker in indexed.iter() {
if seen.insert(worker.url()) {
candidates.push(Arc::clone(worker));
}
}
for worker in wildcards.iter() {
if seen.insert(worker.url()) {
candidates.push(Arc::clone(worker));
}
}
candidates
}References
- To avoid holding DashMap shard locks during slow operations, collect the keys and/or values into a temporary collection (e.g., Vec) before iterating and performing the operations.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| let worker_infos: Vec<WorkerInfo> = match model { | ||
| Some(model) => self | ||
| .worker_registry | ||
| .get_candidates_for_model(model) |
There was a problem hiding this comment.
🟡 Nit: get_candidates_for_model returns workers from the per-model index + wildcards, but workers reachable only via a ModelCard alias are not in the per-model index (the test empty_bounded_set_falls_back_to_full_scan in worker_selection.rs explicitly asserts this). Unlike the routing path in get_candidates, the admin endpoint has no fallback to the full scan, so list_workers?model=<alias> will always return an empty list for alias-only models — a behavioral regression from the old code which iterated get_all_with_ids() and checked supports_model on every worker.
If aliases are uncommon in practice this may be acceptable, but worth documenting or adding a full-scan fallback similar to the routing path.
| if req.model_id != UNKNOWN_MODEL_ID { | ||
| let bounded = | ||
| Self::filter_candidates(self.registry.get_candidates_for_model(req.model_id), req); | ||
| if !bounded.is_empty() { | ||
| return bounded; | ||
| } |
There was a problem hiding this comment.
🟡 Nit: The PR description notes that a stale wildcard (one that has since called set_models but stays in wildcard_workers until the next mutation) is "harmless" because supports_model re-filters. That's true downstream in find_best_worker, but the interaction with this early return is subtle: if the bounded set is non-empty only because of stale wildcards (all later rejected by supports_model), and the requested model is reachable only via alias (not indexed), the fallback to full scan never runs and the alias-only worker is never found.
The scenario is narrow (stale wildcard + alias-only model + no indexed workers for the model), but it's the one case where the "never worse" fallback guarantee doesn't hold. Worth noting in the doc comment or adding a supports_model pre-check to the emptiness test:
| if req.model_id != UNKNOWN_MODEL_ID { | |
| let bounded = | |
| Self::filter_candidates(self.registry.get_candidates_for_model(req.model_id), req); | |
| if !bounded.is_empty() { | |
| return bounded; | |
| } | |
| if req.model_id != UNKNOWN_MODEL_ID { | |
| let bounded = | |
| Self::filter_candidates(self.registry.get_candidates_for_model(req.model_id), req); | |
| if bounded.iter().any(|w| w.supports_model(req.model_id)) { | |
| return bounded; | |
| } | |
| } |
This way the fallback triggers when the bounded set has only stale wildcards, at the cost of one extra supports_model pass on the bounded set (which is cheap and already done in find_best_worker).
There was a problem hiding this comment.
Good work — the bounded candidate lookup is well-reasoned and the fallback-to-full-scan approach is a sound safety net. The wildcard tracking via DashSet with mutation-time capture is clean, and the test suite is thorough (wildcard union, dedup, replace lifecycle, alias fallback, filter passthrough).
Two 🟡 nits flagged:
- Admin
list_workers?model=aliasregression — the?modelpath now uses the bounded lookup without a full-scan fallback, so alias-only models won't appear in the listing. - Stale wildcard + alias interaction — when the bounded set is non-empty only due to stale wildcards (which
supports_modelrejects), the full-scan fallback is skipped, and alias-only workers are missed. Narrow scenario but worth tightening the emptiness check.
No 🔴 issues. LGTM.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/common/worker_selection.rs`:
- Around line 119-123: The bounded lookup optimization that filters candidates
by model ID before provider filtering can cause provider-scoped requests to be
routed incorrectly by hiding workers from other providers in the candidate set.
When a provider is specified in the request, the bounded path should be skipped
to allow the full provider-aware filtering logic to work correctly. Modify the
condition that checks `req.model_id != UNKNOWN_MODEL_ID` to also require
`req.provider.is_none()` before taking the bounded path. Apply this same fix to
both occurrences of this pattern (the one shown at startLine 119 and the other
mentioned around line 194-199).
- Around line 120-124: The early return in the bounded candidates check does not
verify that the returned candidates actually support the requested model. Before
returning the bounded set when it is non-empty, add a check to ensure at least
one candidate in bounded supports the model by filtering or checking against
supports_model. If no candidates in the bounded set support the requested model,
allow execution to continue to the full-scan alias fallback mechanism instead of
returning an unsupported candidate set. This prevents stale wildcard entries
from causing premature returns and skipping the fallback that might find a
healthy alias-only worker.
In `@model_gateway/src/worker/service.rs`:
- Around line 368-385: The current implementation of getting worker candidates
for a model in the worker service only uses the literal model index and
wildcards via get_candidates_for_model(model). When this returns an empty
bounded set, workers that support the model only through a ModelCard alias are
missed. Add a fallback mechanism similar to what the router implements: when the
initial get_candidates_for_model(model) call results in an empty set, perform a
full-scan lookup of the worker registry to find workers that support the model
via an alias. This ensures that GET /workers?model=<alias> returns results for
all servable aliases, not just literal model matches.
- Around line 378-383: The code in the worker registry lookup uses
unwrap_or_default() on the result of get_id_by_url, which creates a fabricated
default ID when the worker has been removed from the registry due to a race
condition. Instead of proceeding with this invalid default ID in the
count_and_build call, handle the None case by either skipping the stale
candidate worker entirely with a continue statement or returning an appropriate
error. Only call count_and_build when get_id_by_url successfully returns a valid
worker ID from the registry.
🪄 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: bd2d1a9b-0a6e-487a-a234-0be743100ed1
📒 Files selected for processing (3)
model_gateway/src/routers/common/worker_selection.rsmodel_gateway/src/worker/registry.rsmodel_gateway/src/worker/service.rs
| if req.model_id != UNKNOWN_MODEL_ID { | ||
| let bounded = | ||
| Self::filter_candidates(self.registry.get_candidates_for_model(req.model_id), req); | ||
| if !bounded.is_empty() { | ||
| return bounded; |
There was a problem hiding this comment.
Keep provider-scoped requests on the full provider-aware path.
The bounded lookup shrinks the candidate set before filter_by_provider decides whether a multi-provider filter is needed. In a fleet with an OpenAI worker for the requested model and an Anthropic worker for another model, an Anthropic request can see only the OpenAI worker in bounded, so filter_by_provider treats it as a single-provider set and allows routing to the wrong provider. That violates the SelectWorkerRequest::provider credential-leakage guard used by the Anthropic router. A conservative fix is to skip the bounded path whenever req.provider.is_some() unless provider diversity is tracked registry-wide.
🛡️ Conservative fix
- if req.model_id != UNKNOWN_MODEL_ID {
+ if req.model_id != UNKNOWN_MODEL_ID && req.provider.is_none() {
let bounded =
Self::filter_candidates(self.registry.get_candidates_for_model(req.model_id), req);
if !bounded.is_empty() {
return bounded;
}
@@
- if req.model_id != UNKNOWN_MODEL_ID {
+ if req.model_id != UNKNOWN_MODEL_ID && req.provider.is_none() {
let bounded = Self::healthy_supporting_candidates(
self.registry.get_candidates_for_model(req.model_id),
req,
);Also applies to: 194-199
🤖 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/common/worker_selection.rs` around lines 119 - 123,
The bounded lookup optimization that filters candidates by model ID before
provider filtering can cause provider-scoped requests to be routed incorrectly
by hiding workers from other providers in the candidate set. When a provider is
specified in the request, the bounded path should be skipped to allow the full
provider-aware filtering logic to work correctly. Modify the condition that
checks `req.model_id != UNKNOWN_MODEL_ID` to also require
`req.provider.is_none()` before taking the bounded path. Apply this same fix to
both occurrences of this pattern (the one shown at startLine 119 and the other
mentioned around line 194-199).
| let bounded = | ||
| Self::filter_candidates(self.registry.get_candidates_for_model(req.model_id), req); | ||
| if !bounded.is_empty() { | ||
| return bounded; | ||
| } |
There was a problem hiding this comment.
Fall back when bounded candidates do not support the requested model.
get_candidates returns any non-empty bounded set before find_best_worker applies supports_model. Because stale wildcard entries are explicitly allowed, a stale-but-available wildcard can make bounded non-empty, then get rejected later by supports_model, skipping the full-scan alias fallback and missing a healthy alias-only worker.
🐛 Proposed fix
let bounded =
Self::filter_candidates(self.registry.get_candidates_for_model(req.model_id), req);
- if !bounded.is_empty() {
+ if bounded.iter().any(|w| w.supports_model(req.model_id)) {
return bounded;
}📝 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.
| let bounded = | |
| Self::filter_candidates(self.registry.get_candidates_for_model(req.model_id), req); | |
| if !bounded.is_empty() { | |
| return bounded; | |
| } | |
| let bounded = | |
| Self::filter_candidates(self.registry.get_candidates_for_model(req.model_id), req); | |
| if bounded.iter().any(|w| w.supports_model(req.model_id)) { | |
| return bounded; | |
| } |
🤖 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/common/worker_selection.rs` around lines 120 - 124,
The early return in the bounded candidates check does not verify that the
returned candidates actually support the requested model. Before returning the
bounded set when it is non-empty, add a check to ensure at least one candidate
in bounded supports the model by filtering or checking against supports_model.
If no candidates in the bounded set support the requested model, allow execution
to continue to the full-scan alias fallback mechanism instead of returning an
unsupported candidate set. This prevents stale wildcard entries from causing
premature returns and skipping the fallback that might find a healthy alias-only
worker.
| let worker_infos: Vec<WorkerInfo> = match model { | ||
| Some(model) => self | ||
| .worker_registry | ||
| .get_candidates_for_model(model) | ||
| .into_iter() | ||
| // The bounded set may include wildcard workers that have since | ||
| // discovered specific models; re-filter via `supports_model` | ||
| // so the listing matches the unfiltered semantics exactly. | ||
| .filter(|worker| worker.supports_model(model)) | ||
| .map(|worker| { | ||
| let id = self | ||
| .worker_registry | ||
| .get_id_by_url(worker.url()) | ||
| .map(|id| id.as_str().to_string()) | ||
| .unwrap_or_default(); | ||
| count_and_build(id, &worker) | ||
| }) | ||
| .collect(), |
There was a problem hiding this comment.
Mirror the router fallback for alias-only model matches.
get_candidates_for_model(model) only looks up the literal model index plus wildcards, so workers that support model only via a ModelCard alias are omitted when the bounded set is empty. The router handles this with a full-scan fallback; the admin ?model= path should do the same or GET /workers?model=<alias> regresses to an empty listing for servable aliases.
🐛 Proposed fix
- let worker_infos: Vec<WorkerInfo> = match model {
- Some(model) => self
- .worker_registry
- .get_candidates_for_model(model)
- .into_iter()
- // The bounded set may include wildcard workers that have since
- // discovered specific models; re-filter via `supports_model`
- // so the listing matches the unfiltered semantics exactly.
- .filter(|worker| worker.supports_model(model))
- .map(|worker| {
- let id = self
- .worker_registry
- .get_id_by_url(worker.url())
- .map(|id| id.as_str().to_string())
- .unwrap_or_default();
- count_and_build(id, &worker)
- })
- .collect(),
+ let worker_infos: Vec<WorkerInfo> = match model {
+ Some(model) => {
+ let mut workers: Vec<_> = self
+ .worker_registry
+ .get_candidates_for_model(model)
+ .into_iter()
+ .filter(|worker| worker.supports_model(model))
+ .filter_map(|worker| {
+ self.worker_registry
+ .get_id_by_url(worker.url())
+ .map(|id| (id, worker))
+ })
+ .collect();
+
+ if workers.is_empty() {
+ workers = self
+ .worker_registry
+ .get_all_with_ids()
+ .into_iter()
+ .filter(|(_, worker)| worker.supports_model(model))
+ .collect();
+ }
+
+ workers
+ .into_iter()
+ .map(|(worker_id, worker)| count_and_build(worker_id.as_str().to_string(), &worker))
+ .collect()
+ }
None => self🤖 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/worker/service.rs` around lines 368 - 385, The current
implementation of getting worker candidates for a model in the worker service
only uses the literal model index and wildcards via
get_candidates_for_model(model). When this returns an empty bounded set, workers
that support the model only through a ModelCard alias are missed. Add a fallback
mechanism similar to what the router implements: when the initial
get_candidates_for_model(model) call results in an empty set, perform a
full-scan lookup of the worker registry to find workers that support the model
via an alias. This ensures that GET /workers?model=<alias> returns results for
all servable aliases, not just literal model matches.
| let id = self | ||
| .worker_registry | ||
| .get_id_by_url(worker.url()) | ||
| .map(|id| id.as_str().to_string()) | ||
| .unwrap_or_default(); | ||
| count_and_build(id, &worker) |
There was a problem hiding this comment.
Do not emit a default worker ID on lookup races.
If a worker is removed after the bounded candidate snapshot but before get_id_by_url, unwrap_or_default() returns an invalid/default ID in the API response. Skip that stale candidate or return an error instead; WorkerInfo.id should never be fabricated.
🐛 Proposed fix
- .map(|worker| {
- let id = self
- .worker_registry
- .get_id_by_url(worker.url())
- .map(|id| id.as_str().to_string())
- .unwrap_or_default();
- count_and_build(id, &worker)
+ .filter_map(|worker| {
+ let id = self.worker_registry.get_id_by_url(worker.url())?;
+ Some(count_and_build(id.as_str().to_string(), &worker))
})📝 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.
| let id = self | |
| .worker_registry | |
| .get_id_by_url(worker.url()) | |
| .map(|id| id.as_str().to_string()) | |
| .unwrap_or_default(); | |
| count_and_build(id, &worker) | |
| .filter_map(|worker| { | |
| let id = self.worker_registry.get_id_by_url(worker.url())?; | |
| Some(count_and_build(id.as_str().to_string(), &worker)) | |
| }) |
🤖 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/worker/service.rs` around lines 378 - 383, The code in the
worker registry lookup uses unwrap_or_default() on the result of get_id_by_url,
which creates a fabricated default ID when the worker has been removed from the
registry due to a race condition. Instead of proceeding with this invalid
default ID in the count_and_build call, handle the None case by either skipping
the stale candidate worker entirely with a continue statement or returning an
appropriate error. Only call count_and_build when get_id_by_url successfully
returns a valid worker ID from the registry.
1 participant
What
Part of #1692. Removes the remaining O(total fleet) per-request cost in the common/HTTP worker-selection path (
WorkerSelector), which scanned the entire worker map viaget_workers_filtered(None, ...)and then filtered bysupports_model. (The gRPC hot path andregistry.get_workers_filteredare already per-model bounded.)Why the
Nonefull scan existed (and the correctness trap)That
Nonewas deliberate.BasicWorker::supports_modelreturnstruefor wildcard workers (those advertising no specific models, i.e.WorkerModels::Wildcard) on anymodel_id, and wildcard workers are not inget_by_model(specific_model)— their model index key is amodel_idlabel orUNKNOWN_MODEL_ID, not the requested model. So naively switching toget_by_model(model)would drop wildcard workers and causemodel_not_foundregressions.Approach: bounded + wildcard-safe
How wildcards are tracked.
worker_model_ids()(used to populate the model index) derives keys fromworker.models(), which is empty for a wildcard, so it falls back tomodel_id()→ amodel_idlabel orUNKNOWN_MODEL_ID. Wildcards are therefore not reliably under one sentinel key, so I maintain an explicit set rather than rely on a key:WorkerRegistry: newwildcard_workers: DashSet<WorkerId>, updated under the existing per-worker mutation lock inregister_inner/replace_inner/remove_inner, keyed on the trait-levelhas_models_discovered()predicate (a worker is a routing wildcard exactly when it has not declared/discovered specific models).get_candidates_for_model(model): returnsget_by_model(model)unioned with the wildcard set, deduped by URL (a wildcard may also be model-indexed under amodel_idlabel). Cost is O(per-model + wildcards); wildcards are few. A fast path returns the per-model slice directly (no dedup allocation) when there are no wildcards.Hot path.
get_candidatesandany_worker_supports_modeluse the bounded lookup when the model is known (mirroring the gRPCUNKNOWN_MODEL_IDguard;UNKNOWN_MODEL_IDkeeps the full-scan path). All existing filters (worker_type / connection_mode / runtime_type / availability / provider /supports_model) are still applied to the bounded set —supports_modelis now a cheap safety re-check.Fallback (never worse). If the bounded set yields no candidate, both methods fall back to the original full scan. This covers alias-only matches (a worker indexed under its id but matching the request via a
ModelCardalias) and any brief index lag after a concurrent registration — so no newmodel_not_found.Admin. The
?model-filteredlist_workerspath now uses the same bounded, wildcard-safe lookup (per-model index + wildcards) instead ofget_all()+ filter, with asupports_modelre-filter so the listing is identical to before. The unfiltered/workerslisting is left as-is (it must serialize all workers — inherently O(workers), acceptable for admin).One-way staleness note (safe)
Membership is captured at mutation time. A wildcard worker that later discovers specific models via lazy
/v1/modelsrefresh (set_models) is not a registry mutation, so it can linger in the wildcard set until its next register/replace/remove. This is harmless: it only causes slight over-inclusion in the union, and thesupports_modelre-filter rejects it. The reverse (a worker that should be in the set but isn't) cannot happen — discovery is one-way (wildcard → specific;set_modelsis only called with a non-empty list).Tests
replace(specific ⇄ wildcard); dedup when a wildcard is also model-indexed under a label.worker_selection/ registry / adminlist_workerstests still pass.Verification
cargo +nightly fmt --all— cleancargo clippy -p smg --all-targets -- -D warnings(default features) — cleancargo test -p smg(incl. integrationtests/binaries) — all green (lib: 1086 passed / 0 failed; spec_test 105; wasm_test 17; etc.)Scope:
registry.rs+routers/common/worker_selection.rs+ the adminworker/service.rshandler only. Policy files (least_load.rs,cache_aware.rs) untouched — owned by sibling work on #1692. Not marked "Closes" since other PRs also address #1692.Summary by CodeRabbit
Refactor
Tests