From 523e4cb7b56c28ba8f844be7e21a1097a98c3544 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Thu, 11 Jun 2026 05:09:58 +0200 Subject: [PATCH] feat(document): configurable OCR timeout and fail-fast PDF size guard Two ingest-robustness fixes from card 309 (OHR-Bench smoke-test triage). The OCR backend timeout was a hardcoded 180s module constant, so a tenant whose gateway has its own shorter ceiling couldn't tune it. Promote it to DOCUMENT_OCR_TIMEOUT_SECONDS (default 180), resolved per call via get_settings so an override applies without a restart. Large, awkward PDFs (e.g. a 42 MB scanned DUDE) were handed straight to the fast/OCR tiers, where they burned the full OCR timeout for zero recovered text. Add a pre-parse size guard in the tiered PDF pipeline: a PDF over DOCUMENT_MAX_PDF_SIZE_MB (default 50, 0 disables) fails fast with parse_failed_reason="oversize" before any tier runs, so the existing permanent-failure path marks the placeholder failed and records astrolabe_document_parse_failed_total{reason="oversize"} instead of retrying. Both knobs go through Settings + dynaconf validators (env-var keys verified by regression tests) and are documented under Background Indexing Configuration. Refs: Deck board 12 card 309 (AC #3 OCR timeout + size guard). Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/configuration.md | 17 ++++++++ nextcloud_mcp_server/config.py | 22 +++++++++++ .../document_processors/ocr.py | 9 ++++- .../document_processors/registry.py | 21 ++++++++++ tests/unit/test_config.py | 18 +++++++++ tests/unit/test_ocr_processor.py | 39 +++++++++++++++++++ tests/unit/test_registry_tiering.py | 39 +++++++++++++++++++ 7 files changed, 163 insertions(+), 2 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 18223d71..cc928444 100644 --- a/docs/configuration.md +++ b/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 diff --git a/nextcloud_mcp_server/config.py b/nextcloud_mcp_server/config.py index 43330fdf..3afe22ce 100644 --- a/nextcloud_mcp_server/config.py +++ b/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 "/" 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", diff --git a/nextcloud_mcp_server/document_processors/ocr.py b/nextcloud_mcp_server/document_processors/ocr.py index 6178ab6c..ee3fc064 100644 --- a/nextcloud_mcp_server/document_processors/ocr.py +++ b/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,11 @@ class _GatewayOcrBackend(_OcrBackend): "document_b64": base64.b64encode(content).decode("ascii"), "mime_type": mime_type, } + # Resolve the timeout per call (get_settings builds fresh, so a test or + # tenant override is honoured without a restart). + 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() diff --git a/nextcloud_mcp_server/document_processors/registry.py b/nextcloud_mcp_server/document_processors/registry.py index d152d2a2..a4e84dee 100644 --- a/nextcloud_mcp_server/document_processors/registry.py +++ b/nextcloud_mcp_server/document_processors/registry.py @@ -202,6 +202,27 @@ 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. + 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 "", + 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: diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 2e6ba0e2..c8ada4ec 100644 --- a/tests/unit/test_config.py +++ b/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 == 180.0 + with patch.dict(os.environ, {"DOCUMENT_OCR_TIMEOUT_SECONDS": "45"}, clear=True): + _reload_config() + assert get_settings().document_ocr_timeout_seconds == 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 == 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 == 12.5 + def test_valid_chunk_settings(self): """Test valid chunk size and overlap configuration.""" settings = Settings( diff --git a/tests/unit/test_ocr_processor.py b/tests/unit/test_ocr_processor.py index 690f0c38..fdcdc1cb 100644 --- a/tests/unit/test_ocr_processor.py +++ b/tests/unit/test_ocr_processor.py @@ -14,6 +14,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 +129,41 @@ 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_gateway_backend_uses_configured_timeout(monkeypatch): + """The gateway OCR call must use DOCUMENT_OCR_TIMEOUT_SECONDS (resolved per + call), not the old hardcoded 180s constant.""" + captured: dict[str, Any] = {} + + class _FakeResponse: + def raise_for_status(self): + pass + + def json(self): + return {"pages": [{"index": 0, "markdown": "ok"}]} + + class _FakeClient: + def __init__(self, *, timeout=None, **kw): + captured["timeout"] = timeout + + async def __aenter__(self): + return self + + async def __aexit__(self, *exc): + return False + + async def post(self, url, json=None, headers=None): + return _FakeResponse() + + monkeypatch.setattr(ocr.httpx, "AsyncClient", _FakeClient) + monkeypatch.setattr( + ocr, "get_settings", lambda: _settings(document_ocr_timeout_seconds=42.0) + ) + + backend = ocr._GatewayOcrBackend("http://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 == 42.0 + assert captured["timeout"].connect == 10.0 diff --git a/tests/unit/test_registry_tiering.py b/tests/unit/test_registry_tiering.py index 5637f4da..818b0cad 100644 --- a/tests/unit/test_registry_tiering.py +++ b/tests/unit/test_registry_tiering.py @@ -74,6 +74,7 @@ class _Settings: page_fraction=0.5, min_page_chars=16, detect_scanned=False, + max_pdf_size_mb=0.0, ): self.document_tier1_engine = engine self.document_classify_enabled = classify @@ -82,6 +83,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 +100,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))