From 59b40f0eeff3b87b207a8598dbb9a79023afd01b Mon Sep 17 00:00:00 2001 From: Sahil D Shah Date: Fri, 13 Feb 2026 15:51:21 -0500 Subject: [PATCH 01/13] REFACTOR Pull apart get_closest_embeddings to make testing easier --- server/api/services/embedding_services.py | 161 +++++++++++++++------- 1 file changed, 112 insertions(+), 49 deletions(-) diff --git a/server/api/services/embedding_services.py b/server/api/services/embedding_services.py index e35f7965..0720a9c8 100644 --- a/server/api/services/embedding_services.py +++ b/server/api/services/embedding_services.py @@ -11,18 +11,17 @@ logger = logging.getLogger(__name__) -def get_closest_embeddings( - user, message_data, document_name=None, guid=None, num_results=10 -): + +def build_query(user, embedding_vector, document_name=None, guid=None, num_results=10): """ - Find the closest embeddings to a given message for a specific user. + Build an unevaluated QuerySet for the closest embeddings. Parameters ---------- user : User The user whose uploaded documents will be searched - message_data : str - The input message to find similar embeddings for + embedding_vector : array-like + Pre-computed embedding vector to compare against document_name : str, optional Filter results to a specific document name guid : str, optional @@ -32,59 +31,52 @@ def get_closest_embeddings( Returns ------- - list[dict] - List of dictionaries containing embedding results with keys: - - name: document name - - text: embedded text content - - page_number: page number in source document - - chunk_number: chunk number within the document - - distance: L2 distance from query embedding - - file_id: GUID of the source file + QuerySet + Unevaluated Django QuerySet ordered by L2 distance, sliced to num_results """ - - encoding_start = time.time() - transformerModel = TransformerModel.get_instance().model - embedding_message = transformerModel.encode(message_data) - encoding_time = time.time() - encoding_start - - db_query_start = time.time() - # Django QuerySets are lazily evaluated if user.is_authenticated: # User sees their own files + files uploaded by superusers - closest_embeddings_query = ( - Embeddings.objects.filter( - Q(upload_file__uploaded_by=user) | Q(upload_file__uploaded_by__is_superuser=True) - ) - .annotate( - distance=L2Distance("embedding_sentence_transformers", embedding_message) - ) - .order_by("distance") + queryset = Embeddings.objects.filter( + Q(upload_file__uploaded_by=user) | Q(upload_file__uploaded_by__is_superuser=True) ) else: # Unauthenticated users only see superuser-uploaded files - closest_embeddings_query = ( - Embeddings.objects.filter(upload_file__uploaded_by__is_superuser=True) - .annotate( - distance=L2Distance("embedding_sentence_transformers", embedding_message) - ) - .order_by("distance") - ) + queryset = Embeddings.objects.filter(upload_file__uploaded_by__is_superuser=True) + + queryset = ( + queryset + .annotate(distance=L2Distance("embedding_sentence_transformers", embedding_vector)) + .order_by("distance") + ) # Filtering to a document GUID takes precedence over a document name if guid: - closest_embeddings_query = closest_embeddings_query.filter( - upload_file__guid=guid - ) + queryset = queryset.filter(upload_file__guid=guid) elif document_name: - closest_embeddings_query = closest_embeddings_query.filter(name=document_name) + queryset = queryset.filter(name=document_name) # Slicing is equivalent to SQL's LIMIT clause - closest_embeddings_query = closest_embeddings_query[:num_results] + return queryset[:num_results] + + +def format_results(queryset): + """ + Evaluate a QuerySet and return a list of result dicts. + + Parameters + ---------- + queryset : iterable + Iterable of Embeddings objects (or any objects with the expected attributes) + Returns + ------- + list[dict] + List of dicts with keys: name, text, page_number, chunk_number, distance, file_id + """ # Iterating evaluates the QuerySet and hits the database # TODO: Research improving the query evaluation performance - results = [ + return [ { "name": obj.name, "text": obj.text, @@ -93,13 +85,36 @@ def get_closest_embeddings( "distance": obj.distance, "file_id": obj.upload_file.guid if obj.upload_file else None, } - for obj in closest_embeddings_query + for obj in queryset ] - db_query_time = time.time() - db_query_start +def log_search_usage( + results, message_data, user, guid, document_name, num_results, encoding_time, db_query_time +): + """ + Create a SemanticSearchUsage record. Swallows exceptions so search isn't interrupted. + + Parameters + ---------- + results : list[dict] + The search results, each containing a "distance" key + message_data : str + The original search query text + user : User + The user who performed the search + guid : str or None + Document GUID filter used in the search + document_name : str or None + Document name filter used in the search + num_results : int + Number of results requested + encoding_time : float + Time in seconds to encode the query + db_query_time : float + Time in seconds for the database query + """ try: - # Handle user having no uploaded docs or doc filtering returning no matches if results: distances = [r["distance"] for r in results] SemanticSearchUsage.objects.create( @@ -113,11 +128,10 @@ def get_closest_embeddings( num_results_returned=len(results), max_distance=max(distances), median_distance=median(distances), - min_distance=min(distances) + min_distance=min(distances), ) else: logger.warning("Semantic search returned no results") - SemanticSearchUsage.objects.create( query_text=message_data, user=user if (user and user.is_authenticated) else None, @@ -129,9 +143,58 @@ def get_closest_embeddings( num_results_returned=0, max_distance=None, median_distance=None, - min_distance=None + min_distance=None, ) except Exception as e: logger.error(f"Failed to create semantic search usage database record: {e}") + +def get_closest_embeddings( + user, message_data, document_name=None, guid=None, num_results=10 +): + """ + Find the closest embeddings to a given message for a specific user. + + Parameters + ---------- + user : User + The user whose uploaded documents will be searched + message_data : str + The input message to find similar embeddings for + document_name : str, optional + Filter results to a specific document name + guid : str, optional + Filter results to a specific document GUID (takes precedence over document_name) + num_results : int, default 10 + Maximum number of results to return + + Returns + ------- + list[dict] + List of dictionaries containing embedding results with keys: + - name: document name + - text: embedded text content + - page_number: page number in source document + - chunk_number: chunk number within the document + - distance: L2 distance from query embedding + - file_id: GUID of the source file + + Notes + ----- + Creates a SemanticSearchUsage record. Swallows exceptions so search isn't interrupted. + """ + encoding_start = time.time() + model = TransformerModel.get_instance().model + embedding_vector = model.encode(message_data) + encoding_time = time.time() - encoding_start + + db_query_start = time.time() + queryset = build_query(user, embedding_vector, document_name, guid, num_results) + results = format_results(queryset) + db_query_time = time.time() - db_query_start + + log_search_usage( + results, message_data, user, guid, document_name, num_results, encoding_time, db_query_time + ) + return results From 3ffb74af318cc927d9c11ba37e63a3093d5ecfc6 Mon Sep 17 00:00:00 2001 From: Sahil D Shah Date: Fri, 13 Feb 2026 16:12:49 -0500 Subject: [PATCH 02/13] ADD Add infra required to run pytest --- .github/workflows/python-app.yml | 5 +++++ server/pytest.ini | 3 +++ server/requirements.txt | 4 +++- 3 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 server/pytest.ini diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index dcb7a4bb..41bc74d9 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -27,3 +27,8 @@ jobs: run: pipx install ruff - name: Lint code with Ruff run: ruff check --output-format=github --target-version=py39 + - name: Install test dependencies + run: pip install -r server/requirements.txt + # Discover and run all files matching test_*.py or *_test.py under server/ + - name: Run tests + run: pytest server/ -v diff --git a/server/pytest.ini b/server/pytest.ini new file mode 100644 index 00000000..235b9752 --- /dev/null +++ b/server/pytest.ini @@ -0,0 +1,3 @@ +[pytest] +DJANGO_SETTINGS_MODULE = balancer_backend.settings +pythonpath = . diff --git a/server/requirements.txt b/server/requirements.txt index bbaf7bc9..001708e9 100644 --- a/server/requirements.txt +++ b/server/requirements.txt @@ -18,4 +18,6 @@ sentence_transformers PyMuPDF==1.24.0 Pillow pytesseract -anthropic \ No newline at end of file +anthropic +pytest +pytest-django \ No newline at end of file From 12b09a733a1dc79b8fbec63d358d4354764a5116 Mon Sep 17 00:00:00 2001 From: Sahil D Shah Date: Fri, 13 Feb 2026 16:33:41 -0500 Subject: [PATCH 03/13] ADD Start adding tests for embedding_services" --- server/api/services/embedding_services.py | 8 +- .../api/services/test_embedding_services.py | 85 +++++++++++++++++++ 2 files changed, 89 insertions(+), 4 deletions(-) create mode 100644 server/api/services/test_embedding_services.py diff --git a/server/api/services/embedding_services.py b/server/api/services/embedding_services.py index 0720a9c8..3fa9bb68 100644 --- a/server/api/services/embedding_services.py +++ b/server/api/services/embedding_services.py @@ -60,7 +60,7 @@ def build_query(user, embedding_vector, document_name=None, guid=None, num_resul return queryset[:num_results] -def format_results(queryset): +def evaluate_query(queryset): """ Evaluate a QuerySet and return a list of result dicts. @@ -89,7 +89,7 @@ def format_results(queryset): ] -def log_search_usage( +def log_usage( results, message_data, user, guid, document_name, num_results, encoding_time, db_query_time ): """ @@ -190,10 +190,10 @@ def get_closest_embeddings( db_query_start = time.time() queryset = build_query(user, embedding_vector, document_name, guid, num_results) - results = format_results(queryset) + results = evaluate_query(queryset) db_query_time = time.time() - db_query_start - log_search_usage( + log_usage( results, message_data, user, guid, document_name, num_results, encoding_time, db_query_time ) diff --git a/server/api/services/test_embedding_services.py b/server/api/services/test_embedding_services.py new file mode 100644 index 00000000..677c1e7b --- /dev/null +++ b/server/api/services/test_embedding_services.py @@ -0,0 +1,85 @@ +from unittest.mock import MagicMock, patch + +from api.services.embedding_services import evaluate_query, log_usage + + +def test_evaluate_query_maps_fields(): + obj = MagicMock() + obj.name = "doc.pdf" + obj.text = "some text" + obj.page_num = 3 + obj.chunk_number = 1 + obj.distance = 0.42 + obj.upload_file.guid = "abc-123" + + results = evaluate_query([obj]) + + assert results == [ + { + "name": "doc.pdf", + "text": "some text", + "page_number": 3, + "chunk_number": 1, + "distance": 0.42, + "file_id": "abc-123", + } + ] + + +def test_evaluate_query_none_upload_file(): + obj = MagicMock() + obj.name = "doc.pdf" + obj.text = "some text" + obj.page_num = 1 + obj.chunk_number = 0 + obj.distance = 1.0 + obj.upload_file = None + + results = evaluate_query([obj]) + + assert results[0]["file_id"] is None + + +@patch("api.services.embedding_services.SemanticSearchUsage.objects.create") +def test_log_usage_computes_distance_stats(mock_create): + results = [{"distance": 1.0}, {"distance": 3.0}, {"distance": 2.0}] + user = MagicMock(is_authenticated=True) + + log_usage( + results, + message_data="test query", + user=user, + guid=None, + document_name=None, + num_results=10, + encoding_time=0.1, + db_query_time=0.2, + ) + + mock_create.assert_called_once() + kwargs = mock_create.call_args.kwargs + assert kwargs["min_distance"] == 1.0 + assert kwargs["max_distance"] == 3.0 + assert kwargs["median_distance"] == 2.0 + assert kwargs["num_results_returned"] == 3 + + +@patch( + "api.services.embedding_services.SemanticSearchUsage.objects.create", + side_effect=Exception("DB error"), +) +def test_log_usage_swallows_exceptions(mock_create): + results = [{"distance": 1.0}] + user = MagicMock(is_authenticated=True) + + # pytest fails the test if it catches unhandled Exception + log_usage( + results, + message_data="test query", + user=user, + guid=None, + document_name=None, + num_results=10, + encoding_time=0.1, + db_query_time=0.2, + ) From da9afaa905a43218e1f947360aa141662359e1aa Mon Sep 17 00:00:00 2001 From: Sahil D Shah Date: Tue, 17 Feb 2026 14:40:13 -0500 Subject: [PATCH 04/13] DOC Add a note about running pytest in the README --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index f1cea06b..15018d37 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,11 @@ df = pd.read_sql(query, engine) #### Django REST - The email and password are set in `server/api/management/commands/createsu.py` +- Backend tests can be run using `pytest` by running the below command inside the running backend container: + +``` +docker compose exec backend pytest api/ -v +``` ## Local Kubernetes Deployment From 5ce77823c25f0a45fbcdc828e13214b515496bc2 Mon Sep 17 00:00:00 2001 From: Sahil D Shah Date: Fri, 27 Feb 2026 16:09:05 -0500 Subject: [PATCH 05/13] Preload SentenceTransformer model at Django startup before traffic is routed to the application instance --- server/api/apps.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/api/apps.py b/server/api/apps.py index 66656fd2..4d502cba 100644 --- a/server/api/apps.py +++ b/server/api/apps.py @@ -4,3 +4,7 @@ class ApiConfig(AppConfig): default_auto_field = 'django.db.models.BigAutoField' name = 'api' + + def ready(self): + from .services.sentencetTransformer_model import TransformerModel + TransformerModel.get_instance() From 795f21885e766de9f6379319c5555ef7db48d989 Mon Sep 17 00:00:00 2001 From: Sahil D Shah Date: Wed, 11 Mar 2026 13:09:14 -0400 Subject: [PATCH 06/13] Run python-app workflow on pushes and PRs to develop branch --- .github/workflows/python-app.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index 41bc74d9..a6c07075 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -5,9 +5,9 @@ name: Python application on: push: - branches: [ "listOfMed" ] + branches: [ "develop" ] pull_request: - branches: [ "listOfMed" ] + branches: [ "develop" ] permissions: contents: read From d498a0057008b1b2c80bdb176e5940940899639b Mon Sep 17 00:00:00 2001 From: Sahil D Shah Date: Thu, 19 Mar 2026 13:15:47 -0400 Subject: [PATCH 07/13] =?UTF-8?q?Pytest=20won=E2=80=99t=20automatically=20?= =?UTF-8?q?discover=20config=20files=20in=20subdirectories?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/python-app.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index a6c07075..2afa2828 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -29,6 +29,6 @@ jobs: run: ruff check --output-format=github --target-version=py39 - name: Install test dependencies run: pip install -r server/requirements.txt - # Discover and run all files matching test_*.py or *_test.py under server/ + # Pytest won’t automatically discover config files in subdirectories - name: Run tests - run: pytest server/ -v + run: pytest -c server/pytest.ini server/ -v From 3824d81ae0fb722383f7e80df07ca7ba28d0c4e6 Mon Sep 17 00:00:00 2001 From: Sahil D Shah Date: Thu, 19 Mar 2026 13:47:52 -0400 Subject: [PATCH 08/13] Suppress E402 import violations --- evaluation/evals.py | 16 ++++++++-------- server/balancer_backend/urls.py | 6 +++--- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/evaluation/evals.py b/evaluation/evals.py index 8eb7e9e6..5110076f 100755 --- a/evaluation/evals.py +++ b/evaluation/evals.py @@ -21,18 +21,18 @@ # Ensure the parent directory is in the path to import ModelFactory sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))) -import argparse -import logging -import asyncio -import time +import argparse # noqa: E402 +import logging # noqa: E402 +import asyncio # noqa: E402 +import time # noqa: E402 -import pandas as pd +import pandas as pd # noqa: E402 # lighteval depends on `sentencepiece` and it only has prebuilt wheels for Python 3.11 or below -from lighteval.tasks.requests import Doc -from lighteval.metrics.metrics_sample import Extractiveness +from lighteval.tasks.requests import Doc # noqa: E402 +from lighteval.metrics.metrics_sample import Extractiveness # noqa: E402 -from server.api.services.llm_services import ModelFactory +from server.api.services.llm_services import ModelFactory # noqa: E402 logging.basicConfig( level=logging.INFO, format="%(asctime)s - %(levelname)s - %(message)s" diff --git a/server/balancer_backend/urls.py b/server/balancer_backend/urls.py index 55bd2032..cdb92dbb 100644 --- a/server/balancer_backend/urls.py +++ b/server/balancer_backend/urls.py @@ -58,9 +58,9 @@ path("api/redoc/", SpectacularRedocView.as_view(url_name="schema"), name="redoc"), ] -import os -from django.conf import settings -from django.http import HttpResponseNotFound +import os # noqa: E402 +from django.conf import settings # noqa: E402 +from django.http import HttpResponseNotFound # noqa: E402 def spa_fallback(request): From 46e9969dade55777098286cd6316bc18444e5b1f Mon Sep 17 00:00:00 2001 From: Sahil D Shah Date: Fri, 20 Mar 2026 12:00:44 -0400 Subject: [PATCH 09/13] Add build_query tests and document coverage gaps in embedding_services --- server/api/services/embedding_services.py | 1 + .../api/services/test_embedding_services.py | 202 +++++++++++++++++- 2 files changed, 201 insertions(+), 2 deletions(-) diff --git a/server/api/services/embedding_services.py b/server/api/services/embedding_services.py index 3fa9bb68..aca99133 100644 --- a/server/api/services/embedding_services.py +++ b/server/api/services/embedding_services.py @@ -2,6 +2,7 @@ import logging from statistics import median +# filter() only does ADD logic from django.db.models import Q from pgvector.django import L2Distance diff --git a/server/api/services/test_embedding_services.py b/server/api/services/test_embedding_services.py index 677c1e7b..ea322645 100644 --- a/server/api/services/test_embedding_services.py +++ b/server/api/services/test_embedding_services.py @@ -1,9 +1,175 @@ from unittest.mock import MagicMock, patch -from api.services.embedding_services import evaluate_query, log_usage +from django.db.models import Q +from api.services.embedding_services import build_query, evaluate_query, log_usage + +# --------------------------------------------------------------------------- +# build_query tests +# +# build_query only constructs a lazy Django QuerySet — it never evaluates it +# (no iteration, .get(), .exists(), etc.), so no database is needed. +# +# We patch Embeddings.objects so every chained ORM call (.filter, .annotate, +# .order_by, __getitem__) returns a MagicMock instead of hitting the DB. +# All assertions inspect which methods were called with which arguments. +# --------------------------------------------------------------------------- + +# Only forwarded to L2Distance +EMBEDDING_VECTOR = [0.1, 0.2, 0.3] + +# Test authenticated/unauthenticated user access control + +@patch("api.services.embedding_services.Embeddings.objects") +def test_build_query_authenticated_uses_or_filter(mock_objects): + # An authenticated user should see their own files OR files uploaded by a + # superuser. The initial filter must use an OR-connected Q expression. + user = MagicMock(is_authenticated=True) + + build_query(user, EMBEDDING_VECTOR) + + # Q objects support equality comparison in pure Python — no DB needed. + expected_q = Q(upload_file__uploaded_by=user) | Q(upload_file__uploaded_by__is_superuser=True) + actual_q = mock_objects.filter.call_args.args[0] + assert actual_q == expected_q + + +@patch("api.services.embedding_services.Embeddings.objects") +def test_build_query_unauthenticated_uses_superuser_only_filter(mock_objects): + # An unauthenticated user may only see files uploaded by superusers. + # The OR branch for the user's own files must NOT be present. + user = MagicMock(is_authenticated=False) + + build_query(user, EMBEDDING_VECTOR) + + expected_q = Q(upload_file__uploaded_by__is_superuser=True) + actual_q = mock_objects.filter.call_args.args[0] + assert actual_q == expected_q + +# Test application of annotate and order_by + +# TODO: Strengthen test_build_query_annotates_and_orders_by_distance to also +# assert the *arguments* to annotate — specifically that it receives +# distance=L2Distance("embedding_sentence_transformers", EMBEDDING_VECTOR). +# Currently only the call count is checked, so a wrong field name or a +# dropped vector would go undetected. + +@patch("api.services.embedding_services.Embeddings.objects") +def test_build_query_annotates_and_orders_by_distance(mock_objects): + # Regardless of other arguments, annotate(distance=L2Distance(...)) and + # order_by("distance") must always be applied to the queryset. + user = MagicMock(is_authenticated=True) + + build_query(user, EMBEDDING_VECTOR) + + # Retrieve the mock chain that .filter() returned, then check its methods. + filtered_qs = mock_objects.filter.return_value + filtered_qs.annotate.assert_called_once() + filtered_qs.annotate.return_value.order_by.assert_called_once_with("distance") + +# Test guid-over-document precedence logic + +@patch("api.services.embedding_services.Embeddings.objects") +def test_build_query_no_document_filter_when_both_none(mock_objects): + # When neither guid nor document_name is provided, only the access-control + # filter should fire — no secondary filter call for a document. + user = MagicMock(is_authenticated=True) + + build_query(user, EMBEDDING_VECTOR, document_name=None, guid=None) + + # Exactly one filter call: the auth/access-control filter. + assert mock_objects.filter.call_count == 1 + + + +@patch("api.services.embedding_services.Embeddings.objects") +def test_build_query_guid_takes_precedence_over_document_name(mock_objects): + # When both guid and document_name are provided, the guid branch runs and + # the document_name branch is skipped entirely (only two filter calls total). + user = MagicMock(is_authenticated=True) + + build_query(user, EMBEDDING_VECTOR, guid="abc-123", document_name="study.pdf") + + # Two calls: auth filter + guid filter. No third call for document_name. + assert mock_objects.filter.call_count == 2 + + # The second filter must use upload_file__guid, not name. + # We follow the mock chain to the queryset that .annotate().order_by() returned. + ordered_qs = mock_objects.filter.return_value.annotate.return_value.order_by.return_value + ordered_qs.filter.assert_called_once_with(upload_file__guid="abc-123") + + +@patch("api.services.embedding_services.Embeddings.objects") +def test_build_query_guid_filter_applied(mock_objects): + # When only guid is given, a second filter on upload_file__guid is applied. + user = MagicMock(is_authenticated=True) + + build_query(user, EMBEDDING_VECTOR, guid="doc-guid-456") + + ordered_qs = mock_objects.filter.return_value.annotate.return_value.order_by.return_value + ordered_qs.filter.assert_called_once_with(upload_file__guid="doc-guid-456") + + +@patch("api.services.embedding_services.Embeddings.objects") +def test_build_query_document_name_filter_applied(mock_objects): + # When only document_name is given (guid is None), a second filter on + # name is applied instead of upload_file__guid. + user = MagicMock(is_authenticated=True) + + build_query(user, EMBEDDING_VECTOR, document_name="study.pdf", guid=None) + + ordered_qs = mock_objects.filter.return_value.annotate.return_value.order_by.return_value + ordered_qs.filter.assert_called_once_with(name="study.pdf") + + +@patch("api.services.embedding_services.Embeddings.objects") +def test_build_query_empty_string_guid_falls_back_to_document_name(mock_objects): + # An empty-string guid is falsy in Python, so it should not trigger the + # guid branch. The document_name filter should fire instead. This guards + # against callers passing guid="" from an unset form field. + user = MagicMock(is_authenticated=True) + + build_query(user, EMBEDDING_VECTOR, guid="", document_name="fallback.pdf") + + ordered_qs = mock_objects.filter.return_value.annotate.return_value.order_by.return_value + ordered_qs.filter.assert_called_once_with(name="fallback.pdf") + +# Cover LIMIT slicing + +@patch("api.services.embedding_services.Embeddings.objects") +def test_build_query_respects_num_results(mock_objects): + # num_results controls the SQL LIMIT via queryset slicing. Verify that a + # non-default value propagates correctly to the __getitem__ call. + user = MagicMock(is_authenticated=True) + + build_query(user, EMBEDDING_VECTOR, num_results=5) + + # Django translates qs[:5] into qs.__getitem__(slice(None, 5, None)). + ordered_qs = mock_objects.filter.return_value.annotate.return_value.order_by.return_value + ordered_qs.__getitem__.assert_called_once_with(slice(None, 5, None)) + +@patch("api.services.embedding_services.Embeddings.objects") +def test_build_query_returns_unevaluated_queryset(mock_objects): + # build_query must NOT evaluate the queryset (no list(), no iteration). + # The return value should be the mock produced by the final __getitem__ call. + user = MagicMock(is_authenticated=True) + + result = build_query(user, EMBEDDING_VECTOR) + + ordered_qs = mock_objects.filter.return_value.annotate.return_value.order_by.return_value + assert result is ordered_qs.__getitem__.return_value + assert not isinstance(result, list) + + +# --------------------------------------------------------------------------- +# evaluate_query tests +# --------------------------------------------------------------------------- + +# TODO: Add test for empty queryset — evaluate_query([]) should return []. def test_evaluate_query_maps_fields(): + # Verify that each Embeddings model attribute is mapped to the correct + # output dict key. Note the rename: obj.page_num -> result["page_number"]. obj = MagicMock() obj.name = "doc.pdf" obj.text = "some text" @@ -27,6 +193,8 @@ def test_evaluate_query_maps_fields(): def test_evaluate_query_none_upload_file(): + # When upload_file is None (e.g. the FK was deleted), file_id must be None + # rather than raising an AttributeError on None.guid. obj = MagicMock() obj.name = "doc.pdf" obj.text = "some text" @@ -39,9 +207,26 @@ def test_evaluate_query_none_upload_file(): assert results[0]["file_id"] is None +# --------------------------------------------------------------------------- +# log_usage tests +# --------------------------------------------------------------------------- + +# TODO: Add test for empty results list — log_usage([]) hits the else branch and +# should call SemanticSearchUsage.objects.create with num_results_returned=0 +# and max_distance=None, median_distance=None, min_distance=None. + +# TODO: Add test for unauthenticated user — user.is_authenticated=False should +# result in user=None being stored in the SemanticSearchUsage record. + +# TODO: Add test for user=None — passing None directly as the user argument +# should also store user=None (the expression `user if (user and +# user.is_authenticated) else None` handles both cases, but only the +# authenticated path is currently exercised). @patch("api.services.embedding_services.SemanticSearchUsage.objects.create") def test_log_usage_computes_distance_stats(mock_create): + # Verify min, max, and median are computed correctly from the distance + # values in the results list and forwarded to the DB record. results = [{"distance": 1.0}, {"distance": 3.0}, {"distance": 2.0}] user = MagicMock(is_authenticated=True) @@ -69,10 +254,12 @@ def test_log_usage_computes_distance_stats(mock_create): side_effect=Exception("DB error"), ) def test_log_usage_swallows_exceptions(mock_create): + # log_usage must not propagate exceptions — a logging failure should never + # interrupt the caller's search flow. + # pytest fails the test if it catches unhandled Exception results = [{"distance": 1.0}] user = MagicMock(is_authenticated=True) - # pytest fails the test if it catches unhandled Exception log_usage( results, message_data="test query", @@ -83,3 +270,14 @@ def test_log_usage_swallows_exceptions(mock_create): encoding_time=0.1, db_query_time=0.2, ) + + +# --------------------------------------------------------------------------- +# get_closest_embeddings tests +# --------------------------------------------------------------------------- + +# TODO: Add smoke test for get_closest_embeddings verifying the wiring between +# its three steps: encode → build_query → evaluate_query → log_usage. +# Patch TransformerModel.get_instance, build_query, evaluate_query, and +# log_usage. Assert that evaluate_query receives the queryset returned by +# build_query, and that the function returns evaluate_query's result. From 64a19ef56dad988d1a2ea61a0b409c7878d0370c Mon Sep 17 00:00:00 2001 From: Sahil D Shah Date: Fri, 20 Mar 2026 12:39:51 -0400 Subject: [PATCH 10/13] Fill test gaps in test_embedding_services --- server/api/services/embedding_services.py | 2 +- .../api/services/test_embedding_services.py | 149 ++++++++++++++---- 2 files changed, 119 insertions(+), 32 deletions(-) diff --git a/server/api/services/embedding_services.py b/server/api/services/embedding_services.py index aca99133..dada28a2 100644 --- a/server/api/services/embedding_services.py +++ b/server/api/services/embedding_services.py @@ -2,7 +2,7 @@ import logging from statistics import median -# filter() only does ADD logic +# Django filter() only does ADD logic from django.db.models import Q from pgvector.django import L2Distance diff --git a/server/api/services/test_embedding_services.py b/server/api/services/test_embedding_services.py index ea322645..8cbc1be9 100644 --- a/server/api/services/test_embedding_services.py +++ b/server/api/services/test_embedding_services.py @@ -1,20 +1,21 @@ from unittest.mock import MagicMock, patch from django.db.models import Q +from pgvector.django import L2Distance -from api.services.embedding_services import build_query, evaluate_query, log_usage +from api.services.embedding_services import ( + build_query, + evaluate_query, + get_closest_embeddings, + log_usage, +) # --------------------------------------------------------------------------- # build_query tests -# -# build_query only constructs a lazy Django QuerySet — it never evaluates it -# (no iteration, .get(), .exists(), etc.), so no database is needed. -# -# We patch Embeddings.objects so every chained ORM call (.filter, .annotate, -# .order_by, __getitem__) returns a MagicMock instead of hitting the DB. -# All assertions inspect which methods were called with which arguments. # --------------------------------------------------------------------------- +# All assertions inspect which methods and arguments were called on Embeddings.objects + # Only forwarded to L2Distance EMBEDDING_VECTOR = [0.1, 0.2, 0.3] @@ -48,12 +49,6 @@ def test_build_query_unauthenticated_uses_superuser_only_filter(mock_objects): # Test application of annotate and order_by -# TODO: Strengthen test_build_query_annotates_and_orders_by_distance to also -# assert the *arguments* to annotate — specifically that it receives -# distance=L2Distance("embedding_sentence_transformers", EMBEDDING_VECTOR). -# Currently only the call count is checked, so a wrong field name or a -# dropped vector would go undetected. - @patch("api.services.embedding_services.Embeddings.objects") def test_build_query_annotates_and_orders_by_distance(mock_objects): # Regardless of other arguments, annotate(distance=L2Distance(...)) and @@ -67,6 +62,12 @@ def test_build_query_annotates_and_orders_by_distance(mock_objects): filtered_qs.annotate.assert_called_once() filtered_qs.annotate.return_value.order_by.assert_called_once_with("distance") + # L2Distance is a Django Func subclass, which implements __eq__ by comparing + # class and source expressions — so we can assert the exact field name and + # vector without patching L2Distance itself. + actual_distance_expr = filtered_qs.annotate.call_args.kwargs["distance"] + assert actual_distance_expr == L2Distance("embedding_sentence_transformers", EMBEDDING_VECTOR) + # Test guid-over-document precedence logic @patch("api.services.embedding_services.Embeddings.objects") @@ -165,7 +166,10 @@ def test_build_query_returns_unevaluated_queryset(mock_objects): # evaluate_query tests # --------------------------------------------------------------------------- -# TODO: Add test for empty queryset — evaluate_query([]) should return []. +def test_evaluate_query_empty_queryset(): + # An empty iterable should return an empty list, not raise an exception. + assert evaluate_query([]) == [] + def test_evaluate_query_maps_fields(): # Verify that each Embeddings model attribute is mapped to the correct @@ -193,8 +197,8 @@ def test_evaluate_query_maps_fields(): def test_evaluate_query_none_upload_file(): - # When upload_file is None (e.g. the FK was deleted), file_id must be None - # rather than raising an AttributeError on None.guid. + # When upload_file is None, file_id must be None rather than raising + # an AttributeError on None.guid. obj = MagicMock() obj.name = "doc.pdf" obj.text = "some text" @@ -211,17 +215,71 @@ def test_evaluate_query_none_upload_file(): # log_usage tests # --------------------------------------------------------------------------- -# TODO: Add test for empty results list — log_usage([]) hits the else branch and -# should call SemanticSearchUsage.objects.create with num_results_returned=0 -# and max_distance=None, median_distance=None, min_distance=None. +@patch("api.services.embedding_services.SemanticSearchUsage.objects.create") +def test_log_usage_empty_results(mock_create): + # Empty results hits the else branch. The record should still be created + # with num_results_returned=0 and all distance fields set to None. + user = MagicMock(is_authenticated=True) + + log_usage( + [], + message_data="test query", + user=user, + guid=None, + document_name=None, + num_results=10, + encoding_time=0.1, + db_query_time=0.2, + ) + + mock_create.assert_called_once() + kwargs = mock_create.call_args.kwargs + assert kwargs["num_results_returned"] == 0 + assert kwargs["max_distance"] is None + assert kwargs["median_distance"] is None + assert kwargs["min_distance"] is None + + +@patch("api.services.embedding_services.SemanticSearchUsage.objects.create") +def test_log_usage_unauthenticated_user_stored_as_none(mock_create): + # An unauthenticated user should be stored as None in the DB record, not as + # the user object itself, so the FK constraint is not violated. + user = MagicMock(is_authenticated=False) + + log_usage( + [{"distance": 1.0}], + message_data="test query", + user=user, + guid=None, + document_name=None, + num_results=10, + encoding_time=0.1, + db_query_time=0.2, + ) + + kwargs = mock_create.call_args.kwargs + assert kwargs["user"] is None + + +@patch("api.services.embedding_services.SemanticSearchUsage.objects.create") +def test_log_usage_none_user_stored_as_none(mock_create): + # Passing user=None directly (e.g. from an anonymous request) should also + # store None — the expression `user if (user and user.is_authenticated)` + # short-circuits on the falsy None before accessing .is_authenticated. + log_usage( + [{"distance": 1.0}], + message_data="test query", + user=None, + guid=None, + document_name=None, + num_results=10, + encoding_time=0.1, + db_query_time=0.2, + ) -# TODO: Add test for unauthenticated user — user.is_authenticated=False should -# result in user=None being stored in the SemanticSearchUsage record. + kwargs = mock_create.call_args.kwargs + assert kwargs["user"] is None -# TODO: Add test for user=None — passing None directly as the user argument -# should also store user=None (the expression `user if (user and -# user.is_authenticated) else None` handles both cases, but only the -# authenticated path is currently exercised). @patch("api.services.embedding_services.SemanticSearchUsage.objects.create") def test_log_usage_computes_distance_stats(mock_create): @@ -276,8 +334,37 @@ def test_log_usage_swallows_exceptions(mock_create): # get_closest_embeddings tests # --------------------------------------------------------------------------- -# TODO: Add smoke test for get_closest_embeddings verifying the wiring between -# its three steps: encode → build_query → evaluate_query → log_usage. -# Patch TransformerModel.get_instance, build_query, evaluate_query, and -# log_usage. Assert that evaluate_query receives the queryset returned by -# build_query, and that the function returns evaluate_query's result. +@patch("api.services.embedding_services.log_usage") +@patch("api.services.embedding_services.evaluate_query") +@patch("api.services.embedding_services.build_query") +@patch("api.services.embedding_services.TransformerModel") +def test_get_closest_embeddings_wiring(mock_transformer, mock_build, mock_evaluate, mock_log): + # Smoke test verifying that get_closest_embeddings correctly wires together + # encode → build_query → evaluate_query → log_usage and returns the results. + user = MagicMock(is_authenticated=True) + + # Simulate the model encoding the message to a vector. + fake_vector = [0.1, 0.2, 0.3] + mock_transformer.get_instance.return_value.model.encode.return_value = fake_vector + + # build_query returns a queryset; evaluate_query turns it into a results list. + fake_queryset = MagicMock() + mock_build.return_value = fake_queryset + fake_results = [{"name": "doc.pdf", "distance": 0.5}] + mock_evaluate.return_value = fake_results + + result = get_closest_embeddings(user, "some query", document_name="doc.pdf", guid=None, num_results=5) + + # The encoded vector must be forwarded to build_query. + mock_build.assert_called_once_with(user, fake_vector, "doc.pdf", None, 5) + + # evaluate_query must receive the queryset that build_query returned. + mock_evaluate.assert_called_once_with(fake_queryset) + + # log_usage must be called with the results and original parameters. + mock_log.assert_called_once() + log_kwargs = mock_log.call_args.args + assert log_kwargs[0] is fake_results + + # The function must return evaluate_query's result unchanged. + assert result is fake_results From dec3c12a71c1fefc81f30768fe2aec8e48df2fb8 Mon Sep 17 00:00:00 2001 From: Sahil D Shah Date: Fri, 20 Mar 2026 14:05:17 -0400 Subject: [PATCH 11/13] Fix incorrect build_query test assertions --- .../api/services/test_embedding_services.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/server/api/services/test_embedding_services.py b/server/api/services/test_embedding_services.py index 8cbc1be9..dcbb2fc7 100644 --- a/server/api/services/test_embedding_services.py +++ b/server/api/services/test_embedding_services.py @@ -38,14 +38,13 @@ def test_build_query_authenticated_uses_or_filter(mock_objects): @patch("api.services.embedding_services.Embeddings.objects") def test_build_query_unauthenticated_uses_superuser_only_filter(mock_objects): # An unauthenticated user may only see files uploaded by superusers. - # The OR branch for the user's own files must NOT be present. + # The source uses a plain kwarg here (not a positional Q object), so the + # value lives in call_args.kwargs, not call_args.args. user = MagicMock(is_authenticated=False) build_query(user, EMBEDDING_VECTOR) - expected_q = Q(upload_file__uploaded_by__is_superuser=True) - actual_q = mock_objects.filter.call_args.args[0] - assert actual_q == expected_q + assert mock_objects.filter.call_args.kwargs == {"upload_file__uploaded_by__is_superuser": True} # Test application of annotate and order_by @@ -86,16 +85,18 @@ def test_build_query_no_document_filter_when_both_none(mock_objects): @patch("api.services.embedding_services.Embeddings.objects") def test_build_query_guid_takes_precedence_over_document_name(mock_objects): # When both guid and document_name are provided, the guid branch runs and - # the document_name branch is skipped entirely (only two filter calls total). + # the document_name branch is skipped entirely. user = MagicMock(is_authenticated=True) build_query(user, EMBEDDING_VECTOR, guid="abc-123", document_name="study.pdf") - # Two calls: auth filter + guid filter. No third call for document_name. - assert mock_objects.filter.call_count == 2 + # The auth filter fires on mock_objects.filter (call_count == 1). + # The document filter fires on the chained ordered_qs.filter — a different + # mock object — so mock_objects.filter.call_count stays at 1. + assert mock_objects.filter.call_count == 1 - # The second filter must use upload_file__guid, not name. - # We follow the mock chain to the queryset that .annotate().order_by() returned. + # The document filter must use upload_file__guid, not name, and must be + # called exactly once (confirming document_name branch was skipped). ordered_qs = mock_objects.filter.return_value.annotate.return_value.order_by.return_value ordered_qs.filter.assert_called_once_with(upload_file__guid="abc-123") From f9e890a21a3c716fe2bcfb17b8dfef92fcebb905 Mon Sep 17 00:00:00 2001 From: Sahil D Shah Date: Mon, 23 Mar 2026 14:43:39 -0400 Subject: [PATCH 12/13] Guard TransformerModel preload to runserver processes only --- server/api/apps.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/server/api/apps.py b/server/api/apps.py index 4d502cba..d8b9eaa7 100644 --- a/server/api/apps.py +++ b/server/api/apps.py @@ -6,5 +6,28 @@ class ApiConfig(AppConfig): name = 'api' def ready(self): + import os + import sys + + # ready() runs in every Django process: migrate, test, shell, runserver, etc. + # Only preload the model when we're actually going to serve requests. + # Dev (docker-compose.yml) runs `manage.py runserver 0.0.0.0:8000`. + # Prod (Dockerfile.prod CMD) runs `manage.py runserver 0.0.0.0:8000 --noreload`. + # entrypoint.prod.sh also runs migrate, createsu, and populatedb before exec'ing + # runserver — the guard below correctly skips model loading for those commands too. + if sys.argv[1:2] != ['runserver']: + return + + # Dev's autoreloader spawns two processes: a parent file-watcher and a child + # server. ready() runs in both, but only the child (RUN_MAIN=true) serves + # requests. Skip the parent to avoid loading the model twice on each file change. + # Prod uses --noreload so RUN_MAIN is never set; 'noreload' in sys.argv handles that case. + if os.environ.get('RUN_MAIN') != 'true' and '--noreload' not in sys.argv: + return + + # Note: paraphrase-MiniLM-L6-v2 (~80MB) is downloaded from HuggingFace on first + # use and cached to ~/.cache/torch/sentence_transformers/ inside the container. + # That cache is ephemeral — every container rebuild re-downloads the model unless + # a volume is mounted at that path. from .services.sentencetTransformer_model import TransformerModel TransformerModel.get_instance() From 67176a8541be4d2862d213783873e1651f6fe761 Mon Sep 17 00:00:00 2001 From: Sahil D Shah Date: Wed, 25 Mar 2026 16:42:20 -0400 Subject: [PATCH 13/13] Revert GitHub Workflow changes --- .github/workflows/python-app.yml | 9 ++------- evaluation/evals.py | 16 ++++++++-------- server/balancer_backend/urls.py | 6 +++--- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index 2afa2828..dcb7a4bb 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -5,9 +5,9 @@ name: Python application on: push: - branches: [ "develop" ] + branches: [ "listOfMed" ] pull_request: - branches: [ "develop" ] + branches: [ "listOfMed" ] permissions: contents: read @@ -27,8 +27,3 @@ jobs: run: pipx install ruff - name: Lint code with Ruff run: ruff check --output-format=github --target-version=py39 - - name: Install test dependencies - run: pip install -r server/requirements.txt - # Pytest won’t automatically discover config files in subdirectories - - name: Run tests - run: pytest -c server/pytest.ini server/ -v diff --git a/evaluation/evals.py b/evaluation/evals.py index 5110076f..8eb7e9e6 100755 --- a/evaluation/evals.py +++ b/evaluation/evals.py @@ -21,18 +21,18 @@ # Ensure the parent directory is in the path to import ModelFactory sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))) -import argparse # noqa: E402 -import logging # noqa: E402 -import asyncio # noqa: E402 -import time # noqa: E402 +import argparse +import logging +import asyncio +import time -import pandas as pd # noqa: E402 +import pandas as pd # lighteval depends on `sentencepiece` and it only has prebuilt wheels for Python 3.11 or below -from lighteval.tasks.requests import Doc # noqa: E402 -from lighteval.metrics.metrics_sample import Extractiveness # noqa: E402 +from lighteval.tasks.requests import Doc +from lighteval.metrics.metrics_sample import Extractiveness -from server.api.services.llm_services import ModelFactory # noqa: E402 +from server.api.services.llm_services import ModelFactory logging.basicConfig( level=logging.INFO, format="%(asctime)s - %(levelname)s - %(message)s" diff --git a/server/balancer_backend/urls.py b/server/balancer_backend/urls.py index cdb92dbb..55bd2032 100644 --- a/server/balancer_backend/urls.py +++ b/server/balancer_backend/urls.py @@ -58,9 +58,9 @@ path("api/redoc/", SpectacularRedocView.as_view(url_name="schema"), name="redoc"), ] -import os # noqa: E402 -from django.conf import settings # noqa: E402 -from django.http import HttpResponseNotFound # noqa: E402 +import os +from django.conf import settings +from django.http import HttpResponseNotFound def spa_fallback(request):