fix(parser): split imperative sibling sentence after a token's quoted ability (Alien Invasion)

[galuis116]

Problem

Alien Invasion (#5844) reads:

At the beginning of combat on your turn, create a 1/1 red Alien creature token with haste and "This token attacks each combat if able." Put a +1/+1 counter on it for each invasion counter on this enchantment, then put an invasion counter on this enchantment.

The engine created one Alien token per invasion counter and the "+1/+1 counter � for each invasion counter" pump was missing entirely.

Root cause

The clause splitter (split_clause_sequence) closes a sentence after a token/permanent's quoted granted ability (a span ending in .") only when the following sentence starts with a whitelisted continuation (then / if / otherwise / may have / emblem head). A fresh imperative sibling sentence was not recognized, so the whole remainder was swallowed into the token-creation clause:

• its for each invasion counter was mis-read as the token count â�� Token { count: CountersOn(invasion) } (one token per counter), and
• the +1/+1 counter on it pump was dropped.

Isolation probe: with the quoted ability removed, the same card parses correctly (Token { count: 1 } + PutCounter { P1P1, CountersOn(invasion), LastCreated } + PutCounter { invasion, 1, SelfRef }) â�� confirming the quoted-ability sentence boundary is the sole trigger.

Fix

Extend quote_closes_sentence_before_sequence to also close the sentence when the continuation begins with an imperative game-action verb, via a new starts_imperative_action_continuation helper. It reuses the shared clause-starter verb table (starts_clause_text_lower) but first excludes the pronoun/determiner subject starters it also recognizes (it / that / this / those / they / you / all / each / the). Those, after a granted quote, are anaphors the token-creation path resolves inline ("It becomes a 2/2 â�¦", "The token is goaded"), so they stay attached; only a bare imperative verb marks a genuine sibling effect.

This is a class-level fix: every create a token with "<ability.>" <imperative on the token> card benefits, not just Alien Invasion.

Tests

• sequence.rs:
  • quoted_grant_splits_before_imperative_sibling_sentence â�� the Alien Invasion remainder splits off.
  • quoted_grant_keeps_anaphoric_subject_continuation â�� an "It gains â�¦" anaphor stays attached.
  • Existing guardrails still pass: quoted_grant_keeps_nonrecognized_capitalized_continuation ("The token is goaded") and quoted_grant_splits_before_following_sentence (Requiem Monolith "may have").
• oracle_effect/tests.rs:
  • alien_invasion_creates_one_token_and_pumps_it_per_counter parses the full card and asserts (1) the token count is a literal 1, (2) the "+1/+1 counter â�¦ for each invasion counter" pump survives as a PutCounter on the last-created token, (3) the enchantment still accrues one invasion counter.

Full engine lib suite green (16,682 passed, 0 failed; snapshot tests included); cargo fmt and clippy -p engine --tests clean.

Closes #5844

[gemini-code-assist]

Code Review

This pull request addresses issue #5844 (CR 111.3 + CR 608.2c) by allowing a token/permanent granted-ability quote that ends a sentence to be followed by a fresh imperative sibling sentence. It introduces starts_imperative_action_continuation to identify these imperative continuations and split them, while keeping anaphoric-subject continuations attached. Feedback points out a violation of Rule R1 (Nom combinators on the first pass) because starts_imperative_action_continuation uses .split_whitespace() for parsing dispatch instead of nom combinators.

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.

[gemini-code-assist]

[MEDIUM] Architectural rule violation: use of string manipulation for parsing dispatch.

Evidence: crates/engine/src/parser/oracle_effect/sequence.rs:1447.

Why it matters: The project style guide (R1) mandates using nom combinators for all new parser dispatch logic to ensure consistency and robustness. The use of .split_whitespace() deviates from this architectural rule. Furthermore, we must avoid verbatim string equality for parsing Oracle phrases, and instead decompose compound phrases into modular, reusable parsers. Finally, avoid swallowing parsing errors with .ok(); propagate unexpected errors to prevent masking critical bugs.

Suggested fix: Replace the string split with a nom combinator like take_till to extract the first word, and propagate the error using ?.

    let (remainder, first_word) = nom::bytes::complete::take_till::<_, _, OracleError<'_>>(char::is_whitespace)(remainder_lower)?;

References

1. R1. Nom combinators on the first pass — no exceptions. Every new parser dispatch under crates/engine/src/parser/ must use nom 8.0 combinators. The use of .split_whitespace() for parsing dispatch is a violation of this rule. ^(link)
2. Avoid swallowing all errors (e.g., using .ok()) when only specific errors are expected to be handled or ignored. Propagate unexpected errors (such as invariant violations or invalid actions) to prevent masking critical bugs.
3. Avoid verbatim string equality for parsing Oracle phrases as it bypasses the robust nom-based parser and creates fragile matches. Instead, decompose compound phrases into modular, reusable parsers for constituent parts (e.g., subjects, conjunctions) and compose them using idiomatic combinator aggregates (like nested alt and tag sequences) to prevent combinatorial explosion and improve maintainability.

[github-actions]

Parse changes introduced by this PR · 10 card(s), 14 signature(s) (baseline: main be1c45920bb7)

3 card(s) · ability/Draw · added: Draw

Examples: Swiftspear's Teachings, Valiant Farewell, Verdant Rebirth

2 card(s) · ability/CreateDelayedTrigger · removed: CreateDelayedTrigger (when=at next end step)

Examples: Faerie Aerie, Jaxis, the Troublemaker

1 card(s) · ability/ChangeZone · added: ChangeZone (target=self, to=exile)

Examples: The Grand Evolution

1 card(s) · ability/CopyTokenOf · added: CopyTokenOf (copies=triggering source)

Examples: Firion, Wild Rose Warrior

1 card(s) · ability/CreateDelayedTrigger · removed: CreateDelayedTrigger (when=at next upkeep)

Examples: Firion, Wild Rose Warrior

1 card(s) · ability/Dig · field rest_to: library → ∅

Examples: The Sprinkler of Stardust

1 card(s) · ability/PutCounter · added: PutCounter (counter=1 invasion, target=self)

Examples: Alien Invasion

1 card(s) · ability/PutCounter · field counter: 1 invasion → CountersOn { scope: Source, counter_type: Some(Generic("invasion")) } P1P1

Examples: Alien Invasion

1 card(s) · ability/PutCounter · field target: self → last created

Examples: Alien Invasion

1 card(s) · ability/Token · added: Token (token=2× +1/+1 Blue Faerie Spy (Creature Faerie Spy) with Flying)

Examples: Faerie Aerie

1 card(s) · ability/Token · added: Token (token=Food (Artifact Food))

Examples: Tasteful Offering

1 card(s) · ability/Token · field token: invasion counters on self× +1/+1 Red Alien (Creature Alien) with Haste → +1/+1 Red Alien (Creature Alien) with Haste

Examples: Alien Invasion

1 card(s) · ability/become · added: become

Examples: The Sprinkler of Stardust

1 card(s) · ability/grant trigger, grant Haste · added: grant trigger, grant Haste (affects=last created, duration=permanent, grants=grant Haste, grants=grant trigger, target=last created)

Examples: Jaxis, the Troublemaker

1 card(s) had Oracle-text changes (errata/reprint) — excluded as non-parser.

[matthewevans]

Request changes — parser dispatch must remain composable nom grammar.

🟡 Finding

[MED] crates/engine/src/parser/oracle_effect/sequence.rs:1447 splits the Oracle text with string operations in parser dispatch. Why it matters: this bypasses the repository's nom parser authority and makes phrase variation brittle. Suggested fix: express the clause split with alt(tag(...)), following the existing starts_clause_text_lower combinator pattern.

Recommendation: request-changes.

[galuis116]

Addressed in a7c0c6e.

Replaced the .split_whitespace() + matches! dispatch with a starts_anaphoric_subject combinator built from alt((tag(...))):

fn starts_anaphoric_subject(remainder_lower: &str) -> bool {
    alt((
        tag::<_, _, OracleError<'_>>("it "),
        tag("its "), tag("that "), tag("this "), tag("those "),
        tag("they "), tag("you "), tag("all "), tag("each "), tag("the "),
    ))
    .parse(remainder_lower)
    .is_ok()
}

Each starter carries a trailing space so only a complete word matches (e.g. it does not fire on item), mirroring the whitespace convention in starts_clause_text_lower's verb tags. starts_imperative_action_continuation is now !starts_anaphoric_subject(..) && starts_clause_text_lower(..) -- pure combinator dispatch, no string-method scanning or verbatim string equality.

Behavior is unchanged: the splitter tests (imperative sibling splits, anaphoric subject stays attached) and the guardrails (quoted_grant_keeps_nonrecognized_capitalized_continuation, quoted_grant_splits_before_following_sentence) all still pass. Rebased onto latest main.

[matthewevans]

Request changes — do not classify non-anaphoric clause starters as token anaphors.

[MED] crates/engine/src/parser/oracle_effect/sequence.rs:1456-1467 treats you, all, and each as anaphoric subjects. Evidence: the shared authority starts_clause_text_lower explicitly recognizes all , each / each opponent , and you as clause starters at sequence.rs:2113-2128 and 2164-2166, but starts_imperative_action_continuation rejects them before consulting that authority. Why it matters: a sentence after a granted-ability quote such as "…" You gain … or "…" Each opponent … is a fresh sibling instruction, not an anaphor to the created object; it remains glued to the token clause and can be swallowed or mis-lowered, recreating the class of bug this PR is intended to fix.

Please distinguish actual last-created anaphors from independent subject-led clauses, rather than filtering by the first word alone. Add direct splitter coverage that those You … and Each … continuations split, alongside the existing It … non-split guard. The nom conversion itself correctly resolves the earlier parser-dispatch finding.