feat(phase-ai): pick the least-costly land for a self-bounce, never the loop

crates/phase-ai/src/policies/mod.rs

@@ -50,6 +50,7 @@ mod redundancy_avoidance;
 pub mod registry;
 mod sacrifice_land_protection;
 mod sacrifice_value;
+mod self_bounce_target;
 mod self_cost;
 mod self_cost_value;
 mod self_protection_classify;

crates/phase-ai/src/policies/registry.rs

@@ -142,6 +142,8 @@ pub enum PolicyId {
     GraveyardTypes,
     CrewTiming,
     CombatWithdrawal,
+    /// CR 608.2c: "return a land you control" self-bounce target choice.
+    SelfBounceTarget,
 }
 
 /// Coarse routing kind for a candidate decision. Each policy declares which
@@ -396,6 +398,7 @@ impl Default for PolicyRegistry {
             Box::new(PayoffPolicy::new(&REANIMATOR_PAYOFF)),
             Box::new(PayoffPolicy::new(&BLINK_PAYOFF)),
             Box::new(LoopShortcutPolicy),
+            Box::new(super::self_bounce_target::SelfBounceTargetPolicy),
         ];
         let mut by_kind: HashMap<DecisionKind, Vec<usize>> = HashMap::new();
         for (idx, policy) in policies.iter().enumerate() {

crates/phase-ai/src/policies/self_bounce_target.rs

@@ -0,0 +1,146 @@
+//! Self-bounce target selection — when a "return a land you control" ETB forces
+//! the AI to return one of its own lands, pick the least-costly one and NEVER
+//! the land that just entered (which loops).
+//!
+//! ## The defect this closes (#4730)
+//!
+//! Azorius Chancery and the rest of the Ravnica/MOM bounce-land ("Karoo") cycle
+//! enter and their ETB returns "a land you control" to hand (CR 608.2c, chosen
+//! at resolution). No policy scored that choice, so the AI kept returning the
+//! bounce-land it had just played — putting it back in hand to be replayed and
+//! bounced again, the tempo loop the reporter observed. `land_sequencing` fixed
+//! the *sequencing* half and its own doc-comment deferred this half verbatim:
+//! "the AI should return the least-useful land, never the just-played
+//! bounce-land — that needs its own target-selection policy."
+//!
+//! ## Heuristic (building block, not a card fix)
+//!
+//! The choice surfaces as a `WaitingFor::EffectZoneChoice` returning battlefield
+//! lands you control to hand; each candidate is a `SelectCards` selection.
+//! Detection is structural (no card names) — it covers every "return a land you
+//! control" bouncer. Each land in the selection is scored by how little
+//! returning it costs:
+//! * the ability's own source — the just-played bounce-land — takes a strong
+//!   penalty, so it is chosen only when it is the sole eligible land (which
+//!   breaks the loop);
+//! * an already-tapped land is preferred (its mana is spent this turn, so
+//!   replaying it next turn costs the least tempo);
+//! * an untapped land is mildly penalized (returning it forfeits mana still
+//!   available this turn) but still beats looping the bounce-land.
+
+use engine::types::ability::EffectKind;
+use engine::types::actions::GameAction;
+use engine::types::card_type::CoreType;
+use engine::types::game_state::{GameState, WaitingFor};
+use engine::types::identifiers::ObjectId;
+use engine::types::player::PlayerId;
+use engine::types::zones::Zone;
+
+use crate::features::DeckFeatures;
+
+use super::context::PolicyContext;
+use super::registry::{DecisionKind, PolicyId, PolicyReason, PolicyVerdict, TacticalPolicy};
+
+/// Penalty for returning the ability's own source — the just-played bounce-land.
+/// Returning it re-buys the land drop for nothing and loops, so it must lose to
+/// any other land the pool offers.
+pub(crate) const RETURN_SOURCE_PENALTY: f64 = -3.0;
+/// Bonus for returning an already-tapped land: its mana is spent this turn, so
+/// replaying it next turn costs the least tempo.
+pub(crate) const RETURN_TAPPED_BONUS: f64 = 1.0;
+/// Penalty for returning an untapped land: doing so forfeits mana still
+/// available this turn. Milder than the source penalty — an untapped non-source
+/// land is still a better return than looping the bounce-land.
+pub(crate) const RETURN_UNTAPPED_PENALTY: f64 = -0.5;
+
+pub struct SelfBounceTargetPolicy;
+
+impl SelfBounceTargetPolicy {
+    /// Desirability of returning `land_id` to hand; higher = better to bounce.
+    pub(crate) fn return_desirability(
+        state: &GameState,
+        land_id: ObjectId,
+        source_id: ObjectId,
+    ) -> f64 {
+        // CR 608.2c: never bounce the permanent that generated the choice — that
+        // is the tempo loop this policy exists to break.
+        if land_id == source_id {
+            return RETURN_SOURCE_PENALTY;
+        }
+        match state.objects.get(&land_id) {
+            Some(land) if land.tapped => RETURN_TAPPED_BONUS,
+            Some(_) => RETURN_UNTAPPED_PENALTY,
+            None => 0.0,
+        }
+    }
+}
+
+impl TacticalPolicy for SelfBounceTargetPolicy {
+    fn id(&self) -> PolicyId {
+        PolicyId::SelfBounceTarget
+    }
+
+    fn decision_kinds(&self) -> &'static [DecisionKind] {
+        // `WaitingFor::EffectZoneChoice` routes through the ActivateAbility
+        // catch-all bucket (see `decision_kind::classify`).
+        &[DecisionKind::ActivateAbility]
+    }
+
+    fn activation(
+        &self,
+        _features: &DeckFeatures,
+        _state: &GameState,
+        _player: PlayerId,
+    ) -> Option<f32> {
+        // Universal; the verdict's bounce-choice guard self-gates.
+        // activation-constant: self-bounce target choice, universal.
+        Some(1.0)
+    }
+
+    fn verdict(&self, ctx: &PolicyContext<'_>) -> PolicyVerdict {
+        let na = || PolicyVerdict::neutral(PolicyReason::new("self_bounce_target_na"));
+
+        let GameAction::SelectCards { cards } = &ctx.candidate.action else {
+            return na();
+        };
+        // CR 608.2c: a resolution-time "return a land you control to hand"
+        // choice — from the battlefield, to hand, scoped to the AI.
+        let WaitingFor::EffectZoneChoice {
+            player,
+            source_id,
+            effect_kind: EffectKind::ChangeZone,
+            zone: Zone::Battlefield,
+            destination: Some(Zone::Hand),
+            cards: pool,
+            ..
+        } = &ctx.decision.waiting_for
+        else {
+            return na();
+        };
+        if *player != ctx.ai_player {
+            return na();
+        }
+        // Restrict to the "return a LAND you control" class: every eligible
+        // object is a land the AI controls. A value self-bounce of a creature
+        // (blink, save-from-removal) is intentionally left untouched.
+        let all_own_lands = pool.iter().all(|id| {
+            ctx.state.objects.get(id).is_some_and(|o| {
+                o.controller == ctx.ai_player && o.card_types.core_types.contains(&CoreType::Land)
+            })
+        });
+        if pool.is_empty() || !all_own_lands || cards.is_empty() {
+            return na();
+        }
+
+        let delta: f64 = cards
+            .iter()
+            .map(|&id| Self::return_desirability(ctx.state, id, *source_id))
+            .sum();
+
+        PolicyVerdict::score(
+            delta,
+            PolicyReason::new("self_bounce_target")
+                .with_fact("returns_source", i64::from(cards.contains(source_id))),
+        )
+    }
+}

crates/phase-ai/src/policies/tests/mod.rs

@@ -15,3 +15,4 @@ pub mod mulligan_input_lint;
 pub mod poison;
 pub mod reanimator_payoff;
 pub mod score_contract_lint;
+pub mod self_bounce_target;

crates/phase-ai/src/policies/tests/self_bounce_target.rs

@@ -0,0 +1,213 @@
+//! Unit tests for `policies::self_bounce_target` — CR 608.2c "return a land you
+//! control" self-bounce target choice (#4730). No `#[cfg(test)]` in SOURCE
+//! files; tests live here.
+//!
+//! `return_desirability` is the pure core (source-loop guard + tapped/untapped
+//! tempo ordering); the composed `verdict` runs against a real `PolicyContext`
+//! built over a `WaitingFor::EffectZoneChoice` + `SelectCards` candidate, the
+//! seam the engine surfaces for a non-targeted battlefield→hand land bounce.
+
+use engine::ai_support::{ActionMetadata, AiDecisionContext, CandidateAction, TacticalClass};
+use engine::game::zones::create_object;
+use engine::types::ability::EffectKind;
+use engine::types::actions::GameAction;
+use engine::types::card_type::{CardType, CoreType};
+use engine::types::format::FormatConfig;
+use engine::types::game_state::{GameState, WaitingFor};
+use engine::types::identifiers::{CardId, ObjectId};
+use engine::types::player::PlayerId;
+use engine::types::zones::{EtbTapState, Zone};
+
+use crate::config::AiConfig;
+use crate::context::AiContext;
+use crate::policies::context::{PolicyContext, SearchDepth};
+use crate::policies::registry::{PolicyVerdict, TacticalPolicy};
+use crate::policies::self_bounce_target::*;
+
+const AI: PlayerId = PlayerId(0);
+
+fn state() -> GameState {
+    GameState::new(FormatConfig::standard(), 2, 42)
+}
+
+// ─── return_desirability (pure core) ────────────────────────────────────────
+
+#[test]
+fn source_ranks_below_every_other_land() {
+    let mut st = state();
+    let source = land(&mut st, 1, true); // Karoo enters tapped
+    let tapped = land(&mut st, 2, true);
+    let untapped = land(&mut st, 3, false);
+
+    let d_source = SelfBounceTargetPolicy::return_desirability(&st, source, source);
+    let d_tapped = SelfBounceTargetPolicy::return_desirability(&st, tapped, source);
+    let d_untapped = SelfBounceTargetPolicy::return_desirability(&st, untapped, source);
+
+    assert_eq!(d_source, RETURN_SOURCE_PENALTY);
+    assert_eq!(d_tapped, RETURN_TAPPED_BONUS);
+    assert_eq!(d_untapped, RETURN_UNTAPPED_PENALTY);
+    // The whole point of #4730: the just-played bounce-land is the worst return,
+    // and a spent (tapped) land is the best.
+    assert!(
+        d_source < d_untapped && d_untapped < d_tapped,
+        "ordering source < untapped < tapped, got {d_source} {d_untapped} {d_tapped}"
+    );
+}
+
+// ─── verdict (composed over EffectZoneChoice) ───────────────────────────────
+
+#[test]
+fn verdict_prefers_a_spent_land_over_the_bounce_source() {
+    // Pool: the just-played Karoo (source, tapped) + a spent basic + an untapped
+    // basic. Returning the source must score lowest, the tapped basic highest.
+    let d_source = verdict_delta(&[(true, true), (false, true), (false, false)], &[0]);
+    let d_tapped = verdict_delta(&[(true, true), (false, true), (false, false)], &[1]);
+    let d_untapped = verdict_delta(&[(true, true), (false, true), (false, false)], &[2]);
+    assert_eq!(d_source, RETURN_SOURCE_PENALTY);
+    assert_eq!(d_tapped, RETURN_TAPPED_BONUS);
+    assert_eq!(d_untapped, RETURN_UNTAPPED_PENALTY);
+    assert!(
+        d_source < d_untapped && d_untapped < d_tapped,
+        "AI must rank the just-played bounce-land last"
+    );
+}
+
+#[test]
+fn verdict_is_neutral_for_a_non_land_pool() {
+    // A value self-bounce of creatures (blink / save-from-removal) must be left
+    // to the value policies — this policy only governs land bounces.
+    assert_eq!(
+        verdict_delta_kinds(
+            &[(true, true), (false, true)],
+            &[1],
+            false,
+            Some(Zone::Hand),
+            EffectKind::ChangeZone
+        ),
+        0.0
+    );
+}
+
+#[test]
+fn verdict_is_neutral_for_non_hand_destination() {
+    // Battlefield→exile / other ChangeZone choices are not the "return to hand"
+    // bounce class.
+    assert_eq!(
+        verdict_delta_kinds(
+            &[(true, true), (false, true)],
+            &[1],
+            true,
+            Some(Zone::Exile),
+            EffectKind::ChangeZone
+        ),
+        0.0
+    );
+}
+
+// ─── helpers ────────────────────────────────────────────────────────────────
+
+fn land(state: &mut GameState, idx: u64, tapped: bool) -> ObjectId {
+    make_object(state, idx, true, tapped)
+}
+
+fn make_object(state: &mut GameState, idx: u64, is_land: bool, tapped: bool) -> ObjectId {
+    let oid = create_object(
+        state,
+        CardId(idx),
+        AI,
+        format!("Perm {idx}"),
+        Zone::Battlefield,
+    );
+    let obj = state.objects.get_mut(&oid).unwrap();
+    obj.card_types = CardType {
+        supertypes: Vec::new(),
+        core_types: vec![if is_land {
+            CoreType::Land
+        } else {
+            CoreType::Creature
+        }],
+        subtypes: Vec::new(),
+    };
+    obj.tapped = tapped;
+    oid
+}
+
+/// Build the land pool (index 0 is the bounce source) and score the given
+/// selection under a battlefield→hand `ChangeZone` land bounce.
+fn verdict_delta(pool_spec: &[(bool, bool)], selection: &[usize]) -> f64 {
+    verdict_delta_kinds(
+        pool_spec,
+        selection,
+        true,
+        Some(Zone::Hand),
+        EffectKind::ChangeZone,
+    )
+}
+
+/// `pool_spec[i] = (is_source_placeholder_unused, tapped)`; index 0 is always
+/// the ability source. `lands` toggles land vs creature pool objects.
+fn verdict_delta_kinds(
+    pool_spec: &[(bool, bool)],
+    selection: &[usize],
+    lands: bool,
+    destination: Option<Zone>,
+    effect_kind: EffectKind,
+) -> f64 {
+    let mut st = state();
+    let pool: Vec<ObjectId> = pool_spec
+        .iter()
+        .enumerate()
+        .map(|(i, &(_, tapped))| make_object(&mut st, 100 + i as u64, lands, tapped))
+        .collect();
+    let source_id = pool[0];
+    let selected: Vec<ObjectId> = selection.iter().map(|&i| pool[i]).collect();
+
+    let decision = AiDecisionContext {
+        waiting_for: WaitingFor::EffectZoneChoice {
+            player: AI,
+            cards: pool,
+            count: 1,
+            min_count: 1,
+            up_to: false,
+            source_id,
+            effect_kind,
+            zone: Zone::Battlefield,
+            destination,
+            enter_tapped: EtbTapState::Unspecified,
+            enter_transformed: false,
+            enters_under_player: None,
+            enters_attacking: false,
+            owner_library: false,
+            track_exiled_by_source: false,
+            face_down_profile: None,
+            enter_with_counters: Vec::new(),
+            conditional_enter_with_counters: Vec::new(),
+            count_param: 0,
+            library_position: None,
+            is_cost_payment: false,
+            enters_modified_if: None,
+            duration: None,
+        },
+        candidates: Vec::new(),
+    };
+    let candidate = CandidateAction {
+        action: GameAction::SelectCards { cards: selected },
+        metadata: ActionMetadata::for_actor(Some(AI), TacticalClass::Ability),
+    };
+    let config = AiConfig::default();
+    let aicontext = AiContext::empty(&config.weights);
+    let ctx = PolicyContext {
+        state: &st,
+        decision: &decision,
+        candidate: &candidate,
+        ai_player: AI,
+        config: &config,
+        context: &aicontext,
+        cast_facts: None,
+        search_depth: SearchDepth::Root,
+    };
+    match SelfBounceTargetPolicy.verdict(&ctx) {
+        PolicyVerdict::Score { delta, .. } => delta,
+        PolicyVerdict::Reject { reason } => panic!("unexpected Reject: {reason:?}"),
+    }
+}