fix(ingest): address review round 2 (Literal reason + exhaustive branch + test)

- escalation: EscalationDecision.reason is now Literal["empty_text",
  "low_confidence"] (parity with kind; ty catches a bad label at call sites).
- processor: nest the decision handling so the hop branch is reached via an
  explicit else under `if decision is not None` — exhaustive over the Literal
  kind, no None-attribute risk.
- tests: add the "OCR processor unregistered (not just disabled) → None"
  quadrant, locking in absent != suppressed.

Deck #324.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Chris Coutinho
2026-06-13 15:24:55 +02:00
co-authored by Claude Opus 4.8
parent f8e8645fc2
commit c0fd7dd67b
3 changed files with 46 additions and 28 deletions
@@ -49,7 +49,7 @@ class EscalationDecision:
kind: Literal["hop", "suppressed"] kind: Literal["hop", "suppressed"]
to_tier: str to_tier: str
reason: str # empty_text | low_confidence reason: Literal["empty_text", "low_confidence"]
def next_tier(current: str) -> str | None: def next_tier(current: str) -> str | None:
+29 -27
View File
@@ -157,33 +157,35 @@ async def _parse_pdf_tier(
decision = registry.evaluate_escalation( decision = registry.evaluate_escalation(
result, content, tier, settings, filename=filename result, content, tier, settings, filename=filename
) )
if decision is not None and decision.kind == "suppressed": if decision is not None:
# The ideal next tier (e.g. ocr) is disabled, so we do NOT hop: index if decision.kind == "suppressed":
# this tier's output as terminal and record the would-be escalation # The ideal next tier (e.g. ocr) is disabled, so we do NOT hop:
# so operators see the latent demand ("what-if OCR enabled"; #324). # index this tier's output as terminal and record the would-be
record_document_escalation_suppressed( # escalation so operators see the latent demand ("what-if OCR
tier, decision.to_tier, decision.reason # enabled"; #324).
) record_document_escalation_suppressed(
logger.info( tier, decision.to_tier, decision.reason
"Escalation suppressed for %s: %s->%s disabled (reason=%s), " )
"indexing at current tier", logger.info(
filename or "<bytes>", "Escalation suppressed for %s: %s->%s disabled (reason=%s), "
tier, "indexing at current tier",
decision.to_tier, filename or "<bytes>",
decision.reason, tier,
) decision.to_tier,
elif decision is not None: decision.reason,
record_document_escalation(tier, decision.to_tier, decision.reason) )
logger.info( else: # "hop" — the Literal kind makes this branch exhaustive.
"Escalating %s %s->%s (reason=%s)", record_document_escalation(tier, decision.to_tier, decision.reason)
filename or "<bytes>", logger.info(
tier, "Escalating %s %s->%s (reason=%s)",
decision.to_tier, filename or "<bytes>",
decision.reason, tier,
) decision.to_tier,
raise EscalateError( decision.reason,
from_tier=tier, to_tier=decision.to_tier, reason=decision.reason )
) raise EscalateError(
from_tier=tier, to_tier=decision.to_tier, reason=decision.reason
)
return result return result
+16
View File
@@ -482,3 +482,19 @@ def test_evaluate_escalation_structured_hop_not_suppressed_when_ocr_off(monkeypa
) )
decision = r.evaluate_escalation(res, b"%PDF", "fast", _Settings(ocr=False)) decision = r.evaluate_escalation(res, b"%PDF", "fast", _Settings(ocr=False))
assert decision == EscalationDecision("hop", "structured", "low_confidence") assert decision == EscalationDecision("hop", "structured", "low_confidence")
def test_evaluate_escalation_terminal_when_ocr_unregistered_and_off(monkeypatch):
"""No OCR processor registered at all (not merely disabled) → genuinely
terminal: returns None, NOT a suppressed decision. 'Absent' != 'disabled'."""
monkeypatch.setattr(reg_mod, "record_document_classification", MagicMock())
r = _registry((_Fake("fast", "fast"), 20)) # only fast; no ocr processor
res = ProcessingResult(
text="",
metadata={
"page_count": 1,
"page_boundaries": [{"page": 1, "start_offset": 0, "end_offset": 0}],
},
processor="fast",
)
assert r.evaluate_escalation(res, b"%PDF", "fast", _Settings(ocr=False)) is None