From aa898a85891f9ac09c5cb87f274960ebe2ea1456 Mon Sep 17 00:00:00 2001 From: charles Date: Tue, 6 Jan 2026 14:44:14 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=A8(backend)=20various=20review=20fixe?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I am doing various small review fixies --- .gitignore | 3 ++ docs/env.md | 2 +- .../document_rag_backends/find_rag_backend.py | 2 +- src/backend/chat/clients/pydantic_ai.py | 8 ++-- src/backend/chat/tests/views/chat/conftest.py | 17 ++++++++ .../test_conversation_with_document_upload.py | 12 +----- .../test_conversation_with_document_url.py | 11 ----- .../test_conversation_with_image_url.py | 11 ----- .../conversations/configuration/llm/dev.json | 43 ------------------- src/backend/utils/oidc.py | 6 +-- 10 files changed, 29 insertions(+), 86 deletions(-) create mode 100644 src/backend/chat/tests/views/chat/conftest.py delete mode 100644 src/backend/conversations/configuration/llm/dev.json diff --git a/.gitignore b/.gitignore index 679c41e..b3d259f 100644 --- a/.gitignore +++ b/.gitignore @@ -44,6 +44,9 @@ env.d/development/* !env.d/development/*.dist env.d/terraform +# Configuration +**/conversations/configuration/llm/dev.json + # npm node_modules diff --git a/docs/env.md b/docs/env.md index b9b9361..dff03ef 100644 --- a/docs/env.md +++ b/docs/env.md @@ -96,7 +96,7 @@ These are the environment variables you can set for the `conversations-backend` | THEME_CUSTOMIZATION_FILE_PATH | full path to the file customizing the theme. An example is provided in src/backend/conversations/configuration/theme/default.json | BASE_DIR/conversations/configuration/theme/default.json | | THEME_CUSTOMIZATION_CACHE_TIMEOUT | Cache duration for the customization settings | 86400 | | FIND_API_KEY | API key of Find | | -| FIND_API_URL | URL of Find | https://app-find/api | +| FIND_API_URL | URL of Find | `https://app-find/api` | | FIND_API_TIMEOUT | Find API timeout | 30 | diff --git a/src/backend/chat/agent_rag/document_rag_backends/find_rag_backend.py b/src/backend/chat/agent_rag/document_rag_backends/find_rag_backend.py index 6cec9b9..68f8145 100644 --- a/src/backend/chat/agent_rag/document_rag_backends/find_rag_backend.py +++ b/src/backend/chat/agent_rag/document_rag_backends/find_rag_backend.py @@ -48,7 +48,7 @@ class FindRagBackend(BaseRagBackend): """ init collection_id """ - self.collection_id = self.collection_id or uuid.uuid4() + self.collection_id = self.collection_id or str(uuid.uuid4()) return self.collection_id def delete_collection(self) -> None: diff --git a/src/backend/chat/clients/pydantic_ai.py b/src/backend/chat/clients/pydantic_ai.py index ab8eb91..23da540 100644 --- a/src/backend/chat/clients/pydantic_ai.py +++ b/src/backend/chat/clients/pydantic_ai.py @@ -242,9 +242,7 @@ class AIAgentService: # pylint: disable=too-many-instance-attributes # --------------------------------------------------------------------- # # Core agent runner # --------------------------------------------------------------------- # - async def parse_input_documents( - self, documents: List[BinaryContent | DocumentUrl], user_sub: str - ): + async def parse_input_documents(self, documents: List[BinaryContent | DocumentUrl]): """ Parse and store input documents in the conversation's document store. """ @@ -288,7 +286,7 @@ class AIAgentService: # pylint: disable=too-many-instance-attributes name=document.identifier, content_type=document.media_type, content=document_data, - user_sub=user_sub, + user_sub=self.user.sub, ) else: # Remote URL @@ -436,7 +434,7 @@ class AIAgentService: # pylint: disable=too-many-instance-attributes ) try: - await self.parse_input_documents(input_documents, user_sub=self.user.sub) + await self.parse_input_documents(input_documents) except Exception as exc: # pylint: disable=broad-except logger.exception("Error parsing input documents: %s", exc) yield events_v4.ToolResultPart( diff --git a/src/backend/chat/tests/views/chat/conftest.py b/src/backend/chat/tests/views/chat/conftest.py new file mode 100644 index 0000000..d4be3b6 --- /dev/null +++ b/src/backend/chat/tests/views/chat/conftest.py @@ -0,0 +1,17 @@ +"""Common test fixtures for chat views tests.""" + +from unittest import mock + +import pytest + + +@pytest.fixture(autouse=True) +def mock_process_request(): + """ + Mock process_request to bypass OIDC authentication in tests. + """ + with mock.patch( + "lasuite.oidc_login.decorators.RefreshOIDCAccessToken.process_request" + ) as mocked_process_request: + mocked_process_request.return_value = None + yield mocked_process_request diff --git a/src/backend/chat/tests/views/chat/conversations/test_conversation_with_document_upload.py b/src/backend/chat/tests/views/chat/conversations/test_conversation_with_document_upload.py index 4d20aa9..74aae36 100644 --- a/src/backend/chat/tests/views/chat/conversations/test_conversation_with_document_upload.py +++ b/src/backend/chat/tests/views/chat/conversations/test_conversation_with_document_upload.py @@ -75,16 +75,6 @@ def ai_settings(request, settings): return settings -@pytest.fixture(autouse=True) -def mock_process_request(): - """Mock process_request to bypass authentication in tests.""" - with mock.patch( - "lasuite.oidc_login.decorators.RefreshOIDCAccessToken.process_request" - ) as mocked_process_request: - mocked_process_request.return_value = None - yield mocked_process_request - - @pytest.fixture(autouse=True) def mock_refresh_access_token(): """Mock refresh_access_token to bypass token refresh in tests.""" @@ -168,7 +158,7 @@ def fixture_mock_document_api(): "score": search_score, } ], - "usage": {"prompt_tokens": 10, "completion_tokens": 20}, + "usage": {"prompt_tokens": prompt_tokens, "completion_tokens": completion_tokens}, }, status=status.HTTP_200_OK, ) diff --git a/src/backend/chat/tests/views/chat/conversations/test_conversation_with_document_url.py b/src/backend/chat/tests/views/chat/conversations/test_conversation_with_document_url.py index a54756d..9a519d6 100644 --- a/src/backend/chat/tests/views/chat/conversations/test_conversation_with_document_url.py +++ b/src/backend/chat/tests/views/chat/conversations/test_conversation_with_document_url.py @@ -4,7 +4,6 @@ import uuid # pylint: disable=too-many-lines from io import BytesIO -from unittest import mock from django.core.files.storage import default_storage from django.utils import formats, timezone @@ -67,16 +66,6 @@ def fixture_sample_document_content(): ) -@pytest.fixture(autouse=True) -def mock_process_request(): - """Mock process_request to bypass authentication in tests.""" - with mock.patch( - "lasuite.oidc_login.decorators.RefreshOIDCAccessToken.process_request" - ) as mocked_process_request: - mocked_process_request.return_value = None - yield mocked_process_request - - @responses.activate @freeze_time() def test_post_conversation_with_local_pdf_document_url( diff --git a/src/backend/chat/tests/views/chat/conversations/test_conversation_with_image_url.py b/src/backend/chat/tests/views/chat/conversations/test_conversation_with_image_url.py index 89865bf..2ea172f 100644 --- a/src/backend/chat/tests/views/chat/conversations/test_conversation_with_image_url.py +++ b/src/backend/chat/tests/views/chat/conversations/test_conversation_with_image_url.py @@ -1,7 +1,6 @@ """Unit tests for chat conversation actions with image URL.""" import uuid -from unittest import mock from django.utils import formats, timezone @@ -54,16 +53,6 @@ def fixture_sample_image_content(): ) -@pytest.fixture(autouse=True) -def mock_process_request(): - """Mock process_request to bypass authentication in tests.""" - with mock.patch( - "lasuite.oidc_login.decorators.RefreshOIDCAccessToken.process_request" - ) as mocked_process_request: - mocked_process_request.return_value = None - yield mocked_process_request - - @freeze_time("2025-10-18T20:48:20.286204Z") def test_post_conversation_with_local_image_url( api_client, diff --git a/src/backend/conversations/configuration/llm/dev.json b/src/backend/conversations/configuration/llm/dev.json deleted file mode 100644 index a004952..0000000 --- a/src/backend/conversations/configuration/llm/dev.json +++ /dev/null @@ -1,43 +0,0 @@ -{ - "models": [ - { - "hrid": "default-model", - "model_name": "settings.AI_MODEL", - "human_readable_name": "Default Model", - "provider_name": "default-provider", - "profile": null, - "settings": {}, - "is_active": true, - "icon": [ - "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABwAAAAcCAMAAABF0y+mAAAAn1BMVEUALosAKoovTZjw8vb////+9/jlPUniAAz", - "iABUAGIWbpsTwq7HhAAAAI4dle7DrdX4AJohRaaboXWj7+/zn6On5//9NZaT29vfoWmVHYKDoUl/k5OUAIYddc6vpbHYCM47Y3+v53+LiFCUA", - "HIWnsckYPJHi6PL77O7jJjW3wdf1w8jre4QgQ5TZ2txwg7Pr3+I8WZ6OnsTuoamClL7tlZ5xz5y8AAAAzUlEQVR4AZ3RRQKDQBBEUSTu7h5c4", - "vc/W6Yp3KG2Dz4ynDdeEBvOmq12xx2E1u0B+4NOEocj4DgNJ1PgLAvni8WyBq5Yc71ubFJx23C2q4P7dRYejg1xzvCUgvz5guz11k7gXYKF/1", - "8oyiYuvHAYeVkhXCzolVStHcGDjiQzNmMQxsMI5rEJRdQSPZvbpE2E8aY6gC6Z+2Hg4dFA0Yb4YedNL/v4Fk8WJuwiGhrChJNXI210rnib9Fs", - "JlXRUC/HwTscPIXf/iklq/tjb/gHAdxkCUjAg2QAAAABJRU5ErkJggg==" - ], - "system_prompt": "settings.AI_AGENT_INSTRUCTIONS", - "tools": "settings.AI_AGENT_TOOLS" - }, - { - "hrid": "default-summarization-model", - "model_name": "settings.AI_MODEL", - "human_readable_name": "Default Summarization Model", - "provider_name": "default-provider", - "profile": null, - "settings": {}, - "is_active": true, - "icon": null, - "system_prompt": "settings.SUMMARIZATION_SYSTEM_PROMPT", - "tools": [] - } - ], - "providers": [ - { - "hrid": "default-provider", - "base_url": "settings.AI_BASE_URL", - "api_key": "settings.AI_API_KEY", - "kind": "mistral" - } - ] -} diff --git a/src/backend/utils/oidc.py b/src/backend/utils/oidc.py index 5f1fb33..aed4901 100644 --- a/src/backend/utils/oidc.py +++ b/src/backend/utils/oidc.py @@ -45,10 +45,10 @@ def with_fresh_access_token(func): @wraps(func) def wrapper(*args, **kwargs): - session = kwargs.get("session") + session = kwargs.pop("session", None) if session is None: raise AuthenticationFailed({"error": "Session is required but not provided"}) - kwargs["session"] = refresh_access_token(session) - return func(*args, **kwargs) + refreshed_session = refresh_access_token(session) + return func(*args, session=refreshed_session, **kwargs) return wrapper