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>
- processor.py: raise on an empty mail_message payload (OCS data=null with a
<400 meta) so the task dead-letters instead of indexing a near-empty
placeholder — mirrors the nc_mail_get_message tool guard. (the round-6
approve-gating item)
- server/mail.py: clamp limit once in nc_mail_list_messages and base has_more on
the effective (post-clamp) limit, so a caller passing limit<=0 doesn't get a
misleading count.
- server/mail.py: note in nc_mail_get_message that attachments with id=null are
inline body parts and can't be fetched via nc_mail_get_attachment.
- tests: add the first-time-missing incremental scanner case (enters the grace
period, nothing queued/deleted).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
- 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>
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>
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>
Round-4 review nits on PR #920 (none blocking):
- record_document_dead_lettered: enumerate the oversize reason (added this PR)
alongside timeout/oom/error in the docstring + counter comment.
- Note the clear-dead-letter-before-upsert ordering implication (a transient
upsert failure re-parses once, never a silent drop).
- Clarify the orphan sweep's kept counter for tenant-wide dead-letter markers.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Round-3 review nits on PR #920 (none blocking):
- escalation_tiers_signature: TODO noting future settings that can rescue a
previously-terminal document (a toggleable llm tier, a raised oversize cap)
should be folded into the signature so raising them auto-retries dead-letters.
- Terminal-path placeholder cleanup: a delete failure here is real Qdrant I/O,
not control-flow -- log at warning (was debug) for observability. Non-fatal
(the durable marker is already written).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses round-2 review on PR #920:
- Only dead-letter a terminal failure when the file has an etag to
content-address the marker; without one, fall back to the legacy per-user
placeholder mark (an etagless marker is unmatchable). + test.
- _dead_letter_filter now also matches is_placeholder=True (redundant with
dead_letter=True but lets Qdrant use the is_placeholder payload index).
- TODO(deck-349) documenting the dead-lettered-then-deleted orphan-marker leak
(out of scope; needs a marker sweep or TTL field) per reviewer.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses round-1 review on PR #920:
- Delete path now clears the file's dead-letter marker after
release_document_for_user (whose principal-based filter misses the
user-agnostic, principal-less marker), preventing orphan-marker accumulation
for dead-lettered-then-deleted files.
- Oversize PDFs (rejected by the pre-parse size guard, no pipeline_tier stamped)
are now treated as terminal regardless of failing_tier -- no tier can parse an
oversize file -- so they dead-letter instead of falling to the legacy per-user
mark on the inline path.
- Gate the success-path clear on a non-empty etag (an etag-less file can never
have a marker, mirroring is_dead_lettered's early return).
- dead_letter.py: payload typed dict[str, Any] (CLAUDE.md).
Tests: oversize-terminal dead-letter and delete-path marker clear.
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>
Address round-1 review on #913 and the SonarCloud new_reliability_rating gate:
- credential_cleanup_task no longer catches the cancellation exception
(Sonar python:S7497). A task-group cancel must propagate for structured-
concurrency teardown; graceful shutdown still flows through shutdown_event,
so the sleep no longer needs a cancel/break.
- Parametrize the scanner self-heal tests over 401 AND 403 (handled
identically at both call sites) and add a test that a failing periodic
sweep is logged non-fatally and does not crash the task.
- Log the stored-user count before the startup sweep (operability signal),
add a debug line when the credential row was already gone, and document
the at-most-one extra-401 convergence in _remove_stale_credential.
Refs Deck #198.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deleted/disabled Nextcloud users left their app_passwords row in storage,
so user_manager_task re-spawned their scanner every poll interval only to
401 again — an endless re-spawn/auth-failure loop (observed on
tenant-blackbox-demo: ~534 respawns/3h, matching the 60s poll interval).
- Delete the stored app password on a hard 401/403 in user_scanner_task
(both the pre-validation and in-scan-loop paths), breaking the re-spawn
loop at the source so the user-manager stops recreating the scanner.
- Add a periodic credential_cleanup_task backstop (hourly) that sweeps
cleanup_invalid_app_passwords for anything the per-scanner path misses.
- Run the startup cleanup for all deployment modes: drop the stale
`not oauth_enabled` guard so login_flow tenants (the cloud default) are
covered. NOTE: login_flow startup now makes one concurrent OCS
validation call per stored user before readiness.
Refs Deck #198.
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>
Round 4 review (PR #910), no blockers:
- poll(): a 2xx body with no `status` now fails fast (logged) instead of being
treated as perpetually pending until the deadline; defensive page index
(`p.get("index", i)`) so a malformed page degrades rather than KeyError-ing.
- Document on poll() that job_id is namespaced (embeds "/") so the gateway route
must be a path-capture param (GET /v1/ocr/batch/{job_id:path}).
- Drop the vestigial `status` + `updated_at` columns from batch_ocr_jobs: a row
only ever exists while pending (terminal jobs are deleted) and the live status
comes from a fresh poll, so a stored mirror was permanently "pending" /
redundant with submitted_at. Simplifies the migration, store, and dataclass.
- Tests: submit() ValueError on missing job_id; poll() missing-status → failed.
1653 unit tests pass; ruff + ty green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Round 2 review (PR #910):
- BLOCKING: BatchOcrJobStore._shared_lock is now lazy-init (anyio.Lock | None,
created on first shared() call) instead of at class-definition time — matches
the CLAUDE.md "no anyio primitives at import time" rule and OcrProcessor's
pattern. The None-check->assign has no await between, so it's race-free.
- document_ocr_mode now normalizes via _enum_fields (case-insensitive, like
document_ocr_provider) instead of a strict dynaconf is_in Validator, so
DOCUMENT_OCR_MODE=Batch normalizes to "batch" rather than erroring. Tests for
case-normalization + invalid-value rejection.
- TYPE_CHECKING-gated GatewayBatchOcrClient import so build_gateway_batch_client
/ _get_batch_client are typed `GatewayBatchOcrClient | None` instead of Any
(runtime import stays lazy to avoid the import cycle).
- Rename ocr_options -> doc_identity_options (it's threaded to all tiers; only
OCR reads it) + clarify the comment.
- Drop the redundant forward-ref quotes on _shared_instance.
- Add direct _batch_identity unit tests (partial/empty options branches).
Left as follow-up: reusing one httpx.AsyncClient across submit/poll (same
per-call pattern as the existing sync _GatewayOcrBackend; no clean aclose hook
on the cached client today).
1653 unit tests pass; ruff + ty green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add DOCUMENT_OCR_MODE=sync|batch (default sync). In batch mode the tier-3 OCR
processor submits documents to the embedding gateway's async Batch OCR routes
(POST /v1/ocr/batch + GET /v1/ocr/batch/{job_id}, astrolabe-cloud-website#372)
for ~50% cheaper large-corpus backfill. The direct Mistral OCR path is left
untouched. Tracked on Deck #332.
Batch jobs run minutes-hours, so the OCR tier cannot block (the procrastinate
worker reclaims jobs in `doing` after INGEST_STALLED_JOB_SECONDS). Instead it
submits, records the gateway job id in a new per-tenant `batch_ocr_jobs` table
(procrastinate args are immutable across retries), and raises a BatchPending
signal that TieredEscalationStrategy turns into a same-queue deferred re-poll —
releasing the worker slot between polls. On completion the per-page markdown is
indexed like the sync path; a failure or a job past
DOCUMENT_OCR_BATCH_MAX_WAIT_SECONDS marks the document parse-failed.
Batch is opt-in and gateway-only: with the direct mistral backend, no gateway
URL, or the inline/memory pipeline (which can't defer), it falls back to sync.
One batch job per document (coalescing N docs/job is a follow-up).
- embedding/gateway_batch_client.py: submit/poll client (reuses GatewayTokenProvider).
- vector/batch_ocr_store.py + migration 008: job tracking (portable SQLite+PG).
- document_processors/escalation.py: BatchPending control-flow signal.
- document_processors/ocr.py: batch state machine + sync fallback.
- vector/processor.py: thread doc identity to the OCR tier; raise BatchPending
from the pending sentinel; propagate it as control flow (not a failure).
- vector/queue/procrastinate.py: BatchPending -> same-queue retry_in, exempt
from the transient cap (bounded by the processor's deadline).
- config + docs; tests across client/store/processor/strategy/parse-tier.
1653 unit tests pass; ruff + ty green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GHSA-8vh3-g2qg-2h2c (CVSS 9.1, CWE-306): POST /webhooks/nextcloud had no
authentication when WEBHOOK_SECRET was unset (the default). The receiver
trusted the attacker-supplied user.uid and fed it to Qdrant, letting an
unauthenticated network caller delete or re-index any user's vector
embeddings.
Webhooks now require WEBHOOK_SECRET end-to-end:
- app.py: the /webhooks/nextcloud route is only mounted when WEBHOOK_SECRET
is set; otherwise it 404s and a startup warning notes vector sync falls
back to the polling scanner.
- webhook_receiver.py: removed the warn-and-accept fallback. No secret -> 503,
missing/invalid bearer -> 401; the payload is never processed unauthenticated.
- webhook_routes.py / api/webhooks.py: webhook_auth_pair() raises
WebhookSecretNotConfigured instead of returning authMethod="none"; both
registration entry points return a clear 503 so no dead unauthenticated
webhooks are created.
Also expose webhooks availability to the Astrolabe UI via GET /api/v1/status
("webhooks_enabled": bool), set WEBHOOK_SECRET on the docker-compose
semantic-search dev services, and update env.sample + ADR-010 / ADR-018 /
webhook-management-guide docs.
Vector sync still works without a secret via the polling scanner.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- escalation: EscalationDecision.reason is now Literal["empty_text",
"low_confidence"] (parity with kind; ty catches a bad label at call sites).
- processor: nest the decision handling so the hop branch is reached via an
explicit else under `if decision is not None` — exhaustive over the Literal
kind, no None-attribute risk.
- tests: add the "OCR processor unregistered (not just disabled) → None"
quadrant, locking in absent != suppressed.
Deck #324.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- escalation: EscalationDecision.kind is now Literal["hop","suppressed"] so ty
catches a bad kind statically (and the processor branch is exhaustive).
- processor: simplify the suppressed-escalation log line (no longer repeats
to_tier / tier).
- registry: clarify _tier_available's ignore_enabled drops the OCR-enabled gate
specifically (a future per-tier gate would extend the condition).
Deck #324.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
OCR is the paid, opt-in tier (DOCUMENT_OCR_ENABLED, default off). The per-tier
escalation gate already declines to hop to OCR when it's disabled (the pre-OCR
tier is terminal — no surprise cost), but that left operators blind to how much
OCR demand exists.
evaluate_escalation now returns a structured EscalationDecision:
- "hop" — a higher tier can run; the caller raises EscalateError (queue-hop).
- "suppressed" — the ideal next tier (e.g. ocr) exists but is DISABLED; the caller
indexes the current tier's output as terminal and records the
would-be hop on the new astrolabe_document_escalation_suppressed_total
{from_tier,to_tier,reason} counter instead of hopping.
- None — index as-is (good text, or no such tier at all).
So with OCR off, escalation_suppressed_total{to_tier="ocr"} is the latent OCR
demand an operator weighs before enabling OCR; enabling it converts these into
real document_escalation_total{to_tier="ocr"} hops. next_available_tier gains an
ignore_enabled flag to compute the *ideal* (enabled-gate-ignored) target.
Tests: registry suppressed vs hop vs terminal (incl. structured-hop-not-suppressed
when OCR off but structured available); _parse_pdf_tier records suppressed +
indexes without raising.
Deck #324 (parent #323).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A stall is often systemic (a Qdrant/embedding outage stalls every in-flight
job), so reclaiming the whole batch at now() every */5min tick would
thundering-herd a recovering dependency, bypassing TieredEscalationStrategy's
per-job backoff. reclaim_stalled_ingest_jobs now offsets retry_at by a fixed
delay (INGEST_RECLAIM_RETRY_DELAY_SECONDS, default 30s; 0 = legacy immediate).
Also document the hot-vs-restart flag asymmetry: INGEST_ESCALATION_ENABLED is
re-read per job; INGEST_TRANSIENT_MAX_ATTEMPTS is snapshotted at worker startup.
Deck #323.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- metrics: update_ingest_queue_depth guarded on `not by_queue`, which conflated
None (memory backend no-op) with {} (postgres, ALL queues drained). When every
queue drains at once, get_ingest_job_counts_by_queue returns {} and the
pre-zero loop was skipped, leaving a stale ghost backlog in the gauge. Guard on
`by_queue is None` only; add an all-drained regression test.
- procrastinate: note that INGEST_TRANSIENT_MAX_ATTEMPTS is snapshotted at
blueprint-build time (restart to pick up changes).
Deck #323.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- tests: make the transient-backoff progression assertion load-independent by
bracketing the get_retry_decision call with before/after timestamps instead of
measuring against a second datetime.now() (no freezegun dependency).
- tests: use pytest.approx for the ingest-queue-depth gauge assertions —
SonarCloud python:S1244 (float == ) was a MAJOR reliability finding that
tripped the new_reliability_rating quality gate.
- processor: tighten the EscalateError lazy-bind comment (file processing already
imports the document stack via get_registry; the gating only spares the
delete / text-doc paths and module-load time).
Deck #323.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- metrics: update_ingest_queue_depth now pre-zeroes every managed ingest queue
before applying live counts, so a queue that drains to empty (and drops out of
procrastinate's list_queues_async) reads 0 instead of sticking at its last
non-zero value (ghost backlog in Grafana/alerts). Adds a regression test.
- procrastinate: comment that _is_transient_infra_error treats all qdrant errors
as transient deliberately (bounded same-tier retry; over-broad is acceptable).
- escalation: note next_tier is the building block; production routing uses
ProcessorRegistry.next_available_tier.
- tests: add evaluate_escalation fast+ocr-only low-confidence -> ocr case.
Deck #323.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Register the periodic stalled-job reclaim on a dedicated ingest-maintenance
queue that every worker drains (any --tier), so reclaim still fires when the
fast fleet is scaled to zero and only ocr workers run. procrastinate's
periodic-defer dedup keeps it single-run across drainers.
- escalation: mark `unsupported`/`forced` reason labels as reserved (not raised).
- processor: note that options/progress_callback are intentionally not threaded
through _parse_pdf_tier yet (symmetric with the inline path).
- tests: assert TieredEscalationStrategy backoff progression (4/8/16/…/300s);
cover get_ingest_pending per-queue aggregation + the legacy job_counts
fallback; add an external-path zero-page no-escalation case; use the canonical
INGEST_QUEUE_FAST instead of the back-compat alias.
Deck #323.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Split external (procrastinate) document processing into per-tier queues so a
document is attempted at most once per tier and requeued to the next tier's
queue on a low-quality parse, using procrastinate's native retry.
- escalation.py: TIER_LADDER (fast->structured->ocr) + EscalateError signal
- registry: process_tier (one tier) + evaluate_escalation post-parse gate
(reuses classify_from_text) + next_available_tier; shared _classify_result
and _oversize_result with the inline pipeline
- processor: process_document(tier=...) runs one tier and raises EscalateError
before embed (junk text never indexed); inline memory path unchanged
- queue/procrastinate: ingest-fast|structured|ocr queues; TieredEscalationStrategy
(queue-hop on EscalateError, bounded same-tier transient retry); queue-aware
task; producer defers to ingest-fast; per-queue counts + all-queue reclaim
- cli: worker --tier {fast,structured,ocr}
- billing: pages_ocr usage event + pipeline_tier metadata (paid OCR billed apart)
- observability: astrolabe_ingest_queue_depth{queue,status} gauge + per-queue
counts in nc_get_vector_sync_status / management status endpoint
- config: INGEST_ESCALATION_ENABLED (default true), INGEST_TRANSIENT_MAX_ATTEMPTS
INGEST_ESCALATION_ENABLED=false and INGEST_QUEUE=memory preserve prior behaviour.
Deck #323.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Quality-gate fixes (new-code conditions on PR #902):
- new_security_hotspots_reviewed: drop the fake "http://nextcloud" host in the
manager tests to https:// (python:S5332 ×2).
- new_security_rating: generate the integration test's fake app password with
secrets.token_urlsafe instead of a hardcoded literal (python:S2068).
- new_reliability_rating: restructure the user_manager sleep so an explicit
await checkpoint lives inside the cancellation scope — await one waiter
directly while watching shutdown via start_soon (python:S7490). Behaviour is
unchanged: timeout, shutdown, or a provisioning ring all end the sleep.
Review nits:
- Move the shutdown test's fail_after(2) to wrap the whole task group so it
actually bounds the task-group exit (was guarding a no-op sleep); drop the
sleep(0) stub (python:S7491).
- Type _wake_on's wait_fn as Callable[[], Awaitable[object]].
- Note in _wire_vector_sync_state why provision_signal is set on the singleton
only, not fanned out to app.state.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- ProvisionSignal.wait() re-arms in a finally so a cancelled wait (shutdown
racing the doorbell) leaves a fresh unset event, not a stale set-but-consumed
one; preserves the no-await-before-swap lost-wakeup guarantee.
- Hoist user_manager_task's _wake_on helper out of the while loop (one object,
not one per iteration).
- Test: assert ProvisionSignal via its public wait() contract instead of the
private _event attribute.
- Add test_provision_app_password_wakes_user_manager covering the
api/passwords.py wake path (previously only LFv2 web + MCP tool were tested).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Background vector sync discovered newly provisioned users only on the
periodic user-manager poll (VECTOR_SYNC_USER_POLL_INTERVAL, default 60s),
delaying first indexing by up to a minute. Add a ProvisionSignal doorbell
that provisioning paths ring after storing a user's app password, waking
user_manager_task to re-poll and spawn the user's scanner at once. The
periodic poll remains the backstop (covers cross-replica provisioning).
- ProvisionSignal (stable reference, wait-and-re-arm) held on
VectorSyncState; closes the lost-wakeup window (no await between observing
the ring and re-arming; anyio.Event stickiness covers a mid-poll ring)
- user_manager_task races its poll timeout against the doorbell + shutdown
- notify_user_provisioned() rung from the three app-password provisioning
sites: Login Flow v2 web, MCP provisioning tool, management/BasicAuth API
Note: the pre-existing scanner_wake_event was never .set() and only wakes
existing scanners; a brand-new user has none, so the manager is what must
be nudged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Round-3 review on PR #891 (no blockers):
- Add test_encode_dav_path_encodes_exactly_once pinning the documented
decoded-input precondition ("already%20encoded.pdf" -> "already%2520...").
- format_exception_group: proper singular/plural ("1 sub-exception" vs
"N sub-exceptions") instead of "(s)".
- oauth_sync: use `if doc_task is not None:` to match processor_task's guard.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Round-3 review on PR #893:
- record_qdrant_operation("upsert","error") now fires only when the exhausted
retry was actually a Qdrant failure (reason=="qdrant"); an embed/connection
failure exhausts retries before Qdrant is called, so attributing it to
mcp_qdrant_operations_total{error} inflated that signal. The cause is still
captured by record_ingest_dropped.
- Add test_mistral_embed_retries_on_5xx: exercises the full Mistral retry path
(5xx SDKError then success), not just the predicate.
- Add test_generate_does_not_retry_on_bad_request: generate() fast-fails on a
permanent 4xx.
- Move astrolabe_vector_ingest_dropped_total's definition into the astrolabe_
pipeline-metrics block (was in the mcp_ section).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Round-2 review on PR #893:
- Add test_generate_retries_on_connection_error (generate() shares the transient
retry; guards the decorator against accidental removal).
- Add test_process_document_records_drop_on_exhausted_retries: drives
process_document to retry-exhaustion and asserts record_ingest_dropped is
called once with the classified reason (processor-level coverage, not just the
_drop_reason unit).
- Note in _drop_reason that a multi-failure group is labelled by its first leaf
(best-effort, no "mixed" bucket).
SonarCloud: the quality gate was failing on new_security_hotspots_reviewed
(S5332 "use https") from http:// URLs in the test _req() helpers — switched to
https:// (mirrors commit 98c9d58e).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
