From 0347e96679c3b35934ebb27b60b0820acddc2695 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Fri, 5 Jun 2026 00:12:17 +0200 Subject: [PATCH] fix(review): sample last page, document flags-vs-routing, add flag-path tests Address PR #855 review (all non-blocking): - classifier: _sample_indices now always includes the first AND last page (the old evenly-spaced sample missed the tail, e.g. last sampled index 95 on a 100-page doc -- a scanned tail could be missed). - classifier + metrics: document that flags are diagnostic and fire independently of routing (image_heavy on ANY page vs the ocr route needing a page FRACTION), so flag{image_heavy} is expected to exceed classified{ocr}. - classifier: clarify the text-quality whitespace comment (caps at 12%) and note the image double-count approximation (min() caps coverage). - tests: add the scanned (no text layer) and bad_text_layer (junk text over an image) flag paths, and a test pinning first/last-page sampling. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../document_processors/classifier.py | 21 ++++++++-- nextcloud_mcp_server/observability/metrics.py | 3 ++ tests/unit/test_doc_classifier.py | 42 +++++++++++++++++++ 3 files changed, 62 insertions(+), 4 deletions(-) diff --git a/nextcloud_mcp_server/document_processors/classifier.py b/nextcloud_mcp_server/document_processors/classifier.py index 22e34d7b..e31be914 100644 --- a/nextcloud_mcp_server/document_processors/classifier.py +++ b/nextcloud_mcp_server/document_processors/classifier.py @@ -82,7 +82,8 @@ def _text_quality(text: str) -> float: whitespace_ratio = sum(c.isspace() for c in text) / len(text) mean_token_len = sum(len(t) for t in tokens) / len(tokens) overlong_frac = sum(len(t) > 20 for t in tokens) / len(tokens) - # Clean prose: ~15-20% whitespace, mean token ~4-6 chars, ~no overlong tokens. + # Caps at 1.0 from 12% whitespace (conservative; clean prose runs 15-20%), + # mean token ~4-6 chars, ~no overlong tokens. ws_score = min(whitespace_ratio / 0.12, 1.0) len_score = ( 1.0 if mean_token_len <= 10 else max(0.0, 1.0 - (mean_token_len - 10) / 15) @@ -94,9 +95,13 @@ def _text_quality(text: str) -> float: def _sample_indices(page_count: int) -> list[int]: if page_count <= MAX_SAMPLED_PAGES: return list(range(page_count)) - # Evenly spaced sample across the document. - step = page_count / MAX_SAMPLED_PAGES - return sorted({int(i * step) for i in range(MAX_SAMPLED_PAGES)}) + # Evenly spaced sample that always includes the first AND last page, so a + # scanned tail on an otherwise-digital doc isn't missed. Rounding collisions + # just yield a slightly smaller (still bounded) sample. + last = page_count - 1 + return sorted( + {round(i * last / (MAX_SAMPLED_PAGES - 1)) for i in range(MAX_SAMPLED_PAGES)} + ) def classify_pdf(content: bytes) -> DocClassification: @@ -122,6 +127,9 @@ def classify_pdf(content: bytes) -> DocClassification: 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) # 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, @@ -143,6 +151,11 @@ def classify_pdf(content: bytes) -> DocClassification: ) ocr_frac = (sum(p.needs_ocr for p in pages) / sampled) if sampled else 0.0 + # Flags are diagnostic signals, intentionally independent of the routing + # verdict: image_heavy fires if ANY page is image-heavy, while the OCR route + # needs a FRACTION of pages (OCR_PAGE_FRACTION). So a mostly-digital doc with + # one full-page photo is flagged image_heavy yet still routes "fast" -- the + # flag_total{image_heavy} count is expected to exceed classified{ocr}. flags: set[str] = set() if any(p.image_coverage >= IMAGE_COVERAGE_SCANNED for p in pages): flags.add("image_heavy") diff --git a/nextcloud_mcp_server/observability/metrics.py b/nextcloud_mcp_server/observability/metrics.py index 2ed3c5a1..7afc87f0 100644 --- a/nextcloud_mcp_server/observability/metrics.py +++ b/nextcloud_mcp_server/observability/metrics.py @@ -285,6 +285,9 @@ document_classified_total = Counter( ) document_classifier_flag_total = Counter( + # Diagnostic flags, independent of the routing verdict: image_heavy fires if + # ANY page is image-heavy whereas the ocr route needs a fraction of pages, + # so flag{image_heavy} is expected to exceed classified{recommended_tier=ocr}. "astrolabe_document_classifier_flag_total", "Tier-0 classifier flags raised on documents", ["flag"], # image_heavy | scanned | bad_text_layer diff --git a/tests/unit/test_doc_classifier.py b/tests/unit/test_doc_classifier.py index 24bacb0f..9460c708 100644 --- a/tests/unit/test_doc_classifier.py +++ b/tests/unit/test_doc_classifier.py @@ -84,3 +84,45 @@ def test_large_doc_is_sampled(): c = clf.classify_pdf(_digital_pdf(pages=120)) assert c.page_count == 120 assert c.sampled_pages <= clf.MAX_SAMPLED_PAGES + + +def test_sample_indices_includes_first_and_last_page(): + idx = clf._sample_indices(100) + assert idx[0] == 0 + assert idx[-1] == 99 # last page must be sampled (scanned-tail case) + assert len(idx) <= clf.MAX_SAMPLED_PAGES + + +# --- flag paths -------------------------------------------------------------- + + +def _image_with_mashed_text_pdf(pages: int = 2) -> bytes: + # Full-page image with a junk (mashed/space-less) text layer over it -- a + # scan whose OCR'd text layer is unusable. + doc = pymupdf.open() + pix = pymupdf.Pixmap(pymupdf.csRGB, pymupdf.IRect(0, 0, 600, 850)) + pix.clear_with(255) + img = pix.tobytes("png") + mashed = "01322234567mobileoutstandingresilienceacademicachievement " * 3 + for _ in range(pages): + page = doc.new_page(width=595, height=842) + page.insert_image(page.rect, stream=img) + page.insert_text((50, 60), mashed) + data: bytes = doc.tobytes() + doc.close() + return data + + +def test_scanned_flag_when_no_text_layer(): + c = clf.classify_pdf(_full_page_image_pdf()) + assert c.total_chars == 0 + assert "scanned" in c.flags + assert c.recommended_tier == "ocr" + + +def test_bad_text_layer_flag_on_image_with_junk_text(): + c = clf.classify_pdf(_image_with_mashed_text_pdf()) + assert c.total_chars > 0 + assert c.mean_text_quality < clf.MIN_TEXT_QUALITY + assert "bad_text_layer" in c.flags + assert c.recommended_tier == "ocr"