Skip to content

fix(nodes): remove misleading aggregate SNR headlines - #1969

Merged
efiten merged 2 commits into
Kpa-clawbot:masterfrom
n30nex:codex/node-snr-headline
Sep 6, 2026
Merged

fix(nodes): remove misleading aggregate SNR headlines#1969
efiten merged 2 commits into
Kpa-clawbot:masterfrom
n30nex:codex/node-snr-headline

Conversation

@n30nex

@n30nex n30nex commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Red commit: 2bf9be8 (local Chromium: 3 passed, 3 intended assertion failures). CI: run awaits maintainer approval (action_required); 0 jobs started.

Remove the unqualified aggregate Avg SNR row from node side-panel Overview and full-detail stats, following option 3 in #1149. Heard By retains each observer's SNR reading.

Fixes #1149.

  • E2E assertion added: test-issue-1281-location-row-e2e.js:224. Three new browser cases cover desktop side/full and mobile full views with a numeric aggregate and distinct positive/negative observer readings. Existing packet-location assertions remain intact.
  • Browser verified: local Chromium; 6 cases passed after push. Screenshots: coverage/issue-1149/issue-1149-desktop-side-panel.png, coverage/issue-1149/issue-1149-desktop-full-detail.png, and coverage/issue-1149/issue-1149-mobile-full-detail.png.
  • Validation: packet filter 99/99, aging 18/18, frontend helpers 666/666; XSS, CSS-variable, syntax, whitespace and PII checks passed.
  • Independent reviews: adversarial, lifecycle expert and TDD reviewers found no required changes. One initial browser navigation timed out; the unchanged parent rerun passed 6/6.
  • Performance/config: two production row deletions; no new requests, loops, timers, settings or customizer implications. Backend unchanged; Go suites were not rerun.

Fix commit: d7c68f3.

Preflight overrides

  • External run-all.sh is unavailable on this host. Scoped branch, red/green, PII, CSS, XSS and whitespace checks were run directly. The diff adds no migrations, SQL attribution or image markup.

@efiten

efiten commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Merging. CI had never run on this (stuck in action_required, now approved for all seven of your PRs) and came back green.

Removing the aggregate is the right call rather than trying to caveat it. An average SNR over every observer that heard a node is a number without a referent: it mixes a receiver 500 m away with one 40 km away, so it moves when the observer mix changes and not when the node's RF situation does. Keeping the per-observer "Heard By" readings, which each mean something on their own, and dropping the single figure that means nothing is the correct trade.

Verified the fixtures keep SNR non-null, so the test proves the field is deliberately not rendered rather than passing because there was nothing to render. That is the difference between this test and a vacuous one.

@efiten
efiten merged commit 108ea02 into Kpa-clawbot:master Sep 6, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(node-detail): "Avg SNR" headline metric is mathematically meaningless without time window or per-observer normalization

2 participants