Merge pull request #892 from cbcoutinho/feat/309-ocr-timeout-pdf-size-guard

feat(document): configurable OCR timeout and fail-fast PDF size guard

docs/configuration.md

@@ -536,6 +536,23 @@ DOCUMENT_CHUNK_OVERLAP=200            # Overlapping characters between chunks (d
 
 > **Note:** The `VECTOR_SYNC_*` tuning parameters keep their names as they're implementation details. Only the user-facing feature flag was renamed to `ENABLE_SEMANTIC_SEARCH`.
 
+#### Document parsing robustness (PDF)
+
+These guard the parse/OCR tiers against pathological PDFs. Defaults are safe;
+tune per tenant when a corpus has very large scans or a gateway with its own
+shorter OCR ceiling:
+
+```dotenv
+DOCUMENT_PARSE_TIMEOUT_SECONDS=120    # Wall-clock cap per isolated parse (default: 120)
+DOCUMENT_OCR_TIMEOUT_SECONDS=180      # OCR backend request timeout (default: 180)
+DOCUMENT_MAX_PDF_SIZE_MB=50           # Pre-parse size cap; 0 disables (default: 50)
+```
+
+A PDF larger than `DOCUMENT_MAX_PDF_SIZE_MB` fails fast with reason `oversize`
+(exported on `astrolabe_document_parse_failed_total{reason="oversize"}`) instead
+of being handed to the tiers, where a 40+ MB scan would otherwise burn the full
+OCR timeout for zero recovered text.
+
 ### Embedding Service Configuration
 
 The server picks an embedding provider via auto-detection. Priority order

nextcloud_mcp_server/config.py

@@ -146,6 +146,10 @@ _DEFAULTS: dict[str, Any] = {
     "document_pdf_graphics_limit": 1000,
     "document_parse_timeout_seconds": 120.0,
     "document_parse_mem_limit_mb": 1536,
+    # Pre-parse size cap (MB): PDFs larger than this fail fast with reason
+    # "oversize" instead of burning the OCR timeout to 0 chars on a pathological
+    # file. 0 disables the guard.
+    "document_max_pdf_size_mb": 50.0,
     # Tier-0 classifier (records classification metrics on the tiered path)
     "document_classify_enabled": True,
     # Tiered PDF pipeline: pypdfium2 is the default/only hot-path extractor;
@@ -168,6 +172,10 @@ _DEFAULTS: dict[str, Any] = {
     "document_ocr_page_fraction": 0.5,
     "document_ocr_min_page_chars": 16,
     "document_ocr_detect_scanned": True,
+    # 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.
+    "document_ocr_timeout_seconds": 180.0,
     # Observability
     "metrics_enabled": True,
     "metrics_port": 9090,
@@ -319,7 +327,10 @@ _dynaconf = Dynaconf(
         Validator("VERIFICATION_CONCURRENCY", gte=1),
         Validator("DOCUMENT_CHUNK_SIZE", gte=1),
         Validator("DOCUMENT_PARSE_TIMEOUT_SECONDS", gte=1),
+        Validator("DOCUMENT_OCR_TIMEOUT_SECONDS", gte=1),
         Validator("DOCUMENT_PARSE_MEM_LIMIT_MB", gte=128),
+        # 0 disables the pre-parse PDF size cap; otherwise it must be positive.
+        Validator("DOCUMENT_MAX_PDF_SIZE_MB", gte=0),
         # >=1: pymupdf4llm treats graphics_limit=0 as "no cap", which would
         # re-expose the OOM this guards against.
         Validator("DOCUMENT_PDF_GRAPHICS_LIMIT", gte=1),
@@ -782,6 +793,11 @@ class Settings:
     # float so a fractional DOCUMENT_PARSE_TIMEOUT_SECONDS is honoured, matching
     # anyio.move_on_after's float seconds.
     document_parse_timeout_seconds: float = 120.0
+    # Pre-parse PDF size cap (MB). A PDF larger than this fails fast with
+    # parse_failed_reason="oversize" (placeholder marked "failed") rather than
+    # being handed to the fast/OCR tiers, where a pathological large file burns
+    # the OCR timeout for 0 chars. 0 disables the guard.
+    document_max_pdf_size_mb: float = 50.0
     # RLIMIT_AS in the parse subprocess (below the pod limit). Applied once per
     # worker for its lifetime, so changing it needs a pod restart.
     document_parse_mem_limit_mb: int = 1536
@@ -801,6 +817,10 @@ class Settings:
     # gateway routes on the "<provider>/" prefix; the direct mistral backend
     # strips it.
     document_ocr_model: str = "mistral/mistral-ocr-latest"
+    # OCR backend HTTP request timeout (seconds). float for parity with the
+    # parse timeout / httpx.Timeout; per-tenant tunable so a gateway with a
+    # shorter ceiling isn't masked by the 180s default.
+    document_ocr_timeout_seconds: float = 180.0
     # OCR escalation triggers (tier-0), per-tenant tunable. A page is OCR-worthy
     # if near-empty (< min_page_chars) OR low text-quality (< min_text_quality)
     # OR (when detect_scanned, image-analysis only runs when OCR is enabled)
@@ -1431,12 +1451,14 @@ def get_settings() -> Settings:
         "document_chunk_page_aware": "DOCUMENT_CHUNK_PAGE_AWARE",
         "document_pdf_graphics_limit": "DOCUMENT_PDF_GRAPHICS_LIMIT",
         "document_parse_timeout_seconds": "DOCUMENT_PARSE_TIMEOUT_SECONDS",
+        "document_max_pdf_size_mb": "DOCUMENT_MAX_PDF_SIZE_MB",
         "document_parse_mem_limit_mb": "DOCUMENT_PARSE_MEM_LIMIT_MB",
         "document_classify_enabled": "DOCUMENT_CLASSIFY_ENABLED",
         "document_tier1_engine": "DOCUMENT_TIER1_ENGINE",
         "document_ocr_enabled": "DOCUMENT_OCR_ENABLED",
         "document_ocr_provider": "DOCUMENT_OCR_PROVIDER",
         "document_ocr_model": "DOCUMENT_OCR_MODEL",
+        "document_ocr_timeout_seconds": "DOCUMENT_OCR_TIMEOUT_SECONDS",
         "document_ocr_min_text_quality": "DOCUMENT_OCR_MIN_TEXT_QUALITY",
         "document_ocr_page_fraction": "DOCUMENT_OCR_PAGE_FRACTION",
         "document_ocr_min_page_chars": "DOCUMENT_OCR_MIN_PAGE_CHARS",

nextcloud_mcp_server/document_processors/ocr.py

@@ -31,7 +31,9 @@ from .base import DocumentProcessor, ProcessingResult
 
 logger = logging.getLogger(__name__)
 
-_OCR_TIMEOUT_SECONDS = 180.0
+# Connect timeout for the OCR backend request. The overall (read) timeout is
+# configurable via DOCUMENT_OCR_TIMEOUT_SECONDS and resolved per call.
+_OCR_CONNECT_TIMEOUT_SECONDS = 10.0
 
 
 def _pages_to_text(
@@ -93,8 +95,12 @@ class _GatewayOcrBackend(_OcrBackend):
             "document_b64": base64.b64encode(content).decode("ascii"),
             "mime_type": mime_type,
         }
+        # Resolved per call (get_settings builds fresh) so test monkeypatching is
+        # honoured; a live tenant change still needs a restart because the backend
+        # instance itself is cached for the pod's lifetime.
+        ocr_timeout = get_settings().document_ocr_timeout_seconds
         async with httpx.AsyncClient(
-            timeout=httpx.Timeout(_OCR_TIMEOUT_SECONDS, connect=10.0)
+            timeout=httpx.Timeout(ocr_timeout, connect=_OCR_CONNECT_TIMEOUT_SECONDS)
         ) as client:
             resp = await client.post(self._url, json=payload, headers=headers)
             resp.raise_for_status()
@@ -120,10 +126,17 @@ class _MistralOcrBackend(_OcrBackend):
         data_url = (
             f"data:{mime_type};base64,{base64.b64encode(content).decode('ascii')}"
         )
-        resp = await self._client.ocr.process_async(
-            model=self._model,
-            document={"type": "document_url", "document_url": data_url},
-        )
+        # Apply DOCUMENT_OCR_TIMEOUT_SECONDS uniformly with the gateway backend.
+        # The Mistral SDK manages its own httpx client, so wrap the call in an
+        # anyio cancel-scope timeout rather than threading a per-request timeout
+        # through the SDK; on expiry this raises TimeoutError, which the
+        # OcrProcessor turns into a clean parse failure.
+        ocr_timeout = get_settings().document_ocr_timeout_seconds
+        with anyio.fail_after(ocr_timeout):
+            resp = await self._client.ocr.process_async(
+                model=self._model,
+                document={"type": "document_url", "document_url": data_url},
+            )
         pages = [(p.index, p.markdown or "") for p in (resp.pages or [])]
         return _pages_to_text(pages)
 
@@ -251,6 +264,24 @@ class OcrProcessor(DocumentProcessor):
             text, boundaries = await backend.ocr(
                 content, content_type.split(";")[0].strip().lower()
             )
+        except (TimeoutError, httpx.TimeoutException):
+            # Two timeout shapes reach here: the Mistral backend's
+            # anyio.fail_after raises the builtin TimeoutError, while the gateway
+            # backend's httpx.Timeout raises httpx.ReadTimeout (a
+            # httpx.TimeoutException, NOT a TimeoutError). Catch both so a
+            # too-low DOCUMENT_OCR_TIMEOUT_SECONDS lands in its own reason bucket
+            # rather than being conflated with provider errors.
+            timeout = settings.document_ocr_timeout_seconds
+            logger.warning(
+                "OCR timed out for %s after %.1fs", filename or "<bytes>", timeout
+            )
+            return ProcessingResult(
+                text="",
+                metadata={"parse_failed_reason": "timeout"},
+                processor=self.name,
+                success=False,
+                error=f"OCR timed out after {timeout:.1f}s",
+            )
         except Exception as e:
             logger.warning("OCR failed for %s: %s", filename or "<bytes>", e)
             return ProcessingResult(

nextcloud_mcp_server/document_processors/registry.py

@@ -202,6 +202,33 @@ class ProcessorRegistry:
         """
         settings = get_settings()
 
+        # Pre-parse size guard: a pathologically large PDF (e.g. a 42 MB scanned
+        # DUDE) burns the OCR timeout for 0 chars. Fail fast with an explicit
+        # reason so the caller marks the placeholder "failed" instead of
+        # retrying. 0 disables the cap. This lives on the auto-tiered path only:
+        # an explicit processor_name="ocr" override (registry.process) bypasses
+        # _process_pdf entirely and is intentionally not size-gated (power-user
+        # escape hatch). Returning here also skips _run_processor, so the
+        # rejection is counted on astrolabe_document_parse_failed_total{oversize}
+        # (via vector/processor.py) but deliberately not on the parse-duration
+        # histogram -- there is no parse to time.
+        max_pdf_mb = settings.document_max_pdf_size_mb
+        if max_pdf_mb > 0 and len(content) > max_pdf_mb * 1024 * 1024:
+            size_mb = len(content) / (1024 * 1024)
+            logger.warning(
+                "PDF %s is %.1f MB (> %.1f MB cap); failing fast as oversize",
+                filename or "<bytes>",
+                size_mb,
+                max_pdf_mb,
+            )
+            return ProcessingResult(
+                text="",
+                metadata={"parse_failed_reason": "oversize"},
+                processor="size_guard",
+                success=False,
+                error=(f"PDF exceeds size cap: {size_mb:.1f} MB > {max_pdf_mb:.1f} MB"),
+            )
+
         if settings.document_tier1_engine == "pymupdf":
             processor = self._pdf_processor_for_tier("structured")
             if processor is None:

tests/unit/test_config.py

@@ -213,6 +213,24 @@ class TestChunkConfigValidation:
         _reload_config()
         assert get_settings().document_chunk_page_aware is False
 
+    def test_ocr_timeout_default_and_env_override(self):
+        """document_ocr_timeout_seconds defaults to 180 and reads its env var.
+
+        Guards the _DEFAULTS-key-must-match-env-var footgun: a mismatch would
+        leave the override silently ignored.
+        """
+        assert Settings().document_ocr_timeout_seconds == pytest.approx(180.0)
+        with patch.dict(os.environ, {"DOCUMENT_OCR_TIMEOUT_SECONDS": "45"}, clear=True):
+            _reload_config()
+            assert get_settings().document_ocr_timeout_seconds == pytest.approx(45.0)
+
+    def test_max_pdf_size_default_and_env_override(self):
+        """document_max_pdf_size_mb defaults to 50 and reads its env var."""
+        assert Settings().document_max_pdf_size_mb == pytest.approx(50.0)
+        with patch.dict(os.environ, {"DOCUMENT_MAX_PDF_SIZE_MB": "12.5"}, clear=True):
+            _reload_config()
+            assert get_settings().document_max_pdf_size_mb == pytest.approx(12.5)
+
     def test_valid_chunk_settings(self):
         """Test valid chunk size and overlap configuration."""
         settings = Settings(
@@ -491,6 +509,22 @@ class TestDynaconfValidators:
         with pytest.raises(ValidationError, match="DOCUMENT_CHUNK_SIZE"):
             _reload_config()
 
+    @patch.dict(os.environ, {"DOCUMENT_OCR_TIMEOUT_SECONDS": "0"}, clear=True)
+    def test_ocr_timeout_zero_rejected(self):
+        """DOCUMENT_OCR_TIMEOUT_SECONDS=0 fails the gte=1 validator."""
+        from dynaconf import ValidationError
+
+        with pytest.raises(ValidationError, match="DOCUMENT_OCR_TIMEOUT_SECONDS"):
+            _reload_config()
+
+    @patch.dict(os.environ, {"DOCUMENT_MAX_PDF_SIZE_MB": "-1"}, clear=True)
+    def test_max_pdf_size_negative_rejected(self):
+        """DOCUMENT_MAX_PDF_SIZE_MB=-1 fails the gte=0 validator (0 = disabled)."""
+        from dynaconf import ValidationError
+
+        with pytest.raises(ValidationError, match="DOCUMENT_MAX_PDF_SIZE_MB"):
+            _reload_config()
+
     @patch.dict(os.environ, {"METRICS_PORT": "8080"}, clear=True)
     def test_valid_metrics_port(self):
         """Test valid METRICS_PORT passes validation."""

tests/unit/test_ocr_processor.py

@@ -3,6 +3,7 @@
 from types import SimpleNamespace
 from typing import Any
 
+import anyio
 import pytest
 
 from nextcloud_mcp_server.document_processors import ocr
@@ -14,6 +15,7 @@ def _settings(**kw) -> Any:  # a Settings stand-in (only the read fields matter)
     base = dict(
         document_ocr_provider="auto",
         document_ocr_model="mistral/mistral-ocr-latest",
+        document_ocr_timeout_seconds=180.0,
         embedding_gateway_url=None,
         embedding_gateway_client_id=None,
         embedding_gateway_client_secret=None,
@@ -128,3 +130,91 @@ async def test_processor_backend_error_returns_success_false(monkeypatch):
     r = await ocr.OcrProcessor().process(b"%PDF-1.7", "application/pdf")
     assert r.success is False
     assert r.metadata["parse_failed_reason"] == "error"
+
+
+async def test_processor_timeout_returns_timeout_reason(monkeypatch):
+    """A backend TimeoutError gets its own reason bucket (not 'error')."""
+
+    class _TimeoutBackend:
+        async def ocr(self, content, mime_type):
+            raise TimeoutError
+
+    monkeypatch.setattr(
+        ocr, "get_settings", lambda: _settings(document_ocr_timeout_seconds=5.0)
+    )
+    monkeypatch.setattr(ocr, "build_ocr_backend", lambda s: _TimeoutBackend())
+    r = await ocr.OcrProcessor().process(b"%PDF-1.7", "application/pdf")
+    assert r.success is False
+    assert r.metadata["parse_failed_reason"] == "timeout"
+    assert "timed out" in r.error
+
+
+async def test_gateway_httpx_timeout_maps_to_timeout_reason(monkeypatch):
+    """A gateway httpx.ReadTimeout (not a builtin TimeoutError) must still map to
+    parse_failed_reason='timeout', not 'error'."""
+    import httpx
+
+    class _HttpxTimeoutBackend:
+        async def ocr(self, content, mime_type):
+            raise httpx.ReadTimeout("read timed out")
+
+    monkeypatch.setattr(
+        ocr, "get_settings", lambda: _settings(document_ocr_timeout_seconds=5.0)
+    )
+    monkeypatch.setattr(ocr, "build_ocr_backend", lambda s: _HttpxTimeoutBackend())
+    r = await ocr.OcrProcessor().process(b"%PDF-1.7", "application/pdf")
+    assert r.success is False
+    assert r.metadata["parse_failed_reason"] == "timeout"
+    assert "timed out" in r.error
+
+
+async def test_gateway_backend_uses_configured_timeout(mocker, monkeypatch):
+    """The gateway OCR call must use DOCUMENT_OCR_TIMEOUT_SECONDS (resolved per
+    call), not the old hardcoded 180s constant."""
+    resp = mocker.Mock()
+    resp.raise_for_status = mocker.Mock()
+    resp.json = mocker.Mock(return_value={"pages": [{"index": 0, "markdown": "ok"}]})
+
+    client = mocker.MagicMock()
+    client.__aenter__ = mocker.AsyncMock(return_value=client)
+    client.__aexit__ = mocker.AsyncMock(return_value=False)
+    client.post = mocker.AsyncMock(return_value=resp)
+
+    captured: dict[str, Any] = {}
+
+    def _make_client(*args, **kwargs):
+        captured["timeout"] = kwargs.get("timeout")
+        return client
+
+    monkeypatch.setattr(ocr.httpx, "AsyncClient", _make_client)
+    monkeypatch.setattr(
+        ocr, "get_settings", lambda: _settings(document_ocr_timeout_seconds=42.0)
+    )
+
+    backend = ocr._GatewayOcrBackend("https://gw", "mistral/mistral-ocr-latest")
+    await backend.ocr(b"%PDF-1.7", "application/pdf")
+
+    # httpx.Timeout(42.0, connect=10.0): the read/overall budget is the setting.
+    assert captured["timeout"].read == pytest.approx(42.0)
+    assert captured["timeout"].connect == pytest.approx(10.0)
+
+
+async def test_mistral_backend_applies_timeout(mocker, monkeypatch):
+    """The Mistral backend wraps process_async in DOCUMENT_OCR_TIMEOUT_SECONDS,
+    so a slow OCR call fails fast instead of hanging on the SDK default."""
+    monkeypatch.setattr(
+        ocr, "get_settings", lambda: _settings(document_ocr_timeout_seconds=0.01)
+    )
+
+    # Bypass the SDK constructor; only the two attributes ocr() reads matter.
+    backend = ocr._MistralOcrBackend.__new__(ocr._MistralOcrBackend)
+    backend._model = "mistral-ocr-latest"
+
+    async def _slow(*args, **kwargs):
+        await anyio.sleep(1.0)
+
+    backend._client = mocker.MagicMock()
+    backend._client.ocr.process_async = _slow
+
+    with pytest.raises(TimeoutError):
+        await backend.ocr(b"%PDF-1.7", "application/pdf")

tests/unit/test_registry_tiering.py

@@ -74,6 +74,9 @@ class _Settings:
         page_fraction=0.5,
         min_page_chars=16,
         detect_scanned=False,
+        # 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,
     ):
         self.document_tier1_engine = engine
         self.document_classify_enabled = classify
@@ -82,6 +85,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_max_pdf_size_mb = max_pdf_size_mb
 
 
 def _registry(*procs: tuple[DocumentProcessor, int]) -> ProcessorRegistry:
@@ -98,6 +102,43 @@ async def test_pdf_routes_to_fast_tier(monkeypatch):
     assert res.processor == "fast"
 
 
+async def test_oversize_pdf_fails_fast_without_parsing(monkeypatch):
+    """A PDF over the size cap must fail fast as 'oversize' before any tier runs."""
+    monkeypatch.setattr(
+        reg_mod, "get_settings", lambda: _Settings(max_pdf_size_mb=0.001)
+    )
+    fast = _Fake("fast", "fast")
+    ran = False
+    orig = fast.process
+
+    async def _tracking(*a, **k):
+        nonlocal ran
+        ran = True
+        return await orig(*a, **k)
+
+    fast.process = _tracking  # type: ignore[method-assign]
+    r = _registry((fast, 20))
+
+    # ~2 KB > 0.001 MB (~1 KB) cap.
+    res = await r.process(b"%PDF-1.7" + b"0" * 2048, "application/pdf", "big.pdf")
+
+    assert res.success is False
+    assert res.metadata["parse_failed_reason"] == "oversize"
+    assert res.processor == "size_guard"
+    assert ran is False, "size guard must short-circuit before the fast tier runs"
+
+
+async def test_under_cap_pdf_still_parses(monkeypatch):
+    """A PDF under the cap is unaffected by the guard."""
+    monkeypatch.setattr(
+        reg_mod, "get_settings", lambda: _Settings(max_pdf_size_mb=10.0)
+    )
+    r = _registry((_Fake("fast", "fast"), 20))
+    res = await r.process(b"%PDF-1.7", "application/pdf")
+    assert res.success is True
+    assert res.processor == "fast"
+
+
 async def test_engine_rollback_uses_structured(monkeypatch):
     monkeypatch.setattr(reg_mod, "get_settings", lambda: _Settings(engine="pymupdf"))
     r = _registry((_Fake("fast", "fast"), 20), (_Fake("structured", "structured"), 10))