Fix Namor, Atlantean King

[JacobWoodson]

Summary

Fixes a parse-fidelity defect on Namor, Atlantean King.

Issue: Third trigger drops the "who has more life than you" life-comparison condition (condition:null) and pumps target "Any" instead of scoping to "other creatures you control attacking that player".

Files changed

• G:\Repos\phase-card-runs\crates\engine\src\parser\oracle_target.rs
• G:\Repos\phase-card-runs\crates\engine\src\parser\oracle_trigger.rs
• G:\Repos\phase-card-runs\crates\engine\src\parser\oracle_effect\mod.rs
• G:\Repos\phase-card-runs\crates\engine\src\parser\oracle_effect\lower.rs
• G:\Repos\phase-card-runs\crates\engine\src\parser\oracle_nom\quantity.rs
• G:\Repos\phase-card-runs\crates\engine\src\parser\oracle_static\restriction.rs
• G:\Repos\phase-card-runs\crates\engine\src\parser\oracle_tests.rs
• G:\Repos\phase-card-runs\crates\engine\src\types\ability.rs
• G:\Repos\phase-card-runs\crates\engine\src\types\events.rs
• G:\Repos\phase-card-runs\crates\engine\src\game\ability_utils.rs
• G:\Repos\phase-card-runs\crates\engine\src\game\trigger_matchers.rs
• G:\Repos\phase-card-runs\crates\engine\src\game\filter.rs
• G:\Repos\phase-card-runs\crates\engine\src\game\layers.rs
• G:\Repos\phase-card-runs\crates\engine\src\game\coverage.rs
• G:\Repos\phase-card-runs\crates\engine\src\game\ability_scan.rs
• G:\Repos\phase-card-runs\crates\engine\src\game\ability_rw.rs
• G:\Repos\phase-card-runs\crates\engine\src\game\cost_payability.rs
• G:\Repos\phase-card-runs\crates\engine\src\game\effects\token.rs
• G:\Repos\phase-card-runs\crates\engine\src\database\synthesis.rs
• G:\Repos\phase-card-runs\crates\engine\tests\integration\namor_attacking_that_player.rs
• G:\Repos\phase-card-runs\crates\engine\tests\integration\main.rs
• G:\Repos\phase-card-runs\crates\mtgish-import\src\convert\condition.rs

CR references

• CR 102.2
• CR 102.3
• CR 107.1
• CR 109.4
• CR 109.5
• CR 119
• CR 119.1
• CR 120.3
• CR 122.1
• CR 402.1
• CR 508.1a
• CR 508.1b
• CR 508.5
• CR 508.5a
• CR 508.6
• CR 603.2
• CR 603.3d
• CR 603.4
• CR 608.2b
• CR 608.2c
• CR 611.2c
• CR 611.3a
• CR 702.105a
• CR 810.9a

Track

Developer

LLM

Model: claude-opus-4-8
Thinking: high

Tier: Frontier

Verification

• export CARGO_INCREMENTAL=0 (same shell for all steps; no CARGO_TARGET_DIR set) — pass
• cargo fmt --all — pass - no reformatting needed, working tree unchanged
• ./scripts/check-parser-combinators.sh — pass - Gate G PASS, Gate A PASS (head=566888a9487dc7091a28b4e68e456c7d44dbfbf1 base=9169d8f44788f6c56b37667c4d033088a70f24ac); no Family-D skip notice emitted
• CARGO_INCREMENTAL=0 cargo clippy -p phase-engine --all-targets -- -D warnings — pass - zero warnings
• CARGO_INCREMENTAL=0 cargo test -p phase-engine — pass - lib 19335 passed/0 failed/6 ignored; bins 21 passed and 9 passed; integration 5095 passed/0 failed/2 ignored (897.56s); doc-tests 0 passed/7 ignored. No tests skipped. All 7 namor_attacking_that_player integration rows passed.
• CARGO_INCREMENTAL=0 cargo export-cards data --output data/card-data.json --stats — pass - 35009 cards, 35798 faces, 2848 with unimplemented effects, 32161/35009 fully implemented (91.9%)
• cp data/card-data.json client/public/card-data.json — pass - both files 98725429 bytes, identical mtime
• cargo coverage — pass - "Namor, Atlantean King" supported:true gap_count:0; all 3 parse_details rows (Flying keyword, SpellCast trigger, Attacks trigger + PumpAll child) supported:true
• cargo semantic-audit — pass - 32769 cards audited, 257 flagged overall; 0 findings for "Namor, Atlantean King" (0 matches for any Namor card)
• AST fidelity diff vs verbatim MTGJSON Oracle text (manual, per task note) — pass - valid_target=PlayerMatching{PlayerAttribute{relation:All, attr:LifeTotal{ScopedPlayer}, GT, Ref(LifeTotal{Controller})}} with condition:null (CR 603.2 event channel, not CR 603.4); PumpAll +2/+0 UntilEndOfTurn over Typed{Creature, controller:You, props:[Attacking{DefendingPlayer}, Another]}; Flying keyword and SpellCast trigger (valid_card Non-Creature, valid_target Controller) also correct
• CR annotation verification against docs/MagicCompRules.txt — pass - 508.5, 508.5a, 603.2, 603.3d, 603.4, 611.2c, 119.1, 102.1, 102.3, 109.5, 608.2c, 120.3 all confirmed present

Scope Expansion

Scope grew from Namor's two defects to the full 4-card grammar class it shares: Owlbear Cub's attacked-player predicate (needed anyway, since the fail-closed guard would otherwise regress it to honest-RED) plus Ordruun Mentor / Echoing Assault, whose targeted use of the same anaphor required the ability_utils.rs enumeration-door fix to avoid a CR 603.3d silent trigger removal; also one display-only coverage.rs arm and a layers.rs life-read routing correction the new variant exposed.

Validation Failures

None blocking: all verification gates passed (tests, coverage supported:true gap:0, semantic-audit clean). Note: the automated review loop was capped before returning fully clean, so some non-blocking reviewer suggestions may remain unaddressed.

CI Failures

None.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 16 minutes

Limit details: You’ve used all 2 included reviews currently available under your plan.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

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: 45d6b910-f254-4fed-acfc-f3c8394e22fc

📥 Commits

Reviewing files that changed from the base of the PR and between 12ea904 and e2d61c2.

📒 Files selected for processing (31)

• crates/engine/src/database/synthesis.rs
• crates/engine/src/game/ability_rw.rs
• crates/engine/src/game/ability_scan.rs
• crates/engine/src/game/ability_utils.rs
• crates/engine/src/game/casting.rs
• crates/engine/src/game/cost_payability.rs
• crates/engine/src/game/coverage.rs
• crates/engine/src/game/effects/gain_control.rs
• crates/engine/src/game/effects/token.rs
• crates/engine/src/game/filter.rs
• crates/engine/src/game/layers.rs
• crates/engine/src/game/quantity.rs
• crates/engine/src/game/targeting.rs
• crates/engine/src/game/trigger_matchers.rs
• crates/engine/src/game/zone_pipeline.rs
• crates/engine/src/parser/oracle_effect/lower.rs
• crates/engine/src/parser/oracle_effect/mod.rs
• crates/engine/src/parser/oracle_nom/primitives.rs
• crates/engine/src/parser/oracle_nom/quantity.rs
• crates/engine/src/parser/oracle_static/restriction.rs
• crates/engine/src/parser/oracle_target.rs
• crates/engine/src/parser/oracle_tests.rs
• crates/engine/src/parser/oracle_trigger.rs
• crates/engine/src/types/ability.rs
• crates/engine/src/types/events.rs
• crates/engine/tests/integration/main.rs
• crates/engine/tests/integration/namor_attacking_that_player.rs
• crates/mtgish-import/src/convert/condition.rs
• crates/phase-ai/src/policies/effect_classify.rs
• crates/phase-ai/src/policies/hand_disruption.rs
• crates/phase-ai/src/policies/self_cost.rs

---

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.}

[matthewevans]

Request changes — the YouAttack anaphor has the wrong authority in multiplayer, and the required current-head parse-diff evidence is absent.

🔴 Blocker

[HIGH] attacking that player is bound through the attacking-creature DefendingPlayer authority even when the trigger source is not attacking. Evidence: crates/engine/src/parser/oracle_target.rs:5035-5040,5097-5105 lowers the phrase to ControllerRef::DefendingPlayer, and crates/engine/src/game/ability_utils.rs:3371-3389 deliberately routes that value into combat::defending_player_cr508_5. That authority's documented and implemented binding is for the defender of the asking attacking creature; its fallback becomes the batch-global defender when Ordruun Mentor is not attacking. matching_you_attack_pairs already enumerates the actual (attacker, target) pairs at crates/engine/src/game/trigger_matchers.rs:4212-4278, but the resulting triggered abilities do not carry the attacked player as their own bound value. The existing two-defender regression knowingly accepts this: crates/engine/tests/integration/namor_attacking_that_player.rs:478-535 calls its result "MEASURED behaviour, not an endorsement" and asserts only that either lane is offered.

Why it matters: Ordruun Mentor's "Whenever you attack a player" is a player-attacks-player trigger. Official CR 508.3e says, "An ability that reads ‘Whenever [a player] attacks [another player], . . .’ triggers if one or more creatures the first player controls are declared as attackers attacking the second player." On a declaration attacking P1 and P2, its two firings must each bind the corresponding attacked player. CR 508.5 instead governs an ability of an attacking creature, or an effect referring to both an attacking creature and a defending player; Ordruun Mentor is not attacking, so that authority cannot supply this pronoun's referent.

Suggested fix: carry the attacked player from each YouAttack match/trigger event as a typed, per-firing binding, and resolve this grammar through that binding rather than an attacking-source defender. Keep it general for player-attacks-player triggers—do not add an Ordruun-specific path. Add a three-player runtime regression with one attacker at P1 and one at P2 that proves two separately bound effects/target sets (P1's effect cannot offer P2's attacker and vice versa).

🔴 Required evidence

[MED] The exact-head coverage parse-diff sticky comment is missing. Evidence: the current e2d61c20e0cd700da3764f794a448cb47a246c75 changes engine/parser source, Card data completed successfully, but the PR conversation has no <!-- coverage-parse-diff --> comment. The green check alone does not show the card-level parser blast radius for this head.

Suggested fix: restore or regenerate the current-head parse-diff artifact/comment and reconcile every gained, lost, or changed card with the intended grammar before requesting rereview.

✅ Clean

The parser arm is compositional (tag("attacking") + tag("that player")) and the two-attacker same-defender test correctly catches the former empty-target-slot failure.

Recommendation: rework the trigger-time player binding, add the discriminating two-defender runtime test, and provide the current-head parse-diff evidence before rereview.

[github-actions]

Generated for head e2d61c20e0cd700da3764f794a448cb47a246c75.

Parse changes introduced by this PR · 4 card(s), 6 signature(s) (baseline: main 12ea90497eb2)

🟢 Added (1 signature)

• 1 card · ➕ ability/PumpAll · added: PumpAll (duration=until end of turn, filter=attacking defending player another you control creature, p/t=+2/+0)
  • Affected (first 3): Namor, Atlantean King

🔴 Removed (1 signature)

• 1 card · ➖ ability/Pump · removed: Pump (duration=until end of turn, p/t=+2/+0, target=any target)
  • Affected (first 3): Namor, Atlantean King

🟡 Modified fields (4 signatures)

• 1 card · 🔄 trigger/Attacks · changed field valid target: ∅ → player matching each player who controls GE Fixed { value: 8 } matching permanents
  • Affected (first 3): Owlbear Cub
• 1 card · 🔄 trigger/Attacks · changed field valid target: ∅ → player matching each player whose LifeTotal { player: ScopedPlayer } GT Ref { qty: LifeTotal { player: Controller } }
  • Affected (first 3): Namor, Atlantean King
• 1 card · 🔄 ability/TargetOnly · changed field target: nontoken creature → nontoken attacking defending player creature
  • Affected (first 3): Echoing Assault
• 1 card · 🔄 ability/grant FirstStrike · changed field target: creature → attacking defending player creature
  • Affected (first 3): Ordruun Mentor

[matthewevans]

Clarification for current head e2d61c20e0cd700da3764f794a448cb47a246c75: the current-head <!-- coverage-parse-diff --> artifact is now present and reconciles the four affected cards, so that evidence request is resolved.

The HIGH requested-changes blocker remains unchanged. attacking that player for a nonattacking YouAttack trigger must carry the attacked player as a typed per-firing binding, rather than resolve through the source creature's DefendingPlayer. Before re-review, add the three-player P1/P2 runtime regression proving each firing's target set cannot cross into the other attacked-player lane.