fix: parse coordinated damage targets

[andriypolanski]

Summary

• parse coordinated target phrases that combine a player leg with object legs while eliding repeated target
• represent All Will Be One's damage target as one union of an opponent, an opponent-controlled creature, or an opponent-controlled planeswalker
• add parser and cast-pipeline regressions proving an opponent-controlled creature can be selected instead of silently targeting the opponent

Fixes #5237.

Root cause

The target parser stopped after target opponent in:

target opponent, creature an opponent controls, or planeswalker an opponent controls

The exported effect therefore contained only an empty-type opponent filter. The fix parses each coordinated object alternative independently, preserving its controller qualifier, and merges all alternatives into the existing TargetFilter::Or representation.

Validation

• cargo fmt --all
• ./scripts/check-parser-combinators.sh
• ./scripts/check-skill-doc.sh
• cargo test -p phase-engine target_opponent_with_elided_coordinated_object_alternatives -- --nocapture
• cargo test -p phase-engine coordinated_player_and_object_target_preserves_plain_player_behavior -- --nocapture
• cargo test -p phase-engine awbo_can_target_opponent_controlled_creature_instead_of_opponent -- --nocapture
• cargo run -p phase-engine --features cli --bin oracle-gen -- data --filter "All Will Be One"
• cargo clippy -p phase-engine --all-targets --features cli -- -D warnings

Summary by CodeRabbit

• Bug Fixes

  • Improved target selection for coordinated options involving players and objects, such as opponent-controlled creatures or planeswalkers.
  • Ensured selecting an opponent-controlled object does not also incorrectly select the opponent.
  • Preserved correct handling of remaining input and syntax in complex target phrases.

• Tests

  • Added coverage confirming counter-triggered effects select the intended creature and apply damage correctly.

[superagent-security]

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

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 23 minutes

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: ea39cbd9-7dec-4b41-9799-c4b7c24f16d9

📥 Commits

Reviewing files that changed from the base of the PR and between c5a2a73 and dae9317.

📒 Files selected for processing (2)

• crates/engine/src/parser/oracle_target.rs
• crates/engine/tests/integration/l02_bbfu2_batched_counter_triggers.rs

📝 Walkthrough

Walkthrough

The target parser now supports coordinated phrases with an explicit player or opponent followed by object alternatives. Parser tests and an All Will Be One integration test validate opponent-controlled target selection and damage behavior.

Changes

Coordinated target parsing

Layer / File(s)	Summary
Compose coordinated target filters crates/engine/src/parser/oracle_target.rs	The parser combines player heads with comma-separated object alternatives and preserves controller and type qualifiers. Parser tests cover opponent-controlled creatures, planeswalkers, and plain player/object coordination.
Validate All Will Be One target selection crates/engine/tests/integration/l02_bbfu2_batched_counter_triggers.rs	The integration test verifies batched counter placement, 2 damage to the selected opponent-controlled creature, and no damage to the opponent player.

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

Possibly related PRs

• phase-rs/phase#6793: Both PRs modify target parsing for player and object target phrases.

Suggested labels: bug

Suggested reviewers: matthewevans

🚥 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 parser fix for coordinated damage targets.
Linked Issues check	✅ Passed	The parser changes and regression test address issue #5237 by enabling opponent-controlled creature targets.
Out of Scope Changes check	✅ Passed	All changes support coordinated target parsing and regression coverage for issue #5237.
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.}

[coderabbitai]

Actionable comments posted: 2

🤖 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/parser/oracle_target.rs`:
- Around line 996-1000: Update the initial separator `alt` in the coordinated
target parser to accept `", and "` alongside the existing separator forms. Keep
it consistent with the continuation separator used by the surrounding loop so
two-leg coordinated targets retain the object leg.
- Around line 1004-1034: Update the coordinated-leg loop in the parser around
parse_type_phrase_with_ctx so a TargetFilter::Any result after earlier legs have
been merged returns the accumulated combined filter, with the remainder based on
the unconsumed leg_text, instead of falling through to the bare player_filter
return. Preserve the existing initial-leg fallback behavior when no leg has been
merged.

🪄 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: 62fce449-d75c-4aca-bdb0-171d865dfe7e

📥 Commits

Reviewing files that changed from the base of the PR and between 73fd7f6 and c5a2a73.

📒 Files selected for processing (2)

• crates/engine/src/parser/oracle_target.rs
• crates/engine/tests/integration/l02_bbfu2_batched_counter_triggers.rs

[github-actions]

Parse changes introduced by this PR · 4 card(s), 3 signature(s) (baseline: main 5674d7578d05)

🟡 Modified fields (3 signatures)

• 2 cards · 🔄 ability/DealDamage · changed field target: opponent → opponent or opponent controls creature or opponent controls planeswalker
  • Affected (first 3): All Will Be One, Nicol Bolas, God-Pharaoh
• 1 card · 🔄 ability/DealDamage · changed field target: opponent → opponent or battle or planeswalker
  • Affected (first 3): Jeska and Kamahl
• 1 card · 🔄 ability/PreventDamage · changed field target: player → player or planeswalker or creature Sliver
  • Affected (first 3): Cautery Sliver

[matthewevans]

Blocked — the coordinated player/object grammar drops the object leg for the ordinary , and connector.

🔴 Blocker

[MED] The first player→object separator omits , and . Evidence: crates/engine/src/parser/oracle_target.rs:996-1000 accepts only , and/or , , or , and , ; the continuation separator at :1014-1019 already accepts , and . Consequently target player, and creature you control falls through to the bare Player return at :1030-1034 and silently drops the object leg. Why it matters: one normal connector changes the legal target domain. Suggested fix: add tag(", and ") to the initial longest-first separator alt, then add a parser regression asserting the two-leg form lowers to Player ∨ Typed(Creature, controller You).

✅ Clean

The All Will Be One runtime test is discriminating for the supported opponent/object behavior, but it does not cover this , and connector sibling.

Recommendation: request changes — cover the missing initial connector and verify the complete two-leg filter before another review.

[matthewevans]

Maintainer completion verified on dae9317: current CI is green, parser artifact contains the intended coordinated-targeting deltas, and there are no unresolved review threads.