fix(engine): keep a "return to hand" card as the referent for "that card" (#6486) by philluiz2323 · Pull Request #6561 · phase-rs/phase

philluiz2323 · 2026-07-23T20:14:22Z

Summary

Fixes #6486. Volcanic Vision — "Return target instant or sorcery card from your graveyard to your hand. Volcanic Vision deals damage equal to that card's mana value to each creature your opponents control. Exile Volcanic Vision." — returned the card but dealt no damage.

Implementation method (required)

Produced via the /engine-implementer pipeline
Not /engine-implementer — explain why below

Single-condition runtime fix to one referent helper, root-caused from a parse dump and covered by a card-level cast test through the real pipeline plus the full --lib regression. No parser, targeting, or effect-shape changes.

Root cause

The spell parses correctly — the damage sub-effect's amount is the Demonstrative ObjectManaValue { scope: Demonstrative } ("that card's mana value"). That resolves against the earlier-instruction referent (effect_context_object), which parent_referent_context_from_events derives from the parent effect's ZoneChanged events via moved_object_context_from_events. That helper only captured moves to a public zone:

} if is_public_zone(*to) => Some(...)   // Hand and Library excluded

Volcanic Vision returns the card to hand (hidden), so no referent was bound, that card's mana value resolved to 0, and every opponent creature took 0 damage.

Fix

CR 608.2c: a later instruction may refer to an object an earlier instruction returned to hand — its identity was established in the public source zone (graveyard), and the reference reads the move-time last-known mana value, so the hidden destination is immaterial. Capture the moved referent for a move to hand when the source zone is public:

} if is_public_zone(*to) || (*to == Zone::Hand && is_public_zone(*from_zone)) => Some(...)

A move from a hidden source (a draw: library → hand) establishes no such referent and is excluded.
Library destinations stay excluded entirely (a shuffle/reorder loses the object's identity).

This is a building-block fix to the shared referent helper, so it covers the whole "return/move a card to hand from a public zone, then reference that card" class rather than one card.

CR references

CR 608.2c — later instructions may refer to an object an earlier instruction acted on; the reference reads last-known information.

Verification

cargo test -p engine --lib → 17591 passed, 0 failed (the referent helper is shared across many cards).
New card-level cast test (volcanic_vision_deals_returned_cards_mana_value_after_return_to_hand) drives the real pipeline: with a mana-value-3 instant in the graveyard, each opponent creature takes 3 damage, the returned card is in hand, and Volcanic Vision is exiled. Fails (0 damage) if the referent fix is reverted.
cargo fmt --all clean.

Summary by CodeRabbit

Bug Fixes
- Fixed referent snapshot handling for zone changes so effects keep the correct move-time mana value when cards end up in a hidden hand zone (including relevant public→hand scenarios).
- Corrected Volcanic Vision so delayed damage matches the returned spell’s move-time mana value even after it moves into a hidden hand zone.
Tests
- Added regression and unit coverage for CR 608.2h “that card’s mana value” behavior across multiple parent-referent binding scenarios.

…ard" (phase-rs#6486) Volcanic Vision ("Return target instant or sorcery card from your graveyard to your hand. Volcanic Vision deals damage equal to that card's mana value to each creature your opponents control. Exile Volcanic Vision.") returned the card but dealt no damage. The spell parses correctly — the damage sub-effect's amount is the Demonstrative "that card's mana value". That resolves against the earlier-instruction referent (`effect_context_object`), which `parent_referent_context_from_events` derives from the parent effect's `ZoneChanged` events via `moved_object_context_from_events`. That helper only captured moves to a PUBLIC zone, so the graveyard->HAND return bound no referent, "that card's mana value" resolved to 0, and every opponent creature took 0 damage. CR 608.2c: a later instruction may refer to an object an earlier instruction returned to hand — its identity was established in the public source zone (graveyard), and the reference reads the move-time last-known mana value, so the hidden destination is immaterial. Capture the moved referent for a move to hand when the SOURCE zone is public. A move from a hidden source (a draw, library -> hand) establishes no such referent and is excluded; library destinations stay excluded entirely (a shuffle loses identity). Verified: cargo test -p engine --lib -> 17591 passed, 0 failed. New card-level cast test drives Volcanic Vision through the real pipeline: with a mana-value-3 instant in the graveyard, each opponent creature takes 3 damage, the card is in hand, and Volcanic Vision is exiled.

superagent-security · 2026-07-23T20:14:31Z

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

coderabbitai · 2026-07-23T20:14:50Z

📝 Walkthrough

Walkthrough

ZoneChanged snapshot creation now covers cards moved from public zones to hidden hands. Volcanic Vision and CR 608.2h tests verify that qualifying moves preserve move-time referent data.

Changes

Zone-change snapshot tracking

Layer / File(s)	Summary
Snapshot handling and regression coverage `crates/engine/src/game/effects/mod.rs`	`CostPaidObjectSnapshot` now captures public-source moves to `Zone::Hand`, and Volcanic Vision damage uses the returned card’s move-time mana value.
Referent binding semantics `crates/engine/src/game/effects/mod.rs`	Tests cover Library-to-Hand, Graveyard-to-Hand, and multiple qualifying moves under CR 608.2h referent rules.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

phase-rs/phase#6350: Both changes modify CostPaidObjectSnapshot and referent construction for GameEvent::ZoneChanged.

Suggested reviewers: matthewevans, lgray, andriypolanski, claytonlin1110

🚥 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 accurately summarizes the main fix: preserving the referent for a card returned to hand.
Linked Issues check	✅ Passed	The changes address `#6486` by making Volcanic Vision use the returned card’s mana value and adding tests for the referent behavior.
Out of Scope Changes check	✅ Passed	The added rules comments and helper-level tests stay aligned with the same referent-handling fix and do not appear unrelated.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

github-actions · 2026-07-23T20:30:06Z

Parse changes introduced by this PR

✓ No card-parse changes detected.

matthewevans · 2026-07-23T20:35:31Z

Right seam, right shape, real bug — two small fixups before this enqueues: the CR annotation cites the wrong rule for the clause it implements, and the widened predicate has no building-block test pinning its boundaries.

Reviewed at head 30b5b361a8fa6c3cc9859317fa3c4838c93aee8d.

Premise verified against Scryfall (/cards/named?exact=Volcanic Vision), verbatim:

Return target instant or sorcery card from your graveyard to your hand. Volcanic Vision deals damage equal to that card's mana value to each creature your opponents control. Exile Volcanic Vision.

Mana cost {5}{R}{R}, Sorcery. The PR body quotes the card accurately, and the bug is real: with no referent bound, ObjectManaValue { scope: Demonstrative } resolves to 0 — confirmed by the existing contract comment at crates/engine/src/game/quantity.rs:344, "object_id_for_scope returns None for them ... would silently read 0 every layer tick."

🔴 Blocker

None.

🟡 Non-blocking

1. The CR annotation cites CR 608.2c, but the rule that authorizes this change is CR 608.2h — and the guard being relaxed encodes CR 400.7j.

crates/engine/src/game/effects/mod.rs:1856

} if is_public_zone(*to)
    // CR 608.2c: a later instruction may refer to an object an earlier
    // instruction returned to hand, provided its identity was established
    // in a PUBLIC source zone — Volcanic Vision (...). The reference reads the
    // move-time LKI snapshot, so the hidden destination is immaterial.
    || (*to == Zone::Hand && is_public_zone(*from_zone)) =>

Grep-verified against docs/MagicCompRules.txt. CR 608.2c (line 2793) reads:

The controller of the spell or ability follows its instructions in the order written. However, replacement effects may modify these actions. In some cases, later text on the card may modify the meaning of earlier text (for example, "Destroy target creature. It can't be regenerated" ...)

That is instruction ordering and later-text-modifies-earlier-text. It says nothing about source-zone publicity or last-known information, so it does not support the two propositions the comment actually makes.

CR 608.2h (line 2806) states the implemented predicate almost verbatim:

If the effect requires information from a specific object, including the source of the ability itself, the effect uses the current information of that object if it's in the public zone it was expected to be in; if it's no longer in that zone, or if the effect has moved it from a public zone to a hidden zone, the effect uses the object's last known information.

And CR 400.7j (line 1970) is precisely why the original guard was public-only:

If an effect causes an object to move to a public zone, other parts of that effect can find that object. If the cost of a spell or ability causes an object to move to a public zone, that spell or ability's effects can find that object.

So the rules story for this fix is exact and clean: CR 400.7j governs findability and is public-only; CR 608.2h supplies the last-known-information path for the public→hidden move, which is the Volcanic Vision case word for word. This is the same argument this repo already litigated and landed for attached_to_referent — see the doc comment quoting CR 608.2h at crates/engine/src/game/filter.rs:880-895.

CR 608.2c is not a hallucinated number and it is the cascade's house citation for anaphora generally, so keep it if you like; the gap is that the one rule making this change defensible is missing.

Fix: annotate // CR 608.2h + CR 400.7j: ... (the compound + form is house convention) and quote the "moved it from a public zone to a hidden zone" clause.

2. No building-block test pins the widened predicate's boundaries — most importantly the draw exclusion.

crates/engine/src/game/effects/mod.rs:1849-1865 (predicate); the only new test is card-level, at :11270.

After this change, is_public_zone(*from_zone) is the single thing standing between the engine and every draw in the game binding an anaphoric referent. Library → Hand is excluded only because is_public_zone (:1948) happens to list Library alongside Hand. That is correct today and entirely untested. A future edit to is_public_zone, or a refactor of this predicate, flips a silent, pool-wide wrong answer with nothing red to catch it.

The repo bar is explicit here — "Test the building block, not the special case." The neighbouring helpers already have exactly these unit tests (stack_pushed_parent_referent_requires_singular_copy at :11399, duplicate_tap_events_for_same_creature_still_capture_single_referent at :11434), so the pattern is sitting right there.

Fix: three cheap unit tests on parent_referent_context_from_events, in the style of the siblings above:

ZoneChanged { from: Some(Zone::Library), to: Zone::Hand } ⇒ is_none() — a draw establishes no referent
ZoneChanged { from: Some(Zone::Graveyard), to: Zone::Hand } ⇒ Some(id) — the new arm at helper level
graveyard→hand plus battlefield→graveyard in one span ⇒ is_none() — the singular guard now has a wider surface to trip on

3. The blast radius is wider than the PR body describes: every bounce now binds a moved-referent.

crates/engine/src/game/effects/mod.rs:1865

The body frames this as the "return/move a card to hand from a public zone" class, which reads as graveyard/exile recursion. The dominant newly-captured population is actually Battlefield → Hand — ordinary bounce. Battlefield is public, so every Return target creature to its owner's hand now binds a referent where none bound before. Since moved_object_context_from_events outranks both the tapped and the damaged referents in the cascade (:1675-1716), a parent effect that bounces one permanent and taps or damages another now binds the bounced one instead.

Evidence this is benign in practice, and worth crediting: the existing bounce_followup_draws_when_caster_controlled_parent_target (:15046) drives exactly this Battlefield→Hand class through resolve_ability_chain and still passes, the parse-diff sticky reports "✓ No card-parse changes detected", and both Rust test shards plus the card-data job are green at this head.

Fix: name the bounce class in the code comment so the next reader sizes the surface correctly. A precedence test (bounce + tap in one span binds the bounced object) would pin it, though the existing coverage makes this optional.

4. Test fixture uses a real card name with a fabricated mana cost.

crates/engine/src/game/effects/mod.rs:11290

.add_spell_to_graveyard(P0, "Fire Blast", true)
.with_mana_cost(ManaCost::generic(3))

Real Fire Blast is {4}{R} (mana value 5), and it is an instant with an alternative cost rather than a generic-3 spell. Nothing breaks — the fixture never consults a card database — but a synthetic name avoids a reader diffing the fixture against the real card.

5. Process note (not a code defect). The PR body is missing the required AI-contributor template sections (Files changed, Track, LLM, Anchored on, Gate A, Final review-impl) and leaves the implementation-method checkbox unticked. The admission gate is in audit mode so this is advisory today, and ai-template-gap is this account's most recurrent tracked signal (26 occurrences, 8 distinct PRs in window). Worth closing out as a habit.

✅ Clean

Right seam, and the shared helper was extended rather than forked. The fix lands on moved_object_context_from_events, the existing member of the referent cascade that already owns zone-change-derived referents, alongside sacrificed_object_context_from_events / stack_pushed_object_context_from_events / revealed_object_context_from_events. No new fail-closed sibling group was introduced, which is exactly what the campaign-hotspots redirect for referent work asks for. One predicate widened, priority order untouched.
Gate B (zone authority) is not implicated and passes. scripts/zone_authority_census.py --check → Gate B PASS: 37 classified production hits carry allow-raw-zone annotations (23 rows). This PR adds no zone movement at all; it only reads ZoneChanged events, so zone_pipeline::move_object and the named zones.rs helpers are untouched.
attached_to_referent authority untouched and correctly not duplicated. That authority lives at crates/engine/src/game/filter.rs:896 and owns AttachedToSource/AttachedToRecipient; this is the distinct effect_context_object cascade in effects/mod.rs. No domain vocabulary was duplicated across the two.
Scope discipline. One file, one predicate, one test. Parse-diff sticky confirms zero card-parse movement — appropriate for a pure resolution-layer fix.
The test is discriminating and drives the real pipeline. GameScenario → runner.cast(volcanic).target_objects(&[bolt]).resolve() is the sanctioned cast-pipeline recipe, with no hand-written TargetRef vectors. On revert, the referent is None, the Demonstrative scope reads 0, and assert_eq!(outcome.damage_marked(goblin), 3) fails — the fallback-to-0 contract is documented at quantity.rs:344. The assertions also cover the full instruction chain (card in hand, damage on both opponent creatures, source exiled) rather than the damage alone.
The hidden-source exclusion is the correct call and correctly reasoned. Gating on is_public_zone(*from_zone) rather than accepting all hand destinations is what keeps draws out, and it is the same distinction CR 608.2h draws.
Correct root-cause narrative. The body traces parse → ObjectManaValue { Demonstrative } → parent_referent_context_from_events → moved_object_context_from_events and lands on the actual guard. No speculation, no shotgun edits.

Recommendation: two fixups, then enqueue. (1) Change the annotation at effects/mod.rs:1856 to CR 608.2h + CR 400.7j and quote the "moved it from a public zone to a hidden zone" clause — the fix is rules-correct, the citation just doesn't carry it. (2) Add the three helper-level unit tests from finding 2, with the Library→Hand is_none() case being the one that matters. Findings 3 and 4 are comment/naming polish and can ride along or be skipped. The engine behavior itself is correct and I'd land it as-is on the logic.

Note that the required Rust (fmt, clippy, test, coverage-gate) check is still pending at this head, and Contributor trust is ACTION_REQUIRED; the parse-diff sticky is present and clean, so no CI evidence is missing on the parser side.

The referent widening is rules-correct, but its annotation cited CR 608.2c (instruction ordering / later-text-modifies-earlier-text), which does not speak to source-zone publicity or last known information. Cite the two rules that actually carry the clause, both grep-verified against docs/MagicCompRules.txt: CR 400.7j findability on a move to a PUBLIC zone (the pre-existing arm) CR 608.2h "... or if the effect has moved it from a public zone to a hidden zone, the effect uses the object's last known information" (the new public -> Hand arm) Add three helper-level tests on parent_referent_context_from_events so the widened predicate's boundaries are pinned: - Library -> Hand (a draw) binds no referent. After the widening, is_public_zone's exclusion of Library is the only thing keeping every draw in the game from binding one, and nothing tested it. - Graveyard -> Hand binds the referent and carries the move-time mana value (the Volcanic Vision case, at helper level). - Two qualifying moves in one span bind nothing — the CR 608.2k singular guard now has a wider surface to trip on. Also name the dominant newly-captured population (ordinary Battlefield -> Hand bounce) in the predicate comment so the next reader sizes the blast radius correctly. Co-authored-by: philluiz2323 <philluiz2323@gmail.com>

matthewevans

Maintainer sign-off. The referent widening is at the right seam (moved_object_context_from_events, the existing zone-change member of the anaphoric-referent cascade) and is rules-correct.

Two maintainer fixups pushed in 5a182ec (co-authored to you, @philluiz2323 — the engine logic is unchanged):

CR citation corrected. The clause implements CR 608.2h ("...or if the effect has moved it from a public zone to a hidden zone, the effect uses the object's last known information") alongside CR 400.7j for the pre-existing public-destination arm. CR 608.2c is instruction ordering / later-text-modifies-earlier-text and does not carry source-zone publicity or LKI. Both replacements grep-verified against docs/MagicCompRules.txt.
Three helper-level tests on parent_referent_context_from_events, pinning the widened predicate's boundaries — most importantly Library -> Hand (a draw) binding nothing, since after this change is_public_zone's exclusion of Library is the only thing standing between the engine and every draw binding a referent, and nothing tested it.

Also named the dominant newly-captured population (ordinary Battlefield -> Hand bounce) in the predicate comment.

Nice work on this one: correct root-cause trace, one predicate widened rather than a new fail-closed sibling, and a discriminating cast-pipeline test that resolves to 0 on revert.

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/effects/mod.rs`:
- Around line 11555-11569: Update the documentation comment above
multiple_qualifying_moves_bind_no_parent_referent to cite CR 608.2c and CR
400.7j, matching the basis used by parent_referent_context_from_events, or
remove the CR citation if neither applies cleanly; preserve the existing
explanation of singular referent resolution.

🪄 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: db0a2c98-2bb4-48cf-acd5-29e06d0a269f

📥 Commits

Reviewing files that changed from the base of the PR and between 30b5b36 and 5a182ec.

📒 Files selected for processing (1)

crates/engine/src/game/effects/mod.rs

…mp (#6568) `release: v0.35.2` (21a53d5) bumped `client/src-tauri/Cargo.toml` to 0.35.2 via cargo-release's `pre-release-replacements`, but `client/src-tauri/` is a separate cargo workspace with its own lockfile that cargo-release does not manage. The lock still recorded `phase-tauri 0.35.1`, so the `tauri-check` CI job's `cargo check --locked --manifest-path client/src-tauri/Cargo.toml` refused to reconcile the mismatch and exited 101. That reds the required `Rust (fmt, clippy, test, coverage-gate)` aggregator (which `needs: [... tauri-check]`) on every open PR whose merge ref includes the release commit, blocking the merge queue repo-wide. Evidence: PR #6561 tauri-check PASSED at 20:43:00Z; the release commit landed at 20:49:54Z; PRs #6564 (20:56:21Z) and #6563 (21:15:11Z) both FAILED with the identical --locked error. None of the three touched any Cargo manifest. Follow-up (not in this change): cargo-release should keep the nested lockfile in sync so the next release does not re-break it. Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>

matthewevans · 2026-07-23T22:56:06Z

Temporarily dequeued by a maintainer — nothing is wrong with this PR, and it will be re-enqueued shortly.

main is currently broken in a way that reds the merge queue for everyone. The release: v0.35.2 commit (21a53d50cb) bumped client/src-tauri/Cargo.toml to 0.35.2 without regenerating client/src-tauri/Cargo.lock, which still pins phase-tauri 0.35.1. The Tauri compile check job builds with cargo check --locked, so it fails:

error: cannot update the lock file /home/runner/work/phase/phase/client/src-tauri/Cargo.lock
       because --locked was passed to prevent this

That job is a needs: dependency of the required Rust (fmt, clippy, test, coverage-gate) aggregator, so it hard-blocks merges even though GitHub reports it as not required. Any merge-group ref that does not contain the fix fails and gets ejected — which is what was happening to the PRs in the queue, including this one.

The fix is #6568. It was queued behind the PRs it needs to unblock, which is a deadlock: the queue merges in order, so it could not land until those cleared, and they could not clear until it landed. Dequeuing the entries ahead of it is how that gets broken.

No action needed from you. Your PR keeps its approval and its head is unchanged. Once #6568 merges, this will be re-enqueued and should go through on the same green it already had. Apologies for the churn — this was our breakage, not yours.

matthewevans · 2026-07-24T02:49:13Z

Second infrastructure-caused delay on this PR, and again nothing to do with your changes — apologies for the churn.

What happened this time: GitHub's hosted runners stalled repo-wide from roughly 00:18Z to 02:20Z, so no check could report back. The merge queue's ruleset has a 60-minute check-response timeout; this PR's merge group was created at 00:37:43Z, and github-merge-queue[bot] evicted it at 01:37:53Z — ten seconds past the deadline. That very same CI run then finished successfully at 02:01:55Z, 24 minutes after the eviction.

Both required checks are green at the current head and the approval is on this commit. Re-enqueuing now.

(The Contributor trust check is not required and its failure is cosmetic.)

…mp (phase-rs#6568) `release: v0.35.2` (21a53d5) bumped `client/src-tauri/Cargo.toml` to 0.35.2 via cargo-release's `pre-release-replacements`, but `client/src-tauri/` is a separate cargo workspace with its own lockfile that cargo-release does not manage. The lock still recorded `phase-tauri 0.35.1`, so the `tauri-check` CI job's `cargo check --locked --manifest-path client/src-tauri/Cargo.toml` refused to reconcile the mismatch and exited 101. That reds the required `Rust (fmt, clippy, test, coverage-gate)` aggregator (which `needs: [... tauri-check]`) on every open PR whose merge ref includes the release commit, blocking the merge queue repo-wide. Evidence: PR phase-rs#6561 tauri-check PASSED at 20:43:00Z; the release commit landed at 20:49:54Z; PRs phase-rs#6564 (20:56:21Z) and phase-rs#6563 (21:15:11Z) both FAILED with the identical --locked error. None of the three touched any Cargo manifest. Follow-up (not in this change): cargo-release should keep the nested lockfile in sync so the next release does not re-break it. Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>

…ard" (phase-rs#6486) (phase-rs#6561) * fix(engine): keep a "return to hand" card as the referent for "that card" (phase-rs#6486) Volcanic Vision ("Return target instant or sorcery card from your graveyard to your hand. Volcanic Vision deals damage equal to that card's mana value to each creature your opponents control. Exile Volcanic Vision.") returned the card but dealt no damage. The spell parses correctly — the damage sub-effect's amount is the Demonstrative "that card's mana value". That resolves against the earlier-instruction referent (`effect_context_object`), which `parent_referent_context_from_events` derives from the parent effect's `ZoneChanged` events via `moved_object_context_from_events`. That helper only captured moves to a PUBLIC zone, so the graveyard->HAND return bound no referent, "that card's mana value" resolved to 0, and every opponent creature took 0 damage. CR 608.2c: a later instruction may refer to an object an earlier instruction returned to hand — its identity was established in the public source zone (graveyard), and the reference reads the move-time last-known mana value, so the hidden destination is immaterial. Capture the moved referent for a move to hand when the SOURCE zone is public. A move from a hidden source (a draw, library -> hand) establishes no such referent and is excluded; library destinations stay excluded entirely (a shuffle loses identity). Verified: cargo test -p engine --lib -> 17591 passed, 0 failed. New card-level cast test drives Volcanic Vision through the real pipeline: with a mana-value-3 instant in the graveyard, each opponent creature takes 3 damage, the card is in hand, and Volcanic Vision is exiled. * fix(PR-6561): correct CR citation and pin the widened move predicate The referent widening is rules-correct, but its annotation cited CR 608.2c (instruction ordering / later-text-modifies-earlier-text), which does not speak to source-zone publicity or last known information. Cite the two rules that actually carry the clause, both grep-verified against docs/MagicCompRules.txt: CR 400.7j findability on a move to a PUBLIC zone (the pre-existing arm) CR 608.2h "... or if the effect has moved it from a public zone to a hidden zone, the effect uses the object's last known information" (the new public -> Hand arm) Add three helper-level tests on parent_referent_context_from_events so the widened predicate's boundaries are pinned: - Library -> Hand (a draw) binds no referent. After the widening, is_public_zone's exclusion of Library is the only thing keeping every draw in the game from binding one, and nothing tested it. - Graveyard -> Hand binds the referent and carries the move-time mana value (the Volcanic Vision case, at helper level). - Two qualifying moves in one span bind nothing — the CR 608.2k singular guard now has a wider surface to trip on. Also name the dominant newly-captured population (ordinary Battlefield -> Hand bounce) in the predicate comment so the next reader sizes the blast radius correctly. Co-authored-by: philluiz2323 <philluiz2323@gmail.com> --------- Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>

philluiz2323 requested a review from matthewevans as a code owner July 23, 2026 20:14

superagent-security Bot added the contributor:flagged Contributor flagged for review by trust analysis. label Jul 23, 2026

matthewevans self-assigned this Jul 23, 2026

matthewevans added the bug Bug fix label Jul 23, 2026

matthewevans approved these changes Jul 23, 2026

View reviewed changes

matthewevans enabled auto-merge July 23, 2026 20:43

coderabbitai Bot reviewed Jul 23, 2026

View reviewed changes

Comment thread crates/engine/src/game/effects/mod.rs

matthewevans added this pull request to the merge queue Jul 23, 2026

matthewevans removed their assignment Jul 23, 2026

matthewevans mentioned this pull request Jul 23, 2026

fix(ci): sync client/src-tauri/Cargo.lock with the v0.35.2 version bump #6568

Merged

matthewevans assigned matthewevans and unassigned matthewevans Jul 23, 2026

matthewevans removed this pull request from the merge queue due to a manual request Jul 23, 2026

matthewevans self-assigned this Jul 23, 2026

matthewevans added this pull request to the merge queue Jul 23, 2026

github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Jul 24, 2026

matthewevans removed their assignment Jul 24, 2026

matthewevans added this pull request to the merge queue Jul 24, 2026

Merged via the queue into phase-rs:main with commit 7de942f Jul 24, 2026
14 of 15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(engine): keep a "return to hand" card as the referent for "that card" (#6486)#6561

fix(engine): keep a "return to hand" card as the referent for "that card" (#6486)#6561
matthewevans merged 2 commits into
phase-rs:mainfrom
philluiz2323:fix/return-to-hand-that-card-referent-6486

philluiz2323 commented Jul 23, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

superagent-security Bot commented Jul 23, 2026

Uh oh!

coderabbitai Bot commented Jul 23, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

matthewevans commented Jul 23, 2026

Uh oh!

matthewevans left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

matthewevans commented Jul 23, 2026

Uh oh!

Uh oh!

matthewevans commented Jul 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

philluiz2323 commented Jul 23, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Implementation method (required)

Root cause

Fix

CR references

Verification

Summary by CodeRabbit

Uh oh!

superagent-security Bot commented Jul 23, 2026

Uh oh!

coderabbitai Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

github-actions Bot commented Jul 23, 2026

Parse changes introduced by this PR

Uh oh!

matthewevans commented Jul 23, 2026

🔴 Blocker

🟡 Non-blocking

✅ Clean

Uh oh!

matthewevans left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

matthewevans commented Jul 23, 2026

Uh oh!

Uh oh!

matthewevans commented Jul 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

philluiz2323 commented Jul 23, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 23, 2026 •

edited

Loading