From 820be135fb8fde53dc688d6d4f4688c19a80f9ba Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Fri, 5 Jun 2026 05:10:09 +0200 Subject: [PATCH] fix(review): unify "scanned" flag name + log image_coverage length drift Address PR #863 round 3: - classify_from_text emits the "scanned" flag (was "no_text_layer") for the empty-text-layer case -- same name + meaning as classify_pdf, so astrolabe_document_classifier_flag_total isn't split across two labels for the same concept (and matches the metric's documented vocab). - classify_from_text logs at DEBUG when image_coverage length != the expected min(pages, MAX_SAMPLED_PAGES), so a 1:1-alignment contract break (extractor reorders/skips pages) surfaces instead of silently misattributing coverage. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../document_processors/classifier.py | 21 ++++++++++++++++++- tests/unit/test_doc_classifier.py | 4 ++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/nextcloud_mcp_server/document_processors/classifier.py b/nextcloud_mcp_server/document_processors/classifier.py index 5d1c6035..3313d856 100644 --- a/nextcloud_mcp_server/document_processors/classifier.py +++ b/nextcloud_mcp_server/document_processors/classifier.py @@ -247,6 +247,22 @@ def classify_from_text( ``page_boundaries`` are ``{page, start_offset, end_offset}`` indexing into ``full_text``; ``image_coverage[i]`` (if given) aligns with the i-th boundary. """ + # image_coverage is expected to be one entry per page, capped at + # MAX_SAMPLED_PAGES (see image_coverage_per_page). Any other length means the + # 1:1 page alignment drifted (e.g. the extractor reordered/skipped pages) -- + # log it so a contract break surfaces rather than silently misattributing + # coverage to the wrong pages. + if image_coverage is not None: + expected = min(len(page_boundaries), MAX_SAMPLED_PAGES) + if len(image_coverage) != expected: + logger.debug( + "image_coverage length %s != expected %s for %s boundaries; " + "scan signal may be misaligned", + len(image_coverage), + expected, + len(page_boundaries), + ) + pages: list[PageSignals] = [] for idx, b in enumerate(page_boundaries): seg = full_text[b["start_offset"] : b["end_offset"]] @@ -283,7 +299,10 @@ def classify_from_text( flags: set[str] = set() if sampled and ocr_frac >= page_fraction: if total_chars == 0: - flags.add("no_text_layer") + # "scanned" (not "no_text_layer"): same name + meaning as classify_pdf + # so astrolabe_document_classifier_flag_total isn't split across two + # labels for the empty-text-layer case. + flags.add("scanned") elif mean_quality < min_text_quality: flags.add("bad_text_layer") if any(p.image_coverage >= IMAGE_COVERAGE_SCANNED for p in pages): diff --git a/tests/unit/test_doc_classifier.py b/tests/unit/test_doc_classifier.py index c810e1be..f81cf379 100644 --- a/tests/unit/test_doc_classifier.py +++ b/tests/unit/test_doc_classifier.py @@ -174,7 +174,7 @@ def test_classify_from_text_clean_routes_fast(): def test_classify_from_text_empty_routes_ocr(): c = clf.classify_from_text("", [{"page": 1, "start_offset": 0, "end_offset": 0}]) assert c.recommended_tier == "ocr" - assert "no_text_layer" in c.flags + assert "scanned" in c.flags # unified with classify_pdf's flag name assert c.total_chars == 0 @@ -202,7 +202,7 @@ def test_classify_from_text_junk_layer_flags_bad_text_layer(): 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 + assert "scanned" not in c.flags # has text, just junk -> not the empty case # --- quality + scan escalation triggers (Deck #207) --------------------------