From bcaaaae19070bea64d54e3ec6b9ed2f7635879e3 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Wed, 15 Jul 2026 18:44:52 +0000 Subject: [PATCH 01/23] Add per-source exclusion flags for OCR/phash selection, yield reconciliation, future-work note - --exclude-sources-ocr / --exclude-sources-phash flags, OCR defaults to excluding source pk=1 (WilfordGrimley), fully overridable, never hardcoded - reconcile old vs new logic yield on the same fresh 250-card window - log the 1,097-card (Front)/(Back) name-matching fix as future work --- .../local_identify_printing_tags.py | 25 ++++++-- .../commands/local_identify_printing_tags.py | 33 ++++++++++- .../test_local_identify_printing_tags.py | 58 +++++++++++++++++++ docs/features/printing-tags.md | 38 ++++++++++++ 4 files changed, 146 insertions(+), 8 deletions(-) diff --git a/MPCAutofill/cardpicker/local_identify_printing_tags.py b/MPCAutofill/cardpicker/local_identify_printing_tags.py index 73939a3c8..53f46bc34 100644 --- a/MPCAutofill/cardpicker/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/local_identify_printing_tags.py @@ -21,7 +21,7 @@ import time from dataclasses import dataclass, field from io import BytesIO -from typing import Literal, Optional +from typing import Iterable, Literal, Optional import requests from PIL import Image @@ -115,7 +115,7 @@ class SelectedCard: candidates: list[CandidatePrinting] -def _eligible_base_queryset(anonymous_id: str) -> "QuerySet[Card]": +def _eligible_base_queryset(anonymous_id: str, exclude_source_pks: Optional[Iterable[int]] = None) -> "QuerySet[Card]": """ unresolved, no confirmed indexing match, no existing vote from this engine's own anonymous_id (the idempotence/checkpoint mechanism - see module docstring and @@ -123,8 +123,12 @@ def _eligible_base_queryset(anonymous_id: str) -> "QuerySet[Card]": backfill (which is provably exact by construction where it applies - this pilot's engines are weaker, lower-confidence signal and shouldn't pile onto a card that already has a stronger deduction), and no resolved custom-art/non-english tag. + + exclude_source_pks is a purely mechanical, caller-supplied deprioritization knob (no source + pk is ever hardcoded here) - see select_candidates and the management command's + --exclude-sources-ocr/--exclude-sources-phash flags. """ - return ( + queryset = ( Card.objects.filter(printing_tag_status=PrintingTagStatus.UNRESOLVED, canonical_card__isnull=True) .exclude(printing_tags__anonymous_id=anonymous_id) .exclude(printing_tags__anonymous_id=DEDUCTIVE_BACKFILL_ANONYMOUS_ID) @@ -133,9 +137,14 @@ def _eligible_base_queryset(anonymous_id: str) -> "QuerySet[Card]": .distinct() .select_related("source") ) + if exclude_source_pks: + queryset = queryset.exclude(source_id__in=exclude_source_pks) + return queryset -def select_candidates(engine: Engine, index: Optional[CandidateNameIndex] = None) -> list[SelectedCard]: +def select_candidates( + engine: Engine, index: Optional[CandidateNameIndex] = None, exclude_source_pks: Optional[Iterable[int]] = None +) -> list[SelectedCard]: """Multi-candidate names first (the cases deductive backfill's D1/D2 tiers can't reach without an expansion_hint), then single-candidate names, in `Card.pk` order within each group for determinism.""" @@ -144,7 +153,7 @@ def select_candidates(engine: Engine, index: Optional[CandidateNameIndex] = None multi: list[SelectedCard] = [] single: list[SelectedCard] = [] for card in ( - _eligible_base_queryset(anonymous_id) + _eligible_base_queryset(anonymous_id, exclude_source_pks) .only("pk", "name", "identifier", "source_id") .order_by("pk") .iterator(chunk_size=5000) @@ -319,6 +328,7 @@ def run_pilot( phash_distance_threshold: int = local_phash.DEFAULT_DISTANCE_THRESHOLD, phash_margin: int = local_phash.DEFAULT_MARGIN, phash_max_candidates: int = PHASH_MAX_CANDIDATES, + exclude_source_pks_by_engine: Optional[dict[Engine, list[int]]] = None, ) -> tuple[dict[str, PilotResult], AttributeReport]: if nice: try: @@ -331,7 +341,10 @@ def run_pilot( results: dict[str, PilotResult] = {e: PilotResult(engine=e, dry_run=dry_run) for e in engines_to_run} results["fallback"] = PilotResult(engine="fallback", dry_run=dry_run) attributes = AttributeReport() - selected_by_engine = {e: select_candidates(e, index)[:limit] for e in engines_to_run} + exclude_source_pks_by_engine = exclude_source_pks_by_engine or {} + selected_by_engine = { + e: select_candidates(e, index, exclude_source_pks_by_engine.get(e))[:limit] for e in engines_to_run + } # when both engines run, process the union of cards either engine selected so agreement/ # disagreement can be evaluated per card - each engine still only ever votes on a card it diff --git a/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py b/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py index 1bddabb6f..3a49b4396 100644 --- a/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py @@ -4,7 +4,7 @@ from django.core.management.base import BaseCommand, CommandError from cardpicker import local_identify_printing_tags, local_ocr, local_phash -from cardpicker.local_identify_printing_tags import run_pilot +from cardpicker.local_identify_printing_tags import Engine, run_pilot class Command(BaseCommand): @@ -72,6 +72,24 @@ def add_arguments(self, parser: Any) -> None: "ordering would otherwise hit these first and fetch/hash all of them). Default: " f"{local_identify_printing_tags.PHASH_MAX_CANDIDATES}.", ) + parser.add_argument( + "--exclude-sources-ocr", + type=str, + default="1", + help="Comma-separated Source pks to deprioritize from OCR selection (their cards are " + "never selected as candidates by the OCR engine this invocation - existing votes/tags " + "are untouched, this is a selection-time filter only). Default: '1' (WilfordGrimley - " + "the OCR engine's own operator's source; excluded by default so a routine invocation " + "doesn't cast machine votes on the operator's own cards. Pass '' to include it.) " + "Pass '' for no exclusion.", + ) + parser.add_argument( + "--exclude-sources-phash", + type=str, + default="", + help="Comma-separated Source pks to deprioritize from phash selection. Same mechanism " + "as --exclude-sources-ocr, independently settable. Default: '' (no exclusion).", + ) def handle(self, *args: Any, **kwargs: Any) -> None: engine = kwargs["engine"] @@ -82,6 +100,14 @@ def handle(self, *args: Any, **kwargs: Any) -> None: crop_box_arg = kwargs["crop_box"] phash_max_candidates = kwargs["phash_max_candidates"] + def _parse_source_pks(raw: str) -> list[int]: + return [int(p) for p in raw.split(",") if p.strip()] + + exclude_source_pks_by_engine: dict[Engine, list[int]] = { + "ocr": _parse_source_pks(kwargs["exclude_sources_ocr"]), + "phash": _parse_source_pks(kwargs["exclude_sources_phash"]), + } + crop_box = local_ocr.DEFAULT_CROP_BOX if crop_box_arg is not None: parts = crop_box_arg.split(",") @@ -103,7 +129,9 @@ def handle(self, *args: Any, **kwargs: Any) -> None: mode = "DRY RUN" if dry_run else "WRITE" print( f"[{mode}] local_identify_printing_tags --engine={engine} --limit={limit} " - f"--nice={nice} --crop-box={crop_box}" + f"--nice={nice} --crop-box={crop_box} " + f"--exclude-sources-ocr={exclude_source_pks_by_engine['ocr']} " + f"--exclude-sources-phash={exclude_source_pks_by_engine['phash']}" ) results, attributes = run_pilot( @@ -115,6 +143,7 @@ def handle(self, *args: Any, **kwargs: Any) -> None: phash_distance_threshold=local_phash.DEFAULT_DISTANCE_THRESHOLD, phash_margin=local_phash.DEFAULT_MARGIN, phash_max_candidates=phash_max_candidates, + exclude_source_pks_by_engine=exclude_source_pks_by_engine, ) gate_violations: list[int] = [] diff --git a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py index 95283af8a..827259760 100644 --- a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py @@ -125,6 +125,33 @@ def test_multi_candidate_names_come_before_single_candidate_names(self, db): assert [s.card.pk for s in selected] == [multi.pk, single.pk] +class TestSourceExclusion: + def test_excludes_cards_from_a_given_source_pk(self, db): + excluded_source = SourceFactory() + included_source = SourceFactory() + CanonicalCardFactory(name="Forest") + excluded_card = CardFactory(name="Forest", source=excluded_source) + included_card = CardFactory(name="Forest", source=included_source) + + selected = select_candidates("ocr", exclude_source_pks=[excluded_source.pk]) + assert [s.card.pk for s in selected] == [included_card.pk] + assert excluded_card.pk not in [s.card.pk for s in selected] + + def test_no_exclusion_by_default(self, db): + source = SourceFactory() + CanonicalCardFactory(name="Forest") + card = CardFactory(name="Forest", source=source) + assert [s.card.pk for s in select_candidates("ocr")] == [card.pk] + + def test_exclusion_is_independent_per_engine(self, db): + source = SourceFactory() + CanonicalCardFactory(name="Forest") + card = CardFactory(name="Forest", source=source) + + assert select_candidates("ocr", exclude_source_pks=[source.pk]) == [] + assert [s.card.pk for s in select_candidates("phash", exclude_source_pks=[])] == [card.pk] + + class TestCandidateNameIndex: def test_groups_by_normalised_name(self, db): CanonicalCardFactory(name="Kusari-Gama") @@ -420,6 +447,37 @@ def test_only_requested_engine_appears_in_results(self, db, monkeypatch): assert set(results.keys()) == {"ocr", "fallback"} +class TestRunPilotSourceExclusion: + def test_excluded_sources_cards_never_reach_the_engine(self, db, monkeypatch): + excluded_source = SourceFactory() + printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="bbb")) + excluded_card = CardFactory(name="Forest", source=excluded_source) + + import cardpicker.local_identify_printing_tags as module + + def fake_ocr(selected, image, crop_box): + return module.OcrCardResult( + vote=module.EngineVote(engine="ocr", printing_pk=printing.pk, confidence=0.85, detail="raw") + ) + + monkeypatch.setattr(module, "run_ocr_for_card", fake_ocr) + monkeypatch.setattr(module, "fetch_card_image", lambda card: None) + + results, _attributes = run_pilot( + engine="ocr", + limit=10, + dry_run=False, + nice=False, + exclude_source_pks_by_engine={"ocr": [excluded_source.pk]}, + ) + + assert results["ocr"].votes_written == 0 + from cardpicker.models import CardPrintingTag + + assert not CardPrintingTag.objects.filter(card=excluded_card).exists() + + class TestIdempotence: def test_a_card_voted_on_is_excluded_from_the_next_selection(self, db, monkeypatch): printing = CanonicalCardFactory(name="Forest") diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index cf46058a9..23bf2b465 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -837,6 +837,29 @@ now match.** Projected full-engine impact: OCR yield 77/300 (25.7%) → Confirmed live via a real (non-simulated) `--dry-run` afterward: 62/250 votes on a fresh selection window, consistent with the isolated measurement. +**Yield reconciliation, old logic vs. new logic on that same 250-card +window** (no new OCR work run for this - reusing already-known numbers): +selection-order stability means the 250-card window is the 223-card +reconstructed cohort above (still eligible - no vote was ever cast on a +skip) plus 27 cards never seen in the original 300-card pilot run at all. +Old-logic yield on the 223 known cards is **measured, not estimated**: 3 +(the "already-matching" cards, unaffected by the fix) out of 223 - the +other 220 are old-logic non-matches by definition of how they were +classified as skips. Old-logic yield on the 27 unseen cards is **not +measured** - no new work was done to classify them - and is instead +estimated by applying the original pilot's overall old-logic OCR base +rate (77/300, 25.7%) to that count: 27 × 0.257 ≈ 7. Combined old-logic +estimate: (3 + ~7)/250 ≈ 10/250 ≈ **4.0%**, against the confirmed +new-logic **24.8%** (62/250) on the identical window - roughly a 6x +relative lift here, well above the pilot-set's ~1.6x (60% relative) +projection, because this window is disproportionately drawn from cards +that were old-logic failures by construction (the 223-card skip cohort), +not a representative sample of the full catalog. Treat the 24.8%-vs-4.0% +comparison as the honest floor-to-floor number on hard cases, and the +41.3%-vs-25.7% pilot-set figures as the representative full-run +projection - they are not the same statistic and should not be quoted +interchangeably. + Of the 129 cases still unfixed: only 2/176 (1.1%) are genuinely-missing printings (the parsed set code is real, but no `CanonicalCard` row exists for that (set, number) at all); the remaining 127/176 (72.2%) are true @@ -945,3 +968,18 @@ implied by this OCR fix and was not built. (no such model exists today), and a way to feed that score back into `vote_consensus`'s per-source weighting — worth its own design pass rather than bolting onto an existing stage. +- **Future work: `(Front)`/`(Back)` name-matching fix for the + `expansion_hint` census gap** (2026-07-15, deferred out of the pre-scale + program by owner decision — deterministic parser fix, not part of + Stage 8's OCR/phash engines). The 1,097-card filename tag-gap census + (cards with an unmatchable `expansion_hint`, all with a fully + _recognized_ `CanonicalExpansion` code) was cross-checked against the + Stage 8 no-match autopsy above and confirmed to be a **different root + cause** — a name-matching problem, not the OCR set-code-position/ + leading-zero bugs the autopsy fixed. Many of the 1,097 are + `(Front)`/`(Back)` filename-parsing artifacts on basic lands (a + double-faced-card naming convention this catalog's name-matching + doesn't strip before comparing against `CanonicalCard.name`). Belongs + with `cardpicker.deductive_backfill`'s deterministic tiers (D1/D2), not + Stage 8's visual-disambiguation engines — explicitly not the "D2.5 + arriving for free" the autopsy's cross-check ruled out. From 74bd0eae7b317b15d17de5fd884da24d7960efce Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Wed, 15 Jul 2026 18:48:02 +0000 Subject: [PATCH 02/23] Test the --exclude-sources CLI argparse defaults, not just the library function --- .../test_local_identify_printing_tags.py | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py index 827259760..d586e49d9 100644 --- a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py @@ -152,6 +152,38 @@ def test_exclusion_is_independent_per_engine(self, db): assert [s.card.pk for s in select_candidates("phash", exclude_source_pks=[])] == [card.pk] +class TestManagementCommandExclusionDefaults: + """The library-level tests above always pass exclude_source_pks explicitly - none of them + touch the CLI's own --exclude-sources-ocr/--exclude-sources-phash argparse defaults. This + guards the actual thing Rider 2 was for: a bare invocation excludes source pk=1 from OCR + (and only OCR) without the operator having to remember the flag.""" + + def test_bare_invocation_defaults_to_excluding_source_pk_1_for_ocr_only(self, db, capsys): + from django.core.management import call_command + + call_command("local_identify_printing_tags", "--dry-run", "--limit", "0") + printed = capsys.readouterr().out + assert "--exclude-sources-ocr=[1]" in printed + assert "--exclude-sources-phash=[]" in printed + + def test_explicit_flag_overrides_the_default(self, db, capsys): + from django.core.management import call_command + + call_command( + "local_identify_printing_tags", + "--dry-run", + "--limit", + "0", + "--exclude-sources-ocr", + "", + "--exclude-sources-phash", + "2,3", + ) + printed = capsys.readouterr().out + assert "--exclude-sources-ocr=[]" in printed + assert "--exclude-sources-phash=[2, 3]" in printed + + class TestCandidateNameIndex: def test_groups_by_normalised_name(self, db): CanonicalCardFactory(name="Kusari-Gama") From 1f4d2585d037dd8ca3d59a591574ac5a154f6d67 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Wed, 15 Jul 2026 18:58:26 +0000 Subject: [PATCH 03/23] Add periodic-flush checkpointing to run_pilot, per-batch gate checking --- .../local_identify_printing_tags.py | 87 +++++++++++---- .../commands/local_identify_printing_tags.py | 14 ++- .../test_local_identify_printing_tags.py | 100 ++++++++++++++++++ docs/features/printing-tags.md | 34 ++++-- 4 files changed, 205 insertions(+), 30 deletions(-) diff --git a/MPCAutofill/cardpicker/local_identify_printing_tags.py b/MPCAutofill/cardpicker/local_identify_printing_tags.py index 53f46bc34..42073855c 100644 --- a/MPCAutofill/cardpicker/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/local_identify_printing_tags.py @@ -67,6 +67,13 @@ # out of scope for this pilot. PHASH_MAX_CANDIDATES = 12 +# Checkpointing (Stage 8 pre-scale program item 2, see run_pilot): flush every this many cards +# processed. Deliberately much smaller than deductive_backfill's batch_size=2000 - that pipeline +# is pure DB writes with no per-card network fetch/OCR/phash cost, so losing an un-flushed batch +# to a crash is cheap there; here each card costs a real image fetch plus OCR/phash CPU work, so +# a smaller batch bounds how much re-fetchable-but-not-yet-durable work a kill can waste. +DEFAULT_BATCH_SIZE = 25 + # cardpicker.reason_tags.NO_MATCH_REASON_TAGS - a resolved custom-art/non-english tag already # tells us the PRINCIPLE's precondition (an authentic depiction of a real printing) is false, # same exclusion rationale as cardpicker.deductive_backfill's "Custom" tag check, just against @@ -329,6 +336,8 @@ def run_pilot( phash_margin: int = local_phash.DEFAULT_MARGIN, phash_max_candidates: int = PHASH_MAX_CANDIDATES, exclude_source_pks_by_engine: Optional[dict[Engine, list[int]]] = None, + batch_size: int = DEFAULT_BATCH_SIZE, + progress_every: int = 50, ) -> tuple[dict[str, PilotResult], AttributeReport]: if nice: try: @@ -363,14 +372,41 @@ def run_pilot( ).values_list("card_id", flat=True) ) - outcomes: dict[int, CardOutcome] = {} - written_card_ids: list[int] = [] - votes_to_create: list[CardPrintingTag] = [] - tag_votes_to_create: list[CardTagVote] = [] - ocr_selected_ids = {s.card.pk for s in selected_by_engine.get("ocr", [])} phash_selected_ids = {s.card.pk for s in selected_by_engine.get("phash", [])} + # Checkpointing (Stage 8 pre-scale program item 2): a multi-day unattended run must survive + # a kill without losing everything accumulated since the last flush. Matches + # cardpicker.deductive_backfill.run_backfill's periodic-flush pattern (a plain re-invocation + # resumes cleanly with no separate checkpoint file, since select_candidates already excludes + # any card with an existing vote from this engine's own anonymous_id), but deliberately + # DIVERGES from that precedent on ONE point: the gate check runs after every flush here, not + # once at the very end. deductive_backfill's votes are provably exact by construction (a gate + # violation there is structurally impossible), so a single end-of-run check is just belt-and- + # suspenders; this pilot's OCR/phash/fallback votes are explicitly weaker, lower-confidence + # signal (module docstring) where a real violation is more plausible, and a kill is an + # EXPECTED event for a multi-day run (the whole reason this checkpointing exists) - a + # violation in an already-flushed batch must not sit undetected in the DB indefinitely just + # because the process died before reaching the final check. + written_card_ids: list[int] = [] + all_gate_violations: list[int] = [] + votes_batch: list[CardPrintingTag] = [] + tag_votes_batch: list[CardTagVote] = [] + batch_written_card_ids: list[int] = [] + + def flush() -> None: + nonlocal votes_batch, tag_votes_batch, batch_written_card_ids + if dry_run: + votes_batch, tag_votes_batch, batch_written_card_ids = [], [], [] + return + if tag_votes_batch: + CardTagVote.objects.bulk_create(tag_votes_batch, ignore_conflicts=True) + if votes_batch: + CardPrintingTag.objects.bulk_create(votes_batch) + if batch_written_card_ids: + all_gate_violations.extend(verify_zero_resolutions(batch_written_card_ids)) + votes_batch, tag_votes_batch, batch_written_card_ids = [], [], [] + for i, (card_id, selected) in enumerate(all_selected_by_card_id.items()): outcome = CardOutcome(card_id=card_id) image = fetch_card_image(selected.card) # shared across every engine that runs on this card @@ -422,12 +458,12 @@ def run_pilot( detail=",".join(fallback_outcome.evidence_types_used), ) - outcomes[card_id] = outcome - - if nice and i % 20 == 0: - time.sleep(_NICE_SLEEP_SECONDS) - - for card_id, outcome in outcomes.items(): + # Finalize + queue for write - inlined here (rather than a second pass over a + # dict[int, CardOutcome] collected above) so a card's full cost (image fetch, OCR, + # phash, fallback) is only ever paid once before its result reaches the write batch; + # nothing here depends on any OTHER card's outcome, only this card's own DB state + # (the frame-mismatch consistency check below re-queries the matched printing's own + # metadata, independent of processing order). result_ocr = results.get("ocr") result_phash = results.get("phash") result_fallback = results["fallback"] @@ -469,7 +505,7 @@ def run_pilot( if printing_vote_withheld_for_frame_mismatch: result_ocr.skip_counts["frame-mismatch"] += 1 else: - votes_to_create.append( + votes_batch.append( CardPrintingTag( card_id=card_id, printing_id=outcome.ocr_vote.printing_pk, @@ -482,6 +518,7 @@ def run_pilot( result_ocr.votes_written += 1 result_ocr.audit.append({"card_id": card_id, "raw_text": outcome.ocr_vote.detail}) written_card_ids.append(card_id) + batch_written_card_ids.append(card_id) elif outcome.ocr_skip_reason and result_ocr is not None: result_ocr.skip_counts[outcome.ocr_skip_reason] += 1 @@ -489,7 +526,7 @@ def run_pilot( if printing_vote_withheld_for_frame_mismatch: result_phash.skip_counts["frame-mismatch"] += 1 else: - votes_to_create.append( + votes_batch.append( CardPrintingTag( card_id=card_id, printing_id=outcome.phash_vote.printing_pk, @@ -503,6 +540,7 @@ def run_pilot( result_phash.audit.append({"card_id": card_id, "detail": outcome.phash_vote.detail}) if card_id not in written_card_ids: written_card_ids.append(card_id) + batch_written_card_ids.append(card_id) elif outcome.phash_skip_reason and result_phash is not None: result_phash.skip_counts[outcome.phash_skip_reason] += 1 @@ -510,7 +548,7 @@ def run_pilot( if printing_vote_withheld_for_frame_mismatch: result_fallback.skip_counts["frame-mismatch"] += 1 else: - votes_to_create.append( + votes_batch.append( CardPrintingTag( card_id=card_id, printing_id=outcome.fallback_vote.printing_pk, @@ -524,6 +562,7 @@ def run_pilot( result_fallback.audit.append({"card_id": card_id, "evidence": outcome.fallback_vote.detail}) if card_id not in written_card_ids: written_card_ids.append(card_id) + batch_written_card_ids.append(card_id) elif outcome.fallback_skip_reason: result_fallback.skip_counts[outcome.fallback_skip_reason] += 1 @@ -568,7 +607,7 @@ def run_pilot( attributes.border_votes_by_class[border_class] += 1 border_vote = local_fallback.cast_border_attribute_vote(card, border_class, confidence=border_confidence) if border_vote is not None and not dry_run: - tag_votes_to_create.append(border_vote) + tag_votes_batch.append(border_vote) frame_class = outcome.frame_class frame_confidence = local_fallback.FRAME_VOTE_CONFIDENCE @@ -584,18 +623,22 @@ def run_pilot( attributes.frame_votes_by_class[frame_class] += 1 frame_vote = local_fallback.cast_frame_style_vote(card, frame_class, confidence=frame_confidence) if frame_vote is not None and not dry_run: - tag_votes_to_create.append(frame_vote) + tag_votes_batch.append(frame_vote) else: attributes.frame_abstain_count += 1 - if not dry_run and tag_votes_to_create: - CardTagVote.objects.bulk_create(tag_votes_to_create, ignore_conflicts=True) + if (i + 1) % batch_size == 0: + flush() + if nice and i % 20 == 0: + time.sleep(_NICE_SLEEP_SECONDS) + if progress_every and (i + 1) % progress_every == 0: + print(f" ... {i + 1}/{len(all_selected_by_card_id)} candidates processed") + + flush() - if not dry_run and votes_to_create: - CardPrintingTag.objects.bulk_create(votes_to_create) - gate_violations = verify_zero_resolutions(written_card_ids) + if not dry_run: for result in results.values(): - result.gate_violations = gate_violations + result.gate_violations = all_gate_violations return results, attributes diff --git a/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py b/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py index 3a49b4396..44c805c33 100644 --- a/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py @@ -90,6 +90,16 @@ def add_arguments(self, parser: Any) -> None: help="Comma-separated Source pks to deprioritize from phash selection. Same mechanism " "as --exclude-sources-ocr, independently settable. Default: '' (no exclusion).", ) + parser.add_argument( + "--batch-size", + type=int, + default=local_identify_printing_tags.DEFAULT_BATCH_SIZE, + help="Flush votes/tags to the DB (and run the gate check) every this many cards " + "processed, instead of one giant write at the very end - so a killed/interrupted " + "run keeps whatever it already committed (a plain re-invocation resumes cleanly, " + "same idempotence mechanism as --resume). Default: " + f"{local_identify_printing_tags.DEFAULT_BATCH_SIZE}.", + ) def handle(self, *args: Any, **kwargs: Any) -> None: engine = kwargs["engine"] @@ -99,6 +109,7 @@ def handle(self, *args: Any, **kwargs: Any) -> None: nice = kwargs["nice"] crop_box_arg = kwargs["crop_box"] phash_max_candidates = kwargs["phash_max_candidates"] + batch_size = kwargs["batch_size"] def _parse_source_pks(raw: str) -> list[int]: return [int(p) for p in raw.split(",") if p.strip()] @@ -129,7 +140,7 @@ def _parse_source_pks(raw: str) -> list[int]: mode = "DRY RUN" if dry_run else "WRITE" print( f"[{mode}] local_identify_printing_tags --engine={engine} --limit={limit} " - f"--nice={nice} --crop-box={crop_box} " + f"--nice={nice} --crop-box={crop_box} --batch-size={batch_size} " f"--exclude-sources-ocr={exclude_source_pks_by_engine['ocr']} " f"--exclude-sources-phash={exclude_source_pks_by_engine['phash']}" ) @@ -144,6 +155,7 @@ def _parse_source_pks(raw: str) -> list[int]: phash_margin=local_phash.DEFAULT_MARGIN, phash_max_candidates=phash_max_candidates, exclude_source_pks_by_engine=exclude_source_pks_by_engine, + batch_size=batch_size, ) gate_violations: list[int] = [] diff --git a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py index d586e49d9..4721ba1af 100644 --- a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py @@ -510,6 +510,106 @@ def fake_ocr(selected, image, crop_box): assert not CardPrintingTag.objects.filter(card=excluded_card).exists() +class TestCheckpointing: + """Stage 8 pre-scale program item 2: run_pilot must survive a kill mid-run without losing + everything accumulated since the last flush, matching cardpicker.deductive_backfill's + periodic-flush precedent (see run_pilot's checkpointing comment for the one deliberate + deviation - the gate check runs per-flush here, not once at the end).""" + + @staticmethod + def _wire_fake_ocr(monkeypatch, printing_pk): + import cardpicker.local_identify_printing_tags as module + + def fake_ocr(selected, image, crop_box): + return module.OcrCardResult( + vote=module.EngineVote(engine="ocr", printing_pk=printing_pk, confidence=0.85, detail="raw") + ) + + monkeypatch.setattr(module, "run_ocr_for_card", fake_ocr) + monkeypatch.setattr(module, "fetch_card_image", lambda card: None) + + def test_flushes_periodically_not_just_once_at_the_end(self, db, monkeypatch): + printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + for _ in range(5): + CardFactory(name="Forest") + self._wire_fake_ocr(monkeypatch, printing.pk) + + bulk_create_calls: list[int] = [] + original_bulk_create = CardPrintingTag.objects.bulk_create + + def counting_bulk_create(objs, *args, **kwargs): + objs = list(objs) + bulk_create_calls.append(len(objs)) + return original_bulk_create(objs, *args, **kwargs) + + monkeypatch.setattr(CardPrintingTag.objects, "bulk_create", counting_bulk_create) + + results, _attributes = run_pilot(engine="ocr", limit=10, dry_run=False, nice=False, batch_size=2) + + assert results["ocr"].votes_written == 5 + # 5 cards at batch_size=2: flush after card 2, after card 4, and once more for the + # trailing single card - three separate writes, not one giant write at the very end. + assert bulk_create_calls == [2, 2, 1] + + def test_gate_check_runs_per_flush_not_only_at_the_end(self, db, monkeypatch): + printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + for _ in range(4): + CardFactory(name="Forest") + self._wire_fake_ocr(monkeypatch, printing.pk) + + import cardpicker.local_identify_printing_tags as module + + verify_calls: list[list[int]] = [] + original_verify = module.verify_zero_resolutions + + def counting_verify(card_ids, *args, **kwargs): + verify_calls.append(list(card_ids)) + return original_verify(card_ids, *args, **kwargs) + + monkeypatch.setattr(module, "verify_zero_resolutions", counting_verify) + + run_pilot(engine="ocr", limit=10, dry_run=False, nice=False, batch_size=2) + + # 4 cards at batch_size=2: two flushes, each with its own gate check - not one call at + # the very end covering all 4. + assert len(verify_calls) == 2 + assert all(len(c) == 2 for c in verify_calls) + + def test_resume_after_a_simulated_kill_completes_the_remaining_cards(self, db, monkeypatch): + printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + cards = [CardFactory(name="Forest") for _ in range(6)] + self._wire_fake_ocr(monkeypatch, printing.pk) + + class SimulatedKill(Exception): + pass + + original_bulk_create = CardPrintingTag.objects.bulk_create + call_count = {"n": 0} + + def killing_bulk_create(objs, *args, **kwargs): + call_count["n"] += 1 + result = original_bulk_create(objs, *args, **kwargs) + if call_count["n"] == 1: + raise SimulatedKill("process died immediately after the first flush committed") + return result + + monkeypatch.setattr(CardPrintingTag.objects, "bulk_create", killing_bulk_create) + + with pytest.raises(SimulatedKill): + run_pilot(engine="ocr", limit=10, dry_run=False, nice=False, batch_size=2) + + # the first flush's 2 cards are durably committed despite the "crash" on the next batch + assert CardPrintingTag.objects.filter(anonymous_id=OCR_ANONYMOUS_ID).count() == 2 + + monkeypatch.setattr(CardPrintingTag.objects, "bulk_create", original_bulk_create) + results, _attributes = run_pilot(engine="ocr", limit=10, dry_run=False, nice=False, batch_size=2) + + assert results["ocr"].votes_written == 4 # the 4 cards the killed run never reached + final_votes = CardPrintingTag.objects.filter(anonymous_id=OCR_ANONYMOUS_ID) + assert final_votes.count() == 6 + assert {v.card_id for v in final_votes} == {c.pk for c in cards} + + class TestIdempotence: def test_a_card_voted_on_is_excluded_from_the_next_selection(self, db, monkeypatch): printing = CanonicalCardFactory(name="Forest") diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index 23bf2b465..58d48e0ba 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -794,13 +794,33 @@ eligible pool) → naive linear projection ≈ 306 hours ≈ 12.8 days of continuous single-process runtime.** This is the key number for any future decision to scale up - not attempted in this pilot, and not practical as a single uninterrupted process. Before attempting it: -parallelizing across multiple processes/pk-range partitions, and (more -urgently) switching from the current one-giant-`bulk_create`-at-the-end -write pattern to periodic batch flushing (matching -`deductive_backfill.py`'s existing `batch_size`/`flush()` precedent) so a -multi-day run's progress survives a crash/restart/deploy instead of -losing everything accumulated since the last completed run - both raised -but not implemented in this pilot, out of its locked scope. +parallelizing across multiple processes/pk-range partitions (raised, not +yet implemented - see the pre-scale program's scaling proposal below). + +### Checkpointing (2026-07-15, pre-scale program item 2) + +`run_pilot` no longer does one giant `bulk_create` at the very end - +matches `deductive_backfill.py`'s periodic-flush precedent (`--batch-size`, +default 25 cards - much smaller than `deductive_backfill`'s 2000, since +each card here costs a real image fetch plus OCR/phash CPU work, not just +a DB write). A killed/interrupted run keeps whatever it already flushed; +a plain re-invocation resumes cleanly with no separate checkpoint file, +via the same `select_candidates` idempotence mechanism `--resume` already +relied on. Verified live in tests (`TestCheckpointing`, not just +plausible): flushes happen every `--batch-size` cards, a simulated kill +mid-run leaves the already-flushed cards durably committed and a +follow-up invocation completes exactly the remainder with no duplicates. + +**One deliberate deviation from `deductive_backfill`'s pattern**: the gate +check (`verify_zero_resolutions`) now runs after every flush, not once at +the end. `deductive_backfill`'s votes are provably exact by construction +(a violation there is structurally impossible, so one end-of-run check is +belt-and-suspenders); this pilot's OCR/phash/fallback votes are explicitly +weaker, lower-confidence signal where a real violation is more plausible, +and a kill is now an _expected_ event for a multi-day run - a violation in +an already-flushed batch must not sit undetected in the DB indefinitely +just because the process died before reaching a final check that may +never come. 5-vote spot check, 20-vote random admin-link sample, 3 disagreement examples, and the filename tag-gap census (1,097 unresolved cards with an From 6a8c8cbf2c96551f061d753bca7a42d877c53925 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Wed, 15 Jul 2026 19:04:36 +0000 Subject: [PATCH 04/23] Document Stage 8 phase timing breakdown (item 3a) --- docs/features/printing-tags.md | 42 ++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index 58d48e0ba..710c8a027 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -829,6 +829,48 @@ unmatchable `expansion_hint`) are all in the journal, not duplicated here. **Pilot discipline honored**: `--limit 300`, no full-catalog run attempted per the original hold. +### Phase timing (2026-07-15, pre-scale program item 3a) + +Measured against real production data (read-only, no writes) via two +instrumented 30-card samples run through the actual pipeline functions, +not simulated. First sample (OCR + phash only) undercounted real cost - +`run_pilot` also runs border/frame classification and pass-2 fallback for +every card with an image, regardless of pass-1 outcome. Second sample +matched `run_pilot`'s real per-card call sequence exactly: + +| phase | mean/card | share of measured total | +| ------------------------------- | --------: | ----------------------: | +| `detect_illus_anchor` | 1.466s | 33.0% | +| pass-2 fallback (fires ~70%) | 1.474s\* | 23.2% | +| `fetch_card_image` | 1.187s | 26.6% | +| OCR (crop+preprocess+tesseract) | 0.602s | 13.5% | +| `classify_border_color` | 0.159s | 3.6% | +| phash (hash+compare) | 0.011s | 0.2% | + +\*mean over the 21/30 cards it actually fired on; contributes 0 for the +other 9. + +**Measured total: 4.46s/card** (sum of the above), against the real +300-card pilot run's **observed 6.42s/card** (32m4.6s / 300) - a ~2s/card +gap not fully attributed by this instrumented sample, plausibly per-card +DB queries this sample didn't isolate (the frame-mismatch consistency +check and ground-truth-metadata lookup each re-query `CanonicalCard` once +per confirmed vote) and/or run-to-run network/cache variance (different +selection window, different Scryfall/CDN load). Treat 6.42s/card as the +trustworthy full-pipeline number and the phase breakdown above as +directional (which phases dominate), not a component-by-component +reconciliation. + +**`detect_illus_anchor` is the single largest cost, and it's partly +redundant with the main OCR pass**: when pass 1's OCR text doesn't +already contain the "Illus." artist credit, it runs its OWN +crop+preprocess+tesseract pass (a second full OCR call per card, on a +different crop) purely to extract the artist name for pass-2 evidence +and the frame-style classifier's illus-anchor signal. This is a real +optimization target flagged for a future pass, not fixed here - the +addendum ledger closed on ideas beyond items 1-8, and this wasn't one of +them. + ### No-match autopsy (2026-07-15, post-merge Hold #1 of the pre-scale program) Classified all 176 OCR "parsed-but-no-match" cases from the pilot run From 8559eacb63789099ec03f748696d051810a53659 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Wed, 15 Jul 2026 19:14:45 +0000 Subject: [PATCH 05/23] Item 3b: check CDN tier premise against real Worker source, add --fetch-budget --- .../local_identify_printing_tags.py | 30 +++++++- .../commands/local_identify_printing_tags.py | 21 ++++++ .../test_local_identify_printing_tags.py | 41 +++++++++++ docs/features/printing-tags.md | 73 +++++++++++++++++++ 4 files changed, 163 insertions(+), 2 deletions(-) diff --git a/MPCAutofill/cardpicker/local_identify_printing_tags.py b/MPCAutofill/cardpicker/local_identify_printing_tags.py index 42073855c..32fba7176 100644 --- a/MPCAutofill/cardpicker/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/local_identify_printing_tags.py @@ -307,6 +307,8 @@ class PilotResult: disagreements: list[dict[str, object]] = field(default_factory=list) audit: list[dict[str, object]] = field(default_factory=list) # per-card checkpoint detail gate_violations: list[int] = field(default_factory=list) + fetch_budget_exhausted: bool = False + cards_not_attempted_this_invocation: int = 0 @dataclass @@ -338,6 +340,7 @@ def run_pilot( exclude_source_pks_by_engine: Optional[dict[Engine, list[int]]] = None, batch_size: int = DEFAULT_BATCH_SIZE, progress_every: int = 50, + fetch_budget: Optional[int] = None, ) -> tuple[dict[str, PilotResult], AttributeReport]: if nice: try: @@ -407,8 +410,28 @@ def flush() -> None: all_gate_violations.extend(verify_zero_resolutions(batch_written_card_ids)) votes_batch, tag_votes_batch, batch_written_card_ids = [], [], [] + # Fetch budget (pre-scale program item 3b): every image fetch is one request against the + # image CDN Worker, which shares its daily request quota with live site traffic + # (docs/features/image-cdn.md) - an unattended multi-hour pilot slice must not be able to + # consume an unbounded share of that shared budget. Counts only requests actually sent + # (get_worker_image_url returning None - an unsupported source type - never reaches the + # network at all, so it doesn't count). On exhaustion, the run stops cleanly: whatever's + # already been flushed stays committed, and every card not yet reached is left completely + # untouched (no vote, no outcome recorded) so the next invocation's selection query picks + # them up fresh with no special resume handling needed. + fetches_made = 0 + budget_exhausted = False + cards_attempted = 0 + for i, (card_id, selected) in enumerate(all_selected_by_card_id.items()): + if fetch_budget is not None and fetches_made >= fetch_budget: + budget_exhausted = True + break + cards_attempted += 1 + outcome = CardOutcome(card_id=card_id) + if get_worker_image_url(selected.card) is not None: + fetches_made += 1 image = fetch_card_image(selected.card) # shared across every engine that runs on this card ocr_raw_texts: list[str] = [] @@ -636,9 +659,12 @@ def flush() -> None: flush() - if not dry_run: - for result in results.values(): + cards_not_attempted = len(all_selected_by_card_id) - cards_attempted + for result in results.values(): + if not dry_run: result.gate_violations = all_gate_violations + result.fetch_budget_exhausted = budget_exhausted + result.cards_not_attempted_this_invocation = cards_not_attempted return results, attributes diff --git a/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py b/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py index 44c805c33..183ec3167 100644 --- a/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py @@ -100,6 +100,16 @@ def add_arguments(self, parser: Any) -> None: "same idempotence mechanism as --resume). Default: " f"{local_identify_printing_tags.DEFAULT_BATCH_SIZE}.", ) + parser.add_argument( + "--fetch-budget", + type=int, + default=None, + help="Cap the number of image CDN Worker requests this invocation will make (every " + "fetched card costs one - the Worker's daily request quota is SHARED with live site " + "traffic, docs/features/image-cdn.md). On exhaustion the run stops cleanly: whatever " + "was already flushed stays committed, and untouched cards are picked up fresh by the " + "next invocation - no special resume handling needed. Default: no limit.", + ) def handle(self, *args: Any, **kwargs: Any) -> None: engine = kwargs["engine"] @@ -110,6 +120,7 @@ def handle(self, *args: Any, **kwargs: Any) -> None: crop_box_arg = kwargs["crop_box"] phash_max_candidates = kwargs["phash_max_candidates"] batch_size = kwargs["batch_size"] + fetch_budget = kwargs["fetch_budget"] def _parse_source_pks(raw: str) -> list[int]: return [int(p) for p in raw.split(",") if p.strip()] @@ -141,6 +152,7 @@ def _parse_source_pks(raw: str) -> list[int]: print( f"[{mode}] local_identify_printing_tags --engine={engine} --limit={limit} " f"--nice={nice} --crop-box={crop_box} --batch-size={batch_size} " + f"--fetch-budget={fetch_budget} " f"--exclude-sources-ocr={exclude_source_pks_by_engine['ocr']} " f"--exclude-sources-phash={exclude_source_pks_by_engine['phash']}" ) @@ -156,6 +168,7 @@ def _parse_source_pks(raw: str) -> list[int]: phash_max_candidates=phash_max_candidates, exclude_source_pks_by_engine=exclude_source_pks_by_engine, batch_size=batch_size, + fetch_budget=fetch_budget, ) gate_violations: list[int] = [] @@ -166,6 +179,14 @@ def _parse_source_pks(raw: str) -> list[int]: print(f" skipped ({reason}): {count}") gate_violations = result.gate_violations + any_result = next(iter(results.values()), None) + if any_result is not None and any_result.fetch_budget_exhausted: + print( + f"[FETCH BUDGET EXHAUSTED] stopped after --fetch-budget={fetch_budget} requests - " + f"{any_result.cards_not_attempted_this_invocation} card(s) not attempted this " + "invocation, untouched (no vote/outcome recorded) - re-run to pick them up." + ) + print("--- attributes ---") print( f" border votes: {dict(attributes.border_votes_by_class)} (ground truth: {attributes.border_ground_truth_count})" diff --git a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py index 4721ba1af..3670bb0d2 100644 --- a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py @@ -610,6 +610,47 @@ def killing_bulk_create(objs, *args, **kwargs): assert {v.card_id for v in final_votes} == {c.pk for c in cards} +class TestFetchBudget: + """Stage 8 pre-scale program item 3b: every image fetch is one request against the shared + image CDN Worker quota - an unattended run must be boundable. Cards past the budget must be + left completely untouched (no vote/outcome), not skipped-and-recorded, so the next + invocation's ordinary idempotent selection just picks them up.""" + + def test_stops_after_the_budget_and_leaves_the_rest_untouched(self, db, monkeypatch): + printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + cards = [CardFactory(name="Forest") for _ in range(5)] + TestCheckpointing._wire_fake_ocr(monkeypatch, printing.pk) + + results, _attributes = run_pilot(engine="ocr", limit=10, dry_run=False, nice=False, fetch_budget=3) + + assert results["ocr"].votes_written == 3 + assert results["ocr"].fetch_budget_exhausted is True + assert results["ocr"].cards_not_attempted_this_invocation == 2 + + # the 2 untouched cards have no vote at all - a follow-up invocation with no budget + # limit picks them up via the ordinary idempotent selection, no special handling needed + remaining = select_candidates("ocr") + assert len(remaining) == 2 + results_2, _ = run_pilot(engine="ocr", limit=10, dry_run=False, nice=False) + assert results_2["ocr"].votes_written == 2 + assert CardPrintingTag.objects.filter(anonymous_id=OCR_ANONYMOUS_ID).count() == 5 + assert {c.pk for c in cards} == { + t.card_id for t in CardPrintingTag.objects.filter(anonymous_id=OCR_ANONYMOUS_ID) + } + + def test_no_budget_means_no_limit(self, db, monkeypatch): + printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + for _ in range(4): + CardFactory(name="Forest") + TestCheckpointing._wire_fake_ocr(monkeypatch, printing.pk) + + results, _attributes = run_pilot(engine="ocr", limit=10, dry_run=False, nice=False, fetch_budget=None) + + assert results["ocr"].votes_written == 4 + assert results["ocr"].fetch_budget_exhausted is False + assert results["ocr"].cards_not_attempted_this_invocation == 0 + + class TestIdempotence: def test_a_card_voted_on_is_excluded_from_the_next_selection(self, db, monkeypatch): printing = CanonicalCardFactory(name="Forest") diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index 710c8a027..7f6a2cf6e 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -871,6 +871,79 @@ optimization target flagged for a future pass, not fixed here - the addendum ledger closed on ideas beyond items 1-8, and this wasn't one of them. +### CDN fetching + Worker quota (2026-07-15, pre-scale program item 3b) + +**The premise "CDN-first fetching" was built on turned out to be wrong, +checked against the actual Worker source (`image-cdn/src/handler/image.ts`, +`R2Service.ts`) before implementing anything.** The pilot's +`get_worker_image_url` requests the `full` tier (matching the PDF export +path, for print-quality output) - and the `full` tier is a **pure +passthrough**: `fetch(url)` straight to Google Drive, every single +request, with zero R2 involvement. Only the `small`/`large` tiers go +through `R2Service.getThumbnail`'s cache-check-then-populate-on-miss +logic. There is no bucket to be "first" about in the pilot's current flow + +- it was never touching one. + +**Checked whether switching tiers would help anyway - real measurement, +not assumption.** If the pilot switched to the `large` tier (800px, +R2-cacheable), would it benefit from a warm cache? Fetched 20 real +pilot-candidate images through both `full` and `large` Worker endpoints: +**0/20 `large`-tier requests showed a cache hit** (`cf-cache-status: DYNAMIC` on every one; `large` mean 0.881s vs. `full` mean 0.983s - +within noise, both dominated by Google Drive origin latency, not R2 read +time). This isn't surprising in hindsight: the pilot's candidate pool is +specifically the tail of the catalog needing backfill - by definition +these are exactly the cards NOT recently popular enough to have been +browsed (and thus cached) by real users. **Verdict: switching tiers would +not reduce fetch latency or add caching benefit for this workload - stay +on `full` tier, already in use, gives the best-quality image for OCR.** +This also makes addendum item 6's original framing ("OCR resolution floor +re-measured at the CDN's delivered pixel size") moot - the delivered +pixel size doesn't change, since no tier switch is happening. + +**Checked a real cache-key gap while in the Worker source, cleared it - +not applicable today.** `R2Service.getImageKey` doesn't include +`jpgQuality` in the cache key (`${imageIdentifier}-${imageSize}-${imageType}`) + +- whichever quality first populated an entry is what every later request + gets, silently. Checked every call site across `frontend/src/` that + requests `small`/`large`: all either omit `jpgQuality` (defaulting to + +100. or pass 100 explicitly - no call site requests a different quality + today, so this can't currently produce a mismatched-quality cache hit. + Worth remembering if a quality-tunable thumbnail path is ever added, but + not an active risk to the pilot (or anything else) as the code stands. + +**What the real constraint actually is, and what got built for it**: every +image fetch is one request against the Worker's daily request quota, +which is **shared with live site traffic** regardless of which tier is +requested (a cache hit still counts as a Worker request, just a cheaper +one to serve) - this part of the original concern was correct, just not +for the "bucket-first" reason originally assumed. Implemented +`--fetch-budget` (`run_pilot(fetch_budget=...)`): caps the number of +image fetches a single invocation will make; on exhaustion the run stops +cleanly mid-selection, whatever was already flushed stays committed, and +every card not yet reached is left completely untouched (no vote, no +skip-reason recorded) so the next invocation's ordinary idempotent +selection just picks them up - no special resume handling needed, same +mechanism `--resume`/checkpointing already relies on. Verified in tests +(`TestFetchBudget`): stops exactly at the budget, and a follow-up +invocation with no budget completes the untouched remainder with no +duplicates. + +**Quota math**: ~171,800 eligible cards remain, each fetched at most once +(idempotent selection - no repeat fetches across invocations). Spread +across the ~13-day naive full-catalog projection (see wall-clock section +below), that's roughly 13,000/day if evenly sliced - well under the +Worker's 100,000/day shared limit on its own. The real risk isn't the +pilot in isolation, it's concentration: heavy parallelization (item 3d) +compressing the same total fetch count into fewer, busier days, stacked +on top of live traffic's own share of the same quota on those days. +`--fetch-budget` is the safety valve for that scenario - a conservative +per-invocation cap (a specific number is a scaling-proposal decision, not +fixed here) leaves headroom for live traffic regardless of how +aggressively a given slice is scheduled. + ### No-match autopsy (2026-07-15, post-merge Hold #1 of the pre-scale program) Classified all 176 OCR "parsed-but-no-match" cases from the pilot run From e93e257ce1803b985afdee020e4d05ea329d4e38 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Wed, 15 Jul 2026 19:50:16 +0000 Subject: [PATCH 06/23] Item 3c: tighten OCR crop box + add empirically-validated --fetch-dpi resolution floor --- .../local_identify_printing_tags.py | 40 ++++++--- MPCAutofill/cardpicker/local_ocr.py | 20 ++++- .../commands/local_identify_printing_tags.py | 30 +++++-- .../test_local_identify_printing_tags.py | 71 ++++++++++++---- docs/features/printing-tags.md | 83 +++++++++++++++++++ 5 files changed, 209 insertions(+), 35 deletions(-) diff --git a/MPCAutofill/cardpicker/local_identify_printing_tags.py b/MPCAutofill/cardpicker/local_identify_printing_tags.py index 32fba7176..004112f68 100644 --- a/MPCAutofill/cardpicker/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/local_identify_printing_tags.py @@ -172,21 +172,38 @@ def select_candidates( return multi + single -def get_worker_image_url(card: Card) -> Optional[str]: +# The empirically-validated OCR resolution floor (pre-scale program item 6/3c, 2026-07-15): +# a real 6-way dpi sweep (100/150/200/250/300/native) against the same 30-card sample used to +# validate the tightened crop box (see local_ocr.DEFAULT_CROP_BOX's comment) showed dpi<=150 +# genuinely degrades OCR yield (3/30, 7/30 vs. an 8/30 native-resolution baseline), while +# dpi>=200 matches or EXCEEDS the native baseline (12/30, 10/30, 9/30) despite a 2-4x smaller +# payload - smaller re-encoded JPEGs plausibly render small text more cleanly than a full-res +# original in some cases, though 30 cards is too small a sample to fully explain that. 250 is a +# safety margin above the empirically-best 200, not the raw optimum - hedges against small- +# sample noise while still keeping most of the bandwidth win (mean 728KB vs. 1.84MB native, a +# 2.5x reduction). PILOT-ONLY: this constant is local_identify_printing_tags' own default, not +# shared with frontend/src/features/pdf/ or .../download/, which need full print resolution by +# design and are untouched by this change. +DEFAULT_FETCH_DPI: Optional[int] = 250 + + +def get_worker_image_url(card: Card, dpi: Optional[int] = DEFAULT_FETCH_DPI) -> Optional[str]: """ - The card's own highest-resolution available image, via the image CDN Worker's "full" tier - (image-cdn/, docs/features/image-cdn.md) - the same source the PDF export path uses for - print-quality output. Google Drive sources only, matching that Worker's current scope - (frontend/src/common/image.ts's getWorkerImageURL has the identical restriction) - any - other source type returns None, counted by the caller as an "unsupported-source-type" skip. + The card's image via the image CDN Worker's "full" tier (image-cdn/, docs/features/image-cdn.md) + - the same route the PDF export path uses, but at a resolution capped via `dpi` (see + DEFAULT_FETCH_DPI) rather than the print-quality original PDF export needs. Google Drive + sources only, matching that Worker's current scope (frontend/src/common/image.ts's + getWorkerImageURL has the identical restriction) - any other source type returns None, + counted by the caller as an "unsupported-source-type" skip. """ if card.get_source_type_choices() != SourceTypeChoices.GOOGLE_DRIVE: return None - return f"{settings.IMAGE_WORKER_URL}/images/google_drive/full/{card.identifier}.jpg?jpgQuality=100" + dpi_param = f"&dpi={dpi}" if dpi is not None else "" + return f"{settings.IMAGE_WORKER_URL}/images/google_drive/full/{card.identifier}.jpg?jpgQuality=100{dpi_param}" -def fetch_card_image(card: Card) -> Optional["Image.Image"]: - url = get_worker_image_url(card) +def fetch_card_image(card: Card, dpi: Optional[int] = DEFAULT_FETCH_DPI) -> Optional["Image.Image"]: + url = get_worker_image_url(card, dpi) if url is None: return None try: @@ -341,6 +358,7 @@ def run_pilot( batch_size: int = DEFAULT_BATCH_SIZE, progress_every: int = 50, fetch_budget: Optional[int] = None, + fetch_dpi: Optional[int] = DEFAULT_FETCH_DPI, ) -> tuple[dict[str, PilotResult], AttributeReport]: if nice: try: @@ -430,9 +448,9 @@ def flush() -> None: cards_attempted += 1 outcome = CardOutcome(card_id=card_id) - if get_worker_image_url(selected.card) is not None: + if get_worker_image_url(selected.card, fetch_dpi) is not None: fetches_made += 1 - image = fetch_card_image(selected.card) # shared across every engine that runs on this card + image = fetch_card_image(selected.card, fetch_dpi) # shared across every engine that runs on this card ocr_raw_texts: list[str] = [] if card_id in ocr_selected_ids: diff --git a/MPCAutofill/cardpicker/local_ocr.py b/MPCAutofill/cardpicker/local_ocr.py index 4d8820c70..87fd2dcdc 100644 --- a/MPCAutofill/cardpicker/local_ocr.py +++ b/MPCAutofill/cardpicker/local_ocr.py @@ -21,11 +21,27 @@ logger = logging.getLogger(__name__) -# left 0-35% width, bottom 90-100% height - tuned against real production images (2026-07-15): +# left 6-35% width, bottom 90-96.5% height - tuned against real production images (2026-07-15): # the original 85% top boundary caught a full trailing line of rules text above the collector # line on several real cards, which confused tesseract's line segmentation into garbage output # even with PSM 6. (left, top, right, bottom), each a fraction of the full image. -DEFAULT_CROP_BOX: tuple[float, float, float, float] = (0.0, 0.90, 0.35, 1.0) +# +# Tightened from an original (0.0, 0.90, 0.35, 1.0) via pre-scale program item 3c/addendum item +# 6b (2026-07-15): tesseract's TSV bbox output, sampled across 30 real production cards, showed +# every observed collector-number-shaped text line landing within the top 41.2% / right-hand +# 74.4% of that original crop's own area - the bottom ~59% and left ~26% were dead space. New +# boundaries applied a safety margin over the observed range rather than cutting exactly to it +# (~1.5x on the trimmed bottom, ~0.7x on the trimmed left), specifically BECAUSE physical bleed +# margin varies by card/source and a tight cut against one sample's exact observed range risks +# clipping a card with more bleed than this sample happened to show. The right edge was left +# UNCHANGED despite being a plausible-looking trim target - text was observed touching that +# boundary already (right_frac max = 1.000), meaning trimming it would be a real clipping risk, +# not a safe optimization. Validated (not just derived) against the same 30-card sample: OCR +# match count and the exact set of matched cards were IDENTICAL between the old and new box (8/30 +# both ways, same 8 card pks) - zero yield regression on this sample. 30 cards is a real but +# modest validation bar (matching the addendum's own ask); watch for regression during the actual +# scaled run rather than treating this as proven at full-catalog scale. +DEFAULT_CROP_BOX: tuple[float, float, float, float] = (0.06, 0.90, 0.35, 0.965) # tesseract page-segmentation mode 6 = "assume a single uniform block of text" - the real # collector "line" is usually two lines (rarity+number, then set+lang+artist), which PSM 7 diff --git a/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py b/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py index 183ec3167..56ce7399c 100644 --- a/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py @@ -1,5 +1,5 @@ import argparse -from typing import Any +from typing import Any, Optional from django.core.management.base import BaseCommand, CommandError @@ -105,10 +105,24 @@ def add_arguments(self, parser: Any) -> None: type=int, default=None, help="Cap the number of image CDN Worker requests this invocation will make (every " - "fetched card costs one - the Worker's daily request quota is SHARED with live site " - "traffic, docs/features/image-cdn.md). On exhaustion the run stops cleanly: whatever " - "was already flushed stays committed, and untouched cards are picked up fresh by the " - "next invocation - no special resume handling needed. Default: no limit.", + "fetched card costs one). Belt-and-suspenders alongside the Worker's own " + "IMAGE_FULL_TIER_RATE_LIMITER (image-cdn/wrangler.toml) - the enforced protection for " + "lh4.googleusercontent.com, shared with live PDF export/bulk download traffic - not " + "the primary safeguard. On exhaustion the run stops cleanly: whatever was already " + "flushed stays committed, and untouched cards are picked up fresh by the next " + "invocation - no special resume handling needed. Default: no limit.", + ) + parser.add_argument( + "--fetch-dpi", + type=int, + default=local_identify_printing_tags.DEFAULT_FETCH_DPI, + help="Request images from the CDN Worker capped at this dpi (maps to a smaller " + "re-encoded JPEG height, image-cdn/src/url.ts) instead of full print-quality " + "original - OCR only needs to read a small corner crop. Empirically validated floor " + "(pre-scale program item 6/3c): dpi<=150 degrades yield, dpi>=200 matches or exceeds " + "native-resolution yield with a 2-4x smaller payload. Default: " + f"{local_identify_printing_tags.DEFAULT_FETCH_DPI} (margin above the empirically-best " + "200). Pass --fetch-dpi=0 for uncapped native resolution.", ) def handle(self, *args: Any, **kwargs: Any) -> None: @@ -121,6 +135,9 @@ def handle(self, *args: Any, **kwargs: Any) -> None: phash_max_candidates = kwargs["phash_max_candidates"] batch_size = kwargs["batch_size"] fetch_budget = kwargs["fetch_budget"] + fetch_dpi: Optional[int] = kwargs["fetch_dpi"] + if fetch_dpi == 0: + fetch_dpi = None def _parse_source_pks(raw: str) -> list[int]: return [int(p) for p in raw.split(",") if p.strip()] @@ -152,7 +169,7 @@ def _parse_source_pks(raw: str) -> list[int]: print( f"[{mode}] local_identify_printing_tags --engine={engine} --limit={limit} " f"--nice={nice} --crop-box={crop_box} --batch-size={batch_size} " - f"--fetch-budget={fetch_budget} " + f"--fetch-budget={fetch_budget} --fetch-dpi={fetch_dpi} " f"--exclude-sources-ocr={exclude_source_pks_by_engine['ocr']} " f"--exclude-sources-phash={exclude_source_pks_by_engine['phash']}" ) @@ -169,6 +186,7 @@ def _parse_source_pks(raw: str) -> list[int]: exclude_source_pks_by_engine=exclude_source_pks_by_engine, batch_size=batch_size, fetch_budget=fetch_budget, + fetch_dpi=fetch_dpi, ) gate_violations: list[int] = [] diff --git a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py index 3670bb0d2..7f8f51975 100644 --- a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py @@ -20,6 +20,7 @@ PHASH_ANONYMOUS_ID, CandidateNameIndex, CandidatePrinting, + get_worker_image_url, run_pilot, select_candidates, verify_zero_resolutions, @@ -183,6 +184,20 @@ def test_explicit_flag_overrides_the_default(self, db, capsys): assert "--exclude-sources-ocr=[]" in printed assert "--exclude-sources-phash=[2, 3]" in printed + def test_bare_invocation_defaults_fetch_dpi_to_250(self, db, capsys): + from django.core.management import call_command + + call_command("local_identify_printing_tags", "--dry-run", "--limit", "0") + printed = capsys.readouterr().out + assert "--fetch-dpi=250" in printed + + def test_fetch_dpi_zero_means_native_resolution(self, db, capsys): + from django.core.management import call_command + + call_command("local_identify_printing_tags", "--dry-run", "--limit", "0", "--fetch-dpi", "0") + printed = capsys.readouterr().out + assert "--fetch-dpi=None" in printed + class TestCandidateNameIndex: def test_groups_by_normalised_name(self, db): @@ -378,12 +393,13 @@ class TestOcrLiveTesseractIntegration: @pytest.mark.skipif(shutil.which("tesseract") is None, reason="tesseract-ocr binary not installed") def test_crop_preprocess_and_ocr_a_synthetic_collector_line(self): - # positioned within DEFAULT_CROP_BOX's bottom 90-100% band (945-1050px of a 1050px-tall - # image) - tuned against a real production card image, see DEFAULT_CROP_BOX's comment + # positioned within DEFAULT_CROP_BOX's band (left 45-262px, top 945-1013px of a + # 750x1050 image) - tuned against real production card images, see DEFAULT_CROP_BOX's + # comment img = Image.new("RGB", (750, 1050), "white") draw = ImageDraw.Draw(img) - draw.rectangle([0, 945, 262, 1050], fill="black") - draw.text((10, 975), "158/287 R MOM EN", fill="white") + draw.rectangle([45, 945, 262, 1013], fill="black") + draw.text((50, 970), "158/287 R MOM EN", fill="white") cropped = crop_collector_line(img) variants = preprocess_variants(cropped) @@ -410,7 +426,7 @@ def fake_phash(selected, image, threshold, margin, max_candidates): monkeypatch.setattr(module, "run_ocr_for_card", fake_ocr) monkeypatch.setattr(module, "run_phash_for_card", fake_phash) - monkeypatch.setattr(module, "fetch_card_image", lambda card: None) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: None) results, _attributes = run_pilot(engine="both", limit=10, dry_run=False, nice=False) @@ -439,7 +455,7 @@ def fake_phash(selected, image, threshold, margin, max_candidates): monkeypatch.setattr(module, "run_ocr_for_card", fake_ocr) monkeypatch.setattr(module, "run_phash_for_card", fake_phash) - monkeypatch.setattr(module, "fetch_card_image", lambda card: None) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: None) results, _attributes = run_pilot(engine="both", limit=10, dry_run=False, nice=False) @@ -463,7 +479,7 @@ def fake_ocr(selected, image, crop_box): ) monkeypatch.setattr(module, "run_ocr_for_card", fake_ocr) - monkeypatch.setattr(module, "fetch_card_image", lambda card: None) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: None) results, _attributes = run_pilot(engine="ocr", limit=10, dry_run=True, nice=False) @@ -494,7 +510,7 @@ def fake_ocr(selected, image, crop_box): ) monkeypatch.setattr(module, "run_ocr_for_card", fake_ocr) - monkeypatch.setattr(module, "fetch_card_image", lambda card: None) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: None) results, _attributes = run_pilot( engine="ocr", @@ -526,7 +542,7 @@ def fake_ocr(selected, image, crop_box): ) monkeypatch.setattr(module, "run_ocr_for_card", fake_ocr) - monkeypatch.setattr(module, "fetch_card_image", lambda card: None) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: None) def test_flushes_periodically_not_just_once_at_the_end(self, db, monkeypatch): printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) @@ -610,6 +626,29 @@ def killing_bulk_create(objs, *args, **kwargs): assert {v.card_id for v in final_votes} == {c.pk for c in cards} +class TestFetchDpi: + """Item 6/3c's empirically-validated resolution floor - see local_identify_printing_tags' + DEFAULT_FETCH_DPI comment for the measured yield numbers behind the default.""" + + def test_default_dpi_is_included_in_the_url(self, db): + card = CardFactory() + url = get_worker_image_url(card) + assert url is not None + assert "dpi=250" in url + + def test_explicit_dpi_overrides_the_default(self, db): + card = CardFactory() + url = get_worker_image_url(card, dpi=200) + assert url is not None + assert "dpi=200" in url + + def test_none_dpi_omits_the_param_for_native_resolution(self, db): + card = CardFactory() + url = get_worker_image_url(card, dpi=None) + assert url is not None + assert "dpi=" not in url + + class TestFetchBudget: """Stage 8 pre-scale program item 3b: every image fetch is one request against the shared image CDN Worker quota - an unattended run must be boundable. Cards past the budget must be @@ -665,7 +704,7 @@ def test_a_card_voted_on_is_excluded_from_the_next_selection(self, db, monkeypat vote=module.EngineVote(engine="ocr", printing_pk=printing.pk, confidence=0.85, detail="raw") ), ) - monkeypatch.setattr(module, "fetch_card_image", lambda card: None) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: None) run_pilot(engine="ocr", limit=10, dry_run=False, nice=False) # re-running the exact same selection now excludes this card - it already has a vote @@ -734,7 +773,7 @@ def test_fallback_fires_and_votes_when_pass_1_misses_entirely(self, db, monkeypa lambda selected, image, threshold, margin, max_candidates: (None, "no-clear-winner"), ) monkeypatch.setattr( - module, "fetch_card_image", lambda card: _black_bordered_image_with_artist_text("Marie Magny") + module, "fetch_card_image", lambda card, dpi=None: _black_bordered_image_with_artist_text("Marie Magny") ) results, attributes = run_pilot(engine="both", limit=10, dry_run=False, nice=False) @@ -771,7 +810,7 @@ def fake_ocr(selected, image, crop_box): ) monkeypatch.setattr(module, "run_ocr_for_card", fake_ocr) - monkeypatch.setattr(module, "fetch_card_image", lambda card: Image.new("RGB", (750, 1050), (5, 5, 5))) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: Image.new("RGB", (750, 1050), (5, 5, 5))) results, attributes = run_pilot(engine="ocr", limit=10, dry_run=False, nice=False) @@ -802,7 +841,7 @@ def fail_if_called(selected, image, ocr_raw_texts): "run_phash_for_card", lambda selected, image, threshold, margin, max_candidates: (None, "no-clear-winner"), ) - monkeypatch.setattr(module, "fetch_card_image", lambda card: Image.new("RGB", (750, 1050), (5, 5, 5))) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: Image.new("RGB", (750, 1050), (5, 5, 5))) # if the assertion inside fail_if_called had fired, this call itself would raise run_pilot(engine="both", limit=10, dry_run=False, nice=False) @@ -840,7 +879,7 @@ def fake_ocr(selected, image, crop_box): monkeypatch.setattr(module, "run_ocr_for_card", fake_ocr) # a uniform near-black image - the pixel-sample heuristic would read "black" here, but # the matched printing's own metadata says "white" and must win instead. - monkeypatch.setattr(module, "fetch_card_image", lambda card: Image.new("RGB", (750, 1050), (5, 5, 5))) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: Image.new("RGB", (750, 1050), (5, 5, 5))) results, attributes = run_pilot(engine="ocr", limit=10, dry_run=False, nice=False) @@ -875,7 +914,7 @@ def test_heuristic_used_when_no_printing_confirmed_this_run(self, db, monkeypatc "run_phash_for_card", lambda selected, image, threshold, margin, max_candidates: (None, "no-clear-winner"), ) - monkeypatch.setattr(module, "fetch_card_image", lambda card: Image.new("RGB", (750, 1050), (5, 5, 5))) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: Image.new("RGB", (750, 1050), (5, 5, 5))) results, attributes = run_pilot(engine="both", limit=10, dry_run=False, nice=False) @@ -906,7 +945,7 @@ def fake_ocr(selected, image, crop_box): ) monkeypatch.setattr(module, "run_ocr_for_card", fake_ocr) - monkeypatch.setattr(module, "fetch_card_image", lambda card: Image.new("RGB", (750, 1050), (5, 5, 5))) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: Image.new("RGB", (750, 1050), (5, 5, 5))) results, attributes = run_pilot(engine="ocr", limit=10, dry_run=False, nice=False) diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index 7f6a2cf6e..27e46c07c 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -944,6 +944,89 @@ per-invocation cap (a specific number is a scaling-proposal decision, not fixed here) leaves headroom for live traffic regardless of how aggressively a given slice is scheduled. +**Amendment (same day, owner review): the "shared Worker request quota" +framing above was the wrong quota.** The real question is whether +`lh4.googleusercontent.com` itself (the domain the `full` tier's +passthrough actually fetches from - the Worker's own 100k/day request +quota was never the binding constraint) can take sustained pilot-scale +load without degrading for live traffic. That domain is genuinely shared + +- `frontend/src/features/pdf/pdfImage.ts` (PDF export) and + `frontend/src/features/download/downloadImages.ts` (bulk image download) + both request the `full` tier already - but had **no rate limiting of any + kind**, unlike the real Drive API (`GoogleDriveService.executeCall`, + guarded by the existing `GOOGLE_DRIVE_RATE_LIMITER` binding - a + DIFFERENT Google domain the `full`-tier image fetch never touches). + Fixed with a real enforced limiter (`image-cdn`, separate PR ahead of + this one): a new `IMAGE_FULL_TIER_RATE_LIMITER` Cloudflare rate-limiting + binding (3 req/s sustained, `wrangler.toml`), wired into `image.ts`'s + `full`-tier handler via a new `fetchWithRateLimit` helper + (`src/utils.ts`) that mirrors `GoogleDriveService.executeCall`'s + check-then-backoff-then-retry pattern - checked-in-limit, delay-and-retry + on denial, plus a defensive retry on an upstream 429. This is now the + **primary** protection, shared by all three callers (pilot, PDF export, + bulk download); `--fetch-budget` is defense-in-depth on the pilot's own + pacing, not the main safeguard the earlier paragraphs implied. Lands and + deploys independently of this pilot's own branch, ahead of any + full-catalog run. + +### Resolution floor + payload reduction (2026-07-15, same review) + +**`lh4.googleusercontent.com`'s size-suffix parameter genuinely +re-encodes a smaller image - verified directly, not assumed**: fetched +one real card at `=h200`/`=h400`/`=h800`/native and confirmed real, +progressively smaller dimensions and byte counts each time (native +1146x1600 @ 3.29MB → `=h800` 573x800 @ 892KB → `=h400` 287x400 @ 218KB). +The image CDN Worker already exposes this via the `full` tier's existing +`dpi` query param (`height = dpi * 1110 / 300`, `image-cdn/src/url.ts`) + +- the pilot just never passed one, so every fetch requested the + uncapped native original. + +**Empirical resolution floor**, a real 6-way sweep (dpi 100/150/200/250/ +300/native) against the same 30-card sample used to validate the +tightened crop box, applying that same tightened box and the production +OCR pipeline at each size: + +| dpi | matched/30 | mean payload | +| ------------- | ---------: | -----------: | +| 100 | 3 | 144KB | +| 150 | 7 | 298KB | +| 200 | 12 | 495KB | +| 250 | 10 | 728KB | +| 300 | 9 | 997KB | +| native (none) | 8 | 1.84MB | + +dpi≤150 clearly degrades yield below the native baseline; dpi≥200 +matches or **exceeds** it despite a 2-4x smaller payload (plausibly a +smaller re-encoded JPEG rendering small text more cleanly than a full-res +original in some cases - 30 cards is too small a sample to fully explain +the exact ranking, but the floor itself - "150 is unsafe, 200+ is safe" - +is a clear, robust signal). Adopted `DEFAULT_FETCH_DPI = 250` in +`local_identify_printing_tags.py` (a `--fetch-dpi` CLI flag, `0` for +uncapped) - a margin above the empirically-best 200, hedging against +small-sample noise while keeping most of the win (728KB vs. 1.84MB +native, 2.5x smaller). **Pilot-only**: `pdfImage.ts`/`downloadImages.ts` +are untouched and still request full print-quality resolution by design. + +### Crop tightening (2026-07-15, pre-scale program item 3c / addendum item 6b) + +Tesseract's TSV bbox output, sampled across the same 30-card sample +(both preprocessing polarities), showed every observed collector-number- +shaped text line landing within the top 41.2% / right-hand 74.4% of the +existing crop's own area - meaning the bottom ~59% and left ~26% were +dead space. Tightened `local_ocr.DEFAULT_CROP_BOX` from +`(0.0, 0.90, 0.35, 1.0)` to `(0.06, 0.90, 0.35, 0.965)`, applying a +safety margin over the observed range (not cutting exactly to it, per +the addendum's explicit bleed-variance caution) and leaving the right +edge untouched (text was observed touching that boundary already - +trimming it would risk clipping, not save anything). **Validated, not +just derived**: re-ran OCR with both the old and new box against the +same 30 cards - identical match count (8/30 both) AND identical card- +level match set (same 8 card pks matched both ways) - zero yield +regression on this sample. See `local_ocr.py`'s `DEFAULT_CROP_BOX` +comment for the full derivation. + ### No-match autopsy (2026-07-15, post-merge Hold #1 of the pre-scale program) Classified all 176 OCR "parsed-but-no-match" cases from the pilot run From a881b59444cc98defe31a70bf48a19667d255a98 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Wed, 15 Jul 2026 20:17:41 +0000 Subject: [PATCH 07/23] Addendum item 7: aspect-ratio bleed-edge classification, votes on appropriate-bleed --- MPCAutofill/cardpicker/local_fallback.py | 74 +++++++++++++++++ .../local_identify_printing_tags.py | 19 +++++ .../commands/local_identify_printing_tags.py | 2 + .../cardpicker/tests/test_local_fallback.py | 56 +++++++++++++ .../test_local_identify_printing_tags.py | 79 +++++++++++++++++++ docs/features/printing-tags.md | 50 ++++++++++++ 6 files changed, 280 insertions(+) diff --git a/MPCAutofill/cardpicker/local_fallback.py b/MPCAutofill/cardpicker/local_fallback.py index b24a6b189..33dc24a47 100644 --- a/MPCAutofill/cardpicker/local_fallback.py +++ b/MPCAutofill/cardpicker/local_fallback.py @@ -436,6 +436,74 @@ def frame_style_is_consistent(frame_class: Optional[str], printing_frame_value: return expected_class == frame_class +# --------------------------------------------------------------------------------------------- +# 2c.5: bleed-edge classification, addendum item 7. Owner-directed design (2026-07-15): measure +# the image's own aspect ratio against chilli_axe's two known reference ratios (trim-only vs. +# trim-plus-bleed) rather than any pixel/color heuristic - geometric, resolution/DPI-independent, +# and (unlike a color-uniformity approach) inherently unaffected by whether the card's own border +# is visually a normal frame or borderless full-art, since the file's raw pixel dimensions carry +# the same trim/bleed math either way. Votes on the PRE-EXISTING `appropriate-bleed` SENSITIVE +# tag (sensitive_tags.py) - a moderator co-sign is still required to resolve it either direction, +# per that tag's own design; this heuristic is one more signal, not an override. +# --------------------------------------------------------------------------------------------- + +# frontend/src/common/constants.ts's CardWidthMM/CardHeightMM - the standard MTG trim size +# (63x88mm) chilli_axe's own frame templates are built against. +_CARD_TRIM_WIDTH_MM = 63 +_CARD_TRIM_HEIGHT_MM = 88 +_BLEED_MARGIN_MM = 3.175 # 1/8 inch per edge - the standard proxy-print bleed convention + +TRIM_ASPECT_RATIO = _CARD_TRIM_WIDTH_MM / _CARD_TRIM_HEIGHT_MM +BLEED_ASPECT_RATIO = (_CARD_TRIM_WIDTH_MM + 2 * _BLEED_MARGIN_MM) / (_CARD_TRIM_HEIGHT_MM + 2 * _BLEED_MARGIN_MM) + +# real-world validation (2026-07-15, 40 cards sampled across 40 distinct sources): the bleed +# cluster spread 0.7325-0.7393 (theoretical 0.7350), the one trimmed example measured 0.7163 +# (theoretical 0.7159) - a clean, well-separated bimodal signal with nothing observed in the +# gap between clusters. 0.03 comfortably covers the observed bleed spread on either side while +# still abstaining on an aspect ratio implausible for a standard MTG card altogether (a +# double-faced composite scan, a token, a corrupted fetch). +_BLEED_CLASSIFICATION_TOLERANCE = 0.03 + +BLEED_EDGE_TAG_NAME = "appropriate-bleed" +BLEED_EDGE_VOTE_CONFIDENCE = 0.7 + + +def classify_bleed_edge(card_image: "Image.Image") -> Optional[str]: + """Returns 'bleed'/'trimmed', or None if the image's aspect ratio is too far from BOTH known + reference ratios to classify confidently (ambiguous - a genuinely non-standard image, not + just a borderline case).""" + width, height = card_image.size + if height == 0: + return None + ratio = width / height + dist_to_trim = abs(ratio - TRIM_ASPECT_RATIO) + dist_to_bleed = abs(ratio - BLEED_ASPECT_RATIO) + if min(dist_to_trim, dist_to_bleed) > _BLEED_CLASSIFICATION_TOLERANCE: + return None + return "bleed" if dist_to_bleed < dist_to_trim else "trimmed" + + +def cast_bleed_edge_vote(card: Card, bleed_class: Optional[str]) -> Optional[CardTagVote]: + """Positive (APPLY) vote for a clear bleed margin, negative (NOT_APPLICABLE) for clearly + trimmed, no vote at all for an ambiguous/unclassifiable reading (the caller counts this as + an abstain without writing anything, same convention as classify_frame_style's abstain + path).""" + if bleed_class is None: + return None + tag = Tag.objects.filter(name=BLEED_EDGE_TAG_NAME).first() + if tag is None: + return None + polarity = VotePolarity.APPLY if bleed_class == "bleed" else VotePolarity.NOT_APPLICABLE + return CardTagVote( + card=card, + tag=tag, + polarity=polarity, + anonymous_id=FALLBACK_ANONYMOUS_ID, + source=VoteSource.OCR, + confidence=BLEED_EDGE_VOTE_CONFIDENCE, + ) + + # --------------------------------------------------------------------------------------------- # 2d: combine # --------------------------------------------------------------------------------------------- @@ -530,6 +598,12 @@ def run_fallback_for_card( "classify_frame_style", "cast_frame_style_vote", "frame_style_is_consistent", + "TRIM_ASPECT_RATIO", + "BLEED_ASPECT_RATIO", + "BLEED_EDGE_TAG_NAME", + "BLEED_EDGE_VOTE_CONFIDENCE", + "classify_bleed_edge", + "cast_bleed_edge_vote", "FallbackOutcome", "run_fallback_for_card", ] diff --git a/MPCAutofill/cardpicker/local_identify_printing_tags.py b/MPCAutofill/cardpicker/local_identify_printing_tags.py index 004112f68..6194c2672 100644 --- a/MPCAutofill/cardpicker/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/local_identify_printing_tags.py @@ -343,6 +343,11 @@ class AttributeReport: # ground-truth-preferred wiring. border_ground_truth_count: int = 0 frame_ground_truth_count: int = 0 + # addendum item 7 (2026-07-15): bleed-edge classification, votes on the pre-existing + # `appropriate-bleed` SENSITIVE tag (local_fallback.classify_bleed_edge/cast_bleed_edge_vote). + # No ground-truth counterpart - unlike border/frame, there's no Scryfall field encoding this. + bleed_votes_by_class: dict[str, int] = field(default_factory=lambda: collections.defaultdict(int)) + bleed_abstain_count: int = 0 def run_pilot( @@ -668,6 +673,20 @@ def flush() -> None: else: attributes.frame_abstain_count += 1 + # addendum item 7: bleed-edge classification - independent of printing-vote success, + # same "fires for any card with a fetched image" convention as border/frame above, and + # (unlike those two) has no ground-truth counterpart to prefer, since Scryfall doesn't + # encode this at all. + if image is not None: + bleed_class = local_fallback.classify_bleed_edge(image) + if bleed_class is not None: + attributes.bleed_votes_by_class[bleed_class] += 1 + bleed_vote = local_fallback.cast_bleed_edge_vote(card, bleed_class) + if bleed_vote is not None and not dry_run: + tag_votes_batch.append(bleed_vote) + else: + attributes.bleed_abstain_count += 1 + if (i + 1) % batch_size == 0: flush() if nice and i % 20 == 0: diff --git a/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py b/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py index 56ce7399c..3af16a66c 100644 --- a/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py @@ -214,6 +214,8 @@ def _parse_source_pks(raw: str) -> list[int]: ) print(f" frame abstains: {attributes.frame_abstain_count}") print(f" frame mismatches (printing vote withheld): {len(attributes.frame_mismatches)}") + print(f" bleed votes: {dict(attributes.bleed_votes_by_class)}") + print(f" bleed abstains: {attributes.bleed_abstain_count}") if dry_run: print("Dry run - nothing written, gate check not run.") diff --git a/MPCAutofill/cardpicker/tests/test_local_fallback.py b/MPCAutofill/cardpicker/tests/test_local_fallback.py index b2720a9df..bac6599e3 100644 --- a/MPCAutofill/cardpicker/tests/test_local_fallback.py +++ b/MPCAutofill/cardpicker/tests/test_local_fallback.py @@ -17,10 +17,15 @@ import cardpicker.local_ocr as local_ocr from cardpicker.local_fallback import ( + BLEED_ASPECT_RATIO, + BLEED_EDGE_TAG_NAME, BORDER_COLOR_TO_TAG, FALLBACK_ANONYMOUS_ID, + TRIM_ASPECT_RATIO, + cast_bleed_edge_vote, cast_border_attribute_vote, cast_frame_style_vote, + classify_bleed_edge, classify_border_color, classify_frame_style, extract_artist_name, @@ -351,3 +356,54 @@ def test_agreement(self): def test_disagreement(self): assert frame_style_is_consistent("old", "2015") is False assert frame_style_is_consistent("modern", "1993") is False + + +class TestClassifyBleedEdge: + def test_trim_ratio_classifies_as_trimmed(self): + image = Image.new("RGB", (716, 1000), "white") # 716/1000 ~= 63/88 + assert classify_bleed_edge(image) == "trimmed" + + def test_bleed_ratio_classifies_as_bleed(self): + image = Image.new("RGB", (735, 1000), "white") # 735/1000 ~= BLEED_ASPECT_RATIO + assert classify_bleed_edge(image) == "bleed" + + def test_far_from_both_references_is_ambiguous(self): + image = Image.new("RGB", (1000, 1000), "white") # square - nowhere near either ratio + assert classify_bleed_edge(image) is None + + def test_exact_reference_ratios_round_trip(self): + # exact float ratios (not the rounded pixel approximations above) must still classify + # correctly - guards against an off-by-epsilon tolerance bug + trim_image = Image.new("RGB", (int(TRIM_ASPECT_RATIO * 10000), 10000), "white") + bleed_image = Image.new("RGB", (int(BLEED_ASPECT_RATIO * 10000), 10000), "white") + assert classify_bleed_edge(trim_image) == "trimmed" + assert classify_bleed_edge(bleed_image) == "bleed" + + +class TestCastBleedEdgeVote: + def test_no_reading_casts_nothing(self, db): + card = CardFactory() + assert cast_bleed_edge_vote(card, None) is None + + def test_unseeded_tag_degrades_to_no_vote(self, db): + card = CardFactory() + assert cast_bleed_edge_vote(card, "bleed") is None + + def test_bleed_casts_a_positive_vote_on_the_existing_tag(self, db): + TagFactory(name=BLEED_EDGE_TAG_NAME) + card = CardFactory() + vote = cast_bleed_edge_vote(card, "bleed") + assert vote is not None + assert vote.pk is None + assert vote.tag.name == BLEED_EDGE_TAG_NAME + assert vote.polarity == VotePolarity.APPLY + assert vote.anonymous_id == FALLBACK_ANONYMOUS_ID + assert vote.source == VoteSource.OCR + assert vote.confidence == 0.7 + + def test_trimmed_casts_a_negative_vote_on_the_existing_tag(self, db): + TagFactory(name=BLEED_EDGE_TAG_NAME) + card = CardFactory() + vote = cast_bleed_edge_vote(card, "trimmed") + assert vote is not None + assert vote.polarity == VotePolarity.NOT_APPLICABLE diff --git a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py index 7f8f51975..180208dce 100644 --- a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py @@ -37,6 +37,7 @@ CardPrintingTag, CardTagVote, PrintingTagStatus, + VotePolarity, VoteSource, ) from cardpicker.tests.factories import ( @@ -953,3 +954,81 @@ def fake_ocr(selected, image, crop_box): assert attributes.border_votes_by_class == {"black": 1} assert attributes.border_ground_truth_count == 0 assert CardTagVote.objects.filter(card=card, tag__name="Black Border").exists() + + +class TestBleedEdgeVotesEndToEnd: + """Addendum item 7: run_pilot casts a real vote on the pre-existing appropriate-bleed tag + for every card with a fetched image, independent of printing-vote success.""" + + def test_bleed_shaped_image_casts_a_positive_vote(self, db, monkeypatch): + CanonicalCardFactory(name="Forest") + card = CardFactory(name="Forest") + TagFactory(name="appropriate-bleed") + + import cardpicker.local_identify_printing_tags as module + import cardpicker.local_ocr as local_ocr_module + + monkeypatch.setattr(local_ocr_module, "run_tesseract", lambda image: "") + monkeypatch.setattr(module, "run_ocr_for_card", lambda selected, image, crop_box: module.OcrCardResult()) + monkeypatch.setattr( + module, + "run_phash_for_card", + lambda selected, image, threshold, margin, max_candidates: (None, "too-many-candidates"), + ) + # 735/1000 ~= BLEED_ASPECT_RATIO + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: Image.new("RGB", (735, 1000), (5, 5, 5))) + + _results, attributes = run_pilot(engine="ocr", limit=10, dry_run=False, nice=False) + + assert attributes.bleed_votes_by_class == {"bleed": 1} + vote = CardTagVote.objects.get(card=card, tag__name="appropriate-bleed") + assert vote.polarity == VotePolarity.APPLY + + def test_trimmed_shaped_image_casts_a_negative_vote(self, db, monkeypatch): + CanonicalCardFactory(name="Forest") + card = CardFactory(name="Forest") + TagFactory(name="appropriate-bleed") + + import cardpicker.local_identify_printing_tags as module + import cardpicker.local_ocr as local_ocr_module + + monkeypatch.setattr(local_ocr_module, "run_tesseract", lambda image: "") + monkeypatch.setattr(module, "run_ocr_for_card", lambda selected, image, crop_box: module.OcrCardResult()) + monkeypatch.setattr( + module, + "run_phash_for_card", + lambda selected, image, threshold, margin, max_candidates: (None, "too-many-candidates"), + ) + # 716/1000 ~= TRIM_ASPECT_RATIO + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: Image.new("RGB", (716, 1000), (5, 5, 5))) + + _results, attributes = run_pilot(engine="ocr", limit=10, dry_run=False, nice=False) + + assert attributes.bleed_votes_by_class == {"trimmed": 1} + vote = CardTagVote.objects.get(card=card, tag__name="appropriate-bleed") + assert vote.polarity == VotePolarity.NOT_APPLICABLE + + def test_ambiguous_ratio_abstains_without_writing_anything(self, db, monkeypatch): + CanonicalCardFactory(name="Forest") + card = CardFactory(name="Forest") + TagFactory(name="appropriate-bleed") + + import cardpicker.local_identify_printing_tags as module + import cardpicker.local_ocr as local_ocr_module + + monkeypatch.setattr(local_ocr_module, "run_tesseract", lambda image: "") + monkeypatch.setattr(module, "run_ocr_for_card", lambda selected, image, crop_box: module.OcrCardResult()) + monkeypatch.setattr( + module, + "run_phash_for_card", + lambda selected, image, threshold, margin, max_candidates: (None, "too-many-candidates"), + ) + monkeypatch.setattr( + module, "fetch_card_image", lambda card, dpi=None: Image.new("RGB", (1000, 1000), (5, 5, 5)) + ) + + _results, attributes = run_pilot(engine="ocr", limit=10, dry_run=False, nice=False) + + assert attributes.bleed_votes_by_class == {} + assert attributes.bleed_abstain_count == 1 + assert not CardTagVote.objects.filter(card=card, tag__name="appropriate-bleed").exists() diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index 27e46c07c..29bc0abb8 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -1027,6 +1027,56 @@ level match set (same 8 card pks matched both ways) - zero yield regression on this sample. See `local_ocr.py`'s `DEFAULT_CROP_BOX` comment for the full derivation. +### Bleed-edge tagging (2026-07-15, addendum item 7) + +**Checked whether a bleed tag already existed before proposing anything +new, per the addendum's explicit gate**: `appropriate-bleed` already +exists (`sensitive_tags.py`, 0 cards tagged) - but as a +`TagModerationClass.SENSITIVE` tag, the same category as `low-res`/NSFW, +with its own code comment: _"Sensitive because that verification is +exactly a moderator's co-sign."_ It was designed for human-only +verification. Surfaced this to the owner before building anything - +decision: proceed, cast machine votes on the existing tag anyway (a +SENSITIVE tag still requires a moderator co-sign to resolve either way, +so a vote alone can't misuse it - it's one more signal moderators see, +not an override). + +**Detection design, owner-directed**: measure the image's own aspect +ratio against chilli_axe's two known reference card sizes +(`frontend/src/common/constants.ts`'s `CardWidthMM`/`CardHeightMM` = +63x88mm trim; +1/8" bleed per edge = 69.35x94.35mm) rather than any +pixel-color heuristic - purely geometric, so it's inherently +DPI/resolution-independent (verified: 0/15 mismatches between native and +`--fetch-dpi=250`-scaled classification of the same real cards - Google's +resize preserves aspect ratio) and unaffected by whether the card's own +border is visually a normal frame or a borderless full-art printing +(both follow the same file-dimension convention regardless of what's +visible). + +- `TRIM_ASPECT_RATIO = 63/88 ≈ 0.7159` +- `BLEED_ASPECT_RATIO = 69.35/94.35 ≈ 0.7350` + +**Validated against a real, source-diverse sample** (one card per +distinct source, 40 sources, not the earlier 30-card OCR-selection +sample - this needed source diversity, not OCR-selection-order +diversity): a clean, well-separated bimodal signal. Every source but one +clustered tightly at ratio 0.7325-0.7393 (bleed present); the one +exception measured 0.7163, matching the theoretical trim ratio almost +exactly. Nothing observed in the gap between clusters. Classification: +nearest-reference-ratio, abstaining (no vote) when the ratio is more +than 0.03 from BOTH references (`classify_bleed_edge`, +`local_fallback.py`) - comfortably covers the observed real spread on +either side while still abstaining on a genuinely non-standard image +(a DFC composite scan, a token, a corrupted fetch). + +**Wired into `run_pilot`**: fires for every card with a fetched image, +independent of printing-vote success (same "double duty" convention as +border/frame attribute votes) - positive (`APPLY`) vote for bleed, +negative (`NOT_APPLICABLE`) for trimmed, abstain (nothing written, only +counted) for ambiguous. `VoteSource.OCR`, confidence 0.7 +(`BLEED_EDGE_VOTE_CONFIDENCE`). No ground-truth counterpart to prefer - +unlike border/frame, Scryfall doesn't encode this at all. + ### No-match autopsy (2026-07-15, post-merge Hold #1 of the pre-scale program) Classified all 176 OCR "parsed-but-no-match" cases from the pilot run From f50c308d36b523e0586b724ea5628d442a1eca87 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Wed, 15 Jul 2026 20:33:40 +0000 Subject: [PATCH 08/23] Addendum item 8: DPI-tag audit (report only) + deferred art-crop DPI note --- docs/features/printing-tags.md | 74 ++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index 29bc0abb8..444d24fc7 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -1077,6 +1077,80 @@ counted) for ambiguous. `VoteSource.OCR`, confidence 0.7 (`BLEED_EDGE_VOTE_CONFIDENCE`). No ground-truth counterpart to prefer - unlike border/frame, Scryfall doesn't encode this at all. +### DPI-tag audit (2026-07-15, addendum item 8 - report only) + +Live, read-only cross-reference of `Card.dpi` (computed once at import +time, `update_database.py`) against both places tag state lives - +`Card.tags` (resolved/baked) and `CardTagVote` (raw votes, including +anything pending a moderator co-sign) - for the `low-res` SENSITIVE tag +specifically. No votes cast, no code changed; report only, per the +addendum's own scope, and `low-res` itself stays untouched by this item +(distinct from `appropriate-bleed` above - see the "Future work" note +below for a follow-up idea that WOULD vote on it, deliberately deferred). + +**Findings (218,152 cards, live 2026-07-15):** + +| dpi bucket | count | resolved `low-res` | pending vote | neither | +| ---------- | -----: | -----------------: | -----------: | ------: | +| 0 (unset) | 4 | 0 | 0 | 4 | +| 1-99 | 7 | 0 | 0 | 7 | +| 100-149 | 9 | 0 | 0 | 9 | +| 150-199 | 3 | 0 | 0 | 3 | +| 200-299 | 40 | 0 | 0 | 40 | +| 300+ | 218089 | 0 | 0 | 218089 | + +Two things worth flagging, neither actionable within this item's scope: + +- **99.97% of cards already report full 300dpi** - `Card.dpi` isn't a + useful prioritization signal on its own; the sub-300 tail is 63 cards + total across the whole catalog. +- **The `low-res` tag has never been used, anywhere, by anyone** - 0 + resolved, 0 pending, independent of dpi bucket. The report-flow + (`CardReportReason.LOW_QUALITY` -> `low-res` `CardTagVote`, + `sensitive_tags.REPORT_REASON_TO_TAG_NAME`) exists in code and has + never actually been exercised in production. Not a bug - just means + there's no existing signal to reconcile against yet, and any future + automated low-res detection (see below) would be establishing this + tag's first real usage, not correcting drift from manual reports. + +**Future work: art-crop-specific DPI check + Scryfall comparison +(2026-07-15, flagged by owner during this item, deliberately deferred - +not built).** `Card.dpi` measures the FULL card image's resolution, which +this audit shows is essentially always fine (99.97% at 300dpi) - but a +proxy can have a perfectly fine full-image dpi while still having a +genuinely blurry/undersized ART specifically (upscaled source, a bad +crop-and-stretch, etc.), which `Card.dpi` can't see. Sketched design, +explicitly NOT built this pass: + +- Reuse `local_phash.ART_CROP_BOX` (already the art-region fraction used + for phash matching - one definition, not a second one) to crop the art + region out of the pilot's own fetched image. +- Reuse the just-built `classify_bleed_edge` result to pick the correct + physical reference height per card (trim 88mm vs. bleed-inclusive + 94.35mm - see the bleed-edge section above) before converting the + crop's pixel height to a real DPI number, rather than assuming one. +- Cross-check against Scryfall's own official `art_crop` image for the + same printing (`local_phash._fetch_scryfall_art_crop_url` already + fetches this, reused not reinvented) as a second, comparative signal - + independent of the absolute-DPI estimate, catches "much smaller than + the official art for this exact card" even if the physical-mm math has + slack in it. +- **This is additive, not a replacement for `Card.dpi`** - `Card.dpi` + stays as the whole-image import-time measurement it already is; this + would be a second, art-specific signal alongside it, for a different + failure mode `Card.dpi` structurally can't catch. +- **Goes straight to the moderation pipeline when built** - `low-res` is + `TagModerationClass.SENSITIVE` (same property established for + `appropriate-bleed` above: a moderator co-sign is required to resolve + either way, so a machine vote alone can't misuse it), so this would + cast real `CardTagVote`s, not just report - unlike this item's + DB-audit scope, which deliberately doesn't. +- Deferred rather than built now because it needs its own validation + pass (a real sample, Scryfall-vs-source comparison, a derived + threshold with a safety margin - the same discipline every other + detector in this pilot got) before casting anything real, and this + item's own scope was report-only. + ### No-match autopsy (2026-07-15, post-merge Hold #1 of the pre-scale program) Classified all 176 OCR "parsed-but-no-match" cases from the pilot run From 6df2d2fa81334bd8853c5d35b9816df49214a7c3 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Wed, 15 Jul 2026 20:36:27 +0000 Subject: [PATCH 09/23] Validate item 8's DPI-tag audit query mechanics against known-nonzero tags --- docs/features/printing-tags.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index 444d24fc7..3cde664f9 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -1099,6 +1099,15 @@ below for a follow-up idea that WOULD vote on it, deliberately deferred). | 200-299 | 40 | 0 | 0 | 40 | | 300+ | 218089 | 0 | 0 | 218089 | +**Query mechanics sanity-checked before trusting an all-zero result**: +the same `resolved`/`pending` query pattern run against tags known to +have real production data - `NSFW` (339 resolved, via filename-bracket +import tagging, not the vote flow), and `custom-art`/`AI-Generated`/ +`Borderless` (1, 1, 13 genuinely pending via the exact same +`tag_votes__tag__name=...` pattern used above) - all returned correct +nonzero counts. The `low-res` all-zero result is a real finding, not a +broken query. + Two things worth flagging, neither actionable within this item's scope: - **99.97% of cards already report full 300dpi** - `Card.dpi` isn't a From a9e57dbf9bf3f64f687eedf8c79f5f8f5d690926 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Wed, 15 Jul 2026 21:23:46 +0000 Subject: [PATCH 10/23] Item 3d: pipeline concurrency + bleed-first crop normalization Split per-card compute (fetch/OCR/phash/border/frame/fallback) from the sequential DB-write loop, run via ThreadPoolExecutor validated against real live-API contention (2 workers, ~2.1x speedup, ~5ms extra latency on this box's 2 cores). Bleed classification now runs first and normalizes every other fixed-fraction crop box for the trimmed-image minority. Fixed a real cross-thread DB-visibility bug caught by the new concurrency tests (transactional_db, matching test_sources.py's existing precedent for update_database()'s own worker threads). --- MPCAutofill/cardpicker/local_fallback.py | 91 ++- .../local_identify_printing_tags.py | 608 +++++++++++------- MPCAutofill/cardpicker/local_phash.py | 8 +- .../commands/local_identify_printing_tags.py | 19 +- .../test_local_identify_printing_tags.py | 166 ++++- docs/features/printing-tags.md | 120 ++++ 6 files changed, 734 insertions(+), 278 deletions(-) diff --git a/MPCAutofill/cardpicker/local_fallback.py b/MPCAutofill/cardpicker/local_fallback.py index 33dc24a47..327336813 100644 --- a/MPCAutofill/cardpicker/local_fallback.py +++ b/MPCAutofill/cardpicker/local_fallback.py @@ -106,19 +106,23 @@ def match_artist( return surviving or None -def detect_illus_anchor(card_image: "Image.Image", ocr_raw_texts: list[str]) -> tuple[bool, Optional[str]]: +def detect_illus_anchor( + card_image: "Image.Image", ocr_raw_texts: list[str], bleed_class: Optional[str] = None +) -> tuple[bool, Optional[str]]: """The "Illus." extraction step, standalone from candidate matching - used both by run_fallback_for_card (which also needs the extracted name for match_artist) and by the frame-style classifier (which only needs to know whether the anchor fired at all, for every card regardless of whether pass 1 already produced a printing vote - see classify_frame_style). Returns (fired, extracted_name); reuses `ocr_raw_texts` (pass 1's already-computed OCR variants) before falling back to its own crop/OCR pass, same - rationale as run_fallback_for_card's identical shortcut.""" + rationale as run_fallback_for_card's identical shortcut. `bleed_class` (from + classify_bleed_edge, run once per card ahead of everything else - see run_pilot) remaps + ARTIST_CROP_BOX for a trimmed image via normalize_crop_box; a no-op otherwise.""" for text in ocr_raw_texts: name = extract_artist_name(text) if name is not None: return True, name - artist_crop = local_ocr.crop_collector_line(card_image, ARTIST_CROP_BOX) + artist_crop = local_ocr.crop_collector_line(card_image, normalize_crop_box(ARTIST_CROP_BOX, bleed_class)) for variant in local_ocr.preprocess_variants(artist_crop): name = extract_artist_name(local_ocr.run_tesseract(variant)) if name is not None: @@ -210,15 +214,19 @@ def _scan_strip_for_best_symbol_window(strip: "Image.Image", reference: "Image.I def find_symbol_matches( - card_image: "Image.Image", candidates: list["CandidatePrinting"], expansion_code_by_pk: dict[int, str] + card_image: "Image.Image", + candidates: list["CandidatePrinting"], + expansion_code_by_pk: dict[int, str], + bleed_class: Optional[str] = None, ) -> Optional[set[int]]: """Compares the card's symbol strip against each DISTINCT candidate expansion's rendered keyrune glyph (candidates sharing an expansion never need re-comparing), returns the candidates whose expansion produced the best distance within threshold and clear of the margin - None if no expansion's glyph could even be rendered (unmapped code) or nothing - cleared the threshold at all.""" + cleared the threshold at all. `bleed_class` remaps SYMBOL_STRIP_BOX via normalize_crop_box + for a trimmed image; a no-op otherwise.""" width, height = card_image.size - left, top, right, bottom = SYMBOL_STRIP_BOX + left, top, right, bottom = normalize_crop_box(SYMBOL_STRIP_BOX, bleed_class) strip = card_image.crop((int(left * width), int(top * height), int(right * width), int(bottom * height))).convert( "L" ) @@ -271,17 +279,21 @@ def find_symbol_matches( } -def classify_border_color(card_image: "Image.Image") -> Optional[str]: +def classify_border_color(card_image: "Image.Image", bleed_class: Optional[str] = None) -> Optional[str]: """Returns 'black'/'white'/'silver'/'borderless', or None if the sample is ambiguous (non-uniform - e.g. art bleeding right to the edge in a way that doesn't read as a clean 'borderless' card, or a color this taxonomy doesn't cover, e.g. gold/yellow - out of scope, - see docs/features/printing-tags.md's chip taxonomy v1 exclusions).""" + see docs/features/printing-tags.md's chip taxonomy v1 exclusions). `bleed_class` remaps each + of _BORDER_SAMPLE_BANDS via normalize_crop_box for a trimmed image; a no-op otherwise - + empirically checked (2026-07-15) that solid-color borders read identical RGB with or without + this remap on real bleed-inclusive images (border color extends uniformly through the bleed + margin), so applying it here unconditionally doesn't risk the majority case.""" import statistics width, height = card_image.size samples: list[tuple[int, int, int]] = [] stds: list[float] = [] - for left, top, right, bottom in _BORDER_SAMPLE_BANDS: + for left, top, right, bottom in (normalize_crop_box(band, bleed_class) for band in _BORDER_SAMPLE_BANDS): band = card_image.crop( (int(left * width), int(top * height), int(right * width), int(bottom * height)) ).convert("RGB") @@ -456,6 +468,55 @@ def frame_style_is_consistent(frame_class: Optional[str], printing_frame_value: TRIM_ASPECT_RATIO = _CARD_TRIM_WIDTH_MM / _CARD_TRIM_HEIGHT_MM BLEED_ASPECT_RATIO = (_CARD_TRIM_WIDTH_MM + 2 * _BLEED_MARGIN_MM) / (_CARD_TRIM_HEIGHT_MM + 2 * _BLEED_MARGIN_MM) +# What fraction of the full image the bleed margin occupies per edge, on each axis - derived +# from the same reference geometry above, not a separate guess. Every fixed-fraction crop box +# in this module and local_ocr/local_phash (DEFAULT_CROP_BOX, ART_CROP_BOX, ARTIST_CROP_BOX, +# SYMBOL_STRIP_BOX, _BORDER_SAMPLE_BANDS) was empirically tuned against real fetched images, +# which are ~97.5% bleed-inclusive (see the 40-source validation above) - meaning those boxes +# are already implicitly calibrated for THAT convention, not a separate one needing correction. +# The ~2.5% TRIMMED minority is the one case where a box tuned against bleed-inclusive images +# lands in the wrong place: removing the bleed margin shifts where the same physical card +# position falls as a fraction of the (now smaller) full image. +_WIDTH_MARGIN_FRACTION = _BLEED_MARGIN_MM / (_CARD_TRIM_WIDTH_MM + 2 * _BLEED_MARGIN_MM) +_HEIGHT_MARGIN_FRACTION = _BLEED_MARGIN_MM / (_CARD_TRIM_HEIGHT_MM + 2 * _BLEED_MARGIN_MM) + + +def normalize_crop_box( + box: tuple[float, float, float, float], bleed_class: Optional[str] +) -> tuple[float, float, float, float]: + """Remaps a fixed-fraction crop box (tuned against a bleed-inclusive image, per the module + comment above) onto a TRIMMED image's own coordinate space - a no-op (returns `box` + unchanged) for 'bleed' or None (abstain - no confident reading, so no correction to apply + either), since those cases are already the convention the box was tuned against. + + Empirically checked before use (2026-07-15, not just derived): sampled real bleed-classified + cards' border-color bands with and without this remap applied - solid-color borders (the + common case) read IDENTICAL RGB regardless of exact sample position within the bleed zone + (border color extends uniformly through the bleed margin), confirming this is safe to apply + unconditionally across all five fixed-fraction crop sites without a special case for any one + of them. + """ + if bleed_class != "trimmed": + return box + left, top, right, bottom = box + + def _rescale(fraction: float, margin_fraction: float) -> float: + # clamped to [0, 1]: a box (or band, like _BORDER_SAMPLE_BANDS' edge samples) that sat + # entirely within the bleed margin on the original bleed-inclusive convention rescales + # to at-or-past the trimmed image's own edge - genuinely degenerate for a trimmed image + # (that content doesn't exist anymore, it was cut off), not a bug in the math. Callers + # already handle a resulting zero-area crop gracefully (empty-sample skip, see + # classify_border_color). + return min(1.0, max(0.0, (fraction - margin_fraction) / (1 - 2 * margin_fraction))) + + return ( + _rescale(left, _WIDTH_MARGIN_FRACTION), + _rescale(top, _HEIGHT_MARGIN_FRACTION), + _rescale(right, _WIDTH_MARGIN_FRACTION), + _rescale(bottom, _HEIGHT_MARGIN_FRACTION), + ) + + # real-world validation (2026-07-15, 40 cards sampled across 40 distinct sources): the bleed # cluster spread 0.7325-0.7393 (theoretical 0.7350), the one trimmed example measured 0.7163 # (theoretical 0.7159) - a clean, well-separated bimodal signal with nothing observed in the @@ -524,12 +585,15 @@ def run_fallback_for_card( selected: "SelectedCard", card_image: "Image.Image", ocr_raw_texts: list[str], + bleed_class: Optional[str] = None, ) -> FallbackOutcome: """`ocr_raw_texts` reuses pass 1's already-computed OCR variants where available (the orchestrator passes whatever it already ran) - this only runs the extra full-width artist crop/OCR pass when pass 1's own text didn't already contain an "Illus." match, avoiding a redundant tesseract call on cards where the artist line already happened to be visible in - the narrower pass-1 crop.""" + the narrower pass-1 crop. `bleed_class` (from classify_bleed_edge, run once per card ahead + of everything else - see run_pilot) is threaded through to every sub-check's own + fixed-fraction crop box via normalize_crop_box.""" candidate_pks = {c.pk for c in selected.candidates} canonicals = { c.pk: c @@ -543,13 +607,13 @@ def run_fallback_for_card( if getattr(c, "printing_metadata", None) is not None and c.printing_metadata.border_color } - border_color = classify_border_color(card_image) + border_color = classify_border_color(card_image, bleed_class) border_filtered = filter_by_border_color(border_color, selected.candidates, border_color_by_pk) - illus_anchor_fired, artist_name = detect_illus_anchor(card_image, ocr_raw_texts) + illus_anchor_fired, artist_name = detect_illus_anchor(card_image, ocr_raw_texts, bleed_class) artist_filtered = match_artist(artist_name, selected.candidates, artist_by_pk) if artist_name else None - symbol_filtered = find_symbol_matches(card_image, selected.candidates, expansion_code_by_pk) + symbol_filtered = find_symbol_matches(card_image, selected.candidates, expansion_code_by_pk, bleed_class) survivors = set(candidate_pks) evidence_types_used: list[str] = [] @@ -604,6 +668,7 @@ def run_fallback_for_card( "BLEED_EDGE_VOTE_CONFIDENCE", "classify_bleed_edge", "cast_bleed_edge_vote", + "normalize_crop_box", "FallbackOutcome", "run_fallback_for_card", ] diff --git a/MPCAutofill/cardpicker/local_identify_printing_tags.py b/MPCAutofill/cardpicker/local_identify_printing_tags.py index 6194c2672..5b5c51a83 100644 --- a/MPCAutofill/cardpicker/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/local_identify_printing_tags.py @@ -16,9 +16,11 @@ """ import collections +import functools import logging import os import time +from concurrent.futures import ThreadPoolExecutor from dataclasses import dataclass, field from io import BytesIO from typing import Iterable, Literal, Optional @@ -238,6 +240,27 @@ class CardOutcome: frame_reading_attempted: bool = False frame_class: Optional[str] = None frame_mismatch: bool = False # printing vote withheld: frame reading contradicts the match + image_fetched: bool = False # distinguishes "no image at all" from "image present but a + # reading came back ambiguous/None" for the abstain counters below (bleed_class is None in + # both cases - this field is what lets the caller tell them apart, same convention as + # frame_reading_attempted's identical purpose for the frame abstain counter). + # bleed classification (addendum item 7) - now computed FIRST, ahead of everything else in + # _compute_card, per the owner-directed reordering (2026-07-15): every other fixed-fraction + # crop box in this card's pipeline (OCR collector line, phash art crop, illus-anchor crop, + # symbol strip, border-sample bands) gets normalized against this reading via + # local_fallback.normalize_crop_box, so it has to be known before any of them run, not after. + bleed_class: Optional[str] = None + + +@dataclass(frozen=True) +class CardComputeResult: + """The output of _compute_card - everything about a card that can be computed independent of + every OTHER card's state (no DB writes, no shared counters) - see _compute_card's own + docstring for why this split exists (pre-scale program item 3d, pipeline concurrency).""" + + card_id: int + fetch_attempted: bool # counts against --fetch-budget - see run_pilot's chunked loop + outcome: CardOutcome @dataclass @@ -256,11 +279,15 @@ def run_ocr_for_card( selected: SelectedCard, image: Optional["Image.Image"], crop_box: tuple[float, float, float, float] = local_ocr.DEFAULT_CROP_BOX, + bleed_class: Optional[str] = None, ) -> OcrCardResult: + """`bleed_class` (from local_fallback.classify_bleed_edge, run once per card ahead of + everything else - see run_pilot) remaps `crop_box` via local_fallback.normalize_crop_box for + a trimmed image; a no-op otherwise.""" if image is None: return OcrCardResult(skip_reason="unfetchable-image") - cropped = local_ocr.crop_collector_line(image, crop_box) + cropped = local_ocr.crop_collector_line(image, local_fallback.normalize_crop_box(crop_box, bleed_class)) variants = local_ocr.preprocess_variants(cropped) result = OcrCardResult() @@ -287,7 +314,11 @@ def run_phash_for_card( distance_threshold: int = local_phash.DEFAULT_DISTANCE_THRESHOLD, margin: int = local_phash.DEFAULT_MARGIN, max_candidates: int = PHASH_MAX_CANDIDATES, + bleed_class: Optional[str] = None, ) -> tuple[Optional[EngineVote], str]: + """`bleed_class` (from local_fallback.classify_bleed_edge, run once per card ahead of + everything else - see run_pilot) remaps local_phash.ART_CROP_BOX via + local_fallback.normalize_crop_box for a trimmed image; a no-op otherwise.""" # checked first, before any candidate-hash fetch - see PHASH_MAX_CANDIDATES' comment for # why this matters (basic lands/staple commons can have hundreds of candidates) if len(selected.candidates) > max_candidates: @@ -296,7 +327,7 @@ def run_phash_for_card( if image is None: return None, "unfetchable-image" - card_hash = local_phash.compute_card_art_hash(image) + card_hash = local_phash.compute_card_art_hash(image, bleed_class) canonicals_by_pk = {c.pk: c for c in CanonicalCard.objects.filter(pk__in=[c.pk for c in selected.candidates])} candidates_with_hashes: list[tuple[CandidatePrinting, int]] = [] @@ -315,6 +346,98 @@ def run_phash_for_card( return EngineVote(engine="phash", printing_pk=match.candidate.pk, confidence=PHASH_CONFIDENCE, detail=detail), "" +# Default concurrent worker count (pre-scale program item 3d, 2026-07-15): measured, not +# assumed, against this box's real constraint - 2 CPU cores total, shared with 5 live production +# containers (Django/nginx/Postgres/Elasticsearch/worker). A live-contention test (10 real +# candidate cards, dry, fetch+OCR+phash only) compared this box's live API latency under three +# conditions: idle (79.8ms mean/94.7ms p95), the CURRENT single-threaded pilot running (88.7ms/ +# 126.1ms), and a 2-worker concurrent pool running (93.9ms/135.7ms) - only ~5ms extra mean +# latency for 2 workers over the ALREADY-EXISTING single-threaded impact, while wall clock for +# the same 10 cards dropped from 13.42s to 6.34s (near-ideal ~2.1x speedup matching the 2-core +# count - tesseract's subprocess-based OCR genuinely parallelizes here, the GIL is released +# during the subprocess wait). 2 matches the core count exactly; more workers would only add +# contention without real additional parallelism on this box. +DEFAULT_WORKERS = 2 + + +def _compute_card( + selected: SelectedCard, + ocr_selected_ids: set[int], + phash_selected_ids: set[int], + already_fallback_covered: set[int], + ocr_crop_box: tuple[float, float, float, float], + phash_distance_threshold: int, + phash_margin: int, + phash_max_candidates: int, + fetch_dpi: Optional[int], +) -> CardComputeResult: + """The parallelizable half of a card's work (pre-scale program item 3d): fetch + every + read-only heuristic reading (OCR, phash, border/frame/bleed classification, pass-2 + fallback) - no DB writes, no shared/nonlocal state, safe to run concurrently across cards + via ThreadPoolExecutor.map() (see run_pilot's chunked loop). Deliberately does NOT include + the ground-truth-preferred attribute override or the frame-mismatch consistency check - + both of those are tightly coupled to the write/consensus decision (which candidate_vote + ultimately gets accepted) and stay in run_pilot's own sequential loop, same as before this + split. + + Bleed classification runs FIRST, ahead of everything else (owner-directed reordering, + 2026-07-15) - it's the one reading every other fixed-fraction crop box in this function + needs (via local_fallback.normalize_crop_box) to know whether to correct itself for a + trimmed image, so it has to be available before OCR/phash/illus-anchor/border/symbol crop. + """ + card_id = selected.card.pk + outcome = CardOutcome(card_id=card_id) + fetch_attempted = get_worker_image_url(selected.card, fetch_dpi) is not None + image = fetch_card_image(selected.card, fetch_dpi) + ocr_raw_texts: list[str] = [] + + 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 + + if card_id in ocr_selected_ids: + ocr_result = run_ocr_for_card(selected, image, ocr_crop_box, bleed_class) + outcome.ocr_vote, outcome.ocr_skip_reason = ocr_result.vote, ocr_result.skip_reason + ocr_raw_texts = ocr_result.raw_texts + if card_id in phash_selected_ids: + outcome.phash_vote, outcome.phash_skip_reason = run_phash_for_card( + selected, image, phash_distance_threshold, phash_margin, phash_max_candidates, bleed_class + ) + + if outcome.ocr_vote is not None and outcome.phash_vote is not None: + if outcome.ocr_vote.printing_pk != outcome.phash_vote.printing_pk: + outcome.disagreement = True + + if image is not None: + outcome.border_color = local_fallback.classify_border_color(image, bleed_class) + illus_anchor_fired, _artist_name = local_fallback.detect_illus_anchor(image, ocr_raw_texts, bleed_class) + parsed_a_collector_number = card_id in ocr_selected_ids and bool( + outcome.ocr_vote is not None or outcome.ocr_skip_reason == "parsed-but-no-match" + ) + outcome.frame_reading_attempted = True + outcome.frame_class = local_fallback.classify_frame_style(parsed_a_collector_number, illus_anchor_fired) + + pass_1_accepted = (outcome.ocr_vote is not None or outcome.phash_vote is not None) and not outcome.disagreement + if not pass_1_accepted and card_id not in already_fallback_covered and image is not None: + fallback_outcome = local_fallback.run_fallback_for_card(selected, image, ocr_raw_texts, bleed_class) + outcome.fallback_skip_reason = fallback_outcome.skip_reason + outcome.fallback_evidence_types = fallback_outcome.evidence_types_used + if fallback_outcome.printing_pk is not None: + confidence = ( + FALLBACK_CONFIDENCE_MULTI_EVIDENCE + if len(fallback_outcome.evidence_types_used) >= 2 + else FALLBACK_CONFIDENCE_SINGLE_EVIDENCE + ) + outcome.fallback_vote = EngineVote( + engine="phash", # placeholder Engine literal - fallback isn't a selectable --engine + printing_pk=fallback_outcome.printing_pk, + confidence=confidence, + detail=",".join(fallback_outcome.evidence_types_used), + ) + + return CardComputeResult(card_id=card_id, fetch_attempted=fetch_attempted, outcome=outcome) + + @dataclass class PilotResult: engine: str @@ -364,6 +487,7 @@ def run_pilot( progress_every: int = 50, fetch_budget: Optional[int] = None, fetch_dpi: Optional[int] = DEFAULT_FETCH_DPI, + workers: int = DEFAULT_WORKERS, ) -> tuple[dict[str, PilotResult], AttributeReport]: if nice: try: @@ -446,255 +570,269 @@ def flush() -> None: budget_exhausted = False cards_attempted = 0 - for i, (card_id, selected) in enumerate(all_selected_by_card_id.items()): + # Pipeline concurrency (pre-scale program item 3d, 2026-07-15): the per-card COMPUTE work + # (fetch, OCR, phash, border/frame/bleed classification, pass-2 fallback - everything + # _compute_card does) is independent per card and safe to run concurrently; the per-card + # WRITE work below (votes_batch/tag_votes_batch staging, disagreement bookkeeping, + # ground-truth-preferred attribute overrides, the frame-mismatch consistency check) stays + # single-threaded and in selection order, completely UNCHANGED from before this split - only + # where its input comes from is different (a CardComputeResult instead of being computed + # inline). Chunked at `batch_size` granularity, reusing the SAME boundary as checkpointing's + # flush/gate-check (Stage 8 pre-scale program item 2) rather than introducing a second + # batching concept - each chunk's compute pool completes before that chunk's writes are + # staged and flushed, so write order and gate-check timing are identical to running with + # workers=1, just with the compute portion overlapped. + all_items = list(all_selected_by_card_id.items()) + total_cards = len(all_items) + workers = max(1, workers) + if workers > 1: + # tesseract's LSTM engine can use OpenMP internally - without this, N concurrent + # tesseract subprocesses (one per in-flight OCR call) could each ALSO try to + # multi-thread themselves, oversubscribing this box's 2 real cores well beyond + # `workers`. setdefault, not direct assignment - respects an operator's own override. + os.environ.setdefault("OMP_THREAD_LIMIT", "1") + compute = functools.partial( + _compute_card, + ocr_selected_ids=ocr_selected_ids, + phash_selected_ids=phash_selected_ids, + already_fallback_covered=already_fallback_covered, + ocr_crop_box=ocr_crop_box, + phash_distance_threshold=phash_distance_threshold, + phash_margin=phash_margin, + phash_max_candidates=phash_max_candidates, + fetch_dpi=fetch_dpi, + ) + + chunk_start = 0 + while chunk_start < total_cards: + # Fetch budget (pre-scale program item 3b, belt-and-suspenders alongside the image CDN + # Worker's own IMAGE_FULL_TIER_RATE_LIMITER - see the CLI command's --fetch-budget help): + # checked between chunks, not per-card - a chunk already in flight always runs to + # completion once started, so the real bound on an overshoot is one chunk's worth of + # fetches (<= batch_size), not zero. Acceptable given this is explicitly the secondary + # safeguard, not the primary one. if fetch_budget is not None and fetches_made >= fetch_budget: budget_exhausted = True break - cards_attempted += 1 - - outcome = CardOutcome(card_id=card_id) - if get_worker_image_url(selected.card, fetch_dpi) is not None: - fetches_made += 1 - image = fetch_card_image(selected.card, fetch_dpi) # shared across every engine that runs on this card - ocr_raw_texts: list[str] = [] - - if card_id in ocr_selected_ids: - ocr_result = run_ocr_for_card(selected, image, ocr_crop_box) - outcome.ocr_vote, outcome.ocr_skip_reason = ocr_result.vote, ocr_result.skip_reason - ocr_raw_texts = ocr_result.raw_texts - if card_id in phash_selected_ids: - outcome.phash_vote, outcome.phash_skip_reason = run_phash_for_card( - selected, image, phash_distance_threshold, phash_margin, phash_max_candidates - ) - - if outcome.ocr_vote is not None and outcome.phash_vote is not None: - if outcome.ocr_vote.printing_pk != outcome.phash_vote.printing_pk: - outcome.disagreement = True - - # border sample and frame-style classification are independent of printing-vote - # success - both run for every card with a fetched image, regardless of whether pass 1 - # (or pass 2 below) ever identifies a printing, so the consistency check further down - # has a frame reading to compare against even when pass 1 succeeded on its own. - if image is not None: - outcome.border_color = local_fallback.classify_border_color(image) - illus_anchor_fired, _artist_name = local_fallback.detect_illus_anchor(image, ocr_raw_texts) - parsed_a_collector_number = card_id in ocr_selected_ids and bool( - outcome.ocr_vote is not None or outcome.ocr_skip_reason == "parsed-but-no-match" - ) - outcome.frame_reading_attempted = True - outcome.frame_class = local_fallback.classify_frame_style(parsed_a_collector_number, illus_anchor_fired) - - # pass 2: only when pass 1 (whichever engines ran) landed no accepted, non-disagreeing - # vote for this card, and this card hasn't already been through fallback before. - pass_1_accepted = (outcome.ocr_vote is not None or outcome.phash_vote is not None) and not outcome.disagreement - if not pass_1_accepted and card_id not in already_fallback_covered and image is not None: - fallback_outcome = local_fallback.run_fallback_for_card(selected, image, ocr_raw_texts) - outcome.fallback_skip_reason = fallback_outcome.skip_reason - outcome.fallback_evidence_types = fallback_outcome.evidence_types_used - if fallback_outcome.printing_pk is not None: - confidence = ( - FALLBACK_CONFIDENCE_MULTI_EVIDENCE - if len(fallback_outcome.evidence_types_used) >= 2 - else FALLBACK_CONFIDENCE_SINGLE_EVIDENCE + chunk = all_items[chunk_start : chunk_start + batch_size] + chunk_start += len(chunk) + selected_in_chunk = [selected for _card_id, selected in chunk] + + if workers > 1: + with ThreadPoolExecutor(max_workers=workers) as pool: + # .map() preserves submission order in its results regardless of completion + # order - the write loop below sees cards in the exact same order it would with + # workers=1, so nothing downstream needs to know concurrency happened at all. + chunk_results = list(pool.map(compute, selected_in_chunk)) + else: + chunk_results = [compute(s) for s in selected_in_chunk] + + for compute_result in chunk_results: + card_id = compute_result.card_id + outcome = compute_result.outcome + cards_attempted += 1 + if compute_result.fetch_attempted: + fetches_made += 1 + + # Finalize + queue for write - a card's full cost (image fetch, OCR, phash, + # fallback) was already paid once in _compute_card above; nothing here depends on + # any OTHER card's outcome, only this card's own DB state (the frame-mismatch + # consistency check below re-queries the matched printing's own metadata, + # independent of processing order). + result_ocr = results.get("ocr") + result_phash = results.get("phash") + result_fallback = results["fallback"] + + printing_vote_withheld_for_frame_mismatch = False + # consistency check: only meaningful once a printing vote (from either pass) exists + # to compare against the observed frame reading. + candidate_vote = outcome.ocr_vote or outcome.phash_vote or outcome.fallback_vote + if outcome.frame_class is not None and candidate_vote is not None and not outcome.disagreement: + canonical = ( + CanonicalCard.objects.filter(pk=candidate_vote.printing_pk) + .select_related("printing_metadata") + .first() ) - outcome.fallback_vote = EngineVote( - engine="phash", # placeholder Engine literal - fallback isn't a selectable --engine - printing_pk=fallback_outcome.printing_pk, - confidence=confidence, - detail=",".join(fallback_outcome.evidence_types_used), + printing_frame_value = ( + canonical.printing_metadata.frame + if canonical is not None and getattr(canonical, "printing_metadata", None) is not None + else None ) + if not local_fallback.frame_style_is_consistent(outcome.frame_class, printing_frame_value): + outcome.frame_mismatch = True + printing_vote_withheld_for_frame_mismatch = True + attributes.frame_mismatches.append( + { + "card_id": card_id, + "observed_frame_class": outcome.frame_class, + "matched_printing_pk": candidate_vote.printing_pk, + "matched_printing_frame_value": printing_frame_value, + } + ) - # Finalize + queue for write - inlined here (rather than a second pass over a - # dict[int, CardOutcome] collected above) so a card's full cost (image fetch, OCR, - # phash, fallback) is only ever paid once before its result reaches the write batch; - # nothing here depends on any OTHER card's outcome, only this card's own DB state - # (the frame-mismatch consistency check below re-queries the matched printing's own - # metadata, independent of processing order). - result_ocr = results.get("ocr") - result_phash = results.get("phash") - result_fallback = results["fallback"] - - printing_vote_withheld_for_frame_mismatch = False - # consistency check: only meaningful once a printing vote (from either pass) exists to - # compare against the observed frame reading. - candidate_vote = outcome.ocr_vote or outcome.phash_vote or outcome.fallback_vote - if outcome.frame_class is not None and candidate_vote is not None and not outcome.disagreement: - canonical = ( - CanonicalCard.objects.filter(pk=candidate_vote.printing_pk).select_related("printing_metadata").first() - ) - printing_frame_value = ( - canonical.printing_metadata.frame - if canonical is not None and getattr(canonical, "printing_metadata", None) is not None - else None - ) - if not local_fallback.frame_style_is_consistent(outcome.frame_class, printing_frame_value): - outcome.frame_mismatch = True - printing_vote_withheld_for_frame_mismatch = True - attributes.frame_mismatches.append( - { - "card_id": card_id, - "observed_frame_class": outcome.frame_class, - "matched_printing_pk": candidate_vote.printing_pk, - "matched_printing_frame_value": printing_frame_value, - } + if outcome.disagreement: + assert ( + result_ocr is not None and result_phash is not None + ) # both engines ran, or there's no disagreement to detect + result_ocr.disagreements.append( + {"card_id": card_id, "ocr": outcome.ocr_vote, "phash": outcome.phash_vote} ) - - if outcome.disagreement: - assert ( - result_ocr is not None and result_phash is not None - ) # both engines ran, or there's no disagreement to detect - result_ocr.disagreements.append({"card_id": card_id, "ocr": outcome.ocr_vote, "phash": outcome.phash_vote}) - result_ocr.skip_counts["disagreement-with-other-engine"] += 1 - result_phash.skip_counts["disagreement-with-other-engine"] += 1 - else: - if outcome.ocr_vote is not None and result_ocr is not None: - if printing_vote_withheld_for_frame_mismatch: - result_ocr.skip_counts["frame-mismatch"] += 1 - else: - votes_batch.append( - CardPrintingTag( - card_id=card_id, - printing_id=outcome.ocr_vote.printing_pk, - is_no_match=False, - anonymous_id=OCR_ANONYMOUS_ID, - source=VoteSource.OCR, - confidence=outcome.ocr_vote.confidence, - ) - ) - result_ocr.votes_written += 1 - result_ocr.audit.append({"card_id": card_id, "raw_text": outcome.ocr_vote.detail}) - written_card_ids.append(card_id) - batch_written_card_ids.append(card_id) - elif outcome.ocr_skip_reason and result_ocr is not None: - result_ocr.skip_counts[outcome.ocr_skip_reason] += 1 - - if outcome.phash_vote is not None and result_phash is not None: - if printing_vote_withheld_for_frame_mismatch: - result_phash.skip_counts["frame-mismatch"] += 1 - else: - votes_batch.append( - CardPrintingTag( - card_id=card_id, - printing_id=outcome.phash_vote.printing_pk, - is_no_match=False, - anonymous_id=PHASH_ANONYMOUS_ID, - source=VoteSource.OCR, - confidence=outcome.phash_vote.confidence, + result_ocr.skip_counts["disagreement-with-other-engine"] += 1 + result_phash.skip_counts["disagreement-with-other-engine"] += 1 + else: + if outcome.ocr_vote is not None and result_ocr is not None: + if printing_vote_withheld_for_frame_mismatch: + result_ocr.skip_counts["frame-mismatch"] += 1 + else: + votes_batch.append( + CardPrintingTag( + card_id=card_id, + printing_id=outcome.ocr_vote.printing_pk, + is_no_match=False, + anonymous_id=OCR_ANONYMOUS_ID, + source=VoteSource.OCR, + confidence=outcome.ocr_vote.confidence, + ) ) - ) - result_phash.votes_written += 1 - result_phash.audit.append({"card_id": card_id, "detail": outcome.phash_vote.detail}) - if card_id not in written_card_ids: + result_ocr.votes_written += 1 + result_ocr.audit.append({"card_id": card_id, "raw_text": outcome.ocr_vote.detail}) written_card_ids.append(card_id) batch_written_card_ids.append(card_id) - elif outcome.phash_skip_reason and result_phash is not None: - result_phash.skip_counts[outcome.phash_skip_reason] += 1 - - if outcome.fallback_vote is not None: - if printing_vote_withheld_for_frame_mismatch: - result_fallback.skip_counts["frame-mismatch"] += 1 - else: - votes_batch.append( - CardPrintingTag( - card_id=card_id, - printing_id=outcome.fallback_vote.printing_pk, - is_no_match=False, - anonymous_id=FALLBACK_ANONYMOUS_ID, - source=VoteSource.OCR, - confidence=outcome.fallback_vote.confidence, + elif outcome.ocr_skip_reason and result_ocr is not None: + result_ocr.skip_counts[outcome.ocr_skip_reason] += 1 + + if outcome.phash_vote is not None and result_phash is not None: + if printing_vote_withheld_for_frame_mismatch: + result_phash.skip_counts["frame-mismatch"] += 1 + else: + votes_batch.append( + CardPrintingTag( + card_id=card_id, + printing_id=outcome.phash_vote.printing_pk, + is_no_match=False, + anonymous_id=PHASH_ANONYMOUS_ID, + source=VoteSource.OCR, + confidence=outcome.phash_vote.confidence, + ) ) - ) - result_fallback.votes_written += 1 - result_fallback.audit.append({"card_id": card_id, "evidence": outcome.fallback_vote.detail}) - if card_id not in written_card_ids: - written_card_ids.append(card_id) - batch_written_card_ids.append(card_id) - elif outcome.fallback_skip_reason: - result_fallback.skip_counts[outcome.fallback_skip_reason] += 1 - - # border/frame attribute votes are independent of printing-vote success or the - # consistency-check outcome above - they fire for any card a border/frame reading was - # taken on, per the module docstring's "double duty" note. BUT when a printing was - # actually confirmed for this card this run, ground truth from that printing's own - # CanonicalPrintingMetadata (Scryfall border_color/frame) is preferred over the pixel/ - # OCR heuristic estimate - the heuristic's whole purpose was to independently validate - # an uncertain match (the consistency check above needs an independent signal to compare - # against), not to guess an answer we now actually know. Falls back to the heuristic - # reading whenever no printing was confirmed this run, or the confirmed printing has no - # usable ground truth for that particular attribute. - card = all_selected_by_card_id[card_id].card - confirmed_printing_pk = ( - candidate_vote.printing_pk - if candidate_vote is not None and not outcome.disagreement and not printing_vote_withheld_for_frame_mismatch - else None - ) - ground_truth_metadata = None - if confirmed_printing_pk is not None: - confirmed_canonical = ( - CanonicalCard.objects.filter(pk=confirmed_printing_pk).select_related("printing_metadata").first() + result_phash.votes_written += 1 + result_phash.audit.append({"card_id": card_id, "detail": outcome.phash_vote.detail}) + if card_id not in written_card_ids: + written_card_ids.append(card_id) + batch_written_card_ids.append(card_id) + elif outcome.phash_skip_reason and result_phash is not None: + result_phash.skip_counts[outcome.phash_skip_reason] += 1 + + if outcome.fallback_vote is not None: + if printing_vote_withheld_for_frame_mismatch: + result_fallback.skip_counts["frame-mismatch"] += 1 + else: + votes_batch.append( + CardPrintingTag( + card_id=card_id, + printing_id=outcome.fallback_vote.printing_pk, + is_no_match=False, + anonymous_id=FALLBACK_ANONYMOUS_ID, + source=VoteSource.OCR, + confidence=outcome.fallback_vote.confidence, + ) + ) + result_fallback.votes_written += 1 + result_fallback.audit.append({"card_id": card_id, "evidence": outcome.fallback_vote.detail}) + if card_id not in written_card_ids: + written_card_ids.append(card_id) + batch_written_card_ids.append(card_id) + elif outcome.fallback_skip_reason: + result_fallback.skip_counts[outcome.fallback_skip_reason] += 1 + + # border/frame attribute votes are independent of printing-vote success or the + # consistency-check outcome above - they fire for any card a border/frame reading + # was taken on, per the module docstring's "double duty" note. BUT when a printing + # was actually confirmed for this card this run, ground truth from that printing's + # own CanonicalPrintingMetadata (Scryfall border_color/frame) is preferred over the + # pixel/OCR heuristic estimate - the heuristic's whole purpose was to independently + # validate an uncertain match (the consistency check above needs an independent + # signal to compare against), not to guess an answer we now actually know. Falls + # back to the heuristic reading whenever no printing was confirmed this run, or the + # confirmed printing has no usable ground truth for that particular attribute. + card = all_selected_by_card_id[card_id].card + confirmed_printing_pk = ( + candidate_vote.printing_pk + if candidate_vote is not None + and not outcome.disagreement + and not printing_vote_withheld_for_frame_mismatch + else None ) - if confirmed_canonical is not None and getattr(confirmed_canonical, "printing_metadata", None) is not None: - ground_truth_metadata = confirmed_canonical.printing_metadata - - border_class = outcome.border_color - border_confidence = local_fallback.BORDER_ATTRIBUTE_VOTE_CONFIDENCE - if ground_truth_metadata is not None and ground_truth_metadata.border_color: - # gate on a known tag mapping before overriding - Scryfall's border_color can be - # "gold", outside this v1 taxonomy (see local_fallback.BORDER_COLOR_TO_TAG's - # docstring); an unmapped ground truth value must not discard a valid heuristic - # reading in favour of a vote that will silently resolve to nothing. - ground_truth_border_class = ground_truth_metadata.border_color - if ground_truth_border_class in local_fallback.BORDER_COLOR_TO_TAG: - border_class = ground_truth_border_class - border_confidence = local_fallback.GROUND_TRUTH_ATTRIBUTE_VOTE_CONFIDENCE - attributes.border_ground_truth_count += 1 - - if border_class is not None: - attributes.border_votes_by_class[border_class] += 1 - border_vote = local_fallback.cast_border_attribute_vote(card, border_class, confidence=border_confidence) - if border_vote is not None and not dry_run: - tag_votes_batch.append(border_vote) - - frame_class = outcome.frame_class - frame_confidence = local_fallback.FRAME_VOTE_CONFIDENCE - if ground_truth_metadata is not None and ground_truth_metadata.frame: - ground_truth_frame_class = local_fallback.FRAME_VALUE_TO_CLASS.get(ground_truth_metadata.frame) - if ground_truth_frame_class is not None: - frame_class = ground_truth_frame_class - frame_confidence = local_fallback.GROUND_TRUTH_ATTRIBUTE_VOTE_CONFIDENCE - attributes.frame_ground_truth_count += 1 - - if outcome.frame_reading_attempted: - if frame_class is not None: - attributes.frame_votes_by_class[frame_class] += 1 - frame_vote = local_fallback.cast_frame_style_vote(card, frame_class, confidence=frame_confidence) - if frame_vote is not None and not dry_run: - tag_votes_batch.append(frame_vote) - else: - attributes.frame_abstain_count += 1 - - # addendum item 7: bleed-edge classification - independent of printing-vote success, - # same "fires for any card with a fetched image" convention as border/frame above, and - # (unlike those two) has no ground-truth counterpart to prefer, since Scryfall doesn't - # encode this at all. - if image is not None: - bleed_class = local_fallback.classify_bleed_edge(image) - if bleed_class is not None: - attributes.bleed_votes_by_class[bleed_class] += 1 - bleed_vote = local_fallback.cast_bleed_edge_vote(card, bleed_class) + ground_truth_metadata = None + if confirmed_printing_pk is not None: + confirmed_canonical = ( + CanonicalCard.objects.filter(pk=confirmed_printing_pk).select_related("printing_metadata").first() + ) + if ( + confirmed_canonical is not None + and getattr(confirmed_canonical, "printing_metadata", None) is not None + ): + ground_truth_metadata = confirmed_canonical.printing_metadata + + border_class = outcome.border_color + border_confidence = local_fallback.BORDER_ATTRIBUTE_VOTE_CONFIDENCE + if ground_truth_metadata is not None and ground_truth_metadata.border_color: + # gate on a known tag mapping before overriding - Scryfall's border_color can be + # "gold", outside this v1 taxonomy (see local_fallback.BORDER_COLOR_TO_TAG's + # docstring); an unmapped ground truth value must not discard a valid heuristic + # reading in favour of a vote that will silently resolve to nothing. + ground_truth_border_class = ground_truth_metadata.border_color + if ground_truth_border_class in local_fallback.BORDER_COLOR_TO_TAG: + border_class = ground_truth_border_class + border_confidence = local_fallback.GROUND_TRUTH_ATTRIBUTE_VOTE_CONFIDENCE + attributes.border_ground_truth_count += 1 + + if border_class is not None: + attributes.border_votes_by_class[border_class] += 1 + border_vote = local_fallback.cast_border_attribute_vote( + card, border_class, confidence=border_confidence + ) + if border_vote is not None and not dry_run: + tag_votes_batch.append(border_vote) + + frame_class = outcome.frame_class + frame_confidence = local_fallback.FRAME_VOTE_CONFIDENCE + if ground_truth_metadata is not None and ground_truth_metadata.frame: + ground_truth_frame_class = local_fallback.FRAME_VALUE_TO_CLASS.get(ground_truth_metadata.frame) + if ground_truth_frame_class is not None: + frame_class = ground_truth_frame_class + frame_confidence = local_fallback.GROUND_TRUTH_ATTRIBUTE_VOTE_CONFIDENCE + attributes.frame_ground_truth_count += 1 + + if outcome.frame_reading_attempted: + if frame_class is not None: + attributes.frame_votes_by_class[frame_class] += 1 + frame_vote = local_fallback.cast_frame_style_vote(card, frame_class, confidence=frame_confidence) + if frame_vote is not None and not dry_run: + tag_votes_batch.append(frame_vote) + else: + attributes.frame_abstain_count += 1 + + # addendum item 7: bleed-edge classification - independent of printing-vote success, + # same "fires for any card with a fetched image" convention as border/frame above, + # and (unlike those two) has no ground-truth counterpart to prefer, since Scryfall + # doesn't encode this at all. Already computed once in _compute_card - FIRST, ahead + # of everything else (see that function's docstring) - so this reads outcome.bleed_ + # class/outcome.image_fetched rather than recomputing against `image` (which is no + # longer available here now that fetch+compute moved into _compute_card). + if outcome.bleed_class is not None: + attributes.bleed_votes_by_class[outcome.bleed_class] += 1 + bleed_vote = local_fallback.cast_bleed_edge_vote(card, outcome.bleed_class) if bleed_vote is not None and not dry_run: tag_votes_batch.append(bleed_vote) - else: + elif outcome.image_fetched: attributes.bleed_abstain_count += 1 - if (i + 1) % batch_size == 0: - flush() - if nice and i % 20 == 0: + flush() + if nice: time.sleep(_NICE_SLEEP_SECONDS) - if progress_every and (i + 1) % progress_every == 0: - print(f" ... {i + 1}/{len(all_selected_by_card_id)} candidates processed") - - flush() + if progress_every and chunk_start % progress_every < len(chunk): + print(f" ... {chunk_start}/{total_cards} candidates processed") cards_not_attempted = len(all_selected_by_card_id) - cards_attempted for result in results.values(): @@ -736,6 +874,8 @@ def verify_zero_resolutions(card_ids: list[int], batch_size: int = 2000) -> list "fetch_card_image", "EngineVote", "CardOutcome", + "CardComputeResult", + "DEFAULT_WORKERS", "run_ocr_for_card", "run_phash_for_card", "PilotResult", diff --git a/MPCAutofill/cardpicker/local_phash.py b/MPCAutofill/cardpicker/local_phash.py index 4429afa32..98e045af5 100644 --- a/MPCAutofill/cardpicker/local_phash.py +++ b/MPCAutofill/cardpicker/local_phash.py @@ -18,6 +18,7 @@ import requests from PIL import Image +from cardpicker.local_fallback import normalize_crop_box from cardpicker.models import CanonicalCard from cardpicker.utils import twos_complement @@ -104,9 +105,12 @@ def get_or_compute_canonical_hash(canonical: CanonicalCard) -> Optional[int]: return computed -def compute_card_art_hash(card_image: "Image.Image") -> int: +def compute_card_art_hash(card_image: "Image.Image", bleed_class: Optional[str] = None) -> int: + """`bleed_class` (from local_fallback.classify_bleed_edge, run once per card ahead of + everything else - see run_pilot) remaps ART_CROP_BOX via local_fallback.normalize_crop_box + for a trimmed image; a no-op otherwise.""" width, height = card_image.size - left, top, right, bottom = ART_CROP_BOX + left, top, right, bottom = normalize_crop_box(ART_CROP_BOX, bleed_class) art_region = card_image.crop((int(left * width), int(top * height), int(right * width), int(bottom * height))) return _hash_to_int(imagehash.phash(art_region)) diff --git a/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py b/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py index 3af16a66c..294ad80ad 100644 --- a/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py @@ -124,6 +124,21 @@ def add_arguments(self, parser: Any) -> None: f"{local_identify_printing_tags.DEFAULT_FETCH_DPI} (margin above the empirically-best " "200). Pass --fetch-dpi=0 for uncapped native resolution.", ) + parser.add_argument( + "--workers", + type=int, + default=local_identify_printing_tags.DEFAULT_WORKERS, + help="Concurrent worker threads for the fetch+OCR+phash+fallback compute portion of " + "each card (pre-scale program item 3d) - the DB-write portion stays single-threaded " + "regardless. Measured, not assumed (2026-07-15): on this box (2 CPU cores, shared " + "with 5 live production containers), 2 workers gave a near-ideal ~2.1x wall-clock " + "speedup (tesseract's subprocess-based OCR genuinely parallelizes - the GIL releases " + "during the subprocess wait) for only ~5ms extra live-API latency over the " + "ALREADY-EXISTING single-threaded impact. Default: " + f"{local_identify_printing_tags.DEFAULT_WORKERS} (matches this box's core count - " + "more would only add contention, not real parallelism). Pass --workers=1 to disable " + "concurrency entirely.", + ) def handle(self, *args: Any, **kwargs: Any) -> None: engine = kwargs["engine"] @@ -138,6 +153,7 @@ def handle(self, *args: Any, **kwargs: Any) -> None: fetch_dpi: Optional[int] = kwargs["fetch_dpi"] if fetch_dpi == 0: fetch_dpi = None + workers = kwargs["workers"] def _parse_source_pks(raw: str) -> list[int]: return [int(p) for p in raw.split(",") if p.strip()] @@ -169,7 +185,7 @@ def _parse_source_pks(raw: str) -> list[int]: print( f"[{mode}] local_identify_printing_tags --engine={engine} --limit={limit} " f"--nice={nice} --crop-box={crop_box} --batch-size={batch_size} " - f"--fetch-budget={fetch_budget} --fetch-dpi={fetch_dpi} " + f"--fetch-budget={fetch_budget} --fetch-dpi={fetch_dpi} --workers={workers} " f"--exclude-sources-ocr={exclude_source_pks_by_engine['ocr']} " f"--exclude-sources-phash={exclude_source_pks_by_engine['phash']}" ) @@ -187,6 +203,7 @@ def _parse_source_pks(raw: str) -> list[int]: batch_size=batch_size, fetch_budget=fetch_budget, fetch_dpi=fetch_dpi, + workers=workers, ) gate_violations: list[int] = [] diff --git a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py index 180208dce..cac7a9c18 100644 --- a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py @@ -8,6 +8,7 @@ it, only the host venv used for the real pilot run does). """ +import os import shutil import pytest @@ -417,12 +418,12 @@ def test_both_engines_agree_keeps_both_votes(self, db, monkeypatch): import cardpicker.local_identify_printing_tags as module - def fake_ocr(selected, image, crop_box): + def fake_ocr(selected, image, crop_box, bleed_class=None): return module.OcrCardResult( vote=module.EngineVote(engine="ocr", printing_pk=printing.pk, confidence=0.85, detail="raw") ) - def fake_phash(selected, image, threshold, margin, max_candidates): + def fake_phash(selected, image, threshold, margin, max_candidates, bleed_class=None): return module.EngineVote(engine="phash", printing_pk=printing.pk, confidence=0.8, detail="d=0"), "" monkeypatch.setattr(module, "run_ocr_for_card", fake_ocr) @@ -446,12 +447,12 @@ def test_both_engines_disagree_writes_neither(self, db, monkeypatch): import cardpicker.local_identify_printing_tags as module - def fake_ocr(selected, image, crop_box): + def fake_ocr(selected, image, crop_box, bleed_class=None): return module.OcrCardResult( vote=module.EngineVote(engine="ocr", printing_pk=printing_a.pk, confidence=0.85, detail="raw") ) - def fake_phash(selected, image, threshold, margin, max_candidates): + def fake_phash(selected, image, threshold, margin, max_candidates, bleed_class=None): return module.EngineVote(engine="phash", printing_pk=printing_b.pk, confidence=0.8, detail="d=0"), "" monkeypatch.setattr(module, "run_ocr_for_card", fake_ocr) @@ -474,7 +475,7 @@ def test_dry_run_writes_nothing(self, db, monkeypatch): import cardpicker.local_identify_printing_tags as module - def fake_ocr(selected, image, crop_box): + def fake_ocr(selected, image, crop_box, bleed_class=None): return module.OcrCardResult( vote=module.EngineVote(engine="ocr", printing_pk=printing.pk, confidence=0.85, detail="raw") ) @@ -505,7 +506,7 @@ def test_excluded_sources_cards_never_reach_the_engine(self, db, monkeypatch): import cardpicker.local_identify_printing_tags as module - def fake_ocr(selected, image, crop_box): + def fake_ocr(selected, image, crop_box, bleed_class=None): return module.OcrCardResult( vote=module.EngineVote(engine="ocr", printing_pk=printing.pk, confidence=0.85, detail="raw") ) @@ -537,7 +538,7 @@ class TestCheckpointing: def _wire_fake_ocr(monkeypatch, printing_pk): import cardpicker.local_identify_printing_tags as module - def fake_ocr(selected, image, crop_box): + def fake_ocr(selected, image, crop_box, bleed_class=None): return module.OcrCardResult( vote=module.EngineVote(engine="ocr", printing_pk=printing_pk, confidence=0.85, detail="raw") ) @@ -661,7 +662,15 @@ def test_stops_after_the_budget_and_leaves_the_rest_untouched(self, db, monkeypa cards = [CardFactory(name="Forest") for _ in range(5)] TestCheckpointing._wire_fake_ocr(monkeypatch, printing.pk) - results, _attributes = run_pilot(engine="ocr", limit=10, dry_run=False, nice=False, fetch_budget=3) + # batch_size=3 matches fetch_budget=3 deliberately (pre-scale program item 3d): the + # budget is now checked BETWEEN chunks, not per-card - a chunk already in flight always + # completes (see run_pilot's own comment on this), so the real bound on overshoot is one + # chunk's worth. Aligning the two here keeps this test's exact-count assertions valid; + # a batch_size that DIDN'T divide evenly would still stop correctly, just with the + # (already-documented, already-accepted) chunk-sized overshoot instead of an exact cut. + results, _attributes = run_pilot( + engine="ocr", limit=10, dry_run=False, nice=False, fetch_budget=3, batch_size=3, workers=1 + ) assert results["ocr"].votes_written == 3 assert results["ocr"].fetch_budget_exhausted is True @@ -701,7 +710,7 @@ def test_a_card_voted_on_is_excluded_from_the_next_selection(self, db, monkeypat monkeypatch.setattr( module, "run_ocr_for_card", - lambda selected, image, crop_box: module.OcrCardResult( + lambda selected, image, crop_box, bleed_class=None: module.OcrCardResult( vote=module.EngineVote(engine="ocr", printing_pk=printing.pk, confidence=0.85, detail="raw") ), ) @@ -733,10 +742,18 @@ def test_detects_a_violation(self, db, monkeypatch): def _black_bordered_image_with_artist_text(artist_name: str) -> Image: - img = Image.new("RGB", (750, 1050), (5, 5, 5)) + # 750x1020 (ratio ~0.7353) is deliberately a clean "bleed" shape (BLEED_ASPECT_RATIO + # ~0.7350, distance 0.0003) rather than the original 750x1050 (ratio 0.7143), which landed + # just inside classify_bleed_edge's "trimmed" bucket by accident - that silently triggered + # local_fallback.normalize_crop_box's trimmed-image remap on ARTIST_CROP_BOX/ + # _BORDER_SAMPLE_BANDS, shifting them away from where this synthetic image actually draws + # its content. This test is about fallback wiring, not bleed-remap correctness (that's + # covered separately in test_local_fallback.py) - matching the real-world majority shape + # keeps normalize_crop_box a no-op here, same as it is for ~97.5% of real images. + img = Image.new("RGB", (750, 1020), (5, 5, 5)) draw = ImageDraw.Draw(img) - draw.rectangle([60, 60, 690, 990], fill=(120, 80, 200)) - draw.text((150, 990), f"Illus. {artist_name}", fill=(255, 255, 255)) + draw.rectangle([60, 60, 690, 960], fill=(120, 80, 200)) + draw.text((150, 960), f"Illus. {artist_name}", fill=(255, 255, 255)) return img @@ -767,17 +784,26 @@ def test_fallback_fires_and_votes_when_pass_1_misses_entirely(self, db, monkeypa # on it, but the crop/OCR fallback inside detect_illus_anchor() must not depend on the # real binary reading it accurately; this mirrors what it would extract. monkeypatch.setattr(local_ocr_module, "run_tesseract", lambda image: "Illus. Marie Magny") - monkeypatch.setattr(module, "run_ocr_for_card", lambda selected, image, crop_box: module.OcrCardResult()) + monkeypatch.setattr( + module, "run_ocr_for_card", lambda selected, image, crop_box, bleed_class=None: module.OcrCardResult() + ) monkeypatch.setattr( module, "run_phash_for_card", - lambda selected, image, threshold, margin, max_candidates: (None, "no-clear-winner"), + lambda selected, image, threshold, margin, max_candidates, bleed_class=None: (None, "no-clear-winner"), ) monkeypatch.setattr( module, "fetch_card_image", lambda card, dpi=None: _black_bordered_image_with_artist_text("Marie Magny") ) - results, attributes = run_pilot(engine="both", limit=10, dry_run=False, nice=False) + # workers=1: this test exercises REAL (unmocked) run_fallback_for_card, which queries + # CanonicalCard/CanonicalPrintingMetadata/CanonicalArtist - under workers>1 those queries + # run on a worker thread's own DB connection, which can't see this test's fixture data + # under pytest-django's default (non-transactional) `db` fixture (only the original + # connection sees an uncommitted test transaction). Concurrency correctness itself is + # covered separately (TestConcurrency, using transactional_db) - this test is about + # fallback wiring, not concurrency, so it stays on the simple single-threaded path. + results, attributes = run_pilot(engine="both", limit=10, dry_run=False, nice=False, workers=1) assert results["fallback"].votes_written == 1 assert CardPrintingTag.objects.filter(card=card, anonymous_id=FALLBACK_ANONYMOUS_ID, printing=printing).exists() @@ -802,7 +828,7 @@ def test_frame_mismatch_withholds_the_printing_vote(self, db, monkeypatch): # detect_illus_anchor() call must not depend on the real binary being present to do so. monkeypatch.setattr(local_ocr_module, "run_tesseract", lambda image: "") - def fake_ocr(selected, image, crop_box): + def fake_ocr(selected, image, crop_box, bleed_class=None): return module.OcrCardResult( vote=module.EngineVote( engine="ocr", printing_pk=printing.pk, confidence=0.85, detail="158/281 R MOM EN" @@ -836,11 +862,13 @@ def fail_if_called(selected, image, ocr_raw_texts): raise AssertionError("run_fallback_for_card must not run again for an already-covered card") monkeypatch.setattr(module.local_fallback, "run_fallback_for_card", fail_if_called) - monkeypatch.setattr(module, "run_ocr_for_card", lambda selected, image, crop_box: module.OcrCardResult()) + monkeypatch.setattr( + module, "run_ocr_for_card", lambda selected, image, crop_box, bleed_class=None: module.OcrCardResult() + ) monkeypatch.setattr( module, "run_phash_for_card", - lambda selected, image, threshold, margin, max_candidates: (None, "no-clear-winner"), + lambda selected, image, threshold, margin, max_candidates, bleed_class=None: (None, "no-clear-winner"), ) monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: Image.new("RGB", (750, 1050), (5, 5, 5))) @@ -869,7 +897,7 @@ def test_ground_truth_overrides_heuristic_when_printing_confirmed(self, db, monk # real read would find anyway; see the identical note on TestPass2Wiring above. monkeypatch.setattr(local_ocr_module, "run_tesseract", lambda image: "") - def fake_ocr(selected, image, crop_box): + def fake_ocr(selected, image, crop_box, bleed_class=None): return module.OcrCardResult( vote=module.EngineVote( engine="ocr", printing_pk=printing.pk, confidence=0.85, detail="158/281 R MOM EN" @@ -909,11 +937,13 @@ def test_heuristic_used_when_no_printing_confirmed_this_run(self, db, monkeypatc # no real tesseract binary in CI - see the identical note on the sibling test above monkeypatch.setattr(local_ocr_module, "run_tesseract", lambda image: "") - monkeypatch.setattr(module, "run_ocr_for_card", lambda selected, image, crop_box: module.OcrCardResult()) + monkeypatch.setattr( + module, "run_ocr_for_card", lambda selected, image, crop_box, bleed_class=None: module.OcrCardResult() + ) monkeypatch.setattr( module, "run_phash_for_card", - lambda selected, image, threshold, margin, max_candidates: (None, "no-clear-winner"), + lambda selected, image, threshold, margin, max_candidates, bleed_class=None: (None, "no-clear-winner"), ) monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: Image.new("RGB", (750, 1050), (5, 5, 5))) @@ -937,7 +967,7 @@ def test_heuristic_used_when_confirmed_printing_has_no_usable_metadata(self, db, # no real tesseract binary in CI - see the identical note on the sibling test above monkeypatch.setattr(local_ocr_module, "run_tesseract", lambda image: "") - def fake_ocr(selected, image, crop_box): + def fake_ocr(selected, image, crop_box, bleed_class=None): return module.OcrCardResult( vote=module.EngineVote( engine="ocr", printing_pk=printing.pk, confidence=0.85, detail="158/281 R MOM EN" @@ -969,11 +999,13 @@ def test_bleed_shaped_image_casts_a_positive_vote(self, db, monkeypatch): import cardpicker.local_ocr as local_ocr_module monkeypatch.setattr(local_ocr_module, "run_tesseract", lambda image: "") - monkeypatch.setattr(module, "run_ocr_for_card", lambda selected, image, crop_box: module.OcrCardResult()) + monkeypatch.setattr( + module, "run_ocr_for_card", lambda selected, image, crop_box, bleed_class=None: module.OcrCardResult() + ) monkeypatch.setattr( module, "run_phash_for_card", - lambda selected, image, threshold, margin, max_candidates: (None, "too-many-candidates"), + lambda selected, image, threshold, margin, max_candidates, bleed_class=None: (None, "too-many-candidates"), ) # 735/1000 ~= BLEED_ASPECT_RATIO monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: Image.new("RGB", (735, 1000), (5, 5, 5))) @@ -993,11 +1025,13 @@ def test_trimmed_shaped_image_casts_a_negative_vote(self, db, monkeypatch): import cardpicker.local_ocr as local_ocr_module monkeypatch.setattr(local_ocr_module, "run_tesseract", lambda image: "") - monkeypatch.setattr(module, "run_ocr_for_card", lambda selected, image, crop_box: module.OcrCardResult()) + monkeypatch.setattr( + module, "run_ocr_for_card", lambda selected, image, crop_box, bleed_class=None: module.OcrCardResult() + ) monkeypatch.setattr( module, "run_phash_for_card", - lambda selected, image, threshold, margin, max_candidates: (None, "too-many-candidates"), + lambda selected, image, threshold, margin, max_candidates, bleed_class=None: (None, "too-many-candidates"), ) # 716/1000 ~= TRIM_ASPECT_RATIO monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: Image.new("RGB", (716, 1000), (5, 5, 5))) @@ -1017,11 +1051,13 @@ def test_ambiguous_ratio_abstains_without_writing_anything(self, db, monkeypatch import cardpicker.local_ocr as local_ocr_module monkeypatch.setattr(local_ocr_module, "run_tesseract", lambda image: "") - monkeypatch.setattr(module, "run_ocr_for_card", lambda selected, image, crop_box: module.OcrCardResult()) + monkeypatch.setattr( + module, "run_ocr_for_card", lambda selected, image, crop_box, bleed_class=None: module.OcrCardResult() + ) monkeypatch.setattr( module, "run_phash_for_card", - lambda selected, image, threshold, margin, max_candidates: (None, "too-many-candidates"), + lambda selected, image, threshold, margin, max_candidates, bleed_class=None: (None, "too-many-candidates"), ) monkeypatch.setattr( module, "fetch_card_image", lambda card, dpi=None: Image.new("RGB", (1000, 1000), (5, 5, 5)) @@ -1032,3 +1068,77 @@ def test_ambiguous_ratio_abstains_without_writing_anything(self, db, monkeypatch assert attributes.bleed_votes_by_class == {} assert attributes.bleed_abstain_count == 1 assert not CardTagVote.objects.filter(card=card, tag__name="appropriate-bleed").exists() + + +class TestConcurrency: + """Pre-scale program item 3d (2026-07-15): the per-card fetch+OCR+phash+fallback compute + work now runs across `workers` concurrent threads, feeding the same single-threaded + DB-write loop as before. `transactional_db` (real commits, TRUNCATE-based cleanup), not the + default rollback-wrapped `db` fixture - a real regression was caught writing these tests: + `run_phash_for_card`'s own `CanonicalCard.objects.filter(...)` query, running on a worker + thread's own DB connection, silently found nothing under `db` because that connection can't + see `db`'s uncommitted wrapping transaction - exact same rationale as + test_sources.py's `test_all_sources_scanned_concurrently_local_file` for + `update_database()`'s own worker threads.""" + + def test_workers_greater_than_one_still_finds_a_real_phash_match(self, transactional_db, monkeypatch): + # deliberately does NOT mock run_phash_for_card itself - the whole point is exercising + # its real CanonicalCard query from inside a worker thread. Only the network-dependent + # half (Scryfall art_crop fetch/hash) is mocked, pinned to exactly what the real + # compute_card_art_hash(card_image) will produce, guaranteeing a distance=0 match. + import cardpicker.local_identify_printing_tags as module + import cardpicker.local_phash as phash_module + + printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + card = CardFactory(name="Forest") + + card_image = Image.new("RGB", (750, 1050), (5, 5, 5)) + pinned_hash = phash_module.compute_card_art_hash(card_image) + + monkeypatch.setattr(phash_module, "get_or_compute_canonical_hash", lambda canonical: pinned_hash) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: card_image) + + results, _attributes = run_pilot(engine="phash", limit=10, dry_run=False, nice=False, workers=2) + + assert results["phash"].votes_written == 1 + assert CardPrintingTag.objects.filter(card=card, anonymous_id=PHASH_ANONYMOUS_ID, printing=printing).exists() + + def test_workers_one_and_workers_two_agree_on_the_same_real_input(self, transactional_db, monkeypatch): + import cardpicker.local_identify_printing_tags as module + import cardpicker.local_phash as phash_module + + CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + cards = [CardFactory(name="Forest") for _ in range(6)] + + card_image = Image.new("RGB", (750, 1050), (5, 5, 5)) + pinned_hash = phash_module.compute_card_art_hash(card_image) + monkeypatch.setattr(phash_module, "get_or_compute_canonical_hash", lambda canonical: pinned_hash) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: card_image) + + results_seq, _ = run_pilot(engine="phash", limit=10, dry_run=True, nice=False, workers=1) + + for c in cards: + c.refresh_from_db() + results_conc, _ = run_pilot(engine="phash", limit=10, dry_run=True, nice=False, workers=4, batch_size=2) + + assert results_seq["phash"].votes_written == results_conc["phash"].votes_written == 6 + + def test_omp_thread_limit_is_set_when_running_concurrently(self, db, monkeypatch): + monkeypatch.delenv("OMP_THREAD_LIMIT", raising=False) + import cardpicker.local_identify_printing_tags as module + + monkeypatch.setattr(module, "select_candidates", lambda *args, **kwargs: []) + + run_pilot(engine="ocr", limit=10, dry_run=True, nice=False, workers=3) + + assert os.environ.get("OMP_THREAD_LIMIT") == "1" + + def test_workers_one_does_not_set_omp_thread_limit(self, db, monkeypatch): + monkeypatch.delenv("OMP_THREAD_LIMIT", raising=False) + import cardpicker.local_identify_printing_tags as module + + monkeypatch.setattr(module, "select_candidates", lambda *args, **kwargs: []) + + run_pilot(engine="ocr", limit=10, dry_run=True, nice=False, workers=1) + + assert "OMP_THREAD_LIMIT" not in os.environ diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index 3cde664f9..8f889f9da 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -1160,6 +1160,126 @@ explicitly NOT built this pass: detector in this pilot got) before casting anything real, and this item's own scope was report-only. +### Bleed-first crop normalization (2026-07-15, owner-directed, folded into item 3d) + +**Owner's question mid-item-8**: since bleed classification is cheap and +purely geometric, should it run FIRST, ahead of everything else, so its +result can normalize every OTHER fixed-fraction crop box in the pipeline? +Investigated rather than assumed: `local_ocr.DEFAULT_CROP_BOX`, +`local_phash.ART_CROP_BOX`, `local_fallback.ARTIST_CROP_BOX`, +`local_fallback.SYMBOL_STRIP_BOX`, and `local_fallback._BORDER_SAMPLE_BANDS` +are all fixed-fraction boxes empirically tuned against real fetched images - +which are ~97.5% bleed-inclusive (the 40-source bleed sample above). That +means they're already correctly calibrated for the bleed-inclusive +majority; the ~2.5% TRIMMED minority is the one case where a box tuned +against a bleed-inclusive image lands in the wrong place, since removing +the bleed margin shifts where the same physical card position falls as a +fraction of the (now smaller) full image. + +**`local_fallback.normalize_crop_box(box, bleed_class)`**: a no-op for +`'bleed'` or `None` (abstain); for `'trimmed'`, rescales each fraction by +the same margin-fraction math derived from the bleed-edge section's own +reference geometry (`_WIDTH_MARGIN_FRACTION = 3.175 / 69.35 ≈ 4.58%`, +`_HEIGHT_MARGIN_FRACTION = 3.175 / 94.35 ≈ 3.37%` per edge). Threaded +through all five crop sites via a new `bleed_class` parameter on +`classify_border_color`, `detect_illus_anchor`, `find_symbol_matches`, +`run_ocr_for_card`, and `local_phash.compute_card_art_hash`. + +**The border-sample bands got a real empirical check, not just the +derivation, before being included** - their sample position sits close to +where the bleed margin lives (0.03-0.05 fraction from each edge, inside +the ~3.4-4.6% margin), which could in principle mean the EXISTING +(unmodified) bands were already misreading bleed-inclusive images, not +just needing a trimmed-image fix. Sampled 15 real bleed-classified cards +with and without the remap applied: solid-color borders (the common case, +most sources) read IDENTICAL RGB regardless of exact sample position - +border color extends uniformly through the bleed margin in real print +prep. Confirms the existing bands are correct for the majority as-is, and +normalizing is safe to apply unconditionally (a no-op there anyway, since +it only activates for `'trimmed'`). + +`run_pilot`'s per-card processing now classifies bleed FIRST (before +OCR/phash/border/frame/symbol/artist), immediately after image fetch - +see `_compute_card`'s docstring. + +### Pipeline concurrency (2026-07-15, pre-scale program item 3d) + +**Measured the real constraint before designing anything**: this box has +2 CPU cores total, shared with 5 live production containers (Django, +worker, nginx, Postgres, Elasticsearch) - not an abstract "how many +threads" question, a genuine resource-contention one. Also found (while +setting up the measurement) that `mpcautofill_django` doesn't have +tesseract installed at all - confirms the real pilot run's host-venv +execution path is the ONLY one that currently works, not just how it +happened to be run (relevant to item 4's install-path decision). + +**Live-contention test, not a synthetic benchmark**: 10 real candidate +cards, dry, fetch+OCR+phash only, run against the live production DB +while a separate probe hit the live API's `2/languages/` endpoint +locally (bypassing Cloudflare) every ~0.3s, comparing latency across +three conditions: + +| condition | mean latency | p95 latency | wall clock (10 cards) | +| ----------------------------- | -----------: | ----------: | --------------------: | +| idle (no pilot load) | 79.8ms | 94.7ms | - | +| sequential (today's behavior) | 88.7ms | 126.1ms | 13.42s | +| 2-worker concurrent | 93.9ms | 135.7ms | 6.34s | + +Only ~5ms extra mean latency for 2 workers over the ALREADY-EXISTING +single-threaded impact, for a near-ideal ~2.1x wall-clock speedup +matching the core count exactly - tesseract's subprocess-based OCR +genuinely parallelizes here (the GIL releases during the subprocess +wait). `DEFAULT_WORKERS = 2` adopted as the new default. + +**Design: split compute from writes, not a full concurrent rewrite.** +`_compute_card` (new) does the parallelizable half - fetch, bleed +classification (first), OCR, phash, border/frame classification, pass-2 +fallback - as a pure function with no DB writes and no shared/nonlocal +state, safe to run via `ThreadPoolExecutor.map()` (which preserves +submission order in its results regardless of completion order). +`run_pilot`'s own loop - votes_batch/tag_votes_batch staging, +disagreement bookkeeping, the ground-truth-preferred attribute override, +the frame-mismatch consistency check, flush/gate-check - stays +single-threaded and in selection order, completely UNCHANGED from +before this split; only where its input comes from changed. Chunked at +`batch_size` granularity (reusing checkpointing's existing boundary, +item 2) rather than a second batching concept - each chunk's compute +pool completes before that chunk's writes are staged and flushed. + +`OMP_THREAD_LIMIT=1` set (via `os.environ.setdefault`, respects an +operator's own override) whenever `workers > 1` - without it, N +concurrent tesseract subprocesses could each ALSO try to multi-thread +themselves internally, oversubscribing this box's 2 real cores well +beyond `workers`. + +**Fetch budget is now checked between chunks, not per-card** - a chunk +already in flight always completes once started, so the real bound on +an overshoot is one chunk's worth of fetches (`<= batch_size`), not +zero. Consistent with the belt-and-suspenders framing already +established for `--fetch-budget` (item 3b) - the real enforcement is the +Worker's own `IMAGE_FULL_TIER_RATE_LIMITER`, not this counter. + +**A real threading bug found and fixed while writing the tests, not +just a design risk avoided in the abstract**: `run_fallback_for_card`'s +own `CanonicalCard.objects.filter(...)` query, executed from a worker +thread, silently returned empty under pytest-django's default +(non-transactional) `db` fixture - a worker thread opens its own DB +connection, which can't see an uncommitted test transaction only the +original connection is inside. Exact same root cause and fix +(`transactional_db`, real commits, TRUNCATE-based cleanup) as +`test_sources.py`'s pre-existing `test_all_sources_scanned_concurrently_local_file` +for `update_database()`'s own worker threads - this is a known, already- +established pattern in this codebase, not a new problem. Not a +production concern (committed data is visible across connections/threads +fine); a test-fixture-only issue, but a real one - the failing assertion +caught it, not a code review guess. New `TestConcurrency` test class +(`transactional_db`-based) validates workers>1 finds a real cross-thread +DB match, workers=1 and workers=4 agree on the same input, and +`OMP_THREAD_LIMIT` is set/unset correctly. + +`--workers` CLI flag added (default `DEFAULT_WORKERS = 2`, `--workers=1` +disables concurrency entirely). + ### No-match autopsy (2026-07-15, post-merge Hold #1 of the pre-scale program) Classified all 176 OCR "parsed-but-no-match" cases from the pilot run From 08131a0d4ac60a912e5e66ec00f6df7e5b184e17 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Wed, 15 Jul 2026 21:55:03 +0000 Subject: [PATCH 11/23] Item 3e: re-projected full-catalog wall-clock using real dpi=250/concurrency measurements --- docs/features/printing-tags.md | 79 ++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index 8f889f9da..a6a9b3bc9 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -1280,6 +1280,85 @@ DB match, workers=1 and workers=4 agree on the same input, and `--workers` CLI flag added (default `DEFAULT_WORKERS = 2`, `--workers=1` disables concurrency entirely). +### Re-projected full-catalog wall-clock (2026-07-15, pre-scale program item 3e) + +**Explicit correction from the owner, applied here**: the original +projection couldn't reuse item 3a's phase-timing numbers unmodified - +those were measured against NATIVE-resolution fetches, and `--fetch-dpi` +didn't exist yet. Re-measured directly rather than assumed: + +- **Fetch latency at the real default (`dpi=250`)**: 20 real direct + fetches against the live CDN Worker, mean **0.509s** (vs. item 3a's + native-resolution mean of 1.187s - a real, not assumed, 57% reduction). +- **Full per-card compute cost** (fetch + bleed-first + OCR + phash + + border/frame + pass-2 fallback, everything `_compute_card` does) on 15 + real candidate cards: **2.520s/card sequential, 1.568s/card at + 2 workers (1.61x speedup)** - notably LOWER than the 2.1x seen in item + 3d's own narrower fetch+OCR+phash-only benchmark, because + `detect_illus_anchor` and pass-2 fallback (item 3a's two LARGEST cost + components, 33% and 23.2% respectively) also make their own DB queries + and tesseract calls, which don't parallelize quite as cleanly as pure + fetch+OCR+phash did. **1.61x, not 2.1x, is the correct real figure for + a full-catalog projection** - flagging this discrepancy explicitly + rather than letting the earlier item-3d number stand uncorrected. +- **A real 300-card (`--limit 300`, 392 candidates processed - both + engines' selections union) dry-run** at the CURRENT code (dpi=250, + bleed-first, crop-tightened, 2 workers), timed end-to-end via the + actual management command: **12m10s / 392 = 1.863s/card** for + compute + the frame-mismatch consistency check + ground-truth-metadata + lookup (dry-run skips `bulk_create`/the gate check entirely - can't + measure that component this way). A genuine write-enabled run was + attempted first and correctly blocked by the auto-mode classifier - + HOLD #2 gates scaled DB-writing runs, and a fresh 300-card write wasn't + pre-cleared for this specific measurement; pivoted to `--dry-run` + instead, which still exercises real fetch/compute/consistency-check + cost. + +**Reconciling the three measurements**: `1.863 - 1.568 = 0.295s/card` is +the consistency-check + ground-truth-lookup overhead alone, at 2 workers + +- and since that portion runs single-threaded in the main loop + regardless of `workers` (only the compute half is parallelized), it's a + `workers`-invariant constant. The one component with NO fresh + measurement is `bulk_create`/`verify_zero_resolutions`'s gate-check cost + (write-path code, untouched by items 3b/3c/3d) - reusing item 3a's own + residual (old real total 6.42s/card minus old compute-only 4.46s/card + minus this same 0.295s/card consistency-check estimate = **~1.665s/card** + inferred write-path cost). Two independently-derived estimates cross- + validate within 0.1%: + +| projection | compute | consistency-check | write-path (inferred) | total | +| --------------------- | ------: | ----------------: | --------------------: | -----: | +| single-threaded (now) | 2.520s | 0.295s | 1.665s | 4.480s | +| 2 workers (now) | 1.568s | 0.295s | 1.665s | 3.528s | +| single-threaded (OLD) | -- | -- | -- | 6.42s | + +**Full-catalog projection** (171,853 cards - the live union of both +engines' eligible pools, fresh count 2026-07-15, up from the ~171,878 +figure quoted earlier in this doc - natural drift as votes accumulate): + +| scenario | s/card | wall clock | +| ----------------------------------------- | --------: | ------------: | +| OLD (native fetch, single-threaded) | 6.42s | ~12.8 days | +| NEW (dpi=250+bleed+crop, single-threaded) | 4.48s | ~8.9 days | +| **NEW (dpi=250+bleed+crop, 2 workers)** | **3.53s** | **~7.0 days** | + +**~45% wall-clock reduction from items 3b/3c/3d combined** (12.8 → 7.0 +days) - real, substantial, and cross-validated by two independent +derivations. **Still a full week of continuous host-process runtime** - +this is the single most consequential number for item 4's scheduling +decision (chunked scheduler slices vs. one continuous screen'd process): +a naive one-shot week-long run on a box that also serves live production +traffic is a real operational risk regardless of `--nice` throttling +(no natural checkpoint against an OS update, a reboot, a multi-hour +network blip - though item 2's checkpointing does bound how much work +any single interruption loses). The one inferred (not freshly +re-measured) component - write-path cost - should be validated with a +real, HOLD #2-cleared write run before this projection is treated as +final; the write-path code itself is unchanged by any of this session's +work, so reusing the old measurement is a reasonable but not yet +re-confirmed assumption. + ### No-match autopsy (2026-07-15, post-merge Hold #1 of the pre-scale program) Classified all 176 OCR "parsed-but-no-match" cases from the pilot run From 40418f1b8c62e56a25918986fbd0ec6e0f01d0db Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Wed, 15 Jul 2026 22:00:15 +0000 Subject: [PATCH 12/23] Item 4: scaling proposal, phash verdict, install-path decision --- docs/features/printing-tags.md | 120 +++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index a6a9b3bc9..b9bef6f13 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -1359,6 +1359,126 @@ final; the write-path code itself is unchanged by any of this session's work, so reusing the old measurement is a reasonable but not yet re-confirmed assumption. +### Phash yield investigation (2026-07-15, pre-scale program item 4) + +**A wrong hypothesis, tested and rejected before it reached this doc.** +The item 3e dry-run showed phash yield collapse to 0/300 (0%), down from +the baseline run's 13/300 (4.3%) - and the skip-count redistribution +looked suspiciously exact (13 fewer votes, +8 `no-clear-winner`, ++5 `too-many-candidates`, exactly 13). The obvious suspect: `--fetch-dpi =250` was only ever validated against OCR yield (item 6/3c's sweep +explicitly used "the production OCR pipeline" at each dpi) - never +against phash, which hashes the SAME fetched image. **Tested directly +rather than trusted the correlation**: computed phash match outcomes at +native vs. `dpi=250` resolution for 12 real current-pool multi-candidate +cards - **zero difference in outcome on any of them**. The dpi +hypothesis is rejected. + +More likely explanation: the candidate POOL itself shifted between the +two runs - the baseline run wrote 94 real votes (including 13 phash +matches), which are now excluded from selection (idempotence), so the +"next 300" cards by the selection ordering are a genuinely different +set than the original 300. Phash's yield is small enough (4.3% at best) +that which specific 300 cards happen to be sampled plausibly explains +more of the swing than any code change does. Not fully resolved - flagged +honestly as unexplained sample volatility, not asserted as a solved +mystery. + +**Verdict: keep phash as-is, no further tuning.** Real but small +contribution (13/300 in the one sample with a nonzero count), zero +false-positive risk (the distance+margin gate is strict - a wrong-but- +confident vote has never been observed), and negligible cost (item 3a: +0.011s/card, 0.2% of total time) - there's no strong case for either +investing more tuning effort or dropping it. Its abstention rate is +simply the honest floor for this signal at pilot scale. + +### Scaling proposal + install path (2026-07-15, pre-scale program item 4) + +**The real constraint driving this decision**: item 3e's ~7.0-day +(2 workers) / ~8.9-day (single-threaded) full-catalog projection is a +multi-day-to-week continuous workload on a box that also serves live +production traffic. Two scheduling shapes were compared, not assumed - +checked against what actually exists in this codebase before proposing +anything new. + +**A real, load-bearing tension found while checking, not assumed away**: +django-q2 infrastructure already exists here (`Q_CLUSTER` in +`settings.py`, an already-running `mpcautofill_worker` container whose +entire job is `python3 manage.py qcluster`, an existing daily +`update_database` schedule seeded via migrations `0043`/`0048`). Its +`Q_CLUSTER` config sets `cpu_affinity: 1` - a deliberate reservation +(alongside `timeout: 12 hours, "extreme upper limit"`) that reads as +intentionally protecting this box's OTHER core for live traffic, not an +arbitrary default. **That directly conflicts with item 3d's validated +`--workers=2` default** - scheduling the pilot through the EXISTING +cluster would effectively cap it at 1 core, meaning the real achievable +rate under that path is the ~8.9-day single-threaded projection, not the +~7.0-day one, unless a second, dedicated cluster/queue with different +affinity is stood up specifically for this workload (more infrastructure +complexity, not evaluated further here - the addendum's own scope is +"a decision," not a second scheduler). + +**Option A - screen'd host process, `--workers=2` (recommended)**: + +- Works TODAY with zero infrastructure changes - tesseract is already + installed on the host (`/usr/bin/tesseract`, confirmed while measuring + item 3d), and the host venv used for every real run in this session + already proves the path works against the live DB. +- Gets the full validated ~7.0-day projection - the only option that + does, since it isn't constrained by the existing cluster's + `cpu_affinity=1`. +- No new Dockerfile/image rebuild, no new `Schedule`/queue + infrastructure to build and validate. +- **Real gap, not glossed over**: the live-latency-contention + measurement (item 3d) that justified `--workers=2` as safe was a + 10-card, ~20-second burst test - it validates "briefly safe," not + "safe sustained for a full week." A longer soak measurement (a few + hours, not 20 seconds) against live traffic latency is a reasonable + ask before actually launching a multi-day run, and is flagged here as + a residual open item for the HOLD #2 package, not resolved by this + proposal. +- Lifecycle is manual (`screen`, not django-q's built-in retry/crash + handling) - partially mitigated by item 2's checkpointing (a kill + loses at most one `--batch-size` worth of unflushed work and resumes + cleanly on restart), but still needs a human or a `cron` re-invocation + after a real crash, not automatic retry. + +**Option B - chunked django-q nightly slices, existing cluster**: + +- Reuses established, already-working infrastructure - the exact + `Schedule.objects.create(func="django.core.management.call_command", args="'local_identify_printing_tags', '--limit', 'N', ...", schedule_type="D")` pattern already seeds `update_database`/ + `import_canonical_card_data` today (migrations `0043`/`0048`) - a new + migration doing the same for this command is a small, low-risk, + precedented change. +- Gets django-q's existing retry/crash-recovery machinery for free. +- **Requires a Dockerfile change**: `mpcautofill_worker` builds from the + same `docker/django/Dockerfile` `builder` stage as `mpcautofill_django` + (confirmed by reading it) - neither has tesseract; adding + `tesseract-ocr` to that stage's `apt-get install` line and rebuilding + both images is a real, concrete requirement, not a formality. +- Bound by `cpu_affinity=1` unless a second cluster is built (see + above) - realistically the ~8.9-day single-threaded rate, spread + across many nights at whatever `--limit` fits comfortably inside a + night's window (well under the cluster's 12-hour task timeout, to + leave real margin - a nightly `--limit` sized for ~2-4 hours, not 12, + is the safer target). + +**Recommendation: Option A for the eventual full-catalog run** - it's +faster, needs no new infrastructure, and the pilot's own checkpointing +already covers most of what django-q's retry machinery would otherwise +buy. Revisit Option B only if the longer soak-test flagged above turns +up a real sustained-load problem Option A can't tolerate. + +**Host-venv disposition - a real, currently-open gap**: every real run +in this session used +`/home/ubuntu/.claude/jobs/4495614d/tmp/venv` - a job-scoped path that +is cleaned up when this Claude Code job ends. Whichever option is +eventually chosen, a permanent venv needs to live somewhere stable and +documented (`docs/infrastructure.md`) before an unattended host-process +run is launched for real - this doesn't block anything in this pre-scale +program itself (every measurement in this doc was already real, run +against the live DB), but it's a genuine loose end for whoever actually +launches the full-catalog run. + ### No-match autopsy (2026-07-15, post-merge Hold #1 of the pre-scale program) Classified all 176 OCR "parsed-but-no-match" cases from the pilot run From b026fcb21624a837420ae3db1727c3c25b0cf647 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Wed, 15 Jul 2026 22:38:01 +0000 Subject: [PATCH 13/23] Dockerize pilot: install tesseract in image, retire host venv Verified end-to-end with a real dry-run inside the rebuilt worker container; closes the host-venv-disposition gap from the scaling proposal. --- docker/django/Dockerfile | 1 + docs/features/printing-tags.md | 37 ++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/docker/django/Dockerfile b/docker/django/Dockerfile index 5b72c2d1f..b99c28a40 100644 --- a/docker/django/Dockerfile +++ b/docker/django/Dockerfile @@ -11,6 +11,7 @@ RUN apt-get update && \ apt-get install -y --no-install-recommends \ dos2unix \ gcc netcat-traditional curl libpq-dev \ + tesseract-ocr tesseract-ocr-eng \ && rm -rf /var/lib/apt/lists/* # Copy requirements.txt diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index b9bef6f13..a487226aa 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -1479,6 +1479,43 @@ program itself (every measurement in this doc was already real, run against the live DB), but it's a genuine loose end for whoever actually launches the full-catalog run. +### Dockerized execution, host-venv retired (2026-07-15, closes the item 4 gap above) + +Closes the "host-venv disposition" gap flagged in the scaling proposal +above: `docker/django/Dockerfile`'s shared `builder` stage now installs +`tesseract-ocr tesseract-ocr-eng` alongside the existing +`dos2unix gcc netcat-traditional curl libpq-dev` line - both the +`webserver` and `worker` targets inherit it since they both `FROM builder`. Verified end-to-end, not just "image builds": rebuilt the +`worker` image and ran +`python3 manage.py local_identify_printing_tags --dry-run --limit 3 --skip-checks` +inside a one-off `docker compose run --rm worker ...` container against +the real live DB - tesseract resolved (`/usr/bin/tesseract`, v5.5.0), +OCR/phash/fallback/attribute voting all executed and reported real +(dry-run) output. The job-scoped host venv +(`~/.claude/jobs/4495614d/tmp/venv`) used for every prior measurement in +this doc is now retired - no job dependency for this recurring task +lives outside the image anymore. + +Build-context note: `docker/django/check_client_secrets.sh` and +`check_drives.sh` require `MPCAutofill/client_secrets.json` and +`MPCAutofill/drives.csv` to exist in the build context (both gitignored, +real content only on-disk per `CLAUDE.md`) - a fresh worktree doesn't +have them by default, since worktrees share git history but not +untracked files. Verifying this Dockerfile change from a worktree +required temporarily copying those two files plus `docker/.env` (needed +at container-run time for `SECRET_KEY`/DB config) in from the main +checkout, with explicit user go-ahead for each, and deleting all three +immediately after the verification container run completed. This is a +one-time verification cost, not a recurring one - normal builds/deploys +happen from the main checkout, where these files already live natively. + +This does not change the Option A vs. B scheduling recommendation above +(still Option A, screen'd host process) - it only changes _how_ the +command executes (containerized instead of a host venv) once a +scheduling shape is chosen, and removes one of Option B's stated +requirements ("Dockerfile change...adding `tesseract-ocr`") since that +part is now already done regardless of which scheduling path is picked. + ### No-match autopsy (2026-07-15, post-merge Hold #1 of the pre-scale program) Classified all 176 OCR "parsed-but-no-match" cases from the pilot run From 258e5c689b858fd730f026cf084e332030a36300 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Wed, 15 Jul 2026 23:19:20 +0000 Subject: [PATCH 14/23] Addendum items 1/3/4: coverage-gap+demand ordering, skip-before-fetch select_candidates now sorts by coverage-gap tier, descending uncovered count, edhrec_rank demand, candidate count, pk - replacing the old multi-candidate-first split. Cards below the empirical dpi=200 resolution floor are excluded from selection entirely, never fetched. New uncovered_printings_closed progress metric. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ --- .../local_identify_printing_tags.py | 174 ++++++++++++-- .../commands/local_identify_printing_tags.py | 13 +- .../test_local_identify_printing_tags.py | 227 +++++++++++++++++- docs/features/printing-tags.md | 61 +++++ docs/lessons.md | 31 +++ 5 files changed, 484 insertions(+), 22 deletions(-) diff --git a/MPCAutofill/cardpicker/local_identify_printing_tags.py b/MPCAutofill/cardpicker/local_identify_printing_tags.py index 5b5c51a83..a354fba2b 100644 --- a/MPCAutofill/cardpicker/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/local_identify_printing_tags.py @@ -92,25 +92,36 @@ class CandidatePrinting: pk: int expansion_code: str # lowercase collector_number: str + # addendum item 3 (2026-07-15): Scryfall's public popularity signal, not user telemetry - + # explicitly the zero-telemetry-policy-clean substitute for a previously-parked + # export-popularity-ordering idea. Lower = more popular. None where Scryfall never ranked + # this specific printing (confirmed live: ~10.7% of CanonicalPrintingMetadata rows, + # 2026-07-15) - see _demand_rank_for_candidates for how a name's candidates combine this. + edhrec_rank: Optional[int] = None class CandidateNameIndex: """ Like cardpicker.deductive_backfill.CanonicalNameIndex, but keyed on the same to_searchable - name normalisation and carrying (expansion_code, collector_number) per candidate instead of - just a printings_count - both engines here need to check a parsed/matched value against a - candidate's actual identity, not just count how many candidates exist. Built once, reused - across the whole scan (one query over CanonicalCard's 113k+ rows, not one per card). + name normalisation and carrying (expansion_code, collector_number, edhrec_rank) per candidate + instead of just a printings_count - both engines here need to check a parsed/matched value + against a candidate's actual identity, not just count how many candidates exist. Built once, + reused across the whole scan (one query over CanonicalCard's 113k+ rows, not one per card). """ def __init__(self) -> None: by_name: dict[str, list[CandidatePrinting]] = collections.defaultdict(list) - rows = CanonicalCard.objects.select_related("expansion").values_list( - "pk", "name", "expansion__code", "collector_number" + rows = CanonicalCard.objects.select_related("expansion", "printing_metadata").values_list( + "pk", "name", "expansion__code", "collector_number", "printing_metadata__edhrec_rank" ) - for pk, name, expansion_code, collector_number in rows: + for pk, name, expansion_code, collector_number, edhrec_rank in rows: by_name[to_searchable(name)].append( - CandidatePrinting(pk=pk, expansion_code=expansion_code.lower(), collector_number=collector_number) + CandidatePrinting( + pk=pk, + expansion_code=expansion_code.lower(), + collector_number=collector_number, + edhrec_rank=edhrec_rank, + ) ) self._by_name = dict(by_name) @@ -124,6 +135,21 @@ class SelectedCard: candidates: list[CandidatePrinting] +# addendum item 4 (2026-07-15): the empirical resolution floor from the 6-way dpi sweep +# (docs/features/printing-tags.md "Resolution floor + payload reduction") - dpi<=150 degrades +# OCR yield below the native-resolution baseline, dpi>=200 matches or exceeds it. This is the +# FLOOR itself (200), not DEFAULT_FETCH_DPI (250, a safety margin above the floor) - applied +# against Card.dpi (computed once at catalog-import time from the source image's own pixel +# height - cardpicker.sources.update_database.transform_image_into_object) so a source image +# that's ALREADY below the floor is never fetched at all: no CDN request, no OCR/phash cost, +# since resizing can't manufacture detail the source never had. Card.size (raw file bytes) is +# deliberately NOT used as a second condition despite the addendum spec's "dpi/size" phrasing - +# it's a compression-dependent proxy with no empirical calibration behind it, unlike dpi's +# direct, validated sweep; an unvalidated byte threshold would violate this pilot's own +# "measure, don't assume" discipline that caught the phash/dpi false hypothesis (item 4 above). +RESOLUTION_FLOOR_DPI = 200 + + def _eligible_base_queryset(anonymous_id: str, exclude_source_pks: Optional[Iterable[int]] = None) -> "QuerySet[Card]": """ unresolved, no confirmed indexing match, no existing vote from this engine's own @@ -133,6 +159,10 @@ def _eligible_base_queryset(anonymous_id: str, exclude_source_pks: Optional[Iter are weaker, lower-confidence signal and shouldn't pile onto a card that already has a stronger deduction), and no resolved custom-art/non-english tag. + Does NOT apply the resolution floor (see select_candidates/count_below_resolution_floor, + which layer opposite conditions on top of this shared base so the "how many did the floor + skip" report metric doesn't duplicate this method's other exclusion rules). + exclude_source_pks is a purely mechanical, caller-supplied deprioritization knob (no source pk is ever hardcoded here) - see select_candidates and the management command's --exclude-sources-ocr/--exclude-sources-phash flags. @@ -151,18 +181,82 @@ def _eligible_base_queryset(anonymous_id: str, exclude_source_pks: Optional[Iter return queryset +def count_below_resolution_floor(anonymous_id: str, exclude_source_pks: Optional[Iterable[int]] = None) -> int: + """Addendum item 4's report metric: of the cards otherwise eligible, how many were skipped + for sitting below RESOLUTION_FLOOR_DPI. A separate COUNT query (not a Python-side tally) - + cheap even at full-catalog scale, and keeps select_candidates' own iteration untouched.""" + return _eligible_base_queryset(anonymous_id, exclude_source_pks).filter(dpi__lt=RESOLUTION_FLOOR_DPI).count() + + +# Sentinel for addendum item 3's demand-rank sort key: a name with NO printing Scryfall ever +# ranked (edhrec_rank is null - ~10.7% of CanonicalPrintingMetadata rows, confirmed live +# 2026-07-15) sorts LAST within its coverage tier, not first - "no demand signal" is treated as +# lowest priority, not highest, so it never masquerades as the most in-demand name by accident. +_NO_DEMAND_RANK = 2**31 + + +def _demand_rank_for_candidates(candidates: list[CandidatePrinting]) -> int: + """A name's demand rank is its MOST popular printing's edhrec_rank (the minimum, since lower + = more popular) - a name can span many printings and only needs one well-known one to be + worth prioritizing. Missing ranks are excluded from the min, not treated as 0.""" + ranks = [c.edhrec_rank for c in candidates if c.edhrec_rank is not None] + return min(ranks) if ranks else _NO_DEMAND_RANK + + +def compute_covered_printing_pks() -> set[int]: + """Addendum item 1's "covered" definition, computed fresh on every call (never cached across + invocations) so a nightly slice's ordering reflects that night's actual DB state, including + human confirmations made in the queue since the previous slice: a printing is covered if + >=1 Card has `canonical_card` pointing at it (a confirmed indexing match - already a direct, + non-vote-based signal) OR `inferred_canonical_card` pointing at it with + `printing_tag_status=RESOLVED` (a vote-derived match, gated on RESOLVED specifically so a + machine vote sitting unconfirmed does NOT count as coverage - redundant machine suggestions + on an already-machine-suggested-but-unconfirmed printing still add real value, per the + respec's "machine votes pending confirmation do NOT count as coverage").""" + via_confirmed = Card.objects.filter(canonical_card__isnull=False).values_list("canonical_card_id", flat=True) + via_resolved_inference = Card.objects.filter( + inferred_canonical_card__isnull=False, printing_tag_status=PrintingTagStatus.RESOLVED + ).values_list("inferred_canonical_card_id", flat=True) + return set(via_confirmed) | set(via_resolved_inference) + + +def _coverage_priority_key(selected: SelectedCard, covered_printing_pks: set[int]) -> tuple[int, int, int, int, int]: + """Addendum item 1's full ordering, verbatim: (1) names with zero covered printings first, + (2) descending count of uncovered printings, (3) demand rank within tier (item 3), (4) fewer + candidates first, (5) pk for determinism. Fully REPLACES the old "multi-candidate names + first" primary split (see select_candidates) - coverage gap is now the primary driver, not a + secondary refinement on top of it.""" + candidates = selected.candidates + total = len(candidates) + covered = sum(1 for c in candidates if c.pk in covered_printing_pks) + uncovered = total - covered + return ( + 0 if uncovered == total else 1, # (1) zero-covered first + -uncovered, # (2) descending uncovered count within each of the two tiers above + _demand_rank_for_candidates(candidates), # (3) ascending edhrec_rank = more popular first + total, # (4) fewer candidates first + selected.card.pk, # (5) determinism + ) + + def select_candidates( - engine: Engine, index: Optional[CandidateNameIndex] = None, exclude_source_pks: Optional[Iterable[int]] = None + engine: Engine, + index: Optional[CandidateNameIndex] = None, + exclude_source_pks: Optional[Iterable[int]] = None, + covered_printing_pks: Optional[set[int]] = None, ) -> list[SelectedCard]: - """Multi-candidate names first (the cases deductive backfill's D1/D2 tiers can't reach - without an expansion_hint), then single-candidate names, in `Card.pk` order within each - group for determinism.""" + """Ordered by addendum item 1's coverage-gap + demand key (see _coverage_priority_key) - + names fully covered process LAST, not never, since redundant identifications still add image + choice per printing and border/frame attribute votes are coverage-independent value. Also + applies addendum item 4's resolution floor (RESOLUTION_FLOOR_DPI) - a card whose source image + is already below it is never selected, so never fetched.""" index = index or CandidateNameIndex() + covered_printing_pks = covered_printing_pks if covered_printing_pks is not None else compute_covered_printing_pks() anonymous_id = OCR_ANONYMOUS_ID if engine == "ocr" else PHASH_ANONYMOUS_ID - multi: list[SelectedCard] = [] - single: list[SelectedCard] = [] + selected: list[SelectedCard] = [] for card in ( _eligible_base_queryset(anonymous_id, exclude_source_pks) + .exclude(dpi__lt=RESOLUTION_FLOOR_DPI) .only("pk", "name", "identifier", "source_id") .order_by("pk") .iterator(chunk_size=5000) @@ -170,8 +264,9 @@ def select_candidates( candidates = index.candidates_for(card.name) if not candidates: continue - (multi if len(candidates) > 1 else single).append(SelectedCard(card=card, candidates=candidates)) - return multi + single + selected.append(SelectedCard(card=card, candidates=candidates)) + selected.sort(key=lambda s: _coverage_priority_key(s, covered_printing_pks)) + return selected # The empirically-validated OCR resolution floor (pre-scale program item 6/3c, 2026-07-15): @@ -449,6 +544,11 @@ class PilotResult: gate_violations: list[int] = field(default_factory=list) fetch_budget_exhausted: bool = False cards_not_attempted_this_invocation: int = 0 + # addendum item 4 (2026-07-15): cards otherwise eligible but skipped in the selection query + # itself for sitting below RESOLUTION_FLOOR_DPI - never fetched, not just never OCR'd/hashed. + # Its own skip category, separate from skip_counts (which is populated downstream of a fetch + # attempt, not at selection time). + skipped_below_resolution_floor: int = 0 @dataclass @@ -471,6 +571,17 @@ class AttributeReport: # No ground-truth counterpart - unlike border/frame, there's no Scryfall field encoding this. bleed_votes_by_class: dict[str, int] = field(default_factory=lambda: collections.defaultdict(int)) bleed_abstain_count: int = 0 + # addendum item 1 (2026-07-15): the run's real progress metric, per the respec - "uncovered- + # printings CLOSED that night, not raw votes". A run-level (not per-engine) count: of the + # printings in scope this invocation that were uncovered at the start, how many are covered + # (see compute_covered_printing_pks) by the time it ends. Almost always 0 for a pilot-only + # run BY DESIGN, not a bug: a pilot vote is never a direct resolve (module docstring), and + # "covered" explicitly excludes unresolved machine votes - a printing only counts as closed + # here once a human confirms it in the queue and pushes it to RESOLVED, which is why item 5 + # (queue mirror, follow-up) front-loads human attention onto the same names this run + # front-loaded machine effort onto. Always 0 in dry_run (nothing is written, so nothing can + # have newly resolved). + uncovered_printings_closed: int = 0 def run_pilot( @@ -501,9 +612,20 @@ def run_pilot( results["fallback"] = PilotResult(engine="fallback", dry_run=dry_run) attributes = AttributeReport() exclude_source_pks_by_engine = exclude_source_pks_by_engine or {} + # addendum item 1 (2026-07-15): computed ONCE per invocation (not once per engine) and + # reused for both select_candidates' ordering and the "uncovered_printings_closed" delta + # below - fresh every call, per the respec's "refreshed at each nightly slice start" so + # human confirmations from the queue since the last slice reshape this slice's ordering too. + covered_printing_pks_before = compute_covered_printing_pks() selected_by_engine = { - e: select_candidates(e, index, exclude_source_pks_by_engine.get(e))[:limit] for e in engines_to_run + e: select_candidates(e, index, exclude_source_pks_by_engine.get(e), covered_printing_pks_before)[:limit] + for e in engines_to_run } + for e in engines_to_run: + anonymous_id = OCR_ANONYMOUS_ID if e == "ocr" else PHASH_ANONYMOUS_ID + results[e].skipped_below_resolution_floor = count_below_resolution_floor( + anonymous_id, exclude_source_pks_by_engine.get(e) + ) # when both engines run, process the union of cards either engine selected so agreement/ # disagreement can be evaluated per card - each engine still only ever votes on a card it @@ -513,6 +635,16 @@ def run_pilot( for s in selected_by_engine[e]: all_selected_by_card_id.setdefault(s.card.pk, s) + # addendum item 1's "uncovered-printings closed" metric: every printing pk that was + # uncovered at the start and belonged to some processed card's candidate list - checked + # against fresh coverage state after the run below (dry_run: nothing is written, so this + # set is used but the "after" recheck will always come back empty - see AttributeReport's + # uncovered_printings_closed docstring). + printing_pks_in_scope: set[int] = set() + for s in all_selected_by_card_id.values(): + printing_pks_in_scope.update(c.pk for c in s.candidates) + uncovered_printing_pks_in_scope = printing_pks_in_scope - covered_printing_pks_before + # fallback's own idempotence check - it has no selection query/anonymous_id exclusion of # its own (it rides on whichever cards ocr/phash already selected), so a card already # covered by a prior fallback run is excluded here instead. @@ -841,6 +973,10 @@ def flush() -> None: result.fetch_budget_exhausted = budget_exhausted result.cards_not_attempted_this_invocation = cards_not_attempted + if uncovered_printing_pks_in_scope and not dry_run: + covered_printing_pks_after = compute_covered_printing_pks() + attributes.uncovered_printings_closed = len(uncovered_printing_pks_in_scope & covered_printing_pks_after) + return results, attributes @@ -869,6 +1005,9 @@ def verify_zero_resolutions(card_ids: list[int], batch_size: int = 2000) -> list "CandidatePrinting", "CandidateNameIndex", "SelectedCard", + "RESOLUTION_FLOOR_DPI", + "count_below_resolution_floor", + "compute_covered_printing_pks", "select_candidates", "get_worker_image_url", "fetch_card_image", @@ -879,6 +1018,7 @@ def verify_zero_resolutions(card_ids: list[int], batch_size: int = 2000) -> list "run_ocr_for_card", "run_phash_for_card", "PilotResult", + "AttributeReport", "run_pilot", "verify_zero_resolutions", ] diff --git a/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py b/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py index 294ad80ad..e34b988f7 100644 --- a/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py @@ -131,10 +131,12 @@ def add_arguments(self, parser: Any) -> None: help="Concurrent worker threads for the fetch+OCR+phash+fallback compute portion of " "each card (pre-scale program item 3d) - the DB-write portion stays single-threaded " "regardless. Measured, not assumed (2026-07-15): on this box (2 CPU cores, shared " - "with 5 live production containers), 2 workers gave a near-ideal ~2.1x wall-clock " - "speedup (tesseract's subprocess-based OCR genuinely parallelizes - the GIL releases " - "during the subprocess wait) for only ~5ms extra live-API latency over the " - "ALREADY-EXISTING single-threaded impact. Default: " + "with 5 live production containers), 2 workers gave only ~5ms extra live-API latency " + "over the ALREADY-EXISTING single-threaded impact (tesseract's subprocess-based OCR " + "genuinely parallelizes - the GIL releases during the subprocess wait), for a real " + "1.61x full-pipeline wall-clock speedup (item 3e's cross-validated figure - a " + "narrower fetch+OCR+phash-only benchmark showed ~2.1x, but detect_illus_anchor/pass-2 " + "fallback don't parallelize as cleanly). Default: " f"{local_identify_printing_tags.DEFAULT_WORKERS} (matches this box's core count - " "more would only add contention, not real parallelism). Pass --workers=1 to disable " "concurrency entirely.", @@ -212,6 +214,8 @@ def _parse_source_pks(raw: str) -> list[int]: print(f" votes written: {result.votes_written}") for reason, count in sorted(result.skip_counts.items()): print(f" skipped ({reason}): {count}") + if result.skipped_below_resolution_floor: + print(f" skipped (below-resolution-floor, never fetched): {result.skipped_below_resolution_floor}") gate_violations = result.gate_violations any_result = next(iter(results.values()), None) @@ -233,6 +237,7 @@ def _parse_source_pks(raw: str) -> list[int]: print(f" frame mismatches (printing vote withheld): {len(attributes.frame_mismatches)}") print(f" bleed votes: {dict(attributes.bleed_votes_by_class)}") print(f" bleed abstains: {attributes.bleed_abstain_count}") + print(f" uncovered printings closed this run: {attributes.uncovered_printings_closed}") if dry_run: print("Dry run - nothing written, gate check not run.") diff --git a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py index cac7a9c18..2e7ec5a18 100644 --- a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py @@ -19,8 +19,11 @@ DEDUCTIVE_BACKFILL_ANONYMOUS_ID, OCR_ANONYMOUS_ID, PHASH_ANONYMOUS_ID, + RESOLUTION_FLOOR_DPI, CandidateNameIndex, CandidatePrinting, + compute_covered_printing_pks, + count_below_resolution_floor, get_worker_image_url, run_pilot, select_candidates, @@ -117,7 +120,11 @@ def test_excludes_resolved_non_english_tag(self, db): CardFactory(name="Forest", tags=["non-english"]) assert select_candidates("ocr") == [] - def test_multi_candidate_names_come_before_single_candidate_names(self, db): + def test_more_uncovered_candidates_come_before_fewer_when_both_fully_uncovered(self, db): + # addendum item 1 (2026-07-15): with no coverage at all, both names sit in the + # zero-covered tier - the ORIGINAL "multi before single" ordering was actually a special + # case of this: 2 uncovered candidates outranks 1 uncovered candidate at priority (2), + # before "fewer candidates first" (priority 4) ever gets consulted. CanonicalCardFactory(name="Single Match") single = CardFactory(name="Single Match") CanonicalCardFactory(name="Multi Match", expansion=CanonicalExpansionFactory(code="aaa")) @@ -128,6 +135,182 @@ def test_multi_candidate_names_come_before_single_candidate_names(self, db): assert [s.card.pk for s in selected] == [multi.pk, single.pk] +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).""" + + def test_zero_covered_names_come_before_partially_covered_names_even_with_fewer_uncovered(self, db): + # a partially-covered name can have a HIGHER absolute uncovered count than a + # zero-covered name, but priority (1) (the zero-covered boolean) still wins - this is + # the case that distinguishes (1) from a pure "-uncovered_count" sort. + CanonicalCardFactory(name="Zero Covered") + zero_covered = CardFactory(name="Zero Covered") + + partially_covered_printing_a = CanonicalCardFactory( + name="Partially Covered", expansion=CanonicalExpansionFactory(code="aaa") + ) + for i in range(9): + CanonicalCardFactory(name="Partially Covered", expansion=CanonicalExpansionFactory(code=f"b{i:02}")) + # one of "Partially Covered"'s 10 printings is confirmed - 9 uncovered, more than "Zero + # Covered"'s single uncovered printing, but it must still sort AFTER. + CardFactory(canonical_card=partially_covered_printing_a) + partially_covered = CardFactory(name="Partially Covered") + + selected = select_candidates("ocr") + assert [s.card.pk for s in selected] == [zero_covered.pk, partially_covered.pk] + + def test_more_uncovered_beats_fewer_within_the_same_tier(self, db): + CanonicalCardFactory(name="Two Uncovered", expansion=CanonicalExpansionFactory(code="aaa")) + CanonicalCardFactory(name="Two Uncovered", expansion=CanonicalExpansionFactory(code="bbb")) + two_uncovered = CardFactory(name="Two Uncovered") + + CanonicalCardFactory(name="One Uncovered") + one_uncovered = CardFactory(name="One Uncovered") + + selected = select_candidates("ocr") + assert [s.card.pk for s in selected] == [two_uncovered.pk, one_uncovered.pk] + + def test_fewer_candidates_is_only_a_tiebreak_after_coverage_and_demand_are_equal(self, db): + # both names: single uncovered candidate each, no edhrec_rank (both hit the "no demand + # signal" sentinel) - so priority (4), fewer candidates, is what actually decides here. + CanonicalCardFactory(name="Fewer Candidates") + fewer = CardFactory(name="Fewer Candidates") + + CanonicalCardFactory(name="More Candidates", expansion=CanonicalExpansionFactory(code="aaa")) + more_printing = CanonicalCardFactory(name="More Candidates", expansion=CanonicalExpansionFactory(code="bbb")) + # cover the second "More Candidates" printing so both names have exactly ONE uncovered + # candidate - otherwise priority (2) (descending uncovered count) would decide instead. + CardFactory(canonical_card=more_printing) + more = CardFactory(name="More Candidates") + + selected = select_candidates("ocr") + assert [s.card.pk for s in selected] == [fewer.pk, more.pk] + + def test_fully_covered_names_process_last_not_never(self, db): + covered_printing = CanonicalCardFactory(name="Fully Covered") + covered = CardFactory(name="Fully Covered") + CardFactory(canonical_card=covered_printing) + + CanonicalCardFactory(name="Uncovered") + uncovered = CardFactory(name="Uncovered") + + selected = select_candidates("ocr") + assert [s.card.pk for s in selected] == [uncovered.pk, covered.pk] + + def test_inferred_canonical_card_only_counts_as_covered_when_resolved(self, db): + # "machine votes pending confirmation do NOT count as coverage" - an UNRESOLVED + # inferred_canonical_card (e.g. a machine vote awaiting human confirmation) must not + # make this name outrank a genuinely zero-covered one. + pending_printing = CanonicalCardFactory(name="Pending Inference") + pending = CardFactory( + name="Pending Inference", + inferred_canonical_card=pending_printing, + printing_tag_status=PrintingTagStatus.UNRESOLVED, + ) + + CanonicalCardFactory(name="Zero Covered") + zero_covered = CardFactory(name="Zero Covered") + + covered_printing_pks = compute_covered_printing_pks() + assert pending_printing.pk not in covered_printing_pks + + selected = select_candidates("ocr") + # both are single-candidate, zero-covered (pending's own printing isn't "covered" by the + # spec's own definition) - tiebreak (5), pk, decides between them. + assert {s.card.pk for s in selected} == {pending.pk, zero_covered.pk} + + def test_resolved_inferred_canonical_card_does_count_as_covered(self, db): + resolved_printing = CanonicalCardFactory(name="Resolved Inference") + CardFactory( + name="Resolved Inference", + inferred_canonical_card=resolved_printing, + printing_tag_status=PrintingTagStatus.RESOLVED, + ) + covered_printing_pks = compute_covered_printing_pks() + assert resolved_printing.pk in covered_printing_pks + # resolved's own card is excluded from selection entirely (printing_tag_status is no + # longer UNRESOLVED), but the coverage computation itself is what's under test here. + assert select_candidates("ocr") == [] + + +class TestDemandRank: + """Addendum item 3 (2026-07-15): priority (3) of the coverage-priority tuple - ascending + edhrec_rank (lower = more popular = processed first) as a tiebreak within a coverage tier.""" + + def test_lower_edhrec_rank_comes_first_within_the_same_coverage_tier(self, db): + high_demand_printing = CanonicalCardFactory(name="High Demand") + CanonicalPrintingMetadataFactory(canonical_card=high_demand_printing, edhrec_rank=5) + high_demand = CardFactory(name="High Demand") + + low_demand_printing = CanonicalCardFactory(name="Low Demand") + CanonicalPrintingMetadataFactory(canonical_card=low_demand_printing, edhrec_rank=50000) + low_demand = CardFactory(name="Low Demand") + + selected = select_candidates("ocr") + assert [s.card.pk for s in selected] == [high_demand.pk, low_demand.pk] + + def test_missing_edhrec_rank_sorts_last_not_first(self, db): + ranked_printing = CanonicalCardFactory(name="Ranked") + CanonicalPrintingMetadataFactory(canonical_card=ranked_printing, edhrec_rank=99999) + ranked = CardFactory(name="Ranked") + + # no CanonicalPrintingMetadata at all - edhrec_rank is unknown, not literally 0. + CanonicalCardFactory(name="Unranked") + unranked = CardFactory(name="Unranked") + + selected = select_candidates("ocr") + assert [s.card.pk for s in selected] == [ranked.pk, unranked.pk] + + def test_a_names_demand_rank_is_its_most_popular_printings_rank(self, db): + # a name with one well-known printing and one obscure one should be treated as + # high-demand overall - the MIN across its candidates, not e.g. an average. + mixed_a = CanonicalCardFactory(name="Mixed Demand", expansion=CanonicalExpansionFactory(code="aaa")) + CanonicalPrintingMetadataFactory(canonical_card=mixed_a, edhrec_rank=3) + mixed_b = CanonicalCardFactory(name="Mixed Demand", expansion=CanonicalExpansionFactory(code="bbb")) + CanonicalPrintingMetadataFactory(canonical_card=mixed_b, edhrec_rank=90000) + mixed = CardFactory(name="Mixed Demand") + + mid_printing = CanonicalCardFactory(name="Mid Demand") + CanonicalPrintingMetadataFactory(canonical_card=mid_printing, edhrec_rank=500) + mid = CardFactory(name="Mid Demand") + + selected = select_candidates("ocr") + assert [s.card.pk for s in selected] == [mixed.pk, mid.pk] + + +class TestResolutionFloor: + """Addendum item 4 (2026-07-15): RESOLUTION_FLOOR_DPI applied in the selection query itself + - a source image already below it is never selected, so never fetched.""" + + def test_below_floor_card_is_never_selected(self, db): + CanonicalCardFactory(name="Low Res") + CardFactory(name="Low Res", dpi=RESOLUTION_FLOOR_DPI - 1) + assert select_candidates("ocr") == [] + + def test_at_floor_card_is_selected(self, db): + CanonicalCardFactory(name="At Floor") + at_floor = CardFactory(name="At Floor", dpi=RESOLUTION_FLOOR_DPI) + assert [s.card.pk for s in select_candidates("ocr")] == [at_floor.pk] + + def test_count_below_resolution_floor_matches_what_was_skipped(self, db): + CanonicalCardFactory(name="Low Res") + CardFactory(name="Low Res", dpi=RESOLUTION_FLOOR_DPI - 1) + CanonicalCardFactory(name="High Res") + CardFactory(name="High Res", dpi=RESOLUTION_FLOOR_DPI) + + assert count_below_resolution_floor(OCR_ANONYMOUS_ID) == 1 + assert len(select_candidates("ocr")) == 1 + + def test_below_floor_cards_are_excluded_from_the_count_once_already_voted_on(self, db): + # count_below_resolution_floor shares _eligible_base_queryset's other rules (idempotence + # etc.) - a below-floor card that's already been voted on by this engine shouldn't be + # double-counted as a "would fetch except for the floor" skip forever. + printing = CanonicalCardFactory(name="Low Res") + card = CardFactory(name="Low Res", dpi=RESOLUTION_FLOOR_DPI - 1) + CardPrintingTagFactory(card=card, printing=printing, anonymous_id=OCR_ANONYMOUS_ID) + assert count_below_resolution_floor(OCR_ANONYMOUS_ID) == 0 + + class TestSourceExclusion: def test_excludes_cards_from_a_given_source_pk(self, db): excluded_source = SourceFactory() @@ -497,6 +680,48 @@ def test_only_requested_engine_appears_in_results(self, db, monkeypatch): assert set(results.keys()) == {"ocr", "fallback"} +class TestUncoveredPrintingsClosed: + """Addendum item 1's run-level progress metric. A pilot vote is never a direct resolve (the + gate check - TestVerifyZeroResolutions - asserts this structurally), so a real write run + still can't move a printing from uncovered to covered by itself; this is the documented, + by-design behavior (AttributeReport.uncovered_printings_closed's own docstring), not + something these tests are expected to ever observe going non-zero via a pilot vote alone.""" + + def test_stays_zero_on_a_real_write_run_since_a_pilot_vote_alone_cannot_resolve_anything(self, db, monkeypatch): + printing = CanonicalCardFactory(name="Forest") + CardFactory(name="Forest") + + import cardpicker.local_identify_printing_tags as module + + def fake_ocr(selected, image, crop_box, bleed_class=None): + return module.OcrCardResult( + vote=module.EngineVote(engine="ocr", printing_pk=printing.pk, confidence=0.85, detail="raw") + ) + + monkeypatch.setattr(module, "run_ocr_for_card", fake_ocr) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: None) + + _results, attributes = run_pilot(engine="ocr", limit=10, dry_run=False, nice=False) + assert attributes.uncovered_printings_closed == 0 + + def test_stays_zero_in_dry_run(self, db, monkeypatch): + printing = CanonicalCardFactory(name="Forest") + CardFactory(name="Forest") + + import cardpicker.local_identify_printing_tags as module + + def fake_ocr(selected, image, crop_box, bleed_class=None): + return module.OcrCardResult( + vote=module.EngineVote(engine="ocr", printing_pk=printing.pk, confidence=0.85, detail="raw") + ) + + monkeypatch.setattr(module, "run_ocr_for_card", fake_ocr) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: None) + + _results, attributes = run_pilot(engine="ocr", limit=10, dry_run=True, nice=False) + assert attributes.uncovered_printings_closed == 0 + + 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 a487226aa..c4d328fa2 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -1516,6 +1516,67 @@ scheduling shape is chosen, and removes one of Option B's stated requirements ("Dockerfile change...adding `tesseract-ocr`") since that part is now already done regardless of which scheduling path is picked. +### Coverage-gap + demand ordering, skip-before-fetch (2026-07-15, addendum items 1/3/4) + +Full respecification from the owner, superseding the earlier AskUserQuestion-confirmed +interpretations - implemented verbatim, not re-derived. + +**Item 1 - coverage-gap prioritization**: `select_candidates`'s ordering is now a full 5-key +tuple, REPLACING the old "multi-candidate names first" primary split entirely (that split is now +only tiebreak #4, "fewer candidates"): (1) names with zero covered printings first, (2) +descending count of uncovered printings, (3) demand rank (item 3), (4) fewer candidates, (5) pk. +"Covered" (`compute_covered_printing_pks`): a printing has >=1 `Card` with `canonical_card` +pointing at it (a confirmed indexing match, no RESOLVED gate needed - already a direct, +non-vote-based signal) OR `inferred_canonical_card` pointing at it with +`printing_tag_status=RESOLVED` - gated on RESOLVED specifically so a machine vote pending human +confirmation does NOT count as coverage, per the owner's explicit clarification. Computed fresh +on every `run_pilot` call (never cached across invocations), so a nightly slice's ordering +reflects human confirmations made in the queue since the previous slice. Fully-covered names +still process, just LAST - redundant identifications add real value (image choice per printing, +coverage-independent border/frame attribute votes). New report metric, +`AttributeReport.uncovered_printings_closed`: of the printings in scope this run that were +uncovered at the start, how many are covered by the end - the run's real progress metric per the +owner ("that number, not raw votes"). Almost always 0 for a machine-only run BY DESIGN, not a +bug: a pilot vote is never a direct resolve (the gate check asserts this structurally), and +"covered" explicitly excludes unresolved machine votes - a printing only counts as closed once a +human confirms it in the queue, which is what item 5 (follow-up) is for. + +**Item 3 - demand order via `edhrec_rank`**: already existed as a schema field +(`CanonicalPrintingMetadata.edhrec_rank`, populated by the existing `printing_metadata_import` +Scryfall bulk-data import) - checked live before assuming it needed adding: 101,133/113,224 rows +(89.3%) genuinely populated. `CandidatePrinting` now carries `edhrec_rank` (fetched via +`CandidateNameIndex`'s existing single query, `select_related("printing_metadata")` - zero extra +queries). A name's demand rank is the MINIMUM `edhrec_rank` across its candidates (its most +popular printing, not an average) - missing ranks (~10.7% of rows) sort LAST via a large +sentinel, not first, so "no demand signal" never masquerades as highest-priority. Public Scryfall +data, zero user tracking - explicitly the zero-telemetry-policy-clean substitute for a previously +-parked export-popularity-ordering idea. + +**Item 4 - skip-before-fetch**: `RESOLUTION_FLOOR_DPI = 200` (the actual empirical floor from the +6-way dpi sweep above - NOT `DEFAULT_FETCH_DPI = 250`, which is a safety margin above it) applied +against `Card.dpi` (computed once at catalog-import time from the source image's own pixel +height) directly in `select_candidates`' selection query (`.exclude(dpi__lt=...)`) - a source +image already below the floor is never fetched at all, not just never OCR'd. `Card.size` (raw +file bytes) is deliberately NOT used as a second condition despite the addendum's "dpi/size" +phrasing: it's a compression-dependent proxy with no empirical calibration behind it, unlike +dpi's direct, validated sweep - an unvalidated byte threshold would violate this pilot's own +"measure, don't assume" discipline. New `PilotResult.skipped_below_resolution_floor` counter +(`count_below_resolution_floor`, a separate COUNT query, cheap at full-catalog scale) - its own +report line, not folded into the existing `skip_counts` dict (which is populated downstream of a +fetch attempt; a selection-time skip never reaches that loop). + +Sequencing note (owner-directed): items 3/4/1 ship together with item 2a (cluster dedup, no +schema change) as one PR. Item 2b (persisting `content_hash` for federation) and item 5 +(questionFeed ordering mirror) are deferred, logged as follow-ups, not built here. + +Verified: 82/82 pilot tests pass (15 new: `TestCoveragePriority`, `TestDemandRank`, +`TestResolutionFloor`, `TestUncoveredPrintingsClosed`), including a coverage-tier test that +specifically distinguishes "zero-covered" from "most uncovered" (a partially-covered name with +MORE absolute uncovered printings than a zero-covered name must still sort after it) and an +`inferred_canonical_card`-without-`RESOLVED` test (confirms an unconfirmed machine vote doesn't +count as coverage). mypy clean (`MPCAutofill/`, whole-package invocation). `black`/`prettier` +clean. + ### No-match autopsy (2026-07-15, post-merge Hold #1 of the pre-scale program) Classified all 176 OCR "parsed-but-no-match" cases from the pilot run diff --git a/docs/lessons.md b/docs/lessons.md index 83ae98984..d345ddc1f 100644 --- a/docs/lessons.md +++ b/docs/lessons.md @@ -267,3 +267,34 @@ instantly — Jest doesn't run Strict Mode's double-invoke the same way a real browser mount does. Fix: make the mock's "have I served the real item yet" state track a genuine domain event the flow itself causes (e.g. a specific vote being submitted), not a raw request count. + +## Ad hoc prod DB/ES access goes through `docker compose run`/`exec`, never a persistent host-side connection script + +The base `docker-compose.yml` publishes Postgres/ES to `127.0.0.1`, and +the DB credentials are public dev defaults (no secret needed) — so a +host-side script pointed at `127.0.0.1:5432`/`9200` connects to live +production data with no further authorization required to _run_ it again +later. That's the hazard: the container boundary (`docker exec`/`docker compose run`) is the actual behavioral guard on an otherwise-open +localhost port, and a saved wrapper script quietly removes it, becoming +ambient capability for whichever future session finds the file — same +class of risk as leaving a dev server squatting a shared port. One-off +reads for a specific task are fine; a durable script that outlives the +task's intent is not, even when nothing in it is secret. + +**Scope, made explicit (2026-07-15)**: the rule guards paths to +**production** data specifically, not "any DB access from a host venv." +`pytest`'s own `testcontainers` fixtures (`cardpicker/tests/conftest.py`) +spin up throwaway, isolated Postgres/ES on different ports +(`47000`/`9300`, not `5432`/`9200`) for the lifetime of one test session +and destroy them after — no path to the real service ever exists in that +flow, so running the test suite from a host venv is not an exception to +this rule, it's simply outside its scope. The venv still never gets +settings/scripts pointing at the real `127.0.0.1:5432`/`9200` ports - +that boundary is unchanged. If a test or fixture is ever found reaching +the real ports instead of its testcontainer, that's a stop-and-report, +not a judgment call. Corollary: mounting the Docker socket into a +container to sidestep this (so tests run "through docker" too) is a +strictly worse trade, not a safer one - it hands the container the +equivalent of host root, a larger ambient capability than the +direct-DB-connection risk it would replace. Declined as an option; don't +build it for this or future workarounds. From 296e51cf5f8d97a12d0cbf4e233f950b743f3950 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Wed, 15 Jul 2026 23:34:26 +0000 Subject: [PATCH 15/23] Addendum item 2a: run-scoped cluster dedup with vote propagation compute_own_image_clusters phashes our own eligible images and collapses distance-0 clusters to one representative before slicing; absorbed members skip OCR/phash/fallback entirely and get their vote via propagation instead. Guards against double-voting a member that already has its own vote from a prior run. No schema change (item 2b deferred). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ --- .../local_identify_printing_tags.py | 149 ++++++++++++- .../commands/local_identify_printing_tags.py | 4 + .../test_local_identify_printing_tags.py | 210 ++++++++++++++++++ docs/features/printing-tags.md | 46 ++++ 4 files changed, 407 insertions(+), 2 deletions(-) diff --git a/MPCAutofill/cardpicker/local_identify_printing_tags.py b/MPCAutofill/cardpicker/local_identify_printing_tags.py index a354fba2b..7ca6b1a8e 100644 --- a/MPCAutofill/cardpicker/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/local_identify_printing_tags.py @@ -582,6 +582,68 @@ class AttributeReport: # front-loaded machine effort onto. Always 0 in dry_run (nothing is written, so nothing can # have newly resolved). uncovered_printings_closed: int = 0 + # addendum item 2a (2026-07-15): cluster dedup report - see compute_own_image_clusters. + cluster_count: int = 0 + cards_absorbed_into_clusters: int = 0 + + +@dataclass(frozen=True) +class ClusterResult: + representatives: list[SelectedCard] + # representative card_id -> the OTHER card_ids (never including the representative itself) + # whose distance-0-identical image means an accepted vote on the representative should + # propagate to them too. Absent entries mean "no cluster" (singleton). + members_by_representative: dict[int, list[int]] + + +def compute_own_image_clusters( + selected: list[SelectedCard], fetch_dpi: Optional[int] = DEFAULT_FETCH_DPI +) -> ClusterResult: + """Addendum item 2a (2026-07-15): phash OUR OWN eligible images (local only - no candidate/ + Scryfall downloads, no extra network cost beyond one fetch per selected card) and collapse + distance-0 (EXACT 64-bit hash match) clusters to one representative (lowest pk, for + determinism) in the work queue. "One read answers N cards": an accepted vote on the + representative propagates as identical votes (same anonymous_id) to every other cluster + member (see run_pilot's write loop) - sound by construction, since a distance-0 match among + OUR OWN uploaded images most plausibly means a duplicate/shared-source image (not + independent depictions that coincidentally look alike - that's the *candidate* art-crop + clustering problem this pilot's phash engine already has to handle via a real + DEFAULT_DISTANCE_THRESHOLD=20, not distance 0), so identical image genuinely entails + identical printing. + + Costs one extra fetch per selected card (this function's own hashing pass, ahead of + _compute_card's own separate fetch) to save the far more expensive OCR/phash/border/frame/ + fallback compute on every absorbed non-representative card - only representatives reach + _compute_card afterward. Scoped to the PRINTING vote only, not border/frame/bleed attribute + votes - absorbed members never get their own image classified at all, so there is nothing of + theirs to propagate for those; documented as a known limitation, not silently dropped. + """ + hash_by_card_id: dict[int, int] = {} + for s in selected: + image = fetch_card_image(s.card, fetch_dpi) + if image is None: + continue + bleed_class = local_fallback.classify_bleed_edge(image) + hash_by_card_id[s.card.pk] = local_phash.compute_card_art_hash(image, bleed_class) + + card_ids_by_hash: dict[int, list[int]] = collections.defaultdict(list) + for s in selected: + h = hash_by_card_id.get(s.card.pk) + if h is not None: + card_ids_by_hash[h].append(s.card.pk) + + members_by_representative: dict[int, list[int]] = {} + absorbed_member_ids: set[int] = set() + for card_ids in card_ids_by_hash.values(): + if len(card_ids) < 2: + continue + representative_id = min(card_ids) + others = [c for c in card_ids if c != representative_id] + members_by_representative[representative_id] = others + absorbed_member_ids.update(others) + + representatives = [s for s in selected if s.card.pk not in absorbed_member_ids] + return ClusterResult(representatives=representatives, members_by_representative=members_by_representative) def run_pilot( @@ -645,6 +707,30 @@ def run_pilot( printing_pks_in_scope.update(c.pk for c in s.candidates) uncovered_printing_pks_in_scope = printing_pks_in_scope - covered_printing_pks_before + # addendum item 2a (2026-07-15): collapse distance-0 duplicate-image clusters to one + # representative BEFORE slicing/chunking - only representatives reach _compute_card; an + # absorbed member's vote comes from propagation in the write loop below instead. + cluster_result = compute_own_image_clusters(list(all_selected_by_card_id.values()), fetch_dpi) + all_selected_by_card_id = {s.card.pk: s for s in cluster_result.representatives} + attributes.cluster_count = len(cluster_result.members_by_representative) + attributes.cards_absorbed_into_clusters = sum(len(m) for m in cluster_result.members_by_representative.values()) + + # a member can be a cluster member (via one engine's selection) while ALREADY having its own + # vote from a DIFFERENT engine's anonymous_id from a prior invocation - e.g. only + # phash-eligible this run (so it appears here) but already has an OCR vote from a previous + # run (which is exactly why it was excluded from THIS run's OCR selection). Propagating a + # same-anonymous_id vote to it anyway would violate CardPrintingTag's own + # (card, printing, anonymous_id) uniqueness constraint - checked once, up front, per + # anonymous_id, not re-queried per propagation call. + _cluster_member_ids = {m for members in cluster_result.members_by_representative.values() for m in members} + members_already_voted_by_anonymous_id: dict[str, set[int]] = collections.defaultdict(set) + if _cluster_member_ids: + for _card_id, _anonymous_id in CardPrintingTag.objects.filter( + card_id__in=_cluster_member_ids, + anonymous_id__in=[OCR_ANONYMOUS_ID, PHASH_ANONYMOUS_ID, FALLBACK_ANONYMOUS_ID], + ).values_list("card_id", "anonymous_id"): + members_already_voted_by_anonymous_id[_anonymous_id].add(_card_id) + # fallback's own idempotence check - it has no selection query/anonymous_id exclusion of # its own (it rides on whichever cards ocr/phash already selected), so a card already # covered by a prior fallback run is excluded here instead. @@ -654,8 +740,19 @@ def run_pilot( ).values_list("card_id", flat=True) ) - ocr_selected_ids = {s.card.pk for s in selected_by_engine.get("ocr", [])} - phash_selected_ids = {s.card.pk for s in selected_by_engine.get("phash", [])} + def _absorb_engine_selection(engine_selected_ids: set[int]) -> set[int]: + # a cluster's representative must run an engine if EITHER it or any absorbed member was + # independently selected for that engine - otherwise clustering could silently drop an + # engine's own eligibility just because the specific card that happened to become the + # representative wasn't itself selected for it. + absorbed = set(engine_selected_ids) + for representative_id, member_ids in cluster_result.members_by_representative.items(): + if any(m in engine_selected_ids for m in member_ids): + absorbed.add(representative_id) + return absorbed + + ocr_selected_ids = _absorb_engine_selection({s.card.pk for s in selected_by_engine.get("ocr", [])}) + phash_selected_ids = _absorb_engine_selection({s.card.pk for s in selected_by_engine.get("phash", [])}) # Checkpointing (Stage 8 pre-scale program item 2): a multi-day unattended run must survive # a kill without losing everything accumulated since the last flush. Matches @@ -689,6 +786,40 @@ def flush() -> None: all_gate_violations.extend(verify_zero_resolutions(batch_written_card_ids)) votes_batch, tag_votes_batch, batch_written_card_ids = [], [], [] + def propagate_cluster_vote( + representative_card_id: int, printing_pk: int, anonymous_id: str, confidence: float + ) -> int: + """Addendum item 2a: an accepted vote on a cluster representative propagates as an + identical vote (same anonymous_id, printing, confidence) to every OTHER cluster member - + absorbed members never ran their own OCR/phash/fallback, so this is the only vote they + ever get. Skips any member that already has a vote from this SAME anonymous_id (e.g. one + engine's vote from a prior invocation, on a member only newly eligible for a DIFFERENT + engine this run) - propagating anyway would violate CardPrintingTag's own + (card, printing, anonymous_id) uniqueness constraint, and would silently double-vote or + attempt to overwrite an existing vote regardless. Returns how many propagated votes were + actually queued, for the engine's votes_written tally.""" + member_ids = cluster_result.members_by_representative.get(representative_card_id, []) + already_voted = members_already_voted_by_anonymous_id.get(anonymous_id, set()) + propagated = 0 + for member_id in member_ids: + if member_id in already_voted: + continue + votes_batch.append( + CardPrintingTag( + card_id=member_id, + printing_id=printing_pk, + is_no_match=False, + anonymous_id=anonymous_id, + source=VoteSource.OCR, + confidence=confidence, + ) + ) + if member_id not in written_card_ids: + written_card_ids.append(member_id) + batch_written_card_ids.append(member_id) + propagated += 1 + return propagated + # Fetch budget (pre-scale program item 3b): every image fetch is one request against the # image CDN Worker, which shares its daily request quota with live site traffic # (docs/features/image-cdn.md) - an unattended multi-hour pilot slice must not be able to @@ -830,6 +961,9 @@ def flush() -> None: result_ocr.audit.append({"card_id": card_id, "raw_text": outcome.ocr_vote.detail}) written_card_ids.append(card_id) batch_written_card_ids.append(card_id) + result_ocr.votes_written += propagate_cluster_vote( + card_id, outcome.ocr_vote.printing_pk, OCR_ANONYMOUS_ID, outcome.ocr_vote.confidence + ) elif outcome.ocr_skip_reason and result_ocr is not None: result_ocr.skip_counts[outcome.ocr_skip_reason] += 1 @@ -852,6 +986,9 @@ def flush() -> None: if card_id not in written_card_ids: written_card_ids.append(card_id) batch_written_card_ids.append(card_id) + result_phash.votes_written += propagate_cluster_vote( + card_id, outcome.phash_vote.printing_pk, PHASH_ANONYMOUS_ID, outcome.phash_vote.confidence + ) elif outcome.phash_skip_reason and result_phash is not None: result_phash.skip_counts[outcome.phash_skip_reason] += 1 @@ -874,6 +1011,12 @@ def flush() -> None: if card_id not in written_card_ids: written_card_ids.append(card_id) batch_written_card_ids.append(card_id) + result_fallback.votes_written += propagate_cluster_vote( + card_id, + outcome.fallback_vote.printing_pk, + FALLBACK_ANONYMOUS_ID, + outcome.fallback_vote.confidence, + ) elif outcome.fallback_skip_reason: result_fallback.skip_counts[outcome.fallback_skip_reason] += 1 @@ -1008,6 +1151,8 @@ def verify_zero_resolutions(card_ids: list[int], batch_size: int = 2000) -> list "RESOLUTION_FLOOR_DPI", "count_below_resolution_floor", "compute_covered_printing_pks", + "ClusterResult", + "compute_own_image_clusters", "select_candidates", "get_worker_image_url", "fetch_card_image", diff --git a/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py b/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py index e34b988f7..ef8d52f93 100644 --- a/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/management/commands/local_identify_printing_tags.py @@ -238,6 +238,10 @@ def _parse_source_pks(raw: str) -> list[int]: print(f" bleed votes: {dict(attributes.bleed_votes_by_class)}") print(f" bleed abstains: {attributes.bleed_abstain_count}") print(f" uncovered printings closed this run: {attributes.uncovered_printings_closed}") + print( + f" image clusters: {attributes.cluster_count} " + f"(cards absorbed: {attributes.cards_absorbed_into_clusters})" + ) if dry_run: print("Dry run - nothing written, gate check not run.") diff --git a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py index 2e7ec5a18..674405327 100644 --- a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py @@ -23,6 +23,7 @@ CandidateNameIndex, CandidatePrinting, compute_covered_printing_pks, + compute_own_image_clusters, count_below_resolution_floor, get_worker_image_url, run_pilot, @@ -1367,3 +1368,212 @@ def test_workers_one_does_not_set_omp_thread_limit(self, db, monkeypatch): run_pilot(engine="ocr", limit=10, dry_run=True, nice=False, workers=1) assert "OMP_THREAD_LIMIT" not in os.environ + + +class TestClusterDedup: + """Addendum item 2a (2026-07-15): distance-0 (byte-identical fetched image) clustering, + scoped to this run only - no schema/content_hash persistence (that's item 2b, deferred).""" + + def test_two_cards_with_identical_images_cluster_with_lower_pk_as_representative(self, db): + CanonicalCardFactory(name="Forest") + card_a = CardFactory(name="Forest") + card_b = CardFactory(name="Forest") + assert card_a.pk < card_b.pk + + selected = select_candidates("ocr") + assert {s.card.pk for s in selected} == {card_a.pk, card_b.pk} + + import cardpicker.local_identify_printing_tags as module + + identical_image = Image.new("RGB", (750, 1050), (5, 5, 5)) + module_monkeypatch_target = module.fetch_card_image + try: + module.fetch_card_image = lambda card, dpi=None: identical_image + cluster_result = compute_own_image_clusters(selected) + finally: + module.fetch_card_image = module_monkeypatch_target + + assert cluster_result.members_by_representative == {card_a.pk: [card_b.pk]} + assert [s.card.pk for s in cluster_result.representatives] == [card_a.pk] + + def test_different_images_do_not_cluster(self, db, monkeypatch): + # a solid, uniform fill has ZERO frequency content, so a DCT-based perceptual hash + # (imagehash's phash) can't distinguish one flat color from another - real art crops + # always have texture/detail, so distinguishable synthetic fixtures need actual drawn + # content, not just a different fill color (this genuinely tripped the first version of + # this test - a plain color-swap fixture accidentally clustered against production code + # that was working correctly). + CanonicalCardFactory(name="Forest") + card_a = CardFactory(name="Forest") + card_b = CardFactory(name="Forest") + + import cardpicker.local_identify_printing_tags as module + + image_a = Image.new("RGB", (750, 1050), (5, 5, 5)) + draw_a = ImageDraw.Draw(image_a) + draw_a.rectangle([100, 100, 300, 300], fill=(200, 30, 30)) + + image_b = Image.new("RGB", (750, 1050), (5, 5, 5)) + draw_b = ImageDraw.Draw(image_b) + draw_b.ellipse([400, 400, 700, 700], fill=(30, 200, 30)) + + images_by_card_id = {card_a.pk: image_a, card_b.pk: image_b} + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: images_by_card_id[card.pk]) + + cluster_result = module.compute_own_image_clusters(select_candidates("ocr")) + + assert cluster_result.members_by_representative == {} + assert {s.card.pk for s in cluster_result.representatives} == {card_a.pk, card_b.pk} + + def test_unfetchable_image_stays_a_singleton_representative(self, db, monkeypatch): + CanonicalCardFactory(name="Forest") + card = CardFactory(name="Forest") + + import cardpicker.local_identify_printing_tags as module + + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: None) + + cluster_result = module.compute_own_image_clusters(select_candidates("ocr")) + + assert cluster_result.members_by_representative == {} + assert [s.card.pk for s in cluster_result.representatives] == [card.pk] + + def test_accepted_vote_on_representative_propagates_to_absorbed_member(self, db, monkeypatch): + printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + card_a = CardFactory(name="Forest") + card_b = CardFactory(name="Forest") + + import cardpicker.local_identify_printing_tags as module + + identical_image = Image.new("RGB", (750, 1050), (5, 5, 5)) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: identical_image) + monkeypatch.setattr( + module, + "run_ocr_for_card", + lambda selected, image, crop_box, bleed_class=None: module.OcrCardResult( + vote=module.EngineVote(engine="ocr", printing_pk=printing.pk, confidence=0.85, detail="raw") + ), + ) + + results, attributes = run_pilot(engine="ocr", limit=10, dry_run=False, nice=False) + + # one real OCR call, one propagated vote - both cards end up with an identical vote. + assert results["ocr"].votes_written == 2 + assert attributes.cluster_count == 1 + assert attributes.cards_absorbed_into_clusters == 1 + vote_a = CardPrintingTag.objects.get(card=card_a, anonymous_id=OCR_ANONYMOUS_ID) + vote_b = CardPrintingTag.objects.get(card=card_b, anonymous_id=OCR_ANONYMOUS_ID) + # not just "some vote exists" - the propagated vote is a genuine copy: same printing, + # same anonymous_id, same confidence, same source as the representative's real vote. + assert vote_a.printing_id == vote_b.printing_id == printing.pk + assert vote_a.anonymous_id == vote_b.anonymous_id == OCR_ANONYMOUS_ID + assert vote_a.confidence == vote_b.confidence == 0.85 + assert vote_a.source == vote_b.source == VoteSource.OCR + assert vote_a.is_no_match == vote_b.is_no_match is False + + def test_member_with_an_existing_vote_from_a_prior_run_is_not_double_voted_or_overwritten(self, db, monkeypatch): + printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + other_printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="bbb")) + CardFactory(name="Forest") + card_b = CardFactory(name="Forest") + # card_b already has its OWN OCR vote from a prior run, on a DIFFERENT printing than + # what card_a (the representative) is about to vote for this run - simulates the exact + # scenario that would violate the (card, printing, anonymous_id) uniqueness constraint + # (or silently create a second conflicting OCR vote) if propagation didn't guard it. + existing_vote = CardPrintingTagFactory( + card=card_b, printing=other_printing, anonymous_id=OCR_ANONYMOUS_ID, source=VoteSource.OCR + ) + + import cardpicker.local_identify_printing_tags as module + + identical_image = Image.new("RGB", (750, 1050), (5, 5, 5)) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: identical_image) + monkeypatch.setattr( + module, + "run_ocr_for_card", + lambda selected, image, crop_box, bleed_class=None: module.OcrCardResult( + vote=module.EngineVote(engine="ocr", printing_pk=printing.pk, confidence=0.85, detail="raw") + ), + ) + # card_b is already excluded from OCR selection (existing vote), so it only reaches + # all_selected_by_card_id (and thus clustering) via an independent phash eligibility. + monkeypatch.setattr( + module, + "run_phash_for_card", + lambda selected, image, threshold, margin, max_candidates, bleed_class=None: (None, "no-clear-winner"), + ) + + results, _attributes = run_pilot(engine="both", limit=10, dry_run=False, nice=False) + + # exactly one OCR vote written this run (card_a's real vote) - propagation to card_b was + # correctly skipped, not attempted and silently failed. + assert results["ocr"].votes_written == 1 + assert CardPrintingTag.objects.filter(card=card_b, anonymous_id=OCR_ANONYMOUS_ID).count() == 1 + untouched_vote = CardPrintingTag.objects.get(card=card_b, anonymous_id=OCR_ANONYMOUS_ID) + assert untouched_vote.pk == existing_vote.pk + assert untouched_vote.printing_id == other_printing.pk # unchanged, not overwritten + + def test_absorbed_member_never_reaches_ocr_or_phash_processing(self, db, monkeypatch): + # the whole point of dedup is not re-running the expensive engines on cluster members - + # this is the test that actually proves the efficiency win, not just vote correctness. + CanonicalCardFactory(name="Forest") + card_a = CardFactory(name="Forest") + card_b = CardFactory(name="Forest") + assert card_a.pk < card_b.pk + + import cardpicker.local_identify_printing_tags as module + + identical_image = Image.new("RGB", (750, 1050), (5, 5, 5)) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: identical_image) + + ocr_called_for_card_ids: list[int] = [] + + def recording_run_ocr_for_card(selected, image, crop_box, bleed_class=None): + ocr_called_for_card_ids.append(selected.card.pk) + return module.OcrCardResult() + + monkeypatch.setattr(module, "run_ocr_for_card", recording_run_ocr_for_card) + + run_pilot(engine="ocr", limit=10, dry_run=False, nice=False) + + assert ocr_called_for_card_ids == [card_a.pk] + assert card_b.pk not in ocr_called_for_card_ids + + def test_absorbed_members_own_engine_eligibility_still_runs_via_the_representative(self, db, monkeypatch): + # card_a (the lower-pk representative) is only phash-eligible; card_b (absorbed member) + # is only ocr-eligible - the representative must still run OCR on card_a's behalf, or + # card_b's OCR opportunity is silently lost when it gets absorbed. + printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) + card_a = CardFactory(name="Forest") + card_b = CardFactory(name="Forest") + + import cardpicker.local_identify_printing_tags as module + + identical_image = Image.new("RGB", (750, 1050), (5, 5, 5)) + monkeypatch.setattr(module, "fetch_card_image", lambda card, dpi=None: identical_image) + + def fake_select_candidates(engine, index=None, exclude_source_pks=None, covered_printing_pks=None): + real = select_candidates(engine, index, exclude_source_pks, covered_printing_pks) + if engine == "ocr": + return [s for s in real if s.card.pk == card_b.pk] + return [s for s in real if s.card.pk == card_a.pk] + + monkeypatch.setattr(module, "select_candidates", fake_select_candidates) + monkeypatch.setattr( + module, + "run_ocr_for_card", + lambda selected, image, crop_box, bleed_class=None: module.OcrCardResult( + vote=module.EngineVote(engine="ocr", printing_pk=printing.pk, confidence=0.85, detail="raw") + ), + ) + monkeypatch.setattr( + module, + "run_phash_for_card", + lambda selected, image, threshold, margin, max_candidates, bleed_class=None: (None, "no-clear-winner"), + ) + + results, _attributes = run_pilot(engine="both", limit=10, dry_run=False, nice=False) + + assert results["ocr"].votes_written == 2 + assert CardPrintingTag.objects.filter(card=card_a, anonymous_id=OCR_ANONYMOUS_ID).exists() + assert CardPrintingTag.objects.filter(card=card_b, anonymous_id=OCR_ANONYMOUS_ID).exists() diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index c4d328fa2..c4fcdc5f4 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -1577,6 +1577,52 @@ MORE absolute uncovered printings than a zero-covered name must still sort after count as coverage). mypy clean (`MPCAutofill/`, whole-package invocation). `black`/`prettier` clean. +### Cluster dedup, run-scoped (2026-07-15, addendum item 2a) + +Before slicing, `compute_own_image_clusters` phashes OUR OWN eligible images (local only - no +candidate/Scryfall downloads) via the same `local_phash.compute_card_art_hash` the phash engine +already uses, and collapses distance-0 (EXACT 64-bit hash match) groups to one representative +(lowest pk, for determinism) - only representatives reach `_compute_card`; absorbed members never +run their own OCR/phash/border/frame/fallback at all. "One read answers N cards": an accepted +vote on the representative propagates as an identical `CardPrintingTag` (same anonymous*id, +printing, confidence, source) to every absorbed member. Sound by construction: a distance-0 match +among OUR OWN uploaded images most plausibly means a duplicate/shared-source image, not +independent depictions that coincidentally look alike (that's the \_candidate* art-crop clustering +problem the phash engine already handles separately via `DEFAULT_DISTANCE_THRESHOLD=20`, a much +looser bar than 0) - identical image genuinely entails identical printing. Costs one extra fetch +per selected card (the clustering pass itself) to save the far more expensive per-card compute +pipeline on every absorbed duplicate. + +Scoped to the printing-identification vote only, not border/frame/bleed attribute votes - +absorbed members never get their own image classified, so there's nothing of theirs to +propagate for those; a documented limitation, not a silent gap. Run-scoped only, no schema +change: no `content_hash` persisted anywhere (that's item 2b, deferred as a standalone future +task for federation-v1's content_hash groundwork). New `AttributeReport.cluster_count`/ +`cards_absorbed_into_clusters` report fields. + +**A real idempotence gap found and fixed before landing, not just anticipated**: a cluster +member can reach clustering via one engine's independent eligibility (e.g. phash) while already +carrying a vote from a DIFFERENT engine's `anonymous_id` from a prior invocation (the exact +reason it was excluded from that OTHER engine's selection this run). Propagating a same- +`anonymous_id` vote to it anyway would violate `CardPrintingTag`'s own +`(card, printing, anonymous_id)` uniqueness constraint - checked once per run (a single query +across all cluster members, not re-queried per propagation call) and skipped, not attempted. +`ocr_selected_ids`/`phash_selected_ids` also get absorbed into the representative's own +eligibility (a representative must run an engine if EITHER it or any absorbed member was +independently selected for that engine), so clustering can't silently drop an engine's +eligibility just because the specific card that became the representative wasn't itself +originally selected for it. + +Verified: 96/96 pilot tests pass (7 new: `TestClusterDedup`), including a test that caught a +genuine test-fixture bug during development (two different solid-color images accidentally +hashed identically - imagehash's DCT-based `phash` has zero frequency content on any uniform +fill regardless of color, so distinguishable synthetic fixtures need actual drawn shapes, not +just a different fill color; the production clustering logic was working correctly the whole +time), a test proving the double-vote/overwrite gap above is actually fixed (not just that some +vote exists), and a test proving the efficiency win itself (an absorbed member's card id is +never passed to `run_ocr_for_card`, not just that it ends up with a vote). mypy clean. +`black`/`prettier` clean. + ### No-match autopsy (2026-07-15, post-merge Hold #1 of the pre-scale program) Classified all 176 OCR "parsed-but-no-match" cases from the pilot run From 25eb8aa44f0efbc79174c6d1419d5fcb5590bd29 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Thu, 16 Jul 2026 00:04:15 +0000 Subject: [PATCH 16/23] Bleed tag: negative-only voting (supersedes both-directions design) cast_bleed_edge_vote now writes a vote only for a 'trimmed' reading; 'bleed' (the ~97.5% common case) casts nothing at all, so absence of a vote becomes the documented convention for normal bleed - avoids flooding moderation with routine APPLY confirmations on a SENSITIVE tag meant to flag the rare exception. Updated sensitive_tags.py's comment, which previously documented the opposite convention. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ --- MPCAutofill/cardpicker/local_fallback.py | 18 +++++++----- MPCAutofill/cardpicker/sensitive_tags.py | 15 ++++++---- .../cardpicker/tests/test_local_fallback.py | 24 ++++++++-------- .../test_local_identify_printing_tags.py | 14 ++++++---- docs/features/printing-tags.md | 28 +++++++++++++++---- 5 files changed, 66 insertions(+), 33 deletions(-) diff --git a/MPCAutofill/cardpicker/local_fallback.py b/MPCAutofill/cardpicker/local_fallback.py index 327336813..0eed78d54 100644 --- a/MPCAutofill/cardpicker/local_fallback.py +++ b/MPCAutofill/cardpicker/local_fallback.py @@ -545,20 +545,24 @@ def classify_bleed_edge(card_image: "Image.Image") -> Optional[str]: def cast_bleed_edge_vote(card: Card, bleed_class: Optional[str]) -> Optional[CardTagVote]: - """Positive (APPLY) vote for a clear bleed margin, negative (NOT_APPLICABLE) for clearly - trimmed, no vote at all for an ambiguous/unclassifiable reading (the caller counts this as - an abstain without writing anything, same convention as classify_frame_style's abstain - path).""" - if bleed_class is None: + """Negative-only (2026-07-15, consolidated respec item 4b, supersedes this function's + original both-directions design): a vote is cast ONLY for a clearly 'trimmed' reading + (NOT_APPLICABLE) - no vote at all for 'bleed' (the ~97.5% common case, per the 40-source + validation) or an ambiguous/unclassifiable reading. Absence of any vote is the documented + convention for "this card has normal bleed" - see BLEED_EDGE_TAG_NAME's own description and + docs/features/printing-tags.md's Stage 8 section. Rationale: `appropriate-bleed` is a + SENSITIVE tag needing moderator co-sign regardless of machine votes - voting APPLY on the + routine 97.5% case would flood moderation with confirmations of normalcy rather than + surfacing the rare real exception, which is what a SENSITIVE tag is for.""" + if bleed_class != "trimmed": return None tag = Tag.objects.filter(name=BLEED_EDGE_TAG_NAME).first() if tag is None: return None - polarity = VotePolarity.APPLY if bleed_class == "bleed" else VotePolarity.NOT_APPLICABLE return CardTagVote( card=card, tag=tag, - polarity=polarity, + polarity=VotePolarity.NOT_APPLICABLE, anonymous_id=FALLBACK_ANONYMOUS_ID, source=VoteSource.OCR, confidence=BLEED_EDGE_VOTE_CONFIDENCE, diff --git a/MPCAutofill/cardpicker/sensitive_tags.py b/MPCAutofill/cardpicker/sensitive_tags.py index 14b9f8650..96ae93c47 100644 --- a/MPCAutofill/cardpicker/sensitive_tags.py +++ b/MPCAutofill/cardpicker/sensitive_tags.py @@ -30,11 +30,16 @@ (NSFW, "Mature/adult content - excluded from search by default", "NSFW"), ("low-res", "Image quality too poor to print", "Low quality"), ("incorrect-info", "Card text/details do not match the real card", "Incorrect card info"), - # Deliberately the POSITIVE framing ("has appropriate bleed"), not a negative - # "missing-bleed": upstream drives REQUIRE appropriate bleed on every card, so the useful - # verified state is the positive one - absence just means "not yet verified", and a - # definitive "lacks bleed" verdict is still expressible as this tag resolving REJECT. - # Sensitive because that verification is exactly a moderator's co-sign. + # Positive framing in the NAME ("has appropriate bleed"), but the VOTING convention + # changed 2026-07-16 (consolidated respec item 4b) once local_fallback.classify_bleed_edge + # gave reliable machine coverage of the negative case: the pilot casts a vote ONLY for a + # detected 'trimmed' image (NOT_APPLICABLE) - absence of ANY vote is now the documented + # convention for "presumed normal bleed", not "not yet verified" as originally designed. + # This deliberately supersedes the original human-moderation-era framing (absence used to + # mean "unchecked") - a SENSITIVE tag existing to catch the RARE exception is a better fit + # once ~97.5% of cards can be machine-confirmed normal (see local_fallback.py's + # cast_bleed_edge_vote for the full rationale) than voting APPLY on the routine majority + # ever was. Sensitive because a moderator co-sign is still required either direction. ("appropriate-bleed", "Verified to include the full bleed margin required for printing", "Appropriate Bleed"), ] diff --git a/MPCAutofill/cardpicker/tests/test_local_fallback.py b/MPCAutofill/cardpicker/tests/test_local_fallback.py index bac6599e3..7dad9a3ba 100644 --- a/MPCAutofill/cardpicker/tests/test_local_fallback.py +++ b/MPCAutofill/cardpicker/tests/test_local_fallback.py @@ -381,29 +381,31 @@ def test_exact_reference_ratios_round_trip(self): class TestCastBleedEdgeVote: + """Negative-only (2026-07-16, consolidated respec item 4b, supersedes the original + both-directions design): a vote is cast ONLY for 'trimmed' - 'bleed' casts nothing at all, + regardless of whether the tag exists, since absence of a vote IS the "normal bleed" signal.""" + def test_no_reading_casts_nothing(self, db): card = CardFactory() assert cast_bleed_edge_vote(card, None) is None - def test_unseeded_tag_degrades_to_no_vote(self, db): + def test_bleed_reading_casts_nothing_even_with_the_tag_seeded(self, db): + TagFactory(name=BLEED_EDGE_TAG_NAME) card = CardFactory() assert cast_bleed_edge_vote(card, "bleed") is None - def test_bleed_casts_a_positive_vote_on_the_existing_tag(self, db): - TagFactory(name=BLEED_EDGE_TAG_NAME) + def test_unseeded_tag_degrades_trimmed_to_no_vote(self, db): card = CardFactory() - vote = cast_bleed_edge_vote(card, "bleed") - assert vote is not None - assert vote.pk is None - assert vote.tag.name == BLEED_EDGE_TAG_NAME - assert vote.polarity == VotePolarity.APPLY - assert vote.anonymous_id == FALLBACK_ANONYMOUS_ID - assert vote.source == VoteSource.OCR - assert vote.confidence == 0.7 + assert cast_bleed_edge_vote(card, "trimmed") is None def test_trimmed_casts_a_negative_vote_on_the_existing_tag(self, db): TagFactory(name=BLEED_EDGE_TAG_NAME) card = CardFactory() vote = cast_bleed_edge_vote(card, "trimmed") assert vote is not None + assert vote.pk is None + assert vote.tag.name == BLEED_EDGE_TAG_NAME assert vote.polarity == VotePolarity.NOT_APPLICABLE + assert vote.anonymous_id == FALLBACK_ANONYMOUS_ID + assert vote.source == VoteSource.OCR + assert vote.confidence == 0.7 diff --git a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py index 674405327..c491e20e3 100644 --- a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py @@ -1213,10 +1213,13 @@ def fake_ocr(selected, image, crop_box, bleed_class=None): class TestBleedEdgeVotesEndToEnd: - """Addendum item 7: run_pilot casts a real vote on the pre-existing appropriate-bleed tag - for every card with a fetched image, independent of printing-vote success.""" + """Addendum item 7 + consolidated respec item 4b (2026-07-16, negative-only): run_pilot casts + a vote on the pre-existing appropriate-bleed tag ONLY for a 'trimmed' reading. A 'bleed' + reading (the ~97.5% common case) still counts toward the census (bleed_votes_by_class) but + writes NO vote at all - absence of any vote is the documented convention for "presumed + normal bleed", per sensitive_tags.py's SENSITIVE_TAGS comment.""" - def test_bleed_shaped_image_casts_a_positive_vote(self, db, monkeypatch): + def test_bleed_shaped_image_is_censused_but_casts_no_vote(self, db, monkeypatch): CanonicalCardFactory(name="Forest") card = CardFactory(name="Forest") TagFactory(name="appropriate-bleed") @@ -1238,9 +1241,10 @@ def test_bleed_shaped_image_casts_a_positive_vote(self, db, monkeypatch): _results, attributes = run_pilot(engine="ocr", limit=10, dry_run=False, nice=False) + # census still reflects the classification... assert attributes.bleed_votes_by_class == {"bleed": 1} - vote = CardTagVote.objects.get(card=card, tag__name="appropriate-bleed") - assert vote.polarity == VotePolarity.APPLY + # ...but no vote was actually written - absence IS the signal for this case. + assert not CardTagVote.objects.filter(card=card, tag__name="appropriate-bleed").exists() def test_trimmed_shaped_image_casts_a_negative_vote(self, db, monkeypatch): CanonicalCardFactory(name="Forest") diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index c4fcdc5f4..c06b6a1af 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -1071,11 +1071,29 @@ either side while still abstaining on a genuinely non-standard image **Wired into `run_pilot`**: fires for every card with a fetched image, independent of printing-vote success (same "double duty" convention as -border/frame attribute votes) - positive (`APPLY`) vote for bleed, -negative (`NOT_APPLICABLE`) for trimmed, abstain (nothing written, only -counted) for ambiguous. `VoteSource.OCR`, confidence 0.7 -(`BLEED_EDGE_VOTE_CONFIDENCE`). No ground-truth counterpart to prefer - -unlike border/frame, Scryfall doesn't encode this at all. +border/frame attribute votes) - classification is censused +(`AttributeReport.bleed_votes_by_class`) for every card regardless, but +see the negative-only voting change below for what actually gets +written. `VoteSource.OCR`, confidence 0.7 (`BLEED_EDGE_VOTE_CONFIDENCE`). +No ground-truth counterpart to prefer - unlike border/frame, Scryfall +doesn't encode this at all. + +**Negative-only voting (2026-07-16, consolidated respec item 4b - +supersedes the original both-directions design above)**: a vote is now +cast ONLY for a `trimmed` reading (`NOT_APPLICABLE`) - a `bleed` reading +(the ~97.5% common case per the 40-source validation) still counts +toward the census but writes NO `CardTagVote` at all. **Absence of any +vote is the documented convention for "presumed normal bleed"** - +updated in `sensitive_tags.py`'s `SENSITIVE_TAGS` comment alongside this +doc, since the tag's _original_ design comment said the opposite +("absence just means not yet verified") from before this pilot existed. +Rationale: `appropriate-bleed` is `SENSITIVE` and needs a moderator +co-sign regardless of machine votes - voting `APPLY` on the routine 97.5% +case would flood moderation with confirmations of normalcy instead of +surfacing the rare real exception, which is what a SENSITIVE tag is for. +Confidence unchanged (0.7). No new tag seeded - the existing-tag check +(`Tag.objects.filter(name=...).first()`, degrades to no vote if absent) +was already in place before this change. ### DPI-tag audit (2026-07-15, addendum item 8 - report only) From 65f3fa6aba80397b69256e8d3e03f0dcedef4f53 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Thu, 16 Jul 2026 00:14:08 +0000 Subject: [PATCH 17/23] Document bottleneck-split measurement: CPU-bound, not I/O-bound Fresh instrumented 50-card run against current code shows fetch is only ~13% of per-card cost; border/frame classification + pass-2 fallback dominate at ~65-72%. No separate fetch-thread pool exists currently - _compute_card's single worker pool handles fetch+OCR+ phash+fallback together. This favors a core-count resize or manifest mode over decoupling fetch threads. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ --- docs/features/printing-tags.md | 44 ++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index c06b6a1af..6393c38e4 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -1641,6 +1641,50 @@ vote exists), and a test proving the efficiency win itself (an absorbed member's never passed to `run_ocr_for_card`, not just that it ends up with a vote). mypy clean. `black`/`prettier` clean. +### Bottleneck split, current pipeline state (2026-07-16, throughput track item 2a) + +Re-measured phase timing against the CURRENT code (post items 1/2a/3/4/4b) rather than trusting +item 3a's original breakdown, which predates dpi=250, crop tightening, bleed-first +classification, and clustering entirely. Real instrumented run, 50 selected candidates +(representative-only, post-clustering), against the live DB/API - not simulated: + +| phase | mean/card | share (uncorrected) | +| ------------------------------------- | --------: | ------------------: | +| `fetch_card_image` | 0.450s | 13.4% | +| `classify_bleed_edge` | ~0.000s | ~0.0% | +| OCR (crop+preprocess+tesseract) | 0.478s | 14.3% | +| phash (hash+compare) | ~0.000s | ~0.0% | +| border/frame (`detect_illus_anchor`+) | 1.206s | 36.0% | +| pass-2 fallback | 1.218s | 36.3% | + +**Measurement caveat, stated plainly**: this run called fallback unconditionally for every +representative (not gated on pass-1's real accept/reject outcome), so its 36.3% share is +inflated relative to real `run_pilot` behavior (item 3a's original sample: fallback fires +~70% of the time). Corrected estimate using that same 70% rate: +`0.450 + 0.478 + 1.206 + (1.218 × 0.7) ≈ 2.99s/card` sequential, for cards that reach full +compute (clustering representatives only). + +**Bonus real data point from the same sample**: 13/50 selected cards (26%) were absorbed into +10 clusters by item 2a's dedup - a materially higher rate than assumed, though from one +50-card sample, not a claim about the full-catalog rate. + +**The clear finding: this is CPU-bound, not I/O-bound.** `fetch_card_image` is ~13% of +per-card cost; `detect_illus_anchor`-plus-border-classification and pass-2 fallback together +are ~72% (uncorrected) / ~65% (corrected). This directly answers throughput track item 2a's own +question: **the "6-8 fetch threads, I/O-bound, no core needed" idea does not currently exist as +a mechanism** - `_compute_card`'s single `ThreadPoolExecutor(max_workers=workers)` runs fetch +AND OCR AND phash AND fallback all in the SAME worker, sized for CPU-bound work +(`DEFAULT_WORKERS=2`, matching this box's core count). Decoupling fetch into its own larger pool +would only ever attack the ~13% fetch share - a real potential improvement, but not the +dominant cost, and not built in this pass. This bottleneck split is the evidence that makes +manifest mode (item 2c) and a core-count resize (item 2b) the higher-leverage levers, not a +larger fetch pool. + +**Current instance shape** (OCI instance-metadata endpoint, no auth needed - confirmed +`169.254.169.254/opc/v2/instance/`): `VM.Standard.A1.Flex`, **2 OCPUs, 12 GB RAM**, +`ca-montreal-1`. Matches `DEFAULT_WORKERS=2`'s own derivation (item 3d) exactly - this box has +never had spare cores for a bigger pool without a resize. + ### No-match autopsy (2026-07-15, post-merge Hold #1 of the pre-scale program) Classified all 176 OCR "parsed-but-no-match" cases from the pilot run From e08f348fd2945480ca48f4ae2b85d48d2aed3eee Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Thu, 16 Jul 2026 00:25:23 +0000 Subject: [PATCH 18/23] Document 250-card soak test: throughput stable, clustering rate confirmed Real 250-card dry run (not a burst) confirms ~1.94s/effective-card, consistent with the prior top-down measurement, and independently corroborates the ~26-28% cluster-absorption rate seen in the bottleneck-split sample. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ --- docs/features/printing-tags.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index 6393c38e4..449a82ac3 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -1685,6 +1685,24 @@ larger fetch pool. `ca-montreal-1`. Matches `DEFAULT_WORKERS=2`'s own derivation (item 3d) exactly - this box has never had spare cores for a bigger pool without a resize. +### Soak test at the current box (2026-07-16, throughput track item 2d) + +Real 250-card `--dry-run --workers 2` run (not a burst - the prior `--workers=2` safety +validation was only ~20 seconds/10 cards) against the live DB/API with live services running +normally. Clustering (item 2a) absorbed 70/250 selected candidates (28%) into 59 clusters before +the main loop even started, leaving 180 representatives actually processed - closely matching +the bottleneck-split sample's independently-observed 26% (13/50) absorption rate, two samples +now agreeing rather than one small anecdote. Total wall-clock ~400s (00:16:10 start to 00:22:50 +log-file mtime), including container startup/migrate/collectstatic overhead (~45-60s fixed cost, +not pilot processing) - effective throughput **≈1.94s/card** across the 180 processed +representatives, consistent with the previously-established top-down 1.863s/card figure from +the original 392-candidate real run. **Caveat, stated plainly**: this run's progress markers +(50/100/150-candidate checkpoints) weren't individually timestamped, so this confirms +AGGREGATE throughput held up over a real multi-hundred-card window (not just a burst) but +doesn't give intra-run stability granularity (e.g. whether the first 50 cards processed at a +different rate than the last 50) - a finer-grained timing pass would be needed for that +specific claim, not done here. + ### No-match autopsy (2026-07-15, post-merge Hold #1 of the pre-scale program) Classified all 176 OCR "parsed-but-no-match" cases from the pilot run From 402996df57dc6a693f2ea4365a98670910bc73e5 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Thu, 16 Jul 2026 00:29:13 +0000 Subject: [PATCH 19/23] Clarify soak test is the pre-resize baseline, not the workers=3 number --- docs/features/printing-tags.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index 449a82ac3..ab914cd89 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -1685,7 +1685,13 @@ larger fetch pool. `ca-montreal-1`. Matches `DEFAULT_WORKERS=2`'s own derivation (item 3d) exactly - this box has never had spare cores for a bigger pool without a resize. -### Soak test at the current box (2026-07-16, throughput track item 2d) +### Soak test at the current box, PRE-RESIZE baseline (2026-07-16, throughput track item 2d) + +**This measurement is at the CURRENT shape (2 OCPU/12GB, `--workers 2`) - it is the pre-resize +baseline, NOT the workers=3 post-resize number the resize decision is waiting on.** A separate +post-resize soak test (same 250-card window, same selection/dedup) at `--workers 3` on 4 +OCPU/24GB is required before comparing - see the entry below once that lands. Do not conflate +the two numbers. Real 250-card `--dry-run --workers 2` run (not a burst - the prior `--workers=2` safety validation was only ~20 seconds/10 cards) against the live DB/API with live services running From b8f749c2849feac896e966a41ec35b29f89dd0c0 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Thu, 16 Jul 2026 01:24:58 +0000 Subject: [PATCH 20/23] Redact specific instance shape/region from public doc Exact OCPU/RAM/region values shouldn't sit in a public-facing doc - keeping the substantive finding (core count matches DEFAULT_WORKERS) without the specific numbers. History still has the prior values; owner declined a history rewrite for this pass. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ --- docs/features/printing-tags.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index ab914cd89..87b212568 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -1680,18 +1680,19 @@ dominant cost, and not built in this pass. This bottleneck split is the evidence manifest mode (item 2c) and a core-count resize (item 2b) the higher-leverage levers, not a larger fetch pool. -**Current instance shape** (OCI instance-metadata endpoint, no auth needed - confirmed -`169.254.169.254/opc/v2/instance/`): `VM.Standard.A1.Flex`, **2 OCPUs, 12 GB RAM**, -`ca-montreal-1`. Matches `DEFAULT_WORKERS=2`'s own derivation (item 3d) exactly - this box has -never had spare cores for a bigger pool without a resize. +**Current instance shape** (checked via the cloud provider's own instance-metadata endpoint, +no auth needed - exact shape/region kept out of this public doc, see CLAUDE.local.md/journal +for the specific values): core count matches `DEFAULT_WORKERS=2`'s own derivation (item 3d) +exactly - this box has never had spare cores for a bigger pool without a resize. ### Soak test at the current box, PRE-RESIZE baseline (2026-07-16, throughput track item 2d) -**This measurement is at the CURRENT shape (2 OCPU/12GB, `--workers 2`) - it is the pre-resize +**This measurement is at the box's PRE-RESIZE core count (`--workers 2`) - it is the pre-resize baseline, NOT the workers=3 post-resize number the resize decision is waiting on.** A separate -post-resize soak test (same 250-card window, same selection/dedup) at `--workers 3` on 4 -OCPU/24GB is required before comparing - see the entry below once that lands. Do not conflate -the two numbers. +post-resize soak test (same 250-card window, same selection/dedup) at a higher `--workers` count +on the resized shape is required before comparing - see the entry below once that lands. Do not +conflate the two numbers. (Exact shape/OCPU/RAM values kept out of this public doc - see +CLAUDE.local.md/journal.) Real 250-card `--dry-run --workers 2` run (not a burst - the prior `--workers=2` safety validation was only ~20 seconds/10 cards) against the live DB/API with live services running From 3d1ea7e0f01e9334993df60aa0c087b70568a5d3 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Thu, 16 Jul 2026 01:45:02 +0000 Subject: [PATCH 21/23] Exclude tokens/cardbacks from pilot selection - unmatchable by design Diagnosed live: a token's printed collector line reads its parent set's code, while its CanonicalCard candidates use token-specific set codes that never match - structural, not a parsing bug. Item 1's descending-uncovered-count ordering was front-loading generic multi-set token names (huge candidate counts, near-zero coverage) to the very front of every real selection, yielding 0/250 OCR votes in today's soak tests where the original pre-items-1/3/4 baseline (94/300) had none of this population dominating. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ --- .../local_identify_printing_tags.py | 19 +++++++++++++++++-- .../test_local_identify_printing_tags.py | 16 ++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/MPCAutofill/cardpicker/local_identify_printing_tags.py b/MPCAutofill/cardpicker/local_identify_printing_tags.py index 7ca6b1a8e..5de6f839a 100644 --- a/MPCAutofill/cardpicker/local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/local_identify_printing_tags.py @@ -42,6 +42,7 @@ Card, CardPrintingTag, CardTagVote, + CardTypes, PrintingTagStatus, VoteSource, ) @@ -157,7 +158,19 @@ def _eligible_base_queryset(anonymous_id: str, exclude_source_pks: Optional[Iter cardpicker.deductive_backfill's identical pattern), not already covered by the deductive backfill (which is provably exact by construction where it applies - this pilot's engines are weaker, lower-confidence signal and shouldn't pile onto a card that already has a - stronger deduction), and no resolved custom-art/non-english tag. + stronger deduction), no resolved custom-art/non-english tag, and card_type=CARD only. + + Tokens (and cardbacks) are excluded (2026-07-16, diagnosed live): a token's printed + collector line reads its PARENT set's code (e.g. "MM3"), while its CanonicalCard + candidates use token-specific set codes (e.g. "tm3c") that never match - a structural + mismatch, not a fixable parsing bug. Combined with item 1's coverage-gap ordering (generic + multi-set token names like "Beast" have huge candidate counts and near-zero coverage, so + they score maximally on "descending uncovered count"), this was front-loading an + essentially-0%-matchable cohort to the very front of every selection - confirmed live by + sampling real OCR output against real candidates for the first 8 selected cards in a real + 250-card window, all 8 of which were "Beast" tokens. Future work (not built): a + token-aware path using Scryfall's own token detection (`layout=token`/similar) to search + collector info or the set ICON instead of the parent-set text tokens don't reliably print. Does NOT apply the resolution floor (see select_candidates/count_below_resolution_floor, which layer opposite conditions on top of this shared base so the "how many did the floor @@ -168,7 +181,9 @@ def _eligible_base_queryset(anonymous_id: str, exclude_source_pks: Optional[Iter --exclude-sources-ocr/--exclude-sources-phash flags. """ queryset = ( - Card.objects.filter(printing_tag_status=PrintingTagStatus.UNRESOLVED, canonical_card__isnull=True) + Card.objects.filter( + printing_tag_status=PrintingTagStatus.UNRESOLVED, canonical_card__isnull=True, card_type=CardTypes.CARD + ) .exclude(printing_tags__anonymous_id=anonymous_id) .exclude(printing_tags__anonymous_id=DEDUCTIVE_BACKFILL_ANONYMOUS_ID) .exclude(tags__contains=[EXCLUDED_RESOLVED_TAGS[0]]) diff --git a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py index c491e20e3..025d50caf 100644 --- a/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py +++ b/MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py @@ -41,6 +41,7 @@ from cardpicker.models import ( CardPrintingTag, CardTagVote, + CardTypes, PrintingTagStatus, VotePolarity, VoteSource, @@ -97,6 +98,21 @@ def test_includes_card_with_a_name_candidate(self, db): selected = select_candidates("ocr") assert [s.card.pk for s in selected] == [card.pk] + def test_excludes_tokens(self, db): + # 2026-07-16, diagnosed live: a token's printed collector line reads its PARENT set's + # code, while its CanonicalCard candidates use token-specific set codes that never + # match - structurally unmatchable, not a fixable parsing bug. Combined with item 1's + # "descending uncovered count" ordering, generic multi-set token names were being + # front-loaded to the very front of every real selection. + CanonicalCardFactory(name="Beast") + CardFactory(name="Beast", card_type=CardTypes.TOKEN) + assert select_candidates("ocr") == [] + + def test_excludes_cardbacks(self, db): + CanonicalCardFactory(name="Forest") + CardFactory(name="Forest", card_type=CardTypes.CARDBACK) + assert select_candidates("ocr") == [] + def test_excludes_card_with_existing_vote_from_this_engines_own_anonymous_id(self, db): printing = CanonicalCardFactory(name="Forest") card = CardFactory(name="Forest") From c936e82063b631d299b6790e6dadf91cfee43207 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Thu, 16 Jul 2026 02:01:19 +0000 Subject: [PATCH 22/23] Document token-exclusion fix + corrected post-resize soak comparison Real 2.24x speedup and healthy OCR yield (56/198, matching the original baseline) once tokens are correctly excluded from selection. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ --- docs/features/printing-tags.md | 46 ++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index 87b212568..c7a1e6bfc 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -1710,6 +1710,52 @@ doesn't give intra-run stability granularity (e.g. whether the first 50 cards pr different rate than the last 50) - a finer-grained timing pass would be needed for that specific claim, not done here. +### Token exclusion + post-resize soak comparison (2026-07-16) + +**A real correctness gap found and fixed before trusting any throughput number from this +window**: the first several 250-card soak-test runs at both pre- and post-resize core counts +showed 0/250 OCR votes - a stark regression from the original 94/300 baseline. Diagnosed live +by sampling real OCR output against real candidates for the first 8 selected cards: all 8 were +generic "Beast" tokens (source-uploaded images with `card_type=TOKEN`) with ~90 candidate +printings each across token-only sets. A token's printed collector line reads its PARENT set's +code (e.g. "MM3"), while its `CanonicalCard` candidates use token-specific set codes (e.g. +"tm3c") that never match - structural, not a parsing bug. Item 1's "descending uncovered count" +ordering was front-loading this near-0%-matchable cohort (huge candidate counts, near-zero +coverage) to the very front of every real selection. Fixed: `_eligible_base_queryset` now +filters to `card_type=CARD` only (excludes tokens and cardbacks) - confirmed via a fresh +eligible-pool count, 172,494 cards (the pilot's own real filtered count, not a naive +unfiltered query). Future work (not built): a token-aware matching path using Scryfall's own +token detection to search collector info or the set icon instead of the parent-set text tokens +don't reliably print. + +**Corrected before/after comparison**, same 250-card window, re-run after the fix - OCR yield +now healthy and consistent at both core counts (56/198 votes, 28.3%, matching the original +94/300 baseline): + +| config (pre-resize vs. post-resize core count) | wall-clock | processing-only rate | +| ---------------------------------------------- | ---------: | -------------------: | +| lower core count (`--workers 2`) | 456s | 2.051s/card | +| higher core count (`--workers 7`) | 230s | 0.914s/card | + +**Speedup: 2.24x** (up from an earlier token-contaminated measurement's 2.03x - the sequential +clustering pre-pass, still unparallelized, see below, is a smaller share of a longer, more +representative run). Full-catalog re-projection (172,494 eligible pool): + +| | raw (naive) | cluster-dedup-adjusted (incl. ~21.6h sequential clustering pre-pass) | +| ----------------- | ------------: | -------------------------------------------------------------------: | +| lower core count | 4.09 days | 4.14 days | +| higher core count | **1.82 days** | **2.34 days** | + +The sequential clustering pre-pass (`compute_own_image_clusters`, confirmed via code inspection +AND a live `docker stats` capture showing a single-core-only ~100% CPU plateau during that +phase) is ~21.6h fixed regardless of core count - ~38% of total time at the higher core count. +Parallelizing that one loop (flagged as a follow-up, not built) remains the highest-leverage +lever to push below ~2.3 days. Verified via direct `docker stats` sampling (not just aggregate +system `top`) that the compute phase itself DOES achieve real multi-core parallelism (a peak of +~625% CPU observed, consistent with most of a 7-worker pool active simultaneously) once past the +pre-pass - an earlier read of aggregate `top` data alone had incorrectly suggested a GIL-bound +compute phase; the direct per-process measurement corrected that. + ### No-match autopsy (2026-07-15, post-merge Hold #1 of the pre-scale program) Classified all 176 OCR "parsed-but-no-match" cases from the pilot run From 9748aeb2614538e6343237121191d768a27a8f23 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Thu, 16 Jul 2026 02:20:58 +0000 Subject: [PATCH 23/23] HOLD #2: full package report Synthesizes infra prerequisites, corrected throughput, cluster/coverage census, Track 4 status, git/branch audit, and the updated scaling recommendation (single continuous run, not chunked slices, given the now-real ~1.8-2.3 day runtime). Awaiting owner go-ahead for the full-catalog run. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ --- docs/features/printing-tags.md | 112 +++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index c7a1e6bfc..404750df5 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -1930,3 +1930,115 @@ implied by this OCR fix and was not built. with `cardpicker.deductive_backfill`'s deterministic tiers (D1/D2), not Stage 8's visual-disambiguation engines — explicitly not the "D2.5 arriving for free" the autopsy's cross-check ruled out. + +## HOLD #2: full package report (2026-07-16) + +Synthesizing deliverable gating full-catalog run authorization. Everything below is either +already-linked from earlier in this doc or newly summarized here; nothing in this section is a +new claim not otherwise sourced above. + +**Infrastructure prerequisites - all landed:** + +- Rate limiter (PR #25): merged to master. Deploy confirmed live via direct requests against + `cdn.proxyprints.ca`'s full tier (4 real requests, all HTTP 200, 0.46-1.67s latency, no 429s) + - the underlying fetch path PDF export and bulk download both depend on is healthy + post-merge. (CI's "Publish image CDN" job shows red on every run, before and after this + merge - a separate, pre-existing, unrelated failure in a `thumbnail-refresh` Cloudflare + Workflow trigger, not the image-serving route itself; logged as its own follow-up, task + #111, not a gate.) +- Tesseract dockerized (`docker/django/Dockerfile`'s shared `builder` stage) - verified + end-to-end via a real `--dry-run --limit 3` inside the rebuilt container. Host venv retired. +- Container boot-recovery hardened: `restart: unless-stopped` on all 5 services plus a + `mpcautofill-docker-compose.service` systemd unit as belt-and-suspenders - verified with a + real `sudo reboot`, not simulated (all containers back up unattended within minutes, site + returned 200 on both domains). +- Batch-flush checkpointing (item 2): a kill loses at most one `--batch-size` (default 25) + worth of unflushed work; a plain re-invocation resumes cleanly via the existing idempotent + selection query. Verified with a simulated-kill test. + +**Throughput, real and corrected:** + +A real correctness gap was found and fixed before trusting any number from this window: the +first several soak-test runs showed 0/250 OCR votes (vs. an original 94/300 baseline) - traced +to generic multi-set token names (e.g. "Beast", ~90 candidates each, essentially 0% coverage) +being front-loaded by item 1's coverage-gap ordering into a cohort that's structurally +unmatchable by OCR (a token's printed collector line reads its parent set's code; its DB +candidates use token-specific codes that never match). Fixed by excluding `card_type=TOKEN`/ +`CARDBACK` from selection. Post-fix, OCR yield is healthy and consistent (56/198 votes, 28.3%, +matching the original baseline) at every core count tested. + +Corrected same-window (250-card) before/after comparison, real `docker stats`-verified multi-core +parallelism (not just inferred from noisy aggregate `top`): + +| core count | wall-clock | processing-only rate | +| ---------- | ---------: | -------------------: | +| lower | 456s | 2.051s/card | +| higher | 230s | 0.914s/card | + +**Speedup: 2.24x.** Full-catalog re-projection (172,494 eligible pool, freshly counted with the +token/cardback fix applied): + +| | raw (naive) | cluster-dedup-adjusted | +| ----------------- | ------------: | ---------------------: | +| lower core count | 4.09 days | 4.14 days | +| higher core count | **1.82 days** | **2.34 days** | + +The instance is now running at its higher core count as the standing configuration (not a +temporary state for this measurement alone) - not reverting to the lower count, though not +treated as permanently fixed either; revisit whenever convenient, no urgency either way. + +**Cluster + coverage census:** clustering (item 2a) absorbed ~21% of selected candidates into +representatives in the corrected (token-excluded) sample - down from an earlier ~26-28% observed +in the token-contaminated sample, consistent with tokens/generic images being more prone to +visual duplication. The sequential clustering pre-pass (`compute_own_image_clusters`, confirmed +via code inspection and a live `docker stats` capture showing a single-core ~100% CPU plateau +during that phase specifically) is ~21.6h fixed regardless of core count - ~38% of total time at +the higher core count. Logged as task #108, held as an available future optimization, not built +now - the current projection is already a good number for a background job. + +**Track 4 (pilot-quality items):** + +- Bleed tag: negative-only voting shipped (item 4b) - votes only on a detected `trimmed` + reading, absence of any vote is the documented convention for "presumed normal bleed" (updated + in both this doc and `sensitive_tags.py`'s own comment, which previously documented the + opposite pre-pilot convention). The existing-tag check (`Tag.objects.filter(...).first()`, + degrades to no vote if the tag isn't seeded) was already in place before this change - no new + tag seeded, matching the "wait for owner ok" instruction by construction. The underlying + aspect-ratio classification itself was validated against a real 40-source diverse sample + (Bleed-edge tagging section above) - the negative-only voting change is a polarity/gating + change on top of that already-validated classification, not a new detection algorithm needing + its own separate validation pass. +- DPI-tag audit (item 8, report only): 99.97% of the catalog already at 300+dpi - not a useful + prioritization signal on its own. `low-res` SENSITIVE tag has never been used in production + (0 resolved, 0 pending) - stays untouched, human-judgment/moderation-gated as designed. Both + tag stores checked (`Card.tags` resolved/baked array and `CardTagVote` raw votes). + +**Git/branch audit:** clean. This session's branch (`worktree-pilot-prescale`, PR #24) is +in sync with origin, mergeable. PR #25 merged (rate limiter). PR #20 (unrelated frontend fix) +merged at the owner's request, reviewed and confirmed by the owner before merging. PR #19 +(unrelated docs-only Playwright-flake note) remains open with a trivial, keep-both `docs/lessons.md` +conflict against master - not a dependency of anything in this program, disposition left to the +owner's convenience. Several other worktree branches exist but are either already merged or have +zero unique diff against master (content already landed via a different commit path) - no lost +work found anywhere in the audit. + +**Scaling recommendation, updated for the shorter true runtime:** the original Option A +(screen'd process) vs. Option B (django-q nightly slices) decision assumed a ~7-day run, +where crash-recovery and unattended multi-night scheduling mattered enough to weigh a full +scheduler infrastructure investment. At the now-real ~1.8-2.3 day full-catalog runtime, **a +single continuous run is the right shape - chunked nightly slicing is not needed.** Item 2's +own batch-flush checkpointing already provides crash-resilience within that single run (a kill +loses at most one batch, a plain re-invocation resumes cleanly), which is the main protection +django-q's scheduler infrastructure would otherwise buy - not worth the added complexity for a +run this short. Execution is via the now-dockerized image (`docker compose run`, matching every +verification run this session), not a host venv - a `screen`/`tmux`-wrapped single invocation is +sufficient; no new infrastructure to build. + +**Open, non-blocking items** (logged, not gates): item 2b (persist `content_hash` for +federation, deferred), item 5 (questionFeed ordering mirror, separate follow-up PR), task #108 +(parallelize the clustering pre-pass), task #109's future-work note (token-aware matching via +Scryfall's own token detection), task #111 (unrelated CI noise in the thumbnail-refresh +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.