Skip to content

feat(parser): leading except-for exempt-list restriction static — Akron Legionnaire (CR 508.1c) - #5409

Merged
matthewevans merged 9 commits into
phase-rs:mainfrom
jsdevninja:fix/akron-legionnaire-except-for-restriction
Jul 9, 2026
Merged

feat(parser): leading except-for exempt-list restriction static — Akron Legionnaire (CR 508.1c)#5409
matthewevans merged 9 commits into
phase-rs:mainfrom
jsdevninja:fix/akron-legionnaire-except-for-restriction

Conversation

@jsdevninja

@jsdevninja jsdevninja commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds engine support for Akron Legionnaire's leading exempt-list restriction static — and the class of sentence-initial "Except for <A> and <B>, <rule-static>" exception clauses it represents.

Its printed static, Except for creatures named Akron Legionnaire and artifact creatures, creatures you control can't attack., silently dropped: parse_compound_subject_rule_static (crates/engine/src/parser/oracle_static/evasion.rs) hands the FULL prefix before the first recognized predicate to parse_rule_static_subject_filter, which has no "except for" arm, so the whole line strict-failed.

parse_leading_except_for_rule_static (dispatched immediately before parse_compound_subject_rule_static) splits the exempt list into two conjuncts on " and ", resolves each independently — a bare type-phrase exemption ("artifact creatures") or a named exemption within a type class ("creatures named Akron Legionnaire", via FilterProp::Named) — Or-combines them, then ANDs Not{Or{..}} onto the affected filter the existing single-subject dispatcher produces for the remainder. No new runtime: TargetFilter::And/Not/Or and FilterProp::Named are all existing, already-evaluated primitives.

Files changed

  • crates/engine/src/parser/oracle_static/evasion.rs
  • crates/engine/src/parser/oracle_static/shared.rs
  • crates/engine/src/parser/oracle_static/mod.rs
  • crates/engine/src/parser/oracle_static/tests.rs

Anchored on

  • crates/engine/src/parser/oracle_static/evasion.rs parse_compound_subject_rule_static — the single-subject sibling; the new handler is dispatched immediately before it and delegates the remainder to it unchanged.
  • #5219 (compound-subject animation statics, Life and Limb) — the same "resolve conjuncts independently, recombine generically" shape, applied to a different static category (restriction, not type-change).
  • #5384 (N-way Oxford-comma compound-subject keyword static, Shalai) — the precedent for evidence-scoping arity: build exactly the conjunct-count the printed card uses, not a speculative unbounded grammar.

CR references

  • CR 508.1c — the active player checks each creature for restrictions ("effects that say a creature can't attack") when declaring attackers.
  • CR 201.2a — two or more objects have the same name if they share at least one name; the basis for the "creatures named <Name>" exemption conjunct.

Scope

Scoped to the 2-conjunct "<A> and <B>" form: a Scryfall full-text search (o:/^Except for/) returns exactly one printed card in this shape (Akron Legionnaire), so an unbounded Oxford-comma exempt list would be speculative — there's no card to validate a 3+-conjunct split against the genuine ambiguity of which comma ends the exempt list vs. starts the next conjunct.

Verification

  • cargo fmt --all — clean.
  • Full cargo check/test/clippy could not be run in this session — local machine's MSVC C++ Build Tools installation is incomplete (vswhere finds no installation with the VC.Tools component), so link.exe resolves to Git's coreutils shim instead of the real MSVC linker, and every crate (including unrelated third-party dependencies) fails to link before reaching this crate's code. Tilt was also not running in this session. Verified by close manual trace instead: every referenced type, function signature, and visibility (TargetFilter::{And,Not,Or,Typed,Any}, FilterProp::Named, TypedFilter, StaticDefinition.{affected,description}, nom_primitives::split_once_on, parse_type_phrase, merge_filter_prop [bumped fnpub(crate) fn], parse_compound_subject_rule_static) was confirmed against the actual current source, and the dispatch chain (parse_static_line_multiparse_static_line_multi_irparse_static_line_multi_innerparse_static_line_multi_dispatch) was traced by hand to confirm the new arm is reachable and ordered correctly relative to its sibling. Given the local toolchain is broken independent of this change, deferring final compiler/test confirmation to CI.
  • New tests (oracle_static/tests.rs): a positive parse test on Akron Legionnaire's real Oracle text asserting the full And{Typed(creatures you control), Not{Or{Typed(creatures named "akron legionnaire"), Typed(artifact creatures)}}} shape; a guard confirming the untouched sibling ("Creatures you control can't attack.", no exempt clause) still resolves unwrapped; a guard confirming a single-conjunct exempt clause (no " and ") correctly declines rather than mis-parsing.

Track

Community

LLM

Model: claude-sonnet-5

Scope Expansion

None.

Validation Failures

None — see Verification note on the local toolchain limitation.

CI Failures

Unknown at PR open time — local compiler verification was unavailable (see Verification).

Closes #5408

…n Legionnaire)

Akron Legionnaire's "Except for creatures named Akron Legionnaire and
artifact creatures, creatures you control can't attack." strict-failed:
parse_compound_subject_rule_static hands the FULL prefix before the first
recognized predicate to parse_rule_static_subject_filter, which has no
"except for" arm, so the sentence-initial exception clause was unparseable.

Adds parse_leading_except_for_rule_static, dispatched immediately before
its single-subject sibling. Splits the exempt list on " and " into two
conjuncts, resolves each independently as a bare type-phrase exemption
("artifact creatures") or a named exemption within a type class ("creatures
named Akron Legionnaire", via FilterProp::Named), Or-combines them, and ANDs
Not{Or{..}} onto the affected filter the existing dispatcher produces for
the remainder — the same "resolve conjuncts independently, recombine
generically" shape as phase-rs#5219's compound-subject animation dispatcher.

Scoped to the 2-conjunct form: a Scryfall full-text search (o:/^Except for/)
returns exactly one printed card in this shape, so an unbounded Oxford-comma
exempt list would be speculative with no card to validate the ambiguous
comma-boundary split against.

Fixes phase-rs#5408.
@jsdevninja
jsdevninja requested a review from matthewevans as a code owner July 9, 2026 02:34
@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 added the needs-maintainer AI-contribution PR requires human triage (Non-dev track or unresolved gaps) label Jul 9, 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.

[MED] Parser-only tests do not prove the new Akron Legionnaire static works through the production attack-declaration path. Evidence: crates/engine/src/parser/oracle_static/tests.rs:4243. Why it matters: the PR claims card support and builds a supported CantAttack static, but the added tests only assert AST shape; a regression in CantAttack/affected filter evaluation, FilterProp::Named, or validate_attackers would still pass. Suggested fix: add an integration/scenario test that parses Akron Legionnaire’s real Oracle text and declares attackers, proving a normal nonartifact creature you control is rejected while a creature named Akron Legionnaire and an artifact creature are legal attackers.

…er declaration

The parser-level test only asserted the parsed AST shape (affected filter
structure), which doesn't prove the static actually gates combat -- a
regression in CantAttack evaluation, FilterProp::Named matching, or
validate_attackers would still pass it.

Adds an integration test that attaches the REAL parsed static (via
parse_static_line_multi, the entry point that reaches
parse_leading_except_for_rule_static) to a battlefield Akron Legionnaire,
then declares attackers: a plain nonartifact creature you control is
rejected, while Akron Legionnaire itself (exempted by name) and an artifact
creature you control (exempted by type) are legal attackers.

Addresses review feedback on phase-rs#5409.
@jsdevninja

Copy link
Copy Markdown
Contributor Author

Addressed: the original tests only asserted the parsed AST shape (oracle_static::tests.rs), which doesn't prove the static gates real combat.

Added combat::tests::akron_legionnaire_exempts_named_and_artifact_creatures_from_attacking — attaches the actual parsed static (via parse_static_line_multi, on a battlefield Akron Legionnaire) and declares attackers: a plain nonartifact creature you control is rejected by validate_attackers/excluded from get_valid_attacker_ids, while Akron Legionnaire itself (exempted by name) and an artifact creature you control (exempted by type) remain legal attackers.

cargo fmt --all is clean. Full cargo check/test still couldn't run locally in this session (broken MSVC linker on this machine, unrelated to the change — see the PR description's Verification note); traced the new test's harness calls (create_creature, static_definitions.push, get_valid_attacker_ids, validate_attackers) against their real signatures in combat.rs by hand.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

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

1 card(s) · static/CantAttack · added: CantAttack (affects=you control creature + not named "~" creature or artifact creature)

Examples: Akron Legionnaire

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

Examples: Akron Legionnaire

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

@matthewevans matthewevans self-assigned this Jul 9, 2026
@matthewevans matthewevans added the enhancement New feature or request label Jul 9, 2026
@matthewevans matthewevans removed their assignment Jul 9, 2026
@matthewevans matthewevans self-assigned this Jul 9, 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 fixup: current head avoids the helper-name collision, preserves the existing subject-rule parser, and keeps the runtime Akron combat coverage; auto-merge can wait for restarted checks.

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

Current head has the runtime Akron combat coverage, parse-diff matches the claimed single-card change, and the parser/combat seam is clean.

@matthewevans
matthewevans added this pull request to the merge queue Jul 9, 2026
@matthewevans matthewevans removed their assignment Jul 9, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Jul 9, 2026
@matthewevans matthewevans self-assigned this Jul 9, 2026
…20260709

# Conflicts:
#	crates/engine/src/game/combat.rs
@matthewevans
matthewevans enabled auto-merge July 9, 2026 05:48
@matthewevans matthewevans removed their assignment Jul 9, 2026
@matthewevans
matthewevans added this pull request to the merge queue Jul 9, 2026
@matthewevans matthewevans self-assigned this Jul 9, 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 keeps the Akron parser/combat seam clean, has discriminating runtime coverage, and parse-diff matches the claimed single-card restriction change.

@matthewevans matthewevans removed their assignment Jul 9, 2026
Merged via the queue into phase-rs:main with commit 89695eb Jul 9, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request needs-maintainer AI-contribution PR requires human triage (Non-dev track or unresolved gaps)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(parser): leading except-for exempt-list restriction static — Akron Legionnaire (CR 508.1c)

2 participants