Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 73 additions & 16 deletions MPCAutofill/cardpicker/image_evidence.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@
)
from cardpicker.local_phash import ART_CROP_BOX
from cardpicker.models import Card, CardScanLog, ImageEvidence
from cardpicker.modern_artist_credit import LexiconIndex, recognize_artist_credit
from cardpicker.utils import twos_complement

logger = logging.getLogger(__name__)
Expand All @@ -289,16 +290,22 @@
LAYOUT_CLASS_EXTRACTOR_VERSION = "layout-class-v1"
CROP_COORDINATES_EXTRACTOR_VERSION = "crop-coordinates-v1"
# v1 -> v2 (issue #480's combined pass, THE FLIP: settings.OCR_ENGINE default -> "tesserocr" -
# see settings.py's own comment). These three extractors are the ones whose STORED VALUE actually
# depends on which OCR engine produced it (they all read through `local_ocr.
# run_tesseract_text_and_words`, which now runs on tesserocr's differently-compiled tesseract/
# leptonica build by default - issue #423's spike already found byte-identical output is
# structurally unreachable given that vendored-build mismatch). Bumping the version is what makes
# every existing row under the OLD tag stale, so the next pass re-extracts it under the new
# engine rather than silently mixing two engines' output under one provenance label (issue #480's
# correction comment: "Engine swap WITHOUT a version bump is forbidden").
# see settings.py's own comment). These extractors' STORED VALUE actually depends on which OCR
# engine produced it (they read through `local_ocr.run_tesseract_text_and_words`, which now runs
# on tesserocr's differently-compiled tesseract/leptonica build by default - issue #423's spike
# already found byte-identical output is structurally unreachable given that vendored-build
# mismatch). Bumping the version is what makes every existing row under the OLD tag stale, so the
# next pass re-extracts it under the new engine rather than silently mixing two engines' output
# under one provenance label (issue #480's correction comment: "Engine swap WITHOUT a version
# bump is forbidden").
COLLECTOR_LINE_OCR_EXTRACTOR_VERSION = "collector-line-ocr-v2"
ARTIST_OCR_EXTRACTOR_VERSION = "artist-ocr-v2"
# v2 -> v3 (PR #685): wires modern_artist_credit.recognize_artist_credit in as a third artist
# fallback inside compute_card_evidence, recovering a name for ~19,478 cards that previously
# stored none. The extractor's own OCR pass is unchanged from v2 (still issue #480's tesserocr
# engine) - this bump exists solely so Stage C's version-aware resume filter
# (MANIFEST_EXTRACTOR_CURRENT_VERSIONS) selects the whole catalogue for one re-extraction pass
# under the new fallback, per issue #509's stale-value comparison.
ARTIST_OCR_EXTRACTOR_VERSION = "artist-ocr-v3"
COLLECTOR_LINE_TSV_EXTRACTOR_VERSION = "collector-line-tsv-v2"
# NOT bumped: symbol_region is a raw phash of a crop region (imagehash, no tesseract call at all)
# - engine-independent by construction.
Expand Down Expand Up @@ -651,6 +658,7 @@ def fetch_and_compute_card_evidence_for_tests(
artist_lexicon: Optional[ArtistLexicon] = None,
printing_artist_lookup: Optional[Callable[[Optional[str], Optional[str]], Optional[str]]] = None,
name_artist_lookup: Optional[Callable[[str], tuple[str, ...]]] = None,
modern_artist_lexicon: Optional[LexiconIndex] = None,
) -> ExtractionResult:
"""
A TEST-ONLY CONVENIENCE WRAPPER. It has no production caller and has not had one since the
Expand Down Expand Up @@ -720,6 +728,10 @@ def fetch_and_compute_card_evidence_for_tests(
picklable `ProcessPoolExecutor` entrypoint, and a resolver holding a 113k-row index is not
something to send across that boundary. `None` (the default) means no narrowing - every
pre-2026-07-29 caller's behaviour, unchanged.

`modern_artist_lexicon`, if given, is forwarded straight through to `compute_card_evidence`
below - see that function's own docstring for the ARTIST-CROP FALLBACK it controls. `None`
(the default) leaves it off, behavior identical to every caller that predates it.
"""

fetch_started_at = time.monotonic()
Expand All @@ -743,6 +755,7 @@ def fetch_and_compute_card_evidence_for_tests(
artist_lexicon=artist_lexicon,
printing_artist_lookup=printing_artist_lookup,
card_artist_names=() if name_artist_lookup is None else name_artist_lookup(card.name),
modern_artist_lexicon=modern_artist_lexicon,
md5_checksum=card.md5_checksum,
sha256_checksum=card.sha256_checksum,
)
Expand All @@ -763,6 +776,7 @@ def compute_card_evidence(
artist_lexicon: Optional[ArtistLexicon] = None,
printing_artist_lookup: Optional[Callable[[Optional[str], Optional[str]], Optional[str]]] = None,
card_artist_names: tuple[str, ...] = (),
modern_artist_lexicon: Optional[LexiconIndex] = None,
md5_checksum: Optional[str] = None,
sha256_checksum: Optional[str] = None,
) -> ExtractionResult:
Expand Down Expand Up @@ -921,6 +935,39 @@ def compute_card_evidence(
`local_identify_printing_tags.CandidateNameIndex.candidates_for` - the codebase's existing
normaliser, not a new one. Empty (the default) means no narrowing.

`modern_artist_lexicon` (2026-08-04): the ARTIST-CROP FALLBACK - a third, independent source
for the `artist_ocr_name` storage fallback above, tried only after BOTH the "Illus." anchor
AND the collector/legal-line recovery have found nothing storable.

THE GAP THIS CLOSES. `recover_artist_from_card_text` reads only `collector_line_raw_text` and
`legal_line_raw_text` - the bottom PRINT ROW. A real, measured population of cards (old-border
proxies with a centred "Illus. <name>" credit and no collector line/set code/copyright row
printed at all) has NO text in either of those two fields, so that recovery is structurally
blind to them even when the anchor regex itself missed the credit to ordinary OCR noise
(`_ILLUS_RE` requires literal "llus", and a misread like "Soot Itus." - real production text,
card evidence id 221268 - never matches it). The ONE stored string that DOES carry the credit
on these cards is `artist_ocr_raw_text` - the artist-crop OCR text this same extractor already
computed a few lines above - and until now nothing in the live extraction path ever re-read it.

`cardpicker.modern_artist_credit.recognize_artist_credit` already exists for exactly this
shape of input (its own module docstring: "a wholly independent, parse-only re-reader of the
SAME already-stored `artist_ocr_raw_text` strings") and was previously reachable only through
its own standalone `backfill_modern_artist_names` command, never during real extraction - so a
freshly-extracted or re-extracted row could go on carrying the same gap forever. Verified
against three real evidence rows this way blank in production (ids 221241, 221268, 221274):
`recognize_artist_credit` recovers "Sebastian Giacobino"/"Aaron Miller"/"Andrey Kuzinskiy" from
their stored `artist_ocr_raw_text` at ratio 1.0 with a comfortable margin over the runner-up,
using the real ~2.5k-name production lexicon - not a toy fixture.

ORDER: tried strictly AFTER `recover_artist_from_card_text` returns nothing storable, never
instead of it or in competition with it - the print-row read is format-anchored (a fixed
layout, narrowed by `card_artist_names`) and stays authoritative when it succeeds; this is
purely an ADDITIONAL fallback for the population it can never reach. `None` (the default, and
every pre-2026-08-04 caller) leaves this off entirely - byte-identical to before. No
extractor_versions key is added and no version bumped, deliberately, for the same reason
`artist_lexicon` above isn't: either would invalidate every existing row and force a full
catalog re-extraction to recover text this repository already has.

`md5_checksum`/`sha256_checksum` (2026-07-25, issue #473 PR-2, folded with issue #472): the
calling card's own live `Card.md5_checksum`/`Card.sha256_checksum` at the moment of THIS real
extraction pass, stamped verbatim onto the result's `fields` (so
Expand Down Expand Up @@ -1251,18 +1298,28 @@ def compute_card_evidence(
# key is added and no version bumped, deliberately - either would invalidate every
# existing row against `run_image_evidence_cohort.MANIFEST_EXTRACTOR_CURRENT_VERSIONS` and
# force a full 220k-card Stage C re-extraction.
if artist_name is None and artist_lexicon is not None:
recovered_artist = recover_artist_from_card_text(
fields["collector_line_raw_text"],
legal_line_raw_text,
artist_lexicon,
allowed_artist_names=card_artist_names,
)
if artist_name is None:
recovered_artist = None
if artist_lexicon is not None:
recovered_artist = recover_artist_from_card_text(
fields["collector_line_raw_text"],
legal_line_raw_text,
artist_lexicon,
allowed_artist_names=card_artist_names,
)
if recovered_artist is not None and recovered_artist.canonical_name is not None:
# `canonical_name` is a verbatim `CanonicalArtist.name` and is `None` unless the
# reading is compatible with exactly one of them - fuzzy MATCHING is permitted,
# fuzzy STORAGE is not (owner ruling, 2026-07-29).
fields["artist_ocr_name"] = recovered_artist.canonical_name
elif modern_artist_lexicon is not None:
# ARTIST-CROP FALLBACK (2026-08-04) - see this function's own `modern_artist_
# lexicon` docstring paragraph. Re-reads `artist_raw_text` (the artist-crop OCR
# text this extractor already computed above), the one stored string the
# collector/legal-line recovery above structurally cannot reach.
recognized_artist = recognize_artist_credit(artist_raw_text, modern_artist_lexicon)
if recognized_artist is not None:
fields["artist_ocr_name"] = recognized_artist.matched_name

extractor_versions["collector_line_ocr"] = COLLECTOR_LINE_OCR_EXTRACTOR_VERSION
extractor_versions["artist_ocr"] = ARTIST_OCR_EXTRACTOR_VERSION
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@
from cardpicker.harvest_fetch_limiter import GoogleFetchLockoutError
from cardpicker.local_calculate_verdicts import known_set_codes
from cardpicker.models import CanonicalCard, Card, ImageEvidence, PilotRunLedger
from cardpicker.modern_artist_credit import LexiconIndex, load_lexicon_index
from cardpicker.pilot_run_lifecycle import (
add_dry_run_guard_arguments,
enforce_dry_run_precondition,
Expand Down Expand Up @@ -329,7 +330,7 @@
"layout_class": "layout-class-v1",
"crop_coordinates": "crop-coordinates-v1",
"collector_line_ocr": "collector-line-ocr-v2",
"artist_ocr": "artist-ocr-v2",
"artist_ocr": "artist-ocr-v3",
"collector_line_tsv": "collector-line-tsv-v2",
"artbox_phash": "artbox-phash-v1",
"symbol_region": "symbol-region-v1",
Expand Down Expand Up @@ -410,9 +411,13 @@ def already_extracted_card_ids(run_id: str, only_never_extracted: bool = False)
# only production constructor) always passes it.
_WORKER_ARTIST_LEXICON: Optional[ArtistLexicon] = None
_WORKER_PRINTING_ARTIST_LOOKUP: Optional[PrintingArtistLookup] = None
_WORKER_MODERN_ARTIST_LEXICON: Optional[LexiconIndex] = None


def _init_worker(artist_lexicon: Optional[ArtistLexicon] = None) -> None:
def _init_worker(
artist_lexicon: Optional[ArtistLexicon] = None,
modern_artist_lexicon: Optional[LexiconIndex] = None,
) -> None:
"""Compute pool `initializer=` - runs once per worker PROCESS, immediately after it starts
(fork on Linux), before that worker executes its first task. Three jobs (the rate-limiter
descaling job is gone entirely under the decoupled design - see module docstring):
Expand All @@ -423,6 +428,10 @@ def _init_worker(artist_lexicon: Optional[ArtistLexicon] = None) -> None:
ARTIST WIRING section for why this one travels by initializer while `known_set_codes` travels
per submit, and why `printing_artist_lookup` is built here rather than shipped from the
parent.

`modern_artist_lexicon` (2026-08-04) is `handle()`'s own `load_lexicon_index()` result,
delivered the same way for the same reason - see `compute_card_evidence`'s own
`modern_artist_lexicon` docstring paragraph for what it feeds, the ARTIST-CROP FALLBACK.
"""
# 1. tesseract's LSTM engine can multi-thread itself internally via OpenMP - without this, N
# worker PROCESSES (not just N threads within one process) would each ALSO spread across
Expand All @@ -442,9 +451,10 @@ def _init_worker(artist_lexicon: Optional[ArtistLexicon] = None) -> None:
# can only ever open a connection this worker owns. Skipped entirely when no lexicon was
# supplied - a `PrintingArtistLookup` with no lexicon alongside it can never be consulted
# (`_parse_artist_is_contradicted` requires both), so building one would be pure waste.
global _WORKER_ARTIST_LEXICON, _WORKER_PRINTING_ARTIST_LOOKUP
global _WORKER_ARTIST_LEXICON, _WORKER_PRINTING_ARTIST_LOOKUP, _WORKER_MODERN_ARTIST_LEXICON
_WORKER_ARTIST_LEXICON = artist_lexicon
_WORKER_PRINTING_ARTIST_LOOKUP = None if artist_lexicon is None else build_printing_artist_lookup()
_WORKER_MODERN_ARTIST_LEXICON = modern_artist_lexicon


def _get_rss_mb() -> Optional[float]:
Expand Down Expand Up @@ -645,13 +655,14 @@ def _compute_one_card(
list stays picklable exactly as `compute_card_evidence`'s own docstring requires. `()` (the
default) means "don't narrow", byte-identical to the pre-2026-07-29 behaviour.

THE OTHER TWO ARTIST INPUTS ARE PROCESS STATE, NOT ARGUMENTS. `artist_lexicon` and
`printing_artist_lookup` are read off `_WORKER_ARTIST_LEXICON`/`_WORKER_PRINTING_ARTIST_LOOKUP`,
THE OTHER ARTIST INPUTS ARE PROCESS STATE, NOT ARGUMENTS. `artist_lexicon`,
`printing_artist_lookup`, and (2026-08-04) `modern_artist_lexicon` are read off
`_WORKER_ARTIST_LEXICON`/`_WORKER_PRINTING_ARTIST_LOOKUP`/`_WORKER_MODERN_ARTIST_LEXICON`,
which `_init_worker` set once when this worker process started - see the module docstring for
why each travels the way it does. Both being `None` (an un-initialised process: a direct unit
call, a pool built without the initializer) disables the escalation gate and the
`artist_ocr_name` storage fallback entirely, which is exactly what this command did before
2026-07-29 - never an error, and never a partially-wired read."""
why each travels the way it does. All being `None` (an un-initialised process: a direct unit
call, a pool built without the initializer) disables the escalation gate and both
`artist_ocr_name` storage fallbacks entirely, which is exactly what this command did before
2026-07-29/2026-08-04 - never an error, and never a partially-wired read."""
from cardpicker.image_evidence import compute_card_evidence, persist_evidence

wall_started_at = time.monotonic() if profile else None
Expand All @@ -676,6 +687,7 @@ def _compute_one_card(
artist_lexicon=_WORKER_ARTIST_LEXICON,
printing_artist_lookup=_WORKER_PRINTING_ARTIST_LOOKUP,
card_artist_names=card_artist_names,
modern_artist_lexicon=_WORKER_MODERN_ARTIST_LEXICON,
md5_checksum=md5_checksum,
sha256_checksum=sha256_checksum,
)
Expand Down Expand Up @@ -801,6 +813,7 @@ def _run_cohort(
known_set_codes: Optional[frozenset[str]] = None,
artist_lexicon: Optional[ArtistLexicon] = None,
name_artist_lookup: Optional[NameArtistLookup] = None,
modern_artist_lexicon: Optional[LexiconIndex] = None,
) -> tuple[int, int, bool, int, bool, Optional[float]]:
"""
The decoupled fetch/compute driver itself. Two concurrent executors:
Expand Down Expand Up @@ -854,6 +867,10 @@ def _run_cohort(
`None` (the default, and every test that doesn't thread them) leaves the artist gate and the
`artist_ocr_name` storage fallback off, exactly as before 2026-07-29.

`modern_artist_lexicon` (2026-08-04): built ONCE by `handle()` below and handed to the compute
pool's `initializer=` alongside `artist_lexicon` - same lifetime, same route. `None` (the
default) leaves the ARTIST-CROP FALLBACK off - see `compute_card_evidence`'s own docstring.

Returns `(completed, fetch_failures, lockout_hit, short_circuited, rss_limit_hit, peak_rss_mb)`
- the same three figures the old single-loop design printed in its final summary line, plus the
short-circuit counter (item 1's own "count it during the real run" ask), the RSS-limit flag, and
Expand All @@ -865,7 +882,7 @@ def _run_cohort(
stats = _CohortStats(total=len(cohort_ids), stdout_write=stdout_write, stop_event=stop_event, max_rss_mb=max_rss_mb)

with ThreadPoolExecutor(max_workers=fetch_threads) as fetch_pool, ProcessPoolExecutor(
max_workers=workers, initializer=_init_worker, initargs=(artist_lexicon,)
max_workers=workers, initializer=_init_worker, initargs=(artist_lexicon, modern_artist_lexicon)
) as compute_pool:
cohort_iter = iter(cohort_ids)
outstanding_fetch: "set[Future[Any]]" = set()
Expand Down Expand Up @@ -1280,6 +1297,10 @@ def priority_key(pair: tuple[int, str]) -> tuple[float, int]:
# `run_join_key_calculator` already practise.
artist_lexicon = load_artist_lexicon()
name_artist_lookup = build_name_artist_lookup()
# ARTIST-CROP FALLBACK (2026-08-04): same "query once, pass through explicitly"
# convention as the two lookups immediately above - see `compute_card_evidence`'s own
# `modern_artist_lexicon` docstring paragraph.
modern_artist_lexicon = load_lexicon_index()

# Close the parent's own DB connection(s) before forking the compute pool - belt-and-
# braces alongside each compute worker's own _init_worker close_all() call, so the
Expand Down Expand Up @@ -1307,6 +1328,7 @@ def priority_key(pair: tuple[int, str]) -> tuple[float, int]:
known_set_codes=lexicon,
artist_lexicon=artist_lexicon,
name_artist_lookup=name_artist_lookup,
modern_artist_lexicon=modern_artist_lexicon,
)
finally:
if profile_file is not None:
Expand Down
7 changes: 7 additions & 0 deletions MPCAutofill/cardpicker/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,13 @@ def serialise_as_printing_candidate(self) -> PrintingCandidate:
illustrationId=(
str(metadata.illustration_id) if metadata is not None and metadata.illustration_id else None
),
# Scryfall's art-crop image URL for this printing - see
# CanonicalPrintingMetadata.art_crop_url's own docstring for provenance. Null-tolerant
# for the same two reasons illustrationId is above: metadata can be absent entirely,
# and art_crop_url is `blank=True` so a printing can legitimately carry an empty
# value - both collapse to None here so the frontend can fall back to
# mediumThumbnailUrl rather than render a broken image.
artCropUrl=metadata.art_crop_url if metadata is not None and metadata.art_crop_url else None,
)


Expand Down
Loading
Loading