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]>

1 files changed, 89 insertions, 2 deletions

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];