feat: Introduce ModelId type

[grahamking]

Identify model ID's (e.g. "openai/gpt-oss-120b") with their own type
ModelId. That makes it clear what we are passing, and prevent
accidentally passing the wrong thing.

ModelId behaves like a string. It can be printed, deref-ed, compared, etc.

Previously that was either String or LlmTarget. This PR removes
LlmTarget and LlmTargetSet, in favor of ModelId and
Vec<ModelId>.

Also rename resolve_target and require_target function to more clearly communicate now that we have target / model ID distinction.

To review start with new file crates/protocol/src/model_id.rs which
contains the type.

Assisted-by: Claude:Opus 5 medium
Assisted-by: Codex:GPT 5.6 Sol high

Signed-off-by: Graham King grahamk@nvidia.com

Summary by CodeRabbit

• New Features

  • Added a shared, strongly typed model identifier used consistently across routing, configuration, observability, errors, statistics, and client interactions.
  • Simplified model target configuration by accepting model IDs directly.
  • Updated Python bindings and server integrations to support the new model identifier format.

• Documentation

  • Updated architecture diagrams and README guidance to describe model IDs directly.

[grahamking]

@CodeRabbit review

[coderabbitai]

⚠️ Action not completed

Head commit changed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[github-actions]

PR Preview Action v1.8.1
🚀 View preview at https://NVIDIA-NeMo.github.io/Switchyard/pr-preview/pr-373/
Built to branch gh-pages at 2026-08-12 13:32 UTC. Preview will be ready when the GitHub Pages deployment is complete.

[coderabbitai]

Walkthrough

The change adds the protocol-level ModelId type and migrates client, routing, classifier, server, binding, observability, and test code from string-based model targets and LlmTarget wrappers.

Changes

Model identity migration

Layer / File(s)	Summary
Protocol and client contracts crates/protocol/src/*, crates/libsy-llm-client/src/*	Adds ModelId with string conversion, comparison, lookup, formatting, and serialization support. Updates client decisions, errors, observations, model configuration, and routing APIs to use ModelId.
Core routing execution crates/libsy/src/core/*, crates/libsy/src/error.rs, crates/libsy/src/lib.rs	Removes LlmTarget and LlmTargetSet. Target validation, resolution, fallback handling, classifier scores, errors, and public exports now use ModelId.
Algorithm and classifier migration crates/libsy/src/algorithms/*	Updates fall-through, random, stage, passthrough, classifier, affinity, judge, prompt, subagent, and selector paths to store and route ModelId values directly.
Integration and validation crates/switchyard-server/*, crates/switchyard-py/src/libsy_bindings.rs, crates/libsy-llm-client/tests/observability.rs, crates/libsy/README.md, AGENTS.md	Updates server target resolution, statistics, routing logs, Python client mappings, observability tests, README text, and the architecture diagram for the new representation.

Estimated code review effort: 4 (Complex) | ~45 minutes

Poem

I’m a rabbit with a typed little trail,
ModelId hops where strings used to fail.
Targets shed wrappers and run clean and bright,
Routers and judges now point just right.
Tests follow softly through each routing lane—
One neat identity in every domain.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 96.63% which is sufficient. The required threshold is 80.00%.
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: adding the public ModelId type and migrating model identifiers to it.

---

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

[grahamking]

@CodeRabbit review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (1)

crates/libsy-llm-client/tests/observability.rs (1)

432-432: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep the test router typed as ModelId.

SingleCallAlgo.target_set retains Vec<String>. Line 451 converts the value only when it creates Decision. Store Vec<ModelId> and construct ModelId in algo. This makes the helper compile against the same routing contract as production algorithms.

The PR objective requires ModelId across routing and test code.

Also applies to: 451-451, 476-476

🤖 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/libsy-llm-client/tests/observability.rs` at line 432, Update the test
router helper around SingleCallAlgo so target_set uses Vec<ModelId> instead of
Vec<String>. Construct ModelId values when initializing algo, and pass the typed
targets directly into Decision without converting them at the call sites,
including the other referenced usages.

🤖 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/libsy/src/core/algorithm.rs`:
- Around line 473-483: Before call_model_with_fallback, add a block comment
documenting its fallback-loop invariants: which errors are retryable, how failed
targets are excluded, how routing state is updated, when fallback decisions are
published, and that the original error is returned when all attempts fail.

In `@crates/libsy/src/lib.rs`:
- Line 8: Restore the public LlmTarget and LlmTargetSet exports in the libsy
compatibility surface and retain their definitions for at least one release,
marking them deprecated with migration guidance rather than removing them
immediately. Update the core migration path to preserve these types until the
approved deprecation window is complete.

In `@crates/protocol/src/model_id.rs`:
- Line 114: Remove the project-management comment “This is likely excessive. We
can reduce when things settle.” from the ModelId implementation, leaving the
surrounding code unchanged.

In `@crates/switchyard-server/src/config.rs`:
- Around line 153-157: Update build_targets to reject routes whose callable
targets resolve to the same ModelId, returning a configuration error before
constructing the ClientRouter. Preserve valid distinct targets, and add a
regression test covering duplicate model IDs configured with different
llm_client values to ensure the route fails validation rather than overwriting a
client mapping.

---

Nitpick comments:
In `@crates/libsy-llm-client/tests/observability.rs`:
- Line 432: Update the test router helper around SingleCallAlgo so target_set
uses Vec<ModelId> instead of Vec<String>. Construct ModelId values when
initializing algo, and pass the typed targets directly into Decision without
converting them at the call sites, including the other referenced usages.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ef0c9a9d-3763-4741-b444-a170acbe7d09

📥 Commits

Reviewing files that changed from the base of the PR and between 58f355a and c8fb4d6.

📒 Files selected for processing (33)

• AGENTS.md
• crates/libsy-llm-client/src/client.rs
• crates/libsy-llm-client/src/observation.rs
• crates/libsy-llm-client/src/run.rs
• crates/libsy-llm-client/tests/observability.rs
• crates/libsy/README.md
• crates/libsy/src/algorithms/fall_through.rs
• crates/libsy/src/algorithms/llm_class.rs
• crates/libsy/src/algorithms/passthrough.rs
• crates/libsy/src/algorithms/rand.rs
• crates/libsy/src/algorithms/stage.rs
• crates/libsy/src/algorithms/subagent_affinity_tests.rs
• crates/libsy/src/algorithms/util/affinity.rs
• crates/libsy/src/algorithms/util/escalation.rs
• crates/libsy/src/algorithms/util/llm_judge.rs
• crates/libsy/src/algorithms/util/prompts.rs
• crates/libsy/src/algorithms/util/stage.rs
• crates/libsy/src/algorithms/util/subagent.rs
• crates/libsy/src/algorithms/util/target_selector.rs
• crates/libsy/src/core/algorithm.rs
• crates/libsy/src/core/classifier.rs
• crates/libsy/src/error.rs
• crates/libsy/src/lib.rs
• crates/libsy/src/observability.rs
• crates/protocol/src/client.rs
• crates/protocol/src/lib.rs
• crates/protocol/src/model_id.rs
• crates/switchyard-py/src/libsy_bindings.rs
• crates/switchyard-server/src/config.rs
• crates/switchyard-server/src/lib.rs
• crates/switchyard-server/src/routing_log.rs
• crates/switchyard-server/src/stats/accumulator.rs
• crates/switchyard-server/tests/server.rs

[ayushag-nv]

@grahamking can you take care of server side (or whichever is duplicate) LLMTarget one which we discussed to delete or update. The one that loads from the config ?

[ayushag-nv]

Looks good except from one comment related to complexity / lines of code introduced by model_id.rs

[grahamking]

@grahamking can you take care of server side (or whichever is duplicate) LLMTarget one which we discussed to delete or update. The one that loads from the config ?

This PR also deletes LLMTarget .