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: 32 additions & 0 deletions MPCAutofill/cardpicker/migrations/0108_hiddencard.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Generated by Django 4.2.30 on 2026-08-07 00:50

import django.db.models.deletion
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("cardpicker", "0107_question_feed_pools_schedule_dedupe"),
]

operations = [
migrations.CreateModel(
name="HiddenCard",
fields=[
("id", models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")),
("anonymous_id", models.CharField(max_length=40)),
("created_at", models.DateTimeField(auto_now_add=True)),
(
"card",
models.ForeignKey(
on_delete=django.db.models.deletion.CASCADE, related_name="hidden_by", to="cardpicker.card"
),
),
],
),
migrations.AddConstraint(
model_name="hiddencard",
constraint=models.UniqueConstraint(fields=("card", "anonymous_id"), name="hiddencard_unique_hide"),
),
]
25 changes: 25 additions & 0 deletions MPCAutofill/cardpicker/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1652,6 +1652,31 @@ def __str__(self) -> str:
return f"{self.card.name} -> {self.reason} ({self.anonymous_id})"


class HiddenCard(models.Model):
"""
A durable per-anonymous_id record that `card` should be excluded from that identity's own
future question-feed items (see docs/features/moderation.md's hidden-card section). Written
by `views.post_report_card` when the report carries `hide=True` (`ReportCardRequest.hide`,
additive to the existing report payload) - always alongside a `CardReport` row, in the same
transaction, never in place of one. Scoped to the client-generated anonymous_id only, same
as every other vote/report table here - no account linkage yet (see that doc section for
what this deliberately does not do). `get_or_create`d at the write site, so a repeat report
with `hide=True` for the same (card, anonymous_id) is a no-op rather than an IntegrityError.
"""

card = models.ForeignKey(to=Card, on_delete=models.CASCADE, related_name="hidden_by")
anonymous_id = models.CharField(max_length=40)
created_at = models.DateTimeField(auto_now_add=True)

class Meta:
constraints = [
models.UniqueConstraint(fields=["card", "anonymous_id"], name="hiddencard_unique_hide"),
]

def __str__(self) -> str:
return f"{self.card.name} hidden for {self.anonymous_id}"


class TagSuggestionStatus(models.TextChoices):
PENDING = "pending", "Pending"
AUTO_ACCEPTED = "auto_accepted", "Auto-accepted"
Expand Down
119 changes: 105 additions & 14 deletions MPCAutofill/cardpicker/question_feed.py

Large diffs are not rendered by default.

30 changes: 26 additions & 4 deletions MPCAutofill/cardpicker/question_feed_pools.py
Original file line number Diff line number Diff line change
Expand Up @@ -362,26 +362,34 @@ def _fetch_unresolved_printing_card(card_id: int) -> Optional[Card]:
return Card.objects.filter(pk=card_id, printing_tag_status=PrintingTagStatus.UNRESOLVED).first()


def draw_resolution_imminent_card(answered_card_ids: set[int]) -> Optional[Card]:
def draw_resolution_imminent_card(
answered_card_ids: set[int], hidden_card_ids: Optional[set[int]] = None
) -> Optional[Card]:
entries = _get_cached_pool(LANE_RESOLUTION_IMMINENT)
if not entries:
return None
hidden_card_ids = hidden_card_ids or set()
for entry in _iter_from_random_offset(entries):
if entry.card_id in answered_card_ids:
continue
if entry.card_id in hidden_card_ids:
continue
card = _fetch_unresolved_printing_card(entry.card_id)
if card is not None:
return card
return None


def draw_confirm_card(answered_card_ids: set[int]) -> Optional[Card]:
def draw_confirm_card(answered_card_ids: set[int], hidden_card_ids: Optional[set[int]] = None) -> Optional[Card]:
entries = _get_cached_pool(LANE_CONFIRM)
if not entries:
return None
hidden_card_ids = hidden_card_ids or set()
for entry in _iter_from_random_offset(entries):
if entry.card_id in answered_card_ids:
continue
if entry.card_id in hidden_card_ids:
continue
card = _fetch_unresolved_printing_card(entry.card_id)
if card is not None:
return card
Expand All @@ -393,16 +401,24 @@ def draw_contested_entry(
answered_artist_card_ids: set[int],
answered_tag_card_ids_by_tag: dict[str, set[int]],
not_official_art_card_ids: set[int],
hidden_card_ids: Optional[set[int]] = None,
) -> Optional[tuple[str, Card, Optional[str], Optional[str]]]:
"""Returns `(kind, card, tag_name, reason)` for the first unexcluded, unstale entry found
from a random offset, or `None`. Exclusion sets match `_tier_2_contested`'s own exactly - all
three (`answered_card_ids`/`answered_artist_card_ids`/`answered_tag_card_ids_by_tag`) are
already the md5-widened, per-request-memoised sets `get_next_question_feed_item` computes
once and threads through, same as the live tier."""
once and threads through, same as the live tier. `hidden_card_ids` (this voter's
`question_feed._voter_hidden_card_ids` set, `None` for a direct caller meaning no hidden
exclusion) applies to all three kinds at once - the live tier excludes a hidden card from
its printing AND artist querysets and its tag loop, and this reproduces that card-level
exclusion here rather than per-kind."""
entries = _get_cached_pool(LANE_CONTESTED)
if not entries:
return None
hidden_card_ids = hidden_card_ids or set()
for entry in _iter_from_random_offset(entries):
if entry.card_id in hidden_card_ids:
continue
if entry.kind == KIND_PRINTING:
if entry.card_id in answered_card_ids:
continue
Expand Down Expand Up @@ -432,6 +448,7 @@ def draw_cold_entry(
answered_card_ids: set[int],
not_official_art_card_ids: set[int],
contested_card_ids: list[int],
hidden_card_ids: Optional[set[int]] = None,
) -> Optional[tuple[str, Card, Optional[str], Optional[str]]]:
"""The cold-lane analogue of `draw_contested_entry`. `contested_card_ids` is re-checked
in-memory (a card can have gone from fresh to contested since this pool's last warm) -
Expand All @@ -441,12 +458,17 @@ def draw_cold_entry(
itself keeps its own pre-existing unwidened form for both (see that function's own docstring
for why: the widened convention is scoped to `_tier_2_contested` only), so reproducing it
here means one extra indexed query per artist/tag candidate scanned rather than a second,
possibly-diverging exclusion rule."""
possibly-diverging exclusion rule. `hidden_card_ids` (`question_feed._voter_hidden_card_ids`,
`None` for a direct caller meaning no hidden exclusion) is applied card-level, same as the
contested lane above."""
entries = _get_cached_pool(LANE_COLD)
if not entries:
return None
hidden_card_ids = hidden_card_ids or set()
contested_card_id_set = set(contested_card_ids)
for entry in _iter_from_random_offset(entries):
if entry.card_id in hidden_card_ids:
continue
if entry.kind == KIND_PRINTING:
if entry.card_id in answered_card_ids or entry.card_id in contested_card_id_set:
continue
Expand Down
6 changes: 5 additions & 1 deletion MPCAutofill/cardpicker/schema_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -2143,6 +2143,7 @@ class ReportCardRequest(BaseModel):
anonymousId: str
identifier: str
reason: Reason
hide: Optional[bool] = None
text: Optional[str] = None

@staticmethod
Expand All @@ -2151,14 +2152,17 @@ def from_dict(obj: Any) -> "ReportCardRequest":
anonymousId = from_str(obj.get("anonymousId"))
identifier = from_str(obj.get("identifier"))
reason = Reason(obj.get("reason"))
hide = from_union([from_bool, from_none], obj.get("hide"))
text = from_union([from_str, from_none], obj.get("text"))
return ReportCardRequest(anonymousId, identifier, reason, text)
return ReportCardRequest(anonymousId, identifier, reason, hide, text)

def to_dict(self) -> dict:
result: dict = {}
result["anonymousId"] = from_str(self.anonymousId)
result["identifier"] = from_str(self.identifier)
result["reason"] = to_enum(Reason, self.reason)
if self.hide is not None:
result["hide"] = from_union([from_bool, from_none], self.hide)
if self.text is not None:
result["text"] = from_union([from_str, from_none], self.text)
return result
Expand Down
8 changes: 8 additions & 0 deletions MPCAutofill/cardpicker/tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,14 @@ class Meta:
text = ""


class HiddenCardFactory(factory.django.DjangoModelFactory):
class Meta:
model = models.HiddenCard

card = factory.SubFactory(CardFactory)
anonymous_id = factory.Sequence(lambda n: f"anonymous_{n}")


class SavedDeckFactory(factory.django.DjangoModelFactory):
class Meta:
model = models.SavedDeck
Expand Down
56 changes: 55 additions & 1 deletion MPCAutofill/cardpicker/tests/test_moderation_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
CardReportReason,
CardTagVote,
CardTypes,
HiddenCard,
Source,
TagModerationClass,
TagVoteStatus,
Expand Down Expand Up @@ -151,10 +152,19 @@ def autouse_django_settings(self, django_settings):
pass

@staticmethod
def report(client, card, reason: str, text: str | None = None, anonymous_id: str = "anon-1"):
def report(
client,
card,
reason: str,
text: str | None = None,
anonymous_id: str = "anon-1",
hide: bool | None = None,
):
body: dict = {"identifier": card.identifier, "anonymousId": anonymous_id, "reason": reason}
if text is not None:
body["text"] = text
if hide is not None:
body["hide"] = hide
return client.post(reverse(views.post_report_card), body, content_type="application/json")

def test_report_writes_audit_row(self, client):
Expand Down Expand Up @@ -245,6 +255,50 @@ def test_rate_limit_is_per_anonymous_id(self, client, settings):
assert self.report(client, card, "broken_image", anonymous_id="anon-a").status_code == 200
assert self.report(client, card, "broken_image", anonymous_id="anon-b").status_code == 200

def test_hide_true_writes_hidden_card_in_the_same_transaction(self, client):
card = CardFactory()
response = self.report(client, card, "broken_image", hide=True)
assert response.status_code == 200
# the report always lands, and the hide adds the exclusion alongside it - never in
# place of the report (the `HiddenCard` docstring's contract)
report = CardReport.objects.get()
assert report.card == card
hidden = HiddenCard.objects.get()
assert hidden.card == card
assert hidden.anonymous_id == "anon-1"

def test_hide_absent_or_false_never_writes_a_hidden_card(self, client):
card = CardFactory()
self.report(client, card, "broken_image")
self.report(client, card, "broken_image", anonymous_id="anon-2", hide=False)
assert CardReport.objects.count() == 2
assert HiddenCard.objects.count() == 0

def test_repeat_hide_is_a_no_op_via_get_or_create(self, client):
card = CardFactory()
self.report(client, card, "broken_image", hide=True)
self.report(client, card, "broken_image", hide=True)
assert CardReport.objects.count() == 2
assert HiddenCard.objects.count() == 1

def test_hide_is_scoped_per_anonymous_id(self, client):
card = CardFactory()
self.report(client, card, "broken_image", anonymous_id="anon-a", hide=True)
# a second identity reporting the same card without hide must not inherit the exclusion
self.report(client, card, "broken_image", anonymous_id="anon-b")
hidden = HiddenCard.objects.get()
assert hidden.anonymous_id == "anon-a"

def test_rate_limited_report_with_hide_writes_nothing(self, client, settings):
settings.CARD_REPORT_RATE = "1/d"
card = CardFactory()
assert self.report(client, card, "broken_image", anonymous_id="anon-rate").status_code == 200
assert self.report(client, card, "nsfw", anonymous_id="anon-rate", hide=True).status_code == 429
# the 429 path returns before the transaction, so neither row exists for the hidden
# report - a hide can never outlive the report it travels with
assert CardReport.objects.count() == 1
assert HiddenCard.objects.count() == 0


class TestRejectUntrustedOrigin:
@pytest.fixture(autouse=True)
Expand Down
36 changes: 36 additions & 0 deletions MPCAutofill/cardpicker/tests/test_question_feed.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
ArtistVoteStatus,
CardPrintingTag,
CardScanLog,
HiddenCard,
PrintingTagStatus,
QuestionFeedServedLog,
QuestionFeedServedPool,
Expand Down Expand Up @@ -245,6 +246,41 @@ def test_own_vote_exclusion_is_scoped_to_the_specific_tag_not_the_whole_card(sel
assert item.card.identifier == card.identifier
assert item.tagName == tag_b.name

def test_a_hidden_card_is_excluded_from_this_voters_feed(self, db):
"""Issue #714: a card this voter hid for themselves (`HiddenCard`, written by
`views.post_report_card` when a report carries `hide=True`) must never come back in
their own feed items, whichever tier would otherwise have served it."""
card = CardFactory(printing_tag_status=PrintingTagStatus.UNRESOLVED)
HiddenCard.objects.create(card=card, anonymous_id="anon-1")

item = get_next_question_feed_item("anon-1")

# the only candidate is hidden for this voter - nothing else exists, so None
assert item is None or item.card.identifier != card.identifier

def test_a_hidden_card_is_still_served_to_other_voters(self, db):
"""The exclusion is per-anonymous_id, not global: hiding a card for yourself never
hides it for anyone else - same scoping as every other vote/report table here."""
card = CardFactory(printing_tag_status=PrintingTagStatus.UNRESOLVED)
HiddenCard.objects.create(card=card, anonymous_id="anon-1")

item = get_next_question_feed_item("anon-2")

assert item is not None
assert item.card.identifier == card.identifier

def test_a_hidden_card_is_excluded_even_when_it_is_the_only_contested_candidate(self, db):
"""Tier 2's contested printing half must respect the hidden exclusion too - a card the
voter hid must not resurface just because it became the highest-priority contested one."""
hidden_card = CardFactory(printing_tag_status=PrintingTagStatus.UNRESOLVED)
CardPrintingTagFactory(card=hidden_card, printing=CanonicalCardFactory(), source=VoteSource.USER)
CardPrintingTagFactory(card=hidden_card, printing=CanonicalCardFactory(), source=VoteSource.USER)
HiddenCard.objects.create(card=hidden_card, anonymous_id="anon-1")

item = get_next_question_feed_item("anon-1")

assert item is None or item.card.identifier != hidden_card.identifier


class TestContestedIdsMemoizedPerRequest:
"""`get_contested_card_ids`/`get_contested_artist_card_ids` are expensive (issue #726:
Expand Down
39 changes: 39 additions & 0 deletions MPCAutofill/cardpicker/tests/test_question_feed_pools.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,16 @@ def test_a_resolved_card_is_excluded_at_read_time_even_though_still_pooled(self,

assert draw_resolution_imminent_card(answered_card_ids=set()) is None

def test_excludes_a_card_this_voter_hid_for_themselves(self, db):
"""Issue #714: the draw-time analogue of `_tier_4_fresh`/tier 1's hidden-card
exclusion - a hidden card is skipped even though it is still pooled."""
card, _ = make_one_vote_from_resolving_card()
warm_pool_cache(LANE_RESOLUTION_IMMINENT)

assert draw_resolution_imminent_card(answered_card_ids=set(), hidden_card_ids={card.pk}) is None
# no hidden exclusion = still served, for this voter or any other
assert draw_resolution_imminent_card(answered_card_ids=set()) is not None


class TestDrawConfirmCard:
def test_returns_the_pooled_card(self, db):
Expand All @@ -270,6 +280,12 @@ def test_excludes_a_card_this_voter_already_answered(self, db):
warm_pool_cache(LANE_CONFIRM)
assert draw_confirm_card(answered_card_ids={card.pk}) is None

def test_excludes_a_card_this_voter_hid_for_themselves(self, db):
card, _ = make_ai_suggested_card()
warm_pool_cache(LANE_CONFIRM)

assert draw_confirm_card(answered_card_ids=set(), hidden_card_ids={card.pk}) is None


class TestDrawContestedEntry:
def test_returns_a_printing_entry(self, db):
Expand Down Expand Up @@ -312,6 +328,17 @@ def test_a_tag_entry_respects_the_per_tag_exclusion_dict(self, db):
assert drawn[0] == KIND_TAG
assert drawn[2] == tag.name

def test_a_hidden_card_is_excluded_from_the_printing_half(self, db):
"""Issue #714: the card-level hidden exclusion applies across every contested kind -
the printing half is the common case, the artist/tag halves share the same entry-level
skip (asserted via the cold lane's equivalent test below)."""
card = CardFactory(printing_tag_status=PrintingTagStatus.UNRESOLVED)
CardPrintingTagFactory(card=card, printing=CanonicalCardFactory(), source=VoteSource.USER)
CardPrintingTagFactory(card=card, printing=CanonicalCardFactory(), source=VoteSource.USER)
warm_pool_cache(LANE_CONTESTED)

assert draw_contested_entry(set(), set(), {}, set(), hidden_card_ids={card.pk}) is None

def test_returns_none_on_a_cache_miss(self, db):
assert draw_contested_entry(set(), set(), {}, set()) is None

Expand Down Expand Up @@ -358,6 +385,18 @@ def test_artist_half_uses_an_unwidened_per_voter_check(self, db):
assert drawn is not None
assert drawn[1].pk == card.pk

def test_a_hidden_card_is_excluded(self, db):
"""Issue #714: the cold lane's card-level hidden exclusion, keyed on the same
entry-level skip as the contested lane."""
card = CardFactory(
printing_tag_status=PrintingTagStatus.UNRESOLVED, artist_vote_status=ArtistVoteStatus.RESOLVED
)
warm_pool_cache(LANE_COLD)

assert draw_cold_entry("anon-1", set(), set(), contested_card_ids=[], hidden_card_ids={card.pk}) is None
# no hidden exclusion = still served
assert draw_cold_entry("anon-1", set(), set(), contested_card_ids=[]) is not None

def test_returns_none_on_a_cache_miss(self, db):
assert draw_cold_entry("anon-1", set(), set(), contested_card_ids=[]) is None

Expand Down
Loading
Loading