Skip to content

fix(engine): scope the tracked-set cast filter context to the published set - #7035

Merged
matthewevans merged 1 commit into
mainfrom
fix/tracked-set-cast-filter-context
Aug 5, 2026
Merged

fix(engine): scope the tracked-set cast filter context to the published set#7035
matthewevans merged 1 commit into
mainfrom
fix/tracked-set-cast-filter-context

Conversation

@matthewevans

@matthewevans matthewevans commented Aug 5, 2026

Copy link
Copy Markdown
Member

Follow-up to #7034. Three CodeRabbit findings were posted on that PR, but the merge queue landed it before the fixes could be pushed — queued branches can't be updated, and by the time I dequeued it had already merged. These are those fixes, unchanged in substance from what was verified against #7034's head.

1. Scope the filter context to the published set (Major, correctness)

tracked_set_cast_candidates stopped reading ability.targets for its candidates but still built the FilterContext from the same unscoped ability. FilterContext::from_ability carries ability.targets — for Sanar, the whole reveal window the chain seam injected — so a filter leg that reads object scope (a ParentTarget-relative comparison, a same-name or shares-a-type leg) would still have evaluated against the injected window rather than the members actually published.

Same defect class as the bug #7034 fixed, one layer down.

Latent, not live: all 51 tracked-set cast cards bind filter: Any, which reads no object scope, so nothing misbehaves today. It is closed so it stays that way. Both sibling sites in this same resolver already clone and rescope the ability before constructing the context — one of them under the same CR 607.2a. This makes the third site consistent with them.

2. CR 614.6CR 607.2a at assembly.rs:2434

Missed in #7034's citation sweep because that sweep used a hand-listed file set which omitted assembly.rs.

614.6 is "if an event is replaced, it never happens" and does not describe this code. 607.2a is the linked-ability rule for an activated or triggered ability that instructs a player to exile, which is what this block narrows against.

3. Pin the three unpinned uncaused exile shapes

uncaused_exilers in exiled_cause_publishers_all_stamp_exiled_at_runtime named five shapes in its doc comment and pinned two. Adds Dig { destination: Some(Exile) }, ExileHaunting and ExileResolvingSpellInsteadOfGraveyard.

Each now carries both the positive chain_clause_is_exile_producer check and the negative publishes_exiled_cause_at_resolution / this_way_cause_for_effect checks. All three confirm as genuinely uncaused — the test passes with them added, which is what establishes that a cause-filtered anaphor must not be bound after any of them.

Verification

  • exiled_cause_publishers_all_stamp_exiled_at_runtime green with all five shapes pinned
  • 9 integration tests green (issue_4253_sanar_vivid ×7, urza_lord_high_artificer ×2), covering the cast_from_zone behavioural change
  • Cherry-picked onto post-merge main (14b11cf5bf) with no conflicts

Summary by CodeRabbit

  • Bug Fixes

    • Improved tracked-set casting behavior by removing duplicate candidates while preserving their original order.
    • Ensured filtering evaluates only the intended published targets, preventing unrelated targets from affecting results.
  • Tests

    • Expanded coverage for effects that exile cards, including resolving spells and haunting effects.
  • Documentation

    • Corrected a tracked-set rules reference.

…ed set

Addresses three review findings on this PR.

1. `tracked_set_cast_candidates` stopped reading `ability.targets` for its
   candidates but still built the `FilterContext` from the same unscoped
   ability, so a filter leg that reads object scope (a ParentTarget-relative
   comparison, a same-name or shares-a-type leg) would still have evaluated
   against the injected reveal window. Same defect class as the bug this PR
   fixes, one layer down. Latent today — all 51 cards bind `filter: Any`,
   which reads no object scope — and both sibling sites in this resolver
   already clone and rescope, one of them under the same CR 607.2a.

2. CR 614.6 -> CR 607.2a at assembly.rs:2434. Missed in the previous commit
   because the citation sweep used a hand-listed file set that omitted
   assembly.rs.

3. `uncaused_exilers` named five shapes in its doc and pinned two. Add
   `Dig{destination: Exile}`, `ExileHaunting` and
   `ExileResolvingSpellInsteadOfGraveyard` so every named shape carries both
   the positive chain_clause_is_exile_producer check and the negative
   publishes_exiled_cause_at_resolution / this_way_cause_for_effect checks.
   All three confirm as uncaused.
@matthewevans
matthewevans enabled auto-merge August 5, 2026 15:36
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8dd49ee4-eb1a-4ef4-b428-dc8bb40cfa20

📥 Commits

Reviewing files that changed from the base of the PR and between 14b11cf and 2901477.

📒 Files selected for processing (3)
  • crates/engine/src/game/effects/cast_from_zone.rs
  • crates/engine/src/parser/oracle_effect/assembly.rs
  • crates/engine/src/parser/oracle_effect/tests.rs

📝 Walkthrough

Walkthrough

Tracked-set cast candidates are now deduplicated before filtering. Filter evaluation uses only the deduplicated published members. A tracked-set rules citation and parser test fixtures were also updated.

Changes

Tracked-set cast filtering

Layer / File(s) Summary
Deduplicate candidates before filtering
crates/engine/src/game/effects/cast_from_zone.rs
Tracked-set members are deduplicated in publication order. The bound filter evaluates a cloned ability containing only those members.
Update parser tracking support
crates/engine/src/parser/oracle_effect/assembly.rs, crates/engine/src/parser/oracle_effect/tests.rs
The tracked-set rules citation now references CR 607.2a. The uncaused_exilers fixtures include additional exile-producing effects.

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

Possibly related PRs

Suggested labels: bug

Suggested reviewers: andriypolanski, invalidcards, 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 identifies the main engine change: limiting tracked-set cast filter context to the published set.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/tracked-set-cast-filter-context

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@matthewevans
matthewevans disabled auto-merge August 5, 2026 15:38
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Parse changes introduced by this PR

Baseline pending for 14b11cf5bf79a57be96ca6446d25b374469dfc9c — this populates once main publishes its coverage snapshot (a few minutes after that commit landed).

@matthewevans
matthewevans added this pull request to the merge queue Aug 5, 2026
Merged via the queue into main with commit f55fb79 Aug 5, 2026
15 checks passed
@matthewevans
matthewevans deleted the fix/tracked-set-cast-filter-context branch August 5, 2026 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant