Illustration consensus: md5 identity-group pooling and sibling propagation, built in from the first line - #573
Conversation
Mutation proofs — all 24, and the per-test coverage mapMethod: apply one mutation to the implementation (never to the tests), run
Coverage: 50 of 50 tests fail under at least one mutation. Two were strengthened after the first batch showed them passing for the wrong reason, and are re-proven above:
|
431fd73 to
a61ebe5
Compare
Rebased onto master; migration renumbered
|
…ation from the first line `CardIllustrationVote` (issue #524) has been WRITTEN since it landed and read by nothing but the admin and the human write path. `printing_consensus.py`, `artist_consensus.py` and `tag_consensus.py` each reconcile their own vote model; there was no illustration equivalent, so PR #565's projected ~10,277 machine rows would have been recorded rather than reasoned over. 1. `cardpicker/illustration_consensus.py`, modelled on its two siblings and built on the shared `vote_consensus` core - no weighting, quorum or human-backed gate is re-derived. `is_unknown` is tallied as an ORDINARY OUTCOME KEY (the `UNKNOWN` sentinel, exactly as `artist_consensus` treats `CardArtistVote.is_unknown`): the abstention on this model is the ABSENCE OF A ROW, so an `is_unknown=True` row is a positive claim that must be able to resolve on its own AND to contest a uuid. 2. MD5 POOLING IS BUILT IN, NOT RETROFITTED. Every read path is group-scoped: `resolve_illustration` tallies `card`'s whole md5 identity group, pooled per agent by `vote_consensus.pool_group_votes`, keyed on `agent_dedupe_key` (the versionless calculator family - #565 bumping this calculator v1->v2 is exactly the case a raw-id key would misread as two agents). The group primitives are IMPORTED from `printing_consensus`, so there is one definition of a group, one completeness guard and one agent-identity rule. md5 is strictly stronger than the perceptual art hash here: byte-identical files are necessarily the same artwork, with no threshold and no tolerance. 3. PROPAGATION IS A CONSEQUENCE OF GROUP-SCOPED RESOLUTION, NOT A SEPARATE STEP. A member whose decorated name fails candidate resolution abstains with `no-candidate-match` (367 of 2,350 considered cards in #565's replay) even though a byte-identical sibling resolved. Its tally IS the group's tally, and `resolve_and_persist_illustration` writes the outcome to every member, so it receives the resolved artwork with no path aware it abstained. The rejected alternative - writing a copied `CardIllustrationVote` row - would contribute exactly ZERO weight (it pools under the same agent key; pinned by a test), would manufacture a claim no agent made, and would collide with the model's unconditional (card, anonymous_id) constraint. `Card.inferred_illustration_id` (plain UUIDField, not an FK - no `CanonicalIllustration` table, and no reference-data join to go stale) plus `Card.illustration_vote_status` (migration 0096), written for every group member. `ILLUSTRATION_MIN_VOTES`/`ILLUSTRATION_MIN_SHARE` default to the printing values, changing nothing; there is deliberately no illustration machine weight, since a vote's weight is a property of who cast it, never of what is being voted on. REFERENCE DATA (owner ruling 2026-07-29): this module reads neither `CanonicalCard` nor `CanonicalPrintingMetadata` - it tallies uuids off vote rows and stores the winner verbatim, asserted by `TestReferenceDataIndependence`. A stale snapshot can under-supply the uuids agents have to vote for (upstream) and narrow what a resolved uuid maps to (downstream, a live join every consumer performs itself); it cannot change a tally, flip a winner, or move a propagation. TESTS. 50 new tests, and every one was verified RED against a deliberately mutated implementation (24 mutations; each of the 50 fails under at least one). Coverage includes md5 pooling (agreeing siblings collapse; a self-contradicting agent is withheld order-independently; distinct agents still sum; a group of one is a byte-for-byte no-op including query shape), propagation (a voteless sibling inherits; it does NOT inherit across an identical `content_phash`, with a same-phash-plus-matching-md5 positive control proving the negative is not vacuous), and the human-backed gate for this vote type. HARNESS FIX, and it is not incidental. `test_shared_cache.py::TestSharedCacheTable Migration` carries `@pytest.mark.django_db(transaction=True)`, so its migration round-trip REALLY COMMITS - and its `finally` restored only as far as `0092`, leaving every later migration unapplied for the rest of the session. Latent and free until now purely by alphabetical luck (0093/0094 are data-only; 0095 is read only by modules sorting earlier). A migration adding a `Card` column is the first thing to step on it: `test_sources.py` and `test_stage_e_dispatch.py` sort later and use `transactional_db`, and failed with `column "inferred_illustration_id" ... does not exist` - a message pointing at this branch for a defect entirely in that `finally`. Now restores to `graph.leaf_nodes("cardpicker")`, derived rather than hardcoded. VERIFIED: full `cardpicker/tests/` suite green (3,090 passed, 9 skipped) against a 3,040-passed baseline on the same commit of master. black/isort/ruff/mypy and docs-lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN
a61ebe5 to
b984cc3
Compare
…e "cross-verified against Scryfall" claim
`CanonicalPrintingMetadata.printings_count` sat on a model whose docstring
said "Scryfall printing-level fields", and the docs read it that way. It is
not Scryfall data. `import_scryfall_printing_metadata` builds a Counter over
`CanonicalCard.canonical_id` — our own table — and stores each row's oracle
group size. Rows with a NULL canonical_id are stored as 1 by fiat.
That difference is load-bearing. A column derived from our catalogue cannot
detect that our catalogue is incomplete, which is exactly what deductive
backfill's first tier advertised it as doing ("cross-verified against
Scryfall's own printings_count (not just 'our table happens to have one
row')" — it was precisely the latter).
Measured against the live catalogue, 2026-07-29:
- 14,893 normalised names have exactly one CanonicalCard row. All 14,893
carry a count of 1. Zero carry >1. Zero are NULL. The tier's second
condition is entailed by the name-uniqueness test one line above it.
- 137 cards reach that condition out of an eligible pool of 104,969;
137 pass. The gate excludes nothing and never could.
- Counting the Scryfall bulk file directly finds 2 oracle ids where we
hold one row and Scryfall publishes more — the exact case the gate
claimed to catch, invisible to it by construction.
The condition is left in the code, labelled as entailed rather than deleted,
so the gap stays visible; issue #592 tracks what a real external check is.
Per the "do not make false assertions" directive the claim is deleted, not
softened: the tier's documented claim is now the one it can support — the
name matches exactly one row in our catalogue.
Also corrects the derived claim in local_identify_printing_tags, which said
its selection "revisits single-candidate names deductive backfill's Scryfall
printings_count cross-check rejected". That cohort is empty and always was.
Migration 0098 is a pure RenameField (ALTER TABLE ... RENAME COLUMN — no
table rewrite, no data touched). NOTE: PR #573 also adds an 0098; whichever
merges second must renumber to 0099.
…e "cross-verified against Scryfall" claim
`CanonicalPrintingMetadata.printings_count` sat on a model whose docstring
said "Scryfall printing-level fields", and the docs read it that way. It is
not Scryfall data. `import_scryfall_printing_metadata` builds a Counter over
`CanonicalCard.canonical_id` — our own table — and stores each row's oracle
group size. Rows with a NULL canonical_id are stored as 1 by fiat.
That difference is load-bearing. A column derived from our catalogue cannot
detect that our catalogue is incomplete, which is exactly what deductive
backfill's first tier advertised it as doing ("cross-verified against
Scryfall's own printings_count (not just 'our table happens to have one
row')" — it was precisely the latter).
Measured against the live catalogue, 2026-07-29:
- 14,893 normalised names have exactly one CanonicalCard row. All 14,893
carry a count of 1. Zero carry >1. Zero are NULL. The tier's second
condition is entailed by the name-uniqueness test one line above it.
- 137 cards reach that condition out of an eligible pool of 104,969;
137 pass. The gate excludes nothing and never could.
- Counting the Scryfall bulk file directly finds 2 oracle ids where we
hold one row and Scryfall publishes more — the exact case the gate
claimed to catch, invisible to it by construction.
The condition is left in the code, labelled as entailed rather than deleted,
so the gap stays visible; issue #592 tracks what a real external check is.
Per the "do not make false assertions" directive the claim is deleted, not
softened: the tier's documented claim is now the one it can support — the
name matches exactly one row in our catalogue.
Also corrects the derived claim in local_identify_printing_tags, which said
its selection "revisits single-candidate names deductive backfill's Scryfall
printings_count cross-check rejected". That cohort is empty and always was.
Migration 0098 is a pure RenameField (ALTER TABLE ... RENAME COLUMN — no
table rewrite, no data touched). NOTE: PR #573 also adds an 0098; whichever
merges second must renumber to 0099.
…0098 #573 merged its own `0098_card_illustration_consensus_fields` first, also depending on `0097_freeze_deductive_backfill_zero_weight_cohort`. This branch's `0098_rename_printings_count_catalogued` depended on the same 0097, so the merge of the two would have given `cardpicker` TWO leaf nodes - and pytest-django builds its test database by running `migrate`, so the fork errors at test-database SETUP on every branch in the repo, not just this one. That is the outage #576 had to repair at 0096. Nothing normally trusted showed it. The filenames differ so there was no textual conflict; GitHub reported this PR MERGEABLE/CLEAN; its checks were 10/10 green because they had run against master BEFORE #573 landed, and GitHub does not re-run a PR's checks when its base moves. - rebase onto master (d044223) - `git mv` the migration to `0099_rename_printings_count_catalogued.py` and repoint `dependencies` at `0098_card_illustration_consensus_fields` - rewrite the migration's own MIGRATION-GRAPH NOTE, which stated the old number and predicted this collision, to record what actually happened - update the three other places that state the number in prose: `models.py`'s field comment, `deductive_backfill.py`'s module docstring, `docs/features/printing-tags.md` The operation is unchanged: still a single `RenameField` on `CanonicalPrintingMetadata.printings_count`, which PostgreSQL executes as `ALTER TABLE ... RENAME COLUMN` - catalogue metadata only, no table rewrite, no row read or written, fully reversible. Only the number and the dependency moved. Verified: `cardpicker` has exactly one leaf (`0099_rename_printings_count_catalogued`); `makemigrations --check --dry-run` reports no changes detected; `cardpicker/tests/` 3263 passed, 8 skipped; `docs_lint.py --strict` clean; pre-commit clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN
… superseded votes
"Prior runs must not suppress work in a new run. The CURRENT run's own
output must, so a killed run resumes rather than redoing completed batches."
- owner directive, 2026-07-29
"Keep at least one prior generation of votes, whose votes are NOT counted."
- ratified separately
WHY THESE THREE ARE ONE COMMIT AND NOT THREE
They are not independently reviewable, and shipping any one alone is worse than
shipping none:
- Un-suppressing eligibility ALONE buys exactly nothing. The calculator
recomputes the verdict and the pre-write split then drops it before the
write, and because `purge_and_write_votes` scopes its purge to the rows
being written, a dropped row purges NOTHING - the stale vote survives
verbatim, with no error, no counter moving, and a recomputation's worth of
work discarded. Two independent suppression layers, and the second silently
defeats a fix to the first.
- The archive has no effect until the split permits an overwrite, and the
owner ruled it must not ship separately from the work that creates
superseded rows in the first place.
The run_id AUDIT this depends on is a separate, genuinely independent PR
(`fix/run-id-population-gaps`), which this is stacked on.
LAYER 1 - RUN-SCOPED ELIGIBILITY
Every Stage D printing-channel calculator asked "have I EVER voted on / abstained
on this card?". That predicate grows monotonically, so each pass could only ever
see a subset of the previous pass's pool, and a repaired engine could never
re-examine anything the broken one had answered. Not hypothetical:
`stage-d-illustration` had to be version-bumped v1 -> v2 purely to escape its own
non-rescannable scan-log rows after its layout_class gate turned out to be
reading a border colour - 3,409 wrongly-skipped cards were otherwise unreachable
to a repaired v1. Under run-scoping the repair alone would have sufficed.
Both self-suppressing excludes - the printing-tag vote exclude and the
non-rescannable `CardScanLog` exclude - now additionally match the CURRENT
run_id, in `_eligible_cards_queryset` (join-key + fallback),
`_slow_path_eligible_cards_queryset`, `_eligible_illustration_cards_queryset`
and `_eligible_base_queryset` (opt-in there; only lands passes a run_id today -
`run_pilot`/`run_name_frequency_elimination` are a different workload with their
own fetch budgets and resume semantics, and flipping them is a separate decision
with a separate blast radius).
`run_id=None` keeps the pre-change behaviour BYTE-IDENTICALLY, deliberately and
not vestigially: `stream_backstop_sweep.verify_chunk` asks "is there ANY Stage D
backlog", a question about the catalogue rather than about a run, and answering
it run-scoped would report the whole catalogue as backlog on every fresh run_id.
A BUG THIS ALMOST SHIPPED WITH, FOUND BY COMPILING THE SQL RATHER THAN REASONING
ABOUT IT. The obvious spelling is
`.exclude(printing_tags__anonymous_id=X, printing_tags__run_id=Y)`. Django does
NOT combine those into one subquery the same related row must satisfy - it emits
`NOT (EXISTS(... anonymous_id=X ...) AND EXISTS(... run_id=Y ...))`, two
INDEPENDENT clauses. A card carrying THIS identity's vote from an OLD run plus
some OTHER identity's vote from THIS run satisfies both halves and is wrongly
excluded, re-creating the exact cross-run suppression this work removes, in the
hardest direction to notice: fewer cards processed, no error, no counter. It is
the same negated-multi-valued-lookup trap `_eligible_base_queryset`'s own
docstring already documented for the scan-log exclusion. The scoped path uses an
explicit `pk__in` subquery instead; `TestTheCompiledSqlTrap` pins both the SQL
shape and the behaviour.
LAYER 2 - THE SPLIT COMPARES THE VALUE, NOT JUST THE KEY
`_split_new_printing_tag_votes` compared `(card_id, anonymous_id)` alone. It now
compares the whole SET of `(printing_id, is_no_match)` a batch proposes for a
group against what is stored - the shape
`local_illustration._split_new_illustration_votes` has always had, whose own
docstring calls it "THE ONE DIFFERENCE, AND IT IS LOAD-BEARING".
- existing rows, SAME verdict -> skipped, counted in `already_voted`.
Re-running over a converged catalogue stays a no-op, which is what stops
run-scoping becoming an overwrite-everything churn machine.
- existing rows, DIFFERENT verdict -> kept. The purge moves the stale
generation into the archive and the new one lands.
- no existing row -> kept.
Compared PER GROUP, ALL-OR-NOTHING, because one identity can legally hold
several rows for a card (`cardprintingtag_unique_printing_vote` constrains the
triple, not the pair - `run_illustration_calculator`'s loop over
`verdict.printing_pks` is a live caller shaped that way) and the purge is
family-keyed on `card_id`. Keeping only PART of a group would delete the rest
and never re-insert them.
The "skip-and-count, not retract-and-recast" reasoning is untouched: it was
always conditioned on both racing invocations computing the SAME verdict, which
is exactly the case still skipped. What no longer gets swallowed is the case
that reasoning already named as NOT covered - a genuinely changed conclusion.
`local_lands_identify._split_new_votes` needed no change: it already compares the
full (card, printing, anonymous_id) triple, so a changed answer already reached
its purge. A test now pins that rather than leaving it a coincidence.
LAYER 3 - `ArchivedCardPrintingTag`
`purge_stale_machine_votes` copies every row it is about to delete into the
archive first, stamped with the run that overwrote it. That function is THE choke
point for "a machine vote is superseded by a later machine vote", so no caller
can supersede without archiving and no new caller has to remember to.
`vote_write.purge_and_write_votes` supplies `superseded_by_run_id`, derived from
the batch it is writing, and its existing `transaction.atomic()` now covers three
statements instead of two - the same cancel-safety property, one statement wider.
A batch that cannot name a single run_id records NULL rather than a guess: a
wrong stamp is worse than a missing one, since the diff report and issue #575's
janitor both select on it and a plausible wrong value is indistinguishable from a
right one after the fact.
WHY A SEPARATE TABLE AND NOT RETAINED GENERATIONS IN THE LIVE ONE - MEASURED, NOT
PREFERRED. Of the thirteen modules that read `CardPrintingTag`, NINE bypass
`vote_consensus.resolve_vote_weight` entirely: `views.py`, `catalog_stats.py`,
`local_calculate_verdicts.py`, `models.py`, `local_identify_printing_tags.py`,
`soak_gate.py`, `harvest_probe.py`, `illustration_vote.py`,
`local_lands_identify.py`. A zero-weight-by-run_id rule (migration 0097's
pattern) protects only the four that route through weight resolution. A retained
generation left in the live table would still be DISPLAYED by views, COUNTED by
catalog-stats, and - fatally - would make eligibility treat the card as already
voted, re-creating the very suppression this work removes. Keeping the live table
single-generation means no consumer can be wrong about it: no unique-constraint
change, no audit of thirteen modules, no new rule any future reader has to know.
Rows are unreachable from `Card`/`CanonicalCard` (`related_name="+"` on both
FKs), are not an `AbstractWeightedVote` subclass, and copy `created_at` verbatim
rather than re-stamping it, with `archived_at` as the separate honest answer to
"when did this stop being live". Append-only, no unique constraints - the same
shape `CardScanLog` already has. Human votes never reach it: `calculator_family`
returns None for the UUIDs humans use and the purge returns before touching
anything.
Retention is issue #575's janitor's ("keep the N most recent runs per calculator,
sweep the oldest, operator-authorised with a dry run, never delete wholesale").
Both `run_id` (the superseded generation's own run) and `superseded_by_run_id`
(the run that overwrote it) are indexed so a sweep can select a generation
without a table scan.
THE ONLY READER: `manage.py local_calculate_verdicts --generation-diff <path>`,
one JSONL line per vote this run superseded. Per the owner's ruling,
generation-diffing is an opt-in DEBUG FLAG, never a default write path - the
archive WRITE is unconditional (a paper trail that only exists when somebody
remembered to ask for it is not a paper trail); the READ is what is opt-in.
ORDER IS A CORRECTNESS CONSTRAINT, NOT A PERFORMANCE ONE
`fallback`, `illustration` and `slow-path` select POSITIVELY from join-key's
output. So "purge everything, then run the calculators in parallel" gives THREE
OF THE FOUR an empty eligible set - a silent near-no-op that reports success.
Required order stays join-key -> fallback -> illustration -> slow-path, which is
what both dispatchers already do.
This is also why the upstream populations are deliberately NOT run-scoped, and
that asymmetry is the whole correctness argument rather than an oversight: a
converged join-key pass writes NOTHING under a fresh run_id (an identical
recomputed verdict is skipped, and the stored row keeps its original run), so
"cards join-key voted no-match IN THIS RUN" is empty on every re-run. Slow-path's
fallback-voted exclusion must stay unscoped for the same reason and in a worse
direction: a run-scoped version would route a card fallback SOLVED in an earlier
run to a human reviewer.
CONSEQUENCE WORTH KNOWING: THE RETRACTION RUNBOOKS ARE PARTLY OBSOLETED
`reparse_collector_evidence` and `rejudge_fallback_channel`'s two-step runbook
existed partly because a stale scan-log row permanently locked a card out of its
calculator. It no longer does. Their retraction step is still needed to remove
the stale RECORD (and, for votes, to stop a stale row being counted by
consensus), but it is no longer what unlocks eligibility. Those tests now assert
the exclusion under the recorded row's OWN run_id and say why, rather than
quietly pinning the weaker claim.
A RESUMPTION GAP THAT IS ACCEPTED, NOT OVERLOOKED
A card whose verdict a run recomputes as UNCHANGED writes no row, so it carries
no marker for that run and a restarted run recomputes it. Resumption skips
completed WRITES, not completed recomputations. Closing it would mean stamping
the current run_id onto an existing row, destroying the provenance migration
0097's frozen cohort depends on being able to state. Not worth it.
VERIFICATION
- `cardpicker/tests/test_run_scoped_eligibility.py`: 29 new tests across
prior-run/current-run eligibility, the compiled-SQL trap, illustration's own
duplicated copy of the exclusion, changed-verdict overwrite + archiving,
archive-is-not-a-live-vote, superseding-run stamping, dependency ordering
(including the out-of-order empty-set failure, constructed on purpose), and
two- and three-pass convergence.
- 26 MUTATIONS applied one at a time, each run red, then restored: every
exclusion (vote and scan-log, in all four eligibility functions), every
calculator's forwarding of its run_id, the split's value comparison, the
split's skip-if-identical branch, the archive copy, `related_name="+"`,
`created_at` fidelity, `original_id`, `superseded_by_run_id`, and each of the
three upstream populations that must NOT be run-scoped. Three mutants came
back GREEN on the first pass and each was a genuine coverage gap rather than
a false alarm - the illustration calculator's run_id forwarding, and both
call sites of the evidence-transfer stamp - so tests were added until every
one went red.
- A pre-existing latent flake fixed on the way past:
`test_local_illustration.TestPrintingsForIllustration::
test_the_scope_reaches_the_compiled_sql` asserted `str(pk) not in sql`, which
is only true while the pk is a digit string appearing nowhere else - and the
query embeds `illustration_id` UUIDs verbatim, so a single-digit pk is a
substring of almost any of them. It passed or failed on where the sequence
happened to be, i.e. on which other test files ran first. Now asserted on the
predicate it actually means.
- Full `pytest cardpicker`: 3192 passed, 8 skipped. black, ruff, isort, mypy
clean; docs-lint clean; `makemigrations --check`: no changes, single leaf.
MIGRATION NUMBERING - 0100, AND A DEPENDENCY THAT DOES NOT EXIST YET
`0100_superseded_card_printing_tag_archive`, depending on
`0099_rename_printings_count_catalogued` (PR #601). THAT MIGRATION DOES NOT EXIST ANYWHERE YET -
not on master, and not yet on #601's own branch, which still carries the file under its original
name `0098_rename_printings_count_catalogued`. The name assumed here is #601's file renumbered
0098 -> 0099 with its slug unchanged, exactly the transformation #576 performed
(`0096_freeze_...` -> `0097_freeze_...`, slug preserved). IF #601 LANDS UNDER ANY OTHER NAME THIS
STRING MUST BE CORRECTED BEFORE MERGE, or `migrate` fails with NodeNotFoundError and no test
database can be built on any branch. THIS PR THEREFORE CANNOT MERGE BEFORE #601.
The chain 0098 (#573, merged) -> 0099 (#601) -> 0100 (this) is coordinator-assigned. There is no
substantive ordering constraint between the three - #601 renames a column on
`cardpicker_canonicalcard`, #573 added columns to `cardpicker_cardillustrationvote`, this creates a
new table - the chain exists solely to keep `cardpicker` at a SINGLE LEAF NODE.
HOW THIS WAS ORIGINALLY GOT WRONG, recorded so the reasoning is not repeated. It was first numbered
0098-on-0097 on the then-correct reasoning that #573 was still open and that depending on a
migration absent from master makes a branch unmigratable today, with certainty, to avoid a
collision that might never happen. #573 then MERGED, inverting the trade-off: the collision stopped
being hypothetical and became a fact on master - and one invisible to every normal signal, since
different filenames mean no textual conflict, GitHub still reported the PR mergeable, and CI stayed
green because it had run against the pre-#573 tree. Only `makemigrations --check` and the migration
loader's leaf count catch it.
VERIFIED AFTER THE RENUMBER: `makemigrations --check --dry-run` reports "No changes detected";
`MigrationLoader.graph.leaf_nodes()` returns exactly one cardpicker leaf,
`0100_superseded_card_printing_tag_archive`, with the forward plan ending
0097 -> 0098 -> 0099 -> 0100. Verified against a LOCAL STUB standing in for #601's migration (no
operations, so it cannot alter model state); the stub is not committed.
DOCS: TWO RUNBOOK CLAIMS THIS INVALIDATES, CORRECTED IN PLACE
- `docs/troubleshooting.md`'s "A reparse_collector_evidence/Stage D retraction pass silently
never routes its own newly-touched cards to slow-path review" is RESOLVED by this change, and
NOT by the fix it had spec'd. Run-scoping means a stale `stage-d-slow-path-v1` marker no longer
excludes anything from a new run. The spec'd fix - teach `reparse_and_retract` to also delete
the slow-path row - was deliberately NOT built: it would make every retraction command
responsible for knowing which downstream calculators had left markers, which is the coupling
that produced the symptom.
- `docs/features/stage-e-operations.md`'s `rejudge_fallback_channel` runbook described retraction
as "making those cards eligible for a fresh local_calculate_verdicts pass". That was the
mechanism and no longer is. Revised to say what retraction still buys: removing a stale RECORD,
which for a VOTE is load-bearing (an un-retracted stale vote keeps its consensus weight until
something overwrites it), while eligibility is now unlocked by every new run regardless.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN
…0098 #573 merged its own `0098_card_illustration_consensus_fields` first, also depending on `0097_freeze_deductive_backfill_zero_weight_cohort`. This branch's `0098_rename_printings_count_catalogued` depended on the same 0097, so the merge of the two would have given `cardpicker` TWO leaf nodes - and pytest-django builds its test database by running `migrate`, so the fork errors at test-database SETUP on every branch in the repo, not just this one. That is the outage #576 had to repair at 0096. Nothing normally trusted showed it. The filenames differ so there was no textual conflict; GitHub reported this PR MERGEABLE/CLEAN; its checks were 10/10 green because they had run against master BEFORE #573 landed, and GitHub does not re-run a PR's checks when its base moves. - rebase onto master (d044223) - `git mv` the migration to `0099_rename_printings_count_catalogued.py` and repoint `dependencies` at `0098_card_illustration_consensus_fields` - rewrite the migration's own MIGRATION-GRAPH NOTE, which stated the old number and predicted this collision, to record what actually happened - update the three other places that state the number in prose: `models.py`'s field comment, `deductive_backfill.py`'s module docstring, `docs/features/printing-tags.md` The operation is unchanged: still a single `RenameField` on `CanonicalPrintingMetadata.printings_count`, which PostgreSQL executes as `ALTER TABLE ... RENAME COLUMN` - catalogue metadata only, no table rewrite, no row read or written, fully reversible. Only the number and the dependency moved. Verified: `cardpicker` has exactly one leaf (`0099_rename_printings_count_catalogued`); `makemigrations --check --dry-run` reports no changes detected; `cardpicker/tests/` 3263 passed, 8 skipped; `docs_lint.py --strict` clean; pre-commit clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN
… superseded votes
"Prior runs must not suppress work in a new run. The CURRENT run's own
output must, so a killed run resumes rather than redoing completed batches."
- owner directive, 2026-07-29
"Keep at least one prior generation of votes, whose votes are NOT counted."
- ratified separately
WHY THESE THREE ARE ONE COMMIT AND NOT THREE
They are not independently reviewable, and shipping any one alone is worse than
shipping none:
- Un-suppressing eligibility ALONE buys exactly nothing. The calculator
recomputes the verdict and the pre-write split then drops it before the
write, and because `purge_and_write_votes` scopes its purge to the rows
being written, a dropped row purges NOTHING - the stale vote survives
verbatim, with no error, no counter moving, and a recomputation's worth of
work discarded. Two independent suppression layers, and the second silently
defeats a fix to the first.
- The archive has no effect until the split permits an overwrite, and the
owner ruled it must not ship separately from the work that creates
superseded rows in the first place.
The run_id AUDIT this depends on is a separate, genuinely independent PR
(`fix/run-id-population-gaps`), which this is stacked on.
LAYER 1 - RUN-SCOPED ELIGIBILITY
Every Stage D printing-channel calculator asked "have I EVER voted on / abstained
on this card?". That predicate grows monotonically, so each pass could only ever
see a subset of the previous pass's pool, and a repaired engine could never
re-examine anything the broken one had answered. Not hypothetical:
`stage-d-illustration` had to be version-bumped v1 -> v2 purely to escape its own
non-rescannable scan-log rows after its layout_class gate turned out to be
reading a border colour - 3,409 wrongly-skipped cards were otherwise unreachable
to a repaired v1. Under run-scoping the repair alone would have sufficed.
Both self-suppressing excludes - the printing-tag vote exclude and the
non-rescannable `CardScanLog` exclude - now additionally match the CURRENT
run_id, in `_eligible_cards_queryset` (join-key + fallback),
`_slow_path_eligible_cards_queryset`, `_eligible_illustration_cards_queryset`
and `_eligible_base_queryset` (opt-in there; only lands passes a run_id today -
`run_pilot`/`run_name_frequency_elimination` are a different workload with their
own fetch budgets and resume semantics, and flipping them is a separate decision
with a separate blast radius).
`run_id=None` keeps the pre-change behaviour BYTE-IDENTICALLY, deliberately and
not vestigially: `stream_backstop_sweep.verify_chunk` asks "is there ANY Stage D
backlog", a question about the catalogue rather than about a run, and answering
it run-scoped would report the whole catalogue as backlog on every fresh run_id.
A BUG THIS ALMOST SHIPPED WITH, FOUND BY COMPILING THE SQL RATHER THAN REASONING
ABOUT IT. The obvious spelling is
`.exclude(printing_tags__anonymous_id=X, printing_tags__run_id=Y)`. Django does
NOT combine those into one subquery the same related row must satisfy - it emits
`NOT (EXISTS(... anonymous_id=X ...) AND EXISTS(... run_id=Y ...))`, two
INDEPENDENT clauses. A card carrying THIS identity's vote from an OLD run plus
some OTHER identity's vote from THIS run satisfies both halves and is wrongly
excluded, re-creating the exact cross-run suppression this work removes, in the
hardest direction to notice: fewer cards processed, no error, no counter. It is
the same negated-multi-valued-lookup trap `_eligible_base_queryset`'s own
docstring already documented for the scan-log exclusion. The scoped path uses an
explicit `pk__in` subquery instead; `TestTheCompiledSqlTrap` pins both the SQL
shape and the behaviour.
LAYER 2 - THE SPLIT COMPARES THE VALUE, NOT JUST THE KEY
`_split_new_printing_tag_votes` compared `(card_id, anonymous_id)` alone. It now
compares the whole SET of `(printing_id, is_no_match)` a batch proposes for a
group against what is stored - the shape
`local_illustration._split_new_illustration_votes` has always had, whose own
docstring calls it "THE ONE DIFFERENCE, AND IT IS LOAD-BEARING".
- existing rows, SAME verdict -> skipped, counted in `already_voted`.
Re-running over a converged catalogue stays a no-op, which is what stops
run-scoping becoming an overwrite-everything churn machine.
- existing rows, DIFFERENT verdict -> kept. The purge moves the stale
generation into the archive and the new one lands.
- no existing row -> kept.
Compared PER GROUP, ALL-OR-NOTHING, because one identity can legally hold
several rows for a card (`cardprintingtag_unique_printing_vote` constrains the
triple, not the pair - `run_illustration_calculator`'s loop over
`verdict.printing_pks` is a live caller shaped that way) and the purge is
family-keyed on `card_id`. Keeping only PART of a group would delete the rest
and never re-insert them.
The "skip-and-count, not retract-and-recast" reasoning is untouched: it was
always conditioned on both racing invocations computing the SAME verdict, which
is exactly the case still skipped. What no longer gets swallowed is the case
that reasoning already named as NOT covered - a genuinely changed conclusion.
`local_lands_identify._split_new_votes` needed no change: it already compares the
full (card, printing, anonymous_id) triple, so a changed answer already reached
its purge. A test now pins that rather than leaving it a coincidence.
LAYER 3 - `ArchivedCardPrintingTag`
`purge_stale_machine_votes` copies every row it is about to delete into the
archive first, stamped with the run that overwrote it. That function is THE choke
point for "a machine vote is superseded by a later machine vote", so no caller
can supersede without archiving and no new caller has to remember to.
`vote_write.purge_and_write_votes` supplies `superseded_by_run_id`, derived from
the batch it is writing, and its existing `transaction.atomic()` now covers three
statements instead of two - the same cancel-safety property, one statement wider.
A batch that cannot name a single run_id records NULL rather than a guess: a
wrong stamp is worse than a missing one, since the diff report and issue #575's
janitor both select on it and a plausible wrong value is indistinguishable from a
right one after the fact.
WHY A SEPARATE TABLE AND NOT RETAINED GENERATIONS IN THE LIVE ONE - MEASURED, NOT
PREFERRED. Of the thirteen modules that read `CardPrintingTag`, NINE bypass
`vote_consensus.resolve_vote_weight` entirely: `views.py`, `catalog_stats.py`,
`local_calculate_verdicts.py`, `models.py`, `local_identify_printing_tags.py`,
`soak_gate.py`, `harvest_probe.py`, `illustration_vote.py`,
`local_lands_identify.py`. A zero-weight-by-run_id rule (migration 0097's
pattern) protects only the four that route through weight resolution. A retained
generation left in the live table would still be DISPLAYED by views, COUNTED by
catalog-stats, and - fatally - would make eligibility treat the card as already
voted, re-creating the very suppression this work removes. Keeping the live table
single-generation means no consumer can be wrong about it: no unique-constraint
change, no audit of thirteen modules, no new rule any future reader has to know.
Rows are unreachable from `Card`/`CanonicalCard` (`related_name="+"` on both
FKs), are not an `AbstractWeightedVote` subclass, and copy `created_at` verbatim
rather than re-stamping it, with `archived_at` as the separate honest answer to
"when did this stop being live". Append-only, no unique constraints - the same
shape `CardScanLog` already has. Human votes never reach it: `calculator_family`
returns None for the UUIDs humans use and the purge returns before touching
anything.
Retention is issue #575's janitor's ("keep the N most recent runs per calculator,
sweep the oldest, operator-authorised with a dry run, never delete wholesale").
Both `run_id` (the superseded generation's own run) and `superseded_by_run_id`
(the run that overwrote it) are indexed so a sweep can select a generation
without a table scan.
THE ONLY READER: `manage.py local_calculate_verdicts --generation-diff <path>`,
one JSONL line per vote this run superseded. Per the owner's ruling,
generation-diffing is an opt-in DEBUG FLAG, never a default write path - the
archive WRITE is unconditional (a paper trail that only exists when somebody
remembered to ask for it is not a paper trail); the READ is what is opt-in.
ORDER IS A CORRECTNESS CONSTRAINT, NOT A PERFORMANCE ONE
`fallback`, `illustration` and `slow-path` select POSITIVELY from join-key's
output. So "purge everything, then run the calculators in parallel" gives THREE
OF THE FOUR an empty eligible set - a silent near-no-op that reports success.
Required order stays join-key -> fallback -> illustration -> slow-path, which is
what both dispatchers already do.
This is also why the upstream populations are deliberately NOT run-scoped, and
that asymmetry is the whole correctness argument rather than an oversight: a
converged join-key pass writes NOTHING under a fresh run_id (an identical
recomputed verdict is skipped, and the stored row keeps its original run), so
"cards join-key voted no-match IN THIS RUN" is empty on every re-run. Slow-path's
fallback-voted exclusion must stay unscoped for the same reason and in a worse
direction: a run-scoped version would route a card fallback SOLVED in an earlier
run to a human reviewer.
CONSEQUENCE WORTH KNOWING: THE RETRACTION RUNBOOKS ARE PARTLY OBSOLETED
`reparse_collector_evidence` and `rejudge_fallback_channel`'s two-step runbook
existed partly because a stale scan-log row permanently locked a card out of its
calculator. It no longer does. Their retraction step is still needed to remove
the stale RECORD (and, for votes, to stop a stale row being counted by
consensus), but it is no longer what unlocks eligibility. Those tests now assert
the exclusion under the recorded row's OWN run_id and say why, rather than
quietly pinning the weaker claim.
A RESUMPTION GAP THAT IS ACCEPTED, NOT OVERLOOKED
A card whose verdict a run recomputes as UNCHANGED writes no row, so it carries
no marker for that run and a restarted run recomputes it. Resumption skips
completed WRITES, not completed recomputations. Closing it would mean stamping
the current run_id onto an existing row, destroying the provenance migration
0097's frozen cohort depends on being able to state. Not worth it.
VERIFICATION
- `cardpicker/tests/test_run_scoped_eligibility.py`: 29 new tests across
prior-run/current-run eligibility, the compiled-SQL trap, illustration's own
duplicated copy of the exclusion, changed-verdict overwrite + archiving,
archive-is-not-a-live-vote, superseding-run stamping, dependency ordering
(including the out-of-order empty-set failure, constructed on purpose), and
two- and three-pass convergence.
- 26 MUTATIONS applied one at a time, each run red, then restored: every
exclusion (vote and scan-log, in all four eligibility functions), every
calculator's forwarding of its run_id, the split's value comparison, the
split's skip-if-identical branch, the archive copy, `related_name="+"`,
`created_at` fidelity, `original_id`, `superseded_by_run_id`, and each of the
three upstream populations that must NOT be run-scoped. Three mutants came
back GREEN on the first pass and each was a genuine coverage gap rather than
a false alarm - the illustration calculator's run_id forwarding, and both
call sites of the evidence-transfer stamp - so tests were added until every
one went red.
- A pre-existing latent flake fixed on the way past:
`test_local_illustration.TestPrintingsForIllustration::
test_the_scope_reaches_the_compiled_sql` asserted `str(pk) not in sql`, which
is only true while the pk is a digit string appearing nowhere else - and the
query embeds `illustration_id` UUIDs verbatim, so a single-digit pk is a
substring of almost any of them. It passed or failed on where the sequence
happened to be, i.e. on which other test files ran first. Now asserted on the
predicate it actually means.
- Full `pytest cardpicker`: 3192 passed, 8 skipped. black, ruff, isort, mypy
clean; docs-lint clean; `makemigrations --check`: no changes, single leaf.
MIGRATION NUMBERING - 0100, AND A DEPENDENCY THAT DOES NOT EXIST YET
`0100_superseded_card_printing_tag_archive`, depending on
`0099_rename_printings_count_catalogued` (PR #601). THAT MIGRATION DOES NOT EXIST ANYWHERE YET -
not on master, and not yet on #601's own branch, which still carries the file under its original
name `0098_rename_printings_count_catalogued`. The name assumed here is #601's file renumbered
0098 -> 0099 with its slug unchanged, exactly the transformation #576 performed
(`0096_freeze_...` -> `0097_freeze_...`, slug preserved). IF #601 LANDS UNDER ANY OTHER NAME THIS
STRING MUST BE CORRECTED BEFORE MERGE, or `migrate` fails with NodeNotFoundError and no test
database can be built on any branch. THIS PR THEREFORE CANNOT MERGE BEFORE #601.
The chain 0098 (#573, merged) -> 0099 (#601) -> 0100 (this) is coordinator-assigned. There is no
substantive ordering constraint between the three - #601 renames a column on
`cardpicker_canonicalcard`, #573 added columns to `cardpicker_cardillustrationvote`, this creates a
new table - the chain exists solely to keep `cardpicker` at a SINGLE LEAF NODE.
HOW THIS WAS ORIGINALLY GOT WRONG, recorded so the reasoning is not repeated. It was first numbered
0098-on-0097 on the then-correct reasoning that #573 was still open and that depending on a
migration absent from master makes a branch unmigratable today, with certainty, to avoid a
collision that might never happen. #573 then MERGED, inverting the trade-off: the collision stopped
being hypothetical and became a fact on master - and one invisible to every normal signal, since
different filenames mean no textual conflict, GitHub still reported the PR mergeable, and CI stayed
green because it had run against the pre-#573 tree. Only `makemigrations --check` and the migration
loader's leaf count catch it.
VERIFIED AFTER THE RENUMBER: `makemigrations --check --dry-run` reports "No changes detected";
`MigrationLoader.graph.leaf_nodes()` returns exactly one cardpicker leaf,
`0100_superseded_card_printing_tag_archive`, with the forward plan ending
0097 -> 0098 -> 0099 -> 0100. Verified against a LOCAL STUB standing in for #601's migration (no
operations, so it cannot alter model state); the stub is not committed.
DOCS: TWO RUNBOOK CLAIMS THIS INVALIDATES, CORRECTED IN PLACE
- `docs/troubleshooting.md`'s "A reparse_collector_evidence/Stage D retraction pass silently
never routes its own newly-touched cards to slow-path review" is RESOLVED by this change, and
NOT by the fix it had spec'd. Run-scoping means a stale `stage-d-slow-path-v1` marker no longer
excludes anything from a new run. The spec'd fix - teach `reparse_and_retract` to also delete
the slow-path row - was deliberately NOT built: it would make every retraction command
responsible for knowing which downstream calculators had left markers, which is the coupling
that produced the symptom.
- `docs/features/stage-e-operations.md`'s `rejudge_fallback_channel` runbook described retraction
as "making those cards eligible for a fresh local_calculate_verdicts pass". That was the
mechanism and no longer is. Revised to say what retraction still buys: removing a stale RECORD,
which for a VOTE is load-bearing (an un-retracted stale vote keeps its consensus weight until
something overwrites it), while eligibility is now unlocked by every new run regardless.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN
…-verified against Scryfall" claim (#601) * Rename printings_count -> catalogued_printings_count; delete the false "cross-verified against Scryfall" claim `CanonicalPrintingMetadata.printings_count` sat on a model whose docstring said "Scryfall printing-level fields", and the docs read it that way. It is not Scryfall data. `import_scryfall_printing_metadata` builds a Counter over `CanonicalCard.canonical_id` — our own table — and stores each row's oracle group size. Rows with a NULL canonical_id are stored as 1 by fiat. That difference is load-bearing. A column derived from our catalogue cannot detect that our catalogue is incomplete, which is exactly what deductive backfill's first tier advertised it as doing ("cross-verified against Scryfall's own printings_count (not just 'our table happens to have one row')" — it was precisely the latter). Measured against the live catalogue, 2026-07-29: - 14,893 normalised names have exactly one CanonicalCard row. All 14,893 carry a count of 1. Zero carry >1. Zero are NULL. The tier's second condition is entailed by the name-uniqueness test one line above it. - 137 cards reach that condition out of an eligible pool of 104,969; 137 pass. The gate excludes nothing and never could. - Counting the Scryfall bulk file directly finds 2 oracle ids where we hold one row and Scryfall publishes more — the exact case the gate claimed to catch, invisible to it by construction. The condition is left in the code, labelled as entailed rather than deleted, so the gap stays visible; issue #592 tracks what a real external check is. Per the "do not make false assertions" directive the claim is deleted, not softened: the tier's documented claim is now the one it can support — the name matches exactly one row in our catalogue. Also corrects the derived claim in local_identify_printing_tags, which said its selection "revisits single-candidate names deductive backfill's Scryfall printings_count cross-check rejected". That cohort is empty and always was. Migration 0098 is a pure RenameField (ALTER TABLE ... RENAME COLUMN — no table rewrite, no data touched). NOTE: PR #573 also adds an 0098; whichever merges second must renumber to 0099. * Renumber 0098_rename_printings_count_catalogued -> 0099, onto #573's 0098 #573 merged its own `0098_card_illustration_consensus_fields` first, also depending on `0097_freeze_deductive_backfill_zero_weight_cohort`. This branch's `0098_rename_printings_count_catalogued` depended on the same 0097, so the merge of the two would have given `cardpicker` TWO leaf nodes - and pytest-django builds its test database by running `migrate`, so the fork errors at test-database SETUP on every branch in the repo, not just this one. That is the outage #576 had to repair at 0096. Nothing normally trusted showed it. The filenames differ so there was no textual conflict; GitHub reported this PR MERGEABLE/CLEAN; its checks were 10/10 green because they had run against master BEFORE #573 landed, and GitHub does not re-run a PR's checks when its base moves. - rebase onto master (d044223) - `git mv` the migration to `0099_rename_printings_count_catalogued.py` and repoint `dependencies` at `0098_card_illustration_consensus_fields` - rewrite the migration's own MIGRATION-GRAPH NOTE, which stated the old number and predicted this collision, to record what actually happened - update the three other places that state the number in prose: `models.py`'s field comment, `deductive_backfill.py`'s module docstring, `docs/features/printing-tags.md` The operation is unchanged: still a single `RenameField` on `CanonicalPrintingMetadata.printings_count`, which PostgreSQL executes as `ALTER TABLE ... RENAME COLUMN` - catalogue metadata only, no table rewrite, no row read or written, fully reversible. Only the number and the dependency moved. Verified: `cardpicker` has exactly one leaf (`0099_rename_printings_count_catalogued`); `makemigrations --check --dry-run` reports no changes detected; `cardpicker/tests/` 3263 passed, 8 skipped; `docs_lint.py --strict` clean; pre-commit clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…branch (#611) Two branches can each add `0098_<something>.py` depending on `0097`. The filenames differ, so there is no textual conflict, GitHub reports the second PR MERGEABLE/CLEAN, and both branches are individually valid. The moment the second merges, `cardpicker` has two leaf nodes - and pytest-django builds its test database by running `migrate`, so the fork fails at test-database SETUP on EVERY branch in the repo, not just the one that introduced it. This has now happened twice: at 0096 (#568 vs #570) and at 0098 (#573 vs #601). #576 repaired the first fork but prevented nothing, which is why the second arrived within days. Nothing in CI failed either time. WHY THE MERGE RESULT IS THE WHOLE POINT #601's checks were 10/10 green with the collision already live on master - they had run against master BEFORE #573 landed, and GitHub does not re-run a PR's checks when its base moves. A check reading only the PR branch's files sees one leaf and passes; the fork exists only in the merge. So `check_migration_leaves.py --base origin/<base_ref>` unions the worktree's migrations with the base branch's CURRENT tip, resolved at run time, honouring anything the PR deletes (`--no-renames` is load-bearing: a renumber is a delete+add of near-identical content and git otherwise reports it as a rename, which would resurrect the old number and fail a PR that had already fixed itself). HOW IT DECIDES Static `ast` read of every `migrations/` package: filenames are nodes, each file's `dependencies` gives same-app edges, `run_before` gives reversed ones, and a squash's `replaces` removes the nodes it stands in for. Migration modules are never imported or executed, so this needs no settings module, no installed apps, no postgres and no `requirements.txt` - it runs on a bare `actions/setup-python` in about a second. Non-literal dependency entries (`migrations.swappable_dependency(settings.AUTH_USER_MODEL)`, in seven of this repo's migrations) are cross-app by construction and are skipped, not guessed at. Exit code is the finding count, matching docs_lint.py's and check_protected_core_license.py's convention. Findings: more than one leaf per app (the failure), a duplicate NNNN number prefix within an app (the same defect one step earlier, and the actionable instruction), and a dependency naming a migration that does not exist. WHAT IT CANNOT DO, STATED PLAINLY A check run that PASSED before the base moved stays green in GitHub's UI. No CI job can fix that from the inside; branch protection's "Require branches to be up to date before merging" is the setting that closes it, and this makes the forced re-run meaningful. `merge_group` is wired up so a merge queue would close it too. The workflow is its own file rather than another entry in docs-lint.yml, which four open PRs are already editing. Every path glob uses `**`: a single `*` does not match a slash, so `MPCAutofill/cardpicker/*.py` would miss `migrations/` entirely (#588 hit exactly that). Demonstrated red-then-green against the real collision, and kept as permanent regression coverage in `.github/scripts/tests/test_check_migration_leaves.py` (15 tests) rather than as a one-off local run: a scratch repo where master has `0098_card_illustration_consensus_fields` and a feature branch has `0098_rename_printings_count_catalogued`, both on 0097, asserts clean on the branch alone, two leaves against the merge, and clean again once renumbered to 0099 - plus no-finding cases for a normal single-migration PR, a PR touching no migrations, cross-app dependencies, swappable dependencies, squashes and this repo's own tree. Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
… superseded votes
"Prior runs must not suppress work in a new run. The CURRENT run's own
output must, so a killed run resumes rather than redoing completed batches."
- owner directive, 2026-07-29
"Keep at least one prior generation of votes, whose votes are NOT counted."
- ratified separately
WHY THESE THREE ARE ONE COMMIT AND NOT THREE
They are not independently reviewable, and shipping any one alone is worse than
shipping none:
- Un-suppressing eligibility ALONE buys exactly nothing. The calculator
recomputes the verdict and the pre-write split then drops it before the
write, and because `purge_and_write_votes` scopes its purge to the rows
being written, a dropped row purges NOTHING - the stale vote survives
verbatim, with no error, no counter moving, and a recomputation's worth of
work discarded. Two independent suppression layers, and the second silently
defeats a fix to the first.
- The archive has no effect until the split permits an overwrite, and the
owner ruled it must not ship separately from the work that creates
superseded rows in the first place.
The run_id AUDIT this depends on is a separate, genuinely independent PR
(`fix/run-id-population-gaps`), which this is stacked on.
LAYER 1 - RUN-SCOPED ELIGIBILITY
Every Stage D printing-channel calculator asked "have I EVER voted on / abstained
on this card?". That predicate grows monotonically, so each pass could only ever
see a subset of the previous pass's pool, and a repaired engine could never
re-examine anything the broken one had answered. Not hypothetical:
`stage-d-illustration` had to be version-bumped v1 -> v2 purely to escape its own
non-rescannable scan-log rows after its layout_class gate turned out to be
reading a border colour - 3,409 wrongly-skipped cards were otherwise unreachable
to a repaired v1. Under run-scoping the repair alone would have sufficed.
Both self-suppressing excludes - the printing-tag vote exclude and the
non-rescannable `CardScanLog` exclude - now additionally match the CURRENT
run_id, in `_eligible_cards_queryset` (join-key + fallback),
`_slow_path_eligible_cards_queryset`, `_eligible_illustration_cards_queryset`
and `_eligible_base_queryset` (opt-in there; only lands passes a run_id today -
`run_pilot`/`run_name_frequency_elimination` are a different workload with their
own fetch budgets and resume semantics, and flipping them is a separate decision
with a separate blast radius).
`run_id=None` keeps the pre-change behaviour BYTE-IDENTICALLY, deliberately and
not vestigially: `stream_backstop_sweep.verify_chunk` asks "is there ANY Stage D
backlog", a question about the catalogue rather than about a run, and answering
it run-scoped would report the whole catalogue as backlog on every fresh run_id.
A BUG THIS ALMOST SHIPPED WITH, FOUND BY COMPILING THE SQL RATHER THAN REASONING
ABOUT IT. The obvious spelling is
`.exclude(printing_tags__anonymous_id=X, printing_tags__run_id=Y)`. Django does
NOT combine those into one subquery the same related row must satisfy - it emits
`NOT (EXISTS(... anonymous_id=X ...) AND EXISTS(... run_id=Y ...))`, two
INDEPENDENT clauses. A card carrying THIS identity's vote from an OLD run plus
some OTHER identity's vote from THIS run satisfies both halves and is wrongly
excluded, re-creating the exact cross-run suppression this work removes, in the
hardest direction to notice: fewer cards processed, no error, no counter. It is
the same negated-multi-valued-lookup trap `_eligible_base_queryset`'s own
docstring already documented for the scan-log exclusion. The scoped path uses an
explicit `pk__in` subquery instead; `TestTheCompiledSqlTrap` pins both the SQL
shape and the behaviour.
LAYER 2 - THE SPLIT COMPARES THE VALUE, NOT JUST THE KEY
`_split_new_printing_tag_votes` compared `(card_id, anonymous_id)` alone. It now
compares the whole SET of `(printing_id, is_no_match)` a batch proposes for a
group against what is stored - the shape
`local_illustration._split_new_illustration_votes` has always had, whose own
docstring calls it "THE ONE DIFFERENCE, AND IT IS LOAD-BEARING".
- existing rows, SAME verdict -> skipped, counted in `already_voted`.
Re-running over a converged catalogue stays a no-op, which is what stops
run-scoping becoming an overwrite-everything churn machine.
- existing rows, DIFFERENT verdict -> kept. The purge moves the stale
generation into the archive and the new one lands.
- no existing row -> kept.
Compared PER GROUP, ALL-OR-NOTHING, because one identity can legally hold
several rows for a card (`cardprintingtag_unique_printing_vote` constrains the
triple, not the pair - `run_illustration_calculator`'s loop over
`verdict.printing_pks` is a live caller shaped that way) and the purge is
family-keyed on `card_id`. Keeping only PART of a group would delete the rest
and never re-insert them.
The "skip-and-count, not retract-and-recast" reasoning is untouched: it was
always conditioned on both racing invocations computing the SAME verdict, which
is exactly the case still skipped. What no longer gets swallowed is the case
that reasoning already named as NOT covered - a genuinely changed conclusion.
`local_lands_identify._split_new_votes` needed no change: it already compares the
full (card, printing, anonymous_id) triple, so a changed answer already reached
its purge. A test now pins that rather than leaving it a coincidence.
LAYER 3 - `ArchivedCardPrintingTag`
`purge_stale_machine_votes` copies every row it is about to delete into the
archive first, stamped with the run that overwrote it. That function is THE choke
point for "a machine vote is superseded by a later machine vote", so no caller
can supersede without archiving and no new caller has to remember to.
`vote_write.purge_and_write_votes` supplies `superseded_by_run_id`, derived from
the batch it is writing, and its existing `transaction.atomic()` now covers three
statements instead of two - the same cancel-safety property, one statement wider.
A batch that cannot name a single run_id records NULL rather than a guess: a
wrong stamp is worse than a missing one, since the diff report and issue #575's
janitor both select on it and a plausible wrong value is indistinguishable from a
right one after the fact.
WHY A SEPARATE TABLE AND NOT RETAINED GENERATIONS IN THE LIVE ONE - MEASURED, NOT
PREFERRED. Of the thirteen modules that read `CardPrintingTag`, NINE bypass
`vote_consensus.resolve_vote_weight` entirely: `views.py`, `catalog_stats.py`,
`local_calculate_verdicts.py`, `models.py`, `local_identify_printing_tags.py`,
`soak_gate.py`, `harvest_probe.py`, `illustration_vote.py`,
`local_lands_identify.py`. A zero-weight-by-run_id rule (migration 0097's
pattern) protects only the four that route through weight resolution. A retained
generation left in the live table would still be DISPLAYED by views, COUNTED by
catalog-stats, and - fatally - would make eligibility treat the card as already
voted, re-creating the very suppression this work removes. Keeping the live table
single-generation means no consumer can be wrong about it: no unique-constraint
change, no audit of thirteen modules, no new rule any future reader has to know.
Rows are unreachable from `Card`/`CanonicalCard` (`related_name="+"` on both
FKs), are not an `AbstractWeightedVote` subclass, and copy `created_at` verbatim
rather than re-stamping it, with `archived_at` as the separate honest answer to
"when did this stop being live". Append-only, no unique constraints - the same
shape `CardScanLog` already has. Human votes never reach it: `calculator_family`
returns None for the UUIDs humans use and the purge returns before touching
anything.
Retention is issue #575's janitor's ("keep the N most recent runs per calculator,
sweep the oldest, operator-authorised with a dry run, never delete wholesale").
Both `run_id` (the superseded generation's own run) and `superseded_by_run_id`
(the run that overwrote it) are indexed so a sweep can select a generation
without a table scan.
THE ONLY READER: `manage.py local_calculate_verdicts --generation-diff <path>`,
one JSONL line per vote this run superseded. Per the owner's ruling,
generation-diffing is an opt-in DEBUG FLAG, never a default write path - the
archive WRITE is unconditional (a paper trail that only exists when somebody
remembered to ask for it is not a paper trail); the READ is what is opt-in.
ORDER IS A CORRECTNESS CONSTRAINT, NOT A PERFORMANCE ONE
`fallback`, `illustration` and `slow-path` select POSITIVELY from join-key's
output. So "purge everything, then run the calculators in parallel" gives THREE
OF THE FOUR an empty eligible set - a silent near-no-op that reports success.
Required order stays join-key -> fallback -> illustration -> slow-path, which is
what both dispatchers already do.
This is also why the upstream populations are deliberately NOT run-scoped, and
that asymmetry is the whole correctness argument rather than an oversight: a
converged join-key pass writes NOTHING under a fresh run_id (an identical
recomputed verdict is skipped, and the stored row keeps its original run), so
"cards join-key voted no-match IN THIS RUN" is empty on every re-run. Slow-path's
fallback-voted exclusion must stay unscoped for the same reason and in a worse
direction: a run-scoped version would route a card fallback SOLVED in an earlier
run to a human reviewer.
CONSEQUENCE WORTH KNOWING: THE RETRACTION RUNBOOKS ARE PARTLY OBSOLETED
`reparse_collector_evidence` and `rejudge_fallback_channel`'s two-step runbook
existed partly because a stale scan-log row permanently locked a card out of its
calculator. It no longer does. Their retraction step is still needed to remove
the stale RECORD (and, for votes, to stop a stale row being counted by
consensus), but it is no longer what unlocks eligibility. Those tests now assert
the exclusion under the recorded row's OWN run_id and say why, rather than
quietly pinning the weaker claim.
A RESUMPTION GAP THAT IS ACCEPTED, NOT OVERLOOKED
A card whose verdict a run recomputes as UNCHANGED writes no row, so it carries
no marker for that run and a restarted run recomputes it. Resumption skips
completed WRITES, not completed recomputations. Closing it would mean stamping
the current run_id onto an existing row, destroying the provenance migration
0097's frozen cohort depends on being able to state. Not worth it.
VERIFICATION
- `cardpicker/tests/test_run_scoped_eligibility.py`: 29 new tests across
prior-run/current-run eligibility, the compiled-SQL trap, illustration's own
duplicated copy of the exclusion, changed-verdict overwrite + archiving,
archive-is-not-a-live-vote, superseding-run stamping, dependency ordering
(including the out-of-order empty-set failure, constructed on purpose), and
two- and three-pass convergence.
- 26 MUTATIONS applied one at a time, each run red, then restored: every
exclusion (vote and scan-log, in all four eligibility functions), every
calculator's forwarding of its run_id, the split's value comparison, the
split's skip-if-identical branch, the archive copy, `related_name="+"`,
`created_at` fidelity, `original_id`, `superseded_by_run_id`, and each of the
three upstream populations that must NOT be run-scoped. Three mutants came
back GREEN on the first pass and each was a genuine coverage gap rather than
a false alarm - the illustration calculator's run_id forwarding, and both
call sites of the evidence-transfer stamp - so tests were added until every
one went red.
- A pre-existing latent flake fixed on the way past:
`test_local_illustration.TestPrintingsForIllustration::
test_the_scope_reaches_the_compiled_sql` asserted `str(pk) not in sql`, which
is only true while the pk is a digit string appearing nowhere else - and the
query embeds `illustration_id` UUIDs verbatim, so a single-digit pk is a
substring of almost any of them. It passed or failed on where the sequence
happened to be, i.e. on which other test files ran first. Now asserted on the
predicate it actually means.
- Full `pytest cardpicker`: 3192 passed, 8 skipped. black, ruff, isort, mypy
clean; docs-lint clean; `makemigrations --check`: no changes, single leaf.
MIGRATION NUMBERING - 0100, AND A DEPENDENCY THAT DOES NOT EXIST YET
`0100_superseded_card_printing_tag_archive`, depending on
`0099_rename_printings_count_catalogued` (PR #601). THAT MIGRATION DOES NOT EXIST ANYWHERE YET -
not on master, and not yet on #601's own branch, which still carries the file under its original
name `0098_rename_printings_count_catalogued`. The name assumed here is #601's file renumbered
0098 -> 0099 with its slug unchanged, exactly the transformation #576 performed
(`0096_freeze_...` -> `0097_freeze_...`, slug preserved). IF #601 LANDS UNDER ANY OTHER NAME THIS
STRING MUST BE CORRECTED BEFORE MERGE, or `migrate` fails with NodeNotFoundError and no test
database can be built on any branch. THIS PR THEREFORE CANNOT MERGE BEFORE #601.
The chain 0098 (#573, merged) -> 0099 (#601) -> 0100 (this) is coordinator-assigned. There is no
substantive ordering constraint between the three - #601 renames a column on
`cardpicker_canonicalcard`, #573 added columns to `cardpicker_cardillustrationvote`, this creates a
new table - the chain exists solely to keep `cardpicker` at a SINGLE LEAF NODE.
HOW THIS WAS ORIGINALLY GOT WRONG, recorded so the reasoning is not repeated. It was first numbered
0098-on-0097 on the then-correct reasoning that #573 was still open and that depending on a
migration absent from master makes a branch unmigratable today, with certainty, to avoid a
collision that might never happen. #573 then MERGED, inverting the trade-off: the collision stopped
being hypothetical and became a fact on master - and one invisible to every normal signal, since
different filenames mean no textual conflict, GitHub still reported the PR mergeable, and CI stayed
green because it had run against the pre-#573 tree. Only `makemigrations --check` and the migration
loader's leaf count catch it.
VERIFIED AFTER THE RENUMBER: `makemigrations --check --dry-run` reports "No changes detected";
`MigrationLoader.graph.leaf_nodes()` returns exactly one cardpicker leaf,
`0100_superseded_card_printing_tag_archive`, with the forward plan ending
0097 -> 0098 -> 0099 -> 0100. Verified against a LOCAL STUB standing in for #601's migration (no
operations, so it cannot alter model state); the stub is not committed.
DOCS: TWO RUNBOOK CLAIMS THIS INVALIDATES, CORRECTED IN PLACE
- `docs/troubleshooting.md`'s "A reparse_collector_evidence/Stage D retraction pass silently
never routes its own newly-touched cards to slow-path review" is RESOLVED by this change, and
NOT by the fix it had spec'd. Run-scoping means a stale `stage-d-slow-path-v1` marker no longer
excludes anything from a new run. The spec'd fix - teach `reparse_and_retract` to also delete
the slow-path row - was deliberately NOT built: it would make every retraction command
responsible for knowing which downstream calculators had left markers, which is the coupling
that produced the symptom.
- `docs/features/stage-e-operations.md`'s `rejudge_fallback_channel` runbook described retraction
as "making those cards eligible for a fresh local_calculate_verdicts pass". That was the
mechanism and no longer is. Revised to say what retraction still buys: removing a stale RECORD,
which for a VOTE is load-bearing (an un-retracted stale vote keeps its consensus weight until
something overwrites it), while eligibility is now unlocked by every new run regardless.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN
… superseded votes (#604) * Run-scoped eligibility, the value-comparing split, and an archive for superseded votes "Prior runs must not suppress work in a new run. The CURRENT run's own output must, so a killed run resumes rather than redoing completed batches." - owner directive, 2026-07-29 "Keep at least one prior generation of votes, whose votes are NOT counted." - ratified separately WHY THESE THREE ARE ONE COMMIT AND NOT THREE They are not independently reviewable, and shipping any one alone is worse than shipping none: - Un-suppressing eligibility ALONE buys exactly nothing. The calculator recomputes the verdict and the pre-write split then drops it before the write, and because `purge_and_write_votes` scopes its purge to the rows being written, a dropped row purges NOTHING - the stale vote survives verbatim, with no error, no counter moving, and a recomputation's worth of work discarded. Two independent suppression layers, and the second silently defeats a fix to the first. - The archive has no effect until the split permits an overwrite, and the owner ruled it must not ship separately from the work that creates superseded rows in the first place. The run_id AUDIT this depends on is a separate, genuinely independent PR (`fix/run-id-population-gaps`), which this is stacked on. LAYER 1 - RUN-SCOPED ELIGIBILITY Every Stage D printing-channel calculator asked "have I EVER voted on / abstained on this card?". That predicate grows monotonically, so each pass could only ever see a subset of the previous pass's pool, and a repaired engine could never re-examine anything the broken one had answered. Not hypothetical: `stage-d-illustration` had to be version-bumped v1 -> v2 purely to escape its own non-rescannable scan-log rows after its layout_class gate turned out to be reading a border colour - 3,409 wrongly-skipped cards were otherwise unreachable to a repaired v1. Under run-scoping the repair alone would have sufficed. Both self-suppressing excludes - the printing-tag vote exclude and the non-rescannable `CardScanLog` exclude - now additionally match the CURRENT run_id, in `_eligible_cards_queryset` (join-key + fallback), `_slow_path_eligible_cards_queryset`, `_eligible_illustration_cards_queryset` and `_eligible_base_queryset` (opt-in there; only lands passes a run_id today - `run_pilot`/`run_name_frequency_elimination` are a different workload with their own fetch budgets and resume semantics, and flipping them is a separate decision with a separate blast radius). `run_id=None` keeps the pre-change behaviour BYTE-IDENTICALLY, deliberately and not vestigially: `stream_backstop_sweep.verify_chunk` asks "is there ANY Stage D backlog", a question about the catalogue rather than about a run, and answering it run-scoped would report the whole catalogue as backlog on every fresh run_id. A BUG THIS ALMOST SHIPPED WITH, FOUND BY COMPILING THE SQL RATHER THAN REASONING ABOUT IT. The obvious spelling is `.exclude(printing_tags__anonymous_id=X, printing_tags__run_id=Y)`. Django does NOT combine those into one subquery the same related row must satisfy - it emits `NOT (EXISTS(... anonymous_id=X ...) AND EXISTS(... run_id=Y ...))`, two INDEPENDENT clauses. A card carrying THIS identity's vote from an OLD run plus some OTHER identity's vote from THIS run satisfies both halves and is wrongly excluded, re-creating the exact cross-run suppression this work removes, in the hardest direction to notice: fewer cards processed, no error, no counter. It is the same negated-multi-valued-lookup trap `_eligible_base_queryset`'s own docstring already documented for the scan-log exclusion. The scoped path uses an explicit `pk__in` subquery instead; `TestTheCompiledSqlTrap` pins both the SQL shape and the behaviour. LAYER 2 - THE SPLIT COMPARES THE VALUE, NOT JUST THE KEY `_split_new_printing_tag_votes` compared `(card_id, anonymous_id)` alone. It now compares the whole SET of `(printing_id, is_no_match)` a batch proposes for a group against what is stored - the shape `local_illustration._split_new_illustration_votes` has always had, whose own docstring calls it "THE ONE DIFFERENCE, AND IT IS LOAD-BEARING". - existing rows, SAME verdict -> skipped, counted in `already_voted`. Re-running over a converged catalogue stays a no-op, which is what stops run-scoping becoming an overwrite-everything churn machine. - existing rows, DIFFERENT verdict -> kept. The purge moves the stale generation into the archive and the new one lands. - no existing row -> kept. Compared PER GROUP, ALL-OR-NOTHING, because one identity can legally hold several rows for a card (`cardprintingtag_unique_printing_vote` constrains the triple, not the pair - `run_illustration_calculator`'s loop over `verdict.printing_pks` is a live caller shaped that way) and the purge is family-keyed on `card_id`. Keeping only PART of a group would delete the rest and never re-insert them. The "skip-and-count, not retract-and-recast" reasoning is untouched: it was always conditioned on both racing invocations computing the SAME verdict, which is exactly the case still skipped. What no longer gets swallowed is the case that reasoning already named as NOT covered - a genuinely changed conclusion. `local_lands_identify._split_new_votes` needed no change: it already compares the full (card, printing, anonymous_id) triple, so a changed answer already reached its purge. A test now pins that rather than leaving it a coincidence. LAYER 3 - `ArchivedCardPrintingTag` `purge_stale_machine_votes` copies every row it is about to delete into the archive first, stamped with the run that overwrote it. That function is THE choke point for "a machine vote is superseded by a later machine vote", so no caller can supersede without archiving and no new caller has to remember to. `vote_write.purge_and_write_votes` supplies `superseded_by_run_id`, derived from the batch it is writing, and its existing `transaction.atomic()` now covers three statements instead of two - the same cancel-safety property, one statement wider. A batch that cannot name a single run_id records NULL rather than a guess: a wrong stamp is worse than a missing one, since the diff report and issue #575's janitor both select on it and a plausible wrong value is indistinguishable from a right one after the fact. WHY A SEPARATE TABLE AND NOT RETAINED GENERATIONS IN THE LIVE ONE - MEASURED, NOT PREFERRED. Of the thirteen modules that read `CardPrintingTag`, NINE bypass `vote_consensus.resolve_vote_weight` entirely: `views.py`, `catalog_stats.py`, `local_calculate_verdicts.py`, `models.py`, `local_identify_printing_tags.py`, `soak_gate.py`, `harvest_probe.py`, `illustration_vote.py`, `local_lands_identify.py`. A zero-weight-by-run_id rule (migration 0097's pattern) protects only the four that route through weight resolution. A retained generation left in the live table would still be DISPLAYED by views, COUNTED by catalog-stats, and - fatally - would make eligibility treat the card as already voted, re-creating the very suppression this work removes. Keeping the live table single-generation means no consumer can be wrong about it: no unique-constraint change, no audit of thirteen modules, no new rule any future reader has to know. Rows are unreachable from `Card`/`CanonicalCard` (`related_name="+"` on both FKs), are not an `AbstractWeightedVote` subclass, and copy `created_at` verbatim rather than re-stamping it, with `archived_at` as the separate honest answer to "when did this stop being live". Append-only, no unique constraints - the same shape `CardScanLog` already has. Human votes never reach it: `calculator_family` returns None for the UUIDs humans use and the purge returns before touching anything. Retention is issue #575's janitor's ("keep the N most recent runs per calculator, sweep the oldest, operator-authorised with a dry run, never delete wholesale"). Both `run_id` (the superseded generation's own run) and `superseded_by_run_id` (the run that overwrote it) are indexed so a sweep can select a generation without a table scan. THE ONLY READER: `manage.py local_calculate_verdicts --generation-diff <path>`, one JSONL line per vote this run superseded. Per the owner's ruling, generation-diffing is an opt-in DEBUG FLAG, never a default write path - the archive WRITE is unconditional (a paper trail that only exists when somebody remembered to ask for it is not a paper trail); the READ is what is opt-in. ORDER IS A CORRECTNESS CONSTRAINT, NOT A PERFORMANCE ONE `fallback`, `illustration` and `slow-path` select POSITIVELY from join-key's output. So "purge everything, then run the calculators in parallel" gives THREE OF THE FOUR an empty eligible set - a silent near-no-op that reports success. Required order stays join-key -> fallback -> illustration -> slow-path, which is what both dispatchers already do. This is also why the upstream populations are deliberately NOT run-scoped, and that asymmetry is the whole correctness argument rather than an oversight: a converged join-key pass writes NOTHING under a fresh run_id (an identical recomputed verdict is skipped, and the stored row keeps its original run), so "cards join-key voted no-match IN THIS RUN" is empty on every re-run. Slow-path's fallback-voted exclusion must stay unscoped for the same reason and in a worse direction: a run-scoped version would route a card fallback SOLVED in an earlier run to a human reviewer. CONSEQUENCE WORTH KNOWING: THE RETRACTION RUNBOOKS ARE PARTLY OBSOLETED `reparse_collector_evidence` and `rejudge_fallback_channel`'s two-step runbook existed partly because a stale scan-log row permanently locked a card out of its calculator. It no longer does. Their retraction step is still needed to remove the stale RECORD (and, for votes, to stop a stale row being counted by consensus), but it is no longer what unlocks eligibility. Those tests now assert the exclusion under the recorded row's OWN run_id and say why, rather than quietly pinning the weaker claim. A RESUMPTION GAP THAT IS ACCEPTED, NOT OVERLOOKED A card whose verdict a run recomputes as UNCHANGED writes no row, so it carries no marker for that run and a restarted run recomputes it. Resumption skips completed WRITES, not completed recomputations. Closing it would mean stamping the current run_id onto an existing row, destroying the provenance migration 0097's frozen cohort depends on being able to state. Not worth it. VERIFICATION - `cardpicker/tests/test_run_scoped_eligibility.py`: 29 new tests across prior-run/current-run eligibility, the compiled-SQL trap, illustration's own duplicated copy of the exclusion, changed-verdict overwrite + archiving, archive-is-not-a-live-vote, superseding-run stamping, dependency ordering (including the out-of-order empty-set failure, constructed on purpose), and two- and three-pass convergence. - 26 MUTATIONS applied one at a time, each run red, then restored: every exclusion (vote and scan-log, in all four eligibility functions), every calculator's forwarding of its run_id, the split's value comparison, the split's skip-if-identical branch, the archive copy, `related_name="+"`, `created_at` fidelity, `original_id`, `superseded_by_run_id`, and each of the three upstream populations that must NOT be run-scoped. Three mutants came back GREEN on the first pass and each was a genuine coverage gap rather than a false alarm - the illustration calculator's run_id forwarding, and both call sites of the evidence-transfer stamp - so tests were added until every one went red. - A pre-existing latent flake fixed on the way past: `test_local_illustration.TestPrintingsForIllustration:: test_the_scope_reaches_the_compiled_sql` asserted `str(pk) not in sql`, which is only true while the pk is a digit string appearing nowhere else - and the query embeds `illustration_id` UUIDs verbatim, so a single-digit pk is a substring of almost any of them. It passed or failed on where the sequence happened to be, i.e. on which other test files ran first. Now asserted on the predicate it actually means. - Full `pytest cardpicker`: 3192 passed, 8 skipped. black, ruff, isort, mypy clean; docs-lint clean; `makemigrations --check`: no changes, single leaf. MIGRATION NUMBERING - 0100, AND A DEPENDENCY THAT DOES NOT EXIST YET `0100_superseded_card_printing_tag_archive`, depending on `0099_rename_printings_count_catalogued` (PR #601). THAT MIGRATION DOES NOT EXIST ANYWHERE YET - not on master, and not yet on #601's own branch, which still carries the file under its original name `0098_rename_printings_count_catalogued`. The name assumed here is #601's file renumbered 0098 -> 0099 with its slug unchanged, exactly the transformation #576 performed (`0096_freeze_...` -> `0097_freeze_...`, slug preserved). IF #601 LANDS UNDER ANY OTHER NAME THIS STRING MUST BE CORRECTED BEFORE MERGE, or `migrate` fails with NodeNotFoundError and no test database can be built on any branch. THIS PR THEREFORE CANNOT MERGE BEFORE #601. The chain 0098 (#573, merged) -> 0099 (#601) -> 0100 (this) is coordinator-assigned. There is no substantive ordering constraint between the three - #601 renames a column on `cardpicker_canonicalcard`, #573 added columns to `cardpicker_cardillustrationvote`, this creates a new table - the chain exists solely to keep `cardpicker` at a SINGLE LEAF NODE. HOW THIS WAS ORIGINALLY GOT WRONG, recorded so the reasoning is not repeated. It was first numbered 0098-on-0097 on the then-correct reasoning that #573 was still open and that depending on a migration absent from master makes a branch unmigratable today, with certainty, to avoid a collision that might never happen. #573 then MERGED, inverting the trade-off: the collision stopped being hypothetical and became a fact on master - and one invisible to every normal signal, since different filenames mean no textual conflict, GitHub still reported the PR mergeable, and CI stayed green because it had run against the pre-#573 tree. Only `makemigrations --check` and the migration loader's leaf count catch it. VERIFIED AFTER THE RENUMBER: `makemigrations --check --dry-run` reports "No changes detected"; `MigrationLoader.graph.leaf_nodes()` returns exactly one cardpicker leaf, `0100_superseded_card_printing_tag_archive`, with the forward plan ending 0097 -> 0098 -> 0099 -> 0100. Verified against a LOCAL STUB standing in for #601's migration (no operations, so it cannot alter model state); the stub is not committed. DOCS: TWO RUNBOOK CLAIMS THIS INVALIDATES, CORRECTED IN PLACE - `docs/troubleshooting.md`'s "A reparse_collector_evidence/Stage D retraction pass silently never routes its own newly-touched cards to slow-path review" is RESOLVED by this change, and NOT by the fix it had spec'd. Run-scoping means a stale `stage-d-slow-path-v1` marker no longer excludes anything from a new run. The spec'd fix - teach `reparse_and_retract` to also delete the slow-path row - was deliberately NOT built: it would make every retraction command responsible for knowing which downstream calculators had left markers, which is the coupling that produced the symptom. - `docs/features/stage-e-operations.md`'s `rejudge_fallback_channel` runbook described retraction as "making those cards eligible for a fresh local_calculate_verdicts pass". That was the mechanism and no longer is. Revised to say what retraction still buys: removing a stale RECORD, which for a VOTE is load-bearing (an un-retracted stale vote keeps its consensus weight until something overwrites it), while eligibility is now unlocked by every new run regardless. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN * Migration 0100: rewrite the dependency note now that 0099 is real on master Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
CardIllustrationVote(issue #524) has been WRITTEN since it landed and read by nothing but the admin and the human write path.printing_consensus.py/artist_consensus.py/tag_consensus.pyeach reconcile their own vote model; there was no illustration equivalent, so PR #565's projected ~10,277 machine rows would have been recorded rather than reasoned over — and two ratified rulings (the art hash resolves ARTWORK; illustration identity rules premium-vs-base conflicts) depend on them.1.
cardpicker/illustration_consensus.pyModelled on its two siblings, built on the shared
vote_consensuscore. No weighting, quorum, or human-backed gate is re-derived here.is_unknownis an ordinary outcome key, theUNKNOWNsentinel — exactly asartist_consensustreatsCardArtistVote.is_unknownandprinting_consensustreatsis_no_match. The XOR constraint makes the outcome space {uuid} ∪ {UNKNOWN}, and the abstention on this model is the absence of a row (the calculator's skip paths write aCardScanLogand no vote; a human who doesn't answer leaves nothing). A presentis_unknown=Truerow is a positive, falsifiable claim — "someone looked and there is no Scryfall artwork behind this" — so it must be able both to resolve on its own and to contest a uuid. Treating it as an abstention would make an UNKNOWN-vs-uuid disagreement read as an uncontested uuid, and would leave a card everyone agrees is unidentifiable sitting UNRESOLVED forever, indistinguishable from one nobody has looked at.2. md5 pooling built in from the start
Every read path is group-scoped.
resolve_illustrationtalliescard's whole md5 identity group, pooled per agent byvote_consensus.pool_group_votes, keyed onagent_dedupe_key— the versionless calculator family, not the rawanonymous_id. That is live, not hypothetical, for this calculator specifically: #565 bumped itstage-d-illustration-v1→-v2, and a version bump re-votes incrementally, so an md5 group straddling the migration holds rows under both strings at once. A raw-id key would read that as two independent agents and let one calculator buy a whole quorum on a routine redeploy.The group primitives (
md5_group_card_ids,md5_group_cards,_require_full_md5_group,agent_dedupe_key) are imported fromprinting_consensus, not reimplemented — one definition of a group, one completeness guard, one agent-identity rule.Doing this now rather than retrofitting is the point of the change. The printing side acquired pooling late and the defect that mattered lived in the seam between the un-pooled original and the retrofit (its own docstrings record both halves: human votes initially left unkeyed, and
_require_full_md5_group's "a subset yields a DIFFERENT tally, not a weaker one"). A resolver that is group-scoped from its first line has no such seam — no un-pooled call path left behind, no caller that predates the contract.md5 is strictly stronger than the perceptual art hash here. Byte-identical files are necessarily the same artwork: no threshold, no tolerance, no distance metric, because identical bytes decode to identical pixels. A phash match is a different claim — two cards can share one and be genuinely different artworks at any radius. Since pooling deliberately suppresses evidence and propagation pushes one member's answer onto another, both are sound only on byte identity. Nothing in the module reads
content_phash, structurally: membership has exactly one source.3. Propagation — a consequence of group-scoped resolution, not a separate step
The calculator needs BOTH evidence AND a candidate-name match. Evidence transfers across an md5 group (
evidence_transfer); the decorated NAME does not — so a member whose name fails candidate resolution abstains withno-candidate-match(367 of 2,350 considered cards, ~15.6% in #565's 30,000-card replay) even though a byte-identical sibling resolved cleanly.Closed by the tally being defined over the group: the abstainer's tally is its sibling's tally, and
resolve_and_persist_illustrationwrites the outcome to every member. It ends up with a resolvedinferred_illustration_idwith no code path aware it abstained.The rejected alternative — an explicit step writing a copied
CardIllustrationVoterow onto the abstainer — is worse on three counts, in order of weight:anonymous_id, so it pools under the samededupe_keyand collapses to the same event: exactly zero added weight. The only thing it could change is per-card display, which group-scoped persistence already provides. Tested, not merely argued (TestPropagatedVoteRowsWouldBeWeightNeutral).CardIllustrationVote's unique constraint on (card, anonymous_id) is unconditional (stage-d-illustration-v1 at N>1 casts N full-weight votes for mutually exclusive printings; the /N confidence spread never reaches the tally #525). A propagated row occupies the exact slot the calculator wants when that member later becomes resolvable, and_purge_and_write_illustration_votescompares the stored value — it could not tell a propagated row from the calculator's own stale answer.Honest limit, stated as a test (
test_machine_only_evidence_does_not_propagate): nothing propagates until the group actually RESOLVES, and the human-backed gate means a machine-only group never does. The ~15.6% are cards that become resolvable once a human weighs in on any member, not cards resolved from machine votes alone.Persistence, thresholds, reference data
Card.inferred_illustration_id(plainUUIDField, not an FK — noCanonicalIllustrationtable, mirroring the vote model andCanonicalPrintingMetadata.illustration_id) plusCard.illustration_vote_status(IllustrationVoteStatus, the four membersArtistVoteStatushas), migration 0096. Written for every group member, in pk order so concurrent votes on two members queue rather than deadlock.ILLUSTRATION_MIN_VOTES/ILLUSTRATION_MIN_SHAREdefault toPRINTING_TAG_MIN_VOTES/_MIN_SHARE, so this ships changing nothing and the illustration bar can later move without moving the printing bar. Whether it should differ is genuinely open — an illustration claim is strictly coarser (1:N, ~2.2 printings per illustration) so easier to get right, but does less work once resolved — and with 3 votes in production there is no data to settle it, so the defaults settle it by changing nothing. NoILLUSTRATION_MACHINE_WEIGHT: a vote's weight is a property of who cast it and by what method, never of what is being voted on (same argumentresolve_vote_weightmakes against confidence-scaled weight); per-vote-type weights would also count one agent'sCardIllustrationVoteand its derivedCardPrintingTagdifferently for one judgement.Reference data (owner ruling 2026-07-29). This module reads neither
CanonicalCardnorCanonicalPrintingMetadata— it tallies uuids off vote rows and stores the winner verbatim, asserted directly byTestReferenceDataIndependence(consensus resolves with zero canonical rows in existence, and for a uuid no metadata row references). What a stale snapshot does, precisely: upstream it changes which uuids exist to be voted for, so the pipeline under-resolves and never mis-resolves from that cause; downstream it changes what a resolved uuid maps to, a consumer-side narrowing done by a live join each consumer performs itself. What it cannot do is change a tally, flip a winner, or move a propagation. If reference data is later found wrong, the votes stand and re-running reproduces the same outcome — the correction belongs at the calculator, which is where it was consulted.Tests — 50 new, every one proven able to fail
Production holds 3 illustration votes, so this cannot be validated by observation. 24 mutations were applied to the implementation and each of the 50 tests was verified RED under at least one; the full mutation table is in the thread below. Highlights:
is_unknownrows dropped (abstention reading)pool_group_votesbypassedagent_dedupe_key→ rawanonymous_idcontent_phashis_human_backed=Truefor every voteCanonicalPrintingMetadataThe phash tests carry a positive control (
test_the_same_pair_with_a_matching_md5_does_propagate) that changes only the md5 — without it both negatives would stay green against an implementation that propagates to nothing at all.test_lowering_the_illustration_quorum_does_not_lower_the_printing_oneis stated behaviourally (one human vote resolves the illustration, the same voter's printing vote does not resolve the printing) rather than as a comparison of two settings values, which could not fail.Harness fix — not incidental
test_shared_cache.py::TestSharedCacheTableMigrationcarries@pytest.mark.django_db(transaction=True), so its migration round-trip really commits — and itsfinallyrestored only as far as0092, leaving every later migration unapplied for the remainder of the pytest session, in a database later tests go on using.Latent and free until now purely by alphabetical luck: 0093/0094 are data-only
RunPython, and 0095 (face_illustrations) is read only by modules sorting beforetest_shared_cache.py. A migration adding aCardcolumn is the first thing to step on it —test_sources.pyandtest_stage_e_dispatch.pysort later and usetransactional_db, so they write realCardrows through a connection no rollback can save, and failed withcolumn "inferred_illustration_id" ... does not exist: a message pointing squarely at this branch for a defect entirely in thatfinally. Now restores tograph.leaf_nodes("cardpicker")— derived, so it stays correct for every future migration without anyone remembering this file exists.Verification
cardpicker/tests/suite: 3,090 passed, 9 skipped, 0 failed, against a 3,040-passed baseline on the same commit ofmaster(85d88bf).black/isort/ruff/mypyclean;docs_lint.pyclean (no new*_ANONYMOUS_ID, so no roster entry is required —docs/theory.md§4 item 3 and §10a's neighbourhood-lookup list are updated instead, the latter from four live instances to five).masterafter Per-face illustration ids; delete the border-colour "multi-faced" gate; stage-d-illustration-v2 #565 merged; migration renumbered 0095 → 0096 behind0095_canonicalprintingmetadata_face_illustrations,makemigrations --checkreports no pending changes.Not in this PR
consensus_recomputeis not wired to the illustration domain — the human write path (cast_illustration_vote) recomputes inline, which is the path that actually produces resolutions, but there is no batch recompute over the ~10,277 machine rows yet. Deliberate: a machine-only group cannot resolve, so a batch pass over them today would write nothing, and the wiring touches a file under concurrent edit. Flagged rather than silently omitted.🤖 Generated with Claude Code
https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN