diff --git a/MPCAutofill/cardpicker/local_lands_identify.py b/MPCAutofill/cardpicker/local_lands_identify.py index 2fad30f1f..32b450b2a 100644 --- a/MPCAutofill/cardpicker/local_lands_identify.py +++ b/MPCAutofill/cardpicker/local_lands_identify.py @@ -50,19 +50,58 @@ cleared and a real run is authorized. land_pool_size and the pre-filter per-name candidate counts are always computed over the FULL pool regardless of sample_size (both are free DB-only queries, no reason to sample them). + +EVIDENCE-FIRST DATA SOURCE (issue #359, Phase 1 of the post-2026-07-23 sequencing): before this +patch, EVERY selected card paid a real fetch + fresh OCR (step 1) even though Stage C's +harvest-calculate pipeline (`image_evidence.py`) has, as of 2026-07-23, already extracted and +persisted the exact same underlying signals (`ImageEvidence.collector_line_*`/`artist_ocr_*`) for +the overwhelming majority of this pool (1,603/1,609 = 99.6% of the unresolved basic-land cohort +specifically, per the issue's live sizing) - re-paying that cost is pure waste. This is a DATA- +SOURCE swap only, never a logic change: steps 2-3 (`identify_land_printing`) are completely +untouched, and step 1's outcome is reproduced via the SAME `local_ocr.validate_against_candidates` +call `run_ocr_for_card` itself makes, just fed a `local_ocr.OcrParseResult` reconstructed from +already-persisted fields instead of a freshly-OCR'd one - the identical technique +`local_calculate_verdicts.calculate_join_key_verdict` already established for Stage D's own +join-key calculator (see that function's own docstring: "reconstructs an OcrParseResult from +Stage C's already-persisted fields... calls the EXISTING, unmodified validate_against_candidates"). + +CURRENCY (same convention every other Stage C consumer in this codebase uses - see +`local_calculate_verdicts.run_join_key_calculator`/`run_fallback_calculator`/ +`run_slow_path_calculator`, all three): an `ImageEvidence` row is CURRENT for a card only when its +`content_hash` matches that card's own LIVE `content_phash` (an evidence row computed against a +prior image upload is never trusted for a card whose upload has since changed) AND its +`extractor_versions` carries both `collector_line_ocr` and `artist_ocr` keys (the two extractor +groups this module actually consumes - both are always written together by +`image_evidence.extract_card_evidence`'s single OCR-group block, so in practice checking either +key alone would suffice, but both are checked so this stays correct even under a future partial- +extractor-manifest write). See `_current_evidence_for_card`. + +Per-card branching (`run_lands_identify`'s own loop): a card WITH current evidence never touches +`fetch_card_image`/`fetch_budget` at all - `_ocr_result_from_evidence` replaces `run_ocr_for_card` +and `evidence.artist_ocr_name` (already `local_fallback.extract_artist_name`'s own tolerant parse, +computed once by Stage C) replaces `detect_illus_anchor`, so zero network cost and zero tesseract +calls are spent per evidence-backed card. A card WITHOUT current evidence falls back to the +pre-existing live fetch + `run_ocr_for_card` + `detect_illus_anchor` path, unchanged, still gated +by `fetch_budget`. `LandsIdentifyResult.evidence_backed` counts the former population separately +from `fetch_attempted` (the latter counts only real network fetches, exactly as before this patch) +so a report can show how much of a run's cost this patch actually avoided. """ from dataclasses import dataclass, field from typing import Optional -from cardpicker import local_phash +from cardpicker import local_ocr, local_phash from cardpicker.image_cdn_fetch import fetch_card_image from cardpicker.local_fallback import detect_illus_anchor, match_artist from cardpicker.local_identify_printing_tags import ( OCR_ANONYMOUS_ID, + OCR_CONFIDENCE_BOTH, + OCR_CONFIDENCE_COLLECTOR_ONLY, PHASH_MAX_CANDIDATES, CandidateNameIndex, CandidatePrinting, + EngineVote, + OcrCardResult, SelectedCard, _eligible_base_queryset, generate_run_id, @@ -70,7 +109,9 @@ ) from cardpicker.models import ( CanonicalCard, + Card, CardPrintingTag, + ImageEvidence, LandsAmbiguousResidue, VoteSource, ) @@ -140,6 +181,10 @@ class LandIdentifyOutcome: card_name: str candidate_count: int fetched: bool = False + # True when this card's outcome was produced entirely from a stored, current ImageEvidence + # row (issue #359's evidence-first data source) rather than a real fetch - mutually exclusive + # with `fetched` (never both True: an evidence-backed card never touches fetch_card_image). + evidence_backed: bool = False ocr_resolved_pk: Optional[int] = None artist_extracted: bool = False artist_matched_pks: Optional[frozenset[int]] = None @@ -167,6 +212,11 @@ class LandsIdentifyResult: sampled: int = 0 fetch_budget: int = 0 fetch_attempted: int = 0 + # Cards resolved via a stored, current ImageEvidence row (issue #359) - paid zero fetch/OCR + # cost, distinct from fetch_attempted (real network fetches only, unchanged meaning). Not + # bounded by fetch_budget - a DB-only read, same "free" category as land_pool_size/ + # per_name_candidate_counts. + evidence_backed: int = 0 ocr_resolved: int = 0 artist_extracted: int = 0 artist_extraction_failed: int = 0 @@ -273,6 +323,171 @@ def identify_land_printing( return match.candidate.pk, confidence, "", frozen_matched_pks, phash_distances +def _current_evidence_for_card(card: Card) -> Optional[ImageEvidence]: + """The module docstring's CURRENCY check - identical shape to `local_calculate_verdicts`'s + three own eligible-cards loops (`run_join_key_calculator`/`run_fallback_calculator`/ + `run_slow_path_calculator`, all filter `ImageEvidence.objects.filter(card_id=..., content_hash + =card.content_phash)`): a row is only trusted for this card if its `content_hash` matches the + card's own LIVE `content_phash` (an evidence row from a prior image upload is never reused for + a card whose upload has since changed) and it actually carries both extractor groups this + module consumes. `card.content_phash is None` (no stable hash yet) always misses - same "no + stable hash yet to key a CURRENT ImageEvidence lookup against" case those three callers each + skip early for their own reasons. `.order_by("-updated_at").first()` picks the most recently + written row on the rare chance more than one somehow exists for the same (card, content_hash) + pair (the model's own unique constraint means this is normally exactly one or zero).""" + if card.content_phash is None: + return None + return ( + ImageEvidence.objects.filter(card_id=card.pk, content_hash=card.content_phash) + .filter(extractor_versions__has_key="collector_line_ocr") + .filter(extractor_versions__has_key="artist_ocr") + .order_by("-updated_at") + .first() + ) + + +def _ocr_result_from_evidence(evidence: ImageEvidence, selected: SelectedCard) -> OcrCardResult: + """Step 1's evidence-first replacement for `run_ocr_for_card` (module docstring's "EVIDENCE- + FIRST DATA SOURCE" section) - reconstructs a `local_ocr.OcrParseResult` from Stage C's + already-persisted, already-parsed `collector_line_set_code`/`collector_line_collector_number` + fields (no re-fetch, no re-OCR) and calls the EXISTING, unmodified + `local_ocr.validate_against_candidates` - the same technique + `local_calculate_verdicts.calculate_join_key_verdict` already established for Stage D's own + join-key calculator. `raw_texts` is populated with the stored collector-line text alone (a + single already-selected reading, not every preprocessing variant a live pass would try) so a + caller falling through to the artist step still gets a real `raw_texts` list shape to work + with, though this module's own evidence-backed artist step never actually reads it (it reads + `evidence.artist_ocr_name` directly instead - see `run_lands_identify`).""" + parsed = local_ocr.OcrParseResult( + raw_text=evidence.collector_line_raw_text, + set_code=evidence.collector_line_set_code or None, + collector_number=evidence.collector_line_collector_number or None, + ) + matched, reason = local_ocr.validate_against_candidates(parsed, selected.candidates) + if matched is not None: + confidence = OCR_CONFIDENCE_BOTH if parsed.set_code is not None else OCR_CONFIDENCE_COLLECTOR_ONLY + return OcrCardResult( + vote=EngineVote( + engine="ocr", printing_pk=matched.pk, confidence=confidence, detail=parsed.raw_text.strip() + ), + raw_texts=[evidence.collector_line_raw_text], + parsed_a_collector_number=parsed.collector_number is not None, + ) + return OcrCardResult( + skip_reason=reason, + raw_texts=[evidence.collector_line_raw_text], + parsed_a_collector_number=parsed.collector_number is not None, + ) + + +def _process_land_card( + selected: SelectedCard, + ocr_result: OcrCardResult, + artist_name: Optional[str], + *, + evidence_backed: bool, + fetched: bool, + dry_run: bool, + run_id: str, + result: LandsIdentifyResult, + votes_batch: list[CardPrintingTag], + residue_batch: list[LandsAmbiguousResidue], +) -> None: + """Steps 1 (already resolved via `ocr_result`) through 3 of the module docstring's pipeline, + given an already-computed `ocr_result`/`artist_name` pair - shared by BOTH the evidence-backed + and live-fetch-fallback branches of `run_lands_identify`'s own loop so the two data sources + are guaranteed to produce IDENTICAL outcomes for identical (ocr_result, artist_name) inputs + (issue #359's "behavior-neutral data-source swap" requirement) - this function has no idea + which branch called it. `evidence_backed`/`fetched` are purely descriptive (recorded onto the + outcome for reporting), never branched on internally.""" + card = selected.card + if ocr_result.vote is not None: + result.ocr_resolved += 1 + if not dry_run: + votes_batch.append( + CardPrintingTag( + card_id=card.pk, + printing_id=ocr_result.vote.printing_pk, + is_no_match=False, + anonymous_id=OCR_ANONYMOUS_ID, + source=VoteSource.OCR, + confidence=ocr_result.vote.confidence, + run_id=run_id, + ) + ) + result.outcomes.append( + LandIdentifyOutcome( + card_id=card.pk, + card_name=card.name, + candidate_count=len(selected.candidates), + fetched=fetched, + evidence_backed=evidence_backed, + ocr_resolved_pk=ocr_result.vote.printing_pk, + ) + ) + return + + if artist_name is not None: + result.artist_extracted += 1 + else: + result.artist_extraction_failed += 1 + + printing_pk, confidence, skip_reason, artist_matched_pks, phash_distances = identify_land_printing( + selected, artist_name + ) + + if printing_pk is not None: + if confidence == LANDS_SINGLETON_CONFIDENCE: + result.singleton_votes += 1 + else: + result.tiebreak_votes += 1 + if not dry_run: + votes_batch.append( + CardPrintingTag( + card_id=card.pk, + printing_id=printing_pk, + is_no_match=False, + anonymous_id=LANDS_ANONYMOUS_ID, + source=VoteSource.OCR, + confidence=confidence, + run_id=run_id, + ) + ) + elif skip_reason.startswith("phash-"): + result.ambiguous_phash += 1 + # Routing data, not a vote (see LandsAmbiguousResidue's own docstring) - the artist + # match already paid the real narrowing cost; persist it so a future funnel surface + # can serve "which of these N?" instead of recomputing from the name's full pool. + if not dry_run and artist_name is not None and artist_matched_pks and phash_distances is not None: + residue_batch.append( + LandsAmbiguousResidue( + card_id=card.pk, + run_id=run_id, + artist_name=artist_name, + candidate_pks=sorted(artist_matched_pks), + phash_distances={str(pk): distance for pk, distance in phash_distances.items()}, + ) + ) + + if artist_matched_pks is not None: + result.per_name_post_filter_candidate_counts.setdefault(card.name, []).append(len(artist_matched_pks)) + + result.outcomes.append( + LandIdentifyOutcome( + card_id=card.pk, + card_name=card.name, + candidate_count=len(selected.candidates), + fetched=fetched, + evidence_backed=evidence_backed, + artist_extracted=artist_name is not None, + artist_matched_pks=artist_matched_pks, + printing_pk=printing_pk, + confidence=confidence, + skip_reason=skip_reason, + ) + ) + + def run_lands_identify( run_id: Optional[str] = None, dry_run: bool = True, @@ -280,11 +495,13 @@ def run_lands_identify( fetch_budget: int = 0, audit_sample_size: int = 20, ) -> LandsIdentifyResult: - """Orchestrator. See module docstring for the full pipeline and dry_run/sample_size - semantics. fetch_budget bounds real image fetches (shared CDN rate limiter) independent of - sample_size - pass fetch_budget=0 to get land_pool_size + per_name_candidate_counts (both - free) with zero network cost, useful for a first, instant read of pool shape before spending - any fetch budget on the artist-extraction-rate sample.""" + """Orchestrator. See module docstring for the full pipeline, dry_run/sample_size semantics, + and the evidence-first data source (issue #359). fetch_budget bounds real image fetches + (shared CDN rate limiter) independent of sample_size - pass fetch_budget=0 to get + land_pool_size + per_name_candidate_counts (both free) with zero network cost, useful for a + first, instant read of pool shape before spending any fetch budget on the artist-extraction- + rate sample. fetch_budget only bounds the LIVE-FETCH FALLBACK branch - a card with current + stored evidence never touches it, regardless of how small fetch_budget is.""" run_id = run_id or generate_run_id() index = CandidateNameIndex() @@ -306,6 +523,32 @@ def run_lands_identify( residue_batch: list[LandsAmbiguousResidue] = [] for selected in sampled_selected: card = selected.card + + # EVIDENCE-FIRST (module docstring, issue #359): a card with a CURRENT ImageEvidence row + # never touches fetch_card_image/run_ocr_for_card/detect_illus_anchor at all - steps 1-2's + # signals are read straight off the already-persisted row instead. + evidence = _current_evidence_for_card(card) + if evidence is not None: + result.evidence_backed += 1 + ocr_result = _ocr_result_from_evidence(evidence, selected) + artist_name = None if ocr_result.vote is not None else (evidence.artist_ocr_name or None) + _process_land_card( + selected, + ocr_result, + artist_name, + evidence_backed=True, + fetched=False, + dry_run=dry_run, + run_id=run_id, + result=result, + votes_batch=votes_batch, + residue_batch=residue_batch, + ) + continue + + # LIVE-FETCH FALLBACK (unchanged from before issue #359, except gated by fetch_budget + # only for cards actually reaching this branch - an evidence-backed card above never + # counts against it). if result.fetch_attempted >= fetch_budget: result.outcomes.append( LandIdentifyOutcome( @@ -332,89 +575,21 @@ def run_lands_identify( continue ocr_result = run_ocr_for_card(selected, image) - if ocr_result.vote is not None: - result.ocr_resolved += 1 - if not dry_run: - votes_batch.append( - CardPrintingTag( - card_id=card.pk, - printing_id=ocr_result.vote.printing_pk, - is_no_match=False, - anonymous_id=OCR_ANONYMOUS_ID, - source=VoteSource.OCR, - confidence=ocr_result.vote.confidence, - run_id=run_id, - ) - ) - result.outcomes.append( - LandIdentifyOutcome( - card_id=card.pk, - card_name=card.name, - candidate_count=len(selected.candidates), - fetched=True, - ocr_resolved_pk=ocr_result.vote.printing_pk, - ) - ) - continue - - _illus_anchor_fired, artist_name = detect_illus_anchor(image, ocr_result.raw_texts) - if artist_name is not None: - result.artist_extracted += 1 - else: - result.artist_extraction_failed += 1 - - printing_pk, confidence, skip_reason, artist_matched_pks, phash_distances = identify_land_printing( - selected, artist_name - ) - - if printing_pk is not None: - if confidence == LANDS_SINGLETON_CONFIDENCE: - result.singleton_votes += 1 - else: - result.tiebreak_votes += 1 - if not dry_run: - votes_batch.append( - CardPrintingTag( - card_id=card.pk, - printing_id=printing_pk, - is_no_match=False, - anonymous_id=LANDS_ANONYMOUS_ID, - source=VoteSource.OCR, - confidence=confidence, - run_id=run_id, - ) - ) - elif skip_reason.startswith("phash-"): - result.ambiguous_phash += 1 - # Routing data, not a vote (see LandsAmbiguousResidue's own docstring) - the artist - # match already paid the real narrowing cost; persist it so a future funnel surface - # can serve "which of these N?" instead of recomputing from the name's full pool. - if not dry_run and artist_name is not None and artist_matched_pks and phash_distances is not None: - residue_batch.append( - LandsAmbiguousResidue( - card_id=card.pk, - run_id=run_id, - artist_name=artist_name, - candidate_pks=sorted(artist_matched_pks), - phash_distances={str(pk): distance for pk, distance in phash_distances.items()}, - ) - ) - - if artist_matched_pks is not None: - result.per_name_post_filter_candidate_counts.setdefault(card.name, []).append(len(artist_matched_pks)) - - result.outcomes.append( - LandIdentifyOutcome( - card_id=card.pk, - card_name=card.name, - candidate_count=len(selected.candidates), - fetched=True, - artist_extracted=artist_name is not None, - artist_matched_pks=artist_matched_pks, - printing_pk=printing_pk, - confidence=confidence, - skip_reason=skip_reason, - ) + artist_name = None + if ocr_result.vote is None: + _illus_anchor_fired, artist_name = detect_illus_anchor(image, ocr_result.raw_texts) + + _process_land_card( + selected, + ocr_result, + artist_name, + evidence_backed=False, + fetched=True, + dry_run=dry_run, + run_id=run_id, + result=result, + votes_batch=votes_batch, + residue_batch=residue_batch, ) if not dry_run and votes_batch: diff --git a/MPCAutofill/cardpicker/management/commands/local_lands_identify.py b/MPCAutofill/cardpicker/management/commands/local_lands_identify.py index ac8ba5b30..cec156b56 100644 --- a/MPCAutofill/cardpicker/management/commands/local_lands_identify.py +++ b/MPCAutofill/cardpicker/management/commands/local_lands_identify.py @@ -9,6 +9,13 @@ ) from cardpicker.local_lands_identify import run_lands_identify from cardpicker.models import PilotRunLedger +from cardpicker.pilot_run_lifecycle import ( + add_dry_run_guard_arguments, + enforce_dry_run_precondition, + initial_counters, + merge_counters, + resilient_terminal_output, +) from cardpicker.utils import find_stale_applied_migrations, get_baked_git_sha @@ -19,7 +26,9 @@ class Command(BaseCommand): "over-cap name). Defaults to dry-run and requires an explicit --write to actually cast " "votes (HOLD #B - mirrors Part 3's --write gate convention). --sample-size defaults to " "300 per the plan doc's own HOLD #B ask; pass --sample-size 0 for a full-pool run once " - "the hold clears and a real run is authorized." + "the hold clears and a real run is authorized. Evidence-first (issue #359): a card with " + "a current ImageEvidence row consumes it directly (zero fetch/OCR cost) rather than " + "paying for its own fetch - see run_lands_identify's own module docstring." ) def add_arguments(self, parser: Any) -> None: @@ -46,9 +55,16 @@ def add_arguments(self, parser: Any) -> None: type=int, default=0, help="Max real image fetches to spend (shared CDN Worker rate limiter - see " - "image-cdn/wrangler.toml's IMAGE_FULL_TIER_RATE_LIMITER). Default: 0 (land_pool_" - "size + per_name_candidate_counts only, zero network cost).", + "image-cdn/wrangler.toml's IMAGE_FULL_TIER_RATE_LIMITER). Only bounds cards WITHOUT " + "a current ImageEvidence row (issue #359) - an evidence-backed card never counts " + "against this. Default: 0 (land_pool_size + per_name_candidate_counts only, zero " + "network cost, though evidence-backed cards still resolve fully at this setting).", ) + # local_lands_identify always operates over "whatever's currently eligible" (no + # --card-ids-file/--selector-style caller-chosen cohort), same shape as + # local_calculate_verdicts/consensus_recompute - so the guard below always passes + # scope=None, matching both of those commands' own identical comment. + add_dry_run_guard_arguments(parser, write_flag="--write") def handle(self, *args: Any, **kwargs: Any) -> None: stale = find_stale_applied_migrations() @@ -66,12 +82,21 @@ def handle(self, *args: Any, **kwargs: Any) -> None: mode = "WRITE" if kwargs["write"] else "DRY RUN" print(f"[{mode}] local_lands_identify run_id={run_id} git_sha={get_baked_git_sha()}") + skip_used = enforce_dry_run_precondition( + command="local_lands_identify", + write_mode=kwargs["write"], + skip_check=kwargs["skip_dryrun_check"], + window_hours=kwargs["dry_run_window_hours"], + scope=None, + ) + ledger = PilotRunLedger.objects.create( run_id=run_id, command="local_lands_identify", dry_run=dry_run, status=PilotRunLedger.Status.RUNNING, git_sha=get_baked_git_sha(), + counters=initial_counters(skip_dryrun_check_used=skip_used), ) try: @@ -82,33 +107,6 @@ def handle(self, *args: Any, **kwargs: Any) -> None: fetch_budget=kwargs["fetch_budget"], ) - print( - f"[lands] land_pool_size={result.land_pool_size} sample_size={result.sample_size or 'ALL'} " - f"sampled={result.sampled} fetch_budget={result.fetch_budget} " - f"fetch_attempted={result.fetch_attempted}" - ) - print( - f"[lands] ocr_resolved={result.ocr_resolved} artist_extracted={result.artist_extracted} " - f"artist_extraction_failed={result.artist_extraction_failed} " - f"artist_extraction_rate=" - f"{result.artist_extracted / result.fetch_attempted if result.fetch_attempted else 0:.3f}" - ) - print( - f"[lands] singleton_votes({'would_cast' if dry_run else 'written'})={result.singleton_votes} " - f"tiebreak_votes({'would_cast' if dry_run else 'written'})={result.tiebreak_votes} " - f"ambiguous_phash={result.ambiguous_phash} " - f"residue_rows({'would_write' if dry_run else 'written'})=" - f"{result.ambiguous_phash if dry_run else result.residue_written}" - ) - print("[lands] per_name_candidate_counts (pre-artist-filter, full pool):") - for name, count in sorted(result.per_name_candidate_counts.items(), key=lambda kv: -kv[1])[:20]: - print(f" {name}: {count}") - print("[lands] per_name_post_filter_candidate_counts (sampled cards with a successful artist match):") - for name, counts in sorted(result.per_name_post_filter_candidate_counts.items()): - print(f" {name}: {counts}") - for outcome in result.outcomes: - print(f" sample: {outcome}") - votes_written = result.votes_written touched_card_ids = [o.card_id for o in result.outcomes if o.printing_pk is not None or o.ocr_resolved_pk] @@ -122,18 +120,76 @@ def handle(self, *args: Any, **kwargs: Any) -> None: f"backed gate - STOP and investigate. Affected card pks: " f"{violations[:50]}" + (" (truncated)" if len(violations) > 50 else "") ) - print(f"Gate check passed: 0/{len(touched_card_ids)} touched cards resolved machine-only.") + # Counters-before-output (production incident 2026-07-23, see + # cardpicker.pilot_run_lifecycle's own module docstring point 1): the ledger row is + # saved COMPLETED here, BEFORE the terminal summary print block below - a + # BrokenPipeError on a severed stdout while printing that summary (which can be long + # for a full-pool run) must never look like this run failed. ledger.status = PilotRunLedger.Status.COMPLETED ledger.finished_at = timezone.now() ledger.votes_written = votes_written - ledger.save(update_fields=["status", "finished_at", "votes_written"]) - print( - f"[{mode}] done. run_id={run_id} total_votes=" - f"{'written' if not dry_run else 'would_cast'}={votes_written}" + ledger.counters = merge_counters( + ledger.counters, + { + "land_pool_size": result.land_pool_size, + "sampled": result.sampled, + "fetch_budget": result.fetch_budget, + "fetch_attempted": result.fetch_attempted, + "evidence_backed": result.evidence_backed, + "ocr_resolved": result.ocr_resolved, + "artist_extracted": result.artist_extracted, + "artist_extraction_failed": result.artist_extraction_failed, + "singleton_votes": result.singleton_votes, + "tiebreak_votes": result.tiebreak_votes, + "ambiguous_phash": result.ambiguous_phash, + "residue_written": result.residue_written, + }, ) + ledger.save(update_fields=["status", "finished_at", "votes_written", "counters"]) + + with resilient_terminal_output(): + print( + f"[lands] land_pool_size={result.land_pool_size} sample_size={result.sample_size or 'ALL'} " + f"sampled={result.sampled} fetch_budget={result.fetch_budget} " + f"fetch_attempted={result.fetch_attempted} evidence_backed={result.evidence_backed}" + ) + print( + f"[lands] ocr_resolved={result.ocr_resolved} artist_extracted={result.artist_extracted} " + f"artist_extraction_failed={result.artist_extraction_failed} " + f"artist_extraction_rate=" + f"{result.artist_extracted / result.fetch_attempted if result.fetch_attempted else 0:.3f}" + ) + print( + f"[lands] singleton_votes({'would_cast' if dry_run else 'written'})={result.singleton_votes} " + f"tiebreak_votes({'would_cast' if dry_run else 'written'})={result.tiebreak_votes} " + f"ambiguous_phash={result.ambiguous_phash} " + f"residue_rows({'would_write' if dry_run else 'written'})=" + f"{result.ambiguous_phash if dry_run else result.residue_written}" + ) + print("[lands] per_name_candidate_counts (pre-artist-filter, full pool):") + for name, count in sorted(result.per_name_candidate_counts.items(), key=lambda kv: -kv[1])[:20]: + print(f" {name}: {count}") + print("[lands] per_name_post_filter_candidate_counts (sampled cards with a successful artist match):") + for name, counts in sorted(result.per_name_post_filter_candidate_counts.items()): + print(f" {name}: {counts}") + for outcome in result.outcomes: + print(f" sample: {outcome}") + + if not dry_run and touched_card_ids: + print(f"Gate check passed: 0/{len(touched_card_ids)} touched cards resolved machine-only.") + + print( + f"[{mode}] done. run_id={run_id} total_votes=" + f"{'written' if not dry_run else 'would_cast'}={votes_written}" + ) except Exception: - ledger.status = PilotRunLedger.Status.FAILED - ledger.finished_at = timezone.now() - ledger.save(update_fields=["status", "finished_at"]) + # Only a still-RUNNING row gets marked FAILED here - a run this invocation already + # marked COMPLETED above (including the GATE VIOLATION CommandError path, which is + # raised BEFORE the ledger is marked COMPLETED and so is still correctly caught here) + # must never be overwritten by a later exception. + if ledger.status == PilotRunLedger.Status.RUNNING: + ledger.status = PilotRunLedger.Status.FAILED + ledger.finished_at = timezone.now() + ledger.save(update_fields=["status", "finished_at"]) raise diff --git a/MPCAutofill/cardpicker/tests/test_command_local_lands_identify.py b/MPCAutofill/cardpicker/tests/test_command_local_lands_identify.py new file mode 100644 index 000000000..bd778ab26 --- /dev/null +++ b/MPCAutofill/cardpicker/tests/test_command_local_lands_identify.py @@ -0,0 +1,183 @@ +""" +Tests for cardpicker.management.commands.local_lands_identify - the "house lifecycle" half of +issue #359 (Phase 0 rails, matching issues #345/#373's own PilotRunLedger self-recording/ +counters-before-output/forced-dry-run-guard pattern, already used by local_calculate_verdicts/ +consensus_recompute). run_lands_identify itself is exercised in test_local_lands_identify.py - +this file only covers the command's own lifecycle wiring, not the identification pipeline logic. +""" + +from unittest.mock import patch + +import pytest + +from django.core.management import CommandError, call_command + +from cardpicker.models import CardPrintingTag, PilotRunLedger +from cardpicker.tests.factories import ( + CanonicalArtistFactory, + CanonicalCardFactory, + CanonicalExpansionFactory, + CardFactory, + SourceFactory, +) + +# See test_local_lands_identify.py's identical fixture for the full rationale - +# factory.Sequence counters are process-global across the whole pytest run. +_SHARED_FACTORIES = [ + CardFactory, + SourceFactory, + CanonicalArtistFactory, + CanonicalExpansionFactory, + CanonicalCardFactory, +] + + +@pytest.fixture(autouse=True) +def _preserve_shared_factory_sequences(): + before = {f: f._meta.next_sequence() for f in _SHARED_FACTORIES} + for f, n in before.items(): + f.reset_sequence(n, force=True) + yield + for f, n in before.items(): + f.reset_sequence(n, force=True) + + +class TestLocalLandsIdentifyCommand: + def test_command_defaults_to_dry_run_and_writes_no_votes(self, db, capsys): + CanonicalCardFactory(name="Plains") + CardFactory(name="Plains") + + call_command("local_lands_identify", "--sample-size", "300", "--fetch-budget", "0") + + output = capsys.readouterr().out + assert "[DRY RUN]" in output + assert CardPrintingTag.objects.count() == 0 + + def test_command_refuses_to_run_against_a_stale_image(self, db): + with patch( + "cardpicker.management.commands.local_lands_identify.find_stale_applied_migrations", + return_value=[("cardpicker", "0099_fake_future_migration")], + ): + with pytest.raises(CommandError, match="STALE IMAGE"): + call_command("local_lands_identify") + + +class TestLocalLandsIdentifyLedger: + def test_dry_run_writes_a_completed_ledger_row_with_counters(self, db): + CanonicalCardFactory(name="Plains") + CardFactory(name="Plains") + + call_command("local_lands_identify", "--sample-size", "300", "--fetch-budget", "0") + + ledger = PilotRunLedger.objects.get(command="local_lands_identify") + assert ledger.dry_run is True + assert ledger.status == PilotRunLedger.Status.COMPLETED + assert ledger.finished_at is not None + assert ledger.counters["land_pool_size"] == 1 + assert ledger.counters["evidence_backed"] == 0 + assert ledger.counters["fetch_attempted"] == 0 + + def test_apply_writes_a_completed_ledger_row_with_votes_written(self, db): + printing = CanonicalCardFactory(name="Plains") + CardFactory(name="Plains") + + with patch("cardpicker.management.commands.local_lands_identify.run_lands_identify") as mock_run: + from cardpicker.local_lands_identify import ( + LandIdentifyOutcome, + LandsIdentifyResult, + ) + + mock_run.return_value = LandsIdentifyResult( + dry_run=False, + run_id="test-run", + land_pool_size=1, + sample_size=300, + sampled=1, + fetch_budget=10, + fetch_attempted=0, + evidence_backed=1, + ocr_resolved=1, + votes_written=1, + outcomes=[ + LandIdentifyOutcome( + card_id=1, + card_name="Plains", + candidate_count=1, + evidence_backed=True, + ocr_resolved_pk=printing.pk, + ) + ], + ) + call_command("local_lands_identify", "--write", "--skip-dryrun-check") + + ledger = PilotRunLedger.objects.get(command="local_lands_identify") + assert ledger.dry_run is False + assert ledger.status == PilotRunLedger.Status.COMPLETED + assert ledger.votes_written == 1 + assert ledger.counters["evidence_backed"] == 1 + + def test_a_genuine_failure_marks_the_ledger_row_failed(self, db): + with patch( + "cardpicker.management.commands.local_lands_identify.run_lands_identify", + side_effect=RuntimeError("boom"), + ): + with pytest.raises(RuntimeError): + call_command("local_lands_identify", "--write", "--skip-dryrun-check") + + ledger = PilotRunLedger.objects.get(command="local_lands_identify") + assert ledger.status == PilotRunLedger.Status.FAILED + assert ledger.finished_at is not None + + def test_broken_pipe_during_terminal_summary_does_not_flip_completed_to_failed(self, db, monkeypatch): + """Production incident 2026-07-23: a client-side timeout severed stdout AFTER every write + had already committed and the ledger row had already been saved COMPLETED - the terminal + summary print must never be able to flip that back to FAILED.""" + import cardpicker.management.commands.local_lands_identify as cmd_module + + CanonicalCardFactory(name="Plains") + CardFactory(name="Plains") + + real_print = print + + def raising_print(*args, **kwargs): + msg = args[0] if args else "" + if isinstance(msg, str) and msg.startswith("[DRY RUN] done."): + raise BrokenPipeError("stdout severed") + real_print(*args, **kwargs) + + monkeypatch.setattr(cmd_module, "print", raising_print, raising=False) + + call_command("local_lands_identify", "--sample-size", "300", "--fetch-budget", "0") + + ledger = PilotRunLedger.objects.get(command="local_lands_identify") + assert ledger.status == PilotRunLedger.Status.COMPLETED + assert ledger.finished_at is not None + + +class TestLocalLandsIdentifyDryRunGuard: + """The forced-dry-run guard (issue #362, wired in here per #359's "house lifecycle" ask) - + local_lands_identify always operates over the whole currently-eligible pool (no + --card-ids-file/--selector-style caller-chosen cohort), so scope=None: any matching recent + dry-run of this command satisfies the guard, regardless of --sample-size/--fetch-budget.""" + + def test_write_refused_without_a_prior_matching_dry_run(self, db): + with pytest.raises(CommandError, match="FORCED DRY-RUN GUARD"): + call_command("local_lands_identify", "--write") + assert not PilotRunLedger.objects.filter(command="local_lands_identify").exists() + + def test_write_succeeds_after_a_matching_dry_run(self, db): + call_command("local_lands_identify", "--sample-size", "300", "--fetch-budget", "0") # dry-run + call_command("local_lands_identify", "--write", "--sample-size", "300", "--fetch-budget", "0") + + ledgers = list(PilotRunLedger.objects.filter(command="local_lands_identify").order_by("started_at")) + assert len(ledgers) == 2 + assert ledgers[0].dry_run is True and ledgers[0].status == PilotRunLedger.Status.COMPLETED + assert ledgers[1].dry_run is False and ledgers[1].status == PilotRunLedger.Status.COMPLETED + + def test_skip_dryrun_check_bypasses_the_guard_and_is_recorded(self, db, capsys): + call_command("local_lands_identify", "--write", "--skip-dryrun-check") + + printed = capsys.readouterr().out + assert "[SKIP-DRYRUN-CHECK]" in printed + ledger = PilotRunLedger.objects.get(command="local_lands_identify") + assert ledger.counters["skip_dryrun_check_used"] is True diff --git a/MPCAutofill/cardpicker/tests/test_local_lands_identify.py b/MPCAutofill/cardpicker/tests/test_local_lands_identify.py index 79e1098f1..f422a2a45 100644 --- a/MPCAutofill/cardpicker/tests/test_local_lands_identify.py +++ b/MPCAutofill/cardpicker/tests/test_local_lands_identify.py @@ -3,13 +3,22 @@ HOLD #B) - artist-decomposed identification for names whose candidate count blocks the normal phash engine. No network calls: fetch_card_image/run_ocr_for_card/detect_illus_anchor are mocked exactly like test_local_residual_classify.py mocks the same functions. + +Evidence-first data source (issue #359): TestCurrentEvidenceForCard/TestOcrResultFromEvidence +cover the two new pure helpers directly; TestRunLandsIdentifyEvidenceFirst covers the orchestrator +branching (evidence-backed cards never call fetch_card_image/run_ocr_for_card/detect_illus_anchor +at all); TestEvidenceFirstAndFetchFallbackProduceIdenticalVerdicts is the explicit "same verdict +regardless of data source" fixture the issue asks for. """ import pytest import cardpicker.local_lands_identify as module +from cardpicker import local_ocr from cardpicker.local_identify_printing_tags import ( OCR_ANONYMOUS_ID, + OCR_CONFIDENCE_BOTH, + OCR_CONFIDENCE_COLLECTOR_ONLY, PHASH_MAX_CANDIDATES, CandidateNameIndex, EngineVote, @@ -31,6 +40,7 @@ CanonicalCardFactory, CanonicalExpansionFactory, CardFactory, + ImageEvidenceFactory, SourceFactory, ) @@ -324,3 +334,326 @@ def test_idempotent_via_scan_log_row(self, db, monkeypatch): result = run_lands_identify(dry_run=True, sample_size=300, fetch_budget=0) assert result.land_pool_size == 0 + + +def _evidence(card, **overrides): + """Same shape as test_local_calculate_verdicts.py's own `_evidence` helper - a CURRENT + ImageEvidence row (content_hash matching the card's own content_phash) carrying both + extractor groups `_current_evidence_for_card` requires by default.""" + defaults = dict( + content_hash=card.content_phash or 0, + extractor_versions={"collector_line_ocr": "collector-line-ocr-v1", "artist_ocr": "artist-ocr-v1"}, + collector_line_raw_text="", + collector_line_set_code="", + collector_line_collector_number="", + artist_ocr_raw_text="", + artist_ocr_name="", + ) + defaults.update(overrides) + return ImageEvidenceFactory(card=card, **defaults) + + +class TestCurrentEvidenceForCard: + """Unit tests for the module docstring's CURRENCY check (issue #359).""" + + def test_no_content_phash_never_matches(self, db): + card = CardFactory(name="Plains", content_phash=None) + _evidence(card, content_hash=0) + + assert module._current_evidence_for_card(card) is None + + def test_mismatched_content_hash_is_not_current(self, db): + card = CardFactory(name="Plains", content_phash=5) + _evidence(card, content_hash=999) # a prior image version, since superseded + + assert module._current_evidence_for_card(card) is None + + def test_missing_artist_ocr_extractor_key_is_not_current(self, db): + card = CardFactory(name="Plains", content_phash=5) + _evidence(card, content_hash=5, extractor_versions={"collector_line_ocr": "v1"}) + + assert module._current_evidence_for_card(card) is None + + def test_missing_collector_line_ocr_extractor_key_is_not_current(self, db): + card = CardFactory(name="Plains", content_phash=5) + _evidence(card, content_hash=5, extractor_versions={"artist_ocr": "v1"}) + + assert module._current_evidence_for_card(card) is None + + def test_matching_hash_with_both_extractor_keys_is_current(self, db): + card = CardFactory(name="Plains", content_phash=5) + evidence = _evidence(card, content_hash=5) + + assert module._current_evidence_for_card(card) == evidence + + +class TestOcrResultFromEvidence: + """Unit tests for `_ocr_result_from_evidence` - the evidence-first replacement for step 1 + (`run_ocr_for_card`), reusing `local_ocr.validate_against_candidates` unmodified.""" + + def test_direct_set_and_number_match_casts_the_both_confidence_tier(self, db): + expansion = CanonicalExpansionFactory(code="lea") + printing = CanonicalCardFactory(name="Plains", expansion=expansion, collector_number="288") + card = CardFactory(name="Plains", content_phash=1) + index = CandidateNameIndex() + selected = SelectedCard(card=card, candidates=index.candidates_for("Plains")) + evidence = _evidence( + card, + content_hash=1, + collector_line_raw_text="288/264 LEA EN", + collector_line_set_code="lea", + collector_line_collector_number="288", + ) + + result = module._ocr_result_from_evidence(evidence, selected) + + assert result.vote is not None + assert result.vote.printing_pk == printing.pk + assert result.vote.confidence == OCR_CONFIDENCE_BOTH + + def test_collector_number_only_match_casts_the_collector_only_confidence_tier(self, db): + printing = CanonicalCardFactory(name="Plains", collector_number="288") + card = CardFactory(name="Plains", content_phash=1) + index = CandidateNameIndex() + selected = SelectedCard(card=card, candidates=index.candidates_for("Plains")) + evidence = _evidence( + card, + content_hash=1, + collector_line_raw_text="288", + collector_line_set_code="", + collector_line_collector_number="288", + ) + + result = module._ocr_result_from_evidence(evidence, selected) + + assert result.vote is not None + assert result.vote.printing_pk == printing.pk + assert result.vote.confidence == OCR_CONFIDENCE_COLLECTOR_ONLY + + def test_ambiguous_collector_number_yields_ambiguous_skip_reason_not_a_vote(self, db): + CanonicalCardFactory(name="Plains", collector_number="288") + CanonicalCardFactory(name="Plains", collector_number="288") + card = CardFactory(name="Plains", content_phash=1) + index = CandidateNameIndex() + selected = SelectedCard(card=card, candidates=index.candidates_for("Plains")) + evidence = _evidence(card, content_hash=1, collector_line_set_code="", collector_line_collector_number="288") + + result = module._ocr_result_from_evidence(evidence, selected) + + assert result.vote is None + assert result.skip_reason == "ambiguous" + + def test_parsed_but_no_match_is_reported_as_such(self, db): + CanonicalCardFactory(name="Plains", collector_number="1") + card = CardFactory(name="Plains", content_phash=1) + index = CandidateNameIndex() + selected = SelectedCard(card=card, candidates=index.candidates_for("Plains")) + evidence = _evidence(card, content_hash=1, collector_line_set_code="", collector_line_collector_number="999") + + result = module._ocr_result_from_evidence(evidence, selected) + + assert result.vote is None + assert result.skip_reason == "parsed-but-no-match" + + def test_no_stored_collector_number_is_no_text(self, db): + CanonicalCardFactory(name="Plains") + card = CardFactory(name="Plains", content_phash=1) + index = CandidateNameIndex() + selected = SelectedCard(card=card, candidates=index.candidates_for("Plains")) + evidence = _evidence(card, content_hash=1) # collector_line_collector_number left blank + + result = module._ocr_result_from_evidence(evidence, selected) + + assert result.vote is None + assert result.skip_reason == "no-text" + + +class TestRunLandsIdentifyEvidenceFirst: + """Orchestrator-level coverage for the evidence-first branch (issue #359) - an evidence- + backed card must never touch fetch_card_image/run_ocr_for_card/detect_illus_anchor, and must + never count against fetch_attempted/fetch_budget.""" + + def test_evidence_backed_ocr_resolve_never_fetches(self, db, monkeypatch): + expansion = CanonicalExpansionFactory(code="lea") + printing = CanonicalCardFactory(name="Plains", expansion=expansion, collector_number="288") + card = CardFactory(name="Plains", content_phash=1) + _evidence( + card, + content_hash=1, + collector_line_raw_text="288/264 LEA EN", + collector_line_set_code="lea", + collector_line_collector_number="288", + ) + + def _unexpected_fetch(c, dpi=None): + raise AssertionError("evidence-backed card should never call fetch_card_image") + + def _unexpected_ocr(selected, image, **kw): + raise AssertionError("evidence-backed card should never call run_ocr_for_card") + + def _unexpected_artist(image, raw_texts): + raise AssertionError("evidence-backed card should never call detect_illus_anchor") + + monkeypatch.setattr(module, "fetch_card_image", _unexpected_fetch) + monkeypatch.setattr(module, "run_ocr_for_card", _unexpected_ocr) + monkeypatch.setattr(module, "detect_illus_anchor", _unexpected_artist) + + result = run_lands_identify(dry_run=False, sample_size=300, fetch_budget=0) + + assert result.evidence_backed == 1 + assert result.fetch_attempted == 0 + assert result.ocr_resolved == 1 + vote = CardPrintingTag.objects.get() + assert vote.printing_id == printing.pk + assert vote.anonymous_id == OCR_ANONYMOUS_ID + outcome = result.outcomes[0] + assert outcome.evidence_backed is True + assert outcome.fetched is False + + def test_evidence_backed_artist_singleton_never_fetches(self, db, monkeypatch): + artist = CanonicalArtistFactory(name="Rebecca Guay") + printing = CanonicalCardFactory(name="Plains", artist=artist, image_hash=7) + card = CardFactory(name="Plains", content_phash=7) + _evidence(card, content_hash=7, artist_ocr_name="Rebecca Guay") + + def _unexpected_fetch(c, dpi=None): + raise AssertionError("evidence-backed card should never call fetch_card_image") + + monkeypatch.setattr(module, "fetch_card_image", _unexpected_fetch) + + result = run_lands_identify(dry_run=False, sample_size=300, fetch_budget=0) + + assert result.evidence_backed == 1 + assert result.fetch_attempted == 0 + assert result.singleton_votes == 1 + vote = CardPrintingTag.objects.get() + assert vote.printing_id == printing.pk + assert vote.anonymous_id == LANDS_ANONYMOUS_ID + assert vote.confidence == LANDS_SINGLETON_CONFIDENCE + + def test_evidence_backed_cards_do_not_count_against_fetch_budget(self, db, monkeypatch): + artist = CanonicalArtistFactory(name="Rebecca Guay") + CanonicalCardFactory(name="Plains", artist=artist, image_hash=7) + evidence_card = CardFactory(name="Plains", content_phash=7) + _evidence(evidence_card, content_hash=7, artist_ocr_name="Rebecca Guay") + fetch_card = CardFactory(name="Plains", content_phash=None) + + monkeypatch.setattr(module, "fetch_card_image", lambda c, dpi=None: object()) + monkeypatch.setattr(module, "run_ocr_for_card", lambda selected, image, **kw: OcrCardResult()) + monkeypatch.setattr(module, "detect_illus_anchor", lambda image, raw_texts: (False, None)) + + # fetch_budget=0: the evidence-backed card still resolves fully (free), the non-evidence + # card hits the budget wall - proves the two populations are counted independently. + result = run_lands_identify(dry_run=True, sample_size=300, fetch_budget=0) + + assert result.evidence_backed == 1 + assert result.fetch_attempted == 0 + outcomes_by_card = {o.card_id: o for o in result.outcomes} + assert outcomes_by_card[evidence_card.pk].skip_reason != "fetch-budget-exhausted" + assert outcomes_by_card[fetch_card.pk].skip_reason == "fetch-budget-exhausted" + + def test_stale_evidence_content_hash_falls_back_to_live_fetch(self, db, monkeypatch): + CanonicalCardFactory(name="Plains") + card = CardFactory(name="Plains", content_phash=1) + _evidence(card, content_hash=999) # a prior image version, since superseded + + fetched = [] + monkeypatch.setattr(module, "fetch_card_image", lambda c, dpi=None: fetched.append(c.pk) or object()) + monkeypatch.setattr(module, "run_ocr_for_card", lambda selected, image, **kw: OcrCardResult()) + monkeypatch.setattr(module, "detect_illus_anchor", lambda image, raw_texts: (False, None)) + + result = run_lands_identify(dry_run=True, sample_size=300, fetch_budget=10) + + assert result.evidence_backed == 0 + assert result.fetch_attempted == 1 + assert fetched == [card.pk] + + def test_evidence_missing_a_required_extractor_key_falls_back_to_live_fetch(self, db, monkeypatch): + CanonicalCardFactory(name="Plains") + card = CardFactory(name="Plains", content_phash=1) + _evidence(card, content_hash=1, extractor_versions={"collector_line_ocr": "v1"}) # no artist_ocr + + fetched = [] + monkeypatch.setattr(module, "fetch_card_image", lambda c, dpi=None: fetched.append(c.pk) or object()) + monkeypatch.setattr(module, "run_ocr_for_card", lambda selected, image, **kw: OcrCardResult()) + monkeypatch.setattr(module, "detect_illus_anchor", lambda image, raw_texts: (False, None)) + + result = run_lands_identify(dry_run=True, sample_size=300, fetch_budget=10) + + assert result.evidence_backed == 0 + assert result.fetch_attempted == 1 + assert fetched == [card.pk] + + +class TestEvidenceFirstAndFetchFallbackProduceIdenticalVerdicts: + """issue #359's explicit ask: the SAME underlying signal (a collector-line read, or an artist + credit), fed through the evidence-first path vs the live-fetch fallback path, must produce + byte-identical outcomes - proving the data-source swap is behavior-neutral, not just that each + path works in isolation.""" + + def test_ocr_direct_match_is_identical_via_both_paths(self, db, monkeypatch): + expansion = CanonicalExpansionFactory(code="lea") + printing = CanonicalCardFactory(name="Plains", expansion=expansion, collector_number="288") + evidence_card = CardFactory(name="Plains", content_phash=1) + fetch_card = CardFactory(name="Plains", content_phash=None) + _evidence( + evidence_card, + content_hash=1, + collector_line_raw_text="288/264 LEA EN", + collector_line_set_code="lea", + collector_line_collector_number="288", + ) + + def fake_run_ocr_for_card(selected, image, **kw): + # A real live OCR pass that happens to read the IDENTICAL text the stored evidence + # above already carries - same input, live channel instead of stored, run through the + # same validate_against_candidates the evidence-first path itself uses internally. + parsed = local_ocr.OcrParseResult(raw_text="288/264 LEA EN", set_code="lea", collector_number="288") + matched, _reason = local_ocr.validate_against_candidates(parsed, selected.candidates) + assert matched is not None + return OcrCardResult( + vote=EngineVote(engine="ocr", printing_pk=matched.pk, confidence=OCR_CONFIDENCE_BOTH, detail="") + ) + + monkeypatch.setattr(module, "fetch_card_image", lambda c, dpi=None: object()) + monkeypatch.setattr(module, "run_ocr_for_card", fake_run_ocr_for_card) + + result = run_lands_identify(dry_run=False, sample_size=300, fetch_budget=10) + + votes = {v.card_id: v for v in CardPrintingTag.objects.all()} + assert votes[evidence_card.pk].printing_id == votes[fetch_card.pk].printing_id == printing.pk + assert votes[evidence_card.pk].confidence == votes[fetch_card.pk].confidence == OCR_CONFIDENCE_BOTH + assert votes[evidence_card.pk].anonymous_id == votes[fetch_card.pk].anonymous_id == OCR_ANONYMOUS_ID + + outcomes_by_card = {o.card_id: o for o in result.outcomes} + assert outcomes_by_card[evidence_card.pk].evidence_backed is True + assert outcomes_by_card[evidence_card.pk].fetched is False + assert outcomes_by_card[fetch_card.pk].evidence_backed is False + assert outcomes_by_card[fetch_card.pk].fetched is True + assert ( + outcomes_by_card[evidence_card.pk].ocr_resolved_pk + == outcomes_by_card[fetch_card.pk].ocr_resolved_pk + == printing.pk + ) + + def test_artist_singleton_match_is_identical_via_both_paths(self, db, monkeypatch): + artist = CanonicalArtistFactory(name="Rebecca Guay") + printing = CanonicalCardFactory(name="Plains", artist=artist, image_hash=7) + evidence_card = CardFactory(name="Plains", content_phash=7) + fetch_card = CardFactory(name="Plains", content_phash=7) + _evidence(evidence_card, content_hash=7, artist_ocr_name="Rebecca Guay") + + monkeypatch.setattr(module, "fetch_card_image", lambda c, dpi=None: object()) + monkeypatch.setattr(module, "run_ocr_for_card", lambda selected, image, **kw: OcrCardResult()) + monkeypatch.setattr(module, "detect_illus_anchor", lambda image, raw_texts: (True, "Rebecca Guay")) + + result = run_lands_identify(dry_run=False, sample_size=300, fetch_budget=10) + + votes = {v.card_id: v for v in CardPrintingTag.objects.all()} + assert votes[evidence_card.pk].printing_id == votes[fetch_card.pk].printing_id == printing.pk + assert votes[evidence_card.pk].confidence == votes[fetch_card.pk].confidence == LANDS_SINGLETON_CONFIDENCE + assert votes[evidence_card.pk].anonymous_id == votes[fetch_card.pk].anonymous_id == LANDS_ANONYMOUS_ID + + outcomes_by_card = {o.card_id: o for o in result.outcomes} + assert outcomes_by_card[evidence_card.pk].evidence_backed is True + assert outcomes_by_card[fetch_card.pk].evidence_backed is False diff --git a/docs/features/catalog-completion-plan.md b/docs/features/catalog-completion-plan.md index ba720b7fb..46e1b6159 100644 --- a/docs/features/catalog-completion-plan.md +++ b/docs/features/catalog-completion-plan.md @@ -650,6 +650,30 @@ confirmed post-stop) - that cache is permanent and independent of this run's own vote/residue loss, and is exactly the pre-warm the harvest-calculate pipeline's lands chunk inherits. +**Evidence-first data source (issue #359, 2026-07-23):** now that the +harvest-calculate pipeline below has fired end to end, `run_lands_identify` +was patched to read a card's own already-persisted, CURRENT `ImageEvidence` +row first (same currency test every Stage D calculator uses - `content_hash` +matching the card's live `content_phash`, plus both the `collector_line_ocr` +and `artist_ocr` extractor-version keys) rather than always paying its own +fresh fetch + OCR. This is a pure data-source swap - steps 2-3 +(`identify_land_printing`) are untouched, and step 1's outcome is reproduced +by feeding an `OcrParseResult` reconstructed from stored fields through the +same, unmodified `local_ocr.validate_against_candidates` call +`local_calculate_verdicts.calculate_join_key_verdict` already established +this technique for. A card lacking current evidence still falls back to the +original live fetch + OCR + artist-detection path unchanged. Read-only sizing +against the live pool (how many of the currently-unresolved basics carry +current evidence today) is deferred to the next `--fetch-budget 0` dry-run of +the deployed command - this PR's own test suite proves the branching and +parity (`identify_land_printing` untouched, both data sources produce +identical verdicts on identical inputs) against synthetic fixtures only, no +production DB access. The command also picked up the forced-dry-run guard/ +counters-before-output/ +ledger-counters rails (`cardpicker.pilot_run_lifecycle`, issues #345/#373's +convention) in the same change, since a cheap evidence-backed dry-run now +makes that guard's cheap-preview rationale hold here for the first time. + --- ## Harvest-calculate pipeline (Stages A–F, supersedes Part 4's remaining write run)