diff --git a/nextcloud_mcp_server/document_processors/ocr.py b/nextcloud_mcp_server/document_processors/ocr.py index bb788f53..855516d0 100644 --- a/nextcloud_mcp_server/document_processors/ocr.py +++ b/nextcloud_mcp_server/document_processors/ocr.py @@ -287,7 +287,9 @@ def build_ocr_backend( class OcrProcessor(DocumentProcessor): - """Tier-3 OCR processor (gateway or direct Mistral backend).""" + """OCR processor for both OCR rungs — tier2 in-cluster (gateway-only GPU) and + tier3 upstream (gateway or direct Mistral backend). One class, two registered + instances bound to different ``(tier, model_setting, gateway_only)``.""" def __init__( self, @@ -303,6 +305,11 @@ class OcrProcessor(DocumentProcessor): # gateway-only (its model, e.g. surya, is reachable solely via the # gateway); the upstream rung keeps the configurable gateway/mistral # selection. surya is NEVER hard-coded here — only a config default. + # Fail fast on a misconfigured model_setting (a typo in a constructor call) + # so it surfaces at startup, not as an AttributeError mid-OCR. The string + # only ever comes from hardcoded defaults in __init__.py, never user input. + if not hasattr(Settings, model_setting): + raise ValueError(f"Unknown model_setting: {model_setting!r}") self._name = name self._tier = tier self._model_setting = model_setting @@ -450,9 +457,10 @@ class OcrProcessor(DocumentProcessor): self._batch_client_lock = anyio.Lock() async with self._batch_client_lock: if not self._batch_client_resolved: # double-checked + settings = get_settings() self._batch_client = build_gateway_batch_client( - get_settings(), - model=getattr(get_settings(), self._model_setting), + settings, + model=getattr(settings, self._model_setting), ) self._batch_client_resolved = True return self._batch_client diff --git a/nextcloud_mcp_server/document_processors/registry.py b/nextcloud_mcp_server/document_processors/registry.py index b5a59638..1230b5c1 100644 --- a/nextcloud_mcp_server/document_processors/registry.py +++ b/nextcloud_mcp_server/document_processors/registry.py @@ -360,6 +360,10 @@ class ProcessorRegistry: from_tier, settings, minimum="ocr-incluster" ) ocr = self._pdf_processor_for_tier(ocr_tier) if ocr_tier else None + # `ocr is not None` already implies `ocr_tier is not None` at runtime, + # but the type checker can't infer that across the conditional above, + # so the explicit guard narrows `ocr_tier` to `str` for the + # record_document_escalation(from_tier, ocr_tier, reason) call below. if ocr is not None and ocr_tier is not None: reason = ( "corrupt_glyphs" @@ -514,8 +518,8 @@ class ProcessorRegistry: ``ignore_ocr_enabled`` drops only the OCR-enabled gate (not the registered- processor requirement): it answers "would this tier run if OCR were turned on?" — used to compute the *ideal* escalation target for the what-if-OCR - suppressed-escalation signal. (Today only ``ocr`` has an enabled gate; a - future per-tier gate would extend the condition below.) + suppressed-escalation signal. (Both OCR rungs — ``ocr-incluster`` and + ``ocr-upstream`` — have their own enabled gate; non-OCR tiers have none.) """ if self._pdf_processor_for_tier(tier) is None: return False @@ -624,9 +628,11 @@ class ProcessorRegistry: Target-tier routing: - - ``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. + - ``total_chars == 0`` (scanned / no text layer) -> target the cheapest + OCR rung (``ocr-incluster``) directly; ``next_available_tier`` falls + through to ``ocr-upstream`` if in-cluster is disabled/unregistered. + 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. diff --git a/nextcloud_mcp_server/vector/queue/procrastinate.py b/nextcloud_mcp_server/vector/queue/procrastinate.py index e8a38e72..8205ea05 100644 --- a/nextcloud_mcp_server/vector/queue/procrastinate.py +++ b/nextcloud_mcp_server/vector/queue/procrastinate.py @@ -119,6 +119,15 @@ def tier_for_queue(queue: str | None) -> str: The queue-aware task uses this to pick which single tier to parse with: the job's current queue *is* its tier. A job on the legacy ``ingest`` queue (or any unrecognised queue) defaults to the cheapest tier. + + The pre-split legacy OCR queue ``ingest-ocr`` (``LEGACY_INGEST_QUEUE_OCR``) + is deliberately NOT mapped here, so it also resolves to ``fast``. These are + in-flight jobs enqueued before the tier2/tier3 split; running them at fast + re-extracts (an empty layer for a scanned doc), which re-enters the ladder + and naturally re-escalates to ``ocr-incluster`` (the cheap GPU rung) — one + extra cheap hop, but it keeps stranded legacy OCR jobs OFF the paid upstream + rung rather than mapping them straight to ``ocr-upstream``. The set is + transient (only during a single rollout window). """ return _QUEUE_TIERS.get(queue or "", "fast") diff --git a/tests/unit/test_registry_tiering.py b/tests/unit/test_registry_tiering.py index 54acf67c..90a119e8 100644 --- a/tests/unit/test_registry_tiering.py +++ b/tests/unit/test_registry_tiering.py @@ -803,6 +803,26 @@ async def test_inline_incluster_disabled_skips_to_upstream(monkeypatch): esc.assert_called_once_with("fast", "ocr-upstream", "empty_text") +async def test_inline_incluster_failure_falls_back_to_fast_not_upstream(monkeypatch): + """CURRENT behavior (pins the known follow-up gap): when the chosen in-cluster + rung runs but FAILS (e.g. GPU 503 -> success=False), the inline path keeps the + tier-1 result rather than cascading to the paid upstream rung. OCR is an + enhancement, not a gate. (A future change will escalate transient GPU failures + to ocr-upstream; this test makes that diff explicit.)""" + monkeypatch.setattr( + reg_mod, "get_settings", lambda: _Settings(ocr=True, ocr_incluster=True) + ) + monkeypatch.setattr(reg_mod, "record_document_escalation", MagicMock()) + r = _registry( + (_Fake("fast", "fast", text=""), 20), + (_Fake("ocr-incluster", "ocr-incluster", text="", success=False), 6), + (_Fake("ocr-upstream", "ocr-upstream", text="upstream ocr text"), 5), + ) + res = await r.process(b"%PDF-1.7", "application/pdf") + assert res.processor == "fast" + assert res.success is True + + def _empty_result() -> ProcessingResult: return ProcessingResult( text="", diff --git a/tests/unit/vector/test_tiered_escalation_strategy.py b/tests/unit/vector/test_tiered_escalation_strategy.py index 04147c9a..cf0f342c 100644 --- a/tests/unit/vector/test_tiered_escalation_strategy.py +++ b/tests/unit/vector/test_tiered_escalation_strategy.py @@ -51,6 +51,10 @@ class TestLadder: # Legacy / unknown / None all fall back to the cheapest tier. assert pq.tier_for_queue(pq.LEGACY_INGEST_QUEUE) == "fast" assert pq.tier_for_queue(None) == "fast" + # The pre-split legacy OCR queue also resolves to fast (NOT ocr-upstream): + # stranded in-flight jobs re-extract empty and re-escalate via the ladder + # to the cheap ocr-incluster rung, never straight to paid upstream. + assert pq.tier_for_queue(pq.LEGACY_INGEST_QUEUE_OCR) == "fast" class TestTieredEscalationStrategy: