fix(parser): port the phrasing-gap restriction families into the shared condition grammar (P02-U3b) - #5723
Merged
Merged
Conversation
…ed condition grammar (P02-U3b)
The restriction-only fallback held four parser families that the shared
static-condition grammar could already have expressed — it just did not SPELL
their phrasings. Teach `parse_inner_condition` the five surfaces and delete the
fallback parsers; the shared grammar runs first, so each phrase now converts
through the ordinary `QuantityComparison` vocabulary that every static ability
with the same words already uses.
Ported (population 35,396 faces; instrument: full-pool dual export, whole-face
structural diff over ALL channels):
- CR 508.1a "you attacked with [N or more creatures | a <type>][ this turn]"
-> AttackedThisTurn { scope, filter }. The trailing " this turn" is OPTIONAL:
a full-pool reachability probe showed Thaumaton Torpedo reaching the fallback
WITHOUT it (an upstream duration parser peels it), so requiring the suffix
would have silently dropped that face.
- CR 201.2 + CR 109.3 "you control N or more lands with the same name"
-> ObjectCountBySharedQuality { quality: Name, aggregate: Max }. The
same-quality mirror of the existing `with different names` arm; `Max` is what
makes it "some ONE name is shared by N lands" and not "you control N lands".
- CR 102.2 + CR 608.2h "an opponent had [N] <type> enter the battlefield under
their control this turn" -> BattlefieldEntriesThisTurn { player: Opponent{Max} }.
- CR 701.23a "an opponent [has] searched their library this turn"
-> PlayerActionsThisTurn { player: Opponent{Max}, action: SearchedLibrary }.
- CR 402.1 + CR 608.2c "you have exactly N or M cards in hand" -> Or over
HandSize EQ leaves. "zero" needs its own tag: `parse_number`'s English table
starts at "one".
MULTIPLAYER SEMANTIC FIX (Whiplash Trap, Lavaball Trap). The fallback encoded the
opponent INSIDE the TargetFilter (`controller: Opponent`), so the runtime SUMMED
battlefield entries across ALL opponents. Two DIFFERENT opponents with one creature
each therefore satisfied "an opponent had TWO OR MORE creatures enter" — but no
single opponent had two. "An opponent" binds one player and "their control" binds
the count to that same player, so the threshold is per-opponent: `Opponent { Max }`.
The two readings coincide at two players and only diverge at three or more, which is
why the runtime witness needs three seats.
Retained, with honest justifications — the (c) phrasing-gap class is now EMPTY and
`PINNED_RETAINED_FAMILIES` drops 6 -> 4. Everything left is (a) a restriction-context
referent or (b) a real vocabulary gap:
- parse_spell_targets_filter: CR 601.3d in-flight-spell referent. Permanent.
- parse_source_condition: ParsedCondition has no filter-carrying source predicate,
so StaticCondition::SourceMatchesFilter cannot be converted. Teaching the shared
grammar a source phrasing today would make the shared parse SUCCEED and the
conversion then REJECT it — a NET LOSS of restriction support.
- parse_event_condition: `BeenAttackedThisStep` has no per-STEP counterpart at all
(StaticCondition's attack history is per-TURN), plus the ETB surface that elides
"the battlefield".
- parse_hand_condition: "you have no <kind> cards in hand" is an OWNER-relative count
in a HIDDEN zone. `ZoneCoreTypeCardCountAtLeast` reads player_zone_ids(player, Hand);
the shared ObjectCount path discriminates by CONTROLLER and TargetFilter has NO owner
axis. Converting it would not re-spell the phrase, it would ask a different question.
Proven-dead code deleted, not ported: the shared `parse_hand_size_predicate` already
spelled "no cards in hand", "one or fewer", "exactly N", and "more cards in hand than
each opponent". A full-pool reachability probe (nonvacuous — it registered hits on
every leaf kept) showed ZERO faces reaching those leaves across all 35,396.
Evidence: red-first — the six new `shared_grammar_owns_*` tests were watched FAILING on
a pristine worktree at this commit's base, each with "NoMatch (fell to restriction-only
grammar)". The whole-face full-pool diff caught a regression this change first
introduced (The Biblioplex, supported -> Unimplemented, because "zero" did not parse)
and it is fixed here.
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
matthewevans
enabled auto-merge
July 13, 2026 05:48
parse_numeric_threshold + parse_count_word lost their last caller when the retired restriction fallbacks were deleted (their zero-handling now lives in parse_hand_size_count in oracle_nom/condition.rs); PlayerScope import likewise orphaned. cargo test builds keep dead pub-in-crate fns alive under cfg(test); the lib-only CI build with -D warnings is what surfaces them.
Parse changes introduced by this PR✓ No card-parse changes detected. |
…Comparison shapes The U3b port routes these families through parse_inner_condition, which emits typed QuantityComparison conditions (per-opponent Max aggregation — the multiplayer-correct CR 102.2 semantics) instead of the bespoke OpponentSearchedLibraryThisTurn / BattlefieldEntriesThisTurn / YouAttackedWithAtLeast variants these pins asserted. The sibling Ravenous Trap pin was already updated; these three were missed. New shapes transcribed from the CI red run's actual parser output.
Round-2 fallout of the same class as dcda963 — CI shards surface one subset per round, so this round sweeps the FULL census of old-variant references instead of only the reported failures: - Thaumaton pins (oracle_tests, casting_tests): YouAttackedWithAtLeast -> QuantityComparison over filtered AttackedThisTurn - Lavaball pin (oracle_casting): GE 2 per-opponent Max — the intentional semantic-fix face (one opponent with 2+ lands, never two with 1 each) - Two restrictions parse_and_evaluate tests were harness gaps: they hand-populated only the legacy ledgers; production writes BOTH (search_library emits PlayerPerformedAction; declare_attackers records declaration snapshots), so the tests now simulate both halves like their sibling zero_attacker_declaration test already documents - integration comment refresh (shape-insensitive assert, no behavior change)
matthewevans
added a commit
to jaytbarimbao-collab/phase
that referenced
this pull request
Jul 13, 2026
… parity rows it exposes (CR 603.3b + CR 603.4) (phase-rs#5732) `integration_cards.json` is a cached subset of the card-data export, but it was last FULLY regenerated at b9685cc (phase-rs#5695). Nineteen parser/types PRs merged since then; every fixture touch in between was surgical (phase-rs#5672 +1, phase-rs#5679 +2, phase-rs#5727 2 entries), so the parse values silently drifted. Regenerated from the export at 8c35dc5 (oracle-gen, MTGJSON 5.3.0+20260629). Population (json deep-equality, not line counts — the file is one line): committed 2658 entries -> 2726. 70 added, 2 removed, 44 changed values. Attribution of the 44 changed (causal: exports built at phase-rs#5720 / phase-rs#5717 / phase-rs#5730 and compared, NOT shape-guessing): phase-rs#5723 (P02-U3b shared condition grammar) ...... 3 archive trap, temple of civilization, thaumaton torpedo (all gained the comparator/lhs/rhs/qty/scope condition shape) phase-rs#5721 + phase-rs#5719 (where-X quantity channel + ..... 21 restriction grammar; both merged BEFORE phase-rs#5717 — merge order != PR-number order) bellowsbreath ogre, cryptex, deadly rollick, deflecting swat, desert, dread wanderer, esquire of the king, flesh, fraying sanity, gloomlake verge, great desert hellion, gutterbones, officious interrogation, once upon a time, potioner's trove, ribald shanty, rock jockey, second little pig, shifting woodland, snuff out, starport security phase-rs#5695..phase-rs#5720 no-regen window (bloc) ........... 20 Stale already at phase-rs#5720, so attributable to the 19-PR window above the b9685cc anchor, not to any single PR: alrund god of the cosmos, animal friend, approach of the second sun, cavernous maw, fblthp the lost, from father to son, hour of revelation, increasing vengeance, jodah the unifier, mana reflection, misty salon, puca's eye, ram through, reidane god of the worthy, secrets of the key, sevinne's reclamation, temple of the dead, the dining car, unleash the flux, valgavoth terror eater The 70 added keys are new test-source card references the generator collects; phase-rs#5729 (tests-only) contributed zero parse delta, as expected. Corrected premise: 44 entries are truly stale, not 7. Six of the seven originally reported reproduce; `osteomancer adept` is NOT stale (committed == fresh). The regen turns `ordering_parity_sweep` red, so the gate's evidence rows ship ATOMICALLY with it. Both rows are population entries, not ordering regressions: the sweep skips Unimplemented-bearing triggers, so a card only enters it once its parse binds. great desert hellion -> BATCH_GENUINE_ROWS. Its LTB Draw was Unimplemented until phase-rs#5721/phase-rs#5719 bound Intensity{Source}. Each co-departing Hellion draws off its OWN intensity but discards the SHARED hand, so the second trigger discards the cards the first just drew: with intensities a != b the final hand, graveyard and library differ by order. The members are not identical functions, so commutation genuinely fails and the new prompt is the CR 603.3b choice the legacy serde walk wrongly auto-ordered (CR 603.5: each "may" is chosen on resolution). planar collapse -> DOCUMENTED_OVER_PROMPT (L8-held family). New fixture key. Upkeep ObjectCount(Creature) >= 4 intervening-if x DestroyAll + self-Sacrifice: the first copy's sweep drives the census to 0, so the sibling's CR 603.4 re-check is false and it does nothing. Monotone and self-limiting — identical siblings commute up to relabeling, so the prompt is conservative, fail-closed and rules-correct. Neither row weakens the gate: both are direction-gated over-prompts (an under-prompt is never suppressible), and both are consumed by the ledger's exact-set asserts (over_prompt_hit 18->19, batch_genuine_hit 1->2), so a misclassification still trips the STRICT PROOF-GATE. Verification: engine lib 16481/16481 pass (was 16480 + 1 red); integration 2929/2929 pass. Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The restriction-only fallback held four parser families that the shared
static-condition grammar could already have expressed — it just did not SPELL
their phrasings. Teach
parse_inner_conditionthe five surfaces and delete thefallback parsers; the shared grammar runs first, so each phrase now converts
through the ordinary
QuantityComparisonvocabulary that every static abilitywith the same words already uses.
Ported (population 35,396 faces; instrument: full-pool dual export, whole-face
structural diff over ALL channels):
-> AttackedThisTurn { scope, filter }. The trailing " this turn" is OPTIONAL:
a full-pool reachability probe showed Thaumaton Torpedo reaching the fallback
WITHOUT it (an upstream duration parser peels it), so requiring the suffix
would have silently dropped that face.
-> ObjectCountBySharedQuality { quality: Name, aggregate: Max }. The
same-quality mirror of the existing
with different namesarm;Maxis whatmakes it "some ONE name is shared by N lands" and not "you control N lands".
their control this turn" -> BattlefieldEntriesThisTurn { player: Opponent{Max} }.
-> PlayerActionsThisTurn { player: Opponent{Max}, action: SearchedLibrary }.
HandSize EQ leaves. "zero" needs its own tag:
parse_number's English tablestarts at "one".
MULTIPLAYER SEMANTIC FIX (Whiplash Trap, Lavaball Trap). The fallback encoded the
opponent INSIDE the TargetFilter (
controller: Opponent), so the runtime SUMMEDbattlefield entries across ALL opponents. Two DIFFERENT opponents with one creature
each therefore satisfied "an opponent had TWO OR MORE creatures enter" — but no
single opponent had two. "An opponent" binds one player and "their control" binds
the count to that same player, so the threshold is per-opponent:
Opponent { Max }.The two readings coincide at two players and only diverge at three or more, which is
why the runtime witness needs three seats.
Retained, with honest justifications — the (c) phrasing-gap class is now EMPTY and
PINNED_RETAINED_FAMILIESdrops 6 -> 4. Everything left is (a) a restriction-contextreferent or (b) a real vocabulary gap:
so StaticCondition::SourceMatchesFilter cannot be converted. Teaching the shared
grammar a source phrasing today would make the shared parse SUCCEED and the
conversion then REJECT it — a NET LOSS of restriction support.
BeenAttackedThisStephas no per-STEP counterpart at all(StaticCondition's attack history is per-TURN), plus the ETB surface that elides
"the battlefield".
in a HIDDEN zone.
ZoneCoreTypeCardCountAtLeastreads player_zone_ids(player, Hand);the shared ObjectCount path discriminates by CONTROLLER and TargetFilter has NO owner
axis. Converting it would not re-spell the phrase, it would ask a different question.
Proven-dead code deleted, not ported: the shared
parse_hand_size_predicatealreadyspelled "no cards in hand", "one or fewer", "exactly N", and "more cards in hand than
each opponent". A full-pool reachability probe (nonvacuous — it registered hits on
every leaf kept) showed ZERO faces reaching those leaves across all 35,396.
Evidence: red-first — the six new
shared_grammar_owns_*tests were watched FAILING ona pristine worktree at this commit's base, each with "NoMatch (fell to restriction-only
grammar)". The whole-face full-pool diff caught a regression this change first
introduced (The Biblioplex, supported -> Unimplemented, because "zero" did not parse)
and it is fixed here.