Non-blocking consistency fixes:
- scanner.py: skip re-queuing a mail_message whose placeholder status is
"failed" (mirrors the file scanner's permanent-failure guard); the
modified_at branch still retries once the message changes.
- search/context.py: return None on an empty get_message payload during context
expansion, mirroring the processor's index-time empty-payload guard.
- tests: cover the non-numeric mailbox_id fail-open branch in the verifier.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
No blockers raised; hardening + clarity:
- client/mail.py: URL-encode the caller-supplied attachment_id
(quote(..., safe="")) — defense-in-depth against path traversal.
- server/mail.py: measure attachment content in UTF-8 bytes (not characters)
for the size cap and the sentinel message.
- scanner.py: bound _mail_cap_logged (insertion-ordered dict + oldest-first
eviction at 50k, mirroring _consent_backstop_done) so the cap-log dedup set
can't leak in a long-running multi-tenant process; reword the cap log to not
imply MAIL_SCAN_MAX_PER_MAILBOX is operator-tunable (it's the Mail OCS max).
- models/mail.py: comment why GetAttachmentResponse doesn't nest MailAttachment
(different OCS endpoint shape).
- mail_content.py: document format_mail_addresses' empty-entry skip contract.
_potentially_deleted doc_type-in-key remains tracked as Deck #376 (pre-existing
cross-cutting; reviewer confirmed deferral).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address PR #935 round-3 review:
- search/verification.py: rewrite _verify_mail_messages to batch by mailbox.
get_message triggers a server-side IMAP body fetch, so per-result verify
issued one IMAP FETCH per hit; now it calls the DB-cached list_messages once
per mailbox (mailbox_id comes from the Qdrant payload via result.metadata)
and intersects — O(unique mailboxes) light calls instead of O(results) IMAP.
- vector/mail_content.py: include Cc/Bcc in the indexed text so recipient
queries match; move MAIL_SCAN_MAX_PER_MAILBOX here (shared by scanner index
window + verifier presence window) with a note that it equals the Mail OCS
per-request max (100), so it's a fixed constant not a config knob.
- client/mail.py: clamp list_messages limit to 1..100 at the client layer.
- tests: add test_mail_content.py (exact-layout contract for build_mail_content);
rewrite the mail verifier tests for the batch-per-mailbox shape.
Left as-is: ValidationError isn't caught in the list-endpoint tools — consistent
with nc_notes_*/nc_deck_* and not a regression.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- server/mail.py: guard nc_mail_get_message against an empty OCS payload so
MailMessage(**{}) can't raise an uncaught ValidationError (returns a clean
'not found' instead).
- server/mail.py: cap inlined attachment content at MAX_ATTACHMENT_CONTENT_BYTES
(5 MiB), replacing oversized bodies with a sentinel so a large attachment
can't blow up the MCP response.
- client/mail.py: harden the OCS meta statuscode parse against a non-numeric
value (treat as success) instead of letting int() raise an uncaught
ValueError.
- scanner.py: log the newest-N cap hit once per (user, mailbox) at info level
(discoverable without flooding multi-tenant logs on every scan tick).
- tests: add incremental-sync scanner cases (new message queued, reappeared
message clears grace, deletion after grace expiry).
Deferred (tracked, card #376): include doc_type in the _potentially_deleted
grace-period key — a pre-existing cross-cutting collision the reviewer flagged
as a follow-up, not a blocker.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add read-only support for the Nextcloud Mail app, plus semantic indexing
of mail messages. The MCP server never speaks IMAP/POP3 itself: it calls
the Mail app's CSRF-free OCS API (/ocs/v2.php/apps/mail/api/...) with the
existing Basic-Auth app-password flow and an OCS-APIRequest header, and the
Mail app handles IMAP server-side.
- client/mail.py: MailClient (accounts, mailboxes, messages, message,
attachment), OCS-envelope aware.
- models/mail.py: Pydantic models with the API's camelCase aliases.
- server/mail.py: 5 read-only MCP tools (mail.read scope), registered in
AVAILABLE_APPS.
- Vector pipeline: new "mail_message" doc_type wired into scanner
(scan_mail_messages, newest-N per mailbox), processor (body -> markdown
embedding), per-id verifier, and context expansion.
- Tests: client API, model round-trips, verifier behavior; consent-backstop
test now derives its allowed set from INDEXED_DOC_TYPES.
- README + semantic-search docstrings updated.
Requires Mail 5.x / Nextcloud 32+ and a mail account configured in the
Mail app. Follow-up: astrolabe must advertise "mail_message" in its
enabled_doc_types capability for search under admin doc_type restriction.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A pathological PDF (a 206-page ChronoScan scan with ~3400 JBIG2/JPX images)
jammed a tenant's structured ingest worker in an infinite reprocess loop,
re-burning a 120s pymupdf4llm parse (and occasionally OOM-racing the 2Gi pod)
every few minutes.
Root cause: the per-user placeholder "failed" mark could not stop the loop. The
placeholder point ID is user-agnostic (uuid5("file:<doc_id>:placeholder")) but
the scanner's freshness gate, query, and status update all filter by user_id.
For a file visible to several users the single shared placeholder's user_id is
overwritten by whoever scanned last, so every other user's scan sees "no record"
and re-queues -- an N-user ping-pong that never honours the failed status.
Fix: when a parse fails terminally (no higher escalation tier available, e.g.
structured with OCR off) record a durable, content-addressed, user-agnostic
dead-letter marker (mirrors vector/sharing_state.py). The scanner consults it
tenant-wide for every user and skips re-queuing until the content (etag) OR the
escalation-tier set (tiers_sig -- e.g. OCR enabled) changes, so the document is
attempted once per content-version instead of forever.
- new vector/dead_letter.py: mark/is/clear, content-addressed marker carrying
is_placeholder=True (inherits search exclusion) + dead_letter=True
- escalation.escalation_tiers_signature(settings): retry-on-tier-change key
- processor: dead-letter terminal failures, clear on successful (re-)index
- scanner: user-agnostic is_dead_lettered skip beside claim_existing_index
- placeholder: exempt dead_letter markers from the orphan sweep (durability)
- metrics: astrolabe_document_dead_lettered_total{reason}
Deck #349.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The consent gate added three `_app_enabled(...) and is_doc_type_allowed(...)`
conditions to scan_user_documents, pushing its cognitive complexity over the
SonarQube threshold. Fold the pair into a _should_scan() helper (alongside the
earlier _enqueue_deletes refactor). Also document the accepted doc_types=None
per-type-query trade-off at the search consent gate (round-10 review item).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extract _mark_backstop_done() (overflow eviction + marker write) and
_backstop_delete_doc_type() (per-type scroll + enqueue) so
_enqueue_deletes_for_disabled_types drops from cognitive complexity 17 to well
under the 15 threshold. Behavior unchanged; tests still pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- vector_sync route: rename the response dict from `body` to `resp` so it no
longer shadows the request `body` (maintenance trap)
- scanner: comment the intentional files-vs-text purge timing asymmetry
- tests: add the all-text-types-disabled backstop case (empty allow-set)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- purge route: include a "failed" key in the 200 body listing requested doc
types that were not purged, so Astrolabe knows consent isn't yet enforced
for them (scanner backstop still catches up)
- tests: add @pytest.mark.unit / module-level pytestmark to the new test
modules so they run under `pytest -m unit`; add a partial-failure route test
- capabilities: comment why the cache is keyed per-user despite a global value
- semantic/scanner: doc/comment clarifications (sorted-order, eviction timing)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- tests: cover the process_document consent gate (drops an admin-disabled
index task with record_ingest_dropped("admin_disabled"); allows approved)
- scanner: _consent_backstop_done is now an insertion-ordered dict and evicts
the oldest entries to half capacity on overflow, so a bound hit re-fires the
backstop for only the oldest markers instead of the whole fleet at once
- semantic: reword the short-circuit log (consent, not installation)
- capabilities: comment why move_to_end is needed after an expired-key update
- test: assert the global purge delete-filter is owner-agnostic (doc_type only);
fix a pre-existing ty error on UnexpectedResponse(headers=None) in the file
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- app.py: register /api/v1/vector-sync/purge only when vector_sync_enabled, so
it returns 404 (not a 500 from get_qdrant_client) when sync is off
- scanner: bound _consent_backstop_done so a long-running multi-tenant process
with user churn can't grow it without limit (clears on overflow)
- purge route: distinct 400 for a missing doc_types key; enforce the admin
check even for an empty no-op request (destructive route)
- tests: missing-key 400, admin-gated empty no-op, non-admin empty 403
The _consent_narrowed_doc_types precondition is enforced by its non-Optional
frozenset[str] signature (ty rejects a None caller). The httpx.BasicAuth
SonarCloud hotspot matches the existing webhook routes (false positive,
credential from the app-password store) — left consistent for UI triage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- scanner: gate the consent backstop with a per-(user,doc_type) one-shot
marker so a standing admin-disable doesn't re-enqueue idempotent deletes
every scan tick; the marker clears when the type is re-enabled. Derive
_TEXT_BACKSTOP_DOC_TYPES from INDEXED_DOC_TYPES so new indexed types are
covered automatically
- semantic: extract _consent_narrowed_doc_types so the search-side narrowing
is unit-testable; add tests for restrict/intersect/disjoint/empty
- purge route: cap doc_types length (abuse guard) -> 400
- tests: one-shot + re-enable backstop, too-many-doc_types 400
Deferred (noted on PR): per-document allowed_doc_types call is cache-hot;
purge "last error wins" — both logged. SonarCloud broad-except hotspots are
deliberate (noqa BLE001), reviewable in the UI.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- purge route: 400 (not 500) on a valid-JSON non-object body
- scanner: backstop-purge admin-disabled note/news_item/deck_card points
(their deletion-tracking lives inside the skipped scan_* fns), mirroring the
files path; gated on a concrete allow-set so fail-open never deletes
- processor: record_ingest_dropped("admin_disabled") so consent-skipped index
tasks are observable/alertable
- app.py: list /api/v1/vector-sync/purge in the endpoints log line
- capabilities: drop empty-string doc types; return frozenset throughout
- purge: document the count-before-delete approximation
- tests: non-object body -> 400, ProvisioningRequiredError -> 428, cache TTL
expiry refetch, and the scanner consent backstop
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Consume the astrolabe.semantic_search capability as the source of truth for
which content sources an admin has approved for semantic search, and enforce
it independently of Astrolabe (this server queries Qdrant directly).
- capabilities.py: cached per-user reader for enabled_doc_types (TTL+LRU,
fail-open so older Astrolabe / transient OCS errors don't break search)
- semantic search: intersect requested doc_types with the allowed set;
restrict to the allowed set when none requested; short-circuit when empty
- scanner: skip disabled sources during discovery (files discovery yields
nothing when disabled, so the existing grace-period reconcile purges them)
- processor: drop near-real-time index tasks for disabled doc_types
(webhook events bypass the scanner gate); deletes always proceed
- vector/purge.py + POST /api/v1/vector-sync/purge: admin-only global
delete-by-doc_type, called by Astrolabe when a source is disabled so
consent is binding on data-at-rest
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Extract `_app_enabled` to a module-level helper so the gate predicate is
unit-tested directly instead of via an inline copy that could drift.
- Move `import logging` to module scope in test_scanner_app_gating.py.
- Harden `get_enabled_apps` OCS-envelope parsing (`X or {}` / `or []`) so a
present-but-null `ocs`/`data` coerces to empty instead of raising on
`None.get`; add parametrized malformed-envelope tests.
- Use https:// in the test request URL (SonarCloud S5332 hotspot).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The vector-sync scanner polled every indexed app (Notes, Files, News,
Deck) for every provisioned user on each scan cycle. When a user lacks
an app, its REST API returns 404; these were caught (indexing
continued) but flooded tenant logs with repeated 404s, scaling with
users x disabled-apps x scan-frequency and masking real failures.
Add NextcloudClient.get_enabled_apps(), which reads the per-user
/ocs/v2.php/core/navigation/apps endpoint (respects group
restrictions). Chosen over /cloud/capabilities because the News app
advertises no capability and never appears there.
scan_user_documents now resolves the enabled-app set once per cycle and
skips the Notes/News/Deck scans for apps the user lacks. Files stays
unconditional (core Tags API, not a 404 source). Detection failures
fall back to scanning every app (prior behaviour), so a transient
nav-endpoint blip never silently halts indexing; the per-app 404 guards
remain as the safety net.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Round-1 review follow-ups (PR #857):
- scanner.py: skip the rename-reconcile when the existing metadata point is a
placeholder. reconcile_document_path 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.
- test_sharing_state.py: add a dedup-hit case where the file was renamed AND the
user is new to the ACL, asserting both set_payload writes fire (file_path/title
and acl_principals).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The vector-sync pipeline derived an indexed file's display title from the
document's embedded metadata (e.g. a PDF's /Title), falling back to the
filename only when absent. That embedded title frequently disagrees with how
the user named the file in Nextcloud and is confusing in the astrolabe
vector-viz UI (a passive consumer of the `title` payload field).
For files, always derive the title from the Nextcloud filename via a shared
`file_title_from_path` helper. Notes/deck/news keep their metadata titles.
A rename/move in Nextcloud keeps the fileid (doc_id) and content (etag/mtime)
but changes the path, so both the dedup claim and the scanner freshness gate
skip re-embedding and the stored file_path/title go stale. Add
`reconcile_document_path`: a metadata-only set_payload that refreshes
file_path + title on the existing real chunks without re-fetch/re-embed.
Wire it into both skip paths:
- dedup hit (etag unchanged on rename) via claim_existing_index(current_path=...)
- scanner incremental skip (etag changed, mtime stable)
Both reuse already-fetched payloads, so steady-state scans add no extra
round-trip (reconcile is a no-op when the path is unchanged).
Refs: Deck #204
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The document processor crash-looped on one pathological PDF: pymupdf4llm's
table/graphics detection over a page with ~1M vector path items ballooned past
the 2 GiB pod limit. The parse ran in a thread, so nothing could interrupt or
memory-bound it -- a single bad file OOM-killed the whole pod.
Run the parse in an isolated worker subprocess (anyio.to_process, cancellable)
with an RLIMIT_AS memory cap and a wall-clock timeout, so a pathological file
fails THAT document instead of the pod (new document_processors/_isolation.py).
Also pass graphics_limit (default 5000) to to_markdown -- validated to cut the
known trigger page from 112 s to 23 s with bounded memory.
On a permanent parse failure the processor returns success=False (instead of
raising, which would retry 3x); vector/processor.py marks the placeholder
"failed" and skips indexing, and the scanner stops re-queuing failed placeholders
until the file changes -- so a doomed file no longer churns.
New per-tenant (per-pod env) settings: DOCUMENT_PDF_GRAPHICS_LIMIT,
DOCUMENT_PARSE_TIMEOUT_SECONDS, DOCUMENT_PARSE_MEM_LIMIT_MB. New metric
astrolabe_document_parse_failed_total{reason=timeout|oom|error} surfaces hard
failures that previously killed the process before any except ran.
First PR of the tiered document-processor effort (Deck #199); tier 0/1/3
pipeline tracked separately.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Starlette lifespan started `vector_sync_metrics_task` with undefined
names `task_producer` and `receive_stream`. Those locals only exist inside
the `_wire_vector_sync_state` helper; in the lifespan the transport is bound
as `ingest_transport`. The undefined reference raised `NameError`, which
aborted the background-sync task group and crashed startup in every
deployment mode ("Application startup failed. Exiting.").
Introduced by fbe70ecd ("feat: backend-agnostic vector-sync gauges").
Pass `ingest_transport.producer` / `ingest_transport.receive_stream` at both
call sites (single-user app.py:1791, OAuth/login-flow app.py:2012).
Also fix 10 pre-existing `ty` possibly-missing-attribute diagnostics: the
deck indexing code in scanner.py, processor.py and search/context.py reads
full-DeckCard-only fields (description, type, owner, etag, lastModified) off
`stack.cards`, typed `list[DeckCard | DeckCardSummary]`. Freshly-fetched
stacks from `get_stacks()` always hold full DeckCards (the summary
projection only happens in the tool layer), so narrow with
`cast(list[DeckCard], ...)` — matching the existing pattern in
server/deck.py.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a comment at the deletion-tracking scroll noting that a user who
gained access to a shared file via the tenant-wide dedup path (without
indexing it) is absent from the user_id-filtered indexed_file_ids, so the
grace-period sweep never enqueues a delete for them — their stale
acl_principals entry is reclaimed lazily by verify-on-read eviction.
Addresses review nit #3.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A file shared across many users — directly, or via a group folder shared
to a group — was parsed and embedded once per user. Chunk point IDs are
user-agnostic (uuid5(tenant_id, doc_id=fileid, chunk_index)), but the
per-user freshness gate filtered Qdrant by user_id, so two readers
ping-ponged: each overwrote the other's points and each kept seeing "not
indexed for me", reprocessing every scan. Production telemetry (note
386945, finding #5) measured identical docs re-processed every few hours
at 7-13s each, with PDF parse ~62% of per-doc cost.
Layer 1 — tenant-wide dedup:
- Thread the scanner's tag-REPORT etag into the file DocumentTask and the
chunk payload; index `etag` as a KEYWORD field.
- vector/sharing_state.find_indexed_content scrolls tenant-wide (no
user_id filter) for a non-placeholder point matching
(doc_id, doc_type, etag), gated on embedding_identity in Python so a
model switch correctly forces a re-embed.
- Scanner skips enqueue and the processor skips fetch/parse/embed when a
match exists (cross-worker race-guard before WebDAV read). Dedup is
fail-safe: a Qdrant error degrades to "process normally".
Layer 2 — observed-access ACL (no admin / GroupFolders API needed):
- Each point carries `acl_principals` = the set of user:<uid> whose
scanner has observed (hence can read) the file. The per-user tag REPORT
is the access oracle; group membership/GroupFolders enumeration is
admin-only and unavailable in multi-user modes.
- build_ownership_filter ORs MatchAny(acl_principals, ["user:<me>"]) so a
deduplicated shared/group-folder point surfaces to every reader;
verify-on-read (_verify_files) remains the precise ACL gate.
- Deletion/eviction become "release one user": drop the principal and
delete the points only when the set empties, so one user untagging a
shared file doesn't evict it for the others. Legacy points without the
field keep the original per-user delete.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Verify-on-read only checked file *accessibility* (file_accessible_by_id),
never tag membership, so a file removed from the `vector-index` tag (but
still readable) kept surfacing in semantic search, and stale points only
got evicted when they happened to rank in a search's top-K.
Rework `_verify_files` to gate on current `vector-index` tag membership via
a single batch `find_files_by_tag(tag, mime_type_filter="application/pdf")`
REPORT per search (plus a one-shot EXCLUDED_TAGS lookup for exclusion-wins
parity) — exactly what the scanner indexes. A file is kept iff it is in that
set, so untagged / deleted / excluded files drop out immediately and the
existing eviction wiring reclaims their Qdrant points. The gate is strict
for all file results, own and shared. Mirrors the batch-fetch-and-intersect
shape of `_verify_news_items` (one semaphore slot, fail-open on fetch error,
malformed-id keep).
- Promote the tag name to a `vector_sync_pdf_tag` Settings field (dynaconf
env mapping VECTOR_SYNC_PDF_TAG) used by both scanner and verifier;
drop the scanner's direct os.getenv.
- Expose `find_files_by_tag` on NextcloudClientProtocol.
- Rewrite the file-verifier unit tests (tagged/untagged/deleted/excluded/
fail-open/non-numeric); update the ACL + verify-on-read integration tests
to seed tagged PDFs.
- Amend ADR-019 and the configuration.md verify-on-read latency budget.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Notes scan in scan_user_documents ran inline without a try/except, while
files/news/deck each had their own guard. On instances without the Notes app
installed, notes.get_all_notes() raises HTTPStatusError 404, which propagated
out of scan_user_documents and aborted the entire per-user vector sync before
files/news/deck were ever reached -- yielding "0 documents indexed" and, after
5 consecutive errors, stopping the scanner.
Extract the Notes scan into scan_notes() (mirroring scan_news_items /
scan_deck_cards) and wrap the call in a per-app try/except. A 404 (app not
installed/disabled) is now logged at info and skipped; other apps still scan.
Deletion-tracking runs only after a successful Notes fetch, so a failed fetch
can never mass-delete a user's indexed notes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SonarCloud:
- Resolve 6 S5332 hotspots (http→https in test fixture URLs).
- S6418: hoist the unauthenticated AsyncOpenAI placeholder to a named constant
+ NOSONAR (genuine non-secret; gateway ignores it when unauthenticated).
- Fix two reliability bugs: None-index guard in the gateway token-cache test
(S2259) and float `> 0.0` instead of `!= 0.0` in the sentinel test (S1244).
- status.py idle path sleeps 0.1s instead of sleep(0) (S7491); NOSONAR on the
protocol-required async no-await aclose() stubs (S7503).
Claude review:
- Remove three leftover debug print() calls in app.py (logger.info already
covers them).
- payload_backfill: drop parsed_at from the backfilled-keys docstring (it is
per-document state, not a deployment scalar); add a clean 404 precondition
for BasicAuth deployments without an OAuth token verifier.
- status.py: task_status typed TaskStatus | None (drop type: ignore).
- nats.py: TODO to thread etags for file/deck/news; note etag default → None.
- factory: warn on unknown INGEST_BUS_URL scheme; raise ValueError instead of
assert for the external-mode preconditions.
- docs/configuration.md: document the decomposition hook-point env vars + that
nats-py ships core (lazy-imported) and external+bus uses two NATS connections.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the seven §10.2 hook-point modules + five env vars so Astrolabe Cloud can
offload document processing to the external document-processor / embedding
gateway. Purely additive: with every setting unset the server behaves exactly
as today, so self-hosters are unaffected (Deck #92).
Hook points (all default to current monolith behavior):
- config: EMBEDDING_PROVIDER, INGEST_MODE, STATUS_BACKEND,
COLLECTION_METADATA_SOURCE, FACT_EVENT_EMITTER (+ supporting settings),
validated in Settings.__post_init__ (fail-fast STATUS_BACKEND=local with
INGEST_MODE=external); shared canonical.py.
- vector/payload_keys.py + acl_hash.py: cross-impl NAMESPACE/point_id (§2.2)
and BLAKE2b-128 ACL hash (§11), pinned by fixtures shared with the
document-processor repo.
- embedding/gateway_client.py: OpenAI-compatible GatewayProvider authenticating
via M2M OIDC client-credentials (separate realm); manual-only registry entry.
- vector/collection_metadata.py: sentinel-point / API metadata source with env
fallback.
- vector/queue/: hexagonal ingest producer ports + memory/NATS adapters
(Postgres seam); INGEST_MODE=external publishes mcp.ingest.requested.{tenant}
instead of the in-memory stream and skips the in-process processor pool. The
lifespan becomes a composition root across both deployment branches.
- vector/queue/status.py: STATUS_BACKEND=bus subscriber feeding a StatusStore
the vector-sync status endpoint reads.
- admin/payload_backfill.py: POST /api/v1/admin/payload-backfill (admin scope);
processor writes the new payload keys; query-side ACL pre-filter gated behind
ACL_PREFILTER_ENABLED (default off).
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>
Detect pre-existing payload indexes with the wrong schema type in
`_ensure_payload_indexes`. The previous "field already in
existing_schema → skip" branch silently survived a collection migrated
from the int-doc_id era where `doc_id` is indexed as INTEGER, letting
`MatchValue(value="123")` searches keep failing with HTTP 400 on Qdrant
Cloud strict mode — exactly the production failure this PR was meant to
fix. New behaviour: compare `existing_schema[field].data_type` against
the declared type; on mismatch log a WARNING and append to
`failed_fields` so the consolidated end-of-function summary picks it up.
No auto-repair (operator intervention only — see docs/configuration.md
recovery procedure). New test exercises the doc_id-INTEGER scenario
end-to-end and asserts both the per-field WARNING and the summary line.
Clarify the `_verify_news_items` malformed-doc_id rationale: the news
API has no per-item endpoint, so a malformed doc_id genuinely cannot be
verified against the source of truth. We err toward false-positive
(keep) over false-negative (drop) — same conservative posture as
`_verify_notes` and `_verify_deck_cards`. The producer-side validation
is the real security boundary; the verifier is defence-in-depth. Both
the inline comment and the WARNING message now spell this out.
Add a TODO in `get_last_indexed_timestamp` flagging the O(N) cost on
every incremental sync tick. The previous single-page `limit=10_000`
silently bounded the scroll; paginating fixed correctness but made the
unbounded cost visible. The follow-up tracker (canonical TODO at
`api/visualization.py`) covers migrating the max-`indexed_at` to a
sentinel point or collection metadata for O(1) lookup.
Consolidate the duplicate non-numeric-doc_type TODOs at
`api/visualization.py:508` and `auth/viz_routes.py:570` into a single
canonical comment in `visualization.py`; `viz_routes.py` is reduced to
a back-reference. Removes the rot risk of "fixed in one place,
forgotten in the other." The canonical comment also references the
O(1) timestamp follow-up in `scanner.py`.
Document the `batch_size = 256` (qdrant_client.py) vs
`_DELETION_TRACKING_PAGE_SIZE = 1024` (scanner.py) split with
cross-referencing comments at each site: the smaller batch is for the
read-write backfill upsert path (Qdrant accepts ~256-point chunks
comfortably); the larger page is for read-only deletion-tracking
scrolls where no per-page write round-trip applies.
Replace `assert qdrant_client is not None` in `scan_user_documents`
with `cast(AsyncQdrantClient, qdrant_client)` plus an explanatory
comment. `assert` is silently elided under `-O`; `cast` is the
conventional zero-cost narrower for branches the type checker can't
infer from the surrounding `if not initial_sync` ternary.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Defer publication of `_qdrant_client` until after the in-lock backfill +
payload-index migration awaits complete. The fast-path check at the top of
`get_qdrant_client` reads the singleton without holding the init lock, so
publishing the constructed-but-unmigrated client let concurrent fast-path
callers fire filtered searches before `_ensure_payload_indexes` ran —
producing HTTP 400 ("Index required but not found") on Qdrant Cloud strict
mode. Local `provisional` is now used for every await inside the lock; the
global is assigned exactly once, last.
Replace the five hand-rolled `scroll(..., limit=10000)` calls in
`vector/scanner.py` (notes / files / news / deck-cards deletion tracking,
plus the timestamp scroll) with a single paginated `_scroll_all_points`
helper. The previous single-page cap silently dropped deletion-tracking
points beyond the first 10 k for any user past that threshold. Pagination
follows Qdrant's documented contract (loop until `next_page_offset is
None`) with a fixed per-page `_DELETION_TRACKING_PAGE_SIZE = 1024`.
Extract `_create_one_payload_index` from `_ensure_payload_indexes` to drop
its cognitive complexity below the SonarQube limit (17 → ≤ 15) without
losing the per-field error-containment rationale; every comment is
preserved verbatim on the helper.
Drop the stale `SearchResult.id` `int | str` comment and the redundant
`str(d)` coercion in `_verify_news_items` — the contract has been
str-only since the producer-side stringification landed earlier in this
PR.
Fix eight `doc_id=<int>` test calls in `test_chunk_context_offset_gate.py`
that violated the `doc_id: str` signature of `get_chunk_with_context`,
plus align `_make_result` in `test_verification.py` to coerce `id=str(...)`
matching the production contract — and update 30+ assertions from int
sets (`{1, 2, 3}`) to str sets (`{"1", "2", "3"}`) so the tests now model
the post-PR `SearchResult.id: str` reality end-to-end. Previously these
were masked by the `str(d)` coercion now removed from production.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- search/context.py: drop the doc_type=='file' guard on skip_offset_lookup
so notes / deck cards / news items also bypass the unindexed offset
fallback when chunk_index is available. Legacy chunk_index=None data
still uses the offset path.
- vector/qdrant_client.py: clarify the backfill/_ensure_payload_indexes
ordering invariant (backfill rewrites payload values only, never schema
or indexes). Acknowledge OSS-vs-Cloud uncertainty in the 400-branch
comment and the new-collection call-site comment.
- vector/scanner.py: hoist qdrant_client to function scope so the
file-scroll block doesn't depend on a name bound inside the
notes-scroll block.
- tests/unit/test_chunk_context_offset_gate.py: flip the note-with-
chunk_index test to assert the offset fallback is skipped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reviewer feedback (3 important + 3 nits):
- Wrap _ensure_keyword_payload_indexes' get_collection() call in
try/except. The qdrant_client singleton is already assigned by the
time this function runs, so a transient timeout/DNS failure
propagating out left the process holding a usable client with the
migration silently skipped on every subsequent call. Now logs ERROR
with exc_info and returns; next process restart retries.
- Add `and "doc_id" in point.payload` guard to the four set
comprehensions in scanner.py (indexed_doc_ids, indexed_file_ids,
indexed_item_ids, indexed_card_ids). Previously a payload missing
the doc_id key would raise KeyError and crash the entire scan.
- Tighten test_ensure_keyword_payload_indexes_logs_400_as_warning to
match the per-field warning prefix exactly (`startswith("Schema
conflict on payload index")`), so a future change adding 400s to
the partial-failure summary surfaces here as a count mismatch.
- Add new-collection vs existing-collection context to the
_backfill_doc_id_to_string docstring's `dimension` parameter.
- Replace the misleading "rewrote 0/N from int to str" wording when
no rewriting was needed with "N points scanned, none required
rewriting (collection already in str form)".
- Add test_ensure_keyword_payload_indexes_logs_and_returns_when_
get_collection_raises mirroring the scroll-failure test.
SonarCloud (1 CRITICAL + 1 MINOR):
- Refactor _backfill_doc_id_to_string to bring cognitive complexity
under 15 (was 19). Extracted two pure helpers: _group_int_doc_ids
(group point IDs by stringified doc_id) and _apply_backfill_writes
(apply set_payload calls and return rewritten count). The main
function's scroll/loop/sentinel structure is unchanged.
- Add `await asyncio.sleep(0)` to the three async test side_effect
helpers (_scroll_raises, _upsert_raises, _create_index) so they use
an actual async feature (S7503). The async-callable shape is still
required to avoid the AsyncMock unawaited-coroutine warning when
side_effect raises.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Remove three stale `# Use numeric file ID` / `# Pass file path` comments
in scanner.py. file_id is already normalized to str() above each call
site, so the inline comments mislead readers.
- Wrap `_backfill_doc_id_to_string` scroll loop + sentinel upsert in
try/except Exception. The qdrant_client singleton is assigned before
this migration runs, so a transient scroll failure was leaving the
process holding a usable client with int payloads permanently
unbackfilled until the next restart. Catch broadly, log ERROR with
exc_info, and return without writing the sentinel — next process
restart retries from scratch.
- Note `:memory:` mode behavior near the sentinel constants so future
readers don't read the every-start scroll as a bug.
- Document the two degraded-migration ERROR log signals in
docs/configuration.md so operators know when a clean restart is
required to recover indexing.
- Add unit test asserting scroll-time exceptions are logged and swallowed
without writing the sentinel.
Closes round-4 review feedback on PR #773.
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>
NextcloudClient.find_files_by_tag now mirrors the directory semantics
already used by the exclusion path (issue #710): when a tagged item is
a folder, walk its descendants via WebDAV SEARCH (Depth: infinity) and
include any files matching the MIME filter. Without this, tagging the
root of a corpus with `vector-index` indexed nothing because the tag
applies to the directory only, not to its children.
The vector scanner additionally consults EXCLUDED_TAGS now, so a folder
marked off-limits is skipped even if it (or an ancestor) carries the
include tag — defense-in-depth, matching the "exclusion wins" contract
already enforced by the MCP file tools.
Also addressed a recurring memory-style nit: pre-existing f-string log
lines in find_files_by_tag were converted to lazy %-style.
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>
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>
Addresses reviewer feedback on PR #395 about O(n²) performance issue.
Changes:
- scanner.py: Add metadata field to DocumentTask with board_id/stack_id
- scanner.py: Populate metadata during deck card scanning (both initial and incremental sync)
- processor.py: Use metadata for O(1) card lookup via get_card() API when available
- processor.py: Fallback to iteration for legacy data without metadata
- context.py: Add _get_deck_metadata_from_qdrant() helper to retrieve metadata from Qdrant
- context.py: Use metadata for fast path lookup in chunk context expansion
- context.py: Add user_id parameter to _fetch_document_text() for metadata retrieval
Performance Impact:
- Before: O(boards × stacks × cards) iteration for each card lookup
- After: O(1) direct API call using stored board_id/stack_id
- Graceful degradation: Falls back to iteration for legacy data
Testing:
- All existing integration tests pass (test_deck_vector_search.py)
- Type checking passes with no new errors
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 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>
Fixed 8 type checker errors across the codebase:
- vector/scanner.py: Handle None scroll results with null-safe iteration
- search/{bm25_hybrid,semantic}.py: Add None checks for result.payload
- auth/{unified_verifier,webhook_routes}.py: Assert non-None auth credentials
- client/webdav.py: Add None checks before int() conversions
- providers/openai.py: Assert embedding_model is not None
- search/algorithms.py: Explicitly type doc_types set and cast values
- observability/logging_config.py: Match parent class signature (log_data)
Also fixed test_create_tag_creates_system_tag to match WebDAV implementation
(was testing OCS API endpoint, now tests correct WebDAV endpoint with
Content-Location header).
Type checker: 0 errors (down from 8), 20 warnings (ignored)
Tests: All 192 unit tests passing
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Remove the complex starred+unread filtering logic in scan_news_items().
The News app's auto-purge feature (default: 200 items per feed) already
limits the total number of items, making explicit filtering unnecessary.
Changes:
- Replace two API calls (starred + unread) with single all-items call
- Remove deduplication logic that merged both lists
- Update docstring to explain the simpler approach
This reduces code complexity while maintaining the same effective coverage.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Add full integration for the Nextcloud News (RSS/Atom reader) app:
- Add NewsClient with complete CRUD operations for folders, feeds, and items
- Add 8 read-only MCP tools for listing/getting folders, feeds, items
- Add Pydantic models for News entities with camelCase alias support
- Add vector sync support for starred + unread items
- Add HTML to Markdown converter using markdownify for better embeddings
- Add Docker post-install hook to enable News app
- Add 25 unit tests for NewsClient API methods
Vector sync indexes starred and unread items, providing a balanced approach
that captures important (starred) and current (unread) content without
indexing the entire article history.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
- Changed from 2x (120s) to 5x (300s) scan interval
- Large PDFs take 3-4 minutes to process, need longer threshold
- Prevents premature requeuing of in-flight documents
- Only requeue documents if placeholder is older than 2x scan interval (120s default)
- Prevents scanner from immediately requeuing in-flight documents
- Fixes issue where PDFs were being reprocessed every 60 seconds
- Staleness check applied to both notes and files scanning logic
Introduces a placeholder-based state tracking system to prevent duplicate
document processing during the gap between scanner queuing and processor
completion.
**Key Changes:**
1. **Placeholder Helper Functions** (`vector/placeholder.py`):
- `write_placeholder_point()` - Creates zero-vector placeholder when queuing
- `query_document_metadata()` - Queries for existing entry (placeholder or real)
- `delete_placeholder_point()` - Removes placeholder before writing real vectors
- `get_placeholder_filter()` - Filters placeholders from user-facing queries
2. **Scanner Updates** (`vector/scanner.py`):
- Replace `indexed_at` comparison with `modified_at` comparison
- Write placeholder before queuing each document
- Query per-document metadata instead of bulk-querying indexed_at
- Fixes bug where files were resubmitted every scan cycle
3. **Processor Updates** (`vector/processor.py`):
- Delete placeholder before upserting real vectors
- Ensures no duplicate points in Qdrant
4. **Query Filters** (all search files):
- Add `get_placeholder_filter()` to all user-facing queries
- Ensures placeholders never appear in search results or visualizations
- Applied to: bm25_hybrid.py, semantic.py, viz_routes.py, algorithms.py
**Architecture:**
- Placeholders use zero vectors with dimension from embedding service
- Payload includes `is_placeholder: True` flag for filtering
- Status field tracks: "pending", "processing", "completed", "failed"
- Deterministic UUIDs using uuid5 for consistent point IDs
**Impact:**
- Eliminates duplicate processing of same documents
- Fixes race condition where long-running documents get queued multiple times
- Prevents scanner from resubmitting files every scan cycle
- Maintains clean separation between in-flight and indexed documents
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
