From 92b2d50cd743dc344ec4349bf042260004d7fd50 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Fri, 8 May 2026 22:59:46 +0200 Subject: [PATCH] =?UTF-8?q?fix(vector):=20address=20PR=20review=20round=20?= =?UTF-8?q?3=20=E2=80=94=20sentinel=20guard,=20skip=20indexed=20fields,=20?= =?UTF-8?q?narrow=20types?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add a fixed-UUID sentinel point written after a successful doc_id backfill so subsequent restarts retrieve it and short-circuit the O(N) scroll. Sentinel has no user_id/doc_id/doc_type payload so production search filters never see it. - Pre-fetch payload_schema in _ensure_keyword_payload_indexes and silently skip fields that are already indexed; the "Created KEYWORD payload index" INFO log fires only on actual creation. - Narrow stale `int | str` doc_id annotations to `str` across search/verification.py (BatchVerifier return type, per-verifier accessible sets, by_type / accessible_by_type / inaccessible collections); drop the now-redundant `type(d).__name__` prefix in the dropped-docs log. - Align the backfill log message with the PR description's "Running doc_id backfill" promise; add a caller cross-reference to the wait=True comment. - Fix _get_file_path_from_qdrant docstring (file_id is str, not numeric). - Convert legacy `id=1` to `id="1"` in test_search_result.py to match the SearchResult.id: str annotation. Three new unit tests cover sentinel-found, sentinel-written, and skip-existing-index branches; existing backfill tests pass dimension and explicit retrieve.return_value=[] for the no-sentinel path. Co-Authored-By: Claude Opus 4.7 (1M context) --- nextcloud_mcp_server/search/context.py | 3 +- nextcloud_mcp_server/search/verification.py | 28 ++--- nextcloud_mcp_server/vector/qdrant_client.py | 101 ++++++++++++--- tests/unit/search/test_search_result.py | 16 +-- tests/unit/vector/test_qdrant_client.py | 122 +++++++++++++++++-- 5 files changed, 223 insertions(+), 47 deletions(-) diff --git a/nextcloud_mcp_server/search/context.py b/nextcloud_mcp_server/search/context.py index c3d4827a..1771c087 100644 --- a/nextcloud_mcp_server/search/context.py +++ b/nextcloud_mcp_server/search/context.py @@ -151,7 +151,8 @@ async def _get_file_path_from_qdrant( Args: user_id: User ID who owns the file - file_id: Numeric file ID + file_id: Stringified file ID (Qdrant payload value, post-doc_id + normalization — see vector/qdrant_client.py) chunk_start: Character offset where chunk starts chunk_end: Character offset where chunk ends diff --git a/nextcloud_mcp_server/search/verification.py b/nextcloud_mcp_server/search/verification.py index e5665f3e..a4b0a0d6 100644 --- a/nextcloud_mcp_server/search/verification.py +++ b/nextcloud_mcp_server/search/verification.py @@ -49,7 +49,7 @@ logger = logging.getLogger(__name__) BatchVerifier = Callable[ [NextcloudClientProtocol, list[SearchResult], anyio.Semaphore], - Awaitable[set[int | str]], + Awaitable[set[str]], ] """(client, results, semaphore) -> set of doc_ids accessible to the user.""" @@ -75,9 +75,9 @@ async def _verify_notes( client: NextcloudClientProtocol, results: list[SearchResult], semaphore: anyio.Semaphore, -) -> set[int | str]: +) -> set[str]: # safe: cooperative concurrency, no lock needed (see verify_search_results) - accessible: set[int | str] = set() + accessible: set[str] = set() async def check(result: SearchResult) -> None: doc_id = result.id @@ -130,9 +130,9 @@ async def _verify_files( client: NextcloudClientProtocol, results: list[SearchResult], semaphore: anyio.Semaphore, -) -> set[int | str]: +) -> set[str]: # safe: cooperative concurrency, no lock needed (see verify_search_results) - accessible: set[int | str] = set() + accessible: set[str] = set() async def check(result: SearchResult) -> None: doc_id = result.id @@ -201,9 +201,9 @@ async def _verify_deck_cards( client: NextcloudClientProtocol, results: list[SearchResult], semaphore: anyio.Semaphore, -) -> set[int | str]: +) -> set[str]: # safe: cooperative concurrency, no lock needed (see verify_search_results) - accessible: set[int | str] = set() + accessible: set[str] = set() async def check(result: SearchResult) -> None: doc_id = result.id @@ -283,7 +283,7 @@ async def _verify_news_items( client: NextcloudClientProtocol, results: list[SearchResult], semaphore: anyio.Semaphore, -) -> set[int | str]: +) -> set[str]: """Batch-verify news items with a single fetch. The Nextcloud News API has no per-item endpoint, so ``news.get_item`` is @@ -386,7 +386,7 @@ async def _verify_news_items( # for THAT item only — not the whole batch. Mirrors the per-item # shape of the notes/files/deck verifiers. See the granularity note # above for why this is narrower than the API-response failure path. - accessible: set[int | str] = set() + accessible: set[str] = set() for d in doc_ids: try: if int(d) in present_ids: @@ -474,7 +474,7 @@ async def verify_search_results( # deduplicated batch. We pick one SearchResult per (id, doc_type) to carry # metadata (path, board_id/stack_id) into the verifier — chunks of the # same document share these fields, so any chunk works. - by_type: dict[str, dict[int | str, SearchResult]] = {} + by_type: dict[str, dict[str, SearchResult]] = {} for r in results: by_type.setdefault(r.doc_type, {}).setdefault(r.id, r) @@ -494,7 +494,7 @@ async def verify_search_results( # same write. Adding a lock would be dead weight; using ``anyio.Lock`` # here would force serialization on a path that is intentionally # parallel. - accessible_by_type: dict[str, set[int | str]] = {} + accessible_by_type: dict[str, set[str]] = {} async def run_verifier(doc_type: str, unique_results: list[SearchResult]) -> None: verifier = _VERIFIERS.get(doc_type) @@ -526,7 +526,7 @@ async def verify_search_results( tg.start_soon(run_verifier, doc_type, list(id_to_result.values())) # Compute (doc_id, doc_type) pairs that failed verification - inaccessible: set[tuple[int | str, str]] = set() + inaccessible: set[tuple[str, str]] = set() for doc_type, id_to_result in by_type.items(): # The .get() default is defensive only — run_verifier always populates # accessible_by_type[doc_type], either with the verifier's result or @@ -537,12 +537,10 @@ async def verify_search_results( inaccessible.add((doc_id, doc_type)) if inaccessible: - # Tag ids with their type (int vs str) so ghost-record logs are - # unambiguous: int 42 and str "42" both render as "42" otherwise. logger.info( "Verification dropped %d inaccessible document(s): %s", len(inaccessible), - sorted((f"{type(d).__name__}:{d}", t) for d, t in inaccessible), + sorted(inaccessible), ) # Filter results, preserving order. All chunks of an inaccessible document diff --git a/nextcloud_mcp_server/vector/qdrant_client.py b/nextcloud_mcp_server/vector/qdrant_client.py index 9c3808f7..bf676923 100644 --- a/nextcloud_mcp_server/vector/qdrant_client.py +++ b/nextcloud_mcp_server/vector/qdrant_client.py @@ -5,7 +5,12 @@ from typing import Any from qdrant_client import AsyncQdrantClient, models from qdrant_client.http.exceptions import UnexpectedResponse -from qdrant_client.models import Distance, PayloadSchemaType, VectorParams +from qdrant_client.models import ( + Distance, + PayloadSchemaType, + PointStruct, + VectorParams, +) from nextcloud_mcp_server.config import get_settings from nextcloud_mcp_server.embedding import get_embedding_service @@ -20,6 +25,14 @@ logger = logging.getLogger(__name__) # correct schema (see ADR notes in commit message). _KEYWORD_PAYLOAD_FIELDS: tuple[str, ...] = ("doc_id", "user_id", "doc_type") +# Sentinel point that records "this collection has been backfilled to str +# doc_id". Written after a successful pass of _backfill_doc_id_to_string so +# subsequent restarts can short-circuit the O(N) scroll. Carries no +# user_id/doc_id/doc_type, so production search filters (which always +# require user_id) never see it. +_DOC_ID_BACKFILL_SENTINEL_ID: str = "00000000-0000-0000-0000-d0c1d0d1d0c1" +_DOC_ID_BACKFILL_SENTINEL_PAYLOAD: dict[str, str] = {"_migration_marker": "doc_id_v1"} + # Singleton instance _qdrant_client: AsyncQdrantClient | None = None @@ -29,12 +42,22 @@ async def _ensure_keyword_payload_indexes( ) -> None: """Create KEYWORD payload indexes for fields used in exact-match filters. - Idempotent at the Qdrant layer: re-creating an identical index returns - 200, so this can run on every startup. Schema conflicts (a pre-existing - index with a different type) surface as a 400 — log loudly so operators - can intervene, but keep going so the remaining fields still get indexed. + Pre-fetches the existing payload schema and skips fields that are + already indexed, 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. """ + collection_info = await client.get_collection(collection_name) + existing_schema = collection_info.payload_schema or {} + for field in _KEYWORD_PAYLOAD_FIELDS: + 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. + continue try: await client.create_payload_index( collection_name=collection_name, @@ -64,22 +87,48 @@ async def _ensure_keyword_payload_indexes( async def _backfill_doc_id_to_string( - client: AsyncQdrantClient, collection_name: str + client: AsyncQdrantClient, collection_name: str, dimension: int ) -> None: """Rewrite legacy integer doc_id payloads to strings. Producers now uniformly write str(doc_id), but historical points may carry int values from before normalization. A KEYWORD index does not match int payloads, so any leftover int doc_ids would be silently invisible to - filters. Scrolls all points once and converts in-place; idempotent (a - second pass over the same collection performs zero writes). + filters. Scrolls all points once, converts in-place, and writes a + sentinel point on success; subsequent restarts retrieve the sentinel + and skip the scroll entirely. Idempotent in both directions (a second + pass on a migrated collection short-circuits via the sentinel; a + second pass with the sentinel manually deleted is the same zero-write + scroll the first pass would do on an already-clean collection). Within each scroll batch, points sharing the same int doc_id are batched into a single ``set_payload`` call to minimize Qdrant round-trips. + + Args: + client: Qdrant client instance. + collection_name: Target collection. + dimension: Dense-vector dimension for the sentinel point's vector + (forwarded by ``get_qdrant_client`` from the embedding model). """ + # Sentinel guard: if the migration ran successfully against this + # collection on a previous start, retrieve() returns the marker point + # and we skip the scroll. Cheap single-key lookup vs. an O(N) scroll. + sentinel = await client.retrieve( + collection_name=collection_name, + ids=[_DOC_ID_BACKFILL_SENTINEL_ID], + with_payload=False, + with_vectors=False, + ) + if sentinel: + logger.debug( + "doc_id backfill sentinel found on '%s'; skipping scroll", + collection_name, + ) + return + logger.info( - "Scanning '%s' for legacy int doc_id payloads (this is a one-time " - "migration on first start after upgrade)", + "Running doc_id backfill on '%s' (one-time migration on first " + "start after upgrade; subsequent restarts skip via sentinel)", collection_name, ) @@ -117,10 +166,11 @@ async def _backfill_doc_id_to_string( by_value.setdefault(str(value), []).append(point.id) for str_val, point_ids in by_value.items(): - # wait=True is required: _ensure_keyword_payload_indexes runs - # immediately after this function and only indexes committed - # data — fire-and-forget writes would leave int payloads - # invisible to KEYWORD filters. + # wait=True is required because _ensure_keyword_payload_indexes + # runs immediately after this function (see get_qdrant_client + # near the call site) and only indexes committed data — + # fire-and-forget writes would leave int payloads invisible + # to KEYWORD filters. await client.set_payload( collection_name=collection_name, payload={"doc_id": str_val}, @@ -132,6 +182,25 @@ async def _backfill_doc_id_to_string( if next_offset is None: break + # Write the sentinel after a successful scroll so a future restart can + # short-circuit. Empty sparse vector mirrors the placeholder.py + # convention (vector/placeholder.py); zero dense vector is fine + # because the sentinel never participates in a search (no user_id / + # doc_id / doc_type payload to match). + sentinel_point = PointStruct( + id=_DOC_ID_BACKFILL_SENTINEL_ID, + vector={ + "dense": [0.0] * dimension, + "sparse": models.SparseVector(indices=[], values=[]), + }, + payload=dict(_DOC_ID_BACKFILL_SENTINEL_PAYLOAD), + ) + await client.upsert( + collection_name=collection_name, + points=[sentinel_point], + wait=True, + ) + logger.info( "doc_id backfill complete: rewrote %d/%d payloads from int to str", rewritten, @@ -237,7 +306,9 @@ async def get_qdrant_client() -> AsyncQdrantClient: # Existing collections may pre-date the doc_id normalization / # payload-index work. Backfill before creating the index so the # index covers every point. - await _backfill_doc_id_to_string(_qdrant_client, collection_name) + await _backfill_doc_id_to_string( + _qdrant_client, collection_name, expected_dimension + ) await _ensure_keyword_payload_indexes(_qdrant_client, collection_name) else: diff --git a/tests/unit/search/test_search_result.py b/tests/unit/search/test_search_result.py index e29ed496..95eece1e 100644 --- a/tests/unit/search/test_search_result.py +++ b/tests/unit/search/test_search_result.py @@ -23,7 +23,7 @@ def _make_point(point_id, payload, score=0.5): def test_search_result_rrf_score_in_range(): """Test SearchResult accepts RRF scores in [0.0, 1.0] range.""" result = SearchResult( - id=1, + id="1", doc_type="note", title="Test Note", excerpt="Test excerpt", @@ -37,7 +37,7 @@ def test_search_result_rrf_score_in_range(): def test_search_result_rrf_score_at_lower_bound(): """Test SearchResult accepts RRF score at lower bound (0.0).""" result = SearchResult( - id=1, + id="1", doc_type="note", title="Test Note", excerpt="Test excerpt", @@ -51,7 +51,7 @@ def test_search_result_rrf_score_at_lower_bound(): def test_search_result_rrf_score_at_upper_bound(): """Test SearchResult accepts RRF score at upper bound (1.0).""" result = SearchResult( - id=1, + id="1", doc_type="note", title="Test Note", excerpt="Test excerpt", @@ -71,7 +71,7 @@ def test_search_result_dbsf_score_above_one(): """ # Typical DBSF score when both systems agree result = SearchResult( - id=1, + id="1", doc_type="note", title="Highly Relevant Note", excerpt="Contains keywords and is semantically similar", @@ -88,7 +88,7 @@ def test_search_result_dbsf_score_edge_case(): Maximum DBSF score with 2 systems: 1.0 (dense) + 1.0 (sparse) = 2.0 """ result = SearchResult( - id=1, + id="1", doc_type="note", title="Perfect Match", excerpt="Perfect semantic and keyword match", @@ -103,7 +103,7 @@ def test_search_result_negative_score_raises_error(): """Test SearchResult rejects negative scores.""" with pytest.raises(ValueError) as exc_info: SearchResult( - id=1, + id="1", doc_type="note", title="Test Note", excerpt="Test excerpt", @@ -118,7 +118,7 @@ def test_search_result_negative_score_raises_error(): def test_search_result_with_metadata(): """Test SearchResult with optional metadata field.""" result = SearchResult( - id=1, + id="1", doc_type="note", title="Test Note", excerpt="Test excerpt", @@ -136,7 +136,7 @@ def test_search_result_with_metadata(): def test_search_result_with_chunk_offsets(): """Test SearchResult with chunk offset information.""" result = SearchResult( - id=1, + id="1", doc_type="note", title="Test Note", excerpt="matching chunk text", diff --git a/tests/unit/vector/test_qdrant_client.py b/tests/unit/vector/test_qdrant_client.py index 2ec93ec9..92314034 100644 --- a/tests/unit/vector/test_qdrant_client.py +++ b/tests/unit/vector/test_qdrant_client.py @@ -23,12 +23,32 @@ from qdrant_client.http.exceptions import UnexpectedResponse from qdrant_client.models import PayloadSchemaType from nextcloud_mcp_server.vector.qdrant_client import ( + _DOC_ID_BACKFILL_SENTINEL_ID, _KEYWORD_PAYLOAD_FIELDS, _backfill_doc_id_to_string, _ensure_keyword_payload_indexes, ) +def _empty_collection_info() -> SimpleNamespace: + """Stand-in for a CollectionInfo with no payload indexes yet. + + Tests for _ensure_keyword_payload_indexes only read ``payload_schema`` + off the result. None / empty dict both signal "no indexes" — use {} + here to match the production-code default. + """ + return SimpleNamespace(payload_schema={}) + + +def _backfill_dimension() -> int: + """Vector dimension for sentinel writes in backfill tests. + + Any positive int is fine — the sentinel point is never read by the + test bodies, only the upsert call site is asserted. + """ + return 4 + + def _make_unexpected(status_code: int, body: bytes) -> UnexpectedResponse: """Build a real UnexpectedResponse for raise_for_status-style branches.""" return UnexpectedResponse( @@ -58,6 +78,7 @@ def _record(point_id: int | str, doc_id: int | str | None) -> SimpleNamespace: async def test_ensure_keyword_payload_indexes_creates_each_field(mocker): """Happy path: every field in _KEYWORD_PAYLOAD_FIELDS gets a KEYWORD index.""" client = mocker.AsyncMock() + client.get_collection.return_value = _empty_collection_info() await _ensure_keyword_payload_indexes(client, "test-collection") @@ -74,6 +95,37 @@ async def test_ensure_keyword_payload_indexes_creates_each_field(mocker): client.create_payload_index.assert_has_awaits(expected_calls, any_order=False) +@pytest.mark.unit +async def test_ensure_keyword_payload_indexes_skips_fields_already_indexed( + mocker, caplog +): + """Routine restart path: existing payload indexes are silently skipped. + + Without the pre-fetch, every restart logs `Created KEYWORD payload + index on ''` for every field — noise that hides genuinely + interesting first-time-creation lines. With the pre-fetch, no log + fires and no Qdrant write round-trip happens for already-indexed + fields. + """ + client = mocker.AsyncMock() + client.get_collection.return_value = SimpleNamespace( + payload_schema={"doc_id": object()} + ) + + with caplog.at_level("INFO", logger="nextcloud_mcp_server.vector.qdrant_client"): + await _ensure_keyword_payload_indexes(client, "test-collection") + + # Only the two missing fields are created. + assert client.create_payload_index.await_count == 2 + created_fields = { + c.kwargs["field_name"] for c in client.create_payload_index.await_args_list + } + assert created_fields == {"user_id", "doc_type"} + # No INFO log fires for the already-indexed field. + info_messages = [r.getMessage() for r in caplog.records if r.levelname == "INFO"] + assert not any("doc_id" in m for m in info_messages), info_messages + + @pytest.mark.unit async def test_ensure_keyword_payload_indexes_logs_400_as_warning(mocker, caplog): """Any 400 from create_payload_index is logged at WARNING and skipped. @@ -84,6 +136,7 @@ async def test_ensure_keyword_payload_indexes_logs_400_as_warning(mocker, caplog remaining fields still get indexed. """ client = mocker.AsyncMock() + client.get_collection.return_value = _empty_collection_info() client.create_payload_index.side_effect = [ _make_unexpected( 400, @@ -112,6 +165,7 @@ async def test_ensure_keyword_payload_indexes_logs_non_400_as_error(mocker, capl The loop still continues so the remaining fields get attempted. """ client = mocker.AsyncMock() + client.get_collection.return_value = _empty_collection_info() client.create_payload_index.side_effect = [ _make_unexpected(500, b'{"status":{"error":"internal server error"}}'), None, @@ -138,17 +192,20 @@ async def test_ensure_keyword_payload_indexes_logs_non_400_as_error(mocker, capl async def test_backfill_clean_collection_makes_no_writes(mocker, caplog): """A collection with only str doc_ids triggers zero set_payload calls. - Verifies idempotency: a second pass over an already-migrated collection - is a no-op modulo the read. + Verifies the no-write path: scroll runs, no payloads need rewriting, + and a sentinel is written so subsequent restarts can short-circuit. """ client = mocker.AsyncMock() + client.retrieve.return_value = [] # No sentinel — backfill must run client.scroll.return_value = ( [_record(1, "abc"), _record(2, "def")], None, ) with caplog.at_level("INFO", logger="nextcloud_mcp_server.vector.qdrant_client"): - await _backfill_doc_id_to_string(client, "test-collection") + await _backfill_doc_id_to_string( + client, "test-collection", _backfill_dimension() + ) client.set_payload.assert_not_awaited() completion_logs = [ @@ -158,10 +215,53 @@ async def test_backfill_clean_collection_makes_no_writes(mocker, caplog): assert "0/2" in completion_logs[0] +@pytest.mark.unit +async def test_backfill_skips_when_sentinel_present(mocker, caplog): + """If the sentinel exists, retrieve() returns it and the scroll is skipped. + + This is the routine-restart fast path: the migration already ran on a + previous start, so we avoid the O(N) scroll entirely. + """ + client = mocker.AsyncMock() + client.retrieve.return_value = [SimpleNamespace(id=_DOC_ID_BACKFILL_SENTINEL_ID)] + + with caplog.at_level("DEBUG", logger="nextcloud_mcp_server.vector.qdrant_client"): + await _backfill_doc_id_to_string( + client, "test-collection", _backfill_dimension() + ) + + client.scroll.assert_not_awaited() + client.set_payload.assert_not_awaited() + client.upsert.assert_not_awaited() + debug_msgs = [r.getMessage() for r in caplog.records if r.levelname == "DEBUG"] + assert any("sentinel" in m and "skipping" in m for m in debug_msgs), debug_msgs + + +@pytest.mark.unit +async def test_backfill_writes_sentinel_after_successful_scroll(mocker): + """Successful backfill writes a sentinel point so future restarts skip.""" + client = mocker.AsyncMock() + client.retrieve.return_value = [] # No sentinel — backfill must run + client.scroll.return_value = ([_record(1, "abc")], None) + + await _backfill_doc_id_to_string(client, "test-collection", _backfill_dimension()) + + # Single upsert with the sentinel UUID + migration marker payload. + assert client.upsert.await_count == 1 + upsert_kwargs = client.upsert.await_args.kwargs + assert upsert_kwargs["collection_name"] == "test-collection" + assert upsert_kwargs["wait"] is True + points = upsert_kwargs["points"] + assert len(points) == 1 + assert points[0].id == _DOC_ID_BACKFILL_SENTINEL_ID + assert points[0].payload == {"_migration_marker": "doc_id_v1"} + + @pytest.mark.unit async def test_backfill_rewrites_int_doc_ids_to_str(mocker): """Mixed int/str payload across two scroll pages: only ints get rewritten.""" client = mocker.AsyncMock() + client.retrieve.return_value = [] # Two scroll calls: batch 1 is mixed and reports a next_offset; batch 2 # is mixed with next_offset=None to terminate. client.scroll.side_effect = [ @@ -169,7 +269,7 @@ async def test_backfill_rewrites_int_doc_ids_to_str(mocker): ([_record(3, 200), _record(4, "def")], None), ] - await _backfill_doc_id_to_string(client, "test-collection") + await _backfill_doc_id_to_string(client, "test-collection", _backfill_dimension()) # One set_payload per *unique* int value — point 1 (100) and point 3 # (200) are in different batches with different values, so two calls. @@ -196,6 +296,7 @@ async def test_backfill_batches_points_with_same_doc_id(mocker): backfill should issue one set_payload call covering the chunk batch. """ client = mocker.AsyncMock() + client.retrieve.return_value = [] client.scroll.side_effect = [ ( [ @@ -208,7 +309,7 @@ async def test_backfill_batches_points_with_same_doc_id(mocker): ), ] - await _backfill_doc_id_to_string(client, "test-collection") + await _backfill_doc_id_to_string(client, "test-collection", _backfill_dimension()) # All three int-payload points share doc_id=42, so a single call covers them. assert client.set_payload.await_count == 1 @@ -224,12 +325,15 @@ async def test_backfill_batches_points_with_same_doc_id(mocker): async def test_backfill_emits_completion_log(mocker, caplog): """Backfill logs final rewritten/scanned counts at INFO.""" client = mocker.AsyncMock() + client.retrieve.return_value = [] client.scroll.side_effect = [ ([_record(1, 7), _record(2, "x")], None), ] with caplog.at_level("INFO", logger="nextcloud_mcp_server.vector.qdrant_client"): - await _backfill_doc_id_to_string(client, "test-collection") + await _backfill_doc_id_to_string( + client, "test-collection", _backfill_dimension() + ) completion_logs = [ r.getMessage() for r in caplog.records if "backfill complete" in r.getMessage() @@ -243,11 +347,12 @@ async def test_backfill_emits_completion_log(mocker, caplog): async def test_backfill_handles_none_payload(mocker): """A point with payload=None is skipped without crashing.""" client = mocker.AsyncMock() + client.retrieve.return_value = [] client.scroll.side_effect = [ ([_record(1, None), _record(2, 99)], None), ] - await _backfill_doc_id_to_string(client, "test-collection") + await _backfill_doc_id_to_string(client, "test-collection", _backfill_dimension()) # Only the int doc_id at point 2 was rewritten; the None-payload point was skipped. assert client.set_payload.await_count == 1 @@ -263,6 +368,7 @@ async def test_backfill_handles_none_payload(mocker): async def test_backfill_handles_payload_with_explicit_none_doc_id(mocker): """A payload of {doc_id: None, ...} is skipped just like payload=None.""" client = mocker.AsyncMock() + client.retrieve.return_value = [] # Build the record manually to distinguish payload=None from payload={"doc_id": None}. point_with_explicit_none = SimpleNamespace( id=1, payload={"doc_id": None, "doc_type": "file"} @@ -271,7 +377,7 @@ async def test_backfill_handles_payload_with_explicit_none_doc_id(mocker): ([point_with_explicit_none, _record(2, 99)], None), ] - await _backfill_doc_id_to_string(client, "test-collection") + await _backfill_doc_id_to_string(client, "test-collection", _backfill_dimension()) # Only the int doc_id at point 2 was rewritten; the explicit-None payload was skipped. assert client.set_payload.await_count == 1