From 60ac37f3a90f30daeb0a0857bdcd1c88de0099f7 Mon Sep 17 00:00:00 2001 From: Derek Higgins Date: Tue, 11 Nov 2025 16:39:07 +0000 Subject: [PATCH] fix: remove_disabled_providers filtering models with None fields Fixed bug where models with No provider_model_id were incorrectly filtered from the startup config display. The function was checking multiple fields when it should only filter items with explicitly disabled provider_id. Changes: o Modified remove_disabled_providers to only check provider_id field o Changed condition from checking multiple fields with None to only checking provider_id for "__disabled__", None or empty string o Added comprehensive unit tests Closes: #4131 Signed-off-by: Derek Higgins --- src/llama_stack/core/server/server.py | 4 +- tests/unit/server/test_server.py | 69 ++++++++++++++++++++++++++- 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/src/llama_stack/core/server/server.py b/src/llama_stack/core/server/server.py index 80505c3f9..5bf876c02 100644 --- a/src/llama_stack/core/server/server.py +++ b/src/llama_stack/core/server/server.py @@ -526,8 +526,8 @@ def extract_path_params(route: str) -> list[str]: def remove_disabled_providers(obj): if isinstance(obj, dict): - keys = ["provider_id", "shield_id", "provider_model_id", "model_id"] - if any(k in obj and obj[k] in ("__disabled__", "", None) for k in keys): + # Filter out items where provider_id is explicitly disabled or empty + if "provider_id" in obj and obj["provider_id"] in ("__disabled__", "", None): return None return {k: v for k, v in ((k, remove_disabled_providers(v)) for k, v in obj.items()) if v is not None} elif isinstance(obj, list): diff --git a/tests/unit/server/test_server.py b/tests/unit/server/test_server.py index d6d4f4f23..53f193672 100644 --- a/tests/unit/server/test_server.py +++ b/tests/unit/server/test_server.py @@ -12,7 +12,7 @@ from pydantic import ValidationError from llama_stack.core.access_control.access_control import AccessDeniedError from llama_stack.core.datatypes import AuthenticationRequiredError -from llama_stack.core.server.server import translate_exception +from llama_stack.core.server.server import remove_disabled_providers, translate_exception class TestTranslateException: @@ -194,3 +194,70 @@ class TestTranslateException: assert isinstance(result3, HTTPException) assert result3.status_code == 403 assert result3.detail == "Permission denied: Access denied" + + +class TestRemoveDisabledProviders: + """Test cases for the remove_disabled_providers function.""" + + def test_remove_explicitly_disabled_provider(self): + """Test that providers with provider_id='__disabled__' are removed.""" + config = { + "providers": { + "inference": [ + {"provider_id": "openai", "provider_type": "remote::openai", "config": {}}, + {"provider_id": "__disabled__", "provider_type": "remote::vllm", "config": {}}, + ] + } + } + result = remove_disabled_providers(config) + assert len(result["providers"]["inference"]) == 1 + assert result["providers"]["inference"][0]["provider_id"] == "openai" + + def test_remove_empty_provider_id(self): + """Test that providers with empty provider_id are removed.""" + config = { + "providers": { + "inference": [ + {"provider_id": "openai", "provider_type": "remote::openai", "config": {}}, + {"provider_id": "", "provider_type": "remote::vllm", "config": {}}, + ] + } + } + result = remove_disabled_providers(config) + assert len(result["providers"]["inference"]) == 1 + assert result["providers"]["inference"][0]["provider_id"] == "openai" + + def test_keep_models_with_none_provider_model_id(self): + """Test that models with None provider_model_id are NOT removed.""" + config = { + "registered_resources": { + "models": [ + { + "model_id": "llama-3-2-3b", + "provider_id": "vllm-inference", + "model_type": "llm", + "provider_model_id": None, + "metadata": {}, + }, + { + "model_id": "gpt-4o-mini", + "provider_id": "openai", + "model_type": "llm", + "provider_model_id": None, + "metadata": {}, + }, + { + "model_id": "granite-embedding-125m", + "provider_id": "sentence-transformers", + "model_type": "embedding", + "provider_model_id": "ibm-granite/granite-embedding-125m-english", + "metadata": {"embedding_dimension": 768}, + }, + ] + } + } + result = remove_disabled_providers(config) + assert len(result["registered_resources"]["models"]) == 3 + assert result["registered_resources"]["models"][0]["model_id"] == "llama-3-2-3b" + assert result["registered_resources"]["models"][1]["model_id"] == "gpt-4o-mini" + assert result["registered_resources"]["models"][2]["model_id"] == "granite-embedding-125m"