From 154330c5a0c5c4345ed7f93fdb230d1dc90628af Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Thu, 3 Sep 2026 13:25:37 -0600 Subject: [PATCH] fix: resolve duplicate model ids by requested type --- src/client/model.rs | 45 ++++++++++++++++++++++++++++++----- src/config/request_context.rs | 4 +++- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/client/model.rs b/src/client/model.rs index 51d3a20..6d9408e 100644 --- a/src/client/model.rs +++ b/src/client/model.rs @@ -65,12 +65,16 @@ impl Model { }; match model_name { Some(model_name) => { - if let Some(model) = models.iter().find(|v| v.id() == model_id) { - if model.model_type() == model_type { - return Ok((*model).clone()); - } else { - bail!("Model '{model_id}' is not a {model_type} model") - } + // Ids are not unique across types (e.g. a model that is both + // an embedder and a reranker), so match on id AND type. + if let Some(model) = models + .iter() + .find(|v| v.id() == model_id && v.model_type() == model_type) + { + return Ok((*model).clone()); + } + if models.iter().any(|v| v.id() == model_id) { + bail!("Model '{model_id}' is not a {model_type} model") } if list_client_names(config) .into_iter() @@ -458,6 +462,35 @@ mod tests { assert!(model.data.require_max_tokens); } + #[test] + fn duplicate_id_across_types_resolves_by_requested_type() { + // Mirrors jina:jina-colbert-v2, which is both an embedder and a + // reranker under the same id. + let claude: ClientConfig = serde_yaml::from_str( + "type: claude\nmodels:\n - name: test-dual-model\n type: embedding\n - name: test-dual-model\n type: reranker\n", + ) + .unwrap(); + let config = AppConfig { + clients: vec![claude], + ..AppConfig::default() + }; + + let embedder = + Model::retrieve_model(&config, "claude:test-dual-model", ModelType::Embedding).unwrap(); + assert_eq!(embedder.model_type(), ModelType::Embedding); + + let reranker = + Model::retrieve_model(&config, "claude:test-dual-model", ModelType::Reranker).unwrap(); + assert_eq!(reranker.model_type(), ModelType::Reranker); + + // An id that exists only under other types still teaches instead of + // silently creating a chat model from the name. + let err = Model::retrieve_model(&config, "claude:test-dual-model", ModelType::Chat) + .unwrap_err() + .to_string(); + assert!(err.contains("is not a chat model"), "got: {err}"); + } + #[test] fn description_shows_brain_glyph() { let mut model = Model::new("claude", "some-model"); diff --git a/src/config/request_context.rs b/src/config/request_context.rs index f0e4b45..e2f2547 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -5263,8 +5263,10 @@ mod tests { // It deliberately carries one embedding model: an empty `models` list // would fall back to the full embedded claude catalog and register // real chat models, breaking the no-chat-model invariant above. + // The test-dual-model pair shares one id across two types, mirroring + // jina:jina-colbert-v2, for the type-aware lookup tests. let claude: ClientConfig = serde_yaml::from_str( - "type: claude\nmodels:\n - name: test-claude-embedder\n type: embedding\n", + "type: claude\nmodels:\n - name: test-claude-embedder\n type: embedding\n - name: test-dual-model\n type: embedding\n - name: test-dual-model\n type: reranker\n", ) .unwrap(); let config = AppConfig {