From 74322bf6b05bb6db746c491f426340e58e72878f Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Sat, 25 Jul 2026 18:54:03 +0000 Subject: [PATCH 1/4] Pool printing-consensus votes across md5 identity groups Co-Authored-By: Claude Fable 5 --- .../commands/consensus_recompute.py | 70 ++- MPCAutofill/cardpicker/printing_consensus.py | 288 ++++++++--- MPCAutofill/cardpicker/question_feed.py | 79 ++- .../tests/test_md5_group_pooling.py | 467 ++++++++++++++++++ MPCAutofill/cardpicker/vote_consensus.py | 65 +++ docs/features/printing-tags.md | 24 + docs/theory.md | 38 +- 7 files changed, 932 insertions(+), 99 deletions(-) create mode 100644 MPCAutofill/cardpicker/tests/test_md5_group_pooling.py diff --git a/MPCAutofill/cardpicker/management/commands/consensus_recompute.py b/MPCAutofill/cardpicker/management/commands/consensus_recompute.py index fe4047870..22c6cdb85 100644 --- a/MPCAutofill/cardpicker/management/commands/consensus_recompute.py +++ b/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: diff --git a/MPCAutofill/cardpicker/printing_consensus.py b/MPCAutofill/cardpicker/printing_consensus.py index 1e3d04c90..86083ccc4 100644 --- a/MPCAutofill/cardpicker/printing_consensus.py +++ b/MPCAutofill/cardpicker/printing_consensus.py @@ -1,5 +1,5 @@ from dataclasses import dataclass -from typing import Iterable, Literal, TypedDict +from typing import Hashable, Iterable, Literal, Sequence, TypedDict from django.conf import settings @@ -8,12 +8,102 @@ VoteTuple, contested_queryset, is_human_backed_source, + pool_group_votes, resolve_vote_weight, resolve_weighted_consensus, ) NO_MATCH: Literal["NO_MATCH"] = "NO_MATCH" +# `Card.md5_checksum` (the Drive-API-reported checksum of the image file this catalogue row +# indexes), referenced by NAME rather than as an attribute so this module is importable and +# correct both before and after that field exists: it is added by issue #473's PR-1 +# (`md5-checksum-substrate`), which this branch is cut BEFORE and merges AFTER. Every read of it +# funnels through `_card_md5_checksum`/`_card_ids_with_md5_checksums` below - the only two places +# in this module that touch the column - so on a checkout without the field every card is a +# group of one and every group-aware path below degenerates, provably, to its pre-#473 behavior. +MD5_CHECKSUM_FIELD = "md5_checksum" + + +def _card_md5_checksum(card: Card) -> str | None: + """ + `card`'s file checksum, or `None` when it has none (`LOCAL_FILE` and other checksum-less + sources, per issue #473 ruling 3) - and, until PR-1 lands, for every card, since the field + doesn't exist yet and `getattr` reports the default. Empty string is normalized to `None`: + "" is not an identity, and grouping every checksum-less card into one giant group would be a + catastrophic misreading of exactly this degenerate case. + """ + return getattr(card, MD5_CHECKSUM_FIELD, None) or None + + +def _card_ids_with_md5_checksums(checksums: set[str]) -> list[int]: + """ + Every `Card.pk` whose checksum is in `checksums` - the one query in this module that filters + ON the column (see `MD5_CHECKSUM_FIELD`). Unreachable while the field doesn't exist, because + its only callers below skip it when they hold no non-null checksum, and `_card_md5_checksum` + can only report non-null once PR-1 has added the column. + """ + return list(Card.objects.filter(**{f"{MD5_CHECKSUM_FIELD}__in": checksums}).values_list("pk", flat=True)) + + +def md5_group_key(card: Card) -> Hashable: + """ + Stable identity of `card`'s md5 group, for callers that need to visit each group ONCE across + a large iteration (`consensus_recompute`) rather than re-resolving the same group per member. + A checksum-less card keys on its own pk, so it is always a group of one and never collides + with another card's key. + """ + checksum = _card_md5_checksum(card) + return ("md5", checksum) if checksum is not None else ("card", card.pk) + + +def md5_group_card_ids(card: Card) -> list[int]: + """ + The pks of `card`'s md5 identity group - every card indexing a byte-identical image file, + `card` included - sorted, so the tally built from it is deterministic. `[card.pk]` for a + checksum-less or unique-checksum card (issue #473 ruling 3's group of one), which is also + the shape every card has before PR-1 adds the checksum column. + """ + checksum = _card_md5_checksum(card) + if checksum is None: + return [card.pk] + return sorted(set(_card_ids_with_md5_checksums({checksum})) | {card.pk}) + + +def md5_group_cards(card: Card) -> list[Card]: + """ + `card`'s md5 group as `Card` INSTANCES, with the caller's own `card` object first and + unreplaced - `resolve_and_persist_printing` writes through these instances, and its callers + (e.g. `consensus_recompute`, the vote-submission views) read `card.printing_tag_status` off + their own object afterwards, so substituting a freshly-fetched copy of the same row would + silently strand them on a stale status. A group of one performs no query at all. + """ + group_card_ids = md5_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 md5_group_expanded_card_ids(card_ids: Iterable[int]) -> set[int]: + """ + `card_ids` widened to include every md5 sibling of every card in it - "the cards this voter + has already answered" widened to "the identity groups this voter has already answered", for + `question_feed`'s serve-one-member-per-group exclusion. Returns `card_ids` unchanged when + none of them carry a checksum (which, before PR-1, is always). + """ + ids = set(card_ids) + if not ids: + return ids + checksums = { + checksum + for checksum in (_card_md5_checksum(card) for card in Card.objects.filter(pk__in=ids)) + if checksum is not None + } + if not checksums: + return ids + return ids | set(_card_ids_with_md5_checksums(checksums)) + @dataclass(frozen=True) class ResolvedPrinting: @@ -56,28 +146,57 @@ def get_resolved_printings(identifiers: Iterable[str]) -> dict[str, ResolvedPrin return result -def resolve_printing(card: Card) -> CanonicalCard | Literal["NO_MATCH"] | None: +def group_printing_votes(card: Card, group_card_ids: Sequence[int] | None = None) -> tuple[list[CardPrintingTag], bool]: """ - Reconciles all `CardPrintingTag` votes cast against `card` 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 - weighting/threshold rules (votes weighted by `source`, `PRINTING_TAG_MIN_VOTES`/ - `MIN_SHARE` gates, non-AI gate) - this is a thin wrapper translating `CardPrintingTag` - rows into `VoteTuple`s and the winning outcome key back into a `CanonicalCard`. + Every `CardPrintingTag` row cast against any member of `card`'s md5 identity group, plus + whether that group actually has more than one member. Pass `group_card_ids` when the caller + already knows the group (e.g. it is about to persist to those same members) to avoid + re-deriving it. + + A group of ONE reads `card.printing_tags.all()` - deliberately the identical expression + this module used before issue #473, not a `filter(card_id__in=[card.pk])` that happens to + return the same rows: that expression is what honours a caller's own + `prefetch_related("printing_tags")` (`consensus_recompute` batches on exactly that, one + query per batch instead of one per card), and keeping it is what makes the singleton case a + byte-for-byte no-op in query shape as well as in outcome. The multi-member branch orders by + `(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) + if len(group_card_ids) <= 1: + return list(card.printing_tags.all()), False + votes = list( + CardPrintingTag.objects.filter(card_id__in=group_card_ids).select_related("printing").order_by("card_id", "pk") + ) + return votes, True + + +def build_group_printing_vote_tuples( + votes: Iterable[CardPrintingTag], pool: bool, printings_by_id: dict[int, CanonicalCard] | None = None +) -> list[VoteTuple]: + """ + Translates `CardPrintingTag` rows into the `VoteTuple`s `resolve_weighted_consensus` reads, + pooling them across an md5 identity group when `pool` is True (issue #473 ruling 1, applied + by `vote_consensus.pool_group_votes`): every non-human-backed vote is keyed on the + `anonymous_id` of the agent that cast it, so one machine agent's verdict about a set of + byte-identical images is ONE event at its maximum weight no matter how many members carry a + copy of it, while human-backed votes stay unkeyed and therefore sum, being genuinely + independent people looking at the image. With `pool=False` (a group of one) no vote is + keyed, `pool_group_votes` is never called, and the returned list is exactly what this + module built before #473. + + Passing a `printings_by_id` dict populates it with each voted `CanonicalCard` (needed to map + a winning outcome key back to a printing). Callers that only need the outcome KEY - e.g. + `question_feed.is_likely_resolve_printing`, which runs this in a scan loop - pass `None` and + this never touches `vote.printing`, so it costs no per-vote related lookup for them. Per-vote weight is resolved via `vote_consensus.resolve_vote_weight` (not a bare `_SOURCE_WEIGHTS[vote.source]` lookup) so the 2026-07-23 owner ruling zeroing the deductive-backfill cohort's weight (see that function's own docstring) is honoured here - the one call site every printing consensus computation (winner selection, the gate checks - and share math inside `resolve_weighted_consensus`, and every caller of this function, + and share math inside `resolve_weighted_consensus`, and every caller of `resolve_printing`, including `consensus_impact_report`/`consensus_recompute`) ultimately funnels through. """ - votes = list(card.printing_tags.all()) - if not votes: - return None - - printings_by_id: dict[int, CanonicalCard] = {} vote_tuples: list[VoteTuple] = [] for vote in votes: key: int | Literal["NO_MATCH"] @@ -86,16 +205,46 @@ def resolve_printing(card: Card) -> CanonicalCard | Literal["NO_MATCH"] | None: else: # guaranteed non-null here by the model's printing_xor_no_match CheckConstraint assert vote.printing_id is not None - assert vote.printing is not None key = vote.printing_id - printings_by_id[vote.printing_id] = vote.printing + if printings_by_id is not None: + assert vote.printing is not None + printings_by_id[vote.printing_id] = vote.printing + is_human_backed = is_human_backed_source(vote.source) vote_tuples.append( VoteTuple( outcome_key=key, weight=resolve_vote_weight(vote.source, vote.anonymous_id), - is_human_backed=is_human_backed_source(vote.source), + is_human_backed=is_human_backed, + dedupe_key=None if (is_human_backed or not pool) else vote.anonymous_id, ) ) + return pool_group_votes(vote_tuples) if pool else vote_tuples + + +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 + 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 + weighting/threshold rules (votes weighted by `source`, `PRINTING_TAG_MIN_VOTES`/ + `MIN_SHARE` gates, non-AI 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. `group_card_ids` may be passed by a caller that already derived the group. + """ + votes, is_group = group_printing_votes(card, group_card_ids) + if not votes: + return None + + printings_by_id: dict[int, CanonicalCard] = {} + vote_tuples = build_group_printing_vote_tuples(votes, pool=is_group, printings_by_id=printings_by_id) winning_key = resolve_weighted_consensus( vote_tuples, min_weight=settings.PRINTING_TAG_MIN_VOTES, min_share=settings.PRINTING_TAG_MIN_SHARE @@ -121,49 +270,68 @@ def _effective_indexed_printing_id(status: str, printing_id: int | None) -> int return printing_id if status == PrintingTagStatus.RESOLVED else None -def resolve_and_persist_printing(card: Card) -> CanonicalCard | Literal["NO_MATCH"] | None: - """ - Runs `resolve_printing(card)` and writes the outcome onto `card.inferred_canonical_card` - and `card.printing_tag_status` together, so that `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) both - stay in sync with the latest votes. Intended to be called synchronously right after a - vote is submitted for `card` - cheap, since it only touches this one card's own votes. - Returns the same outcome `resolve_printing` returned, so callers don't need to - recompute it again immediately afterwards. - - Also pushes `card` into Elasticsearch, but only when the outcome actually changes what's - indexed (see `_effective_indexed_printing_id`) - entering RESOLVED, leaving RESOLVED - (contested/unresolved again after new votes), or the resolved printing itself changing - while remaining RESOLVED. A re-resolve that lands on the same outcome as before (the - common case whenever this runs against a card that already has a settled consensus) does - not touch the index. The push itself is failure-isolated (`reindex_card_safely`) - an ES - hiccup is logged, never raised; this function's own DB write has already committed by - that point regardless. - """ - prior_status = card.printing_tag_status - prior_printing_id = card.inferred_canonical_card_id - prior_effective = _effective_indexed_printing_id(prior_status, prior_printing_id) - - result = resolve_printing(card) - if result is None: - card.inferred_canonical_card = None - card.printing_tag_status = PrintingTagStatus.UNRESOLVED - elif result == NO_MATCH: - card.inferred_canonical_card = None - card.printing_tag_status = PrintingTagStatus.NO_MATCH - else: - card.inferred_canonical_card = result - card.printing_tag_status = PrintingTagStatus.RESOLVED - card.save(update_fields=["inferred_canonical_card", "printing_tag_status"]) - - new_effective = _effective_indexed_printing_id(card.printing_tag_status, card.inferred_canonical_card_id) - if new_effective != prior_effective: - from cardpicker.documents import ( - reindex_card_safely, # local import - avoids a top-level ES dependency in this module - ) +def resolve_and_persist_printing( + card: Card, members: Sequence[Card] | None = None +) -> 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 + 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). + + 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, + leaving RESOLVED (contested/unresolved again after new votes), or the resolved printing + itself changing while remaining RESOLVED. A re-resolve that lands on the same outcome as + before (the common case whenever this runs against a card that already has a settled + consensus) does not touch the index; the per-member gate means propagating an unchanged + outcome across a group reindexes only the members that were actually out of step. The push + itself is failure-isolated (`reindex_card_safely`) - an ES hiccup is logged, never raised; + this function's own DB write has already committed by that point regardless. + + Members are WRITTEN in pk order (not in the caller-first order they arrive in), so two + concurrent votes landing on two different members of the same group take that group's row + 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) + result = resolve_printing(card, group_card_ids=[member.pk for member in group_cards]) + + for member in sorted(group_cards, key=lambda group_card: group_card.pk): + prior_status = member.printing_tag_status + prior_printing_id = member.inferred_canonical_card_id + prior_effective = _effective_indexed_printing_id(prior_status, prior_printing_id) + + if result is None: + member.inferred_canonical_card = None + member.printing_tag_status = PrintingTagStatus.UNRESOLVED + elif result == NO_MATCH: + member.inferred_canonical_card = None + member.printing_tag_status = PrintingTagStatus.NO_MATCH + else: + member.inferred_canonical_card = result + member.printing_tag_status = PrintingTagStatus.RESOLVED + member.save(update_fields=["inferred_canonical_card", "printing_tag_status"]) + + new_effective = _effective_indexed_printing_id(member.printing_tag_status, member.inferred_canonical_card_id) + if new_effective != prior_effective: + from cardpicker.documents import ( + reindex_card_safely, # local import - avoids a top-level ES dependency in this module + ) - reindex_card_safely(card) + reindex_card_safely(member) return result diff --git a/MPCAutofill/cardpicker/question_feed.py b/MPCAutofill/cardpicker/question_feed.py index 2d1737f95..e827df3ab 100644 --- a/MPCAutofill/cardpicker/question_feed.py +++ b/MPCAutofill/cardpicker/question_feed.py @@ -41,6 +41,18 @@ recorded in `QuestionFeedServedLog` - the bias-conditioning record the data brief's SOUNDNESS NOTE calls for, so a future audit can correlate click behavior against a session's easy-question exposure. See `_served_mix_ratio`/`_log_served` below. + +md5 identity groups (issue #473, owner-ratified 2026-07-25): a set of cards indexing a +byte-identical image file is ONE identification target, so this feed asks about it once. Two +consequences here, both delegated to `printing_consensus` rather than reimplemented: the +likely-resolve classification reads the GROUP's pooled tally (`_printing_vote_tuples` -> +`group_printing_votes`/`build_group_printing_vote_tuples`), and every printing tier excludes +the full identity group of every card this voter has already answered +(`_voter_answered_printing_card_ids`), so a voter is served at most one member per group rather +than N copies of the same question. Both degenerate exactly to the pre-#473 behavior for a card +whose group is itself alone, which - until that issue's PR-1 populates `Card.md5_checksum` - is +every card in the catalogue. The artist and tag tiers are untouched by this: identity grouping +is a statement about the IMAGE FILE, and those questions are already keyed differently. """ from collections import defaultdict @@ -68,6 +80,7 @@ from cardpicker.models import ( ArtistVoteStatus, Card, + CardPrintingTag, CardScanLog, CardTagVote, PrintingTagStatus, @@ -78,12 +91,16 @@ VoteSource, ) from cardpicker.printing_candidates import get_ranked_printing_candidates -from cardpicker.printing_consensus import NO_MATCH, get_contested_card_ids +from cardpicker.printing_consensus import ( + build_group_printing_vote_tuples, + get_contested_card_ids, + group_printing_votes, + md5_group_expanded_card_ids, +) from cardpicker.schema_types import QuestionFeedCounts, QuestionFeedItem, TypeEnum from cardpicker.tag_consensus import get_tag_net_polarity, get_tag_review_queue_pairs from cardpicker.vote_consensus import ( VoteTuple, - is_human_backed_source, resolve_vote_weight, resolve_weighted_consensus, ) @@ -167,28 +184,46 @@ def _tag_item(card: Card, tag_name: str) -> QuestionFeedItem: def _printing_vote_tuples(card: Card) -> list[VoteTuple]: """ - Builds `VoteTuple`s for `card`'s current `CardPrintingTag` rows - the exact same per-vote - weight/human-backed resolution `printing_consensus.resolve_printing` uses - (`resolve_vote_weight`/`is_human_backed_source`, both imported from `vote_consensus` rather - than reimplemented), just without that function's private printing-lookup bookkeeping this - caller doesn't need (only the outcome KEY, an int pk or the `NO_MATCH` sentinel, matters - for the likely-resolve check below). + Builds `VoteTuple`s for the current `CardPrintingTag` rows of `card`'s md5 IDENTITY GROUP - + every card indexing a byte-identical image file, `card` included (issue #473) - by calling + `printing_consensus.group_printing_votes`/`build_group_printing_vote_tuples`, the exact + functions `printing_consensus.resolve_printing` itself uses, so the group expansion, the + per-vote weight/human-backed resolution, and the machine-evidence pooling are one shared + implementation rather than a second copy that could drift from the real resolver. Passing no + `printings_by_id` keeps this off `vote.printing` entirely: only the outcome KEY (an int pk or + the `NO_MATCH` sentinel) matters for the likely-resolve check below, and this runs in a scan + loop. A group of one yields exactly the tuples this function built before #473. """ - return [ - VoteTuple( - outcome_key=NO_MATCH if vote.is_no_match else vote.printing_id, - weight=resolve_vote_weight(vote.source, vote.anonymous_id), - is_human_backed=is_human_backed_source(vote.source), - ) - for vote in card.printing_tags.all() - ] + votes, is_group = group_printing_votes(card) + return build_group_printing_vote_tuples(votes, pool=is_group) + + +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). + + 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 + same set of cards, expressed as pks) - which, for a checksum-less catalogue, is every card. + One indexed query, plus the expansion's own (at most) two - the tiers each scan or `.first()` + over the result, so this is not a per-candidate cost. + """ + 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) def is_likely_resolve_printing(card: Card) -> bool: """ True when ONE hypothetical additional agreeing human vote (`VoteSource.USER` weight) added - to `card`'s current highest-weighted printing outcome group would resolve it under the REAL - resolver (`vote_consensus.resolve_weighted_consensus` - the same function + to the current highest-weighted printing outcome group of `card`'s md5 IDENTITY GROUP (issue + #473 - the pooled tally of every byte-identical sibling, via `_printing_vote_tuples`, not + this one card's rows in isolation; a group one human vote from resolving is likely-resolve + for every member of it, and resolving it resolves all of them) would resolve it under the + REAL resolver (`vote_consensus.resolve_weighted_consensus` - the same function `printing_consensus.resolve_printing` calls; this never reimplements its weight/threshold arithmetic). This is the serve-time LIKELY-RESOLVE classification the 2026-07-24 data brief's exact-code simulation approach specifies (the same method that produced its @@ -249,7 +284,7 @@ def _likely_resolve_printing_card(anonymous_id: str) -> Optional[Card]: """ candidates = ( Card.objects.filter(printing_tag_status=PrintingTagStatus.UNRESOLVED, printing_tags__isnull=False) - .exclude(printing_tags__anonymous_id=anonymous_id) + .exclude(pk__in=_voter_answered_printing_card_ids(anonymous_id)) .distinct() .order_by("date_created") ) @@ -278,7 +313,7 @@ def _tier_1_confirm_suggestion(anonymous_id: str) -> Optional[QuestionFeedItem]: printing_tags__source__in=[VoteSource.DEDUCTION, VoteSource.OCR], ) .exclude(printing_tags__source__in=[VoteSource.USER, VoteSource.ADMIN, VoteSource.FEDERATED]) - .exclude(printing_tags__anonymous_id=anonymous_id) + .exclude(pk__in=_voter_answered_printing_card_ids(anonymous_id)) .distinct() .order_by("date_created") ) @@ -292,7 +327,7 @@ def _tier_1_confirm_suggestion(anonymous_id: str) -> Optional[QuestionFeedItem]: def _tier_2_contested(anonymous_id: str) -> Optional[tuple[QuestionFeedItem, str]]: printing_card = ( Card.objects.filter(printing_tag_status=PrintingTagStatus.UNRESOLVED, pk__in=get_contested_card_ids()) - .exclude(printing_tags__anonymous_id=anonymous_id) + .exclude(pk__in=_voter_answered_printing_card_ids(anonymous_id)) .order_by("-date_created") .first() ) @@ -362,7 +397,7 @@ def _tier_4_fresh(anonymous_id: str) -> Optional[tuple[QuestionFeedItem, str]]: printing_card = ( Card.objects.filter(printing_tag_status=PrintingTagStatus.UNRESOLVED) .exclude(pk__in=get_contested_card_ids()) - .exclude(printing_tags__anonymous_id=anonymous_id) + .exclude(pk__in=_voter_answered_printing_card_ids(anonymous_id)) .annotate(vote_count=Count("printing_tags", distinct=True)) .annotate(origin_reason=_latest_stage_d_origin_reason_subquery()) .annotate( diff --git a/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py b/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py new file mode 100644 index 000000000..f585190ab --- /dev/null +++ b/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py @@ -0,0 +1,467 @@ +""" +Group-level vote pooling for md5 identity groups (issue #473 PR-3, owner-ratified 2026-07-25). + +Two halves, and the split matters: + +1. `TestPoolGroupVotes` exercises `vote_consensus.pool_group_votes` as the pure function it is - + no database, no models, no md5 anywhere. This is where the "dedupes weight, never fabricates + it" property docs/theory.md Β§4's group-pooling item claims is actually pinned down. +2. Everything else exercises the real resolver/persistence/feed/recompute paths against real + `Card`/`CardPrintingTag` rows, with group MEMBERSHIP supplied by the `md5_groups` fixture + below rather than by a populated `Card.md5_checksum` column - because that column arrives in + this issue's PR-1 (`md5-checksum-substrate`), which this branch is cut BEFORE. The fixture + replaces the two - and only two - functions in `printing_consensus` that touch the column + (`_card_md5_checksum`, `_card_ids_with_md5_checksums`), so every line of grouping, pooling, + propagation, and feed logic under test is the real one; only the storage of the checksum is + faked. Once PR-1 is merged into this branch, these tests keep passing unchanged and can be + supplemented with column-backed equivalents. + +The SINGLETON NO-OP proof (ruling 3) is deliberately NOT concentrated in one test here: it is +the entire pre-existing consensus/printing/tag/question-feed/recompute suite, which passes +UNMODIFIED, because a card with no checksum is a group of one and every path below degenerates +to its pre-#473 behavior. `TestSingletonIsANoOp` adds the direct, mechanical statements of that +degeneration (group of one, no pooling keys, no extra queries, prefetch still honoured) that the +existing suite asserts only indirectly. +""" + +from unittest.mock import patch + +import pytest + +from cardpicker import printing_consensus +from cardpicker.management.commands.consensus_recompute import run_consensus_recompute +from cardpicker.models import Card, PrintingTagStatus, VoteSource +from cardpicker.printing_consensus import ( + build_group_printing_vote_tuples, + group_printing_votes, + md5_group_card_ids, + md5_group_cards, + md5_group_expanded_card_ids, + md5_group_key, + resolve_and_persist_printing, + resolve_printing, +) +from cardpicker.question_feed import ( + _tier_1_confirm_suggestion, + _voter_answered_printing_card_ids, + is_likely_resolve_printing, +) +from cardpicker.tests.factories import ( + CanonicalCardFactory, + CardFactory, + CardPrintingTagFactory, +) +from cardpicker.vote_consensus import VoteTuple, pool_group_votes + + +@pytest.fixture +def md5_groups(monkeypatch): + """ + Assigns md5 identity groups to `Card` rows without `Card.md5_checksum` existing yet (see this + module's docstring). Returns a callable: `md5_groups("checksum", card_a, card_b)` puts those + cards in one group. Cards never passed to it stay checksum-less - i.e. groups of one, the + ruling-3 degenerate case - exactly as every card in the catalogue is today. + """ + checksum_by_card_id: dict[int, str] = {} + + def fake_card_md5_checksum(card: Card) -> str | None: + return checksum_by_card_id.get(card.pk) + + def fake_card_ids_with_md5_checksums(checksums: set[str]) -> list[int]: + return [card_id for card_id, checksum in checksum_by_card_id.items() if checksum in checksums] + + monkeypatch.setattr(printing_consensus, "_card_md5_checksum", fake_card_md5_checksum) + monkeypatch.setattr(printing_consensus, "_card_ids_with_md5_checksums", fake_card_ids_with_md5_checksums) + + def assign(checksum: str, *cards: Card) -> None: + for card in cards: + checksum_by_card_id[card.pk] = checksum + + return assign + + +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 TestPoolGroupVotes: + """The pure pooling primitive - no DB, no md5, no models.""" + + def test_unkeyed_votes_pass_through_unchanged(self): + votes = [ + VoteTuple(outcome_key=1, weight=1.0, is_human_backed=True), + VoteTuple(outcome_key=1, weight=1.0, is_human_backed=True), + VoteTuple(outcome_key=2, weight=0.5, is_human_backed=False), + ] + assert pool_group_votes(votes) == votes + + def test_same_dedupe_key_collapses_to_one_event(self): + votes = [ + VoteTuple(outcome_key=1, weight=0.5, is_human_backed=False, dedupe_key="ocr-bot"), + VoteTuple(outcome_key=1, weight=0.5, is_human_backed=False, dedupe_key="ocr-bot"), + VoteTuple(outcome_key=1, weight=0.5, is_human_backed=False, dedupe_key="ocr-bot"), + ] + pooled = pool_group_votes(votes) + assert len(pooled) == 1 + assert pooled[0].weight == 0.5 + + def test_distinct_dedupe_keys_are_independent_events(self): + votes = [ + VoteTuple(outcome_key=1, weight=0.5, is_human_backed=False, dedupe_key="ocr-bot"), + VoteTuple(outcome_key=1, weight=0.5, is_human_backed=False, dedupe_key="fallback-bot"), + ] + assert len(pool_group_votes(votes)) == 2 + + def test_collapse_keeps_the_maximum_weight(self): + votes = [ + VoteTuple(outcome_key=1, weight=0.25, is_human_backed=False, dedupe_key="bot"), + VoteTuple(outcome_key=1, weight=1.0, is_human_backed=False, dedupe_key="bot"), + VoteTuple(outcome_key=1, weight=0.5, is_human_backed=False, dedupe_key="bot"), + ] + pooled = pool_group_votes(votes) + assert [vote.weight for vote in pooled] == [1.0] + + def test_equal_weights_keep_the_first_vote_in_input_order(self): + votes = [ + VoteTuple(outcome_key="first", weight=0.5, is_human_backed=False, dedupe_key="bot"), + VoteTuple(outcome_key="second", weight=0.5, is_human_backed=False, dedupe_key="bot"), + ] + assert [vote.outcome_key for vote in pool_group_votes(votes)] == ["first"] + + def test_pooling_never_increases_total_weight(self): + votes = [ + VoteTuple(outcome_key=1, weight=1.0, is_human_backed=True), + VoteTuple(outcome_key=1, weight=0.5, is_human_backed=False, dedupe_key="bot"), + VoteTuple(outcome_key=1, weight=0.5, is_human_backed=False, dedupe_key="bot"), + VoteTuple(outcome_key=2, weight=0.5, is_human_backed=False, dedupe_key="bot"), + ] + pooled = pool_group_votes(votes) + assert sum(vote.weight for vote in pooled) <= sum(vote.weight for vote in votes) + # and specifically: the one human event survives intact, the one agent collapses to one + assert len(pooled) == 2 + + +class TestSingletonIsANoOp: + """ + Ruling 3's degenerate case, stated mechanically. The substantive proof is the pre-existing + suite passing unmodified; these pin the specific properties that make that true. + """ + + def test_checksumless_card_is_a_group_of_one(self, db): + card = CardFactory() + assert md5_group_card_ids(card) == [card.pk] + assert md5_group_cards(card) == [card] + assert md5_group_key(card) == ("card", card.pk) + + def test_unique_checksum_card_is_a_group_of_one(self, db, md5_groups): + card = CardFactory() + other = CardFactory() + md5_groups("checksum-a", card) + md5_groups("checksum-b", other) + assert md5_group_card_ids(card) == [card.pk] + assert md5_group_cards(card) == [card] + + def test_singleton_votes_carry_no_pooling_key(self, db): + card = CardFactory() + printing = CanonicalCardFactory() + human_vote(card, printing, "human-1") + votes, is_group = group_printing_votes(card) + assert is_group is False + 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): + 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): + votes, is_group = group_printing_votes(prefetched) + + assert is_group is False + assert len(votes) == 1 + + def test_singleton_expansion_returns_its_input(self, db): + card = CardFactory() + assert md5_group_expanded_card_ids([card.pk]) == {card.pk} + assert md5_group_expanded_card_ids([]) == set() + + +class TestGroupTally: + def test_one_machine_agent_across_three_siblings_is_one_event(self, db, md5_groups): + card_a, card_b, card_c = CardFactory(), CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b, card_c) + printing = CanonicalCardFactory() + for card in (card_a, card_b, card_c): + machine_vote(card, printing, "ocr-bot") + + votes, is_group = group_printing_votes(card_a) + assert is_group is True + assert len(votes) == 3 # all three rows are read... + + vote_tuples = build_group_printing_vote_tuples(votes, pool=is_group) + assert len(vote_tuples) == 1 # ...and pool to ONE event + assert vote_tuples[0].weight == 0.5 + + def test_machine_dedupe_denies_a_lone_human_a_fabricated_quorum(self, db, md5_groups): + # the concrete reason ruling 1 exists: three byte-identical siblings each carrying the + # same agent's OCR verdict must not add up to 1.5 machine weight behind one human vote. + card_a, card_b, card_c = CardFactory(), CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b, card_c) + printing = CanonicalCardFactory() + for card in (card_a, card_b, card_c): + machine_vote(card, printing, "ocr-bot") + human_vote(card_a, printing, "human-1") + + # 1.0 (human) + 0.5 (one pooled machine event) = 1.5, short of PRINTING_TAG_MIN_VOTES=2 + assert resolve_printing(card_a) is None + + def test_distinct_machine_agents_still_count_separately(self, db, md5_groups): + card_a, card_b = CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b) + printing = CanonicalCardFactory() + machine_vote(card_a, printing, "ocr-bot") + machine_vote(card_b, printing, "fallback-bot") + + votes, is_group = group_printing_votes(card_a) + vote_tuples = build_group_printing_vote_tuples(votes, pool=is_group) + assert len(vote_tuples) == 2 + + def test_human_votes_sum_across_members(self, db, md5_groups): + card_a, card_b = CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b) + printing = CanonicalCardFactory() + human_vote(card_a, printing, "human-1") + human_vote(card_b, printing, "human-2") + + # two independent people, 1.0 each, pooled to 2.0 = PRINTING_TAG_MIN_VOTES + assert resolve_printing(card_a) == printing + assert resolve_printing(card_b) == printing + + def test_a_group_never_resolves_on_machine_weight_alone(self, db, md5_groups): + # four siblings, four DIFFERENT agents (nothing dedupes), 2.0 of machine weight - which + # would clear the quorum threshold on arithmetic alone. The human-backed gate holds at + # group level exactly as it does per card. + cards = [CardFactory() for _ in range(4)] + md5_groups("same-bytes", *cards) + printing = CanonicalCardFactory() + for index, card in enumerate(cards): + machine_vote(card, printing, f"bot-{index}") + + votes, is_group = group_printing_votes(cards[0]) + vote_tuples = build_group_printing_vote_tuples(votes, pool=is_group) + assert sum(vote.weight for vote in vote_tuples) == 2.0 + assert all(resolve_printing(card) is None for card in cards) + + def test_human_dissent_across_members_is_one_group_level_contest(self, db, md5_groups): + # each card, alone, resolves to its own printing; as ONE identification target they are a + # 2-vs-2 contest (share 0.5, below PRINTING_TAG_MIN_SHARE) and neither wins. Ruling 2: + # nothing new invented - this is the standard matrix, run on the pooled tally. + card_a, card_b = CardFactory(), CardFactory() + printing_a, printing_b = CanonicalCardFactory(), CanonicalCardFactory() + human_vote(card_a, printing_a, "human-1") + human_vote(card_a, printing_a, "human-2") + human_vote(card_b, printing_b, "human-3") + human_vote(card_b, printing_b, "human-4") + + assert resolve_printing(card_a) == printing_a + assert resolve_printing(card_b) == printing_b + + md5_groups("same-bytes", card_a, card_b) + + assert resolve_printing(card_a) is None + assert resolve_printing(card_b) is None + + def test_machine_dissent_cannot_tip_a_group_level_human_contest(self, db, md5_groups): + # the matrix's no-machine-tipping mechanism, unchanged, on a pooled tally: a machine pile + # behind one side of a live human-vs-human group contest is excluded from the math. + card_a, card_b = CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b) + printing_a, printing_b = CanonicalCardFactory(), CanonicalCardFactory() + human_vote(card_a, printing_a, "human-1") + human_vote(card_a, printing_a, "human-2") + human_vote(card_b, printing_b, "human-3") + human_vote(card_b, printing_b, "human-4") + for index in range(6): + machine_vote(card_a, printing_a, f"bot-{index}") + + assert resolve_printing(card_a) is None + + +class TestGroupPropagation: + def test_resolution_is_written_to_every_member(self, db, md5_groups): + card_a, card_b, card_c = CardFactory(), CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b, card_c) + printing = CanonicalCardFactory() + human_vote(card_a, printing, "human-1") + human_vote(card_a, printing, "human-2") + + with patch("cardpicker.documents.reindex_card_safely"): + result = resolve_and_persist_printing(card_a) + + assert result == printing + for card in (card_a, card_b, card_c): + card.refresh_from_db() + assert card.printing_tag_status == PrintingTagStatus.RESOLVED + assert card.inferred_canonical_card_id == printing.pk + + def test_de_resolution_is_written_to_every_member(self, db, md5_groups): + card_a, card_b = CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b) + printing_a, printing_b = CanonicalCardFactory(), CanonicalCardFactory() + human_vote(card_a, printing_a, "human-1") + human_vote(card_a, printing_a, "human-2") + with patch("cardpicker.documents.reindex_card_safely"): + resolve_and_persist_printing(card_a) + assert card_a.printing_tag_status == PrintingTagStatus.RESOLVED + + # an equal-weight human disagreement lands on the SIBLING, not on card_a + human_vote(card_b, printing_b, "human-3") + human_vote(card_b, printing_b, "human-4") + with patch("cardpicker.documents.reindex_card_safely"): + result = resolve_and_persist_printing(card_b) + + assert result is None + for card in (card_a, card_b): + card.refresh_from_db() + assert card.printing_tag_status == PrintingTagStatus.UNRESOLVED + assert card.inferred_canonical_card_id is None + + def test_only_out_of_step_members_are_reindexed(self, db, md5_groups): + card_a, card_b = CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b) + printing = CanonicalCardFactory() + human_vote(card_a, printing, "human-1") + human_vote(card_a, printing, "human-2") + + with patch("cardpicker.documents.reindex_card_safely") as mock_reindex: + resolve_and_persist_printing(card_a) + assert mock_reindex.call_count == 2 # both members entered RESOLVED + + with patch("cardpicker.documents.reindex_card_safely") as mock_reindex: + resolve_and_persist_printing(card_a) + mock_reindex.assert_not_called() # same outcome, nothing to push + + def test_the_callers_own_instance_is_the_one_written_through(self, db, md5_groups): + card_a, card_b = CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b) + printing = CanonicalCardFactory() + human_vote(card_b, printing, "human-1") + human_vote(card_b, printing, "human-2") + + with patch("cardpicker.documents.reindex_card_safely"): + resolve_and_persist_printing(card_a) + + # no refresh_from_db: the caller reads its own object's status straight after the call + assert card_a.printing_tag_status == PrintingTagStatus.RESOLVED + assert card_a.inferred_canonical_card_id == printing.pk + + +class TestConsensusRecomputeGroups: + def test_a_group_is_visited_once_and_every_member_is_reported(self, db, md5_groups): + card_a, card_b, card_c = CardFactory(), CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b, card_c) + printing = CanonicalCardFactory() + # votes on all three members: pre-#473 this walked three cards and resolved three times + human_vote(card_a, printing, "human-1") + human_vote(card_b, printing, "human-2") + machine_vote(card_c, printing, "ocr-bot") + + with patch("cardpicker.documents.reindex_card_safely"): + with patch( + "cardpicker.management.commands.consensus_recompute.resolve_and_persist_printing", + side_effect=resolve_and_persist_printing, + ) as spy: + report = run_consensus_recompute(apply=True) + + assert spy.call_count == 1 + assert report["printing"]["checked"] == 3 # every member still counted + assert report["printing"]["transitions"]["unresolved->resolved"] == 3 + + def test_recompute_is_idempotent_over_groups(self, db, md5_groups): + card_a, card_b = CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b) + printing = CanonicalCardFactory() + human_vote(card_a, printing, "human-1") + human_vote(card_b, printing, "human-2") + + with patch("cardpicker.documents.reindex_card_safely"): + run_consensus_recompute(apply=True) + second = run_consensus_recompute(apply=True) + + assert second["printing"]["transitions"] == {} + for card in (card_a, card_b): + card.refresh_from_db() + assert card.printing_tag_status == PrintingTagStatus.RESOLVED + assert card.inferred_canonical_card_id == printing.pk + + def test_dry_run_predicts_the_group_outcome_for_every_member(self, db, md5_groups): + card_a, card_b = CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b) + printing = CanonicalCardFactory() + human_vote(card_a, printing, "human-1") + human_vote(card_b, printing, "human-2") + + report = run_consensus_recompute(apply=False) + + assert report["printing"]["written"] == 0 + assert report["printing"]["transitions"]["unresolved->resolved"] == 2 + for card in (card_a, card_b): + card.refresh_from_db() + assert card.printing_tag_status == PrintingTagStatus.UNRESOLVED + + +class TestQuestionFeedGroups: + def test_likely_resolve_reads_the_group_tally(self, db, md5_groups): + card_a, card_b = CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b) + printing = CanonicalCardFactory() + # one human vote on one member: the GROUP is one agreeing human vote from resolving, so + # every member of it is a likely-resolve question, including the voteless one. + human_vote(card_a, printing, "human-1") + + assert is_likely_resolve_printing(card_a) is True + assert is_likely_resolve_printing(card_b) is True + + def test_pooled_machine_weight_does_not_make_a_group_likely_resolve(self, db, md5_groups): + card_a, card_b, card_c = CardFactory(), CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b, card_c) + printing = CanonicalCardFactory() + for card in (card_a, card_b, card_c): + machine_vote(card, printing, "ocr-bot") + + # 0.5 pooled + 1.0 hypothetical human = 1.5, still short of the threshold. Unpooled it + # would have been 1.5 + 1.0 = 2.5 and this would have read as "one vote from resolving". + assert is_likely_resolve_printing(card_a) is False + + def test_answered_card_ids_expand_to_the_whole_group(self, db, md5_groups): + card_a, card_b = CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b) + printing = CanonicalCardFactory() + human_vote(card_a, printing, "voter-1") + + assert _voter_answered_printing_card_ids("voter-1") == {card_a.pk, card_b.pk} + assert _voter_answered_printing_card_ids("voter-2") == set() + + def test_the_feed_serves_at_most_one_member_per_group(self, db, md5_groups): + card_a = CardFactory(printing_tag_status=PrintingTagStatus.UNRESOLVED) + card_b = CardFactory(printing_tag_status=PrintingTagStatus.UNRESOLVED) + md5_groups("same-bytes", card_a, card_b) + printing = CanonicalCardFactory() + machine_vote(card_a, printing, "ocr-bot") + machine_vote(card_b, printing, "ocr-bot") + + # a fresh voter is offered the group (as one of its members)... + served = _tier_1_confirm_suggestion("voter-1") + assert served is not None + assert served.card.identifier in {card_a.identifier, card_b.identifier} + + # ...and once they have answered one member, the sibling is not offered as a second, + # identical question. + human_vote(card_a, printing, "voter-1") + assert _tier_1_confirm_suggestion("voter-1") is None diff --git a/MPCAutofill/cardpicker/vote_consensus.py b/MPCAutofill/cardpicker/vote_consensus.py index b21186517..0e15e08db 100644 --- a/MPCAutofill/cardpicker/vote_consensus.py +++ b/MPCAutofill/cardpicker/vote_consensus.py @@ -129,6 +129,65 @@ class VoteTuple(NamedTuple): # docstring for how this is used: capped per-outcome-group, and excluded entirely (alongside # every other non-human-backed vote) whenever that function's D1/D4 mechanisms engage. is_implicit: bool = False + # Identity of the EVENT this vote reports, for `pool_group_votes` (md5 identity groups, issue + # #473 ruling 1). Two votes carrying the same non-None `dedupe_key` are the same underlying + # evidence event observed on more than one member of an identity group, and collapse to ONE + # vote before any weight is summed. `None` (the default, and the value every pre-existing + # call site constructs) means "an independent event" and never collapses with anything - + # which is what makes a group of one a byte-for-byte no-op: nothing to collapse against. + # Set by `printing_consensus.build_group_printing_vote_tuples` for non-human-backed votes + # inside a multi-member group only; see that function and `pool_group_votes` below. + dedupe_key: Hashable | None = None + + +def pool_group_votes(votes: Iterable[VoteTuple]) -> list[VoteTuple]: + """ + Collapses the votes of an md5 IDENTITY GROUP (issue #473: a set of `Card`s whose stored + file checksums are equal, i.e. byte-identical images - ONE identification target, not N) + into the tally `resolve_weighted_consensus` should actually see, per the owner's 2026-07-25 + ruling 1: an evidence EVENT that was merely observed on several members of the group counts + ONCE, at its maximum weight, never summed; genuinely independent events all count. + + Mechanically: every vote carrying a non-None `dedupe_key` (see `VoteTuple.dedupe_key` - + today set only for non-human-backed votes, keyed on the casting agent's `anonymous_id`, and + only when the group has more than one member) collapses with every other vote sharing that + key, keeping the single highest-weighted one; `dedupe_key=None` votes (every human-backed + vote, and EVERY vote of a group of one) pass through untouched, in input order. The result + is fed to `resolve_weighted_consensus` unchanged - none of the matrix logic (the implicit + cap, D1/D4 non-human exclusion, the human-backed gate, the deductive-backfill zero-weight + override) is aware that pooling happened, and none of it needed to change. + + Soundness (the property docs/theory.md Β§4's group-pooling item states): this function can + only ever REMOVE weight from a tally, never add any. Pooling therefore cannot create a + resolution that the same evidence could not already have produced on a single card, and the + Β§7b false-accept bound is preserved or tightened, never loosened - the reason machine + evidence transferred between byte-identical siblings (issue #473 PR-2) cannot masquerade as + N independent confirmations of the same printing. + + Ties and self-contradiction: equal weights keep the FIRST vote in input order (callers pass + a deterministically ordered group tally - see `printing_consensus.group_printing_votes` - + so this is stable across runs, not arbitrary per-query). If one agent's deduped votes argue + for DIFFERENT outcomes across the group (possible: `CardPrintingTag`'s uniqueness + constraints allow one `anonymous_id` several printing votes per card, e.g. a rescan that + matched differently), that agent contradicts itself about byte-identical bytes and, per the + ruling's "ONE event" wording, still contributes exactly one vote. Keeping one side rather + than both is strictly less machine weight than the pre-pooling tally carried, so it cannot + make anything easier to resolve; and no volume of machine weight can resolve a group by + itself regardless (the human-backed gate is untouched by this function). + """ + pooled: list[VoteTuple] = [] + index_by_dedupe_key: dict[Hashable, int] = {} + for vote in votes: + if vote.dedupe_key is None: + pooled.append(vote) + continue + index = index_by_dedupe_key.get(vote.dedupe_key) + if index is None: + index_by_dedupe_key[vote.dedupe_key] = len(pooled) + pooled.append(vote) + elif vote.weight > pooled[index].weight: + pooled[index] = vote + return pooled class _PendingPrivileged: @@ -232,6 +291,11 @@ def resolve_weighted_consensus( lone human vote plus agreeing machine weight can still be promoted to a resolution the same way it always could (D2: `exclude_non_human` is false in that shape, since there's only one human-backed group and its own human weight doesn't clear `min_weight` by itself). + + This function is unaware of md5 identity groups (issue #473) and deliberately stays that + way: a group-aware caller pools its members' votes through `pool_group_votes` FIRST and + hands the result here, so every mechanism above runs on the pooled tally with no branch of + its own. A group of one pools to itself, which is what makes grouping a no-op for it. """ votes = list(votes) if not votes: @@ -330,6 +394,7 @@ def contested_queryset( __all__ = [ "VoteTuple", + "pool_group_votes", "resolve_weighted_consensus", "PENDING_PRIVILEGED", "_SOURCE_WEIGHTS", diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index c64e53ac9..6ff7b4198 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -138,6 +138,30 @@ 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 + [`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 + `vote_consensus.pool_group_votes()` collapses it: **non-human-backed + votes dedupe per casting `anonymous_id`** (one machine agent's verdict + about identical bytes is one event, at its max weight β€” never summed + across siblings), while **human-backed votes all count and sum** (real, + independent people). The pooled tally then runs through the UNCHANGED + `resolve_weighted_consensus` β€” same weights, same thresholds, same two + mechanisms above, same human-backed gate, applied once per group + instead of once per member. `resolve_and_persist_printing()` writes the + outcome (`inferred_canonical_card` + `printing_tag_status`) to **every** + member, in pk order, reindexing only the members whose indexed printing + actually changed; `consensus_recompute` walks each group once; + `question_feed` classifies likely-resolve on the group tally and serves + at most one member of a group per voter. A card with a null or unique + checksum 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). - **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/theory.md b/docs/theory.md index e8d678c26..509bc93d0 100644 --- a/docs/theory.md +++ b/docs/theory.md @@ -212,7 +212,7 @@ distance distribution, don't pick a cutoff in isolation.** ## 4. Soundness mechanisms -Two structural properties keep this decoder safe to run unattended at +Three structural properties keep this decoder safe to run unattended at catalog scale, independent of how accurate any single engine's evidence turns out to be: @@ -281,6 +281,42 @@ turns out to be: actually verifying β€” not "43,425 correct decisions," but "43,425 decisions that were structurally incapable of resolving anything on their own." +3. **Identity-group pooling, i.e. one target gets one tally** + (owner-ratified 2026-07-25; `vote_consensus.pool_group_votes`, + `printing_consensus.resolve_and_persist_printing`). Several catalog + records can index the _same image file_ β€” different uploaders, same + bytes, byte-equality established by the storage provider's own + checksum, not by any similarity measure of ours. Such a set is **one + identification target**, and is tallied as one: an evidence event + observed on several members counts **once**, at its maximum weight + (keyed on the identity of the agent that produced it), while + human-backed votes, being genuinely independent observers of the same + target, sum as they always did. The resolved outcome is then written + to every member, so byte-identical images cannot disagree with each + other about what they depict β€” a class of catalog inconsistency that + is now unreachable by construction rather than merely unlikely. + The soundness claim is deliberately narrow and worth stating exactly: + pooling can only **remove** weight from a tally, never add any, so no + resolution becomes reachable that the same underlying evidence could + not already have produced on a single record. Β§7b's false-accept + bound is therefore preserved or tightened, never loosened β€” and the + `gβ‚…` gate above is untouched by pooling, since a group's tally is + still subject to the same human-backed requirement, the same + quorum/share thresholds, and the same D1/D4 exclusions, applied once + instead of _n_ times. What this closes is an **independence** + failure, not a weighting one: the moment identical bytes let the + pipeline reuse one card's extracted evidence for its siblings instead + of re-deriving it (the point of establishing byte-equality at all β€” + the catalog is not permitted to hoard images, so re-fetching is the + expensive step), an un-pooled tally would read one observation as _n_ + agreeing machine confirmations. That is precisely the mutual + independence Β§7a's `Ρ₁…Ρ₄` composition assumes, so pooling is what + keeps the composed bound honest once evidence is shared. Human + disagreement inside a group is not special-cased: it is one visible + contest on one target, decided by the same vote-weight matrix as any + other. A record whose checksum is unknown or unique is a group of + one, for which every statement above is the identity β€” the pre-2026-07-25 + per-record behavior, unchanged. Together these mean the system's worst-case failure mode, even under a badly miscalibrated engine, is a wasted human review cycle (a bad From ec18ecd844f15e5ab0983e26f10a6596b6c28489 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Sat, 25 Jul 2026 19:18:16 +0000 Subject: [PATCH 2/4] Dedupe pooled group votes per agent and withhold self-contradicting agents Co-Authored-By: Claude Fable 5 --- MPCAutofill/cardpicker/printing_consensus.py | 79 ++++++++--- MPCAutofill/cardpicker/question_feed.py | 69 ++++++++-- .../tests/test_md5_group_pooling.py | 130 ++++++++++++++++-- MPCAutofill/cardpicker/vote_consensus.py | 96 ++++++++----- docs/features/printing-tags.md | 27 +++- docs/theory.md | 101 +++++++++----- 6 files changed, 381 insertions(+), 121 deletions(-) diff --git a/MPCAutofill/cardpicker/printing_consensus.py b/MPCAutofill/cardpicker/printing_consensus.py index 86083ccc4..9c416a57e 100644 --- a/MPCAutofill/cardpicker/printing_consensus.py +++ b/MPCAutofill/cardpicker/printing_consensus.py @@ -2,6 +2,7 @@ from typing import Hashable, Iterable, Literal, Sequence, TypedDict from django.conf import settings +from django.core.exceptions import FieldDoesNotExist from cardpicker.models import CanonicalCard, Card, CardPrintingTag, PrintingTagStatus from cardpicker.vote_consensus import ( @@ -19,9 +20,10 @@ # indexes), referenced by NAME rather than as an attribute so this module is importable and # correct both before and after that field exists: it is added by issue #473's PR-1 # (`md5-checksum-substrate`), which this branch is cut BEFORE and merges AFTER. Every read of it -# funnels through `_card_md5_checksum`/`_card_ids_with_md5_checksums` below - the only two places -# in this module that touch the column - so on a checkout without the field every card is a -# group of one and every group-aware path below degenerates, provably, to its pre-#473 behavior. +# funnels through `_card_md5_checksum`/`_md5_checksums_for_card_ids`/`_card_ids_with_md5_checksums` +# below - the only three places in this module that touch the column - so on a checkout without +# the field every card is a group of one and every group-aware path below degenerates, provably, +# to its pre-#473 behavior. MD5_CHECKSUM_FIELD = "md5_checksum" @@ -36,6 +38,38 @@ def _card_md5_checksum(card: Card) -> str | None: return getattr(card, MD5_CHECKSUM_FIELD, None) or None +def _md5_checksum_column_exists() -> bool: + """ + Whether `Card.md5_checksum` exists on this checkout - False until issue #473's PR-1 merges + into this branch, True forever after. Only the BULK reader below needs to ask: it filters on + the column by name (`.values_list`), which raises rather than degrading if the field is + absent, unlike `_card_md5_checksum`'s per-instance `getattr`. A `_meta` lookup, so this is a + dict access, not a query - safe to call per request. + """ + try: + Card._meta.get_field(MD5_CHECKSUM_FIELD) + except FieldDoesNotExist: + return False + return True + + +def _md5_checksums_for_card_ids(card_ids: Iterable[int]) -> set[str]: + """ + The distinct non-null checksums held by `card_ids` - ONE column read, not a fetch of whole + `Card` rows (2026-07-25 gate on PR #482, condition f1: the row-fetch form regressed the + question feed's per-request cost even before PR-1, since it hydrated a model instance per + card the voter had ever voted on purely to read one string off it). Returns an empty set, + without querying at all, while the column doesn't exist. + """ + if not _md5_checksum_column_exists(): + return set() + return { + checksum + for checksum in Card.objects.filter(pk__in=card_ids).values_list(MD5_CHECKSUM_FIELD, flat=True) + if checksum + } + + def _card_ids_with_md5_checksums(checksums: set[str]) -> list[int]: """ Every `Card.pk` whose checksum is in `checksums` - the one query in this module that filters @@ -90,16 +124,14 @@ def md5_group_expanded_card_ids(card_ids: Iterable[int]) -> set[int]: `card_ids` widened to include every md5 sibling of every card in it - "the cards this voter has already answered" widened to "the identity groups this voter has already answered", for `question_feed`'s serve-one-member-per-group exclusion. Returns `card_ids` unchanged when - none of them carry a checksum (which, before PR-1, is always). + none of them carry a checksum (which, before PR-1, is always - at a cost of zero queries, + see `_md5_checksums_for_card_ids`). At most two queries otherwise, neither of which + materializes a `Card` instance. """ ids = set(card_ids) if not ids: return ids - checksums = { - checksum - for checksum in (_card_md5_checksum(card) for card in Card.objects.filter(pk__in=ids)) - if checksum is not None - } + checksums = _md5_checksums_for_card_ids(ids) if not checksums: return ids return ids | set(_card_ids_with_md5_checksums(checksums)) @@ -177,13 +209,23 @@ def build_group_printing_vote_tuples( """ Translates `CardPrintingTag` rows into the `VoteTuple`s `resolve_weighted_consensus` reads, pooling them across an md5 identity group when `pool` is True (issue #473 ruling 1, applied - by `vote_consensus.pool_group_votes`): every non-human-backed vote is keyed on the - `anonymous_id` of the agent that cast it, so one machine agent's verdict about a set of - byte-identical images is ONE event at its maximum weight no matter how many members carry a - copy of it, while human-backed votes stay unkeyed and therefore sum, being genuinely - independent people looking at the image. With `pool=False` (a group of one) no vote is - keyed, `pool_group_votes` is never called, and the returned list is exactly what this - module built before #473. + by `vote_consensus.pool_group_votes`): EVERY vote is keyed on the `anonymous_id` of the agent + that cast it - human-backed votes included - so one agent's agreeing votes about a set of + byte-identical images are ONE event no matter how many members carry a copy, and one agent + that contradicts itself across members is withheld from the tally entirely. Distinct agents + still sum: two different people voting on two members are two votes, which is the point of + tallying a group as one target. + + Human-backed votes were NOT keyed in this function's first form, on the reading that separate + people are separate events regardless. That was wrong for the case that matters and was + rejected at review (2026-07-25 gate on PR #482, condition 1): `anonymous_id` identifies the + VOTER, so leaving human votes unkeyed let ONE person reach a 2.0 quorum by answering the same + image twice under two of its identifiers - a resolution neither card could reach alone, from + one human judgement. Keying humans too is what makes `PRINTING_TAG_MIN_VOTES` a count of + distinct agents rather than of rows. + + With `pool=False` (a group of one) no vote is keyed, `pool_group_votes` is never called, and + the returned list is exactly what this module built before #473. Passing a `printings_by_id` dict populates it with each voted `CanonicalCard` (needed to map a winning outcome key back to a printing). Callers that only need the outcome KEY - e.g. @@ -209,13 +251,12 @@ def build_group_printing_vote_tuples( if printings_by_id is not None: assert vote.printing is not None printings_by_id[vote.printing_id] = vote.printing - is_human_backed = is_human_backed_source(vote.source) vote_tuples.append( VoteTuple( outcome_key=key, weight=resolve_vote_weight(vote.source, vote.anonymous_id), - is_human_backed=is_human_backed, - dedupe_key=None if (is_human_backed or not pool) else vote.anonymous_id, + is_human_backed=is_human_backed_source(vote.source), + dedupe_key=vote.anonymous_id if pool else None, ) ) return pool_group_votes(vote_tuples) if pool else vote_tuples diff --git a/MPCAutofill/cardpicker/question_feed.py b/MPCAutofill/cardpicker/question_feed.py index e827df3ab..05155a387 100644 --- a/MPCAutofill/cardpicker/question_feed.py +++ b/MPCAutofill/cardpicker/question_feed.py @@ -209,8 +209,14 @@ def _voter_answered_printing_card_ids(anonymous_id: str) -> set[int]: 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 same set of cards, expressed as pks) - which, for a checksum-less catalogue, is every card. - One indexed query, plus the expansion's own (at most) two - the tiers each scan or `.first()` - over the result, so this is not a per-candidate cost. + One indexed query, plus the expansion's own (at most two, and zero before PR-1 adds the + checksum column - see `printing_consensus._md5_checksums_for_card_ids`). + + COMPUTED ONCE PER FEED REQUEST, in `get_next_question_feed_item`, and passed down to every + tier that needs it (2026-07-25 gate on PR #482, condition f1: each tier calling this for + itself multiplied the cost by the number of tiers consulted, for an answer that cannot change + within one request). The tiers keep an optional parameter rather than a required one so a + 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) @@ -248,6 +254,10 @@ def is_likely_resolve_printing(card: Card) -> bool: current_weight_by_key[vote.outcome_key] += vote.weight leading_key = max(current_weight_by_key.items(), key=lambda pair: pair[1])[0] + # No `dedupe_key`: this stands for a NEW voter, distinct from everyone already in the pooled + # tally, so it must not collapse into any of them (issue #473 - inside a group, real votes + # are keyed on their caster's `anonymous_id`; see `vote_consensus.pool_group_votes`). The + # tuples it joins are already pooled, and this list is not re-pooled. hypothetical_vote = VoteTuple( outcome_key=leading_key, weight=resolve_vote_weight(VoteSource.USER, _HYPOTHETICAL_VOTE_ANONYMOUS_ID), @@ -261,7 +271,7 @@ def is_likely_resolve_printing(card: Card) -> bool: return winning_key == leading_key -def _likely_resolve_printing_card(anonymous_id: str) -> Optional[Card]: +def _likely_resolve_printing_card(anonymous_id: str, answered_card_ids: Optional[set[int]] = None) -> Optional[Card]: """ First UNRESOLVED printing card (in `date_created` order, same scan convention tier 1 uses) that both carries at least one existing `CardPrintingTag` row and passes @@ -281,10 +291,22 @@ def _likely_resolve_printing_card(anonymous_id: str) -> Optional[Card]: pre-filtered ~97k rows, not the full 218k-card catalog and not unbounded - accepted as a v1 cost matching this module's own "known v1 property, not a bug" convention (see the module docstring), not solved with a materialized/cached index here. + + ONE ADDITIONAL QUERY PER SCANNED CARD once issue #473's PR-1 populates `Card.md5_checksum`: + `is_likely_resolve_printing` reads the card's identity GROUP, and a card carrying a checksum + costs a group-membership lookup to find its siblings before its tally can be built (a + checksum-less card still costs nothing extra - it takes the group-of-one path with no query). + Accepted explicitly here rather than discovered later (2026-07-25 gate on PR #482, condition + f2): it rides on the same bounded, stop-at-first-match scan this docstring already accepts as + a v1 cost, and the pooling it pays for is what stops that pool from serving n copies of one + question. If this scan is ever the profile's hot spot, the fix is the materialized + likely-resolve index this docstring already defers, not un-grouping the tally. """ + if answered_card_ids is None: + answered_card_ids = _voter_answered_printing_card_ids(anonymous_id) candidates = ( Card.objects.filter(printing_tag_status=PrintingTagStatus.UNRESOLVED, printing_tags__isnull=False) - .exclude(pk__in=_voter_answered_printing_card_ids(anonymous_id)) + .exclude(pk__in=answered_card_ids) .distinct() .order_by("date_created") ) @@ -306,14 +328,18 @@ def _likely_resolve_item(card: Card) -> QuestionFeedItem: return _identify_printing_item(card) -def _tier_1_confirm_suggestion(anonymous_id: str) -> Optional[QuestionFeedItem]: +def _tier_1_confirm_suggestion( + anonymous_id: str, answered_card_ids: Optional[set[int]] = None +) -> Optional[QuestionFeedItem]: + if answered_card_ids is None: + answered_card_ids = _voter_answered_printing_card_ids(anonymous_id) cards = ( Card.objects.filter( printing_tag_status=PrintingTagStatus.UNRESOLVED, printing_tags__source__in=[VoteSource.DEDUCTION, VoteSource.OCR], ) .exclude(printing_tags__source__in=[VoteSource.USER, VoteSource.ADMIN, VoteSource.FEDERATED]) - .exclude(pk__in=_voter_answered_printing_card_ids(anonymous_id)) + .exclude(pk__in=answered_card_ids) .distinct() .order_by("date_created") ) @@ -324,10 +350,14 @@ def _tier_1_confirm_suggestion(anonymous_id: str) -> Optional[QuestionFeedItem]: return None -def _tier_2_contested(anonymous_id: str) -> Optional[tuple[QuestionFeedItem, str]]: +def _tier_2_contested( + anonymous_id: str, answered_card_ids: Optional[set[int]] = None +) -> Optional[tuple[QuestionFeedItem, str]]: + if answered_card_ids is None: + answered_card_ids = _voter_answered_printing_card_ids(anonymous_id) printing_card = ( Card.objects.filter(printing_tag_status=PrintingTagStatus.UNRESOLVED, pk__in=get_contested_card_ids()) - .exclude(pk__in=_voter_answered_printing_card_ids(anonymous_id)) + .exclude(pk__in=answered_card_ids) .order_by("-date_created") .first() ) @@ -372,7 +402,9 @@ def _latest_stage_d_origin_reason_subquery() -> Subquery: ) -def _tier_4_fresh(anonymous_id: str) -> Optional[tuple[QuestionFeedItem, str]]: +def _tier_4_fresh( + anonymous_id: str, answered_card_ids: Optional[set[int]] = None +) -> Optional[tuple[QuestionFeedItem, str]]: # named "tier 4" (not renumbered to 3) even though moderation's former tier 3 was removed # (see module docstring) - keeps this name stable against every docstring/test/comment # that already refers to "tier 4" rather than triggering a pure-renumbering diff. @@ -394,10 +426,12 @@ def _tier_4_fresh(anonymous_id: str) -> Optional[tuple[QuestionFeedItem, str]]: # slice, ahead of the smallest "hard/open-ended" slice. Most tier-4 candidates share # `vote_count=0` (the "totally fresh" case), so in practice this origin-reason tiebreak is # what actually decides ordering among them, not a rarely-reached fallback. + if answered_card_ids is None: + answered_card_ids = _voter_answered_printing_card_ids(anonymous_id) printing_card = ( Card.objects.filter(printing_tag_status=PrintingTagStatus.UNRESOLVED) .exclude(pk__in=get_contested_card_ids()) - .exclude(pk__in=_voter_answered_printing_card_ids(anonymous_id)) + .exclude(pk__in=answered_card_ids) .annotate(vote_count=Count("printing_tags", distinct=True)) .annotate(origin_reason=_latest_stage_d_origin_reason_subquery()) .annotate( @@ -479,25 +513,32 @@ def get_next_question_feed_item(anonymous_id: str) -> Optional[QuestionFeedItem] infinite-loops or blocks on a starved pool - each branch is a single bounded query/scan, and an exhausted likely-resolve pool simply falls through to the remainder every time, letting the session's ratio drop honestly rather than stalling to protect it. + + The voter's answered-card exclusion set (`_voter_answered_printing_card_ids`, md5-group- + expanded per issue #473) is resolved ONCE here and passed to every printing tier below - it + cannot change mid-request, and recomputing it per tier was a real per-request regression the + 2026-07-25 PR #482 gate (condition f1) called out. """ + answered_card_ids = _voter_answered_printing_card_ids(anonymous_id) + if _served_mix_ratio(anonymous_id) < settings.QUESTION_FEED_LIKELY_RESOLVE_MIX_RATIO: - likely_resolve_card = _likely_resolve_printing_card(anonymous_id) + likely_resolve_card = _likely_resolve_printing_card(anonymous_id, answered_card_ids) if likely_resolve_card is not None: item = _likely_resolve_item(likely_resolve_card) return _log_served( anonymous_id, item, QuestionFeedServedPool.LIKELY_RESOLVE, "printing_one_vote_from_resolving" ) - tier_1_item = _tier_1_confirm_suggestion(anonymous_id) + tier_1_item = _tier_1_confirm_suggestion(anonymous_id, answered_card_ids) if tier_1_item is not None: return _log_served(anonymous_id, tier_1_item, QuestionFeedServedPool.REMAINDER, "tier_1_confirm_suggestion") - tier_2_result = _tier_2_contested(anonymous_id) + tier_2_result = _tier_2_contested(anonymous_id, answered_card_ids) if tier_2_result is not None: tier_2_item, tier_2_reason = tier_2_result return _log_served(anonymous_id, tier_2_item, QuestionFeedServedPool.REMAINDER, tier_2_reason) - tier_4_result = _tier_4_fresh(anonymous_id) + tier_4_result = _tier_4_fresh(anonymous_id, answered_card_ids) if tier_4_result is not None: tier_4_item, tier_4_reason = tier_4_result return _log_served(anonymous_id, tier_4_item, QuestionFeedServedPool.REMAINDER, tier_4_reason) diff --git a/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py b/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py index f585190ab..925659b9f 100644 --- a/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py +++ b/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py @@ -4,17 +4,18 @@ Two halves, and the split matters: 1. `TestPoolGroupVotes` exercises `vote_consensus.pool_group_votes` as the pure function it is - - no database, no models, no md5 anywhere. This is where the "dedupes weight, never fabricates - it" property docs/theory.md Β§4's group-pooling item claims is actually pinned down. + no database, no models, no md5 anywhere. This is where the two properties docs/theory.md Β§4's + group-pooling item actually claims are pinned down: one agent counts once (agreeing votes + collapse), and an agent that contradicts itself counts for nothing (order-independently). 2. Everything else exercises the real resolver/persistence/feed/recompute paths against real `Card`/`CardPrintingTag` rows, with group MEMBERSHIP supplied by the `md5_groups` fixture below rather than by a populated `Card.md5_checksum` column - because that column arrives in this issue's PR-1 (`md5-checksum-substrate`), which this branch is cut BEFORE. The fixture - replaces the two - and only two - functions in `printing_consensus` that touch the column - (`_card_md5_checksum`, `_card_ids_with_md5_checksums`), so every line of grouping, pooling, - propagation, and feed logic under test is the real one; only the storage of the checksum is - faked. Once PR-1 is merged into this branch, these tests keep passing unchanged and can be - supplemented with column-backed equivalents. + replaces the three - and only three - functions in `printing_consensus` that touch the column + (`_card_md5_checksum`, `_md5_checksums_for_card_ids`, `_card_ids_with_md5_checksums`), so + every line of grouping, pooling, propagation, and feed logic under test is the real one; only + the storage of the checksum is faked. Once PR-1 is merged into this branch, these tests keep + passing unchanged and can be supplemented with column-backed equivalents. The SINGLETON NO-OP proof (ruling 3) is deliberately NOT concentrated in one test here: it is the entire pre-existing consensus/printing/tag/question-feed/recompute suite, which passes @@ -67,10 +68,14 @@ def md5_groups(monkeypatch): def fake_card_md5_checksum(card: Card) -> str | None: return checksum_by_card_id.get(card.pk) + def fake_md5_checksums_for_card_ids(card_ids) -> set[str]: + return {checksum for card_id, checksum in checksum_by_card_id.items() if card_id in set(card_ids)} + def fake_card_ids_with_md5_checksums(checksums: set[str]) -> list[int]: return [card_id for card_id, checksum in checksum_by_card_id.items() if checksum in checksums] monkeypatch.setattr(printing_consensus, "_card_md5_checksum", fake_card_md5_checksum) + monkeypatch.setattr(printing_consensus, "_md5_checksums_for_card_ids", fake_md5_checksums_for_card_ids) monkeypatch.setattr(printing_consensus, "_card_ids_with_md5_checksums", fake_card_ids_with_md5_checksums) def assign(checksum: str, *cards: Card) -> None: @@ -125,23 +130,52 @@ def test_collapse_keeps_the_maximum_weight(self): pooled = pool_group_votes(votes) assert [vote.weight for vote in pooled] == [1.0] - def test_equal_weights_keep_the_first_vote_in_input_order(self): + def test_an_agent_that_contradicts_itself_is_withheld_entirely(self): + # NOT "keep the heavier side", and specifically NOT "keep whichever came first" - an + # agent that says two different things about byte-identical bytes is evidence for + # neither (see `pool_group_votes`' rule 2, and the order-independence test below for the + # failure the earlier keep-the-max form actually had). votes = [ VoteTuple(outcome_key="first", weight=0.5, is_human_backed=False, dedupe_key="bot"), - VoteTuple(outcome_key="second", weight=0.5, is_human_backed=False, dedupe_key="bot"), + VoteTuple(outcome_key="second", weight=1.0, is_human_backed=False, dedupe_key="bot"), + ] + assert pool_group_votes(votes) == [] + + def test_withholding_is_scoped_to_the_contradicting_agent(self): + votes = [ + VoteTuple(outcome_key="x", weight=1.0, is_human_backed=True, dedupe_key="human-1"), + VoteTuple(outcome_key="x", weight=0.5, is_human_backed=False, dedupe_key="bot-a"), + VoteTuple(outcome_key="x", weight=0.5, is_human_backed=False, dedupe_key="bot-b"), + VoteTuple(outcome_key="y", weight=0.5, is_human_backed=False, dedupe_key="bot-b"), + ] + pooled = pool_group_votes(votes) + assert [(vote.outcome_key, vote.dedupe_key) for vote in pooled] == [("x", "human-1"), ("x", "bot-a")] + + def test_pooling_is_order_independent(self): + # the concrete defect the 2026-07-25 gate found in the keep-the-max form: with equal + # weights it resolved a self-contradiction by INPUT order, which in the real caller is + # `card_id` order - so which sibling happened to have the lower pk decided which outcome + # a contradicting agent appeared to support, manufacturing correlated agreement. + votes = [ + VoteTuple(outcome_key="x", weight=1.0, is_human_backed=True, dedupe_key="human-1"), + VoteTuple(outcome_key="x", weight=0.5, is_human_backed=False, dedupe_key="bot"), + VoteTuple(outcome_key="y", weight=0.5, is_human_backed=False, dedupe_key="bot"), + VoteTuple(outcome_key="x", weight=0.5, is_human_backed=False, dedupe_key="other-bot"), ] - assert [vote.outcome_key for vote in pool_group_votes(votes)] == ["first"] + forward = pool_group_votes(votes) + reversed_order = pool_group_votes(list(reversed(votes))) + assert sorted(forward) == sorted(reversed_order) def test_pooling_never_increases_total_weight(self): votes = [ - VoteTuple(outcome_key=1, weight=1.0, is_human_backed=True), + VoteTuple(outcome_key=1, weight=1.0, is_human_backed=True, dedupe_key="human-1"), VoteTuple(outcome_key=1, weight=0.5, is_human_backed=False, dedupe_key="bot"), VoteTuple(outcome_key=1, weight=0.5, is_human_backed=False, dedupe_key="bot"), - VoteTuple(outcome_key=2, weight=0.5, is_human_backed=False, dedupe_key="bot"), ] pooled = pool_group_votes(votes) assert sum(vote.weight for vote in pooled) <= sum(vote.weight for vote in votes) - # and specifically: the one human event survives intact, the one agent collapses to one + # and specifically: the one human agent survives intact, the one machine agent's two + # agreeing observations collapse to one assert len(pooled) == 2 @@ -239,10 +273,78 @@ def test_human_votes_sum_across_members(self, db, md5_groups): human_vote(card_a, printing, "human-1") human_vote(card_b, printing, "human-2") - # two independent people, 1.0 each, pooled to 2.0 = PRINTING_TAG_MIN_VOTES + # two DISTINCT people, 1.0 each, pooled to 2.0 = PRINTING_TAG_MIN_VOTES. This is the + # intended multiplier: one target, two independent human confirmations of it. assert resolve_printing(card_a) == printing assert resolve_printing(card_b) == printing + def test_one_human_answering_two_siblings_is_one_vote(self, db, md5_groups): + # 2026-07-25 gate on PR #482, condition 1 (its scenario A, reproduced): the SAME + # `anonymous_id` voting once on each of two byte-identical members must not add up to a + # 2.0 quorum. One person answering the same image twice under two of its identifiers is + # one answer - neither card could resolve alone, and the group must not either. + card_a, card_b = CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b) + printing = CanonicalCardFactory() + human_vote(card_a, printing, "human-1") + human_vote(card_b, printing, "human-1") + + votes, is_group = group_printing_votes(card_a) + assert len(votes) == 2 # both rows are read... + assert len(build_group_printing_vote_tuples(votes, pool=is_group)) == 1 # ...one agent + + assert resolve_printing(card_a) is None + assert resolve_printing(card_b) is None + + def test_a_third_distinct_human_still_resolves_that_group(self, db, md5_groups): + # the complement of the test above: deduping one repeat voter must not make a group + # unresolvable, only un-inflatable. A second real person tips it exactly as it should. + card_a, card_b = CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b) + printing = CanonicalCardFactory() + human_vote(card_a, printing, "human-1") + human_vote(card_b, printing, "human-1") + assert resolve_printing(card_a) is None + + human_vote(card_b, printing, "human-2") + assert resolve_printing(card_a) == printing + + def test_a_self_contradicting_machine_agent_contributes_nothing(self, db, md5_groups): + # 2026-07-25 gate on PR #482, condition 2 (its scenario B at k=2): one human vote for X + # plus two OCR agents that each say X on one sibling and Y on the other. Each such agent + # has contradicted itself about identical bytes, so it withholds entirely - leaving 1.0 + # of human weight, short of quorum. Under the earlier keep-the-max collapse this + # resolved to X purely because the X rows had the lower card_id. + card_a, card_b = CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b) + printing_x, printing_y = CanonicalCardFactory(), CanonicalCardFactory() + human_vote(card_a, printing_x, "human-1") + for index in range(2): + machine_vote(card_a, printing_x, f"bot-{index}") + machine_vote(card_b, printing_y, f"bot-{index}") + + votes, is_group = group_printing_votes(card_a) + vote_tuples = build_group_printing_vote_tuples(votes, pool=is_group) + assert [vote.weight for vote in vote_tuples] == [1.0] # the human, and nothing else + assert resolve_printing(card_a) is None + + def test_self_contradiction_handling_is_independent_of_card_id_order(self, db, md5_groups): + # same shape as above, built twice with the outcomes swapped between the low-pk and + # high-pk sibling. The tally - and therefore the outcome - must be identical, since + # nothing about which sibling was created first says anything about the printing. + def build(x_first: bool) -> list[float]: + card_low, card_high = CardFactory(), CardFactory() + md5_groups(f"same-bytes-{x_first}", card_low, card_high) + printing_x, printing_y = CanonicalCardFactory(), CanonicalCardFactory() + human_vote(card_low, printing_x, f"human-{x_first}") + machine_vote(card_low if x_first else card_high, printing_x, f"bot-{x_first}") + machine_vote(card_high if x_first else card_low, printing_y, f"bot-{x_first}") + votes, is_group = group_printing_votes(card_low) + assert resolve_printing(card_low) is None + return sorted(vote.weight for vote in build_group_printing_vote_tuples(votes, pool=is_group)) + + assert build(True) == build(False) == [1.0] + def test_a_group_never_resolves_on_machine_weight_alone(self, db, md5_groups): # four siblings, four DIFFERENT agents (nothing dedupes), 2.0 of machine weight - which # would clear the quorum threshold on arithmetic alone. The human-backed gate holds at diff --git a/MPCAutofill/cardpicker/vote_consensus.py b/MPCAutofill/cardpicker/vote_consensus.py index 0e15e08db..e25192bcf 100644 --- a/MPCAutofill/cardpicker/vote_consensus.py +++ b/MPCAutofill/cardpicker/vote_consensus.py @@ -129,14 +129,15 @@ class VoteTuple(NamedTuple): # docstring for how this is used: capped per-outcome-group, and excluded entirely (alongside # every other non-human-backed vote) whenever that function's D1/D4 mechanisms engage. is_implicit: bool = False - # Identity of the EVENT this vote reports, for `pool_group_votes` (md5 identity groups, issue - # #473 ruling 1). Two votes carrying the same non-None `dedupe_key` are the same underlying - # evidence event observed on more than one member of an identity group, and collapse to ONE - # vote before any weight is summed. `None` (the default, and the value every pre-existing - # call site constructs) means "an independent event" and never collapses with anything - - # which is what makes a group of one a byte-for-byte no-op: nothing to collapse against. - # Set by `printing_consensus.build_group_printing_vote_tuples` for non-human-backed votes - # inside a multi-member group only; see that function and `pool_group_votes` below. + # Identity of the AGENT this vote came from, for `pool_group_votes` (md5 identity groups, + # issue #473 ruling 1) - an `anonymous_id` in practice. Votes sharing a non-None `dedupe_key` + # are one agent speaking about one identification target across several of its byte-identical + # members: agreeing ones collapse to a single vote, disagreeing ones withhold that agent + # entirely (see `pool_group_votes`). `None` (the default, and the value every pre-existing + # call site constructs) means "never collapse this with anything" - which is what makes a + # group of one a byte-for-byte no-op: nothing to collapse against. Set by + # `printing_consensus.build_group_printing_vote_tuples` for EVERY vote (human-backed + # included - one person is one agent) inside a multi-member group, and for none outside one. dedupe_key: Hashable | None = None @@ -146,41 +147,68 @@ def pool_group_votes(votes: Iterable[VoteTuple]) -> list[VoteTuple]: file checksums are equal, i.e. byte-identical images - ONE identification target, not N) into the tally `resolve_weighted_consensus` should actually see, per the owner's 2026-07-25 ruling 1: an evidence EVENT that was merely observed on several members of the group counts - ONCE, at its maximum weight, never summed; genuinely independent events all count. - - Mechanically: every vote carrying a non-None `dedupe_key` (see `VoteTuple.dedupe_key` - - today set only for non-human-backed votes, keyed on the casting agent's `anonymous_id`, and - only when the group has more than one member) collapses with every other vote sharing that - key, keeping the single highest-weighted one; `dedupe_key=None` votes (every human-backed - vote, and EVERY vote of a group of one) pass through untouched, in input order. The result - is fed to `resolve_weighted_consensus` unchanged - none of the matrix logic (the implicit - cap, D1/D4 non-human exclusion, the human-backed gate, the deductive-backfill zero-weight + ONCE, never summed. What makes summed weight mean anything is that the summands are + DISTINCT AGENTS, so that is exactly what this enforces. + + Mechanically, two rules over votes carrying a non-None `dedupe_key` (see + `VoteTuple.dedupe_key` - the casting agent's `anonymous_id`, set by + `printing_consensus.build_group_printing_vote_tuples` for every vote, human-backed or not, + when and only when the group has more than one member): + + 1. **Agreement collapses.** All of one agent's votes for the SAME outcome across the group + become one vote, at that agent's highest weight for it. One person answering the same + question twice under two identifiers of the same image file is one answer, not two - + and one machine agent's verdict about identical bytes is one verdict, however many + members carry a copy of it. + 2. **Self-contradiction withholds.** An agent whose votes across the group argue for + DIFFERENT outcomes (possible: `CardPrintingTag`'s uniqueness constraints allow one + `anonymous_id` several printing votes per card, e.g. a rescan that matched differently) + contributes NOTHING to this group's tally - not its maximum, not either side. It has + contradicted itself about byte-identical bytes, so it is not evidence for either + outcome. This mirrors `gβ‚„`'s withhold-never-manufacture philosophy (docs/theory.md + Β§7a): a cross-check that fails removes a claim rather than picking a winner for it. + The earlier keep-the-max form of this rule was rejected at review (2026-07-25 gate on + PR #482): with equal weights it resolved the contradiction by INPUT ORDER, i.e. by + `card_id`, which silently manufactured correlated agreement between an arbitrary + sibling and whatever else agreed with it. + + `dedupe_key=None` votes pass through untouched, in input order - which is EVERY vote of a + group of one, so a singleton pools to itself and grouping is a no-op for it. The result is + fed to `resolve_weighted_consensus` unchanged: none of the matrix logic (the implicit cap, + D1/D4 non-human exclusion, the human-backed gate, the deductive-backfill zero-weight override) is aware that pooling happened, and none of it needed to change. - Soundness (the property docs/theory.md Β§4's group-pooling item states): this function can - only ever REMOVE weight from a tally, never add any. Pooling therefore cannot create a - resolution that the same evidence could not already have produced on a single card, and the - Β§7b false-accept bound is preserved or tightened, never loosened - the reason machine - evidence transferred between byte-identical siblings (issue #473 PR-2) cannot masquerade as - N independent confirmations of the same printing. - - Ties and self-contradiction: equal weights keep the FIRST vote in input order (callers pass - a deterministically ordered group tally - see `printing_consensus.group_printing_votes` - - so this is stable across runs, not arbitrary per-query). If one agent's deduped votes argue - for DIFFERENT outcomes across the group (possible: `CardPrintingTag`'s uniqueness - constraints allow one `anonymous_id` several printing votes per card, e.g. a rescan that - matched differently), that agent contradicts itself about byte-identical bytes and, per the - ruling's "ONE event" wording, still contributes exactly one vote. Keeping one side rather - than both is strictly less machine weight than the pre-pooling tally carried, so it cannot - make anything easier to resolve; and no volume of machine weight can resolve a group by - itself regardless (the human-backed gate is untouched by this function). + Soundness, stated narrowly (docs/theory.md Β§4 item 3 carries the full statement): this + function only ever REMOVES weight from the tally it is given - it never adds, upgrades, or + re-labels one. What it therefore preserves EXACTLY is the machine-alone bound: no volume of + non-human-backed weight can resolve a group, because the human-backed gate is untouched and + pooling cannot manufacture a human-backed vote. What it makes true, rather than assumes, is + the independence the quorum threshold rests on: after pooling, `min_weight` counts distinct + agents, so neither one person answering n siblings nor one machine agent's evidence + transferred to n siblings (issue #473 PR-2) can reach quorum by repetition. Note the + direction of that claim carefully: pooling REPLACES a per-card tally with a group tally, and + a group tally can resolve things no single card's tally could (two different people, one + vote each, on two members) - that is the intended multiplier, not an accident. + + Ordering: the retained set is a function of the votes, not of the order they arrive in + (agreement is order-insensitive, and a contradicting agent is dropped wholesale), so a + caller's row ordering cannot influence any outcome. """ + votes = list(votes) + outcomes_by_dedupe_key: dict[Hashable, set[Hashable]] = defaultdict(set) + for vote in votes: + if vote.dedupe_key is not None: + outcomes_by_dedupe_key[vote.dedupe_key].add(vote.outcome_key) + withheld_dedupe_keys = {key for key, outcomes in outcomes_by_dedupe_key.items() if len(outcomes) > 1} + pooled: list[VoteTuple] = [] index_by_dedupe_key: dict[Hashable, int] = {} for vote in votes: if vote.dedupe_key is None: pooled.append(vote) continue + if vote.dedupe_key in withheld_dedupe_keys: + continue index = index_by_dedupe_key.get(vote.dedupe_key) if index is None: index_by_dedupe_key[vote.dedupe_key] = len(pooled) diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index 6ff7b4198..125efb13c 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -145,17 +145,32 @@ printings, artists, tags, and moderation from one screen. 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 - `vote_consensus.pool_group_votes()` collapses it: **non-human-backed - votes dedupe per casting `anonymous_id`** (one machine agent's verdict - about identical bytes is one event, at its max weight β€” never summed - across siblings), while **human-backed votes all count and sum** (real, - independent people). The pooled tally then runs through the UNCHANGED + `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 + same image under two of its identifiers is one answer; a machine + agent's verdict about identical bytes is one verdict), and an agent + whose votes across members **disagree** with each other is withheld + from the tally entirely (it has contradicted itself about identical + bytes, so it is evidence for neither side β€” the same + withhold-never-manufacture rule the `gβ‚„` cross-checks follow). What + sums is **distinct agents**: two different people voting on two + different members are two votes, and that is the intended multiplier. + Both rules were tightened at the 2026-07-25 gate on PR #482 β€” human + votes were originally left unkeyed (which let ONE person reach quorum + by answering two siblings) and self-contradiction originally kept the + max-weight side (which, at equal weights, let `card_id` order decide + which outcome an agent appeared to support). The pooled tally then runs + through the UNCHANGED `resolve_weighted_consensus` β€” same weights, same thresholds, same two mechanisms above, same human-backed gate, applied once per group instead of once per member. `resolve_and_persist_printing()` writes the outcome (`inferred_canonical_card` + `printing_tag_status`) to **every** member, in pk order, reindexing only the members whose indexed printing - actually changed; `consensus_recompute` walks each group once; + actually changed β€” so members cannot diverge while that shared path is + the only writer, though a change in group MEMBERSHIP (checksum + backfill, re-upload) needs a `consensus_recompute` pass for the + affected group; `consensus_recompute` walks each group once; `question_feed` classifies likely-resolve on the group tally and serves at most one member of a group per voter. A card with a null or unique checksum is a **group of one**, for which all of the above is provably diff --git a/docs/theory.md b/docs/theory.md index 509bc93d0..cea5a7948 100644 --- a/docs/theory.md +++ b/docs/theory.md @@ -283,40 +283,73 @@ turns out to be: their own." 3. **Identity-group pooling, i.e. one target gets one tally** (owner-ratified 2026-07-25; `vote_consensus.pool_group_votes`, - `printing_consensus.resolve_and_persist_printing`). Several catalog - records can index the _same image file_ β€” different uploaders, same - bytes, byte-equality established by the storage provider's own - checksum, not by any similarity measure of ours. Such a set is **one - identification target**, and is tallied as one: an evidence event - observed on several members counts **once**, at its maximum weight - (keyed on the identity of the agent that produced it), while - human-backed votes, being genuinely independent observers of the same - target, sum as they always did. The resolved outcome is then written - to every member, so byte-identical images cannot disagree with each - other about what they depict β€” a class of catalog inconsistency that - is now unreachable by construction rather than merely unlikely. - The soundness claim is deliberately narrow and worth stating exactly: - pooling can only **remove** weight from a tally, never add any, so no - resolution becomes reachable that the same underlying evidence could - not already have produced on a single record. Β§7b's false-accept - bound is therefore preserved or tightened, never loosened β€” and the - `gβ‚…` gate above is untouched by pooling, since a group's tally is - still subject to the same human-backed requirement, the same - quorum/share thresholds, and the same D1/D4 exclusions, applied once - instead of _n_ times. What this closes is an **independence** - failure, not a weighting one: the moment identical bytes let the - pipeline reuse one card's extracted evidence for its siblings instead - of re-deriving it (the point of establishing byte-equality at all β€” - the catalog is not permitted to hoard images, so re-fetching is the - expensive step), an un-pooled tally would read one observation as _n_ - agreeing machine confirmations. That is precisely the mutual - independence Β§7a's `Ρ₁…Ρ₄` composition assumes, so pooling is what - keeps the composed bound honest once evidence is shared. Human - disagreement inside a group is not special-cased: it is one visible - contest on one target, decided by the same vote-weight matrix as any - other. A record whose checksum is unknown or unique is a group of - one, for which every statement above is the identity β€” the pre-2026-07-25 - per-record behavior, unchanged. + `printing_consensus.build_group_printing_vote_tuples`, + `resolve_and_persist_printing`). Several catalog records can index the + _same image file_ β€” different uploaders, same bytes, byte-equality + established by the storage provider's own checksum, not by any + similarity measure of ours. Such a set is **one identification + target**, and is tallied as one, under a single rule: **the tally + counts distinct agents, not rows.** All of one agent's agreeing votes + across the group collapse into one β€” human and machine alike, keyed + on the caster's identity β€” and an agent whose votes across the group + _disagree_ with each other contributes nothing at all, on the same + withhold-don't-manufacture logic `gβ‚„` applies to its cross-checks + (Β§7a). Distinct agents still sum, which is the point of grouping: + two different people, one vote each, on two members of a group are + two independent confirmations of one target. + + Three claims, separated by how strong each actually is, because + conflating them is how a bound like this gets oversold: + + - **Preserved exactly**: the machine-alone bound. `gβ‚…` is untouched + by pooling β€” a group's tally faces the same human-backed + requirement, the same `min_weight`/`min_share` thresholds, and the + same D1/D4 exclusions, applied once instead of _n_ times β€” and + pooling only ever drops votes, so it cannot manufacture the + human-backed one the gate demands. `P(resolved | machine evidence alone) = 0` + survives intact, at group scope. + - **Newly rested on something real**: the independence the quorum + threshold assumes. Summed weight only means "several agents agree" + if the summands are different agents; per-record tallying got that + for free only because a person or a scanner could vote on a record + once. Byte-identical siblings break that for free-ness in both + directions β€” one person can answer the same image under _n_ + identifiers, and (once identical bytes let the pipeline reuse one + record's extracted evidence for its siblings rather than + re-deriving it β€” the point of establishing byte-equality at all, + since the catalog is not permitted to hoard images and re-fetching + is the expensive step) one machine observation can appear as _n_ + agreeing confirmations. Group-level per-agent dedupe is what makes + `min_weight` a count of distinct agents again, and it is what keeps + Β§7a's `Ρ₁…Ρ₄` composition's mutual-independence assumption honest + once evidence is shared. This is a **restored** assumption, not a + new guarantee. + - **Explicitly NOT claimed**: that pooling reaches no new + resolutions. It reaches some. A group tally replaces _n_ per-record + tallies, so two different people voting on two different members + now resolve a target that neither record could resolve alone. That + is the intended multiplier β€” the whole reason to treat the set as + one question β€” and it is a change in what is reachable, not merely + a restriction of it. (An earlier draft of this item claimed the + opposite, that pooling could only remove weight and therefore reach + nothing new. That was false, and its own test suite proved it; it + is corrected here rather than quietly dropped.) + + Β§7b's false-accept bound is unchanged in form: it is stated per + identification target, and an identity group is exactly one target. + Consistency across members is a **write-path** property, not a + metaphysical one: the outcome is resolved once and written to every + member through the one shared path, so members cannot diverge while + that path is the only writer. Group MEMBERSHIP can still change (a + checksum backfill, a re-upload, a corrected file), and a membership + change requires a recompute for the affected group before its members + are back in agreement β€” that recompute is `consensus_recompute`, + which walks groups, not rows. Human disagreement inside a group is + not special-cased: it is one visible contest on one target, decided + by the same vote-weight matrix as any other. A record whose checksum + is unknown or unique is a group of one, for which every statement + above is the identity β€” the pre-2026-07-25 per-record behavior, + unchanged. Together these mean the system's worst-case failure mode, even under a badly miscalibrated engine, is a wasted human review cycle (a bad From 59eff9256ae1aa0cb23a602ba575e393c2c08f28 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Sat, 25 Jul 2026 20:05:47 +0000 Subject: [PATCH 3/4] Address delta-gate wording conditions and query-cost/withholding test pins for md5 group pooling Co-Authored-By: Claude Fable 5 --- .../tests/test_md5_group_pooling.py | 90 +++++++++++++++++++ MPCAutofill/cardpicker/vote_consensus.py | 13 +-- docs/features/printing-tags.md | 9 +- docs/theory.md | 23 ++--- 4 files changed, 114 insertions(+), 21 deletions(-) diff --git a/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py b/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py index 925659b9f..0b8735583 100644 --- a/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py +++ b/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py @@ -45,6 +45,7 @@ from cardpicker.question_feed import ( _tier_1_confirm_suggestion, _voter_answered_printing_card_ids, + get_next_question_feed_item, is_likely_resolve_printing, ) from cardpicker.tests.factories import ( @@ -345,6 +346,43 @@ def build(x_first: bool) -> list[float]: assert build(True) == build(False) == [1.0] + def test_a_self_contradicting_human_alone_does_not_resolve_the_group(self, db, md5_groups): + # the human-vote analogue of test_a_self_contradicting_machine_agent_contributes_nothing + # above: ONE human (source=user) voting X on one sibling and Y on the other has + # contradicted itself about byte-identical bytes, same as a machine agent would, and is + # withheld entirely - the group must not resolve on that agent's votes alone. + card_a, card_b = CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b) + printing_x, printing_y = CanonicalCardFactory(), CanonicalCardFactory() + human_vote(card_a, printing_x, "human-1") + human_vote(card_b, printing_y, "human-1") + + votes, is_group = group_printing_votes(card_a) + vote_tuples = build_group_printing_vote_tuples(votes, pool=is_group) + assert vote_tuples == [] + assert resolve_printing(card_a) is None + assert resolve_printing(card_b) is None + + def test_self_contradicting_human_withheld_not_latest_wins(self, db, md5_groups): + # issue #483: pins WHICH of the candidate contradiction policies this pipeline actually + # implements. h1 votes X on sibling1, then Y on sibling2 - a self-contradiction, withheld + # entirely per `pool_group_votes` rule 2. h2 votes Y once. A "latest wins" policy would + # count h1's most recent vote (Y) alongside h2's Y for a 2.0 quorum on Y and wrongly + # resolve the group; withhold-entirely leaves only h2's 1.0, short of + # PRINTING_TAG_MIN_VOTES=2, so the group stays unresolved. + card_1, card_2 = CardFactory(), CardFactory() + md5_groups("same-bytes", card_1, card_2) + printing_x, printing_y = CanonicalCardFactory(), CanonicalCardFactory() + human_vote(card_1, printing_x, "h1") + human_vote(card_2, printing_y, "h1") + human_vote(card_1, printing_y, "h2") + + votes, is_group = group_printing_votes(card_1) + vote_tuples = build_group_printing_vote_tuples(votes, pool=is_group) + assert [(vote.outcome_key, vote.dedupe_key) for vote in vote_tuples] == [(printing_y.pk, "h2")] + assert resolve_printing(card_1) is None + assert resolve_printing(card_2) is None + def test_a_group_never_resolves_on_machine_weight_alone(self, db, md5_groups): # four siblings, four DIFFERENT agents (nothing dedupes), 2.0 of machine weight - which # would clear the quorum threshold on arithmetic alone. The human-backed gate holds at @@ -567,3 +605,55 @@ def test_the_feed_serves_at_most_one_member_per_group(self, db, md5_groups): # identical question. human_vote(card_a, printing, "voter-1") assert _tier_1_confirm_suggestion("voter-1") is None + + +class TestGroupExpansionQueryCostWithoutTheChecksumColumn: + """ + `_md5_checksums_for_card_ids`'s and `md5_group_expanded_card_ids`'s own docstrings both claim + ZERO queries while `Card.md5_checksum` doesn't exist (issue #473's PR-1 hasn't merged into + this branch - see this module's docstring, point 2). Deliberately NOT using the `md5_groups` + fixture here: that fixture monkeypatches the three checksum-reading functions themselves, so + it can't tell us anything about what the REAL functions do when the column is genuinely + absent, which is exactly the claim being pinned. + """ + + def test_group_expansion_issues_zero_queries_without_the_checksum_column(self, db, django_assert_num_queries): + card_a, card_b = CardFactory(), CardFactory() + + with django_assert_num_queries(0): + expanded = md5_group_expanded_card_ids([card_a.pk, card_b.pk]) + + assert expanded == {card_a.pk, card_b.pk} + + def test_group_key_and_card_ids_issue_zero_queries_without_the_checksum_column(self, db, django_assert_num_queries): + card = CardFactory() + + with django_assert_num_queries(0): + key = md5_group_key(card) + card_ids = md5_group_card_ids(card) + + assert key == ("card", card.pk) + assert card_ids == [card.pk] + + +class TestAnsweredSetComputedOncePerFeedRequest: + """ + 2026-07-25 gate on PR #482, condition f1: `_voter_answered_printing_card_ids` must be resolved + ONCE per `get_next_question_feed_item` call and threaded down to every tier it consults, not + recomputed per tier. + """ + + def test_answered_set_is_computed_exactly_once(self, db, md5_groups): + card_a, card_b = CardFactory(), CardFactory() + md5_groups("same-bytes", card_a, card_b) + printing = CanonicalCardFactory() + machine_vote(card_a, printing, "ocr-bot") + machine_vote(card_b, printing, "ocr-bot") + + with patch( + "cardpicker.question_feed._voter_answered_printing_card_ids", + side_effect=_voter_answered_printing_card_ids, + ) as spy: + get_next_question_feed_item("voter-1") + + assert spy.call_count == 1 diff --git a/MPCAutofill/cardpicker/vote_consensus.py b/MPCAutofill/cardpicker/vote_consensus.py index e25192bcf..688b565dc 100644 --- a/MPCAutofill/cardpicker/vote_consensus.py +++ b/MPCAutofill/cardpicker/vote_consensus.py @@ -183,12 +183,13 @@ def pool_group_votes(votes: Iterable[VoteTuple]) -> list[VoteTuple]: re-labels one. What it therefore preserves EXACTLY is the machine-alone bound: no volume of non-human-backed weight can resolve a group, because the human-backed gate is untouched and pooling cannot manufacture a human-backed vote. What it makes true, rather than assumes, is - the independence the quorum threshold rests on: after pooling, `min_weight` counts distinct - agents, so neither one person answering n siblings nor one machine agent's evidence - transferred to n siblings (issue #473 PR-2) can reach quorum by repetition. Note the - direction of that claim carefully: pooling REPLACES a per-card tally with a group tally, and - a group tally can resolve things no single card's tally could (two different people, one - vote each, on two members) - that is the intended multiplier, not an accident. + the independence the quorum threshold rests on: after pooling, `min_weight` is a sum over + distinct agents rather than over rows, so neither one person answering n siblings nor one + machine agent's evidence transferred to n siblings (issue #473 PR-2) can reach quorum by + repetition. Note the direction of that claim carefully: pooling REPLACES a per-card tally + with a group tally, and a group tally can resolve things no single card's tally could (two + different people, one vote each, on two members) - that is the intended multiplier, not an + accident. Ordering: the retained set is a function of the votes, not of the order they arrive in (agreement is order-insensitive, and a contradicting agent is dropped wholesale), so a diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index 125efb13c..38a750183 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -171,10 +171,11 @@ printings, artists, tags, and moderation from one screen. the only writer, though a change in group MEMBERSHIP (checksum backfill, re-upload) needs a `consensus_recompute` pass for the affected group; `consensus_recompute` walks each group once; - `question_feed` classifies likely-resolve on the group tally and serves - at most one member of a group per voter. A card with a null or unique - checksum 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 + `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 + 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). - **Frontend consumer (funnel round, docs/features/grid-selector.md's diff --git a/docs/theory.md b/docs/theory.md index cea5a7948..2da73229c 100644 --- a/docs/theory.md +++ b/docs/theory.md @@ -291,9 +291,9 @@ turns out to be: target**, and is tallied as one, under a single rule: **the tally counts distinct agents, not rows.** All of one agent's agreeing votes across the group collapse into one β€” human and machine alike, keyed - on the caster's identity β€” and an agent whose votes across the group - _disagree_ with each other contributes nothing at all, on the same - withhold-don't-manufacture logic `gβ‚„` applies to its cross-checks + on the caster's identity β€” and any agent, human included, whose votes + across the group disagree with each other contributes nothing, on the + same withhold-don't-manufacture logic `gβ‚„` applies to its cross-checks (Β§7a). Distinct agents still sum, which is the point of grouping: two different people, one vote each, on two members of a group are two independent confirmations of one target. @@ -320,10 +320,10 @@ turns out to be: since the catalog is not permitted to hoard images and re-fetching is the expensive step) one machine observation can appear as _n_ agreeing confirmations. Group-level per-agent dedupe is what makes - `min_weight` a count of distinct agents again, and it is what keeps - Β§7a's `Ρ₁…Ρ₄` composition's mutual-independence assumption honest - once evidence is shared. This is a **restored** assumption, not a - new guarantee. + `min_weight` a sum over distinct agents rather than over rows, and it + is what keeps Β§7a's `Ρ₁…Ρ₄` composition's mutual-independence + assumption honest once evidence is shared. This is a **restored** + assumption, not a new guarantee. - **Explicitly NOT claimed**: that pooling reaches no new resolutions. It reaches some. A group tally replaces _n_ per-record tallies, so two different people voting on two different members @@ -344,10 +344,11 @@ turns out to be: checksum backfill, a re-upload, a corrected file), and a membership change requires a recompute for the affected group before its members are back in agreement β€” that recompute is `consensus_recompute`, - which walks groups, not rows. Human disagreement inside a group is - not special-cased: it is one visible contest on one target, decided - by the same vote-weight matrix as any other. A record whose checksum - is unknown or unique is a group of one, for which every statement + which walks groups, not rows. Human disagreement BETWEEN DISTINCT + PEOPLE inside a group is not special-cased: it is one visible contest + on one target, decided by the same vote-weight matrix as any other. A + record whose checksum is unknown or unique is a group of one, for + which every statement above is the identity β€” the pre-2026-07-25 per-record behavior, unchanged. From 6981a59e294148276eec434251d21b06e5d2a376 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Sat, 25 Jul 2026 21:07:45 +0000 Subject: [PATCH 4/4] Retire PR-2's interim Stage D guard now that group-level pooling covers it Per issue #473's own coordination note ("PR-3 removes PR-2's interim Stage D guard"): the join-key/fallback calculators no longer exclude transferred-evidence cards outright, since pool_group_votes now dedupes a transferred row's vote against its source at the group tally level instead. TRANSFERRED_INTERIM_GUARD_SKIP_REASON stays defined for historical CardScanLog rows only. Also fixes three comments (models.py, evidence_transfer.py, image_evidence.py) that mislocated the guard at _eligible_cards_queryset - it lived in the two calculators' own loop bodies. Co-Authored-By: Claude Fable 5 --- MPCAutofill/cardpicker/evidence_transfer.py | 22 +-- MPCAutofill/cardpicker/image_evidence.py | 13 +- .../cardpicker/local_calculate_verdicts.py | 89 ++++------- MPCAutofill/cardpicker/models.py | 39 ++--- .../tests/test_local_calculate_verdicts.py | 108 ++++++------- .../tests/test_md5_group_pooling.py | 143 +++++++++++++++--- docs/features/stage-e-operations.md | 35 +++-- docs/theory.md | 9 ++ 8 files changed, 275 insertions(+), 183 deletions(-) diff --git a/MPCAutofill/cardpicker/evidence_transfer.py b/MPCAutofill/cardpicker/evidence_transfer.py index 4ea3ea7d7..685b48d1b 100644 --- a/MPCAutofill/cardpicker/evidence_transfer.py +++ b/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: diff --git a/MPCAutofill/cardpicker/image_evidence.py b/MPCAutofill/cardpicker/image_evidence.py index a036f5d39..eaf893c5f 100644 --- a/MPCAutofill/cardpicker/image_evidence.py +++ b/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: diff --git a/MPCAutofill/cardpicker/local_calculate_verdicts.py b/MPCAutofill/cardpicker/local_calculate_verdicts.py index 59a74282a..e29f237c2 100644 --- a/MPCAutofill/cardpicker/local_calculate_verdicts.py +++ b/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") diff --git a/MPCAutofill/cardpicker/models.py b/MPCAutofill/cardpicker/models.py index 6d13d468e..c60238626 100755 --- a/MPCAutofill/cardpicker/models.py +++ b/MPCAutofill/cardpicker/models.py @@ -1988,25 +1988,26 @@ class ImageEvidence(models.Model): md5_checksum = models.CharField(max_length=32, null=True, blank=True, db_index=True) sha256_checksum = models.CharField(max_length=64, null=True, blank=True, db_index=True) - # transferred (issue #473 PR-2's INTERIM STAGE D GUARD, temporary by design): True iff this - # row's own field values were COPIED from an md5-sibling's own current evidence - # (evidence_transfer.transfer_evidence) rather than produced by a real fetch+extraction pass - # against this card's own image. `local_calculate_verdicts._eligible_cards_queryset`'s two - # MACHINE-VOTING Stage D calculators (join-key/fallback - both cast a `CardPrintingTag` vote) - # exclude any card whose CURRENT evidence carries this flag from machine voting - a transferred - # row's own machine "observation" is the SAME underlying bytes a sibling card already voted - # from, not an independent one, so casting a vote from it here would fabricate independence the - # vote-weight matrix assumes is real (docs/theory.md's independence-assumptions section). The - # third Stage D calculator, slow-path, is deliberately NOT guarded - it casts no machine vote, - # only a human-review routing marker, which is exactly the safety net this guard exists to - # preserve, not a case it needs to protect against. REMOVAL IS PR-3's OWN BUSINESS (issue - # #473's build plan, PR-3 section: "Removes PR-2's interim Stage D guard") - once group-level - # vote pooling lands, a transferred row's vote is correctly deduped at the GROUP level instead - # of excluded outright, so this flag (and the guard reading it) stops being needed; do not - # remove either before that PR merges. `transferred_from_card_id` is a plain (non-FK) audit - # trail of which sibling card's row this one was copied from - never queried by the guard - # itself, kept only for a future incident's own "why does this row look like that one" - # question. + # transferred: True iff this row's own field values were COPIED from an md5-sibling's own + # current evidence (evidence_transfer.transfer_evidence) rather than produced by a real + # fetch+extraction pass against this card's own image. Until issue #473 PR-3 merged + # (2026-07-25), `local_calculate_verdicts`'s two MACHINE-VOTING Stage D calculators (join-key/ + # fallback, IN THEIR OWN LOOP BODIES - not `_eligible_cards_queryset`, which never read this + # field) excluded any card whose CURRENT evidence carried this flag from machine voting + # outright: a transferred row's own machine "observation" is the SAME underlying bytes a + # sibling card already voted from, not an independent one, and casting a vote from it would + # have fabricated independence the vote-weight matrix assumes is real (docs/theory.md's + # independence-assumptions section). That interim guard is RETIRED as of PR-3 + # (`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 instead, by `vote_consensus.pool_group_votes` deduping a + # transferred card's vote against the sibling's it was copied from (both cast under the same + # calculator's fixed `anonymous_id`, so they share a `dedupe_key`), which is strictly more + # correct than excluding the vote outright: a transferred card can still contribute when a + # DIFFERENT agent is the one voting on the sibling. This field remains a plain, still-accurate + # provenance flag, read by no calculator anymore. `transferred_from_card_id` is a plain + # (non-FK) audit trail of which sibling card's row this one was copied from, kept only for a + # future incident's own "why does this row look like that one" question. transferred = models.BooleanField(default=False) transferred_from_card_id = models.IntegerField(null=True, blank=True) diff --git a/MPCAutofill/cardpicker/tests/test_local_calculate_verdicts.py b/MPCAutofill/cardpicker/tests/test_local_calculate_verdicts.py index 5fcc8d9a5..538230cb6 100644 --- a/MPCAutofill/cardpicker/tests/test_local_calculate_verdicts.py +++ b/MPCAutofill/cardpicker/tests/test_local_calculate_verdicts.py @@ -764,15 +764,21 @@ def _stale_split(votes_batch): assert CardPrintingTag.objects.filter(card=card, anonymous_id=JOIN_KEY_ANONYMOUS_ID).count() == 1 -class TestTransferredInterimGuard: - """Issue #473 PR-2's INTERIM STAGE D GUARD (see TRANSFERRED_INTERIM_GUARD_SKIP_REASON's own - module-level comment in local_calculate_verdicts.py): a card whose CURRENT evidence row was - created via evidence transfer must never receive a machine vote from the join-key or fallback - calculators - its own "observation" is the same bytes an md5-sibling already voted from, not - an independent one. The slow-path calculator is NOT guarded (it casts no machine vote, only a - human-review routing marker - see its own run_slow_path_calculator loop comment).""" - - def test_join_key_skips_a_transferred_evidence_card(self, db): +class TestTransferredEvidenceIsEligible: + """Issue #473 PR-2's INTERIM STAGE D GUARD (TRANSFERRED_INTERIM_GUARD_SKIP_REASON) excluded a + card whose CURRENT evidence row was created via evidence transfer from the join-key/fallback + calculators outright - its own "observation" is the same bytes an md5-sibling already voted + from, not an independent one. PR-3 (2026-07-25, owner-ratified group-level vote pooling) + RETIRES that guard: a transferred card is now exactly as eligible as any other card in every + Stage D calculator, because the independence concern the guard existed to protect is now + handled correctly at the GROUP tally level instead (`vote_consensus.pool_group_votes` - see + `test_md5_group_pooling.py::TestTransferredEvidencePoolsWithItsSource` for the pooling-level + pin of that same claim). These tests replace the old skip-assertions with the mirror-image + vote-cast assertions; `TRANSFERRED_INTERIM_GUARD_SKIP_REASON` itself stays imported/exported + only because historical `CardScanLog` rows from before this change still carry it (see its own + module-level comment) - no test here expects a NEW row with that reason.""" + + def test_join_key_votes_a_transferred_evidence_card(self, db): card = CardFactory(name="Some Card", content_phash=42) CanonicalCardFactory(name="Some Card", expansion__code="mom", collector_number="158") _evidence( @@ -785,75 +791,69 @@ def test_join_key_skips_a_transferred_evidence_card(self, db): result = run_join_key_calculator(dry_run=False) - assert result.cards_considered == 0 - assert result.votes_written == 0 - assert CardPrintingTag.objects.count() == 0 - log = CardScanLog.objects.get(card=card, anonymous_id=JOIN_KEY_ANONYMOUS_ID) - assert log.skip_reason == TRANSFERRED_INTERIM_GUARD_SKIP_REASON - - def test_join_key_still_votes_a_real_extraction_card(self, db): - """Control - the exact same evidence, minus transferred=True, votes normally. Proves the - guard is keyed on the flag, not some other incidental difference in the fixture.""" - card = CardFactory(name="Some Card", content_phash=42) - CanonicalCardFactory(name="Some Card", expansion__code="mom", collector_number="158") - _evidence( - card, - collector_line_set_code="mom", - collector_line_collector_number="158", - transferred=False, - ) - - result = run_join_key_calculator(dry_run=False) - assert result.cards_considered == 1 assert result.votes_written == 1 - assert CardPrintingTag.objects.filter(card=card).count() == 1 - - def test_transferred_guard_is_rescannable_after_a_real_extraction_lands(self, db): - card = CardFactory(name="Some Card", content_phash=42) + vote = CardPrintingTag.objects.get(card=card, anonymous_id=JOIN_KEY_ANONYMOUS_ID) + assert vote.is_no_match is False + # no historical-guard skip row is written for this card at all + assert not CardScanLog.objects.filter( + card=card, anonymous_id=JOIN_KEY_ANONYMOUS_ID, skip_reason=TRANSFERRED_INTERIM_GUARD_SKIP_REASON + ).exists() + + def test_join_key_votes_identically_regardless_of_transferred(self, db): + """Control - the exact same evidence, minus transferred=True, casts the same vote. Proves + the retired guard's absence is a true no-op on outcome, not just "doesn't skip".""" + card_transferred = CardFactory(name="Some Card", content_phash=42) + card_real = CardFactory(name="Some Card", content_phash=43) CanonicalCardFactory(name="Some Card", expansion__code="mom", collector_number="158") _evidence( - card, + card_transferred, collector_line_set_code="mom", collector_line_collector_number="158", transferred=True, transferred_from_card_id=999, ) + _evidence( + card_real, + collector_line_set_code="mom", + collector_line_collector_number="158", + transferred=False, + ) - first = run_join_key_calculator(dry_run=False) - assert first.cards_considered == 0 - - # a real extraction pass lands (persist_evidence always clears transferred - see - # image_evidence.persist_evidence's own docstring) - re-running now casts the vote. - evidence = card.image_evidence.get() - evidence.transferred = False - evidence.transferred_from_card_id = None - evidence.save() + result = run_join_key_calculator(dry_run=False) - second = run_join_key_calculator(dry_run=False) - assert second.cards_considered == 1 - assert second.votes_written == 1 + assert result.cards_considered == 2 + assert result.votes_written == 2 + transferred_vote = CardPrintingTag.objects.get(card=card_transferred) + real_vote = CardPrintingTag.objects.get(card=card_real) + assert transferred_vote.printing_id == real_vote.printing_id - def test_fallback_skips_a_transferred_evidence_card(self, db): + def test_fallback_votes_a_transferred_evidence_card(self, db): card = CardFactory(name="Some Card", content_phash=42) + printing = CanonicalCardFactory(name="Some Card", expansion__code="mom", collector_number="158") + CanonicalPrintingMetadataFactory(canonical_card=printing, border_color="black") CardScanLog.objects.create(card=card, anonymous_id=JOIN_KEY_ANONYMOUS_ID, skip_reason="no-text") _evidence( card, - collector_line_collector_number="", - symbol_phash=_hash_of("mom"), + layout_class="black", transferred=True, transferred_from_card_id=999, ) result = run_fallback_calculator(dry_run=False) - assert result.cards_considered == 0 - log = CardScanLog.objects.get(card=card, anonymous_id=STAGE_D_FALLBACK_ANONYMOUS_ID) - assert log.skip_reason == TRANSFERRED_INTERIM_GUARD_SKIP_REASON + assert result.cards_considered == 1 + assert result.votes_written == 1 + vote = CardPrintingTag.objects.get(card=card, anonymous_id=STAGE_D_FALLBACK_ANONYMOUS_ID) + assert vote.printing_id == printing.pk + assert not CardScanLog.objects.filter( + card=card, anonymous_id=STAGE_D_FALLBACK_ANONYMOUS_ID, skip_reason=TRANSFERRED_INTERIM_GUARD_SKIP_REASON + ).exists() - def test_slow_path_is_not_guarded_transferred_evidence_still_routes_to_review(self, db): + def test_slow_path_still_routes_transferred_evidence_to_review(self, db): """The slow-path calculator casts no machine vote (only a CardScanLog routing marker to a - HUMAN reviewer) - deliberately excluded from the guard, see its own loop comment.""" + HUMAN reviewer) - it was never guarded on `transferred` either before or after PR-3, so + this behavior is unchanged; see its own loop comment.""" card = CardFactory(name="Some Card", content_phash=42) CardPrintingTag.objects.create( card=card, printing=None, is_no_match=True, anonymous_id=JOIN_KEY_ANONYMOUS_ID, source=VoteSource.OCR diff --git a/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py b/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py index 0b8735583..24d8f752a 100644 --- a/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py +++ b/MPCAutofill/cardpicker/tests/test_md5_group_pooling.py @@ -9,13 +9,17 @@ collapse), and an agent that contradicts itself counts for nothing (order-independently). 2. Everything else exercises the real resolver/persistence/feed/recompute paths against real `Card`/`CardPrintingTag` rows, with group MEMBERSHIP supplied by the `md5_groups` fixture - below rather than by a populated `Card.md5_checksum` column - because that column arrives in - this issue's PR-1 (`md5-checksum-substrate`), which this branch is cut BEFORE. The fixture - replaces the three - and only three - functions in `printing_consensus` that touch the column - (`_card_md5_checksum`, `_md5_checksums_for_card_ids`, `_card_ids_with_md5_checksums`), so - every line of grouping, pooling, propagation, and feed logic under test is the real one; only - the storage of the checksum is faked. Once PR-1 is merged into this branch, these tests keep - passing unchanged and can be supplemented with column-backed equivalents. + below rather than by a populated `Card.md5_checksum` column - originally because that column + hadn't arrived yet (issue #473's PR-1, `md5-checksum-substrate`, which this module was first + written cut BEFORE). PR-1 has SINCE merged to `master` and reached this branch via the + 2026-07-25 master merge - `Card.md5_checksum` is a real column here now - but the fixture is + kept as the grouping mechanism regardless, since it exercises the exact same real grouping/ + pooling/propagation/feed code paths a column-backed group does (the fixture replaces the three + - and only three - functions in `printing_consensus` that touch the column: + `_card_md5_checksum`, `_md5_checksums_for_card_ids`, `_card_ids_with_md5_checksums`) without + needing every test to hand-craft matching checksum strings. `TestSingletonIsANoOp` and + `TestGroupExpansionQueryCostWithoutTheChecksumColumn` below deliberately do NOT use the + fixture, to pin real, column-backed behavior directly. The SINGLETON NO-OP proof (ruling 3) is deliberately NOT concentrated in one test here: it is the entire pre-existing consensus/printing/tag/question-feed/recompute suite, which passes @@ -30,8 +34,12 @@ import pytest from cardpicker import printing_consensus +from cardpicker.local_calculate_verdicts import ( + JOIN_KEY_ANONYMOUS_ID, + run_join_key_calculator, +) from cardpicker.management.commands.consensus_recompute import run_consensus_recompute -from cardpicker.models import Card, PrintingTagStatus, VoteSource +from cardpicker.models import Card, CardPrintingTag, PrintingTagStatus, VoteSource from cardpicker.printing_consensus import ( build_group_printing_vote_tuples, group_printing_votes, @@ -52,17 +60,20 @@ CanonicalCardFactory, CardFactory, CardPrintingTagFactory, + ImageEvidenceFactory, ) -from cardpicker.vote_consensus import VoteTuple, pool_group_votes +from cardpicker.vote_consensus import VoteTuple, pool_group_votes, resolve_vote_weight @pytest.fixture def md5_groups(monkeypatch): """ - Assigns md5 identity groups to `Card` rows without `Card.md5_checksum` existing yet (see this - module's docstring). Returns a callable: `md5_groups("checksum", card_a, card_b)` puts those - cards in one group. Cards never passed to it stay checksum-less - i.e. groups of one, the - ruling-3 degenerate case - exactly as every card in the catalogue is today. + Assigns md5 identity groups to `Card` rows via an in-memory stand-in rather than a populated + `Card.md5_checksum` column (see this module's own docstring, point 2, for why the fixture + approach is kept even now that the column is real on this branch). Returns a callable: + `md5_groups("checksum", card_a, card_b)` puts those cards in one group. Cards never passed to + it stay checksum-less - i.e. groups of one, the ruling-3 degenerate case - exactly as most of + the catalogue still is pending the backfill (`backfill_md5_checksums`) fully enrolling it. """ checksum_by_card_id: dict[int, str] = {} @@ -609,23 +620,32 @@ def test_the_feed_serves_at_most_one_member_per_group(self, db, md5_groups): class TestGroupExpansionQueryCostWithoutTheChecksumColumn: """ - `_md5_checksums_for_card_ids`'s and `md5_group_expanded_card_ids`'s own docstrings both claim - ZERO queries while `Card.md5_checksum` doesn't exist (issue #473's PR-1 hasn't merged into - this branch - see this module's docstring, point 2). Deliberately NOT using the `md5_groups` - fixture here: that fixture monkeypatches the three checksum-reading functions themselves, so - it can't tell us anything about what the REAL functions do when the column is genuinely - absent, which is exactly the claim being pinned. + Deliberately NOT using the `md5_groups` fixture here: that fixture monkeypatches the three + checksum-reading functions themselves, so it can't tell us anything about what the REAL + functions cost - which is exactly the claim being pinned. `Card.md5_checksum` is a real column + on this branch (issue #473's PR-1 merged to `master` and reached here via the 2026-07-25 + master merge - see this module's own docstring, point 2), so `_md5_checksums_for_card_ids` + now issues its one checksum-lookup query even for card_ids that carry no checksum at all - + it returns an empty result rather than skipping the query outright, and its own docstring's + "zero queries... while the column doesn't exist" clause no longer applies on this branch. + What stays true, and is what this class actually pins, is the CHEAP-CASE bound: at most ONE + query for the common no-checksums-set-yet case, never the second (`_card_ids_with_md5_checksums`) + lookup, because the first query's empty result short-circuits before that second one is ever + issued. `md5_group_key`/`md5_group_card_ids` are unaffected either way - `_card_md5_checksum` + is a plain `getattr` on an already-loaded instance, never a query, column present or not. """ - def test_group_expansion_issues_zero_queries_without_the_checksum_column(self, db, django_assert_num_queries): + def test_group_expansion_issues_at_most_one_query_and_never_the_second_lookup(self, db, django_assert_num_queries): card_a, card_b = CardFactory(), CardFactory() - with django_assert_num_queries(0): + with django_assert_num_queries(1): expanded = md5_group_expanded_card_ids([card_a.pk, card_b.pk]) assert expanded == {card_a.pk, card_b.pk} - def test_group_key_and_card_ids_issue_zero_queries_without_the_checksum_column(self, db, django_assert_num_queries): + def test_group_key_and_card_ids_issue_zero_queries_regardless_of_the_checksum_column( + self, db, django_assert_num_queries + ): card = CardFactory() with django_assert_num_queries(0): @@ -657,3 +677,82 @@ def test_answered_set_is_computed_exactly_once(self, db, md5_groups): get_next_question_feed_item("voter-1") assert spy.call_count == 1 + + +def _join_key_evidence(card, **overrides): + defaults = dict( + content_hash=card.content_phash, + extractor_versions={"collector_line_ocr": "collector-line-ocr-v1"}, + collector_line_set_code="mom", + collector_line_collector_number="158", + transferred=False, + ) + defaults.update(overrides) + return ImageEvidenceFactory(card=card, **defaults) + + +class TestTransferredEvidencePoolsWithItsSource: + """ + Issue #473 PR-3 (2026-07-25): retiring the interim Stage D guard + (`local_calculate_verdicts.TRANSFERRED_INTERIM_GUARD_SKIP_REASON`) makes a transferred- + evidence card exactly as Stage-D-eligible as any other card - see + test_local_calculate_verdicts.py::TestTransferredEvidenceIsEligible for that half of the pin + (the guard's ABSENCE is intentional, not a regression). THIS class pins the other half: group- + level pooling is what actually prevents the guard's old failure mode (a transferred row's vote + fabricating an independent second confirmation of the same underlying bytes), because the + source card's REAL extraction and its md5 sibling's TRANSFERRED copy of it are cast by the + SAME calculator under one fixed `anonymous_id` (`JOIN_KEY_ANONYMOUS_ID`) - so `pool_group_ + votes` collapses them to ONE event, the same rule that collapses one human voting twice. + """ + + def test_transferred_sibling_vote_pools_with_its_source_instead_of_doubling_it(self, db, md5_groups): + card_source, card_transferred = ( + CardFactory(name="Some Card", content_phash=42), + CardFactory(name="Some Card", content_phash=42), + ) + md5_groups("same-bytes", card_source, card_transferred) + CanonicalCardFactory(name="Some Card", expansion__code="mom", collector_number="158") + _join_key_evidence(card_source, transferred=False) + _join_key_evidence(card_transferred, transferred=True, transferred_from_card_id=card_source.pk) + + result = run_join_key_calculator(dry_run=False) + + # both cards are Stage-D-eligible and both cast a vote - the retired guard no longer + # excludes the transferred one (mirrors TestTransferredEvidenceIsEligible's own pin). + assert result.cards_considered == 2 + assert result.votes_written == 2 + assert CardPrintingTag.objects.filter(anonymous_id=JOIN_KEY_ANONYMOUS_ID).count() == 2 + + # but the GROUP tally pools them to ONE event, since both votes share JOIN_KEY_ANONYMOUS_ID + votes, is_group = group_printing_votes(card_source) + assert len(votes) == 2 # both rows are read... + vote_tuples = build_group_printing_vote_tuples(votes, pool=is_group) + assert len(vote_tuples) == 1 # ...and pool to ONE event + assert vote_tuples[0].weight == resolve_vote_weight(VoteSource.OCR, JOIN_KEY_ANONYMOUS_ID) + + # never enough to resolve alone - exactly the outcome the retired guard used to guarantee + # by excluding the transferred vote outright, now achieved by pooling instead. + assert resolve_printing(card_source) is None + assert resolve_printing(card_transferred) is None + + def test_a_second_distinct_agent_can_still_tip_a_group_containing_a_transferred_vote(self, db, md5_groups): + """Contrast: pooling doesn't turn the transferred card into dead weight excluded from + ever influencing an outcome (that WAS the old guard's effect) - a genuinely independent + second agent's vote on the group still resolves it, same as any other group.""" + card_source, card_transferred = ( + CardFactory(name="Some Card", content_phash=42), + CardFactory(name="Some Card", content_phash=42), + ) + md5_groups("same-bytes", card_source, card_transferred) + printing = CanonicalCardFactory(name="Some Card", expansion__code="mom", collector_number="158") + _join_key_evidence(card_source, transferred=False) + _join_key_evidence(card_transferred, transferred=True, transferred_from_card_id=card_source.pk) + + run_join_key_calculator(dry_run=False) + assert resolve_printing(card_source) is None # 0.5 pooled machine weight, short of quorum + + human_vote(card_transferred, printing, "human-1") + human_vote(card_source, printing, "human-2") + + assert resolve_printing(card_source) == printing + assert resolve_printing(card_transferred) == printing diff --git a/docs/features/stage-e-operations.md b/docs/features/stage-e-operations.md index 1ebadfb7c..83504cc01 100644 --- a/docs/features/stage-e-operations.md +++ b/docs/features/stage-e-operations.md @@ -387,19 +387,28 @@ source never carries an md5 at all (e.g. `LOCAL_FILE`), stays current under the content-hash check alone β€” only a row that stamped a REAL, actively DISAGREEING md5 is treated as stale. -**INTERIM Stage D guard (#473 PR-2, temporary by design).** A card whose -CURRENT evidence row was created by transfer is excluded from all three -Stage D calculators (`TRANSFERRED_INTERIM_GUARD_SKIP_REASON`, -rescannable) β€” its own machine "observation" is the same bytes a sibling -card already voted from, not an independent one, so casting a vote here -would fabricate the independence the vote-weight matrix assumes is real. -The slow-path calculator is deliberately NOT guarded β€” it casts no machine -vote, only a `CardScanLog` routing marker to a HUMAN reviewer, which is -exactly the safety net the guard exists to preserve. **Removal is PR-3's -own business** (issue #473's build plan: group-level vote pooling correctly -dedupes a transferred row's vote at the GROUP level instead of excluding it -outright) β€” do not remove the guard, or the `ImageEvidence.transferred` -flag it reads, before that PR merges. +**Interim Stage D guard, RETIRED (#473 PR-2, added temporary-by-design; +removed by PR-3, 2026-07-25).** From PR-2 until PR-3 merged, a card whose +CURRENT evidence row was created by transfer was excluded outright from the +two MACHINE-VOTING Stage D calculators, join-key and fallback +(`TRANSFERRED_INTERIM_GUARD_SKIP_REASON`, rescannable, lived in each +calculator's own loop body β€” never in `_eligible_cards_queryset`) β€” its own +machine "observation" is the same bytes a sibling card already voted from, +not an independent one, so casting a vote here would have fabricated the +independence the vote-weight matrix assumes is real. The slow-path +calculator was never guarded either way β€” it casts no machine vote, only a +`CardScanLog` routing marker to a HUMAN reviewer, which is exactly the +safety net the guard existed to preserve. Issue #473's own PR-3 (group-level +vote pooling, `vote_consensus.pool_group_votes`) now handles that same +independence concern correctly at the GROUP tally level instead: a +transferred row and the sibling it was copied from are both cast under the +SAME calculator's fixed `anonymous_id`, so they share one `pool_group_votes` +`dedupe_key` and collapse to one event rather than being excluded outright β€” +strictly more correct, since a transferred card can still contribute when a +DIFFERENT agent votes on its sibling. `TRANSFERRED_INTERIM_GUARD_SKIP_REASON` +stays defined for historical `CardScanLog` rows' readability; no code path +writes a new one. See `docs/theory.md` Β§4 item 3 for the full soundness +argument. **Decoupled fetch-ahead (#472).** `stage_e_streaming.md` Β§4 item 3 ratified "adopt, unconditionally" β€” the streaming conveyor's compute stage should diff --git a/docs/theory.md b/docs/theory.md index 2da73229c..f42356e08 100644 --- a/docs/theory.md +++ b/docs/theory.md @@ -1130,3 +1130,12 @@ yet exist in the written record β€” see Β§10's own "Not yet backed by written data" note. Like §§7-9, Β§10's **text** is pending the same owner review §§1-6 received; the top-of-document STATUS banner is unchanged by this addition. + +**Β§4 item 3 (identity-group pooling) reviewed and approved by the +owner, 2026-07-25**: the delta-gate round addressing the 2026-07-25 +NO-GO's wording conditions (the withholding rule's "any agent, human +included" phrasing, the "sum over distinct agents rather than over +rows" correction in two places, and the "human disagreement BETWEEN +DISTINCT PEOPLE" scoping) closes that gate. Text status matches Β§7c/Β§7d +above: corroborating a mechanism already merged (`ec18ecd8`, +`vote_consensus.pool_group_votes`), not a new calibrated number.