From b7479b0d07afb26bda2e3164af1204d5c7e0261a Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Fri, 5 Jun 2026 01:23:13 +0200 Subject: [PATCH] docs(review): correct reconcile docstring + clarify scanner rename comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review follow-ups (PR #857), both documentation-only: - sharing_state.py: reconcile_document_path docstring no longer claims it returns False when no real points exist — it returns True and the set_payload is a Qdrant-side no-op (callers discard the return value). - scanner.py: reword the rename-reconcile comment to state the precise reason (modified_at stable so not re-queued; path may be stale from a rename) rather than the loose "dedup miss / etag changed" phrasing. Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/vector/scanner.py | 19 ++++++++++--------- nextcloud_mcp_server/vector/sharing_state.py | 8 +++++--- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/nextcloud_mcp_server/vector/scanner.py b/nextcloud_mcp_server/vector/scanner.py index 755fbf8a..2fbe2109 100644 --- a/nextcloud_mcp_server/vector/scanner.py +++ b/nextcloud_mcp_server/vector/scanner.py @@ -590,15 +590,16 @@ async def scan_user_documents( elif existing_metadata is not None and not existing_metadata.get( "is_placeholder", False ): - # Unchanged content (not re-queued) but the file may have - # been renamed/moved: a rename keeps the fileid while - # changing the path, and the dedup miss here means the - # etag changed without a modified_at bump. Refresh the - # stale path/title metadata without re-embedding. No-op - # when the path is unchanged. Skip placeholders: reconcile - # only touches real chunks, so a not-yet-indexed file would - # just incur a 0-point set_payload (the real index writes - # the current path anyway). + # Reached only on the rename-with-stable-mtime path: a + # fresh modified_at would have set needs_indexing, and an + # etag dedup hit would have continued above -- so here the + # content wasn't re-queued (modified_at stable) yet the + # stored path may be stale from a rename/move (the fileid + # is unchanged). Refresh path/title without re-embedding; + # reconcile_document_path no-ops when the path matches. + # Skip placeholders: reconcile only touches real chunks, so + # a not-yet-indexed file would just incur a 0-point + # set_payload (the real index writes the current path). try: await reconcile_document_path( file_id, diff --git a/nextcloud_mcp_server/vector/sharing_state.py b/nextcloud_mcp_server/vector/sharing_state.py index 190b91d2..df160e54 100644 --- a/nextcloud_mcp_server/vector/sharing_state.py +++ b/nextcloud_mcp_server/vector/sharing_state.py @@ -188,9 +188,11 @@ async def reconcile_document_path( rewrites ``file_path`` and the derived ``title`` on every real chunk via a single metadata-only ``set_payload`` (no re-fetch, no re-embed). - No-op (returns False) when the path is unchanged or no real points exist yet - (filter matches nothing). A legacy point with no stored ``file_path`` is - treated as changed, backfilling both fields. + Returns False (no write attempted) only when the path is unchanged or empty. + When the path differs it returns True after issuing the ``set_payload``; that + write is itself a Qdrant-side no-op if no real chunks exist yet (e.g. only a + placeholder), which the callers tolerate. A legacy point with no stored + ``file_path`` is treated as changed, backfilling both fields. """ if not current_path or stored_path == current_path: return False