From ce29e8e48c5a7bdc34b59a2b13d5074d31c46df8 Mon Sep 17 00:00:00 2001
From: utkarshqz <utkarshsonawane25@gmail.com>
Date: Mon, 9 Mar 2026 10:32:14 +0530
Subject: [PATCH] test: replace broken test suite with 52 passing tests

---
 docs/TESTING.md         |  57 ++++++++
 tests/conftest.py       |  15 ++-
 tests/test_forms.py     | 132 +++++++++++++++----
 tests/test_llm.py       | 278 ++++++++++++++++++++++++++++++++++++++++
 tests/test_templates.py | 144 ++++++++++++++++++---
 5 files changed, 578 insertions(+), 48 deletions(-)
 create mode 100644 docs/TESTING.md
 create mode 100644 tests/test_llm.py

diff --git a/docs/TESTING.md b/docs/TESTING.md
new file mode 100644
index 0000000..bf3edcd
--- /dev/null
+++ b/docs/TESTING.md
@@ -0,0 +1,57 @@
+# 🧪 Testing
+
+This document describes how to run the FireForm test suite locally.
+
+## Prerequisites
+
+Make sure you have installed all dependencies:
+
+```bash
+pip install -r requirements.txt
+```
+
+## Running Tests
+
+From the project root directory:
+
+```bash
+python -m pytest tests/ -v
+```
+
+> **Note:** Use `python -m pytest` instead of `pytest` directly to ensure the project root is on the Python path.
+
+## Test Coverage
+
+| File | Tests | What it covers |
+|------|-------|----------------|
+| `tests/test_llm.py` | 15 | LLM class — batch prompt, field extraction, plural handling |
+| `tests/test_templates.py` | 10 | `POST /templates/create`, `GET /templates`, `GET /templates/{id}` |
+| `tests/test_forms.py` | 7 | `POST /forms/fill`, `GET /forms/{id}`, `GET /forms/download/{id}` |
+
+**Total: 52 tests**
+
+## Test Design
+
+- All tests use an **in-memory SQLite database** — your local `fireform.db` is never touched
+- Each test gets a **fresh empty database** — no data leaks between tests
+- Ollama is **never called** during tests — all LLM calls are mocked
+
+## Key Test Cases
+
+**LLM extraction (`test_llm.py`)**
+- Batch prompt contains all field keys and human-readable labels
+- `main_loop()` makes exactly **1 Ollama call** regardless of field count (O(1) assertion)
+- Graceful fallback when Mistral returns invalid JSON
+- `-1` responses stored as `None`, not as the string `"-1"`
+
+**Template endpoints (`test_templates.py`)**
+- Valid PDF upload returns 200 with field data
+- Non-PDF upload returns 400
+- Missing file returns 422
+- Non-existent template returns 404
+
+**Form endpoints (`test_forms.py`)**
+- Non-existent template returns 404
+- Ollama connection failure returns 503
+- Missing filled PDF on disk returns 404
+- Non-existent submission returns 404
diff --git a/tests/conftest.py b/tests/conftest.py
index 7cb4db3..ff92c19 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -3,12 +3,10 @@
 from sqlalchemy.pool import StaticPool
 import pytest
 
-
 from api.main import app
 from api.deps import get_db
 from api.db.models import Template, FormSubmission
 
-# In-memory SQLite database for tests
 TEST_DATABASE_URL = "sqlite://"
 
 engine = create_engine(
@@ -23,12 +21,12 @@ def override_get_db():
         yield session
 
 
-# Apply dependency override
 app.dependency_overrides[get_db] = override_get_db
 
 
-@pytest.fixture(scope="session", autouse=True)
-def create_test_db():
+@pytest.fixture(autouse=True)
+def reset_db():
+    SQLModel.metadata.drop_all(engine)
     SQLModel.metadata.create_all(engine)
     yield
     SQLModel.metadata.drop_all(engine)
@@ -37,3 +35,10 @@ def create_test_db():
 @pytest.fixture
 def client():
     return TestClient(app)
+
+
+@pytest.fixture
+def db_session():
+    """Direct DB session for test setup."""
+    with Session(engine) as session:
+        yield session
diff --git a/tests/test_forms.py b/tests/test_forms.py
index 8f432bf..5e32755 100644
--- a/tests/test_forms.py
+++ b/tests/test_forms.py
@@ -1,25 +1,107 @@
-def test_submit_form(client):
-    pass
-    # First create a template
-    # form_payload = {
-    #     "template_id": 3,
-    #     "input_text": "Hi. The employee's name is John Doe. His job title is managing director. His department supervisor is Jane Doe. His phone number is 123456. His email is jdoe@ucsc.edu. The signature is <Mamañema>, and the date is 01/02/2005",
-    # }
-
-    # template_res = client.post("/templates/", json=template_payload)
-    # template_id = template_res.json()["id"]
-
-    # # Submit a form
-    # form_payload = {
-    #     "template_id": template_id,
-    #     "data": {"rating": 5, "comment": "Great service"},
-    # }
-
-    # response = client.post("/forms/", json=form_payload)
-
-    # assert response.status_code == 200
-
-    # data = response.json()
-    # assert data["id"] is not None
-    # assert data["template_id"] == template_id
-    # assert data["data"] == form_payload["data"]
+"""
+Tests for /forms endpoints.
+Closes #165, #205, #163
+"""
+
+import pytest
+from unittest.mock import patch
+from api.db.models import Template, FormSubmission
+from datetime import datetime
+
+
+# ── helpers ───────────────────────────────────────────────────────────────────
+
+def make_template(db_session):
+    t = Template(
+        name="Test Form",
+        fields={"JobTitle": "Job Title"},
+        pdf_path="/tmp/test.pdf",
+        created_at=datetime.utcnow(),
+    )
+    db_session.add(t)
+    db_session.commit()
+    db_session.refresh(t)
+    return t.id
+
+
+def make_submission(db_session, template_id, output_path="/tmp/filled.pdf"):
+    s = FormSubmission(
+        template_id=template_id,
+        input_text="John Smith is a firefighter.",
+        output_pdf_path=output_path,
+        created_at=datetime.utcnow(),
+    )
+    db_session.add(s)
+    db_session.commit()
+    db_session.refresh(s)
+    return s.id
+
+
+# ── POST /forms/fill ──────────────────────────────────────────────────────────
+
+class TestFillForm:
+
+    def test_fill_form_template_not_found(self, client):
+        """Returns 404 when template_id does not exist."""
+        response = client.post("/forms/fill", json={
+            "template_id": 999999,
+            "input_text": "John Smith is a firefighter.",
+        })
+        assert response.status_code == 404
+
+    def test_fill_form_missing_fields_returns_422(self, client):
+        """Returns 422 when required fields are missing."""
+        response = client.post("/forms/fill", json={
+            "template_id": 1,
+        })
+        assert response.status_code == 422
+
+    def test_fill_form_ollama_down_returns_503(self, client, db_session):
+        """Returns 503 when Ollama is not reachable."""
+        template_id = make_template(db_session)
+
+        with patch("src.controller.Controller.fill_form",
+                   side_effect=ConnectionError("Ollama not running")):
+            response = client.post("/forms/fill", json={
+                "template_id": template_id,
+                "input_text": "John Smith is a firefighter.",
+            })
+
+        assert response.status_code == 503
+
+
+# ── GET /forms/{submission_id} ────────────────────────────────────────────────
+
+class TestGetSubmission:
+
+    def test_get_submission_not_found(self, client):
+        """Returns 404 for non-existent submission ID."""
+        response = client.get("/forms/999999")
+        assert response.status_code == 404
+
+    def test_get_submission_invalid_id(self, client):
+        """Returns 422 for non-integer submission ID."""
+        response = client.get("/forms/not-an-id")
+        assert response.status_code == 422
+
+
+# ── GET /forms/download/{submission_id} ───────────────────────────────────────
+
+class TestDownloadSubmission:
+
+    def test_download_not_found_submission(self, client):
+        """Returns 404 when submission does not exist."""
+        response = client.get("/forms/download/999999")
+        assert response.status_code == 404
+
+    def test_download_file_missing_on_disk(self, client, db_session):
+        """Returns 404 when submission exists but PDF missing on disk."""
+        template_id = make_template(db_session)
+        submission_id = make_submission(
+            db_session, template_id, "/nonexistent/filled.pdf"
+        )
+
+        with patch("os.path.exists", return_value=False):
+            response = client.get(f"/forms/download/{submission_id}")
+
+        assert response.status_code == 404
diff --git a/tests/test_llm.py b/tests/test_llm.py
new file mode 100644
index 0000000..cfe483b
--- /dev/null
+++ b/tests/test_llm.py
@@ -0,0 +1,278 @@
+"""
+Unit tests for src/llm.py — LLM class.
+
+Closes: #186 (Unit tests for LLM class methods)
+Covers: batch prompt, per-field prompt, add_response_to_json,
+        handle_plural_values, type_check_all, main_loop (mocked)
+"""
+
+import json
+import pytest
+from unittest.mock import patch, MagicMock
+from src.llm import LLM
+
+
+# ── Fixtures ─────────────────────────────────────────────────────────────────
+
+@pytest.fixture
+def dict_fields():
+    """Realistic dict fields: {internal_name: human_label}"""
+    return {
+        "NAME/SID":       "Employee Or Student Name",
+        "JobTitle":       "Job Title",
+        "Department":     "Department",
+        "Phone Number":   "Phone Number",
+        "email":          "Email",
+    }
+
+@pytest.fixture
+def list_fields():
+    """Legacy list fields: [internal_name, ...]"""
+    return ["officer_name", "location", "incident_date"]
+
+@pytest.fixture
+def transcript():
+    return (
+        "Employee name is John Smith. Employee ID is EMP-2024-789. "
+        "Job title is Firefighter Paramedic. Department is Emergency Medical Services. "
+        "Phone number is 916-555-0147."
+    )
+
+@pytest.fixture
+def llm_dict(dict_fields, transcript):
+    return LLM(transcript_text=transcript, target_fields=dict_fields)
+
+@pytest.fixture
+def llm_list(list_fields, transcript):
+    return LLM(transcript_text=transcript, target_fields=list_fields)
+
+
+# ── type_check_all ────────────────────────────────────────────────────────────
+
+class TestTypeCheckAll:
+
+    def test_raises_on_non_string_transcript(self, dict_fields):
+        llm = LLM(transcript_text=12345, target_fields=dict_fields)
+        with pytest.raises(TypeError, match="Transcript must be text"):
+            llm.type_check_all()
+
+    def test_raises_on_none_transcript(self, dict_fields):
+        llm = LLM(transcript_text=None, target_fields=dict_fields)
+        with pytest.raises(TypeError):
+            llm.type_check_all()
+
+    def test_raises_on_invalid_fields_type(self, transcript):
+        llm = LLM(transcript_text=transcript, target_fields="not_a_list_or_dict")
+        with pytest.raises(TypeError, match="list or dict"):
+            llm.type_check_all()
+
+    def test_passes_with_dict_fields(self, llm_dict):
+        # Should not raise
+        llm_dict.type_check_all()
+
+    def test_passes_with_list_fields(self, llm_list):
+        # Should not raise
+        llm_list.type_check_all()
+
+
+# ── build_batch_prompt ────────────────────────────────────────────────────────
+
+class TestBuildBatchPrompt:
+
+    def test_contains_all_field_keys(self, llm_dict, dict_fields):
+        prompt = llm_dict.build_batch_prompt()
+        for key in dict_fields.keys():
+            assert key in prompt, f"Field key '{key}' missing from batch prompt"
+
+    def test_contains_human_labels(self, llm_dict, dict_fields):
+        prompt = llm_dict.build_batch_prompt()
+        for label in dict_fields.values():
+            assert label in prompt, f"Label '{label}' missing from batch prompt"
+
+    def test_contains_transcript(self, llm_dict, transcript):
+        prompt = llm_dict.build_batch_prompt()
+        assert transcript in prompt
+
+    def test_contains_json_instruction(self, llm_dict):
+        prompt = llm_dict.build_batch_prompt()
+        assert "JSON" in prompt
+
+    def test_list_fields_batch_prompt(self, llm_list, list_fields):
+        prompt = llm_list.build_batch_prompt()
+        for field in list_fields:
+            assert field in prompt
+
+    def test_labels_used_as_comments(self, llm_dict):
+        """Human labels should appear after // in the prompt"""
+        prompt = llm_dict.build_batch_prompt()
+        assert "//" in prompt
+
+
+# ── build_prompt (legacy per-field) ──────────────────────────────────────────
+
+class TestBuildPrompt:
+
+    def test_officer_field_gets_officer_guidance(self, llm_dict):
+        prompt = llm_dict.build_prompt("officer_name")
+        assert "OFFICER" in prompt.upper() or "EMPLOYEE" in prompt.upper()
+
+    def test_location_field_gets_location_guidance(self, llm_dict):
+        prompt = llm_dict.build_prompt("incident_location")
+        assert "LOCATION" in prompt.upper() or "ADDRESS" in prompt.upper()
+
+    def test_victim_field_gets_victim_guidance(self, llm_dict):
+        prompt = llm_dict.build_prompt("victim_name")
+        assert "VICTIM" in prompt.upper()
+
+    def test_phone_field_gets_phone_guidance(self, llm_dict):
+        prompt = llm_dict.build_prompt("Phone Number")
+        assert "PHONE" in prompt.upper()
+
+    def test_prompt_contains_transcript(self, llm_dict, transcript):
+        prompt = llm_dict.build_prompt("some_field")
+        assert transcript in prompt
+
+    def test_generic_field_still_builds_prompt(self, llm_dict):
+        prompt = llm_dict.build_prompt("textbox_0_0")
+        assert len(prompt) > 50
+
+
+# ── handle_plural_values ──────────────────────────────────────────────────────
+
+class TestHandlePluralValues:
+
+    def test_splits_on_semicolon(self, llm_dict):
+        result = llm_dict.handle_plural_values("Mark Smith;Jane Doe")
+        assert "Mark Smith" in result
+        assert "Jane Doe" in result
+
+    def test_strips_whitespace(self, llm_dict):
+        result = llm_dict.handle_plural_values("Mark Smith; Jane Doe; Bob")
+        assert all(v == v.strip() for v in result)
+
+    def test_returns_list(self, llm_dict):
+        result = llm_dict.handle_plural_values("A;B;C")
+        assert isinstance(result, list)
+
+    def test_raises_without_semicolon(self, llm_dict):
+        with pytest.raises(ValueError, match="separator"):
+            llm_dict.handle_plural_values("no semicolon here")
+
+    def test_three_values(self, llm_dict):
+        result = llm_dict.handle_plural_values("Alice;Bob;Charlie")
+        assert len(result) == 3
+
+
+# ── add_response_to_json ──────────────────────────────────────────────────────
+
+class TestAddResponseToJson:
+
+    def test_stores_value_under_field(self, llm_dict):
+        llm_dict.add_response_to_json("NAME/SID", "John Smith")
+        assert llm_dict._json["NAME/SID"] == "John Smith"
+
+    def test_ignores_minus_one(self, llm_dict):
+        llm_dict.add_response_to_json("email", "-1")
+        assert llm_dict._json["email"] is None
+
+    def test_strips_quotes(self, llm_dict):
+        llm_dict.add_response_to_json("JobTitle", '"Firefighter"')
+        assert llm_dict._json["JobTitle"] == "Firefighter"
+
+    def test_strips_whitespace(self, llm_dict):
+        llm_dict.add_response_to_json("Department", "  EMS  ")
+        assert llm_dict._json["Department"] == "EMS"
+
+    def test_plural_value_becomes_list(self, llm_dict):
+        llm_dict.add_response_to_json("victims", "Mark Smith;Jane Doe")
+        assert isinstance(llm_dict._json["victims"], list)
+
+    def test_existing_field_becomes_list(self, llm_dict):
+        """Adding to existing field should not overwrite silently."""
+        llm_dict._json["NAME/SID"] = "John"
+        llm_dict.add_response_to_json("NAME/SID", "Jane")
+        assert isinstance(llm_dict._json["NAME/SID"], list)
+
+
+# ── get_data ──────────────────────────────────────────────────────────────────
+
+class TestGetData:
+
+    def test_returns_dict(self, llm_dict):
+        assert isinstance(llm_dict.get_data(), dict)
+
+    def test_returns_same_reference_as_internal_json(self, llm_dict):
+        llm_dict._json["test_key"] = "test_value"
+        assert llm_dict.get_data()["test_key"] == "test_value"
+
+
+# ── main_loop (mocked Ollama) ─────────────────────────────────────────────────
+
+class TestMainLoop:
+
+    def _mock_response(self, json_body: dict):
+        """Build a mock requests.Response returning a valid Mistral JSON reply."""
+        mock_resp = MagicMock()
+        mock_resp.raise_for_status = MagicMock()
+        mock_resp.json.return_value = {
+            "response": json.dumps(json_body)
+        }
+        return mock_resp
+
+    def test_batch_success_fills_all_fields(self, llm_dict, dict_fields):
+        expected = {
+            "NAME/SID":     "John Smith",
+            "JobTitle":     "Firefighter Paramedic",
+            "Department":   "Emergency Medical Services",
+            "Phone Number": "916-555-0147",
+            "email":        None,
+        }
+        with patch("requests.post", return_value=self._mock_response(expected)):
+            llm_dict.main_loop()
+
+        result = llm_dict.get_data()
+        assert result["NAME/SID"] == "John Smith"
+        assert result["JobTitle"] == "Firefighter Paramedic"
+        assert result["Department"] == "Emergency Medical Services"
+        assert result["Phone Number"] == "916-555-0147"
+
+    def test_batch_makes_exactly_one_ollama_call(self, llm_dict, dict_fields):
+        """Core performance requirement — O(1) not O(N)."""
+        expected = {k: "value" for k in dict_fields.keys()}
+        with patch("requests.post", return_value=self._mock_response(expected)) as mock_post:
+            llm_dict.main_loop()
+
+        assert mock_post.call_count == 1, (
+            f"Expected 1 Ollama call, got {mock_post.call_count}. "
+            "main_loop() must use batch extraction, not per-field."
+        )
+
+    def test_fallback_on_invalid_json(self, llm_dict, dict_fields):
+        """If Mistral returns non-JSON, fallback per-field runs without crash."""
+        bad_response = MagicMock()
+        bad_response.raise_for_status = MagicMock()
+        bad_response.json.return_value = {"response": "This is not JSON at all."}
+
+        good_response = MagicMock()
+        good_response.raise_for_status = MagicMock()
+        good_response.json.return_value = {"response": "John Smith"}
+
+        # First call returns bad JSON, rest return single values
+        with patch("requests.post", side_effect=[bad_response] + [good_response] * len(dict_fields)):
+            llm_dict.main_loop()  # should not raise
+
+    def test_connection_error_raises_connection_error(self, llm_dict):
+        import requests as req
+        with patch("requests.post", side_effect=req.exceptions.ConnectionError):
+            with pytest.raises(ConnectionError, match="Ollama"):
+                llm_dict.main_loop()
+
+    def test_null_values_stored_as_none(self, llm_dict, dict_fields):
+        """Mistral returning null should be stored as None, not the string 'null'."""
+        response_with_nulls = {k: None for k in dict_fields.keys()}
+        with patch("requests.post", return_value=self._mock_response(response_with_nulls)):
+            llm_dict.main_loop()
+
+        result = llm_dict.get_data()
+        for key in dict_fields.keys():
+            assert result[key] is None, f"Expected None for '{key}', got {result[key]!r}"
diff --git a/tests/test_templates.py b/tests/test_templates.py
index bbced2b..9b7cf8e 100644
--- a/tests/test_templates.py
+++ b/tests/test_templates.py
@@ -1,18 +1,126 @@
-def test_create_template(client):
-    payload = {
-        "name": "Template 1",
-        "pdf_path": "src/inputs/file.pdf",
-        "fields": {
-            "Employee's name": "string",
-            "Employee's job title": "string",
-            "Employee's department supervisor": "string",
-            "Employee's phone number": "string",
-            "Employee's email": "string",
-            "Signature": "string",
-            "Date": "string",
-        },
-    }
-
-    response = client.post("/templates/create", json=payload)
-
-    assert response.status_code == 200
+"""
+Tests for /templates endpoints.
+Closes #162, #160, #163
+"""
+
+import io
+import pytest
+from unittest.mock import patch, MagicMock
+from api.db.models import Template
+from datetime import datetime
+
+
+# ── POST /templates/create ────────────────────────────────────────────────────
+
+class TestCreateTemplate:
+
+    def test_create_template_success(self, client):
+        """Uploading a valid PDF returns 200 with template data."""
+        pdf_bytes = (
+            b"%PDF-1.4\n1 0 obj<</Type/Catalog/Pages 2 0 R>>endobj\n"
+            b"2 0 obj<</Type/Pages/Kids[3 0 R]/Count 1>>endobj\n"
+            b"3 0 obj<</Type/Page/MediaBox[0 0 612 792]/Parent 2 0 R>>endobj\n"
+            b"xref\n0 4\n0000000000 65535 f\n"
+            b"trailer<</Size 4/Root 1 0 R>>\nstartxref\n0\n%%EOF"
+        )
+
+        mock_fields = {
+            "JobTitle": {"/T": "JobTitle", "/FT": "/Tx"},
+            "Department": {"/T": "Department", "/FT": "/Tx"},
+        }
+
+        with patch("commonforms.prepare_form"), \
+             patch("pypdf.PdfReader") as mock_reader, \
+             patch("shutil.copyfileobj"), \
+             patch("builtins.open", MagicMock()), \
+             patch("os.path.exists", return_value=True), \
+             patch("os.remove"):
+
+            mock_reader.return_value.get_fields.return_value = mock_fields
+
+            response = client.post(
+                "/templates/create",
+                files={"file": ("form.pdf", io.BytesIO(pdf_bytes), "application/pdf")},
+                data={"name": "Vaccine Form"},
+            )
+
+        assert response.status_code == 200
+        data = response.json()
+        assert data["name"] == "Vaccine Form"
+        assert "id" in data
+        assert "fields" in data
+
+    def test_create_template_without_file_returns_422(self, client):
+        """Missing file field returns 422 Unprocessable Entity."""
+        response = client.post(
+            "/templates/create",
+            data={"name": "No File"},
+        )
+        assert response.status_code == 422
+
+    def test_create_template_non_pdf_returns_400(self, client):
+        """Uploading a non-PDF returns 400."""
+        with patch("shutil.copyfileobj"), \
+             patch("builtins.open", MagicMock()):
+            response = client.post(
+                "/templates/create",
+                files={"file": ("data.csv", io.BytesIO(b"a,b,c"), "text/csv")},
+                data={"name": "CSV attempt"},
+            )
+        assert response.status_code == 400
+
+
+# ── GET /templates ────────────────────────────────────────────────────────────
+
+class TestListTemplates:
+
+    def test_list_templates_returns_200(self, client):
+        """GET /templates returns 200."""
+        response = client.get("/templates")
+        assert response.status_code == 200
+
+    def test_list_templates_returns_list(self, client):
+        """Response is always a list."""
+        response = client.get("/templates")
+        assert isinstance(response.json(), list)
+
+    def test_list_templates_empty_on_fresh_db(self, client):
+        """Fresh DB returns empty list."""
+        response = client.get("/templates")
+        assert response.json() == []
+
+    def test_list_templates_pagination_accepted(self, client):
+        """Pagination params accepted without error."""
+        response = client.get("/templates?limit=5&offset=0")
+        assert response.status_code == 200
+
+
+# ── GET /templates/{template_id} ──────────────────────────────────────────────
+
+class TestGetTemplate:
+
+    def test_get_template_not_found(self, client):
+        """Returns 404 for non-existent ID."""
+        response = client.get("/templates/999999")
+        assert response.status_code == 404
+
+    def test_get_template_invalid_id_type(self, client):
+        """Returns 422 for non-integer ID."""
+        response = client.get("/templates/not-an-id")
+        assert response.status_code == 422
+
+    def test_get_template_by_id(self, client, db_session):
+        """Returns correct template for valid ID."""
+        t = Template(
+            name="Cal Fire Form",
+            fields={"officer_name": "Officer Name"},
+            pdf_path="/tmp/cal_fire.pdf",
+            created_at=datetime.utcnow(),
+        )
+        db_session.add(t)
+        db_session.commit()
+        db_session.refresh(t)
+
+        response = client.get(f"/templates/{t.id}")
+        assert response.status_code == 200
+        assert response.json()["name"] == "Cal Fire Form"