diff --git a/MPCAutofill/cardpicker/local_calculate_verdicts.py b/MPCAutofill/cardpicker/local_calculate_verdicts.py index 5184ba729..13579f3a5 100644 --- a/MPCAutofill/cardpicker/local_calculate_verdicts.py +++ b/MPCAutofill/cardpicker/local_calculate_verdicts.py @@ -555,6 +555,24 @@ # HISTORICAL rows only - see its own module-level comment for why it is retired, not removed. JOIN_KEY_RESCANNABLE_SKIP_REASONS = frozenset({JOIN_KEY_NO_EVIDENCE_SKIP_REASON, TRANSFERRED_INTERIM_GUARD_SKIP_REASON}) +# THE FRAME AGREEMENT VETO'S OWN REQUIRED EXTRACTORS (2026-07-30). `classify_frame_style` takes +# exactly two inputs and each comes from a DIFFERENT extractor: `collector_line_collector_number` +# from `collector_line_ocr`, and `illus_anchor_fired` from `artist_ocr`. This calculator's +# eligibility query only ever guaranteed the first. +# +# PER-CHECK, NOT PER-CALCULATOR, deliberately. `local_detect_ai_art`/`local_lands_identify`/ +# `local_layout_class_cast` each declare ONE module-level `REQUIRED_EXTRACTOR_KEYS` because each +# runs one check. This calculator runs several with genuinely different needs - the join-key +# deduction itself needs only the collector line - so a calculator-wide gate would drop cards that +# have everything their own decision requires. Scoping the requirement to the check that has it is +# what keeps the gate from costing coverage it has no reason to cost. +# +# See `calculate_join_key_verdict`'s own call site for why THIS check, of the six ungated reads the +# 2026-07-29 composition audit found, is the one that had to be gated: it is the only one whose +# missing-data degradation is STRICT, and the skip reason it produces is not rescannable, so its +# wrong answer is permanent for that content hash. +FRAME_CHECK_REQUIRED_EXTRACTOR_KEYS = frozenset({"collector_line_ocr", "artist_ocr"}) + # THE SET-CODE LEXICON GATE (module docstring) - a parsed `set_code` that matches no # `CanonicalExpansion.code` at all, same permanent-conclusion category as "no-text"/"ambiguous" # (re-selecting the same card against the same stored evidence would deterministically reproduce @@ -796,14 +814,46 @@ def _apply_agreement_checks( # live-pilot pass already uses to compute it: whether a collector NUMBER was parsed # (post-2003 templates print one; pre-M15 templates never do) and whether the "Illus." # anchor fired (artist_ocr's own byproduct). PROTECTED CORE call, not a reimplementation. - frame_class = classify_frame_style( - parsed_a_collector_number=bool(evidence.collector_line_collector_number), - illus_anchor_fired=bool(evidence.illus_anchor_fired), - ) - if not frame_style_is_consistent(frame_class, metadata.frame): - # THE FRAME AGREEMENT VETO (module docstring) - mirrors - # local_identify_printing_tags.py's own frame-mismatch-withholding exactly. - return JoinKeyVerdict(card_id=card_id, skip_reason=JOIN_KEY_FRAME_MISMATCH_SKIP_REASON, detail=detail) + # + # GATED ON `artist_ocr` HAVING ACTUALLY RUN (2026-07-30, closing the 2026-07-29 composition + # audit's §5 second row). This calculator's eligibility query filters on + # `extractor_versions__has_key="collector_line_ocr"` and then reads SIX extractors' fields + # ungated. Most of those degrade PERMISSIVELY when their extractor never ran - a blank + # legal line reads as "nothing to compare", a null `image_is_truncated` reads as "not + # truncated" - and a permissive degradation is recoverable, because the human-backed + # consensus gate still stands between it and any resolution. + # + # THIS ONE DEGRADES STRICT, WHICH IS WHY IT IS THE ONE FIXED FIRST. `illus_anchor_fired` is + # NULLABLE, and `bool(None)` is `False`, which is indistinguishable from "artist_ocr ran + # and found no anchor". With no collector number either, `classify_frame_style` then + # returns "modern" for a card it has no anchor evidence about at all - and a genuine + # OLD-frame printing is vetoed `frame-mismatch`. That reason is deliberately NOT in + # `JOIN_KEY_RESCANNABLE_SKIP_REASONS`, so the wrong conclusion is PERMANENT for that + # content hash: the card never becomes eligible again, and no later Stage C pass can undo + # it. A wrong answer nothing can revisit is strictly worse than a missing one. + # + # So an absent `artist_ocr` skips the frame check entirely rather than evaluating it on + # invented input. That is not a new rule - it is the "missing data is not evidence" rule + # this function's own docstring already states, and which the copyright-year check and the + # `metadata is None` case above already follow. The card keeps its match at its + # already-computed confidence, and once Stage C fills `artist_ocr` in, the check runs for + # real. `artist_ocr` is at 220,579/220,579 coverage in production today, so this changes + # nothing about the current catalogue: it removes a trap, it does not loosen a live gate. + # + # `REQUIRED_EXTRACTOR_KEYS` is the pattern `local_detect_ai_art`, `local_lands_identify` + # and `local_layout_class_cast` already use, applied here per-CHECK rather than + # per-calculator: this calculator's other checks have genuinely different key + # requirements, and one calculator-wide gate would drop cards that only ever needed the + # collector line. + if FRAME_CHECK_REQUIRED_EXTRACTOR_KEYS <= evidence.extractor_versions.keys(): + frame_class = classify_frame_style( + parsed_a_collector_number=bool(evidence.collector_line_collector_number), + illus_anchor_fired=bool(evidence.illus_anchor_fired), + ) + if not frame_style_is_consistent(frame_class, metadata.frame): + # THE FRAME AGREEMENT VETO (module docstring) - mirrors + # local_identify_printing_tags.py's own frame-mismatch-withholding exactly. + return JoinKeyVerdict(card_id=card_id, skip_reason=JOIN_KEY_FRAME_MISMATCH_SKIP_REASON, detail=detail) # THE COPYRIGHT-YEAR ERA CHECK (module docstring) - reuses the SAME metadata row the # border/frame checks above already fetched, no second query. Skipped entirely (not a @@ -2470,6 +2520,7 @@ def run_slow_path_calculator( "JOIN_KEY_CONFIDENCE_SYMBOL_TIEBREAK", "JOIN_KEY_NO_MATCH_CONFIDENCE", "JOIN_KEY_CONFIDENCE_ARTIST_DISAGREEMENT", + "FRAME_CHECK_REQUIRED_EXTRACTOR_KEYS", "JOIN_KEY_RESCANNABLE_SKIP_REASONS", "TRANSFERRED_INTERIM_GUARD_SKIP_REASON", "JOIN_KEY_NO_HIT_SKIP_REASONS", diff --git a/MPCAutofill/cardpicker/tests/test_local_calculate_verdicts.py b/MPCAutofill/cardpicker/tests/test_local_calculate_verdicts.py index 85f6f7c31..0d2031343 100644 --- a/MPCAutofill/cardpicker/tests/test_local_calculate_verdicts.py +++ b/MPCAutofill/cardpicker/tests/test_local_calculate_verdicts.py @@ -35,14 +35,18 @@ FALLBACK_NO_EVIDENCE_SKIP_REASON, FALLBACK_NO_SUB_CHECK_EVIDENCE_SKIP_REASON, FALLBACK_RESCANNABLE_SKIP_REASONS, + FRAME_CHECK_REQUIRED_EXTRACTOR_KEYS, JOIN_KEY_ANONYMOUS_ID, JOIN_KEY_ARTIST_MISMATCH_SKIP_REASON, + JOIN_KEY_BORDER_MISMATCH_SKIP_REASON, JOIN_KEY_CONFIDENCE_ARTIST_DISAGREEMENT, JOIN_KEY_CONFIDENCE_BOTH, JOIN_KEY_CONFIDENCE_COLLECTOR_ONLY, JOIN_KEY_CONFIDENCE_SYMBOL_TIEBREAK, + JOIN_KEY_FRAME_MISMATCH_SKIP_REASON, JOIN_KEY_NO_HIT_SKIP_REASONS, JOIN_KEY_NO_MATCH_CONFIDENCE, + JOIN_KEY_RESCANNABLE_SKIP_REASONS, JOIN_KEY_UNKNOWN_SET_CODE_SKIP_REASON, RESOLUTION_FLOOR_DPI, SLOW_PATH_ANONYMOUS_ID, @@ -122,7 +126,13 @@ def _hash_of(expansion_code: str) -> int: def _evidence(card, **overrides): defaults = dict( content_hash=card.content_phash or 0, - extractor_versions={"collector_line_ocr": "collector-line-ocr-v1"}, + # BOTH OCR extractors by default (2026-07-30). Production has `artist_ocr` at + # 220,579/220,579 coverage, so the default fixture must too - the frame agreement veto + # reads `illus_anchor_fired` and is now gated on `artist_ocr` having actually run + # (`FRAME_CHECK_REQUIRED_EXTRACTOR_KEYS`), and a default that silently omitted it would + # turn every frame-veto test into a no-op. `TestFrameVetoRequiresArtistOcr` overrides this + # to exercise the absent case deliberately. + extractor_versions={"collector_line_ocr": "collector-line-ocr-v1", "artist_ocr": "artist-ocr-v2"}, collector_line_raw_text="", collector_line_set_code="", collector_line_collector_number="", @@ -3434,3 +3444,119 @@ def test_the_full_calculator_writes_no_routing_row_for_an_illustration_resolved_ assert not CardScanLog.objects.filter( card=resolved, anonymous_id=SLOW_PATH_ANONYMOUS_ID, skip_reason=SLOW_PATH_TO_REVIEW_SKIP_REASON ).exists() + + +class TestFrameVetoRequiresArtistOcr: + """ + THE ONE UNGATED READ THAT DEGRADES STRICT (2026-07-30, closing the 2026-07-29 composition + audit's §5 second row). + + `run_join_key_calculator` filters eligibility on + `extractor_versions__has_key="collector_line_ocr"` and then reads SIX extractors' fields + ungated. Five of those degrade PERMISSIVELY when their extractor never ran - a blank legal line + reads as "nothing to compare", a null `image_is_truncated` reads as "not truncated" - and a + permissive degradation is recoverable, because the human-backed consensus gate still stands + between it and any resolution. + + The frame veto is the exception. `illus_anchor_fired` is NULLABLE and `bool(None)` is `False`, + which is indistinguishable from "artist_ocr ran and found no anchor". With no collector number + either, `classify_frame_style` returns "modern" for a card it has no anchor evidence about, and + a genuine OLD-frame printing is vetoed `frame-mismatch` - a reason deliberately NOT in + `JOIN_KEY_RESCANNABLE_SKIP_REASONS`, so the wrong conclusion is PERMANENT for that content + hash. That is why this read, of the six, is the one gated. + """ + + def _old_frame_setup(self, extractor_versions): + """A pre-M15 printing: `frame="1993"` (old), and NO collector number on the card face - + exactly the shape whose frame class is decided entirely by `illus_anchor_fired`. The match + itself comes from a unique collector-number-free candidate, so the frame veto is the only + thing that can withhold it.""" + printing = CanonicalCardFactory(name="Frame Gate Card", expansion__code="lea", collector_number="158") + CanonicalPrintingMetadataFactory(canonical_card=printing, frame="1993") + card = CardFactory(name="Frame Gate Card") + candidates = [CandidatePrinting(pk=printing.pk, expansion_code="lea", collector_number="158")] + evidence = _evidence( + card, + extractor_versions=extractor_versions, + collector_line_set_code="lea", + collector_line_collector_number="158", + illus_anchor_fired=None, + ) + return printing, card, candidates, evidence + + def test_a_missing_artist_ocr_does_not_permanently_veto_a_genuine_old_frame_card(self, db): + """THE DEFECT. `artist_ocr` never ran, so `illus_anchor_fired` is NULL - unknown, not + False. The frame check must not run at all rather than manufacture "modern" from it.""" + printing, card, candidates, evidence = self._old_frame_setup({"collector_line_ocr": "collector-line-ocr-v2"}) + + verdict = calculate_join_key_verdict(card.pk, evidence, candidates) + + assert verdict.skip_reason != JOIN_KEY_FRAME_MISMATCH_SKIP_REASON + assert verdict.printing_pk == printing.pk + + def test_the_same_card_is_still_vetoed_once_artist_ocr_has_run(self, db): + """THE CONTROL, and the half that stops this being a licence to drop the veto. With + `artist_ocr` present, a NULL `illus_anchor_fired` means the extractor ran and found no + anchor - a real negative - so "modern" is a genuine reading and the veto fires exactly as + it always did. Same card, same evidence, one extra manifest key.""" + printing, card, candidates, evidence = self._old_frame_setup( + {"collector_line_ocr": "collector-line-ocr-v2", "artist_ocr": "artist-ocr-v2"} + ) + + verdict = calculate_join_key_verdict(card.pk, evidence, candidates) + + assert verdict.skip_reason == JOIN_KEY_FRAME_MISMATCH_SKIP_REASON + assert verdict.printing_pk is None + + def test_the_frame_mismatch_reason_is_still_not_rescannable(self, db): + """WHY THE GATE HAD TO EXIST RATHER THAN THE REASON BEING MADE RESCANNABLE. If + `frame-mismatch` were re-selectable, a later Stage C pass could undo a wrong veto and this + would be a transient bug. It is not, deliberately - a genuine frame contradiction is a + repeatable conclusion about stored evidence. Pinned here so a future change that adds it to + the rescannable set has to confront the fact that it is what makes the veto permanent.""" + assert JOIN_KEY_FRAME_MISMATCH_SKIP_REASON not in JOIN_KEY_RESCANNABLE_SKIP_REASONS + + def test_the_gate_names_both_extractors_the_classifier_actually_reads(self, db): + """`classify_frame_style`'s two arguments come from two DIFFERENT extractors, and the + eligibility query only ever guaranteed the first. Pinning both means a future extractor + rename cannot quietly shrink the gate to the half that was never the problem.""" + assert FRAME_CHECK_REQUIRED_EXTRACTOR_KEYS == frozenset({"collector_line_ocr", "artist_ocr"}) + + def test_an_agreeing_frame_still_matches_when_artist_ocr_is_absent(self, db): + """The gate must not turn a good match into a skip either - skipping the CHECK is not + skipping the CARD.""" + printing = CanonicalCardFactory(name="Frame Gate Modern", expansion__code="mom", collector_number="158") + CanonicalPrintingMetadataFactory(canonical_card=printing, frame="2015") + card = CardFactory(name="Frame Gate Modern") + candidates = [CandidatePrinting(pk=printing.pk, expansion_code="mom", collector_number="158")] + evidence = _evidence( + card, + extractor_versions={"collector_line_ocr": "collector-line-ocr-v2"}, + collector_line_set_code="mom", + collector_line_collector_number="158", + ) + + verdict = calculate_join_key_verdict(card.pk, evidence, candidates) + + assert verdict.printing_pk == printing.pk + assert verdict.confidence == JOIN_KEY_CONFIDENCE_BOTH + + def test_the_border_veto_is_unaffected_by_the_frame_gate(self, db): + """The gate is scoped to the frame check alone. A border mismatch on a card with no + `artist_ocr` must still withhold - `layout_class` comes from a different extractor and its + own degradation is permissive, which is a separate question this PR does not touch.""" + printing = CanonicalCardFactory(name="Border Still Vetoes", expansion__code="mom", collector_number="158") + CanonicalPrintingMetadataFactory(canonical_card=printing, border_color="white", frame="2015") + card = CardFactory(name="Border Still Vetoes") + candidates = [CandidatePrinting(pk=printing.pk, expansion_code="mom", collector_number="158")] + evidence = _evidence( + card, + extractor_versions={"collector_line_ocr": "collector-line-ocr-v2"}, + collector_line_set_code="mom", + collector_line_collector_number="158", + layout_class="black", + ) + + verdict = calculate_join_key_verdict(card.pk, evidence, candidates) + + assert verdict.skip_reason == JOIN_KEY_BORDER_MISMATCH_SKIP_REASON diff --git a/docs/identification-pipeline.md b/docs/identification-pipeline.md index ffd461652..c6df04282 100644 --- a/docs/identification-pipeline.md +++ b/docs/identification-pipeline.md @@ -218,6 +218,31 @@ identity, and (safety) nothing already resolved. Then five stages per card: catalog-required on every genuine upload, so presence proves nothing about which printing this is. (Until the #294 re-scan it wrongly vetoed 1,552 validated matches — the re-scan un-blocks them.) + + **A check whose extractor never ran does not run (2026-07-30).** The + calculator's eligibility query only guarantees `collector_line_ocr`, and it + then reads six extractors' fields. Most degrade _permissively_ when their + extractor is absent — a blank legal line is "nothing to compare", a null + truncation flag is "not truncated" — and permissive is recoverable, because + the human-backed gate below still stands in the way. + + The frame check was the exception, and it is now gated on `artist_ocr`. + `illus_anchor_fired` is nullable, so `bool(None)` reads as "the anchor did + not fire" — indistinguishable from "the extractor never looked". On a card + with no collector number either, the classifier then answers `modern` about + a card it has no anchor evidence for, and a genuine old-frame printing is + withheld as `frame-mismatch`. That reason is deliberately **not** + rescannable, so the wrong conclusion is _permanent_ for that content hash: + no later pass can revisit it. A wrong answer nothing can revisit is worse + than a missing one, so an absent `artist_ocr` now skips the check and leaves + the match standing — the same "missing data is not evidence" rule the + copyright-year check already follows. Skipping the _check_ is not skipping + the _card_. + + The requirement is declared per-check, not per-calculator: the join-key + deduction itself needs only the collector line, and one calculator-wide gate + would drop cards that have everything their own decision requires. + - **g5 — the vote, never the verdict.** A match casts one machine CardPrintingTag vote: weight 0.5, with an ordinal confidence label (0.85/0.75/0.65 — a pipeline-state rank, _not_ a probability, and verified