fix(review): align quality threshold, cap + DRY scan coverage, warn on failure

Address PR #863 review:

- MIN_TEXT_QUALITY 0.45 -> 0.5 so the module/diagnostic default matches the
  DOCUMENT_OCR_MIN_TEXT_QUALITY setting (registry always passes the setting; this
  keeps classify_pdf and the test/default path on the production threshold).
- image_coverage_per_page is bounded to MAX_SAMPLED_PAGES (the image pass is the
  costly part, so a 200-page scan isn't fully rasterised on the hot path); pages
  beyond the cap fall back to the text-quality signal, and page_fraction still
  gates over every page.
- Extracted _page_image_coverage(page) helper, shared by classify_pdf and
  image_coverage_per_page (DRY + keeps the tiling-double-count note in one place).
- Scan-detection failure logs at WARNING (not DEBUG) so a systematic failure on
  an OCR-enabled tenant is visible at LOG_LEVEL=INFO.
- Add the missing DOCUMENT_OCR_MIN_PAGE_CHARS range-validator test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

nextcloud_mcp_server/document_processors/classifier.py

@@ -39,7 +39,10 @@ MAX_SAMPLED_PAGES = 24
 # A page counts as "scanned-like" when a raster image covers most of it.
 IMAGE_COVERAGE_SCANNED = 0.80
 # Text-quality score below which the layer is treated as junk (mashed tokens).
-MIN_TEXT_QUALITY = 0.45
+# Kept in sync with the DOCUMENT_OCR_MIN_TEXT_QUALITY setting default so the
+# module/diagnostic default matches production (the registry always passes the
+# setting).
+MIN_TEXT_QUALITY = 0.5
 # Fraction of sampled pages that must look scanned/bad for a doc->ocr verdict.
 OCR_PAGE_FRACTION = 0.5
 # A page with fewer extracted chars than this has effectively no text layer.
@@ -116,6 +119,21 @@ def _sample_indices(page_count: int) -> list[int]:
     )
 
 
+def _page_image_coverage(page: Any) -> float:
+    """Fraction of a pymupdf page covered by raster images, in ``[0, 1]``.
+
+    Approximate: an image placed multiple times (tiled backgrounds) is
+    double-counted, so the raw area can exceed the page -- the min() caps
+    coverage at 1.0, which is all the scanned/digital split needs.
+    """
+    page_area = abs(page.rect.width * page.rect.height) or 1.0
+    img_area = 0.0
+    for img in page.get_images(full=True):
+        for rect in page.get_image_rects(img[0]):
+            img_area += abs(rect.width * rect.height)
+    return min(img_area / page_area, 1.0)
+
+
 def classify_pdf(content: bytes) -> DocClassification:
     """Classify a PDF from its bytes.
 
@@ -133,15 +151,7 @@ def classify_pdf(content: bytes) -> DocClassification:
             page = doc.load_page(n)
             text = page.get_text("text")
             quality = _text_quality(text)
-            page_area = abs(page.rect.width * page.rect.height) or 1.0
-            img_area = 0.0
-            for img in page.get_images(full=True):
-                for rect in page.get_image_rects(img[0]):
-                    img_area += abs(rect.width * rect.height)
-            # Approximate: an image placed multiple times (tiled backgrounds) is
-            # double-counted, so img_area can exceed page_area -- the min() caps
-            # coverage at 1.0, which is all the scanned/digital split needs.
-            coverage = min(img_area / page_area, 1.0)
+            coverage = _page_image_coverage(page)
             # A page that is mostly a raster image is a scan/photo: its content
             # (handwriting, stamps, figure text) is not fully in any text layer,
             # so OCR is needed to capture it -- regardless of whether a partial
@@ -197,20 +207,20 @@ def image_coverage_per_page(content: bytes) -> list[float]:
     Lets the hot path flag scanned pages whose embedded text layer is junk but
     statistically clean-looking. Re-opens the PDF, so the registry calls it only
     when OCR + scan detection are enabled (the cost is borne by OCR-opted-in
-    tenants). Returned list is aligned by index with the page boundaries.
+    tenants). Returned list is aligned by index with the leading page boundaries.
+
+    Bounded to the first ``MAX_SAMPLED_PAGES`` pages -- the image pass is the
+    costly part, so a 200-page scan isn't fully rasterised on the hot path. Pages
+    beyond the cap fall back to the text-quality signal in ``classify_from_text``
+    (a scanned tail has junk text too), and ``page_fraction`` still gates over
+    every page.
     """
     import pymupdf  # noqa: PLC0415 -- keep the heavy import lazy
 
     cov: list[float] = []
     with pymupdf.open("pdf", content) as doc:
-        for n in range(doc.page_count):
-            page = doc.load_page(n)
-            page_area = abs(page.rect.width * page.rect.height) or 1.0
-            img_area = 0.0
-            for img in page.get_images(full=True):
-                for rect in page.get_image_rects(img[0]):
-                    img_area += abs(rect.width * rect.height)
-            cov.append(min(img_area / page_area, 1.0))
+        for n in range(min(doc.page_count, MAX_SAMPLED_PAGES)):
+            cov.append(_page_image_coverage(doc.load_page(n)))
     return cov
 
 

nextcloud_mcp_server/document_processors/registry.py

@@ -248,7 +248,10 @@ class ProcessorRegistry:
                     try:
                         image_coverage = image_coverage_per_page(content)
                     except Exception:
-                        logger.debug(
+                        # Best-effort: fall back to text-only signals. WARNING
+                        # (not DEBUG) so a systematic scan-detection failure on an
+                        # OCR-enabled tenant is visible at LOG_LEVEL=INFO.
+                        logger.warning(
                             "Scan detection failed for %s; using text-only signals",
                             filename or "<bytes>",
                             exc_info=True,

tests/unit/test_config.py

@@ -397,6 +397,14 @@ class TestDynaconfValidators:
         with pytest.raises(ValidationError, match="DOCUMENT_OCR_PAGE_FRACTION"):
             _reload_config()
 
+    @patch.dict(os.environ, {"DOCUMENT_OCR_MIN_PAGE_CHARS": "-1"}, clear=True)
+    def test_ocr_min_page_chars_negative(self):
+        """DOCUMENT_OCR_MIN_PAGE_CHARS must be non-negative."""
+        from dynaconf import ValidationError
+
+        with pytest.raises(ValidationError, match="DOCUMENT_OCR_MIN_PAGE_CHARS"):
+            _reload_config()
+
     @patch.dict(os.environ, {"LOG_LEVEL": "VERBOSE"}, clear=True)
     def test_invalid_log_level(self):
         """Test invalid LOG_LEVEL raises ValidationError."""