From f0634e19bb0b0024025d5e41aae36946cc82cbb1 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Thu, 23 Jul 2026 12:51:45 +0200 Subject: [PATCH 01/16] Dispatch move-weight heuristics on the precomputed findex. Profiles showed call_heuristic as the second-hottest function: every per-suit call rebuilt the full HeuristicContext and re-derived hand_rel, trump state and voidness that MoveGen0/MoveGen123 already knew. The context is now built once per move-generation call and dispatched on the caller's findex. Solve drops from 16.97 to 15.09 ms/hand single-threaded on list1000 (2.9: 16.43); calc from 81.94 to 73.2 ms/hand on list100. Co-authored-by: Cursor --- .../heuristic_sorting/heuristic_sorting.cpp | 83 ++++---- .../heuristic_sorting/heuristic_sorting.hpp | 19 +- library/src/moves/moves.cpp | 82 +++++--- library/src/moves/moves.hpp | 16 +- library/tests/heuristic_sorting/BUILD.bazel | 11 ++ .../dispatch_findex_test.cpp | 186 ++++++++++++++++++ 6 files changed, 316 insertions(+), 81 deletions(-) create mode 100644 library/tests/heuristic_sorting/dispatch_findex_test.cpp diff --git a/library/src/heuristic_sorting/heuristic_sorting.cpp b/library/src/heuristic_sorting/heuristic_sorting.cpp index d175e451..d6176e44 100644 --- a/library/src/heuristic_sorting/heuristic_sorting.cpp +++ b/library/src/heuristic_sorting/heuristic_sorting.cpp @@ -2,8 +2,34 @@ #include #include -// New overload: accepts a pre-built HeuristicContext. This contains the -// same inline logic that used to be in the previous function body. +// Hot-path overload: the caller passes the dispatch case it already knows, +// so nothing is re-derived from the context here. +void call_heuristic(const HeuristicContext& context, const int findex) +{ + auto& ctx = const_cast(context); + switch (findex) { + case 0: weight_alloc_nt0(ctx); break; // leading, no trump winner + case 1: weight_alloc_trump0(ctx); break; // leading, trump game + case 4: weight_alloc_nt_notvoid1(ctx); break; // hand_rel=1, can follow, no trump + case 5: weight_alloc_trump_notvoid1(ctx); break; // hand_rel=1, can follow, trump + case 6: weight_alloc_nt_void1(ctx); break; // hand_rel=1, void, no trump + case 7: weight_alloc_trump_void1(ctx); break; // hand_rel=1, void, trump + case 8: weight_alloc_nt_notvoid2(ctx); break; // hand_rel=2, can follow, no trump + case 9: weight_alloc_trump_notvoid2(ctx); break; // hand_rel=2, can follow, trump + case 10: weight_alloc_nt_void2(ctx); break; // hand_rel=2, void, no trump + case 11: weight_alloc_trump_void2(ctx); break; // hand_rel=2, void, trump + case 12: weight_alloc_combined_notvoid3(ctx); break; // hand_rel=3, can follow, no trump + case 13: weight_alloc_combined_notvoid3(ctx); break; // hand_rel=3, can follow, trump + case 14: weight_alloc_nt_void3(ctx); break; // hand_rel=3, void, no trump + case 15: weight_alloc_trump_void3(ctx); break; // hand_rel=3, void, trump + default: + // Should not happen, but default to basic sorting + break; + } +} + +// Legacy overload: derives the dispatch case from the context, then +// delegates. Kept for callers/tests that do not have the findex at hand. void call_heuristic(const HeuristicContext& context) { // Determine which position in trick (0=leading, 1-3=following) @@ -13,57 +39,26 @@ void call_heuristic(const HeuristicContext& context) hand_rel = (context.curr_hand + 4 - context.lead_hand) % 4; } + // Check if trump game with trump winner available + const int ftest = ((context.trump != DDS_NOTRUMP) && + (context.trump >= 0 && context.trump < DDS_SUITS) && + (context.tpos.winner[context.trump].rank != 0) ? 1 : 0); + // Leading hand (hand_rel == 0) - MoveGen0 logic if (hand_rel == 0) { - // Check if trump game with trump winner available - bool trump_game = (context.trump != DDS_NOTRUMP) && - (context.trump >= 0 && context.trump < DDS_SUITS) && - (context.tpos.winner[context.trump].rank != 0); - - if (trump_game) { - weight_alloc_trump0(const_cast(context)); - } else { - weight_alloc_nt0(const_cast(context)); - } + call_heuristic(context, ftest); return; } // Following hands (hand_rel 1-3) - MoveGen123 logic - // Check trump game condition - int ftest = ((context.trump != DDS_NOTRUMP) && - (context.trump >= 0 && context.trump < DDS_SUITS) && - (context.tpos.winner[context.trump].rank != 0) ? 1 : 0); - // Check if current hand can follow suit (not void) - unsigned short ris = context.tpos.rank_in_suit[context.curr_hand][context.lead_suit]; - bool can_follow_suit = (ris != 0); + const unsigned short ris = + context.tpos.rank_in_suit[context.curr_hand][context.lead_suit]; + const bool can_follow_suit = (ris != 0); // Calculate function index using same logic as original - int findex; - if (can_follow_suit) { - findex = 4 * hand_rel + ftest; - } else { - findex = 4 * hand_rel + ftest + 2; - } - - // Following hands function dispatch table (MoveGen123 logic) - switch (findex) { - case 4: weight_alloc_nt_notvoid1(const_cast(context)); break; // hand_rel=1, can follow, no trump - case 5: weight_alloc_trump_notvoid1(const_cast(context)); break; // hand_rel=1, can follow, trump - case 6: weight_alloc_nt_void1(const_cast(context)); break; // hand_rel=1, void, no trump - case 7: weight_alloc_trump_void1(const_cast(context)); break; // hand_rel=1, void, trump - case 8: weight_alloc_nt_notvoid2(const_cast(context)); break; // hand_rel=2, can follow, no trump - case 9: weight_alloc_trump_notvoid2(const_cast(context)); break; // hand_rel=2, can follow, trump - case 10: weight_alloc_nt_void2(const_cast(context)); break; // hand_rel=2, void, no trump - case 11: weight_alloc_trump_void2(const_cast(context)); break; // hand_rel=2, void, trump - case 12: weight_alloc_combined_notvoid3(const_cast(context)); break; // hand_rel=3, can follow, no trump - case 13: weight_alloc_combined_notvoid3(const_cast(context)); break; // hand_rel=3, can follow, trump - case 14: weight_alloc_nt_void3(const_cast(context)); break; // hand_rel=3, void, no trump - case 15: weight_alloc_trump_void3(const_cast(context)); break; // hand_rel=3, void, trump - default: - // Should not happen, but default to basic sorting - break; - } + const int findex = 4 * hand_rel + ftest + (can_follow_suit ? 0 : 2); + call_heuristic(context, findex); } // The following functions are extracted from Moves.cpp and refactored to be diff --git a/library/src/heuristic_sorting/heuristic_sorting.hpp b/library/src/heuristic_sorting/heuristic_sorting.hpp index 83ed14df..62ec4087 100644 --- a/library/src/heuristic_sorting/heuristic_sorting.hpp +++ b/library/src/heuristic_sorting/heuristic_sorting.hpp @@ -35,7 +35,9 @@ struct HeuristicContext int num_moves; int last_num_moves; const int trump; - const int suit; // For MoveGen0, the suit being considered + // For MoveGen0, the suit being considered. Mutable so callers can build + // the context once per move generation and update it per suit iteration. + int suit; const TrackType* trackp; const int curr_trick; const int curr_hand; @@ -75,3 +77,18 @@ struct HeuristicContext /// construction overhead in hot paths. void call_heuristic(const HeuristicContext& context); +/// @brief Apply heuristic sorting using a precomputed dispatch index. +/// +/// Move generation already knows the dispatch case; passing it avoids +/// re-deriving the relative hand, trump state and voidness from the context +/// on every call (hot path). +/// +/// Encoding (matches the findex used by Moves::MoveGen123): +/// - Leading hand: 0 = no trump winner available, 1 = trump game +/// - Following hand: 4 * hand_rel + trump_game + (void_in_lead_suit ? 2 : 0) +/// for hand_rel in 1..3, i.e. values 4..15. +/// +/// @param context Pre-constructed HeuristicContext (see other overload). +/// @param findex Precomputed dispatch index as encoded above. +void call_heuristic(const HeuristicContext& context, int findex); + diff --git a/library/src/moves/moves.cpp b/library/src/moves/moves.cpp index 00a42101..389fc450 100644 --- a/library/src/moves/moves.cpp +++ b/library/src/moves/moves.cpp @@ -173,6 +173,14 @@ auto Moves::MoveGen0(const int tricks, const Pos &tpos, trackp->lowest_win[0][s] = 0; numMoves = 0; + // Leading-hand dispatch case, known here once instead of re-derived per + // suit inside the heuristic dispatcher. trump is always DDS_NOTRUMP or a + // valid suit at this point. + const int lead_findex = + ((trump != DDS_NOTRUMP) && (tpos.winner[trump].rank != 0)) ? 1 : 0; + HeuristicContext hctx = + make_heuristic_context(tpos, bestMove, bestMoveTT, thrp_rel); + for (suit = 0; suit < DDS_SUITS; suit++) { unsigned short ris = tpos.rank_in_suit[leadHand][suit]; if (ris == 0) @@ -202,12 +210,14 @@ auto Moves::MoveGen0(const int tricks, const Pos &tpos, g--; } - Moves::call_heuristic(tpos, bestMove, bestMoveTT, thrp_rel); + hctx.suit = suit; + hctx.last_num_moves = lastNumMoves; + hctx.num_moves = numMoves; + ::call_heuristic(hctx, lead_findex); } #ifdef DDS_MOVES - bool ftest = ((trump != DDS_NOTRUMP) && (tpos.winner[trump].rank != 0)); - if (ftest) + if (lead_findex) MG_REGISTER(MgType::TRUMP0, 0); else MG_REGISTER(MgType::NT0, 0); @@ -239,10 +249,14 @@ auto Moves::MoveGen123(const int tricks, const int handRel, const Pos &tpos) trackp->lowest_win[handRel][s] = 0; numMoves = 0; - [[maybe_unused]] int findex; + int findex; int ftest = ((trump != DDS_NOTRUMP) && (tpos.winner[trump].rank != 0) ? 1 : 0); + // Empty best-move placeholders must outlive the hoisted context, which + // holds references to them. + const MoveType empty_move{}; + unsigned short ris = tpos.rank_in_suit[currHand][leadSuit]; if (ris != 0) { @@ -274,7 +288,10 @@ auto Moves::MoveGen123(const int tricks, const int handRel, const Pos &tpos) list.last = numMoves - 1; if (numMoves == 1) return numMoves; - Moves::call_heuristic(tpos, MoveType{}, MoveType{}, nullptr); + + HeuristicContext hctx = + make_heuristic_context(tpos, empty_move, empty_move, nullptr); + ::call_heuristic(hctx, findex); Moves::MergeSort(); return numMoves; @@ -286,6 +303,9 @@ auto Moves::MoveGen123(const int tricks, const int handRel, const Pos &tpos) MG_REGISTER(RegisterList[findex], handRel); #endif + HeuristicContext hctx = + make_heuristic_context(tpos, empty_move, empty_move, nullptr); + for (suit = 0; suit < DDS_SUITS; suit++) { ris = tpos.rank_in_suit[currHand][suit]; if (ris == 0) @@ -311,7 +331,10 @@ auto Moves::MoveGen123(const int tricks, const int handRel, const Pos &tpos) g--; } - Moves::call_heuristic(tpos, MoveType{}, MoveType{}, nullptr); + hctx.suit = suit; + hctx.last_num_moves = lastNumMoves; + hctx.num_moves = numMoves; + ::call_heuristic(hctx, findex); } list.current = 0; @@ -640,36 +663,35 @@ auto Moves::Sort(const int tricks, const int relHand) -> void { } /** - * @brief Build a heuristic context and invoke heuristic sorting. + * @brief Build a heuristic context snapshot from current move-gen state. * - * Centralizes the construction of HeuristicContext to keep the call sites - * consistent and avoid exposing mutable state to heuristic helpers. + * Built once per move-generation call (not per suit); callers update the + * suit/num_moves/last_num_moves fields per iteration and dispatch with the + * findex they already computed, avoiding per-suit reconstruction and + * re-derivation of the dispatch case (hot path). */ -auto Moves::call_heuristic(const Pos &tpos, const MoveType &best_move, - const MoveType &best_move_tt, - const RelRanksType thrp_rel[]) -> void { - // Construct context once here and call the context-taking overload. +auto Moves::make_heuristic_context(const Pos &tpos, const MoveType &best_move, + const MoveType &best_move_tt, + const RelRanksType thrp_rel[]) const + -> HeuristicContext { HeuristicContext context{ tpos, best_move, best_move_tt, thrp_rel, mply, numMoves, lastNumMoves, trump, suit, trackp, currTrick, currHand, leadHand, leadSuit}; - // Snapshot removed_ranks into the context to avoid direct dependence on - // the mutable Moves::trackp buffer inside heuristic code. - for (int s = 0; s < DDS_SUITS; ++s) { - context.removed_ranks[s] = trackp ? trackp->removed_ranks[s] : 0; - } - // Snapshot minimal trick state for helper usage. - context.move1_rank = (trackp ? trackp->move[1].rank : 0); - context.high1 = (trackp ? trackp->high[1] : 0); - context.move1_suit = (trackp ? trackp->move[1].suit : 0); - // Third-hand snapshots - context.move2_rank = (trackp ? trackp->move[2].rank : 0); - context.move2_suit = (trackp ? trackp->move[2].suit : 0); - context.high2 = (trackp ? trackp->high[2] : 0); - // Leader snapshot - context.lead0_rank = (trackp ? trackp->move[0].rank : 0); - - ::call_heuristic(context); + // Snapshot removed_ranks and minimal trick state into the context to avoid + // direct dependence on the mutable Moves::trackp buffer inside heuristic + // code. trackp is always bound by MoveGen0/MoveGen123 before this runs. + const TrackType &tr = *trackp; + for (int s = 0; s < DDS_SUITS; ++s) + context.removed_ranks[s] = tr.removed_ranks[s]; + context.move1_rank = tr.move[1].rank; + context.high1 = tr.high[1]; + context.move1_suit = tr.move[1].suit; + context.move2_rank = tr.move[2].rank; + context.move2_suit = tr.move[2].suit; + context.high2 = tr.high[2]; + context.lead0_rank = tr.move[0].rank; + return context; } /** diff --git a/library/src/moves/moves.hpp b/library/src/moves/moves.hpp index 97dd55e0..a01e3be3 100644 --- a/library/src/moves/moves.hpp +++ b/library/src/moves/moves.hpp @@ -184,16 +184,20 @@ class Moves { auto MergeSort() -> void; /** - * @brief Invoke heuristic sorting for current move list. + * @brief Build a HeuristicContext snapshot from current move-gen state. + * + * Built once per move-generation call; the caller updates suit and move + * counts per suit iteration and dispatches via call_heuristic(ctx, findex). * * @param tpos Current position - * @param best_move Best move from search - * @param best_move_tt Best move from transposition table + * @param best_move Best move from search (must outlive the context) + * @param best_move_tt Best move from transposition table (must outlive the context) * @param thrp_rel Relative ranks per hand */ - auto call_heuristic(const Pos &tpos, const MoveType &best_move, - const MoveType &best_move_tt, const RelRanksType thrp_rel[]) - -> void; + auto make_heuristic_context(const Pos &tpos, const MoveType &best_move, + const MoveType &best_move_tt, + const RelRanksType thrp_rel[]) const + -> HeuristicContext; // (logging accessors removed) diff --git a/library/tests/heuristic_sorting/BUILD.bazel b/library/tests/heuristic_sorting/BUILD.bazel index 3f564dd7..695a1bb3 100644 --- a/library/tests/heuristic_sorting/BUILD.bazel +++ b/library/tests/heuristic_sorting/BUILD.bazel @@ -14,6 +14,17 @@ cc_test( ], ) +cc_test( + name = "dispatch_findex_test", + size = "small", + srcs = ["dispatch_findex_test.cpp"], + deps = [ + "//library/src/api:api_definitions", + "//library/src/heuristic_sorting:testable_heuristic_sorting", + "@googletest//:gtest_main", + ], +) + cc_test( name = "merge_scratch_test", size = "small", diff --git a/library/tests/heuristic_sorting/dispatch_findex_test.cpp b/library/tests/heuristic_sorting/dispatch_findex_test.cpp new file mode 100644 index 00000000..113a94cd --- /dev/null +++ b/library/tests/heuristic_sorting/dispatch_findex_test.cpp @@ -0,0 +1,186 @@ +/** + * @file dispatch_findex_test.cpp + * @brief Equivalence tests for findex-based heuristic dispatch. + * + * Move generation already knows the dispatch case (leading vs following + * hand, trump vs NT, void vs not void) as a small integer. The + * call_heuristic(ctx, findex) overload must select exactly the same weight + * function as the legacy overload that re-derives the case from the context, + * for every reachable findex. + */ + +#include +#include + +#include "heuristic_sorting/heuristic_sorting.hpp" +#include + +namespace +{ + +constexpr int kNumMoves = 4; + +struct DispatchFixture +{ + Pos tpos{}; + MoveType best_move{}; + MoveType best_move_tt{}; + RelRanksType rel[8192] = {}; + TrackType track{}; + MoveType moves_legacy[kNumMoves]{}; + MoveType moves_findex[kNumMoves]{}; +}; + +// Populate a deterministic pseudo-position that exercises the weight +// arithmetic (non-zero ranks, lengths, winners) without needing to be a +// fully consistent deal: both dispatchers run the same weight function on +// identical inputs, so equal outputs prove equal dispatch. +void fill_position(DispatchFixture& f, const bool trump_game, const bool is_void, + const int curr_hand, const int lead_suit) +{ + for (int h = 0; h < DDS_HANDS; h++) + { + for (int s = 0; s < DDS_SUITS; s++) + { + f.tpos.rank_in_suit[h][s] = + static_cast(0x0155u << ((h + s) % 3)); + f.tpos.length[h][s] = static_cast(3 + ((h + s) % 3)); + } + f.tpos.hand_dist[h] = 13; + } + for (int s = 0; s < DDS_SUITS; s++) + { + f.tpos.aggr[s] = 0x1FF5; + f.tpos.winner[s].rank = 14; + f.tpos.winner[s].hand = (s + 1) % DDS_HANDS; + f.tpos.second_best[s].rank = 13; + f.tpos.second_best[s].hand = (s + 2) % DDS_HANDS; + } + + const int trump = trump_game ? 1 : DDS_NOTRUMP; + if (trump_game) + f.tpos.winner[trump].rank = 14; + + if (is_void) + { + f.tpos.rank_in_suit[curr_hand][lead_suit] = 0; + f.tpos.length[curr_hand][lead_suit] = 0; + } + + f.track.lead_suit = lead_suit; + f.track.move[0] = ExtCard{lead_suit, 8, 0}; + f.track.move[1] = ExtCard{lead_suit, 10, 0}; + f.track.move[2] = ExtCard{lead_suit, 6, 0}; + f.track.high[1] = 1; + f.track.high[2] = 1; + for (int s = 0; s < DDS_SUITS; s++) + f.track.removed_ranks[s] = 0x0022; + + for (int k = 0; k < kNumMoves; k++) + { + const int suit = is_void ? ((lead_suit + 1) % DDS_SUITS) : lead_suit; + f.moves_legacy[k] = MoveType{suit, 12 - 2 * k, 0, 0}; + f.moves_findex[k] = f.moves_legacy[k]; + } +} + +HeuristicContext make_context(DispatchFixture& f, MoveType* mply, + const bool trump_game, const bool is_void, + const int curr_hand, const int lead_hand, + const int lead_suit) +{ + const int move_suit = is_void ? ((lead_suit + 1) % DDS_SUITS) : lead_suit; + HeuristicContext ctx = { + f.tpos, + f.best_move, + f.best_move_tt, + f.rel, + mply, + kNumMoves, // num_moves + 0, // last_num_moves + trump_game ? 1 : DDS_NOTRUMP, + move_suit, // suit under consideration + &f.track, + 5, // curr_trick + curr_hand, + lead_hand, + lead_suit, + }; + for (int s = 0; s < DDS_SUITS; s++) + ctx.removed_ranks[s] = f.track.removed_ranks[s]; + ctx.move1_rank = f.track.move[1].rank; + ctx.high1 = f.track.high[1]; + ctx.move1_suit = f.track.move[1].suit; + ctx.move2_rank = f.track.move[2].rank; + ctx.move2_suit = f.track.move[2].suit; + ctx.high2 = f.track.high[2]; + ctx.lead0_rank = f.track.move[0].rank; + return ctx; +} + +void expect_same_weights(const DispatchFixture& f, const int findex) +{ + for (int k = 0; k < kNumMoves; k++) + EXPECT_EQ(f.moves_legacy[k].weight, f.moves_findex[k].weight) + << "findex=" << findex << " move=" << k; +} + +} // namespace + +// Leading hand: findex 0 (NT) and 1 (trump) must match the legacy dispatch, +// which selects the weight function from the trump/winner state. +TEST(DispatchFindex, LeadHandMatchesLegacyDispatch) +{ + for (const bool trump_game : {false, true}) + { + DispatchFixture f; + const int lead_hand = 2; + fill_position(f, trump_game, /*is_void=*/false, lead_hand, /*lead_suit=*/0); + + HeuristicContext legacy = make_context( + f, f.moves_legacy, trump_game, false, lead_hand, lead_hand, 0); + HeuristicContext with_findex = make_context( + f, f.moves_findex, trump_game, false, lead_hand, lead_hand, 0); + + call_heuristic(legacy); + call_heuristic(with_findex, trump_game ? 1 : 0); + + expect_same_weights(f, trump_game ? 1 : 0); + } +} + +// Following hands: findex 4..15 (4*hand_rel + trump + 2*void) must match the +// legacy dispatch that re-derives hand_rel, trump state and voidness. +TEST(DispatchFindex, FollowingHandsMatchLegacyDispatchForAllFindexes) +{ + const int lead_hand = 1; + const int lead_suit = 2; + + for (int hand_rel = 1; hand_rel <= 3; hand_rel++) + { + for (const bool trump_game : {false, true}) + { + for (const bool is_void : {false, true}) + { + const int curr_hand = (lead_hand + hand_rel) % 4; + const int findex = + 4 * hand_rel + (trump_game ? 1 : 0) + (is_void ? 2 : 0); + + DispatchFixture f; + fill_position(f, trump_game, is_void, curr_hand, lead_suit); + + HeuristicContext legacy = make_context( + f, f.moves_legacy, trump_game, is_void, curr_hand, lead_hand, + lead_suit); + HeuristicContext with_findex = make_context( + f, f.moves_findex, trump_game, is_void, curr_hand, lead_hand, + lead_suit); + + call_heuristic(legacy); + call_heuristic(with_findex, findex); + + expect_same_weights(f, findex); + } + } + } +} From a6e6d343942ee87c37c2e94b326c62c822fd4225 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Thu, 23 Jul 2026 17:39:48 +0200 Subject: [PATCH 02/16] Fix findex encoding docs to match trump-winner semantics. Clarify that bit 0/1 means a trump winner is available, not merely a trump game, and pin the edge case with a dispatch equivalence test. Co-authored-by: Cursor --- .../heuristic_sorting/heuristic_sorting.cpp | 26 +++++------ .../heuristic_sorting/heuristic_sorting.hpp | 10 +++-- .../dispatch_findex_test.cpp | 43 +++++++++++++++++-- 3 files changed, 58 insertions(+), 21 deletions(-) diff --git a/library/src/heuristic_sorting/heuristic_sorting.cpp b/library/src/heuristic_sorting/heuristic_sorting.cpp index d6176e44..fce35abe 100644 --- a/library/src/heuristic_sorting/heuristic_sorting.cpp +++ b/library/src/heuristic_sorting/heuristic_sorting.cpp @@ -9,19 +9,19 @@ void call_heuristic(const HeuristicContext& context, const int findex) auto& ctx = const_cast(context); switch (findex) { case 0: weight_alloc_nt0(ctx); break; // leading, no trump winner - case 1: weight_alloc_trump0(ctx); break; // leading, trump game - case 4: weight_alloc_nt_notvoid1(ctx); break; // hand_rel=1, can follow, no trump - case 5: weight_alloc_trump_notvoid1(ctx); break; // hand_rel=1, can follow, trump - case 6: weight_alloc_nt_void1(ctx); break; // hand_rel=1, void, no trump - case 7: weight_alloc_trump_void1(ctx); break; // hand_rel=1, void, trump - case 8: weight_alloc_nt_notvoid2(ctx); break; // hand_rel=2, can follow, no trump - case 9: weight_alloc_trump_notvoid2(ctx); break; // hand_rel=2, can follow, trump - case 10: weight_alloc_nt_void2(ctx); break; // hand_rel=2, void, no trump - case 11: weight_alloc_trump_void2(ctx); break; // hand_rel=2, void, trump - case 12: weight_alloc_combined_notvoid3(ctx); break; // hand_rel=3, can follow, no trump - case 13: weight_alloc_combined_notvoid3(ctx); break; // hand_rel=3, can follow, trump - case 14: weight_alloc_nt_void3(ctx); break; // hand_rel=3, void, no trump - case 15: weight_alloc_trump_void3(ctx); break; // hand_rel=3, void, trump + case 1: weight_alloc_trump0(ctx); break; // leading, trump winner available + case 4: weight_alloc_nt_notvoid1(ctx); break; // hand_rel=1, can follow, no trump winner + case 5: weight_alloc_trump_notvoid1(ctx); break; // hand_rel=1, can follow, trump winner + case 6: weight_alloc_nt_void1(ctx); break; // hand_rel=1, void, no trump winner + case 7: weight_alloc_trump_void1(ctx); break; // hand_rel=1, void, trump winner + case 8: weight_alloc_nt_notvoid2(ctx); break; // hand_rel=2, can follow, no trump winner + case 9: weight_alloc_trump_notvoid2(ctx); break; // hand_rel=2, can follow, trump winner + case 10: weight_alloc_nt_void2(ctx); break; // hand_rel=2, void, no trump winner + case 11: weight_alloc_trump_void2(ctx); break; // hand_rel=2, void, trump winner + case 12: weight_alloc_combined_notvoid3(ctx); break; // hand_rel=3, can follow, no trump winner + case 13: weight_alloc_combined_notvoid3(ctx); break; // hand_rel=3, can follow, trump winner + case 14: weight_alloc_nt_void3(ctx); break; // hand_rel=3, void, no trump winner + case 15: weight_alloc_trump_void3(ctx); break; // hand_rel=3, void, trump winner default: // Should not happen, but default to basic sorting break; diff --git a/library/src/heuristic_sorting/heuristic_sorting.hpp b/library/src/heuristic_sorting/heuristic_sorting.hpp index 62ec4087..86d37d3d 100644 --- a/library/src/heuristic_sorting/heuristic_sorting.hpp +++ b/library/src/heuristic_sorting/heuristic_sorting.hpp @@ -83,10 +83,12 @@ void call_heuristic(const HeuristicContext& context); /// re-deriving the relative hand, trump state and voidness from the context /// on every call (hot path). /// -/// Encoding (matches the findex used by Moves::MoveGen123): -/// - Leading hand: 0 = no trump winner available, 1 = trump game -/// - Following hand: 4 * hand_rel + trump_game + (void_in_lead_suit ? 2 : 0) -/// for hand_rel in 1..3, i.e. values 4..15. +/// Encoding (matches the findex used by Moves::MoveGen0 / MoveGen123): +/// - Leading hand: 0 = no trump winner available, 1 = trump winner available +/// (trump != DDS_NOTRUMP && winner[trump].rank != 0) +/// - Following hand: 4 * hand_rel + trump_winner + (void_in_lead_suit ? 2 : 0) +/// for hand_rel in 1..3, i.e. values 4..15, where trump_winner is the +/// same 0/1 bit as for the leading hand. /// /// @param context Pre-constructed HeuristicContext (see other overload). /// @param findex Precomputed dispatch index as encoded above. diff --git a/library/tests/heuristic_sorting/dispatch_findex_test.cpp b/library/tests/heuristic_sorting/dispatch_findex_test.cpp index 113a94cd..c91c2d0b 100644 --- a/library/tests/heuristic_sorting/dispatch_findex_test.cpp +++ b/library/tests/heuristic_sorting/dispatch_findex_test.cpp @@ -127,8 +127,9 @@ void expect_same_weights(const DispatchFixture& f, const int findex) } // namespace -// Leading hand: findex 0 (NT) and 1 (trump) must match the legacy dispatch, -// which selects the weight function from the trump/winner state. +// Leading hand: findex 0 (NT) and 1 (trump winner available) must match the +// legacy dispatch, which selects the weight function from the trump/winner +// state — not merely whether a trump suit is set. TEST(DispatchFindex, LeadHandMatchesLegacyDispatch) { for (const bool trump_game : {false, true}) @@ -149,8 +150,42 @@ TEST(DispatchFindex, LeadHandMatchesLegacyDispatch) } } -// Following hands: findex 4..15 (4*hand_rel + trump + 2*void) must match the -// legacy dispatch that re-derives hand_rel, trump state and voidness. +// findex bit 0/1 is "trump winner available", not "trump suit set". +// A trump deal with winner[trump].rank == 0 must dispatch as findex 0. +TEST(DispatchFindex, LeadHandTrumpWithoutWinnerUsesFindex0) +{ + DispatchFixture f; + const int lead_hand = 2; + fill_position(f, /*trump_game=*/true, /*is_void=*/false, lead_hand, + /*lead_suit=*/0); + f.tpos.winner[1].rank = 0; + + HeuristicContext legacy = make_context( + f, f.moves_legacy, /*trump_game=*/true, false, lead_hand, lead_hand, 0); + HeuristicContext with_findex = make_context( + f, f.moves_findex, /*trump_game=*/true, false, lead_hand, lead_hand, 0); + + call_heuristic(legacy); + call_heuristic(with_findex, /*findex=*/0); + + expect_same_weights(f, /*findex=*/0); + + // Passing 1 ("trump game") would select the wrong weight function. + for (int k = 0; k < kNumMoves; k++) + f.moves_findex[k].weight = 0; + call_heuristic(with_findex, /*findex=*/1); + bool any_differ = false; + for (int k = 0; k < kNumMoves; k++) + { + if (f.moves_legacy[k].weight != f.moves_findex[k].weight) + any_differ = true; + } + EXPECT_TRUE(any_differ) + << "findex 1 must not match when trump is set but no winner is available"; +} + +// Following hands: findex 4..15 (4*hand_rel + trump_winner + 2*void) must match +// the legacy dispatch that re-derives hand_rel, trump-winner state and voidness. TEST(DispatchFindex, FollowingHandsMatchLegacyDispatchForAllFindexes) { const int lead_hand = 1; From cfc12babb2b5cf9ea1a1b9d20266043edf64a03f Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Thu, 23 Jul 2026 17:44:09 +0200 Subject: [PATCH 03/16] Require an explicit TrackType in make_heuristic_context. Avoid dereferencing the nullable Moves::trackp member so misuse is a compile-time error instead of undefined behavior. Co-authored-by: Cursor --- library/src/moves/moves.cpp | 20 +++++++++------ library/src/moves/moves.hpp | 5 +++- library/tests/moves/moves_test.cpp | 39 ++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 8 deletions(-) diff --git a/library/src/moves/moves.cpp b/library/src/moves/moves.cpp index 389fc450..591e657e 100644 --- a/library/src/moves/moves.cpp +++ b/library/src/moves/moves.cpp @@ -179,7 +179,8 @@ auto Moves::MoveGen0(const int tricks, const Pos &tpos, const int lead_findex = ((trump != DDS_NOTRUMP) && (tpos.winner[trump].rank != 0)) ? 1 : 0; HeuristicContext hctx = - make_heuristic_context(tpos, bestMove, bestMoveTT, thrp_rel); + make_heuristic_context(tpos, bestMove, bestMoveTT, thrp_rel, + track[tricks]); for (suit = 0; suit < DDS_SUITS; suit++) { unsigned short ris = tpos.rank_in_suit[leadHand][suit]; @@ -290,7 +291,8 @@ auto Moves::MoveGen123(const int tricks, const int handRel, const Pos &tpos) return numMoves; HeuristicContext hctx = - make_heuristic_context(tpos, empty_move, empty_move, nullptr); + make_heuristic_context(tpos, empty_move, empty_move, nullptr, + track[tricks]); ::call_heuristic(hctx, findex); Moves::MergeSort(); @@ -304,7 +306,8 @@ auto Moves::MoveGen123(const int tricks, const int handRel, const Pos &tpos) #endif HeuristicContext hctx = - make_heuristic_context(tpos, empty_move, empty_move, nullptr); + make_heuristic_context(tpos, empty_move, empty_move, nullptr, + track[tricks]); for (suit = 0; suit < DDS_SUITS; suit++) { ris = tpos.rank_in_suit[currHand][suit]; @@ -669,19 +672,22 @@ auto Moves::Sort(const int tricks, const int relHand) -> void { * suit/num_moves/last_num_moves fields per iteration and dispatch with the * findex they already computed, avoiding per-suit reconstruction and * re-derivation of the dispatch case (hot path). + * + * @param tr Bound trick track passed by the caller (MoveGen0/MoveGen123), + * never read from the nullable Moves::trackp member. */ auto Moves::make_heuristic_context(const Pos &tpos, const MoveType &best_move, const MoveType &best_move_tt, - const RelRanksType thrp_rel[]) const + const RelRanksType thrp_rel[], + const TrackType &tr) const -> HeuristicContext { HeuristicContext context{ tpos, best_move, best_move_tt, thrp_rel, mply, numMoves, lastNumMoves, - trump, suit, trackp, currTrick, currHand, leadHand, leadSuit}; + trump, suit, &tr, currTrick, currHand, leadHand, leadSuit}; // Snapshot removed_ranks and minimal trick state into the context to avoid // direct dependence on the mutable Moves::trackp buffer inside heuristic - // code. trackp is always bound by MoveGen0/MoveGen123 before this runs. - const TrackType &tr = *trackp; + // code. for (int s = 0; s < DDS_SUITS; ++s) context.removed_ranks[s] = tr.removed_ranks[s]; context.move1_rank = tr.move[1].rank; diff --git a/library/src/moves/moves.hpp b/library/src/moves/moves.hpp index a01e3be3..eb830da1 100644 --- a/library/src/moves/moves.hpp +++ b/library/src/moves/moves.hpp @@ -193,10 +193,13 @@ class Moves { * @param best_move Best move from search (must outlive the context) * @param best_move_tt Best move from transposition table (must outlive the context) * @param thrp_rel Relative ranks per hand + * @param tr Bound trick track (must outlive the context). Passed explicitly + * so callers cannot accidentally dereference a null Moves::trackp. */ auto make_heuristic_context(const Pos &tpos, const MoveType &best_move, const MoveType &best_move_tt, - const RelRanksType thrp_rel[]) const + const RelRanksType thrp_rel[], + const TrackType &tr) const -> HeuristicContext; // (logging accessors removed) diff --git a/library/tests/moves/moves_test.cpp b/library/tests/moves/moves_test.cpp index f36b97cc..afa3a898 100644 --- a/library/tests/moves/moves_test.cpp +++ b/library/tests/moves/moves_test.cpp @@ -262,6 +262,45 @@ TEST_F(MovesTest, MemorySafetyFeaturesArePresent) { EXPECT_FALSE(moves->funcName[0].empty()); // funcName array exists and is initialized } +/** + * make_heuristic_context must take an explicit TrackType rather than + * dereferencing Moves::trackp (which starts as nullptr). Callers pass the + * bound track; snapshots must come from that argument, not a nullable member. + */ +TEST_F(MovesTest, MakeHeuristicContextSnapshotsExplicitTrack) +{ + Pos tpos{}; + const MoveType best{}; + const MoveType best_tt{}; + TrackType tr{}; + for (int s = 0; s < DDS_SUITS; ++s) + tr.removed_ranks[s] = 0x10 + s; + tr.move[0].rank = 14; + tr.move[1].rank = 12; + tr.move[1].suit = 2; + tr.high[1] = 1; + tr.move[2].rank = 9; + tr.move[2].suit = 3; + tr.high[2] = 0; + + // trackp remains nullptr after construction — the API must not need it. + ASSERT_EQ(moves->trackp, nullptr); + + const HeuristicContext ctx = + moves->make_heuristic_context(tpos, best, best_tt, nullptr, tr); + + for (int s = 0; s < DDS_SUITS; ++s) + EXPECT_EQ(ctx.removed_ranks[s], 0x10 + s) << "suit=" << s; + EXPECT_EQ(ctx.lead0_rank, 14); + EXPECT_EQ(ctx.move1_rank, 12); + EXPECT_EQ(ctx.move1_suit, 2); + EXPECT_EQ(ctx.high1, 1); + EXPECT_EQ(ctx.move2_rank, 9); + EXPECT_EQ(ctx.move2_suit, 3); + EXPECT_EQ(ctx.high2, 0); + EXPECT_EQ(ctx.trackp, &tr); +} + /** * @section Performance Tests */ From 29730d0674e5b193f6f4f0637c6a107e20b34dd8 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Thu, 23 Jul 2026 17:47:47 +0200 Subject: [PATCH 04/16] Fall back to NT0 weights for invalid findex dispatch. An out-of-range findex previously left stale move weights untouched, which could make MergeSort ordering nondeterministic. Co-authored-by: Cursor --- .../heuristic_sorting/heuristic_sorting.cpp | 4 +- .../dispatch_findex_test.cpp | 39 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/library/src/heuristic_sorting/heuristic_sorting.cpp b/library/src/heuristic_sorting/heuristic_sorting.cpp index fce35abe..d57c079e 100644 --- a/library/src/heuristic_sorting/heuristic_sorting.cpp +++ b/library/src/heuristic_sorting/heuristic_sorting.cpp @@ -23,7 +23,9 @@ void call_heuristic(const HeuristicContext& context, const int findex) case 14: weight_alloc_nt_void3(ctx); break; // hand_rel=3, void, no trump winner case 15: weight_alloc_trump_void3(ctx); break; // hand_rel=3, void, trump winner default: - // Should not happen, but default to basic sorting + // Should not happen; fall back to NT leading weights so moves get + // deterministic ordering instead of stale/uninitialized weights. + weight_alloc_nt0(ctx); break; } } diff --git a/library/tests/heuristic_sorting/dispatch_findex_test.cpp b/library/tests/heuristic_sorting/dispatch_findex_test.cpp index c91c2d0b..7257bb0b 100644 --- a/library/tests/heuristic_sorting/dispatch_findex_test.cpp +++ b/library/tests/heuristic_sorting/dispatch_findex_test.cpp @@ -13,6 +13,7 @@ #include #include "heuristic_sorting/heuristic_sorting.hpp" +#include "heuristic_sorting/internal.hpp" #include namespace @@ -184,6 +185,44 @@ TEST(DispatchFindex, LeadHandTrumpWithoutWinnerUsesFindex0) << "findex 1 must not match when trump is set but no winner is available"; } +// Out-of-range findex must still assign deterministic weights (NT0 fallback), +// not leave stale/uninitialized move weights that would scramble MergeSort. +TEST(DispatchFindex, InvalidFindexFallsBackToBasicNt0Weights) +{ + for (const int bad_findex : {2, 3, 16, -1, 99}) + { + DispatchFixture f; + const int lead_hand = 0; + fill_position(f, /*trump_game=*/false, /*is_void=*/false, lead_hand, + /*lead_suit=*/0); + + constexpr int kStale = 0x7f0f0f0f; + for (int k = 0; k < kNumMoves; k++) + { + f.moves_legacy[k].weight = 0; + f.moves_findex[k].weight = kStale; + } + + HeuristicContext expected = make_context( + f, f.moves_legacy, /*trump_game=*/false, false, lead_hand, lead_hand, 0); + HeuristicContext with_findex = make_context( + f, f.moves_findex, /*trump_game=*/false, false, lead_hand, lead_hand, 0); + + weight_alloc_nt0(expected); + call_heuristic(with_findex, bad_findex); + + for (int k = 0; k < kNumMoves; k++) + { + EXPECT_NE(f.moves_findex[k].weight, kStale) + << "findex=" << bad_findex << " move=" << k + << " left a stale weight"; + EXPECT_EQ(f.moves_legacy[k].weight, f.moves_findex[k].weight) + << "findex=" << bad_findex << " move=" << k + << " must match weight_alloc_nt0 fallback"; + } + } +} + // Following hands: findex 4..15 (4*hand_rel + trump_winner + 2*void) must match // the legacy dispatch that re-derives hand_rel, trump-winner state and voidness. TEST(DispatchFindex, FollowingHandsMatchLegacyDispatchForAllFindexes) From f08a1e1e11ed372f91e95746f28fce4274059599 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Thu, 23 Jul 2026 19:14:58 +0200 Subject: [PATCH 05/16] Initialize Moves fields before make_heuristic_context in tests. The helper reads lead/curr/trump/suit/move-count members that the constructor leaves uninitialized; set them so the snapshot test avoids UB. Co-authored-by: Cursor --- library/tests/moves/moves_test.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/library/tests/moves/moves_test.cpp b/library/tests/moves/moves_test.cpp index afa3a898..6de3526f 100644 --- a/library/tests/moves/moves_test.cpp +++ b/library/tests/moves/moves_test.cpp @@ -286,6 +286,17 @@ TEST_F(MovesTest, MakeHeuristicContextSnapshotsExplicitTrack) // trackp remains nullptr after construction — the API must not need it. ASSERT_EQ(moves->trackp, nullptr); + // Initialize Moves state used by make_heuristic_context() to avoid reading + // uninitialized members in tests. + moves->leadHand = 0; + moves->currHand = 0; + moves->leadSuit = 0; + moves->currTrick = 0; + moves->trump = DDS_NOTRUMP; + moves->suit = 0; + moves->numMoves = 0; + moves->lastNumMoves = 0; + const HeuristicContext ctx = moves->make_heuristic_context(tpos, best, best_tt, nullptr, tr); From e7ad3307eb2eb9fa570ad331546b1a09966a9cc0 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Thu, 23 Jul 2026 19:16:00 +0200 Subject: [PATCH 06/16] Store RelRanksType table statically in dispatch fixture tests. Avoid ~1MB of per-fixture stack for a zero-initialized table that tests only need by pointer. Co-authored-by: Cursor --- library/tests/heuristic_sorting/dispatch_findex_test.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/tests/heuristic_sorting/dispatch_findex_test.cpp b/library/tests/heuristic_sorting/dispatch_findex_test.cpp index 7257bb0b..b28e5239 100644 --- a/library/tests/heuristic_sorting/dispatch_findex_test.cpp +++ b/library/tests/heuristic_sorting/dispatch_findex_test.cpp @@ -20,13 +20,15 @@ namespace { constexpr int kNumMoves = 4; +constexpr int kNumRelRanks = 8192; +static RelRanksType rel_ranks[kNumRelRanks] = {}; struct DispatchFixture { Pos tpos{}; MoveType best_move{}; MoveType best_move_tt{}; - RelRanksType rel[8192] = {}; + RelRanksType* rel = rel_ranks; TrackType track{}; MoveType moves_legacy[kNumMoves]{}; MoveType moves_findex[kNumMoves]{}; From 933d495b949061df502384b27274bcf5157d32dc Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Thu, 23 Jul 2026 20:10:00 +0200 Subject: [PATCH 07/16] Guard out-of-range trump and take mutable HeuristicContext in call_heuristic. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Match the legacy dispatcher’s suit-range check before indexing winner[trump] in MoveGen, and drop const_cast by making weighting overloads non-const. Co-authored-by: Cursor --- .../heuristic_sorting/heuristic_sorting.cpp | 35 ++++---- .../heuristic_sorting/heuristic_sorting.hpp | 10 ++- library/src/moves/moves.cpp | 25 ++++-- .../dispatch_findex_test.cpp | 47 ++++++++--- library/tests/moves/moves_test.cpp | 81 +++++++++++++++++++ specs/heuristic-sorting.md | 2 +- 6 files changed, 160 insertions(+), 40 deletions(-) diff --git a/library/src/heuristic_sorting/heuristic_sorting.cpp b/library/src/heuristic_sorting/heuristic_sorting.cpp index d57c079e..e9ae4557 100644 --- a/library/src/heuristic_sorting/heuristic_sorting.cpp +++ b/library/src/heuristic_sorting/heuristic_sorting.cpp @@ -4,35 +4,34 @@ // Hot-path overload: the caller passes the dispatch case it already knows, // so nothing is re-derived from the context here. -void call_heuristic(const HeuristicContext& context, const int findex) +void call_heuristic(HeuristicContext& context, const int findex) { - auto& ctx = const_cast(context); switch (findex) { - case 0: weight_alloc_nt0(ctx); break; // leading, no trump winner - case 1: weight_alloc_trump0(ctx); break; // leading, trump winner available - case 4: weight_alloc_nt_notvoid1(ctx); break; // hand_rel=1, can follow, no trump winner - case 5: weight_alloc_trump_notvoid1(ctx); break; // hand_rel=1, can follow, trump winner - case 6: weight_alloc_nt_void1(ctx); break; // hand_rel=1, void, no trump winner - case 7: weight_alloc_trump_void1(ctx); break; // hand_rel=1, void, trump winner - case 8: weight_alloc_nt_notvoid2(ctx); break; // hand_rel=2, can follow, no trump winner - case 9: weight_alloc_trump_notvoid2(ctx); break; // hand_rel=2, can follow, trump winner - case 10: weight_alloc_nt_void2(ctx); break; // hand_rel=2, void, no trump winner - case 11: weight_alloc_trump_void2(ctx); break; // hand_rel=2, void, trump winner - case 12: weight_alloc_combined_notvoid3(ctx); break; // hand_rel=3, can follow, no trump winner - case 13: weight_alloc_combined_notvoid3(ctx); break; // hand_rel=3, can follow, trump winner - case 14: weight_alloc_nt_void3(ctx); break; // hand_rel=3, void, no trump winner - case 15: weight_alloc_trump_void3(ctx); break; // hand_rel=3, void, trump winner + case 0: weight_alloc_nt0(context); break; // leading, no trump winner + case 1: weight_alloc_trump0(context); break; // leading, trump winner available + case 4: weight_alloc_nt_notvoid1(context); break; // hand_rel=1, can follow, no trump winner + case 5: weight_alloc_trump_notvoid1(context); break; // hand_rel=1, can follow, trump winner + case 6: weight_alloc_nt_void1(context); break; // hand_rel=1, void, no trump winner + case 7: weight_alloc_trump_void1(context); break; // hand_rel=1, void, trump winner + case 8: weight_alloc_nt_notvoid2(context); break; // hand_rel=2, can follow, no trump winner + case 9: weight_alloc_trump_notvoid2(context); break; // hand_rel=2, can follow, trump winner + case 10: weight_alloc_nt_void2(context); break; // hand_rel=2, void, no trump winner + case 11: weight_alloc_trump_void2(context); break; // hand_rel=2, void, trump winner + case 12: weight_alloc_combined_notvoid3(context); break; // hand_rel=3, can follow, no trump winner + case 13: weight_alloc_combined_notvoid3(context); break; // hand_rel=3, can follow, trump winner + case 14: weight_alloc_nt_void3(context); break; // hand_rel=3, void, no trump winner + case 15: weight_alloc_trump_void3(context); break; // hand_rel=3, void, trump winner default: // Should not happen; fall back to NT leading weights so moves get // deterministic ordering instead of stale/uninitialized weights. - weight_alloc_nt0(ctx); + weight_alloc_nt0(context); break; } } // Legacy overload: derives the dispatch case from the context, then // delegates. Kept for callers/tests that do not have the findex at hand. -void call_heuristic(const HeuristicContext& context) +void call_heuristic(HeuristicContext& context) { // Determine which position in trick (0=leading, 1-3=following) int hand_rel = 0; diff --git a/library/src/heuristic_sorting/heuristic_sorting.hpp b/library/src/heuristic_sorting/heuristic_sorting.hpp index 86d37d3d..6b6dc4e5 100644 --- a/library/src/heuristic_sorting/heuristic_sorting.hpp +++ b/library/src/heuristic_sorting/heuristic_sorting.hpp @@ -71,11 +71,11 @@ struct HeuristicContext /// /// @param context Pre-constructed HeuristicContext containing position data, /// move arrays, and cached snapshots for efficient evaluation. +/// Non-const because weighting writes through context.mply. /// -/// @note This function mutates the context's move weighting arrays. /// @note Prefer this overload over parameterized versions to minimize /// construction overhead in hot paths. -void call_heuristic(const HeuristicContext& context); +void call_heuristic(HeuristicContext& context); /// @brief Apply heuristic sorting using a precomputed dispatch index. /// @@ -85,12 +85,14 @@ void call_heuristic(const HeuristicContext& context); /// /// Encoding (matches the findex used by Moves::MoveGen0 / MoveGen123): /// - Leading hand: 0 = no trump winner available, 1 = trump winner available -/// (trump != DDS_NOTRUMP && winner[trump].rank != 0) +/// (trump in [0, DDS_SUITS) && trump != DDS_NOTRUMP && +/// winner[trump].rank != 0) /// - Following hand: 4 * hand_rel + trump_winner + (void_in_lead_suit ? 2 : 0) /// for hand_rel in 1..3, i.e. values 4..15, where trump_winner is the /// same 0/1 bit as for the leading hand. /// /// @param context Pre-constructed HeuristicContext (see other overload). +/// Non-const because weighting writes through context.mply. /// @param findex Precomputed dispatch index as encoded above. -void call_heuristic(const HeuristicContext& context, int findex); +void call_heuristic(HeuristicContext& context, int findex); diff --git a/library/src/moves/moves.cpp b/library/src/moves/moves.cpp index 591e657e..eb67454d 100644 --- a/library/src/moves/moves.cpp +++ b/library/src/moves/moves.cpp @@ -49,6 +49,22 @@ const MgType RegisterList[16] = {MgType::NT0, MgType::TRUMP0, #define MG_REGISTER(a, b) 1; #endif +namespace +{ + +// Same trump-winner bit as the legacy call_heuristic dispatcher: never index +// winner[trump] unless trump is a suit in [0, DDS_SUITS). +auto trump_winner_findex(const Pos& tpos, const int trump) -> int +{ + return ((trump != DDS_NOTRUMP) && + (trump >= 0 && trump < DDS_SUITS) && + (tpos.winner[trump].rank != 0)) + ? 1 + : 0; +} + +} // namespace + Moves::Moves() { // Initialize non-owning pointers to nullptr for safety trackp = nullptr; @@ -174,10 +190,8 @@ auto Moves::MoveGen0(const int tricks, const Pos &tpos, numMoves = 0; // Leading-hand dispatch case, known here once instead of re-derived per - // suit inside the heuristic dispatcher. trump is always DDS_NOTRUMP or a - // valid suit at this point. - const int lead_findex = - ((trump != DDS_NOTRUMP) && (tpos.winner[trump].rank != 0)) ? 1 : 0; + // suit inside the heuristic dispatcher. + const int lead_findex = trump_winner_findex(tpos, trump); HeuristicContext hctx = make_heuristic_context(tpos, bestMove, bestMoveTT, thrp_rel, track[tricks]); @@ -251,8 +265,7 @@ auto Moves::MoveGen123(const int tricks, const int handRel, const Pos &tpos) numMoves = 0; int findex; - int ftest = - ((trump != DDS_NOTRUMP) && (tpos.winner[trump].rank != 0) ? 1 : 0); + const int ftest = trump_winner_findex(tpos, trump); // Empty best-move placeholders must outlive the hoisted context, which // holds references to them. diff --git a/library/tests/heuristic_sorting/dispatch_findex_test.cpp b/library/tests/heuristic_sorting/dispatch_findex_test.cpp index b28e5239..10f9180a 100644 --- a/library/tests/heuristic_sorting/dispatch_findex_test.cpp +++ b/library/tests/heuristic_sorting/dispatch_findex_test.cpp @@ -10,7 +10,6 @@ */ #include -#include #include "heuristic_sorting/heuristic_sorting.hpp" #include "heuristic_sorting/internal.hpp" @@ -88,7 +87,7 @@ void fill_position(DispatchFixture& f, const bool trump_game, const bool is_void } HeuristicContext make_context(DispatchFixture& f, MoveType* mply, - const bool trump_game, const bool is_void, + const int trump, const bool is_void, const int curr_hand, const int lead_hand, const int lead_suit) { @@ -101,7 +100,7 @@ HeuristicContext make_context(DispatchFixture& f, MoveType* mply, mply, kNumMoves, // num_moves 0, // last_num_moves - trump_game ? 1 : DDS_NOTRUMP, + trump, move_suit, // suit under consideration &f.track, 5, // curr_trick @@ -141,10 +140,11 @@ TEST(DispatchFindex, LeadHandMatchesLegacyDispatch) const int lead_hand = 2; fill_position(f, trump_game, /*is_void=*/false, lead_hand, /*lead_suit=*/0); + const int trump = trump_game ? 1 : DDS_NOTRUMP; HeuristicContext legacy = make_context( - f, f.moves_legacy, trump_game, false, lead_hand, lead_hand, 0); + f, f.moves_legacy, trump, false, lead_hand, lead_hand, 0); HeuristicContext with_findex = make_context( - f, f.moves_findex, trump_game, false, lead_hand, lead_hand, 0); + f, f.moves_findex, trump, false, lead_hand, lead_hand, 0); call_heuristic(legacy); call_heuristic(with_findex, trump_game ? 1 : 0); @@ -153,6 +153,30 @@ TEST(DispatchFindex, LeadHandMatchesLegacyDispatch) } } +// Out-of-range trump must not index winner[trump]; treat as no trump winner +// (same range guard Moves::MoveGen0 / MoveGen123 must apply). +TEST(DispatchFindex, LegacyTreatsOutOfRangeTrumpAsNoWinner) +{ + // DDS_NOTRUMP == DDS_SUITS, so use values strictly outside [0, DDS_SUITS). + for (const int bad_trump : {-1, DDS_SUITS + 1, 99}) + { + DispatchFixture f; + const int lead_hand = 0; + fill_position(f, /*trump_game=*/false, /*is_void=*/false, lead_hand, + /*lead_suit=*/0); + + HeuristicContext legacy = make_context( + f, f.moves_legacy, bad_trump, false, lead_hand, lead_hand, 0); + HeuristicContext with_findex = make_context( + f, f.moves_findex, DDS_NOTRUMP, false, lead_hand, lead_hand, 0); + + call_heuristic(legacy); + call_heuristic(with_findex, /*findex=*/0); + + expect_same_weights(f, /*findex=*/0); + } +} + // findex bit 0/1 is "trump winner available", not "trump suit set". // A trump deal with winner[trump].rank == 0 must dispatch as findex 0. TEST(DispatchFindex, LeadHandTrumpWithoutWinnerUsesFindex0) @@ -164,9 +188,9 @@ TEST(DispatchFindex, LeadHandTrumpWithoutWinnerUsesFindex0) f.tpos.winner[1].rank = 0; HeuristicContext legacy = make_context( - f, f.moves_legacy, /*trump_game=*/true, false, lead_hand, lead_hand, 0); + f, f.moves_legacy, /*trump=*/1, false, lead_hand, lead_hand, 0); HeuristicContext with_findex = make_context( - f, f.moves_findex, /*trump_game=*/true, false, lead_hand, lead_hand, 0); + f, f.moves_findex, /*trump=*/1, false, lead_hand, lead_hand, 0); call_heuristic(legacy); call_heuristic(with_findex, /*findex=*/0); @@ -206,9 +230,9 @@ TEST(DispatchFindex, InvalidFindexFallsBackToBasicNt0Weights) } HeuristicContext expected = make_context( - f, f.moves_legacy, /*trump_game=*/false, false, lead_hand, lead_hand, 0); + f, f.moves_legacy, DDS_NOTRUMP, false, lead_hand, lead_hand, 0); HeuristicContext with_findex = make_context( - f, f.moves_findex, /*trump_game=*/false, false, lead_hand, lead_hand, 0); + f, f.moves_findex, DDS_NOTRUMP, false, lead_hand, lead_hand, 0); weight_alloc_nt0(expected); call_heuristic(with_findex, bad_findex); @@ -245,11 +269,12 @@ TEST(DispatchFindex, FollowingHandsMatchLegacyDispatchForAllFindexes) DispatchFixture f; fill_position(f, trump_game, is_void, curr_hand, lead_suit); + const int trump = trump_game ? 1 : DDS_NOTRUMP; HeuristicContext legacy = make_context( - f, f.moves_legacy, trump_game, is_void, curr_hand, lead_hand, + f, f.moves_legacy, trump, is_void, curr_hand, lead_hand, lead_suit); HeuristicContext with_findex = make_context( - f, f.moves_findex, trump_game, is_void, curr_hand, lead_hand, + f, f.moves_findex, trump, is_void, curr_hand, lead_hand, lead_suit); call_heuristic(legacy); diff --git a/library/tests/moves/moves_test.cpp b/library/tests/moves/moves_test.cpp index 6de3526f..0f7ced15 100644 --- a/library/tests/moves/moves_test.cpp +++ b/library/tests/moves/moves_test.cpp @@ -312,6 +312,87 @@ TEST_F(MovesTest, MakeHeuristicContextSnapshotsExplicitTrack) EXPECT_EQ(ctx.trackp, &tr); } +namespace +{ + +auto make_pos(const unsigned short (*rank_in_suit)[4]) -> Pos +{ + Pos tpos{}; + for (int h = 0; h < DDS_HANDS; ++h) + { + for (int s = 0; s < DDS_SUITS; ++s) + { + tpos.rank_in_suit[h][s] = rank_in_suit[h][s]; + tpos.length[h][s] = 13; + } + } + return tpos; +} + +auto move_list_weights(const Moves& m, const int tricks, const int hand_rel, + const int n) -> std::vector +{ + std::vector weights; + weights.reserve(static_cast(n)); + for (int i = 0; i < n; ++i) + weights.push_back(m.moveList[tricks][hand_rel].move[i].weight); + return weights; +} + +} // namespace + +// MoveGen0 must range-check trump before reading winner[trump], matching the +// legacy heuristic dispatcher. Out-of-range values behave like no-trump. +TEST_F(MovesTest, MoveGen0OutOfRangeTrumpMatchesNoTrump) +{ + static RelRanksType rel[8192] = {}; + constexpr int tricks = 5; + const unsigned short (*rank_in_suit)[4] = getSampleRankInSuit(); + const Pos tpos = make_pos(rank_in_suit); + const MoveType best{}; + + auto run = [&](const int trump) { + auto local = std::make_unique(); + local->Init(tricks, 0, nullptr, nullptr, rank_in_suit, trump, 0); + const int n = local->MoveGen0(tricks, tpos, best, best, rel); + return move_list_weights(*local, tricks, 0, n); + }; + + const auto nt = run(DDS_NOTRUMP); + // DDS_NOTRUMP == DDS_SUITS, so use values strictly outside [0, DDS_SUITS). + for (const int bad_trump : {-1, DDS_SUITS + 1, 99}) + { + const auto bad = run(bad_trump); + EXPECT_EQ(bad, nt) << "trump=" << bad_trump; + } +} + +// MoveGen123 likewise must not index winner[trump] when trump is out of range. +TEST_F(MovesTest, MoveGen123OutOfRangeTrumpMatchesNoTrump) +{ + constexpr int tricks = 5; + constexpr int hand_rel = 1; + constexpr int lead_suit = 0; + const unsigned short (*rank_in_suit)[4] = getSampleRankInSuit(); + const Pos tpos = make_pos(rank_in_suit); + + auto run = [&](const int trump) { + auto local = std::make_unique(); + local->Init(tricks, 0, nullptr, nullptr, rank_in_suit, trump, 0); + local->track[tricks].lead_suit = lead_suit; + local->track[tricks].move[0] = ExtCard{lead_suit, 8, 0}; + const int n = local->MoveGen123(tricks, hand_rel, tpos); + return move_list_weights(*local, tricks, hand_rel, n); + }; + + const auto nt = run(DDS_NOTRUMP); + for (const int bad_trump : {-1, DDS_SUITS + 1, 99}) + { + const auto bad = run(bad_trump); + EXPECT_EQ(bad, nt) << "trump=" << bad_trump; + } +} + /** * @section Performance Tests */ diff --git a/specs/heuristic-sorting.md b/specs/heuristic-sorting.md index 3b268593..7fd10223 100644 --- a/specs/heuristic-sorting.md +++ b/specs/heuristic-sorting.md @@ -53,7 +53,7 @@ solve. ## Key entry points - `library/src/heuristic_sorting/heuristic_sorting.hpp` — `HeuristicContext`, - `TrackType`, and `call_heuristic(const HeuristicContext&)`. Doxygen documents the + `TrackType`, and `call_heuristic(HeuristicContext&)`. Doxygen documents the fields. - `library/src/heuristic_sorting/heuristic_sorting.cpp` + `internal.hpp` — the per-situation weighting helpers. From 631cb5534751ef92faf6e28db70f5756e11410b5 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Thu, 23 Jul 2026 20:27:19 +0200 Subject: [PATCH 08/16] Initialize Moves scalars before hoisting heuristic context. make_heuristic_context snapshots suit/lastNumMoves/leadSuit; define them in the constructor and reset them in MoveGen0/123 so the hoist does not read indeterminate values. Co-authored-by: Cursor --- library/src/moves/moves.cpp | 20 +++++++++++ library/tests/moves/moves_test.cpp | 55 ++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/library/src/moves/moves.cpp b/library/src/moves/moves.cpp index eb67454d..0bb161dd 100644 --- a/library/src/moves/moves.cpp +++ b/library/src/moves/moves.cpp @@ -70,6 +70,16 @@ Moves::Moves() { trackp = nullptr; mply = nullptr; + // Scalars snapshotted by make_heuristic_context must not be indeterminate. + leadHand = 0; + currHand = 0; + leadSuit = 0; + currTrick = 0; + trump = DDS_NOTRUMP; + suit = 0; + numMoves = 0; + lastNumMoves = 0; + funcName[static_cast(MgType::NT0)] = "NT0"; funcName[static_cast(MgType::TRUMP0)] = "Trump0"; funcName[static_cast(MgType::NT_VOID1)] = "NT_Void1"; @@ -187,7 +197,12 @@ auto Moves::MoveGen0(const int tricks, const Pos &tpos, mply = list.move; for (int s = 0; s < DDS_SUITS; s++) trackp->lowest_win[0][s] = 0; + // Reset fields snapshotted by make_heuristic_context before the hoist; + // suit/lastNumMoves are overwritten again before each call_heuristic. numMoves = 0; + lastNumMoves = 0; + suit = 0; + leadSuit = 0; // Leading-hand dispatch case, known here once instead of re-derived per // suit inside the heuristic dispatcher. @@ -262,7 +277,12 @@ auto Moves::MoveGen123(const int tricks, const int handRel, const Pos &tpos) for (int s = 0; s < DDS_SUITS; s++) trackp->lowest_win[handRel][s] = 0; + // Reset fields snapshotted by make_heuristic_context before either hoist. + // Follow-suit uses suit == leadSuit; the void path overwrites suit per + // iteration before call_heuristic. numMoves = 0; + lastNumMoves = 0; + suit = leadSuit; int findex; const int ftest = trump_winner_findex(tpos, trump); diff --git a/library/tests/moves/moves_test.cpp b/library/tests/moves/moves_test.cpp index 0f7ced15..001069b7 100644 --- a/library/tests/moves/moves_test.cpp +++ b/library/tests/moves/moves_test.cpp @@ -67,6 +67,17 @@ TEST_F(MovesTest, ConstructorInitializesState) { // Verify statistics are zeroed EXPECT_EQ(moves->trickFuncTable.nfuncs, 0); EXPECT_EQ(moves->trickFuncSuitTable.nfuncs, 0); + + // make_heuristic_context snapshots these; they must not be indeterminate + // after construction (and MoveGen0/123 re-set them before hoisting a context). + EXPECT_EQ(moves->leadHand, 0); + EXPECT_EQ(moves->currHand, 0); + EXPECT_EQ(moves->leadSuit, 0); + EXPECT_EQ(moves->currTrick, 0); + EXPECT_EQ(moves->trump, DDS_NOTRUMP); + EXPECT_EQ(moves->suit, 0); + EXPECT_EQ(moves->numMoves, 0); + EXPECT_EQ(moves->lastNumMoves, 0); } TEST_F(MovesTest, InitializesTrackingState) { @@ -393,6 +404,50 @@ TEST_F(MovesTest, MoveGen123OutOfRangeTrumpMatchesNoTrump) } } +// Hoisted make_heuristic_context must not observe stale suit/lastNumMoves from +// a prior call. Poison those members; MoveGen should reset them before +// snapshotting so weights match a clean instance. +TEST_F(MovesTest, MoveGenResetsSnapshottedFieldsBeforeHeuristicContext) +{ + static RelRanksType rel[8192] = {}; + constexpr int tricks = 5; + constexpr int lead_suit = 0; + const unsigned short (*rank_in_suit)[4] = getSampleRankInSuit(); + Pos tpos = make_pos(rank_in_suit); + // Force a void-in-lead path for hand_rel=1 so suit/lastNumMoves are used + // by the void weight functions after the hoisted context is built. + tpos.rank_in_suit[1][lead_suit] = 0; + tpos.length[1][lead_suit] = 0; + const MoveType best{}; + + auto run_gen0 = [&](Moves& m) { + m.Init(tricks, 0, nullptr, nullptr, rank_in_suit, DDS_NOTRUMP, 0); + return move_list_weights( + m, tricks, 0, m.MoveGen0(tricks, tpos, best, best, rel)); + }; + auto run_gen123 = [&](Moves& m) { + m.Init(tricks, 0, nullptr, nullptr, rank_in_suit, DDS_NOTRUMP, 0); + m.track[tricks].lead_suit = lead_suit; + m.track[tricks].move[0] = ExtCard{lead_suit, 8, 0}; + return move_list_weights( + m, tricks, 1, m.MoveGen123(tricks, /*handRel=*/1, tpos)); + }; + + Moves clean; + const auto gen0_clean = run_gen0(clean); + const auto gen123_clean = run_gen123(clean); + + Moves poisoned; + poisoned.suit = 3; + poisoned.lastNumMoves = 99; + poisoned.leadSuit = 2; + EXPECT_EQ(run_gen0(poisoned), gen0_clean); + poisoned.suit = 3; + poisoned.lastNumMoves = 99; + poisoned.leadSuit = 2; + EXPECT_EQ(run_gen123(poisoned), gen123_clean); +} + /** * @section Performance Tests */ From 3d3b62e6a72c70eb1f7d783287dfad7f34912c40 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Thu, 23 Jul 2026 20:47:16 +0200 Subject: [PATCH 09/16] Document both call_heuristic overloads in the heuristic-sorting spec. Align the behaviour bullet with the mutable HeuristicContext& API and the legacy vs findex dispatch forms. Co-authored-by: Cursor --- specs/heuristic-sorting.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/specs/heuristic-sorting.md b/specs/heuristic-sorting.md index 7fd10223..e15c756d 100644 --- a/specs/heuristic-sorting.md +++ b/specs/heuristic-sorting.md @@ -1,7 +1,7 @@ --- capability: heuristic-sorting owners: [heuristic_sorting] -last-updated: 2026-07-18 +last-updated: 2026-07-23 --- # Heuristic Sorting @@ -37,12 +37,14 @@ solve. `doc/heuristic-sorting.md` is narrative intent (including lead/void per-suit top-card bonuses) and may lag the code; when they disagree, trust the implementation. -- **`call_heuristic` takes a pre-built `HeuristicContext`.** The caller constructs +- **`call_heuristic` takes a pre-built `HeuristicContext&`.** The caller constructs one context (position, move array, best moves, relative ranks, and cached per-trick snapshots such as `removed_ranks`, `move1_rank`, `high1`) and passes - it by const reference. Implementations `const_cast` and mutate move weights - in place. There is a single free overload — the older parameterized form is - gone. + it by mutable reference; weighting writes through `context.mply`. Two free + overloads exist: a legacy form that derives the dispatch case from the + context, and a hot-path form that takes a precomputed `findex` from move + generation (`MoveGen0` / `MoveGen123`) so that case is not re-derived per + suit. - **`TrackType` is shared per-trick position state** defined here and consumed by both [move-generation](move-generation.md) and the heuristics. `Moves` owns the `track[]` array and mutates `trackp`; heuristics read the snapshots copied into @@ -53,8 +55,8 @@ solve. ## Key entry points - `library/src/heuristic_sorting/heuristic_sorting.hpp` — `HeuristicContext`, - `TrackType`, and `call_heuristic(HeuristicContext&)`. Doxygen documents the - fields. + `TrackType`, and both `call_heuristic` overloads (`HeuristicContext&`, and + with `findex`). Doxygen documents the fields. - `library/src/heuristic_sorting/heuristic_sorting.cpp` + `internal.hpp` — the per-situation weighting helpers. - Narrative algorithm: `doc/heuristic-sorting.md`. From 358c1cad55265aa5cb59e9760c5cb7518d29d37c Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Thu, 23 Jul 2026 20:56:32 +0200 Subject: [PATCH 10/16] Use a compact deal in MoveGen tests to avoid move-list overflow. The previous 0x3fff-everywhere fixture wrote past MovePlyType::move[14] and failed under ASan. Co-authored-by: Cursor --- library/tests/moves/moves_test.cpp | 69 +++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 20 deletions(-) diff --git a/library/tests/moves/moves_test.cpp b/library/tests/moves/moves_test.cpp index 001069b7..61091427 100644 --- a/library/tests/moves/moves_test.cpp +++ b/library/tests/moves/moves_test.cpp @@ -326,23 +326,54 @@ TEST_F(MovesTest, MakeHeuristicContextSnapshotsExplicitTrack) namespace { -auto make_pos(const unsigned short (*rank_in_suit)[4]) -> Pos +// Compact deal for MoveGen*: lead hand has only a few cards so the move list +// stays within MovePlyType::move[14]. The Init-only sample (0x3fff in every +// suit) overflows that buffer and trips ASan. +struct CompactDeal { + unsigned short rank_in_suit[DDS_HANDS][DDS_SUITS]{}; Pos tpos{}; +}; + +auto popcount16(unsigned short bits) -> int +{ + int n = 0; + for (; bits != 0; bits = static_cast(bits & (bits - 1))) + ++n; + return n; +} + +auto make_compact_deal() -> CompactDeal +{ + CompactDeal d{}; + // Hand 0 (lead): AK of suit 0, Q of suit 1. + d.rank_in_suit[0][0] = 0x1800; + d.rank_in_suit[0][1] = 0x0400; + // Other hands: one card each for length/winner lookups. + d.rank_in_suit[1][0] = 0x0200; + d.rank_in_suit[1][2] = 0x0100; + d.rank_in_suit[2][0] = 0x0080; + d.rank_in_suit[3][1] = 0x0040; + for (int h = 0; h < DDS_HANDS; ++h) { for (int s = 0; s < DDS_SUITS; ++s) { - tpos.rank_in_suit[h][s] = rank_in_suit[h][s]; - tpos.length[h][s] = 13; + d.tpos.rank_in_suit[h][s] = d.rank_in_suit[h][s]; + d.tpos.length[h][s] = + static_cast(popcount16(d.rank_in_suit[h][s])); + d.tpos.aggr[s] = + static_cast(d.tpos.aggr[s] | d.rank_in_suit[h][s]); } } - return tpos; + return d; } auto move_list_weights(const Moves& m, const int tricks, const int hand_rel, const int n) -> std::vector { + EXPECT_GT(n, 0); + EXPECT_LE(n, 14); std::vector weights; weights.reserve(static_cast(n)); for (int i = 0; i < n; ++i) @@ -358,14 +389,13 @@ TEST_F(MovesTest, MoveGen0OutOfRangeTrumpMatchesNoTrump) { static RelRanksType rel[8192] = {}; constexpr int tricks = 5; - const unsigned short (*rank_in_suit)[4] = getSampleRankInSuit(); - const Pos tpos = make_pos(rank_in_suit); + const CompactDeal deal = make_compact_deal(); const MoveType best{}; auto run = [&](const int trump) { auto local = std::make_unique(); - local->Init(tricks, 0, nullptr, nullptr, rank_in_suit, trump, 0); - const int n = local->MoveGen0(tricks, tpos, best, best, rel); + local->Init(tricks, 0, nullptr, nullptr, deal.rank_in_suit, trump, 0); + const int n = local->MoveGen0(tricks, deal.tpos, best, best, rel); return move_list_weights(*local, tricks, 0, n); }; @@ -384,15 +414,14 @@ TEST_F(MovesTest, MoveGen123OutOfRangeTrumpMatchesNoTrump) constexpr int tricks = 5; constexpr int hand_rel = 1; constexpr int lead_suit = 0; - const unsigned short (*rank_in_suit)[4] = getSampleRankInSuit(); - const Pos tpos = make_pos(rank_in_suit); + const CompactDeal deal = make_compact_deal(); auto run = [&](const int trump) { auto local = std::make_unique(); - local->Init(tricks, 0, nullptr, nullptr, rank_in_suit, trump, 0); + local->Init(tricks, 0, nullptr, nullptr, deal.rank_in_suit, trump, 0); local->track[tricks].lead_suit = lead_suit; local->track[tricks].move[0] = ExtCard{lead_suit, 8, 0}; - const int n = local->MoveGen123(tricks, hand_rel, tpos); + const int n = local->MoveGen123(tricks, hand_rel, deal.tpos); return move_list_weights(*local, tricks, hand_rel, n); }; @@ -412,25 +441,25 @@ TEST_F(MovesTest, MoveGenResetsSnapshottedFieldsBeforeHeuristicContext) static RelRanksType rel[8192] = {}; constexpr int tricks = 5; constexpr int lead_suit = 0; - const unsigned short (*rank_in_suit)[4] = getSampleRankInSuit(); - Pos tpos = make_pos(rank_in_suit); + CompactDeal deal = make_compact_deal(); // Force a void-in-lead path for hand_rel=1 so suit/lastNumMoves are used // by the void weight functions after the hoisted context is built. - tpos.rank_in_suit[1][lead_suit] = 0; - tpos.length[1][lead_suit] = 0; + deal.rank_in_suit[1][lead_suit] = 0; + deal.tpos.rank_in_suit[1][lead_suit] = 0; + deal.tpos.length[1][lead_suit] = 0; const MoveType best{}; auto run_gen0 = [&](Moves& m) { - m.Init(tricks, 0, nullptr, nullptr, rank_in_suit, DDS_NOTRUMP, 0); + m.Init(tricks, 0, nullptr, nullptr, deal.rank_in_suit, DDS_NOTRUMP, 0); return move_list_weights( - m, tricks, 0, m.MoveGen0(tricks, tpos, best, best, rel)); + m, tricks, 0, m.MoveGen0(tricks, deal.tpos, best, best, rel)); }; auto run_gen123 = [&](Moves& m) { - m.Init(tricks, 0, nullptr, nullptr, rank_in_suit, DDS_NOTRUMP, 0); + m.Init(tricks, 0, nullptr, nullptr, deal.rank_in_suit, DDS_NOTRUMP, 0); m.track[tricks].lead_suit = lead_suit; m.track[tricks].move[0] = ExtCard{lead_suit, 8, 0}; return move_list_weights( - m, tricks, 1, m.MoveGen123(tricks, /*handRel=*/1, tpos)); + m, tricks, 1, m.MoveGen123(tricks, /*handRel=*/1, deal.tpos)); }; Moves clean; From e657e5046dd93337219e4ad77c1954252db14853 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Thu, 23 Jul 2026 20:05:19 +0100 Subject: [PATCH 11/16] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- library/src/heuristic_sorting/heuristic_sorting.hpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/src/heuristic_sorting/heuristic_sorting.hpp b/library/src/heuristic_sorting/heuristic_sorting.hpp index 6b6dc4e5..6e80e47f 100644 --- a/library/src/heuristic_sorting/heuristic_sorting.hpp +++ b/library/src/heuristic_sorting/heuristic_sorting.hpp @@ -73,8 +73,9 @@ struct HeuristicContext /// move arrays, and cached snapshots for efficient evaluation. /// Non-const because weighting writes through context.mply. /// -/// @note Prefer this overload over parameterized versions to minimize -/// construction overhead in hot paths. +/// @note Prefer the `call_heuristic(context, findex)` overload when the dispatch +/// index is already known (hot path). Use this overload when callers do +/// not have `findex` available. void call_heuristic(HeuristicContext& context); /// @brief Apply heuristic sorting using a precomputed dispatch index. From d4e87f6d2d26b147774c55a88d42b27a975c5b1e Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Thu, 23 Jul 2026 21:10:22 +0200 Subject: [PATCH 12/16] Snapshot only defined trick-card fields in make_heuristic_context. Gate lead/move1/move2 copies on hand_rel so MoveGen0 and early following hands do not read unpopulated TrackType slots. Co-authored-by: Cursor --- library/src/moves/moves.cpp | 30 +++++--- library/tests/moves/moves_test.cpp | 115 +++++++++++++++++++++-------- 2 files changed, 106 insertions(+), 39 deletions(-) diff --git a/library/src/moves/moves.cpp b/library/src/moves/moves.cpp index 0bb161dd..c7b49dc1 100644 --- a/library/src/moves/moves.cpp +++ b/library/src/moves/moves.cpp @@ -718,18 +718,28 @@ auto Moves::make_heuristic_context(const Pos &tpos, const MoveType &best_move, tpos, best_move, best_move_tt, thrp_rel, mply, numMoves, lastNumMoves, trump, suit, &tr, currTrick, currHand, leadHand, leadSuit}; - // Snapshot removed_ranks and minimal trick state into the context to avoid - // direct dependence on the mutable Moves::trackp buffer inside heuristic - // code. + // Snapshot removed_ranks and only those trick-card fields that are defined + // for the current relative hand. Earlier slots may be unpopulated (MoveGen0 + // / hand_rel 1–2), so leave the rest at HeuristicContext's zero defaults. for (int s = 0; s < DDS_SUITS; ++s) context.removed_ranks[s] = tr.removed_ranks[s]; - context.move1_rank = tr.move[1].rank; - context.high1 = tr.high[1]; - context.move1_suit = tr.move[1].suit; - context.move2_rank = tr.move[2].rank; - context.move2_suit = tr.move[2].suit; - context.high2 = tr.high[2]; - context.lead0_rank = tr.move[0].rank; + + const int hand_rel = + (currHand == leadHand) ? 0 : (currHand + 4 - leadHand) % 4; + if (hand_rel >= 1) + context.lead0_rank = tr.move[0].rank; + if (hand_rel >= 2) + { + context.move1_rank = tr.move[1].rank; + context.move1_suit = tr.move[1].suit; + context.high1 = tr.high[1]; + } + if (hand_rel >= 3) + { + context.move2_rank = tr.move[2].rank; + context.move2_suit = tr.move[2].suit; + context.high2 = tr.high[2]; + } return context; } diff --git a/library/tests/moves/moves_test.cpp b/library/tests/moves/moves_test.cpp index 61091427..713d680f 100644 --- a/library/tests/moves/moves_test.cpp +++ b/library/tests/moves/moves_test.cpp @@ -273,6 +273,43 @@ TEST_F(MovesTest, MemorySafetyFeaturesArePresent) { EXPECT_FALSE(moves->funcName[0].empty()); // funcName array exists and is initialized } +namespace +{ + +auto poisoned_track() -> TrackType +{ + TrackType tr{}; + for (int s = 0; s < DDS_SUITS; ++s) + tr.removed_ranks[s] = 0x10 + s; + // Non-zero trick slots: only those defined for the current hand_rel should + // appear in the HeuristicContext snapshot. + tr.move[0] = ExtCard{0, 14, 0}; + tr.move[1] = ExtCard{2, 12, 0}; + tr.high[1] = 1; + tr.move[2] = ExtCard{3, 9, 0}; + tr.high[2] = 2; + return tr; +} + +auto context_for_hand_rel(Moves& m, const TrackType& tr, const int hand_rel) + -> HeuristicContext +{ + Pos tpos{}; + const MoveType best{}; + const MoveType best_tt{}; + m.leadHand = 0; + m.currHand = hand_rel; // leadHand 0 ⇒ hand_rel == currHand + m.leadSuit = 0; + m.currTrick = 0; + m.trump = DDS_NOTRUMP; + m.suit = 0; + m.numMoves = 0; + m.lastNumMoves = 0; + return m.make_heuristic_context(tpos, best, best_tt, nullptr, tr); +} + +} // namespace + /** * make_heuristic_context must take an explicit TrackType rather than * dereferencing Moves::trackp (which starts as nullptr). Callers pass the @@ -280,36 +317,10 @@ TEST_F(MovesTest, MemorySafetyFeaturesArePresent) { */ TEST_F(MovesTest, MakeHeuristicContextSnapshotsExplicitTrack) { - Pos tpos{}; - const MoveType best{}; - const MoveType best_tt{}; - TrackType tr{}; - for (int s = 0; s < DDS_SUITS; ++s) - tr.removed_ranks[s] = 0x10 + s; - tr.move[0].rank = 14; - tr.move[1].rank = 12; - tr.move[1].suit = 2; - tr.high[1] = 1; - tr.move[2].rank = 9; - tr.move[2].suit = 3; - tr.high[2] = 0; - - // trackp remains nullptr after construction — the API must not need it. + const TrackType tr = poisoned_track(); ASSERT_EQ(moves->trackp, nullptr); - // Initialize Moves state used by make_heuristic_context() to avoid reading - // uninitialized members in tests. - moves->leadHand = 0; - moves->currHand = 0; - moves->leadSuit = 0; - moves->currTrick = 0; - moves->trump = DDS_NOTRUMP; - moves->suit = 0; - moves->numMoves = 0; - moves->lastNumMoves = 0; - - const HeuristicContext ctx = - moves->make_heuristic_context(tpos, best, best_tt, nullptr, tr); + const HeuristicContext ctx = context_for_hand_rel(*moves, tr, /*hand_rel=*/3); for (int s = 0; s < DDS_SUITS; ++s) EXPECT_EQ(ctx.removed_ranks[s], 0x10 + s) << "suit=" << s; @@ -319,10 +330,56 @@ TEST_F(MovesTest, MakeHeuristicContextSnapshotsExplicitTrack) EXPECT_EQ(ctx.high1, 1); EXPECT_EQ(ctx.move2_rank, 9); EXPECT_EQ(ctx.move2_suit, 3); - EXPECT_EQ(ctx.high2, 0); + EXPECT_EQ(ctx.high2, 2); EXPECT_EQ(ctx.trackp, &tr); } +// Leading hand: move[0..2] are not played yet — leave trick snapshots at 0 +// even if the TrackType buffer holds stale/poisoned values. +TEST_F(MovesTest, MakeHeuristicContextLeadHandOmitsUnsetTrickCards) +{ + const TrackType tr = poisoned_track(); + const HeuristicContext ctx = context_for_hand_rel(*moves, tr, /*hand_rel=*/0); + + EXPECT_EQ(ctx.lead0_rank, 0); + EXPECT_EQ(ctx.move1_rank, 0); + EXPECT_EQ(ctx.move1_suit, 0); + EXPECT_EQ(ctx.high1, 0); + EXPECT_EQ(ctx.move2_rank, 0); + EXPECT_EQ(ctx.move2_suit, 0); + EXPECT_EQ(ctx.high2, 0); +} + +// Second hand: only the lead card is defined. +TEST_F(MovesTest, MakeHeuristicContextSecondHandSnapshotsOnlyLead) +{ + const TrackType tr = poisoned_track(); + const HeuristicContext ctx = context_for_hand_rel(*moves, tr, /*hand_rel=*/1); + + EXPECT_EQ(ctx.lead0_rank, 14); + EXPECT_EQ(ctx.move1_rank, 0); + EXPECT_EQ(ctx.move1_suit, 0); + EXPECT_EQ(ctx.high1, 0); + EXPECT_EQ(ctx.move2_rank, 0); + EXPECT_EQ(ctx.move2_suit, 0); + EXPECT_EQ(ctx.high2, 0); +} + +// Third hand: lead + second-hand card/high are defined; move[2] is not. +TEST_F(MovesTest, MakeHeuristicContextThirdHandSnapshotsThroughMove1) +{ + const TrackType tr = poisoned_track(); + const HeuristicContext ctx = context_for_hand_rel(*moves, tr, /*hand_rel=*/2); + + EXPECT_EQ(ctx.lead0_rank, 14); + EXPECT_EQ(ctx.move1_rank, 12); + EXPECT_EQ(ctx.move1_suit, 2); + EXPECT_EQ(ctx.high1, 1); + EXPECT_EQ(ctx.move2_rank, 0); + EXPECT_EQ(ctx.move2_suit, 0); + EXPECT_EQ(ctx.high2, 0); +} + namespace { From a6329c51c48b7b22d1924f5e2d1f35bc52a693c2 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Fri, 24 Jul 2026 04:26:35 +0200 Subject: [PATCH 13/16] Bind HeuristicContext test refs to static storage. Locals in context_for_hand_rel died on return while HeuristicContext kept references to them, leaving dangling pointers in the snapshot. Co-authored-by: Cursor --- library/tests/moves/moves_test.cpp | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/library/tests/moves/moves_test.cpp b/library/tests/moves/moves_test.cpp index 713d680f..f01e2dcf 100644 --- a/library/tests/moves/moves_test.cpp +++ b/library/tests/moves/moves_test.cpp @@ -291,12 +291,15 @@ auto poisoned_track() -> TrackType return tr; } +// HeuristicContext stores references to these; they must outlive the returned +// value (locals in context_for_hand_rel would dangle after return). +static const Pos kTpos{}; +static const MoveType kBest{}; +static const MoveType kBestTt{}; + auto context_for_hand_rel(Moves& m, const TrackType& tr, const int hand_rel) -> HeuristicContext { - Pos tpos{}; - const MoveType best{}; - const MoveType best_tt{}; m.leadHand = 0; m.currHand = hand_rel; // leadHand 0 ⇒ hand_rel == currHand m.leadSuit = 0; @@ -305,7 +308,7 @@ auto context_for_hand_rel(Moves& m, const TrackType& tr, const int hand_rel) m.suit = 0; m.numMoves = 0; m.lastNumMoves = 0; - return m.make_heuristic_context(tpos, best, best_tt, nullptr, tr); + return m.make_heuristic_context(kTpos, kBest, kBestTt, nullptr, tr); } } // namespace @@ -334,6 +337,18 @@ TEST_F(MovesTest, MakeHeuristicContextSnapshotsExplicitTrack) EXPECT_EQ(ctx.trackp, &tr); } +// HeuristicContext holds references to tpos / best_move / best_move_tt; the +// helper must bind them to storage that outlives the returned context. +TEST_F(MovesTest, MakeHeuristicContextBindsStableRefs) +{ + const TrackType tr = poisoned_track(); + const HeuristicContext ctx = context_for_hand_rel(*moves, tr, /*hand_rel=*/0); + + EXPECT_EQ(&ctx.tpos, &kTpos); + EXPECT_EQ(&ctx.best_move, &kBest); + EXPECT_EQ(&ctx.best_move_tt, &kBestTt); +} + // Leading hand: move[0..2] are not played yet — leave trick snapshots at 0 // even if the TrackType buffer holds stale/poisoned values. TEST_F(MovesTest, MakeHeuristicContextLeadHandOmitsUnsetTrickCards) From d87f77190642401e245a99d7790677fcf146383f Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Fri, 24 Jul 2026 10:38:41 +0200 Subject: [PATCH 14/16] Remove legacy call_heuristic overload that re-derives findex. Move generation already passes a precomputed findex everywhere; drop the unused context-only API and the equivalence tests that depended on it. Co-authored-by: Cursor --- .../heuristic_sorting/heuristic_sorting.cpp | 37 +--- .../heuristic_sorting/heuristic_sorting.hpp | 21 +-- library/src/moves/moves.cpp | 4 +- .../dispatch_findex_test.cpp | 176 ++---------------- specs/heuristic-sorting.md | 20 +- 5 files changed, 31 insertions(+), 227 deletions(-) diff --git a/library/src/heuristic_sorting/heuristic_sorting.cpp b/library/src/heuristic_sorting/heuristic_sorting.cpp index e9ae4557..4d25bb3e 100644 --- a/library/src/heuristic_sorting/heuristic_sorting.cpp +++ b/library/src/heuristic_sorting/heuristic_sorting.cpp @@ -2,8 +2,8 @@ #include #include -// Hot-path overload: the caller passes the dispatch case it already knows, -// so nothing is re-derived from the context here. +// The caller passes the dispatch case it already knows, so nothing is +// re-derived from the context here. void call_heuristic(HeuristicContext& context, const int findex) { switch (findex) { @@ -29,39 +29,6 @@ void call_heuristic(HeuristicContext& context, const int findex) } } -// Legacy overload: derives the dispatch case from the context, then -// delegates. Kept for callers/tests that do not have the findex at hand. -void call_heuristic(HeuristicContext& context) -{ - // Determine which position in trick (0=leading, 1-3=following) - int hand_rel = 0; - if (context.curr_hand != context.lead_hand) { - // Calculate relative position: 1, 2, or 3 based on lead hand - hand_rel = (context.curr_hand + 4 - context.lead_hand) % 4; - } - - // Check if trump game with trump winner available - const int ftest = ((context.trump != DDS_NOTRUMP) && - (context.trump >= 0 && context.trump < DDS_SUITS) && - (context.tpos.winner[context.trump].rank != 0) ? 1 : 0); - - // Leading hand (hand_rel == 0) - MoveGen0 logic - if (hand_rel == 0) { - call_heuristic(context, ftest); - return; - } - - // Following hands (hand_rel 1-3) - MoveGen123 logic - // Check if current hand can follow suit (not void) - const unsigned short ris = - context.tpos.rank_in_suit[context.curr_hand][context.lead_suit]; - const bool can_follow_suit = (ris != 0); - - // Calculate function index using same logic as original - const int findex = 4 * hand_rel + ftest + (can_follow_suit ? 0 : 2); - call_heuristic(context, findex); -} - // The following functions are extracted from Moves.cpp and refactored to be // standalone. They now accept a HeuristicContext struct which contains all the // necessary state that was previously accessed as members of the Moves class. diff --git a/library/src/heuristic_sorting/heuristic_sorting.hpp b/library/src/heuristic_sorting/heuristic_sorting.hpp index 6e80e47f..8f1417dc 100644 --- a/library/src/heuristic_sorting/heuristic_sorting.hpp +++ b/library/src/heuristic_sorting/heuristic_sorting.hpp @@ -62,27 +62,13 @@ struct HeuristicContext int lead0_rank = 0; // trackp->move[0].rank }; -/// @brief Apply heuristic sorting to candidate moves in the given context. +/// @brief Apply heuristic sorting using a precomputed dispatch index. /// /// Evaluates candidate moves using position-dependent heuristics to assign /// weights that guide search algorithms toward the most promising lines. -/// Heuristics vary based on position characteristics (leading/following hand, -/// trump presence, suit availability) to optimize move ordering efficiency. -/// -/// @param context Pre-constructed HeuristicContext containing position data, -/// move arrays, and cached snapshots for efficient evaluation. -/// Non-const because weighting writes through context.mply. -/// -/// @note Prefer the `call_heuristic(context, findex)` overload when the dispatch -/// index is already known (hot path). Use this overload when callers do -/// not have `findex` available. -void call_heuristic(HeuristicContext& context); - -/// @brief Apply heuristic sorting using a precomputed dispatch index. -/// /// Move generation already knows the dispatch case; passing it avoids /// re-deriving the relative hand, trump state and voidness from the context -/// on every call (hot path). +/// on every call. /// /// Encoding (matches the findex used by Moves::MoveGen0 / MoveGen123): /// - Leading hand: 0 = no trump winner available, 1 = trump winner available @@ -92,7 +78,8 @@ void call_heuristic(HeuristicContext& context); /// for hand_rel in 1..3, i.e. values 4..15, where trump_winner is the /// same 0/1 bit as for the leading hand. /// -/// @param context Pre-constructed HeuristicContext (see other overload). +/// @param context Pre-constructed HeuristicContext containing position data, +/// move arrays, and cached snapshots for efficient evaluation. /// Non-const because weighting writes through context.mply. /// @param findex Precomputed dispatch index as encoded above. void call_heuristic(HeuristicContext& context, int findex); diff --git a/library/src/moves/moves.cpp b/library/src/moves/moves.cpp index c7b49dc1..7aabe90a 100644 --- a/library/src/moves/moves.cpp +++ b/library/src/moves/moves.cpp @@ -52,8 +52,8 @@ const MgType RegisterList[16] = {MgType::NT0, MgType::TRUMP0, namespace { -// Same trump-winner bit as the legacy call_heuristic dispatcher: never index -// winner[trump] unless trump is a suit in [0, DDS_SUITS). +// Trump-winner bit of the heuristic findex: never index winner[trump] +// unless trump is a suit in [0, DDS_SUITS). auto trump_winner_findex(const Pos& tpos, const int trump) -> int { return ((trump != DDS_NOTRUMP) && diff --git a/library/tests/heuristic_sorting/dispatch_findex_test.cpp b/library/tests/heuristic_sorting/dispatch_findex_test.cpp index 10f9180a..3edc24c9 100644 --- a/library/tests/heuristic_sorting/dispatch_findex_test.cpp +++ b/library/tests/heuristic_sorting/dispatch_findex_test.cpp @@ -1,12 +1,9 @@ /** * @file dispatch_findex_test.cpp - * @brief Equivalence tests for findex-based heuristic dispatch. + * @brief Behaviour of findex-based heuristic dispatch. * - * Move generation already knows the dispatch case (leading vs following - * hand, trump vs NT, void vs not void) as a small integer. The - * call_heuristic(ctx, findex) overload must select exactly the same weight - * function as the legacy overload that re-derives the case from the context, - * for every reachable findex. + * Move generation always passes a precomputed findex. This file covers the + * dispatcher's fallback when an out-of-range findex reaches call_heuristic. */ #include @@ -29,16 +26,11 @@ struct DispatchFixture MoveType best_move_tt{}; RelRanksType* rel = rel_ranks; TrackType track{}; - MoveType moves_legacy[kNumMoves]{}; + MoveType moves_expected[kNumMoves]{}; MoveType moves_findex[kNumMoves]{}; }; -// Populate a deterministic pseudo-position that exercises the weight -// arithmetic (non-zero ranks, lengths, winners) without needing to be a -// fully consistent deal: both dispatchers run the same weight function on -// identical inputs, so equal outputs prove equal dispatch. -void fill_position(DispatchFixture& f, const bool trump_game, const bool is_void, - const int curr_hand, const int lead_suit) +void fill_position(DispatchFixture& f, const int curr_hand, const int lead_suit) { for (int h = 0; h < DDS_HANDS; h++) { @@ -59,16 +51,6 @@ void fill_position(DispatchFixture& f, const bool trump_game, const bool is_void f.tpos.second_best[s].hand = (s + 2) % DDS_HANDS; } - const int trump = trump_game ? 1 : DDS_NOTRUMP; - if (trump_game) - f.tpos.winner[trump].rank = 14; - - if (is_void) - { - f.tpos.rank_in_suit[curr_hand][lead_suit] = 0; - f.tpos.length[curr_hand][lead_suit] = 0; - } - f.track.lead_suit = lead_suit; f.track.move[0] = ExtCard{lead_suit, 8, 0}; f.track.move[1] = ExtCard{lead_suit, 10, 0}; @@ -80,18 +62,15 @@ void fill_position(DispatchFixture& f, const bool trump_game, const bool is_void for (int k = 0; k < kNumMoves; k++) { - const int suit = is_void ? ((lead_suit + 1) % DDS_SUITS) : lead_suit; - f.moves_legacy[k] = MoveType{suit, 12 - 2 * k, 0, 0}; - f.moves_findex[k] = f.moves_legacy[k]; + f.moves_expected[k] = MoveType{lead_suit, 12 - 2 * k, 0, 0}; + f.moves_findex[k] = f.moves_expected[k]; } } HeuristicContext make_context(DispatchFixture& f, MoveType* mply, - const int trump, const bool is_void, const int curr_hand, const int lead_hand, const int lead_suit) { - const int move_suit = is_void ? ((lead_suit + 1) % DDS_SUITS) : lead_suit; HeuristicContext ctx = { f.tpos, f.best_move, @@ -100,8 +79,8 @@ HeuristicContext make_context(DispatchFixture& f, MoveType* mply, mply, kNumMoves, // num_moves 0, // last_num_moves - trump, - move_suit, // suit under consideration + DDS_NOTRUMP, + lead_suit, // suit under consideration &f.track, 5, // curr_trick curr_hand, @@ -120,97 +99,8 @@ HeuristicContext make_context(DispatchFixture& f, MoveType* mply, return ctx; } -void expect_same_weights(const DispatchFixture& f, const int findex) -{ - for (int k = 0; k < kNumMoves; k++) - EXPECT_EQ(f.moves_legacy[k].weight, f.moves_findex[k].weight) - << "findex=" << findex << " move=" << k; -} - } // namespace -// Leading hand: findex 0 (NT) and 1 (trump winner available) must match the -// legacy dispatch, which selects the weight function from the trump/winner -// state — not merely whether a trump suit is set. -TEST(DispatchFindex, LeadHandMatchesLegacyDispatch) -{ - for (const bool trump_game : {false, true}) - { - DispatchFixture f; - const int lead_hand = 2; - fill_position(f, trump_game, /*is_void=*/false, lead_hand, /*lead_suit=*/0); - - const int trump = trump_game ? 1 : DDS_NOTRUMP; - HeuristicContext legacy = make_context( - f, f.moves_legacy, trump, false, lead_hand, lead_hand, 0); - HeuristicContext with_findex = make_context( - f, f.moves_findex, trump, false, lead_hand, lead_hand, 0); - - call_heuristic(legacy); - call_heuristic(with_findex, trump_game ? 1 : 0); - - expect_same_weights(f, trump_game ? 1 : 0); - } -} - -// Out-of-range trump must not index winner[trump]; treat as no trump winner -// (same range guard Moves::MoveGen0 / MoveGen123 must apply). -TEST(DispatchFindex, LegacyTreatsOutOfRangeTrumpAsNoWinner) -{ - // DDS_NOTRUMP == DDS_SUITS, so use values strictly outside [0, DDS_SUITS). - for (const int bad_trump : {-1, DDS_SUITS + 1, 99}) - { - DispatchFixture f; - const int lead_hand = 0; - fill_position(f, /*trump_game=*/false, /*is_void=*/false, lead_hand, - /*lead_suit=*/0); - - HeuristicContext legacy = make_context( - f, f.moves_legacy, bad_trump, false, lead_hand, lead_hand, 0); - HeuristicContext with_findex = make_context( - f, f.moves_findex, DDS_NOTRUMP, false, lead_hand, lead_hand, 0); - - call_heuristic(legacy); - call_heuristic(with_findex, /*findex=*/0); - - expect_same_weights(f, /*findex=*/0); - } -} - -// findex bit 0/1 is "trump winner available", not "trump suit set". -// A trump deal with winner[trump].rank == 0 must dispatch as findex 0. -TEST(DispatchFindex, LeadHandTrumpWithoutWinnerUsesFindex0) -{ - DispatchFixture f; - const int lead_hand = 2; - fill_position(f, /*trump_game=*/true, /*is_void=*/false, lead_hand, - /*lead_suit=*/0); - f.tpos.winner[1].rank = 0; - - HeuristicContext legacy = make_context( - f, f.moves_legacy, /*trump=*/1, false, lead_hand, lead_hand, 0); - HeuristicContext with_findex = make_context( - f, f.moves_findex, /*trump=*/1, false, lead_hand, lead_hand, 0); - - call_heuristic(legacy); - call_heuristic(with_findex, /*findex=*/0); - - expect_same_weights(f, /*findex=*/0); - - // Passing 1 ("trump game") would select the wrong weight function. - for (int k = 0; k < kNumMoves; k++) - f.moves_findex[k].weight = 0; - call_heuristic(with_findex, /*findex=*/1); - bool any_differ = false; - for (int k = 0; k < kNumMoves; k++) - { - if (f.moves_legacy[k].weight != f.moves_findex[k].weight) - any_differ = true; - } - EXPECT_TRUE(any_differ) - << "findex 1 must not match when trump is set but no winner is available"; -} - // Out-of-range findex must still assign deterministic weights (NT0 fallback), // not leave stale/uninitialized move weights that would scramble MergeSort. TEST(DispatchFindex, InvalidFindexFallsBackToBasicNt0Weights) @@ -219,20 +109,19 @@ TEST(DispatchFindex, InvalidFindexFallsBackToBasicNt0Weights) { DispatchFixture f; const int lead_hand = 0; - fill_position(f, /*trump_game=*/false, /*is_void=*/false, lead_hand, - /*lead_suit=*/0); + fill_position(f, lead_hand, /*lead_suit=*/0); constexpr int kStale = 0x7f0f0f0f; for (int k = 0; k < kNumMoves; k++) { - f.moves_legacy[k].weight = 0; + f.moves_expected[k].weight = 0; f.moves_findex[k].weight = kStale; } HeuristicContext expected = make_context( - f, f.moves_legacy, DDS_NOTRUMP, false, lead_hand, lead_hand, 0); + f, f.moves_expected, lead_hand, lead_hand, 0); HeuristicContext with_findex = make_context( - f, f.moves_findex, DDS_NOTRUMP, false, lead_hand, lead_hand, 0); + f, f.moves_findex, lead_hand, lead_hand, 0); weight_alloc_nt0(expected); call_heuristic(with_findex, bad_findex); @@ -242,46 +131,9 @@ TEST(DispatchFindex, InvalidFindexFallsBackToBasicNt0Weights) EXPECT_NE(f.moves_findex[k].weight, kStale) << "findex=" << bad_findex << " move=" << k << " left a stale weight"; - EXPECT_EQ(f.moves_legacy[k].weight, f.moves_findex[k].weight) + EXPECT_EQ(f.moves_expected[k].weight, f.moves_findex[k].weight) << "findex=" << bad_findex << " move=" << k << " must match weight_alloc_nt0 fallback"; } } } - -// Following hands: findex 4..15 (4*hand_rel + trump_winner + 2*void) must match -// the legacy dispatch that re-derives hand_rel, trump-winner state and voidness. -TEST(DispatchFindex, FollowingHandsMatchLegacyDispatchForAllFindexes) -{ - const int lead_hand = 1; - const int lead_suit = 2; - - for (int hand_rel = 1; hand_rel <= 3; hand_rel++) - { - for (const bool trump_game : {false, true}) - { - for (const bool is_void : {false, true}) - { - const int curr_hand = (lead_hand + hand_rel) % 4; - const int findex = - 4 * hand_rel + (trump_game ? 1 : 0) + (is_void ? 2 : 0); - - DispatchFixture f; - fill_position(f, trump_game, is_void, curr_hand, lead_suit); - - const int trump = trump_game ? 1 : DDS_NOTRUMP; - HeuristicContext legacy = make_context( - f, f.moves_legacy, trump, is_void, curr_hand, lead_hand, - lead_suit); - HeuristicContext with_findex = make_context( - f, f.moves_findex, trump, is_void, curr_hand, lead_hand, - lead_suit); - - call_heuristic(legacy); - call_heuristic(with_findex, findex); - - expect_same_weights(f, findex); - } - } - } -} diff --git a/specs/heuristic-sorting.md b/specs/heuristic-sorting.md index e15c756d..48665e77 100644 --- a/specs/heuristic-sorting.md +++ b/specs/heuristic-sorting.md @@ -1,7 +1,7 @@ --- capability: heuristic-sorting owners: [heuristic_sorting] -last-updated: 2026-07-23 +last-updated: 2026-07-24 --- # Heuristic Sorting @@ -37,14 +37,12 @@ solve. `doc/heuristic-sorting.md` is narrative intent (including lead/void per-suit top-card bonuses) and may lag the code; when they disagree, trust the implementation. -- **`call_heuristic` takes a pre-built `HeuristicContext&`.** The caller constructs - one context (position, move array, best moves, relative ranks, and cached - per-trick snapshots such as `removed_ranks`, `move1_rank`, `high1`) and passes - it by mutable reference; weighting writes through `context.mply`. Two free - overloads exist: a legacy form that derives the dispatch case from the - context, and a hot-path form that takes a precomputed `findex` from move - generation (`MoveGen0` / `MoveGen123`) so that case is not re-derived per - suit. +- **`call_heuristic` takes a pre-built `HeuristicContext&` and a `findex`.** The + caller constructs one context (position, move array, best moves, relative + ranks, and cached per-trick snapshots such as `removed_ranks`, `move1_rank`, + `high1`) and passes it by mutable reference; weighting writes through + `context.mply`. Move generation (`MoveGen0` / `MoveGen123`) always passes the + precomputed dispatch `findex` so the case is not re-derived per suit. - **`TrackType` is shared per-trick position state** defined here and consumed by both [move-generation](move-generation.md) and the heuristics. `Moves` owns the `track[]` array and mutates `trackp`; heuristics read the snapshots copied into @@ -55,8 +53,8 @@ solve. ## Key entry points - `library/src/heuristic_sorting/heuristic_sorting.hpp` — `HeuristicContext`, - `TrackType`, and both `call_heuristic` overloads (`HeuristicContext&`, and - with `findex`). Doxygen documents the fields. + `TrackType`, and `call_heuristic(context, findex)`. Doxygen documents the + fields. - `library/src/heuristic_sorting/heuristic_sorting.cpp` + `internal.hpp` — the per-situation weighting helpers. - Narrative algorithm: `doc/heuristic-sorting.md`. From bd361a8803cc269f5d386a28aca8ee6e7e7ff5da Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Fri, 24 Jul 2026 10:42:00 +0200 Subject: [PATCH 15/16] Drop duplicate make_heuristic_context doxygen from the .cpp. Documentation for the method already lives in moves.hpp. Co-authored-by: Cursor --- library/src/moves/moves.cpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/library/src/moves/moves.cpp b/library/src/moves/moves.cpp index 7aabe90a..533f230b 100644 --- a/library/src/moves/moves.cpp +++ b/library/src/moves/moves.cpp @@ -698,17 +698,6 @@ auto Moves::Sort(const int tricks, const int relHand) -> void { mply[j] = tmp; \ } -/** - * @brief Build a heuristic context snapshot from current move-gen state. - * - * Built once per move-generation call (not per suit); callers update the - * suit/num_moves/last_num_moves fields per iteration and dispatch with the - * findex they already computed, avoiding per-suit reconstruction and - * re-derivation of the dispatch case (hot path). - * - * @param tr Bound trick track passed by the caller (MoveGen0/MoveGen123), - * never read from the nullable Moves::trackp member. - */ auto Moves::make_heuristic_context(const Pos &tpos, const MoveType &best_move, const MoveType &best_move_tt, const RelRanksType thrp_rel[], From 4767a77e61fc0597f3147de779191e5eecf1f8cd Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Fri, 24 Jul 2026 14:14:12 +0200 Subject: [PATCH 16/16] Replace heuristic findex with a WeightCase enum. Name the weight-function dispatch cases explicitly so move generation and call_heuristic share a typed API instead of a magic integer encoding. Co-authored-by: Cursor --- .../heuristic_sorting/heuristic_sorting.cpp | 36 ++++++------ .../heuristic_sorting/heuristic_sorting.hpp | 37 +++++++++---- library/src/moves/moves.cpp | 38 ++++++++----- library/src/moves/moves.hpp | 2 +- .../dispatch_findex_test.cpp | 55 +++++++++++++------ specs/heuristic-sorting.md | 10 ++-- 6 files changed, 113 insertions(+), 65 deletions(-) diff --git a/library/src/heuristic_sorting/heuristic_sorting.cpp b/library/src/heuristic_sorting/heuristic_sorting.cpp index 4d25bb3e..1270fa59 100644 --- a/library/src/heuristic_sorting/heuristic_sorting.cpp +++ b/library/src/heuristic_sorting/heuristic_sorting.cpp @@ -2,25 +2,27 @@ #include #include -// The caller passes the dispatch case it already knows, so nothing is +// The caller passes the weight case it already knows, so nothing is // re-derived from the context here. -void call_heuristic(HeuristicContext& context, const int findex) +void call_heuristic(HeuristicContext& context, const WeightCase weight_case) { - switch (findex) { - case 0: weight_alloc_nt0(context); break; // leading, no trump winner - case 1: weight_alloc_trump0(context); break; // leading, trump winner available - case 4: weight_alloc_nt_notvoid1(context); break; // hand_rel=1, can follow, no trump winner - case 5: weight_alloc_trump_notvoid1(context); break; // hand_rel=1, can follow, trump winner - case 6: weight_alloc_nt_void1(context); break; // hand_rel=1, void, no trump winner - case 7: weight_alloc_trump_void1(context); break; // hand_rel=1, void, trump winner - case 8: weight_alloc_nt_notvoid2(context); break; // hand_rel=2, can follow, no trump winner - case 9: weight_alloc_trump_notvoid2(context); break; // hand_rel=2, can follow, trump winner - case 10: weight_alloc_nt_void2(context); break; // hand_rel=2, void, no trump winner - case 11: weight_alloc_trump_void2(context); break; // hand_rel=2, void, trump winner - case 12: weight_alloc_combined_notvoid3(context); break; // hand_rel=3, can follow, no trump winner - case 13: weight_alloc_combined_notvoid3(context); break; // hand_rel=3, can follow, trump winner - case 14: weight_alloc_nt_void3(context); break; // hand_rel=3, void, no trump winner - case 15: weight_alloc_trump_void3(context); break; // hand_rel=3, void, trump winner + switch (weight_case) { + case WeightCase::Nt0: weight_alloc_nt0(context); break; + case WeightCase::Trump0: weight_alloc_trump0(context); break; + case WeightCase::NtNotVoid1: weight_alloc_nt_notvoid1(context); break; + case WeightCase::TrumpNotVoid1: weight_alloc_trump_notvoid1(context); break; + case WeightCase::NtVoid1: weight_alloc_nt_void1(context); break; + case WeightCase::TrumpVoid1: weight_alloc_trump_void1(context); break; + case WeightCase::NtNotVoid2: weight_alloc_nt_notvoid2(context); break; + case WeightCase::TrumpNotVoid2: weight_alloc_trump_notvoid2(context); break; + case WeightCase::NtVoid2: weight_alloc_nt_void2(context); break; + case WeightCase::TrumpVoid2: weight_alloc_trump_void2(context); break; + case WeightCase::CombinedNotVoid3: + case WeightCase::CombinedNotVoid3Trump: + weight_alloc_combined_notvoid3(context); + break; + case WeightCase::NtVoid3: weight_alloc_nt_void3(context); break; + case WeightCase::TrumpVoid3: weight_alloc_trump_void3(context); break; default: // Should not happen; fall back to NT leading weights so moves get // deterministic ordering instead of stale/uninitialized weights. diff --git a/library/src/heuristic_sorting/heuristic_sorting.hpp b/library/src/heuristic_sorting/heuristic_sorting.hpp index 8f1417dc..ced35abf 100644 --- a/library/src/heuristic_sorting/heuristic_sorting.hpp +++ b/library/src/heuristic_sorting/heuristic_sorting.hpp @@ -62,7 +62,30 @@ struct HeuristicContext int lead0_rank = 0; // trackp->move[0].rank }; -/// @brief Apply heuristic sorting using a precomputed dispatch index. +/// @brief Which weight_alloc_* helper to run for the current move list. +/// +/// Move generation already knows the lead/follow, trump-winner, and void +/// situation; it passes the matching case so call_heuristic does not re-derive +/// it from the context. Numeric values match the historical findex encoding +/// used by Moves::MoveGen0 / MoveGen123 (and DDS_MOVES RegisterList). +enum class WeightCase : int { + Nt0 = 0, ///< Leading hand, no trump winner available + Trump0 = 1, ///< Leading hand, trump winner available + NtNotVoid1 = 4, ///< 2nd hand, can follow, no trump winner + TrumpNotVoid1 = 5, ///< 2nd hand, can follow, trump winner + NtVoid1 = 6, ///< 2nd hand, void in lead suit, no trump winner + TrumpVoid1 = 7, ///< 2nd hand, void in lead suit, trump winner + NtNotVoid2 = 8, ///< 3rd hand, can follow, no trump winner + TrumpNotVoid2 = 9, ///< 3rd hand, can follow, trump winner + NtVoid2 = 10, ///< 3rd hand, void in lead suit, no trump winner + TrumpVoid2 = 11, ///< 3rd hand, void in lead suit, trump winner + CombinedNotVoid3 = 12, ///< 4th hand, can follow, no trump winner + CombinedNotVoid3Trump = 13,///< 4th hand, can follow, trump winner + NtVoid3 = 14, ///< 4th hand, void in lead suit, no trump winner + TrumpVoid3 = 15, ///< 4th hand, void in lead suit, trump winner +}; + +/// @brief Apply heuristic sorting using a precomputed weight case. /// /// Evaluates candidate moves using position-dependent heuristics to assign /// weights that guide search algorithms toward the most promising lines. @@ -70,17 +93,9 @@ struct HeuristicContext /// re-deriving the relative hand, trump state and voidness from the context /// on every call. /// -/// Encoding (matches the findex used by Moves::MoveGen0 / MoveGen123): -/// - Leading hand: 0 = no trump winner available, 1 = trump winner available -/// (trump in [0, DDS_SUITS) && trump != DDS_NOTRUMP && -/// winner[trump].rank != 0) -/// - Following hand: 4 * hand_rel + trump_winner + (void_in_lead_suit ? 2 : 0) -/// for hand_rel in 1..3, i.e. values 4..15, where trump_winner is the -/// same 0/1 bit as for the leading hand. -/// /// @param context Pre-constructed HeuristicContext containing position data, /// move arrays, and cached snapshots for efficient evaluation. /// Non-const because weighting writes through context.mply. -/// @param findex Precomputed dispatch index as encoded above. -void call_heuristic(HeuristicContext& context, int findex); +/// @param weight_case Precomputed weight case (see WeightCase). +void call_heuristic(HeuristicContext& context, WeightCase weight_case); diff --git a/library/src/moves/moves.cpp b/library/src/moves/moves.cpp index 533f230b..e2d5d939 100644 --- a/library/src/moves/moves.cpp +++ b/library/src/moves/moves.cpp @@ -52,9 +52,9 @@ const MgType RegisterList[16] = {MgType::NT0, MgType::TRUMP0, namespace { -// Trump-winner bit of the heuristic findex: never index winner[trump] +// Trump-winner bit of WeightCase: never index winner[trump] // unless trump is a suit in [0, DDS_SUITS). -auto trump_winner_findex(const Pos& tpos, const int trump) -> int +auto trump_winner_bit(const Pos& tpos, const int trump) -> int { return ((trump != DDS_NOTRUMP) && (trump >= 0 && trump < DDS_SUITS) && @@ -63,6 +63,14 @@ auto trump_winner_findex(const Pos& tpos, const int trump) -> int : 0; } +// Encode following-hand WeightCase: 4 * hand_rel + trump_winner + (void ? 2 : 0). +auto weight_case_follow(const int hand_rel, const int trump_winner, + const bool is_void) -> WeightCase +{ + return static_cast( + 4 * hand_rel + trump_winner + (is_void ? 2 : 0)); +} + } // namespace Moves::Moves() { @@ -204,9 +212,11 @@ auto Moves::MoveGen0(const int tricks, const Pos &tpos, suit = 0; leadSuit = 0; - // Leading-hand dispatch case, known here once instead of re-derived per + // Leading-hand weight case, known here once instead of re-derived per // suit inside the heuristic dispatcher. - const int lead_findex = trump_winner_findex(tpos, trump); + const WeightCase lead_case = trump_winner_bit(tpos, trump) + ? WeightCase::Trump0 + : WeightCase::Nt0; HeuristicContext hctx = make_heuristic_context(tpos, bestMove, bestMoveTT, thrp_rel, track[tricks]); @@ -243,11 +253,11 @@ auto Moves::MoveGen0(const int tricks, const Pos &tpos, hctx.suit = suit; hctx.last_num_moves = lastNumMoves; hctx.num_moves = numMoves; - ::call_heuristic(hctx, lead_findex); + ::call_heuristic(hctx, lead_case); } #ifdef DDS_MOVES - if (lead_findex) + if (lead_case == WeightCase::Trump0) MG_REGISTER(MgType::TRUMP0, 0); else MG_REGISTER(MgType::NT0, 0); @@ -284,8 +294,8 @@ auto Moves::MoveGen123(const int tricks, const int handRel, const Pos &tpos) lastNumMoves = 0; suit = leadSuit; - int findex; - const int ftest = trump_winner_findex(tpos, trump); + WeightCase weight_case; + const int trump_winner = trump_winner_bit(tpos, trump); // Empty best-move placeholders must outlive the hoisted context, which // holds references to them. @@ -313,9 +323,9 @@ auto Moves::MoveGen123(const int tricks, const int handRel, const Pos &tpos) g--; } - findex = 4 * handRel + ftest; + weight_case = weight_case_follow(handRel, trump_winner, /*is_void=*/false); #ifdef DDS_MOVES - MG_REGISTER(RegisterList[findex], handRel); + MG_REGISTER(RegisterList[static_cast(weight_case)], handRel); #endif list.current = 0; @@ -326,16 +336,16 @@ auto Moves::MoveGen123(const int tricks, const int handRel, const Pos &tpos) HeuristicContext hctx = make_heuristic_context(tpos, empty_move, empty_move, nullptr, track[tricks]); - ::call_heuristic(hctx, findex); + ::call_heuristic(hctx, weight_case); Moves::MergeSort(); return numMoves; } - findex = 4 * handRel + ftest + 2; + weight_case = weight_case_follow(handRel, trump_winner, /*is_void=*/true); #ifdef DDS_MOVES - MG_REGISTER(RegisterList[findex], handRel); + MG_REGISTER(RegisterList[static_cast(weight_case)], handRel); #endif HeuristicContext hctx = @@ -370,7 +380,7 @@ auto Moves::MoveGen123(const int tricks, const int handRel, const Pos &tpos) hctx.suit = suit; hctx.last_num_moves = lastNumMoves; hctx.num_moves = numMoves; - ::call_heuristic(hctx, findex); + ::call_heuristic(hctx, weight_case); } list.current = 0; diff --git a/library/src/moves/moves.hpp b/library/src/moves/moves.hpp index eb830da1..99842b62 100644 --- a/library/src/moves/moves.hpp +++ b/library/src/moves/moves.hpp @@ -187,7 +187,7 @@ class Moves { * @brief Build a HeuristicContext snapshot from current move-gen state. * * Built once per move-generation call; the caller updates suit and move - * counts per suit iteration and dispatches via call_heuristic(ctx, findex). + * counts per suit iteration and dispatches via call_heuristic(ctx, weight_case). * * @param tpos Current position * @param best_move Best move from search (must outlive the context) diff --git a/library/tests/heuristic_sorting/dispatch_findex_test.cpp b/library/tests/heuristic_sorting/dispatch_findex_test.cpp index 3edc24c9..94bebc01 100644 --- a/library/tests/heuristic_sorting/dispatch_findex_test.cpp +++ b/library/tests/heuristic_sorting/dispatch_findex_test.cpp @@ -1,9 +1,9 @@ /** * @file dispatch_findex_test.cpp - * @brief Behaviour of findex-based heuristic dispatch. + * @brief Behaviour of WeightCase-based heuristic dispatch. * - * Move generation always passes a precomputed findex. This file covers the - * dispatcher's fallback when an out-of-range findex reaches call_heuristic. + * Move generation always passes a precomputed WeightCase. This file covers the + * dispatcher's fallback when an unrecognized case reaches call_heuristic. */ #include @@ -27,7 +27,7 @@ struct DispatchFixture RelRanksType* rel = rel_ranks; TrackType track{}; MoveType moves_expected[kNumMoves]{}; - MoveType moves_findex[kNumMoves]{}; + MoveType moves_dispatched[kNumMoves]{}; }; void fill_position(DispatchFixture& f, const int curr_hand, const int lead_suit) @@ -63,7 +63,7 @@ void fill_position(DispatchFixture& f, const int curr_hand, const int lead_suit) for (int k = 0; k < kNumMoves; k++) { f.moves_expected[k] = MoveType{lead_suit, 12 - 2 * k, 0, 0}; - f.moves_findex[k] = f.moves_expected[k]; + f.moves_dispatched[k] = f.moves_expected[k]; } } @@ -101,11 +101,12 @@ HeuristicContext make_context(DispatchFixture& f, MoveType* mply, } // namespace -// Out-of-range findex must still assign deterministic weights (NT0 fallback), -// not leave stale/uninitialized move weights that would scramble MergeSort. -TEST(DispatchFindex, InvalidFindexFallsBackToBasicNt0Weights) +// Unrecognized WeightCase values must still assign deterministic weights +// (Nt0 fallback), not leave stale/uninitialized move weights that would +// scramble MergeSort. +TEST(DispatchWeightCase, InvalidWeightCaseFallsBackToBasicNt0Weights) { - for (const int bad_findex : {2, 3, 16, -1, 99}) + for (const int bad_value : {2, 3, 16, -1, 99}) { DispatchFixture f; const int lead_hand = 0; @@ -115,25 +116,45 @@ TEST(DispatchFindex, InvalidFindexFallsBackToBasicNt0Weights) for (int k = 0; k < kNumMoves; k++) { f.moves_expected[k].weight = 0; - f.moves_findex[k].weight = kStale; + f.moves_dispatched[k].weight = kStale; } HeuristicContext expected = make_context( f, f.moves_expected, lead_hand, lead_hand, 0); - HeuristicContext with_findex = make_context( - f, f.moves_findex, lead_hand, lead_hand, 0); + HeuristicContext dispatched = make_context( + f, f.moves_dispatched, lead_hand, lead_hand, 0); weight_alloc_nt0(expected); - call_heuristic(with_findex, bad_findex); + call_heuristic(dispatched, static_cast(bad_value)); for (int k = 0; k < kNumMoves; k++) { - EXPECT_NE(f.moves_findex[k].weight, kStale) - << "findex=" << bad_findex << " move=" << k + EXPECT_NE(f.moves_dispatched[k].weight, kStale) + << "weight_case=" << bad_value << " move=" << k << " left a stale weight"; - EXPECT_EQ(f.moves_expected[k].weight, f.moves_findex[k].weight) - << "findex=" << bad_findex << " move=" << k + EXPECT_EQ(f.moves_expected[k].weight, f.moves_dispatched[k].weight) + << "weight_case=" << bad_value << " move=" << k << " must match weight_alloc_nt0 fallback"; } } } + +// Known WeightCase values must select the matching weight_alloc_* helper. +TEST(DispatchWeightCase, Nt0DispatchesToWeightAllocNt0) +{ + DispatchFixture f; + const int lead_hand = 0; + fill_position(f, lead_hand, /*lead_suit=*/0); + + HeuristicContext expected = make_context( + f, f.moves_expected, lead_hand, lead_hand, 0); + HeuristicContext dispatched = make_context( + f, f.moves_dispatched, lead_hand, lead_hand, 0); + + weight_alloc_nt0(expected); + call_heuristic(dispatched, WeightCase::Nt0); + + for (int k = 0; k < kNumMoves; k++) + EXPECT_EQ(f.moves_expected[k].weight, f.moves_dispatched[k].weight) + << "move=" << k; +} diff --git a/specs/heuristic-sorting.md b/specs/heuristic-sorting.md index 48665e77..d7af3957 100644 --- a/specs/heuristic-sorting.md +++ b/specs/heuristic-sorting.md @@ -37,12 +37,12 @@ solve. `doc/heuristic-sorting.md` is narrative intent (including lead/void per-suit top-card bonuses) and may lag the code; when they disagree, trust the implementation. -- **`call_heuristic` takes a pre-built `HeuristicContext&` and a `findex`.** The - caller constructs one context (position, move array, best moves, relative +- **`call_heuristic` takes a pre-built `HeuristicContext&` and a `WeightCase`.** + The caller constructs one context (position, move array, best moves, relative ranks, and cached per-trick snapshots such as `removed_ranks`, `move1_rank`, `high1`) and passes it by mutable reference; weighting writes through `context.mply`. Move generation (`MoveGen0` / `MoveGen123`) always passes the - precomputed dispatch `findex` so the case is not re-derived per suit. + precomputed `WeightCase` so the case is not re-derived per suit. - **`TrackType` is shared per-trick position state** defined here and consumed by both [move-generation](move-generation.md) and the heuristics. `Moves` owns the `track[]` array and mutates `trackp`; heuristics read the snapshots copied into @@ -53,8 +53,8 @@ solve. ## Key entry points - `library/src/heuristic_sorting/heuristic_sorting.hpp` — `HeuristicContext`, - `TrackType`, and `call_heuristic(context, findex)`. Doxygen documents the - fields. + `TrackType`, `WeightCase`, and `call_heuristic(context, weight_case)`. + Doxygen documents the fields. - `library/src/heuristic_sorting/heuristic_sorting.cpp` + `internal.hpp` — the per-situation weighting helpers. - Narrative algorithm: `doc/heuristic-sorting.md`.