summaryrefslogtreecommitdiff
path: root/src/docs/TESTING_PLAN.md
diff options
context:
space:
mode:
authorScott Gasch <[email protected]>2026-10-15 13:11:22 -0700
committerScott Gasch <[email protected]>2026-10-15 13:11:22 -0700
commit993b6eeb2a9ac7b659c473b6883fa53af1ad3a0b (patch)
treec5388824dec15a7691ce9952a6eb4af66e837127 /src/docs/TESTING_PLAN.md
parent53b380c658f828c4c0e096be2763e6b012c23abd (diff)
Phase A NNUE: feature-indexing spec, annotation/training pipeline, dump nnuefeatures
Builds out the NNUE Phase A prototype per docs/NNUE.md: - src/nnuefeatures.c: standalone HalfKAv2_hm feature-index reference implementation (new `dump nnuefeatures` command), sourced from Stockfish's half_ka_v2_hm.{h,cpp} (sf_16 tag), including the 0x88->64 square conversion typhoon's own board representation needs. - annotate_pgns/: sharded TWIC/self-play annotation harness (annotate.py), position extraction, quiet-position filtering (flat and game-shaped, plus an upstream --prefilter-quiet option), train/holdout game-level stratified split, WDL-sigmoid K fitting, the Python HalfKAv2_hm reference encoder (nnue_feature_spec.py) and its bit-for-bit cross-checker against the C side (cross_check_features.py), and a first PyTorch training pipeline (nnue_model.py, nnue_train.py, build_training_tensors.py) -- a 256x2-32-32-1-ish net trained locally on CPU against ~1.85M quiet TWIC positions as a pipeline/framework validation pass. - Finish an in-progress InitMagic/InitLMRTable/InitEval -> InitializeBitboardMagic/InitializeLMRTable/InitializeEval rename: chess.h's prototypes hadn't been updated to match the already-renamed definitions/call sites, leaving 4 implicit-function-declaration warnings. - tests/arasan.epd: new curated EPD suite. - docs/: add the project's design docs (previously untracked). Co-Authored-By: Claude Sonnet 5 <[email protected]>
Diffstat (limited to 'src/docs/TESTING_PLAN.md')
-rw-r--r--src/docs/TESTING_PLAN.md298
1 files changed, 298 insertions, 0 deletions
diff --git a/src/docs/TESTING_PLAN.md b/src/docs/TESTING_PLAN.md
new file mode 100644
index 0000000..7d2bfc2
--- /dev/null
+++ b/src/docs/TESTING_PLAN.md
@@ -0,0 +1,298 @@
+# Testing plan: closing coverage gaps in test*.c
+
+Written 2026-09-16 after an audit of every `src/test*.c` file and how
+(or whether) each is wired into `main.c`'s self-test sequence. Goal
+per Scott: not more test code for its own sake -- find the
+years-old bugs that asserts have been missing. Priorities: search,
+eval, generate (not exclusively). Perf testing is also explicitly
+in scope.
+
+## Dead/disconnected infrastructure (fix first -- near-zero cost)
+
+Several tests already exist but don't run. Recovering this coverage is
+strictly cheaper than writing anything new, so do this before anything
+else below.
+
+1. **SEE has zero active correctness testing.** `testsee.c`'s own
+ header comment admits it: `DebugSEE`/`DebugSEERecursive` (a
+ brute-force make/unmake exchange-walk oracle meant to check `SEE()`
+ against) is wrapped in `#ifdef TEST_BROKEN` -- a define nothing
+ sets. `testsee.c`'s only *called* functions (`TestGetAttacks`,
+ `TestIsAttackedBB`) test attack generation, not SEE. Given SEE is
+ what `GetMoveSee`/extensions/QSearch pruning all trust (and is the
+ exact code reworked in commit c0ac3d7), this is the single
+ highest-value fix here. Get `TEST_BROKEN` -> live (`TEST`), fix
+ whatever bitrot surfaces, and confirm it's called from `main.c`.
+2. **`TestEvalWithKnownPositions`** (`testeval.c`, ~20 hand-curated
+ "side X should be better here" positions from chess literature) is
+ defined but **never called from anywhere** -- not `main.c`, not
+ `command.c`. Wire it into the self-test sequence.
+3. **`TestRecognExhaustiveKNKP`** (`testrecogn.c`, exhaustive KNKP
+ endgame-recognizer enumeration) is defined and prototyped in
+ `chess.h` but referenced only in a comment -- never invoked. Only
+ the smaller table-driven `TestRecogn` runs. Wire it in.
+4. **The hash table has no correctness self-test at all.**
+ `testhash.c` contains `SanityCheckHashEntry` (an entry-shape
+ validator) and `AnalyzeFullHashTable` -- but the latter is a
+ post-search *diagnostic dump* called from `root.c`, not a test, and
+ it's never invoked from the `TEST` self-test sequence. See "New
+ test: hash correctness" below for what should replace this gap, not
+ just a wiring fix.
+5. **`TestEvalWithSymmetry`** only runs a *single* random symmetric
+ position per invocation, and only when a human types `eval` at the
+ prompt in an `EVAL_DUMP` build (`command.c:2203`) -- not part of the
+ automated suite, never loops. See "New test: eval symmetry fuzz"
+ below.
+
+## New tests, by area
+
+### Search (thinnest area relative to its bug surface)
+
+- `TestSearch` (`testsearch.c`) runs 20 random positions at fixed
+ **depth 2** only, checking solely "returned move is legal" and
+ "didn't blow the time limit."
+ - **DONE -- hash-consistency test** (`TestSearchHashConsistency`,
+ testsearch.c): searches a position to depth D twice in a row with
+ a warm hash table (scores must match), and separately confirms a
+ depth-D2 search seeded by a shallower depth-D1 warm-up agrees with
+ a from-scratch depth-D2 search. Wired into the `TEST=1` self-test
+ sequence (main.c).
+ - **DONE (partially) -- QSearch sanity test**
+ (`TestQSearchSanity`, testsearch.c): checks search termination
+ (no `INVALID_SCORE`/garbage) and a whole-search node-count
+ ceiling. Caveat found and documented in the function's own header
+ comment: the self-test suite builds with plain `gmake TEST=1`,
+ which doesn't define `PERF_COUNTERS` -- every `sCounters` field
+ (including `u64QNodeCount`, the qsearch-only counter this was
+ originally meant to check) is wrapped in `chess.h`'s `INC()` macro,
+ a no-op without that define. Confirmed live (`sd 3; go` on a
+ `TEST`-only binary reports "0 qnodes" regardless of real qsearch
+ activity). The test currently checks
+ `ctx->sCounters.tree.u64TotalNodeCount` instead (root.c's
+ unconditional per-node `++`, always live) -- a real but strictly
+ weaker signal, since it can't distinguish "qsearch exploded" from
+ "full-width search exploded." Building a `PERF_COUNTERS`-enabled
+ `TEST` profile (or adding an always-live qsearch-only counter) is
+ the natural follow-up to close this for real. The hand-auditable-
+ tree idea (`Trace()` every qsearch node on a small position) is
+ still worth doing but wasn't implemented this session.
+ - **NOT DONE -- determinism test** (same position/depth searched
+ twice, PV must match): explicitly descoped -- Scott's call is that
+ a stable-PV requirement makes the test break every time move
+ ordering changes for unrelated reasons, which is just noise, not a
+ real regression signal. A same-*score* (not same-PV) check is
+ already covered by the hash-consistency test above.
+ - **NOT DONE -- MP-vs-single-thread agreement test**: still open.
+ `split.c` has no dedicated correctness test.
+ - **DONE (as an EPD suite, not a C test) -- null-move/zugzwang
+ soundness**: `tests/zugzwang.epd` (5 positions, run via the new
+ `epd` command -- see below) checks actual best-move outcome
+ instead of an internal counter. An earlier attempt asserted
+ against `ctx->sCounters.extension.uZugzwang` (the verification-
+ search hit counter) using the 3 classic literature zugzwang
+ positions, but none of them structurally reach that code path on
+ this build: 2 have too little material for
+ `WeShouldTryNullmovePruning`'s own `> 2` non-pawn-count gate to
+ attempt null-move pruning at all, and the 3rd (despite landing in
+ the eligible window) gets resolved directly off this box's local
+ Syzygy tablebase before search ever touches null-move logic --
+ confirmed live via the `TABLEBASE MOVE` trace line. Removed in
+ favor of the EPD suite, which doesn't care about internal
+ mechanism, only the final answer. See `testsearch.c`'s comment
+ where `TestNullMoveZugzwang` used to be for the full writeup.
+
+**New infrastructure that came out of chasing the above (search.c
+touches none of this, but it's now how any future EPD-based search
+test should be added):**
+
+- **`epd <file>` engine command** (`EpdCommand`, script.c) parses and
+ runs a raw EPD suite file directly -- no perl preprocessing step
+ first. Backed by `ParseEpdRecord` (fen.c), real per-record EPD
+ parsing (4 position fields, optional FEN-style halfmove/fullmove
+ counters, then `;`-separated `bm`/`am`/`id` opcodes).
+- **Found and fixed while building this**: `tests/convert_epd.pl` (the
+ old perl-based EPD-to-script converter) had a real, silent-
+ corruption bug -- its `bm`/`am` regex used `[^b]+` as the filler
+ between the board and the opcode, which breaks on *every*
+ Black-to-move record (the side-to-move field is literally the
+ character `b`), and since `$fen`/`$id`/`$solution` were unscoped
+ globals rather than per-line locals, a failed match silently carried
+ the *previous* record's values forward instead of erroring.
+ Confirmed live: feeding it a 5-position batch with one Black-to-move
+ entry produced a byte-for-byte duplicate of the previous record for
+ that one line, no diagnostic at all. The script is left in place
+ (marked superseded in its own header) in case something still shells
+ out to it, but new suites should use `epd` instead.
+ Cross-checked the new parser against the old pipeline on
+ `ecm_ringers`: `script` (old `.ep_` format) and `epd` (new
+ `tests/ecm_ringers.epd`, produced by `ep_to_epd.pl`, see next item)
+ produced byte-identical node counts and solve stats.
+- **`tests/ep_to_epd.pl`**: one-off reverse converter, turns the
+ existing `setboard`/`solution`/`avoid`/`id`/`go` script-format
+ suites back into real EPD now that `epd` can read it directly. Run
+ against every genuine tactical suite in `tests/` (not `bench.ep_` or
+ `trial.ep_`, which use `sd`/`st`/`easy`/`quit` and aren't EPD
+ underneath) -- produced `.epd` siblings for `arasan`,
+ `ecm_confident_quick`, `ecm_confident_sample2`, `ecm_confident`,
+ `ecm_hard_quick`, `ecm_hard`, `ecm_late`, `ecm_quick`, `ecm_ringers`,
+ `ecm-gcp`, `ecm-hard`, `ecm`, `ecm016_only`, `LCT2`, `mes400`,
+ `newiq`, `quick`, `swiss`, `wac`. `ecm.epd` came out to 879 records,
+ matching the `606/879`/`622/879` solve-count baselines already
+ quoted elsewhere in CLAUDE.md.
+
+### Eval
+
+- Automate `TestEvalWithSymmetry` into a real fuzz loop (N random
+ symmetric positions, assert `Eval() == 0` for all, as part of the
+ standard self-test run) instead of the current single-shot
+ manual-only invocation.
+- **Adversarial/degenerate-position term-bounds test**: 0-pawn
+ positions, 9-queen positions, fully-open-board positions -- extend
+ `GenerateRandomLegalPosition` or hand-build cases, assert no eval
+ term produces an out-of-range score (reuse the existing
+ `EvalTrace`/`EVAL_DUMP` term-tracing machinery for this).
+- Audit for any **incrementally-maintained eval state** (pawn hash,
+ king-safety cache) and add a from-scratch-recompute-vs-incremental
+ consistency check if any exists -- classic "works until the hash
+ collides" bug class.
+- **Eval speed regression test**: no benchmark currently exists for
+ eval itself despite it being the named NPS bottleneck vs. Crafty
+ (see CLAUDE.md). Same interleaved-comparison shape as the existing
+ `TestGenerate*Speed` benchmarks, broken down by game phase.
+
+### Generate (best-covered area, but gaps remain)
+
+- Perft (`TestMoveGenerator`) only checks to **ply 4** even though
+ `uLeaves[]` already has reference values to ply 6/7 sitting unused --
+ extend to a `--perft-deep` slow path, and add perft-**divide**
+ (per-root-move leaf breakdown) to localize failures to a specific
+ move instead of just tripping a panic.
+- Add perft positions specifically chosen to stress **en passant,
+ underpromotion, and castling-through-check** individually (the
+ current 5 FENs give general coverage but weren't chosen for this).
+- `TestGenerateAllMovesSpeed` and the per-piece-type speed tests are
+ **pure microbenchmarks with no correctness assertion** -- they print
+ cycles and never diff the move *sets* produced by the mailbox vs.
+ bitboard generators being compared. Add a sorted-move-list-equality
+ assertion alongside each (both generators already run back-to-back
+ in the same loop, so this is nearly free) -- otherwise a bitboard
+ generator silently missing a move class just prints a suspiciously
+ fast number and passes.
+- `GetMoveSee`'s new non-pawn-capture SEE path (c0ac3d7) has no
+ dedicated test yet -- covered by the SEE oracle fix above once live.
+
+### Move/Position plumbing
+
+- Reasonably solid already (`TestMakeUnmakeMove`, `TestExposesCheck`,
+ `TestIsAttacked`). Add a **long-game fuzz test**: play N random games
+ of ~200 plies each (not just 1-ply-at-a-time exercises), asserting
+ `VerifyPositionConsistency` and `ComputeSig`-from-scratch agreement
+ with the incrementally-maintained signature at every ply. Confirm
+ `VerifyPositionConsistency` is actually compiled live in the
+ `TEST`/`DEBUG` build, not just present in source.
+
+### Hash (currently zero correctness coverage)
+
+New test needed, not just a wiring fix:
+- Store/probe roundtrip correctness.
+- Replacement-scheme behavior under forced collisions (fill table past
+ capacity with distinct signatures, confirm replacement policy
+ behaves as designed, not just "doesn't crash").
+- `ADJUST_MATE_IN_N` ply-distance adjustment on `HASH_FLAG_EXACT`
+ entries specifically -- the exact mechanism CLAUDE.md flags as
+ audited-by-hand but not test-covered after this session's `ASSERT`
+ removals in `QSearch()`.
+
+### Recognizer
+
+- Wire in `TestRecognExhaustiveKNKP` (see dead-infra list above).
+ Consider the same exhaustive-enumeration treatment for any other
+ small recognized endgame class in `recogn.c` (KPK, KRK) if one
+ exists without a test.
+
+## Perf testing
+
+Current coverage is narrow: five isolated per-piece-type
+mailbox-vs-bitboard microbenchmarks (`TestGenerate*Speed`), nothing for
+search, eval, or hash.
+
+- **Eval NPS/cycles-per-call benchmark**, same interleaved-comparison
+ shape as the existing generate benchmarks, by game phase -- eval is
+ the named bottleneck (CLAUDE.md's 1.5M vs 7.5M nps gap vs. Crafty)
+ and has no dedicated speed test today.
+- **Whole-search NPS benchmark**: `bench.c` already exists for this --
+ confirm whether it's part of the automated `TEST` self-test pass or
+ purely a manual `bench` command; if manual-only, wire a
+ fixed-node-budget version into the automated sequence so a
+ regression surfaces without someone remembering to run it by hand.
+- **Hash-probe-under-load benchmark**: cycles/probe at low occupancy
+ vs. near-full table -- replacement-scheme cost can regress silently
+ today with nothing to measure it.
+- **Stored baselines with pass/fail thresholds**, not print-only
+ output: today every perf test prints cycles/call and leaves
+ interpretation to a human reading the log. Store a last-known-good
+ baseline (same idea as `head_reference/` for solve-counts) and fail
+ loud if a call site regresses past some tolerance.
+
+## Suggested priority order
+
+1. Un-bitrot the three dead tests (SEE oracle,
+ `TestEvalWithKnownPositions`, `TestRecognExhaustiveKNKP`) --
+ near-zero cost, immediate coverage recovery.
+2. Hash-table correctness test (store/probe/replace/mate-adjust) --
+ currently the single biggest correctness blind spot.
+3. Search determinism + MP-vs-single-thread agreement test -- targets
+ the exact class of bug most likely to hide in threading/hash/
+ extension interactions that per-function unit tests can't see.
+4. Extend perft to deeper plies + divide output; add move-set-equality
+ assertions to the existing generate speed benchmarks (turns
+ dead-weight microbenchmarks into correctness tests for free).
+5. Eval: automate the symmetry fuzz test (loop N, not 1) and add
+ adversarial/degenerate-position term-bounds checks.
+6. Round out perf testing per the list above, with stored baselines
+ and pass/fail thresholds instead of print-only output.
+
+## "Fuzz" testing
+
+**Implemented: `fuzz_test.sh [binary] [depth] [batch_size] [cpus]`**
+(defaults: `./_typhoon`, `sd 6`, 200 positions/batch, `--cpus 2`). Loops
+forever: each iteration draws a fresh random batch of `setboard`/`go`
+records from the full 400,000-line / 200k-position
+`tests/cross_sectional_200k.ep_` pool, runs them through a `DEBUG=1`
+build under gdb, and scans the gdb output for a crash/assert signature
+before starting the next batch. Stops (exit 1) and prints the offending
+batch + engine log paths as soon as one is found; otherwise runs
+indefinitely (`^C` to stop). Logs land under `/usr/local/tmp/typhoon/fuzz/`
+(one sampled `.ep_` batch + one `.gdb.txt` + one engine `--logfile` per
+iteration).
+
+**Two build/invocation bugs found and fixed getting this working, neither
+a sandbox limitation despite first appearances:**
+
+1. **The DEBUG binary must be built with `MP=1` as well as `DEBUG=1`**
+ (`gmake clean; gmake DEBUG=1 MP=1`) -- `--cpus` is compiled in only
+ under `#ifdef MP` (`main.c:311`). A `DEBUG=1`-only build silently
+ treats `--cpus` and its numeric argument as two separate unrecognized
+ arguments (`main.c:400`'s "Error (unknown argument)" path) and falls
+ back to the 1-cpu default with no hard failure -- easy to miss, and it
+ also means `split.c`'s parallel-search code never actually links in,
+ defeating the point of passing `--cpus 2` at all.
+2. **`gdb -batch -ex "handle SIGTRAP nostop noprint pass" -ex run ...`
+ breaks thread control against this MP/pthread build specifically**,
+ producing `ptrace (PT_SUSPEND): Operation not permitted` followed by
+ `Child process unexpectedly missing: No child processes` -- looked
+ exactly like a sandboxed environment blocking ptrace outright (and was
+ initially misdiagnosed as that), but reproduced identically outside
+ any harness/sandbox, and disappeared entirely once that one `-ex` line
+ was dropped. `fuzz_test.sh` omits it; a plain `run`/`bt full`/
+ `info registers` gdb batch invocation debugs the MP binary fine,
+ including catching a genuine `int3` breakpoint from a failed `ASSERT`.
+
+Not yet done: no crash has been found by a run so far, so the
+crash-detection path (the `grep -qE` in `fuzz_test.sh`) is exercised by
+its own logic reading correctly, not yet by a real hit. Worth periodically
+running this for stretches (it's unbounded/`^C`-driven, unlike the other
+gates) rather than treating it as a one-time pass/fail check like
+`debug_smoke_test.sh`. See also: `./debug_smoke_test.sh` (fast, curated
+sample, single pass/fail) for the pre-commit-sized version of the same
+idea.