fix(parser): Nefarious Lich damage-substitution replacement + counted graveyard exile

[jaytbarimbao-collab]

Fixes #5649.

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

Summary

Nefarious Lich's defining mechanic — "If damage would be dealt to you, exile that many cards from your graveyard instead. If you can't, you lose the game." — silently dropped to an inert standalone ChangeZone ability: the substitution never fired as a replacement, the impossibility rider was lost, and the count was unbound. This makes it parse as a proper DealtDamage substitution replacement that exiles that many (= the damage amount) cards from the graveyard, with the lose-the-game rider gated on the exile being impossible.

Scope — this is "build for the class, not the card". The graveyard-exile count fix (part 2) repairs the same defect on 9 cards total, all silently dropping their count on main; the parse-diff sticky reports exactly these:

Card	Oracle	Fix
Aegis Sculptor, Bloodcurdler, Egon, God of Death	"exile two cards from your graveyard"	count → Fixed{2}
Insatiable Frugivore	"exile three …"	Fixed{3}
Kroxa and Kunoros	"exile five …"	Fixed{5}
Emeritus of Ideation, Ultimecia, Time Sorceress	"exile eight …"	Fixed{8} (were binding a nonsensical any target)
Nefarious Lich	"exile that many …"	EventContextAmount

(Emeritus and Ultimecia were binding any target for a graveyard exile; the other six dropped their count to targets: ∅. All nine now bind the correct filtered graveyard-card multi-target.)

Implementation method (required)

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

A targeted parser fix scoped to one replacement recognizer and the exile AST/lowering path, verified against the real card via probe and pinned with a unit test; no new engine effect variants or runtime semantics (reuses the existing MultiTargetSpec count path and the shared "if you can't" lowering).

Root cause

Two independent gaps, both on the damage clause:

1. The recognizer bailed on the rider. parse_damage_to_self_instead_followup (oracle_replacement.rs) matched "If damage would be dealt to you, <effect> instead" but required the remainder after "instead." to be empty. Nefarious Lich's trailing "If you can't, you lose the game." made the remainder non-empty → None → the whole clause fell through to an inert abilities: [ChangeZone] entry.

2. The exile dropped its count. "exile that many cards from your graveyard" lowered to ChangeZone { target: ParentTarget } — the count and the graveyard filter were lost. The shared count-phrase guard (parse_dynamic_count_phrase) deliberately rejects "that many cards from …" to protect the top-of-library impulse idiom, and the generic exile tail carries no quantity.

Fix

1. Accept the impossibility rider (oracle_replacement.rs). Recognize an optional "If you can't, <consequence>" tail and fold it back onto the substituted effect, so the shared "if you can't" lowering threads it as a conditional continuation (Not { ZoneChangedThisWay }). nom_on_lower returns the remainder in original case, so the rider is matched via a lowercased copy. Any other trailing text still bails, preserving the recognizer's single-clause scope.

2. Bind the graveyard-exile count via MultiTargetSpec (CR 700.4, following Forage). Effect::ChangeZone has no count slot, so the quantity rides the clause's MultiTargetSpec: a multi_target field on ZoneCounterImperativeAst::Exile, a recognizer anchored on "cards from your graveyard" (that many → EventContextAmount, number words → Fixed), and a lowering arm mirroring the Tap/Untap multi_target seam. The anchor never touches the impulse-draw guard, so that idiom is unaffected.

Result:

DealtDamage replacement (Prevention shield)
  execute: ChangeZone{ Graveyard → Exile, Typed<Card in your graveyard> }
    multi_target: exact(EventContextAmount)      // exile N = the damage amount
    └ LoseTheGame  if  Not{ ZoneChangedThisWay } // "if you can't, you lose the game"

replacements: 2, abilities: 0 (was 1 / 1 inert).

CR references

CR 614.1a (substitution replacement + impossibility rider), CR 700.4 (numeric counts), CR 608.2c (the "if you can't" prior-instruction condition).

Verification

• nefarious_lich_damage_substitution_exiles_that_many_with_impossibility_rider — asserts the DealtDamage replacement, the Graveyard → Exile execute, the EventContextAmount MultiTargetSpec, and the conditional LoseTheGame rider (the "that many" arm).
• exile_count_from_your_graveyard_binds_fixed_and_dynamic_counts — pins the Fixed arm (the eight numeric cards: "two cards from your graveyard" → Fixed{2}, "eight …" → Fixed{8}), the dynamic arm, the end-to-end ChangeZone + exact(Fixed{2}) MultiTargetSpec, and a negative that the top-of-library impulse idiom stays untouched by the possessive-graveyard anchor.
• Regression siblings unaffected — Delaying Shield / Force Bubble (counter substitutions, no rider) still parse; the new graveyard-exile recognizer fires only when the whole exile effect is "exile <count> cards from your graveyard".
• Full parser lib suite green; parser-combinator gate + clippy clean.

[gemini-code-assist]

Code Review

This pull request implements support for counted graveyard exile effects and impossibility riders, specifically targeting the damage substitution clause of Nefarious Lich. It introduces parsing for graveyard exile counts, threads the count onto the clause's MultiTargetSpec during lowering, and updates the replacement parser to handle "if you can't" impossibility riders. Feedback on the changes suggests refactoring a redundant match pattern in lower_imperative_family_ast to use a direct reference binding instead of an unreachable wildcard fallback, aligning with the codebase's idiomatic Rust guidelines.

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] Avoid redundant matching with wildcard fallbacks when the variant is already guaranteed by the outer pattern.

Why it matters: The inner match &ast uses a wildcard _ fallback which is technically unreachable and violates the exhaustive matching principle (L5). We can bind multi_target directly in the outer pattern using a reference binding, making the code cleaner and more idiomatic Rust.

        ImperativeFamilyAst::ZoneCounter(
            ast @ ZoneCounterImperativeAst::Exile {
                multi_target: Some(ref multi_target),
                ..
            },
        ) => {
            let mut clause = parsed_clause(lower_zone_counter_ast(ast));
            clause.multi_target = Some(multi_target.clone());
            clause
        }

References

1. Rule L5: Wildcard _ match arms that should be exhaustive — let the compiler catch missing variants. ^(link)

[github-actions]

Parse changes introduced by this PR

Baseline pending for bd2971dd611067ecec230ff1cbb02947ea8db38a — this populates once main publishes its coverage snapshot (a few minutes after that commit landed).

[matthewevans]

The fix is correct and it repairs eight more cards than you claimed — but the branch that drives those eight cards has no test, and the body understates the scope. Both are small.

I verified all nine affected cards against Scryfall before reading the diff. Nefarious Lich's Oracle text is exactly as quoted, and CR 614.1a / CR 700.4 / CR 608.2c are the right citations for a substitution replacement with an impossibility rider.

The parse-diff is wider than the body — and that is good news

Your body scopes this to Nefarious Lich. The sticky reports 9 cards. I checked every one, and each is the same defect class your recognizer fixes:

Card	Oracle	Before	After
Aegis Sculptor	"exile two cards from your graveyard"	targets: ∅	2-2 ✅
Bloodcurdler	"exile two cards from your graveyard"	targets: ∅	2-2 ✅
Egon, God of Death	"exile two cards from your graveyard"	targets: ∅	2-2 ✅
Insatiable Frugivore	"exile three cards from your graveyard"	targets: ∅	3-3 ✅
Kroxa and Kunoros	"exile five cards from your graveyard"	targets: ∅	5-5 ✅
Emeritus of Ideation	"exile eight cards from your graveyard"	any target	8-8, graveyard filter ✅
Ultimecia, Time Sorceress	"exile eight cards from your graveyard"	any target	8-8, graveyard filter ✅

Every one of those was silently dropping its count on main, and Emeritus/Ultimecia were binding a nonsensical any target for a graveyard exile. Each count in the sticky matches the printed Oracle number exactly. This is "build for the class, not the card" working as intended — the collateral is eight additional correct repairs.

But it means the claim and the measurement disagree, which is the one thing that has to hold. Please update the body to state the real population.

🔴 Blocker — the branch that fixes those eight cards is untested

parse_exile_count_from_your_graveyard has two arms:

alt((
    value(QuantityExpr::Ref { qty: EventContextAmount }, tag("that many")),   // ← tested
    map(parse_number, |n| QuantityExpr::Fixed { value: n as i32 }),           // ← NOT tested
))

Your test covers only the "that many" arm (Nefarious Lich). The Fixed arm is what fires for eight of the nine cards above, and nothing exercises it. Delete map(parse_number, …) today and nefarious_lich_damage_substitution_exiles_that_many_with_impossibility_rider still passes green.

Fix: add a parser test over the building block itself — a couple of number cases ("exile two cards from your graveyard" → Fixed { value: 2 }, "exile eight cards from your graveyard" → Fixed { value: 8 }) plus a negative asserting the impulse-draw idiom ("exile the top two cards of your library") is still untouched by the new anchor. That last one matters: your comment claims the possessive anchor keeps the recognizer clear of parse_dynamic_count_phrase's guard, and the test should hold you to it.

🟡 Non-blocking

• exact(N) is the right count semantics and worth stating: MultiTargetSpec::exact means min == max, so a graveyard with fewer than N cards genuinely can't satisfy it — which is what makes Nefarious Lich's "if you can't, you lose the game" and Egon's "if you can't, sacrifice" fire correctly. Good.
• tag(" cards from your graveyard") hardcodes the plural. "exile a card from your graveyard" falls through to the generic tail, same as today, so this is not a regression — just noting the axis is half-covered if you want the singular later.
• rest_text.to_ascii_lowercase() allocates a String per call inside the exile parse path. parse_exile_ast already receives a lower: &str; the TextPair offset idiom would avoid the allocation.
• Rider over-acceptance. The rider check only tags the prefix "if you can't," but then folds the entire remainder into followup_text. Trailing text past the rider would ride along and have to parse. Harmless today; tighten if it bites.

✅ Clean

• Right seam. Effect::ChangeZone genuinely has no count slot, and riding the clause's MultiTargetSpec follows the existing Forage precedent instead of adding a new effect variant. That is the correct call.
• The test drives parse_oracle_text — the real card pipeline — rather than parse_effect_chain on a fresh ParseContext. That distinction matters in this repo, and you got it right: a test on the standalone entry point would not have exercised what database::synthesis actually runs. It is strongly discriminating; the DealtDamage replacement does not exist on main.
• CR citations all grep-verified, and the assertion set (ShieldKind::Prevention, Graveyard → Exile, EventContextAmount, Not { .. }-gated LoseTheGame) pins the whole shape rather than a single field.

Recommendation: request-changes. Add the Fixed-arm parser test (plus the impulse-draw negative), and correct the body's scope to the nine cards it actually fixes. The implementation itself I would take as-is.

[jaytbarimbao-collab]

Both addressed — thanks for verifying all nine against Scryfall and catching the untested arm.

Blocker — Fixed arm now tested. Added exile_count_from_your_graveyard_binds_fixed_and_dynamic_counts (commit 001fdfb):

• Fixed arm: "two cards from your graveyard" → Fixed{2}, "eight cards from your graveyard" → Fixed{8} — the eight numeric cards.
• dynamic arm: "that many …" → EventContextAmount.
• end-to-end: "exile two cards from your graveyard" rides a ChangeZone{Graveyard→Exile} with exact(Fixed{2}) on the clause MultiTargetSpec.
• negatives, exactly as you asked: "the top two cards of your library" and "two cards from the top of your library" both return None — the possessive "your graveyard" anchor stays clear of parse_dynamic_count_phrase's idiom. Deleting the map(parse_number, …) arm now fails this test.

Body updated to the real 9-card population (the table you posted) with the "build for the class" framing — no longer scoped to Nefarious Lich alone.

On the non-blocking notes, all fair:

• exact(N) = min == max semantics — added to the body.
• to_ascii_lowercase() allocation and the rider's fold-the-whole-remainder over-acceptance — I'd rather not thread the lower offset / tighten the rider tail in this PR since you're taking the implementation as-is; happy to do both as a fast follow-up.
• tag(" cards from your graveyard") hardcoding the plural (singular "a card" falls to the generic tail) — noted; no card in the pool needs the singular today.

[matthewevans]

Verdict: approved

Both blockers from the prior head are cleared, and I verified the fix is live rather than taking the diff's word for it.

Blocker 1 — untested Fixed arm: fixed

exile_count_from_your_graveyard_binds_fixed_and_dynamic_counts now pins both arms plus the guard:

• Fixed arm (two → Fixed{2}, eight → Fixed{8}) — the eight numeric cards. Deleting map(parse_number, Fixed) now fails the suite; on the prior head it did not.
• Dynamic arm (that many → EventContextAmount) — Nefarious Lich.
• Two negatives pinning the top-of-library impulse idiom (the top two cards of your library, two cards from the top of your library → None). These are load-bearing: the combinator's rest.trim().is_empty() anchor is the only thing keeping the possessive your graveyard tail from capturing them, and nothing else in the suite would notice if it did.
• End-to-end asserting the count rides MultiTargetSpec::exact(Fixed{2}) — the exact seam I flagged.

Blocker 2 — body population: fixed

The body now states the real 9-card population card-by-card. I independently confirmed all nine against Scryfall last round: every one is a genuine "exile N cards from your graveyard" text, each count matching the printed Oracle number, and every one was silently dropping its count on main. The wide diff is eight additional correct repairs, not scope creep.

Measurement

The parse-diff sticky is baseline-pending (main moved to bd2971dd mid-review), so I did not review against it. Head-authoritative numbers from the Card data job on 001fdfbf:

• REGRESSED (engine): 0 · REGRESSED (coverage honesty): 0 · ORACLE CHANGED: 0
• GAINED: 2 — Emeritus of Ideation, Ultimecia, Time Sorceress
• Diagnostic improvement: target-fallback 57 → 55

The +2 is not in tension with the 9-card claim: only those two flipped unsupported → supported. The other seven already counted as supported while emitting a ChangeZone with the count silently dropped — a misparse is invisible to the coverage counter, which is precisely why this class of defect accumulates. The regression rows are the instrument that matters here, and they are clean.

Parser mandate

Manually grepped the diff for the shapes check-parser-combinators.sh cannot see (matches! alternation, .rfind(, .split(, char-literal trim_*_matches). Clean: every matches! is on a typed enum variant in a test assertion, and .trim_end_matches('.') is punctuation stripping, which CLAUDE.md explicitly permits as a structural use. The combinator is proper nom — alt over the two count forms, tag anchor, is_empty terminator.

Nice work on the negatives in particular — they are the part most contributors skip.