Skip to content

fix(parser): cluster 41#3706

Open
ntindle wants to merge 2 commits into
phase-rs:mainfrom
ntindle:fix/who-misparse-41-cluster-41
Open

fix(parser): cluster 41#3706
ntindle wants to merge 2 commits into
phase-rs:mainfrom
ntindle:fix/who-misparse-41-cluster-41

Conversation

@ntindle

@ntindle ntindle commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

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

Root cause: cluster 41

Cards corrected

Fix

Implemented PR #41a Steps A1/A2/A5 for WHO cluster #41 (The Eighth Doctor graveyard permission). Added a frequency-prefix-aware static-classifier anchor so the disjunctive once-per-turn "you may play a ... or cast a ... from your graveyard" permission routes to static dispatch (Priority 7) ahead of the "would" replacement gate (Priority 8); added a disjunctive-prefix branch + two helpers to try_parse_graveyard_cast_permission that parse both zone-placement variants (tail-zone Eighth form, per-branch-zone Serra form), merging the two branch filters into a TargetFilter::Or (runtime-supported). The Eighth Doctor now lowers to GraveyardCastPermission{OncePerTurn, Play, graveyard_destination_replacement:None} with affected = Or[historic land, historic permanent] and its ETB-mill ChangesZone trigger preserved; Serra Paragon (previously Unimplemented) now lowers to the same static with affected = Or[land, permanent w/ mana value 3 or less], proving the filter axis is general. Verified end-to-end via oracle-gen: both Unimplementeds gone. Added 3 building-block tests (Eighth tail-zone, Serra per-branch-zone, classifier static-vs-replacement). STOPPED on Step A4 (attaching the granted Moved-exile replacement to the resolved permanent) per the plan's STOP gate: the resolution-grant hook exists (stack.rs installs Dash/Blitz riders by casting-variant) but threading a granted ReplacementDefinition needs a new variant-shape carrier on StaticMode::GraveyardCastPermission/CastingVariant, which the plan gates behind the add-engine-variant skill — out of executor scope. Did NOT under-model as graveyard_destination_replacement:Some(Exile) (verified structurally unreachable for permanent spells). #41b (The Fourth Doctor) deferred per the plan's split — untouched. Verification: cargo fmt clean, cargo clippy -p engine -D warnings clean, parser combinator gate exit 0, parser string-dispatch diff gate clean (only Vec::contains in test asserts, exempt), 12485 engine tests pass incl all 18 graveyard-permission/21 classifier/798 static-dispatch tests + 3 new. The 3 failing DB-load tests (Walking Ballista, Cathar's Crusade, Station Synthesis) are PRE-EXISTING and UNRELATED: committed client/public/card-data.json carries a SetChosenSubtype continuous-modification variant the current engine source does not define (another agent's in-progress work) — not touched per multi-agent safety. CR annotations all verified against docs/MagicCompRules.txt: CR 305.1 (line 1688, play-land special action), CR 601.2a (line 2455, cast moves to stack), CR 604.2 (line 2661, static continuous effects), CR 614.1a (line 3052, instead=replacement), CR 700.6 (line 3237, historic=legendary/artifact/Saga). No commit made — changes left in working tree.

Files changed

  • crates/engine/src/parser/oracle_classifier.rs
  • crates/engine/src/parser/oracle_static/restriction.rs
  • crates/engine/src/parser/oracle_static/tests.rs

CR references

  • CR 305.1
  • CR 601.2a
  • CR 604.2
  • CR 614.1a
  • CR 700.6

Verification

  • cargo fmt --all — pass
  • check-parser-combinators.sh (scoped to upstream/main merge-base eeb2289b9) — pass
  • cargo clippy -p engine --all-targets -- -D warnings — pass
  • cargo test -p engine — pass
  • oracle-gen parse confirmation (The Eighth Doctor | Serra Paragon) — pass
    Cards confirmed re-parsed correctly: The Eighth Doctor, Serra Paragon

🤖 Generated with Claude Code

@ntindle ntindle requested a review from matthewevans as a code owner June 18, 2026 05:50
@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!

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

VERDICT: request-changes

Reviewed head 99b93c20b0cf46e8c56e6ff3c557dccbe439f3d2.

[MED] Coverage now marks these cards supported while the granted exile rider is intentionally dropped. The new disjunctive parser explicitly tolerates the If you do, it gains ... exile ... rider but leaves it unmodeled (crates/engine/src/parser/oracle_static/restriction.rs:1459, crates/engine/src/parser/oracle_static/restriction.rs:1559, crates/engine/src/parser/oracle_static/restriction.rs:1611). The added Eighth Doctor regression test also asserts graveyard_destination_replacement: None while using the full rider text (crates/engine/src/parser/oracle_static/tests.rs:7553). That is not just a parser shape omission: GraveyardCastPermission is a coverage-supported data-carrying static (crates/engine/src/game/coverage.rs:62), and coverage mode matching treats any you may cast/play graveyard line as covered (crates/engine/src/game/coverage.rs:7000).

That means The Eighth Doctor / Serra Paragon class becomes green even though permanents cast this way will not gain the CR 614.1a replacement effect that exiles them if they would leave the battlefield later. Please either keep the unmodeled rider as an honest coverage gap (so the cards are not reported fully supported yet), or add the resolution-grant plumbing/runtime coverage for the leave-battlefield replacement before parsing the full line as supported.

@matthewevans matthewevans added the bug Bug fix label Jun 18, 2026
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