feat(printing-consensus): phash distance-0 grouping unions into the md5 identity group (issue #661) - #695
Merged
Merged
Conversation
…d5 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.
(issue #661) 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
identity_group_card_ids/identity_group_cards/identity_group_key/identity_group_expanded_card_idstoprinting_consensus.py: the pre-existing md5 identity group (issue md5 identity groups: checksum substrate, evidence transfer, and group-level vote pooling #473, byte identity) UNIONED with a new artbox-phash-distance-0 group (ImageEvidence.artbox_phash, perceptual identity of the art-box crop) -cardincluded, sorted, deduplicated. A single union, not an iterative transitive closure:artbox_phashis a deterministic function of the image bytes, so any md5-clique member that also carries a current phash already shares that exact phash value with the rest of the clique, so one md5 lookup plus one phash lookup rooted at the target card already reaches the full connected component (identity_group_card_ids's own docstring works this argument in full, including the "safe under-grouping, never false-merge" caveat for an extractor-version bump straddling two siblings' extraction times).phash_group_card_ids/_card_artbox_phash/_artbox_phashes_for_card_ids/_card_ids_with_artbox_phashes/_current_artbox_phash_querysetmirror the shape of the existing md5 helpers, but readImageEvidence(a related model, one query, not aCardfield) rather thangetattr, and are CURRENCY-checked the same bulk waymodern_artist_credit.eligible_evidence_querysetalready does (content_hash=F("card__content_phash")+ the shared, null-tolerantevidence_transfer.md5_currency_q()) - a staleImageEvidencerow (the card's image has changed since that row was written) never seeds or joins a group._require_full_identity_groupmirrors_require_full_md5_group's completeness guard against the new combined group.group_printing_votes/resolve_printing/resolve_and_persist_printingnow derive/validate the COMBINED identity group instead of the md5-only one - this is the actual pooling wire-up: a resolution reached on one member (machine or human votes, weighted/deduped exactly asvote_consensus/agent_dedupe_keyalready do) is written to every member, phash-only siblings included.question_feed.py's four per-voter/per-card answered-set widening helpers (_voter_answered_printing_card_ids,_voter_answered_artist_card_ids,_voter_answered_tag_card_ids_by_tag,_not_official_art_card_ids) now widen throughidentity_group_expanded_card_idsinstead ofmd5_group_expanded_card_ids- a voter who answers one member of a phash-d0 group is not re-served the same art under a sibling's identifier, same as md5 already guaranteed.consensus_recompute.py's printing recompute loop visits each combined group once (identity_group_key/identity_group_cardsin place of the md5-only pair) instead of once per md5 group ---dry-run/--applyboth covered.Cards with no artbox_phash are NOT a group of NULLs:_current_artbox_phash_queryset'sartbox_phash__isnull=Falsefilter is what enforces this; pinned directly bytest_phash_group_pooling.py::TestPhashGroupCardIds::test_null_phash_cohort_forms_no_group.Why a union, not a separate mechanism, and why d=0 only
image_evidence.py's own "SOUNDNESS NOTE FOR ANY FUTURE CONSUMER" anddocs/theory.md§4's two-threshold split both reserve exact (d=0) phash equality, and ONLY d=0, as sound vote-pooling entailment - "the same uploaded image, transitively true" - while any Hamming-distance threshold match (find_best_match's 20/5 cutoffs, issue #419's open recalibration) stays narrowing-only. This PR implements exactly that pre-ratified exception, nothing wider.The real tension the brief for this issue calls out: md5 equality means byte-identical (unambiguously one printing); phash equality at d=0 means the art-box CROP is perceptually identical, which two genuinely different printings sharing the same digital art asset could in principle also produce. This PR does not add new gating against that beyond what the ratified d=0 exception already accepts - a d=0 collision between two real, different printings is exactly as rare/tight a bound as any other 64-bit exact-hash match, and the composition is additive (UNION with md5, never a replacement or a separate pooling path), so the blast radius of being wrong about one d=0 match is bounded to that one group, exactly as it already is for md5. Measured against production 2026-08-04 (see issue #661): 19,065 phash groups >1 member (40,493 cards), 20,490 cards phash-grouped but md5-unique (the incremental population), 10,078 groups spanning >1 md5, 1,348 of those with a mixed vote state, 1,511 cards that would gain a vote from transfer - modest immediate yield; the larger value is duplicate copies no longer each running full Stage D independently.
Deviations from the brief
illustration_consensus.pyandstage_e_dispatch.py's own inlined md5 grouping are deliberately UNTOUCHED - both are separate consumers/mechanisms (illustration-tag consensus; the verdict-transfer dispatch queue, which by its own docstring never imports the consensus layer at all) and out of the brief's explicit framing (printing votes + question feed only). Noted as an open item below, not silently dropped.test_md5_group_pooling.py::TestSingletonIsANoOppinned is now "1 extra query":artbox_phashlives onImageEvidence, a related model, so checking it costs a query no matter what the md5 checksum lookup (agetattr, free) found. Updated that test's assertion + docstring to1, rather than skip the phash check for checksum-less cards, since skipping it would silently under-group every checksum-less card in the catalogue - the wrong direction to save one query._require_full_identity_group'sValueErrormessage text is deliberately close to (not a link-out from)_require_full_md5_group's wording -test_md5_group_pooling.py::TestCallerSuppliedGroupMustBeComplete::test_a_strict_subset_raisespins specific substrings ("DIFFERENT one", "DIFFERENT WINNING PRINTING", "group_card_ids=None") that bothgroup_printing_votes/resolve_printingnow raise via the identity-group guard instead of the md5-only one.Test plan
cardpicker/tests/test_phash_group_pooling.py(new, 19 tests):_current_artbox_phash_querysetcurrency (stale row excluded), null-phash-cohort-forms-no-group, phash group spanning multiple md5s (both the group-ids proof and an actualresolve_printing/resolve_and_persist_printingvote-transfer proof, including persistence to a phash sibling with zero votes of its own), a 2-hop md5-then-phash chain reachable via a single union,identity_group_keypriority/agreement, calculator-family dedupe collapsing correctly across a phash-only group (and staying withheld from resolving on machine weight alone), distinct calculator families still counting separately, andidentity_group_expanded_card_ids(empty/singleton/phash-only expansion).cardpicker/tests/test_md5_group_pooling.py(91 tests, 2 updated for the query-cost/error-message deviations above) - full pre-existing md5-only surface still green.cardpicker/tests/test_question_feed.py(71 combined with consensus_recompute below) - answered-set widening callers.cardpicker/tests/test_consensus_recompute.py- printing recompute dry-run/apply loop.cardpicker/tests/test_illustration_consensus.py,test_evidence_transfer.py,test_backfill_modern_artist_names.py- untouched consumers of the md5 helpers /ImageEvidencecurrency pattern this PR reuses, confirmed no regression (87 tests).pre-commit run(ruff, isort, black, mypy, prettier) green on all five touched/added files.Documentation
Extends
docs/theory.md§4 item 3 anddocs/features/printing-tags.md's identity-group-pooling entry to describe the pooled group as md5 ∪ artbox-phash-d0, not md5 alone; correctsdocs/identification-pipeline.md's Stage C+ section (previously described issue #661 as a still-pending change to phash's role in Stage C+ itself) anddocs/reference/self-referential-reasoning.md's K8 finding (scoped: no longer holds unqualified for printing-vote pooling) to match what actually shipped. Measured independently against production 2026-08-05: 19,065 phash-d0 groups >1 member covering 40,493 cards (matches the issue's own figure); 22,454 cards reachable via phash that md5 alone cannot reach; 1,492 cards with no printing vote of their own gain one through the union (both differ from the issue's 20,490/1,511 - reported as directly measured, not reconciled, since the issue's own methodology wasn't available to diff against).