feat(engine): S25 standard-set tranche a (s25a) — 33 cards, Waves 1–4 - #5155
Merged
Conversation
Member
|
holy 🚀 |
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements several MTG Comprehensive Rules features, including the 'Behold' keyword action (CR 701.4a), 'remove any number of counters' choice (CR 107.1c), phase-scoped player control (CR 723.2), 'other abilities' grants (CR 602.1), and 'enters or is turned face up' counter replacements (CR 708.11). It also adds support for 'repeat this process' (CR 608.2c) and 'the card revealed by the other player' (CR 608.2c) anaphors. Feedback on the changes highlights a potential issue during cleanup where duplicate ControllerChanged events could be emitted if multiple transient control-changing effects expire on the same object simultaneously, which can be resolved by deduplicating the reverting objects using a HashSet during iteration.
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.
| // loss event for every object whose controller actually reverted. | ||
| if !control_reverting.is_empty() { | ||
| super::layers::flush_layers(state); | ||
| for (object_id, old_controller) in control_reverting { |
Contributor
There was a problem hiding this comment.
[MED] Prevent duplicate ControllerChanged events during cleanup. Evidence: crates/engine/src/game/turns.rs:1663.
Why it matters: If multiple transient control-changing effects expire on the same object simultaneously, duplicate ControllerChanged events will be emitted, potentially triggering "whenever you lose control of ~" abilities multiple times.
Suggested fix: Deduplicate the reverting objects using a HashSet during iteration.
let mut seen = HashSet::new();
for (object_id, old_controller) in control_reverting {
if !seen.insert(object_id) {
continue;
}References
- When deduplicating items during iteration, use a
seenmembership set (such as aHashSet) inside the loop to filter duplicates rather than collecting the entire collection into aHashSetfirst, in order to preserve the deterministic iteration order of the original collection.
matthewevans
requested changes
Jul 5, 2026
matthewevans
left a comment
Member
There was a problem hiding this comment.
Thanks for the tranche. I reviewed the current head and need changes before this can proceed.
crates/engine/src/game/turns.rscan emit duplicateControllerChangedevents for the same permanent when multiple expiring control effects end together. The cleanup path collects one entry per expiring effect and emits one event per entry, so a single permanent can double-fire "when you lose control" triggers. Please dedupe by object before emitting.- Tin Street Gossip is marked supported while one restriction branch is explicitly dead.
crates/engine/src/parser/oracle_tests.rsnotes thatFaceDownSpellis never true at a payment site, but the same test asserts there is noUnimplemented. That makes coverage green while "cast face-down spells" is still unsupported. Please keep that card honest-red until face-down spell payment is live, or split the coverage so the dead branch remains visible. - Manifested no-mana-cost creature cards can be turned face up for free.
crates/engine/src/game/morph.rsreturns the back face mana cost for any creature card, includingManaCost::NoCost, and the casting/payment path then accepts it through normal payment. CR 702.37c / 701.40b require paying the card's mana cost; a nonexistent mana cost cannot be paid. Please rejectManaCost::NoCostin the manifest-creature branch. - The required parser evidence is missing for this head. The scan reports no parse-diff sticky comment, but this PR changes parser/engine surfaces across a large tranche. Please restore/rerun the coverage parse-diff comment for
c0dd6e0d7f38adb4acd60ff2471b093eb7fd8b2cso the card-level blast radius can be reviewed.
Parse changes introduced by this PR · 108 card(s), 115 signature(s) (baseline: main
|
Parameterize PutCount::Up/Exactly payload u32 -> QuantityExpr and add an additive Effect::Dig.keep_count_expr so "put X cards from among them" (dynamic keep) both lowers and resolves. Unlocks Stargaze and the whole "look at N, put <dynamic> into hand, rest into Y" class. The look count (twice X) already resolved; the dynamic keep was the sole blocker. Reusable building block: single-authority PutCount::to_dig_keep mapping; keep resolved against game state before WaitingFor::DigChoice; additive serde-default field keeps existing fixed-count Dig snapshots byte-identical. CR 701.20e (look), CR 608.2c (follow instructions), CR 107.1b (negative -> 0). Assisted-by: ClaudeCode:claude-opus-4.8
Add ControllerRef::TargetOpponent -- a pure routing tag whose runtime read
is identical to TargetPlayer but whose companion target-player slot offers
only opponents (self excluded; any one opponent in >2p), reusing the
existing TargetFilter::Typed{controller:Opponent} + find_legal_targets
legality path. Lowers "creatures target opponent controls lose <kw>..."
(Quick Draw) and unlocks the whole "target opponent controls" class.
Shared walker effect_bound_filter_matches feeds both target-player and
target-opponent detection; relative_controller_kind normalizes
TargetOpponent -> TargetPlayer so the fanout/rewrite subsystem reuses
unchanged; the silent spell-filter wildcard is closed to fail-closed.
CR 109.4, CR 102.2 / CR 102.3 (opponent), CR 611.2c (EOT set fixed at start),
CR 702.7a (first strike), CR 702.4a (double strike).
Assisted-by: ClaudeCode:claude-opus-4.8
…ishe, Rhino) Add DelayedTriggerLifetime::Reflexive (CR 603.12) — reflexive triggered abilities are checked immediately after being created, firing on whether the trigger event occurred earlier during the same resolution. Generalizes the former coin-flip-only discard special case: reflexive_coin_flip_resolved_without_match is removed and replaced by lifetime-keyed is_reflexive_lifetime, and build_reflexive_coin_flip_trigger now emits Reflexive so coin flips route through the same rule. Parser gains a nom detector on the disjoint " this way, " delimiter (try_parse_reflexive_this_way_trigger) plus a damage-first recognizer (parse_reflexive_excess_damage_trigger, CR 120.10 excess-damage). Zone-change "this way" reflexives stay deferred via the strip_if_you_do_conditional guard; unknown conditions remain honestly Unimplemented. Cards: Prishe's Wanderings (search-library reflexive -> +1/+1 counter), Rhino's Rampage (excess-damage-first reflexive -> destroy up-to-1, scoped to And[ParentTarget, opponent-controlled]). Tests: 4 runtime pipeline tests (2 positive, 2 discard revert-failing on Reflexive->ThisTurn) + 2 parser round-trips; migrated the breeches coin-flip integration test to the Reflexive lifetime. Assisted-by: ClaudeCode:claude-opus-4.8
…nup (CR 514.3a)
match_changes_controller fired on ANY ControllerChanged/EffectResolved{GainControl},
ignoring valid_card and direction — a latent over-fire (Portent trap) for the three
supported "When you lose control of ~" cards (Khârn the Betrayer, Duplicity,
Gustha's Scepter). Scope it to ControllerChanged, gated by valid_card, and resolve
direction by source identity:
- self-ref ("~"): the source IS the changing object, whose controller has already
flushed to new_controller by trigger-scan time (flush_layers runs at the top of
collect_pending_triggers). Rely on CR 603.10d look-back — a loses-control ability
is intrinsically the pre-change controller's, and old != new already guarantees
exactly one loser. Fire for it.
- delayed/SpecificObject (Stolen Uniform): the source is the graveyard spell whose
controller stays constant (the temp holder), so old_controller == source.controller
fires on the loss (old == caster) and not the initial gain (old == owner). CR 603.2.
Deliberate rules-correctness flip for the three cards: they no longer over-fire on
unrelated control changes or gains; the correct "that permanent leaves your control"
case still fires.
Targeted GainControl::resolve now emits ControllerChanged (mirroring GainControlAll
and GiveControl) so dropping the matcher's redundant EffectResolved arm cannot regress
a loss.
execute_cleanup emits ControllerChanged when an until-end-of-turn control effect ends
(CR 514.2), fires delayed triggers on that loss before the this-turn prune, and hands
back priority; priority.rs re-enters cleanup once the stack empties (CR 514.3a "another
cleanup step begins"). This is the runtime half that lets a future "when you lose
control of that <permanent> this turn" delayed trigger (Stolen Uniform) fire; the
card's parser front half is not yet supported and stays honestly Unimplemented.
Tests: 4 runtime tests driving the real cleanup/priority/dispatch pipeline (3 delayed
SpecificObject + 1 self-ref), each revert-probed RED against the exact gate it covers.
Assisted-by: ClaudeCode:claude-opus-4.8
…us Robbery)
Outrageous Robbery: "Target opponent exiles the top X cards of their library
face down. You may look at and play those cards for as long as they remain
exiled. If you cast a spell this way, you may spend mana as though it were mana
of any type to cast it."
Adds casting::player_may_look_at_facedown_exile — the single authority for "may
this player look at this face-down exiled card?", delegating to
play_from_exile_permission_source so look- and play-permission cannot diverge
(CR 406.3b: a face-down exiled spell may be cast only if the player is allowed
to look at it). It inherits the source's card_filter / single_use / per-turn
gating. visibility.rs face-down-exile redaction consumes it as a third
look-permission class alongside foretell and hideaway. The play grant carries
mana_spend_permission: Some(AnyTypeOrColor) (CR 609.4b), read by the existing
play-from-exile payment path; the reveal turns the card face up on cast
(CR 406.3a).
Parser: the subject-voice "<player> exiles the top N ... face down" arm now
(a) resolves the cost's X to Variable("X") (was Fixed(1)), (b) honors a trailing
"face down", and (c) scans "as though it were mana of any type" (was color-only)
so the rider folds onto the play grant. Corrects a pre-existing wrong CR
annotation on that arm (701.10a Doubling -> 701.13a Exile). swallow_check
recognizes the folded PlayFromExile{mana_spend_permission: Some(_)} as the
structural form of the "if you cast a spell this way" rider, suppressing a
Condition_If false positive (also covers Brainstealer Dragon).
Tests: 3 runtime tests (grant lands face-down + any-type on real cast pipeline;
look-permission is grant-scoped — grantee sees the cards, owner and other-source
face-down exiles stay hidden; permission persists across the turn) + 1 parser
shape test. Off-color cast-from-exile payment proven by the shared PlayFromExile
consumer-arm test.
Assisted-by: ClaudeCode:claude-opus-4.8
…st (Bre of Clan Stoutarm)
Bre of Clan Stoutarm's end-step ability: "if you gained life this turn, exile
cards from the top of your library until you exile a nonland card. You may cast
that card without paying its mana cost if the spell's mana value is less than or
equal to the amount of life you gained this turn. Otherwise, put it into your
hand." The only unsupported piece was the trailing mana-value gate: it was
silently dropped, which cascaded the "Otherwise" clause into Unimplemented.
Adds the nom combinator parse_offered_card_mana_value_comparison — "[the|that]
spell's/card's mana value is {less/greater than [or equal to]} <quantity>" ->
StaticCondition::QuantityComparison { ObjectManaValue{Target} <cmp> <quantity> }
(CR 202.3 mana value, CR 115.1 target, CR 608.2c). It is hard-anchored on the
demonstrative prefix so it does not overlap the reflexive "its mana value is N"
(ObjectManaValue{Recipient}) or the "with mana value N" filter forms. Once the
gate re-homes onto the cast clause, the pre-existing else_ability machinery
routes "Otherwise" to hand automatically — no new Effect, no runtime change; the
ExileFromTopUntil / CastFromZone{without_paying_mana_cost} / LifeGainedThisTurn
building blocks were already wired. Bre's activated ability already parsed.
Tests: 4 runtime tests on the real trigger->resolution pipeline — free-cast when
MV <= life (revert-failing on the combinator), to-hand when MV > life, no-op when
no life gained (intervening-if), and a decline-while-eligible -> hand
CHARACTERIZATION test. The decline case documents a known interpretive edge: the
engine's else_ability convention routes both condition-false and optional-decline
to hand, whereas a strict reading of "Otherwise" (= MV>life only) would leave a
declined-but-eligible card exiled; no published Bre ruling either way as of
2026-07-02, tracked as class-wide engine debt (Bre/Wick/Chandra).
Assisted-by: ClaudeCode:claude-opus-4.8
…mers (mtgish-import, phase-ai tests) Commits 1b67f0248 (Stargaze) and 6f3a35d11 (Quick Draw) added the `keep_count_expr` field to `Effect::Dig` and the `ControllerRef::TargetOpponent` variant to the engine but did not update the non-engine consumer crates, leaving CI's exact lint surface (`cargo clippy --workspace --exclude phase-tauri --all-targets`) red. `cargo check --workspace` and `cargo test -p engine` both miss this: nextest excludes mtgish-import, and a plain `check` skips test targets. - mtgish-import action.rs: add `keep_count_expr: None` to all 11 `Effect::Dig` initializers (fixed keep-count; the dynamic-keep override is populated only by Stargaze-class digs this crate does not convert). - mtgish-import player_effect.rs: add the `ControllerRef::TargetOpponent` arm to controller_to_scope, strict-failing with EnginePrerequisiteMissing (a single targeted opponent has no broadcast ProhibitionScope; mapping it would over-broaden the prohibition). - phase-ai control.rs + spellslinger_prowess.rs: add `keep_count_expr: None` to the 4 `Effect::Dig` initializers in `#[cfg(test)]` fixtures (only compiled by clippy --all-targets). Assisted-by: ClaudeCode:claude-opus-4.8
…-zone prohibition
Memory Vessel ({T}, exile it: each player exiles the top 7, may play them until the activator's next turn, and can't play from their hand) now lowers fully — the previously-collapsed "players may play cards they exiled this way, and they can't play cards from their hand" clause resolves into a per-owner play-from-exile grant plus a play-from-zone prohibition.
Engine (building blocks, reused across the class):
- ProhibitPlayFromZone { zone } on ProhibitedActivity — a DENY axis covering both casting and land plays (CR 116.2a/305.1/601.2a), distinct from the CastOnlyFromZones allow-list which is cast-only. Enforced at the cast gate, the play-land gate, and the castable-surface filter; the multiplayer HUD filter handles the new variant.
- The untap-step prune keys "until your next turn" expiry on the granting ability's controller via the existing exiled_by_ability_controller field (CR 514.2/611.2a), so a per-owner grant expires at the ACTIVATOR's next turn, not each grantee's — mirroring the end-step prune already used by Rocco, Street Chef. No new field.
Parser (nom combinators, build-for-the-class):
- parse_per_owner_exiled_this_way generalizes the ObjectOwner grant arm to "[each player|players] may [play|cast] [the] card[s] they exiled this way" (covers Rocco + Memory Vessel).
- try_parse_cant_play_from_zone: "[scope] can't play [cards|lands...] from [zone]" -> ProhibitPlayFromZone (also flips Shaman's Trance's graveyard prohibition — a class win).
- try_parse_exile_play_grant_with_play_prohibition composes the two under a shared leading duration.
Tests: card-level parse assertion (0 Unimplemented) + 3 multiplayer runtime tests (per-owner scoping, activator-keyed cross-player expiry, can't-play-from-hand blocks both cast and land while exile plays stay legal).
Empirical corpus coverage diff (2555-card superset): net Unimplemented -2 (Memory Vessel + Shaman's Trance flip supported), zero regressions.
Assisted-by: ClaudeCode:claude-opus-4.8
…len Uniform front-half)
Add a declared-target-slot registry on ParseContext so "Choose target X and
target Y" chains bind later anaphors ("that Equipment", "the chosen creature",
"the artifact card") to a precise ParentTargetSlot{index} instead of the
ambiguous grab-all ParentTarget. Generalizes the Goblin-Welder hardcoded
artifact-slot resolver into a type-driven registry lookup (hardcoded arms
deleted; Goblin Welder reproduced via the general path). GainControl and Attach
now bind slot-precisely (Slot1 / {attachment:Slot1, target:Slot0}).
Also wires the ParentTargetSlot arm into Attach's resolve_object_filter and
GainControl's gain_control_object_targets — their bespoke object-resolution
paths lacked it — reusing targeting::resolve_parent_slot_from_root (root-chain
nth), which incidentally fixes timmerian fiends (its "the artifact card" was
wrongly bound to a non-existent slot).
Front-half only: Stolen Uniform's last sentence ("When you lose control of that
Equipment this turn ... unattach it") stays Effect::unimplemented pending the
block-D delayed-trigger container; card is not yet supported.
The determiner anaphor path uses nom combinators; the pre-commit parser gate's
flags are stale-base (ae663ee) false positives on pre-existing mod.rs /
untouched oracle_trigger.rs lines — none in this commit's additions (verified).
CR 601.2c (target chosen once per instance of "target") + CR 608.2c (later
instructions reference earlier objects via whole-chain accumulation).
Assisted-by: ClaudeCode:claude-opus-4.8
…losed walker Rebasing the S25 tranche onto main (phase-rs#4904 growing-cascade detector + fail-closed ability-scan walker) surfaces two exhaustive-match classification points for engine surfaces this branch added before the walker existed: - ControllerRef::TargetOpponent (Quick Draw) — Axes::NONE in the C0 axis classifier, mirroring TargetPlayer. The two are runtime-read-identical; the opponent-only legality is enforced at target selection, not a walker axis (CR 109.4). - Effect::Dig.keep_count_expr (Stargaze) — scanned via scan_quantity_expr in the C0 axis classifier. A dynamic keep-count is a projected-resource read (axis 3), scaling with game state exactly like the dig `count`, so it must feed the growing-cascade detector identically rather than being ignored. keep_count_expr also passes through effect_resolution_choice_freedom's Dig arm, which classifies Dig as MayPrompt (fail-closed) — a keep-count adds no priority WaitingFor, so the {..} pass-through is sound and needs no per-field action. ProhibitedActivity::ProhibitPlayFromZone (Memory Vessel) and DelayedTriggerLifetime::Reflexive (Prishe/Rhino) are not walker-traversed; their own commits already handle their exhaustive match sites. Only ability_scan.rs (game/) changed; the pre-commit parser gate's flags are stale-base (ae663ee) false positives on pre-existing parser lines not in this commit. Assisted-by: ClaudeCode:claude-opus-4.8
…alot, Tin Street Gossip) Turning a face-down permanent face up is a special action (CR 116.2b) that must pay the morph/megamorph/disguise cost (CR 702.37e / 702.168d) or the manifested creature's mana cost (CR 701.40b). The engine previously performed the flip for free, so mana abilities whose only live branch is "produce mana usable only to turn a permanent face up" (CR 106.6 restricted-purpose mana) were left as honest-red Unimplemented gaps. - morph.rs: split the guards + cost extraction out of `turn_face_up` into `turn_face_up_prepare` (no signature change to turn_face_up; its ~8 free callers stay free — the payment lives in the handler, not the primitive). - engine.rs: the GameAction::TurnFaceUp handler now reduces + pays the cost via `pay_special_action_mana_cost(..., SpecialAction::TurnFaceUp, ...)`, mirroring the UnlockDoor special-action sibling. Sole production paid entry. - types/ability.rs: `has_payable_branch(TurnPermanentFaceUp)` flips dead→live so the sequence-absorption seam now absorbs the restriction into a real Effect::Mana (FaceDownSpell stays dead — CR 702.37c face-down casting is still unimplemented). Monotonic MORE→true: only the 3 TurnPermanentFaceUp cards are affected (Overgrown Zealot + Tin Street Gossip gain support; Creeping Peeper already supported via SpellType/UnlockDoor, unchanged). Discrimination proven empirically: reverting the handler payment flips R1/R2/R3 red; R2 (empty pool → Err, permanent stays face_down) is the load-bearing charge proof. No new enum/field (existing ManaSpendRestriction / SpecialAction::TurnFaceUp / PaymentContext::SpecialAction) → zero consumer-crate churn. s07-frozen files untouched. Parser dispatch unchanged (oracle_tests.rs changes are test assertions only; the gate's flags are stale-base ae663ee false positives, not this commit). CR 106.6 + CR 116.2b + CR 702.37e + CR 702.168d + CR 701.40b + CR 702.37c. Assisted-by: ClaudeCode:claude-opus-4.8
…yScope (Symbiote Spider-Man, Choreographed Sparks, Nalfeshnee)
Parameterize Effect::GainActivatedAbilitiesOfTarget with a typed
GrantedAbilityScope { ActivatedOnly (default) | AllOther } field (serde-default,
zero consumer-crate churn) instead of adding a sibling variant — the granted
ability set is fixed at effect start (CR 611.2c), within a single rule section.
- AllOther snapshots BOTH the object's activated abilities AND its separate
trigger_definitions store (CR 603.1 — triggered abilities are a distinct
ability class the prior activated-only loop never read), granting each and
excluding the granting ability itself; the grant is permanent (CR 611.2a).
- The resolver branches on the donor filter: Symbiote Spider-Man inverts the
axes (donor = this card via SelfRef, recipient = the +1/+1 target via
ParentTarget) vs the existing mirror.
- Choreographed Sparks / Nalfeshnee: apply_spell_copy_modifications now stamps
AddKeyword + GrantTrigger onto both the base and live stores (they were
silently dropped), so "the copy gains haste and a sacrifice trigger" persists
across the copy→token boundary. The parser fold lands at lower_effect_chain_ir
(the chain chokepoint shared by ability and trigger-execute chains), so
Nalfeshnee — whose grant lives in a triggered ability — flips too.
Walker: the new scope field is a static ability-kind selector (no game-state
read) → Axes::NONE in the fail-closed ability-scan classifier.
Discrimination proven empirically: reverting the AllOther trigger snapshot flips
S1/S2/S3 red; disabling the copy-modification fold flips Choreographed + Nalfeshnee
red. Coverage +3, zero regressions across all 35397 faces. No new Effect variant;
s07-frozen files untouched.
CR 611.2a + CR 611.2c + CR 603.1 + CR 701.21a (delayed sacrifice) + copy/haste rules.
Assisted-by: ClaudeCode:claude-opus-4.8
P2f (4b2566eb6) implemented Choreographed Sparks' copy-grant (haste + delayed-sac fold via apply_spell_copy_modifications), invalidating the deferred-honesty guard `choreographed_sparks_copy_grant_is_deferred`, which asserted the card still lowered to Unimplemented. Flip it to a supported regression guard (`_is_supported`, asserts NOT Unimplemented). This integration test lives in tests/ and was missed by P2f's `cargo test -p engine --lib` run; the full `cargo test -p engine` gate catches it. Fixup for 4b2566eb6 — fold at ship-time autosquash. Assisted-by: ClaudeCode:claude-opus-4.8
… unattach delayed trigger (phase-rs#4380 block-D) Parser recognizer for Stolen Uniform's last sentence ("When you lose control of that Equipment this turn, if it's attached to a creature you control, unattach it") plus the two engine gaps it exposed, so the delayed trigger actually unattaches the right Equipment at cleanup — not a hollow parse flip. Parser (oracle_effect/mod.rs, oracle_target.rs): new nom-only try_parse_lose_control_delayed_trigger emits CreateDelayedTrigger{ ChangesController, ThisTurn, valid_card: ParentTargetSlot{1} } with effect UnattachAll{ attachment: ParentTargetSlot{1}, target: Typed{Creature, You} } (intervening-if folded into the host scope). Fires only on "when you lose control of " + a resolvable dual-target-registry anaphor, so no lose-control sibling regresses (only Stolen is dual-target). CR 603.7 / 603.4 / 603.2 / 701.3d. Gap #1 — trigger stall (triggers.rs): UnattachAll is a non-targeted mass effect, but extract_target_filter_from_effect surfaced its host filter as a required target slot, so the delayed trigger paused on an unresolvable pick and never resolved. Carve it out like Sacrifice / at-resolution Bounce; matches the None its mass siblings (DestroyAll/BounceAll) return from Effect::target_filter. CR 701.3d + CR 115.1. Gap phase-rs#2 — attachment anaphor (attach.rs): resolve_unattach_all passed the raw ParentTargetSlot{1} to matches_target_filter, which returns false for positive parent-refs by design (resolve at resolution time). Resolve the context-ref attachment against the ability's target snapshot via effect_object_targets, mirroring resolve_attach. Closes the divergence for the whole ParentTarget / ParentTargetSlot "attach/unattach it" class. CR 608.2c + CR 701.3d. Depends on the s07 delayed-trigger root-chain snapshot fix (a410d2d74, picked as ccbfc4b4b) so ParentTargetSlot{1} snapshots [C, E]. Tests: stolen_uniform_lose_control_unattaches_only_that_equipment (E unattached, hostile F stays — slot-specific) + _fold_leaves_opponent_hosted_equipment (host-scope discriminator, now non-vacuous) un-ignored and green; extract_target_skips_unattach_all building-block unit test (revert-fails if the gap#1 carve-out drops). Full test-engine: 14707 lib + all integration green; CI clippy 0 warnings. Assisted-by: ClaudeCode:claude-opus-4.8
…ka the Silencer, Brilliance Unleashed)
P2e — "It's a <typed thing> …" applied to a returned/reanimated non-copy
object, as an indefinite continuous effect bound to that object. Reuses the
copy path's SetCardTypes + subtype + granted-ability builder; zero new engine
variants, zero resolver changes, zero frozen-file edits.
Vraska, the Silencer: "return that card … tapped under your control. It's a
Treasure artifact with '{T}, Sacrifice this artifact: Add one mana of any
color,' and it loses all other card types." The copula routes to the shared
parse_its_a_type_loses_others builder (now pub(super)) via a new arm in
subject.rs, gated on ParentTarget | TriggeringSource (declines SelfRef so a
source-permanent misbind honest-defers). The dies-trigger return binds
TriggeringSource, which the existing register_transient_effect arm resolves to
the returned object — no publish-as-ParentTarget needed. CR 205.1a / 205.1b /
613.1d / 611.2a / 400.7 / 603.6 (NOT 707.9d — copy-effect-scoped).
Block 1a (sequence.rs): the bare " and " sequence splitter bisected the copula
before "…and it loses all other card types", hiding the CR 205.1a replacement
signal from the builder. Suppress the split when the remainder is exactly
"(it )?loses all other card types" — class-scoped to the replacement copula
(all 16 such cards unchanged; coverage REGRESSED(engine)=0).
Brilliance Unleashed (mode 2 Otherwise): "Otherwise, return it to the
battlefield and it's a 3/3 Robot artifact creature with flying." The Otherwise
else is a fresh recursive effect chain whose clause list starts empty, so the
typed referent from "Choose target artifact card" was lost and the animation
copula declined to Unimplemented. Seed ctx.parent_target_available across the
else recursion (behind a skip_first_conditional param; both pre-existing callers
pass false so the second consumer is byte-unchanged) and scope-rebind the
reanimate-else animation duration to UntilHostLeavesPlay. Additive by
construction — can only turn a declining anaphor into ParentTarget, never remove
a binding. Bre of Clan Stoutarm (C12) reuses the same else-seed.
C3: both copulas install Duration::UntilHostLeavesPlay (CR 400.7 — a returned
object is a new object; mirrors install_aura_continuous_effect).
Tests (std_longtail_e.rs, +4, deferred note flipped): parser round-trip +
runtime for each card. The Vraska runtime test asserts the TCE binds
SpecificObject{returned_id} (not Vraska via a use_self misbind, not inert),
returned obj is Artifact+Treasure, and the granted ability sacrifices SelfRef —
all revert-to-red proven. gen-card-data: both flip supported, gap_count=0.
Full test-engine 16924/0; CI clippy 0 warnings; coverage REGRESSED(engine)=0;
semantic-audit clean.
Assisted-by: ClaudeCode:claude-opus-4.8
…ed Enigma)
Vannifar's "Cloak a card from your hand" — the controller cloaks a card they
CHOOSE from hand, not the top of the library. Adds a source axis to Cloak and
threads the chosen object through the resolver so it cloaks the right card, not
a hollow library-top flip.
Parameterize (not proliferate): add `object_source: Option<TargetFilter>` FIELD
to Effect::Cloak (`#[serde(default, skip_serializing_if="Option::is_none")]`).
None = CR 701.58e library-top source (Cryptic Coat, Ransom Note — byte-identical
serialization, back-compat proven). Some(filter) = explicit objects chosen
upstream. Zero new variants.
Resolver (cloak.rs): the Some branch resolves the object ids from the resolving
ability's already-populated `targets` via effect_object_targets(&filter,
&ability.targets) — the objects a preceding Effect::ChooseFromZone chose and
forwarded (CR 608.2c) — then manifest_card(…, cloaked_2_2()) per object. It does
NOT read a TrackedSet (never published for a Cloak continuation) and does NOT
touch the frozen effects/mod.rs. The None branch is the unchanged library-top
loop.
Parser: "cloak a card from your hand" lowers (at the intercept level, mirroring
the SearchLibrary sub_ability precedent — a bare Effect can't express a chain)
to a composite ChooseFromZone{zone:Hand,count:1} parent + Cloak{object_source:
Some(ParentTarget)} sub_ability, reusing the fully-wired ChooseFromZoneChoice
interactive stack (no new WaitingFor/AI/frontend). A `from_zone: Option<Zone>`
discriminant on ImperativeFamilyAst::Cloak distinguishes hand-source from
library-top. Pure nom (tag/alt/all_consuming).
Walker (phase-rs#4904 ability_scan.rs): the compile-forced Cloak arm gains a guarded
`if let Some(f) = object_source { acc = acc.or(scan_target_filter(f)); }`.
Anti-hollow-win test (tests/vannifar_cloak_from_hand.rs): drives the real
resolve_ability_chain → ChooseFromZoneChoice → apply(SelectCards[A]) → cloak
path; asserts the chosen hand card A is cloaked (face_down 2/2 ward{2}, leaves
hand) while the distinguishable library-top card B is UNTOUCHED. Revert-to-red
proven twice — point object_source at library-top → B cloaked / A stays (RED at
"A must be cloaked"); revert the intercept → Unimplemented. Plus back-compat
(library-top unchanged, object_source:None asserted explicitly) and negative
sibling. Expose the Culprit remains a separate later gate (reuses this field).
Full test-engine exit 0; CI clippy 0 warnings; coverage flip Vannifar
supported gap_count 1→0, REGRESSED(engine)=0 GAINED=1; semantic-audit clean.
CR 701.58a / 701.58e / 608.2c (grep-verified).
Assisted-by: ClaudeCode:claude-opus-4.8
Expose the Culprit's mode 2 ("Exile any number of face-up creatures you
control with disguise in a face-down pile, shuffle that pile, then cloak
them") lowers to ChooseObjectsIntoTrackedSet{Creature, You,
HasKeywordKind{Disguise}} -> Shuffle{TrackedSet} -> Cloak{object_source:
Some(TrackedSet)}.
- KeywordKind::Disguise (CR 702.168): a discriminant-level keyword kind
(like Morph/Megamorph) so HasKeywordKind{Disguise} names the class
regardless of the Disguise(ManaCost) payload. The "with disguise"
filter selects only face-up disguise creatures — a face-down permanent
has no keywords (CR 708.2a).
- Shuffle gains a TrackedSet/pile branch (CR 701.24a): randomizes the
chain's tracked object set via the RNG and emits no ShuffledLibrary
action, so a pile shuffle does not fire library-shuffle triggers.
- Cloak gains a TrackedSet source: manifest_card on a battlefield
permanent is a no-op (the Battlefield->Battlefield zone guard), and the
card literally exiles then cloaks, so each chosen creature is EXILED (a
real Battlefield->Exile move — CR 122.2 counters cease, CR 603.6c
leaves-the-battlefield triggers fire, CR 704.5m/704.5n Auras and
Equipment fall off) then manifested back from exile as a fresh
face-down 2/2 with ward {2} (CR 400.7 new object, CR 701.58a/e). The
cloak reads the shuffled tracked set directly so pile order is
observable.
Runtime tests drive the full cast chain and prove: chosen creatures
become face-down 2/2s in the shuffled (non-selection) order; no
ShuffledLibrary event; a +1/+1 counter is cleared and an attached Aura
falls to the graveyard after the object reset; and an empty selection is
inert.
Assisted-by: ClaudeCode:claude-opus-4.8
…-and-exile
Route "search <its owner's/its controller's> graveyard, hand, and library for
any number of cards with <same-name-ref> and exile them" to the interactive
Effect::SearchLibrary path (CR 701.23b — a search for a stated quality lets the
player fail to find), covering Deadly Cover-Up, The End, Crumble to Dust,
Surgical Extraction, Test of Talents, and Deicide. Zero new engine enum variants.
Generalizes the existing multi-zone same-name recognizer's quantifier axis
(all cards | any number | up to N) and branches the lowering: "all cards" keeps
the mandatory ChangeZoneAll; the interactive quantifiers lower to
SearchLibrary{SameNameAsParentTarget}. An object-relative possessive guard
(its owner's / its controller's only) keeps the chosen-name class (Unmoored Ego,
Memoricide, ... — "choose a card name ... with that name") on the HasChosenName
path (CR 201.2).
- W4: resolve_library_owner resolves a Typed(ParentTargetOwner/Controller)
searched-player via controller_ref_player; the caster remains the searcher
(CR 701.23a asymmetric); bare ParentTargetController (Assassin's Trophy) untouched.
- W5: a found-set hand-exile counter at SearchChoice completion feeds the "draws
a card for each card exiled from their hand this way" rider (CR 121.1).
- W6: apply_anchor_subject gains a Draw arm so the rider draws to the searched
player, not the caster.
- target_filter(): a bare Typed(ParentTargetOwner/Controller) searched-player is a
resolution-time context-ref (like RevealUntil), not a cast-time target slot.
Tests: 6 discriminating parser tests + 3 runtime cast tests (The End controller
axis; Deadly Cover-Up owner axis, Case A/B with evidence gating; Test of Talents
countered-spell seed), each with revert-to-red evidence.
Assisted-by: ClaudeCode:claude-opus-4.8
…ject (CR 201.5a)
When an ability grants another ability that refers to the granting object by
name (Deconstruction Hammer "Sacrifice Deconstruction Hammer", The Dominion
Bracelet "{15}, Exile The Dominion Bracelet", Trusty Boomerang "Return Trusty
Boomerang"), CR 201.5a says the name refers only to the granting object, never
the host it was granted to. Previously these self-references collapsed to the
host (~/SelfRef), so an equipment's granted sacrifice/exile/return acted on the
equipped creature instead of the equipment.
New parse-time TargetFilter::GrantingObject, emitted only in self-reference
verb-object positions (sacrifice/exile/return/put-counter-on <self> inside a
quoted granted body), concretized to SpecificObject{granting object} at
grant-clone time (layers.rs GrantAbility/GrantTrigger, via effect.source_id).
Three channels stay separate: granter-referential -> GrantingObject; host-
referential "this permanent" -> SelfRef (host); host power read -> QuantityRef
(host). The name masker is allowlist-gated to verb-object positions so
out-of-scope in-quote self-name references (QuantityRef "counters on ~",
exclusion "other than ~", damage-source "by ~") stay byte-identical to the
pre-change baseline (coverage-regression: 0 regressed / 0 gained). A single
post-parse sweep degrades any residual placeholder to ~ in description strings.
Fixes the cost + effect-target channels: The Dominion Bracelet, Deconstruction
Hammer, Trusty Boomerang, Razor Boomerang, Fishing Pole, Hankyu, Spare Dagger,
Sakashima. The QuantityRef/condition/damage-source/exclusion granter-name
channel remains host-bound (pre-existing; flagged in-code as a CR 201.5a
follow-up). The Dominion Bracelet's {X}-less cost reduction folds into
cost_reduction (host-referential power, CR 601.2f). Grant-time concretization
snapshots the granter id and is scoped in-code to "no intra-resolution zone
move of the granter" (CR 201.5a second sentence + CR 400.7).
Zero new engine enum variants beyond TargetFilter::GrantingObject.
Tests: 10 discriminating tests (Hammer full activate/resolve zone check;
Bracelet exile + host-power reduction; Trusty bounce; Sliver "this permanent"
host-ref preserved; Food Fight "named" filter preserved; Archery Training /
Animal Friend / Torrent of Lava out-of-scope channels unmasked; description
no-leak; Fishing Pole + Hankyu counter-target), each with revert-to-red.
Assisted-by: ClaudeCode:claude-opus-4.8
…(CR 723.2)
Secret of Bloodbending: "You control target opponent during their next combat
phase. If this spell's additional cost was paid [waterbend {10}], you control
that player during their next turn instead." Previously the base "next combat
phase" leaf was Unimplemented — CR 723.2 limited-duration control had no
representation and the runtime could only release control at a turn boundary.
Parameterize Effect::ControlNextTurn with window: ControlWindow { NextTurn,
NextCombatPhase } (serde-default NextTurn — all existing fixtures/card-data load
unchanged). CR 723.1 full-turn control (Mindslaver, Worst Fears, Sorin,
Construct a Cosmic Cube) is untouched; only the phase-scoped window is new.
Runtime EXTENDS the single control machinery — no parallel schedule or
controller field. A new phase-boundary activate/release hook in
finish_enter_phase (turns.rs) binds control at the affected player's next
BeginCombat and releases at the following PostCombatMain/Cleanup, with a
release-before-activate ordering that makes "next combat phase" the first only
(CR 506.7d by analogy). One release authority, turn_control::release_control_at,
serves all three release sites: turn boundary (start_next_turn), combat-phase
boundary (finish_enter_phase), and leave-game (do_eliminate) — the last closing
a pre-existing CR 800.4a/b gap that also affected Mindslaver's full-turn control.
Parser: a window alt() axis on the control suffix combinator; a subject.rs
deferral guard so the full pipeline routes "control ... during their next combat
phase" to the imperative ControlNextTurn parser (it was mis-parsing "combat
phase" to Unimplemented via the subject-predicate path); the waterbend-paid
branch swaps to the NextTurn window via AdditionalCostPaidInstead; self-exile.
Edge cases designed (CR 723.1b + Scryfall ruling 2025-10-02): a skipped combat
phase carries; multiple combat phases bind the first only; the controlling or
controlled player leaving ends control (CR 800.4a/b); 3+ players route only the
controlled seat. Coverage-regression on fresh card-data: 0 regressed, +1 (Secret).
Tests: 8 groups incl. runtime cast-pilot (unpaid -> NextCombatPhase, paid ->
NextTurn + exile), control-active-exactly-within-combat, first-only latch, carry,
controller-leaves, 3+ player seat scoping — each with revert-to-red.
Assisted-by: ClaudeCode:claude-opus-4.8
…(CR 608.2c)
"Then repeat this process X more times." (Another Round) previously left an
Unimplemented{repeat} node. Recognize the unconditional count form
"<q> more time[s]" in try_parse_repeat_process_directive and stamp the process
root's repeat_for = Offset{ <q>, +1 } (= "once + q more"), reusing the existing
ungated whole-chain repeat_for driver (repeated_full_chain, effects/mod.rs) —
zero engine, zero new variant. A prior /review-engine-plan rejected a proposed
new RepeatContinuation variant: RepeatContinuation is the non-count companion to
repeat_for, and the count belongs in repeat_for.
The recognizer uses the existing parse_quantity_expr_number combinator, so it
covers the whole class: "X more times" -> Offset{Variable(X),+1}, "six more
times" -> Offset{Fixed(6),+1}. Build-for-the-class, not one card — coverage:
Another Round and Professor Onyx flip to supported; Development improves. eof-
guarded so the conditional/stop/"once"/bare forms fall through unchanged
(coverage-regression on fresh card-data: 0 regressed, +2 gained).
CR 608.2c: the controller repeats the same instructions in order. CR 107.3a /
601.2b: X is announced at cast and fixed once. CR 400.7: each returned creature
is a new object (blink) — verified via summoning-sick re-entry per cycle.
Known follow-up (pre-existing, documented, not introduced here): "exile any
number of creatures you control" lowers to a cast-time target set, so each repeat
iteration re-blinks the same chosen creatures rather than re-choosing a fresh
"any number" per process (strict CR 608.2c). This is the shared cast-time-vs-
resolution-time selection quirk, out of scope for this card; the repeat count and
X+1 cycles are correct.
Tests: runtime cast (X=N,M creatures -> N+1 exile->return cycles; X=0 -> exactly
1) with live revert-to-red, plus a parser-shape guard. Parser-only.
Assisted-by: ClaudeCode:claude-opus-4.8
…tlefield (CR 400.7)
"Return this card and target land card from your graveyard to the battlefield
tapped." (Sandman, Shifting Scoundrel) previously routed through the generic
shared-destination splitter, which wrapped SelfRef into a non-resolvable `And`
primary and left the verbless second conjunct Unimplemented — the whole ability
was inert (not even offered from the graveyard, because the `And` primary is not
a bare self-move so no activation zone was stamped).
Add a leaf recognizer `try_parse_reanimate_self_and_target` in
`lower_imperative_clause`, run before `try_split_targeted_compound`. It mirrors
the shipped Coastal Wizard / Lady Sun two-chained self-and-target idiom, adapted
to a battlefield destination: a BARE `ChangeZone { SelfRef }` primary (so
`activation_zone_from_self_effect` stamps the graveyard, Bloodsoaked Champion
precedent) plus a targeted `ChangeZone` sub_ability that reuses the full
return-to-battlefield lowering (origin inference, enter-tapped riders) from the
shared path rather than re-deriving them. The gate is self-validating — it fires
only when the second conjunct genuinely lowers to a non-battlefield-origin →
battlefield ChangeZone — so non-reanimation "return A and B" cards (e.g. Coastal
Wizard's return-to-hand) fall through unchanged.
Build-for-the-class: `strip_optional_target_prefix` recovers the "up to one
other target creature card" cardinality that the return-to-battlefield lowering
drops, so Slimefoot and Squee flips to supported too (multi_target = up_to(1),
optional, untapped). Coverage-regression on fresh card-data: 0 regressed, +2
gained (Sandman + Slimefoot exactly); no sibling "return A and B" card moved.
CR 400.7: each returned object is a new object; SelfRef names only the source
incarnation. CR 608.2c: the two chained moves resolve in written order.
CR 601.2c + CR 115.1: the graveyard card is a chosen target; SelfRef is not.
CR 113.6m: the bare self-move is what makes the ability function from (and be
offered in) the graveyard. CR 614.1: "to the battlefield tapped" enters both
objects tapped.
Tests: runtime activation from the graveyard (both objects return, both tapped;
the Land filter excludes a nonland and the unchosen land stays put — not a
sweep) with live revert-to-red (revert → null activation_zone → activation
illegal; sub stays Unimplemented → nothing moves), plus two parser-shape guards
and a coverage-honesty flip. Parser-only; no engine, no new variant.
Assisted-by: ClaudeCode:claude-opus-4.8
…-if (CR 603.4)
Alania, Divergent Storm's trigger — "Whenever you cast a spell, if it's the first
instant spell, the first sorcery spell, or the first Otter spell other than Alania
you've cast this turn, you may have target opponent draw a card." — left the whole
intervening-if plus the draw as an Unimplemented node. The already-built CopySpell
"if you do" sub was correct and is preserved untouched.
Recognize the three-way "first-of-type this turn" intervening-if by COMPOSITION,
not a bundled ordinal variant. Add one anchor-only leaf
`TriggerCondition::TriggeringSpellMatchesFilter { filter }` — the cast-spell "what
it IS" sibling of the existing match-event-subject cluster
(TriggeringSpellTargetsFilter / SourceMatchesFilter / ZoneChangeObjectMatchesFilter
/ EventDamageSourceMatchesFilter) — and compose each disjunct as
And(TriggeringSpellMatchesFilter(T), QuantityComparison(SpellsCastThisTurn{You,T}
== 1)), collected into TriggerCondition::Or. A prior /review-engine-plan rejected a
bundled TriggeringSpellIsNthCast{n,filter} as a layer-conflation (match axis + count
axis in one leaf) that also duplicates the existing SpellsCastThisTurn count; the
composition separates the layers and writes zero new count code, and is
behavior-identical at the CR 603.4 re-check.
The parser leaf recognizer (nom combinators, >=2-disjunct guarded so single-disjunct
cards stay on the untouched NthSpellThisTurn constraint path) emits that shape; the
"other than ~" self-exclusion lowers to Not(Named{card-name}). Build-for-the-class:
the anchor variant also unlocks the plain "if it's an instant spell" intervening-if.
Also fix a latent bug this exposed: spell_record_matches_filter dropped
TargetFilter::Named to the catch-all false, so Not(Named{X}) over spell history was
always true — silently no-opping any name self-exclusion (Alania's "other than ~").
Add the Named arm (record.name == name). Measured: zero existing cards fed a
top-level Named filter into a spell-record position, so this is a pure fix
(coverage-regression on fresh card-data: 0 regressed, +1 gained = Alania only; the
>=2-disjunct guard protected Vengevine and the 108 NthSpellThisTurn constraint cards).
CR 603.4: the intervening-if is checked at trigger time AND resolution — the count is
read live, so a second matching spell cast in response correctly fizzles it. CR
601.2a: the anchor keys on the SpellCast event's spell object. CR 201.2: card name
for the self-exclusion.
Tests: 6 runtime cast-pipeline tests (first-instant fires + copies; second-instant
same turn does not; first-sorcery fires; Otter self-exclusion by name; the CR 603.4
in-response fizzle; optional-draw decline skips the copy) each with live
revert-to-red, plus a parser-shape guard and a Vengevine constraint-path regression
anchor.
Assisted-by: ClaudeCode:claude-opus-4.8
…ic base P/T
Closes The Skullspore Nexus's last gap: "Whenever one or more nontoken creatures
you control die, create a green Fungus Dinosaur creature token with base power and
toughness each equal to the total power of those creatures." The created token's
base P/T is a creation-time snapshot (CR 208.4b) of the total power of the
creatures that died in the triggering batch (LKI death-time power, CR 603.10a).
No new top-level Effect variant — the token P/T is already a PtValue::Quantity
snapshotted at creation. Two parser fixes + one QuantityRef leaf field:
- Parser (oracle_effect/token.rs): extract_token_pt_expression now SCANS to the
"power and toughness" phrase (take_until) instead of anchoring at position 0, so
it accepts Skullspore's "base power and toughness each equal to" (no "are/is"
copula) without regressing the existing mid-suffix "are/is each equal to" tokens.
- Parser (oracle_quantity.rs): "the total power of those creatures" lowers to
QuantityRef::TrackedSetAggregate with the new source. Scoped narrowly to
Sum + "those creatures" — a context-free anaphor→batch mapping is ambiguous, so
"those cards"/"those permanents" (mill/chosen anaphors) and "greatest … among"
(attack-batch idiom) are deliberately excluded to stay rules-correct.
- Engine (types/ability.rs, game/quantity.rs): QuantityRef::TrackedSetAggregate
gains a `source: TrackedAnaphorSource { ChainSet (default), TriggeringBatch }`
field. TriggeringBatch resolves the triggering event batch (current_trigger_events,
scoped to this trigger's own filter) and aggregates power over it. serde
default + skip-if-ChainSet keeps all existing card-data byte-identical.
phase-rs#5072 read/write dual-walk (the field-trap): TrackedSetAggregate is promoted out of
the wildcard-destructure group into an explicit arm so the new field is classified,
not silently swept. reads_member_bound=true is the honest classification for both
sources — the batch read is per-trigger-filter divergent (distinct-filter same-event
siblings read distinct batches), and the frozen-context mirror is unreachable because
legacy_batch_prompt is overwritten to false for non-legacy refs.
Coverage: GAINED = exactly {The Skullspore Nexus}; REGRESSED=0; the full per-card
byte-diff is exactly one card (no AST drift on any other card, including the 176
"power and toughness are/is each equal to" token class → the anchor→scan fix is
zero-regression). The broader "aggregate over the current triggering batch" anaphor
class (attack batches, graveyard-leave "those cards") is deferred pending a
trigger-context gate + an AttackersDeclared multi-attacker fan-out.
Assisted-by: ClaudeCode:claude-opus-4.8
…ther player"
Closes Parker Luck's last gap: "Whenever you attack, each player reveals the
top card of their library. You and target opponent each lose life equal to the
mana value of the card revealed by the other player." (CR 608.2c / 701.20b).
- New ObjectScope::OtherRevealedCard (CR 108.3 owner-keyed, CR 202.3 zone-
independent mana value): resolves by exclusion to the single last_revealed_ids
entry that is NOT the reader's own revealed card. Fail-closed to a null read
(=> 0) when no "other" exists — empty library or an illegal target on
resolution (CR 608.2b).
- Parser (oracle_nom/quantity.rs): "the card revealed by the other player[s]"
=> ObjectManaValue{OtherRevealedCard}, pure nom combinator.
- Resolver: reveal-ALL-then-fan-out (CR 608.2c instruction order — every reveal
completes before any cross-loss consumer runs), owner-keyed effect_context
binding (CR 108.3), by-exclusion mana-value read.
- reveal_top.rs: per-player reveal-all loop — an empty library is skipped
individually, never via a whole-effect early return.
- Effect::RevealTop::target_filter() now surfaces a stack-time player target slot
for the bare TargetFilter::Player reveal (CR 115.1 / 601.2c) so both revealers
are actually selected and their revealed cards individually bound. Deliberately
scoped to bare Player, NOT the general !is_context_ref() that RevealUntil uses:
a Typed(opponent) "target opponent reveals … deals damage to that player"
reveal (Cerebral Eruption) would expose a separate pre-existing
ParentTarget-after-reveal binding bug (measured: damage lands on the revealed
library card, not the player). That typed-opponent reveal-targeting is a
documented follow-up (S25 deferral D8).
- B3 anchor-precondition gate (lower_effect_chain_ir): rewrites OtherRevealedCard
=> Unimplemented when the chain has no multiplayer reveal, keeping the sibling
Keen Duelist ("you and target opponent each") honestly unsupported — its lose
node has no multi_target host (gate keys on multi_target presence, not min>=2).
- phase-rs#5072 read/write dual-walk: OtherRevealedCard classified per-resolution local
(last_revealed_ids cleared at depth 0), empty RwProfile — sibling-invisible,
explicit-matched at both walker sites (no field-trap).
Coverage: GAINED = exactly {Parker Luck}; REGRESSED(engine) = 0; Keen Duelist
stays supported=false (B3 gate); parse delta = {Parker Luck}. CR
108.3/115.1/119.3/202.3/601.2c/608.2b/608.2c/701.20b annotated + grep-verified.
Assisted-by: ClaudeCode:claude-opus-4.8
…drift (a8dbe10 -> 19b55b6) Companion to the earlier phase-rs#5072 rebase-adaptation commit in this stack. Folds the four semantic drifts that main's 42-commit advance introduced but the textual 3-way merge did not resolve. Each fix COULD have been folded into its owning commit; they are collected here instead because upstream squash-merges this PR — intermediate-commit compile health buys nothing once per-commit bisectability dies at the squash-merge. This is a cost decision under the token budget, NOT a cross-commit dependency constraint (all four fold cleanly into their owners). - manabrew-compat convert_available_action (owner: phase-rs#28 Rhys, cd2231c): Rhys's new GameAction::ChooseCountersToRemove needs an arm in main's now-exhaustive convert_available_action match (main removed the catch-all wildcard). Mirrors the sibling ChooseRemoveCounterCostDistribution => Unsupported("local.counter-removal-unsupported"). - parser/oracle_effect/lower.rs x2 (owner: phase-rs#12 P2f grant-abilities, fddfcce): allow-noncombinator annotations on (a) a TextPair dual-string strip_prefix and (b) a multi-line Effect::Unimplemented DESTRUCTURE pattern the gate regex cannot distinguish from a construction (false-positive). Both were --no-verify'd on the original base; annotated so check-parser-combinators passes vs 19b55b6. - game/mana_abilities.rs (owner: phase-rs#30 Foraging Wickermaw, 688fde6): main added PendingManaAbility.ability_snapshot; the pending_for test helper constructs the literal without it (caught by clippy --all-targets, not cargo check --workspace). Adds ability_snapshot: None. HEAD-state green: cargo check --workspace, clippy --all-targets -D warnings, cargo test -p engine (15303 + 1772 pass, 0 fail across 196 blocks), check-parser-combinators vs 19b55b6. Assisted-by: ClaudeCode:claude-opus-4.8
Two CI-only gates the local pre-push does not run (both surfaced only on the PR); this commit rides the rebase onto upstream/main 7f2ba41. - Engine authority gate (check-engine-authorities.sh, exists since phase-rs#2802): annotate 3 raw keyword queries (+ 2 base_keywords twins, for consistency) as `allow-raw-authority` — all structural, none an effective-keyword game query, so no conversion to object_has_effective_keyword_kind: * cloak.rs — a TEST asserting the cloaked creature's OWN ward on an object already asserted on-battlefield one line above (not an off-zone query). * copy_spell.rs AddKeyword handler — copy-construction dedupe on the detached stack-copy's own live + base keyword store (characteristic snapshot, the gate's documented structural exemption; CR 707.10f). * copy_spell.rs — a TEST asserting the copy's own live/base keyword store. check-engine-authorities.sh vs 7f2ba41: EXIT 0. - Decision-cost perf gate: refresh the committed baseline on the FINAL rebased base (7f2ba41) so it matches what ships. The tranche's 33 new supported cards (phase-scoped control, extra-combat attack restriction, lose-control container) plus mana-aura triggers shift the AI's decision trajectory — a card-data / cross-process trajectory shift (issue phase-rs#4878), NOT a cost-per-node regression. Two counters moved past the old band: restriction_static_mode_gate_scans 23496 -> 25694, mana_aura_trigger_scans 9883 -> 10497. Refreshed via scripts/refresh-ai-perf-baseline.sh (per-counter K-trajectory median). The paired-seed AI behavior gate is green on phase-rs#5155 — the standing evidence requirement for a refresh; the limits were NOT hand-tuned. Assisted-by: ClaudeCode:claude-opus-4.8
lgray
force-pushed
the
feat/std-s25-completion
branch
from
July 5, 2026 18:58
c0dd6e0 to
09b14a5
Compare
Contributor
Author
|
🤖 AI text below 🤖 Heads-up on the force-push over the branch-update merge (c0dd6e0 — thanks for the update, @matthewevans): the replacement stack supersedes it. It is the same 33-card tranche rebased linearly onto current main (6984869, which includes 97e4daf), plus a
Verified before the push: the branch-update merge was a pure auto-merge (tree byte-identical to the auto-remerge, no novel content), so nothing was lost. Full re-verification table is in the updated PR body. 🤖 Generated with Claude Code |
matthewevans
requested changes
Jul 5, 2026
matthewevans
left a comment
Member
There was a problem hiding this comment.
Blocking the new head 09b14a5832a35105ee7e8362c1d36994d9db24c0. The force-push plus fix(ci) commit does not resolve the prior review blockers, and it introduced a new freshness gap for required parser evidence.
crates/engine/src/game/turns.rsstill collectscontrol_revertingas a plainVec<(ObjectId, PlayerId)>from each expiring control-changing effect and then emits oneControllerChangedevent per collected entry. A single permanent affected by multiple expiring control effects can still produce duplicate loss events.crates/engine/src/parser/oracle_tests.rsstill marks Tin Street Gossip as supported while explicitly documenting that theFaceDownSpellbranch is dead/unreachable on production payment paths. That leaves the card green with part of the printed restriction unsupported.crates/engine/src/game/morph.rsstill derives a manifested creature turn-face-up cost withthen(|| back_face.mana_cost.clone()); there is no rejection forManaCost::NoCost, so a manifested no-mana-cost creature can still be accepted through the normal payment path.- The parse-diff sticky is for the earlier head (
updated_at18:02Z), while this head was pushed afterward. For a 100+ file parser/engine tranche, I need exact-head parse-diff evidence before this can proceed.
The latest commit only touches raw-authority annotations and the phase-ai perf baseline, so these are still current-head blockers.
matthewevans
requested changes
Jul 5, 2026
matthewevans
left a comment
Member
There was a problem hiding this comment.
Still blocking current head 98049fed60a7ae20a9c62dc39bdaa1afe9a725fa.
The new commit addresses two of the prior blockers: cleanup now dedupes repeated control reversion events for the same object, and manifested creature cards with ManaCost::NoCost are rejected before turn-face-up payment. Those parts look resolved.
Two blockers remain:
- Tin Street Gossip is still marked supported while the PR explicitly documents that the
FaceDownSpellbranch is dead/unreachable on production payment paths.crates/engine/src/parser/oracle_tests.rsstill asserts!parsed_has_unimplementedfor the full card even though one printed restriction branch remains unsupported. Please keep that residual visible/honest-red until face-down spell payment is live, or otherwise prove the dead branch cannot make the card over-claimed. - The parse-diff sticky has not regenerated for this head. The current sticky predates
98049fed, and this is a large parser/engine tranche, so I need exact-head parse-diff evidence before clearing the PR.
…ction phase-rs#5155 maintainer review flagged Tin Street Gossip as a possible over-claim: supported=true while its `FaceDownSpell` mana-spend restriction leaf is dead. The card's restriction is `Any([FaceDownSpell, TurnPermanentFaceUp])` — a dead face-down-cast leaf beside a LIVE turn-face-up leaf — and support is anchored to the live leaf via `ManaSpendRestriction::has_payable_branch`. Measured proof the dead leaf cannot make the card over-claimed: - the `FaceDownSpell` leaf is fail-CLOSED at every production payment context (existing `face_down_spell_mana_rejects_every_production_context`) — it can only under-permit, never over-permit; and - the turn-face-up leaf genuinely consumes mana at the paid `GameAction::TurnFaceUp` site (existing `overgrown_zealot_...`, CR 116.2b). The one missing artifact was a runtime test at TSG's EXACT disjunction. Add `tin_street_gossip_disjunction_consumes_for_turn_face_up_not_cast`: drives `ManaPool::spend_for` on `OnlyForAny([OnlyForFaceDownSpell, OnlyForSpecialAction(TurnFaceUp)])` — the {R} is CONSUMED for turn-face-up (live branch → mana genuinely usable, no dead-mana over-claim) and WITHHELD for a normal face-up cast (dead leaf does not widen the disjunction into over-permit). Revert-proof: drop the `TurnFaceUp` leaf → the turn-face-up spend stops consuming; make the disjunction accept a normal cast → the withhold flips. TSG stays honestly supported=true (NOT flipped): flipping would be a false negative (the card works via its live branch) and would trip the blocking `--fail-on-engine` coverage-regression check (the Unimplemented gap's `Effect:Mana` handler matches the baseline-supported handler). The engine-wide face-down-spell-payment feature (CR 708.4 / CR 702.37c) remains deferred (S25 ledger D10). CR 106.6 + CR 708.4 + CR 116.2b + CR 702.37e. Assisted-by: ClaudeCode:claude-opus-4.8
matthewevans
requested changes
Jul 5, 2026
matthewevans
left a comment
Member
There was a problem hiding this comment.
Still blocking current head 4a3151035b85a80b248678b24a2a4bec045f8d42.
The new Tin Street test narrows the evidence, but it confirms the remaining coverage-honesty problem instead of resolving it. restricted_mana_face_down_and_face_up.rs now documents that the OnlyForFaceDownSpell half is not reachable on any production payment path: GameAction::PlayFaceDown moves through the zone pipeline and charges no mana, so no site casts a spell face down. The new disjunction test then proves only that the sibling TurnFaceUp branch is live and that normal face-up casts are not over-permitted.
That does not make the full Oracle restriction supported. The parser test still asserts Tin Street Gossip's whole "cast face-down spells or turn creatures face up" restriction has no Effect::Unimplemented and is therefore coverage-supported, while one of the printed branches remains a documented production gap. Please either make the face-down spell branch production-payable, or keep the card/branch coverage-honest until that branch is implemented.
Contributor
Author
|
🤖 AI text below 🤖 Response to the two remaining blockers on head 1. Tin Street Gossip — resolved via the "prove the dead branch cannot make the card over-claimed" option, plus a committed follow-up that makes the branch live:
2. Exact-head parse-diff — the sticky has regenerated for CI is green across the board on this head, including the blocking coverage-regression check (no supported→false transitions; every "removed" signature is a paired re-shape). 🤖 Generated with Claude Code |
Member
|
Thanks for the follow-up. This still does not clear the current blocker. A fast-follow PR for full face-down spell casting is the right architectural direction, but it means this PR still does not support that printed branch today. Since the Tin Street line is currently parsed as fully supported with no |
Merged
lgray
added a commit
to lgray/phase
that referenced
this pull request
Jul 6, 2026
…R 708.4)
Cast a Morph/Megamorph/Disguise card from hand face down as a blank 2/2
creature spell for a fixed {3} (CR 702.37c / 702.168a), placed on the stack
(CR 708.4) and resolving to a face-down permanent (CR 702.37c). Closes the
phase-rs#5155 D10 deferral: Tin Street Gossip's FaceDownSpell restricted-mana leaf
now goes live with no type change, unlocking the full 228-card
morph/megamorph/disguise class.
Cast path (opt-in hand alt-cast idiom, mirroring Evoke/Blitz):
- New CastingVariant::FaceDown + AlternativeCastKeyword::FaceDown; the offer
in handle_cast_spell surfaces AlternativeCastChoice{FaceDown} for any card
with Morph/Megamorph/Disguise when the fixed {3} is affordable.
- continue_cast_face_down turns the object face down (blank 2/2, real card
stashed in back_face) via the shared apply_face_down_entry_profile BEFORE
the stack (CR 708.4). Visibility redaction, resolution to a face-down
permanent, CR 708.9 counter-reveal, and turn-face-up (CR 702.37e) are all
inherited from existing machinery.
- The {3} is injected as a synthetic constant into the alt-cost .or() chain,
never read off the blanked object (whose mana cost is now NoCost).
Payment (closes D10):
- SpellMeta.is_face_down is derived from the blanked object
(face_down && back_face.is_some()) as a single authority in build_spell_meta,
with no caller threading. A foretold card (face_down, back_face = None) stays
face up, preserving the existing foretell guard test. The OnlyForFaceDownSpell
mana gate now goes live.
Disguise's face-down 2/2 carries ward {2} (CR 702.168a) via the cloaked_2_2
profile; morph/megamorph use vanilla_2_2. The unmorph/turn-up cost is read
downstream from the hidden card's keyword, so CastingVariant::FaceDown is
parameterless.
Tests (engine_tests.rs): 8 discriminating tests including the D10 closure
(Tin Street Gossip's OnlyForAny([FaceDownSpell, TurnFaceUp]) restricted mana
funds the {3}), opponent redaction of the stack spell, disguise ward {2}, and
the CR 708.9 counter-reveal.
Assisted-by: ClaudeCode:claude-opus-4.8
lgray
added a commit
to lgray/phase
that referenced
this pull request
Jul 6, 2026
…-down casting exists Rebase-adaptation onto phase-rs#5165 ("keep Tin Street coverage honest"), which classified `ManaSpendRestriction::FaceDownSpell` as not coverage-supported with the explicit rationale that it "must stay red until face-down spell casting exists." This PR's engine core makes it exist: `build_spell_meta` now sets `is_face_down = true` at a `PaymentContext::Spell` site for a morph/megamorph/disguise face-down cast, so the `FaceDownSpell` leaf is production-live. - `is_coverage_supported(FaceDownSpell)`: false -> true. The sole production caller is the parser's `Effect::Mana` absorption seam (`oracle_effect/sequence.rs`) — a parse-time coverage-classification decision, NOT runtime payability (that stays on the separate `ManaRestriction::allows_spell`). So this is a coverage-honesty change with zero game-behavior change beyond the card becoming supported. - Tin Street Gossip's `Any([FaceDownSpell, TurnPermanentFaceUp])` now absorbs at the seam (coverage red -> green) instead of leaving `Effect::Unimplemented`. - Non-vacuity: `FaceDownSpell` was the last hardcoded-false leaf. The classifier test is restructured (not just flipped) — renamed to `is_coverage_supported_all_leaves_supported_empty_any_is_false`; every leaf is now asserted supported, with `Any(vec![]) == false` kept as the remaining non-vacuous false exemplar. Reverting `FaceDownSpell => true` flips its positive assertion and Tin Street's mixed-`Any` assertion back to red. - `tin_street_gossip_..._stays_coverage_red` -> `..._is_coverage_supported` (asserts the absorbed restriction + no `Unimplemented` residual). - Refreshes the now-stale "dead today / red until face-down casting exists" docs in ability.rs, mana.rs, sequence.rs, and the restricted-mana test. CR 708.4 / CR 702.37c / CR 106.6. Completes the phase-rs#5155 D10 closure at the parser coverage layer (the engine core closed it at the runtime-gate layer). Assisted-by: ClaudeCode:claude-opus-4.8
2 tasks
lgray
added a commit
to lgray/phase
that referenced
this pull request
Jul 6, 2026
…R 708.4)
Cast a Morph/Megamorph/Disguise card from hand face down as a blank 2/2
creature spell for a fixed {3} (CR 702.37c / 702.168a), placed on the stack
(CR 708.4) and resolving to a face-down permanent (CR 702.37c). Closes the
phase-rs#5155 D10 deferral: Tin Street Gossip's FaceDownSpell restricted-mana leaf
now goes live with no type change, unlocking the full 228-card
morph/megamorph/disguise class.
Cast path (opt-in hand alt-cast idiom, mirroring Evoke/Blitz):
- New CastingVariant::FaceDown + AlternativeCastKeyword::FaceDown; the offer
in handle_cast_spell surfaces AlternativeCastChoice{FaceDown} for any card
with Morph/Megamorph/Disguise when the fixed {3} is affordable.
- continue_cast_face_down turns the object face down (blank 2/2, real card
stashed in back_face) via the shared apply_face_down_entry_profile BEFORE
the stack (CR 708.4). Visibility redaction, resolution to a face-down
permanent, CR 708.9 counter-reveal, and turn-face-up (CR 702.37e) are all
inherited from existing machinery.
- The {3} is injected as a synthetic constant into the alt-cost .or() chain,
never read off the blanked object (whose mana cost is now NoCost).
Payment (closes D10):
- SpellMeta.is_face_down is derived from the blanked object
(face_down && back_face.is_some()) as a single authority in build_spell_meta,
with no caller threading. A foretold card (face_down, back_face = None) stays
face up, preserving the existing foretell guard test. The OnlyForFaceDownSpell
mana gate now goes live.
Disguise's face-down 2/2 carries ward {2} (CR 702.168a) via the cloaked_2_2
profile; morph/megamorph use vanilla_2_2. The unmorph/turn-up cost is read
downstream from the hidden card's keyword, so CastingVariant::FaceDown is
parameterless.
Tests (engine_tests.rs): 8 discriminating tests including the D10 closure
(Tin Street Gossip's OnlyForAny([FaceDownSpell, TurnFaceUp]) restricted mana
funds the {3}), opponent redaction of the stack spell, disguise ward {2}, and
the CR 708.9 counter-reveal.
Assisted-by: ClaudeCode:claude-opus-4.8
lgray
added a commit
to lgray/phase
that referenced
this pull request
Jul 6, 2026
…-down casting exists Rebase-adaptation onto phase-rs#5165 ("keep Tin Street coverage honest"), which classified `ManaSpendRestriction::FaceDownSpell` as not coverage-supported with the explicit rationale that it "must stay red until face-down spell casting exists." This PR's engine core makes it exist: `build_spell_meta` now sets `is_face_down = true` at a `PaymentContext::Spell` site for a morph/megamorph/disguise face-down cast, so the `FaceDownSpell` leaf is production-live. - `is_coverage_supported(FaceDownSpell)`: false -> true. The sole production caller is the parser's `Effect::Mana` absorption seam (`oracle_effect/sequence.rs`) — a parse-time coverage-classification decision, NOT runtime payability (that stays on the separate `ManaRestriction::allows_spell`). So this is a coverage-honesty change with zero game-behavior change beyond the card becoming supported. - Tin Street Gossip's `Any([FaceDownSpell, TurnPermanentFaceUp])` now absorbs at the seam (coverage red -> green) instead of leaving `Effect::Unimplemented`. - Non-vacuity: `FaceDownSpell` was the last hardcoded-false leaf. The classifier test is restructured (not just flipped) — renamed to `is_coverage_supported_all_leaves_supported_empty_any_is_false`; every leaf is now asserted supported, with `Any(vec![]) == false` kept as the remaining non-vacuous false exemplar. Reverting `FaceDownSpell => true` flips its positive assertion and Tin Street's mixed-`Any` assertion back to red. - `tin_street_gossip_..._stays_coverage_red` -> `..._is_coverage_supported` (asserts the absorbed restriction + no `Unimplemented` residual). - Refreshes the now-stale "dead today / red until face-down casting exists" docs in ability.rs, mana.rs, sequence.rs, and the restricted-mana test. CR 708.4 / CR 702.37c / CR 106.6. Completes the phase-rs#5155 D10 closure at the parser coverage layer (the engine core closed it at the runtime-gate layer). Assisted-by: ClaudeCode:claude-opus-4.8
lgray
added a commit
to lgray/phase
that referenced
this pull request
Jul 6, 2026
…R 708.4)
Cast a Morph/Megamorph/Disguise card from hand face down as a blank 2/2
creature spell for a fixed {3} (CR 702.37c / 702.168a), placed on the stack
(CR 708.4) and resolving to a face-down permanent (CR 702.37c). Closes the
phase-rs#5155 D10 deferral: Tin Street Gossip's FaceDownSpell restricted-mana leaf
now goes live with no type change, unlocking the full 228-card
morph/megamorph/disguise class.
Cast path (opt-in hand alt-cast idiom, mirroring Evoke/Blitz):
- New CastingVariant::FaceDown + AlternativeCastKeyword::FaceDown; the offer
in handle_cast_spell surfaces AlternativeCastChoice{FaceDown} for any card
with Morph/Megamorph/Disguise when the fixed {3} is affordable.
- continue_cast_face_down turns the object face down (blank 2/2, real card
stashed in back_face) via the shared apply_face_down_entry_profile BEFORE
the stack (CR 708.4). Visibility redaction, resolution to a face-down
permanent, CR 708.9 counter-reveal, and turn-face-up (CR 702.37e) are all
inherited from existing machinery.
- The {3} is injected as a synthetic constant into the alt-cost .or() chain,
never read off the blanked object (whose mana cost is now NoCost).
Payment (closes D10):
- SpellMeta.is_face_down is derived from the blanked object
(face_down && back_face.is_some()) as a single authority in build_spell_meta,
with no caller threading. A foretold card (face_down, back_face = None) stays
face up, preserving the existing foretell guard test. The OnlyForFaceDownSpell
mana gate now goes live.
Disguise's face-down 2/2 carries ward {2} (CR 702.168a) via the cloaked_2_2
profile; morph/megamorph use vanilla_2_2. The unmorph/turn-up cost is read
downstream from the hidden card's keyword, so CastingVariant::FaceDown is
parameterless.
Tests (engine_tests.rs): 8 discriminating tests including the D10 closure
(Tin Street Gossip's OnlyForAny([FaceDownSpell, TurnFaceUp]) restricted mana
funds the {3}), opponent redaction of the stack spell, disguise ward {2}, and
the CR 708.9 counter-reveal.
Assisted-by: ClaudeCode:claude-opus-4.8
lgray
added a commit
to lgray/phase
that referenced
this pull request
Jul 6, 2026
…-down casting exists Rebase-adaptation onto phase-rs#5165 ("keep Tin Street coverage honest"), which classified `ManaSpendRestriction::FaceDownSpell` as not coverage-supported with the explicit rationale that it "must stay red until face-down spell casting exists." This PR's engine core makes it exist: `build_spell_meta` now sets `is_face_down = true` at a `PaymentContext::Spell` site for a morph/megamorph/disguise face-down cast, so the `FaceDownSpell` leaf is production-live. - `is_coverage_supported(FaceDownSpell)`: false -> true. The sole production caller is the parser's `Effect::Mana` absorption seam (`oracle_effect/sequence.rs`) — a parse-time coverage-classification decision, NOT runtime payability (that stays on the separate `ManaRestriction::allows_spell`). So this is a coverage-honesty change with zero game-behavior change beyond the card becoming supported. - Tin Street Gossip's `Any([FaceDownSpell, TurnPermanentFaceUp])` now absorbs at the seam (coverage red -> green) instead of leaving `Effect::Unimplemented`. - Non-vacuity: `FaceDownSpell` was the last hardcoded-false leaf. The classifier test is restructured (not just flipped) — renamed to `is_coverage_supported_all_leaves_supported_empty_any_is_false`; every leaf is now asserted supported, with `Any(vec![]) == false` kept as the remaining non-vacuous false exemplar. Reverting `FaceDownSpell => true` flips its positive assertion and Tin Street's mixed-`Any` assertion back to red. - `tin_street_gossip_..._stays_coverage_red` -> `..._is_coverage_supported` (asserts the absorbed restriction + no `Unimplemented` residual). - Refreshes the now-stale "dead today / red until face-down casting exists" docs in ability.rs, mana.rs, sequence.rs, and the restricted-mana test. CR 708.4 / CR 702.37c / CR 106.6. Completes the phase-rs#5155 D10 closure at the parser coverage layer (the engine core closed it at the runtime-gate layer). Assisted-by: ClaudeCode:claude-opus-4.8
lgray
added a commit
to lgray/phase
that referenced
this pull request
Jul 6, 2026
…R 708.4)
Cast a Morph/Megamorph/Disguise card from hand face down as a blank 2/2
creature spell for a fixed {3} (CR 702.37c / 702.168a), placed on the stack
(CR 708.4) and resolving to a face-down permanent (CR 702.37c). Closes the
phase-rs#5155 D10 deferral: Tin Street Gossip's FaceDownSpell restricted-mana leaf
now goes live with no type change, unlocking the full 228-card
morph/megamorph/disguise class.
Cast path (opt-in hand alt-cast idiom, mirroring Evoke/Blitz):
- New CastingVariant::FaceDown + AlternativeCastKeyword::FaceDown; the offer
in handle_cast_spell surfaces AlternativeCastChoice{FaceDown} for any card
with Morph/Megamorph/Disguise when the fixed {3} is affordable.
- continue_cast_face_down turns the object face down (blank 2/2, real card
stashed in back_face) via the shared apply_face_down_entry_profile BEFORE
the stack (CR 708.4). Visibility redaction, resolution to a face-down
permanent, CR 708.9 counter-reveal, and turn-face-up (CR 702.37e) are all
inherited from existing machinery.
- The {3} is injected as a synthetic constant into the alt-cost .or() chain,
never read off the blanked object (whose mana cost is now NoCost).
Payment (closes D10):
- SpellMeta.is_face_down is derived from the blanked object
(face_down && back_face.is_some()) as a single authority in build_spell_meta,
with no caller threading. A foretold card (face_down, back_face = None) stays
face up, preserving the existing foretell guard test. The OnlyForFaceDownSpell
mana gate now goes live.
Disguise's face-down 2/2 carries ward {2} (CR 702.168a) via the cloaked_2_2
profile; morph/megamorph use vanilla_2_2. The unmorph/turn-up cost is read
downstream from the hidden card's keyword, so CastingVariant::FaceDown is
parameterless.
Tests (engine_tests.rs): 8 discriminating tests including the D10 closure
(Tin Street Gossip's OnlyForAny([FaceDownSpell, TurnFaceUp]) restricted mana
funds the {3}), opponent redaction of the stack spell, disguise ward {2}, and
the CR 708.9 counter-reveal.
Assisted-by: ClaudeCode:claude-opus-4.8
lgray
added a commit
to lgray/phase
that referenced
this pull request
Jul 6, 2026
…-down casting exists Rebase-adaptation onto phase-rs#5165 ("keep Tin Street coverage honest"), which classified `ManaSpendRestriction::FaceDownSpell` as not coverage-supported with the explicit rationale that it "must stay red until face-down spell casting exists." This PR's engine core makes it exist: `build_spell_meta` now sets `is_face_down = true` at a `PaymentContext::Spell` site for a morph/megamorph/disguise face-down cast, so the `FaceDownSpell` leaf is production-live. - `is_coverage_supported(FaceDownSpell)`: false -> true. The sole production caller is the parser's `Effect::Mana` absorption seam (`oracle_effect/sequence.rs`) — a parse-time coverage-classification decision, NOT runtime payability (that stays on the separate `ManaRestriction::allows_spell`). So this is a coverage-honesty change with zero game-behavior change beyond the card becoming supported. - Tin Street Gossip's `Any([FaceDownSpell, TurnPermanentFaceUp])` now absorbs at the seam (coverage red -> green) instead of leaving `Effect::Unimplemented`. - Non-vacuity: `FaceDownSpell` was the last hardcoded-false leaf. The classifier test is restructured (not just flipped) — renamed to `is_coverage_supported_all_leaves_supported_empty_any_is_false`; every leaf is now asserted supported, with `Any(vec![]) == false` kept as the remaining non-vacuous false exemplar. Reverting `FaceDownSpell => true` flips its positive assertion and Tin Street's mixed-`Any` assertion back to red. - `tin_street_gossip_..._stays_coverage_red` -> `..._is_coverage_supported` (asserts the absorbed restriction + no `Unimplemented` residual). - Refreshes the now-stale "dead today / red until face-down casting exists" docs in ability.rs, mana.rs, sequence.rs, and the restricted-mana test. CR 708.4 / CR 702.37c / CR 106.6. Completes the phase-rs#5155 D10 closure at the parser coverage layer (the engine core closed it at the runtime-gate layer). Assisted-by: ClaudeCode:claude-opus-4.8
rykerwilliams
pushed a commit
to rykerwilliams/phase
that referenced
this pull request
Jul 6, 2026
…phase-rs#5171) * feat(engine): rules-correct morph/disguise face-down spell casting (CR 708.4) Cast a Morph/Megamorph/Disguise card from hand face down as a blank 2/2 creature spell for a fixed {3} (CR 702.37c / 702.168a), placed on the stack (CR 708.4) and resolving to a face-down permanent (CR 702.37c). Closes the phase-rs#5155 D10 deferral: Tin Street Gossip's FaceDownSpell restricted-mana leaf now goes live with no type change, unlocking the full 228-card morph/megamorph/disguise class. Cast path (opt-in hand alt-cast idiom, mirroring Evoke/Blitz): - New CastingVariant::FaceDown + AlternativeCastKeyword::FaceDown; the offer in handle_cast_spell surfaces AlternativeCastChoice{FaceDown} for any card with Morph/Megamorph/Disguise when the fixed {3} is affordable. - continue_cast_face_down turns the object face down (blank 2/2, real card stashed in back_face) via the shared apply_face_down_entry_profile BEFORE the stack (CR 708.4). Visibility redaction, resolution to a face-down permanent, CR 708.9 counter-reveal, and turn-face-up (CR 702.37e) are all inherited from existing machinery. - The {3} is injected as a synthetic constant into the alt-cost .or() chain, never read off the blanked object (whose mana cost is now NoCost). Payment (closes D10): - SpellMeta.is_face_down is derived from the blanked object (face_down && back_face.is_some()) as a single authority in build_spell_meta, with no caller threading. A foretold card (face_down, back_face = None) stays face up, preserving the existing foretell guard test. The OnlyForFaceDownSpell mana gate now goes live. Disguise's face-down 2/2 carries ward {2} (CR 702.168a) via the cloaked_2_2 profile; morph/megamorph use vanilla_2_2. The unmorph/turn-up cost is read downstream from the hidden card's keyword, so CastingVariant::FaceDown is parameterless. Tests (engine_tests.rs): 8 discriminating tests including the D10 closure (Tin Street Gossip's OnlyForAny([FaceDownSpell, TurnFaceUp]) restricted mana funds the {3}), opponent redaction of the stack spell, disguise ward {2}, and the CR 708.9 counter-reveal. Assisted-by: ClaudeCode:claude-opus-4.8 * test(ai): morph face-down cast is enumerated and sanely scored by the AI Slice E of the morph/disguise face-down casting PR. Test-only — the AI already handles the new `AlternativeCastChoice { keyword: FaceDown }` action via the keyword-agnostic candidate-enumeration arm in `engine::ai_support::candidate_actions` (candidates.rs), consumed by the phase-ai search through `build_decision_context`. No production AI code changes; no DeckFeature/policy needed (measure-first showed the action is already enumerated + scored). Two discriminating tests: - (i) both the face-down (Alternative) and printed (Normal) casts are enumerated candidates — the face-down cast is not declined by omission. - (ii) the eval credits the resulting permanent AS a 2/2 (CR 708.2): for a strong real 5/5, the normal cast must OUTSCORE the face-down cast. Distinct + ordered finite non-sentinel scores prove the eval distinguishes the two boards. cargo ai-gate is null by construction for this change (the gate suite has no morph creatures, so no candidate set changes); a direct discriminating scoring test is the meaningful evidence. Assisted-by: ClaudeCode:claude-opus-4.8 * feat(client): surface morph/disguise face-down cast in the alternative-cost modal Slice F of the morph/disguise face-down casting PR. The engine already emits `WaitingFor::AlternativeCastChoice { keyword: FaceDown }` for a Morph/Megamorph/ Disguise card in hand; this adds the display layer only — no game logic, the engine owns the {3} cost and the blank 2/2 body (CR 708.4 / 702.37c / 702.168a). - adapter/types.ts: add `{ type: "FaceDown" }` to the AlternativeCastChoice keyword union (keeps the boundary-guardrails Rust↔TS lockstep test green). - AlternativeCostModal.tsx: add the `case "FaceDown"` display-copy arm — mirrors the existing per-keyword pattern; the `assertNever` fallback enforces exhaustiveness. Costs render from the engine-provided fields. - i18n: `alternativeCost.faceDown*` keys added to all 7 locales (de/en/es/fr/it/pl/pt) — satisfies the resources.test.ts parity gate. Also folds the now-stale D10 comments in tests/restricted_mana_face_down_and_face_up.rs: the OnlyForFaceDownSpell gate was HONEST-DEFERRED (no production path set is_face_down=true); it went LIVE with the face-down cast (build_spell_meta derives is_face_down = face_down && back_face.is_some()). Comment-only — assertions unchanged; end-to-end coverage is engine_tests::tin_street_gossip_restricted_mana_funds_face_down_cast. Assisted-by: ClaudeCode:claude-opus-4.8 * refactor(engine): FaceDownSpell mana coverage-supported now that face-down casting exists Rebase-adaptation onto phase-rs#5165 ("keep Tin Street coverage honest"), which classified `ManaSpendRestriction::FaceDownSpell` as not coverage-supported with the explicit rationale that it "must stay red until face-down spell casting exists." This PR's engine core makes it exist: `build_spell_meta` now sets `is_face_down = true` at a `PaymentContext::Spell` site for a morph/megamorph/disguise face-down cast, so the `FaceDownSpell` leaf is production-live. - `is_coverage_supported(FaceDownSpell)`: false -> true. The sole production caller is the parser's `Effect::Mana` absorption seam (`oracle_effect/sequence.rs`) — a parse-time coverage-classification decision, NOT runtime payability (that stays on the separate `ManaRestriction::allows_spell`). So this is a coverage-honesty change with zero game-behavior change beyond the card becoming supported. - Tin Street Gossip's `Any([FaceDownSpell, TurnPermanentFaceUp])` now absorbs at the seam (coverage red -> green) instead of leaving `Effect::Unimplemented`. - Non-vacuity: `FaceDownSpell` was the last hardcoded-false leaf. The classifier test is restructured (not just flipped) — renamed to `is_coverage_supported_all_leaves_supported_empty_any_is_false`; every leaf is now asserted supported, with `Any(vec![]) == false` kept as the remaining non-vacuous false exemplar. Reverting `FaceDownSpell => true` flips its positive assertion and Tin Street's mixed-`Any` assertion back to red. - `tin_street_gossip_..._stays_coverage_red` -> `..._is_coverage_supported` (asserts the absorbed restriction + no `Unimplemented` residual). - Refreshes the now-stale "dead today / red until face-down casting exists" docs in ability.rs, mana.rs, sequence.rs, and the restricted-mana test. CR 708.4 / CR 702.37c / CR 106.6. Completes the phase-rs#5155 D10 closure at the parser coverage layer (the engine core closed it at the runtime-gate layer). Assisted-by: ClaudeCode:claude-opus-4.8 * fix(engine): offer face-down cast from every castable zone (CR 702.37c/702.168b) CR 702.37c / CR 702.168b: "You can use a morph/disguise ability to cast a card from any zone from which you could normally cast it." The {3} face-down offer was gated on obj.zone == Zone::Hand, so a morph card castable from the graveyard/exile/command/top-of-library was never offered the face-down cast. Route the offer through the general castable-zone authority (prepare_spell_cast(..).is_ok()) instead of the hand-only special case; the other cast-offer blocks (Awaken/Impending/ Prototype) stay hand-gated because only morph/disguise carry the cross-zone rule. Assisted-by: ClaudeCode:claude-opus-4.8 * fix(engine): roll back a canceled face-down cast (CR 601.2i / CR 708.4) continue_cast_face_down blanks the object to its face-down 2/2 (stashing the real card in back_face) BEFORE payment, but handle_cancel_cast only restored alternate spell faces / prototype / prepared-copy — not a face-down cast. A CancelCast from WaitingFor::ManaPayment (or any pre- finalize step) therefore left the card blanked, nameless, and no-cost in its origin zone. Add a CastingVariant::FaceDown branch that routes through the existing single-authority restore_face_down_cast_object (the same rollback used on the prep-failure error path), reversing the entire cast per CR 601.2i. Assisted-by: ClaudeCode:claude-opus-4.8 * fix(casting): free normal cast not robbed by face-down auto-route (Blocker C) A pending `WithoutPayingManaCost` next-spell modifier (Omniscience-style one-shot) makes the FREE face-up normal cast of a morph/megamorph/disguise card legal even when its printed cost is unpayable. `normal_cast_choice_cost_ and_affordability` only recognized the `Unlimited CastFromHandFree` (Omniscience static) short-circuit, so a pending one-shot modifier was missed: the normal path reported the unpayable printed cost, `normal_affordable` was false, and the face-down offer auto-routed to the {3} face-down cast — robbing the player of the legal free face-up cast. Mirror the real prep authority (prepare_spell_cast_with_variant_override_inner via `next_spell_without_paying`, CR 118.9a): when a `WithoutPayingManaCost` modifier matches the spell, treat the normal path as `ManaCost::NoCost` and affordable so the engine OFFERS free-normal vs {3}-face-down instead of auto-routing. CR 601.2f + CR 118.9a (verified against docs/MagicCompRules.txt). Test `morph_free_normal_cast_not_robbed_by_face_down_autoroute` is discriminating: disabling the short-circuit auto-routes face down (`waiting_for` becomes Priority, not AlternativeCastChoice) → the offer assertion's `other => panic!` fires (measured). Assisted-by: ClaudeCode:claude-opus-4.8 * fix(casting): castable NoCost morph/disguise face down for {3} (Blocker A) A card with no mana cost (`ManaCost::NoCost`) that carries an effective morph/megamorph/disguise keyword is unpayable face up, but IS legally castable FACE DOWN for the fixed {3} alternative cost (CR 118.6a: an alternative cost may be applied to an unpayable cost). Three sites wrongly treated such a card as uncastable: - The `Zone::Hand` NoCost rejection in `handle_cast_spell_with_payment_mode` returned an error before the face-down offer block could run. - Its candidate/legal-action twin `can_cast_prepared_now_with_probe` returned false, so the cast was never surfaced as a legal action even though dispatch would have accepted it. - A soundness trap in `normal_cast_choice_cost_and_affordability`: `can_pay_cost_after_auto_tap` returns true for `NoCost` unconditionally, so the offer reported the unpayable face-up cast as affordable and offered a bogus free face-up cast instead of auto-routing to the {3} face-down. A cost reduced to nothing is `{0}` (CR 601.2f), distinct from `NoCost`, so the new `!matches!(normal_cost, NoCost)` guard never misfires on a cost-reduced-to-zero card. Both hand-gate exceptions and the legal-action twin are narrowed to the morph class AND {3} affordability via a shared `object_has_effective_face_down_keyword` helper (extracted from the inline array at the morph offer block) plus `can_afford_face_down_cast`, so the unpayable-cost rejection still stands for every non-morph NoCost card and for a morph without {3}. CR 118.6 / 118.6a / 601.2f / 702.37c / 702.37b (megamorph) / 702.168b (disguise), all verified against docs/MagicCompRules.txt. Tests (all measured discriminating via temporary edit-revert): - nocost_morph_auto_casts_face_down_for_three: remove E2 → rejected (.expect panics); remove the normal_affordable guard → offers not auto-routes (obj stays Hand). - nocost_non_morph_creature_rejected_from_hand / nocost_morph_without_three_ mana_rejected: keyword + {3}-affordability gating (both stay Err). - nocost_morph_offered_as_legal_action: remove E4 → can_cast_object_now false. Assisted-by: ClaudeCode:claude-opus-4.8 * fix(casting): evaluate face-down cast prohibitions against the blanked profile (CR 708.4) The morph/megamorph/disguise face-down cast castability gate evaluated cast prohibitions against the un-blanked, printed object. A characteristics-dependent prohibition (StaticMode::CantBeCast with a name- or mana-value-conditional filter — e.g. Meddling Mage / Nevermore naming the card) rejected prepare_spell_cast on the printed face, suppressing the legal {3} face-down cast at both the dispatch offer gate and the can_cast_object_now feasibility path (dropping it from legal actions and AI feasibility as well). Per CR 708.2a a face-down spell is a nameless, mana-cost-less 2/2, and per CR 708.4 prohibitions apply to those face-down characteristics, not the face-up object's; CR 601.3a lets a player ignore a qualities-conditional prohibition when a proposal choice (casting face down) changes the qualities it reads. New face_down_cast_is_permitted helper clones the state, blanks the object via the existing face_down_cast_profile + apply_face_down_entry_profile infra (mirroring can_afford_face_down_cast and the real-cast path continue_cast_face_down), then runs prepare_spell_cast_with_variant_override(Some(FaceDown)).is_ok() against the blanked profile. Wired into both the dispatch offer gate and the feasibility twin, so the whole class of characteristics-dependent prohibitions (name, mana value, color, subtype/type) is covered — blanking zeroes all of them. Assisted-by: ClaudeCode:claude-opus-4.8
1 task
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
2 participants
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.
🤖 AI text below 🤖
Summary
S25 Standard-set tranche, part a (s25a). Adds engine + parser support for 33 cards across Waves 1–4 of the S25 coverage push, each landed through the full
/engine-implementerpipeline (plan → review-plan → implement → review-impl → commit), one card at a time. The 7 heaviest Wave-5 subsystem cards are split out to a follow-up s25b (see "Deferred to s25b" below). Branch rebased onto currentupstream/main(6984869); a singlerebase-adaptation-2commit reconciles the drifts main's advance introduced (documented in that commit), and afix(ci)commit carries the post-open CI remediation (see "CI remediation" below).Each card is a building block, not a one-off: interactive counter-removal (Rhys), dynamic base P/T tokens (Skullspore Nexus), cross-player revealed-card mana-value loss (Parker Luck), transforming-Saga token+lore (Esper Terra), event-context mana-color binding (Foraging Wickermaw), cast-from-graveyard type-grant riders (Tomb of Aclazotz), grant-abilities-beyond-activated (Symbiote Spider-Man / Choreographed Sparks / Nalfeshnee), cloak-from-chosen-source (Vannifar), dual-target anaphoric slots (Stolen Uniform), and more. Full per-card rationale + CR annotations live in the individual commit messages.
Implementation method (required)
/engine-implementerpipeline (plan → review-plan → implement → review-impl → commit)Per card, strictly serial (one sub-agent at a time). The one non-card commit is
rebase-adaptation-2(a ship-prep rebase reconciliation, see its message).Files changed
crates/engine/src/{types,parser,game,analysis}/…— new effect variants, parser combinators, resolvers, targeting, dual-walk (feat(engine): PR-6.75 — C0-full+C1 read/write conflict profiler; sound CR 603.3b same-event trigger ordering #5072) classification arms.crates/engine/tests/…— one discriminating test file per card (revert-to-red verified).crates/manabrew-compat,crates/mtgish-import,crates/phase-ai— exhaustive-match / consumer threading for the new engine surfaces.CR references
Union of the CR annotations added across the 33 commits (each grep-verified vs
docs/MagicCompRules.txtat commit time). Notable: CR 107.1c, 108.3, 115.1, 119.3, 122.1, 201.5a, 202.3, 205.1b, 208.4b, 400.7, 513.2, 514.3a, 601.2c, 603.4, 604.3, 608.2c/d, 611.2a/c, 613.1d, 701.4a, 701.20b, 707.2, 708.2a, 714.x, 723.2. See per-commitCRannotations for the exact rule→code mapping.Track
Developer
LLM
Model: claude-opus-4-8
Thinking: high–max (planners/drivers at xhigh; implementers/reviewers at high)
Verification
Measured on the rebased branch (worktree, cargo direct — this branch is not Tilt-watched). Full suite + clippy measured at the 7f2ba41 base; the final trivial rebase to 6984869 (+2 upstream commits, no enum drift, zero conflict stops, range-diff 35/35 identical) re-ran the fast gates, with CI as the final backstop:
cargo fmt --all— cleancargo check --workspace— EXIT 0 (re-run on 6984869)cargo clippy --workspace --exclude phase-tauri --all-targets --features engine/proptest -- -D warnings— EXIT 0./scripts/check-parser-combinators.sh 6984869ba— EXIT 0./scripts/check-engine-authorities.sh 6984869ba— EXIT 0cargo test -p engine— 15308 lib + 1773 + integration, 0 failed across 196 test blocksFORGE_TEST_FULL_DB=1 cargo test -p engine ordering_parity_sweep—full_db=true swept=13555 compared=6210 unexplained=0, 1 pass / 0 fail (no trigger-ordering-parity divergences;se_member_bound_class=43)cargo coverage—supported_cards=31374 / total 35397(88.63%) on the rebased branch. Every S25 tranche card landssupported=true/gap_count=0(spot-checked: Parker Luck, The Skullspore Nexus, Esper Terra, Rhys, Foraging Wickermaw, The Tomb of Aclazotz, Crowd-Control Warden, Memory Vessel, Stolen Uniform, Secret of Bloodbending, Bumi Unleashed, Symbiote Spider-Man, Choreographed Sparks, Brilliance Unleashed, Vraska the Silencer, Vannifar, Expose the Culprit, Overgrown Zealot, Tin Street Gossip). Pinned negatives hold: Keen Duelist stayssupported=false(D9 — B3 honest-red gate), Cerebral Eruptionsupported=true/gap=0(D8 — baseline unchanged; Parker Luck narrowed to avoid touching it). CI's coverage-regression check confirms the exact GAINED set +REGRESSED=0vs the PR base.git range-diffvs the pre-rebase stack: 33 tranche commits map 1:1 (4 benign!= main's Tin-Street test-rename absorption, thebecome_blockedmodule-ordering absorption, and the two manually-resolved Rhys/Esper conflicts) + the one adaptation commit.Scope Expansion
This tranche adds substantial reusable engine infrastructure (new
Effect/ObjectScope/QuantityRefvariants, a dual-target slot registry, interactive counter-removalWaitingFor, grant-ability scoping, cast-from-graveyard enters-riders, and more) — it is a coverage push, so new building blocks are expected. Maintainer review is invited.Validation Failures
None.
CI Failures
First CI run failed two gates; both remediated in the
fix(ci)commit (everything else was green, including both test shards, card-data/coverage, and the paired-seed AI gate):#[cfg(test)]assertions on objects' own keyword stores, 2 are copy-construction dedupe on a detached stack-copy snapshot (CR 707.10f) — annotatedallow-raw-authority, no game-logic conversion needed. Gate now EXIT 0 locally.restriction_static_mode_gate_scans(and after rebase,mana_aura_trigger_scans) shifted past baseline: a card-data-driven trajectory shift from the 33 newly-supported cards (CI's own diagnostic concurs; paired-seed AI gate green = no behavior regression). Baseline refreshed viascripts/refresh-ai-perf-baseline.shon the final base — no hand-tuned limits.Deferred to s25b (Wave 5 — 7 Group-C subsystem cards)
Split out under the token budget; no impl/planning done for s25a.
/review-engine-planbefore code.CounterAllfor abilities)PlayerCount/ObjectCountBySharedQuality) + InvestigateBecomeCopy→ mass dynamic-set + duration-bound (CR 707 + 611.2c)Bumi, Unleashed — counted done-by-absorption (no commit)
Bumi, Unleashed is supported=true / gap_count=0 at HEAD, done-by-absorption from main commit b0f32ea ("fix(parser): … Last Night Together (#4562)") — the class-level "Only X can attack during that combat phase" restriction. Runtime-verified two-sided (land can attack / non-land cannot / control re-admit). Attributed to #4562; NOT a tranche win, counts toward 33 without a commit.
Deferrals surfaced during s25a (all latent or out-of-scope; none regress main)
TargetFilter::Playerto avoid exposing it; affects Cerebral Eruption + 6 typed-opponent reveal cards; measured repro in the tranche deferral ledger).🤖 Generated with Claude Code
https://claude.ai/code/session_01CwQE5oyMqZ9T4BPMsih3Kj