fix(search): address PR #834 review findings

Resolve the latest PR-review comment on the verify-on-read tag-gate work:

- tests: stringify note IDs in test_verify_on_read.py so SearchResult.id
  matches production (scanner stringifies all IDs on write) — helper and the
  keeps/deleted/mixed/dedupe assertions (blocking).
- tests: make the unshared-file negative control a PDF so the drop is
  unambiguously "unshared", not a mime_type_filter mismatch.
- config: add Validator("VECTOR_SYNC_PDF_TAG", len_min=1) — an empty tag name
  would make find_files_by_tag("") misbehave in the verifier and scanner.
- verification: correct the _verify_files comment — two batch fetches (tag
  REPORT + EXCLUDED_TAGS lookup) are held under one semaphore slot; the
  pure-Python intersection runs outside it.
- tests: de-duplicate the minimal-PDF constant into a shared PDF_BYTES in
  tests/integration/conftest.py, imported by both integration modules.

Verified: ruff/format/ty/unit all green; the two integration modules
(10 tests) pass against a local Nextcloud (app-only, no MCP profile needed).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

nextcloud_mcp_server/config.py

@@ -259,6 +259,8 @@ _dynaconf = Dynaconf(
         Validator("DOCUMENT_CHUNK_SIZE", gte=1),
         # Non-negative
         Validator("DOCUMENT_CHUNK_OVERLAP", gte=0),
+        # Non-empty strings
+        Validator("VECTOR_SYNC_PDF_TAG", len_min=1),
         # Enum constraints
         Validator("LOG_FORMAT", is_in=["text", "json"]),
         Validator(

nextcloud_mcp_server/search/verification.py

@@ -174,10 +174,14 @@ async def _verify_files(
 
     tag_name = get_settings().vector_sync_pdf_tag
 
-    # One batch fetch per search, holding a single semaphore slot (same
+    # Two batch fetches per search held under a single semaphore slot (same
     # backpressure rationale as _verify_news_items): the tagged-file REPORT
-    # plus optional Depth:infinity folder expansion — and the EXCLUDED_TAGS
+    # (plus optional Depth:infinity folder expansion) and the EXCLUDED_TAGS
-    # lookup — are one round-trip set, not one per result.
+    # lookup (get_excluded_file_paths, itself ~2×N concurrent calls). Both are
+    # batched once per search, not once per result, and the slot bounds them.
+    # The pure-Python intersection that builds tagged_ids/accessible runs
+    # *outside* the slot — it needs no Nextcloud round-trip (mirrors the
+    # post-fetch present_ids build in _verify_news_items).
     #
     # TODO(perf): if folder expansion dominates query latency, cache the
     # tagged-id set per user with a short TTL (mirroring the

tests/integration/conftest.py

@@ -13,6 +13,19 @@ logger = logging.getLogger(__name__)
 # Valid provider names
 VALID_PROVIDERS = ["openai", "ollama", "anthropic", "bedrock"]
 
+# Canonical minimal valid PDF for integration tests. verify-on-read gates file
+# results on the vector-index tag via
+# find_files_by_tag(..., mime_type_filter="application/pdf"), so file fixtures
+# must be PDFs (matching what the scanner indexes), not .txt. Shared here so the
+# constant is defined once rather than drifting across test modules.
+PDF_BYTES = (
+    b"%PDF-1.4\n"
+    b"1 0 obj<</Type/Catalog/Pages 2 0 R>>endobj\n"
+    b"2 0 obj<</Type/Pages/Kids[3 0 R]/Count 1>>endobj\n"
+    b"3 0 obj<</Type/Page/Parent 2 0 R/MediaBox[0 0 200 200]>>endobj\n"
+    b"trailer<</Root 1 0 R>>\n%%EOF\n"
+)
+
 
 def pytest_addoption(parser):
     """Add --provider command line option for RAG tests."""

tests/integration/test_acl_shared_search.py

@@ -40,6 +40,7 @@ from nextcloud_mcp_server.search.access_filter import (
 from nextcloud_mcp_server.search.context import get_chunk_with_context
 from nextcloud_mcp_server.search.semantic import SemanticSearchAlgorithm
 from nextcloud_mcp_server.search.verification import verify_search_results
+from tests.integration.conftest import PDF_BYTES
 
 pytestmark = pytest.mark.integration
 
@@ -54,16 +55,6 @@ def _reset_owners_cache():
 
 
 _DOC_TEXT = "Confidential quarterly infrastructure budget and capacity plan"
-# Minimal valid PDF. verify-on-read gates file results on the vector-index tag
-# via find_files_by_tag(..., mime_type_filter="application/pdf"), so the shared
-# file must be a PDF (matching what the scanner actually indexes), not a .txt.
-_PDF_BYTES = (
-    b"%PDF-1.4\n"
-    b"1 0 obj<</Type/Catalog/Pages 2 0 R>>endobj\n"
-    b"2 0 obj<</Type/Pages/Kids[3 0 R]/Count 1>>endobj\n"
-    b"3 0 obj<</Type/Page/Parent 2 0 R/MediaBox[0 0 200 200]>>endobj\n"
-    b"trailer<</Root 1 0 R>>\n%%EOF\n"
-)
 
 
 def _user_client(username: str, password: str) -> NextcloudClient:
@@ -109,7 +100,7 @@ async def shared_file(acl_users):
 
     await alice.webdav.create_directory(test_dir)
     await alice.webdav.create_directory(nested)
-    await alice.webdav.write_file(path, _PDF_BYTES, "application/pdf")
+    await alice.webdav.write_file(path, PDF_BYTES, "application/pdf")
     file_id = (await alice.webdav.get_file_info(path))["id"]
     tag = await alice.webdav.get_or_create_tag(
         name=get_settings().vector_sync_pdf_tag,

tests/integration/test_verify_on_read.py

@@ -34,26 +34,16 @@ from nextcloud_mcp_server.config import get_settings
 from nextcloud_mcp_server.search import verification
 from nextcloud_mcp_server.search.algorithms import SearchResult
 from nextcloud_mcp_server.search.verification import verify_search_results
+from tests.integration.conftest import PDF_BYTES
 
 logger = logging.getLogger(__name__)
 
 pytestmark = pytest.mark.integration
 
-# Minimal valid PDF — the file verifier gates on the vector-index tag via
-# find_files_by_tag(..., mime_type_filter="application/pdf"), so file fixtures
-# must be PDFs (matching what the scanner indexes), not .txt.
-_PDF_BYTES = (
-    b"%PDF-1.4\n"
-    b"1 0 obj<</Type/Catalog/Pages 2 0 R>>endobj\n"
-    b"2 0 obj<</Type/Pages/Kids[3 0 R]/Count 1>>endobj\n"
-    b"3 0 obj<</Type/Page/Parent 2 0 R/MediaBox[0 0 200 200]>>endobj\n"
-    b"trailer<</Root 1 0 R>>\n%%EOF\n"
-)
-
 
 def _result_for_note(note_id: int) -> SearchResult:
     return SearchResult(
-        id=note_id,
+        id=str(note_id),
         doc_type="note",
         title=f"note_{note_id}",
         excerpt="...",
@@ -96,7 +86,7 @@ async def test_verify_keeps_accessible_note(
 
     kept, dropped_count = await verify_search_results(nc_client, results)
 
-    assert [r.id for r in kept] == [note_id]
+    assert [r.id for r in kept] == [str(note_id)]
     assert dropped_count == 0
     spy_evict.assert_not_awaited()
 
@@ -139,7 +129,7 @@ async def test_verify_drops_deleted_note_and_schedules_eviction(
 
     assert kept == [], "deleted note must not pass verification"
     assert dropped_count == 1
-    spy_evict.assert_awaited_once_with(note_id, "note", nc_client.username)
+    spy_evict.assert_awaited_once_with(str(note_id), "note", nc_client.username)
 
 
 async def test_verify_mixed_accessible_and_deleted(
@@ -168,9 +158,9 @@ async def test_verify_mixed_accessible_and_deleted(
     ]
     kept, dropped_count = await verify_search_results(nc_client, results)
 
-    assert [r.id for r in kept] == [accessible_id]
+    assert [r.id for r in kept] == [str(accessible_id)]
     assert dropped_count == 1
-    spy_evict.assert_awaited_once_with(ghost_id, "note", nc_client.username)
+    spy_evict.assert_awaited_once_with(str(ghost_id), "note", nc_client.username)
 
 
 async def test_verify_dedupes_chunks_of_same_document(
@@ -189,7 +179,7 @@ async def test_verify_dedupes_chunks_of_same_document(
     # Three chunks of the same note (chunk_index varies)
     results = [
         SearchResult(
-            id=note_id,
+            id=str(note_id),
             doc_type="note",
             title="note",
             excerpt=f"chunk {i}",
@@ -254,7 +244,7 @@ async def test_verify_keeps_nested_file_shared_with_recipient(
 
     await alice.webdav.create_directory(test_dir)
     await alice.webdav.create_directory(nested_dir)
-    await alice.webdav.write_file(shared_path, _PDF_BYTES, "application/pdf")
+    await alice.webdav.write_file(shared_path, PDF_BYTES, "application/pdf")
     file_id = (await alice.webdav.get_file_info(shared_path))["id"]
     tag = await alice.webdav.get_or_create_tag(
         name=get_settings().vector_sync_pdf_tag,
@@ -295,10 +285,10 @@ async def test_verify_drops_unshared_file_for_other_user(alice_bob_clients, mock
     alice, bob = alice_bob_clients
     suffix = uuid.uuid4().hex[:8]
     test_dir = f"acl_verify_priv_{suffix}"
-    private_path = f"{test_dir}/private.txt"
+    private_path = f"{test_dir}/private.pdf"
 
     await alice.webdav.create_directory(test_dir)
-    await alice.webdav.write_file(private_path, b"alice's private note", "text/plain")
+    await alice.webdav.write_file(private_path, PDF_BYTES, "application/pdf")
     file_id = (await alice.webdav.get_file_info(private_path))["id"]
 
     try: