perf(image-evidence): collapse Stage C OCR attempt ladder from 3 tiers to 2 (issue #677) (re-land of #698)

.github/scripts/check_extractor_ownership_totality.py

@@ -50,16 +50,13 @@
 
 DELIBERATE EXCLUSIONS, and why they are not silent
 ----------------------------------------------------
-`EXCLUDED_HELPERS = {"_collector_line_ocr_attempts"}` — the OCR
-attempt-tier ladder (issue #259). Named out of scope by this PR's own brief
-("being worked in parallel by other branches"); touching its ownership here
-would either fight that branch's own edits or require this script to freeze
-a design that branch is actively changing. Its own body is excluded from
-the call-scan too (not just the function name itself), so a name ONLY
-called from inside it (`preprocess_fallback_variants`, at this writing) is
-correctly not required to have an entry either — it is reachable exclusively
-through the excluded ladder, and re-included in full the day that ladder's
-own PR lands and this exclusion is lifted.
+The OCR attempt-tier ladder (`_collector_line_ocr_attempts`, issue #259) was
+excluded here (`EXCLUDED_HELPERS = {"_collector_line_ocr_attempts"}`) while a
+parallel branch was actively restructuring it — that branch was issue #677
+("collapse the Stage C OCR attempt ladder"), which has now landed, so the
+exclusion is LIFTED (`EXCLUDED_HELPERS = frozenset()`): the ladder function
+and everything reachable only through it (`preprocess_fallback_variants`)
+now carry real `EXTRACTOR_OWNERSHIP` entries like every other contributor.
 
 `local_fallback.py`'s own exported helpers (`classify_bleed_edge`,
 `classify_border_color`, `classify_frame_style`, `compute_bleed_diff_mm`,
@@ -91,9 +88,9 @@
 
 SOURCE_REL = "MPCAutofill/cardpicker/image_evidence.py"
 
-# The OCR attempt-tier ladder (issue #259) — out of scope for this PR, being
-# worked on in parallel. See module docstring's "DELIBERATE EXCLUSIONS".
-EXCLUDED_HELPERS = frozenset({"_collector_line_ocr_attempts"})
+# No excluded helpers - the OCR attempt-tier ladder exclusion (issue #259) was lifted by issue
+# #677. See module docstring's "DELIBERATE EXCLUSIONS".
+EXCLUDED_HELPERS = frozenset()
 
 # Modules whose imported-and-called names are in this script's scope.
 # `cardpicker.local_fallback` and `cardpicker.local_phash` are deliberately
@@ -150,6 +147,12 @@
     # Pre-classification short-circuit's own acceptance predicate — same
     # reach as `_short_circuit_enabled_by_env`/`_contains_digit` above.
     "_confidently_digit_free": frozenset({"collector_line_ocr", "collector_line_tsv", "artist_ocr"}),
+    # The OCR attempt-tier ladder itself (issue #259, collapsed to 2 tiers by issue #677) — the
+    # ordered (variant, config, tier) sequence collector_line_ocr's own loop consumes, which
+    # therefore governs collector_line_tsv's word-box source and artist_ocr's raw-text-reuse
+    # population too. Was `EXCLUDED_HELPERS` while #677 was in flight; that exclusion is now
+    # lifted (see module docstring).
+    "_collector_line_ocr_attempts": frozenset({"collector_line_ocr", "collector_line_tsv", "artist_ocr"}),
     # --- cardpicker.collector_line_artist ---
     # Called from both `_parse_artist_is_contradicted` (gates
     # collector_line_ocr/collector_line_tsv/artist_ocr's raw-text-reuse
@@ -171,21 +174,20 @@
     "parse_collector_line": frozenset({"collector_line_ocr", "collector_line_tsv"}),
     # legal_line's own tolerant parse - called only from `_extract_legal_line`.
     "parse_legal_line": frozenset({"legal_line"}),
-    # Called directly (outside the excluded tier ladder) from
-    # `_extract_legal_line` and the artist_ocr crop+OCR fallback loop -
-    # NOT from collector_line_ocr's own tier-1 attempts, which live inside
-    # the excluded `_collector_line_ocr_attempts` generator (see module
-    # docstring's "DELIBERATE EXCLUSIONS" - that reach is this contributor's
-    # too, but is out of scope for this PR and left to the parallel branch).
-    "preprocess_variants": frozenset({"legal_line", "artist_ocr"}),
+    # Called from `_extract_legal_line`, the artist_ocr crop+OCR fallback loop, AND (since #677
+    # lifted the ladder exclusion) `_collector_line_ocr_attempts`' own tier-1 yield - all three
+    # reaches declared together.
+    "preprocess_variants": frozenset({"legal_line", "artist_ocr", "collector_line_ocr", "collector_line_tsv"}),
+    # Tier 2's own heavier-preprocessed variants (issue #259) - reachable exclusively through
+    # `_collector_line_ocr_attempts`, same reach as the ladder function itself.
+    "preprocess_fallback_variants": frozenset({"collector_line_ocr", "collector_line_tsv", "artist_ocr"}),
     # Same two in-scope call sites as `preprocess_variants` above (legal_line's
-    # OCR pass, artist_ocr's crop+OCR fallback) - not the excluded ladder.
+    # OCR pass, artist_ocr's crop+OCR fallback).
     "run_tesseract": frozenset({"legal_line", "artist_ocr"}),
     # Called directly inside compute_card_evidence's own OCR loop (the
     # `_collector_line_ocr_attempts` generator only yields preprocessed
     # variants + config + tier; the actual tesseract call, and therefore
-    # this function's own reach, is in the loop body, outside the excluded
-    # generator's subtree).
+    # this function's own reach, is in the loop body).
     "run_tesseract_text_and_words": frozenset({"collector_line_ocr", "collector_line_tsv"}),
 }
 

.github/scripts/tests/test_check_extractor_ownership_totality.py

@@ -30,10 +30,14 @@
 # A minimal fixture `image_evidence.py`: one module-private helper
 # (`_helper_one`) called from `compute_card_evidence`, one scoped-external
 # import (`external_thing`, standing in for e.g. `recover_artist_from_card_text`)
-# also called from there, one excluded-ladder helper (`_collector_line_ocr_attempts`)
-# whose own internal call (`preprocess_fallback_variants`-analogue,
-# `ladder_only_thing`) must NOT be required, and the manifest wiring
-# `check_extractor_manifest_sync`'s own derivation needs to find real keys.
+# also called from there, one helper (`_would_be_excluded`) whose own internal
+# call (`ladder_only_thing`) exercises the EXCLUDED_HELPERS mechanism itself -
+# that mechanism is generic and still live in the script even though issue
+# #677 emptied the REAL `EXCLUDED_HELPERS` set (it previously held the OCR
+# ladder, `_collector_line_ocr_attempts`, while a parallel branch worked on
+# it - see the script's own module docstring); tests that need the mechanism
+# ON pass `excluded_helpers=frozenset({"_would_be_excluded"})` to `fixture_repo`
+# explicitly rather than relying on a name that is no longer excluded for real.
 SOURCE_OK = '''
 """fixture image_evidence"""
 from cardpicker.collector_line_artist import external_thing
@@ -47,7 +51,7 @@ def _helper_one(x):
     return x
 
 
-def _collector_line_ocr_attempts(cropped):
+def _would_be_excluded(cropped):
     yield ladder_only_thing(cropped)
 
 
@@ -57,7 +61,7 @@ def compute_card_evidence(card):
     extractor_versions["legal_line"] = LEGAL_LINE_EXTRACTOR_VERSION
     _helper_one(card)
     external_thing(card)
-    for _ in _collector_line_ocr_attempts(card):
+    for _ in _would_be_excluded(card):
         pass
     return extractor_versions
 '''
@@ -70,14 +74,23 @@ def compute_card_evidence(card):
 }
 """
 
+# Matches SOURCE_OK's default (no exclusion in effect - EXCLUDED_HELPERS is empty for real too),
+# so `_would_be_excluded`/`ladder_only_thing` need entries like any other contributor.
 OWNERSHIP_OK = {
     "_helper_one": frozenset({"fetch_health"}),
     "external_thing": frozenset({"legal_line"}),
+    "_would_be_excluded": frozenset({"legal_line"}),
+    "ladder_only_thing": frozenset({"legal_line"}),
 }
 
 
 @contextlib.contextmanager
-def fixture_repo(source: str = SOURCE_OK, cohort: str = COHORT_OK, ownership: dict = None):
+def fixture_repo(
+    source: str = SOURCE_OK,
+    cohort: str = COHORT_OK,
+    ownership: dict = None,
+    excluded_helpers: frozenset = frozenset(),
+):
     with tempfile.TemporaryDirectory() as tmp:
         root = Path(tmp)
         for rel, text in ((lint.SOURCE_REL, source), (manifest_sync.COHORT_REL, cohort)):
@@ -87,15 +100,18 @@ def fixture_repo(source: str = SOURCE_OK, cohort: str = COHORT_OK, ownership: di
         saved_lint_root = lint.REPO_ROOT
         saved_manifest_root = manifest_sync.REPO_ROOT
         saved_ownership = lint.EXTRACTOR_OWNERSHIP
+        saved_excluded = lint.EXCLUDED_HELPERS
         lint.REPO_ROOT = root
         manifest_sync.REPO_ROOT = root
         lint.EXTRACTOR_OWNERSHIP = OWNERSHIP_OK if ownership is None else ownership
+        lint.EXCLUDED_HELPERS = excluded_helpers
         try:
             yield root
         finally:
             lint.REPO_ROOT = saved_lint_root
             manifest_sync.REPO_ROOT = saved_manifest_root
             lint.EXTRACTOR_OWNERSHIP = saved_ownership
+            lint.EXCLUDED_HELPERS = saved_excluded
 
 
 def joined(findings) -> str:
@@ -107,24 +123,32 @@ def test_derives_private_helper_and_called_external_import(self):
         with fixture_repo():
             contributors, findings = lint.derive_reachable_contributors()
             self.assertEqual(findings, [])
-            self.assertEqual(contributors, {"_helper_one", "external_thing"})
+            self.assertEqual(contributors, {"_helper_one", "external_thing", "_would_be_excluded", "ladder_only_thing"})
 
-    def test_excluded_ladder_function_itself_is_not_a_contributor(self):
-        # _collector_line_ocr_attempts is in EXCLUDED_HELPERS by name - it
-        # must never itself require an entry.
-        with fixture_repo():
+    def test_excluded_helper_itself_is_not_a_contributor(self):
+        # a name in EXCLUDED_HELPERS must never itself require an entry -
+        # the mechanism issue #677 emptied for the real ladder but which
+        # stays generic/reusable in the script itself.
+        with fixture_repo(excluded_helpers=frozenset({"_would_be_excluded"})):
             contributors, _ = lint.derive_reachable_contributors()
-            self.assertNotIn("_collector_line_ocr_attempts", contributors)
+            self.assertNotIn("_would_be_excluded", contributors)
 
-    def test_name_called_only_inside_the_excluded_ladder_is_not_a_contributor(self):
+    def test_name_called_only_inside_an_excluded_helper_is_not_a_contributor(self):
         # ladder_only_thing is a scoped-external import, but its one call
-        # site is inside _collector_line_ocr_attempts's own body - excluded
-        # per this PR's own brief (that ladder is being worked on in
-        # parallel by another branch).
-        with fixture_repo():
+        # site is inside the excluded helper's own body.
+        with fixture_repo(excluded_helpers=frozenset({"_would_be_excluded"})):
             contributors, _ = lint.derive_reachable_contributors()
             self.assertNotIn("ladder_only_thing", contributors)
 
+    def test_no_exclusion_by_default_requires_entries_for_both(self):
+        # SOURCE_OK's own default fixture_repo() call has EXCLUDED_HELPERS
+        # empty (matching the real script since issue #677) - both names
+        # ARE contributors now, and OWNERSHIP_OK declares both.
+        with fixture_repo():
+            contributors, _ = lint.derive_reachable_contributors()
+            self.assertIn("_would_be_excluded", contributors)
+            self.assertIn("ladder_only_thing", contributors)
+
     def test_import_used_only_as_a_type_hint_is_not_a_contributor(self):
         source = SOURCE_OK.replace(
             "from cardpicker.collector_line_artist import external_thing",
@@ -239,13 +263,16 @@ def test_derivation_sees_the_real_contributors(self):
             "parse_collector_line",
             "parse_legal_line",
             "run_tesseract_text_and_words",
+            # the OCR ladder itself and its own tier-2 helper (issue #677 lifted the
+            # EXCLUDED_HELPERS entry that used to hide both from this derivation)
+            "_collector_line_ocr_attempts",
+            "preprocess_fallback_variants",
         ):
             self.assertIn(name, contributors)
 
-    def test_excluded_ladder_is_really_excluded_in_the_real_module(self):
-        contributors, _ = lint.derive_reachable_contributors()
-        self.assertNotIn("_collector_line_ocr_attempts", contributors)
-        self.assertNotIn("preprocess_fallback_variants", contributors)
+    def test_no_helpers_are_excluded_in_the_real_module(self):
+        # issue #677 emptied EXCLUDED_HELPERS - the real script no longer excludes anything.
+        self.assertEqual(lint.EXCLUDED_HELPERS, frozenset())
 
     def test_every_declared_owning_key_set_is_non_empty(self):
         for name, keys in lint.EXTRACTOR_OWNERSHIP.items():

MPCAutofill/cardpicker/golden_set.py

@@ -392,6 +392,17 @@ class GoldenExpectation:
     # (real tesseract found nothing plausible on this sample), which is an honest outcome, not a
     # placeholder: only 10/30 produced a parseable collector number on this real run, several of
     # those a 4-digit "year" collector number (mtg/proxy sets) rather than a classic 3-digit one.
+    #
+    # Re-verified 2026-08-05 (issue #677, "collapse the Stage C OCR attempt ladder" - dropped the
+    # ladder's tier 3): none of these 30 values changed. All 30 golden cards were walked through
+    # the FULL pre-#677 8-attempt ladder with per-tier attribution - same method and the same live
+    # 450-card (300 blank + 150 success) probe documented in docs/features/catalog-completion-plan.md's
+    # own "#677" section (the probe script itself is a worktree-only analysis tool, not committed
+    # here, matching this repo's established convention for one-off probes - see e.g.
+    # docs/reports/2026-07-23-ocr-preprocessing-probe.md's own LIVE STATE) - not one ever resolved
+    # (genuine match, lexicon-valid parse, or "best invalid" fallback) uniquely at the removed
+    # tier, so `COLLECTOR_LINE_OCR_EXTRACTOR_VERSION`'s v2->v3 bump changes zero values here even
+    # though it changes some values outside this set (see that version constant's own comment).
     "collector_line_ocr": [
         GoldenExpectation(card_id=cid, value=value)
         for cid, value in {