diff --git a/nextcloud_mcp_server/search/verification.py b/nextcloud_mcp_server/search/verification.py index 291b2cae..ca8f5c0e 100644 --- a/nextcloud_mcp_server/search/verification.py +++ b/nextcloud_mcp_server/search/verification.py @@ -389,9 +389,9 @@ async def _verify_news_items( # above for why this is narrower than the API-response failure path. accessible: set[str] = set() for d in doc_ids: - # SearchResult.id is `int | str` (D1: forward-compat widening). Coerce - # to str so the validator's regex applies consistently to both shapes. - if not is_valid_nextcloud_doc_id(str(d)): + # SearchResult.id is always str (Qdrant payload doc_id is keyword- + # indexed; producers stringify on write). Pass through verbatim. + if not is_valid_nextcloud_doc_id(d): logger.warning( "Malformed news_item doc_id %r in verifier; keeping (cannot verify)", d, diff --git a/nextcloud_mcp_server/vector/qdrant_client.py b/nextcloud_mcp_server/vector/qdrant_client.py index 7217966a..0263cb92 100644 --- a/nextcloud_mcp_server/vector/qdrant_client.py +++ b/nextcloud_mcp_server/vector/qdrant_client.py @@ -71,6 +71,71 @@ _qdrant_client: AsyncQdrantClient | None = None _qdrant_init_lock: anyio.Lock | None = None +async def _create_one_payload_index( + client: AsyncQdrantClient, + collection_name: str, + field: str, + schema_type: PayloadSchemaType, +) -> bool: + """Create one payload index with per-field error containment. + + Returns True on success or benign 400 schema-conflict (caller treats as + indexed). Returns False if the field should be added to the caller's + failed-fields list. Never re-raises: the singleton in + ``get_qdrant_client`` is already assigned by the time this runs, so + propagating a network blip would leave the process holding a usable + client with the migration silently incomplete. + """ + try: + await client.create_payload_index( + collection_name=collection_name, + field_name=field, + field_schema=schema_type, + wait=True, + ) + logger.info("Created %s payload index on '%s'", schema_type.name, field) + return True + except UnexpectedResponse as e: + body = getattr(e, "content", b"") or b"" + body_text = body.decode("utf-8", errors="replace") + # 400 is the expected schema-conflict path (index already exists + # with a different type). Verified for Qdrant OSS, where an + # idempotent re-create against a matching schema returns 200; if + # Qdrant Cloud diverges and returns 400 for benign re-creates, + # the WARNING below will fire on every restart against an + # already-indexed collection — read the response body before + # treating that as a real schema conflict. 5xx is unexpected — + # keep the loop going so the remaining fields still get + # attempted, but log at error so operators see it. + if e.status_code == 400: + logger.warning( + "Schema conflict on payload index '%s': %s", field, body_text + ) + return True + logger.error( + "Unexpected error creating payload index on '%s' (status %s): %s", + field, + e.status_code, + body_text, + ) + return False + except Exception: + # Raw network / timeout failures (httpx.ConnectError, + # asyncio.TimeoutError, etc.) reach here — outside the HTTP-status + # taxonomy that UnexpectedResponse covers. Same containment + # rationale as above: one transient failure on one field must not + # skip the rest, and the singleton in get_qdrant_client is already + # assigned by this point so re-raising would leave the process + # holding a usable client with the migration silently incomplete. + logger.error( + "Network error creating payload index on '%s'; " + "field will remain unindexed until next successful restart", + field, + exc_info=True, + ) + return False + + async def _ensure_payload_indexes( client: AsyncQdrantClient, collection_name: str, @@ -82,13 +147,9 @@ async def _ensure_payload_indexes( schema type (KEYWORD for string fields, BOOL for ``is_placeholder``, INTEGER for ``chunk_index``). Skips fields that are already in ``existing_schema`` so routine restarts make no Qdrant write round-trips - and emit no INFO log lines. Schema conflicts (a pre-existing index with - a different type) still surface as a 400 — log loudly so operators can - intervene, but keep going so the remaining fields still get indexed. - The same per-field error containment applies to raw network errors - (e.g. ``httpx.ConnectError`` from a transient Qdrant unavailability): - log at ERROR with ``exc_info`` and continue, so a single transient - failure on one field does not skip the rest. + and emit no INFO log lines. Per-field error handling (schema conflicts, + network errors) lives in ``_create_one_payload_index``; this loop is + flat so a single transient failure on one field does not skip the rest. Args: client: Qdrant client instance. @@ -117,60 +178,17 @@ async def _ensure_payload_indexes( ) return existing_schema = collection_info.payload_schema or {} - failed_fields: list[str] = [] + failed_fields: list[str] = [] for field, schema_type in _PAYLOAD_INDEX_FIELDS.items(): if field in existing_schema: # Index already present — silent skip. Logging here on every # restart would be noise that hides the genuinely interesting - # "first-time creation" line below. + # "first-time creation" line in _create_one_payload_index. continue - try: - await client.create_payload_index( - collection_name=collection_name, - field_name=field, - field_schema=schema_type, - wait=True, - ) - logger.info("Created %s payload index on '%s'", schema_type.name, field) - except UnexpectedResponse as e: - body = getattr(e, "content", b"") or b"" - body_text = body.decode("utf-8", errors="replace") - # 400 is the expected schema-conflict path (index already exists - # with a different type). Verified for Qdrant OSS, where an - # idempotent re-create against a matching schema returns 200; if - # Qdrant Cloud diverges and returns 400 for benign re-creates, - # the WARNING below will fire on every restart against an - # already-indexed collection — read the response body before - # treating that as a real schema conflict. 5xx is unexpected — - # keep the loop going so the remaining fields still get - # attempted, but log at error so operators see it. - if e.status_code == 400: - logger.warning( - "Schema conflict on payload index '%s': %s", field, body_text - ) - else: - logger.error( - "Unexpected error creating payload index on '%s' (status %s): %s", - field, - e.status_code, - body_text, - ) - failed_fields.append(field) - except Exception: - # Raw network / timeout failures (httpx.ConnectError, - # asyncio.TimeoutError, etc.) reach here — outside the HTTP-status - # taxonomy that UnexpectedResponse covers. Same containment - # rationale as above: one transient failure on one field must not - # skip the rest, and the singleton in get_qdrant_client is already - # assigned by this point so re-raising would leave the process - # holding a usable client with the migration silently incomplete. - logger.error( - "Network error creating payload index on '%s'; " - "field will remain unindexed until next successful restart", - field, - exc_info=True, - ) + if not await _create_one_payload_index( + client, collection_name, field, schema_type + ): failed_fields.append(field) # A single per-field ERROR line is easy to miss in startup noise. Surface @@ -467,11 +485,21 @@ async def get_qdrant_client() -> AsyncQdrantClient: if _qdrant_client is None: settings = get_settings() + # Build the client into a local ``provisional`` and only publish + # it to the global ``_qdrant_client`` after the migration awaits + # below have all completed. The fast-path check at the top of + # this function reads ``_qdrant_client`` without the lock, so + # publishing the constructed-but-unmigrated client would let a + # concurrent caller short-circuit the lock and fire a filtered + # search before ``_ensure_payload_indexes`` runs — that search + # would 400 with "Index required but not found". + provisional: AsyncQdrantClient + # Detect mode and initialize client accordingly if settings.qdrant_url: # Network mode logger.info(f"Using Qdrant network mode: {settings.qdrant_url}") - _qdrant_client = AsyncQdrantClient( + provisional = AsyncQdrantClient( url=settings.qdrant_url, api_key=settings.qdrant_api_key, timeout=30, @@ -480,17 +508,17 @@ async def get_qdrant_client() -> AsyncQdrantClient: # Local mode (either :memory: or persistent path) if settings.qdrant_location == ":memory:": logger.info("Using Qdrant in-memory mode: :memory:") - _qdrant_client = AsyncQdrantClient(":memory:") + provisional = AsyncQdrantClient(":memory:") else: # Persistent local mode - use path parameter logger.info( f"Using Qdrant persistent mode: {settings.qdrant_location}" ) - _qdrant_client = AsyncQdrantClient(path=settings.qdrant_location) + provisional = AsyncQdrantClient(path=settings.qdrant_location) else: # Should not happen due to __post_init__ validation, but handle gracefully logger.warning("No Qdrant mode configured, defaulting to :memory:") - _qdrant_client = AsyncQdrantClient(":memory:") + provisional = AsyncQdrantClient(":memory:") # Get collection name (auto-generated from deployment ID + model) collection_name = settings.get_collection_name() @@ -505,7 +533,7 @@ async def get_qdrant_client() -> AsyncQdrantClient: # Explicitly check if collection exists logger.debug(f"Checking if collection '{collection_name}' exists...") - collections = await _qdrant_client.get_collections() + collections = await provisional.get_collections() collection_names = [c.name for c in collections.collections] if collection_name in collection_names: @@ -513,7 +541,7 @@ async def get_qdrant_client() -> AsyncQdrantClient: logger.debug( f"Collection '{collection_name}' found, validating dimensions..." ) - collection_info = await _qdrant_client.get_collection(collection_name) + collection_info = await provisional.get_collection(collection_name) # Handle both named vectors (dict) and legacy single vector vectors = collection_info.config.params.vectors if isinstance(vectors, dict): @@ -551,10 +579,10 @@ async def get_qdrant_client() -> AsyncQdrantClient: # never schema or indexes, so the snapshot remains accurate # across the backfill call. await _backfill_doc_id_to_string( - _qdrant_client, collection_name, expected_dimension + provisional, collection_name, expected_dimension ) await _ensure_payload_indexes( - _qdrant_client, + provisional, collection_name, existing_schema=collection_info.payload_schema or {}, ) @@ -566,7 +594,7 @@ async def get_qdrant_client() -> AsyncQdrantClient: f"Collection '{collection_name}' not found, creating with " f"dimension={expected_dimension}, model={embedding_model}..." ) - await _qdrant_client.create_collection( + await provisional.create_collection( collection_name=collection_name, vectors_config={ "dense": VectorParams( @@ -601,9 +629,15 @@ async def get_qdrant_client() -> AsyncQdrantClient: # implicit auto-indexes, etc.) worth investigating before # suppressing. await _ensure_payload_indexes( - _qdrant_client, collection_name, existing_schema={} + provisional, collection_name, existing_schema={} ) + # Publish only after the migration awaits completed. From this + # point on, fast-path callers may short-circuit the lock and + # use the client; every payload index they could filter on now + # exists. + _qdrant_client = provisional + # Lock released. ``_qdrant_client`` is guaranteed non-None here: # either the fast path returned earlier, the lock-protected branch # set it, or a sibling waiter set it before we got the lock. diff --git a/nextcloud_mcp_server/vector/scanner.py b/nextcloud_mcp_server/vector/scanner.py index 2dccdb3f..0f2204e4 100644 --- a/nextcloud_mcp_server/vector/scanner.py +++ b/nextcloud_mcp_server/vector/scanner.py @@ -13,7 +13,8 @@ from email.utils import parsedate_to_datetime import anyio from anyio.abc import TaskStatus from anyio.streams.memory import MemoryObjectSendStream -from qdrant_client.models import FieldCondition, Filter, MatchValue +from qdrant_client import AsyncQdrantClient +from qdrant_client.models import FieldCondition, Filter, MatchValue, Record from nextcloud_mcp_server.client import NextcloudClient from nextcloud_mcp_server.client.news import NewsItemType @@ -43,6 +44,49 @@ INDEXED_DOC_TYPES: frozenset[str] = frozenset( ) +# Page size for paginated deletion-tracking scrolls. Chosen to keep per-page +# memory bounded while making the round-trip count manageable in the typical +# < 100 k point per (user_id, doc_type) case. The previous single-page +# ``limit=10_000`` silently truncated deletion sets for any user past the +# cap, so anything indexed beyond the first 10 k was never reconciled. +_DELETION_TRACKING_PAGE_SIZE: int = 1024 + + +async def _scroll_all_points( + qdrant_client: AsyncQdrantClient, + *, + collection_name: str, + scroll_filter: Filter, + payload_fields: list[str], + page_size: int = _DELETION_TRACKING_PAGE_SIZE, +) -> list[Record]: + """Scroll every point matching the filter, paginating until exhausted. + + Replaces the prior single-page ``limit=10_000`` calls that silently + dropped points beyond the first page. Pagination follows Qdrant's + documented contract: ``scroll`` returns ``(points, next_page_offset)`` + and ``next_page_offset`` is ``None`` once the cursor reaches the end. + Errors propagate to the caller — the scanner's outer ``try`` already + handles them by skipping the deletion-tracking pass for this scan + (worse: extra-scan latency; never: bad data). + """ + all_points: list[Record] = [] + offset = None + while True: + points, offset = await qdrant_client.scroll( + collection_name=collection_name, + scroll_filter=scroll_filter, + with_payload=payload_fields, + with_vectors=False, + limit=page_size, + offset=offset, + ) + all_points.extend(points) + if offset is None: + break + return all_points + + @dataclass class DocumentTask: """Document task for processing queue.""" @@ -79,8 +123,11 @@ async def get_last_indexed_timestamp(user_id: str) -> int | None: try: qdrant_client = await get_qdrant_client() - # Query for user's notes, ordered by indexed_at descending, limit 1 - scroll_result = await qdrant_client.scroll( + # Scroll across every indexed note for this user — paginated so users + # with > 10 k indexed notes still produce a correct max (the prior + # single-page ``limit=10_000`` would have silently undercounted). + points = await _scroll_all_points( + qdrant_client, collection_name=get_settings().get_collection_name(), scroll_filter=Filter( must=[ @@ -88,19 +135,16 @@ async def get_last_indexed_timestamp(user_id: str) -> int | None: FieldCondition(key="doc_type", match=MatchValue(value="note")), ] ), - with_payload=["indexed_at"], - with_vectors=False, - limit=10000, # Get all to find max + payload_fields=["indexed_at"], ) - # Find max indexed_at across all results - num_points = len(scroll_result[0]) if scroll_result[0] else 0 + num_points = len(points) logger.info(f"Found {num_points} indexed notes in Qdrant for user {user_id}") - if scroll_result[0]: + if points: timestamps = [ point.payload.get("indexed_at", 0) - for point in scroll_result[0] + for point in points if point.payload is not None ] max_timestamp = max(timestamps) if timestamps else 0 @@ -220,7 +264,8 @@ async def scan_user_documents( indexed_doc_ids = set() if not initial_sync: assert qdrant_client is not None # narrow for the type checker - scroll_result = await qdrant_client.scroll( + points = await _scroll_all_points( + qdrant_client, collection_name=get_settings().get_collection_name(), scroll_filter=Filter( must=[ @@ -228,14 +273,12 @@ async def scan_user_documents( FieldCondition(key="doc_type", match=MatchValue(value="note")), ] ), - with_payload=["doc_id"], - with_vectors=False, - limit=10000, + payload_fields=["doc_id"], ) indexed_doc_ids = { str(point.payload["doc_id"]) - for point in (scroll_result[0] or []) + for point in points if point.payload is not None and "doc_id" in point.payload } @@ -394,7 +437,8 @@ async def scan_user_documents( indexed_file_ids = set() if not initial_sync: assert qdrant_client is not None # narrow for the type checker - file_scroll_result = await qdrant_client.scroll( + points = await _scroll_all_points( + qdrant_client, collection_name=settings.get_collection_name(), scroll_filter=Filter( must=[ @@ -402,14 +446,12 @@ async def scan_user_documents( FieldCondition(key="doc_type", match=MatchValue(value="file")), ] ), - limit=10000, # Reasonable limit for file count - with_payload=["doc_id"], - with_vectors=False, + payload_fields=["doc_id"], ) indexed_file_ids = { str(point.payload["doc_id"]) - for point in (file_scroll_result[0] or []) + for point in points if point.payload is not None and "doc_id" in point.payload } @@ -675,7 +717,8 @@ async def scan_news_items( indexed_item_ids: set[str] = set() if not initial_sync: qdrant_client = await get_qdrant_client() - scroll_result = await qdrant_client.scroll( + points = await _scroll_all_points( + qdrant_client, collection_name=settings.get_collection_name(), scroll_filter=Filter( must=[ @@ -683,13 +726,11 @@ async def scan_news_items( FieldCondition(key="doc_type", match=MatchValue(value="news_item")), ] ), - with_payload=["doc_id"], - with_vectors=False, - limit=10000, + payload_fields=["doc_id"], ) indexed_item_ids = { str(point.payload["doc_id"]) - for point in (scroll_result[0] or []) + for point in points if point.payload is not None and "doc_id" in point.payload } logger.debug(f"Found {len(indexed_item_ids)} indexed news items in Qdrant") @@ -854,7 +895,8 @@ async def scan_deck_cards( indexed_card_ids: set[str] = set() if not initial_sync: qdrant_client = await get_qdrant_client() - scroll_result = await qdrant_client.scroll( + points = await _scroll_all_points( + qdrant_client, collection_name=settings.get_collection_name(), scroll_filter=Filter( must=[ @@ -862,13 +904,11 @@ async def scan_deck_cards( FieldCondition(key="doc_type", match=MatchValue(value="deck_card")), ] ), - with_payload=["doc_id"], - with_vectors=False, - limit=10000, + payload_fields=["doc_id"], ) indexed_card_ids = { str(point.payload["doc_id"]) - for point in (scroll_result[0] or []) + for point in points if point.payload is not None and "doc_id" in point.payload } logger.debug(f"Found {len(indexed_card_ids)} indexed deck cards in Qdrant") diff --git a/tests/unit/search/test_verification.py b/tests/unit/search/test_verification.py index da5b8065..3cee6981 100644 --- a/tests/unit/search/test_verification.py +++ b/tests/unit/search/test_verification.py @@ -35,8 +35,11 @@ def _make_result( score: float = 0.9, metadata: dict | None = None, ) -> SearchResult: + # Mirror the producer-side stringification (scanner writes str(note["id"]) + # etc. into Qdrant payloads). Tests pass int literals for readability; + # the SearchResult contract is ``id: str``. return SearchResult( - id=doc_id, + id=str(doc_id), doc_type=doc_type, title=f"{doc_type}_{doc_id}", excerpt="...", @@ -84,7 +87,7 @@ async def test_verify_notes_200_keeps_all(mocker): client, [_make_result(1), _make_result(2), _make_result(3)], _sem() ) - assert result == {1, 2, 3} + assert result == {"1", "2", "3"} assert notes_client.get_note.await_count == 3 @@ -122,7 +125,7 @@ async def test_verify_notes_transient_5xx_keeps(mocker): result = await _verify_notes(client, [_make_result(42)], _sem()) - assert result == {42} + assert result == {"42"} @pytest.mark.unit @@ -140,7 +143,7 @@ async def test_verify_notes_429_keeps_as_transient(mocker): result = await _verify_notes(client, [_make_result(42)], _sem()) - assert result == {42} + assert result == {"42"} @pytest.mark.unit @@ -152,7 +155,7 @@ async def test_verify_notes_unexpected_exception_keeps(mocker): result = await _verify_notes(client, [_make_result(7)], _sem()) - assert result == {7} + assert result == {"7"} @pytest.mark.unit @@ -193,7 +196,7 @@ async def test_verify_notes_mixed_outcomes(mocker): client, [_make_result(1), _make_result(2), _make_result(3)], _sem() ) - assert result == {1, 3} + assert result == {"1", "3"} @pytest.mark.unit @@ -242,7 +245,7 @@ async def test_verify_news_items_intersects_with_fetched_set(mocker): _sem(), ) - assert result == {10, 20} + assert result == {"10", "20"} assert news_client.get_items.await_count == 1 @@ -304,7 +307,7 @@ async def test_verify_news_items_transient_keeps_all(mocker): _sem(), ) - assert result == {1, 2, 3} + assert result == {"1", "2", "3"} @pytest.mark.unit @@ -325,7 +328,7 @@ async def test_verify_news_items_429_keeps_as_transient(mocker): _sem(), ) - assert result == {1, 2, 3} + assert result == {"1", "2", "3"} @pytest.mark.unit @@ -350,7 +353,7 @@ async def test_verify_news_items_unexpected_exception_keeps_all(mocker): _sem(), ) - assert result == {1, 2} + assert result == {"1", "2"} @pytest.mark.unit @@ -377,7 +380,7 @@ async def test_verify_news_items_non_numeric_id_keeps_only_bad_item(mocker): ) # 10 and 20 are verified present; "abc" is unverifiable so kept fail-open. - assert result == {10, 20, "abc"} + assert result == {"10", "20", "abc"} @pytest.mark.unit @@ -402,7 +405,7 @@ async def test_verify_news_items_drops_missing_when_other_id_is_non_numeric( ) # 10 verified present, 20 verified missing (dropped), "abc" unverifiable. - assert result == {10, "abc"} + assert result == {"10", "abc"} @pytest.mark.unit @@ -426,7 +429,7 @@ async def test_verify_news_items_malformed_api_response_keeps_all(mocker): ) # Batch fail-open: API broken, every requested id preserved. - assert result == {10, 20} + assert result == {"10", "20"} # --------------------------------------------------------------------------- @@ -448,7 +451,7 @@ async def test_verify_files_uses_path_from_metadata(mocker): _sem(), ) - assert result == {100} + assert result == {"100"} webdav_client.get_file_info.assert_awaited_once_with("Documents/foo.txt") @@ -487,7 +490,7 @@ async def test_verify_files_malformed_propfind_keeps_result(mocker): _sem(), ) - assert result == {123}, "ambiguous None must keep result, not evict" + assert result == {"123"}, "ambiguous None must keep result, not evict" @pytest.mark.unit @@ -517,14 +520,14 @@ async def test_verify_files_missing_path_metadata_keeps_unverified(mocker): # No metadata at all result = await _verify_files(client, [_make_result(555, doc_type="file")], _sem()) - assert result == {555} + assert result == {"555"} webdav_client.get_file_info.assert_not_awaited() # Metadata present but no "path" key result = await _verify_files( client, [_make_result(556, doc_type="file", metadata={})], _sem() ) - assert result == {556} + assert result == {"556"} webdav_client.get_file_info.assert_not_awaited() @@ -541,7 +544,7 @@ async def test_verify_files_transient_5xx_keeps(mocker): _sem(), ) - assert result == {7} + assert result == {"7"} @pytest.mark.unit @@ -558,7 +561,7 @@ async def test_verify_files_429_keeps_as_transient(mocker): _sem(), ) - assert result == {7} + assert result == {"7"} @pytest.mark.unit @@ -580,7 +583,7 @@ async def test_verify_files_unexpected_exception_keeps(mocker): _sem(), ) - assert result == {8} + assert result == {"8"} # --------------------------------------------------------------------------- @@ -606,7 +609,7 @@ async def test_verify_deck_cards_uses_metadata_fast_path(mocker): _sem(), ) - assert result == {42} + assert result == {"42"} deck_client.get_card.assert_awaited_once_with(board_id=1, stack_id=2, card_id=42) @@ -676,7 +679,7 @@ async def test_verify_deck_cards_transient_5xx_keeps(mocker): _sem(), ) - assert result == {42} + assert result == {"42"} @pytest.mark.unit @@ -699,7 +702,7 @@ async def test_verify_deck_cards_429_keeps_as_transient(mocker): _sem(), ) - assert result == {42} + assert result == {"42"} @pytest.mark.unit @@ -722,7 +725,7 @@ async def test_verify_deck_cards_unexpected_exception_keeps(mocker): _sem(), ) - assert result == {42} + assert result == {"42"} @pytest.mark.unit @@ -749,7 +752,7 @@ async def test_verify_deck_cards_non_numeric_metadata_keeps(mocker): ], _sem(), ) - assert result == {42} + assert result == {"42"} # Non-numeric stack_id result = await _verify_deck_cards( @@ -763,7 +766,7 @@ async def test_verify_deck_cards_non_numeric_metadata_keeps(mocker): ], _sem(), ) - assert result == {43} + assert result == {"43"} # Non-numeric card_id (doc_id itself) result = await _verify_deck_cards( @@ -794,7 +797,7 @@ async def test_verify_deck_cards_missing_metadata_keeps_unverified(mocker): result = await _verify_deck_cards( client, [_make_result(42, doc_type="deck_card")], _sem() ) - assert result == {42} + assert result == {"42"} # Only board_id (stack_id missing) result = await _verify_deck_cards( @@ -802,7 +805,7 @@ async def test_verify_deck_cards_missing_metadata_keeps_unverified(mocker): [_make_result(43, doc_type="deck_card", metadata={"board_id": 1})], _sem(), ) - assert result == {43} + assert result == {"43"} # Only stack_id (board_id missing) result = await _verify_deck_cards( @@ -810,7 +813,7 @@ async def test_verify_deck_cards_missing_metadata_keeps_unverified(mocker): [_make_result(44, doc_type="deck_card", metadata={"stack_id": 2})], _sem(), ) - assert result == {44} + assert result == {"44"} deck_client.get_card.assert_not_awaited() @@ -829,7 +832,7 @@ async def test_verify_search_results_empty_input_passthrough(): @pytest.mark.unit async def test_verify_search_results_dedupes_chunks_per_document(mocker): """Two chunks of the same note → ONE call to the underlying verifier.""" - spy = mocker.AsyncMock(return_value={1}) + spy = mocker.AsyncMock(return_value={"1"}) mocker.patch.dict(verification._VERIFIERS, {"note": spy}, clear=False) mocker.patch.object(verification, "delete_document_points", mocker.AsyncMock()) @@ -848,7 +851,7 @@ async def test_verify_search_results_dedupes_chunks_per_document(mocker): # Verifier received exactly one SearchResult (the deduplicated representative) args, _kwargs = spy.call_args assert len(args[1]) == 1 - assert args[1][0].id == 1 + assert args[1][0].id == "1" # And a semaphore as the third arg assert isinstance(args[2], anyio.Semaphore) @@ -1035,7 +1038,7 @@ async def test_verify_search_results_verifier_blowup_keeps_all(mocker): kept, dropped_count = await verify_search_results(client, results) - assert [r.id for r in kept] == [1, 2] + assert [r.id for r in kept] == ["1", "2"] assert dropped_count == 0 # fail-open: nothing dropped spy_evict.assert_not_awaited() @@ -1043,7 +1046,7 @@ async def test_verify_search_results_verifier_blowup_keeps_all(mocker): @pytest.mark.unit async def test_verify_search_results_preserves_order(mocker): """Order of original results must be preserved after filtering.""" - note_verifier = mocker.AsyncMock(return_value={1, 3}) + note_verifier = mocker.AsyncMock(return_value={"1", "3"}) mocker.patch.dict(verification._VERIFIERS, {"note": note_verifier}, clear=False) mocker.patch.object(verification, "delete_document_points", mocker.AsyncMock()) @@ -1056,7 +1059,7 @@ async def test_verify_search_results_preserves_order(mocker): kept, dropped_count = await verify_search_results(client, results) - assert [r.id for r in kept] == [1, 3] + assert [r.id for r in kept] == ["1", "3"] assert dropped_count == 1 @@ -1083,8 +1086,8 @@ async def test_verify_search_results_eviction_failure_does_not_propagate(mocker) @pytest.mark.unit async def test_verify_search_results_dispatches_per_doc_type_concurrently(mocker): """Mixed doc_types must be routed to their respective verifiers.""" - note_verifier = mocker.AsyncMock(return_value={1}) - file_verifier = mocker.AsyncMock(return_value={500}) + note_verifier = mocker.AsyncMock(return_value={"1"}) + file_verifier = mocker.AsyncMock(return_value={"500"}) mocker.patch.dict( verification._VERIFIERS, {"note": note_verifier, "file": file_verifier}, @@ -1101,7 +1104,7 @@ async def test_verify_search_results_dispatches_per_doc_type_concurrently(mocker kept, dropped_count = await verify_search_results(client, results) - assert {(r.id, r.doc_type) for r in kept} == {(1, "note"), (500, "file")} + assert {(r.id, r.doc_type) for r in kept} == {("1", "note"), ("500", "file")} assert dropped_count == 1 note_verifier.assert_awaited_once() file_verifier.assert_awaited_once() diff --git a/tests/unit/test_chunk_context_offset_gate.py b/tests/unit/test_chunk_context_offset_gate.py index 8c7f85cc..c8e6dedd 100644 --- a/tests/unit/test_chunk_context_offset_gate.py +++ b/tests/unit/test_chunk_context_offset_gate.py @@ -56,7 +56,7 @@ class TestOffsetFallbackGate: result = await get_chunk_with_context( nc_client=mock_nc_client, user_id="alice", - doc_id=12345, + doc_id="12345", doc_type="file", chunk_start=0, chunk_end=100, @@ -101,7 +101,7 @@ class TestOffsetFallbackGate: await get_chunk_with_context( nc_client=mock_nc_client, user_id="alice", - doc_id=42, + doc_id="42", doc_type="note", chunk_start=0, chunk_end=10, @@ -133,7 +133,7 @@ class TestOffsetFallbackGate: await get_chunk_with_context( nc_client=mock_nc_client, user_id="alice", - doc_id=12345, + doc_id="12345", doc_type="file", chunk_start=0, chunk_end=100, @@ -175,7 +175,7 @@ class TestNullableChunkIndexPropagation: result = await get_chunk_with_context( nc_client=mock_nc_client, user_id="alice", - doc_id=42, + doc_id="42", doc_type="note", chunk_start=100, chunk_end=200, @@ -222,7 +222,7 @@ class TestNullableChunkIndexPropagation: result = await get_chunk_with_context( nc_client=mock_nc_client, user_id="alice", - doc_id=42, + doc_id="42", doc_type="note", chunk_start=0, chunk_end=10, @@ -264,7 +264,7 @@ class TestNullableChunkIndexPropagation: result = await get_chunk_with_context( nc_client=mock_nc_client, user_id="alice", - doc_id=42, + doc_id="42", doc_type="note", chunk_start=100, chunk_end=200, @@ -307,7 +307,7 @@ class TestAdjacentChunkBoundary: result = await get_chunk_with_context( nc_client=mock_nc_client, user_id="alice", - doc_id=42, + doc_id="42", doc_type="note", chunk_start=0, chunk_end=10, @@ -348,7 +348,7 @@ class TestAdjacentChunkBoundary: result = await get_chunk_with_context( nc_client=mock_nc_client, user_id="alice", - doc_id=42, + doc_id="42", doc_type="note", chunk_start=0, chunk_end=10,