refactor(search): address PR #750 round 10 review feedback

- Tighten verify_search_results signature: client: Any → NextcloudClientProtocol
- Collapse 3 copy-pasted lock-justification comments to a single-line pointer
- Add logger.debug timing around the verify_search_results call site
- Add logger.debug timing around the unbounded news.get_items fetch
- Rename SemanticSearchResponse.dropped_count → dropped_document_count to make
  the chunks-vs-documents unit asymmetry explicit at the API boundary
- Drop unreachable duplicate 409 branch in WebDAVClient.move_resource

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

nextcloud_mcp_server/client/webdav.py

@@ -483,13 +483,6 @@ class WebDAVClient(BaseNextcloudClient):
                     "status_code": 409,
                     "message": "Parent directory of destination doesn't exist",
                 }
-                logger.debug(
-                    f"Parent directory of destination '{destination_path}' doesn't exist"
-                )
-                return {
-                    "status_code": 409,
-                    "message": "Parent directory of destination doesn't exist",
-                }
             else:
                 logger.error(
                     f"HTTP error moving resource from '{source_path}' to '{destination_path}': {e}"

nextcloud_mcp_server/models/semantic.py

@@ -86,18 +86,18 @@ class SemanticSearchResponse(BaseResponse):
             "Number of search result chunks that passed verify-on-read "
             "access checks (ADR-019). Equals len(verified_results) before "
             "trimming to limit. Sized in chunks (result rows), NOT in "
-            "unique documents — pair with dropped_count carefully: "
+            "unique documents — see dropped_document_count for the "
-            "dropped_count is sized in unique (doc_id, doc_type) pairs."
+            "per-document counterpart."
         ),
     )
-    dropped_count: int = Field(
+    dropped_document_count: int = Field(
         default=0,
         description=(
             "Number of unique (doc_id, doc_type) pairs dropped as ghost "
             "records during verify-on-read (ADR-019). A short result page "
-            "(len(results) < limit) combined with a non-zero dropped_count "
+            "(len(results) < limit) combined with a non-zero "
-            "indicates ghost density rather than scarcity of relevant "
+            "dropped_document_count indicates ghost density rather than "
-            "content."
+            "scarcity of relevant content."
         ),
     )
 

nextcloud_mcp_server/search/verification.py

@@ -32,7 +32,6 @@ Failure policy:
 
 import logging
 from collections.abc import Awaitable, Callable
-from typing import Any
 
 import anyio
 from anyio.abc import TaskGroup
@@ -77,10 +76,7 @@ async def _verify_notes(
     results: list[SearchResult],
     semaphore: anyio.Semaphore,
 ) -> set[int | str]:
-    # Mutated by inner check() tasks under the task group below. Safe
+    # safe: cooperative concurrency, no lock needed (see verify_search_results)
-    # without a lock: anyio is cooperative, .add() is not an await
-    # point, so two tasks cannot race on the same write. Same rationale
-    # as accessible_by_type in verify_search_results.
     accessible: set[int | str] = set()
 
     async def check(result: SearchResult) -> None:
@@ -135,10 +131,7 @@ async def _verify_files(
     results: list[SearchResult],
     semaphore: anyio.Semaphore,
 ) -> set[int | str]:
-    # Mutated by inner check() tasks under the task group below. Safe
+    # safe: cooperative concurrency, no lock needed (see verify_search_results)
-    # without a lock: anyio is cooperative, .add() is not an await
-    # point, so two tasks cannot race on the same write. Same rationale
-    # as accessible_by_type in verify_search_results.
     accessible: set[int | str] = set()
 
     async def check(result: SearchResult) -> None:
@@ -209,10 +202,7 @@ async def _verify_deck_cards(
     results: list[SearchResult],
     semaphore: anyio.Semaphore,
 ) -> set[int | str]:
-    # Mutated by inner check() tasks under the task group below. Safe
+    # safe: cooperative concurrency, no lock needed (see verify_search_results)
-    # without a lock: anyio is cooperative, .add() is not an await
-    # point, so two tasks cannot race on the same write. Same rationale
-    # as accessible_by_type in verify_search_results.
     accessible: set[int | str] = set()
 
     async def check(result: SearchResult) -> None:
@@ -334,7 +324,13 @@ async def _verify_news_items(
             # fetching every item the user has access to. See the news caveat
             # in docs/configuration.md (Verify-on-Read) for the latency
             # tradeoff and follow-up paths.
+            news_fetch_start = anyio.current_time()
             items = await client.news.get_items(batch_size=-1, get_read=True)
+            logger.debug(
+                "News fetch for verification took %.2fs (%d item(s) returned)",
+                anyio.current_time() - news_fetch_start,
+                len(items),
+            )
         except HTTPStatusError as e:
             # If the News API itself is gone (app disabled, user lost access),
             # treat *all* requested items as inaccessible. Eviction will reclaim.
@@ -411,7 +407,7 @@ def get_supported_doc_types() -> set[str]:
 
 
 async def verify_search_results(
-    client: Any,
+    client: NextcloudClientProtocol,
     results: list[SearchResult],
     *,
     evict_on_missing: bool = True,

nextcloud_mcp_server/server/semantic.py

@@ -94,9 +94,9 @@ def configure_semantic_tools(mcp: FastMCP):
               (sized in chunks; counted before trimming to ``limit``, so it
               can exceed ``len(results)`` when a doc has multiple matching
               chunks).
-            - dropped_count: unique ``(doc_id, doc_type)`` pairs evicted as
+            - dropped_document_count: unique ``(doc_id, doc_type)`` pairs
-              ghost records during this search (sized in documents, not
+              evicted as ghost records during this search (sized in
-              chunks).
+              documents, not chunks).
         """
         settings = get_settings()
         client = await get_client(ctx)
@@ -197,12 +197,19 @@ def configure_semantic_tools(mcp: FastMCP):
             eviction_task_group = (
                 ctx.request_context.lifespan_context.eviction_task_group
             )
+            verification_start = anyio.current_time()
             verified_results, dropped_count = await verify_search_results(
                 client,
                 all_results,
                 eviction_task_group=eviction_task_group,
             )
             verified_chunk_count = len(verified_results)
+            logger.debug(
+                "Verification completed in %.2fs: kept %d chunk(s), dropped %d doc(s)",
+                anyio.current_time() - verification_start,
+                verified_chunk_count,
+                dropped_count,
+            )
             search_results = verified_results[:limit]
 
             # Convert SearchResult objects to SemanticSearchResult for response.
@@ -362,7 +369,7 @@ def configure_semantic_tools(mcp: FastMCP):
                 total_found=len(results),
                 search_method=f"bm25_hybrid_{fusion}",
                 verified_chunk_count=verified_chunk_count,
-                dropped_count=dropped_count,
+                dropped_document_count=dropped_count,
             )
 
         except ValueError as e: