Skip to content

Commit 7e9ee98

Browse files
committed
fix(routing): preserve modality routing invariants
Signed-off-by: Todd Fisher <todd.fisher@gmail.com>
1 parent 8f0409f commit 7e9ee98

8 files changed

Lines changed: 198 additions & 23 deletions

File tree

crates/libsy/src/algorithms/fall_through.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,13 @@ where
284284
eligible_targets = ?eligible,
285285
"computed modality-compatible targets"
286286
);
287-
if let [target] = eligible.as_slice() {
287+
let needs_scoring = self
288+
.classifiers
289+
.iter()
290+
.any(|classifier| classifier.needs_single_eligible_scoring());
291+
if let [target] = eligible.as_slice()
292+
&& !needs_scoring
293+
{
288294
tracing::info!(
289295
target = %target,
290296
required_modalities = ?required,

crates/libsy/src/algorithms/rand.rs

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,7 @@ impl RandomClassifier {
9898
}
9999

100100
/// Samples from compatible targets using their original relative weights.
101-
fn select_eligible_target(
102-
&self,
103-
eligible_targets: &BTreeSet<ModelId>,
104-
) -> Result<Option<ModelId>> {
101+
fn select_eligible_target(&self, eligible_targets: &BTreeSet<ModelId>) -> Result<ModelId> {
105102
let eligible = self
106103
.targets
107104
.iter()
@@ -113,13 +110,15 @@ impl RandomClassifier {
113110
.map(|(_, weight)| **weight)
114111
.collect::<Vec<_>>();
115112
if !weights.iter().any(|weight| *weight > 0.0) {
116-
return Ok(None);
113+
return Err(LibsyError::AlgorithmError {
114+
message: "no modality-compatible random target has a positive weight".to_string(),
115+
});
117116
}
118117
let distribution =
119118
WeightedIndex::new(weights).map_err(|error| invalid_weights(error.to_string()))?;
120119
let mut rng = self.rng.lock();
121120
let index = distribution.sample(&mut *rng);
122-
Ok(Some(eligible[index].0.clone()))
121+
Ok(eligible[index].0.clone())
123122
}
124123
}
125124

@@ -134,6 +133,10 @@ impl<S> Classifier<S> for RandomClassifier
134133
where
135134
S: Send + 'static,
136135
{
136+
fn needs_single_eligible_scoring(&self) -> bool {
137+
true
138+
}
139+
137140
async fn score(
138141
&self,
139142
_state: &mut S,
@@ -156,15 +159,13 @@ where
156159
_driver: Option<&Driver>,
157160
eligible_targets: &BTreeSet<ModelId>,
158161
) -> Result<(Classification, Option<Response>)> {
159-
let scores = self
160-
.select_eligible_target(eligible_targets)?
161-
.map(|target| Score {
162+
Ok((
163+
Classification::Scores(vec![Score {
162164
confidence: 1.0,
163-
target,
164-
})
165-
.into_iter()
166-
.collect();
167-
Ok((Classification::Scores(scores), None))
165+
target: self.select_eligible_target(eligible_targets)?,
166+
}]),
167+
None,
168+
))
168169
}
169170
}
170171

@@ -447,6 +448,28 @@ mod tests {
447448
Ok(())
448449
}
449450

451+
#[tokio::test]
452+
async fn rejects_a_zero_weight_only_compatible_target() -> Result<()> {
453+
// Modality filtering must not re-enable a target disabled by a zero weight.
454+
let router: Arc<dyn Algorithm> = Arc::new(
455+
algorithm(&["text", "vision"], Some(vec![1.0, 0.0]), Some(42))?.with_target_modalities(
456+
target_modalities(&[
457+
("text", &[InputModality::Text]),
458+
("vision", &[InputModality::Text, InputModality::Image]),
459+
]),
460+
),
461+
);
462+
463+
let error = test_drive(router, image_request(), echo()).await.err();
464+
465+
assert!(matches!(
466+
error,
467+
Some(LibsyError::AlgorithmError { message })
468+
if message == "no modality-compatible random target has a positive weight"
469+
));
470+
Ok(())
471+
}
472+
450473
#[tokio::test]
451474
async fn affinity_reuses_the_initial_random_selection() -> Result<()> {
452475
let names = ["a/model", "b/model"];
@@ -502,7 +525,7 @@ mod tests {
502525
let affinity = Arc::new(AffinityRouter::new());
503526
let random = Arc::new(RandomClassifier::new(
504527
target_set(&names),
505-
Some(vec![1.0, 0.0]),
528+
Some(vec![100.0, 1.0]),
506529
Some(42),
507530
)?);
508531
let algorithm: Arc<dyn Algorithm> = Arc::new(

crates/libsy/src/core/classifier.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,11 @@ pub trait Classifier<S = ()>: Send + Sync {
9292
None
9393
}
9494

95+
/// Whether this classifier must score when modality filtering leaves one eligible target.
96+
fn needs_single_eligible_scoring(&self) -> bool {
97+
false
98+
}
99+
95100
/// Score the classifier's targets given the current state and request.
96101
///
97102
/// When present, `driver` lets a classifier offload model calls. It is `None`

crates/switchyard-server/src/config.rs

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,14 @@ impl ServerConfig {
174174
.map(|modalities| modalities.iter().copied().collect::<BTreeSet<_>>())
175175
.ok_or_else(|| ServerError::new("validated target modalities were missing"))?;
176176
advertised.extend(modalities.iter().copied());
177+
if let Some(existing) = target_modalities.get(&target.id)
178+
&& existing != &modalities
179+
{
180+
return Err(ServerError::new(format!(
181+
"route {route_name} maps model id {} to conflicting input_modalities",
182+
target.id
183+
)));
184+
}
177185
target_modalities.insert(target.id.clone(), modalities);
178186
}
179187
Ok((Some(target_modalities), advertised.into_iter().collect()))
@@ -1319,6 +1327,62 @@ target = "weak"
13191327
);
13201328
}
13211329

1330+
#[test]
1331+
fn duplicate_model_ids_require_matching_route_modalities() {
1332+
const DUPLICATE_MODEL_CONFIG: &str = r#"
1333+
schema_version = 1
1334+
1335+
[llm_clients.primary]
1336+
format = "openai_chat"
1337+
base_url = "https://example.test/v1"
1338+
1339+
[targets.first]
1340+
id = "shared/model"
1341+
llm_client = "primary"
1342+
input_modalities = ["text", "image"]
1343+
1344+
[targets.second]
1345+
id = "shared/model"
1346+
llm_client = "primary"
1347+
input_modalities = ["image", "text"]
1348+
1349+
[routes.shared]
1350+
id = "switchyard/shared"
1351+
type = "random"
1352+
targets = ["first", "second"]
1353+
"#;
1354+
1355+
// Repeated declarations for one model id may differ in ordering, but not content.
1356+
let config: ServerConfig =
1357+
toml::from_str(DUPLICATE_MODEL_CONFIG).expect("test config should parse");
1358+
let route = config
1359+
.routes
1360+
.get("shared")
1361+
.expect("test route should exist");
1362+
let (target_modalities, advertised) = config
1363+
.route_modalities("shared", route)
1364+
.expect("matching declarations should be accepted");
1365+
assert_eq!(
1366+
target_modalities
1367+
.expect("target modalities should be declared")
1368+
.get(&ModelId::from("shared/model")),
1369+
Some(&BTreeSet::from(
1370+
[InputModality::Text, InputModality::Image,]
1371+
))
1372+
);
1373+
assert_eq!(advertised, [InputModality::Text, InputModality::Image]);
1374+
1375+
let conflicting = DUPLICATE_MODEL_CONFIG.replace(
1376+
"input_modalities = [\"image\", \"text\"]",
1377+
"input_modalities = [\"text\"]",
1378+
);
1379+
assert!(
1380+
error_message(&conflicting).contains(
1381+
"route shared maps model id shared/model to conflicting input_modalities"
1382+
)
1383+
);
1384+
}
1385+
13221386
#[test]
13231387
fn judge_modality_declarations_require_text_but_are_optional() -> ServerResult<()> {
13241388
// Undeclared judge capabilities retain the legacy behavior.

crates/switchyard-server/tests/server.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2069,7 +2069,7 @@ input_modalities = ["text", "image"]
20692069
id = "switchyard/multimodal"
20702070
type = "random"
20712071
targets = ["text", "vision"]
2072-
weights = [1, 0]
2072+
weights = [1, 1]
20732073
"#,
20742074
base_url = upstream.base_url,
20752075
))?;

switchyard/cli/launchers/codex_cli_launcher.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,10 +177,18 @@ def _run_codex_with_switchyard(
177177
)
178178
return 1
179179
use_openai_auth = caller_auth == "openai"
180+
modalities_by_model = {display_model: input_modalities}
181+
for entry_model, _display, _description in codex_model_catalog:
182+
if entry_model in modalities_by_model:
183+
continue
184+
try:
185+
modalities_by_model[entry_model] = server.input_modalities(entry_model)
186+
except ValueError:
187+
continue
180188
model_catalog_json = _write_codex_model_catalog(
181189
codex_bin,
182190
codex_model_catalog,
183-
input_modalities_by_model={display_model: input_modalities},
191+
input_modalities_by_model=modalities_by_model,
184192
)
185193
command = _codex_command(
186194
codex_bin,

switchyard/cli/launchers/codex_model_catalog.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,7 @@ def _build_codex_model_catalog(
121121
model["supported_in_api"] = True
122122
model["availability_nux"] = None
123123
model["upgrade"] = None
124-
modalities = (
125-
input_modalities_by_model.get(model_id, ("text",))
126-
if input_modalities_by_model is not None
127-
else ("text",)
128-
)
124+
modalities = (input_modalities_by_model or {}).get(model_id, ("text",))
129125
model["input_modalities"] = list(modalities)
130126
models.append(model)
131127
return {"models": models}

tests/test_launchers.py

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,79 @@ def test_codex_catalog_uses_route_modalities_and_defaults_undeclared_to_text(
153153
assert undeclared["models"][0]["input_modalities"] == ["text"]
154154

155155

156+
def test_codex_launcher_discovers_modalities_for_every_catalog_entry(
157+
monkeypatch: pytest.MonkeyPatch,
158+
tmp_path: Path,
159+
) -> None:
160+
import switchyard.cli.launchers.codex_cli_launcher as launcher
161+
162+
class FakeServer:
163+
port = 4321
164+
stats = object()
165+
166+
def __init__(self) -> None:
167+
self.queried_models: list[str] = []
168+
self.closed = False
169+
170+
def caller_auth_kind(self, model: str) -> str | None:
171+
assert model == "switchyard/text"
172+
return None
173+
174+
def input_modalities(self, model: str) -> list[str]:
175+
self.queried_models.append(model)
176+
return {
177+
"switchyard/text": ["text"],
178+
"switchyard/vision": ["text", "image"],
179+
}[model]
180+
181+
def close(self) -> None:
182+
self.closed = True
183+
184+
server = FakeServer()
185+
captured_modalities: dict[str, list[str]] = {}
186+
187+
def write_catalog(
188+
_codex_bin: str,
189+
_entries: object,
190+
input_modalities_by_model: dict[str, list[str]] | None = None,
191+
) -> None:
192+
captured_modalities.update(input_modalities_by_model or {})
193+
194+
monkeypatch.setattr(launcher, "_find_codex_binary", lambda: "codex")
195+
monkeypatch.setattr(launcher, "silence_launch_loggers", lambda **_kwargs: None)
196+
monkeypatch.setattr(
197+
launcher,
198+
"configure_debug_file_logging",
199+
lambda **_kwargs: tmp_path / "switchyard.log",
200+
)
201+
monkeypatch.setattr(launcher, "_start_native_server", lambda _config: server)
202+
monkeypatch.setattr(launcher, "_write_codex_model_catalog", write_catalog)
203+
monkeypatch.setattr(launcher, "_wait_ready", lambda _port: True)
204+
monkeypatch.setattr(launcher, "print_ready_banner", lambda **_kwargs: None)
205+
monkeypatch.setattr(launcher, "stdin_is_tty", lambda: False)
206+
monkeypatch.setattr(launcher, "_supervise_codex", lambda _command, _env: 0)
207+
monkeypatch.setattr(launcher, "print_session_summary", lambda _stats: None)
208+
monkeypatch.setattr(launcher, "_remove_codex_model_catalog", lambda _path: None)
209+
210+
result = launcher._run_codex_with_switchyard(
211+
tmp_path / "routes.toml",
212+
display_model="switchyard/text",
213+
codex_args=[],
214+
codex_model_catalog=[
215+
("switchyard/text", "Text", "Text route"),
216+
("switchyard/vision", "Vision", "Vision route"),
217+
],
218+
)
219+
220+
assert result == 0
221+
assert server.queried_models == ["switchyard/text", "switchyard/vision"]
222+
assert captured_modalities == {
223+
"switchyard/text": ["text"],
224+
"switchyard/vision": ["text", "image"],
225+
}
226+
assert server.closed
227+
228+
156229
def test_native_server_exposes_route_derived_modalities(tmp_path: Path) -> None:
157230
config = tmp_path / "modalities.toml"
158231
config.write_text(

0 commit comments

Comments
 (0)