From 137e0bc3eaf0062aa3d8353fdba03f8f14ec9a4b Mon Sep 17 00:00:00 2001 From: minion1227 Date: Wed, 29 Jul 2026 04:04:46 -0700 Subject: [PATCH 1/4] feat(phase-ai): add discard-matters deck-feature axis + DiscardPayoffPolicy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CR 701.9: a deck built on Archfiend of Ifnir, Bone Miser, Waste Not or Containment Construct *wants* to discard — every card pitched is a repeatable value trigger. The AI has the opposite instinct: `card_advantage` scores a card leaving hand as a loss, and nothing credits the trigger it fires. So the AI declines its own engine, routing around rummaging outlets and treating a discard cost as pure downside even when the discard IS the payoff. Feature (`features/discard_matters.rs`), structural over `CardFace` AST: - `source_count` — self-discard outlets. BOTH enabler spellings are read: `Effect::Discard` (rich form) and `Effect::DiscardCard` (older simple form). They are separate `Effect` variants with separate resolver paths and real cards use each, so classifying only one would silently drop half the class. - `payoff_count` — `TriggerMode::Discarded` / `DiscardedAll` engines, keeping only controller-scoped ones (a "whenever an opponent discards" punisher is a different deck) and excluding `SelfRef` triggers, so a pile of madness cards cannot masquerade as an engine base. - `commitment` — geometric mean over (source, payoff): both pillars mandatory. Discard with no engine is pure card disadvantage and the AI is RIGHT to avoid it; this axis must not push it to. Policy (`policies/discard_payoff.rs`, `CastSpell` + `ActivateAbility`) credits a discard only when a LIVE engine is on the battlefield, preflighted through the engine's `hypothetical_trigger_fireable` authority so a rate-limited, off-timing or no-legal-target engine is not credited value it cannot produce. The card-local check runs first, so every non-discard candidate is rejected after reading one card's AST — no board sweep, no affordability query in the search inner loop. Boundaries, stated in the module docs so they are reviewable rather than inferred: - `hand_disruption` scores OPPONENT discard (`TargetFilter::Opponent`); this axis is the disjoint half (`Controller`). The scope that qualifies here is exactly the one that disqualifies there, so the two never read the same card. - `cycling_discipline` owns when to pay a cycling cost. `TriggerMode:: CycledOrDiscarded` is deliberately NOT read here — counting it would double-score one card across two policies with different intents. Calibration anchors are computed, not asserted from intuition: 10 outlets + 4 engines over 36 nonland → 0.878; 6 + 2 → 0.481; incidental 2 + 1 → 0.196, below the 0.35 floor. Each is pinned by a test to the value the formula returns. Both scoring scalars land in `UNTUNED_POLICY_PENALTY_FIELDS` pending a paired-seed calibration. 60 tests. Five are proven RED by breaking the specific invariant each guards: dropping the `Effect::DiscardCard` sibling (fails at BOTH the deck-time and live seams), dropping the opponent-scope guard, admitting cycling triggers, and dropping the `SelfRef` madness exclusion. Co-Authored-By: Claude Opus 5 (1M context) --- crates/phase-ai/src/config.rs | 12 + .../phase-ai/src/features/discard_matters.rs | 282 ++++++++++++ crates/phase-ai/src/features/mod.rs | 6 + .../src/features/tests/discard_matters.rs | 332 ++++++++++++++ crates/phase-ai/src/features/tests/mod.rs | 1 + .../phase-ai/src/policies/discard_payoff.rs | 299 ++++++++++++ crates/phase-ai/src/policies/mod.rs | 1 + crates/phase-ai/src/policies/registry.rs | 4 + .../src/policies/tests/discard_payoff.rs | 431 ++++++++++++++++++ crates/phase-ai/src/policies/tests/mod.rs | 1 + 10 files changed, 1369 insertions(+) create mode 100644 crates/phase-ai/src/features/discard_matters.rs create mode 100644 crates/phase-ai/src/features/tests/discard_matters.rs create mode 100644 crates/phase-ai/src/policies/discard_payoff.rs create mode 100644 crates/phase-ai/src/policies/tests/discard_payoff.rs diff --git a/crates/phase-ai/src/config.rs b/crates/phase-ai/src/config.rs index 13f99c6ae0..dc30557a9f 100644 --- a/crates/phase-ai/src/config.rs +++ b/crates/phase-ai/src/config.rs @@ -541,6 +541,10 @@ pub struct PolicyPenalties { /// reducer — the discount should be deployed first. #[serde(default = "default_cost_reduction_defer_penalty")] pub cost_reduction_defer_penalty: f64, + /// CR 701.9: card-equivalent value of discarding into one active "whenever + /// you discard" engine (preference band, per engine). + #[serde(default = "default_discard_payoff_bonus")] + pub discard_payoff_bonus: f64, } impl Default for PolicyPenalties { @@ -618,6 +622,7 @@ impl Default for PolicyPenalties { draw_payoff_bonus: default_draw_payoff_bonus(), cost_reduction_deploy_bonus: default_cost_reduction_deploy_bonus(), cost_reduction_defer_penalty: default_cost_reduction_defer_penalty(), + discard_payoff_bonus: default_discard_payoff_bonus(), } } } @@ -752,6 +757,9 @@ fn default_cost_reduction_deploy_bonus() -> f64 { fn default_cost_reduction_defer_penalty() -> f64 { -0.25 } +fn default_discard_payoff_bonus() -> f64 { + 0.6 +} fn default_sacrifice_token_cost() -> f64 { 0.5 } @@ -909,6 +917,10 @@ pub const UNTUNED_POLICY_PENALTY_FIELDS: &[(&str, &str)] = &[ "CR 601.2f sequencing nudge for casting past a cheaper unplayed reducer — \ awaiting a paired-seed ai-gate calibration.", ), + ( + "discard_payoff_bonus", + "CR 701.9 per-engine discard-payoff weight — awaiting a paired-seed ai-gate calibration.", + ), ( "poison_clock_pressure", "CR 104.3d win-detector weight — a critical-band term whose magnitude is \ diff --git a/crates/phase-ai/src/features/discard_matters.rs b/crates/phase-ai/src/features/discard_matters.rs new file mode 100644 index 0000000000..e7c2ecb83b --- /dev/null +++ b/crates/phase-ai/src/features/discard_matters.rs @@ -0,0 +1,282 @@ +//! Discard-matters feature — structural detection of a deck that discards its +//! OWN cards on purpose, as fuel for a payoff engine. +//! +//! Parser AST verification — VERIFIED against engine source: +//! - `Effect::Discard { count, target, .. }` at +//! `crates/engine/src/types/ability.rs:11384` — the rich enabler form +//! (`QuantityExpr` count, `CardSelectionMode`, unless-filter). +//! - `Effect::DiscardCard { count, target }` at `ability.rs:10311` — the older +//! simple enabler form (`u32` count). BOTH are live in the resolver +//! (`game/effects/discard.rs` and `game/effects/mod.rs`), so a classifier +//! that reads only one silently misses part of the class. +//! - `TriggerMode::Discarded` at `crates/engine/src/types/triggers.rs:321` and +//! `TriggerMode::DiscardedAll` at `triggers.rs:322` (CR 701.9) — the payoffs. +//! - `TriggerDefinition.valid_target` — used to keep only YOUR-discard engines, +//! excluding the "whenever an opponent discards" punisher shape. +//! +//! No parser remediation required — every axis is expressible over existing +//! typed AST. +//! +//! ## Why this axis exists +//! +//! CR 701.9: a deck built on Archfiend of Ifnir, Bone Miser, Waste Not or +//! Containment Construct *wants* to discard — each card pitched is a repeatable +//! value trigger. The AI has the opposite instinct: discarding is a cost, and +//! `card_advantage` scores the card leaving hand as a loss. Nothing values the +//! trigger it fires, so a deck whose entire engine is self-discard reads as +//! having no plan at all. +//! +//! ## Boundary with `hand_disruption` +//! +//! `hand_disruption` scores making an OPPONENT discard (`Effect::Discard` / +//! `Effect::DiscardCard` scoped to `TargetFilter::Opponent`) — stripping their +//! resources. This axis is the disjoint half: YOUR OWN discard +//! (`TargetFilter::Controller`) as fuel. The two never read the same card, +//! because the scope that qualifies here is exactly the one that disqualifies +//! there. +//! +//! ## Boundary with `cycling_discipline` +//! +//! `CyclingDisciplinePolicy` governs *when to pay a cycling cost* — patience +//! about spending a card for a replacement draw. That is a cost-discipline +//! question about one activation. This axis is a deck-composition question: +//! does a discard EVENT fire an engine on the battlefield? `TriggerMode:: +//! CycledOrDiscarded` is deliberately NOT read here — a cycling trigger is +//! `cycling_discipline`'s subject, and counting it would double-score the same +//! card across two policies with different intents. Only the discard-specific +//! modes qualify. + +use engine::game::ability_utils::ability_definition_supported; +use engine::game::quantity::resolve_quantity; +use engine::game::DeckEntry; +use engine::types::ability::{ + AbilityDefinition, Effect, QuantityExpr, TargetFilter, TriggerDefinition, +}; +use engine::types::card_type::CoreType; +use engine::types::game_state::GameState; +use engine::types::identifiers::ObjectId; +use engine::types::player::PlayerId; +use engine::types::triggers::TriggerMode; + +use crate::ability_chain::collect_scoped_effects; +pub(crate) use crate::ability_chain::AbilityScope; +use crate::features::commitment; + +/// Commitment at or above which self-discard is a deliberate engine rather than +/// incidental rummaging. Gates `DiscardPayoffPolicy::activation`. +pub const DISCARD_MATTERS_FLOOR: f32 = 0.35; + +/// CR 701.9: per-deck discard-matters classification. +/// +/// Populated once per game from `DeckEntry` data. Detection is structural over +/// `CardFace.abilities` and `CardFace.triggers` — never by card name. +#[derive(Debug, Clone, Default)] +pub struct DiscardMattersFeature { + /// Cards that discard YOU cards — the enablers that feed the payoff engine. + pub source_count: u32, + /// Permanents carrying a "whenever you discard a card" engine trigger + /// (CR 701.9), controller-scoped and not self-referential — the payoffs that + /// make pitching cards actively good. + pub payoff_count: u32, + /// `0.0..=1.0` — how central discarding-as-a-payoff is to this deck. + /// Consumed by `DiscardPayoffPolicy::activation` as the single scaling knob. + pub commitment: f32, +} + +/// Structural detection over each `DeckEntry`'s `CardFace` AST. +pub fn detect(deck: &[DeckEntry]) -> DiscardMattersFeature { + if deck.is_empty() { + return DiscardMattersFeature::default(); + } + + let mut source_count = 0u32; + let mut payoff_count = 0u32; + let mut total_nonland = 0u32; + + for entry in deck { + let face = &entry.card; + if !face.card_type.core_types.contains(&CoreType::Land) { + total_nonland = total_nonland.saturating_add(entry.count); + } + + // Deck-time: a modal card whose discard lives in a branch still counts as + // an enabler for the archetype, so scan the full potential tree. + if is_discard_source_parts( + &face.abilities, + AbilityScope::Potential, + &DiscardQuantity::Any, + ) { + source_count = source_count.saturating_add(entry.count); + } + if is_discard_payoff_parts(&face.triggers) { + payoff_count = payoff_count.saturating_add(entry.count); + } + } + + let commitment = compute_commitment(source_count, payoff_count, total_nonland); + + DiscardMattersFeature { + source_count, + payoff_count, + commitment, + } +} + +/// Whether a discard instruction's COUNT must be established positive. +/// +/// CR 701.9 + CR 107.1b: "discard N cards" resolves its quantity at resolution, +/// so a count of zero moves no card and emits no discard event — it fires no +/// "whenever you discard" engine. Deck classification and live candidate scoring +/// want different answers about that, so the requirement is a parameter of the +/// one classifier rather than a second forked copy of it. +pub(crate) enum DiscardQuantity<'a> { + /// Deck-time: any discard instruction marks the card regardless of count. A + /// "discard X" or "discard your hand" card is still an enabler for archetype + /// classification — its count is unknowable at deck-build time. + Any, + /// Live candidate: the count must resolve to at least one card *now*. + /// + /// Delegates to the engine's `resolve_quantity` authority rather than + /// re-deriving quantity semantics, so this agrees with the resolver by + /// construction. That also yields the correct conservative behavior for an + /// unbound `X`: it reads `cost_x_paid` off the source and falls back to 0 + /// when X has not been announced yet, so an unbound dynamic discard stays + /// neutral until it is known positive. + ResolvesPositive { + state: &'a GameState, + controller: PlayerId, + source: ObjectId, + }, +} + +impl DiscardQuantity<'_> { + /// CR 701.9: does this discard move at least one card under this requirement? + fn expr_is_satisfied(&self, count: &QuantityExpr) -> bool { + match self { + DiscardQuantity::Any => true, + DiscardQuantity::ResolvesPositive { + state, + controller, + source, + } => resolve_quantity(state, count, *controller, *source) >= 1, + } + } + + /// The `Effect::DiscardCard` sibling carries a plain `u32`, so its count is + /// already concrete and needs no game state to settle. + fn fixed_is_satisfied(&self, count: u32) -> bool { + match self { + DiscardQuantity::Any => true, + DiscardQuantity::ResolvesPositive { .. } => count >= 1, + } + } +} + +/// CR 701.9: the abilities discard YOU one or more cards — a repeatable enabler +/// for the payoff engine. Parts-based so it classifies both a deck-time +/// `CardFace.abilities` slice and the action's runtime effect chain. +/// +/// The caller chooses the `scope`: `Potential` for deck-time (a modal discard +/// mode still marks the card), `Unconditional` for a live candidate before its +/// mode is selected (CR 700.2 — a modal "choose one — discard / …" must NOT be +/// credited a discard until that mode is actually chosen). +/// +/// Both `Effect` spellings are read. They are separate variants with separate +/// resolver paths, and real cards use each, so classifying only the richer one +/// would drop half the class. +pub(crate) fn is_discard_source_parts<'a>( + abilities: impl IntoIterator, + scope: AbilityScope, + quantity: &DiscardQuantity<'_>, +) -> bool { + abilities.into_iter().any(|ability| { + collect_scoped_effects(ability, scope) + .iter() + .any(|effect| match effect { + Effect::Discard { target, count, .. } => { + discards_controller(target) && quantity.expr_is_satisfied(count) + } + Effect::DiscardCard { target, count } => { + discards_controller(target) && quantity.fixed_is_satisfied(*count) + } + _ => false, + }) + }) +} + +/// CR 701.9: the triggers carry a "whenever you discard a card" engine — a +/// repeatable payoff. Parts-based so it classifies both a deck-time +/// `CardFace.triggers` slice and a live `GameObject.trigger_definitions` +/// iterator (the runtime trigger authority). +pub(crate) fn is_discard_payoff_parts<'a>( + triggers: impl IntoIterator, +) -> bool { + triggers.into_iter().any(is_discard_payoff_trigger) +} + +/// Single-trigger structural classifier (mode + scope), exposed so the policy +/// can pair it with live per-turn firing eligibility per trigger entry. +pub(crate) fn is_discard_payoff_trigger(t: &TriggerDefinition) -> bool { + // 1. Mode fires on a discard event (CR 701.9). `DiscardedAll` is the + // "discards their hand" shape (Anje / Containment Construct class) and is + // the same event class. `CycledOrDiscarded` is deliberately excluded — + // see the module docs' `cycling_discipline` boundary. + if !matches!(t.mode, TriggerMode::Discarded | TriggerMode::DiscardedAll) { + return false; + } + // 2. Your-discard only: "whenever an opponent discards" is a punisher for a + // different deck, not a reason for YOU to pitch cards. + if !matches!(&t.valid_target, None | Some(TargetFilter::Controller)) { + return false; + } + // 3. Exclude a self-referential "when this card is discarded" trigger — + // madness and Obsidian-Charmaw-style recursion fire from the card being + // discarded itself, not from a battlefield engine. Those are the + // enabler's own payoff, already priced by the card, and counting them + // would let a pile of madness cards masquerade as an engine base. + if matches!(&t.valid_card, Some(TargetFilter::SelfRef)) { + return false; + } + // 4. The payoff must resolve to a real effect. A missing execute or an + // unsupported one (`TriggerNoExecute` / `Effect::Unimplemented`) produces + // no value, so it is not an engine — the same shared support authority the + // live fireability preflight consults. + t.execute + .as_deref() + .is_some_and(ability_definition_supported) +} + +/// True when the discard instruction makes the controller (you) discard, not an +/// opponent. `TargetFilter::Any` is the serde default on both variants and is +/// NOT treated as "you": an unscoped discard is exactly the ambiguous case, and +/// crediting it would pull `hand_disruption`'s opponent-facing cards into this +/// axis. +fn discards_controller(target: &TargetFilter) -> bool { + matches!(target, TargetFilter::Controller) +} + +/// Calibration (computed, not asserted from intuition — the anchors below are +/// the values `compute_commitment` actually returns): +/// - Realistic Rakdos pitch shell — 10 self-discard outlets + 4 engines +/// (Archfiend of Ifnir / Bone Miser / Waste Not) over 36 nonland → **0.878**. +/// - A lighter build, 6 outlets + 2 engines → 0.481: still a real plan, still +/// above `DISCARD_MATTERS_FLOOR`. +/// +/// Anti-calibration: +/// - Incidental rummaging, 2 outlets + 1 engine over 36 nonland → **0.196**, +/// comfortably below the floor. +/// - Outlets with no engine, or an engine with no outlet → 0.0. +/// +/// Geometric mean over (source, payoff): BOTH pillars are mandatory. Discard +/// with no engine is pure card disadvantage — the AI is RIGHT to avoid it, and +/// this axis must not push it to. An engine with no outlet only ever fires on a +/// cleanup-step discard. +fn compute_commitment(source_count: u32, payoff_count: u32, total_nonland: u32) -> f32 { + // ~18 self-discard outlets per 60 nonland is a fully-committed pitch shell. + let source_density = (commitment::density_per_60(source_count, total_nonland) / 18.0).min(1.0); + // ~8 engine payoffs per 60 nonland is a fully-committed payoff base. Engines + // are scarcer than outlets, but not as scarce as a draw engine: several are + // cheap enchantments/creatures a pitch deck runs in multiples. + let payoff_density = (commitment::density_per_60(payoff_count, total_nonland) / 8.0).min(1.0); + commitment::geometric_mean(&[source_density, payoff_density]) +} diff --git a/crates/phase-ai/src/features/mod.rs b/crates/phase-ai/src/features/mod.rs index 41c0a1414f..278aa182de 100644 --- a/crates/phase-ai/src/features/mod.rs +++ b/crates/phase-ai/src/features/mod.rs @@ -14,6 +14,7 @@ pub mod commitment; pub mod control; pub mod cost_reduction; pub mod devotion; +pub mod discard_matters; pub mod draw_matters; pub mod enchantments; pub mod energy; @@ -40,6 +41,7 @@ pub use blink::BlinkFeature; pub use control::ControlFeature; pub use cost_reduction::CostReductionFeature; pub use devotion::DevotionFeature; +pub use discard_matters::DiscardMattersFeature; pub use draw_matters::DrawMattersFeature; pub use enchantments::EnchantmentsFeature; pub use energy::EnergyFeature; @@ -98,6 +100,9 @@ pub struct DeckFeatures { pub graveyard_types: GraveyardTypesFeature, /// CR 121.1: "whenever you draw" payoff density (draw sources + engines). pub draw_matters: DrawMattersFeature, + /// CR 701.9: "whenever you discard" payoff density (self-discard outlets + + /// engines). Disjoint from `hand_disruption`, which scores OPPONENT discard. + pub discard_matters: DiscardMattersFeature, /// Declaration-derived: the deck's declared bracket tier. Unlike the /// other fields here, this is not structurally detected from card text — /// it is a per-deck declaration set at deck-analysis time from deck @@ -147,6 +152,7 @@ impl DeckFeatures { poison: poison::detect(deck), graveyard_types: graveyard_types::detect(deck), draw_matters: draw_matters::detect(deck), + discard_matters: discard_matters::detect(deck), bracket_tier: tier, } } diff --git a/crates/phase-ai/src/features/tests/discard_matters.rs b/crates/phase-ai/src/features/tests/discard_matters.rs new file mode 100644 index 0000000000..f5323b8ae5 --- /dev/null +++ b/crates/phase-ai/src/features/tests/discard_matters.rs @@ -0,0 +1,332 @@ +//! Unit tests for `features::discard_matters` — CR 701.9 "whenever you discard" +//! detection. No `#[cfg(test)]` in SOURCE files; tests live here. + +use engine::game::DeckEntry; +use engine::types::ability::CardSelectionMode; +use engine::types::ability::{ + AbilityDefinition, AbilityKind, Effect, QuantityExpr, TargetFilter, TriggerDefinition, +}; +use engine::types::card::CardFace; +use engine::types::card_type::{CardType, CoreType}; +use engine::types::triggers::TriggerMode; + +use crate::features::discard_matters::*; + +fn face(name: &str, core: CoreType) -> CardFace { + CardFace { + name: name.to_string(), + card_type: CardType { + supertypes: Vec::new(), + core_types: vec![core], + subtypes: Vec::new(), + }, + ..Default::default() + } +} + +fn entry(card: CardFace, count: u32) -> DeckEntry { + DeckEntry { card, count } +} + +/// The rich enabler form: "discard two cards" scoped to you. +fn discard_source(name: &str) -> CardFace { + let mut f = face(name, CoreType::Sorcery); + f.abilities = vec![AbilityDefinition::new( + AbilityKind::Spell, + Effect::Discard { + count: QuantityExpr::Fixed { value: 2 }, + target: TargetFilter::Controller, + selection: CardSelectionMode::Chosen, + unless_filter: None, + filter: None, + }, + )]; + f +} + +/// The older simple enabler form — a separate `Effect` variant with its own +/// resolver path. Reading only `Effect::Discard` would drop this half. +fn discard_card_source(name: &str) -> CardFace { + let mut f = face(name, CoreType::Sorcery); + f.abilities = vec![AbilityDefinition::new( + AbilityKind::Spell, + Effect::DiscardCard { + count: 1, + target: TargetFilter::Controller, + }, + )]; + f +} + +/// Opponent-facing discard — `hand_disruption`'s subject, not this axis. +fn opponent_discard(name: &str) -> CardFace { + let mut f = face(name, CoreType::Sorcery); + f.abilities = vec![AbilityDefinition::new( + AbilityKind::Spell, + Effect::DiscardCard { + count: 1, + target: TargetFilter::Opponent, + }, + )]; + f +} + +fn payoff_trigger( + mode: TriggerMode, + valid_card: Option, + valid_target: Option, +) -> TriggerDefinition { + let mut t = TriggerDefinition::new(mode); + if let Some(vc) = valid_card { + t = t.valid_card(vc); + } + if let Some(vt) = valid_target { + t = t.valid_target(vt); + } + t.execute(AbilityDefinition::new( + AbilityKind::Spell, + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 1 }, + player: TargetFilter::Controller, + }, + )) +} + +/// The Archfiend of Ifnir / Bone Miser shape. +fn engine_card(name: &str) -> CardFace { + let mut f = face(name, CoreType::Creature); + f.triggers = vec![payoff_trigger(TriggerMode::Discarded, None, None)]; + f +} + +fn vanilla(name: &str) -> CardFace { + face(name, CoreType::Creature) +} + +/// `sources` outlets + `engines` payoffs, padded to 36 nonland. +fn deck(sources: u32, engines: u32) -> Vec { + let filler = 36u32.saturating_sub(sources + engines); + vec![ + entry(discard_source("Outlet"), sources), + entry(engine_card("Engine"), engines), + entry(vanilla("Filler"), filler), + ] +} + +#[test] +fn empty_deck_produces_defaults() { + let f = detect(&[]); + assert_eq!(f.source_count, 0); + assert_eq!(f.payoff_count, 0); + assert_eq!(f.commitment, 0.0); +} + +#[test] +fn vanilla_creature_not_registered() { + let f = detect(&[entry(vanilla("Bear"), 4)]); + assert_eq!(f.source_count, 0); + assert_eq!(f.payoff_count, 0); +} + +#[test] +fn detects_rich_discard_effect_source() { + let f = detect(&deck(18, 5)); + assert_eq!(f.source_count, 18); +} + +#[test] +fn detects_simple_discard_card_effect_source() { + // Sibling-variant coverage: `Effect::DiscardCard` is a distinct enum variant + // with its own resolver path, and must count as an enabler too. + let f = detect(&[ + entry(discard_card_source("Simple Outlet"), 18), + entry(engine_card("Engine"), 5), + entry(vanilla("Filler"), 13), + ]); + assert_eq!(f.source_count, 18); + assert!(f.commitment > 0.0); +} + +#[test] +fn detects_discarded_all_payoff() { + let mut e = face("Hand Dumper", CoreType::Creature); + e.triggers = vec![payoff_trigger(TriggerMode::DiscardedAll, None, None)]; + let f = detect(&[ + entry(discard_source("Outlet"), 18), + entry(e, 5), + entry(vanilla("Filler"), 13), + ]); + assert_eq!(f.payoff_count, 5); +} + +#[test] +fn opponent_discard_is_not_a_source() { + // `hand_disruption`'s domain — the axes must never read the same card. + let f = detect(&[ + entry(opponent_discard("Coercion"), 18), + entry(engine_card("Engine"), 5), + entry(vanilla("Filler"), 13), + ]); + assert_eq!(f.source_count, 0); + assert_eq!(f.commitment, 0.0); +} + +#[test] +fn opponent_scoped_payoff_is_not_an_engine() { + // "Whenever an opponent discards" is a punisher for a different deck. + let mut e = face("Punisher", CoreType::Creature); + e.triggers = vec![payoff_trigger( + TriggerMode::Discarded, + None, + Some(TargetFilter::Opponent), + )]; + let f = detect(&[ + entry(discard_source("Outlet"), 18), + entry(e, 5), + entry(vanilla("Filler"), 13), + ]); + assert_eq!(f.payoff_count, 0); + assert_eq!(f.commitment, 0.0); +} + +#[test] +fn self_referential_discard_trigger_is_not_an_engine() { + // Madness / "when this is discarded" fires from the card being pitched, not + // from a battlefield engine — counting it would let a pile of madness cards + // masquerade as an engine base. + let mut e = face("Madness Card", CoreType::Creature); + e.triggers = vec![payoff_trigger( + TriggerMode::Discarded, + Some(TargetFilter::SelfRef), + None, + )]; + let f = detect(&[ + entry(discard_source("Outlet"), 18), + entry(e, 5), + entry(vanilla("Filler"), 13), + ]); + assert_eq!(f.payoff_count, 0); +} + +#[test] +fn cycling_trigger_is_not_counted() { + // Boundary with `cycling_discipline`: `CycledOrDiscarded` is that policy's + // subject; counting it here would double-score one card across two policies. + let mut e = face("Cycler Payoff", CoreType::Creature); + e.triggers = vec![payoff_trigger(TriggerMode::CycledOrDiscarded, None, None)]; + let f = detect(&[ + entry(discard_source("Outlet"), 18), + entry(e, 5), + entry(vanilla("Filler"), 13), + ]); + assert_eq!(f.payoff_count, 0); +} + +#[test] +fn payoff_without_a_resolvable_execute_is_not_an_engine() { + let mut e = face("No Execute", CoreType::Creature); + e.triggers = vec![TriggerDefinition::new(TriggerMode::Discarded)]; + let f = detect(&[ + entry(discard_source("Outlet"), 18), + entry(e, 5), + entry(vanilla("Filler"), 13), + ]); + assert_eq!(f.payoff_count, 0); +} + +#[test] +fn outlets_without_an_engine_collapse_commitment() { + let f = detect(&[ + entry(discard_source("Outlet"), 18), + entry(vanilla("Filler"), 18), + ]); + assert_eq!(f.payoff_count, 0); + assert_eq!(f.commitment, 0.0); +} + +#[test] +fn engine_without_outlets_collapses_commitment() { + let f = detect(&[ + entry(engine_card("Engine"), 5), + entry(vanilla("Filler"), 31), + ]); + assert_eq!(f.source_count, 0); + assert_eq!(f.commitment, 0.0); +} + +#[test] +fn pitch_shell_hits_calibration_anchor() { + // Docstring anchor: 10 outlets + 4 engines over 36 nonland → 0.878. + let f = detect(&deck(10, 4)); + assert!( + (f.commitment - 0.878).abs() < 0.01, + "expected ≈0.878, got {}", + f.commitment + ); + assert!(f.commitment >= DISCARD_MATTERS_FLOOR); +} + +#[test] +fn light_pitch_build_still_clears_the_floor() { + // Docstring anchor: 6 outlets + 2 engines → 0.481, a real but lean plan. + let f = detect(&deck(6, 2)); + assert!( + (f.commitment - 0.481).abs() < 0.01, + "expected ≈0.481, got {}", + f.commitment + ); + assert!(f.commitment >= DISCARD_MATTERS_FLOOR); +} + +#[test] +fn incidental_rummaging_stays_below_floor() { + // Docstring anti-anchor: 2 outlets + 1 engine over 36 nonland → 0.196. + let f = detect(&deck(2, 1)); + assert!( + (f.commitment - 0.196).abs() < 0.01, + "expected ≈0.196, got {}", + f.commitment + ); + assert!( + f.commitment < DISCARD_MATTERS_FLOOR, + "expected below floor, got {}", + f.commitment + ); +} + +#[test] +fn commitment_is_format_size_neutral() { + let sixty = detect(&deck(18, 5)); + let commander = detect(&[ + entry(discard_source("Outlet"), 31), + entry(engine_card("Engine"), 9), + entry(vanilla("Filler"), 23), + ]); + assert!( + (sixty.commitment - commander.commitment).abs() < 0.05, + "{} vs {}", + sixty.commitment, + commander.commitment + ); +} + +#[test] +fn commitment_clamps_to_one() { + let f = detect(&[ + entry(discard_source("Outlet"), 30), + entry(engine_card("Engine"), 10), + ]); + assert!(f.commitment <= 1.0); +} + +#[test] +fn lands_are_excluded_from_the_denominator() { + let with_lands = detect(&[ + entry(discard_source("Outlet"), 18), + entry(engine_card("Engine"), 5), + entry(vanilla("Filler"), 13), + entry(face("Swamp", CoreType::Land), 24), + ]); + let without = detect(&deck(18, 5)); + assert_eq!(with_lands.commitment, without.commitment); +} diff --git a/crates/phase-ai/src/features/tests/mod.rs b/crates/phase-ai/src/features/tests/mod.rs index 00b8926771..3ab65d79cf 100644 --- a/crates/phase-ai/src/features/tests/mod.rs +++ b/crates/phase-ai/src/features/tests/mod.rs @@ -5,6 +5,7 @@ pub mod artifacts; pub mod blink; pub mod cost_reduction; pub mod devotion; +pub mod discard_matters; pub mod draw_matters; pub mod enchantments; pub mod energy; diff --git a/crates/phase-ai/src/policies/discard_payoff.rs b/crates/phase-ai/src/policies/discard_payoff.rs new file mode 100644 index 0000000000..0330d1909a --- /dev/null +++ b/crates/phase-ai/src/policies/discard_payoff.rs @@ -0,0 +1,299 @@ +//! `DiscardPayoffPolicy` — makes an on-battlefield "whenever you discard" engine +//! a reason the AI can see to pitch cards WILLINGLY. +//! +//! ## The gap this closes +//! +//! CR 701.9: with Archfiend of Ifnir, Bone Miser, Waste Not or Containment +//! Construct on the battlefield, every card the AI discards is a repeatable +//! value trigger. The AI's default instinct is the opposite one — `card_advantage` +//! scores a card leaving hand as a loss, and nothing credits the trigger it +//! fires. So the AI declines its own engine: it routes around rummaging outlets +//! and treats a discard cost as pure downside even when the discard IS the +//! payoff. This policy adds that positive signal. +//! +//! It is deliberately narrow in one direction: it only ever ADDS value for a +//! discard that is about to fire a live engine. It never encourages discarding +//! without one, because without a payoff the AI's instinct is correct. +//! +//! ## Performance +//! +//! `verdict()` runs per candidate per search node. The card-local check — does +//! this action actually discard the controller a card (its own `CastFacts` +//! primary effects, or the activated ability's effects) — runs FIRST and rejects +//! every non-discard action. Only a confirmed discard pays for the battlefield +//! engine scan (a structural trigger match over each permanent's live +//! `trigger_definitions`), and only in a deck whose `activation` floor is already +//! cleared. No affordability sweep, no `find_legal_targets`. + +use engine::game::triggers::hypothetical_trigger_fireable; +use engine::types::actions::GameAction; +use engine::types::game_state::GameState; +use engine::types::player::PlayerId; + +use crate::features::discard_matters::{ + is_discard_payoff_trigger, is_discard_source_parts, AbilityScope, DiscardQuantity, + DISCARD_MATTERS_FLOOR, +}; +use crate::features::DeckFeatures; + +use super::context::PolicyContext; +use super::registry::{DecisionKind, PolicyId, PolicyReason, PolicyVerdict, TacticalPolicy}; + +pub struct DiscardPayoffPolicy; + +/// Cap on how many simultaneous engines are rewarded, so a stacked board can't +/// push a single discard into the critical band. +/// +/// `pub(crate)` so the bounded-score regression asserts against this constant +/// rather than a copied literal — raising the cap must move the test with it. +pub(crate) const MAX_REWARDED_ENGINES: usize = 3; + +impl TacticalPolicy for DiscardPayoffPolicy { + fn id(&self) -> PolicyId { + PolicyId::DiscardPayoff + } + + fn decision_kinds(&self) -> &'static [DecisionKind] { + &[DecisionKind::CastSpell, DecisionKind::ActivateAbility] + } + + fn activation( + &self, + features: &DeckFeatures, + _state: &GameState, + _player: PlayerId, + ) -> Option { + if features.discard_matters.commitment < DISCARD_MATTERS_FLOOR { + None + } else { + Some(features.discard_matters.commitment) + } + } + + fn verdict(&self, ctx: &PolicyContext<'_>) -> PolicyVerdict { + // Card-local first: does this action actually discard the controller a card? + if !candidate_discards_controller(ctx) { + return PolicyVerdict::neutral(PolicyReason::new("discard_payoff_na")); + } + + // Only now pay for the battlefield scan. A permanent counts only when it + // carries a "whenever you discard" trigger (CR 701.9) that is actually + // LIVE: the engine's `hypothetical_trigger_fireable` authority preflights + // the trigger's constraint AND its execution target legality (CR 603.3d), + // so a rate-limited, off-timing, conditional, or no-legal-target engine is + // not credited value it cannot produce. + let engines = ctx + .state + .battlefield + .iter() + .filter(|id| { + ctx.state.objects.get(id).is_some_and(|obj| { + obj.controller == ctx.ai_player + && obj.trigger_definitions.iter_unchecked().any(|entry| { + is_discard_payoff_trigger(&entry.definition) + && hypothetical_trigger_fireable(ctx.state, obj, entry) + }) + }) + }) + .count(); + if engines == 0 { + return PolicyVerdict::neutral(PolicyReason::new("discard_payoff_no_engine")); + } + + // Each active engine turns this discard into a value trigger — roughly a + // card-equivalent apiece, capped so one discard stays a preference. + let rewarded = engines.min(MAX_REWARDED_ENGINES) as f64; + PolicyVerdict::score( + ctx.config.policy_penalties.discard_payoff_bonus * rewarded, + PolicyReason::new("discard_payoff_engine_active").with_fact("engines", engines as i64), + ) + } +} + +/// CR 701.9: the live-candidate quantity requirement — this discard must resolve +/// to at least one card, or it moves nothing and fires no engine. `source` is the +/// object whose `cost_x_paid` binds an announced `X`, so an un-announced X +/// resolves to zero and the candidate stays neutral. +fn positive_discard_quantity<'a>( + ctx: &PolicyContext<'a>, + source: engine::types::identifiers::ObjectId, +) -> DiscardQuantity<'a> { + DiscardQuantity::ResolvesPositive { + state: ctx.state, + controller: ctx.ai_player, + source, + } +} + +/// Card-local structural test: does this candidate's own AST discard its +/// controller a card, in a quantity that actually moves one? +/// +/// * `CastSpell` → the spell's own resolution chain (`CastFacts::primary_effects`). +/// * `ActivateAbility` → the ability at the runtime-enumerated index, which is +/// where the rummaging outlets live (Anje, Wild Mongrel, Faithless-Looting +/// style activated pitch). +/// +/// CR 700.2: a live candidate is scored before its modes are chosen, so only an +/// UNCONDITIONAL discard counts — a modal "choose one — discard / …" must not be +/// credited a discard here. +fn candidate_discards_controller(ctx: &PolicyContext<'_>) -> bool { + match &ctx.candidate.action { + GameAction::CastSpell { .. } => ctx.cast_facts().is_some_and(|facts| { + is_discard_source_parts( + facts.primary_effects.iter().copied(), + AbilityScope::Unconditional, + &positive_discard_quantity(ctx, facts.object.id), + ) + }), + GameAction::ActivateAbility { source_id, .. } => { + ctx.effective_activated_ability().is_some_and(|ability| { + is_discard_source_parts( + std::iter::once(&ability), + AbilityScope::Unconditional, + &positive_discard_quantity(ctx, *source_id), + ) + }) + } + // CR 601.2 + CR 702.34a: cast-shaped siblings of the plain `CastSpell` + // seam. `PolicyContext::cast_facts` is populated only for the `CastSpell` + // announcement seam, so this policy has no AST to classify for these and + // must report neutral rather than guess. + // + // `CastSpellAsMadness` is listed here deliberately despite belonging to + // this archetype thematically: a madness cast is the payoff the DISCARD + // already earned, not a new discard. Crediting it would double-count one + // event. Listed explicitly, not wildcarded: if `cast_facts` later covers + // one of these, this arm is where the decision to credit it gets made. + GameAction::Foretell { .. } + | GameAction::PlayFaceDown { .. } + | GameAction::ActivateNinjutsu { .. } + | GameAction::CastSpellAsSneak { .. } + | GameAction::CastSpellAsWebSlinging { .. } + | GameAction::CastSpellForFree { .. } + | GameAction::CastSpellAsMiracle { .. } + | GameAction::CastSpellAsMadness { .. } + | GameAction::CastPreparedCopy { .. } + | GameAction::CastParadigmCopy { .. } => false, + // Every remaining action: not a spell cast or ability activation, so it + // cannot discard its controller a card as part of the candidate itself. + // Enumerated rather than wildcarded so a newly added `GameAction` fails + // this match at compile time and forces an intentional classification + // instead of silently bypassing the discard payoff (CR 701.9). + GameAction::PassPriority + | GameAction::ChooseMeldPair { .. } + | GameAction::ChooseEntryAttackTarget { .. } + | GameAction::PlayLand { .. } + | GameAction::DeclareAttackers { .. } + | GameAction::DeclareBlockers { .. } + | GameAction::ChooseUntap { .. } + | GameAction::ChooseExert { .. } + | GameAction::ChooseEnlist { .. } + | GameAction::ChooseClashOpponent { .. } + | GameAction::ChooseZoneOpponentChooser { .. } + | GameAction::ChoosePileOpponent { .. } + | GameAction::ChooseAnnouncingOpponent { .. } + | GameAction::ChooseGiftRecipient { .. } + | GameAction::ChooseAssistPlayer { .. } + | GameAction::CommitAssistPayment { .. } + | GameAction::MulliganDecision { .. } + | GameAction::ReorderHand { .. } + | GameAction::TapLandForMana { .. } + | GameAction::UntapLandForMana { .. } + | GameAction::SpendPoolMana { .. } + | GameAction::UnspendPoolMana { .. } + | GameAction::SelectCards { .. } + | GameAction::ChooseRemoveCounterCostDistribution { .. } + | GameAction::SelectCoinFlips { .. } + | GameAction::ChooseOutsideGameCards { .. } + | GameAction::SelectTargets { .. } + | GameAction::ChooseTarget { .. } + | GameAction::ChooseReplacement { .. } + | GameAction::OrderTriggers { .. } + | GameAction::CancelCast + | GameAction::Equip { .. } + | GameAction::CrewVehicle { .. } + | GameAction::ActivateStation { .. } + | GameAction::SaddleMount { .. } + | GameAction::Transform { .. } + | GameAction::TurnFaceUp { .. } + | GameAction::SubmitSideboard { .. } + | GameAction::ChoosePlayDraw { .. } + | GameAction::ChooseOption { .. } + | GameAction::SubmitVoteCandidate { .. } + | GameAction::SubmitSpellbookDraft { .. } + | GameAction::SubmitPilePartition { .. } + | GameAction::ChoosePile { .. } + | GameAction::ChooseBranch { .. } + | GameAction::SubmitLifeRedistribution { .. } + | GameAction::ChooseDamageSource { .. } + | GameAction::SelectModes { .. } + | GameAction::DecideOptionalCost { .. } + | GameAction::ChooseAdventureFace { .. } + | GameAction::ChooseModalFace { .. } + | GameAction::ChooseAlternativeCast { .. } + | GameAction::ChooseCastingVariant { .. } + | GameAction::KeepAllCopyTargets + | GameAction::ChoosePermanentTypeSlot { .. } + | GameAction::DecideOptionalEffect { .. } + | GameAction::RespondToSpliceOffer { .. } + | GameAction::DecideOptionalEffectAndRemember { .. } + | GameAction::PayUnlessCost { .. } + | GameAction::ChooseUnlessCostBranch { .. } + | GameAction::ChooseActivationCostBranch { .. } + | GameAction::PayCombatTax { .. } + | GameAction::ChooseRingBearer { .. } + | GameAction::ChoosePair { .. } + | GameAction::ChooseDungeon { .. } + | GameAction::ChooseDungeonRoom { .. } + | GameAction::UnlockRoomDoor { .. } + | GameAction::RollPlanarDie + | GameAction::ChooseRoomDoor { .. } + | GameAction::TapForConvoke { .. } + | GameAction::HarmonizeTap { .. } + | GameAction::DeclareCompanion { .. } + | GameAction::CompanionToHand + | GameAction::DiscoverChoice { .. } + | GameAction::GraveyardPaidCastChoice { .. } + | GameAction::CascadeChoice { .. } + | GameAction::RippleChoice { .. } + | GameAction::FreeCastWindowChoice { .. } + | GameAction::ChooseTopOrBottom { .. } + | GameAction::ChooseMutateMergeSide { .. } + | GameAction::CipherEncode { .. } + | GameAction::ChooseLegend { .. } + | GameAction::ChooseBattleProtector { .. } + | GameAction::SetAutoPass { .. } + | GameAction::CancelAutoPass + | GameAction::SetPhaseStops { .. } + | GameAction::SetPriorityPassingMode { .. } + | GameAction::SetPriorityYield { .. } + | GameAction::SetMayTriggerAutoChoice { .. } + | GameAction::SetTriggerOrderTemplate { .. } + | GameAction::AssignCombatDamage { .. } + | GameAction::AssignBlockerDamage { .. } + | GameAction::DistributeAmong { .. } + | GameAction::ChooseCounterMoveDistribution { .. } + | GameAction::ChooseCountersToRemove { .. } + | GameAction::SubmitPayAmount { .. } + | GameAction::RetargetSpell { .. } + | GameAction::LearnDecision { .. } + | GameAction::SelectCategoryPermanents { .. } + | GameAction::ChooseKeptCreatures { .. } + | GameAction::ChooseKeptPermanents { .. } + | GameAction::ChooseX { .. } + | GameAction::SubmitPhyrexianChoices { .. } + | GameAction::ChooseManaColor { .. } + | GameAction::PayManaAbilityMana { .. } + | GameAction::ChooseSpecializeColor { .. } + | GameAction::PassParadigmOffer + | GameAction::Debug(..) + | GameAction::GrantDebugPermission { .. } + | GameAction::RevokeDebugPermission { .. } + | GameAction::Concede { .. } + | GameAction::DeclareShortcut { .. } + | GameAction::RespondToShortcut { .. } + | GameAction::DeclineShortcut + | GameAction::PrecastCopyShortcut { .. } + | GameAction::EndContinuousEffect { .. } => false, + } +} diff --git a/crates/phase-ai/src/policies/mod.rs b/crates/phase-ai/src/policies/mod.rs index ca085f93de..63ed9ce62e 100644 --- a/crates/phase-ai/src/policies/mod.rs +++ b/crates/phase-ai/src/policies/mod.rs @@ -18,6 +18,7 @@ mod cost_reduction; mod crew_timing; mod cycling_discipline; mod devotion; +mod discard_payoff; mod downside_awareness; mod draw_payoff; pub(crate) mod effect_classify; diff --git a/crates/phase-ai/src/policies/registry.rs b/crates/phase-ai/src/policies/registry.rs index cfcdca48f3..375205ccd0 100644 --- a/crates/phase-ai/src/policies/registry.rs +++ b/crates/phase-ai/src/policies/registry.rs @@ -156,6 +156,9 @@ pub enum PolicyId { CostReduction, /// CR 121.1: reward drawing into an on-battlefield "whenever you draw" engine. DrawPayoff, + /// CR 701.9: reward discarding into an on-battlefield "whenever you discard" + /// engine — disjoint from `HandDisruption`, which scores OPPONENT discard. + DiscardPayoff, } /// Coarse routing kind for a candidate decision. Each policy declares which @@ -414,6 +417,7 @@ impl Default for PolicyRegistry { Box::new(super::self_bounce_target::SelfBounceTargetPolicy), Box::new(super::cost_reduction::CostReductionPolicy), Box::new(super::draw_payoff::DrawPayoffPolicy), + Box::new(super::discard_payoff::DiscardPayoffPolicy), ]; let mut by_kind: HashMap> = HashMap::new(); for (idx, policy) in policies.iter().enumerate() { diff --git a/crates/phase-ai/src/policies/tests/discard_payoff.rs b/crates/phase-ai/src/policies/tests/discard_payoff.rs new file mode 100644 index 0000000000..0cf71bea87 --- /dev/null +++ b/crates/phase-ai/src/policies/tests/discard_payoff.rs @@ -0,0 +1,431 @@ +//! Unit tests for `policies::discard_payoff` — CR 701.9 "whenever you discard" +//! payoff policy. No `#[cfg(test)]` in SOURCE files; tests live here. +//! +//! Direct-`verdict` tests cover each branch; a registry-routed regression +//! exercises the production seam (registration + routing). + +use std::sync::Arc; + +use engine::ai_support::{ActionMetadata, AiDecisionContext, CandidateAction, TacticalClass}; +use engine::game::zones::create_object; +use engine::types::ability::{ + AbilityDefinition, AbilityKind, CardSelectionMode, Effect, QuantityExpr, TargetFilter, + TriggerDefinition, +}; +use engine::types::actions::GameAction; +use engine::types::card_type::CoreType; +use engine::types::format::FormatConfig; +use engine::types::game_state::{CastPaymentMode, GameState, WaitingFor}; +use engine::types::identifiers::{CardId, ObjectId}; +use engine::types::player::PlayerId; +use engine::types::triggers::TriggerMode; +use engine::types::zones::Zone; + +use crate::config::AiConfig; +use crate::context::AiContext; +use crate::features::discard_matters::{DiscardMattersFeature, DISCARD_MATTERS_FLOOR}; +use crate::features::DeckFeatures; +use crate::policies::context::{PolicyContext, SearchDepth}; +use crate::policies::discard_payoff::*; +use crate::policies::registry::{ + PolicyId, PolicyReason, PolicyRegistry, PolicyVerdict, TacticalPolicy, +}; +use crate::session::AiSession; + +const AI: PlayerId = PlayerId(0); + +fn state() -> GameState { + GameState::new(FormatConfig::standard(), 2, 42) +} + +/// "Discard N cards" scoped to the caster (the rich `Effect::Discard` form). +fn discard_effect(count: i32, target: TargetFilter) -> Effect { + Effect::Discard { + count: QuantityExpr::Fixed { value: count }, + target, + selection: CardSelectionMode::Chosen, + unless_filter: None, + filter: None, + } +} + +/// A hand spell whose resolution runs `effect`. +fn spell(state: &mut GameState, effect: Effect) -> (ObjectId, CardId) { + let card_id = CardId(state.next_object_id); + let id = create_object(state, card_id, AI, "Pitch Spell".to_string(), Zone::Hand); + let obj = state.objects.get_mut(&id).unwrap(); + obj.card_types.core_types.push(CoreType::Sorcery); + Arc::make_mut(&mut obj.abilities).push(AbilityDefinition::new(AbilityKind::Spell, effect)); + (id, card_id) +} + +/// A battlefield permanent whose activated ability at index 0 runs `effect` — +/// the rummaging-outlet shape (Wild Mongrel / Anje). +fn activated_permanent(state: &mut GameState, effect: Effect) -> ObjectId { + let card_id = CardId(state.next_object_id); + let id = create_object(state, card_id, AI, "Outlet".to_string(), Zone::Battlefield); + let obj = state.objects.get_mut(&id).unwrap(); + obj.card_types.core_types.push(CoreType::Creature); + Arc::make_mut(&mut obj.abilities).push(AbilityDefinition::new(AbilityKind::Activated, effect)); + id +} + +/// The Archfiend of Ifnir shape: a no-target on-discard payoff, so target +/// legality never blocks it. +fn discarded_engine_trigger() -> TriggerDefinition { + TriggerDefinition::new(TriggerMode::Discarded).execute(AbilityDefinition::new( + AbilityKind::Spell, + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 1 }, + player: TargetFilter::Controller, + }, + )) +} + +fn engine_on_battlefield(state: &mut GameState, trigger: Option) { + let card_id = CardId(state.next_object_id); + let id = create_object( + state, + card_id, + AI, + "Archfiend".to_string(), + Zone::Battlefield, + ); + let obj = state.objects.get_mut(&id).unwrap(); + obj.card_types.core_types.push(CoreType::Creature); + if let Some(t) = trigger { + obj.trigger_definitions.push(t); + } +} + +fn feature(commitment: f32) -> DiscardMattersFeature { + DiscardMattersFeature { + source_count: 10, + payoff_count: 4, + commitment, + } +} + +fn session(commitment: f32) -> AiSession { + let features = DeckFeatures { + discard_matters: feature(commitment), + ..Default::default() + }; + let mut session = AiSession::empty(); + session.features.insert(AI, features); + session +} + +fn context(config: &AiConfig, session: AiSession) -> AiContext { + let mut context = AiContext::empty(&config.weights); + context.session = Arc::new(session); + context.player = AI; + context +} + +fn cast(object_id: ObjectId, card_id: CardId) -> CandidateAction { + CandidateAction { + action: GameAction::CastSpell { + object_id, + card_id, + targets: Vec::new(), + payment_mode: CastPaymentMode::default(), + }, + metadata: ActionMetadata::for_actor(Some(AI), TacticalClass::Spell), + } +} + +fn activate(source_id: ObjectId, ability_index: usize) -> CandidateAction { + CandidateAction { + action: GameAction::ActivateAbility { + source_id, + ability_index, + }, + metadata: ActionMetadata::for_actor(Some(AI), TacticalClass::Ability), + } +} + +fn ctx<'a>( + state: &'a GameState, + candidate: &'a CandidateAction, + decision: &'a AiDecisionContext, + context: &'a AiContext, + config: &'a AiConfig, +) -> PolicyContext<'a> { + PolicyContext { + state, + decision, + candidate, + ai_player: AI, + config, + context, + cast_facts: None, + search_depth: SearchDepth::Root, + } +} + +fn priority_decision(candidate: &CandidateAction) -> AiDecisionContext { + AiDecisionContext { + waiting_for: WaitingFor::Priority { player: AI }, + candidates: vec![candidate.clone()], + } +} + +fn score_of(verdict: PolicyVerdict) -> (f64, PolicyReason) { + match verdict { + PolicyVerdict::Score { delta, reason } => (delta, reason), + PolicyVerdict::Reject { reason } => panic!("unexpected Reject: {reason:?}"), + } +} + +// ─── activation ────────────────────────────────────────────────────────────── + +#[test] +fn activation_opts_out_below_floor() { + let features = DeckFeatures { + discard_matters: feature(DISCARD_MATTERS_FLOOR - 0.01), + ..Default::default() + }; + assert!(DiscardPayoffPolicy + .activation(&features, &state(), AI) + .is_none()); +} + +#[test] +fn activation_opts_in_above_floor() { + let features = DeckFeatures { + discard_matters: feature(0.8), + ..Default::default() + }; + assert_eq!( + DiscardPayoffPolicy.activation(&features, &state(), AI), + Some(0.8) + ); +} + +// ─── verdict ───────────────────────────────────────────────────────────────── + +#[test] +fn discarding_with_an_engine_out_scores_positive() { + let mut st = state(); + engine_on_battlefield(&mut st, Some(discarded_engine_trigger())); + let (obj, card) = spell(&mut st, discard_effect(2, TargetFilter::Controller)); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = cast(obj, card); + let decision = priority_decision(&candidate); + let (delta, reason) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "discard_payoff_engine_active"); + assert!( + delta > 0.0, + "expected a positive payoff credit, got {delta}" + ); +} + +#[test] +fn discarding_without_an_engine_is_neutral() { + // Without a payoff the AI's instinct to avoid discarding is CORRECT, so this + // policy must stay silent rather than push it to pitch cards. + let mut st = state(); + let (obj, card) = spell(&mut st, discard_effect(2, TargetFilter::Controller)); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = cast(obj, card); + let decision = priority_decision(&candidate); + let (delta, reason) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "discard_payoff_no_engine"); + assert_eq!(delta, 0.0); +} + +#[test] +fn opponent_discard_is_not_credited() { + // `hand_disruption`'s subject — making THEM discard fires no engine of mine. + let mut st = state(); + engine_on_battlefield(&mut st, Some(discarded_engine_trigger())); + let (obj, card) = spell(&mut st, discard_effect(2, TargetFilter::Opponent)); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = cast(obj, card); + let decision = priority_decision(&candidate); + let (delta, reason) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "discard_payoff_na"); + assert_eq!(delta, 0.0); +} + +#[test] +fn zero_count_discard_is_not_credited() { + // CR 701.9 + CR 107.1b: a discard that moves no card emits no event. + let mut st = state(); + engine_on_battlefield(&mut st, Some(discarded_engine_trigger())); + let (obj, card) = spell(&mut st, discard_effect(0, TargetFilter::Controller)); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = cast(obj, card); + let decision = priority_decision(&candidate); + let (_, reason) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "discard_payoff_na"); +} + +#[test] +fn non_discard_spell_is_not_applicable() { + let mut st = state(); + engine_on_battlefield(&mut st, Some(discarded_engine_trigger())); + let (obj, card) = spell( + &mut st, + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 3 }, + player: TargetFilter::Controller, + }, + ); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = cast(obj, card); + let decision = priority_decision(&candidate); + let (_, reason) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "discard_payoff_na"); +} + +#[test] +fn activated_rummaging_outlet_is_credited() { + // The Wild Mongrel / Anje shape: the outlet is an activated ability, which + // is where most real self-discard lives. + let mut st = state(); + engine_on_battlefield(&mut st, Some(discarded_engine_trigger())); + let outlet = activated_permanent(&mut st, discard_effect(1, TargetFilter::Controller)); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = activate(outlet, 0); + let decision = priority_decision(&candidate); + let (delta, reason) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "discard_payoff_engine_active"); + assert!(delta > 0.0); +} + +#[test] +fn simple_discard_card_variant_is_credited() { + // Sibling-variant coverage at the LIVE seam: `Effect::DiscardCard` must be + // classified as an enabler exactly like `Effect::Discard`. + let mut st = state(); + engine_on_battlefield(&mut st, Some(discarded_engine_trigger())); + let outlet = activated_permanent( + &mut st, + Effect::DiscardCard { + count: 1, + target: TargetFilter::Controller, + }, + ); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = activate(outlet, 0); + let decision = priority_decision(&candidate); + let (delta, reason) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "discard_payoff_engine_active"); + assert!(delta > 0.0); +} + +#[test] +fn name_only_impostor_is_not_an_engine() { + // A permanent with no live discard trigger must not be credited, even though + // it sits on the battlefield under our control. + let mut st = state(); + engine_on_battlefield(&mut st, None); + let (obj, card) = spell(&mut st, discard_effect(1, TargetFilter::Controller)); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = cast(obj, card); + let decision = priority_decision(&candidate); + let (_, reason) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "discard_payoff_no_engine"); +} + +#[test] +fn credit_is_bounded_by_the_engine_cap() { + let mut st = state(); + for _ in 0..8 { + engine_on_battlefield(&mut st, Some(discarded_engine_trigger())); + } + let (obj, card) = spell(&mut st, discard_effect(1, TargetFilter::Controller)); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = cast(obj, card); + let decision = priority_decision(&candidate); + let (delta, _) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + let ceiling = config.policy_penalties.discard_payoff_bonus * MAX_REWARDED_ENGINES as f64; + assert!( + delta <= ceiling + f64::EPSILON, + "delta {delta} exceeded ceiling {ceiling}" + ); +} + +// ─── production seam ───────────────────────────────────────────────────────── + +#[test] +fn registry_routes_cast_spell_to_this_policy() { + let mut st = state(); + engine_on_battlefield(&mut st, Some(discarded_engine_trigger())); + let (obj, card) = spell(&mut st, discard_effect(1, TargetFilter::Controller)); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = cast(obj, card); + let decision = priority_decision(&candidate); + let verdicts = + PolicyRegistry::default().verdicts(&ctx(&st, &candidate, &decision, &context, &config)); + + let found = verdicts + .iter() + .find(|(id, _)| *id == PolicyId::DiscardPayoff) + .map(|(_, v)| v.clone()) + .expect("DiscardPayoffPolicy must be registered and routed for CastSpell"); + let (delta, reason) = score_of(found); + assert_eq!(reason.kind, "discard_payoff_engine_active"); + assert!(delta > 0.0); +} + +#[test] +fn registry_stays_silent_below_the_activation_floor() { + let mut st = state(); + engine_on_battlefield(&mut st, Some(discarded_engine_trigger())); + let (obj, card) = spell(&mut st, discard_effect(1, TargetFilter::Controller)); + + let config = AiConfig::default(); + let context = context(&config, session(DISCARD_MATTERS_FLOOR - 0.01)); + let candidate = cast(obj, card); + let decision = priority_decision(&candidate); + let verdicts = + PolicyRegistry::default().verdicts(&ctx(&st, &candidate, &decision, &context, &config)); + + assert!( + !verdicts + .iter() + .any(|(id, _)| *id == PolicyId::DiscardPayoff), + "policy must not contribute below its activation floor" + ); +} diff --git a/crates/phase-ai/src/policies/tests/mod.rs b/crates/phase-ai/src/policies/tests/mod.rs index 64b74f2e9d..90515eceea 100644 --- a/crates/phase-ai/src/policies/tests/mod.rs +++ b/crates/phase-ai/src/policies/tests/mod.rs @@ -5,6 +5,7 @@ pub mod artifact_synergy; pub mod blink_payoff; pub mod cost_reduction; pub mod devotion; +pub mod discard_payoff; pub mod draw_payoff; pub mod effect_classify_snapshot; pub mod enchantments_payoff; From 4db334089363fca94b7825164d38f094e2c4da83 Mon Sep 17 00:00:00 2001 From: minion1227 Date: Wed, 29 Jul 2026 05:02:48 -0700 Subject: [PATCH 2/4] fix(phase-ai): classify discard paid as an ability COST, not only as an effect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the blocker on #6786. The axis was built for the rummaging class — Wild Mongrel, Anje Falkenrath, flashback-discard — and then never reached it. Those cards spell the discard as `AbilityCost::Discard` (ability.rs:8309), while the source predicate read only `Effect::Discard` / `Effect::DiscardCard`. So activating a real outlet stayed neutral even with a live engine on the battlefield: exactly the case the policy exists to fix. Worse, the regression that claimed to cover it did not. `activated_rummaging_ outlet_is_credited` built an ability whose *effect* was a discard — a shape no printed card has — so it exercised the effect path a second time and witnessed nothing about costs. A test that constructs its own subject can confirm a behavior that never occurs. The fix extends the SHARED `is_discard_source_parts`, so deck detection and activation scoring pick it up from one place rather than growing a second cost-only path that could drift from the effect one: - `ability_cost_discards` gates on the engine's own `AbilityDefinition::cost_categories()` authority, which already flattens `Composite`/`OneOf` nesting, before walking anything. - `cost_discards` then walks for a discard the payer actually makes, reusing the existing `AbilityScope` parameter for the branch question. CR 601.2h: every component of a `Composite` is paid, so a discard inside it is guaranteed. CR 118.12a: a `OneOf` resolves to one branch, so its discard is something the DECK can plan around (`Potential`) but not something a live candidate is committed to (`Unconditional`) — crediting it there would score a discard the player may never make. - Count positivity flows through the existing `DiscardQuantity` parameter, so a zero-count cost is not credited at the live seam. - Non-recursive cost forms defer to `categories()` rather than a hand-written match arm: a newly added discarding cost variant is then picked up automatically, and the count check that path skips can only UNDER-credit. A discard cost needs no controller filter — it is always paid by the activating player, so there is no opponent-scoped case to exclude the way there is for `Effect::Discard`. Tests 60 → 68. Six of the eight new ones fail without the cost axis (three deck-time, three live), including the real Wild Mongrel shape. The two negatives — `one_of_cost_is_not_credited_at_the_live_seam` and `zero_count_discard_cost_is_not_credited` — would pass vacuously against the old code, so they were separately proven by mutating the `OneOf` branch gate and the count check; both fail under that mutation. Verified: `cargo test -p phase-ai` 1853 passed, `cargo clippy -p phase-ai -p phase-engine --all-targets --features proptest -- -D warnings` exit 0, `check-engine-authorities.sh` exit 0. Co-Authored-By: Claude Opus 5 (1M context) --- .../phase-ai/src/features/discard_matters.rs | 91 +++++++++-- .../src/features/tests/discard_matters.rs | 75 +++++++++- .../src/policies/tests/discard_payoff.rs | 141 +++++++++++++++++- 3 files changed, 292 insertions(+), 15 deletions(-) diff --git a/crates/phase-ai/src/features/discard_matters.rs b/crates/phase-ai/src/features/discard_matters.rs index e7c2ecb83b..460f263ded 100644 --- a/crates/phase-ai/src/features/discard_matters.rs +++ b/crates/phase-ai/src/features/discard_matters.rs @@ -9,6 +9,10 @@ //! simple enabler form (`u32` count). BOTH are live in the resolver //! (`game/effects/discard.rs` and `game/effects/mod.rs`), so a classifier //! that reads only one silently misses part of the class. +//! - `AbilityCost::Discard { count, .. }` at `ability.rs:8309` — the rummaging +//! class (Wild Mongrel / Anje / flashback) spells its discard as a COST, not +//! an effect, so the source predicate reads both axes. Nesting is resolved +//! through the engine's `AbilityDefinition::cost_categories()` authority. //! - `TriggerMode::Discarded` at `crates/engine/src/types/triggers.rs:321` and //! `TriggerMode::DiscardedAll` at `triggers.rs:322` (CR 701.9) — the payoffs. //! - `TriggerDefinition.valid_target` — used to keep only YOUR-discard engines, @@ -50,7 +54,8 @@ use engine::game::ability_utils::ability_definition_supported; use engine::game::quantity::resolve_quantity; use engine::game::DeckEntry; use engine::types::ability::{ - AbilityDefinition, Effect, QuantityExpr, TargetFilter, TriggerDefinition, + AbilityCost, AbilityDefinition, CostCategory, Effect, QuantityExpr, TargetFilter, + TriggerDefinition, }; use engine::types::card_type::CoreType; use engine::types::game_state::GameState; @@ -190,20 +195,82 @@ pub(crate) fn is_discard_source_parts<'a>( quantity: &DiscardQuantity<'_>, ) -> bool { abilities.into_iter().any(|ability| { - collect_scoped_effects(ability, scope) - .iter() - .any(|effect| match effect { - Effect::Discard { target, count, .. } => { - discards_controller(target) && quantity.expr_is_satisfied(count) - } - Effect::DiscardCard { target, count } => { - discards_controller(target) && quantity.fixed_is_satisfied(*count) - } - _ => false, - }) + ability_cost_discards(ability, scope, quantity) + || collect_scoped_effects(ability, scope) + .iter() + .any(|effect| match effect { + Effect::Discard { target, count, .. } => { + discards_controller(target) && quantity.expr_is_satisfied(count) + } + Effect::DiscardCard { target, count } => { + discards_controller(target) && quantity.fixed_is_satisfied(*count) + } + _ => false, + }) }) } +/// CR 118.3 + CR 601.2h: does PAYING this ability's cost discard a card? +/// +/// The rummaging class this axis exists for — Wild Mongrel, Anje Falkenrath, +/// Faithless Looting's flashback — spells the discard as an `AbilityCost`, not as +/// an `Effect`. Reading only the effect chain classifies none of them, so the +/// shared source predicate has to look at both axes. +/// +/// A discard COST has no target filter because it is always paid by the +/// activating player, i.e. by you — there is no opponent-scoped cost to exclude +/// the way there is for `Effect::Discard`. +fn ability_cost_discards( + ability: &AbilityDefinition, + scope: AbilityScope, + quantity: &DiscardQuantity<'_>, +) -> bool { + // Cheap gate through the engine's own cost-category authority, which already + // flattens `Composite`/`OneOf` nesting. If it says no discard exists + // anywhere in the tree, there is nothing to walk. + if !ability.cost_categories().contains(&CostCategory::Discards) { + return false; + } + ability + .cost + .as_ref() + .is_some_and(|cost| cost_discards(cost, scope, quantity)) +} + +/// Walk a cost tree for a discard the payer will actually make. +/// +/// The `scope` distinction is the same one the effect walk uses, applied to +/// costs: `Potential` asks "could paying this discard a card?", `Unconditional` +/// asks "will it?". That matters for `AbilityCost::OneOf` (CR 118.12a) — a +/// "discard a card OR pay 2 life" cost is a discard the deck can plan around, +/// but NOT one a live candidate is committed to, so crediting it at the live +/// seam would score a discard the player may never make. +fn cost_discards(cost: &AbilityCost, scope: AbilityScope, quantity: &DiscardQuantity<'_>) -> bool { + match cost { + AbilityCost::Discard { count, .. } => quantity.expr_is_satisfied(count), + // CR 601.2h: every component of a composite cost is paid, so a discard + // anywhere inside it is guaranteed. + AbilityCost::Composite { costs } => costs + .iter() + .any(|inner| cost_discards(inner, scope, quantity)), + // CR 118.12a: only one branch is chosen, so the discard is possible but + // not certain. + AbilityCost::OneOf { costs } => { + matches!(scope, AbilityScope::Potential) + && costs + .iter() + .any(|inner| cost_discards(inner, scope, quantity)) + } + AbilityCost::PerCounter { base, .. } => cost_discards(base, scope, quantity), + // Every other cost form: defer to the engine's category authority rather + // than enumerating here. That is deliberate — a newly added discarding + // cost variant is picked up automatically instead of being silently + // dropped by a stale match arm, and the count check it skips can only + // make this UNDER-credit, never over-credit. + other => other.categories().contains(&CostCategory::Discards), + } +} + /// CR 701.9: the triggers carry a "whenever you discard a card" engine — a /// repeatable payoff. Parts-based so it classifies both a deck-time /// `CardFace.triggers` slice and a live `GameObject.trigger_definitions` diff --git a/crates/phase-ai/src/features/tests/discard_matters.rs b/crates/phase-ai/src/features/tests/discard_matters.rs index f5323b8ae5..1021f0b50c 100644 --- a/crates/phase-ai/src/features/tests/discard_matters.rs +++ b/crates/phase-ai/src/features/tests/discard_matters.rs @@ -4,7 +4,8 @@ use engine::game::DeckEntry; use engine::types::ability::CardSelectionMode; use engine::types::ability::{ - AbilityDefinition, AbilityKind, Effect, QuantityExpr, TargetFilter, TriggerDefinition, + AbilityCost, AbilityDefinition, AbilityKind, DiscardSelfScope, Effect, QuantityExpr, + TargetFilter, TriggerDefinition, }; use engine::types::card::CardFace; use engine::types::card_type::{CardType, CoreType}; @@ -330,3 +331,75 @@ fn lands_are_excluded_from_the_denominator() { let without = detect(&deck(18, 5)); assert_eq!(with_lands.commitment, without.commitment); } + +// ─── review #6786: the discard-as-COST path at deck time ──────────────────── + +fn discard_cost(count: i32) -> AbilityCost { + AbilityCost::Discard { + count: QuantityExpr::Fixed { value: count }, + filter: None, + selection: CardSelectionMode::Chosen, + self_scope: DiscardSelfScope::FromHand, + } +} + +/// Wild Mongrel: an activated ability whose COST is the discard. +fn cost_outlet(name: &str, cost: AbilityCost) -> CardFace { + let mut f = face(name, CoreType::Creature); + f.abilities = vec![AbilityDefinition::new( + AbilityKind::Activated, + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 1 }, + player: TargetFilter::Controller, + }, + ) + .cost(cost)]; + f +} + +#[test] +fn detects_discard_cost_outlet() { + let f = detect(&[ + entry(cost_outlet("Wild Mongrel", discard_cost(1)), 10), + entry(engine_card("Engine"), 4), + entry(vanilla("Filler"), 22), + ]); + assert_eq!(f.source_count, 10, "a discard COST must count as an outlet"); + assert!(f.commitment >= DISCARD_MATTERS_FLOOR); +} + +#[test] +fn deck_time_counts_a_one_of_discard_cost() { + // Deck-time asks "could this discard?", so an optional branch still marks the + // card for archetype classification — the live seam is the strict one. + let f = detect(&[ + entry( + cost_outlet( + "Optional Outlet", + AbilityCost::OneOf { + costs: vec![ + discard_cost(1), + AbilityCost::PayLife { + amount: QuantityExpr::Fixed { value: 2 }, + }, + ], + }, + ), + 10, + ), + entry(engine_card("Engine"), 4), + entry(vanilla("Filler"), 22), + ]); + assert_eq!(f.source_count, 10); +} + +#[test] +fn zero_count_discard_cost_is_still_a_deck_time_outlet() { + // `DiscardQuantity::Any` at deck time: the count is unknowable when building. + let f = detect(&[ + entry(cost_outlet("Weird", discard_cost(0)), 10), + entry(engine_card("Engine"), 4), + entry(vanilla("Filler"), 22), + ]); + assert_eq!(f.source_count, 10); +} diff --git a/crates/phase-ai/src/policies/tests/discard_payoff.rs b/crates/phase-ai/src/policies/tests/discard_payoff.rs index 0cf71bea87..9a9c308bc8 100644 --- a/crates/phase-ai/src/policies/tests/discard_payoff.rs +++ b/crates/phase-ai/src/policies/tests/discard_payoff.rs @@ -9,8 +9,8 @@ use std::sync::Arc; use engine::ai_support::{ActionMetadata, AiDecisionContext, CandidateAction, TacticalClass}; use engine::game::zones::create_object; use engine::types::ability::{ - AbilityDefinition, AbilityKind, CardSelectionMode, Effect, QuantityExpr, TargetFilter, - TriggerDefinition, + AbilityCost, AbilityDefinition, AbilityKind, CardSelectionMode, DiscardSelfScope, Effect, + QuantityExpr, TargetFilter, TriggerDefinition, }; use engine::types::actions::GameAction; use engine::types::card_type::CoreType; @@ -135,6 +135,37 @@ fn cast(object_id: ObjectId, card_id: CardId) -> CandidateAction { } } +/// The REAL Wild Mongrel / Anje shape: the discard is the ability's COST, not +/// its effect. This is the class the axis exists for, and the effect-only +/// classifier never reached it. +fn discard_cost(count: i32) -> AbilityCost { + AbilityCost::Discard { + count: QuantityExpr::Fixed { value: count }, + filter: None, + selection: CardSelectionMode::Chosen, + self_scope: DiscardSelfScope::FromHand, + } +} + +/// A battlefield permanent whose activated ability PAYS `cost` and does +/// something unrelated (pump) on resolution. +fn cost_paying_permanent(state: &mut GameState, cost: AbilityCost) -> ObjectId { + let card_id = CardId(state.next_object_id); + let id = create_object(state, card_id, AI, "Mongrel".to_string(), Zone::Battlefield); + let obj = state.objects.get_mut(&id).unwrap(); + obj.card_types.core_types.push(CoreType::Creature); + let ability = AbilityDefinition::new( + AbilityKind::Activated, + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 1 }, + player: TargetFilter::Controller, + }, + ) + .cost(cost); + Arc::make_mut(&mut obj.abilities).push(ability); + id +} + fn activate(source_id: ObjectId, ability_index: usize) -> CandidateAction { CandidateAction { action: GameAction::ActivateAbility { @@ -429,3 +460,109 @@ fn registry_stays_silent_below_the_activation_floor() { "policy must not contribute below its activation floor" ); } + +// ─── review #6786: the discard-as-COST path (the real rummaging class) ─────── + +#[test] +fn activated_discard_cost_outlet_is_credited() { + // The blocker this PR was returned for: Wild Mongrel pays `AbilityCost:: + // Discard`, so the effect-only classifier scored it neutral even with a live + // engine out. Fails without the cost-axis classification. + let mut st = state(); + engine_on_battlefield(&mut st, Some(discarded_engine_trigger())); + let outlet = cost_paying_permanent(&mut st, discard_cost(1)); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = activate(outlet, 0); + let decision = priority_decision(&candidate); + let (delta, reason) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "discard_payoff_engine_active"); + assert!(delta > 0.0, "a discard COST must be credited, got {delta}"); +} + +#[test] +fn composite_cost_containing_a_discard_is_credited() { + // CR 601.2h: every component of a composite cost is paid, so the discard is + // guaranteed — tap AND discard. + let mut st = state(); + engine_on_battlefield(&mut st, Some(discarded_engine_trigger())); + let outlet = cost_paying_permanent( + &mut st, + AbilityCost::Composite { + costs: vec![AbilityCost::Tap, discard_cost(1)], + }, + ); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = activate(outlet, 0); + let decision = priority_decision(&candidate); + let (_, reason) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "discard_payoff_engine_active"); +} + +#[test] +fn one_of_cost_is_not_credited_at_the_live_seam() { + // CR 118.12a: only one branch is chosen. "Discard a card OR pay 2 life" is a + // discard the DECK can plan around, but not one this candidate is committed + // to — crediting it would score a discard the player may never make. + let mut st = state(); + engine_on_battlefield(&mut st, Some(discarded_engine_trigger())); + let outlet = cost_paying_permanent( + &mut st, + AbilityCost::OneOf { + costs: vec![ + discard_cost(1), + AbilityCost::PayLife { + amount: QuantityExpr::Fixed { value: 2 }, + }, + ], + }, + ); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = activate(outlet, 0); + let decision = priority_decision(&candidate); + let (delta, reason) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "discard_payoff_na"); + assert_eq!(delta, 0.0); +} + +#[test] +fn zero_count_discard_cost_is_not_credited() { + let mut st = state(); + engine_on_battlefield(&mut st, Some(discarded_engine_trigger())); + let outlet = cost_paying_permanent(&mut st, discard_cost(0)); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = activate(outlet, 0); + let decision = priority_decision(&candidate); + let (_, reason) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "discard_payoff_na"); +} + +#[test] +fn discard_cost_without_an_engine_is_neutral() { + let mut st = state(); + let outlet = cost_paying_permanent(&mut st, discard_cost(1)); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = activate(outlet, 0); + let decision = priority_decision(&candidate); + let (_, reason) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "discard_payoff_no_engine"); +} From 8e38e8b4d97905ec63d15a606d04447b5d1ddb88 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Wed, 29 Jul 2026 05:06:10 -0700 Subject: [PATCH 3/4] fix(PR-6786): score guaranteed discard alternatives --- .../phase-ai/src/features/discard_matters.rs | 17 ++++++++----- .../src/policies/tests/discard_payoff.rs | 24 +++++++++++++++++++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/crates/phase-ai/src/features/discard_matters.rs b/crates/phase-ai/src/features/discard_matters.rs index 460f263ded..d4e41c5605 100644 --- a/crates/phase-ai/src/features/discard_matters.rs +++ b/crates/phase-ai/src/features/discard_matters.rs @@ -253,13 +253,18 @@ fn cost_discards(cost: &AbilityCost, scope: AbilityScope, quantity: &DiscardQuan AbilityCost::Composite { costs } => costs .iter() .any(|inner| cost_discards(inner, scope, quantity)), - // CR 118.12a: only one branch is chosen, so the discard is possible but - // not certain. + // CR 118.12a: only one branch is chosen. A discard is guaranteed only + // when every legal branch discards; otherwise it is merely possible. AbilityCost::OneOf { costs } => { - matches!(scope, AbilityScope::Potential) - && costs - .iter() - .any(|inner| cost_discards(inner, scope, quantity)) + !costs.is_empty() + && match scope { + AbilityScope::Potential => costs + .iter() + .any(|inner| cost_discards(inner, scope, quantity)), + AbilityScope::Unconditional => costs + .iter() + .all(|inner| cost_discards(inner, scope, quantity)), + } } AbilityCost::PerCounter { base, .. } => cost_discards(base, scope, quantity), // Every other cost form: defer to the engine's category authority rather diff --git a/crates/phase-ai/src/policies/tests/discard_payoff.rs b/crates/phase-ai/src/policies/tests/discard_payoff.rs index 9a9c308bc8..62e1dae9a9 100644 --- a/crates/phase-ai/src/policies/tests/discard_payoff.rs +++ b/crates/phase-ai/src/policies/tests/discard_payoff.rs @@ -536,6 +536,30 @@ fn one_of_cost_is_not_credited_at_the_live_seam() { assert_eq!(delta, 0.0); } +#[test] +fn one_of_cost_with_only_discard_branches_is_credited_at_the_live_seam() { + // CR 118.12a: only one branch is paid, but this ability discards no matter + // which branch is selected. It is therefore a guaranteed live discard, + // unlike the mixed discard-or-life sibling above. + let mut st = state(); + engine_on_battlefield(&mut st, Some(discarded_engine_trigger())); + let outlet = cost_paying_permanent( + &mut st, + AbilityCost::OneOf { + costs: vec![discard_cost(1), discard_cost(2)], + }, + ); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = activate(outlet, 0); + let decision = priority_decision(&candidate); + let (_, reason) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "discard_payoff_engine_active"); +} + #[test] fn zero_count_discard_cost_is_not_credited() { let mut st = state(); From 275d6812376e64a42508c33b933bb3238050caa0 Mon Sep 17 00:00:00 2001 From: minion1227 Date: Wed, 29 Jul 2026 05:21:12 -0700 Subject: [PATCH 4/4] test(phase-ai): cover nested optional discard costs and assert the score deltas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the two CodeRabbit findings on #6786, on top of the maintainer's `OneOf` fixup. 1. Nested `OneOf` inside a `Composite` was unproven. A top-level `OneOf` and a flat `Composite` do not show that recursion carries the "not guaranteed" answer UP through the composite, so `Composite([Tap, OneOf([Discard, PayLife])])` could have been credited by treating any nested discard as certain. Added that case plus its positive twin, `Composite([Tap, OneOf([Discard, Discard])])`, so the boundary is pinned from both sides rather than only the rejecting one. 2. Several cost-path tests asserted only `reason.kind`, which would stay green if the guaranteed-composite path returned 0.0 or a neutral path returned a nonzero score. They now capture and assert `delta` too — including the maintainer's `one_of_cost_with_only_discard_branches_is_credited_at_the_live_seam`, which promised positive credit without checking it. `empty_one_of_cost_is_not_credited` is deliberately NOT claimed as a guard on the `!costs.is_empty()` precondition. Mutation testing showed it stays green with that check deleted: an empty `OneOf` never reaches the walk, because the engine's `cost_categories()` gate reports no `Discards` category for a branch list with nothing in it. The test is kept because the outcome is worth pinning at whichever layer enforces it, and its comment now says exactly that instead of implying coverage it does not provide. Tests 69 → 72. `composite_wrapping_a_mixed_one_of_is_not_credited` is proven RED by flipping the `OneOf` live-seam quantifier from `all` to `any`. Verified: `cargo test -p phase-ai` 1857 passed, `cargo clippy -p phase-ai -p phase-engine --all-targets --features proptest -- -D warnings` exit 0. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/policies/tests/discard_payoff.rs | 101 +++++++++++++++++- 1 file changed, 98 insertions(+), 3 deletions(-) diff --git a/crates/phase-ai/src/policies/tests/discard_payoff.rs b/crates/phase-ai/src/policies/tests/discard_payoff.rs index 62e1dae9a9..f7ce903d4e 100644 --- a/crates/phase-ai/src/policies/tests/discard_payoff.rs +++ b/crates/phase-ai/src/policies/tests/discard_payoff.rs @@ -500,10 +500,14 @@ fn composite_cost_containing_a_discard_is_credited() { let context = context(&config, session(0.8)); let candidate = activate(outlet, 0); let decision = priority_decision(&candidate); - let (_, reason) = + let (delta, reason) = score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); assert_eq!(reason.kind, "discard_payoff_engine_active"); + assert!( + delta > 0.0, + "a guaranteed composite discard must earn positive credit, got {delta}" + ); } #[test] @@ -554,10 +558,11 @@ fn one_of_cost_with_only_discard_branches_is_credited_at_the_live_seam() { let context = context(&config, session(0.8)); let candidate = activate(outlet, 0); let decision = priority_decision(&candidate); - let (_, reason) = + let (delta, reason) = score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); assert_eq!(reason.kind, "discard_payoff_engine_active"); + assert!(delta > 0.0, "expected positive credit, got {delta}"); } #[test] @@ -570,10 +575,11 @@ fn zero_count_discard_cost_is_not_credited() { let context = context(&config, session(0.8)); let candidate = activate(outlet, 0); let decision = priority_decision(&candidate); - let (_, reason) = + let (delta, reason) = score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); assert_eq!(reason.kind, "discard_payoff_na"); + assert_eq!(delta, 0.0); } #[test] @@ -590,3 +596,92 @@ fn discard_cost_without_an_engine_is_neutral() { assert_eq!(reason.kind, "discard_payoff_no_engine"); } + +#[test] +fn composite_wrapping_a_mixed_one_of_is_not_credited() { + // CR 601.2h + CR 118.12a: the composite is fully paid, but its discard sits + // inside a mixed `OneOf`, so the player can settle the cost without ever + // discarding. Recursion must carry the "not guaranteed" answer up through + // the composite rather than treating any nested discard as certain. + let mut st = state(); + engine_on_battlefield(&mut st, Some(discarded_engine_trigger())); + let outlet = cost_paying_permanent( + &mut st, + AbilityCost::Composite { + costs: vec![ + AbilityCost::Tap, + AbilityCost::OneOf { + costs: vec![ + discard_cost(1), + AbilityCost::PayLife { + amount: QuantityExpr::Fixed { value: 2 }, + }, + ], + }, + ], + }, + ); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = activate(outlet, 0); + let decision = priority_decision(&candidate); + let (delta, reason) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "discard_payoff_na"); + assert_eq!(delta, 0.0); +} + +#[test] +fn composite_wrapping_an_all_discard_one_of_is_credited() { + // The positive twin: nesting must not lose a guaranteed discard either. + // Every branch of the inner `OneOf` discards, so the composite does too. + let mut st = state(); + engine_on_battlefield(&mut st, Some(discarded_engine_trigger())); + let outlet = cost_paying_permanent( + &mut st, + AbilityCost::Composite { + costs: vec![ + AbilityCost::Tap, + AbilityCost::OneOf { + costs: vec![discard_cost(1), discard_cost(2)], + }, + ], + }, + ); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = activate(outlet, 0); + let decision = priority_decision(&candidate); + let (delta, reason) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "discard_payoff_engine_active"); + assert!(delta > 0.0); +} + +#[test] +fn empty_one_of_cost_is_not_credited() { + // Behavioral assertion, NOT a guard on `cost_discards`'s `!costs.is_empty()` + // precondition: an empty `OneOf` never reaches that walk, because the engine's + // `cost_categories()` gate reports no `Discards` category for a branch list + // with nothing in it. Verified by mutation — deleting the emptiness check + // leaves this green. It is kept because the OUTCOME is worth pinning (a cost + // that pays nothing must not be credited a discard) at whichever layer + // enforces it, but it must not be counted as covering that precondition. + let mut st = state(); + engine_on_battlefield(&mut st, Some(discarded_engine_trigger())); + let outlet = cost_paying_permanent(&mut st, AbilityCost::OneOf { costs: Vec::new() }); + + let config = AiConfig::default(); + let context = context(&config, session(0.8)); + let candidate = activate(outlet, 0); + let decision = priority_decision(&candidate); + let (delta, reason) = + score_of(DiscardPayoffPolicy.verdict(&ctx(&st, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "discard_payoff_na"); + assert_eq!(delta, 0.0); +}