fix(parser): Kinetic Ooze p0 — bind 'any number of target creatures' on typed counter-doubling

[jaytbarimbao-collab]

Fixes #5247.

Tier: Frontier
Model: claude-opus-4-8

Summary

Kinetic Ooze breaks the game (p0) when it enters: casting it raises Invalid action: Unused selected target slots at ChooseTarget. Its X≥10 ETB clause, "double the number of +1/+1 counters on any number of other target creatures", lowered to a single required creature target instead of a variable "any number" (min 0) multi-target. This restores the multi-target bound so the clause offers 0-or-more targets and resolves cleanly.

Implementation method (required)

• Produced via the /engine-implementer pipeline
• Not /engine-implementer

A two-line parser fix (a recognizer and its dispatch gate) with a cast→resolve integration test. Root cause and the corrected AST were verified by probing the real card; no engine/runtime changes.

Root cause (CR 115.1d + CR 701.10e)

"any number of … target creatures" is a variable-count multi-target (min 0), recovered post-hoc by extract_double_counter_multi_target (lower.rs). Two gaps kept it from firing on Kinetic Ooze:

1. The extractor's prefix matched only "double the number of each kind of counter on " — not a typed counter ("double the number of +1/+1 counters on …").
2. Its dispatch (mod.rs) only ran for Effect::Double { Counters }, but a typed counter ×2 lowers to Effect::MultiplyCounter, which was never routed to the extractor.

So clause.multi_target stayed None; the MultiplyCounter bound a single required Typed<Creature>. At resolution the deferred (X≥10) target slot then didn't match the chain's consumed slots → the panic.

Fix

1. Broaden the extractor (lower.rs) to consume "double the number of <descriptor> counter(s) on " — the descriptor is either "each kind of" (unchanged, via the singular " counter on " arm) or a typed counter (" counters on "). Then apply the existing any number of … target check.
2. Route MultiplyCounter to it (mod.rs) alongside Effect::Double { Counters }.

Verified parse: the MultiplyCounter clause now carries multi_target { min: 0, max: None } (was absent).

CR references

CR 115.1d ("any number of target" = min-0 multi-target), CR 701.10e (counter doubling), CR 601.2c (conditional target selection).

Verification

• New cast→resolve integration test kinetic_ooze_x10_doubles_counters_on_other_target_creature_without_panic: casts Kinetic Ooze with X=10 targeting an other creature that has one +1/+1 counter, asserts the ETB resolves to Priority (no "Unused selected target slots" panic) and the creature's counters double 1 → 2. Fails on main (panics).
• Full parser lib suite 7980 passed / 0 failed — the extractor broadening + MultiplyCounter routing regress none of the 53 "double the number of … counters on …" cards. Parser-combinator gate + clippy clean.

[gemini-code-assist]

Code Review

This pull request addresses issue #5247 by updating the extract_double_counter_multi_target parser to support typed counters and singular/plural variations, preventing panics on cards like Kinetic Ooze. An integration test has been added to verify this behavior. The review feedback correctly identifies a high-severity greedy matching bug in the parser where take_until can scan past clause boundaries, and provides an idiomatic fix using nom combinators to ensure robust parsing.

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]

[HIGH] Greedy matching bug in extract_double_counter_multi_target due to unbounded take_until inside alt.

Why it matters

Using take_until inside alt to match " counters on " or " counter on " can scan arbitrarily far ahead and cross clause boundaries. For example, if the input is "double the number of each kind of counter on target creature with two +1/+1 counters on it", the first arm of alt (take_until(" counters on ")) will match all the way to the second clause's " counters on ", leaving "it" as the target_text. Swapping the arms causes a similar failure on inputs with a plural descriptor and a singular relative clause.

Suggested fix

Use take_until(" on ") to find the first " on " (which is guaranteed to be the boundary of the descriptor since no counter descriptor contains " on "), and then validate that the extracted descriptor ends with " counter" or " counters" using a non-greedy nom parser. Ensure that we propagate parsing errors directly instead of swallowing them with .ok().

    let (target_text, descriptor) = preceded(
        tag::<_, _, OracleError<'_>>("double the number of "),
        terminated(
            take_until::<_, _, OracleError<'_>>(" on "),
            tag(" on "),
        ),
    )
    .parse(lower.as_str())?;

    let _ = terminated(
        take_until::<_, _, OracleError<'_>>(" counter"),
        terminated(
            alt((
                tag::<_, _, OracleError<'_>>(" counters"),
                tag::<_, _, OracleError<'_>>(" counter"),
            )),
            eof,
        ),
    )
    .parse(descriptor)?;

References

1. R1. Nom combinators on the first pass — no exceptions. Every new parser dispatch must use nom combinators and avoid greedy or fragile matching patterns. ^(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.

[github-actions]

Parse changes introduced by this PR · 1 card(s), 1 signature(s) (baseline: main 5e003e1f9b38)

1 card(s) · ability/MultiplyCounter · field targets: ∅ → 0+

Examples: Kinetic Ooze

[matthewevans]

Approving — a p0 panic fix that routes a second effect into the existing extractor instead of forking a parallel one, with a claim that matches its measurement exactly. This is the shape these PRs should have.

✅ Verified, not taken on the body's word

• The card is real and the text is verbatim. KINETIC_OOZE in the test matches Scryfall character-for-character, including "double the number of +1/+1 counters on any number of other target creatures." No paraphrase, so there is no room for a fabricated clause to survive.
• CR 701.10e grep-verifies and is the right rule — "To double the number of a kind of counters on a player or permanent…" — cited exactly where the doubling is lowered.
• The parse-diff is exactly the claim, with zero collateral. 1 card · 1 signature · ability/MultiplyCounter · field targets: ∅ → 0+ on Kinetic Ooze. This is the measurement that mattered most here: the fix widens a single verbatim tag("double the number of each kind of counter on ") into a take_until class match over any typed counter, and a widening like that is exactly where collateral hides. It bound one card. Claimed 1, measured 1.
• The test discriminates. issue_5247_kinetic_ooze_conditional_multi_target.rs drives a real cast → resolve through GameScenario and pins the actual failure (Invalid action: Unused selected target slots), not a proxy for it. It is registered in tests/integration/main.rs, so it is not an inert false-green.
• Right seam. Routing Effect::MultiplyCounter into the existing extract_double_counter_multi_target (mod.rs) rather than writing a second extractor is the correct call — one authority recovers the MultiTargetSpec for both counter-doubling shapes. The alt ordering is also right: the plural " counters on " arm takes typed counters, and the singular " counter on " arm preserves the "each kind of counter on" form unchanged.
• Combinator-first. preceded / alt / take_until / tag throughout, and every matches! in the diff dispatches on enum variants (Effect::MultiplyCounter, Effect::Double), never on Oracle text.

🟡 Non-blocking

• The template gate flags you, and it is a formatting artifact — worth a 10-second fix next time. The audit gate reports invalid_implementation_method and wants anchored on / gate a / final review-impl. Those three are /engine-implementer pipeline sections, and you honestly declared Not /engine-implementer with a justification, so they do not apply and I have not recorded a template-gap signal against this PR. The method_valid: false is almost certainly the bold markers inside the checkbox — - [x] **Not** \/engine-implementer`— defeating the parser. Dropping the**` should make the gate read your declaration correctly.

Recommendation: approve, label bug + quality, enqueue. bug because a p0 panic is wrong→right regardless of the fix(parser): title. quality because it is one-shot with no rework round, the body's card count equals the sticky's measured count on the same card, it extended an existing authority instead of forking a path, and it ships a discriminating test that fails on main. Closes #5247.