Skip to content

fix(image_evidence): wire the artist-crop fallback into live extraction - #685

Merged
WilfordGrimley merged 6 commits into
masterfrom
fix-artist-ocr-crop-fallback
Aug 5, 2026
Merged

fix(image_evidence): wire the artist-crop fallback into live extraction#685
WilfordGrimley merged 6 commits into
masterfrom
fix-artist-ocr-crop-fallback

Conversation

@WilfordGrimley

@WilfordGrimley WilfordGrimley commented Aug 4, 2026

Copy link
Copy Markdown

Summary

artist_ocr_name is blank on 87,371 evidence rows that carry non-blank
artist_ocr_raw_text. Root cause: the "Illus." anchor (local_fallback. extract_artist_name) misses OCR-noisy reads (e.g. Soot Itus. never
matches _ILLUS_RE's literal llus), and the 2026-07-29 collector-line
recovery (collector_line_artist.recover_artist_from_card_text) only reads
collector_line_raw_text/legal_line_raw_text — the bottom print row — so
it is structurally blind to old-border proxies whose only on-card credit is
a centred "Illus. <name>" line with no collector line/set code/copyright
row printed at all.

cardpicker.modern_artist_credit.recognize_artist_credit already exists for
exactly this shape of input (a parse-only re-reader of the same stored
artist_ocr_raw_text), but was only ever reachable through its own
standalone backfill_modern_artist_names command, never during real
extraction — so a freshly (re-)extracted row could carry the same gap
forever. This PR wires it into compute_card_evidence as a third fallback
(modern_artist_lexicon, new optional param, default None), tried only
after both the anchor and the collector/legal-line recovery find nothing
storable, and threads it through both live Stage C entry points
(stage_e_dispatch.py's per-worker-process singleton, and
run_image_evidence_cohort.py's initargs). No extractor_versions bump
— same "purely additive, no forced re-extraction" posture the 2026-07-29
feature already established.

Verified against three real, currently-blank production evidence rows (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 against the real ~2.5k-name lexicon. A
fourth reproduction row (id 221289, "David Rapoza") turned out to be a
correct abstention, not a bug: the real lexicon holds both "Dave
Rapoza" and "David Rapoza" as distinct CanonicalArtist rows, so the
reading is genuinely ambiguous under the 2026-07-29 owner ruling (fuzzy
matching yes, fuzzy storage no) — confirmed by direct measurement against
the live lexicon, not assumed.

Full-population measurement of the 87,371-row split (read-only, against
production, no writes):

  • 19,478 (22.3%) genuinely recoverable via the artist-crop fallback
    this PR adds
  • 329 (0.4%) genuinely ambiguous — correctly abstained, same shape as
    the "Dave"/"David" Rapoza case above
  • 67,564 (77.3%) carry no artist credit findable by any means —
    consistent with this catalog's known proxy population rendered without
    standard bottom-row metadata (the same finding the collector-number side
    of this problem already established)

Landing this code fix does not retroactively fill the 19,478 already-
extracted rows — they were computed before this fallback existed and the
extractor version is deliberately unchanged, so nothing re-triggers their
extraction. Backfilling them is a separate, already-existing tool
(backfill_modern_artist_names, unmodified here) that an owner can choose
to run; this PR does not run it (out of scope: "Launching any catalog
pass").

Test plan

  • pytest cardpicker/tests/test_image_evidence.py — 115 passed
    (individually)
  • pytest cardpicker/tests/test_image_evidence.py cardpicker/tests/test_collector_line_artist.py cardpicker/tests/test_modern_artist_credit.py cardpicker/tests/test_backfill_collector_line_artist.py cardpicker/tests/test_stage_e_dispatch.py cardpicker/tests/test_run_image_evidence_cohort.py — 364 passed
    (together, per issue Test suite is order-dependent: leaked fetch-failure window trips the envelope across files (8 failures on master) #679's order-dependency note)
  • New test class TestExtractCardEvidenceArtistCropFallback in
    test_image_evidence.py: recovers a name the anchor and print-row
    recovery both miss; the gap stays open when modern_artist_lexicon is
    omitted (backward-compat control); the "Illus." anchor still wins when it
    fires; the print-row recovery still wins over this fallback when both
    would resolve (precedence)
  • python -m py_compile on all three touched production modules
  • mypy scoped to the touched files — zero errors attributable to this
    change (the whole-project run surfaces ~294 pre-existing baseline errors
    in unrelated files, none touched here)
  • Reproduced all four brief cases directly against the live production
    DB/lexicon (read-only) before writing the fix, and again after, to
    confirm the fix's actual effect
  • lsp_diagnostics — attempted once, timed out (known best-effort
    limitation on this box); not retried, per instructions

Task-end checks (CLAUDE.md)

  • Docs convention (edit-in-place, no dated sections): no docs/ file
    documents this extractor's internals outside code docstrings, so none
    needed updating; no blocker exceeded 15 minutes, so no
    docs/troubleshooting.md entry needed.
  • Policy text dates: not applicable (no on-site policy text touched).
  • Wiki maintenance: not applicable — this changes neither what a user sees
    nor what an admin does; it's an internal Stage C extraction fallback.
  • Extractable-primitives ledger: not applicable — this touches
    vote/consensus-adjacent artist_ocr_name storage but adds no new
    extractable primitive and removes none.
  • Constant renames: none — no module-level constant was renamed, extracted,
    or retired.
  • Push policy: pushed to a dedicated branch (fix-artist-ocr-crop-fallback,
    cut fresh from origin/master) rather than master, and this PR is
    opened per this task's explicit deliverable instructions, not the
    solo-work default. Not merging — merges are owner-only.

Addendum: per-candidate art-crop URL (2026-08-04)

Summary

Adds PrintingCandidate.artCropUrl, closing the backend half of a gap the
question-feed-frontend-fixes branch (PR #687, not yet merged) documented:
illustration-clustered candidate tiles in the question feed have no shared
art crop to render in place of each candidate's own full-card scan, because
no per-candidate or per-illustration art-crop URL existed on the
questionFeed payload. No new fetch/harvest was needed — the data was
already on disk in CanonicalPrintingMetadata.art_crop_url (populated from
Scryfall's bulk-data dump, same field the item-level
scryfallIllustrationUrl artist question already reads).

Follows the illustrationId precedent added for the same reason: the field
is populated in CanonicalCard.serialise_as_printing_candidate
(models.py), added to PrintingCandidate in both schema_types.py
(backend) and schema_types.ts (frontend wire-contract type only — no
frontend consumer added), and is null-tolerant for both the no-sidecar and
the legitimately-empty (art_crop_url is blank=True) cases, collapsing
both to None/undefined so the frontend can fall back to
mediumThumbnailUrl.

Query efficiency finding: every code path that reaches
serialise_as_printing_candidate (both branches of
get_ranked_printing_candidates, and the ai_vote lookup in
question_feed._confirm_suggestion_item) already select_relateds
printing_metadata — added earlier for illustrationId's sake — so this
change needed no new prefetch wiring. A new test proves the candidate-grid
serialisation issues zero additional queries regardless of candidate count.

docs/features/printing-tags.md's Known gaps section gets a new bullet:
PR #687's own gap bullet lives only on that unmerged sibling branch, not in
this branch's history, so a fresh bullet was added here rather than editing
text that doesn't exist in this checkout.

Test plan (addendum)

  • pytest cardpicker/tests/test_serialise_as_printing_candidate.py — 6
    passed (individually)
  • pytest cardpicker/tests/test_printing_candidates.py — 12 passed
    (individually)
  • pytest cardpicker/tests/test_question_feed.py — 46 passed
    (individually)
  • All three together (issue Test suite is order-dependent: leaked fetch-failure window trips the envelope across files (8 failures on master) #679 order-dependency check) — 64 passed,
    matching the individual sum exactly
  • New coverage: artCropUrl populated from the sidecar; degrades to
    None for both the no-sidecar and the empty-art_crop_url cases; a
    django_assert_num_queries(0) test proving flat query cost serialising a
    10-candidate grid
  • python -m py_compile on all four touched Python modules
  • mypy scoped to touched files — zero errors on any touched line
    (schema_types.py carries # mypy: ignore-errors; models.py's 46
    baseline errors are all outside the touched 139-176 range)
  • npx tsc --noEmit — clean (required a fresh npm ci in this
    worktree first; node_modules didn't exist here yet)
  • pre-commit run on all six touched files — all hooks green
  • lsp_diagnostics — attempted once, timed out; not retried, per
    instructions (compile+mypy+tsc+tests substitute)

Task-end checks (addendum)

  • Docs convention: edited docs/features/printing-tags.md's existing Known
    gaps section in place (new bullet, not a dated append) — see the
    provenance note above for why it's a new bullet rather than an edit to
    PR fix(question-feed): direct-access Yes/No chips, narrowing after no-match, art-crop gap documented #687's.
  • Wiki maintenance: not applicable — no user-visible or admin-visible
    behavior changed; the frontend does not consume this field yet.
  • Extractable-primitives ledger: not applicable — no new primitive
    independent of the vote/consensus system was created.
  • Constant renames: none.
  • Push policy: pushed directly to this existing branch
    (fix-artist-ocr-crop-fallback), extending this same open PR, per this
    addendum's explicit brief.

artist_ocr_name stays blank on 87,371 evidence rows that carry non-blank
artist_ocr_raw_text - the "Illus." anchor misses OCR-noisy reads (e.g.
"Soot Itus." never matches _ILLUS_RE), and the 2026-07-29 collector-line
recovery (recover_artist_from_card_text) only reads collector_line_raw_text/
legal_line_raw_text, so it is structurally blind to old-border proxies whose
only credit is a centred "Illus. <name>" line with no bottom print row at
all.

cardpicker.modern_artist_credit.recognize_artist_credit already exists for
exactly this shape of input (re-reading artist_ocr_raw_text) but was only
ever reachable through its own standalone backfill_modern_artist_names
command, never during real extraction. This wires it into
compute_card_evidence as a third fallback, tried only after both the anchor
and the collector/legal-line recovery find nothing storable - strictly
additive (new modern_artist_lexicon param, default None, no
extractor_versions bump), and threaded through both live Stage C entry
points (stage_e_dispatch.py, run_image_evidence_cohort.py).

Verified against real production evidence rows (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
against the real ~2.5k-name lexicon. A fourth reproduction row (id 221289,
"David Rapoza") turned out to be a correct abstention, not a bug: the real
lexicon holds both "Dave Rapoza" and "David Rapoza" as distinct canonical
artists, so the reading is genuinely ambiguous under the 2026-07-29 ruling
(fuzzy matching yes, fuzzy storage no) - confirmed by direct measurement,
not assumed.

Full-population measurement of the 87,371-row split: 19,478 (22.3%)
genuinely recoverable via the artist-crop fallback, 329 (0.4%) genuinely
ambiguous (correctly abstained), 67,564 (77.3%) carry no artist credit
findable by any means - consistent with this catalog's known proxy
population rendered without standard bottom-row metadata.
…rd_evidence stubs

test_stage_e_shakedown.py and test_stream_full_catalog.py's _install_ok_stage_c_stub
helpers mirrored compute_card_evidence's pre-modern_artist_lexicon signature,
missed by the sibling repair in test_stage_e_dispatch.py and
test_run_image_evidence_cohort.py (bf17be1) because neither module was run
locally. Same fix, same shape: accept and ignore the new keyword-only param.
…didate

Adds PrintingCandidate.artCropUrl, sourced from the existing
CanonicalPrintingMetadata.art_crop_url sidecar field (no new fetch or
harvest - the data is already on disk), following the illustrationId
precedent in shape and style:

- CanonicalCard.serialise_as_printing_candidate populates it, null-
  tolerant for both the no-sidecar and empty-art_crop_url cases.
- PrintingCandidate in both schema_types.py and schema_types.ts gains
  the optional field (backend from_dict/to_dict, frontend interface +
  typeMap entry).
- docs/features/printing-tags.md's Known gaps section notes the field
  now exists but nothing consumes it yet - rendering it is future
  frontend work.

Query efficiency: every path that reaches serialise_as_printing_candidate
(get_ranked_printing_candidates's two branches, and the ai_vote lookup in
question_feed._confirm_suggestion_item) already select_related's
printing_metadata for illustrationId's sake, so no new prefetch wiring
was needed. A new test proves the candidate-grid serialisation issues
zero additional queries regardless of candidate count.

This closes the backend half of the gap PR #687 (question-feed-frontend-
fixes branch, not yet merged) identified: illustration-clustered
candidate tiles have no shared art crop to render instead of each
candidate's own full-card scan. Consuming this field in the frontend
grid remains out of scope here.
…-fallback

# Conflicts:
#	docs/features/printing-tags.md
…s; reconcile docs

Item 1: merged origin/master (PR #685 was CONFLICTING) - the reconciled
Known-gaps bullet takes the branch side (artCropUrl now exists, nothing
consumed it) since it is the informed successor to master's #687 bullet.

Item 2: illustration-clustered candidate tiles now render
PrintingCandidate.artCropUrl in place of mediumThumbnailUrl, falling back
to the printing scan when a candidate's metadata sidecar has none.
Ungrouped/identify_printing tiles are unaffected.

Item 4: the Known-gaps bullet this closes is removed; the shipped
behaviour is documented on the existing 'Illustration grouping'
architecture bullet instead.
modern_artist_credit.recognize_artist_credit lands in this PR as a
third artist fallback inside compute_card_evidence, but without a
version bump the fix reaches no existing row: Stage C's resume filter
(MANIFEST_EXTRACTOR_CURRENT_VERSIONS, issue #509) compares stored
extractor_versions against the manifest and skips any row whose value
already matches, and all 230,378 production rows currently carry
artist_ocr: artist-ocr-v2.

Bumps ARTIST_OCR_EXTRACTOR_VERSION and its MANIFEST_EXTRACTOR_CURRENT_VERSIONS
counterpart to artist-ocr-v3, with a dedicated version-history comment
on the constant (the shared v1->v2 OCR-engine-swap comment above it
now covers only collector_line_ocr/collector_line_tsv, which are not
bumped here). Updates the three test fixtures asserting the literal
"artist-ocr-v2" string to match.
@WilfordGrimley
WilfordGrimley merged commit 534675d into master Aug 5, 2026
18 checks passed
WilfordGrimley added a commit that referenced this pull request Aug 5, 2026
…'s v3

ARTIST_OCR_EXTRACTOR_VERSION was set to "artist-ocr-v3" on this branch for
issue #677's ladder-collapse dedup, but PR #685 (merged to master
separately) already claims "artist-ocr-v3" for an unrelated change
(wiring modern_artist_credit's crop fallback into live extraction). Two
different behaviours under one version string would defeat the staleness
filter MANIFEST_EXTRACTOR_CURRENT_VERSIONS exists to enforce.

Bumps artist_ocr straight to v4, updates the matching manifest entry in
run_image_evidence_cohort.py, and documents the collision in both the
version constant's own history comment and catalog-completion-plan.md's
#677 section. collector_line_ocr/collector_line_tsv stay at v3 (master
still has both at v2, no collision).

Also fixes golden_set.py's #677 re-verification comment, which cited
MPCAutofill/scripts/experiments/ocr_ladder_tier_attribution.py - a
worktree-only analysis script never committed to this repo, per the
established convention (see docs/reports/2026-07-23-ocr-preprocessing-
probe.md's own LIVE STATE) of not committing one-off probes. Points at
catalog-completion-plan.md's already-committed #677 section instead,
which documents the same 450-card probe's methodology and results.
WilfordGrimley added a commit that referenced this pull request Aug 5, 2026
…s to 2 (issue #677) (#698)

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

_collector_line_ocr_attempts drops tier 3 (a PSM-11 re-try of tier 1's own
variants) - measured, not guessed. Two fresh real-production probes
(450 forced-escalation cards combined) walked the old 3-tier ladder to
full completion and recorded, per card, the first tier at which a
candidate-validated genuine match appeared: tier 2 produced 2 genuine
matches across the sample, tier 3 produced zero - only more lexicon-valid
-but-uncorroborated noise, consistent with the 2026-07-23 preprocessing
probes' own ~99% hopeless-art-noise finding for this population.
Cross-checked against all 30 golden_set.GOLDEN_CARD_IDS cards: none ever
resolved uniquely at tier 3, so this collapse changes zero golden-set
expectations.

Worst case drops from 8 tesseract calls to 6 (down from #259-#677's own
4x pre-#259 cost to 3x).

collector_line_ocr/collector_line_tsv/artist_ocr extractor versions
bumped v2->v3 (all three read through the same escalation loop, per
EXTRACTOR_OWNERSHIP's own 'bump every listed key together' convention);
legal_line unchanged.

Investigated and explicitly did NOT ship the companion 'unify the
collector-line/legal-line OCR reads into one full-width pass' idea named
in this issue's own brief: a real-image accuracy probe found it regresses
46/150 (30.7%) of currently-correct collector-number reads (mostly proxy/
watermark text getting picked up by the set-code regex in place of the
real code, or the wider crop causing tesseract to drop the collector-
number line outright), with zero improvements. Reported as a legitimate
negative finding rather than shipped anyway.

Lifts the EXTRACTOR_OWNERSHIP EXCLUDED_HELPERS entry for the OCR ladder
(check_extractor_ownership_totality.py) that PR #690 left in place while
this branch was in flight - _collector_line_ocr_attempts and
preprocess_fallback_variants now carry real ownership entries.

Verification: full backend suite (3598 passed, 8 skipped, 0 failed),
ownership-totality + manifest-sync scripts clean with their own unit
tests green (40/40), docs_lint --strict clean, pre-commit clean on all
tracked files.

* fix(image-evidence): bump artist_ocr to v4 to avoid colliding with #685's v3

ARTIST_OCR_EXTRACTOR_VERSION was set to "artist-ocr-v3" on this branch for
issue #677's ladder-collapse dedup, but PR #685 (merged to master
separately) already claims "artist-ocr-v3" for an unrelated change
(wiring modern_artist_credit's crop fallback into live extraction). Two
different behaviours under one version string would defeat the staleness
filter MANIFEST_EXTRACTOR_CURRENT_VERSIONS exists to enforce.

Bumps artist_ocr straight to v4, updates the matching manifest entry in
run_image_evidence_cohort.py, and documents the collision in both the
version constant's own history comment and catalog-completion-plan.md's
#677 section. collector_line_ocr/collector_line_tsv stay at v3 (master
still has both at v2, no collision).

Also fixes golden_set.py's #677 re-verification comment, which cited
MPCAutofill/scripts/experiments/ocr_ladder_tier_attribution.py - a
worktree-only analysis script never committed to this repo, per the
established convention (see docs/reports/2026-07-23-ocr-preprocessing-
probe.md's own LIVE STATE) of not committing one-off probes. Points at
catalog-completion-plan.md's already-committed #677 section instead,
which documents the same 450-card probe's methodology and results.

* fix(docs): cite OCR ladder probe scripts as prose, not backticked paths

Both scripts are deliberately uncommitted (worktree-only analysis
tooling, same convention as the 2026-07-23 probe reports). Backticked
citations to their paths tripped docs_lint.py's path-reference rule
since MPCAutofill/scripts/ doesn't exist in the repo. Rephrase as
prose so the provenance stays legible without a machine-checkable
path reference to a file that was never meant to be committed.
WilfordGrimley added a commit that referenced this pull request Aug 5, 2026
…s to 2 (issue #677) (#698) (#702)

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

_collector_line_ocr_attempts drops tier 3 (a PSM-11 re-try of tier 1's own
variants) - measured, not guessed. Two fresh real-production probes
(450 forced-escalation cards combined) walked the old 3-tier ladder to
full completion and recorded, per card, the first tier at which a
candidate-validated genuine match appeared: tier 2 produced 2 genuine
matches across the sample, tier 3 produced zero - only more lexicon-valid
-but-uncorroborated noise, consistent with the 2026-07-23 preprocessing
probes' own ~99% hopeless-art-noise finding for this population.
Cross-checked against all 30 golden_set.GOLDEN_CARD_IDS cards: none ever
resolved uniquely at tier 3, so this collapse changes zero golden-set
expectations.

Worst case drops from 8 tesseract calls to 6 (down from #259-#677's own
4x pre-#259 cost to 3x).

collector_line_ocr/collector_line_tsv/artist_ocr extractor versions
bumped v2->v3 (all three read through the same escalation loop, per
EXTRACTOR_OWNERSHIP's own 'bump every listed key together' convention);
legal_line unchanged.

Investigated and explicitly did NOT ship the companion 'unify the
collector-line/legal-line OCR reads into one full-width pass' idea named
in this issue's own brief: a real-image accuracy probe found it regresses
46/150 (30.7%) of currently-correct collector-number reads (mostly proxy/
watermark text getting picked up by the set-code regex in place of the
real code, or the wider crop causing tesseract to drop the collector-
number line outright), with zero improvements. Reported as a legitimate
negative finding rather than shipped anyway.

Lifts the EXTRACTOR_OWNERSHIP EXCLUDED_HELPERS entry for the OCR ladder
(check_extractor_ownership_totality.py) that PR #690 left in place while
this branch was in flight - _collector_line_ocr_attempts and
preprocess_fallback_variants now carry real ownership entries.

Verification: full backend suite (3598 passed, 8 skipped, 0 failed),
ownership-totality + manifest-sync scripts clean with their own unit
tests green (40/40), docs_lint --strict clean, pre-commit clean on all
tracked files.

* fix(image-evidence): bump artist_ocr to v4 to avoid colliding with #685's v3

ARTIST_OCR_EXTRACTOR_VERSION was set to "artist-ocr-v3" on this branch for
issue #677's ladder-collapse dedup, but PR #685 (merged to master
separately) already claims "artist-ocr-v3" for an unrelated change
(wiring modern_artist_credit's crop fallback into live extraction). Two
different behaviours under one version string would defeat the staleness
filter MANIFEST_EXTRACTOR_CURRENT_VERSIONS exists to enforce.

Bumps artist_ocr straight to v4, updates the matching manifest entry in
run_image_evidence_cohort.py, and documents the collision in both the
version constant's own history comment and catalog-completion-plan.md's
still has both at v2, no collision).

Also fixes golden_set.py's #677 re-verification comment, which cited
MPCAutofill/scripts/experiments/ocr_ladder_tier_attribution.py - a
worktree-only analysis script never committed to this repo, per the
established convention (see docs/reports/2026-07-23-ocr-preprocessing-
probe.md's own LIVE STATE) of not committing one-off probes. Points at
catalog-completion-plan.md's already-committed #677 section instead,
which documents the same 450-card probe's methodology and results.

* fix(docs): cite OCR ladder probe scripts as prose, not backticked paths

Both scripts are deliberately uncommitted (worktree-only analysis
tooling, same convention as the 2026-07-23 probe reports). Backticked
citations to their paths tripped docs_lint.py's path-reference rule
since MPCAutofill/scripts/ doesn't exist in the repo. Rephrase as
prose so the provenance stays legible without a machine-checkable
path reference to a file that was never meant to be committed.

(cherry picked from commit c5eee1c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant