Commit Graph
2737 Commits
Author SHA1 Message Date
Chris CoutinhoandClaude Opus 4.8 b7479b0d07 docs(review): correct reconcile docstring + clarify scanner rename comment
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>
2026-06-05 01:23:13 +02:00
Chris CoutinhoandClaude Opus 4.8 a03bc0af66 fix(review): guard placeholder in scanner reconcile + test dual-write path
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>
2026-06-05 01:18:58 +02:00
Chris CoutinhoandClaude Opus 4.8 bded41de5d feat: use Nextcloud filename for indexed file title + reconcile on rename
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>
2026-06-05 01:16:45 +02:00
github-actions[bot] 967298ddbe bump: version 0.101.4 → 0.102.0 2026-06-04 22:47:40 +00:00
Chris CoutinhoandGitHub 93c1fb9b95 Merge pull request #855 from cbcoutinho/feat/tiered-doc-processor-b1-classifier
feat: tier-0 document classifier (shadow mode)
2026-06-05 00:47:19 +02:00
Chris CoutinhoandClaude Opus 4.8 5a90ebabf2 test(review): use pytest.approx for float assertions (SonarQube S1244)
SonarQube flagged three float equality checks in the classifier tests
(python:S1244, "do not perform equality checks with floating point values"):
the _text_quality empty case and the ocr_page_fraction 0.0/1.0 assertions now
use pytest.approx.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-05 00:38:53 +02:00
Chris CoutinhoandClaude Opus 4.8 23cc6cca43 test(review): pin the image_heavy-flag-without-ocr-routing invariant
Address PR #855 round 3 (non-blocking test completeness):

- Add a test that a mostly-digital doc with one full-page image carries the
  image_heavy flag yet still routes fast (ocr_frac < OCR_PAGE_FRACTION) -- the
  flag-vs-routing asymmetry operators read in the metrics, now guarded against
  silent regression.
- test_full_page_image_routes_ocr also asserts the scanned flag (no text layer).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-05 00:36:25 +02:00
Chris CoutinhoandClaude Opus 4.8 4bdb0bc6d6 fix(review): warn (not debug) on shadow-classify failure; tidy pymupdf usage
Address PR #855 round 2:

- 🔴 _shadow_classify swallowed all exceptions at DEBUG, so a systematic
  failure (pymupdf bug, memory pressure) is invisible at LOG_LEVEL=INFO and
  trips SonarQube S2221/S5754. Log at WARNING instead (still best-effort --
  indexing is unaffected).
- classifier: use `with pymupdf.open(...) as doc` instead of manual try/finally.
- tests: release the Pixmap's native memory (del pix) in the image fixtures.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-05 00:22:17 +02:00
Chris CoutinhoandClaude Opus 4.8 0347e96679 fix(review): sample last page, document flags-vs-routing, add flag-path tests
Address PR #855 review (all non-blocking):

- classifier: _sample_indices now always includes the first AND last page (the
  old evenly-spaced sample missed the tail, e.g. last sampled index 95 on a
  100-page doc -- a scanned tail could be missed).
- classifier + metrics: document that flags are diagnostic and fire
  independently of routing (image_heavy on ANY page vs the ocr route needing a
  page FRACTION), so flag{image_heavy} is expected to exceed classified{ocr}.
- classifier: clarify the text-quality whitespace comment (caps at 12%) and note
  the image double-count approximation (min() caps coverage).
- tests: add the scanned (no text layer) and bad_text_layer (junk text over an
  image) flag paths, and a test pinning first/last-page sampling.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-05 00:12:26 +02:00
Chris CoutinhoandClaude Opus 4.8 044c1da750 feat: tier-0 document classifier in shadow mode
First step of the tiered document-processor effort (Deck #203): a cheap, local
pre-pass that recommends which extraction tier a PDF should start in, emitting
metrics WITHOUT changing routing yet -- so we gather per-tenant doc-mix data
before turning escalation on.

document_processors/classifier.py: classify_pdf(content) -> DocClassification.
Page-sampled (bounded on large docs), <~1s. Cheap signals only -- text-layer
chars, a text-quality score (catches the "Student 147" failure where a text
layer exists but is mashed/space-less junk), and image coverage. A page that is
mostly a raster image routes to OCR: its content (handwriting, stamps) isn't in
any text layer. Deliberately no get_drawings/graphics-density signal -- it's
slow on the exact pages it'd flag, the hotfix's graphics_limit already makes the
parse safe, and the (future) tier-1 quality gate catches lost tables.

Validated on the sample corpus: born-digital 2-col arxiv and a digital student
record -> fast (tier 1); a scanned+handwritten form -> ocr (tier 3).

Wiring (vector/processor.py): _shadow_classify runs the classifier on PDFs in a
worker thread, best-effort (never blocks/fails indexing), gated by the new
DOCUMENT_CLASSIFY_ENABLED setting. Metrics: astrolabe_document_classified_total
{recommended_tier}, astrolabe_document_classifier_flag_total{flag},
astrolabe_document_text_quality histogram.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-05 00:12:25 +02:00
github-actions[bot] dd335275ac bump: version 0.101.3 → 0.101.4 2026-06-04 22:09:33 +00:00
Chris CoutinhoandGitHub 650b740e17 Merge pull request #856 from cbcoutinho/fix/graphics-limit-timeout-tuning
fix: lower DOCUMENT_PDF_GRAPHICS_LIMIT default 5000 → 1000 (parse timeouts)
2026-06-05 00:09:12 +02:00
github-actions[bot] dc01e55b60 bump: version 0.101.2 → 0.101.3 2026-06-04 22:06:48 +00:00
Chris CoutinhoandClaude Opus 4.8 77a36fa821 fix: lower DOCUMENT_PDF_GRAPHICS_LIMIT default 5000 -> 1000
The OOM hotfix (#852) set graphics_limit=5000, which caught the 955k-drawing
bomb but let a second pathology through: form/table PDFs (e.g. student records)
have ~1.5k grid-line vector drawings per page -- under 5000, so uncapped. With
those pages uncapped, pymupdf4llm's O(n^2) find_tables grinds ~17s/page, so a
7-page form hits the 120s timeout. All 6 current backfill parse failures in
tenant-blackbox-demo are this exact timeout (zero OOM, zero error).

Measured on a 7-page sample: graphics_limit=2000 -> 119s (timeout), 1000 -> 2.9s
-- with identical extracted text and ZERO recovered tables either way (the
expensive analysis produces nothing useful on these dense forms). Lowering the
default to 1000 makes them index in ~3s; the bomb file (955k >> 1000) stays
capped, and pages with genuine simple tables (<1000 drawings) still get table
detection.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-05 00:06:40 +02:00
Chris CoutinhoandGitHub d48a669624 Merge pull request #854 from cbcoutinho/fix/astrolabe-multi-user-integration-tests
fix(tests): repair multi-user-basic Astrolabe integration suite
2026-06-05 00:06:26 +02:00
Chris CoutinhoandClaude Opus 4.8 042b9aa295 fix(tests): moderate re-scan interval to stop multi-user index churn
Lowering VECTOR_SYNC_SCAN_INTERVAL to 5s (previous commit) fixed user
discovery on nc31 but exposed re-scan churn on the slower nc32 runner: each
scan re-queues the user's entire corpus, so a 5s cadence floods the single
processor worker faster than it drains (pending climbed to 20-30+ while
indexed stayed 0, status "syncing").

Discovery latency and re-scan churn are separate knobs. Keep
USER_POLL_INTERVAL short (5s) for prompt discovery — the per-user scanner
runs its initial scan immediately on start, so the corpus is queued once
right away — but restore a moderate SCAN_INTERVAL (30s) so re-scans don't
re-flood the queue. Indexing of the one-time initial scan completes well
inside the test's 90s budget; a note created just after that scan is still
picked up by the 30s re-scan.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 23:49:03 +02:00
Chris CoutinhoandClaude Opus 4.8 533bd79949 fix(tests): fast vector-sync cadence for multi-user-basic CI service
After restoring the background-sync UI ids, the two indexing-dependent
multi-user-basic tests (chunk_context_uses_app_password, plotly_with_basic_auth)
surfaced a second, previously-masked failure: they provision a user, create a
note, then wait ~90s for it to be indexed.

The mcp-multi-user-basic service ran with the production cadence — scan
interval 60s and the default user-poll interval 60s. A freshly-provisioned
user isn't even *discovered* by the background-sync user manager for up to
60s, leaving too little of the 90s budget for the scan + single-worker
indexing to finish (observed: pending docs still "syncing" at timeout, or the
scanner not yet started → "idle" with 0 indexed).

Match the single-user service's short cadence (5s) and add a matching 5s
user-poll interval so discovery + scan + index complete well within the test
budget. Test-only config; production deployments set their own intervals.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 23:33:03 +02:00
Chris Coutinho 338fb78199 ci: add astrolabe submodule to docker-compose 2026-06-04 22:55:10 +02:00
Chris CoutinhoandClaude Opus 4.8 531607a1a1 fix(tests): repair multi-user-basic Astrolabe integration suite
The Astrolabe PHP→Vue settings refactor dropped three stable element ids
(#mcp-enable-background-button, #mcp-revoke-background-button,
#mcp-revoke-background-form) that the multi-user-basic integration suite
drives the background-sync enable/disable/revoke flows through. Their
absence timed out the 5s Playwright locators and failed four tests:

- test_astrolabe_multi_user_background_sync::test_multi_user_astrolabe_background_sync_enablement
- test_astrolabe_multi_user_background_sync::test_revoke_background_sync_access
- test_astrolabe_chunk_context::test_chunk_context_endpoint_uses_app_password
- test_astrolabe_plotly_visualization::test_astrolabe_plotly_visualization_with_basic_auth

(the latter two enable background sync via complete_astrolabe_authorization
before exercising the app-password / indexed-search paths).

Two-part fix:

1. Bump the astrolabe submodule to v0.20.1 (cbcoutinho/astrolabe#116),
   which restores the three element ids on the refactored NcButtons.

2. Defense-in-depth in the test helpers: resolve the enable/revoke buttons
   by their stable id first, falling back to the button's accessible name
   so a future id rename degrades to a slower-but-working lookup instead of
   a hard timeout. Avoids a combined `.or_()` locator, which would
   strict-mode-violate (the id button also matches by text).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 22:53:43 +02:00
github-actions[bot] eb5f59f085 bump: version 0.101.1 → 0.101.2 2026-06-04 20:43:01 +00:00
Chris CoutinhoandGitHub 30d05a6f7d Merge pull request #852 from cbcoutinho/fix/pdf-parse-oom-isolation
fix: isolate PDF parse in a subprocess so a pathological file can't OOM the pod
2026-06-04 22:42:37 +02:00
Chris CoutinhoandClaude Opus 4.8 93f0f4f881 fix(review): type timeout as float; document worker reuse + identity check
Address PR #852 round 3 (all 🟡, no blockers):

- config: DOCUMENT_PARSE_TIMEOUT_SECONDS is now float (default 120.0) so a
  fractional value is honoured rather than silently stored in an int field;
  matches anyio.move_on_after's float seconds.
- _isolation: comment that a clean rlimit MemoryError leaves the worker alive
  in anyio's pool (vs the SIGKILL/BrokenWorkerProcess path that respawns) --
  acceptable since RLIMIT_AS caps virtual address space, not RSS.
- processor: note the `if indexed is False` is a deliberate identity check --
  a successful index (incl. dedup hit) returns None and must not be mistaken
  for a parse failure.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 22:32:35 +02:00
Chris CoutinhoandClaude Opus 4.8 6589e8e7fc fix(review): require graphics_limit>=1, type _index_document, cover rlimit branch
Address PR #852 round 2:

- config: DOCUMENT_PDF_GRAPHICS_LIMIT validator is now gte=1 (pymupdf4llm treats
  0 as "no cap", which would re-expose the OOM); documented the zero semantics
  and that the per-worker mem rlimit needs a pod restart to change.
- processor: annotate `_index_document -> bool | None` and document the contract
  so the `if indexed is False` check is explicit/type-checkable.
- tests: add the RLIM_INFINITY-hard branch assertion for _apply_mem_limit
  (soft==target, hard stays unbounded).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 22:32:34 +02:00
Chris CoutinhoandClaude Opus 4.8 7ec116a3c7 fix(review): close doc via try/finally; don't count parse failures as indexed
Address PR #852 review:

- pymupdf.py: the metadata `doc` was only closed on the PdfParseFailed and
  success paths, so a failure in `_extract_metadata`/`mkdir`/`get_settings`
  leaked it. `doc` is only needed for metadata + page_count (the heavy parse
  works from `content` bytes in the worker), so open it, read metadata, and
  close it immediately under try/finally; drop the two later doc.close() calls.

- processor.py: a permanent parse failure early-returned from `_index_document`,
  after which `process_document` still recorded record_qdrant_operation("upsert",
  "success") + record_vector_sync_processing(success) -- counting an OOM/timeout
  bomb as astrolabe_documents_indexed_total{status="success"}. `_index_document`
  now returns False on that path and the caller skips the success metrics (the
  failure is already recorded via document_parse_failed_total + the registry's
  document_parse_total{error}).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 22:32:34 +02:00
Chris CoutinhoandClaude Opus 4.8 7db8d3e301 fix: isolate PDF parse in a subprocess so a bad file can't OOM the pod
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>
2026-06-04 22:32:34 +02:00
github-actions[bot] 09e84783e5 bump: version 0.101.0 → 0.101.1 2026-06-04 20:32:07 +00:00
Chris CoutinhoandGitHub e1e8af0426 Merge pull request #853 from cbcoutinho/fix/vector-sync-metrics-startup-nameerror
fix: resolve startup NameError in vector-sync metrics task
2026-06-04 22:31:44 +02:00
Chris CoutinhoandClaude Opus 4.8 1f8b3ba95e fix: resolve startup NameError in vector-sync metrics task
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>
2026-06-04 22:10:40 +02:00
Chris Coutinho c47467f769 build: Bump astrolabe submodule 2026-06-04 21:33:05 +02:00
github-actions[bot] af4a264aa3 bump: version 0.100.0 → 0.101.0 2026-06-04 19:27:41 +00:00
Chris CoutinhoandGitHub d95ed7be68 Merge pull request #850 from cbcoutinho/feat/ingest-pending-documents-metric
feat: backend-agnostic vector-sync gauges (pending / documents / chunks)
2026-06-04 21:27:18 +02:00
github-actions[bot] b84ac24d10 bump: version 0.99.0 → 0.100.0 2026-06-04 19:22:47 +00:00
Chris CoutinhoandGitHub ddfba901fa Merge pull request #851 from cbcoutinho/feat/ingest-transport-port
feat: IngestTransport port for local/distributed ingest backends
2026-06-04 21:22:24 +02:00
Chris CoutinhoandClaude Opus 4.8 6c7b679b52 docs: correct ADR-028 aclose() description (PR #851 round 6 nit)
LocalTransport.aclose() (added in round 3) closes its owned stream ends; the ADR
still described aclose() as a no-op for the memory stream. Update the prose to
match the shipped behaviour. Doc-only.

Refs: Deck #196

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 20:53:36 +02:00
Chris CoutinhoandClaude Opus 4.8 bf84db35b4 refactor: address PR #851 review round 5 (ingest transport)
- _clear_vector_sync_state also nulls shutdown_event / scanner_wake_event on
  shutdown, symmetric with the stream/producer fields (the next startup rebinds
  them via _wire_vector_sync_state).
- Comment that the "DocumentTask" string subscript in LocalTransport is
  intentional (TYPE_CHECKING-only class; anyio ignores the runtime type arg).
- Move app.py's annotation-only IngestTransport / TaskProducer imports under
  TYPE_CHECKING (the module uses `from __future__ import annotations`), keeping
  only build_transport at runtime.

Refs: Deck #196

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 20:49:17 +02:00
Chris CoutinhoandClaude Opus 4.8 c4401af9c6 refactor: address PR #851 review round 4 (ingest transport)
- Clear the module-singleton ingest references (task_producer,
  document_send_stream, document_receive_stream) on lifespan shutdown via a new
  _clear_vector_sync_state() helper, mirroring the eviction_task_group cleanup.
  Defense-in-depth so a late webhook (or a module-singleton integration test)
  can't touch a producer/stream backed by an already-closed resource.
- Add IngestTransport.backend_name ("memory"/"postgres") and use it in both
  lifespan log lines, removing the last settings.ingest_queue read from the
  background-sync setup — the lifespan no longer inspects the backend at all.
- Cover backend_name in the build_transport adapter-selection tests.

Refs: Deck #196

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 20:42:57 +02:00
Chris CoutinhoandClaude Opus 4.8 c0c52c1b34 refactor: address PR #851 review round 3 (ingest transport)
- LocalTransport.run_consumers increments active_consumer_count per worker
  (instead of once after the loop) so the count is accurate if a later
  tg.start() raises mid-pool.
- Add LocalTransport.aclose() to explicitly close its owned send/receive stream
  ends (belt-and-suspenders against unclosed-resource warnings; anyio aclose is
  idempotent, and by shutdown the scanner is already winding down). Reworded the
  base IngestTransport.aclose() docstring to point at the overrides.
- Inline ingest_transport.producer at the scanner/user_manager call sites,
  dropping the single-use task_producer alias in both lifespan paths.
- Annotate DistributedTransport._producer explicitly as ProcrastinateTaskProducer
  so the drain() coupling is visible and ty catches drift.
- Add a unit test for LocalTransport.aclose() (closes the owned streams,
  idempotent).

Refs: Deck #196

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 20:36:58 +02:00
Chris CoutinhoandClaude Opus 4.8 e4d81d47d9 feat: harmonize MCP tool + userinfo page to documents/chunks model
Extend the documents-vs-chunks split to the remaining status surfaces so all
three report consistently (Deck #195):

- nc_get_vector_sync_status MCP tool + VectorSyncStatusResponse: add
  indexed_documents (distinct) and indexed_chunks; keep indexed_count as a
  deprecated alias of indexed_chunks. Reuses count_indexed.
- userinfo HTML page (/app/vector-sync/status): show Indexed Documents AND
  Indexed Chunks rows; switch its count to count_indexed (which also excludes
  placeholder points — the old raw count included them).
- /api/v1/vector-sync/status: restore indexed_count as a deprecated alias of
  indexed_chunks so existing consumers (integration tests, pre-#115 UI) keep
  working; the change is now purely additive for indexed_count.

Tests: VectorSyncStatusResponse documents/chunks/alias + zeroed defaults.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 20:28:44 +02:00
Chris CoutinhoandClaude Opus 4.8 2179e9ddb0 refactor: address PR #851 review round 2 (ingest transport)
- Rename the lifespan-local `transport` to `ingest_transport` in both paths so it
  no longer shadows the get_app(transport=...) HTTP-transport parameter.
- Log the memory backend selection in build_transport, symmetric with the
  postgres branch, so startup logs name the chosen ingest backend either way.
- Note in _wire_vector_sync_state why eviction_task_group is intentionally not
  set there (it only exists once the lifespan's task group is running).

Refs: Deck #196

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 20:27:40 +02:00
Chris CoutinhoandClaude Opus 4.8 655d608fb7 refactor: address PR #851 review round 1 (ingest transport)
- Add IngestTransport.active_consumer_count (0 by default; LocalTransport stores
  the started count) so app.py logs the worker count without re-checking
  INGEST_QUEUE — the last backend-knowledge leak in the lifespan is gone.
- Document that DistributedTransport is postgres/procrastinate-specific by design
  (aclose() calls ProcrastinateTaskProducer.drain()); other distributed backends
  would be separate IngestTransport subclasses.
- Clarify the _wire_vector_sync_state log line (writes app.state + singleton, not
  only the singleton).
- Strengthen the LocalTransport test: assert active_consumer_count transitions
  0→N and that each worker receives a distinct cloned receive stream.

Refs: Deck #196

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 20:07:19 +02:00
Chris CoutinhoandClaude Opus 4.8 8c9f97d6c4 docs: clarify postgres-mode None + test exact=True default (review #850)
- Note at both metrics-task call sites that receive_stream is None in postgres
  mode (get_ingest_pending falls back to procrastinate counts).
- Add test_default_is_exact_true covering the status-endpoint count path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 19:47:29 +02:00
Chris CoutinhoandClaude Opus 4.8 e7bcdb1950 feat: add IngestTransport port for local/distributed ingest backends
Finish the hexagonal ports-&-adapters split started in #183. The producer side
already had a TaskProducer port + adapters, but the consumer side was
unabstracted and the INGEST_QUEUE selection leaked into a duplicated
`if use_postgres:` branch across both app.py lifespan paths.

Introduce an IngestTransport ABC (vector/queue/transport.py) that bundles the
producer with running (or not running) the in-process consumer pool, built by a
single build_transport() factory:

- LocalTransport (INGEST_QUEUE=memory): in-process anyio stream drained by an
  N-worker pool that run_consumers starts.
- DistributedTransport (INGEST_QUEUE=postgres): wraps ProcrastinateTaskProducer;
  run_consumers is a no-op because the consumer is the external `worker` role.

Both lifespan paths now call build_transport + _wire_vector_sync_state (new
helper that centralizes the app.state / module-singleton / browser-app writes) +
transport.run_consumers + transport.aclose(), with no INGEST_QUEUE branching and
no getattr drain probe. Adding a future backend (Redis/NATS/SQS) is one new
adapter + one build_transport arm, with no app.py or scanner change.

Preserves the single-tenant parallelism invariant (one shared multiplexed queue
+ N-worker pool, per-document not per-user dispatch) and documents it in
ADR-028. The worker CLI is unchanged (it is the external consumer).

Refs: Deck #196 (Deck #197 tracks the explicit parallelism regression test)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 19:43:31 +02:00
Chris CoutinhoandClaude Opus 4.8 fc3e0f28f6 fix: add metrics-interval validator + type/test gaps (review #850)
- Add Validator("VECTOR_SYNC_METRICS_REFRESH_INTERVAL", gte=1) so a 0/negative
  value can't turn the publish loop into a busy-spin.
- Annotate count_indexed's qdrant_client param as AsyncQdrantClient.
- Add tests: exact kwarg is forwarded to qdrant count, and the placeholder
  filter matches False (excludes placeholders) with chunk_index pinned to 0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 19:39:46 +02:00
Chris CoutinhoandClaude Opus 4.8 fbe70ecd9c feat: backend-agnostic vector-sync gauges (pending/documents/chunks)
The only queue metric, mcp_vector_sync_queue_size, was updated inline by the
single-user consumer (processor_task) but never by the multi-user consumer
(oauth_processor_task). On multi-user tenants (e.g. blackbox-demo, 5 users) the
gauge read 0 for 24h while the live anyio buffer held ~2214 pending documents
(shown by /api/v1/vector-sync/status). The "indexed" figure was also a chunk
count (16039 points ≈ 480 docs) mislabelled as documents.

Publish a consumer-independent snapshot from a periodic task
(vector/metrics_publisher.vector_sync_metrics_task), spawned in BOTH lifespan
task groups (single-user and multi-user) and every queue backend:
- mcp_vector_sync_pending_documents — outstanding work via
  ingest_status.get_ingest_pending() (anyio buffer depth or procrastinate
  todo+doing); also keeps the legacy queue_size gauge meaningful on all paths.
- mcp_vector_sync_indexed_documents — distinct documents, counted exactly and
  cheaply via the one chunk_index=0 point per document (no facet).
- mcp_vector_sync_indexed_chunks — total non-placeholder points.

The /api/v1/vector-sync/status endpoint now returns indexed_documents (distinct
docs) AND indexed_chunks separately, so documents and chunks are no longer
conflated. The publisher uses approximate Qdrant counts (every-N-seconds gauge);
the on-demand endpoint counts exactly. New knob:
VECTOR_SYNC_METRICS_REFRESH_INTERVAL (default 20s). Fail-safe: a metrics refresh
never disturbs ingest.

BREAKING CHANGE: /api/v1/vector-sync/status field `indexed_documents` now holds
the distinct-document count (was the chunk count); the chunk count moved to the
new `indexed_chunks` field. The Astrolabe UI + the nc_get_vector_sync_status MCP
tool / userinfo page are harmonized in a follow-up (Deck #195).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 19:30:10 +02:00
github-actions[bot] 55ea8dd358 bump: version 0.98.1 → 0.99.0 2026-06-04 16:00:02 +00:00
Chris CoutinhoandGitHub a7d043e43c Merge pull request #848 from cbcoutinho/feat/vector-sync-cross-user-dedup
feat: dedup shared-file parsing/embedding across users in vector sync
2026-06-04 17:59:31 +02:00
Chris CoutinhoandGitHub aa0aa5dc6b Merge pull request #849 from cbcoutinho/fix/vector-sync-pagination-churn-observability
fix: paginate tagged-folder SEARCH so the vector-sync scanner discovers all files
2026-06-04 17:59:17 +02:00
Chris CoutinhoandClaude Opus 4.8 9452057570 fix(webdav): harden offset/key truthiness and escape SEARCH mime type
Optional review hardening on #849 (non-blocking nits from the approve):

- `_build_search_xml`: emit `<d:firstresult>` on `offset is not None` rather
  than truthiness, so a future explicit offset=0 isn't silently dropped.
- `_key`: key on `file_id is not None` so a (hypothetical) file_id of 0 isn't
  treated as absent and mis-keyed onto path.
- `_type_search_args`: XML-escape the MIME type before interpolating it into
  the SEARCH literal (defense-in-depth for any future user-supplied value),
  with a unit test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 14:19:30 +02:00
Chris CoutinhoandClaude Opus 4.8 eaa898e6eb fix(webdav): await fallback, guard dedup key, split paging for complexity
Address review on #849:

- Critical: add missing `await` on the exception-path fallback in
  search_files_all -- it returned a coroutine instead of the result list.
  Add unit tests for both the offset-page-raises (fallback) and
  offset-zero-raises (propagate) paths, which previously had no coverage.
- Guard `_key` dedup against items missing both file_id and path (fall back
  to id(item)) so they can't collapse under a shared None key and drop rows.
- Document the offset-ignored discard-and-refetch decision.
- Split the offset paging into `_search_offset_paged` (returns None to signal
  fallback) and share the truncation warning via `_warn_if_truncated`,
  cutting cognitive complexity below the threshold (SonarCloud S3776).
- Make the test side_effect helpers synchronous (SonarCloud S7503).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 13:44:37 +02:00
Chris CoutinhoandClaude Opus 4.8 31eb2d4e74 docs: explain pure-claimer eviction path in scanner (review #848)
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>
2026-06-04 13:24:41 +02:00