ship/t95 where x coverage#5763
Merged
Merged
Conversation
matthewevans
commented
Jul 13, 2026
matthewevans
commented
Member
- fix(parser): bind where-X in every quantity-carrying effect (CR 107.3c)
- test(parser): pin the newly-bound where-X faces and witness them at runtime
- fix(parser): rewrite a cost-X token's P/T, not just its count (CR 107.3m)
The def-level where-X pass enumerated only 23 of the 62 `Effect` variants that
carry a `QuantityExpr`. The other 39 fell through `_ => {}` without ever
attempting a bind, so the bare `QuantityRef::Variable { name: "X" }` survived and
resolved to 0 at runtime — discard 0, surveil 0, monstrosity 0 — while the face
rendered as fully supported. #5753's totality guard converted that fabrication
into an honest red (CR 107.3c); this enumerates the arms so the representable
ones bind back to green.
Enumeration alone was not the whole hole. Three further shapes leaked X:
* PARTIAL ARMS — variants that WERE enumerated but left a quantity slot
unbound. `Token.enter_with_counters` is a third X site beside the token's
count and P/T (G'raha Tia creates a fixed 1/1 Hero and puts X counters on
it, so X lives only in the rider). `CastFromZone.constraint` holds the cast
permission's mana-value bound (CR 601.2e), not the target filter, so Kiora's
"mana value less than X" stayed a placeholder and permitted only mana value
< 0 — nothing at all. `BounceAll.count` and `Dig.keep_count_expr` had the
same gap with no card exercising them yet.
* THE P/T WALK — `apply_where_x_expression` matched only
`PtValue::Variable("X")`. When the clause grammar has already lowered the
slot to a quantity, X survives one level down inside
`PtValue::Quantity(Ref { Variable("X") })`. Tivash's X/X Demon rides exactly
that shape, so it entered as an 0/0 and died immediately to state-based
actions (CR 704.5f) under a green badge. `Animate` was the fourth `PtValue`
carrier and had no arm at all.
The wildcard is now reached only by variants with no X slot to bind. It is not an
escape hatch: the totality guard still asserts the post-condition, so a future
variant that adds a quantity slot without an arm reds honestly instead of
fabricating.
Enumeration is necessary but not sufficient for a green face — an expression the
parser cannot represent still reds, which is correct and is left alone here.
…untime Parse-level pins for the three seams the enumeration closed (the newly-enumerated quantity carriers, the token enters-with rider, the cast-permission constraint, and the `PtValue::Quantity` P/T slot). Each was watched FAILING against the pre-change parser; the two preserve controls and the guard test stayed green throughout, so the pins can fail and they fail for the right reason. Also adds RUNTIME witnesses. The parse pins prove the gap node is gone; they do not prove the bound quantity reads the right NUMBER — a slot could bind to a plausible-looking `QuantityRef` that reads the wrong state and still show green on every ledger. Each witness hands the parsed quantity to the live resolver against a state where the referenced fact is true, and is written so the correct answer is distinguishable from every wrong one: Tivash's Demon must read 5 (life gained), not 0 (still unbound) and not 1 (the token's `count` slot); Maester Seymour must read 7 (counters on YOUR creatures), not 0 and not 9 (the "you control" filter dropped). Re-documents `unenumerated_effect_variant_reds_instead_of_fabricating` as `unrepresentable_where_x_expression_reds_instead_of_fabricating`. Its premise — "Surveil is unenumerated, so it reds" — is now false, yet the face still reds, for a different reason: its expression has no `QuantityRef`. The invariant worth pinning is "an unrepresentable where-X reds", not the transient marker; left as written it would have kept passing while quietly meaning something else. Its non-vacuity control is scoped honestly: it discriminates a blanket-red guard, and it does NOT prove the enumeration is load-bearing (the clause-level parser already binds that simple shape). The coverage pins are what prove that.
….3m) Landed under a lead ruling on #96's behalf, as a single pattern-following arm. NOT a closure of #96's class — see the scope note below and in the test. `rewrite_cost_x_in_effect` turns an X-cost spell's `Variable("X")` into `CostXPaid` so its own ETB trigger can read the X actually paid (CR 107.3m stashes it on the permanent). It rewrote `Effect::Token`'s `count` but never the token's `power`/`toughness`, so an X/X token kept a bare `Variable("X")` in its P/T — which resolves to 0. Arboreal Alliance ("When this enchantment enters, create an X/X green Treefolk creature token") therefore made an 0/0 Treefolk that died immediately to state-based actions (CR 704.5f), while the card rendered as fully supported. `Pump`/`PumpAll` already routed their P/T through `rewrite_trigger_pt_variable_x`, and that helper already handled every shape needed here (including a `PtValue::Quantity` wrapping the placeholder). `Token` simply never reached it. This gives `Token` its own arm that rewrites all three slots. SCOPE — what this does NOT fix. Shark Typhoon's cycling token is still an 0/0, and this commit does not touch it. Its trigger is "When you cycle this card", which is not a ChangesZone->Battlefield self-ETB, so `trigger_should_rewrite_cost_x` gates the whole walk off and this arm is never reached. Nor would reaching it be enough: `cost_x_paid` is stamped only by `finalize_cast`, and cycling is an activated ability, not a cast — so `CostXPaid` would resolve to 0 all the same, trading one silent zero for a better-disguised one. Fixing it needs a gate widening AND an engine carrier for an activated ability's X. That is a two-part change, and it stays with #96. Whole-face full-pool ledger, 35,396 faces: this arm changes exactly one face (Arboreal Alliance, Variable("X") -> CostXPaid in the token's P/T). Zero new reds, zero cleared reds.
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Parse changes introduced by this PR · 10 card(s), 13 signature(s) (baseline: main
|
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
1 participant
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.