Add modern-bare-name artist-credit recognizer (issue #368) - #390
Merged
Conversation
#368) New cardpicker.modern_artist_credit module: a from-scratch, in-house recognizer that re-parses already-stored ImageEvidence.artist_ocr_raw_text against the CanonicalArtist lexicon (tolerant of OCR garble via a ratio-plus-margin threshold) to recover modern bare-name credit lines the existing Illus.-anchored extractor (PROTECTED CORE, untouched) can't reach. backfill_modern_artist_names management command (dry-run default, --write to persist, never overwrites a non-blank name) fills the gap; no image fetch or OCR involved, and no new wiring needed downstream since local_calculate_verdicts already consumes artist_ocr_name in production. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…pshots CardFactory/CanonicalArtistFactory calls in test_backfill_modern_artist_names.py were permanently advancing factory_boy's process-global Sequence counters, shifting the ambient cumulative count test_views.py's snapshot tests hardcode (e.g. "Artist 102") when run later in the same suite. Adds the repo's established _preserve_shared_factory_sequences autouse fixture (see test_printing_consensus.py and ~30 other test files using the same pattern); verified via a throwaway probe that the fixture now leaves CardFactory/CanonicalArtistFactory's sequence counters untouched across the file's own tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Issue #368, owner-endorsed approach "2-B":
ImageEvidence.artist_ocr_raw_textis already captured for 215,049 cards (98.5% of evidence), butartist_ocr_nameextraction succeeds on only 13,588 of them, because the existing extractor (local_fallback.extract_artist_name, PROTECTED CORE, untouched by this PR) only recognizes old-border"Illus. <name>"credit lines. Modern (post-2003) frames instead print a bare name beside a brush-glyph icon tesseract reliably mangles — a shape that anchor was never built to catch.New module
cardpicker/modern_artist_credit.py— a from-scratch, in-house recognizer (PROVENANCE noted in its own header: entirely original code, no external patterns copied) that re-parses the already-stored raw OCR text: extracts every short (≤4-word) token-window per line as a name candidate, then validates each against the realCanonicalArtistlexicon (~2,523 names) via a tolerantdifflib-ratio fuzzy match. Two conservative guards, both found and fixed by sampling real production rows before finalizing thresholds:local_fallback.SYMBOL_MARGIN's own established convention for this exact risk shape.artist_ocr_raw_textis OCR'd from a wide bottom-of-card crop band that often also catches a fragment of flavor text. Sampling surfaced a real false positive: card evidence id 517's raw text contained French flavor text ("...ensuite, plus rien." = "...then, nothing more.") right next to the actual credit line, and "rien." happens to also be a real (if obscure) artist's stage name in the lexicon. Fixed by rejecting any candidate immediately preceded by a closed-class grammatical connector word (plus,and,the,avec, ...) — verified this rejects zero true positives across ~6,500 sampled rows while fixing the one false positive found.Handles the flagship garble case from the task brief directly: card 83867's raw text OCR'd "Kalk Kopinski" for the real "Karl Kopinski" (edit-distance 2) — correctly recovered.
New management command
backfill_modern_artist_names— house pattern (dry-run default,--writeto persist,PilotRunLedgerself-recording, chunked.iterator(), capped audit sample). Re-parses storedartist_ocr_raw_textonly — no image fetch, no OCR call anywhere in either module. Never overwrites a non-blankartist_ocr_name(enforced by the eligibility queryset itself, plus a defence-in-depth re-check immediately before any write).Downstream — nothing new wired, on purpose:
local_calculate_verdicts.calculate_join_key_verdict's existing "ARTIST-OCR corroboration" step already readsevidence.artist_ocr_namein production. Every name this backfill fills flows into printing identification automatically on that calculator's next pass over the affected cards.Docs:
docs/features/catalog-completion-plan.md's OCR-group section (which documents the originalartist_ocr_name/Illus.-anchor gap) updated in place with a short paragraph pointing at this closure, per house doc convention.Verification (production dry-run, read-only)
Ran
backfill_modern_artist_names(no--write) directly against the live production database (read-only reads + its own singlePilotRunLedgerrow — the only write, exactly as intended):Confirmed zero
ImageEvidence.artist_ocr_namerows changed (13,588non-blank before and after).20-name confidence sample from the real run (ratio / margin-over-runner-up):
Every sampled ratio ≥0.92, every margin ≥0.24 — comfortably clear of the conservative floors (
MIN_RATIO_SINGLE_WORD=0.92,MIN_RATIO_MULTI_WORD=0.85,MIN_MATCH_MARGIN=0.06).Full backend test suite (
pytest cardpicker/, host venv against testcontainers Postgres/ES, never production): 1,700 passed / 4 skipped, plus 47 new tests for this change (31 in the two new test modules) — all passing. 6 pre-existing snapshot failures intest_views.py(TestGetSampleCards/TestNewCardsFirstPages/TestNewCardsPage/TestPostExploreSearchResults) reproduced identically on a cleanmastercheckout with none of this PR's files present (verified viagit stash) — confirmed pre-existing/environmental, not caused by this change.Checklist
pre-commitand installed the hooks withpre-commit installbefore creating any commits.cardpicker/test suite run twice, confirming the 6 failing tests are pre-existing onmasterindependent of this change.PilotRunLedgerrow written) — see counts/sample above.Closes #368