diff --git a/backend/routes/models.py b/backend/routes/models.py index 600e0221..b7533465 100644 --- a/backend/routes/models.py +++ b/backend/routes/models.py @@ -59,7 +59,7 @@ def _resolve_model_config(model_name: str): known = sorted(cfg.model_name for cfg in get_all_model_configs()) raise HTTPException( status_code=400, - detail=f"Unknown model: {model_name}. Available: {known}", + detail=f"Unknown model: {model_name!r}. Available: {known}", ) return config @@ -71,10 +71,12 @@ async def load_model( ): """Load a model into memory. - Pass ``{"model_name": "kokoro"}`` to target any registered engine. Without - a name this falls back to the default Qwen TTS backend, selected by the - ``model_size`` query parameter, which is what callers did before - ``model_name`` existed. + Pass ``{"model_name": "kokoro"}`` to target any registered engine. Only an + absent ``model_name`` falls back to the default Qwen TTS backend, selected + by the ``model_size`` query parameter, which is what callers did before + ``model_name`` existed. A supplied-but-empty name is a bad request, not a + request for the default — silently loading Qwen there is the very bug this + endpoint had. """ from ..backends import get_model_load_func from ..services import tts @@ -82,7 +84,7 @@ async def load_model( requested_name = request.model_name if request else None requested_size = (request.model_size if request else None) or model_size - if requested_name: + if requested_name is not None: config = _resolve_model_config(requested_name) try: result = get_model_load_func(config)() @@ -106,15 +108,16 @@ async def unload_model(request: models.ModelLoadRequest | None = None): """Unload a model from memory. Pass ``{"model_name": "chatterbox-tts"}`` to target a specific engine — - same behaviour as ``POST /models/{model_name}/unload``. Without a name - this unloads the default Qwen TTS model, as it always has. + same behaviour as ``POST /models/{model_name}/unload``. Only an absent + ``model_name`` unloads the default Qwen TTS model, as it always has; a + supplied-but-empty name is a bad request. """ from ..backends import unload_model_by_config from ..services import tts requested_name = request.model_name if request else None - if requested_name: + if requested_name is not None: config = _resolve_model_config(requested_name) try: was_loaded = unload_model_by_config(config) diff --git a/backend/tests/test_models_load_by_name.py b/backend/tests/test_models_load_by_name.py index 4773c76a..1e1f50a3 100644 --- a/backend/tests/test_models_load_by_name.py +++ b/backend/tests/test_models_load_by_name.py @@ -96,12 +96,32 @@ def test_load_unknown_model_is_rejected(client, loaded): assert loaded["qwen_size"] == [] +def test_load_empty_model_name_is_rejected(client, loaded): + """A supplied-but-empty name is a bad request, not a request for Qwen. + + Falling back here would reproduce #977 in miniature: a caller that named a + model (an unbound UI select, say) gets a silent 3.6 GB Qwen download. + """ + response = client.post("/models/load", json={"model_name": ""}) + + assert response.status_code == 400 + assert loaded["by_config"] == [] + assert loaded["qwen_size"] == [] + + def test_load_without_body_keeps_the_qwen_default(client, loaded): """Pre-existing callers post no body at all.""" assert client.post("/models/load").status_code == 200 assert loaded["qwen_size"] == ["1.7B"] +def test_load_null_model_name_keeps_the_qwen_default(client, loaded): + """An explicit null is 'unspecified', unlike an empty string.""" + assert client.post("/models/load", json={"model_name": None}).status_code == 200 + assert loaded["qwen_size"] == ["1.7B"] + assert loaded["by_config"] == [] + + def test_load_still_honors_the_model_size_query_param(client, loaded): """The legacy ?model_size= form must keep selecting the Qwen variant.""" assert client.post("/models/load?model_size=0.6B").status_code == 200 @@ -121,12 +141,26 @@ def test_unload_by_model_name_targets_that_model(client, loaded): assert loaded["qwen_unload"] == 0, "must not unload the default Qwen model instead" +def test_unload_empty_model_name_is_rejected(client, loaded): + response = client.post("/models/unload", json={"model_name": ""}) + + assert response.status_code == 400 + assert loaded["by_config"] == [] + assert loaded["qwen_unload"] == 0 + + def test_unload_without_body_keeps_the_qwen_default(client, loaded): assert client.post("/models/unload").status_code == 200 assert loaded["qwen_unload"] == 1 assert loaded["by_config"] == [] +def test_unload_null_model_name_keeps_the_qwen_default(client, loaded): + assert client.post("/models/unload", json={"model_name": None}).status_code == 200 + assert loaded["qwen_unload"] == 1 + assert loaded["by_config"] == [] + + def test_unload_unknown_model_is_rejected(client, loaded): response = client.post("/models/unload", json={"model_name": "not-a-model"})