fix(sensing): finish #611 NaN-panic audit — 7 more sites missed by #613 (#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 commit is contained in:
Rahul 2026-05-19 19:32:08 +05:30 committed by GitHub
parent f54f0285bd
commit c00f45e296
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 34 additions and 9 deletions

View File

@ -29,6 +29,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
process. Swapped for `unwrap_or(Ordering::Equal)`, matching the pattern the
same file already used at lines 149-150 and 155. Per-frame hot path; this was
a real production crash vector.
- **Completed the #611 NaN-panic audit across the sensing-server crate** (follow-up
to #613). The original audit grepped for the literal `partial_cmp(b).unwrap()`
and missed seven additional production sites that use comparator variants
(`partial_cmp(b.1).unwrap()`, `partial_cmp(&variances[b]).unwrap()`). All share
the same crash class — a single `NaN` in CSI-derived state panics the whole
sensing-server. Fixed:
- `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 #613 IQR fix.
- `adaptive_classifier.rs:480, 500` — training-loop argmax in `train()`
(training/per-class accuracy reporting).
- `main.rs:2446, 2449` and `csi.rs:602, 605` — variance-based source/sink
selection in `count_persons_mincut`. The outer `unwrap_or((0, &0))` only
catches an empty iterator; it cannot rescue a comparator panic.
Remaining `partial_cmp(...).unwrap()` sites in the workspace are all inside
`#[cfg(test)]` / `#[test]` blocks (`spectrogram.rs:269`, `depth.rs:234`,
`connectivity.rs:477`, `vital_signs.rs:737`) where inputs are controlled.
- **`ui/utils/pose-renderer.js` no longer divides by zero** when two render frames land in the same `performance.now()` tick (issue #519 Bug 2). `deltaTime` is now `Math.max(currentTime - lastFrameTime, 1)` before the `1000 / deltaTime` division, capping displayed FPS at 1000 — far above any real render rate, but finite so the EMA `averageFps = averageFps * 0.9 + fps * 0.1` no longer poisons itself to `Infinity` on a single zero-dt tick.
### Removed

View File

@ -200,9 +200,11 @@ impl AdaptiveModel {
probs[c] = ((logits[c] - max_logit).exp()) / exp_sum;
}
// Pick argmax.
// Pick argmax. Same NaN-panic class as #611: if any raw_feature is NaN
// it propagates through normalize → logits → softmax, then partial_cmp
// returns None and unwrap() panics the sensing server on every frame.
let (best_c, best_p) = probs.iter().enumerate()
.max_by(|a, b| a.1.partial_cmp(b.1).unwrap())
.max_by(|a, b| a.1.partial_cmp(b.1).unwrap_or(std::cmp::Ordering::Equal))
.unwrap();
let label = if best_c < self.class_names.len() {
self.class_names[best_c].clone()
@ -477,7 +479,7 @@ pub fn train_from_recordings(recordings_dir: &Path) -> Result<AdaptiveModel, Str
}
}
let pred = logits.iter().enumerate()
.max_by(|a, b| a.1.partial_cmp(b.1).unwrap())
.max_by(|a, b| a.1.partial_cmp(b.1).unwrap_or(std::cmp::Ordering::Equal))
.unwrap().0;
if pred == *target { correct += 1; }
}
@ -497,7 +499,7 @@ pub fn train_from_recordings(recordings_dir: &Path) -> Result<AdaptiveModel, Str
}
}
let pred = logits.iter().enumerate()
.max_by(|a, b| a.1.partial_cmp(b.1).unwrap())
.max_by(|a, b| a.1.partial_cmp(b.1).unwrap_or(std::cmp::Ordering::Equal))
.unwrap().0;
if pred == *target { class_correct[*target] += 1; }
}

View File

@ -598,11 +598,13 @@ pub fn estimate_persons_from_correlation(frame_history: &VecDeque<Vec<f64>>) ->
}
}
// partial_cmp returns None on NaN; the outer unwrap_or only catches an
// empty iterator, not a comparator panic. Same NaN-panic class as #611.
let (max_var_idx, _) = active.iter().enumerate()
.max_by(|(_, &a), (_, &b)| variances[a].partial_cmp(&variances[b]).unwrap())
.max_by(|(_, &a), (_, &b)| variances[a].partial_cmp(&variances[b]).unwrap_or(std::cmp::Ordering::Equal))
.unwrap_or((0, &0));
let (min_var_idx, _) = active.iter().enumerate()
.min_by(|(_, &a), (_, &b)| variances[a].partial_cmp(&variances[b]).unwrap())
.min_by(|(_, &a), (_, &b)| variances[a].partial_cmp(&variances[b]).unwrap_or(std::cmp::Ordering::Equal))
.unwrap_or((0, &0));
if max_var_idx == min_var_idx { return 1; }

View File

@ -2559,12 +2559,15 @@ fn estimate_persons_from_correlation(frame_history: &VecDeque<Vec<f64>>) -> usiz
}
}
// Source → highest-variance subcarrier, Sink → lowest-variance
// Source → highest-variance subcarrier, Sink → lowest-variance.
// partial_cmp returns None on NaN; the outer unwrap_or only catches an
// empty iterator, not a comparator panic. Same NaN-panic class as #611
// — a single NaN variance frame would kill the sensing-server process.
let (max_var_idx, _) = active.iter().enumerate()
.max_by(|(_, &a), (_, &b)| variances[a].partial_cmp(&variances[b]).unwrap())
.max_by(|(_, &a), (_, &b)| variances[a].partial_cmp(&variances[b]).unwrap_or(std::cmp::Ordering::Equal))
.unwrap_or((0, &0));
let (min_var_idx, _) = active.iter().enumerate()
.min_by(|(_, &a), (_, &b)| variances[a].partial_cmp(&variances[b]).unwrap())
.min_by(|(_, &a), (_, &b)| variances[a].partial_cmp(&variances[b]).unwrap_or(std::cmp::Ordering::Equal))
.unwrap_or((0, &0));
if max_var_idx == min_var_idx {