Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions MPCAutofill/cardpicker/question_feed.py
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,8 @@ def _tier_2_contested(
answered_artist_card_ids: Optional[set[int]] = None,
answered_tag_card_ids_by_tag: Optional[dict[str, set[int]]] = None,
not_official_art_card_ids: Optional[set[int]] = None,
contested_card_ids: Optional[list[int]] = None,
contested_artist_card_ids: Optional[list[int]] = None,
) -> Optional[tuple[QuestionFeedItem, str]]:
if answered_card_ids is None:
answered_card_ids = _voter_answered_printing_card_ids(anonymous_id)
Expand All @@ -482,9 +484,13 @@ def _tier_2_contested(
answered_tag_card_ids_by_tag = _voter_answered_tag_card_ids_by_tag(anonymous_id)
if not_official_art_card_ids is None:
not_official_art_card_ids = _not_official_art_card_ids()
if contested_card_ids is None:
contested_card_ids = get_contested_card_ids()
if contested_artist_card_ids is None:
contested_artist_card_ids = get_contested_artist_card_ids()

printing_card = (
Card.objects.filter(printing_tag_status=PrintingTagStatus.UNRESOLVED, pk__in=get_contested_card_ids())
Card.objects.filter(printing_tag_status=PrintingTagStatus.UNRESOLVED, pk__in=contested_card_ids)
.exclude(pk__in=answered_card_ids)
.order_by("-date_created")
.first()
Expand All @@ -493,7 +499,7 @@ def _tier_2_contested(
return _identify_printing_item(printing_card), "tier_2_contested_printing"

artist_card = (
Card.objects.filter(artist_vote_status=ArtistVoteStatus.CONTESTED, pk__in=get_contested_artist_card_ids())
Card.objects.filter(artist_vote_status=ArtistVoteStatus.CONTESTED, pk__in=contested_artist_card_ids)
.exclude(pk__in=answered_artist_card_ids)
.exclude(pk__in=not_official_art_card_ids)
.order_by("-date_created")
Expand Down Expand Up @@ -537,6 +543,7 @@ def _tier_4_fresh(
anonymous_id: str,
answered_card_ids: Optional[set[int]] = None,
not_official_art_card_ids: Optional[set[int]] = None,
contested_card_ids: Optional[list[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
Expand All @@ -563,9 +570,11 @@ def _tier_4_fresh(
answered_card_ids = _voter_answered_printing_card_ids(anonymous_id)
if not_official_art_card_ids is None:
not_official_art_card_ids = _not_official_art_card_ids()
if contested_card_ids is None:
contested_card_ids = get_contested_card_ids()
printing_card = (
Card.objects.filter(printing_tag_status=PrintingTagStatus.UNRESOLVED)
.exclude(pk__in=get_contested_card_ids())
.exclude(pk__in=contested_card_ids)
.exclude(pk__in=answered_card_ids)
.annotate(vote_count=Count("printing_tags", distinct=True))
.annotate(origin_reason=_latest_stage_d_origin_reason_subquery())
Expand Down Expand Up @@ -679,18 +688,33 @@ 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
# 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()
contested_artist_card_ids = get_contested_artist_card_ids()

tier_2_result = _tier_2_contested(
anonymous_id,
answered_card_ids,
answered_artist_card_ids=answered_artist_card_ids,
answered_tag_card_ids_by_tag=answered_tag_card_ids_by_tag,
not_official_art_card_ids=not_official_art_card_ids,
contested_card_ids=contested_card_ids,
contested_artist_card_ids=contested_artist_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, answered_card_ids, not_official_art_card_ids=not_official_art_card_ids)
tier_4_result = _tier_4_fresh(
anonymous_id,
answered_card_ids,
not_official_art_card_ids=not_official_art_card_ids,
contested_card_ids=contested_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)
Expand Down
49 changes: 47 additions & 2 deletions MPCAutofill/cardpicker/tests/test_question_feed.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
from unittest.mock import patch

from django.urls import reverse

from cardpicker import views
from cardpicker.artist_consensus import resolve_and_persist_artist
from cardpicker.artist_consensus import (
get_contested_artist_card_ids,
resolve_and_persist_artist,
)
from cardpicker.local_calculate_verdicts import (
JOIN_KEY_ANONYMOUS_ID,
JOIN_KEY_UNKNOWN_SET_CODE_SKIP_REASON,
Expand All @@ -17,7 +22,10 @@
VotePolarity,
VoteSource,
)
from cardpicker.printing_consensus import resolve_and_persist_printing
from cardpicker.printing_consensus import (
get_contested_card_ids,
resolve_and_persist_printing,
)
from cardpicker.question_feed import (
_artist_item,
_scryfall_illustration_url,
Expand Down Expand Up @@ -213,6 +221,43 @@ def test_own_vote_exclusion_is_scoped_to_the_specific_tag_not_the_whole_card(sel
assert item.tagName == tag_b.name


class TestContestedIdsMemoizedPerRequest:
"""`get_contested_card_ids`/`get_contested_artist_card_ids` are expensive (issue #726:
330-400ms each on production data) and, before this fix, were recomputed once per tier that
consulted them instead of once per `get_next_question_feed_item` call."""

def test_get_contested_card_ids_computed_once_when_tier_2_and_tier_4_are_both_consulted(self, db):
# a plain unresolved card with no votes: tier 2 finds nothing contested and falls
# through to tier 4, which - before this fix - called get_contested_card_ids() a
# second time for the same answer within the same request
card = CardFactory(printing_tag_status=PrintingTagStatus.UNRESOLVED)

with (
patch("cardpicker.question_feed.get_contested_card_ids", wraps=get_contested_card_ids) as mock_contested,
patch(
"cardpicker.question_feed.get_contested_artist_card_ids", wraps=get_contested_artist_card_ids
) as mock_contested_artist,
):
item = get_next_question_feed_item("anon-1")

assert item is not None
assert item.card.identifier == card.identifier
assert mock_contested.call_count == 1
assert mock_contested_artist.call_count == 1

def test_get_contested_card_ids_not_called_when_tier_1_serves_the_item(self, db):
# tier 1 (confirm_suggestion) and the likely-resolve pool both resolve before tier 2 is
# ever reached, so neither contested-ids function should run at all
make_ai_suggested_card()

with patch("cardpicker.question_feed.get_contested_card_ids", wraps=get_contested_card_ids) as mock_contested:
item = get_next_question_feed_item("anon-1")

assert item is not None
assert item.type.value == "confirm_suggestion"
assert mock_contested.call_count == 0


class TestPhaseCNotOfficialArtRouting:
"""
2026-08-04 gate on the phase-C/md5 routing brief (item 2): a card carrying a positive,
Expand Down
Loading