fix(server): reject duplicate target model ids - #180
Open
elyasmnvidian wants to merge 1 commit into
Open
Conversation
Signed-off-by: Elyas Mehtabuddin <emehtabuddin@nvidia.com>
elyasmnvidian
force-pushed
the
emehtabuddin/fix-duplicate-target-diagnostic
branch
from
July 29, 2026 18:11
27ad0ce to
ec76568
Compare
elyasmnvidian
marked this pull request as ready for review
July 29, 2026 18:11
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughChanges
Target configuration validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
switchyard-servernow rejects a config where two targets set the same upstream model id on the same llm client. The config fails to load before any client is built, and the error names the id, the client, and both targets:The same upstream model id on different llm clients is still allowed — that is how one model is served through two providers.
Why
Each
TranslatingLlmClientkeys its models by id. When two targets shared an id on the same client, the second target silently overwrote the first in that client's model map (last one wins), so the server started and routed that id to whichever target loaded last. The only prior guard was on random routes (random targets must be unique), so a passthrough- or classifier-only config with the duplicate started with no error.Rejecting the collision at config load fails closed. The check is scoped to
(llm_client, id)because that is exactly where the overwrite happens — a shared id across two clients is a valid cross-provider setup and keeps working.How tested
cargo test -p switchyard-server— 20 tests pass (11 unit + 9 HTTP integration). Two config tests target this change:rejects_duplicate_target_model_ids_on_one_client— two targets share an id and a client; asserts the exact error message.accepts_same_model_id_on_different_llm_clients— the same id on two clients under two routes builds successfully.Live
--dry-runagainst the built binary:cargo clippy -p switchyard-server --all-targets -- -D warningspasses;cargo fmtclean.Checklist
--helptext changed.Notes for reviewers
The check is scoped to
(llm_client, id), not to global target-id uniqueness. The real ambiguity is the per-client model-map overwrite; the same upstream id on two different clients (cross-provider A/B) dispatches correctly and stays valid, so the check must not reject it.cargo test --workspacestill reaches the known macOS PyO3 linker failure forswitchyard-py; the server crate itself passes all unit and HTTP integration tests.Summary by CodeRabbit