Linearise the cardpicker migration graph: two 0096 leaf nodes block every deploy - #576
Merged
Merged
Conversation
#568 (`0096_card_scan_log_anon_skip_idx`) and #570 (`0096_freeze_deductive_backfill_zero_weight_cohort`) merged independently and both declared `0095_canonicalprintingmetadata_face_illustrations` as their sole dependency. That left `cardpicker` with two leaf nodes, so `manage.py migrate` refuses to run at all: CommandError: Conflicting migrations detected; multiple leaf nodes in the migration graph: (0096_card_scan_log_anon_skip_idx, 0096_freeze_deductive_backfill_zero_weight_cohort in cardpicker). Production is on 85d88bf, which predates both, so production itself is healthy - but the next deploy would have failed at the migrate step and nothing could ship until this landed. WHAT CHANGED `0096_freeze_deductive_backfill_zero_weight_cohort` is renumbered to `0097_...` and its dependency repointed from 0095 to `0096_card_scan_log_anon_skip_idx`, giving a single chain and a single leaf. Renaming rather than only adding a dependency, because a file rename is only dangerous where the old name is already recorded in a `django_migrations` row, and it is not recorded anywhere: the production database's latest applied `cardpicker` migration is 0095 (verified read-only against the live database), and no other environment holds anything but a throwaway test database. WHY THE FREEZE MIGRATION IS THE ONE THAT MOVED Ordering is NOT a correctness constraint here, and the commit says so rather than leaving it assumed: 0096 adds an index to `cardpicker_cardscanlog`, and the freeze migration UPDATEs `run_id` on ~28,112 `cardpicker_cardprintingtag` rows. Disjoint tables, no shared column, constraint or trigger, so neither can affect the other's outcome in either order. Two things break the tie: - Merge order. #568 landed first (d7f28d4) and #570 second (1b861e0); numbering that matches the order they landed is the one a reader can check against `git log`. - Re-run safety. If some unknown database HAS applied the old name, the rename makes it re-run. The freeze migration's `run_id__isnull=True` conjunct makes a second application a no-op; re-running the AddIndex would error with "relation already exists". So the migration that is safe to have been renamed is the one that was renamed. REFERENCES UPDATED `test_vote_consensus.TestZeroWeightCohortScopeIsPinned` imports the migration module by dotted path to assert the code constant equals what the migration actually wrote - that string is updated, and it is the only functional reference. Prose references to the freeze migration are updated in `vote_consensus.py`, `deductive_backfill.py`, `test_printing_consensus.py`, `docs/theory.md`, `docs/pipeline-fidelity-gate.md` and `docs/features/printing-tags.md`. References to "migration 0096" in `models.py`, `test_catalog_stats.py` and `docs/features/catalog-stats.md` denote the INDEX migration, which keeps its number, and are deliberately left alone. VERIFICATION - `makemigrations --check --dry-run`: "No changes detected", exit 0 (was: CommandError, two leaf nodes). - `showmigrations cardpicker --plan`: single chain ending 0095 -> 0096_card_scan_log_anon_skip_idx -> 0097_freeze_deductive_backfill_zero_weight_cohort. - `manage.py migrate` run to completion against a scratch database - every migration applied, the freeze migration reporting "0 rows" (correct on any database that is not production). - `cardpicker/tests/test_vote_consensus.py`, `test_printing_consensus.py`, `test_catalog_stats.py`: 198 passed. - docs-lint clean; black, ruff, isort, mypy clean. Both management commands need a staticfiles manifest that a bare checkout does not have ("Missing staticfiles manifest entry for 'cardpicker/favicon.ico'"). Confirmed to reproduce identically on unmodified master, and resolved locally by running `collectstatic`; it is a local environment gap, unrelated to this change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN
This was referenced Jul 29, 2026
Author
Severity is higher than "the next deploy fails"While rebasing #567/#573 against master I hit this, which the PR body understates: The two-leaf graph blocks the entire test suite on Reproduced on unmodified All three suite runs behind #567 (3148 passed), #574 (3150 passed) and #573 (3157 passed) were done with this commit layered on top, because that was the only way to get a test database at all. |
This was referenced Jul 29, 2026
Merged
WilfordGrimley
added a commit
that referenced
this pull request
Jul 29, 2026
…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
WilfordGrimley
added a commit
that referenced
this pull request
Jul 29, 2026
… 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
WilfordGrimley
added a commit
that referenced
this pull request
Jul 29, 2026
…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
WilfordGrimley
added a commit
that referenced
this pull request
Jul 29, 2026
… 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
WilfordGrimley
added a commit
that referenced
this pull request
Jul 29, 2026
…-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>
WilfordGrimley
added a commit
that referenced
this pull request
Jul 29, 2026
…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>
WilfordGrimley
added a commit
that referenced
this pull request
Jul 30, 2026
… 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
WilfordGrimley
added a commit
that referenced
this pull request
Jul 30, 2026
… 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
origin/masterhas a broken migration graph. #568 (0096_card_scan_log_anon_skip_idx) and #570 (0096_freeze_deductive_backfill_zero_weight_cohort) merged independently and both declared0095_canonicalprintingmetadata_face_illustrationsas their sole dependency, leavingcardpickerwith two leaf nodes:manage.py migraterefuses to run at all. Production is on85d88bfe, which predates both, so production itself is healthy — but the next deploy fails at the migrate step and nothing can ship until this lands.What changed
0096_freeze_deductive_backfill_zero_weight_cohort→0097_freeze_deductive_backfill_zero_weight_cohort, dependency repointed from 0095 to0096_card_scan_log_anon_skip_idx. Single chain, single leaf.Renamed rather than only adding a dependency. A migration rename is dangerous exactly where the old name already sits in a
django_migrationsrow — and it does not, anywhere. Verified read-only against the live production database: the latest appliedcardpickermigration is0095_canonicalprintingmetadata_face_illustrations. Neither 0096 is applied in production, and no other environment holds anything but a throwaway test database.Why the freeze migration is the one that moved
Ordering is not a correctness constraint, and this is stated rather than assumed: 0096 adds an index to
cardpicker_cardscanlog; the freeze migrationUPDATEsrun_idon ~28,112cardpicker_cardprintingtagrows. Disjoint tables, no shared column, constraint or trigger — neither can affect the other's outcome in either order.Two things break the tie:
d7f28d4a), Re-scope the deductive-backfill zero-weight rule from the METHOD to the COHORT #570 second (1b861e05). Numbering that matches the order they landed is the one a reader can check againstgit log.run_id__isnull=Trueconjunct makes a second application a no-op; re-running theAddIndexwould error with relation already exists. The migration that is safe to have been renamed is the one that was renamed.References updated
test_vote_consensus.TestZeroWeightCohortScopeIsPinnedimports the migration module by dotted path to assert the code constant equals what the migration actually wrote — that string is updated, and it is the only functional reference. Prose references updated invote_consensus.py,deductive_backfill.py,test_printing_consensus.py,docs/theory.md,docs/pipeline-fidelity-gate.md,docs/features/printing-tags.md.References to "migration 0096" in
models.py,test_catalog_stats.pyanddocs/features/catalog-stats.mddenote the index migration, which keeps its number, and are deliberately left alone.Verification
makemigrations --check --dry-runCommandError, two leaf nodesNo changes detected, exit 0showmigrations cardpicker --plan0096s0095 → 0096_card_scan_log_anon_skip_idx → 0097_freeze_…manage.py migrate(scratch DB)0 rows, correct on any non-production databasecardpicker/tests/test_vote_consensus.py,test_printing_consensus.py,test_catalog_stats.py: 198 passed.Both management commands need a staticfiles manifest a bare checkout lacks (
Missing staticfiles manifest entry for 'cardpicker/favicon.ico'). Confirmed to reproduce identically on unmodified master, and resolved locally withcollectstatic— a local environment gap, unrelated to this change.Scope
Hotfix. Single purpose — no other work bundled in.
🤖 Generated with Claude Code
https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN