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, 168 insertions, 21 deletions

diff --git a/src/search.c b/src/search.c
index 8ee2d3b..4e89a2c 100644
--- a/src/search.c
+++ b/src/search.c
@@ -235,8 +235,13 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
     }
     DTEnterNode(ctx, uDepth, FALSE, iAlpha, iBeta);
     iInitialAlpha = iAlpha;
-    pi->fPvNode = (iBeta != iAlpha + 1);
-    pi->fMovesRescoredByIID = FALSE;
+    // pi->eNodeType is set centrally by CommonSearchInit (see its own
+    // comment) -- covers both Search() and QSearch() from one place.
+    // Safe default until the real move-generation/scoring call below
+    // sets this to whatever scheme actually ran (generate.c's four
+    // _Score* functions and RescoreMovesViaSearch are the authoritative
+    // setters -- see MOVE_SCORING_SCHEME's own comment in chess.h).
+    pi->eMoveScoringScheme = MOVE_SCORING_NORMAL;
     ASSERT((IS_CHECKING_MOVE(mvLast) && (TRUE == pi->fInCheck)) ||
            (!IS_CHECKING_MOVE(mvLast) && (FALSE == pi->fInCheck)));
 
@@ -472,7 +477,7 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
                     // we generated at this ply using a shallower
                     // search.  "Internal Iterative Deepening" or
                     // something like it.
-                    if ((TRUE == pi->fPvNode) &&
+                    if ((NODE_PV == pi->eNodeType) &&
                         (mvHash.uMove == 0) &&
                         (ctx->sMoveStack.mvf[x].iValue < SORT_THESE_FIRST) &&
                         (0 == (ctx->sMoveStack.mvf[x].iValue &
@@ -530,7 +535,7 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
                 // this closes a real gap rather than relying on it not
                 // mattering in practice.
                 ASSERT(!uFutilityMargin);
-                if ((FALSE == pi->fPvNode) &&
+                if ((NODE_PV != pi->eNodeType) &&
                     (iOrigExtend == 0) &&
                     (ctx->uPly >= 2) &&
                     (ctx->sPlyInfo[ctx->uPly - 2].iExtensionAmount <= 0))
@@ -590,7 +595,7 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
                     // never classifies an IID-rescored move as a
                     // leftover (matches pre-retirement behavior).
                     fIsLeftoverMove = FALSE;
-                    if (TRUE == pi->fMovesRescoredByIID)
+                    if (MOVE_SCORING_IID_RESCORED == pi->eMoveScoringScheme)
                     {
                         SelectBestNoHistory(ctx, x);
                     }
@@ -650,7 +655,7 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
         // g_fCanSplit[] indexing ever evaluates it.
         if (((uLegalMoves >= 2) && fIsLeftoverMove) &&
             (0 != g_uNumHelpersAvailable) &&
-            (FALSE == pi->fMovesRescoredByIID) &&
+            (MOVE_SCORING_IID_RESCORED != pi->eMoveScoringScheme) &&
             (0 == uFutilityMargin) &&
             (uDepth >= ONE_PLY) &&
             (TRUE == g_fCanSplit[uDepth / ONE_PLY - 1]) &&
@@ -681,6 +686,52 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
             if (IS_VALID_SCORE(iScore))
             {
                 pi->mvBest = mvBest;
+                if (iBestScore >= iBeta)
+                {
+                    //
+                    // The split failed high. StartParallelSearch leaves
+                    // iAlpha untouched on this path (see its own
+                    // comment), so falling through to no_more_moves'
+                    // generic iAlpha-moved classifier would misread
+                    // this as a fail-low/exact-score case -- skipping
+                    // UpdateDynamicMoveOrdering entirely and storing
+                    // the fail-high score as an (inverted-sense,
+                    // unsound) upper bound instead of a lower bound.
+                    // Handle it explicitly here instead, matching the
+                    // non-split fail-high branch above -- except for
+                    // uCurrent: mvBest may have been found by a helper
+                    // thread with its own separate ctx->sMoveStack, not
+                    // this (originating) thread's, so passing this
+                    // thread's own move index (the non-split call's
+                    // convention) doesn't identify mvBest's position in
+                    // any coherent move array. Pass 0 to skip the
+                    // "decrement history for moves tried earlier at
+                    // this node" loop entirely -- there's no single
+                    // thread-local move sequence to decrement across a
+                    // split, so it's dropped rather than fed a bogus
+                    // index (which previously tripped this exact
+                    // assert in dynamic.c: mvf[uCurrent] wasn't
+                    // mvBest). The killer/counter-move/history-increment
+                    // side of the update below is unaffected.
+                    //
+                    if (!IS_CAPTURE_OR_PROMOTION(mvBest))
+                    {
+                        UpdateDynamicMoveOrdering(ctx,
+                                                  uDepth,
+                                                  mvBest,
+                                                  iBestScore,
+                                                  0);
+                    }
+                    StoreLowerBound(mvBest, pos, iBestScore, uDepth, fThreat);
+                    if (mvBest.pCaptured && !IS_PAWN(mvBest.pCaptured) &&
+                        (iBeta < +NMATE))
+                    {
+                        ASSERT(ctx->uPly > 0);
+                        RecordEnprisePieceAtPly(ctx, ctx->uPly - 1,
+                                                mvBest.cTo);
+                    }
+                    goto end;
+                }
                 goto no_more_moves;
             }
             else
@@ -735,8 +786,7 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
             // Decide how much (if any) to reduce this move's depth --
             // graded LMR.
             if ((uDepth > TWO_PLY) &&
-                !pi->fPvNode &&
-                !(ctx->sPlyInfo[ctx->uPly - 1].fPvNode) &&
+                (NODE_PV != pi->eNodeType) &&
                 (uLegalMoves > 5) &&
                 (0 == iExtend) &&
                 (!IS_ESCAPING_CHECK(mv)) &&
@@ -757,7 +807,6 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
                 {
                     ASSERT(iExtend == 0);
                     iExtend = iLMR;
-                    pi->iExtensionAmount = iLMR;
                 }
             }
 
@@ -828,6 +877,16 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
                     INC(ctx->sCounters.tree.u64CMEvidenceTries[uCMEvidenceBucket]);
                 }
 #endif
+                // Record the extension/reduction actually applied to
+                // THIS move, unconditionally -- not just when it's
+                // negative. Leaving this stale from whatever earlier
+                // move at this ply last happened to set it let a child
+                // node checking "was my parent's move reduced" via
+                // (pi-1)->iExtensionAmount < 0 read a leftover value
+                // from a completely different sibling move, not the
+                // one actually played.
+                pi->iExtensionAmount = iExtend;
+
                 // Compute the next search depth for this move/subtree.
                 uNextDepth = uDepth - ONE_PLY + iExtend;
                 if (uNextDepth >= MAX_DEPTH_PER_SEARCH) uNextDepth = 0;
@@ -1046,6 +1105,34 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
     ASSERT(IS_VALID_SCORE(iAlpha));
     ASSERT(IS_VALID_SCORE(iBestScore) || WE_SHOULD_STOP_SEARCHING);
     ASSERT(PositionsAreEquivalent(pos, &pi->sPosition));
+#ifdef PERF_COUNTERS
+    if ((!WE_SHOULD_STOP_SEARCHING) && IS_VALID_SCORE(iBestScore))
+    {
+        FLAG fFailedLow = (iBestScore <= iAlpha);
+        FLAG fFailedHigh = (iBestScore >= iBeta);
+        switch (pi->eNodeType)
+        {
+            case NODE_PV:
+                if (fFailedLow || fFailedHigh)
+                {
+                    INC(ctx->sCounters.tree.u64PVNodeSurprised);
+                }
+                break;
+            case NODE_ALL:
+                if (fFailedHigh)
+                {
+                    INC(ctx->sCounters.tree.u64AllNodeFailedHigh);
+                }
+                break;
+            case NODE_CUT:
+                if (fFailedLow)
+                {
+                    INC(ctx->sCounters.tree.u64CutNodeFailedLow);
+                }
+                break;
+        }
+    }
+#endif
     DTLeaveNode(ctx, FALSE, iBestScore, mvBest);
     return(iBestScore);
 }
@@ -1400,9 +1487,17 @@ _ShouldWeConsiderThisMove(IN SEARCHER_THREAD_CONTEXT *ctx,
         if (iMoveValue >= SORT_THESE_FIRST)
         {
             fHaveMoveValue = TRUE;
-            iMoveValue &= STRIP_OFF_FLAGS;
-            ASSERT(iMoveValue >= 0);
-            iMoveValue -= MOVE_SCORE_ORDERING_BIAS(mv);
+            // This function is only ever called outside of check (see
+            // the ASSERTs above), so uMoveNum always indexes one of
+            // QSearch's two schemes -- never MOVE_SCORING_ESCAPE. Which
+            // of the two matters here: InclChecks bakes in a +4
+            // checking-move nudge NoChecks doesn't, so a raw
+            // MOVE_SCORE_ORDERING_BIAS subtraction alone (correct for
+            // NoChecks) silently over-values a checking capture scored
+            // under InclChecks by 4. DecodeMoveValueEstimate (searchsup.c)
+            // is the one place that knows the difference.
+            iMoveValue = DecodeMoveValueEstimate(
+                mv, iMoveValue, ctx->sPlyInfo[ctx->uPly].eMoveScoringScheme);
             if (mv.pCaptured)
             {
                 // If there are very few pieces left on the board,
@@ -1572,7 +1667,7 @@ _ShouldWeConsiderThisMove(IN SEARCHER_THREAD_CONTEXT *ctx,
 // hundreds of centipawns short of the bar, regular-lazy was already
 // close to safe (1.26% down to 0.43%), super-lazy was only risky very
 // close to the bar (7.96% at distance 0-25, 0.54% by 400+). Indexed
-// by ctx->uLastPositionalSource (set inside Eval() -- see
+// by PLY_INFO.uLastPositionalSource (set inside Eval() -- see
 // EVAL_POSITIONAL_SOURCE_* in chess.h). Provisional, derived from one
 // run at sd 6; re-derive with `calibrate qsearchfutility` if search
 // behavior affecting typical qsearch iEval/iFutility gaps changes.
@@ -1691,7 +1786,8 @@ QSearchFromCheckNoStandPat(IN SEARCHER_THREAD_CONTEXT *ctx,
             if (FALSE == fEvalComputed)
             {
                 iEval = Eval(ctx, iAlpha, iBeta, &rgiPositional);
-                ASSERT(ctx->uLastPositionalSource < EVAL_POSITIONAL_SOURCE_COUNT);
+                ASSERT(ctx->sPlyInfo[ctx->uPly].uLastPositionalSource <
+                       EVAL_POSITIONAL_SOURCE_COUNT);
                 fEvalComputed = TRUE;
             }
 
@@ -1705,9 +1801,16 @@ QSearchFromCheckNoStandPat(IN SEARCHER_THREAD_CONTEXT *ctx,
             // so this reuses the same margin rather than inventing a
             // separate, unvalidated one -- better to be at least as
             // conservative as the calibrated case than to guess looser.
+            // uLastPositionalSource is read off this ply's own
+            // PLY_INFO slot, not a per-thread scalar, so a recursive
+            // QSearch() call for an earlier move in this same loop
+            // (which writes ctx->sPlyInfo[ctx->uPly+1], a different
+            // slot) can't clobber it before a later iteration reads it
+            // back.
             if (iEval < iAlpha &&
                 ((iEval + PIECE_VALUE(mv.pCaptured)) + rgiPositional[pos->uToMove] +
-                 FUTILITY_BASE_MARGIN_BY_SOURCE[ctx->uLastPositionalSource]) < iAlpha)
+                 FUTILITY_BASE_MARGIN_BY_SOURCE[
+                     ctx->sPlyInfo[ctx->uPly].uLastPositionalSource]) < iAlpha)
             {
                 continue;
             }
@@ -1761,6 +1864,13 @@ QSearchFromCheckNoStandPat(IN SEARCHER_THREAD_CONTEXT *ctx,
  end:
     ASSERT((uLegalMoves > 0) || (iBestScore <= -NMATE));
     ASSERT(IS_VALID_SCORE(iBestScore) || WE_SHOULD_STOP_SEARCHING);
+    // No node-type-surprise counting here (unlike Search()/QSearch()'s
+    // own "end:" labels) -- this function has exactly one call site
+    // (QSearch(), which does `goto end` immediately after calling this
+    // at the SAME ply, no MakeMove in between) and QSearch()'s own
+    // "end:" block already counts that identical (iBestScore, ctx->uPly,
+    // eNodeType) outcome; counting here too would double-count every
+    // in-check qsearch node.
     return(iBestScore);
 }
 
@@ -1882,7 +1992,6 @@ QSearch(IN SEARCHER_THREAD_CONTEXT *ctx,
     ASSERT(!InCheck(pos, pos->uToMove));
 
     pi->iEval = iEval = iBestScore = Eval(ctx, iAlpha, iBeta, &rgiPositional);
-
     // If that Eval (above) was full (i.e. not lazy) it may have set
     // en prise and trapped piece indicators.  Likewise, other nodes
     // at this depth may have set en prise piece hints.  If these are
@@ -1948,9 +2057,9 @@ QSearch(IN SEARCHER_THREAD_CONTEXT *ctx,
         // would be for the qsearch-futility calibration harness.
         iFutility = -NMATE;
 #else
-        ASSERT(ctx->uLastPositionalSource < EVAL_POSITIONAL_SOURCE_COUNT);
+        ASSERT(pi->uLastPositionalSource < EVAL_POSITIONAL_SOURCE_COUNT);
         iFutility = iAlpha -
-            (FUTILITY_BASE_MARGIN_BY_SOURCE[ctx->uLastPositionalSource] +
+            (FUTILITY_BASE_MARGIN_BY_SOURCE[pi->uLastPositionalSource] +
              rgiPositional[pos->uToMove]) - iEval;
         iFutility = MAX0(iFutility);
 #endif
@@ -2004,7 +2113,7 @@ QSearch(IN SEARCHER_THREAD_CONTEXT *ctx,
                                                , iAlpha
                                                , iBeta
                                                , rgiPositional[pos->uToMove]
-                                               , ctx->uLastPositionalSource
+                                               , pi->uLastPositionalSource
 #endif
                                                ))
         {
@@ -2070,11 +2179,20 @@ QSearch(IN SEARCHER_THREAD_CONTEXT *ctx,
                         // Readjust futility margin here; it can be wider now.
                         if (iAlpha < +NMATE)
                         {
-                            ASSERT(ctx->uLastPositionalSource <
+                            // pi->uLastPositionalSource is this ply's
+                            // own PLY_INFO slot -- the recursive
+                            // QSearch() call just above (for the move
+                            // that raised alpha) wrote its own nested
+                            // Eval() results to ctx->uPly+1, a
+                            // different slot, so this one is
+                            // untouched and still describes the same
+                            // Eval() call that produced iEval/
+                            // rgiPositional above.
+                            ASSERT(pi->uLastPositionalSource <
                                    EVAL_POSITIONAL_SOURCE_COUNT);
                             iFutility = (iAlpha -
                                          (FUTILITY_BASE_MARGIN_BY_SOURCE[
-                                              ctx->uLastPositionalSource] +
+                                              pi->uLastPositionalSource] +
                                           rgiPositional[pos->uToMove]) -
                                          iEval);
                             iFutility = MAX0(iFutility);
@@ -2091,6 +2209,35 @@ QSearch(IN SEARCHER_THREAD_CONTEXT *ctx,
     ASSERT(PositionsAreEquivalent(pos, &pi->sPosition));
     ASSERT(IS_VALID_SCORE(iBestScore) || WE_SHOULD_STOP_SEARCHING);
     DTLeaveNode(ctx, TRUE, iBestScore, pi->mvBest);
+#ifdef PERF_COUNTERS
+    // See Search()'s own "end:" label for the full rationale.
+    if ((!WE_SHOULD_STOP_SEARCHING) && IS_VALID_SCORE(iBestScore))
+    {
+        FLAG fFailedLow = (iBestScore <= iAlpha);
+        FLAG fFailedHigh = (iBestScore >= iBeta);
+        switch (pi->eNodeType)
+        {
+            case NODE_PV:
+                if (fFailedLow || fFailedHigh)
+                {
+                    INC(ctx->sCounters.tree.u64PVNodeSurprised);
+                }
+                break;
+            case NODE_ALL:
+                if (fFailedHigh)
+                {
+                    INC(ctx->sCounters.tree.u64AllNodeFailedHigh);
+                }
+                break;
+            case NODE_CUT:
+                if (fFailedLow)
+                {
+                    INC(ctx->sCounters.tree.u64CutNodeFailedLow);
+                }
+                break;
+        }
+    }
+#endif
 
     // Note: iBestScore can be +INFINITY or -INFINITY here even in the
     // absence of a legitimate mate detected if we disallowed stand