admin/mcp-nextcloud
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(review): cache OCR backend, drop asserts, real pipeline_tier, guard zero-page

Browse Source 
Address PR #858 review:

- 🔴 OcrProcessor now resolves its backend once and reuses it. Rebuilding per
  call created a fresh GatewayTokenProvider each time -- discarding its M2M-token
  cache, so every OCR'd document fetched a new token -- and a new Mistral client.
- 🔴 build_ocr_backend uses explicit ValueError (not assert, which is stripped
  under `python -O`) for the gateway M2M triple.
- PIPELINE_TIER in the Qdrant payload now reflects the tier that actually
  produced the doc: the registry stamps result.metadata["pipeline_tier"] and the
  processor reads it (was hardcoded "fast", wrong for OCR/structured).
- Escalation now requires classification.page_count > 0, so a zero-page
  (empty/corrupt) PDF isn't pointlessly sent to OCR; documented that a fast
  FAILURE (encrypted/unopenable) is a hard failure and is not OCR-escalated.
- Documented the OCR page_boundaries separator-attribution choice.
- Downgraded the per-document page-boundary / page-assignment INFO logs to debug.

New tests: zero-page no-escalation, pipeline_tier stamping.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Chris Coutinho
2026-06-05 02:13:09 +02:00
co-authored by Claude Opus 4.8
parent 4dbf362261
commit 1634e8adc2
4 changed files with 85 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
nextcloud_mcp_server/document_processors/ocr.py
+32 -6
View File
@@ -38,9 +38,13 @@ def _pages_to_text(
) -> tuple[str, list[dict[str, Any]]]:
"""Join per-page markdown (ordered by index) into one string + boundaries.
Pages are joined with a blank line; each page owns its leading separator so
``page_boundaries`` stay contiguous and index exactly into the returned text
(the ``search/pdf_highlighter`` contract).
Pages are joined with a blank line. Boundaries are kept CONTIGUOUS (each
page owns its leading ``\\n\\n`` separator) so they index exactly into the
returned text and ``boundaries[-1]["end_offset"] == len(text)`` -- the
``search/pdf_highlighter`` contract. Consequence: a page's range starts at
its separator, not its first glyph (the fast pypdfium2 path joins with no
separator, so its ranges are glyph-tight). The 2-char offset is immaterial
to page-level chunk attribution.
"""
sep = "\n\n"
parts: list[str] = []
@@ -137,8 +141,18 @@ def build_ocr_backend(settings: Settings) -> _OcrBackend | None:
GatewayTokenProvider,
)
assert settings.embedding_gateway_token_url is not None
assert settings.embedding_gateway_client_secret is not None
# Explicit (not assert -- assert is stripped under `python -O`): the
# M2M triple is all-or-nothing.
if not settings.embedding_gateway_token_url:
raise ValueError(
"EMBEDDING_GATEWAY_TOKEN_URL is required when "
"EMBEDDING_GATEWAY_CLIENT_ID is set"
)
if not settings.embedding_gateway_client_secret:
raise ValueError(
"EMBEDDING_GATEWAY_CLIENT_SECRET is required when "
"EMBEDDING_GATEWAY_CLIENT_ID is set"
)
token_provider = GatewayTokenProvider(
token_url=settings.embedding_gateway_token_url,
client_id=settings.embedding_gateway_client_id,
@@ -162,6 +176,15 @@ def build_ocr_backend(settings: Settings) -> _OcrBackend | None:
class OcrProcessor(DocumentProcessor):
"""Tier-3 OCR processor (gateway or direct Mistral backend)."""
def __init__(self) -> None:
# Resolve the backend once and reuse it: rebuilding per call would create
# a fresh GatewayTokenProvider each time (discarding its M2M-token cache
# -> a token fetch per document) and a new Mistral SDK client per call.
# A config change needs a pod restart anyway, so caching for the pod's
# lifetime is safe.
self._backend_resolved = False
self._backend: _OcrBackend | None = None
@property
def name(self) -> str:
return "ocr"
@@ -185,7 +208,10 @@ class OcrProcessor(DocumentProcessor):
) = None,
) -> ProcessingResult:
settings = get_settings()
backend = build_ocr_backend(settings)
if not self._backend_resolved:
self._backend = build_ocr_backend(settings)
self._backend_resolved = True
backend = self._backend
if backend is None:
logger.warning(
"OCR requested for %s but no backend is configured (provider=%s)",
nextcloud_mcp_server/document_processors/registry.py
+11 -1
View File
@@ -245,10 +245,16 @@ class ProcessorRegistry:
)
# Escalate scanned / no-text-layer PDFs to OCR (tier-3) when enabled and
# a provider is registered. The fast tier is terminal otherwise.
# 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.
if (
classification is not None
and classification.recommended_tier == "ocr"
and classification.page_count > 0
and settings.document_ocr_enabled
):
ocr = self._pdf_processor_for_tier("ocr")
@@ -359,6 +365,10 @@ class ProcessorRegistry:
raise
duration = time.time() - start_time
# Record the tier that actually produced this result so downstream
# (Qdrant payload pipeline_tier, analytics) reflects escalation
# instead of a hardcoded "fast".
result.metadata.setdefault("pipeline_tier", tier)
pages = int(result.metadata.get("page_count", 0) or 0)
chars = len(result.text)
status = "success" if result.success else "error"
nextcloud_mcp_server/vector/processor.py
+7 -3
View File
@@ -589,7 +589,7 @@ async def _index_document(
# Diagnostic: Log page boundary information if available
if "page_boundaries" in file_metadata:
page_boundaries = file_metadata["page_boundaries"]
logger.info(
logger.debug(
"Page boundaries for %s: %s pages, text length: %s",
file_path,
len(page_boundaries),
@@ -652,7 +652,7 @@ async def _index_document(
# Diagnostic: Verify page number assignment
assigned_count = sum(1 for c in chunks if c.page_number is not None)
logger.info(
logger.debug(
"Assigned page numbers to %s/%s chunks for %s",
assigned_count,
len(chunks),
@@ -926,7 +926,11 @@ async def _index_document(
# Decomposition payload keys (design §10.2), additive.
payload_keys.PROCESSOR_VERSION: "monolith-v1",
payload_keys.PARSED_AT: indexed_at,
payload_keys.PIPELINE_TIER: "fast",
# Actual tier that produced this doc (registry stamps it on
# the result metadata); non-PDF doc types stay "fast".
payload_keys.PIPELINE_TIER: file_metadata.get(
"pipeline_tier", "fast"
),
payload_keys.EMBEDDING_IDENTITY: _embedding_identity,
payload_keys.ACL_HASH: _acl_hash,
# File-specific metadata (PDF, etc.)
tests/unit/test_registry_tiering.py
+35 -7
View File
@@ -20,12 +20,18 @@ pytestmark = pytest.mark.unit
class _Fake(DocumentProcessor):
def __init__(
self, name: str, tier: str, text: str = "clean text here", success=True
self,
name: str,
tier: str,
text: str = "clean text here",
success=True,
pages: int = 1,
):
self._name = name
self._tier = tier
self._text = text
self._success = success
self._pages = pages
@property
def name(self) -> str:
@@ -42,14 +48,14 @@ class _Fake(DocumentProcessor):
async def process(
self, content, content_type, filename=None, options=None, progress_callback=None
):
boundaries = (
[{"page": 1, "start_offset": 0, "end_offset": len(self._text)}]
if self._pages
else []
)
return ProcessingResult(
text=self._text,
metadata={
"page_count": 1,
"page_boundaries": [
{"page": 1, "start_offset": 0, "end_offset": len(self._text)}
],
},
metadata={"page_count": self._pages, "page_boundaries": boundaries},
processor=self._name,
success=self._success,
)
@@ -117,6 +123,28 @@ async def test_ocr_escalation_on_empty_text(monkeypatch):
esc.assert_called_once()
async def test_zero_page_pdf_does_not_escalate(monkeypatch):
# An empty/corrupt PDF (no pages) classifies "ocr" but must NOT escalate --
# OCR can't help and it would be wasteful.
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="", pages=0), 20),
(_Fake("ocr", "ocr"), 5),
)
res = await r.process(b"%PDF-1.7", "application/pdf")
assert res.processor == "fast"
esc.assert_not_called()
async def test_pipeline_tier_stamped_on_metadata(monkeypatch):
monkeypatch.setattr(reg_mod, "get_settings", lambda: _Settings())
r = _registry((_Fake("fast", "fast"), 20))
res = await r.process(b"%PDF-1.7", "application/pdf")
assert res.metadata["pipeline_tier"] == "fast"
async def test_ocr_failure_falls_back_to_fast(monkeypatch):
# OCR enabled but the backend can't run (no creds / API down) -> keep the
# tier-1 result instead of failing the document.