Run-scoped eligibility, the value-comparing split, and an archive for superseded votes

[WilfordGrimley]

"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

Stacked on #603 (the run_id audit). Review that first; this PR's base is that branch.

Why these three are one PR 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.

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 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 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(... U1.anonymous_id = X ...) AND EXISTS(... U1.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".

stored vs proposed	outcome
same verdict	skipped, counted in already_voted — re-running over a converged catalogue stays a no-op
different verdict	kept → purge moves the stale generation into the archive, 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 triple). 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; its existing transaction.atomic() now covers three statements instead of two — 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 #575's janitor both select on it.

Why a separate table and not retained generations in the live one — measured, not preferred

Of the 13 modules that read CardPrintingTag, 9 bypass 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 (archived_at is 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 #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 and superseded_by_run_id are indexed so a sweep can select a generation without a table scan.

The only reader is manage.py local_calculate_verdicts --generation-diff <path>. 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 all 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, 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.

TestDependencyOrdering constructs the out-of-order failure on purpose — a suite that only ever runs them in the right order is not evidence that the order matters.

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 — 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 empty-set failure), 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, on a dependency that does not exist yet

Rebased onto current origin/master (post-#573, #579, #581, #582, #584, #586).

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 dependency 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; two leaves make manage.py migrate refuse to run at all, which breaks test-database setup on every branch in the repo, not just the offender's.

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 this 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 → No changes detected, exit 0.
• MigrationLoader.graph.leaf_nodes() → exactly one cardpicker leaf, 0100_superseded_card_printing_tag_archive; forward plan ends 0097 → 0098 → 0099 → 0100.
• Verified against a local stub standing in for printings_count counts our own rows: rename it, and delete the "cross-verified against Scryfall" claim #601's migration (no operations, so it cannot alter model state). The stub is not committed.

Rebase note: this PR and #579 touch the same querysets

#579 (push card_ids into every Stage D dependency subquery) landed while this was open and refactored the join-key no-hit population into a shared _join_key_no_hit_subqueries(card_ids) helper. Two conflicts, both resolved by composing rather than choosing: _fallback_eligible_cards_queryset and _slow_path_eligible_cards_queryset keep #579's helper and card_ids pushdown and gain this PR's run_id narrowing (which applies to slow-path's own already_routed sub-population only, never to the upstream join-key/fallback ones). The mutants that target those two functions were respelled against the new code shape and re-run — both still red.

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 in the first place.
• 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.

Post-rebase verification

Full pytest cardpicker: 3304 passed, 8 skipped. black, ruff, isort, mypy clean; docs-lint clean. Seven mutations re-run against the rebased tree (including the two respelled for #579's refactor) — all red.

🤖 Generated with Claude Code

https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN

[WilfordGrimley]

Rebased onto origin/master (4a3babc). The old NodeNotFoundError failure was stale; CI is now green.

The Backend tests FAILURE that sat on this PR was produced before 0099 landed on master. GitHub does not re-run checks when the base moves, so the stale result kept looking authoritative. 0099_rename_printings_count_catalogued is on master now, 0100's dependency on it is correct and unchanged, and the re-run is green including #611's One leaf per app (merged with the base branch) job.

MigrationLoader(None).graph.leaf_nodes() → exactly one cardpicker leaf, 0100_superseded_card_printing_tag_archive. Verified mechanically, not by eye.

Merge order: this PR first, then #615. #615's 0101_delete_printingtagvote has been repointed to depend on 0100, so its own CI is red until this lands — deliberately, and documented there.

Rebase composed rather than chose

The rebase onto post-#601 master was clean, and both PRs' semantics were re-verified present afterwards: #601's printings_count → catalogued_printings_count rename and its deletion of the false "cross-verified against Scryfall" claim, and this PR's run-scoping and superseded-vote archive. 0100's docstring, which had been written against a 0099 that did not exist yet, was rewritten to describe the real merged graph.

The mutation re-run found a genuine coverage gap

Re-running mutants against the rebased tree, targeting _eligible_base_queryset in local_identify_printing_tags.py (one of the files #601 moved):

Mutant	Result
Delete run_id narrowing on the own-CardScanLog exclude	red — killed by test_a_PRIOR_run_s_scan_log_row_does_not_suppress_a_new_run
Delete run_id from the own-vote CardPrintingTag narrowing	GREEN — survived the entire suite (3381 passed)

The function run-scopes two self-suppressing excludes and only the abstention half was asserted anywhere. The vote half — the one the owner directive is actually about — was covered by no test at all. Fixed by adding TestRunLandsIdentify::test_this_run_s_own_VOTE_removes_the_card_from_the_land_pool and ::test_a_PRIOR_run_s_vote_does_not_suppress_a_new_run; the mutant now dies on the second.

Full pytest cardpicker/tests/: 3383 passed, 11 skipped. Merged with #615 on a scratch tree: clean merge, single leaf, 3350 passed.

🤖 Generated with Claude Code