Pool printing-consensus votes across md5 identity groups

MPCAutofill/cardpicker/evidence_transfer.py

@@ -262,15 +262,19 @@ def transfer_evidence(card: Card, source: ImageEvidence, run_id: Optional[str] =
     sibling's, but stays correct even in the degenerate case where the two could ever disagree
     post-verification-race) and sets `transferred=True` + `transferred_from_card_id=source.card_id`.
 
-    INTERIM STAGE D GUARD (issue #473 PR-2, temporary by design - see `ImageEvidence.transferred`'s
-    own model-field docstring and `local_calculate_verdicts._eligible_cards_queryset`'s own
-    coordination-note comment): `transferred=True` here is what that guard reads to exclude this
-    card from the TWO machine-voting Stage D calculators (join-key/fallback - both cast a
-    `CardPrintingTag` vote) until PR-3's group-level vote pooling lands and removes the guard - a
-    transferred row's own machine "observation" is the SAME bytes a sibling card already voted
-    from, not an independent one. The third calculator, slow-path, is deliberately NOT guarded -
-    it casts no machine vote at all, only a human-review routing marker, which is exactly the
-    safety net the guard exists to preserve.
+    `transferred=True` set here USED TO BE what an INTERIM STAGE D GUARD (issue #473 PR-2,
+    temporary by design - see `ImageEvidence.transferred`'s own model-field docstring for the full
+    history) read to exclude this card outright from the TWO machine-voting Stage D calculators
+    (join-key/fallback, in their own loop bodies in `local_calculate_verdicts.py` - never
+    `_eligible_cards_queryset`), on the reasoning that a transferred row's own machine
+    "observation" is the SAME bytes a sibling card already voted from, not an independent one.
+    That guard is RETIRED as of PR-3 (2026-07-25, `TRANSFERRED_INTERIM_GUARD_SKIP_REASON`'s own
+    module-level comment in `local_calculate_verdicts.py` carries the full history): the
+    independence concern is now handled at the GROUP tally level by `vote_consensus.
+    pool_group_votes` instead, since a transferred card's vote and the sibling's it was copied
+    from are cast under the same calculator's fixed `anonymous_id` and so collapse under one
+    `dedupe_key`. `transferred`/`transferred_from_card_id` remain a plain provenance record, read
+    by no calculator anymore.
     """
     evidence, _ = ImageEvidence.objects.get_or_create(card_id=card.pk, content_hash=card.content_phash)
     for field_name in _TRANSFERABLE_FIELD_NAMES:

MPCAutofill/cardpicker/image_evidence.py

@@ -1006,11 +1006,14 @@ def persist_evidence(result: ExtractionResult, run_id: Optional[str] = None) ->
     `False`/`None` here (2026-07-25, issue #473 PR-2) - `persist_evidence` is called ONLY for a
     REAL extraction pass (`evidence_transfer.transfer_evidence` is the separate, only other writer
     of an `ImageEvidence` row, and it never calls this function), so every call here represents
-    genuine fresh extraction. A row that was previously TRANSFERRED (`transferred=True`) and later
-    receives a real extraction pass (e.g. `stage_e_shakedown`'s own `force_stage_c_reextract`) is
-    no longer a transferred row once this returns - leaving the flag stale would wrongly keep it
-    excluded from Stage D machine voting (the interim guard, `local_calculate_verdicts.
-    _eligible_cards_queryset`) even though it now carries a genuine independent extraction.
+    genuine fresh extraction. This reset stays correct and worth keeping even now that PR-3
+    (2026-07-25) has retired the interim Stage D guard that used to read this flag (see
+    `TRANSFERRED_INTERIM_GUARD_SKIP_REASON`'s own module-level comment in
+    `local_calculate_verdicts.py` for that history - the guard lived in the two calculators' own
+    loop bodies, never in `_eligible_cards_queryset`): `transferred` is still a real provenance
+    fact about this row, and a row that receives a genuine independent extraction is no longer a
+    copy of a sibling's bytes, so it should say so regardless of whether anything downstream still
+    reads the flag to make a decision.
     """
 
     if result.content_hash is None:

MPCAutofill/cardpicker/local_calculate_verdicts.py

@@ -457,31 +457,32 @@
 # two already-established tiers immediately above and below it.
 JOIN_KEY_CONFIDENCE_ARTIST_DISAGREEMENT = 0.65
 
-# INTERIM STAGE D GUARD (issue #473 PR-2, TEMPORARY BY DESIGN - see `ImageEvidence.transferred`'s
-# own model-field docstring and `evidence_transfer.transfer_evidence`'s own docstring for the full
-# rationale): a card whose CURRENT evidence row was created by `evidence_transfer.transfer_evidence`
-# rather than a real per-card extraction pass is excluded from the two MACHINE-VOTING Stage D
-# calculators below (join-key/fallback - both cast a `CardPrintingTag` vote) - its own "machine
-# observation" is the SAME bytes an md5-sibling card already voted from, not an independent one,
-# so casting a vote from it here would fabricate the independence the vote-weight matrix assumes
-# is real. The THIRD Stage D calculator, slow-path, is deliberately NOT guarded - it casts no
-# machine vote at all, only a `CardScanLog` routing marker handing the card to a HUMAN reviewer
-# (see `run_slow_path_calculator`'s own loop comment), which is exactly the safety net this guard
-# exists to preserve, not a case it needs to protect against. RESCANNABLE (a future real extraction
-# pass, or PR-3's own group-level vote pooling landing and removing this guard entirely, both
-# un-stick a card stuck here) - included in each of the two guarded calculators' own
-# RESCANNABLE_SKIP_REASONS set below. ISSUE #473's OWN COORDINATION NOTE (PR-3 build-plan section):
-# "Removes PR-2's interim Stage D guard" - do not remove this guard, or the `transferred` flag it
-# reads, before PR-3
-# (group-level vote pooling) actually merges and the group-aware calculators no longer need it.
+# RETIRED: INTERIM STAGE D GUARD (issue #473 PR-2, TEMPORARY BY DESIGN, removed by PR-3 -
+# 2026-07-25 owner-ratified group-level vote pooling, `vote_consensus.pool_group_votes`). A card
+# whose CURRENT evidence row was created by `evidence_transfer.transfer_evidence` rather than a
+# real per-card extraction pass USED TO BE excluded outright from the two MACHINE-VOTING Stage D
+# calculators below (join-key/fallback - both cast a `CardPrintingTag` vote), because its own
+# "machine observation" is the SAME bytes an md5-sibling card already voted from, not an
+# independent one, and casting a vote from it would have fabricated the independence the
+# vote-weight matrix assumes is real. That independence concern is now handled correctly at the
+# TALLY level instead: both calculators cast every vote under one fixed `anonymous_id`
+# (`JOIN_KEY_ANONYMOUS_ID`/`STAGE_D_FALLBACK_ANONYMOUS_ID`), so a transferred-evidence card's vote
+# and the sibling's it was copied from share a `pool_group_votes` `dedupe_key` and collapse to ONE
+# event within the group tally - exactly the outcome exclusion existed to protect, achieved without
+# discarding the vote outright. `ImageEvidence.transferred`/`transferred_from_card_id` remain (see
+# that field's own docstring) as a plain audit trail of provenance, no longer read by either
+# calculator loop. `TRANSFERRED_INTERIM_GUARD_SKIP_REASON` below stays defined, and stays a member
+# of both calculators' own `RESCANNABLE_SKIP_REASONS` sets, purely so a HISTORICAL `CardScanLog`
+# row written by a pre-PR-3 run still reads sensibly and still marks that card eligible for
+# reselection - no code path writes a NEW row with this reason anymore.
 TRANSFERRED_INTERIM_GUARD_SKIP_REASON = "transferred-interim-guard"
 
 # A degenerate/skip outcome that stays eligible for re-selection on a future invocation, same
 # convention as local_identify_printing_tags.RESCANNABLE_SKIP_REASONS - "no-evidence" here
 # because ImageEvidence simply hadn't been extracted yet for this card at selection time is a
 # transient state (a future extraction run may still land it), not a permanent conclusion.
-# TRANSFERRED_INTERIM_GUARD_SKIP_REASON (above) is rescannable for the same reason - both a real
-# extraction landing later and PR-3's own guard removal un-stick a card stuck here.
+# TRANSFERRED_INTERIM_GUARD_SKIP_REASON (above) is included for the same reason, scoped now to
+# HISTORICAL rows only - see its own module-level comment for why it is retired, not removed.
 JOIN_KEY_RESCANNABLE_SKIP_REASONS = frozenset({"no-evidence", TRANSFERRED_INTERIM_GUARD_SKIP_REASON})
 
 # THE SET-CODE LEXICON GATE (module docstring) - a parsed `set_code` that matches no
@@ -1234,23 +1235,6 @@ def run_join_key_calculator(
                 )
             continue
 
-        # INTERIM STAGE D GUARD (issue #473 PR-2, temporary by design - see
-        # TRANSFERRED_INTERIM_GUARD_SKIP_REASON's own module-level comment above).
-        if evidence.transferred:
-            result.skip_counts[TRANSFERRED_INTERIM_GUARD_SKIP_REASON] = (
-                result.skip_counts.get(TRANSFERRED_INTERIM_GUARD_SKIP_REASON, 0) + 1
-            )
-            if not dry_run:
-                scan_log_batch.append(
-                    CardScanLog(
-                        card_id=card.pk,
-                        anonymous_id=JOIN_KEY_ANONYMOUS_ID,
-                        run_id=run_id,
-                        skip_reason=TRANSFERRED_INTERIM_GUARD_SKIP_REASON,
-                    )
-                )
-            continue
-
         result.cards_considered += 1
         if index is None:
             index = _get_cached_candidate_name_index()
@@ -1332,8 +1316,9 @@ def run_join_key_calculator(
 # two carry the same meaning here as there, no rename needed.
 FALLBACK_NO_EVIDENCE_SKIP_REASON = "no-evidence"  # this calculator's own ImageEvidence-row-missing case, same meaning as JOIN_KEY's own identical string, different anonymous_id scope
 FALLBACK_NO_SUB_CHECK_EVIDENCE_SKIP_REASON = "no-sub-check-evidence"  # local_fallback.FallbackOutcome's own "no-evidence" concept, renamed to avoid colliding with the line above
-# TRANSFERRED_INTERIM_GUARD_SKIP_REASON (module-level comment above, issue #473 PR-2) is
-# rescannable here too - same reasoning as JOIN_KEY_RESCANNABLE_SKIP_REASONS' own inclusion of it.
+# TRANSFERRED_INTERIM_GUARD_SKIP_REASON (module-level comment above, issue #473 PR-2, retired by
+# PR-3) is included here too, scoped to HISTORICAL rows only - same reasoning as
+# JOIN_KEY_RESCANNABLE_SKIP_REASONS' own inclusion of it.
 FALLBACK_RESCANNABLE_SKIP_REASONS = frozenset({FALLBACK_NO_EVIDENCE_SKIP_REASON, TRANSFERRED_INTERIM_GUARD_SKIP_REASON})
 
 
@@ -1572,23 +1557,6 @@ def run_fallback_calculator(
                 )
             continue
 
-        # INTERIM STAGE D GUARD (issue #473 PR-2, temporary by design - see
-        # TRANSFERRED_INTERIM_GUARD_SKIP_REASON's own module-level comment above).
-        if evidence.transferred:
-            result.skip_counts[TRANSFERRED_INTERIM_GUARD_SKIP_REASON] = (
-                result.skip_counts.get(TRANSFERRED_INTERIM_GUARD_SKIP_REASON, 0) + 1
-            )
-            if not dry_run:
-                scan_log_batch.append(
-                    CardScanLog(
-                        card_id=card.pk,
-                        anonymous_id=STAGE_D_FALLBACK_ANONYMOUS_ID,
-                        run_id=run_id,
-                        skip_reason=TRANSFERRED_INTERIM_GUARD_SKIP_REASON,
-                    )
-                )
-            continue
-
         result.cards_considered += 1
         if index is None:
             index = _get_cached_candidate_name_index()
@@ -1860,12 +1828,11 @@ def run_slow_path_calculator(
         if card.content_phash is None:
             continue  # no stable hash yet to key a CURRENT ImageEvidence lookup against
 
-        # NOTE (issue #473 PR-2): the interim Stage D guard (TRANSFERRED_INTERIM_GUARD_SKIP_REASON,
-        # see its own module-level comment) deliberately does NOT apply here - this calculator
-        # casts no machine vote at all, only a CardScanLog routing marker that hands the card to a
-        # HUMAN reviewer. A human looking at transferred-evidence-derived signals is exactly the
-        # safety net the guard exists to preserve, not a case it needs to protect against - the
-        # fabricated-independence risk is specific to an automated vote, never a human decision.
+        # NOTE (issue #473, TRANSFERRED_INTERIM_GUARD_SKIP_REASON's own module-level comment): this
+        # calculator was NEVER guarded on `evidence.transferred` even while PR-2's now-retired
+        # interim guard excluded the other two calculators - it casts no machine vote at all, only
+        # a CardScanLog routing marker that hands the card to a HUMAN reviewer, so a
+        # transferred-evidence card routing here behaves exactly as it always has.
         evidence = (
             current_evidence_queryset(card)
             .filter(extractor_versions__has_key="collector_line_ocr")

MPCAutofill/cardpicker/management/commands/consensus_recompute.py

@@ -3,7 +3,9 @@
 reuses its iteration/grouping structure and its module docstring's own flagged "worth doing
 before running at scale" batching note, which this command is the one that actually does).
 
-Iterates every voted (card, printing)/(card, artist)/(card, tag) pair on record and calls the
+Iterates every voted (card, printing)/(card, artist)/(card, tag) pair on record - printing by md5
+IDENTITY GROUP, once per group rather than once per member (issue #473; a checksum-less card is a
+group of one, so this is the same walk it always was) - and calls the
 REAL `resolve_and_persist_printing`/`resolve_and_persist_artist`/`resolve_and_persist_tag_votes`
 paths (from `cardpicker.printing_consensus`/`cardpicker.artist_consensus`/`cardpicker.tag_consensus`
 - PROTECTED CORE, imported and called here, never modified) so persisted status matches what the
@@ -26,7 +28,10 @@
   - printing/artist: `resolve_and_persist_printing`/`resolve_and_persist_artist` read
     `card.printing_tags.all()`/`card.artist_votes.all()` - a `prefetch_related` per batch (not
     per card) already makes this ONE query per batch of cards, not one per card, so no further
-    batching work was needed here.
+    batching work was needed here. (Since issue #473, that prefetch is still what the printing
+    path uses for every card in a group of ONE - i.e. every checksum-less card; a genuine
+    multi-member md5 group reads its members' pooled votes in one further query per group, and
+    is visited once rather than once per member - see `_recompute_printing`.)
   - tag: `resolve_and_persist_tag_votes` already resolves every tag on ONE card in a single call
     (3 queries total per card, regardless of how many tags that card has votes for) - so the
     APPLY path is already card-granular, not pair-granular, and needed no further batching either.
@@ -67,7 +72,7 @@
 """
 
 from collections import Counter, defaultdict
-from typing import Any, Iterable, Iterator
+from typing import Any, Hashable, Iterable, Iterator, Sequence
 
 from django.conf import settings
 from django.core.management.base import BaseCommand, CommandError
@@ -105,6 +110,8 @@
 )
 from cardpicker.printing_consensus import (
     NO_MATCH,
+    md5_group_cards,
+    md5_group_key,
     resolve_and_persist_printing,
     resolve_printing,
 )
@@ -125,14 +132,19 @@ def _chunked(items: list[int], size: int) -> Iterator[list[int]]:
         yield items[i : i + size]
 
 
-def _would_be_printing_status(card: Card) -> str:
+def _would_be_printing_status(card: Card, group_card_ids: Sequence[int] | None = None) -> str:
     """
-    Exact duplicate of `consensus_impact_report._would_be_printing_status` - kept local rather
-    than imported (that function is module-private by its leading underscore, and this file's
-    own scope is meant to stay a self-contained management command) - see this module's own
-    docstring for why the dry-run path needs a non-writing prediction at all, unlike --apply.
+    `consensus_impact_report._would_be_printing_status` plus one optional argument - kept local
+    rather than imported (that function is module-private by its leading underscore, and this
+    file's own scope is meant to stay a self-contained management command) - see this module's
+    own docstring for why the dry-run path needs a non-writing prediction at all, unlike --apply.
+
+    `group_card_ids` is the md5 identity group `_recompute_printing` has already materialized
+    for this card (issue #473), passed through so the dry run doesn't re-derive the same group
+    per prediction. Omitting it (as `consensus_impact_report` does) is still correct, just one
+    query less efficient: `resolve_printing` derives the group itself when it isn't told.
     """
-    result = resolve_printing(card)
+    result = resolve_printing(card, group_card_ids=group_card_ids)
     if result is None:
         return PrintingTagStatus.UNRESOLVED
     if result == NO_MATCH:
@@ -242,21 +254,47 @@ def _record_transition(section: dict[str, Any], before: Any, after: Any, sample:
 
 
 def _recompute_printing(report: dict[str, Any], apply: bool, batch_size: int, sample_limit: int) -> None:
+    """
+    Iterates md5 identity GROUPS once each, not their members N times (issue #473): the group's
+    pooled tally resolves to one outcome and `resolve_and_persist_printing` writes that outcome
+    to every member in the same call, so visiting a second member would recompute an identical
+    result and rewrite identical rows. The first member reached in `date_created`-agnostic pk
+    order claims its group via `seen_group_keys`; every member is still COUNTED and still
+    reported on (transitions are recorded per card identifier, exactly as before, including for
+    members that carry no votes of their own but inherit the group's resolution).
+
+    Idempotence is unchanged and unconditional: the outcome is a pure function of the group's
+    current vote rows, written back on every call, so a second run over an already-recomputed
+    pool produces byte-identical persisted state - see this module's own docstring.
+
+    A checksum-less catalogue (every card a group of one - and, until #473's PR-1 lands, that is
+    every card) keys every card to its own pk, so `seen_group_keys` never skips anything and this
+    walks exactly the cards, queries, writes, and counters it walked before #473.
+    """
     section = report["printing"]
     card_ids = list(Card.objects.filter(printing_tags__isnull=False).values_list("id", flat=True).distinct())
+    seen_group_keys: set[Hashable] = set()
     for batch_ids in _chunked(card_ids, batch_size):
         with transaction.atomic():
             cards = Card.objects.filter(pk__in=batch_ids).prefetch_related("printing_tags")
             for card in cards:
-                section["checked"] += 1
-                before = card.printing_tag_status
+                group_key = md5_group_key(card)
+                if group_key in seen_group_keys:
+                    continue
+                seen_group_keys.add(group_key)
+                members = md5_group_cards(card)
+                before_by_member = [(member, member.printing_tag_status) for member in members]
                 if apply:
-                    resolve_and_persist_printing(card)
-                    section["written"] += 1
-                    after = card.printing_tag_status
+                    resolve_and_persist_printing(card, members=members)
+                    for member, before in before_by_member:
+                        section["checked"] += 1
+                        section["written"] += 1
+                        _record_transition(section, before, member.printing_tag_status, member.identifier, sample_limit)
                 else:
-                    after = _would_be_printing_status(card)
-                _record_transition(section, before, after, card.identifier, sample_limit)
+                    after = _would_be_printing_status(card, group_card_ids=[member.pk for member in members])
+                    for member, before in before_by_member:
+                        section["checked"] += 1
+                        _record_transition(section, before, after, member.identifier, sample_limit)
 
 
 def _recompute_artist(report: dict[str, Any], apply: bool, batch_size: int, sample_limit: int) -> None: