diff --git a/app/api/routes/llm.py b/app/api/routes/llm.py index 3a206ed..3d35aa6 100644 --- a/app/api/routes/llm.py +++ b/app/api/routes/llm.py @@ -6,6 +6,7 @@ from fastapi import APIRouter from fastapi import Depends from fastapi.responses import StreamingResponse +from starlette.background import BackgroundTask from sqlmodel import Session from app.api.deps import get_db @@ -13,6 +14,8 @@ from app.api.schemas.system import ( DownloadableModel, LLMModelChoice, + LLMModelDeleteRequest, + LLMModelDeleteResult, LLMModels, LLMProviderModels, LLMPullRequest, @@ -21,7 +24,11 @@ ModelInfo, ) from app.services import llm -from app.services.llm.errors import LLMNotAvailableError +from app.services.llm.errors import ( + LLMModelInUseError, + LLMModelNotInstalledError, + LLMNotAvailableError, +) from app.services.llm.ollama import OllamaLLM router = APIRouter(prefix="/system/llm", tags=["system"]) @@ -96,4 +103,50 @@ def pull_llm_model(req: LLMPullRequest, db: Session = Depends(get_db)): req.model, on_success=lambda: registry.provider_catalog(LLMProvider.ollama, refresh=True), ) - return StreamingResponse(stream, media_type="application/x-ndjson") + return StreamingResponse( + stream, + media_type="application/x-ndjson", + background=BackgroundTask(stream.close), + ) + + +@router.delete( + "/models", + response_model=LLMModelDeleteResult, + responses={ + 404: {"description": "Ollama does not have the model installed"}, + 409: {"description": "The model is a default or currently being pulled"}, + 422: {"description": "Invalid request or model is not allowed"}, + 502: {"description": "Ollama returned an unexpected response"}, + 503: {"description": "Ollama service unavailable"}, + }, + summary="Delete an installed Ollama model", +) +def delete_llm_model(req: LLMModelDeleteRequest, db: Session = Depends(get_db)): + registry = llm.effective_registry(db) + registry.validate_pull(req.provider, req.model) + + client = registry.client(LLMProvider.ollama) + if not isinstance(client, OllamaLLM): + raise LLMNotAvailableError("Model deletion is only supported for Ollama.") + + catalog = registry.provider_catalog(LLMProvider.ollama, refresh=True) + if catalog.error: + raise type(catalog.error)(catalog.error.message) + if not any(model.name == req.model and model.installed is True for model in catalog.models): + raise LLMModelNotInstalledError(f"Ollama does not have model {req.model} installed.") + + ollama_default = registry.settings.providers[LLMProvider.ollama].default_model + if ( + registry.default.provider is LLMProvider.ollama + and registry.default.model == req.model + ): + raise LLMModelInUseError(f"Cannot delete {req.model}: it is the global default model.") + if ollama_default == req.model: + raise LLMModelInUseError(f"Cannot delete {req.model}: it is the Ollama provider default model.") + if client.is_model_pulling(req.model): + raise LLMModelInUseError(f"Cannot delete {req.model}: a download is currently active.") + + client.delete_model(req.model) + registry.provider_catalog(LLMProvider.ollama, refresh=True) + return LLMModelDeleteResult(model=req.model, deleted=True) diff --git a/app/api/schemas/system.py b/app/api/schemas/system.py index 18e2360..b3f958b 100644 --- a/app/api/schemas/system.py +++ b/app/api/schemas/system.py @@ -103,6 +103,21 @@ def _validate_model_name(cls, value: str) -> str: return validate_model_name(value) +class LLMModelDeleteRequest(BaseModel): + model: str + provider: str | None = "ollama" + + @field_validator("model") + @classmethod + def _validate_model_name(cls, value: str) -> str: + return validate_model_name(value) + + +class LLMModelDeleteResult(BaseModel): + model: str + deleted: bool + + class LLMTestRequest(BaseModel): provider: LLMProvider | None = None model: str | None = None diff --git a/app/services/llm/__init__.py b/app/services/llm/__init__.py index 280c462..d6bd6d0 100644 --- a/app/services/llm/__init__.py +++ b/app/services/llm/__init__.py @@ -20,6 +20,8 @@ LLMConfigError, LLMError, LLMExternalConsentError, + LLMModelInUseError, + LLMModelNotInstalledError, LLMModelNotFoundError, LLMNotAvailableError, LLMRateLimitError, @@ -48,6 +50,8 @@ def get_registry() -> LLMRegistry: "LLMError", "LLMExternalConsentError", "LLMHealth", + "LLMModelInUseError", + "LLMModelNotInstalledError", "LLMModelNotFoundError", "LLMNotAvailableError", "LLMProvider", diff --git a/app/services/llm/errors.py b/app/services/llm/errors.py index adb7d61..a7b720b 100644 --- a/app/services/llm/errors.py +++ b/app/services/llm/errors.py @@ -68,6 +68,20 @@ class LLMModelNotFoundError(LLMError): error_code = "LLM_MODEL_NOT_FOUND" +class LLMModelNotInstalledError(LLMError): + """Ollama does not have the requested model installed.""" + + status_code = 404 + error_code = "LLM_MODEL_NOT_INSTALLED" + + +class LLMModelInUseError(LLMError): + """The model cannot be deleted while it is a default or a pull is active.""" + + status_code = 409 + error_code = "LLM_MODEL_IN_USE" + + class LLMResponseError(LLMError): """The server answered, but not with a usable JSON object.""" diff --git a/app/services/llm/ollama.py b/app/services/llm/ollama.py index 7ca8fdd..d7ce994 100644 --- a/app/services/llm/ollama.py +++ b/app/services/llm/ollama.py @@ -6,6 +6,7 @@ import json import logging +import threading import time from collections.abc import Callable, Iterator @@ -13,10 +14,86 @@ from app.api.schemas.enums import HealthState, LLMProvider from app.services.llm.base import LLM, PROBE_TIMEOUT_SECONDS, LLMHealth, ModelInfo -from app.services.llm.errors import LLMError, LLMModelNotFoundError, LLMResponseError, LLMUnavailableError +from app.services.llm.errors import ( + LLMError, + LLMModelNotFoundError, + LLMModelNotInstalledError, + LLMResponseError, + LLMUnavailableError, +) logger = logging.getLogger(__name__) +_ACTIVE_PULL_MODELS: set[str] = set() +_ACTIVE_PULL_COUNTS: dict[str, int] = {} +_ACTIVE_PULL_LOCK = threading.Lock() + + +def _register_active_pull(model: str) -> None: + with _ACTIVE_PULL_LOCK: + _ACTIVE_PULL_COUNTS[model] = _ACTIVE_PULL_COUNTS.get(model, 0) + 1 + _ACTIVE_PULL_MODELS.add(model) + + +def _unregister_active_pull(model: str) -> None: + with _ACTIVE_PULL_LOCK: + count = _ACTIVE_PULL_COUNTS.get(model, 0) + if count <= 1: + _ACTIVE_PULL_COUNTS.pop(model, None) + _ACTIVE_PULL_MODELS.discard(model) + else: + _ACTIVE_PULL_COUNTS[model] = count - 1 + + +class _PullStream(Iterator[str]): + def __init__(self, response: requests.Response, iterator: Iterator[str], model: str): + self._response = response + self._iterator = iterator + self._model = model + self._started = False + self._finished = False + self._closed = False + + def __iter__(self) -> "_PullStream": + return self + + def __next__(self) -> str: + if self._closed: + raise StopIteration + self._started = True + try: + return next(self._iterator) + except StopIteration: + self._finished = True + self.close() + raise + except BaseException: + self.close() + raise + + def close(self) -> None: + if self._closed: + return + self._closed = True + try: + close = getattr(self._iterator, "close", None) + if close: + close() + finally: + try: + self._response.close() + finally: + _unregister_active_pull(self._model) + if not self._started and not self._finished: + logger.info("pull of %s cancelled by the client", self._model) + + def __del__(self): + try: + self.close() + except Exception: + logger.exception("Failed to close Ollama pull stream for %s", self._model) + + OPTIONS = { "num_ctx": 32768, # Backstop for runaway generation; the largest real output (ICS 209) is ~8k tokens @@ -95,14 +172,58 @@ def _to_models(tags: dict, running: list[str]) -> list[ModelInfo]: if m.get("name") ] - def pull_model(self, model: str, on_success: Callable[[], None] | None = None) -> Iterator[str]: + def pull_model(self, model: str, on_success: Callable[[], None] | None = None) -> "_PullStream": """Stream pull progress from Ollama as newline-delimited JSON. Opens the connection before returning the generator so a down Ollama raises LLMUnavailableError (HTTP 503) instead of committing a 200 response. """ - response = self._open_pull_stream(model) - return self._stream_pull(response, on_success=on_success) + _register_active_pull(model) + try: + response = self._open_pull_stream(model) + except BaseException: + _unregister_active_pull(model) + raise + try: + return self._stream_pull(response, on_success=on_success, model=model) + except BaseException: + try: + response.close() + finally: + _unregister_active_pull(model) + raise + + @staticmethod + def is_model_pulling(model: str) -> bool: + with _ACTIVE_PULL_LOCK: + return model in _ACTIVE_PULL_MODELS + + def delete_model(self, model: str) -> None: + """Delete an installed model from Ollama.""" + try: + response = requests.delete( + self.host + "/api/delete", + json={"model": model}, + timeout=PROBE_TIMEOUT_SECONDS, + ) + except requests.Timeout as exc: + raise LLMUnavailableError(f"Ollama at {self.host} timed out deleting {model}") from exc + except requests.RequestException as exc: + raise LLMUnavailableError(f"Could not reach Ollama at {self.host}. Check OLLAMA_HOST: {exc}") from exc + + try: + if response.status_code == 404: + raise LLMModelNotInstalledError(f"Ollama does not have model {model} installed.") + if response.status_code >= 500: + raise LLMUnavailableError( + f"Ollama had a server error {response.status_code}: {_reason(response)}" + ) + if not 200 <= response.status_code < 300: + raise LLMResponseError( + f"Ollama refused to delete {model} with {response.status_code}: {_reason(response)}" + ) + finally: + response.close() def _open_pull_stream(self, model: str) -> requests.Response: url = self.host + "/api/pull" @@ -121,7 +242,15 @@ def _open_pull_stream(self, model: str) -> requests.Response: raise LLMUnavailableError(f"Could not reach Ollama at {self.host}. Check OLLAMA_HOST: {exc}") from exc def _stream_pull( - self, response: requests.Response, on_success: Callable[[], None] | None = None + self, response: requests.Response, on_success: Callable[[], None] | None = None, model: str = "" + ) -> _PullStream: + return _PullStream(response, self._iterate_pull(response, on_success, model), model) + + def _iterate_pull( + self, + response: requests.Response, + on_success: Callable[[], None] | None = None, + model: str = "", ) -> Iterator[str]: saw_error = False success_line: str | None = None @@ -164,6 +293,9 @@ def inspect_line(line: str) -> bool: if buffer.strip(): if not inspect_line(buffer): yield buffer.strip() + "\n" + except GeneratorExit: + logger.info("pull of %s cancelled by the client", model) + raise except requests.RequestException as exc: saw_error = True yield json.dumps({"error": str(exc)}) + "\n" diff --git a/contracts/path/system.yaml b/contracts/path/system.yaml index f6c1d4f..751d0a0 100644 --- a/contracts/path/system.yaml +++ b/contracts/path/system.yaml @@ -5,6 +5,7 @@ # GET /api/v1/system/llm/models # POST /api/v1/system/llm/test # POST /api/v1/system/llm/pull +# DELETE /api/v1/system/llm/models # GET /api/v1/system/llm/providers/config # PUT/DELETE /api/v1/system/llm/providers/config/{provider} # POST /api/v1/system/llm/providers/config/{provider}/validate @@ -230,6 +231,89 @@ llm_models: example: error_code: "LLM_CONFIG_ERROR" message: "GEMINI_API_KEY is set but GEMINI_MODELS is empty. Add GEMINI_MODELS=gemini-2.5-flash, a comma separated list, or * for every model gemini lists." + delete: + operationId: deleteLLMModel + summary: Delete an installed Ollama model + description: | + Deletes an installed Ollama model. Only Ollama is supported, and the model + must be permitted by OLLAMA_MODELS (and OLLAMA_INSTALLABLE_MODELS when + OLLAMA_MODELS=*). The global default model, Ollama's provider default, and + models with an active pull cannot be deleted. A successful deletion + refreshes the Ollama catalog cache. + tags: + - system + requestBody: + required: true + content: + application/json: + schema: + $ref: "../schemas/system.yaml#/LLMModelDeleteRequest" + example: + provider: "ollama" + model: "qwen2.5:3b" + responses: + "200": + description: The Ollama model was deleted + content: + application/json: + schema: + $ref: "../schemas/system.yaml#/LLMModelDeleteResult" + example: + model: "qwen2.5:3b" + deleted: true + "404": + description: Ollama does not have the model installed + content: + application/json: + schema: + $ref: "../schemas/common.yaml#/ErrorResponse" + example: + error_code: "LLM_MODEL_NOT_INSTALLED" + message: "Ollama does not have model qwen2.5:3b installed." + "409": + description: The model is a default or currently being pulled + content: + application/json: + schema: + $ref: "../schemas/common.yaml#/ErrorResponse" + examples: + default: + summary: Model is configured as a default + value: + error_code: "LLM_MODEL_IN_USE" + message: "Cannot delete qwen2.5:1.5b: it is the Ollama provider default model." + active_pull: + summary: Model is currently being downloaded + value: + error_code: "LLM_MODEL_IN_USE" + message: "Cannot delete qwen2.5:3b: a download is currently active." + "422": + description: Invalid model name, provider is not Ollama, Ollama is disabled, or model is not allowed + content: + application/json: + schema: + $ref: "../schemas/common.yaml#/ErrorResponse" + example: + error_code: "LLM_NOT_AVAILABLE" + message: "mistral:7b is not in OLLAMA_MODELS." + "502": + description: Ollama returned an unexpected response + content: + application/json: + schema: + $ref: "../schemas/common.yaml#/ErrorResponse" + example: + error_code: "LLM_BAD_RESPONSE" + message: "Ollama refused to delete qwen2.5:3b with 400." + "503": + description: Ollama could not be reached + content: + application/json: + schema: + $ref: "../schemas/common.yaml#/ErrorResponse" + example: + error_code: "LLM_UNAVAILABLE" + message: "Could not reach Ollama at http://ollama:11434. Check OLLAMA_HOST." llm_test: post: diff --git a/contracts/schemas/system.yaml b/contracts/schemas/system.yaml index b1869a8..abf2de1 100644 --- a/contracts/schemas/system.yaml +++ b/contracts/schemas/system.yaml @@ -296,6 +296,33 @@ LLMPullRequest: default: ollama description: Must be ollama; other providers are rejected +LLMModelDeleteRequest: + type: object + required: + - model + properties: + model: + type: string + maxLength: 128 + pattern: "^[A-Za-z0-9][A-Za-z0-9._\\-/:]*$" + description: Ollama model tag to delete + provider: + type: string + nullable: true + default: ollama + description: Must be ollama; other providers are rejected + +LLMModelDeleteResult: + type: object + required: + - model + - deleted + properties: + model: + type: string + deleted: + type: boolean + LLMModels: type: object required: diff --git a/docs/multi-llm-support/api.md b/docs/multi-llm-support/api.md index 781b887..77ee091 100644 --- a/docs/multi-llm-support/api.md +++ b/docs/multi-llm-support/api.md @@ -88,6 +88,42 @@ If Ollama cannot be reached before the stream opens, the API returns **503** wit After a successful pull, the server refreshes the Ollama catalog cache, so the new model shows up on `GET /system/llm/models` immediately (no need to wait 60 seconds or pass `refresh=true`). +Closing or cancelling the client's pull request cancels the upstream Ollama +request, closes its connection, and does not emit a success line or refresh the +catalog. Cancelling a pull means closing the request. Ollama keeps the partial +download, so pulling the same model again resumes from where it stopped (checked +with the bundled Ollama; it may vary between Ollama versions). + +## Delete an Ollama model + +`DELETE /api/v1/system/llm/models` + +```json +{ "model": "qwen2.5:3b" } +``` + +`provider` is optional and defaults to `ollama`. The model must pass the same +name and installable-model checks as a pull, and it must currently be installed. +The global default model, Ollama's provider default model, and a model with an +active pull cannot be deleted. + +Success returns `200` and refreshes the Ollama catalog cache: + +```json +{ "model": "qwen2.5:3b", "deleted": true } +``` + +The endpoint returns `404` (`LLM_MODEL_NOT_INSTALLED`) when Ollama does not +have the model, `409` (`LLM_MODEL_IN_USE`) when it is a default or its pull is +active, and `422` (`LLM_NOT_AVAILABLE`) for an invalid provider or model that +the server does not allow. Ollama connection failures and server errors return +`503` (`LLM_UNAVAILABLE`); other unexpected Ollama responses return +`502` (`LLM_BAD_RESPONSE`). + +Active pull tracking is held in a lock-protected in-process set. In deployments +with multiple API processes, it is process-local and therefore only +authoritative for pulls handled by the same process. + ## Test a model `POST /api/v1/system/llm/test` @@ -134,6 +170,8 @@ The pair is checked before anything is stored or queued. For jobs, the provider | `LLM_CONFIG_ERROR` | 500 | The LLM settings are wrong. The message names the setting and the fix. | | `LLM_AUTH_FAILED` | 502 | The provider rejected the API key. | | `LLM_MODEL_NOT_FOUND` | 502 | The provider has no such model. | +| `LLM_MODEL_NOT_INSTALLED` | 404 | Ollama does not have the requested model installed. | +| `LLM_MODEL_IN_USE` | 409 | The Ollama model is a configured default or has an active pull. | | `LLM_BAD_RESPONSE` | 502 | The provider refused the request or did not return JSON. | | `LLM_UNAVAILABLE` | 503 | Could not reach the provider, or it timed out. | | `LLM_RATE_LIMITED` | 503 | Still rate limited after retries, or out of quota. | diff --git a/docs/multi-llm-support/troubleshooting.md b/docs/multi-llm-support/troubleshooting.md index 409c502..bff92b3 100644 --- a/docs/multi-llm-support/troubleshooting.md +++ b/docs/multi-llm-support/troubleshooting.md @@ -23,6 +23,8 @@ Check `make logs-app` for the app and `make logs-worker` for queued fills. Every | --------------------- | ----------------------------------------------------- | -------------------------------------------------------------------------------- | | `LLM_AUTH_FAILED` | Wrong or expired key | Replace `_API_KEY`, then `make up`. | | `LLM_MODEL_NOT_FOUND` | Typo, retired model, or not pulled in Ollama | Pick a model from `GET /api/v1/system/llm/models`. If it appears under `downloadable`, install it with `POST /api/v1/system/llm/pull` (when the server allows that model), or run `ollama pull ` on the host. | +| `LLM_MODEL_NOT_INSTALLED` | Ollama does not have the model being deleted | Install it first, or choose a model shown as installed in `GET /api/v1/system/llm/models`. | +| `LLM_MODEL_IN_USE` | The model is a default or has an active pull | Change the configured default or wait for/cancel the pull, then retry `DELETE /api/v1/system/llm/models`. | | `LLM_UNAVAILABLE` | Server down, wrong URL, or timeout | Check `_BASE_URL` or `OLLAMA_HOST`. Raise `LLM_TIMEOUT` for slow models. | | `LLM_RATE_LIMITED` | Provider quota or rate limit | Wait, or check the plan on the provider side. | | `LLM_BAD_RESPONSE` | Server rejected an option, or the answer was cut off | Set `_STRUCTURED_OUTPUT=false`, or raise `_MAX_TOKENS`. | diff --git a/tests/test_llm_registry.py b/tests/test_llm_registry.py index bcab69d..2f0458c 100644 --- a/tests/test_llm_registry.py +++ b/tests/test_llm_registry.py @@ -1066,3 +1066,159 @@ def pull(model, on_success=None): lines = [json.loads(line) for line in resp.text.strip().split("\n") if line.strip()] assert any("error" in p for p in lines) assert not any(p.get("status") == "success" for p in lines) + + +class TestDeleteEndpoint: + + _DELETE = f"{API_PREFIX}/system/llm/models" + + def test_delete_success_refreshes_catalog(self, client, test_llm, monkeypatch): + offered = ["qwen2.5:1.5b", "qwen2.5:3b"] + monkeypatch.setattr( + test_llm, + "list_models", + lambda: [ModelInfo(name=name, installed=True) for name in offered], + ) + + before = client.get(f"{API_PREFIX}/system/llm/models") + assert before.status_code == 200 + before_models = before.json()["providers"][0]["models"] + assert any(model["name"] == "qwen2.5:3b" for model in before_models) + + deleted = [] + + def delete_model(model): + deleted.append(model) + offered.remove(model) + + monkeypatch.setattr(test_llm, "delete_model", delete_model) + response = client.request("DELETE", self._DELETE, json={"model": "qwen2.5:3b"}) + + assert response.status_code == 200 + assert response.json() == {"model": "qwen2.5:3b", "deleted": True} + assert deleted == ["qwen2.5:3b"] + after = client.get(f"{API_PREFIX}/system/llm/models").json()["providers"][0] + assert not any(model["name"] == "qwen2.5:3b" for model in after["models"]) + assert any( + model["name"] == "qwen2.5:3b" and model["installed"] is False + for model in after["downloadable"] + ) + + def test_delete_rejects_invalid_model_name(self, client): + response = client.request("DELETE", self._DELETE, json={"model": "bad model!"}) + + assert response.status_code == 422 + assert response.json()["error_code"] == "VALIDATION_ERROR" + + def test_delete_rejects_non_ollama_provider(self, client): + response = client.request( + "DELETE", + self._DELETE, + json={"provider": "gemini", "model": "gemini-2.5-flash"}, + ) + + assert response.status_code == 422 + assert response.json()["error_code"] == "LLM_NOT_AVAILABLE" + + def test_delete_rejects_when_ollama_disabled(self, client, monkeypatch): + settings = LLMSettings( + providers={GEMINI: _provider(GEMINI, "g1", "g1")}, + default_provider=GEMINI, + ) + disabled_registry = LLMRegistry(settings, {GEMINI: FakeLLM(GEMINI, ["g1"])}) + monkeypatch.setattr( + "app.api.routes.llm.llm.effective_registry", + lambda _db: disabled_registry, + ) + + response = client.request("DELETE", self._DELETE, json={"model": "qwen2.5:3b"}) + + assert response.status_code == 422 + assert response.json()["error_code"] == "LLM_NOT_AVAILABLE" + + def test_delete_rejects_model_outside_ollama_models(self, client): + response = client.request("DELETE", self._DELETE, json={"model": "secret:model"}) + + assert response.status_code == 422 + assert response.json()["error_code"] == "LLM_NOT_AVAILABLE" + + def test_delete_returns_not_installed_for_missing_model(self, client, test_llm, monkeypatch): + monkeypatch.setattr( + test_llm, + "list_models", + lambda: [ + ModelInfo(name="qwen2.5:1.5b", installed=True), + ModelInfo(name="qwen2.5:3b", manual=True), + ], + ) + delete = MagicMock() + monkeypatch.setattr(test_llm, "delete_model", delete) + + response = client.request("DELETE", self._DELETE, json={"model": "qwen2.5:3b"}) + + assert response.status_code == 404 + assert response.json()["error_code"] == "LLM_MODEL_NOT_INSTALLED" + delete.assert_not_called() + + def test_delete_rejects_global_default_model(self, client, test_llm, monkeypatch): + monkeypatch.setattr( + test_llm, + "list_models", + lambda: [ModelInfo(name="qwen2.5:1.5b", installed=True)], + ) + + response = client.request("DELETE", self._DELETE, json={"model": "qwen2.5:1.5b"}) + + assert response.status_code == 409 + assert response.json()["error_code"] == "LLM_MODEL_IN_USE" + assert "global default" in response.json()["message"] + + def test_delete_rejects_ollama_provider_default(self, client, test_registry, test_llm, monkeypatch): + settings = LLMSettings( + providers={ + OLLAMA: test_registry.settings.providers[OLLAMA], + GEMINI: _provider(GEMINI, "g1", "g1"), + }, + default_provider=GEMINI, + ) + registry = LLMRegistry( + settings, + {OLLAMA: test_llm, GEMINI: FakeLLM(GEMINI, ["g1"])}, + ) + monkeypatch.setattr( + "app.api.routes.llm.llm.effective_registry", + lambda _db: registry, + ) + monkeypatch.setattr( + test_llm, + "list_models", + lambda: [ModelInfo(name="qwen2.5:1.5b", installed=True)], + ) + + response = client.request("DELETE", self._DELETE, json={"model": "qwen2.5:1.5b"}) + + assert response.status_code == 409 + assert response.json()["error_code"] == "LLM_MODEL_IN_USE" + assert "Ollama provider default" in response.json()["message"] + + def test_delete_rejects_model_with_active_pull(self, client, test_llm, monkeypatch): + from app.services.llm.ollama import _register_active_pull, _unregister_active_pull + + model = "qwen2.5:3b" + monkeypatch.setattr( + test_llm, + "list_models", + lambda: [ + ModelInfo(name="qwen2.5:1.5b", installed=True), + ModelInfo(name=model, installed=True), + ], + ) + _register_active_pull(model) + try: + response = client.request("DELETE", self._DELETE, json={"model": model}) + finally: + _unregister_active_pull(model) + + assert response.status_code == 409 + assert response.json()["error_code"] == "LLM_MODEL_IN_USE" + assert "download is currently active" in response.json()["message"] diff --git a/tests/test_llm_service.py b/tests/test_llm_service.py index ad8043a..202dfe4 100644 --- a/tests/test_llm_service.py +++ b/tests/test_llm_service.py @@ -866,6 +866,7 @@ def __init__(self, chunks: list[bytes], raise_at: int | None = None, raise_exc=N self._raise_at = raise_at # raise raise_exc after this many chunks self._raise_exc = raise_exc self.status_code = 200 + self.closed = False def raise_for_status(self): pass @@ -882,6 +883,9 @@ def __enter__(self): def __exit__(self, *a): pass + def close(self): + self.closed = True + def _pull_llm(pull_timeout=30): return OllamaLLM(host="http://ollama:11434", model="qwen2.5:1.5b", timeout=300, pull_timeout=pull_timeout) @@ -909,6 +913,147 @@ def test_happy_path_streams_lines_and_calls_callback(self, monkeypatch): assert parsed[-1]["status"] == "success" assert fired == [1] + def test_client_cancellation_closes_response_and_cleans_active_pull(self, monkeypatch, caplog): + from app.services.llm.ollama import _ACTIVE_PULL_MODELS + + model = "qwen2.5:3b" + response = _FakeStreamResponse([ + b'{"status": "downloading"}\n', + b'{"status": "success"}\n', + ]) + monkeypatch.setattr("app.services.llm.ollama.requests.post", lambda *a, **k: response) + fired = [] + stream = _pull_llm().pull_model(model, on_success=lambda: fired.append(True)) + + assert model in _ACTIVE_PULL_MODELS + assert next(stream) == '{"status": "downloading"}\n' + stream.close() + + assert response.closed + assert fired == [] + assert model not in _ACTIVE_PULL_MODELS + with pytest.raises(StopIteration): + next(stream) + assert [record.getMessage() for record in caplog.records].count( + f"pull of {model} cancelled by the client" + ) == 1 + + def test_uniterated_client_cancellation_closes_response_and_cleans_active_pull(self, monkeypatch): + from app.services.llm.ollama import _ACTIVE_PULL_MODELS + + model = "qwen2.5:3b" + response = _FakeStreamResponse([b'{"status": "success"}\n']) + monkeypatch.setattr("app.services.llm.ollama.requests.post", lambda *a, **k: response) + fired = [] + stream = _pull_llm().pull_model(model, on_success=lambda: fired.append(True)) + + assert model in _ACTIVE_PULL_MODELS + stream.close() + + assert response.closed + assert fired == [] + assert model not in _ACTIVE_PULL_MODELS + + def test_completed_pull_cleans_active_model(self, monkeypatch): + from app.services.llm.ollama import _ACTIVE_PULL_MODELS + + model = "qwen2.5:3b" + response = _FakeStreamResponse([b'{"status": "success"}\n']) + monkeypatch.setattr("app.services.llm.ollama.requests.post", lambda *a, **k: response) + + assert list(_pull_llm().pull_model(model)) == ['{"status": "success"}\n'] + + assert response.closed + assert model not in _ACTIVE_PULL_MODELS + + def test_failed_pull_open_cleans_active_model(self, monkeypatch): + from app.services.llm.ollama import _ACTIVE_PULL_MODELS + + model = "qwen2.5:3b" + monkeypatch.setattr( + "app.services.llm.ollama.requests.post", + lambda *a, **k: (_ for _ in ()).throw(requests.exceptions.ConnectionError("down")), + ) + + with pytest.raises(LLMUnavailableError): + _pull_llm().pull_model(model) + + assert model not in _ACTIVE_PULL_MODELS + + +class _FakeDeleteResponse: + def __init__(self, status_code, body=None): + self.status_code = status_code + self._body = body or {} + self.text = str(self._body) + self.closed = False + + def json(self): + return self._body + + def close(self): + self.closed = True + + +class TestOllamaDelete: + + def test_delete_model_sends_ollama_request(self, monkeypatch): + response = _FakeDeleteResponse(200) + calls = [] + + def fake_delete(url, **kwargs): + calls.append((url, kwargs)) + return response + + monkeypatch.setattr("app.services.llm.ollama.requests.delete", fake_delete) + _pull_llm().delete_model("qwen2.5:3b") + + assert calls == [( + "http://ollama:11434/api/delete", + {"json": {"model": "qwen2.5:3b"}, "timeout": 5}, + )] + assert response.closed + + def test_delete_model_reports_not_installed(self, monkeypatch): + from app.services.llm import LLMModelNotInstalledError + + response = _FakeDeleteResponse(404, {"error": "model not found"}) + monkeypatch.setattr("app.services.llm.ollama.requests.delete", lambda *a, **k: response) + + with pytest.raises(LLMModelNotInstalledError) as info: + _pull_llm().delete_model("missing:1b") + + assert info.value.status_code == 404 + assert info.value.error_code == "LLM_MODEL_NOT_INSTALLED" + assert response.closed + + def test_delete_model_maps_unreachable_ollama_to_unavailable(self, monkeypatch): + monkeypatch.setattr( + "app.services.llm.ollama.requests.delete", + lambda *a, **k: (_ for _ in ()).throw(requests.exceptions.ConnectionError("down")), + ) + + with pytest.raises(LLMUnavailableError): + _pull_llm().delete_model("qwen2.5:3b") + + def test_delete_model_maps_server_error_to_unavailable(self, monkeypatch): + response = _FakeDeleteResponse(500, {"error": "server failed"}) + monkeypatch.setattr("app.services.llm.ollama.requests.delete", lambda *a, **k: response) + + with pytest.raises(LLMUnavailableError, match="server error 500"): + _pull_llm().delete_model("qwen2.5:3b") + + assert response.closed + + def test_delete_model_maps_unexpected_response(self, monkeypatch): + response = _FakeDeleteResponse(400, {"error": "bad request"}) + monkeypatch.setattr("app.services.llm.ollama.requests.delete", lambda *a, **k: response) + + with pytest.raises(LLMResponseError, match="refused to delete"): + _pull_llm().delete_model("qwen2.5:3b") + + assert response.closed + def test_error_after_success_suppresses_success_and_callback(self, monkeypatch): """An error anywhere in the stream prevents success, even after Ollama's success line.""" import json