fix(engine): bind mana-value discard costs to X

[matthewevans]

Fixes #6908.

Preserves Discard a card with mana value X as a typed cost filter, requires X selection before target selection, and applies the selected X while selecting the discard payment. Adds a Kozilek activation regression covering both bindings.

Verification: cargo fmt --all; parser combinator and PreLowered pre-commit gates. Per workspace policy, no cargo build/test was run in this worktree.

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of discard and exile costs that depend on an announced X value.
  • The game now rejects actions when the selected X value makes required costs unpayable.
  • Discard targets are correctly matched against ability-specific filters.
  • Fixed parsing for discard effects involving mana-value requirements.
  • Improved validation for composite and alternative costs after choosing X.

• Tests

  • Added coverage for matching discarded cards and target spells to the announced mana value.
  • Added validation for rejecting unavailable discard choices.

[coderabbitai]

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: 5bc9fa29-31e6-427f-abc4-93dbf976ae93

📥 Commits

Reviewing files that changed from the base of the PR and between 2f2f0d7 and 2ac793f.

📒 Files selected for processing (3)

• crates/engine/src/game/engine.rs
• crates/engine/src/parser/oracle_cost.rs
• crates/engine/tests/integration/main.rs

🚧 Files skipped from review as they are similar to previous changes (3)

• crates/engine/tests/integration/main.rs
• crates/engine/src/parser/oracle_cost.rs
• crates/engine/src/game/engine.rs

---

📝 Walkthrough

Walkthrough

The change preserves mana-value X filters, resolves discard targets with ability context, validates activation costs after X selection, and adds integration coverage for Kozilek’s discard-and-counter ability.

Changes

X discard-cost handling

Layer / File(s)	Summary
Filter and parser contracts crates/engine/src/game/cost_payability.rs, crates/engine/src/parser/...	Parser paths preserve mana-value X filters. Payability helpers detect and relax these constraints before X announcement.
Ability-aware discard resolution crates/engine/src/game/casting.rs, crates/engine/src/game/casting_costs.rs	Discard target discovery uses resolved ability context and excludes the source object. Casting paths use pre-announcement cost filters.
X-choice validation and integration coverage crates/engine/src/game/casting_costs.rs, crates/engine/src/game/engine.rs, crates/engine/tests/integration/...	Activation X choices are checked against recursive cost payability. The Kozilek discard-and-counter flow has integration coverage.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Player
  participant Engine
  participant CastingCosts
  participant GameState
  Player->>Engine: Announce X value
  Engine->>CastingCosts: Apply X to ability and cost
  CastingCosts->>GameState: Find eligible discard cards
  GameState-->>CastingCosts: Return matching cards
  CastingCosts-->>Engine: Return payable or InvalidAction
  Engine-->>Player: Continue activation or reject X choice

Loading

Possibly related PRs

• phase-rs/phase#6596: Extends shared non-self discard resolution and casting-cost paths with ability context and X-based discard eligibility.
• phase-rs/phase#6786: Modifies discard-cost handling and eligibility semantics, including nested discard costs and mana-value filters.
• phase-rs/phase#7030: Modifies discard-cost parsing, eligibility, and payability in the same engine paths.

Suggested labels: bug, test

🚥 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 primary fix: binding mana-value discard costs to X.
Linked Issues check	✅ Passed	The changes address issue #6908 by preserving the mana-value X filter, validating X-dependent discard payments, and adding Kozilek regression tests.
Out of Scope Changes check	✅ Passed	The parser, casting, payability, engine, and regression-test changes all support the linked Kozilek discard-cost objective.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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 ship/fix-engine-bind-mana-value-discard-costs

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

Generated for head 0176293fa8ae83131d32bc6c6571ae97c76246cc.

Parse changes introduced by this PR

✓ No card-parse changes detected.

[coderabbitai]

Actionable comments posted: 5

🤖 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/casting.rs`:
- Around line 17677-17702: Add a verified CR annotation immediately above
find_eligible_discard_targets_for_ability describing that it selects legal cards
in the player’s hand for an activated-ability discard cost, excluding the source
object and applying the optional TargetFilter. Preserve the helper’s existing
filtering behavior.

In `@crates/engine/src/game/cost_payability.rs`:
- Around line 112-139: Preserve the originally announced X-constrained
TargetFilter after X is chosen, and reuse it when validating and selecting every
object in multi-object costs. Update deferred target handling and the
AbilityCost::Exile discard/exile eligibility path so Cmc == X is not removed or
recomputed per object, including recursive filters handled by
relax_x_mana_value_constraint. Add regressions for discard and exile costs with
count = 2 and objects having different mana values, ensuring all selected
objects satisfy the same X.

In `@crates/engine/src/game/engine.rs`:
- Around line 8946-8960: Move the activation cost validation using
activation_cost_is_payable_after_x_choice out of the
pending.deferred_target_selection-only path and run it for every activated X
choice, including no-target abilities. Ensure the trial cost is rejected
atomically before deferred-target handling when the chosen value cannot pay the
discard or other activation cost.

In `@crates/engine/src/parser/oracle_cost.rs`:
- Line 843: Replace the verbatim rest_lower equality check in the parser
dispatch with all_consuming(tag::<_, _, E<'_>>("a
card")).parse(rest_lower.as_str()).is_ok(). Preserve the exact bare “a card”
match while allowing longer phrases to continue to parse_discard_card_filter.

In `@crates/engine/tests/integration/issue_6908_kozilek_discard_mana_value.rs`:
- Around line 28-63: Add a second discard candidate in the scenario with a mana
value different from three, then add a separate X=3 activation attempt that
selects this nonmatching card and asserts the payment or activation is rejected.
Preserve the existing successful discard-and-resolve flow as the reach guard,
using the test’s existing scenario and runner APIs.

🪄 Autofix

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: f19c87b9-aa05-45ce-8831-a14bd4c77b83

📥 Commits

Reviewing files that changed from the base of the PR and between c162191 and e46ef24.

📒 Files selected for processing (8)

• crates/engine/src/game/casting.rs
• crates/engine/src/game/casting_costs.rs
• crates/engine/src/game/cost_payability.rs
• crates/engine/src/game/engine.rs
• crates/engine/src/parser/oracle_cost.rs
• crates/engine/src/parser/oracle_effect/imperative.rs
• crates/engine/tests/integration/issue_6908_kozilek_discard_mana_value.rs
• crates/engine/tests/integration/main.rs