monkey-w1n5t0n dbe0f5d8ba fix(ml): one named example capacity; train() and trainAsync() no longer diverge ...

Phase 2, S35. Two real defects from one root cause, both confirmed by trace
rather than taken from the audit:

1. Divergence. WasmIML built its TS Dataset mirror with a cap of 100 while
   every addExample() ALSO pushed into the C++ FIFO ring, capped at 128. Since
   train() reads the C++ ring and trainAsync() reads the TS mirror, past 100
   examples the two trained on different datasets — silently.
2. Latent OOB read. nisps_ml_train sizes its sample-weight span by the C++
   side's example_count() (up to 128), but wasm-iml.ts allocates that heap
   buffer from the TS dataset's size (<=100). Once the ring exceeds the mirror,
   the span reads past the end of the caller's allocation.

Fix: name the capacity ONCE as nisps::ml::kDefaultMaxExamples = 128, used by
FixedStorage's default template arg, DynamicStorage's default ctor arg, and the
MLP<> alias (which is the only real FixedStorage instantiation path and carried
its own independent 128 literal — the last copy of this dual truth). Expose it
through nisps_ml_describe and have the TS side read it instead of hardcoding.
Dataset's constructor default is removed entirely: a default was what invited
this bug class, and the sole call site now always supplies the describe() value.

ABI NOTE: this extends nisps_ml_describe from a 6-int to a 7-int descriptor.
nisps_ml_describe always writes 7 ints regardless of the caller's buffer, so
every call site had to grow in the same change or it would overflow the WASM
heap by 4 bytes per call. All five sites updated: three in wasm-iml.ts (init
defaults, init per-instance, reshape re-describe — the finding said there were
two), one in wasm-worker.ts, one in tests/cpp/parity_wasm.mjs. The parity
harness's expected-dims check now also pins the new max_examples slot.

Regression test: tests/cpp/test_mlp_storage_defaults.cpp — pins the two storage
policies to one constant, and drives MLPCore<DynamicStorage> exactly as
bindings.cpp does past the old TS cap, asserting it saturates at 128 and not at
100. Fail-before/pass-after confirmed by temporarily setting the constant to
100: 2 failures, named. Reverted: green.

Audit correction: the cited dataset.ts:81 is the FIFO eviction check; the
hardcoded default was at dataset.ts:45.

Gates: run-all-tests.sh ALL GREEN, parity PASS.

2026-07-21 13:22:38 +02:00

127 KiB

Executable file

Raw History

View raw