From 9ffe0645b87aecbf85034c5e1d3084809182a361 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Fri, 5 Jun 2026 03:21:10 +0200 Subject: [PATCH] fix: close pypdfium2 page handle on error + cover classifier/OCR edge cases Follow-up to the tiered document processor (#858), landing the round-4 review nits the reviewer approved without: - pypdfium2_fast: free the page handle in an outer finally so a corrupt page that makes get_textpage() raise can't orphan it. - test: classify_from_text junk-text-layer path (non-zero chars, low quality, high ocr_frac) flags bad_text_layer -- the hot-path coverage gap. - test: build_ocr_backend raises ValueError when the gateway M2M client_id is set without its token_url/secret. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../document_processors/pypdfium2_fast.py | 10 +++++++--- tests/unit/test_doc_classifier.py | 17 +++++++++++++++++ tests/unit/test_ocr_processor.py | 13 +++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/nextcloud_mcp_server/document_processors/pypdfium2_fast.py b/nextcloud_mcp_server/document_processors/pypdfium2_fast.py index 2096d97e..52f64472 100644 --- a/nextcloud_mcp_server/document_processors/pypdfium2_fast.py +++ b/nextcloud_mcp_server/document_processors/pypdfium2_fast.py @@ -40,11 +40,15 @@ def _extract(content: bytes) -> tuple[str, dict[str, Any]]: page_texts: list[str] = [] for i in range(len(pdf)): page = pdf[i] - textpage = page.get_textpage() try: - page_texts.append(textpage.get_text_bounded() or "") + textpage = page.get_textpage() + try: + page_texts.append(textpage.get_text_bounded() or "") + finally: + textpage.close() finally: - textpage.close() + # Outer finally so the page handle is freed even if + # get_textpage() raises on a corrupt page. page.close() doc_meta = pdf.get_metadata_dict() or {} finally: diff --git a/tests/unit/test_doc_classifier.py b/tests/unit/test_doc_classifier.py index bc5306d1..587e1caa 100644 --- a/tests/unit/test_doc_classifier.py +++ b/tests/unit/test_doc_classifier.py @@ -185,3 +185,20 @@ def test_classify_from_text_no_pages_routes_fast(): assert c.recommended_tier == "fast" assert c.ocr_page_fraction == pytest.approx(0.0) assert c.flags == set() + + +def test_classify_from_text_junk_layer_flags_bad_text_layer(): + # Non-zero chars but low quality on every page (high ocr_frac) -> ocr + + # bad_text_layer (gated on ocr_frac, matching classify_pdf). + text = "x1y2zx1y2z" + c = clf.classify_from_text( + text, + [ + {"page": 1, "start_offset": 0, "end_offset": 5}, + {"page": 2, "start_offset": 5, "end_offset": 10}, + ], + ) + assert c.recommended_tier == "ocr" + assert c.total_chars > 0 + assert "bad_text_layer" in c.flags + assert "no_text_layer" not in c.flags diff --git a/tests/unit/test_ocr_processor.py b/tests/unit/test_ocr_processor.py index 07883027..690f0c38 100644 --- a/tests/unit/test_ocr_processor.py +++ b/tests/unit/test_ocr_processor.py @@ -71,6 +71,19 @@ def test_build_backend_auto_none_configured(): assert ocr.build_ocr_backend(_settings()) is None +def test_build_backend_gateway_missing_m2m_raises(): + # client_id set but token_url/secret missing -> explicit ValueError (not a + # stripped assert), surfaced on backend resolution. + with pytest.raises(ValueError, match="EMBEDDING_GATEWAY_TOKEN_URL"): + ocr.build_ocr_backend( + _settings( + document_ocr_provider="gateway", + embedding_gateway_url="http://gw", + embedding_gateway_client_id="cid", + ) + ) + + def test_gateway_backend_url_normalization(): b = ocr._GatewayOcrBackend("http://gw", "mistral/mistral-ocr-latest") assert b._url == "http://gw/v1/ocr"