Fix several MP/qsearch correctness bugs found by auditing ctx->uPly

indexing and per-thread state across MakeMove/recursion boundaries;
add a position-signature stamp so full-eval-only data can be validated
cheaply instead of approximated.

- chess.h/searchsup.c/search.c/root.c: add NODE_TYPE (PV/CUT/ALL)
  classification, set centrally in CommonSearchInit, replacing the old
  binary fPvNode flag. Fixes split.c's node-type check reading stale
  data on helper threads (they never call CommonSearchInit for the
  split-point ply) and removes a redundant, always-true parent-type
  check in search.c's own LMR gate.

- split.c: StartParallelSearch now copies the split point's eNodeType
  into SPLIT_INFO for helper threads to read (mirroring the existing
  iEval/iImprovement pattern); a split-node fail-high is now detected
  explicitly in search.c's move loop instead of falling through to the
  generic no_more_moves classifier, which misread it as a fail-low
  (skipping killer/history updates and storing an inverted-sense upper
  bound instead of a lower bound); ThreadUnderTerminatingSplit now
  checks all nested split pointers instead of just the first, so a
  thread notices when an inner split terminates while it's still
  helping an outer one; MAX_SPLITS bumped 20->64 for higher thread
  counts.

- chess.h/searchsup.c/search.c: convert
  SEARCHER_THREAD_CONTEXT.uLastPositionalSource (a single per-thread
  scalar) to PLY_INFO.uLastPositionalSource (per-ply). The scalar
  version was clobbered whenever QSearch's own move loop recursed
  before a later iteration read it back, corrupting which qsearch
  futility margin got applied; also fixes the same hazard in the
  newly-added check-evasion futility prune.

- chess.h/generate.c/searchsup.c: generalize move-scoring interpretation
  via a new MOVE_SCORING_SCHEME tag (per-ply, set by whichever
  generate.c routine last scored that ply's moves) and a single shared
  DecodeMoveValueEstimate, replacing scheme-blind decode logic in
  ComputeMoveScore and _ShouldWeConsiderThisMove.

- chess.h/eval.c/dynamic.c: stamp PLY_INFO.u64FullEvalSig with the
  position signature whenever a node completes a full (non-lazy) Eval()
  call. cEnprise/pEnprise/cTrapped/pTrapped/iKingScore/the eval-scoped
  bb*Attacks bitboards are only valid when this matches the position's
  live signature; _EnpriseSlotValid/_TrappedSlotValid now compare sigs
  directly instead of re-checking piece identity on the live board.

- searchsup.c: CommonSearchInit clears pi->mvBest on both its IsDraw and
  too-deep early-exit paths to prevent a later sibling node from reading
  stale mvBest data left by an unrelated earlier visit to the same ply.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01C9r36BydPnfpyYLFhkTo6d

1 files changed, 199 insertions, 66 deletions

diff --git a/src/searchsup.c b/src/searchsup.c
index 41415dc..32e1be8 100644
--- a/src/searchsup.c
+++ b/src/searchsup.c
@@ -201,12 +201,22 @@ ThreadUnderTerminatingSplit(IN SEARCHER_THREAD_CONTEXT *ctx)
 {
     ULONG u;
 
+    //
+    // A thread can be nested in more than one split at once (a helper
+    // thread can itself call StartParallelSearch while inside
+    // HelpSearch for an outer split), so pSplitInfo[] is a stack, not
+    // a single slot -- slot 0 is the outermost split this thread is
+    // part of, later slots are progressively more nested ones. Any of
+    // them terminating means this thread's current work is moot, so
+    // all populated slots must be checked, not just the first one
+    // found.
+    //
     for (u = 0; u < NUM_SPLIT_PTRS_IN_CONTEXT; u++)
     {
-        if (ctx->pSplitInfo[u] != NULL)
+        if ((ctx->pSplitInfo[u] != NULL) &&
+            (ctx->pSplitInfo[u]->fTerminate == TRUE))
         {
-            if (ctx->pSplitInfo[u]->fTerminate == TRUE) return(TRUE);
-            return(FALSE);
+            return(TRUE);
         }
     }
     return(FALSE);
@@ -260,63 +270,66 @@ Return value:
 
 
 SCORE
-ComputeMoveScore(IN SEARCHER_THREAD_CONTEXT *ctx,
-                 IN MOVE mv,
-                 IN ULONG uMoveNum)
+DecodeMoveValueEstimate(IN MOVE mv,
+                        IN SCORE iRawValue,
+                        IN MOVE_SCORING_SCHEME eScheme)
+/**
+
+Routine description:
+
+    generate.c has (at least) four distinct conventions for packing
+    sMoveStack[].iValue (see MOVE_SCORING_SCHEME's own comment, chess.h),
+    plus two more that aren't generate.c tier encodings at all
+    (RescoreMovesViaSearch's real eval-axis score, and root.c's node-
+    count/score composite). This function is the one place that knows
+    how to turn a raw iValue back into an eval-axis (100=1-pawn)
+    material estimate, given which scheme produced it -- every caller
+    that needs an actual value rather than just a tier/leftover
+    classification should go through here rather than inlining its own
+    guess at the encoding.
+
+Parameters:
+
+    IN MOVE mv,
+    IN SCORE iRawValue,
+    IN MOVE_SCORING_SCHEME eScheme
+
+Return value:
+
+    SCORE
+
+**/
 {
-    SCORE iMoveScore = (PIECE_VALUE(mv.pCaptured) +
-                        PIECE_VALUE(mv.pPromoted));
-    if (uMoveNum != (ULONG)-1)
+    switch (eScheme)
     {
-        ASSERT(uMoveNum < MAX_MOVE_STACK);
-        ASSERT(IS_SAME_MOVE(mv, ctx->sMoveStack.mvf[uMoveNum].mv));
-        ASSERT(ctx->uPly > 0);
-        iMoveScore = ctx->sMoveStack.mvf[uMoveNum].iValue;
-        // Every current caller (search.c's EFP check, and
-        // ComputeMoveExtension's two ComputeMoveScore() call sites) only
-        // reaches here after MakeMove(ctx, mv) has already succeeded, so
-        // ctx->uPly is always the *child's* ply here -- uMoveNum indexes
-        // the parent's move list, i.e. ctx->uPly - 1, not ctx->uPly. That's
-        // the ply RescoreMovesViaSearch (if it ran) would have rescored,
-        // so that's the flag to check.
-        if (TRUE == ctx->sPlyInfo[ctx->uPly - 1].fMovesRescoredByIID)
-        {
-            // RescoreMovesViaSearch already put a real, searched eval-axis
-            // score here -- better than SEE/MVV-LVA, since it reflects an
-            // entire subtree, not just the immediate exchange. Trust it
-            // exactly as-is; don't run it through the capture-bias
-            // subtraction or collapse it via MIN0, both of which assume
-            // generate.c's ordering-encoded format, which this isn't.
-        }
-        else if ((iMoveScore >= SORT_THESE_FIRST) && IS_CAPTURE_OR_PROMOTION(mv))
-        {
-            ASSERT(iMoveScore > 0);
-            iMoveScore &= STRIP_OFF_FLAGS;
+        case MOVE_SCORING_IID_RESCORED:
+            // Already a real, searched eval-axis score (reflects an
+            // entire subtree, not just the immediate exchange) --
+            // trust it exactly as-is.
+            return(iRawValue);
 
-            // _ScoreAllMoves (generate.c) bakes a flat +120 move-ordering
-            // bias plus small MVV-LVA tie-break nudges
-            // (PIECE_VALUE_OVER_100 terms) on top of the real SEE/
-            // material-diff value for winning/even captures and
-            // promotions -- harmless for sorting (every capture gets
-            // the same treatment), but this function's callers use the
-            // result as an eval-axis quantity (compared against
-            // material-scale margins in futility/extension code), so
-            // back the ordering-only bias back out to recover the pure
-            // SEE/material-diff value, same axis as the raw
-            // PIECE_VALUE() fallback below.
-            iMoveScore -= (PIECE_VALUE_OVER_100(mv.pCaptured) + 120 +
-                          PIECE_VALUE_OVER_100(mv.pPromoted) -
-                          PIECE_VALUE_OVER_100(mv.pMoved));
-        }
-        else
-        {
-            // All quiet moves (including killer/killer-mate quiet moves,
-            // which can reach SORT_THESE_FIRST or the killer-tier bits via
-            // generate.c's ordering bonuses with no relation to a move's
-            // real eval-axis value) collapse to 0 here -- correct, since
-            // this function's contract is "estimate the move's value on
-            // the 100=1-pawn axis," and a quiet move's ordering bonus
-            // carries no such estimate.
+        case MOVE_SCORING_NORMAL:
+        case MOVE_SCORING_QSEARCH_NO_CHECKS:
+            // Identical +120 flat move-ordering bias plus small
+            // MVV-LVA tie-break nudges for winning/even captures and
+            // promotions -- harmless for sorting, but wrong on the
+            // eval axis until backed out.
+            if ((iRawValue >= SORT_THESE_FIRST) && IS_CAPTURE_OR_PROMOTION(mv))
+            {
+                ASSERT(iRawValue > 0);
+                iRawValue &= STRIP_OFF_FLAGS;
+                iRawValue -= MOVE_SCORE_ORDERING_BIAS(mv);
+                return(iRawValue);
+            }
+            // All quiet moves (including killer/killer-mate quiet
+            // moves, which can reach SORT_THESE_FIRST or the killer-
+            // tier bits via generate.c's ordering bonuses with no
+            // relation to a move's real eval-axis value) collapse to
+            // 0 here -- correct, since a quiet move's ordering bonus
+            // carries no such estimate. A genuinely negative raw value
+            // (a losing capture, scored without any tier flag) passes
+            // through MIN0 unchanged, preserving its real (negative)
+            // material estimate.
             //
             // FUTURE WORK: 0 is uniform across every quiet move, which
             // throws away signal we plausibly have. Two candidate
@@ -334,8 +347,90 @@ ComputeMoveScore(IN SEARCHER_THREAD_CONTEXT *ctx,
             //      moves -- eval.c's real value is far more than per-square
             //      tables (mobility, king safety, pawn structure), so this
             //      would only ever be directionally suggestive, not exact.
-            iMoveScore = MIN0(iMoveScore);
-        }
+            return(MIN0(iRawValue));
+
+        case MOVE_SCORING_QSEARCH_CHECKS:
+            // Same +120 convention, plus two QSearch-only ordering
+            // nudges baked into the same value by
+            // _ScoreQSearchMovesInclChecks: +4 for a checking move,
+            // +64 for a same-square recapture. The checking-move term
+            // is backed out here; the same-square-recapture term isn't
+            // (this function doesn't receive mvLast) -- a known,
+            // small (<=64, well under a minor piece) residual
+            // imprecision for that specific case, not silently wrong
+            // by an unbounded amount the way the pre-fix code was for
+            // escapes.
+            if ((iRawValue >= SORT_THESE_FIRST) && IS_CAPTURE_OR_PROMOTION(mv))
+            {
+                ASSERT(iRawValue > 0);
+                iRawValue &= STRIP_OFF_FLAGS;
+                iRawValue -= MOVE_SCORE_ORDERING_BIAS(mv);
+                if (IS_CHECKING_MOVE(mv))
+                {
+                    iRawValue -= 4;
+                }
+                return(iRawValue);
+            }
+            return(MIN0(iRawValue));
+
+        case MOVE_SCORING_ESCAPE:
+            // _ScoreAllEscapes' winning-capture encoding uses a
+            // DIFFERENT bias than _ScoreAllMoves (no +120) plus an
+            // IS_KING(mv.pMoved) right-shift that's lossy -- can't be
+            // inverted exactly. Even captures are tagged FIRST_KILLER,
+            // not SORT_THESE_FIRST, so they would not even reach a
+            // capture-decode branch keyed on that bit. Rather than try
+            // to invert either packed encoding, just recompute the
+            // cheap raw material estimate directly from the move's own
+            // fields -- exactly what _ScoreAllEscapes itself computes
+            // as ITS OWN first-pass estimate before ever calling SEE
+            // (deliberately not re-invoking SEE here -- same axis,
+            // just cheaper and not dependent on reversing a lossy
+            // encoding).
+            if (IS_CAPTURE_OR_PROMOTION(mv))
+            {
+                return((SCORE)(PIECE_VALUE(mv.pCaptured) -
+                               PIECE_VALUE(mv.pMoved)));
+            }
+            return(0);
+
+        case MOVE_SCORING_ROOT:
+        default:
+            // Not a generate.c tier encoding at all -- a node-count/
+            // score composite (root.c), only ever populates ply 0's
+            // move stack. No current caller asks to decode a root-ply
+            // move here; fail loudly rather than silently return a
+            // number that looks plausible but isn't.
+            ASSERT(FALSE);
+            return(0);
+    }
+}
+
+
+SCORE
+ComputeMoveScore(IN SEARCHER_THREAD_CONTEXT *ctx,
+                 IN MOVE mv,
+                 IN ULONG uMoveNum)
+{
+    SCORE iMoveScore = (PIECE_VALUE(mv.pCaptured) +
+                        PIECE_VALUE(mv.pPromoted));
+    if (uMoveNum != (ULONG)-1)
+    {
+        ASSERT(uMoveNum < MAX_MOVE_STACK);
+        ASSERT(IS_SAME_MOVE(mv, ctx->sMoveStack.mvf[uMoveNum].mv));
+        ASSERT(ctx->uPly > 0);
+        // Every current caller (search.c's EFP check, and
+        // ComputeMoveExtension's two ComputeMoveScore() call sites) only
+        // reaches here after MakeMove(ctx, mv) has already succeeded, so
+        // ctx->uPly is always the *child's* ply here -- uMoveNum indexes
+        // the parent's move list, i.e. ctx->uPly - 1, not ctx->uPly. That's
+        // the ply whatever scoring pass ran (generate.c's scorers,
+        // RescoreMovesViaSearch) actually scored, so that's the ply
+        // whose eMoveScoringScheme is authoritative here.
+        iMoveScore = DecodeMoveValueEstimate(
+            mv,
+            ctx->sMoveStack.mvf[uMoveNum].iValue,
+            ctx->sPlyInfo[ctx->uPly - 1].eMoveScoringScheme);
     }
     return(iMoveScore);
 }
@@ -735,7 +830,7 @@ Return value:
         {
             ctx->sMoveStack.mvf[x].iValue = rgScores[x - uBegin];
         }
-        ctx->sPlyInfo[ctx->uPly].fMovesRescoredByIID = TRUE;
+        ctx->sPlyInfo[ctx->uPly].eMoveScoringScheme = MOVE_SCORING_IID_RESCORED;
     }
     else
     {
@@ -750,11 +845,11 @@ Return value:
         // ordering value completely untouched and just bump uBest into
         // killer-tier territory so the normal (non-rescored) selection
         // path -- SelectBestWithHistory/NoHistory -- picks it first, the
-        // same trick generate.c uses for a real killer move. Do NOT set
-        // fMovesRescoredByIID: this ply's iValue is back to being
-        // generate.c's ordering encoding, not real scores, for every
-        // move including uBest (whose bump is itself just an ordering
-        // flag, not part of its own eval-axis value).
+        // same trick generate.c uses for a real killer move. Do NOT
+        // change eMoveScoringScheme: this ply's iValue is back to being
+        // whatever generate.c scheme originally scored it, not real
+        // scores, for every move including uBest (whose bump is itself
+        // just an ordering flag, not part of its own eval-axis value).
         ctx->sMoveStack.mvf[uBest].iValue |= FIRST_KILLER;
     }
 
@@ -1028,6 +1123,15 @@ Return value:
     //
     if (TRUE == IsDraw(ctx))
     {
+        // Every early exit from this function must clear pi->mvBest
+        // itself, since the real reset further down is never reached
+        // from here. Found via a DEBUG-build assertion failure:
+        // TryNullmovePruning reading (pi+1)->mvBest as if it reflected
+        // the null-move probe's own search, when it was actually stale
+        // data left by a completely unrelated earlier node that once
+        // occupied the same ply slot and bailed out via this exact path
+        // without ever being cleared.
+        pi->mvBest.uMove = 0;
         INC(ctx->sCounters.tree.u64LeafCount);
         *piScore = g_iDrawScore[ctx->sPosition.uToMove];
         if ((*piAlpha < *piScore) && (*piScore < *piBeta))
@@ -1084,6 +1188,8 @@ Return value:
             *piScore = Eval(ctx, *piAlpha, *piBeta, NULL);
             pi->iEval = *piScore;
         }
+        // See the IsDraw branch above -- same staleness hazard, same fix.
+        pi->mvBest.uMove = 0;
         DTTRACE("<toodeep/>");
         goto end;
     }
@@ -1131,6 +1237,33 @@ Return value:
     pi->iAlpha = *piAlpha;
     pi->iBeta = *piBeta;
 #endif
+    // Classic PVS node typing (see NODE_TYPE's definition in chess.h).
+    // Centralized here rather than in each caller's move loop
+    // (search.c, root.c, split.c) since CommonSearchInit is the one
+    // place both Search() and QSearch() funnel through -- a wide
+    // window is unambiguously PV regardless of move number (this
+    // covers "first move of a PV node" for free, no need to track
+    // which move number got us here); a narrow window's type depends
+    // only on the PARENT's own type, not on move number either: a CUT
+    // parent's children are ALL (expected to fail low, must search
+    // everything to prove it), and both a PV parent's non-first
+    // children AND an ALL parent's children are CUT (expected to fail
+    // high). The ctx->uPly==0 guard is defensive only -- root is
+    // always called with a wide window in practice, so the width check
+    // above already handles it.
+    if (*piBeta != *piAlpha + 1)
+    {
+        pi->eNodeType = NODE_PV;
+    }
+    else if (ctx->uPly == 0)
+    {
+        pi->eNodeType = NODE_CUT;
+    }
+    else
+    {
+        NODE_TYPE eParentType = ctx->sPlyInfo[ctx->uPly - 1].eNodeType;
+        pi->eNodeType = (NODE_CUT == eParentType) ? NODE_ALL : NODE_CUT;
+    }
     fRet = FALSE;
 
  end:
@@ -1182,7 +1315,7 @@ FLAG WeShouldTryNullmovePruning(SEARCHER_THREAD_CONTEXT *ctx,
         (pos->uNonPawnCount[pos->uToMove][0] > 2) &&
         (FALSE == pi->fInCheck) &&
         (iBeta != +INFINITY) &&
-        (FALSE == pi->fPvNode))
+        (NODE_PV != pi->eNodeType))
     {
         if (uNullDepth <= 6 * ONE_PLY)
         {