Address PR #1379 review: import consistency, exists tradeoff, partial deletion test

- Move ``DocumentPath`` into the same method-local import statement as
  ``Document`` (the existing pattern for documents.models inside
  ``_build_base_queryset``). Reconciles the inconsistency flagged in
  review where ``DocumentPath`` was the only inline-deeper import after
  ``StructuralAnnotationSet`` was promoted module-level.

- Document the ``active_doc_ids_qs.exists()`` trade-off: one extra
  boolean SELECT on the happy path buys an early
  ``Annotation.objects.none()`` short-circuit for empty/all-deleted
  corpora and preserves the operational warning that the previous
  list-materialisation provided.

- Add ``test_deletion_aware_path_partial_deletion_keeps_active_drops_deleted``
  to cover the actual subquery-filter branch (one active doc + one
  deleted doc in the same corpus). The existing
  ``test_deletion_aware_path_excludes_deleted_document_structural`` only
  exercises the early-exit branch; this test exercises the
  ``Q(document_id__in=...) | Q(structural=True, structural_set_id__in=...)``
  filter — the case most likely to regress silently if the OR clause is
  rewritten.

Author: claude

opencontractserver/llms/vector_stores/core_vector_stores.py

@@ -208,7 +208,7 @@ async def _build_base_queryset(self) -> QuerySet[Annotation]:
         # enumeration attacks.
         # -------------------------------------------------------------------------
         from opencontractserver.corpuses.models import Corpus
-        from opencontractserver.documents.models import Document
+        from opencontractserver.documents.models import Document, DocumentPath
 
         user = None
         if self.user_id:
@@ -289,8 +289,6 @@ async def _build_base_queryset(self) -> QuerySet[Annotation]:
 
         # Check for deleted documents in corpus
         if self.check_corpus_deletion and self.corpus_id and not self.document_id:
-            from opencontractserver.documents.models import DocumentPath
-
             # Lazy subquery — never round-trips through Python, so the
             # generated SQL stays a single statement with a real subquery
             # rather than a giant ``IN (val, val, ...)`` literal even for
@@ -302,6 +300,15 @@ async def _build_base_queryset(self) -> QuerySet[Annotation]:
                 .values("document_id")
                 .distinct()
             )
+            # Trade-off: this ``EXISTS`` adds one extra round-trip on the
+            # happy path, but lets us short-circuit the entire vector search
+            # for empty/all-deleted corpora (returning ``Annotation.none()``
+            # spares a downstream HNSW probe and keeps the existing
+            # operational warning). For corpora with at least one active
+            # document the cost is a single boolean SELECT and is dwarfed
+            # by the main query. Removing the check would also remove the
+            # debug log of the active-doc count that the materialised list
+            # used to provide.
             has_active_docs = await sync_to_async(active_doc_ids_qs.exists)()
 
             if has_active_docs:

opencontractserver/tests/test_corpus_isolation_vector_store.py

@@ -294,6 +294,86 @@ def test_deletion_aware_path_excludes_deleted_document_structural(self) -> None:
             "leaked through the deletion-aware corpus-only path",
         )
 
+    def test_deletion_aware_path_partial_deletion_keeps_active_drops_deleted(
+        self,
+    ) -> None:
+        """Partial deletion: with two docs in one corpus, only the active stays.
+
+        The previous test stresses the early-exit branch (every doc deleted
+        → ``has_active_docs`` is False → ``Annotation.none()``). This test
+        stresses the actual subquery filter:
+
+            active_filters &= Q(document_id__in=active_doc_ids_qs) | Q(
+                structural=True, structural_set_id__in=active_struct_set_ids
+            )
+
+        With one active document and one deleted document in the same
+        corpus, ``active_doc_ids_qs`` is non-empty but partial. The
+        deleted document's structural annotation must be excluded while
+        the active document's must come through. This is the scenario
+        most likely to regress silently if the OR clause is rebuilt or
+        the subquery is rewritten incorrectly.
+        """
+        # Add a second document to corpus_a with its own structural set,
+        # then mark its DocumentPath deleted while doc_a's stays active.
+        deleted_set = StructuralAnnotationSet.objects.create(
+            content_hash=hashlib.sha256(b"corpus-a-deleted-doc").hexdigest(),
+            creator=self.user,
+            parser_name="TestParser",
+            parser_version="1.0",
+        )
+        deleted_doc = Document.objects.create(
+            title="Doc A2 (deleted)",
+            creator=self.user,
+            is_public=True,
+            structural_annotation_set=deleted_set,
+        )
+        DocumentPath.objects.create(
+            document=deleted_doc,
+            corpus=self.corpus_a,
+            path="/doc_a2.pdf",
+            version_number=1,
+            is_current=True,
+            is_deleted=True,
+            creator=self.user,
+        )
+        deleted_struct = Annotation.objects.create(
+            structural_set=deleted_set,
+            annotation_label=self.label,
+            creator=self.user,
+            raw_text="Deleted-doc structural paragraph",
+            structural=True,
+            page=1,
+        )
+        deleted_struct.add_embedding(
+            get_default_embedder_path(), _constant_vector(384, 0.15)
+        )
+
+        store = CoreAnnotationVectorStore(
+            user_id=self.user.id,
+            corpus_id=self.corpus_a.id,
+            document_id=None,
+            check_corpus_deletion=True,
+        )
+        query = VectorSearchQuery(
+            query_embedding=_constant_vector(384, 0.5), similarity_top_k=10
+        )
+        results = store.search(query)
+        returned_ids = {r.annotation.id for r in results}
+
+        self.assertIn(
+            self.struct_a.id,
+            returned_ids,
+            "Active document's structural annotation was excluded by the "
+            "partial-deletion subquery filter",
+        )
+        self.assertNotIn(
+            deleted_struct.id,
+            returned_ids,
+            "Deleted document's structural annotation leaked through the "
+            "partial-deletion subquery filter",
+        )
+
     def test_document_scoped_search_still_returns_structural(self) -> None:
         """Document-scoped retrieval is the existing well-tested path.