fix(sensing-server): fix CSI per-node count clamp — #803 (part 2)

The pure-CSI per-node path clamped its own occupancy estimate before the
aggregator could read it. estimate_persons_from_correlation (DynamicMinCut)
returns 0-3, but it was mapped to a score via `corr_persons / 3.0`, putting
2 people at 0.667 — just under the 0.70 up-threshold of
score_to_person_count — so the per-node count never climbed past 1, leaving
node_max stuck at 1 for CSI-only nodes even when the min-cut cleanly
separated two people.

Replace the lossy /3.0 mapping with a threshold-aligned corr_persons_to_score
(1->0.40, 2->0.74, 3->0.96) whose steady state round-trips back to the same
count through the EMA + hysteresis bands, while still gating transient noise.

A convergence test replays the exact CSI-loop EMA and asserts min-cut=2 now
reports 2 / 3 reports 3 / 1 reports 1, plus a regression test documenting
that the old /3.0 mapping pinned two people to 1.

Full suite: 586 passed, 0 failed.

Co-Authored-By: claude-flow <ruv@ruv.net>
This commit is contained in:
ruv
2026-05-31 10:09:58 -04:00
parent a933fc7732
commit 4007db5d13
2 changed files with 80 additions and 2 deletions
+1 -1
View File
@@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]
### Fixed
- **Person count no longer pinned to 1 — addresses #803.** The aggregate occupancy reported by the sensing server was derived from `smoothed_person_score`, an EMA-smoothed *activity* score (amplitude variance / motion / spectral energy). That score saturates near a single occupant — one moving person maxes it out — so it cannot discriminate occupancy *count* and stayed clamped at 1 across S3/C6 and the Python/Docker/Rust servers. Meanwhile the count-aware per-node estimates the ESP32 paths already compute (firmware `n_persons`, and the DynamicMinCut `corr_persons`) were stashed in `NodeState::prev_person_count` and then **discarded** by the aggregator (same dead-wiring class as #872). The aggregator now takes `max(activity_count, node_max)` via a unit-tested `aggregate_person_count` helper, so a node positively estimating 23 occupants is surfaced instead of overwritten. The fix can only ever *raise* the count when a node reports more people, so the single-occupant case is provably never inflated (regression-guarded by test). Full multi-person accuracy still depends on the per-node estimator quality; this removes the server-side clamp that masked it. 582 sensing-server tests pass.
- **Person count no longer pinned to 1 — addresses #803.** The aggregate occupancy reported by the sensing server was derived from `smoothed_person_score`, an EMA-smoothed *activity* score (amplitude variance / motion / spectral energy). That score saturates near a single occupant — one moving person maxes it out — so it cannot discriminate occupancy *count* and stayed clamped at 1 across S3/C6 and the Python/Docker/Rust servers. Meanwhile the count-aware per-node estimates the ESP32 paths already compute (firmware `n_persons`, and the DynamicMinCut `corr_persons`) were stashed in `NodeState::prev_person_count` and then **discarded** by the aggregator (same dead-wiring class as #872). The aggregator now takes `max(activity_count, node_max)` via a unit-tested `aggregate_person_count` helper, so a node positively estimating 23 occupants is surfaced instead of overwritten. The fix can only ever *raise* the count when a node reports more people, so the single-occupant case is provably never inflated (regression-guarded by test). **Second half:** the pure-CSI per-node path itself clamped its own estimate — the DynamicMinCut occupancy (`estimate_persons_from_correlation`, 03) was mapped to a score via `corr_persons / 3.0`, putting 2 people at 0.667, *just under* the 0.70 up-threshold of `score_to_person_count`, so the per-node count never climbed past 1 (so `node_max` was also stuck at 1 for CSI-only nodes). Replaced it with a threshold-aligned `corr_persons_to_score` mapping (1→0.40, 2→0.74, 3→0.96) whose steady state round-trips back to the same count through the EMA + hysteresis, while still gating transient noise. A convergence test replays the exact EMA loop to prove min-cut=2 now reports 2 (and documents that the old `/3.0` mapping reported 1). Full multi-person accuracy still depends on the underlying estimator quality; this removes the two server-side clamps that masked it. 586 sensing-server tests pass.
- **MQTT publisher now actually runs (`--mqtt`) — closes #872.** The `--mqtt*` flags were defined only in `cli::Args` (dead code, referenced nowhere) while the binary parses a *separate* `main::Args` with no mqtt fields, and `main.rs` never started the `mqtt::` publisher — so MQTT/Home-Assistant integration was completely unwired (`--mqtt` errored as an unexpected argument, and even with the Docker image's `--features mqtt` build the publisher never ran). Earlier attempts chased a Docker *rebuild*; the real cause was disconnected *code*. Extracted the flags into a shared `cli::MqttArgs` (`#[command(flatten)]` into both structs), spawn the publisher on `--mqtt`, and bridge the JSON sensing broadcast into the typed `VitalsSnapshot` stream with a defensive `serde_json::Value` mapping. Verified end-to-end against `mosquitto`: 20 HA auto-discovery entities + live state (presence/person-count/…). 577 (default) / 580 (`--features mqtt`) tests pass.
### Added
@@ -3024,6 +3024,80 @@ fn estimate_persons_from_correlation(frame_history: &VecDeque<Vec<f64>>) -> usiz
}
}
/// Map a DynamicMinCut occupancy estimate (`estimate_persons_from_correlation`,
/// 03) onto a target score whose steady state round-trips back through
/// `score_to_person_count` to the *same* count (issue #803).
///
/// The CSI path EMA-smooths this target and re-discretises it via
/// `score_to_person_count`. The previous `corr_persons / 3.0` mapping put a
/// 2-person estimate at 0.667 — just under the 0.70 up-threshold — so the
/// smoothed score could never climb past 1, pinning the per-node count to 1
/// even when the min-cut cleanly separated two people. These anchors sit
/// inside the hysteresis bands so a *sustained* estimate converges to the
/// matching count while transient noise stays gated by the EMA:
/// 1 → 0.40 (below the 0.55 down-threshold)
/// 2 → 0.74 (between the 0.70 up- and 0.78 down-thresholds → reachable
/// both climbing from 1 and falling from 3)
/// 3 → 0.96 (above the 0.92 up-threshold)
fn corr_persons_to_score(corr_persons: usize) -> f64 {
match corr_persons {
0 => 0.20,
1 => 0.40,
2 => 0.74,
_ => 0.96,
}
}
#[cfg(test)]
mod corr_persons_round_trip_tests {
//! Issue #803 — a sustained min-cut occupancy estimate must survive the
//! CSI path's EMA + `score_to_person_count` re-discretisation instead of
//! collapsing back to 1.
use super::*;
/// Replays the CSI-loop smoothing (`score = score*0.92 + target*0.08`)
/// followed by `score_to_person_count`, exactly as the per-node path does,
/// and returns the steady-state reported count.
fn converge(corr_persons: usize) -> usize {
let mut score = 0.0f64;
let mut count = 1usize;
for _ in 0..400 {
let target = corr_persons_to_score(corr_persons);
score = score * 0.92 + target * 0.08;
count = score_to_person_count(score, count);
}
count
}
#[test]
fn sustained_one_person_estimate_reports_one() {
assert_eq!(converge(1), 1);
}
#[test]
fn sustained_two_person_estimate_reports_two() {
assert_eq!(converge(2), 2, "#803: min-cut=2 must round-trip to count 2");
}
#[test]
fn sustained_three_person_estimate_reports_three() {
assert_eq!(converge(3), 3);
}
#[test]
fn old_div3_mapping_would_pin_two_people_to_one() {
// Regression-documents the bug: 2/3 = 0.667 never crosses the 0.70
// up-threshold, so the old mapping reported 1 for two people.
let mut score = 0.0f64;
let mut count = 1usize;
for _ in 0..400 {
score = score * 0.92 + (2.0 / 3.0) * 0.08;
count = score_to_person_count(score, count);
}
assert_eq!(count, 1, "old corr_persons/3.0 mapping was the #803 bug");
}
}
/// Convert smoothed person score to discrete count with hysteresis.
///
/// Uses asymmetric thresholds: higher threshold to *add* a person, lower to
@@ -5041,7 +5115,11 @@ async fn udp_receiver_task(state: SharedState, udp_port: u16) {
// DynamicMinCut person estimation from subcarrier correlation.
let corr_persons = estimate_persons_from_correlation(&ns.frame_history);
let raw_score = corr_persons as f64 / 3.0;
// #803: map the min-cut count onto a threshold-aligned score
// so it round-trips back to the same count. The old
// `corr_persons / 3.0` left 2 people at 0.667 — under the
// 0.70 up-threshold — so the count was pinned at 1.
let raw_score = corr_persons_to_score(corr_persons);
ns.smoothed_person_score = ns.smoothed_person_score * 0.92 + raw_score * 0.08;
if classification.presence {
let count =