Skip to content

fix(parser): strip ability-word prefix on subject-anchored statics (CR 207.2c)#5198

Merged
matthewevans merged 3 commits into
phase-rs:mainfrom
minion1227:minion_static_ability_word
Jul 6, 2026
Merged

fix(parser): strip ability-word prefix on subject-anchored statics (CR 207.2c)#5198
matthewevans merged 3 commits into
phase-rs:mainfrom
minion1227:minion_static_ability_word

Conversation

@minion1227

Copy link
Copy Markdown
Contributor

Summary

Fixes a class of static abilities that silently drop when their Oracle line carries a leading ability-word label (CR 207.2c — ability words are italic flavor with no rules meaning).

The subject-anchored static parsers match their subject at the start of the line, so a label like Chroma — / Fateful hour — / Protector — prevents the subject match and the whole static is lost — leaving the permanent strictly off its printed rules. (Threshold — /Metalcraft — self-pumps slipped through only because their scanner matches mid-line.)

parse_static_line_multi_inner now retries the dispatch after stripping a recognized ability-word label — whitelist-gated through the shared is_known_ability_word authority, exactly as the token-grant path in keyword_grant.rs already does. It is a strict fallback: any line the ordinary dispatch already parses is returned untouched (zero regression), and the stripped body carries no further label so the retry recurses at most once.

Recovered cards (and every future known-ability-word subject-anchored static):

  • Light from WithinChroma — Each creature you control gets +1/+1 for each white mana symbol in its mana cost.
  • Gavony IronwrightFateful hour — As long as you have 5 or less life, other creatures you control get +1/+4.
  • Echo of DuskDescend 4 — As long as there are four or more permanent cards in your graveyard, ~ gets +1/+1 and has lifelink.
  • CryptothrallProtector — Other artifact creatures you control have hexproof.
  • ClamavusProclamator Hailer — Each creature you control gets +1/+1 for each +1/+1 counter on it.

Files changed

  • crates/engine/src/parser/oracle_static/shared.rs
  • crates/engine/src/parser/oracle_modal.rs
  • crates/engine/src/parser/oracle_static/tests.rs

Anchored on

  • crates/engine/src/parser/oracle_static/keyword_grant.rs:1768 — the token-granted-ability path already strips an em-dash label with strip_ability_word_with_name and gates it on is_known_ability_word before re-dispatching the body; this change applies the identical, whitelist-gated pattern to the static-line dispatch.
  • crates/engine/src/parser/oracle_modal.rs:1602ABILITY_WORD_NAMES (CR 207.2c) is the single source of recognized ability words; the two 40K flavor words are added here so the gate accepts them.

Gate A


(exit 0)

CR references

  • CR 207.2c — ability words are italicized flavor text with no rules meaning (verified against docs/MagicCompRules.txt).

Track

Developer

LLM

Model: claude-opus-4-8
Thinking: high

Verification

  • cargo fmt --all — clean
  • ./scripts/check-parser-combinators.sh — clean (Gate A above)
  • cargo test -p engine --lib parser::oracle_static — 1008 pass / 0 fail
  • cargo test -p engine --lib parser::oracle_modal — 53 pass / 0 fail
  • New regression test ability_word_prefix_is_stripped_from_subject_anchored_statics — passes, incl. a safety case asserting an unknown em-dash label is NOT stripped (whitelist gate holds).

Scope Expansion

None.

Validation Failures

None.

CI Failures

None.

🤖 Generated with Claude Code

…R 207.2c)

Ability words are italic flavor with no rules meaning (CR 207.2c). The
subject-anchored static parsers match their subject at the start of the
line, so a leading ability-word label ('Chroma - ', 'Fateful hour - ',
'Protector - ') blocked them and the whole static silently dropped.

parse_static_line_multi_inner now retries through the dispatch after
stripping a *recognized* ability-word label (whitelist-gated via the shared
is_known_ability_word authority, exactly as the token-grant path does). It
is a strict fallback: any line the dispatch already parses is returned
untouched, so no existing coverage can regress, and the stripped body
carries no further label so the retry recurses at most once.

Also adds the two Warhammer 40K flavor ability words 'proclamator hailer'
and 'protector' to ABILITY_WORD_NAMES. Recovers Light from Within, Gavony
Ironwright, Echo of Dusk, Cryptothrall, Clamavus and the whole class of
known-ability-word subject-anchored statics.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@minion1227 minion1227 requested a review from matthewevans as a code owner July 6, 2026 14:13
@github-actions github-actions Bot added the needs-maintainer AI-contribution PR requires human triage (Non-dev track or unresolved gaps) label Jul 6, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a fallback mechanism in the static parser to strip recognized, whitelisted ability words (such as "Chroma", "Fateful hour", and newly added Warhammer 40k flavor words "protector" and "proclamator hailer") from subject-anchored static lines before parsing, preventing them from being silently dropped. It also adds corresponding unit tests. The review feedback points out a violation of Rule R6, noting that the comment added in oracle_modal.rs lacks the mandatory CR 207.2c annotation.

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.

"pack tactics",
"paradox",
"parley",
// Warhammer 40,000 Commander (40K) flavor ability words — no rules meaning.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

[MEDIUM] Missing mandatory CR annotation on rules-touching comment. Evidence: crates/engine/src/parser/oracle_modal.rs:1645.

Why it matters: Rule R6 requires every rules-touching line of engine code to carry a comment of the form CR <number>: <description> to ensure strict fidelity to the MTG Comprehensive Rules.

Suggested fix: Update the comment to include the verified CR reference (CR 207.2c).

Suggested change
// Warhammer 40,000 Commander (40K) flavor ability words — no rules meaning.
// CR 207.2c: Warhammer 40,000 Commander (40K) flavor ability words — no rules meaning.
References
  1. Every rules-touching line of engine code must carry a comment of the form CR <number>: <description> (regex CR \d{3}(\.\d+[a-z]?)?). (link)

@matthewevans matthewevans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the scoped parser fix. The fallback seam looks reasonable, but the regression proof needs to be stronger before this lands.

The new positive cases in crates/engine/src/parser/oracle_static/tests.rs only assert parse_static_line_multi(...).len() == 1 for lines 28-57. That would still pass if the stripped body parsed into the wrong static shape, affected filter, condition, or modification, so it does not prove the ability-word fallback preserves the intended semantics. Please assert the resulting StaticDefinition shape for representative cases, for example: Chroma's dynamic P/T quantity, Fateful hour's condition plus "other creatures you control" pump, Protector's Other artifact creatures hexproof grant, and keep the unknown-label negative case.

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

Parse changes introduced by this PR · 21 card(s), 21 signature(s) (baseline: main 407100377d23)

3 card(s) · ability/PutCounter · field counter: 2 additional +1/+12 P1P1

Examples: Curator Beastie, Heroic Return, Necromantic Summons

2 card(s) · ability/Draw · field count: # of named "~ in your graveyard" card# of named "~" in graveyard you control card

Examples: Frantic Inventory, Take Inventory

1 card(s) · static/Continuous · added: Continuous (affects=self, conditional=# of in battlefield you control artifact ≥ 3, mods=base power 5, base toughness 5, add type artifact, add type creature, …

Examples: Rusted Relic

1 card(s) · ability/DealDamage · field amount: # of named "~ in your graveyard" card+2# of named "~" in graveyard you control card+2

Examples: Galvanic Bombardment

1 card(s) · ability/Draw · field count: # of named "~ in all graveyards" card# of named "~" in graveyard card

Examples: Accumulated Knowledge

1 card(s) · ability/GainLife · field amount: 4*# of named "~ in each graveyard" card4*# of named "~" in graveyard card

Examples: Life Burst

1 card(s) · ability/Mana · field mana: # of named "~ in each graveyard" card of {R}# of named "~" in graveyard card of {R}

Examples: Rite of Flame

1 card(s) · ability/Pump · field p/t: +-1*# of named "~ in your graveyard" card/+-1*# of named "~ in your graveyard" card+-1*# of named "~" in graveyard you control card/+-1*# of named "~" in graveyard you control card

Examples: Compound Fracture

1 card(s) · ability/Pump · field p/t: +2*# of named "~ in your graveyard" card/+2*# of named "~ in your graveyard" card+2*# of named "~" in graveyard you control card/+2*# of named "~" in graveyard you control card

Examples: Growth Cycle

1 card(s) · ability/PutCounter · field counter: 1 P1P1PlayerCount { filter: OpponentDealtDamage { kind: Any, source: None } } P1P1

Examples: Furious Spinesplitter

1 card(s) · ability/PutCounter · field counter: 3 additional +1/+13 P1P1

Examples: Turntimber Symbiosis

1 card(s) · ability/PutCounter · field counter: 3 additional time3 time

Examples: Ravaging Riftwurm

1 card(s) · ability/PutCounter · field duration: until end of turn

Examples: Furious Spinesplitter

1 card(s) · ability/Sacrifice · field target: creature or planeswalkeryou control creature or you control planeswalker

Examples: Vraska's Fall

1 card(s) · ability/Token · field duration: until end of turn

Examples: You've Been Caught Stealing

1 card(s) · ability/Token · field token: # of named "~ in your graveyard" card× +2/+2 Black Zombie (Creature Zombie)# of named "~" in graveyard you control card× +2/+2 Black Zombie (Creature Zombie)

Examples: Undead Servant

1 card(s) · ability/Token · field token: Powerstone (Artifact Powerstone) tapped# of in graveyard zone changed this turn from Battlefield to Graveyard scoped player controls card non-land× Powerstone…

Examples: Wreck Hunter

1 card(s) · ability/Token · field token: Treasure (Artifact Treasure)# of each opponent who was dealt damage this turn× Treasure (Artifact Treasure)

Examples: You've Been Caught Stealing

1 card(s) · ability/Token · field token: Treasure (Artifact Treasure)# of target player controls artifact× Treasure (Artifact Treasure)

Examples: Cavern-Hoard Dragon

1 card(s) · ability/Token · field token: Treasure (Artifact Treasure) tapped# of opponent controls creature× Treasure (Artifact Treasure) tapped

Examples: Covetous Elegy

1 card(s) · ability/unknown · removed: unknown

Examples: Rusted Relic

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

…tics; CR-annotate 40K words (review)

matthewevans[CHANGES_REQUESTED]: the positive cases only asserted len()==1,
which would pass even if the stripped body parsed into the wrong shape. Assert
the resulting StaticDefinition by field for representative cases:
- Chroma (Light from Within): creatures-you-control affected + dynamic P/T =
  white pips in each recipient's mana cost (AddDynamicPower/Toughness ==
  ManaSymbolsInManaCost{White}).
- Fateful hour (Gavony Ironwright): OTHER creatures you control, gated on
  life <= 5 (QuantityComparison LE Fixed 5), +1/+4 pump.
- Protector (Cryptothrall): OTHER artifact creatures you control grant Hexproof.
Keeps the unknown-em-dash-label negative case (whitelist gate holds).

gemini[MED]: add the mandatory CR 207.2c annotation to the 40K ability-word
comment in ABILITY_WORD_NAMES.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@minion1227

Copy link
Copy Markdown
Contributor Author

Addressed both in 110bc87d9.

@matthewevans — stronger regression proof. The positive cases now assert the resulting StaticDefinition by field, not just len() == 1:

  • Chroma (Light from Within) — affected = creatures you control (no Another), no condition, and modifications = [AddDynamicPower, AddDynamicToughness] each equal to ManaSymbolsInManaCost { color: Some(White) } (and power == toughness).
  • Fateful hour (Gavony Ironwright) — affected = other creatures you control (FilterProp::Another), condition = QuantityComparison { comparator: LE, rhs: Fixed { value: 5 } } (your life ≤ 5), pump +1/+4.
  • Protector (Cryptothrall) — affected = other artifact creatures you control ([Creature, Artifact] + Another), modifications = [AddKeyword { Hexproof }].

The unknown-em-dash-label negative case (Vigilant Sentry — … ⇒ 0 statics) is kept, so the fallback stays whitelist-gated.

@gemini — CR annotation. Added CR 207.2c to the 40K ability-word comment in ABILITY_WORD_NAMES (verified against docs/MagicCompRules.txt:1497).

oracle_static suite 1009 pass / 0 fail; parser gate + fmt clean.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Thank you for the update and the thorough regression testing. The implementation of the whitelist-gated fallback in parse_static_line_multi_inner and the corresponding updates to ABILITY_WORD_NAMES look correct and align well with the existing patterns in the codebase. The regression tests effectively demonstrate that the parser now correctly handles these ability-word prefixes while maintaining safety for unknown labels.

@matthewevans matthewevans self-assigned this Jul 6, 2026

@matthewevans matthewevans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tightening the regression proof. One cleanup still needs to happen before this lands: the new test accidentally splits the existing Serra's Emissary rules doc comment.

At current head, crates/engine/src/parser/oracle_static/tests.rs:18-19 starts the Serra's Emissary CR 702.16 / CR 609.6 comment immediately above ability_word_prefix_is_stripped_from_subject_anchored_statics, and line 130 has the orphaned tail (/// type.) above the original Serra test. Please move the full Serra's Emissary doc comment back above compound_subject_keyword_static_splits_serras_emissary and leave only the CR 207.2c ability-word comment above the new test.

I don't have a behavior/code-path objection to the parser seam after this comment-layout fix.

@matthewevans matthewevans removed their assignment Jul 6, 2026
The ability-word regression test was inserted into the middle of the
existing Serra's Emissary CR 702.16 / CR 609.6 doc comment, stranding
its head above the new test and orphaning its tail above the original
test. Move the full Serra comment back above
`compound_subject_keyword_static_splits_serras_emissary` and leave only
the CR 207.2c ability-word comment above the new test. Comment-only; no
behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@minion1227

Copy link
Copy Markdown
Contributor Author

@matthewevans — comment-layout fix pushed in f34d9d4d6.

The Serra's Emissary CR 702.16 / CR 609.6 doc comment is now whole again above compound_subject_keyword_static_splits_serras_emissary, and only the CR 207.2c ability-word comment sits above ability_word_prefix_is_stripped_from_subject_anchored_statics. Comment-only move — no behavior change, cargo fmt --all --check clean.

@matthewevans matthewevans self-assigned this Jul 6, 2026

@matthewevans matthewevans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed current head after the follow-up. The prior regression-proof request is addressed with shape assertions for Chroma, Fateful hour, and Protector, and the Serra's Emissary CR doc comment is whole again above its original test. The implementation stays at the existing ability-word/static-parser seam, CR 207.2c is verified, and the source diff remains scoped to the parser/test files. Approving and enqueueing; the merge queue will wait for the in-progress checks on this head.

@matthewevans matthewevans added the bug Bug fix label Jul 6, 2026
@matthewevans matthewevans enabled auto-merge July 6, 2026 17:34
@matthewevans matthewevans removed their assignment Jul 6, 2026
@matthewevans matthewevans added this pull request to the merge queue Jul 6, 2026
Merged via the queue into phase-rs:main with commit d0eda04 Jul 6, 2026
11 checks passed
@minion1227 minion1227 deleted the minion_static_ability_word branch July 9, 2026 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bug fix needs-maintainer AI-contribution PR requires human triage (Non-dev track or unresolved gaps)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants