fix(engine): preserve cast-payment stamps for CR 603.4 re-checks; never leak them across identities (Emptiness evoke riders)

[michiot05]

Tier: Frontier
Model: claude-fable-5
Thinking: high

Closes #5943.

Summary

Emptiness (ECL, {4}{W/B}{W/B}, Evoke {W/B}{W/B}) — "When this creature enters, if {W}{W} was spent to cast it, …" / "…if {B}{B} was spent to cast it, …". Discord report: evoking with two white or two black mana never fires the matching rider.

Root cause (runtime, not parser — the AST is faithful): colors_spent_to_cast is stamped correctly at payment finalization (including hybrid halves per CR 107.4e and the evoke alternative-cost path per CR 702.74a), but clear_post_collection_transients zeroed it on every object in every zone after each non-empty trigger batch. The CR 603.4 intervening-if re-check at resolution reads the live object while the source is still ExactLive — by then the tally is gone, so ManaColorSpent conditions silently failed. This kills the whole class: the Incarnation hybrid-ETB cycle, all Adamant cards (CR 207.2c), the Shadowmoor "if {C} was spent" family, and converge-style stack reads across intermediate batches.

The fix preserves the tally on Battlefield | Stack under the same guard that already protects cast_from_zone — the function's own documented treatment for cast facts that CR 603.4 re-checks. But preservation alone would leak payment provenance across object identities, so this PR gives all four cast-payment stamps (mana_spent_to_cast, colors_spent_to_cast, mana_spent_to_cast_amount, mana_spent_source_snapshots) one complete lifecycle through a new single authority, GameObject::clear_cast_payment_stamps():

1. Preserved on Stack/Battlefield for resolution-time reads (CR 603.4).
2. Cleared outside Battlefield|Stack at the next collection pass — a countered/fizzled spell loses its payment record, so counter-then-reanimate cannot resurrect phantom payment (mirrors cast_from_zone).
3. Cleared at battlefield exit (CR 400.7 — a re-entering permanent is a new object). This also fixes a live pre-existing leak: mana_spent_to_cast_amount survived blink/reanimation, so Satoru-class "no mana was spent to cast it" read stale values.
4. Never inherited by spell copies (CR 707.10: a copy is not cast and mana is not an object — the Dawnglow Infusion example; CR 707.12 for cast copies). The allow-raw-zone: …spell-copy birth markers enumerate all four stack-copy birth sites — copy_spell.rs, epic.rs, cast_copy_of_card.rs, paradigm.rs — and each now calls the shared authority; prepare.rs's exile-copy is out of class (later cast for real, re-stamped at payment). Without this, preserving the tally would have made every Fork/Twincast/Epic/paradigm copy wrongly satisfy "was spent" conditions the moment the original paid matching mana.

Relationship to #6340

#6340 (opened while this fix was in its mandated review pipeline) identifies the same root cause and takes the approach of never clearing colors_spent_to_cast at all, aligning it with mana_spent_to_cast_amount's current always-durable treatment. Respectfully submitting this PR for comparison because the unconditioned-durability approach inherits the exact identity leaks this PR closes and tests:

• Spell copies inherit the caster's tally — a copy of Emptiness would wrongly fire its riders (contradicts CR 707.10's Dawnglow example and the card's own ruling: "If this spell is copied, the copy will not have had any colors of mana paid for it"). Covered here by the copy_of_emptiness_fires_neither integration row plus per-site resolver tests at all four copy-birth sites.
• Blink/reanimation keeps phantom payment (CR 400.7) — covered here by blink_reanimate_clears_cast_payment_stamps and countered_spell_clears_cast_payment_stamps_before_reanimation, both revert-proven (left: 2, right: 0).
• The mana_spent_to_cast_amount precedent it aligns to is itself the source of the pre-existing Satoru-class blink leak that this PR fixes.

Implementation method (required)

Method: /engine-implementer — plan → /review-engine-plan (6 rounds to certification; the loop successively surfaced: the copy-inheritance regression, the epic/cast_copy_of_card sites, the paradigm site via the marker-sweep completeness proof, and a consolidation pass) → implement → /review-impl (clean; Maintainer-Simulation Gate PASS) → final read-only /review-impl on the committed head (returned one finding — the countered-spell path — fixed with the prescribed lifecycle alignment + regression, delta re-certified). Each step in a fresh agent context. Final head: 441c22b.

Gate A

./scripts/check-parser-combinators.sh → Gate A PASS head=441c22bfc333874df8b1e06c048c51b11910f5b9 base=094d4e5630de0659ee3d62b959cb851b12c3e632 (Gate G PASS). No files under crates/engine/src/parser/ are touched.

Anchored on

• crates/engine/src/game/triggers.rs clear_post_collection_transients — the existing !matches!(obj.zone, Battlefield | Stack) guard preserving cast_from_zone for exactly this CR 603.4 re-check reason; the fix extends the same guard to the payment tally.
• crates/engine/src/game/game_object.rs:2288 reset_for_battlefield_exit — the established CR 400.7 exit authority (cast_from_zone = None precedent) that now delegates stamp clearing to the shared helper.
• crates/engine/src/game/effects/copy_spell.rs copy-normalization block (CR 707.10, issue Sevinne's Reclamation — Sevinne's Reclamation copies itself when a copy of a flashback Sevinne's Reclamation resolves. #3283 cast_from_zone precedent) — the copy-birth reset idiom now applied uniformly via the single authority at all four marker-enumerated sites.

Verification

• cargo fmt --all clean; cargo clippy -p engine --tests 0 warnings (final head).
• Full engine suite: lib 17,506 passed / 0 failed; integration 3,735 passed / 0 failed.
• New integration file (emptiness_mana_spent_evoke_5943.rs, registered in main.rs) — 11 passed, all through the real cast pipeline with verbatim Oracle text: evoke {W}{W} with explicit trigger ordering (ETB-first, mid-drive reach guard proving the source is still on the battlefield at the re-check — the ExactLive path the bug killed), sac-first sibling (latched-snapshot path), evoke {B}{B}, hostile mixed {W}+{B} (neither fires; entered+sacrificed reach guards), hard cast, hostile intermediate-batch row, hostile copy row, blink row, counter-then-reanimate row, convoke boundary pin (CR 702.51a), and a Dawnglow-class resolution-side AbilityCondition row.
• New unit tests — 8: zone-guard both arms (battlefield survives / graveyard cleared, identical stamps), exit-reset clears all four + cast_from_zone pin, helper contract pin, and per-site copy-birth resets in copy_spell.rs, epic.rs (file's first test module), cast_copy_of_card.rs, paradigm.rs — each revert-fails its own site.
• Stay-green guards: Satoru pin, Adamant filter (6), convoke tests (46), colors_spent_to_cast_persists_through_zone_change_replacement, full mana_spent filter (61), all copy/paradigm/epic/cast-copy suites.
• Fails before, passes after (tests written first): evoke-WW, hard-cast, intermediate-batch and Dawnglow rows all red pre-fix (Bears stranded in graveyard — the reported bug); copy row red under a seam-3 revert probe with both Bears returning (the inherited-tally failure); counter-reanimate red under a branch revert with left: 2, right: 0. All green on the final head.
• CR annotations: every number grep-verified against docs/MagicCompRules.txt before writing — CR 601.2h (:2472), 603.4 (:2592), 603.3b (:2586), 107.4e (:504), 400.7 (:1950), 702.74a (:4556), 707.10 (:5666), 707.10f (:5682), 707.12 (:5689), 608.3f (:2836), 702.50a (:4391), 702.51a (:4397), 207.2c (:1497). Zero UNVERIFIED.

Claimed parse impact

None. Engine-only: no parser files, no serialized AST shapes, no coverage change (Emptiness's parse was already faithful; its unrelated has_gaps facet is untouched). The parse-diff artifact should report no card-parse changes — consistent with a pure runtime-lifecycle fix. (A local regen diff against the pre-change baseline showed only upstream MTGJSON corpus drift — 13 new cards + set-code additions — with zero entries touching any payment-related field.)

Scope Expansion

None. Deliberately excluded (noted as optional follow-ups): deduplicating casting.rs's two payment-finalization clear blocks through the new helper; the adjacent reset_for_battlefield_entry hygiene gap documented in l02_bb6.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Fixed cast-payment tracking hygiene so spell copies, upkeep copies, paradigm copies, and cast-copy effects always start from a clean “no payment” baseline.
  • Corrected cast-payment stamp clearing/preservation across battlefield exits and intervening-if re-checks, ensuring mana-spent conditions evaluate using the correct latched/event context.

• Tests

  • Added unit and integration regression coverage for copy behaviors and complex mana-spent/{Emptiness} interactions, including blink/re-entry, countered spells, and convoke.

[coderabbitai]

Warning

Review limit reached

@matthewevans, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 34 seconds

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5e32ef74-7748-41bb-b58f-1672f2537219

📥 Commits

Reviewing files that changed from the base of the PR and between 5972cd7 and 5c7bb86.

📒 Files selected for processing (3)

• crates/engine/src/game/effects/copy_spell.rs
• crates/engine/src/types/game_state.rs
• crates/engine/tests/integration/main.rs

📝 Walkthrough

Walkthrough

Cast-payment tracking is centralized in GameObject, preserved during relevant Battlefield/Stack trigger checks, cleared for copied objects and zone transitions, and validated through unit and Emptiness integration tests.

Changes

Cast-payment stamp lifecycle

Layer / File(s)	Summary
Centralized cast-payment stamp lifecycle crates/engine/src/game/game_object.rs, crates/engine/src/types/game_state.rs	Adds centralized clearing and provenance snapshots for cast-payment stamps, applies clearing during battlefield exit, and tests lifecycle behavior.
Copy creation resets payment stamps crates/engine/src/game/effects/{cast_copy_of_card,copy_spell,epic,paradigm}.rs, crates/engine/src/game/effects/mod.rs	Copy paths clear inherited cast-payment metadata, with tests confirming copies reset stamps while originals retain them.
Zone-aware cleanup and trigger-source reads crates/engine/src/game/triggers.rs, crates/engine/src/game/quantity.rs, crates/engine/src/game/effects/delayed_trigger.rs	Post-collection cleanup preserves Battlefield/Stack stamps, clears stamps elsewhere, and resolves mana-spent conditions from live or latched trigger-source records.
Emptiness and payment-state regressions crates/engine/tests/integration/emptiness_mana_spent_evoke_5943.rs, crates/engine/tests/integration/main.rs	Adds integration coverage for Emptiness ordering, spell copies, intermediate collections, reanimation, countered spells, convoke, stack tally reads, and blinked trigger provenance.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CastSpell
  participant GameObject
  participant TriggerCollection
  participant TriggerResolver
  participant EffectResolution
  CastSpell->>GameObject: record cast-payment stamps
  TriggerCollection->>GameObject: preserve Battlefield/Stack stamps
  TriggerResolver->>GameObject: read live or latched payment provenance
  EffectResolution->>GameObject: evaluate ManaColorSpent
  EffectResolution->>GameObject: clear stamps on copies and exited objects

Loading

Possibly related PRs

• phase-rs/phase#6340: Updates related cast-payment tally preservation in clear_post_collection_transients.

Suggested reviewers: matthewevans, lgray

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main fix: preserving cast-payment stamps for Emptiness re-checks while preventing identity leaks.
Linked Issues check	✅ Passed	The changes address #5943 by making Emptiness’s W/W and B/B evoke riders read preserved cast-payment data during the CR 603.4 re-check.
Out of Scope Changes check	✅ Passed	The broader stamp-lifecycle and copy-handling edits still support the same cast-payment provenance fix and are not obviously unrelated.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[matthewevans]

[MED] Battlefield exit clears source-qualified payment provenance needed by a pending trigger. Evidence: game_object.rs line 2340 now calls clear_cast_payment_stamps(), which clears mana_spent_source_snapshots; snapshot_for_zone_change copies only bool/color/total fields into TriggerSourceContext at game_object.rs lines 1839-1841; and QuantityRef::ManaSpentToCast with FromSource reads only the live object vector in game/quantity.rs lines 1555-1572. Why it matters: if a permanent with an ETB rider that asks how much mana from a Treasure/Cave/artifact source was spent to cast it leaves before its intervening-if/effect resolves, the exact source context is latched but has no source-payment vector, and the newly cleared object reads as zero. CR 400.7d expressly permits a permanent ability to reference mana spent to cast the spell that became it. This avoids stale reanimation identity only by losing legitimate resolving-trigger provenance. Suggested fix: preserve the source-payment snapshots in the exact trigger-source/latch representation (and route FromSource resolution through that representation), while continuing to clear the new incarnation at the zone-change boundary; add a runtime fixture that sacrifices/blinks the ETB source before the source-qualified rider resolves.

Maintainer hold: this review is changes-requested. Current-head CI still has Rust lint/tests/card-data pending; CodeRabbit is still processing and has posted no findings; the required coverage-parse-diff artifact is absent, so the engine-only no-parse-change claim cannot yet be admitted. Security scan was clean. I also found open #6340 for the same issue/root cause; these alternatives must not both be enqueued.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/engine/src/game/game_object.rs`:
- Around line 2401-2406: Reset phyrexian_life_paid in
GameObject::clear_cast_payment_stamps and extend its default-state test. In
crates/engine/src/game/effects/cast_copy_of_card.rs:194-199,
copy_spell.rs:83-86, epic.rs:138-141, and paradigm.rs:196-198, extend each
regression test to verify copied objects or spells clear inherited
Phyrexian-life payment, preserving Comprehensive Rules behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: db98c503-b03e-485e-90d3-63e8f7efca41

📥 Commits

Reviewing files that changed from the base of the PR and between 7195a8e and 441c22b.

📒 Files selected for processing (9)

• crates/engine/src/game/effects/cast_copy_of_card.rs
• crates/engine/src/game/effects/copy_spell.rs
• crates/engine/src/game/effects/epic.rs
• crates/engine/src/game/effects/mod.rs
• crates/engine/src/game/effects/paradigm.rs
• crates/engine/src/game/game_object.rs
• crates/engine/src/game/triggers.rs
• crates/engine/tests/integration/emptiness_mana_spent_evoke_5943.rs
• crates/engine/tests/integration/main.rs

[github-actions]

Parse changes introduced by this PR

✓ No card-parse changes detected.

[michiot05]

Fixed on head d40486c88 — your CR 400.7d finding is exactly right, and the latch was carrying three of the four cast-payment stamps but not the source-payment vector.

The fix. TriggerSourceContext now carries mana_spent_source_snapshots (stamped in snapshot_for_zone_change beside mana_spent_to_cast_amount/colors, so it's captured at trigger collection — before the zone exit). resolve_mana_spent_to_cast_metric takes an Option<&TriggerSourceContext> and reads Total/DistinctColors/FromSource through source_read — the same live-vs-latched authority check_trigger_condition's ManaColorSpent/ManaSpentCondition arms already use — gated on trigger_source.identity.reference.object_id == cast_object. So a departed self-referential source resolves Latched (reads the snapshots), while TriggeringSpell/AbilityTarget scopes that reference a different object keep reading live (the gate misses, resolve_event_scoped_ref also passes None). The new incarnation is still cleared at the zone boundary per CR 400.7 — the mid-state guard in the fixture asserts the re-entered object carries no snapshots.

Regression. marut_treasure_rider_survives_blink_via_latched_snapshots (verbatim Marut + Cloudshift): cast Marut paying 2 Treasure-tagged mana (reach-guard: exactly 2 snapshots latched at commit), resolve so the ETB rider is on the stack, blink Marut with Cloudshift in response, then resolve the rider → 2 Treasures via the latch. Revert the latch routing and it reads 0 (the bug you found); the resolved count being exactly 2, not 4, also confirms the intervening-if re-check is latch-routed and the re-entered incarnation spawned no second rider. Plus a snapshot_for_zone_change_latches_cast_payment_stamps unit pin.

Verification on d40486c88: Gate A + G PASS; cargo clippy -p engine --tests clean; emptiness integration 12/12, mana_spent 61, copy/cast_copy suites 65, quantity units 653 — all green. CR 400.7d / 405.5 / 603.4 grep-verified. Serde is additive (skip_serializing_if = "Vec::is_empty", matches sibling fields). Engine-only, no parse-diff impact — the coverage-parse-diff artifact should confirm zero card-parse changes once CI reposts it for this head.

Model: claude-opus-4-8

[matthewevans]

Blocked — the current head loses triggering-spell payment provenance after a countered spell changes zones.

🔴 Blocker

[HIGH] Preserve payment facts for distinct sources that trigger on a spell. clear_post_collection_transients now clears mana_spent_to_cast_amount and the source snapshots for every object outside the battlefield or stack (crates/engine/src/game/triggers.rs:4652-4664). But TriggeringSpell quantity resolution reads the event object's live record without a latch (crates/engine/src/game/quantity.rs:1562-1576); the new latch only applies when the trigger source and cast object are identical (:1602-1622). Countering a spell before an Aetherflux Conduit-style trigger resolves therefore changes the recorded amount to zero. origin/main deliberately preserved this amount in the same cleanup path.

Why it matters: the trigger refers to the specific spell cast, so its amount must survive that spell leaving its expected zone. CR 608.2h requires use of last-known information when the specified object is no longer in the expected public zone.

Suggested fix: snapshot the triggering spell's payment facts in the trigger event/pending context and route TriggeringSpell through that event authority (or retain those facts until all event consumers resolve). Add a runtime Aetherflux Conduit regression that counters the spell before its trigger resolves.

✅ Clean

• The source-qualified latch and the copied-spell cleanup address the earlier scoped feedback; this remaining issue is the distinct-source event path.

Recommendation: request changes; retain the latch and repair event-scoped triggering-spell provenance.

[matthewevans]

Approved after the current-head review.

• Payment provenance remains source-qualified and is cleared for every spell-copy birth path.
• The Compleated regression covers the runtime resolution path; the copy enters with five loyalty rather than inheriting paid-life history.
• The merge refresh preserves the reviewed 12-file PR diff; the current CodeRabbit status has no actionable finding.

[matthewevans]

Approved after the current-head fixture correction.

• The copy-payment reset path is unchanged and remains covered by all five stamp assertions.
• The runtime assertion now uses the established untargeted permanent-spell fixture, so it reaches stack resolution and battlefield entry instead of correctly fizzling an untargeted damage spell.