From 8172b1791b1db3d6b7424a6c6d6c3a6b6cb0ba3b Mon Sep 17 00:00:00 2001
From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com>
Date: Wed, 5 Aug 2026 01:16:00 +0000
Subject: [PATCH 1/2] feat(printing-consensus): phash distance-0 grouping
unions into the md5 identity group (issue #661)
`identity_group_card_ids`/`identity_group_cards`/`identity_group_key`/
`identity_group_expanded_card_ids` in printing_consensus.py compose the pre-existing md5
identity group (issue #473, byte identity) with a new artbox-phash-distance-0 group
(perceptual identity, `ImageEvidence.artbox_phash`) via a single union - no iterative
transitive closure, since artbox_phash is a deterministic function of image bytes, so an
md5 clique member that also carries a current phash already shares it with the rest of the
clique (see identity_group_card_ids's own docstring for the full argument).
This combined group is now what group_printing_votes/resolve_printing/
resolve_and_persist_printing pool votes across, and what question_feed's per-voter
answered-set widening expands through, replacing the md5-only group both used before.
Distance-0 only (never a Hamming threshold) - image_evidence.py's own "SOUNDNESS NOTE FOR
ANY FUTURE CONSUMER" and docs/theory.md's two-threshold split both reserve d=0 as sound
entailment; find_best_match's 20/5 narrowing-only thresholds are untouched.
Cards with no artbox_phash are excluded from phash grouping entirely (a group of one), never
collapsed into a shared NULL bucket. Currency-checked via the existing
evidence_transfer.md5_currency_q + content_hash=F("card__content_phash") pattern
(modern_artist_credit.eligible_evidence_queryset's own bulk-currency convention) - a stale
ImageEvidence row never seeds or joins a group.
Scope: printing_consensus.py, question_feed.py, consensus_recompute.py's printing recompute
path only. illustration_consensus.py and stage_e_dispatch.py's own inlined md5 grouping are
deliberately untouched - separate consumers/mechanisms, out of this PR's scope.
---
.../commands/consensus_recompute.py | 17 +-
MPCAutofill/cardpicker/printing_consensus.py | 309 +++++++++++++++---
MPCAutofill/cardpicker/question_feed.py | 32 +-
.../tests/test_md5_group_pooling.py | 15 +-
.../tests/test_phash_group_pooling.py | 270 +++++++++++++++
5 files changed, 575 insertions(+), 68 deletions(-)
create mode 100644 MPCAutofill/cardpicker/tests/test_phash_group_pooling.py
diff --git a/MPCAutofill/cardpicker/management/commands/consensus_recompute.py b/MPCAutofill/cardpicker/management/commands/consensus_recompute.py
index 22c6cdb85..f7417e02e 100644
--- a/MPCAutofill/cardpicker/management/commands/consensus_recompute.py
+++ b/MPCAutofill/cardpicker/management/commands/consensus_recompute.py
@@ -110,8 +110,8 @@
)
from cardpicker.printing_consensus import (
NO_MATCH,
- md5_group_cards,
- md5_group_key,
+ identity_group_cards,
+ identity_group_key,
resolve_and_persist_printing,
resolve_printing,
)
@@ -139,10 +139,11 @@ def _would_be_printing_status(card: Card, group_card_ids: Sequence[int] | None =
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.
+ `group_card_ids` is the combined identity group `_recompute_printing` has already
+ materialized for this card (issue #473, widened by #661), 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 less efficient: `resolve_printing` derives the group itself
+ when it isn't told.
"""
result = resolve_printing(card, group_card_ids=group_card_ids)
if result is None:
@@ -278,11 +279,11 @@ def _recompute_printing(report: dict[str, Any], apply: bool, batch_size: int, sa
with transaction.atomic():
cards = Card.objects.filter(pk__in=batch_ids).prefetch_related("printing_tags")
for card in cards:
- group_key = md5_group_key(card)
+ group_key = identity_group_key(card)
if group_key in seen_group_keys:
continue
seen_group_keys.add(group_key)
- members = md5_group_cards(card)
+ members = identity_group_cards(card)
before_by_member = [(member, member.printing_tag_status) for member in members]
if apply:
resolve_and_persist_printing(card, members=members)
diff --git a/MPCAutofill/cardpicker/printing_consensus.py b/MPCAutofill/cardpicker/printing_consensus.py
index a1521497c..7db458d94 100644
--- a/MPCAutofill/cardpicker/printing_consensus.py
+++ b/MPCAutofill/cardpicker/printing_consensus.py
@@ -3,11 +3,14 @@
from django.conf import settings
from django.core.exceptions import FieldDoesNotExist
+from django.db.models import F, QuerySet
+from cardpicker.evidence_transfer import md5_currency_q
from cardpicker.models import (
CanonicalCard,
Card,
CardPrintingTag,
+ ImageEvidence,
PrintingTagStatus,
calculator_family,
)
@@ -245,6 +248,221 @@ def md5_group_expanded_card_ids(card_ids: Iterable[int]) -> set[int]:
return ids | set(_card_ids_with_md5_checksums(checksums))
+# `ImageEvidence.artbox_phash` (issue #480) is a 64-bit perceptual hash of the card's own art-box
+# region, populated by a whole-catalogue Stage C pass rather than being upload-time metadata like
+# `Card.md5_checksum`. Distance-0 (exact) equality on it is a WIDER identity relation than md5:
+# two files that are NOT byte-identical (a re-encode, a fresh re-upload, a genuine reprint using
+# the same digital art asset) can still hash identically here. Issue #661 is what authorizes
+# treating that as sound entailment rather than mere narrowing: `image_evidence.py`'s own
+# "SOUNDNESS NOTE FOR ANY FUTURE CONSUMER" and `docs/theory.md` §4's two-threshold split both
+# reserve d=0, and ONLY d=0, as "the same uploaded image, transitively true" - a Hamming-distance
+# threshold match (`find_best_match`'s 20/5 cutoffs) stays narrowing-only, untouched by this
+# module. The functions below mirror the md5 helpers above in shape (see each one's own
+# docstring for the one place it must differ - `artbox_phash` lives on `ImageEvidence`, not on
+# `Card`, so a per-card read costs a query current-checksum reads do not) and are never consulted
+# on their own: `identity_group_card_ids` below is the sole entry point every caller outside this
+# module (and `group_printing_votes`/`resolve_and_persist_printing` inside it) is meant to use -
+# a phash-only grouping mechanism living BESIDE the md5 one is the outcome this module's own
+# docstring framing exists to avoid.
+
+
+def _current_artbox_phash_queryset() -> "QuerySet[ImageEvidence]":
+ """
+ Every `ImageEvidence` row that is (a) CURRENT for its card - `content_hash` agrees with the
+ card's own live `content_phash`, and the row's own stamped `md5_checksum` doesn't actively
+ disagree with the card's (the same bulk currency rule `modern_artist_credit.
+ eligible_evidence_queryset` applies, via the shared, null-tolerant `evidence_transfer.
+ md5_currency_q` - never reinvented here) - and (b) carries a computed `artbox_phash`. A STALE
+ row (the card's image has changed since this row was written) is exactly as untrustworthy for
+ grouping as it is for any other Stage C/D read in this codebase: it may describe art that no
+ longer exists at this card, so it must never seed a vote-pooling group.
+ """
+ return ImageEvidence.objects.filter(content_hash=F("card__content_phash"), artbox_phash__isnull=False).filter(
+ md5_currency_q()
+ )
+
+
+def _card_artbox_phash(card: Card) -> int | None:
+ """
+ `card`'s own CURRENT `artbox_phash`, or `None` if it has none - no evidence yet, a stale row,
+ or a card whose art-box was never classifiable (`image_evidence.py`'s own docstring: an
+ unclassifiable frame or a degenerate crop box). Unlike `_card_md5_checksum` (a plain
+ `getattr`, zero queries - `md5_checksum` lives directly on `Card`), this costs one query:
+ `artbox_phash` lives on the related `ImageEvidence` row, not on `Card` itself.
+ """
+ return _current_artbox_phash_queryset().filter(card_id=card.pk).values_list("artbox_phash", flat=True).first()
+
+
+def _artbox_phashes_for_card_ids(card_ids: Iterable[int]) -> set[int]:
+ """
+ The phash analogue of `_md5_checksums_for_card_ids`: the distinct CURRENT, non-null
+ `artbox_phash` values held by `card_ids` - one query, no `ImageEvidence` instances
+ materialized. `phash is not None`, not a truthy check: unlike a checksum, `0` is a real,
+ reachable hash value here, not an empty-string-style sentinel.
+ """
+ return {
+ phash
+ for phash in _current_artbox_phash_queryset()
+ .filter(card_id__in=card_ids)
+ .values_list("artbox_phash", flat=True)
+ if phash is not None
+ }
+
+
+def _card_ids_with_artbox_phashes(phashes: set[int]) -> list[int]:
+ """
+ The phash analogue of `_card_ids_with_md5_checksums`: every `Card.pk` whose CURRENT
+ `artbox_phash` is in `phashes`.
+ """
+ return list(_current_artbox_phash_queryset().filter(artbox_phash__in=phashes).values_list("card_id", flat=True))
+
+
+def phash_group_card_ids(card: Card) -> list[int]:
+ """
+ The pks of `card`'s artbox-phash-d0 group - every card whose CURRENT `artbox_phash` exactly
+ equals `card`'s own, `card` included - sorted, mirroring `md5_group_card_ids`. `[card.pk]`
+ for a card with no current phash: absence of `artbox_phash` is a group of ONE, never a shared
+ group with every other phash-less card - the same catastrophic-misread risk `_card_md5_
+ checksum`'s own docstring already warns about for a checksum-less card, and exactly the
+ failure mode issue #661's brief calls out by name ("cards with no artbox_phash are not a
+ group of NULLs").
+ """
+ phash = _card_artbox_phash(card)
+ if phash is None:
+ return [card.pk]
+ return sorted(set(_card_ids_with_artbox_phashes({phash})) | {card.pk})
+
+
+def identity_group_key(card: Card) -> Hashable:
+ """
+ Stable identity of `card`'s COMBINED (md5 union phash-d0) group, for callers that visit each
+ group once across a large iteration (`consensus_recompute`), mirroring `md5_group_key`'s own
+ contract. Checksum is checked FIRST and, when present, used ALONE (never combined with phash
+ in the key itself): every member of an md5 clique that also carries a current phash
+ necessarily shares that SAME phash value too (`artbox_phash` is a deterministic function of
+ the image bytes), so keying on checksum already reaches every such member - see
+ `identity_group_card_ids`'s docstring for the full argument this relies on.
+
+ This key can UNDER-collapse relative to the true combined group in one specific, harmless
+ way: two true members of one group reached by DIFFERENT keys (one via `("md5", X)` because it
+ has no phash, another via `("phash", Y)` because it has no checksum) both still return the
+ same group from `identity_group_card_ids` when actually resolved - membership is never
+ decided by this key, only VISIT-ONCE is. At worst this costs a redundant re-resolution of the
+ same group from a second visited member (an extra query and an idempotent rewrite), never a
+ wrong one.
+ """
+ checksum = _card_md5_checksum(card)
+ if checksum is not None:
+ return ("md5", checksum)
+ phash = _card_artbox_phash(card)
+ if phash is not None:
+ return ("phash", phash)
+ return ("card", card.pk)
+
+
+def identity_group_card_ids(card: Card) -> list[int]:
+ """
+ `card`'s full pooling identity group (issue #661): the UNION of its md5 group (byte-identical
+ files) and its artbox-phash-d0 group (perceptually-identical art-box crop), `card` included,
+ sorted and deduplicated. This is THE group `group_printing_votes`/`resolve_printing`/
+ `resolve_and_persist_printing` pool votes across - md5 alone was #473's definition of "one
+ identification target"; issue #661 WIDENS that definition, it does not add a second one
+ beside it.
+
+ WHY A SINGLE UNION - NOT AN ITERATIVE TRANSITIVE CLOSURE - IS ALREADY THE FULL COMPONENT
+ ------------------------------------------------------------------------------------------
+ Two cards could in principle be linked only through a CHAIN - A shares a checksum with B, B
+ (not A) shares a phash with C - in which case unioning A's own two DIRECT groups could look
+ like it risks missing C. It never actually does, because `artbox_phash` is a deterministic
+ function of the image bytes: if A and B are byte-identical (an md5 edge) and BOTH carry a
+ current phash, that phash is necessarily the SAME value on both rows - so B's phash edge to C
+ is already, independently, an edge from A to C directly (A and C share that same phash
+ value), reachable by A's own phash-group lookup without visiting B first. The same argument
+ runs symmetrically for two cards linked only by a shared phash whose md5-sibling reaches a
+ third. One md5 lookup plus one phash lookup, both rooted at `card` itself, therefore already
+ return the full connected component - no BFS/union-find needed.
+
+ (This relies on `artbox_phash` having been computed consistently for both byte-identical
+ rows, under the same extractor version. A version bump straddling two siblings' extraction
+ times could, in the worst case, UNDER-group them - the safe direction, the same tolerance
+ `agent_dedupe_key`'s own version-bump handling elsewhere in this module accepts - never
+ falsely merge two genuinely different targets.)
+
+ A card with neither a checksum nor a current phash is a group of one, same as ruling 3 always
+ was for md5 alone.
+ """
+ return sorted(set(md5_group_card_ids(card)) | set(phash_group_card_ids(card)))
+
+
+def _require_full_identity_group(card: Card, group_card_ids: Sequence[int], parameter: str) -> None:
+ """
+ Raises unless `group_card_ids` is EXACTLY `card`'s full combined identity group - the
+ combined-group analogue of `_require_full_md5_group`; read THAT function's docstring for the
+ complete argument (the silent-different-winner failure mode a partial group causes, why plain
+ set equality is insufficient, the `ValueError`-not-`assert` choice). Everything there applies
+ unchanged here, checked against `identity_group_card_ids(card)` in place of
+ `md5_group_card_ids(card)`.
+ """
+ authoritative = identity_group_card_ids(card)
+ supplied = list(group_card_ids)
+ if sorted(supplied) == authoritative:
+ return
+
+ supplied_set = set(supplied)
+ missing = sorted(set(authoritative) - supplied_set)
+ foreign = sorted(supplied_set - set(authoritative))
+ duplicated = sorted({card_id for card_id in supplied_set if supplied.count(card_id) > 1})
+ raise ValueError(
+ f"`{parameter}` is not card {card.pk}'s full identity group "
+ f"(missing {missing}, not in the group {foreign}, duplicated {duplicated}). "
+ "This parameter is an OPTIMISATION ONLY and MUST be exactly `identity_group_card_ids(card)`. "
+ "A partial group does not produce a weaker tally, it produces a DIFFERENT one: consensus "
+ "pools votes across the whole group and deduplicates per agent, so dropping members "
+ "changes which agents are counted and which are withheld for self-contradiction, and can "
+ "therefore select a DIFFERENT WINNING PRINTING - silently, with a plausible-looking "
+ "result written to every member. If you arrived here from a batch-scoping pass (#533/"
+ "#541): scope the batch's TARGETS by its card_ids, never a target's identity neighbourhood "
+ f"lookup. The fix is to pass `{parameter}=None` and let this module derive the group "
+ "itself, which costs a few indexed queries - never to widen or delete this check."
+ )
+
+
+def identity_group_cards(card: Card) -> list[Card]:
+ """
+ `card`'s combined identity group as `Card` INSTANCES, `card` itself first and unreplaced -
+ mirrors `md5_group_cards` exactly (see its docstring for why identity, not just pk, must be
+ preserved: callers write through and later read off their own `card` object).
+ """
+ group_card_ids = identity_group_card_ids(card)
+ other_ids = [card_id for card_id in group_card_ids if card_id != card.pk]
+ if not other_ids:
+ return [card]
+ return [card, *Card.objects.filter(pk__in=other_ids)]
+
+
+def identity_group_expanded_card_ids(card_ids: Iterable[int]) -> set[int]:
+ """
+ `card_ids` widened to every member of each card's combined identity group - the phash-aware
+ analogue of `md5_group_expanded_card_ids`, used the same way by `question_feed.py`: "cards
+ this voter has answered" widened to "identity groups this voter has answered", so a voter who
+ answered one member of a phash-d0 group is not re-asked the same art under a sibling's
+ identifier either. At most four queries (the existing checksum pair, plus the phash pair) -
+ see `identity_group_card_ids`'s docstring for why widening through each channel once, rooted
+ in the ORIGINAL `card_ids`, already reaches the full component with no further iteration.
+ """
+ ids = set(card_ids)
+ if not ids:
+ return ids
+ expanded = set(ids)
+ checksums = _md5_checksums_for_card_ids(ids)
+ if checksums:
+ expanded |= set(_card_ids_with_md5_checksums(checksums))
+ phashes = _artbox_phashes_for_card_ids(ids)
+ if phashes:
+ expanded |= set(_card_ids_with_artbox_phashes(phashes))
+ return expanded
+
+
@dataclass(frozen=True)
class ResolvedPrinting:
expansion_code: str
@@ -288,14 +506,15 @@ def get_resolved_printings(identifiers: Iterable[str]) -> dict[str, ResolvedPrin
def group_printing_votes(card: Card, group_card_ids: Sequence[int] | None = None) -> tuple[list[CardPrintingTag], bool]:
"""
- Every `CardPrintingTag` row cast against any member of `card`'s md5 identity group, plus
- whether that group actually has more than one member.
+ Every `CardPrintingTag` row cast against any member of `card`'s combined identity group (md5
+ union artbox-phash-d0, issue #661 - see `identity_group_card_ids`'s own docstring for the
+ union-not-closure argument), plus whether that group actually has more than one member.
- `group_card_ids`, when given, MUST be `card`'s COMPLETE md5 identity group - it is a
- convenience for a caller that already derived the group (e.g. it is about to persist to those
- same members), NOT a way to ask this function about part of one. That is CHECKED, not merely
+ `group_card_ids`, when given, MUST be `card`'s COMPLETE identity group - it is a convenience
+ for a caller that already derived the group (e.g. it is about to persist to those same
+ members), NOT a way to ask this function about part of one. That is CHECKED, not merely
documented: this is the one place in this module where a caller-supplied group is consumed,
- so `_require_full_md5_group` is called here, before either use of the value below, and a
+ so `_require_full_identity_group` is called here, before either use of the value below, and a
narrowed group (the shape a batch-scoping pass would naturally produce - see #533/#541)
raises instead of quietly returning a different tally's worth of rows. Read that function's
docstring before changing this line, and note that the check must MOVE WITH THE CONSUMPTION:
@@ -311,9 +530,9 @@ def group_printing_votes(card: Card, group_card_ids: Sequence[int] | None = None
`(card_id, pk)` so the pooled tally `pool_group_votes` builds is deterministic across runs.
"""
if group_card_ids is None:
- group_card_ids = md5_group_card_ids(card)
+ group_card_ids = identity_group_card_ids(card)
else:
- _require_full_md5_group(card, group_card_ids, "group_card_ids")
+ _require_full_identity_group(card, group_card_ids, "group_card_ids")
if len(group_card_ids) <= 1:
return list(card.printing_tags.all()), False
votes = list(
@@ -474,7 +693,7 @@ def resolve_printing(
card: Card, group_card_ids: Sequence[int] | None = None
) -> CanonicalCard | Literal["NO_MATCH"] | None:
"""
- Reconciles all `CardPrintingTag` votes cast against `card`'s md5 identity group into a
+ Reconciles all `CardPrintingTag` votes cast against `card`'s combined identity group into a
single resolved outcome: a specific `CanonicalCard` printing, the `NO_MATCH` sentinel
(consensus is that no printing matches), or `None` if there isn't yet enough signal to
conclude anything. See `cardpicker.vote_consensus.resolve_weighted_consensus` for the shared
@@ -482,31 +701,34 @@ def resolve_printing(
`MIN_SHARE` gates, non-machine gate) - this is a thin wrapper translating `CardPrintingTag`
rows into `VoteTuple`s and the winning outcome key back into a `CanonicalCard`.
- The identity group (issue #473) is every card indexing a byte-identical image file: ONE
- identification target, so its votes are tallied once, together, and the outcome applies to
- all of it. A card with no checksum, or the only card with its checksum, is a group of one
- (ruling 3) and takes the pre-#473 path unchanged - same rows, same query, same tuples, same
- result.
+ The identity group is the UNION (issue #661) of every card indexing a byte-identical image
+ file (issue #473) and every card sharing `card`'s own artbox-phash at distance 0 - see
+ `identity_group_card_ids`'s own docstring for why a single union already reaches the full
+ connected component. Byte-identical files and phash-d0-identical art-box crops are both, by
+ this module's own ruling, ONE identification target, so their votes are tallied once,
+ together, and the outcome applies to all of it. A card with neither a checksum nor a current
+ phash is a group of one (ruling 3) and takes the pre-#473 path unchanged - same rows, same
+ query, same tuples, same result.
`group_card_ids` - THE CONTRACT, in full
----------------------------------------
- Optional. When given it MUST be `card`'s COMPLETE md5 identity group, i.e. exactly
- `md5_group_card_ids(card)` (ordering is normalised and irrelevant; duplicates are not
+ Optional. When given it MUST be `card`'s COMPLETE identity group, i.e. exactly
+ `identity_group_card_ids(card)` (ordering is normalised and irrelevant; duplicates are not
permitted). It is a convenience for a caller that has already derived the group, never a way
to scope this call to part of one.
A BATCH-NARROWED GROUP IS THE SPECIFIC MISUSE THIS GUARDS. If you are threading a batch's
`card_ids` through the pipeline (#533/#541), do not thread it into here: a batch's `card_ids`
- scopes which TARGETS get resolved, and this argument is a target's md5 NEIGHBOURHOOD, whose
- members may lie outside the batch entirely. Scoping it by the batch is the natural-looking
- move and it is wrong.
+ scopes which TARGETS get resolved, and this argument is a target's identity NEIGHBOURHOOD,
+ whose members may lie outside the batch entirely. Scoping it by the batch is the
+ natural-looking move and it is wrong.
Passing anything else raises `ValueError`, because the failure it would otherwise cause is
silent and is not a mere loss of signal: a subset yields a DIFFERENT tally, not a weaker one,
- and can select a different winning printing. See `_require_full_md5_group` for the full
- argument, what exactly is compared, and what the check costs; the check itself runs inside
- `group_printing_votes` below, where the value is actually consumed. Omitting the argument is
- always correct and costs one indexed query.
+ and can select a different winning printing. See `_require_full_identity_group`/
+ `_require_full_md5_group` for the full argument, what exactly is compared, and what the check
+ costs; the check itself runs inside `group_printing_votes` below, where the value is actually
+ consumed. Omitting the argument is always correct.
"""
votes, is_group = group_printing_votes(card, group_card_ids)
if not votes:
@@ -544,37 +766,38 @@ def resolve_and_persist_printing(
) -> CanonicalCard | Literal["NO_MATCH"] | None:
"""
Runs `resolve_printing(card)` and writes the outcome onto `inferred_canonical_card` and
- `printing_tag_status` together - for EVERY member of `card`'s md5 identity group, not just
- `card` (issue #473 ruling 1: byte-identical images are one identification target, so a
- resolution reached on one of them is a resolution for all of them, and cannot be allowed to
- disagree with itself across the group by construction). `Card.serialise()` (which already
- reads `inferred_canonical_card`) and the printing-tag review queue (which filters on the
- indexed `printing_tag_status`, rather than recomputing consensus for every card) therefore
- stay in sync with the latest votes for every member at once. Intended to be called
- synchronously right after a vote is submitted for `card` - cheap, since it only touches this
- one group's own votes. Returns the same outcome `resolve_printing` returned, so callers
- don't need to recompute it again immediately afterwards.
-
- A group of one (a checksum-less or unique-checksum card - ruling 3) writes exactly the one
+ `printing_tag_status` together - for EVERY member of `card`'s combined identity group, not
+ just `card` (issue #473 ruling 1, widened by issue #661: byte-identical images and
+ phash-d0-identical art-box crops are each one identification target, so a resolution reached
+ on one member is a resolution for all of them, and cannot be allowed to disagree with itself
+ across the group by construction). `Card.serialise()` (which already reads
+ `inferred_canonical_card`) and the printing-tag review queue (which filters on the indexed
+ `printing_tag_status`, rather than recomputing consensus for every card) therefore stay in
+ sync with the latest votes for every member at once. Intended to be called synchronously
+ right after a vote is submitted for `card` - cheap, since it only touches this one group's
+ own votes. Returns the same outcome `resolve_printing` returned, so callers don't need to
+ recompute it again immediately afterwards.
+
+ A group of one (neither a checksum nor a current phash - ruling 3) writes exactly the one
row it always did, through the caller's own `card` instance, with no additional query.
`members` may be passed by a caller that already materialized the group (see
- `md5_group_cards`, whose contract this expects: `card` itself, first, unreplaced). Both
+ `identity_group_cards`, whose contract this expects: `card` itself, first, unreplaced). Both
halves of that contract are now CHECKED rather than assumed, because both fail silently:
- COMPLETENESS is enforced transitively and for free - the pks of `members` become
`resolve_printing`'s `group_card_ids`, so a partial `members` is rejected by
- `_require_full_md5_group` on the line below, BEFORE anything is written. That matters
- twice over here: a narrowed `members` would not only compute a different tally, it would
- also persist the result to only part of the group, leaving siblings on a stale
+ `_require_full_identity_group` on the line below, BEFORE anything is written. That
+ matters twice over here: a narrowed `members` would not only compute a different tally,
+ it would also persist the result to only part of the group, leaving siblings on a stale
`printing_tag_status` and putting the group into exactly the self-disagreeing state
ruling 1 says must be impossible by construction.
- IDENTITY (`card` itself present, not a freshly-fetched equal-pk copy) is checked here,
at no query cost, since the pk-level check above cannot see it. Substituting a copy
leaves this function writing through a different instance from the one the caller holds
and will read `printing_tag_status` off afterwards - the caller silently strands on a
- stale status, which is the failure the `md5_group_cards` docstring already warns about
- and which nothing verified until now.
+ stale status, which is the failure the `identity_group_cards` docstring already warns
+ about and which nothing verified until now.
Also pushes each written card into Elasticsearch, but only when the outcome actually changes
what's indexed for THAT card (see `_effective_indexed_printing_id`) - entering RESOLVED,
@@ -591,14 +814,14 @@ def resolve_and_persist_printing(
locks in the same order and queue behind each other instead of deadlocking. For a group of
one this is the same single write, in the same place, it always was.
"""
- group_cards = list(members) if members is not None else md5_group_cards(card)
+ group_cards = list(members) if members is not None else identity_group_cards(card)
if members is not None and not any(member is card for member in group_cards):
# identity, not `card.pk in {m.pk for m in group_cards}`: an equal-pk COPY is precisely
# the case this rejects (see the `members` paragraph above). Completeness is left to
# `resolve_printing`'s own guard on the next line rather than re-derived here.
raise ValueError(
f"`members` must contain the caller's own `card` instance (pk {card.pk}) itself, "
- "unreplaced - see `md5_group_cards`, whose output this expects. A freshly-fetched "
+ "unreplaced - see `identity_group_cards`, whose output this expects. A freshly-fetched "
"copy of the same row has the same pk but is a different object: this function would "
"write the resolution through the copy, leaving the caller's `card` on a stale "
"`printing_tag_status`/`inferred_canonical_card` with nothing to indicate it."
diff --git a/MPCAutofill/cardpicker/question_feed.py b/MPCAutofill/cardpicker/question_feed.py
index db2d268ca..02b25f7e7 100644
--- a/MPCAutofill/cardpicker/question_feed.py
+++ b/MPCAutofill/cardpicker/question_feed.py
@@ -100,7 +100,7 @@
build_group_printing_vote_tuples,
get_contested_card_ids,
group_printing_votes,
- md5_group_expanded_card_ids,
+ identity_group_expanded_card_ids,
)
from cardpicker.reason_tags import NOT_OFFICIAL_ART_REASON_TAGS
from cardpicker.schema_types import QuestionFeedCounts, QuestionFeedItem, TypeEnum
@@ -245,11 +245,12 @@ def _printing_vote_tuples(card: Card) -> list[VoteTuple]:
def _voter_answered_printing_card_ids(anonymous_id: str) -> set[int]:
"""
- Every card this voter has already cast a printing vote on, WIDENED to those cards' full md5
- identity groups (`printing_consensus.md5_group_expanded_card_ids`) - the exclusion set the
- printing tiers below filter against, so a voter who answered one member of a group is never
- asked the same byte-identical image again under a sibling's identifier (issue #473: the feed
- serves one member per group, not N).
+ Every card this voter has already cast a printing vote on, WIDENED to those cards' full
+ combined identity groups (`printing_consensus.identity_group_expanded_card_ids` - md5 union
+ artbox-phash-d0, issue #661) - the exclusion set the printing tiers below filter against, so
+ a voter who answered one member of a group is never asked the same byte-identical or
+ phash-d0-identical image again under a sibling's identifier (issue #473: the feed serves one
+ member per group, not N).
This replaces the `.exclude(printing_tags__anonymous_id=anonymous_id)` clause those tiers
used before, and is exactly equivalent to it for a card whose group is itself alone (the
@@ -264,15 +265,16 @@ def _voter_answered_printing_card_ids(anonymous_id: str) -> set[int]:
direct caller - a test, a shell - can still ask for one tier by `anonymous_id` alone.
"""
voted_card_ids = CardPrintingTag.objects.filter(anonymous_id=anonymous_id).values_list("card_id", flat=True)
- return md5_group_expanded_card_ids(voted_card_ids)
+ return identity_group_expanded_card_ids(voted_card_ids)
def _voter_answered_artist_card_ids(anonymous_id: str) -> set[int]:
"""
The artist-tier analogue of `_voter_answered_printing_card_ids` above: every card this voter
- has already cast a `CardArtistVote` on, widened to those cards' full md5 identity groups, so
- a voter who answered one member of a byte-identical group is not re-asked the same artist
- question under a sibling's identifier (issue #473). Scoped to `_tier_2_contested` only
+ has already cast a `CardArtistVote` on, widened to those cards' full combined identity
+ groups, so a voter who answered one member of a byte-identical or phash-d0-identical group is
+ not re-asked the same artist question under a sibling's identifier (issue #473, widened by
+ #661). Scoped to `_tier_2_contested` only
(2026-08-04 gate on the phase-C/md5 routing brief) - `_tier_4_fresh`'s own artist exclusion
keeps its pre-existing, unwidened `.exclude(artist_votes__anonymous_id=...)` form.
@@ -280,13 +282,13 @@ def _voter_answered_artist_card_ids(anonymous_id: str) -> set[int]:
convention exactly.
"""
voted_card_ids = CardArtistVote.objects.filter(anonymous_id=anonymous_id).values_list("card_id", flat=True)
- return md5_group_expanded_card_ids(voted_card_ids)
+ return identity_group_expanded_card_ids(voted_card_ids)
def _voter_answered_tag_card_ids_by_tag(anonymous_id: str) -> dict[str, set[int]]:
"""
For every tag name this voter has cast a `CardTagVote` on, the set of card ids - each widened
- to its full md5 identity group - that count as "already answered" for THAT tag. Widening is
+ to its full combined identity group - that count as "already answered" for THAT tag. Widening is
on the CARD axis only, never the tag axis: `_tier_2_contested`'s own-vote exclusion is
deliberately scoped to (card, tag, anonymous_id), not (card, anonymous_id) - a card carries
~11 independent attribute-chip tags, and a card-level exclude would silently hide every other
@@ -304,7 +306,7 @@ def _voter_answered_tag_card_ids_by_tag(anonymous_id: str) -> dict[str, set[int]
card_ids_by_tag: dict[str, set[int]] = defaultdict(set)
for tag_name, card_id in rows:
card_ids_by_tag[tag_name].add(card_id)
- return {tag_name: md5_group_expanded_card_ids(card_ids) for tag_name, card_ids in card_ids_by_tag.items()}
+ return {tag_name: identity_group_expanded_card_ids(card_ids) for tag_name, card_ids in card_ids_by_tag.items()}
def _not_official_art_card_ids() -> set[int]:
@@ -322,7 +324,7 @@ def _not_official_art_card_ids() -> set[int]:
caster from writing one in principle, and this routing signal is meant to represent an
actual human declaration that the artwork question is meaningless for this card - a future
machine-cast source earning the same trust would need its own explicit decision, not a
- silent inclusion here.
+ silent inclusion here. Widened via the combined identity group, not md5 alone (issue #661).
Unlike `_voter_answered_printing_card_ids`/`_voter_answered_artist_card_ids` above, this is
NOT per-voter: it is a fact about the CARD, so it applies identically to every voter's feed.
@@ -332,7 +334,7 @@ def _not_official_art_card_ids() -> set[int]:
tag__name__in=NOT_OFFICIAL_ART_REASON_TAGS, polarity=VotePolarity.APPLY
).values_list("card_id", "source")
human_backed_card_ids = {card_id for card_id, source in rows if is_human_backed_source(source)}
- return md5_group_expanded_card_ids(human_backed_card_ids)
+ return identity_group_expanded_card_ids(human_backed_card_ids)
def is_likely_resolve_printing(card: Card) -> bool:
diff --git a/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py b/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py
index 287c18e7e..d91fb8425 100644
--- a/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py
+++ b/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py
@@ -246,13 +246,24 @@ def test_singleton_votes_carry_no_pooling_key(self, db):
vote_tuples = build_group_printing_vote_tuples(votes, pool=is_group)
assert [vote.dedupe_key for vote in vote_tuples] == [None]
- def test_singleton_read_honours_a_callers_prefetch_and_adds_no_query(self, db, django_assert_num_queries):
+ def test_singleton_read_honours_a_callers_prefetch_and_adds_one_phash_check_query(
+ self, db, django_assert_num_queries
+ ):
+ """
+ Zero EXTRA queries beyond the one issue #661 always adds: `group_printing_votes` now
+ derives `identity_group_card_ids`, not `md5_group_card_ids` alone, and the phash half of
+ that (`_card_artbox_phash`) is a query against `ImageEvidence` no matter what the md5
+ checksum lookup found - `artbox_phash` lives on a related model, not on `Card` itself
+ (see `_card_artbox_phash`'s own docstring). The checksum half stays free (a `getattr`),
+ and the caller's own `prefetch_related("printing_tags")` is still honoured for the votes
+ read itself - this is the one query the phash channel adds, not a second per-vote cost.
+ """
card = CardFactory()
printing = CanonicalCardFactory()
human_vote(card, printing, "human-1")
prefetched = list(Card.objects.filter(pk=card.pk).prefetch_related("printing_tags"))[0]
- with django_assert_num_queries(0):
+ with django_assert_num_queries(1):
votes, is_group = group_printing_votes(prefetched)
assert is_group is False
diff --git a/MPCAutofill/cardpicker/tests/test_phash_group_pooling.py b/MPCAutofill/cardpicker/tests/test_phash_group_pooling.py
new file mode 100644
index 000000000..096b9f5dc
--- /dev/null
+++ b/MPCAutofill/cardpicker/tests/test_phash_group_pooling.py
@@ -0,0 +1,270 @@
+"""
+Tests for artbox-phash-d0 grouping as a pooling primitive (issue #661) -
+`printing_consensus.identity_group_card_ids`/`identity_group_cards`/`identity_group_key`/
+`identity_group_expanded_card_ids`/`phash_group_card_ids`: the union of the pre-existing md5
+identity group (issue #473) with a new artbox-phash-distance-0 group, which now feeds
+`group_printing_votes`/`resolve_printing`/`resolve_and_persist_printing` and `question_feed`'s
+answered-set widening in place of the md5-only group those used before.
+
+Companion to `test_md5_group_pooling.py`, which this deliberately does NOT duplicate: the pure
+pooling primitive (`vote_consensus.pool_group_votes`) and the full md5-only surface are pinned
+there. This file is scoped to what issue #661 actually adds - the phash channel itself, and the
+union composition - using REAL `Card.md5_checksum`/`ImageEvidence.artbox_phash` columns (both
+real fields on this branch, unlike `test_md5_group_pooling.py`'s `md5_groups` monkeypatch
+fixture, which predates the checksum column and is kept there for reasons that don't apply here).
+"""
+
+from cardpicker.models import VoteSource, calculator_family
+from cardpicker.printing_consensus import (
+ agent_dedupe_key,
+ build_group_printing_vote_tuples,
+ group_printing_votes,
+ identity_group_card_ids,
+ identity_group_cards,
+ identity_group_expanded_card_ids,
+ identity_group_key,
+ md5_group_card_ids,
+ phash_group_card_ids,
+ resolve_and_persist_printing,
+ resolve_printing,
+)
+from cardpicker.tests.factories import (
+ CanonicalCardFactory,
+ CardFactory,
+ CardPrintingTagFactory,
+ ImageEvidenceFactory,
+)
+
+
+def phash_evidence(card, phash, content_hash=None, **overrides):
+ """
+ A CURRENT `ImageEvidence` row for `card` carrying `phash`. `content_hash` defaults to
+ `card.content_phash` so the row passes `image_evidence.current_evidence_queryset`'s currency
+ check; `md5_checksum` is left at `ImageEvidenceFactory`'s own default (`None`), which is
+ null-tolerant under `evidence_transfer.md5_currency_q` and never disagrees with the card's
+ own. Passing an explicit `content_hash` that does NOT match `card.content_phash` is how the
+ staleness test below builds a deliberately-stale row.
+ """
+ defaults = dict(
+ content_hash=content_hash if content_hash is not None else (card.content_phash or 0), artbox_phash=phash
+ )
+ defaults.update(overrides)
+ return ImageEvidenceFactory(card=card, **defaults)
+
+
+def machine_vote(card, printing, anonymous_id):
+ return CardPrintingTagFactory(card=card, printing=printing, source=VoteSource.OCR, anonymous_id=anonymous_id)
+
+
+def human_vote(card, printing, anonymous_id):
+ return CardPrintingTagFactory(card=card, printing=printing, source=VoteSource.USER, anonymous_id=anonymous_id)
+
+
+class TestPhashGroupCardIds:
+ def test_no_phash_is_a_group_of_one(self, db):
+ card = CardFactory(content_phash=1)
+ assert phash_group_card_ids(card) == [card.pk]
+
+ def test_shared_phash_forms_a_group(self, db):
+ card_a = CardFactory(content_phash=1)
+ card_b = CardFactory(content_phash=2)
+ phash_evidence(card_a, phash=555)
+ phash_evidence(card_b, phash=555)
+
+ assert phash_group_card_ids(card_a) == sorted([card_a.pk, card_b.pk])
+ assert phash_group_card_ids(card_b) == sorted([card_a.pk, card_b.pk])
+
+ def test_stale_evidence_row_is_excluded(self, db):
+ """A card whose image has changed since its evidence row was written (`content_hash` no
+ longer matches the card's own live `content_phash`) must not seed - or join - a phash
+ group, the same staleness rule every other Stage C/D reader in this codebase applies."""
+ card_a = CardFactory(content_phash=1)
+ card_b = CardFactory(content_phash=2)
+ phash_evidence(card_a, phash=555)
+ phash_evidence(card_b, phash=555, content_hash=999) # stale: card_b.content_phash is 2
+
+ assert phash_group_card_ids(card_a) == [card_a.pk]
+
+ def test_null_phash_cohort_forms_no_group(self, db):
+ """
+ The ruling issue #661's own brief calls out by name: cards with no `artbox_phash` are
+ NOT a shared group of NULLs. `_current_artbox_phash_queryset`'s `artbox_phash__isnull=
+ False` filter is what enforces this - a bare `artbox_phash=None` lookup would otherwise
+ match every phash-less card at once, exactly the catastrophic misread this test pins
+ against (the same failure mode `_card_md5_checksum`'s own docstring already warns about
+ for a checksum-less card).
+ """
+ cards = [CardFactory(content_phash=n) for n in range(5)]
+ for card in cards:
+ assert phash_group_card_ids(card) == [card.pk]
+ # and the combined group agrees - no cross-card merge via a shared absence of evidence
+ all_ids = {card_id for card in cards for card_id in identity_group_card_ids(card)}
+ assert all_ids == {card.pk for card in cards}
+
+
+class TestIdentityGroupComposition:
+ def test_group_spanning_multiple_md5s_unions_via_phash(self, db):
+ """The headline issue #661 case: two cards with DIFFERENT checksums (different files) but
+ the SAME artbox_phash (same art) end up in one combined identity group, even though md5
+ alone sees no relation between them at all."""
+ card_a = CardFactory(md5_checksum="checksum-a", content_phash=1)
+ card_b = CardFactory(md5_checksum="checksum-b", content_phash=2)
+ phash_evidence(card_a, phash=777)
+ phash_evidence(card_b, phash=777)
+
+ assert identity_group_card_ids(card_a) == sorted([card_a.pk, card_b.pk])
+ assert md5_group_card_ids(card_a) == [card_a.pk]
+
+ def test_md5_siblings_join_via_checksum_alone(self, db):
+ card_a = CardFactory(md5_checksum="same-bytes", content_phash=1)
+ card_b = CardFactory(md5_checksum="same-bytes", content_phash=1)
+ assert identity_group_card_ids(card_a) == sorted([card_a.pk, card_b.pk])
+
+ def test_chain_through_a_bridging_card_is_covered_by_one_union(self, db):
+ """A-md5-B-phash-C: card_b bridges a checksum edge to card_a and a phash edge to card_c.
+ Per `identity_group_card_ids`'s own docstring this is reachable from card_a WITHOUT an
+ iterative closure pass, because all three rows carry the identical stamped phash here -
+ card_a's own phash lookup already reaches card_c directly."""
+ card_a = CardFactory(md5_checksum="same-bytes", content_phash=1)
+ card_b = CardFactory(md5_checksum="same-bytes", content_phash=1)
+ card_c = CardFactory(md5_checksum="different-bytes", content_phash=2)
+ phash_evidence(card_a, phash=42)
+ phash_evidence(card_b, phash=42)
+ phash_evidence(card_c, phash=42)
+
+ assert identity_group_card_ids(card_a) == sorted([card_a.pk, card_b.pk, card_c.pk])
+
+ def test_identity_group_cards_preserves_callers_own_instance(self, db):
+ card_a = CardFactory(content_phash=1)
+ card_b = CardFactory(content_phash=2)
+ phash_evidence(card_a, phash=9)
+ phash_evidence(card_b, phash=9)
+
+ group = identity_group_cards(card_a)
+ assert any(member is card_a for member in group)
+
+ def test_identity_group_key_agrees_for_phash_only_siblings(self, db):
+ card_a = CardFactory(content_phash=1)
+ card_b = CardFactory(content_phash=2)
+ phash_evidence(card_a, phash=9)
+ phash_evidence(card_b, phash=9)
+
+ assert identity_group_key(card_a) == identity_group_key(card_b) == ("phash", 9)
+
+ def test_checksum_takes_priority_in_the_key_but_group_still_includes_the_checksum_sibling(self, db):
+ card_a = CardFactory(md5_checksum="same-bytes", content_phash=1)
+ card_b = CardFactory(md5_checksum="same-bytes", content_phash=1)
+ phash_evidence(card_a, phash=9)
+
+ assert identity_group_key(card_a) == ("md5", "same-bytes")
+ assert identity_group_card_ids(card_a) == sorted([card_a.pk, card_b.pk])
+
+
+class TestVoteTransferAcrossAPhashGroup:
+ def test_phash_group_spanning_multiple_md5s_transfers_a_vote(self, db):
+ """The measured, headline case from issue #661's brief: two cards, different files
+ (different md5), same art (same artbox_phash at d=0) - votes cast across both members
+ pool exactly as an md5-identical pair's already do."""
+ card_a = CardFactory(md5_checksum="checksum-a", content_phash=1)
+ card_b = CardFactory(md5_checksum="checksum-b", content_phash=2)
+ phash_evidence(card_a, phash=777)
+ phash_evidence(card_b, phash=777)
+ printing = CanonicalCardFactory()
+ human_vote(card_a, printing, "human-1")
+ human_vote(card_b, printing, "human-2")
+
+ assert resolve_printing(card_a) == printing
+ assert resolve_printing(card_b) == printing
+
+ def test_resolution_persists_to_the_phash_sibling_with_no_votes_of_its_own(self, db):
+ card_a = CardFactory(md5_checksum="checksum-a", content_phash=1)
+ card_b = CardFactory(md5_checksum="checksum-b", content_phash=2)
+ phash_evidence(card_a, phash=777)
+ phash_evidence(card_b, phash=777)
+ printing = CanonicalCardFactory()
+ human_vote(card_a, printing, "human-1")
+ human_vote(card_a, printing, "human-2")
+
+ resolve_and_persist_printing(card_a)
+
+ card_b.refresh_from_db()
+ assert card_b.inferred_canonical_card_id == printing.pk
+
+ def test_unrelated_phash_does_not_widen_an_md5_only_group(self, db):
+ """Two md5-identical cards resolve exactly as before when a THIRD, unrelated card happens
+ to carry a phash that matches neither of them - no accidental widening."""
+ card_a = CardFactory(md5_checksum="same-bytes", content_phash=1)
+ card_b = CardFactory(md5_checksum="same-bytes", content_phash=1)
+ unrelated = CardFactory(content_phash=99)
+ phash_evidence(unrelated, phash=12345)
+ printing = CanonicalCardFactory()
+ human_vote(card_a, printing, "human-1")
+ human_vote(card_b, printing, "human-2")
+
+ assert resolve_printing(card_a) == printing
+ assert identity_group_card_ids(unrelated) == [unrelated.pk]
+
+
+class TestCalculatorFamilyDedupeAcrossAPhashGroup:
+ def test_same_calculator_family_across_phash_siblings_collapses_to_one_agent(self, db):
+ """`agent_dedupe_key`'s own invariant, exercised across the NEW (phash) channel: two
+ phash-d0 siblings both carrying the SAME calculator family's vote (a version bump between
+ them) must pool to one event, not two - a phash group must not be able to manufacture
+ apparent independent agreement any more than an md5 group can."""
+ card_a = CardFactory(content_phash=1)
+ card_b = CardFactory(content_phash=2)
+ phash_evidence(card_a, phash=42)
+ phash_evidence(card_b, phash=42)
+ printing = CanonicalCardFactory()
+ machine_vote(card_a, printing, "stage-d-join-key-v1")
+ machine_vote(card_b, printing, "stage-d-join-key-v2")
+
+ assert calculator_family("stage-d-join-key-v1") == calculator_family("stage-d-join-key-v2")
+ assert agent_dedupe_key("stage-d-join-key-v1") == agent_dedupe_key("stage-d-join-key-v2")
+
+ votes, is_group = group_printing_votes(card_a)
+ assert is_group is True
+ vote_tuples = build_group_printing_vote_tuples(votes, pool=is_group)
+ assert len(vote_tuples) == 1
+ assert vote_tuples[0].weight == 0.5
+
+ def test_pooled_machine_weight_alone_still_cannot_resolve_a_phash_group(self, db):
+ card_a = CardFactory(content_phash=1)
+ card_b = CardFactory(content_phash=2)
+ phash_evidence(card_a, phash=42)
+ phash_evidence(card_b, phash=42)
+ printing = CanonicalCardFactory()
+ machine_vote(card_a, printing, "stage-d-join-key-v1")
+ machine_vote(card_b, printing, "stage-d-join-key-v2")
+
+ assert resolve_printing(card_a) is None
+
+ def test_distinct_calculator_families_across_phash_siblings_still_count_separately(self, db):
+ card_a = CardFactory(content_phash=1)
+ card_b = CardFactory(content_phash=2)
+ phash_evidence(card_a, phash=42)
+ phash_evidence(card_b, phash=42)
+ printing = CanonicalCardFactory()
+ machine_vote(card_a, printing, "stage-d-join-key-v1")
+ machine_vote(card_b, printing, "stage-d-fallback-v1")
+
+ votes, is_group = group_printing_votes(card_a)
+ vote_tuples = build_group_printing_vote_tuples(votes, pool=is_group)
+ assert len(vote_tuples) == 2
+
+
+class TestIdentityGroupExpandedCardIds:
+ def test_expands_through_phash_when_no_checksum_present(self, db):
+ card_a = CardFactory(content_phash=1)
+ card_b = CardFactory(content_phash=2)
+ phash_evidence(card_a, phash=9)
+ phash_evidence(card_b, phash=9)
+
+ assert identity_group_expanded_card_ids([card_a.pk]) == {card_a.pk, card_b.pk}
+
+ def test_empty_input_returns_empty(self, db):
+ assert identity_group_expanded_card_ids([]) == set()
+
+ def test_singleton_input_with_no_evidence_returns_itself(self, db):
+ card = CardFactory(content_phash=1)
+ assert identity_group_expanded_card_ids([card.pk]) == {card.pk}
From 0102ec7dfd777e41cf9c314d1640944cde9e12d8 Mon Sep 17 00:00:00 2001
From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com>
Date: Wed, 5 Aug 2026 01:47:02 +0000
Subject: [PATCH 2/2] docs: document the phash distance-0 identity grouping
shipped in PR #695 (issue #661)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Extends docs/theory.md's §4 item 3 (identity-group pooling) as the primary
conceptual home: the identity group pooled for printing-vote consensus is
now the union of md5 checksum equality and artbox-phash distance-0
equality, not md5 alone. Covers why d=0 only (the existing two-threshold
split already reserves it as sound entailment), why a single union
suffices without iterative transitive closure, currency enforcement, the
excluded-not-grouped null cohort, and explicit scope boundaries
(illustration/artist consensus and Stage C+'s own phash propagation tier
are untouched).
Corrects two now-stale claims this shipment invalidated:
- identification-pipeline.md's Stage C+ section previously described
issue #661 as a still-future change to phash's role in Stage C+ itself
(phash-shares-an-illustration). #661 shipped as PR #695, but as a
change to printing_consensus.py's own pooling instead - Stage C+ is
untouched. Corrected in place, cross-referenced to theory.md.
- self-referential-reasoning.md's K8 finding ('evidence is propagated
across the phash notion; independence is enforced across the md5
notion') no longer holds without qualification for printing-vote
pooling specifically, though it remains accurate for artist evidence
and Stage C+. Added a scoped correction rather than rewriting the
finding.
Also updates theory.md's §10a neighbourhood-lookups list and
printing-tags.md's md5-identity-group-pooling bullet (anchor kept
stable) to name printing_consensus.identity_group_card_ids and note
illustration_consensus.resolve_illustration remains md5-only.
Measured against production 2026-08-05 (own verification, not copied
from the task brief): 19,065 phash-d0 groups of size >1 covering 40,493
cards (exact match to the brief); 22,454 cards reachable via phash that
md5 cannot reach; 1,492 cards with no printing vote of their own gain
access to one through the union. The latter two differ from the brief's
20,490/1,511 - reported as directly measured rather than reconciled,
since the brief's own methodology wasn't available to diff against.
No code, test, or behaviour change - #695's implementation is untouched.
---
docs/features/printing-tags.md | 43 ++++++++--
docs/identification-pipeline.md | 29 +++++--
docs/reference/self-referential-reasoning.md | 18 +++++
docs/theory.md | 83 ++++++++++++++++++--
4 files changed, 152 insertions(+), 21 deletions(-)
diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md
index 88f899778..2e51b6d8a 100644
--- a/docs/features/printing-tags.md
+++ b/docs/features/printing-tags.md
@@ -268,13 +268,20 @@ printings, artists, tags, and moderation from one screen.
unchanged here (no persisted `CONTESTED` status exists for printing;
artist's own CONTESTED-vs-UNRESOLVED split is a separate, untouched
raw-outcome-count heuristic).
-- **md5 identity-group pooling**
- (issue #473 PR-3, owner-ratified 2026-07-25; soundness statement in
+- **identity-group pooling (md5 ∪
+ phash-d0)** (issue #473 PR-3, owner-ratified 2026-07-25, md5-only;
+ widened to union in artbox-phash distance-0 by issue #661/PR #695,
+ 2026-08-05; soundness statement in
[`theory.md`](../theory.md)'s §4 item 3): cards sharing a non-null
`Card.md5_checksum` index the **same image file** and are ONE
- identification target, so printing consensus tallies them together.
- `printing_consensus.md5_group_card_ids()` expands a card to its group;
- `build_group_printing_vote_tuples()` builds the group's tally and
+ identification target, so printing consensus tallies them together — and,
+ since PR #695, so do cards sharing a CURRENT `ImageEvidence.artbox_phash`
+ at exact (distance-0) equality, the same sound-entailment tier
+ `docs/theory.md`'s two-threshold split already reserves for phash; a card
+ with neither is a group of one. `printing_consensus.identity_group_card_ids()`
+ expands a card to its combined group (`md5_group_card_ids()` /
+ `phash_group_card_ids()` are its two direct components, still separately
+ named); `build_group_printing_vote_tuples()` builds the group's tally and
`vote_consensus.pool_group_votes()` collapses it by **casting
`anonymous_id`, for every vote — human-backed included**: one agent's
agreeing votes across members become ONE vote (a person answering the
@@ -303,11 +310,35 @@ printings, artists, tags, and moderation from one screen.
affected group; `consensus_recompute` walks each group once;
`question_feed` classifies likely-resolve on the group tally and never
serves a second member of a group a voter has already answered. A card
- with a null or unique checksum is a **group of one**, for which all of
+ with neither a checksum nor a current phash is a **group of one**, for
+ which all of
the above is provably the pre-#473 behavior — which is also every
card until #473's PR-1
populates the column (`LOCAL_FILE` and other checksum-less sources stay
null permanently).
+
+ **The phash-d0 union is narrower in scope than it looks.** It replaces
+ md5-alone in exactly the callers named above (`group_printing_votes`/
+ `resolve_printing`/`resolve_and_persist_printing`,
+ `question_feed.py`'s four answered-set widening helpers,
+ `consensus_recompute.py`'s printing loop) and nowhere else. It does not
+ touch the human-backed gate itself, and it does not extend to
+ illustration or artist consensus — `illustration_consensus.py` and
+ `stage_e_dispatch.py`'s own separately-inlined md5 grouping, and Stage
+ C+'s own phash-d0 vote-propagation tier
+ ([`identification-pipeline.md`'s "Stage C+"
+ section](../identification-pipeline.md#stage-c--the-md5-group-behaves-as-one-unit)),
+ are deliberately untouched — different consumers, different mechanisms.
+ A single union of a card's md5 group and its own phash-d0 group, not an
+ iterative transitive closure, is already the full connected component,
+ because `artbox_phash` is a deterministic function of the image bytes
+ (see `identity_group_card_ids`'s own docstring for the argument).
+ Measured against production 2026-08-05: **19,065** phash-d0 groups of
+ size >1 covering **40,493** cards; **22,454** cards reachable by a
+ phash-d0 sibling their md5 group alone would not reach; **1,492** cards
+ with no printing vote of their own that gain access to one through the
+ union.
+
- **Frontend consumer (funnel round, docs/features/grid-selector.md's
"art-picker FUNNEL" section)**: the two endpoints below are called
from the `/display` rail's Select Version FUNNEL
diff --git a/docs/identification-pipeline.md b/docs/identification-pipeline.md
index a0873fcf1..9f0f93e9f 100644
--- a/docs/identification-pipeline.md
+++ b/docs/identification-pipeline.md
@@ -143,14 +143,27 @@ carrying a resolved `custom-art` / `non-english` tag is skipped. `custom-art` is
the catalogue declaring the image is _not_ a faithful depiction of a printing,
and a checksum must not overturn that.
-**The phash distance-0 tier still runs, second, unchanged.** Its future is
-issue #661: the intended direction is that phash shares an _illustration_ (same
-artwork, possibly a different printing — a near-identity claim at a grain where
-a weaker claim is appropriate), not a printing verdict. Until that is built it
-stays, because it is the only propagation reaching cards with no md5 at all —
-md5 is NULL for every `LOCAL_FILE` source by design and is never invented. Both
-tiers call one propagation engine that takes the grouping as a parameter, so
-adding the illustration-grain tier later is a new grouping, not a restructure.
+**The phash distance-0 tier still runs, second, unchanged.** Issue #661 has
+since landed, as PR #695 (2026-08-05) — but as a change to a **different**
+mechanism than the one this paragraph originally anticipated. What shipped is
+a widening of `printing_consensus.py`'s own identity group (the one
+`group_printing_votes`/`resolve_printing` pool votes across at resolution
+time — see [`theory.md`§4 item
+3](theory.md#4-soundness-mechanisms)) to union in phash-d0 alongside md5. The
+propagation tier described in **this** section is untouched by that change
+and still does exactly what it always did: it propagates a _printing verdict_
+under the casting calculator's own identity, not an illustration identity,
+and it is still the only propagation reaching cards with no md5 at all — md5
+is NULL for every `LOCAL_FILE` source by design and is never invented. The
+"intended direction... phash shares an illustration" framing this paragraph
+used to describe as issue #661's own future is **not** what #661 shipped as;
+an illustration-grain propagation tier for Stage C+, if built, remains future
+work, not something PR #695 delivered (the code's own comment in
+`run_pipeline.py::_propagate_cluster_votes`, "Issue #661 holds the question
+of what phash grouping is FOR," is equally stale as of PR #695 and not yet
+corrected). Both tiers here still call one propagation engine that takes the
+grouping as a parameter, so adding that illustration-grain tier later is
+still a new grouping, not a restructure.
It contains **no pipeline logic of its own**. Each stage below is reached by
importing and calling the thing that already owned it; the command is
diff --git a/docs/reference/self-referential-reasoning.md b/docs/reference/self-referential-reasoning.md
index c51d90eeb..c694a221c 100644
--- a/docs/reference/self-referential-reasoning.md
+++ b/docs/reference/self-referential-reasoning.md
@@ -690,6 +690,24 @@ groups covering **71,208** cards, vs **16,957** md5 groups covering
group. **Evidence is propagated across the phash notion; independence is
enforced across the md5 notion.**
+**Partially narrowed for printing votes, 2026-08-05 (issue #661/PR
+#695).** The asymmetry above is unchanged for artist evidence (this
+finding's own `local_residual_classify.py` channel, still keyed on
+`Card.content_phash`, still outside `pool_group_votes`) and for Stage
+C+'s own printing-verdict propagation tier (also `content_phash`-keyed —
+see [`../identification-pipeline.md`'s "Stage C+"
+section](../identification-pipeline.md#stage-c--the-md5-group-behaves-as-one-unit)).
+It has narrowed specifically for printing-vote **pooling** itself:
+`printing_consensus.py`'s identity group
+([`../theory.md`](../theory.md)'s §4 item 3) now unions in
+`ImageEvidence.artbox_phash` distance-0 equality — a different, narrower
+hash (the art-box crop only, not the whole card) than the
+`Card.content_phash` this finding measures, so the two grains still do
+not coincide exactly, but independence is no longer enforced across md5
+alone for that one channel. Not a general resolution of K8: the finding's
+core claim — evidence propagates on a wider relation than independence is
+enforced on — remains true pipeline-wide, artist consensus included.
+
#### K9 — The OCR lexicon contains names our own voters typed
LIVE, small. `MPCAutofill/cardpicker/collector_line_artist.py`,
diff --git a/docs/theory.md b/docs/theory.md
index fa44437fa..13adbe9d7 100644
--- a/docs/theory.md
+++ b/docs/theory.md
@@ -355,6 +355,71 @@ turns out to be:
above is the identity — the pre-2026-07-25 per-record behavior,
unchanged.
+ **Widened to a second identity relation, 2026-08-05** (issue #661, PR
+ #695; `printing_consensus.identity_group_card_ids` and its siblings —
+ `identity_group_cards`, `identity_group_key`,
+ `identity_group_expanded_card_ids`): the pooling target above is no
+ longer md5 checksum equality alone. `ImageEvidence.artbox_phash` (a
+ 64-bit perceptual hash of the card's own art-box crop, populated by a
+ whole-catalogue Stage C pass rather than being upload-time metadata
+ like `Card.md5_checksum`) is now unioned into the same identity group
+ **at distance 0 only** — exact equality, never the narrowing-only
+ Hamming threshold `find_best_match`'s 20/5 cutoffs use elsewhere in
+ this pipeline. This is the entailment tier item 1 above already
+ reserves for phash: d=0 propagates as sound entailment ("literally the
+ same uploaded image, transitively true"), the same tier md5 checksum
+ equality occupies, so widening the pooling target to include it is
+ applying an existing rule, not adding a new one. Two files that are
+ NOT byte-identical (a re-encode, a fresh re-upload, a genuine reprint
+ using the same digital art asset) can share this hash where they would
+ never share an md5 checksum — which is exactly why the union reaches
+ cards the md5-alone group could not.
+
+ A single union of a card's md5 group and its own phash-d0 group — not
+ an iterative transitive closure — is already the full connected
+ component, because `artbox_phash` is a deterministic function of the
+ image bytes: any md5-clique member that also carries a current phash
+ necessarily carries the _same_ phash value as the rest of the clique,
+ so a phash edge reachable from one member is reachable directly from
+ any other member without visiting it first (see
+ `identity_group_card_ids`'s own docstring for the full argument, and
+ its noted tolerance: an extractor-version bump straddling two
+ byte-identical siblings' extraction times could under-group them in
+ the worst case — the same safe direction `agent_dedupe_key`'s own
+ version-bump handling elsewhere in this module accepts — never falsely
+ merge two genuinely different targets). Currency is enforced the same
+ way `modern_artist_credit.eligible_evidence_queryset` already enforces
+ it in bulk (`evidence_transfer.md5_currency_q` plus
+ `content_hash=F("card__content_phash")`): a stale `ImageEvidence` row
+ — one written before the card's current image — never seeds or joins a
+ group. A card with no current `artbox_phash` is excluded from phash
+ grouping entirely, a group of one, never collapsed into a shared
+ bucket with every other phash-less card.
+
+ This widening changes what pools, not what resolves. It replaces
+ md5-alone in `group_printing_votes`/`resolve_printing`/
+ `resolve_and_persist_printing` and in `question_feed.py`'s four
+ answered-set widening helpers (a voter who answered one member of a
+ phash-d0 group is not re-asked the same art under a sibling's
+ identifier either), and `consensus_recompute.py`'s printing-recompute
+ loop now walks the combined group instead of the md5-only one. It does
+ **not** touch the human-backed gate above (`g₅`/item 2 are unchanged),
+ and it does **not** extend to illustration or artist consensus:
+ `illustration_consensus.py` and `stage_e_dispatch.py`'s own
+ separately-inlined md5 grouping, and Stage C+'s own phash-d0
+ vote-propagation tier
+ ([`identification-pipeline.md`'s "Stage C+"
+ section](identification-pipeline.md#stage-c--the-md5-group-behaves-as-one-unit)),
+ are deliberately untouched by this change — different consumers,
+ different mechanisms, out of this widening's scope.
+
+ Measured against production the day this landed: **19,065** phash-d0
+ groups of size >1, covering **40,493** cards; of those, **22,454**
+ cards have at least one phash-sibling their md5 group alone would not
+ reach; **1,492** cards had no printing vote reachable through their
+ own md5 group but gain access to one through the union. Like §§7-10,
+ this text is pending the same owner review §§1-6 received.
+
Together these mean the system's worst-case failure mode, even under a
badly miscalibrated engine, is a wasted human review cycle (a bad
suggestion surfaced for confirmation) — never a silent wrong answer
@@ -1069,18 +1134,22 @@ neighbourhood is defined by a **join key** — an md5 checksum, a
perceptual hash, a name — and not by the batch. The set of cards
sharing a given checksum is the same set no matter which cards happen
to be dispatched together. Five live instances:
-`printing_consensus.md5_group_card_ids` (every card indexing a
-byte-identical image file — §4 item 3's identity group);
+`printing_consensus.identity_group_card_ids` (every card indexing a
+byte-identical image file, unioned since issue #661/PR #695 with every
+card sharing a current artbox-phash at distance 0 — §4 item 3's identity
+group, widened);
`evidence_transfer.find_transfer_source` (the md5-sibling
`ImageEvidence` row eligible to be copied onto a card instead of
re-fetching it); `local_residual_classify.run_d0_sibling_artist_propagation`
(an artist propagated from a `content_phash`-sharing sibling); and
-consensus resolution itself, on two vote types —
+consensus resolution itself, on two vote types, no longer sharing one
+grouping definition as they did before PR #695 —
`printing_consensus.resolve_printing` reads a card's votes **pooled
-across its md5 group**, via `group_printing_votes`, and
-`illustration_consensus.resolve_illustration` does the same through
-`group_illustration_votes` (sharing this file's md5 primitives rather
-than reimplementing them, so there is one definition of a group).
+across its combined md5-union-phash-d0 identity group**, via
+`group_printing_votes`, while `illustration_consensus.resolve_illustration`
+still pools through `group_illustration_votes` over the **md5-only**
+group (this module's `md5_group_card_ids`/`md5_group_cards` primitives,
+deliberately untouched by #661/#695 — see §4 item 3's own note on scope).
The illustration case also carries the propagation the other four make
available: because the tally is defined over the group and