feat(engine): S25b standard tranche — 7 commits, 12 cards + FIN Tiered 7/7#5534
Merged
Conversation
…costs
Rewrite parse_meld_gate to handle two meld-instigator grammar axes it
previously rejected, and protect meld result names during self-reference
normalization. A measured class fix: Vanille, Cheerful l'Cie and Titania,
Voice of Gaea flip supported=false->true, and Urza, Lord Protector's
corrupted result string is repaired.
- Leading intervening-if condition ("if there are four or more land cards
in your graveyard and you both own and control ...") is parsed via
parse_inner_condition and prepended to the trigger's And (CR 603.4).
Unlocks Titania.
- Optional-cost reflexive form ("... you may pay {C}. If you do, exile
them, then meld them into R"): delete the period-guard that deferred it
and reshape the gate residual so the existing reflexive PayCost /
OptionalEffectPerformed machinery gates the meld (CR 118.12). Unlocks
Vanille. Declining the payment must not meld.
- Guarded per-clause meld dispatch (parser/oracle_effect/mod.rs): the
reflexive meld sub-clause is a single chunk ("meld" is in no verb table)
that bypassed the pre-chunk meld dispatch; route it through the sole
authority parse_meld_effect_clause (CR 701.42a). pending_meld_partner is
threaded into chunk_ctx.
- Meld-result name mask (parser/oracle_util.rs): the name after "meld them
into" is a distinct object (CR 201.2a), not a shortened self-reference
(CR 201.5c). Mask it during normalization so a result sharing a token
with the card's comma-short name is not folded to "~, ..." -- fixes the
measured live corruption (Titania -> "~, Gaea Incarnate", Urza ->
"~, Planeswalker") that made perform_meld miss the card_face registry
and no-op. Class-level: protects every meld result uniformly.
The meld Effect, resolver, targeting, and multiplayer filter are unchanged;
this is a parser-only extension with no new engine variant.
Tests: a runtime accept/decline discrimination test (pay => meld, decline
=> no meld, CR 118.12) driving the production resolve/act path; parse
round-trips asserting the meld is the OptionalEffectPerformed-gated child
of the PayCost; Titania's 3-conjunct And with the >=4-land threshold; a
mask unit test; and regression guards keeping the 5 clean meld cards
(Urza, Mightstone, Argoth, Gisela, Bruna) supported. Also fixes a
pre-existing vacuous find_meld helper in database/meld_tests.rs that only
inspected top-level effects (it silently passed a stale deferral test).
Assisted-by: ClaudeCode:claude-opus-4.8
…ent (Moonlit Meditation)
Implement Moonlit Meditation: "The first time you would create one or more
tokens each turn, you may instead create that many tokens that are copies of
enchanted permanent." (supported=false->true). A class extension of the
existing token-creation replacement subsystem, not a new subsystem.
- New ReplacementCondition::FirstTokenCreationEachTurn { player: ControllerRef }
-- a per-source, from-existence once-per-turn gate. Per CR 614.4 (a
replacement can't act on an event that occurred before it existed), this
tracks whether THIS source's replacement has applied this turn, not a global
"any token this turn" latch: a Moonlit entering mid-turn after an earlier
creation still fires on the next creation. Reuses the per-ObjectId
once-per-turn pattern of crew_activated_this_turn via a new
GameState.first_token_replacement_used_this_turn set, consumed at mark_applied
(on accept AND decline -- declining still spends "the first time"), cleared at
turn start.
- The substitute is Effect::CopyTokenOf { target: AttachedTo } (the Springheart
Nantuko building block): copies of the Aura's enchanted host, count = "that
many" via QuantityRef::EventContextAmount.
- Owner scope via token_owner_scope(You) (CR 111.2 / 109.5), NOT valid_card -- a
CreateToken event has no affected_object_id, so a valid_card gate is
unsatisfiable and would make the replacement never fire.
- Self-suppression: the CopyTokenOf continuation re-enters the replacement
pipeline (the phase-rs#1511 doubler-visibility path); it now inherits the
post_replacement_token_choice_applied set carrying its own ReplacementId, so
Moonlit self-suppresses on its own copies while a doubler (different id) still
doubles them.
- Count provenance: a transient GameState.post_replacement_token_substitution_count
stamped at the copy-substitution seed gate and read first in the
EventContextAmount cascade -- the accept path zeros the original count, and
EventContextAmount could otherwise be shadowed by an unrelated trigger-event
amount. Both transient continuation seeds are also scrubbed at turn start.
CR 614.1a / 614.4 / 614.6 / 616.1 / 111.2 / 109.5, all grep-verified.
Tests (crates/engine/tests/integration/std_longtail_e.rs): accept creates
copies of the host; opponent-owned creation is ignored (owner scope); a Moonlit
entering mid-turn fires on the next creation (per-source, not global); "that
many" = N (not 0, not shadowed); decline consumes the turn window; latch resets
next turn; two Moonlits are independent; Doubling Season yields exactly two
host-copies with no recursion (phase-rs#1511 doubler + self-suppression); parse
round-trip; and a turn-start transient-seed scrub guard. Jinnie Fay / Divine
Visitation / doublers regression-verified unchanged.
Assisted-by: ClaudeCode:claude-opus-4.8
Implement Niko, Light of Hope's middle clause "Shards you control become
copies of it until the next end step" (supported=false->true). A class
extension of Effect::BecomeCopy, not a new subsystem -- the same change also
flips Absorb Identity and Deceiver of Form (each "[filter] you control become
copies of ..."), a build-for-the-class win.
- Parameterize Effect::BecomeCopy with recipient: TargetFilter (default
SelfRef, serde-skipped when SelfRef so every existing single-subject card is
byte-identical), mirroring GainActivatedAbilitiesOfTarget.recipient. SelfRef
= the source ~ (all prior cards, exact prior behavior). A typed group filter
or ParentTarget selects a mass recipient set, resolved to concrete object ids
at resolution and locked (CR 611.2c) -- one transient continuous effect per
id keyed SpecificObject{id}, mirroring the gain-activated-abilities resolver.
- New PlayerScope::AnyTurn: a turn-AGNOSTIC end-step deadline for the definite
article "the next end step" (the next end step to begin, whoever's turn),
distinct from Controller for the possessive "your next end step"
(Rocco/Street Chef). Mirrors the delayed-trigger split AtNextPhase (agnostic)
vs AtNextPhaseForPlayer (scoped). Niko's copy expiry co-fires with the
return's agnostic AtNextPhase{End} on the same PhaseChanged{End} -- correct
even for instant-speed activation on an opponent's turn (the ability has no
sorcery restriction, CR 602.2), where a controller-scoped duration would
wrongly persist the copies a full rotation past the return.
- Extend the existing prune_until_next_end_step_effects additively with one
ungated AnyTurn case; the {End, Controller} controller-scoped case is
textually unchanged, so Rocco/Street Chef stay byte-behavior-identical.
- Donor "it" = the exiled creature (ParentTarget); copiable values are read
from exile (CR 707.2, no zone gate -- ObjectId is preserved across exile) and
snapshotted once into the copy modification (CR 707.2b).
- RW ordering profile: the mass path writes the recipient scope and reads the
donor (CR 707.2 / 611.2c / 603.3b); the SelfRef path keeps the exact prior
write-scope, so the existing copy corpus is byte-identical and FORGE
ordering-parity delta is 0.
CR 110.5d / 513.1 / 603.7b / 611.2a / 611.2b / 611.2c / 707.2, all grep-verified.
Tests (crates/engine/tests/integration/niko_light_of_hope.rs): parser
round-trip (middle clause lowers to BecomeCopy{recipient=Typed(Shard,You),
target=ParentTarget, dur=UntilNextStepOf{End,AnyTurn}}, chained between exile
and return, zero Unimplemented); locked recipient set (a Shard created after
resolution is NOT copied, CR 611.2c); copies are of the exiled donor not Niko
(a 5/5 donor != Niko's 3/4); opponent-turn co-fire (Shards revert AND the
creature returns at the opponent's end step, not a turn later -- fails under
Controller scope) with a your-turn reach-guard; recipient is not a target slot;
single-subject byte-identity; the class covers Absorb Identity + Deceiver of
Form; and a Gate-1 profile test that AnyTurn walks the RW duration path without
panicking. Revert-probes: AnyTurn->Controller fails the opponent-turn test
while the your-turn guard passes; recipient->SelfRef fails 6 of 7 core tests.
ponytail: mass become-copy with enter-as-copy counter exceptions ("except they
enter with a +1/+1 counter") is not a printed card class; those resolution-time
mods stay scoped to the single-subject path (become_copy.rs:175), a documented
ceiling until such a card appears.
Assisted-by: ClaudeCode:claude-opus-4.8
…ative (No Witnesses)
Implement No Witnesses' first clause "Each player who controls the most
creatures investigates" (supported=false->true). A class-wide parser fix, not a
card special-case: the same superlative arm also flips Tectonic Hellion ("each
player who controls the most lands sacrifices two lands of their choice").
- One arm in parse_controls_permanent_object (oracle_effect/lower.rs) recognizes
"who controls the most <type>" and lowers to ControlsCount{ relation: All,
filter: <bare Typed>, comparator: GE, count: Ref(ControlledByEachPlayer{ <bare
Typed>, Max }) } -- reusing the existing cross-player extremum (Balance's
building block) composed into the existing player-scope. "investigates"
deconjugates to the already-wired Effect::Investigate; the "Then destroy all
creatures" sibling detaches as the once-run UNCONDITIONAL tail (the wrath is
unscoped -- verified structurally and at runtime).
- GE vs the global Max is equivalent to EQ (CR 107.1 integers: no player's count
exceeds the max) and selects exactly the tied-for-most set; all-equal
(including all-at-0) means everyone, matching the Tectonic Hellion "if everyone
controls the same number of lands, everyone sacrifices two lands" ruling.
- The filter stays BARE (controller-less) on both the extremum and the
player-scope: the resolvers (player_control_count_compares,
ControlledByEachPlayer) apply the per-player controller gate themselves, so a
.controller(You) here would be redundant. Honest-red guard: a type phrase that
fails to parse stays Unimplemented rather than building a bogus extremum.
- No new engine variant; reuses ControlledByEachPlayer, ControlsCount,
Effect::Investigate, DestroyAll. Parser-only -- no engine/AI/frontend change.
CR 107.1 / 109.4 / 109.5 / 111.10f / 701.16a, all grep-verified.
Tests (crates/engine/tests/integration/no_witnesses_most_creatures_investigate.rs):
parser round-trip (clause 1 -> ControlsCount{...ControlledByEachPlayer{Max}} +
Effect::Investigate, zero Unimplemented); 2p single-most (only the most-creatures
player gets a Clue); 3p distinct (strict max only); 3p tie (both tied players
investigate); all-at-0 (everyone investigates); the wrath destroys a
non-investigator's creatures unconditionally; and the Tectonic Hellion class
sibling (-> Effect::Sacrifice). Revert-probes prove non-vacuity: neutering the
arm fails all 7 tests; Max->Min mis-selects (6 fail); GE->GT excludes the max
holder (7 fail).
REGRESSED=0: the arm is provably inert on any card lacking "who control(s) the
most " -- a bounded before/after parse over the corpus cards containing that
phrase shows only No Witnesses + Tectonic Hellion change; the rest are
byte-identical.
Assisted-by: ClaudeCode:claude-opus-4.8
…des (Vincent's Limit Break)
Implement Vincent's Limit Break's Tiered mechanic (supported=false->true),
completing the FIN Tiered class (7/7 cards). Fixes TWO gaps in one arm: the
three tier option lines were Unimplemented, and the main effect parsed green but
SILENTLY DROPPED "and has the chosen base power and toughness" (no base-P/T set).
- Root cause (3 Unimplemented): Vincent's has a shared effect line between the
"Tiered" keyword and the parameter-only bullets, so parse_oracle_block saw the
effect line (not a bullet) at start+1, collected zero modes, and returned None
-- the existing Tiered arm never fired and the bullets fell through to
standalone Unimplemented. Root cause (silent drop): "the chosen base power and
toughness" has no N/M, so parse_base_pt_mod returned None and the
SetPower/SetToughness were dropped (the dies-return GrantTrigger, from the
quoted segment, is a separate additive push and survived).
- Fix (parser-only, oracle_modal.rs): a shared-effect distributor
(parse_tiered_shared_effect_block + parse_shared_chosen_pt_template) that
substitutes each tier's literal P/T for the "the chosen base power and
toughness" anaphor and reuses the existing modal-additional-cost machinery --
mirroring the established distribute_shared_mode_effect / parse_shared_those_template
sibling. Each mode lowers through the SAME path that produced the main effect,
now emitting SetPower/SetToughness alongside the dies-return GrantTrigger under
one UntilEndOfTurn GenericEffect. The chosen tier's additional cost
({0}/{1}/{3}) is charged via the existing compute_modal_total_cost single
authority.
- The base P/T is a CR 613.4b layer-7b SET (Layer::SetPT, ordered before the 7c
counter layer): a +1/+1 counter applies on top at 7c (3/2 -> 4/3), matching
the ruling ("overwrites prior P/T-setting effects; counters still apply"). The
mode choice is bound at cast and immutable (CR 700.2a).
- No new engine variant, keyword, effect handler, or frontend; reuses SetPower /
SetToughness, OracleBlockAst::Modal, ModalChoice, GrantTrigger, and
compute_modal_total_cost. A one-line pub(crate) re-export exposes the existing
parse_pt_mod building block (grammar is a private module).
CR 702.183a / 700.2 / 601.2f / 613.4b / 613.4c, all grep-verified.
Tests: parser round-trip (Vincent's -> modal{3 modes, costs {0}/{1}/{3}} + each
mode carries SetPower/SetToughness AND the dies-return GrantTrigger, zero
Unimplemented; Restoration Magic, a standard Tiered card, unchanged -- the arm is
anaphor-gated); runtime cast via the real pipeline (Galian Beast {0} -> target
base P/T 3/2; Hellmasker {3} -> 7/2 AND total {4}{B} spent, proving the {3}
additional cost is actually charged; a +1/+1 counter -> 4/3, proving the
7b-before-7c layer order; only {1}{B} available -> Hellmasker unaffordable,
proving the additional cost is a real castability gate). Revert-probes
(empirically confirmed): disabling the arm makes the modal fail to form and
every test fail; skipping the additional cost fails the mana-spent assertion;
Max/wrong-tier binding fails the P/T assertions.
REGRESSED=0: the arm fires only for a Tiered header + the "the chosen base power
and toughness" anaphor (a single corpus card) + all-P/T bullets, returning None
(the identical pre-existing path) for every other card.
Assisted-by: ClaudeCode:claude-opus-4.8
…Takedown + Friendly Rivalry)
Implement the compound-source variant of "N target creatures each deal damage
equal to their power to a target" (supported=false->true for TWO cards). The
honest-deferral gate `is_compound_source_each_power_damage` covered a class of
two -- deleting it un-defers BOTH Graceful Takedown and Friendly Rivalry, which
would otherwise have shipped wrong-but-green (their second source group silently
dropped).
- Graceful Takedown ({1}{G} Sorcery): "Any number of target enchanted creatures
you control and up to one other target creature you control each deal damage
equal to their power to target creature you don't control."
- Friendly Rivalry ({R}{G} Instant): "Target creature you control and up to one
other target legendary creature you control each deal damage equal to their
power to target creature you don't control."
- Parameterize the existing `Effect::EachDealsDamageEqualToPower` with an
optional second source group `extra_source: Option<TargetFilter>`
(#[serde(default, skip_serializing_if="Option::is_none")] -> the ~24 existing
family cards, e.g. Band Together, are byte-identical). The source-count-agnostic
resolver `resolve_each_deals_equal_to_power` is reused VERBATIM -- the A union B
chosen sources arrive as more object targets in the flat `[source.., recipient]`
vector, each dealing its own power (CR 120.1 / 608.2h). No resolver change, no
new effect/variant/state-machine.
- Parser (nom): an "any number of" count arm (-> unlimited(0)) and a count-1
"target" arm (-> fixed(1,1)) for the two group-A cardinalities; a new
`parse_extra_other_source` combinator for "and up to one other target <filter>
you control" that REQUIRES the `Another` marker (CR 115.4) so a "one target"
without "other" is not mis-parsed. Filters compose existing props: EnchantedBy
(Graceful group A, the attached-Aura predicate CR 303.4), HasSupertype Legendary
(Friendly Rivalry group B, CR 205.4a), Another + You (both group Bs), Opponent
(both recipients, CR 109.5).
- Targeting (both slot builders): the group-B slot/spec is inserted BETWEEN the
group-A source slots and the recipient (recipient stays LAST, so the resolver's
`split_last` treats group B as a source); it gets its own target instance so
the `Another` distinctness (CR 115.4) excludes every group-A pick at selection
time. The RwProfile arm is field-agnostic and UNTOUCHED -> FORGE ordering-parity
delta = 0 for every card.
CR 115.1d / 115.3 / 115.4 / 205.4a / 303.4 / 601.2c / 109.5 / 120.1 / 208.1 /
608.2h, all grep-verified.
Tests: parser round-trips (both cards -> the compound structure with the exact
per-card filters; zero Unimplemented; the "other" marker AND the Legendary
supertype each non-vacuously asserted -- the Legendary assertion flips to red if
the supertype filter is dropped); runtime cast-pipeline (Graceful: two
aura-enchanted sources + one other each deal their own power, Sigma; group-B-only
when no enchanted sources; slot legality -- non-enchanted not a group-A source,
group B can't repick a group-A creature, recipient must be you-don't-control;
Friendly Rivalry: singular group A + legendary group B, and a non-legendary is
rejected as group B so Sigma = 1+2 = 3 not 1+4 = 5, discriminating the legendary
filter at runtime). Revert-probes: dropping the group-B slot changes Sigma;
reverting the parser leaves Unimplemented; the deleted gate's team-scope
fail-closed (Combo Attack, CR 810) is preserved.
REGRESSED=0: the deleted gate + the new arms are reachable only inside the
verb-tag-gated compound-source parse; a bounded card-data scan confirms EXACTLY
{Graceful Takedown, Friendly Rivalry} flip supported=false->true and every other
card is byte-identical. FORGE ordering_parity_sweep delta=0 (the family cards' RW
profiles are unchanged -- the field-agnostic RwProfile arm is untouched).
Assisted-by: ClaudeCode:claude-opus-4.8
…h (Glen Elendra's Answer)
Implement Glen Elendra's Answer's mass counter (supported=false->true) in two
parts: (1) fold "counter all spells your opponents control and all abilities your
opponents control" into ONE CounterAll over opponent-controlled stack objects;
(2) a class-level correctness fix so "for each spell and ability countered this
way" counts the countered ABILITIES, not just spells.
Card: {2}{U}{U} Instant -- "This spell can't be countered. Counter all spells
your opponents control and all abilities your opponents control. Create a 1/1
blue and black Faerie creature token with flying for each spell and ability
countered this way."
- Root cause: "counter all spells your opponents control" already parsed to
CounterAll{And[StackSpell, opponent]}, but "and all abilities your opponents
control" dropped to Unimplemented -- try_split_targeted_compound bisects the
compound before parse_counter_ast, and its all/each carry-forward is skipped
because extract_effect_verb registers no "counter" verb.
- Step 1 (parser, nom): a new handler try_parse_counter_all_conjunction
recognizes "counter all/each <A> and all/each <B> ..." and reparses each
conjunct through the existing parse_counter_ast, composing ONE
CounterAll{ Or[ And[StackSpell, Typed{opponent}], StackAbility{opponent} ] } --
the same shape becomes_target_source_filter builds and the stack matcher
enforces (spell entries route to the And[StackSpell,..] leg, ability entries to
the StackAbility leg). One merged counter (not two chained), so the "countered
this way" set is a single population. Reuses the existing StackAbility
ability-counter infra (Kadena's Silencer) + the source-count-agnostic
CounterAll resolver (UNCHANGED -- already counters abilities and emits
SpellCountered per object). Dispatched before try_split_targeted_compound;
returns None for single-conjunct text (Kadena / Summary Dismissal / Grimoire
Thief stay byte-identical).
- Step 2 (engine, class-level fix): affected_objects_from_events had no Counter
arm, so it collected ZoneChanged ids -- but a countered ABILITY leaves the
stack with no ZoneChanged (CR 405.1: an ability has no card and no zone), so it
was silently omitted from the count. A dedicated Counter/CounterAll arm reads
SpellCountered (emitted per countered object, spell or ability), fixing the
count for the whole counter-for-each family. The arm is EXCLUSIVE (reads only
SpellCountered, not also ZoneChanged), so a countered spell -- which emits both
-- contributes exactly one id (no double-count).
CR 113.9 (abilities on the stack are counterable only by effects that
specifically counter abilities) / 405.1 / 608.2c / 701.6a / 704.5e, all
grep-verified.
Tests: parser round-trip (Glen -> CounterAll{Or[And[StackSpell,opponent],
StackAbility{opponent}]} + the Token{TrackedSetSize} sub + the CantBeCountered
static; zero Unimplemented; Kadena / Summary Dismissal / Grimoire Thief
byte-identical). Runtime cast-pipeline: an opponent's spell AND activated ability
are both countered; the Faerie count = spells + abilities countered (the ability
is the discriminator -- revert the count fix and it drops 2->1); a
your-controlled ability is NOT countered; 2 opponent spells -> count 2 not 4
(locks the exclusive-arm no-double-count). Swift Silence -- the existing
counter-for-each card on the changed seam -- is byte-identical: N non-copy spells
-> draws N, and a countered copy + real spell -> draws 2 (copies emit their own
stack->graveyard ZoneChanged before the CR 704.5e cease-to-exist SBA -- the
move guard is is_token, not is_copy -- so copies were ALWAYS counted; this is a
correctness lock, not a Step-2 change). Test of Talents' exile-caused count is
unaffected (Counter members carry no this-way cause).
REGRESSED=0: the parser handler matches only "counter all/each ... and all/each
..." (Glen is the sole corpus match); the counter-for-each family whose tracked
set the Step-2 arm feeds is {Glen Elendra's Answer, Swift Silence} (the other
Counter+TrackedSetSize cards use the Counter as a terminal effect, so their
tracked set is never published for it). FORGE ordering_parity_sweep delta=0 (the
RW profile is unchanged; only the runtime count value changes, which
ordering-parity does not observe).
Assisted-by: ClaudeCode:claude-opus-4.8
…ipient + AppliedReplacementKey
Rebase onto upstream/main (+49) introduced three drift points the +49 commits added after the tranche was authored. Thread the Niko `recipient` field through two NEW upstream `Effect::BecomeCopy` construction sites (serde-default `SelfRef`, so single-subject copy semantics are byte-identical), qualify a `HashSet` path at a new call site (E0433), and migrate the Moonlit test's applied-set literal to upstream's `ReplacementId` -> `AppliedReplacementKey::object` refactor (E0308; object(source, index) is the semantic equivalent, self-suppression re-verified).
- engine_replacement.rs:1145 handle_copy_target_choice: BecomeCopy += recipient: SelfRef
- token_copy.rs:3671 test BecomeCopy += recipient: SelfRef; :560 applied: std::collections::HashSet::new()
- std_longtail_e.rs:1188 ReplacementId{source,index} -> AppliedReplacementKey::object(moonlit, 0)
Assisted-by: ClaudeCode:claude-opus-4.8
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
github-actions
Bot
added
the
needs-maintainer
matthewevans
removed
the
needs-maintainer
github-actions
Bot
added
the
needs-maintainer
Parse changes introduced by this PR · 14 card(s), 34 signature(s) (baseline: main
|
matthewevans
requested changes
Jul 11, 2026
matthewevans
left a comment
matthewevans
left a comment
Member
There was a problem hiding this comment.
Decomposed per-mechanic review — each of the 8 mechanics reviewed independently against the Comprehensive Rules and live Scryfall Oracle text. 7 of 8 are clean and independently landable; 1 is blocked on a genuine rules-correctness defect. Recommend splitting the blocked mechanic so the clean work lands now.
🔴 Blocker — Moonlit Meditation token-copy replacement
The "first time each turn" window is keyed to the enchantment's ObjectId (the per-source latch around replacement.rs:4466). That scope is wrong. The card reads "The first time you would create one or more tokens each turn…" — a per-player window — and the card's official ruling makes the failure mode explicit:
If you create one or more tokens, and then Moonlit Meditation comes under your control that same turn, the replacement effect won't apply to any tokens you create for the rest of the turn. You'll have to wait until the next turn.
With a per-source latch, a token created before Moonlit enters mid-turn never consumes the window, so the replacement fires when the ruling says it must not.
The engine already has the correct primitive: players_who_created_token_this_turn (set by record_token_created, reset at turn start, already consumed by ParsedCondition::YouCreatedTokenThisTurn). Gate on that per-player set instead of a new per-ObjectId field — it is both the rules-correct scope and the existing building block, so this resolves the rules bug and the reuse violation together.
Two tests currently enshrine the incorrect semantics and would make the correct fix look like a regression:
crates/engine/tests/integration/std_longtail_e.rs—moonlit_fires_from_existence_not_globally(B1)crates/engine/tests/integration/std_longtail_e.rs—moonlit_latch_is_per_object_id(B5)
Also: the CR 614.4 annotation at replacement.rs:4463 is misapplied — 614.4 forbids replacing an already-past event; it does not justify the window reopening when the source enters.
🟡 Non-blocking (fine to ride along with the clean mechanics)
PlayerScope::AnyTurn (types/ability.rs:4711) plants a whose-turn timing referent inside a player-selector enum, forcing two unreachable!() arms in quantity.rs. It's contained and fail-loud, but it conflates two semantic layers — the AtNextPhase vs AtNextPhaseForPlayer precedent kept these on separate variants. Worth tidying, not worth blocking.
✅ Clean mechanics (approve on split)
Vanille meld triggers (leading condition + optional cost), Niko mass become-copy (turn-agnostic end-step expiry), No Witnesses "controls the most" superlative (with the 3-player tie discriminator), Vincent's Limit Break Tiered P/T distribution, Graceful Takedown multi-source each-deals-power damage, Glen Elendra's Answer counter-all (spells and abilities), and the rebase-reconcile commit — all with byte-verified Oracle text, grep-verified CR annotations, and discriminating, registered tests.
Recommendation: split Moonlit Meditation into a follow-up PR — keep the AppliedReplacementKey migration plumbing on the landing side and carve out only the Moonlit-specific replacement condition, latch, and std_longtail_e.rs tests. With Moonlit removed, this tranche is good to land.
…not per-source
Moonlit Meditation's "The first time you would create one or more tokens each
turn, you may instead create that many tokens that are copies of enchanted
permanent" gated its token-copy replacement on a per-SOURCE latch
(`first_token_replacement_used_this_turn`, keyed by ObjectId). The Oracle "you"
and the official ruling make it a per-PLAYER window: "If you create one or more
tokens, and then Moonlit Meditation comes under your control that same turn, the
replacement effect won't apply to any tokens you create for the rest of the
turn." A token created before Moonlit enters mid-turn consumes the window; the
per-source latch missed this and wrongly fired.
Retarget the `FirstTokenCreationEachTurn` condition to the existing per-player
primitive `players_who_created_token_this_turn` (populated by
`record_token_created` on every token creation — the same set backing
`YouCreatedTokenThisTurn`). This closes the window on both accept (copy tokens)
and decline (original tokens) with no separate bookkeeping, so the per-source
field and its `note_first_token_replacement_applied` writer are now dead and are
removed. Copy re-entry stays suppressed by the applied-set check (CR 614.5), not
the latch. Fix the misapplied CR 614.4 annotation (temporal "can't go back in
time") to CR 614.1a ("instead" = replacement effect) + CR 614.5 (a replacement
doesn't invoke itself repeatedly).
Tests (crates/engine/tests/integration/std_longtail_e.rs): B1 and B5 rewritten as
the two switch-discriminators — a source entering mid-turn after an earlier
creation, and a second source under the same controller — both revert-fail under
the per-source latch. A new accept-path test guards that the window still closes
via record_token_created after removing the note fn. Accept/decline/turn-reset
field assertions retargeted to the per-player set.
Assisted-by: ClaudeCode:claude-opus-4.8
Member
|
Update — the Moonlit Meditation blocker from my prior review (#pullrequestreview-4675737531) is resolved on the new head; holding only for CI + rebase. ✅ Blocker resolved
🟡 Holding on
RecommendationOn settled green, update-branch + approve + enqueue as |
matthewevans
approved these changes
Jul 11, 2026
matthewevans
left a comment
matthewevans
left a comment
Member
There was a problem hiding this comment.
Approving — the Moonlit Meditation blocker from the decomposed review is resolved, and the 7 other mechanics were already cleared.
✅ Resolved
Commit 81312e7c scopes the token-copy window per-player exactly as recommended: the per-ObjectId first_token_replacement_used_this_turn latch is gone, replaced by the shared players_who_created_token_this_turn primitive (set by record_token_created, reset at turn start), the annotation is corrected (CR 614.4 → 614.1a), and the moonlit_* tests that had enshrined the per-source model are fixed. This now matches the card's official ruling that a token created before Moonlit enters mid-turn consumes the once-per-turn window.
✅ Prior decomposed review (unchanged mechanics)
Vanille meld, Niko mass become-copy, No Witnesses superlative, Vincent's Tiered P/T, Graceful Takedown multi-source damage, Glen Elendra counter-all, and the rebase-reconcile commit — all previously verified (Oracle text, grep-verified CR annotations, discriminating registered tests). The only delta since that review is the Moonlit fix plus two merge-main commits; no new mechanics were introduced.
Full rollup settled green (13/13 required checks SUCCESS), merge CLEAN. Enqueuing.
matthewevans
added
enhancement
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 b (s25b). The 7 Group-C Wave-5 subsystem cards deferred from s25a (#5155) — each landed through the full
/engine-implementerpipeline (plan → review-plan → implement → review-impl → commit), one card at a time, strictly serial. Seven commits flip 12 cards tosupported=true, complete the FIN Tiered class (7/7), and add two class-level correctness fixes. Branch rebased onto currentupstream/main(ffbfc916c); the AnyTurn / recipient / extra_source enum surfaces re-verified against the rebased base (details in Verification).Each card is a building block, not a one-off:
supported=false→true192e92f7e8920879a3ReplacementCondition::FirstTokenCreationEachTurn, CR 614.4), substitutingCopyTokenOf{AttachedTo}b10361bbfEffect::BecomeCopywith arecipientset + a turn-agnosticPlayerScope::AnyTurnend-step expiry (CR 611.2c locked set)e1e9a66ef<type>" superlative (ControlsCount+ControlledByEachPlayer{Max}) +Effect::Investigate9513279581de202e98EachDealsDamageEqualToPowerwith an optional distinct second source group (extra_source)043095b11CounterAll{Or[..]}) + a class-level count fix so "for each spell and ability countered this way" counts countered abilities (they emit noZoneChanged)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; shared parser/type files collide). One
rebase-adaptationcommit on top reconciles the drift main's +49 advance introduced — documented in that commit: two new upstreamEffect::BecomeCopyconstruction sites threaded with the Nikorecipientfield (serde-defaultSelfRef), aHashSetpath qualification, and theReplacementId → AppliedReplacementKeyapplied-set key refactor in the Moonlit test (the resolver auto-merged to the new key;AppliedReplacementKey::object(source, index)is the semantic equivalent, self-suppression re-verified). Thetests/integration/main.rsmod-line reconciliation was folded into the Niko commit at rebase.Files changed
crates/engine/src/types/ability.rs—Effect::BecomeCopy.recipient,PlayerScope::AnyTurn,EachDealsDamageEqualToPower.extra_source,ReplacementCondition::FirstTokenCreationEachTurn(all serde-defaulted → existing card-data byte-identical).crates/engine/src/parser/…— nom combinators: meld leading-condition/optional-cost, the "most<type>" superlative, the Tiered shared-effect distributor, the compound each-deals source, the counter-all conjunction handler.crates/engine/src/game/…— resolvers/targeting/layers/replacement + the counter-for-eachSpellCounteredcount arm; exhaustive-match fan-out (incl.PlayerScope::AnyTurnacrossability_rw/ability_scan/coverage/quantity/effects).crates/engine/tests/integration/…— one discriminating test file per card (revert-to-red verified).crates/mtgish-import,crates/phase-ai— exhaustive-match / consumer threading for the new engine surfaces.CR references
Union of the grep-verified CR annotations across the 7 commits: CR 107.1, 109.4, 109.5, 110.5d, 111.2, 111.10f, 113.9, 115.1d, 115.3, 115.4, 118.12, 120.1, 201.2a, 201.5c, 205.4a, 208.1, 303.4, 405.1, 513.1, 601.2c, 601.2f, 603.4, 603.7b, 608.2c, 608.2h, 611.2a, 611.2b, 611.2c, 613.4b, 613.4c, 614.1a, 614.4, 614.6, 616.1, 700.2, 701.16a, 701.42a, 701.6a, 702.183a, 704.5e, 707.2. See per-commit
CRannotations for the exact rule→code mapping.Track
Developer
LLM
Model: claude-opus-4.8
Thinking: high–max (planners/reviewers at xhigh; implementers/impl-reviewers at high)
Verification
Measured in the worktree (cargo direct — this branch is not Tilt-watched).
Base-timing note (read before the table):
upstream/mainadvanced +3 (bfe318f73→ffbfc916c) mid-gate. The behavioral gates below (clippy,test -p engine, FORGE, coverage) were measured on the +49 basebfe318f73. The branch was then re-rebased onto the final baseffbfc916c— a clean 0-conflict rebase that adds no enum variants — andcargo check --workspace --all-targetswas re-verified EXIT 0 on that final base. FORGEdelta-vs-mainand coverage are base-relative numbers; becauseffbfc916cintroduces no engine-types/evolution, they are expected to carry over, and CI runs the full suite on the final baseffbfc916cas the authoritative head-SHA measurement.cargo fmt --all -- --check— EXIT 0 (no diffs)cargo check --workspace --all-targets— EXIT 0 (re-run on the final baseffbfc916c; also confirmsphase-ai+mtgish-importexhaustive arms compile)cargo clippy --workspace --all-targets -- -D warnings— EXIT 0 (0 warnings)./scripts/check-parser-combinators.sh— EXIT 0./scripts/check-engine-authorities.sh— EXIT 0cargo test -p engine(lib + integration) — 18730 passed / 0 failed (16071 lib + 2644 integration + 6 + 9; 15 ignored)FORGE_TEST_FULL_DB=1 cargo test -p engine ordering_parity_sweep— swept=13598, compared=6218, unexplained=20, delta-vs-main = 0. The 20 unexplained rows are exactly the pre-existing hidden/veiled/opal/lurking morph cycle (Urza's Saga block); zero s25b cards appear in the unexplained set or anywhere in the sweep. The opt-in full-DB run always surfaces these 20 (strict gate is CI-fixture-scoped); the acceptance criterion is delta-vs-main=0, met.cargo coverage— EXIT 0 on a corpus regenerated from HEAD (offline, cached MTGJSON — no network): supported_cards 31510 / 35396 (89.02%). All 12 GAINED cards measuredUnimplemented=0on the fresh corpus (incl. Glen's mergedCounterAll{Or[StackSpell, StackAbility]}— the pre-fix splitCounterAll(spells)+Unimplemented("abilities")is gone). REGRESSED = 0 (the ~24-cardEachDealsDamageEqualToPowerfamily, the counter-for-each family {Swift Silence}, and every existingBecomeCopy/replacement/counter card byte-identical; per-card bounded-blast probes confirmed no sibling regressions). CI's coverage-regression check is the full-DB backstop confirming the exact GAINED set +REGRESSED=0vs the PR base.Post-rebase enum-surface re-verification:
PlayerScope::AnyTurn(Niko) remains threaded through every exhaustivePlayerScopematch site after the +49 rebase (rw_player_scope→Controller-identical profile,scan→Axes::NONE,coverage→display,quantity→unreachable!(duration-only),effects→controller,layersprune), with the sole production construction still the "the next end step" parser arm. FORGE ordering-parity delta = 0 (measured on the +49 basebfe318f73; see the base-timing note above — CI re-measures on the final baseffbfc916c).Scope Expansion
This tranche parameterizes existing effects (
BecomeCopy.recipient,EachDealsDamageEqualToPower.extra_source), adds one duration-timingPlayerScope::AnyTurnand oneReplacementCondition, completes the FIN Tiered mechanic, and lands two class-level correctness fixes (the counter-for-each ability count; the Tiered chosen-base-P/T silent-drop). No new subsystems — the heavy mechanics (meld, Tiered, mass counter, per-source power damage, Investigate) all pre-existed and were reused. Maintainer review invited.Validation Failures
None.
Predecessor
Continues s25a — #5155 (the "Deferred to s25b" table there is exactly these 7 cards).