From d8ce4aaf0c167de33e93186389f057f5b9931ef4 Mon Sep 17 00:00:00 2001 From: minion1227 Date: Thu, 23 Jul 2026 03:10:01 -0700 Subject: [PATCH 1/5] feat(phase-ai): add poison-clock deck-feature axis + PoisonClockPolicy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CR 104.3d makes ten poison counters a loss condition entirely independent of life total, tracked in a dedicated `Player.poison_counters` field. Nothing in the AI scored progress along it, so an infect/toxic deck's whole plan read as doing nothing: a proliferate taking an opponent from 9 to 10 poison scored the same as one taking them from 0 to 1. Feature (`features/poison.rs`) — three structural axes, no card names: * sources: Keyword::Toxic(N) / Infect / Poisonous(N) on a creature face (CR 702.164 / 702.90 / 702.70 — all three are combat-damage abilities, so a noncreature face carrying the keyword is rejected) * direct: Effect::GivePlayerCounter { counter_kind: Poison, target } whose target can resolve to an opponent (CR 122.1f). A Controller/SelfRef scope is rejected so a self-poison drawback is not read as a payoff * proliferate: Effect::Proliferate | ProliferateTarget (CR 701.34a) Commitment is a weighted sum, not a geometric mean: a dedicated Infect deck runs zero direct-poison spells and often zero proliferate and must still read as fully committed. Calibrated to Modern Infect (12 infect creatures + 2 proliferate over 37 nonland) > 0.85, with a superfriends anti-calibration (2 proliferate, no source) staying below the policy floor. Policy (`policies/poison.rs`) scores CastSpell + ActivateAbility by clock progress, critical band when the action reaches ten. The branch structure is driven by a rules detail: CR 701.34a defines proliferate over permanents and players that ALREADY have a counter, so proliferating with no poisoned opponent advances nothing and scores zero rather than a nudge — the inverse would push the AI to durdle before the clock has started. Every predicate is card-local or a plain u32 field read; no board-wide sweep, no affordability call, nothing on the documented inner-loop landmine list. `poison_clock_pressure` is registered UNTUNED: it is a CR 104.3d win-detector weight whose magnitude is load-bearing for correctness, not taste. Boundary with plus_one_counters: that axis already counts Proliferate as a +1/+1 enabler. The overlap is intentional and the axes stay independent — one proliferate card genuinely serves both decks, and a Hardened Scales deck scores high there while scoring ~0 here. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/phase-ai/src/config.rs | 11 + crates/phase-ai/src/features/mod.rs | 5 + crates/phase-ai/src/features/poison.rs | 219 +++++++++++++++++++ crates/phase-ai/src/features/tests/mod.rs | 1 + crates/phase-ai/src/features/tests/poison.rs | 207 ++++++++++++++++++ crates/phase-ai/src/policies/mod.rs | 1 + crates/phase-ai/src/policies/poison.rs | 168 ++++++++++++++ crates/phase-ai/src/policies/registry.rs | 4 + crates/phase-ai/src/policies/tests/mod.rs | 1 + crates/phase-ai/src/policies/tests/poison.rs | 54 +++++ 10 files changed, 671 insertions(+) create mode 100644 crates/phase-ai/src/features/poison.rs create mode 100644 crates/phase-ai/src/features/tests/poison.rs create mode 100644 crates/phase-ai/src/policies/poison.rs create mode 100644 crates/phase-ai/src/policies/tests/poison.rs diff --git a/crates/phase-ai/src/config.rs b/crates/phase-ai/src/config.rs index 049149279c..118c09f420 100644 --- a/crates/phase-ai/src/config.rs +++ b/crates/phase-ai/src/config.rs @@ -463,6 +463,10 @@ pub struct PolicyPenalties { /// scalar.) #[serde(default = "default_loop_shortcut_winning_declare_bonus")] pub loop_shortcut_winning_declare_bonus: f64, + /// CR 104.3d: card-equivalent weight for advancing the poison clock. + /// Critical band when the action reaches ten poison (a win), scaled by + /// clock progress below that. + pub poison_clock_pressure: f64, } impl Default for PolicyPenalties { @@ -527,6 +531,7 @@ impl Default for PolicyPenalties { cycling_patience_penalty: default_cycling_patience_penalty(), cycling_needed_land_penalty: default_cycling_needed_land_penalty(), loop_shortcut_winning_declare_bonus: default_loop_shortcut_winning_declare_bonus(), + poison_clock_pressure: 6.0, } } } @@ -724,6 +729,12 @@ pub const ACTIVE_POLICY_PENALTY_FIELDS: &[&str] = &[ /// Policy penalties intentionally not present in an active CMA-ES parameter /// vector yet. pub const UNTUNED_POLICY_PENALTY_FIELDS: &[(&str, &str)] = &[ + ( + "poison_clock_pressure", + "CR 104.3d win-detector weight — a critical-band term whose magnitude is \ + load-bearing for correctness, not taste. Promote to ACTIVE only with a \ + paired-seed ai-gate calibration.", + ), ( "artifact_cost_payoff_bonus", "new ArtifactSynergyPolicy knob; awaiting a paired-seed ai-gate calibration before joining the CMA-ES vector", diff --git a/crates/phase-ai/src/features/mod.rs b/crates/phase-ai/src/features/mod.rs index a0430f971a..798a031ad8 100644 --- a/crates/phase-ai/src/features/mod.rs +++ b/crates/phase-ai/src/features/mod.rs @@ -20,6 +20,7 @@ pub mod lifegain; pub mod mana_ramp; pub mod mill; pub mod plus_one_counters; +pub mod poison; pub mod reanimator; pub mod spellslinger_prowess; pub mod tokens_wide; @@ -41,6 +42,7 @@ pub use lifegain::LifegainFeature; pub use mana_ramp::ManaRampFeature; pub use mill::MillFeature; pub use plus_one_counters::PlusOneCountersFeature; +pub use poison::PoisonFeature; pub use reanimator::ReanimatorFeature; pub use spellslinger_prowess::SpellslingerProwessFeature; pub use tokens_wide::TokensWideFeature; @@ -78,6 +80,8 @@ pub struct DeckFeatures { pub reanimator: ReanimatorFeature, pub mill: MillFeature, pub energy: EnergyFeature, + /// CR 104.3d: the alternate poison win clock (toxic / infect / proliferate). + pub poison: PoisonFeature, /// 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 @@ -122,6 +126,7 @@ impl DeckFeatures { reanimator: reanimator::detect(deck), mill: mill::detect(deck), energy: energy::detect(deck), + poison: poison::detect(deck), bracket_tier: tier, } } diff --git a/crates/phase-ai/src/features/poison.rs b/crates/phase-ai/src/features/poison.rs new file mode 100644 index 0000000000..5fe74fdb1f --- /dev/null +++ b/crates/phase-ai/src/features/poison.rs @@ -0,0 +1,219 @@ +//! Poison feature — structural detection of the alternate-win poison clock. +//! +//! Parser AST verification — VERIFIED against engine source: +//! - `Keyword::Toxic(u32)` at `crates/engine/src/types/keywords.rs:912` +//! (CR 702.164: "Toxic N — when this creature deals combat damage to a +//! player, that player gets N poison counters"). +//! - `Keyword::Infect` at `crates/engine/src/types/keywords.rs:612` +//! (CR 702.90: damage to players is dealt as poison counters). +//! - `Keyword::Poisonous(u32)` at `crates/engine/src/types/keywords.rs:898` +//! (CR 702.70). +//! - `Effect::GivePlayerCounter { counter_kind, count, target }` at +//! `crates/engine/src/types/ability.rs:12230`; `PlayerCounterKind::Poison` at +//! `crates/engine/src/types/player.rs:56` (CR 122.1f). Note this effect scopes +//! by `TargetFilter`, NOT by `CountScope` — the sibling `QuantityRef` arm at +//! `ability.rs:5443` is a counter *reader*, not a granter. +//! - `Effect::Proliferate` / `Effect::ProliferateTarget { target }` at +//! `crates/engine/src/types/ability.rs:10450` and `:10457` (CR 701.34a). +//! +//! No parser remediation required — every axis is expressible over existing +//! typed AST. +//! +//! ## Why this axis exists +//! +//! CR 104.3d makes ten poison counters a loss condition entirely independent of +//! life total, and `Player.poison_counters` is a dedicated engine field +//! (`crates/engine/src/types/player.rs:117`). The AI's evaluation is +//! life-total-centric, so a deck whose whole clock is poison reads as doing +//! nothing. This axis lets a policy see the second clock. +//! +//! ## Boundary with `plus_one_counters` +//! +//! `plus_one_counters` already counts `Effect::Proliferate` as a +1/+1 counter +//! enabler. That overlap is intentional and the axes stay independent: one +//! proliferate card genuinely serves both decks, and a Hardened Scales deck +//! scores high there while scoring ~0 here (no poison sources). + +use engine::game::DeckEntry; +use engine::types::ability::{AbilityDefinition, ControllerRef, Effect, TargetFilter}; +use engine::types::card_type::CoreType; +use engine::types::keywords::Keyword; +use engine::types::player::PlayerCounterKind; + +use crate::ability_chain::collect_chain_effects; +use crate::features::commitment; + +/// Commitment at or above which the poison clock is a real plan for this deck +/// rather than an incidental splash. Gates `PoisonClockPolicy::activation`. +pub const POISON_CLOCK_FLOOR: f32 = 0.35; + +/// CR 104.3d: a player with ten or more poison counters loses the game. +pub const LETHAL_POISON: u32 = 10; + +/// CR 104.3d + CR 122.1f: per-deck poison-clock classification. +/// +/// Populated once per game from `DeckEntry` data. Detection is structural over +/// `CardFace.keywords` and `CardFace.abilities` — never by card name. +#[derive(Debug, Clone, Default)] +pub struct PoisonFeature { + /// Creatures that convert combat damage into poison counters — CR 702.164 + /// Toxic, CR 702.90 Infect, CR 702.70 Poisonous. + pub source_count: u32, + /// Cards whose effect chain gives poison counters to opponents outright, + /// without needing combat damage (CR 122.1f). + pub direct_count: u32, + /// Cards that proliferate (CR 701.34a) — the accelerant on an established + /// poison clock. + pub proliferate_count: u32, + /// `0.0..=1.0` — how central the poison clock is to this deck. Consumed by + /// `PoisonClockPolicy::activation` as the single scaling knob. + pub commitment: f32, + /// Names of detected poison sources. NOT used for classification — that + /// already happened against the AST. Used as battlefield identifiers at + /// decision time (identity lookup, exempt from the name-matching lint). + pub source_names: Vec, +} + +/// Structural detection over each `DeckEntry`'s `CardFace` AST. +pub fn detect(deck: &[DeckEntry]) -> PoisonFeature { + if deck.is_empty() { + return PoisonFeature::default(); + } + + let mut source_count = 0u32; + let mut direct_count = 0u32; + let mut proliferate_count = 0u32; + let mut total_nonland = 0u32; + let mut source_names: Vec = Vec::new(); + + 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); + } + + let is_source = is_poison_source_parts(&face.card_type.core_types, &face.keywords); + let is_direct = gives_opponents_poison_parts(&face.abilities); + + if is_source { + source_count = source_count.saturating_add(entry.count); + } + if is_direct { + direct_count = direct_count.saturating_add(entry.count); + } + // One push per UNIQUE face, and once even when both axes fire — + // guards the per-copy and double-push traps. + if is_source || is_direct { + source_names.push(face.name.clone()); + } + if proliferates_parts(&face.abilities) { + proliferate_count = proliferate_count.saturating_add(entry.count); + } + } + + let commitment = + compute_commitment(source_count, direct_count, proliferate_count, total_nonland); + + PoisonFeature { + source_count, + direct_count, + proliferate_count, + commitment, + source_names, + } +} + +/// Calibration: Modern Infect (12 infect creatures + 2 proliferate over 37 +/// nonland) → commitment ≈ 0.90. Anti-calibration: a superfriends deck running +/// 2 proliferate cards and no poison source → ≈ 0.06, far below +/// `POISON_CLOCK_FLOOR`; UW control → 0.0. +/// +/// Weighted sum rather than geometric mean: missing pillars are tolerable here. +/// A dedicated Infect deck runs zero direct-poison spells and often zero +/// proliferate, and must still read as fully committed. +fn compute_commitment( + source_count: u32, + direct_count: u32, + proliferate_count: u32, + total_nonland: u32, +) -> f32 { + commitment::weighted_sum(&[ + // Full pillar at ~15 sources per 60 nonland — a dedicated poison deck. + ( + 0.65 / 15.0, + commitment::density_per_60(source_count, total_nonland), + ), + // Direct poison is rarer per deck, so it saturates sooner. + ( + 0.20 / 6.0, + commitment::density_per_60(direct_count, total_nonland), + ), + // Proliferate alone is never a poison plan — it only accelerates one. + ( + 0.15 / 8.0, + commitment::density_per_60(proliferate_count, total_nonland), + ), + ]) +} + +/// CR 702.164 / CR 702.90 / CR 702.70: a creature whose combat damage becomes +/// poison counters. Non-creature faces never qualify — all three keywords are +/// combat-damage abilities. +pub(crate) fn is_poison_source_parts(core_types: &[CoreType], keywords: &[Keyword]) -> bool { + if !core_types.contains(&CoreType::Creature) { + return false; + } + keywords.iter().any(|k| { + matches!( + k, + Keyword::Toxic(_) | Keyword::Infect | Keyword::Poisonous(_) + ) + }) +} + +/// CR 122.1f: an ability chain that gives poison counters to a player who can +/// be an opponent. +/// +/// A `TargetFilter::Controller` / `SelfRef` scope is rejected — a card that +/// poisons ITS OWN controller (a drawback clause, e.g. Phyrexian Vatmother) is +/// not a poison payoff. +pub(crate) fn gives_opponents_poison_parts(abilities: &[AbilityDefinition]) -> bool { + abilities.iter().any(|ability| { + collect_chain_effects(ability).iter().any(|effect| { + matches!( + effect, + Effect::GivePlayerCounter { + counter_kind: PlayerCounterKind::Poison, + target, + .. + } if filter_can_hit_opponent(target) + ) + }) + }) +} + +/// True when a `TargetFilter` can resolve to a player other than the ability's +/// controller. Mirrors `landfall::filter_matches_land_you_control`'s recursion, +/// including the CR 109.3 conjunction rule for `And`. +fn filter_can_hit_opponent(filter: &TargetFilter) -> bool { + match filter { + TargetFilter::Opponent | TargetFilter::Player | TargetFilter::Any => true, + TargetFilter::Typed(typed) => !matches!(typed.controller, Some(ControllerRef::You)), + TargetFilter::Or { filters } => filters.iter().any(filter_can_hit_opponent), + // CR 109.3: every constraint of an `And` must hold for the match. + TargetFilter::And { filters } => filters.iter().all(filter_can_hit_opponent), + _ => false, + } +} + +/// CR 701.34a: an ability chain containing either proliferate form. +pub(crate) fn proliferates_parts(abilities: &[AbilityDefinition]) -> bool { + abilities.iter().any(|ability| { + collect_chain_effects(ability).iter().any(|effect| { + matches!( + effect, + Effect::Proliferate | Effect::ProliferateTarget { .. } + ) + }) + }) +} diff --git a/crates/phase-ai/src/features/tests/mod.rs b/crates/phase-ai/src/features/tests/mod.rs index 5e2c7e1e87..a7a85c1374 100644 --- a/crates/phase-ai/src/features/tests/mod.rs +++ b/crates/phase-ai/src/features/tests/mod.rs @@ -9,4 +9,5 @@ pub mod equipment; pub mod lifegain; pub mod mill; pub mod no_name_matching; +pub mod poison; pub mod reanimator; diff --git a/crates/phase-ai/src/features/tests/poison.rs b/crates/phase-ai/src/features/tests/poison.rs new file mode 100644 index 0000000000..072da660b3 --- /dev/null +++ b/crates/phase-ai/src/features/tests/poison.rs @@ -0,0 +1,207 @@ +//! Unit tests for `features::poison` — structural detection + calibration +//! anchors for the CR 104.3d poison clock. No `#[cfg(test)]` in SOURCE +//! files; tests live here. + +use engine::game::DeckEntry; +use engine::types::ability::{AbilityDefinition, Effect, TargetFilter}; +use engine::types::card_type::CoreType; +use engine::types::keywords::Keyword; +use engine::types::player::PlayerCounterKind; + +use crate::features::poison::*; +use engine::types::ability::{AbilityKind, QuantityExpr}; +use engine::types::card::CardFace; +use engine::types::card_type::CardType; + +fn creature(name: &str) -> CardFace { + CardFace { + name: name.to_string(), + card_type: CardType { + supertypes: Vec::new(), + core_types: vec![CoreType::Creature], + subtypes: Vec::new(), + }, + ..Default::default() + } +} + +fn spell(name: &str) -> CardFace { + CardFace { + name: name.to_string(), + card_type: CardType { + supertypes: Vec::new(), + core_types: vec![CoreType::Instant], + subtypes: Vec::new(), + }, + ..Default::default() + } +} + +fn entry(card: CardFace, count: u32) -> DeckEntry { + DeckEntry { card, count } +} + +fn infect_creature(name: &str) -> CardFace { + let mut face = creature(name); + face.keywords = vec![Keyword::Infect]; + face +} + +/// CR 122.1f: gives poison to a player who can be an opponent. +fn poison_spell(name: &str, target: TargetFilter) -> CardFace { + let mut face = spell(name); + face.abilities = vec![AbilityDefinition::new( + AbilityKind::Spell, + Effect::GivePlayerCounter { + counter_kind: PlayerCounterKind::Poison, + count: QuantityExpr::Fixed { value: 1 }, + target, + }, + )]; + face +} + +fn proliferate_spell(name: &str) -> CardFace { + let mut face = spell(name); + face.abilities = vec![AbilityDefinition::new( + AbilityKind::Spell, + Effect::Proliferate, + )]; + face +} + +#[test] +fn empty_deck_produces_defaults() { + let feature = detect(&[]); + assert_eq!(feature.source_count, 0); + assert_eq!(feature.commitment, 0.0); + assert!(feature.source_names.is_empty()); +} + +#[test] +fn vanilla_creature_not_registered() { + let feature = detect(&[entry(creature("Grizzly Bears"), 4)]); + assert_eq!(feature.source_count, 0); + assert_eq!(feature.commitment, 0.0); +} + +#[test] +fn detects_infect_toxic_and_poisonous_sources() { + for keyword in [Keyword::Infect, Keyword::Toxic(1), Keyword::Poisonous(2)] { + let mut face = creature("Source"); + face.keywords = vec![keyword.clone()]; + let feature = detect(&[entry(face, 4)]); + assert_eq!( + feature.source_count, 4, + "keyword {keyword:?} must register as a poison source" + ); + } +} + +#[test] +fn detects_direct_poison_effect() { + let feature = detect(&[entry( + poison_spell("Virulent Wound", TargetFilter::Opponent), + 4, + )]); + assert_eq!(feature.direct_count, 4); +} + +#[test] +fn detects_proliferate() { + let feature = detect(&[entry(proliferate_spell("Contagion Clasp"), 2)]); + assert_eq!(feature.proliferate_count, 2); +} + +/// CR 702.164 / 702.90 / 702.70 are combat-damage abilities — a noncreature +/// face carrying the keyword is not a poison clock. +#[test] +fn noncreature_with_infect_does_not_count() { + let mut face = spell("Not A Creature"); + face.keywords = vec![Keyword::Infect]; + let feature = detect(&[entry(face, 4)]); + assert_eq!(feature.source_count, 0); +} + +/// A drawback clause that poisons its OWN controller (Phyrexian Vatmother +/// shape) is not a payoff. +#[test] +fn self_poison_drawback_not_counted() { + let feature = detect(&[entry( + poison_spell("Self Poisoner", TargetFilter::Controller), + 4, + )]); + assert_eq!(feature.direct_count, 0); +} + +/// One push per UNIQUE face, not per playset copy. +#[test] +fn source_names_dedup_per_face() { + let feature = detect(&[entry(infect_creature("Glistener Elf"), 4)]); + assert_eq!(feature.source_count, 4); + assert_eq!(feature.source_names, vec!["Glistener Elf".to_string()]); +} + +/// A face that is BOTH a source and a direct-poison card pushes its name once. +#[test] +fn source_and_direct_face_pushes_name_once() { + let mut face = infect_creature("Hybrid Threat"); + face.abilities = vec![AbilityDefinition::new( + AbilityKind::Spell, + Effect::GivePlayerCounter { + counter_kind: PlayerCounterKind::Poison, + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Opponent, + }, + )]; + let feature = detect(&[entry(face, 2)]); + assert_eq!(feature.source_names.len(), 1); +} + +/// Calibration anchor: Modern Infect — 12 infect creatures + 2 proliferate +/// over 37 nonland cards → strongly committed. +#[test] +fn modern_infect_hits_calibration_floor() { + let deck = vec![ + entry(infect_creature("Glistener Elf"), 4), + entry(infect_creature("Blighted Agent"), 4), + entry(infect_creature("Phyrexian Crusader"), 4), + entry(proliferate_spell("Contagion Clasp"), 2), + entry(spell("Pump Spell"), 23), + ]; + let feature = detect(&deck); + assert_eq!(feature.source_count, 12); + assert!( + feature.commitment > 0.85, + "Modern Infect must clear 0.85, got {}", + feature.commitment + ); +} + +/// Anti-calibration: a superfriends deck running proliferate but no poison +/// source must stay far below the policy floor. +#[test] +fn superfriends_proliferate_stays_below_floor() { + let deck = vec![ + entry(proliferate_spell("Contagion Clasp"), 2), + entry(spell("Planeswalker Filler"), 35), + ]; + let feature = detect(&deck); + assert!( + feature.commitment < POISON_CLOCK_FLOOR, + "proliferate alone is not a poison plan, got {}", + feature.commitment + ); +} + +#[test] +fn control_deck_below_floor() { + let deck = vec![entry(spell("Counterspell"), 37)]; + assert_eq!(detect(&deck).commitment, 0.0); +} + +#[test] +fn commitment_clamps_to_one() { + let deck = vec![entry(infect_creature("All Infect"), 37)]; + assert_eq!(detect(&deck).commitment, 1.0); +} diff --git a/crates/phase-ai/src/policies/mod.rs b/crates/phase-ai/src/policies/mod.rs index 237ab920e2..bba1bcf225 100644 --- a/crates/phase-ai/src/policies/mod.rs +++ b/crates/phase-ai/src/policies/mod.rs @@ -38,6 +38,7 @@ mod payment_selection; mod payoff; mod planeswalker_loyalty; mod plus_one_counters; +mod poison; mod ramp_timing; mod reactive_self_protection; mod recursion_awareness; diff --git a/crates/phase-ai/src/policies/poison.rs b/crates/phase-ai/src/policies/poison.rs new file mode 100644 index 0000000000..f3a9f852f3 --- /dev/null +++ b/crates/phase-ai/src/policies/poison.rs @@ -0,0 +1,168 @@ +//! `PoisonClockPolicy` — makes the CR 104.3d poison clock visible to an AI +//! whose evaluation is otherwise life-total-centric. +//! +//! ## The defect this closes +//! +//! CR 104.3d: "If a player has ten or more poison counters, that player loses +//! the game the next time a player would receive priority." That is a win +//! condition entirely independent of life total, tracked in a dedicated engine +//! field (`Player.poison_counters`). Nothing in the AI scored progress along +//! it, so an infect/toxic deck's whole plan registered as doing nothing: a +//! proliferate that takes an opponent from 9 to 10 poison scored the same as +//! one that took them from 0 to 1. +//! +//! ## Rules-correctness note that drives the branch structure +//! +//! CR 701.34a defines proliferate over "permanents and/or players that **have +//! a counter**". Proliferating when no opponent is poisoned adds nothing on +//! this axis — so that branch scores zero rather than a nudge. Getting this +//! backwards would push the AI to durdle with proliferate before the clock has +//! started. +//! +//! ## Performance +//! +//! `verdict()` runs per candidate per search node. Every predicate here is +//! card-local (`obj.abilities` / `obj.keywords`) or a plain `u32` field read on +//! `Player.poison_counters`. No board-wide sweep, no affordability call, no +//! `find_legal_targets` — nothing this policy touches is on the documented +//! inner-loop landmine list. + +use engine::types::actions::GameAction; +use engine::types::game_state::GameState; +use engine::types::player::PlayerId; + +use crate::features::poison::{LETHAL_POISON, POISON_CLOCK_FLOOR}; +use crate::features::DeckFeatures; + +use super::context::PolicyContext; +use super::registry::{DecisionKind, PolicyId, PolicyReason, PolicyVerdict, TacticalPolicy}; + +pub struct PoisonClockPolicy; + +/// What the candidate action contributes to the poison clock. Typed rather +/// than a pair of bools so the branch set stays exhaustive. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum PoisonContribution { + /// Adds poison counters to a player who can be an opponent (CR 122.1f). + DirectPoison, + /// Proliferates (CR 701.34a) — only advances the clock on an already + /// poisoned opponent. + Proliferate, + /// Nothing to do with the poison clock. + None, +} + +impl PoisonClockPolicy { + /// Re-classify the LIVE candidate structurally. Deck-time classification is + /// deliberately not trusted here — the object on the battlefield may have + /// been modified since the deck was analyzed. + fn contribution(&self, ctx: &PolicyContext<'_>) -> PoisonContribution { + let abilities = match &ctx.candidate.action { + GameAction::CastSpell { object_id, .. } => ctx + .state + .objects + .get(object_id) + .map(|obj| obj.abilities.as_slice()), + GameAction::ActivateAbility { + source_id, + ability_index, + } => ctx + .state + .objects + .get(source_id) + .and_then(|obj| obj.abilities.get(*ability_index)) + .map(std::slice::from_ref), + _ => None, + }; + let Some(abilities) = abilities else { + return PoisonContribution::None; + }; + + // Cheapest discriminator first: direct poison outranks proliferate + // because it advances the clock without needing an existing counter. + if crate::features::poison::gives_opponents_poison_parts(abilities) { + PoisonContribution::DirectPoison + } else if crate::features::poison::proliferates_parts(abilities) { + PoisonContribution::Proliferate + } else { + PoisonContribution::None + } + } +} + +/// CR 104.3d: the highest poison total among the AI's opponents, and whether +/// any opponent is "poisoned" (CR 122.1f — one or more poison counters). +pub(crate) fn most_poisoned_opponent(state: &GameState, ai_player: PlayerId) -> u32 { + state + .players + .iter() + .enumerate() + .filter(|(index, _)| PlayerId(*index as u8) != ai_player) + .map(|(_, player)| player.poison_counters) + .max() + .unwrap_or(0) +} + +/// CR 104.3d: would one more poison counter put this player at ten or more, +/// losing them the game the next time a player would receive priority? +pub(crate) fn reaches_lethal(current_poison: u32) -> bool { + current_poison.saturating_add(1) >= LETHAL_POISON +} + +impl TacticalPolicy for PoisonClockPolicy { + fn id(&self) -> PolicyId { + PolicyId::PoisonClock + } + + fn decision_kinds(&self) -> &'static [DecisionKind] { + &[DecisionKind::CastSpell, DecisionKind::ActivateAbility] + } + + fn activation( + &self, + features: &DeckFeatures, + _state: &GameState, + _player: PlayerId, + ) -> Option { + if features.poison.commitment < POISON_CLOCK_FLOOR { + None + } else { + Some(features.poison.commitment) + } + } + + fn verdict(&self, ctx: &PolicyContext<'_>) -> PolicyVerdict { + let contribution = self.contribution(ctx); + if contribution == PoisonContribution::None { + return PolicyVerdict::neutral(PolicyReason::new("poison_clock_na")); + } + + let highest = most_poisoned_opponent(ctx.state, ctx.ai_player); + let scalar = ctx.config.policy_penalties.poison_clock_pressure; + + // CR 701.34a: proliferate needs an existing counter. With no poisoned + // opponent it advances nothing, so it earns nothing on this axis. + if contribution == PoisonContribution::Proliferate && highest == 0 { + return PolicyVerdict::neutral(PolicyReason::new( + "poison_clock_no_counters_to_proliferate", + )); + } + + // CR 104.3d: one more poison counter ends the game. + if reaches_lethal(highest) { + return PolicyVerdict::critical( + scalar, + PolicyReason::new("poison_clock_lethal") + .with_fact("opponent_poison", highest as i64), + ); + } + + // Below lethal, value scales with how far the clock has already run — + // the last counters are worth more than the first. + let progress = f64::from(highest) / f64::from(LETHAL_POISON); + PolicyVerdict::preference( + scalar * progress.max(0.25), + PolicyReason::new("poison_clock_pressure").with_fact("opponent_poison", highest as i64), + ) + } +} diff --git a/crates/phase-ai/src/policies/registry.rs b/crates/phase-ai/src/policies/registry.rs index ab6796c3eb..1da04d6872 100644 --- a/crates/phase-ai/src/policies/registry.rs +++ b/crates/phase-ai/src/policies/registry.rs @@ -29,6 +29,7 @@ use super::payoff::{ EQUIPMENT_PAYOFF, LIFEGAIN_PAYOFF, MILL_PAYOFF, REANIMATOR_PAYOFF, }; use super::plus_one_counters::PlusOneCountersPolicy; +use super::poison::PoisonClockPolicy; use super::ramp_timing::RampTimingPolicy; use super::reactive_self_protection::ReactiveSelfProtectionPolicy; use super::recursion_awareness::RecursionAwarenessPolicy; @@ -129,6 +130,8 @@ pub enum PolicyId { SeparatePilesTiming, XCastGate, LoopShortcut, + /// CR 104.3d: the alternate poison win clock. + PoisonClock, } /// Coarse routing kind for a candidate decision. Each policy declares which @@ -320,6 +323,7 @@ impl Default for PolicyRegistry { Box::new(PayoffPolicy::new(&ARTIFACT_SYNERGY)), Box::new(BoardDevelopmentPolicy), Box::new(EtbValuePolicy), + Box::new(PoisonClockPolicy), Box::new(PayoffPolicy::new(&ENCHANTMENTS_PAYOFF)), Box::new(PayoffPolicy::new(&EQUIPMENT_PAYOFF)), Box::new(CopyValuePolicy), diff --git a/crates/phase-ai/src/policies/tests/mod.rs b/crates/phase-ai/src/policies/tests/mod.rs index 46f9fc14d2..032349f134 100644 --- a/crates/phase-ai/src/policies/tests/mod.rs +++ b/crates/phase-ai/src/policies/tests/mod.rs @@ -10,5 +10,6 @@ pub mod equipment_payoff; pub mod lifegain_payoff; pub mod mill_payoff; pub mod mulligan_input_lint; +pub mod poison; pub mod reanimator_payoff; pub mod score_contract_lint; diff --git a/crates/phase-ai/src/policies/tests/poison.rs b/crates/phase-ai/src/policies/tests/poison.rs new file mode 100644 index 0000000000..35c25957ee --- /dev/null +++ b/crates/phase-ai/src/policies/tests/poison.rs @@ -0,0 +1,54 @@ +//! Unit tests for `policies::poison` — the CR 104.3d poison-clock policy. +//! No `#[cfg(test)]` in SOURCE files; tests live here. + +use crate::features::poison::{LETHAL_POISON, POISON_CLOCK_FLOOR}; +use crate::features::DeckFeatures; +use crate::policies::registry::TacticalPolicy; +use engine::types::game_state::GameState; +use engine::types::player::PlayerId; + +use crate::policies::poison::*; + +/// CR 104.3d: ten or more poison counters loses the game, so the ninth +/// counter is the lethal setup and the eighth is not. +#[test] +fn reaches_lethal_matches_cr_104_3d_boundary() { + assert_eq!(LETHAL_POISON, 10); + assert!(reaches_lethal(9), "9 + 1 == 10 is lethal"); + assert!(!reaches_lethal(8), "8 + 1 == 9 is not yet lethal"); + assert!(reaches_lethal(u32::MAX), "saturating add must not wrap"); +} + +#[test] +fn activation_opts_out_below_floor() { + let mut features = DeckFeatures::default(); + features.poison.commitment = POISON_CLOCK_FLOOR - 0.01; + let state = GameState::default(); + assert!(PoisonClockPolicy + .activation(&features, &state, PlayerId(0)) + .is_none()); +} + +#[test] +fn activation_opts_in_above_floor() { + let mut features = DeckFeatures::default(); + features.poison.commitment = 0.9; + let state = GameState::default(); + assert_eq!( + PoisonClockPolicy.activation(&features, &state, PlayerId(0)), + Some(0.9) + ); +} + +#[test] +fn most_poisoned_opponent_ignores_the_ai_itself() { + let mut state = GameState::default(); + while state.players.len() < 2 { + state.players.push(Default::default()); + } + state.players[0].poison_counters = 7; + state.players[1].poison_counters = 3; + // The AI's own 7 poison must not be read as pressure it is applying. + assert_eq!(most_poisoned_opponent(&state, PlayerId(0)), 3); + assert_eq!(most_poisoned_opponent(&state, PlayerId(1)), 7); +} From 5c235a5e86ab250ebe5bb3ba88b1e67de889dac8 Mon Sep 17 00:00:00 2001 From: minion1227 Date: Thu, 23 Jul 2026 13:04:06 -0700 Subject: [PATCH 2/5] fix(phase-ai): address review on poison-clock axis (#6543) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves the five blocking items from the maintainer review of #6543. 1. Preserve pre-field PolicyPenalties artifacts. `poison_clock_pressure` now has `#[serde(default = "default_poison_clock_pressure")]`, sharing one default fn with the `Default` impl (matching every sibling field). `ai_tune` deserializes the `policy_penalties` section straight into the struct, so an artifact written before this field must still load — a compatibility test drives that with a pre-poison artifact. 2. Handle modal abilities at the actual choice seam. `ability_chain` gains a typed `AbilityScope` (Unconditional | Potential) and one mode-aware authority, `collect_scoped_effects`. Deck-time detection walks `Potential` (mode_abilities + else_ability included, CR 700.2 / 608.2c); the live policy walks `Unconditional`. A modal cast is no longer credited with a mode's poison — the selected branch is scored at `SelectModes` (routed via ModeChoice / AbilityModeChoice → ActivateAbility), where the mode is actually chosen (CR 601.2b). The engine's own `modal_spell_mode_ability_refs` is the shared mode-list authority. 3. Route the creature poison clock through combat. The policy now declares `DeclareAttackers` and scores an attack by the poison its sources convert (CR 702.90b infect = power, CR 702.164c toxic = summed N, CR 702.70a poisonous), summed per defending player, keyed on damage to a PLAYER only (a planeswalker/battle attack adds nothing, CR 506.4c). 4. Use only live opponents. `most_poisoned_opponent` and the new `live_opponent_poison` filter `is_eliminated` seats (CR 800.4), so a dead player's counters can't produce phantom lethal/progress pressure. 5. Complete opponent-target classification. `filter_can_hit_opponent` gains a sound `Not` arm via a `filter_matches_every_opponent` complement, so `Not { SelfRef }` / `Not { Typed(controller: You) }` read as opponent poison while `Not { Opponent }` does not. Also fixes a latent scoring-contract bug the new verdict tests surfaced: the sub-lethal delta (`pressure` scalar 6.0 × progress) escaped the preference band, tripping `score_in_band`'s debug assert and silently clamping in release. Scoring now routes through `PolicyVerdict::score` with the non-lethal ceiling held at `STRONG_MAX`, so only reaching ten lands critical. Adds direct `verdict()` coverage (lethal / no-counters-to-proliferate / progress-scaled), the modal seam, combat routing (incl. eliminated-seat and planeswalker cases), negated player scopes, and registry-level routing for the modal and combat decision kinds. cargo test -p phase-ai --lib — 1412 passed cargo clippy -p phase-ai -p engine --all-targets --features proptest — clean Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/engine/src/game/ability_utils.rs | 13 +- crates/phase-ai/src/ability_chain.rs | 83 +- crates/phase-ai/src/config.rs | 40 +- crates/phase-ai/src/features/poison.rs | 101 ++- crates/phase-ai/src/features/tests/poison.rs | 247 +++++- crates/phase-ai/src/policies/poison.rs | 291 +++++-- crates/phase-ai/src/policies/tests/poison.rs | 788 ++++++++++++++++++- 7 files changed, 1461 insertions(+), 102 deletions(-) diff --git a/crates/engine/src/game/ability_utils.rs b/crates/engine/src/game/ability_utils.rs index 84f727bff0..09ff5c75d6 100644 --- a/crates/engine/src/game/ability_utils.rs +++ b/crates/engine/src/game/ability_utils.rs @@ -839,11 +839,20 @@ pub fn spell_modal_unavailable_modes( pub fn modal_spell_mode_abilities( obj: &crate::game::game_object::GameObject, ) -> Vec { + modal_spell_mode_ability_refs(obj).cloned().collect() +} + +/// Borrowing view of [`modal_spell_mode_abilities`] — the same predicate +/// without the per-call clone, for read-only consumers that only inspect the +/// modes (AI classification, coverage reporting). Both share this one +/// definition of "which abilities on this object are its printed modes" so the +/// owned and borrowed forms can never disagree. +pub fn modal_spell_mode_ability_refs( + obj: &crate::game::game_object::GameObject, +) -> impl Iterator { obj.abilities .iter() .filter(|a| a.kind == AbilityKind::Spell) - .cloned() - .collect() } /// CR 700.2a-b + CR 700.2f: Extends `unavailable_modes` with mode indices diff --git a/crates/phase-ai/src/ability_chain.rs b/crates/phase-ai/src/ability_chain.rs index eaf1e2a77d..87a7a06229 100644 --- a/crates/phase-ai/src/ability_chain.rs +++ b/crates/phase-ai/src/ability_chain.rs @@ -1,4 +1,4 @@ -//! Shared helpers for walking an `AbilityDefinition`'s effect chain. +//! Shared helpers for walking an `AbilityDefinition`'s effect tree. //! //! `AbilityDefinition` composes a primary `effect` with an optional //! `sub_ability` that itself has an `effect` plus another optional @@ -8,19 +8,84 @@ //! land onto the battlefield?"), so they collect the chain into a flat //! slice and iterate with `matches!`. //! +//! Two branches of that definition are *conditional* rather than part of the +//! unconditional chain: `else_ability` (the CR 608.2c "Otherwise, ..." leg) and +//! `mode_abilities` (the CR 700.2 modal alternatives). Whether they belong in +//! the walk depends on the question being asked, which is what [`AbilityScope`] +//! names — see its docs. [`collect_scoped_effects`] is the single authority for +//! both walks; `collect_chain_effects` is the unconditional shorthand. +//! //! Keep this module small — it is a single building block shared across //! `features/*` and `policies/*`. use engine::types::ability::{AbilityDefinition, Effect}; +/// Which part of an ability tree a classification question is asking about. +/// +/// The distinction is a decision-boundary one, not a convenience one: +/// +/// * [`AbilityScope::Unconditional`] answers *"does resolving this ability as +/// already announced produce effect X?"* — the walk a LIVE per-action policy +/// needs. CR 601.2b makes mode selection a distinct step of announcing a +/// spell, so at `CastSpell` time no mode has been chosen yet and a modal +/// branch must not be credited to the cast. +/// * [`AbilityScope::Potential`] answers *"can this card ever produce effect +/// X?"* — the walk DECK-TIME detection needs, where every branch the card +/// could take is in scope. +/// +/// A typed scope rather than a `bool` so each call site states which question +/// it is asking. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum AbilityScope { + /// `effect` plus the `sub_ability` chain — everything that happens no + /// matter which branch or mode is taken. + Unconditional, + /// The full tree: `Unconditional` plus the `else_ability` branch (CR + /// 603.4) and every entry of `mode_abilities` (CR 700.2), recursively. + Potential, +} + +/// Walk an ability tree at `scope`, returning borrowed effect references in +/// traversal order. +/// +/// This is the mode-aware authority both deck-time detection and live policy +/// classification share; they differ only in the `scope` they pass, so the two +/// can never drift apart on which branches exist. +pub(crate) fn collect_scoped_effects( + ability: &AbilityDefinition, + scope: AbilityScope, +) -> Vec<&Effect> { + let mut effects: Vec<&Effect> = Vec::new(); + push_scoped_effects(ability, scope, &mut effects); + effects +} + +fn push_scoped_effects<'a>( + ability: &'a AbilityDefinition, + scope: AbilityScope, + out: &mut Vec<&'a Effect>, +) { + out.push(&ability.effect); + if let Some(sub) = &ability.sub_ability { + push_scoped_effects(sub, scope, out); + } + if scope == AbilityScope::Unconditional { + return; + } + // CR 608.2c: the "Otherwise, ..." leg is one of two mutually exclusive + // outcomes, so it is potential rather than unconditional. + if let Some(other) = &ability.else_ability { + push_scoped_effects(other, scope, out); + } + // CR 700.2: each mode is an alternative the controller may choose. + for mode in &ability.mode_abilities { + push_scoped_effects(mode, scope, out); + } +} + /// Walk `ability.effect` plus each `sub_ability.effect` in turn, returning -/// borrowed references in chain order. +/// borrowed references in chain order. Shorthand for +/// [`collect_scoped_effects`] at [`AbilityScope::Unconditional`]. pub(crate) fn collect_chain_effects(ability: &AbilityDefinition) -> Vec<&Effect> { - let mut effects: Vec<&Effect> = vec![&ability.effect]; - let mut current = &ability.sub_ability; - while let Some(sub) = current { - effects.push(&sub.effect); - current = &sub.sub_ability; - } - effects + collect_scoped_effects(ability, AbilityScope::Unconditional) } diff --git a/crates/phase-ai/src/config.rs b/crates/phase-ai/src/config.rs index 118c09f420..29f2a56d70 100644 --- a/crates/phase-ai/src/config.rs +++ b/crates/phase-ai/src/config.rs @@ -466,6 +466,7 @@ pub struct PolicyPenalties { /// CR 104.3d: card-equivalent weight for advancing the poison clock. /// Critical band when the action reaches ten poison (a win), scaled by /// clock progress below that. + #[serde(default = "default_poison_clock_pressure")] pub poison_clock_pressure: f64, } @@ -531,7 +532,7 @@ impl Default for PolicyPenalties { cycling_patience_penalty: default_cycling_patience_penalty(), cycling_needed_land_penalty: default_cycling_needed_land_penalty(), loop_shortcut_winning_declare_bonus: default_loop_shortcut_winning_declare_bonus(), - poison_clock_pressure: 6.0, + poison_clock_pressure: default_poison_clock_pressure(), } } } @@ -539,6 +540,12 @@ impl Default for PolicyPenalties { fn default_wasted_cast_penalty() -> f64 { -8.0 } +/// CR 104.3d. Shared by `Default` and `#[serde(default)]` so a tuning artifact +/// written before this field existed still deserializes (`ai_tune` reads the +/// `policy_penalties` section directly into this struct). +fn default_poison_clock_pressure() -> f64 { + 6.0 +} fn default_untap_own_tapped_bonus() -> f64 { 8.0 } @@ -1505,6 +1512,37 @@ mod tests { assert_eq!(p.cycling_needed_land_penalty, -2.0); } + /// Artifact compatibility: `ai_tune` deserializes a persisted + /// `policy_penalties` section straight into `PolicyPenalties` + /// (`bin/ai_tune.rs`, `TuneGroup::Penalties`), so an artifact written + /// before `poison_clock_pressure` existed must still load — with its own + /// tuned values intact and the new field filled from the shared default. + #[test] + fn policy_penalties_load_pre_poison_clock_artifact() { + let mut artifact = serde_json::to_value(PolicyPenalties::default()).unwrap(); + let object = artifact.as_object_mut().expect("serializes as object"); + object + .remove("poison_clock_pressure") + .expect("field must be present before removal"); + // A value CMA-ES could plausibly have tuned, to prove the round-trip + // reads the artifact rather than silently falling back to Default. + object.insert("wasted_cast_penalty".into(), serde_json::json!(-3.5)); + + let loaded: PolicyPenalties = serde_json::from_value(artifact) + .expect("a pre-poison_clock_pressure artifact must still deserialize"); + assert_eq!(loaded.wasted_cast_penalty, -3.5, "tuned value preserved"); + assert_eq!( + loaded.poison_clock_pressure, + default_poison_clock_pressure(), + "absent field must fall back to the shared default" + ); + assert_eq!( + PolicyPenalties::default().poison_clock_pressure, + default_poison_clock_pressure(), + "Default and serde must share one source of truth" + ); + } + #[test] fn every_policy_penalty_is_tuning_registered_or_explicitly_untuned() { let value = serde_json::to_value(PolicyPenalties::default()).unwrap(); diff --git a/crates/phase-ai/src/features/poison.rs b/crates/phase-ai/src/features/poison.rs index 5fe74fdb1f..18703975f8 100644 --- a/crates/phase-ai/src/features/poison.rs +++ b/crates/phase-ai/src/features/poison.rs @@ -40,7 +40,7 @@ use engine::types::card_type::CoreType; use engine::types::keywords::Keyword; use engine::types::player::PlayerCounterKind; -use crate::ability_chain::collect_chain_effects; +use crate::ability_chain::{collect_scoped_effects, AbilityScope}; use crate::features::commitment; /// Commitment at or above which the poison clock is a real plan for this deck @@ -92,8 +92,12 @@ pub fn detect(deck: &[DeckEntry]) -> PoisonFeature { total_nonland = total_nonland.saturating_add(entry.count); } + // Deck time asks "can this card ever advance the clock", so a poison + // mode of a modal card counts here even though the mode has not been + // chosen. The live policy asks the narrower question at the seam where + // the mode IS chosen — see `AbilityScope`. let is_source = is_poison_source_parts(&face.card_type.core_types, &face.keywords); - let is_direct = gives_opponents_poison_parts(&face.abilities); + let is_direct = gives_opponents_poison_parts(&face.abilities, AbilityScope::Potential); if is_source { source_count = source_count.saturating_add(entry.count); @@ -106,7 +110,7 @@ pub fn detect(deck: &[DeckEntry]) -> PoisonFeature { if is_source || is_direct { source_names.push(face.name.clone()); } - if proliferates_parts(&face.abilities) { + if proliferates_parts(&face.abilities, AbilityScope::Potential) { proliferate_count = proliferate_count.saturating_add(entry.count); } } @@ -171,15 +175,52 @@ pub(crate) fn is_poison_source_parts(core_types: &[CoreType], keywords: &[Keywor }) } -/// CR 122.1f: an ability chain that gives poison counters to a player who can -/// be an opponent. +/// CR 702.90b / CR 702.164c / CR 702.70a: how many poison counters this +/// creature's combat damage to a PLAYER would produce. +/// +/// * Infect (CR 702.90b) converts the damage itself, so it yields `power`; +/// CR 702.90f makes multiple instances redundant. +/// * Toxic (CR 702.164c) adds the creature's total toxic value, which +/// CR 702.164b defines as the SUM of every toxic ability's N. +/// * Poisonous N (CR 702.70a) is a separate triggered ability per instance. +/// +/// Parts-based so the arithmetic is testable without a `GameObject`; the live +/// caller passes `obj.card_types.core_types`, `obj.keywords`, and `obj.power`. +pub(crate) fn poison_yield_parts(core_types: &[CoreType], keywords: &[Keyword], power: i32) -> u32 { + if !is_poison_source_parts(core_types, keywords) { + return 0; + } + let mut has_infect = false; + let mut flat = 0u32; + for keyword in keywords { + match keyword { + Keyword::Infect => has_infect = true, + Keyword::Toxic(n) | Keyword::Poisonous(n) => flat = flat.saturating_add(*n), + _ => {} + } + } + // CR 702.90b: damage dealt to a player by an infect source becomes that + // many poison counters. Negative or absent power deals no damage. + let from_infect = if has_infect { + u32::try_from(power).unwrap_or(0) + } else { + 0 + }; + from_infect.saturating_add(flat) +} + +/// CR 122.1f: an ability tree that gives poison counters to a player who can +/// be an opponent, walked at `scope`. /// /// A `TargetFilter::Controller` / `SelfRef` scope is rejected — a card that /// poisons ITS OWN controller (a drawback clause, e.g. Phyrexian Vatmother) is /// not a poison payoff. -pub(crate) fn gives_opponents_poison_parts(abilities: &[AbilityDefinition]) -> bool { - abilities.iter().any(|ability| { - collect_chain_effects(ability).iter().any(|effect| { +pub(crate) fn gives_opponents_poison_parts<'a>( + abilities: impl IntoIterator, + scope: AbilityScope, +) -> bool { + abilities.into_iter().any(|ability| { + collect_scoped_effects(ability, scope).iter().any(|effect| { matches!( effect, Effect::GivePlayerCounter { @@ -195,6 +236,9 @@ pub(crate) fn gives_opponents_poison_parts(abilities: &[AbilityDefinition]) -> b /// True when a `TargetFilter` can resolve to a player other than the ability's /// controller. Mirrors `landfall::filter_matches_land_you_control`'s recursion, /// including the CR 109.3 conjunction rule for `And`. +/// +/// Player-scope arms follow `game::filter::player_matches_target_filter_with`, +/// the engine's authority for matching a player against a `TargetFilter`. fn filter_can_hit_opponent(filter: &TargetFilter) -> bool { match filter { TargetFilter::Opponent | TargetFilter::Player | TargetFilter::Any => true, @@ -202,14 +246,47 @@ fn filter_can_hit_opponent(filter: &TargetFilter) -> bool { TargetFilter::Or { filters } => filters.iter().any(filter_can_hit_opponent), // CR 109.3: every constraint of an `And` must hold for the match. TargetFilter::And { filters } => filters.iter().all(filter_can_hit_opponent), + // A negated scope hits an opponent exactly when some opponent is NOT + // matched by the inner filter — so `Not { SelfRef }` and + // `Not { Typed(controller: You) }` ("each other player") are poison + // payoffs, while `Not { Opponent }` resolves to you alone and is not. + TargetFilter::Not { filter } => !filter_matches_every_opponent(filter), + _ => false, + } +} + +/// True when EVERY opponent matches `filter`, the complement +/// `filter_can_hit_opponent` needs to decide a negated scope. Anything this +/// function cannot prove universal answers `false`, which makes the negation +/// fail open toward "can hit an opponent" only for scopes that genuinely leave +/// an opponent unmatched. +fn filter_matches_every_opponent(filter: &TargetFilter) -> bool { + match filter { + TargetFilter::Opponent | TargetFilter::Player | TargetFilter::Any => true, + TargetFilter::Typed(typed) => { + typed.type_filters.is_empty() + && matches!(typed.controller, Some(ControllerRef::Opponent)) + } + // Universality distributes the opposite way from `filter_can_hit_opponent`: + // an `Or` covers every opponent as soon as one branch does, an `And` + // only when all of them do (CR 109.3). + TargetFilter::Or { filters } => filters.iter().any(filter_matches_every_opponent), + TargetFilter::And { filters } => filters.iter().all(filter_matches_every_opponent), + // Double negation: `Not { X }` covers every opponent exactly when `X` + // covers none. + TargetFilter::Not { filter } => !filter_can_hit_opponent(filter), _ => false, } } -/// CR 701.34a: an ability chain containing either proliferate form. -pub(crate) fn proliferates_parts(abilities: &[AbilityDefinition]) -> bool { - abilities.iter().any(|ability| { - collect_chain_effects(ability).iter().any(|effect| { +/// CR 701.34a: an ability tree containing either proliferate form, walked at +/// `scope`. +pub(crate) fn proliferates_parts<'a>( + abilities: impl IntoIterator, + scope: AbilityScope, +) -> bool { + abilities.into_iter().any(|ability| { + collect_scoped_effects(ability, scope).iter().any(|effect| { matches!( effect, Effect::Proliferate | Effect::ProliferateTarget { .. } diff --git a/crates/phase-ai/src/features/tests/poison.rs b/crates/phase-ai/src/features/tests/poison.rs index 072da660b3..3bb541d7d9 100644 --- a/crates/phase-ai/src/features/tests/poison.rs +++ b/crates/phase-ai/src/features/tests/poison.rs @@ -8,8 +8,9 @@ use engine::types::card_type::CoreType; use engine::types::keywords::Keyword; use engine::types::player::PlayerCounterKind; +use crate::ability_chain::AbilityScope; use crate::features::poison::*; -use engine::types::ability::{AbilityKind, QuantityExpr}; +use engine::types::ability::{AbilityKind, ControllerRef, ModalChoice, QuantityExpr, TypedFilter}; use engine::types::card::CardFace; use engine::types::card_type::CardType; @@ -205,3 +206,247 @@ fn commitment_clamps_to_one() { let deck = vec![entry(infect_creature("All Infect"), 37)]; assert_eq!(detect(&deck).commitment, 1.0); } + +// ─── negated player scopes (CR 122.1f) ────────────────────────────────────── + +fn poison_effect(target: TargetFilter) -> Effect { + Effect::GivePlayerCounter { + counter_kind: PlayerCounterKind::Poison, + count: QuantityExpr::Fixed { value: 1 }, + target, + } +} + +fn not_filter(inner: TargetFilter) -> TargetFilter { + TargetFilter::Not { + filter: Box::new(inner), + } +} + +fn controlled_by(controller: ControllerRef) -> TargetFilter { + TargetFilter::Typed(TypedFilter { + controller: Some(controller), + ..TypedFilter::default() + }) +} + +/// A negated self-scope ("each player other than you") leaves every opponent +/// unmatched by the inner filter, so it IS opponent poison. +#[test] +fn negated_self_scope_is_opponent_poison() { + for inner in [ + TargetFilter::SelfRef, + TargetFilter::Controller, + controlled_by(ControllerRef::You), + ] { + let feature = detect(&[entry( + poison_spell("Negated Self", not_filter(inner.clone())), + 1, + )]); + assert_eq!( + feature.direct_count, 1, + "Not {{ {inner:?} }} must read as opponent poison" + ); + } +} + +/// The mirror image: negating a scope that already covers every opponent +/// resolves to the controller alone, which is a drawback, not a payoff. +#[test] +fn negated_opponent_scope_is_not_opponent_poison() { + for inner in [ + TargetFilter::Opponent, + TargetFilter::Player, + TargetFilter::Any, + controlled_by(ControllerRef::Opponent), + ] { + let feature = detect(&[entry( + poison_spell("Negated Opponent", not_filter(inner.clone())), + 1, + )]); + assert_eq!( + feature.direct_count, 0, + "Not {{ {inner:?} }} resolves to the controller and must not count" + ); + } +} + +/// Double negation collapses back to the inner scope. +#[test] +fn double_negation_round_trips() { + let poisons_opponents = detect(&[entry( + poison_spell( + "Twice Negated", + not_filter(not_filter(TargetFilter::Opponent)), + ), + 1, + )]); + assert_eq!(poisons_opponents.direct_count, 1); + + let poisons_self = detect(&[entry( + poison_spell( + "Twice Negated Self", + not_filter(not_filter(TargetFilter::Controller)), + ), + 1, + )]); + assert_eq!(poisons_self.direct_count, 0); +} + +// ─── modal / conditional branches (CR 700.2, CR 608.2c) ───────────────────── + +/// A modal ACTIVATED ability keeps its modes in `mode_abilities`, which the +/// unconditional chain walk does not visit. +fn modal_poison_ability() -> AbilityDefinition { + let mut ability = AbilityDefinition::new( + AbilityKind::Activated, + Effect::GenericEffect { + static_abilities: Vec::new(), + duration: None, + target: None, + }, + ); + ability.modal = Some(ModalChoice { + min_choices: 1, + max_choices: 1, + mode_count: 2, + ..ModalChoice::default() + }); + ability.mode_abilities = vec![ + AbilityDefinition::new( + AbilityKind::Activated, + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + ), + AbilityDefinition::new( + AbilityKind::Activated, + poison_effect(TargetFilter::Opponent), + ), + ]; + ability +} + +/// CR 700.2: deck time asks "can this card ever poison", so a poison-only mode +/// must register — the gap that `AbilityScope::Potential` closes. +#[test] +fn detects_poison_inside_a_modal_ability_mode() { + let mut face = creature("Modal Poisoner"); + face.abilities = vec![modal_poison_ability()]; + assert_eq!(detect(&[entry(face, 3)]).direct_count, 3); +} + +/// CR 601.2b: the same card is NOT unconditional poison — nothing is decided +/// until a mode is chosen, which is the scope the live policy classifies at. +#[test] +fn modal_poison_mode_is_not_unconditional() { + let abilities = vec![modal_poison_ability()]; + assert!(gives_opponents_poison_parts( + &abilities, + AbilityScope::Potential + )); + assert!(!gives_opponents_poison_parts( + &abilities, + AbilityScope::Unconditional + )); +} + +/// CR 608.2c: an "Otherwise, ..." branch is a reachable outcome, so deck-time +/// detection must see poison sitting in it. +#[test] +fn detects_poison_in_an_else_branch() { + let mut ability = AbilityDefinition::new( + AbilityKind::Spell, + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + ); + ability.else_ability = Some(Box::new(AbilityDefinition::new( + AbilityKind::Spell, + poison_effect(TargetFilter::Opponent), + ))); + + let mut face = spell("Else Poisoner"); + face.abilities = vec![ability]; + assert_eq!(detect(&[entry(face, 2)]).direct_count, 2); +} + +/// The proliferate axis walks the same scopes as the direct-poison axis. +#[test] +fn detects_proliferate_inside_a_modal_ability_mode() { + let mut ability = AbilityDefinition::new( + AbilityKind::Activated, + Effect::GenericEffect { + static_abilities: Vec::new(), + duration: None, + target: None, + }, + ); + ability.modal = Some(ModalChoice { + min_choices: 1, + max_choices: 1, + mode_count: 1, + ..ModalChoice::default() + }); + ability.mode_abilities = vec![AbilityDefinition::new( + AbilityKind::Activated, + Effect::Proliferate, + )]; + + let mut face = creature("Modal Proliferator"); + face.abilities = vec![ability]; + assert_eq!(detect(&[entry(face, 2)]).proliferate_count, 2); +} + +// ─── combat conversion (CR 702.90b / 702.164c / 702.70a) ──────────────────── + +const CREATURE: &[CoreType] = &[CoreType::Creature]; + +/// CR 702.90b: infect converts the damage itself, so the yield is power. +#[test] +fn infect_yields_its_power_in_poison() { + assert_eq!(poison_yield_parts(CREATURE, &[Keyword::Infect], 3), 3); + assert_eq!(poison_yield_parts(CREATURE, &[Keyword::Infect], 0), 0); + // A negative power deals no damage, so it converts nothing. + assert_eq!(poison_yield_parts(CREATURE, &[Keyword::Infect], -2), 0); +} + +/// CR 702.164b: total toxic value is the SUM of every toxic ability's N, and +/// CR 702.164c adds it on top of the damage's other results — so an ordinary +/// (non-infect) creature's power contributes nothing. +#[test] +fn toxic_sums_and_ignores_power() { + assert_eq!(poison_yield_parts(CREATURE, &[Keyword::Toxic(2)], 5), 2); + assert_eq!( + poison_yield_parts(CREATURE, &[Keyword::Toxic(2), Keyword::Toxic(1)], 5), + 3, + "CR 702.164b: total toxic value is the sum of all N" + ); +} + +/// CR 702.70a is a separate triggered ability, and CR 702.164c is "in addition +/// to the damage's other results" — an infect creature with toxic gets both. +#[test] +fn infect_and_toxic_stack() { + assert_eq!( + poison_yield_parts(CREATURE, &[Keyword::Infect, Keyword::Toxic(2)], 3), + 5 + ); + assert_eq!( + poison_yield_parts(CREATURE, &[Keyword::Poisonous(1), Keyword::Infect], 2), + 3 + ); +} + +#[test] +fn non_source_yields_no_poison() { + assert_eq!(poison_yield_parts(CREATURE, &[Keyword::Flying], 7), 0); + // CR 702.90/702.164/702.70 are all combat-damage abilities — a noncreature + // face carrying one converts nothing. + assert_eq!( + poison_yield_parts(&[CoreType::Artifact], &[Keyword::Infect], 7), + 0 + ); +} diff --git a/crates/phase-ai/src/policies/poison.rs b/crates/phase-ai/src/policies/poison.rs index f3a9f852f3..16c5d1533a 100644 --- a/crates/phase-ai/src/policies/poison.rs +++ b/crates/phase-ai/src/policies/poison.rs @@ -19,23 +19,50 @@ //! backwards would push the AI to durdle with proliferate before the clock has //! started. //! +//! ## Where each branch is scored +//! +//! The clock advances through three distinct decisions, and each is scored at +//! the seam where it is actually decided: +//! +//! | Decision | Seam | Rule | +//! |---|---|---| +//! | direct poison / proliferate | `CastSpell` · `ActivateAbility` | CR 122.1f · CR 701.34a | +//! | a modal card's poison mode | `SelectModes` | CR 601.2b | +//! | attacking with a poison source | `DeclareAttackers` | CR 702.90b · 702.164c · 702.70a | +//! +//! CR 601.2b makes mode selection a step of *announcing* a spell, so a +//! `CastSpell` candidate has no chosen mode yet and must not be credited with +//! a mode's poison. The unconditional-vs-modal split is expressed once, in +//! [`crate::ability_chain::AbilityScope`], and shared with deck-time detection. +//! //! ## Performance //! //! `verdict()` runs per candidate per search node. Every predicate here is //! card-local (`obj.abilities` / `obj.keywords`) or a plain `u32` field read on -//! `Player.poison_counters`. No board-wide sweep, no affordability call, no +//! `Player.poison_counters`; the combat branch is linear in the candidate's own +//! attack list. No board-wide sweep, no affordability call, no //! `find_legal_targets` — nothing this policy touches is on the documented //! inner-loop landmine list. +use engine::game::ability_utils::modal_spell_mode_ability_refs; +use engine::game::combat::AttackTarget; +use engine::types::ability::AbilityDefinition; use engine::types::actions::GameAction; -use engine::types::game_state::GameState; +use engine::types::game_state::{GameState, WaitingFor}; +use engine::types::identifiers::ObjectId; use engine::types::player::PlayerId; -use crate::features::poison::{LETHAL_POISON, POISON_CLOCK_FLOOR}; +use crate::ability_chain::AbilityScope; +use crate::features::poison::{ + gives_opponents_poison_parts, poison_yield_parts, proliferates_parts, LETHAL_POISON, + POISON_CLOCK_FLOOR, +}; use crate::features::DeckFeatures; use super::context::PolicyContext; -use super::registry::{DecisionKind, PolicyId, PolicyReason, PolicyVerdict, TacticalPolicy}; +use super::registry::{ + DecisionKind, PolicyId, PolicyReason, PolicyVerdict, TacticalPolicy, STRONG_MAX, +}; pub struct PoisonClockPolicy; @@ -48,6 +75,10 @@ enum PoisonContribution { /// Proliferates (CR 701.34a) — only advances the clock on an already /// poisoned opponent. Proliferate, + /// CR 508.1: an attack declaration whose poison sources are pointed at a + /// live opponent. `current` is that defender's poison total now, `added` + /// what the declared attackers convert if the damage connects. + CombatDamage { current: u32, added: u32 }, /// Nothing to do with the poison clock. None, } @@ -57,12 +88,16 @@ impl PoisonClockPolicy { /// deliberately not trusted here — the object on the battlefield may have /// been modified since the deck was analyzed. fn contribution(&self, ctx: &PolicyContext<'_>) -> PoisonContribution { - let abilities = match &ctx.candidate.action { - GameAction::CastSpell { object_id, .. } => ctx - .state - .objects - .get(object_id) - .map(|obj| obj.abilities.as_slice()), + match &ctx.candidate.action { + GameAction::CastSpell { object_id, .. } => match ctx.state.objects.get(object_id) { + // CR 601.2b + CR 700.2: a modal spell's poison lives in one of + // its printed modes, and mode selection is a later step of + // announcing the spell. Crediting the cast would score a branch + // the AI may never take — `SelectModes` is where it is decided. + Some(obj) if obj.modal.is_some() => PoisonContribution::None, + Some(obj) => classify_abilities(obj.abilities.iter()), + None => PoisonContribution::None, + }, GameAction::ActivateAbility { source_id, ability_index, @@ -71,42 +106,143 @@ impl PoisonClockPolicy { .objects .get(source_id) .and_then(|obj| obj.abilities.get(*ability_index)) - .map(std::slice::from_ref), - _ => None, + .map_or(PoisonContribution::None, |ability| { + classify_abilities(std::slice::from_ref(ability).iter()) + }), + // CR 601.2b: the mode IS chosen here — classify exactly the branch + // the candidate selects, so a poison mode and a non-poison mode of + // the same card score differently. + GameAction::SelectModes { indices } => { + let modes = pending_mode_abilities(ctx.state, &ctx.decision.waiting_for); + let selected: Vec<&AbilityDefinition> = indices + .iter() + .filter_map(|index| modes.get(*index).copied()) + .collect(); + classify_abilities(selected.iter().copied()) + } + GameAction::DeclareAttackers { attacks, .. } => combat_contribution(ctx, attacks), + _ => PoisonContribution::None, + } + } +} + +/// Classify an already-scoped ability set. Direct poison outranks proliferate +/// because it advances the clock without needing an existing counter. +/// +/// The walk is always [`AbilityScope::Unconditional`]: every caller has already +/// resolved which branch this candidate commits to, so a still-unchosen mode +/// must not leak in. +fn classify_abilities<'a, I>(abilities: I) -> PoisonContribution +where + I: IntoIterator + Clone, +{ + if gives_opponents_poison_parts(abilities.clone(), AbilityScope::Unconditional) { + PoisonContribution::DirectPoison + } else if proliferates_parts(abilities, AbilityScope::Unconditional) { + PoisonContribution::Proliferate + } else { + PoisonContribution::None + } +} + +/// CR 700.2: the modes a pending `SelectModes` decision is choosing among. +/// A modal SPELL carries them as the spell-kind abilities of the object being +/// cast (`modal_spell_mode_ability_refs`, the engine's authority, which +/// `handle_select_modes` indexes with the same `indices`); a modal activated or +/// triggered ABILITY carries them on the waiting payload. +fn pending_mode_abilities<'a>( + state: &'a GameState, + waiting_for: &'a WaitingFor, +) -> Vec<&'a AbilityDefinition> { + match waiting_for { + WaitingFor::ModeChoice { pending_cast, .. } => state + .objects + .get(&pending_cast.object_id) + .map(|obj| modal_spell_mode_ability_refs(obj).collect()) + .unwrap_or_default(), + WaitingFor::AbilityModeChoice { mode_abilities, .. } => mode_abilities.iter().collect(), + _ => Vec::new(), + } +} + +/// CR 508.1: score an attack declaration by the poison it converts. +/// +/// CR 702.90b / CR 702.164c / CR 702.70a all key on combat damage dealt **to a +/// player**, so an attack aimed at a planeswalker or a battle adds nothing on +/// this axis (CR 506.4c). Poison is summed per defending player — several +/// infect creatures attacking the same seat share one clock — and the seat +/// closest to CR 104.3d's ten is the one scored. +fn combat_contribution( + ctx: &PolicyContext<'_>, + attacks: &[(ObjectId, AttackTarget)], +) -> PoisonContribution { + // (defending seat, its poison now, poison this declaration would add). + let mut per_defender: Vec<(PlayerId, u32, u32)> = Vec::new(); + for (attacker_id, target) in attacks { + let AttackTarget::Player(defender) = target else { + continue; }; - let Some(abilities) = abilities else { - return PoisonContribution::None; + let Some(current) = live_opponent_poison(ctx.state, ctx.ai_player, *defender) else { + continue; }; - - // Cheapest discriminator first: direct poison outranks proliferate - // because it advances the clock without needing an existing counter. - if crate::features::poison::gives_opponents_poison_parts(abilities) { - PoisonContribution::DirectPoison - } else if crate::features::poison::proliferates_parts(abilities) { - PoisonContribution::Proliferate - } else { - PoisonContribution::None + let Some(attacker) = ctx.state.objects.get(attacker_id) else { + continue; + }; + let yielded = poison_yield_parts( + &attacker.card_types.core_types, + &attacker.keywords, + attacker.power.unwrap_or(0), + ); + if yielded == 0 { + continue; + } + match per_defender.iter_mut().find(|(seat, ..)| seat == defender) { + Some((_, _, added)) => *added = added.saturating_add(yielded), + None => per_defender.push((*defender, current, yielded)), } } + + per_defender + .into_iter() + .max_by_key(|(_, current, added)| current.saturating_add(*added)) + .map_or(PoisonContribution::None, |(_, current, added)| { + PoisonContribution::CombatDamage { current, added } + }) } -/// CR 104.3d: the highest poison total among the AI's opponents, and whether -/// any opponent is "poisoned" (CR 122.1f — one or more poison counters). +/// CR 104.3d: the highest poison total among the AI's LIVE opponents. +/// +/// CR 800.4: a multiplayer game continues after a player leaves, and the +/// eliminated seat stays in `GameState.players` — so a dead player's counters +/// must not be read as pressure the AI is still applying. pub(crate) fn most_poisoned_opponent(state: &GameState, ai_player: PlayerId) -> u32 { state .players .iter() - .enumerate() - .filter(|(index, _)| PlayerId(*index as u8) != ai_player) - .map(|(_, player)| player.poison_counters) + .filter(|player| player.id != ai_player && !player.is_eliminated) + .map(|player| player.poison_counters) .max() .unwrap_or(0) } -/// CR 104.3d: would one more poison counter put this player at ten or more, -/// losing them the game the next time a player would receive priority? -pub(crate) fn reaches_lethal(current_poison: u32) -> bool { - current_poison.saturating_add(1) >= LETHAL_POISON +/// CR 104.3d + CR 800.4: this seat's poison total, or `None` when it is not a +/// live opponent of `ai_player`. +pub(crate) fn live_opponent_poison( + state: &GameState, + ai_player: PlayerId, + seat: PlayerId, +) -> Option { + state + .players + .iter() + .find(|player| player.id == seat && player.id != ai_player && !player.is_eliminated) + .map(|player| player.poison_counters) +} + +/// CR 104.3d: would `added` more poison counters put this player at ten or +/// more, losing them the game the next time a player would receive priority? +pub(crate) fn reaches_lethal(current_poison: u32, added: u32) -> bool { + current_poison.saturating_add(added) >= LETHAL_POISON } impl TacticalPolicy for PoisonClockPolicy { @@ -115,7 +251,13 @@ impl TacticalPolicy for PoisonClockPolicy { } fn decision_kinds(&self) -> &'static [DecisionKind] { - &[DecisionKind::CastSpell, DecisionKind::ActivateAbility] + // `SelectModes` routes through `ActivateAbility` (decision_kind.rs maps + // both `ModeChoice` and `AbilityModeChoice` to that bucket). + &[ + DecisionKind::CastSpell, + DecisionKind::ActivateAbility, + DecisionKind::DeclareAttackers, + ] } fn activation( @@ -132,37 +274,64 @@ impl TacticalPolicy for PoisonClockPolicy { } fn verdict(&self, ctx: &PolicyContext<'_>) -> PolicyVerdict { - let contribution = self.contribution(ctx); - if contribution == PoisonContribution::None { - return PolicyVerdict::neutral(PolicyReason::new("poison_clock_na")); - } - - let highest = most_poisoned_opponent(ctx.state, ctx.ai_player); let scalar = ctx.config.policy_penalties.poison_clock_pressure; - - // CR 701.34a: proliferate needs an existing counter. With no poisoned - // opponent it advances nothing, so it earns nothing on this axis. - if contribution == PoisonContribution::Proliferate && highest == 0 { - return PolicyVerdict::neutral(PolicyReason::new( - "poison_clock_no_counters_to_proliferate", - )); + match self.contribution(ctx) { + PoisonContribution::None => { + PolicyVerdict::neutral(PolicyReason::new("poison_clock_na")) + } + // A single counter is the conservative floor: `GivePlayerCounter.count` + // is a `QuantityExpr` that need not be statically known. + PoisonContribution::DirectPoison => { + score_clock(scalar, most_poisoned_opponent(ctx.state, ctx.ai_player), 1) + } + PoisonContribution::Proliferate => { + let highest = most_poisoned_opponent(ctx.state, ctx.ai_player); + // CR 701.34a: proliferate chooses among permanents and players + // that ALREADY have a counter. With no poisoned opponent it + // advances nothing, so it earns nothing on this axis. + if highest == 0 { + return PolicyVerdict::neutral(PolicyReason::new( + "poison_clock_no_counters_to_proliferate", + )); + } + score_clock(scalar, highest, 1) + } + PoisonContribution::CombatDamage { current, added } => { + score_clock(scalar, current, added) + } } + } +} - // CR 104.3d: one more poison counter ends the game. - if reaches_lethal(highest) { - return PolicyVerdict::critical( - scalar, - PolicyReason::new("poison_clock_lethal") - .with_fact("opponent_poison", highest as i64), - ); - } +/// Floor on the progress multiplier so the first counters still read as a real +/// play rather than a rounding error — a clock at 1/10 is worth more than +/// one tenth of a clock at 10/10, because it is the only way to reach ten. +const MIN_CLOCK_PROGRESS: f64 = 0.25; - // Below lethal, value scales with how far the clock has already run — - // the last counters are worth more than the first. - let progress = f64::from(highest) / f64::from(LETHAL_POISON); - PolicyVerdict::preference( - scalar * progress.max(0.25), - PolicyReason::new("poison_clock_pressure").with_fact("opponent_poison", highest as i64), - ) +/// Shared scoring for every branch: game-ending when the action reaches CR +/// 104.3d's ten, otherwise scaled by how far the clock has run — the last +/// counters are worth more than the first. +/// +/// Both magnitudes are state- and config-dependent, so they route through +/// `PolicyVerdict::score`, which selects the band. The sub-lethal ceiling is +/// held at `STRONG_MAX` so only an action that actually reaches ten can land in +/// the critical band; an advancing-but-not-lethal clock must never outrank a +/// genuine win. +fn score_clock(scalar: f64, current: u32, added: u32) -> PolicyVerdict { + let facts = |reason: PolicyReason| { + reason + .with_fact("opponent_poison", i64::from(current)) + .with_fact("poison_added", i64::from(added)) + }; + + if reaches_lethal(current, added) { + return PolicyVerdict::score(scalar, facts(PolicyReason::new("poison_clock_lethal"))); } + + let projected = current.saturating_add(added); + let progress = f64::from(projected) / f64::from(LETHAL_POISON); + PolicyVerdict::score( + scalar.min(STRONG_MAX) * progress.max(MIN_CLOCK_PROGRESS), + facts(PolicyReason::new("poison_clock_pressure")), + ) } diff --git a/crates/phase-ai/src/policies/tests/poison.rs b/crates/phase-ai/src/policies/tests/poison.rs index 35c25957ee..5932049097 100644 --- a/crates/phase-ai/src/policies/tests/poison.rs +++ b/crates/phase-ai/src/policies/tests/poison.rs @@ -1,31 +1,198 @@ //! Unit tests for `policies::poison` — the CR 104.3d poison-clock policy. //! No `#[cfg(test)]` in SOURCE files; tests live here. +//! +//! The `verdict` path runs against a real `PolicyContext` built over a +//! multiplayer `GameState`, mirroring the `energy_payoff` policy-test shape. +use std::sync::Arc; + +use engine::ai_support::{ActionMetadata, AiDecisionContext, CandidateAction, TacticalClass}; +use engine::game::combat::AttackTarget; +use engine::game::zones::create_object; +use engine::types::ability::{ + AbilityDefinition, AbilityKind, Effect, ModalChoice, QuantityExpr, TargetFilter, +}; +use engine::types::actions::GameAction; +use engine::types::card_type::{CardType, CoreType}; +use engine::types::format::FormatConfig; +use engine::types::game_state::{CastPaymentMode, GameState, WaitingFor}; +use engine::types::identifiers::{CardId, ObjectId}; +use engine::types::keywords::Keyword; +use engine::types::player::{PlayerCounterKind, PlayerId}; +use engine::types::zones::Zone; + +use crate::config::AiConfig; +use crate::context::AiContext; use crate::features::poison::{LETHAL_POISON, POISON_CLOCK_FLOOR}; use crate::features::DeckFeatures; -use crate::policies::registry::TacticalPolicy; -use engine::types::game_state::GameState; -use engine::types::player::PlayerId; +use crate::policies::context::{PolicyContext, SearchDepth}; +use crate::policies::registry::{ + PolicyId, PolicyReason, PolicyRegistry, PolicyVerdict, TacticalPolicy, +}; +use crate::session::AiSession; use crate::policies::poison::*; +const AI: PlayerId = PlayerId(0); +const OPPONENT: PlayerId = PlayerId(1); + +// ─── fixtures ─────────────────────────────────────────────────────────────── + +fn state_with_players(count: u8) -> GameState { + GameState::new(FormatConfig::standard(), count, 42) +} + +fn config() -> AiConfig { + AiConfig::default() +} + +fn ai_context(config: &AiConfig) -> AiContext { + let mut context = AiContext::empty(&config.weights); + context.player = AI; + context +} + +fn priority_decision() -> AiDecisionContext { + AiDecisionContext { + waiting_for: WaitingFor::Priority { player: AI }, + candidates: Vec::new(), + } +} + +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 poison_effect(target: TargetFilter) -> Effect { + Effect::GivePlayerCounter { + counter_kind: PlayerCounterKind::Poison, + count: QuantityExpr::Fixed { value: 1 }, + target, + } +} + +fn draw_effect() -> Effect { + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + } +} + +/// A spell object in hand carrying `abilities`. +fn spell_object(state: &mut GameState, idx: u64, abilities: Vec) -> ObjectId { + let oid = create_object(state, CardId(idx), AI, format!("Spell {idx}"), Zone::Hand); + let object = state.objects.get_mut(&oid).unwrap(); + object.card_types = CardType { + supertypes: Vec::new(), + core_types: vec![CoreType::Instant], + subtypes: Vec::new(), + }; + *Arc::make_mut(&mut object.abilities) = abilities; + oid +} + +/// A battlefield creature with `keywords` and `power`. +fn creature_object( + state: &mut GameState, + idx: u64, + keywords: Vec, + power: i32, +) -> ObjectId { + let oid = create_object( + state, + CardId(idx), + AI, + format!("Creature {idx}"), + Zone::Battlefield, + ); + let object = state.objects.get_mut(&oid).unwrap(); + object.card_types = CardType { + supertypes: Vec::new(), + core_types: vec![CoreType::Creature], + subtypes: Vec::new(), + }; + object.keywords = keywords; + object.power = Some(power); + object.toughness = Some(power.max(1)); + oid +} + +fn cast_candidate(object_id: ObjectId) -> CandidateAction { + CandidateAction { + action: GameAction::CastSpell { + object_id, + card_id: CardId(object_id.0), + targets: Vec::new(), + payment_mode: CastPaymentMode::default(), + }, + metadata: ActionMetadata::for_actor(Some(AI), TacticalClass::Spell), + } +} + +fn attack_candidate(attacks: Vec<(ObjectId, AttackTarget)>) -> CandidateAction { + CandidateAction { + action: GameAction::DeclareAttackers { + attacks, + bands: Vec::new(), + }, + metadata: ActionMetadata::for_actor(Some(AI), TacticalClass::Attack), + } +} + +fn select_modes_candidate(indices: Vec) -> CandidateAction { + CandidateAction { + action: GameAction::SelectModes { indices }, + metadata: ActionMetadata::for_actor(Some(AI), TacticalClass::Selection), + } +} + +/// Unwrap a `Score` verdict into `(delta, reason)`; fail on `Reject`. +fn score_of(verdict: PolicyVerdict) -> (f64, PolicyReason) { + match verdict { + PolicyVerdict::Score { delta, reason } => (delta, reason), + PolicyVerdict::Reject { reason } => panic!("unexpected Reject: {reason:?}"), + } +} + +// ─── helper boundaries ────────────────────────────────────────────────────── + /// CR 104.3d: ten or more poison counters loses the game, so the ninth /// counter is the lethal setup and the eighth is not. #[test] fn reaches_lethal_matches_cr_104_3d_boundary() { assert_eq!(LETHAL_POISON, 10); - assert!(reaches_lethal(9), "9 + 1 == 10 is lethal"); - assert!(!reaches_lethal(8), "8 + 1 == 9 is not yet lethal"); - assert!(reaches_lethal(u32::MAX), "saturating add must not wrap"); + assert!(reaches_lethal(9, 1), "9 + 1 == 10 is lethal"); + assert!(!reaches_lethal(8, 1), "8 + 1 == 9 is not yet lethal"); + assert!(reaches_lethal(4, 6), "a multi-counter swing can reach ten"); + assert!( + !reaches_lethal(9, 0), + "nine with nothing added is not lethal" + ); + assert!(reaches_lethal(u32::MAX, 1), "saturating add must not wrap"); } #[test] fn activation_opts_out_below_floor() { let mut features = DeckFeatures::default(); features.poison.commitment = POISON_CLOCK_FLOOR - 0.01; - let state = GameState::default(); + let state = state_with_players(2); assert!(PoisonClockPolicy - .activation(&features, &state, PlayerId(0)) + .activation(&features, &state, AI) .is_none()); } @@ -33,22 +200,611 @@ fn activation_opts_out_below_floor() { fn activation_opts_in_above_floor() { let mut features = DeckFeatures::default(); features.poison.commitment = 0.9; - let state = GameState::default(); + let state = state_with_players(2); assert_eq!( - PoisonClockPolicy.activation(&features, &state, PlayerId(0)), + PoisonClockPolicy.activation(&features, &state, AI), Some(0.9) ); } #[test] fn most_poisoned_opponent_ignores_the_ai_itself() { - let mut state = GameState::default(); - while state.players.len() < 2 { - state.players.push(Default::default()); - } + let mut state = state_with_players(2); state.players[0].poison_counters = 7; state.players[1].poison_counters = 3; // The AI's own 7 poison must not be read as pressure it is applying. - assert_eq!(most_poisoned_opponent(&state, PlayerId(0)), 3); - assert_eq!(most_poisoned_opponent(&state, PlayerId(1)), 7); + assert_eq!(most_poisoned_opponent(&state, AI), 3); + assert_eq!(most_poisoned_opponent(&state, OPPONENT), 7); +} + +/// CR 800.4: a multiplayer game continues after a player leaves, and the +/// eliminated seat stays in +/// `GameState.players`. Their counters must not produce phantom pressure. +#[test] +fn most_poisoned_opponent_ignores_eliminated_seats() { + let mut state = state_with_players(4); + state.players[1].poison_counters = 9; + state.players[1].is_eliminated = true; + state.players[2].poison_counters = 2; + state.players[3].poison_counters = 0; + + assert_eq!( + most_poisoned_opponent(&state, AI), + 2, + "a dead seat at 9 poison must not read as one counter from lethal" + ); + assert_eq!(live_opponent_poison(&state, AI, PlayerId(1)), None); + assert_eq!(live_opponent_poison(&state, AI, PlayerId(2)), Some(2)); + assert_eq!( + live_opponent_poison(&state, AI, AI), + None, + "not an opponent" + ); +} + +// ─── verdict: direct poison and proliferate ───────────────────────────────── + +/// CR 104.3d: an opponent at nine is one counter from losing, so a direct +/// poison spell is a game-ending play, not a value play. +#[test] +fn verdict_scores_lethal_direct_poison_as_critical() { + let config = config(); + let context = ai_context(&config); + let mut state = state_with_players(2); + state.players[1].poison_counters = 9; + let spell = spell_object( + &mut state, + 1, + vec![AbilityDefinition::new( + AbilityKind::Spell, + poison_effect(TargetFilter::Opponent), + )], + ); + + let candidate = cast_candidate(spell); + let decision = priority_decision(); + let (delta, reason) = + score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))); + + assert_eq!(reason.kind, "poison_clock_lethal"); + assert_eq!(delta, config.policy_penalties.poison_clock_pressure); + assert!( + delta > crate::policies::registry::STRONG_MAX, + "a game-ending play must land in the critical band" + ); +} + +/// Below lethal the value scales with how far the clock has already run — the +/// last counters are worth more than the first. +/// +/// Swept across every reachable clock value rather than sampled, because the +/// delta is state-dependent: a magnitude that escapes its declared band trips +/// `score_in_band`'s debug assert and silently clamps in release, flattening +/// the very progress signal this policy exists to provide. The sub-lethal +/// ceiling must also stay under `STRONG_MAX`, so that only an action reaching +/// CR 104.3d's ten can outrank one that does. +#[test] +fn verdict_scales_direct_poison_by_clock_progress() { + let config = config(); + let context = ai_context(&config); + let decision = priority_decision(); + + let score_at = |existing: u32| { + let mut state = state_with_players(2); + state.players[1].poison_counters = existing; + let spell = spell_object( + &mut state, + 1, + vec![AbilityDefinition::new( + AbilityKind::Spell, + poison_effect(TargetFilter::Opponent), + )], + ); + let candidate = cast_candidate(spell); + score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))) + }; + + let mut previous = 0.0; + for existing in 0..LETHAL_POISON - 1 { + let (delta, reason) = score_at(existing); + assert_eq!(reason.kind, "poison_clock_pressure", "at {existing} poison"); + assert!( + delta > crate::policies::registry::NUDGE_MAX, + "advancing the clock is never a mere nudge (at {existing}): {delta}" + ); + assert!( + delta <= crate::policies::registry::STRONG_MAX, + "a non-lethal advance must stay out of the critical band (at {existing}): {delta}" + ); + assert!( + delta >= previous, + "score must not decrease as the clock advances (at {existing}): {delta} < {previous}" + ); + previous = delta; + } + + let (lethal_delta, lethal_reason) = score_at(LETHAL_POISON - 1); + assert_eq!(lethal_reason.kind, "poison_clock_lethal"); + assert!( + lethal_delta > previous, + "reaching ten must outrank every non-lethal advance: {lethal_delta} vs {previous}" + ); +} + +/// CR 701.34a: proliferate chooses among permanents and players that ALREADY +/// have a counter. With no poisoned opponent it advances nothing. +#[test] +fn verdict_declines_proliferate_with_no_poisoned_opponent() { + let config = config(); + let context = ai_context(&config); + let decision = priority_decision(); + let mut state = state_with_players(2); + let spell = spell_object( + &mut state, + 1, + vec![AbilityDefinition::new( + AbilityKind::Spell, + Effect::Proliferate, + )], + ); + + let candidate = cast_candidate(spell); + let (delta, reason) = + score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))); + assert_eq!(reason.kind, "poison_clock_no_counters_to_proliferate"); + assert_eq!(delta, 0.0); +} + +/// The same proliferate becomes real value once the clock has started. +#[test] +fn verdict_rewards_proliferate_on_a_poisoned_opponent() { + let config = config(); + let context = ai_context(&config); + let decision = priority_decision(); + let mut state = state_with_players(2); + state.players[1].poison_counters = 4; + let spell = spell_object( + &mut state, + 1, + vec![AbilityDefinition::new( + AbilityKind::Spell, + Effect::Proliferate, + )], + ); + + let candidate = cast_candidate(spell); + let (delta, reason) = + score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))); + assert_eq!(reason.kind, "poison_clock_pressure"); + assert!(delta > 0.0); +} + +#[test] +fn verdict_is_neutral_for_an_unrelated_spell() { + let config = config(); + let context = ai_context(&config); + let decision = priority_decision(); + let mut state = state_with_players(2); + state.players[1].poison_counters = 9; + let spell = spell_object( + &mut state, + 1, + vec![AbilityDefinition::new(AbilityKind::Spell, draw_effect())], + ); + + let candidate = cast_candidate(spell); + let (delta, reason) = + score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))); + assert_eq!(reason.kind, "poison_clock_na"); + assert_eq!(delta, 0.0); +} + +// ─── verdict: the modal seam (CR 601.2b / CR 700.2) ───────────────────────── + +/// Two-mode activated ability: mode 0 draws, mode 1 poisons. +fn modal_ability() -> AbilityDefinition { + let mut ability = AbilityDefinition::new( + AbilityKind::Activated, + Effect::GenericEffect { + static_abilities: Vec::new(), + duration: None, + target: None, + }, + ); + ability.modal = Some(ModalChoice { + min_choices: 1, + max_choices: 1, + mode_count: 2, + ..ModalChoice::default() + }); + ability.mode_abilities = vec![ + AbilityDefinition::new(AbilityKind::Activated, draw_effect()), + AbilityDefinition::new( + AbilityKind::Activated, + poison_effect(TargetFilter::Opponent), + ), + ]; + ability +} + +fn ability_mode_decision(source_id: ObjectId) -> AiDecisionContext { + let ability = modal_ability(); + AiDecisionContext { + waiting_for: WaitingFor::AbilityModeChoice { + player: AI, + modal: ability.modal.clone().unwrap(), + source_id, + mode_abilities: ability.mode_abilities.clone(), + is_activated: true, + ability_index: Some(0), + ability_cost: None, + unavailable_modes: Vec::new(), + }, + candidates: Vec::new(), + } +} + +/// CR 601.2b: the poison mode is scored at the seam where it is chosen. +#[test] +fn verdict_scores_the_selected_poison_mode() { + let config = config(); + let context = ai_context(&config); + let mut state = state_with_players(2); + state.players[1].poison_counters = 9; + let source = creature_object(&mut state, 1, Vec::new(), 1); + let decision = ability_mode_decision(source); + + let candidate = select_modes_candidate(vec![1]); + let (_, reason) = + score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))); + assert_eq!(reason.kind, "poison_clock_lethal"); +} + +/// The non-poison mode of the SAME ability must score nothing — the modes are +/// discriminated, not the card. +#[test] +fn verdict_ignores_a_selected_non_poison_mode() { + let config = config(); + let context = ai_context(&config); + let mut state = state_with_players(2); + state.players[1].poison_counters = 9; + let source = creature_object(&mut state, 1, Vec::new(), 1); + let decision = ability_mode_decision(source); + + let candidate = select_modes_candidate(vec![0]); + let (delta, reason) = + score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))); + assert_eq!(reason.kind, "poison_clock_na"); + assert_eq!(delta, 0.0); +} + +/// CR 601.2b: announcing a modal spell chooses no mode yet, so the cast must +/// not be credited with a mode's poison — that would score a branch the AI may +/// never take. +#[test] +fn verdict_does_not_credit_a_modal_cast_before_mode_selection() { + let config = config(); + let context = ai_context(&config); + let decision = priority_decision(); + let mut state = state_with_players(2); + state.players[1].poison_counters = 9; + + // A modal spell's printed modes are separate spell-kind abilities on the + // object (engine `modal_spell_mode_abilities`), one of which poisons. + let spell = spell_object( + &mut state, + 1, + vec![ + AbilityDefinition::new(AbilityKind::Spell, draw_effect()), + AbilityDefinition::new(AbilityKind::Spell, poison_effect(TargetFilter::Opponent)), + ], + ); + state.objects.get_mut(&spell).unwrap().modal = Some(ModalChoice { + min_choices: 1, + max_choices: 1, + mode_count: 2, + ..ModalChoice::default() + }); + + let candidate = cast_candidate(spell); + let (delta, reason) = + score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))); + assert_eq!(reason.kind, "poison_clock_na"); + assert_eq!(delta, 0.0); +} + +/// Control for the previous test: the identical ability list on a NON-modal +/// object is unconditional and does score. +#[test] +fn verdict_credits_a_non_modal_cast_with_the_same_effect() { + let config = config(); + let context = ai_context(&config); + let decision = priority_decision(); + let mut state = state_with_players(2); + state.players[1].poison_counters = 9; + let spell = spell_object( + &mut state, + 1, + vec![ + AbilityDefinition::new(AbilityKind::Spell, draw_effect()), + AbilityDefinition::new(AbilityKind::Spell, poison_effect(TargetFilter::Opponent)), + ], + ); + + let candidate = cast_candidate(spell); + let (_, reason) = + score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))); + assert_eq!(reason.kind, "poison_clock_lethal"); +} + +// ─── verdict: combat (CR 702.90b / 702.164c / 702.70a) ────────────────────── + +/// An infect deck's primary progression action is attacking with its clock. +#[test] +fn verdict_scores_a_poison_source_attacking_a_player() { + let config = config(); + let context = ai_context(&config); + let decision = priority_decision(); + let mut state = state_with_players(2); + state.players[1].poison_counters = 3; + let attacker = creature_object(&mut state, 1, vec![Keyword::Infect], 2); + + let candidate = attack_candidate(vec![(attacker, AttackTarget::Player(OPPONENT))]); + let (delta, reason) = + score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))); + assert_eq!(reason.kind, "poison_clock_pressure"); + assert!(delta > 0.0); + assert_eq!( + reason + .facts + .iter() + .find(|(key, _)| *key == "poison_added") + .map(|(_, value)| *value), + Some(2), + "CR 702.90b: a 2-power infect attacker converts two counters" + ); +} + +/// CR 104.3d: an 8-poison opponent facing a 2-power infect attacker is dead if +/// the damage connects. +#[test] +fn verdict_scores_a_lethal_infect_attack_as_critical() { + let config = config(); + let context = ai_context(&config); + let decision = priority_decision(); + let mut state = state_with_players(2); + state.players[1].poison_counters = 8; + let attacker = creature_object(&mut state, 1, vec![Keyword::Infect], 2); + + let candidate = attack_candidate(vec![(attacker, AttackTarget::Player(OPPONENT))]); + let (delta, reason) = + score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))); + assert_eq!(reason.kind, "poison_clock_lethal"); + assert_eq!(delta, config.policy_penalties.poison_clock_pressure); +} + +/// Poison from several attackers pointed at one seat shares that seat's clock. +#[test] +fn verdict_sums_poison_per_defending_player() { + let config = config(); + let context = ai_context(&config); + let decision = priority_decision(); + let mut state = state_with_players(2); + state.players[1].poison_counters = 6; + let first = creature_object(&mut state, 1, vec![Keyword::Infect], 2); + let second = creature_object(&mut state, 2, vec![Keyword::Toxic(2)], 1); + + let candidate = attack_candidate(vec![ + (first, AttackTarget::Player(OPPONENT)), + (second, AttackTarget::Player(OPPONENT)), + ]); + let (_, reason) = + score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))); + // 6 + (2 infect + 2 toxic) == 10. + assert_eq!(reason.kind, "poison_clock_lethal"); +} + +/// CR 702.90b / 702.164c / 702.70a all key on combat damage dealt to a PLAYER, +/// so an attack aimed at a planeswalker advances nothing on this axis. +#[test] +fn verdict_ignores_a_poison_source_attacking_a_planeswalker() { + let config = config(); + let context = ai_context(&config); + let decision = priority_decision(); + let mut state = state_with_players(2); + state.players[1].poison_counters = 9; + let attacker = creature_object(&mut state, 1, vec![Keyword::Infect], 2); + let walker = creature_object(&mut state, 2, Vec::new(), 0); + + let candidate = attack_candidate(vec![(attacker, AttackTarget::Planeswalker(walker))]); + let (delta, reason) = + score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))); + assert_eq!(reason.kind, "poison_clock_na"); + assert_eq!(delta, 0.0); +} + +/// Negative control: a vanilla beater is not a poison clock. +#[test] +fn verdict_ignores_an_attack_with_no_poison_source() { + let config = config(); + let context = ai_context(&config); + let decision = priority_decision(); + let mut state = state_with_players(2); + state.players[1].poison_counters = 9; + let attacker = creature_object(&mut state, 1, vec![Keyword::Flying], 5); + + let candidate = attack_candidate(vec![(attacker, AttackTarget::Player(OPPONENT))]); + let (delta, reason) = + score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))); + assert_eq!(reason.kind, "poison_clock_na"); + assert_eq!(delta, 0.0); +} + +/// CR 800.4: attacking a seat that has already left the game advances no clock. +#[test] +fn verdict_ignores_an_attack_on_an_eliminated_seat() { + let config = config(); + let context = ai_context(&config); + let decision = priority_decision(); + let mut state = state_with_players(4); + state.players[1].poison_counters = 9; + state.players[1].is_eliminated = true; + let attacker = creature_object(&mut state, 1, vec![Keyword::Infect], 2); + + let candidate = attack_candidate(vec![(attacker, AttackTarget::Player(PlayerId(1)))]); + let (delta, reason) = + score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))); + assert_eq!(reason.kind, "poison_clock_na"); + assert_eq!(delta, 0.0); +} + +/// With two live opponents, the seat the declaration pushes closest to ten is +/// the one scored. +#[test] +fn verdict_picks_the_defender_closest_to_lethal() { + let config = config(); + let context = ai_context(&config); + let decision = priority_decision(); + let mut state = state_with_players(3); + state.players[1].poison_counters = 1; + state.players[2].poison_counters = 9; + let first = creature_object(&mut state, 1, vec![Keyword::Infect], 2); + let second = creature_object(&mut state, 2, vec![Keyword::Infect], 1); + + let candidate = attack_candidate(vec![ + (first, AttackTarget::Player(PlayerId(1))), + (second, AttackTarget::Player(PlayerId(2))), + ]); + let (_, reason) = + score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))); + assert_eq!( + reason.kind, "poison_clock_lethal", + "the 9-poison seat is one infect counter from losing" + ); +} + +// ─── registry routing (the real pipeline) ─────────────────────────────────── + +/// An `AiContext` whose cached deck features clear `POISON_CLOCK_FLOOR`, so the +/// registry's activation gate lets the policy run. +fn committed_context(config: &AiConfig) -> AiContext { + let mut features = DeckFeatures::default(); + features.poison.commitment = 0.9; + let mut session = AiSession::empty(); + session.features.insert(AI, features); + let mut context = AiContext::empty(&config.weights); + context.session = Arc::new(session); + context.player = AI; + context +} + +/// The poison-clock verdict as the registry produces it, or `None` when the +/// policy did not run at all (wrong `DecisionKind`, or activation opted out). +fn routed_verdict(ctx: &PolicyContext<'_>) -> Option<(f64, PolicyReason)> { + PolicyRegistry::default() + .verdicts(ctx) + .into_iter() + .find(|(id, _)| *id == PolicyId::PoisonClock) + .map(|(_, verdict)| score_of(verdict)) +} + +#[test] +fn registry_registers_the_policy() { + assert!(PolicyRegistry::default().has_policy(PolicyId::PoisonClock)); +} + +/// End-to-end routing for the modal seam: `WaitingFor::AbilityModeChoice` +/// classifies to `DecisionKind::ActivateAbility`, the policy declares that +/// kind, and the two mode alternatives of the SAME ability come out +/// discriminated. +#[test] +fn registry_routes_modal_alternatives_to_the_policy() { + let config = config(); + let context = committed_context(&config); + let mut state = state_with_players(2); + state.players[1].poison_counters = 9; + let source = creature_object(&mut state, 1, Vec::new(), 1); + let decision = ability_mode_decision(source); + + let poison_mode = select_modes_candidate(vec![1]); + let (poison_delta, poison_reason) = + routed_verdict(&ctx(&state, &poison_mode, &decision, &context, &config)) + .expect("the poison mode must reach the policy through the registry"); + assert_eq!(poison_reason.kind, "poison_clock_lethal"); + assert!(poison_delta > 0.0); + + let draw_mode = select_modes_candidate(vec![0]); + let (draw_delta, draw_reason) = + routed_verdict(&ctx(&state, &draw_mode, &decision, &context, &config)) + .expect("the non-poison mode is still routed, it just scores nothing"); + assert_eq!(draw_reason.kind, "poison_clock_na"); + assert_eq!(draw_delta, 0.0); + assert!( + poison_delta > draw_delta, + "the poison mode must outrank its sibling: {poison_delta} vs {draw_delta}" + ); +} + +/// End-to-end routing for combat: `WaitingFor::DeclareAttackers` classifies to +/// `DecisionKind::DeclareAttackers`, which the policy now declares — an infect +/// deck's primary progression action reaches it. +#[test] +fn registry_routes_declare_attackers_to_the_policy() { + let config = config(); + let context = committed_context(&config); + let mut state = state_with_players(2); + state.players[1].poison_counters = 4; + let attacker = creature_object(&mut state, 1, vec![Keyword::Infect], 2); + let decision = AiDecisionContext { + waiting_for: WaitingFor::DeclareAttackers { + player: AI, + valid_attacker_ids: vec![attacker], + valid_attack_targets: vec![], + valid_attack_targets_by_attacker: None, + attacker_constraints: Default::default(), + }, + candidates: Vec::new(), + }; + + let attacking = attack_candidate(vec![(attacker, AttackTarget::Player(OPPONENT))]); + let (delta, reason) = routed_verdict(&ctx(&state, &attacking, &decision, &context, &config)) + .expect("a poison-source attack must reach the policy through the registry"); + assert_eq!(reason.kind, "poison_clock_pressure"); + assert!(delta > 0.0); + + let staying_home = attack_candidate(Vec::new()); + let (idle_delta, idle_reason) = + routed_verdict(&ctx(&state, &staying_home, &decision, &context, &config)) + .expect("declining to attack is still routed"); + assert_eq!(idle_reason.kind, "poison_clock_na"); + assert!( + delta > idle_delta, + "attacking with the clock must outrank holding it back" + ); +} + +/// The activation gate is what keeps this policy off every non-poison deck: +/// with commitment below the floor the registry never invokes it, even on the +/// exact board that would otherwise score critical. +#[test] +fn registry_skips_the_policy_for_an_uncommitted_deck() { + let config = config(); + let context = ai_context(&config); // no cached features → commitment 0.0 + let decision = priority_decision(); + let mut state = state_with_players(2); + state.players[1].poison_counters = 9; + let spell = spell_object( + &mut state, + 1, + vec![AbilityDefinition::new( + AbilityKind::Spell, + poison_effect(TargetFilter::Opponent), + )], + ); + + let candidate = cast_candidate(spell); + assert!( + routed_verdict(&ctx(&state, &candidate, &decision, &context, &config)).is_none(), + "below POISON_CLOCK_FLOOR the policy must not run at all" + ); } From c351235a57db0d3424013a0063da7b97bd67160a Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 23 Jul 2026 15:19:18 -0700 Subject: [PATCH 3/5] fix(phase-ai): cite CR 608.2c for the else_ability leg (maintainer fixup) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `AbilityScope::Potential`'s doc cited CR 603.4 for the `else_ability` branch. CR 603.4 is the intervening-"if" clause rule for triggered abilities and describes no "Otherwise, ..." leg. The repo convention for `else_ability` is uniformly CR 608.2c (resolve instructions in the order written; later text may modify earlier text) — and `push_scoped_effects` in this same file already cites CR 608.2c for that exact branch, so the two annotations contradicted each other about one construct. Also drops the CR 506.4c parenthetical from `combat_contribution`: CR 506.4c governs a planeswalker or battle being REMOVED from combat, not an attack aimed at one. The load-bearing justification — CR 702.90b / CR 702.164c / CR 702.70a each keying on combat damage dealt to a *player* — is already cited in the same doc block and stands alone. Comment-only; no behavior change. All CR numbers grep-verified against docs/MagicCompRules.txt. Co-authored-by: minion1227 --- crates/phase-ai/src/ability_chain.rs | 2 +- crates/phase-ai/src/policies/poison.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/phase-ai/src/ability_chain.rs b/crates/phase-ai/src/ability_chain.rs index 87a7a06229..2ecbe0e84e 100644 --- a/crates/phase-ai/src/ability_chain.rs +++ b/crates/phase-ai/src/ability_chain.rs @@ -41,7 +41,7 @@ pub(crate) enum AbilityScope { /// matter which branch or mode is taken. Unconditional, /// The full tree: `Unconditional` plus the `else_ability` branch (CR - /// 603.4) and every entry of `mode_abilities` (CR 700.2), recursively. + /// 608.2c) and every entry of `mode_abilities` (CR 700.2), recursively. Potential, } diff --git a/crates/phase-ai/src/policies/poison.rs b/crates/phase-ai/src/policies/poison.rs index 16c5d1533a..667536ad39 100644 --- a/crates/phase-ai/src/policies/poison.rs +++ b/crates/phase-ai/src/policies/poison.rs @@ -169,7 +169,7 @@ fn pending_mode_abilities<'a>( /// /// CR 702.90b / CR 702.164c / CR 702.70a all key on combat damage dealt **to a /// player**, so an attack aimed at a planeswalker or a battle adds nothing on -/// this axis (CR 506.4c). Poison is summed per defending player — several +/// this axis. Poison is summed per defending player — several /// infect creatures attacking the same seat share one clock — and the seat /// closest to CR 104.3d's ten is the one scored. fn combat_contribution( From 7d23e6c2099efb80ac135c5ed36bdc5fa1fbd20d Mon Sep 17 00:00:00 2001 From: minion1227 Date: Thu, 23 Jul 2026 15:38:28 -0700 Subject: [PATCH 4/5] test(phase-ai): registry-route every poison seam + tighten calibration (#6543) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses matthewevans' second review. The five original blockers were confirmed resolved in production; this round closes the HIGH test-gap and the recommended follow-ups, plus two small correctness fixes he flagged. HIGH — the policy's primary seam was declared but never registry-tested. Every direct-poison test called `verdict()` directly, so `DecisionKind::Cast\ Spell` could be deleted from `decision_kinds()` with the whole suite green. Add `registry_routes_cast_spell_to_the_policy` (poison cast scores through the pipeline, inert cast routes-but-scores-nil), and companion routed coverage: * `registry_routes_activate_ability_to_the_policy` — exercises the `obj.abilities.get(index)` lookup, the per-index discrimination, and the out-of-range → neutral fallback. * `registry_routes_modal_spell_mode_choice_to_the_policy` — drives a real `WaitingFor::ModeChoice` + `PendingCast`, the sole production consumer of the new `modal_spell_mode_ability_refs` engine API. Calibration and predicate coverage: * Or/And fixtures for BOTH filter predicates, each flipping one branch's outcome under a `.any`/`.all` swap, so the opposite-quantifier inversion between `filter_can_hit_opponent` and `filter_matches_every_opponent` can no longer pass silently. * `CoreType::Land` calibration fixture — 20 lands must not move commitment; pins the nonland-denominator guard whose removal would silently rescale. * Two-sided calibration bounds (Modern Infect 0.904 ± 0.01, superfriends 0.061 ± 0.005) replacing one-sided asserts that a saturating source weight could slip past. * `MIN_CLOCK_PROGRESS` pinned by a flat-plateau equality (0 and 1 poison score identically); deleting the floor splits them. Correctness (maintainer-flagged, non-blocking): * Combat lethal is capped at `STRONG_MAX`, strictly below the critical band a guaranteed direct poison reaches — CR 509.1a: a declared attack can be blocked or prevented, so a poison swing is a strong play, not a booked win. `score_clock` gains a per-branch `ceiling`. * `source_names` had no consumer — dropped the field, its population, and its two guarding tests rather than leave dead data. * CR nits: `608.2c` (not 603.4) for the `else_ability` branch at ability_chain.rs; dropped the off-point `506.4c` from combat_contribution. Deliberately deferred (per review): the multiplayer `SelectTarget` refinement (DirectPoison scores vs the most-poisoned seat but doesn't yet own target choice in 3+ player games) and the inherited `CR 109.3` citation, which the reviewer noted belongs on its own repo-wide commit. cargo test -p phase-ai --lib — 1420 passed (poison suite 50 → 58) cargo clippy -p phase-ai -p engine --all-targets --features proptest — clean Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/phase-ai/src/features/poison.rs | 18 +- crates/phase-ai/src/features/tests/poison.rs | 198 ++++++++++++-- crates/phase-ai/src/policies/poison.rs | 49 ++-- crates/phase-ai/src/policies/tests/poison.rs | 270 ++++++++++++++++++- 4 files changed, 468 insertions(+), 67 deletions(-) diff --git a/crates/phase-ai/src/features/poison.rs b/crates/phase-ai/src/features/poison.rs index 18703975f8..767468eb1c 100644 --- a/crates/phase-ai/src/features/poison.rs +++ b/crates/phase-ai/src/features/poison.rs @@ -68,10 +68,6 @@ pub struct PoisonFeature { /// `0.0..=1.0` — how central the poison clock is to this deck. Consumed by /// `PoisonClockPolicy::activation` as the single scaling knob. pub commitment: f32, - /// Names of detected poison sources. NOT used for classification — that - /// already happened against the AST. Used as battlefield identifiers at - /// decision time (identity lookup, exempt from the name-matching lint). - pub source_names: Vec, } /// Structural detection over each `DeckEntry`'s `CardFace` AST. @@ -84,7 +80,6 @@ pub fn detect(deck: &[DeckEntry]) -> PoisonFeature { let mut direct_count = 0u32; let mut proliferate_count = 0u32; let mut total_nonland = 0u32; - let mut source_names: Vec = Vec::new(); for entry in deck { let face = &entry.card; @@ -96,20 +91,12 @@ pub fn detect(deck: &[DeckEntry]) -> PoisonFeature { // mode of a modal card counts here even though the mode has not been // chosen. The live policy asks the narrower question at the seam where // the mode IS chosen — see `AbilityScope`. - let is_source = is_poison_source_parts(&face.card_type.core_types, &face.keywords); - let is_direct = gives_opponents_poison_parts(&face.abilities, AbilityScope::Potential); - - if is_source { + if is_poison_source_parts(&face.card_type.core_types, &face.keywords) { source_count = source_count.saturating_add(entry.count); } - if is_direct { + if gives_opponents_poison_parts(&face.abilities, AbilityScope::Potential) { direct_count = direct_count.saturating_add(entry.count); } - // One push per UNIQUE face, and once even when both axes fire — - // guards the per-copy and double-push traps. - if is_source || is_direct { - source_names.push(face.name.clone()); - } if proliferates_parts(&face.abilities, AbilityScope::Potential) { proliferate_count = proliferate_count.saturating_add(entry.count); } @@ -123,7 +110,6 @@ pub fn detect(deck: &[DeckEntry]) -> PoisonFeature { direct_count, proliferate_count, commitment, - source_names, } } diff --git a/crates/phase-ai/src/features/tests/poison.rs b/crates/phase-ai/src/features/tests/poison.rs index 3bb541d7d9..181964a440 100644 --- a/crates/phase-ai/src/features/tests/poison.rs +++ b/crates/phase-ai/src/features/tests/poison.rs @@ -38,6 +38,18 @@ fn spell(name: &str) -> CardFace { } } +fn land(name: &str) -> CardFace { + CardFace { + name: name.to_string(), + card_type: CardType { + supertypes: Vec::new(), + core_types: vec![CoreType::Land], + subtypes: Vec::new(), + }, + ..Default::default() + } +} + fn entry(card: CardFace, count: u32) -> DeckEntry { DeckEntry { card, count } } @@ -75,8 +87,9 @@ fn proliferate_spell(name: &str) -> CardFace { fn empty_deck_produces_defaults() { let feature = detect(&[]); assert_eq!(feature.source_count, 0); + assert_eq!(feature.direct_count, 0); + assert_eq!(feature.proliferate_count, 0); assert_eq!(feature.commitment, 0.0); - assert!(feature.source_names.is_empty()); } #[test] @@ -135,34 +148,20 @@ fn self_poison_drawback_not_counted() { assert_eq!(feature.direct_count, 0); } -/// One push per UNIQUE face, not per playset copy. +/// Counts scale per playset copy, not per unique face. #[test] -fn source_names_dedup_per_face() { +fn counts_scale_by_copy_count() { let feature = detect(&[entry(infect_creature("Glistener Elf"), 4)]); assert_eq!(feature.source_count, 4); - assert_eq!(feature.source_names, vec!["Glistener Elf".to_string()]); -} - -/// A face that is BOTH a source and a direct-poison card pushes its name once. -#[test] -fn source_and_direct_face_pushes_name_once() { - let mut face = infect_creature("Hybrid Threat"); - face.abilities = vec![AbilityDefinition::new( - AbilityKind::Spell, - Effect::GivePlayerCounter { - counter_kind: PlayerCounterKind::Poison, - count: QuantityExpr::Fixed { value: 1 }, - target: TargetFilter::Opponent, - }, - )]; - let feature = detect(&[entry(face, 2)]); - assert_eq!(feature.source_names.len(), 1); } -/// Calibration anchor: Modern Infect — 12 infect creatures + 2 proliferate -/// over 37 nonland cards → strongly committed. +/// Calibration anchor: Modern Infect — 12 infect creatures + 2 proliferate over +/// 37 nonland cards → commitment ≈ 0.904. Two-sided: the lower bound is the +/// policy calibration; the upper bound pins the source weight, since +/// `weighted_sum` clamps at 1.0 and a one-sided `> 0.85` would still pass if the +/// source contribution were inflated toward saturation. #[test] -fn modern_infect_hits_calibration_floor() { +fn modern_infect_calibration_is_pinned() { let deck = vec![ entry(infect_creature("Glistener Elf"), 4), entry(infect_creature("Blighted Agent"), 4), @@ -173,26 +172,55 @@ fn modern_infect_hits_calibration_floor() { let feature = detect(&deck); assert_eq!(feature.source_count, 12); assert!( - feature.commitment > 0.85, - "Modern Infect must clear 0.85, got {}", + (0.894..=0.914).contains(&feature.commitment), + "Modern Infect must land at 0.904 ± 0.01, got {}", feature.commitment ); } /// Anti-calibration: a superfriends deck running proliferate but no poison -/// source must stay far below the policy floor. +/// source stays at ≈ 0.061 — far below the floor. Two-sided so the assertion +/// pins the proliferate weight rather than leaving 5.7× of headroom to the +/// floor. #[test] -fn superfriends_proliferate_stays_below_floor() { +fn superfriends_proliferate_is_pinned_below_floor() { let deck = vec![ entry(proliferate_spell("Contagion Clasp"), 2), entry(spell("Planeswalker Filler"), 35), ]; let feature = detect(&deck); assert!( - feature.commitment < POISON_CLOCK_FLOOR, - "proliferate alone is not a poison plan, got {}", + (0.056..=0.066).contains(&feature.commitment), + "proliferate-only must land at 0.061 ± 0.005, got {}", feature.commitment ); + assert!(feature.commitment < POISON_CLOCK_FLOOR); +} + +/// CR 104.3d: lands are excluded from the commitment denominator (they are not +/// part of the poison plan's nonland payload). Two decks with identical nonland +/// content score identically no matter how many lands pad the manabase — pinning +/// the `CoreType::Land` guard in `detect`, whose removal would inflate the +/// denominator and silently lower every commitment. +#[test] +fn lands_are_excluded_from_commitment_denominator() { + let core = |extra: Vec| { + let mut deck = vec![ + entry(infect_creature("Glistener Elf"), 4), + entry(infect_creature("Blighted Agent"), 4), + entry(infect_creature("Phyrexian Crusader"), 4), + entry(spell("Pump Spell"), 25), + ]; + deck.extend(extra); + detect(&deck).commitment + }; + let landless = core(Vec::new()); + let with_lands = core(vec![entry(land("Inkmoth Nexus"), 20)]); + assert_eq!( + landless, with_lands, + "20 lands must not change commitment; the nonland payload is identical" + ); + assert!(landless > 0.0); } #[test] @@ -293,6 +321,118 @@ fn double_negation_round_trips() { assert_eq!(poisons_self.direct_count, 0); } +// ─── Or / And player scopes ───────────────────────────────────────────────── +// +// `Or`/`And` distribute their quantifier the OPPOSITE way between the two +// predicates: `filter_can_hit_opponent` is existential over `Or` / universal +// over `And` (some/every constraint must admit an opponent), while +// `filter_matches_every_opponent` — reached only through a negated scope — is +// universal over `Or` / existential over `And`. Each fixture below flips +// exactly one branch's outcome if its `.any`/`.all` is swapped, so a mistaken +// inversion in either predicate fails a test rather than passing silently. + +fn or_filter(filters: Vec) -> TargetFilter { + TargetFilter::Or { filters } +} + +fn and_filter(filters: Vec) -> TargetFilter { + TargetFilter::And { filters } +} + +/// `filter_can_hit_opponent` over `Or` is existential: one opponent-admitting +/// branch is enough, and no branch admitting one means it cannot hit. +#[test] +fn or_scope_hits_opponent_iff_some_branch_does() { + // Opponent branch admits an opponent → hits (swapping `.any`→`.all` here + // would drop this, since the Controller branch does not). + let hits = detect(&[entry( + poison_spell( + "Or Hits", + or_filter(vec![TargetFilter::Controller, TargetFilter::Opponent]), + ), + 1, + )]); + assert_eq!(hits.direct_count, 1); + + // No branch admits an opponent → cannot hit. + let misses = detect(&[entry( + poison_spell( + "Or Misses", + or_filter(vec![TargetFilter::Controller, TargetFilter::SelfRef]), + ), + 1, + )]); + assert_eq!(misses.direct_count, 0); +} + +/// `filter_can_hit_opponent` over `And` is universal (CR 109.3): every +/// constraint must admit an opponent, so one self-only constraint blocks it. +#[test] +fn and_scope_hits_opponent_iff_every_branch_does() { + // Both constraints admit an opponent → hits. + let hits = detect(&[entry( + poison_spell( + "And Hits", + and_filter(vec![TargetFilter::Player, TargetFilter::Opponent]), + ), + 1, + )]); + assert_eq!(hits.direct_count, 1); + + // Controller constraint cannot be an opponent → the conjunction cannot + // (swapping `.all`→`.any` here would wrongly count it). + let misses = detect(&[entry( + poison_spell( + "And Misses", + and_filter(vec![TargetFilter::Opponent, TargetFilter::Controller]), + ), + 1, + )]); + assert_eq!(misses.direct_count, 0); +} + +/// `filter_matches_every_opponent` (reached via `Not`) over `Or` is universal: +/// `Not { Or }` hits an opponent only if some `Or` branch leaves one unmatched. +#[test] +fn negated_or_scope_pins_every_opponent_quantifier() { + // `Or { Controller, Opponent }` covers every opponent (the Opponent branch + // does), so its negation resolves to you alone → does not count. Swapping + // the `Or` arm of `filter_matches_every_opponent` to `.all` would make the + // Controller branch drag universality to false → wrongly count. + let feature = detect(&[entry( + poison_spell( + "Not Or", + not_filter(or_filter(vec![ + TargetFilter::Controller, + TargetFilter::Opponent, + ])), + ), + 1, + )]); + assert_eq!(feature.direct_count, 0); +} + +/// `filter_matches_every_opponent` over `And` is existential: `Not { And }` +/// hits an opponent unless every conjunct covers all opponents. +#[test] +fn negated_and_scope_pins_every_opponent_quantifier() { + // `And { Opponent, Controller }` does NOT cover every opponent (the + // Controller conjunct is not universal), so its negation leaves an opponent + // matched → counts. Swapping the `And` arm to `.any` would make the + // Opponent conjunct alone assert universality → wrongly drop it. + let feature = detect(&[entry( + poison_spell( + "Not And", + not_filter(and_filter(vec![ + TargetFilter::Opponent, + TargetFilter::Controller, + ])), + ), + 1, + )]); + assert_eq!(feature.direct_count, 1); +} + // ─── modal / conditional branches (CR 700.2, CR 608.2c) ───────────────────── /// A modal ACTIVATED ability keeps its modes in `mode_abilities`, which the diff --git a/crates/phase-ai/src/policies/poison.rs b/crates/phase-ai/src/policies/poison.rs index 667536ad39..16ef3a1a51 100644 --- a/crates/phase-ai/src/policies/poison.rs +++ b/crates/phase-ai/src/policies/poison.rs @@ -61,7 +61,7 @@ use crate::features::DeckFeatures; use super::context::PolicyContext; use super::registry::{ - DecisionKind, PolicyId, PolicyReason, PolicyVerdict, TacticalPolicy, STRONG_MAX, + DecisionKind, PolicyId, PolicyReason, PolicyVerdict, TacticalPolicy, CRITICAL_MAX, STRONG_MAX, }; pub struct PoisonClockPolicy; @@ -280,10 +280,15 @@ impl TacticalPolicy for PoisonClockPolicy { PolicyVerdict::neutral(PolicyReason::new("poison_clock_na")) } // A single counter is the conservative floor: `GivePlayerCounter.count` - // is a `QuantityExpr` that need not be statically known. - PoisonContribution::DirectPoison => { - score_clock(scalar, most_poisoned_opponent(ctx.state, ctx.ai_player), 1) - } + // is a `QuantityExpr` that need not be statically known. A resolving + // spell's counter is guaranteed, so its lethal case may reach the + // critical band (`CRITICAL_MAX` ceiling). + PoisonContribution::DirectPoison => score_clock( + scalar, + most_poisoned_opponent(ctx.state, ctx.ai_player), + 1, + CRITICAL_MAX, + ), PoisonContribution::Proliferate => { let highest = most_poisoned_opponent(ctx.state, ctx.ai_player); // CR 701.34a: proliferate chooses among permanents and players @@ -294,10 +299,15 @@ impl TacticalPolicy for PoisonClockPolicy { "poison_clock_no_counters_to_proliferate", )); } - score_clock(scalar, highest, 1) + score_clock(scalar, highest, 1, CRITICAL_MAX) } + // CR 509.1a: declared combat damage is not guaranteed — the attack + // can be blocked or prevented — so even a would-be-lethal poison + // swing is held below the critical band (`STRONG_MAX` ceiling) that + // a guaranteed direct poison earns. A committed attacker is still a + // strong play, just not a booked win. PoisonContribution::CombatDamage { current, added } => { - score_clock(scalar, current, added) + score_clock(scalar, current, added, STRONG_MAX) } } } @@ -308,16 +318,18 @@ impl TacticalPolicy for PoisonClockPolicy { /// one tenth of a clock at 10/10, because it is the only way to reach ten. const MIN_CLOCK_PROGRESS: f64 = 0.25; -/// Shared scoring for every branch: game-ending when the action reaches CR -/// 104.3d's ten, otherwise scaled by how far the clock has run — the last -/// counters are worth more than the first. +/// Shared scoring for every branch: reaching CR 104.3d's ten is the top of the +/// scale, otherwise scaled by how far the clock has run — the last counters are +/// worth more than the first. /// -/// Both magnitudes are state- and config-dependent, so they route through -/// `PolicyVerdict::score`, which selects the band. The sub-lethal ceiling is -/// held at `STRONG_MAX` so only an action that actually reaches ten can land in -/// the critical band; an advancing-but-not-lethal clock must never outrank a -/// genuine win. -fn score_clock(scalar: f64, current: u32, added: u32) -> PolicyVerdict { +/// `ceiling` is the highest band this branch may reach: `CRITICAL_MAX` for a +/// guaranteed counter (a resolving direct-poison spell), `STRONG_MAX` for a +/// merely-declared one (a combat swing that can still be blocked). Both +/// magnitudes are state- and config-dependent, so they route through +/// `PolicyVerdict::score`, which selects the band from the clamped value. The +/// sub-lethal case is always held under `STRONG_MAX`, so an advancing-but-not- +/// lethal clock never outranks a booked win. +fn score_clock(scalar: f64, current: u32, added: u32, ceiling: f64) -> PolicyVerdict { let facts = |reason: PolicyReason| { reason .with_fact("opponent_poison", i64::from(current)) @@ -325,7 +337,10 @@ fn score_clock(scalar: f64, current: u32, added: u32) -> PolicyVerdict { }; if reaches_lethal(current, added) { - return PolicyVerdict::score(scalar, facts(PolicyReason::new("poison_clock_lethal"))); + return PolicyVerdict::score( + scalar.min(ceiling), + facts(PolicyReason::new("poison_clock_lethal")), + ); } let projected = current.saturating_add(added); diff --git a/crates/phase-ai/src/policies/tests/poison.rs b/crates/phase-ai/src/policies/tests/poison.rs index 5932049097..bed28ebccc 100644 --- a/crates/phase-ai/src/policies/tests/poison.rs +++ b/crates/phase-ai/src/policies/tests/poison.rs @@ -9,15 +9,17 @@ use std::sync::Arc; use engine::ai_support::{ActionMetadata, AiDecisionContext, CandidateAction, TacticalClass}; use engine::game::combat::AttackTarget; use engine::game::zones::create_object; +use engine::types::ability::ResolvedAbility; use engine::types::ability::{ AbilityDefinition, AbilityKind, Effect, ModalChoice, QuantityExpr, TargetFilter, }; use engine::types::actions::GameAction; use engine::types::card_type::{CardType, CoreType}; use engine::types::format::FormatConfig; -use engine::types::game_state::{CastPaymentMode, GameState, WaitingFor}; +use engine::types::game_state::{CastPaymentMode, GameState, PendingCast, WaitingFor}; use engine::types::identifiers::{CardId, ObjectId}; use engine::types::keywords::Keyword; +use engine::types::mana::ManaCost; use engine::types::player::{PlayerCounterKind, PlayerId}; use engine::types::zones::Zone; @@ -161,6 +163,16 @@ fn select_modes_candidate(indices: Vec) -> CandidateAction { } } +fn activate_ability_candidate(source_id: ObjectId, ability_index: usize) -> CandidateAction { + CandidateAction { + action: GameAction::ActivateAbility { + source_id, + ability_index, + }, + metadata: ActionMetadata::for_actor(Some(AI), TacticalClass::Ability), + } +} + /// Unwrap a `Score` verdict into `(delta, reason)`; fail on `Reject`. fn score_of(verdict: PolicyVerdict) -> (f64, PolicyReason) { match verdict { @@ -331,6 +343,50 @@ fn verdict_scales_direct_poison_by_clock_progress() { ); } +/// `MIN_CLOCK_PROGRESS` floors the earliest counters onto a flat plateau: at 0 +/// and 1 existing poison the projected progress (0.1, 0.2) is below the floor, +/// so both score the SAME delta. Deleting the floor would split them (0.50 vs +/// 1.00), so the equality — not just a magnitude — is what pins the constant. +#[test] +fn verdict_floors_early_clock_progress_onto_a_plateau() { + let config = config(); + let context = ai_context(&config); + let decision = priority_decision(); + + let score_at = |existing: u32| { + let mut state = state_with_players(2); + state.players[1].poison_counters = existing; + let spell = spell_object( + &mut state, + 1, + vec![AbilityDefinition::new( + AbilityKind::Spell, + poison_effect(TargetFilter::Opponent), + )], + ); + let candidate = cast_candidate(spell); + score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))) + .0 + }; + + let at_zero = score_at(0); + let at_one = score_at(1); + assert_eq!( + at_zero, at_one, + "0 and 1 existing poison both sit on the MIN_CLOCK_PROGRESS floor" + ); + // STRONG_MAX (the sub-lethal ceiling) × the 0.25 floor. + assert!( + (at_zero - crate::policies::registry::STRONG_MAX * 0.25).abs() < 1e-9, + "plateau delta must be STRONG_MAX × 0.25, got {at_zero}" + ); + // And the plateau ends: 2 existing poison (progress 0.3) clears the floor. + assert!( + score_at(2) > at_one, + "the third counter must clear the floor and score higher" + ); +} + /// CR 701.34a: proliferate chooses among permanents and players that ALREADY /// have a counter. With no poisoned opponent it advances nothing. #[test] @@ -565,10 +621,12 @@ fn verdict_scores_a_poison_source_attacking_a_player() { ); } -/// CR 104.3d: an 8-poison opponent facing a 2-power infect attacker is dead if -/// the damage connects. +/// CR 509.1a: an 8-poison opponent facing a 2-power infect attacker would be +/// dead if the damage connects — but a declared attack can still be blocked or +/// prevented, so the lethal combat swing is held at the `STRONG_MAX` ceiling, +/// strictly below the critical band a guaranteed direct poison reaches. #[test] -fn verdict_scores_a_lethal_infect_attack_as_critical() { +fn verdict_caps_a_lethal_infect_attack_below_critical() { let config = config(); let context = ai_context(&config); let decision = priority_decision(); @@ -580,7 +638,36 @@ fn verdict_scores_a_lethal_infect_attack_as_critical() { let (delta, reason) = score_of(PoisonClockPolicy.verdict(&ctx(&state, &candidate, &decision, &context, &config))); assert_eq!(reason.kind, "poison_clock_lethal"); - assert_eq!(delta, config.policy_penalties.poison_clock_pressure); + assert_eq!( + delta, + crate::policies::registry::STRONG_MAX, + "combat lethal is capped at STRONG_MAX (the swing can be blocked)" + ); + assert!( + delta < config.policy_penalties.poison_clock_pressure, + "and stays strictly below the guaranteed-direct-poison magnitude" + ); + + // Control: a resolving direct-poison spell that ALSO reaches ten (from 9, + // +1) is a guaranteed counter, so it may enter the critical band that the + // blockable combat swing cannot. + state.players[1].poison_counters = 9; + let spell = spell_object( + &mut state, + 9, + vec![AbilityDefinition::new( + AbilityKind::Spell, + poison_effect(TargetFilter::Opponent), + )], + ); + let cast = cast_candidate(spell); + let (cast_delta, cast_reason) = + score_of(PoisonClockPolicy.verdict(&ctx(&state, &cast, &decision, &context, &config))); + assert_eq!(cast_reason.kind, "poison_clock_lethal"); + assert!( + cast_delta > crate::policies::registry::STRONG_MAX, + "a guaranteed lethal counter reaches critical while a blockable combat swing does not" + ); } /// Poison from several attackers pointed at one seat shares that seat's clock. @@ -808,3 +895,176 @@ fn registry_skips_the_policy_for_an_uncommitted_deck() { "below POISON_CLOCK_FLOOR the policy must not run at all" ); } + +/// End-to-end routing for the policy's PRIMARY seam: a direct-poison +/// `CastSpell` under `WaitingFor::Priority` classifies to +/// `DecisionKind::CastSpell`, which the policy declares. Without this the +/// `CastSpell` entry in `decision_kinds()` could be deleted with the whole +/// suite still green — the seam would be dead in production while every +/// direct-poison test still passed by calling `verdict()` directly. +#[test] +fn registry_routes_cast_spell_to_the_policy() { + let config = config(); + let context = committed_context(&config); + let mut state = state_with_players(2); + state.players[1].poison_counters = 9; + let spell = spell_object( + &mut state, + 1, + vec![AbilityDefinition::new( + AbilityKind::Spell, + poison_effect(TargetFilter::Opponent), + )], + ); + let decision = priority_decision(); + + let candidate = cast_candidate(spell); + let (delta, reason) = routed_verdict(&ctx(&state, &candidate, &decision, &context, &config)) + .expect("a direct-poison cast must reach the policy through the registry"); + assert_eq!(reason.kind, "poison_clock_lethal"); + assert!(delta > 0.0); + + // A non-poison cast is still routed under the same kind; it just scores nil. + let inert = spell_object( + &mut state, + 2, + vec![AbilityDefinition::new(AbilityKind::Spell, draw_effect())], + ); + let (inert_delta, inert_reason) = routed_verdict(&ctx( + &state, + &cast_candidate(inert), + &decision, + &context, + &config, + )) + .expect("a non-poison cast is still routed under DecisionKind::CastSpell"); + assert_eq!(inert_reason.kind, "poison_clock_na"); + assert!( + delta > inert_delta, + "the poison cast must outrank an inert one through the pipeline" + ); +} + +/// End-to-end routing for an activated poison ability: an +/// `ActivateAbility` action under `WaitingFor::Priority` classifies to +/// `DecisionKind::ActivateAbility`, exercising the `obj.abilities.get(index)` +/// lookup that `verdict()`-direct tests bypass. +#[test] +fn registry_routes_activate_ability_to_the_policy() { + let config = config(); + let context = committed_context(&config); + let mut state = state_with_players(2); + state.players[1].poison_counters = 9; + // A permanent with two activated abilities: index 0 draws, index 1 poisons. + let source = creature_object(&mut state, 1, Vec::new(), 1); + *Arc::make_mut(&mut state.objects.get_mut(&source).unwrap().abilities) = vec![ + AbilityDefinition::new(AbilityKind::Activated, draw_effect()), + AbilityDefinition::new( + AbilityKind::Activated, + poison_effect(TargetFilter::Opponent), + ), + ]; + let decision = priority_decision(); + + let (poison_delta, poison_reason) = routed_verdict(&ctx( + &state, + &activate_ability_candidate(source, 1), + &decision, + &context, + &config, + )) + .expect("the poison ability must reach the policy through the registry"); + assert_eq!(poison_reason.kind, "poison_clock_lethal"); + assert!(poison_delta > 0.0); + + // The draw ability on the same object is routed but scores nothing — the + // per-ability `ability_index` lookup is discriminating, not the object. + let (draw_delta, draw_reason) = routed_verdict(&ctx( + &state, + &activate_ability_candidate(source, 0), + &decision, + &context, + &config, + )) + .expect("the non-poison ability is still routed"); + assert_eq!(draw_reason.kind, "poison_clock_na"); + assert_eq!(draw_delta, 0.0); + + // An out-of-range index degrades to neutral, never panics. + let (oob_delta, oob_reason) = routed_verdict(&ctx( + &state, + &activate_ability_candidate(source, 9), + &decision, + &context, + &config, + )) + .expect("an out-of-range ability index is still routed"); + assert_eq!(oob_reason.kind, "poison_clock_na"); + assert_eq!(oob_delta, 0.0); +} + +/// End-to-end routing for a modal SPELL (as opposed to the modal *ability* +/// covered above): `WaitingFor::ModeChoice` carries a `PendingCast`, and the +/// policy reads the chosen mode from the spell object's spell-kind abilities via +/// `modal_spell_mode_ability_refs` — the sole production consumer of that new +/// engine API. The poison and non-poison modes come out discriminated. +#[test] +fn registry_routes_modal_spell_mode_choice_to_the_policy() { + let config = config(); + let context = committed_context(&config); + let mut state = state_with_players(2); + state.players[1].poison_counters = 9; + + // A modal spell: its two printed modes are spell-kind abilities on the + // object (mode 0 draws, mode 1 poisons). + let spell = spell_object( + &mut state, + 1, + vec![ + AbilityDefinition::new(AbilityKind::Spell, draw_effect()), + AbilityDefinition::new(AbilityKind::Spell, poison_effect(TargetFilter::Opponent)), + ], + ); + let modal = ModalChoice { + min_choices: 1, + max_choices: 1, + mode_count: 2, + ..ModalChoice::default() + }; + state.objects.get_mut(&spell).unwrap().modal = Some(modal.clone()); + + // A `WaitingFor::ModeChoice` whose PendingCast points at that spell object. + let resolved = ResolvedAbility::new(draw_effect(), Vec::new(), spell, AI); + let pending_cast = PendingCast::new(spell, CardId(spell.0), resolved, ManaCost::zero()); + let decision = AiDecisionContext { + waiting_for: WaitingFor::ModeChoice { + player: AI, + modal, + pending_cast: Box::new(pending_cast), + unavailable_modes: Vec::new(), + }, + candidates: Vec::new(), + }; + + let (poison_delta, poison_reason) = routed_verdict(&ctx( + &state, + &select_modes_candidate(vec![1]), + &decision, + &context, + &config, + )) + .expect("the poison spell-mode must reach the policy through the registry"); + assert_eq!(poison_reason.kind, "poison_clock_lethal"); + assert!(poison_delta > 0.0); + + let (draw_delta, draw_reason) = routed_verdict(&ctx( + &state, + &select_modes_candidate(vec![0]), + &decision, + &context, + &config, + )) + .expect("the non-poison spell-mode is still routed"); + assert_eq!(draw_reason.kind, "poison_clock_na"); + assert_eq!(draw_delta, 0.0); +} From 5e83f7e639a79846cb71326abb36129afd286206 Mon Sep 17 00:00:00 2001 From: minion1227 Date: Thu, 23 Jul 2026 16:44:13 -0700 Subject: [PATCH 5/5] docs(phase-ai): drop the wrong CR 104.3d cite on the land-exclusion test (#6543) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CR 104.3d is the ten-poison-counters loss rule; it says nothing about excluding lands from a commitment denominator. That exclusion is a deck-modelling choice in `detect`, not a rules requirement, so it carries no CR citation at all. Per CLAUDE.md a wrong CR number is worse than none — it creates false confidence that the code was verified against that rule. Comment-only; the guard and its assertions are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/phase-ai/src/features/tests/poison.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/phase-ai/src/features/tests/poison.rs b/crates/phase-ai/src/features/tests/poison.rs index 181964a440..b6cc4b242c 100644 --- a/crates/phase-ai/src/features/tests/poison.rs +++ b/crates/phase-ai/src/features/tests/poison.rs @@ -197,11 +197,12 @@ fn superfriends_proliferate_is_pinned_below_floor() { assert!(feature.commitment < POISON_CLOCK_FLOOR); } -/// CR 104.3d: lands are excluded from the commitment denominator (they are not -/// part of the poison plan's nonland payload). Two decks with identical nonland -/// content score identically no matter how many lands pad the manabase — pinning -/// the `CoreType::Land` guard in `detect`, whose removal would inflate the -/// denominator and silently lower every commitment. +/// Lands are excluded from the commitment denominator — they are not part of +/// the poison plan's nonland payload. This is a deck-modelling choice, not a +/// rules requirement, so it carries no CR citation. Two decks with identical +/// nonland content score identically no matter how many lands pad the manabase, +/// pinning the `CoreType::Land` guard in `detect`, whose removal would inflate +/// the denominator and silently lower every commitment. #[test] fn lands_are_excluded_from_commitment_denominator() { let core = |extra: Vec| {