diff --git a/.github/scripts/tests/test_docs_lint.py b/.github/scripts/tests/test_docs_lint.py index 7bf96369f..b8c24686d 100644 --- a/.github/scripts/tests/test_docs_lint.py +++ b/.github/scripts/tests/test_docs_lint.py @@ -449,10 +449,17 @@ def test_roster_derivation_sees_the_real_calculators(self): # tether is keyed on the CURRENT identity, version suffix included. "stage-d-illustration-v2", "local-name-frequency-v1", - # Declared in `management/commands/`, which the pre-2026-07-29 - # non-recursive glob never read. If this scan ever narrows - # again, this is the assertion that says so. - "scryfall-tagger-v1", + # `scryfall-tagger-v1` stood here from 2026-07-29 as the one + # identity declared under `management/commands/` — the subtree the + # pre-2026-07-29 non-recursive glob never read. It retired together + # with `PrintingTagVote` on 2026-07-30, and no live identity is + # declared under `management/commands/` any more, so this list + # cannot carry a real-repo witness for the recursion. The recursion + # itself stays covered — on fixtures rather than on the real tree — + # by `test_scan_is_recursive_but_bounded`, which is the assertion + # that fails if the scan ever narrows again. If an identity is ever + # declared under `management/commands/` again, add it here and this + # list becomes a real-repo witness once more. ): self.assertIn(identity, found) diff --git a/MPCAutofill/cardpicker/admin.py b/MPCAutofill/cardpicker/admin.py index ea0cd0979..6bf768685 100644 --- a/MPCAutofill/cardpicker/admin.py +++ b/MPCAutofill/cardpicker/admin.py @@ -23,7 +23,6 @@ ImageEvidence, LandsAmbiguousResidue, PilotRunLedger, - PrintingTagVote, Project, ProjectMember, SavedDeck, @@ -234,14 +233,6 @@ class AdminCardTagVote(admin.ModelAdmin[CardTagVote]): raw_id_fields = ["card", "tag"] -@admin.register(PrintingTagVote) -class AdminPrintingTagVote(admin.ModelAdmin[PrintingTagVote]): - list_display = ("printing", "tag", "polarity", "source", "confidence", "anonymous_id", "created_at") - list_filter = ("source", "polarity") - search_fields = ("printing__name", "tag__name") - raw_id_fields = ["printing", "tag"] - - @admin.register(CardReport) class AdminCardReport(admin.ModelAdmin[CardReport]): list_display = ("card", "reason", "text", "anonymous_id", "user", "created_at") diff --git a/MPCAutofill/cardpicker/integrations/game/scryfall_bulk_data.py b/MPCAutofill/cardpicker/integrations/game/scryfall_bulk_data.py index e2b8b0724..d61933e0d 100644 --- a/MPCAutofill/cardpicker/integrations/game/scryfall_bulk_data.py +++ b/MPCAutofill/cardpicker/integrations/game/scryfall_bulk_data.py @@ -7,7 +7,9 @@ * `cardpicker/printing_metadata_import.py` (its own `BulkDataEntry`), * `cardpicker/integrations/game/mtg.py` (its own `BulkDataRow`/`BulkDataResponse`), - * `cardpicker/management/commands/import_external_ip_tags.py` (its own `_BulkDataEntry`). + * `cardpicker/management/commands/import_external_ip_tags.py` (its own `_BulkDataEntry`) - + RETIRED 2026-07-29 along with `PrintingTagVote`; it is named here because it is half the + reason this module exists, not because it still calls in. Two importers remain. When Scryfall retired the old bulk format (blog "Two New Ways to Sync Scryfall Data", 2026-07-01; retirement 2026-07-20), exactly ONE of the three had been hardened for it - the @@ -20,9 +22,8 @@ `GameIntegration`) that exists precisely to keep game-specific vendor knowledge out of `cardpicker/`. Putting Scryfall's wire format at `cardpicker/` level would make the generic layer own an MTG vendor's schema. The dependency direction is also already established and is -preserved here: `cardpicker.printing_metadata_import` and -`cardpicker.management.commands.import_external_ip_tags` ALREADY import from -`cardpicker.integrations.game.mtg` (both pull `Scryfall` for its headers), i.e. cardpicker -> +preserved here: `cardpicker.printing_metadata_import` ALREADY imports from +`cardpicker.integrations.game.mtg` (pulling `Scryfall` for its headers), i.e. cardpicker -> integrations. Nothing in this module imports back out of the integrations package, so no game integration is made to depend on a cardpicker-level module. @@ -70,6 +71,12 @@ # each call site. DEFAULT_CARDS = "default_cards" ORACLE_CARDS = "oracle_cards" +# `ART_TAGS` has had NO consumer since `import_external_ip_tags` was retired on 2026-07-29 with +# `PrintingTagVote`. Kept, with its live-endpoint test, because the Scryfall Tagger art-tag feed +# is the named input of the unified-Scryfall-importer work item that inherits that capability +# (docs/features/printing-tags.md's retirement record) - a bare constant plus one test is a +# cheaper thing to carry than a re-derivation of which bulk entry type the feed lives under, and +# the test is what would tell us if the entry type disappeared upstream in the meantime. ART_TAGS = "art_tags" # gzip's zlib window-bits selector: 16 + MAX_WBITS means "expect a gzip (RFC 1952) wrapper", @@ -240,8 +247,8 @@ def download_and_decompress(url: str, path: Path, timeout: int = 60) -> None: ON-DISK FORMAT NOTE: we store the file DECOMPRESSED even though the remote is compressed. Keeping it gzipped would save ~596MB of the persistent `scryfall_cache` volume but would - force every reader (`printing_metadata_import._parse_rows`, the back-face lookup, - `import_external_ip_tags`' illustration index) to re-inflate 620MB on every pass, and would + force every reader (`printing_metadata_import._parse_rows`, the back-face lookup) to + re-inflate 620MB on every pass, and would invalidate the already-deployed on-disk cache and the `ensure_scryfall_cache_present` guard that points at it. Disk is already provisioned for the uncompressed size; CPU on every read is not worth trading for it. diff --git a/MPCAutofill/cardpicker/management/commands/import_external_ip_tags.py b/MPCAutofill/cardpicker/management/commands/import_external_ip_tags.py deleted file mode 100644 index c1d8871c8..000000000 --- a/MPCAutofill/cardpicker/management/commands/import_external_ip_tags.py +++ /dev/null @@ -1,579 +0,0 @@ -""" -Imports Scryfall Tagger's `art:external-ip` community art tag (tagger.scryfall.com) as -machine-cast `PrintingTagVote` rows (fix-batch plan 2026-07-27, work item W9, revised -per-printing spec) — Universes Beyond illustrations (Lord of the Rings, Doctor Who, -Warhammer 40K, etc.), identified by the Tagger community rather than by anything in our -own image pipeline. - -Data flow (plan W9's own diagram, revised to per-printing): - Art Tags bulk JSONL (https://api.scryfall.com/bulk-data, the `art_tags` entry's - `jsonl_download_uri`, or a local file via --file) - -> find tag slug "external-ip" -> BFS its child_ids subtree (the parent tag carries no - direct taggings - Tagger bulk data only ever stores DIRECT taggings, on the leaf tags; - see https://scryfall.com/docs/api/tags) - -> collect taggings[].illustration_id across the subtree - -> join through the already-on-disk default_cards bulk data (illustration_id -> card id; - top-level on single-faced rows, per-face under card_faces for double-faced rows) - -> match CanonicalCard.identifier DIRECTLY - -> write PrintingTagVote rows for every confirmed CanonicalCard (printing), casting - BOTH polarities on the external-ip tag: polarity=APPLY for positive Tagger matches, - and polarity=NOT_APPLICABLE for confirmed printings (present in the default_cards - bulk data) NOT in the positive Tagger set. Every confirmed printing gets exactly one - machine vote; printings with no data at all abstain (no vote of either polarity). - -PER-PRINTING DESIGN (revised 2026-07-27): votes target the Scryfall printing itself -(`CanonicalCard`) rather than the catalog images (`Card`) that depict it. The same -physical printing may be depicted by many `Card` images in the catalog; the Tagger -community tag belongs to the printing once, not duplicated per image. Card-level display/ -attribution of external-ip flows FROM the printing via the metadata deduction seam -(see `printing_tag_consensus.py`), never stored per card. - -SOURCE CHOICE (why not literally source="scryfall_tagger"): `AbstractWeightedVote.source` is a -CharField(max_length=10, choices=VoteSource.choices), the plan explicitly requires NO model/ -migration change beyond PrintingTagVote itself, and "scryfall_tagger" (15 chars) is not a -VoteSource value. The established convention for a new machine caster is an existing machine -VoteSource plus its OWN anonymous_id -(local-ocr-v1/local-phash-v1/local-fallback-v1/deductive-backfill-v1/ai-art-detector-v1 - -see local_detect_ai_art.py's AI_ART_ANONYMOUS_ID comment). This import is pure logical -inference from already-trusted structured data with ZERO image inspection - exactly -`VoteSource.DEDUCTION`'s own definition (VoteSource's docstring) - so votes are written as -(source=DEDUCTION, anonymous_id="scryfall-tagger-v1"). Weight resolves to -PRINTING_TAG_MACHINE_WEIGHT (default 0.5) via vote_consensus._SOURCE_WEIGHTS, and the -2026-07-23 zero-weight override is scoped (as re-scoped by the 2026-07-29 owner clarification) -to the single frozen 2026-07-14 deductive-backfill RUN - source=DEDUCTION, the -"deductive-backfill" calculator family, AND vote_consensus.DEDUCTIVE_BACKFILL_ZERO_WEIGHT_RUN_ID -on the row's run_id, all three together - so these votes are unaffected by it. - -RE-RUN SEMANTICS (matches the existing machine-vote casters exactly): the -(printing, tag, anonymous_id) uniqueness constraint on PrintingTagVote plus the eligibility -exclusion below makes re-runs idempotent - a printing this identity already voted on is -skipped, so a second invocation only ever votes on NEW matches (e.g. illustrations the Tagger -community tagged since the last run). RETRACTION is the existing run-scoped mechanism, not -anything bespoke: every invocation stamps a fresh run_id on its votes, and -`manage.py purge_machine_votes --run-id ` deletes exactly one invocation's votes. To -refresh against updated Tagger data where a printing was UN-tagged upstream: purge the old -run_id, then re-run. -""" - -import tempfile -import uuid -from dataclasses import dataclass, field -from pathlib import Path -from typing import Any, Optional - -import requests -from pydantic import BaseModel, ValidationError - -from django.core.management.base import BaseCommand, CommandError -from django.utils import timezone - -from cardpicker.integrations.game import scryfall_bulk_data -from cardpicker.local_identify_printing_tags import generate_run_id -from cardpicker.models import ( - CanonicalCard, - PilotRunLedger, - PrintingTagVote, - Tag, - VotePolarity, - VoteSource, -) -from cardpicker.printing_metadata_import import ( - _cache_path, - ensure_scryfall_cache_present, -) -from cardpicker.utils import find_stale_applied_migrations, get_baked_git_sha -from cardpicker.vote_write import purge_and_write_votes - -# Own anonymous_id, per the per-engine convention (module docstring's SOURCE CHOICE section) - -# independently purgeable/re-runnable via the existing purge_machine_votes --run-id mechanism, -# and the (printing, tag, anonymous_id) uniqueness constraint is what makes "skip a printing -# already voted by this identity" a plain query rather than bespoke bookkeeping. -SCRYFALL_TAGGER_ANONYMOUS_ID = "scryfall-tagger-v1" - -# The Tagger slug to look up in the art_tags bulk data (a URL-safe identifier that Scryfall -# warns may change over time - if a future run fails to find it, check the Tagger site for the -# tag's current slug; the tag `id` UUID is the stable reference, but slug lookup matches the -# plan's own data flow and keeps the fixture human-readable). -EXTERNAL_IP_TAG_SLUG = "external-ip" - -# Our own Tag.name these votes are cast for. Tag.name is the immutable machine key (votes, -# Card.tags, federation) - deliberately the same string as the Tagger slug so the provenance is -# obvious, but the two are independent namespaces and must not be conflated elsewhere. -EXTERNAL_IP_TAG_NAME = "external-ip" - -_ART_TAGS_BULK_TYPE = scryfall_bulk_data.ART_TAGS - - -# NOTE: this module used to declare its OWN `_BulkDataEntry`/`_BulkDataResponse` here - the one -# of the three importer-local copies that had been hardened for the JSONL cutover, carrying -# `jsonl_download_uri: Optional[str] = None` with a fallback to `download_uri`. That fallback is -# now DEAD: `download_uri` no longer exists on any entry (retired 2026-07-20), so the "optional" -# field is in fact the only one there is. The model lives once now, in `scryfall_bulk_data`, -# with `jsonl_download_uri` REQUIRED - which is what makes the next upstream field removal fail -# loudly in CI instead of silently resolving to None. That exactly one of three importers had -# been hardened, while the other two hard-failed in production, is why that module exists. - - -class _Tagging(BaseModel): - # Nullable per Scryfall's own tags documentation (oracle taggings carry oracle_id instead) - - # art_tags rows should always have it, but a null is skipped rather than crashing the pass. - illustration_id: Optional[uuid.UUID] = None - - -class _TagRow(BaseModel): - # A Tagger tag object (https://scryfall.com/docs/api/tags). Extra keys (label, uri, type, - # description, parent_ids, aliases, weight on taggings, ...) are silently ignored by - # pydantic, same convention as printing_metadata_import.PrintingMetadataRow. - id: uuid.UUID - slug: str - child_ids: Optional[list[uuid.UUID]] = None - taggings: list[_Tagging] = [] - - -class _DefaultCardsRow(BaseModel): - # Just the two shapes this import joins on, out of the full Scryfall card object - matching - # PrintingMetadataRow's own "curated subset, extras ignored" convention. Single-faced cards - # carry illustration_id top-level; double-faced cards nest one illustration_id per face - # under card_faces (Scryfall's documented convention, same as PrintingMetadataRow's - # art_crop_url handling). - id: uuid.UUID - illustration_id: Optional[uuid.UUID] = None - card_faces: Optional[list[dict[str, Any]]] = None - - -# This module's own tolerant JSONL(.gz)/pretty-array reader has moved to -# `scryfall_bulk_data.iter_json_lines` - same behaviour, one copy, now shared with -# `printing_metadata_import._parse_rows` and `mtg.py`'s catalog pass. Aliased rather than -# renamed at every call site so this module's three passes stay readable. -_iter_json_lines = scryfall_bulk_data.iter_json_lines - - -def find_external_ip_subtree(tags_path: Path) -> tuple[set[uuid.UUID], int]: - """ - Pass 1 over the tag bulk data: index every tag's (id -> slug + child_ids), find the tag - whose slug is EXTERNAL_IP_TAG_SLUG, and BFS its child_ids subtree to the full set of tag ids - whose taggings count (root included - it carries no direct taggings per Scryfall's docs, but - including it is harmless if that ever changes). Returns (subtree tag ids, total tags seen). - - Deliberately BFS to fixpoint rather than plan W9's literal one-level "~56 child tags" - traversal: the Tagger hierarchy may deepen under external-ip later (a child IP tag gaining - its own children), and the subtree closure costs nothing extra since child_ids are already - in memory. Raises RuntimeError if the slug is absent (slugs may change - see - EXTERNAL_IP_TAG_SLUG's own comment) so a taxonomy rename fails LOUD rather than silently - importing zero votes. - """ - slug_by_id: dict[uuid.UUID, str] = {} - child_ids_by_id: dict[uuid.UUID, list[uuid.UUID]] = {} - tags_seen = 0 - for line in _iter_json_lines(tags_path): - try: - row = _TagRow.model_validate_json(line) - except ValidationError: - continue # same skip-malformed tolerance as printing_metadata_import._parse_rows - tags_seen += 1 - slug_by_id[row.id] = row.slug - child_ids_by_id[row.id] = row.child_ids or [] - - root_id = next((tag_id for tag_id, slug in slug_by_id.items() if slug == EXTERNAL_IP_TAG_SLUG), None) - if root_id is None: - raise RuntimeError( - f"Tag slug {EXTERNAL_IP_TAG_SLUG!r} not found in {tags_path} ({tags_seen} tags parsed) - " - "Tagger slugs may change over time (Scryfall's own tags documentation warns against " - "treating them as permanent identifiers); check tagger.scryfall.com for the tag's " - "current slug before re-running." - ) - - subtree: set[uuid.UUID] = {root_id} - frontier = [root_id] - while frontier: - next_frontier = [] - for tag_id in frontier: - for child_id in child_ids_by_id.get(tag_id, []): - if child_id not in subtree: - subtree.add(child_id) - next_frontier.append(child_id) - frontier = next_frontier - return subtree, tags_seen - - -def collect_illustration_ids(tags_path: Path, subtree: set[uuid.UUID]) -> set[uuid.UUID]: - """ - Pass 2 over the tag bulk data: the set of every illustration_id tagged by any tag in the - subtree. Kept separate from find_external_ip_subtree so pass 1 never holds the (much - larger) taggings payload in memory - only ids/slugs/child_ids survive it. - """ - illustration_ids: set[uuid.UUID] = set() - for line in _iter_json_lines(tags_path): - try: - row = _TagRow.model_validate_json(line) - except ValidationError: - continue - if row.id not in subtree: - continue - for tagging in row.taggings: - if tagging.illustration_id is not None: - illustration_ids.add(tagging.illustration_id) - return illustration_ids - - -def build_illustration_index(default_cards_path: Path) -> dict[uuid.UUID, set[uuid.UUID]]: - """ - illustration_id -> set of Scryfall card ids (`id`, the printing UUID that CanonicalCard. - identifier stores), from the same on-disk default_cards bulk data import_canonical_card_data/ - import_scryfall_printing_metadata already maintain. One illustration maps to N printings - (reprints reusing the same art all count - the ART is what's tagged), and a double-faced - row contributes one entry per face (each face is its own illustration). - """ - index: dict[uuid.UUID, set[uuid.UUID]] = {} - for line in _iter_json_lines(default_cards_path): - try: - row = _DefaultCardsRow.model_validate_json(line) - except ValidationError: - continue - illustration_ids = [row.illustration_id] if row.illustration_id is not None else [] - if row.card_faces: - for face in row.card_faces: - face_illustration_id = face.get("illustration_id") - if face_illustration_id: - illustration_ids.append(uuid.UUID(face_illustration_id)) - for illustration_id in illustration_ids: - index.setdefault(illustration_id, set()).add(row.id) - return index - - -@dataclass -class ExternalIpImportResult: - dry_run: bool = False - run_id: str = "" - tags_seen: int = 0 - subtree_tag_count: int = 0 - illustrations_tagged: int = 0 - # skip-count reasons: illustrations no default_cards row mapped (art not in any English - # printing - e.g. art-series-only illustrations), and printings no CanonicalCard row - # matched (not canonical per import_canonical_card_data's own filtering). - skip_counts: dict[str, int] = field(default_factory=dict) - canonical_cards_matched: int = 0 - printings_eligible: int = 0 - votes_would_cast: int = 0 - votes_written: int = 0 - audit: list[dict[str, object]] = field(default_factory=list) - negative_printings_eligible: int = 0 - negative_votes_would_cast: int = 0 - negative_votes_written: int = 0 - negative_audit: list[dict[str, object]] = field(default_factory=list) - - -def run_external_ip_tag_import( - tags_path: Path, - default_cards_path: Path, - run_id: Optional[str] = None, - dry_run: bool = True, - chunk_size: int = 500, - audit_sample_size: int = 20, -) -> ExternalIpImportResult: - """ - The import itself - a plain, testable function with Command.handle() kept thin, matching - this module family's own convention (purge_machine_votes.purge_run, local_detect_ai_art. - run_ai_art_detector). `dry_run=True` (the default, matching every other Stage 3+ command's - opt-in-to-write convention) parses/joins/counts everything without writing any - PrintingTagVote row. GATE VERIFICATION is layered on by the management command, not here - - same split as run_ai_art_detector/purge_machine_votes (see their docstrings). - """ - run_id = run_id or generate_run_id() - result = ExternalIpImportResult(dry_run=dry_run, run_id=run_id) - - subtree, result.tags_seen = find_external_ip_subtree(tags_path) - result.subtree_tag_count = len(subtree) - illustration_ids = collect_illustration_ids(tags_path, subtree) - result.illustrations_tagged = len(illustration_ids) - - illustration_index = build_illustration_index(default_cards_path) - all_card_ids: set[uuid.UUID] = set() - for indexed_card_ids in illustration_index.values(): - all_card_ids.update(indexed_card_ids) - candidate_identifiers: set[uuid.UUID] = set() - unmatched_illustrations = 0 - for illustration_id in illustration_ids: - card_ids = illustration_index.get(illustration_id) - if not card_ids: - unmatched_illustrations += 1 - continue - candidate_identifiers.update(card_ids) - if unmatched_illustrations: - result.skip_counts["illustration-not-in-default-cards"] = unmatched_illustrations - negative_identifiers = all_card_ids - candidate_identifiers - - tag, _ = Tag.objects.get_or_create(name=EXTERNAL_IP_TAG_NAME) - - # Count total canonical cards matched (for reporting, before eligibility exclusion). - all_matched = CanonicalCard.objects.filter(identifier__in=candidate_identifiers) - result.canonical_cards_matched = all_matched.count() - unmatched_printings = len(candidate_identifiers) - result.canonical_cards_matched - if unmatched_printings: - result.skip_counts["printing-not-canonical"] = unmatched_printings - - # Eligible = matched minus already-voted by this identity for this tag (idempotency). - # Direct printing-level join: no Card-level inference or effective-printing logic needed — - # the illustration_id -> CanonicalCard.identifier join is the complete eligibility check. - # The single-exclude-call form (both conditions on the same related row) is correct here, - # matching the same pattern local_detect_ai_art._eligible_cards_queryset uses. - eligible_printings = all_matched.exclude( - printing_tag_votes__anonymous_id=SCRYFALL_TAGGER_ANONYMOUS_ID, - printing_tag_votes__tag=tag, - ).distinct() - - votes_batch: list[PrintingTagVote] = [] - for printing in eligible_printings.iterator(chunk_size=chunk_size): - result.printings_eligible += 1 - result.votes_would_cast += 1 - if len(result.audit) < audit_sample_size: - result.audit.append({"printing_id": str(printing.identifier), "printing_name": printing.name}) - if not dry_run: - votes_batch.append( - PrintingTagVote( - printing_id=printing.pk, - tag=tag, - polarity=VotePolarity.APPLY, - anonymous_id=SCRYFALL_TAGGER_ANONYMOUS_ID, - source=VoteSource.DEDUCTION, - run_id=run_id, - ) - ) - - if not dry_run: - # ignore_conflicts=True: belt-and-suspenders against the (printing, tag, anonymous_id) - # uniqueness constraint - the eligibility query above already excludes any printing this - # identity has voted on, so a conflict here would only ever come from two concurrent - # invocations racing, not from this invocation's own logic. - # - # CANCEL-SAFETY (2026-07-28): purge and insert are two separate statements, so an import - # killed between them left the affected PRINTINGS with their previous same-family vote - # deleted and nothing written back. `vote_write.purge_and_write_votes` runs the pair - # inside one `transaction.atomic()`, scoped to exactly the rows it inserts. This is the - # only `printing_id`-keyed call site in the app - hence the explicit `target_field`. - purge_and_write_votes( - PrintingTagVote, - votes_batch, - anonymous_id=SCRYFALL_TAGGER_ANONYMOUS_ID, - target_field="printing_id", - ignore_conflicts=True, - ) - result.votes_written = len(votes_batch) - - # Negative pass: vote NOT_APPLICABLE for confirmed printings not in the positive set - negative_printings = ( - CanonicalCard.objects.filter(identifier__in=negative_identifiers) - .exclude( - printing_tag_votes__anonymous_id=SCRYFALL_TAGGER_ANONYMOUS_ID, - printing_tag_votes__tag=tag, - ) - .distinct() - ) - - negative_batch: list[PrintingTagVote] = [] - for printing in negative_printings.iterator(chunk_size=chunk_size): - result.negative_printings_eligible += 1 - result.negative_votes_would_cast += 1 - if len(result.negative_audit) < audit_sample_size: - result.negative_audit.append({"printing_id": str(printing.identifier), "printing_name": printing.name}) - if not dry_run: - negative_batch.append( - PrintingTagVote( - printing_id=printing.pk, - tag=tag, - polarity=VotePolarity.NOT_APPLICABLE, - anonymous_id=SCRYFALL_TAGGER_ANONYMOUS_ID, - source=VoteSource.DEDUCTION, - run_id=run_id, - ) - ) - - if not dry_run and negative_batch: - # CANCEL-SAFETY (2026-07-28) - identical reasoning to the positive pass above; see - # `vote_write.purge_and_write_votes`' docstring. - purge_and_write_votes( - PrintingTagVote, - negative_batch, - anonymous_id=SCRYFALL_TAGGER_ANONYMOUS_ID, - target_field="printing_id", - ignore_conflicts=True, - ) - result.negative_votes_written = len(negative_batch) - - return result - - -def _find_art_tags_download_uri() -> str: - """ - The `art_tags` bulk entry's `.jsonl.gz` URL, via the one shared index fetch + lookup-by-type. - `CommandError` is preserved as this command's own failure surface (Django prints it without a - traceback), wrapping the shared lookup's RuntimeError. - """ - try: - entry = scryfall_bulk_data.get_bulk_data_entry(_ART_TAGS_BULK_TYPE) - except RuntimeError as e: - raise CommandError(str(e)) from e - return entry.jsonl_download_uri - - -class Command(BaseCommand): - help = ( - "Imports Scryfall Tagger's art:external-ip community tag (and its child-IP tags) as " - "machine PrintingTagVote rows (source=deduction, anonymous_id=scryfall-tagger-v1, " - "machine weight 0.5) for CanonicalCard (printing) rows whose illustration the Tagger " - "community tagged - fix-batch plan 2026-07-27 W9 (revised per-printing spec). Defaults " - "to dry-run and requires an explicit --write to actually write, matching every other " - "Stage 3+ command's own convention. Idempotent on re-run via the " - "(printing, tag, anonymous_id) uniqueness constraint; retract one invocation's votes " - "with purge_machine_votes --run-id ." - ) - - def add_arguments(self, parser: Any) -> None: - parser.add_argument( - "--file", - type=Path, - default=None, - help="Local art-tags JSONL(.gz) file to parse instead of downloading the current " - "bulk data from Scryfall (also accepts a pretty-printed .json array). Tests use this.", - ) - parser.add_argument( - "--default-cards", - type=Path, - default=None, - help="Local default_cards bulk file for the illustration_id join. Default: the " - "shared scryfall_cache/default_cards.json cache import_canonical_card_data/" - "import_scryfall_printing_metadata already maintain (must exist - this command " - "never downloads it).", - ) - parser.add_argument( - "--write", - action="store_true", - default=False, - help="Actually write PrintingTagVote rows. Default is dry-run: parse, join, and " - "count everything without writing.", - ) - parser.add_argument( - "--dry-run", - action="store_true", - default=False, - help="Explicit dry-run (the default when --write is absent) - counts only, no writes. " - "Passing both --write and --dry-run is an error.", - ) - parser.add_argument("--run-id", default=None, help="Reuse a specific run_id. Default: freshly generated.") - parser.add_argument( - "--chunk-size", type=int, default=500, help="Queryset .iterator() chunk size. Default: 500." - ) - - def handle(self, *args: Any, **kwargs: Any) -> None: - stale = find_stale_applied_migrations() - if stale: - raise CommandError( - f"STALE IMAGE: the DB has {len(stale)} migration(s) applied that this image's " - f"own code doesn't know about ({stale[:10]}{'...' if len(stale) > 10 else ''}) - " - "this image is older than a previously-deployed one. Rebuild with the current " - "code before running this command." - ) - - if kwargs["write"] and kwargs["dry_run"]: - raise CommandError("Pass only one of --write / --dry-run.") - dry_run = not kwargs["write"] - run_id = kwargs["run_id"] or generate_run_id() - mode = "DRY RUN" if dry_run else "WRITE" - print(f"[{mode}] import_external_ip_tags run_id={run_id} git_sha={get_baked_git_sha()}") - - # The illustration join needs the default_cards bulk data. Explicit path wins (tests); - # otherwise the shared cache MUST already exist - downloading 600MB implicitly inside a - # vote-casting command would hide a big side effect, and the staleness guard's own - # CommandError names the two commands that populate it. - _raw_default_cards = kwargs["default_cards"] - default_cards_path: Path = Path(_raw_default_cards) if _raw_default_cards else _cache_path() - if _raw_default_cards is None: - ensure_scryfall_cache_present(default_cards_path) - - _raw_file = kwargs["file"] - tags_path: Optional[Path] = Path(_raw_file) if _raw_file is not None else None - if tags_path is None: - uri = _find_art_tags_download_uri() - print(f"Downloading art tags from {uri}") - # Deliberately kept GZIPPED on disk here, unlike the default_cards cache: this is a - # transient tempfile deleted in the `finally` below, read exactly once, and - # `_iter_json_lines` gunzips `.gz` transparently - so inflating ~40MB to several - # hundred MB in /tmp would buy nothing. The persistent default_cards cache makes the - # opposite trade (see scryfall_bulk_data.download_and_decompress' own note) because - # it is re-read on every pass by several commands. The suffix is derived from the - # URL rather than assumed, so a future non-gzip artefact still lands readable. - suffix = ".jsonl.gz" if uri.endswith(".gz") else ".jsonl" - with tempfile.NamedTemporaryFile(prefix="art-tags-", suffix=suffix, delete=False) as tmp: - tmp_path = Path(tmp.name) - with requests.get(uri, stream=True, headers=scryfall_bulk_data.get_headers(), timeout=60) as r: - r.raise_for_status() - for chunk in r.iter_content(chunk_size=1024 * 1024): - tmp.write(chunk) - tags_path = tmp_path - print(f"Tag data: {tags_path} (default_cards: {default_cards_path})") - - ledger = PilotRunLedger.objects.create( - run_id=run_id, - command="import_external_ip_tags", - dry_run=dry_run, - status=PilotRunLedger.Status.RUNNING, - git_sha=get_baked_git_sha(), - ) - - try: - result = run_external_ip_tag_import( - tags_path=tags_path, - default_cards_path=default_cards_path, - run_id=run_id, - dry_run=dry_run, - chunk_size=kwargs["chunk_size"], - ) - print( - f"[external-ip] tags_seen={result.tags_seen} subtree_tags={result.subtree_tag_count} " - f"illustrations={result.illustrations_tagged} canonical_cards={result.canonical_cards_matched} " - f"eligible_printings={result.printings_eligible} " - f"negative_eligible={result.negative_printings_eligible} " - f"{'positive_written=' + str(result.votes_written) if not dry_run else 'positive_would_cast=' + str(result.votes_would_cast)} " - f"{'negative_written=' + str(result.negative_votes_written) if not dry_run else 'negative_would_cast=' + str(result.negative_votes_would_cast)} " - f"skip_counts={dict(result.skip_counts)}" - ) - for entry in result.audit[:10]: - print(f" sample: {entry}") - - # Note: verify_no_machine_only_resolutions (purge_machine_votes) is card-level - # (checks Card.printing_tag_status / tag_vote_statuses). PrintingTagVote writes do - # not affect any Card-level resolution status — per-printing consensus resolution - # is a separate concern tracked in printing_tag_consensus.py, with no persisted - # Card-side status field today. The gate is therefore not applicable here and is - # omitted; it remains available for CardTagVote/CardPrintingTag/CardArtistVote runs. - - ledger.status = PilotRunLedger.Status.COMPLETED - ledger.finished_at = timezone.now() - ledger.votes_written = result.votes_written + result.negative_votes_written - ledger.save(update_fields=["status", "finished_at", "votes_written"]) - print( - f"[{mode}] done. run_id={run_id} " - f"total_votes_{'written' if not dry_run else 'would_cast'}=" - f"{result.votes_written + result.negative_votes_written if not dry_run else result.votes_would_cast + result.negative_votes_would_cast}" - ) - except Exception: - ledger.status = PilotRunLedger.Status.FAILED - ledger.finished_at = timezone.now() - ledger.save(update_fields=["status", "finished_at"]) - raise - finally: - # Only the downloaded tempfile is ours to remove - a --file path is the caller's. - if kwargs["file"] is None and tags_path is not None: - tags_path.unlink(missing_ok=True) - - -__all__ = [ - "SCRYFALL_TAGGER_ANONYMOUS_ID", - "EXTERNAL_IP_TAG_SLUG", - "EXTERNAL_IP_TAG_NAME", - "find_external_ip_subtree", - "collect_illustration_ids", - "build_illustration_index", - "ExternalIpImportResult", - "run_external_ip_tag_import", -] diff --git a/MPCAutofill/cardpicker/management/commands/purge_machine_votes.py b/MPCAutofill/cardpicker/management/commands/purge_machine_votes.py index 355e786ba..2460862ae 100644 --- a/MPCAutofill/cardpicker/management/commands/purge_machine_votes.py +++ b/MPCAutofill/cardpicker/management/commands/purge_machine_votes.py @@ -71,7 +71,6 @@ CardTagVote, PilotRunLedger, PrintingTagStatus, - PrintingTagVote, TagVoteStatus, VoteSource, calculator_family, @@ -99,7 +98,6 @@ class PurgeResult: printing_votes_deleted: int = 0 artist_votes_deleted: int = 0 tag_votes_deleted: int = 0 - printing_tag_votes_deleted: int = 0 # `CardScanLog` rows removed - always 0 unless the `--anonymous-id` mode was run with # `--include-scan-log`. Reported as its OWN number, never folded into the vote totals: it is # audit-trail data, not votes, and an operator reading the output needs to see which of the @@ -136,9 +134,10 @@ def verify_no_machine_only_resolutions(card_ids: list[int]) -> list[int]: it means something upstream is broken, not that the purge itself did anything wrong. NOTE: this function checks Card-level resolution status only (printing_tag_status, - artist_vote_status, tag_vote_statuses). PrintingTagVote rows have no persisted per-printing - resolution status on CanonicalCard today, so per-printing consensus is not checked here. - When a per-printing resolution status field is added, add the analogous check then. + artist_vote_status, tag_vote_statuses). There is no per-CanonicalCard resolution status in + the schema to check: the one model that would have needed one, `PrintingTagVote`, was + retired on 2026-07-29 (migration 0101) having never had a resolver or a row. If a + per-printing resolution status field is ever added, add the analogous check then. Returns the list of violating card pks (empty means clean). """ @@ -234,7 +233,6 @@ def purge_by_anonymous_id(anonymous_id: str, dry_run: bool = False, include_scan printing_votes = CardPrintingTag.objects.filter(anonymous_id=anonymous_id, source__in=_MACHINE_SOURCES) artist_votes = CardArtistVote.objects.filter(anonymous_id=anonymous_id, source__in=_MACHINE_SOURCES) tag_votes = CardTagVote.objects.filter(anonymous_id=anonymous_id, source__in=_MACHINE_SOURCES) - printing_tag_votes = PrintingTagVote.objects.filter(anonymous_id=anonymous_id, source__in=_MACHINE_SOURCES) # NOT source-filtered: CardScanLog has no `source` field at all (it is an abstention record, # not a vote - see the model's own docstring). The `calculator_family` guard above is what # keeps this exact-id filter off human territory, and only a calculator ever writes here. @@ -248,7 +246,6 @@ def purge_by_anonymous_id(anonymous_id: str, dry_run: bool = False, include_scan printing_count = printing_votes.count() artist_count = artist_votes.count() tag_count = tag_votes.count() - printing_tag_count = printing_tag_votes.count() # Counted even when the flag is off, but reported as "would be left behind" rather than # "deleted" - see Command.handle. An operator who forgot the flag needs to see the number # they are leaving in place, since it is the number that will keep their cards ineligible. @@ -261,7 +258,6 @@ def purge_by_anonymous_id(anonymous_id: str, dry_run: bool = False, include_scan printing_votes_deleted=printing_count, artist_votes_deleted=artist_count, tag_votes_deleted=tag_count, - printing_tag_votes_deleted=printing_tag_count, scan_log_rows_deleted=scan_log_count if include_scan_log else 0, include_scan_log=include_scan_log, affected_card_count=len(affected_card_ids), @@ -270,9 +266,6 @@ def purge_by_anonymous_id(anonymous_id: str, dry_run: bool = False, include_scan printing_votes.delete() artist_votes.delete() tag_votes.delete() - # PrintingTagVote rows are deleted but require no re-resolution - there is no persisted - # per-printing resolution status on CanonicalCard today, so consensus is computed on demand. - printing_tag_votes.delete() if include_scan_log: scan_log_rows.delete() @@ -285,7 +278,6 @@ def purge_by_anonymous_id(anonymous_id: str, dry_run: bool = False, include_scan printing_votes_deleted=printing_count, artist_votes_deleted=artist_count, tag_votes_deleted=tag_count, - printing_tag_votes_deleted=printing_tag_count, scan_log_rows_deleted=scan_log_count if include_scan_log else 0, include_scan_log=include_scan_log, affected_card_count=len(affected_card_ids), @@ -310,7 +302,6 @@ def purge_run(run_id: str, dry_run: bool = False) -> PurgeResult: printing_votes = CardPrintingTag.objects.filter(run_id=run_id) artist_votes = CardArtistVote.objects.filter(run_id=run_id) tag_votes = CardTagVote.objects.filter(run_id=run_id) - printing_tag_votes = PrintingTagVote.objects.filter(run_id=run_id) affected_card_ids: set[int] = set() affected_card_ids.update(printing_votes.values_list("card_id", flat=True)) @@ -320,7 +311,6 @@ def purge_run(run_id: str, dry_run: bool = False) -> PurgeResult: printing_count = printing_votes.count() artist_count = artist_votes.count() tag_count = tag_votes.count() - printing_tag_count = printing_tag_votes.count() if dry_run: return PurgeResult( @@ -329,16 +319,12 @@ def purge_run(run_id: str, dry_run: bool = False) -> PurgeResult: printing_votes_deleted=printing_count, artist_votes_deleted=artist_count, tag_votes_deleted=tag_count, - printing_tag_votes_deleted=printing_tag_count, affected_card_count=len(affected_card_ids), ) printing_votes.delete() artist_votes.delete() tag_votes.delete() - # PrintingTagVote rows are deleted but require no re-resolution - there is no persisted - # per-printing resolution status on CanonicalCard today, so consensus is computed on demand. - printing_tag_votes.delete() cards_unresolved = _re_resolve_and_count_unresolved(affected_card_ids) gate_violations = verify_no_machine_only_resolutions(sorted(affected_card_ids)) @@ -351,7 +337,6 @@ def purge_run(run_id: str, dry_run: bool = False) -> PurgeResult: printing_votes_deleted=printing_count, artist_votes_deleted=artist_count, tag_votes_deleted=tag_count, - printing_tag_votes_deleted=printing_tag_count, affected_card_count=len(affected_card_ids), cards_unresolved_by_purge=cards_unresolved, gate_violations=gate_violations, @@ -422,7 +407,13 @@ def _report_counts(result: PurgeResult) -> None: The per-vote-table line keeps its exact pre-2026-07-28 wording so an operator's (or a log scraper's) existing reading of `--run-id` output is unchanged - the new information - is added around it, never by rewording it. + is added around it, never by rewording it. ONE DELIBERATE EXCEPTION, 2026-07-29: the + `printing tag votes: N` field is gone, along with `PurgeResult.printing_tag_votes_deleted`, + because `PrintingTagVote` itself is gone (migration 0101). The alternative - keeping the + field pinned at 0 forever for output-shape stability - was rejected: a purge report naming + a table that no longer exists is a claim about work this command did not do, and this + module's whole design premise is that its output must be sufficient on its own to + reconstruct what was destroyed. Three vote tables are enumerated here now, not four. """ verb = "WOULD DELETE" if result.dry_run else "DELETED" target = f"--anonymous-id={result.anonymous_id}" if result.anonymous_id else f"--run-id={result.run_id}" @@ -431,7 +422,6 @@ def _report_counts(result: PurgeResult) -> None: f"printing votes: {result.printing_votes_deleted}, " f"artist votes: {result.artist_votes_deleted}, " f"tag votes: {result.tag_votes_deleted}, " - f"printing tag votes: {result.printing_tag_votes_deleted}, " f"affected cards: {result.affected_card_count}" ) if result.include_scan_log: diff --git a/MPCAutofill/cardpicker/migrations/0101_delete_printingtagvote.py b/MPCAutofill/cardpicker/migrations/0101_delete_printingtagvote.py new file mode 100644 index 000000000..c7e256f72 --- /dev/null +++ b/MPCAutofill/cardpicker/migrations/0101_delete_printingtagvote.py @@ -0,0 +1,64 @@ +""" +Drop `cardpicker_printingtagvote` (owner ruling, 2026-07-29: retire `PrintingTagVote`). + +SAFE TO RUN BECAUSE THE TABLE IS EMPTY, VERIFIED AGAINST PRODUCTION RATHER THAN ASSUMED. +`PrintingTagVote.objects.count()` was `0` on the live database on 2026-07-29 (read-only shell, +re-confirmed by the author of this migration on top of PR #599's own measurement), with 0 rows of +any source and therefore 0 human votes. Human votes are the one thing in this system that cannot +be re-derived; there are none here, so there is nothing to migrate, export or rescue. If a future +operator finds this migration unapplied on a database where the table is NOT empty, STOP: dump the +rows before proceeding. `DeleteModel` is not reversible in the sense that matters - reversing it +recreates an EMPTY table, never the rows. + +WHY, IN ONE LINE: it had no consensus resolver anywhere in the repo's history, no reader outside +the Django admin, no frontend caller of its submit endpoint, and its only machine writer +(`manage.py import_external_ip_tags`, retired in the same change) never ran once. See PR #599 §§3.1, +7.1 and 8 (its report, `2026-07-29-printing-vs-illustration-tag-grain.md`, lands under docs/reports/ +when that PR merges), and the retirement record in docs/features/printing-tags.md. + +NOT `CardPrintingTag`, WHICH HAS 167,229 ROWS AND IS THE ENTIRE STAGE D PRINTING CHANNEL. The two +names share four characters and nothing else; an earlier orchestrator conflated the two while +briefing this very change, and a closely-related identity-vs-channel confusion destroyed 53,966 +vote rows on this project on 2026-07-27 (operations correction log OPS-CORR-0008). The table this +migration drops is `cardpicker_printingtagvote`. `cardpicker_cardprintingtag` is untouched. + +MIGRATION NUMBERING - READ BEFORE MERGING, AND WHY THIS BRANCH'S CI IS RED ON PURPOSE. +This depends on `0100_superseded_card_printing_tag_archive` (PR #604). THAT MIGRATION IS NOT ON +`master` YET, so `migrate` on this branch alone fails with `NodeNotFoundError` and every test +errors at `pytest-django`'s test-database setup. That red is expected and correct, and it clears +the moment #604 merges. Do not "fix" it by repointing at `0099`. + + => MERGE ORDER IS FIXED: #604 FIRST, THEN THIS. Chosen deliberately on 2026-07-30 while + repairing both PRs together. #604 is the larger change and the Stage-D monolith's core, so + it is the one that gets to be verifiable and green on its own branch; this PR is small and + cheap to hold. The two orders are symmetric - exactly one of the two branches can be + migratable at a time, because only one of them can own the dependency on `master`'s real + leaf - so the choice is which PR to leave unverifiable, not whether to leave one. + +The rejected alternative, recorded so it is not re-proposed: depend on `0099_rename_printings_count_catalogued` +(`master`'s real leaf today) and renumber this 0100. That makes THIS branch green immediately, but +#604 already holds 0100-on-0099, so after both merge `cardpicker` has TWO leaves - 0100 and 0100's +sibling - and a two-leaf graph is not a cosmetic problem: `pytest-django` builds its test database +by running `migrate`, so it takes EVERY branch's CI in the repository down at test-database setup, +not just the offender's. PR #576 exists to repair exactly that, by hand. A local red on one PR +costs one PR; a forked graph costs the whole repo. The earlier version of this docstring preferred +the green-now convention, on the then-true premise that no other PR had claimed a number on top of +`master`'s leaf. #604 has now claimed 0100, which inverts the trade-off. + +The chain is therefore 0098 (#573) -> 0099 (#601) -> 0100 (#604) -> 0101 (this), and it is +verified mechanically rather than by eye: `MigrationLoader(None).graph.leaf_nodes("cardpicker")` +returns exactly one node on the simulated merge of `master` + #604 + this branch. PR #611's +"One leaf per app (merged with the base branch)" job is what enforces it from here on. +""" + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("cardpicker", "0100_superseded_card_printing_tag_archive"), + ] + + operations = [ + migrations.DeleteModel(name="PrintingTagVote"), + ] diff --git a/MPCAutofill/cardpicker/models.py b/MPCAutofill/cardpicker/models.py index b1c02f0ad..d7413bbe6 100755 --- a/MPCAutofill/cardpicker/models.py +++ b/MPCAutofill/cardpicker/models.py @@ -1468,35 +1468,27 @@ def __str__(self) -> str: return f"[{self.source}] {self.card.name} -> {self.tag} ({VotePolarity(self.polarity).label})" -class PrintingTagVote(AbstractWeightedVote): - """ - A vote on whether a descriptor `Tag` applies to a `CanonicalCard` (Scryfall printing). - - DISTINCT FROM `CardPrintingTag`: that model votes on WHICH printing a `Card` (catalog image) - depicts. This model votes on whether a descriptor tag (e.g., "external-ip") applies to the - printing itself — independent of any specific catalog image. - - DISTINCT FROM `CardTagVote`: that model votes per `Card` (catalog image). This model votes - per `CanonicalCard` (printing). The same physical printing may be depicted by many `Card` - images in this catalog; this vote belongs to the printing once, not duplicated per image. - - Uniqueness is (printing, tag, anonymous_id) — the same `update_or_create` idiom as - `CardTagVote`'s own (card, tag, anonymous_id) constraint — so a voter can independently - update their opinion on one tag for a printing via update_or_create without affecting any - other vote they've cast on the same printing. - """ - - printing = models.ForeignKey(to=CanonicalCard, on_delete=models.CASCADE, related_name="printing_tag_votes") - tag = models.ForeignKey(to=Tag, on_delete=models.CASCADE, related_name="printing_votes") - polarity = models.SmallIntegerField(choices=VotePolarity.choices) - - class Meta: - constraints = [ - models.UniqueConstraint(fields=["printing", "tag", "anonymous_id"], name="printingtagvote_unique_vote"), - ] - - def __str__(self) -> str: - return f"[{self.source}] {self.printing} -> {self.tag} ({VotePolarity(self.polarity).label})" +# RETIRED 2026-07-29 (owner ruling): `PrintingTagVote` used to sit here - a vote keyed +# (CanonicalCard, Tag, anonymous_id) asserting "this descriptor tag applies to this Scryfall +# printing". It is gone, model and table (migration 0101). It held 0 rows in production, 0 of +# them human, had no consensus resolver anywhere in the codebase, no reader outside the Django +# admin, and no frontend caller; its one machine writer (`import_external_ip_tags`) never ran. +# The full evidence is PR #599 (report: `2026-07-29-printing-vs-illustration-tag-grain.md`, which +# lands under docs/reports/ when that PR merges; it was still open when this was written); +# the replacement is `CanonicalPrintingMetadata.promo_types` for imported Scryfall facts (an +# imported fact is not a disputable claim, so it is not a vote) plus `CardTagVote` at card grain +# for anything a human genuinely disputes. +# +# THE TWO MODELS WITH CONFUSINGLY SIMILAR NAMES ARE BOTH LOAD-BEARING AND STILL HERE: +# * `CardPrintingTag` (above) - (Card, CanonicalCard, anonymous_id), "this catalogue IMAGE +# depicts this Scryfall PRINTING". 167k rows, read by `printing_consensus` into +# `Card.printing_tag_status` / `inferred_canonical_card` / Elasticsearch. +# * `CardTagVote` (above) - (Card, Tag, anonymous_id), "this descriptor TAG applies to this +# catalogue IMAGE". 224k rows, read by `tag_consensus` into `Card.tags`. +# Likewise `PRINTING_TAG_MIN_VOTES` / `PRINTING_TAG_IMPLICIT_CAP` / `PRINTING_TAG_MACHINE_WEIGHT` +# and `local_calculate_verdicts._split_new_printing_tag_votes` never governed the retired model - +# the first three are the app-wide consensus weights and the fourth is a `CardPrintingTag` +# collision guard. Do not "finish the job" by touching any of them. class CardReportReason(models.TextChoices): diff --git a/MPCAutofill/cardpicker/reason_tags.py b/MPCAutofill/cardpicker/reason_tags.py index 3f5c939ee..27cd64607 100644 --- a/MPCAutofill/cardpicker/reason_tags.py +++ b/MPCAutofill/cardpicker/reason_tags.py @@ -53,6 +53,14 @@ # Axis inline below (see the module docstring's "WTC phase B" section for the full # rationale) - not a schema change, just a reading aid: which of the two questions # (not-official-printing vs. not-official-art) each row answers. +# Our own `Tag.name` for the external-IP no-match reason, exported as a named constant because it +# is a convergence contract, not just a row in the list below: any machine channel that ever +# derives external-IP-ness from Scryfall data must write THIS string. It previously lived in +# `management/commands/import_external_ip_tags.py` (the Scryfall Tagger import), which was retired +# on 2026-07-29 along with `PrintingTagVote`; the constant outlived it deliberately so the contract +# survives the code that used to honour it. `test_reason_tags` pins it against the list below. +EXTERNAL_IP_TAG_NAME = "external-ip" + NO_MATCH_REASON_TAGS: list[tuple[str, str, str]] = [ # axis: not-official-art ("custom-art", "Original or alternate artwork - does not depict a real printing", "Custom art"), @@ -68,11 +76,14 @@ ("non-english", "Non-English printing", "Non-English"), # axis: not-official-art (see the dedicated comment on this tag below for its own, # unrelated "why this exact string" rationale) - # Deliberately the SAME string as EXTERNAL_IP_TAG_NAME in - # management/commands/import_external_ip_tags.py (2026-07-28) - both channels (this human - # no-match reason and that machine Scryfall-Tagger import) write into the same card.tags - # array, so one shared name makes `tag:external-ip` a single predicate over the whole - # catalog rather than two names that would permanently fragment it. Deliberately NOT named + # EXTERNAL_IP_TAG_NAME (defined above) used to live in + # management/commands/import_external_ip_tags.py, which owned the machine half of this tag; + # that command and its `PrintingTagVote` target were retired on 2026-07-29 and the constant + # moved here, to the module that owns the surviving (human) channel. The convergence rule it + # encodes is unchanged and still binding on whatever rebuilds the machine half: both channels + # write into the same card.tags array, so one shared name makes `tag:external-ip` a single + # predicate over the whole catalog rather than two names that would permanently fragment it. + # Deliberately NOT named # after the official Wizards "Universes Beyond" product line: that name covers OFFICIAL # Magic printings, so a custom proxy bearing e.g. Warhammer or Lord of the Rings art isn't # one of those - it's non-official art drawn from an external IP. There is no @@ -81,7 +92,7 @@ # resolve authoritatively from `set_type`/`security_stamp` at the PRINTING level, not as a # human-cast no-match reason. ( - "external-ip", + EXTERNAL_IP_TAG_NAME, "Art is drawn from an external IP (crossover / licensed property) rather than original Magic art", "External IP", ), diff --git a/MPCAutofill/cardpicker/tests/test_import_external_ip_tags.py b/MPCAutofill/cardpicker/tests/test_import_external_ip_tags.py deleted file mode 100644 index b3293194f..000000000 --- a/MPCAutofill/cardpicker/tests/test_import_external_ip_tags.py +++ /dev/null @@ -1,445 +0,0 @@ -""" -Tests for cardpicker.management.commands.import_external_ip_tags — the Scryfall Tagger -art:external-ip community tag import (fix-batch plan 2026-07-27, work item W9, -revised per-printing spec). - -Covers: - - JSONL parsing: find_external_ip_subtree (BFS from root tag slug), - collect_illustration_ids (second-pass over subtree only), - build_illustration_index (illustration_id → card id, including card_faces) - - run_external_ip_tag_import: dry-run/write, idempotent re-run, - illustration-to-CanonicalCard join, unmatched illustration/canonical-card skip, - dual-polarity voting (APPLY for positive Tagger matches, NOT_APPLICABLE for - confirmed printings outside the positive set, abstain when no default_cards data) - -Per-printing design: votes target CanonicalCard (Scryfall printing) rows directly — -no Card (catalog image) rows are needed or created. The illustration_id → -CanonicalCard.identifier join is the complete eligibility check. - -No network calls, no live DB writes — uses hand-built fixture JSONL files in -tests/fixtures/ and the Django test DB. -""" - -import uuid -from pathlib import Path - -import pytest - -from django.core.management import call_command - -from cardpicker.management.commands.import_external_ip_tags import ( - EXTERNAL_IP_TAG_NAME, - SCRYFALL_TAGGER_ANONYMOUS_ID, - ExternalIpImportResult, - build_illustration_index, - collect_illustration_ids, - find_external_ip_subtree, - run_external_ip_tag_import, -) -from cardpicker.models import ( - CanonicalCard, - PrintingTagVote, - Tag, - VotePolarity, - VoteSource, -) -from cardpicker.tests.factories import CanonicalCardFactory - -_FIXTURES = Path(__file__).resolve().parent / "fixtures" -_ART_TAGS_FIXTURE = _FIXTURES / "art_tags.jsonl" -_DEFAULT_CARDS_FIXTURE = _FIXTURES / "default_cards.jsonl" - -# CanonicalCard identifiers matching the default_cards fixture. -_RING_ID = uuid.UUID("11111111-8888-8888-8888-888888888888") -_GANDALF_ID = uuid.UUID("22222222-9999-9999-9999-999999999999") -_MARINE_ID = uuid.UUID("33333333-aaaa-aaaa-aaaa-aaaaaaaaaaaa") -_UNMATCHED_PRINTING_ID = uuid.UUID("44444444-bbbb-bbbb-bbbb-bbbbbbbbbbbb") -_DFC_ID = uuid.UUID("55555555-dddd-dddd-dddd-dddddddddddd") - - -# --------------------------------------------------------------------------- -# Parsing unit tests (only the module-level functions, no DB) -# --------------------------------------------------------------------------- - - -class TestFindExternalIpSubtree: - def test_finds_root_by_slug_and_bfs_subtree(self): - subtree, tags_seen = find_external_ip_subtree(_ART_TAGS_FIXTURE) - - # root + lotr + the-one-ring + warhammer = 4 tags in subtree - assert len(subtree) == 4 - assert subtree == { - uuid.UUID("aaaaaaaa-1111-1111-1111-111111111111"), # external-ip - uuid.UUID("bbbbbbbb-2222-2222-2222-222222222222"), # lotr - uuid.UUID("dddddddd-4444-4444-4444-444444444444"), # the-one-ring - uuid.UUID("cccccccc-3333-3333-3333-333333333333"), # warhammer - } - assert tags_seen == 5 # all five rows parsed - - def test_nonexistent_slug_raises_runtime_error(self, tmp_path): - empty = tmp_path / "empty.jsonl" - empty.write_text('{"id":"00000000-0000-0000-0000-000000000000","slug":"other","child_ids":[],"taggings":[]}\n') - try: - find_external_ip_subtree(empty) - except RuntimeError as e: - assert "external-ip" in str(e) - else: - raise AssertionError("expected RuntimeError") - - -class TestCollectIllustrationIds: - def test_collects_only_from_subtree_tags(self): - subtree = { - uuid.UUID("aaaaaaaa-1111-1111-1111-111111111111"), # external-ip (no direct taggings) - uuid.UUID("bbbbbbbb-2222-2222-2222-222222222222"), # lotr (no direct taggings) - uuid.UUID("dddddddd-4444-4444-4444-444444444444"), # the-one-ring (2 taggings) - uuid.UUID("cccccccc-3333-3333-3333-333333333333"), # warhammer (1 tagging) - } - - result = collect_illustration_ids(_ART_TAGS_FIXTURE, subtree) - - assert len(result) == 3 - assert uuid.UUID("eeeee111-5555-5555-5555-555555555555") in result # from the-one-ring - assert uuid.UUID("fffff222-6666-6666-6666-666666666666") in result # from the-one-ring - assert uuid.UUID("99999aaa-7777-7777-7777-777777777777") in result # from warhammer - # unrelated tag not in subtree → not collected - assert uuid.UUID("00000000-0000-0000-0000-000000000000") not in result - - -class TestBuildIllustrationIndex: - def test_maps_illustration_ids_to_card_ids(self): - index = build_illustration_index(_DEFAULT_CARDS_FIXTURE) - - # single-faced cards - assert index[uuid.UUID("eeeee111-5555-5555-5555-555555555555")] == {_RING_ID} - assert index[uuid.UUID("fffff222-6666-6666-6666-666666666666")] == {_GANDALF_ID} - assert index[uuid.UUID("99999aaa-7777-7777-7777-777777777777")] == {_MARINE_ID} - # unmatched illustration - assert index[uuid.UUID("55555555-cccc-cccc-cccc-cccccccccccc")] == {_UNMATCHED_PRINTING_ID} - # DFC: one entry per face illustration_id - assert index[uuid.UUID("66666666-eeee-eeee-eeee-eeeeeeeeeeee")] == {_DFC_ID} - assert index[uuid.UUID("77777777-ffff-ffff-ffff-ffffffffffff")] == {_DFC_ID} - - def test_empty_file_returns_empty_index(self, tmp_path): - empty = tmp_path / "empty.jsonl" - empty.write_text("") - assert build_illustration_index(empty) == {} - - -# --------------------------------------------------------------------------- -# Integration tests (DB-backed, via the real run_external_ip_tag_import) -# --------------------------------------------------------------------------- - - -def _make_canonical(identifier: uuid.UUID) -> CanonicalCard: - return CanonicalCardFactory(identifier=identifier, is_default=True) - - -class TestRunExternalIpTagImport: - def test_dry_run_counts_without_writing(self, db): - _make_canonical(_RING_ID) - _make_canonical(_GANDALF_ID) - _make_canonical(_MARINE_ID) - - result = run_external_ip_tag_import( - tags_path=_ART_TAGS_FIXTURE, - default_cards_path=_DEFAULT_CARDS_FIXTURE, - dry_run=True, - ) - - assert isinstance(result, ExternalIpImportResult) - assert result.dry_run is True - assert result.tags_seen == 5 - assert result.subtree_tag_count == 4 - assert result.illustrations_tagged == 3 - assert result.canonical_cards_matched == 3 - assert result.printings_eligible == 3 - assert result.votes_would_cast == 3 - assert PrintingTagVote.objects.count() == 0 # nothing persisted - - def test_write_casts_votes_and_persists(self, db): - ring = _make_canonical(_RING_ID) - gandalf = _make_canonical(_GANDALF_ID) - marine = _make_canonical(_MARINE_ID) - - result = run_external_ip_tag_import( - tags_path=_ART_TAGS_FIXTURE, - default_cards_path=_DEFAULT_CARDS_FIXTURE, - dry_run=False, - ) - - assert result.votes_written == 3 - assert PrintingTagVote.objects.count() == 3 - - for printing in [ring, gandalf, marine]: - vote = PrintingTagVote.objects.get(printing=printing) - assert vote.tag.name == "external-ip" - assert vote.polarity == VotePolarity.APPLY - assert vote.anonymous_id == SCRYFALL_TAGGER_ANONYMOUS_ID - assert vote.source == VoteSource.DEDUCTION - assert vote.run_id == result.run_id - - def test_idempotent_rerun_skips_already_voted_printings(self, db): - _make_canonical(_RING_ID) - _make_canonical(_GANDALF_ID) - - first = run_external_ip_tag_import( - tags_path=_ART_TAGS_FIXTURE, - default_cards_path=_DEFAULT_CARDS_FIXTURE, - dry_run=False, - ) - assert first.votes_written == 2 - - second = run_external_ip_tag_import( - tags_path=_ART_TAGS_FIXTURE, - default_cards_path=_DEFAULT_CARDS_FIXTURE, - dry_run=False, - ) - # both printings already voted by this identity → eligibility excludes them - assert second.printings_eligible == 0 - assert second.votes_written == 0 - assert PrintingTagVote.objects.count() == 2 # unchanged - - def test_skips_printing_without_canonical_card_match(self, db): - # Only one of the three tagged printings has a CanonicalCard row - _make_canonical(_RING_ID) - - result = run_external_ip_tag_import( - tags_path=_ART_TAGS_FIXTURE, - default_cards_path=_DEFAULT_CARDS_FIXTURE, - dry_run=True, - ) - - assert result.canonical_cards_matched == 1 - assert result.printings_eligible == 1 - assert result.skip_counts.get("printing-not-canonical") == 2 - - def test_source_and_anonymous_id(self, db): - """Each vote is cast as (source=DEDUCTION, anonymous_id=scryfall-tagger-v1) - so weight resolves to PRINTING_TAG_MACHINE_WEIGHT (0.5) through the - normal _SOURCE_WEIGHTS path, and the 2026-07-23 zero-weight override - scoped to deductive-backfill-v1 is never triggered.""" - _make_canonical(_RING_ID) - - run_external_ip_tag_import( - tags_path=_ART_TAGS_FIXTURE, - default_cards_path=_DEFAULT_CARDS_FIXTURE, - dry_run=False, - ) - - vote = PrintingTagVote.objects.get() - assert vote.source == VoteSource.DEDUCTION - assert vote.anonymous_id == "scryfall-tagger-v1" - - def test_audit_sample_contains_printing_fields(self, db): - """Audit sample entries use printing_id/printing_name (not card_id/card_name).""" - _make_canonical(_RING_ID) - - result = run_external_ip_tag_import( - tags_path=_ART_TAGS_FIXTURE, - default_cards_path=_DEFAULT_CARDS_FIXTURE, - dry_run=True, - ) - - assert len(result.audit) == 1 - entry = result.audit[0] - assert "printing_id" in entry - assert "printing_name" in entry - assert "card_id" not in entry - - def test_write_casts_both_polarities(self, db): - ring = _make_canonical(_RING_ID) - gandalf = _make_canonical(_GANDALF_ID) - marine = _make_canonical(_MARINE_ID) - unmatched = _make_canonical(_UNMATCHED_PRINTING_ID) - - result = run_external_ip_tag_import( - tags_path=_ART_TAGS_FIXTURE, - default_cards_path=_DEFAULT_CARDS_FIXTURE, - dry_run=False, - ) - - assert PrintingTagVote.objects.count() == 4 - assert PrintingTagVote.objects.filter(polarity=VotePolarity.APPLY).count() == 3 - for printing in [ring, gandalf, marine]: - vote = PrintingTagVote.objects.get(printing=printing) - assert vote.polarity == VotePolarity.APPLY - - negative_vote = PrintingTagVote.objects.get(polarity=VotePolarity.NOT_APPLICABLE) - assert negative_vote.printing == unmatched - assert negative_vote.tag.name == "external-ip" - assert negative_vote.anonymous_id == SCRYFALL_TAGGER_ANONYMOUS_ID - assert negative_vote.source == VoteSource.DEDUCTION - assert negative_vote.run_id == result.run_id - assert result.votes_written == 3 - assert result.negative_votes_written == 1 - - def test_negative_excludes_printings_not_in_default_cards(self, db): - # Absent from default_cards entirely = no data = abstain (no vote of either polarity) - unknown = _make_canonical(uuid.UUID("66666666-0000-0000-0000-000000000000")) - - run_external_ip_tag_import( - tags_path=_ART_TAGS_FIXTURE, - default_cards_path=_DEFAULT_CARDS_FIXTURE, - dry_run=False, - ) - - assert PrintingTagVote.objects.filter(printing=unknown).count() == 0 - assert PrintingTagVote.objects.count() == 0 - - def test_idempotent_rerun_skips_both_polarities(self, db): - _make_canonical(_RING_ID) - _make_canonical(_UNMATCHED_PRINTING_ID) - - first = run_external_ip_tag_import( - tags_path=_ART_TAGS_FIXTURE, - default_cards_path=_DEFAULT_CARDS_FIXTURE, - dry_run=False, - ) - assert first.votes_written == 1 - assert first.negative_votes_written == 1 - - second = run_external_ip_tag_import( - tags_path=_ART_TAGS_FIXTURE, - default_cards_path=_DEFAULT_CARDS_FIXTURE, - dry_run=False, - ) - assert second.printings_eligible == 0 - assert second.negative_printings_eligible == 0 - assert second.votes_written == 0 - assert second.negative_votes_written == 0 - assert PrintingTagVote.objects.count() == 2 # unchanged - - def test_dry_run_counts_both_polarities(self, db): - _make_canonical(_RING_ID) - _make_canonical(_UNMATCHED_PRINTING_ID) - - result = run_external_ip_tag_import( - tags_path=_ART_TAGS_FIXTURE, - default_cards_path=_DEFAULT_CARDS_FIXTURE, - dry_run=True, - ) - - assert result.votes_would_cast == 1 - assert result.negative_votes_would_cast == 1 - assert PrintingTagVote.objects.count() == 0 # nothing persisted - assert len(result.negative_audit) == 1 - entry = result.negative_audit[0] - assert "printing_id" in entry - assert "printing_name" in entry - - -class TestPurgeWriteAtomicity: - """Cancel-safety at this command's two vote-write sites (2026-07-28, generalising PR #526's - fix for the Stage D calculators). Each purge is a DELETE and its insert a separate statement, - so an import killed between them left the affected PRINTINGS with their previous same-family - vote deleted and nothing written back. Both pairs now run inside one `transaction.atomic()`. - - These are the app's only `printing_id`-keyed purge sites, so they also pin - `vote_write.purge_and_write_votes`' `target_field` parameter against a real caller. - - The victim row carries `scryfall-tagger-v0`: same calculator family (so the purge targets it), - different anonymous_id (so the eligibility exclude still selects the printing and the run - genuinely reaches the write).""" - - @staticmethod - def _boom(*args, **kwargs): - raise RuntimeError("simulated mid-flight kill between DELETE and INSERT") - - def _stale_vote(self, printing, polarity): - tag, _ = Tag.objects.get_or_create(name=EXTERNAL_IP_TAG_NAME) - return PrintingTagVote.objects.create( - printing=printing, - tag=tag, - polarity=polarity, - anonymous_id="scryfall-tagger-v0", - source=VoteSource.DEDUCTION, - ) - - def test_positive_pass_insert_failure_rolls_its_purge_back(self, db, monkeypatch): - ring = _make_canonical(_RING_ID) - stale = self._stale_vote(ring, VotePolarity.APPLY) - - monkeypatch.setattr(PrintingTagVote.objects, "bulk_create", self._boom) - - with pytest.raises(RuntimeError): - run_external_ip_tag_import( - tags_path=_ART_TAGS_FIXTURE, - default_cards_path=_DEFAULT_CARDS_FIXTURE, - dry_run=False, - ) - - assert PrintingTagVote.objects.filter(pk=stale.pk).exists() - - def test_negative_pass_insert_failure_rolls_its_purge_back(self, db, monkeypatch): - # only a printing OUTSIDE the positive Tagger set, so the positive pass writes nothing and - # the failure necessarily lands on the NOT_APPLICABLE write. - unmatched = _make_canonical(_UNMATCHED_PRINTING_ID) - stale = self._stale_vote(unmatched, VotePolarity.NOT_APPLICABLE) - - monkeypatch.setattr(PrintingTagVote.objects, "bulk_create", self._boom) - - with pytest.raises(RuntimeError): - run_external_ip_tag_import( - tags_path=_ART_TAGS_FIXTURE, - default_cards_path=_DEFAULT_CARDS_FIXTURE, - dry_run=False, - ) - - assert PrintingTagVote.objects.filter(pk=stale.pk).exists() - - -class TestManagementCommand: - """Exercise the Django management-command entry point (thin wrapper around - run_external_ip_tag_import) via call_command.""" - - def test_dry_run_default_without_write_flag(self, db): - _make_canonical(_RING_ID) - - call_command( - "import_external_ip_tags", - file=str(_ART_TAGS_FIXTURE), - default_cards=str(_DEFAULT_CARDS_FIXTURE), - ) - - assert PrintingTagVote.objects.count() == 0 # default is dry-run - - def test_write_flag_persists(self, db): - _make_canonical(_RING_ID) - - call_command( - "import_external_ip_tags", - file=str(_ART_TAGS_FIXTURE), - default_cards=str(_DEFAULT_CARDS_FIXTURE), - write=True, - ) - - assert PrintingTagVote.objects.count() == 1 - - def test_dry_run_explicit(self, db): - _make_canonical(_RING_ID) - - call_command( - "import_external_ip_tags", - file=str(_ART_TAGS_FIXTURE), - default_cards=str(_DEFAULT_CARDS_FIXTURE), - dry_run=True, - ) - - assert PrintingTagVote.objects.count() == 0 - - def test_mutually_exclusive_flags_raises(self, db): - from django.core.management import CommandError - - try: - call_command( - "import_external_ip_tags", - file=str(_ART_TAGS_FIXTURE), - default_cards=str(_DEFAULT_CARDS_FIXTURE), - write=True, - dry_run=True, - ) - except CommandError as e: - assert "only one" in str(e).lower() - else: - raise AssertionError("expected CommandError for mutually exclusive flags") diff --git a/MPCAutofill/cardpicker/tests/test_purge_machine_votes.py b/MPCAutofill/cardpicker/tests/test_purge_machine_votes.py index 05246a43c..f6682a808 100644 --- a/MPCAutofill/cardpicker/tests/test_purge_machine_votes.py +++ b/MPCAutofill/cardpicker/tests/test_purge_machine_votes.py @@ -26,8 +26,6 @@ CardTypes, PilotRunLedger, PrintingTagStatus, - PrintingTagVote, - VotePolarity, VoteSource, calculator_family, ) @@ -40,7 +38,6 @@ CardFactory, CardPrintingTagFactory, CardTagVoteFactory, - TagFactory, ) @@ -299,20 +296,15 @@ def test_reaches_null_run_id_rows_that_the_run_id_mode_provably_cannot(self, db) assert purge_by_anonymous_id(self.CALC).printing_votes_deleted == 1 assert not CardPrintingTag.objects.filter(anonymous_id=self.CALC).exists() - def test_covers_all_four_vote_tables(self, db): + def test_covers_every_vote_table(self, db): + """Three vote tables since 2026-07-29, not four: `PrintingTagVote` was retired (migration + 0101) with 0 rows, and `PurgeResult.printing_tag_votes_deleted` went with it. If a fourth + vote model is ever added, it belongs in this assertion the same day it is added.""" printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa")) card = CardFactory(name="Forest") CardPrintingTagFactory(card=card, printing=printing, source=VoteSource.OCR, anonymous_id=self.CALC, run_id=None) CardArtistVoteFactory(card=card, source=VoteSource.OCR, anonymous_id=self.CALC, run_id="run-A") CardTagVoteFactory(card=card, source=VoteSource.OCR, anonymous_id=self.CALC, run_id="run-B") - PrintingTagVote.objects.create( - printing=printing, - tag=TagFactory(), - polarity=VotePolarity.APPLY, - source=VoteSource.OCR, - anonymous_id=self.CALC, - run_id=None, - ) result = purge_by_anonymous_id(self.CALC) @@ -320,9 +312,7 @@ def test_covers_all_four_vote_tables(self, db): result.printing_votes_deleted, result.artist_votes_deleted, result.tag_votes_deleted, - result.printing_tag_votes_deleted, - ) == (1, 1, 1, 1) - assert not PrintingTagVote.objects.filter(anonymous_id=self.CALC).exists() + ) == (1, 1, 1) def test_other_calculators_and_other_versions_are_untouched(self, db): """One EXACT id per invocation - not a family, not a prefix. `-v2` of the same diff --git a/MPCAutofill/cardpicker/tests/test_reason_tags.py b/MPCAutofill/cardpicker/tests/test_reason_tags.py index 1be35e523..ef98ed273 100644 --- a/MPCAutofill/cardpicker/tests/test_reason_tags.py +++ b/MPCAutofill/cardpicker/tests/test_reason_tags.py @@ -1,7 +1,10 @@ from cardpicker.default_tags import DEFAULT_TAGS -from cardpicker.management.commands.import_external_ip_tags import EXTERNAL_IP_TAG_NAME from cardpicker.models import Tag -from cardpicker.reason_tags import NO_MATCH_REASON_TAGS, seed_no_match_reason_tags +from cardpicker.reason_tags import ( + EXTERNAL_IP_TAG_NAME, + NO_MATCH_REASON_TAGS, + seed_no_match_reason_tags, +) class TestSeedNoMatchReasonTags: @@ -23,19 +26,21 @@ def test_display_name_set_at_creation(self, db): def test_external_ip_tag_seeded_with_expected_display_name(self, db): # Replaces a never-committed pair of Universes-product-line-named tags (2026-07-28) - - # deliberately the same name as EXTERNAL_IP_TAG_NAME in - # management/commands/import_external_ip_tags.py, so this pins both the name and its - # seeded display_name directly rather than relying only on the generic + # deliberately the same name as `reason_tags.EXTERNAL_IP_TAG_NAME`, so this pins both the + # name and its seeded display_name directly rather than relying only on the generic # test_display_name_set_at_creation loop above. seed_no_match_reason_tags() tag = Tag.objects.get(name="external-ip") assert tag.display_name == "External IP" def test_external_ip_name_matches_the_machine_importer(self, db): - # Both channels (this human no-match reason and import_external_ip_tags' machine - # Scryfall-Tagger import) must converge on the exact same Tag.name so `tag:external-ip` - # is a single predicate over the whole catalog - pinned directly so the two modules - # can't silently drift apart. + # The convergence contract, kept alive across the retirement of the channel that + # motivated it: `import_external_ip_tags` (the machine Scryfall-Tagger import) and its + # `PrintingTagVote` target were removed on 2026-07-29, so `EXTERNAL_IP_TAG_NAME` now + # lives in `reason_tags` beside the surviving human channel. Whatever rebuilds the + # machine half - see docs/features/printing-tags.md's retirement record - must write + # this exact string, so `tag:external-ip` stays ONE predicate over the whole catalog + # instead of two names that would permanently fragment it. reason_tag_names = {name for name, _description, _display_name in NO_MATCH_REASON_TAGS} assert EXTERNAL_IP_TAG_NAME in reason_tag_names diff --git a/MPCAutofill/cardpicker/tests/test_scryfall_bulk_data.py b/MPCAutofill/cardpicker/tests/test_scryfall_bulk_data.py index 9c6a0e31b..d46d98d60 100644 --- a/MPCAutofill/cardpicker/tests/test_scryfall_bulk_data.py +++ b/MPCAutofill/cardpicker/tests/test_scryfall_bulk_data.py @@ -510,10 +510,13 @@ def test_no_importer_declares_its_own_bulk_data_model_any_more(): """ root = Path(__file__).resolve().parents[1] offenders = [] + # `management/commands/import_external_ip_tags.py` was the third importer this tether + # covered; it was retired on 2026-07-29 with `PrintingTagVote` and is dropped from the list + # rather than left behind to `FileNotFoundError`. The assertion's point is unchanged - it + # fails if ANY importer re-declares a bulk-data field outside the shared module. for relative in ( "printing_metadata_import.py", "integrations/game/mtg.py", - "management/commands/import_external_ip_tags.py", ): source = (root / relative).read_text() for line in source.splitlines(): diff --git a/MPCAutofill/cardpicker/tests/test_submit_printing_tag_vote.py b/MPCAutofill/cardpicker/tests/test_submit_printing_tag_vote.py deleted file mode 100644 index 0b27b23e3..000000000 --- a/MPCAutofill/cardpicker/tests/test_submit_printing_tag_vote.py +++ /dev/null @@ -1,184 +0,0 @@ -""" -Tests for POST 2/submitPrintingTagVote/ — cast/update/retract a user vote on whether -a descriptor Tag applies to a CanonicalCard (Scryfall printing). Commit 3 of PR #497. -""" - -import json -import uuid - -import pytest - -from django.test import Client - -from cardpicker.models import PrintingTagVote, VotePolarity, VoteSource -from cardpicker.tests.factories import CanonicalCardFactory, TagFactory - -_URL = "/2/submitPrintingTagVote/" -_ANON_ID = "test-anon-id-abc123" - - -def _post(client: Client, payload: dict) -> object: - return client.post( - _URL, - data=json.dumps(payload), - content_type="application/json", - HTTP_ORIGIN="http://localhost", - ) - - -@pytest.fixture() -def printing(db): - return CanonicalCardFactory(identifier=uuid.uuid4(), is_default=True) - - -@pytest.fixture() -def tag(db): - return TagFactory(name="external-ip") - - -class TestSubmitPrintingTagVote: - def test_apply_creates_vote(self, client, printing, tag): - resp = _post( - client, - { - "anonymousId": _ANON_ID, - "printingIdentifier": str(printing.identifier), - "tagName": tag.name, - "polarity": VotePolarity.APPLY, - }, - ) - assert resp.status_code == 200 - vote = PrintingTagVote.objects.get(printing=printing, tag=tag, anonymous_id=_ANON_ID) - assert vote.polarity == VotePolarity.APPLY - assert vote.source == VoteSource.USER - - def test_not_applicable_creates_vote(self, client, printing, tag): - resp = _post( - client, - { - "anonymousId": _ANON_ID, - "printingIdentifier": str(printing.identifier), - "tagName": tag.name, - "polarity": VotePolarity.NOT_APPLICABLE, - }, - ) - assert resp.status_code == 200 - vote = PrintingTagVote.objects.get(printing=printing, tag=tag, anonymous_id=_ANON_ID) - assert vote.polarity == VotePolarity.NOT_APPLICABLE - - def test_update_or_create_changes_existing_vote(self, client, printing, tag): - _post( - client, - { - "anonymousId": _ANON_ID, - "printingIdentifier": str(printing.identifier), - "tagName": tag.name, - "polarity": VotePolarity.APPLY, - }, - ) - _post( - client, - { - "anonymousId": _ANON_ID, - "printingIdentifier": str(printing.identifier), - "tagName": tag.name, - "polarity": VotePolarity.NOT_APPLICABLE, - }, - ) - # still one row, polarity flipped - assert PrintingTagVote.objects.filter(printing=printing, tag=tag, anonymous_id=_ANON_ID).count() == 1 - assert ( - PrintingTagVote.objects.get(printing=printing, tag=tag, anonymous_id=_ANON_ID).polarity - == VotePolarity.NOT_APPLICABLE - ) - - def test_retract_deletes_existing_vote(self, client, printing, tag): - _post( - client, - { - "anonymousId": _ANON_ID, - "printingIdentifier": str(printing.identifier), - "tagName": tag.name, - "polarity": VotePolarity.APPLY, - }, - ) - assert PrintingTagVote.objects.filter(printing=printing, tag=tag).exists() - - resp = _post( - client, - { - "anonymousId": _ANON_ID, - "printingIdentifier": str(printing.identifier), - "tagName": tag.name, - "polarity": 0, # RETRACT - }, - ) - assert resp.status_code == 200 - assert not PrintingTagVote.objects.filter(printing=printing, tag=tag).exists() - - def test_retract_with_no_existing_vote_is_noop(self, client, printing, tag): - resp = _post( - client, - { - "anonymousId": _ANON_ID, - "printingIdentifier": str(printing.identifier), - "tagName": tag.name, - "polarity": 0, - }, - ) - assert resp.status_code == 200 - assert PrintingTagVote.objects.count() == 0 - - def test_unknown_printing_returns_400(self, client, tag, db): - resp = _post( - client, - { - "anonymousId": _ANON_ID, - "printingIdentifier": str(uuid.uuid4()), - "tagName": tag.name, - "polarity": VotePolarity.APPLY, - }, - ) - assert resp.status_code == 400 - - def test_unknown_tag_returns_400(self, client, printing): - resp = _post( - client, - { - "anonymousId": _ANON_ID, - "printingIdentifier": str(printing.identifier), - "tagName": "nonexistent-tag", - "polarity": VotePolarity.APPLY, - }, - ) - assert resp.status_code == 400 - - def test_invalid_polarity_returns_400(self, client, printing, tag): - resp = _post( - client, - { - "anonymousId": _ANON_ID, - "printingIdentifier": str(printing.identifier), - "tagName": tag.name, - "polarity": 99, - }, - ) - assert resp.status_code == 400 - - def test_get_method_returns_400(self, client, db): - resp = client.get(_URL, HTTP_ORIGIN="http://localhost") - assert resp.status_code == 400 - - def test_vote_surface_is_persisted(self, client, printing, tag): - _post( - client, - { - "anonymousId": _ANON_ID, - "printingIdentifier": str(printing.identifier), - "tagName": tag.name, - "polarity": VotePolarity.APPLY, - "voteSurface": "printing-tag-picker", - }, - ) - vote = PrintingTagVote.objects.get(printing=printing, tag=tag, anonymous_id=_ANON_ID) - assert vote.vote_surface == "printing-tag-picker" diff --git a/MPCAutofill/cardpicker/tests/test_vote_consensus.py b/MPCAutofill/cardpicker/tests/test_vote_consensus.py index ff4fcfdf0..7c12532ea 100644 --- a/MPCAutofill/cardpicker/tests/test_vote_consensus.py +++ b/MPCAutofill/cardpicker/tests/test_vote_consensus.py @@ -94,9 +94,12 @@ def test_ordinary_deduction_vote_keeps_its_normal_weight(self): def test_another_calculator_carrying_the_frozen_run_id_is_unaffected(self): # defensive: the run stamp alone is never enough - all three conjuncts must hold, so a - # stray re-stamp onto some other calculator's row cannot pull it into a ratified ruling + # stray re-stamp onto some other calculator's row cannot pull it into a ratified ruling. + # The example identity was "scryfall-tagger-v1" until 2026-07-29; that identity retired + # with `PrintingTagVote`, so a LIVE calculator stands in - the assertion is about the + # conjunction, but an example nobody can look up is a worse example. assert ( - resolve_vote_weight(VoteSource.DEDUCTION, "scryfall-tagger-v1", DEDUCTIVE_BACKFILL_ZERO_WEIGHT_RUN_ID) + resolve_vote_weight(VoteSource.DEDUCTION, "stage-d-illustration-v1", DEDUCTIVE_BACKFILL_ZERO_WEIGHT_RUN_ID) == _SOURCE_WEIGHTS[VoteSource.DEDUCTION] ) diff --git a/MPCAutofill/cardpicker/tests/test_vote_write.py b/MPCAutofill/cardpicker/tests/test_vote_write.py index 0849c7713..09382a45d 100644 --- a/MPCAutofill/cardpicker/tests/test_vote_write.py +++ b/MPCAutofill/cardpicker/tests/test_vote_write.py @@ -7,7 +7,11 @@ pair is ATOMIC (this project's operator kills long runs mid-flight, and an untransacted DELETE-then-INSERT loses votes outright when killed between the two), the purge is SCOPED TO THE ROWS ACTUALLY WRITTEN (so a target the caller's already-voted split skipped keeps the winner's -committed row), and it is correct across all four vote models / both target fields the app uses. +committed row), and it is correct across every vote model / both target fields the primitive +supports. The `printing_id` target field had exactly one production caller, the Scryfall-Tagger +`import_external_ip_tags` command, retired 2026-07-29 along with `PrintingTagVote`; the two tests +that covered it are re-pointed at `CardPrintingTag.printing_id` rather than deleted, so +`target_field` stays a tested parameter instead of quietly becoming a `card_id` constant. `_purge_and_write_printing_tag_votes`' own tests in `test_local_calculate_verdicts.py` are kept as they were - they now exercise this function through that binding. @@ -20,7 +24,6 @@ CardArtistVote, CardPrintingTag, CardTagVote, - PrintingTagVote, VotePolarity, VoteSource, ) @@ -303,64 +306,72 @@ def test_card_artist_vote_rolls_back(self, db, monkeypatch): assert CardArtistVote.objects.filter(pk=stale.pk).exists() - def test_printing_tag_vote_keys_on_printing_id_not_card_id(self, db, monkeypatch): - """The only `printing_id`-keyed call site in the app (`import_external_ip_tags`) - the - target field is a real parameter, not a card_id assumption baked into the primitive.""" + def test_target_field_is_a_real_parameter_not_a_card_id_assumption(self, db): + """`target_field="printing_id"` genuinely re-keys the purge. Proven the sharp way: the + stale row belongs to a DIFFERENT card and the same printing, so it can only be reached by + a purge scoped on `printing_id` - a `card_id`-keyed purge would leave it alone. + + This had a production caller until 2026-07-29 (`import_external_ip_tags`, keyed on + `PrintingTagVote.printing_id`); both were retired with the model. `CardPrintingTag` also + carries a `printing_id` column, so the parameter stays exercised against a live model + rather than becoming untested machinery the day its only user left.""" printing = CanonicalCardFactory(name="Some Card") - tag = TagFactory(name="External IP") - stale = PrintingTagVote.objects.create( + stale_card = CardFactory(name="Some Card (other scan)") + fresh_card = CardFactory(name="Some Card") + stale = CardPrintingTag.objects.create( + card=stale_card, printing=printing, - tag=tag, - polarity=VotePolarity.APPLY, - anonymous_id="scryfall-tagger-v0", - source=VoteSource.DEDUCTION, + is_no_match=False, + anonymous_id="local-ocr-v0", + source=VoteSource.OCR, ) purge_and_write_votes( - PrintingTagVote, + CardPrintingTag, [ - PrintingTagVote( + CardPrintingTag( + card_id=fresh_card.pk, printing_id=printing.pk, - tag_id=tag.pk, - polarity=VotePolarity.APPLY, - anonymous_id="scryfall-tagger-v1", + is_no_match=False, + anonymous_id="local-ocr-v1", ) ], - anonymous_id="scryfall-tagger-v1", + anonymous_id="local-ocr-v1", target_field="printing_id", ignore_conflicts=True, ) - assert not PrintingTagVote.objects.filter(pk=stale.pk).exists() - assert PrintingTagVote.objects.filter(printing=printing, anonymous_id="scryfall-tagger-v1").count() == 1 + assert not CardPrintingTag.objects.filter(pk=stale.pk).exists() + assert CardPrintingTag.objects.filter(card=fresh_card, anonymous_id="local-ocr-v1").count() == 1 - def test_printing_tag_vote_rolls_back(self, db, monkeypatch): + def test_printing_id_keyed_write_rolls_back(self, db, monkeypatch): printing = CanonicalCardFactory(name="Some Card") - tag = TagFactory(name="External IP") - stale = PrintingTagVote.objects.create( + stale_card = CardFactory(name="Some Card (other scan)") + fresh_card = CardFactory(name="Some Card") + stale = CardPrintingTag.objects.create( + card=stale_card, printing=printing, - tag=tag, - polarity=VotePolarity.APPLY, - anonymous_id="scryfall-tagger-v0", - source=VoteSource.DEDUCTION, + is_no_match=False, + anonymous_id="local-ocr-v0", + source=VoteSource.OCR, ) - monkeypatch.setattr(PrintingTagVote.objects, "bulk_create", _raise_instead_of_inserting) + monkeypatch.setattr(CardPrintingTag.objects, "bulk_create", _raise_instead_of_inserting) with pytest.raises(RuntimeError): purge_and_write_votes( - PrintingTagVote, + CardPrintingTag, [ - PrintingTagVote( + CardPrintingTag( + card_id=fresh_card.pk, printing_id=printing.pk, - tag_id=tag.pk, - polarity=VotePolarity.APPLY, - anonymous_id="scryfall-tagger-v1", + is_no_match=False, + anonymous_id="local-ocr-v1", ) ], - anonymous_id="scryfall-tagger-v1", + anonymous_id="local-ocr-v1", target_field="printing_id", ignore_conflicts=True, ) - assert PrintingTagVote.objects.filter(pk=stale.pk).exists() + assert CardPrintingTag.objects.filter(pk=stale.pk).exists() diff --git a/MPCAutofill/cardpicker/urls.py b/MPCAutofill/cardpicker/urls.py index 4a5722026..abe16cde6 100755 --- a/MPCAutofill/cardpicker/urls.py +++ b/MPCAutofill/cardpicker/urls.py @@ -34,7 +34,6 @@ path("2/submitArtistWriteInVote/", views.post_submit_artist_writein_vote), path("2/tagConsensus/", views.post_tag_consensus), path("2/submitTagVote/", views.post_submit_tag_vote), - path("2/submitPrintingTagVote/", views.post_submit_printing_tag_vote), path("2/castImplicitVote/", views.post_cast_implicit_vote), path("2/retractImplicitVote/", views.post_retract_implicit_vote), path("2/voteQueue/", views.post_vote_queue), diff --git a/MPCAutofill/cardpicker/views.py b/MPCAutofill/cardpicker/views.py index 9c34effc0..a2282d419 100644 --- a/MPCAutofill/cardpicker/views.py +++ b/MPCAutofill/cardpicker/views.py @@ -16,7 +16,7 @@ import pycountry from django_ratelimit.decorators import ratelimit from elasticsearch_dsl.index import Index -from pydantic import BaseModel, ValidationError +from pydantic import ValidationError from django.conf import settings from django.contrib.auth.models import User @@ -79,7 +79,6 @@ CardTypes, DFCPair, PrintingTagStatus, - PrintingTagVote, SavedDeck, SavedDeckKind, SavedDeckShare, @@ -2972,94 +2971,4 @@ def get_funnel_counts(request: HttpRequest) -> HttpResponse: return JsonResponse(result) -class _SubmitPrintingTagVoteRequest(BaseModel): - """ - Request body for POST 2/submitPrintingTagVote/ — cast or update a user vote on whether - a descriptor tag applies to a CanonicalCard (Scryfall printing). - - Mirrors SubmitTagVoteRequest's shape but targets a printing (CanonicalCard.identifier) - rather than a Card image. The `polarity` values are the same: 1=APPLY, -1=NOT_APPLICABLE, - 0=retract (deletes the existing vote from this anonymous_id for this printing+tag pair). - """ - - anonymousId: str - printingIdentifier: str - tagName: str - polarity: int - voteSurface: Optional[str] = None - - -def _get_canonical_card_or_400(identifier: str) -> CanonicalCard: - try: - return CanonicalCard.objects.get(identifier=identifier) - except (CanonicalCard.DoesNotExist, ValueError): - raise BadRequestException(f"No canonical card (printing) found with identifier {identifier!r}.") - - -@csrf_exempt -@reject_untrusted_origin -@ratelimit( # type: ignore - key=_printing_tag_rate_limit_key, rate=_printing_tag_rate_limit_rate, method="POST", block=False -) -@ErrorWrappers.to_json -def post_submit_printing_tag_vote(request: HttpRequest) -> HttpResponse: - """ - Cast or update a vote on whether a descriptor Tag applies to a CanonicalCard (Scryfall - printing). Mirrors the CardTagVote submit idiom (post_submit_tag_vote / _cast_tag_vote_and_ - resolve) but targets PrintingTagVote — one vote per (printing, tag, anonymous_id) so a - voter can independently update their opinion via update_or_create without affecting any - other vote they've cast. - - Polarity 0 is a retract: deletes the existing vote from this identity for this - (printing, tag) pair, identical to post_submit_tag_vote's own RETRACT_POLARITY sentinel. - - Per-printing consensus resolution is intentionally not triggered here. Printing-level - tag resolution (external-ip, UW/UB) follows a primary-signal / fallback pattern: - Scryfall structured data (tagger art_tags, security_stamp) is the authoritative source - and resolves without a vote. User votes fill gaps where Scryfall data is ambiguous. - The resolution function will be added when a pipeline consumer needs it (card - serialisation / question feed expansion — see #437). Until then, votes land correctly - but no resolution is computed or surfaced. - """ - 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 = _SubmitPrintingTagVoteRequest.model_validate(json.loads(request.body)) - printing = _get_canonical_card_or_400(req.printingIdentifier) - 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, RETRACT_POLARITY): - raise BadRequestException( - f"Invalid polarity {req.polarity!r} - must be 1 (apply), -1 (not applicable), or 0 (retract)." - ) - - anonymous_id = req.anonymousId - with transaction.atomic(): - if req.polarity == RETRACT_POLARITY: - PrintingTagVote.objects.filter(printing=printing, tag=tag, anonymous_id=anonymous_id).delete() - else: - PrintingTagVote.objects.update_or_create( - printing=printing, - tag=tag, - anonymous_id=anonymous_id, - defaults={ - "polarity": req.polarity, - "source": VoteSource.USER, - "user": _requesting_user(request), - "vote_surface": req.voteSurface, - }, - ) - - return JsonResponse({"status": "ok"}) - - # endregion diff --git a/MPCAutofill/cardpicker/vote_write.py b/MPCAutofill/cardpicker/vote_write.py index 49b8bc4c1..d15974210 100644 --- a/MPCAutofill/cardpicker/vote_write.py +++ b/MPCAutofill/cardpicker/vote_write.py @@ -6,17 +6,23 @@ plus a cancel-safety hole at the three `CardPrintingTag`-casting Stage D call sites. The same two defects existed, unfixed, at every OTHER `purge_stale_machine_votes(...)` + `bulk_create(...)` pair wired up by #519/#520 - nine more sites across seven modules, casting four -different vote models (`CardPrintingTag`, `CardTagVote`, `CardArtistVote`, `PrintingTagVote`) and -keyed on two different target fields (`card_id`, `printing_id`). This module is #526's primitive -with those four axes parameterised; the semantics below are #526's, unchanged. +different vote models (`CardPrintingTag`, `CardTagVote`, `CardArtistVote`, and the since-retired +`PrintingTagVote`) and keyed on two different target fields (`card_id`, `printing_id`). This module +is #526's primitive with those four axes parameterised; the semantics below are #526's, unchanged. +`PrintingTagVote` was retired on 2026-07-29 (migration 0101) and took the app's only +`printing_id`-keyed call site with it, so every LIVE caller today passes the default +`target_field="card_id"`. The parameter stays because it is what makes this a primitive rather +than a `CardPrintingTag` helper, and `purge_stale_machine_votes` is keyed the same way. IT IS A SEPARATE MODULE, NOT A FUNCTION IN `local_calculate_verdicts`, BECAUSE OF AN IMPORT CYCLE: `local_calculate_verdicts` imports from `local_identify_printing_tags` (its `CandidateNameIndex`, `_eligible_base_queryset`, `generate_run_id`, ...), and `local_identify_printing_tags` is itself one of the call sites that needs this primitive - so it cannot import back. `local_lands_identify` -and `local_residual_classify` sit on that same import chain, and -`management/commands/import_external_ip_tags.py` has no business importing a Stage D calculator at -all just to write a vote batch. A leaf module whose only dependencies are `django.db.transaction` +and `local_residual_classify` sit on that same import chain, and the management commands that +write vote batches have no business importing a Stage D calculator at all just to do it (the +motivating case was `management/commands/import_external_ip_tags.py`, retired 2026-07-29 with +`PrintingTagVote`; the argument is unchanged for the ones that remain). A leaf module whose only +dependencies are `django.db.transaction` and `cardpicker.models` is importable from all of them. It is deliberately NOT in `models.py` alongside `purge_stale_machine_votes` itself: `models.py` is the schema, and this is write-path policy that composes a model-layer helper with `bulk_create`. @@ -79,10 +85,11 @@ def purge_and_write_votes( is that callers must pass POST-SPLIT rows, because `rows` is simultaneously the purge scope and the insert payload. - `model_class` is the vote model (`CardPrintingTag`, `CardTagVote`, `CardArtistVote`, - `PrintingTagVote`); `target_field` is the column `purge_stale_machine_votes` keys the purge on - and the attribute read off each row to build that scope (`card_id` for the per-card vote - models, `printing_id` for `PrintingTagVote`). + `model_class` is the vote model (`CardPrintingTag`, `CardTagVote`, `CardArtistVote`); + `target_field` is the column `purge_stale_machine_votes` keys the purge on and the attribute + read off each row to build that scope - `card_id` for every vote model that exists today. It + is still a parameter because the primitive is not `CardPrintingTag`-specific: the retired + `PrintingTagVote` passed `printing_id`, and any future non-card-keyed vote model would too. `anonymous_id` is the identity whose FAMILY is purged. Pass it explicitly when the caller purges under one fixed identity even though `rows` may carry others - `local_lands_identify` diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index 50928d7bd..effcd64a1 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -388,12 +388,16 @@ printings, artists, tags, and moderation from one screen. etc.), since one is cast at upload-time from filename parsing and the other is a human's queue-time judgment — kept exact-string-distinct so the two vote populations don't silently merge. `external-ip` (added - 2026-07-28, WTC artist question re-frame) is deliberately the same - string as `EXTERNAL_IP_TAG_NAME` in - `management/commands/import_external_ip_tags.py` — both the human - no-match reason and that machine Scryfall-Tagger import converge on one - `Tag.name` so `tag:external-ip` is a single predicate over the catalog. - Not named after the official "Universes Beyond" Wizards product line: + 2026-07-28, WTC artist question re-frame) is the string + `reason_tags.EXTERNAL_IP_TAG_NAME` — a convergence contract, not just a + row in the list: the human no-match reason and any machine channel that + ever derives external-IP-ness must write one `Tag.name`, so + `tag:external-ip` is a single predicate over the catalog rather than two + names that permanently fragment it. The constant lived in + `management/commands/import_external_ip_tags.py` until that command was + retired on 2026-07-29 (see the retirement record below) and moved to + `reason_tags.py` so the contract outlives the code that used to honour + it. Not named after the official "Universes Beyond" Wizards product line: that name covers OFFICIAL Magic printings, so a custom proxy bearing e.g. Warhammer or Lord of the Rings art isn't one of those — it's non-official art drawn from an external IP. @@ -457,67 +461,111 @@ printings, artists, tags, and moderation from one screen. check would be. D1's actual claim is the one it can support: the name matches exactly one row _in our catalogue_. -- **External-IP tag import (Scryfall Tagger)** (W9 per-printing design, - revised 2026-07-27): `manage.py import_external_ip_tags` imports - `art:external-ip` — a Scryfall Tagger community art tag identifying - Universes Beyond illustrations (Lord of the Rings, Doctor Who, Warhammer - 40K, etc.) — as machine-cast `PrintingTagVote` rows. Votes target the - Scryfall printing (`CanonicalCard`) directly, not the catalog images - (`Card`) that depict it. The same physical printing may be depicted by many - `Card` images in the catalog; the Tagger community tag belongs to the - printing once, not duplicated per image. - - Data flow: - - 1. Fetches `https://api.scryfall.com/bulk-data`, finds the `art_tags` - entry, and downloads its `jsonl_download_uri` (or reads a local file - via `--file`). - 2. Parses the tag tree: finds the tag with slug `external-ip`, then BFS - its `child_ids` to collect the full subtree (~56 child IP tags). - 3. Collects all `illustration_id` values from `taggings` across the - subtree (only leaf tags carry taggings per Scryfall's documentation). - 4. Joins `illustration_id` → `default_cards.json` `illustration_id` → - `CanonicalCard.identifier` **directly** (the same bulk data - `import_canonical_card_data`/`import_scryfall_printing_metadata` - already maintain). No Card-level effective-printing inference is done — - the illustration → printing join is the complete eligibility check. - 5. Writes `PrintingTagVote` rows: `tag="external-ip"`, `source=DEDUCTION`, - `anonymous_id="scryfall-tagger-v1"`, `polarity=APPLY`. - - **User vote endpoint**: `POST 2/submitPrintingTagVote/` mirrors the - `CardTagVote` submit idiom — `update_or_create` keyed on - `(printing, tag, anonymous_id)`, `source=USER`, polarity 1/−1/0 (0 = retract). - Frontend UI is out of scope for this commit; the backend is wired and tested. - - **Weighting**: votes carry `PRINTING_TAG_MACHINE_WEIGHT` (default 0.5) - through the normal `vote_consensus._SOURCE_WEIGHTS` path — no override - applies (the 2026-07-23 zero-weight rule is scoped to - `anonymous_id="deductive-backfill-v1"` only). `source=DEDUCTION` is used - rather than a bespoke `VoteSource` value because `VoteSource.source` is - limited to 10 characters; `anonymous_id` carries the identifiable provenance - for purge/re-run. - - **Idempotent re-run**: the `(printing, tag, anonymous_id)` uniqueness - constraint on `PrintingTagVote` means a printing this identity has already - voted on is skipped. To refresh against updated upstream data (where a - printing was un-tagged), purge the old `run_id` via - `manage.py purge_machine_votes --run-id ` then re-run. Every invocation - stamps a fresh `run_id` on its votes for independent lifecycle management. - `purge_machine_votes` deletes `PrintingTagVote` rows by `run_id` alongside - the other vote models. - - **Gate**: `verify_no_machine_only_resolutions` in `purge_machine_votes` - checks Card-level resolution status (printing, artist, tag). PrintingTagVote - writes do not affect any Card-level resolution status today — per-printing - consensus resolution is a separate concern with no persisted CanonicalCard - status field yet (see OPEN ITEMS in PR #497). The function therefore does not - apply to PrintingTagVote runs and is omitted from `import_external_ip_tags`'s - write path; it remains available for CardTagVote/CardPrintingTag/CardArtistVote. - - **Default dry-run**: like every other Stage 3+ command, `import_external_ip_tags` - defaults to dry-run (count-only) and requires an explicit `--write` flag - to persist votes. The `--file` argument accepts a local art-tags JSONL(.gz) - file (or pretty-printed JSON array) for offline runs and testing. +- **External-IP tag import (Scryfall Tagger) — RETIRED 2026-07-29, + together with `PrintingTagVote`.** Owner ruling: _"i am willing to not + need the printing tag. i am happy to reduce things to the minimum that + gives us our expected results."_ This entry is the durable record of + what was removed, why, and what any rebuild must carry — the code is + gone, so this is the only place the knowledge survives. Full evidence: + **PR #599**, whose report lands under `docs/reports/` as + `2026-07-29-printing-vs-illustration-tag-grain.md` when it merges (it was + still open when this was written, which is why the path is spelled out + rather than linked). + + **What was removed.** `PrintingTagVote` (model + table, migration + `0101_delete_printingtagvote`), `POST 2/submitPrintingTagVote/` and its + URL route, the Django admin registration, + `manage.py import_external_ip_tags` (579 lines) and its tests, and the + `PrintingTagVote` arm of `manage.py purge_machine_votes` (including + `PurgeResult.printing_tag_votes_deleted` — the purge report enumerates + three vote tables now, not four). The last commit carrying the deleted + files is `e6c6429a`; read them with + `git show e6c6429a:MPCAutofill/cardpicker/management/commands/import_external_ip_tags.py`. + **Not touched, despite the adjacent names**: `CardPrintingTag` + (167,229 rows, the entire Stage D printing channel) and `CardTagVote` + (223,999 rows, resolves into `Card.tags`). Nor + `PRINTING_TAG_MIN_VOTES` / `PRINTING_TAG_IMPLICIT_CAP` / + `PRINTING_TAG_MACHINE_WEIGHT`, which are the app-wide consensus weights, + nor `_split_new_printing_tag_votes`, which is a `CardPrintingTag` + collision guard. + + **Why.** Measured against production on 2026-07-29: **0 rows, 0 of them + human, 0 ever resolved.** There was **no consensus resolver** — the + `printing_tag_consensus.py` that two docstrings forward-referenced never + existed on any branch in the repo's history — **no reader** outside the + Django admin, and **no frontend caller** of the submit endpoint. The + importer, its only machine writer, never ran once (`PilotRunLedger` has + zero rows for it, and it writes its ledger row before branching on + dry-run, so even a dry run would have left a trace). Underneath all of + that sits the design principle that makes it unsalvageable rather than + merely unused: **an imported Scryfall fact is not a disputable claim, so + it must not be modelled as a vote.** `resolve_weighted_consensus`'s + `has_human_backed` gate is absolute and independent of the weight sum, so + a machine-only channel returns `None` at any volume (verified by + execution at n=1…1000). Automatic Scryfall-derived tagging could never + have displayed anything through the vote system, at any grain, at any + threshold. That gate is the invariant this project is built on, working + correctly; the mistake was routing an indisputable fact through it. + + **What serves the use case instead, today, with no new machinery.** + `CanonicalPrintingMetadata.promo_types` already carries Scryfall's + `universesbeyond` token on **10,407 of 113,224 printings**, at 100% + per-set recall on every dedicated UB set and correct partial behaviour on + mixed sets (`sld`, `clu`) — it has been ingested all along and nothing + read it for this. **3,450 catalogue images already resolve to a UB + printing** through the existing ingestion-time `Card.canonical_card` + link. For custom/proxy images — which have no Scryfall printing and no + `illustration_id` at all, so no Scryfall-derived tag can reach them by + construction — the channel is `CardTagVote` on the `external-ip` `Tag`, + which already resolves through `tag_consensus` into `Card.tags` and is + already Elasticsearch-indexed. Both converge on one `Tag.name` by design. + + **The algorithm, for whoever rebuilds it.** The reusable core was ~150 + lines and is worth restating rather than re-derived. Scryfall's Tagger + publishes an `art_tags` bulk entry (`scryfall_bulk_data.ART_TAGS`, still + defined and still tested against the live endpoint although nothing + consumes it today); its `jsonl_download_uri` is ~12 MB gzipped. + Pass 1 indexes every tag row's `id -> (slug, child_ids)`, finds the tag + whose slug is `external-ip`, and BFS-closes its `child_ids` **to a + fixpoint** — not the one level the original plan called for, because the + hierarchy can deepen; only leaf tags carry taggings, per Scryfall's own + documentation, and slugs are explicitly not permanent identifiers, so a + missing slug must fail LOUD rather than import zero rows. Pass 2 re-reads + the file collecting `taggings[].illustration_id` for tags in the subtree + (kept separate from pass 1 so the much larger taggings payload is never + held alongside the tag index). Pass 3 builds + `illustration_id -> {scryfall card id}` from the already-on-disk + `default_cards` bulk data — top-level `illustration_id` on single-faced + rows, one entry per face under `card_faces` for double-faced rows — and + joins straight onto `CanonicalCard.identifier`. Measured 2026-07-29 + against the live feed: the subtree closes over **2,799 tags / 8,332 + distinct illustrations**, reaching **~13,166** of our printings — about + **2,759 more than `promo_types`**, because `art:external-ip` marks the + _artwork's IP origin_ while `promo_types` marks Wizards' _product line_. + **What that ~2,759 consists of is not established**, and characterising a + sample of it is the cheapest thing that would decide whether the Tagger + dependency is worth carrying at all. + + **What a rebuild must NOT carry.** (1) Votes — store it as an imported + attribute, readable directly, with no threshold and no resolver. + (2) The negative pass: the removed command voted `NOT_APPLICABLE` on + every confirmed printing outside the positive set, so a `--write` run + would have inserted ~13k `APPLY` plus ~100k negative rows into a table + nothing read. An attribute expresses absence by being absent. (3) The + vote-system plumbing that went with it — `source=DEDUCTION`, + `anonymous_id="scryfall-tagger-v1"`, `run_id` stamping, + `purge_machine_votes` integration — which also correctly drops that + identity off the calculator roster, since it was never a calculator. + (4) `security_stamp` as a UB signal: issue #437's own Phase-1 research + established it is not deterministic (`stamp:triangle` 2,413 vs + `is:universesbeyond` 4,415; LotR, Avatar, Final Fantasy and Assassin's + Creed carry oval or null stamps). Grain note: **UB-ness travels with the + artwork** — of 50,828 distinct stored `illustration_id` values, **0** + appear on both a UB and a non-UB printing, despite 25,381 being reprinted + and 20,542 crossing set boundaries — so an illustration-keyed column is + the right shape, not a printing-keyed one, and not a vote either way. + Owner direction: fold this into the unified Scryfall importer rather than + resurrect a standalone command. - **Moderation layer**: builds on the same consensus system — see [[moderation.md]] for the sensitive-tag taxonomy, privileged-approval diff --git a/docs/pipeline-fidelity-gate.md b/docs/pipeline-fidelity-gate.md index e81914b52..e9c9fcd98 100644 --- a/docs/pipeline-fidelity-gate.md +++ b/docs/pipeline-fidelity-gate.md @@ -1274,38 +1274,49 @@ whole subtree with it as a side effect. The scan is now recursive with an `_roster_source_files()`), which is the same exclusion stated as a decision rather than obtained as an accident. -- **`scryfall-tagger-v1`** (`SCRYFALL_TAGGER_ANONYMOUS_ID`, - [`MPCAutofill/cardpicker/management/commands/import_external_ip_tags.py`](../MPCAutofill/cardpicker/management/commands/import_external_ip_tags.py)) — - **DORMANT: zero rows written to date.** It imports Scryfall Tagger's +- **`scryfall-tagger-v1`** (`SCRYFALL_TAGGER_ANONYMOUS_ID`) — **RETIRED + 2026-07-30, together with `PrintingTagVote` and its importer + (`management/commands/import_external_ip_tags.py`, deleted).** It is no + longer a calculator identity, and the roster tether no longer derives it + from the code, because there is no code. The entry is kept rather than + deleted so that a reader meeting the string in an old report, run log or + database column can find out what it was; everything below is written in + the past tense and describes a design that never ran. Retirement + rationale and the full behavioural record: + [`features/printing-tags.md`](features/printing-tags.md). + + **It wrote zero rows, ever.** It would have imported Scryfall Tagger's `art:external-ip` community art tag (Universes Beyond illustrations — - Lord of the Rings, Doctor Who, Warhammer 40K) and casts machine + Lord of the Rings, Doctor Who, Warhammer 40K) and cast machine `PrintingTagVote` rows against the `external-ip` tag, at the PRINTING - level (`CanonicalCard`), not the catalog-image level. It writes BOTH - polarities: `APPLY` for positive Tagger matches, `NOT_APPLICABLE` for - confirmed printings absent from the positive set; a printing with no - data at all abstains rather than voting. `source=DEDUCTION` with its own - `anonymous_id` per the machine-caster convention — pure logical - inference over already-trusted structured data, zero image inspection. - Weight resolves to `PRINTING_TAG_MACHINE_WEIGHT` (0.5); the 2026-07-23 - zero-weight override does NOT touch it (that override is scoped to - source + the `deductive-backfill` family + one frozen `run_id`, all - three together). Re-runs are idempotent via the - `(printing, tag, anonymous_id)` uniqueness constraint; retraction is the - ordinary `purge_machine_votes --run-id` mechanism. Full behavioural - writeup: [`features/printing-tags.md`](features/printing-tags.md) — this - entry is the ROSTER entry, recording what the gate can and cannot say - about it, and the answer today is **nothing, because it has produced - nothing**. Do not read its absence from every vote count on this page as - evidence it is working correctly; it has never run against production. - -**Open, and an owner call rather than a lint fix**: `PrintingTagVote` -(`models.py`, added by PR #497) is a **third vote family** alongside -`CardPrintingTag` and `CardTagVote`, and it appears **zero times** on this -page and zero times in [`theory.md`](theory.md). Every vote-population -figure above — the topline counts, the FIG-2 funnel, the recompute -dry-runs — is silent about it. Whether `PrintingTagVote` belongs inside -this gate's scope, or is deliberately outside it, has never been decided; -it is recorded here as an open question, not answered. + level (`CanonicalCard`), not the catalog-image level. It was designed to + write BOTH polarities: `APPLY` for positive Tagger matches, + `NOT_APPLICABLE` for confirmed printings absent from the positive set, + with a printing that had no data at all abstaining rather than voting. + `source=DEDUCTION` with its own `anonymous_id` per the machine-caster + convention — pure logical inference over already-trusted structured + data, zero image inspection. Weight resolved to + `PRINTING_TAG_MACHINE_WEIGHT` (0.5); the 2026-07-23 zero-weight override + did NOT touch it (that override is scoped to source + the + `deductive-backfill` family + one frozen `run_id`, all three together). + Re-runs would have been idempotent via the + `(printing, tag, anonymous_id)` uniqueness constraint, with retraction + by the ordinary `purge_machine_votes --run-id` mechanism. What the gate + could say about it was always **nothing, because it produced nothing** — + and its absence from every vote count on this page was never evidence + that it worked. + +**Resolved 2026-07-30, previously an open owner call**: `PrintingTagVote` +(`models.py`, added by PR #497) was a **third vote family** alongside +`CardPrintingTag` and `CardTagVote`, and it appeared **zero times** on +this page and zero times in [`theory.md`](theory.md) — every +vote-population figure above was silent about it. The question recorded +here was whether it belonged inside this gate's scope or was deliberately +outside it. It has now been answered by removal rather than by scoping: +the model, its table and its only writer were retired on the owner's +ruling, the table having held 0 rows on production throughout its life. +The figures on this page are therefore complete as they stand, which they +were not while this question was open. ### Operational notes diff --git a/docs/reports/2026-07-29-fidelity-gate-recheck.md b/docs/reports/2026-07-29-fidelity-gate-recheck.md index 21a658e1d..41a5b086b 100644 --- a/docs/reports/2026-07-29-fidelity-gate-recheck.md +++ b/docs/reports/2026-07-29-fidelity-gate-recheck.md @@ -1,5 +1,19 @@ # Pipeline-fidelity gate — artifact-2 re-check and verdict re-test (2026-07-29) +> **PARTLY SUPERSEDED, same day.** Everything this report says about +> `PrintingTagVote`, `scryfall-tagger-v1` and +> `management/commands/import_external_ip_tags.py` was accurate when measured +> and is now history: the owner ruled that day to retire `PrintingTagVote`, and +> the model, its table, its submit endpoint and that command were all removed +> (migration `0101_delete_printingtagvote`). This report's own finding — 0 rows, +> nothing ever resolved, an identity the roster tether could not see — is part +> of what motivated the removal, so it is left standing rather than rewritten. +> `scryfall-tagger-v1` is no longer a calculator identity at all and should not +> be expected on any roster. The retirement record, including the import +> algorithm deleted with the command, is the External-IP entry in +> [`../features/printing-tags.md`](../features/printing-tags.md). Nothing else +> in this report is affected. + Read-only audit. No writes, no management command, no migration, no deploy. Every live figure below was queried against production Postgres via `sudo docker exec mpcautofill_django python manage.py shell` on 2026-07-29.