Skip to content

fix(parser): UNSUPPORTED seam: Once-per-turn {0} alternative cost gated on a dynamic mana-value thre#5755

Merged
matthewevans merged 3 commits into
phase-rs:mainfrom
ntindle:fix/who-misparse-112-unsupported-seam-once-per
Jul 13, 2026
Merged

fix(parser): UNSUPPORTED seam: Once-per-turn {0} alternative cost gated on a dynamic mana-value thre#5755
matthewevans merged 3 commits into
phase-rs:mainfrom
ntindle:fix/who-misparse-112-unsupported-seam-once-per

Conversation

@ntindle

@ntindle ntindle commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes a parser misparse affecting 1 card(s) in the Doctor Who Commander precons.

Root cause: UNSUPPORTED seam: Once-per-turn {0} alternative cost gated on a dynamic mana-value threshold

Cards corrected

  • As Foretold

Fix

Implemented Cluster 112 (As Foretold): the once-per-turn, dynamic-mana-value alternative-cost grant class, as a pure parameterization of the existing StaticMode::CastWithAlternativeCost seam (no new enum variant). Two class-level axes were added over the existing unlimited grants (Fist of Suns / Rooftop Storm / Jodah / Primal Prayers):

  1. A frequency: CastFrequency field on CastWithAlternativeCost (reusing the existing enum; default Unlimited, skip-serialized so existing cards' JSON is byte-identical).
  2. A dynamic mana-value gate resolved from game state via the existing FilterProp::Cmc { LE, QuantityRef::CountersOn { Source, Time } } machinery.

Runtime once-per-turn enforcement mirrors the established hand_cast_free_permissions_used lifecycle: a new GameState::alt_cost_grant_permissions_used HashSet is skipped-when-used in granted_spell_alternative_cost_for, consumed at finalize_cast, and cleared at turn start in turns.rs. Provenance flows via new Option<ObjectId> fields on SpellContext and PendingCast across the OptionalCostChoice round-trip. The parser gained a parse_alt_cost_frequency_prefix combinator (single authority shared by the classifier pre-filter and the lowering), a relaxed classifier admitting the "Once each turn," prefix + singular "spell you cast", a bare-article ("a"/"an") widening to any-spell, a strict-fail on unconsumed mana-value tails, and the building-block win: parse_mana_value_suffix now binds a trailing ", where X is " clause onto a bare-X gate so every consumer inherits dynamic-MV support.

VERIFICATION (worktree not under Tilt; cargo run directly): cargo fmt clean; targeted parser diff-gate vs HEAD clean (zero string dispatch in my parser diffs); cargo clippy -p engine -p phase-ai -p engine-wasm -- -D warnings (NO --all-targets) exit 0 — cross-crate exhaustiveness satisfied; cargo test -p engine -p phase-ai --no-fail-fast exit 0 (all lib + integration pass, including 5 new As Foretold tests: parser building-block, full-parse, runtime offer/accept/consume, runtime decline-not-consumed, runtime MV-gate). oracle-gen export confirms As Foretold now emits the OncePerTurn CastWithAlternativeCost static with Cmc LE CountersOn(Source,time) and ZERO Unimplemented, while Fist of Suns/Rooftop Storm/Jodah/Conspiracy Unraveler stay byte-identical (frequency unset = Unlimited).

JUDGEMENT CALLS: (1) Finalize consumption reads ability.context.alt_cost_grant_source BEFORE the placeholder branch, not from stack_ability — vanilla permanent spells carry stack_ability=None but their alt cost was still applied and must consume the slot (caught by the first integration run). (2) Split the MV-gate runtime check into its own test because a no-mana cast leaves a spell on the stack, blocking a later sorcery-speed cast. (3) Article handling via matches!(type_prefix_lower.trim(), "" | "a" | "an") — a tokenized equality check, not substring dispatch.

STOP-AND-RETURN: none — no CR ambiguity, seam fit cleanly.

DEVIATIONS FROM PLAN: plan cited CR 707.10c for the copy reset; correct rule is CR 707.10 ("a copy of a spell isn't cast") — used 707.10. Plan's finalize capture read stack_ability.context; corrected to ability.context pre-placeholder. Scope expansion to keep the workspace compiling after the type changes (see scopeExpansion).

RISKS: full-corpus gen-card-data.sh + cargo semantic-audit/coverage were NOT run (disk ceiling ~27Gi, worktree not the card-data source of truth). Changes are narrow and guarded and the full 16502-test lib suite passes, but recommend the orchestrator run a corpus semantic-audit under Tilt to confirm no other "...spell you cast with mana value N ..." card now strict-fails to honest Unimplemented (by-design, not a silent misparse). Known SHARED pre-existing limitation (not introduced): payable_spell_alternative_cost_details is Zone::Hand-gated, so As Foretold applies to hand-cast spells only — same as every existing grant; out of scope, note in PR body.

Files changed

  • crates/engine/src/types/statics.rs
  • crates/engine/src/types/game_state.rs
  • crates/engine/src/types/ability.rs
  • crates/engine/src/game/casting.rs
  • crates/engine/src/game/casting_costs.rs
  • crates/engine/src/game/turns.rs
  • crates/engine/src/game/effects/copy_spell.rs
  • crates/engine/src/parser/oracle_classifier.rs
  • crates/engine/src/parser/oracle_static/cost_mod.rs
  • crates/engine/src/parser/oracle_static/mod.rs
  • crates/engine/src/parser/oracle_target.rs
  • crates/engine/src/parser/oracle_tests.rs
  • crates/engine/src/parser/oracle_static/tests.rs
  • crates/engine/src/game/casting_tests.rs
  • crates/engine/src/game/engine_tests.rs
  • crates/engine/src/game/visibility.rs
  • crates/engine/tests/integration/rules/casting.rs
  • crates/mtgish-import/src/convert/player_effect.rs

CR references

  • CR 118.9 (docs/MagicCompRules.txt:1016 — alternative cost: 'You may [action] rather than pay [this object's] mana cost')
  • CR 118.9a (docs/MagicCompRules.txt:1018 — only one alternative cost applied per spell)
  • CR 601.2b (docs/MagicCompRules.txt:2459 — announce alternative/additional costs while casting)
  • CR 601.2f (docs/MagicCompRules.txt:2468 — total-cost determination)
  • CR 601.2a (docs/MagicCompRules.txt:2457 — spell moved to stack at announcement)
  • CR 202.3 (docs/MagicCompRules.txt:1359 — mana value)
  • CR 107.3a (docs/MagicCompRules.txt:466 — X variable binding)
  • CR 122.1 (docs/MagicCompRules.txt:1178 — counters)
  • CR 400.7 (docs/MagicCompRules.txt:1950 — zone change makes a new object, naturally resetting the per-source slot)
  • CR 707.10 (docs/MagicCompRules.txt:5666 — a copy of a spell isn't cast; corrects plan's 707.10c)
  • CR 701.59a (docs/MagicCompRules.txt:3799 — collect evidence N; on the pre-existing sibling constructor)

Verification

  • cargo fmt --all — pass
  • check-parser-combinators.sh <merge-base> — pass
  • cargo clippy -p engine -p phase-ai -p engine-wasm -- -D warnings — pass
  • cargo test -p engine -p phase-ai — pass
  • oracle-gen data --filter "as foretold" — pass
    Cards confirmed re-parsed correctly: As Foretold

🤖 Generated with Claude Code

@ntindle
ntindle requested a review from matthewevans as a code owner July 13, 2026 19:45

@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 implements support for once-per-turn alternative cost grants (such as As Foretold) by introducing a frequency axis to alternative cost definitions and tracking used permissions in the game state. It also adds a parser mechanism to rebind bare X mana-value gates from trailing 'where X is...' clauses to dynamic quantities like counters on the source. The implementation is highly idiomatic, adheres strictly to the MTG Comprehensive Rules (CR 118.9, CR 202.3, CR 601.2b), and uses composable building blocks with thorough test coverage. No review comments were provided to assess.

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.

@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown

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

1 card(s) · static/CastWithAlternativeCost(Mana { cost: Cost { shards: [], generic: 0 } }) · added: CastWithAlternativeCost(Mana { cost: Cost { shards: [], generic: 0 } }) (affects=mv time counters on self- you control card)

Examples: As Foretold

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

Examples: As Foretold

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

@matthewevans matthewevans self-assigned this Jul 13, 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.

Approved — the current ported head cleanly parameterizes the existing alternative-cost seam.

🔴 Blocker

  • None.

🟡 Non-blocking

  • None.

✅ Clean

  • crates/engine/src/game/casting.rs and casting_costs.rs carry the selected once-per-turn source through the existing cast choice and consume it only at finalized casts; decline and copied-spell paths leave the permission unspent.
  • The current parse artifact matches the stated scope exactly: As Foretold loses unknown and gains the zero-cost alternative-cost static with the source-counter mana-value filter.
  • Registered runtime coverage reaches the cast pipeline for above-threshold exclusion, acceptance, and decline; current Rust lint/tests, card-data, and platform checks are green.

Recommendation: enqueue via the merge queue.

@matthewevans matthewevans added enhancement New feature or request quality For high-quality minimal to no-churn PRs labels Jul 13, 2026
@matthewevans
matthewevans added this pull request to the merge queue Jul 13, 2026
@matthewevans matthewevans removed their assignment Jul 13, 2026
Merged via the queue into phase-rs:main with commit 7c63d29 Jul 13, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request quality For high-quality minimal to no-churn PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants