Skip to content

fix(parser): parse source-exile play durations#5116

Merged
matthewevans merged 2 commits into
phase-rs:mainfrom
ntindle:codex/root25-effect-duration
Jul 8, 2026
Merged

fix(parser): parse source-exile play durations#5116
matthewevans merged 2 commits into
phase-rs:mainfrom
ntindle:codex/root25-effect-duration

Conversation

@ntindle

@ntindle ntindle commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fix root #25 subcluster for source-linked impulse draw durations: cards that say "you may play that card/it until you exile another card with this [source]" were parsed as PlayFromExile { duration: UntilEndOfTurn }.

This adds a Duration::UntilSourceExilesAnotherCard grammar arm, expires matching source-linked durations when the same source exiles another card, and removes the fixed cards from docs/parser-misparse-backlog.md.

Fixed cards removed from backlog:

  • Furious Rise
  • Superior Foes of Spider-Man
  • Unstable Amulet

Root Cause

The impulse PlayFromExile parser used strip_trailing_duration, but the duration grammar had no representation for "until you exile another card with ~", so these permissions fell back to the impulse default UntilEndOfTurn.

Changed-Card Audit

Focused before/after export against origin/main for:
Furious Rise|Superior Foes of Spider-Man|Unstable Amulet

Raw JSON changed exactly these three cards:

  • furious rise
  • superior foes of spider-man
  • unstable amulet

For each card, the only parsed value change was:
PlayFromExile.duration: UntilEndOfTurn -> UntilSourceExilesAnotherCard

coverage-parse-diff reported no card-parse signature changes, which is expected here because the support surface did not add/remove abilities; the raw JSON audit above is the discriminating check for this duration-field fix.

Validation

  • cargo fmt --all
  • ./scripts/check-parser-combinators.sh
  • git diff --check
  • cargo test -p engine --features cli --lib play_from_exile_until_source_exiles_another_card_uses_source_exile_duration -- --nocapture
  • cargo test -p engine --features cli --lib source_exile_duration_expires_previous_permission_on_next_source_exile -- --nocapture
  • cargo test -p engine --features cli --lib duration_roundtrip -- --nocapture
  • cargo clippy -p engine --features cli --all-targets -- -D warnings
  • cargo run -p engine --features cli --bin oracle-gen -- data --filter "Furious Rise|Superior Foes of Spider-Man|Unstable Amulet" --output /tmp/phase-root25-after.json
  • cargo run -p engine --features cli --bin card-data-validate -- /tmp/phase-root25-after.json
  • cargo run -p engine --features cli --bin coverage-parse-diff -- /tmp/phase-root25-base.json /tmp/phase-root25-after.json --markdown /tmp/phase-root25-diff.md --json /tmp/phase-root25-diff.json

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@ntindle ntindle marked this pull request as ready for review July 5, 2026 02:11
@ntindle ntindle requested a review from matthewevans as a code owner July 5, 2026 02:11
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

Parse changes introduced by this PR

✓ No card-parse changes detected.

@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.

[MED] The new duration parser misses the with this ability half of the claimed card class. Evidence: crates/engine/src/parser/oracle_nom/duration.rs:72 only accepts tag("you exile another card with ~"), and the added regression test at crates/engine/src/parser/oracle_effect/tests.rs:34607 only covers the with ~ spelling. The local card reference data for this class includes Superior Foes of Spider-Man and Unstable Amulet as UntilAPlayerExilesACardWithThisAbility, while SELF_REF_TYPE_PHRASES/SELF_REF_PARSE_ONLY_PHRASES do not normalize this ability to ~ (crates/engine/src/parser/oracle_util.rs:866). Why it matters: the PR claims the Furious Rise / Superior Foes / Unstable Amulet class, but two of the named examples still won't reach Duration::UntilSourceExilesAnotherCard, so the fix is narrower than its coverage and tests imply. Suggested fix: compose the duration grammar over both source-reference forms (with ~ / with this ability) and add a regression test using the literal with this ability wording.

@matthewevans matthewevans added the bug Bug fix label Jul 6, 2026
# Conflicts:
#	crates/engine/src/parser/oracle_effect/tests.rs
@ntindle

ntindle commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Updated this branch with current origin/main (bf4c7bf) and addressed the requested source-reference coverage. The shared duration body now accepts with ~, with this ability, and the existing object-source spellings, and the invalidation scanner reuses that parser so the lists do not drift. Added regression coverage for literal with this ability; preserved the upstream same-source invalidation test while resolving the merge conflict. Also fixed one merge-fallout initializer by setting invalidation: None. Local checks: cargo fmt --all; WSL RUSTC_WRAPPER= CARGO_NET_OFFLINE=false cargo test -p engine --lib source_exiles_another_card (2 passed).

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

Maintainer review: source-exile invalidation is now threaded through the shared duration grammar, the PlayFromExile grant invalidation path, and same-source runtime expiration. The prior with this ability blocker is resolved.

@matthewevans matthewevans enabled auto-merge July 8, 2026 07:30
@matthewevans matthewevans added this pull request to the merge queue Jul 8, 2026
@matthewevans matthewevans removed their assignment Jul 8, 2026
Merged via the queue into phase-rs:main with commit de39a32 Jul 8, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants