fix(parser): capture tapped-and-attacking when trailed by where-X clause (closes #2379)

[nickmopen]

Fixes #2379

Root cause

parse_token_description uses a scan loop to find the "that are tapped and attacking" suffix in the token text. The loop required the attacking phrase to be followed by EOF. When a variable-count binding (", where X is …") trails the clause — as in Anim Pakal's

create X 1/1 colorless Gnome artifact creature tokens that are tapped and attacking, where X is the number of +1/+1 counters on Anim Pakal.

— the EOF check failed at every byte position and both tapped and enters_attacking stayed false.

Fix

Two minimal changes to parse_token_description in oracle_effect/token.rs:

1. Extend the terminator check — the attacking-clause combinator now accepts , where as a valid terminator in addition to EOF (CR 107.3: where-clause binds a variable; it does not affect entry state).

2. Preserve the where-expression — after the attacking clause is detected the text is truncated at pos, stripping the trailing where-clause from what suffix sees. saved_where_x_expr captures the expression from the pre-truncation text and is used as a fallback in the X-binding step, keeping the variable count correctly resolved.

Coverage

Covers the full class of token-creation effects that combine an entry modifier (tapped and attacking) with a variable-count rebind (where X is …). Cards besides Anim Pakal that follow this pattern include any future cards using the same Oracle grammar.

Tests

• New regression test tapped_and_attacking_with_trailing_where_x_clause — verifies tapped=true, enters_attacking=true, and count=CountersOn(Source, P1P1) for the Anim Pakal token text.
• Existing plural_that_are_tapped_and_attacking_suffix_strips (Leonin Warleader) — still passes, eof path unaffected.

[gemini-code-assist]

Code Review

This pull request updates the token parser in crates/engine/src/parser/oracle_effect/token.rs to correctly handle token creation clauses that specify entering tapped and/or attacking followed by a variable expression (e.g., ', where X is...'). It allows the attacking clause parser to terminate on either EOF or ', where ', extracts and saves the trailing expression before truncation, and falls back to it when resolving the variable count. A unit test has been added to verify this behavior. There are no review comments, and we have no additional 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]

Review: PR #2807 — tapped-and-attacking with trailing where-X clause

Verdict: APPROVE. Correct seam, idiomatic, genuinely class-general, and the test is discriminating. The bug is real on origin/main (#2379 still open) and this fixes it cleanly. Thanks for the thorough comments and CR annotations — this is a clean contribution.

Correct seam

The fix lives in parse_token_description (crates/engine/src/parser/oracle_effect/token.rs), the function that owns entry-modifier + count parsing for non-copy tokens. That is the right layer — no symptom-patching downstream.

(a) Combinator compliance — PASS

The terminator widening is a proper nom combinator, no raw string dispatch:

let (i, _) = alt((value((), nom::combinator::eof), value((), tag(", where ")))).parse(i)?;  // token.rs:306

tag(", where ") factored into the existing alt; the surrounding scan loop is the documented word-boundary-scan idiom. No contains/find/split_once/starts_with introduced. Compliant.

(b) saved_where_x_expr correctness + no EOF regression — PASS

Traced end-to-end:

• With a where-clause: entry_clause now matches (terminator accepts ", where "), saved_where_x_expr captures the expression from the pre-truncation text &text[pos..] via the existing extract_token_where_x_expression (token.rs:42-43), then text is truncated to text[..pos]. The where clause is gone from suffix, so extract_token_where_x_expression(suffix) is None and .or(saved_where_x_expr) supplies it (token.rs:385). It feeds the existing typed parse_where_x_quantity_expression → parse_cda_quantity → parse_quantity_ref path, which yields CountersOn{Source, Plus1Plus1} for Anim Pakal. Idiomatic — no stringly-typed binding.
• No EOF-path leak: when there is no where-clause (Leonin Warleader / Parhelion II), &text[pos..] contains no "where x is", so saved_where_x_expr is None and .or(None) is a no-op. The count-rebind guard at token.rs:389 only fires when count == Variable("X"), so fixed-count tokens are untouched.
• Existing EOF tests (plural_that_are_tapped_and_attacking_suffix_strips, plural_that_are_attacking_suffix_strips_without_tapping) still pass — eof remains the first alt arm.

(c) Class-generality — PASS

The terminator is structural (, where ), not a card-name match, so it covers the class of "non-copy token + entry-modifier + variable-X rebind." Card-data survey of tapped and attacking token cards: 2 carry a , where tail (Anim Pakal; Nacatl War-Pride — though Nacatl is a copy-token that routes through the separate parse_copy_token_entry_modifiers branch, so this path's current primary beneficiary is Anim Pakal). Any future non-copy entry-modifier token with a trailing where-X binding is now covered. Existing building blocks reused, nothing re-implemented.

(d) Test discrimination + CR — PASS

tapped_and_attacking_with_trailing_where_x_clause drives the real try_parse_token entry (production-faithful, not a constructor shortcut) and asserts all three regressed properties: tapped == true, enters_attacking == true, and count == CountersOn{Source, Plus1Plus1}. On origin/main the clause fails the eof anchor at every scan position, so all three assertions fail; with the fix all three pass. Genuinely discriminating.

• CR 107.3 grep-verified in docs/MagicCompRules.txt:464 ("Many objects use the letter X as a placeholder for a number…"; 107.3f covers "X appears in the text") — correctly describes the where-X binding. CR 508.4 / 506.3a annotations also verified.

Notes (non-blocking)

• LOW (pre-existing, not introduced here): pos is a byte offset into lower_trimmed (lowercased) but is used to slice the original-case text at both &text[..len] (token.rs:322, pre-existing) and the new &text[pos..] (token.rs:43). This is only safe while every character before the clause has identical byte length under to_lowercase(). True for ASCII token descriptors (Gnome/Cat/Angel, source normalized to ~), so not reachable in practice — flagging only because the PR adds a second consumer of the same offset. No action required.

[natefinch]

Review: PR #2807 — capture tapped-and-attacking when trailed by where-X clause

VERDICT: approve

Independent pass (review-impl lenses). Confirms the prior review and adds traced evidence.

Correct seam — PASS

Fix lives in parse_token_description (crates/engine/src/parser/oracle_effect/token.rs), the single authority for non-copy token entry-modifier + count parsing. No downstream symptom-patch. Right layer.

Bug is live on main / fix reachable — PASS

git show origin/main:.../token.rs still has the hard eof(i)? anchor at line 306, so #2379 is genuinely unfixed on origin/main. With a trailing , where x is … the attacking clause fails eof at every scan position, leaving enters_attacking/tapped both false. This PR widens the terminator to alt((eof, tag(", where "))) — class-general, structural, no card-name match.

Combinator + no-regression trace — PASS

• Terminator widening is proper nom (value((), tag(", where ")) folded into the existing alt); no contains/find/starts_with introduced.
• saved_where_x_expr is captured from the pre-truncation &text[pos..] via the existing extract_token_where_x_expression, then text is truncated to text[..pos]. The where tail is gone from suffix, so extract_token_where_x_expression(suffix).or(saved_where_x_expr) (line 385) supplies it and feeds the existing typed parse_where_x_quantity_expression → parse_cda_quantity path → CountersOn{Source, Plus1Plus1} for Anim Pakal.
• No EOF-path leak: with no where-clause, &text[pos..] has no "where x is", so saved_where_x_expr is None and .or(None) is a no-op; the count rebind at line 389 only fires when count == Variable("X"), so fixed-count tokens (Leonin Warleader / Parhelion II) are untouched. eof remains the first alt arm, so existing EOF suffix tests are unaffected.

Test discrimination — PASS

tapped_and_attacking_with_trailing_where_x_clause drives the real try_parse_token entry (not a constructor shortcut) and asserts all three regressed properties (tapped, enters_attacking, count == CountersOn{Source, Plus1Plus1}). All three fail on origin/main, all pass with the fix. Genuinely discriminating.

CR — PASS

CR 107.3/107.3f (X placeholder defined by text), CR 508.4 (creature put onto battlefield attacking), CR 506.3a (noncreature permanent attacking) all grep-verified in docs/MagicCompRules.txt and accurately describe the annotated code.

Note (non-blocking, pre-existing)

LOW: pos is a byte offset into the lowercased lower_trimmed but slices original-case text at both &text[..len] (pre-existing line 322) and the new &text[pos..]. Safe only while every char before the clause has identical byte length under to_lowercase() — true for ASCII token descriptors (source normalized to ~), so not reachable in practice. Flagging only because this PR adds a second consumer of the same offset; no action required.

[matthewevans]

Maintainer sign-off: class-general nom terminator widening at the single token-description authority; bug live on main (#2379), discriminating unit test; Gemini clean + two independent review-impl approvals; CI green.