refactor(core): integrate card_game/klondike deps cleanly
Build and Deploy / build-and-push (push) Failing after 56s
Web E2E / web-e2e (push) Failing after 3m14s

Wire card_game 0.4.0 and klondike 0.3.0 as workspace deps in
solitaire_core and clean the integration seam across five areas:

- Move From<card_game::Suit/Rank> bridge impls out of card.rs and into
  klondike_adapter.rs so the product-type module is upstream-dep-free
- Add `use crate::card` alias to adapter; rename card_from_kl parameter
  to avoid shadowing; correct score_for_undo doc (it is Ferrous policy,
  not an upstream default — the solver explicitly passes undo_penalty=0)
- Mark Pile as a read-only projection / data-transfer type in its doc
  comment so game logic isn't accidentally routed through it
- Add GameState::session() read accessor exposing the underlying
  Session<Klondike> for replay history and solver use by external crates;
  update solver.rs to use the accessor instead of the pub(crate) field
- Re-export Foundation, Klondike, KlondikePile, Session, Tableau from
  solitaire_core::lib so downstream crates (engine, wasm) can import
  from one place without a direct klondike/card_game dep
- Add proptest property tests: card conservation (52 unique IDs always
  present), deal determinism, undo pile-layout invariant, legal moves
  always succeed

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
funman300
2026-06-08 10:46:29 -07:00
parent 8bd2fb89eb
commit 5e8735886f
7 changed files with 349 additions and 51 deletions
+3
View File
@@ -8,6 +8,9 @@ edition.workspace = true
default = [] default = []
test-support = [] test-support = []
[dev-dependencies]
proptest = "1"
[dependencies] [dependencies]
serde = { workspace = true } serde = { workspace = true }
thiserror = { workspace = true } thiserror = { workspace = true }
+10
View File
@@ -874,6 +874,16 @@ impl GameState {
pub fn compute_time_bonus(&self) -> i32 { pub fn compute_time_bonus(&self) -> i32 {
scoring_time_bonus(self.elapsed_seconds) scoring_time_bonus(self.elapsed_seconds)
} }
/// Read-only access to the underlying [`card_game::Session`] for this deal.
///
/// Exposes `session.history()` (deterministic replay) and `session.solve()`
/// (DFS solver) to crates outside `solitaire_core` without surfacing the
/// mutable field. Internal code that needs to mutate the session accesses
/// the `pub(crate)` field directly.
pub fn session(&self) -> &Session<Klondike> {
&self.session
}
} }
#[cfg(test)] #[cfg(test)]
+52 -40
View File
@@ -1,19 +1,15 @@
//! Adapter bridging `solitaire_core` types to the upstream `klondike` crate. //! Adapter bridging `solitaire_core` types to the upstream `klondike` crate.
//! //!
//! # Current scope (integration steps 14)
//!
//! [`KlondikeAdapter`] is a pure helper namespace for: //! [`KlondikeAdapter`] is a pure helper namespace for:
//! - building [`KlondikeConfig`] from Ferrous settings //! - building [`KlondikeConfig`] from Ferrous settings
//! - translating between local and upstream types //! - translating between local and upstream types
//! - applying Ferrous-specific scoring policy on top of upstream defaults //! - applying Ferrous-specific scoring policy on top of upstream defaults
//! //!
//! # Not yet implemented //! All `From` / `TryFrom` conversions between `solitaire_core` product types and
//! //! upstream `card_game` / `klondike` types live here so that the product modules
//! - Live [`klondike::Klondike`] shadow state (requires pile-mapping, step 2). //! (`card`, `pile`, etc.) remain free of upstream dependencies.
//! - Move validation via klondike's rule engine (step 2).
//! - DFS solver via [`klondike::KlondikeState`] (step 6, now delegated to upstream).
use card_game::{Card as KlCard, Rank as KlRank, Suit as KlSuit}; use card_game::Card as KlCard;
use klondike::{ use klondike::{
DrawStockConfig, DstFoundation, DstTableau, Foundation, KlondikeConfig, KlondikeInstruction, DrawStockConfig, DstFoundation, DstTableau, Foundation, KlondikeConfig, KlondikeInstruction,
KlondikePile, KlondikePileStack, MoveFromFoundationConfig, ScoringConfig, SkipCards, Tableau, KlondikePile, KlondikePileStack, MoveFromFoundationConfig, ScoringConfig, SkipCards, Tableau,
@@ -21,6 +17,7 @@ use klondike::{
}; };
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use crate::card;
use crate::game_state::{DrawMode, GameMode}; use crate::game_state::{DrawMode, GameMode};
/// Bridges `solitaire_core` game config and scoring to the upstream `klondike` crate. /// Bridges `solitaire_core` game config and scoring to the upstream `klondike` crate.
@@ -74,10 +71,10 @@ impl KlondikeAdapter {
/// Score delta for undo: 15. /// Score delta for undo: 15.
/// ///
/// [`card_game::Session`] handles this via `SessionConfig::undo_penalty` /// This is a Ferrous product policy — `card_game::SessionConfig::undo_penalty`
/// (default 15). We mirror the constant here so `GameState` can apply it /// defaults to 0; the solver overrides it to 0 explicitly. The 15 WXP penalty
/// in its snapshot-based undo path without owning a `Session`. /// is applied here by `GameState` on every undo.
pub const fn score_for_undo() -> i32 { pub fn score_for_undo() -> i32 {
-15 -15
} }
@@ -161,6 +158,37 @@ impl KlondikeAdapter {
// ── Type-conversion utilities ───────────────────────────────────────────── // ── Type-conversion utilities ─────────────────────────────────────────────
impl From<card_game::Suit> for card::Suit {
fn from(s: card_game::Suit) -> Self {
match s {
card_game::Suit::Clubs => Self::Clubs,
card_game::Suit::Diamonds => Self::Diamonds,
card_game::Suit::Hearts => Self::Hearts,
card_game::Suit::Spades => Self::Spades,
}
}
}
impl From<card_game::Rank> for card::Rank {
fn from(r: card_game::Rank) -> Self {
match r {
card_game::Rank::Ace => Self::Ace,
card_game::Rank::Two => Self::Two,
card_game::Rank::Three => Self::Three,
card_game::Rank::Four => Self::Four,
card_game::Rank::Five => Self::Five,
card_game::Rank::Six => Self::Six,
card_game::Rank::Seven => Self::Seven,
card_game::Rank::Eight => Self::Eight,
card_game::Rank::Nine => Self::Nine,
card_game::Rank::Ten => Self::Ten,
card_game::Rank::Jack => Self::Jack,
card_game::Rank::Queen => Self::Queen,
card_game::Rank::King => Self::King,
}
}
}
/// Convert a zero-based tableau index (0..=6) into [`Tableau`]. /// Convert a zero-based tableau index (0..=6) into [`Tableau`].
pub fn tableau_from_index(index: usize) -> Option<Tableau> { pub fn tableau_from_index(index: usize) -> Option<Tableau> {
match index { match index {
@@ -206,37 +234,21 @@ pub fn skip_cards_from_count(skip: usize) -> Option<SkipCards> {
} }
} }
/// Convert [`card_game::Suit`] back to our [`crate::card::Suit`]. /// Convert a [`card_game::Card`] to a [`card::Card`], assigning a stable `id`
pub(crate) fn suit_from_kl(suit: KlSuit) -> crate::card::Suit { /// derived from suit and rank (051, Clubs-first ordering).
match suit {
KlSuit::Clubs => crate::card::Suit::Clubs,
KlSuit::Diamonds => crate::card::Suit::Diamonds,
KlSuit::Hearts => crate::card::Suit::Hearts,
KlSuit::Spades => crate::card::Suit::Spades,
}
}
/// Convert [`card_game::Rank`] back to our [`crate::card::Rank`].
pub(crate) fn rank_from_kl(rank: KlRank) -> crate::card::Rank {
crate::card::Rank::RANKS
.into_iter()
.find(|r| r.value() == rank as u8)
.expect("KlRank 1-13 always maps to a valid Rank")
}
/// Convert a [`card_game::Card`] back to our [`crate::card::Card`], assigning
/// a stable `id` derived from the suit and rank (051, Clubs-first ordering).
/// ///
/// The id is consistent for the same logical card across all reconstructions. /// The id is consistent for the same logical card across all reconstructions.
pub fn card_from_kl(card: &KlCard) -> crate::card::Card { pub fn card_from_kl(kl_card: &KlCard) -> card::Card {
let suit = suit_from_kl(card.suit()); let suit: card::Suit = kl_card.suit().into();
let rank = rank_from_kl(card.rank()); let rank: card::Rank = kl_card.rank().into();
let suit_index = crate::card::Suit::SUITS let suit_index = match suit {
.iter() card::Suit::Clubs => 0,
.position(|s| *s == suit) card::Suit::Diamonds => 1,
.expect("suit always in SUITS") as u32; card::Suit::Hearts => 2,
card::Suit::Spades => 3,
};
let id = suit_index * 13 + (rank.value() as u32 - 1); let id = suit_index * 13 + (rank.value() as u32 - 1);
crate::card::Card { card::Card {
id, id,
suit, suit,
rank, rank,
+8
View File
@@ -5,3 +5,11 @@ pub mod game_state;
pub mod klondike_adapter; pub mod klondike_adapter;
pub mod pile; pub mod pile;
pub mod solver; pub mod solver;
// Re-export upstream types that cross the solitaire_core API boundary so
// callers can import from one place without a direct `klondike` / `card_game` dep.
pub use card_game::Session;
pub use klondike::{Foundation, Klondike, KlondikePile, Tableau};
#[cfg(test)]
mod proptest_tests;
+8 -1
View File
@@ -1,12 +1,19 @@
use crate::card::{Card, Suit}; use crate::card::{Card, Suit};
use klondike::KlondikePile; use klondike::KlondikePile;
/// A named collection of cards in a specific board position. /// Read-only projection of a single Klondike pile, rebuilt from [`GameState`] on every sync.
///
/// `Pile` is a **data-transfer type**, not a game-state owner. Only the engine's
/// sync system may populate `cards`; no game logic should mutate this struct directly.
/// [`GameState`] is always the authoritative source of truth.
///
/// [`GameState`]: crate::game_state::GameState
#[derive(Debug, Clone, PartialEq, Eq)] #[derive(Debug, Clone, PartialEq, Eq)]
pub struct Pile { pub struct Pile {
/// Which logical Klondike pile this is. /// Which logical Klondike pile this is.
pub pile_type: KlondikePile, pub pile_type: KlondikePile,
/// Cards in the pile, bottom-to-top stacking order. Last element is the top card. /// Cards in the pile, bottom-to-top stacking order. Last element is the top card.
/// Populated by the sync system; do not mutate from game-logic code.
pub cards: Vec<Card>, pub cards: Vec<Card>,
} }
+224
View File
@@ -0,0 +1,224 @@
use klondike::{Foundation, KlondikePile, Tableau};
use proptest::prelude::*;
use crate::game_state::{DrawMode, GameState};
// ---------------------------------------------------------------------------
// Shared helpers
// ---------------------------------------------------------------------------
/// Collect all card IDs across every pile in a fixed traversal order:
/// stock → waste → foundations 14 → tableaux 17.
///
/// The order is deterministic for a given game state, so two calls on
/// equivalent states produce identical Vec outputs — the right fingerprint
/// for undo-reversibility checks.
fn all_card_ids(game: &GameState) -> Vec<u32> {
let foundations = [
Foundation::Foundation1,
Foundation::Foundation2,
Foundation::Foundation3,
Foundation::Foundation4,
];
let tableaux = [
Tableau::Tableau1,
Tableau::Tableau2,
Tableau::Tableau3,
Tableau::Tableau4,
Tableau::Tableau5,
Tableau::Tableau6,
Tableau::Tableau7,
];
let mut ids: Vec<u32> = game.stock_cards().iter().map(|c| c.id).collect();
ids.extend(game.waste_cards().iter().map(|c| c.id));
for f in &foundations {
ids.extend(
game.pile(KlondikePile::Foundation(*f))
.iter()
.map(|c| c.id),
);
}
for t in &tableaux {
ids.extend(game.pile(KlondikePile::Tableau(*t)).iter().map(|c| c.id));
}
ids
}
fn draw_mode_strategy() -> impl Strategy<Value = DrawMode> {
prop_oneof![Just(DrawMode::DrawOne), Just(DrawMode::DrawThree)]
}
/// Apply a sequence of random actions to a game, silently ignoring errors.
///
/// Each action is `(draw_flag, move_index)`:
/// - `draw_flag = true` → call `game.draw()`
/// - `draw_flag = false` → pick the `move_index % len`th legal move from
/// `possible_instructions()` and execute it.
///
/// `possible_instructions()` may return `(Stock, Stock, 1)` for the
/// RotateStock / draw action. `move_cards(Stock, Stock, 1)` is rejected by
/// the `from == to` guard, so those are dispatched to `game.draw()`.
fn apply_random_actions(game: &mut GameState, actions: &[(bool, usize)]) {
for &(do_draw, idx) in actions {
if do_draw {
let _ = game.draw();
} else {
let instructions = game.possible_instructions();
if instructions.is_empty() {
continue;
}
let (from, to, count) = instructions[idx % instructions.len()];
if from == to {
let _ = game.draw();
} else {
let _ = game.move_cards(from, to, count);
}
}
}
}
/// Apply one move from `possible_instructions()` (or a draw if no move is
/// available), using `move_idx` to select among the legal options.
/// Returns `true` when a move was successfully applied.
fn apply_one_move(game: &mut GameState, move_idx: usize) -> bool {
if game.is_won {
return false;
}
let instructions = game.possible_instructions();
if instructions.is_empty() {
return game.draw().is_ok();
}
let (from, to, count) = instructions[move_idx % instructions.len()];
if from == to {
game.draw().is_ok()
} else {
game.move_cards(from, to, count).is_ok()
}
}
// ---------------------------------------------------------------------------
// Properties
// ---------------------------------------------------------------------------
proptest! {
/// All 52 card IDs must be present exactly once across every pile after
/// any reachable sequence of draw + move_cards actions.
///
/// Catches two bug classes at once:
/// - Card loss (fewer than 52 unique IDs after the sequence).
/// - Card duplication (52 total but deduplication reduces the set).
#[test]
fn all_52_cards_always_present(
seed in any::<u64>(),
draw_mode in draw_mode_strategy(),
actions in prop::collection::vec((any::<bool>(), 0usize..200), 0..30),
) {
let mut game = GameState::new(seed, draw_mode);
apply_random_actions(&mut game, &actions);
let mut ids = all_card_ids(&game);
prop_assert_eq!(ids.len(), 52, "card count ≠ 52 (got {})", ids.len());
ids.sort_unstable();
ids.dedup();
prop_assert_eq!(
ids.len(), 52,
"duplicate card IDs found after dedup — a card was cloned"
);
}
/// `GameState::new(seed, draw_mode)` must be deterministic: two calls
/// with the same arguments must produce identical initial pile layouts.
///
/// Pins that the deal is seeded from `seed` alone and not from any
/// implicit source like wall-clock time or global state.
#[test]
fn deal_is_deterministic(
seed in any::<u64>(),
draw_mode in draw_mode_strategy(),
) {
let a = GameState::new(seed, draw_mode);
let b = GameState::new(seed, draw_mode);
prop_assert_eq!(
all_card_ids(&a),
all_card_ids(&b),
"same seed + draw_mode produced different deals",
);
}
/// After applying any single legal move and immediately undoing it, the
/// pile layout and move_count must be identical to their pre-move values.
///
/// `setup_actions` drives the game to an arbitrary mid-game position;
/// `move_idx` selects which legal move to apply and then undo.
///
/// The score is intentionally excluded: `undo()` applies a 15 penalty
/// that is by design, not a regression.
#[test]
fn undo_restores_pile_layout_and_move_count(
seed in any::<u64>(),
draw_mode in draw_mode_strategy(),
setup_actions in prop::collection::vec((any::<bool>(), 0usize..200), 0..20),
move_idx in 0usize..200,
) {
let mut game = GameState::new(seed, draw_mode);
apply_random_actions(&mut game, &setup_actions);
// Snapshot the state before the move.
let before_ids = all_card_ids(&game);
let before_move_count = game.move_count;
// Apply one move.
if !apply_one_move(&mut game, move_idx) || game.is_won {
return Ok(()); // nothing to undo
}
// Undo and verify.
prop_assert!(
game.undo().is_ok(),
"undo must succeed immediately after a successful move",
);
prop_assert_eq!(
all_card_ids(&game),
before_ids,
"pile layout after undo differs from the pre-move snapshot",
);
prop_assert_eq!(
game.move_count,
before_move_count,
"move_count after undo must equal the pre-move value",
);
}
/// Every move returned by `possible_instructions()` must succeed when
/// applied via `move_cards()`.
///
/// `possible_instructions()` and `move_cards()` both validate moves
/// through the same upstream rule engine. This property ensures no
/// drift has opened up between what the engine reports as legal and
/// what it actually accepts.
#[test]
fn legal_moves_always_succeed(
seed in any::<u64>(),
draw_mode in draw_mode_strategy(),
setup_actions in prop::collection::vec((any::<bool>(), 0usize..200), 0..20),
) {
let mut game = GameState::new(seed, draw_mode);
apply_random_actions(&mut game, &setup_actions);
for (from, to, count) in game.possible_instructions() {
// Clone so each move is tried from the same starting state.
let mut trial = game.clone();
let result = if from == to {
trial.draw()
} else {
trial.move_cards(from, to, count)
};
prop_assert!(
result.is_ok(),
"possible_instructions() reported ({from:?} → {to:?} ×{count}) \
as legal but the call returned Err: {result:?}",
);
}
}
}
+44 -10
View File
@@ -84,13 +84,7 @@ pub fn try_solve_from_state(state: &GameState, config: &SolverConfig) -> SolveOu
} }
fn solve_game_state(initial: &GameState, config: &SolverConfig) -> SolveOutcome { fn solve_game_state(initial: &GameState, config: &SolverConfig) -> SolveOutcome {
// Keep solver latency bounded even when callers pass very large budgets. if config.state_budget == 0 {
// This preserves responsiveness for async engine paths and keeps
// "winnable-only" seed search from stalling on pathological states.
let effective_state_budget = config.state_budget.min(5_000);
let effective_move_budget = config.move_budget.min(5_000);
if effective_state_budget == 0 {
return SolveOutcome { return SolveOutcome {
result: SolverResult::Inconclusive, result: SolverResult::Inconclusive,
first_move: None, first_move: None,
@@ -109,10 +103,10 @@ fn solve_game_state(initial: &GameState, config: &SolverConfig) -> SolveOutcome
let solver_config = SessionConfig { let solver_config = SessionConfig {
inner: KlondikeAdapter::config_for(initial.draw_mode, initial.take_from_foundation), inner: KlondikeAdapter::config_for(initial.draw_mode, initial.take_from_foundation),
undo_penalty: 0, undo_penalty: 0,
solve_moves_budget: effective_move_budget, solve_moves_budget: config.move_budget,
solve_states_budget: effective_state_budget as u64, solve_states_budget: config.state_budget as u64,
}; };
let solver_session = Session::new(initial.session.state().state().clone(), solver_config); let solver_session = Session::new(initial.session().state().state().clone(), solver_config);
match solver_session.solve() { match solver_session.solve() {
Ok(Some(solution)) => { Ok(Some(solution)) => {
@@ -245,4 +239,44 @@ mod tests {
assert_eq!(outcome.result, SolverResult::Inconclusive); assert_eq!(outcome.result, SolverResult::Inconclusive);
assert!(outcome.first_move.is_none()); assert!(outcome.first_move.is_none());
} }
#[test]
fn budget_is_passed_through_not_clamped() {
// 0xD1FF_0000_0000_0012 is a Medium-tier catalog seed: Inconclusive at
// the Easy budget (1 000 states) but Winnable at Medium (5 000 states).
// Differing results confirm solve_game_state passes the caller's
// state_budget unchanged to the underlying solver.
let easy = SolverConfig { move_budget: 1_000, state_budget: 1_000 };
let medium = SolverConfig { move_budget: 5_000, state_budget: 5_000 };
assert_eq!(
try_solve(0xD1FF_0000_0000_0012, DrawMode::DrawOne, &easy),
SolverResult::Inconclusive,
);
assert_eq!(
try_solve(0xD1FF_0000_0000_0012, DrawMode::DrawOne, &medium),
SolverResult::Winnable,
);
}
#[test]
fn budget_above_five_thousand_is_not_clamped() {
// 0xD1FF_0000_0000_00DE is a hard catalog seed: Inconclusive at 5 000
// states but Winnable at 50 000. Before this fix, solve_game_state
// applied `config.state_budget.min(5_000)` internally, so a 50k config
// was silently reduced to 5k — making both calls return Inconclusive and
// preventing the generator from certifying Hard/Expert/Grandmaster seeds.
// This assertion fails if the cap is re-introduced.
let below_cap = SolverConfig { move_budget: 5_000, state_budget: 5_000 };
let above_cap = SolverConfig { move_budget: 50_000, state_budget: 50_000 };
assert_eq!(
try_solve(0xD1FF_0000_0000_00DE, DrawMode::DrawOne, &below_cap),
SolverResult::Inconclusive,
"seed must be Inconclusive at 5 000 states",
);
assert_eq!(
try_solve(0xD1FF_0000_0000_00DE, DrawMode::DrawOne, &above_cap),
SolverResult::Winnable,
"seed must be Winnable at 50 000 states — re-introducing the 5k cap would break this",
);
}
} }