diff --git a/MPCAutofill/cardpicker/local_identify_printing_tags.py b/MPCAutofill/cardpicker/local_identify_printing_tags.py index 648c1aefa..3a6284b0f 100644 --- a/MPCAutofill/cardpicker/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/local_identify_printing_tags.py @@ -273,7 +273,7 @@ def select_candidates( for card in ( _eligible_base_queryset(anonymous_id, exclude_source_pks) .exclude(dpi__lt=RESOLUTION_FLOOR_DPI) - .only("pk", "name", "identifier", "source_id") + .only("pk", "name", "identifier", "source_id", "expansion_hint") .order_by("pk") .iterator(chunk_size=5000) ): @@ -471,6 +471,37 @@ def run_phash_for_card( DEFAULT_WORKERS = 2 +def _narrow_candidates_by_expansion_hint(selected: SelectedCard) -> SelectedCard: + """Fast-follow (2026-07-16): a confidence PRIOR, not an entailment - narrows the candidate + list an engine considers when the card's own `expansion_hint` (extracted from its filename + at import time by `cardpicker.tags.Tags.extract` - a lone set-code bracket token that + didn't resolve a direct CanonicalCard match, e.g. "[UNF]" with no collector number) matches + at least one of its name's real candidates. Never narrows to empty: if the hint matches zero + candidates (a real, measured ~9% data-quality case - the hint may be stale or mismatched), + the full candidate list is used instead, exactly as if there were no hint at all - narrowing + that made matching IMPOSSIBLE would be worse than not narrowing. + + Scoped to engine-matching ONLY - never call this from select_candidates/ + compute_covered_printing_pks/anything computing coverage or ordering, which need the true, + unnarrowed candidate set to stay correct. The returned SelectedCard is a LOCAL substitute + used only for this card's own OCR/phash/fallback calls within _compute_card; nothing + downstream (run_pilot's own all_selected_by_card_id) ever sees the narrowed version. + + Real yield (measured live, 2026-07-15): of 2,466 pilot-eligible cards with a real + expansion_hint, 645 currently exceed PHASH_MAX_CANDIDATES and get skipped entirely - + narrowing brings 407 of those back under the cap, giving phash a real shot where it + currently never runs at all. OCR's own exact-match logic doesn't benefit from narrowing + (a smaller candidate list doesn't change whether a parsed code+number is IN it) - this is a + phash-only unlock in practice, though harmless to apply uniformly to all three engines.""" + hint = selected.card.expansion_hint + if not hint: + return selected + narrowed = [c for c in selected.candidates if c.expansion_code == hint] + if not narrowed: + return selected + return SelectedCard(card=selected.card, candidates=narrowed) + + def _compute_card( selected: SelectedCard, ocr_selected_ids: set[int], @@ -502,6 +533,11 @@ def _compute_card( image = fetch_card_image(selected.card, fetch_dpi) ocr_raw_texts: list[str] = [] + # fast-follow (2026-07-16): narrow the candidate list every engine below sees, using this + # card's own expansion_hint if it has one - `selected.card`/`card_id` above still reference + # the ORIGINAL card either way; only the candidate list used for matching changes. + selected = _narrow_candidates_by_expansion_hint(selected) + outcome.image_fetched = image is not None bleed_class = local_fallback.classify_bleed_edge(image) if image is not None else None outcome.bleed_class = bleed_class @@ -1175,6 +1211,105 @@ def propagate_cluster_vote( return results, attributes +# Fast-follow (2026-07-16): name-frequency elimination - see run_name_frequency_elimination's +# own docstring for the full design rationale (in particular the SAFE 1:1 gate that makes this +# sound, not just "one uncovered printing"). +NAME_FREQUENCY_ANONYMOUS_ID = "local-name-frequency-v1" +# Deliberately modest relative to OCR/phash's own confidences (0.85/0.75/0.8) - this is a purely +# structural deduction (no visual confirmation of THIS card at all), weaker evidence than an +# engine that actually looked at the image, even though the 1:1 gate makes it sound. +NAME_FREQUENCY_CONFIDENCE = 0.6 + + +@dataclass +class NameFrequencyResult: + dry_run: bool = False + votes_written: int = 0 + gate_violations: list[int] = field(default_factory=list) + + +def run_name_frequency_elimination(dry_run: bool = False, batch_size: int = DEFAULT_BATCH_SIZE) -> NameFrequencyResult: + """Fast-follow (2026-07-16): for a NAME where exactly one of its printings remains + uncovered (see compute_covered_printing_pks) AND exactly one pilot-eligible card is + unresolved for that name, the match is deducible by elimination alone - no image fetch, no + OCR/phash, no visual disambiguation needed at all. + + The SAFE gate is "exactly one uncovered printing AND exactly one unresolved-eligible card", + not just "exactly one uncovered printing" - a name can have one uncovered printing while + SEVERAL unresolved cards share that name, in which case elimination does NOT tell you WHICH + of those cards is the missing one (any of the others could just as easily be a redundant + depiction of an ALREADY-covered printing, uploaded by a different source). Gating on "and + exactly one unresolved card too" is what makes the deduction airtight; it is not a + nice-to-have refinement, it is the difference between a sound inference and a coin flip. + Measured live against the full (not sampled) catalog, 2026-07-16: 2,076 names have exactly + one uncovered printing; only 1,678 of those also have exactly one unresolved eligible card. + The naive, ungated version would have voted - incorrectly, on average - for the other ~400 + names' multiple candidate cards. + + Still just a VOTE (this function's own anonymous_id), never a direct resolve - same + consensus/gate-check discipline as every other engine in this module. Reuses + _eligible_base_queryset for the exact same base eligibility rules (unresolved, no confirmed + match, card_type=CARD, not deductive-backfill-covered, no custom-art/non-english tag) plus + this function's own anonymous_id for idempotence, and the SAME batch-flush + gate-check + pattern as run_pilot (a kill loses at most one batch; a plain re-invocation resumes cleanly). + """ + covered_printing_pks = compute_covered_printing_pks() + index = CandidateNameIndex() + + cards_by_name: dict[str, list[int]] = collections.defaultdict(list) + for card_id, name in ( + _eligible_base_queryset(NAME_FREQUENCY_ANONYMOUS_ID) + .values_list("pk", "name") + .order_by("pk") + .iterator(chunk_size=5000) + ): + cards_by_name[name].append(card_id) + + result = NameFrequencyResult(dry_run=dry_run) + votes_batch: list[CardPrintingTag] = [] + batch_written_card_ids: list[int] = [] + + def flush() -> None: + nonlocal votes_batch, batch_written_card_ids + if dry_run: + votes_batch, batch_written_card_ids = [], [] + return + if votes_batch: + CardPrintingTag.objects.bulk_create(votes_batch) + if batch_written_card_ids: + result.gate_violations.extend(verify_zero_resolutions(batch_written_card_ids)) + votes_batch, batch_written_card_ids = [], [] + + for name, card_ids in cards_by_name.items(): + if len(card_ids) != 1: + continue + candidates = index.candidates_for(name) + if not candidates: + continue + uncovered = [c for c in candidates if c.pk not in covered_printing_pks] + if len(uncovered) != 1: + continue + + votes_batch.append( + CardPrintingTag( + card_id=card_ids[0], + printing_id=uncovered[0].pk, + is_no_match=False, + anonymous_id=NAME_FREQUENCY_ANONYMOUS_ID, + source=VoteSource.OCR, + confidence=NAME_FREQUENCY_CONFIDENCE, + ) + ) + batch_written_card_ids.append(card_ids[0]) + result.votes_written += 1 + + if len(batch_written_card_ids) >= batch_size: + flush() + + flush() + return result + + def verify_zero_resolutions(card_ids: list[int], batch_size: int = 2000) -> list[int]: """Identical rationale/mechanism to cardpicker.deductive_backfill.verify_zero_resolutions - the *pure* resolve_printing (never resolve_and_persist_printing, which must never itself @@ -1217,5 +1352,9 @@ def verify_zero_resolutions(card_ids: list[int], batch_size: int = 2000) -> list "PilotResult", "AttributeReport", "run_pilot", + "NAME_FREQUENCY_ANONYMOUS_ID", + "NAME_FREQUENCY_CONFIDENCE", + "NameFrequencyResult", + "run_name_frequency_elimination", "verify_zero_resolutions", ] diff --git a/MPCAutofill/cardpicker/management/commands/local_name_frequency_elimination.py b/MPCAutofill/cardpicker/management/commands/local_name_frequency_elimination.py new file mode 100644 index 000000000..21833453b --- /dev/null +++ b/MPCAutofill/cardpicker/management/commands/local_name_frequency_elimination.py @@ -0,0 +1,56 @@ +from typing import Any + +from django.core.management.base import BaseCommand, CommandError + +from cardpicker.local_identify_printing_tags import run_name_frequency_elimination + + +class Command(BaseCommand): + help = ( + "Fast-follow (see docs/features/printing-tags.md Stage 8): casts OCR-weight votes for " + "cards whose name has exactly one uncovered printing AND exactly one unresolved " + "pilot-eligible card - a pure structural deduction, no image fetch or OCR/phash " + "involved. Never resolves a card by itself (the human-backed consensus gate still " + "applies) - see run_name_frequency_elimination's own docstring for the safety gate " + "that makes this deduction sound." + ) + + def add_arguments(self, parser: Any) -> None: + parser.add_argument( + "--dry-run", + action="store_true", + default=False, + help="Evaluate without writing anything or running the gate check.", + ) + parser.add_argument( + "--batch-size", + type=int, + default=25, + help="Flush votes to the DB (and run the gate check) every this many cards, same " + "checkpointing convention as local_identify_printing_tags. Default: 25.", + ) + + def handle(self, *args: Any, **kwargs: Any) -> None: + dry_run = kwargs["dry_run"] + batch_size = kwargs["batch_size"] + + mode = "DRY RUN" if dry_run else "WRITE" + print(f"[{mode}] local_name_frequency_elimination --batch-size={batch_size}") + + result = run_name_frequency_elimination(dry_run=dry_run, batch_size=batch_size) + + print(f"votes written: {result.votes_written}") + + if dry_run: + print("Dry run - nothing written, gate check not run.") + return + + if result.gate_violations: + raise CommandError( + f"GATE VIOLATION: {len(result.gate_violations)} card(s) resolved after a " + f"machine-only vote, which should be structurally impossible - STOP and " + f"investigate before continuing. Affected card pks: {result.gate_violations[:50]}" + + (" (truncated)" if len(result.gate_violations) > 50 else "") + ) + + print(f"Gate check passed: 0/{result.votes_written} affected cards resolved.") diff --git a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py index 193312d13..5e6943cdc 100644 --- a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py @@ -17,6 +17,8 @@ from cardpicker.local_fallback import FALLBACK_ANONYMOUS_ID from cardpicker.local_identify_printing_tags import ( DEDUCTIVE_BACKFILL_ANONYMOUS_ID, + NAME_FREQUENCY_ANONYMOUS_ID, + NAME_FREQUENCY_CONFIDENCE, OCR_ANONYMOUS_ID, PHASH_ANONYMOUS_ID, RESOLUTION_FLOOR_DPI, @@ -26,6 +28,7 @@ compute_own_image_clusters, count_below_resolution_floor, get_worker_image_url, + run_name_frequency_elimination, run_pilot, select_candidates, verify_zero_resolutions, @@ -152,6 +155,85 @@ def test_more_uncovered_candidates_come_before_fewer_when_both_fully_uncovered(s assert [s.card.pk for s in selected] == [multi.pk, single.pk] +class TestExpansionHintNarrowing: + """Fast-follow (2026-07-16): _narrow_candidates_by_expansion_hint narrows the candidate + list an engine considers using Card.expansion_hint (already populated at import time by + cardpicker.tags.Tags.extract - not a new field, just newly wired into the pilot).""" + + def test_no_hint_returns_selected_unchanged(self, db): + import cardpicker.local_identify_printing_tags as module + + CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="bbb")) + card = CardFactory(name="Forest", expansion_hint="") + index = CandidateNameIndex() + selected = module.SelectedCard(card=card, candidates=index.candidates_for("Forest")) + + narrowed = module._narrow_candidates_by_expansion_hint(selected) + + assert narrowed is selected + assert len(narrowed.candidates) == 2 + + def test_hint_narrows_to_only_matching_candidates(self, db): + import cardpicker.local_identify_printing_tags as module + + expansion_bbb = CanonicalExpansionFactory(code="bbb") + CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + CanonicalCardFactory(name="Forest", expansion=expansion_bbb) + CanonicalCardFactory(name="Forest", expansion=expansion_bbb) + card = CardFactory(name="Forest", expansion_hint="bbb") + index = CandidateNameIndex() + selected = module.SelectedCard(card=card, candidates=index.candidates_for("Forest")) + assert len(selected.candidates) == 3 + + narrowed = module._narrow_candidates_by_expansion_hint(selected) + + assert len(narrowed.candidates) == 2 + assert all(c.expansion_code == "bbb" for c in narrowed.candidates) + assert narrowed.card is card + + def test_hint_matching_zero_candidates_falls_back_to_full_list(self, db): + # a real, measured data-quality case: the hint doesn't match anything in this name's + # actual candidate pool - narrowing to empty would make matching IMPOSSIBLE, strictly + # worse than not narrowing at all. + import cardpicker.local_identify_printing_tags as module + + CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + card = CardFactory(name="Forest", expansion_hint="zzz") + index = CandidateNameIndex() + selected = module.SelectedCard(card=card, candidates=index.candidates_for("Forest")) + + narrowed = module._narrow_candidates_by_expansion_hint(selected) + + assert len(narrowed.candidates) == 1 + + def test_phash_unlocked_when_narrowing_crosses_under_the_candidate_cap(self, db, monkeypatch): + # the real, measured benefit: a name with MORE than PHASH_MAX_CANDIDATES total + # printings gets skipped entirely ("too-many-candidates") - but if this card's own + # expansion_hint narrows it down to a small handful, phash gets a real shot instead. + import cardpicker.local_identify_printing_tags as module + + for i in range(module.PHASH_MAX_CANDIDATES + 3): + CanonicalCardFactory(name="Beast", expansion=CanonicalExpansionFactory(code=f"e{i:02}")) + CanonicalCardFactory(name="Beast", expansion=CanonicalExpansionFactory(code="hnt")) + CardFactory(name="Beast", expansion_hint="hnt") + + phash_call_candidate_counts: list[int] = [] + + def recording_run_phash_for_card(selected, image, threshold, margin, max_candidates, bleed_class=None): + phash_call_candidate_counts.append(len(selected.candidates)) + return None, "no-clear-winner" + + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: Image.new("RGB", (750, 1050))) + monkeypatch.setattr(module, "run_phash_for_card", recording_run_phash_for_card) + + run_pilot(engine="phash", limit=10, dry_run=True, nice=False) + + # narrowed to just the "hnt" candidate (1), not the full 13+ - phash actually ran + # (recorded a call) instead of being skipped at selection-adjacent "too-many-candidates". + assert phash_call_candidate_counts == [1] + + class TestCoveragePriority: """Addendum item 1 (2026-07-15): coverage-gap + demand ordering, the full 5-key tuple (zero-covered first, descending uncovered count, demand rank, fewer candidates, pk).""" @@ -739,6 +821,126 @@ def fake_ocr(selected, image, crop_box, bleed_class=None): assert attributes.uncovered_printings_closed == 0 +class TestNameFrequencyElimination: + """Fast-follow (2026-07-16): run_name_frequency_elimination's SAFE 1:1 gate - exactly one + uncovered printing AND exactly one unresolved-eligible card for that name - not just "one + uncovered printing" (which is unsound whenever more than one unresolved card shares a name; + see the function's own docstring for the full rationale, backed by a live measurement).""" + + def test_votes_for_the_single_uncovered_printing_when_exactly_one_card_and_one_gap(self, db): + covered_printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + uncovered_printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="bbb")) + CardFactory(canonical_card=covered_printing) # confirms "aaa" as covered + card = CardFactory(name="Forest") # the single unresolved card for this name + + result = run_name_frequency_elimination(dry_run=False) + + assert result.votes_written == 1 + vote = CardPrintingTag.objects.get(card=card, anonymous_id=NAME_FREQUENCY_ANONYMOUS_ID) + assert vote.printing_id == uncovered_printing.pk + assert vote.confidence == NAME_FREQUENCY_CONFIDENCE + assert vote.is_no_match is False + + def test_does_not_vote_when_multiple_unresolved_cards_share_the_name(self, db): + # the unsafe case this gate exists specifically to exclude: TWO unresolved cards for + # "Forest", only one uncovered printing - elimination can't tell you which card (if + # either) is the missing one, so it must abstain for BOTH, not guess for either. + covered_printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="bbb")) + CardFactory(canonical_card=covered_printing) + CardFactory(name="Forest") + CardFactory(name="Forest") + + result = run_name_frequency_elimination(dry_run=False) + + assert result.votes_written == 0 + assert not CardPrintingTag.objects.filter(anonymous_id=NAME_FREQUENCY_ANONYMOUS_ID).exists() + + def test_does_not_vote_when_more_than_one_printing_is_uncovered(self, db): + CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="bbb")) + CardFactory(name="Forest") # single unresolved card, but nothing covered at all + + result = run_name_frequency_elimination(dry_run=False) + + assert result.votes_written == 0 + + def test_does_not_vote_when_fully_covered(self, db): + printing = CanonicalCardFactory(name="Forest") + CardFactory(canonical_card=printing) + CardFactory(name="Forest") + + result = run_name_frequency_elimination(dry_run=False) + + assert result.votes_written == 0 + + def test_dry_run_writes_nothing(self, db): + covered_printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="bbb")) + CardFactory(canonical_card=covered_printing) + CardFactory(name="Forest") + + result = run_name_frequency_elimination(dry_run=True) + + assert result.votes_written == 1 # counted, even though nothing is persisted + assert not CardPrintingTag.objects.exists() + + def test_idempotent_on_a_second_invocation(self, db): + covered_printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="bbb")) + CardFactory(canonical_card=covered_printing) + CardFactory(name="Forest") + + first = run_name_frequency_elimination(dry_run=False) + second = run_name_frequency_elimination(dry_run=False) + + assert first.votes_written == 1 + assert second.votes_written == 0 + assert CardPrintingTag.objects.filter(anonymous_id=NAME_FREQUENCY_ANONYMOUS_ID).count() == 1 + + def test_excludes_tokens_and_cardbacks(self, db): + covered_printing = CanonicalCardFactory(name="Beast", expansion=CanonicalExpansionFactory(code="aaa")) + CanonicalCardFactory(name="Beast", expansion=CanonicalExpansionFactory(code="bbb")) + CardFactory(canonical_card=covered_printing) + CardFactory(name="Beast", card_type=CardTypes.TOKEN) + + result = run_name_frequency_elimination(dry_run=False) + + assert result.votes_written == 0 + + +class TestNameFrequencyEliminationCommand: + def test_dry_run_writes_nothing(self, db, capsys): + from django.core.management import call_command + + covered_printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="bbb")) + CardFactory(canonical_card=covered_printing) + CardFactory(name="Forest") + + call_command("local_name_frequency_elimination", "--dry-run") + + printed = capsys.readouterr().out + assert "[DRY RUN]" in printed + assert "votes written: 1" in printed + assert not CardPrintingTag.objects.exists() + + def test_real_run_writes_and_passes_gate_check(self, db, capsys): + from django.core.management import call_command + + covered_printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="bbb")) + CardFactory(canonical_card=covered_printing) + card = CardFactory(name="Forest") + + call_command("local_name_frequency_elimination") + + printed = capsys.readouterr().out + assert "[WRITE]" in printed + assert "Gate check passed: 0/1 affected cards resolved." in printed + assert CardPrintingTag.objects.filter(card=card, anonymous_id=NAME_FREQUENCY_ANONYMOUS_ID).exists() + + class TestRunPilotSourceExclusion: def test_excluded_sources_cards_never_reach_the_engine(self, db, monkeypatch): excluded_source = SourceFactory() diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index b976df9d7..00fa9ceb1 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -2043,6 +2043,59 @@ trigger), PR #19's disposition (owner's convenience). **Full-catalog run: not yet fired.** This report is the synthesizing deliverable requested before that authorization - awaiting explicit owner go-ahead. +## Two fast-follows, built after HOLD #2 (2026-07-16) + +Both researched and sized before building (see the HOLD #2 section above and this doc's earlier +feasibility notes) - neither required schema changes, both reuse already-existing, already- +populated data. + +### `expansion_hint` candidate narrowing + +`_narrow_candidates_by_expansion_hint` (`local_identify_printing_tags.py`) narrows the +candidate list every engine considers, using `Card.expansion_hint` - a field that already +existed and is already populated at import time by `cardpicker.tags.Tags.extract` (a lone +set-code bracket token in the filename that didn't resolve a direct match, e.g. `[UNF]` with no +collector number). Not a new signal - just newly wired into the pilot; `deductive_backfill`'s +own D2 tier already trusts this same field for direct resolution when it narrows to exactly +one candidate. + +A confidence PRIOR, not an entailment: narrows the list passed to `run_ocr_for_card`/ +`run_phash_for_card`/`run_fallback_for_card` inside `_compute_card` only - never touches +`select_candidates`'s ordering, `compute_covered_printing_pks`, or the +`uncovered_printings_closed` metric, all of which need the true, unnarrowed candidate set to +stay correct. Never narrows to empty: if the hint matches zero of the name's real candidates (a +real, measured ~9% data-quality case - the hint may be stale or mismatched), the full list is +used instead. + +**Real yield, measured live**: of 2,466 pilot-eligible cards with a real `expansion_hint`, 645 +currently get skipped by phash outright (`too-many-candidates`) - narrowing brings 407 of those +back under `PHASH_MAX_CANDIDATES`, giving phash a real shot where it currently never runs. +OCR's own exact-match logic doesn't benefit (a smaller candidate list doesn't change whether a +parsed code+number is in it) - this is a phash-only unlock in practice. + +### Name-frequency elimination + +`run_name_frequency_elimination` (new function, new management command +`local_name_frequency_elimination`) - for a NAME where exactly one printing remains uncovered +AND exactly one pilot-eligible card is unresolved for that name, the match is deducible by +elimination alone: no image fetch, no OCR/phash, no visual disambiguation at all. + +**The safety gate is the whole point, not a refinement.** A name can have exactly one uncovered +printing while SEVERAL unresolved cards share that name - in that case elimination does NOT +tell you WHICH card is the missing one (any of the others could just as easily be a redundant +depiction of an already-covered printing uploaded by a different source). The naive version +(gate on "one uncovered printing" alone) was the original researched number; adding "and +exactly one unresolved card too" is what makes the deduction airtight. Measured live against +the full catalog (not a sample), 2026-07-16: 2,076 names have exactly one uncovered printing; +only 1,678 of those also have exactly one unresolved eligible card - the naive version would +have voted incorrectly, on average, for the other ~400 names' multiple candidate cards. + +Confidence deliberately modest (0.6, vs. OCR/phash's 0.85/0.75/0.8) - a purely structural +deduction is weaker evidence than an engine that actually looked at the image, even with the +1:1 gate making it sound. Still just a vote (`NAME_FREQUENCY_ANONYMOUS_ID`), never a direct +resolve - same consensus/gate-check discipline as every other engine in this module, same +batch-flush checkpointing pattern as `run_pilot`. + ## Incident: per-chunk thread pool leaked Postgres connections, crashed the live run (2026-07-16) The second full-catalog relaunch (post cluster-dedup removal) died ~3 minutes in with @@ -2067,3 +2120,76 @@ the code comment at the fix site); regression test `TestConcurrency::test_thread_pool_is_created_once_for_the_whole_run_not_per_chunk` asserts pool construction count stays at 1 across multiple real chunks of work, not just that votes still get written. + +## Prior-art read: phash calibration in other MTG card-ID projects (2026-07-16) + +Timeboxed (~1hr) research task, ahead of designing the two-threshold clustering (item 3) and +art-region hash variant (item 4) follow-ups. Examined +[`tmikonen/magic_card_detector`](https://github.com/tmikonen/magic_card_detector) and +[`freeall/mtg-card-detector`](https://github.com/freeall/mtg-card-detector), both MIT-licensed +(copyright Timo Ikonen). **These are not two independent implementations** - freeall's repo is +an explicit fork of tmikonen's; the core hashing/matching code (`magic_card_detector.py`) is +essentially unmodified between them, freeall's changes being CLI ergonomics and a filename +convention for carrying Scryfall IDs through. Credit: threshold/matching approach below is +tmikonen's original work, referenced here as prior art per project attribution policy - no code +adopted verbatim, MIT terms would apply if that changes. + +**Their "threshold" is not directly reusable as a Hamming-distance number.** They use +`imagehash.phash(hash_size=32)` (a 32x32/1024-bit hash, far larger than imagehash's 8x8 default), +but the match decision isn't a flat distance cutoff - it's a per-query statistical outlier test: +the best (smallest) Hamming distance among all candidates is compared to the _mean and standard +deviation of the distances to every other candidate_, and accepted only if it's more than 4 +standard deviations below that mean. Reusing "4" as if it were a raw phash bit-distance (the way +this pilot's own d=0/d<=2 tiers are expressed) would be a category error - the two numbers aren't +on the same scale. The transferable idea, if any, is the _method_: validating a distance +threshold against the population's own distance distribution rather than picking a fixed cutoff +in isolation - a possible cross-check for calibrating d<=2, not a value to copy. + +**No working art-region hash code exists in either project.** tmikonen's own blog post +(tmikonen.github.io) names hashing a separate art-only reference image as future work, never +implemented in either repo. Nothing to borrow beyond "someone else independently considered this +useful," which is a weak signal, not a design. + +Other notes: both preprocess with CLAHE histogram equalization and hash at all 4 rotations +(a "photo of a physical card" concern from unknown-orientation scans - doesn't apply to this +pilot's Scryfall-sourced digital images, which are already upright). Neither repo touches the +Scryfall API directly; both assume a pre-populated local image folder, matched by brute-force +linear scan against every reference hash (no indexing/bucketing) - not a scale precedent worth +following at 172k+ cards regardless of threshold source. + +## Phash accuracy at small CDN sizes (2026-07-16) + +Investigated whether the disabled cluster-dedup pre-pass (`compute_own_image_clusters`, see the +disablement entry above) could be cheaply re-added by hashing small CDN-resized images instead +of full resolution. There's only one fetch path in the whole module +(`fetch_card_image`/`get_worker_image_url`) - OCR, the main phash engine, and clustering all go +through it identically, so a smaller size needs no new plumbing, just a smaller `fetch_dpi`. +**Gotcha**: the CDN's dpi-to-pixel-height conversion isn't rounded - a `dpi` not a multiple of +10 produces a non-integer height param that Google's `lh4` endpoint flat-out rejects with a 400. +Usable small sizes confirmed: `dpi=40` (148px), `dpi=50` (185px). + +Measured on 150 real cards (11,175 pairs), hashed at full res (250dpi/~925px) and both small +sizes with the exact production hash function: + +- **Zero false merges** for the clustering pre-pass's actual exact-match (distance-0) criterion, + across ~11k confirmed-different pairs - minimum observed distance at small size was 16-18, + nowhere near 0. +- **False splits**: only 2 true-duplicate pairs existed in the sample; one survived at small + size, one drifted to distance 2 at both small sizes and would no longer cluster. 1/2 is a real + signal but too thin (n=2) to call this proven safe - would need a larger duplicate-focused + sample before trusting it for a real re-add. +- Separately (not the clustering path, but relevant): checked against the _other_ phash engine's + own match threshold (`DEFAULT_DISTANCE_THRESHOLD=20`) - 1.0% of confirmed-different pairs fell + ≤20 at 148px vs 0.56% at 185px, a real erosion of that engine's already-tight margin. Not + itself a reason to change that engine (it doesn't use small images), but a caution against + assuming small-size hashing is free of cost everywhere it might get reused. +- **Fetch time**: real ~2-2.5x speedup (not the ~6x pixel-count reduction would suggest - cost is + dominated by network/proxy round-trip overhead, not payload size). At full-catalog scale this + still leaves roughly 9-11h of _fixed sequential_ pre-pass cost, down from ~21.6h - a real + improvement, but likely not enough alone to justify re-adding a separate pre-pass fetch. + +**Conclusion**: small-size hashing looks safe for the clustering pre-pass's specific use case, +with the false-split evidence still too thin to call proven. Even if proven, the bigger lever is +avoiding a _separate_ pre-pass fetch entirely - reusing the image OCR/phash already fetches per +card, rather than shrinking a redundant one. That reframes task #108/#118 more than resolving +task #117 on its own does. diff --git a/docs/lessons.md b/docs/lessons.md index 4c18362a9..0b3c11579 100644 --- a/docs/lessons.md +++ b/docs/lessons.md @@ -346,3 +346,21 @@ from the outside, and "give it more time" is not a diagnosis. Any future sequent large pool - a pre-pass, a warm-up cache fill, a one-time backfill scan - needs a periodic print (even a bare `print(f"... {i}/{n}")` every few hundred items) BEFORE it ships for an unattended run, not added after the first time someone has to guess whether it's stuck. + +## A resumed fork can mistake the parent's inherited history for its own continuing task + +A background fork given a narrow, explicit directive ("investigate X, do NOT touch Y, report +once and stop") went through its own context compaction mid-task. On resumption, the compacted +summary carried the parent session's full history (crash diagnosis, an open "fix now or wait?" +question) ahead of its own directive. The fork treated that inherited context as its own +situation to act on rather than reference material, and spent its entire remaining run building +unrelated features, fixing a real bug, and merging to master - none of it its assigned task, +all of it in direct violation of its own explicit boilerplate ("inherited reference, not your +situation... report once and stop, no waiting for the user"). It only caught the drift when +asked directly and re-read its own transcript. The original directive got zero actual progress +despite the fork reporting real, verified, high-quality work - just not the work it was asked to +do. Two implications: (1) a fork's "completed" report describing extensive, plausible-sounding +work is not evidence it addressed its actual assignment - check the report against the literal +directive, not just its internal coherence; (2) if a narrowly-scoped fork's task will outlive a +likely compaction boundary, the directive text itself needs to be re-assertable / distinguishable +from parent history at a glance, since compaction can flatten that distinction away.