diff --git a/backend/tests/openai_wire_capture.py b/backend/tests/openai_wire_capture.py new file mode 100644 index 000000000..553b38b15 --- /dev/null +++ b/backend/tests/openai_wire_capture.py @@ -0,0 +1,51 @@ +"""Shared helper for capturing the real OpenAI SDK request body at the wire. + +Patching ``AsyncOpenAI`` itself (as most tests in this suite do) proves only +that production code passes the right arguments to the SDK -- it never runs +the SDK's own request-construction logic, so a test built that way can stay +green while the SDK's actual request path drifts (Devin Review, naruon#1529: +"wire coverage stops before transport"). ``CapturingTransport`` patches one +level lower, at ``httpx``'s transport boundary, so the genuine +``AsyncOpenAI`` client and all its internal logic run for real; only the +actual TCP/network send is replaced. +""" + +from __future__ import annotations + +import json +from typing import Any + +import httpx + + +class CapturingTransport(httpx.AsyncBaseTransport): + """An httpx transport that records the outgoing request instead of sending it.""" + + def __init__(self, response_content: dict[str, Any]) -> None: + """Store the fixed response body to return for every captured request.""" + self.captured_request: httpx.Request | None = None + self.captured_body: dict[str, Any] | None = None + self._response_content = response_content + + async def handle_async_request(self, request: httpx.Request) -> httpx.Response: + """Record the request and its JSON body, then return the fixed response.""" + self.captured_request = request + self.captured_body = json.loads(request.content) + return httpx.Response(200, json=self._response_content, request=request) + + +def chat_completion_response(content: str) -> dict[str, Any]: + """Return a minimal valid OpenAI chat-completion response body for ``content``.""" + return { + "id": "chatcmpl-test", + "object": "chat.completion", + "created": 0, + "model": "gpt-test", + "choices": [ + { + "index": 0, + "message": {"role": "assistant", "content": content}, + "finish_reason": "stop", + } + ], + } diff --git a/backend/tests/test_llm_service.py b/backend/tests/test_llm_service.py index 463603a45..4accdc6bd 100644 --- a/backend/tests/test_llm_service.py +++ b/backend/tests/test_llm_service.py @@ -1,7 +1,9 @@ import asyncio +import json import httpx import pytest +from openai import AsyncOpenAI from unittest.mock import AsyncMock, MagicMock, patch from core.config import settings @@ -15,6 +17,7 @@ extract_action_items_and_summary, translate_email_body, ) +from tests.openai_wire_capture import CapturingTransport, chat_completion_response @pytest.fixture @@ -276,6 +279,106 @@ async def test_extract_action_items_and_summary_success(mock_openai): mock_openai.beta.chat.completions.parse.call_args.kwargs["model"] == settings.OPENAI_MODEL ) + # This proves the *local* contract only: `extract_action_items_and_summary` + # passes the `ExtractionResult` model class as `response_format`. It does + # not exercise the openai SDK's own Pydantic-to-JSON-schema serialization + # (AsyncOpenAI is mocked above), so it does not by itself prove the actual + # wire envelope -- see + # `test_extraction_result_serializes_to_the_openai_json_schema_wire_envelope` + # below for that, unmocked (Devin Review, naruon#1529: this comment + # previously claimed passing the model class here "is" the wire shape, + # which this test alone does not demonstrate). + assert ( + mock_openai.beta.chat.completions.parse.call_args.kwargs["response_format"] + is ExtractionResult + ) + + +def test_extraction_result_serializes_to_the_openai_json_schema_wire_envelope(): + """Prove the actual wire envelope the OpenAI SDK sends, not just the local kwarg. + + Calls `openai.lib._parsing.type_to_response_format_param` directly, with + no mocking, against the real `ExtractionResult` model -- the exact + function `.beta.chat.completions.parse()` calls internally to build the + request body. Also confirms `confidence`'s `Field(ge=0, le=100)` + constraint survives into the wire schema as `minimum`/`maximum`, not just + that the field exists. + """ + from openai.lib._parsing import type_to_response_format_param + + envelope = type_to_response_format_param(ExtractionResult) + + assert envelope["type"] == "json_schema" + assert envelope["json_schema"]["name"] == "ExtractionResult" + assert envelope["json_schema"]["strict"] is True + schema = envelope["json_schema"]["schema"] + assert schema["type"] == "object" + assert set(schema["properties"]) == { + "summary", + "action_items", + "provenance", + "confidence", + } + assert schema["additionalProperties"] is False + confidence_variants = schema["properties"]["confidence"]["anyOf"] + bounded = next(v for v in confidence_variants if v.get("type") == "integer") + assert bounded["minimum"] == 0 + assert bounded["maximum"] == 100 + + +@pytest.mark.asyncio +async def test_extract_action_items_and_summary_sends_the_actual_wire_body(): + """Prove the real request bytes, not just a transformation function in isolation. + + The previous test above proves `type_to_response_format_param` builds + the right envelope in isolation -- it does not prove + `extract_action_items_and_summary`'s real `AsyncOpenAI` client actually + uses that function (or its result verbatim) when constructing a genuine + outgoing request, through the circuit-breaker/retry wrapper this + function adds on top of the raw SDK call (Devin Review: "wire coverage + stops before transport"). This patches `AsyncOpenAI` with a *real* + client wired to a custom `httpx` transport instead of a mock, so the + SDK's entire real request-construction path runs; only the actual + network send is intercepted. + """ + transport = CapturingTransport( + chat_completion_response( + json.dumps({"summary": "s", "action_items": [], "confidence": 50}) + ) + ) + real_client = AsyncOpenAI( + api_key="test-key", http_client=httpx.AsyncClient(transport=transport) + ) + + with patch("services.llm_service.AsyncOpenAI", return_value=real_client): + result = await extract_action_items_and_summary( + "Test email", "test-key", model="gpt-test" + ) + + assert result.summary == "s" + assert transport.captured_body is not None + assert transport.captured_body["model"] == "gpt-test" + response_format = transport.captured_body["response_format"] + assert response_format["type"] == "json_schema" + assert response_format["json_schema"]["name"] == "ExtractionResult" + assert response_format["json_schema"]["strict"] is True + + +@pytest.mark.asyncio +async def test_extract_action_items_and_summary_raises_on_unparsable_response( + mock_openai, +): + """A schema-violating/empty completion must fail closed, not pass through.""" + mock_response = MagicMock() + mock_message = MagicMock() + mock_message.parsed = None + mock_choice = MagicMock() + mock_choice.message = mock_message + mock_response.choices = [mock_choice] + mock_openai.beta.chat.completions.parse = AsyncMock(return_value=mock_response) + + with pytest.raises(RuntimeError, match="Failed to parse LLM response"): + await extract_action_items_and_summary("Test email", "test-key") @pytest.mark.asyncio diff --git a/backend/tests/test_project_graph_llm_extractor.py b/backend/tests/test_project_graph_llm_extractor.py index 0da1f5165..2de70fdc0 100644 --- a/backend/tests/test_project_graph_llm_extractor.py +++ b/backend/tests/test_project_graph_llm_extractor.py @@ -1,14 +1,18 @@ """Tests for the LLM-grounded project extractor and its import selection.""" +import json import types import pytest -from unittest.mock import AsyncMock, Mock +from unittest.mock import AsyncMock, Mock, patch + +import httpx import services.email_import_service as import_service import services.project_graph.extractor_registry as extractor_registry import services.project_graph.llm_extractor as llm_extractor from services.project_graph import ProjectObjectType, ProjectSourceSegment +from tests.openai_wire_capture import CapturingTransport, chat_completion_response def _segment(uid: str, text: str) -> ProjectSourceSegment: @@ -179,6 +183,134 @@ async def test_empty_segments_short_circuit_without_llm_call(monkeypatch): assert result.objects == () +@pytest.mark.asyncio +async def test_call_llm_sends_openai_json_schema_response_format(): + """`_call_llm` must request OpenAI structured output for the extraction shape. + + Proves the *local* contract: `_call_llm` passes `ExtractionPayload` as + `response_format` and the right `model`. It mocks `AsyncOpenAI` entirely, + so it does not exercise the SDK's own Pydantic-to-JSON-schema + serialization -- that is what + `test_extraction_payload_serializes_to_the_openai_json_schema_wire_envelope` + below verifies directly, unmocked (Devin Review: the wire-format claim + this docstring previously made here was not actually proven by this test). + """ + mock_client = Mock() + mock_client.close = AsyncMock() + mock_response = Mock() + mock_message = Mock() + mock_message.parsed = llm_extractor.ExtractionPayload(objects=[]) + mock_response.choices = [Mock(message=mock_message)] + mock_client.beta.chat.completions.parse = AsyncMock(return_value=mock_response) + + with patch( + "services.project_graph.llm_extractor.AsyncOpenAI", return_value=mock_client + ): + result = await llm_extractor._call_llm( + api_key="key", + base_url=None, + model="gpt-test", + segments_json="{}", + ) + + assert result.objects == [] + call_kwargs = mock_client.beta.chat.completions.parse.call_args.kwargs + assert call_kwargs["response_format"] is llm_extractor.ExtractionPayload + assert call_kwargs["model"] == "gpt-test" + + +def test_extraction_payload_serializes_to_the_openai_json_schema_wire_envelope(): + """Prove the actual wire envelope the OpenAI SDK sends, not just the local kwarg. + + ``test_call_llm_sends_openai_json_schema_response_format`` above mocks + ``AsyncOpenAI`` entirely, so it only proves ``_call_llm`` passes + ``ExtractionPayload`` as the ``response_format`` kwarg -- it never runs the + openai SDK's own Pydantic-model-to-JSON-schema serialization, so a bug in + that serialization (or in ``ExtractionPayload``'s shape triggering one) + would still pass every mocked test (Devin Review). This calls + ``openai.lib._parsing.type_to_response_format_param`` directly, with no + mocking, against the real ``ExtractionPayload`` model -- the exact + function ``.beta.chat.completions.parse()`` calls internally to build the + request body, so this is the actual wire format, not an approximation of + it. + """ + from openai.lib._parsing import type_to_response_format_param + + envelope = type_to_response_format_param(llm_extractor.ExtractionPayload) + + assert envelope["type"] == "json_schema" + assert envelope["json_schema"]["name"] == "ExtractionPayload" + assert envelope["json_schema"]["strict"] is True + schema = envelope["json_schema"]["schema"] + assert schema["type"] == "object" + assert set(schema["properties"]) == {"objects", "relations"} + assert schema["additionalProperties"] is False + + +@pytest.mark.asyncio +async def test_call_llm_sends_the_actual_wire_body_through_a_real_client(): + """Prove the real request bytes, not just a transformation function in isolation. + + The previous test above proves ``type_to_response_format_param`` builds + the right envelope in isolation -- it does not prove ``_call_llm``'s real + ``AsyncOpenAI`` client actually uses that function (or its result + verbatim) when constructing a genuine outgoing request (Devin Review: + "wire coverage stops before transport" -- a changed request path could + leave that test green while transmitting a different body). This + patches ``AsyncOpenAI`` with a *real* client wired to a custom + ``httpx`` transport instead of a mock, so the SDK's entire real request- + construction path runs; only the actual network send is intercepted, + one layer below all of the SDK's own logic. + """ + transport = CapturingTransport( + chat_completion_response(json.dumps({"objects": [], "relations": []})) + ) + real_client = llm_extractor.AsyncOpenAI( + api_key="test-key", http_client=httpx.AsyncClient(transport=transport) + ) + + with patch( + "services.project_graph.llm_extractor.AsyncOpenAI", return_value=real_client + ): + result = await llm_extractor._call_llm( + api_key="key", + base_url=None, + model="gpt-test", + segments_json="{}", + ) + + assert result.objects == [] + assert transport.captured_body is not None + assert transport.captured_body["model"] == "gpt-test" + response_format = transport.captured_body["response_format"] + assert response_format["type"] == "json_schema" + assert response_format["json_schema"]["name"] == "ExtractionPayload" + assert response_format["json_schema"]["strict"] is True + + +@pytest.mark.asyncio +async def test_call_llm_raises_on_unparsable_response(): + """A schema-violating completion must fail closed, not return corrupted data.""" + mock_client = Mock() + mock_client.close = AsyncMock() + mock_response = Mock() + mock_message = Mock() + mock_message.parsed = None + mock_response.choices = [Mock(message=mock_message)] + mock_client.beta.chat.completions.parse = AsyncMock(return_value=mock_response) + + with patch( + "services.project_graph.llm_extractor.AsyncOpenAI", return_value=mock_client + ): + with pytest.raises(RuntimeError, match="unparsable payload"): + await llm_extractor._call_llm( + api_key="key", + base_url=None, + model="gpt-test", + segments_json="{}", + ) + + def _object( *, object_type: str, diff --git a/backend/tests/test_search_answer.py b/backend/tests/test_search_answer.py index 2a095c069..bac6567c1 100644 --- a/backend/tests/test_search_answer.py +++ b/backend/tests/test_search_answer.py @@ -1,9 +1,133 @@ """Tests for the grounded-answer endpoint and RAG citation enforcement.""" +import json + +import httpx import pytest -from unittest.mock import AsyncMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import services.rag_service as rag_service +from tests.openai_wire_capture import CapturingTransport, chat_completion_response + + +@pytest.mark.asyncio +async def test_call_llm_sends_openai_json_schema_response_format(): + """`_call_llm` must request OpenAI structured output for the answer shape. + + Proves the *local* contract: `_call_llm` passes `GroundedAnswerPayload` as + `response_format` and the right `model`. It mocks `AsyncOpenAI` entirely, + so it does not exercise the SDK's own Pydantic-to-JSON-schema + serialization -- that is what + `test_grounded_answer_payload_serializes_to_the_openai_json_schema_wire_envelope` + below verifies directly, unmocked (Devin Review: the wire-format claim + this docstring previously made here was not actually proven by this test). + """ + mock_client = MagicMock() + mock_client.close = AsyncMock() + mock_response = MagicMock() + mock_message = MagicMock() + mock_message.parsed = rag_service.GroundedAnswerPayload( + answer="grounded", cited_email_ids=[1] + ) + mock_response.choices = [MagicMock(message=mock_message)] + mock_client.beta.chat.completions.parse = AsyncMock(return_value=mock_response) + + with patch("services.rag_service.AsyncOpenAI", return_value=mock_client): + result = await rag_service._call_llm( + api_key="k", + base_url=None, + model="gpt-test", + question="q", + emails_json="{}", + ) + + assert result.answer == "grounded" + call_kwargs = mock_client.beta.chat.completions.parse.call_args.kwargs + assert call_kwargs["response_format"] is rag_service.GroundedAnswerPayload + assert call_kwargs["model"] == "gpt-test" + + +def test_grounded_answer_payload_serializes_to_the_openai_json_schema_wire_envelope(): + """Prove the actual wire envelope the OpenAI SDK sends, not just the local kwarg. + + Calls ``openai.lib._parsing.type_to_response_format_param`` directly, with + no mocking, against the real ``GroundedAnswerPayload`` model -- the exact + function ``.beta.chat.completions.parse()`` calls internally to build the + request body. + """ + from openai.lib._parsing import type_to_response_format_param + + envelope = type_to_response_format_param(rag_service.GroundedAnswerPayload) + + assert envelope["type"] == "json_schema" + assert envelope["json_schema"]["name"] == "GroundedAnswerPayload" + assert envelope["json_schema"]["strict"] is True + schema = envelope["json_schema"]["schema"] + assert schema["type"] == "object" + assert set(schema["properties"]) == {"answer", "cited_email_ids"} + assert schema["additionalProperties"] is False + + +@pytest.mark.asyncio +async def test_call_llm_sends_the_actual_wire_body_through_a_real_client(): + """Prove the real request bytes, not just a transformation function in isolation. + + The previous test above proves ``type_to_response_format_param`` builds + the right envelope in isolation -- it does not prove ``_call_llm``'s real + ``AsyncOpenAI`` client actually uses that function (or its result + verbatim) when constructing a genuine outgoing request (Devin Review: + "wire coverage stops before transport"). This patches ``AsyncOpenAI`` + with a *real* client wired to a custom ``httpx`` transport instead of a + mock, so the SDK's entire real request-construction path runs; only the + actual network send is intercepted. + """ + transport = CapturingTransport( + chat_completion_response( + json.dumps({"answer": "grounded", "cited_email_ids": [1]}) + ) + ) + real_client = rag_service.AsyncOpenAI( + api_key="test-key", http_client=httpx.AsyncClient(transport=transport) + ) + + with patch("services.rag_service.AsyncOpenAI", return_value=real_client): + result = await rag_service._call_llm( + api_key="k", + base_url=None, + model="gpt-test", + question="q", + emails_json="{}", + ) + + assert result.answer == "grounded" + assert transport.captured_body is not None + assert transport.captured_body["model"] == "gpt-test" + response_format = transport.captured_body["response_format"] + assert response_format["type"] == "json_schema" + assert response_format["json_schema"]["name"] == "GroundedAnswerPayload" + assert response_format["json_schema"]["strict"] is True + + +@pytest.mark.asyncio +async def test_call_llm_raises_on_unparsable_response(): + """A schema-violating completion must fail closed, not return corrupted data.""" + mock_client = MagicMock() + mock_client.close = AsyncMock() + mock_response = MagicMock() + mock_message = MagicMock() + mock_message.parsed = None + mock_response.choices = [MagicMock(message=mock_message)] + mock_client.beta.chat.completions.parse = AsyncMock(return_value=mock_response) + + with patch("services.rag_service.AsyncOpenAI", return_value=mock_client): + with pytest.raises(RuntimeError, match="unparsable payload"): + await rag_service._call_llm( + api_key="k", + base_url=None, + model="gpt-test", + question="q", + emails_json="{}", + ) def _context(email_id: int, content: str = "body") -> dict: