feat(parser): dynamic P/T pump scaling by source intensity (Minthara/Teysa/Quickbeast)

[nickmopen]

Summary

Parse the dropped intensity-scaling P/T pump — get/gets +X/+0 or +X/+X, where X is <source>'s intensity — for Minthara of the Absolute, Teysa of the Ghost Council, and Quickbeast Amulet. Grammar-only: QuantityRef::Intensity { scope: Source } is already runtime-evaluated; the parser simply had no producer for it on the P/T-pump axis (it was parse-side dead).

Implementation method

Hand-authored (not via /engine-implementer). Per review on this PR, the parse is a single combinator — terminated(nom_quantity::parse_self_possessive, tag(\" intensity\")) inside nom_parse_lower — wired into the existing where X quantity chain in parse_dynamic_pt_in_text. No new vocabulary; it reuses the existing self-possessive axis, so it also covers its / this card's / the gendered forms for free.

CR references

None — Intensity is digital-only Alchemy with no Comprehensive Rules entry; the doc comment states that rather than inventing a CR number.

Files changed

• parser/oracle_static/anthem.rs — parse_source_intensity_where_x + .or_else into the where X chain.
• parser/oracle_nom/quantity.rs — parse_self_possessive made pub(crate).
• parser/oracle_static/mod.rs — prelude: add nom_parse_lower + nom_quantity alias.
• parser/oracle_static/tests.rs — dynamic_pt_pump_scales_by_source_intensity (typed AST; both fixtures use the normalized ~'s intensity form the card actually produces post-normalize_card_name_refs).

Verification (local, committed head d1d0bfc0b1b15010095d058df69ef7846d15daba)

• cargo test -p engine --lib dynamic_pt_pump_scales_by_source_intensity — ok (1 passed)
• cargo test -p engine --lib parser::oracle_static — ok (1112 passed, 0 failed)
• cargo fmt -p engine -- --check — clean
• cargo clippy -p engine --lib (-D warnings) — clean
• scripts/check-parser-combinators.sh <base> — pass (now a combinator, not verbatim matches!)
• scripts/check-engine-authorities.sh <base> — pass

Transparency: I did not run scripts/ai-gate.sh (Gate A) this session — it does a fresh --release build in a separate target dir and the local disk was ~100% full. I'm not fabricating a Gate A PASS line. CI's full suite (parser gate + both test shards) is green on this head.

Anchored on

• crates/engine/src/parser/oracle_nom/quantity.rs:2140 — parse_self_possessive, the self-possessive axis this reuses.
• crates/engine/src/parser/oracle_static/anthem.rs — the existing where X quantity chain (parse_cda_quantity / parse_event_context_quantity) this arm extends.

Claimed parse impact

• Minthara of the Absolute — +X/+0 pump now scales by source intensity
• Teysa of the Ghost Council — +X/+0 pump now scales by source intensity
• Quickbeast Amulet — +X/+X pump now scales by source intensity (both axes)

[gemini-code-assist]

Code Review

This pull request introduces parsing support for the digital-only Alchemy mechanic "where X is 's intensity" to handle dynamic power/toughness pumps scaling by a source's intensity, along with corresponding unit tests. The review feedback correctly points out a violation of the repository's architectural rule (R1) regarding the use of verbatim string matching instead of nom combinators for parsing Oracle phrases, and provides a modular nom-based parser implementation as a replacement.

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] Avoid verbatim string matching for parsing Oracle phrases. Use nom combinators to build a composable parser instead.

Why it matters: Verbatim string matching is fragile, bypasses the robust nom-based parser, and creates a maintenance burden. Using nom combinators allows us to handle all variations (like this planeswalker's intensity or this land's intensity) automatically without proliferating hardcoded strings.

This violates the repository rule against verbatim string equality for parsing Oracle phrases.

fn parse_source_intensity_where_x(wx: &str) -> Option<QuantityExpr> {
    use nom::{
        branch::alt,
        bytes::complete::tag_no_case,
        character::complete::{alpha1, space1},
        combinator::{all_consuming, value},
        sequence::tuple,
    };

    let input = wx.trim().trim_end_matches('.');
    let subject = alt((
        tag_no_case("~'s"),
        tag_no_case("its"),
        value(
            (),
            tuple((
                tag_no_case("this"),
                space1,
                alpha1,
                tag_no_case("'s"),
            )),
        ),
    ));

    let mut parser = all_consuming(tuple((
        subject,
        space1,
        tag_no_case("intensity"),
    )));

    parser(input).ok().map(|_| QuantityExpr::Ref {
        qty: QuantityRef::Intensity {
            scope: ObjectScope::Source,
        },
    })
}

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 or delegate to existing helpers. Verbatim string equality on full Oracle phrases is highly prohibited. ^(link)
2. 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.

[matthewevans]

The defect is real, the population is exactly right, and the target type is the correct one — but the parse is hand-rolled string matching where an existing combinator already covers the axis, and 6 of the 7 arms are unreachable on the card pipeline. Request changes; the fix is ~3 lines.

I verified the premise before the diff. All three cards are real, are in the pool, and today produce no statics at all on main (client/public/card-data.json: statics: none, no AddDynamicPower, no Intensity for Minthara of the Absolute, Teysa of the Ghost Council, and Quickbeast Amulet). The pump really is dropped, and these three are precisely the intensity cards whose X scales a P/T pump — the other 17 intensity cards scale damage, draw, mana, or conjure, so they belong to different seams. The scope is correct.

🔴 Blocker 1 — Nom combinator mandate: matches! on verbatim Oracle strings

crates/engine/src/parser/oracle_static/anthem.rs:1189-1210 dispatches with

let is_source_intensity = matches!(
    normalized.as_str(),
    "~'s intensity" | "its intensity" | "this creature's intensity" | ...
);

This is the pattern CLAUDE.md names as the single most prohibited one in the codebase ("NEVER match on verbatim Oracle text strings"), and it enumerates a permutation set the nom mandate explicitly says to factor ("Compose nom combinators, don't enumerate permutations… never expand into N! individual tag("full string") alternatives").

The axis is already a building block. parse_self_possessive — crates/engine/src/parser/oracle_nom/quantity.rs:2140 — is exactly this:

alt((tag("its"), tag("~'s"), tag("this creature's"), tag("this card's"),
     tag("his"), tag("her"), tag("their")))

So the whole function collapses to one terminated over the existing combinator (make parse_self_possessive pub(crate)):

fn parse_source_intensity_where_x(wx: &str) -> Option<QuantityExpr> {
    nom_parse_lower(wx.trim().trim_end_matches('.'), |i| {
        value(
            QuantityExpr::Ref {
                qty: QuantityRef::Intensity { scope: ObjectScope::Source },
            },
            terminated(parse_self_possessive, tag(" intensity")),
        )
        .parse(i)
    })
}

That is strictly broader coverage than the seven hand-written arms (it also picks up this card's and the gendered forms), in fewer lines, with no new vocabulary.

Note that CI's parser gate will not catch this — scripts/check-parser-combinators.sh's FORBIDDEN_MATCH_ARM regex requires a => after the literal, and matches!(s, "a" | "b") has none; FORBIDDEN_VERBATIM_EQ requires ==/!=. A green gate here is a blind spot, not a pass. I'm filing that separately — it is not your bug.

🔴 Blocker 2 — 6 of the 7 arms are dead code, and the Quickbeast test only passes by bypassing normalization

parse_oracle_text normalizes the entire Oracle text through normalize_card_name_refs (oracle.rs:391) before any line reaches the static parser. That normalizer rewrites every phrase in SELF_REF_TYPE_PHRASES (oracle_util.rs:867-886 — including this creature, this permanent, this artifact, this enchantment, this equipment) to ~, and it resolves of-form legendary short names (oracle_util.rs:2167-2217).

Traced per card, this is what the static parser actually receives:

Card	Printed Oracle	After normalization
Minthara of the Absolute	where X is Minthara of the Absolute's intensity	where X is ~'s intensity
Teysa of the Ghost Council	where X is Teysa's intensity	where X is ~'s intensity (short name via the of path)
Quickbeast Amulet	where X is this Equipment's intensity	where X is ~'s intensity

All three funnel through ~'s intensity — the one arm that matters. The five "this <type>'s intensity" arms and "its intensity" are unreachable on the card pipeline.

The reason this isn't visible from inside the PR is the test helper: parse_static_line_multi (oracle_static/shared.rs:874) takes raw text and does not apply normalize_card_name_refs. So the Quickbeast assertion

parse_static_line_multi("Equipped creature gets +X/+X, where X is this Equipment's intensity.")

drives a branch the real card can never reach, and therefore does not demonstrate that Quickbeast Amulet parses. This file already documents the convention it should follow — oracle_static/tests.rs:25748: "(see normalize_card_name_refs), so the AST test feeds the ~ form."

Fix: feed the normalized form (where X is ~'s intensity) in the Quickbeast fixture. The +X/+X axis assertion is worth keeping — it's the discriminating half — it just needs to be reached by a phrase the card can actually produce.

🟡 Non-blocking

• The parse-diff sticky (<!-- coverage-parse-diff -->) hasn't been posted yet — the Card data (generate, validate, coverage) job is still pending. It is required evidence here: on a corrected head it should show exactly these three cards flipping to a dynamic pump and nothing else. I'll confirm against it rather than against the claim.
• The AI-contributor template sections are missing from the PR body (summary / files changed / track / llm / verification). This has now recurred across several of your PRs; please fill it in — it's what lets the gates run against your head instead of a maintainer reconstructing scope by hand.

✅ Clean

• Premise verified, not asserted. The three cards are real and their Oracle text matches the PR body verbatim. The abbreviation of "Quickbeast Amulet" to "Quickbeast" in the title briefly read as a fabricated card name; it isn't.
• Right target type, no new vocabulary. Reusing QuantityRef::Intensity { scope: ObjectScope::Source } is correct: game::quantity already resolves it (quantity.rs:364/638/833) and coverage.rs:1332/7160 already projects it. This variant had no parser producer at all before this PR — it was parse-side dead. Making the grammar the only missing piece is the right diagnosis.
• No fabricated CR annotation. Intensity is digital-only Alchemy with no Comprehensive Rules entry, and the doc comment says exactly that instead of inventing a plausible-looking CR number. That is the correct call and it matches the existing annotations on the type.
• The +X/+0 assertion is discriminating — it asserts toughness is not pumped, so it would fail a naive AddDynamicPower+AddDynamicToughness implementation.

Recommendation: request-changes. Swap the hand-rolled matches! for terminated(parse_self_possessive, tag(" intensity")), drop the now-redundant arms, and re-point the Quickbeast fixture at the normalized ~ form. That should be a small push, and the parse-diff sticky on the new head is what I'll approve against.

[github-actions]

Parse changes introduced by this PR · 3 card(s), 4 signature(s) (baseline: main 79befe8ae9ac)

3 card(s) · ability/static_structure · removed: static_structure

Examples: Minthara of the Absolute, Quickbeast Amulet, Teysa of the Ghost Council

1 card(s) · static/Continuous · added: Continuous (affects=equipped by self creature, mods=add dynamic power, add dynamic toughness)

Examples: Quickbeast Amulet

1 card(s) · static/Continuous · added: Continuous (affects=you control creature Spirit, mods=add dynamic power)

Examples: Teysa of the Ghost Council

1 card(s) · static/Continuous · added: Continuous (affects=you control creature, mods=add dynamic power)

Examples: Minthara of the Absolute

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

[nickmopen]

Thanks — precise review, and the combinator was right there. Addressed in d1d0bfc0b:

Blocker 1 (nom mandate): dropped the matches!-on-verbatim-strings and replaced the whole body with your suggested form —

nom_parse_lower(wx.trim().trim_end_matches('.').trim(), |i| {
    value(
        QuantityExpr::Ref { qty: QuantityRef::Intensity { scope: ObjectScope::Source } },
        terminated(nom_quantity::parse_self_possessive, tag(" intensity")),
    ).parse(i)
})

Made parse_self_possessive pub(crate) and exposed nom_parse_lower + a nom_quantity alias in the oracle_static prelude. Strictly broader than the seven hand arms (picks up this card's and the gendered forms), fewer lines, no new vocabulary. The parser-combinator gate passes — and thanks for flagging its blind spot on matches!(…| …); that's a real gap worth its own fix.

Blocker 2 (dead arms / bypassed normalization): you're right that everything funnels to ~'s intensity — the combinator now expresses the reachable axis directly instead of enumerating unreachable spellings. The Quickbeast fixture now feeds the normalized where X is ~'s intensity form (with a comment citing normalize_card_name_refs, matching the convention at tests.rs:25748), keeping the discriminating +X/+X two-axis assertion.

Non-blocking: filled in the AI-contributor template (summary / method / files / verification / anchors / claimed impact). One honest note: I did not run scripts/ai-gate.sh (Gate A) — it does a fresh --release build in a separate target dir and my local disk was ~100% full, so rather than fabricate a PASS line I left it out; CI's parser gate + both test shards are green on this head. The card-data parse-diff sticky should now show exactly the three cards flipping to a dynamic pump.

Local: parser::oracle_static 1112 passed; fmt / clippy -D warnings / parser-combinator + engine-authorities gates clean.

[matthewevans]

Both blockers resolved on d1d0bfc0 — approving. The verbatim-string dispatch is gone, replaced by the exact shared combinator, and the fresh parse-diff confirms the rewrite is not inert.

✅ Clean

The nom-mandate blocker is fixed at the right seam. The seven-arm matches! is now:

terminated(nom_quantity::parse_self_possessive, tag(" intensity"))

parse_self_possessive (oracle_nom/quantity.rs:2140) already was the alt this needed — promoting it to pub(crate) and composing against it is the correct move, and it widens coverage for free: the whole its / ~'s / this creature's / this card's / his / her / their class now rides one path instead of an enumerated list that would drift the moment a new templating appeared.

The dead-arm blocker dissolves by construction. With the enumeration gone there are no unreachable arms left to carry. The new doc comment states the reason explicitly — normalize_card_name_refs funnels every card-name and permanent-type self-reference to ~ upstream, so ~'s intensity is what the static parser actually receives. That is the right thing to write down: it is the non-obvious fact that made six of the original seven arms unreachable, and it is invisible from inside anthem.rs.

Verified non-inert against the refreshed sticky, not inferred. A refactor that swaps a hand-rolled predicate for a shared combinator is precisely where a fix can go silently no-op, so I waited for the card-data job to re-run on d1d0bfc0 rather than reasoning from the alt contents. The parse output is identical to the previous head — 3 cards, 4 signatures:

• Minthara of the Absolute → Continuous (affects=you control creature, mods=add dynamic power)
• Teysa of the Ghost Council → Continuous (affects=you control creature Spirit, mods=add dynamic power)
• Quickbeast Amulet → Continuous (affects=equipped by self creature, mods=add dynamic power, add dynamic toughness)

Scope is exactly the claimed population — no collateral. That population is also complete: these are the only intensity cards whose X scales a P/T pump; the others scale damage, draw, mana, or conjure and are untouched.

The test now drives the branch the cards actually reach. Feeding ~'s intensity (the post-normalization form) rather than the printed this Equipment's form is the correction that matters — the previous version passed only because parse_static_line_multi skips normalization, so it exercised a path no real card could take. The +X/+0 case also asserts toughness is not pumped, which makes it discriminating on both axes: delete parse_source_intensity_where_x and parse_cda_quantity/parse_event_context_quantity fall through to None, the static disappears, and the length assert fails.

QuantityRef::Intensity { scope: Source } was already modeled and runtime-evaluated — this stayed a grammar-only change and reused the existing seam instead of adding a parallel one.

Approving and enqueueing on green. Labeled bug: the three cards were dropping their pump entirely on main (no statics at all), so this is wrong→right regardless of the feat( title.