fix(ai): stop a single NaN from silently disabling two subsystems - #147
Merged
ApiliumDevTeam merged 1 commit intoJul 30, 2026
Conversation
ApiliumDevTeam
force-pushed
the
fix/nan-in-clamp-shaped-expressions
branch
from
July 29, 2026 10:06
30aaba4 to
9b62d8d
Compare
Both sites carried a clamp-shaped expression that clippy wanted rewritten. Following that advice mechanically would have introduced a bug in one and papered over a worse one in the other, so each is settled here instead. SurpriseGate. `min`/`max` discard NaN and return the other operand; `clamp` propagates it. But the real entry point is upstream: `record_surprise` feeds Welford's algorithm, and one non-finite sample makes the running mean NaN permanently — every later update keeps it, the adaptive threshold inherits it, and because every comparison against NaN is false the gate stops firing with nothing to show for it. Reject non-finite samples at the boundary, and fall back to the CONFIGURED threshold — the non-adaptive baseline this type already has — rather than to an invented constant, if unusable statistics arrive some other way (deserialised state is not covered by the entry guard). TransactionClassifier::confidence. `partial_cmp().unwrap()` panics on a NaN distance, which `l2_distance` produces from a corrupt centroid, so the process died inside a confidence calculation. `total_cmp` is total and also sorts NaN last, so a corrupt centroid can no longer pass itself off as the nearest one. And with every distance unusable the old code fell past the `second_dist > 0.0` test to return 1.0 — FULL confidence at the exact moment there is no information. It now answers 0.5, the same neutral value it already returns when there are no centroids at all. Five tests, each pinning a failure that was silent: a poisoned mean, a NaN threshold, the panic, and full confidence from no information.
ApiliumDevTeam
force-pushed
the
fix/nan-in-clamp-shaped-expressions
branch
from
July 29, 2026 10:25
9b62d8d to
98fa442
Compare
This was referenced Jul 30, 2026
ApiliumDevTeam
merged commit Jul 30, 2026
aa3fba8
into
ci/lint-gate-matches-the-declared-bar
12 checks passed
This was referenced Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Third of the stack. Base is #146. This is a behaviour change, not cleanup — that is why it is not in #145.
Why the lint's advice would have been wrong
Both sites carried
x.min(hi).max(lo), which clippy wants asx.clamp(lo, hi). They are not equivalent:min/maxdiscard NaN and return the other operand,clamppropagates it.SurpriseGateFollowing the lint would have turned a NaN into a NaN threshold — and every comparison against NaN is false, so
should_updatestops firing and the memory quietly stops learning. Worse than either bound.But the real entry point is upstream.
record_surprisefeeds Welford's algorithm, and one non-finite sample poisons the running mean permanently:mean += delta / nwith a NaN delta makes the mean NaN, and every later update keeps it that way. So:set_thresholdalready provides the non-adaptive baseline.TransactionClassifier::confidencepartial_cmp().unwrap()panics on a NaN distance, whichl2_distanceproduces from a corrupt centroid — so the process died inside a confidence calculation.total_cmpis total and also sorts NaN last, so a corrupt centroid can no longer pass itself off as the nearest one.And with every distance unusable, the old code fell past
second_dist > 0.0and returned 1.0 — full confidence at the exact moment there is no information. It now answers 0.5, the same neutral value the function already returns when there are no centroids at all.Verification
Five tests, each pinning a failure that was silent: a poisoned mean, a NaN threshold, the fallback choice, the panic, and full-confidence-from-nothing. Both
#[allow(clippy::manual_clamp)]removed.cargo clippy --workspace --all-targets -- -D warningsexit 0 · 2247 tests green.The new gate caught two lints in my own test code while writing this, which is the first evidence it works.