fix(document-processors): correct cascade escalation metric + review nits
Address round-1 review on PR #914: - Attribute the OCR hop in a fast->structured->ocr inline cascade to from_tier="structured" (not a second "fast" escalation), so astrolabe_document_escalation_total per-tier counts stay accurate. - Add test_inline_fast_structured_ocr_cascade pinning that two-hop path and the metric attribution. - Note in classify_from_text that its doc-level control ratio is over full_text (all pages), not the sampled subset classify_pdf uses. - Clarify that corrupt_glyphs never lands in the suppressed-escalation counter. - Dedupe the glyph-corrupt test string into tests/fixtures/glyph_corruption.py. - Use pytest.approx for the control-char-ratio zero checks (SonarCloud S1244). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
cf7209cd85
commit
d5286e39d6
@@ -407,9 +407,12 @@ def classify_from_text(
|
|||||||
# page_count guard also skips escalation; defaulting to 0.0 keeps the
|
# page_count guard also skips escalation; defaulting to 0.0 keeps the
|
||||||
# recorded classification metric accurate rather than a misleading "ocr").
|
# recorded classification metric accurate rather than a misleading "ocr").
|
||||||
ocr_frac = (sum(p.needs_ocr for p in pages) / sampled) if sampled else 0.0
|
ocr_frac = (sum(p.needs_ocr for p in pages) / sampled) if sampled else 0.0
|
||||||
# Doc-level (char-weighted) control-char ratio -- the glyph-leak signal that
|
# Doc-level control-char ratio -- the glyph-leak signal that routes to the
|
||||||
# routes to the structured tier. Computed over full_text so it is robust to
|
# structured tier. Computed over the WHOLE full_text (all pages), unlike
|
||||||
# boundary edge cases.
|
# classify_pdf which char-weights the up-to-MAX_SAMPLED_PAGES sample; the two
|
||||||
|
# are therefore not numerically identical for a >24-page doc with corruption
|
||||||
|
# concentrated outside the sample. full_text is used here because it is exactly
|
||||||
|
# the text that gets chunked + indexed and is robust to boundary edge cases.
|
||||||
control_ratio = _control_char_ratio(full_text)
|
control_ratio = _control_char_ratio(full_text)
|
||||||
|
|
||||||
flags, recommended = _route_from_signals(
|
flags, recommended = _route_from_signals(
|
||||||
|
|||||||
@@ -247,6 +247,12 @@ class ProcessorRegistry:
|
|||||||
result, content, settings, record=True, filename=filename
|
result, content, settings, record=True, filename=filename
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# The tier whose output produced the current ``classification`` -- used as
|
||||||
|
# ``from_tier`` for a subsequent OCR hop so a fast->structured->ocr cascade
|
||||||
|
# is attributed correctly (the OCR hop is from ``structured``, not a second
|
||||||
|
# ``fast`` escalation).
|
||||||
|
from_tier = "fast"
|
||||||
|
|
||||||
# Escalate a poor fast extraction up the ladder (fast -> structured -> ocr),
|
# Escalate a poor fast extraction up the ladder (fast -> structured -> ocr),
|
||||||
# mirroring the external per-tier path so both modes behave identically. A
|
# mirroring the external per-tier path so both modes behave identically. A
|
||||||
# glyph-corrupt layer (the extractor leaked raw glyph codes -- the
|
# glyph-corrupt layer (the extractor leaked raw glyph codes -- the
|
||||||
@@ -293,6 +299,7 @@ class ProcessorRegistry:
|
|||||||
)
|
)
|
||||||
if structured_result.success:
|
if structured_result.success:
|
||||||
result = structured_result
|
result = structured_result
|
||||||
|
from_tier = "structured"
|
||||||
classification = self._classify_result(
|
classification = self._classify_result(
|
||||||
result, content, settings, record=False, filename=filename
|
result, content, settings, record=False, filename=filename
|
||||||
)
|
)
|
||||||
@@ -331,10 +338,11 @@ class ProcessorRegistry:
|
|||||||
if classification.total_chars == 0
|
if classification.total_chars == 0
|
||||||
else "low_confidence"
|
else "low_confidence"
|
||||||
)
|
)
|
||||||
record_document_escalation("fast", "ocr", reason)
|
record_document_escalation(from_tier, "ocr", reason)
|
||||||
logger.info(
|
logger.info(
|
||||||
"Escalating %s fast->ocr (reason=%s)",
|
"Escalating %s %s->ocr (reason=%s)",
|
||||||
filename or "<bytes>",
|
filename or "<bytes>",
|
||||||
|
from_tier,
|
||||||
reason,
|
reason,
|
||||||
)
|
)
|
||||||
ocr_result = await self._run_processor(
|
ocr_result = await self._run_processor(
|
||||||
|
|||||||
@@ -287,7 +287,9 @@ document_escalation_total = Counter(
|
|||||||
document_escalation_suppressed_total = Counter(
|
document_escalation_suppressed_total = Counter(
|
||||||
"astrolabe_document_escalation_suppressed_total",
|
"astrolabe_document_escalation_suppressed_total",
|
||||||
"Would-be tier escalations suppressed because the target tier is disabled",
|
"Would-be tier escalations suppressed because the target tier is disabled",
|
||||||
# reason: low_confidence | empty_text
|
# reason: low_confidence | empty_text. (corrupt_glyphs never appears here: it
|
||||||
|
# targets the structured tier, which has no enabled-gate -- if registered it
|
||||||
|
# runs, else there is nothing to suppress -- so it only ever hops or returns.)
|
||||||
["from_tier", "to_tier", "reason"],
|
["from_tier", "to_tier", "reason"],
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
Vendored
+11
@@ -0,0 +1,11 @@
|
|||||||
|
"""Shared test data for the tier-0 glyph-corruption signal.
|
||||||
|
|
||||||
|
A fast-tier text layer that looks like words -- normal spacing and token lengths,
|
||||||
|
so it scores HIGH on ``_text_quality`` -- but leaks C0 control characters: the
|
||||||
|
broken-/ToUnicode signature that ``classifier._control_char_ratio`` catches. The
|
||||||
|
alphabetic tokens decode to a pangram under a -3 (Caesar) shift.
|
||||||
|
|
||||||
|
Kept in one place so the classifier and registry tiering tests can't diverge.
|
||||||
|
"""
|
||||||
|
|
||||||
|
GLYPH_CORRUPT_TEXT = "WKH \x0f TXLFN \x10 EURZQ \x11 IRA MXPSV \x0f RYHU \x10 GRJ " * 6
|
||||||
@@ -14,6 +14,7 @@ import pymupdf
|
|||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from nextcloud_mcp_server.document_processors import classifier as clf
|
from nextcloud_mcp_server.document_processors import classifier as clf
|
||||||
|
from tests.fixtures.glyph_corruption import GLYPH_CORRUPT_TEXT
|
||||||
|
|
||||||
pytestmark = pytest.mark.unit
|
pytestmark = pytest.mark.unit
|
||||||
|
|
||||||
@@ -324,17 +325,17 @@ def test_scan_coverage_shorter_than_pages_aligns_without_crash():
|
|||||||
|
|
||||||
# --- glyph-corruption signal (broken /ToUnicode -> structured escalation) -----
|
# --- glyph-corruption signal (broken /ToUnicode -> structured escalation) -----
|
||||||
|
|
||||||
# pypdfium2-style leak: a uniform glyph/Caesar offset turns clean prose into
|
# A uniform glyph/Caesar offset turns clean prose into alphabetic-but-wrong tokens
|
||||||
# alphabetic-but-wrong tokens (normal spacing + token length => HIGH text_quality)
|
# (normal spacing + token length => HIGH text_quality) while digits/punctuation map
|
||||||
# while digits/punctuation map to C0 control bytes. The control-char ratio is the
|
# to C0 control bytes. The control-char ratio is the only signal that catches this;
|
||||||
# only signal that catches this; _text_quality scores it ~1.0.
|
# _text_quality scores it ~1.0. Shared with the registry tiering tests.
|
||||||
_GLYPH_CORRUPT = "WKH \x0f TXLFN \x10 EURZQ \x11 IRA MXPSV \x0f RYHU \x10 GRJ " * 6
|
_GLYPH_CORRUPT = GLYPH_CORRUPT_TEXT
|
||||||
|
|
||||||
|
|
||||||
def test_control_char_ratio_clean_is_zero():
|
def test_control_char_ratio_clean_is_zero():
|
||||||
assert clf._control_char_ratio("the quick brown fox") == 0.0
|
assert clf._control_char_ratio("the quick brown fox") == pytest.approx(0.0)
|
||||||
# legitimate whitespace controls (tab/newline/CR/form-feed/vtab) don't count
|
# legitimate whitespace controls (tab/newline/CR/form-feed/vtab) don't count
|
||||||
assert clf._control_char_ratio("a\tb\nc\r\nd\f\ve") == 0.0
|
assert clf._control_char_ratio("a\tb\nc\r\nd\f\ve") == pytest.approx(0.0)
|
||||||
|
|
||||||
|
|
||||||
def test_control_char_ratio_detects_glyph_leak():
|
def test_control_char_ratio_detects_glyph_leak():
|
||||||
|
|||||||
@@ -4,7 +4,7 @@ Covers: default fast-tier routing, the pymupdf rollback toggle, classification
|
|||||||
recording derived from the extraction, and OCR escalation (on/off).
|
recording derived from the extraction, and OCR escalation (on/off).
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from unittest.mock import MagicMock
|
from unittest.mock import MagicMock, call
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
@@ -15,6 +15,7 @@ from nextcloud_mcp_server.document_processors.base import (
|
|||||||
)
|
)
|
||||||
from nextcloud_mcp_server.document_processors.escalation import EscalationDecision
|
from nextcloud_mcp_server.document_processors.escalation import EscalationDecision
|
||||||
from nextcloud_mcp_server.document_processors.registry import ProcessorRegistry
|
from nextcloud_mcp_server.document_processors.registry import ProcessorRegistry
|
||||||
|
from tests.fixtures.glyph_corruption import GLYPH_CORRUPT_TEXT
|
||||||
|
|
||||||
pytestmark = pytest.mark.unit
|
pytestmark = pytest.mark.unit
|
||||||
|
|
||||||
@@ -250,10 +251,10 @@ async def test_no_ocr_escalation_when_disabled(monkeypatch):
|
|||||||
|
|
||||||
# --- glyph-corruption escalation + full-ladder parity ------------------------
|
# --- glyph-corruption escalation + full-ladder parity ------------------------
|
||||||
|
|
||||||
# A fast-tier text layer that looks like words (normal spacing/token lengths ->
|
# A fast-tier text layer that looks like words (HIGH text_quality) but leaks C0
|
||||||
# HIGH text_quality) but leaks C0 control chars: the broken-/ToUnicode signature
|
# control chars -- the broken-/ToUnicode signature the control-char ratio catches.
|
||||||
# the control-char ratio catches. Decodes to a pangram under a -3 shift.
|
# Shared with the classifier tests so the two can't diverge.
|
||||||
_GLYPH = "WKH \x0f TXLFN \x10 EURZQ \x11 IRA MXPSV \x0f RYHU \x10 GRJ " * 6
|
_GLYPH = GLYPH_CORRUPT_TEXT
|
||||||
|
|
||||||
|
|
||||||
async def test_glyph_corrupt_escalates_fast_to_structured(monkeypatch):
|
async def test_glyph_corrupt_escalates_fast_to_structured(monkeypatch):
|
||||||
@@ -311,6 +312,27 @@ async def test_inline_empty_skips_structured_straight_to_ocr(monkeypatch):
|
|||||||
esc.assert_called_once_with("fast", "ocr", "empty_text")
|
esc.assert_called_once_with("fast", "ocr", "empty_text")
|
||||||
|
|
||||||
|
|
||||||
|
async def test_inline_fast_structured_ocr_cascade(monkeypatch):
|
||||||
|
# Full cascade: a junk-but-non-empty fast layer hops to structured, the
|
||||||
|
# structured re-extract is empty (a doc that was ALSO scanned), so it then
|
||||||
|
# hops to OCR. The second hop must be attributed from_tier="structured",
|
||||||
|
# NOT a second "fast" escalation.
|
||||||
|
monkeypatch.setattr(reg_mod, "get_settings", lambda: _Settings(ocr=True))
|
||||||
|
esc = MagicMock()
|
||||||
|
monkeypatch.setattr(reg_mod, "record_document_escalation", esc)
|
||||||
|
r = _registry(
|
||||||
|
(_Fake("fast", "fast", text="x" * 40), 20), # quality ~0, non-empty
|
||||||
|
(_Fake("structured", "structured", text=""), 10), # re-extract empty
|
||||||
|
(_Fake("ocr", "ocr", text="ocr recovered text"), 5),
|
||||||
|
)
|
||||||
|
res = await r.process(b"%PDF-1.7", "application/pdf")
|
||||||
|
assert res.processor == "ocr"
|
||||||
|
assert esc.call_args_list == [
|
||||||
|
call("fast", "structured", "low_confidence"),
|
||||||
|
call("structured", "ocr", "empty_text"),
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
def test_evaluate_escalation_glyph_corrupt_goes_structured(monkeypatch):
|
def test_evaluate_escalation_glyph_corrupt_goes_structured(monkeypatch):
|
||||||
# External path mirrors the inline path: glyph-corrupt -> structured, never OCR.
|
# External path mirrors the inline path: glyph-corrupt -> structured, never OCR.
|
||||||
monkeypatch.setattr(reg_mod, "record_document_classification", MagicMock())
|
monkeypatch.setattr(reg_mod, "record_document_classification", MagicMock())
|
||||||
|
|||||||
Reference in New Issue
Block a user