fix(sensing): adaptive_classifier sorts with unwrap_or(Equal) — NaN panic (#611) - #613
Merged
Conversation
…611) Reported by @bannned-bit. v2/crates/wifi-densepose-sensing-server/src/ adaptive_classifier.rs:94 did: sorted.sort_by(|a, b| a.partial_cmp(b).unwrap()); f64::partial_cmp returns None on NaN, so `.unwrap()` panics. CSI data from real ESP32 hardware can produce NaN (silent DSP div-by-zero, empty buffer, etc.), and this code path runs on every frame in the classify() hot path — a single NaN frame kills the entire sensing server process. Fix swaps for unwrap_or(Ordering::Equal), matching the pattern the same file already uses at lines 149-150 and 155 (those sites were already NaN-safe; this site was an oversight). Scoped audit: greped the v2/ tree for `partial_cmp(b).unwrap()`. The other 3 hits are in #[cfg(test)] blocks (spectrogram.rs:269, depth.rs:234, connectivity.rs:477) where panic-on-NaN is acceptable because test inputs are controlled. Only adaptive_classifier.rs:94 was a production-path crash. Severity: critical per reporter — runtime panic on real-world data. Patch: 1-line behavioural change + comment. Co-Authored-By: claude-flow <ruv@ruv.net>
3 tasks
ruvnet
pushed a commit
that referenced
this pull request
May 19, 2026
… (#624) #613 fixed adaptive_classifier.rs:94 (the IQR sort) and called the audit done, but the grep used `partial_cmp(b).unwrap()` as a literal and missed seven additional production sites that use comparator variants: adaptive_classifier.rs:205 AdaptiveModel::classify() argmax over softmax probs — same per-frame hot path as #611. NaN flows through normalise → logits → softmax and still reaches this site even after the IQR fix. adaptive_classifier.rs:480 train() argmax (training accuracy loop) adaptive_classifier.rs:500 train() per-class argmax main.rs:2446, 2449 count_persons_mincut variance source/sink select csi.rs:602, 605 count_persons_mincut variance source/sink select (duplicate of main.rs logic in csi.rs) For the variance-select sites, note that the *outer* `unwrap_or((0, &0))` only catches an empty iterator — it cannot rescue a panic raised inside the comparator. A single NaN in `variances[]` still aborts the process. Same fix as #613: swap `.unwrap()` for `.unwrap_or(std::cmp::Ordering::Equal)` inside the comparator closure. Pure behavioural change, no API surface. Re-audit of the remaining `partial_cmp(...).unwrap()` matches in v2/: they are all inside `#[cfg(test)]` / `#[test]` blocks (spectrogram.rs:269, depth.rs:234, connectivity.rs:477, vital_signs.rs:737) where inputs are controlled and panic-on-NaN is acceptable.
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
1 participant
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.
Closes #611 — credit @bannned-bit for the precise reproduction + diff.
Bug
v2/crates/wifi-densepose-sensing-server/src/adaptive_classifier.rs:94:f64::partial_cmpreturnsNoneonNaN, so.unwrap()panics. CSI data from real ESP32 hardware can produceNaN(silent DSP div-by-zero, empty buffer, etc.), and this code runs on every frame in theclassify()hot path. A singleNaNframe kills the entire sensing-server process. Critical severity.Fix
Matches the pattern the same file already uses at lines 149-150 and 155 — this was an oversight, not a deliberate panic.
Audit of related sites
grep -rn 'partial_cmp(b)\.unwrap()' v2/found 4 hits. Onlyadaptive_classifier.rs:94is in production code; the other 3 are in#[cfg(test)]blocks where panic-on-NaN is acceptable because test inputs are controlled:v2/crates/wifi-densepose-pointcloud/src/depth.rs:234—#[test](line 226), assert! followsv2/crates/wifi-densepose-signal/src/spectrogram.rs:269—#[cfg(test)]mod (line 222)v2/crates/ruv-neural/ruv-neural-signal/src/connectivity.rs:477— also test context (line 480 assert!)Leaving those alone — fixing test sites would just hide test bugs.
Test plan
cargo check -p wifi-densepose-sensing-server --no-default-features— passes in 22sclassify()— should now produce sensible output instead of panic. Reporter already verified the bug; the fix is mechanical.🤖 Generated with claude-flow