fix(discovery): allow served aliases for the same model source

[jthomson04]

Summary

Fixes DYN-4449.

Allow non-LoRA workers with different served names to register on the same endpoint when both advertise the same non-empty source path and their primary/secondary served names do not overlap. Preserve registration compatibility for equal primary names at different local paths. Mixed base/LoRA registrations require matching base identities and reject adapter names that collide with the base primary or its aliases, in either registration order. Adapter-to-adapter comparisons remain unchanged. Frontend admission and model-card checksum rules remain unchanged.

The separate served names select separate worker groups, as required by the ticket's two-worker classify/pooling case. ModelDeploymentCard::aliases instead exposes multiple names for one worker group. This fix does not change single-worker registration keys or disaggregated topology.

Source paths use exact string equality. No path normalization or model-content comparison is introduced. Each newcomer must be compatible with every existing registration; conflicting configurations remain arrival-order dependent, and incompatible concurrent newcomers can both roll back. No registration arbitration or retry guarantee is added.

Deployment requirement

Upgrade frontends and all participating workers to versions containing the required admission filter and this registration fix before enabling different primary served names on a shared endpoint. Older workers can reject the configuration when they restart. Inspected v1.3/v1.4 frontend code lacks the current model-specific worker admission filter, so this new deployment configuration must not be enabled during a mixed-version rollout.

Validation

• cargo test -p dynamo-runtime --no-default-features --lib discovery::mock::tests — 15 passed. Includes base/LoRA name collisions in both registration orders, preservation of existing registrations, both three-card arrival orders, and source-path boundaries.
• cargo test -p dynamo-llm --no-default-features --lib discovery::watcher::tests — 24 passed. Includes two classify workers using file discovery and TCP requests, separate alias routing, and independent removal. The subprocess check requires exactly one passing test.
• rustfmt --edition 2024 --check on all three changed Rust files and git diff --check — passed.
• GPU/vLLM validation remains incomplete. Previous checks found two local GPUs but no vLLM installation in the checked host or existing Dynamo container environments. The Rust regression uses mock workers and does not establish vLLM end-to-end success.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e40d976b-45cb-4805-a85a-c2240cf0a991

📥 Commits

Reviewing files that changed from the base of the PR and between 055e989 and 3b41797.

📒 Files selected for processing (3)

• lib/llm/src/discovery/watcher.rs
• lib/runtime/src/discovery/mock.rs
• lib/runtime/src/discovery/mod.rs

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

---

Walkthrough

The runtime now permits compatible non-LoRA aliases with shared non-empty source paths. Tests cover registration compatibility and independent classify alias discovery, routing, and removal.

Changes

Discovery alias compatibility

Layer / File(s)	Summary
Non-LoRA registration compatibility lib/runtime/src/discovery/mod.rs, lib/runtime/src/discovery/mock.rs	Non-LoRA registrations remain compatible when they share a non-empty source_path. Table-driven tests cover matching, differing, missing, and empty paths.
Classify alias discovery lifecycle lib/llm/src/discovery/watcher.rs	An end-to-end test verifies independent readiness, request routing, unregistration, catalog removal, and continued availability for two aliases sharing one endpoint.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 3b417

The alias compatibility change preserves the existing name and LoRA rules while adding the intended same-source-path case. The covered routing and removal behavior does not show a remaining merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: allowing served aliases for the same model source.
Description check	✅ Passed	The description provides the change scope, implementation details, deployment requirement, related issue DYN-4449, validation results, and known validation limits. It does not use every template headi…

• Fix all pre-merge checks with AI

---

_{Comment @coderabbitai help to get the list of available commands.}

[jthomson04]

Review: fix(discovery): allow served aliases for the same model source

Reviewed the 3 files (240 added / 31 removed), of which 5 lines are production code. Inline comments cover 12 findings; 3 more are on files outside this diff and are listed below.

The one that blocks: is_compatible_with goes from an equivalence relation to a non-transitive one, and Discovery::register's post-registration re-check relies on pairwise compatibility implying set compatibility. Two concurrent registrations can now mutually roll each other back, and the surviving set depends on arrival order. Details inline on mod.rs.

Three findings on files this PR doesn't touch — the change doesn't fully deliver the aliasing it promises:

1. lib/runtime/src/discovery/kv_store.rs:86 — a single worker still can't register two aliases. model_key is {ns}/{comp}/{endpoint}/{instance_id:x} with no display-name component. One worker process calling register() twice on the same endpoint with alias-a / alias-b (both model_suffix: None) now passes the conflict check, then writes the identical KV key twice and the second silently clobbers the first — no error, one alias just never appears in the catalog. The new test sidesteps this by giving each alias its own DistributedRuntime. If DYN-4449's scenario is one worker serving two names, this change doesn't reach it.

2. lib/bindings/python/rust/lib.rs:677 — TensorBased/Images/Videos/Realtime are excluded by construction. That path does if source_path != model_name { card.source_path = Some(...) }, and #[serde(skip_serializing_if = "Option::is_none")] drops the field entirely. So a worker registering an images model without --served-model-name yields source_path: None, the new is_some_and short-circuits, and it is rejected exactly as before. The fix silently does not cover these model types.

3. lib/llm/src/discovery/model_manager.rs:687 — two co-registered workers sharing an aliases entry deadlock the frontend. Worker A registers alias-a with aliases: ["shared"], worker B registers alias-b with aliases: ["shared"] and the same source_path — now legal. Group alias-a commits and claims alias_to_primary["shared"]; group alias-b hits anyhow::bail!("alias {alias:?} is already reserved by ..."). controller.rs:738 puts it into Blocked with backoff and retries forever, rebuilding the full WorkerSet (tokenizer load, client, routers) each attempt. alias-b never serves, and nothing is logged above warn.

Worth deciding before the mechanics: Dynamo already has ModelDeploymentCard::aliases + ModelManager::alias_to_primary, which vLLM's --served-model-name a b c already maps into, and which serves N names from one WorkerSet. This PR's path builds a separate GroupKey/WorkerSet/router stack per alias for what is by construction the same model, and splits the P/D rendezvous in disaggregated deployments. Commented inline.

---

🤖 Generated with Claude Code

[dynamo-review-agent]

Previously reported defects still present:

• Original discussion: The non-transitive compatibility rule still permits concurrent registrations AB and BC while A conflicts with C. Both newcomers can pass the pre-check, register, then each observe the other in the post-check and roll itself back, leaving neither served. The new PR description acknowledges this behavior but does not provide arbitration or a retry path.
• Original discussion: The shared-source compatibility relation remains non-transitive. With an incumbent compatible by name to one worker and by source_path to another, concurrent registrations can both pass the pre-check, then both fail the post-registration check and roll back. The two valid alias workers are left unserved and may repeat this on restart.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[jthomson04]

/ok to test 71896cf

[dynamo-review-agent]

Previously reported defects still present:

• Original discussion: Still present: register_non_lora_alias_compatibility retains the two one-sided empty source_path rows at lines 620-621, which add no distinct regression coverage beyond the retained both-empty row and ordinary unequal-source row.
• Original discussion: Verified still present: LoRA compatibility still uses an empty source_path as its base identity, so unrelated cards with source_path: "" can be treated as sharing a base model.
• Original discussion: Verified still present: compatibility remains non-transitive. Concurrent registrations that are each compatible with an incumbent but incompatible with each other can both pass the first check, register, fail the post-registration check, and roll back, leaving neither new worker serving.
• Original discussion: Verified still present: the new name-or-shared-source compatibility relation is non-transitive. Concurrent registrations that each match an incumbent but conflict with one another can both pass the pre-check, then both roll back in the post-registration check, leaving neither new worker served.

[glamr-agent]

factory: automated evidence record for this change — validation complete. Details are collapsed below.

Automated evidence record — validation complete

Validation status: complete

Evidence summary: [3/3 validated]

AI review assessment: sound. This is an automated assessment and is advisory only. It is not an approval, and it does not speak for repository CI or for any human reviewer.

Validation result: pass. Every nominated check ran green against the materialized diff. The four new tests each fail when the 21-line production hunk in lib/runtime/src/discovery/mod.rs is reverted and pass when it is restored. The ticket's exact two-worker classify command reproduces Cannot register model without the diff and returns HTTP 200 on both served names with it. One command did fail: a /v1/chat/completions request returned HTTP 500 from ImportError: cannot import name 'VLLMClientError' from 'vllm.exceptions'. That comes from vLLM 0.26.0 missing a symbol imported by a Python file this three-Rust-file branch does not touch, so it is unrelated to this change.

Evidence audit: complete [3/3 validated] — the command report below comes from recorded runs.

Commands and results [3/3 validated]

Generated from the commands recorded during this run.

Check 1

Builds the changed Dynamo source and confirms that Python can import its compiled extension.

Result: Passed (exit 0)

Command:

Not shown because the exact command contained private run data.

Check 2

Checks the changed Rust crates with cargo check and Clippy.

Result: Passed (exit 0)

Command:

Not shown because the exact command contained private run data.

Check 3

Starts Dynamo with vLLM on one GPU and sends a real request.

Result: Passed (exit 0)

Command:

Not shown because the exact command contained private run data.

[jthomson04]

/ok to test 2e063a5

[dynamo-review-agent]

Previously reported defects still present:

• Original discussion: Verified still present: base_identity() treats a present empty source_path as a shared base identity. A LoRA registration and an unrelated base registration with source_path: "" can therefore pass the mixed base/LoRA compatibility check.
• Original discussion: Verified still present: base_identity() treats Some("") as the base identity rather than falling back to display_name. A LoRA card and an unrelated base card that both advertise an empty source_path are therefore compatible for the same endpoint, despite representing different models. This predates the change but remains unaddressed.
• Original discussion: Verified still present: compatibility is non-transitive. With incumbent B(name=a, source=/mnt/b), concurrent A(name=a, source=/mnt/a) and C(name=b, source=/mnt/b) can both pass the pre-registration check, then each sees the other as incompatible during the post-registration check and rolls itself back. Both workers can remain absent until a later retry.
• Original discussion: Verified still present: the new same-name-or-same-source compatibility relation is non-transitive. Concurrent registrations that each match an incumbent but conflict with each other can both pass the pre-check, then both roll back during the post-registration check, leaving neither worker registered.

[dynamo-review-agent]

Previously reported defects still present:

• Original discussion: Still present: base_identity() treats source_path: "" as a base identity, so unrelated base and LoRA registrations with empty source paths can be accepted as sharing a base model.
• Original discussion: Still present: the new name-or-shared-source compatibility rule is non-transitive. Concurrent registrations that each match an incumbent but conflict with each other can both pass the initial check, then both roll back after registration and leave neither worker served.
• Original discussion: Verified still present: base_identity() treats Some("") as a shared base identity. A LoRA adapter and unrelated base card with empty source paths can therefore be admitted together as though they share a base model. This predates the PR's non-LoRA empty-source guard.
• Original discussion: Verified still present: base_identity() treats source_path: "" as a shared base identity. A LoRA card and an unrelated base card with empty source paths can pass mixed base/LoRA registration compatibility on the same endpoint.
• Original discussion: Verified still present: compatibility is non-transitive, so concurrent registrations each compatible with an incumbent can both pass the pre-check, observe each other after registration, roll themselves back, and repeatedly fail to converge. The added test covers sequential arrival order but not this concurrent rollback path.
• Original discussion: Verified still present: the new same-name-or-shared-source compatibility relation is non-transitive. Concurrent registrations that each match an incumbent but conflict with one another can both pass the initial check, then both roll back during the post-registration check, leaving neither newly registered worker available.

[dynamo-review-agent]

Previously reported defects still present:

• Original discussion: Still present: base_identity() treats source_path: "" as a shared identity, allowing unrelated base and LoRA registrations with empty source paths to be admitted together.
• Original discussion: Still present: the new non-transitive compatibility rule allows concurrent registrations that each match an incumbent but conflict with each other to pass pre-registration, then both roll back during the post-registration check.
• Original discussion: Verified still present: base_identity() still treats source_path: "" as a shared base identity, so a LoRA adapter and an unrelated base registration with empty source paths can pass mixed base/LoRA compatibility on the same endpoint.
• Original discussion: Verified still present: base_identity() treats source_path: "" as a shared base identity. An unrelated base card and LoRA adapter with empty source paths can therefore pass mixed registration compatibility as though they share a base model.
• Original discussion: Verified still present: base_identity() treats a present empty source_path as a shared base identity, so unrelated base and LoRA registrations with source_path: "" can be admitted together.
• Original discussion: Verified still present: compatibility is non-transitive. Concurrent registrations that each match an incumbent but conflict with each other can pass the initial check, then both detect the other after registration, roll themselves back, and leave neither worker available.
• Original discussion: Verified still present: the new name-or-shared-source compatibility relation is non-transitive. Concurrent registrations individually compatible with an incumbent can both pass pre-registration, conflict during the post-registration check, roll back, and leave neither worker served.
• Original discussion: Verified still present: the same-name-or-shared-source compatibility relation remains non-transitive. Concurrent registrations that each match an incumbent but conflict with one another can both pass the initial check, then both roll back during the post-registration check, leaving neither new worker registered.

[jthomson04]

/ok to test 918ffe1

[dynamo-review-agent]

Previously reported defects still present:

• Original discussion: Still present: base_identity() treats source_path: "" as a shared identity, so an unrelated base card and LoRA card with empty source paths can pass mixed registration compatibility on the same endpoint.
• Original discussion: Still present: compatibility is non-transitive. Concurrent registrations that are each compatible with an incumbent can both pass the pre-registration check, observe each other as incompatible after registration, and both roll back, leaving neither new worker available.
• Original discussion: Verified still present: base_identity() still uses a present empty source_path as the base identity, so an unrelated base registration and LoRA adapter with source_path: "" can pass mixed base/LoRA compatibility on the same endpoint.
• Original discussion: Verified still present: base_identity() treats source_path: "" as a shared identity for LoRA comparisons. An unrelated base card and LoRA adapter with empty source paths can therefore be admitted on the same endpoint as though they share a base model.
• Original discussion: Verified still present: shared-name-or-source compatibility is non-transitive. Concurrent newcomers that are each compatible with an incumbent but conflict with each other can both pass the first check, register, fail the post-registration check, and roll themselves back, leaving neither available.
• Original discussion: Verified still present: the name-or-shared-source compatibility relation remains non-transitive, and Discovery::register still performs separate pre- and post-registration checks with rollback. Concurrent registrations that each match an incumbent but conflict with each other can both pass the pre-check, then both roll back during the post-check.