Skip to content

feat(parser): N-way Oxford-comma compound-subject keyword static — Shalai, Voice of Plenty (CR 611.3a)#5384

Merged
matthewevans merged 3 commits into
phase-rs:mainfrom
jsdevninja:feat/shalai-oxford-compound-subject-keyword-static
Jul 8, 2026
Merged

feat(parser): N-way Oxford-comma compound-subject keyword static — Shalai, Voice of Plenty (CR 611.3a)#5384
matthewevans merged 3 commits into
phase-rs:mainfrom
jsdevninja:feat/shalai-oxford-compound-subject-keyword-static

Conversation

@jsdevninja

Copy link
Copy Markdown
Contributor

Summary

Fixes #5383.

parse_compound_subject_keyword_static (crates/engine/src/parser/oracle_static/evasion.rs) decomposes the compound player+object keyword-grant pattern "You and <object subject> have <keyword>" (Sigarda, Heron's Grace; Serra's Emissary; Gruul Spellbreaker) into an object-half Continuous/AddKeyword def and a player-half Hexproof/Shroud/PlayerProtection def, since a single StaticDefinition can't carry both a player scope and an object scope.

Its subject match was hard-coded to the literal seam "you and " — exactly one object conjunct. Shalai, Voice of Plenty's real Oracle text uses a 3-item Oxford-comma list instead:

Flying
You, planeswalkers you control, and other creatures you control have hexproof.
{4}{G}{G}: Put a +1/+1 counter on each creature you control.

(verified via Scryfall, oracle_id: a0c47ab6-dfb4-46ee-a3f7-9e1521b4bb4b)

Because the line starts with "You, " rather than "You and ", the tag match failed immediately and the line fell through to the generic fallback, which couldn't resolve it either — client/public/card-data.json recorded it as Unimplemented("static_structure", "Static pattern matched but line failed static parser: …"). Shalai's entire signature ability was silently dropped.

Why this is a class, not a card

The object-only N-way Oxford-comma case already works (parse_type_phrase + distribute_controller_to_or, proven by the Silkguard test and Kismet), but only when a single trailing suffix distributes backward across bare type-noun legs with no clause of their own. Shalai's shape is different: "you" is the first list item, and each subsequent conjunct ("planeswalkers you control", "other creatures you control") is a complete, independently-resolvable subject phrase — exactly what parse_rule_static_subject_filter already resolves one at a time inside the existing 2-item function. Nothing peeled the leading "you" token off an arbitrary-length Oxford list and Or-combined the resolved remainder.

This is a straight generalization of the function's "object subject" axis from exactly-one-conjunct to N-conjuncts, following this repo's "parameterize, don't proliferate" convention rather than hand-rolling a Shalai-specific branch.

Changes

  • evasion.rs: parse_compound_subject_keyword_static now accepts "you, " (Oxford-comma N-item form) as an alternate lead to the existing "you and " (bare 2-item form). The comma gate keeps the two grammars disjoint — the 2-item path's existing fallthrough to parse_rule_static_subject_filter (which itself resolves an object subject with an internal bare "and", e.g. "artifacts and creatures you control", via parse_type_phrase's own trailing-suffix distribution) is completely unchanged.
  • New parse_oxford_object_conjuncts helper: nom-based (split_once_on), peels ", "-separated conjuncts plus the final ", and "/"and " connector, resolves each conjunct through the existing parse_rule_static_subject_filter, and Or-combines them. Applies the same "no second player scope" guard the 2-item form already applies, now per conjunct.
  • Both existing call sites of parse_compound_subject_keyword_static (the direct dispatch and the inverted "As long as <cond>, …" rewrite path) pick up the generalization automatically since it's one function, not a new sibling dispatcher.
  • shared.rs: updated the dispatch-site comment to document the new N-item form.
  • tests.rs: flips no existing test (none previously exercised this exact shape); adds:
    • compound_subject_keyword_static_splits_shalai_three_way — the real card, asserting the 2-way object Or (planeswalkers you control; other creatures you control) and the player-half Hexproof mode.
    • compound_subject_keyword_static_declines_oxford_list_unresolvable_conjunct — boundary guard: an unresolvable conjunct must strict-fail the whole line, not silently drop the bad disjunct.
    • compound_subject_keyword_static_splits_four_item_oxford_list — arity generality check (synthetic 4-item list → 3-way object Or), proving the splitter isn't hard-coded to Shalai's 3-item arity.

CR references

  • CR 611.3a — a continuous effect generated by a static ability applies to whatever its text indicates.
  • CR 702.11a/CR 702.11b — hexproof.
  • CR 702.16 — protection.
  • CR 702.18 — shroud.

Verification

  • cargo fmt -p engine -- --check — clean, no diff.
  • Local cargo check/cargo test could not be run to completion in the environment this PR was authored in — the sandbox has no working MSVC linker (link.exe on PATH resolves to Git for Windows' coreutils link, not the MSVC linker) and no Docker/WSL fallback, so native build-script compilation fails for unrelated foundational crates (serde, proc-macro2, etc.) before reaching this crate's code. I verified the change through careful manual review (type/signature cross-checks against every function called, hand-traced the split logic against the real Shalai text and three synthetic inputs) and via cargo fmt's parse-clean check, but please treat CI as the first real compile/test signal for this PR and let me know if anything surfaces.

Scope Expansion

None.

…se-rs#5383)

Shalai, Voice of Plenty's "You, planeswalkers you control, and other
creatures you control have hexproof." uses a 3-item Oxford-comma list
instead of the bare 2-item "you and <object>" form that
parse_compound_subject_keyword_static hard-codes, so the line fell
through to the generic fallback and strict-failed.

Generalizes the existing 2-item splitter to also accept the
comma-led "you, ..." form, delegating the object-list split to a new
parse_oxford_object_conjuncts helper that resolves each conjunct
independently through the same parse_rule_static_subject_filter the
2-item form already uses, then Or-combines them. The comma gate keeps
the literal "you and " path (and its existing internal-"and" handling
via parse_type_phrase's trailing-suffix distribution) completely
unchanged.

Fixes phase-rs#5383.
@jsdevninja jsdevninja requested a review from matthewevans as a code owner July 8, 2026 18:51

@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 adds support for parsing the Oxford-comma N-item form of compound-subject keyword-grant statics (such as on Shalai, Voice of Plenty) by introducing the parse_oxford_object_conjuncts helper. This allows resolving multiple independent object subjects into an Or filter alongside the player-half keyword grant. Symmetrical unit tests have been added to verify both successful multi-item parsing and strict-fail behavior on unresolvable conjuncts. No review comments were provided, and the changes are architecturally sound and compliant with the repository style guide.

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.

The parser-combinator gate flagged the raw .strip_prefix("and ") call
in parse_oxford_object_conjuncts as parsing dispatch on a string
literal. Rewritten per PATTERNS.md Pattern 1 (optional fixed prefix)
using opt(tag("and ")), matching the existing idiom already used
elsewhere in this file (parse_source_power_block_restriction).
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

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

1 card(s) · static/Continuous · added: Continuous (affects=you control planeswalker or another you control creature, mods=grant Hexproof)

Examples: Shalai, Voice of Plenty

1 card(s) · static/Hexproof · added: Hexproof (affects=you)

Examples: Shalai, Voice of Plenty

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

Examples: Shalai, Voice of Plenty

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

@matthewevans matthewevans self-assigned this Jul 8, 2026
@matthewevans matthewevans added the enhancement New feature or request label 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: current head cleanly handles the Oxford-comma compound-subject static class; parse-diff limited to Shalai.

@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 6a31607 Jul 8, 2026
13 checks passed
Whovencroft pushed a commit to Whovencroft/phase that referenced this pull request Jul 9, 2026
… splitting (issue phase-rs#5412) (phase-rs#5414)

* feat(parser): compound-subject "don't untap" restriction (CR 502.3) — Exhaustion, Icebreaker Kraken

"Creatures and lands target opponent controls don't untap during their next
untap step." (Exhaustion) and "...artifacts and creatures target opponent
controls don't untap during that player's next untap step." (Icebreaker
Kraken) both strict-failed: the single-subject sibling predicate is fully
supported (Misstep, Mana Vapors both parse correctly today), but no dispatcher
resolved a compound "<X> and <Y> target player|opponent controls" subject
before reaching the generic split, which claims the whole subject in one
parse_subject_application call.

try_parse_compound_target_controller_cant_untap (dispatched before the
generic " can't "/" cannot "/" doesn't untap"/" don't untap" split) splits the
subject into per-conjunct type words sharing the trailing "target
player/opponent controls" suffix, resolves each through the SAME
parse_subject_application call shape already proven correct for the
single-subject siblings, and Or-combines them — mirroring the compound-subject
dispatcher family established by parse_compound_all_subjects_type_change
(phase-rs#5219) and its siblings. parse_compound_target_controller_filter handles both
the bare "X and Y" form (both known member cards) and an Oxford-comma N-way
list (mirroring parse_oxford_object_conjuncts, phase-rs#5384/Shalai) for a future 3-way
member of this class, per CLAUDE.md's build-for-the-class discipline.

Predicate resolution (build_restriction_clause, CantUntap +
Duration::UntilNextStepOf) is untouched and reused as-is — this is purely a
subject-side fix.

Fixes phase-rs#5412

CR 502.3 (untap-step restriction) + CR 611.3 (Or-distributed affected set) +
CR 109.4 / CR 115.1 (target-player/target-opponent controller references), all
grep-verified against docs/MagicCompRules.txt.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(parser): avoid clippy doc_lazy_continuation in diagnostic test comment

A doc-comment line started with "+ ", which rustdoc's markdown parser reads
as an unordered-list marker; the following wrapped lines then triggered
clippy::doc_lazy_continuation as unindented list continuations. Reword to
avoid a line-leading "+" instead of suppressing the lint.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(parser): revert redundant CantUntap dispatcher, keep regression test only

The compound-subject "don't/doesn't untap" case (Exhaustion, Icebreaker
Kraken) turned out to already be handled by the existing generic restriction
split in try_parse_subject_restriction_clause, via the general
compound-core-type-Or splitter in parse_type_phrase_with_ctx
(TYPE_SEPARATORS + distribute_controller_to_or). Verified against this PR's
true parse-diff baseline (base-sha fced6f1): both cards are already
supported: true, gap_count: 0 there — the checked-in data/coverage-data.json
I used for initial verification was stale relative to actual parser
capability on that same commit, which is what made the gap look real.

Reverts the dedicated try_parse_compound_target_controller_cant_untap
dispatcher and parse_compound_target_controller_filter added in 94e3f58
(subject.rs is now byte-identical to main). Keeps
compound_creatures_and_lands_target_opponent_controls in oracle_target.rs as
a standing building-block regression guard for the general splitter's
handling of the "target player/opponent controls" suffix family under a
compound subject — reworded from a diagnostic to a regression test now that
the general path is confirmed to already be correct.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(parser): N-way Oxford-comma compound-subject keyword static — Shalai, Voice of Plenty (CR 611.3a)

2 participants