port(upstream#1969): remove the aggregate Avg SNR row from node views - #45
Merged
Merged
Conversation
Red commit: `2bf9be8` (local Chromium: 3 passed, 3 intended assertion failures). CI: [run](https://github.com/Kpa-clawbot/CoreScope/actions/runs/33988112224) 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 Kpa-clawbot#1149. Heard By retains each observer's SNR reading. Fixes Kpa-clawbot#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. (cherry picked from commit 108ea02) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Brings the branch up to master 637a4ee so CI runs on current master. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Split out of #25 (commit
224e2719there). This branch holds exactly one upstream change so it can be reviewed, tested and reverted on its own.Upstream
108ea020f74a9d0a460b3e426ef0d20d0e9f0de9git cherry-pick -xonto masterfda24ca5; upstream authorship kept, and the commit message carries the(cherry picked from commit …)line.public/nodes.js,test-issue-1281-location-row-e2e.js.Problem
The node side panel and full detail page showed an unqualified “Avg SNR” averaged across all observers. That mixes links of very different quality into one number that describes none of them.
Change
Removes the aggregate “Avg SNR” row from both views. Per-observer SNR readings under “Heard By” are unchanged.
Adaptation to this fork
None in content. The changed lines are identical to upstream; only the surrounding diff context differs because this fork's files have diverged around them.
Dependencies and merge order
fda24ca5and needs no other PR from this split.TestPruneOldNeighborMetricsdeterministic). If test(ingestor): make neighbor metrics pruning deterministic #33 lands first, the expected CI failure named below disappears; nothing in this PR depends on it.Verification
Local run of the same commands as CI's “Go Build & Test” job (server tests with
-race), on this branch and on masterfda24ca5under the same conditions (same machine, run one after another):fda24ca5xss-gate-diffchannel-lib-testdecrypt-cli-build-testdockerfile-copy-invariantsdeclare -A), macOS has 3.2; identical on masterstaging-disk-monitorcss-vars-linttest-issue-1375-scope-stats-fetch.jsExactly oneapi('/scope-stats'call exists (the fixed loader) — found 2test-issue-1648-m4-emoji-scan.jsmap.js has 1 emoji/misc-icon hit(s):test-a11y-axe-routes-coverage.jsaxe ROUTES missing analytics tabs (issue #1706): areas, foreign-traffic, wardrivingtest-frontend-helpers.jsfavStar returns empty star for non-favorite: The expression evaluated to a falsy value:,favStar returns filled star for favorite: The expression evaluated to a falsy value:Baseline failures (fail identically on master; not introduced or changed here): see rows marked baseline failure, unchanged.
Browser validation (local, fixture DB, no staging/production): Local Go server (built from master) on the committed E2E fixture DB (freshened, migrated and seeded exactly like CI), serving this branch's
public/, compared side by side with the same server serving master'spublic/. Full node detail for a fixture repeater. The fixture has no SNR, so — as the upstream E2E test does with Playwright routing —/api/nodes/<pk>/healthwas answered in-page withstats.avgSnr: 6.3and two observers (12.5 dB / −8.5 dB); every other endpoint was real. master: shows an "Avg SNR" row with "6.3 dB" plus the per-observer values under Heard By. this branch: no "Avg SNR" row, "Packets Today" and the other rows intact, per-observer 12.5 / −8.5 still shown under Heard By. (Side panel and mobile variants are covered by the extended E2E test, which was not run.)Not run:
test-issue-1281-location-row-e2e.js(Playwright) have not run anywhere for this fork.eslint(not installed locally; CI installs it on the fly).-race/tests for modules this PR does not touch (unchanged code, identical to master).Expected GitHub CI: “Go Build & Test” is expected to fail on
TestPruneOldNeighborMetrics, which already fails on master (see #25's run). Downstream jobs (Playwright, image build) are therefore skipped. “Deploy Staging” and all GHCR publish steps only run onpushtomasterand cannot run for this PR.Two further ingestor tests have failed intermittently in this split's CI on branches whose
cmd/ingestortree is byte-identical to master (#27, #28), so they can also appear here without being caused by this change:TestBackfillTxLastSeen_ResolvesFromMaxObservationTimestamp: also reproduced locally on unmodified master.TestMQTTStallWatchdog_DisconnectedEscalationThrottled_1749: the suite flake that upstream test(ingestor): join the watchdog loop goroutine instead of only asking it to stop Kpa-clawbot/CoreScope#2003 (also split out of port(upstream): 26 clean upstream fixes — prune batching, /ws limits, observer liveness, watchdog race #25) addresses.GitHub CI result: run 34751738913 on
07d8a875. Go Build & Test: failure; all downstream jobs incl. Deploy Staging skipped. Failed tests:TestPruneOldNeighborMetrics: fails on master, documented baselineTestBackfillTxLastSeen_ResolvesFromMaxObservationTimestamp: intermittent, reproduced on unmodified master locally🤖 Generated with Claude Code