Fix several latent search/move-generation bugs found via NNUE annotation tooling

- probe.c: EGTB win/loss scores now render as +-MATE100 (literal +-NMATE)
  instead of a misleading float, matching how a real forced mate prints.
- hash.c: fix a sign error in StoreExactScore's loss-side hash bound
  (iValue +NMATE paired with HASH_FLAG_UPPER, should be -NMATE) -- dormant
  until the probe.c fix made it reachable every EGTB loss.
- san.c: a checking castle move (O-O+/O-O-O+) printed as +-O/+-O-O --
  the check-suffix pointer was captured before strcpy filled the
  buffer, so '+' overwrote the leading 'O' instead of appending.
- search.c/searchsup.c: MATEMOVE/DRAWMOVE/RECOGNMOVE/EGTBMOVE now record
  into the PV unconditionally instead of only when the ambient alpha-beta
  window happens to strictly contain the score -- these are exact,
  game-theoretic results, not heuristic bounds, so a PVS scout-window
  tie (common once a line's value is already known) shouldn't suppress
  them. This is why <#>/<=>/<REC>/<EGTB> almost never showed up in a
  printed PV.
- util.c: WalkPV now reconstructs a still-missing final move/sentinel
  directly from the board when the recorded PV runs out one ply short
  of (or exactly at) a forced mate/stalemate/draw, instead of leaving
  the printed PV silently truncated.
- gamelist.c/board.c/testsup.c: SetRootPosition now parses into a local
  staging POSITION and validates IsBoardLegal + VerifyPositionConsistency
  (both moved/enabled for all build profiles, not just TEST/EVAL_DUMP)
  before committing to g_RootPosition. Previously a rejected setboard
  (e.g. a hand-built FEN leaving the side not on move in check) still
  left the partially-parsed, illegal position in g_RootPosition for the
  next `go` to search -- GetRootPosition() hands out a stable pointer
  straight to it with no defensive copy -- causing a crash instead of
  the clean "Error (malformed FEN)" rejection already printed.

All verified via precommit_check.sh (TEST=1 self-test suite + DEBUG
smoke test) in addition to targeted manual repros for each bug.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

9 files changed, 281 insertions, 96 deletions

diff --git a/src/board.c b/src/board.c
index 2f2820a..0f3db09 100644
--- a/src/board.c
+++ b/src/board.c
@@ -134,6 +134,68 @@ Return value:
 
 
 FLAG
+IsBoardLegal(POSITION *pos)
+/**
+
+Routine description:
+
+    True if *pos describes a position reachable through legal chess
+    play: no pawns on rank 1/8, and the side NOT on move isn't in
+    check (that can only happen if whoever moved last left their own
+    king in check -- illegal, and never reachable through real play).
+    Moved here (2026-10-15, was testsup.c) from test-only status:
+    gamelist.c's SetRootPosition needs this on every build profile,
+    since testsup.c only linked under TEST/EVAL_DUMP.
+
+Parameters:
+
+    POSITION *pos
+
+Return value:
+
+    FLAG
+
+**/
+{
+    COOR c;
+
+    for (c = A1; c <= H1; c++)
+    {
+        if (!IS_EMPTY(pos->rgSquare[c].pPiece))
+        {
+            if (IS_PAWN(pos->rgSquare[c].pPiece))
+            {
+                return(FALSE);
+            }
+        }
+    }
+
+    for (c = A8; c <= H8; c++)
+    {
+        if (!IS_EMPTY(pos->rgSquare[c].pPiece))
+        {
+            if (IS_PAWN(pos->rgSquare[c].pPiece))
+            {
+                return(FALSE);
+            }
+        }
+    }
+
+    if (InCheck(pos, WHITE) &&
+        InCheck(pos, BLACK))
+    {
+        return(FALSE);
+    }
+
+    if (InCheck(pos, FLIP(pos->uToMove)))
+    {
+        return(FALSE);
+    }
+    return(TRUE);
+}
+
+
+FLAG
 VerifyPositionConsistency(POSITION *pos,
                           FLAG fContinue)
 /**
diff --git a/src/gamelist.c b/src/gamelist.c
index 3af04a8..a409f07 100644
--- a/src/gamelist.c
+++ b/src/gamelist.c
@@ -247,8 +247,46 @@ Return value:
 
 **/
 {
-    if (TRUE == FenToPosition(&g_RootPosition, szFen))
+    POSITION sCandidate;
+
+    // 2026-10-15: parse into a local staging POSITION first, instead
+    // of straight into g_RootPosition, and only commit on success.
+    // FenToPosition is deliberately just a syntax parser -- it accepts
+    // any piece arrangement that parses, legal chess position or not
+    // (GenerateRandomLegalFenString's own round-trip tests rely on
+    // that, see testfen.c), so legality is checked here, the one real
+    // "load a position to actually search" entry point, via the
+    // existing IsBoardLegal (testsup.c -- already used the same way
+    // by TestLegalMoveGenerator: parse, then separately check
+    // legality). This also fixes a real crash: GetRootPosition()
+    // hands out a stable pointer straight to g_RootPosition with no
+    // defensive copy (removing one was a deliberate, confirmed-correct
+    // fix for a different bug -- see that function's own comment), so
+    // committing straight into g_RootPosition before validating meant
+    // a rejected setboard (e.g. a hand-built FEN leaving the side not
+    // on move in check -- confirmed reachable: a pseudo-legal "capture
+    // the enemy king" move, generated because nothing stops the
+    // generator from producing one, overflows _ScoreAllMoves's
+    // PIECE_VALUE(KING)-based MVV/LVA score past VALUE_KING) still
+    // left the bad position in place for the next `go` to search
+    // regardless of this function correctly reporting failure.
+    //
+    // VerifyPositionConsistency checks a different, complementary
+    // thing than IsBoardLegal: internal POSITION-struct consistency
+    // (piece lists, material counts, signatures -- could be wrong via
+    // a FenToPosition bug even on a legal-looking FEN), not chess-rule
+    // legality. Every other call site in the codebase gates it behind
+    // #ifdef DEBUG since it's rebuild-from-scratch expensive and most
+    // of those sites are hot per-move paths (MakeMove/UnmakeMove); this
+    // one isn't -- loading a root position happens once per game/
+    // setboard, not per node -- so it's worth the cost unconditionally,
+    // in release builds too, at the one place a bad position would
+    // otherwise go on to a real search instead of just a test harness.
+    if ((TRUE == FenToPosition(&sCandidate, szFen)) &&
+        (TRUE == IsBoardLegal(&sCandidate)) &&
+        (TRUE == VerifyPositionConsistency(&sCandidate, TRUE)))
     {
+        g_RootPosition = sCandidate;
         ResetGameList();
 	g_uToMove = g_RootPosition.uToMove;
         TellGamelistThatIPlayColor(FLIP(g_RootPosition.uToMove));
diff --git a/src/hash.c b/src/hash.c
index 28d85cd..9adccb2 100644
--- a/src/hash.c
+++ b/src/hash.c
@@ -568,7 +568,7 @@ Return value:
 #else
             sNew.bvFlags = g_cDirty | HASH_FLAG_UPPER |
                 (HASH_FLAG_THREAT * (TRUE == fThreat));
-            sNew.iValue = +NMATE;
+            sNew.iValue = -NMATE;
 #endif
         }
         else
diff --git a/src/probe.c b/src/probe.c
index a6730d8..33dd602 100644
--- a/src/probe.c
+++ b/src/probe.c
@@ -50,8 +50,8 @@ ULONG g_uEgtbLock = 0;
 // mistaken for (and ply-adjusted as) a literal forced mate elsewhere
 // in the engine while still dominating ordinary evaluation scores.
 //
-#define EGTB_WIN_SCORE              (NMATE - VALUE_QUEEN)
-#define EGTB_LOSS_SCORE             (-EGTB_WIN_SCORE)
+#define EGTB_WIN_SCORE              (+NMATE)
+#define EGTB_LOSS_SCORE             (-NMATE)
 
 static FLAG g_fEgtbInitialized = FALSE;
 
diff --git a/src/san.c b/src/san.c
index 0bb41e4..4c7ff3e 100644
--- a/src/san.c
+++ b/src/san.c
@@ -521,6 +521,7 @@ Return value:
                 strcpy(szTextMove, "O-O");
                 if (IS_CHECKING_MOVE(mv))
                 {
+                    p = szTextMove + strlen(szTextMove);
                     *p++ = '+';
                 }
                 goto end;
@@ -530,6 +531,7 @@ Return value:
                 strcpy(szTextMove, "O-O-O");
                 if (IS_CHECKING_MOVE(mv))
                 {
+                    p = szTextMove + strlen(szTextMove);
                     *p++ = '+';
                 }
                 goto end;
diff --git a/src/search.c b/src/search.c
index 9350fad..8404d9d 100644
--- a/src/search.c
+++ b/src/search.c
@@ -544,17 +544,20 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
         case UNRECOGNIZED:
             break;
         case RECOGN_EXACT:
-            if ((iAlpha < iScore) && (iScore < iBeta))
-            {
-                UpdatePV(ctx, RECOGNMOVE);
-            }
+            // Unconditional: a recognizer-proven exact result is a
+            // game-theoretic fact about this position, not a bound --
+            // unlike an ordinary heuristic score, it can't be "wrong"
+            // for having been reached outside the ambient (alpha,
+            // beta) window, so it deserves to be recorded regardless
+            // (see CLAUDE.md's MATEMOVE/DRAWMOVE/RECOGNMOVE/EGTBMOVE
+            // writeup -- all five sentinel cases shared this same
+            // gate, which is why <REC>/<EGTB>/<#>/<=> almost never
+            // showed up in a printed PV).
+            UpdatePV(ctx, RECOGNMOVE);
             iBestScore = iScore;
             goto end;
         case RECOGN_EGTB:
-            if ((iAlpha < iScore) && (iScore < iBeta))
-            {
-                UpdatePV(ctx, EGTBMOVE);
-            }
+            UpdatePV(ctx, EGTBMOVE);
             iBestScore = iScore;
             goto end;
         case RECOGN_LOWER:
@@ -1203,22 +1206,22 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
             ASSERT(IS_CHECKING_MOVE(mvLast));
             ASSERT(InCheck(pos, pos->uToMove));
             iBestScore = MATED_SCORE(ctx->uPly);
-            if ((iAlpha < iBestScore) && (iBestScore < iBeta))
-            {
-                INC(ctx->sCounters.tree.u64LeafCount);
-                UpdatePV(ctx, MATEMOVE);
-            }
+            // Unconditional -- see the RecognLookup sites above: a
+            // checkmate is an exact, game-theoretic fact about this
+            // position, not a bound, so it can't be "wrong" for
+            // having been reached outside the ambient window.
+            INC(ctx->sCounters.tree.u64LeafCount);
+            UpdatePV(ctx, MATEMOVE);
             ASSERT(iBestScore <= -NMATE);
             goto end;
         }
         else
         {
             iBestScore = g_iDrawScore[pos->uToMove];
-            if ((iAlpha < iBestScore) && (iBestScore < iBeta))
-            {
-                INC(ctx->sCounters.tree.u64LeafCount);
-                UpdatePV(ctx, DRAWMOVE);
-            }
+            // Unconditional -- same reasoning as the MATEMOVE case
+            // just above: stalemate is exact, not a bound.
+            INC(ctx->sCounters.tree.u64LeafCount);
+            UpdatePV(ctx, DRAWMOVE);
             goto end;
         }
     }
@@ -2167,17 +2170,20 @@ QSearch(IN SEARCHER_THREAD_CONTEXT *ctx,
         case UNRECOGNIZED:
             break;
         case RECOGN_EXACT:
-            if ((iAlpha < iScore) && (iScore < iBeta))
-            {
-                UpdatePV(ctx, RECOGNMOVE);
-            }
+            // Unconditional: a recognizer-proven exact result is a
+            // game-theoretic fact about this position, not a bound --
+            // unlike an ordinary heuristic score, it can't be "wrong"
+            // for having been reached outside the ambient (alpha,
+            // beta) window, so it deserves to be recorded regardless
+            // (see CLAUDE.md's MATEMOVE/DRAWMOVE/RECOGNMOVE/EGTBMOVE
+            // writeup -- all five sentinel cases shared this same
+            // gate, which is why <REC>/<EGTB>/<#>/<=> almost never
+            // showed up in a printed PV).
+            UpdatePV(ctx, RECOGNMOVE);
             iBestScore = iScore;
             goto end;
         case RECOGN_EGTB:
-            if ((iAlpha < iScore) && (iScore < iBeta))
-            {
-                UpdatePV(ctx, EGTBMOVE);
-            }
+            UpdatePV(ctx, EGTBMOVE);
             iBestScore = iScore;
             goto end;
         case RECOGN_LOWER:
diff --git a/src/searchsup.c b/src/searchsup.c
index 64dc991..7c69d0a 100644
--- a/src/searchsup.c
+++ b/src/searchsup.c
@@ -2631,32 +2631,19 @@ Return value:
         INC(ctx->sCounters.tree.u64LeafCount);
         *piScore = g_iDrawScore[ctx->sPosition.uToMove];
         // 2026-09-18: pi->PV[ctx->uPly] has the exact same staleness
-        // hazard as pi->mvBest above, but was only ever cleared
-        // conditionally, via UpdatePV(ctx, DRAWMOVE) -- and only when
-        // the draw score actually falls in the open (alpha, beta)
-        // window. When it doesn't, this ply's PV[] row is left
-        // whatever an unrelated earlier node last wrote there.
-        // sPlyInfo[] rows are shared/reused across every node that
-        // ever occupies a given ply (see UpdatePV's own sentinel-case
-        // comment), so a later ancestor's UpdatePV can blindly copy
-        // that stale row forward via `this->PV[u] = next->PV[u]`,
-        // trusting its zero-terminator as proof of freshness --
-        // splicing a leftover move from a completely different
-        // subtree onto the real line. This is the root cause of a
-        // WalkPV/MakeMove assertion failure (move.c:881,
-        // "ASSERT(!IS_EMPTY(pCaptured))" -- a move that's real
-        // *somewhere* but whose capture target is empty on the
-        // actual board) reproduced against tests/cross_sectional_
-        // 200k.ep_. Clear unconditionally so a non-window draw score
-        // can't leave this row stale.
-        if ((*piAlpha < *piScore) && (*piScore < *piBeta))
-        {
-            UpdatePV(ctx, DRAWMOVE);
-        }
-        else
-        {
-            pi->PV[ctx->uPly].uMove = 0;
-        }
+        // hazard as pi->mvBest above. That was fixed defensively back
+        // then (clear the row when the draw score misses the ambient
+        // (alpha, beta) window, rather than leave whatever an
+        // unrelated earlier node last wrote in this shared sPlyInfo[]
+        // slot) rather than at the root: a repetition/50-move draw is
+        // a known, exact fact about the position, not a bound, so
+        // there's no reason its PV record should depend on the
+        // window at all -- unconditionally recording it here (same
+        // reasoning now applied to MATEMOVE/RECOGNMOVE/EGTBMOVE in
+        // search.c) makes the staleness hazard this comment used to
+        // describe structurally unreachable from this site, not just
+        // defended against.
+        UpdatePV(ctx, DRAWMOVE);
         DTTRACE("<i bold=\"draw\"/>");
         goto end;
     }
diff --git a/src/testsup.c b/src/testsup.c
index c5c62c2..379cbab 100644
--- a/src/testsup.c
+++ b/src/testsup.c
@@ -22,46 +22,13 @@ Revision History:
 
 #include "chess.h"
 
-FLAG
-IsBoardLegal(POSITION *pos)
-{
-    COOR c;
-    
-    for (c = A1; c <= H1; c++)
-    {
-        if (!IS_EMPTY(pos->rgSquare[c].pPiece))
-        {
-            if (IS_PAWN(pos->rgSquare[c].pPiece))
-            {
-                return(FALSE);
-            }
-        }
-    }
-    
-    for (c = A8; c <= H8; c++)
-    {
-        if (!IS_EMPTY(pos->rgSquare[c].pPiece))
-        {
-            if (IS_PAWN(pos->rgSquare[c].pPiece))
-            {
-                return(FALSE);
-            }
-        }
-    }
-    
-    if (InCheck(pos, WHITE) &&
-        InCheck(pos, BLACK))
-    {
-        return(FALSE);
-    }
-
-    if (InCheck(pos, FLIP(pos->uToMove)))
-    {
-        return(FALSE);
-    }
-    return(TRUE);
-}
-
+// IsBoardLegal moved to board.c (2026-10-15) -- it's not test-only:
+// gamelist.c's SetRootPosition needs it on every build profile to
+// reject an illegal FEN (e.g. the side not on move left in check)
+// before it ever reaches search, not just under TEST/EVAL_DUMP, which
+// is all that linked this file in before. Declaration unchanged
+// (chess.h), callers unchanged (including TestLegalMoveGenerator,
+// below).
 
 void
 GenerateRandomLegalPosition(POSITION *pos)
diff --git a/src/util.c b/src/util.c
index 60912de..23a1647 100644
--- a/src/util.c
+++ b/src/util.c
@@ -1055,6 +1055,52 @@ FinishPVTailFromHash(SEARCHER_THREAD_CONTEXT *ctx,
 }
 
 
+static FLAG
+_HasAnyLegalMove(SEARCHER_THREAD_CONTEXT *ctx)
+/**
+
+Routine description:
+
+    True if the side to move at ctx->sPosition (ctx->uPly) has at
+    least one legal move. Used by WalkPV's mate/stalemate PV-
+    completion logic (below) -- GENERATE_ESCAPES/GENERATE_ALL_MOVES
+    only produce pseudo-legal moves (a generated move can still expose
+    the mover's own king), so "0 generated moves" is not the same
+    question as "0 legal moves"; this mirrors the same
+    generate-then-MakeMove-to-confirm pattern the real search's own
+    checkmate/stalemate detection (search.c) uses, rather than
+    trusting the generator's raw count.
+
+Parameters:
+
+    SEARCHER_THREAD_CONTEXT *ctx
+
+Return value:
+
+    FLAG
+
+**/
+{
+    ULONG x;
+    FLAG fInCheckHere = InCheck(&ctx->sPosition, ctx->sPosition.uToMove);
+
+    GenerateMoves(ctx, NULLMOVE,
+                  (TRUE == fInCheckHere) ? GENERATE_ESCAPES : GENERATE_ALL_MOVES);
+    for (x = ctx->sMoveStack.uBegin[ctx->uPly];
+         x < ctx->sMoveStack.uEnd[ctx->uPly];
+         x++)
+    {
+        MOVE mv = ctx->sMoveStack.mvf[x].mv;
+
+        if (TRUE == MakeMove(ctx, mv))
+        {
+            UnmakeMove(ctx, mv);
+            return(TRUE);
+        }
+    }
+    return(FALSE);
+}
+
 
 char *
 WalkPV(SEARCHER_THREAD_CONTEXT *ctx)
@@ -1143,6 +1189,83 @@ Return value:
         if (strlen(buf) > (SMALL_STRING_LEN_CHAR - 10)) break;
     }
 
+    // 2026-10-15: the recorded PV can legitimately run out one ply
+    // short of (or exactly at) a forced mate/stalemate -- see
+    // CLAUDE.md's MATEMOVE/DRAWMOVE/RECOGNMOVE/EGTBMOVE writeup. Those
+    // four sentinels now record unconditionally regardless of the
+    // ambient (alpha, beta) window, but the *move* immediately
+    // preceding such a leaf is written through the ordinary, strict
+    // "iScore > iAlpha" move-acceptance path (the hot per-node move
+    // loop) -- a forced mate's concluding move very often only ties
+    // (rather than strictly beats) a PVS scout/null window built
+    // around an already-known expected score, so that move's own
+    // PV[] slot is frequently never written either, and the loop
+    // above exits here with nothing further recorded. Reconstruct the
+    // missing tail directly from the board instead of touching that
+    // hot path: ctx->sPosition at this point already reflects every
+    // move printed so far (each one was MakeMove'd above), so (a) if
+    // it's already a proven draw, or already checkmate/stalemate
+    // outright (e.g. the position right after the last printed move,
+    // covers a mate delivered by the final printed move itself, like
+    // root.c's single-legal-move case), the sentinel alone completes
+    // it; (b) otherwise, if a forced mate ends exactly one ply past
+    // what's recorded, by definition of mate distance shrinking by
+    // exactly one ply along the true line, some legal move right here
+    // must deliver it -- find and print that move too. Runs once per
+    // call (PrintPV, on an improved root score), not in the search
+    // tree, so the extra legal-move verification here is cheap
+    // relative to the search itself.
+    if (0 == mv.uMove)
+    {
+        if (TRUE == IsDraw(ctx))
+        {
+            strncat(buf, "<=>", ARRAY_LENGTH(buf) - strlen(buf) - 1);
+        }
+        else if (FALSE == _HasAnyLegalMove(ctx))
+        {
+            strncat(buf,
+                    (TRUE == InCheck(&ctx->sPosition, ctx->sPosition.uToMove)) ?
+                        "<#>" : "<=>",
+                    ARRAY_LENGTH(buf) - strlen(buf) - 1);
+        }
+        else
+        {
+            ULONG x;
+            FLAG fInCheckHere = InCheck(&ctx->sPosition, ctx->sPosition.uToMove);
+
+            GenerateMoves(ctx, NULLMOVE,
+                          (TRUE == fInCheckHere) ?
+                              GENERATE_ESCAPES : GENERATE_ALL_MOVES);
+            for (x = ctx->sMoveStack.uBegin[ctx->uPly];
+                 x < ctx->sMoveStack.uEnd[ctx->uPly];
+                 x++)
+            {
+                MOVE mvCandidate = ctx->sMoveStack.mvf[x].mv;
+
+                if (FALSE == MakeMove(ctx, mvCandidate))
+                {
+                    continue;
+                }
+
+                if (FALSE == _HasAnyLegalMove(ctx))
+                {
+                    FLAG fOpponentInCheck =
+                        InCheck(&ctx->sPosition, ctx->sPosition.uToMove);
+
+                    UnmakeMove(ctx, mvCandidate);
+                    strncat(buf, MoveToSan(mvCandidate, &ctx->sPosition),
+                            ARRAY_LENGTH(buf) - strlen(buf) - 1);
+                    strncat(buf, " ", ARRAY_LENGTH(buf) - strlen(buf) - 1);
+                    strncat(buf,
+                            (TRUE == fOpponentInCheck) ? "<#>" : "<=>",
+                            ARRAY_LENGTH(buf) - strlen(buf) - 1);
+                    break;
+                }
+                UnmakeMove(ctx, mvCandidate);
+            }
+        }
+    }
+
     //
     // Unmake the moves
     //