Add a path_prefix filter to semantic search, honoured on both the MCP tool and
the dense-only visualization/API paths through the shared filter contract.
- build_base_filter_conditions: append FieldCondition(file_path,
MatchText(path_prefix)) when set. file_path is only on doc_type == "file"
points, so a non-empty path_prefix implicitly restricts to files.
- Promote path_prefix to an explicit keyword param on the SearchAlgorithm ABC
and both algorithms; thread it through nc_semantic_search (blank ⇒ no filter),
the /api/v1 search endpoints, and the viz route.
- Add a file_path TEXT payload index to _PAYLOAD_INDEX_FIELDS (no content
re-index; idempotent startup migration). MatchText tokenizes on server Qdrant
and matches by substring on local/embedded qdrant-client — both serve folder
scoping.
- Update ADR-027 (Phase 2 implemented; readiness table; semantics note). Tests.
Refs ADR-027 Phase 2. Deck #177.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a modified_after/modified_before date-range filter to semantic search,
honoured on both the MCP tool path (BM25HybridSearchAlgorithm) and the
dense-only visualization/API path (SemanticSearchAlgorithm) through one shared
contract.
- Promote modified_after/modified_before to explicit keyword params on the
SearchAlgorithm ABC and both concrete algorithms; factor the shared
placeholder+ownership+doc_type+date filter into
access_filter.build_base_filter_conditions so new filters land in one place.
- nc_semantic_search: accept RFC 3339 / ISO 8601 (or Unix seconds) bounds via
utils.validation.parse_modified_timestamp; Annotated/Field constraints on the
numeric args; explicit McpError guard for after > before. Thread the parsed
bounds through the cross-app and per-doc_type dispatch.
- /api/v1 search endpoints + viz route parse the same formats and 400 on bad or
inverted ranges.
- Add a modified_at INTEGER payload index to _PAYLOAD_INDEX_FIELDS; the
idempotent _ensure_payload_indexes() startup path migrates existing
collections with no content re-index.
- Update ADR-027 to resolve the review feedback (validation placement, shared
algorithm contract, deferral of nc_semantic_search_answer, payload index,
RFC-3339-at-the-boundary rationale). Add unit tests.
Refs ADR-027. Deck #177.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🟡 Important: nc_semantic_search's include_context branch did not forward
accessible_owners to get_chunk_with_context, so context expansion for shared
files stayed self-only, found nothing in Qdrant, and silently fell back to the
plain excerpt. Forward accessible_owners (the per-file file_accessible_by_id
gate still enforces access).
🟡 Performance: auth/viz_routes.py's multi-doc_type branch sorted but did not
cap the candidate pool before verify-on-read, so N doc_types × limit*2 went
into verification (N× the Nextcloud round-trips). Cap to limit*2 after the
sort, matching server/semantic.py and the cross-app branch.
Also clear the SonarCloud gate (new_duplicated_lines_density 5.1% > 3%) the
ACL wiring introduced: extract the duplicated /api/v1 client-resolution +
owner-expansion + verify-on-read block from unified_search/vector_search into a
shared _search_with_acl helper, define a constant for the repeated
"Nextcloud host not configured" literal (S1192), and reword the access_filter
move_to_end comment so it isn't misread as commented-out code (S125) while
adding the other-owner count to its debug log (review nits).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1. Don't log unverified result titles: both search algorithms logged top-5
titles at DEBUG before verify-on-read; with owner-level share expansion the
unverified set can contain other users' docs. Algorithms now log a count
only; the verifying callers (server/semantic, viz_routes, api/visualization)
log verified titles after verify-on-read.
2. Cross-user FILE chunk context: get_chunk_with_context + the Qdrant chunk
helpers now take accessible_owners and use build_ownership_filter. For files
the expanded scope is honoured only after a per-file file_accessible_by_id
check (accessible_owners is owner-level, so the gate prevents a one-file
share recipient from reading any of the owner's cached chunks). note/deck/
news stay self-only (per-user APIs) — a documented gap. Both chunk endpoints
pass accessible_owners.
3. Algorithm usage: SemanticSearchAlgorithm is not dead (it backs the dense-only
option on the viz/API surfaces); added a clarifying comment in server/
semantic.py. Additionally wired accessible_owners + verify-on-read into the
/api/v1 search routes (unified_search, vector_search) so the astrolabe
surface is ACL-aware too — degrading gracefully to self-only/unverified for
non-provisioned callers instead of 401.
4. Overlapping conditions: build_ownership_filter no longer lists self in the
owner_id MatchAny branch (self is already covered by the user_id branch);
the owner_id branch carries only the OTHER owners.
Tests: build_ownership_filter dedup + chunk-bbox filter-shape updates; new
ACL-aware get_indexed_doc_types, cached-chunk lookup, and end-to-end cross-user
file chunk-context (recipient gets the chunk, non-recipient denied) tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The vector index has always been strictly per-user: every Qdrant payload
carries a `user_id` and the search filter is `user_id == querying_user`.
A file Alice indexed cannot be discovered by Bob even if she has shared
it with him — Bob would have to re-index it under his own user_id to
make it searchable, which means duplicate index entries for every share
recipient.
Switch to ownership-with-ACL-expansion:
- New `nextcloud_mcp_server.search.access_filter` module:
- `list_accessible_owners(sharing_client, user_id)` calls the OCS
Sharing API (`shared_with_me=true`) and returns
`{user_id} ∪ {uid_owner of each share}`. Fails open to `[user_id]`
so a misbehaving Sharing API doesn't black-hole search.
- `build_ownership_filter(user_id, accessible_owners)` returns a
Qdrant `Filter` whose `should` branch matches either the new
`owner_id IN accessible_owners` field or the legacy `user_id` field.
The legacy branch keeps points indexed before this change reachable
without a migration backfill.
- Indexer payload (`vector/processor.py`) now writes `owner_id` alongside
`user_id`. `DocumentTask` gains an optional `owner_id` field; today the
scanner always runs as the owner so the processor falls back to
`user_id`, but the field is plumbed so a future shared-with-me crawler
can set the true owner without reshaping the payload contract.
- `SemanticSearchAlgorithm.search` and `BM25HybridSearchAlgorithm.search`
accept `accessible_owners` via kwargs and use the new ownership filter.
Default behaviour with no kwarg is unchanged (self-only).
- Both user-facing callers — the MCP tool path (`server/semantic.py`) and
the visualization Starlette route (`auth/viz_routes.py`) — compute
`accessible_owners` from the authenticated Nextcloud client before
invoking the search algorithm. Eviction, scanner deletion, placeholder,
and chunk-context paths intentionally keep the legacy `user_id`
semantics (those are "operations on a specific user's records", not
cross-user reads).
- 10 new unit tests in `tests/unit/search/test_access_filter.py` cover
self-only default, owner expansion, dedup, fallback fields, OCS
failure, and the legacy `should`-branch shape.
Pairs with cbcoutinho/astrolabe#89 — together they let an Astrolabe user
find content owners have shared with them without going through any
re-authorization flow or re-indexing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Production was logging two cascading classes of Qdrant errors against the
welcomed-malamute deployment:
1. HTTP 400 — "Bad request: Index required but not found for \"doc_id\" of
one of the following types: [keyword]". The collection was created via
create_collection() with no payload indexes, so any FieldCondition
filter on doc_id failed at the Qdrant layer (placeholder writes/reads,
eviction, search context lookups).
2. Compounding the missing index, producers wrote a mix of int and str
doc_ids: webhook_parser stringified node_id, scanner stringified note
IDs, news IDs, and deck card IDs — but the file scanner passed the
numeric file_id through unchanged. A keyword index would not have
covered both kinds even if it had existed.
This change:
- Normalizes doc_id to str at every producer site (scanner.py:459,
DocumentTask.doc_id, indexed_*_ids reads from Qdrant).
- Tightens str|int annotations to str across placeholder.py,
eviction.py, search/verification.py, search/context.py,
SearchResult.id, and the auth/api visualization endpoints.
- Defensive str() coercion on doc_id reads in semantic.py /
bm25_hybrid.py / vector/visualization.py for the transition window
before the backfill runs.
- Adds an idempotent startup migration in get_qdrant_client():
- _ensure_keyword_payload_indexes creates KEYWORD indexes for
doc_id, user_id, and doc_type (tolerates "already exists" 400s).
- _backfill_doc_id_to_string scrolls the collection once and rewrites
int doc_ids to str. Skipped after a quick sample shows no legacy
int payloads.
- Public API preserved: SemanticSearchResult.id stays int via explicit
int(r.id) narrowing in server/semantic.py — surfaces a TypeError with
actionable context if a future doc_type ships non-numeric ids.
- Documents the startup migration in docs/configuration.md.
Tests: 11 new unit tests in tests/unit/vector/test_qdrant_client.py
covering happy path / already-exists / unrelated-400 for the index
helpers, and sample-skip / mixed-batch rewrite / payload=None edge cases
for the backfill. 889 unit tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Six review items raised; four required code changes (#3, #4, #5, #6) and
two were resolved without code changes (#1 audit-only, #2 informational).
* search/verification.py — clarify the granularity asymmetry between the
whole-batch fail-open (structural API failure) and the per-item fail-open
(single bad stored doc_id). Future readers no longer need to derive why
the two paths have different blast radii from the code alone.
* models/semantic.py — `dropped_document_count` description now explicitly
notes that subtracting it from `verified_chunk_count` is not a meaningful
operation, since the two fields count different units (documents vs
chunks). Surfaces the unit mismatch where MCP clients actually see it.
* server/semantic.py — clarify the per-doc_type over-fetch comment so the
N×2 pre-merge Qdrant cost (vs the cross-app branch's 1×2) is explicit
rather than implied by "same 2× over-fetch budget".
* tests/unit/search/test_verification.py — add four new 429 unit tests
(notes/news/files/deck) mirroring the existing 5xx-keeps pattern. Locks
in that `_is_definitive_404_or_403` returns False for 429 so a future
refactor cannot accidentally treat rate-limit responses as permanent
revocations.
Audit confirmation for review item #1: all four `WebDAVClient.get_file_info`
call sites already handle the new `HTTPStatusError`-on-404 contract
(verification.py:156, tests/integration/test_rag.py:139,
tests/unit/client/test_webdav.py:153/190). No silent breakage internal to
this repo.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three minor fixes from the round-11 review on PR #750:
- bm25_hybrid.py:209 — Comment said `doc_id` is `int (notes) or str (files)`,
which is backwards. Notes, news_items, and deck_cards are stored as `str`
(scanner.py:241, 666, 867); files are stored as `int` (scanner.py:425).
Updated to point readers at scanner.py as the source of truth.
- verification.py:338 — Lowered the News-API 403/404 log line from `info`
to `debug`. The News app being uninstalled or disabled is a predictable
operational state (matching the other verifiers' debug-on-not-found
paths), so this should not generate operator-dashboard noise. Transient
errors immediately below stay at `warning` because they're unexpected.
- semantic.py:809 — `nc_get_vector_sync_status` was reading
`document_receive_stream` via `getattr(..., None)`, but the attribute is
guaranteed-defined on both `AppContext` and `OAuthAppContext` (as a
field with `None` default). The defensive `getattr` masked typos that
the eviction_task_group access at semantic.py:197-199 deliberately
surfaces. Switched to direct access; the `if … is None:` value-check
below is preserved (the attribute can legitimately be None before sync
starts).
Items deliberately deferred (with rationale in the plan file):
- News verifier semaphore-hold during get_items (reviewer: "not required
here, just worth tracking"; ADR already lists follow-ups).
- Hardcoded 2× over-fetch / VERIFICATION_OVERFETCH (TODO already in code).
- Integration test for the real Qdrant eviction filter (reviewer marked
low-priority; type-preservation chain is unit-tested).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- 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>
- Add concurrency-safety comments to per-verifier accessible sets in
_verify_notes/_verify_files/_verify_deck_cards. Same rationale as
accessible_by_type in verify_search_results: anyio is cooperative,
set.add() is not an await point.
- Document 401 exclusion in _is_definitive_404_or_403 (treated as
transient because it usually signals expired credentials, not
permanent denial).
- Note multi-user compounding in the news verifier semaphore comment:
N concurrent users hold N slots out of the shared budget.
- Log inaccessible doc ids with a type tag (e.g. "int:42" vs "str:42")
so ghost-record logs disambiguate id types.
- Type the BatchVerifier alias and the four verifier function signatures
with NextcloudClientProtocol instead of Any (algorithms.py exposes
the right interface; the protocol is runtime_checkable).
- Surface verified_chunk_count vs dropped_count semantics in the
nc_semantic_search tool docstring Returns block (chunks vs unique
documents).
- Add comments to the two max_concurrent=20 sites in server/semantic.py
noting they are intentionally distinct from
settings.verification_concurrency (different request phases).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Rename `verified_count` → `verified_chunk_count` to make the count
granularity explicit at the field name (chunks vs unique docs).
- News verifier now fails open *per-item* on non-numeric stored doc_ids
(matches notes/files/deck shape); a single bad id no longer rescues
definitively-missing siblings from eviction.
- Update note-verifier integration test to use string doc_ids end-to-end
to match production storage (scanner.py:241 stringifies note ids).
- Add regression test for the closed-task-group race guard in
`verify_search_results` so the RuntimeError swallow is locked in.
- Convert remaining f-string logger calls in `server/semantic.py` to
lazy %-style formatting (per repo convention).
- Document `evict_on_missing` as a developer/test flag (no env var) and
flag the `get_file_info` 404→raise contract change in its docstring.
- Add a TODO(ADR-019) breadcrumb for the hardcoded 2× over-fetch so
future tuning has a clear hook.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes out the remaining nits flagged in the round-6 review.
Critical:
- _verify_files contract comment now enumerates all None-return cases
(404 + malformed PROPFIND XML) and documents the false-eviction
trade-off; self-healing via re-indexing recovers
- int(r.id) cast at the SemanticSearchResult boundary now raises a
TypeError with explicit doc_type/value context instead of bubbling
up as an opaque "Search failed: ..." McpError
Design observations:
- nc_semantic_search_answer docstring documents the per-note
round-trip cost from the post-verification race guard
- News verification latency hint added to configuration.md
- SemanticSearchResponse exposes verified_count + dropped_count so
short result pages on high-ghost-density indexes are
distinguishable from genuine scarcity. verify_search_results now
returns (kept, dropped_count); production caller and tests updated
Minor:
- Comment clarifies the .get() fallback in verify_search_results is
defensive only (run_verifier always populates the entry)
- Eviction task-group guard narrowed from except Exception to
except RuntimeError (the only documented failure mode of
TaskGroup.start_soon on a closed group)
- Indexer logs a warning when a deck_card task is missing
board_id/stack_id, surfacing data-quality issues at index time
rather than at verification time
- New unit test covers the news verifier's non-numeric-id fail-open
path (one bad doc_id keeps the entire batch)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- _verify_deck_cards: hoist int(board_id|stack_id|doc_id) out of the generic
except Exception into an explicit try/except (TypeError, ValueError) before
the network call, mirroring _verify_news_items. Malformed payloads now log
a specific warning instead of "unexpected error".
- _verify_news_items: add TODO(perf) above the get_items(batch_size=-1) call
to mark the known fetch-all cost as a future profiling target.
- SemanticSearchResult.id: revert from int|str back to int. The internal
SearchResult.id stays int|str for forward-compat; the MCP response model
narrows at the boundary. server/semantic.py casts r.id to int when
constructing the response so future string-id types fail loudly here
instead of silently widening the public API.
- nc_semantic_search: replace the terse "extra for access filtering" comment
with an ADR-019 NOTE block explaining the 2x over-fetch trade-off and the
ghost-density under-delivery case (self-heals via lazy eviction).
- tests/integration/test_verify_on_read.py: extend the module docstring to
call out that only the note verifier is exercised against real Nextcloud,
while file/deck_card/news_item are unit-only — documenting the suite split
for future contributors.
- ADR-019: rewrite "Module shape", "Verifier registry", example verifier,
and "Deduplication" sections to match the shipped BatchVerifier interface
(was per-id Verifier in the original draft). Add a "Why batch?" paragraph
explaining the design choice. Update implementation checklist — every
item is now [x] with corrected verifier names (plural) and the eviction
module path (vector/eviction.py).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements fire-and-forget eviction (ADR-019 §"Lazy eviction"): the
search response no longer waits on Qdrant deletes, instead spawning
evict() on a long-lived lifespan-owned task group. Falls back to inline
eviction in modes without vector sync and in unit tests.
Also: harden _verify_news_items against non-numeric ids (fail open
instead of crashing the verifier); document the get_file_info None-on-404
contract; add INDEXED_DOC_TYPES single source of truth in vector/scanner.py
referenced by the CI-guard test; write a Verify-on-Read Latency Budget
section in docs/configuration.md covering the unbounded news.get_items
fetch. Closes the two remaining ADR-019 implementation checklist items.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Cap all_results to limit*2 after sort in the per-doc_types branch of
nc_semantic_search to bound over-verification (was unbounded N-types).
- Switch BatchVerifier from (client, doc_ids, user_id) to (client, results,
semaphore). Verifiers now read file paths and deck board/stack ids from
SearchResult.metadata instead of doing fresh Qdrant scrolls — eliminates
one duplicate round-trip per file/deck-card verification.
- Bound per-id verification concurrency with a shared anyio.Semaphore
(default 20, matching server/semantic.py context-expansion convention).
Prevents httpx pool exhaustion / rate limiting on large search pages.
- Propagate stack_id from Qdrant payload to SearchResult.metadata in both
bm25_hybrid.py and semantic.py (board_id was already propagated).
- Drop now-unused _resolve_file_path / _resolve_deck_metadata helpers.
- Drop redundant int(d) in requested predicate from _verify_news_items.
- Rewrite eviction comment to be honest about inline (not background)
execution and the resulting latency coupling.
- ADR-019 status: Proposed -> Accepted.
- Add news property to NextcloudClientProtocol.
- Widen SearchResult.id and SemanticSearchResult.id to int | str to match
BatchVerifier signature and document support for future string-id types.
- Flip openWorldHint to True on nc_semantic_search_answer (it calls into
Nextcloud via nc_semantic_search).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The vector index lags Nextcloud (5-min webhook cron + scanner interval),
producing ghost records for deleted/unshared documents until the next
reconciliation. Verify each unique document against Nextcloud at query
time, drop inaccessible results, and lazily evict the corresponding
Qdrant points.
Per-doc_type batch verifiers: notes/files/deck cards run concurrently
per id; news items use a single fetch + intersect to avoid the per-item
fetch-all amplification. Transient errors fail open (keep result, log
warning) — only definitive 4xx drops. Multiple chunks of the same doc
collapse to one verification call.
Wired into nc_semantic_search before the limit trim and before context
expansion. nc_semantic_search_answer's per-note re-fetch retained as a
sub-second race guard since verification now happens upstream.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Many identity providers (AWS Cognito, Okta, Azure AD) reject or mishandle
colons in OAuth scope names. This migrates all custom scopes from
`resource:action` to `resource.action` format (e.g., `notes:read` →
`notes.read`), which is universally accepted and aligns with industry
conventions (Microsoft, Google).
Includes Alembic migration 004 for stored scope strings and ADR-024
documenting the rationale and RFC references.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Enable ruff PLC0415 rule for all source files (tests excluded via
per-file-ignores). Move 136 inline imports to top-level across 33 files.
8 imports suppressed with noqa for legitimate reasons: circular
dependencies (client/__init__.py, context.py), optional dependency
guards (app.py document processors, auth/userinfo_routes.py), and
post-env-setup imports (smithery_main.py).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace direct os.getenv() calls with get_settings().vector_sync_enabled
to ensure consistent behavior with both VECTOR_SYNC_ENABLED (deprecated)
and ENABLE_SEMANTIC_SEARCH environment variables.
Also add webhook management documentation guide.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Adds comprehensive vector search support for Nextcloud Deck cards,
including semantic search indexing, chunk preview in the vector viz UI,
and proper deep linking to cards.
**Vector Search Indexing**
- Add deck_card scanning in scanner.py (scan_deck_cards function)
- Index cards from non-archived, non-deleted boards
- Store metadata: board_id, board_title, stack_id, stack_title, card_type, duedate, owner
- Content structure: title + "\n\n" + description (matches indexing format)
- Incremental sync based on lastModified timestamp
- Deletion tracking with grace period
**Vector Visualization Support**
- Add deck_card handler in context.py for chunk preview expansion
- Include board_id in search result metadata (bm25_hybrid.py, semantic.py)
- Expose metadata in viz_routes.py JSON responses
- Update vector-viz.js to construct proper Deck URLs: /apps/deck/board/{board_id}/card/{card_id}
- Update vector_viz.html filter label from "Deck" to "Deck Cards"
**Bug Fixes**
- Skip soft-deleted boards (deletedAt > 0) to prevent 403 Forbidden errors
- Applies to scanner, processor, and context expansion code paths
- Deck API returns deleted boards but rejects stack access with 403
**Testing**
- Add integration tests in test_deck_vector_search.py:
- test_deck_card_semantic_search: Filtered search with doc_type="deck_card"
- test_deck_card_appears_in_cross_app_search: Cross-app search includes deck cards
- test_deck_card_chunk_context: Chunk context fetching for viz preview
**Documentation**
- Update README.md: Add Deck cards to semantic search feature list
- Update semantic-search-architecture.md: Document deck_card support
- Update nc_semantic_search tool documentation
**Type Safety**
- Fix type narrowing for page_boundaries (could be None) using cast()
- Fix scanner.py payload None check for type safety
Resolves vector search for Deck cards across indexing, search, and visualization.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Address all reviewer comments from PR #387:
1. ✅ Add unit tests for annotations (tests/server/test_annotations.py)
- 10 comprehensive test functions validating all annotation patterns
- Tests for titles, read-only, destructive, idempotent operations
- Validates specific ADR-017 decisions (webdav write, semantic search)
- Cross-category consistency checks
2. ✅ Fix nc_webdav_write_file idempotency classification
- Changed from idempotentHint=False to idempotentHint=True
- Rationale: Uses HTTP PUT without version control
- Writing same content to same path = same end state (idempotent)
3. ✅ Fix semantic search openWorldHint inconsistency
- Changed from openWorldHint=False to openWorldHint=True
- Rationale: Consistent with other Nextcloud tools
- Nextcloud is external to MCP server (indexed data is implementation detail)
4. ✅ Update ADR-017 with resolved decisions
- Converted Open Questions to Resolved Questions
- Added detailed rationale for webdav write and semantic search
- Updated status from Proposed to Implemented
- Added decision timeline with dates
5. ✅ Add MCP Tool Annotations guidelines to CLAUDE.md
- Comprehensive section with code examples for all patterns
- Key principles documented (idempotency, destructive, open world)
- References ADR-017 for detailed rationale
All OAuth tools verified to have proper annotations (oauth_tools.py lines 686-751).
- Increase sampling timeout from 30s to 300s in semantic.py to accommodate
slower local LLMs like Ollama
- Refactor RAG integration tests to support multiple providers (ollama,
openai, anthropic, bedrock)
- Remove unnecessary embedding_provider fixture since MCP server handles
embeddings internally
- Add --provider flag via tests/integration/conftest.py
- Add provider_fixtures.py with factory functions for generation providers
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
The Smithery scanner was reporting "0 tools" despite the server returning
valid tool definitions. Root cause: the server was returning SSE-formatted
responses (event: message\ndata: {...}) which the scanner couldn't parse.
Changes:
- Add json_response=True to FastMCP for Smithery stateless mode
- Clean up verbose docstring examples in semantic.py and webdav.py
The MCP spec allows both SSE and plain JSON responses for HTTP transport.
Setting json_response=True returns Content-Type: application/json with
plain JSON-RPC instead of text/event-stream with SSE format.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Implements optional context expansion for semantic search results that
fetches adjacent chunks (N-1 and N+1) from Qdrant to provide before/after
context. Removes configurable chunk overlap (default 200 chars) to avoid
duplicate text appearing in both context and excerpt.
Key changes:
- Add include_context and context_chars parameters to nc_semantic_search
and nc_semantic_search_answer tools
- Implement Qdrant cache fast path for chunk retrieval (avoids re-fetching
and re-parsing documents, especially important for PDFs)
- Add _get_chunk_by_index_from_qdrant() to fetch adjacent chunks
- Remove chunk overlap from before_context (last N chars) and after_context
(first N chars) to prevent duplicate text
- Fetch context in parallel with anyio.Semaphore (max 20 concurrent)
- Pass through page_number from SearchResult to SemanticSearchResult
- Remove document-level deduplication (keep chunk-level dedup from algorithm)
Context expansion is opt-in via include_context=true parameter. When enabled:
- Populates has_context_expansion, marked_text, before_context, after_context
- Adds truncation flags when context exceeds context_chars limit
- Falls back to document fetch for legacy data with truncated excerpts
Related: nextcloud_mcp_server/search/context.py:87-382,
nextcloud_mcp_server/server/semantic.py:161-255
Track character offsets (start_offset, end_offset) for each chunk in vector
database metadata, enabling precise chunk highlighting in visualization pane.
Changes:
- processor.py: Store chunk_start_offset and chunk_end_offset in Qdrant metadata
- processor.py: Added metadata_version=2 to indicate position tracking support
- search/semantic.py: Return chunk positions from search results
- server/semantic.py: Expose chunk positions in API responses (SemanticSearchResult)
Enables viz pane to:
1. Display exact matched chunk with surrounding context
2. Highlight the precise portion of text that matched the query
3. Build user trust by showing what the RAG system actually retrieved
Position tracking uses ChunkWithPosition dataclass from document_chunker.py
which provides character-accurate offsets in the original document.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
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>
BREAKING CHANGE: Search algorithms now require Qdrant to be populated.
Vector sync must be enabled and documents indexed for search to work.
- Keyword and fuzzy search now query Qdrant scroll API for title/excerpt
- Remove inefficient Nextcloud API fetching pattern
- Add optional Nextcloud verification for security
- Deduplicate by (doc_id, doc_type) tuple, keeping chunk_index=0
- Align with document processor pattern that already stores text in Qdrant
Implements ADR-012 by adding multi-algorithm support to the MCP tool.
Key changes:
- Added algorithm parameter: "semantic"|"keyword"|"fuzzy"|"hybrid" (default: "hybrid")
- Added weight parameters for hybrid mode configuration
- Replaced direct Qdrant/embedding calls with search module abstractions
- Updated docstring to describe all four algorithms
- Simplified implementation: ~50 lines vs ~150 lines (67% reduction)
- Better error handling for missing vector sync
Algorithm selection:
- semantic: Pure vector similarity (requires VECTOR_SYNC_ENABLED=true)
- keyword: Token-based matching with weighted title/content scoring
- fuzzy: Character overlap for typo tolerance
- hybrid: RRF fusion with configurable weights (default: 0.5/0.3/0.2)
Backward compatibility:
- Tool name unchanged (nc_semantic_search)
- New parameters have sensible defaults
- Existing clients get hybrid search automatically (better than pure semantic)
- search_method field in response reflects actual algorithm used
Weight validation:
- Performed in HybridSearchAlgorithm constructor
- Must sum to ≤1.0 and all non-negative
- At least one weight must be > 0
- Clear error messages on validation failure
Next: Update viz pane to use same algorithms
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
This PR enables safe switching between embedding models and multi-server
deployments by implementing auto-generated Qdrant collection names based on
deployment ID and model name.
## Problem
Previously, all deployments used a single hardcoded collection name
"nextcloud_content", which caused two critical issues:
1. **Dimension mismatches when switching models**: Changing
OLLAMA_EMBEDDING_MODEL (e.g., nomic-embed-text at 768D → all-minilm at
384D) would cause runtime errors as vectors couldn't be inserted into a
collection with incompatible dimensions.
2. **Collection collisions in multi-server setups**: Multiple MCP servers
sharing a single Qdrant instance would overwrite each other's data,
making horizontal scaling impossible.
## Solution
### Auto-Generated Collection Naming
Collections are now automatically named using the pattern:
\`{deployment-id}-{model-name}\`
**Deployment ID**: Uses \`OTEL_SERVICE_NAME\` if configured (and not default
value), otherwise falls back to \`hostname\` for simple Docker deployments.
**Model Name**: From \`OLLAMA_EMBEDDING_MODEL\` with path separators sanitized.
**Examples**:
- \`my-mcp-server-nomic-embed-text\` (with OTEL_SERVICE_NAME=my-mcp-server)
- \`mcp-container-all-minilm\` (simple Docker, hostname=mcp-container)
**Override**: Users can still set \`QDRANT_COLLECTION\` explicitly to bypass
auto-generation for backward compatibility.
### Dimension Validation
Added startup validation that checks collection dimensions match the
embedding service. If a mismatch is detected, the server fails fast with a
clear error message explaining:
- Expected vs actual dimensions
- Likely cause (model change)
- Solutions (delete collection, use different name, or revert model)
### Improved Sampling Error Handling
Enhanced MCP sampling rejection handling to treat user rejections as normal
behavior rather than errors:
- **User rejections** ("rejected", "denied") → INFO log, no traceback
- **Unsupported clients** → INFO log, no traceback
- **Other MCP errors** → WARNING log, no traceback
- **Unexpected errors** → ERROR log WITH traceback
This aligns with the MCP specification where clients SHOULD prompt users for
approval/denial of sampling requests.
## Changes
### Core Implementation
- **nextcloud_mcp_server/config.py**: Added \`get_collection_name()\` method
with deployment ID detection and model name sanitization
- **nextcloud_mcp_server/vector/qdrant_client.py**: Dimension validation on
collection open with helpful error messages
- **nextcloud_mcp_server/vector/{scanner,processor}.py**: Updated to use
\`get_collection_name()\`
- **nextcloud_mcp_server/auth/userinfo_routes.py**: Vector sync status uses
\`get_collection_name()\`
- **nextcloud_mcp_server/server/semantic.py**:
- Updated semantic search tools to use \`get_collection_name()\`
- Improved sampling rejection error handling (McpError vs Exception)
### Documentation
- **docs/semantic-search-architecture.md**: New comprehensive architecture
document (557 lines) covering background sync, semantic search flow, RAG
implementation, and deployment modes
- **docs/configuration.md**: Added detailed "Qdrant Collection Naming"
section with examples and multi-server deployment guidance
- **docker-compose.yml**: Added comments explaining collection naming behavior
- **README.md**: Updated semantic search descriptions to clarify
experimental status, Notes-only support, and infrastructure requirements
## Migration Guide
**For existing single-server deployments:**
Option 1 (Recommended): Use explicit collection name for continuity
\`\`\`bash
QDRANT_COLLECTION=nextcloud_content # Keep existing collection
\`\`\`
Option 2: Allow auto-generation and re-embed
\`\`\`bash
# Remove QDRANT_COLLECTION override
# New collection will be created based on deployment ID + model
# Requires re-embedding all documents (may take time)
\`\`\`
**For new multi-server deployments:**
Set unique OTEL service names per server:
\`\`\`bash
# Server 1
OTEL_SERVICE_NAME=mcp-prod
OLLAMA_EMBEDDING_MODEL=nomic-embed-text
# → Collection: "mcp-prod-nomic-embed-text"
# Server 2
OTEL_SERVICE_NAME=mcp-staging
OLLAMA_EMBEDDING_MODEL=nomic-embed-text
# → Collection: "mcp-staging-nomic-embed-text"
\`\`\`
## Benefits
✅ **Safe model switching**: Each model gets its own collection, preventing
dimension mismatch errors
✅ **Multi-server support**: Multiple MCP servers can share one Qdrant
instance without conflicts
✅ **Clear ownership**: Collection names show which deployment and model owns
the data
✅ **Better error messages**: Dimension validation provides actionable
guidance
✅ **Backward compatible**: Existing deployments can continue using
\`QDRANT_COLLECTION\` override
## Testing
Validated with:
- Single-server deployments (default hostname-based naming)
- Multi-server deployments (OTEL service name-based naming)
- Model switching scenarios (dimension validation)
- Collection override scenarios (backward compatibility)
Next steps: Testing various Ollama embedding models to investigate optimal
chunk sizes and performance characteristics.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Replace asyncio.Queue with anyio.create_memory_object_stream() throughout
the vector sync system for better library consistency and improved shutdown
semantics.
## Changes Made
**scanner.py**:
- Changed parameter type from `asyncio.Queue` to `MemoryObjectSendStream[DocumentTask]`
- Replaced all `await document_queue.put()` calls with `await send_stream.send()`
- Wrapped scanner loop in `async with send_stream:` context manager for automatic cleanup
- Updated log messages: "Queued" → "Sent"
- Removed `import asyncio` (no longer needed)
**processor.py**:
- Changed parameter type from `asyncio.Queue` to `MemoryObjectReceiveStream[DocumentTask]`
- Replaced `asyncio.wait_for(document_queue.get(), timeout=1.0)` with `anyio.fail_after(1.0)` + `await receive_stream.receive()`
- Removed all `document_queue.task_done()` calls (not needed with streams)
- Added `anyio.EndOfStream` exception handling for graceful shutdown when scanner closes
- Removed `import asyncio` (no longer needed)
**app.py**:
- Removed `import asyncio` from top-level imports
- Added `from anyio.streams.memory import MemoryObjectReceiveStream, MemoryObjectSendStream`
- Updated AppContext dataclass:
- Replaced `document_queue: Optional[asyncio.Queue]` with:
- `document_send_stream: Optional[MemoryObjectSendStream]`
- `document_receive_stream: Optional[MemoryObjectReceiveStream]`
- Updated `app_lifespan_basic()`:
- Replaced `asyncio.Queue(maxsize=...)` with `anyio.create_memory_object_stream(max_buffer_size=...)`
- Pass `send_stream` to scanner_task
- Pass `receive_stream.clone()` to each processor_task (enables multiple consumers)
- Updated AppContext yield to include both streams
- Updated `starlette_lifespan()`:
- Same changes as app_lifespan_basic for streamable-http transport
- Removed `import asyncio as asyncio_module` (no longer needed)
- Updated app.state storage to use send_stream and receive_stream
**semantic.py**:
- Updated `nc_get_vector_sync_status()` tool:
- Access `document_receive_stream` instead of `document_queue` from lifespan context
- Use `stream_stats.current_buffer_used` instead of `queue.qsize()` for pending count
- More reliable metrics (qsize() was not guaranteed accurate)
## Benefits
1. **Library Consistency**: Pure anyio throughout codebase (was mixing asyncio.Queue with anyio.Event and anyio.create_task_group)
2. **Graceful Shutdown**: `async with send_stream:` automatically closes stream on exit, signaling EndOfStream to all processors
3. **Better Timeout Handling**: `anyio.fail_after()` is more idiomatic than `asyncio.wait_for()`
4. **Stream Cloning**: Easy to add multiple consumers via `receive_stream.clone()`
5. **Better Statistics**: `.statistics()` provides accurate buffer metrics (qsize() was unreliable)
6. **Type Safety**: Separate send/receive types prevent accidental misuse
7. **No task_done() tracking**: Streams handle completion automatically
## Testing
- ✅ All 69 unit tests passing
- ✅ All 5 smoke tests passing
- ✅ No regressions in functionality
- ✅ Graceful shutdown behavior improved
## References
- https://anyio.readthedocs.io/en/stable/why.html#queue-fix
- https://anyio.readthedocs.io/en/stable/streams.html#memory-object-streams🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
This implements ADR-009, which documents the decision to use a generic
`semantic:read` OAuth scope instead of requiring all app-specific scopes
for semantic search functionality.
Changes:
- Created new `nextcloud_mcp_server/models/semantic.py` with semantic search models
- SemanticSearchResult (with new doc_type field for multi-app support)
- SemanticSearchResponse
- SamplingSearchResponse
- VectorSyncStatusResponse
- Created new `nextcloud_mcp_server/server/semantic.py` with semantic search tools
- nc_semantic_search (renamed from nc_notes_semantic_search)
- nc_semantic_search_answer (renamed from nc_notes_semantic_search_answer)
- nc_get_vector_sync_status (renamed from nc_notes_get_vector_sync_status)
- All tools now use @require_scopes("semantic:read") instead of "notes:read"
- Updated `nextcloud_mcp_server/server/notes.py`
- Removed semantic search tools (moved to semantic.py)
- Removed semantic search model imports
- Removed unused MCP imports (ModelHint, ModelPreferences, etc.)
- Updated `nextcloud_mcp_server/models/notes.py`
- Removed semantic search models (moved to semantic.py)
- Updated `nextcloud_mcp_server/app.py`
- Import configure_semantic_tools
- Register semantic tools when VECTOR_SYNC_ENABLED=true
- Updated `nextcloud_mcp_server/server/__init__.py`
- Export configure_semantic_tools
- Updated tests
- tests/integration/test_sampling.py: Use new tool names
- tests/unit/test_response_models.py: Import from semantic.py, add doc_type field
Architecture:
- Semantic search is now a cross-app feature, not tied to Notes
- Uses dual-phase authorization: semantic:read scope + per-document verification
- Supports future multi-app indexing (notes, calendar, deck, files, contacts)
Test results:
- All 69 unit tests passing
- All 5 smoke tests passing
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
