fix: resolve duplicate model ids by requested type
This commit is contained in:
2 files changed
+42
-7
No files matched your search
+39
-6
@@ -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");
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in new issue
Block a user