Address claude-review round 5 on PR #868: add
test_all_blank_pages_returns_empty_list documenting that PageAwareChunker
returns [] when every page is blank — and asserting parity with
DocumentChunker, which already returns [] for whitespace-only non-empty
content. The empty-chunk-list case is therefore pre-existing pipeline
behavior, not new to this PR.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address claude-review round 4 on PR #868: tighten the assign_page_numbers
guard from `page_boundaries is not None` to a truthy check, so a PDF with an
empty boundary list no longer enters the trace span and fires the alarming
"NO page numbers assigned" warning for a harmless no-op.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address claude-review round 3 on PR #868: add module-level
`pytestmark = pytest.mark.unit` so TestPageAwareChunker and
TestDocumentChunkerPositions are collected under `-m unit`, matching
test_processor_routing.py.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address claude-review round 2 on PR #868:
- Extract the use_page_aware branching into a pure `should_use_page_aware`
helper and cover the (doc_type, page_boundaries, page_aware_setting) matrix
in tests/unit/test_processor_routing.py (file+boundaries+enabled, empty
list, None, non-file doc types, disabled setting).
- Clarify the PageAwareChunker.chunk_text no-boundaries comment: the processor
pre-filters via should_use_page_aware, so that branch is a direct-call safety
net, not a production indexing path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address claude-review round 1 on PR #868:
- use_page_aware now gates on `bool(page_boundaries)` instead of
`is not None`, so a PDF that yields an empty boundary list takes the
char-based path explicitly (assign_page_numbers no-ops on []) rather than
the page-aware chunker's no-boundaries fallback. Same result, clearer intent.
- add test_oversized_page_with_leading_whitespace_offsets, exercising the
start+start_index offset path for an oversized page whose sub-chunks have
leading whitespace.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
DOCUMENT_CHUNK_SIZE/OVERLAP were documented as "words" with a 512/50
default; the implementation measures characters and defaults to 2048/200
(config.py, DocumentChunker). Update docs/configuration.md (config block,
tuning guidance, examples, env-var table) and env.sample accordingly, and
cross-reference DOCUMENT_CHUNK_PAGE_AWARE for the PDF path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add PageAwareChunker, which splits paginated documents (PDFs) on page
boundaries first and only character-splits pages larger than chunk_size.
No chunk spans a page boundary, so page_number is always exact and stored
excerpts never lead with a neighbouring page's text. When chunk_size is at
least the largest page, this yields exactly one chunk per page: a
predictable vector count (== page count), a flat per-page embedding cost,
and zero cross-page overlap duplication.
Gated by DOCUMENT_CHUNK_PAGE_AWARE (default true). When false, the legacy
char-based DocumentChunker + post-hoc assign_page_numbers path runs
unchanged. Only doc_type="file" with page_boundaries (PDFs) takes the
page-aware path; notes/deck/news are unaffected.
Measured on a 15-page record (query "leadership award louis", target =
top-half of page 15): char-based degraded the target to dense-rank 10 at
cs=2048 (OCR) and mislabeled its page; page-aware restored rank 1 across
every fusion/modality and chunk size, with correct page labels and clean
snippets.
BREAKING CHANGE: PDFs are re-chunked page-aware by default. Existing
deployments will re-index PDF content on the next vector sync (different
chunk counts and page_number labels). Set DOCUMENT_CHUNK_PAGE_AWARE=false
to retain the previous char-based behaviour.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Tagging an existing file/folder emits only OCP\SystemTag\MapperEvent — never a
Node*Event — so tagged PDFs were previously only picked up by the hourly
scanner. Subscribe to the tag event and reconcile membership so adding/removing
the `vector-index` tag (re)indexes in near-real time.
- webhook_presets: add OCP\SystemTag\MapperEvent to the files_sync preset
(NC 32+, where MapperEvent gained getWebhookSerializable(); harmless on older
servers — it just never fires).
- webhook_parser: parse MapperEvent (objectType=files) into a path-less file
"reconcile" task. The payload carries only a fileid + tagIds (no name/path),
so assign and unassign both collapse to a reconcile.
- processor._reconcile_tag_event: resolve the fileid against the user's current
vector-index PDFs (find_files_by_tag). Present -> index with the resolved
path/etag; absent -> flip to delete. Naturally handles "an unrelated tag
changed" and a tagged folder's own fileid (no-op; the scanner still expands
folders to descendants).
- Unit tests for the parser branch and the reconcile.
The matching admin-UI preset change ships separately in the astrolabe app repo.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address PR #863 round 4: classify_from_text's docstring now states that the
image_heavy flag (and the image-coverage trigger) are only set when
image_coverage is supplied, so the flag reads zero for tenants with
DOCUMENT_OCR_DETECT_SCANNED=false -- self-documenting the metric semantics.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address PR #863 round 3:
- classify_from_text emits the "scanned" flag (was "no_text_layer") for the
empty-text-layer case -- same name + meaning as classify_pdf, so
astrolabe_document_classifier_flag_total isn't split across two labels for the
same concept (and matches the metric's documented vocab).
- classify_from_text logs at DEBUG when image_coverage length != the expected
min(pages, MAX_SAMPLED_PAGES), so a 1:1-alignment contract break (extractor
reorders/skips pages) surfaces instead of silently misattributing coverage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address PR #863 round 2:
- classify_pdf now flags a page needs_ocr on the SAME three signals as
classify_from_text (image scan OR low text-quality OR near-empty), not image
coverage alone. Previously a word-merged digital doc with no images routed
"fast" via classify_pdf but "ocr" via the pipeline -- so an operator
reproducing routing offline got a different answer. They now match.
- Add a test that when image_coverage is shorter than the page boundaries (the
MAX_SAMPLED_PAGES cap on large scans), the leading page uses the scan signal
and later pages fall back to text-quality.
Left as-is: overlong_score (>20) partially overlaps merge_score (>12) -- the
double-penalty on very-long tokens is intentional, not a bug (per review).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address PR #863 review:
- MIN_TEXT_QUALITY 0.45 -> 0.5 so the module/diagnostic default matches the
DOCUMENT_OCR_MIN_TEXT_QUALITY setting (registry always passes the setting; this
keeps classify_pdf and the test/default path on the production threshold).
- image_coverage_per_page is bounded to MAX_SAMPLED_PAGES (the image pass is the
costly part, so a 200-page scan isn't fully rasterised on the hot path); pages
beyond the cap fall back to the text-quality signal, and page_fraction still
gates over every page.
- Extracted _page_image_coverage(page) helper, shared by classify_pdf and
image_coverage_per_page (DRY + keeps the tiling-double-count note in one place).
- Scan-detection failure logs at WARNING (not DEBUG) so a systematic failure on
an OCR-enabled tenant is visible at LOG_LEVEL=INFO.
- Add the missing DOCUMENT_OCR_MIN_PAGE_CHARS range-validator test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The hot-path classifier escalated to OCR purely on character count, so a
scanned/handwritten PDF with a low-quality embedded text layer (>16 chars/page
but garbled) routed `fast` and indexed the junk -- e.g. Student 147.pdf's
"Little Acoms Primary"/"0110912020", which pollutes the vector and demotes the
doc in search (Deck #207).
- classifier: recalibrate `_text_quality` with a long-token-fraction term that
detects word-merging (dropped inter-word spaces) -- the dominant junk-layer
failure the old whitespace/overlong(>20) terms missed. Measured: the Student
147 scan ~0.42 (60% pages junk) vs >=0.94 for clean digital docs.
- classify_from_text now routes on quality + scan: a page is OCR-worthy if
near-empty OR low text-quality OR (when OCR + scan detection are enabled) it's
mostly a raster image. New `image_coverage_per_page` re-opens the PDF for the
scan signal, so that cost is paid only by OCR-opted-in tenants. Thresholds are
passed in from per-tenant settings (keyword-only).
- config: 4 per-tenant settings -- DOCUMENT_OCR_MIN_TEXT_QUALITY (0.5),
DOCUMENT_OCR_PAGE_FRACTION (0.5), DOCUMENT_OCR_MIN_PAGE_CHARS (16),
DOCUMENT_OCR_DETECT_SCANNED (true) -- with range validators.
- metrics: new astrolabe_document_ocr_page_fraction histogram (the value the
page-fraction threshold acts on) alongside document_text_quality, so operators
can tune the OCR escalation per tenant (quality vs cost).
Escalation gate, OCR backends, and off-by-default behavior unchanged (#858).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The high ocr_frac is driven by each segment being shorter than MIN_PAGE_CHARS
(needs_ocr), not by text quality; quality drives bad_text_layer separately.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to the tiered document processor (#858), landing the round-4 review
nits the reviewer approved without:
- pypdfium2_fast: free the page handle in an outer finally so a corrupt page
that makes get_textpage() raise can't orphan it.
- test: classify_from_text junk-text-layer path (non-zero chars, low quality,
high ocr_frac) flags bad_text_layer -- the hot-path coverage gap.
- test: build_ocr_backend raises ValueError when the gateway M2M client_id is
set without its token_url/secret.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address PR #858 review round 3:
- document_tier1_engine / document_ocr_provider now validate + normalize via
Settings.__post_init__ _enum_fields (the repo's canonical opt-in-enum pattern;
case-insensitive) instead of dynaconf Validators. A typo now raises ValueError
at load and "Gateway" normalizes to "gateway".
- classify_from_text gates no_text_layer/bad_text_layer on ocr_frac >=
OCR_PAGE_FRACTION, matching classify_pdf -- a "fast"-routed doc with a few junk
pages no longer emits a misleading flag (keeps the shadow vs hot-path
classification metrics consistent).
- build_ocr_backend warns when an EXPLICIT provider is misconfigured
(gateway without EMBEDDING_GATEWAY_URL, mistral without MISTRAL_API_KEY)
instead of silently returning None.
- Pypdfium2FastProcessor.health_check probes the import; documented why
OcrProcessor.health_check is unconditionally True (lazy per-tenant backends).
- Removed the leftover per-boundary / per-chunk debug logging loops.
Tests: enum normalization + rejection for the two new settings.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address PR #858 review round 2:
- OcrProcessor backend resolution is now guarded by an anyio.Lock (lazy-init,
double-checked) so a burst of concurrent first-OCR calls resolves the backend
once instead of each fetching its own gateway M2M token.
- The document_tier1_engine=pymupdf rollback now logs a warning when it falls
back to the fast processor (no 'structured' registered) instead of silently
using the very engine the operator opted out of.
- classify_from_text defaults ocr_frac to 0.0 (not 1.0) for a zero-page PDF, so
the recorded classification metric is "fast" (no OCR evidence) rather than a
misleading "ocr"; the no_text_layer/bad_text_layer flags are gated on having
sampled at least one page.
New tests: zero-page classify routes fast, rollback-fallback warning.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address PR #858 review:
- 🔴 OcrProcessor now resolves its backend once and reuses it. Rebuilding per
call created a fresh GatewayTokenProvider each time -- discarding its M2M-token
cache, so every OCR'd document fetched a new token -- and a new Mistral client.
- 🔴 build_ocr_backend uses explicit ValueError (not assert, which is stripped
under `python -O`) for the gateway M2M triple.
- PIPELINE_TIER in the Qdrant payload now reflects the tier that actually
produced the doc: the registry stamps result.metadata["pipeline_tier"] and the
processor reads it (was hardcoded "fast", wrong for OCR/structured).
- Escalation now requires classification.page_count > 0, so a zero-page
(empty/corrupt) PDF isn't pointlessly sent to OCR; documented that a fast
FAILURE (encrypted/unopenable) is a hard failure and is not OCR-escalated.
- Documented the OCR page_boundaries separator-attribution choice.
- Downgraded the per-document page-boundary / page-assignment INFO logs to debug.
New tests: zero-page no-escalation, pipeline_tier stamping.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
OCR is an enhancement, not a gate. Previously, escalating a scanned doc to the
OCR tier returned the OCR result unconditionally -- so with DOCUMENT_OCR_ENABLED
=true but no backend configured (no gateway URL / no MISTRAL_API_KEY) the OCR
processor returned success=False and the whole document was marked failed and
skipped: strictly worse than leaving OCR off (where it would at least index the
tier-1 text).
Now the registry keeps the tier-1 fast result when the OCR escalation doesn't
succeed (no backend, API down, empty output), logging a warning. A
misconfiguration degrades gracefully instead of dropping scanned docs.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the OCR escalation target the tiered registry already routes to. Scanned /
no-text-layer PDFs (the tier-0 "ocr" verdict) escalate here when
document_ocr_enabled (default off).
Two interchangeable backends, selected by document_ocr_provider
(auto | gateway | mistral | none):
- gateway: POST to the Astrolabe Cloud model gateway's /v1/ocr -- the same
M2M-authenticated gateway as embeddings, so NO provider keys live in the pod
(the platform default; reuses EMBEDDING_GATEWAY_URL + the M2M creds).
- mistral: call the Mistral OCR API directly from the pod (MISTRAL_API_KEY), for
self-hosters / deployments without the gateway.
"auto" prefers the gateway, then direct Mistral.
Both return per-page markdown joined into text + exact page_boundaries (the
pdf_highlighter contract; bbox re-derived from the PDF bytes as for other tiers).
Validated end-to-end via direct Mistral on the scanned Student 147.pdf:
success, 15 pages, 22k chars, offsets exact, ~4s.
Settings: document_ocr_provider (enum-validated), document_ocr_model
("mistral/mistral-ocr-latest" -- gateway routes on the prefix, the direct mistral
backend strips it). OcrProcessor registered at lowest priority so it is never the
non-tiered default.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replaces single-engine pymupdf4llm extraction with a tiered pipeline (Deck #205,
follows the tier-0 classifier #855). pypdfium2 becomes the default and only
hot-path PDF extractor; pymupdf4llm is deprecated to a rollback toggle.
Why: pymupdf4llm's O(n^2) find_tables drove the OOM (#852) and the form-PDF
parse timeouts (#856), carries AGPL/commercial licensing liability, and -- per
the benchmarks -- recovers near-zero usable tables on the real corpus. pypdfium2
(Apache/BSD) extracts the same text far faster (Student 1a.pdf: 120s timeout ->
0.2s) with no table-detection bomb.
- document_processors/pypdfium2_fast.py: tier-1 "fast" processor emitting text +
exact page_boundaries (the pdf_highlighter contract). pymupdf processor is now
tier "structured" (the rollback engine), registered but not default.
- registry: tiered routing in ProcessorRegistry. tier-1 fast extracts, then
classification is DERIVED from that text (classifier.classify_from_text -- no
PDF re-open), records the classification metrics, and escalates scanned /
no-text-layer docs to the "ocr" tier when document_ocr_enabled (default off;
no provider yet, so fast is terminal). Wires record_document_escalation + the
real "escalated" span attribute (was hardcoded False).
- Removes the separate _shadow_classify pass from vector/processor.py -- it
re-opened every PDF and re-extracted text (~0.5-1.3s/doc of pure duplicated
CPU that lowered throughput); classification now rides the tier-1 extraction.
- Settings: document_tier1_engine ("pypdfium2" default | "pymupdf" rollback,
enum-validated), document_ocr_enabled (default false).
Tests: pypdfium2 extractor, registry tiering (fast routing, rollback, classify
recording, OCR escalation on/off), classify_from_text. Full unit suite green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>