fix(parser): derive activation zone from a self-move's origin, not its destination

crates/engine/src/database/synthesis.rs

@@ -18597,6 +18597,33 @@ mod sorcery_speed_invariant_tests {
             )),
             "materials-exile sub-cost present (CR 702.167a/b)"
         );
+
+        // CR 702.167a + CR 113.6m: Craft's cost EXILES THE PERMANENT FROM THE
+        // BATTLEFIELD, so CR 113.6m's `unless` clause ("a previous part of its
+        // cost … specifies that the object is put into that zone") exempts it,
+        // and CR 113.6j makes the battlefield the only zone the cost is payable
+        // from. Craft is synthesized here, never through
+        // `parse_activated_ability_ir`, so the CR 113.6m effect-side derivation
+        // cannot reach it — this pins that.
+        assert_eq!(
+            def.activation_zone, None,
+            "craft functions from the battlefield"
+        );
+        // Second, independent line of defense: even on a hypothetical parser
+        // path, `activation_zone_from_self_cost` matches this cost component
+        // FIRST and yields Battlefield, so the effect side is never consulted.
+        assert!(
+            costs.iter().any(|c| matches!(
+                c,
+                AbilityCost::Exile {
+                    zone: Some(Zone::Battlefield),
+                    filter: Some(TargetFilter::SelfRef),
+                    ..
+                }
+            )),
+            "the self-exile cost names the battlefield, so the cost-side \
+             derivation wins before the effect side is consulted"
+        );
     }
 
     /// CR 702.87a: Level Up synthesis must carry AsSorcery.

crates/engine/src/parser/oracle.rs

@@ -1,4 +1,4 @@
-use std::borrow::Cow;
+use std::{borrow::Cow, ops::ControlFlow};
 
 use crate::parser::oracle_nom::error::{OracleError, OracleResult};
 use nom::branch::alt;
@@ -19,6 +19,7 @@ use crate::types::ability::{
     ReplacementDefinition, SolveCondition, SpellCastingOption, StaticCondition, StaticDefinition,
     TapStateChange, TargetFilter, TriggerCondition, TriggerDefinition, TypedFilter,
 };
+use crate::types::ability_visit::{visit_ability_def_scoped, ResolutionScope};
 use crate::types::format::DeckCopyLimit;
 use crate::types::keywords::{EscapeCost, FlashbackCost, Keyword, KeywordKind};
 use crate::types::mana::ManaCost;
@@ -6754,6 +6755,9 @@ fn activation_zone_from_self_cost(cost: &AbilityCost) -> Option<Zone> {
             zone: Some(zone),
             ..
         } => Some(*zone),
+        AbilityCost::Sacrifice(sacrifice) if sacrifice.target == TargetFilter::SelfRef => {
+            Some(Zone::Battlefield)
+        }
         AbilityCost::Composite { costs } => costs.iter().find_map(activation_zone_from_self_cost),
         _ => None,
     }
@@ -6762,27 +6766,76 @@ fn activation_zone_from_self_cost(cost: &AbilityCost) -> Option<Zone> {
 /// Effect-side companion to `activation_zone_from_self_cost`.
 ///
 /// CR 113.6m + CR 602.1: an activated ability whose *effect* moves the object
-/// it's printed on out of a particular non-battlefield zone (e.g. "Put this
-/// card from your hand onto the battlefield") functions only from that zone.
-/// The cost-based derivation cannot see this because the zone lives in the
-/// effect, not the cost. This walks the parsed effect chain for a self-
-/// `ChangeZone` whose `origin` is a non-battlefield zone and `destination` is
-/// the battlefield, returning that origin as the activation zone.
+/// it's printed on out of a particular non-battlefield zone functions only from
+/// that zone. The cost-based derivation cannot see this because the zone lives
+/// in the effect, not the cost. This walks the parsed effect chain for a self-
+/// `ChangeZone` with a non-battlefield `origin`, returning that origin as the
+/// activation zone.
+///
+/// **The rule quantifies over the ORIGIN zone only.** CR 113.6m reads "an
+/// ability whose cost or effect specifies that it moves the object it's on
+/// **out of a particular zone** functions only in that zone" — the destination
+/// appears nowhere in it. Both destinations are live in the corpus and both
+/// derive the same way: `→ Battlefield` (Reassembling Skeleton /
+/// Bloodsoaked Champion, CR 113.6m's own printed example) and `→ Hand`
+/// (Gutterbones / Bestial Bloodline, "Return this card from your graveyard to
+/// your hand"). Do not re-add a `destination` field to the pattern.
+///
+/// `origin != Zone::Battlefield` is the CR 113.6 default guard, **not** part of
+/// CR 113.6m: an ability whose effect moves its own source *off* the
+/// battlefield already functions there by default, so there is nothing to
+/// derive. Keep it — it is the correct default and costs nothing — but do not
+/// mistake it for load-bearing: **its class is empty at this corpus vintage.**
+/// 0 of the 22,794 parsed abilities carry a self-`ChangeZone` with
+/// `origin: Some(Zone::Battlefield)`, so no card and no test reaches this line.
+/// The shape that would reach it is an effect lowering to
+/// `ChangeZone { origin: Some(Zone::Battlefield), target: TargetFilter::SelfRef, .. }`
+/// — a self-move whose text names the battlefield as the zone it moves out of.
+/// No printed self-move does today: they leave the origin unstated
+/// (`origin: None`) or lower to a different variant. The two Auras that look
+/// like this class are rejected by *earlier* parts of the pattern and never
+/// arrive here — Cooped Up (`{2}{W}: Exile enchanted creature.`) by
+/// `target: TargetFilter::SelfRef`, because it moves the enchanted creature and
+/// not its own source, and Cage of Hands (`{1}{W}: Return this Aura to its
+/// owner's hand.`) by the `Effect::ChangeZone` variant match, because it lowers
+/// to `Effect::Bounce`.
+///
+/// The canonical own-resolution traversal is **kind-agnostic** and walks direct
+/// sub-, otherwise-, and modal branches. Lochmere Serpent depends on exactly
+/// that: its `Graveyard → Hand` self-move sits on a sub-ability whose kind is
+/// `Spell`, not `Activated`. Three parts of CR 113.6m are deliberately **not** implemented because
+/// each governs a measurably empty class at this corpus vintage; each has its
+/// extension point named here:
+/// - the `unless` clause's effect half ("a previous part of its … effect
+///   specifies that the object is put into that zone") — 0 operative cards;
+///   extension point: skip a later self-move whose zone an earlier part filled.
+/// - the Aura half of the `unless` clause (satisfiable by a cost, an effect
+///   **or** a trigger condition specifying that the enchanted object leaves the
+///   battlefield) — none of the Auras in the class qualifies; extension point:
+///   a cost-chain inspection in this function.
+/// - CR 113.6m sentence 2 (an effect that creates a delayed triggered ability
+///   which moves the object out of a zone, CR 603.7) — 0 operative cards (the
+///   abilities carrying that shape are synthesized Unearth, CR 702.84, whose
+///   delayed move is `Battlefield → Exile`, i.e. the CR 113.6 default);
+///   extension point: an `Effect::CreateDelayedTrigger` arm here that recurses
+///   into the carried `AbilityDefinition`.
 fn activation_zone_from_self_effect(def: &AbilityDefinition) -> Option<Zone> {
-    if let Effect::ChangeZone {
-        origin: Some(origin),
-        destination: Zone::Battlefield,
-        target: TargetFilter::SelfRef,
-        ..
-    } = *def.effect
-    {
-        if origin != Zone::Battlefield {
-            return Some(origin);
+    let mut activation_zone = None;
+    let _ = visit_ability_def_scoped(def, ResolutionScope::OwnResolutionOnly, &mut |effect| {
+        if let Effect::ChangeZone {
+            origin: Some(origin),
+            target: TargetFilter::SelfRef,
+            ..
+        } = effect
+        {
+            if *origin != Zone::Battlefield {
+                activation_zone = Some(*origin);
+                return ControlFlow::Break(());
+            }
         }
-    }
-    def.sub_ability
-        .as_deref()
-        .and_then(activation_zone_from_self_effect)
+        ControlFlow::Continue(())
+    });
+    activation_zone
 }
 
 /// CR 608.2k: Source zone of a non-self `AbilityCost::Exile` component
@@ -6843,13 +6896,45 @@ fn parse_activated_ability_ir(
     ctx.current_ability_exile_cost_zone = prev_exile_zone;
     ctx.current_ability_index = prev_ability_index;
     let lowered_for_activation_zone = lower_ability_ir(&ir);
-    // CR 113.6m: fall back to the effect-side derivation — an ability whose
-    // effect moves the source out of a non-battlefield zone functions only
-    // from that zone. Cost-based derivation keeps priority.
-    ir.shell.activation_zone = lowered_for_activation_zone
-        .activation_zone
-        .or_else(|| activation_zone_from_self_cost(&cost))
-        .or_else(|| activation_zone_from_self_effect(&lowered_for_activation_zone));
+    // Three-authority precedence for the activation zone. The ORDER IS A RULES
+    // BOUNDARY, not a style choice — see Kogla and Yidaro below.
+    //
+    // 1. CR 113.6b: "An ability that states which zones it functions in
+    //    functions only from those zones." When the card states the zone there
+    //    is nothing to derive, and "only from those zones" is exclusive. Today
+    //    this link is reachable only from the whole-line dispatch sites that
+    //    stamp the shell directly (Channel, CR 207.2c; Forecast, CR 702.57a) and
+    //    from the `database/` synthesis writers — never from inside this
+    //    function, whose `ir` is built fresh from the post-colon effect text.
+    //    It is a deliberate forward guard for the day an explicit-zone grammar
+    //    routes through here, NOT dead code to be tidied away.
+    // 2. CR 113.6j + CR 118.3: a cost-derived source zone takes priority over
+    //    a conflicting effect origin. Battlefield remains the implicit default
+    //    representation unless that priority is needed.
+    // 3. CR 113.6m: an ability whose effect moves the source out of a
+    //    non-battlefield zone functions only from that zone.
+    //
+    // 2 ≻ 3 is discriminating on **Kogla and Yidaro**: "{2}{R}{G}, Discard this
+    // card: … Shuffle this card into your library from your graveyard, …".
+    // The cost yields `Hand` and the effect yields `Graveyard`; `Hand` is
+    // correct, because discarding is what put the card into the graveyard, so
+    // CR 113.6m's `unless` clause ("a previous part of its cost … specifies
+    // that the object is put into that zone") makes the effect side
+    // inapplicable by rule, and CR 118.3 makes a graveyard activation
+    // unpayable rather than merely suboptimal. Reversing this precedence
+    // regresses that card.
+    let cost_activation_zone = activation_zone_from_self_cost(&cost);
+    let effect_activation_zone = activation_zone_from_self_effect(&lowered_for_activation_zone);
+    ir.shell.activation_zone = lowered_for_activation_zone.activation_zone.or({
+        match (cost_activation_zone, effect_activation_zone) {
+            // A self-sacrifice is paid from the battlefield, but Battlefield is
+            // the default activation zone. Preserve `None` until it must defeat
+            // a derived non-battlefield effect origin.
+            (Some(Zone::Battlefield), None) => None,
+            (Some(cost_zone), _) => Some(cost_zone),
+            (None, effect_zone) => effect_zone,
+        }
+    });
     ir.shell.cost = Some(cost);
     ir.shell.description = Some(description.to_string());
     if !constraints.restrictions.is_empty() {