Skip to content

Fix local_lands_identify write-path vote collision and dry-run yield display - #411

Merged
WilfordGrimley merged 1 commit into
masterfrom
fix-lands-identify-write-collision-408-407
Jul 24, 2026
Merged

Fix local_lands_identify write-path vote collision and dry-run yield display#411
WilfordGrimley merged 1 commit into
masterfrom
fix-lands-identify-write-collision-408-407

Conversation

@WilfordGrimley

@WilfordGrimley WilfordGrimley commented Jul 24, 2026

Copy link
Copy Markdown

Description

Fixes two bugs in local_lands_identify (docs/features/catalog-completion-plan.md's Part 4):

  • local_lands_identify --write crashes on duplicate local-ocr-v1 vote #408 (blocker): the OCR-resolved branch casts votes under OCR_ANONYMOUS_ID (local-ocr-v1), an identity _land_pool_selected_cards's own eligibility query never checks (it's scoped to LANDS_ANONYMOUS_ID alone). A card can legitimately still be selected into this pool while an earlier pass/pilot run already cast an identical (card, printing, OCR_ANONYMOUS_ID) vote — --write run 20260724T021229-15c88eba hit exactly this and crashed with IntegrityError on cardprintingtag_unique_printing_vote, rolling back with zero rows landed. Fixed with an explicit pre-write skip-if-exists (_split_new_votes) rather than a blanket ignore_conflicts=True: a single batched existence query partitions the batch into (safe to write, already-voted), the latter counted in a new LandsIdentifyResult.already_voted field (surfaced in the ledger counters and terminal output) rather than silently dropped, so the run survives the collision and every other vote in the batch still commits.
  • local_lands_identify dry-run prints would_cast=0 despite real would-cast counters #407 (rider): the dry-run terminal summary printed total_votes=would_cast=0 unconditionally (reusing votes_written, the write-gated counter, which is always 0 in dry-run) even on runs whose ledger counters recorded a real prediction (e.g. ocr_resolved=102+singleton=7+tiebreak=4=113/300 on runs 20260724T013302/20260724T014701), misleading a review into treating a real-yield run as zero-yield. Now prints the real ocr_resolved+singleton_votes+tiebreak_votes breakdown in dry-run mode, matching local_calculate_verdicts's own would_cast= convention.

Closes #408
Closes #407

Checklist

  • I have installed pre-commit and installed the hooks with pre-commit install before creating any commits.
  • I have updated any related tests for code I modified or added new tests where appropriate.
  • I have manually tested my changes as follows:
    • TestSplitNewVotes (4 tests): direct unit coverage of the new pre-write guard, including the "different identity, same card+printing" non-collision case.
    • TestRunLandsIdentify::test_ocr_resolved_vote_colliding_with_an_existing_identical_vote_is_skipped_not_crashed: regression for local_lands_identify --write crashes on duplicate local-ocr-v1 vote #408 — seeds an identical pre-existing OCR_ANONYMOUS_ID vote, runs --write, asserts no crash, no duplicate row, already_voted == 1, and the other card's vote still commits.
    • test_dry_run_prints_the_real_would_cast_breakdown_not_the_write_gated_zero: regression for local_lands_identify dry-run prints would_cast=0 despite real would-cast counters #407 — mocks a dry-run result with real ocr_resolved/singleton/tiebreak counters and asserts the printed summary contains total_votes=would_cast=113, not would_cast=0.
    • Full backend suite: 1906 passed, 4 skipped (up from 1899 passed, 4 skipped on master — the 7 new tests), via pytest cardpicker in /home/ubuntu/.venvs/mpcautofill-pilot.
    • ruff/isort/black/mypy all clean (pre-commit hook ran clean on the commit).
  • I have updated any relevant documentation or created new documentation where appropriate.
    • docs/features/catalog-completion-plan.md's Part 4 section: added a dated status entry documenting the crash, root cause, and fix, matching that section's own established history-of-runs convention.

Notes for reviewer

  • No prod/DB writes were made from this session — code + tests only, per task scope. The authorized lands --write run this unblocks has not been re-run.
  • already_voted is a new counter, not just a bugfix side-effect: it's surfaced in PilotRunLedger.counters and the terminal summary so a future run's ledger stays honest about computed-vs-written when this identity-scoping mismatch recurs (it's structural, not a one-off — any card resolved via plain OCR in this module can hit it again on a future re-run).

@WilfordGrimley
WilfordGrimley merged commit 75d4264 into master Jul 24, 2026
8 checks passed
This was referenced Jul 24, 2026
Merged
Merged
WilfordGrimley added a commit that referenced this pull request Jul 24, 2026
@WilfordGrimley @claude
Fix Stage E concurrent-dispatch vote-collision IntegrityError (#448)
41181e4 
* Fix Stage E concurrent-dispatch vote-collision IntegrityError

Two concurrent dispatch_micro_batch invocations (django-q2's 8 workers,
or the backstop sweep racing an event trigger) could both pass Stage D's
per-identity eligibility check before either committed, then race to
bulk_create the same (card, anonymous_id) CardPrintingTag - the loser hit
IntegrityError and aborted its whole micro-batch (trip
envtrip-20260724T214616-be6e5db9, failed run_ids stage-e-stream-20260724T2144*).

Adds a pre-write skip-if-exists guard (_split_new_printing_tag_votes,
mirroring PR #411's precedent) to run_join_key_calculator and
run_fallback_calculator - skip-and-count, not retract-and-recast, since a
concurrent race yields the same verdict from the same evidence, not a
genuine conclusion change. run_slow_path_calculator needs no equivalent
guard (CardScanLog carries no DB uniqueness constraint). Corrects
stage-e-operations.md's overstated "eligibility exclude alone is
idempotent" claim for the concurrent (not just sequential) case.

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

* Correct incident facts, add ignore_conflicts belt-and-suspenders (Tron gate)

Tron gate on PR #448: seven failed run_ids + one winner (not four) =
Q_CLUSTER workers=8; separates the vote-collision failure (this guard)
from the SEPARATE envtrip-20260724T214616 host-load trip (11.85 vs 7.0,
8 concurrent OCR dispatches on 7 cores, not fixed by this change);
qualifies the "same verdict" premise as contingent on unchanged code/
evidence/lexicon, naming reparse_collector_evidence as the remedy
otherwise; adds bulk_create(..., ignore_conflicts=True) as the actual
crash-proofing against the guard's own residual check-then-insert race
window (precedent: local_layout_class_cast.py:300,
local_detect_ai_art.py:459, local_identify_printing_tags.py:1246), with
a regression test defeating the pre-write check to prove it; adds an
ops-doc runbook line against running BULK-mode writes while PASSIVE
streaming is enabled.

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

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@WilfordGrimley WilfordGrimley mentioned this pull request Jul 28, 2026
Merged
4 tasks
WilfordGrimley added a commit that referenced this pull request Jul 28, 2026
@WilfordGrimley @claude
fix: Stage D illustration hot-path regressions, vote-write ordering/a…
084927b 
…tomicity, and 1:1 printing resolution (#525) (#526)

* fix: remove dead import of renamed measure_bleed_diff_mm in image_evidence

53c1484 renamed measure_bleed_diff_mm -> compute_bleed_diff_mm in
local_fallback.py without leaving an alias. image_evidence.py still
imported the old name, raising ImportError at module import time for
every one of its ten importers (Stage C/D/E). This removes the dead
import and the now-unreachable fallback call at the old bleed_diff_mm
assignment, letting compute_bleed_diff_mm's own assignment stand as
the sole source of that field.

* test: remove orphaned test_artist_external_links.py

53c1484 added this test file for an MTGAC proxy view
(get_artist_external_links) and imports from cardpicker.views that
were never actually committed - the feature has an unstarted design.
The test aborted collection for the entire backend suite
(Interrupted: 1 error during collection) and was pushed unformatted
(isort + black both flagged it). Deleting it is safe and reversible:
the file remains in git history at 53c1484 and will be recovered
when the MTGAC integration is actually built. Not implementing the
missing view, adding skip/xfail markers, or merely reformatting -
those would leave a test asserting against code that does not exist.

* fix: Stage D illustration hot-path regressions + vote-write ordering/atomicity

`dispatch_micro_batch` is a streaming conveyor processing cards in 25-card
micro-batches; anything it calls must cost O(batch), never O(catalog)
(issues #458/#460). `_run_stage_d` calls `run_illustration_calculator` once
per micro-batch. The illustration calculator (added 2026-07-28) never carried
over the issue-#469 fixes that `local_calculate_verdicts.py` already has.

1. CandidateNameIndex cache bypass (local_illustration.py:468)
   `candidate_name_index = CandidateNameIndex()` — a direct constructor call
   whose own comment claimed "same cached pattern as
   `_get_cached_candidate_name_index()`". It was not: it bypassed the cache
   entirely. Cost before: a 113,224-row CanonicalCard scan, measured 1.48s,
   per micro-batch with at least one artist-OCR card. Now calls
   `local_calculate_verdicts._get_cached_candidate_name_index()`.

2. Unscoped materialization (local_illustration.py:385-394)
   Both join-key no-hit populations were wrapped in `list(...)`. Cost before:
   every join-key `is_no_match` CardPrintingTag row and every join-key no-hit
   CardScanLog row (CardScanLog is 2,093,147 rows live, append-only) pulled
   into process memory on every micro-batch, before any card_ids scoping.
   Now lazy querysets compiled into SQL subqueries, matching
   `_fallback_eligible_cards_queryset`.

3. card_ids applied late (local_illustration.py:211-253, 396-401)
   `_eligible_illustration_cards_queryset` took no `card_ids`; the caller
   filtered afterwards, so the CardScanLog exclusion subquery inside it
   compiled unscoped. Cost before: a full pass over 2,093,147 rows per
   micro-batch. Now mirrors `_eligible_cards_queryset:966-996` exactly,
   including `.filter(card_id__in=card_ids)` on the subquery, with issue
   #469's reasoning preserved.

4. IllustrationIndex rebuild (local_illustration.py:128-165, 411)
   Two catalog-wide CanonicalCard queries (one filtered on illustration_id,
   one UNFILTERED over every card with an artist) plus full-catalog dicts,
   constructed per invocation i.e. per micro-batch. Now memoized per worker
   process behind a version stamp, same shape as
   `_get_cached_candidate_name_index()`. The stamp adds a fifth term — the
   count of non-null `illustration_id` — because that column is BACKFILLED
   IN PLACE by `import_scryfall_printing_metadata`, an UPDATE that moves
   neither max pk nor row count. The no-op `.select_related(...)` chained
   before a `.values_list()` with traversal args was removed.

5. `already_voted` was structurally 0 (local_calculate_verdicts.py:1286-1293,
   1614-1620; local_illustration.py:532)
   `purge_stale_machine_votes` deletes by calculator FAMILY, which includes
   the caller's own current anonymous_id — so running it before
   `_split_new_printing_tag_votes` deleted exactly the rows the split then
   looked for. The counter read 0 in every deployment forever: the literal
   "zero forever would suggest the guard itself is dead code" failure
   `stage_e_dispatch.py:250-253` warns about. Split now runs first, and the
   purge is scoped to `new_votes` so a skipped card keeps the winner's row.
   Calculator-version self-overwrite (#519/#520) is preserved: the split
   checks the exact current anonymous_id, so a stale `-v1` row is never a
   collision and its card is still purged and overwritten.

6. Cancel-safety (same three sites)
   The purge is a DELETE and the insert a separate statement. A process
   killed between them — which this operator does deliberately, mid-flight —
   lost votes with nothing written back. Both are now inside one
   `transaction.atomic()`, via the new shared
   `_purge_and_write_printing_tag_votes` primitive.

Tests: +17 net. New coverage for item 3 (the CardScanLog subquery is
genuinely narrowed in the compiled SQL, and BULK mode is untouched), item 4
(process cache reuse, invalidation on insert and on in-place illustration_id
backfill, zero index builds for an empty micro-batch), item 6 (a failed
insert rolls the purge back), and items 1/5.

Three existing tests changed behaviour, all of them assertions added by
#519/#520 that codified the regression above ("Update two concurrent-dispatch
collision tests: purge changes 'skip' semantics to 'overwrite'"):
`test_local_calculate_verdicts.py` join-key and fallback collision tests and
`test_stage_e_dispatch.py`'s ledger assertion now expect `already_voted == 1`
and the winner's row surviving, which is the semantic
`local_lands_identify`'s equivalent test has asserted since PR #411.

Refs #458, #460, #469, #507, #519

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

* test: derive RSS-ceiling tests from RSS_MB_PER_WORKER_CEILING, not a hardcoded 512

70225df raised the RSS ceiling 512->768MB in operating_envelope.py but updated
zero tests. test_operating_envelope.py::test_trips_above_512mb_per_worker fed
a literal 512.1, which is now under the 768 ceiling, so check_envelope()
returned None and the trip-not-None assertion failed.
test_resolve_envelope_trip.py::test_acknowledging_one_trip_does_not_touch_another
cascaded from the same cause: it built its second trip with a literal 600.0,
also now under the ceiling, so no trip was ever created for it to acknowledge.

Both now derive the RSS value from RSS_MB_PER_WORKER_CEILING (already imported
and already used this way in test_operating_envelope.py's own
test_exactly_at_the_ceiling_does_not_trip) instead of a hardcoded number, so a
future ceiling change can't silently rot these tests the same way again.
Renamed test_trips_above_512mb_per_worker -> test_trips_above_rss_ceiling_per_worker
since its name no longer hardcodes the ceiling either.

No production code touched - operating_envelope.py and the 768 ceiling are
unchanged.

* fix: illustration calculator resolves a printing only at 1:1 (#525)

BEHAVIOUR CHANGE — reduces illustration vote coverage. Deliberately its own
commit so it can be evaluated or reverted independently of the performance
work in the previous commit.

THE DEFECT. `calculate_illustration_verdict` returned MULTIPLE `printing_pks`
from both terminal branches, and the runner emits one `CardPrintingTag` per
printing pk. One machine identity therefore voted simultaneously for several
MUTUALLY EXCLUSIVE printings of the same card:

  - N>1 illustrations (local_illustration.py:305-320): "union of printings
    across all illustrations, each at BASE_CONFIDENCE/N".
  - N==1 illustration (local_illustration.py:322-329): "vote every printing at
    full BASE_CONFIDENCE". Since illustration_id → printing is 1:N, this fired
    for ANY reprinted artwork — the common case, not an edge case.

The confidence spread does not mitigate it. `resolve_vote_weight(source,
anonymous_id)` takes no confidence argument and `VoteTuple` has no confidence
field, so confidence never reaches the tally: every row landed at full
`PRINTING_TAG_MACHINE_WEIGHT`. `cardprintingtag_unique_printing_vote` is on
(card, printing, anonymous_id), so all N rows persist. Where md5 identity-group
pooling is active they read as self-contradiction and get withheld; where it is
not, they all count for outcomes that cannot all be true.

THE FIX. A printing vote is cast ONLY when the surviving set resolves to
exactly one printing pk — exactly one illustration AND that illustration
mapping to exactly one printing. Every other case abstains with a CardScanLog
row.

TWO DISTINCT ABSTAIN REASONS, because only one of them is recoverable:

  - `multiple-illustrations` (N>1 illustrations): no single illustration
    identity exists. Counts only; no representative is invented.
  - `multiple-printings-one-illustration` (1 illustration, N printings): the
    illustration identity IS known with full confidence — only the printing
    choice is undetermined. `IllustrationVerdict` retains `illustration_id` and
    `candidate_printing_pks` so issue #524 (`CardIllustrationVote`) can persist
    it without re-deriving anything.

The retained values live in new verdict fields, NOT in `printing_pks`, so no
future reordering of the runner's loop can turn retained evidence into cast
votes. Nothing is persisted beyond the skip_reason string: no new model, no new
field, no migration. Persistence is #524 and separately owned — this abstain is
the seam it plugs into, not the intended end state.

MEASURABILITY. `IllustrationCalculatorResult` gains
`cards_abstained_ambiguous` and `printing_votes_withheld` (the exact number of
CardPrintingTag rows the pre-#525 code would have written). Both are counted in
dry-run mode, so the coverage figure is obtainable without a live write. The
audit sample carries the retained narrowing per abstained card.

Not changed, deliberately: `stage-d-illustration-v1`'s anonymous_id and version
string (#525 records that a bump is unsafe pending separate investigation);
`vote_consensus.py`/`printing_consensus.py` (confidence is NOT made
load-bearing — that is a separate owner decision and the consensus code carries
its own warning against it); the artist input, which stays OCR-evidence-derived
(#523).

Tests: one existing test asserted the defect and was rewritten, not quietly
changed — `test_multiple_illustrations_spreads_confidence` asserted
`skip_reason == ""`, two printing_pks and `confidence == BASE_CONFIDENCE / 2`;
it is now `test_multiple_illustrations_abstains_and_retains_no_identity`.
`test_single_illustration_votes_all_printings` was renamed (its fixture only
ever built one printing, so it never exercised the multi-printing path its name
described). New: 1-illustration-N-printings abstains and retains the
illustration; N>1 abstains and retains nothing; 1:1 casts exactly one vote at
BASE_CONFIDENCE; the two reasons are distinguishable; a printing reached twice
via two candidates sharing an artist is still 1:1; and five end-to-end runner
tests covering what actually reaches the DB.

Refs #525, #524, #523, #507

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
@WilfordGrimley @claude
feat: CardIllustrationVote — the artwork-grain vote model, its machin…
0bceff2 
…e writer, and the #523 invariant lock (#524) (#531)

* fix: remove dead import of renamed measure_bleed_diff_mm in image_evidence

53c1484 renamed measure_bleed_diff_mm -> compute_bleed_diff_mm in
local_fallback.py without leaving an alias. image_evidence.py still
imported the old name, raising ImportError at module import time for
every one of its ten importers (Stage C/D/E). This removes the dead
import and the now-unreachable fallback call at the old bleed_diff_mm
assignment, letting compute_bleed_diff_mm's own assignment stand as
the sole source of that field.

* test: remove orphaned test_artist_external_links.py

53c1484 added this test file for an MTGAC proxy view
(get_artist_external_links) and imports from cardpicker.views that
were never actually committed - the feature has an unstarted design.
The test aborted collection for the entire backend suite
(Interrupted: 1 error during collection) and was pushed unformatted
(isort + black both flagged it). Deleting it is safe and reversible:
the file remains in git history at 53c1484 and will be recovered
when the MTGAC integration is actually built. Not implementing the
missing view, adding skip/xfail markers, or merely reformatting -
those would leave a test asserting against code that does not exist.

* fix: Stage D illustration hot-path regressions + vote-write ordering/atomicity

`dispatch_micro_batch` is a streaming conveyor processing cards in 25-card
micro-batches; anything it calls must cost O(batch), never O(catalog)
(issues #458/#460). `_run_stage_d` calls `run_illustration_calculator` once
per micro-batch. The illustration calculator (added 2026-07-28) never carried
over the issue-#469 fixes that `local_calculate_verdicts.py` already has.

1. CandidateNameIndex cache bypass (local_illustration.py:468)
   `candidate_name_index = CandidateNameIndex()` — a direct constructor call
   whose own comment claimed "same cached pattern as
   `_get_cached_candidate_name_index()`". It was not: it bypassed the cache
   entirely. Cost before: a 113,224-row CanonicalCard scan, measured 1.48s,
   per micro-batch with at least one artist-OCR card. Now calls
   `local_calculate_verdicts._get_cached_candidate_name_index()`.

2. Unscoped materialization (local_illustration.py:385-394)
   Both join-key no-hit populations were wrapped in `list(...)`. Cost before:
   every join-key `is_no_match` CardPrintingTag row and every join-key no-hit
   CardScanLog row (CardScanLog is 2,093,147 rows live, append-only) pulled
   into process memory on every micro-batch, before any card_ids scoping.
   Now lazy querysets compiled into SQL subqueries, matching
   `_fallback_eligible_cards_queryset`.

3. card_ids applied late (local_illustration.py:211-253, 396-401)
   `_eligible_illustration_cards_queryset` took no `card_ids`; the caller
   filtered afterwards, so the CardScanLog exclusion subquery inside it
   compiled unscoped. Cost before: a full pass over 2,093,147 rows per
   micro-batch. Now mirrors `_eligible_cards_queryset:966-996` exactly,
   including `.filter(card_id__in=card_ids)` on the subquery, with issue
   #469's reasoning preserved.

4. IllustrationIndex rebuild (local_illustration.py:128-165, 411)
   Two catalog-wide CanonicalCard queries (one filtered on illustration_id,
   one UNFILTERED over every card with an artist) plus full-catalog dicts,
   constructed per invocation i.e. per micro-batch. Now memoized per worker
   process behind a version stamp, same shape as
   `_get_cached_candidate_name_index()`. The stamp adds a fifth term — the
   count of non-null `illustration_id` — because that column is BACKFILLED
   IN PLACE by `import_scryfall_printing_metadata`, an UPDATE that moves
   neither max pk nor row count. The no-op `.select_related(...)` chained
   before a `.values_list()` with traversal args was removed.

5. `already_voted` was structurally 0 (local_calculate_verdicts.py:1286-1293,
   1614-1620; local_illustration.py:532)
   `purge_stale_machine_votes` deletes by calculator FAMILY, which includes
   the caller's own current anonymous_id — so running it before
   `_split_new_printing_tag_votes` deleted exactly the rows the split then
   looked for. The counter read 0 in every deployment forever: the literal
   "zero forever would suggest the guard itself is dead code" failure
   `stage_e_dispatch.py:250-253` warns about. Split now runs first, and the
   purge is scoped to `new_votes` so a skipped card keeps the winner's row.
   Calculator-version self-overwrite (#519/#520) is preserved: the split
   checks the exact current anonymous_id, so a stale `-v1` row is never a
   collision and its card is still purged and overwritten.

6. Cancel-safety (same three sites)
   The purge is a DELETE and the insert a separate statement. A process
   killed between them — which this operator does deliberately, mid-flight —
   lost votes with nothing written back. Both are now inside one
   `transaction.atomic()`, via the new shared
   `_purge_and_write_printing_tag_votes` primitive.

Tests: +17 net. New coverage for item 3 (the CardScanLog subquery is
genuinely narrowed in the compiled SQL, and BULK mode is untouched), item 4
(process cache reuse, invalidation on insert and on in-place illustration_id
backfill, zero index builds for an empty micro-batch), item 6 (a failed
insert rolls the purge back), and items 1/5.

Three existing tests changed behaviour, all of them assertions added by
#519/#520 that codified the regression above ("Update two concurrent-dispatch
collision tests: purge changes 'skip' semantics to 'overwrite'"):
`test_local_calculate_verdicts.py` join-key and fallback collision tests and
`test_stage_e_dispatch.py`'s ledger assertion now expect `already_voted == 1`
and the winner's row surviving, which is the semantic
`local_lands_identify`'s equivalent test has asserted since PR #411.

Refs #458, #460, #469, #507, #519

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

* test: derive RSS-ceiling tests from RSS_MB_PER_WORKER_CEILING, not a hardcoded 512

70225df raised the RSS ceiling 512->768MB in operating_envelope.py but updated
zero tests. test_operating_envelope.py::test_trips_above_512mb_per_worker fed
a literal 512.1, which is now under the 768 ceiling, so check_envelope()
returned None and the trip-not-None assertion failed.
test_resolve_envelope_trip.py::test_acknowledging_one_trip_does_not_touch_another
cascaded from the same cause: it built its second trip with a literal 600.0,
also now under the ceiling, so no trip was ever created for it to acknowledge.

Both now derive the RSS value from RSS_MB_PER_WORKER_CEILING (already imported
and already used this way in test_operating_envelope.py's own
test_exactly_at_the_ceiling_does_not_trip) instead of a hardcoded number, so a
future ceiling change can't silently rot these tests the same way again.
Renamed test_trips_above_512mb_per_worker -> test_trips_above_rss_ceiling_per_worker
since its name no longer hardcodes the ceiling either.

No production code touched - operating_envelope.py and the 768 ceiling are
unchanged.

* fix: illustration calculator resolves a printing only at 1:1 (#525)

BEHAVIOUR CHANGE — reduces illustration vote coverage. Deliberately its own
commit so it can be evaluated or reverted independently of the performance
work in the previous commit.

THE DEFECT. `calculate_illustration_verdict` returned MULTIPLE `printing_pks`
from both terminal branches, and the runner emits one `CardPrintingTag` per
printing pk. One machine identity therefore voted simultaneously for several
MUTUALLY EXCLUSIVE printings of the same card:

  - N>1 illustrations (local_illustration.py:305-320): "union of printings
    across all illustrations, each at BASE_CONFIDENCE/N".
  - N==1 illustration (local_illustration.py:322-329): "vote every printing at
    full BASE_CONFIDENCE". Since illustration_id → printing is 1:N, this fired
    for ANY reprinted artwork — the common case, not an edge case.

The confidence spread does not mitigate it. `resolve_vote_weight(source,
anonymous_id)` takes no confidence argument and `VoteTuple` has no confidence
field, so confidence never reaches the tally: every row landed at full
`PRINTING_TAG_MACHINE_WEIGHT`. `cardprintingtag_unique_printing_vote` is on
(card, printing, anonymous_id), so all N rows persist. Where md5 identity-group
pooling is active they read as self-contradiction and get withheld; where it is
not, they all count for outcomes that cannot all be true.

THE FIX. A printing vote is cast ONLY when the surviving set resolves to
exactly one printing pk — exactly one illustration AND that illustration
mapping to exactly one printing. Every other case abstains with a CardScanLog
row.

TWO DISTINCT ABSTAIN REASONS, because only one of them is recoverable:

  - `multiple-illustrations` (N>1 illustrations): no single illustration
    identity exists. Counts only; no representative is invented.
  - `multiple-printings-one-illustration` (1 illustration, N printings): the
    illustration identity IS known with full confidence — only the printing
    choice is undetermined. `IllustrationVerdict` retains `illustration_id` and
    `candidate_printing_pks` so issue #524 (`CardIllustrationVote`) can persist
    it without re-deriving anything.

The retained values live in new verdict fields, NOT in `printing_pks`, so no
future reordering of the runner's loop can turn retained evidence into cast
votes. Nothing is persisted beyond the skip_reason string: no new model, no new
field, no migration. Persistence is #524 and separately owned — this abstain is
the seam it plugs into, not the intended end state.

MEASURABILITY. `IllustrationCalculatorResult` gains
`cards_abstained_ambiguous` and `printing_votes_withheld` (the exact number of
CardPrintingTag rows the pre-#525 code would have written). Both are counted in
dry-run mode, so the coverage figure is obtainable without a live write. The
audit sample carries the retained narrowing per abstained card.

Not changed, deliberately: `stage-d-illustration-v1`'s anonymous_id and version
string (#525 records that a bump is unsafe pending separate investigation);
`vote_consensus.py`/`printing_consensus.py` (confidence is NOT made
load-bearing — that is a separate owner decision and the consensus code carries
its own warning against it); the artist input, which stays OCR-evidence-derived
(#523).

Tests: one existing test asserted the defect and was rewritten, not quietly
changed — `test_multiple_illustrations_spreads_confidence` asserted
`skip_reason == ""`, two printing_pks and `confidence == BASE_CONFIDENCE / 2`;
it is now `test_multiple_illustrations_abstains_and_retains_no_identity`.
`test_single_illustration_votes_all_printings` was renamed (its fixture only
ever built one printing, so it never exercised the multi-printing path its name
described). New: 1-illustration-N-printings abstains and retains the
illustration; N>1 abstains and retains nothing; 1:1 casts exactly one vote at
BASE_CONFIDENCE; the two reasons are distinguishable; a printing reached twice
via two candidates sharing an artist is still 1:1; and five end-to-end runner
tests covering what actually reaches the DB.

Refs #525, #524, #523, #507

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

* feat: CardIllustrationVote + machine writer, read-side narrowing, #523 lock

Closes #524, closes #523. Stacked on #526 (fix/illustration-hotpath-regressions)
— must merge after it.

An `illustration_id` identifies an ARTWORK, and artwork-to-printing is 1:N (~2.2
printings per artwork across the catalogue), so identifying the artwork narrows
the printing but usually does not determine it. None of the four existing vote
tables can express "this card depicts illustration X", so when the illustration
calculator confidently identifies an artwork mapping to several printings, #526
makes it abstain and log a skip reason — establishing the identity with full
confidence and then discarding it. This closes that gap.

THE MODEL. `CardIllustrationVote(AbstractWeightedVote)` with card FK
(related_name="illustration_votes"), a plain indexed UUIDField `illustration_id`
(NOT an FK — there is no CanonicalIllustration table and this must not create
one), and `is_unknown`. Two constraints:

  - CheckConstraint illustration_id XOR is_unknown, mirroring
    `cardartistvote_artist_xor_unknown`.
  - UniqueConstraint on (card, anonymous_id), UNCONDITIONAL — no `condition=`.

That second one deliberately diverges from both sibling identity-vote models,
and the divergence is the point. CardPrintingTag keys on (card, printing,
anonymous_id) and CardArtistVote's artist branch on (card, artist,
anonymous_id); both rely on the submit VIEW deleting prior rows to enforce
one-vote-per-card, and CardArtistVote's own comment concedes its constraint is
"a safety net against a double-submit race, not the primary mechanism". Machine
writers using bulk_create never call that view — which is exactly how the
illustration calculator came to cast several mutually exclusive printing votes
under one identity (#525). An unconditional key makes the contradiction
unrepresentable rather than merely discouraged. The reasoning is in the model
docstring, citing #525, so nobody later "fixes" it to match its siblings.

THE MACHINE WRITER. `run_illustration_calculator` now writes at two independent
grains: a CardPrintingTag only at 1:1 (#526's rule, UNCHANGED and not weakened),
and a CardIllustrationVote whenever exactly ONE illustration was resolved
however many printings it maps to — i.e. the 1:1 cards plus the entire
`multiple-printings-one-illustration` population, so most of the coverage the
1:1 rule withholds at the printing grain is retained at the artwork grain
instead of discarded. At N>1 illustrations nothing is written at either grain
and #526's skip reason is untouched. The trigger is a truthiness check on
`verdict.illustration_id`, which #526 retained for exactly this consumer —
nothing is re-derived. Confidence is BASE_CONFIDENCE, not `verdict.confidence`
(which is confidence in the PRINTING, 0.0 on the abstain path, while this vote's
claim is about the artwork the calculator resolved with full confidence in both
cases). No anonymous_id or version string is bumped.

IDEMPOTENCE, CANCEL-SAFETY, AND THE CHANGED-ANSWER CASE.
`_split_new_illustration_votes`/`_purge_and_write_illustration_votes` follow
`_purge_and_write_printing_tag_votes`' established semantics: split/count BEFORE
any purge, purge scoped to the rows actually being written (never the whole
batch), purge + write inside one `transaction.atomic()`. They live here rather
than in local_calculate_verdicts.py because another worker is concurrently
generalizing that primitive; a comment marks them for unification in a
follow-up.

The one difference is load-bearing. The existing split matches on the KEY only.
Under an unconditional UNIQUE(card, anonymous_id) that makes a CORRECTED answer
permanently unlandable: a metadata refresh that changes the resolved
illustration would be seen as an existing row for the same key, counted as
already-voted, dropped from new_votes — so it never reaches the purge that would
have made room — and ignore_conflicts would swallow any attempt anyway. So this
split compares the illustration_id VALUE as well: an unchanged answer is a no-op
counted as already-voted, a changed answer is kept and overwrites. Comparison
normalises through uuid.UUID so the str-vs-UUID representation gap never reads
as a spurious change. Calculator-version self-overwrite (#519/#520) is preserved
and human votes are never purged.

THE READ HELPER. `printings_for_illustration(illustration_id,
candidate_printing_pks=None)` returns the CanonicalCard printings carrying an
illustration, joined through CanonicalPrintingMetadata. This is the narrowing
and it stays a READ — materialising it as implied printing votes is #525's
defect restated at a new grain. The candidate list intersects (never adds), so
per-batch callers pay batch-scale cost.

THE INVARIANT LOCK (#523). The calculator's artist input must come from OCR
evidence, never from artist votes: `illustration_id -> artist` is functional, so
an illustration answer can derive a CardArtistVote, and wiring that back in as
the calculator's artist input would close a self-confirmation loop. Before this
change there were no illustration votes at all, so the loop was unreachable —
this change is what makes it reachable, which is why the lock lands with it.
Adds the call-site comment #523 asks for plus four tests asserting on the SEAM
(the exact argument passed to `match_artist`), including two that assert no SQL
statement issued by the verdict path or the whole runner touches the
CardArtistVote table.

Also registers the model in admin.py, matching every sibling vote model.

Verification: `pytest .` from MPCAutofill/ — 2414 passed / 7 skipped / 0 failed
before, 2453 passed / 7 skipped / 0 failed after. black, isort, ruff and
mypy clean on every file touched; `makemigrations --check` reports no pending
changes; one migration added and it is the only one. Both subtle behaviours were
mutation-tested: reverting the split to a key-only comparison fails exactly the
two changed-answer tests and nothing else, and rewiring the artist input to read
CardArtistVote fails three of the four #523 tests.

Refs #507, #519, #520, #522, #523, #524, #525, #526

* migrations: renumber 0090_cardillustrationvote -> 0091, chained behind #532's 0090

PR #532 (feat/stream-full-catalog) independently authored
0090_stage_e_full_catalog_cursor against the same 0089 parent. Two migrations
sharing a parent with neither depending on the other leaves the cardpicker graph
with two leaf nodes, which Django rejects at migrate time ('Conflicting
migrations detected; multiple leaf nodes in the migration graph'). #532 was
green and mergeable first, so this one renumbers and chains behind it, giving a
single linear chain 0089 -> 0090_stage_e_full_catalog_cursor ->
0091_cardillustrationvote. #532's migration is not touched.

Purely a graph-ordering dependency: no schema or data relationship between the
two migrations. Operations are byte-identical to the 0090 version of this file.

ORDERING HAZARD, stated plainly: 0090_stage_e_full_catalog_cursor does not exist
on master yet (#532 is still open at time of writing), so until #532 merges this
branch's migration graph has an unresolved dependency and Django raises
NodeNotFoundError. #532 MUST merge before this PR. Verified against a scratch
worktree holding master + #532 + this branch - see the PR body.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
WilfordGrimley added a commit that referenced this pull request Jul 29, 2026
@WilfordGrimley @claude
fix: transactional purge+write at every remaining calculator vote site (
d1b457d 
#534)

* fix: remove dead import of renamed measure_bleed_diff_mm in image_evidence

53c1484 renamed measure_bleed_diff_mm -> compute_bleed_diff_mm in
local_fallback.py without leaving an alias. image_evidence.py still
imported the old name, raising ImportError at module import time for
every one of its ten importers (Stage C/D/E). This removes the dead
import and the now-unreachable fallback call at the old bleed_diff_mm
assignment, letting compute_bleed_diff_mm's own assignment stand as
the sole source of that field.

* test: remove orphaned test_artist_external_links.py

53c1484 added this test file for an MTGAC proxy view
(get_artist_external_links) and imports from cardpicker.views that
were never actually committed - the feature has an unstarted design.
The test aborted collection for the entire backend suite
(Interrupted: 1 error during collection) and was pushed unformatted
(isort + black both flagged it). Deleting it is safe and reversible:
the file remains in git history at 53c1484 and will be recovered
when the MTGAC integration is actually built. Not implementing the
missing view, adding skip/xfail markers, or merely reformatting -
those would leave a test asserting against code that does not exist.

* fix: Stage D illustration hot-path regressions + vote-write ordering/atomicity

`dispatch_micro_batch` is a streaming conveyor processing cards in 25-card
micro-batches; anything it calls must cost O(batch), never O(catalog)
(issues #458/#460). `_run_stage_d` calls `run_illustration_calculator` once
per micro-batch. The illustration calculator (added 2026-07-28) never carried
over the issue-#469 fixes that `local_calculate_verdicts.py` already has.

1. CandidateNameIndex cache bypass (local_illustration.py:468)
   `candidate_name_index = CandidateNameIndex()` — a direct constructor call
   whose own comment claimed "same cached pattern as
   `_get_cached_candidate_name_index()`". It was not: it bypassed the cache
   entirely. Cost before: a 113,224-row CanonicalCard scan, measured 1.48s,
   per micro-batch with at least one artist-OCR card. Now calls
   `local_calculate_verdicts._get_cached_candidate_name_index()`.

2. Unscoped materialization (local_illustration.py:385-394)
   Both join-key no-hit populations were wrapped in `list(...)`. Cost before:
   every join-key `is_no_match` CardPrintingTag row and every join-key no-hit
   CardScanLog row (CardScanLog is 2,093,147 rows live, append-only) pulled
   into process memory on every micro-batch, before any card_ids scoping.
   Now lazy querysets compiled into SQL subqueries, matching
   `_fallback_eligible_cards_queryset`.

3. card_ids applied late (local_illustration.py:211-253, 396-401)
   `_eligible_illustration_cards_queryset` took no `card_ids`; the caller
   filtered afterwards, so the CardScanLog exclusion subquery inside it
   compiled unscoped. Cost before: a full pass over 2,093,147 rows per
   micro-batch. Now mirrors `_eligible_cards_queryset:966-996` exactly,
   including `.filter(card_id__in=card_ids)` on the subquery, with issue
   #469's reasoning preserved.

4. IllustrationIndex rebuild (local_illustration.py:128-165, 411)
   Two catalog-wide CanonicalCard queries (one filtered on illustration_id,
   one UNFILTERED over every card with an artist) plus full-catalog dicts,
   constructed per invocation i.e. per micro-batch. Now memoized per worker
   process behind a version stamp, same shape as
   `_get_cached_candidate_name_index()`. The stamp adds a fifth term — the
   count of non-null `illustration_id` — because that column is BACKFILLED
   IN PLACE by `import_scryfall_printing_metadata`, an UPDATE that moves
   neither max pk nor row count. The no-op `.select_related(...)` chained
   before a `.values_list()` with traversal args was removed.

5. `already_voted` was structurally 0 (local_calculate_verdicts.py:1286-1293,
   1614-1620; local_illustration.py:532)
   `purge_stale_machine_votes` deletes by calculator FAMILY, which includes
   the caller's own current anonymous_id — so running it before
   `_split_new_printing_tag_votes` deleted exactly the rows the split then
   looked for. The counter read 0 in every deployment forever: the literal
   "zero forever would suggest the guard itself is dead code" failure
   `stage_e_dispatch.py:250-253` warns about. Split now runs first, and the
   purge is scoped to `new_votes` so a skipped card keeps the winner's row.
   Calculator-version self-overwrite (#519/#520) is preserved: the split
   checks the exact current anonymous_id, so a stale `-v1` row is never a
   collision and its card is still purged and overwritten.

6. Cancel-safety (same three sites)
   The purge is a DELETE and the insert a separate statement. A process
   killed between them — which this operator does deliberately, mid-flight —
   lost votes with nothing written back. Both are now inside one
   `transaction.atomic()`, via the new shared
   `_purge_and_write_printing_tag_votes` primitive.

Tests: +17 net. New coverage for item 3 (the CardScanLog subquery is
genuinely narrowed in the compiled SQL, and BULK mode is untouched), item 4
(process cache reuse, invalidation on insert and on in-place illustration_id
backfill, zero index builds for an empty micro-batch), item 6 (a failed
insert rolls the purge back), and items 1/5.

Three existing tests changed behaviour, all of them assertions added by
#519/#520 that codified the regression above ("Update two concurrent-dispatch
collision tests: purge changes 'skip' semantics to 'overwrite'"):
`test_local_calculate_verdicts.py` join-key and fallback collision tests and
`test_stage_e_dispatch.py`'s ledger assertion now expect `already_voted == 1`
and the winner's row surviving, which is the semantic
`local_lands_identify`'s equivalent test has asserted since PR #411.

Refs #458, #460, #469, #507, #519

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

* test: derive RSS-ceiling tests from RSS_MB_PER_WORKER_CEILING, not a hardcoded 512

70225df raised the RSS ceiling 512->768MB in operating_envelope.py but updated
zero tests. test_operating_envelope.py::test_trips_above_512mb_per_worker fed
a literal 512.1, which is now under the 768 ceiling, so check_envelope()
returned None and the trip-not-None assertion failed.
test_resolve_envelope_trip.py::test_acknowledging_one_trip_does_not_touch_another
cascaded from the same cause: it built its second trip with a literal 600.0,
also now under the ceiling, so no trip was ever created for it to acknowledge.

Both now derive the RSS value from RSS_MB_PER_WORKER_CEILING (already imported
and already used this way in test_operating_envelope.py's own
test_exactly_at_the_ceiling_does_not_trip) instead of a hardcoded number, so a
future ceiling change can't silently rot these tests the same way again.
Renamed test_trips_above_512mb_per_worker -> test_trips_above_rss_ceiling_per_worker
since its name no longer hardcodes the ceiling either.

No production code touched - operating_envelope.py and the 768 ceiling are
unchanged.

* fix: illustration calculator resolves a printing only at 1:1 (#525)

BEHAVIOUR CHANGE — reduces illustration vote coverage. Deliberately its own
commit so it can be evaluated or reverted independently of the performance
work in the previous commit.

THE DEFECT. `calculate_illustration_verdict` returned MULTIPLE `printing_pks`
from both terminal branches, and the runner emits one `CardPrintingTag` per
printing pk. One machine identity therefore voted simultaneously for several
MUTUALLY EXCLUSIVE printings of the same card:

  - N>1 illustrations (local_illustration.py:305-320): "union of printings
    across all illustrations, each at BASE_CONFIDENCE/N".
  - N==1 illustration (local_illustration.py:322-329): "vote every printing at
    full BASE_CONFIDENCE". Since illustration_id → printing is 1:N, this fired
    for ANY reprinted artwork — the common case, not an edge case.

The confidence spread does not mitigate it. `resolve_vote_weight(source,
anonymous_id)` takes no confidence argument and `VoteTuple` has no confidence
field, so confidence never reaches the tally: every row landed at full
`PRINTING_TAG_MACHINE_WEIGHT`. `cardprintingtag_unique_printing_vote` is on
(card, printing, anonymous_id), so all N rows persist. Where md5 identity-group
pooling is active they read as self-contradiction and get withheld; where it is
not, they all count for outcomes that cannot all be true.

THE FIX. A printing vote is cast ONLY when the surviving set resolves to
exactly one printing pk — exactly one illustration AND that illustration
mapping to exactly one printing. Every other case abstains with a CardScanLog
row.

TWO DISTINCT ABSTAIN REASONS, because only one of them is recoverable:

  - `multiple-illustrations` (N>1 illustrations): no single illustration
    identity exists. Counts only; no representative is invented.
  - `multiple-printings-one-illustration` (1 illustration, N printings): the
    illustration identity IS known with full confidence — only the printing
    choice is undetermined. `IllustrationVerdict` retains `illustration_id` and
    `candidate_printing_pks` so issue #524 (`CardIllustrationVote`) can persist
    it without re-deriving anything.

The retained values live in new verdict fields, NOT in `printing_pks`, so no
future reordering of the runner's loop can turn retained evidence into cast
votes. Nothing is persisted beyond the skip_reason string: no new model, no new
field, no migration. Persistence is #524 and separately owned — this abstain is
the seam it plugs into, not the intended end state.

MEASURABILITY. `IllustrationCalculatorResult` gains
`cards_abstained_ambiguous` and `printing_votes_withheld` (the exact number of
CardPrintingTag rows the pre-#525 code would have written). Both are counted in
dry-run mode, so the coverage figure is obtainable without a live write. The
audit sample carries the retained narrowing per abstained card.

Not changed, deliberately: `stage-d-illustration-v1`'s anonymous_id and version
string (#525 records that a bump is unsafe pending separate investigation);
`vote_consensus.py`/`printing_consensus.py` (confidence is NOT made
load-bearing — that is a separate owner decision and the consensus code carries
its own warning against it); the artist input, which stays OCR-evidence-derived
(#523).

Tests: one existing test asserted the defect and was rewritten, not quietly
changed — `test_multiple_illustrations_spreads_confidence` asserted
`skip_reason == ""`, two printing_pks and `confidence == BASE_CONFIDENCE / 2`;
it is now `test_multiple_illustrations_abstains_and_retains_no_identity`.
`test_single_illustration_votes_all_printings` was renamed (its fixture only
ever built one printing, so it never exercised the multi-printing path its name
described). New: 1-illustration-N-printings abstains and retains the
illustration; N>1 abstains and retains nothing; 1:1 casts exactly one vote at
BASE_CONFIDENCE; the two reasons are distinguishable; a printing reached twice
via two candidates sharing an artist is still 1:1; and five end-to-end runner
tests covering what actually reaches the DB.

Refs #525, #524, #523, #507

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

* fix: transactional purge+write at every remaining calculator vote site

PR #519/#520 wired `purge_stale_machine_votes` into eleven `bulk_create`
call sites across nine calculator modules. At each one the sequence is
DELETE prior votes -> compute -> INSERT new ones, with NO surrounding
transaction: a process killed between the DELETE and the INSERT destroys
committed votes with nothing written back. That is a routine event here,
not a disaster scenario - this project's operator kills long runs
mid-flight deliberately and a full-catalog pass takes hours.

PR #526 fixed three of those sites and introduced
`_purge_and_write_printing_tag_votes` for it. This change generalises
that primitive and applies it at the remaining twelve.

1. `cardpicker/vote_write.py` - the generalised primitive

   `purge_and_write_votes(model_class, rows, *, anonymous_id=None,
   target_field="card_id", ignore_conflicts=False)` parameterises #526's
   primitive over the four axes the remaining sites need: vote model
   (`CardPrintingTag`, `CardTagVote`, `CardArtistVote`,
   `PrintingTagVote`), purge identity, target field (`card_id` /
   `printing_id`), and `ignore_conflicts`. #526's three semantics are
   preserved exactly and are now implemented once:

     - split/count BEFORE purge (the purge deletes by calculator FAMILY,
       which includes the caller's own current anonymous_id, so purging
       first empties the table the already-voted guard then reads -
       `already_voted` structurally 0 in every deployment forever);
     - the purge is scoped to the rows ACTUALLY WRITTEN, never the
       pre-split batch. `rows` is simultaneously the purge scope and the
       insert payload, which makes the trap unexpressible at a call site:
       purging the full batch after a split deletes a skipped target's
       surviving winner row and never re-inserts it, destroying a
       committed vote with no error and no counter movement;
     - purge + `bulk_create` inside ONE `transaction.atomic()`.

   `anonymous_id=None` purges each row under its OWN family, grouped one
   purge call per distinct identity - reproducing by contract the
   hand-rolled grouping `local_identify_printing_tags`' pilot flush
   already did for its multi-engine batches.

   It is a separate leaf module rather than a function in
   `local_calculate_verdicts` because of an import cycle:
   `local_calculate_verdicts` imports from `local_identify_printing_tags`,
   which is itself one of the call sites, so it cannot import back -
   and `local_lands_identify`/`local_residual_classify` sit on that same
   chain, while a management command has no business importing a Stage D
   calculator just to write a vote batch. It is deliberately NOT in
   `models.py`: that file is the schema, this is write-path policy.

   `local_calculate_verdicts._purge_and_write_printing_tag_votes` is kept
   as the `CardPrintingTag`/`card_id` binding and now delegates, which
   leaves `local_illustration.py` completely untouched.

2. Twelve sites migrated

   local_layout_class_cast.run_layout_class_cast        CardTagVote
   local_detect_ai_art.run_ai_art_detector              CardTagVote
   local_residual_classify.run_frame_mismatch_recovery  CardArtistVote
   local_residual_classify.run_frame_mismatch_recovery  CardTagVote
   local_residual_classify.run_d0_sibling_artist_propagation
                                                        CardArtistVote
   local_identify_printing_tags.run_pilot flush         CardTagVote
   local_identify_printing_tags.run_pilot flush         CardPrintingTag
   local_identify_printing_tags.run_name_frequency_elimination flush
                                                        CardPrintingTag
   deductive_backfill.run_backfill flush                CardPrintingTag
   import_external_ip_tags positive pass                PrintingTagVote
   import_external_ip_tags negative pass                PrintingTagVote
   local_lands_identify.run_lands_identify              CardPrintingTag

   `ignore_conflicts` is preserved exactly per site - load-bearing
   crash-proofing where it was set, deliberately absent where it was not.
   `deductive_backfill` and `local_identify_printing_tags` already purged
   inside a chunked flush - better placed than the rest, but still
   untransacted, so a kill inside a chunk deleted that chunk's cards'
   previous votes and wrote no replacement: strictly worse than losing
   the chunk, because the pre-existing votes went with it.

3. `local_lands_identify` had the same ordering bug #526 fixed elsewhere

   It purged the RAW `votes_batch`, then split, then inserted - the purge
   deleted exactly the rows `_split_new_votes` then went looking for.
   That was MASKED rather than harmless: the batch also carries
   OCR_ANONYMOUS_ID votes (`_process_land_card`'s OCR-resolved branch) and
   the lands-family purge never touched those, so the one collision this
   module's tests exercise still counted - while a genuine
   LANDS_ANONYMOUS_ID collision went un-counted AND had its winner's row
   destroyed with nothing re-inserted, since the split drops the loser.
   Split now runs first and the purge is scoped to `new_votes`.

   The purge identity stays LANDS_ANONYMOUS_ID explicitly rather than
   per-row: widening it to every identity present would start purging the
   OCR family too, a behaviour change well outside an atomicity fix.

Tests: new `test_vote_write.py` covers the primitive across all four vote
models, both target fields, both `ignore_conflicts` settings, rollback,
empty-rows-purges-nothing, partial-collision scoping, human-vote
isolation, and both identity-grouping modes. Every migrated site gets a
rollback test through its real entrypoint (`bulk_create` patched to raise;
the pre-existing same-family row must survive), following #526's shape.
`local_lands_identify` additionally gets an ordering test: a racing
lands-family vote committed between selection and write is now counted in
`already_voted` and its row survives.

ONE EXISTING TEST CHANGED BEHAVIOUR, DELIBERATELY:
`test_local_identify_printing_tags.py::TestCheckpointing::
test_resume_after_a_simulated_kill_completes_the_remaining_cards`
simulated the kill by wrapping `bulk_create` and raising AFTER calling
through. Inside the new `transaction.atomic()` that raise happens INSIDE
the transaction and correctly rolls the whole pair back - so it would
have tested the new atomicity instead of the test's actual subject
(chunked checkpointing: a kill loses at most one chunk, and a plain
re-invocation resumes). The kill now fires from `verify_zero_resolutions`,
the last statement in the same `flush()`, i.e. genuinely "immediately
after the first flush committed". Same intent, correct write boundary.

No migration, no anonymous_id or version-string change. `models.py`,
`image_evidence.py`, `local_fallback.py`, `local_illustration.py`,
`printing_consensus.py`, `vote_consensus.py`, `views.py`, `urls.py`,
`question_feed.py`, `schema_types.py`, `reason_tags.py`, `frontend/` and
`schemas/` are untouched.

Refs #507, #519, #520, #526

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

* Drop the now-unused django.db.transaction import in local_calculate_verdicts

The merge of master reinstated #526's import; the atomic block it served
now lives in vote_write.purge_and_write_votes, so ruff F401 flags it.

* Correct an inaccurate inline comment in test_vote_write

The first purge_and_write_votes call in test_card_tag_vote_with_ignore_conflicts
purges a DIFFERENT family from the pre-existing row, so it deletes nothing - the
comment claimed the opposite. Assertions unchanged.

* Fix test fixture violating cardprintingtag_printing_xor_no_match

test_an_explicit_anonymous_id_purges_only_that_family built its no-match
victim row with a printing attached, which the model's own check
constraint forbids. printing=None; assertions unchanged.

---------

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

Reviewers

No reviews

Assignees

@WilfordGrimley WilfordGrimley

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

local_lands_identify --write crashes on duplicate local-ocr-v1 vote local_lands_identify dry-run prints would_cast=0 despite real would-cast counters

1 participant

@WilfordGrimley