Skip to content

fix(parser): stop named filters before origin zones - #5101

Merged
matthewevans merged 2 commits into
phase-rs:mainfrom
ntindle:codex/root30-named-origin-zone
Jul 4, 2026
Merged

fix(parser): stop named filters before origin zones#5101
matthewevans merged 2 commits into
phase-rs:mainfrom
ntindle:codex/root30-named-origin-zone

Conversation

@ntindle

@ntindle ntindle commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Stop named <card> target filters before a following from <zone> origin suffix.
  • Reuse the existing zone-suffix parser so the terminator only fires for real origin zones, preserving card names like Extract from Darkness.
  • Remove Deathpact Angel from root chore: update coverage stats and badges #30 in docs/parser-misparse-backlog.md.

Card-data audit

I audited the all-face MTGJSON candidate class matching named .* from and generated filtered base/head exports for that set. The filter exported 23 cards including substring false positives such as Trivia Contest; raw card-data.json comparison changed exactly these 8 cards:

  • calim, djinn emperor
  • deathpact angel
  • demigod of revenge
  • groffskithur
  • myr servitor
  • ozox, the clattering king
  • pass the torch
  • rekindling phoenix

All 8 changes are the intended same-class correction: FilterProp::Named drops the origin-zone phrase (from your/their graveyard) while the zone-change effect still carries the origin zone. Trivia Contest was included in the candidate export and did not change.

Backlog note: only Deathpact Angel is removed from root #30. Demigod of Revenge and Groffskithur appear elsewhere in the backlog under different root causes, so this PR leaves those entries in place.

Local coverage-parse-diff reported oracle_changed: 0; the raw filtered card-data.json diff above was manually inspected.

Verification

  • cargo fmt --all
  • ./scripts/check-parser-combinators.sh
  • git diff --check
  • cargo test -p engine --features cli --lib named_filter_terminates_at_clause_boundary
  • cargo clippy -p engine --all-targets -- -D warnings
  • Filtered base/head oracle-gen exports for all-face named .* from candidates
  • Raw filtered card-data.json changed-key comparison
  • cargo run -p engine --features cli --bin card-data-validate -- /tmp/phase-root30-class-head.json (OK: 23 cards parsed)
  • cargo run -p engine --features cli --bin coverage-parse-diff -- ... (oracle_changed: 0)

@ntindle
ntindle requested a review from matthewevans as a code owner July 4, 2026 21:02

@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 resolves a parser issue where card names containing the word 'from' (such as 'Extract from Darkness') were incorrectly split, or where origin-zone suffixes (such as 'from your graveyard') were not properly parsed as terminators. It introduces the parse_named_filter_origin_zone_terminator helper using nom combinators to correctly identify origin-zone suffixes, updates the named filter terminator parser, and adds corresponding unit tests. Additionally, the parser misparse backlog documentation is updated to reflect the fix. No review comments were provided, and we have no further feedback as the implementation is highly idiomatic and conforms to the repository's architectural rules.

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.

…igin-zone

# Conflicts:
#	docs/parser-misparse-backlog.md
@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown

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

1 card(s) · ability/Bounce · field target: named "~ from your graveyard" cardnamed "~" card

Examples: Groffskithur

1 card(s) · ability/ChangeZone · field target: another named "~ from your graveyard" cardanother named "~" card

Examples: Calim, Djinn Emperor

1 card(s) · ability/ChangeZoneAll · field target: named "~ from their graveyard" cardnamed "~" card

Examples: Myr Servitor

1 card(s) · ability/ChangeZoneAll · field target: named "~ from your graveyard" cardnamed "~" card

Examples: Demigod of Revenge

@matthewevans matthewevans self-assigned this Jul 4, 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: the current head matches the parse-diff boundary, stays within the existing parser seams, and has green required checks. Enqueuing via merge queue.

@matthewevans
matthewevans added this pull request to the merge queue Jul 4, 2026
@matthewevans matthewevans removed their assignment Jul 4, 2026
@matthewevans matthewevans added the bug Bug fix label Jul 4, 2026
Merged via the queue into phase-rs:main with commit 8ac1d4c Jul 4, 2026
11 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