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

1 files changed, 406 insertions, 0 deletions

diff --git a/src/docs/UNIFY_SEARCH.md b/src/docs/UNIFY_SEARCH.md
new file mode 100644
index 0000000..f38970b
--- /dev/null
+++ b/src/docs/UNIFY_SEARCH.md
@@ -0,0 +1,406 @@
+# Unifying the per-move search kernel in `search.c` and `split.c`
+
+Status: **plan only, not started.** Do as the only change in a commit;
+test with `precommit_check.sh` plus the `head_reference` node-count gate
+below at each step. Line numbers below are current as of this revision
+(EFP's explicit per-move pruning block has been removed from `Search()`;
+only stale names remain — see "EFP is gone" below).
+
+## Motivation
+
+`Search()`'s main move loop (`search.c:909..1780`) and `HelpSearch()`
+(`split.c:1058..1283`) independently implement what is supposed to be the
+same per-move logic: make the move, compute its extension, decide whether
+to prune or reduce it, search it with a PVS window, re-search if a reduced
+move fails high, unmake it. They have drifted apart because every
+improvement to this logic has been written once, in `search.c`, and not
+reliably ported to `split.c`. Concretely, today:
+
+- `Search()` has LMP (late move pruning, via `ShouldLatePrune`/the joint
+  FH% table). `HelpSearch()` does not.
+- `Search()` gates reduction/pruning eligibility through the single
+  `CanWeReduceOrPruneMove()` (searchsup.c:783), which the routine's own
+  comment documents as 9 base checks plus 2 move-identity exemptions
+  (killer-adjacency, countermove match). `HelpSearch()` (split.c:1187-1193)
+  has a hand-rolled 7-condition gate that is missing: the SEE-negative-
+  capture carve-out, the mate-magnitude-alpha check, the `fThreat` check,
+  and both exemptions. **This is a bigger behavioral gap than "HelpSearch
+  lacks LMP"** — `HelpSearch()` is currently reducing/pruning-eligible on
+  moves `Search()` would exempt outright.
+- `Search()` passes real `bbMinorAttacksSnapshot`/`bbRookAttacksSnapshot`
+  attack-bitboard snapshots into `EstimateHangingMaterialCp`. `HelpSearch()`
+  passes zeroed snapshots (`split.c:1215-1216`), degrading its hang
+  estimate (and, by extension, `ShouldLatePrune`'s joint-table lookup once
+  step 4 below gives it one).
+- `split.c:1245` and `split.c:1268` write `ctx->sPlyInfo[ctx->uPly]
+  .iExtensionAmount` — but both writes happen **after** `MakeMove`, i.e.
+  after `ctx->uPly` has already advanced (confirmed: `search.c:465` binds
+  `pi = &ctx->sPlyInfo[ctx->uPly]` *before* the move loop's `MakeMove`
+  calls; `move.c:928` is where `MakeMove` increments `ctx->uPly`).
+  `split.c:1156`'s own `ComputeMoveExtension` call already compensates
+  with `uBegin[ctx->uPly - 1]` one line away, so the "-1 after MakeMove"
+  pattern is known and applied inconsistently, not unknown. **This is two
+  separate bugs, not one:**
+  1. Wrong slot: writes the child's `iExtensionAmount`, not the mover's —
+     invisible to the null-move history-reduced-bailout check one ply
+     down (searchsup.c).
+  2. Stale-on-skip: `search.c:1386`'s own comment documents that it used
+     to only write `pi->iExtensionAmount` when `iExtend < 0`, and that was
+     itself a bug (a later sibling move could read a stale reduction left
+     by an earlier one) — fixed in `Search()` by writing it
+     unconditionally every iteration. `split.c:1245`/`1268` still only
+     write inside `if (iLMR < 0)` / the re-search branch, i.e. still have
+     the stale-on-skip bug `Search()` already fixed once.
+  Both must be fixed together: unconditional write, to
+  `ctx->sPlyInfo[ctx->uPly - 1].iExtensionAmount`.
+
+## EFP is gone — update, don't carry forward
+
+A prior session removed EFP's explicit per-move futility-pruning block
+from `Search()`'s move loop entirely. Confirmed by search: `ShouldFutilityPrune`/
+`FutilityPrun*`/`StaticNullMove`/`ShouldStaticNull` have zero hits anywhere
+in `search.c`/`searchsup.c`/`split.c`. Only inert names remain:
+`EFP_FH_MIN_SAMPLES` (search.c:84, unused #define), `_PostReduceDepthBucketEfpAligned`
+(a depth-bucketing helper, name is now just vestigial), and
+`fAnyMoveEFPPruned` (search.c:1851, read in an `else` branch comment on
+the store-upper-bound path — confirm whether this flag can still ever be
+set anywhere before relying on it in the extraction; if nothing sets it
+any more it's dead and should be dropped in step 0 below, not carried into
+the shared kernel).
+
+This also **resolves the doc's previous closing question** ("unify EFP and
+LMP before this project") — there's no EFP left to unify; `ShouldLatePrune`'s
+joint FH% table is the sole pruning mechanism now. Drop that requirement.
+
+It also means the previously-planned "related gap" about `pi->uPVDistance`
+never written in `HelpSearch()` is **moot**: `uPVDistance` has zero hits
+anywhere in the codebase now (it was EFP-era instrumentation, removed
+along with EFP). Nothing to port.
+
+## What should NOT be extracted
+
+The two loops differ along three axes that are not just stylistic — they
+reflect genuinely different control/data flow, and trying to force them
+into one shape will make the result worse, not better.
+
+1. **Move acquisition.** `Search()` pulls from a staged iterator (hash
+   move, killers, generated moves, ..., tracked via index `x`).
+   `HelpSearch()` pulls from a shared move pool via `_GetNextParallelMove`,
+   which also hands back the *current* shared `iAlpha`/`iBestScore`
+   (another helper thread may have raised them since this thread's last
+   iteration). This is loop-header logic. **Stays in each caller.**
+2. **Result publication.** `Search()` keeps `iAlpha`/`mvBest` as plain
+   thread-local variables, calls `UpdatePV()`, and uses `goto end`/`goto
+   no_more_moves`. `HelpSearch()` publishes through the locked
+   `_UpdateSplitInfo(ctx, mv, iScore, u)` and checks `g_SplitInfo[u]
+   .fTerminate` after every move. Different synchronization models.
+   **Stays in each caller.**
+3. **The split-launch decision itself** (`search.c:1082-1173`,
+   `StartParallelSearch` and the `g_fCanSplit`/`g_uNumHelpersAvailable`
+   gating). Must stay outside the extracted kernel, or a helper thread
+   executing the kernel could attempt to launch a nested split, which the
+   current design does not expect. **Stays in `search.c` only.**
+
+## What SHOULD be extracted
+
+The inner per-move kernel, called once per move after it's been pulled off
+whichever move source the caller uses — current location
+`search.c:1175-1681` (SEE computation through `UnmakeMove` and the
+`iCumulativeExtend` restore), immediately before the publication tail that
+starts at `search.c:1684`:
+
+```
+SEE (pre-move, on pos)
+  -> MakeMove
+  -> ComputeMoveExtension
+  -> CanWeReduceOrPruneMove gate
+       -> ShouldLatePrune (LMP)
+       -> GetLMRReduction (LMR)
+  -> PVS search (null-window, then full-window re-search if it raises alpha)
+  -> re-search to full depth if a reduced move fails high
+  -> UnmakeMove
+```
+
+## The instrumentation problem (the design question the old draft missed)
+
+The span to extract is NOT a clean make/decide/search/unmake block with no
+side channels. The publication tail at `search.c:1684-1777` reads several
+locals the kernel's interior sets, and `JOINT_EXTRA_VERIFY`'s shadow-search
+block sits *between* the PVS re-search and `PRUNE_VERIFY`'s fail-high
+check, inside the make/unmake span — the instrumentation can't be
+boundary-excluded without duplicating the real move-handling logic around
+it. Locals that cross the proposed kernel/caller boundary in both
+directions:
+
+- Caller → kernel (already loop-invariant, computed once outside the
+  per-move loop; pass as params, don't recompute): `uRawDepthB`
+  (`search.c:993`), `uRawImproveB`, `fNodeSplitEligible`.
+- Kernel → caller (read by the publication tail at 1684+, must survive the
+  call): `fIsLeftoverMove`, `fMoveIsPruneReduceEligible`,
+  `uThisMoveHangBucket`/`GapBucket`/`NumBucket`/`DepthBucket` (all
+  `PERF_COUNTERS`/`FH_TABLE_COLLECTION`-gated), `fThisMoveIsCountermoveMatch`/
+  `uCMEvidenceBucket` (`MOVE_ORDERING_EVIDENCE_COLLECTION`-gated),
+  `fThisMoveJointExtraEligible`/`uThisMoveJointExtraTier`
+  (`PERF_COUNTERS`-gated), `fVerifyFlagged`/`uVerifyBand`/`uVerifyDepthB`/
+  `uVerifyAttB`/`uVerifyRateB` (`PRUNE_VERIFY`-gated), and the `TEST_NULL`
+  node-cost pair.
+
+**Decision needed before writing code, not discovered mid-extraction**:
+bundle all of this into one `OUT` struct (e.g. `MOVE_KERNEL_DIAGNOSTICS`,
+`#ifdef`-sectioned to match each field's existing gate) that the kernel
+fills in and the caller's publication tail reads — rather than a long
+flat parameter list. The user's note below ("put what you can in
+`PLY_INFO`") is the other half of this: some of these (the bucket/tier
+values) are genuinely per-move-at-this-ply state and belong as fields on
+`ctx->sPlyInfo[ctx->uPly - 1]` (indexed the same way `iExtensionAmount`
+already is) rather than threaded through call/return at all — this
+removes most of the diagnostics-struct bulk and gives `HelpSearch()`'s
+own ply slot the same fields for free. Resolve this split (struct vs.
+`PLY_INFO` fields vs. a mix) as part of scoping the extraction, before
+step 3.
+
+## Two sequencing traps that will break the "bit-identical node counts" gate
+
+- `pf->iCumulativeExtend += iExtend` (search.c:1387) happens *after* the
+  LMP-prune `continue` (search.c:1313-1315) — the prune path `UnmakeMove`s
+  but never increments/decrements `iCumulativeExtend` for that move at
+  all. A `MOVE_KERNEL_PRUNED` return must preserve that asymmetry exactly;
+  don't "clean up" by making prune and search paths symmetric around
+  `iCumulativeExtend`.
+- `WE_SHOULD_STOP_SEARCHING` (search.c:1684) is checked *after*
+  `UnmakeMove` and *after* the `iCumulativeExtend -=` restore. A
+  `MOVE_KERNEL_ABORTED` return must not skip either of those — the kernel
+  itself must still fully unwind (unmake + restore) before reporting
+  abort, not bail out of the middle of its own cleanup.
+
+## Proposed signature
+
+```c
+typedef enum _MOVE_KERNEL_RESULT
+{
+    MOVE_KERNEL_SEARCHED,   // move was made, searched, and unmade; iScore valid
+    MOVE_KERNEL_PRUNED,     // move was late-move-pruned; not made; iScore invalid
+    MOVE_KERNEL_ABORTED     // search was stopped mid-move; iScore is a partial result
+} MOVE_KERNEL_RESULT;
+
+FORCEINLINE MOVE_KERNEL_RESULT
+SearchOneMove(SEARCHER_THREAD_CONTEXT *ctx,
+              MOVE mv,
+              ULONG uDepth,
+              SCORE iAlpha,
+              SCORE iBeta,
+              SCORE iEval,
+              ULONG uLegalMoves,
+              FLAG fIsLeftoverMove,
+              FLAG fFullWindow,
+              FLAG fThreat,
+              ULONG uRawDepthB,
+              ULONG uRawImproveB,
+              BITBOARD bbMinorAttacksSnapshot[2],
+              BITBOARD bbRookAttacksSnapshot[2],
+              OUT SCORE *piScore,
+              OUT INT *piExtend);
+    // Diagnostics (hang/gap/num/depth buckets, countermove-evidence
+    // bucket, joint-extra tier, prune-verify band) are NOT parameters --
+    // see "the instrumentation problem" above. They land in
+    // ctx->sPlyInfo[ctx->uPly - 1] (written by the kernel before it
+    // unmakes) wherever that's a clean fit, or an OUT diagnostics struct
+    // otherwise. Finalize the split while scoping step 3, not here.
+```
+
+Notes on why each non-obvious parameter is there:
+
+- **`fFullWindow` is explicit, not inferred.** `Search()`'s current code
+  infers "is this the first move at this node" from the sentinel
+  `iBestScore == -INFINITY_SCORE` (search.c:1396). That sentinel is only
+  true for `Search()`'s actual first move. `HelpSearch()` must never take
+  that branch — a helper thread's first move off the shared pool is *not*
+  the node's first move — and today this is only safe because
+  `_GetNextParallelMove` happens to never hand back `-INFINITY_SCORE`.
+  Once this logic is shared, that implicit contract becomes load-bearing
+  in a way that won't assert if violated. Making it an explicit
+  caller-supplied flag removes the trap.
+- **`uLegalMoves` is a parameter, not computed inside.** It feeds
+  `MoveNumBucket`/`ShouldLatePrune`'s floor and `CanWeReduceOrPruneMove`'s
+  own `uLegalMoves > 1` check. `search.c` has it as a running counter;
+  `split.c` must pass `g_SplitInfo[u].uAlreadyDone + uMoveNum + 1`
+  (matching what it already computes locally as `uSplitLegalMoves` at
+  split.c:1217 — confirm this matches the `> 5` floor `split.c:1189`
+  hardcodes today, which should become a call into the shared
+  `CanWeReduceOrPruneMove` instead of staying hand-rolled). Getting this
+  wrong silently changes LMP/LMR aggressiveness with no assert to catch
+  it.
+- **`fThreat` must be plumbed through `g_SplitInfo`.** `CanWeReduceOrPruneMove`
+  takes `fThreat` as a real gating input (searchsup.c:783) but
+  `chess.h:1612-1662`'s `SPLIT_INFO` struct has no such field today — add
+  one, populated by the originating thread at split-launch time (same
+  pattern as `iEval`/`iImprovement`), or `HelpSearch()` can't call the
+  real gate at all.
+- **Attack snapshots are real parameters, not internally zeroed.** This is
+  the fix for the degraded-hang-estimate gap above: `HelpSearch()` must
+  start passing real snapshots instead of the zeroed
+  `bbNoMinorAttacksSnapshot`/`bbNoRookAttacksSnapshot` it uses today
+  (split.c:1215-1216). Requires extending `g_SplitInfo[]` to carry
+  `bbMinorAttacksSnapshot`/`bbRookAttacksSnapshot` from the originating
+  thread (already computed in `Search()` before `StartParallelSearch`).
+  **Open risk, worth raising with the user explicitly (not just noting):**
+  once the kernel is shared, `FH_TABLE_COLLECTION`'s sampling
+  (search.c:1323-1365) fires from helper threads too. If step 4 (unify)
+  lands before step 5 (real snapshots), those samples get logged with
+  zeroed snapshots — training the joint FH% table on degraded inputs, not
+  just giving `HelpSearch()` a degraded *decision*. **Recommendation: land
+  step 5 together with step 4, not as a follow-up**, or gate
+  `FH_TABLE_COLLECTION`'s increments to the originating thread only
+  (`ctx->uPly` vs. some "is this a split helper" check) until step 5 lands.
+  Ask the user which they'd rather do before step 4.
+- **Return value is an enum, not control flow via `goto`.** The kernel
+  can't `goto end`/`goto no_more_moves`/`continue` — those targets don't
+  exist in the extracted function. Each caller's own loop dispatches on
+  the enum: `Search()` on `PRUNED` does `continue` (and must NOT touch
+  `iCumulativeExtend`, see the sequencing trap above); on `ABORTED` it
+  does its `goto end` with `iBestScore = iScore`; `HelpSearch()` on
+  `ABORTED` does `break`. The abort *condition* itself also differs
+  (`WE_SHOULD_STOP_SEARCHING` vs. `g_SplitInfo[u].fTerminate`) — stays a
+  caller-side check made around the kernel call, not a predicate pushed
+  into it.
+
+## Suggested sequencing
+
+0. **Housekeeping**: confirm whether `fAnyMoveEFPPruned` (search.c:1851)
+   can still be set anywhere post-EFP-removal; if dead, delete it rather
+   than carry it into the shared kernel's scope. Small, standalone.
+1. **Standalone fix**: `iExtensionAmount`'s two bugs in `split.c` (wrong
+   slot + stale-on-skip, see Motivation above) — write
+   `ctx->sPlyInfo[ctx->uPly - 1].iExtensionAmount = iExtend` unconditionally,
+   every iteration, matching `search.c:1386`'s already-fixed convention.
+   Commit on its own, verified via `precommit_check.sh`.
+2. **Extraction**: pull the kernel out of `search.c` into a new shared
+   header (`search_inline.h`, `FORCEINLINE` across translation units
+   requires a header), make `Search()` call it, with `HelpSearch()`
+   untouched (still running its own inline copy, including its own
+   gap-riddled gate). This isolates "did the extraction change
+   `Search()`'s behavior" from "did unifying the two callers change
+   `HelpSearch()`'s behavior." Resolve the instrumentation-placement
+   question (PLY_INFO fields vs. diagnostics struct) as part of this
+   step, before writing the header.
+3. **`g_SplitInfo` plumbing**: add `fThreat` and the two attack-snapshot
+   bitboards to `SPLIT_INFO` (chess.h), populated at split-launch time.
+   Do this *before* step 4, not after — see the `FH_TABLE_COLLECTION`
+   risk above.
+4. **Unification**: make `HelpSearch()` call the same kernel via the real
+   `CanWeReduceOrPruneMove()` gate (replacing its hand-rolled 7-condition
+   version) and `ShouldLatePrune`, removing its inline copy entirely. This
+   is the commit that actually changes `HelpSearch()`'s behavior — it
+   gains LMP, the full 9-check+2-exemption gate, and real attack
+   snapshots, all at once, since step 3 already landed the plumbing they
+   need.
+
+## Verification plan
+
+Standard gates at every step (`precommit_check.sh` — `TEST=1` self-test
+suite + `debug_smoke_test.sh`), plus:
+
+1. **Isolate the single-threaded path first (after step 2).** Force
+   `g_uNumHelpersAvailable = 0` (no split ever fires) and confirm **node
+   counts are bit-identical to HEAD** on the three curated suites
+   (`ecm_ringers`, `ecm_confident_quick`, `ecm_hard_quick`) at a fixed `sd`
+   depth, per the `head_reference` protocol in CLAUDE.md. This should be a
+   pure refactor at this stage — "no change whatsoever," not "no
+   significant change." Any node-count delta here is a real extraction
+   bug, not noise. Watch specifically for the two sequencing traps above
+   (`iCumulativeExtend` on the prune path, `WE_SHOULD_STOP_SEARCHING`
+   ordering) — those are exactly the kind of thing that produces a subtle
+   node-count drift instead of a crash.
+2. **After step 4, re-enable helpers and compare separately**, expecting a
+   real difference this time (`HelpSearch()` now has LMP, the full gate,
+   and real snapshots it didn't have before). Compare solve counts and
+   fail-high rates across the three curated suites per CLAUDE.md's
+   existing methodology (measure the pick, not just the game). Rebuild
+   `head_reference/` as the new baseline once this change is committed.
+3. Run the full `precommit_check.sh` gate before every commit in the
+   sequence above, not just at the end — this is a search/move-generation
+   change throughout.
+
+## Performance: `Search()` is the current profiler hotspot
+
+Confirmed with the user: `Search()` is the single hottest function in the
+program right now. That changes the risk profile of this refactor from
+"pure correctness/maintainability" to "must not cost NPS" — a few concrete
+implications for how to do each step:
+
+- **`FORCEINLINE` is load-bearing, not decorative, here.** The whole
+  point of extracting into a header function instead of a normal
+  `.c`-file one is that the compiler can fully inline `SearchOneMove` at
+  both call sites and optimize it in the context of each caller's
+  surrounding code (register allocation across the call, dead-branch
+  elimination of whichever `#ifdef` blocks aren't compiled in) exactly as
+  if the code were still written inline. If it isn't actually inlined in
+  the shipping `GENETIC=1 PERF_COUNTERS=1 MP=1 SIXTYFOUR=1` build, this
+  refactor becomes a real regression (a non-inlined call in the hottest
+  function in the program), not a neutral one. Don't just assert
+  `FORCEINLINE` and move on — check post-step-2 that `clang` actually
+  inlines it (`-Rpass=inline`/`-Rpass-missed=inline`, or inspect
+  `nm`/disassembly for a surviving `SearchOneMove` symbol) before trusting
+  the bit-identical-node-count gate alone; a node-count match doesn't
+  prove NPS didn't drop.
+- **Don't let the diagnostics-boundary decision regress the hot path.**
+  The "instrumentation problem" section above already flags that several
+  locals need to cross the kernel/caller boundary. For the fields that
+  move into `ctx->sPlyInfo[ctx->uPly - 1]`: writing them is already in
+  the hot path today (`iExtensionAmount` already lives there), so this is
+  likely free. For anything kept as an `OUT` diagnostics struct instead:
+  every field in that struct gets written on every move in the
+  `PERF_COUNTERS`/`FH_TABLE_COLLECTION`-eligible path even when the
+  calling code only reads a couple of them, and the struct itself is a
+  stack object passed by pointer — prefer the `PLY_INFO`-field approach
+  wherever a field is genuinely per-ply state (most of them), and reserve
+  an `OUT` struct only for values with no natural per-ply home. This is
+  exactly the tradeoff the "instrumentation problem" section says to
+  resolve before writing code — performance is now part of *why* that
+  decision matters, not just design cleanliness.
+- **Don't widen the non-`PERF_COUNTERS` build's hot path.** The
+  `SearchOneMove` signature above is sized for the worst case (every
+  diagnostic input). In a release build with none of those macros
+  defined, make sure the corresponding parameters either compile away
+  entirely (same `#ifdef`-sectioning convention the rest of this file
+  already uses) or are proven by the compiler to be dead weight — don't
+  let `uRawDepthB`/`uRawImproveB`/the snapshots turn into always-live
+  arguments on every call when the only consumers are instrumentation
+  that's compiled out. Verify by comparing generated code size/a quick
+  NPS run of a `PERF_COUNTERS`-undefined build before and after step 2,
+  not just the instrumented build used for the suite gates.
+- **Opportunistic, in-scope wins to look for while the code is open**
+  (don't go hunting outside this refactor's diff, but don't ignore an
+  obvious one either, since the whole per-move span is being rewritten
+  once regardless): any snapshot/bucket value currently recomputed
+  per-move that's actually loop-invariant per node (the doc's
+  `uRawDepthB`/`uRawImproveB` hoist is already one example carried over
+  from the current code — check for others while rewriting this block,
+  e.g. whether `GetMoveFailHighPctThousandths(mv, &uFHAttempts)` or the
+  joint-table lookup have any sub-computation that's the same across
+  moves at a node and is currently being redone); and whether the
+  diagnostics-struct-vs-PLY_INFO choice above can also drop a redundant
+  write (e.g. if a value is written to both a local and `pi->` today,
+  keeping only the `pi->` copy and reading it back removes a stack
+  write). Treat these as reviewed, measured changes (node-count +
+  NPS-on-`head_reference`-suite comparison), not assumed-safe
+  micro-optimizations — a performance claim needs the same before/after
+  evidence a correctness claim does here.
+- **Verification gate, added to the existing plan**: alongside the
+  bit-identical node-count check in step 2's isolation gate, also record
+  wall-clock NPS for the same `sd`-depth suite runs, before and after
+  each step, on this box (which the project's own CLAUDE.md already
+  warns is noisy under shared load — do a back-to-back A/B, not a single
+  run, per the Environment notes). A refactor that's node-count-identical
+  but measurably slower per node is a regression this plan must not ship
+  silently.
+
+## Open question for the user
+
+Whether to gate `FH_TABLE_COLLECTION`'s sampling to the originating
+thread only until step 3's snapshot plumbing lands, or accept a short
+window (within the same commit sequence, steps 3+4 landing together) where
+no helper-thread sample is ever collected with zeroed snapshots because
+the plumbing and the unification land atomically. Recommendation above is
+the latter (land 3 and 4 together) — flagging here so it's a decision, not
+an assumption.