NNUE accumulator struct scaffolding; fix 3 bugs found by DEBUG+TEST; add that gate

Partial checkpoint on the nnue-accumulator-plumbing branch, before the
MakeMove/UnmakeMove hookup itself:

- chess.h: NNUE_ACCUMULATOR struct (256 floats, pre-activation, float32
  for now -- quantization to int16 deliberately deferred until a real
  quantization-aware export pass exists, per docs/NNUE.md). Added the
  live, always-current SEARCHER_THREAD_CONTEXT.sNnueAccum[2] (one per
  color) and PLY_INFO.sNnueAccumBackup (the king-move full-recompute
  backup slot, DEBUG-only color tag declared for a later ASSERT).
  LIGHTWEIGHT_SEARCHER_CONTEXT also mirrors sNnueAccum[2] at the same
  relative position -- omitting that broke its byte-compatible-prefix
  contract with the full context the moment MakeMove/UnmakeMove will
  start touching it unconditionally (caught by this session's own
  TestSan self-test failure).

- Combining DEBUG=1 and TEST=1 for the first time (apparently ever)
  while chasing that failure immediately turned up three more real,
  pre-existing bugs, all in self-test-only code whose assert-guarded
  invariants had never been checked with asserts actually compiled in:
    - board.c: IsBoardLegal now rejects an en passant square whose rank
      doesn't match the side to move (and verifies the double-moved
      pawn/empty origin square), closing a gap VALID_EP_SQUARE alone
      can't catch.
    - testgenerate.c: TestCapturesAndPromotionsGenerator's fixture table
      had a self-contradictory FEN this new IsBoardLegal check would
      have caught; replaced with a genuine Black-to-move en passant
      position.
    - testgenerate.c: TestFindUnblockedSquaresSpeed called
      _GenerateAllMoves directly instead of the GenerateMoves wrapper,
      skipping the DEBUG-only pStack->board[uPly] snapshot
      WouldGiveCheck's own ASSERT checks against.

- precommit_check.sh / CLAUDE.md: added the DEBUG=1 TEST=1 combination
  as a permanent third pre-commit gate, given what its first-ever run
  turned up. Real cost (~235s of ~266s total, unoptimized build
  dominates), judged worth it.

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

5 files changed, 228 insertions, 12 deletions

diff --git a/src/CLAUDE.md b/src/CLAUDE.md
index f2bbd37..c8e9c08 100644
--- a/src/CLAUDE.md
+++ b/src/CLAUDE.md
@@ -42,9 +42,9 @@ object mismatches at link time.
 
 ## Pre-commit testing
 
-Two cheap, complementary gates exist and should both be run before
+Three cheap, complementary gates exist and should all be run before
 committing a search/eval/move-generation change (not needed for pure
-doc/comment-only changes). Neither replaces the curated-suite/`sd`-`sn`
+doc/comment-only changes). None of these replace the curated-suite/`sd`-`sn`
 comparison workflow below -- these catch outright bugs (crashes, broken
 invariants) fast; the suite workflow measures whether a change is actually
 *better*, which is a slower, separate question.
@@ -61,7 +61,28 @@ invariants) fast; the suite workflow measures whether a change is actually
   original-2004-era and untouched since, but `testeval.c`/`testsearch.c`/
   `testsee.c` have been actively edited by recent sessions -- this is a
   live, working harness, not dead code, despite living outside the normal
-  build profile.
+  build profile. **Note this profile has no `DEBUG`, so every `ASSERT` in
+  these test files is a silent no-op** -- see the next gate.
+- **`gmake DEBUG=1 TEST=1` combined self-test.** Same self-test suite as
+  above, but with asserts actually compiled in. Added 2026-10-15 after
+  running this exact combination for the first time (ever, as far as
+  anyone could tell) immediately turned up three real, previously-
+  undiscovered bugs in one run -- none in production code, all in
+  self-test-harness code whose assert-guarded invariants had apparently
+  never been checked with asserts live: (1) `testgenerate.c`'s
+  `TestCapturesAndPromotionsGenerator` fixture table had a
+  self-contradictory hand-built FEN (en passant square on rank 6 paired
+  with Black to move -- only reachable with White to move; see `board.c`'s
+  `IsBoardLegal`, which now rejects this same mistake if it ever arrives
+  via `setboard`); (2) same file's `TestFindUnblockedSquaresSpeed` called
+  `_GenerateAllMoves` directly instead of the `GenerateMoves` wrapper,
+  skipping the DEBUG-only `pStack->board[uPly]` snapshot
+  `WouldGiveCheck`'s own `ASSERT(PositionsAreEquivalent(...))` checks
+  against. Budget real time for this step -- it's unoptimized (`DEBUG`
+  implies no `-O3`), so it dominates `precommit_check.sh`'s total runtime
+  (measured ~235s of ~266s total on this box, vs. ~16s for the plain
+  `TEST=1` gate) -- but kept anyway: the bug-catching rate on its first
+  ever run made the cost clearly worth it.
 - **`debug_smoke_test.sh [binary] [depth] [count]`** (defaults: `./_typhoon`,
   `sd 4`, 10 positions). Builds/runs a `DEBUG=1` binary (asserts compiled
   in) against a fresh random sample of positions drawn from the full
@@ -79,11 +100,13 @@ invariants) fast; the suite workflow measures whether a change is actually
   flushes the logfile every call) before triggering the `int3` breakpoint,
   so that text survives in the logfile even if stdout capture is lost to
   an abrupt `SIGTRAP` death.
-- **`precommit_check.sh`** runs both gates back-to-back and restores a
+- **`precommit_check.sh`** runs all three gates back-to-back and restores a
   normal release build afterward (`gmake clean` + the standard
   `GENETIC=1 PERF_COUNTERS=1 MP=1 SIXTYFOUR=1` profile), so the working
   directory isn't left with a `TEST`/`DEBUG`-flavored binary as a side
-  effect of running it. ~20s end to end on this box.
+  effect of running it. **~266s (~4.5min) end to end on this box** as of
+  adding the combined DEBUG+TEST gate (was ~20s with just the two original
+  gates) -- almost all of the increase is that one unoptimized step.
 
 **On not making this an automatic git pre-commit hook**: deliberately not
 wired up that way. Two concrns pushed toward manual-by-default: (1) each
diff --git a/src/board.c b/src/board.c
index 0f3db09..80b5acb 100644
--- a/src/board.c
+++ b/src/board.c
@@ -191,6 +191,52 @@ Return value:
     {
         return(FALSE);
     }
+
+    // En passant square must be consistent with whose move it is, not
+    // just "a plausible ep square in isolation" (chess.h's
+    // VALID_EP_SQUARE already checks that -- rank 3 or 6 -- but not
+    // against uToMove). An ep square records the square a just-moved
+    // pawn passed over: if it's White to move, BLACK must have just
+    // double-pushed (passing over rank 6); if Black to move, WHITE
+    // must have (passing over rank 3). Also verify the double-moved
+    // pawn is actually there and its origin square is now empty --
+    // not reachable through real play otherwise. 2026-10-15: a
+    // malformed hand-built FEN (here, testgenerate.c's
+    // TestCapturesAndPromotionsGenerator fixture table) fell through
+    // this exact gap -- ep square on rank 6 with BLACK to move,
+    // self-contradictory, but VALID_EP_SQUARE alone can't catch it.
+    if (IS_ON_BOARD(pos->cEpSquare))
+    {
+        COOR cEp = pos->cEpSquare;
+        ULONG uJustMoved = FLIP(pos->uToMove);
+        COOR cDoubleMovedPawn, cOriginSquare;
+
+        if (WHITE == pos->uToMove)
+        {
+            if (!RANK6(cEp))
+            {
+                return(FALSE);
+            }
+            cDoubleMovedPawn = FILE_RANK_TO_COOR(FILE(cEp), 5);
+            cOriginSquare    = FILE_RANK_TO_COOR(FILE(cEp), 7);
+        }
+        else
+        {
+            if (!RANK3(cEp))
+            {
+                return(FALSE);
+            }
+            cDoubleMovedPawn = FILE_RANK_TO_COOR(FILE(cEp), 4);
+            cOriginSquare    = FILE_RANK_TO_COOR(FILE(cEp), 2);
+        }
+
+        if (!IS_PAWN(pos->rgSquare[cDoubleMovedPawn].pPiece) ||
+            (GET_COLOR(pos->rgSquare[cDoubleMovedPawn].pPiece) != uJustMoved) ||
+            !IS_EMPTY(pos->rgSquare[cOriginSquare].pPiece))
+        {
+            return(FALSE);
+        }
+    }
     return(TRUE);
 }
 
diff --git a/src/chess.h b/src/chess.h
index 05928a3..29833fb 100755
--- a/src/chess.h
+++ b/src/chess.h
@@ -1620,6 +1620,34 @@ typedef enum
 }
 MOVE_SCORING_SCHEME;
 
+// NNUE accumulator -- the feature-transformer's output for ONE color's
+// perspective (own-king-relative HalfKAv2_hm features summed through
+// the single shared [22528, 256] embedding table -- see
+// docs/NNUE.md's "Canonical HalfKAv2_hm feature-indexing spec" and
+// annotate_pgns/nnue_model.py's L1). Stored PRE-activation (matches
+// Stockfish convention and nnue_model.py's own torch.relu() applied
+// AFTER this sum, not baked into it) so incremental MakeMove/
+// UnmakeMove updates stay purely additive -- the activation is only
+// ever applied where the accumulator is actually read (the FC head),
+// never where it's maintained.
+//
+// Deliberately float, not yet the int16 the final architecture
+// decision calls for (NNUE.md's "Quantization" section) -- that needs
+// a calibrated per-layer scale from the quantization-aware export
+// pass, which doesn't exist yet. This branch's job is proving the
+// incremental-maintenance plumbing (king-move backup/restore nesting,
+// capture/promotion edge cases) is correct; swapping the element type
+// is a later, decoupled, mechanical follow-up once real quantized
+// weights exist -- not something to conflate with debugging the
+// harder plumbing now.
+#define NNUE_ACCUMULATOR_SIZE (256)  // L1, nnue_model.py
+
+typedef struct _NNUE_ACCUMULATOR
+{
+    float rgfValue[NNUE_ACCUMULATOR_SIZE];
+}
+NNUE_ACCUMULATOR;
+
 typedef struct _PLY_INFO
 {
 #ifdef DEBUG
@@ -1703,6 +1731,37 @@ typedef struct _PLY_INFO
     ULONG uTotalNonPawns;
     BITV bvCastleInfo;
     COOR cEpSquare;
+
+    // King-move backup for the NNUE accumulator (see NNUE_ACCUMULATOR
+    // above and SEARCHER_THREAD_CONTEXT's sNnueAccum[2] below): every
+    // feature in HalfKAv2_hm is own-king-relative, so when a color's
+    // OWN king moves, that color's ENTIRE accumulator needs a full
+    // recompute, not an incremental delta -- MakeMove backs up the
+    // pre-move accumulator here before recomputing; the MATCHING
+    // UnmakeMove restores it from here. Indexed by ply (this struct is
+    // already sPlyInfo[uPly]), not a single shared slot, so nested own-
+    // king moves at different plies never collide -- same reasoning as
+    // cTrapped/cEnprise's existing per-ply indexing.
+    //
+    // No "is this valid" marker needed in a release build: Make/
+    // UnmakeMove already decide whether to touch this purely from
+    // whether the move being made/unmade is IS_KING(mv.pMoved) for
+    // this ply's mover -- exactly the same information Make/UnmakeMove
+    // already use to dispatch every other move-type-specific update,
+    // not something inferred from this struct's own contents. A
+    // correctly nested Make/Unmake pair guarantees this is always
+    // freshly written by MakeMove before the matching UnmakeMove ever
+    // reads it.
+    NNUE_ACCUMULATOR sNnueAccumBackup;
+#ifdef DEBUG
+    // Which color's accumulator sNnueAccumBackup above holds, checked
+    // by UnmakeMove (ASSERT only -- never a real branch condition, see
+    // above) purely to catch a hypothetical Make/Unmake mismatch, and
+    // then the backup is memset(0)'d (poison, not a real "clear" any
+    // release-build logic depends on) so an accidental future stale
+    // read in some other DEBUG-only path doesn't look like valid data.
+    ULONG uNnueAccumBackupColor;
+#endif
 }
 PLY_INFO;
 
@@ -1859,6 +1918,20 @@ typedef struct _SEARCHER_THREAD_CONTEXT
 {
     ULONG uPly;                               // its distance from root
     POSITION sPosition;                       // the board
+
+    // The live, always-current NNUE accumulator, one per color --
+    // analogous to sPosition just above (always reflects the CURRENT
+    // position, maintained incrementally by MakeMove/UnmakeMove, not
+    // recomputed per call). See NNUE_ACCUMULATOR's own comment and
+    // PLY_INFO's sNnueAccumBackup (this struct's sPlyInfo[uPly] member
+    // below) for the king-move full-recompute case. Indexed by color
+    // (WHITE/BLACK, chess.h), not by perspective-as-concept, since
+    // each color's own accumulator is independently maintained and
+    // independently invalidated (only THAT color's own king moving
+    // forces its recompute -- see Stockfish's requires_refresh(), same
+    // semantics here).
+    NNUE_ACCUMULATOR sNnueAccum[2];
+
     MOVE_STACK sMoveStack;                    // the move stack
     CUMULATIVE_SEARCH_FLAGS sSearchFlags;
     PLY_INFO sPlyInfo[MAX_PLY_PER_SEARCH+1];
@@ -1944,13 +2017,27 @@ SEARCHER_THREAD_CONTEXT;
 //
 //     1. uPly
 //     2. sPosition
-//     3. sPlyInfo
-//     4. sMoveStack
+//     3. sNnueAccum (NNUE_ACCUMULATOR -- MakeMove/UnmakeMove touch
+//        this unconditionally now, not just on a king move, so it
+//        has to physically exist here too, at the same relative
+//        position as in SEARCHER_THREAD_CONTEXT, or a lightweight-
+//        cast caller's write becomes an out-of-bounds overflow past
+//        this smaller struct's real allocation. 2026-10-15: this bit
+//        a real bug the first time sNnueAccum was added to
+//        SEARCHER_THREAD_CONTEXT without a matching mirror here --
+//        every field after it silently shifted out of alignment for
+//        any lightweight-cast caller (ParseMoveSan's self-test caught
+//        it). Any future SEARCHER_THREAD_CONTEXT field MakeMove/
+//        UnmakeMove/Generate touch unconditionally needs the same
+//        treatment -- check this list before adding one.)
+//     4. sPlyInfo
+//     5. sMoveStack
 //
 typedef struct _LIGHTWEIGHT_SEARCHER_CONTEXT
 {
     ULONG uPly;
     POSITION sPosition;
+    NNUE_ACCUMULATOR sNnueAccum[2];
     MOVE_STACK sMoveStack;
     CUMULATIVE_SEARCH_FLAGS sSearchFlags;
     PLY_INFO sPlyInfo[MAX_PLY_PER_SEARCH+1];
diff --git a/src/precommit_check.sh b/src/precommit_check.sh
index c2d7d41..0525765 100755
--- a/src/precommit_check.sh
+++ b/src/precommit_check.sh
@@ -1,5 +1,6 @@
 #!/bin/sh
-# Pre-commit build/test gate: TEST=1 self-test suite + DEBUG=1 random-sample
+# Pre-commit build/test gate: TEST=1 self-test suite + a combined
+# DEBUG=1 TEST=1 self-test (asserts live) + DEBUG=1 random-sample
 # smoke test (debug_smoke_test.sh). Restores a normal release build
 # afterward so the working directory isn't left with a TEST/DEBUG-flavored
 # binary as a side effect of running this.
@@ -16,7 +17,7 @@ mkdir -p "$LOGDIR"
 FAIL=0
 TOTAL_START=$(date +%s)
 
-echo "=== [1/3] Self-test build (gmake TEST=1) ==="
+echo "=== [1/4] Self-test build (gmake TEST=1) ==="
 STEP_START=$(date +%s)
 gmake clean > "$LOGDIR/clean_test.log" 2>&1
 gmake -j5 TEST=1 GENETIC=1 SIXTYFOUR=1 > "$LOGDIR/build_test.log" 2>&1
@@ -36,7 +37,35 @@ fi
 STEP_END=$(date +%s)
 echo "    [$((STEP_END - STEP_START))s]"
 
-echo "=== [2/3] DEBUG smoke test (10 random ECM positions, sd 4) ==="
+# 2026-10-15: added after combining DEBUG=1 and TEST=1 for the first
+# time (ever, as far as anyone could tell) turned up three real,
+# previously-undiscovered bugs in a single run -- none in production
+# code, all in self-test-only harnesses whose assert-guarded invariants
+# had apparently never been checked with asserts actually compiled in
+# (plain TEST=1 has no DEBUG, so ASSERT is a no-op; DEBUG=1 alone never
+# links the self-test .o files at all). Unoptimized, so this step is
+# the slow one -- budget a couple of minutes, not seconds.
+echo "=== [2/4] Combined self-test build (gmake DEBUG=1 TEST=1) ==="
+STEP_START=$(date +%s)
+gmake clean > "$LOGDIR/clean_debugtest.log" 2>&1
+gmake -j5 DEBUG=1 TEST=1 GENETIC=1 > "$LOGDIR/build_debugtest.log" 2>&1
+if [ $? -ne 0 ]; then
+    echo "    BUILD FAILED -- see $LOGDIR/build_debugtest.log"
+    FAIL=1
+else
+    ./_typhoon --batch --command "" > "$LOGDIR/test_run_debugtest.log" 2>&1
+    STATUS=$?
+    if [ "$STATUS" -ne 0 ] || grep -qi "assertion failure\|PANIC" "$LOGDIR/test_run_debugtest.log"; then
+        echo "    SELF-TEST FAILED (exit=$STATUS) -- see $LOGDIR/test_run_debugtest.log"
+        FAIL=1
+    else
+        echo "    self-test passed"
+    fi
+fi
+STEP_END=$(date +%s)
+echo "    [$((STEP_END - STEP_START))s]"
+
+echo "=== [3/4] DEBUG smoke test (10 random ECM positions, sd 4) ==="
 STEP_START=$(date +%s)
 gmake clean > "$LOGDIR/clean_debug.log" 2>&1
 gmake -j5 GENETIC=1 DEBUG=1 MP=1 SIXTYFOUR=1 > "$LOGDIR/build_debug.log" 2>&1
@@ -50,7 +79,7 @@ fi
 STEP_END=$(date +%s)
 echo "    [$((STEP_END - STEP_START))s]"
 
-echo "=== [3/3] Restoring normal release build (GENETIC=1 PERF_COUNTERS=1 MP=1 SIXTYFOUR=1) ==="
+echo "=== [4/4] Restoring normal release build (GENETIC=1 PERF_COUNTERS=1 MP=1 SIXTYFOUR=1) ==="
 STEP_START=$(date +%s)
 gmake clean > "$LOGDIR/clean_release.log" 2>&1
 gmake -j5 GENETIC=1 PERF_COUNTERS=1 MP=1 SIXTYFOUR=1 > "$LOGDIR/build_release.log" 2>&1
diff --git a/src/testgenerate.c b/src/testgenerate.c
index 95c5000..83c187d 100644
--- a/src/testgenerate.c
+++ b/src/testgenerate.c
@@ -772,6 +772,24 @@ TestFindUnblockedSquaresSpeed(void)
         uNumMoves = MOVE_COUNT(ctx, uPly);
         ASSERT(uNumMoves > 0);
 
+#ifdef DEBUG
+        // Calling _GenerateAllMoves directly (above), not the
+        // GenerateMoves wrapper, skips the DEBUG-only
+        // pStack->board[uPly] snapshot GenerateMoves normally takes
+        // (generate.c, right before it dispatches to a generator) --
+        // WouldGiveCheck's own ASSERT(PositionsAreEquivalent(...))
+        // checks the move it's given against exactly that snapshot,
+        // so without this, WouldGiveCheck below trips on whatever
+        // stale snapshot happened to be sitting in pStack->board[uPly]
+        // from some earlier, unrelated call. posBench never changes
+        // for the rest of this position's timed loop (no MakeMove
+        // happens here), so one copy here is sufficient, not one per
+        // iteration. 2026-10-15: found via the first DEBUG+TEST
+        // combined build this self-test suite had ever been run
+        // under.
+        memcpy(&(pStack->board[uPly]), &posBench, sizeof(POSITION));
+#endif
+
         u64Total = 0;
         for (uIter = 0; uIter < uCallsPerPosition; uIter++)
         {
@@ -838,7 +856,20 @@ TestCapturesAndPromotionsGenerator(void)
         "3Q4/1Q4Q1/4Q3/2Q4R/Q4Q2/3Q4/1Q4Rp/1K1BBNNk w - - 0 1",
         "rnbqkbnr/ppp1p1pp/8/3pPp2/8/8/PPPP1PPP/RNBQKBNR w KQkq f6 0 3",
         "rnbqkbnr/pp1p1ppp/8/2pPp3/8/8/PPP1PPPP/RNBQKBNR w KQkq c6 0 3",
-        "rnbqkbnr/pp1ppppp/8/2pP4/8/8/PPP1PPPP/RNBQKBNR b KQkq c6 0 2",
+        // 2026-10-15: this used to read "...b KQkq c6 0 2" -- an ep
+        // square on rank 6 is only reachable with WHITE to move (it
+        // records a BLACK pawn's just-played double push), so pairing
+        // it with "b" (Black to move) was self-contradictory and
+        // unreachable through real play. FenToPosition never rejected
+        // it (it isn't routed through SetRootPosition's IsBoardLegal
+        // gate), so the move generator ran en passant generation
+        // against a position whose ep square didn't correspond to
+        // the side to move at all -- see board.c's IsBoardLegal for
+        // the check that would now catch this same mistake if it
+        // arrived via `setboard`. Replaced with a genuine Black-to-
+        // move en passant position (White just played c2-c4, passing
+        // over c3; Black's pawn already on d4 can capture there).
+        "rnbqkbnr/ppp1pppp/8/8/2Pp4/8/PP1PPPPP/RNBQKBNR b KQkq c3 0 3",
         "8/1P6/8/8/8/8/7p/1k4K1 w - - 0 1",
         "8/1P6/8/8/8/8/7p/1k4K1 b - - 0 1",
         "8/2p5/3p4/KP5r/1R3p1k/8/4P1P1/8 w - - 0 1",