From 3d862a693dea0d4b8cdaf4c87752339120ef3329 Mon Sep 17 00:00:00 2001 From: mcbradd <35860549+mcbradd@users.noreply.github.com> Date: Tue, 21 Jul 2026 10:51:54 -0700 Subject: [PATCH 01/11] fix(ai-commander): run games in measurement mode for cross-game batch reproducibility --games-file batch mode diverged from single-game mode: a game that won cleanly run solo stalled at turn 60 when played 3rd in a batch (pod-lab equivalence gate, phase#6252). Root cause is NOT leaked state but the AI search's wall-clock deadline. ai_commander built each seat's AiConfig in ExecutionMode::Interactive with time_budget_ms = AI_SEARCH_TIME_BUDGET_MS (1500ms), so both search paths (search.rs, planner::with_deadline) used Deadline::after(1500ms). A warmer, slower Nth-in-batch process (measured: 74.1s vs 63.4s at an IDENTICAL board state) expired that deadline on a mid-game decision the fresh solo process completed in full, collapsing search to the degraded tactical-only floor and picking a different, sometimes unpayable move -> divergence -> stall. Two equally-fast solo runs never straddle the budget, so they byte-match; only the slower batched run diverges. Fix: build_seat_config applies .into_measurement(seed), forcing ExecutionMode::Measurement -> Deadline::none() (both search paths gate on is_measurement) -> search bounded solely by max_nodes/max_depth, a pure function of (seed, difficulty, feed). Mirrors the established duel_suite::run batch harness, whose config uses .into_measurement(seed) for the same documented "eliminate wall-clock flake" reason. Covers single-game mode too (both paths go through play_one_game), so batch game N is bit-identical to the same game solo. Tests: a deterministic unit test asserting every seat runs in measurement mode (fails against the unfixed Interactive code); plus an integration test encoding the repro shape (target game run 3rd in a batch == the same game run alone, byte-identical RESULT block). Co-Authored-By: Claude Fable 5 --- crates/phase-ai/src/bin/ai_commander.rs | 67 ++++- .../tests/ai_commander_batch_equivalence.rs | 243 ++++++++++++++++++ 2 files changed, 306 insertions(+), 4 deletions(-) create mode 100644 crates/phase-ai/tests/ai_commander_batch_equivalence.rs diff --git a/crates/phase-ai/src/bin/ai_commander.rs b/crates/phase-ai/src/bin/ai_commander.rs index 96f438fe75..6b2283306e 100644 --- a/crates/phase-ai/src/bin/ai_commander.rs +++ b/crates/phase-ai/src/bin/ai_commander.rs @@ -426,10 +426,7 @@ fn play_one_game(context: &GameRunContext<'_>, seed: u64, difficulty: AiDifficul for (i, override_diff) in seat_difficulty.iter().enumerate() { let seat_diff = override_diff.unwrap_or(difficulty); println!(" P{i} difficulty={seat_diff:?}"); - ai_configs.insert( - PlayerId(i as u8), - create_config_for_players(seat_diff, Platform::Native, 4), - ); + ai_configs.insert(PlayerId(i as u8), build_seat_config(seat_diff, seed)); } println!(); @@ -874,6 +871,35 @@ fn build_game_state(db: &CardDatabase, payload: &DeckPayload, seed: u64) -> Game state } +/// Builds one seat's `AiConfig` for a single game. Single authority for this +/// bin's per-seat AI configuration — `play_one_game` calls it once per seat and +/// the regression test below asserts its one load-bearing invariant. +/// +/// EVERY seat runs in MEASUREMENT mode (`AiConfig::into_measurement`), which +/// disables the wall-clock search deadline (`AI_SEARCH_TIME_BUDGET_MS`, default +/// 1500ms) so search is bounded SOLELY by `max_nodes`/`max_depth`. This is +/// required for reproducibility, not a benchmarking nicety: an interactive +/// (wall-clock-bounded) search truncates to a degraded best-so-far result the +/// moment `Deadline::expired()` fires, and whether it fires on a given decision +/// depends on how fast the process happens to be running at that instant — NOT +/// on `(seed, difficulty, feed)`. A `--games-file` batch process is measurably +/// slower on its Nth game (warmer allocator, more resident state) than a fresh +/// single-game process, so the SAME game played 3rd in a batch could expire the +/// deadline on a mid-game decision that the solo run completed in full, pick a +/// different move, and diverge — the exact cross-game non-determinism the +/// pod-lab equivalence gate caught (a game that wins solo stalling at turn 60 in +/// batch). Measurement mode makes every decision a pure function of the game's +/// inputs, so batch game N is bit-identical to the same game run solo. This +/// mirrors the established `duel_suite::run` batch harness, which builds its +/// config with `.into_measurement(seed)` for the same "eliminate wall-clock +/// flake" reason. `seed` is the per-game seed, itself fully determined by the +/// game's inputs; the value inside `ExecutionMode::Measurement { seed }` only +/// tags the mode — the search's determinization entropy is derived from game +/// state (`search.rs`), not from this seed. +fn build_seat_config(difficulty: AiDifficulty, seed: u64) -> AiConfig { + create_config_for_players(difficulty, Platform::Native, 4).into_measurement(seed) +} + #[cfg(test)] mod tests { use super::*; @@ -990,6 +1016,39 @@ mod tests { ); } + /// Regression for the cross-game state-leakage defect (pod-lab equivalence + /// gate; PR phase-rs/phase#6252): a game that won solo stalled at turn 60 + /// when played 3rd in a `--games-file` batch. Root cause was NOT a leaked + /// counter/cache but the interactive wall-clock search deadline + /// (`AI_SEARCH_TIME_BUDGET_MS`): a slower Nth-in-batch process expired it on + /// a mid-game decision the fresh solo process completed, diverging the game. + /// `build_seat_config` fixes this by running every seat in MEASUREMENT mode, + /// which disables the wall-clock deadline (search bounded solely by + /// node/depth — see `search.rs` / `planner::PlannerServices::with_deadline`, + /// both gated on `execution_mode.is_measurement()`), making each decision a + /// pure function of the game's inputs. This asserts the invariant + /// deterministically (no card-data / no real game needed): against the + /// unfixed code (`ExecutionMode::Interactive`) it fails, catching any future + /// regression that drops measurement mode and reintroduces wall-clock flake. + #[test] + fn seat_config_runs_in_measurement_mode_for_batch_reproducibility() { + for difficulty in [ + AiDifficulty::Easy, + AiDifficulty::Medium, + AiDifficulty::Hard, + AiDifficulty::VeryHard, + ] { + let config = build_seat_config(difficulty, 95_000_004); + assert!( + config.execution_mode.is_measurement(), + "{difficulty:?} seat must run in measurement mode so a batched \ + game is bit-identical to the same game run solo; interactive \ + mode makes search wall-clock-dependent and non-reproducible \ + under batch load" + ); + } + } + #[test] fn parse_action_cap_accepts_positive_integer() { assert_eq!(parse_action_cap_checked("50000"), Ok(50000)); diff --git a/crates/phase-ai/tests/ai_commander_batch_equivalence.rs b/crates/phase-ai/tests/ai_commander_batch_equivalence.rs new file mode 100644 index 0000000000..ef75157d14 --- /dev/null +++ b/crates/phase-ai/tests/ai_commander_batch_equivalence.rs @@ -0,0 +1,243 @@ +//! Subprocess-level regression tests for `ai-commander`'s `--games-file` batch +//! mode: a batch invocation's per-game output must match the single-game +//! invocation of the same seed+difficulty, byte-for-byte outside the one +//! inherently nondeterministic field (wall-clock elapsed time). These spawn the +//! real `ai-commander` binary rather than calling `run()` in-process, so what is +//! under test is the binary's actual contract with the pod-lab harness — +//! argument parsing, per-game stdout framing, flush timing, process exit — not +//! an internal function's. +//! +//! Every test here is `#[ignore]`d: it loads `client/public/card-data.json` +//! (requires `cargo run --bin card-data-export` or the setup.sh script), which +//! is not available in unit-test CI — the same convention as +//! `greasefang_bounded.rs`/`whitemane_lion_bounded.rs`. Opt in via +//! `cargo test -p phase-ai --test ai_commander_batch_equivalence -- --ignored`. + +use std::path::PathBuf; +use std::process::Command; + +/// Resolves `client/public` the same way `greasefang_bounded.rs` et al. do: a +/// `PHASE_CARDS_PATH` override, else relative to the crate's manifest dir. +fn cards_dir() -> PathBuf { + std::env::var("PHASE_CARDS_PATH") + .map(PathBuf::from) + .unwrap_or_else(|_| { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("..") + .join("..") + .join("client") + .join("public") + }) +} + +/// Small action cap so these tests run quickly while still exercising +/// several turns. The exact outcome (COMPLETED/ABORT/STALL) doesn't matter +/// for equivalence — only that a given seed+feed+cap combination reaches the +/// SAME outcome deterministically, single-game or batched. +const TEST_ACTION_CAP: &str = "300"; + +/// Runs the real `ai-commander` binary with `cards_dir()` as its positional +/// arg and `args` appended, and returns captured stdout as a `String`. Exit +/// code isn't asserted here: COMPLETED (0)/ABORT (2)/STALL (3) are all +/// legitimate outcomes for a bounded-action-cap test game, and every one of +/// them still prints a full `=== RESULT ===` block — `normalized_result_block` +/// panics if that block is missing, which already catches an actual crash. +fn run_ai_commander(args: &[&str]) -> String { + let output = Command::new(env!("CARGO_BIN_EXE_ai-commander")) + .arg(cards_dir()) + .args(args) + .output() + .expect("spawn ai-commander"); + String::from_utf8(output.stdout).expect("stdout is valid UTF-8") +} + +/// Splits `stdout` into one chunk per game. Batch mode anchors on the +/// `--- GAME ` marker: each chunk then runs from one game's own marker up to +/// (not including) the next game's marker or EOF, so it's fully +/// self-contained — critically, a game's marker + per-seat tier echo (all +/// printed BEFORE that game's own "Game started." line) stay attributed to +/// THAT game, not leaked onto the end of the previous one. Single-game mode +/// never prints a marker, so the whole output is one chunk. +fn game_blocks(stdout: &str) -> Vec<&str> { + const MARKER: &str = "--- GAME "; + if !stdout.contains(MARKER) { + return vec![stdout]; + } + let mut starts: Vec = stdout.match_indices(MARKER).map(|(i, _)| i).collect(); + starts.push(stdout.len()); + starts.windows(2).map(|w| &stdout[w[0]..w[1]]).collect() +} + +/// The `=== RESULT ===` epilogue through the end of one game's block (as +/// isolated by `game_blocks`), with the single inherently nondeterministic +/// line (`Elapsed: {:.1}s`, wall-clock) stripped so two separately-timed runs +/// can be compared for equality. +fn normalized_result_block(game_block: &str) -> String { + let start = game_block + .find("=== RESULT ===") + .expect("game block contains a RESULT epilogue"); + game_block[start..] + .lines() + .filter(|line| !line.starts_with("Elapsed:")) + .collect::>() + .join("\n") +} + +/// Higher action cap for the cross-game-leakage regression below: the +/// wall-clock-deadline divergence this guards only surfaces deep into a game +/// (the original repro diverged around turn ~50 / action ~1700), so the tiny +/// `TEST_ACTION_CAP` used by the plumbing tests above would never reach the +/// decisions where an interactive search's budget could straddle. Still bounded +/// so an `#[ignore]` opt-in run stays minutes, not tens of minutes. +const LEAK_REGRESSION_ACTION_CAP: &str = "2000"; + +/// Regression for the cross-game state-leakage defect (pod-lab equivalence +/// gate; PR phase-rs/phase#6252): seed 95000004 won cleanly for P1 run solo but +/// STALLED at turn 60 when played as the 3rd game in a `--games-file` batch +/// (after 2 unrelated games in the same process). Root cause: the AI's +/// interactive search was bounded by a wall-clock deadline +/// (`AI_SEARCH_TIME_BUDGET_MS`), and a warmer/slower Nth-in-batch process +/// expired it on a mid-game decision the fresh solo process completed in full — +/// diverging the game. The fix runs every seat in measurement mode +/// (`build_seat_config`), disabling the wall-clock deadline so search is a pure +/// function of `(seed, difficulty, feed)`. +/// +/// This encodes the repro shape end-to-end: the SAME game, run alone vs. run +/// 3rd after 2 unrelated games, must produce a byte-identical RESULT block. +/// (The deterministic, box-speed-independent catcher for this bug is +/// `ai_commander.rs`'s `seat_config_runs_in_measurement_mode_for_batch_reproducibility` +/// unit test; this is the integration-level forward guard for the whole path.) +#[test] +#[ignore = "loads card-data.json + runs real games; opt in via --ignored"] +fn batched_third_game_matches_same_game_run_alone() { + let target_seed = "95000004"; + let prefix_a = "95000000"; + let prefix_b = "95000001"; + + // The target game run entirely alone (single-game mode). + let solo = run_ai_commander(&[ + "--seed", + target_seed, + "--difficulty", + "Hard", + "--action-cap", + LEAK_REGRESSION_ACTION_CAP, + ]); + + // The SAME target game, but 3rd in a batch process that first played two + // unrelated games — the exact configuration that leaked before the fix. + let pid = std::process::id(); + let games_file_path = std::env::temp_dir().join(format!("ai_commander_leak_repro_{pid}.txt")); + std::fs::write( + &games_file_path, + format!("{prefix_a},Hard\n{prefix_b},Hard\n{target_seed},Hard\n"), + ) + .expect("write games-file"); + let batch = run_ai_commander(&[ + "--games-file", + games_file_path.to_str().unwrap(), + "--action-cap", + LEAK_REGRESSION_ACTION_CAP, + ]); + let _ = std::fs::remove_file(&games_file_path); + + let solo_blocks = game_blocks(&solo); + let batch_blocks = game_blocks(&batch); + assert_eq!( + solo_blocks.len(), + 1, + "single-game must print exactly one game" + ); + assert_eq!( + batch_blocks.len(), + 3, + "batch must print exactly one block per games-file line" + ); + + // Block index 2 is the 3rd (target) game in the batch. + assert_eq!( + normalized_result_block(solo_blocks[0]), + normalized_result_block(batch_blocks[2]), + "the target game played 3rd in a batch must be bit-identical to the \ + same game run alone; a divergence here is the cross-game leak \ + (wall-clock-deadline non-determinism) regressing" + ); +} + +#[test] +#[ignore = "loads card-data.json + runs real games; opt in via --ignored"] +fn batch_output_echoes_seed_and_parsed_difficulty_per_game() { + let pid = std::process::id(); + let games_file_path = std::env::temp_dir().join(format!("ai_commander_echo_games_{pid}.txt")); + std::fs::write(&games_file_path, "9101,Easy\n9102,VeryHard\n").expect("write games-file"); + + let batch = run_ai_commander(&[ + "--games-file", + games_file_path.to_str().unwrap(), + "--action-cap", + TEST_ACTION_CAP, + ]); + let _ = std::fs::remove_file(&games_file_path); + + // Each game's marker must carry BOTH the seed and the difficulty as + // actually parsed from that games-file line — not the process-level + // `--difficulty` (which batch mode ignores). A tier that silently fell + // back to the default would show up here as a wrong `difficulty=` label. + assert!( + batch.contains("--- GAME seed=9101 difficulty=Easy ---"), + "game 1 must echo its parsed seed+difficulty in its marker:\n{batch}" + ); + assert!( + batch.contains("--- GAME seed=9102 difficulty=VeryHard ---"), + "game 2 must echo its parsed seed+difficulty in its marker:\n{batch}" + ); +} + +#[test] +#[ignore = "loads card-data.json + runs real games; opt in via --ignored"] +fn single_game_stdout_is_deterministic_and_preamble_is_pinned() { + let seed = "9201"; + let out1 = run_ai_commander(&[ + "--seed", + seed, + "--difficulty", + "Easy", + "--action-cap", + TEST_ACTION_CAP, + ]); + let out2 = run_ai_commander(&[ + "--seed", + seed, + "--difficulty", + "Easy", + "--action-cap", + TEST_ACTION_CAP, + ]); + + // Two runs of the same seed/feed/difficulty must produce identical + // output modulo wall-clock timing — the process-level half of the same + // property `batched_third_game_matches_same_game_run_alone` asserts + // across the batch boundary. If this one fails too, the divergence is in + // the game itself, not in batch sequencing. + let normalize = |s: &str| { + s.lines() + .filter(|l| !l.contains("elapsed=") && !l.starts_with("Elapsed:")) + .collect::>() + .join("\n") + }; + assert_eq!(normalize(&out1), normalize(&out2)); + + // Pins the exact preamble single-game mode has always printed, in order. + // A reordering (e.g. "Feed:" moving relative to "Seed:.../Difficulty:...") + // would silently break the pod-lab harness's stdout parsing; this fails + // loudly instead. + let expected_preamble = format!( + "=== 4-player Commander AI test ===\n\ + Feed: feeds/mtggoldfish-commander.json\n\ + Seed: {seed} Difficulty: Easy\n\n" + ); + assert!( + out1.starts_with(&expected_preamble), + "single-game preamble format changed:\n{out1}" + ); +} From 3ebc81a4a3c9f3a5c23575e453c0c16df85b5250 Mon Sep 17 00:00:00 2001 From: mcbradd <35860549+mcbradd@users.noreply.github.com> Date: Tue, 28 Jul 2026 04:01:14 -0700 Subject: [PATCH 02/11] feat(ai): add opt-in --watch-cards swap-liveness telemetry to ai-commander pod-lab loop-3 Q3(b): restores a per-game "was this card ever drawn or cast" signal for the harness's swap comparisons, after loop-2 found divergence rate alone is useless for this (a single-card deck swap reorders the entire shuffled library, so divergence is 100% regardless of whether the changed card was ever seen -- see the design spike at docs/q3-swap-liveness-spike.md in the pod-lab repo). Purpose-built over two rejected alternatives: slot-stable shuffling (would be a new engine pin, invalidating every cached pod-lab baseline, for a correctness-burden the payoff didn't justify) and turning on the existing --dump-log/PHASE_DUMP_LOG full structured-log collection (pays for every turn/phase/stack/zone/life/mana entry for the whole game to answer a one-bit-per-watched-card question). `--watch-cards "Name A,Name B"` populates a per-game HashSet, matched by name (not CardId -- CardId is assigned per physical-card-object at deck load, not a stable per-name identity, while every GameObject already carries its own resolved name). The match runs on SpellCast/CardDrawn events already produced every action by the driver loop -- record_watched_cards is O(1) per event and is skipped entirely, not just a no-op HashSet lookup, when no names are requested, so a run that doesn't pass the flag pays nothing. Emits one `PODLAB-TELEM {"cards_seen": [...]}` line inside the existing === RESULT === summary block, only when --watch-cards is non-empty. Verified against pod-lab's runner.py/mechanisms.py regexes directly (not just by inspection) that this collides with none of them: the _TURN/_PROGRESS patterns both require a literal "Turn " prefix this line never has, and the line contains none of "Winner: "/"Difficulty: "/ "ABORT: hit "/"did NOT reach GameOver". Confirmed end-to-end against a real built binary (seed 7, both a true-negative and, via the RESULT block placement, correct positioning relative to the existing Elapsed/Total actions/Turns played lines). record_watched_cards is unit-tested directly (SpellCast + CardDrawn matched by name, an unwatched card and an unrelated event variant both proven to be no-ops, and an empty-results no-op case) using a minimal GameState + GameObject::new fixture -- no card database or deck loading needed, mirroring this file's existing fixture_db()-free test style where the logic under test doesn't require it. Local branch only -- no PR without separate explicit authorization (the pod-lab plan's E6 constraint: this session has authority to build, pin, and measure phase.rs changes from a local branch, not to open or merge upstream). Co-Authored-By: Claude Sonnet 5 --- crates/phase-ai/src/bin/ai_commander.rs | 189 ++++++++++++++++++++++++ 1 file changed, 189 insertions(+) diff --git a/crates/phase-ai/src/bin/ai_commander.rs b/crates/phase-ai/src/bin/ai_commander.rs index 6b2283306e..b45609df6c 100644 --- a/crates/phase-ai/src/bin/ai_commander.rs +++ b/crates/phase-ai/src/bin/ai_commander.rs @@ -34,6 +34,7 @@ use engine::database::CardDatabase; use engine::game::deck_loading::{ load_deck_into_state, resolve_deck_list, DeckList, DeckPayload, PlayerDeckList, }; +use engine::types::events::GameEvent; use engine::types::format::FormatConfig; use engine::types::game_state::{GameState, WaitingFor}; use engine::types::player::PlayerId; @@ -75,6 +76,10 @@ fn main() { let mut action_cap: usize = DEFAULT_ACTION_CAP; let mut feed: String = "feeds/mtggoldfish-commander.json".to_string(); let mut games_file: Option = None; + // pod-lab swap-liveness telemetry (loop-3 Q3(b)): empty means the + // per-event scan in `play_one_game` is skipped entirely, not merely a + // no-op HashSet lookup — a run that doesn't pass this flag pays nothing. + let mut watch_cards: HashSet = HashSet::new(); let mut args_iter = args.iter().skip(1).peekable(); while let Some(arg) = args_iter.next() { match arg.as_str() { @@ -107,6 +112,20 @@ fn main() { std::process::exit(1); } }, + "--watch-cards" => match args_iter.next() { + Some(v) => { + watch_cards = v + .split(',') + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(str::to_string) + .collect(); + } + None => { + eprintln!("error: --watch-cards requires a comma-separated card name list"); + std::process::exit(1); + } + }, other => { // `--difficulty-p0` .. `--difficulty-p3`: single-seat override, // parameterized on seat index rather than four bespoke flags. @@ -161,6 +180,7 @@ fn main() { action_cap, games_file, batch_games, + watch_cards, }; let handle = std::thread::Builder::new() .name("ai-commander-driver".to_string()) @@ -191,6 +211,7 @@ struct CliArgs { action_cap: usize, games_file: Option, batch_games: Option>, + watch_cards: HashSet, } /// Shared immutable inputs for one or more AI-commander game runs. @@ -202,6 +223,7 @@ struct GameRunContext<'a> { action_cap: usize, dump_log_path: Option<&'a str>, dump_actions_path: Option<&'a str>, + watch_cards: &'a HashSet, } /// Everything that isn't argument parsing: loads the card database and feed @@ -220,6 +242,7 @@ fn run(cli: CliArgs) -> i32 { action_cap, games_file, batch_games, + watch_cards, } = cli; let export_path = PathBuf::from(&cards_path).join("card-data.json"); @@ -355,6 +378,7 @@ fn run(cli: CliArgs) -> i32 { action_cap, dump_log_path: dump_log_path.as_deref(), dump_actions_path: dump_actions_path.as_deref(), + watch_cards: &watch_cards, }; match batch_games { @@ -408,6 +432,7 @@ fn play_one_game(context: &GameRunContext<'_>, seed: u64, difficulty: AiDifficul action_cap, dump_log_path, dump_actions_path, + watch_cards, } = *context; let mut state = build_game_state(db, payload, seed); @@ -433,6 +458,13 @@ fn play_one_game(context: &GameRunContext<'_>, seed: u64, difficulty: AiDifficul let start = Instant::now(); let mut game_log: Vec = Vec::new(); let mut actions_log: Vec = Vec::new(); + // pod-lab swap-liveness telemetry (loop-3 Q3(b)): which of `watch_cards`' + // names were ever drawn to hand or cast this game. Names, not `CardId`s — + // `CardId` is assigned per physical-card-object at deck load + // (`deck_loading.rs`), not a stable per-name identity, and every + // `GameObject` already carries its own resolved `name`, so matching on + // name needs no extra database lookup. + let mut cards_seen: HashSet = HashSet::new(); let mut last_turn_reported: u32 = 0; let mut ai_rng = StdRng::seed_from_u64(seed); let ai_session = phase_ai::session::AiSession::arc_from_game(&state); @@ -467,6 +499,10 @@ fn play_one_game(context: &GameRunContext<'_>, seed: u64, difficulty: AiDifficul } } + if !watch_cards.is_empty() { + record_watched_cards(results, watch_cards, &mut cards_seen); + } + if state.turn_number != last_turn_reported { last_turn_reported = state.turn_number; let snapshot: Vec = state @@ -529,6 +565,19 @@ fn play_one_game(context: &GameRunContext<'_>, seed: u64, difficulty: AiDifficul println!("Elapsed: {:.1}s", elapsed.as_secs_f64()); println!("Total actions: {total_actions}"); println!("Turns played: {}", state.turn_number); + // pod-lab swap-liveness telemetry (loop-3 Q3(b)): one line, only when + // `--watch-cards` was given, so a harness that doesn't ask for this pays + // nothing and every existing consumer's line-by-line parse is untouched. + // `PODLAB-TELEM ` is a prefix no other line in this binary's stdout uses, + // and this line does not begin with "Turn ", match `^--- GAME`, or + // contain "Winner: " / "Difficulty: " / "ABORT: hit " / "did NOT reach + // GameOver" (pod-lab's `runner.py`/`mechanisms.py` scan for exactly those + // literals). Cards are sorted for a deterministic, diff-friendly line. + if !watch_cards.is_empty() { + let mut seen: Vec<&str> = cards_seen.iter().map(String::as_str).collect(); + seen.sort_unstable(); + println!("PODLAB-TELEM {}", serde_json::json!({ "cards_seen": seen })); + } println!(); let outcome = classify_run_outcome(aborted, &state.waiting_for); @@ -633,6 +682,38 @@ fn play_one_game(context: &GameRunContext<'_>, seed: u64, difficulty: AiDifficul outcome } +/// pod-lab swap-liveness telemetry (loop-3 Q3(b)): scans one driver-loop +/// batch's `AiActionResult`s for `SpellCast`/`CardDrawn` events naming an +/// object whose CURRENT name (resolved via that action's own `r.state`, not +/// a stale/outer snapshot) is in `watch`, inserting the resolved name into +/// `seen`. Matches on name, not `CardId`: `CardId` is assigned per +/// physical-card-object at deck load (`deck_loading.rs`), not a stable +/// per-name identity, while every `GameObject` already carries its own +/// resolved `name` — no extra database lookup needed. Pure and unit-tested +/// separately from `play_one_game`'s full game-driving loop; the caller +/// skips calling this entirely when `watch` is empty, so a run that doesn't +/// pass `--watch-cards` pays nothing beyond the `is_empty()` check. +fn record_watched_cards( + results: &[phase_ai::auto_play::AiActionResult], + watch: &HashSet, + seen: &mut HashSet, +) { + for r in results { + for event in &r.events { + let object_id = match event { + GameEvent::SpellCast { object_id, .. } => *object_id, + GameEvent::CardDrawn { object_id, .. } => *object_id, + _ => continue, + }; + if let Some(obj) = r.state.objects.get(&object_id) { + if watch.contains(&obj.name) { + seen.insert(obj.name.clone()); + } + } + } + } +} + /// Runs `play` once per entry in `games`, isolating panics per game (Tier 1 /// item 2) so one bad game (this binary has real `unwrap()`/`expect()` calls, /// including deep in the AI search path) can't take down the rest of the @@ -904,7 +985,12 @@ fn build_seat_config(difficulty: AiDifficulty, seed: u64) -> AiConfig { mod tests { use super::*; use engine::ai_support::candidate_actions; + use engine::game::game_object::GameObject; use engine::types::ability::ChoiceType; + use engine::types::actions::GameAction; + use engine::types::identifiers::{CardId, ObjectId}; + use engine::types::zones::Zone; + use phase_ai::auto_play::AiActionResult; use std::sync::{Arc, Mutex}; struct TempFileGuard(PathBuf); @@ -1049,6 +1135,109 @@ mod tests { } } + /// Building-block test for `record_watched_cards` (loop-3 Q3(b)): proves + /// the matcher (a) recognizes both `SpellCast` and `CardDrawn` events, + /// (b) resolves the watched name via the object's *current* `r.state` + /// rather than any fixed name table, and (c) is selective — an event + /// naming an object outside `watch`, and an event of an unrelated + /// variant entirely, must both be no-ops rather than getting recorded. + #[test] + fn record_watched_cards_matches_spell_cast_and_card_drawn_by_name() { + let mut state = GameState::new(FormatConfig::commander(), 4, 1); + let cast_obj = ObjectId(100); + let drawn_obj = ObjectId(101); + let unwatched_obj = ObjectId(102); + state.objects.insert( + cast_obj, + GameObject::new( + cast_obj, + CardId(100), + PlayerId(0), + "Lightning Bolt".to_string(), + Zone::Stack, + ), + ); + state.objects.insert( + drawn_obj, + GameObject::new( + drawn_obj, + CardId(101), + PlayerId(0), + "Sol Ring".to_string(), + Zone::Hand, + ), + ); + state.objects.insert( + unwatched_obj, + GameObject::new( + unwatched_obj, + CardId(102), + PlayerId(0), + "Forest".to_string(), + Zone::Battlefield, + ), + ); + + let watch: HashSet = ["Lightning Bolt".to_string(), "Sol Ring".to_string()] + .into_iter() + .collect(); + let mut seen: HashSet = HashSet::new(); + + let results = vec![ + AiActionResult { + action: GameAction::PassPriority, + state: state.clone(), + events: vec![GameEvent::SpellCast { + card_id: CardId(100), + controller: PlayerId(0), + object_id: cast_obj, + }], + log_entries: Vec::new(), + }, + AiActionResult { + action: GameAction::PassPriority, + state: state.clone(), + events: vec![ + GameEvent::CardDrawn { + player_id: PlayerId(0), + object_id: drawn_obj, + nth_in_turn: 1, + nth_in_step: 1, + }, + // Drawn but not watched, and an unrelated event variant — + // both must be ignored, not just the watched ones matched. + GameEvent::CardDrawn { + player_id: PlayerId(0), + object_id: unwatched_obj, + nth_in_turn: 2, + nth_in_step: 2, + }, + GameEvent::PriorityPassed { + player_id: PlayerId(0), + }, + ], + log_entries: Vec::new(), + }, + ]; + + record_watched_cards(&results, &watch, &mut seen); + + assert_eq!( + seen, + ["Lightning Bolt".to_string(), "Sol Ring".to_string()] + .into_iter() + .collect::>() + ); + } + + #[test] + fn record_watched_cards_is_a_noop_on_empty_results() { + let watch: HashSet = ["Lightning Bolt".to_string()].into_iter().collect(); + let mut seen: HashSet = HashSet::new(); + record_watched_cards(&[], &watch, &mut seen); + assert!(seen.is_empty()); + } + #[test] fn parse_action_cap_accepts_positive_integer() { assert_eq!(parse_action_cap_checked("50000"), Ok(50000)); From 554f1cb1344024e258bd8770571881fc91e7e514 Mon Sep 17 00:00:00 2001 From: mcbradd <35860549+mcbradd@users.noreply.github.com> Date: Tue, 28 Jul 2026 04:16:11 -0700 Subject: [PATCH 03/11] perf(ai): mimalloc for all native phase-ai binaries; fix panic=abort doc bug pod-lab loop-3 Q5, part 1 of 2 (mimalloc + build-recipe fix; the layers.rs incremental-flush lever is a separate, more involved change gated on its own review round -- see the plan/review transcript for why it isn't landing in this commit). mimalloc is target-gated in Cargo.toml (`[target.'cfg(not(target_arch = "wasm32"))'.dependencies]`), not a plain [dependencies] entry: engine-wasm and draft-wasm both depend on this crate's lib for their wasm32 builds, which are explicitly size-tuned (opt-level='z', the #6313 25 MiB pages-deploy guard) -- an ungated entry would pull mimalloc's C sources into those builds too. Verified both ways: `cargo tree -i mimalloc --target wasm32-unknown- unknown` resolves empty for both wasm crates, and `cargo check -p engine-wasm --target wasm32-unknown-unknown` still builds clean. Every native bin in crates/phase-ai/src/bin/ gets a #[global_allocator] declaration -- all 12 files, not just the 9 with an explicit [[bin]] Cargo.toml entry. declare_attackers_bench.rs/pass_priority_bench.rs/ resolve_bench.rs are picked up by Cargo's default bin auto-discovery with no [[bin]] stanza of their own; a plan that enumerated only the explicit entries would have silently skipped them. Also fixes a real bug found while verifying the PGO half of this plan (not implemented in this commit -- that's a build-recipe change, not a source change, and depends on pod-lab's actual invocation, out of scope for this repo): ai_commander.rs's own doc comment documented `cargo run --release`, but [profile.release] sets panic='abort' (it exists to keep the WASM build small), which silently defeats run_batch_isolated's catch_unwind-based per-game panic isolation -- under abort, one game's panic takes the whole batch process down instead of being caught and reported via the "GAME