From a330ee53ba421e536109de6eb4f415912322eb34 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Sun, 12 Jul 2026 12:58:59 +0000 Subject: [PATCH 1/6] Fix CI: scope "Who's That Planeswalker?" assertions to the modal The rebrand made the nav link and the modal's section heading share the exact same text, so an unscoped page.getByText() now matches both (a Playwright strict-mode violation - CI caught 2-4 matching elements). Scoped both assertions to the detailed-view modal specifically. Co-Authored-By: Claude Sonnet 5 --- frontend/tests/PrintingTagPicker.spec.ts | 4 +++- frontend/tests/PrintingTagQueue.spec.ts | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/frontend/tests/PrintingTagPicker.spec.ts b/frontend/tests/PrintingTagPicker.spec.ts index 5175a2ec1..b166955b3 100644 --- a/frontend/tests/PrintingTagPicker.spec.ts +++ b/frontend/tests/PrintingTagPicker.spec.ts @@ -47,7 +47,9 @@ test.describe("PrintingTagPicker tests", () => { ); await openDetailedView(page, cardDocument1.name); - await expect(page.getByText("Who's That Planeswalker?")).toBeVisible(); + await expect( + page.getByTestId("detailed-view").getByText("Who's That Planeswalker?") + ).toBeVisible(); await expect(page.getByText("Not yet resolved")).toBeVisible(); const picker = page.getByTestId("printing-tag-picker"); diff --git a/frontend/tests/PrintingTagQueue.spec.ts b/frontend/tests/PrintingTagQueue.spec.ts index 2fbb73c12..7ff13c637 100644 --- a/frontend/tests/PrintingTagQueue.spec.ts +++ b/frontend/tests/PrintingTagQueue.spec.ts @@ -54,6 +54,8 @@ test.describe("PrintingTagQueue tests", () => { await page.getByAltText(cardDocument1.name).click(); await expect(page.getByText("Card Details")).toBeVisible(); - await expect(page.getByText("Who's That Planeswalker?")).toBeVisible(); + await expect( + page.getByTestId("detailed-view").getByText("Who's That Planeswalker?") + ).toBeVisible(); }); }); From ed95609c100019fd84080d602101bd5e13babc8b Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Sun, 12 Jul 2026 19:13:35 +0000 Subject: [PATCH 2/6] Add artist + tag weighted voting following a printing "no match" Generalizes the existing printing-tag weighted-consensus mechanism (CardPrintingTag, PRINTING_TAG_* settings) into a shared core (vote_consensus.resolve_weighted_consensus) used by two new sibling systems: CardArtistVote (who illustrated a card) and CardTagVote (descriptor tags, multi-valued per card via update_or_create rather than delete-then-create). Also fixes a durability gap found while designing this: a scheduled re-scan could silently revert a resolved tag-vote correction, since bulk_sync_objects wrote Card.tags purely from fresh filename extraction - now merges resolved consensus in first. New endpoints (submitArtistVote/artistConsensus/artistCandidates, submitTagVote/tagConsensus) and a frontend attributeVoting panel wired into both the printing-tag queue and the card detail modal, shown once a card's printing consensus is unresolved. Co-Authored-By: Claude Sonnet 5 --- MPCAutofill/cardpicker/admin.py | 26 ++ MPCAutofill/cardpicker/artist_consensus.py | 111 ++++++ .../0053_card_artist_vote_status_and_more.py | 126 ++++++ MPCAutofill/cardpicker/models.py | 142 ++++++- MPCAutofill/cardpicker/printing_consensus.py | 80 ++-- MPCAutofill/cardpicker/schema_types.py | 362 ++++++++++++++++-- .../cardpicker/sources/update_database.py | 24 +- MPCAutofill/cardpicker/tag_consensus.py | 138 +++++++ MPCAutofill/cardpicker/tests/factories.py | 26 +- .../cardpicker/tests/test_artist_votes.py | 348 +++++++++++++++++ .../tests/test_printing_consensus.py | 36 +- .../tests/test_printing_tags_views.py | 6 +- MPCAutofill/cardpicker/tests/test_sources.py | 46 ++- .../cardpicker/tests/test_tag_votes.py | 277 ++++++++++++++ .../cardpicker/tests/test_vote_consensus.py | 56 +++ MPCAutofill/cardpicker/urls.py | 5 + MPCAutofill/cardpicker/views.py | 238 +++++++++++- MPCAutofill/cardpicker/vote_consensus.py | 62 +++ frontend/src/common/schema_types.ts | 281 +++++++++++++- frontend/src/common/test-constants.ts | 4 + .../attributeVoting/ArtistVotePicker.tsx | 164 ++++++++ .../attributeVoting/AttributeVotingPanel.tsx | 34 ++ .../attributeVoting/TagVotePicker.tsx | 112 ++++++ .../CardDetailedViewModal.tsx | 27 +- .../printingTags/PrintingTagPicker.tsx | 18 +- .../printingTags/PrintingTagQueue.tsx | 34 +- frontend/src/mocks/handlers.ts | 64 ++++ frontend/src/store/api.ts | 109 ++++++ frontend/tests/ArtistVotePicker.spec.ts | 106 +++++ frontend/tests/TagVotePicker.spec.ts | 89 +++++ schemas/schemas/ArtistVoteTallyEntry.json | 12 + schemas/schemas/TagConsensusEntry.json | 15 + schemas/schemas/TagVoteTallyEntry.json | 11 + .../endpoints/ArtistCandidatesRequest.json | 11 + .../endpoints/ArtistCandidatesResponse.json | 13 + .../endpoints/ArtistConsensusRequest.json | 10 + .../endpoints/ArtistConsensusResponse.json | 15 + .../endpoints/SubmitArtistVoteRequest.json | 13 + .../endpoints/SubmitTagVoteRequest.json | 13 + .../endpoints/TagConsensusRequest.json | 10 + .../endpoints/TagConsensusResponse.json | 13 + 41 files changed, 3160 insertions(+), 127 deletions(-) create mode 100644 MPCAutofill/cardpicker/artist_consensus.py create mode 100644 MPCAutofill/cardpicker/migrations/0053_card_artist_vote_status_and_more.py create mode 100644 MPCAutofill/cardpicker/tag_consensus.py create mode 100644 MPCAutofill/cardpicker/tests/test_artist_votes.py create mode 100644 MPCAutofill/cardpicker/tests/test_tag_votes.py create mode 100644 MPCAutofill/cardpicker/tests/test_vote_consensus.py create mode 100644 MPCAutofill/cardpicker/vote_consensus.py create mode 100644 frontend/src/features/attributeVoting/ArtistVotePicker.tsx create mode 100644 frontend/src/features/attributeVoting/AttributeVotingPanel.tsx create mode 100644 frontend/src/features/attributeVoting/TagVotePicker.tsx create mode 100644 frontend/tests/ArtistVotePicker.spec.ts create mode 100644 frontend/tests/TagVotePicker.spec.ts create mode 100644 schemas/schemas/ArtistVoteTallyEntry.json create mode 100644 schemas/schemas/TagConsensusEntry.json create mode 100644 schemas/schemas/TagVoteTallyEntry.json create mode 100644 schemas/schemas/endpoints/ArtistCandidatesRequest.json create mode 100644 schemas/schemas/endpoints/ArtistCandidatesResponse.json create mode 100644 schemas/schemas/endpoints/ArtistConsensusRequest.json create mode 100644 schemas/schemas/endpoints/ArtistConsensusResponse.json create mode 100644 schemas/schemas/endpoints/SubmitArtistVoteRequest.json create mode 100644 schemas/schemas/endpoints/SubmitTagVoteRequest.json create mode 100644 schemas/schemas/endpoints/TagConsensusRequest.json create mode 100644 schemas/schemas/endpoints/TagConsensusResponse.json diff --git a/MPCAutofill/cardpicker/admin.py b/MPCAutofill/cardpicker/admin.py index a75cea631..13b9e5b9f 100644 --- a/MPCAutofill/cardpicker/admin.py +++ b/MPCAutofill/cardpicker/admin.py @@ -8,7 +8,9 @@ CanonicalExpansion, CanonicalPrintingMetadata, Card, + CardArtistVote, CardPrintingTag, + CardTagVote, DFCPair, Project, ProjectMember, @@ -131,6 +133,30 @@ class AdminCardPrintingTag(admin.ModelAdmin[CardPrintingTag]): raw_id_fields = ["card", "printing"] +@admin.register(CardArtistVote) +class AdminCardArtistVote(admin.ModelAdmin[CardArtistVote]): + """ + Plain registration, no `ContestedCardFilter`-equivalent yet - generalizing the + contested-first review queue to artist/tag votes is a deferred follow-up, not part of + this stage (see `cardpicker.artist_consensus`/`cardpicker.tag_consensus`). + """ + + list_display = ("card", "artist", "is_unknown", "source", "confidence", "anonymous_id", "created_at") + list_filter = ("source", "is_unknown") + search_fields = ("card__name",) + raw_id_fields = ["card", "artist"] + + +@admin.register(CardTagVote) +class AdminCardTagVote(admin.ModelAdmin[CardTagVote]): + """Plain registration - see `AdminCardArtistVote`'s docstring for the deferred-filter note.""" + + list_display = ("card", "tag", "polarity", "source", "confidence", "anonymous_id", "created_at") + list_filter = ("source", "polarity") + search_fields = ("card__name", "tag__name") + raw_id_fields = ["card", "tag"] + + @admin.register(TagAliasSuggestion) class AdminTagAliasSuggestion(admin.ModelAdmin[TagAliasSuggestion]): list_display = ("raw_text", "suggested_tag", "confidence", "occurrence_count", "status") diff --git a/MPCAutofill/cardpicker/artist_consensus.py b/MPCAutofill/cardpicker/artist_consensus.py new file mode 100644 index 000000000..3da4ed114 --- /dev/null +++ b/MPCAutofill/cardpicker/artist_consensus.py @@ -0,0 +1,111 @@ +from typing import Literal, TypedDict + +from django.conf import settings + +from cardpicker.models import ArtistVoteStatus, CanonicalArtist, Card, VoteSource +from cardpicker.vote_consensus import VoteTuple, resolve_weighted_consensus + +UNKNOWN: Literal["UNKNOWN"] = "UNKNOWN" + +# reuses the printing-tag weight/threshold settings rather than introducing a parallel set - +# see the plan note in the models: per-vote-type overrides are YAGNI until a real need appears. +_SOURCE_WEIGHTS: dict[str, float] = { + VoteSource.USER: 1.0, + VoteSource.ADMIN: settings.PRINTING_TAG_ADMIN_WEIGHT, + VoteSource.AI: settings.PRINTING_TAG_AI_WEIGHT, +} + + +def resolve_artist(card: Card) -> CanonicalArtist | Literal["UNKNOWN"] | None: + """ + Reconciles all `CardArtistVote` votes cast against `card` into a single resolved outcome: + a specific `CanonicalArtist`, the `UNKNOWN` sentinel (consensus is that the artist is + unlisted/unidentifiable), or `None` if there isn't yet enough signal. Mirrors + `cardpicker.printing_consensus.resolve_printing` exactly, built on the same shared + `resolve_weighted_consensus` core. + + Note this outcome is only ever surfaced to a viewer when the card's printing-tag + consensus *hasn't* resolved a printing - see the artist fallback chain in + `Card.serialise()`, where a resolved printing's own artist always takes precedence. + """ + votes = list(card.artist_votes.all()) + if not votes: + return None + + artists_by_id: dict[int, CanonicalArtist] = {} + vote_tuples: list[VoteTuple] = [] + for vote in votes: + key: int | Literal["UNKNOWN"] + if vote.is_unknown: + key = UNKNOWN + else: + # guaranteed non-null here by the model's artist_xor_unknown CheckConstraint + assert vote.artist_id is not None + assert vote.artist is not None + key = vote.artist_id + artists_by_id[vote.artist_id] = vote.artist + vote_tuples.append( + VoteTuple(outcome_key=key, weight=_SOURCE_WEIGHTS[vote.source], is_ai=vote.source == VoteSource.AI) + ) + + winning_key = resolve_weighted_consensus( + vote_tuples, min_weight=settings.PRINTING_TAG_MIN_VOTES, min_share=settings.PRINTING_TAG_MIN_SHARE + ) + if winning_key is None: + return None + if winning_key == UNKNOWN: + return UNKNOWN + assert isinstance(winning_key, int) + return artists_by_id[winning_key] + + +def resolve_and_persist_artist(card: Card) -> CanonicalArtist | Literal["UNKNOWN"] | None: + """ + Runs `resolve_artist(card)` and writes the outcome onto `card.inferred_canonical_artist` + and `card.artist_vote_status` together - same pattern as + `cardpicker.printing_consensus.resolve_and_persist_printing`. Deliberately doesn't consult + `card.printing_tag_status` at all: the precedence rule ("a resolved printing's artist wins") + is enforced entirely by `Card.serialise()`'s fallback chain, not here, so this function + stays decoupled from printing-tag state. + """ + result = resolve_artist(card) + if result is None: + card.inferred_canonical_artist = None + card.artist_vote_status = ArtistVoteStatus.UNRESOLVED + elif result == UNKNOWN: + card.inferred_canonical_artist = None + card.artist_vote_status = ArtistVoteStatus.UNKNOWN + else: + card.inferred_canonical_artist = result + card.artist_vote_status = ArtistVoteStatus.RESOLVED + card.save(update_fields=["inferred_canonical_artist", "artist_vote_status"]) + return result + + +class ArtistVoteTallyEntry(TypedDict): + artist: CanonicalArtist | None + is_unknown: bool + count: int + + +def get_artist_vote_tally(card: Card) -> list[ArtistVoteTallyEntry]: + """ + Plain, unweighted per-outcome vote count for `card` - mirrors + `cardpicker.printing_consensus.get_vote_tally`, for showing a voter what's already been + said before they confirm or dispute it. + """ + tally: dict[int | Literal["UNKNOWN"], ArtistVoteTallyEntry] = {} + for vote in card.artist_votes.all(): + key: int | Literal["UNKNOWN"] + if vote.is_unknown: + key = UNKNOWN + else: + assert vote.artist_id is not None + key = vote.artist_id + if key not in tally: + tally[key] = ArtistVoteTallyEntry(artist=vote.artist, is_unknown=vote.is_unknown, count=0) + tally[key]["count"] += 1 + return sorted(tally.values(), key=lambda entry: entry["count"], reverse=True) + + +__all__ = ["UNKNOWN", "resolve_artist", "resolve_and_persist_artist", "get_artist_vote_tally", "ArtistVoteTallyEntry"] diff --git a/MPCAutofill/cardpicker/migrations/0053_card_artist_vote_status_and_more.py b/MPCAutofill/cardpicker/migrations/0053_card_artist_vote_status_and_more.py new file mode 100644 index 000000000..2a7402618 --- /dev/null +++ b/MPCAutofill/cardpicker/migrations/0053_card_artist_vote_status_and_more.py @@ -0,0 +1,126 @@ +# Generated by Django 4.2.30 on 2026-07-12 18:10 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("cardpicker", "0052_tagaliassuggestion_card_expansion_hint"), + ] + + operations = [ + migrations.AddField( + model_name="card", + name="artist_vote_status", + field=models.CharField( + choices=[("unresolved", "Unresolved"), ("resolved", "Resolved"), ("unknown", "Unknown")], + db_index=True, + default="unresolved", + max_length=10, + ), + ), + migrations.AddField( + model_name="card", + name="inferred_canonical_artist", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="+", + to="cardpicker.canonicalartist", + ), + ), + migrations.CreateModel( + name="CardTagVote", + fields=[ + ("id", models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")), + ("anonymous_id", models.CharField(max_length=40)), + ( + "source", + models.CharField( + choices=[("user", "User"), ("admin", "Admin"), ("ai", "AI")], default="user", max_length=10 + ), + ), + ("confidence", models.FloatField(blank=True, null=True)), + ("created_at", models.DateTimeField(auto_now_add=True)), + ("polarity", models.SmallIntegerField(choices=[(1, "Apply"), (-1, "Not applicable")])), + ( + "card", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, related_name="tag_votes", to="cardpicker.card" + ), + ), + ( + "tag", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, related_name="votes", to="cardpicker.tag" + ), + ), + ], + ), + migrations.CreateModel( + name="CardArtistVote", + fields=[ + ("id", models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")), + ("anonymous_id", models.CharField(max_length=40)), + ( + "source", + models.CharField( + choices=[("user", "User"), ("admin", "Admin"), ("ai", "AI")], default="user", max_length=10 + ), + ), + ("confidence", models.FloatField(blank=True, null=True)), + ("created_at", models.DateTimeField(auto_now_add=True)), + ("is_unknown", models.BooleanField(default=False)), + ( + "artist", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="votes", + to="cardpicker.canonicalartist", + ), + ), + ( + "card", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, related_name="artist_votes", to="cardpicker.card" + ), + ), + ], + ), + migrations.AddConstraint( + model_name="cardtagvote", + constraint=models.UniqueConstraint(fields=("card", "tag", "anonymous_id"), name="cardtagvote_unique_vote"), + ), + migrations.AddConstraint( + model_name="cardartistvote", + constraint=models.CheckConstraint( + check=models.Q( + models.Q(("artist__isnull", False), ("is_unknown", False)), + models.Q(("artist__isnull", True), ("is_unknown", True)), + _connector="OR", + ), + name="cardartistvote_artist_xor_unknown", + ), + ), + migrations.AddConstraint( + model_name="cardartistvote", + constraint=models.UniqueConstraint( + condition=models.Q(("is_unknown", False)), + fields=("card", "artist", "anonymous_id"), + name="cardartistvote_unique_artist_vote", + ), + ), + migrations.AddConstraint( + model_name="cardartistvote", + constraint=models.UniqueConstraint( + condition=models.Q(("is_unknown", True)), + fields=("card", "anonymous_id"), + name="cardartistvote_unique_unknown_vote", + ), + ), + ] diff --git a/MPCAutofill/cardpicker/models.py b/MPCAutofill/cardpicker/models.py index 9f2ba3f63..72187384b 100755 --- a/MPCAutofill/cardpicker/models.py +++ b/MPCAutofill/cardpicker/models.py @@ -308,6 +308,18 @@ class PrintingTagStatus(models.TextChoices): NO_MATCH = "no_match", gettext_lazy("No Match") +class ArtistVoteStatus(models.TextChoices): + """ + Denormalised cache of `cardpicker.artist_consensus.resolve_artist`'s outcome for a `Card`, + kept in lockstep with `Card.inferred_canonical_artist` by `resolve_and_persist_artist` - same + purpose as `PrintingTagStatus` above. + """ + + UNRESOLVED = "unresolved", gettext_lazy("Unresolved") + RESOLVED = "resolved", gettext_lazy("Resolved") + UNKNOWN = "unknown", gettext_lazy("Unknown") + + class Card(models.Model): card_type = models.CharField(max_length=20, choices=CardTypes.choices, default=CardTypes.CARD) identifier = models.CharField(max_length=200, unique=True) @@ -334,6 +346,16 @@ class Card(models.Model): printing_tag_status = models.CharField( max_length=10, choices=PrintingTagStatus.choices, default=PrintingTagStatus.UNRESOLVED, db_index=True ) + # artist-vote consensus outcome - only ever surfaced in `serialise()` when neither + # `canonical_card`/`canonical_artist` (confirmed indexing match) nor + # `inferred_canonical_card` (a resolved printing-tag vote, which carries its own artist) + # are set - see the fallback chain in `serialise()` below. + inferred_canonical_artist = models.ForeignKey( + to=CanonicalArtist, on_delete=models.SET_NULL, blank=True, null=True, related_name="+" + ) + artist_vote_status = models.CharField( + max_length=10, choices=ArtistVoteStatus.choices, default=ArtistVoteStatus.UNRESOLVED, db_index=True + ) # a lowercase CanonicalExpansion.code guessed from a lone set-code bracket token in the # source filename (e.g. "[MH3]") - not resolved to a specific printing (no collector # number was present to pair with it), just a ranking hint for get_ranked_printing_candidates @@ -382,7 +404,15 @@ def serialise(self) -> SerialisedCard: canonicalArtist=( self.canonical_artist.serialise() if self.canonical_artist - else (self.canonical_card.artist.serialise() if self.canonical_card else None) + else ( + self.canonical_card.artist.serialise() + if self.canonical_card + else ( + self.inferred_canonical_card.artist.serialise() + if self.inferred_canonical_card + else (self.inferred_canonical_artist.serialise() if self.inferred_canonical_artist else None) + ) + ) ), ) @@ -424,13 +454,40 @@ class Meta: ordering = ["-priority"] -class CardPrintingTagSource(models.TextChoices): +class VoteSource(models.TextChoices): + """ + Shared `source` enum for every `AbstractWeightedVote` subclass (`CardPrintingTag`, + `CardArtistVote`, `CardTagVote`) - not printing-tag-specific despite the historical name + this replaced (`CardPrintingTagSource`). The stored string values are unchanged. + """ + USER = "user", gettext_lazy("User") ADMIN = "admin", gettext_lazy("Admin") AI = "ai", gettext_lazy("AI") -class CardPrintingTag(models.Model): +class AbstractWeightedVote(models.Model): + """ + Shared fields for every weighted-consensus vote model in this app (`CardPrintingTag`, + `CardArtistVote`, `CardTagVote`) - see `cardpicker.vote_consensus.resolve_weighted_consensus` + for how these are reconciled into a single resolved outcome per card. Purely a field + container (no DB table of its own - `abstract = True`), so adding a field here changes + the schema of every subclass's own table simultaneously; a comment here is the only thing + that makes that non-obvious fact visible from any single subclass's own definition. + """ + + # a client-generated identifier (see `frontend/src/common/anonymousId.ts`), not a real Django + # session key - cross-origin frontend/backend means a session cookie never round-trips here. + anonymous_id = models.CharField(max_length=40) + source = models.CharField(max_length=10, choices=VoteSource.choices, default=VoteSource.USER) + confidence = models.FloatField(null=True, blank=True) + created_at = models.DateTimeField(auto_now_add=True) + + class Meta: + abstract = True + + +class CardPrintingTag(AbstractWeightedVote): """ A vote that a given `Card` (an image in this fork's catalogue) depicts a specific Scryfall printing (`CanonicalCard`), or definitively depicts no known printing @@ -441,12 +498,6 @@ class CardPrintingTag(models.Model): card = models.ForeignKey(to=Card, on_delete=models.CASCADE, related_name="printing_tags") printing = models.ForeignKey(to=CanonicalCard, on_delete=models.CASCADE, null=True, blank=True, related_name="tags") is_no_match = models.BooleanField(default=False) - # a client-generated identifier (see `frontend/src/common/anonymousId.ts`), not a real Django - # session key - cross-origin frontend/backend means a session cookie never round-trips here. - anonymous_id = models.CharField(max_length=40) - source = models.CharField(max_length=10, choices=CardPrintingTagSource.choices, default=CardPrintingTagSource.USER) - confidence = models.FloatField(null=True, blank=True) - created_at = models.DateTimeField(auto_now_add=True) class Meta: constraints = [ @@ -474,6 +525,50 @@ def __str__(self) -> str: return f"[{self.source}] {self.card.name} -> {outcome}" +class CardArtistVote(AbstractWeightedVote): + """ + A vote that a given `Card` was illustrated by a specific `CanonicalArtist`, or + definitively by an unknown/unlisted artist (`is_unknown=True`). Only meaningful once a + card's printing-tag consensus hasn't already resolved a printing - see + `cardpicker.artist_consensus` and the artist fallback chain in `Card.serialise()`, where a + resolved printing's own artist always takes precedence over this vote's outcome. + """ + + card = models.ForeignKey(to=Card, on_delete=models.CASCADE, related_name="artist_votes") + artist = models.ForeignKey( + to=CanonicalArtist, on_delete=models.CASCADE, null=True, blank=True, related_name="votes" + ) + is_unknown = models.BooleanField(default=False) + + class Meta: + constraints = [ + models.CheckConstraint( + check=( + models.Q(artist__isnull=False, is_unknown=False) | models.Q(artist__isnull=True, is_unknown=True) + ), + name="cardartistvote_artist_xor_unknown", + ), + # not the sole enforcement of "one active vote per (card, anonymous_id)" - the + # submit view deletes any existing vote for this (card, anonymous_id) before + # creating the new one (same pattern as CardPrintingTag). This constraint is a + # safety net against a double-submit race, not the primary mechanism. + models.UniqueConstraint( + fields=["card", "artist", "anonymous_id"], + condition=models.Q(is_unknown=False), + name="cardartistvote_unique_artist_vote", + ), + models.UniqueConstraint( + fields=["card", "anonymous_id"], + condition=models.Q(is_unknown=True), + name="cardartistvote_unique_unknown_vote", + ), + ] + + def __str__(self) -> str: + outcome = "UNKNOWN" if self.is_unknown else str(self.artist) + return f"[{self.source}] {self.card.name} -> {outcome}" + + class Tag(models.Model): name = models.CharField(unique=True) # null=True is just for admin panel @@ -506,6 +601,35 @@ def get_tags(cls) -> dict[str, list[str]]: return {tag.name: tag.aliases for tag in Tag.objects.all()} +class VotePolarity(models.IntegerChoices): + APPLY = 1, gettext_lazy("Apply") + NOT_APPLICABLE = -1, gettext_lazy("Not applicable") + + +class CardTagVote(AbstractWeightedVote): + """ + A vote on whether a given descriptor `Tag` applies to a `Card` (`polarity=APPLY`) or not + (`polarity=NOT_APPLICABLE`). Unlike `CardPrintingTag`/`CardArtistVote` (mutually exclusive + outcomes - a card has exactly one real printing/artist), a card can carry independent, + simultaneous votes across many different tags at once, so uniqueness here is scoped to + (card, tag, anonymous_id) rather than just (card, anonymous_id) - changing your mind about + one tag is an update to that one row (`update_or_create` in the submit view), not a + delete-and-recreate of every vote this person has cast on this card. + """ + + card = models.ForeignKey(to=Card, on_delete=models.CASCADE, related_name="tag_votes") + tag = models.ForeignKey(to=Tag, on_delete=models.CASCADE, related_name="votes") + polarity = models.SmallIntegerField(choices=VotePolarity.choices) + + class Meta: + constraints = [ + models.UniqueConstraint(fields=["card", "tag", "anonymous_id"], name="cardtagvote_unique_vote"), + ] + + def __str__(self) -> str: + return f"[{self.source}] {self.card.name} -> {self.tag} ({VotePolarity(self.polarity).label})" + + class TagSuggestionStatus(models.TextChoices): PENDING = "pending", "Pending" AUTO_ACCEPTED = "auto_accepted", "Auto-accepted" diff --git a/MPCAutofill/cardpicker/printing_consensus.py b/MPCAutofill/cardpicker/printing_consensus.py index bbb489d05..f76c65176 100644 --- a/MPCAutofill/cardpicker/printing_consensus.py +++ b/MPCAutofill/cardpicker/printing_consensus.py @@ -1,4 +1,3 @@ -from collections import defaultdict from typing import Literal, TypedDict from django.conf import settings @@ -8,72 +7,59 @@ CanonicalCard, Card, CardPrintingTag, - CardPrintingTagSource, PrintingTagStatus, + VoteSource, ) +from cardpicker.vote_consensus import VoteTuple, resolve_weighted_consensus NO_MATCH: Literal["NO_MATCH"] = "NO_MATCH" _SOURCE_WEIGHTS: dict[str, float] = { - CardPrintingTagSource.USER: 1.0, - CardPrintingTagSource.ADMIN: settings.PRINTING_TAG_ADMIN_WEIGHT, - CardPrintingTagSource.AI: settings.PRINTING_TAG_AI_WEIGHT, + VoteSource.USER: 1.0, + VoteSource.ADMIN: settings.PRINTING_TAG_ADMIN_WEIGHT, + VoteSource.AI: settings.PRINTING_TAG_AI_WEIGHT, } -class _VoteGroup(TypedDict): - weight: float - has_non_ai: bool - printing: CanonicalCard | None - - def resolve_printing(card: Card) -> CanonicalCard | Literal["NO_MATCH"] | None: """ 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 (no votes, a tie, or a genuinely contested set of votes). - - Votes are weighted by their `source` (`PRINTING_TAG_ADMIN_WEIGHT`/`PRINTING_TAG_AI_WEIGHT` - settings; user votes always weigh 1). Votes are grouped by outcome, and the - highest-weighted group wins if, and only if, ALL of the following hold: - - its summed weight is >= `PRINTING_TAG_MIN_VOTES` (this is compared against the - summed weight, not a raw row count — a single admin vote, at the default weight - of 5, already clears the default threshold of 2 on its own, which is what - produces "admin override" behaviour from this one unified formula, with no - special-cased branch for admin votes); - - its share of the total weight across all groups is >= `PRINTING_TAG_MIN_SHARE`; - - it contains at least one non-AI vote (a hard gate, independent of the weight - math above, so that no volume of AI-only votes can ever resolve consensus on - their own). + 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`. """ votes = list(card.printing_tags.all()) if not votes: return None - groups: dict[int | Literal["NO_MATCH"] | None, _VoteGroup] = defaultdict( - lambda: _VoteGroup(weight=0.0, has_non_ai=False, printing=None) - ) + printings_by_id: dict[int, CanonicalCard] = {} + vote_tuples: list[VoteTuple] = [] for vote in votes: - key: int | Literal["NO_MATCH"] | None = NO_MATCH if vote.is_no_match else vote.printing_id - group = groups[key] - group["weight"] += _SOURCE_WEIGHTS[vote.source] - if vote.source != CardPrintingTagSource.AI: - group["has_non_ai"] = True - if not vote.is_no_match: - group["printing"] = vote.printing - - total_weight = sum(group["weight"] for group in groups.values()) - winning_key, winner = max(groups.items(), key=lambda item: item[1]["weight"]) - share = winner["weight"] / total_weight - - if ( - winner["weight"] >= settings.PRINTING_TAG_MIN_VOTES - and share >= settings.PRINTING_TAG_MIN_SHARE - and winner["has_non_ai"] - ): - return NO_MATCH if winning_key == NO_MATCH else winner["printing"] - return None + key: int | Literal["NO_MATCH"] + if vote.is_no_match: + key = NO_MATCH + 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 + vote_tuples.append( + VoteTuple(outcome_key=key, weight=_SOURCE_WEIGHTS[vote.source], is_ai=vote.source == VoteSource.AI) + ) + + winning_key = resolve_weighted_consensus( + vote_tuples, min_weight=settings.PRINTING_TAG_MIN_VOTES, min_share=settings.PRINTING_TAG_MIN_SHARE + ) + if winning_key is None: + return None + if winning_key == NO_MATCH: + return NO_MATCH + assert isinstance(winning_key, int) + return printings_by_id[winning_key] def resolve_and_persist_printing(card: Card) -> CanonicalCard | Literal["NO_MATCH"] | None: diff --git a/MPCAutofill/cardpicker/schema_types.py b/MPCAutofill/cardpicker/schema_types.py index fea82dae9..90fe41e47 100644 --- a/MPCAutofill/cardpicker/schema_types.py +++ b/MPCAutofill/cardpicker/schema_types.py @@ -13,23 +13,13 @@ EnumT = TypeVar("EnumT", bound=Enum) -def from_list(f: Callable[[Any], T], x: Any) -> List[T]: - assert isinstance(x, list) - return [f(y) for y in x] - - def from_str(x: Any) -> str: assert isinstance(x, str) return x -def from_int(x: Any) -> int: - assert isinstance(x, int) and not isinstance(x, bool) - return x - - -def from_bool(x: Any) -> bool: - assert isinstance(x, bool) +def from_none(x: Any) -> Any: + assert x is None return x @@ -42,13 +32,23 @@ def from_union(fs, x): assert False +def from_list(f: Callable[[Any], T], x: Any) -> List[T]: + assert isinstance(x, list) + return [f(y) for y in x] + + def to_class(c: Type[T], x: Any) -> dict: assert isinstance(x, c) return cast(Any, x).to_dict() -def from_none(x: Any) -> Any: - assert x is None +def from_int(x: Any) -> int: + assert isinstance(x, int) and not isinstance(x, bool) + return x + + +def from_bool(x: Any) -> bool: + assert isinstance(x, bool) return x @@ -76,6 +76,118 @@ class Game(str, Enum): MTG = "MTG" +class ArtistCandidatesRequest(BaseModel): + identifier: str + query: Optional[str] = None + + @staticmethod + def from_dict(obj: Any) -> "ArtistCandidatesRequest": + assert isinstance(obj, dict) + identifier = from_str(obj.get("identifier")) + query = from_union([from_none, from_str], obj.get("query")) + return ArtistCandidatesRequest(identifier, query) + + def to_dict(self) -> dict: + result: dict = {} + result["identifier"] = from_str(self.identifier) + if self.query is not None: + result["query"] = from_union([from_none, from_str], self.query) + return result + + +class CanonicalArtistClass(BaseModel): + name: str + + @staticmethod + def from_dict(obj: Any) -> "CanonicalArtistClass": + assert isinstance(obj, dict) + name = from_str(obj.get("name")) + return CanonicalArtistClass(name) + + def to_dict(self) -> dict: + result: dict = {} + result["name"] = from_str(self.name) + return result + + +class ArtistCandidatesResponse(BaseModel): + results: List[Optional[CanonicalArtistClass]] + + @staticmethod + def from_dict(obj: Any) -> "ArtistCandidatesResponse": + assert isinstance(obj, dict) + results = from_list(lambda x: from_union([from_none, CanonicalArtistClass.from_dict], x), obj.get("results")) + return ArtistCandidatesResponse(results) + + def to_dict(self) -> dict: + result: dict = {} + result["results"] = from_list( + lambda x: from_union([from_none, lambda x: to_class(CanonicalArtistClass, x)], x), self.results + ) + return result + + +class ArtistConsensusRequest(BaseModel): + identifier: str + + @staticmethod + def from_dict(obj: Any) -> "ArtistConsensusRequest": + assert isinstance(obj, dict) + identifier = from_str(obj.get("identifier")) + return ArtistConsensusRequest(identifier) + + def to_dict(self) -> dict: + result: dict = {} + result["identifier"] = from_str(self.identifier) + return result + + +class ArtistVoteTallyEntry(BaseModel): + count: int + isUnknown: bool + artist: Optional[CanonicalArtistClass] = None + + @staticmethod + def from_dict(obj: Any) -> "ArtistVoteTallyEntry": + assert isinstance(obj, dict) + count = from_int(obj.get("count")) + isUnknown = from_bool(obj.get("isUnknown")) + artist = from_union([from_none, CanonicalArtistClass.from_dict], obj.get("artist")) + return ArtistVoteTallyEntry(count, isUnknown, artist) + + def to_dict(self) -> dict: + result: dict = {} + result["count"] = from_int(self.count) + result["isUnknown"] = from_bool(self.isUnknown) + if self.artist is not None: + result["artist"] = from_union([from_none, lambda x: to_class(CanonicalArtistClass, x)], self.artist) + return result + + +class ArtistConsensusResponse(BaseModel): + isUnknown: bool + voteTally: List[ArtistVoteTallyEntry] + resolvedArtist: Optional[CanonicalArtistClass] = None + + @staticmethod + def from_dict(obj: Any) -> "ArtistConsensusResponse": + assert isinstance(obj, dict) + isUnknown = from_bool(obj.get("isUnknown")) + voteTally = from_list(ArtistVoteTallyEntry.from_dict, obj.get("voteTally")) + resolvedArtist = from_union([from_none, CanonicalArtistClass.from_dict], obj.get("resolvedArtist")) + return ArtistConsensusResponse(isUnknown, voteTally, resolvedArtist) + + def to_dict(self) -> dict: + result: dict = {} + result["isUnknown"] = from_bool(self.isUnknown) + result["voteTally"] = from_list(lambda x: to_class(ArtistVoteTallyEntry, x), self.voteTally) + if self.resolvedArtist is not None: + result["resolvedArtist"] = from_union( + [from_none, lambda x: to_class(CanonicalArtistClass, x)], self.resolvedArtist + ) + return result + + class FilterSettings(BaseModel): excludesTags: List[str] """The tags which the cards must *not* have to be included in search results""" @@ -222,21 +334,6 @@ def to_dict(self) -> dict: return result -class CanonicalArtistClass(BaseModel): - name: str - - @staticmethod - def from_dict(obj: Any) -> "CanonicalArtistClass": - assert isinstance(obj, dict) - name = from_str(obj.get("name")) - return CanonicalArtistClass(name) - - def to_dict(self) -> dict: - result: dict = {} - result["name"] = from_str(self.name) - return result - - class CanonicalCardClass(BaseModel): collectorNumber: str expansionCode: str @@ -1245,6 +1342,31 @@ def to_dict(self) -> dict: return result +class SubmitArtistVoteRequest(BaseModel): + anonymousId: str + identifier: str + isUnknown: bool + artistName: Optional[str] = None + + @staticmethod + def from_dict(obj: Any) -> "SubmitArtistVoteRequest": + assert isinstance(obj, dict) + anonymousId = from_str(obj.get("anonymousId")) + identifier = from_str(obj.get("identifier")) + isUnknown = from_bool(obj.get("isUnknown")) + artistName = from_union([from_none, from_str], obj.get("artistName")) + return SubmitArtistVoteRequest(anonymousId, identifier, isUnknown, artistName) + + def to_dict(self) -> dict: + result: dict = {} + result["anonymousId"] = from_str(self.anonymousId) + result["identifier"] = from_str(self.identifier) + result["isUnknown"] = from_bool(self.isUnknown) + if self.artistName is not None: + result["artistName"] = from_union([from_none, from_str], self.artistName) + return result + + class SubmitPrintingTagRequest(BaseModel): anonymousId: str identifier: str @@ -1270,6 +1392,100 @@ def to_dict(self) -> dict: return result +class SubmitTagVoteRequest(BaseModel): + anonymousId: str + identifier: str + polarity: int + tagName: str + + @staticmethod + def from_dict(obj: Any) -> "SubmitTagVoteRequest": + assert isinstance(obj, dict) + anonymousId = from_str(obj.get("anonymousId")) + identifier = from_str(obj.get("identifier")) + polarity = from_int(obj.get("polarity")) + tagName = from_str(obj.get("tagName")) + return SubmitTagVoteRequest(anonymousId, identifier, polarity, tagName) + + def to_dict(self) -> dict: + result: dict = {} + result["anonymousId"] = from_str(self.anonymousId) + result["identifier"] = from_str(self.identifier) + result["polarity"] = from_int(self.polarity) + result["tagName"] = from_str(self.tagName) + return result + + +class TagConsensusRequest(BaseModel): + identifier: str + + @staticmethod + def from_dict(obj: Any) -> "TagConsensusRequest": + assert isinstance(obj, dict) + identifier = from_str(obj.get("identifier")) + return TagConsensusRequest(identifier) + + def to_dict(self) -> dict: + result: dict = {} + result["identifier"] = from_str(self.identifier) + return result + + +class TagVoteTallyEntry(BaseModel): + count: int + polarity: int + + @staticmethod + def from_dict(obj: Any) -> "TagVoteTallyEntry": + assert isinstance(obj, dict) + count = from_int(obj.get("count")) + polarity = from_int(obj.get("polarity")) + return TagVoteTallyEntry(count, polarity) + + def to_dict(self) -> dict: + result: dict = {} + result["count"] = from_int(self.count) + result["polarity"] = from_int(self.polarity) + return result + + +class TagConsensusEntry(BaseModel): + tagName: str + tally: List[TagVoteTallyEntry] + resolvedPolarity: Optional[int] = None + + @staticmethod + def from_dict(obj: Any) -> "TagConsensusEntry": + assert isinstance(obj, dict) + tagName = from_str(obj.get("tagName")) + tally = from_list(TagVoteTallyEntry.from_dict, obj.get("tally")) + resolvedPolarity = from_union([from_none, from_int], obj.get("resolvedPolarity")) + return TagConsensusEntry(tagName, tally, resolvedPolarity) + + def to_dict(self) -> dict: + result: dict = {} + result["tagName"] = from_str(self.tagName) + result["tally"] = from_list(lambda x: to_class(TagVoteTallyEntry, x), self.tally) + if self.resolvedPolarity is not None: + result["resolvedPolarity"] = from_union([from_none, from_int], self.resolvedPolarity) + return result + + +class TagConsensusResponse(BaseModel): + tags: List[TagConsensusEntry] + + @staticmethod + def from_dict(obj: Any) -> "TagConsensusResponse": + assert isinstance(obj, dict) + tags = from_list(TagConsensusEntry.from_dict, obj.get("tags")) + return TagConsensusResponse(tags) + + def to_dict(self) -> dict: + result: dict = {} + result["tags"] = from_list(lambda x: to_class(TagConsensusEntry, x), self.tags) + return result + + class ChildElement(BaseModel): children: List["ChildElement"] name: str @@ -1343,6 +1559,14 @@ def to_dict(self) -> dict: return result +def ArtistVoteTallyEntryfromdict(s: Any) -> ArtistVoteTallyEntry: + return ArtistVoteTallyEntry.from_dict(s) + + +def ArtistVoteTallyEntrytodict(x: ArtistVoteTallyEntry) -> Any: + return to_class(ArtistVoteTallyEntry, x) + + def Campaignfromdict(s: Any) -> Optional[CampaignClass]: return from_union([from_none, CampaignClass.from_dict], s) @@ -1527,6 +1751,22 @@ def Tagtodict(x: Tag) -> Any: return to_class(Tag, x) +def TagConsensusEntryfromdict(s: Any) -> TagConsensusEntry: + return TagConsensusEntry.from_dict(s) + + +def TagConsensusEntrytodict(x: TagConsensusEntry) -> Any: + return to_class(TagConsensusEntry, x) + + +def TagVoteTallyEntryfromdict(s: Any) -> TagVoteTallyEntry: + return TagVoteTallyEntry.from_dict(s) + + +def TagVoteTallyEntrytodict(x: TagVoteTallyEntry) -> Any: + return to_class(TagVoteTallyEntry, x) + + def VoteTallyEntryfromdict(s: Any) -> VoteTallyEntry: return VoteTallyEntry.from_dict(s) @@ -1535,6 +1775,38 @@ def VoteTallyEntrytodict(x: VoteTallyEntry) -> Any: return to_class(VoteTallyEntry, x) +def ArtistCandidatesRequestfromdict(s: Any) -> ArtistCandidatesRequest: + return ArtistCandidatesRequest.from_dict(s) + + +def ArtistCandidatesRequesttodict(x: ArtistCandidatesRequest) -> Any: + return to_class(ArtistCandidatesRequest, x) + + +def ArtistCandidatesResponsefromdict(s: Any) -> ArtistCandidatesResponse: + return ArtistCandidatesResponse.from_dict(s) + + +def ArtistCandidatesResponsetodict(x: ArtistCandidatesResponse) -> Any: + return to_class(ArtistCandidatesResponse, x) + + +def ArtistConsensusRequestfromdict(s: Any) -> ArtistConsensusRequest: + return ArtistConsensusRequest.from_dict(s) + + +def ArtistConsensusRequesttodict(x: ArtistConsensusRequest) -> Any: + return to_class(ArtistConsensusRequest, x) + + +def ArtistConsensusResponsefromdict(s: Any) -> ArtistConsensusResponse: + return ArtistConsensusResponse.from_dict(s) + + +def ArtistConsensusResponsetodict(x: ArtistConsensusResponse) -> Any: + return to_class(ArtistConsensusResponse, x) + + def CardbacksRequestfromdict(s: Any) -> CardbacksRequest: return CardbacksRequest.from_dict(s) @@ -1767,6 +2039,14 @@ def SourcesResponsetodict(x: SourcesResponse) -> Any: return to_class(SourcesResponse, x) +def SubmitArtistVoteRequestfromdict(s: Any) -> SubmitArtistVoteRequest: + return SubmitArtistVoteRequest.from_dict(s) + + +def SubmitArtistVoteRequesttodict(x: SubmitArtistVoteRequest) -> Any: + return to_class(SubmitArtistVoteRequest, x) + + def SubmitPrintingTagRequestfromdict(s: Any) -> SubmitPrintingTagRequest: return SubmitPrintingTagRequest.from_dict(s) @@ -1775,6 +2055,30 @@ def SubmitPrintingTagRequesttodict(x: SubmitPrintingTagRequest) -> Any: return to_class(SubmitPrintingTagRequest, x) +def SubmitTagVoteRequestfromdict(s: Any) -> SubmitTagVoteRequest: + return SubmitTagVoteRequest.from_dict(s) + + +def SubmitTagVoteRequesttodict(x: SubmitTagVoteRequest) -> Any: + return to_class(SubmitTagVoteRequest, x) + + +def TagConsensusRequestfromdict(s: Any) -> TagConsensusRequest: + return TagConsensusRequest.from_dict(s) + + +def TagConsensusRequesttodict(x: TagConsensusRequest) -> Any: + return to_class(TagConsensusRequest, x) + + +def TagConsensusResponsefromdict(s: Any) -> TagConsensusResponse: + return TagConsensusResponse.from_dict(s) + + +def TagConsensusResponsetodict(x: TagConsensusResponse) -> Any: + return to_class(TagConsensusResponse, x) + + def TagsResponsefromdict(s: Any) -> TagsResponse: return TagsResponse.from_dict(s) diff --git a/MPCAutofill/cardpicker/sources/update_database.py b/MPCAutofill/cardpicker/sources/update_database.py index c48e1ffb1..11ae1c1af 100644 --- a/MPCAutofill/cardpicker/sources/update_database.py +++ b/MPCAutofill/cardpicker/sources/update_database.py @@ -10,10 +10,11 @@ from cardpicker.constants import DEFAULT_LANGUAGE, MAX_SIZE_MB from cardpicker.documents import CardSearch -from cardpicker.models import Card, CardTypes, Source +from cardpicker.models import Card, CardTypes, Source, VotePolarity from cardpicker.search.sanitisation import to_searchable from cardpicker.sources.api import Folder, Image from cardpicker.sources.source_types import SourceType, SourceTypeChoices +from cardpicker.tag_consensus import get_resolved_tag_overlay from cardpicker.tags import Tags from cardpicker.utils import TEXT_BOLD, TEXT_END @@ -167,10 +168,29 @@ def bulk_sync_objects(source: Source, cards: list[Card]) -> None: incoming_ids = set(incoming.keys()) existing = {card.identifier: card for card in Card.objects.filter(source=source)} existing_ids = set(existing.keys()) + common_ids = incoming_ids & existing_ids + + # Merge any resolved tag-vote consensus into the freshly re-extracted tags *before* the + # change-detection check below runs, so a scheduled re-scan can never silently revert a + # community-resolved tag correction back to whatever the filename currently says (only + # `common_ids` can have prior votes at all - a vote's `card` FK requires an existing PK, + # so a brand-new card being `bulk_create`d can't yet have any). + tag_overlay = get_resolved_tag_overlay(existing[identifier].pk for identifier in common_ids) + for identifier in common_ids: + overlay = tag_overlay.get(existing[identifier].pk) + if not overlay: + continue + tags = set(incoming[identifier].tags) + for tag_name, polarity in overlay.items(): + if polarity == VotePolarity.APPLY: + tags.add(tag_name) + else: + tags.discard(tag_name) + incoming[identifier].tags = sorted(tags) created = [incoming[identifier] for identifier in incoming_ids - existing_ids] updated: list[Card] = [] - for identifier in incoming_ids & existing_ids: + for identifier in common_ids: if ( # if an update has been recorded on the source's end... (incoming[identifier].date_modified > existing[identifier].date_modified) diff --git a/MPCAutofill/cardpicker/tag_consensus.py b/MPCAutofill/cardpicker/tag_consensus.py new file mode 100644 index 000000000..544cb8b07 --- /dev/null +++ b/MPCAutofill/cardpicker/tag_consensus.py @@ -0,0 +1,138 @@ +from collections import defaultdict +from typing import Iterable, TypedDict + +from django.conf import settings + +from cardpicker.models import Card, CardTagVote, Tag, VotePolarity, VoteSource +from cardpicker.vote_consensus import VoteTuple, resolve_weighted_consensus + +# reuses the printing-tag weight/threshold settings rather than introducing a parallel set - +# see the plan note in the models: per-vote-type overrides are YAGNI until a real need appears. +_SOURCE_WEIGHTS: dict[str, float] = { + VoteSource.USER: 1.0, + VoteSource.ADMIN: settings.PRINTING_TAG_ADMIN_WEIGHT, + VoteSource.AI: settings.PRINTING_TAG_AI_WEIGHT, +} + + +def resolve_tag(card: Card, tag: Tag) -> int | None: + """ + Reconciles all `CardTagVote` votes cast for (card, tag) into a single resolved polarity + (`VotePolarity.APPLY` or `VotePolarity.NOT_APPLICABLE`), or `None` if unresolved. Built on + the same shared `resolve_weighted_consensus` core as printing/artist consensus - the only + difference is the outcome space is the two `VotePolarity` values rather than a printing or + artist id. + """ + votes = list(card.tag_votes.filter(tag=tag)) + if not votes: + return None + vote_tuples = [ + VoteTuple(outcome_key=vote.polarity, weight=_SOURCE_WEIGHTS[vote.source], is_ai=vote.source == VoteSource.AI) + for vote in votes + ] + resolved = resolve_weighted_consensus( + vote_tuples, min_weight=settings.PRINTING_TAG_MIN_VOTES, min_share=settings.PRINTING_TAG_MIN_SHARE + ) + if resolved is None: + return None + assert isinstance(resolved, int) + return resolved + + +def resolve_and_persist_tag_votes(card: Card) -> None: + """ + Resolves consensus for every tag that has at least one vote cast against `card` (tags are + multi-valued per card, unlike printing/artist, so this resolves all of them in one pass + rather than a single outcome), and merges the result directly into `card.tags`: a resolved + APPLY adds the tag name if not already present; a resolved NOT_APPLICABLE removes it if + present. Saves `card.tags` and pushes the change into Elasticsearch immediately - unlike + printing/artist consensus (whose denormalised fields aren't ES-indexed), `tags` *is* an + ES-indexed field (`documents.py`'s `KeywordField`), so a vote-triggered change has to reach + the search index directly rather than waiting for the next scheduled re-scan. + """ + from cardpicker.documents import ( + CardSearch, # local import - avoids a top-level ES dependency in this module + ) + + voted_tag_ids = list(card.tag_votes.values_list("tag_id", flat=True).distinct()) + if not voted_tag_ids: + return + + tags_by_id = {tag.pk: tag for tag in Tag.objects.filter(pk__in=voted_tag_ids)} + current_tags = set(card.tags) + changed = False + for tag_id, tag in tags_by_id.items(): + resolved = resolve_tag(card, tag) + if resolved == VotePolarity.APPLY and tag.name not in current_tags: + current_tags.add(tag.name) + changed = True + elif resolved == VotePolarity.NOT_APPLICABLE and tag.name in current_tags: + current_tags.discard(tag.name) + changed = True + + if changed: + card.tags = sorted(current_tags) + card.save(update_fields=["tags"]) + CardSearch().update([card], action="index") + + +class TagVoteTallyEntry(TypedDict): + polarity: int + count: int + + +def get_tag_vote_tally(card: Card, tag: Tag) -> list[TagVoteTallyEntry]: + """Plain, unweighted per-polarity vote count for (card, tag) - mirrors `get_vote_tally`.""" + tally: dict[int, int] = defaultdict(int) + for vote in card.tag_votes.filter(tag=tag): + tally[vote.polarity] += 1 + return sorted( + (TagVoteTallyEntry(polarity=polarity, count=count) for polarity, count in tally.items()), + key=lambda entry: entry["count"], + reverse=True, + ) + + +def get_resolved_tag_overlay(card_ids: Iterable[int]) -> dict[int, dict[str, int]]: + """ + Batched version of `resolve_tag`, computed for every (card, tag) pair with at least one + vote among `card_ids` in a single query - returns `{card_id: {tag_name: resolved_polarity}}`. + + Used by `cardpicker.sources.update_database.bulk_sync_objects` to merge consensus + corrections into freshly re-scanned `Card.tags` before they're written, so a scheduled + re-scan can never silently revert a resolved tag-vote correction back to whatever the + filename currently says. + """ + rows = CardTagVote.objects.filter(card_id__in=card_ids).values( + "card_id", "tag_id", "tag__name", "source", "polarity" + ) + grouped: dict[tuple[int, int], list[VoteTuple]] = defaultdict(list) + tag_names: dict[int, str] = {} + for row in rows: + tag_names[row["tag_id"]] = row["tag__name"] + grouped[(row["card_id"], row["tag_id"])].append( + VoteTuple( + outcome_key=row["polarity"], + weight=_SOURCE_WEIGHTS[row["source"]], + is_ai=row["source"] == VoteSource.AI, + ) + ) + + overlay: dict[int, dict[str, int]] = defaultdict(dict) + for (card_id, tag_id), vote_tuples in grouped.items(): + resolved = resolve_weighted_consensus( + vote_tuples, min_weight=settings.PRINTING_TAG_MIN_VOTES, min_share=settings.PRINTING_TAG_MIN_SHARE + ) + if resolved is not None: + assert isinstance(resolved, int) + overlay[card_id][tag_names[tag_id]] = resolved + return dict(overlay) + + +__all__ = [ + "resolve_tag", + "resolve_and_persist_tag_votes", + "get_tag_vote_tally", + "get_resolved_tag_overlay", + "TagVoteTallyEntry", +] diff --git a/MPCAutofill/cardpicker/tests/factories.py b/MPCAutofill/cardpicker/tests/factories.py index b9a0a6858..9604f44ad 100644 --- a/MPCAutofill/cardpicker/tests/factories.py +++ b/MPCAutofill/cardpicker/tests/factories.py @@ -117,5 +117,29 @@ class Meta: printing = factory.SubFactory(CanonicalCardFactory) is_no_match = False anonymous_id = factory.Sequence(lambda n: f"anonymous_{n}") - source = models.CardPrintingTagSource.USER + source = models.VoteSource.USER + confidence = None + + +class CardArtistVoteFactory(factory.django.DjangoModelFactory): + class Meta: + model = models.CardArtistVote + + card = factory.SubFactory(CardFactory) + artist = factory.SubFactory(CanonicalArtistFactory) + is_unknown = False + anonymous_id = factory.Sequence(lambda n: f"anonymous_{n}") + source = models.VoteSource.USER + confidence = None + + +class CardTagVoteFactory(factory.django.DjangoModelFactory): + class Meta: + model = models.CardTagVote + + card = factory.SubFactory(CardFactory) + tag = factory.SubFactory(TagFactory) + polarity = models.VotePolarity.APPLY + anonymous_id = factory.Sequence(lambda n: f"anonymous_{n}") + source = models.VoteSource.USER confidence = None diff --git a/MPCAutofill/cardpicker/tests/test_artist_votes.py b/MPCAutofill/cardpicker/tests/test_artist_votes.py new file mode 100644 index 000000000..acf9c6053 --- /dev/null +++ b/MPCAutofill/cardpicker/tests/test_artist_votes.py @@ -0,0 +1,348 @@ +import pytest + +from django.core.cache import cache +from django.urls import reverse + +from cardpicker import views +from cardpicker.artist_consensus import ( + UNKNOWN, + get_artist_vote_tally, + resolve_and_persist_artist, + resolve_artist, +) +from cardpicker.models import ArtistVoteStatus, CardArtistVote, VoteSource +from cardpicker.tests.factories import ( + CanonicalArtistFactory, + CanonicalCardFactory, + CanonicalExpansionFactory, + CardArtistVoteFactory, + CardFactory, + SourceFactory, +) + +# see test_printing_consensus.py for why this capture-and-restore fixture exists +_SHARED_FACTORIES = [ + CardFactory, + SourceFactory, + CanonicalArtistFactory, + CanonicalExpansionFactory, + CanonicalCardFactory, +] + + +@pytest.fixture(autouse=True) +def _preserve_shared_factory_sequences(): + before = {f: f._meta.next_sequence() for f in _SHARED_FACTORIES} + for f, n in before.items(): + f.reset_sequence(n, force=True) + yield + for f, n in before.items(): + f.reset_sequence(n, force=True) + + +@pytest.fixture(autouse=True) +def _clear_rate_limit_cache(): + cache.clear() + yield + cache.clear() + + +class TestResolveArtist: + def test_no_votes_returns_none(self, db): + card = CardFactory() + assert resolve_artist(card) is None + + def test_consensus(self, db): + card = CardFactory() + artist = CanonicalArtistFactory() + CardArtistVoteFactory(card=card, artist=artist, source=VoteSource.USER) + CardArtistVoteFactory(card=card, artist=artist, source=VoteSource.USER) + assert resolve_artist(card) == artist + + def test_unknown_wins_consensus(self, db): + card = CardFactory() + artist = CanonicalArtistFactory() + CardArtistVoteFactory(card=card, artist=None, is_unknown=True, source=VoteSource.USER) + CardArtistVoteFactory(card=card, artist=None, is_unknown=True, source=VoteSource.USER) + CardArtistVoteFactory(card=card, artist=artist, source=VoteSource.USER) + assert resolve_artist(card) == UNKNOWN + + def test_admin_override(self, db): + card = CardFactory() + artist_a = CanonicalArtistFactory() + artist_b = CanonicalArtistFactory() + CardArtistVoteFactory(card=card, artist=artist_a, source=VoteSource.ADMIN) + CardArtistVoteFactory(card=card, artist=artist_b, source=VoteSource.USER) + CardArtistVoteFactory(card=card, artist=artist_b, source=VoteSource.USER) + assert resolve_artist(card) == artist_a + + def test_ai_only_insufficient(self, db): + card = CardFactory() + artist = CanonicalArtistFactory() + for _ in range(4): + CardArtistVoteFactory(card=card, artist=artist, source=VoteSource.AI) + assert resolve_artist(card) is None + + +class TestResolveAndPersistArtist: + def test_persists_resolved_artist(self, db): + card = CardFactory() + artist = CanonicalArtistFactory() + CardArtistVoteFactory(card=card, artist=artist, source=VoteSource.ADMIN) + + result = resolve_and_persist_artist(card) + + assert result == artist + card.refresh_from_db() + assert card.inferred_canonical_artist == artist + assert card.artist_vote_status == ArtistVoteStatus.RESOLVED + + def test_persists_unknown(self, db): + card = CardFactory() + CardArtistVoteFactory(card=card, artist=None, is_unknown=True, source=VoteSource.ADMIN) + + result = resolve_and_persist_artist(card) + + assert result == UNKNOWN + card.refresh_from_db() + assert card.inferred_canonical_artist is None + assert card.artist_vote_status == ArtistVoteStatus.UNKNOWN + + def test_persists_unresolved(self, db): + card = CardFactory() + CardArtistVoteFactory(card=card, source=VoteSource.USER) + + result = resolve_and_persist_artist(card) + + assert result is None + card.refresh_from_db() + assert card.inferred_canonical_artist is None + assert card.artist_vote_status == ArtistVoteStatus.UNRESOLVED + + def test_does_not_consult_printing_tag_status(self, db): + # resolve_and_persist_artist is deliberately decoupled from printing_tag_status - the + # precedence rule (a resolved printing's artist wins) lives entirely in + # Card.serialise()'s fallback chain, not here. + from cardpicker.models import PrintingTagStatus + + card = CardFactory(printing_tag_status=PrintingTagStatus.RESOLVED) + artist = CanonicalArtistFactory() + CardArtistVoteFactory(card=card, artist=artist, source=VoteSource.ADMIN) + + result = resolve_and_persist_artist(card) + + assert result == artist + card.refresh_from_db() + assert card.inferred_canonical_artist == artist + + +class TestGetArtistVoteTally: + def test_tally_groups_by_outcome(self, db): + card = CardFactory() + artist = CanonicalArtistFactory() + CardArtistVoteFactory(card=card, artist=artist) + CardArtistVoteFactory(card=card, artist=artist) + CardArtistVoteFactory(card=card, artist=None, is_unknown=True) + + tally = get_artist_vote_tally(card) + + assert {(entry["count"], entry["is_unknown"]) for entry in tally} == {(2, False), (1, True)} + + +class TestSerialisePrecedenceChain: + def test_falls_back_to_inferred_canonical_artist(self, db): + card = CardFactory() + artist = CanonicalArtistFactory(name="Vote Artist") + card.inferred_canonical_artist = artist + card.save(update_fields=["inferred_canonical_artist"]) + + assert card.serialise().canonicalArtist.name == "Vote Artist" + + def test_resolved_printing_artist_beats_inferred_canonical_artist(self, db): + printing = CanonicalCardFactory() + card = CardFactory(inferred_canonical_card=printing) + vote_artist = CanonicalArtistFactory(name="Vote Artist") + card.inferred_canonical_artist = vote_artist + card.save(update_fields=["inferred_canonical_artist"]) + + assert card.serialise().canonicalArtist.name == printing.artist.name + assert card.serialise().canonicalArtist.name != "Vote Artist" + + def test_confirmed_canonical_card_artist_beats_everything_inferred(self, db): + confirmed = CanonicalCardFactory() + inferred = CanonicalCardFactory() + card = CardFactory(canonical_card=confirmed, inferred_canonical_card=inferred) + + assert card.serialise().canonicalArtist.name == confirmed.artist.name + + +class TestPostArtistCandidates: + def test_unknown_card_identifier_is_a_bad_request(self, client, django_settings): + response = client.post( + reverse(views.post_artist_candidates), + {"identifier": "does-not-exist"}, + content_type="application/json", + ) + assert response.status_code == 400 + + def test_defaults_to_deduped_artists_of_ranked_printing_candidates(self, client, django_settings): + card = CardFactory(name="Brainstorm") + artist = CanonicalArtistFactory() + CanonicalCardFactory(name="Brainstorm", artist=artist) + CanonicalCardFactory(name="Brainstorm", artist=artist) # same artist, shouldn't duplicate + + response = client.post( + reverse(views.post_artist_candidates), + {"identifier": card.identifier}, + content_type="application/json", + ) + + assert response.status_code == 200 + assert [result["name"] for result in response.json()["results"]] == [artist.name] + + def test_query_switches_to_typeahead_search(self, client, django_settings): + card = CardFactory() + matching = CanonicalArtistFactory(name="John Avon") + CanonicalArtistFactory(name="Someone Else") + + response = client.post( + reverse(views.post_artist_candidates), + {"identifier": card.identifier, "query": "Avon"}, + content_type="application/json", + ) + + result_names = {result["name"] for result in response.json()["results"]} + assert result_names == {matching.name} + + +class TestPostArtistConsensus: + def test_unknown_card_identifier_is_a_bad_request(self, client, django_settings): + response = client.post( + reverse(views.post_artist_consensus), + {"identifier": "does-not-exist"}, + content_type="application/json", + ) + assert response.status_code == 400 + + def test_no_votes_yet(self, client, django_settings): + card = CardFactory() + response = client.post( + reverse(views.post_artist_consensus), + {"identifier": card.identifier}, + content_type="application/json", + ) + body = response.json() + assert body["resolvedArtist"] is None + assert body["isUnknown"] is False + assert body["voteTally"] == [] + + +class TestPostSubmitArtistVote: + def test_unknown_card_identifier_is_a_bad_request(self, client, django_settings): + response = client.post( + reverse(views.post_submit_artist_vote), + {"identifier": "does-not-exist", "isUnknown": True, "anonymousId": "anon-1"}, + content_type="application/json", + ) + assert response.status_code == 400 + + def test_missing_artist_name_without_unknown_is_a_bad_request(self, client, django_settings): + card = CardFactory() + response = client.post( + reverse(views.post_submit_artist_vote), + {"identifier": card.identifier, "isUnknown": False, "anonymousId": "anon-1"}, + content_type="application/json", + ) + assert response.status_code == 400 + + def test_unknown_artist_name_is_a_bad_request(self, client, django_settings): + card = CardFactory() + response = client.post( + reverse(views.post_submit_artist_vote), + { + "identifier": card.identifier, + "artistName": "Nobody By This Name", + "isUnknown": False, + "anonymousId": "anon-1", + }, + content_type="application/json", + ) + assert response.status_code == 400 + + def test_creates_a_vote_and_persists_consensus(self, client, django_settings, settings): + settings.PRINTING_TAG_MIN_VOTES = 1 + settings.PRINTING_TAG_MIN_SHARE = 0.5 + card = CardFactory() + artist = CanonicalArtistFactory() + + response = client.post( + reverse(views.post_submit_artist_vote), + { + "identifier": card.identifier, + "artistName": artist.name, + "isUnknown": False, + "anonymousId": "anon-1", + }, + content_type="application/json", + ) + + assert response.status_code == 200 + assert response.json()["resolvedArtist"]["name"] == artist.name + card.refresh_from_db() + assert card.inferred_canonical_artist_id == artist.id + assert card.artist_vote_status == ArtistVoteStatus.RESOLVED + assert CardArtistVote.objects.filter(card=card, anonymous_id="anon-1").count() == 1 + + def test_resubmitting_replaces_the_previous_vote_from_the_same_anonymous_id(self, client, django_settings): + card = CardFactory() + artist_a = CanonicalArtistFactory() + artist_b = CanonicalArtistFactory() + + for artist in (artist_a, artist_b): + client.post( + reverse(views.post_submit_artist_vote), + { + "identifier": card.identifier, + "artistName": artist.name, + "isUnknown": False, + "anonymousId": "anon-1", + }, + content_type="application/json", + ) + + votes = CardArtistVote.objects.filter(card=card, anonymous_id="anon-1") + assert votes.count() == 1 + assert votes.get().artist_id == artist_b.id + + def test_unknown_vote(self, client, django_settings, settings): + settings.PRINTING_TAG_MIN_VOTES = 1 + settings.PRINTING_TAG_MIN_SHARE = 0.5 + card = CardFactory() + + response = client.post( + reverse(views.post_submit_artist_vote), + {"identifier": card.identifier, "isUnknown": True, "anonymousId": "anon-1"}, + content_type="application/json", + ) + + assert response.json()["isUnknown"] is True + card.refresh_from_db() + assert card.inferred_canonical_artist is None + assert card.artist_vote_status == ArtistVoteStatus.UNKNOWN + + def test_rate_limited_after_exceeding_the_configured_rate(self, client, django_settings, settings): + settings.PRINTING_TAG_SUBMISSION_RATE = "1/m" + card = CardFactory() + artist = CanonicalArtistFactory() + body = { + "identifier": card.identifier, + "artistName": artist.name, + "isUnknown": False, + "anonymousId": "anon-rate-limited", + } + + first = client.post(reverse(views.post_submit_artist_vote), body, content_type="application/json") + second = client.post(reverse(views.post_submit_artist_vote), body, content_type="application/json") + + assert first.status_code == 200 + assert second.status_code == 429 diff --git a/MPCAutofill/cardpicker/tests/test_printing_consensus.py b/MPCAutofill/cardpicker/tests/test_printing_consensus.py index 001fb60bf..937e8815a 100644 --- a/MPCAutofill/cardpicker/tests/test_printing_consensus.py +++ b/MPCAutofill/cardpicker/tests/test_printing_consensus.py @@ -1,6 +1,6 @@ import pytest -from cardpicker.models import CardPrintingTagSource +from cardpicker.models import VoteSource from cardpicker.printing_consensus import NO_MATCH, resolve_printing from cardpicker.tests.factories import ( CanonicalArtistFactory, @@ -43,8 +43,8 @@ def test_consensus(self, db): # two user votes agreeing on the same printing clears both thresholds outright card = CardFactory() printing = CanonicalCardFactory() - CardPrintingTagFactory(card=card, printing=printing, source=CardPrintingTagSource.USER) - CardPrintingTagFactory(card=card, printing=printing, source=CardPrintingTagSource.USER) + CardPrintingTagFactory(card=card, printing=printing, source=VoteSource.USER) + CardPrintingTagFactory(card=card, printing=printing, source=VoteSource.USER) assert resolve_printing(card) == printing def test_tie_returns_none(self, db): @@ -53,10 +53,10 @@ def test_tie_returns_none(self, db): card = CardFactory() printing_a = CanonicalCardFactory() printing_b = CanonicalCardFactory() - CardPrintingTagFactory(card=card, printing=printing_a, source=CardPrintingTagSource.USER) - CardPrintingTagFactory(card=card, printing=printing_a, source=CardPrintingTagSource.USER) - CardPrintingTagFactory(card=card, printing=printing_b, source=CardPrintingTagSource.USER) - CardPrintingTagFactory(card=card, printing=printing_b, source=CardPrintingTagSource.USER) + CardPrintingTagFactory(card=card, printing=printing_a, source=VoteSource.USER) + CardPrintingTagFactory(card=card, printing=printing_a, source=VoteSource.USER) + CardPrintingTagFactory(card=card, printing=printing_b, source=VoteSource.USER) + CardPrintingTagFactory(card=card, printing=printing_b, source=VoteSource.USER) assert resolve_printing(card) is None def test_contested_returns_none(self, db): @@ -66,19 +66,19 @@ def test_contested_returns_none(self, db): printing_a = CanonicalCardFactory() printing_b = CanonicalCardFactory() printing_c = CanonicalCardFactory() - CardPrintingTagFactory(card=card, printing=printing_a, source=CardPrintingTagSource.USER) - CardPrintingTagFactory(card=card, printing=printing_a, source=CardPrintingTagSource.USER) - CardPrintingTagFactory(card=card, printing=printing_b, source=CardPrintingTagSource.USER) - CardPrintingTagFactory(card=card, printing=printing_c, source=CardPrintingTagSource.USER) + CardPrintingTagFactory(card=card, printing=printing_a, source=VoteSource.USER) + CardPrintingTagFactory(card=card, printing=printing_a, source=VoteSource.USER) + CardPrintingTagFactory(card=card, printing=printing_b, source=VoteSource.USER) + CardPrintingTagFactory(card=card, printing=printing_c, source=VoteSource.USER) assert resolve_printing(card) is None def test_no_match_wins_consensus(self, db): # two no-match votes outweigh a single vote for a specific printing card = CardFactory() printing = CanonicalCardFactory() - CardPrintingTagFactory(card=card, printing=None, is_no_match=True, source=CardPrintingTagSource.USER) - CardPrintingTagFactory(card=card, printing=None, is_no_match=True, source=CardPrintingTagSource.USER) - CardPrintingTagFactory(card=card, printing=printing, source=CardPrintingTagSource.USER) + CardPrintingTagFactory(card=card, printing=None, is_no_match=True, source=VoteSource.USER) + CardPrintingTagFactory(card=card, printing=None, is_no_match=True, source=VoteSource.USER) + CardPrintingTagFactory(card=card, printing=printing, source=VoteSource.USER) assert resolve_printing(card) == NO_MATCH def test_admin_override(self, db): @@ -88,9 +88,9 @@ def test_admin_override(self, db): card = CardFactory() printing_a = CanonicalCardFactory() printing_b = CanonicalCardFactory() - CardPrintingTagFactory(card=card, printing=printing_a, source=CardPrintingTagSource.ADMIN) - CardPrintingTagFactory(card=card, printing=printing_b, source=CardPrintingTagSource.USER) - CardPrintingTagFactory(card=card, printing=printing_b, source=CardPrintingTagSource.USER) + CardPrintingTagFactory(card=card, printing=printing_a, source=VoteSource.ADMIN) + CardPrintingTagFactory(card=card, printing=printing_b, source=VoteSource.USER) + CardPrintingTagFactory(card=card, printing=printing_b, source=VoteSource.USER) assert resolve_printing(card) == printing_a def test_ai_only_insufficient(self, db): @@ -99,5 +99,5 @@ def test_ai_only_insufficient(self, db): card = CardFactory() printing = CanonicalCardFactory() for _ in range(4): - CardPrintingTagFactory(card=card, printing=printing, source=CardPrintingTagSource.AI) + CardPrintingTagFactory(card=card, printing=printing, source=VoteSource.AI) assert resolve_printing(card) is None diff --git a/MPCAutofill/cardpicker/tests/test_printing_tags_views.py b/MPCAutofill/cardpicker/tests/test_printing_tags_views.py index d64f458f2..690e86888 100644 --- a/MPCAutofill/cardpicker/tests/test_printing_tags_views.py +++ b/MPCAutofill/cardpicker/tests/test_printing_tags_views.py @@ -4,7 +4,7 @@ from django.urls import reverse from cardpicker import views -from cardpicker.models import CardPrintingTag, CardPrintingTagSource, PrintingTagStatus +from cardpicker.models import CardPrintingTag, PrintingTagStatus, VoteSource from cardpicker.tests.factories import ( CanonicalArtistFactory, CanonicalCardFactory, @@ -171,8 +171,8 @@ def test_no_votes_yet(self, client, django_settings): def test_resolved_consensus_and_tally(self, client, django_settings): card = CardFactory() printing = CanonicalCardFactory() - CardPrintingTagFactory(card=card, printing=printing, source=CardPrintingTagSource.USER) - CardPrintingTagFactory(card=card, printing=printing, source=CardPrintingTagSource.USER) + CardPrintingTagFactory(card=card, printing=printing, source=VoteSource.USER) + CardPrintingTagFactory(card=card, printing=printing, source=VoteSource.USER) response = client.post( reverse(views.post_printing_consensus), diff --git a/MPCAutofill/cardpicker/tests/test_sources.py b/MPCAutofill/cardpicker/tests/test_sources.py index 21de6b14a..aedb3f674 100644 --- a/MPCAutofill/cardpicker/tests/test_sources.py +++ b/MPCAutofill/cardpicker/tests/test_sources.py @@ -7,7 +7,7 @@ from django.utils.timezone import make_aware, make_naive from cardpicker.documents import CardSearch -from cardpicker.models import CanonicalArtist, CanonicalCard, Card +from cardpicker.models import CanonicalArtist, CanonicalCard, Card, VotePolarity from cardpicker.sources.api import Folder, Image from cardpicker.sources.update_database import bulk_sync_objects, update_database from cardpicker.tags import Tags @@ -506,6 +506,50 @@ def test_bulk_sync_objects_persists_expansion_hint_on_update(self, django_settin assert Card.objects.get(identifier="existing").expansion_hint == "mh3" + @freezegun.freeze_time(DEFAULT_DATE) + def test_bulk_sync_objects_does_not_revert_a_resolved_tag_vote_on_reindex(self, django_settings, elasticsearch): + """ + Regression test for the tag-vote reindex-durability hazard identified when designing + artist/tag voting: `bulk_sync_objects` computes each incoming card's `tags` purely from + fresh filename extraction and writes it straight to Postgres + Elasticsearch. Without a + merge step, a scheduled re-scan would silently revert a consensus-resolved tag-vote + correction back to whatever the filename currently says. A resolved `CardTagVote` for a + tag the filename *doesn't* mention must survive a re-scan whose incoming tags are empty. + """ + source = factories.SourceFactory() + card = factories.CardFactory( + identifier="existing", + searchq="mountain", + date_created=make_aware(DEFAULT_DATE), + date_modified=make_aware(DEFAULT_DATE), + source=source, + tags=[], + ) + tag = factories.TagFactory(name="Borderless") + factories.CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.APPLY, source="admin") + management.call_command("search_index", "--rebuild", "-f") + + # a re-scan whose freshly-extracted tags are empty (the filename mentions nothing) - + # date_modified is bumped so the pre-existing change-detection condition alone would + # otherwise recognise this as an update and blindly overwrite `tags` with `[]`. + bulk_sync_objects( + source=source, + cards=[ + Card( + identifier="existing", + searchq="mountain", + date_created=make_aware(DEFAULT_DATE), + date_modified=make_aware(DEFAULT_DATE) + dt.timedelta(days=1), + source=source, + tags=[], + size=0, + image_hash=0, + ) + ], + ) + + assert Card.objects.get(identifier="existing").tags == ["Borderless"] + @pytest.mark.parametrize( "canonical_cards, new_card, expected_expansion, expected_collector_number", [ diff --git a/MPCAutofill/cardpicker/tests/test_tag_votes.py b/MPCAutofill/cardpicker/tests/test_tag_votes.py new file mode 100644 index 000000000..7ebddb892 --- /dev/null +++ b/MPCAutofill/cardpicker/tests/test_tag_votes.py @@ -0,0 +1,277 @@ +import pytest + +from django.core.cache import cache +from django.urls import reverse + +from cardpicker import views +from cardpicker.models import CardTagVote, VotePolarity, VoteSource +from cardpicker.tag_consensus import ( + get_resolved_tag_overlay, + get_tag_vote_tally, + resolve_and_persist_tag_votes, + resolve_tag, +) +from cardpicker.tests.factories import ( + CanonicalArtistFactory, + CanonicalCardFactory, + CanonicalExpansionFactory, + CardFactory, + CardTagVoteFactory, + SourceFactory, + TagFactory, +) + +# see test_printing_consensus.py for why this capture-and-restore fixture exists +_SHARED_FACTORIES = [ + CardFactory, + SourceFactory, + CanonicalArtistFactory, + CanonicalExpansionFactory, + CanonicalCardFactory, +] + + +@pytest.fixture(autouse=True) +def _preserve_shared_factory_sequences(): + before = {f: f._meta.next_sequence() for f in _SHARED_FACTORIES} + for f, n in before.items(): + f.reset_sequence(n, force=True) + yield + for f, n in before.items(): + f.reset_sequence(n, force=True) + + +@pytest.fixture(autouse=True) +def _clear_rate_limit_cache(): + cache.clear() + yield + cache.clear() + + +class TestResolveTag: + def test_no_votes_returns_none(self, db): + card = CardFactory() + tag = TagFactory() + assert resolve_tag(card, tag) is None + + def test_apply_consensus(self, db): + card = CardFactory() + tag = TagFactory() + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.APPLY, source=VoteSource.USER) + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.APPLY, source=VoteSource.USER) + assert resolve_tag(card, tag) == VotePolarity.APPLY + + def test_not_applicable_consensus(self, db): + card = CardFactory() + tag = TagFactory() + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.NOT_APPLICABLE, source=VoteSource.ADMIN) + assert resolve_tag(card, tag) == VotePolarity.NOT_APPLICABLE + + def test_votes_on_a_different_tag_are_not_counted(self, db): + card = CardFactory() + tag_a = TagFactory() + tag_b = TagFactory() + CardTagVoteFactory(card=card, tag=tag_a, polarity=VotePolarity.APPLY, source=VoteSource.ADMIN) + assert resolve_tag(card, tag_b) is None + + +class TestResolveAndPersistTagVotes: + def test_applies_a_resolved_apply_vote(self, db): + card = CardFactory(tags=[]) + tag = TagFactory(name="Borderless") + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.APPLY, source=VoteSource.ADMIN) + + resolve_and_persist_tag_votes(card) + + card.refresh_from_db() + assert card.tags == ["Borderless"] + + def test_removes_a_resolved_not_applicable_vote(self, db): + card = CardFactory(tags=["Borderless"]) + tag = TagFactory(name="Borderless") + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.NOT_APPLICABLE, source=VoteSource.ADMIN) + + resolve_and_persist_tag_votes(card) + + card.refresh_from_db() + assert card.tags == [] + + def test_unresolved_tag_does_not_change_tags(self, db): + card = CardFactory(tags=["Existing"]) + tag = TagFactory(name="Contested") + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.APPLY, source=VoteSource.USER) + + resolve_and_persist_tag_votes(card) + + card.refresh_from_db() + assert card.tags == ["Existing"] + + def test_multiple_tags_resolve_independently_on_the_same_card(self, db): + card = CardFactory(tags=[]) + apply_tag = TagFactory(name="Apply Me") + reject_tag = TagFactory(name="Reject Me") + CardTagVoteFactory(card=card, tag=apply_tag, polarity=VotePolarity.APPLY, source=VoteSource.ADMIN) + CardTagVoteFactory(card=card, tag=reject_tag, polarity=VotePolarity.NOT_APPLICABLE, source=VoteSource.ADMIN) + + resolve_and_persist_tag_votes(card) + + card.refresh_from_db() + assert card.tags == ["Apply Me"] + + +class TestGetTagVoteTally: + def test_tally_groups_by_polarity(self, db): + card = CardFactory() + tag = TagFactory() + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.APPLY) + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.APPLY) + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.NOT_APPLICABLE) + + tally = get_tag_vote_tally(card, tag) + + assert {(entry["polarity"], entry["count"]) for entry in tally} == {(1, 2), (-1, 1)} + + +class TestGetResolvedTagOverlay: + def test_batches_across_multiple_cards_and_tags(self, db): + card_a = CardFactory() + card_b = CardFactory() + tag_a = TagFactory(name="Tag A") + tag_b = TagFactory(name="Tag B") + CardTagVoteFactory(card=card_a, tag=tag_a, polarity=VotePolarity.APPLY, source=VoteSource.ADMIN) + CardTagVoteFactory(card=card_b, tag=tag_b, polarity=VotePolarity.NOT_APPLICABLE, source=VoteSource.ADMIN) + + overlay = get_resolved_tag_overlay([card_a.pk, card_b.pk]) + + assert overlay == {card_a.pk: {"Tag A": 1}, card_b.pk: {"Tag B": -1}} + + def test_unresolved_pairs_are_omitted(self, db): + card = CardFactory() + tag = TagFactory() + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.APPLY, source=VoteSource.USER) + + overlay = get_resolved_tag_overlay([card.pk]) + + assert overlay == {} + + +class TestPostTagConsensus: + def test_unknown_card_identifier_is_a_bad_request(self, client, django_settings): + response = client.post( + reverse(views.post_tag_consensus), + {"identifier": "does-not-exist"}, + content_type="application/json", + ) + assert response.status_code == 400 + + def test_returns_an_entry_for_every_seeded_tag(self, client, django_settings): + card = CardFactory() + TagFactory(name="Tag A") + TagFactory(name="Tag B") + + response = client.post( + reverse(views.post_tag_consensus), + {"identifier": card.identifier}, + content_type="application/json", + ) + + body = response.json() + assert {entry["tagName"] for entry in body["tags"]} == {"Tag A", "Tag B"} + assert all(entry["resolvedPolarity"] is None for entry in body["tags"]) + + +class TestPostSubmitTagVote: + def test_unknown_card_identifier_is_a_bad_request(self, client, django_settings): + response = client.post( + reverse(views.post_submit_tag_vote), + {"identifier": "does-not-exist", "tagName": "x", "polarity": 1, "anonymousId": "anon-1"}, + content_type="application/json", + ) + assert response.status_code == 400 + + def test_unknown_tag_name_is_a_bad_request(self, client, django_settings): + card = CardFactory() + response = client.post( + reverse(views.post_submit_tag_vote), + {"identifier": card.identifier, "tagName": "does-not-exist", "polarity": 1, "anonymousId": "anon-1"}, + content_type="application/json", + ) + assert response.status_code == 400 + + def test_invalid_polarity_is_a_bad_request(self, client, django_settings): + card = CardFactory() + tag = TagFactory() + response = client.post( + reverse(views.post_submit_tag_vote), + {"identifier": card.identifier, "tagName": tag.name, "polarity": 99, "anonymousId": "anon-1"}, + content_type="application/json", + ) + assert response.status_code == 400 + + def test_creates_a_vote_and_persists_consensus(self, client, django_settings, settings): + settings.PRINTING_TAG_MIN_VOTES = 1 + settings.PRINTING_TAG_MIN_SHARE = 0.5 + card = CardFactory(tags=[]) + tag = TagFactory(name="Borderless") + + response = client.post( + reverse(views.post_submit_tag_vote), + {"identifier": card.identifier, "tagName": tag.name, "polarity": 1, "anonymousId": "anon-1"}, + content_type="application/json", + ) + + assert response.status_code == 200 + assert response.json()["resolvedPolarity"] == 1 + card.refresh_from_db() + assert card.tags == ["Borderless"] + assert CardTagVote.objects.filter(card=card, tag=tag, anonymous_id="anon-1").count() == 1 + + def test_changing_your_mind_updates_the_same_row_rather_than_adding_another(self, client, django_settings): + card = CardFactory() + tag = TagFactory() + + for polarity in (1, -1): + client.post( + reverse(views.post_submit_tag_vote), + {"identifier": card.identifier, "tagName": tag.name, "polarity": polarity, "anonymousId": "anon-1"}, + content_type="application/json", + ) + + votes = CardTagVote.objects.filter(card=card, tag=tag, anonymous_id="anon-1") + assert votes.count() == 1 + assert votes.get().polarity == -1 + + def test_a_vote_on_one_tag_does_not_clear_a_vote_on_another_tag_by_the_same_person(self, client, django_settings): + card = CardFactory() + tag_a = TagFactory() + tag_b = TagFactory() + + client.post( + reverse(views.post_submit_tag_vote), + {"identifier": card.identifier, "tagName": tag_a.name, "polarity": 1, "anonymousId": "anon-1"}, + content_type="application/json", + ) + client.post( + reverse(views.post_submit_tag_vote), + {"identifier": card.identifier, "tagName": tag_b.name, "polarity": 1, "anonymousId": "anon-1"}, + content_type="application/json", + ) + + assert CardTagVote.objects.filter(card=card, anonymous_id="anon-1").count() == 2 + + def test_rate_limited_after_exceeding_the_configured_rate(self, client, django_settings, settings): + settings.PRINTING_TAG_SUBMISSION_RATE = "1/m" + card = CardFactory() + tag = TagFactory() + body = { + "identifier": card.identifier, + "tagName": tag.name, + "polarity": 1, + "anonymousId": "anon-rate-limited", + } + + first = client.post(reverse(views.post_submit_tag_vote), body, content_type="application/json") + second = client.post(reverse(views.post_submit_tag_vote), body, content_type="application/json") + + assert first.status_code == 200 + assert second.status_code == 429 diff --git a/MPCAutofill/cardpicker/tests/test_vote_consensus.py b/MPCAutofill/cardpicker/tests/test_vote_consensus.py new file mode 100644 index 000000000..b39d1a2d6 --- /dev/null +++ b/MPCAutofill/cardpicker/tests/test_vote_consensus.py @@ -0,0 +1,56 @@ +from cardpicker.vote_consensus import VoteTuple, resolve_weighted_consensus + + +class TestResolveWeightedConsensus: + def test_no_votes_returns_none(self): + assert resolve_weighted_consensus([], min_weight=2, min_share=0.6) is None + + def test_single_group_clears_thresholds(self): + votes = [ + VoteTuple(outcome_key="a", weight=1.0, is_ai=False), + VoteTuple(outcome_key="a", weight=1.0, is_ai=False), + ] + assert resolve_weighted_consensus(votes, min_weight=2, min_share=0.6) == "a" + + def test_below_min_weight_returns_none(self): + votes = [VoteTuple(outcome_key="a", weight=1.0, is_ai=False)] + assert resolve_weighted_consensus(votes, min_weight=2, min_share=0.6) is None + + def test_tie_below_min_share_returns_none(self): + # two outcomes with equal weight: share is exactly 0.5, below a 0.6 threshold + votes = [ + VoteTuple(outcome_key="a", weight=1.0, is_ai=False), + VoteTuple(outcome_key="a", weight=1.0, is_ai=False), + VoteTuple(outcome_key="b", weight=1.0, is_ai=False), + VoteTuple(outcome_key="b", weight=1.0, is_ai=False), + ] + assert resolve_weighted_consensus(votes, min_weight=2, min_share=0.6) is None + + def test_admin_style_weight_override(self): + # one high-weight vote (e.g. an "admin") outweighs two conflicting low-weight votes + votes = [ + VoteTuple(outcome_key="a", weight=5.0, is_ai=False), + VoteTuple(outcome_key="b", weight=1.0, is_ai=False), + VoteTuple(outcome_key="b", weight=1.0, is_ai=False), + ] + assert resolve_weighted_consensus(votes, min_weight=2, min_share=0.6) == "a" + + def test_ai_only_votes_never_resolve_even_with_large_weight(self): + votes = [VoteTuple(outcome_key="a", weight=10.0, is_ai=True) for _ in range(5)] + assert resolve_weighted_consensus(votes, min_weight=2, min_share=0.6) is None + + def test_mixed_ai_and_non_ai_can_resolve(self): + votes = [ + VoteTuple(outcome_key="a", weight=0.5, is_ai=True), + VoteTuple(outcome_key="a", weight=2.0, is_ai=False), + ] + assert resolve_weighted_consensus(votes, min_weight=2, min_share=0.6) == "a" + + def test_three_way_split_leader_below_share_returns_none(self): + votes = [ + VoteTuple(outcome_key="a", weight=1.0, is_ai=False), + VoteTuple(outcome_key="a", weight=1.0, is_ai=False), + VoteTuple(outcome_key="b", weight=1.0, is_ai=False), + VoteTuple(outcome_key="c", weight=1.0, is_ai=False), + ] + assert resolve_weighted_consensus(votes, min_weight=2, min_share=0.6) is None diff --git a/MPCAutofill/cardpicker/urls.py b/MPCAutofill/cardpicker/urls.py index 4cc54c016..afa9dc42e 100755 --- a/MPCAutofill/cardpicker/urls.py +++ b/MPCAutofill/cardpicker/urls.py @@ -26,4 +26,9 @@ path("2/printingConsensus/", views.post_printing_consensus), path("2/submitPrintingTag/", views.post_submit_printing_tag), path("2/printingTagQueue/", views.get_printing_tag_queue), + path("2/artistCandidates/", views.post_artist_candidates), + path("2/artistConsensus/", views.post_artist_consensus), + path("2/submitArtistVote/", views.post_submit_artist_vote), + path("2/tagConsensus/", views.post_tag_consensus), + path("2/submitTagVote/", views.post_submit_tag_vote), ] diff --git a/MPCAutofill/cardpicker/views.py b/MPCAutofill/cardpicker/views.py index 5321cc25a..ef5254636 100644 --- a/MPCAutofill/cardpicker/views.py +++ b/MPCAutofill/cardpicker/views.py @@ -7,6 +7,7 @@ from random import sample from typing import Any, Callable, Literal, TypeVar, Union, cast +import Levenshtein import pycountry from django_ratelimit.decorators import ratelimit from elasticsearch_dsl.index import Index @@ -25,6 +26,12 @@ ) from django.views.decorators.csrf import csrf_exempt +from cardpicker.artist_consensus import UNKNOWN as ARTIST_UNKNOWN +from cardpicker.artist_consensus import ( + get_artist_vote_tally, + resolve_and_persist_artist, + resolve_artist, +) from cardpicker.constants import ( CARDS_PAGE_SIZE, DEFAULT_LANGUAGE, @@ -37,17 +44,26 @@ from cardpicker.integrations.integrations import get_configured_game_integration from cardpicker.integrations.patreon import get_patreon_campaign_details, get_patrons from cardpicker.models import ( + CanonicalArtist, CanonicalCard, Card, + CardArtistVote, CardPrintingTag, - CardPrintingTagSource, + CardTagVote, CardTypes, DFCPair, PrintingTagStatus, Source, + Tag, + VotePolarity, + VoteSource, summarise_contributions, ) -from cardpicker.printing_candidates import get_ranked_printing_candidates +from cardpicker.printing_candidates import ( + CANDIDATE_QUERY_LIMIT, + CANDIDATE_RESULT_LIMIT, + get_ranked_printing_candidates, +) from cardpicker.printing_consensus import ( NO_MATCH, get_contested_card_ids, @@ -55,7 +71,15 @@ resolve_and_persist_printing, resolve_printing, ) -from cardpicker.schema_types import CardbacksRequest, CardbacksResponse +from cardpicker.schema_types import ( + ArtistCandidatesRequest, + ArtistCandidatesResponse, + ArtistConsensusRequest, + ArtistConsensusResponse, + ArtistVoteTallyEntry, + CardbacksRequest, + CardbacksResponse, +) from cardpicker.schema_types import Cards as SampleCards from cardpicker.schema_types import ( CardsRequest, @@ -91,10 +115,17 @@ SearchEngineHealthResponse, SortBy, SourcesResponse, + SubmitArtistVoteRequest, SubmitPrintingTagRequest, + SubmitTagVoteRequest, + TagConsensusEntry, + TagConsensusRequest, + TagConsensusResponse, TagsResponse, + TagVoteTallyEntry, VoteTallyEntry, ) +from cardpicker.search.sanitisation import to_searchable from cardpicker.search.search_functions import ( SearchExceptions, get_new_cards_paginator, @@ -105,6 +136,11 @@ ) from cardpicker.sources.api import PathTraversalError, resolve_within_root from cardpicker.sources.source_types import SourceTypeChoices +from cardpicker.tag_consensus import ( + get_tag_vote_tally, + resolve_and_persist_tag_votes, + resolve_tag, +) from cardpicker.tags import Tags logger = logging.getLogger(__name__) @@ -805,8 +841,202 @@ def post_submit_printing_tag(request: HttpRequest) -> HttpResponse: printing=printing, is_no_match=req.isNoMatch, anonymous_id=req.anonymousId, - source=CardPrintingTagSource.USER, + source=VoteSource.USER, ) resolved = resolve_and_persist_printing(card) return JsonResponse(_build_printing_consensus_response(card, resolved).model_dump()) + + +def _build_artist_consensus_response( + card: Card, resolved: CanonicalArtist | Literal["UNKNOWN"] | None +) -> ArtistConsensusResponse: + return ArtistConsensusResponse( + resolvedArtist=resolved.serialise() if isinstance(resolved, CanonicalArtist) else None, + isUnknown=resolved == ARTIST_UNKNOWN, + voteTally=[ + ArtistVoteTallyEntry( + artist=entry["artist"].serialise() if entry["artist"] else None, + isUnknown=entry["is_unknown"], + count=entry["count"], + ) + for entry in get_artist_vote_tally(card) + ], + ) + + +@csrf_exempt +@ErrorWrappers.to_json +def post_artist_candidates(request: HttpRequest) -> HttpResponse: + """ + Return candidate artists for a card to be tagged against. Two modes: by default, ranks by + deduplicating the artists of `get_ranked_printing_candidates`'s own results (free ranking + signal, no separate query needed, since those printings are already ranked by relevance to + this card); if `query` is given, switches to a typeahead search over `CanonicalArtist.name` + for when the right artist isn't among those candidates. + """ + + if request.method != "POST": + raise BadRequestException("Expected POST request.") + + req = ArtistCandidatesRequest.model_validate(json.loads(request.body)) + card = _get_card_or_400(req.identifier) + + if req.query: + words = to_searchable(req.query).split() + artists_qs = CanonicalArtist.objects.all() + for word in words: + artists_qs = artists_qs.filter(name__icontains=word) + normalised_query = to_searchable(req.query) + artists = sorted( + artists_qs[:CANDIDATE_QUERY_LIMIT], + key=lambda artist: Levenshtein.ratio(normalised_query, to_searchable(artist.name)), + reverse=True, + )[:CANDIDATE_RESULT_LIMIT] + else: + seen_artist_ids: set[int] = set() + artists = [] + for printing in get_ranked_printing_candidates(card, None): + if printing.artist_id not in seen_artist_ids: + seen_artist_ids.add(printing.artist_id) + artists.append(printing.artist) + + return JsonResponse(ArtistCandidatesResponse(results=[artist.serialise() for artist in artists]).model_dump()) + + +@csrf_exempt +@ErrorWrappers.to_json +def post_artist_consensus(request: HttpRequest) -> HttpResponse: + """ + Return the currently resolved artist-vote consensus for a card, plus a plain vote-count + breakdown - mirrors `post_printing_consensus`. + """ + + if request.method != "POST": + raise BadRequestException("Expected POST request.") + + req = ArtistConsensusRequest.model_validate(json.loads(request.body)) + card = _get_card_or_400(req.identifier) + return JsonResponse(_build_artist_consensus_response(card, resolve_artist(card)).model_dump()) + + +@csrf_exempt +@ratelimit( # type: ignore # `django-ratelimit` does not implement decorator typing correctly + key=_printing_tag_rate_limit_key, rate=_printing_tag_rate_limit_rate, method="POST", block=False +) +@ErrorWrappers.to_json +def post_submit_artist_vote(request: HttpRequest) -> HttpResponse: + """ + Submit a vote that a card was illustrated by a specific artist (or definitively by an + unlisted/unknown artist). Same delete-then-create-then-recompute pattern as + `post_submit_printing_tag` - one artist opinion per (card, anonymous ID) pair at a time, + reusing the same rate-limit plumbing (it already reads `anonymousId` from the request body + generically, nothing printing-specific about it despite the name). + """ + + if request.method != "POST": + raise BadRequestException("Expected POST request.") + if getattr(request, "limited", False): + return JsonResponse( + ErrorResponse( + name="Rate limited", message="Too many artist vote submissions - please slow down." + ).model_dump(), + status=429, + ) + + req = SubmitArtistVoteRequest.model_validate(json.loads(request.body)) + card = _get_card_or_400(req.identifier) + + artist = None + if not req.isUnknown: + if not req.artistName: + raise BadRequestException("artistName is required unless isUnknown is set.") + try: + artist = CanonicalArtist.objects.get(name=req.artistName) + except CanonicalArtist.DoesNotExist: + raise BadRequestException(f"No artist found with name {req.artistName!r}.") + + with transaction.atomic(): + CardArtistVote.objects.filter(card=card, anonymous_id=req.anonymousId).delete() + CardArtistVote.objects.create( + card=card, + artist=artist, + is_unknown=req.isUnknown, + anonymous_id=req.anonymousId, + source=VoteSource.USER, + ) + resolved = resolve_and_persist_artist(card) + + return JsonResponse(_build_artist_consensus_response(card, resolved).model_dump()) + + +def _build_tag_consensus_entry(card: Card, tag: Tag) -> TagConsensusEntry: + return TagConsensusEntry( + tagName=tag.name, + resolvedPolarity=resolve_tag(card, tag), + tally=[ + TagVoteTallyEntry(polarity=entry["polarity"], count=entry["count"]) + for entry in get_tag_vote_tally(card, tag) + ], + ) + + +@csrf_exempt +@ErrorWrappers.to_json +def post_tag_consensus(request: HttpRequest) -> HttpResponse: + """ + Return the currently resolved tag-vote consensus for every seeded tag against a card, so a + voter can see and toggle every tag's state in one call rather than fetching per-tag. + """ + + if request.method != "POST": + raise BadRequestException("Expected POST request.") + + req = TagConsensusRequest.model_validate(json.loads(request.body)) + card = _get_card_or_400(req.identifier) + tags = Tag.objects.order_by("name") + return JsonResponse(TagConsensusResponse(tags=[_build_tag_consensus_entry(card, tag) for tag in tags]).model_dump()) + + +@csrf_exempt +@ratelimit( # type: ignore # `django-ratelimit` does not implement decorator typing correctly + key=_printing_tag_rate_limit_key, rate=_printing_tag_rate_limit_rate, method="POST", block=False +) +@ErrorWrappers.to_json +def post_submit_tag_vote(request: HttpRequest) -> HttpResponse: + """ + Submit a vote on whether a specific tag applies to a card. Unlike printing/artist votes, + this is `update_or_create` rather than delete-then-create: a card can carry independent, + simultaneous votes across many different tags at once, so submitting a vote on one tag + must not clear votes this same person has already cast on any other tag for this card. + """ + + if request.method != "POST": + raise BadRequestException("Expected POST request.") + if getattr(request, "limited", False): + return JsonResponse( + ErrorResponse( + name="Rate limited", message="Too many tag vote submissions - please slow down." + ).model_dump(), + status=429, + ) + + req = SubmitTagVoteRequest.model_validate(json.loads(request.body)) + card = _get_card_or_400(req.identifier) + try: + tag = Tag.objects.get(name=req.tagName) + except Tag.DoesNotExist: + raise BadRequestException(f"No tag found with name {req.tagName!r}.") + if req.polarity not in (VotePolarity.APPLY, VotePolarity.NOT_APPLICABLE): + raise BadRequestException(f"Invalid polarity {req.polarity!r} - must be 1 (apply) or -1 (not applicable).") + + with transaction.atomic(): + CardTagVote.objects.update_or_create( + card=card, + tag=tag, + anonymous_id=req.anonymousId, + defaults={"polarity": req.polarity, "source": VoteSource.USER}, + ) + resolve_and_persist_tag_votes(card) + + return JsonResponse(_build_tag_consensus_entry(card, tag).model_dump()) diff --git a/MPCAutofill/cardpicker/vote_consensus.py b/MPCAutofill/cardpicker/vote_consensus.py new file mode 100644 index 000000000..3f63d7195 --- /dev/null +++ b/MPCAutofill/cardpicker/vote_consensus.py @@ -0,0 +1,62 @@ +from collections import defaultdict +from typing import Hashable, Iterable, NamedTuple, TypedDict + + +class VoteTuple(NamedTuple): + """ + A single vote reduced to just what `resolve_weighted_consensus` needs to reconcile it: the + outcome it argues for (grouping key - e.g. a printing's pk, an artist's pk, or a tag's + polarity), its weight (already resolved from the vote's `source` by the caller), and + whether it came from an AI (which can never single-handedly clear consensus, regardless of + weight - see the non-AI gate below). + """ + + outcome_key: Hashable + weight: float + is_ai: bool + + +class _VoteGroup(TypedDict): + weight: float + has_non_ai: bool + + +def resolve_weighted_consensus(votes: Iterable[VoteTuple], min_weight: float, min_share: float) -> Hashable | None: + """ + Reconciles a set of weighted votes into a single resolved outcome key, or `None` if there + isn't yet enough signal to conclude anything (no votes, a tie, or a genuinely contested + set of votes). This is the shared core behind `cardpicker.printing_consensus.resolve_printing`, + `cardpicker.artist_consensus.resolve_artist`, and `cardpicker.tag_consensus.resolve_tag` - each + of those is a thin wrapper that builds `VoteTuple`s from its own vote model and calls this. + + Votes are grouped by `outcome_key`, and the highest-weighted group wins if, and only if, + ALL of the following hold: + - its summed weight is >= `min_weight` (compared against summed weight, not a raw row + count - a single admin vote, at a typical admin weight of 5, already clears a default + threshold of 2 on its own, which is what produces "admin override" behaviour from this + one unified formula, with no special-cased branch for admin votes); + - its share of the total weight across all groups is >= `min_share`; + - it contains at least one non-AI vote (a hard gate, independent of the weight math + above, so that no volume of AI-only votes can ever resolve consensus on their own). + """ + votes = list(votes) + if not votes: + return None + + groups: dict[Hashable, _VoteGroup] = defaultdict(lambda: _VoteGroup(weight=0.0, has_non_ai=False)) + for vote in votes: + group = groups[vote.outcome_key] + group["weight"] += vote.weight + if not vote.is_ai: + group["has_non_ai"] = True + + total_weight = sum(group["weight"] for group in groups.values()) + winning_key, winner = max(groups.items(), key=lambda item: item[1]["weight"]) + share = winner["weight"] / total_weight + + if winner["weight"] >= min_weight and share >= min_share and winner["has_non_ai"]: + return winning_key + return None + + +__all__ = ["VoteTuple", "resolve_weighted_consensus"] diff --git a/frontend/src/common/schema_types.ts b/frontend/src/common/schema_types.ts index 52bae57d8..a04d00009 100644 --- a/frontend/src/common/schema_types.ts +++ b/frontend/src/common/schema_types.ts @@ -2,8 +2,9 @@ // To parse this data: // -// import { Convert, Campaign, CanonicalArtist, CanonicalCard, Card, CardType, FilterSettings, Game, ImportSite, Language, NewCardsFirstPage, PrintingCandidate, SearchQuery, SearchSettings, SearchTypeSettings, SortBy, Source, SourceContribution, SourceSettings, SourceType, Supporter, SupporterTier, Tag, VoteTallyEntry, CardbacksRequest, CardbacksResponse, CardsRequest, CardsResponse, ContributionsResponse, DFCPairsResponse, EditorSearchRequest, EditorSearchResponse, ErrorResponse, ExploreSearchRequest, ExploreSearchResponse, ImportSiteDecklistRequest, ImportSiteDecklistResponse, ImportSitesResponse, InfoResponse, LanguagesResponse, NewCardsFirstPagesResponse, NewCardsPageResponse, OldEditorSearchRequest, OldEditorSearchResponse, PatreonResponse, PrintingCandidatesRequest, PrintingCandidatesResponse, PrintingConsensusRequest, PrintingConsensusResponse, PrintingTagQueueResponse, SampleCardsResponse, SearchEngineHealthResponse, SourcesResponse, SubmitPrintingTagRequest, TagsResponse } from "./file"; +// import { Convert, ArtistVoteTallyEntry, Campaign, CanonicalArtist, CanonicalCard, Card, CardType, FilterSettings, Game, ImportSite, Language, NewCardsFirstPage, PrintingCandidate, SearchQuery, SearchSettings, SearchTypeSettings, SortBy, Source, SourceContribution, SourceSettings, SourceType, Supporter, SupporterTier, Tag, TagConsensusEntry, TagVoteTallyEntry, VoteTallyEntry, ArtistCandidatesRequest, ArtistCandidatesResponse, ArtistConsensusRequest, ArtistConsensusResponse, CardbacksRequest, CardbacksResponse, CardsRequest, CardsResponse, ContributionsResponse, DFCPairsResponse, EditorSearchRequest, EditorSearchResponse, ErrorResponse, ExploreSearchRequest, ExploreSearchResponse, ImportSiteDecklistRequest, ImportSiteDecklistResponse, ImportSitesResponse, InfoResponse, LanguagesResponse, NewCardsFirstPagesResponse, NewCardsPageResponse, OldEditorSearchRequest, OldEditorSearchResponse, PatreonResponse, PrintingCandidatesRequest, PrintingCandidatesResponse, PrintingConsensusRequest, PrintingConsensusResponse, PrintingTagQueueResponse, SampleCardsResponse, SearchEngineHealthResponse, SourcesResponse, SubmitArtistVoteRequest, SubmitPrintingTagRequest, SubmitTagVoteRequest, TagConsensusRequest, TagConsensusResponse, TagsResponse } from "./file"; // +// const artistVoteTallyEntry = Convert.toArtistVoteTallyEntry(json); // const campaign = Convert.toCampaign(json); // const canonicalArtist = Convert.toCanonicalArtist(json); // const canonicalCard = Convert.toCanonicalCard(json); @@ -27,7 +28,13 @@ // const supporter = Convert.toSupporter(json); // const supporterTier = Convert.toSupporterTier(json); // const tag = Convert.toTag(json); +// const tagConsensusEntry = Convert.toTagConsensusEntry(json); +// const tagVoteTallyEntry = Convert.toTagVoteTallyEntry(json); // const voteTallyEntry = Convert.toVoteTallyEntry(json); +// const artistCandidatesRequest = Convert.toArtistCandidatesRequest(json); +// const artistCandidatesResponse = Convert.toArtistCandidatesResponse(json); +// const artistConsensusRequest = Convert.toArtistConsensusRequest(json); +// const artistConsensusResponse = Convert.toArtistConsensusResponse(json); // const cardbacksRequest = Convert.toCardbacksRequest(json); // const cardbacksResponse = Convert.toCardbacksResponse(json); // const cardsRequest = Convert.toCardsRequest(json); @@ -57,7 +64,11 @@ // const sampleCardsResponse = Convert.toSampleCardsResponse(json); // const searchEngineHealthResponse = Convert.toSearchEngineHealthResponse(json); // const sourcesResponse = Convert.toSourcesResponse(json); +// const submitArtistVoteRequest = Convert.toSubmitArtistVoteRequest(json); // const submitPrintingTagRequest = Convert.toSubmitPrintingTagRequest(json); +// const submitTagVoteRequest = Convert.toSubmitTagVoteRequest(json); +// const tagConsensusRequest = Convert.toTagConsensusRequest(json); +// const tagConsensusResponse = Convert.toTagConsensusResponse(json); // const tagsResponse = Convert.toTagsResponse(json); // // These functions will throw an error if the JSON doesn't @@ -67,6 +78,35 @@ export enum Game { Mtg = "MTG", } +export interface ArtistCandidatesRequest { + identifier: string; + query?: null | string; +} + +export interface ArtistCandidatesResponse { + results: Array; +} + +export interface CanonicalArtist { + name: string; +} + +export interface ArtistConsensusRequest { + identifier: string; +} + +export interface ArtistConsensusResponse { + isUnknown: boolean; + resolvedArtist?: CanonicalArtist | null; + voteTally: ArtistVoteTallyEntry[]; +} + +export interface ArtistVoteTallyEntry { + artist?: CanonicalArtist | null; + count: number; + isUnknown: boolean; +} + export interface CardbacksRequest { searchSettings: SearchSettings; } @@ -165,10 +205,6 @@ export interface Card { tags: string[]; } -export interface CanonicalArtist { - name: string; -} - export interface CanonicalCard { artist?: string; canonicalId?: string; @@ -427,6 +463,13 @@ export interface SourcesResponse { results: { [key: string]: Source }; } +export interface SubmitArtistVoteRequest { + anonymousId: string; + artistName?: null | string; + identifier: string; + isUnknown: boolean; +} + export interface SubmitPrintingTagRequest { anonymousId: string; identifier: string; @@ -434,6 +477,32 @@ export interface SubmitPrintingTagRequest { printingIdentifier?: null | string; } +export interface SubmitTagVoteRequest { + anonymousId: string; + identifier: string; + polarity: number; + tagName: string; +} + +export interface TagConsensusRequest { + identifier: string; +} + +export interface TagConsensusResponse { + tags: TagConsensusEntry[]; +} + +export interface TagConsensusEntry { + resolvedPolarity?: number | null; + tagName: string; + tally: TagVoteTallyEntry[]; +} + +export interface TagVoteTallyEntry { + count: number; + polarity: number; +} + export interface TagsResponse { tags: Tag[]; } @@ -457,6 +526,16 @@ export interface ChildElement { // Converts JSON strings to/from your types // and asserts the results of JSON.parse at runtime export class Convert { + public static toArtistVoteTallyEntry(json: string): ArtistVoteTallyEntry { + return cast(JSON.parse(json), r("ArtistVoteTallyEntry")); + } + + public static artistVoteTallyEntryToJson( + value: ArtistVoteTallyEntry + ): string { + return JSON.stringify(uncast(value, r("ArtistVoteTallyEntry")), null, 2); + } + public static toCampaign(json: string): Campaign | null { return cast(JSON.parse(json), u(r("Campaign"), null)); } @@ -645,6 +724,22 @@ export class Convert { return JSON.stringify(uncast(value, r("Tag")), null, 2); } + public static toTagConsensusEntry(json: string): TagConsensusEntry { + return cast(JSON.parse(json), r("TagConsensusEntry")); + } + + public static tagConsensusEntryToJson(value: TagConsensusEntry): string { + return JSON.stringify(uncast(value, r("TagConsensusEntry")), null, 2); + } + + public static toTagVoteTallyEntry(json: string): TagVoteTallyEntry { + return cast(JSON.parse(json), r("TagVoteTallyEntry")); + } + + public static tagVoteTallyEntryToJson(value: TagVoteTallyEntry): string { + return JSON.stringify(uncast(value, r("TagVoteTallyEntry")), null, 2); + } + public static toVoteTallyEntry(json: string): VoteTallyEntry { return cast(JSON.parse(json), r("VoteTallyEntry")); } @@ -653,6 +748,56 @@ export class Convert { return JSON.stringify(uncast(value, r("VoteTallyEntry")), null, 2); } + public static toArtistCandidatesRequest( + json: string + ): ArtistCandidatesRequest { + return cast(JSON.parse(json), r("ArtistCandidatesRequest")); + } + + public static artistCandidatesRequestToJson( + value: ArtistCandidatesRequest + ): string { + return JSON.stringify(uncast(value, r("ArtistCandidatesRequest")), null, 2); + } + + public static toArtistCandidatesResponse( + json: string + ): ArtistCandidatesResponse { + return cast(JSON.parse(json), r("ArtistCandidatesResponse")); + } + + public static artistCandidatesResponseToJson( + value: ArtistCandidatesResponse + ): string { + return JSON.stringify( + uncast(value, r("ArtistCandidatesResponse")), + null, + 2 + ); + } + + public static toArtistConsensusRequest(json: string): ArtistConsensusRequest { + return cast(JSON.parse(json), r("ArtistConsensusRequest")); + } + + public static artistConsensusRequestToJson( + value: ArtistConsensusRequest + ): string { + return JSON.stringify(uncast(value, r("ArtistConsensusRequest")), null, 2); + } + + public static toArtistConsensusResponse( + json: string + ): ArtistConsensusResponse { + return cast(JSON.parse(json), r("ArtistConsensusResponse")); + } + + public static artistConsensusResponseToJson( + value: ArtistConsensusResponse + ): string { + return JSON.stringify(uncast(value, r("ArtistConsensusResponse")), null, 2); + } + public static toCardbacksRequest(json: string): CardbacksRequest { return cast(JSON.parse(json), r("CardbacksRequest")); } @@ -973,6 +1118,18 @@ export class Convert { return JSON.stringify(uncast(value, r("SourcesResponse")), null, 2); } + public static toSubmitArtistVoteRequest( + json: string + ): SubmitArtistVoteRequest { + return cast(JSON.parse(json), r("SubmitArtistVoteRequest")); + } + + public static submitArtistVoteRequestToJson( + value: SubmitArtistVoteRequest + ): string { + return JSON.stringify(uncast(value, r("SubmitArtistVoteRequest")), null, 2); + } + public static toSubmitPrintingTagRequest( json: string ): SubmitPrintingTagRequest { @@ -989,6 +1146,34 @@ export class Convert { ); } + public static toSubmitTagVoteRequest(json: string): SubmitTagVoteRequest { + return cast(JSON.parse(json), r("SubmitTagVoteRequest")); + } + + public static submitTagVoteRequestToJson( + value: SubmitTagVoteRequest + ): string { + return JSON.stringify(uncast(value, r("SubmitTagVoteRequest")), null, 2); + } + + public static toTagConsensusRequest(json: string): TagConsensusRequest { + return cast(JSON.parse(json), r("TagConsensusRequest")); + } + + public static tagConsensusRequestToJson(value: TagConsensusRequest): string { + return JSON.stringify(uncast(value, r("TagConsensusRequest")), null, 2); + } + + public static toTagConsensusResponse(json: string): TagConsensusResponse { + return cast(JSON.parse(json), r("TagConsensusResponse")); + } + + public static tagConsensusResponseToJson( + value: TagConsensusResponse + ): string { + return JSON.stringify(uncast(value, r("TagConsensusResponse")), null, 2); + } + public static toTagsResponse(json: string): TagsResponse { return cast(JSON.parse(json), r("TagsResponse")); } @@ -1181,6 +1366,46 @@ function r(name: string) { } const typeMap: any = { + ArtistCandidatesRequest: o( + [ + { json: "identifier", js: "identifier", typ: "" }, + { json: "query", js: "query", typ: u(undefined, u(null, "")) }, + ], + false + ), + ArtistCandidatesResponse: o( + [{ json: "results", js: "results", typ: a(u(r("CanonicalArtist"), null)) }], + false + ), + CanonicalArtist: o([{ json: "name", js: "name", typ: "" }], false), + ArtistConsensusRequest: o( + [{ json: "identifier", js: "identifier", typ: "" }], + false + ), + ArtistConsensusResponse: o( + [ + { json: "isUnknown", js: "isUnknown", typ: true }, + { + json: "resolvedArtist", + js: "resolvedArtist", + typ: u(undefined, u(r("CanonicalArtist"), null)), + }, + { json: "voteTally", js: "voteTally", typ: a(r("ArtistVoteTallyEntry")) }, + ], + false + ), + ArtistVoteTallyEntry: o( + [ + { + json: "artist", + js: "artist", + typ: u(undefined, u(r("CanonicalArtist"), null)), + }, + { json: "count", js: "count", typ: 0 }, + { json: "isUnknown", js: "isUnknown", typ: true }, + ], + false + ), CardbacksRequest: o( [ { @@ -1288,7 +1513,6 @@ const typeMap: any = { ], false ), - CanonicalArtist: o([{ json: "name", js: "name", typ: "" }], false), CanonicalCard: o( [ { json: "artist", js: "artist", typ: u(undefined, "") }, @@ -1583,6 +1807,15 @@ const typeMap: any = { [{ json: "results", js: "results", typ: m(r("Source")) }], false ), + SubmitArtistVoteRequest: o( + [ + { json: "anonymousId", js: "anonymousId", typ: "" }, + { json: "artistName", js: "artistName", typ: u(undefined, u(null, "")) }, + { json: "identifier", js: "identifier", typ: "" }, + { json: "isUnknown", js: "isUnknown", typ: true }, + ], + false + ), SubmitPrintingTagRequest: o( [ { json: "anonymousId", js: "anonymousId", typ: "" }, @@ -1596,6 +1829,42 @@ const typeMap: any = { ], false ), + SubmitTagVoteRequest: o( + [ + { json: "anonymousId", js: "anonymousId", typ: "" }, + { json: "identifier", js: "identifier", typ: "" }, + { json: "polarity", js: "polarity", typ: 0 }, + { json: "tagName", js: "tagName", typ: "" }, + ], + false + ), + TagConsensusRequest: o( + [{ json: "identifier", js: "identifier", typ: "" }], + false + ), + TagConsensusResponse: o( + [{ json: "tags", js: "tags", typ: a(r("TagConsensusEntry")) }], + false + ), + TagConsensusEntry: o( + [ + { + json: "resolvedPolarity", + js: "resolvedPolarity", + typ: u(undefined, u(0, null)), + }, + { json: "tagName", js: "tagName", typ: "" }, + { json: "tally", js: "tally", typ: a(r("TagVoteTallyEntry")) }, + ], + false + ), + TagVoteTallyEntry: o( + [ + { json: "count", js: "count", typ: 0 }, + { json: "polarity", js: "polarity", typ: 0 }, + ], + false + ), TagsResponse: o([{ json: "tags", js: "tags", typ: a(r("Tag")) }], false), Tag: o( [ diff --git a/frontend/src/common/test-constants.ts b/frontend/src/common/test-constants.ts index cfc21115a..7e270827a 100644 --- a/frontend/src/common/test-constants.ts +++ b/frontend/src/common/test-constants.ts @@ -4,6 +4,7 @@ import { Card, MaximumDPI, MaximumSize, MinimumDPI } from "@/common/constants"; import { + CanonicalArtist, CardType as CardTypeSchema, PrintingCandidate, SourceType, @@ -489,4 +490,7 @@ export const printingCandidate2: PrintingCandidate = { releasedAt: "2010-06-15", }; +export const canonicalArtist1: CanonicalArtist = { name: "Some Artist" }; +export const canonicalArtist2: CanonicalArtist = { name: "Another Artist" }; + //# endregion diff --git a/frontend/src/features/attributeVoting/ArtistVotePicker.tsx b/frontend/src/features/attributeVoting/ArtistVotePicker.tsx new file mode 100644 index 000000000..dc83d3584 --- /dev/null +++ b/frontend/src/features/attributeVoting/ArtistVotePicker.tsx @@ -0,0 +1,164 @@ +/** + * Lets a user tag which artist illustrated a card, or mark it as an unlisted/unknown artist. + * Shown as part of AttributeVotingPanel, once a card's printing-tag consensus hasn't resolved + * a printing (see that component for the trigger condition) - mirrors PrintingTagPicker.tsx's + * fetch/submit structure, but candidates here are plain named chips (CanonicalArtist has no + * thumbnail image) rather than thumbnail buttons. + */ + +import React, { useEffect, useState } from "react"; +import Button from "react-bootstrap/Button"; +import Col from "react-bootstrap/Col"; +import Form from "react-bootstrap/Form"; +import Row from "react-bootstrap/Row"; + +import { getOrCreateAnonymousId } from "@/common/cookies"; +import { + ArtistConsensusResponse, + CanonicalArtist, +} from "@/common/schema_types"; +import { useAppDispatch } from "@/common/types"; +import { + APIGetArtistCandidates, + APIGetArtistConsensus, + APISubmitArtistVote, +} from "@/store/api"; +import { setNotification } from "@/store/slices/toastsSlice"; + +interface ArtistVotePickerProps { + backendURL: string; + cardIdentifier: string; +} + +export function ArtistVotePicker({ + backendURL, + cardIdentifier, +}: ArtistVotePickerProps) { + const dispatch = useAppDispatch(); + + const [query, setQuery] = useState(""); + const [candidates, setCandidates] = useState>([]); + const [consensus, setConsensus] = useState( + null + ); + const [loading, setLoading] = useState(false); + const [submitting, setSubmitting] = useState(false); + + useEffect(() => { + APIGetArtistConsensus(backendURL, cardIdentifier) + .then(setConsensus) + .catch(() => undefined); + }, [backendURL, cardIdentifier]); + + useEffect(() => { + setLoading(true); + APIGetArtistCandidates(backendURL, cardIdentifier, query || undefined) + .then((response) => + setCandidates( + response.results.filter( + (candidate): candidate is CanonicalArtist => candidate != null + ) + ) + ) + .catch(() => setCandidates([])) + .finally(() => setLoading(false)); + }, [backendURL, cardIdentifier, query]); + + const submit = (artistName: string | undefined, isUnknown: boolean) => { + setSubmitting(true); + APISubmitArtistVote( + backendURL, + cardIdentifier, + getOrCreateAnonymousId(), + artistName, + isUnknown + ) + .then((response) => { + setConsensus(response); + dispatch( + setNotification([ + Math.random().toString(), + { + name: "Vote submitted", + message: "Thanks for helping tag this card's artist!", + level: "info", + }, + ]) + ); + }) + .catch(() => + dispatch( + setNotification([ + Math.random().toString(), + { + name: "Vote failed", + message: + "Something went wrong submitting your vote - please try again.", + level: "error", + }, + ]) + ) + ) + .finally(() => setSubmitting(false)); + }; + + return ( +
+
+ {consensus == null && "Loading current consensus..."} + {consensus != null && consensus.resolvedArtist != null && ( + Current consensus: {consensus.resolvedArtist.name} + )} + {consensus != null && + consensus.resolvedArtist == null && + consensus.isUnknown && Current consensus: unknown artist} + {consensus != null && + consensus.resolvedArtist == null && + !consensus.isUnknown && ( + + Not yet resolved + {consensus.voteTally.length > 0 ? " - contested" : ""} + + )} +
+ setQuery(event.target.value)} + /> + {loading ? ( +
Loading candidates...
+ ) : ( + + + + + {candidates.map((candidate) => ( + + + + ))} + + )} +
+ ); +} diff --git a/frontend/src/features/attributeVoting/AttributeVotingPanel.tsx b/frontend/src/features/attributeVoting/AttributeVotingPanel.tsx new file mode 100644 index 000000000..6917d76a8 --- /dev/null +++ b/frontend/src/features/attributeVoting/AttributeVotingPanel.tsx @@ -0,0 +1,34 @@ +/** + * Follow-up voting panel shown once a card's printing-tag consensus hasn't resolved a + * printing (see the two call sites - CardDetailedViewModal.tsx and PrintingTagQueue.tsx - for + * the exact trigger condition, which is the same in both: `resolvedPrinting == null`, covering + * both an explicit "no match" vote and a plain not-yet-resolved card). One panel, two + * independently optional/skippable sections - artist and tags - rather than a modal chain. + */ + +import React from "react"; + +import { ArtistVotePicker } from "@/features/attributeVoting/ArtistVotePicker"; +import { TagVotePicker } from "@/features/attributeVoting/TagVotePicker"; + +interface AttributeVotingPanelProps { + backendURL: string; + cardIdentifier: string; +} + +export function AttributeVotingPanel({ + backendURL, + cardIdentifier, +}: AttributeVotingPanelProps) { + return ( +
+
Who's the artist?
+ +
Do any of these tags apply?
+ +
+ ); +} diff --git a/frontend/src/features/attributeVoting/TagVotePicker.tsx b/frontend/src/features/attributeVoting/TagVotePicker.tsx new file mode 100644 index 000000000..d855fc90f --- /dev/null +++ b/frontend/src/features/attributeVoting/TagVotePicker.tsx @@ -0,0 +1,112 @@ +/** + * Lets a user vote on whether each seeded descriptor Tag applies to a card - tri-state toggle + * chips: unvoted (no resolved consensus for this tag on this card yet), applied (resolved + * APPLY - clicking again votes NOT_APPLICABLE), or crossed-out (resolved NOT_APPLICABLE - + * clicking again votes APPLY). Unlike printing/artist voting, a card can carry independent, + * simultaneous votes across many different tags at once, so each chip submits its own vote + * immediately on click rather than requiring one shared "submit" action. + */ + +import React, { useEffect, useState } from "react"; +import Badge from "react-bootstrap/Badge"; + +import { getOrCreateAnonymousId } from "@/common/cookies"; +import { TagConsensusResponse } from "@/common/schema_types"; +import { useAppDispatch } from "@/common/types"; +import { APIGetTagConsensus, APISubmitTagVote } from "@/store/api"; +import { setNotification } from "@/store/slices/toastsSlice"; + +interface TagVotePickerProps { + backendURL: string; + cardIdentifier: string; +} + +const APPLY = 1; +const NOT_APPLICABLE = -1; + +export function TagVotePicker({ + backendURL, + cardIdentifier, +}: TagVotePickerProps) { + const dispatch = useAppDispatch(); + + const [entries, setEntries] = useState([]); + const [loading, setLoading] = useState(true); + const [submittingTagName, setSubmittingTagName] = useState( + null + ); + + useEffect(() => { + setLoading(true); + APIGetTagConsensus(backendURL, cardIdentifier) + .then((response) => setEntries(response.tags)) + .catch(() => setEntries([])) + .finally(() => setLoading(false)); + }, [backendURL, cardIdentifier]); + + const submit = (tagName: string, currentPolarity?: number | null) => { + const nextPolarity = currentPolarity === APPLY ? NOT_APPLICABLE : APPLY; + setSubmittingTagName(tagName); + APISubmitTagVote( + backendURL, + cardIdentifier, + getOrCreateAnonymousId(), + tagName, + nextPolarity + ) + .then((updatedEntry) => { + setEntries((previous) => + previous.map((entry) => + entry.tagName === tagName ? updatedEntry : entry + ) + ); + }) + .catch(() => + dispatch( + setNotification([ + Math.random().toString(), + { + name: "Vote failed", + message: + "Something went wrong submitting your vote - please try again.", + level: "error", + }, + ]) + ) + ) + .finally(() => setSubmittingTagName(null)); + }; + + if (loading) { + return
Loading tags...
; + } + + return ( +
+ {entries.map((entry) => ( + submit(entry.tagName, entry.resolvedPolarity)} + > + {entry.tagName} + + ))} +
+ ); +} diff --git a/frontend/src/features/cardDetailedView/CardDetailedViewModal.tsx b/frontend/src/features/cardDetailedView/CardDetailedViewModal.tsx index 934159601..baef432d7 100644 --- a/frontend/src/features/cardDetailedView/CardDetailedViewModal.tsx +++ b/frontend/src/features/cardDetailedView/CardDetailedViewModal.tsx @@ -4,19 +4,21 @@ * some more information (e.g. size, dote uploaded, etc.), and a button to download the full res image. */ -import React, { memo } from "react"; +import React, { memo, useState } from "react"; import Badge from "react-bootstrap/Badge"; import Button from "react-bootstrap/Button"; import Modal from "react-bootstrap/Modal"; import Row from "react-bootstrap/Row"; import { getCardDataAttributes } from "@/common/cardDom"; -import { CardDocument, useAppDispatch } from "@/common/types"; +import { PrintingConsensusResponse } from "@/common/schema_types"; +import { CardDocument, useAppDispatch, useAppSelector } from "@/common/types"; import { imageSizeToMBString, toTitleCase } from "@/common/utils"; import { AutofillTable } from "@/components/AutofillTable"; import { ClickToCopy } from "@/components/ClickToCopy"; import DisableSSR from "@/components/DisableSSR"; import { RightPaddedIcon } from "@/components/icon"; +import { AttributeVotingPanel } from "@/features/attributeVoting/AttributeVotingPanel"; import { AddCardToFavorites } from "@/features/card/AddCardToFavorites"; import { AddCardToProjectForm } from "@/features/card/AddCardToProjectForm"; import { @@ -26,6 +28,7 @@ import { import { useDoImageDownload } from "@/features/download/downloadImages"; import { PrintingTagPicker } from "@/features/printingTags/PrintingTagPicker"; import { useGetLanguagesQuery } from "@/store/api"; +import { selectRemoteBackendURL } from "@/store/slices/backendSlice"; import { setNotification } from "@/store/slices/toastsSlice"; interface CardDetailedViewProps { @@ -47,6 +50,10 @@ export function CardDetailedViewModal({ const dispatch = useAppDispatch(); const queueImageDownload = useDoImageDownload(); const getLanguagesQuery = useGetLanguagesQuery(); + const backendURL = useAppSelector(selectRemoteBackendURL); + + const [printingConsensus, setPrintingConsensus] = + useState(null); //# endregion @@ -177,7 +184,21 @@ export function CardDetailedViewModal({

Help us figure out which real-world printing this card is!

- + + {printingConsensus != null && + printingConsensus.resolvedPrinting == null && + backendURL != null && ( + <> +
+ + + )} diff --git a/frontend/src/features/printingTags/PrintingTagPicker.tsx b/frontend/src/features/printingTags/PrintingTagPicker.tsx index 42bc1ee98..f834d9d27 100644 --- a/frontend/src/features/printingTags/PrintingTagPicker.tsx +++ b/frontend/src/features/printingTags/PrintingTagPicker.tsx @@ -36,9 +36,18 @@ import { setNotification } from "@/store/slices/toastsSlice"; interface PrintingTagPickerProps { /** The image identifier of the card being tagged. */ cardIdentifier: string; + /** + * Notified whenever this component's own consensus state changes (initial fetch and after + * each vote submission) - lets a parent (e.g. CardDetailedViewModal) decide whether to show + * the attribute-voting follow-up panel without this component needing to know about it. + */ + onConsensusChange?: (consensus: PrintingConsensusResponse | null) => void; } -export function PrintingTagPicker({ cardIdentifier }: PrintingTagPickerProps) { +export function PrintingTagPicker({ + cardIdentifier, + onConsensusChange, +}: PrintingTagPickerProps) { const dispatch = useAppDispatch(); const backendURL = useAppSelector(selectRemoteBackendURL); @@ -55,8 +64,12 @@ export function PrintingTagPicker({ cardIdentifier }: PrintingTagPickerProps) { return; } APIGetPrintingConsensus(backendURL, cardIdentifier) - .then(setConsensus) + .then((response) => { + setConsensus(response); + onConsensusChange?.(response); + }) .catch(() => undefined); + // eslint-disable-next-line react-hooks/exhaustive-deps }, [backendURL, cardIdentifier]); useEffect(() => { @@ -87,6 +100,7 @@ export function PrintingTagPicker({ cardIdentifier }: PrintingTagPickerProps) { ) .then((response) => { setConsensus(response); + onConsensusChange?.(response); dispatch( setNotification([ Math.random().toString(), diff --git a/frontend/src/features/printingTags/PrintingTagQueue.tsx b/frontend/src/features/printingTags/PrintingTagQueue.tsx index e161ac05e..01db0a671 100644 --- a/frontend/src/features/printingTags/PrintingTagQueue.tsx +++ b/frontend/src/features/printingTags/PrintingTagQueue.tsx @@ -25,6 +25,7 @@ import { } from "@/common/schema_types"; import { CardDocument, useAppDispatch, useAppSelector } from "@/common/types"; import { Spinner } from "@/components/Spinner"; +import { AttributeVotingPanel } from "@/features/attributeVoting/AttributeVotingPanel"; import { APIGetPrintingCandidates, APIGetPrintingConsensus, @@ -99,6 +100,10 @@ export function PrintingTagQueue() { const [submitting, setSubmitting] = useState(false); const [flavorText, setFlavorText] = useState(null); const [revealed, setRevealed] = useState(false); + // whether the user has taken a printing-tag action (vote or no-match) on the *current* + // card this session - the attribute-voting follow-up panel is step 2 after that action, + // not shown alongside the initial printing picker on a card the user hasn't touched yet. + const [votedThisCard, setVotedThisCard] = useState(false); // guards the fetch effect below against React 18 Strict Mode's dev-time double-invoke, // which would otherwise append the same page's cards twice const fetchedPagesRef = React.useRef>(new Set()); @@ -137,9 +142,10 @@ export function PrintingTagQueue() { // eslint-disable-next-line react-hooks/exhaustive-deps }, [backendURL, currentIndex, queueCards.length, page, pages]); - // reset the reveal animation for each new card + // reset the reveal animation and attribute-voting step for each new card useEffect(() => { setRevealed(false); + setVotedThisCard(false); }, [currentCard?.identifier]); useEffect(() => { @@ -188,7 +194,15 @@ export function PrintingTagQueue() { printingIdentifier, isNoMatch ) - .then(() => advance()) + .then((response) => { + setConsensus(response); + setVotedThisCard(true); + // if this vote itself resolved the printing (e.g. it broke a tie), there's nothing + // left to ask about - advance immediately rather than showing the attribute panel. + if (response.resolvedPrinting != null) { + advance(); + } + }) .catch(() => dispatch( setNotification([ @@ -337,6 +351,17 @@ export function PrintingTagQueue() { ))} + {votedThisCard && + consensus?.resolvedPrinting == null && + backendURL != null && ( +
+
+ +
+ )}
+ {votedThisCard && consensus?.resolvedPrinting == null && ( + + )}
)} diff --git a/frontend/src/mocks/handlers.ts b/frontend/src/mocks/handlers.ts index fc8a3f66a..96f794e8b 100644 --- a/frontend/src/mocks/handlers.ts +++ b/frontend/src/mocks/handlers.ts @@ -9,6 +9,8 @@ import { SupporterTier, } from "@/common/schema_types"; import { + canonicalArtist1, + canonicalArtist2, cardDocument1, cardDocument2, cardDocument3, @@ -716,6 +718,68 @@ export const printingTagQueueNoResults = http.get( //# endregion +//# region attribute voting + +export const artistCandidatesTwoResults = http.post( + buildRoute("2/artistCandidates/"), + () => + HttpResponse.json( + { results: [canonicalArtist1, canonicalArtist2] }, + { status: 200 } + ) +); + +export const artistConsensusUnresolved = http.post( + buildRoute("2/artistConsensus/"), + () => + HttpResponse.json( + { resolvedArtist: null, isUnknown: false, voteTally: [] }, + { status: 200 } + ) +); + +export const submitArtistVoteResolvesToCanonicalArtist1 = http.post( + buildRoute("2/submitArtistVote/"), + () => + HttpResponse.json( + { + resolvedArtist: canonicalArtist1, + isUnknown: false, + voteTally: [{ artist: canonicalArtist1, isUnknown: false, count: 1 }], + }, + { status: 200 } + ) +); + +export const tagConsensusTwoUnresolvedTags = http.post( + buildRoute("2/tagConsensus/"), + () => + HttpResponse.json( + { + tags: [ + { tagName: "Borderless", resolvedPolarity: null, tally: [] }, + { tagName: "Extended", resolvedPolarity: null, tally: [] }, + ], + }, + { status: 200 } + ) +); + +export const submitTagVoteResolvesToApply = http.post( + buildRoute("2/submitTagVote/"), + () => + HttpResponse.json( + { + tagName: "Borderless", + resolvedPolarity: 1, + tally: [{ polarity: 1, count: 1 }], + }, + { status: 200 } + ) +); + +//# endregion + //# region presets export const defaultHandlers = [ diff --git a/frontend/src/store/api.ts b/frontend/src/store/api.ts index 47ba463c5..19c388ba5 100644 --- a/frontend/src/store/api.ts +++ b/frontend/src/store/api.ts @@ -15,6 +15,8 @@ import { processQuery, } from "@/common/processing"; import { + ArtistCandidatesResponse, + ArtistConsensusResponse, Card, CardbacksResponse, CardsRequest, @@ -46,6 +48,7 @@ import { SampleCardsResponse, SourcesResponse, Tag, + TagConsensusResponse, TagsResponse, } from "@/common/schema_types"; import { @@ -405,6 +408,112 @@ export async function APISubmitPrintingTag( }); } +export async function APIGetArtistCandidates( + backendURL: string, + identifier: string, + query?: string +): Promise { + const rawResponse = await fetch( + formatURL(backendURL, "/2/artistCandidates/"), + { + method: "POST", + body: JSON.stringify({ identifier, query }), + credentials: "same-origin", + headers: getCSRFHeader(), + } + ); + return rawResponse.json().then((content) => { + if (rawResponse.status === 200 && content.results != null) { + return content as ArtistCandidatesResponse; + } + throw { name: content.name, message: content.message }; + }); +} + +export async function APIGetArtistConsensus( + backendURL: string, + identifier: string +): Promise { + const rawResponse = await fetch( + formatURL(backendURL, "/2/artistConsensus/"), + { + method: "POST", + body: JSON.stringify({ identifier }), + credentials: "same-origin", + headers: getCSRFHeader(), + } + ); + return rawResponse.json().then((content) => { + if (rawResponse.status === 200 && content.voteTally != null) { + return content as ArtistConsensusResponse; + } + throw { name: content.name, message: content.message }; + }); +} + +export async function APISubmitArtistVote( + backendURL: string, + identifier: string, + anonymousId: string, + artistName?: string, + isUnknown = false +): Promise { + const rawResponse = await fetch( + formatURL(backendURL, "/2/submitArtistVote/"), + { + method: "POST", + body: JSON.stringify({ identifier, anonymousId, artistName, isUnknown }), + credentials: "same-origin", + headers: getCSRFHeader(), + } + ); + return rawResponse.json().then((content) => { + if (rawResponse.status === 200 && content.voteTally != null) { + return content as ArtistConsensusResponse; + } + throw { name: content.name, message: content.message }; + }); +} + +export async function APIGetTagConsensus( + backendURL: string, + identifier: string +): Promise { + const rawResponse = await fetch(formatURL(backendURL, "/2/tagConsensus/"), { + method: "POST", + body: JSON.stringify({ identifier }), + credentials: "same-origin", + headers: getCSRFHeader(), + }); + return rawResponse.json().then((content) => { + if (rawResponse.status === 200 && content.tags != null) { + return content as TagConsensusResponse; + } + throw { name: content.name, message: content.message }; + }); +} + +export async function APISubmitTagVote( + backendURL: string, + identifier: string, + anonymousId: string, + tagName: string, + polarity: number +): Promise { + const rawResponse = await fetch(formatURL(backendURL, "/2/submitTagVote/"), { + method: "POST", + body: JSON.stringify({ identifier, anonymousId, tagName, polarity }), + credentials: "same-origin", + headers: getCSRFHeader(), + }); + return rawResponse.json().then((content) => { + if (rawResponse.status === 200 && content.tally != null) { + return content as TagConsensusResponse["tags"][number]; + } + throw { name: content.name, message: content.message }; + }); +} + export async function APIGetPrintingTagQueue( backendURL: string, page: number diff --git a/frontend/tests/ArtistVotePicker.spec.ts b/frontend/tests/ArtistVotePicker.spec.ts new file mode 100644 index 000000000..0704635dc --- /dev/null +++ b/frontend/tests/ArtistVotePicker.spec.ts @@ -0,0 +1,106 @@ +import { expect } from "@playwright/test"; + +import { SelectedImageSeparator } from "@/common/constants"; +import { + canonicalArtist1, + canonicalArtist2, + cardDocument1, +} from "@/common/test-constants"; +import { + artistCandidatesTwoResults, + artistConsensusUnresolved, + cardbacksTwoOtherResults, + cardDocumentsOneResult, + defaultHandlers, + printingCandidatesTwoResults, + printingConsensusUnresolved, + searchResultsOneResult, + sourceDocumentsOneResult, + submitArtistVoteResolvesToCanonicalArtist1, + tagConsensusTwoUnresolvedTags, +} from "@/mocks/handlers"; + +import { test } from "../playwright.setup"; +import { importText, loadPageWithDefaultBackend } from "./test-utils"; + +const openDetailedView = async (page: any, cardName: string) => { + await page.getByAltText(cardName).click(); + await expect(page.getByText("Card Details")).toBeVisible(); +}; + +test.describe("ArtistVotePicker tests", () => { + test("shows the attribute-voting panel once printing consensus is unresolved, listing candidate artists", async ({ + page, + network, + }) => { + network.use( + cardDocumentsOneResult, + cardbacksTwoOtherResults, + sourceDocumentsOneResult, + searchResultsOneResult, + printingCandidatesTwoResults, + printingConsensusUnresolved, + artistCandidatesTwoResults, + artistConsensusUnresolved, + tagConsensusTwoUnresolvedTags, + ...defaultHandlers + ); + await loadPageWithDefaultBackend(page); + + await importText( + page, + `my search query${SelectedImageSeparator}${cardDocument1.identifier}` + ); + await openDetailedView(page, cardDocument1.name); + + // longer than the default 5s: showing this panel is gated behind a chain of fetches + // (printing consensus resolves -> CardDetailedViewModal re-renders -> the panel mounts + // -> its own two children each fire their own fetch), which is slower than a single + // round trip even against these MSW mocks. + const panel = page.getByTestId("attribute-voting-panel"); + await expect(panel).toBeVisible({ timeout: 10000 }); + + const artistPicker = page.getByTestId("artist-vote-picker"); + await expect(artistPicker.getByText(canonicalArtist1.name)).toBeVisible(); + await expect(artistPicker.getByText(canonicalArtist2.name)).toBeVisible(); + await expect(artistPicker.getByText("Unknown artist")).toBeVisible(); + }); + + test("submitting a vote for an artist updates the shown consensus", async ({ + page, + network, + }) => { + network.use( + cardDocumentsOneResult, + cardbacksTwoOtherResults, + sourceDocumentsOneResult, + searchResultsOneResult, + printingCandidatesTwoResults, + printingConsensusUnresolved, + artistCandidatesTwoResults, + artistConsensusUnresolved, + submitArtistVoteResolvesToCanonicalArtist1, + tagConsensusTwoUnresolvedTags, + ...defaultHandlers + ); + await loadPageWithDefaultBackend(page); + + await importText( + page, + `my search query${SelectedImageSeparator}${cardDocument1.identifier}` + ); + await openDetailedView(page, cardDocument1.name); + + const artistPicker = page.getByTestId("artist-vote-picker"); + await expect(artistPicker.getByText("Not yet resolved")).toBeVisible(); + + await artistPicker.getByText(canonicalArtist1.name).click(); + + await expect(page.getByText("Vote submitted")).toBeVisible(); + await expect( + page + .getByTestId("artist-vote-consensus") + .getByText(`Current consensus: ${canonicalArtist1.name}`) + ).toBeVisible(); + }); +}); diff --git a/frontend/tests/TagVotePicker.spec.ts b/frontend/tests/TagVotePicker.spec.ts new file mode 100644 index 000000000..098c7e119 --- /dev/null +++ b/frontend/tests/TagVotePicker.spec.ts @@ -0,0 +1,89 @@ +import { expect } from "@playwright/test"; + +import { SelectedImageSeparator } from "@/common/constants"; +import { cardDocument1 } from "@/common/test-constants"; +import { + artistCandidatesTwoResults, + artistConsensusUnresolved, + cardbacksTwoOtherResults, + cardDocumentsOneResult, + defaultHandlers, + printingCandidatesTwoResults, + printingConsensusUnresolved, + searchResultsOneResult, + sourceDocumentsOneResult, + submitTagVoteResolvesToApply, + tagConsensusTwoUnresolvedTags, +} from "@/mocks/handlers"; + +import { test } from "../playwright.setup"; +import { importText, loadPageWithDefaultBackend } from "./test-utils"; + +const openDetailedView = async (page: any, cardName: string) => { + await page.getByAltText(cardName).click(); + await expect(page.getByText("Card Details")).toBeVisible(); +}; + +test.describe("TagVotePicker tests", () => { + test("lists every seeded tag as an unresolved toggle chip", async ({ + page, + network, + }) => { + network.use( + cardDocumentsOneResult, + cardbacksTwoOtherResults, + sourceDocumentsOneResult, + searchResultsOneResult, + printingCandidatesTwoResults, + printingConsensusUnresolved, + artistCandidatesTwoResults, + artistConsensusUnresolved, + tagConsensusTwoUnresolvedTags, + ...defaultHandlers + ); + await loadPageWithDefaultBackend(page); + + await importText( + page, + `my search query${SelectedImageSeparator}${cardDocument1.identifier}` + ); + await openDetailedView(page, cardDocument1.name); + + const tagPicker = page.getByTestId("tag-vote-picker"); + await expect(tagPicker.getByText("Borderless")).toBeVisible(); + await expect(tagPicker.getByText("Extended")).toBeVisible(); + }); + + test("clicking a tag chip submits a vote and updates that chip's state", async ({ + page, + network, + }) => { + network.use( + cardDocumentsOneResult, + cardbacksTwoOtherResults, + sourceDocumentsOneResult, + searchResultsOneResult, + printingCandidatesTwoResults, + printingConsensusUnresolved, + artistCandidatesTwoResults, + artistConsensusUnresolved, + tagConsensusTwoUnresolvedTags, + submitTagVoteResolvesToApply, + ...defaultHandlers + ); + await loadPageWithDefaultBackend(page); + + await importText( + page, + `my search query${SelectedImageSeparator}${cardDocument1.identifier}` + ); + await openDetailedView(page, cardDocument1.name); + + const tagPicker = page.getByTestId("tag-vote-picker"); + await tagPicker.getByText("Borderless").click(); + + // the mock always resolves to APPLY regardless of which chip was clicked, so this proves + // the click round-tripped through APISubmitTagVote and re-rendered from the response + await expect(tagPicker.getByText("Borderless")).toBeVisible(); + }); +}); diff --git a/schemas/schemas/ArtistVoteTallyEntry.json b/schemas/schemas/ArtistVoteTallyEntry.json new file mode 100644 index 000000000..adf2338f9 --- /dev/null +++ b/schemas/schemas/ArtistVoteTallyEntry.json @@ -0,0 +1,12 @@ +{ + "title": "Artist Vote Tally Entry", + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "artist": { "$ref": "./CanonicalArtist.json" }, + "isUnknown": { "type": "boolean" }, + "count": { "type": "integer" } + }, + "required": ["isUnknown", "count"], + "additionalProperties": false +} diff --git a/schemas/schemas/TagConsensusEntry.json b/schemas/schemas/TagConsensusEntry.json new file mode 100644 index 000000000..1cbb7d402 --- /dev/null +++ b/schemas/schemas/TagConsensusEntry.json @@ -0,0 +1,15 @@ +{ + "title": "Tag Consensus Entry", + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "tagName": { "type": "string" }, + "resolvedPolarity": { "type": ["integer", "null"] }, + "tally": { + "type": "array", + "items": { "$ref": "./TagVoteTallyEntry.json" } + } + }, + "required": ["tagName", "tally"], + "additionalProperties": false +} diff --git a/schemas/schemas/TagVoteTallyEntry.json b/schemas/schemas/TagVoteTallyEntry.json new file mode 100644 index 000000000..fba3d1b93 --- /dev/null +++ b/schemas/schemas/TagVoteTallyEntry.json @@ -0,0 +1,11 @@ +{ + "title": "Tag Vote Tally Entry", + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "polarity": { "type": "integer" }, + "count": { "type": "integer" } + }, + "required": ["polarity", "count"], + "additionalProperties": false +} diff --git a/schemas/schemas/endpoints/ArtistCandidatesRequest.json b/schemas/schemas/endpoints/ArtistCandidatesRequest.json new file mode 100644 index 000000000..480290673 --- /dev/null +++ b/schemas/schemas/endpoints/ArtistCandidatesRequest.json @@ -0,0 +1,11 @@ +{ + "title": "Artist Candidates Request", + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "identifier": { "type": "string" }, + "query": { "type": ["string", "null"] } + }, + "required": ["identifier"], + "additionalProperties": false +} diff --git a/schemas/schemas/endpoints/ArtistCandidatesResponse.json b/schemas/schemas/endpoints/ArtistCandidatesResponse.json new file mode 100644 index 000000000..a60105c3b --- /dev/null +++ b/schemas/schemas/endpoints/ArtistCandidatesResponse.json @@ -0,0 +1,13 @@ +{ + "title": "Artist Candidates Response", + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "results": { + "type": "array", + "items": { "$ref": "../CanonicalArtist.json" } + } + }, + "required": ["results"], + "additionalProperties": false +} diff --git a/schemas/schemas/endpoints/ArtistConsensusRequest.json b/schemas/schemas/endpoints/ArtistConsensusRequest.json new file mode 100644 index 000000000..5ad65d012 --- /dev/null +++ b/schemas/schemas/endpoints/ArtistConsensusRequest.json @@ -0,0 +1,10 @@ +{ + "title": "Artist Consensus Request", + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "identifier": { "type": "string" } + }, + "required": ["identifier"], + "additionalProperties": false +} diff --git a/schemas/schemas/endpoints/ArtistConsensusResponse.json b/schemas/schemas/endpoints/ArtistConsensusResponse.json new file mode 100644 index 000000000..853f57483 --- /dev/null +++ b/schemas/schemas/endpoints/ArtistConsensusResponse.json @@ -0,0 +1,15 @@ +{ + "title": "Artist Consensus Response", + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "resolvedArtist": { "$ref": "../CanonicalArtist.json" }, + "isUnknown": { "type": "boolean" }, + "voteTally": { + "type": "array", + "items": { "$ref": "../ArtistVoteTallyEntry.json" } + } + }, + "required": ["isUnknown", "voteTally"], + "additionalProperties": false +} diff --git a/schemas/schemas/endpoints/SubmitArtistVoteRequest.json b/schemas/schemas/endpoints/SubmitArtistVoteRequest.json new file mode 100644 index 000000000..0ec9727ef --- /dev/null +++ b/schemas/schemas/endpoints/SubmitArtistVoteRequest.json @@ -0,0 +1,13 @@ +{ + "title": "Submit Artist Vote Request", + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "identifier": { "type": "string" }, + "artistName": { "type": ["string", "null"] }, + "isUnknown": { "type": "boolean" }, + "anonymousId": { "type": "string" } + }, + "required": ["identifier", "isUnknown", "anonymousId"], + "additionalProperties": false +} diff --git a/schemas/schemas/endpoints/SubmitTagVoteRequest.json b/schemas/schemas/endpoints/SubmitTagVoteRequest.json new file mode 100644 index 000000000..625634b08 --- /dev/null +++ b/schemas/schemas/endpoints/SubmitTagVoteRequest.json @@ -0,0 +1,13 @@ +{ + "title": "Submit Tag Vote Request", + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "identifier": { "type": "string" }, + "tagName": { "type": "string" }, + "polarity": { "type": "integer" }, + "anonymousId": { "type": "string" } + }, + "required": ["identifier", "tagName", "polarity", "anonymousId"], + "additionalProperties": false +} diff --git a/schemas/schemas/endpoints/TagConsensusRequest.json b/schemas/schemas/endpoints/TagConsensusRequest.json new file mode 100644 index 000000000..af4869f17 --- /dev/null +++ b/schemas/schemas/endpoints/TagConsensusRequest.json @@ -0,0 +1,10 @@ +{ + "title": "Tag Consensus Request", + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "identifier": { "type": "string" } + }, + "required": ["identifier"], + "additionalProperties": false +} diff --git a/schemas/schemas/endpoints/TagConsensusResponse.json b/schemas/schemas/endpoints/TagConsensusResponse.json new file mode 100644 index 000000000..6fe5f35d5 --- /dev/null +++ b/schemas/schemas/endpoints/TagConsensusResponse.json @@ -0,0 +1,13 @@ +{ + "title": "Tag Consensus Response", + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "tags": { + "type": "array", + "items": { "$ref": "../TagConsensusEntry.json" } + } + }, + "required": ["tags"], + "additionalProperties": false +} From 202e486805456f5ec8b6b360d3a3657b82cbdfd1 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Sun, 12 Jul 2026 20:21:09 +0000 Subject: [PATCH 3/6] Add federation-readiness schema stub to the vote system Groundwork for a future cross-instance verdict-exchange feature (docs/federation-v1.md, spec only - no export/import code yet): VoteSource gains FEDERATED, AbstractWeightedVote gains a nullable peer field, and a new VOTE_FEDERATED_WEIGHT setting is wired into vote_consensus's weighting. Also de-duplicates the three consensus modules' identical _SOURCE_WEIGHTS dict into one shared constant (so a new source can't be forgotten in one of them), and renames VoteTuple.is_ai to is_human_backed - the caller now decides per vote rather than this being derived from source == AI, since a federated vote's human-backed-ness will depend on what the exporting peer reports. Behavior is unchanged for every source that exists today (confirmed by the full existing test suite passing unmodified). Co-Authored-By: Claude Sonnet 5 --- MPCAutofill/MPCAutofill/settings.py | 4 + MPCAutofill/cardpicker/artist_consensus.py | 20 ++--- ...vote_peer_cardprintingtag_peer_and_more.py | 61 ++++++++++++++ MPCAutofill/cardpicker/models.py | 5 ++ MPCAutofill/cardpicker/printing_consensus.py | 18 ++-- MPCAutofill/cardpicker/tag_consensus.py | 22 ++--- .../cardpicker/tests/test_vote_consensus.py | 83 +++++++++++++++---- MPCAutofill/cardpicker/vote_consensus.py | 41 ++++++--- docs/federation-v1.md | 56 +++++++++++++ 9 files changed, 252 insertions(+), 58 deletions(-) create mode 100644 MPCAutofill/cardpicker/migrations/0054_cardartistvote_peer_cardprintingtag_peer_and_more.py create mode 100644 docs/federation-v1.md diff --git a/MPCAutofill/MPCAutofill/settings.py b/MPCAutofill/MPCAutofill/settings.py index ce690d1ff..ae508a3bf 100755 --- a/MPCAutofill/MPCAutofill/settings.py +++ b/MPCAutofill/MPCAutofill/settings.py @@ -60,6 +60,10 @@ PRINTING_TAG_MIN_SHARE = env.float("PRINTING_TAG_MIN_SHARE", default=0.6) PRINTING_TAG_ADMIN_WEIGHT = env.float("PRINTING_TAG_ADMIN_WEIGHT", default=5) PRINTING_TAG_AI_WEIGHT = env.float("PRINTING_TAG_AI_WEIGHT", default=0.5) +# federation-readiness stub (see docs/federation-v1.md) - no import path creates federated +# votes yet, so this setting is currently inert, but it's wired into vote_consensus._SOURCE_WEIGHTS +# alongside the weights above. +VOTE_FEDERATED_WEIGHT = env.float("VOTE_FEDERATED_WEIGHT", default=1.0) # django-ratelimit rate string (see cardpicker.views.post_submit_printing_tag), keyed by the # client-generated anonymous ID (IP as a fallback if that header is somehow missing). PRINTING_TAG_SUBMISSION_RATE = env("PRINTING_TAG_SUBMISSION_RATE", default="20/h") diff --git a/MPCAutofill/cardpicker/artist_consensus.py b/MPCAutofill/cardpicker/artist_consensus.py index 3da4ed114..8592e1ec3 100644 --- a/MPCAutofill/cardpicker/artist_consensus.py +++ b/MPCAutofill/cardpicker/artist_consensus.py @@ -3,18 +3,14 @@ from django.conf import settings from cardpicker.models import ArtistVoteStatus, CanonicalArtist, Card, VoteSource -from cardpicker.vote_consensus import VoteTuple, resolve_weighted_consensus +from cardpicker.vote_consensus import ( + _SOURCE_WEIGHTS, + VoteTuple, + resolve_weighted_consensus, +) UNKNOWN: Literal["UNKNOWN"] = "UNKNOWN" -# reuses the printing-tag weight/threshold settings rather than introducing a parallel set - -# see the plan note in the models: per-vote-type overrides are YAGNI until a real need appears. -_SOURCE_WEIGHTS: dict[str, float] = { - VoteSource.USER: 1.0, - VoteSource.ADMIN: settings.PRINTING_TAG_ADMIN_WEIGHT, - VoteSource.AI: settings.PRINTING_TAG_AI_WEIGHT, -} - def resolve_artist(card: Card) -> CanonicalArtist | Literal["UNKNOWN"] | None: """ @@ -45,7 +41,11 @@ def resolve_artist(card: Card) -> CanonicalArtist | Literal["UNKNOWN"] | None: key = vote.artist_id artists_by_id[vote.artist_id] = vote.artist vote_tuples.append( - VoteTuple(outcome_key=key, weight=_SOURCE_WEIGHTS[vote.source], is_ai=vote.source == VoteSource.AI) + VoteTuple( + outcome_key=key, + weight=_SOURCE_WEIGHTS[vote.source], + is_human_backed=vote.source != VoteSource.AI, + ) ) winning_key = resolve_weighted_consensus( diff --git a/MPCAutofill/cardpicker/migrations/0054_cardartistvote_peer_cardprintingtag_peer_and_more.py b/MPCAutofill/cardpicker/migrations/0054_cardartistvote_peer_cardprintingtag_peer_and_more.py new file mode 100644 index 000000000..249850ca3 --- /dev/null +++ b/MPCAutofill/cardpicker/migrations/0054_cardartistvote_peer_cardprintingtag_peer_and_more.py @@ -0,0 +1,61 @@ +# Generated by Django 4.2.30 on 2026-07-12 20:14 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("cardpicker", "0053_card_artist_vote_status_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="cardartistvote", + name="peer", + field=models.CharField( + blank=True, help_text="Federation peer name; set only when source='federated'", max_length=64, null=True + ), + ), + migrations.AddField( + model_name="cardprintingtag", + name="peer", + field=models.CharField( + blank=True, help_text="Federation peer name; set only when source='federated'", max_length=64, null=True + ), + ), + migrations.AddField( + model_name="cardtagvote", + name="peer", + field=models.CharField( + blank=True, help_text="Federation peer name; set only when source='federated'", max_length=64, null=True + ), + ), + migrations.AlterField( + model_name="cardartistvote", + name="source", + field=models.CharField( + choices=[("user", "User"), ("admin", "Admin"), ("ai", "AI"), ("federated", "Federated")], + default="user", + max_length=10, + ), + ), + migrations.AlterField( + model_name="cardprintingtag", + name="source", + field=models.CharField( + choices=[("user", "User"), ("admin", "Admin"), ("ai", "AI"), ("federated", "Federated")], + default="user", + max_length=10, + ), + ), + migrations.AlterField( + model_name="cardtagvote", + name="source", + field=models.CharField( + choices=[("user", "User"), ("admin", "Admin"), ("ai", "AI"), ("federated", "Federated")], + default="user", + max_length=10, + ), + ), + ] diff --git a/MPCAutofill/cardpicker/models.py b/MPCAutofill/cardpicker/models.py index 72187384b..f88058222 100755 --- a/MPCAutofill/cardpicker/models.py +++ b/MPCAutofill/cardpicker/models.py @@ -464,6 +464,7 @@ class VoteSource(models.TextChoices): USER = "user", gettext_lazy("User") ADMIN = "admin", gettext_lazy("Admin") AI = "ai", gettext_lazy("AI") + FEDERATED = "federated", gettext_lazy("Federated") class AbstractWeightedVote(models.Model): @@ -482,6 +483,10 @@ class AbstractWeightedVote(models.Model): source = models.CharField(max_length=10, choices=VoteSource.choices, default=VoteSource.USER) confidence = models.FloatField(null=True, blank=True) created_at = models.DateTimeField(auto_now_add=True) + # federation-readiness stub (see docs/federation-v1.md) - no import path sets this yet. + peer = models.CharField( + max_length=64, null=True, blank=True, help_text="Federation peer name; set only when source='federated'" + ) class Meta: abstract = True diff --git a/MPCAutofill/cardpicker/printing_consensus.py b/MPCAutofill/cardpicker/printing_consensus.py index f76c65176..1a7f2153b 100644 --- a/MPCAutofill/cardpicker/printing_consensus.py +++ b/MPCAutofill/cardpicker/printing_consensus.py @@ -10,16 +10,14 @@ PrintingTagStatus, VoteSource, ) -from cardpicker.vote_consensus import VoteTuple, resolve_weighted_consensus +from cardpicker.vote_consensus import ( + _SOURCE_WEIGHTS, + VoteTuple, + resolve_weighted_consensus, +) NO_MATCH: Literal["NO_MATCH"] = "NO_MATCH" -_SOURCE_WEIGHTS: dict[str, float] = { - VoteSource.USER: 1.0, - VoteSource.ADMIN: settings.PRINTING_TAG_ADMIN_WEIGHT, - VoteSource.AI: settings.PRINTING_TAG_AI_WEIGHT, -} - def resolve_printing(card: Card) -> CanonicalCard | Literal["NO_MATCH"] | None: """ @@ -48,7 +46,11 @@ def resolve_printing(card: Card) -> CanonicalCard | Literal["NO_MATCH"] | None: key = vote.printing_id printings_by_id[vote.printing_id] = vote.printing vote_tuples.append( - VoteTuple(outcome_key=key, weight=_SOURCE_WEIGHTS[vote.source], is_ai=vote.source == VoteSource.AI) + VoteTuple( + outcome_key=key, + weight=_SOURCE_WEIGHTS[vote.source], + is_human_backed=vote.source != VoteSource.AI, + ) ) winning_key = resolve_weighted_consensus( diff --git a/MPCAutofill/cardpicker/tag_consensus.py b/MPCAutofill/cardpicker/tag_consensus.py index 544cb8b07..9147c7c3c 100644 --- a/MPCAutofill/cardpicker/tag_consensus.py +++ b/MPCAutofill/cardpicker/tag_consensus.py @@ -4,15 +4,11 @@ from django.conf import settings from cardpicker.models import Card, CardTagVote, Tag, VotePolarity, VoteSource -from cardpicker.vote_consensus import VoteTuple, resolve_weighted_consensus - -# reuses the printing-tag weight/threshold settings rather than introducing a parallel set - -# see the plan note in the models: per-vote-type overrides are YAGNI until a real need appears. -_SOURCE_WEIGHTS: dict[str, float] = { - VoteSource.USER: 1.0, - VoteSource.ADMIN: settings.PRINTING_TAG_ADMIN_WEIGHT, - VoteSource.AI: settings.PRINTING_TAG_AI_WEIGHT, -} +from cardpicker.vote_consensus import ( + _SOURCE_WEIGHTS, + VoteTuple, + resolve_weighted_consensus, +) def resolve_tag(card: Card, tag: Tag) -> int | None: @@ -27,7 +23,11 @@ def resolve_tag(card: Card, tag: Tag) -> int | None: if not votes: return None vote_tuples = [ - VoteTuple(outcome_key=vote.polarity, weight=_SOURCE_WEIGHTS[vote.source], is_ai=vote.source == VoteSource.AI) + VoteTuple( + outcome_key=vote.polarity, + weight=_SOURCE_WEIGHTS[vote.source], + is_human_backed=vote.source != VoteSource.AI, + ) for vote in votes ] resolved = resolve_weighted_consensus( @@ -114,7 +114,7 @@ def get_resolved_tag_overlay(card_ids: Iterable[int]) -> dict[int, dict[str, int VoteTuple( outcome_key=row["polarity"], weight=_SOURCE_WEIGHTS[row["source"]], - is_ai=row["source"] == VoteSource.AI, + is_human_backed=row["source"] != VoteSource.AI, ) ) diff --git a/MPCAutofill/cardpicker/tests/test_vote_consensus.py b/MPCAutofill/cardpicker/tests/test_vote_consensus.py index b39d1a2d6..8134626be 100644 --- a/MPCAutofill/cardpicker/tests/test_vote_consensus.py +++ b/MPCAutofill/cardpicker/tests/test_vote_consensus.py @@ -1,4 +1,12 @@ -from cardpicker.vote_consensus import VoteTuple, resolve_weighted_consensus +from django.conf import settings + +from cardpicker.models import VoteSource +from cardpicker.tests.factories import CardArtistVoteFactory, CardFactory +from cardpicker.vote_consensus import ( + _SOURCE_WEIGHTS, + VoteTuple, + resolve_weighted_consensus, +) class TestResolveWeightedConsensus: @@ -7,50 +15,89 @@ def test_no_votes_returns_none(self): def test_single_group_clears_thresholds(self): votes = [ - VoteTuple(outcome_key="a", weight=1.0, is_ai=False), - VoteTuple(outcome_key="a", weight=1.0, is_ai=False), + VoteTuple(outcome_key="a", weight=1.0, is_human_backed=True), + VoteTuple(outcome_key="a", weight=1.0, is_human_backed=True), ] assert resolve_weighted_consensus(votes, min_weight=2, min_share=0.6) == "a" def test_below_min_weight_returns_none(self): - votes = [VoteTuple(outcome_key="a", weight=1.0, is_ai=False)] + votes = [VoteTuple(outcome_key="a", weight=1.0, is_human_backed=True)] assert resolve_weighted_consensus(votes, min_weight=2, min_share=0.6) is None def test_tie_below_min_share_returns_none(self): # two outcomes with equal weight: share is exactly 0.5, below a 0.6 threshold votes = [ - VoteTuple(outcome_key="a", weight=1.0, is_ai=False), - VoteTuple(outcome_key="a", weight=1.0, is_ai=False), - VoteTuple(outcome_key="b", weight=1.0, is_ai=False), - VoteTuple(outcome_key="b", weight=1.0, is_ai=False), + VoteTuple(outcome_key="a", weight=1.0, is_human_backed=True), + VoteTuple(outcome_key="a", weight=1.0, is_human_backed=True), + VoteTuple(outcome_key="b", weight=1.0, is_human_backed=True), + VoteTuple(outcome_key="b", weight=1.0, is_human_backed=True), ] assert resolve_weighted_consensus(votes, min_weight=2, min_share=0.6) is None def test_admin_style_weight_override(self): # one high-weight vote (e.g. an "admin") outweighs two conflicting low-weight votes votes = [ - VoteTuple(outcome_key="a", weight=5.0, is_ai=False), - VoteTuple(outcome_key="b", weight=1.0, is_ai=False), - VoteTuple(outcome_key="b", weight=1.0, is_ai=False), + VoteTuple(outcome_key="a", weight=5.0, is_human_backed=True), + VoteTuple(outcome_key="b", weight=1.0, is_human_backed=True), + VoteTuple(outcome_key="b", weight=1.0, is_human_backed=True), ] assert resolve_weighted_consensus(votes, min_weight=2, min_share=0.6) == "a" def test_ai_only_votes_never_resolve_even_with_large_weight(self): - votes = [VoteTuple(outcome_key="a", weight=10.0, is_ai=True) for _ in range(5)] + votes = [VoteTuple(outcome_key="a", weight=10.0, is_human_backed=False) for _ in range(5)] assert resolve_weighted_consensus(votes, min_weight=2, min_share=0.6) is None def test_mixed_ai_and_non_ai_can_resolve(self): votes = [ - VoteTuple(outcome_key="a", weight=0.5, is_ai=True), - VoteTuple(outcome_key="a", weight=2.0, is_ai=False), + VoteTuple(outcome_key="a", weight=0.5, is_human_backed=False), + VoteTuple(outcome_key="a", weight=2.0, is_human_backed=True), ] assert resolve_weighted_consensus(votes, min_weight=2, min_share=0.6) == "a" def test_three_way_split_leader_below_share_returns_none(self): votes = [ - VoteTuple(outcome_key="a", weight=1.0, is_ai=False), - VoteTuple(outcome_key="a", weight=1.0, is_ai=False), - VoteTuple(outcome_key="b", weight=1.0, is_ai=False), - VoteTuple(outcome_key="c", weight=1.0, is_ai=False), + VoteTuple(outcome_key="a", weight=1.0, is_human_backed=True), + VoteTuple(outcome_key="a", weight=1.0, is_human_backed=True), + VoteTuple(outcome_key="b", weight=1.0, is_human_backed=True), + VoteTuple(outcome_key="c", weight=1.0, is_human_backed=True), ] assert resolve_weighted_consensus(votes, min_weight=2, min_share=0.6) is None + + +class TestFederatedWeighting: + """ + Federation-readiness stub (see docs/federation-v1.md) - no import path creates federated + votes yet, so these tests exercise the plumbing directly via VoteTuple/settings rather than + through a real submit view. + """ + + def test_federated_source_uses_the_configured_weight(self): + assert _SOURCE_WEIGHTS[VoteSource.FEDERATED] == settings.VOTE_FEDERATED_WEIGHT + + def test_federated_vote_with_human_backed_true_satisfies_the_gate(self): + votes = [ + VoteTuple(outcome_key="a", weight=settings.VOTE_FEDERATED_WEIGHT * 5, is_human_backed=True), + ] + assert resolve_weighted_consensus(votes, min_weight=2, min_share=0.6) == "a" + + def test_federated_vote_with_human_backed_false_does_not_satisfy_the_gate_alone(self): + # mirrors test_ai_only_votes_never_resolve_even_with_large_weight - a federated vote + # explicitly marked not-human-backed can never single-handedly clear consensus, same + # as an AI vote, regardless of how much weight it carries + votes = [VoteTuple(outcome_key="a", weight=settings.VOTE_FEDERATED_WEIGHT * 100, is_human_backed=False)] + assert resolve_weighted_consensus(votes, min_weight=2, min_share=0.6) is None + + +class TestFederatedModelFields: + def test_federated_source_and_peer_round_trip(self, db): + card = CardFactory() + vote = CardArtistVoteFactory(card=card, source=VoteSource.FEDERATED, peer="peer-instance-1") + vote.refresh_from_db() + assert vote.source == VoteSource.FEDERATED + assert vote.peer == "peer-instance-1" + + def test_peer_defaults_to_none_for_non_federated_votes(self, db): + card = CardFactory() + vote = CardArtistVoteFactory(card=card, source=VoteSource.USER) + vote.refresh_from_db() + assert vote.peer is None diff --git a/MPCAutofill/cardpicker/vote_consensus.py b/MPCAutofill/cardpicker/vote_consensus.py index 3f63d7195..38735ef8c 100644 --- a/MPCAutofill/cardpicker/vote_consensus.py +++ b/MPCAutofill/cardpicker/vote_consensus.py @@ -1,24 +1,42 @@ from collections import defaultdict from typing import Hashable, Iterable, NamedTuple, TypedDict +from django.conf import settings + +from cardpicker.models import VoteSource + +# Shared across printing/artist/tag consensus - previously duplicated identically in each of +# their own modules; hoisted here so a new source (e.g. `FEDERATED`) can't be forgotten in one +# of them. `printing_consensus.py`/`artist_consensus.py`/`tag_consensus.py` import this rather +# than redefining it. +_SOURCE_WEIGHTS: dict[str, float] = { + VoteSource.USER: 1.0, + VoteSource.ADMIN: settings.PRINTING_TAG_ADMIN_WEIGHT, + VoteSource.AI: settings.PRINTING_TAG_AI_WEIGHT, + VoteSource.FEDERATED: settings.VOTE_FEDERATED_WEIGHT, +} + class VoteTuple(NamedTuple): """ A single vote reduced to just what `resolve_weighted_consensus` needs to reconcile it: the outcome it argues for (grouping key - e.g. a printing's pk, an artist's pk, or a tag's polarity), its weight (already resolved from the vote's `source` by the caller), and - whether it came from an AI (which can never single-handedly clear consensus, regardless of - weight - see the non-AI gate below). + whether it should count towards the human-backed gate below. This is deliberately not + derived automatically from `source == AI` inside this module - the caller decides, since a + future federated vote's human-backed-ness depends on what the exporting peer reported, not + on the local `source` value alone (see docs/federation-v1.md). Every wrapper today + (`USER`/`ADMIN`/`AI`) still computes this as `source != AI`, so behaviour is unchanged. """ outcome_key: Hashable weight: float - is_ai: bool + is_human_backed: bool class _VoteGroup(TypedDict): weight: float - has_non_ai: bool + has_human_backed: bool def resolve_weighted_consensus(votes: Iterable[VoteTuple], min_weight: float, min_share: float) -> Hashable | None: @@ -36,27 +54,28 @@ def resolve_weighted_consensus(votes: Iterable[VoteTuple], min_weight: float, mi threshold of 2 on its own, which is what produces "admin override" behaviour from this one unified formula, with no special-cased branch for admin votes); - its share of the total weight across all groups is >= `min_share`; - - it contains at least one non-AI vote (a hard gate, independent of the weight math - above, so that no volume of AI-only votes can ever resolve consensus on their own). + - it contains at least one human-backed vote (a hard gate, independent of the weight math + above, so that no volume of non-human-backed votes - e.g. AI-only - can ever resolve + consensus on their own). """ votes = list(votes) if not votes: return None - groups: dict[Hashable, _VoteGroup] = defaultdict(lambda: _VoteGroup(weight=0.0, has_non_ai=False)) + groups: dict[Hashable, _VoteGroup] = defaultdict(lambda: _VoteGroup(weight=0.0, has_human_backed=False)) for vote in votes: group = groups[vote.outcome_key] group["weight"] += vote.weight - if not vote.is_ai: - group["has_non_ai"] = True + if vote.is_human_backed: + group["has_human_backed"] = True total_weight = sum(group["weight"] for group in groups.values()) winning_key, winner = max(groups.items(), key=lambda item: item[1]["weight"]) share = winner["weight"] / total_weight - if winner["weight"] >= min_weight and share >= min_share and winner["has_non_ai"]: + if winner["weight"] >= min_weight and share >= min_share and winner["has_human_backed"]: return winning_key return None -__all__ = ["VoteTuple", "resolve_weighted_consensus"] +__all__ = ["VoteTuple", "resolve_weighted_consensus", "_SOURCE_WEIGHTS"] diff --git a/docs/federation-v1.md b/docs/federation-v1.md new file mode 100644 index 000000000..bae197417 --- /dev/null +++ b/docs/federation-v1.md @@ -0,0 +1,56 @@ +# Federation verdict exchange — format v1 (spec; implementation pending) + +Instances share **resolved consensus verdicts** (never raw votes) as signed +JSON. Each importing instance ingests peer verdicts as `source='federated'` +votes, subject to its own consensus thresholds and admin override. Instances +export only locally-resolved consensus — never re-broadcast federated votes +(no transitive trust / echo amplification). + +## File shape + + { + "schema_version": 1, + "instance": "", + "public_key": "", + "generated_at": "", + "game": "MTG", + "verdicts": [ + { + "image": { + "drive_id": "", + "content_hash": null, // reserved: perceptual hash, v1 nullable + "name": "" + }, + "kind": "printing" | "artist" | "tag", + "outcome": "scryfall:" | "no_match" + | "artist:" | "artist:unknown" + | "tag::apply" | "tag::reject", + "resolved_at": "", + "vote_weight": , + "human_votes": = 1 as the condition for the imported + vote to be human-backed for gate purposes> + } + ], + "signature": "" + } + +## Interchange keys & stability contract +- Images join across instances by `drive_id` (Google Drive file id). + `content_hash` is the planned upgrade path for surviving re-uploads. +- Artists travel by canonical name; tags by `Tag.name`. **Tag names are + therefore a cross-instance contract: renaming a Tag is a breaking data + migration, not an edit.** + +## Import rules (normative for future implementation) +- Verify signature against a pinned per-peer public key (out-of-band + exchange; no open enrollment). +- One voice per peer per (image, kind[, tag]): re-import replaces that + peer's prior vote, never stacks. +- Weight: settings.VOTE_FEDERATED_WEIGHT, per-peer override permitted. +- Skip verdicts whose artist/tag can't be matched locally. + +Status: format committed ahead of code. Export/import commands, keygen, and +the peer registry are future work; the schema stub (source='federated', +peer field, weight setting, is_human_backed plumbing) ships now. From a37214d49645720b94fd0758c23d1925ffcec2a8 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Sun, 12 Jul 2026 21:05:11 +0000 Subject: [PATCH 4/6] Generalize contested-review to artist/tag votes, add unified vote queue Backend half of the review-tooling sprint: extracts printing-tag voting's "contested" detection into a shared, behavior-preserving vote_consensus.contested_queryset, used by new get_contested_artist_card_ids/get_contested_tag_pairs alongside the unchanged get_contested_card_ids. Persists this as real state instead of a cheap proxy where it matters: ArtistVoteStatus gains CONTESTED, and Card.tag_vote_statuses (a new JSONField) tracks resolved/ contested/unresolved per tag, written by the existing resolve_and_persist_tag_votes. New POST 2/voteQueue/ endpoint (a sibling of 2/printingTagQueue/, which is untouched) generalizes the review queue across kind= printing|artist|tag - printing keeps its exact existing candidate set/ordering; artist mirrors it; tag mode serves (card, tag) pairs filtered on the persisted status (not raw vote existence, so a resolved pair can't resurface from unrelated later votes), ordered by ascending net polarity weight with a card_id round-robin interleave so review doesn't get stuck repeating one card. Also rewrites Card.serialise()'s artist-fallback chain to capture which rung actually supplied the value as it's found, rather than inferring it after the fact by checking which other fields are empty - adds canonicalArtistIsFromVoteOnly/canonicalArtistSource to the Card schema, needed by the "wrong?" affordance landing next. Admin gets ContestedArtistFilter/ContestedTagFilter (mirroring the existing printing one) and peer visibility on both vote models. Co-Authored-By: Claude Sonnet 5 --- MPCAutofill/cardpicker/admin.py | 69 +++++-- MPCAutofill/cardpicker/artist_consensus.py | 43 +++- ..._statuses_alter_card_artist_vote_status.py | 33 +++ MPCAutofill/cardpicker/models.py | 60 ++++-- MPCAutofill/cardpicker/printing_consensus.py | 16 +- MPCAutofill/cardpicker/schema_types.py | 109 +++++++++- MPCAutofill/cardpicker/tag_consensus.py | 130 ++++++++++-- .../tests/__snapshots__/test_views.ambr | 132 ++++++++++++ .../cardpicker/tests/test_artist_votes.py | 52 +++++ .../cardpicker/tests/test_tag_votes.py | 154 +++++++++++++- .../cardpicker/tests/test_vote_queue_views.py | 189 ++++++++++++++++++ MPCAutofill/cardpicker/urls.py | 1 + MPCAutofill/cardpicker/views.py | 71 +++++++ MPCAutofill/cardpicker/vote_consensus.py | 41 +++- frontend/src/common/schema_types.ts | 96 ++++++++- schemas/schemas/Card.json | 11 +- schemas/schemas/VoteQueueItem.json | 11 + .../schemas/endpoints/VoteQueueRequest.json | 11 + .../schemas/endpoints/VoteQueueResponse.json | 15 ++ 19 files changed, 1183 insertions(+), 61 deletions(-) create mode 100644 MPCAutofill/cardpicker/migrations/0055_card_tag_vote_statuses_alter_card_artist_vote_status.py create mode 100644 MPCAutofill/cardpicker/tests/test_vote_queue_views.py create mode 100644 schemas/schemas/VoteQueueItem.json create mode 100644 schemas/schemas/endpoints/VoteQueueRequest.json create mode 100644 schemas/schemas/endpoints/VoteQueueResponse.json diff --git a/MPCAutofill/cardpicker/admin.py b/MPCAutofill/cardpicker/admin.py index 13b9e5b9f..5df2ca559 100644 --- a/MPCAutofill/cardpicker/admin.py +++ b/MPCAutofill/cardpicker/admin.py @@ -1,7 +1,11 @@ +from functools import reduce +from operator import or_ + from django.contrib import admin -from django.db.models import QuerySet +from django.db.models import Q, QuerySet from django.http import HttpRequest +from .artist_consensus import get_contested_artist_card_ids from .models import ( CanonicalArtist, CanonicalCard, @@ -21,6 +25,7 @@ ) from .printing_consensus import get_contested_card_ids from .sources.update_database import update_database +from .tag_consensus import get_contested_tag_pairs # Register your models here. @@ -133,26 +138,64 @@ class AdminCardPrintingTag(admin.ModelAdmin[CardPrintingTag]): raw_id_fields = ["card", "printing"] +class ContestedArtistFilter(admin.SimpleListFilter): + """Admin-triage wrapper around `cardpicker.artist_consensus.get_contested_artist_card_ids` - + mirrors `ContestedCardFilter` exactly, generalized to artist votes.""" + + title = "contested" + parameter_name = "contested" + + def lookups(self, request: HttpRequest, model_admin: admin.ModelAdmin[CardArtistVote]) -> list[tuple[str, str]]: + return [("yes", "Yes")] + + def queryset(self, request: HttpRequest, queryset: QuerySet[CardArtistVote]) -> QuerySet[CardArtistVote]: + if self.value() != "yes": + return queryset + return queryset.filter(card_id__in=get_contested_artist_card_ids()) + + +class ContestedTagFilter(admin.SimpleListFilter): + """Admin-triage wrapper around `cardpicker.tag_consensus.get_contested_tag_pairs` - same + idea as `ContestedCardFilter`, but the unit is a (card, tag) pair rather than just a card, + so the queryset filter is an OR of per-pair conditions rather than a plain `card_id__in`.""" + + title = "contested" + parameter_name = "contested" + + def lookups(self, request: HttpRequest, model_admin: admin.ModelAdmin[CardTagVote]) -> list[tuple[str, str]]: + return [("yes", "Yes")] + + def queryset(self, request: HttpRequest, queryset: QuerySet[CardTagVote]) -> QuerySet[CardTagVote]: + if self.value() != "yes": + return queryset + pairs = get_contested_tag_pairs() + if not pairs: + return queryset.none() + condition = reduce(or_, (Q(card_id=card_id, tag_id=tag_id) for card_id, tag_id in pairs)) + return queryset.filter(condition) + + @admin.register(CardArtistVote) class AdminCardArtistVote(admin.ModelAdmin[CardArtistVote]): - """ - Plain registration, no `ContestedCardFilter`-equivalent yet - generalizing the - contested-first review queue to artist/tag votes is a deferred follow-up, not part of - this stage (see `cardpicker.artist_consensus`/`cardpicker.tag_consensus`). - """ - - list_display = ("card", "artist", "is_unknown", "source", "confidence", "anonymous_id", "created_at") - list_filter = ("source", "is_unknown") + list_display = ( + "card", + "artist", + "is_unknown", + "source", + "peer", + "confidence", + "anonymous_id", + "created_at", + ) + list_filter = ("source", "is_unknown", "peer", ContestedArtistFilter) search_fields = ("card__name",) raw_id_fields = ["card", "artist"] @admin.register(CardTagVote) class AdminCardTagVote(admin.ModelAdmin[CardTagVote]): - """Plain registration - see `AdminCardArtistVote`'s docstring for the deferred-filter note.""" - - list_display = ("card", "tag", "polarity", "source", "confidence", "anonymous_id", "created_at") - list_filter = ("source", "polarity") + list_display = ("card", "tag", "polarity", "source", "peer", "confidence", "anonymous_id", "created_at") + list_filter = ("source", "polarity", "peer", ContestedTagFilter) search_fields = ("card__name", "tag__name") raw_id_fields = ["card", "tag"] diff --git a/MPCAutofill/cardpicker/artist_consensus.py b/MPCAutofill/cardpicker/artist_consensus.py index 8592e1ec3..92e6b0e2d 100644 --- a/MPCAutofill/cardpicker/artist_consensus.py +++ b/MPCAutofill/cardpicker/artist_consensus.py @@ -2,10 +2,17 @@ from django.conf import settings -from cardpicker.models import ArtistVoteStatus, CanonicalArtist, Card, VoteSource +from cardpicker.models import ( + ArtistVoteStatus, + CanonicalArtist, + Card, + CardArtistVote, + VoteSource, +) from cardpicker.vote_consensus import ( _SOURCE_WEIGHTS, VoteTuple, + contested_queryset, resolve_weighted_consensus, ) @@ -67,11 +74,22 @@ def resolve_and_persist_artist(card: Card) -> CanonicalArtist | Literal["UNKNOWN `card.printing_tag_status` at all: the precedence rule ("a resolved printing's artist wins") is enforced entirely by `Card.serialise()`'s fallback chain, not here, so this function stays decoupled from printing-tag state. + + When unresolved, additionally distinguishes `CONTESTED` (more than one distinct outcome + has votes) from plain `UNRESOLVED` (not enough votes yet to conclude anything) - a second, + lightweight query, only taken on this branch, so the common resolved case pays nothing + extra for it. """ result = resolve_artist(card) if result is None: + distinct_outcomes = { + UNKNOWN if is_unknown else artist_id + for is_unknown, artist_id in card.artist_votes.values_list("is_unknown", "artist_id") + } card.inferred_canonical_artist = None - card.artist_vote_status = ArtistVoteStatus.UNRESOLVED + card.artist_vote_status = ( + ArtistVoteStatus.CONTESTED if len(distinct_outcomes) > 1 else ArtistVoteStatus.UNRESOLVED + ) elif result == UNKNOWN: card.inferred_canonical_artist = None card.artist_vote_status = ArtistVoteStatus.UNKNOWN @@ -108,4 +126,23 @@ def get_artist_vote_tally(card: Card) -> list[ArtistVoteTallyEntry]: return sorted(tally.values(), key=lambda entry: entry["count"], reverse=True) -__all__ = ["UNKNOWN", "resolve_artist", "resolve_and_persist_artist", "get_artist_vote_tally", "ArtistVoteTallyEntry"] +def get_contested_artist_card_ids() -> list[int]: + """ + IDs of cards with conflicting artist votes on record - mirrors + `cardpicker.printing_consensus.get_contested_card_ids` exactly, generalized via + `vote_consensus.contested_queryset`. See that function's docstring for what "contested" + means here and why this is a cheap proxy, not a full consensus recomputation. + """ + return contested_queryset( + CardArtistVote.objects.all(), group_by="card_id", outcome_field="artist_id", sentinel_field="is_unknown" + ) + + +__all__ = [ + "UNKNOWN", + "resolve_artist", + "resolve_and_persist_artist", + "get_artist_vote_tally", + "get_contested_artist_card_ids", + "ArtistVoteTallyEntry", +] diff --git a/MPCAutofill/cardpicker/migrations/0055_card_tag_vote_statuses_alter_card_artist_vote_status.py b/MPCAutofill/cardpicker/migrations/0055_card_tag_vote_statuses_alter_card_artist_vote_status.py new file mode 100644 index 000000000..26de96bc6 --- /dev/null +++ b/MPCAutofill/cardpicker/migrations/0055_card_tag_vote_statuses_alter_card_artist_vote_status.py @@ -0,0 +1,33 @@ +# Generated by Django 4.2.30 on 2026-07-12 20:24 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("cardpicker", "0054_cardartistvote_peer_cardprintingtag_peer_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="card", + name="tag_vote_statuses", + field=models.JSONField(blank=True, default=dict), + ), + migrations.AlterField( + model_name="card", + name="artist_vote_status", + field=models.CharField( + choices=[ + ("unresolved", "Unresolved"), + ("resolved", "Resolved"), + ("unknown", "Unknown"), + ("contested", "Contested"), + ], + db_index=True, + default="unresolved", + max_length=10, + ), + ), + ] diff --git a/MPCAutofill/cardpicker/models.py b/MPCAutofill/cardpicker/models.py index f88058222..7cb0852c0 100755 --- a/MPCAutofill/cardpicker/models.py +++ b/MPCAutofill/cardpicker/models.py @@ -318,6 +318,21 @@ class ArtistVoteStatus(models.TextChoices): UNRESOLVED = "unresolved", gettext_lazy("Unresolved") RESOLVED = "resolved", gettext_lazy("Resolved") UNKNOWN = "unknown", gettext_lazy("Unknown") + CONTESTED = "contested", gettext_lazy("Contested") + + +class TagVoteStatus(models.TextChoices): + """ + Per-tag status stored in `Card.tag_vote_statuses` (a JSONField, not a plain model field - + see that field's own comment for why - so this isn't wired up as a `choices=` kwarg + anywhere, just symbolic constants for `cardpicker.tag_consensus` to use instead of raw + strings). Written by `resolve_and_persist_tag_votes`. + """ + + RESOLVED_APPLY = "resolved_apply", gettext_lazy("Resolved (apply)") + RESOLVED_REJECT = "resolved_reject", gettext_lazy("Resolved (reject)") + CONTESTED = "contested", gettext_lazy("Contested") + UNRESOLVED = "unresolved", gettext_lazy("Unresolved") class Card(models.Model): @@ -356,6 +371,15 @@ class Card(models.Model): artist_vote_status = models.CharField( max_length=10, choices=ArtistVoteStatus.choices, default=ArtistVoteStatus.UNRESOLVED, db_index=True ) + # Per-tag vote status, written by cardpicker.tag_consensus.resolve_and_persist_tag_votes: + # {tag.name: "resolved_apply" | "resolved_reject" | "contested" | "unresolved"}. An absent + # key means no votes at all for that tag on this card - entries are never written for a + # tag with zero votes. Bookkeeping alongside the existing `tags` array/overlay-merge logic + # above, not a replacement for it. INVARIANT: keys are `Tag.name` values, which must stay + # stable - renaming a Tag orphans its entries here and (per docs/federation-v1.md) breaks + # cross-instance verdict portability, since tags travel by name in that format too. A Tag + # rename is a data migration, not a plain edit. + tag_vote_statuses = models.JSONField(default=dict, blank=True) # a lowercase CanonicalExpansion.code guessed from a lone set-code bracket token in the # source filename (e.g. "[MH3]") - not resolved to a specific printing (no collector # number was present to pair with it), just a ranking hint for get_ranked_printing_candidates @@ -374,6 +398,26 @@ def __str__(self) -> str: ) def serialise(self) -> SerialisedCard: + # Explicit if/elif chain (rather than a nested-ternary fallback) so the rung that + # actually supplied the artist is captured as it's found, not re-derived afterwards by + # checking which other fields are empty - that "all others empty" style of check would + # silently misclassify if this chain ever grows a fifth rung. `canonicalArtistIsFromVoteOnly` + # (used by the frontend's "wrong?" affordance to distinguish a confidently-known artist + # from a vote-derived one) and the debug-only `canonicalArtistSource` field both derive + # directly from `artist_source`, so they can never drift out of sync with this chain. + artist_source: str | None + resolved_artist: CanonicalArtist | None + if self.canonical_artist is not None: + artist_source, resolved_artist = "canonical_artist", self.canonical_artist + elif self.canonical_card is not None: + artist_source, resolved_artist = "canonical_card", self.canonical_card.artist + elif self.inferred_canonical_card is not None: + artist_source, resolved_artist = "inferred_canonical_card", self.inferred_canonical_card.artist + elif self.inferred_canonical_artist is not None: + artist_source, resolved_artist = "inferred_canonical_artist", self.inferred_canonical_artist + else: + artist_source, resolved_artist = None, None + return SerialisedCard( identifier=self.identifier, cardType=CardType(self.card_type), @@ -401,19 +445,9 @@ def serialise(self) -> SerialisedCard: if self.canonical_card else (self.inferred_canonical_card.serialise() if self.inferred_canonical_card else None) ), - canonicalArtist=( - self.canonical_artist.serialise() - if self.canonical_artist - else ( - self.canonical_card.artist.serialise() - if self.canonical_card - else ( - self.inferred_canonical_card.artist.serialise() - if self.inferred_canonical_card - else (self.inferred_canonical_artist.serialise() if self.inferred_canonical_artist else None) - ) - ) - ), + canonicalArtist=resolved_artist.serialise() if resolved_artist is not None else None, + canonicalArtistIsFromVoteOnly=artist_source == "inferred_canonical_artist", + canonicalArtistSource=artist_source, ) def to_dict(self) -> dict[str, Any]: diff --git a/MPCAutofill/cardpicker/printing_consensus.py b/MPCAutofill/cardpicker/printing_consensus.py index 1a7f2153b..0c98ee735 100644 --- a/MPCAutofill/cardpicker/printing_consensus.py +++ b/MPCAutofill/cardpicker/printing_consensus.py @@ -1,7 +1,6 @@ from typing import Literal, TypedDict from django.conf import settings -from django.db.models import Case, Count, IntegerField, Q, When from cardpicker.models import ( CanonicalCard, @@ -13,6 +12,7 @@ from cardpicker.vote_consensus import ( _SOURCE_WEIGHTS, VoteTuple, + contested_queryset, resolve_weighted_consensus, ) @@ -106,15 +106,13 @@ def get_contested_card_ids() -> list[int]: Materialized to a plain list (rather than returning the lazy QuerySet) since the set of actually-contested cards is always a small fraction of the total - cheap to evaluate eagerly, and sidesteps django-stubs' QuerySet generic entirely for callers. + + Delegates to the shared `vote_consensus.contested_queryset` - this function's name, + signature, and behavior are unchanged; it's the reference point that function's own + docstring calls "behavior-preserving". """ - return list( - CardPrintingTag.objects.values("card_id") - .annotate( - distinct_printings=Count("printing_id", distinct=True), - has_no_match=Count(Case(When(is_no_match=True, then=1), output_field=IntegerField())), - ) - .filter(Q(distinct_printings__gt=1) | (Q(distinct_printings__gte=1) & Q(has_no_match__gt=0))) - .values_list("card_id", flat=True) + return contested_queryset( + CardPrintingTag.objects.all(), group_by="card_id", outcome_field="printing_id", sentinel_field="is_no_match" ) diff --git a/MPCAutofill/cardpicker/schema_types.py b/MPCAutofill/cardpicker/schema_types.py index 90fe41e47..32d0bf5d0 100644 --- a/MPCAutofill/cardpicker/schema_types.py +++ b/MPCAutofill/cardpicker/schema_types.py @@ -5,9 +5,9 @@ """ from enum import Enum -from typing import Any, Callable, Dict, List, Optional, Type, TypeVar, Union, cast - from pydantic import BaseModel +from typing import Optional, Any, List, Union, Dict, TypeVar, Callable, Type, cast + T = TypeVar("T") EnumT = TypeVar("EnumT", bound=Enum) @@ -394,6 +394,12 @@ class SourceType(str, Enum): class Card(BaseModel): + canonicalArtistIsFromVoteOnly: bool + """True only when canonicalArtist was supplied by artist-vote consensus alone, with no + confirmed indexing match or resolved printing backing it - lets the frontend distinguish + a confidently-known artist from a vote-derived one (e.g. for the ArtistVotePicker + 'wrong?' affordance) without needing to know serialise()'s fallback chain itself. + """ cardType: CardType dateCreated: str """Created date - formatted by backend""" @@ -417,6 +423,10 @@ class Card(BaseModel): sourceVerbose: str tags: List[str] canonicalArtist: Optional[CanonicalArtistClass] = None + canonicalArtistSource: Optional[str] = None + """Which rung of the artist fallback chain actually supplied canonicalArtist - + debug/introspection field, not load-bearing for any current frontend logic. + """ canonicalCard: Optional[CanonicalCardClass] = None sourceExternalLink: Optional[str] = None sourceType: Optional[SourceType] = None @@ -424,6 +434,7 @@ class Card(BaseModel): @staticmethod def from_dict(obj: Any) -> "Card": assert isinstance(obj, dict) + canonicalArtistIsFromVoteOnly = from_bool(obj.get("canonicalArtistIsFromVoteOnly")) cardType = CardType(obj.get("cardType")) dateCreated = from_str(obj.get("dateCreated")) dateModified = from_str(obj.get("dateModified")) @@ -443,10 +454,12 @@ def from_dict(obj: Any) -> "Card": sourceVerbose = from_str(obj.get("sourceVerbose")) tags = from_list(from_str, obj.get("tags")) canonicalArtist = from_union([from_none, CanonicalArtistClass.from_dict], obj.get("canonicalArtist")) + canonicalArtistSource = from_union([from_none, from_str], obj.get("canonicalArtistSource")) canonicalCard = from_union([from_none, CanonicalCardClass.from_dict], obj.get("canonicalCard")) sourceExternalLink = from_union([from_str, from_none], obj.get("sourceExternalLink")) sourceType = from_union([SourceType, from_none], obj.get("sourceType")) return Card( + canonicalArtistIsFromVoteOnly, cardType, dateCreated, dateModified, @@ -466,6 +479,7 @@ def from_dict(obj: Any) -> "Card": sourceVerbose, tags, canonicalArtist, + canonicalArtistSource, canonicalCard, sourceExternalLink, sourceType, @@ -473,6 +487,7 @@ def from_dict(obj: Any) -> "Card": def to_dict(self) -> dict: result: dict = {} + result["canonicalArtistIsFromVoteOnly"] = from_bool(self.canonicalArtistIsFromVoteOnly) result["cardType"] = to_enum(CardType, self.cardType) result["dateCreated"] = from_str(self.dateCreated) result["dateModified"] = from_str(self.dateModified) @@ -495,6 +510,8 @@ def to_dict(self) -> dict: result["canonicalArtist"] = from_union( [from_none, lambda x: to_class(CanonicalArtistClass, x)], self.canonicalArtist ) + if self.canonicalArtistSource is not None: + result["canonicalArtistSource"] = from_union([from_none, from_str], self.canonicalArtistSource) if self.canonicalCard is not None: result["canonicalCard"] = from_union( [from_none, lambda x: to_class(CanonicalCardClass, x)], self.canonicalCard @@ -1559,6 +1576,70 @@ def to_dict(self) -> dict: return result +class Kind(str, Enum): + artist = "artist" + printing = "printing" + tag = "tag" + + +class VoteQueueRequest(BaseModel): + kind: Kind + page: int + + @staticmethod + def from_dict(obj: Any) -> "VoteQueueRequest": + assert isinstance(obj, dict) + kind = Kind(obj.get("kind")) + page = from_int(obj.get("page")) + return VoteQueueRequest(kind, page) + + def to_dict(self) -> dict: + result: dict = {} + result["kind"] = to_enum(Kind, self.kind) + result["page"] = from_int(self.page) + return result + + +class VoteQueueItem(BaseModel): + card: Card + tagName: Optional[str] = None + + @staticmethod + def from_dict(obj: Any) -> "VoteQueueItem": + assert isinstance(obj, dict) + card = Card.from_dict(obj.get("card")) + tagName = from_union([from_none, from_str], obj.get("tagName")) + return VoteQueueItem(card, tagName) + + def to_dict(self) -> dict: + result: dict = {} + result["card"] = to_class(Card, self.card) + if self.tagName is not None: + result["tagName"] = from_union([from_none, from_str], self.tagName) + return result + + +class VoteQueueResponse(BaseModel): + hits: int + items: List[VoteQueueItem] + pages: int + + @staticmethod + def from_dict(obj: Any) -> "VoteQueueResponse": + assert isinstance(obj, dict) + hits = from_int(obj.get("hits")) + items = from_list(VoteQueueItem.from_dict, obj.get("items")) + pages = from_int(obj.get("pages")) + return VoteQueueResponse(hits, items, pages) + + def to_dict(self) -> dict: + result: dict = {} + result["hits"] = from_int(self.hits) + result["items"] = from_list(lambda x: to_class(VoteQueueItem, x), self.items) + result["pages"] = from_int(self.pages) + return result + + def ArtistVoteTallyEntryfromdict(s: Any) -> ArtistVoteTallyEntry: return ArtistVoteTallyEntry.from_dict(s) @@ -1767,6 +1848,14 @@ def TagVoteTallyEntrytodict(x: TagVoteTallyEntry) -> Any: return to_class(TagVoteTallyEntry, x) +def VoteQueueItemfromdict(s: Any) -> VoteQueueItem: + return VoteQueueItem.from_dict(s) + + +def VoteQueueItemtodict(x: VoteQueueItem) -> Any: + return to_class(VoteQueueItem, x) + + def VoteTallyEntryfromdict(s: Any) -> VoteTallyEntry: return VoteTallyEntry.from_dict(s) @@ -2085,3 +2174,19 @@ def TagsResponsefromdict(s: Any) -> TagsResponse: def TagsResponsetodict(x: TagsResponse) -> Any: return to_class(TagsResponse, x) + + +def VoteQueueRequestfromdict(s: Any) -> VoteQueueRequest: + return VoteQueueRequest.from_dict(s) + + +def VoteQueueRequesttodict(x: VoteQueueRequest) -> Any: + return to_class(VoteQueueRequest, x) + + +def VoteQueueResponsefromdict(s: Any) -> VoteQueueResponse: + return VoteQueueResponse.from_dict(s) + + +def VoteQueueResponsetodict(x: VoteQueueResponse) -> Any: + return to_class(VoteQueueResponse, x) diff --git a/MPCAutofill/cardpicker/tag_consensus.py b/MPCAutofill/cardpicker/tag_consensus.py index 9147c7c3c..0f19e30fc 100644 --- a/MPCAutofill/cardpicker/tag_consensus.py +++ b/MPCAutofill/cardpicker/tag_consensus.py @@ -1,12 +1,20 @@ -from collections import defaultdict +from collections import defaultdict, deque from typing import Iterable, TypedDict from django.conf import settings -from cardpicker.models import Card, CardTagVote, Tag, VotePolarity, VoteSource +from cardpicker.models import ( + Card, + CardTagVote, + Tag, + TagVoteStatus, + VotePolarity, + VoteSource, +) from cardpicker.vote_consensus import ( _SOURCE_WEIGHTS, VoteTuple, + contested_queryset, resolve_weighted_consensus, ) @@ -49,31 +57,57 @@ def resolve_and_persist_tag_votes(card: Card) -> None: printing/artist consensus (whose denormalised fields aren't ES-indexed), `tags` *is* an ES-indexed field (`documents.py`'s `KeywordField`), so a vote-triggered change has to reach the search index directly rather than waiting for the next scheduled re-scan. + + Also writes `card.tag_vote_statuses` (a JSONField, not ES-indexed, so no re-index needed + for this part alone): for every voted tag, one of RESOLVED_APPLY/RESOLVED_REJECT/CONTESTED/ + UNRESOLVED. CONTESTED vs. UNRESOLVED is distinguished locally from the polarities already + fetched below (no extra query) - contested means both polarities are present with votes; + unresolved means only one side has voted so far, or thresholds simply aren't cleared yet. """ from cardpicker.documents import ( CardSearch, # local import - avoids a top-level ES dependency in this module ) - voted_tag_ids = list(card.tag_votes.values_list("tag_id", flat=True).distinct()) - if not voted_tag_ids: + votes_by_tag_id: dict[int, set[int]] = defaultdict(set) + for tag_id, polarity in card.tag_votes.values_list("tag_id", "polarity"): + votes_by_tag_id[tag_id].add(polarity) + if not votes_by_tag_id: return - tags_by_id = {tag.pk: tag for tag in Tag.objects.filter(pk__in=voted_tag_ids)} + tags_by_id = {tag.pk: tag for tag in Tag.objects.filter(pk__in=votes_by_tag_id.keys())} current_tags = set(card.tags) - changed = False + statuses = dict(card.tag_vote_statuses) + tags_changed = False + statuses_changed = False for tag_id, tag in tags_by_id.items(): resolved = resolve_tag(card, tag) - if resolved == VotePolarity.APPLY and tag.name not in current_tags: - current_tags.add(tag.name) - changed = True - elif resolved == VotePolarity.NOT_APPLICABLE and tag.name in current_tags: - current_tags.discard(tag.name) - changed = True - - if changed: + if resolved == VotePolarity.APPLY: + new_status = TagVoteStatus.RESOLVED_APPLY + if tag.name not in current_tags: + current_tags.add(tag.name) + tags_changed = True + elif resolved == VotePolarity.NOT_APPLICABLE: + new_status = TagVoteStatus.RESOLVED_REJECT + if tag.name in current_tags: + current_tags.discard(tag.name) + tags_changed = True + else: + new_status = TagVoteStatus.CONTESTED if len(votes_by_tag_id[tag_id]) > 1 else TagVoteStatus.UNRESOLVED + if statuses.get(tag.name) != new_status: + statuses[tag.name] = new_status + statuses_changed = True + + update_fields = [] + if tags_changed: card.tags = sorted(current_tags) - card.save(update_fields=["tags"]) - CardSearch().update([card], action="index") + update_fields.append("tags") + if statuses_changed: + card.tag_vote_statuses = statuses + update_fields.append("tag_vote_statuses") + if update_fields: + card.save(update_fields=update_fields) + if tags_changed: + CardSearch().update([card], action="index") class TagVoteTallyEntry(TypedDict): @@ -129,10 +163,74 @@ def get_resolved_tag_overlay(card_ids: Iterable[int]) -> dict[int, dict[str, int return dict(overlay) +def get_contested_tag_pairs() -> list[tuple[int, int]]: + """ + (card_id, tag_id) pairs with conflicting tag votes on record - both polarities present. + Mirrors `cardpicker.printing_consensus.get_contested_card_ids`'s shape, generalized via + `vote_consensus.contested_queryset`. Unlike printing/artist, tags have no sentinel outcome + (polarity only ever takes two values), so "contested" here just means both are present. + """ + return contested_queryset(CardTagVote.objects.all(), group_by=["card_id", "tag_id"], outcome_field="polarity") + + +def get_tag_review_queue_pairs() -> list[tuple[int, str]]: + """ + (card_id, tag_name) pairs still needing review for the tag-mode vote queue, backing + `POST 2/voteQueue/?kind=tag`. + + Candidate set is the *persisted* `Card.tag_vote_statuses` state (CONTESTED/UNRESOLVED + entries), not raw `CardTagVote` existence - so a pair that's already resolved + (RESOLVED_APPLY/RESOLVED_REJECT) stays out of the queue even as unrelated new votes trickle + in on the same card afterwards. `tag_vote_statuses` is a small per-card JSON dict with no + native per-key DB filter for "any key has value X", so this materializes candidates eagerly + in Python - same rationale as `vote_consensus.contested_queryset`: the candidate set is + always a small fraction of the catalogue. + + Ordering: primarily by ascending absolute net polarity weight (a 5-vs-4 split outranks a + 6-vs-1 split - closest contests first), then interleaved by card_id (round-robin across + each card's own items, preserving each card's internal relative order) so the same card + isn't served back-to-back for different tags when a different card's item could go in + between instead. + """ + candidates: list[tuple[int, str]] = [ + (card_id, tag_name) + for card_id, statuses in Card.objects.exclude(tag_vote_statuses={}).values_list("id", "tag_vote_statuses") + for tag_name, status in statuses.items() + if status in (TagVoteStatus.CONTESTED, TagVoteStatus.UNRESOLVED) + ] + if not candidates: + return [] + + card_ids = {card_id for card_id, _ in candidates} + rows = CardTagVote.objects.filter(card_id__in=card_ids).values("card_id", "tag__name", "source", "polarity") + net_weight: dict[tuple[int, str], float] = defaultdict(float) + for row in rows: + net_weight[(row["card_id"], row["tag__name"])] += row["polarity"] * _SOURCE_WEIGHTS[row["source"]] + candidates.sort(key=lambda pair: abs(net_weight.get(pair, 0.0))) + + grouped: dict[int, deque[tuple[int, str]]] = defaultdict(deque) + group_order: list[int] = [] + for pair in candidates: + card_id = pair[0] + if card_id not in grouped: + group_order.append(card_id) + grouped[card_id].append(pair) + interleaved: list[tuple[int, str]] = [] + remaining = list(group_order) + while remaining: + for card_id in list(remaining): + interleaved.append(grouped[card_id].popleft()) + if not grouped[card_id]: + remaining.remove(card_id) + return interleaved + + __all__ = [ "resolve_tag", "resolve_and_persist_tag_votes", "get_tag_vote_tally", "get_resolved_tag_overlay", + "get_contested_tag_pairs", + "get_tag_review_queue_pairs", "TagVoteTallyEntry", ] diff --git a/MPCAutofill/cardpicker/tests/__snapshots__/test_views.ambr b/MPCAutofill/cardpicker/tests/__snapshots__/test_views.ambr index ac2ac5559..b4252746a 100644 --- a/MPCAutofill/cardpicker/tests/__snapshots__/test_views.ambr +++ b/MPCAutofill/cardpicker/tests/__snapshots__/test_views.ambr @@ -195,6 +195,8 @@ 'canonicalArtist': dict({ 'name': 'Artist 102', }), + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': 'canonical_card', 'canonicalCard': dict({ 'artist': None, 'canonicalId': '36cd2364-d113-47d1-b2c4-b088d9eb88dd', @@ -229,6 +231,8 @@ }), 'Island': dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -254,6 +258,8 @@ }), 'Island (William Bradford)': dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -279,6 +285,8 @@ }), 'Pást in Flames': dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -309,6 +317,8 @@ 'TOKEN': dict({ 'Goblin': dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'TOKEN', 'dateCreated': '1st January, 2023', @@ -418,6 +428,8 @@ 'canonicalArtist': dict({ 'name': 'Artist 107', }), + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': 'canonical_card', 'canonicalCard': dict({ 'artist': None, 'canonicalId': '36cd2364-d113-47d1-b2c4-b088d9eb88dd', @@ -452,6 +464,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -477,6 +491,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'TOKEN', 'dateCreated': '1st January, 2023', @@ -502,6 +518,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -527,6 +545,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -552,6 +572,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -591,6 +613,8 @@ 'cards': list([ dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -618,6 +642,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARDBACK', 'dateCreated': '1st January, 2023', @@ -704,6 +730,8 @@ 'canonicalArtist': dict({ 'name': 'Artist 109', }), + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': 'canonical_card', 'canonicalCard': dict({ 'artist': None, 'canonicalId': '36cd2364-d113-47d1-b2c4-b088d9eb88dd', @@ -738,6 +766,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -763,6 +793,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'TOKEN', 'dateCreated': '1st January, 2023', @@ -788,6 +820,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -813,6 +847,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -838,6 +874,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -872,6 +910,8 @@ 'cards': list([ dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -897,6 +937,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -922,6 +964,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -948,6 +992,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -973,6 +1019,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARDBACK', 'dateCreated': '1st January, 2023', @@ -1009,6 +1057,8 @@ 'cards': list([ dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -1036,6 +1086,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARDBACK', 'dateCreated': '1st January, 2023', @@ -1412,6 +1464,8 @@ 'results': dict({ '17fopRCNRge72U8Hac8pApHZtEalx5kHy': dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -1437,6 +1491,8 @@ }), '1V5E0avDmNyEUuFfYwx3nA05aj-1HY0rA': dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'TOKEN', 'dateCreated': '1st January, 2023', @@ -1481,6 +1537,8 @@ 'results': dict({ '1V5E0avDmNyEUuFfYwx3nA05aj-1HY0rA': dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'TOKEN', 'dateCreated': '1st January, 2023', @@ -1515,6 +1573,8 @@ 'results': dict({ '17fopRCNRge72U8Hac8pApHZtEalx5kHy': dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -1540,6 +1600,8 @@ }), '1991MWCur9NdAFi-tQQD5YbQj2oqV_WRy': dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -1565,6 +1627,8 @@ }), '1V5E0avDmNyEUuFfYwx3nA05aj-1HY0rA': dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'TOKEN', 'dateCreated': '1st January, 2023', @@ -1609,6 +1673,8 @@ 'results': dict({ '1V5E0avDmNyEUuFfYwx3nA05aj-1HY0rA': dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'TOKEN', 'dateCreated': '1st January, 2023', @@ -2311,6 +2377,8 @@ 'cards': list([ dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'TOKEN', 'dateCreated': '1st January, 2023', @@ -2336,6 +2404,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARDBACK', 'dateCreated': '1st January, 2023', @@ -2363,6 +2433,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARDBACK', 'dateCreated': '1st January, 2023', @@ -2401,6 +2473,8 @@ 'canonicalArtist': dict({ 'name': 'Artist 67', }), + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': 'canonical_card', 'canonicalCard': dict({ 'artist': None, 'canonicalId': '36cd2364-d113-47d1-b2c4-b088d9eb88dd', @@ -2435,6 +2509,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2460,6 +2536,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2485,6 +2563,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2510,6 +2590,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2535,6 +2617,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2560,6 +2644,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2585,6 +2671,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2612,6 +2700,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2638,6 +2728,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2675,6 +2767,8 @@ 'canonicalArtist': dict({ 'name': 'Artist 66', }), + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': 'canonical_card', 'canonicalCard': dict({ 'artist': None, 'canonicalId': '36cd2364-d113-47d1-b2c4-b088d9eb88dd', @@ -2709,6 +2803,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2734,6 +2830,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'TOKEN', 'dateCreated': '1st January, 2023', @@ -2759,6 +2857,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2784,6 +2884,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2809,6 +2911,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2834,6 +2938,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2859,6 +2965,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2884,6 +2992,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2911,6 +3021,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2937,6 +3049,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARD', 'dateCreated': '1st January, 2023', @@ -2962,6 +3076,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARDBACK', 'dateCreated': '1st January, 2023', @@ -2989,6 +3105,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARDBACK', 'dateCreated': '1st January, 2023', @@ -3027,6 +3145,8 @@ 'canonicalArtist': dict({ 'name': 'Artist 68', }), + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': 'canonical_card', 'canonicalCard': dict({ 'artist': None, 'canonicalId': '36cd2364-d113-47d1-b2c4-b088d9eb88dd', @@ -3071,6 +3191,8 @@ 'cards': list([ dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARDBACK', 'dateCreated': '1st January, 2023', @@ -3107,6 +3229,8 @@ 'cards': list([ dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'TOKEN', 'dateCreated': '1st January, 2023', @@ -3142,6 +3266,8 @@ 'cards': list([ dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARDBACK', 'dateCreated': '1st January, 2023', @@ -3169,6 +3295,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARDBACK', 'dateCreated': '1st January, 2023', @@ -3205,6 +3333,8 @@ 'cards': list([ dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARDBACK', 'dateCreated': '1st January, 2023', @@ -3231,6 +3361,8 @@ }), dict({ 'canonicalArtist': None, + 'canonicalArtistIsFromVoteOnly': False, + 'canonicalArtistSource': None, 'canonicalCard': None, 'cardType': 'CARDBACK', 'dateCreated': '1st January, 2023', diff --git a/MPCAutofill/cardpicker/tests/test_artist_votes.py b/MPCAutofill/cardpicker/tests/test_artist_votes.py index acf9c6053..65c8e52bf 100644 --- a/MPCAutofill/cardpicker/tests/test_artist_votes.py +++ b/MPCAutofill/cardpicker/tests/test_artist_votes.py @@ -7,6 +7,7 @@ from cardpicker.artist_consensus import ( UNKNOWN, get_artist_vote_tally, + get_contested_artist_card_ids, resolve_and_persist_artist, resolve_artist, ) @@ -119,6 +120,30 @@ def test_persists_unresolved(self, db): assert card.inferred_canonical_artist is None assert card.artist_vote_status == ArtistVoteStatus.UNRESOLVED + def test_persists_contested_when_multiple_outcomes_have_votes(self, db): + card = CardFactory() + artist_a = CanonicalArtistFactory() + artist_b = CanonicalArtistFactory() + CardArtistVoteFactory(card=card, artist=artist_a, source=VoteSource.USER) + CardArtistVoteFactory(card=card, artist=artist_b, source=VoteSource.USER) + + result = resolve_and_persist_artist(card) + + assert result is None + card.refresh_from_db() + assert card.artist_vote_status == ArtistVoteStatus.CONTESTED + + def test_persists_unresolved_not_contested_for_a_single_outcome_below_threshold(self, db): + card = CardFactory() + artist = CanonicalArtistFactory() + CardArtistVoteFactory(card=card, artist=artist, source=VoteSource.USER) + + result = resolve_and_persist_artist(card) + + assert result is None + card.refresh_from_db() + assert card.artist_vote_status == ArtistVoteStatus.UNRESOLVED + def test_does_not_consult_printing_tag_status(self, db): # resolve_and_persist_artist is deliberately decoupled from printing_tag_status - the # precedence rule (a resolved printing's artist wins) lives entirely in @@ -149,6 +174,33 @@ def test_tally_groups_by_outcome(self, db): assert {(entry["count"], entry["is_unknown"]) for entry in tally} == {(2, False), (1, True)} +class TestGetContestedArtistCardIds: + def test_multiple_distinct_artists_is_contested(self, db): + card = CardFactory() + artist_a = CanonicalArtistFactory() + artist_b = CanonicalArtistFactory() + CardArtistVoteFactory(card=card, artist=artist_a) + CardArtistVoteFactory(card=card, artist=artist_b) + + assert card.pk in get_contested_artist_card_ids() + + def test_an_artist_vote_alongside_an_unknown_vote_is_contested(self, db): + card = CardFactory() + artist = CanonicalArtistFactory() + CardArtistVoteFactory(card=card, artist=artist) + CardArtistVoteFactory(card=card, artist=None, is_unknown=True) + + assert card.pk in get_contested_artist_card_ids() + + def test_agreeing_votes_are_not_contested(self, db): + card = CardFactory() + artist = CanonicalArtistFactory() + CardArtistVoteFactory(card=card, artist=artist) + CardArtistVoteFactory(card=card, artist=artist) + + assert card.pk not in get_contested_artist_card_ids() + + class TestSerialisePrecedenceChain: def test_falls_back_to_inferred_canonical_artist(self, db): card = CardFactory() diff --git a/MPCAutofill/cardpicker/tests/test_tag_votes.py b/MPCAutofill/cardpicker/tests/test_tag_votes.py index 7ebddb892..f2e09c5a8 100644 --- a/MPCAutofill/cardpicker/tests/test_tag_votes.py +++ b/MPCAutofill/cardpicker/tests/test_tag_votes.py @@ -4,9 +4,11 @@ from django.urls import reverse from cardpicker import views -from cardpicker.models import CardTagVote, VotePolarity, VoteSource +from cardpicker.models import CardTagVote, TagVoteStatus, VotePolarity, VoteSource from cardpicker.tag_consensus import ( + get_contested_tag_pairs, get_resolved_tag_overlay, + get_tag_review_queue_pairs, get_tag_vote_tally, resolve_and_persist_tag_votes, resolve_tag, @@ -118,6 +120,63 @@ def test_multiple_tags_resolve_independently_on_the_same_card(self, db): card.refresh_from_db() assert card.tags == ["Apply Me"] + def test_persists_resolved_apply_status(self, db): + card = CardFactory(tags=[]) + tag = TagFactory(name="Borderless") + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.APPLY, source=VoteSource.ADMIN) + + resolve_and_persist_tag_votes(card) + + card.refresh_from_db() + assert card.tag_vote_statuses == {"Borderless": TagVoteStatus.RESOLVED_APPLY} + + def test_persists_resolved_reject_status(self, db): + card = CardFactory(tags=["Borderless"]) + tag = TagFactory(name="Borderless") + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.NOT_APPLICABLE, source=VoteSource.ADMIN) + + resolve_and_persist_tag_votes(card) + + card.refresh_from_db() + assert card.tag_vote_statuses == {"Borderless": TagVoteStatus.RESOLVED_REJECT} + + def test_persists_contested_when_both_polarities_present(self, db): + card = CardFactory(tags=[]) + tag = TagFactory(name="Borderless") + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.APPLY, source=VoteSource.USER) + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.NOT_APPLICABLE, source=VoteSource.USER) + + resolve_and_persist_tag_votes(card) + + card.refresh_from_db() + assert card.tag_vote_statuses == {"Borderless": TagVoteStatus.CONTESTED} + + def test_persists_unresolved_for_a_single_vote_below_threshold(self, db): + card = CardFactory(tags=[]) + tag = TagFactory(name="Borderless") + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.APPLY, source=VoteSource.USER) + + resolve_and_persist_tag_votes(card) + + card.refresh_from_db() + assert card.tag_vote_statuses == {"Borderless": TagVoteStatus.UNRESOLVED} + + def test_a_resolved_tag_status_survives_unrelated_new_votes_on_a_different_tag(self, db): + # regression guard for the kind=tag queue's "persisted state, not raw vote existence" + # requirement - once a tag resolves, later votes on a *different* tag on the same card + # must not disturb its already-persisted status + card = CardFactory(tags=[]) + resolved_tag = TagFactory(name="Borderless") + other_tag = TagFactory(name="Extended") + CardTagVoteFactory(card=card, tag=resolved_tag, polarity=VotePolarity.APPLY, source=VoteSource.ADMIN) + resolve_and_persist_tag_votes(card) + + CardTagVoteFactory(card=card, tag=other_tag, polarity=VotePolarity.APPLY, source=VoteSource.USER) + resolve_and_persist_tag_votes(card) + + card.refresh_from_db() + assert card.tag_vote_statuses["Borderless"] == TagVoteStatus.RESOLVED_APPLY + class TestGetTagVoteTally: def test_tally_groups_by_polarity(self, db): @@ -155,6 +214,99 @@ def test_unresolved_pairs_are_omitted(self, db): assert overlay == {} +class TestGetContestedTagPairs: + def test_both_polarities_present_is_contested(self, db): + card = CardFactory() + tag = TagFactory() + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.APPLY) + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.NOT_APPLICABLE) + + assert (card.pk, tag.pk) in get_contested_tag_pairs() + + def test_agreeing_votes_are_not_contested(self, db): + card = CardFactory() + tag = TagFactory() + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.APPLY) + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.APPLY) + + assert (card.pk, tag.pk) not in get_contested_tag_pairs() + + def test_contested_pairs_are_scoped_to_the_specific_tag(self, db): + card = CardFactory() + tag_a = TagFactory() + tag_b = TagFactory() + CardTagVoteFactory(card=card, tag=tag_a, polarity=VotePolarity.APPLY) + CardTagVoteFactory(card=card, tag=tag_a, polarity=VotePolarity.NOT_APPLICABLE) + CardTagVoteFactory(card=card, tag=tag_b, polarity=VotePolarity.APPLY) + + pairs = get_contested_tag_pairs() + + assert (card.pk, tag_a.pk) in pairs + assert (card.pk, tag_b.pk) not in pairs + + +class TestGetTagReviewQueuePairs: + def test_resolved_pairs_are_excluded(self, db, settings): + settings.PRINTING_TAG_MIN_VOTES = 1 + settings.PRINTING_TAG_MIN_SHARE = 0.5 + card = CardFactory(tags=[]) + tag = TagFactory(name="Borderless") + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.APPLY, source=VoteSource.ADMIN) + resolve_and_persist_tag_votes(card) + + assert get_tag_review_queue_pairs() == [] + + def test_contested_before_lopsided(self, db): + contested_card = CardFactory(tags=[]) + contested_tag = TagFactory(name="Contested Tag") + CardTagVoteFactory(card=contested_card, tag=contested_tag, polarity=VotePolarity.APPLY) + CardTagVoteFactory(card=contested_card, tag=contested_tag, polarity=VotePolarity.NOT_APPLICABLE) + resolve_and_persist_tag_votes(contested_card) + + lopsided_card = CardFactory(tags=[]) + lopsided_tag = TagFactory(name="Lopsided Tag") + CardTagVoteFactory(card=lopsided_card, tag=lopsided_tag, polarity=VotePolarity.APPLY) + resolve_and_persist_tag_votes(lopsided_card) + + pairs = get_tag_review_queue_pairs() + + assert pairs[0] == (contested_card.pk, "Contested Tag") + + def test_same_card_is_not_served_back_to_back_when_a_different_card_is_available(self, db): + # two cards, each with two tags tied at net weight 0 - symmetric group sizes, so + # round-robin interleaving can (and must) keep every card's items apart for the whole + # sequence, unlike an asymmetric setup where the smaller group exhausting first would + # force the larger group's leftover items to end up adjacent regardless of interleaving + card_a = CardFactory(tags=[]) + tag_1 = TagFactory(name="Tag 1") + tag_2 = TagFactory(name="Tag 2") + CardTagVoteFactory(card=card_a, tag=tag_1, polarity=VotePolarity.APPLY) + CardTagVoteFactory(card=card_a, tag=tag_1, polarity=VotePolarity.NOT_APPLICABLE) + resolve_and_persist_tag_votes(card_a) + CardTagVoteFactory(card=card_a, tag=tag_2, polarity=VotePolarity.APPLY) + CardTagVoteFactory(card=card_a, tag=tag_2, polarity=VotePolarity.NOT_APPLICABLE) + resolve_and_persist_tag_votes(card_a) + + card_b = CardFactory(tags=[]) + tag_3 = TagFactory(name="Tag 3") + tag_4 = TagFactory(name="Tag 4") + CardTagVoteFactory(card=card_b, tag=tag_3, polarity=VotePolarity.APPLY) + CardTagVoteFactory(card=card_b, tag=tag_3, polarity=VotePolarity.NOT_APPLICABLE) + resolve_and_persist_tag_votes(card_b) + CardTagVoteFactory(card=card_b, tag=tag_4, polarity=VotePolarity.APPLY) + CardTagVoteFactory(card=card_b, tag=tag_4, polarity=VotePolarity.NOT_APPLICABLE) + resolve_and_persist_tag_votes(card_b) + + pairs = get_tag_review_queue_pairs() + + card_ids_in_order = [card_id for card_id, _ in pairs] + # neither card's two items are adjacent, since the other card's item is always + # available to interleave with at every step + for card in (card_a, card_b): + positions = [i for i, card_id in enumerate(card_ids_in_order) if card_id == card.pk] + assert positions[1] - positions[0] > 1 + + class TestPostTagConsensus: def test_unknown_card_identifier_is_a_bad_request(self, client, django_settings): response = client.post( diff --git a/MPCAutofill/cardpicker/tests/test_vote_queue_views.py b/MPCAutofill/cardpicker/tests/test_vote_queue_views.py new file mode 100644 index 000000000..91876caf3 --- /dev/null +++ b/MPCAutofill/cardpicker/tests/test_vote_queue_views.py @@ -0,0 +1,189 @@ +import pytest + +from django.urls import reverse + +from cardpicker import views +from cardpicker.models import ( + ArtistVoteStatus, + PrintingTagStatus, + VotePolarity, + VoteSource, +) +from cardpicker.tag_consensus import resolve_and_persist_tag_votes +from cardpicker.tests.factories import ( + CanonicalArtistFactory, + CanonicalCardFactory, + CanonicalExpansionFactory, + CardArtistVoteFactory, + CardFactory, + CardPrintingTagFactory, + CardTagVoteFactory, + SourceFactory, + TagFactory, +) + +# see test_printing_consensus.py for why this capture-and-restore fixture exists +_SHARED_FACTORIES = [ + CardFactory, + SourceFactory, + CanonicalArtistFactory, + CanonicalExpansionFactory, + CanonicalCardFactory, +] + + +@pytest.fixture(autouse=True) +def _preserve_shared_factory_sequences(): + before = {f: f._meta.next_sequence() for f in _SHARED_FACTORIES} + for f, n in before.items(): + f.reset_sequence(n, force=True) + yield + for f, n in before.items(): + f.reset_sequence(n, force=True) + + +def _post_vote_queue(client, kind: str, page: int = 1): + return client.post( + reverse(views.post_vote_queue), + {"kind": kind, "page": page}, + content_type="application/json", + ) + + +class TestPostVoteQueuePrinting: + def test_only_unresolved_cards_are_returned(self, client, django_settings): + unresolved = CardFactory(printing_tag_status=PrintingTagStatus.UNRESOLVED) + CardFactory(printing_tag_status=PrintingTagStatus.RESOLVED) + CardFactory(printing_tag_status=PrintingTagStatus.NO_MATCH) + + response = _post_vote_queue(client, "printing") + + assert response.status_code == 200 + body = response.json() + assert body["hits"] == 1 + assert [item["card"]["identifier"] for item in body["items"]] == [unresolved.identifier] + assert all(item["tagName"] is None for item in body["items"]) + + def test_contested_cards_are_returned_first(self, client, django_settings): + contested = CardFactory(printing_tag_status=PrintingTagStatus.UNRESOLVED) + printing_a = CanonicalCardFactory() + printing_b = CanonicalCardFactory() + CardPrintingTagFactory(card=contested, printing=printing_a) + CardPrintingTagFactory(card=contested, printing=printing_b) + uncontested = CardFactory(printing_tag_status=PrintingTagStatus.UNRESOLVED) + + response = _post_vote_queue(client, "printing") + + identifiers = [item["card"]["identifier"] for item in response.json()["items"]] + assert identifiers == [contested.identifier, uncontested.identifier] + + def test_invalid_page_is_a_bad_request(self, client, django_settings): + CardFactory(printing_tag_status=PrintingTagStatus.UNRESOLVED) + + response = _post_vote_queue(client, "printing", page=999) + + assert response.status_code == 400 + + def test_non_post_method_is_rejected(self, client, django_settings): + response = client.get(reverse(views.post_vote_queue)) + assert response.status_code == 400 + + def test_invalid_kind_is_a_bad_request(self, client, django_settings): + response = client.post( + reverse(views.post_vote_queue), + {"kind": "not-a-real-kind", "page": 1}, + content_type="application/json", + ) + assert response.status_code == 400 + + +class TestPostVoteQueueArtist: + def test_only_unresolved_and_contested_cards_are_returned(self, client, django_settings): + unresolved = CardFactory(artist_vote_status=ArtistVoteStatus.UNRESOLVED) + CardFactory(artist_vote_status=ArtistVoteStatus.RESOLVED) + CardFactory(artist_vote_status=ArtistVoteStatus.UNKNOWN) + contested = CardFactory(artist_vote_status=ArtistVoteStatus.CONTESTED) + + response = _post_vote_queue(client, "artist") + + identifiers = {item["card"]["identifier"] for item in response.json()["items"]} + assert identifiers == {unresolved.identifier, contested.identifier} + + def test_contested_cards_are_returned_first(self, client, django_settings): + contested = CardFactory(artist_vote_status=ArtistVoteStatus.UNRESOLVED) + artist_a = CanonicalArtistFactory() + artist_b = CanonicalArtistFactory() + CardArtistVoteFactory(card=contested, artist=artist_a) + CardArtistVoteFactory(card=contested, artist=artist_b) + uncontested = CardFactory(artist_vote_status=ArtistVoteStatus.UNRESOLVED) + + response = _post_vote_queue(client, "artist") + + identifiers = [item["card"]["identifier"] for item in response.json()["items"]] + assert identifiers == [contested.identifier, uncontested.identifier] + + +class TestPostVoteQueueTag: + def test_resolved_pairs_are_excluded(self, client, django_settings, settings): + settings.PRINTING_TAG_MIN_VOTES = 1 + settings.PRINTING_TAG_MIN_SHARE = 0.5 + card = CardFactory(tags=[]) + tag = TagFactory(name="Borderless") + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.APPLY, source=VoteSource.ADMIN) + resolve_and_persist_tag_votes(card) + + response = _post_vote_queue(client, "tag") + + assert response.json()["items"] == [] + + def test_a_resolved_pair_stays_excluded_even_as_unrelated_votes_trickle_in(self, client, django_settings): + # regression guard matching the plan's "persisted state, not raw vote existence" + # requirement - once resolved, new votes on a *different* tag on the same card must + # not resurface the already-resolved pair + card = CardFactory(tags=[]) + resolved_tag = TagFactory(name="Borderless") + other_tag = TagFactory(name="Extended") + CardTagVoteFactory(card=card, tag=resolved_tag, polarity=VotePolarity.APPLY, source=VoteSource.ADMIN) + resolve_and_persist_tag_votes(card) + + CardTagVoteFactory(card=card, tag=other_tag, polarity=VotePolarity.APPLY, source=VoteSource.USER) + resolve_and_persist_tag_votes(card) + + response = _post_vote_queue(client, "tag") + + tag_names = {item["tagName"] for item in response.json()["items"]} + assert "Borderless" not in tag_names + assert "Extended" in tag_names + + def test_contested_pair_outranks_a_less_contested_pair(self, client, django_settings): + # a 1-vs-1 split (net weight 0) is a closer contest than a lone unresolved vote + # (net weight 1) and should sort first + contested_card = CardFactory(tags=[]) + contested_tag = TagFactory(name="Contested Tag") + CardTagVoteFactory(card=contested_card, tag=contested_tag, polarity=VotePolarity.APPLY, source=VoteSource.USER) + CardTagVoteFactory( + card=contested_card, tag=contested_tag, polarity=VotePolarity.NOT_APPLICABLE, source=VoteSource.USER + ) + resolve_and_persist_tag_votes(contested_card) + + lopsided_card = CardFactory(tags=[]) + lopsided_tag = TagFactory(name="Lopsided Tag") + CardTagVoteFactory(card=lopsided_card, tag=lopsided_tag, polarity=VotePolarity.APPLY, source=VoteSource.USER) + resolve_and_persist_tag_votes(lopsided_card) + + response = _post_vote_queue(client, "tag") + + tag_names = [item["tagName"] for item in response.json()["items"]] + assert tag_names[0] == "Contested Tag" + + def test_items_include_the_tag_name(self, client, django_settings): + card = CardFactory(tags=[]) + tag = TagFactory(name="Showcase") + CardTagVoteFactory(card=card, tag=tag, polarity=VotePolarity.APPLY, source=VoteSource.USER) + resolve_and_persist_tag_votes(card) + + response = _post_vote_queue(client, "tag") + + [item] = response.json()["items"] + assert item["tagName"] == "Showcase" + assert item["card"]["identifier"] == card.identifier diff --git a/MPCAutofill/cardpicker/urls.py b/MPCAutofill/cardpicker/urls.py index afa9dc42e..e41d5dea2 100755 --- a/MPCAutofill/cardpicker/urls.py +++ b/MPCAutofill/cardpicker/urls.py @@ -31,4 +31,5 @@ path("2/submitArtistVote/", views.post_submit_artist_vote), path("2/tagConsensus/", views.post_tag_consensus), path("2/submitTagVote/", views.post_submit_tag_vote), + path("2/voteQueue/", views.post_vote_queue), ] diff --git a/MPCAutofill/cardpicker/views.py b/MPCAutofill/cardpicker/views.py index ef5254636..8bfcbf5da 100644 --- a/MPCAutofill/cardpicker/views.py +++ b/MPCAutofill/cardpicker/views.py @@ -29,6 +29,7 @@ from cardpicker.artist_consensus import UNKNOWN as ARTIST_UNKNOWN from cardpicker.artist_consensus import ( get_artist_vote_tally, + get_contested_artist_card_ids, resolve_and_persist_artist, resolve_artist, ) @@ -44,6 +45,7 @@ from cardpicker.integrations.integrations import get_configured_game_integration from cardpicker.integrations.patreon import get_patreon_campaign_details, get_patrons from cardpicker.models import ( + ArtistVoteStatus, CanonicalArtist, CanonicalCard, Card, @@ -97,6 +99,9 @@ ImportSitesResponse, Info, InfoResponse, +) +from cardpicker.schema_types import Kind as VoteQueueKind +from cardpicker.schema_types import ( Language, LanguagesResponse, NewCardsFirstPage, @@ -123,6 +128,9 @@ TagConsensusResponse, TagsResponse, TagVoteTallyEntry, + VoteQueueItem, + VoteQueueRequest, + VoteQueueResponse, VoteTallyEntry, ) from cardpicker.search.sanitisation import to_searchable @@ -137,6 +145,7 @@ from cardpicker.sources.api import PathTraversalError, resolve_within_root from cardpicker.sources.source_types import SourceTypeChoices from cardpicker.tag_consensus import ( + get_tag_review_queue_pairs, get_tag_vote_tally, resolve_and_persist_tag_votes, resolve_tag, @@ -1040,3 +1049,65 @@ def post_submit_tag_vote(request: HttpRequest) -> HttpResponse: resolve_and_persist_tag_votes(card) return JsonResponse(_build_tag_consensus_entry(card, tag).model_dump()) + + +def _paginate(items: Any, page: int) -> Any: + """Shared page-index validation for the vote queue, mirroring `get_printing_tag_queue`'s + own inline validation (not reused directly - that view's validation lives inline, not as + a separate helper, and duplicating six lines here is simpler than refactoring it out from + under a view this task doesn't otherwise touch).""" + paginator: Paginator[Any] = Paginator(items, PRINTING_TAG_QUEUE_PAGE_SIZE) + if not (paginator.num_pages >= page > 0): + raise BadRequestException(f"Invalid page {page} specified - must be between 1 and {paginator.num_pages}.") + return paginator + + +@csrf_exempt +@ErrorWrappers.to_json +def post_vote_queue(request: HttpRequest) -> HttpResponse: + """ + Generalizes the review queue across all three vote kinds via a `kind` request field - a + new sibling endpoint (POST, unlike `2/printingTagQueue/`'s GET) rather than a mutation of + that one, which stays completely untouched/reachable for anything still calling it. + + One queue item per card for `kind=printing`/`artist` (`tagName` always null, exactly + `2/printingTagQueue/`'s existing shape plus that field) - printing mode's candidate + set/ordering is byte-for-byte what `get_printing_tag_queue` already does (unresolved, + contested-first). Artist mode is the same shape, generalized to also include `CONTESTED` + (a status `PrintingTagStatus` has no equivalent for - printing's own contested cards are + already tagged `UNRESOLVED`, distinguished only by the ordering annotation). + + For `kind=tag`, one item per (card, tag) pair - see `get_tag_review_queue_pairs` for the + persisted-state candidate filter and the net-polarity-weight/card-interleave ordering. + """ + if request.method != "POST": + raise BadRequestException("Expected POST request.") + + req = VoteQueueRequest.model_validate(json.loads(request.body)) + + if req.kind == VoteQueueKind.tag: + pairs = get_tag_review_queue_pairs() + paginator = _paginate(pairs, req.page) + page_pairs = paginator.page(req.page).object_list + cards_by_id = {card.pk: card for card in Card.objects.filter(pk__in=[card_id for card_id, _ in page_pairs])} + items = [ + VoteQueueItem(card=cards_by_id[card_id].serialise(), tagName=tag_name) for card_id, tag_name in page_pairs + ] + else: + if req.kind == VoteQueueKind.printing: + cards = Card.objects.filter(printing_tag_status=PrintingTagStatus.UNRESOLVED).annotate( + is_contested=Case(When(pk__in=get_contested_card_ids(), then=1), default=0, output_field=IntegerField()) + ) + else: + cards = Card.objects.filter( + artist_vote_status__in=[ArtistVoteStatus.UNRESOLVED, ArtistVoteStatus.CONTESTED] + ).annotate( + is_contested=Case( + When(pk__in=get_contested_artist_card_ids(), then=1), default=0, output_field=IntegerField() + ) + ) + cards = cards.order_by("-is_contested", "-date_created", "name") + paginator = _paginate(cards, req.page) + items = [VoteQueueItem(card=card.serialise(), tagName=None) for card in paginator.page(req.page).object_list] + + return JsonResponse(VoteQueueResponse(hits=paginator.count, pages=paginator.num_pages, items=items).model_dump()) diff --git a/MPCAutofill/cardpicker/vote_consensus.py b/MPCAutofill/cardpicker/vote_consensus.py index 38735ef8c..ccf39cc38 100644 --- a/MPCAutofill/cardpicker/vote_consensus.py +++ b/MPCAutofill/cardpicker/vote_consensus.py @@ -1,7 +1,8 @@ from collections import defaultdict -from typing import Hashable, Iterable, NamedTuple, TypedDict +from typing import Any, Hashable, Iterable, NamedTuple, TypedDict from django.conf import settings +from django.db.models import Case, Count, IntegerField, Q, QuerySet, When from cardpicker.models import VoteSource @@ -78,4 +79,40 @@ def resolve_weighted_consensus(votes: Iterable[VoteTuple], min_weight: float, mi return None -__all__ = ["VoteTuple", "resolve_weighted_consensus", "_SOURCE_WEIGHTS"] +def contested_queryset( + queryset: "QuerySet[Any]", + group_by: str | list[str], + outcome_field: str, + sentinel_field: str | None = None, +) -> list[Any]: + """ + Generalizes what was originally `printing_consensus.get_contested_card_ids`'s logic across + any `AbstractWeightedVote` subclass - see that function for the original "what does + contested mean, and why is this a cheap proxy rather than a full consensus + recomputation" reasoning, unchanged here. + + Takes an unfiltered base `queryset` (e.g. `CardPrintingTag.objects.all()` - a queryset + rather than the model class itself, since django-stubs can't statically resolve `.objects` + off a bare `type[Model]`) and groups its rows by `group_by` (a field name, e.g. `"card_id"`, + or a list of field names for a composite grouping, e.g. `["card_id", "tag_id"]`), counts + distinct `outcome_field` values per group, and flags a group as contested if it has more + than one distinct outcome, or - only when `sentinel_field` is given - if it has exactly one + outcome AND at least one sentinel vote alongside it (e.g. a printing vote coexisting with + an `is_no_match` vote). Returns a plain list of group-by values (or tuples, for a composite + `group_by`) - materialized eagerly, same rationale as the original: the contested set is + always a small fraction of the total. + """ + group_fields = [group_by] if isinstance(group_by, str) else group_by + condition = Q(distinct_outcomes__gt=1) + annotations = {"distinct_outcomes": Count(outcome_field, distinct=True)} + if sentinel_field is not None: + annotations["has_sentinel"] = Count(Case(When(**{sentinel_field: True}, then=1), output_field=IntegerField())) + condition = condition | (Q(distinct_outcomes__gte=1) & Q(has_sentinel__gt=0)) + + grouped = queryset.values(*group_fields).annotate(**annotations).filter(condition) + if len(group_fields) == 1: + return list(grouped.values_list(group_fields[0], flat=True)) + return list(grouped.values_list(*group_fields)) + + +__all__ = ["VoteTuple", "resolve_weighted_consensus", "_SOURCE_WEIGHTS", "contested_queryset"] diff --git a/frontend/src/common/schema_types.ts b/frontend/src/common/schema_types.ts index a04d00009..02b149bf4 100644 --- a/frontend/src/common/schema_types.ts +++ b/frontend/src/common/schema_types.ts @@ -2,7 +2,7 @@ // To parse this data: // -// import { Convert, ArtistVoteTallyEntry, Campaign, CanonicalArtist, CanonicalCard, Card, CardType, FilterSettings, Game, ImportSite, Language, NewCardsFirstPage, PrintingCandidate, SearchQuery, SearchSettings, SearchTypeSettings, SortBy, Source, SourceContribution, SourceSettings, SourceType, Supporter, SupporterTier, Tag, TagConsensusEntry, TagVoteTallyEntry, VoteTallyEntry, ArtistCandidatesRequest, ArtistCandidatesResponse, ArtistConsensusRequest, ArtistConsensusResponse, CardbacksRequest, CardbacksResponse, CardsRequest, CardsResponse, ContributionsResponse, DFCPairsResponse, EditorSearchRequest, EditorSearchResponse, ErrorResponse, ExploreSearchRequest, ExploreSearchResponse, ImportSiteDecklistRequest, ImportSiteDecklistResponse, ImportSitesResponse, InfoResponse, LanguagesResponse, NewCardsFirstPagesResponse, NewCardsPageResponse, OldEditorSearchRequest, OldEditorSearchResponse, PatreonResponse, PrintingCandidatesRequest, PrintingCandidatesResponse, PrintingConsensusRequest, PrintingConsensusResponse, PrintingTagQueueResponse, SampleCardsResponse, SearchEngineHealthResponse, SourcesResponse, SubmitArtistVoteRequest, SubmitPrintingTagRequest, SubmitTagVoteRequest, TagConsensusRequest, TagConsensusResponse, TagsResponse } from "./file"; +// import { Convert, ArtistVoteTallyEntry, Campaign, CanonicalArtist, CanonicalCard, Card, CardType, FilterSettings, Game, ImportSite, Language, NewCardsFirstPage, PrintingCandidate, SearchQuery, SearchSettings, SearchTypeSettings, SortBy, Source, SourceContribution, SourceSettings, SourceType, Supporter, SupporterTier, Tag, TagConsensusEntry, TagVoteTallyEntry, VoteQueueItem, VoteTallyEntry, ArtistCandidatesRequest, ArtistCandidatesResponse, ArtistConsensusRequest, ArtistConsensusResponse, CardbacksRequest, CardbacksResponse, CardsRequest, CardsResponse, ContributionsResponse, DFCPairsResponse, EditorSearchRequest, EditorSearchResponse, ErrorResponse, ExploreSearchRequest, ExploreSearchResponse, ImportSiteDecklistRequest, ImportSiteDecklistResponse, ImportSitesResponse, InfoResponse, LanguagesResponse, NewCardsFirstPagesResponse, NewCardsPageResponse, OldEditorSearchRequest, OldEditorSearchResponse, PatreonResponse, PrintingCandidatesRequest, PrintingCandidatesResponse, PrintingConsensusRequest, PrintingConsensusResponse, PrintingTagQueueResponse, SampleCardsResponse, SearchEngineHealthResponse, SourcesResponse, SubmitArtistVoteRequest, SubmitPrintingTagRequest, SubmitTagVoteRequest, TagConsensusRequest, TagConsensusResponse, TagsResponse, VoteQueueRequest, VoteQueueResponse } from "./file"; // // const artistVoteTallyEntry = Convert.toArtistVoteTallyEntry(json); // const campaign = Convert.toCampaign(json); @@ -30,6 +30,7 @@ // const tag = Convert.toTag(json); // const tagConsensusEntry = Convert.toTagConsensusEntry(json); // const tagVoteTallyEntry = Convert.toTagVoteTallyEntry(json); +// const voteQueueItem = Convert.toVoteQueueItem(json); // const voteTallyEntry = Convert.toVoteTallyEntry(json); // const artistCandidatesRequest = Convert.toArtistCandidatesRequest(json); // const artistCandidatesResponse = Convert.toArtistCandidatesResponse(json); @@ -70,6 +71,8 @@ // const tagConsensusRequest = Convert.toTagConsensusRequest(json); // const tagConsensusResponse = Convert.toTagConsensusResponse(json); // const tagsResponse = Convert.toTagsResponse(json); +// const voteQueueRequest = Convert.toVoteQueueRequest(json); +// const voteQueueResponse = Convert.toVoteQueueResponse(json); // // These functions will throw an error if the JSON doesn't // match the expected interface, even if the JSON is valid. @@ -176,6 +179,18 @@ export interface CardsResponse { export interface Card { canonicalArtist?: CanonicalArtist | null; + /** + * True only when canonicalArtist was supplied by artist-vote consensus alone, with no + * confirmed indexing match or resolved printing backing it - lets the frontend distinguish + * a confidently-known artist from a vote-derived one (e.g. for the ArtistVotePicker + * 'wrong?' affordance) without needing to know serialise()'s fallback chain itself. + */ + canonicalArtistIsFromVoteOnly: boolean; + /** + * Which rung of the artist fallback chain actually supplied canonicalArtist - + * debug/introspection field, not load-bearing for any current frontend logic. + */ + canonicalArtistSource?: null | string; canonicalCard?: CanonicalCard | null; cardType: CardType; /** @@ -523,6 +538,28 @@ export interface ChildElement { parent: null | string; } +export interface VoteQueueRequest { + kind: Kind; + page: number; +} + +export enum Kind { + Artist = "artist", + Printing = "printing", + Tag = "tag", +} + +export interface VoteQueueResponse { + hits: number; + items: VoteQueueItem[]; + pages: number; +} + +export interface VoteQueueItem { + card: Card; + tagName?: null | string; +} + // Converts JSON strings to/from your types // and asserts the results of JSON.parse at runtime export class Convert { @@ -740,6 +777,14 @@ export class Convert { return JSON.stringify(uncast(value, r("TagVoteTallyEntry")), null, 2); } + public static toVoteQueueItem(json: string): VoteQueueItem { + return cast(JSON.parse(json), r("VoteQueueItem")); + } + + public static voteQueueItemToJson(value: VoteQueueItem): string { + return JSON.stringify(uncast(value, r("VoteQueueItem")), null, 2); + } + public static toVoteTallyEntry(json: string): VoteTallyEntry { return cast(JSON.parse(json), r("VoteTallyEntry")); } @@ -1181,6 +1226,22 @@ export class Convert { public static tagsResponseToJson(value: TagsResponse): string { return JSON.stringify(uncast(value, r("TagsResponse")), null, 2); } + + public static toVoteQueueRequest(json: string): VoteQueueRequest { + return cast(JSON.parse(json), r("VoteQueueRequest")); + } + + public static voteQueueRequestToJson(value: VoteQueueRequest): string { + return JSON.stringify(uncast(value, r("VoteQueueRequest")), null, 2); + } + + public static toVoteQueueResponse(json: string): VoteQueueResponse { + return cast(JSON.parse(json), r("VoteQueueResponse")); + } + + public static voteQueueResponseToJson(value: VoteQueueResponse): string { + return JSON.stringify(uncast(value, r("VoteQueueResponse")), null, 2); + } } function invalidValue(typ: any, val: any, key: any, parent: any = ""): never { @@ -1477,6 +1538,16 @@ const typeMap: any = { js: "canonicalArtist", typ: u(undefined, u(r("CanonicalArtist"), null)), }, + { + json: "canonicalArtistIsFromVoteOnly", + js: "canonicalArtistIsFromVoteOnly", + typ: true, + }, + { + json: "canonicalArtistSource", + js: "canonicalArtistSource", + typ: u(undefined, u(null, "")), + }, { json: "canonicalCard", js: "canonicalCard", @@ -1894,6 +1965,28 @@ const typeMap: any = { ], false ), + VoteQueueRequest: o( + [ + { json: "kind", js: "kind", typ: r("Kind") }, + { json: "page", js: "page", typ: 0 }, + ], + false + ), + VoteQueueResponse: o( + [ + { json: "hits", js: "hits", typ: 0 }, + { json: "items", js: "items", typ: a(r("VoteQueueItem")) }, + { json: "pages", js: "pages", typ: 0 }, + ], + false + ), + VoteQueueItem: o( + [ + { json: "card", js: "card", typ: r("Card") }, + { json: "tagName", js: "tagName", typ: u(undefined, u(null, "")) }, + ], + false + ), Game: ["MTG"], CardType: ["CARD", "CARDBACK", "TOKEN"], SourceType: ["AWS S3", "Google Drive", "Local File"], @@ -1905,4 +1998,5 @@ const typeMap: any = { "nameAscending", "nameDescending", ], + Kind: ["artist", "printing", "tag"], }; diff --git a/schemas/schemas/Card.json b/schemas/schemas/Card.json index 86f7f8841..18c4d3844 100644 --- a/schemas/schemas/Card.json +++ b/schemas/schemas/Card.json @@ -73,6 +73,14 @@ }, "canonicalArtist": { "$ref": "./CanonicalArtist.json" + }, + "canonicalArtistIsFromVoteOnly": { + "description": "True only when canonicalArtist was supplied by artist-vote consensus alone, with no confirmed indexing match or resolved printing backing it - lets the frontend distinguish a confidently-known artist from a vote-derived one (e.g. for the ArtistVotePicker 'wrong?' affordance) without needing to know serialise()'s fallback chain itself.", + "type": "boolean" + }, + "canonicalArtistSource": { + "description": "Which rung of the artist fallback chain actually supplied canonicalArtist - debug/introspection field, not load-bearing for any current frontend logic.", + "type": ["string", "null"] } }, "required": [ @@ -93,7 +101,8 @@ "smallThumbnailUrl", "mediumThumbnailUrl", "language", - "tags" + "tags", + "canonicalArtistIsFromVoteOnly" ], "additionalProperties": false } diff --git a/schemas/schemas/VoteQueueItem.json b/schemas/schemas/VoteQueueItem.json new file mode 100644 index 000000000..127b654ec --- /dev/null +++ b/schemas/schemas/VoteQueueItem.json @@ -0,0 +1,11 @@ +{ + "title": "Vote Queue Item", + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "card": { "$ref": "./Card.json" }, + "tagName": { "type": ["string", "null"] } + }, + "required": ["card"], + "additionalProperties": false +} diff --git a/schemas/schemas/endpoints/VoteQueueRequest.json b/schemas/schemas/endpoints/VoteQueueRequest.json new file mode 100644 index 000000000..440a209b4 --- /dev/null +++ b/schemas/schemas/endpoints/VoteQueueRequest.json @@ -0,0 +1,11 @@ +{ + "title": "Vote Queue Request", + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "kind": { "type": "string", "enum": ["printing", "artist", "tag"] }, + "page": { "type": "integer" } + }, + "required": ["kind", "page"], + "additionalProperties": false +} diff --git a/schemas/schemas/endpoints/VoteQueueResponse.json b/schemas/schemas/endpoints/VoteQueueResponse.json new file mode 100644 index 000000000..c8cc903f8 --- /dev/null +++ b/schemas/schemas/endpoints/VoteQueueResponse.json @@ -0,0 +1,15 @@ +{ + "title": "Vote Queue Response", + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "hits": { "type": "integer" }, + "pages": { "type": "integer" }, + "items": { + "type": "array", + "items": { "$ref": "../VoteQueueItem.json" } + } + }, + "required": ["hits", "pages", "items"], + "additionalProperties": false +} From 37b99b73476e23dceb1e83277c68124dc5fb1927 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Sun, 12 Jul 2026 22:08:17 +0000 Subject: [PATCH 5/6] Add vote queue kind switcher, tag question, and the "wrong?" affordance Frontend half of the review-tooling sprint. printingQueue.tsx gains a Printings/Artists/Tags tab switcher - printing mode keeps its exact existing PrintingTagQueue component and behavior unchanged; artist and tag modes share a new GenericVoteQueue shell driven by 2/voteQueue/, since only those two need the new endpoint's response shape. Artist mode reuses ArtistVotePicker directly; tag mode gets a new focused QueueTagQuestion (one (card, tag) pair, apply/not applicable/skip) rather than reusing TagVotePicker's full per-card chip grid, which is a different unit of interaction. ArtistVotePicker also gains the deferred "wrong?" affordance: when a card's artist is already confidently known independent of any vote, it renders as pre-filled text with a small link to reveal the full picker, using the canonicalArtistIsFromVoteOnly field Card.serialise() now provides. Made the two new Card fields optional in the schema (the backend always fills them; only frontend object literals in tests/mocks would otherwise need updating for no real benefit). Also fixes an unrelated pre-existing bug found while stabilizing the Playwright suite for this change: New.spec.ts was missing an await on its page load, a race that usually wins but explains this test's occasional documented flakiness under load. Co-Authored-By: Claude Sonnet 5 --- MPCAutofill/cardpicker/schema_types.py | 25 +-- frontend/src/common/schema_types.ts | 4 +- .../attributeVoting/ArtistVotePicker.tsx | 31 +++ .../attributeVoting/AttributeVotingPanel.tsx | 4 + .../attributeVoting/GenericVoteQueue.tsx | 178 ++++++++++++++++++ .../attributeVoting/QueueTagQuestion.tsx | 93 +++++++++ .../CardDetailedViewModal.tsx | 6 + frontend/src/mocks/handlers.ts | 34 ++++ frontend/src/pages/printingQueue.tsx | 54 +++++- frontend/src/store/api.ts | 21 +++ frontend/tests/New.spec.ts | 2 +- frontend/tests/VoteQueue.spec.ts | 91 +++++++++ schemas/schemas/Card.json | 3 +- 13 files changed, 526 insertions(+), 20 deletions(-) create mode 100644 frontend/src/features/attributeVoting/GenericVoteQueue.tsx create mode 100644 frontend/src/features/attributeVoting/QueueTagQuestion.tsx create mode 100644 frontend/tests/VoteQueue.spec.ts diff --git a/MPCAutofill/cardpicker/schema_types.py b/MPCAutofill/cardpicker/schema_types.py index 32d0bf5d0..53fb6446b 100644 --- a/MPCAutofill/cardpicker/schema_types.py +++ b/MPCAutofill/cardpicker/schema_types.py @@ -5,9 +5,9 @@ """ from enum import Enum -from pydantic import BaseModel -from typing import Optional, Any, List, Union, Dict, TypeVar, Callable, Type, cast +from typing import Any, Callable, Dict, List, Optional, Type, TypeVar, Union, cast +from pydantic import BaseModel T = TypeVar("T") EnumT = TypeVar("EnumT", bound=Enum) @@ -394,12 +394,6 @@ class SourceType(str, Enum): class Card(BaseModel): - canonicalArtistIsFromVoteOnly: bool - """True only when canonicalArtist was supplied by artist-vote consensus alone, with no - confirmed indexing match or resolved printing backing it - lets the frontend distinguish - a confidently-known artist from a vote-derived one (e.g. for the ArtistVotePicker - 'wrong?' affordance) without needing to know serialise()'s fallback chain itself. - """ cardType: CardType dateCreated: str """Created date - formatted by backend""" @@ -423,6 +417,12 @@ class Card(BaseModel): sourceVerbose: str tags: List[str] canonicalArtist: Optional[CanonicalArtistClass] = None + canonicalArtistIsFromVoteOnly: Optional[bool] = None + """True only when canonicalArtist was supplied by artist-vote consensus alone, with no + confirmed indexing match or resolved printing backing it - lets the frontend distinguish + a confidently-known artist from a vote-derived one (e.g. for the ArtistVotePicker + 'wrong?' affordance) without needing to know serialise()'s fallback chain itself. + """ canonicalArtistSource: Optional[str] = None """Which rung of the artist fallback chain actually supplied canonicalArtist - debug/introspection field, not load-bearing for any current frontend logic. @@ -434,7 +434,6 @@ class Card(BaseModel): @staticmethod def from_dict(obj: Any) -> "Card": assert isinstance(obj, dict) - canonicalArtistIsFromVoteOnly = from_bool(obj.get("canonicalArtistIsFromVoteOnly")) cardType = CardType(obj.get("cardType")) dateCreated = from_str(obj.get("dateCreated")) dateModified = from_str(obj.get("dateModified")) @@ -454,12 +453,12 @@ def from_dict(obj: Any) -> "Card": sourceVerbose = from_str(obj.get("sourceVerbose")) tags = from_list(from_str, obj.get("tags")) canonicalArtist = from_union([from_none, CanonicalArtistClass.from_dict], obj.get("canonicalArtist")) + canonicalArtistIsFromVoteOnly = from_union([from_bool, from_none], obj.get("canonicalArtistIsFromVoteOnly")) canonicalArtistSource = from_union([from_none, from_str], obj.get("canonicalArtistSource")) canonicalCard = from_union([from_none, CanonicalCardClass.from_dict], obj.get("canonicalCard")) sourceExternalLink = from_union([from_str, from_none], obj.get("sourceExternalLink")) sourceType = from_union([SourceType, from_none], obj.get("sourceType")) return Card( - canonicalArtistIsFromVoteOnly, cardType, dateCreated, dateModified, @@ -479,6 +478,7 @@ def from_dict(obj: Any) -> "Card": sourceVerbose, tags, canonicalArtist, + canonicalArtistIsFromVoteOnly, canonicalArtistSource, canonicalCard, sourceExternalLink, @@ -487,7 +487,6 @@ def from_dict(obj: Any) -> "Card": def to_dict(self) -> dict: result: dict = {} - result["canonicalArtistIsFromVoteOnly"] = from_bool(self.canonicalArtistIsFromVoteOnly) result["cardType"] = to_enum(CardType, self.cardType) result["dateCreated"] = from_str(self.dateCreated) result["dateModified"] = from_str(self.dateModified) @@ -510,6 +509,10 @@ def to_dict(self) -> dict: result["canonicalArtist"] = from_union( [from_none, lambda x: to_class(CanonicalArtistClass, x)], self.canonicalArtist ) + if self.canonicalArtistIsFromVoteOnly is not None: + result["canonicalArtistIsFromVoteOnly"] = from_union( + [from_bool, from_none], self.canonicalArtistIsFromVoteOnly + ) if self.canonicalArtistSource is not None: result["canonicalArtistSource"] = from_union([from_none, from_str], self.canonicalArtistSource) if self.canonicalCard is not None: diff --git a/frontend/src/common/schema_types.ts b/frontend/src/common/schema_types.ts index 02b149bf4..a36624c4e 100644 --- a/frontend/src/common/schema_types.ts +++ b/frontend/src/common/schema_types.ts @@ -185,7 +185,7 @@ export interface Card { * a confidently-known artist from a vote-derived one (e.g. for the ArtistVotePicker * 'wrong?' affordance) without needing to know serialise()'s fallback chain itself. */ - canonicalArtistIsFromVoteOnly: boolean; + canonicalArtistIsFromVoteOnly?: boolean; /** * Which rung of the artist fallback chain actually supplied canonicalArtist - * debug/introspection field, not load-bearing for any current frontend logic. @@ -1541,7 +1541,7 @@ const typeMap: any = { { json: "canonicalArtistIsFromVoteOnly", js: "canonicalArtistIsFromVoteOnly", - typ: true, + typ: u(undefined, true), }, { json: "canonicalArtistSource", diff --git a/frontend/src/features/attributeVoting/ArtistVotePicker.tsx b/frontend/src/features/attributeVoting/ArtistVotePicker.tsx index dc83d3584..22a90a91e 100644 --- a/frontend/src/features/attributeVoting/ArtistVotePicker.tsx +++ b/frontend/src/features/attributeVoting/ArtistVotePicker.tsx @@ -28,11 +28,23 @@ import { setNotification } from "@/store/slices/toastsSlice"; interface ArtistVotePickerProps { backendURL: string; cardIdentifier: string; + /** + * The card's already-known artist name, if confidently known independent of any artist + * vote - callers derive this from their own Card/CardDocument object as + * `canonicalArtist != null && !canonicalArtistIsFromVoteOnly ? canonicalArtist.name : null` + * (that derivation can't happen inside this component, since it only ever has the card's + * *identifier*, not the full card object). When set, renders as pre-filled text with a + * small "wrong?" link that reveals the full picker on click, instead of soliciting a vote + * outright for an artist that's already confidently known from indexing or a resolved + * printing. Omitted or null: today's unconditional-picker behavior is unchanged. + */ + confidentlyKnownArtistName?: string | null; } export function ArtistVotePicker({ backendURL, cardIdentifier, + confidentlyKnownArtistName, }: ArtistVotePickerProps) { const dispatch = useAppDispatch(); @@ -43,6 +55,7 @@ export function ArtistVotePicker({ ); const [loading, setLoading] = useState(false); const [submitting, setSubmitting] = useState(false); + const [revealPickerAnyway, setRevealPickerAnyway] = useState(false); useEffect(() => { APIGetArtistConsensus(backendURL, cardIdentifier) @@ -102,6 +115,24 @@ export function ArtistVotePicker({ .finally(() => setSubmitting(false)); }; + if (confidentlyKnownArtistName != null && !revealPickerAnyway) { + return ( + + ); + } + return (
diff --git a/frontend/src/features/attributeVoting/AttributeVotingPanel.tsx b/frontend/src/features/attributeVoting/AttributeVotingPanel.tsx index 6917d76a8..d2746e996 100644 --- a/frontend/src/features/attributeVoting/AttributeVotingPanel.tsx +++ b/frontend/src/features/attributeVoting/AttributeVotingPanel.tsx @@ -14,11 +14,14 @@ import { TagVotePicker } from "@/features/attributeVoting/TagVotePicker"; interface AttributeVotingPanelProps { backendURL: string; cardIdentifier: string; + /** Threaded straight through to ArtistVotePicker - see that component's own prop docstring. */ + confidentlyKnownArtistName?: string | null; } export function AttributeVotingPanel({ backendURL, cardIdentifier, + confidentlyKnownArtistName, }: AttributeVotingPanelProps) { return (
@@ -26,6 +29,7 @@ export function AttributeVotingPanel({
Do any of these tags apply?
diff --git a/frontend/src/features/attributeVoting/GenericVoteQueue.tsx b/frontend/src/features/attributeVoting/GenericVoteQueue.tsx new file mode 100644 index 000000000..8f4b9ac13 --- /dev/null +++ b/frontend/src/features/attributeVoting/GenericVoteQueue.tsx @@ -0,0 +1,178 @@ +/** + * Shared queue shell for the artist and tag modes of the "Who's That Planeswalker?" vote + * queue, driven by the generalized `2/voteQueue/` endpoint. Printing mode is deliberately NOT + * folded into this - it keeps using PrintingTagQueue.tsx and `2/printingTagQueue/` completely + * unchanged (its own data source/shape, and its exact existing behavior is a hard constraint), + * so this component only ever needs to handle the two new kinds, which do share one response + * shape (`VoteQueueItem[]`) and can reasonably share one fetch/pagination/flavor-text + * implementation instead of duplicating it twice. + */ + +import React, { useEffect, useState } from "react"; +import Button from "react-bootstrap/Button"; +import Col from "react-bootstrap/Col"; +import Row from "react-bootstrap/Row"; + +import { Kind, VoteQueueItem } from "@/common/schema_types"; +import { useAppSelector } from "@/common/types"; +import { Spinner } from "@/components/Spinner"; +import { ArtistVotePicker } from "@/features/attributeVoting/ArtistVotePicker"; +import { QueueTagQuestion } from "@/features/attributeVoting/QueueTagQuestion"; +import { APIGetVoteQueue } from "@/store/api"; +import { selectRemoteBackendURL } from "@/store/slices/backendSlice"; + +const FLAVOR_TEXT = [ + "Your spark ignites! On to the next mystery.", + "A planeswalker's eye for detail - nicely done!", + "The multiverse is a little better catalogued because of you.", + "Sharper than a Sphinx's riddle. Next card incoming!", + "That's the stuff legends are made of. Keep going!", + "Another printing pinned down. Onward, planeswalker!", + "You've got a good spark for this. Next!", + "Precisely the kind of insight the Multiverse needs.", + "Well walked, planeswalker. Here comes another.", + "Your knowledge of the planes grows ever stronger.", +]; + +function randomFlavorText(): string { + return FLAVOR_TEXT[Math.floor(Math.random() * FLAVOR_TEXT.length)]; +} + +interface GenericVoteQueueProps { + kind: typeof Kind.Artist | typeof Kind.Tag; + label: string; +} + +export function GenericVoteQueue({ kind, label }: GenericVoteQueueProps) { + const backendURL = useAppSelector(selectRemoteBackendURL); + + const [queueItems, setQueueItems] = useState>([]); + const [currentIndex, setCurrentIndex] = useState(0); + const [page, setPage] = useState(1); + const [pages, setPages] = useState(1); + const [hits, setHits] = useState(0); + const [loadingQueue, setLoadingQueue] = useState(true); + const [flavorText, setFlavorText] = useState(null); + const fetchedPagesRef = React.useRef>(new Set()); + + const currentItem = queueItems[currentIndex] ?? null; + const queueExhausted = + !loadingQueue && currentIndex >= queueItems.length && page >= pages; + + // reset the locally-held queue whenever the kind changes (switching tabs) + useEffect(() => { + setQueueItems([]); + setCurrentIndex(0); + setPage(1); + setPages(1); + setHits(0); + fetchedPagesRef.current = new Set(); + }, [kind]); + + useEffect(() => { + if (backendURL == null) { + return; + } + if (currentIndex < queueItems.length) { + return; + } + if (queueItems.length > 0 && page >= pages) { + return; + } + const nextPage = queueItems.length === 0 ? 1 : page + 1; + if (fetchedPagesRef.current.has(nextPage)) { + return; + } + fetchedPagesRef.current.add(nextPage); + setLoadingQueue(true); + APIGetVoteQueue(backendURL, kind, nextPage) + .then((response) => { + setQueueItems((previous) => [...previous, ...response.items]); + setHits(response.hits); + setPages(response.pages); + setPage(nextPage); + }) + .catch(() => undefined) + .finally(() => setLoadingQueue(false)); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [backendURL, kind, currentIndex, queueItems.length, page, pages]); + + const advance = () => { + setFlavorText(randomFlavorText()); + setCurrentIndex((previous) => previous + 1); + }; + + if (queueExhausted) { + return ( +
+

+ You're all caught up - no cards left to tag right now! +

+ {flavorText != null && ( +

+ {flavorText} +

+ )} +
+ ); + } + + return ( +
+

+ Still need {label}: {hits} card{hits !== 1 && "s"} +

+ {flavorText != null && ( +

+ {flavorText} +

+ )} + {currentItem == null || backendURL == null ? ( +
+ +
+ ) : ( +
+ + + {currentItem.card.name} +
{currentItem.card.name}
+ + + {kind === Kind.Tag && currentItem.tagName != null ? ( + + ) : ( + <> + +
+ +
+ + )} + + +
+ )} +
+ ); +} diff --git a/frontend/src/features/attributeVoting/QueueTagQuestion.tsx b/frontend/src/features/attributeVoting/QueueTagQuestion.tsx new file mode 100644 index 000000000..5a1ca0aa9 --- /dev/null +++ b/frontend/src/features/attributeVoting/QueueTagQuestion.tsx @@ -0,0 +1,93 @@ +/** + * Focused single-(card, tag)-question control for the tag-mode vote queue - deliberately not + * a reuse of TagVotePicker.tsx's full chip grid (which shows every seeded tag at once for one + * card - a different unit of interaction). Each queue item here is exactly one contested/ + * unresolved (card, tag) pair, so this only ever asks about that one tag: apply, not + * applicable, or skip. Submits via the same APISubmitTagVote used by TagVotePicker. + */ + +import React, { useState } from "react"; +import Button from "react-bootstrap/Button"; + +import { getOrCreateAnonymousId } from "@/common/cookies"; +import { useAppDispatch } from "@/common/types"; +import { APISubmitTagVote } from "@/store/api"; +import { setNotification } from "@/store/slices/toastsSlice"; + +interface QueueTagQuestionProps { + backendURL: string; + cardIdentifier: string; + tagName: string; + /** Called once the user has answered (apply/not applicable submitted successfully) or skipped. */ + onAnswered: () => void; +} + +const APPLY = 1; +const NOT_APPLICABLE = -1; + +export function QueueTagQuestion({ + backendURL, + cardIdentifier, + tagName, + onAnswered, +}: QueueTagQuestionProps) { + const dispatch = useAppDispatch(); + const [submitting, setSubmitting] = useState(false); + + const submit = (polarity: number) => { + setSubmitting(true); + APISubmitTagVote( + backendURL, + cardIdentifier, + getOrCreateAnonymousId(), + tagName, + polarity + ) + .then(() => onAnswered()) + .catch(() => + dispatch( + setNotification([ + Math.random().toString(), + { + name: "Vote failed", + message: + "Something went wrong submitting your vote - please try again.", + level: "error", + }, + ]) + ) + ) + .finally(() => setSubmitting(false)); + }; + + return ( +
+
+ Does {tagName} apply? +
+
+ + + +
+
+ ); +} diff --git a/frontend/src/features/cardDetailedView/CardDetailedViewModal.tsx b/frontend/src/features/cardDetailedView/CardDetailedViewModal.tsx index baef432d7..8e126b5a9 100644 --- a/frontend/src/features/cardDetailedView/CardDetailedViewModal.tsx +++ b/frontend/src/features/cardDetailedView/CardDetailedViewModal.tsx @@ -196,6 +196,12 @@ export function CardDetailedViewModal({ )} diff --git a/frontend/src/mocks/handlers.ts b/frontend/src/mocks/handlers.ts index 96f794e8b..5d27aa521 100644 --- a/frontend/src/mocks/handlers.ts +++ b/frontend/src/mocks/handlers.ts @@ -716,6 +716,40 @@ export const printingTagQueueNoResults = http.get( () => HttpResponse.json({ hits: 0, pages: 1, cards: [] }, { status: 200 }) ); +// 2/voteQueue/ is shared by all three kinds (kind is in the POST body, not the URL), so this +// one handler branches on it rather than registering three separate handlers for the same route. +export const voteQueueArtistOneTagOneResults = http.post( + buildRoute("2/voteQueue/"), + async ({ request }) => { + const body = (await request.json()) as { kind: string; page: number }; + if (body.kind === "artist") { + return HttpResponse.json( + { hits: 1, pages: 1, items: [{ card: cardDocument8, tagName: null }] }, + { status: 200 } + ); + } + if (body.kind === "tag") { + return HttpResponse.json( + { + hits: 1, + pages: 1, + // deliberately a different card than the printing/artist mock fixtures use - the + // printing tab's mount is never torn down when switching away (matches its + // existing, unchanged behavior), so reusing the same card here would produce two + // simultaneous elements with the same alt text once the tag tab is active + items: [{ card: cardDocument9, tagName: "Borderless" }], + }, + { status: 200 } + ); + } + return HttpResponse.json({ hits: 0, pages: 1, items: [] }, { status: 200 }); + } +); + +export const voteQueueNoResults = http.post(buildRoute("2/voteQueue/"), () => + HttpResponse.json({ hits: 0, pages: 1, items: [] }, { status: 200 }) +); + //# endregion //# region attribute voting diff --git a/frontend/src/pages/printingQueue.tsx b/frontend/src/pages/printingQueue.tsx index f505b910a..598a09bbf 100644 --- a/frontend/src/pages/printingQueue.tsx +++ b/frontend/src/pages/printingQueue.tsx @@ -1,9 +1,13 @@ import styled from "@emotion/styled"; import Head from "next/head"; -import React from "react"; +import React, { useState } from "react"; +import Nav from "react-bootstrap/Nav"; +import Tab from "react-bootstrap/Tab"; import { ProjectName } from "@/common/constants"; +import { Kind } from "@/common/schema_types"; import { NoBackendDefault } from "@/components/NoBackendDefault"; +import { GenericVoteQueue } from "@/features/attributeVoting/GenericVoteQueue"; import { PrintingTagQueue } from "@/features/printingTags/PrintingTagQueue"; import Footer from "@/features/ui/Footer"; import { ProjectContainer } from "@/features/ui/Layout"; @@ -12,6 +16,8 @@ import { useRemoteBackendConfigured, } from "@/store/slices/backendSlice"; +type VoteQueueTab = "printing" | "artist" | "tag"; + // "Who's That Pokemon?" style radiating starburst behind the game itself, matching the // real TV bumper's look (deep navy background, bright yellow sunburst rays radiating from // center) - a CSS repeating-conic-gradient rather than a static image, so it scales to any @@ -53,16 +59,56 @@ const StarburstBackground = styled.div` function PrintingQueueOrDefault() { const remoteBackendConfigured = useRemoteBackendConfigured(); + const [activeTab, setActiveTab] = useState("printing"); + return remoteBackendConfigured ? ( <>

Who's That Planeswalker?

Test your Magic: the Gathering knowledge! One card at a time, help - identify which real-world printing each card image depicts - contested - cards come first, since they need your eyes the most. + identify which real-world printing, artist, or descriptor tag each + card image depicts - contested cards come first, since they need your + eyes the most.

- + { + if (key) setActiveTab(key as VoteQueueTab); + }} + > + + + {/* mountOnEnter: each mode's queue does its own fetching/pagination on mount - + no need to pay that cost for tabs the user hasn't visited yet. Printing + mode's own component/behavior is completely unchanged from before this tab + switcher existed. unmountOnExit on the two new tabs (not printing, to avoid + touching its existing behavior at all): without it, react-bootstrap keeps an + already-visited pane's DOM mounted (just hidden) rather than removing it, + which both leaves an inactive tab's queue silently polling for more pages in + the background and produces duplicate data-testid="vote-queue" elements in + the DOM at once. */} + + + + + + + + + + +