fix(ai): surface ai-commander stalls + strict difficulty parse + post-resolution deck counts

crates/phase-ai/src/auto_play.rs

@@ -1,6 +1,6 @@
 use std::collections::{HashMap, HashSet};
 
-use engine::game::engine::apply;
+use engine::game::engine::{apply, EngineError};
 use engine::game::turn_control;
 use engine::types::actions::GameAction;
 use engine::types::events::GameEvent;
@@ -26,6 +26,91 @@ pub struct AiActionResult {
     pub log_entries: Vec<GameLogEntry>,
 }
 
+/// Which of `run_ai_actions`'s four break doors (see its doc comment) ended
+/// the batch before the safety cap. `None` means the loop ran out AI actions
+/// to take for a benign reason the caller already distinguishes elsewhere
+/// (hit `MAX_AI_ACTIONS_PER_SEQUENCE`, or the very first iteration found no
+/// actor at all — that case is folded into `NoActor` below instead, so `None`
+/// in practice only means "hit the safety cap").
+///
+/// Diagnostic surface for phase#6080 (the driver-stall family): today the only
+/// signal at these break points is a `tracing::error`/`tracing::warn` that no
+/// harness subscriber captures. Exposing the reason as typed data lets a
+/// caller like `ai_commander` print it instead of installing a subscriber.
+#[derive(Debug, Clone)]
+pub enum AiActionsBreakReason {
+    /// No AI seat can currently act. Two causes are still folded together
+    /// here: `WaitingFor::acting_players()` returned empty (`GameOver`, or an
+    /// empty pending set), or it returned one or more players and none of
+    /// their `turn_control::authorized_submitter_for_player` mappings is in
+    /// `ai_players` (a human seat, or a human turn-decision controller).
+    /// Deliberately carries no `PlayerId`: the first cause has no player at
+    /// all, and the simultaneous-decision variants (`MulliganDecision`,
+    /// `OpeningHandBottomCards`) can pend several at once, so naming one
+    /// would be arbitrary. A missing AI *configuration* is `MissingAiConfig`.
+    NoActor,
+    /// `player` is in `ai_players` but has no entry in `ai_configs`. Distinct
+    /// from `NoActor`: an actor *was* found and *is* AI-controlled, so the
+    /// remedy is caller wiring (register a config for this seat), not "wait
+    /// for a human" or "the game ended".
+    MissingAiConfig { player: PlayerId },
+    /// `choose_action_with_session` returned `None` for `player` — the AI
+    /// policy stack produced no legal action for a decision it was asked to
+    /// make.
+    ChooseActionNone { player: PlayerId },
+    /// `apply()` rejected `player`'s chosen `action`. `action` is boxed because
+    /// `GameAction` is large relative to the other variants (clippy
+    /// `large_enum_variant`); `EngineError` is four small variants (largest
+    /// payload a `String`) and needs no box.
+    ApplyFailed {
+        player: PlayerId,
+        action: Box<GameAction>,
+        error: EngineError,
+    },
+}
+
+/// Outcome of a `run_ai_actions` batch.
+///
+/// `Deref`s to `Vec<AiActionResult>` so the many existing callers that only
+/// care about the actions taken (`.is_empty()`, `.len()`, indexing, iterating
+/// by reference) are source-compatible; only diagnostic consumers need
+/// `break_reason`.
+pub struct AiActionsRun {
+    pub results: Vec<AiActionResult>,
+    pub break_reason: Option<AiActionsBreakReason>,
+}
+
+impl std::ops::Deref for AiActionsRun {
+    type Target = Vec<AiActionResult>;
+    fn deref(&self) -> &Vec<AiActionResult> {
+        &self.results
+    }
+}
+
+impl IntoIterator for AiActionsRun {
+    type Item = AiActionResult;
+    type IntoIter = std::vec::IntoIter<AiActionResult>;
+    fn into_iter(self) -> Self::IntoIter {
+        self.results.into_iter()
+    }
+}
+
+impl<'a> IntoIterator for &'a AiActionsRun {
+    type Item = &'a AiActionResult;
+    type IntoIter = std::slice::Iter<'a, AiActionResult>;
+    fn into_iter(self) -> Self::IntoIter {
+        self.results.iter()
+    }
+}
+
+impl<'a> IntoIterator for &'a mut AiActionsRun {
+    type Item = &'a mut AiActionResult;
+    type IntoIter = std::slice::IterMut<'a, AiActionResult>;
+    fn into_iter(self) -> Self::IntoIter {
+        self.results.iter_mut()
+    }
+}
+
 /// Run AI actions on the game state until the next actor is human or the game is over.
 ///
 /// Returns one `AiActionResult` per AI action taken, preserving granularity for
@@ -44,8 +129,9 @@ pub fn run_ai_actions(
     ai_configs: &HashMap<PlayerId, AiConfig>,
     rng: &mut impl Rng,
     session: &Arc<AiSession>,
-) -> Vec<AiActionResult> {
+) -> AiActionsRun {
     let mut results = Vec::new();
+    let mut break_reason = None;
 
     for _ in 0..MAX_AI_ACTIONS_PER_SEQUENCE {
         // CR 723.5: Under turn control (Mindslaver, Emrakul), the authorized
@@ -60,13 +146,15 @@ pub fn run_ai_actions(
             .find(|player| ai_players.contains(player));
 
         let Some(actor) = actor else {
+            break_reason = Some(AiActionsBreakReason::NoActor);
             break;
         };
 
         let config = match ai_configs.get(&actor) {
             Some(c) => c,
             None => {
                 tracing::warn!(player = ?actor, "AI seat has no config — stopping AI loop");
+                break_reason = Some(AiActionsBreakReason::MissingAiConfig { player: actor });
                 break;
             }
         };
@@ -75,6 +163,7 @@ pub fn run_ai_actions(
             Some(a) => a,
             None => {
                 tracing::warn!(player = ?actor, "choose_action returned None — stopping AI loop");
+                break_reason = Some(AiActionsBreakReason::ChooseActionNone { player: actor });
                 break;
             }
         };
@@ -93,6 +182,11 @@ pub fn run_ai_actions(
             }
             Err(e) => {
                 tracing::error!(player = ?actor, error = %e, "AI action apply failed — stopping");
+                break_reason = Some(AiActionsBreakReason::ApplyFailed {
+                    player: actor,
+                    action: Box::new(action),
+                    error: e,
+                });
                 break;
             }
         }
@@ -105,5 +199,103 @@ pub fn run_ai_actions(
         );
     }
 
-    results
+    AiActionsRun {
+        results,
+        break_reason,
+    }
+}
+
+/// Driver-relevant outcome of processing one `run_ai_actions` batch: how many
+/// actions to add to a caller's running total, and the break reason to stop
+/// and report at this batch boundary, if the batch carries one.
+///
+/// phase#6080 follow-up: a batch can complete one or more actions (`results`
+/// non-empty) and *still* carry a `break_reason` — e.g. it applies two
+/// actions, then the third choice is `ChooseActionNone` or the fourth
+/// `apply()` call fails. A driver that only inspects `break_reason` when
+/// `results.is_empty()` silently discards the diagnostic for exactly that
+/// case, loops again, and may report a misleading `NoActor`/unknown reason
+/// once a later, unrelated batch happens to come back empty. `driver_step`
+/// is the single place that decision is made, so callers (and tests) don't
+/// re-derive it ad hoc.
+pub struct DriverStep {
+    pub actions_taken: usize,
+    pub break_reason: Option<AiActionsBreakReason>,
+}
+
+/// Extracts the [`DriverStep`] for one batch. Callers should process
+/// `results`'s individual `AiActionResult`s (logging, animation, dumps)
+/// before or after calling this — it only reports the count/stop decision.
+pub fn driver_step(results: AiActionsRun) -> DriverStep {
+    DriverStep {
+        actions_taken: results.results.len(),
+        break_reason: results.break_reason,
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn dummy_result(state: &GameState) -> AiActionResult {
+        AiActionResult {
+            action: GameAction::PassPriority,
+            state: state.clone(),
+            events: Vec::new(),
+            log_entries: Vec::new(),
+        }
+    }
+
+    #[test]
+    fn driver_step_preserves_break_reason_from_non_empty_batch() {
+        // The exact regression: a batch that completed an action must not
+        // have its break_reason discarded just because `results` isn't
+        // empty.
+        let state = GameState::new_two_player(1);
+        let run = AiActionsRun {
+            results: vec![dummy_result(&state)],
+            break_reason: Some(AiActionsBreakReason::ChooseActionNone {
+                player: PlayerId(1),
+            }),
+        };
+        let step = driver_step(run);
+        assert_eq!(step.actions_taken, 1);
+        assert!(
+            matches!(
+                step.break_reason,
+                Some(AiActionsBreakReason::ChooseActionNone { .. })
+            ),
+            "break_reason from a non-empty batch must survive driver_step"
+        );
+    }
+
+    #[test]
+    fn driver_step_empty_batch_behavior_is_unchanged() {
+        // Existing behavior (empty batch + break reason) must still work.
+        let run = AiActionsRun {
+            results: Vec::new(),
+            break_reason: Some(AiActionsBreakReason::NoActor),
+        };
+        let step = driver_step(run);
+        assert_eq!(step.actions_taken, 0);
+        assert!(matches!(
+            step.break_reason,
+            Some(AiActionsBreakReason::NoActor)
+        ));
+    }
+
+    #[test]
+    fn driver_step_ordinary_batch_is_unaffected() {
+        // Ordinary caller path: batch completed actions and hit no break
+        // door (e.g. hit the safety cap) — driver_step must not fabricate a
+        // stop signal.
+        let state = GameState::new_two_player(1);
+        let run = AiActionsRun {
+            results: vec![dummy_result(&state), dummy_result(&state)],
+            break_reason: None,
+        };
+        let step = driver_step(run);
+        assert_eq!(step.actions_taken, 2);
+        assert!(step.break_reason.is_none());
+    }
 }