review: narrow decline-tail verb to discard, add 3-player fan-out test

[bitfathers94]

Summary

Fixes the "each opponent who didn't discard a nonland card this way" misparse for Kroxa, Titan of Death's Hunger. Closes #6007

The clause was parsed as an ordinary, unconditional per-player imperative with no gate at all, so every opponent lost 3 life whenever Kroxa entered or attacked, regardless of what — or whether anything — they discarded.

Files changed

• crates/engine/src/parser/oracle_effect/lower.rs
• crates/engine/src/parser/oracle_effect/mod.rs
• crates/engine/src/game/effects/mod.rs
• crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__kroxa_titan_ir.snap
• crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__kroxa_titan_lowered.snap
• crates/engine/tests/integration/kroxa_titan_nonland_discard_life_loss.rs
• crates/engine/tests/integration/main.rs

CR references

• CR 608.2c — general condition-gating on a sub-ability
• CR 701.9a — discard definition (hand → graveyard), and "discarded this way" reads last_zone_changed_ids
• CR 109.5 — per-iteration recipient/scope binding inside a player_scope fan-out

Implementation method (required)

Method: manual (parser dispatch + engine iteration-attachment fix), verified against the existing strip_each_scope_who_cant_subject / strip_each_scope_who_doesnt_subject / strip_each_scope_who_does_subject decline-tail family

Track

Developer

Verification

• Required checks ran clean locally (Tilt unavailable in this environment; ran cargo directly).

• Gate A output below is for the current committed head.

• Final review-impl below is clean for the current committed head.

• Both anchors cite existing analogous code at the same seam.

• cargo test -p engine --lib — 16840 passed; 0 failed; 7 ignored

• cargo test -p engine --test integration — 3277 passed; 0 failed; 2 ignored (includes the 4 new Kroxa regression tests)

• cargo clippy -p engine --lib --tests -- -D warnings — clean, no warnings

• cargo fmt --all — applied, no diff

• ./scripts/check-parser-combinators.sh — Gate A PASS (see below)

Gate A

Gate A PASS head=e73c70c1762aa268434efc09452ffa33955415db base=2f4608051c4d694d3ec3fd59c3e68c26d3746559

Anchored on

• crates/engine/src/parser/oracle_effect/lower.rs:3868 — strip_each_scope_who_cant_subject, the mandatory-impossible decline-tail sibling this new dispatcher parallels (subject-only "each <scope> who ..." head, PlayerFilter scope detection, comma-optional body split)
• crates/engine/src/game/effects/mod.rs:2216 — condition_depends_on_effect_performed, whose sibling condition_depends_on_zone_change_this_way (already defined, used elsewhere for interactive-choice deferral) is now also consulted by detach_after_player_scope_local_chain so a ZoneChangedThisWay-gated sub-ability stays attached to its own per-player iteration instead of detaching as a once-after-all-iterations tail

Final review-impl

Self-reviewed inline against the universal lenses (correct seam, class vs single case, sibling coverage, vacuous-negative guard, edge cases): narrowed the new dispatcher's verb set from a speculative {discard, sacrifice, exile} to discard only, since sacrifice/exile are unverified against any real card printing this construction — widening is a one-line change once one exists. Added a 3-player regression test to pin the per-opponent iteration-attachment fix (the second, less obvious half of the bug: without it, the life-loss sub-ability detaches from the per-opponent Discard iteration and evaluates once against a stale/aggregate zone-change ledger instead of each opponent's own discard).

Claimed parse impact

• Kroxa, Titan of Death's Hunger

Validation Failures

None.

CI Failures

None.

Closes #6007

[gemini-code-assist]

Code Review

This pull request resolves issue #6007 by implementing support for subject-only mandatory-filtered decline-tail sub-abilities, specifically targeting cards like Kroxa, Titan of Death's Hunger. The parser has been updated to correctly identify and extract 'who didn't' and 'who did not' clauses, preventing them from being orphaned. In the engine, effects gated on ZoneChangedThisWay are now kept within their local player-scoped chain instead of being detached, ensuring they evaluate independently per opponent. A comprehensive suite of integration tests has been added to verify these rules. There are no review comments, and I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[github-actions]

Parse changes introduced by this PR · 2 card(s), 2 signature(s) (baseline: main eb8562809256)

🟡 Modified fields (2 signatures)

• 1 card · 🔄 ability/LoseLife · changed field conditional: ∅ → not (card non-land changed zones this way) and scoped player is each opponent
  • Affected (first 3): Kroxa, Titan of Death's Hunger
• 1 card · 🔄 ability/LoseLife · changed field conditional: ∅ → not (creature changed zones this way) and scoped player is each player
  • Affected (first 3): Strongarm Tactics

[matthewevans]

Verdict: changes requested — the implementation is promising, but the current changed-card class lacks runtime coverage.

🔴 Blocker

• crates/engine/tests/integration/kroxa_titan_nonland_discard_life_loss.rs:159 proves only the each opponent fan-out. The current parse-diff record also changes Strongarm Tactics, whose verified Oracle text is “Each player discards a card. Then each player who didn't discard a creature card this way loses 4 life.” The new parser explicitly maps that form to PlayerFilter::All (crates/engine/src/parser/oracle_effect/lower.rs:3982-3994), but no runtime test exercises the controller-inclusive scope or its per-player ZoneChangedThisWay gate. Please add a Strongarm-style runtime case (including a creature discard, a noncreature discard, and an empty hand) and correct the comment claiming Kroxa is the only verified card (lower.rs:3969-3975).

The Kroxa coverage is a good regression test for the opponent scope, and the narrowed non-discard guard is a useful compatibility check.

Recommendation: add the sibling-class runtime test, then rerun the prescribed project verification.

[matthewevans]

Approved: the current head resolves the Strongarm Tactics all-player runtime coverage blocker. The Kroxa opponent path and Strongarm controller-inclusive path both exercise the per-player ZoneChangedThisWay condition.