From 46c5e9d6d651556c4cb213c636e2db2bbbddb472 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Mon, 3 Aug 2026 14:44:22 -0700 Subject: [PATCH 1/3] fix(ai): gate projection wall-clock cap on measurement mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `projection::project_to` bailed with `TimeCapExceeded` after a 15ms wall-clock cap that was not gated on `ExecutionMode::is_measurement()`, unlike the two existing search budgets (`planner::PlannerServices::with_deadline` and `search.rs`), which both null their deadline under measurement. That made `cargo ai-gate` verdicts host-speed dependent. The reachability is direct: `EvasionRemovalPriorityPolicy` is registered unconditionally, its `velocity_score` projects the opponent, a wall-clock bail scores 0.0 where a completed projection scores up to +3.0, and that term selects which creature the AI targets for removal — changing (winner, turns). Changes: - `TIME_CAP: Duration` becomes private `TIME_CAP_MS: u32`; the new `projection_deadline(ExecutionMode) -> Deadline` is the single producer, so no call site writes the magnitude or builds a projection deadline itself. - `project_to` and `AiSession::get_or_project` take the deadline explicitly. The deadline gates computation only — a cache hit is served regardless of expiry (covered by a dedicated multi-authority test). - `combat_ai` replaces its `combat_lookahead: bool` parameter with a payload-carrying `CombatLookahead` enum, making "lookahead enabled with no budget" unrepresentable. The deadline is constructed inside the crackback block, not at the call site, so the attacker heuristic's prologue does not consume the projection budget. - Adds `projection_fixtures`, the crate's first full-loop (non already-at-horizon) projection fixture class, with reach guards. Measurement mode is now bounded by `STEP_CAP` alone. Live-play behavior is unchanged in kind: the predicate is identical and only CEDH enables combat lookahead. Comment-only: root `Cargo.toml` and `duel_suite/run.rs` both documented this defect as live and are now false; `policies/context.rs` records that `can_afford_projection`'s `is_none_or` is load-bearing and must not be "fixed" into `is_some_and`. No baseline refreshed. Context: #6967 --- Cargo.toml | 31 +- crates/phase-ai/src/combat_ai.rs | 204 +++++++-- crates/phase-ai/src/duel_suite/run.rs | 12 +- crates/phase-ai/src/policies/context.rs | 16 + .../src/policies/evasion_removal_priority.rs | 164 ++++++- crates/phase-ai/src/projection.rs | 411 +++++++++++++++++- crates/phase-ai/src/search.rs | 93 +++- crates/phase-ai/src/session.rs | 140 +++++- 8 files changed, 991 insertions(+), 80 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3e57a3b8c9..e34ccddd5b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,23 +52,20 @@ split-debuginfo = "unpacked" # Measured on the linked artifact: # readelf --debug-dump=info target/debug/ai-gate | grep -A1 'GNU C23' # reported `-O0` for exactly that file before this override and `-O2` after it. -# This is NOT a no-op on the gates' payloads, and the honest statement of why is worth -# more than a clean claim. The AI reads the wall clock on its live decision path: -# `phase-ai/src/projection.rs:110` is `TIME_CAP = 15ms` and `:139-142` bails on it, and -# unlike `search.rs`'s and `planner/mod.rs`'s deadlines it is NOT gated on measurement -# mode. It is reachable at the gate's default `AiDifficulty::Medium` — registry.rs -# registers `EvasionRemovalPriorityPolicy` unconditionally, and its `velocity_score` -# calls `AiSession::get_or_project` → `project_to`, with the `projection_min_budget_ms` -# guard bypassed because a measurement-mode `Deadline` reports no remaining budget to -# compare against. A bail scores 0.0 where a completed projection scores up to +3.0, and -# that term picks the removal target. So making allocation faster lets more projections -# finish, which can change a target, a board, a winner and every counter downstream. -# That hazard is pre-existing and profile-wide (it fires on any faster or slower host); -# this override does not create it and cannot avoid it. It is recorded here because the -# obvious "allocator changes are invisible" claim is false, and the fix — gating -# `TIME_CAP` on measurement mode the way `search.rs:2012` does — belongs in its own -# change with its own baseline sign-off. `RandomState` (#4878) is a separate, -# already-documented source and is seeded from OS randomness, not allocation addresses. +# This override is still not *provably* payload-neutral, but the one known +# mechanism is now closed. The AI used to read the wall clock on its live decision +# path: `phase-ai/src/projection.rs` bailed a projection after a 15 ms wall-clock +# budget, and — unlike `search.rs`'s and `planner/mod.rs`'s deadlines — that +# budget was the one still missing the measurement-mode carve-out, so a faster +# allocator let more projections finish and could change a target, a board, a +# winner and every counter downstream. That is fixed: +# `projection::projection_deadline` returns `Deadline::none()` under +# `ExecutionMode::Measurement`, mirroring the other two, so the gates' payloads no +# longer contain a decision-affecting wall-clock read that allocation speed can +# move. What remains host-variable on the measurement path is `RandomState` +# iteration order (#4878), which is seeded from OS randomness, not allocation +# addresses. This block is kept because the obvious "allocator changes are +# invisible" claim was false once and the record is worth more than a clean claim. # Scoped to the one package so nothing else loses debug fidelity. [profile.dev.package.libmimalloc-sys] opt-level = 2 diff --git a/crates/phase-ai/src/combat_ai.rs b/crates/phase-ai/src/combat_ai.rs index 00da0ac276..05f537524c 100644 --- a/crates/phase-ai/src/combat_ai.rs +++ b/crates/phase-ai/src/combat_ai.rs @@ -17,10 +17,10 @@ use engine::types::player::PlayerId; use engine::types::statics::StaticMode; use engine::types::zones::Zone; -use crate::config::AiProfile; +use crate::config::{AiConfig, AiProfile, ExecutionMode}; use crate::damage_reflection::has_damage_reflection_to_controller; use crate::eval::{evaluate_creature, threat_level}; -use crate::projection::{project_to, Projection, ProjectionHorizon}; +use crate::projection::{project_to, projection_deadline, Projection, ProjectionHorizon}; use crate::session::AiSession; /// Block-legality static slices collected once per combat decision and threaded @@ -76,6 +76,45 @@ enum CombatObjective { Race, } +/// Whether the attacker heuristic may spend an opponent-turn projection on +/// crackback analysis, and the execution regime that projection runs under. +/// +/// The regime travels with the permission so the two cannot drift: a caller +/// cannot enable lookahead without stating a regime, and a caller that disables +/// it never has to invent one (`search::deterministic_combat_choice` has no +/// `AiConfig` at all). Replaces a `combat_lookahead: bool` that could express +/// only half the decision. +/// +/// Carries `ExecutionMode`, NOT a `Deadline`, deliberately: a `Deadline` +/// snapshots an absolute instant at construction, and this value is built as an +/// argument at `search::deterministic_choice` — before this function's opponent +/// enumeration, must-attack sweep, `adversarial_swarm_witness` reducer replay, +/// block-legality collection and per-candidate `defender_best_block` loop have +/// run. Anchoring the 15 ms budget there would let that prologue consume it and +/// silently disable CEDH crackback lookahead on a large board. The `Deadline` is +/// therefore constructed at the point of use, below. +#[derive(Debug, Clone, Copy)] +pub enum CombatLookahead { + Disabled, + Enabled { execution_mode: ExecutionMode }, +} + +impl CombatLookahead { + /// `AiConfig::combat_lookahead` decides permission; `execution_mode` travels + /// with it so the measurement carve-out reaches the projection. Only CEDH + /// enables this today, but `cargo ai-gate --difficulty cedh` is a supported + /// invocation, so the carve-out must hold here too. + pub fn from_config(config: &AiConfig) -> Self { + if config.combat_lookahead { + Self::Enabled { + execution_mode: config.execution_mode, + } + } else { + Self::Disabled + } + } +} + fn emit_attack_trace( player: PlayerId, candidate_attackers: &[ObjectId], @@ -142,7 +181,7 @@ pub fn choose_attackers_with_targets( state, player, &AiProfile::default(), - false, + CombatLookahead::Disabled, None, None, None, @@ -153,7 +192,7 @@ pub fn choose_attackers_with_targets_with_profile( state: &GameState, player: PlayerId, profile: &AiProfile, - combat_lookahead: bool, + lookahead: CombatLookahead, valid_attacker_ids: Option<&[ObjectId]>, valid_attack_targets: Option<&[AttackTarget]>, session: Option<&AiSession>, @@ -340,32 +379,43 @@ pub fn choose_attackers_with_targets_with_profile( // crackback_damage sees scaled creatures (Ouroboroid class) and // attack-trigger pumps (Battle Cry, Mentor). Failure to project // falls through to current state — matches pre-projection behavior. - let projection: Option> = if combat_lookahead { - match session { - // Session present: route through the per-game projection cache - // (turn-scoped key; identical result to project_to on a miss, - // cached on subsequent identical combat decisions this turn). - Some(session) => session - .get_or_project( + let projection: Option> = match lookahead { + CombatLookahead::Disabled => None, + CombatLookahead::Enabled { execution_mode } => { + // Constructed HERE, not at the caller: `Deadline::after` snapshots + // an absolute instant, and everything above in this function + // (must-attack sweep, adversarial_swarm_witness reducer replay, + // block-legality slices, per-candidate defender_best_block) would + // otherwise run inside the 15 ms projection budget. + let deadline = projection_deadline(execution_mode); + match session { + // Session present: route through the per-game projection cache + // (turn-scoped key; identical result to project_to on a miss, + // cached on subsequent identical combat decisions this turn). + Some(session) => session + .get_or_project( + state, + player, + opponents[0], + ProjectionHorizon::OpponentAttackersDeclared, + deadline, + ) + .ok(), + // No session: the planner's production quiescence loop, the + // public `choose_attackers_with_targets` wrapper, and tests. + // Fall back to the free projection, wrapped in Arc to unify + // the branch type. + None => project_to( state, player, opponents[0], ProjectionHorizon::OpponentAttackersDeclared, + deadline, ) - .ok(), - // No session (public wrappers, tests): fall back to the free - // projection, wrapped in Arc to unify the branch type. - None => project_to( - state, - player, - opponents[0], - ProjectionHorizon::OpponentAttackersDeclared, - ) - .ok() - .map(Arc::new), + .ok() + .map(Arc::new), + } } - } else { - None }; let cb_damage = crackback_damage( state, @@ -3505,7 +3555,7 @@ mod tests { runner.state(), PlayerId(0), &AiProfile::default(), - false, + CombatLookahead::Disabled, Some(&valid_attacker_ids), Some(&valid_attack_targets), None, @@ -3571,7 +3621,7 @@ mod tests { runner.state(), PlayerId(0), &AiProfile::default(), - false, + CombatLookahead::Disabled, Some(&valid_attacker_ids), Some(&valid_attack_targets), None, @@ -3646,7 +3696,7 @@ mod tests { runner.state(), PlayerId(0), &AiProfile::default(), - false, + CombatLookahead::Disabled, Some(&valid_attacker_ids), Some(&valid_attack_targets), None, @@ -3699,7 +3749,7 @@ mod tests { runner.state(), PlayerId(0), &AiProfile::default(), - false, + CombatLookahead::Disabled, Some(&valid_attacker_ids), Some(&valid_attack_targets), None, @@ -3782,7 +3832,7 @@ mod tests { runner.state(), PlayerId(0), &AiProfile::default(), - false, + CombatLookahead::Disabled, Some(&valid_attacker_ids), Some(&valid_attack_targets), None, @@ -3839,7 +3889,7 @@ mod tests { runner.state(), PlayerId(0), &AiProfile::default(), - false, + CombatLookahead::Disabled, Some(&valid_attacker_ids), Some(&valid_attack_targets), None, @@ -3867,7 +3917,7 @@ mod tests { &state, PlayerId(0), &AiProfile::default(), - false, + CombatLookahead::Disabled, None, Some(&targets), None, @@ -3902,7 +3952,7 @@ mod tests { &state, PlayerId(0), &AiProfile::default(), - false, + CombatLookahead::Disabled, None, Some(&targets), None, @@ -3928,7 +3978,7 @@ mod tests { &state, PlayerId(0), &AiProfile::default(), - false, + CombatLookahead::Disabled, None, Some(&targets), None, @@ -3957,7 +4007,7 @@ mod tests { &state, PlayerId(0), &AiProfile::default(), - false, + CombatLookahead::Disabled, None, Some(&targets), None, @@ -3987,7 +4037,7 @@ mod tests { &state, PlayerId(0), &AiProfile::default(), - false, + CombatLookahead::Disabled, None, Some(&targets), None, @@ -4037,7 +4087,10 @@ mod tests { &state, PlayerId(0), &profile, - /* combat_lookahead = */ true, + /* lookahead = */ + CombatLookahead::Enabled { + execution_mode: ExecutionMode::Interactive, + }, None, None, Some(&session), @@ -4078,7 +4131,7 @@ mod tests { &state, PlayerId(0), &profile, - /* combat_lookahead = */ false, + /* lookahead = */ CombatLookahead::Disabled, None, None, Some(&session), @@ -4103,7 +4156,10 @@ mod tests { &state, PlayerId(0), &profile, - /* combat_lookahead = */ true, + /* lookahead = */ + CombatLookahead::Enabled { + execution_mode: ExecutionMode::Interactive, + }, None, None, Some(&AiSession::empty()), @@ -4112,7 +4168,10 @@ mod tests { &state, PlayerId(0), &profile, - /* combat_lookahead = */ true, + /* lookahead = */ + CombatLookahead::Enabled { + execution_mode: ExecutionMode::Interactive, + }, None, None, None, @@ -4123,4 +4182,71 @@ mod tests { "session-cached projection must yield the identical attacker decision as the free path" ); } + + /// T5a + T5b — `CombatLookahead::from_config` binds two authorities into one + /// value, and each must survive the binding. + /// + /// T5a (regime survives): the CEDH preset is the only one enabling combat + /// lookahead, so it is the only config that can carry a regime here. Its + /// measurement variant must arrive as `Enabled { execution_mode }` with the + /// measurement regime intact — `from_config` hardcoding + /// `ExecutionMode::Interactive` turns this red, and that is exactly the bug + /// that would leave the gate reading the wall clock at `--difficulty cedh`. + /// + /// T5b (permission dominates): with `combat_lookahead == false` the result + /// is `Disabled` in BOTH regimes, so measurement mode cannot smuggle a + /// projection into a tier that never takes one. + #[test] + fn combat_lookahead_from_config_carries_execution_mode() { + use crate::config::{create_config, AiDifficulty, Platform}; + + // T5a. + let cedh = create_config(AiDifficulty::CEDH, Platform::Native); + assert!( + cedh.combat_lookahead, + "T5a precondition: CEDH must be the tier that enables combat lookahead — if this \ + preset changes, this test is measuring the wrong config" + ); + let measured = CombatLookahead::from_config(&cedh.clone().into_measurement(1)); + assert!( + matches!( + measured, + CombatLookahead::Enabled { execution_mode } if execution_mode.is_measurement() + ), + "a measurement CEDH config must produce Enabled carrying the measurement regime; \ + got {measured:?}" + ); + let interactive = CombatLookahead::from_config(&cedh); + assert!( + matches!( + interactive, + CombatLookahead::Enabled { execution_mode } if !execution_mode.is_measurement() + ), + "an interactive CEDH config must produce Enabled carrying the interactive regime; \ + got {interactive:?}" + ); + + // T5b — the permission axis dominates in both regimes. + let medium = create_config(AiDifficulty::Medium, Platform::Native); + assert!( + !medium.combat_lookahead, + "T5b precondition: Medium must NOT enable combat lookahead — a future preset flip \ + must fail loudly here rather than pass silently" + ); + assert!( + matches!( + CombatLookahead::from_config(&medium), + CombatLookahead::Disabled + ), + "combat_lookahead == false must map to Disabled in interactive mode" + ); + assert!( + matches!( + CombatLookahead::from_config(&medium.into_measurement(1)), + CombatLookahead::Disabled + ), + "combat_lookahead == false must map to Disabled in measurement mode too — \ + measurement must not enable a projection the tier never takes" + ); + } } diff --git a/crates/phase-ai/src/duel_suite/run.rs b/crates/phase-ai/src/duel_suite/run.rs index e3de8690f7..43b66fbf88 100644 --- a/crates/phase-ai/src/duel_suite/run.rs +++ b/crates/phase-ai/src/duel_suite/run.rs @@ -836,12 +836,12 @@ fn run_game_observed( /// the historical `run_game` body, which capped at `MAX_TOTAL_ACTIONS`). The /// result `(winner, turn_number)` is a function of /// `(binary, payload, seed, difficulty, action_cap)` and nothing this function -/// itself reads — but it is NOT wall-clock-free further down. `projection.rs`'s -/// `TIME_CAP` (`projection.rs:110`, 15 ms) is not gated on measurement mode and is -/// reached at `AiDifficulty::Medium` through `EvasionRemovalPriorityPolicy`'s -/// `velocity_score`, so a faster or slower host can change which creature the AI -/// targets. See the run-to-run caveats at the top of [`super::perf`]; this is a -/// second source alongside #4878. +/// itself reads. `projection.rs`'s wall-clock projection cap is now gated on +/// measurement mode (`projection::projection_deadline` returns +/// `Deadline::none()` under `ExecutionMode::Measurement`), so projections here +/// are bounded by `STEP_CAP` and host speed cannot change which creature the AI +/// targets. The remaining run-to-run caveat is `RandomState` iteration order +/// (#4878) — see the notes at the top of [`super::perf`]. pub(crate) fn drive_game( payload: &DeckPayload, seed: u64, diff --git a/crates/phase-ai/src/policies/context.rs b/crates/phase-ai/src/policies/context.rs index 28d534265b..9a17410cc7 100644 --- a/crates/phase-ai/src/policies/context.rs +++ b/crates/phase-ai/src/policies/context.rs @@ -83,6 +83,22 @@ impl<'a> PolicyContext<'a> { /// policies that project should gate their work behind this helper so /// the tightest-budget path (Medium, 1500ms) doesn't pay the ~1.5s /// simulation cost and blow its own budget. + /// + /// The `remaining().is_none_or(..)` resolving to `true` on a + /// `Deadline::none()` deadline is deliberate and load-bearing: measurement + /// runs have no wall clock and MUST still take projections, so `cargo + /// ai-gate` measures the same policy production runs. Changing it to + /// `is_some_and` would pin `velocity_score` to 0.0 for every uncached + /// projection in the gate — a far larger baseline move than any wall-clock + /// fix, measuring a policy that never ships. + /// + /// One production path also reaches here with a never-overwritten + /// `Deadline::none()`: `search::emit_decision_trace` builds its `AiContext` + /// via `build_ai_context_with_session` (which initializes `deadline` to + /// `none()`) and never routes through `PlannerServices::with_deadline`. That + /// path is diagnostic (gated on `phase_ai::decision_trace` DEBUG) and feeds + /// the duel suite's attribution mode, so a flip would also silently change + /// trace output. pub fn can_afford_projection(&self) -> bool { if self.context.deadline.expired() { return false; diff --git a/crates/phase-ai/src/policies/evasion_removal_priority.rs b/crates/phase-ai/src/policies/evasion_removal_priority.rs index 676438a205..5c386420c7 100644 --- a/crates/phase-ai/src/policies/evasion_removal_priority.rs +++ b/crates/phase-ai/src/policies/evasion_removal_priority.rs @@ -137,6 +137,12 @@ fn evasion_score( /// signal and doesn't blow the user-visible turn-time budget for a /// nice-to-have bonus. The threshold comes from /// `SearchConfig::projection_min_budget_ms` so it's tunable per difficulty. +/// +/// Measurement mode (`cargo ai-gate`, duel suite) passes a non-expiring +/// projection deadline (`projection::projection_deadline`), so the simulation is +/// bounded by `STEP_CAP` rather than by host speed — a wall-clock bail here +/// scores `0.0` against a completed projection's up-to-`+3.0`, and this term +/// selects the removal target. fn velocity_score( ctx: &PolicyContext<'_>, target: &engine::game::game_object::GameObject, @@ -160,9 +166,13 @@ fn velocity_score( if !ctx.can_afford_projection() { return 0.0; } - let Ok(fresh) = - session.get_or_project(ctx.state, ctx.ai_player, target.controller, horizon) - else { + let Ok(fresh) = session.get_or_project( + ctx.state, + ctx.ai_player, + target.controller, + horizon, + crate::projection::projection_deadline(ctx.config.execution_mode), + ) else { return 0.0; }; fresh @@ -267,27 +277,42 @@ mod tests { } } - fn policy_score( + /// Score with a CALLER-OWNED `AiContext`, so the test retains a handle on + /// `context.session` (to inspect `projection_cache`) and on + /// `context.deadline` (to inject a pre-expired budget). Mirrors + /// `policies::context::deadline_test_ctx`. `policy_score` is the + /// don't-care-about-the-context wrapper. + fn policy_score_in_context( state: &GameState, decision: &AiDecisionContext, target: ObjectId, config: &AiConfig, + context: &crate::context::AiContext, ) -> f64 { let candidate = candidate_for(target); - let ai_context = crate::context::AiContext::empty(&config.weights); let ctx = PolicyContext { state, decision, candidate: &candidate, ai_player: P0, config, - context: &ai_context, + context, cast_facts: None, search_depth: crate::policies::context::SearchDepth::Root, }; EvasionRemovalPriorityPolicy.score(&ctx) } + fn policy_score( + state: &GameState, + decision: &AiDecisionContext, + target: ObjectId, + config: &AiConfig, + ) -> f64 { + let ai_context = crate::context::AiContext::empty(&config.weights); + policy_score_in_context(state, decision, target, config, &ai_context) + } + fn registry_delta( state: &GameState, decision: &AiDecisionContext, @@ -573,6 +598,133 @@ mod tests { ); } + /// T7 — the evasion production wiring is live under a measurement config: + /// `velocity_score` reaches `get_or_project` and the projection is taken. + /// + /// Revert-failing: replacing the `get_or_project` call with a + /// `cached_projection`-only lookup, or deleting the fresh-projection arm, + /// leaves `projection_cache` empty and turns the positive arm red. + /// + /// The fixture is deliberately ALREADY AT `OpponentBeginCombat`, the horizon + /// `velocity_score` hardcodes. That horizon is not reachable by priority + /// passing at all: `auto_advance_once`'s `Phase::BeginCombat` arm opens a + /// priority window only when a begin-combat trigger fires, and otherwise + /// either advances to `DeclareAttackers` or, per CR 508.8, enters + /// `PostCombatMain`. A "natural" fixture here would fail its reach guard + /// essentially always. + #[test] + fn velocity_score_takes_projection_under_measurement_config() { + let mut scenario = GameScenario::new_n_player(2, 42); + scenario.at_phase(Phase::PreCombatMain); + let bolt = scenario + .add_spell_to_hand_from_oracle(P0, "Lightning Bolt", true, LIGHTNING_BOLT_ORACLE) + .with_mana_cost(ManaCost::Cost { + shards: vec![ManaCostShard::Red], + generic: 0, + }) + .id(); + let killable = scenario + .add_creature(PlayerId(1), "Scrappy Skirmisher", 2, 2) + .id(); + scenario + .add_creature(PlayerId(1), "Looming Colossus", 7, 7) + .id(); + scenario.with_mana_pool( + P0, + vec![ManaUnit::new(ManaType::Red, ObjectId(0), false, vec![])], + ); + + let mut runner = scenario.build(); + let card_id = runner.state().objects[&bolt].card_id; + runner + .act(GameAction::CastSpell { + object_id: bolt, + card_id, + targets: Vec::new(), + payment_mode: CastPaymentMode::Auto, + }) + .expect("the real Lightning Bolt fixture should reach target selection"); + + // `decision` is captured from the PRE-mutation state and carries the + // TargetSelection pending cast, which is what + // `EvasionRemovalPriorityPolicy::score`'s first gate and + // `effect_classify::effect_source_id` both read. Capturing it AFTER the + // mutation below can never work: the mutation sets `waiting_for` to + // `Priority`, which IS the OpponentBeginCombat horizon predicate, and + // `build_decision_context` copies `state.waiting_for` verbatim. + let decision = build_decision_context(runner.state()); + assert!( + matches!(&decision.waiting_for, WaitingFor::TargetSelection { .. }), + "T7 fixture: `decision` must be captured BEFORE the horizon mutation" + ); + + // Mutate into the OpponentBeginCombat already-at-horizon shape: the + // predicate is active_player == target_opponent && phase == BeginCombat + // && stack empty && waiting_for == Priority { target_opponent }. + let mut state = runner.state().clone(); + state.active_player = PlayerId(1); + state.phase = Phase::BeginCombat; + state.stack.clear(); + state.priority_player = PlayerId(1); + state.waiting_for = WaitingFor::Priority { + player: PlayerId(1), + }; + // The bolt leaves the STACK but must remain a live object, because + // `effect_source_id` resolves the impact chain's source through it. + assert!(state.objects.contains_key(&bolt)); + + let config = create_config(AiDifficulty::Medium, Platform::Native).into_measurement(7); + + // Rung 1 — the policy's non-velocity gates still pass on the mutated + // state, AND this is the control arm. `Deadline::after(0)` is expired, so + // `can_afford_projection` returns false and `velocity_score` returns 0.0 + // before reaching `get_or_project`. A non-zero total score therefore + // proves the other gates pass, while the empty cache proves the + // projection did not run. + let mut expired_ctx = crate::context::AiContext::empty(&config.weights); + expired_ctx.deadline = engine::util::Deadline::after(0); + let control = policy_score_in_context(&state, &decision, killable, &config, &expired_ctx); + assert_ne!( + control, 0.0, + "T7 rung 1: EvasionRemovalPriorityPolicy::score must reach velocity_score on this \ + fixture — a 0.0 here means one of its gates rejected the mutated state, so the \ + POSITIVE arm below would be red for a fixture reason, not a wiring reason. Stop \ + and report; do not weaken the positive assertion." + ); + assert!( + expired_ctx + .session + .projection_cache + .read() + .unwrap() + .is_empty(), + "T7 control arm: with can_afford_projection() false, velocity_score must not project" + ); + + // Rung 2 — the projection itself succeeds on this state. + crate::projection::projection_fixtures::assert_already_at_horizon( + &state, + P0, + PlayerId(1), + crate::projection::ProjectionHorizon::OpponentBeginCombat, + ); + + // Positive arm. `AiContext::empty` initializes `deadline` to + // `Deadline::none()` — exactly what `PlannerServices::with_deadline` + // installs in measurement mode — so this reproduces the production + // deadline state rather than approximating it. Medium's + // `projection_min_budget_ms = 2000` would block the projection in + // interactive mode, so this arm also exercises the `is_none_or` bypass. + let ai_ctx = crate::context::AiContext::empty(&config.weights); + let _ = policy_score_in_context(&state, &decision, killable, &config, &ai_ctx); + assert_eq!( + ai_ctx.session.projection_cache.read().unwrap().len(), + 1, + "velocity_score must take the fresh projection through get_or_project under a \ + measurement config (revert-failing: cached_projection-only leaves this empty)" + ); + } + #[test] fn activated_removal_weights_controller_threat_but_beneficial_activation_is_neutral() { let destroy = Effect::Destroy { diff --git a/crates/phase-ai/src/projection.rs b/crates/phase-ai/src/projection.rs index 23c04eba92..e388463bd8 100644 --- a/crates/phase-ai/src/projection.rs +++ b/crates/phase-ai/src/projection.rs @@ -22,9 +22,12 @@ use engine::types::game_state::{ManaChoice, ManaChoicePrompt}; use engine::types::{ CoreType, GameAction, GameState, ObjectId, PayCostKind, Phase, PlayerId, WaitingFor, }; +use engine::util::Deadline; use web_time::{Duration, Instant}; +use crate::config::ExecutionMode; + /// How far into the opponent's upcoming turn to project. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum ProjectionHorizon { @@ -105,21 +108,61 @@ pub struct ProjectionKey { /// Outer dispatch cap. Each dispatch may trigger up to 500 engine-internal /// auto-pass iterations. const STEP_CAP: u32 = 256; -/// Wall-clock guard for projection. The combat path is a heuristic and must -/// fail closed to the pre-projection behavior rather than monopolize the AI -/// turn when the engine path is unusually expensive. -const TIME_CAP: Duration = Duration::from_millis(15); +/// Wall-clock guard for projection, in milliseconds. The combat path is a +/// heuristic and must fail closed to the pre-projection behavior rather than +/// monopolize the AI turn when the engine path is unusually expensive. +/// +/// Interactive callers only. Measurement mode receives a non-expiring budget +/// from [`projection_deadline`] and is bounded by `STEP_CAP` alone, so `cargo +/// ai-gate` verdicts cannot depend on host speed. +/// +/// Deliberately PRIVATE: `projection_deadline` is the only producer of a +/// projection budget, so no other module can construct one from this value. +const TIME_CAP_MS: u32 = 15; + +/// The wall-clock budget one `project_to` call runs under. +/// +/// **Single source of truth** — no caller writes `TIME_CAP_MS` (it is private) +/// and no caller constructs a projection `Deadline` itself. +/// +/// Measurement mode is bounded by `STEP_CAP` only — never wall clock. A bail +/// scores 0.0 where a completed projection scores up to +3.0 +/// (`policies::evasion_removal_priority::velocity_score`), and that term selects +/// the removal target, so a clock-dependent bail would make host speed an input +/// to `cargo ai-gate` verdicts. Mirrors +/// `planner::PlannerServices::with_deadline`. +/// +/// Call this at the point of consumption — the returned `Deadline` snapshots an +/// absolute instant, so binding it to a `let` that outlives one `project_to` +/// call silently shortens (and eventually zeroes) the budget. +pub fn projection_deadline(execution_mode: ExecutionMode) -> Deadline { + if execution_mode.is_measurement() { + Deadline::none() + } else { + Deadline::after(TIME_CAP_MS) + } +} /// Advance from `base` forward until `horizon` is reached on /// `target_opponent`'s next turn. `base` is cloned; never mutated. -/// Deterministic given `(base_fingerprint, ai_player, target_opponent, horizon)`. +/// Deterministic given `(base_fingerprint, ai_player, target_opponent, horizon)` +/// **and** a non-expiring `deadline`, which [`projection_deadline`] supplies in +/// measurement mode. pub fn project_to( base: &GameState, ai_player: PlayerId, target_opponent: PlayerId, horizon: ProjectionHorizon, + deadline: Deadline, ) -> Result { let started_turn = base.turn_number; + // Diagnostic only: feeds BailReason::TimeCapExceeded's `elapsed`. Never + // compared, never affects a decision. NOTE: it measures time spent INSIDE + // project_to, while the budget is anchored a few microseconds earlier at the + // caller's `projection_deadline(..)` call — so on a bail it under-reports the + // budget actually consumed by (caller-side hash + lock probe). Bounded by + // that, and it is a log field only. Unreachable in measurement mode, where + // `Deadline::none()` never expires. let started_at = Instant::now(); let mut state = base.clone(); let mut snapshots: Vec<(ProjectionHorizon, GameState)> = Vec::new(); @@ -137,9 +180,10 @@ pub fn project_to( } for step in 0..STEP_CAP { - let elapsed = started_at.elapsed(); - if elapsed >= TIME_CAP { - return Err(BailReason::TimeCapExceeded { elapsed }); + if deadline.expired() { + return Err(BailReason::TimeCapExceeded { + elapsed: started_at.elapsed(), + }); } capture_snapshots(&state, target_opponent, started_turn, &mut snapshots); @@ -661,6 +705,255 @@ pub fn threat_velocity( samples } +/// Shared full-loop projection fixtures. +/// +/// Every pre-existing projection fixture in this crate is *already at its +/// horizon*, so `project_to` short-circuits at the `Confidence::Exact` branch +/// above and never enters its loop. The states built here are deliberately the +/// opposite class: they are NOT at a horizon on entry, so the loop runs real +/// `apply_for_simulation` dispatches and returns +/// `Confidence::Approximated { choice_count >= 1 }` — the witness that the loop +/// ran rather than the short-circuit. +/// +/// Lives beside `project_to` because the invariant these states encode ("this +/// state traverses the loop to `OpponentAttackersDeclared`") is a property of +/// `project_to`'s own resolution policy, not of any consumer. +#[cfg(test)] +pub(crate) mod projection_fixtures { + use super::*; + use engine::game::zones::create_object; + use engine::types::identifiers::CardId; + use engine::types::zones::Zone; + + /// A battlefield creature that can attack on the turn after it was placed: + /// untapped, not summoning-sick, `entered_battlefield_turn = 1`. + /// + /// Mirrors `GameScenario::add_creature` (`scenario.rs:355-391`), which is + /// the builder recipe for a "pre-existing" (therefore not sick) creature. + fn spawn_vanilla_creature( + state: &mut GameState, + controller: PlayerId, + name: &str, + power: i32, + toughness: i32, + ) -> ObjectId { + let card_id = CardId(state.next_object_id); + let id = create_object( + state, + card_id, + controller, + name.to_string(), + Zone::Battlefield, + ); + let obj = state.objects.get_mut(&id).unwrap(); + obj.card_types.core_types.push(CoreType::Creature); + obj.base_card_types = obj.card_types.clone(); + obj.power = Some(power); + obj.toughness = Some(toughness); + obj.base_power = Some(power); + obj.base_toughness = Some(toughness); + obj.entered_battlefield_turn = Some(1); + obj.summoning_sick = false; + id + } + + /// A state that is NOT at any horizon on entry and that `project_to` + /// traverses to `ProjectionHorizon::OpponentAttackersDeclared` in a handful + /// of dispatches, with `ai_player = P0` and `target_opponent = P1`. + /// + /// Placed at the OPPONENT's own precombat main phase, which is what makes it + /// cheap and robust: + /// * `active_player == P1 == target_opponent`, so no turn boundary is + /// crossed — `turns.rs`'s turn-advance never clears + /// `creatures_attacked_this_turn` during the traversal. + /// * `Phase::PreCombatMain` is AFTER the draw step (CR 500.1 phase order), + /// so no library is ever read and both libraries may stay empty. A + /// projected draw from an empty library would end the game (CR 704.5b) + /// and return `BailReason::GameOverDuringProjection` — a red positive + /// arm for the wrong reason. + /// * No untap step runs, so tapped/untapped state is exactly as built. + /// + /// Traversal, all deterministic: + /// 0. `Priority { P1 }` → `pick_pass_or_first` picks `PassPriority` + /// 1. `Priority { P0 }` → `PassPriority`; both passed with an empty stack, + /// so the engine advances the phase. `auto_advance_once`'s BeginCombat + /// arm sees `has_potential_attackers(state) == true` and continues to + /// `DeclareAttackers` WITHOUT opening a priority window. + /// 2. `WaitingFor::DeclareAttackers` with `acting == target_opponent`, so + /// `resolve_choice` uses `pick_max_attackers_against(&actions, P0)` and + /// declares the bear against P0. `finish_declare_attackers` returns + /// `Priority { P1 }` with `creatures_attacked_this_turn` populated and + /// an empty stack (CR 508.1 turn-based action). + /// 3. Loop head: `reached_horizon` is true → `Ok(Projection { .. })`. + /// + /// `choice_count >= 1` because the DeclareAttackers dispatch is not a + /// `PassPriority`, so the result is `Confidence::Approximated` — the witness + /// that the LOOP ran rather than the already-at-horizon short-circuit, which + /// hardcodes `Confidence::Exact`. + pub(crate) fn opponent_turn_precombat_fixture() -> GameState { + let mut state = GameState::new_two_player(42); + state.turn_number = 2; + state.active_player = PlayerId(1); + state.phase = Phase::PreCombatMain; + // The coherent triple `GameScenario::at_phase` maintains + // (`scenario.rs:212-222`): phase + waiting_for + priority_player. + state.priority_player = PlayerId(1); + state.waiting_for = WaitingFor::Priority { + player: PlayerId(1), + }; + state.stack.clear(); + // creatures_attacked_this_turn stays EMPTY — this is what makes the + // fixture NOT already-at-horizon. + state.creatures_attacked_this_turn.clear(); + + // P1's attacker. `has_potential_attackers` (`combat.rs`) requires + // controller == active, Creature, !tapped, no Defender/can't-attack, and + // `entered_battlefield_turn < turn_number` (or Haste). 1 < 2 holds. + spawn_vanilla_creature(&mut state, PlayerId(1), "Projection Bear", 2, 2); + state + } + + /// A state at P0's declare-attackers step from which BOTH of the following + /// hold: + /// (i) `search::deterministic_choice` routes to the combat branch and + /// `combat_ai` reaches its crackback projection block, and + /// (ii) `project_to(.., P0, P1, OpponentAttackersDeclared, ..)` traverses + /// to its horizon. + /// + /// The tension the construction resolves: (i) needs P1 to have NO untapped + /// blocker, or `combat_ai`'s `if is_unblockable || opponent_blockers + /// .is_empty()` short-circuit does not fire and the value heuristic may + /// decline the attack, emptying `attacking_ids` and skipping the crackback + /// block. But (ii) needs P1 to HAVE an attack-capable creature on its own + /// next turn, or `has_potential_attackers` is false at P1's BeginCombat, the + /// declare-blockers and combat-damage steps are skipped (CR 508.8) and the + /// horizon is unreachable. + /// + /// Resolution: give P1 a **tapped** creature. `opponent_blockers` in + /// `combat_ai` filters on `!obj.tapped`, so it is invisible to (i); the + /// projected P1 untap step untaps it, so it satisfies (ii). + /// + /// Both libraries are stocked because the traversal crosses into P1's turn + /// and P1's draw step reads one card (CR 500.1 phase order; `should_skip_draw` + /// skips only on turn 1 in a 2-player game, and this fixture is on turn 2). + /// An empty library there ends the game (CR 704.5b) and returns + /// `BailReason::GameOverDuringProjection`. P0's library is stocked as cheap + /// insurance; the traversal is not expected to reach a P0 draw. + /// + /// Life totals are 20/20 and the lone attacker is 2 power, so neither + /// `determine_attack_objective` returns `PushLethal` nor + /// `adversarial_swarm_witness` (gated 2-player in `combat_ai`) certifies a + /// lethal, declaration-binding attack that would return before the crackback + /// block. **If either the attacker's power or P1's life is changed, both of + /// those reach conditions can silently flip.** + pub(crate) fn ai_turn_declare_attackers_fixture() -> GameState { + let mut state = GameState::new_two_player(42); + state.turn_number = 2; + state.active_player = PlayerId(0); + state.priority_player = PlayerId(0); + state.phase = Phase::DeclareAttackers; + state.players[0].life = 20; + state.players[1].life = 20; + state.stack.clear(); + state.creatures_attacked_this_turn.clear(); + + // P0's attacker: untapped, not sick. + let attacker = spawn_vanilla_creature(&mut state, PlayerId(0), "AI Bear", 2, 2); + debug_assert!(!state.objects[&attacker].tapped); + + // P1's FUTURE attacker: tapped now (so it is not an `opponent_blocker`), + // untaps during the projected P1 untap step. + let crackback = spawn_vanilla_creature(&mut state, PlayerId(1), "Crackback Bear", 2, 2); + state.objects.get_mut(&crackback).unwrap().tapped = true; + + // CR 704.5b insurance for the projected P1 draw step. + for i in 0..5 { + create_object( + &mut state, + CardId(900 + i), + PlayerId(0), + format!("Filler {i}"), + Zone::Library, + ); + create_object( + &mut state, + CardId(950 + i), + PlayerId(1), + format!("Filler {i}"), + Zone::Library, + ); + } + + // The engine owns the DeclareAttackers payload shape — do not hand-write + // its five fields. `build_declare_attackers_waiting_for` derives every + // one of them from `state` via the single `AttackDeclarationConstraints` + // authority. + state.combat = Some(engine::game::combat::CombatState::default()); + state.waiting_for = engine::game::combat::build_declare_attackers_waiting_for(&state); + state + } + + /// Fail loudly, with the engine's own bail reason in the message, if `state` + /// is ALREADY at `horizon` — which would make `project_to` short-circuit and + /// silently turn every "the loop ran" assertion vacuous. + /// + /// Implemented against the public primitive rather than the private + /// `reached_horizon`: a `Confidence::Exact` result from a fixture that is + /// supposed to traverse IS the short-circuit, so this is a direct + /// observation, not a proxy. + pub(crate) fn assert_traverses_to( + state: &GameState, + ai_player: PlayerId, + target_opponent: PlayerId, + horizon: ProjectionHorizon, + ) { + let result = project_to(state, ai_player, target_opponent, horizon, Deadline::none()); + match &result { + Ok(projection) => { + assert_eq!( + projection.horizon_reached, horizon, + "fixture reached the wrong horizon" + ); + assert!( + matches!( + projection.confidence, + Confidence::Approximated { choice_count } if choice_count >= 1 + ), + "FIXTURE DEFECT, not a wiring defect: this fixture must TRAVERSE \ + project_to's loop, but it returned Confidence::Exact — which the \ + already-at-horizon short-circuit hardcodes and a real traversal \ + through a DeclareAttackers dispatch cannot produce. Re-derive the \ + fixture; do not weaken this assertion." + ); + } + Err(reason) => panic!( + "FIXTURE DEFECT, not a wiring defect: project_to could not reach \ + {horizon:?} under a non-expiring deadline. Bail reason: {reason:?}. \ + GameOverDuringProjection ⇒ stock the libraries; StepCapExceeded ⇒ the \ + horizon is not reachable from this state at all; NoLegalAction ⇒ the \ + waiting_for/priority_player triple is incoherent." + ), + } + } + + /// The already-at-horizon counterpart: assert the state short-circuits, so + /// the determinism claim of a fixture in that class is checked rather than + /// assumed. + pub(crate) fn assert_already_at_horizon( + state: &GameState, + ai_player: PlayerId, + target_opponent: PlayerId, + horizon: ProjectionHorizon, + ) { + let result = project_to(state, ai_player, target_opponent, horizon, Deadline::none()); + assert!( + matches!(&result, Ok(p) if p.horizon_reached == horizon + && matches!(p.confidence, Confidence::Exact)), + "fixture must be already-at-horizon (Confidence::Exact, no simulation); got {result:?}" + ); + } +} + #[cfg(test)] mod tests { use super::*; @@ -672,6 +965,108 @@ mod tests { use engine::types::mana::{ManaColor, ManaCost, ManaCostShard}; use engine::types::zones::Zone; + /// T1. Both directions in one test: measurement maps to a non-expiring + /// budget, interactive maps to a finite one no larger than the 15 ms cap. + /// + /// A stub returning `none()` for both modes fails the interactive half; a + /// stub returning `after(15)` for both fails the measurement half; deleting + /// the `is_measurement()` branch fails the measurement half. + /// + /// No strict lower bound on the interactive budget: `remaining()` uses + /// `saturating_duration_since`, so a >15 ms scheduling stall between + /// construction and assertion would yield `Some(0)` and a spurious red. + /// `is_some()` + `<= 15ms` still discriminates both stubs without the flake. + #[test] + fn projection_deadline_nulls_wall_clock_only_in_measurement() { + let measurement = projection_deadline(ExecutionMode::Measurement { seed: 7 }); + assert!( + measurement.remaining().is_none(), + "measurement mode must receive a non-expiring budget, bounded by STEP_CAP alone" + ); + assert!( + !measurement.expired(), + "a non-expiring budget must never report expiry" + ); + + let interactive = projection_deadline(ExecutionMode::Interactive); + let remaining = interactive + .remaining() + .expect("interactive mode must receive a finite wall-clock budget"); + assert!( + remaining <= Duration::from_millis(15), + "the interactive budget must not exceed the 15 ms cap; got {remaining:?}" + ); + } + + /// T2a + T2b + T2c on one fixture: the full-loop state completes under a + /// non-expiring deadline and bails with the SPECIFIC `TimeCapExceeded` + /// reason under a pre-expired one. + /// + /// Pairing both directions on the identical fixture is what makes the + /// negative non-vacuous: a bare `is_err()` on a fixture that bails anyway + /// (`NoLegalAction`, `StepCapExceeded`, `GameOverDuringProjection`) would + /// pass without the deadline being read at all. + #[test] + fn project_to_completes_under_none_and_bails_under_expired() { + // T2c — instrument witness, asserted first so a broken injection fails + // here rather than silently downstream. + assert!( + Deadline::after(0).expired(), + "instrument: Deadline::after(0) must be expired on the next read" + ); + assert!( + !Deadline::none().expired(), + "instrument: Deadline::none() must never expire" + ); + + let state = projection_fixtures::opponent_turn_precombat_fixture(); + + // Reach guard: this state must TRAVERSE the loop, not short-circuit. + projection_fixtures::assert_traverses_to( + &state, + PlayerId(0), + PlayerId(1), + ProjectionHorizon::OpponentAttackersDeclared, + ); + + // T2a — positive: with no wall clock the loop runs to the horizon. + let completed = project_to( + &state, + PlayerId(0), + PlayerId(1), + ProjectionHorizon::OpponentAttackersDeclared, + Deadline::none(), + ) + .expect("a non-expiring deadline must let the projection complete"); + assert_eq!( + completed.horizon_reached, + ProjectionHorizon::OpponentAttackersDeclared + ); + assert!( + matches!( + completed.confidence, + Confidence::Approximated { choice_count } if choice_count >= 1 + ), + "the loop must have run: the already-at-horizon short-circuit hardcodes \ + Confidence::Exact and cannot produce Approximated" + ); + + // T2b — negative: a pre-expired deadline bails at the loop head with the + // specific wall-clock reason, on the SAME fixture proven completable above. + let bailed = project_to( + &state, + PlayerId(0), + PlayerId(1), + ProjectionHorizon::OpponentAttackersDeclared, + Deadline::after(0), + ); + assert!( + matches!(bailed, Err(BailReason::TimeCapExceeded { .. })), + "a pre-expired deadline must bail with TimeCapExceeded specifically, \ + not merely with some error; got {bailed:?}" + ); + } + #[test] fn projection_declines_optional_loop_shortcut_from_legal_actions() { let mut state = GameState::new_two_player(42); diff --git a/crates/phase-ai/src/search.rs b/crates/phase-ai/src/search.rs index 42151e0f3b..ba4ffebf59 100644 --- a/crates/phase-ai/src/search.rs +++ b/crates/phase-ai/src/search.rs @@ -27,7 +27,9 @@ use engine::types::zones::Zone; use crate::card_value::{cmp_keep, intrinsic_value, keep_key}; use crate::cast_facts::cast_facts_for_action; -use crate::combat_ai::{choose_attackers_with_targets_with_profile, choose_blockers_with_profile}; +use crate::combat_ai::{ + choose_attackers_with_targets_with_profile, choose_blockers_with_profile, CombatLookahead, +}; use crate::config::{AiConfig, PlannerMode, ThreatAwareness}; use crate::context::AiContext; use crate::features::DeckFeatures; @@ -3690,7 +3692,7 @@ pub(crate) fn deterministic_choice( state, ai_player, &config.profile, - config.combat_lookahead, + CombatLookahead::from_config(config), Some(valid_attacker_ids), Some(valid_attack_targets), context.map(|c| c.session.as_ref()), @@ -3755,7 +3757,7 @@ fn deterministic_combat_choice( state, ai_player, profile, - false, + CombatLookahead::Disabled, Some(valid_attacker_ids), Some(valid_attack_targets), session, @@ -4394,6 +4396,91 @@ mod tests { state } + /// T8 — the combat production wiring at `deterministic_choice`'s combat + /// branch derives its lookahead from the config via `from_config`, rather + /// than passing a literal variant. + /// + /// This is the ONLY probe that turns red if that argument is written as the + /// disabled variant. The 15 `combat_ai.rs` call-site tests + /// structurally cannot see the mistake — they call + /// `choose_attackers_with_targets_with_profile` directly and never traverse + /// `deterministic_choice`. + /// + /// Both arms share one `state` deliberately. If any of the three combat + /// reach conditions fails, the positive arm fails loudly but the negative + /// sibling would pass VACUOUSLY (empty cache because the crackback block was + /// never reached, not because lookahead was off), so the negative is only + /// meaningful while the positive is green on the same fixture. Guard 2 + /// converts "the projection never completed" from a silent vacuity into a + /// named panic before either arm runs. + #[test] + fn deterministic_choice_routes_cedh_combat_lookahead_through_config() { + let state = crate::projection::projection_fixtures::ai_turn_declare_attackers_fixture(); + + // Guard 1 — the combat branch has something to work with. + match &state.waiting_for { + WaitingFor::DeclareAttackers { + valid_attacker_ids, + valid_attack_targets, + .. + } => { + assert!( + !valid_attacker_ids.is_empty(), + "T8 guard 1: the engine's own constraints model found no legal attacker, so \ + combat_ai's candidate list is empty and the crackback block is unreachable. \ + Fixture defect." + ); + assert!( + !valid_attack_targets.is_empty(), + "T8 guard 1: no legal attack target" + ); + } + other => panic!("T8 guard 1: fixture must be at DeclareAttackers, got {other:?}"), + } + + // Guard 2 — the projection the crackback block will take completes. + crate::projection::projection_fixtures::assert_traverses_to( + &state, + PlayerId(0), + PlayerId(1), + crate::projection::ProjectionHorizon::OpponentAttackersDeclared, + ); + + // Positive arm — CEDH is the only preset enabling combat lookahead. + let cedh = create_config(AiDifficulty::CEDH, Platform::Native).into_measurement(7); + assert!( + cedh.combat_lookahead, + "T8: the CEDH preset must enable combat lookahead" + ); + let ctx = crate::context::AiContext::empty(&cedh.weights); + let _ = deterministic_choice(&state, PlayerId(0), &cedh, &[], Some(&ctx)); + assert_eq!( + ctx.session.projection_cache.read().unwrap().len(), + 1, + "deterministic_choice must derive the combat lookahead from the config \ + (revert-failing: passing the disabled variant there leaves this cache empty, and \ + the combat_ai.rs call-site tests structurally cannot see that mistake)" + ); + + // Negative sibling, on the SAME state. + let medium = create_config(AiDifficulty::Medium, Platform::Native).into_measurement(7); + assert!( + !medium.combat_lookahead, + "T8 negative: Medium must NOT enable combat lookahead" + ); + let medium_ctx = crate::context::AiContext::empty(&medium.weights); + let _ = deterministic_choice(&state, PlayerId(0), &medium, &[], Some(&medium_ctx)); + assert!( + medium_ctx + .session + .projection_cache + .read() + .unwrap() + .is_empty(), + "with combat_lookahead off, no projection is taken and the cache stays empty" + ); + } + #[test] fn prospective_fetch_choice_survives_to_the_real_search_prompt() { let db = CardDatabase::from_export( diff --git a/crates/phase-ai/src/session.rs b/crates/phase-ai/src/session.rs index 06dec298c1..4a5affe6f4 100644 --- a/crates/phase-ai/src/session.rs +++ b/crates/phase-ai/src/session.rs @@ -18,6 +18,7 @@ use engine::game::DeckEntry; use engine::types::actions::GameAction; use engine::types::game_state::GameState; use engine::types::player::PlayerId; +use engine::util::Deadline; use crate::deck_profile::DeckProfile; use crate::features::DeckFeatures; @@ -228,12 +229,19 @@ impl AiSession { /// Retrieve a cached projection, computing it on miss. Turn-scoped /// key means stale entries never match. Read-path is lock-free; /// write-path briefly acquires a write lock. + /// + /// `deadline` bounds only the cache-miss computation; a cache HIT is + /// returned regardless of expiry. Callers holding an `AiConfig` must pass + /// `projection::projection_deadline(config.execution_mode)` — evaluated + /// inline at the call, never hoisted into a `let` that spans multiple + /// projections. pub fn get_or_project( &self, base: &GameState, ai_player: PlayerId, target_opponent: PlayerId, horizon: ProjectionHorizon, + deadline: Deadline, ) -> Result, BailReason> { let key = ProjectionKey { state_hash: quick_state_hash(base), @@ -250,7 +258,13 @@ impl AiSession { } } - let projection = Arc::new(project_to(base, ai_player, target_opponent, horizon)?); + let projection = Arc::new(project_to( + base, + ai_player, + target_opponent, + horizon, + deadline, + )?); if let Ok(mut cache) = self.projection_cache.write() { cache.insert(key, Arc::clone(&projection)); @@ -383,6 +397,9 @@ mod tests { use engine::types::card_type::{CardType, CoreType}; use engine::types::game_state::{GameState, PersistedGameState, PlayerDeckPool, WaitingFor}; use engine::types::identifiers::ObjectId; + use engine::util::Deadline; + + use crate::projection::BailReason; use engine::types::player::PlayerId; use engine::types::statics::StaticMode; use std::sync::Arc; @@ -552,6 +569,7 @@ mod tests { PlayerId(0), PlayerId(1), ProjectionHorizon::OpponentAttackersDeclared, + Deadline::none(), ) .unwrap(); let b = session @@ -560,6 +578,7 @@ mod tests { PlayerId(0), PlayerId(1), ProjectionHorizon::OpponentAttackersDeclared, + Deadline::none(), ) .unwrap(); assert!( @@ -581,6 +600,7 @@ mod tests { PlayerId(1), PlayerId(1), ProjectionHorizon::OpponentAttackersDeclared, + Deadline::none(), ) .unwrap(); assert!( @@ -594,6 +614,124 @@ mod tests { ); } + /// T3 — the deadline reaches `project_to` THROUGH the cache wrapper, and a + /// bail is not cached. + /// + /// Two arms on the same full-loop fixture. Arm 1 is not optional garnish: + /// without it, arm 2's `len() == 0` cannot distinguish "a bail is not + /// cached" from "nothing ever caches on this fixture." + #[test] + fn get_or_project_forwards_deadline_and_does_not_cache_a_bail() { + let state = crate::projection::projection_fixtures::opponent_turn_precombat_fixture(); + crate::projection::projection_fixtures::assert_traverses_to( + &state, + PlayerId(0), + PlayerId(1), + ProjectionHorizon::OpponentAttackersDeclared, + ); + + // Arm 1 — a non-expiring deadline forwards through the wrapper, the + // projection completes, and the result is cached. + let session = AiSession::empty(); + let ok = session.get_or_project( + &state, + PlayerId(0), + PlayerId(1), + ProjectionHorizon::OpponentAttackersDeclared, + Deadline::none(), + ); + assert!( + ok.is_ok(), + "arm 1: a non-expiring deadline must let the wrapped projection complete; got {:?}", + ok.err() + ); + assert_eq!( + session.projection_cache.read().unwrap().len(), + 1, + "arm 1: a successful projection must be cached" + ); + + // Arm 2 — a fresh session with a pre-expired deadline: the wrapper must + // forward it (so the bail is the wall-clock one) and must not cache it. + let bailing = AiSession::empty(); + let err = bailing.get_or_project( + &state, + PlayerId(0), + PlayerId(1), + ProjectionHorizon::OpponentAttackersDeclared, + Deadline::after(0), + ); + assert!( + matches!(err, Err(BailReason::TimeCapExceeded { .. })), + "arm 2: get_or_project must FORWARD its deadline to project_to — ignoring the \ + parameter leaves the projection to complete; got {err:?}" + ); + assert_eq!( + bailing.projection_cache.read().unwrap().len(), + 0, + "arm 2: a bail must not be cached" + ); + } + + /// T4 — multi-authority hostile fixture: the cache and the deadline both + /// govern "should this call do work", and on a HIT the cache wins. + /// + /// Fails only for one specific wrong implementation — adding an + /// `if deadline.expired() { return Err(..) }` check ahead of the cache read + /// in `get_or_project` — which passes every other test in this change. + /// Deliberately uses the already-at-horizon fixture class, the opposite of + /// T3's, so the first call is deterministic and free. + #[test] + fn get_or_project_serves_cache_hit_under_expired_deadline() { + let mut s = GameState::new_two_player(42); + s.turn_number = 2; + s.active_player = PlayerId(1); + s.creatures_attacked_this_turn.insert(ObjectId(1)); + s.stack.clear(); + s.waiting_for = WaitingFor::Priority { + player: PlayerId(1), + }; + crate::projection::projection_fixtures::assert_already_at_horizon( + &s, + PlayerId(0), + PlayerId(1), + ProjectionHorizon::OpponentAttackersDeclared, + ); + + let session = AiSession::empty(); + let first = session + .get_or_project( + &s, + PlayerId(0), + PlayerId(1), + ProjectionHorizon::OpponentAttackersDeclared, + Deadline::none(), + ) + .expect("the already-at-horizon fixture must project"); + assert_eq!( + session.projection_cache.read().unwrap().len(), + 1, + "the first call must populate the cache" + ); + + let second = session + .get_or_project( + &s, + PlayerId(0), + PlayerId(1), + ProjectionHorizon::OpponentAttackersDeclared, + Deadline::after(0), + ) + .expect( + "a cache HIT must be served regardless of expiry — the deadline gates \ + computation, never lookup", + ); + assert!( + Arc::ptr_eq(&first, &second), + "the expired-deadline call must return the cached Arc, not recompute" + ); + } + #[test] fn bracket_tier_propagates_through_load_deck_into_state() { use engine::game::bracket_estimate::CommanderBracketTier; From 4f45f94c388da489abb61157b74f50e18ab6f0a2 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Mon, 3 Aug 2026 15:25:59 -0700 Subject: [PATCH 2/3] test(ai): make the evasion projection deadline argument discriminating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `/review-impl` mutation pass found T7 (`velocity_score_takes_projection_under_measurement_config`) had zero sensitivity to the deadline argument it was meant to guard at `evasion_removal_priority.rs:174`. All three wrong forms — `ctx.context.deadline`, `Deadline::after(15)`, and `Deadline::after(0)` — left the whole suite green. Cause: T7 uses an already-at-`OpponentBeginCombat` fixture, so `project_to` returns from the `Confidence::Exact` short-circuit before the loop's only `deadline.expired()` read. The deadline was structurally inert there for every possible value — the fixture is reachable but not discriminating. This mattered more than its severity: the evasion seam is the one reachable at the gate's default Medium difficulty and is the stated reason the change exists, yet it was the only production call site without regression protection. The CEDH-only combat seam was already covered by T8. Adds `seed_opponent_begin_combat_horizon` to `projection_fixtures` — a fixture that genuinely traverses to the horizon rather than starting at it. Reaching `OpponentBeginCombat` by traversal requires a begin-combat trigger on the projected opponent's board, because `auto_advance_once` opens a priority window in that phase only when a trigger fires; otherwise it advances past to DeclareAttackers or CR-508.8-skips to PostCombatMain. `assert_begin_combat_trigger_parsed` guards that dependency so engine drift surfaces as a named panic rather than a silently vacuous pass. All three mutations watched RED and reverted byte-exactly. T7 is unchanged and still guards the already-at-horizon path and the cache interaction. --- .../src/policies/evasion_removal_priority.rs | 213 +++++++++++++++++- crates/phase-ai/src/projection.rs | 135 ++++++++++- 2 files changed, 336 insertions(+), 12 deletions(-) diff --git a/crates/phase-ai/src/policies/evasion_removal_priority.rs b/crates/phase-ai/src/policies/evasion_removal_priority.rs index 5c386420c7..207ccac2dc 100644 --- a/crates/phase-ai/src/policies/evasion_removal_priority.rs +++ b/crates/phase-ai/src/policies/evasion_removal_priority.rs @@ -277,6 +277,29 @@ mod tests { } } + /// The `PolicyContext` the scoring helpers below run under. Exposed + /// separately so a test can also interrogate the context's own gates (e.g. + /// `can_afford_projection`) on the exact shape production would see, rather + /// than re-deriving the answer from config fields. + fn policy_ctx<'a>( + state: &'a GameState, + decision: &'a AiDecisionContext, + candidate: &'a CandidateAction, + config: &'a AiConfig, + context: &'a crate::context::AiContext, + ) -> PolicyContext<'a> { + PolicyContext { + state, + decision, + candidate, + ai_player: P0, + config, + context, + cast_facts: None, + search_depth: crate::policies::context::SearchDepth::Root, + } + } + /// Score with a CALLER-OWNED `AiContext`, so the test retains a handle on /// `context.session` (to inspect `projection_cache`) and on /// `context.deadline` (to inject a pre-expired budget). Mirrors @@ -290,17 +313,8 @@ mod tests { context: &crate::context::AiContext, ) -> f64 { let candidate = candidate_for(target); - let ctx = PolicyContext { - state, - decision, - candidate: &candidate, - ai_player: P0, - config, - context, - cast_facts: None, - search_depth: crate::policies::context::SearchDepth::Root, - }; - EvasionRemovalPriorityPolicy.score(&ctx) + EvasionRemovalPriorityPolicy + .score(&policy_ctx(state, decision, &candidate, config, context)) } fn policy_score( @@ -725,6 +739,183 @@ mod tests { ); } + /// T7b — the evasion production wiring passes an EXECUTION-MODE-DERIVED + /// deadline to `get_or_project`, on a fixture where that argument's value + /// decides the outcome. + /// + /// T7 above cannot see that argument at all, and adding an assertion there + /// would not help: its fixture is deliberately already at + /// `OpponentBeginCombat`, so `project_to` returns from the + /// `Confidence::Exact` short-circuit BEFORE the loop's only + /// `deadline.expired()` read. The deadline is structurally inert there, so + /// every possible value of that argument leaves T7 green. This test is the + /// traversing sibling; keep both, since T7 still guards the + /// already-at-horizon path and the cache interaction that this fixture does + /// not exercise. + /// + /// Both arms share one fixture and one target, and each is revert-failing + /// for a different wrong argument: + /// * measurement arm — `projection_deadline` yields `Deadline::none()`, + /// the traversal completes and the projection is cached. Red for + /// `Deadline::after(0)` (bails at the loop head) and for the pre-change + /// hardcoded `Deadline::after(TIME_CAP_MS)` (this traversal costs + /// several times the 15 ms cap), both of which leave the cache empty. + /// * interactive arm — the SAME fixture under `ExecutionMode::Interactive` + /// must NOT complete: the 15 ms cap bails it and the cache stays empty. + /// Red for passing `ctx.context.deadline` instead, which is + /// `Deadline::none()` here — and in production is the planner's + /// whole-turn budget — so the projection would complete and cache one + /// entry. That substitution is invisible under measurement alone, + /// because measurement mode installs `Deadline::none()` on the context + /// too; only the interactive side can discriminate it. + /// + /// The interactive arm is therefore the one assertion here that compares a + /// real elapsed traversal against a wall-clock cap. It is written as a + /// negative (`is_empty`) so a drifted-faster traversal fails LOUDLY with a + /// populated cache rather than passing vacuously, and the fixture's measured + /// traversal is several times the cap in a debug build, which is the only + /// build `cargo test` produces. + #[test] + fn velocity_score_projection_deadline_is_live_on_a_traversing_fixture() { + let mut scenario = GameScenario::new_n_player(2, 42); + scenario.at_phase(Phase::PreCombatMain); + // The grower is both the reachability mechanism for the + // `OpponentBeginCombat` horizon and the removal target under test: a + // creature that grows before the opponent's combat is precisely what + // `velocity_score` exists to prioritize. + let grower = crate::projection::projection_fixtures::seed_opponent_begin_combat_horizon( + &mut scenario, + P0, + PlayerId(1), + ); + let bolt = scenario + .add_spell_to_hand_from_oracle(P0, "Lightning Bolt", true, LIGHTNING_BOLT_ORACLE) + .with_mana_cost(ManaCost::Cost { + shards: vec![ManaCostShard::Red], + generic: 0, + }) + .id(); + scenario.with_mana_pool( + P0, + vec![ManaUnit::new(ManaType::Red, ObjectId(0), false, vec![])], + ); + + let mut runner = scenario.build(); + let card_id = runner.state().objects[&bolt].card_id; + runner + .act(GameAction::CastSpell { + object_id: bolt, + card_id, + targets: Vec::new(), + payment_mode: CastPaymentMode::Auto, + }) + .expect("the real Lightning Bolt fixture should reach target selection"); + + // No horizon mutation here: unlike T7, this state is used exactly as the + // engine produced it, so `decision` and `state` cannot disagree. + let state = runner.state().clone(); + let decision = build_decision_context(&state); + assert!( + matches!(&decision.waiting_for, WaitingFor::TargetSelection { .. }), + "T7b fixture: the policy's first gate needs a live TargetSelection" + ); + + // Guard 1 — the begin-combat trigger still parses. Without it the + // horizon is unreachable from any state, and guard 2 would report a + // confusing GameOverDuringProjection instead. + crate::projection::projection_fixtures::assert_begin_combat_trigger_parsed(&state, grower); + + // Guard 2 — and the state genuinely TRAVERSES `project_to`'s loop to + // the horizon `velocity_score` hardcodes. A `Confidence::Exact` result + // here would mean the fixture had drifted into the short-circuit class, + // which is what makes T7 blind to the deadline; both arms below would + // then be vacuous. + crate::projection::projection_fixtures::assert_traverses_to( + &state, + P0, + PlayerId(1), + crate::projection::ProjectionHorizon::OpponentBeginCombat, + ); + + let measurement = create_config(AiDifficulty::Medium, Platform::Native).into_measurement(7); + + // Control — the policy's non-velocity gates pass on THIS fixture. + // `Deadline::after(0)` is expired, so `can_afford_projection` is false + // and `velocity_score` returns before `get_or_project`. A non-zero total + // therefore proves the other gates accept the fixture, so a red positive + // arm below is a wiring failure and not a fixture failure. + let mut expired_ctx = crate::context::AiContext::empty(&measurement.weights); + expired_ctx.deadline = engine::util::Deadline::after(0); + let control = + policy_score_in_context(&state, &decision, grower, &measurement, &expired_ctx); + assert_ne!( + control, 0.0, + "T7b control: EvasionRemovalPriorityPolicy::score must reach velocity_score on this \ + fixture — a 0.0 means one of its gates rejected it. Stop and report; do not weaken \ + the arms below." + ); + assert!( + expired_ctx + .session + .projection_cache + .read() + .unwrap() + .is_empty(), + "T7b control: with can_afford_projection() false, velocity_score must not project" + ); + + // Measurement arm — kills `Deadline::after(0)` and `Deadline::after(15)`. + let ai_ctx = crate::context::AiContext::empty(&measurement.weights); + let _ = policy_score_in_context(&state, &decision, grower, &measurement, &ai_ctx); + assert_eq!( + ai_ctx.session.projection_cache.read().unwrap().len(), + 1, + "under a measurement config the projection deadline must not expire, so this \ + traversal completes and caches (revert-failing: any finite budget passed here bails \ + a traversal that costs several times the 15 ms interactive cap)" + ); + + // Interactive arm — kills `ctx.context.deadline`. + let interactive = create_config(AiDifficulty::Medium, Platform::Native); + assert!( + !interactive.execution_mode.is_measurement(), + "T7b interactive arm: create_config must default to ExecutionMode::Interactive" + ); + let interactive_ctx = crate::context::AiContext::empty(&interactive.weights); + // Non-vacuity guard: the arm is only meaningful if the policy actually + // REACHES `get_or_project` in this context. `AiContext::empty` carries + // `Deadline::none()`, so `can_afford_projection`'s `is_none_or` floor + // bypass applies despite Medium's 2000 ms `projection_min_budget_ms`. + // Were that false, the cache would be empty because nothing projected, + // and the assertion below would pass no matter what deadline the + // production line passes. + let candidate = candidate_for(grower); + assert!( + policy_ctx( + &state, + &decision, + &candidate, + &interactive, + &interactive_ctx + ) + .can_afford_projection(), + "T7b interactive arm would be vacuous: velocity_score never reaches get_or_project \ + because can_afford_projection() is false" + ); + let _ = policy_score_in_context(&state, &decision, grower, &interactive, &interactive_ctx); + assert!( + interactive_ctx + .session + .projection_cache + .read() + .unwrap() + .is_empty(), + "under an interactive config the 15 ms projection cap must bail this traversal, so \ + nothing is cached (revert-failing: passing ctx.context.deadline here hands the \ + projection the caller's whole-turn budget and it completes)" + ); + } + #[test] fn activated_removal_weights_controller_threat_but_beneficial_activation_is_neutral() { let destroy = Effect::Destroy { diff --git a/crates/phase-ai/src/projection.rs b/crates/phase-ai/src/projection.rs index e388463bd8..69a1bf713b 100644 --- a/crates/phase-ai/src/projection.rs +++ b/crates/phase-ai/src/projection.rs @@ -716,13 +716,15 @@ pub fn threat_velocity( /// ran rather than the short-circuit. /// /// Lives beside `project_to` because the invariant these states encode ("this -/// state traverses the loop to `OpponentAttackersDeclared`") is a property of +/// state traverses the loop to its requested horizon") is a property of /// `project_to`'s own resolution policy, not of any consumer. #[cfg(test)] pub(crate) mod projection_fixtures { use super::*; + use engine::game::scenario::GameScenario; use engine::game::zones::create_object; use engine::types::identifiers::CardId; + use engine::types::triggers::TriggerMode; use engine::types::zones::Zone; /// A battlefield creature that can attack on the turn after it was placed: @@ -936,6 +938,137 @@ pub(crate) mod projection_fixtures { } } + /// Oracle text of the permanent that makes + /// `ProjectionHorizon::OpponentBeginCombat` reachable BY TRAVERSAL. + /// + /// Load-bearing, for a structural reason. The engine never RESTS at + /// `Phase::BeginCombat` holding `WaitingFor::Priority` unless a begin-combat + /// trigger fires: `auto_advance_once`'s `Phase::BeginCombat` arm + /// (`crates/engine/src/game/turns.rs`) opens a priority window only when + /// `process_phase_triggers` reports `triggers_fired`. With no trigger it + /// either advances straight to `DeclareAttackers` (when + /// `combat::has_potential_attackers`) or, per CR 508.8, enters + /// `PostCombatMain` — and neither path stops on `reached_horizon`'s + /// `OpponentBeginCombat` predicate (active player is the target opponent + + /// `Phase::BeginCombat` + empty stack + `Priority` held by the target + /// opponent). Measured, not assumed: with this permanent replaced by a + /// vanilla creature the traversal never stops at the horizon — it runs on + /// past the opponent's combat until a library empties and returns + /// `BailReason::GameOverDuringProjection`. + /// + /// With the trigger the traversal ends this way: the trigger fires and goes + /// on the stack, so the opponent's first `Priority` window fails the + /// empty-stack conjunct; both players pass; the trigger resolves; CR 117.3b + /// returns priority to the active player with an empty stack, which is the + /// horizon. + /// + /// The growth clause is not decoration either. It is exactly the + /// `threat_velocity` signal `policies::evasion_removal_priority:: + /// velocity_score` exists to read, so the projected board differs from the + /// base board the way a production Ouroboroid-class board does. + const BEGIN_COMBAT_GROWTH_ORACLE: &str = + "At the beginning of combat on your turn, put a +1/+1 counter on this creature."; + + /// Seed `scenario` so that a state built from it TRAVERSES `project_to`'s + /// loop to `ProjectionHorizon::OpponentBeginCombat` for + /// `(ai_player, target_opponent)`. Returns the growing permanent's id, which + /// doubles as a removal target whose threat genuinely grows before the + /// projected combat. + /// + /// Unlike Fixtures A and B this is a seeder rather than a whole `GameState`, + /// because its only consumer needs a state that ALSO satisfies a tactical + /// policy's own gates (a real pending cast at `WaitingFor::TargetSelection`). + /// Card-specific setup stays with the policy test; the projection-reachability + /// knowledge stays here. + /// + /// Preconditions the CALLER owns — the ordinary `GameScenario::at_phase` + /// recipe, not extra ceremony: + /// * `ai_player` is the active player and holds priority, and + /// * the scenario sits at `Phase::PreCombatMain` on `ai_player`'s turn, + /// which is where the production evasion policy scores removal targets. + /// + /// What this adds, and why each piece is required: + /// * `target_opponent`'s begin-combat trigger permanent — see + /// [`BEGIN_COMBAT_GROWTH_ORACLE`]; without it the horizon is unreachable + /// from any state at all, not merely awkward to reach. + /// * an untapped, non-summoning-sick attacker for `ai_player`, so the + /// traversal necessarily dispatches one `WaitingFor::DeclareAttackers` + /// choice — `pick_empty_attackers`, since `acting != target_opponent`. + /// That action is not `PassPriority`, so `choice_count >= 1` and the + /// result is `Confidence::Approximated`: the typed witness + /// [`assert_traverses_to`] checks, and the one the already-at-horizon + /// short-circuit structurally cannot emit. + /// * five cards in each library. The traversal crosses into + /// `target_opponent`'s turn, and CR 504.1 has its active player draw a + /// card in the draw step, which precedes the combat phase (CR 500.1 + /// phase order). An empty library there loses the game (CR 704.5b) and + /// the projection returns `BailReason::GameOverDuringProjection` instead + /// of reaching the horizon. + pub(crate) fn seed_opponent_begin_combat_horizon( + scenario: &mut GameScenario, + ai_player: PlayerId, + target_opponent: PlayerId, + ) -> ObjectId { + let grower = scenario + .add_creature_from_oracle( + target_opponent, + "Projection Grower", + 2, + 2, + BEGIN_COMBAT_GROWTH_ORACLE, + ) + .id(); + scenario.add_creature(ai_player, "Projection Bear", 2, 2); + scenario.with_library_top( + ai_player, + &[ + "AI Filler 0", + "AI Filler 1", + "AI Filler 2", + "AI Filler 3", + "AI Filler 4", + ], + ); + scenario.with_library_top( + target_opponent, + &[ + "Opp Filler 0", + "Opp Filler 1", + "Opp Filler 2", + "Opp Filler 3", + "Opp Filler 4", + ], + ); + grower + } + + /// Fail loudly if `permanent` no longer carries the parsed begin-combat + /// trigger that [`seed_opponent_begin_combat_horizon`]'s traversal depends + /// on. + /// + /// Cheaper and far more specific than waiting for the traversal to fail: + /// parser drift on [`BEGIN_COMBAT_GROWTH_ORACLE`] would otherwise surface + /// several hundred dispatches later as a + /// `BailReason::GameOverDuringProjection`, which reads like an unrelated + /// library-stocking defect. + pub(crate) fn assert_begin_combat_trigger_parsed(state: &GameState, permanent: ObjectId) { + let object = state + .objects + .get(&permanent) + .expect("FIXTURE DEFECT: the seeded begin-combat permanent no longer exists"); + assert!( + object.base_trigger_definitions.iter().any(|trigger| { + matches!(trigger.mode, TriggerMode::Phase) + && trigger.phase == Some(Phase::BeginCombat) + }), + "FIXTURE DEFECT, not a wiring defect: {} no longer parses a TriggerMode::Phase \ + trigger on Phase::BeginCombat, so auto_advance_once's BeginCombat arm will not \ + open the priority window the OpponentBeginCombat horizon needs and the traversal \ + cannot reach it. Re-derive the fixture's Oracle text; do not weaken the guard.", + object.name + ); + } + /// The already-at-horizon counterpart: assert the state short-circuits, so /// the determinism claim of a fixture in that class is checked rather than /// assumed. From 7cd269b18372cef58c075fbf5b2733ab8231391d Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 4 Aug 2026 12:19:07 -0700 Subject: [PATCH 3/3] perf(ai): unify all AI gate profiles on server-release Both PR-facing AI gates died at their 60-minute timeout on every run. Optimized, one perf-gate sample completes in <=25s on an M-series Mac and the parent gate is PERF_SAMPLE_COUNT=5 of those, so ~2 minutes. No precise speedup ratio is quoted here on purpose. Every timing run available while preparing this change was taken on a machine running concurrent builds, and wall clock under load is not a measurement. The 25s figure is therefore an upper bound, which is the only direction that is safe to assert. The load-INDEPENDENT evidence is the counter data recorded on the PR; that is what this change actually rests on. Profile authority was split four ways, and two of the four were not merely unoptimized but actively wrong for a native wall-clock gate: * .cargo/config.toml aliases (what CI runs) -> dev, opt-level 0 * scripts/ai-gate.sh -> --release * scripts/ai-perf-gate.sh -> --release * scripts/validate-ai-perf-reproducibility.sh -> dev, deliberately `--release` in this workspace is the WASM-SIZE profile: opt-level 'z', lto = true, codegen-units = 1, panic = 'abort'. It optimizes for binary size, builds slowly, and aborts rather than unwinds. All four now name `server-release` (opt-level 2, thin LTO, codegen-units 16, panic = 'unwind'). refresh-ai-baseline.sh and refresh-ai-perf-baseline.sh delegate to the two wrappers and inherit the profile automatically. They must move as a chain, not one at a time: ai-perf-gate re-spawns ITSELF via current_exe() for each cold-process trial, so the child inherits the parent binary's profile while the scripts hardcode a target// path. Profile skew between the alias and a script yields a silently stale or missing binary, not an error. The reproducibility script's comment recorded this invariant as its reason for pinning debug; the invariant is preserved, only the profile name changes. The shared `cache-shared-key: rust-ai-gate` stays coherent because every job in the workflow moves together -- win-rate jobs populate the cache, perf jobs reuse it. Expect one cold build on the first run after this lands. Counter VALUES are logical event counts and profile-independent, so this does not by itself invalidate a baseline. No baseline refreshed here. Context: #6967 --- .cargo/config.toml | 19 ++++++++++++-- .github/workflows/ai-gate.yml | 18 ++++++++----- scripts/ai-gate.sh | 6 +++-- scripts/ai-perf-gate.sh | 16 +++++++----- scripts/validate-ai-perf-reproducibility.sh | 28 +++++++++++++-------- 5 files changed, 60 insertions(+), 27 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index afba2f26b5..ae7aa1436c 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -13,8 +13,23 @@ semantic-audit = "run --profile tool --features cli --bin oracle-gen -- semantic combo-verify = "run --profile tool --features combo-verify --bin combo-verify -- data/" scrape-feeds = "run --release -p feed-scraper --" tune-ai = "run --release --features tune --bin ai-tune --" -ai-gate = "run --bin ai-gate --" -ai-perf-gate = "run --bin ai-perf-gate --" +# AI gates run `server-release`, NOT the default dev profile and NOT `--release`. +# * dev (opt-level 0) made the PR gates exceed their 60-minute timeout on every +# run; the suite is ~14x faster optimized. +# * `--release` in this workspace is the WASM-SIZE profile (opt-level 'z', +# lto = true, codegen-units = 1, panic = 'abort') — size-optimized, slow to +# build, and the wrong shape for a native wall-clock gate. +# `server-release` is the native speed profile (opt-level 2, thin LTO, +# codegen-units 16, panic = 'unwind'). +# +# These two aliases, scripts/ai-gate.sh, scripts/ai-perf-gate.sh and +# scripts/validate-ai-perf-reproducibility.sh must ALL name the same profile. +# ai-perf-gate re-spawns itself via current_exe() for each cold-process trial, so +# a parent built under one profile would spawn children under it while a script +# reads a different target// path — profile skew there yields confidently +# wrong numbers rather than an error. +ai-gate = "run --profile server-release --bin ai-gate --" +ai-perf-gate = "run --profile server-release --bin ai-perf-gate --" engine-inventory = "run --quiet -p engine-inventory-gen" [env] diff --git a/.github/workflows/ai-gate.yml b/.github/workflows/ai-gate.yml index 81eeb0a627..1eab55ebfa 100644 --- a/.github/workflows/ai-gate.yml +++ b/.github/workflows/ai-gate.yml @@ -135,9 +135,12 @@ jobs: run: | cargo run --profile tool --features cli --bin oracle-gen -- data/ --stats --names-out data/card-names.json > data/card-data.json - # debug profile (authoritative): counter VALUES are profile-independent and the - # shared rust-ai-gate cache is debug-warm (win-rate jobs populate it). Runs - # PERF_SAMPLE_COUNT independent sample processes; compares the per-counter median (#4878). + # server-release profile (authoritative, set by the `cargo ai-perf-gate` alias): + # counter VALUES are profile-independent, and the shared rust-ai-gate cache stays + # coherent because every job in this workflow builds the same profile — the + # win-rate jobs populate it and the perf jobs reuse it. Expect one cold build on + # the first run after the dev -> server-release move. + # Runs PERF_SAMPLE_COUNT independent sample processes; compares the per-counter median (#4878). - name: Run decision-cost perf gate run: cargo ai-perf-gate @@ -162,9 +165,12 @@ jobs: run: | cargo run --profile tool --features cli --bin oracle-gen -- data/ --stats --names-out data/card-names.json > data/card-data.json - # debug profile (authoritative): counter VALUES are profile-independent and the - # shared rust-ai-gate cache is debug-warm (win-rate jobs populate it). Runs - # PERF_SAMPLE_COUNT independent sample processes; compares the per-counter median (#4878). + # server-release profile (authoritative, set by the `cargo ai-perf-gate` alias): + # counter VALUES are profile-independent, and the shared rust-ai-gate cache stays + # coherent because every job in this workflow builds the same profile — the + # win-rate jobs populate it and the perf jobs reuse it. Expect one cold build on + # the first run after the dev -> server-release move. + # Runs PERF_SAMPLE_COUNT independent sample processes; compares the per-counter median (#4878). # `cargo` build progress goes to stderr and spawned children are Stdio::null on # stdout, so the redirect captures only the binary's clean markdown table. - name: Run decision-cost perf gate diff --git a/scripts/ai-gate.sh b/scripts/ai-gate.sh index 1379f7cec5..4e56a9ebb6 100755 --- a/scripts/ai-gate.sh +++ b/scripts/ai-gate.sh @@ -14,5 +14,7 @@ set -euo pipefail ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" export CARGO_TARGET_DIR="$ROOT/target/ai" -cargo build --release --bin ai-gate -exec "$CARGO_TARGET_DIR/release/ai-gate" "$@" +# server-release, matching the `cargo ai-gate` alias CI runs. NOT `--release`: +# that is this workspace's WASM-size profile (opt-level 'z', panic = 'abort'). +cargo build --profile server-release --bin ai-gate +exec "$CARGO_TARGET_DIR/server-release/ai-gate" "$@" diff --git a/scripts/ai-perf-gate.sh b/scripts/ai-perf-gate.sh index d707680689..adf51e7310 100755 --- a/scripts/ai-perf-gate.sh +++ b/scripts/ai-perf-gate.sh @@ -6,10 +6,14 @@ # perf gate its own build lock and fingerprint namespace so it never blocks on # (or thrashes against) Tilt's shared target/debug builds. # -# NOTE: this wrapper builds --release; its WALL-CLOCK is NOT comparable to CI, -# which runs the DEBUG profile via `cargo ai-perf-gate`. Counter VERDICTS are -# profile-independent (logical event counts), so the wrapper's PASS/FAIL is -# correct locally — only its timing must not be transferred to the CI budget. +# This wrapper and CI (`cargo ai-perf-gate`) now build the SAME profile, +# server-release, so wall-clock here is comparable to CI's. Counter VERDICTS were +# already profile-independent (logical event counts); unifying the profile makes +# the TIMING transferable too. +# +# Previously this built `--release` while CI built dev — two different profiles, +# neither of them the native speed one, and `--release` in this workspace is the +# WASM-size profile (opt-level 'z', panic = 'abort'). # # Usage: scripts/ai-perf-gate.sh [ai-perf-gate args...] # scripts/ai-perf-gate.sh # compare against the saved baseline @@ -19,5 +23,5 @@ set -euo pipefail ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" export CARGO_TARGET_DIR="$ROOT/target/ai" -cargo build --release --bin ai-perf-gate -exec "$CARGO_TARGET_DIR/release/ai-perf-gate" "$@" +cargo build --profile server-release --bin ai-perf-gate +exec "$CARGO_TARGET_DIR/server-release/ai-perf-gate" "$@" diff --git a/scripts/validate-ai-perf-reproducibility.sh b/scripts/validate-ai-perf-reproducibility.sh index bae19651b5..cf7394c460 100755 --- a/scripts/validate-ai-perf-reproducibility.sh +++ b/scripts/validate-ai-perf-reproducibility.sh @@ -7,10 +7,15 @@ # TIMES the cold build and every gate run so the executor can apply the CI-budget # check with MEASURED numbers rather than asserted estimates. # -# Runs the DEBUG binary — the authoritative gate profile CI runs -# (`cargo ai-perf-gate`). Under debug, the parent's current_exe() resolves to -# target/debug/ai-perf-gate, so the K spawned children are debug too -# (profile-consistent parent and children). +# Runs the SERVER-RELEASE binary — the authoritative gate profile CI runs +# (`cargo ai-perf-gate`). The parent's current_exe() resolves to +# target/server-release/ai-perf-gate, so the K spawned children are +# server-release too (profile-consistent parent and children). +# +# The profile here is load-bearing and must track the `cargo ai-perf-gate` alias: +# this script hardcodes a target// path while the binary re-spawns ITSELF +# by current_exe(). If the alias and this path ever name different profiles, the +# script silently measures the wrong binary (or a stale one) instead of failing. # # ONLY commit the generated baseline if this script PASSES (margin + all N band # runs exit 0) AND the executor's CI-budget arithmetic passes (see the echoed @@ -19,13 +24,13 @@ set -euo pipefail ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" export CARGO_TARGET_DIR="$ROOT/target/ai" # isolated: no Tilt lock contention (mirrors ai-perf-gate.sh) -# DEBUG profile — the authoritative gate profile CI runs (`cargo ai-perf-gate`). +# SERVER-RELEASE profile — the authoritative gate profile CI runs (`cargo ai-perf-gate`). # Time the cold isolated build: cold-isolated >= CI's warm rust-ai-gate cache # hit, so this is a conservative T_build ceiling for the budget check. build_start=$(date +%s) -cargo build --bin ai-perf-gate # BUILD ONCE (debug, default profile) -echo "T_build (cold isolated debug build) = $(( $(date +%s) - build_start ))s" -BIN="$CARGO_TARGET_DIR/debug/ai-perf-gate" # current_exe() -> debug children (profile-consistent) +cargo build --profile server-release --bin ai-perf-gate +echo "T_build (cold isolated server-release build) = $(( $(date +%s) - build_start ))s" +BIN="$CARGO_TARGET_DIR/server-release/ai-perf-gate" # current_exe() -> server-release children "$BIN" --refresh-baseline # 1) generate the median-of-K baseline @@ -49,6 +54,7 @@ if [ "$band_fail" -ne 0 ] || [ "$margin_rc" -ne 0 ]; then exit 1 fi echo "REPRO VALIDATION PASSED (margin+band) — now apply the CI-budget check before committing:" -echo " T_run_max = max over 'run i wall' above; W_debug = T_run_max / PERF_SAMPLE_COUNT(5)." -echo " Option (c) commit iff T_run_max*2.5 + T_build < 25min (see plan 3.4)." -echo " Else fall back to option (b): release + cache-shared-key rust-ai-perf-release; re-measure." +echo " T_run_max = max over 'run i wall' above; W_run = T_run_max / PERF_SAMPLE_COUNT(5)." +echo " Commit iff T_run_max*2.5 + T_build < 25min." +echo " (The former 'fall back to --release' option is gone: all four gate" +echo " authorities now build server-release, and --release is the WASM-size profile.)"