Skip to content

fix(server): reject duplicate target model ids - #180

Open
elyasmnvidian wants to merge 1 commit into
mainfrom
emehtabuddin/fix-duplicate-target-diagnostic
Open

fix(server): reject duplicate target model ids#180
elyasmnvidian wants to merge 1 commit into
mainfrom
emehtabuddin/fix-duplicate-target-diagnostic

Conversation

@elyasmnvidian

@elyasmnvidian elyasmnvidian commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

What

switchyard-server now 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:

duplicate target model id same/model on llm client primary in targets strong and weak

The same upstream model id on different llm clients is still allowed — that is how one model is served through two providers.

Why

Each TranslatingLlmClient keys 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-run against the built binary:

# valid config
$ switchyard-server --config valid.toml --dry-run
server OK: switchyard/passthrough                       # exit 0

# duplicate id on one client
$ switchyard-server --config duplicate.toml --dry-run
invalid server config duplicate.toml: duplicate target model id \
  same/model on llm client primary in targets strong and weak   # exit 1

# same id on two providers, two routes
$ switchyard-server --config cross-provider.toml --dry-run
server OK: switchyard/azure-gpt4o, switchyard/openai-gpt4o       # exit 0

cargo clippy -p switchyard-server --all-targets -- -D warnings passes; cargo fmt clean.

Checklist

  • Unit tests for both the rejected (same client) and the allowed (different clients) config.
  • No public Python symbols or dependencies changed.
  • No documentation or --help text changed.
  • Commit includes the DCO sign-off.

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 --workspace still reaches the known macOS PyO3 linker failure for switchyard-py; the server crate itself passes all unit and HTTP integration tests.

Summary by CodeRabbit

  • Bug Fixes
    • Improved server configuration validation for target names and model IDs.
    • Configurations with duplicate target IDs under the same LLM client are now rejected with a clear error instead of being silently overwritten.
    • Identical model IDs remain valid when used with different LLM clients.
    • Validation errors are detected consistently during configuration setup.

Signed-off-by: Elyas Mehtabuddin <emehtabuddin@nvidia.com>
@elyasmnvidian
elyasmnvidian force-pushed the emehtabuddin/fix-duplicate-target-diagnostic branch from 27ad0ce to ec76568 Compare July 29, 2026 18:11
@elyasmnvidian
elyasmnvidian marked this pull request as ready for review July 29, 2026 18:11
@elyasmnvidian
elyasmnvidian requested a review from a team as a code owner July 29, 2026 18:11
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

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: 6f7859cb-2f8b-4a38-860e-cdd79a6093e1

📥 Commits

Reviewing files that changed from the base of the PR and between 36b9636 and ec76568.

📒 Files selected for processing (1)
  • crates/switchyard-server/src/config.rs

Walkthrough

Changes

ServerConfig::build now validates target fields and rejects duplicate (llm_client, target.id) pairs. Tests cover rejection on one client and acceptance across different clients.

Target configuration validation

Layer / File(s) Summary
Centralized target validation
crates/switchyard-server/src/config.rs
Target name and model ID validation moved into ServerConfig::build, which also detects duplicate client-target pairs before client construction.
Duplicate and cross-client coverage
crates/switchyard-server/src/config.rs
Tests verify precise duplicate errors and allow identical model IDs across different LLM clients.

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

Poem

I’m a rabbit checking targets bright,
No duplicate models sneak from sight.
One client, one pair, kept neat,
Across clients, repeats are sweet.
Hop, hop—tests make configs complete!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: rejecting duplicate target model IDs in server configuration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant