ship/p03 46 draw scope fixture - #5695
Merged
Merged
Conversation
matthewevans
commented
Jul 12, 2026
Member
- test(engine): regenerate integration_cards.json fixture from current parser
- fix(engine): enforce CR 121.2 draw_scope at the replacement consult seam
…parser The committed fixture predated `DrawReplacementScope` (Plan 03 thread 2a) and carried 7 Draw replacements with no scope, which is why the CR 121.2 `validate_draw_scope` assert could not be wired at the consult seam. Regenerated from `client/public/card-data.json` built at this commit (card-data-meta commit c8d4ea7, sha256 prefix 8e606c04223642ab), via `python3 scripts/gen-test-fixture.py`. Population 2428 -> 2655 cards: +227 added (cards referenced by tests added since the last regen) 165 changed (concurrent parser work merged since the last regen) 0 removed Of the 165 changed: 0 regressed (no card gained an Unimplemented node), 7 improved (animate dead, tin street gossip, infesting radroach, vanille cheerful l'cie, tail swipe, graceful takedown, sliver weftwinder each lost their last Unimplemented node), 158 are neutral payload churn from the merged M1 parser-IR work. All 7 previously-unscoped Draw replacements now carry a scope. Teferi's Ageless Insight is IndividualDraw (singular antecedent), not InstructionCount; Quantum Riddler is the sole InstructionCount row, matching the corpus census.
`ReplacementDefinition::validate_draw_scope` existed but was never called: a Draw replacement that forgot `.draw_scope(...)` was accepted silently, and the engine cannot infer at consult time whether a definition modifies the draw instruction's count (CR 121.2a) or replaces one individual draw (CR 121.2). Wire it as a `debug_assert!` in `replacement_definition_for_id`, the single point where the engine resolves a definition it is about to consult, so a producer that forgets the scope -- in card data, in a test constructor, or in a future runtime producer -- is caught. Release builds remain covered by `scripts/draw_replacement_census.py`, which cross-checks every declared scope against an independently derived one across the full corpus. Uses `Option::inspect` to observe the resolved definition without breaking the existing combinator chain. Watched red before green: against the pre-regen fixture this assert failed 8 integration tests (Abundance, Blood Scrivener, Jace Wielder of Mysteries, Sheoldred+Abundance) at replacement.rs with the CR 121.2 message, proving it fires on real production consult paths rather than a synthetic probe.
matthewevans
enabled auto-merge
July 12, 2026 21:16
Contributor
There was a problem hiding this comment.
Code Review
This pull request activates a debug_assert! validation for DrawReplacementScope on replacement definitions in crates/engine/src/game/replacement.rs using an .inspect block, replacing a previously commented-out assertion. It also updates the associated comments to clarify the validation's purpose and its coverage in release builds. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Parse changes introduced by this PR✓ No card-parse changes detected. |
matthewevans
added a commit
to jaytbarimbao-collab/phase
that referenced
this pull request
Jul 13, 2026
… parity rows it exposes (CR 603.3b + CR 603.4) (phase-rs#5732) `integration_cards.json` is a cached subset of the card-data export, but it was last FULLY regenerated at b9685cc (phase-rs#5695). Nineteen parser/types PRs merged since then; every fixture touch in between was surgical (phase-rs#5672 +1, phase-rs#5679 +2, phase-rs#5727 2 entries), so the parse values silently drifted. Regenerated from the export at 8c35dc5 (oracle-gen, MTGJSON 5.3.0+20260629). Population (json deep-equality, not line counts — the file is one line): committed 2658 entries -> 2726. 70 added, 2 removed, 44 changed values. Attribution of the 44 changed (causal: exports built at phase-rs#5720 / phase-rs#5717 / phase-rs#5730 and compared, NOT shape-guessing): phase-rs#5723 (P02-U3b shared condition grammar) ...... 3 archive trap, temple of civilization, thaumaton torpedo (all gained the comparator/lhs/rhs/qty/scope condition shape) phase-rs#5721 + phase-rs#5719 (where-X quantity channel + ..... 21 restriction grammar; both merged BEFORE phase-rs#5717 — merge order != PR-number order) bellowsbreath ogre, cryptex, deadly rollick, deflecting swat, desert, dread wanderer, esquire of the king, flesh, fraying sanity, gloomlake verge, great desert hellion, gutterbones, officious interrogation, once upon a time, potioner's trove, ribald shanty, rock jockey, second little pig, shifting woodland, snuff out, starport security phase-rs#5695..phase-rs#5720 no-regen window (bloc) ........... 20 Stale already at phase-rs#5720, so attributable to the 19-PR window above the b9685cc anchor, not to any single PR: alrund god of the cosmos, animal friend, approach of the second sun, cavernous maw, fblthp the lost, from father to son, hour of revelation, increasing vengeance, jodah the unifier, mana reflection, misty salon, puca's eye, ram through, reidane god of the worthy, secrets of the key, sevinne's reclamation, temple of the dead, the dining car, unleash the flux, valgavoth terror eater The 70 added keys are new test-source card references the generator collects; phase-rs#5729 (tests-only) contributed zero parse delta, as expected. Corrected premise: 44 entries are truly stale, not 7. Six of the seven originally reported reproduce; `osteomancer adept` is NOT stale (committed == fresh). The regen turns `ordering_parity_sweep` red, so the gate's evidence rows ship ATOMICALLY with it. Both rows are population entries, not ordering regressions: the sweep skips Unimplemented-bearing triggers, so a card only enters it once its parse binds. great desert hellion -> BATCH_GENUINE_ROWS. Its LTB Draw was Unimplemented until phase-rs#5721/phase-rs#5719 bound Intensity{Source}. Each co-departing Hellion draws off its OWN intensity but discards the SHARED hand, so the second trigger discards the cards the first just drew: with intensities a != b the final hand, graveyard and library differ by order. The members are not identical functions, so commutation genuinely fails and the new prompt is the CR 603.3b choice the legacy serde walk wrongly auto-ordered (CR 603.5: each "may" is chosen on resolution). planar collapse -> DOCUMENTED_OVER_PROMPT (L8-held family). New fixture key. Upkeep ObjectCount(Creature) >= 4 intervening-if x DestroyAll + self-Sacrifice: the first copy's sweep drives the census to 0, so the sibling's CR 603.4 re-check is false and it does nothing. Monotone and self-limiting — identical siblings commute up to relabeling, so the prompt is conservative, fail-closed and rules-correct. Neither row weakens the gate: both are direction-gated over-prompts (an under-prompt is never suppressible), and both are consumed by the ledger's exact-set asserts (over_prompt_hit 18->19, batch_genuine_hit 1->2), so a misclassification still trips the STRICT PROOF-GATE. Verification: engine lib 16481/16481 pass (was 16480 + 1 red); integration 2929/2929 pass. Co-authored-by: matthewevans <matthewevans@users.noreply.github.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.