Document Part 4's missing batch-flush checkpointing - #122
Merged
Conversation
WilfordGrimley
added a commit
that referenced
this pull request
Jul 29, 2026
…e bulk file we already parse Closes the open question in #600. Stacked on #601. WHAT WAS WRONG -------------- `deductive_backfill.select_d1_candidates` cast votes at confidence=0.95 on a documented guarantee - "cross-verified against Scryfall's own printings_count (not just 'our table happens to have one row')" - that no code implemented. The condition it ran, `catalogued_printings_count == 1`, is entailed by the `len(matches) == 1` test one line above it: that column counts OUR rows per oracle id, rows sharing an oracle id share a name, so a count above 1 means the card was already rejected. 137 D1 candidates before the condition, 137 after, measured against the live catalogue. WHAT THIS DOES -------------- 1. `CanonicalPrintingMetadata.scryfall_default_cards_printings_count` (migration 0099, nullable): how many rows Scryfall's `default_cards` export lists for this row's oracle card. Populated by `import_scryfall_printing_metadata` as a second `Counter`, over the `oracle_id` of rows it ALREADY parses - zero extra Scryfall requests. The per-card API alternative would be ~113k of them; the standing instruction is not to hammer their servers, and the existing diff/freshness check (`_is_fresh`) is untouched. 2. D1 gates on `catalogued_printings_count == scryfall_default_cards_...`. The entailed condition is deleted, not softened. 3. NULL means "Scryfall publishes no count for this row" and FAILS the gate. That covers the 81 rows with a NULL `canonical_id` - which are exactly the 81 bulk rows Scryfall itself ships with no `oracle_id`, and which the old code handed a fabricated `1`, i.e. an assertion of the very completeness being checked - and any oracle id absent from the bulk file (0 today). Ours > Scryfall's (the stale-file direction) also fails: equality, not >=. WHY THE NAME SAYS `default_cards` --------------------------------- `default_cards` is ~one row per printing, English-preferred. The count INCLUDES every distinct printing/variant Scryfall knows of - reprints, promos, showcase/borderless variants, tokens, and digital-only Arena/MTGO printings (9,354 digital rows of 116,254) - and EXCLUDES the per-language duplicates of a printing that also exists in English, which live only in `all_cards` (not downloaded here). It is not a paper-only count and not an all-languages count, and the field name, docstring and docs all say so. The defect being fixed was a second field acquiring a claim it could not support. MEASURED EFFECT (live catalogue, read-only, 2026-07-29) ------------------------------------------------------- Running the SHIPPED predicate against production, with the Scryfall side supplied from the on-disk bulk file: D1 candidates reaching the gate (len(matches) == 1): 137 passing the OLD condition (catalogued == 1): 137 passing the NEW gate (both known and equal): 137 excluded by the new gate: 0 THE REAL CHECK EXCLUDES 0 TODAY, and that is reported as the result rather than hunted past: the guarantee currently holds for every D1 candidate. It is not inert like its predecessor and it is not a stable property. Of the 14,893 normalised names with exactly one CanonicalCard row, 2 already disagree with Scryfall and are excluded the moment an eligible Card names either - counterfactually verified against the shipped predicate: 'Chandra, Chill of Compliance': catalogued=1 scryfall=2 -> EXCLUDED 'Tragic Trajectory': catalogued=1 scryfall=2 -> EXCLUDED 'Kusari-Gama': catalogued=1 scryfall=1 -> VOTE Both hand-checked against the bulk rows: Chandra has two `fra` printings (#212 and #297), we hold #212; Tragic Trajectory has `eoe` #122 and promo `pw26` #22, we hold the former. 82 of our 35,990 oracle ids hold fewer printings than Scryfall lists; 0 hold more; 0 are absent from the file. TESTS THAT CAN GO RED --------------------- `TestD1ScryfallPrintingCountGate`, including one end-to-end case that drives the REAL importer over a bulk file listing two printings of an oracle card we hold one of - the exact live production shape, which the deleted fixture-only test could not produce. Reverting the gate to `catalogued_printings_count == 1` turns 3 of them red (demonstrated). 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
…e bulk file we already parse (#614) Closes the open question in #600. Stacked on #601. WHAT WAS WRONG -------------- `deductive_backfill.select_d1_candidates` cast votes at confidence=0.95 on a documented guarantee - "cross-verified against Scryfall's own printings_count (not just 'our table happens to have one row')" - that no code implemented. The condition it ran, `catalogued_printings_count == 1`, is entailed by the `len(matches) == 1` test one line above it: that column counts OUR rows per oracle id, rows sharing an oracle id share a name, so a count above 1 means the card was already rejected. 137 D1 candidates before the condition, 137 after, measured against the live catalogue. WHAT THIS DOES -------------- 1. `CanonicalPrintingMetadata.scryfall_default_cards_printings_count` (migration 0099, nullable): how many rows Scryfall's `default_cards` export lists for this row's oracle card. Populated by `import_scryfall_printing_metadata` as a second `Counter`, over the `oracle_id` of rows it ALREADY parses - zero extra Scryfall requests. The per-card API alternative would be ~113k of them; the standing instruction is not to hammer their servers, and the existing diff/freshness check (`_is_fresh`) is untouched. 2. D1 gates on `catalogued_printings_count == scryfall_default_cards_...`. The entailed condition is deleted, not softened. 3. NULL means "Scryfall publishes no count for this row" and FAILS the gate. That covers the 81 rows with a NULL `canonical_id` - which are exactly the 81 bulk rows Scryfall itself ships with no `oracle_id`, and which the old code handed a fabricated `1`, i.e. an assertion of the very completeness being checked - and any oracle id absent from the bulk file (0 today). Ours > Scryfall's (the stale-file direction) also fails: equality, not >=. WHY THE NAME SAYS `default_cards` --------------------------------- `default_cards` is ~one row per printing, English-preferred. The count INCLUDES every distinct printing/variant Scryfall knows of - reprints, promos, showcase/borderless variants, tokens, and digital-only Arena/MTGO printings (9,354 digital rows of 116,254) - and EXCLUDES the per-language duplicates of a printing that also exists in English, which live only in `all_cards` (not downloaded here). It is not a paper-only count and not an all-languages count, and the field name, docstring and docs all say so. The defect being fixed was a second field acquiring a claim it could not support. MEASURED EFFECT (live catalogue, read-only, 2026-07-29) ------------------------------------------------------- Running the SHIPPED predicate against production, with the Scryfall side supplied from the on-disk bulk file: D1 candidates reaching the gate (len(matches) == 1): 137 passing the OLD condition (catalogued == 1): 137 passing the NEW gate (both known and equal): 137 excluded by the new gate: 0 THE REAL CHECK EXCLUDES 0 TODAY, and that is reported as the result rather than hunted past: the guarantee currently holds for every D1 candidate. It is not inert like its predecessor and it is not a stable property. Of the 14,893 normalised names with exactly one CanonicalCard row, 2 already disagree with Scryfall and are excluded the moment an eligible Card names either - counterfactually verified against the shipped predicate: 'Chandra, Chill of Compliance': catalogued=1 scryfall=2 -> EXCLUDED 'Tragic Trajectory': catalogued=1 scryfall=2 -> EXCLUDED 'Kusari-Gama': catalogued=1 scryfall=1 -> VOTE Both hand-checked against the bulk rows: Chandra has two `fra` printings (#212 and #297), we hold #212; Tragic Trajectory has `eoe` #122 and promo `pw26` #22, we hold the former. 82 of our 35,990 oracle ids hold fewer printings than Scryfall lists; 0 hold more; 0 are absent from the file. TESTS THAT CAN GO RED --------------------- `TestD1ScryfallPrintingCountGate`, including one end-to-end case that drives the REAL importer over a bulk file listing two printings of an oracle card we hold one of - the exact live production shape, which the deleted fixture-only test could not produce. Reverting the gate to `catalogued_printings_count == 1` turns 3 of them red (demonstrated). Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN 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.
Summary
Part 4's write run has no per-batch flush (bulk_create fires once, after the full 39,707-card loop) - deviating from Part 1's own run-cohort-safety rails. Documents the risk and the rule for successor runs (Part 4b, Part 5) in docs/lessons.md. No code change.
🤖 Generated with Claude Code