fix(parser): Compound player+object damage parser (try_parse_compound_player_object_damage) fails on

[ntindle]

Summary

Fixes a parser misparse affecting 1 card(s) in the Doctor Who Commander precons.

Root cause: Compound player+object damage parser (try_parse_compound_player_object_damage) fails on the 'where X is ...' dynamic-amount form: emits an empty-filter Opponent DamageAll with player_filter:None, so opponents take no damage and all their permanents (not just creatures) are hit.

Cards corrected

• Impending Flux

Fix

Implemented the cluster-08 fix surgically on upstream/main (HEAD a304e51) in /Users/ntindle/code/random/magic/phase-main-base. The bug was confirmed still present before the change: Impending Flux ("deals X damage to each opponent and each creature they control, where X is 1 plus …") parsed to a DamageAll with player_filter:None and empty type_filters, so opponent PLAYERS took no damage and ALL opponent permanents (not just creatures) were hit.

Root cause: try_parse_compound_player_object_damage (crates/engine/src/parser/oracle_effect/mod.rs) asserts the remainder after the controller-suffix probe (gate_rest) is empty. The full clause reaching it included the trailing ", where x is 1 plus …" binding, so gate_rest was non-empty, the function returned None, and dispatch fell through to a general empty-filter Opponent DamageAll.

Fix (exactly per the approved plan, Step 1): before the trailing-period trim and the controller-suffix probe, strip the trailing where-X binding using the existing pub(crate) helper strip_trailing_where_x(TextPair::new(after_and_each, after_and_each)) — the same tolerance counter.rs/sequence.rs/search.rs already carry. The amount stays QuantityExpr::Ref{Variable("X")}; the binding is applied at lowering by the already-wired apply_where_x_effect_expression (DamageAll arm). No lowering, types, effect-handler, AI, or frontend changes were needed (Step 2 read-only verification confirmed the DamageAll where-X arm already exists). No new helper, no new enum variant — both strip_trailing_where_x and TextPair were already imported at module scope, so no redundant use was added.

Added two building-block-level tests (Step 3): a gate-level test (try_split_damage_compound_player_object_dynamic_x_where_clause) asserting amount stays Variable("X"), player_filter == Opponent, and a Creature@Opponent object filter; and a full-pipeline test (try_split_damage_compound_player_object_dynamic_x_full_pipeline) asserting the where-X binding rewrites the bare Variable("X") amount and that player_filter + Creature restriction survive lowering. Both pass; all 15 existing compound-damage tests (Goblin Chainwhirler, Kumano, Cone of Flame, etc.) stay green — the strip is a no-op when no "where x is" is present.

Live export now correct: DamageAll{ player_filter: Opponent, target: Typed{type_filters:[Creature], controller:Opponent} }. The empty type_filters and missing player_filter — the cluster's high-severity bug — are both gone.

VERIFICATION: cargo fmt clean; ./scripts/check-parser-combinators.sh vs HEAD exit 0 (no string-dispatch introduced; the only .contains() in the diff are tf.type_filters.contains(&TypeFilter::Creature) Vec-membership assertions inside #[test] fns); cargo clippy -p engine --all-targets -D warnings clean; cargo test -p engine = 11854 passed, 3 failed.

The 3 failures are UNRELATED pre-existing failures from another agent's in-progress work: cathars_crusade_db_load_path, walking_ballista_db_load_path, and station_32_tdm_spacecraft_regression_suite all fail with the identical error "load card-data export: unknown variant ZoneChangeAggregateThisTurn". That QuantityRef variant is NOT in current source (grep of crates/engine/src returns nothing) but IS present in the on-disk data/card-data.json and client/public/card-data.json — a stale/foreign export generated by a branch that added the variant. A parser-only change cannot produce an unknown-variant deserialization error. Per CLAUDE.md multi-agent safety I did NOT regenerate or touch card-data.json (that would destroy the other agent's export). No edits were committed; all changes are left in the working tree.

Files changed

• crates/engine/src/parser/oracle_effect/mod.rs

CR references

• CR 107.3i — Normally, all instances of X on an object have the same value at any given time (verified: grep -n "^107.3i" docs/MagicCompRules.txt → line 482). Cited on the where-X strip and gate test: the binding defines X for the whole instruction.
• CR 608.2c — later text on the card may modify the meaning of earlier text; read the whole text (verified: grep -n "^608.2c" docs/MagicCompRules.txt → line 2789). Cited on the where-X strip and full-pipeline test: the trailing binding modifies the amount.
• CR 120.3 — Damage results depend on whether the recipient is a player or permanent (verified: grep -n "^120.3" docs/MagicCompRules.txt → line 1095). Cited on the gate-level test: unified DamageAll over the player set + object set.
• CR 107.3 — X is a placeholder for a number; some abilities define X (verified: grep -n "^107.3" docs/MagicCompRules.txt → line 464). Context for the X-defined-by-binding case.

Verification

• cargo fmt --all — clean (exit 0, no changes)
• check-parser-combinators.sh (merge-base a668283289) — clean (exit 0)
• cargo clippy -p engine --all-targets -- -D warnings — clean (exit 0)
• cargo test -p engine — PASS after regen (11858 passed, 0 failed); 3 initial failures were a stale gitignored client/public/card-data.json carrying a ZoneChangeAggregateThisTurn QuantityRef variant from another branch (absent from this merged tree's enum) - unrelated to cluster chore: update coverage stats and badges #8; fixed by ./scripts/gen-card-data.sh
• oracle-gen --filter "impending flux" — confirmed correct: DamageAll effect, amount Offset(+1) over SpellsCastThisTurn ref, target=opponents' creatures, player_filter=Opponent
  Cards confirmed re-parsed correctly: Impending Flux

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request updates the parser in crates/engine/src/parser/oracle_nom/condition.rs to correctly handle independent ordering of cast-origin qualifiers and timing qualifiers (e.g., 'from anywhere other than your hand this turn'), aligning with CR 601.2a and CR 400.1. A unit test has been added in crates/engine/src/parser/oracle_quantity.rs to verify this parsing behavior. There are no review comments to evaluate, 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.

[matthewevans]

Approved. I re-reviewed the actual head and the fix is at the right parser seam: the compound player+object damage gate now tolerates the trailing where-X binding while preserving the existing lowering path for the X expression. The added tests discriminate both halves of the bug: the structural parse keeps player_filter = Opponent plus the Creature@Opponent object filter, and the full chain lowers X to the spell-history offset expression instead of leaving a bare variable.

Verified locally with cargo fmt --all -- --check, ./scripts/check-parser-combinators.sh, cargo test -p engine try_split_damage_compound_player_object_dynamic_x -- --nocapture, and cargo test -p engine cda_spells_cast_this_turn_cast_origin_qualifier_before_time -- --nocapture. GitHub rollup is green at this head.