From 4bdb0bc6d61a83160dd3686b6a6e9ea5c71b1de0 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Fri, 5 Jun 2026 00:22:17 +0200 Subject: [PATCH] fix(review): warn (not debug) on shadow-classify failure; tidy pymupdf usage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address PR #855 round 2: - 🔴 _shadow_classify swallowed all exceptions at DEBUG, so a systematic failure (pymupdf bug, memory pressure) is invisible at LOG_LEVEL=INFO and trips SonarQube S2221/S5754. Log at WARNING instead (still best-effort -- indexing is unaffected). - classifier: use `with pymupdf.open(...) as doc` instead of manual try/finally. - tests: release the Pixmap's native memory (del pix) in the image fixtures. Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/document_processors/classifier.py | 5 +---- nextcloud_mcp_server/vector/processor.py | 9 ++++++++- tests/unit/test_doc_classifier.py | 2 ++ 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/nextcloud_mcp_server/document_processors/classifier.py b/nextcloud_mcp_server/document_processors/classifier.py index e31be914..f4883ae1 100644 --- a/nextcloud_mcp_server/document_processors/classifier.py +++ b/nextcloud_mcp_server/document_processors/classifier.py @@ -113,8 +113,7 @@ def classify_pdf(content: bytes) -> DocClassification: """ import pymupdf # noqa: PLC0415 -- keep the heavy import lazy / off module load - doc = pymupdf.open("pdf", content) - try: + with pymupdf.open("pdf", content) as doc: page_count = doc.page_count indices = _sample_indices(page_count) pages: list[PageSignals] = [] @@ -141,8 +140,6 @@ def classify_pdf(content: bytes) -> DocClassification: pages.append( PageSignals(n, len(text), round(coverage, 3), quality, needs_ocr) ) - finally: - doc.close() sampled = len(pages) total_chars = sum(p.char_count for p in pages) diff --git a/nextcloud_mcp_server/vector/processor.py b/nextcloud_mcp_server/vector/processor.py index f53a7418..8885da79 100644 --- a/nextcloud_mcp_server/vector/processor.py +++ b/nextcloud_mcp_server/vector/processor.py @@ -186,7 +186,14 @@ async def _shadow_classify(content: bytes, content_type: str, file_path: str) -> c.mean_text_quality, ) except Exception: - logger.debug("Tier-0 classification failed for %s", file_path, exc_info=True) + # Best-effort: shadow classification must never break indexing, but log + # at WARNING (not DEBUG) so a systematic failure -- a pymupdf bug, memory + # pressure on every PDF -- stays visible at the production LOG_LEVEL=INFO. + logger.warning( + "Tier-0 classification failed for %s (shadow mode, indexing unaffected)", + file_path, + exc_info=True, + ) async def process_document( diff --git a/tests/unit/test_doc_classifier.py b/tests/unit/test_doc_classifier.py index 9460c708..cd2adc1d 100644 --- a/tests/unit/test_doc_classifier.py +++ b/tests/unit/test_doc_classifier.py @@ -34,6 +34,7 @@ def _full_page_image_pdf(pages: int = 2) -> bytes: pix = pymupdf.Pixmap(pymupdf.csRGB, pymupdf.IRect(0, 0, 600, 850)) pix.clear_with(255) img = pix.tobytes("png") + del pix # Pixmap holds native memory; release it before the loop for _ in range(pages): page = doc.new_page(width=595, height=842) page.insert_image(page.rect, stream=img) @@ -103,6 +104,7 @@ def _image_with_mashed_text_pdf(pages: int = 2) -> bytes: pix = pymupdf.Pixmap(pymupdf.csRGB, pymupdf.IRect(0, 0, 600, 850)) pix.clear_with(255) img = pix.tobytes("png") + del pix # Pixmap holds native memory; release it before the loop mashed = "01322234567mobileoutstandingresilienceacademicachievement " * 3 for _ in range(pages): page = doc.new_page(width=595, height=842)