fix: stop auto from picking models nobody serves - #1221
Conversation
Auto model selection could choose a "phantom" model — one advertised in gossip but served by no peer (stale gossip, or a peer that unloaded it) — and the request then failed with a 404 naming a model the user never asked for. The readiness filter checked local targets, then remote hosts, then fell through to `true`. A model with neither was therefore treated as ready and stayed in the auto candidate pool. It now fails closed: no routable local target and no remote host means not auto-route eligible. A freshly started serve node is kept eligible via its own hosted/serving model list, because that populates before the election target table and peer gossip catch up. Explicit model requests are unchanged and still return an honest 404. Regression introduced by the combination of #734 (added the readiness filter with the fail-open default) and #1082 (replaced the old route-to-first-available fallback with a hard 404, which made selecting a phantom user-visible).
📝 WalkthroughWalkthroughAuto-routing now rejects models without local or remote routing targets. Models currently served by the node remain eligible while routing targets and peer gossip state are not yet populated. Tests cover both exclusion and local-serving eligibility. ChangesLocal model auto-routing eligibility
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/mesh-llm-host-runtime/src/network/openai/auto_route.rs (1)
156-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the
serving_modelsbranch.This test populates only
hosted_models. Add a case where the model appears only inserving_modelsand assert thatmodel_is_locally_servedreturnstrue.🤖 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 `@crates/mesh-llm-host-runtime/src/network/openai/auto_route.rs` around lines 156 - 168, Extend freshly_loaded_local_model_stays_eligible_before_targets_populate to populate serving_models with a model absent from hosted_models, then assert model_is_locally_served returns true for that model while retaining the existing hosted_models and unrelated-model assertions.
🤖 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 `@crates/mesh-llm-host-runtime/src/network/openai/ingress.rs`:
- Around line 355-363: Ensure auto_route_pool_for_ready_models returns an empty
pool when no candidates pass the readiness predicate, preventing
pool_for_ready_models from restoring all candidates as a fallback. Preserve
pool_for_ready_models’ existing empty-ready-list contract, and extend the
regression test to verify the final pool or resolved model cannot select a
phantom candidate.
---
Nitpick comments:
In `@crates/mesh-llm-host-runtime/src/network/openai/auto_route.rs`:
- Around line 156-168: Extend
freshly_loaded_local_model_stays_eligible_before_targets_populate to populate
serving_models with a model absent from hosted_models, then assert
model_is_locally_served returns true for that model while retaining the existing
hosted_models and unrelated-model assertions.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: e70c0b1e-be1d-4f78-b037-7fb19f872bf3
📒 Files selected for processing (2)
crates/mesh-llm-host-runtime/src/network/openai/auto_route.rscrates/mesh-llm-host-runtime/src/network/openai/ingress.rs
| // No routable local target and no peer advertises this model. Fail closed: | ||
| // such a model is a phantom (stale gossip, or a peer that unloaded it), and | ||
| // letting it stay in the pool means `auto` can pick a model that then 404s | ||
| // on a model the user never named. Explicit requests still 404 honestly. | ||
| // | ||
| // The one exception is a freshly started serve node: its own model is | ||
| // loaded and in `serving_models` before the target table and gossip catch | ||
| // up, so keep it eligible rather than excluding this node's own model. | ||
| auto_route::model_is_locally_served(node, model).await |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Prevent the all-candidates fallback from undoing fail-closed readiness.
If every candidate reaches this branch, ready_models is empty. auto_route_pool_for_ready_models then calls auto_route::pool_for_ready_models, which returns all candidates when the ready list is empty in crates/mesh-llm-host-runtime/src/network/openai/auto_route.rs, Lines 107-109. auto can therefore still select a phantom model.
Make this caller return an empty pool when no candidate is ready, or add an explicit fail-closed mode to the helper. Extend the regression test to assert the final pool or resolved model, not only the predicate. Preserve the existing helper contract tested in crates/mesh-llm-host-runtime/src/network/openai/auto_route.rs, Lines 170-183.
Also applies to: 1057-1082
🤖 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 `@crates/mesh-llm-host-runtime/src/network/openai/ingress.rs` around lines 355
- 363, Ensure auto_route_pool_for_ready_models returns an empty pool when no
candidates pass the readiness predicate, preventing pool_for_ready_models from
restoring all candidates as a fallback. Preserve pool_for_ready_models’ existing
empty-ready-list contract, and extend the regression test to verify the final
pool or resolved model cannot select a phantom candidate.
ndizazzo
changed the title
2 participants
autono longer picks a model that nobody in the mesh is actually serving.Before this, a request with
model: "auto"could land on a "phantom" model — one still advertised in gossip but served by no peer (stale gossip, or a peer that unloaded it). The request then failed with:which is a confusing thing to receive when you asked for
autoand never named that model.Explicit model requests are unchanged — asking for a model nobody serves still returns an honest 404.
Architecture
The auto readiness filter checked local targets, then remote hosts, then fell through to
true. A model with neither was therefore treated as "ready" and stayed in the auto candidate pool, where the weighted draw could select it.It now fails closed: no routable local target and no remote host means not auto-route eligible.
The one case that must not regress is a freshly started serve node — it records its model in
hosted_models/serving_modelsbefore the election target table and peer gossip catch up, so during that window its own model has no target and no remote host. A naive fail-closed would drop the node's own model from auto.model_is_locally_servedkeeps it eligible, and there is a test for exactly that window.Regression origin
Two commits combined to produce this:
916c3221) added the readiness filter with the fail-opentruedefault, putting phantoms in the pool.ed286b90) replaced the old "route to first available target" fallback with a hard 404. That is the right call for an explicit request — silently substituting a different model is worse — but it made selecting a phantom user-visible instead of silently papered over.Validation
cargo test -p mesh-llm-host-runtime --lib— 1930 passed, 0 failedcargo clippy -p mesh-llm-host-runtime --all-targets -- -D warnings— cleancargo clippy -p mesh-llm --all-targets -- -D warnings— cleancargo fmt --all --check— cleanTests fail without the fix. I verified this rather than assuming: temporarily reverting the fail-closed line back to
truemakesphantom_model_is_not_auto_route_eligibleFAIL whilefreshly_served_local_model_is_auto_route_eligiblestill passes, then restoring it makes both pass. So the new coverage genuinely pins the regression.Public mesh
Ran
mesh-llm client --autoagainst the public mesh (5 peers), which had a real phantom present (unsloth/Qwen3-8B-GGUF:Q4_K_M— advertised in/v1/models, served by no peer):model=autorequests across two runs: no phantom ever selected, nomodel_not_found.One caveat worth stating plainly: on this particular mesh the phantom was also being masked by the big/small tier partition (
Qwen3-8Bsorts small, and big-tier models were available), so the public-mesh run alone does not prove the fix — I confirmed that by A/B-ing against an unfixed binary and getting identical results. The unit tests above are the real evidence; the mesh run is a no-regression check.Unrelated to this change, one request in 55 returned a 503 (
all 1 target(s) for model 'local-gguf/...' failed) — a peer whose target died mid-request. That model is served by a peer, so it is peer churn on a path this PR does not touch.Summary by CodeRabbit