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) -------------------------------------