Skip to content

Retire PrintingTagVote: the vote channel that had no resolver, no reader and no rows - #615

Merged
WilfordGrimley merged 3 commits into
masterfrom
feat/retire-printing-tag-vote
Jul 30, 2026
Merged

Retire PrintingTagVote: the vote channel that had no resolver, no reader and no rows#615
WilfordGrimley merged 3 commits into
masterfrom
feat/retire-printing-tag-vote

Conversation

@WilfordGrimley

Copy link
Copy Markdown

Owner ruling, 2026-07-29: "i am willing to not need the printing tag. i am happy to reduce things to the minimum that gives us our expected results. we don't need to reinvent the wheel everyday." And on its origin: "printingtag was half cooked."

Implements PR #599's §7.1 / §8 recommendation. Every claim below was re-verified against production and the working tree by this PR's author, not taken on trust from the investigation.


Why this is safe

check result
PrintingTagVote rows in production 0 (select count(*) from cardpicker_printingtagvote, read-only, 2026-07-29)
…of which human 0 — the table is empty of every source
consensus resolver none exists. printing_tag_consensus.py, forward-referenced by two docstrings, is absent from disk, from git log --all, and from every open PR
readers none outside the Django admin
frontend callers of submitPrintingTagVote none anywhere in frontend/src/ (re-grepped)
PROTECTED CORE not protected. docs/upstreaming/license-provenance.md §2 excludes models.py from file-level protection; its manual-review carve-out names exactly VoteSource, AbstractWeightedVote, CanonicalPrintingMetadata, CardPrintingTag. The three protected consensus modules do not reference PrintingTagVote. No license-provenance review is triggered.

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.

The deeper reason it was unsalvageable rather than merely unused: an imported Scryfall fact is not a disputable claim. 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 — the model was built to carry an indisputable fact through a mechanism designed for disputable ones.

What is NOT touched

The names are confusingly adjacent and an earlier orchestrator conflated two of them while briefing this change. Stated explicitly:

  • CardPrintingTag — 167,206 rows live. "This catalogue IMAGE depicts this Scryfall PRINTING". The entire Stage D printing channel, read by printing_consensus into Card.printing_tag_status / inferred_canonical_card / Elasticsearch. Untouched.
  • CardTagVote — 223,909 rows live. Read by tag_consensus into Card.tags. Untouched.
  • PRINTING_TAG_MIN_VOTES / PRINTING_TAG_IMPLICIT_CAP / PRINTING_TAG_MACHINE_WEIGHT — app-wide consensus weights, never on a PrintingTagVote path. Untouched.
  • _split_new_printing_tag_votes / _purge_and_write_printing_tag_votesCardPrintingTag collision guards (list[CardPrintingTag] signatures). Untouched.

verify_no_machine_only_resolutions already excluded PrintingTagVote at purge_machine_votes.py:139confirmed; only its now-stale explanatory note changed, no gate logic.

What shipped

  1. PrintingTagVote model deleted; migration 0101_delete_printingtagvote (DeleteModel).
  2. views.post_submit_printing_tag_vote, _SubmitPrintingTagVoteRequest, _get_canonical_card_or_400, and the 2/submitPrintingTagVote/ route deleted. No frontend caller existed, so this is not a breaking API change in practice. (The rate-limit helpers named _printing_tag_rate_limit_* stay — five other endpoints share them.)
  3. AdminPrintingTagVote deleted.
  4. purge_machine_votes: the queryset, both .delete() calls, and PurgeResult.printing_tag_votes_deleted removed. The counter was dropped, not pinned at 0 — a purge report naming a table that no longer exists is a claim about work it did not do, and this module's whole premise is that its output must be sufficient on its own to reconstruct what was destroyed. The rationale is written into _report_counts' docstring beside the pre-existing "keep the wording stable for log scrapers" note it deviates from. test_purge_machine_votes.py's test_covers_all_four_vote_tables becomes test_covers_every_vote_table over three.
  5. manage.py import_external_ip_tags (579 lines) and test_import_external_ip_tags.py deleted — see the ruling below.
  6. test_submit_printing_tag_vote.py deleted.
  7. Both phantom printing_tag_consensus.py forward-references are gone, along with the file that carried them.
  8. Docstring corrections in vote_write.py and integrations/game/scryfall_bulk_data.py (whose "three importers" origin story now names two live ones and one retired).

The dormant importer — ruling: REMOVE, with the knowledge relocated, not the PR description

It could not have been kept. The command's entire purpose is writing PrintingTagVote rows; with the model gone it would not import, let alone run. "Keep it and make it fail loudly" would mean gutting its write path anyway, leaving a stub that preserves nothing.

So the knowledge was moved somewhere greppable from a checkout, not left in a PR description. docs/features/printing-tags.md's External-IP entry is rewritten from "here is how the import works" into a retirement record carrying:

  • the reusable algorithm in full prose — the art_tags bulk entry, the BFS-to-fixpoint over child_ids from the external-ip slug (deliberately not the one level the original plan specified) with its fail-loud-on-missing-slug rule, the two-pass tag/tagging split that keeps the taggings payload out of memory during indexing, and the illustration_id -> default_cards (incl. per-face card_faces) -> CanonicalCard.identifier join;
  • the measured numbers on both signals: promo_types reaches 10,407 of 113,224 printings at 100% per-set recall; art:external-ip closes over 2,799 tags / 8,332 illustrations reaching ~13,166 — an uncharacterised ~2,759 delta that is the whole case for the artwork-origin question being distinct;
  • what a rebuild must not carry: the vote framing, the negative pass (~100k NOT_APPLICABLE rows into a table nothing read), the source=DEDUCTION/scryfall-tagger-v1/run_id/purge plumbing, and security_stamp as a UB signal;
  • the grain finding (0 of 50,828 illustrations appear on both a UB and a non-UB printing, despite 25,381 reprinted and 20,542 crossing sets) — so an illustration-keyed attribute is the right shape;
  • git show e6c6429a:MPCAutofill/cardpicker/management/commands/import_external_ip_tags.py to read the deleted code verbatim.

Two contracts were rescued rather than deleted with it:

  • EXTERNAL_IP_TAG_NAME moved to reason_tags.py, beside the surviving human channel. It is a convergence contract — both channels must write one Tag.name so tag:external-ip is a single predicate — and it now outlives the code that used to honour it. test_reason_tags still pins it.
  • vote_write's target_field parameter lost its only production caller. Its two tests are re-pointed at CardPrintingTag.printing_id (which also has that column) rather than deleted, so the parameter stays exercised instead of quietly becoming a card_id constant. The re-pointed test is sharper than the original: the stale row belongs to a different card, same printing, so only a printing_id-keyed purge can reach it.

scryfall_bulk_data.ART_TAGS is kept with its live-endpoint test although nothing now consumes it — one constant plus one test is cheaper to carry than re-deriving which bulk entry type the Tagger feed lives under, and the test is what would tell us if it disappeared upstream in the meantime.

Verification

  • Production row count: 0, confirmed directly before writing the DeleteModel, alongside CardPrintingTag 167,206 and CardTagVote 223,909 as a positive check that the right table was identified.
  • Full cardpicker/tests/ suite: 3237 passed, 8 skipped, 0 failed (4m16s, run twice).
  • makemigrations --check --dry-run: "No changes detected", exit 0.
  • Migration graph leaf count for cardpicker: exactly 1 ([('cardpicker', '0101_delete_printingtagvote')]), asserted via MigrationLoader.
  • docs_lint.py --strict: clean.
  • pre-commit on every changed file (ruff, isort, black, mypy, prettier, README parity): all pass.
  • Grep sweep for PrintingTagVote, printing_tag_consensus, import_external_ip_tags, submitPrintingTagVote, scryfall-tagger: every surviving hit is a deliberate historical aside, plus the two TestSplitNewPrintingTagVotes / TestPurgeAndWritePrintingTagVotes class names that are about CardPrintingTag and must stay.

⚠️ Migration numbering — read before merging

The file is numbered 0101 per the orchestrator's allocation (#601 → 0099, #604 → 0100), but its dependencies point at 0098_card_illustration_consensus_fields, master's real and only leaf, so this branch is migratable and single-leaf today and its CI can actually run.

This follows the convention 0098_superseded_card_printing_tag_archive (#604) states in its own docstring: "a dependency on a migration that does not exist on master makes THIS branch unmigratable today, with certainty, in exchange for avoiding a collision that may never happen. A collision, by contrast, is loud and caught at merge time by makemigrations --check."

=> Whichever of #601 / #604 merges before this, repoint this migration's dependency at the new leaf on rebase. Leaving it on 0098 after 0099/0100 land gives cardpicker two leaves, and because pytest-django builds its test DB by running migrate, that takes every branch's CI down — the failure #576 exists to repair. The migration's own docstring says all of this at the top.

Current state confirmed 2026-07-29: #601 has already renumbered to 0099_rename_printings_count_catalogued; #604 is still at 0098 as 0098_superseded_card_printing_tag_archive and expected to become 0100.

⚠️ Sequencing against #588

#588 (open) adds a docs/pipeline-fidelity-gate.md roster entry for scryfall-tagger-v1 that links to management/commands/import_external_ip_tags.py — a file this PR deletes. If #588 merges first, that entry must be removed (or rewritten as retired) here on rebase, or docs_lint's path check will fail on the dead link. If this merges first, #588's recursive-glob fix will simply not find the identity and its roster row should not be added.

Per PR #599 §10.10 the recommended order is #588 first: the tether fix is correct independent of this ruling, and a row that is later deleted is cheaper than a tether that silently misses the next dormant identity. Either order works; they must not be merged blind to each other.

Deviations from the brief

  1. Migration dependency on 0098 rather than the not-yet-existent 0100. See above. The brief also required verifying a single leaf and running the suite — impossible with a dangling dependency, since Django cannot build the graph at all. Both requirements are met this way; the merge-time repoint is documented in three places.
  2. printing_tag_votes_deleted removed rather than pinned at 0. PR Decision doc: should PrintingTagVote exist? Printing-tag vs illustration-tag grain #599 §8.2 offered both. Reasoning is in _report_counts' docstring and above.
  3. Two vote_write tests re-pointed rather than deleted, to keep target_field a tested parameter.
  4. docs/reports/2026-07-29-fidelity-gate-recheck.md got a dated "PARTLY SUPERSEDED" banner rather than edits. It is a same-day audit whose PrintingTagVote/scryfall-tagger-v1 findings were accurate when measured and are part of what motivated this removal; rewriting a dated report to match a later decision is the opposite of what this repo's correction log exists for.
  5. test_vote_consensus.py's "some other calculator" example swapped from scryfall-tagger-v1 to stage-d-illustration-v1. The assertion is about the three-conjunct zero-weight rule and is unchanged, but an example identity nobody can look up is a worse example.

Open items for the owner

  1. Seed the external-ip Tag in productionmanage.py seed_no_match_reason_tags. It does not exist live (Tag.objects.count() is 30), so the human card-grain channel — the one that works end to end — still cannot record external-ip. Independent of everything here, and one command.
  2. Characterise the ~2,759-printing delta between art:external-ip and promo_types. A ~50-row sample decides whether the Tagger dependency survives into the unified importer at all. The algorithm to do it is preserved in the retirement record.
  3. Surface UB from promo_types — 3,450 catalogue images already resolve to a UB printing today through the existing Card.canonical_card link, with no vote of any kind. Denormalising onto Card.tags gets Elasticsearch filtering free and shares one predicate with the human channel.

🤖 Generated with Claude Code

https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN

@WilfordGrimley

Copy link
Copy Markdown
Author

Rebased onto origin/master (4a3babc); dependency repointed to 0100. This PR's CI is red on purpose.

Merge order is fixed: #604 first, then this one. 0101_delete_printingtagvote now depends on 0100_superseded_card_printing_tag_archive (PR #604), which is not on master yet — so migrate on this branch alone raises NodeNotFoundError, every test errors at pytest-django's test-database setup, and #611's leaf guard reports two leaves. All five failing checks are that one cause. The red clears the moment #604 merges. Do not 'fix' it by repointing at 0099.

Why this order, and why one of the two branches had to be red

Exactly one of #604/#615 can own the dependency on master's real leaf (0099), so exactly one can be migratable at a time. The choice is which PR to leave unverifiable, not whether to leave one. #604 is the larger change and the Stage-D monolith's core, so it keeps the verifiable, green branch; this PR is small and cheap to hold.

The rejected alternative — depend on 0099, renumber this 0100 — makes this branch green immediately but leaves cardpicker with two leaves after both merge, which takes every branch's CI in the repo down at test-database setup. A local red costs one PR; a forked graph costs the repo. (The earlier docstring preferred the green-now convention on the then-true premise that nothing had claimed a number above master's leaf. #604 has now claimed 0100, which inverts that trade-off.)

Verified on a simulated merge, not by eye

master + #604 + this branch merges cleanly, and on that tree:

Two real rebase breakages fixed (unrelated to migrations)

Master's #588 added a fidelity-gate roster entry and a real-repo tether assertion for scryfall-tagger-v1 — the one calculator identity declared under management/commands/, the subtree this PR deletes. Both went stale:

  • docs/pipeline-fidelity-gate.md: roster entry rewritten as a retirement record (broken link to the deleted command removed, knowledge kept). Its adjacent "open owner call" on PrintingTagVote's absence from the gate is now marked resolved — answered by removal rather than by scoping.
  • .github/scripts/tests/test_docs_lint.py: scryfall-tagger-v1 removed from the real-repo tether list, which no longer has a live management/commands/ witness. The recursion property it guarded stays covered on fixtures by test_scan_is_recursive_but_bounded.

docs_lint.py --strict clean; its 47 rule unit tests pass.

🤖 Generated with Claude Code

WilfordGrimley and others added 3 commits July 30, 2026 08:13
…der and no rows

Owner ruling, 2026-07-29: "i am willing to not need the printing tag. i am happy
to reduce things to the minimum that gives us our expected results." PR #599's
investigation measured the case against production and it is not close.

`PrintingTagVote` held 0 rows, 0 of them human, 0 ever resolved. It had no
consensus resolver anywhere in the repo's history - `printing_tag_consensus.py`,
which two docstrings forward-referenced, never existed on any branch - no reader
outside the Django admin, and no frontend caller of its submit endpoint. Its only
machine writer, `manage.py import_external_ip_tags`, never ran once. Underneath
that sits the reason it was unsalvageable rather than merely unused: an imported
Scryfall fact is not a disputable claim, and `resolve_weighted_consensus`'s
`has_human_backed` gate is absolute, so a machine-only channel returns None at any
volume. It was built to carry an indisputable fact through a mechanism designed
for disputable ones.

Removed: the model and its table (migration 0101), `POST 2/submitPrintingTagVote/`
and its route, the admin registration, the importer and its tests, and the
`PrintingTagVote` arm of `purge_machine_votes` (its report now enumerates three
vote tables, not four - the always-zero counter was dropped rather than pinned,
since a purge report naming a table that no longer exists is a claim about work
it did not do).

NOT touched, despite adjacent names: `CardPrintingTag` (167,229 rows, the whole
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`, a `CardPrintingTag` collision guard.

Knowledge preserved rather than evaporated. `docs/features/printing-tags.md`'s
External-IP entry is rewritten as a retirement record carrying the deleted
importer's algorithm (the art_tags BFS-to-fixpoint over `child_ids`, the
two-pass tag/tagging split, the `illustration_id -> default_cards -> CanonicalCard`
join), the measured numbers on both sides (`promo_types` 10,407 vs `art:external-ip`
~13,166, an uncharacterised ~2,759 delta), what a rebuild must not carry, and the
commit to `git show` the deleted file from. `EXTERNAL_IP_TAG_NAME` moved to
`reason_tags.py` so the one-Tag.name convergence contract outlives the code that
used to honour it. `vote_write`'s `target_field` parameter lost its only
production caller, so its two tests are re-pointed at `CardPrintingTag.printing_id`
rather than deleted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN
…615

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…r test

Master's #588 added a roster entry and a real-repo tether assertion for
scryfall-tagger-v1, the one calculator identity declared under
management/commands/. This PR deletes that command, so both went stale on
rebase: a broken path link in docs/pipeline-fidelity-gate.md and a
tether assertion for an identity that no longer exists.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@WilfordGrimley
WilfordGrimley force-pushed the feat/retire-printing-tag-vote branch from 5a8b6ea to bd56be1 Compare July 30, 2026 08:20
@WilfordGrimley
WilfordGrimley merged commit b1fb2c9 into master Jul 30, 2026
11 checks passed
WilfordGrimley added a commit that referenced this pull request Jul 30, 2026
The central open question of this decision document — should
PrintingTagVote exist — was answered no and implemented while the PR sat
unmerged. Adds a dated update note, closes open question 1, and corrects
the reports index entry so the document reads as a record rather than a
live proposal. The analysis itself is left as written.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
WilfordGrimley added a commit that referenced this pull request Jul 30, 2026
…ion-tag grain (#599)

* docs: printing-tag vs illustration-tag grain — decision document

Investigation only; no pipeline code touched. Answers whether
PrintingTagVote should continue to exist.

Measured (production, read-only):
- PrintingTagVote: 0 rows, 0 of them human. No consensus resolver exists
  anywhere; printing_tag_consensus.py, forward-referenced by two
  docstrings, has never existed on any branch. Nothing reads the table.
- resolve_weighted_consensus applies a hard has_human_backed gate
  independent of the weight math. Executed: machine-only vote sets return
  None at n=1,2,4,10,1000. The original "tag art:external-ip cards as UB
  automatically" design was unreachable at every grain, not just this one.
- CanonicalPrintingMetadata.promo_types already carries the Scryfall token
  "universesbeyond" on 10,407 of 113,224 printings, 100% per-set recall on
  every dedicated UB set, correctly partial on sld/clu, correctly zero on
  unf. set_type and security_stamp are not stored at all.
- Counterexample search for illustration grain: 0 of 50,828 illustrations
  appear on both a UB and a non-UB printing, against 25,381 reprinted and
  20,542 crossing set boundaries. UB-ness travels with the artwork.
- art:external-ip is broader than promo_types: ~13,166 printings vs
  10,407. The ~2,759 delta is counted but not characterised.
- Only 4 of 230,770 catalogue images have a vote-resolved printing;
  19,475 have the ingestion-time canonical_card link, of which 3,450
  already resolve to a UB printing today.
- The external-ip Tag does not exist in production, so neither the
  printing-tag endpoint nor the human no-match reason strip can record it.
- import_external_ip_tags was forgotten, not abandoned: an owner-ratified
  step 4d on 2026-07-28 that fell out of the handoff chain, invisible to
  the roster tether's non-recursive glob. Every prerequisite passes today.

Also disambiguates the three adjacent vote models and traces
PRINTING_TAG_MIN_VOTES, PRINTING_TAG_IMPLICIT_CAP and
_split_new_printing_tag_votes to their call sites — none govern
PrintingTagVote.

Ten numbered open questions for the owner. Indexed in docs/reports/
README.md; docs_lint.py --strict clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN

* docs: fold in the three owner rulings of 2026-07-29

1. The machine-only-cannot-resolve gate is the design, not a defect
   ("that is the entire theory in one sentence, we designed things like
   this on purpose"). Reframed as the governing constraint and moved to
   lead the document: any tag category expected to resolve from machine
   evidence alone is mis-specified by construction; the correct design
   either routes it through human confirmation or does not model it as a
   vote at all. Votes are for disputable claims; an imported Scryfall
   fact is not one.

2. Why import_external_ip_tags never ran: never rebuilt after Scryfall
   changed their API process, and it belongs in the new unified Scryfall
   importer. Recorded as the authoritative account. The measured record
   (PR #555 repaired the bulk-data path afterwards; every gate passes
   today; the scheduled run then fell out of the handoff chain) is kept
   alongside rather than dropped, since it does not perfectly match — the
   two accounts compose. New section 8.7 specifies what the unified
   importer must carry and what it must not (no negative pass, no
   vote-system plumbing, no security_stamp revival, illustration grain
   rather than printing).

3. Owner is willing to drop PrintingTagVote, and regards the existing
   implementation as half-cooked. Section 7.1 updated; the recommendation
   and its evidence are unchanged.

New section 0.1 answers the reframed question directly — what is the
smallest thing that delivers the UB filter:
(a) official printings need no vote at all (promo_types is deterministic
    and already ingested; 3,450 catalogue images already resolve to a UB
    printing via the existing canonical_card link);
(b) user-tagging of alternate-art cards needs card grain, not
    illustration grain, because a custom image has no illustration id to
    key on — and CardTagVote already does it end to end, sharing one
    Tag.name with the machine path so tag:external-ip is one predicate;
(c) the migration is empty: 0 rows, 0 human votes.

Open questions renumbered to 11; the three settled ones are recorded as
closed rather than re-asked. docs_lint.py --strict clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN

* docs: mark the PrintingTagVote retirement as ruled and executed (#615)

The central open question of this decision document — should
PrintingTagVote exist — was answered no and implemented while the PR sat
unmerged. Adds a dated update note, closes open question 1, and corrects
the reports index entry so the document reads as a record rather than a
live proposal. The analysis itself is left as written.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant