diff --git a/app/api/routes/forms.py b/app/api/routes/forms.py index 805d18df..5d85cd14 100644 --- a/app/api/routes/forms.py +++ b/app/api/routes/forms.py @@ -1,20 +1,31 @@ +import json + import requests from fastapi import APIRouter, Depends, File, UploadFile, Query +from fastapi.responses import StreamingResponse from sqlmodel import Session from app.api.deps import get_db, verify_api_key from app.api.schemas.forms import ( FormFill, FormFillResponse, + ModelPullRequest, ModelsResponse, TranscriptionResponse, ) from app.core import paths -from app.core.config import OLLAMA_HOST, OLLAMA_MODEL, RETENTION_PERIOD_DAYS +from app.core.config import ( + OLLAMA_HOST, + OLLAMA_MODEL, + OLLAMA_PULL_TIMEOUT, + RECOMMENDED_MODELS, + RETENTION_PERIOD_DAYS, +) from app.services.whisper import call_whisper_asr from app.core.errors.base import AppError from app.db.repositories import get_template, get_form_submission, delete_form_submission from app.services.form import FormService +from app.services.form_filler.exceptions import ModelNotInstalledError router = APIRouter(prefix="/forms", tags=["forms"]) @@ -40,6 +51,12 @@ def fill_form(form: FormFill, db: Session = Depends(get_db)): input_text=submission.input_text, output_pdf_path=submission.output_pdf_path, ) + except ModelNotInstalledError as exc: + raise AppError( + f"{exc} Pull it first via POST /forms/pull.", + status_code=422, + error_code="MODEL_NOT_INSTALLED", + ) from exc except AppError: raise except Exception as e: @@ -49,24 +66,80 @@ def fill_form(form: FormFill, db: Session = Depends(get_db)): @router.get("/models", response_model=ModelsResponse) def list_models(): - """List the Whisper-independent extraction models available in the local - Ollama instance, plus the configured default. Used by the Fill Form UI's - model picker. Falls back to just the default if Ollama is unreachable.""" - default_model = OLLAMA_MODEL - - models: list[str] = [] + """List recommended and installed models for the Fill Form model picker.""" + installed_names: set[str] = set() try: response = requests.get(f"{OLLAMA_HOST}/api/tags", timeout=10) response.raise_for_status() - models = [m["name"] for m in response.json().get("models", []) if m.get("name")] - except requests.exceptions.RequestException: - models = [] - - # Always surface the configured default, even if Ollama hasn't pulled it yet. - if default_model not in models: - models.insert(0, default_model) + payload = response.json() + if not isinstance(payload, dict) or not isinstance(payload.get("models"), list): + raise ValueError("Unexpected Ollama tags response") + for model in payload["models"]: + if not isinstance(model, dict) or not isinstance(model.get("name"), str): + raise ValueError("Unexpected Ollama tags response") + installed_names.add(model["name"]) + except (requests.exceptions.RequestException, ValueError, TypeError): + pass + + all_names = sorted(set(RECOMMENDED_MODELS) | installed_names) + models = [ + { + "name": name, + "installed": name in installed_names, + "recommended": name in RECOMMENDED_MODELS, + } + for name in all_names + ] + return ModelsResponse(current_model=OLLAMA_MODEL, models=models) + + +@router.post( + "/pull", + response_class=StreamingResponse, + responses={200: {"content": {"application/x-ndjson": {}}}}, +) +def pull_model(req: ModelPullRequest): + """Stream pull progress from Ollama as newline-delimited JSON.""" + + def generate(): + saw_error = False + saw_success = False + buffer = "" + + def inspect_lines(chunk: bytes) -> None: + nonlocal buffer, saw_error, saw_success + buffer += chunk.decode("utf-8") + lines = buffer.split("\n") + buffer = lines.pop() or "" + for line in lines: + if not line.strip(): + continue + event = json.loads(line) + saw_error = saw_error or "error" in event + saw_success = saw_success or event.get("status") == "success" - return ModelsResponse(models=models, default=default_model) + try: + with requests.post( + f"{OLLAMA_HOST}/api/pull", + json={"name": req.model, "stream": True}, + stream=True, + timeout=(10, OLLAMA_PULL_TIMEOUT), + ) as response: + response.raise_for_status() + for chunk in response.iter_content(chunk_size=None): + if chunk: + inspect_lines(chunk) + yield chunk + if buffer.strip(): + event = json.loads(buffer) + saw_error = saw_error or "error" in event + saw_success = saw_success or event.get("status") == "success" + if not saw_error and not saw_success: + yield (json.dumps({"status": "success"}) + "\n").encode("utf-8") + except requests.exceptions.RequestException as exc: + yield (json.dumps({"error": str(exc)}) + "\n").encode("utf-8") + + return StreamingResponse(generate(), media_type="application/x-ndjson") @router.post("/transcribe", response_model=TranscriptionResponse) diff --git a/app/api/schemas/forms.py b/app/api/schemas/forms.py index 15d7e099..66113fa1 100644 --- a/app/api/schemas/forms.py +++ b/app/api/schemas/forms.py @@ -1,9 +1,25 @@ +import re from uuid import UUID from pydantic import BaseModel, ConfigDict, field_validator from app.api.schemas.enums import JobStatus, JobType +_MODEL_NAME_PATTERN = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._\-/:]*$") + + +def validate_model_name(value: str | None) -> str | None: + if value is None: + return None + value = value.strip() + if not value: + raise ValueError("model name cannot be empty") + if len(value) > 128: + raise ValueError("model name must be at most 128 characters") + if not _MODEL_NAME_PATTERN.fullmatch(value): + raise ValueError("model name contains invalid characters") + return value + class FormFill(BaseModel): template_id: int @@ -11,6 +27,11 @@ class FormFill(BaseModel): input_id: UUID | None = None model: str | None = None + @field_validator("model") + @classmethod + def _validate_model_name(cls, value: str | None) -> str | None: + return validate_model_name(value) + class FormFillResponse(BaseModel): id: int @@ -26,9 +47,24 @@ class TranscriptionResponse(BaseModel): text: str +class ModelInfo(BaseModel): + name: str + installed: bool + recommended: bool + + class ModelsResponse(BaseModel): - models: list[str] - default: str + current_model: str + models: list[ModelInfo] + + +class ModelPullRequest(BaseModel): + model: str + + @field_validator("model") + @classmethod + def _validate_model_name(cls, value: str) -> str: + return validate_model_name(value) or "" class AsyncFormFill(BaseModel): diff --git a/app/core/config.py b/app/core/config.py index 4ef0e4a3..c022cff6 100644 --- a/app/core/config.py +++ b/app/core/config.py @@ -38,7 +38,14 @@ OLLAMA_HOST = os.getenv("OLLAMA_HOST", "http://localhost:11434").rstrip("/") OLLAMA_MODEL = os.getenv("OLLAMA_MODEL", "qwen2.5:1.5b") OLLAMA_TIMEOUT = int(os.getenv("OLLAMA_TIMEOUT", "300")) +OLLAMA_PULL_TIMEOUT = int(os.getenv("OLLAMA_PULL_TIMEOUT", "120")) WHISPER_HOST = os.getenv("WHISPER_HOST", "http://localhost:9000").rstrip("/") +RECOMMENDED_MODELS: list[str] = [ + "qwen2.5:1.5b", + "qwen2.5:3b", + "qwen2.5:7b", + "llama3.2:3b", +] # --- Celery / Redis ------------------------------------------------------- CELERY_BROKER_URL = os.getenv("CELERY_BROKER_URL", "redis://localhost:6379/0") diff --git a/app/services/form_filler/exceptions.py b/app/services/form_filler/exceptions.py new file mode 100644 index 00000000..63e347ec --- /dev/null +++ b/app/services/form_filler/exceptions.py @@ -0,0 +1,4 @@ +class ModelNotInstalledError(Exception): + def __init__(self, model: str): + self.model = model + super().__init__(f"Model '{model}' is not installed in Ollama.") diff --git a/app/services/form_filler/filler.py b/app/services/form_filler/filler.py index 0fc0205e..33f4d3d4 100644 --- a/app/services/form_filler/filler.py +++ b/app/services/form_filler/filler.py @@ -19,6 +19,7 @@ from app.core.config import OLLAMA_HOST, OLLAMA_MODEL, OLLAMA_TIMEOUT from . import template +from .exceptions import ModelNotInstalledError from .geometry import CHECKBOX, RADIOBUTTON, SIGNATURE, drop_null_parents, on_state, page_widgets OPTIONS = { @@ -97,12 +98,17 @@ def call_model(prompt, schema, model): "think": False, "options": OPTIONS, } - response = requests.post( - OLLAMA_HOST + "/api/generate", - json=payload, - timeout=OLLAMA_TIMEOUT, - ) - response.raise_for_status() + try: + response = requests.post( + OLLAMA_HOST + "/api/generate", + json=payload, + timeout=OLLAMA_TIMEOUT, + ) + response.raise_for_status() + except requests.exceptions.HTTPError as exc: + if exc.response is not None and exc.response.status_code == 404: + raise ModelNotInstalledError(model) from exc + raise return response.json().get("response", "") diff --git a/app/tasks/fill.py b/app/tasks/fill.py index 557c0fde..7fd2b2bd 100644 --- a/app/tasks/fill.py +++ b/app/tasks/fill.py @@ -8,6 +8,7 @@ from app.db.database import get_session from app.db.repositories import get_job_by_celery_id, get_template, update_job from app.services.form import FormService +from app.services.form_filler.exceptions import ModelNotInstalledError logger = logging.getLogger(__name__) @@ -43,6 +44,15 @@ def fill_form_task(self, template_id: int, input_text: str, input_id_str: str, m return {"job_id": job.job_id, "result_url": job.result_url} + except ModelNotInstalledError as e: + logger.exception("fill_form_task failed") + job = get_job_by_celery_id(session, self.request.id) + if job: + job.status = JobStatus.failed + job.error = {"error_code": "MODEL_NOT_INSTALLED", "message": str(e)} + job.updated_at = datetime.now(timezone.utc) + update_job(session, job) + raise except Exception as e: logger.exception("fill_form_task failed") job = get_job_by_celery_id(session, self.request.id) diff --git a/contracts/openapi.yaml b/contracts/openapi.yaml index 6ae8a3c6..62082e45 100644 --- a/contracts/openapi.yaml +++ b/contracts/openapi.yaml @@ -42,6 +42,10 @@ paths: # ── Layer 3: Form Fill ────────────────────────────────────────── /api/v1/forms/fill: $ref: "path/forms.yaml#/fill" + /api/v1/forms/models: + $ref: "path/forms.yaml#/models" + /api/v1/forms/pull: + $ref: "path/forms.yaml#/pull" /api/v1/forms/jobs: $ref: "path/forms.yaml#/async_fill" diff --git a/contracts/path/forms.yaml b/contracts/path/forms.yaml index 4e0fb604..deee61fa 100644 --- a/contracts/path/forms.yaml +++ b/contracts/path/forms.yaml @@ -1,5 +1,7 @@ # Layer 3 Form Fill Endpoints # POST /api/v1/forms/fill +# GET /api/v1/forms/models +# POST /api/v1/forms/pull # POST /api/v1/forms/jobs fill: @@ -56,6 +58,15 @@ fill: message: "Input 550e8400-e29b-41d4-a716-446655440001 is not ready (status: transcribing)" detail: status: "transcribing" + "422": + description: The requested model is not installed in Ollama + content: + application/json: + schema: + $ref: "../schemas/common.yaml#/ErrorResponse" + example: + error_code: "MODEL_NOT_INSTALLED" + message: "Model 'qwen2.5:7b' is not installed in Ollama. Pull it first via POST /forms/pull." "500": description: Form fill failed (LLM or PDF generation error) content: @@ -66,6 +77,82 @@ fill: error_code: "FORM_FILL_ERROR" message: "Ollama did not respond within 120 seconds" +models: + get: + operationId: listModels + summary: List available AI models + description: | + Returns the configured current model together with recommended models and + all models currently installed in Ollama. Models that are not installed + are included so clients can offer them for download. + tags: + - forms + responses: + "200": + description: Current, recommended, and installed AI models + content: + application/json: + schema: + $ref: "../schemas/form-record.yaml#/ModelsResponse" + example: + current_model: "qwen2.5:1.5b" + models: + - name: "qwen2.5:1.5b" + installed: true + recommended: true + - name: "llama3.2:3b" + installed: false + recommended: true + +pull: + post: + operationId: pullModel + summary: Pull an AI model from Ollama + description: | + Starts downloading the requested Ollama model and streams progress as + newline-delimited JSON. The final line has status "success" when the + download completes. Pull failures are returned in the stream as an + object with an "error" field because the HTTP status is already committed. + tags: + - forms + requestBody: + required: true + content: + application/json: + schema: + $ref: "../schemas/form-record.yaml#/ModelPullRequest" + example: + model: "llama3.2:3b" + responses: + "200": + description: Streaming pull progress or an error object + content: + application/x-ndjson: + schema: + type: object + additionalProperties: true + examples: + progress: + summary: Pull progress event + value: + status: "pulling manifest" + completed: 1048576 + total: 4194304 + success: + summary: Pull completed + value: + status: "success" + error: + summary: Pull failed + value: + error: "Ollama connection refused" + "422": + description: Invalid pull request + content: + application/json: + schema: + $ref: "../schemas/common.yaml#/ErrorResponse" + async_fill: post: operationId: submitAsyncFormFill diff --git a/contracts/path/system.yaml b/contracts/path/system.yaml index cd00901c..0a3ded71 100644 --- a/contracts/path/system.yaml +++ b/contracts/path/system.yaml @@ -42,9 +42,9 @@ health: size_gb: 4.7 quantization: "Q4_K_M" loaded: true - - name: "mistral:7b" - size_gb: 4.1 - quantization: "Q4_0" + - name: "qwen2.5:3b" + size_gb: 2.0 + quantization: "Q4_K_M" loaded: false current_load: active_requests: 1 diff --git a/contracts/schemas/form-record.yaml b/contracts/schemas/form-record.yaml index 563117ef..8f82ee29 100644 --- a/contracts/schemas/form-record.yaml +++ b/contracts/schemas/form-record.yaml @@ -4,14 +4,18 @@ FormFillRequest: type: object required: - template_id - - input_id properties: template_id: type: integer description: ID of the template to fill + input_text: + type: string + nullable: true + description: Direct narrative text to use when input_id is not provided input_id: type: string format: uuid + nullable: true description: ID of a stored input (see POST /input/text, POST /input/voice) whose transcript is used to fill this template. The input must have status "ready". @@ -20,6 +24,47 @@ FormFillRequest: nullable: true description: Ollama model override. Defaults to the configured default model. +ModelInfo: + type: object + required: + - name + - installed + - recommended + properties: + name: + type: string + description: Ollama model name + installed: + type: boolean + description: Whether the model is installed in Ollama + recommended: + type: boolean + description: Whether FireForm recommends the model + +ModelsResponse: + type: object + required: + - current_model + - models + properties: + current_model: + type: string + description: Model used when no model override is supplied + models: + type: array + items: + $ref: "#/ModelInfo" + description: Recommended and installed Ollama models + +ModelPullRequest: + type: object + required: + - model + properties: + model: + type: string + description: Ollama model name to download + FormFillResponse: type: object required: diff --git a/docker/.env.example b/docker/.env.example index aa6f5d72..7cf16b72 100644 --- a/docker/.env.example +++ b/docker/.env.example @@ -21,6 +21,9 @@ DATABASE_URL=postgresql+psycopg2://fireform:fireform@localhost:5432/fireform OLLAMA_HOST=http://ollama:11434 OLLAMA_MODEL=qwen2.5:1.5b OLLAMA_TIMEOUT=300 +# Seconds without progress from Ollama before giving up on a model download. +# This is not a limit on total download time. Connecting to Ollama is limited to 10 seconds. +OLLAMA_PULL_TIMEOUT=120 # --- Whisper -------------------------------------------------------------- # Whisper runs in Docker with port mapped to host. App runs on host. diff --git a/docs/MODEL_SELECTION.md b/docs/MODEL_SELECTION.md new file mode 100644 index 00000000..79c1de24 --- /dev/null +++ b/docs/MODEL_SELECTION.md @@ -0,0 +1,82 @@ +# Model Selection + +FireForm uses Ollama for local form filling. This page documents the API for +discovering, selecting, and downloading models. + +## Model discovery + +### `GET /api/v1/forms/models` + +Returns the configured default model and the available models: + +| Field | Meaning | +|---|---| +| `current_model` | Model used when no override is supplied | +| `name` | Ollama model name | +| `installed` | Whether the model is installed locally | +| `recommended` | Whether FireForm recommends the model | + +Recommended models are included even when they are not installed. Additional +installed Ollama models are included as well. If Ollama is unavailable or +returns malformed data, recommended models are returned with +`installed: false`. + +## Selecting a model + +`POST /api/v1/forms/fill` accepts an optional `model` override: + +```json +{ + "template_id": 4, + "input_text": "A wildfire occurred near the main station.", + "model": "qwen2.5:1.5b" +} +``` + +When `model` is omitted, the configured `OLLAMA_MODEL` is used. Model names +must be 128 characters or fewer and match: + +```text +^[A-Za-z0-9][A-Za-z0-9._\-/:]*$ +``` + +If the selected model is not installed, the API returns HTTP `422` with the +error code `MODEL_NOT_INSTALLED`. + +## Downloading a model + +### `POST /api/v1/forms/pull` + +Downloads a model and streams progress as newline-delimited JSON (NDJSON): + +```json +{ "model": "qwen2.5:3b" } +``` + +Example successful stream: + +```text +{"status":"pulling manifest"} +{"status":"downloading","completed":1048576,"total":1929912432} +{"status":"success"} +``` + +The final event is `{"status":"success"}` only after a successful download. +If a download fails after streaming starts, clients must inspect the stream +for an `error` event: + +```text +{"status":"pulling manifest"} +{"error":"Ollama connection refused"} +``` + +## Configuration + +| Variable | Default | Purpose | +|---|---|---| +| `OLLAMA_HOST` | `http://localhost:11434` | Ollama server URL | +| `OLLAMA_MODEL` | See `.env.example` | Default model | +| `OLLAMA_TIMEOUT` | `300` | Seconds allowed for inference | +| `OLLAMA_PULL_TIMEOUT` | `120` | Seconds to wait without receiving progress from Ollama before giving up on a model download; not a limit on total download time. Connecting to Ollama is limited to 10 seconds. | + +Docker values are documented in `docker/.env.example`. diff --git a/docs/README.md b/docs/README.md index 7cf785fa..71a5eea3 100644 --- a/docs/README.md +++ b/docs/README.md @@ -12,6 +12,7 @@ It is an open-source, agnostic system built to solve administrative overhead for - [DATABASE](DATABASE.md): Information on database setup and management. - [DOCKER](DOCKER.md): Detailed guide on using Docker and Docker Compose. - [CONTRIBUTING](CONTRIBUTING.md): How you can help improve FireForm. +- [MODEL SELECTION](MODEL_SELECTION.md): Discover, select, and download Ollama models. - [SETUP](1.%20SETUP.md): Local development setup guide. - [PROJECT STRUCTURE](2.%20PROJECT_STRUCTURE.md): How the codebase is organized. diff --git a/scripts/select-model.sh b/scripts/select-model.sh index 4b3c9d36..dbcc8bdb 100755 --- a/scripts/select-model.sh +++ b/scripts/select-model.sh @@ -5,7 +5,7 @@ ENV_DEV="docker/.env.dev" COMPOSE="docker compose -f docker/dev/compose.yml --env-file $ENV_DEV" # model name | approx size -MODELS="qwen2.5:1.5b|~1GB qwen2.5:3b|~2GB qwen2.5:7b|~4GB llama3.2:3b|~2GB mistral:7b|~4GB" +MODELS="qwen2.5:1.5b|~1GB qwen2.5:3b|~2GB qwen2.5:7b|~4GB llama3.2:3b|~2GB" current_model="" if [ -f "$ENV_DEV" ]; then diff --git a/tests/test_api.py b/tests/test_api.py index 4621daff..967ce013 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -4,6 +4,7 @@ All heavy dependencies (LLM, commonforms, filesystem) are mocked via conftest. """ +import json from uuid import uuid4 from sqlmodel import select @@ -386,35 +387,183 @@ def fake_post(url, params=None, files=None, timeout=None): assert captured["params"]["output"] == "json" def test_list_models(self, client, monkeypatch): - ""f"{API_PREFIX}/forms/models lists Ollama models and always includes the default.""" + """The model list includes recommended and installed Ollama models.""" from unittest.mock import MagicMock fake_response = MagicMock() fake_response.json.return_value = {"models": [{"name": "qwen2.5:3b"}, {"name": "mistral:latest"}]} fake_response.raise_for_status.return_value = None monkeypatch.setattr("app.api.routes.forms.requests.get", lambda *a, **k: fake_response) - monkeypatch.setenv("OLLAMA_MODEL", "qwen2.5:1.5b") + monkeypatch.setattr("app.api.routes.forms.OLLAMA_MODEL", "qwen2.5:1.5b") resp = client.get(f"{API_PREFIX}/forms/models") assert resp.status_code == 200 body = resp.json() - assert body["default"] == "qwen2.5:1.5b" - assert "qwen2.5:1.5b" in body["models"] # default injected even if not pulled - assert "qwen2.5:3b" in body["models"] + assert body["current_model"] == "qwen2.5:1.5b" + models = {model["name"]: model for model in body["models"]} + assert models["qwen2.5:1.5b"] == { + "name": "qwen2.5:1.5b", + "installed": False, + "recommended": True, + } + assert models["qwen2.5:3b"]["installed"] is True + assert models["qwen2.5:3b"]["recommended"] is True + assert models["mistral:latest"]["installed"] is True + assert models["mistral:latest"]["recommended"] is False def test_list_models_ollama_down(self, client, monkeypatch): - """If Ollama is unreachable, still return the default alone.""" + """If Ollama is unreachable, still return all recommended models.""" import requests def boom(*a, **k): raise requests.exceptions.ConnectionError("down") monkeypatch.setattr("app.api.routes.forms.requests.get", boom) - monkeypatch.setenv("OLLAMA_MODEL", "qwen2.5:1.5b") resp = client.get(f"{API_PREFIX}/forms/models") assert resp.status_code == 200 - assert resp.json()["models"] == ["qwen2.5:1.5b"] + body = resp.json() + assert body["current_model"] == "qwen2.5:1.5b" + assert [model["name"] for model in body["models"]] == [ + "llama3.2:3b", + "qwen2.5:1.5b", + "qwen2.5:3b", + "qwen2.5:7b", + ] + assert all(model["installed"] is False for model in body["models"]) + assert all(model["recommended"] is True for model in body["models"]) + + def test_list_models_malformed_ollama_response(self, client, monkeypatch): + from unittest.mock import MagicMock + + fake_response = MagicMock() + fake_response.raise_for_status.return_value = None + fake_response.json.return_value = {"models": {"name": "broken"}} + monkeypatch.setattr("app.api.routes.forms.requests.get", lambda *a, **k: fake_response) + + resp = client.get(f"{API_PREFIX}/forms/models") + + assert resp.status_code == 200 + body = resp.json() + assert all(model["installed"] is False for model in body["models"]) + assert all(model["recommended"] is True for model in body["models"]) + + def test_list_models_invalid_json(self, client, monkeypatch): + import json + from unittest.mock import MagicMock + + fake_response = MagicMock() + fake_response.raise_for_status.return_value = None + fake_response.json.side_effect = json.JSONDecodeError("invalid", "", 0) + monkeypatch.setattr("app.api.routes.forms.requests.get", lambda *a, **k: fake_response) + + resp = client.get(f"{API_PREFIX}/forms/models") + + assert resp.status_code == 200 + body = resp.json() + assert all(model["installed"] is False for model in body["models"]) + assert all(model["recommended"] is True for model in body["models"]) + + def test_pull_model_success(self, client, monkeypatch): + from unittest.mock import MagicMock + + fake_response = MagicMock() + fake_response.iter_content.return_value = [ + b'{"status":"pulling","completed":1,"total":2}\n', + b'{"status":"verifying"}\n', + ] + fake_response.raise_for_status.return_value = None + captured = {} + + def fake_post(*args, **kwargs): + captured["args"] = args + captured["kwargs"] = kwargs + return fake_response + + monkeypatch.setattr("app.api.routes.forms.requests.post", fake_post) + + resp = client.post(f"{API_PREFIX}/forms/pull", json={"model": "llama3.2:3b"}) + + assert resp.status_code == 200 + lines = [json.loads(line) for line in resp.text.splitlines() if line.strip()] + assert lines[-1] == {"status": "success"} + assert captured["kwargs"]["json"] == {"name": "llama3.2:3b", "stream": True} + assert captured["kwargs"]["stream"] is True + assert captured["kwargs"]["timeout"] == (10, 120) + + def test_pull_model_read_timeout_does_not_append_success(self, client, monkeypatch): + from unittest.mock import MagicMock + import requests + + fake_response = MagicMock() + fake_response.raise_for_status.return_value = None + + def iter_content(*args, **kwargs): + yield b'{"status":"pulling"}\n' + raise requests.exceptions.ReadTimeout("read timed out") + + fake_response.iter_content.side_effect = iter_content + fake_response.__enter__.return_value = fake_response + fake_response.__exit__.return_value = None + monkeypatch.setattr("app.api.routes.forms.requests.post", lambda *a, **k: fake_response) + + resp = client.post(f"{API_PREFIX}/forms/pull", json={"model": "llama3.2:3b"}) + + assert resp.status_code == 200 + lines = [json.loads(line) for line in resp.text.splitlines() if line.strip()] + assert lines[0] == {"status": "pulling"} + assert "error" in lines[-1] + assert {"status": "success"} not in lines + + def test_pull_model_failure(self, client, monkeypatch): + """When Ollama is unreachable, the stream returns an error line.""" + import requests + + def boom(*args, **kwargs): + raise requests.exceptions.RequestException("pull failed") + + monkeypatch.setattr("app.api.routes.forms.requests.post", boom) + + resp = client.post(f"{API_PREFIX}/forms/pull", json={"model": "llama3.2:3b"}) + + assert resp.status_code == 200 + lines = [line for line in resp.text.splitlines() if line.strip()] + assert lines + assert "error" in json.loads(lines[-1]) + + def test_pull_model_mid_stream_error_does_not_append_success(self, client, monkeypatch): + from unittest.mock import MagicMock + + fake_response = MagicMock() + fake_response.iter_content.return_value = [ + b'{"status":"pulling"}\n', + b'{"error":"pull failed"}\n', + ] + fake_response.raise_for_status.return_value = None + fake_response.__enter__.return_value = fake_response + fake_response.__exit__.return_value = None + monkeypatch.setattr("app.api.routes.forms.requests.post", lambda *a, **k: fake_response) + + resp = client.post(f"{API_PREFIX}/forms/pull", json={"model": "llama3.2:3b"}) + + lines = [json.loads(line) for line in resp.text.splitlines() if line.strip()] + assert lines[-1] == {"error": "pull failed"} + assert {"status": "success"} not in lines + + def test_pull_model_rejects_invalid_model_name(self, client): + resp = client.post(f"{API_PREFIX}/forms/pull", json={"model": "bad model"}) + + assert resp.status_code == 422 + assert resp.json()["error_code"] == "VALIDATION_ERROR" + + def test_fill_form_rejects_invalid_model_name(self, client): + resp = client.post( + f"{API_PREFIX}/forms/fill", + json={"template_id": 1, "input_text": "text", "model": " "}, + ) + + assert resp.status_code == 422 + assert resp.json()["error_code"] == "VALIDATION_ERROR" def test_fill_form_passes_model_override(self, client, mock_controller, db, tmp_path, monkeypatch): """A `model` in the request reaches filler.fill as a keyword argument.""" @@ -433,6 +582,51 @@ def test_fill_form_passes_model_override(self, client, mock_controller, db, tmp_ call_args = mock_controller["mock_fill"].call_args assert call_args.kwargs.get("model") == "qwen2.5:3b" or call_args.args[3] == "qwen2.5:3b" + def test_fill_form_without_model_preserves_default_path( + self, client, mock_controller, db, tmp_path, monkeypatch + ): + monkeypatch.setattr("app.core.paths.PROJECT_ROOT", tmp_path) + pdf_file = tmp_path / "employee.pdf" + pdf_file.write_bytes(b"%PDF-1.4 fake") + tpl_id = self._seed_template_at(client, str(pdf_file.relative_to(tmp_path))) + input_id = self._seed_input(db, transcript="John Doe") + + resp = client.post(f"{API_PREFIX}/forms/fill", json={ + "template_id": tpl_id, + "input_id": str(input_id), + }) + + assert resp.status_code == 200 + call_args = mock_controller["mock_fill"].call_args + assert call_args.kwargs.get("model") is None + + def test_fill_form_returns_422_for_uninstalled_model( + self, client, mock_controller, db, tmp_path, monkeypatch + ): + from app.services.form_filler.exceptions import ModelNotInstalledError + + monkeypatch.setattr("app.core.paths.PROJECT_ROOT", tmp_path) + pdf_file = tmp_path / "employee.pdf" + pdf_file.write_bytes(b"%PDF-1.4 fake") + tpl_id = self._seed_template_at(client, str(pdf_file.relative_to(tmp_path))) + input_id = self._seed_input(db, transcript="John Doe") + mock_controller["mock_fill"].side_effect = ModelNotInstalledError("missing-model") + + resp = client.post(f"{API_PREFIX}/forms/fill", json={ + "template_id": tpl_id, + "input_id": str(input_id), + "model": "missing-model", + }) + + assert resp.status_code == 422 + assert resp.json() == { + "error_code": "MODEL_NOT_INSTALLED", + "message": ( + "Model 'missing-model' is not installed in Ollama. " + "Pull it first via POST /forms/pull." + ), + } + def test_transcribe_service_unavailable(self, client, monkeypatch): """A down whisper service surfaces as a 503, not a 500.""" import io diff --git a/tests/test_form_filler.py b/tests/test_form_filler.py index 177cc754..41e9c086 100644 --- a/tests/test_form_filler.py +++ b/tests/test_form_filler.py @@ -8,9 +8,12 @@ from pathlib import Path from unittest.mock import patch +import pytest from pypdf import PdfReader +from requests import HTTPError from app.services.form_filler import filler, template +from app.services.form_filler.exceptions import ModelNotInstalledError ICS_205A = str(Path(__file__).resolve().parents[1] / "benchmark/data/pdfs/ics_205a.pdf") TABLE = "resource_summary_0_table" @@ -79,3 +82,14 @@ def test_fill_defaults_to_configured_model(): assert call.call_args.args[2] == filler.OLLAMA_MODEL + +def test_call_model_raises_for_uninstalled_model(): + with patch.object(filler.requests, "post") as post: + response = post.return_value + response.raise_for_status.side_effect = HTTPError( + response=response, + ) + response.status_code = 404 + + with pytest.raises(ModelNotInstalledError, match="test-model"): + filler.call_model("prompt", {"type": "object"}, "test-model") diff --git a/tests/test_jobs.py b/tests/test_jobs.py index 65075d5c..986781bc 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -1,13 +1,17 @@ """Tests for async job submission and status endpoints.""" -from unittest.mock import ANY, patch +from types import SimpleNamespace +from unittest.mock import ANY, MagicMock, patch from uuid import uuid4 +import pytest from sqlmodel import select from app.api.schemas.enums import InputStatus, InputType, JobStatus from app.core.config import API_PREFIX from app.models import Input, Job +from app.services.form_filler.exceptions import ModelNotInstalledError +from app.tasks.fill import fill_form_task class TestJobEndpoints: @@ -198,3 +202,24 @@ def test_submit_async_queue_unavailable(self, mock_task, client, db): job = db.exec(select(Job)).one() assert job.status == JobStatus.failed assert job.error["error_code"] == "QUEUE_UNAVAILABLE" + + def test_fill_task_preserves_model_not_installed_error_code(self): + session = MagicMock() + job = SimpleNamespace(status=JobStatus.queued, progress_percent=0, error=None) + + with ( + patch("app.tasks.fill.get_session", return_value=iter([session])), + patch("app.tasks.fill.get_job_by_celery_id", side_effect=[job, job]), + patch("app.tasks.fill.get_template", return_value=object()), + patch( + "app.tasks.fill.FormService.fill_and_persist", + side_effect=ModelNotInstalledError("missing-model"), + ), + ): + with pytest.raises(ModelNotInstalledError, match="missing-model"): + fill_form_task.run(1, "text", str(uuid4()), "missing-model") + + assert job.error == { + "error_code": "MODEL_NOT_INSTALLED", + "message": "Model 'missing-model' is not installed in Ollama.", + } diff --git a/tests/test_v1_system.py b/tests/test_v1_system.py index d8851213..9dbb1073 100644 --- a/tests/test_v1_system.py +++ b/tests/test_v1_system.py @@ -67,7 +67,7 @@ def _make_mock_response(json_data=None, status_code=200): _TAGS_RESPONSE = { "models": [ {"name": "llama3:8b", "size": 5_000_000_000, "details": {"quantization_level": "Q4_K_M"}}, - {"name": "mistral:7b", "size": 4_000_000_000, "details": {"quantization_level": "Q4_0"}}, + {"name": "qwen2.5:3b", "size": 2_000_000_000, "details": {"quantization_level": "Q4_K_M"}}, ] } _VERSION_RESPONSE = {"version": "0.3.0"} @@ -228,9 +228,9 @@ def test_models_available_and_loaded_flag(self, client, monkeypatch): models = resp.json()["components"]["ollama"]["models_available"] assert len(models) == 2 llama = next(m for m in models if m["name"] == "llama3:8b") - mistral = next(m for m in models if m["name"] == "mistral:7b") + qwen = next(m for m in models if m["name"] == "qwen2.5:3b") assert llama["loaded"] is True - assert mistral["loaded"] is False + assert qwen["loaded"] is False assert llama["quantization"] == "Q4_K_M" assert llama["size_gb"] == pytest.approx(4.66, abs=0.1)