diff --git a/src/backend/api/routers/recipes.py b/src/backend/api/routers/recipes.py index 11cabcf..9ce7d87 100644 --- a/src/backend/api/routers/recipes.py +++ b/src/backend/api/routers/recipes.py @@ -189,6 +189,11 @@ async def parse_technical_sheet( tasks = await ai_service.parse_technical_sheet(pdf_bytes) except ValueError as e: raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e)) + except ai_service.AIResponseError as e: + # The AI model's own response was malformed (bad JSON, missing + # fields): that is the upstream service's fault, not ours (400). + logging.getLogger(__name__).error("AI response malformed: %s", e) + raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail="Risposta del servizio AI non valida") except Exception as e: logging.getLogger(__name__).error("AI parsing error: %s", e) raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail="Errore nel servizio AI") diff --git a/src/backend/services/ai_service.py b/src/backend/services/ai_service.py index 95ccf57..3bd3798 100644 --- a/src/backend/services/ai_service.py +++ b/src/backend/services/ai_service.py @@ -11,6 +11,17 @@ from src.backend.config import settings logger = logging.getLogger(__name__) + +class AIResponseError(Exception): + """The AI model's response could not be used: missing fields or invalid JSON. + + Distinct from ValueError (raised for our own misconfiguration, e.g. a + missing API key) so the router can tell "we are broken" (400) apart from + "the upstream AI service is broken" (502) instead of both landing on the + same except clause. + """ + + SYSTEM_PROMPT = """Sei un assistente specializzato nell'analisi di schede tecniche industriali. Analizza il testo estratto da una scheda tecnica e identifica i blocchi operativi distinti. Per ogni blocco, estrai: @@ -65,7 +76,10 @@ async def parse_technical_sheet(pdf_bytes: bytes) -> list[dict]: resp.raise_for_status() data = resp.json() - content = data["choices"][0]["message"]["content"].strip() + try: + content = data["choices"][0]["message"]["content"].strip() + except (KeyError, IndexError, TypeError, AttributeError) as e: + raise AIResponseError(f"Risposta del modello AI malformata: {e}") from e # Strip markdown fences if present if content.startswith("```"): @@ -74,4 +88,10 @@ async def parse_technical_sheet(pdf_bytes: bytes) -> list[dict]: content = content[:-3] content = content.strip() - return json.loads(content) + try: + return json.loads(content) + except json.JSONDecodeError as e: + # json.JSONDecodeError is itself a ValueError: without this, the + # router's `except ValueError` (meant for our own config errors) + # would misreport a broken upstream response as our fault (400). + raise AIResponseError(f"Risposta del modello AI non e' JSON valido: {e}") from e diff --git a/src/backend/tests/test_ai_service.py b/src/backend/tests/test_ai_service.py new file mode 100644 index 0000000..2a2f0ad --- /dev/null +++ b/src/backend/tests/test_ai_service.py @@ -0,0 +1,67 @@ +"""Tests for ai_service.py error handling (gitea#36 point 7). + +A malformed response from the AI model (missing fields, invalid JSON) must +raise the dedicated AIResponseError, not a bare KeyError/JSONDecodeError — +JSONDecodeError in particular is itself a ValueError subclass, so without +this it was caught by the router's `except ValueError` (meant for our own +config errors) and misreported as a 400 instead of a 502. +""" +import json +from unittest.mock import AsyncMock, patch + +import pytest + +from src.backend.services import ai_service + + +class _FakeResponse: + def __init__(self, json_data): + self._json_data = json_data + + def raise_for_status(self): + pass + + def json(self): + return self._json_data + + +def _patch_openrouter(monkeypatch, response_json: dict): + monkeypatch.setattr(ai_service, "extract_text_from_pdf", lambda _b: "some extracted text") + monkeypatch.setattr(ai_service.settings, "openrouter_api_key", "test-key") + monkeypatch.setattr(ai_service.settings, "openrouter_model", "test-model") + + fake_client = AsyncMock() + fake_client.post = AsyncMock(return_value=_FakeResponse(response_json)) + fake_client.__aenter__ = AsyncMock(return_value=fake_client) + fake_client.__aexit__ = AsyncMock(return_value=False) + + return patch("src.backend.services.ai_service.httpx.AsyncClient", return_value=fake_client) + + +async def test_missing_choices_key_raises_ai_response_error(monkeypatch): + with _patch_openrouter(monkeypatch, {"no_choices_here": True}): + with pytest.raises(ai_service.AIResponseError): + await ai_service.parse_technical_sheet(b"%PDF-fake") + + +async def test_invalid_json_content_raises_ai_response_error(monkeypatch): + bad_content = "not valid json {{{" + response_json = {"choices": [{"message": {"content": bad_content}}]} + with _patch_openrouter(monkeypatch, response_json): + with pytest.raises(ai_service.AIResponseError): + await ai_service.parse_technical_sheet(b"%PDF-fake") + + +async def test_valid_json_content_returns_tasks(monkeypatch): + tasks = [{"title": "MATERIALI", "directive": "Verificare", "description": "..."}] + response_json = {"choices": [{"message": {"content": json.dumps(tasks)}}]} + with _patch_openrouter(monkeypatch, response_json): + result = await ai_service.parse_technical_sheet(b"%PDF-fake") + assert result == tasks + + +async def test_empty_extracted_text_returns_empty_list(monkeypatch): + """No AI call at all when the PDF has no extractable text.""" + monkeypatch.setattr(ai_service, "extract_text_from_pdf", lambda _b: " ") + result = await ai_service.parse_technical_sheet(b"%PDF-fake") + assert result == [] diff --git a/src/backend/tests/test_recipes_ai_parse_error.py b/src/backend/tests/test_recipes_ai_parse_error.py new file mode 100644 index 0000000..969348a --- /dev/null +++ b/src/backend/tests/test_recipes_ai_parse_error.py @@ -0,0 +1,33 @@ +"""Router-level test for gitea#36 point 7: recipes.py translates +ai_service.AIResponseError to 502, not the config-error 400. + +A dedicated file (not test_recipes.py) to avoid colliding with concurrent +work on the copy-on-write recipe logic in that shared test module. +""" +from httpx import AsyncClient +from sqlalchemy.ext.asyncio import AsyncSession + +from src.backend.models.orm.user import User +from src.backend.services import ai_service +from src.backend.tests.conftest import auth_headers, create_test_recipe + + +async def test_ai_response_error_is_502( + client: AsyncClient, + maker_user: User, + db_session: AsyncSession, + monkeypatch, +): + recipe = await create_test_recipe(db_session, maker_user.id, code="AI-PARSE-502") + + async def _raise_ai_response_error(_pdf_bytes): + raise ai_service.AIResponseError("Risposta del modello AI non e' JSON valido: boom") + + monkeypatch.setattr(ai_service, "parse_technical_sheet", _raise_ai_response_error) + + resp = await client.post( + f"/api/recipes/{recipe.id}/parse-technical-sheet", + headers=auth_headers(maker_user), + files={"file": ("sheet.pdf", b"%PDF-1.4 fake", "application/pdf")}, + ) + assert resp.status_code == 502