[Bug] Reject duplicate and colliding model entries when name is omitted in model runtime by Kaap10 · Pull Request #5650 · vllm-project/semantic-router

Kaap10 · 2026-10-30T15:25:58Z

Purpose

When configuring multi-model serving in the model runtime via a --models file or ServeConfig, entries without an explicit name field bypassed duplicate identity validation because uniqueness was only checked when config.name was truthy (config.py:268 and runtime.py:872).

Because ServedModel.label falls back to _name_of(config.model), duplicate model entries or collisions between explicit aliases and another model's derived name passed validation silently:

ProcessHealth.describe() returns a dictionary keyed by served.label, causing colliding model entries to overwrite earlier ones and conceal health states.
Runtime.lookup(name) matches the first model in self.served, silently shadowing subsequent instances.

This PR fixes both validation sites by checking effective names (config.name or _name_of(config.model)) in load_models_file and [served.label for served in self.served] in Runtime.__init__. To prevent circular import cycles between vllm_srun.runtime and vllm_srun.config, _name_of is moved to vllm_srun.config.

Owned by wg/router-models-inference-runtime.

Test Plan

Added parameterized regression tests in src/model-runtime/tests/test_engine_args.py:
- Duplicate unnamed models reject with ValueError: served model names must be unique.
- Cross-namespace models deriving the same name reject with ValueError: served model names must be unique.
- Inferred vs explicit name collisions (both ordering variations) reject with ValueError: served model names must be unique.
- Multi-instance serving with distinct explicit names (opt-0, opt-1) passes and correctly reports both models in describe().
- load_models_file rejects unnamed duplicate and colliding model entries with ValueError: ... duplicate model name ....
Added duplicate test cases to test_malformed_models_files_are_refused in src/model-runtime/tests/test_cli.py.

Executed local test suite:

python -m pytest src/model-runtime/tests/test_engine_args.py -v
python -m ruff check --config tools/linter/python/.ruff.toml src/model-runtime/vllm_srun/config.py src/model-runtime/vllm_srun/runtime.py src/model-runtime/tests/test_engine_args.py src/model-runtime/tests/test_cli.py
python tools/ci/classify_pr_changes.py src/model-runtime/vllm_srun/config.py src/model-runtime/vllm_srun/runtime.py src/model-runtime/tests/test_engine_args.py src/model-runtime/tests/test_cli.py

Test Result

pytest test_engine_args.py: 24 passed in 7.12s (100% pass, including all 8 new regression test cases).
ruff check: All checks passed with 0 errors/warnings.
classify_pr_changes.py: Correctly classified under domain model-runtime.

Semantic Router PR Checklist

PR title begins with exactly one bracketed category, such as [Feature], [Bug], [Docs], [Test], [Research], [Community], or [CI/Build]
The title does not stack prefixes such as [Router][Docs]; affected modules belong in labels and the PR body
The PR links an accepted issue with exactly one owner: one wg/* label for project work or owner/maintainers for repository governance
Commits in this PR are signed off with git commit -s
The Purpose, Test Plan, and Test Result sections reflect the actual scope, commands, and blockers for this change

github-actions · 2026-10-30T16:47:45Z

Waiting for committer approval — /ci run

Automatic checks passed. Required expensive checks have not run yet; this is not a test failure.

A committer with repository write, maintain or admin permission can start the required checks by commenting:

/ci run

Contributors without that permission do not need to request authorization themselves.

Pending qualification: e2e.model-runtime.

Revision 47c34720050f · CI run, attempt 1

Approval applies to this revision only. New commits require fresh qualification.

mergify · 2026-11-10T13:37:32Z

Tick the box to add this pull request to the merge queue (same as @mergifyio queue). If it does not match the queue conditions yet, it enters the queue once it does.

Queue this pull request

Zi-Yi-Ming

Thanks for the fix! Rechecked ee037dda: the models-file loader and runtime now consistently reject duplicate effective names, while distinct explicit aliases still work. The focused local checks and Model Runtime CI pass. LGTM; required E2E qualification remains before merge.

TwilightTechie

Good edge coverage — the cross-org collision and the name-omitted-vs-explicit cases in test_engine_args pin down exactly where the uniqueness check used to miss.

subin9

Thanks, the effective-name check fixes the cases in the issue. One case still collides: label switches to the package's model_name after loading, so two unnamed entries at different paths that hold the same package still overwrite each other in /health. Could the check also cover the loaded label, or could the label stay on the pre-load name?

…ed in model runtime Multi-model configurations loaded through ServeConfig or a --models YAML file bypassed duplicate identity validation when name was omitted, because load_models_file and Runtime.__init__ only evaluated entries where config.name was truthy. Because ServedModel.label falls back to the model path's trailing name (name_of), duplicate model instances or collisions between explicit and inferred model labels passed validation silently, causing shadowing during lookup and clobbering health state entries in ProcessHealth.describe(). Furthermore, ServedModel.label previously switched to package model_name after loading, causing colliding /health keys for distinct unnamed model paths sharing a package. Resolve this by: 1. Moving the name_of helper to vllm_srun.config as a public function to avoid circular imports. 2. Enforcing uniqueness on effective names (config.name or name_of(config.model)) in load_models_file. 3. Keeping ServedModel.label stable on the pre-load effective name (config.name or name_of(config.model)) across model lifecycle so health and metrics keys do not mutate or collide post-load. 4. Enforcing uniqueness on [served.label for served in self.served] in Runtime.__init__. 5. Adding regression test coverage in test_engine_args.py and test_cli.py covering unnamed duplicates, cross-namespace collisions, inferred-vs-explicit alias collisions, and preserving distinct health keys for distinct paths sharing a package. Closes vllm-project#5347 Signed-off-by: Vardhman Gupta <[email protected]>

Kaap10 · 2026-11-13T11:43:11Z

Thanks, the effective-name check fixes the cases in the issue. One case still collides: label switches to the package's model_name after loading, so two unnamed entries at different paths that hold the same package still overwrite each other in /health. Could the check also cover the loaded label, or could the label stay on the pre-load name?

Thanks @subin9, great catch!

Keeping label on the pre-load name (config.name or name_of(config.model)) makes the most sense. That way the label stays invariant throughout the entire lifecycle (loading -> ready) and avoids mutating or colliding in /health.

I updated ServedModel.label to stay on self.config.name or name_of(self.config.model) and added a regression test in test_engine_args.py verifying that two unnamed entries at different paths holding the same package retain distinct labels and both appear in /health.

Pushed the update in 47c3472. Please take a look when you have a moment!

abhinav-m22

Thanks for taking the nit and rebasing. LGTM.

wilsonwu

LGTM

Kaap10 requested review from AayushSaini101, FAUST-BENCHOU, Peterren, WUKUNTAI-0211, drivebyer, ramkrishs, shraderdm, theohsiung and wilsonwu as code owners October 30, 2026 15:26

Kaap10 mentioned this pull request Oct 30, 2026

[Bug] Multi-model runtime accepts duplicate and colliding model entries when name is omitted, causing silent shadowing and health status corruption #5347

Open

github-actions Bot added pr/blocked Blocked on a named decision, dependency, or required check. wg/router-models-inference-runtime Owned by the Router Models and Inference Runtime Workgroup. labels Oct 30, 2026

github-actions Bot added pr/needs-review Ready for reviewer attention. pr/needs-rebase Needs rebase or conflict resolution. and removed pr/needs-rebase Needs rebase or conflict resolution. pr/needs-review Ready for reviewer attention. labels Nov 5, 2026

abhinav-m22 reviewed Nov 10, 2026

View reviewed changes

Comment thread src/model-runtime/vllm_srun/runtime.py Resolved

theohsiung mentioned this pull request Nov 12, 2026

[Maintenance] Bug-fix PR and bug issue cleanup (status 2026-11-12) #6312

Open

1 of 4 tasks

Zi-Yi-Ming previously approved these changes Nov 12, 2026

View reviewed changes

TwilightTechie previously approved these changes Nov 12, 2026

View reviewed changes

subin9 requested changes Nov 13, 2026

View reviewed changes

abhinav-m22 approved these changes Nov 14, 2026

View reviewed changes

Merge branch 'main' into fix/5347-runtime-duplicate-models

79b4569

wilsonwu approved these changes Nov 14, 2026

View reviewed changes

Navigation Menu

Repository navigation

[Bug] Reject duplicate and colliding model entries when name is omitted in model runtime - #5650

[Bug] Reject duplicate and colliding model entries when name is omitted in model runtime#5650
Kaap10 wants to merge 2 commits into
vllm-project:mainfrom
Kaap10:fix/5347-runtime-duplicate-models

Kaap10 commented Oct 30, 2026

Uh oh!

github-actions Bot commented Oct 30, 2026 •

edited

Loading

Uh oh!

mergify Bot commented Nov 10, 2026

Uh oh!

Uh oh!

Zi-Yi-Ming left a comment

Uh oh!

TwilightTechie left a comment

Uh oh!

subin9 left a comment

Uh oh!

Kaap10 commented Nov 13, 2026

Uh oh!

abhinav-m22 left a comment

Uh oh!

wilsonwu left a comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

Kaap10 commented Oct 30, 2026

Purpose

Test Plan

Test Result

Uh oh!

github-actions Bot commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mergify Bot commented Nov 10, 2026

Uh oh!

Uh oh!

Zi-Yi-Ming left a comment

Choose a reason for hiding this comment

Uh oh!

TwilightTechie left a comment

Choose a reason for hiding this comment

Uh oh!

subin9 left a comment

Choose a reason for hiding this comment

Uh oh!

Kaap10 commented Nov 13, 2026

Uh oh!

abhinav-m22 left a comment

Choose a reason for hiding this comment

Uh oh!

wilsonwu left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

github-actions Bot commented Oct 30, 2026 •

edited

Loading