No blockers raised; residual cleanup:
- processor.py: guard int(doc_task.doc_id) with is_valid_nextcloud_doc_id in
the mail_message branch (consistent with search/context.py + the verifier).
- mail metadata symmetry: store `bcc` in file_metadata and the Qdrant payload
alongside cc (build_mail_content already emits a Bcc: line).
- server/mail.py: extract _cap_attachment_content helper (byte-accurate cap)
and unit-test it (small/None/oversized/multibyte).
- client/mail.py: give the synthetic OCS-error Response an explicit empty body;
add a test that a traversal-style attachment_id is percent-encoded.
- models/mail.py: clarify ListMessagesResponse.total_count is the page count,
not the mailbox total.
Deferred (Deck #376): _potentially_deleted doc_type-in-key. It's pre-existing
and spans ~30 sites across the notes/news/deck/file/mail scanners (whose
deletion paths have no unit coverage), so it belongs in its own focused PR
rather than expanding this mail PR's blast radius into other doc types.
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>
- models/mail.py: lowercase `list` generics per CLAUDE.md convention.
- client/mail.py: _ocs_get now inspects ocs.meta.statuscode (re-raises >=400
as HTTPStatusError carrying the OCS code so callers' 404/403 handling
applies) and guards response.json() against non-JSON bodies (RequestError).
- Extract the duplicated _format_addresses + content reconstruction into
vector/mail_content.py, used by both processor.py and context.py (fixes the
SonarCloud new_duplicated_lines_density gate).
- processor.py: add the missing mail_message Qdrant payload block so the
computed mail metadata (subject/from/to/cc/date_int/has_attachments/
account_id/mailbox_id) is actually stored, not dropped.
- Rename the list_messages `filter` param to `search_filter` (avoid shadowing
builtins.filter); still maps to the OCS `filter` query param.
- Docstring notes: has_more heuristic, attachment content size.
- Tests: OCS meta-failure + non-JSON client paths; initial-sync scanner tests
(tests/unit/vector/test_scanner_mail.py).
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>
The prior comment claimed anyio.lowlevel.checkpoint() yields None and is
re-driven, but checkpoint() yields a non-None backend object (asyncio
Future / trio checkpoint) and so trips the RuntimeError guard. Clarify
that only a literal bare yield/yield None is re-driven; any real awaitable
is caught by the non-None guard. Comment-only; no behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Clear SonarCloud hotspot python:S5332 (insecure http:// URL) by using
https:// for the inert placeholder Qdrant URL in the network-mode test.
- Make the test faithful to production: stub the network existence-check
with UnexpectedResponse(404) (what the real HTTP client raises) instead
of the local-mode ValueError("not found"), per round-2 review.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Use the reset_qdrant_singleton fixture in the network-mode test instead
of manual save/restore boilerplate.
- Add a test locking down the sync-callable forwarding branch (callable
returning a non-coroutine is passed through without thread offload).
- Document that _drive_local_coroutine treats a bare `yield None` as a
non-suspension, and that the proxy is wrapped before the startup
migrations so the O(N) backfill scroll also runs off the event loop.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
qdrant-client's embedded backends (:memory: and path= local mode) run
every operation synchronously on the calling thread despite the
AsyncQdrantClient surface — AsyncQdrantLocal contains no thread offload,
and this is unchanged through the latest v1.18.x (the async surface is
autogenerated from the synchronous QdrantLocal).
On a CPU-constrained host a background scan of thousands of tagged files
issues ~3 Qdrant queries per file, all on the event loop thread, pinning
one core at 100% and stalling /health/live, /health/ready, and the
outbound Nextcloud-reachability probe for minutes — the failure mode in
issue #926 (health-check timeouts, "nextcloud_reachable: error").
Wrap the embedded client in a transparent proxy that offloads every
coroutine-returning call to a worker thread via anyio.to_thread.run_sync,
keeping the event loop responsive. A dedicated CapacityLimiter(1)
serialises those offloads to preserve QdrantLocal's single-access
invariant (today guaranteed implicitly by the single-threaded loop).
Network mode (QDRANT_URL) is left untouched — it already does
non-blocking I/O. Centralised at get_qdrant_client() so all call sites
benefit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In-process native code -- pymupdf's classify/metadata `pymupdf.open` and
embedded Qdrant -- can segfault the interpreter during indexing. With only
PYTHONUNBUFFERED set, such a fault makes the container exit 139 (SIGSEGV) /
133 (SIGTRAP) with no logs at all, which is exactly what #926 reports on a
CPU-constrained self-hosted VPS.
Enabling PYTHONFAULTHANDLER makes the interpreter dump a Python + C-level
traceback to stderr on SIGSEGV/SIGABRT/SIGFPE/SIGBUS, so the faulting library
is identifiable from container logs. The handler is dormant during normal
operation (no output, negligible overhead).
Refs #926
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The single-user `mcp` container re-scanned the whole corpus every 5s. The
scanner already runs an initial scan on startup, so the corpus is indexed
without a frequent re-scan — the short interval only re-queues everything faster
than the 2 workers can drain. On a loaded CI runner pending_count snowballs
(observed 1465 pending, status stuck "syncing"), so freshly-created notes aren't
indexed within the tests' 90s sync-wait and test_rag / test_sampling cascade
into 300s pytest timeouts (intermittent single-user failures; passed on nc33 /
faster runs).
Raise it to 30s, matching the already-tuned multi-user-basic cadence and staying
well within the 90s wait budget. multi-user-basic (30s) and login-flow (60s) are
already relaxed; this brings single-user in line.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Enabling NC33 surfaced that every login-flow test failed with "Login Flow v2
did not complete after 15 attempts". Root cause: NC33's "Connect to your
account" page renders BOTH a "Log in" button and an "Alternative log in using
app password" button. The Step-1 locator `get_by_role("button", name="Log in")`
is a non-exact (substring) match, so it matched both -> Playwright strict-mode
error, which the surrounding try/except silently swallowed. The flow stayed on
the connect page, never reached "Grant access", and the poll timed out.
Fix: add exact=True to the Step-1 "Log in" locator in both login-flow helpers.
NC32's connect page has a single match, so exact=True is safe there. Verified
on a live NC33 stack: the exact click reaches the grant page cleanly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Nextcloud 31 reached deprecation (02/2026), so remove it from the integration
matrix. Enable NC33 (previously disabled pending upstream app support) and add
NC34 as a commented, ready-to-enable entry.
- test.yml: nextcloud_version is now [32, 33]; 34 commented. Image pins updated
to match (32.0.11, 33.0.5 active; 34.0.0 commented). The Renovate customManager
regex already tracks commented entries, so 34 is digest-managed once present.
- renovate.json: drop the nextcloud-31 pin rule, add nextcloud-34 (/^34\./).
docker-compose.yml already defaults to 32.0.11 (Renovate-pinned to 32.x), so no
change there — the NC31 seen in local runs comes from a shell-exported
NEXTCLOUD_IMAGE override, not the compose default.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolves the SonarCloud quality gate (new_security_hotspots_reviewed) — the 7
TO_REVIEW hotspots were all `http://gw` fake gateway URLs in test_ocr_processor.py
flagged "use https". Switched the test fixtures to `https://gw` (identical for a
fake URL) so no new hotspots remain to review; all other gate conditions already
passed.
Also address claude-review round 6:
- Stale test docstring: "suppresses to ocr" -> "suppresses to the cheapest
registered OCR rung (here ocr-upstream)".
- OcrProcessor.__init__: comment that the upstream-shaped defaults are
test/bare-construction convenience only; app wiring always passes args
explicitly.
Left as-is (reviewer agreed): the `ocric=` signature abbreviation (opaque but
stable; renaming would invalidate dead-letter retries).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address claude-review round 5 on #922 (verdict: good to merge after the docstring):
- BatchPending docstring: the deferred job stays on its own `(ocr-upstream)` tier
queue, not the retired `(ocr)` — batch mode is the upstream Mistral path only.
- classifier.py: clarify that `recommended_tier == "ocr"` is the classifier's
COARSE vocabulary ("needs OCR"), resolved to a concrete rung
(ocr-incluster -> ocr-upstream) by the registry — NOT a TIER_LADDER tier name.
- Added test_evaluate_escalation_suppressed_targets_incluster_four_rung: with both
OCR rungs registered but both flags off, the suppressed what-if-OCR signal names
the cheapest ideal rung (ocr-incluster), not ocr-upstream.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address claude-review round 4 on #922:
- Important 1: build_ocr_backend now warns when gateway_only + provider=none +
DOCUMENT_OCR_INCLUSTER_ENABLED=true — provider=none suppresses the gateway-only
in-cluster tier too (it never uses the mistral provider), which surprises an
operator who set none just to disable Mistral. Restructured so the gateway_only
branch is evaluated before the generic provider=none return. Two tests cover
the warn-when-enabled / silent-when-disabled cases.
- Nit 3: model fallback uses `model if model is not None else ...` (not `or`), so
an empty model string no longer silently falls back to the upstream default and
misroutes a per-tier rung. Test added.
- Nit 4: the no-surya-literal guard now uses rglob so future
document_processors/ subdirs are covered.
Deferred (pre-existing / out of scope for this PR):
- Important 2 (_GatewayOcrBackend opens a new httpx.AsyncClient per ocr() call):
a pre-existing pattern affecting both OCR rungs; a shared pooled client needs
careful per-pod lifecycle handling (event-loop binding, aclose) and is better
as its own change. Follow-up.
- Nit 5 (_MANAGED_QUEUES vs the CLI all-queues list): the two sets differ
deliberately (the CLI list includes ingest-maintenance, _MANAGED_QUEUES does
not), so a shared constant wouldn't cleanly dedupe them.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address claude-review round 3 on #922:
- Important: _process_batch emitted "no gateway backend (provider=mistral or
EMBEDDING_GATEWAY_URL unset)" for the gateway_only in-cluster rung, where the
gateway IS configured — sending operators chasing a non-existent config
problem. The real reason is "in-cluster GPU is synchronous-only; batch is the
upstream path". Guard the warning with `if not self._gateway_only`. Extended
test_gateway_only_processor_never_uses_batch_mode to drive _process_batch and
assert _batch_fallback_warned stays False.
- Nits: refresh stale ladder in escalation.py module docstring
(fast->structured->ocr-incluster->ocr-upstream); fix "minimum='ocr'" ->
"ocr-incluster" in a test docstring; rename stale tier="ocr" ->
"ocr-upstream" in test_process_tier_oversize_fails_fast.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Round-10 review nit: match the defensive `str(id) == str(note_id)` pattern used
by _poll_astrolabe_search_for_note. nc_semantic_search returns int ids today
(behaviour-neutral now), but the coercion guards against a future schema change
serialising ids as strings, which would otherwise silently break the match and
time out with a generic message.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Root cause of the multi-user-basic/nc32 failure: the appstore installs
DIFFERENT astrolabe versions per NC major (min-version jumped 31->32 at
astrolabe 0.25.0). NC31 pulls astrolabe 0.24.0 (search box = NcTextField ->
<input>, submits on Enter); NC32 pulls 0.29.0 (search box = NcTextArea ->
<textarea>, submits on Ctrl/Cmd+Enter). The test's `.mcp-search-input input`
selector + Enter never matched the textarea on nc32, so it timed out after the
SPA mounted fine. This was latent all along but masked on nc32 by the
vector-sync gauge flake, which failed the test earlier; fixing that flake
unmasked it.
Fix: match either `.mcp-search-input textarea, .mcp-search-input input` and
submit based on the element tag (Ctrl+Enter for textarea, Enter for input).
Verified against a live NC32 + astrolabe 0.29.0 stack: the textarea is found
and Ctrl+Enter fires GET /apps/astrolabe/api/search. All other selectors the
test uses (.mcp-loading/.mcp-error/.mcp-results/scatter3d) still exist in 0.29.0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address claude-review round 2 on #922:
- Legacy `ingest-ocr` tier resolution (important 1): document why it deliberately
resolves to `fast` rather than mapping to `ocr-upstream` — stranded pre-split
jobs re-extract empty and re-escalate via the ladder to the cheap
`ocr-incluster` rung, keeping them OFF the paid upstream rung. Added a
tier_for_queue(LEGACY_INGEST_QUEUE_OCR) == "fast" assertion.
- Double get_settings() in `_get_batch_client` (important 2): bind once to a local.
- Stale docstrings (important 3): OcrProcessor (serves both rungs now),
_tier_available (both OCR rungs gated), evaluate_escalation (targets
ocr-incluster, falls through to ocr-upstream).
- Misconfigured model_setting (nit 5): OcrProcessor.__init__ raises ValueError on
an unknown settings attr (fail-fast at startup vs AttributeError mid-OCR); also
removes the dynamic-getattr static-analysis smell SonarCloud flagged.
- Redundant guard (nit 4): kept `and ocr_tier is not None` — it's required for ty
to narrow ocr_tier to str for record_document_escalation; added a comment.
- Test gap (nit 6): added a test pinning the CURRENT incluster-failure ->
tier-1 fallback (does NOT cascade to upstream) so the future 503-escalation
change is an explicit diff.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address claude-review on #922:
Blocking — test coverage for the new tier2 rung:
- test_registry_tiering.py: inline empty-text routes to ocr-incluster before
ocr-upstream; only-incluster-enabled routes to incluster; disabled-incluster
skips to upstream; evaluate_escalation empty_text hops to ocr-incluster (and
falls through to upstream when incluster off); next_available_tier walks the
full fast→structured→ocr-incluster→ocr-upstream ladder + ignore_ocr_enabled
ideal-target.
- test_escalation_signature.py: enabling document_ocr_incluster_enabled changes
the dead-letter signature (independent of the upstream rung).
- test_tiered_escalation_strategy.py: structured→ocr-incluster hops to
INGEST_QUEUE_OCR_INCLUSTER; tier_for_queue covers the in-cluster queue.
Important — real fixes:
- registry.py: run scan detection (image_coverage_per_page) when EITHER OCR rung
is enabled, not just the upstream one — a tenant with only in-cluster OCR on
was missing image-coverage scan signals.
- ocr.py: the gateway_only (in-cluster) processor never enters batch mode — the
GPU is synchronous/low-latency; batch OCR is the upstream Mistral async path.
_get_batch_client short-circuits to None. Covered by a new test.
Nit:
- cli.py: worker --tier help lists ocr-incluster/ocr-upstream as separate fleets.
Left as-is: the lazy anyio.Lock init in OcrProcessor — instances ARE created at
module import (document_processors/__init__.py), so deferring lock creation off
import time is still required; moving it into __init__ would reintroduce the
import-time-primitive issue the comment guards against.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Insert a configurable in-cluster OCR rung into the escalation ladder (Deck #353):
a tier2-eligible doc is OCR'd on the on-demand burst GPU before falling through to
paid upstream OCR. The in-cluster backend is reached ONLY via the embedding gateway
(model prefix routes to the GPU over the tailnet) and is a config value (default
surya/surya-ocr-2, swappable to e.g. lightonocr) — never hard-coded.
Ladder: fast -> structured -> ocr-incluster -> ocr-upstream
(queues ingest-ocr-incluster / ingest-ocr-upstream).
- escalation.py: 4-tier ladder; in-cluster flag folded into the dead-letter signature.
- ocr.py: OcrProcessor(name, tier, model_setting, gateway_only); build_ocr_backend(
..., model=, gateway_only=) — gateway_only forces the gateway backend (never the
direct Mistral fallback), disabling the tier with a warning if no gateway URL.
- registry.py: per-rung enable map; scanned docs target minimum="ocr-incluster";
inline path runs the cheapest available OCR rung.
- procrastinate.py: two OCR queues; legacy ingest-ocr kept as a drain target.
- config.py: DOCUMENT_OCR_INCLUSTER_ENABLED (off) + DOCUMENT_OCR_INCLUSTER_MODEL.
- __init__.py: register the two OCR instances; vector/processor.py: pages_ocr
metered for the upstream (paid) rung only; cli.py: new --tier choices + legacy drain.
- metrics.py: zero the legacy ingest-ocr queue gauge during rollout.
- tests: migrated to the split ladder + new tests (gateway-only forcing, per-tier
model incl. lightonocr override, no-hard-coded-surya guard). 1792 pass; ruff + ty green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
