fix(engine): bind "can't be regenerated" through a delayed-trigger-wrapped Destroy#5717
Merged
matthewevans merged 2 commits intoJul 13, 2026
Conversation
…apped Destroy
Merieke Ri Berit ("When ~ leaves the battlefield or becomes untapped,
destroy that creature. It can't be regenerated.") failed to bind the
trailing regeneration-denial rider to the Destroy effect nested inside
its delayed trigger. The rider recognizer and its antecedent registry
only looked for a top-level Destroy/DestroyAll, so the clause fell
through to the standalone-clause handler instead, which spuriously
built an unrelated static "can't be regenerated until end of turn"
grant with no basis in the Oracle text, while the actual Destroy kept
cant_regenerate: false.
Note: the "leaves the battlefield or becomes untapped" OR-disjunction
itself (originally the suspected root cause, from misparse-backlog
category phase-rs#6) was verified independently to already be correct and is
untouched by this change.
Fixed by extending the existing CreateDelayedTrigger-unwrapping idiom
(already used for CopySpell riders) to the DestroyLike antecedent
class: a new recursive predicate/mutator pair, widened recognition and
registry-population call sites, and a corrected apply-arm mutation.
Confirmed via a 3-stage live-revert that all three edits are load-
bearing (recognition alone: no-op; +registry: still silently
no-ops; without the corrected apply-arm: panics instead of fixing).
Scope confirmed via card-data query: exactly 3 cards share this
CreateDelayedTrigger + can't-be-regenerated shape today. Gravebind and
Whippoorwill's unrelated delayed triggers (upkeep draw, death-exile)
correctly continue to produce their standalone grant, unaffected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbgwGxbU9NHN9kou9isp8K
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
rykerwilliams
added a commit
to rykerwilliams/phase
that referenced
this pull request
Jul 13, 2026
Parse changes introduced by this PR · 1 card(s), 1 signature(s) (baseline: main
|
Co-authored-by: Rajah James <rajah.james@grantek.com>
matthewevans
approved these changes
Jul 13, 2026
matthewevans
left a comment
Member
There was a problem hiding this comment.
Approve — the delayed-trigger destroy now carries Merieke Ri Berit’s regeneration denial through the production activation path.
🔴 Blocker
- None.
crates/engine/src/parser/oracle_effect/sequence.rs:3176-3202extends the existing delayed-trigger wrapper traversal to the nearestDestroy/DestroyAll, andcrates/engine/tests/integration/merieke_ri_berit_cant_regenerate.rs:94-161proves both disjunctive trigger branches bypass a live regeneration shield.
🟡 Non-blocking
- Gemini is quota-limited; the full current-head review, terminal-green required checks, and exact parse artifact supply the independent evidence.
✅ Clean
- The current parse artifact is exactly one intended change: remove Merieke’s spurious standalone
CantBeRegeneratedgrant. - Parser tests retain top-level destroy behavior and non-destroy delayed-trigger siblings; the runtime control confirms an ordinary
cant_regenerate: falsedestroy still consumes the shield. - The ported head has no workflow, instruction, generated-file, or scope-contamination changes.
Recommendation: approve and enqueue as bug.
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
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
Merieke Ri Berit ({W}{U}{B}, Legendary Creature, Ice Age): "Merieke Ri Berit doesn't untap during your untap step.\n{T}: Gain control of target creature for as long as you control Merieke Ri Berit. When Merieke Ri Berit leaves the battlefield or becomes untapped, destroy that creature. It can't be regenerated."
This card was originally suspected to be misparse-backlog root-cause category #6 ("disjunctive or-list collapsed to first branch") — but investigation found that framing is stale for the card's current state: the "leaves the battlefield or becomes untapped" disjunction is already correctly parsed and resolved at runtime (both branches of a
WhenNextEvent{trigger, or_trigger}delayed trigger are checked against a shared matcher — confirmed via an existing runtime test that exercises the same generic mechanism). No fix was needed there, and it's untouched by this PR.The real bug: the trailing "It can't be regenerated" sentence failed to bind to the
Destroyeffect nested inside the delayed trigger. Instead of settingcant_regenerate: trueon that Destroy, the parser fell through to a standalone-clause handler and spuriously built an unrelated static "can't be regenerated until end of turn" grant with no basis in the Oracle text — while the actual destroy keptcant_regenerate: false, meaning a stolen creature protected by a regeneration shield would incorrectly survive.Root cause was a 3-part gap: the antecedent-recognizer, its cached registry, and the apply-side mutator all only looked for a top-level
Destroy/DestroyAll, none of them descending into aCreateDelayedTriggerwrapper. Fixed by extending the existing "unwrap aCreateDelayedTriggerwrapper" idiom (already used forCopySpellriders elsewhere in the parser) to theDestroyLikeantecedent class — a new recursive predicate/mutator pair, widened recognition and registry-population call sites, and a corrected apply-arm mutation.Scope confirmed via a card-data query: exactly 3 cards share the "delayed trigger + can't-be-regenerated" shape today. Gravebind and Whippoorwill's own delayed triggers wrap unrelated effects (an upkeep draw, a death-exile) — their standalone "can't be regenerated" grant is correct as-is and is confirmed unaffected by this fix.
Test plan
cant_regenerate: trueon the nested Destroy, with no spurious static-ability grant anywhere in the chain.Destroy{cant_regenerate: false}still lets the regeneration shield save its victim — proving the shield mechanism itself is genuinely live, isolating the fixed card's result to thecant_regeneratebinding specifically.OnMiss::Ignoreno-ops silently); reverting only the apply-arm's mutator (restoring a direct match) instead panics — confirming all three edits are independently load-bearing and correctly wired together.cargo clippy -p engine --tests -- -D warningscleancargo fmt --all🤖 Generated with Claude Code
https://claude.ai/code/session_01XbgwGxbU9NHN9kou9isp8K