questionFeed: exclude N>1 illustration-vote answers; dedupe contested_card_ids across the view

MPCAutofill/cardpicker/question_feed.py

@@ -84,6 +84,7 @@
     ArtistVoteStatus,
     Card,
     CardArtistVote,
+    CardIllustrationVote,
     CardPrintingTag,
     CardScanLog,
     CardTagVote,
@@ -258,14 +259,38 @@ def _voter_answered_printing_card_ids(anonymous_id: str) -> set[int]:
     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`).
 
+    ALSO reads `CardIllustrationVote` (issue #713). `identify_printing` has two answer paths on
+    the frontend: a single/unclustered candidate posts straight to `CardPrintingTag`, but a
+    shared-illustration cluster of N>=2 candidates posts to `illustration_vote.
+    cast_illustration_vote` instead, which writes `CardPrintingTag` ONLY when the illustration
+    resolves to exactly one live printing (see that function's own docstring) - at N>1, the
+    premise of the cluster UI that fired, nothing lands on the printing channel at all. Without
+    this, a voter who answered via the cluster path cast a real, persisted vote
+    (`CardIllustrationVote` is always written) that this exclusion could not see, so the card
+    stayed eligible and was immediately re-served - production evidence: both of the only two
+    human illustration votes on record were each followed within seconds by an `is_no_match`
+    escape vote on the same card. Every `CardIllustrationVote` row - not just the N>1 case - is
+    included unconditionally: for the N=1 case the card is already covered by the
+    `CardPrintingTag` query above, so this is a no-op union there, not a special case to branch
+    on; keeping it unconditional means one query shape covers both outcomes of
+    `cast_illustration_vote` rather than two.
+
+    NOT `CardQuestionAbstention` (issue #712/#731): that model records a human "Not sure" - a
+    real non-answer - and reusing it here for a real, weighted answer would conflate the two,
+    corrupting the exact distinction #712 was built to preserve. See this function's own tests
+    and the issue #713 PR description for the full reasoning.
+
     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 identity_group_expanded_card_ids(voted_card_ids)
+    voted_card_ids = set(CardPrintingTag.objects.filter(anonymous_id=anonymous_id).values_list("card_id", flat=True))
+    illustration_voted_card_ids = set(
+        CardIllustrationVote.objects.filter(anonymous_id=anonymous_id).values_list("card_id", flat=True)
+    )
+    return identity_group_expanded_card_ids(voted_card_ids | illustration_voted_card_ids)
 
 
 def _voter_answered_artist_card_ids(anonymous_id: str) -> set[int]:
@@ -647,7 +672,9 @@ def _log_served(anonymous_id: str, item: QuestionFeedItem, pool: str, origin_rea
     return item
 
 
-def get_next_question_feed_item(anonymous_id: str) -> Optional[QuestionFeedItem]:
+def get_next_question_feed_item(
+    anonymous_id: str, contested_card_ids: Optional[list[int]] = None
+) -> Optional[QuestionFeedItem]:
     """
     The ranked union itself. When this session's served-mix ratio (`_served_mix_ratio`) is
     below `settings.QUESTION_FEED_LIKELY_RESOLVE_MIX_RATIO` AND the likely-resolve pool still
@@ -670,6 +697,14 @@ def get_next_question_feed_item(anonymous_id: str) -> Optional[QuestionFeedItem]
     phase-C routing signal: a card a human has declared not-official-art via `reason_tags.
     NOT_OFFICIAL_ART_REASON_TAGS` stops being served as an artist-shaped question, in both
     `_tier_2_contested` and `_tier_4_fresh` - printing questions are unaffected).
+
+    `contested_card_ids` is an optional pre-resolved value (issue #713 part 2, extending PR
+    #729's "compute once, thread as an optional parameter" convention across the view boundary
+    for the first time): `views.get_question_feed` calls `get_contested_card_ids()` once per
+    HTTP request and passes the result to both this function and `get_remaining_estimate`, since
+    both independently called it before (measured 520-562ms per call against live production
+    data) even though neither can see a vote the other cast mid-request. `None` (every direct
+    caller - tests, a shell) still resolves it here exactly as before.
     """
     answered_card_ids = _voter_answered_printing_card_ids(anonymous_id)
     answered_artist_card_ids = _voter_answered_artist_card_ids(anonymous_id)
@@ -688,12 +723,13 @@ def get_next_question_feed_item(anonymous_id: str) -> Optional[QuestionFeedItem]
     if tier_1_item is not None:
         return _log_served(anonymous_id, tier_1_item, QuestionFeedServedPool.REMAINDER, "tier_1_confirm_suggestion")
 
-    # `contested_card_ids`/`contested_artist_card_ids` are resolved ONCE here, only once we've
-    # actually fallen through to the tiers that consult them (tier 1 and the likely-resolve pool
-    # above never touch either), and reused by both tier 2 and tier 4 below - each is otherwise
+    # `contested_card_ids` may already have arrived from the caller (see docstring above); only
+    # resolve it here if not, and only once we've actually fallen through to the tiers that
+    # consult it (tier 1 and the likely-resolve pool above never touch it) - each is otherwise
     # identical on repeat calls within this same request (no vote can be cast mid-request), so
     # recomputing it once per tier just paid the same cost twice for one answer.
-    contested_card_ids = get_contested_card_ids()
+    if contested_card_ids is None:
+        contested_card_ids = get_contested_card_ids()
     contested_artist_card_ids = get_contested_artist_card_ids()
 
     tier_2_result = _tier_2_contested(
@@ -742,7 +778,7 @@ def _tag_review_card_ids_by_status() -> tuple[set[int], set[int]]:
     return contested_ids, unresolved_ids
 
 
-def get_remaining_estimate() -> QuestionFeedCounts:
+def get_remaining_estimate(contested_card_ids: Optional[list[int]] = None) -> QuestionFeedCounts:
     """
     "Still need help with" counts for the feed header - NOT per-voter (doesn't account for
     own-vote exclusion, which is comparatively cheap to skip here since this is advisory copy,
@@ -772,8 +808,14 @@ def get_remaining_estimate() -> QuestionFeedCounts:
     per bucket (4 buckets), for 6 queries overall. No per-card sub-queries in a loop - the only
     Python-side materialization is the tag-status scan, which was already the established
     pattern for this JSONField (see `_tag_review_card_ids_by_status`'s docstring).
+
+    `contested_card_ids` is an optional pre-resolved value - see `get_next_question_feed_item`'s
+    matching parameter docstring for why (issue #713 part 2): `views.get_question_feed` calls
+    `get_contested_card_ids()` once and passes it to both functions, since this one always needs
+    it and the other needed it as often. `None` (every other caller, e.g. `catalog_stats.py`)
+    resolves it here exactly as before.
     """
-    contested_printing_ids = get_contested_card_ids()
+    contested_printing_ids = contested_card_ids if contested_card_ids is not None else get_contested_card_ids()
     tag_contested_ids, tag_unresolved_ids = _tag_review_card_ids_by_status()
 
     confirmable = (

MPCAutofill/cardpicker/tests/test_question_feed.py

@@ -1,3 +1,4 @@
+import uuid
 from unittest.mock import patch
 
 from django.urls import reverse
@@ -7,12 +8,14 @@
     get_contested_artist_card_ids,
     resolve_and_persist_artist,
 )
+from cardpicker.illustration_vote import cast_illustration_vote
 from cardpicker.local_calculate_verdicts import (
     JOIN_KEY_ANONYMOUS_ID,
     JOIN_KEY_UNKNOWN_SET_CODE_SKIP_REASON,
 )
 from cardpicker.models import (
     ArtistVoteStatus,
+    CardPrintingTag,
     CardScanLog,
     PrintingTagStatus,
     QuestionFeedServedLog,
@@ -30,6 +33,7 @@
     _artist_item,
     _scryfall_illustration_url,
     _tier_1_confirm_suggestion,
+    _voter_answered_printing_card_ids,
     get_next_question_feed_item,
     get_remaining_estimate,
     is_likely_resolve_printing,
@@ -47,6 +51,20 @@
 )
 
 
+def make_shared_illustration_group(name: str = "Brainstorm") -> tuple:
+    """Two live printing candidates for a fresh `card`, sharing one `illustration_id` - the N>1
+    shared-illustration-group premise `cast_illustration_vote` (illustration_vote.py) requires to
+    take its no-CardPrintingTag-write branch. Mirrors test_illustration_vote.py's own
+    `_printing_with_illustration` helper."""
+    card = CardFactory(name=name, printing_tag_status=PrintingTagStatus.UNRESOLVED)
+    illustration_id = uuid.uuid4()
+    artist = CanonicalArtistFactory(name="Shared Artist")
+    for _ in range(2):
+        printing = CanonicalCardFactory(name=name, artist=artist)
+        CanonicalPrintingMetadataFactory(canonical_card=printing, illustration_id=illustration_id)
+    return card, illustration_id
+
+
 def make_ai_suggested_card(anonymous_id: str = "ai-bot") -> tuple:
     card = CardFactory(printing_tag_status=PrintingTagStatus.UNRESOLVED)
     printing = CanonicalCardFactory()
@@ -745,3 +763,45 @@ def test_tier_4_does_not_treat_ambiguous_origin_as_quick_negative(self, db):
         assert item is not None
         log = QuestionFeedServedLog.objects.get(anonymous_id="anon-1")
         assert log.origin_reason != "tier_4_quick_negative_to_review"
+
+
+class TestIllustrationVoteAnsweredExclusion:
+    """Issue #713: `cast_illustration_vote` writes `CardPrintingTag` only when the shared-
+    illustration group resolves to exactly one live printing - at N>1 (the premise of the
+    cluster UI that triggers this path) it writes only `CardIllustrationVote`, which the
+    printing exclusion below must also read or the voter is re-served the very card they just
+    answered."""
+
+    def test_voter_answered_printing_card_ids_includes_n_gt_1_illustration_votes(self, db):
+        card, illustration_id = make_shared_illustration_group()
+
+        outcome = cast_illustration_vote(
+            card=card,
+            anonymous_id="voter-1",
+            illustration_id=illustration_id,
+            is_unknown=False,
+            user=None,
+            vote_surface="question-feed",
+        )
+
+        assert outcome.printing_vote_cast is False
+        assert CardPrintingTag.objects.filter(card=card, anonymous_id="voter-1").count() == 0
+        assert card.pk in _voter_answered_printing_card_ids("voter-1")
+
+    def test_a_voter_who_answers_an_n_gt_1_illustration_group_is_not_re_served_that_card(self, db):
+        card, illustration_id = make_shared_illustration_group()
+
+        first_item = get_next_question_feed_item("voter-1")
+        assert first_item is not None
+        assert first_item.card.identifier == card.identifier
+
+        cast_illustration_vote(
+            card=card,
+            anonymous_id="voter-1",
+            illustration_id=illustration_id,
+            is_unknown=False,
+            user=None,
+            vote_surface="question-feed",
+        )
+
+        assert get_next_question_feed_item("voter-1") is None

MPCAutofill/cardpicker/views.py

@@ -2010,8 +2010,13 @@ def get_question_feed(request: HttpRequest) -> HttpResponse:
     if not anonymous_id:
         raise BadRequestException("Missing required anonymousId query parameter.")
 
-    item = get_next_question_feed_item(anonymous_id)
-    remaining_estimate = get_remaining_estimate()
+    # Resolved once and threaded to both calls below: `get_next_question_feed_item` and
+    # `get_remaining_estimate` each independently called `get_contested_card_ids()` before,
+    # measured at 520-562ms per call against live production data - on a single-gunicorn-worker
+    # deployment that's site-wide latency on every request, not one user's.
+    contested_card_ids = get_contested_card_ids()
+    item = get_next_question_feed_item(anonymous_id, contested_card_ids=contested_card_ids)
+    remaining_estimate = get_remaining_estimate(contested_card_ids)
     return JsonResponse(QuestionFeedResponse(item=item, remainingEstimate=remaining_estimate).model_dump())
 
 

docs/features/printing-tags.md

@@ -967,7 +967,10 @@ for history (this doc's own established convention — see the `cardPanel.tsx` b
     illustration resolves, against LIVE data at write time, to exactly one
     candidate printing for this card (`printings_for_card_and_illustration`
     — N>1 casts nothing on the printing channel, matching #526's machine-side
-    rule); and a `CardArtistVote` is derived (`source=USER`,
+    rule; `question_feed._voter_answered_printing_card_ids` reads
+    `CardIllustrationVote` as well as `CardPrintingTag` so an N>1 answer still
+    excludes the card from the printing tiers, issue #713); and a
+    `CardArtistVote` is derived (`source=USER`,
     `vote_surface="illustration_vote_derived_artist"`) whenever the
     resolved artist's name doesn't indicate a combined credit (tests for
     `' & '` only — see the module's own census comment) and no