Skip to content

[codex] fix(parser): preserve named Kookus condition - #5120

Merged
matthewevans merged 1 commit into
phase-rs:mainfrom
ntindle:codex/root30-literal-names
Jul 5, 2026
Merged

[codex] fix(parser): preserve named Kookus condition#5120
matthewevans merged 1 commit into
phase-rs:mainfrom
ntindle:codex/root30-literal-names

Conversation

@ntindle

@ntindle ntindle commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes the Root #30 Kookus subcluster in docs/parser-misparse-backlog.md.

Root cause: normalize_card_name_refs only masked card(s) named ... literal spans, so creature named Keeper of Kookus was allowed to self-reference-normalize the embedded source name to Keeper of ~. Then the named-filter terminator did not recognize the post-normalization , ~ ... clause boundary, so the trigger condition swallowed the trigger body into the Named filter.

Changes:

  • Extend the existing named-literal mask to object/spell/permanent forms such as creature named, permanent named, spell named, etc.
  • Treat , ~ ... as the same referential named-filter boundary class as , it ....
  • Add building-block tests and an end-to-end Kookus trigger regression.
  • Remove Kookus from Root chore: update coverage stats and badges #30, reducing that bucket from 12 to 11 cards.

Parsed-card diff audit

Baseline: /tmp/phase-root30-baseline.json
After: /tmp/phase-root30-after.json
Filter: Dragonstorm Forecaster|Hecatomb|High Marshal Arguel|Kookus|Liu Bei, Lord of Shu|Sift Through Sands|Thran Golem|Thrasta, Tempest's Roar|Wrathful Raptors|Wrathful Red Dragon|Zenos yae Galvus|capital offense

Raw changed card keys: exactly kookus.

Expected Kookus movement:

  • Named filter changes from keeper of ~, ~ deals 3 damage to you and attacks this turn if able to keeper of kookus.
  • The swallowed trigger body now parses as DealDamage { amount: 3, target: Controller } followed by the until-end-of-turn MustAttack continuation.

coverage-parse-diff reported no support-signature clusters (oracle_changed: 0).

Validation

  • cargo fmt --all
  • ./scripts/check-parser-combinators.sh
  • git diff --check
  • cargo test -p engine --features cli --lib normalize_named_object_literal_preserves_embedded_source_name -- --nocapture
  • cargo test -p engine --features cli --lib named_filter_terminates_at_clause_boundary -- --nocapture
  • cargo test -p engine --features cli --lib parse_kookus_named_creature_condition_preserves_literal_name_and_body -- --nocapture
  • cargo run -p engine --features cli --bin oracle-gen -- data --filter "Dragonstorm Forecaster|Hecatomb|High Marshal Arguel|Kookus|Liu Bei, Lord of Shu|Sift Through Sands|Thran Golem|Thrasta, Tempest's Roar|Wrathful Raptors|Wrathful Red Dragon|Zenos yae Galvus|capital offense" --output /tmp/phase-root30-after.json
  • cargo run -p engine --features cli --bin card-data-validate -- /tmp/phase-root30-after.json
  • cargo run -p engine --features cli --bin coverage-parse-diff -- /tmp/phase-root30-baseline.json /tmp/phase-root30-after.json --markdown /tmp/phase-root30-parse-diff.md --json /tmp/phase-root30-parse-diff.json

Tilt is not running locally (No tilt apiserver found: tilt-default), so direct cargo fallback was used.

@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 03:40
@ntindle
ntindle requested a review from matthewevans as a code owner July 5, 2026 03:40
@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 · 4 card(s), 8 signature(s) (baseline: main c4def2e89060)

1 card(s) · ability/FlipCoin · added: FlipCoin (kind=activated, lose=yes, win=yes)

Examples: Wirefly Hive

1 card(s) · ability/FlipCoin · removed: FlipCoin (kind=activated, lose=yes, win=yes)

Examples: Wirefly Hive

1 card(s) · trigger/Phase · added: Phase (active in=battlefield, condition=# of named "keeper of ~" in battlefield you control creature = 0, constraint=only during your turn, phase=upkeep)

Examples: Kookus

1 card(s) · trigger/Phase · removed: Phase (active in=battlefield, condition=# of named "keeper of ~, ~ deals 3 damage to you and attacks this turn if able" in battlefield you control creature = 0…

Examples: Kookus

1 card(s) · ability/SearchLibrary · added: SearchLibrary (find=named "spirit of the night", kind=activated)

Examples: Urborg Panther

1 card(s) · ability/SearchLibrary · removed: SearchLibrary (find=named "spirit of the night", kind=activated)

Examples: Urborg Panther

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

Examples: Galactus, Devourer of Worlds

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

Examples: Galactus, Devourer of Worlds

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

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

The change is at the existing literal-name normalization and named-filter boundary seams, not a Kookus-only parser special case. The parse-diff shows the intended Kookus condition/body repair, with the other listed add/remove entries identical, and the added tests cover both the normalization boundary and the parsed trigger body. No blocking findings.

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