refactor: Optimize Nextcloud access verification with centralized filtering
Move access verification from individual search algorithms to final output stage, eliminating redundant API calls and improving performance. ## Changes **New:** - `search/verification.py`: Centralized verification using anyio task groups - Deduplicates results by (doc_id, doc_type) before verification - Verifies all unique documents in parallel using structured concurrency - Filters out inaccessible documents in single pass **Modified Search Algorithms:** - `search/semantic.py`: Removed _deduplicate_and_verify() and _verify_document_access() - `search/keyword.py`: Removed _verify_access() and parallel verification - `search/fuzzy.py`: Removed _verify_access() and parallel verification - `search/hybrid.py`: Removed nextcloud_client parameter passing All algorithms now return unverified results from Qdrant payload. **Modified Output Stages:** - `server/semantic.py`: Added verify_search_results() call after search - `auth/viz_routes.py`: Added verify_search_results() call after search Both endpoints now verify access once at final stage with deduplication. ## Performance Impact **Before:** - Hybrid mode (limit=10): 30 API calls (10 per algorithm × 3 algorithms) - Single algorithm: 10-20 API calls (with verification buffer) **After:** - Hybrid mode (limit=10): 10 API calls (deduplicated verification) - Single algorithm: 10 API calls (deduplicated verification) **Performance Gain:** 3x reduction in API calls for hybrid search ## Architecture Benefits - **Separation of concerns**: Algorithms handle scoring, output stage handles security - **Deduplication**: Each document verified exactly once - **Parallel execution**: All verifications run concurrently via anyio task groups - **Consistency**: Same verification logic across MCP tools and viz endpoints 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Chris Coutinho
@@ -3,17 +3,12 @@
|
||||
import logging
|
||||
from typing import Any
|
||||
|
||||
from httpx import HTTPStatusError
|
||||
from qdrant_client.models import FieldCondition, Filter, MatchValue
|
||||
|
||||
from nextcloud_mcp_server.config import get_settings
|
||||
from nextcloud_mcp_server.embedding import get_embedding_service
|
||||
from nextcloud_mcp_server.observability.metrics import record_qdrant_operation
|
||||
from nextcloud_mcp_server.search.algorithms import (
|
||||
NextcloudClientProtocol,
|
||||
SearchAlgorithm,
|
||||
SearchResult,
|
||||
)
|
||||
from nextcloud_mcp_server.search.algorithms import SearchAlgorithm, SearchResult
|
||||
from nextcloud_mcp_server.vector.qdrant_client import get_qdrant_client
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@@ -48,21 +43,22 @@ class SemanticSearchAlgorithm(SearchAlgorithm):
|
||||
user_id: str,
|
||||
limit: int = 10,
|
||||
doc_type: str | None = None,
|
||||
nextcloud_client: NextcloudClientProtocol | None = None,
|
||||
**kwargs: Any,
|
||||
) -> list[SearchResult]:
|
||||
"""Execute semantic search using vector similarity.
|
||||
|
||||
Returns unverified results from Qdrant. Access verification should be
|
||||
performed separately at the final output stage using verify_search_results().
|
||||
|
||||
Args:
|
||||
query: Natural language search query
|
||||
user_id: User ID for filtering
|
||||
limit: Maximum results to return
|
||||
doc_type: Optional document type filter (currently only "note" supported)
|
||||
nextcloud_client: NextcloudClient for access verification
|
||||
doc_type: Optional document type filter
|
||||
**kwargs: Additional parameters (score_threshold override)
|
||||
|
||||
Returns:
|
||||
List of SearchResult objects ranked by similarity score
|
||||
List of unverified SearchResult objects ranked by similarity score
|
||||
|
||||
Raises:
|
||||
McpError: If vector sync is not enabled or search fails
|
||||
@@ -118,7 +114,7 @@ class SemanticSearchAlgorithm(SearchAlgorithm):
|
||||
|
||||
logger.info(
|
||||
f"Qdrant returned {len(search_response.points)} results "
|
||||
f"(before deduplication and access verification)"
|
||||
f"(before deduplication)"
|
||||
)
|
||||
|
||||
if search_response.points:
|
||||
@@ -126,47 +122,11 @@ class SemanticSearchAlgorithm(SearchAlgorithm):
|
||||
top_scores = [p.score for p in search_response.points[:3]]
|
||||
logger.debug(f"Top 3 similarity scores: {top_scores}")
|
||||
|
||||
# Deduplicate by document ID (multiple chunks per document)
|
||||
results = await self._deduplicate_and_verify(
|
||||
search_response.points, limit, nextcloud_client
|
||||
)
|
||||
|
||||
logger.info(
|
||||
f"Returning {len(results)} results after deduplication and access verification"
|
||||
)
|
||||
if results:
|
||||
result_details = [
|
||||
f"{r.doc_type}_{r.id} (score={r.score:.3f}, title='{r.title}')"
|
||||
for r in results[:5] # Show top 5
|
||||
]
|
||||
logger.debug(f"Top results: {', '.join(result_details)}")
|
||||
|
||||
return results
|
||||
|
||||
async def _deduplicate_and_verify(
|
||||
self,
|
||||
points: list[Any],
|
||||
limit: int,
|
||||
nextcloud_client: NextcloudClientProtocol | None,
|
||||
) -> list[SearchResult]:
|
||||
"""Deduplicate results by (doc_id, doc_type) and verify access.
|
||||
|
||||
Supports multiple document types with dispatch to appropriate client methods.
|
||||
Deduplication is now by (doc_id, doc_type) tuple to handle cases where
|
||||
the same ID might exist across different document types.
|
||||
|
||||
Args:
|
||||
points: Qdrant search results
|
||||
limit: Maximum results to return
|
||||
nextcloud_client: NextcloudClient for access verification (optional)
|
||||
|
||||
Returns:
|
||||
List of SearchResult objects
|
||||
"""
|
||||
seen_docs = set() # Track (doc_id, doc_type) tuples
|
||||
# Deduplicate by (doc_id, doc_type) - multiple chunks per document
|
||||
seen_docs = set()
|
||||
results = []
|
||||
|
||||
for result in points:
|
||||
for result in search_response.points:
|
||||
doc_id = int(result.payload["doc_id"])
|
||||
doc_type = result.payload.get("doc_type", "note")
|
||||
doc_key = (doc_id, doc_type)
|
||||
@@ -177,99 +137,30 @@ class SemanticSearchAlgorithm(SearchAlgorithm):
|
||||
|
||||
seen_docs.add(doc_key)
|
||||
|
||||
# Verify access via Nextcloud API if client provided
|
||||
# Dispatch to appropriate client based on doc_type
|
||||
verified_result = None
|
||||
|
||||
if nextcloud_client:
|
||||
verified_result = await self._verify_document_access(
|
||||
nextcloud_client, doc_id, doc_type, result
|
||||
)
|
||||
|
||||
if verified_result:
|
||||
results.append(verified_result)
|
||||
elif not nextcloud_client:
|
||||
# No access verification, return result directly
|
||||
results.append(
|
||||
SearchResult(
|
||||
id=doc_id,
|
||||
doc_type=doc_type,
|
||||
title=result.payload["title"],
|
||||
excerpt=result.payload["excerpt"],
|
||||
score=result.score,
|
||||
metadata={
|
||||
"chunk_index": result.payload.get("chunk_index"),
|
||||
"total_chunks": result.payload.get("total_chunks"),
|
||||
},
|
||||
)
|
||||
# Return unverified results (verification happens at output stage)
|
||||
results.append(
|
||||
SearchResult(
|
||||
id=doc_id,
|
||||
doc_type=doc_type,
|
||||
title=result.payload.get("title", "Untitled"),
|
||||
excerpt=result.payload.get("excerpt", ""),
|
||||
score=result.score,
|
||||
metadata={
|
||||
"chunk_index": result.payload.get("chunk_index"),
|
||||
"total_chunks": result.payload.get("total_chunks"),
|
||||
},
|
||||
)
|
||||
)
|
||||
|
||||
if len(results) >= limit:
|
||||
break
|
||||
|
||||
logger.info(f"Returning {len(results)} unverified results after deduplication")
|
||||
if results:
|
||||
result_details = [
|
||||
f"{r.doc_type}_{r.id} (score={r.score:.3f}, title='{r.title}')"
|
||||
for r in results[:5] # Show top 5
|
||||
]
|
||||
logger.debug(f"Top results: {', '.join(result_details)}")
|
||||
|
||||
return results
|
||||
|
||||
async def _verify_document_access(
|
||||
self,
|
||||
nextcloud_client: NextcloudClientProtocol,
|
||||
doc_id: int,
|
||||
doc_type: str,
|
||||
qdrant_result: Any,
|
||||
) -> SearchResult | None:
|
||||
"""Verify user has access to a document via Nextcloud API.
|
||||
|
||||
Dispatches to appropriate client method based on document type.
|
||||
|
||||
Args:
|
||||
nextcloud_client: Client for API access
|
||||
doc_id: Document ID
|
||||
doc_type: Document type ("note", "file", "calendar", etc.)
|
||||
qdrant_result: Original Qdrant search result
|
||||
|
||||
Returns:
|
||||
SearchResult if access verified, None if access denied or error
|
||||
"""
|
||||
try:
|
||||
if doc_type == "note":
|
||||
note = await nextcloud_client.notes.get_note(doc_id)
|
||||
return SearchResult(
|
||||
id=doc_id,
|
||||
doc_type="note",
|
||||
title=qdrant_result.payload["title"],
|
||||
excerpt=qdrant_result.payload["excerpt"],
|
||||
score=qdrant_result.score,
|
||||
metadata={
|
||||
"category": note.get("category", ""),
|
||||
"chunk_index": qdrant_result.payload["chunk_index"],
|
||||
"total_chunks": qdrant_result.payload["total_chunks"],
|
||||
},
|
||||
)
|
||||
elif doc_type == "file":
|
||||
# Future: verify file access when files are indexed
|
||||
logger.info(
|
||||
f"File {doc_id} found in search but file verification not yet implemented"
|
||||
)
|
||||
return None
|
||||
elif doc_type == "calendar":
|
||||
# Future: verify calendar access when calendar events are indexed
|
||||
logger.info(
|
||||
f"Calendar event {doc_id} found in search but calendar verification not yet implemented"
|
||||
)
|
||||
return None
|
||||
else:
|
||||
logger.warning(
|
||||
f"Unknown document type '{doc_type}' for doc_id {doc_id}"
|
||||
)
|
||||
return None
|
||||
|
||||
except HTTPStatusError as e:
|
||||
if e.response.status_code in (403, 404):
|
||||
# User lost access or document deleted
|
||||
logger.debug(f"Skipping {doc_type} {doc_id}: {e.response.status_code}")
|
||||
return None
|
||||
else:
|
||||
# Log other errors but continue processing
|
||||
logger.warning(
|
||||
f"Error verifying access to {doc_type} {doc_id}: {e.response.status_code}"
|
||||
)
|
||||
return None
|
||||
|
||||
Reference in New Issue
Block a user