From d332f482a5ce2bbf839c63194756f05220892fc1 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Mon, 3 Aug 2026 07:13:16 -0700 Subject: [PATCH] fix(ai): answer selection prompts from the engine's issued domain (#6942) The AI fallback returned `SelectCards { cards: Vec::new() }` for a group of `WaitingFor` variants under the comment "empty selection is a valid 'choose nothing'". For six of them the engine rejects an empty selection unconditionally, so the AI proposed an action that could never be accepted and the game stalled. CR 608.2d: a player can't choose an option that's illegal or impossible. The arm made a blanket cardinality assumption across variants whose contracts differ, and several are data-dependent at runtime (`up_to`, `optional`) rather than a property of the variant. A static reclassification would have traded a softlock for a wrong-decision bug. So do not classify at all. Delegate to the engine's own issued candidate set via `build_decision_context_for_semantic_owner`, which is already the authority the gating contract uses. The AI stops restating a cardinality rule it was getting wrong and answers from the domain the engine published -- removing the second hand-maintained list whose disagreement with the first caused this. Two prior drafts mirrored a `pub(crate)` enumerator into phase-ai instead; that would have recreated the defect. The seat is `contract.semantic_owner`, threaded through `fallback_action`. Deriving it from `acting_player()` -- as an earlier draft did -- diverges from the seat the gating contract was issued for, so the fix would have failed precisely in the multi-seat case it exists to serve. The `MulliganDecision` arm's dispatch is moved off `pending.first()` for the same reason, mirroring the existing correct form. Also fixes the engine side of an enumerator/handler disagreement: the candidate enumerator emits `keep_count.min(selectable.len())` for `DigChoice` while the handler demanded exactly `keep_count`, so a dig with fewer legal cards than its keep count was unanswerable. Clamped in the handler, which is the defective side (CR 609.3, CR 101.3). `MultiTargetSelection` is a confirmed member of this defect class and is deliberately NOT converted: the enumerator discards `max_targets` and issues all legal targets at once, so at a min:1/max:1 site with more than one legal target its only non-empty candidate is itself illegal. Delegating there would swap a contract-refusing action for a contract-passing, engine-rejected one. Filed as follow-up rather than half-fixed. Every test's revert baseline is stated per row, because most cannot be observed at bare base at all -- threading the contract changes `fallback_action`'s arity and phase-ai is one compile unit. T11 in particular was observed red on the full fix MINUS ONLY the one-line seat source, which is the only tree that isolates the seat defect; at base+1+2 it would go red merely because the mulligan step is absent. --- .../src/game/engine_resolution_choices.rs | 25 +- .../integration/dig_impossible_keep_count.rs | 113 ++ crates/engine/tests/integration/main.rs | 1 + crates/phase-ai/src/search.rs | 994 +++++++++++++++++- 4 files changed, 1105 insertions(+), 28 deletions(-) create mode 100644 crates/engine/tests/integration/dig_impossible_keep_count.rs diff --git a/crates/engine/src/game/engine_resolution_choices.rs b/crates/engine/src/game/engine_resolution_choices.rs index f03f580240..c1fad005ac 100644 --- a/crates/engine/src/game/engine_resolution_choices.rs +++ b/crates/engine/src/game/engine_resolution_choices.rs @@ -2988,12 +2988,25 @@ pub(super) fn handle_resolution_choice( kept.len() ))); } - } else if kept.len() != keep_count { - return Err(EngineError::InvalidAction(format!( - "Must select exactly {} cards, got {}", - keep_count, - kept.len() - ))); + } else { + // CR 609.3 + CR 101.3: a dig whose filter (or a short library) + // leaves fewer selectable cards than `keep_count` must keep as + // many as possible, not reject every selection. Without the + // clamp no legal action exists in that state — + // `validate_dig_selection` below requires every kept id to be in + // `selectable_cards` while this gate demands more ids than it + // holds — softlocking every controller. Matches the clamp the + // candidate enumerator (`ai_support/candidates.rs:1185`) and + // `cheap_reject_candidate` (`ai_support/mod.rs:702`) already + // apply. + let required = keep_count.min(selectable_cards.len()); + if kept.len() != required { + return Err(EngineError::InvalidAction(format!( + "Must select exactly {} cards, got {}", + required, + kept.len() + ))); + } } // CR 401.2 + CR 608.2c: the keep-selection must be unique, drawn from diff --git a/crates/engine/tests/integration/dig_impossible_keep_count.rs b/crates/engine/tests/integration/dig_impossible_keep_count.rs new file mode 100644 index 0000000000..d63e01c6db --- /dev/null +++ b/crates/engine/tests/integration/dig_impossible_keep_count.rs @@ -0,0 +1,113 @@ +//! Issue #6942: a `DigChoice` whose filter (or a short library) leaves fewer +//! selectable cards than `keep_count` must accept the largest possible +//! selection, not reject every selection. +//! +//! CR 609.3 ("If an effect attempts to do something impossible, it does only as +//! much as possible") and CR 101.3 ("Any part of an instruction that's +//! impossible to perform is ignored"). Before the fix, the exact-cardinality +//! gate in `engine_resolution_choices.rs` demanded `keep_count` ids while +//! `validate_dig_selection` required every kept id to be in `selectable_cards` +//! — so when `selectable_cards.len() < keep_count` the two rules had NO common +//! solution and every controller (AI, human, multiplayer server) softlocked. +//! +//! The candidate enumerator (`ai_support/candidates.rs`) and +//! `cheap_reject_candidate` (`ai_support/mod.rs`) already clamped to +//! `keep_count.min(selectable_cards.len())`; the resolution handler was the +//! outlier. +use engine::game::scenario::GameScenario; +use engine::types::actions::GameAction; +use engine::types::game_state::WaitingFor; +use engine::types::identifiers::ObjectId; +use engine::types::phase::Phase; +use engine::types::zones::Zone; +use engine::types::PlayerId; + +const P0: PlayerId = PlayerId(0); + +/// A filtered dig that looked at three cards but whose filter matched only one, +/// with `keep_count: 2` and `up_to: false`. +/// +/// This is the shape `effects/dig.rs` produces: `selectable_cards` is pruned by +/// the effect's filter while `keep_count` stays at the card-literal value. +fn filtered_dig_runner() -> (engine::game::scenario::GameRunner, Vec) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let looked_at: Vec = ["Dug One", "Dug Two", "Dug Three"] + .iter() + .map(|name| scenario.add_spell_to_library_top(P0, name, false).id()) + .collect(); + let mut runner = scenario.build(); + runner.state_mut().waiting_for = WaitingFor::DigChoice { + player: P0, + library_owner: P0, + cards: looked_at.clone(), + keep_count: 2, + up_to: false, + // The filter matched exactly one of the three looked-at cards. + selectable_cards: vec![looked_at[0]], + kept_destination: Some(Zone::Hand), + rest_destination: Some(Zone::Graveyard), + source_id: None, + enter_tapped: false, + }; + (runner, looked_at) +} + +/// PAIRED NEGATIVE, run first because it must leave the prompt intact: the +/// clamp relaxes the *cardinality* gate only. A selection of the clamped size +/// whose id is outside `selectable_cards` is still rejected by +/// `validate_dig_selection`, so the filter check was not disabled. +#[test] +fn dig_clamp_does_not_disable_the_filter_check() { + let (mut runner, looked_at) = filtered_dig_runner(); + + let err = runner + .act(GameAction::SelectCards { + cards: vec![looked_at[1]], + }) + .expect_err("a non-matching id must still be refused"); + assert!( + format!("{err:?}").contains("does not match the effect's filter"), + "the refusal must come from validate_dig_selection, not the cardinality \ + gate — got {err:?}" + ); + assert!( + matches!(runner.state().waiting_for, WaitingFor::DigChoice { .. }), + "a refused selection must leave the prompt pending" + ); +} + +/// MAIN TEST. FAILS BEFORE THE FIX: `kept.len() != keep_count` evaluates +/// `1 != 2` and returns `InvalidAction("Must select exactly 2 cards, got 1")`, +/// while every larger selection is refused by `validate_dig_selection` — no +/// legal action exists. +#[test] +fn dig_with_fewer_selectable_cards_than_keep_count_keeps_as_many_as_possible() { + let (mut runner, looked_at) = filtered_dig_runner(); + let (kept, unkept) = (looked_at[0], [looked_at[1], looked_at[2]]); + + runner + .act(GameAction::SelectCards { cards: vec![kept] }) + .expect( + "CR 609.3: the only selection the filter permits must be accepted \ + when keep_count exceeds the selectable set", + ); + + let hand = &runner.state().players[P0.0 as usize].hand; + assert!( + hand.contains(&kept), + "the single filter-matching card must reach kept_destination (hand)" + ); + let graveyard = &runner.state().players[P0.0 as usize].graveyard; + for id in unkept { + assert!( + graveyard.contains(&id), + "the unkept cards must reach rest_destination (graveyard); \ + graveyard = {graveyard:?}" + ); + } + assert!( + !matches!(runner.state().waiting_for, WaitingFor::DigChoice { .. }), + "the dig prompt must be resolved, not re-parked" + ); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 34a971b839..6d7e382846 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -149,6 +149,7 @@ mod destroy_redirect_to_battlefield_delivery_tail; mod deterministic_blocker_prompt_order; mod devour_co_entry_regression; mod devour_intellect_treasure_rider; +mod dig_impossible_keep_count; mod dig_rest_pile_stranding_on_etb_pause; mod diligent_farmhand_counts_as_named; mod diluvian_primordial_6754; diff --git a/crates/phase-ai/src/search.rs b/crates/phase-ai/src/search.rs index 8d6002d9be..b67755584b 100644 --- a/crates/phase-ai/src/search.rs +++ b/crates/phase-ai/src/search.rs @@ -292,7 +292,7 @@ fn choose_action_with_session_inner( if scored.is_empty() { // No valid candidates from search — fall back to a safe escape action // so the game never deadlocks waiting for the AI. - return fallback_action(state, config) + return fallback_action(state, config, &contract) .filter(|action| root_action_is_allowed(state, ai_player, action)) .filter(|action| { durable_pact_routes || !is_certified_pact_root(state, ai_player, action) @@ -851,6 +851,42 @@ pub fn emit_trace_for_candidate( ); } +/// Pick a `SelectCards` answer out of the contract that will gate it. +/// +/// `AiDecisionContract::contains_action` tests exact set membership, not +/// cardinality (`engine/src/ai_support/context.rs:77-105`), and the enumeration +/// behind it is capped at `SELECTION_CANDIDATE_CAP` in lexicographic order +/// (`ai_support/candidates.rs:5083-5096`). A synthesized selection can therefore +/// satisfy the resolution handler and STILL be refused by the contract, which +/// degrades to "the AI has no action" (#6942). Taking the answer from the +/// caller's own contract makes membership hold by identity. +/// +/// This is also the single authority for cardinality: the enumerator emits size +/// 0 only where the prompt's runtime `up_to` / `optional` / `allows_partial_find` +/// / `min_count` fields permit it, so no second table is kept here. +/// +/// Prefers the smallest issued selection, preserving the previous arm's +/// conservative "choose as little as legally possible" intent for the windows +/// where an empty pick genuinely is legal. That preference is sound only where +/// the enumerator does not over-issue; congruence was audited per call site and +/// the one known divergence (`PayCost { kind: Sacrifice, resume: ManaAbility }`, +/// whose enumerator issues `min_count..=count` against a handler that demands +/// exactly `count`) leaves this arm byte-identical to its previous behaviour +/// rather than making it worse. +fn issued_selection(contract: &AiDecisionContract) -> Option { + contract + .candidates + .iter() + .filter_map(|candidate| match &candidate.action { + GameAction::SelectCards { cards } => Some((cards.len(), &candidate.action)), + _ => None, + }) + .min_by(|(left_len, left), (right_len, right)| { + left_len.cmp(right_len).then_with(|| left.cmp_stable(right)) + }) + .map(|(_, action)| action.clone()) +} + /// Produce a safe action when the AI has no scored candidates. /// During combat, submit empty declarations. During active play, pass priority. /// Returns None only for terminal states (GameOver) where no action is possible. @@ -871,7 +907,17 @@ pub fn emit_trace_for_candidate( /// /// `config` supplies policy penalties used by selection escapes (e.g. sacrifice /// value ordering); difficulty/search knobs are unused here. -pub fn fallback_action(state: &GameState, config: &AiConfig) -> Option { +/// +/// `contract` is the engine-issued action domain for this decision — the SAME +/// instance that will gate whatever this function returns. Selection escapes +/// answer out of it via [`issued_selection`] rather than synthesizing a +/// cardinality of their own (#6942), and its `semantic_owner` is the authority +/// for which pending seat a multi-seat prompt is being answered for. +pub fn fallback_action( + state: &GameState, + config: &AiConfig, + contract: &AiDecisionContract, +) -> Option { // CR 605.3b: A sacrificial mana prompt is an explicit payment decision, // not a generic pending-cast failure. Pick only an engine-issued source or // the exact BackToManaPayment escape; never synthesize CancelCast here. @@ -1068,7 +1114,18 @@ pub fn fallback_action(state: &GameState, config: &AiConfig) -> Option Option { - Some(GameAction::SelectCards { cards: Vec::new() }) - } + | WaitingFor::UnlessBounceChoice { .. } => issued_selection(contract), // CR 701.4a + CR 608.2d: Behold requires EXACTLY one beholdable object — // an empty selection is illegal. Take the first candidate (any legal pick // resolves the prompt; the evaluated candidate enumerator picks properly). @@ -1243,10 +1298,21 @@ pub fn fallback_action(state: &GameState, config: &AiConfig) -> Option { - let entry = pending.first()?; + // CR 103.5: `pending` may hold several seats at once and their + // phases advance independently (`mulligan.rs:286` removes a settled + // entry, `:295` moves only `pending[idx]` to `BottomCards`), so the + // first entry is not this contract's entry. Select by seat, + // mirroring `deterministic_choice` at search.rs:3017. + let entry = pending + .iter() + .find(|entry| entry.player == contract.semantic_owner)?; match &entry.phase { MulliganDecisionPhase::Declare => { Some(match first_serum_powder_in_hand(state, entry.player) { @@ -1258,14 +1324,12 @@ pub fn fallback_action(state: &GameState, config: &AiConfig) -> Option { - Some(GameAction::SelectCards { cards: Vec::new() }) - } + MulliganDecisionPhase::BottomCards { .. } => issued_selection(contract), } } - WaitingFor::OpeningHandBottomCards { .. } => { - Some(GameAction::SelectCards { cards: Vec::new() }) - } + // TL:R 906.6a/e + CR 103.5: same per-seat owed count, same shared + // `validate_bottom_selection` rejection of an empty pick. + WaitingFor::OpeningHandBottomCards { .. } => issued_selection(contract), // Named choice: prefer an engine-legal ChooseOption. CardName prompts // intentionally keep `options` empty and synthesize candidates from @@ -1763,12 +1827,21 @@ pub fn fallback_action(state: &GameState, config: &AiConfig) -> Option Some(GameAction::SelectCards { cards: Vec::new() }), + } => issued_selection(contract), WaitingFor::PayCost { resume: CostResume::Resolution, .. @@ -3474,6 +3547,52 @@ pub(crate) fn deterministic_choice( } else { scored.sort_by(|a, b| cmp_keep(&a.1, &b.1)); } + + // #6942: rank the engine's OWN issued selections rather than + // synthesizing one. `AiDecisionContract::contains_action` is exact set + // membership (`ai_support/context.rs:100-105`) and the enumeration is + // capped at 64 combinations in lexicographic order + // (`ai_support/candidates.rs:5083-5096`), so a `cmp_keep`-optimal triple + // synthesized from `cards` can be outside the contract — and this path + // populates `scored` at search.rs:2657, which SKIPS the `fallback_action` + // escape entirely, so the whole decision degrades to "no action". + // Scoring the issued actions by their `cmp_keep` rank vector keeps the + // give-up order above (including the CR 723.5 inversion) while making + // membership hold by construction: the ideal pick, when it is issued, + // has rank vector `[0, 1, .., count-1]` and still wins. + let rank_vector = |cards: &[ObjectId]| { + let mut ranks: Vec<_> = cards + .iter() + .map(|card| { + scored + .iter() + .position(|(id, _)| id == card) + .unwrap_or(usize::MAX) + }) + .collect(); + ranks.sort_unstable(); + ranks + }; + let best_issued = actions + .iter() + .filter_map(|action| match action { + GameAction::SelectCards { cards } => Some((rank_vector(cards), action)), + _ => None, + }) + .min_by(|(left_ranks, left), (right_ranks, right)| { + left_ranks + .cmp(right_ranks) + .then_with(|| left.cmp_stable(right)) + }) + .map(|(_, action)| action.clone()); + if let Some(action) = best_issued { + return Some(action); + } + + // No issued `SelectCards` to rank — the rollout quiescence loop + // (`planner/mod.rs:1112-1125`) passes a possibly-empty candidate list and + // has no contract gate, so the synthesized pick is the best available + // answer there and is validated by the caller's own apply probe. let to_discard: Vec<_> = scored.iter().take(*count).map(|(id, _)| *id).collect(); return Some(GameAction::SelectCards { cards: to_discard }); } @@ -4692,9 +4811,28 @@ mod tests { fallback_action( state, &create_config(AiDifficulty::VeryHard, Platform::Native), + &test_contract(state), ) } + /// Issue the decision contract for the seat a test state is prompting. + /// + /// Test-harness seat selector ONLY. Production has exactly one caller + /// (`choose_action_with_session_inner`), which passes the contract it + /// already issued for `ai_player`; nothing derives a seat there. This + /// mirrors `build_decision_context`'s derivation + /// (`ai_support/context.rs:155-158`) so single-seat fixtures need no + /// per-test plumbing; multi-seat rows (T5, T11) issue their own contract + /// for the seat under test instead of using this. + fn test_contract(state: &GameState) -> AiDecisionContract { + let owner = state + .waiting_for + .acting_player() + .or_else(|| state.waiting_for.acting_players().first().copied()) + .unwrap_or(P0); + AiDecisionContract::issue(state, owner) + } + fn resolution_choice_source(state: &GameState, object_id: ObjectId) -> NamedChoiceSource { let context = engine::game::triggers::trigger_source_context_for_latch( state, @@ -6279,7 +6417,7 @@ mod tests { count: 1, }; assert_eq!( - fallback_action(&state, &config), + fallback_action(&state, &config, &test_contract(&state)), Some(expected.clone()), "the fallback consumes the same resolving-effect chooser" ); @@ -6394,7 +6532,7 @@ mod tests { let config = create_config(AiDifficulty::Medium, Platform::Native); assert_eq!( - fallback_action(&state, &config), + fallback_action(&state, &config, &test_contract(&state)), Some(expected.clone()), "the direct fallback selects WB, rather than repeating the higher raw black demand" ); @@ -7571,7 +7709,7 @@ mod tests { ); let config = create_config(AiDifficulty::Medium, Platform::Native); assert_eq!( - fallback_action(&state, &config), + fallback_action(&state, &config, &test_contract(&state)), Some(mana_product(&[ManaType::Red])), "mana-ability fallback follows the engine prompt order; exact-payment reachability remains the engine candidate path's authority" ); @@ -10988,7 +11126,7 @@ mod tests { let config = create_config(AiDifficulty::VeryHard, Platform::Native); assert_eq!( - fallback_action(&state, &config), + fallback_action(&state, &config, &test_contract(&state)), Some(GameAction::SelectCards { cards: vec![creature] }), @@ -11035,4 +11173,816 @@ mod tests { other => panic!("expected SelectCards from the DigChoice arm, got {other:?}"), } } + + // ====================================================================== + // Issue #6942 — selection escapes must answer out of the issued contract. + // + // Every row below drives `fallback_action` (or `deterministic_choice`) — + // the real decision entry points — and asserts against + // `AiDecisionContract::contains_action`, the gate that actually refused the + // synthesized answer in production, plus `apply_as_current` where the + // prompt's handler is reachable from a hand-built state. + // ====================================================================== + + /// A resolution-time `SelectCards` prompt with a hand-sized pool. + fn hand_pool(state: &mut GameState, player: PlayerId, count: usize) -> Vec { + (0..count).map(|_| vanilla_in_hand(state, player)).collect() + } + + /// A minimal resolved ability to hang a `pending_effect` off. + fn stub_pending_effect(source: ObjectId, controller: PlayerId) -> Box { + Box::new(ResolvedAbility::new( + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + Vec::new(), + source, + controller, + )) + } + + /// T1. MAIN TEST, and the reporter's captured shape: a cleanup discard + /// (CR 514.1) owing 3 from a 10-card hand. + /// + /// FAILS AT `base + Step 1 + Step 2`: the arm literally constructs + /// `SelectCards { cards: Vec::new() }`, which `engine_resolution_choices` + /// rejects with "Must discard exactly 3 cards, got 0". + /// + /// The `contains_action` assertion is the one the 64-candidate cap makes + /// non-trivial: a cardinality-only fix can satisfy `apply` and still be + /// refused by the contract, which is what degrades to "no action". + #[test] + fn fallback_discard_to_hand_size_is_accepted_by_the_engine() { + let mut state = make_state(); + let ai = PlayerId(0); + hand_pool(&mut state, ai, 10); + state.waiting_for = discard_waiting_for(&state, ai, 3); + + let contract = AiDecisionContract::issue(&state, ai); + let action = fallback_action_default(&state) + .expect("a cleanup discard the engine will accept must exist"); + match &action { + GameAction::SelectCards { cards } => assert_eq!( + cards.len(), + 3, + "CR 514.1 owes exactly 3; an empty or short pick is rejected" + ), + other => panic!("expected SelectCards, got {other:?}"), + } + assert!( + contract.contains_action(&state, &action), + "the answer must be inside the contract that gates it (#6942)" + ); + assert!( + engine::game::engine::apply_as_current(&mut state, action).is_ok(), + "the engine must accept the fallback's cleanup discard" + ); + } + + /// Build one minimal state per `SelectCards`-answering variant this change + /// converts: the 14 in the shared arm plus the two mulligan siblings. + /// + /// `EffectZoneChoice` deliberately pins a NON-`Sacrifice` `effect_kind`: + /// the earlier `pick_lowest_value_sacrifices` arm intercepts + /// `Sacrifice && !cards.is_empty() && !up_to && count > 0`, so a `Sacrifice` + /// fixture never reaches the delegating arm and the row would pass + /// vacuously. + fn contract_membership_rows() -> Vec<(&'static str, GameState, PlayerId)> { + use engine::types::game_state::{ + MulliganBottomEntry, MulliganDecisionEntry, OpeningHandBottomReason, + }; + + let ai = PlayerId(0); + let mut rows: Vec<(&'static str, GameState, PlayerId)> = Vec::new(); + + let mut push = |name: &'static str, build: &dyn Fn(&mut GameState) -> WaitingFor| { + let mut state = make_state(); + let waiting_for = build(&mut state); + state.waiting_for = waiting_for; + rows.push((name, state, ai)); + }; + + push("ScryChoice", &|state| WaitingFor::ScryChoice { + player: PlayerId(0), + cards: hand_pool(state, PlayerId(0), 2), + }); + push("DigChoice", &|state| { + let pool = hand_pool(state, PlayerId(0), 3); + WaitingFor::DigChoice { + player: PlayerId(0), + library_owner: PlayerId(0), + cards: pool.clone(), + // Filter matched one of three while `keep_count` is 2 — the + // shape Step 6's handler clamp makes answerable at all. + keep_count: 2, + up_to: false, + selectable_cards: vec![pool[0]], + kept_destination: None, + rest_destination: None, + source_id: None, + enter_tapped: false, + } + }); + push("SurveilChoice", &|state| WaitingFor::SurveilChoice { + player: PlayerId(0), + cards: hand_pool(state, PlayerId(0), 2), + }); + push("RevealChoice", &|state| WaitingFor::RevealChoice { + player: PlayerId(0), + cards: hand_pool(state, PlayerId(0), 2), + filter: TargetFilter::Any, + optional: false, + decline_runs_continuation: false, + }); + push("SearchChoice", &|state| WaitingFor::SearchChoice { + player: PlayerId(0), + library_owner: None, + cards: hand_pool(state, PlayerId(0), 3), + count: 1, + reveal: false, + up_to: false, + allows_partial_find: false, + constraint: engine::types::ability::SearchSelectionConstraint::None, + split: None, + }); + push("ChooseFromZoneChoice", &|state| { + let source = vanilla_in_hand(state, PlayerId(0)); + WaitingFor::ChooseFromZoneChoice { + player: PlayerId(0), + cards: hand_pool(state, PlayerId(0), 3), + count: 1, + up_to: false, + constraint: None, + source_id: source, + } + }); + push("DiscardChoice", &|state| { + let source = vanilla_in_hand(state, PlayerId(0)); + WaitingFor::DiscardChoice { + player: PlayerId(0), + count: 1, + cards: hand_pool(state, PlayerId(0), 3), + source_id: source, + effect_kind: EffectKind::DiscardCard, + up_to: false, + unless_filter: None, + } + }); + push("EffectZoneChoice", &|state| { + let source = vanilla_in_hand(state, PlayerId(0)); + WaitingFor::EffectZoneChoice { + player: PlayerId(0), + cards: hand_pool(state, PlayerId(0), 3), + count: 1, + min_count: 1, + up_to: false, + source_id: source, + // NOT Sacrifice — see the doc comment above. + effect_kind: EffectKind::ChangeZone, + zone: Zone::Hand, + destination: Some(Zone::Battlefield), + enter_tapped: engine::types::zones::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, + } + }); + push("ConniveDiscard", &|state| { + let conniver_card = CardId(state.next_object_id); + let conniver_id = create_object( + state, + conniver_card, + PlayerId(0), + "Conniver".to_string(), + Zone::Battlefield, + ); + let conniver = state + .capture_connive_subject(conniver_id) + .expect("a battlefield object yields a connive subject"); + WaitingFor::ConniveDiscard { + player: PlayerId(0), + conniver, + source_id: conniver_id, + cards: hand_pool(state, PlayerId(0), 3), + count: 1, + } + }); + push("DiscardToHandSize", &|state| { + let cards = hand_pool(state, PlayerId(0), 3); + WaitingFor::DiscardToHandSize { + player: PlayerId(0), + count: 1, + cards, + } + }); + push("ManifestDreadChoice", &|state| { + let source = vanilla_in_hand(state, PlayerId(0)); + WaitingFor::ManifestDreadChoice { + player: PlayerId(0), + cards: hand_pool(state, PlayerId(0), 2), + source_id: source, + } + }); + push("WardDiscardChoice", &|state| { + let source = vanilla_in_hand(state, PlayerId(0)); + WaitingFor::WardDiscardChoice { + player: PlayerId(0), + cards: hand_pool(state, PlayerId(0), 3), + pending_effect: stub_pending_effect(source, PlayerId(1)), + remaining: 1, + filter: None, + } + }); + push("WardSacrificeChoice", &|state| { + let source = vanilla_in_hand(state, PlayerId(0)); + let permanents: Vec<_> = (0..3) + .map(|_| add_creature(state, PlayerId(0), 1, 1)) + .collect(); + WaitingFor::WardSacrificeChoice { + player: PlayerId(0), + permanents, + pending_effect: stub_pending_effect(source, PlayerId(1)), + remaining: 1, + min_total_power: None, + } + }); + push("UnlessBounceChoice", &|state| { + let source = vanilla_in_hand(state, PlayerId(0)); + let permanents: Vec<_> = (0..3) + .map(|_| add_creature(state, PlayerId(0), 1, 1)) + .collect(); + WaitingFor::UnlessBounceChoice { + player: PlayerId(0), + permanents, + pending_effect: stub_pending_effect(source, PlayerId(1)), + remaining: 1, + } + }); + push("MulliganDecision::BottomCards", &|state| { + hand_pool(state, PlayerId(0), 7); + WaitingFor::MulliganDecision { + pending: vec![MulliganDecisionEntry { + player: PlayerId(0), + mulligan_count: 2, + phase: MulliganDecisionPhase::BottomCards { + count: 2, + then: PendingMulliganAction::Keep, + }, + }], + free_first_mulligan: false, + } + }); + push("OpeningHandBottomCards", &|state| { + hand_pool(state, PlayerId(0), 7); + WaitingFor::OpeningHandBottomCards { + pending: vec![MulliganBottomEntry { + player: PlayerId(0), + count: 2, + }], + reason: OpeningHandBottomReason::TinyLeadersMultiCommander, + } + }); + + rows + } + + /// T2. Structural invariant across every converted arm: the escape never + /// emits a selection the gating contract refuses. + /// + /// FAILS AT `base + Step 1 + Step 2` for 11 of the 16 rows — the 6 + /// unconditional-rejection prompts, the 3 constructed with `up_to: false`, + /// and both mulligan rows, all of which issue no empty candidate. + /// + /// Post-fix this is definitional for the delegating arms rather than a + /// proof; its standing value is catching a 17th variant added to the arm + /// that bypasses the helper, and the `count_from_contract` check below is + /// what keeps each row non-vacuous. + #[test] + fn fallback_never_emits_a_selection_the_contract_refuses() { + // Collect every offending row rather than aborting on the first. A + // parameterized guard that stops at row 1 reports a SAMPLE; the point of + // this row is the CENSUS, both when it goes red on a reverted tree and + // when a future 17th variant bypasses the helper. + let mut refused = Vec::new(); + for (name, state, seat) in contract_membership_rows() { + let contract = AiDecisionContract::issue(&state, seat); + assert!( + !contract.candidates.is_empty(), + "{name}: fixture premise broken — the engine issued no candidate \ + at all, so this row cannot discriminate" + ); + let action = fallback_action_default(&state) + .unwrap_or_else(|| panic!("{name}: the escape must produce an action")); + assert!( + matches!(action, GameAction::SelectCards { .. }), + "{name}: expected the selection arm, got {action:?} — the fixture \ + is being intercepted by an earlier arm" + ); + if !contract.contains_action(&state, &action) { + refused.push(format!("{name} emitted {action:?}")); + } + } + assert!( + refused.is_empty(), + "the escape emitted selections the gating contract refuses (#6942), \ + in {} of 16 rows:\n {}", + refused.len(), + refused.join("\n ") + ); + } + + /// T3. PAIRED POSITIVE REACH-GUARD for T1/T2. `up_to: true` genuinely + /// admits the empty pick, and the enumerator issues sizes `0..=count`, so + /// prefer-smallest must still return it. Green in BOTH directions: this is + /// what proves the fix did not convert a softlock into a wrong-decision bug + /// by always taking the maximum. + #[test] + fn fallback_choose_from_zone_up_to_still_prefers_the_empty_selection() { + let mut state = make_state(); + let ai = PlayerId(0); + let source = vanilla_in_hand(&mut state, ai); + let cards = hand_pool(&mut state, ai, 3); + state.waiting_for = WaitingFor::ChooseFromZoneChoice { + player: ai, + cards, + count: 2, + up_to: true, + constraint: None, + source_id: source, + }; + + assert_eq!( + fallback_action_default(&state), + Some(GameAction::SelectCards { cards: Vec::new() }), + "an `up_to` prompt legally admits nothing, and the conservative pick \ + is still nothing" + ); + } + + /// T4. Hostile sibling for the "exactly one" sub-family. `WardDiscardChoice` + /// admits no empty pick (`engine_payment_choices` rejects `0 != 1`), and the + /// enumerator emits one size-1 candidate per eligible card. + /// + /// FAILS AT `base + Step 1 + Step 2`: the arm returns the empty selection. + #[test] + fn fallback_ward_discard_selects_exactly_one() { + let mut state = make_state(); + let ai = PlayerId(0); + let source = vanilla_in_hand(&mut state, ai); + let cards = hand_pool(&mut state, ai, 3); + state.waiting_for = WaitingFor::WardDiscardChoice { + player: ai, + cards: cards.clone(), + pending_effect: stub_pending_effect(source, PlayerId(1)), + remaining: 1, + filter: None, + }; + + let contract = AiDecisionContract::issue(&state, ai); + let action = fallback_action_default(&state).expect("ward discard must be answerable"); + match &action { + GameAction::SelectCards { cards: chosen } => { + assert_eq!(chosen.len(), 1, "CR 702.21a ward cost owes exactly one"); + assert!( + cards.contains(&chosen[0]), + "the discard must come from the eligible set" + ); + } + other => panic!("expected SelectCards, got {other:?}"), + } + assert!( + contract.contains_action(&state, &action), + "the ward discard must be inside the gating contract" + ); + } + + /// T5. THE MULTI-AUTHORITY HOSTILE FIXTURE. Two seats are pending + /// simultaneously with disjoint hands and different owed counts; the + /// contract is issued for P0. + /// + /// FAILS AT `base + Step 1 + Step 2`: the arm returns an empty selection, + /// which is in NEITHER seat's domain (`bottom_card_actions` emits the empty + /// candidate only when `count == 0 || hand.is_empty()`). It also fails + /// against any design that derives the seat from + /// `acting_players().first()`, which would build P1's domain here. + #[test] + fn fallback_opening_hand_bottom_answers_the_contracts_seat() { + let mut state = make_state(); + let p1_vanilla = two_player_bottom_fixture(&mut state, 5, 2); + state.waiting_for = WaitingFor::OpeningHandBottomCards { + pending: vec![ + engine::types::game_state::MulliganBottomEntry { + player: P0, + count: 1, + }, + engine::types::game_state::MulliganBottomEntry { + player: P1, + count: 2, + }, + ], + reason: engine::types::game_state::OpeningHandBottomReason::TinyLeadersMultiCommander, + }; + + let config = create_config(AiDifficulty::VeryHard, Platform::Native); + let p0_contract = AiDecisionContract::issue(&state, P0); + let p1_contract = AiDecisionContract::issue(&state, P1); + assert!( + !p1_contract.candidates.is_empty(), + "fixture premise: P1's domain must be non-empty, or the 'not in P1' \ + assertion below is vacuous" + ); + + let action = fallback_action(&state, &config, &p0_contract) + .expect("P0 owes a bottom and must be answerable"); + match &action { + GameAction::SelectCards { cards } => { + assert_eq!(cards.len(), 1, "P0 owes 1, not P1's 2"); + let p0_hand = &state.players[P0.0 as usize].hand; + assert!( + cards.iter().all(|id| p0_hand.contains(id)), + "every bottomed card must come from P0's own hand" + ); + assert!( + !cards.iter().any(|id| p1_vanilla.contains(id)), + "P0's answer must not reach into P1's hand" + ); + } + other => panic!("expected SelectCards, got {other:?}"), + } + assert!( + p0_contract.contains_action(&state, &action), + "the answer must be in the seat's own issued domain" + ); + assert!( + !p1_contract.contains_action(&state, &action), + "the answer must NOT be legal for the other pending seat — that is \ + the seat axis this fixture exists to discriminate" + ); + } + + /// A 10-card `DiscardToHandSize` hand whose two lexicographically-first + /// cards are the two the give-up order wants to KEEP. + /// + /// `combinations` is strict-lexicographic and the enumeration stops at + /// `SELECTION_CANDIDATE_CAP` (64), and C(10,3) = 120. The first 64 combos + /// are C(9,2) = 36 (containing `cards[0]`) + C(8,2) = 28 (containing + /// `cards[1]`) — i.e. exactly the combos touching one of the first two + /// cards. So the `cmp_keep`-optimal triple `{cards[2], cards[3], cards[4]}` + /// is provably OUTSIDE the contract. + fn out_of_contract_discard_state() -> (GameState, Vec) { + let mut state = make_state(); + let ai = PlayerId(0); + let mut cards = vec![fatty_in_hand(&mut state, ai), fatty_in_hand(&mut state, ai)]; + // Distinct, strictly increasing intrinsic values (generic cost i * 0.5) + // so the give-up ranking is total and the assertions are unambiguous. + for generic in 2..10u32 { + let id = junk_instant_in_hand(&mut state, ai); + set_cost(&mut state, id, Vec::new(), generic); + cards.push(id); + } + state.waiting_for = discard_waiting_for(&state, ai, 3); + (state, cards) + } + + /// T6. The SECOND softlock path, distinct from `fallback_action`: a + /// `deterministic_choice` result becomes `vec![(action, 1.0)]`, so `scored` + /// is non-empty and the fallback escape is SKIPPED entirely — the whole + /// decision then degrades to `None` at the contract gate. + /// + /// OBSERVABLE RED AT BARE BASE: it calls `deterministic_choice`, whose + /// signature no step changes. + #[test] + fn deterministic_discard_choice_stays_within_the_issued_candidates() { + let (state, cards) = out_of_contract_discard_state(); + let ai = PlayerId(0); + let config = create_config(AiDifficulty::VeryHard, Platform::Native); + let contract = AiDecisionContract::issue(&state, ai); + + // Fixture premise: the ideal synthesized pick is genuinely unreachable + // through the contract. Without this the test cannot discriminate. + let ideal = GameAction::SelectCards { + cards: vec![cards[2], cards[3], cards[4]], + }; + assert!( + !contract.contains_action(&state, &ideal), + "fixture premise broken: the cap no longer excludes the optimal \ + triple, so this row would pass on base" + ); + + let actions: Vec = validated_candidate_actions_for_semantic_owner(&state, ai) + .into_iter() + .map(|candidate| candidate.action) + .collect(); + assert!( + !actions.is_empty(), + "fixture premise: the pipeline must offer candidates to rank" + ); + + let action = deterministic_choice(&state, ai, &config, &actions, None) + .expect("the discard prompt is always answerable"); + assert!( + contract.contains_action(&state, &action), + "the discard pick must be a member of the issued domain (#6942)" + ); + + // Not merely *a* member — the BEST-ranked member, so Step 5 cannot + // degenerate into "take the first candidate". + // + // The give-up order is derived here from `intrinsic_value` rather than + // from `cmp_keep`, so the expectation is INDEPENDENT of the code under + // test rather than a restatement of it. That is sound and not a second + // authority: `deterministic_choice` is called with `context: None`, so + // no plan exists, every card is `KeepTier::Ordinary`, and `card_value` + // documents that the `(Ordinary, intrinsic)` key then orders identically + // to `intrinsic` alone. `sort_by` is stable in both places, so the two + // 15.5-valued creatures keep their fixture order on both sides. + let mut give_up_order = cards.clone(); + give_up_order.sort_by(|left, right| { + crate::card_value::intrinsic_value(&state, *left) + .partial_cmp(&crate::card_value::intrinsic_value(&state, *right)) + .unwrap_or(std::cmp::Ordering::Equal) + }); + let rank = |id: &ObjectId| { + give_up_order + .iter() + .position(|card| card == id) + .expect("every pick comes from the fixture hand") + }; + let rank_of = |cards: &[ObjectId]| { + let mut ranks: Vec<_> = cards.iter().map(&rank).collect(); + ranks.sort_unstable(); + ranks + }; + let best = actions + .iter() + .filter_map(|candidate| match candidate { + GameAction::SelectCards { cards } => Some(rank_of(cards)), + _ => None, + }) + .min() + .expect("the pipeline offers SelectCards candidates"); + match &action { + GameAction::SelectCards { cards: chosen } => assert_eq!( + rank_of(chosen), + best, + "the ranked pick must be the best `cmp_keep` member of the issued \ + set, not merely inside it" + ), + other => panic!("expected SelectCards, got {other:?}"), + } + } + + /// T8. REACH-GUARD, green in both directions. When the engine issues no + /// selection at all, `None` is the honest answer. + /// + /// The second assertion is what makes this non-vacuous: it proves the + /// `None` came from an empty issued domain rather than from an upstream + /// short-circuit that never reached the arm. + #[test] + fn fallback_returns_none_when_the_contract_issues_no_selection() { + let mut state = make_state(); + let ai = PlayerId(0); + // Owes 3 with nothing to give: `bounded_combinations_for_sizes` skips + // every size larger than the item pool, so the domain is empty. + state.waiting_for = WaitingFor::DiscardToHandSize { + player: ai, + count: 3, + cards: Vec::new(), + }; + + assert!( + AiDecisionContract::issue(&state, ai).candidates.is_empty(), + "reach-guard premise: the engine must issue nothing here" + ); + assert_eq!( + fallback_action_default(&state), + None, + "with no issued selection the escape must decline rather than \ + fabricate one" + ); + } + + /// T9. The `PayCost { resume: ManaAbility }` sibling. `PayCostKind::Discard` + /// is chosen deliberately over `Sacrifice`: the `Sacrifice` enumerator arm + /// issues `min_count..=count` against a handler demanding exactly `count`, + /// so that one shape is inert under this change (disclosed, not fixed here). + /// + /// FAILS AT `base + Step 1 + Step 2`: the arm returns an empty selection and + /// `handle_discard_for_mana_ability` rejects "Must discard exactly 1 + /// card(s), got 0". + #[test] + fn fallback_pay_cost_mana_ability_discard_selects_exactly_count() { + use engine::types::game_state::{ManaAbilityResume, PayCostKind, PendingManaAbility}; + + let mut state = make_state(); + let ai = PlayerId(0); + let source_card = CardId(state.next_object_id); + let source = create_object( + &mut state, + source_card, + ai, + "Discard Rock".to_string(), + Zone::Battlefield, + ); + let mut ability = AbilityDefinition::new( + AbilityKind::Activated, + Effect::Mana { + produced: ManaProduction::Fixed { + colors: vec![ManaColor::Black], + contribution: engine::types::ability::ManaContribution::Base, + }, + restrictions: vec![], + grants: vec![], + expiry: None, + target: None, + }, + ); + ability.cost = Some(AbilityCost::Discard { + count: QuantityExpr::Fixed { value: 1 }, + filter: None, + selection: engine::types::ability::CardSelectionMode::Chosen, + self_scope: engine::types::ability::DiscardSelfScope::default(), + }); + Arc::make_mut(&mut state.objects.get_mut(&source).unwrap().abilities).push(ability.clone()); + + let hand = hand_pool(&mut state, ai, 3); + state.waiting_for = WaitingFor::PayCost { + player: ai, + kind: PayCostKind::Discard, + choices: hand.clone(), + count: 1, + min_count: 0, + resume: CostResume::ManaAbility { + mana_ability: Box::new(PendingManaAbility { + player: ai, + source_id: source, + ability_index: Some(0), + rules_execution_node: None, + ability_snapshot: Some(ability), + color_override: None, + resume: ManaAbilityResume::Priority, + cost_move_resume: None, + chosen_tappers: Vec::new(), + chosen_discards: Vec::new(), + chosen_mana_payment: None, + chosen_counter_count: None, + chosen_x: None, + collected_evidence: Vec::new(), + chosen_exiled: Vec::new(), + chosen_sacrificed_battlefield: Vec::new(), + cost_paid_object: None, + batch_siblings: Vec::new(), + }), + }, + }; + + let contract = AiDecisionContract::issue(&state, ai); + let action = + fallback_action_default(&state).expect("a mana-ability cost must be answerable"); + match &action { + GameAction::SelectCards { cards } => { + assert_eq!( + cards.len(), + 1, + "CR 118.3: the cost is paid in full or not at all" + ); + assert!( + hand.contains(&cards[0]), + "the discard must come from the offered choices" + ); + } + other => panic!("expected SelectCards, got {other:?}"), + } + assert!( + contract.contains_action(&state, &action), + "the cost payment must be inside the gating contract" + ); + assert!( + engine::game::engine::apply_as_current(&mut state, action).is_ok(), + "the engine must accept the fallback's mana-ability cost payment" + ); + } + + /// T10. The single-seat mulligan-bottom row. + /// + /// FAILS AT `base + Step 1 + Step 2`: the arm returns empty and + /// `validate_bottom_selection` rejects "Expected 2 cards to bottom, got 0". + /// + /// With one pending entry, `pending.first()` and the seat lookup are the + /// same entry by construction, so this row is deliberately BLIND to the + /// seat-source defect — that axis belongs to T11. + #[test] + fn fallback_mulligan_bottom_cards_selects_the_owed_count() { + let mut state = make_state(); + let hand = hand_pool(&mut state, P1, 7); + state.waiting_for = WaitingFor::MulliganDecision { + pending: vec![engine::types::game_state::MulliganDecisionEntry { + player: P1, + mulligan_count: 2, + phase: MulliganDecisionPhase::BottomCards { + count: 2, + then: PendingMulliganAction::Keep, + }, + }], + free_first_mulligan: false, + }; + + let config = create_config(AiDifficulty::VeryHard, Platform::Native); + let contract = AiDecisionContract::issue(&state, P1); + let action = fallback_action(&state, &config, &contract) + .expect("the owed bottom must be answerable"); + match &action { + GameAction::SelectCards { cards } => { + assert_eq!(cards.len(), 2, "CR 103.5: the owed count is per-seat"); + assert!( + cards.iter().all(|id| hand.contains(id)), + "every bottomed card must come from that seat's hand" + ); + } + other => panic!("expected SelectCards, got {other:?}"), + } + assert!( + contract.contains_action(&state, &action), + "the bottom selection must be inside the gating contract" + ); + } + + /// T11. THE SEAT-SOURCE FIXTURE — a MIXED-PHASE `MulliganDecision` + /// (`[{P0, Declare}, {P1, BottomCards}]`) with the contract issued for P1. + /// + /// Reachable, not merely constructible: `mulligan.rs` removes a settled + /// entry and moves only `pending[idx]` to `BottomCards`, so phases advance + /// per-seat and P1 declaring before P0 leaves exactly this shape. + /// + /// REVERT BASELINE: `base + Steps 1, 2 and 3b-WITHOUT-the-seat-fix`. On that + /// tree `pending.first()?` binds P0's entry, the match takes `Declare`, and + /// the arm returns `MulliganDecision { choice: Keep }` — a wrong-seat, + /// wrong-KIND action that never reaches the delegation at all. Observing + /// this row red at `base + 1 + 2` would prove nothing except that Step 3b is + /// absent. + /// + /// T10 cannot discriminate here: its single-entry fixture makes positional + /// and semantic seat selection agree by construction. + #[test] + fn fallback_mulligan_bottom_answers_the_contracts_seat_not_the_first_pending() { + let mut state = make_state(); + let p0_hand = hand_pool(&mut state, P0, 7); + let p1_hand = hand_pool(&mut state, P1, 7); + state.waiting_for = WaitingFor::MulliganDecision { + pending: vec![ + engine::types::game_state::MulliganDecisionEntry { + player: P0, + mulligan_count: 0, + phase: MulliganDecisionPhase::Declare, + }, + engine::types::game_state::MulliganDecisionEntry { + player: P1, + mulligan_count: 2, + phase: MulliganDecisionPhase::BottomCards { + count: 2, + then: PendingMulliganAction::Keep, + }, + }, + ], + free_first_mulligan: false, + }; + + let config = create_config(AiDifficulty::VeryHard, Platform::Native); + let contract = AiDecisionContract::issue(&state, P1); + let action = fallback_action(&state, &config, &contract) + .expect("P1 owes a bottom and must be answerable"); + match &action { + GameAction::SelectCards { cards } => { + assert_eq!(cards.len(), 2, "P1's own owed count"); + assert!( + cards.iter().all(|id| p1_hand.contains(id)), + "every bottomed card must come from P1's hand" + ); + assert!( + !cards.iter().any(|id| p0_hand.contains(id)), + "no card may come from the FIRST pending seat's hand" + ); + } + other => panic!( + "expected a SelectCards for P1's BottomCards phase, got {other:?} \ + — the arm dispatched on the first pending entry's phase instead \ + of the contract's seat" + ), + } + assert!( + contract.contains_action(&state, &action), + "the answer must be in P1's issued domain" + ); + } }