From cf7209cd85933acb808ed5fe96976afe065c117d Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Tue, 16 Jun 2026 20:06:35 +0200 Subject: [PATCH 1/5] fix(document-processors): escalate glyph-corrupt PDFs to the structured tier The fast (pypdfium2) extractor can leak raw glyph codes on subset fonts with a broken /ToUnicode CMap. The result scores high on the existing text-quality heuristic -- a uniform glyph/Caesar offset preserves whitespace and token lengths -- yet is unsearchable. The structured (pymupdf) tier extracts the same pages correctly. Add a language-agnostic C0-control-character-ratio signal to the tier-0 classifier that detects this corruption and routes the document to a new `structured` recommended_tier. Wire the fast->structured hop on the inline path and generalise it so a low-quality-but-non-empty layer also tries structured before OCR -- the inline and external ingest modes now follow the full fast->structured->ocr ladder identically. A scanned / no-text-layer document (total_chars == 0) still shortcuts straight to OCR, since a text extractor cannot recover a pure raster. New per-tenant tunable DOCUMENT_GLYPH_CORRUPTION_RATIO (default 0.02); escalation metrics gain a `corrupt_glyphs` reason label. Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/config.py | 12 ++ .../document_processors/classifier.py | 187 ++++++++++++++---- .../document_processors/escalation.py | 11 +- .../document_processors/registry.py | 77 +++++++- nextcloud_mcp_server/observability/metrics.py | 4 +- tests/unit/test_config.py | 24 +++ tests/unit/test_doc_classifier.py | 60 ++++++ tests/unit/test_registry_tiering.py | 91 ++++++++- 8 files changed, 412 insertions(+), 54 deletions(-) diff --git a/nextcloud_mcp_server/config.py b/nextcloud_mcp_server/config.py index 0265b04a..ad9531e4 100644 --- a/nextcloud_mcp_server/config.py +++ b/nextcloud_mcp_server/config.py @@ -171,6 +171,12 @@ _DEFAULTS: dict[str, Any] = { "document_ocr_page_fraction": 0.5, "document_ocr_min_page_chars": 16, "document_ocr_detect_scanned": True, + # Tier-0 glyph-corruption trigger. When the fast (pypdfium2) extraction's + # doc-level C0-control-char ratio exceeds this, the text layer is treated as + # glyph-corrupt (a broken /ToUnicode mapping leaking raw glyph codes) and the + # doc escalates fast->structured (pymupdf re-extracts it correctly -- no OCR). + # 0 disables. Clean docs sit ~0; affected PDFs measured ~1-11% in testing. + "document_glyph_corruption_ratio": 0.02, # OCR backend request timeout (seconds). Slow scanned newspapers can take # 20-60s; raise/lower per tenant. Configurable so a tenant isn't stuck with # the 180s default when its gateway has its own shorter ceiling. @@ -384,6 +390,7 @@ _dynaconf = Dynaconf( Validator("DOCUMENT_OCR_MIN_TEXT_QUALITY", gte=0, lte=1), Validator("DOCUMENT_OCR_PAGE_FRACTION", gte=0, lte=1), Validator("DOCUMENT_OCR_MIN_PAGE_CHARS", gte=0), + Validator("DOCUMENT_GLYPH_CORRUPTION_RATIO", gte=0, lte=1), # Non-negative Validator("DOCUMENT_CHUNK_OVERLAP", gte=0), # Non-empty strings @@ -896,6 +903,10 @@ class Settings: document_ocr_page_fraction: float = 0.5 document_ocr_min_page_chars: int = 16 document_ocr_detect_scanned: bool = True + # Tier-0 glyph-corruption trigger: doc-level C0-control-char ratio above which + # the fast (pypdfium2) text layer is treated as glyph-corrupt and escalated + # fast->structured (pymupdf). 0 disables. See classifier._control_char_ratio. + document_glyph_corruption_ratio: float = 0.02 # Observability settings metrics_enabled: bool = True @@ -1532,6 +1543,7 @@ def get_settings() -> Settings: "document_ocr_page_fraction": "DOCUMENT_OCR_PAGE_FRACTION", "document_ocr_min_page_chars": "DOCUMENT_OCR_MIN_PAGE_CHARS", "document_ocr_detect_scanned": "DOCUMENT_OCR_DETECT_SCANNED", + "document_glyph_corruption_ratio": "DOCUMENT_GLYPH_CORRUPTION_RATIO", # Observability settings "metrics_enabled": "METRICS_ENABLED", "metrics_port": "METRICS_PORT", diff --git a/nextcloud_mcp_server/document_processors/classifier.py b/nextcloud_mcp_server/document_processors/classifier.py index 720f679c..54d99414 100644 --- a/nextcloud_mcp_server/document_processors/classifier.py +++ b/nextcloud_mcp_server/document_processors/classifier.py @@ -27,9 +27,10 @@ Two entry points: Recommended tier: * ``ocr`` -- scanned / no-usable-text-layer (route to tier 3, when enabled) + * ``structured`` -- a text layer that is present but glyph-corrupt (the fast + extractor leaked raw glyph codes; high C0-control-char ratio). A different + in-cluster extractor (the pymupdf ``structured`` tier) recovers it -- no OCR. * ``fast`` -- a usable digital text layer (stay on tier 1) - -``structured`` (tier 2 / docling) is a separate service, not produced here. """ import logging @@ -56,9 +57,20 @@ MIN_TEXT_QUALITY = 0.5 OCR_PAGE_FRACTION = 0.5 # A page with fewer extracted chars than this has effectively no text layer. MIN_PAGE_CHARS = 16 +# Doc-level control-character ratio above which the text layer is treated as +# glyph-corrupt and routed to the ``structured`` (pymupdf) tier, which re-extracts +# such PDFs correctly. Kept in sync with the DOCUMENT_GLYPH_CORRUPTION_RATIO +# setting default (the registry passes the per-tenant value). See +# ``_control_char_ratio``. +GLYPH_CORRUPTION_RATIO = 0.02 _WORD_RE = re.compile(r"\S+") +# Whitespace control characters that legitimately appear in extracted text +# (tab / newline / carriage-return / form-feed / vertical-tab). Every OTHER C0 +# control char is a corruption signal -- see ``_control_char_ratio``. +_TEXT_WHITESPACE_CONTROLS = frozenset("\t\n\r\f\v") + @dataclass class PageSignals: @@ -67,6 +79,7 @@ class PageSignals: image_coverage: float # 0..1 of page area covered by images text_quality: float # 0..1; low = mashed/space-less/garbage layer needs_ocr: bool # scanned or unusable text layer + control_ratio: float = 0.0 # 0..1; high = corrupt/glyph-leak text layer @dataclass @@ -76,10 +89,11 @@ class DocClassification: total_chars: int mean_text_quality: float ocr_page_fraction: float # fraction of sampled pages flagged needs_ocr - recommended_tier: str # "fast" | "ocr" + recommended_tier: str # "fast" | "structured" | "ocr" + mean_control_ratio: float = 0.0 # doc-level C0-control-char ratio (glyph-leak) flags: set[str] = field( default_factory=set - ) # scanned | bad_text_layer | image_heavy + ) # scanned | bad_text_layer | image_heavy | corrupt_glyphs pages: list[PageSignals] = field(default_factory=list) @@ -116,6 +130,25 @@ def _text_quality(text: str) -> float: return round(ws_score * len_score * overlong_score * merge_score, 3) +def _control_char_ratio(text: str) -> float: + """Fraction of C0 control characters (excluding whitespace controls) in ``text``. + + Near 0 for clean text in ANY script; elevated when the extractor leaked raw + glyph codes instead of Unicode -- the broken-/ToUnicode failure mode where a + subset font's character codes are returned uniformly offset (e.g. "WKH" for + "THE"). This is the language-agnostic counterpart to :func:`_text_quality`: a + uniform glyph/Caesar offset preserves whitespace and token lengths (so every + ``_text_quality`` factor scores it ~1.0), but it litters the text with C0 + controls -- digits/punctuation map to bytes below 0x20 -- which clean prose + never contains. Unlike a dictionary or stop-word probe it makes no assumption + about the document's language. + """ + if not text: + return 0.0 + bad = sum(1 for c in text if ord(c) < 0x20 and c not in _TEXT_WHITESPACE_CONTROLS) + return bad / len(text) + + def _sample_indices(page_count: int) -> list[int]: if page_count <= MAX_SAMPLED_PAGES: return list(range(page_count)) @@ -143,6 +176,56 @@ def _page_image_coverage(page: Any) -> float: return min(img_area / page_area, 1.0) +def _route_from_signals( + *, + total_chars: int, + ocr_frac: float, + mean_quality: float, + control_ratio: float, + image_heavy: bool, + page_fraction: float, + min_text_quality: float, + glyph_corruption_ratio: float, +) -> tuple[set[str], str]: + """Shared flag-set + recommended-tier decision for both classifier paths. + + Routing precedence (cheapest correct fix first): + 1. scanned / no text layer (``ocr_frac >= fraction`` AND ``total_chars == 0``) + -> ``"ocr"`` + 2. glyph-corrupt text layer (``control_ratio > glyph_corruption_ratio``) + -> ``"structured"``. pypdfium2 leaked glyph codes; the pymupdf + ``structured`` tier re-extracts these correctly, so no paid OCR is + needed. The registry re-classifies the structured output, so a doc that + is ALSO partly scanned can still escalate to OCR from there. + 3. junk/mashed text layer (``ocr_frac >= fraction``) -> ``"ocr"`` + 4. otherwise -> ``"fast"`` + + Flags are diagnostic and independent of the verdict (e.g. ``image_heavy`` + fires on ANY image-heavy page; the OCR route needs a page FRACTION). + """ + glyph_corrupt = total_chars > 0 and control_ratio > glyph_corruption_ratio + + flags: set[str] = set() + if ocr_frac >= page_fraction and total_chars == 0: + flags.add("scanned") + elif ocr_frac >= page_fraction and mean_quality < min_text_quality: + flags.add("bad_text_layer") + if glyph_corrupt: + flags.add("corrupt_glyphs") + if image_heavy: + flags.add("image_heavy") + + if ocr_frac >= page_fraction and total_chars == 0: + recommended = "ocr" + elif glyph_corrupt: + recommended = "structured" + elif ocr_frac >= page_fraction: + recommended = "ocr" + else: + recommended = "fast" + return flags, recommended + + def classify_pdf(content: bytes) -> DocClassification: """Classify a PDF from its bytes. @@ -171,7 +254,14 @@ def classify_pdf(content: bytes) -> DocClassification: # raises the diagnostic image_heavy flag below. needs_ocr = quality < MIN_TEXT_QUALITY or len(text.strip()) < MIN_PAGE_CHARS pages.append( - PageSignals(n, len(text), round(coverage, 3), quality, needs_ocr) + PageSignals( + n, + len(text), + round(coverage, 3), + quality, + needs_ocr, + round(_control_char_ratio(text), 4), + ) ) sampled = len(pages) @@ -180,25 +270,24 @@ def classify_pdf(content: bytes) -> DocClassification: round(sum(p.text_quality for p in pages) / sampled, 3) if sampled else 0.0 ) ocr_frac = (sum(p.needs_ocr for p in pages) / sampled) if sampled else 0.0 + # Char-weighted doc-level control-char ratio (p.control_ratio * char_count is + # the per-page bad-char count). The glyph-leak signal -- see _control_char_ratio. + control_ratio = ( + sum(p.control_ratio * p.char_count for p in pages) / total_chars + if total_chars + else 0.0 + ) - # Flags are diagnostic signals, intentionally independent of the routing - # verdict: image_heavy fires if ANY page is image-heavy, while the OCR route - # needs a FRACTION of pages (OCR_PAGE_FRACTION). So a mostly-digital doc with - # one full-page photo is flagged image_heavy yet still routes "fast" -- the - # flag_total{image_heavy} count is expected to exceed classified{ocr}. - flags: set[str] = set() - if any(p.image_coverage >= IMAGE_HEAVY_THRESHOLD for p in pages): - flags.add("image_heavy") - if ( - ocr_frac >= OCR_PAGE_FRACTION - and total_chars - and mean_quality < MIN_TEXT_QUALITY - ): - flags.add("bad_text_layer") - if ocr_frac >= OCR_PAGE_FRACTION and total_chars == 0: - flags.add("scanned") - - recommended = "ocr" if ocr_frac >= OCR_PAGE_FRACTION else "fast" + flags, recommended = _route_from_signals( + total_chars=total_chars, + ocr_frac=ocr_frac, + mean_quality=mean_quality, + control_ratio=control_ratio, + image_heavy=any(p.image_coverage >= IMAGE_HEAVY_THRESHOLD for p in pages), + page_fraction=OCR_PAGE_FRACTION, + min_text_quality=MIN_TEXT_QUALITY, + glyph_corruption_ratio=GLYPH_CORRUPTION_RATIO, + ) return DocClassification( page_count=page_count, @@ -207,6 +296,7 @@ def classify_pdf(content: bytes) -> DocClassification: mean_text_quality=mean_quality, ocr_page_fraction=round(ocr_frac, 3), recommended_tier=recommended, + mean_control_ratio=round(control_ratio, 4), flags=flags, pages=pages, ) @@ -239,6 +329,7 @@ def classify_from_text( min_text_quality: float = MIN_TEXT_QUALITY, min_page_chars: int = MIN_PAGE_CHARS, page_fraction: float = OCR_PAGE_FRACTION, + glyph_corruption_ratio: float = GLYPH_CORRUPTION_RATIO, image_coverage: list[float] | None = None, ) -> DocClassification: """Classify from text already extracted by tier-1 -- no PDF re-open by default. @@ -246,9 +337,13 @@ def classify_from_text( The hot-path classifier. A page is OCR-worthy when its text is near-empty (``< min_page_chars``) or its text-quality is junk (``< min_text_quality`` -- the word-merging signal). The doc recommends ``ocr`` once - ``ocr_frac >= page_fraction``. Thresholds are passed in by the registry from - per-tenant settings. ``image_coverage`` (when supplied) only feeds the - ``image_heavy`` diagnostic flag -- it does NOT route (see module docstring). + ``ocr_frac >= page_fraction``. A doc whose text layer is present but + glyph-corrupt (doc-level C0-control-char ratio ``> glyph_corruption_ratio``, + the broken-/ToUnicode failure mode) instead recommends ``structured`` -- the + pymupdf tier re-extracts it correctly, no OCR needed. Thresholds are passed in + by the registry from per-tenant settings. ``image_coverage`` (when supplied) + only feeds the ``image_heavy`` diagnostic flag -- it does NOT route (see + module docstring). ``page_boundaries`` are ``{page, start_offset, end_offset}`` indexing into ``full_text``; ``image_coverage[i]`` (if given) aligns with the i-th boundary. @@ -293,7 +388,14 @@ def classify_from_text( # ``cov`` still feeds the diagnostic ``image_heavy`` flag below. needs_ocr = len(seg.strip()) < min_page_chars or quality < min_text_quality pages.append( - PageSignals(b["page"], len(seg), round(cov, 3), quality, needs_ocr) + PageSignals( + b["page"], + len(seg), + round(cov, 3), + quality, + needs_ocr, + round(_control_char_ratio(seg), 4), + ) ) sampled = len(pages) @@ -305,23 +407,21 @@ def classify_from_text( # page_count guard also skips escalation; defaulting to 0.0 keeps the # recorded classification metric accurate rather than a misleading "ocr"). ocr_frac = (sum(p.needs_ocr for p in pages) / sampled) if sampled else 0.0 + # Doc-level (char-weighted) control-char ratio -- the glyph-leak signal that + # routes to the structured tier. Computed over full_text so it is robust to + # boundary edge cases. + control_ratio = _control_char_ratio(full_text) - # Flags gated on ocr_frac >= page_fraction (matching classify_pdf): a doc that - # routes "fast" must not carry a junk-layer flag just because a few isolated - # pages are bad -- otherwise the metric diverges from classify_pdf. - flags: set[str] = set() - if sampled and ocr_frac >= page_fraction: - if total_chars == 0: - # "scanned" (not "no_text_layer"): same name + meaning as classify_pdf - # so astrolabe_document_classifier_flag_total isn't split across two - # labels for the empty-text-layer case. - flags.add("scanned") - elif mean_quality < min_text_quality: - flags.add("bad_text_layer") - if any(p.image_coverage >= IMAGE_HEAVY_THRESHOLD for p in pages): - flags.add("image_heavy") - - recommended = "ocr" if ocr_frac >= page_fraction else "fast" + flags, recommended = _route_from_signals( + total_chars=total_chars, + ocr_frac=ocr_frac, + mean_quality=mean_quality, + control_ratio=control_ratio, + image_heavy=any(p.image_coverage >= IMAGE_HEAVY_THRESHOLD for p in pages), + page_fraction=page_fraction, + min_text_quality=min_text_quality, + glyph_corruption_ratio=glyph_corruption_ratio, + ) return DocClassification( page_count=len(page_boundaries), @@ -330,6 +430,7 @@ def classify_from_text( mean_text_quality=mean_quality, ocr_page_fraction=round(ocr_frac, 3), recommended_tier=recommended, + mean_control_ratio=round(control_ratio, 4), flags=flags, pages=pages, ) diff --git a/nextcloud_mcp_server/document_processors/escalation.py b/nextcloud_mcp_server/document_processors/escalation.py index 8bfddf50..d9c3126e 100644 --- a/nextcloud_mcp_server/document_processors/escalation.py +++ b/nextcloud_mcp_server/document_processors/escalation.py @@ -49,7 +49,7 @@ class EscalationDecision: kind: Literal["hop", "suppressed"] to_tier: str - reason: Literal["empty_text", "low_confidence"] + reason: Literal["empty_text", "low_confidence", "corrupt_glyphs"] def next_tier(current: str) -> str | None: @@ -80,10 +80,11 @@ class EscalateError(Exception): the junk text is never indexed, and it must never be swallowed by a broad ``except Exception`` on the indexing path. - ``reason`` uses the existing escalation label vocabulary. This PR raises - ``empty_text`` (scanned / no text layer) and ``low_confidence`` (junk text - layer); ``unsupported`` and ``forced`` are reserved for future callers and - not raised yet. + ``reason`` uses the existing escalation label vocabulary: ``empty_text`` + (scanned / no text layer), ``low_confidence`` (junk text layer), and + ``corrupt_glyphs`` (a usable-looking layer whose extractor leaked raw glyph + codes -- the broken-/ToUnicode case -- recovered by a different in-cluster + extractor); ``unsupported`` and ``forced`` are reserved for future callers. """ def __init__(self, *, from_tier: str, to_tier: str, reason: str) -> None: diff --git a/nextcloud_mcp_server/document_processors/registry.py b/nextcloud_mcp_server/document_processors/registry.py index 33174409..c39fa619 100644 --- a/nextcloud_mcp_server/document_processors/registry.py +++ b/nextcloud_mcp_server/document_processors/registry.py @@ -247,6 +247,63 @@ class ProcessorRegistry: result, content, settings, record=True, filename=filename ) + # Escalate a poor fast extraction up the ladder (fast -> structured -> ocr), + # mirroring the external per-tier path so both modes behave identically. A + # glyph-corrupt layer (the extractor leaked raw glyph codes -- the + # broken-/ToUnicode case) OR a low-quality-but-non-empty layer first tries + # the structured (pymupdf) tier: free, in-cluster, and able to recover both. + # Only a scanned / no-text-layer doc (total_chars == 0) skips structured -- + # a text extractor cannot conjure text from a pure raster -- and drops + # straight to OCR via the gate below. Structured is therefore NOT gated on + # document_ocr_enabled. Its output is re-classified (record=False -- the doc + # was already counted at the fast tier) so a doc that is ALSO partly scanned + # still reaches the OCR gate. + if ( + classification is not None + and classification.page_count > 0 + and ( + classification.recommended_tier == "structured" + or ( + classification.recommended_tier == "ocr" + and classification.total_chars > 0 + ) + ) + ): + structured = self._pdf_processor_for_tier("structured") + if structured is not None: + reason = ( + "corrupt_glyphs" + if classification.recommended_tier == "structured" + else "low_confidence" + ) + record_document_escalation("fast", "structured", reason) + logger.info( + "Escalating %s fast->structured (reason=%s)", + filename or "", + reason, + ) + structured_result = await self._run_processor( + structured, + content, + content_type, + filename, + options, + progress_callback, + escalated=True, + ) + if structured_result.success: + result = structured_result + classification = self._classify_result( + result, content, settings, record=False, filename=filename + ) + else: + logger.warning( + "structured escalation did not succeed for %s (%s); keeping " + "the tier-1 result", + filename or "", + structured_result.metadata.get("parse_failed_reason", "error"), + ) + # NOTE: the suppressed-escalation metric (document_escalation_suppressed_total, # the "what-if OCR" signal; Deck #324) is intentionally NOT emitted on this # inline/memory path -- it is instrumented only on the per-tier external @@ -379,6 +436,7 @@ class ProcessorRegistry: min_text_quality=settings.document_ocr_min_text_quality, min_page_chars=settings.document_ocr_min_page_chars, page_fraction=settings.document_ocr_page_fraction, + glyph_corruption_ratio=settings.document_glyph_corruption_ratio, image_coverage=image_coverage, ) except Exception: @@ -520,6 +578,9 @@ class ProcessorRegistry: - ``total_chars == 0`` (scanned / no text layer) -> target the ``ocr`` tier directly. Text-extractor tiers (``structured``) cannot conjure text from a pure raster scan, so a structured hop would just be wasted. + - glyph-corrupt text layer (``recommended_tier == "structured"``) -> target + the ``structured`` tier; pymupdf re-extracts a broken-/ToUnicode layer + correctly, so OCR is never the target for this case. - low-confidence but non-empty layer -> escalate to the next rung, so a different in-cluster extractor can try before paying for OCR. @@ -538,13 +599,23 @@ class ProcessorRegistry: record=(current_tier == TIER_LADDER[0]), filename=filename, ) - if classification is None or classification.recommended_tier != "ocr": + if classification is None or classification.recommended_tier not in ( + "structured", + "ocr", + ): return None # A zero-page (empty/corrupt) PDF gains nothing from any tier. if classification.page_count <= 0: return None - if classification.total_chars == 0: - minimum: str | None = "ocr" + minimum: str | None + if classification.recommended_tier == "structured": + # Glyph-corrupt text layer (the extractor leaked glyph codes): a + # different in-cluster extractor (the structured/pymupdf tier) recovers + # it -- never pay for OCR here. Target the structured rung specifically. + minimum = "structured" + reason = "corrupt_glyphs" + elif classification.total_chars == 0: + minimum = "ocr" reason = "empty_text" else: minimum = None diff --git a/nextcloud_mcp_server/observability/metrics.py b/nextcloud_mcp_server/observability/metrics.py index 92e87871..c1e64a12 100644 --- a/nextcloud_mcp_server/observability/metrics.py +++ b/nextcloud_mcp_server/observability/metrics.py @@ -272,7 +272,7 @@ document_bytes_processed_total = Counter( document_escalation_total = Counter( "astrolabe_document_escalation_total", "Total document parse escalations between tiers", - # reason: low_confidence | empty_text | unsupported | error | forced + # reason: low_confidence | empty_text | corrupt_glyphs | unsupported | error | forced ["from_tier", "to_tier", "reason"], ) @@ -754,7 +754,7 @@ def record_document_escalation(from_tier: str, to_tier: str, reason: str) -> Non Args: from_tier: Tier that could not satisfactorily parse the document to_tier: Tier the document was escalated to - reason: low_confidence | empty_text | unsupported | error | forced + reason: low_confidence | empty_text | corrupt_glyphs | unsupported | error | forced """ document_escalation_total.labels( from_tier=from_tier, to_tier=to_tier, reason=reason diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 4a15d77c..dc23bf8e 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -266,6 +266,30 @@ class TestChunkConfigValidation: _reload_config() assert get_settings().document_max_pdf_size_mb == pytest.approx(12.5) + def test_glyph_corruption_ratio_default_and_env_override(self): + """document_glyph_corruption_ratio defaults to 0.02 and reads its env var. + + Guards the _DEFAULTS-key-must-match-env-var footgun. + """ + assert Settings().document_glyph_corruption_ratio == pytest.approx(0.02) + with patch.dict( + os.environ, {"DOCUMENT_GLYPH_CORRUPTION_RATIO": "0.05"}, clear=True + ): + _reload_config() + assert get_settings().document_glyph_corruption_ratio == pytest.approx(0.05) + + @patch.dict( + os.environ, + {"DOCUMENT_GLYPH_CORRUPTION_RATIO": "1.5"}, + clear=True, + ) + def test_glyph_corruption_ratio_out_of_range_raises_error(self): + """The ratio must be within [0, 1].""" + from dynaconf import ValidationError + + with pytest.raises(ValidationError, match="DOCUMENT_GLYPH_CORRUPTION_RATIO"): + _reload_config() + def test_valid_chunk_settings(self): """Test valid chunk size and overlap configuration.""" settings = Settings( diff --git a/tests/unit/test_doc_classifier.py b/tests/unit/test_doc_classifier.py index b80f7b52..84429c3c 100644 --- a/tests/unit/test_doc_classifier.py +++ b/tests/unit/test_doc_classifier.py @@ -320,3 +320,63 @@ def test_scan_coverage_shorter_than_pages_aligns_without_crash(): assert all(p.needs_ocr is False for p in c.pages) # coverage no longer routes assert "image_heavy" in c.flags # but page 0 still flags image_heavy assert c.recommended_tier == "fast" + + +# --- glyph-corruption signal (broken /ToUnicode -> structured escalation) ----- + +# pypdfium2-style leak: a uniform glyph/Caesar offset turns clean prose into +# alphabetic-but-wrong tokens (normal spacing + token length => HIGH text_quality) +# while digits/punctuation map to C0 control bytes. The control-char ratio is the +# only signal that catches this; _text_quality scores it ~1.0. +_GLYPH_CORRUPT = "WKH \x0f TXLFN \x10 EURZQ \x11 IRA MXPSV \x0f RYHU \x10 GRJ " * 6 + + +def test_control_char_ratio_clean_is_zero(): + assert clf._control_char_ratio("the quick brown fox") == 0.0 + # legitimate whitespace controls (tab/newline/CR/form-feed/vtab) don't count + assert clf._control_char_ratio("a\tb\nc\r\nd\f\ve") == 0.0 + + +def test_control_char_ratio_detects_glyph_leak(): + assert clf._control_char_ratio(_GLYPH_CORRUPT) > clf.GLYPH_CORRUPTION_RATIO + + +def test_clean_text_not_flagged_corrupt(): + txt = "the quick brown fox jumps over the lazy dog " * 3 + c = clf.classify_from_text( + txt, [{"page": 1, "start_offset": 0, "end_offset": len(txt)}] + ) + assert "corrupt_glyphs" not in c.flags + assert c.mean_control_ratio == pytest.approx(0.0) + assert c.recommended_tier == "fast" + + +def test_glyph_corrupt_routes_structured_not_ocr(): + full = _GLYPH_CORRUPT + c = clf.classify_from_text( + full, [{"page": 1, "start_offset": 0, "end_offset": len(full)}] + ) + assert c.recommended_tier == "structured" + assert "corrupt_glyphs" in c.flags + # The point: it is NOT a low-quality signal -- the cipher scores high, so only + # the control-char ratio diverts it (to structured, the free pymupdf re-parse). + assert c.mean_text_quality >= clf.MIN_TEXT_QUALITY + assert c.mean_control_ratio > clf.GLYPH_CORRUPTION_RATIO + + +def test_glyph_corruption_ratio_override_disables_trigger(): + full = _GLYPH_CORRUPT + bounds = [{"page": 1, "start_offset": 0, "end_offset": len(full)}] + # A threshold of 1.0 can never be exceeded => not treated as corrupt => the + # other (high-quality) signals win => fast. + c = clf.classify_from_text(full, bounds, glyph_corruption_ratio=1.0) + assert c.recommended_tier == "fast" + assert "corrupt_glyphs" not in c.flags + + +def test_empty_doc_routes_ocr_not_structured(): + # Precedence: a scanned/empty doc (no text layer) has no control chars to leak, + # so it must stay an OCR case, never structured. + c = clf.classify_from_text("", [{"page": 1, "start_offset": 0, "end_offset": 0}]) + assert c.recommended_tier == "ocr" + assert "corrupt_glyphs" not in c.flags diff --git a/tests/unit/test_registry_tiering.py b/tests/unit/test_registry_tiering.py index 65d08768..c654912d 100644 --- a/tests/unit/test_registry_tiering.py +++ b/tests/unit/test_registry_tiering.py @@ -24,7 +24,9 @@ class _Fake(DocumentProcessor): self, name: str, tier: str, - text: str = "clean text here", + # >= MIN_PAGE_CHARS of clean, whitespace-separated prose so the default + # classifies "fast" (a shorter string trips the near-empty OCR signal). + text: str = "this is clean readable prose text", success=True, pages: int = 1, ): @@ -75,6 +77,7 @@ class _Settings: page_fraction=0.5, min_page_chars=16, detect_scanned=False, + glyph_corruption_ratio=0.02, # Guard off by default so existing tiering tests are unaffected; tests # that exercise the size guard pass an explicit cap. max_pdf_size_mb=0.0, @@ -86,6 +89,7 @@ class _Settings: self.document_ocr_page_fraction = page_fraction self.document_ocr_min_page_chars = min_page_chars self.document_ocr_detect_scanned = detect_scanned + self.document_glyph_corruption_ratio = glyph_corruption_ratio self.document_max_pdf_size_mb = max_pdf_size_mb @@ -244,6 +248,91 @@ async def test_no_ocr_escalation_when_disabled(monkeypatch): assert res.processor == "fast" +# --- glyph-corruption escalation + full-ladder parity ------------------------ + +# A fast-tier text layer that looks like words (normal spacing/token lengths -> +# HIGH text_quality) but leaks C0 control chars: the broken-/ToUnicode signature +# the control-char ratio catches. Decodes to a pangram under a -3 shift. +_GLYPH = "WKH \x0f TXLFN \x10 EURZQ \x11 IRA MXPSV \x0f RYHU \x10 GRJ " * 6 + + +async def test_glyph_corrupt_escalates_fast_to_structured(monkeypatch): + # Not gated on OCR: structured is free + in-cluster, so a glyph-corrupt layer + # escalates fast->structured even with OCR disabled. + monkeypatch.setattr(reg_mod, "get_settings", lambda: _Settings(ocr=False)) + esc = MagicMock() + monkeypatch.setattr(reg_mod, "record_document_escalation", esc) + r = _registry( + (_Fake("fast", "fast", text=_GLYPH), 20), + (_Fake("structured", "structured", text="clean recovered prose text"), 10), + ) + res = await r.process(b"%PDF-1.7", "application/pdf") + assert res.processor == "structured" + esc.assert_called_once_with("fast", "structured", "corrupt_glyphs") + + +async def test_glyph_corrupt_no_structured_stays_fast(monkeypatch): + # No structured processor registered -> nothing to escalate to; keep fast. + monkeypatch.setattr(reg_mod, "get_settings", lambda: _Settings(ocr=False)) + r = _registry((_Fake("fast", "fast", text=_GLYPH), 20)) + res = await r.process(b"%PDF-1.7", "application/pdf") + assert res.processor == "fast" + + +async def test_inline_lowconf_tries_structured_before_ocr(monkeypatch): + # Full-ladder parity with the external path: a junk-but-non-empty fast layer + # tries structured (fast->structured) BEFORE any OCR, even with OCR enabled. + monkeypatch.setattr(reg_mod, "get_settings", lambda: _Settings(ocr=True)) + esc = MagicMock() + monkeypatch.setattr(reg_mod, "record_document_escalation", esc) + r = _registry( + (_Fake("fast", "fast", text="x" * 40), 20), # one long token -> quality ~0 + (_Fake("structured", "structured", text="clean recovered prose text here"), 10), + (_Fake("ocr", "ocr", text="ocr text"), 5), + ) + res = await r.process(b"%PDF-1.7", "application/pdf") + assert res.processor == "structured" + esc.assert_called_once_with("fast", "structured", "low_confidence") + + +async def test_inline_empty_skips_structured_straight_to_ocr(monkeypatch): + # The one intended shortcut: a scanned/no-text-layer doc (total_chars == 0) + # skips structured (it cannot extract text from a raster) and goes to OCR. + monkeypatch.setattr(reg_mod, "get_settings", lambda: _Settings(ocr=True)) + esc = MagicMock() + monkeypatch.setattr(reg_mod, "record_document_escalation", esc) + r = _registry( + (_Fake("fast", "fast", text=""), 20), + (_Fake("structured", "structured", text="should not run"), 10), + (_Fake("ocr", "ocr", text="ocr text"), 5), + ) + res = await r.process(b"%PDF-1.7", "application/pdf") + assert res.processor == "ocr" + esc.assert_called_once_with("fast", "ocr", "empty_text") + + +def test_evaluate_escalation_glyph_corrupt_goes_structured(monkeypatch): + # External path mirrors the inline path: glyph-corrupt -> structured, never OCR. + monkeypatch.setattr(reg_mod, "record_document_classification", MagicMock()) + r = _registry( + (_Fake("fast", "fast"), 20), + (_Fake("structured", "structured"), 10), + (_Fake("ocr", "ocr"), 5), + ) + res = ProcessingResult( + text=_GLYPH, + metadata={ + "page_count": 1, + "page_boundaries": [ + {"page": 1, "start_offset": 0, "end_offset": len(_GLYPH)} + ], + }, + processor="fast", + ) + decision = r.evaluate_escalation(res, b"%PDF", "fast", _Settings(ocr=True)) + assert decision == EscalationDecision("hop", "structured", "corrupt_glyphs") + + # --- Per-tier external path (Deck #323) ------------------------------------- From d5286e39d65a1f493c87e26483335484c6c638f8 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Tue, 16 Jun 2026 20:17:56 +0200 Subject: [PATCH 2/5] fix(document-processors): correct cascade escalation metric + review nits Address round-1 review on PR #914: - Attribute the OCR hop in a fast->structured->ocr inline cascade to from_tier="structured" (not a second "fast" escalation), so astrolabe_document_escalation_total per-tier counts stay accurate. - Add test_inline_fast_structured_ocr_cascade pinning that two-hop path and the metric attribution. - Note in classify_from_text that its doc-level control ratio is over full_text (all pages), not the sampled subset classify_pdf uses. - Clarify that corrupt_glyphs never lands in the suppressed-escalation counter. - Dedupe the glyph-corrupt test string into tests/fixtures/glyph_corruption.py. - Use pytest.approx for the control-char-ratio zero checks (SonarCloud S1244). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../document_processors/classifier.py | 9 ++++-- .../document_processors/registry.py | 12 +++++-- nextcloud_mcp_server/observability/metrics.py | 4 ++- tests/fixtures/glyph_corruption.py | 11 +++++++ tests/unit/test_doc_classifier.py | 15 +++++---- tests/unit/test_registry_tiering.py | 32 ++++++++++++++++--- 6 files changed, 65 insertions(+), 18 deletions(-) create mode 100644 tests/fixtures/glyph_corruption.py diff --git a/nextcloud_mcp_server/document_processors/classifier.py b/nextcloud_mcp_server/document_processors/classifier.py index 54d99414..7ce28fd4 100644 --- a/nextcloud_mcp_server/document_processors/classifier.py +++ b/nextcloud_mcp_server/document_processors/classifier.py @@ -407,9 +407,12 @@ def classify_from_text( # page_count guard also skips escalation; defaulting to 0.0 keeps the # recorded classification metric accurate rather than a misleading "ocr"). ocr_frac = (sum(p.needs_ocr for p in pages) / sampled) if sampled else 0.0 - # Doc-level (char-weighted) control-char ratio -- the glyph-leak signal that - # routes to the structured tier. Computed over full_text so it is robust to - # boundary edge cases. + # Doc-level control-char ratio -- the glyph-leak signal that routes to the + # structured tier. Computed over the WHOLE full_text (all pages), unlike + # classify_pdf which char-weights the up-to-MAX_SAMPLED_PAGES sample; the two + # are therefore not numerically identical for a >24-page doc with corruption + # concentrated outside the sample. full_text is used here because it is exactly + # the text that gets chunked + indexed and is robust to boundary edge cases. control_ratio = _control_char_ratio(full_text) flags, recommended = _route_from_signals( diff --git a/nextcloud_mcp_server/document_processors/registry.py b/nextcloud_mcp_server/document_processors/registry.py index c39fa619..40b9ae1e 100644 --- a/nextcloud_mcp_server/document_processors/registry.py +++ b/nextcloud_mcp_server/document_processors/registry.py @@ -247,6 +247,12 @@ class ProcessorRegistry: result, content, settings, record=True, filename=filename ) + # The tier whose output produced the current ``classification`` -- used as + # ``from_tier`` for a subsequent OCR hop so a fast->structured->ocr cascade + # is attributed correctly (the OCR hop is from ``structured``, not a second + # ``fast`` escalation). + from_tier = "fast" + # Escalate a poor fast extraction up the ladder (fast -> structured -> ocr), # mirroring the external per-tier path so both modes behave identically. A # glyph-corrupt layer (the extractor leaked raw glyph codes -- the @@ -293,6 +299,7 @@ class ProcessorRegistry: ) if structured_result.success: result = structured_result + from_tier = "structured" classification = self._classify_result( result, content, settings, record=False, filename=filename ) @@ -331,10 +338,11 @@ class ProcessorRegistry: if classification.total_chars == 0 else "low_confidence" ) - record_document_escalation("fast", "ocr", reason) + record_document_escalation(from_tier, "ocr", reason) logger.info( - "Escalating %s fast->ocr (reason=%s)", + "Escalating %s %s->ocr (reason=%s)", filename or "", + from_tier, reason, ) ocr_result = await self._run_processor( diff --git a/nextcloud_mcp_server/observability/metrics.py b/nextcloud_mcp_server/observability/metrics.py index c1e64a12..f2ca1060 100644 --- a/nextcloud_mcp_server/observability/metrics.py +++ b/nextcloud_mcp_server/observability/metrics.py @@ -287,7 +287,9 @@ document_escalation_total = Counter( document_escalation_suppressed_total = Counter( "astrolabe_document_escalation_suppressed_total", "Would-be tier escalations suppressed because the target tier is disabled", - # reason: low_confidence | empty_text + # reason: low_confidence | empty_text. (corrupt_glyphs never appears here: it + # targets the structured tier, which has no enabled-gate -- if registered it + # runs, else there is nothing to suppress -- so it only ever hops or returns.) ["from_tier", "to_tier", "reason"], ) diff --git a/tests/fixtures/glyph_corruption.py b/tests/fixtures/glyph_corruption.py new file mode 100644 index 00000000..08e8cb9e --- /dev/null +++ b/tests/fixtures/glyph_corruption.py @@ -0,0 +1,11 @@ +"""Shared test data for the tier-0 glyph-corruption signal. + +A fast-tier text layer that looks like words -- normal spacing and token lengths, +so it scores HIGH on ``_text_quality`` -- but leaks C0 control characters: the +broken-/ToUnicode signature that ``classifier._control_char_ratio`` catches. The +alphabetic tokens decode to a pangram under a -3 (Caesar) shift. + +Kept in one place so the classifier and registry tiering tests can't diverge. +""" + +GLYPH_CORRUPT_TEXT = "WKH \x0f TXLFN \x10 EURZQ \x11 IRA MXPSV \x0f RYHU \x10 GRJ " * 6 diff --git a/tests/unit/test_doc_classifier.py b/tests/unit/test_doc_classifier.py index 84429c3c..2a7d9ee3 100644 --- a/tests/unit/test_doc_classifier.py +++ b/tests/unit/test_doc_classifier.py @@ -14,6 +14,7 @@ import pymupdf import pytest from nextcloud_mcp_server.document_processors import classifier as clf +from tests.fixtures.glyph_corruption import GLYPH_CORRUPT_TEXT pytestmark = pytest.mark.unit @@ -324,17 +325,17 @@ def test_scan_coverage_shorter_than_pages_aligns_without_crash(): # --- glyph-corruption signal (broken /ToUnicode -> structured escalation) ----- -# pypdfium2-style leak: a uniform glyph/Caesar offset turns clean prose into -# alphabetic-but-wrong tokens (normal spacing + token length => HIGH text_quality) -# while digits/punctuation map to C0 control bytes. The control-char ratio is the -# only signal that catches this; _text_quality scores it ~1.0. -_GLYPH_CORRUPT = "WKH \x0f TXLFN \x10 EURZQ \x11 IRA MXPSV \x0f RYHU \x10 GRJ " * 6 +# A uniform glyph/Caesar offset turns clean prose into alphabetic-but-wrong tokens +# (normal spacing + token length => HIGH text_quality) while digits/punctuation map +# to C0 control bytes. The control-char ratio is the only signal that catches this; +# _text_quality scores it ~1.0. Shared with the registry tiering tests. +_GLYPH_CORRUPT = GLYPH_CORRUPT_TEXT def test_control_char_ratio_clean_is_zero(): - assert clf._control_char_ratio("the quick brown fox") == 0.0 + assert clf._control_char_ratio("the quick brown fox") == pytest.approx(0.0) # legitimate whitespace controls (tab/newline/CR/form-feed/vtab) don't count - assert clf._control_char_ratio("a\tb\nc\r\nd\f\ve") == 0.0 + assert clf._control_char_ratio("a\tb\nc\r\nd\f\ve") == pytest.approx(0.0) def test_control_char_ratio_detects_glyph_leak(): diff --git a/tests/unit/test_registry_tiering.py b/tests/unit/test_registry_tiering.py index c654912d..5263f7c3 100644 --- a/tests/unit/test_registry_tiering.py +++ b/tests/unit/test_registry_tiering.py @@ -4,7 +4,7 @@ Covers: default fast-tier routing, the pymupdf rollback toggle, classification recording derived from the extraction, and OCR escalation (on/off). """ -from unittest.mock import MagicMock +from unittest.mock import MagicMock, call import pytest @@ -15,6 +15,7 @@ from nextcloud_mcp_server.document_processors.base import ( ) from nextcloud_mcp_server.document_processors.escalation import EscalationDecision from nextcloud_mcp_server.document_processors.registry import ProcessorRegistry +from tests.fixtures.glyph_corruption import GLYPH_CORRUPT_TEXT pytestmark = pytest.mark.unit @@ -250,10 +251,10 @@ async def test_no_ocr_escalation_when_disabled(monkeypatch): # --- glyph-corruption escalation + full-ladder parity ------------------------ -# A fast-tier text layer that looks like words (normal spacing/token lengths -> -# HIGH text_quality) but leaks C0 control chars: the broken-/ToUnicode signature -# the control-char ratio catches. Decodes to a pangram under a -3 shift. -_GLYPH = "WKH \x0f TXLFN \x10 EURZQ \x11 IRA MXPSV \x0f RYHU \x10 GRJ " * 6 +# A fast-tier text layer that looks like words (HIGH text_quality) but leaks C0 +# control chars -- the broken-/ToUnicode signature the control-char ratio catches. +# Shared with the classifier tests so the two can't diverge. +_GLYPH = GLYPH_CORRUPT_TEXT async def test_glyph_corrupt_escalates_fast_to_structured(monkeypatch): @@ -311,6 +312,27 @@ async def test_inline_empty_skips_structured_straight_to_ocr(monkeypatch): esc.assert_called_once_with("fast", "ocr", "empty_text") +async def test_inline_fast_structured_ocr_cascade(monkeypatch): + # Full cascade: a junk-but-non-empty fast layer hops to structured, the + # structured re-extract is empty (a doc that was ALSO scanned), so it then + # hops to OCR. The second hop must be attributed from_tier="structured", + # NOT a second "fast" escalation. + monkeypatch.setattr(reg_mod, "get_settings", lambda: _Settings(ocr=True)) + esc = MagicMock() + monkeypatch.setattr(reg_mod, "record_document_escalation", esc) + r = _registry( + (_Fake("fast", "fast", text="x" * 40), 20), # quality ~0, non-empty + (_Fake("structured", "structured", text=""), 10), # re-extract empty + (_Fake("ocr", "ocr", text="ocr recovered text"), 5), + ) + res = await r.process(b"%PDF-1.7", "application/pdf") + assert res.processor == "ocr" + assert esc.call_args_list == [ + call("fast", "structured", "low_confidence"), + call("structured", "ocr", "empty_text"), + ] + + def test_evaluate_escalation_glyph_corrupt_goes_structured(monkeypatch): # External path mirrors the inline path: glyph-corrupt -> structured, never OCR. monkeypatch.setattr(reg_mod, "record_document_classification", MagicMock()) From 425eb839bfce5a765f413170cc47f9ba749964ed Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Tue, 16 Jun 2026 20:24:33 +0200 Subject: [PATCH 3/5] docs(document-processors): round-2 review nits + classify_pdf glyph test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address round-2 review on PR #914: - Add corrupt_glyphs to the document_classifier_flag_total label comment (it is a live flag value emitted by record_document_classification). - Mirror the full_text-vs-sampled control-ratio NOTE into classify_pdf so the diagnostic path's under-detection trade-off is documented in place. - Add test_classify_pdf_glyph_corrupt_routes_structured for routing symmetry on the standalone classify_pdf path. (SonarCloud quality gate is green — the prior S1244 finding was fixed last round.) Co-Authored-By: Claude Opus 4.8 (1M context) --- .../document_processors/classifier.py | 4 ++++ nextcloud_mcp_server/observability/metrics.py | 2 +- tests/unit/test_doc_classifier.py | 22 +++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/nextcloud_mcp_server/document_processors/classifier.py b/nextcloud_mcp_server/document_processors/classifier.py index 7ce28fd4..7647769f 100644 --- a/nextcloud_mcp_server/document_processors/classifier.py +++ b/nextcloud_mcp_server/document_processors/classifier.py @@ -272,6 +272,10 @@ def classify_pdf(content: bytes) -> DocClassification: ocr_frac = (sum(p.needs_ocr for p in pages) / sampled) if sampled else 0.0 # Char-weighted doc-level control-char ratio (p.control_ratio * char_count is # the per-page bad-char count). The glyph-leak signal -- see _control_char_ratio. + # NOTE: this is over the <=MAX_SAMPLED_PAGES sample, so unlike classify_from_text + # (which scans the whole full_text) this diagnostic path can under-detect + # corruption concentrated outside the sampled pages. Acceptable here: the hot + # path is classify_from_text; this standalone pass is for diagnostics. control_ratio = ( sum(p.control_ratio * p.char_count for p in pages) / total_chars if total_chars diff --git a/nextcloud_mcp_server/observability/metrics.py b/nextcloud_mcp_server/observability/metrics.py index f2ca1060..26e20421 100644 --- a/nextcloud_mcp_server/observability/metrics.py +++ b/nextcloud_mcp_server/observability/metrics.py @@ -334,7 +334,7 @@ document_classifier_flag_total = Counter( # so flag{image_heavy} is expected to exceed classified{recommended_tier=ocr}. "astrolabe_document_classifier_flag_total", "Tier-0 classifier flags raised on documents", - ["flag"], # image_heavy | scanned | bad_text_layer + ["flag"], # image_heavy | scanned | bad_text_layer | corrupt_glyphs ) document_text_quality = Histogram( diff --git a/tests/unit/test_doc_classifier.py b/tests/unit/test_doc_classifier.py index 2a7d9ee3..e9dc9423 100644 --- a/tests/unit/test_doc_classifier.py +++ b/tests/unit/test_doc_classifier.py @@ -31,6 +31,18 @@ def _digital_pdf( return data +def _glyph_corrupt_pdf(pages: int = 2) -> bytes: + # A born-digital PDF whose text layer carries the glyph-leak control chars, + # for the classify_pdf (diagnostic) path. pymupdf round-trips the C0 controls. + doc = pymupdf.open() + for _ in range(pages): + page = doc.new_page(width=595, height=842) + page.insert_text((50, 60), GLYPH_CORRUPT_TEXT) + data: bytes = doc.tobytes() + doc.close() + return data + + def _full_page_image_pdf(pages: int = 2) -> bytes: # A page whose entire area is a raster image -> looks scanned. doc = pymupdf.open() @@ -381,3 +393,13 @@ def test_empty_doc_routes_ocr_not_structured(): c = clf.classify_from_text("", [{"page": 1, "start_offset": 0, "end_offset": 0}]) assert c.recommended_tier == "ocr" assert "corrupt_glyphs" not in c.flags + + +def test_classify_pdf_glyph_corrupt_routes_structured(): + # Symmetry with the classify_from_text routing on the standalone/diagnostic + # classify_pdf path (which re-opens the PDF and samples pages). + c = clf.classify_pdf(_glyph_corrupt_pdf()) + assert c.recommended_tier == "structured" + assert "corrupt_glyphs" in c.flags + assert c.mean_control_ratio > clf.GLYPH_CORRUPTION_RATIO + assert c.mean_text_quality >= clf.MIN_TEXT_QUALITY # control signal, not quality From 33aadbcf801de2c89fe35671e4d42eb450937102 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Tue, 16 Jun 2026 20:34:07 +0200 Subject: [PATCH 4/5] fix(document-processors): inline/external parity when structured tier is absent Address round-3 review on PR #914: - When a glyph-corrupt doc's structured rung is NOT registered, the inline path now falls through to OCR (with reason corrupt_glyphs), mirroring the external next_available_tier instead of silently keeping the fast result. A structured parse FAILURE remains terminal (tracked via structured_failed), matching the external path which does not escalate a failure. Added a debug log for the unregistered case and "(OCR not attempted)" to the failure warning. - Tests: inline + external glyph-corrupt fallthrough to OCR when structured is unregistered; glyph-corrupt + junk-quality both-flags precedence (structured wins over the bad_text_layer/ocr route). - Note the total_chars>0 mutual-exclusion with the scanned branch in _route_from_signals. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../document_processors/classifier.py | 2 + .../document_processors/registry.py | 44 ++++++++++++++----- tests/unit/test_doc_classifier.py | 15 +++++++ tests/unit/test_registry_tiering.py | 42 +++++++++++++++++- 4 files changed, 91 insertions(+), 12 deletions(-) diff --git a/nextcloud_mcp_server/document_processors/classifier.py b/nextcloud_mcp_server/document_processors/classifier.py index 7647769f..26dcb548 100644 --- a/nextcloud_mcp_server/document_processors/classifier.py +++ b/nextcloud_mcp_server/document_processors/classifier.py @@ -203,6 +203,8 @@ def _route_from_signals( Flags are diagnostic and independent of the verdict (e.g. ``image_heavy`` fires on ANY image-heavy page; the OCR route needs a page FRACTION). """ + # total_chars > 0 also guarantees this never overlaps the scanned branch + # (total_chars == 0), so a doc is never both glyph-corrupt and "scanned". glyph_corrupt = total_chars > 0 and control_ratio > glyph_corruption_ratio flags: set[str] = set() diff --git a/nextcloud_mcp_server/document_processors/registry.py b/nextcloud_mcp_server/document_processors/registry.py index 40b9ae1e..2da9521e 100644 --- a/nextcloud_mcp_server/document_processors/registry.py +++ b/nextcloud_mcp_server/document_processors/registry.py @@ -252,6 +252,10 @@ class ProcessorRegistry: # is attributed correctly (the OCR hop is from ``structured``, not a second # ``fast`` escalation). from_tier = "fast" + # Set when the structured tier ran but failed to parse. The document is then + # terminal -- the external path does not escalate a parse FAILURE either -- + # so the OCR gate below must not treat it as a fallback. + structured_failed = False # Escalate a poor fast extraction up the ladder (fast -> structured -> ocr), # mirroring the external per-tier path so both modes behave identically. A @@ -276,7 +280,18 @@ class ProcessorRegistry: ) ): structured = self._pdf_processor_for_tier("structured") - if structured is not None: + if structured is None: + # Structured isn't registered: mirror the external + # next_available_tier, which skips the missing rung and lands on + # OCR. Leave the recommendation unchanged so the OCR gate below + # picks it up (incl. a glyph-corrupt "structured" recommendation). + logger.debug( + "No structured processor registered; %s falls through to the " + "OCR gate (recommended_tier=%s)", + filename or "", + classification.recommended_tier, + ) + else: reason = ( "corrupt_glyphs" if classification.recommended_tier == "structured" @@ -304,9 +319,10 @@ class ProcessorRegistry: result, content, settings, record=False, filename=filename ) else: + structured_failed = True logger.warning( "structured escalation did not succeed for %s (%s); keeping " - "the tier-1 result", + "the tier-1 result (OCR not attempted)", filename or "", structured_result.metadata.get("parse_failed_reason", "error"), ) @@ -318,23 +334,29 @@ class ProcessorRegistry: # is off here the would-be escalation is simply not taken (the gate below); # operators reading the suppressed counter are on the procrastinate fleet. # - # Escalate scanned / no-text-layer PDFs to OCR (tier-3) when enabled and - # a provider is registered. The fast tier is terminal otherwise. Note: a - # fast FAILURE (encrypted/corrupt -- result.success False, no - # classification) is NOT escalated; a PDF pypdfium2 can't open is treated - # as a hard failure (OCR reads the same bytes and would usually fail - # too). The page_count guard skips a zero-page (empty/corrupt) PDF, which - # OCR can't help either. + # Escalate to OCR (tier-3) when enabled and a provider is registered. Fires + # for a scanned / no-text-layer doc (recommended "ocr"), and also for an + # unresolved "structured" recommendation -- a glyph-corrupt doc whose + # structured rung wasn't registered -- so the inline path falls through to + # OCR exactly like the external next_available_tier. ``structured_failed`` + # excludes a doc whose structured parse FAILED (terminal, like the external + # path). Note: a fast FAILURE (result.success False, no classification) is + # NOT escalated; a PDF pypdfium2 can't open is a hard failure (OCR reads the + # same bytes and would usually fail too). The page_count guard skips a + # zero-page (empty/corrupt) PDF, which OCR can't help either. if ( classification is not None - and classification.recommended_tier == "ocr" + and not structured_failed + and classification.recommended_tier in ("ocr", "structured") and classification.page_count > 0 and settings.document_ocr_enabled ): ocr = self._pdf_processor_for_tier("ocr") if ocr is not None: reason = ( - "empty_text" + "corrupt_glyphs" + if classification.recommended_tier == "structured" + else "empty_text" if classification.total_chars == 0 else "low_confidence" ) diff --git a/tests/unit/test_doc_classifier.py b/tests/unit/test_doc_classifier.py index e9dc9423..5d843f6f 100644 --- a/tests/unit/test_doc_classifier.py +++ b/tests/unit/test_doc_classifier.py @@ -395,6 +395,21 @@ def test_empty_doc_routes_ocr_not_structured(): assert "corrupt_glyphs" not in c.flags +def test_glyph_corrupt_takes_precedence_over_junk_text_layer(): + # A layer that is BOTH glyph-corrupt (high control ratio) AND junk-quality + # (mashed, no whitespace -> low text_quality): both flags fire, but + # glyph-corrupt wins the route (structured, not ocr) -- the structured + # re-extract is the cheaper correct fix, and re-classification catches any + # residual junk afterwards. + text = "WKHTXLFNEURZQIRAMXPSV\x0f\x10\x11\x0f\x10" * 3 + c = clf.classify_from_text( + text, [{"page": 1, "start_offset": 0, "end_offset": len(text)}] + ) + assert c.recommended_tier == "structured" + assert "corrupt_glyphs" in c.flags + assert "bad_text_layer" in c.flags + + def test_classify_pdf_glyph_corrupt_routes_structured(): # Symmetry with the classify_from_text routing on the standalone/diagnostic # classify_pdf path (which re-opens the PDF and samples pages). diff --git a/tests/unit/test_registry_tiering.py b/tests/unit/test_registry_tiering.py index 5263f7c3..6b00a092 100644 --- a/tests/unit/test_registry_tiering.py +++ b/tests/unit/test_registry_tiering.py @@ -273,13 +273,29 @@ async def test_glyph_corrupt_escalates_fast_to_structured(monkeypatch): async def test_glyph_corrupt_no_structured_stays_fast(monkeypatch): - # No structured processor registered -> nothing to escalate to; keep fast. + # No structured processor registered AND OCR off -> nothing to escalate to; + # keep fast (the inline counterpart of the external "suppressed" outcome). monkeypatch.setattr(reg_mod, "get_settings", lambda: _Settings(ocr=False)) r = _registry((_Fake("fast", "fast", text=_GLYPH), 20)) res = await r.process(b"%PDF-1.7", "application/pdf") assert res.processor == "fast" +async def test_glyph_corrupt_no_structured_falls_through_to_ocr(monkeypatch): + # Parity with the external path: structured unregistered but OCR enabled -> + # the glyph-corrupt doc falls through to OCR (not silently kept at fast). + monkeypatch.setattr(reg_mod, "get_settings", lambda: _Settings(ocr=True)) + esc = MagicMock() + monkeypatch.setattr(reg_mod, "record_document_escalation", esc) + r = _registry( + (_Fake("fast", "fast", text=_GLYPH), 20), + (_Fake("ocr", "ocr", text="ocr recovered text"), 5), + ) # no structured registered + res = await r.process(b"%PDF-1.7", "application/pdf") + assert res.processor == "ocr" + esc.assert_called_once_with("fast", "ocr", "corrupt_glyphs") + + async def test_inline_lowconf_tries_structured_before_ocr(monkeypatch): # Full-ladder parity with the external path: a junk-but-non-empty fast layer # tries structured (fast->structured) BEFORE any OCR, even with OCR enabled. @@ -355,6 +371,30 @@ def test_evaluate_escalation_glyph_corrupt_goes_structured(monkeypatch): assert decision == EscalationDecision("hop", "structured", "corrupt_glyphs") +def test_evaluate_escalation_glyph_corrupt_no_structured_falls_through_to_ocr( + monkeypatch, +): + # External path with structured unregistered: next_available_tier skips the + # missing rung and lands on OCR, keeping the corrupt_glyphs reason. + monkeypatch.setattr(reg_mod, "record_document_classification", MagicMock()) + r = _registry( + (_Fake("fast", "fast"), 20), + (_Fake("ocr", "ocr"), 5), + ) # no structured registered + res = ProcessingResult( + text=_GLYPH, + metadata={ + "page_count": 1, + "page_boundaries": [ + {"page": 1, "start_offset": 0, "end_offset": len(_GLYPH)} + ], + }, + processor="fast", + ) + decision = r.evaluate_escalation(res, b"%PDF", "fast", _Settings(ocr=True)) + assert decision == EscalationDecision("hop", "ocr", "corrupt_glyphs") + + # --- Per-tier external path (Deck #323) ------------------------------------- From 4af7c7104b4047b5cb27ffb16d22f25e340c214b Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Tue, 16 Jun 2026 20:43:25 +0200 Subject: [PATCH 5/5] fix(document-processors): make glyph-corruption ratio of 0 disable the signal Address round-4 review on PR #914: - glyph_corruption_ratio <= 0 now disables the signal (previously `control_ratio > 0` fired on any single C0 control byte), matching the "0 disables" convention used elsewhere (document_max_pdf_size_mb) and the config comment. Add a zero-disables test. - Correct the document_escalation_suppressed_total comment: corrupt_glyphs CAN appear there in the narrow case where structured is unregistered and OCR is registered-but-disabled (evaluate_escalation follows minimum="structured" past the missing rung to a gated-off OCR). Add a test for that suppressed decision. - Add a test for the double-corruption edge: a structured re-extract that is also glyph-corrupt escalates structured->ocr with reason corrupt_glyphs. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../document_processors/classifier.py | 12 +++-- nextcloud_mcp_server/observability/metrics.py | 7 +-- tests/unit/test_doc_classifier.py | 9 ++++ tests/unit/test_registry_tiering.py | 45 +++++++++++++++++++ 4 files changed, 67 insertions(+), 6 deletions(-) diff --git a/nextcloud_mcp_server/document_processors/classifier.py b/nextcloud_mcp_server/document_processors/classifier.py index 26dcb548..ecf76877 100644 --- a/nextcloud_mcp_server/document_processors/classifier.py +++ b/nextcloud_mcp_server/document_processors/classifier.py @@ -203,9 +203,15 @@ def _route_from_signals( Flags are diagnostic and independent of the verdict (e.g. ``image_heavy`` fires on ANY image-heavy page; the OCR route needs a page FRACTION). """ - # total_chars > 0 also guarantees this never overlaps the scanned branch - # (total_chars == 0), so a doc is never both glyph-corrupt and "scanned". - glyph_corrupt = total_chars > 0 and control_ratio > glyph_corruption_ratio + # glyph_corruption_ratio <= 0 disables the signal (a ratio of 0 would otherwise + # fire on any single C0 control byte). total_chars > 0 also guarantees this + # never overlaps the scanned branch (total_chars == 0), so a doc is never both + # glyph-corrupt and "scanned". + glyph_corrupt = ( + glyph_corruption_ratio > 0 + and total_chars > 0 + and control_ratio > glyph_corruption_ratio + ) flags: set[str] = set() if ocr_frac >= page_fraction and total_chars == 0: diff --git a/nextcloud_mcp_server/observability/metrics.py b/nextcloud_mcp_server/observability/metrics.py index 26e20421..139b6c87 100644 --- a/nextcloud_mcp_server/observability/metrics.py +++ b/nextcloud_mcp_server/observability/metrics.py @@ -287,9 +287,10 @@ document_escalation_total = Counter( document_escalation_suppressed_total = Counter( "astrolabe_document_escalation_suppressed_total", "Would-be tier escalations suppressed because the target tier is disabled", - # reason: low_confidence | empty_text. (corrupt_glyphs never appears here: it - # targets the structured tier, which has no enabled-gate -- if registered it - # runs, else there is nothing to suppress -- so it only ever hops or returns.) + # reason: low_confidence | empty_text | corrupt_glyphs. (corrupt_glyphs lands + # here only in the narrow case where the structured tier is unregistered AND + # OCR is registered-but-disabled: evaluate_escalation follows minimum="structured" + # past the missing rung to OCR, which is gated off -> suppressed{to_tier="ocr"}.) ["from_tier", "to_tier", "reason"], ) diff --git a/tests/unit/test_doc_classifier.py b/tests/unit/test_doc_classifier.py index 5d843f6f..29400159 100644 --- a/tests/unit/test_doc_classifier.py +++ b/tests/unit/test_doc_classifier.py @@ -387,6 +387,15 @@ def test_glyph_corruption_ratio_override_disables_trigger(): assert "corrupt_glyphs" not in c.flags +def test_glyph_corruption_ratio_zero_disables_trigger(): + full = _GLYPH_CORRUPT + bounds = [{"page": 1, "start_offset": 0, "end_offset": len(full)}] + # 0 disables the signal (rather than firing on any single control byte). + c = clf.classify_from_text(full, bounds, glyph_corruption_ratio=0.0) + assert c.recommended_tier == "fast" + assert "corrupt_glyphs" not in c.flags + + def test_empty_doc_routes_ocr_not_structured(): # Precedence: a scanned/empty doc (no text layer) has no control chars to leak, # so it must stay an OCR case, never structured. diff --git a/tests/unit/test_registry_tiering.py b/tests/unit/test_registry_tiering.py index 6b00a092..d2c9d6df 100644 --- a/tests/unit/test_registry_tiering.py +++ b/tests/unit/test_registry_tiering.py @@ -349,6 +349,26 @@ async def test_inline_fast_structured_ocr_cascade(monkeypatch): ] +async def test_inline_structured_still_corrupt_escalates_to_ocr(monkeypatch): + # Edge: the structured re-extract is ALSO glyph-corrupt (pymupdf also failed to + # decode). Re-classification stays "structured", so the OCR gate fires -- + # attributed from_tier="structured" with reason corrupt_glyphs. + monkeypatch.setattr(reg_mod, "get_settings", lambda: _Settings(ocr=True)) + esc = MagicMock() + monkeypatch.setattr(reg_mod, "record_document_escalation", esc) + r = _registry( + (_Fake("fast", "fast", text=_GLYPH), 20), + (_Fake("structured", "structured", text=_GLYPH), 10), # still corrupt + (_Fake("ocr", "ocr", text="ocr recovered text"), 5), + ) + res = await r.process(b"%PDF-1.7", "application/pdf") + assert res.processor == "ocr" + assert esc.call_args_list == [ + call("fast", "structured", "corrupt_glyphs"), + call("structured", "ocr", "corrupt_glyphs"), + ] + + def test_evaluate_escalation_glyph_corrupt_goes_structured(monkeypatch): # External path mirrors the inline path: glyph-corrupt -> structured, never OCR. monkeypatch.setattr(reg_mod, "record_document_classification", MagicMock()) @@ -395,6 +415,31 @@ def test_evaluate_escalation_glyph_corrupt_no_structured_falls_through_to_ocr( assert decision == EscalationDecision("hop", "ocr", "corrupt_glyphs") +def test_evaluate_escalation_glyph_corrupt_no_structured_ocr_disabled_suppressed( + monkeypatch, +): + # Structured unregistered AND OCR registered-but-disabled: the would-be OCR + # fallthrough is suppressed, and it carries the corrupt_glyphs reason (so the + # "what-if OCR" counter can show latent glyph-corruption demand). + monkeypatch.setattr(reg_mod, "record_document_classification", MagicMock()) + r = _registry( + (_Fake("fast", "fast"), 20), + (_Fake("ocr", "ocr"), 5), + ) # structured not registered; ocr registered but disabled below + res = ProcessingResult( + text=_GLYPH, + metadata={ + "page_count": 1, + "page_boundaries": [ + {"page": 1, "start_offset": 0, "end_offset": len(_GLYPH)} + ], + }, + processor="fast", + ) + decision = r.evaluate_escalation(res, b"%PDF", "fast", _Settings(ocr=False)) + assert decision == EscalationDecision("suppressed", "ocr", "corrupt_glyphs") + + # --- Per-tier external path (Deck #323) -------------------------------------