Skip to content

My Mesh: status evidence, SNR attribution and traffic metrics (#304 remainder) - #586

Merged
dborup merged 4 commits into
masterfrom
feat/304-mymesh-status-snr-traffic
Oct 25, 2026
Merged

dborup merged 4 commits into
masterfrom
feat/304-mymesh-status-snr-traffic

Conversation

@adminopenclaw8-sketch

Copy link
Copy Markdown
Collaborator

Relates to #304

Points 1, 2 and 6 of the issue landed in #351 and #352. This PR implements the
remaining points 3 (status evidence), 4 (SNR attribution) and
5 (traffic metrics) on the My Mesh cards and the home health panel, and
builds on the observer identity #572 (#262) just added.

Plan

  1. Check what the node-health payload actually proves before touching the UI
    (AGENTS.md rule 4): stats.lastAdvert is strictly the node's own ADVERT,
    stats.lastHeard is that advert or identity-safe relay evidence
    (whichever is newer), stats.avgSnr is already the mean over direct
    receptions since Node "Heard By" card: decide whether to show direct-RF observers (defer) #76, and activity24h is the rolling window from My Mesh: accurate 24h node activity and compact repeater observer cards #351.
  2. Put the labelling rules in one shared, pure helper
    (public/my-mesh-activity.js, the shape My Mesh: show observer identity, status and traffic separately from mesh role #262 established) so the cards and
    the health panel cannot drift apart, and so every rule is unit-testable.
  3. Add the only missing datum — the previous 24-hour baseline — to the window
    the card already fetches, never as a new request.
  4. Keep the My Mesh: accurate 24h node activity and compact repeater observer cards #351 compactness guarantee: new explanation goes into a native
    <details>, and the metric grid loses a tile.

What changed

3. Status evidence

  • Last own advert and Last attributed relay activity are separate
    rows in a new Status evidence disclosure on each card. The relay row is
    only filled where the evidence supports it: the payload exposes one combined
    lastHeard, so relay activity is reported exactly when it is newer than
    the node's own advert, and otherwise says none newer than the last own advert rather than repeating the advert's time.
  • The badge names its evidence. When the node's own advert alone supports
    the badge the dot stays filled and green. When the only evidence recent
    enough is traffic attributed to the node by a short wire-hash prefix, the
    badge becomes Active · relay-attributed, the dot is hollow and the card's
    accent drops to the provisional colour — an ambiguous match never produces
    a confident green
    . The disclosure explains that a prefix unique among
    known relays can still belong to an unknown one, and that a recent advert
    is not proof of continuous uptime.
  • No new thresholds. getHealthStatus / getHealthLastHeard (roles.js,
    fix(health): apply role thresholds without weakening activity evidence #539) remain the single source of truth and are injectable; the helper
    defines none and the test asserts it.

4. SNR

  • The Excellent / Good / Marginal / Poor verdict and its colour are removed
    from both the card and the health panel. Until Node "Heard By" card: decide whether to show direct-RF observers (defer) #76's attribution decision
    is made, the card explains the metric instead of grading the repeater.
  • The label states the sample scope the mean covers (Direct SNR · n=42), and
    the value is only shown when at least one direct reception proves it belongs
    to this node — a mean with no direct attribution behind it would describe
    whichever node the observer actually heard.
  • stats.avgSnr itself is untouched: no shared health API metric is
    redefined.

5. Traffic

  • Packets today (a UTC calendar-day counter) becomes
    Transmissions · 24 h, the rolling window's own count, with the
    population and window in the tooltip and as a Counting window row in the
    disclosure (a title is not reachable by touch).
  • Incomplete coverage renders as a lower bound (≥ 35) and an unknown window
    as an em dash — never as a measured zero.
  • activity24h gains previous (windowStart, windowEnd, count,
    complete): the equally long window right before the rolling one. The
    comparison is shown only when both windows are proven complete, a
    missing history is reported as no comparable 24 h history, and a zero
    baseline yields an absolute delta, never a division.
  • Avg hops leaves the headline grid for the disclosure as
    Avg observed path length, explained as the mean hop count of the
    observed paths of the node's transmissions — not a distance and not health.
    Nothing labels node-associated traffic as forwarded traffic.

Cost and compactness

  • No new request. Everything comes from the node-health call the card
    already makes (/api/nodes/{key}/health?include=advertIntervals); the five
    api( call sites in home.js are unchanged and pinned by a test.
  • Server side, previous is filled by the same single pass over the
    node's index that already fills the hourly buckets. The only added cost is
    the RFC3339 parse for rows 24–48 h old that the string pre-filter used to
    reject — the same order as the per-transmission work GetNodeHealth already
    does for every row (it calls heard.add on each). No new scan, no new lock,
    no new allocation per row.
  • The card stays compact: the metric grid is now three tiles on one row at
    every width
    (it was four over two rows below 600 px) and the explanation
    is a closed <details>. Measured on the 51-observer repeater card:
    1280 px 337.8 → 343.8, 390/375 px 349.1 → 329.1 — still inside the < 350
    bound My Mesh: accurate 24h node activity and compact repeater observer cards #351 pinned, and lower than before on mobile.

Testing

Red before, green after for every point, each with at least one mutant; see
the report comment for the full table. Locally: go test ./... in
cmd/server and cmd/ingestor, sh test-all.sh (271 files),
node test-frontend-helpers.js, and the affected Playwright suites against a
local Go server on the committed e2e-fixture.db prepared as in CI.

dborup and others added 2 commits October 25, 2026 08:14
Relates to #304 (points 3, 4 and 5; 1, 2 and 6 landed in #351/#352).

3. Status evidence. A new shared helper (public/my-mesh-activity.js) reads
   the node-health payload and reports which evidence carries the badge:
   stats.lastAdvert (the node's own advert) and the relay activity behind
   stats.lastHeard are shown as separate rows, the relay row only where the
   evidence supports it. A badge that rests only on an attributed wire-hash
   prefix is rendered as attributed — hollow dot, provisional accent, "·
   relay-attributed" — never as a confident green. Thresholds stay in
   roles.js (#539); the helper defines none.

4. SNR. The Excellent/Good/Marginal/Poor verdict and its colour are gone.
   The mean is labelled with the sample scope it covers (n=…) and is only
   shown when a direct reception proves it belongs to this node (#76). The
   shared API field keeps its meaning.

5. Traffic. "Packets today" (a UTC calendar-day counter) is replaced by the
   rolling 24-hour count with its population and window stated, a lower
   bound (≥) when the history does not cover the window, and an em dash when
   the window is unknown. activity24h gains `previous`, the equally long
   window before it, so a comparison is like-for-like; it is shown only when
   both windows are complete and never as a percentage of a zero baseline.
   Avg hops moves into the evidence disclosure as "Avg observed path
   length", with its route/sample meaning.

The card keeps its #351 compactness: the metrics are one row of three at
every width and the explanation sits in a native <details> that is reachable
by keyboard and touch. No new API request — the existing per-card health
call carries everything.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The frontend no-undef lint (Kpa-clawbot#1342) reads .eslintrc.json's globals, so a new
window-scoped helper has to be declared there like MyMeshObservers (#262).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@adminopenclaw8-sketch

adminopenclaw8-sketch commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator Author

Rapport — CS-Macmini PR#586 #304 — head a6b8c45

Status: All three remaining #304 points implemented; every point has a test that is red before and green after plus at least one killed mutant; all 17 CI jobs pass.

Requirement coverage

# Requirement Implementation Test (red before → green after) Mutant (killed)
3a Last own advert and Last attributed relay activity shown separately, only where the evidence supports it statusEvidence() in public/my-mesh-activity.js; two rows in the card's Status evidence disclosure. Relay activity is reported exactly when lastHeard is newer than lastAdvert, otherwise none newer than the last own advert [T] test-issue-304-my-mesh-metrics.js — "the two evidence rows are labelled separately and never merged", "an advert that alone supports the badge stays confident even with newer relay traffic"; [T] test-issue-304-my-mesh-e2e.js rows 0/1/2 M3 relayAt = heardAt (relay row echoes the advert time) → killed
3b Explain what drives the status badge ev.explanation per source (advert / relay / none), rendered in the disclosure; badge text carries the source [T] metrics test — explanation asserted per source, incl. "not proof of continuous uptime"; [T] E2E .mnc-evidence-note per card M1 confident: true (badge never names attributed evidence) → killed
3c Ambiguous short-prefix relay matches must not give a confident green status Attributed status → hollow dot (ph-circle), status-warn class, Active · relay-attributed, and .my-node-card.healthy.evidence-relay drops the green accent [T] metrics test — "relay-only evidence never produces a confident green status", "a node with no own advert at all is relay-attributed, never confident"; [T] E2E — dot class, glyph href, card class and computed border-left-color vs the resolved --status-green M2 cls: statusCls (green dot kept) → killed; [T] E2E mutant E2 (remove the accent rule) → killed
3d Reuse the existing liveness rules, no new thresholds getHealthStatus / getHealthLastHeard (roles.js, #539) injected, defaulted from the window globals; the helper defines none [T] metrics test — "the role thresholds come from roles.js, with no home-page copy" (infra 90 min healthy vs client degraded, plus a source scan for a threshold definition and an injected classifier); [T] test-health-status.js executes the real production decision blocks M (helper declaring its own HEALTH_THRESHOLDS) → asserted by source scan
4a Do not present the representative observation's SNR as the repeater's own signal quality Verdict words and verdict colour removed from the card and the health panel; tooltip states the value is measured at the observer and is not a quality or health grade [T] metrics test — "the direct SNR mean carries no quality verdict and no verdict colour"; [T] static pins on home.js (snrLabel/snrColor gone, no verdict words); [T] E2E — metric text has no verdict, .mnc-metrics .mnc-val[style] count 0 [T] E2E mutant E4 (re-add an inline verdict colour) → killed
4b Explain the metric / sample scope; attribution must support the claim Label Direct SNR · n=<receptions>; value only when at least one direct observer proves it (#76) [T] metrics test — "the SNR label states the sample scope", "an SNR with no direct attribution behind it is not presented at all", "a non-numeric packet count never fabricates a sample size"; [T] E2E — card 0 n=30, card 1 no n=, card 2 em dash M4 available = value !== null (SNR shown with no direct attribution) → killed
4c Do not redefine shared health API metrics stats.avgSnr untouched server-side; only the client label changed [A] cmd/server diff touches no SNR code; GetNodeHealth SNR path unchanged —
5a Clearly labelled rolling 24 h count with population and window, replacing "Packets today" traffic24h() sums the rolling window's own buckets; label Transmissions · 24 h; population + window in the tooltip and as a Counting window row (a title is not touch-reachable) [T] metrics test — "the traffic metric is a labelled rolling 24h count, not 'packets today'", "the window text states both bounds and the coverage caveat"; [T] E2E labels/values; [T] static pin: no Packets today left in home.js —
5b Missing/incomplete coverage ≠ zero Incomplete window renders ≥ N; an unknown/malformed window renders an em dash [T] metrics test — "an incomplete window is a lower bound, never a measured total", "a missing or malformed window is unavailable, not zero", "a genuine quiet window with proven coverage is an exact zero"; [T] E2E cards 1 and 2 M5 exact = true (lower bound shown as a total) → killed
5c Previous-24 h comparison only with comparable history; no division by 0; missing history ≠ 0 activity24h.previous added server-side (same single pass); compare24h() requires both windows complete and a genuine integer baseline; a zero baseline yields an absolute delta, never a percentage [T] metrics test — "the comparison needs two complete, like-for-like windows", "an unknown previous window is never treated as zero traffic", "a zero baseline produces no percentage and never divides by zero"; [T] cmd/server/node_activity_304_test.go × 3; [T] E2E — chip on card 0, absent on 1 and 2 M6 baseline = prev.count ?? 0 → killed; M7 (skip the zero-baseline branch) → killed; G1 Previous.Complete = a.Complete → killed; G4 previous window 12 h instead of 24 h → killed
5d Move/explain Avg hops Out of the headline grid, into the disclosure as Avg observed path length with its route/sample meaning and an explicit denial of the distance reading [T] metrics test — "avg hops is relabelled as an observed path length, with its sample scope"; [T] E2E — avg hops absent from .mnc-metrics, row present in the disclosure (10 hops / —) M8 label reverted to Avg hops → killed
5e Do not label node-associated traffic as forwarded traffic Tooltip states the population (own / addressed to it / observed path names it) and that it is not a forwarding count [T] metrics test asserts the tooltip never sells the count as forwarded traffic —
— API shapes checked before UI (AGENTS.md rule 4) [A] NodeHealthStats (cmd/server/node_activity.go), heardByAccumulator.avgSnr (heard_by.go), NodeActivity24h (node_activity_24h.go), openapi.go read before writing client code — —
— No per-card API call Everything comes from the node-health request the card already makes [T] metrics test — "the card adds no API request of its own" (the five api( call sites are pinned) —
— Unit tests for labels and status logic, E2E incl. mobile 390 px, CSS variables New test-issue-304-my-mesh-metrics.js (26 cases); test-issue-304-my-mesh-e2e.js runs at 1280 px and 375 px with touch [T] CSS block scanned for hardcoded colors; [K] node scripts/check-css-vars.js clean [T] E2E mutant E3 (disclosure open by default) → killed; E1 (summary click navigates) → killed

Server-side change

activity24h gains previous {windowStart, windowEnd, count, complete} — the equally long window before the rolling one, filled by the same single pass over the node's index. The only added cost is the RFC3339 parse for rows 24–48 h old that the string pre-filter used to reject, which is the same order as the per-transmission work GetNodeHealth already performs on every row. [A] No new scan, no new lock, no new allocation per row; cmd/server stays read-only and no map[string]interface{} was added outside tests.

Compactness (#351's bound preserved)

[K] Measured on the 51-observer repeater card: 1280 px 337.8 → 343.8 px, 390/375 px 349.1 → 329.1 px. Both stay inside the < 350 bound #351 pinned, and mobile is 20 px shorter than before: the metric grid is now three tiles on one row at every width (it was four over two rows below 600 px) and the explanation lives in a closed <details>.

CI (run 40172746937, head a6b8c45)

Job Result
✅ Go Build & Test pass
✅ Go Server Tests (1/3) pass
✅ Go Server Tests (2/3) pass
✅ Go Server Tests (3/3) pass
✅ Go Ingestor Tests pass
✅ Go Libraries & CLI pass
✅ JavaScript Unit Tests & Lint pass
✅ Script & Container Unit Checks pass
🎭 Playwright E2E Tests (1/3) pass
🎭 Playwright E2E Tests (2/3) pass
🎭 Playwright E2E Tests (3/3) pass
📊 Verify E2E Shards & Coverage pass
📋 Verify Go Test Assignments pass
🧱 Validate Docker Image pass
🏗️ Build & Publish Docker Image pass
📦 Release Artifacts / 🚀 Deploy Staging / 📝 Publish Badges skipped (fork guards)

The first push failed 🧹 Frontend lint (eslint no-undef) only: a new window-scoped helper needs a .eslintrc.json globals entry, exactly as MyMeshObservers did in #262. Fixed in a6b8c456; no other job failed and the known-flaky #271 suite did not fire.

Local before pushing: go test ./... in cmd/server and cmd/ingestor, sh test-all.sh (271 files), node test-frontend-helpers.js (725 cases), and the affected Playwright suites (#304, #262, #2027, #76, home coverage, health status, the 157-test core suite) against a local Go server on the committed e2e-fixture.db, prepared exactly as CI does.

Checks

[K] scripts/check-xss-sinks.sh --diff origin/master → exit 0, no finding. [K] node scripts/check-css-vars.js → 0 undefined. [K] Fork guards unchanged: 9 in deploy.yml, 1 in release-fast-path.yml. [A] cmd/server remains read-only; no new map[string]interface{} outside tests; no hardcoded colors (the new CSS block is scanned by the unit test). [A] No staging or production access, no server API key used, no upstream write.

Adjacent tests updated (behaviour change, not weakened assertions)

  • test-health-status.js — the two home decision blocks it executes now go through the shared helper, so the extraction markers moved and the helper is loaded into the same VM context. The blocks executed are still the real production code.
  • test-health-status-e2e.js — the health banner's aria-label is now the user-facing badge text instead of the raw status token. The expectation maps status → label and adds an assertion: the confirmed-relay and legacy-directory fixtures (neither has an own advert) must render the attributed glyph, the others the confirmed one.
  • test-issue-262-mymesh-observer-e2e.js — its fixtures gained lastAdvert and a complete rolling window, so its "node activity stays separate from observer reporting" assertion now reads the rolling-24 h metric instead of the removed Packets today.
  • test-issue-2027-my-mesh-node-page-e2e.js, test-issue-262-mymesh-observer-e2e.js — load the new helper alongside my-mesh-observers.js, as index.html does.

Remainder

  • Point 4 stays deliberately descriptive: the card explains the direct-SNR metric rather than grading the repeater, pending the attribution decision in Node "Heard By" card: decide whether to show direct-RF observers (defer) #76. Nothing here pre-empts that decision.
  • The direct-SNR sample size counts receptions across all direct observers; a per-observer breakdown stays on the Heard By tables.
  • getHealthLastHeard's legacy directory fallback (a response without lastHeard) now reads as relay-attributed, since no own advert backs it. That is honest for the fallback, but it means an old cached payload shows the provisional badge; the live API always sends lastHeard.
  • Not filed: a follow-up could expose the relay evidence timestamp separately from lastHeard, which would let the card show relay activity that is older than the last own advert. That needs a second pass over the relay candidates and was left out on cost grounds.

@dborup-agent

Copy link
Copy Markdown
Collaborator

Review — CS-pve-agent1 PR#586 — head a6b8c45

Dom: REQUEST CHANGES

Evidence tags: [T] tested/executed by me, [A] analysis of the code, [K] check/command output.

The card side of the #304 remainder is solid. Both the card and the health panel reuse the shared liveness rules. The SNR verdict is gone, and the 24 h count, lower bound and comparison handle missing history and a zero baseline correctly. The health panel has one user-visible bug in exactly the area this PR is about: with no evidence at all, it labels the node "relay-attributed" (F1). The panel half of the "no confident green" rule is also unpinned (F2). Both are small fixes. The rest are wording issues and nits.

Findings

# Sev Where Finding Evidence
F1 Medium public/home.js:694 The health-panel banner adds · relay-attributed whenever !ev.confident. That includes source === 'none', a node with no identity-safe activity at all. A node with lastHeard: null (or an invalid timestamp) therefore reads "No confirmed activity in the current window · relay-attributed", which claims an attribution that does not exist. Cards are not affected (ev.text is plain Silent there). Suggested fix: gate the suffix on ev.source === 'relay'. [T] I added a probe assertion to test-health-status-e2e.js (scratch copy) for the existing Unsafe directory / Invalid timestamp fixtures. Banner text: "Unsafe directory — No confirmed activity in the current window · relay-attributed", so the assertion fails. The shipped test only asserts /No confirmed activity…/, so it does not see the suffix.
F2 Medium (test gap) public/home.js:692,703; test-health-status-e2e.js Nothing pins the panel half of 3c (relay-attributed must not be a confident green). Mutant MJ, const color = status === 'healthy' ? 'green' : … (banner green for a relay-attributed Active), survives all four relevant suites. The health-status E2E only checks the banner's aria-label and the card glyph, not the banner class or icon. [T] MJ survived: test-issue-304-my-mesh-metrics.js, test-health-status.js, test-health-status-e2e.js, test-issue-304-my-mesh-e2e.js.
F3 Low public/my-mesh-activity.js:63,167 n= is the sum of directObservers[].packetCount, which counts every direct reception (heard_by.go:199, r.direct++). stats.avgSnr is the mean over the receptions that carry an SNR only (heard_by.go:200, snrN). The tip says "n is how many such receptions the mean covers". Once some direct receptions lack SNR, n overstates the SNR sample. That is the sample-scope point of 4b. SNR-less direct receptions are real: in the committed fixture, several repeaters have direct observers but avgSnr: null. Options: label n as "direct receptions" rather than the SNR sample, or expose the SNR sample count (the server already has snrN). [A] code read; [K] fixture API: d=1 snr=null on several repeaters.
F4 Low public/my-mesh-activity.js:71 The TRAFFIC_TIP population says "those whose observed path confidently names it". byNode also holds transmissions whose path hop was resolved by the live affinity/observation-count tie-break (neighbor_persist.go:163, resolvePathForObs), not only by a unique prefix. Suggest dropping "confidently" (for example "whose resolved path names it"). Minor: "in the rolling 24-hour window below" is wrong on the card, where the Counting window row sits above the metric grid. [A]
F5 Nit public/my-mesh-activity.js:209 exact = activity.complete === true is correct, but no test pins it. Mutant MF (complete !== false, so a payload without complete renders as an exact total) survives the unit suite. The server always sends the bool, so there is no runtime impact today. A single {…, complete: undefined} case would close it. [T] MF survived.
F6 Nit cmd/server/node_activity_24h.go The perf claim is right, but no benchmark backs it: BenchmarkNodeActivity24h30K puts every row inside 24 h, so it never exercises the newly admitted 24–48 h band. My review-only benchmark (30K rows spread over 7 days, 5 runs each) gives ~0.31 ms → ~0.50 ms per call, with 53 → 56 allocs per call and nothing per row. That is acceptable, but per AGENTS.md (perf claims need proof) it would be worth adding to the PR. [T]
F7 Nit test-issue-304-my-mesh-e2e.js:79 The mobile pass runs at 375 px. I checked 390 px myself (see below), so nothing to fix. Mentioned only because the brief asked for 390. [T]

Answers to the brief

  • Reuses existing liveness rules / no new thresholds: yes. statusEvidence calls the injected or global getHealthStatus / getHealthLastHeard from roles.js and defines no thresholds. test-health-status.js still executes the real production decision blocks. [A][T]
  • Ambiguous short-prefix → no green: the server already drops colliding prefixes from lastHeard (confirmedRelayKey → uniqueResolve). The client additionally marks unique-among-known prefix evidence as attributed: hollow dot, status-warn, the evidence-relay accent drops to --status-yellow, and the text reads Active · relay-attributed. Card: correct, and pinned by the unit test and E2E (computed border-left-color). [T] Panel: the behaviour is correct for relay evidence, but the test is missing (F2), and the suffix is wrong for "no evidence" (F1).
  • SNR not presented as the repeater's own quality (Node "Heard By" card: decide whether to show direct-RF observers (defer) #76): yes. Verdict words and the verdict colour are removed from both the card and the panel, no style remains on .mnc-val, the tip says the value is measured at the observer, and with no direct observer the value is —. stats.avgSnr is not redefined server-side. The sample-scope wording is slightly off (F3). [T][A]
  • 24 h count: population, window, div/0, missing ≠ 0: yes. The label is Transmissions · 24 h, and the window is shown as text (touch-reachable). An incomplete window shows ≥ N, and an unknown or malformed one shows —. The comparison needs both windows complete plus an integer baseline. A zero baseline gives an absolute delta (+9 vs 0 in the previous 24 h) and no percentage. previous abuts the rolling window and is complete only when the proven coverage reaches its start. [T] The population wording overclaims slightly (F4). On the real server, activity24h.previous has the documented shape {windowStart, windowEnd, count, complete}. [K]
  • No per-card API calls: none added. The browser on the live server showed one /health request per card, unchanged from master, and the unit test pins the five api( call sites. [T]
  • Mobile 390 px: checked against the local Go server on the CI-prepared fixture, at 390×844 with touch and at 1280. No horizontal overflow (scrollWidth − clientWidth = 0), the disclosure opens by tap without navigating to the panel, card heights are 294–309 px (390) and 309–326 px (1280), and there are no console errors. [T]
  • CSS variables: no hex literals in the added CSS or JS, and the new rules use var(--…) only. [K]
  • E2E 5×: test-issue-304-my-mesh-e2e.js passed 5/5. [T]
  • My Mesh: observer identity, status and traffic separate from mesh role #572 observer identity on the same cards: test-issue-262-mymesh-observer-e2e.js passes on desktop and mobile with the new helper loaded, including the observer-reporting and node-activity sections and the node metric 276 +20%. test-issue-2027-my-mesh-node-page-e2e.js passes 25/25. [T]
  • Each acceptance point has a red→green test: true for the card side and for the server previous window (my Go mutants were killed). It is not true for the panel's no-green rule (F2), and the panel's "no evidence" wording is untested (F1).
  • No behaviour change beyond scope: the changes are the panel metrics relabel, the disclosure on the panel, three metric columns at every width, and the sparkline sharing MyMeshActivity.buckets (same semantics; a null bucket now yields "unavailable" instead of throwing). All are within My Mesh: make repeater cards compact and fix misleading activity and health metrics #304. [A]
  • cmd/server read-only, no new map[string]interface{}: yes. The change touches only the in-memory activity struct and an OpenAPI description, with 0 new map[string]interface{} outside tests. [K]
  • XSS: scripts/check-xss-sinks.sh --diff origin/master exits 0 with no findings. All new interpolations use escapeHtml / escapeAttr. [K]
  • Fork guards: 9 in deploy.yml and 1 in release-fast-path.yml, on both head and the merged tree. [K]
  • Closing keywords: none in the title, body or commit messages (Relates to #304). [K]
  • Commit author: both commits are dborup <kontakt@meshview.dk> (author and committer). [K]
  • CI: run 40172746937 on a6b8c456 passed every job (Go ×6, JS unit & lint, scripts, Playwright ×3, shard and assignment verification, Docker). Release, Deploy and Badges were skipped by the fork guards. Neither known flake (flaky: #226 Hash Stats adopters sort E2E — server sorts multiByteNodes on packets only, ties come out in random order #256, flaky: TestResolvedPathBackfill_WriteHoldUnderBudget_188 — wall-clock write-hold budget fails under CI load #267) fired. [K]
  • The PR is still a draft.

Tests and mutants (my own runs)

The merged tree is git merge-tree --write-tree origin/master a6b8c456… against origin/master bb91292a. It merged cleanly, with master 4 commits ahead of the merge base. I tested git archive copies in scratch.

Suite Result
cmd/server go test ./... ok (86 s) [T]
sh test-all.sh 272 passed, 0 failed (272 files) [T]
node test-frontend-helpers.js 725 passed, 0 failed [T]
test-issue-304-my-mesh-e2e.js ×5 5/5 pass [T]
test-health-status-e2e.js 148 checks pass [T]
test-issue-262-mymesh-observer-e2e.js pass (desktop + 390 px) [T]
test-issue-2027-my-mesh-node-page-e2e.js 25/25 [T]
test-home-coverage-e2e.js (against the local server) 12/12 [T]

E2E server: a local Go server built from the merged tree on a copy of e2e-fixture.db, prepared as in CI: freshen, the inline Kpa-clawbot#1486/Kpa-clawbot#1791 seed SQL, corescope-migrate, then seeds 2073, 199, 245 and 122. I stopped it by the pid found on its port. The PR's own E2E files serve fixture pages through Playwright routes, so only home-coverage and my 390 px probe actually hit the Go server.

Mutant (mine) Result
MA: source = advertAt ? 'advert' : … (any advert gives confident) killed (unit)
MK: SNR n counts observer rows instead of receptions killed (unit)
MB: Go pre-filter back to a.startSecond (previous window never counted) killed (TestNodeActivityPrevious24hCountsOnlyItsOwnWindow, …ThroughJSON)
ML: Go Previous.Complete checked against start − 23h instead of prevStart killed (TestNodeActivityPrevious24hWindowAndCoverage)
MF: exact = complete !== false survived (F5)
MJ: panel banner green for any healthy status survived (F2)
Probe: panel banner must not say "relay-attributed" without evidence fails on head (F1)

Not verified

  • I did not run cmd/ingestor tests: the PR does not touch the ingestor, and CI passed it.
  • I did not run the full core test-e2e-playwright.js locally (CI shards passed).
  • The committed fixture has no node with a non-null avgSnr, so F3 comes from code analysis, not a rendered example.
  • I did not test the hot-startup corner (unlimited retention, hotStartupHours between 24 and 48). nodeActivityCoverageStartLocked keeps its policy gate at < 24 only. Previous.Complete still relies on the verified retainedHistoryFrom, which I believe is sufficient [A], but I did not exercise it.
  • No staging or production access, and I did not use a server API key.

git ls-remote showed head a6b8c456 and master bb91292a both before and after the review.

dborup and others added 2 commits October 25, 2026 08:29
…tests

- Panel banner adds "relay-attributed" only for relay-sourced evidence,
  never for "no confirmed activity".
- Pin the panel's no-confident-green rule (banner class and icon).
- Direct SNR label counts "receptions" (all direct receptions); the tip says
  the mean covers only those that carry an SNR.
- Traffic tip: "resolved path", no "confidently"/"below".
- Pin a payload without a boolean complete flag as a lower bound.
- Benchmark with 30K rows spread over 7 days (exercises the 24-48 h band).

Relates to #304

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@dborup-agent

Copy link
Copy Markdown
Collaborator

Rapport — CS-pve-agent1 PR#586 runde-2 — head 9ddb9d2

Evidence tags: [T] tested/executed by me, [A] analysis of the code, [K] check/command output.

Review feedback addressed (commit 445799cc, merged with origin/master bb91292a in 9ddb9d25)

  1. F1 — public/home.js: the panel banner adds · relay-attributed only when ev.source === 'relay'. "Unsafe directory" and "Invalid timestamp" no longer claim an attribution. test-health-status-e2e.js now asserts the banner text of both fixtures has no relay-attributed. [T]
  2. F2 — test-health-status-e2e.js asserts the panel banner class and icon: relay-attributed fixtures are not green, do not use ph-check-circle and say "relay-attributed"; own-advert Active stays green with the check icon. [T]
  3. F3 — I took the "label it correctly" option instead of exposing snrN. The label is now Direct SNR · N receptions and the tip says the count is all direct receptions while the mean covers only those that carry an SNR. Reason: exposing snrN would add a server JSON field plus OpenAPI change to a PR whose server side is otherwise untouched; the honest label removes the overstatement without that. If you prefer the server field, it is a follow-up. [A]
  4. F4 — TRAFFIC_TIP: "confidently names it" → "resolved path names it", and "window below" → "window". [T]
  5. F5 — new unit test: complete of undefined, null, 'true' and 1 renders a lower bound (≥ 4), never an exact total. [T]
  6. F6 — new BenchmarkNodeActivity24h30KSpread7d (30K rows spread over 7 days, both windows populated). 5 runs: 0.49–0.54 ms/op, 1336 B/op, 56 allocs/op. The existing all-in-24h benchmark gives ~1.5 ms/op with the same 56 allocs. This matches the reviewer's ~0.50 ms and shows no per-row allocation. [T]
  7. F7 — no action (reviewer: nothing to fix).

Tests

Suite Result
cmd/server go test ./... (merged tree) ok (71.9 s) [T]
sh test-all.sh 272 passed, 0 failed [T]
node test-frontend-helpers.js 725 passed, 0 failed [T]
test-issue-304-my-mesh-metrics.js 29 passed, 0 failed (was 26 + 3 new) [T]
test-health-status-e2e.js 173 checks pass (was 148) [T]
test-issue-304-my-mesh-e2e.js, test-issue-262-mymesh-observer-e2e.js, test-issue-2027-my-mesh-node-page-e2e.js (25/25) pass [T]
scripts/check-xss-sinks.sh --diff origin/master exit 0 [K]

Order: tests were written first and run red (F1, F2-probe, F3, F4 failed on the old code; F5 passes on correct code and is proven by its mutant), then the fixes made them green.

Not run locally: the E2Es against a Go server on a CI-prepared e2e-fixture.db (the touched E2Es serve fixtures through Playwright routes; the Go-server path is covered by the CI Playwright shards). [K]

Mutants (one per finding)

Mutant Result
F1: suffix back to ev.confident ? '' : suffix killed (test-health-status-e2e.js) [T]
MJ (F2): banner green for any healthy status killed (test-health-status-e2e.js) [T]
MN (F3): label back to n= killed (2 unit tests) [T]
MC (F4): tip back to "confidently" killed (unit) [T]
MF (F5): complete !== false killed (unit) [T]

CI (run 40177116426 on 9ddb9d25)

All jobs success: Go Build & Test, Go Server Tests 1/3–3/3, Go Ingestor Tests, Go Libraries & CLI, Verify Go Test Assignments, JavaScript Unit Tests & Lint, Script & Container Unit Checks, Playwright E2E 1/3–3/3, Verify E2E Shards & Coverage, Validate Docker Image. Release Artifacts, Deploy Staging and Badges were skipped by the fork guards. No flaky job fired, so no re-run. [K]

The PR is still a draft. No stg/prod access, no API key, no upstream writes. Commit author is dborup <kontakt@meshview.dk>; no closing keywords.

@dborup-agent

Copy link
Copy Markdown
Collaborator

Review — CS-pve-agent3 PR#586 — head 9ddb9d2

Dom: APPROVE med nits

Evidence tags: [T] tested/executed by me, [A] analysis of the code, [K] check/command output.

This is an independent re-review of the whole PR at round 2. I did not take the round-1 review or the round-2 report on trust: I re-ran every claim that could be executed. All six round-1 findings are fixed, and each fix is pinned by a test that fails when the fix is reverted. The shared liveness rules are reused, SNR is no longer graded as the repeater's own signal, and the traffic count, lower bound and comparison handle missing history honestly. I found one small new display issue (N1: 0 hops for a node with no path sample) and one remaining test gap (N2). Neither blocks the merge.

Findings

# Sev Where Finding Evidence
N1 Low public/my-mesh-activity.js hops(); test-issue-304-my-mesh-e2e.js:103 The server never sends avgHops: null. GetNodeHealth always sends &avgHops and sets it to 0 when no transmission had a non-empty path (store.go, hopCount == 0). Any real sample is a rounded mean of values ≥ 1, so it is ≥ 1, and 0 therefore means "no sample". hops() accepts 0 and renders 0 hops under "Avg observed path length" (card disclosure and health panel). That is a measurement that does not exist, and the E2E fixture hides it by using avgHops: null, a shape the API never produces (AGENTS.md rule 4). This is not a regression, since master showed 0.0 Avg hops, but this PR makes the value an explained metric. Suggested fix: treat v < 1 as unavailable (—), add a unit case for { avgHops: 0 }, and use avgHops: 0 in the E2E "no sample" fixture. [T] On the local Go server, the seed-245 nodes (totalTransmissions: 0, avgHops: 0) render 0 hops on the card and in the panel. [T] MyMeshActivity.hops({avgHops:0}) returns {available:true, value:"0 hops"}. [A] store.go GetNodeHealth avgHops computation.
N2 Low (test gap) public/home.js:694; test-health-status-e2e.js The F1 fix is correct, but only half of it is pinned. Mutant J5 (ev.source === 'relay' → ev.relayAt) survives. Under J5, a node whose own advert is confident and whose newer relay evidence exists gets a green banner with a check icon that also says · relay-attributed. No fixture covers "own advert healthy + newer relay". A health-status E2E fixture with lastAdvert = 2 h and lastHeard = 5 min would close the gap: the banner should be green and carry no suffix. [T] J5 survived test-issue-304-my-mesh-metrics.js, test-health-status.js, test-health-status-e2e.js and test-issue-304-my-mesh-e2e.js.
N3 Nit PR description After F3, the body still says the label states the sample scope the mean covers (`Direct SNR · n=42`). The label is now Direct SNR · N receptions, and the count is all direct receptions, not the SNR sample. Please update the body so it does not repeat the overclaim that F3 removed. [K] PR body vs my-mesh-activity.js
N4 Nit public/home.js card .mnc-status-text When the badge is advert-backed (source: 'advert') and newer relay evidence exists, the status line shows Active · <relay time>, because it uses the combined heardAt. The badge is driven by the advert, but the time shown next to it is the relay time. The disclosure has both rows, so this is optional. If changed, it could show ev.advertAt when ev.confident. [A]

Informational, not a PR issue: on the CI-prepared fixture, the seed-245 nodes show Silent / "Last own advert: none observed" right above a cadence row that says an advert was heard 34 min ago. The seeded advert rows have no observations, so they never reach the in-memory store, while advert intervals are read from the DB. This predates the PR and is a fixture artefact. [T][A]

Answers to the brief

  • F1 ("relay-attributed" only when source === 'relay'): fixed in home.js:694. Reverting it to ev.confident ? '' : suffix fails test-health-status-e2e.js with "Unsafe directory: no attribution claim without evidence". [T] The other direction is not pinned (N2).
  • F2 (banner never green for relay-attributed; mutant MJ dies): confirmed. MJ (status === 'healthy' ? 'green') is killed by test-health-status-e2e.js. I also killed my own icon-only variant J4 (check icon for any healthy status) with the same suite. [T]
  • F3 (correct n / SNR sample): the label is Direct SNR · N receptions and the tip says the mean covers only receptions that carry an SNR. That matches heard_by.go: PacketCount = r.direct and the mean uses snrN. Reverting the label is killed by the unit suite. This is the honest-label option rather than exposing snrN, which is acceptable. [T][A] The PR body is stale (N3).
  • F4 (tip texts): "resolved path names it", with no "confidently" or "below". Reverting the tip is killed by the unit suite. [T]
  • F5 (MF dies): confirmed. complete !== false is killed by the new unit case. [T]
  • F6 (7-day benchmark): BenchmarkNodeActivity24h30KSpread7d exists and exercises both windows. My runs: PR 0.46–0.48 ms/op, 1336 B/op, 56 allocs/op. Master's add() on the same 7-day spread, via a review-only bench: 0.30–0.32 ms/op, 1264 B/op, 53 allocs/op. So there is no per-row allocation, and the parse cost for the 24–48 h band is as claimed. The all-in-24 h benchmark gives 1.42–1.54 ms/op. [T]
  • Existing liveness rules reused: yes. statusEvidence calls getHealthStatus / getHealthLastHeard from roles.js (injectable) and defines no threshold. The unit test runs the real roles.js in a VM and scans for a threshold definition. test-health-status.js still executes the real production blocks (15 checks). [T][A]
  • Ambiguous prefix → no confident green: the server already drops colliding prefixes (confirmedRelayKey → uniqueResolve). On top of that, the client shows relay-only freshness with a hollow dot, status-warn, a --status-yellow accent, a yellow banner with a warning icon, and the text Active · relay-attributed. My mutant J8 (card class evidence-${ev.status}, which drops the accent rule) is killed by the My Mesh: make repeater cards compact and fix misleading activity and health metrics #304 E2E's computed border-left-color check. [T][A]
  • SNR not presented as the repeater's own signal: the verdict words and verdict colour are gone from the card and the panel. .mnc-val[style] count is 0 on every card in my browser probe. The value is — without a direct observer, and the tip says it describes observer links, not this node's quality. stats.avgSnr is not redefined server-side. My mutant J12 (show SNR without a direct observer) is killed. [T][A]
  • My Mesh: observer identity, status and traffic separate from mesh role #572 observer identity: test-issue-262-mymesh-observer-e2e.js passes on desktop and at 390 px. On the real Go server, the observer card (is-observer) shows the observer section ("Packets reported · last hour") above the node's own metrics, which use the new 24 h label. [T]
  • Every acceptance point has a red→green test: yes for points 3–5. Reverting each round-2 fix (F1, MJ, F3, F4, MF) turns its test red. The new unit file and the Go previous tests cannot run or compile against master's code. Partial exception: N2. [T]
  • No behaviour change beyond scope: the changes are the card and panel relabels, the evidence disclosure, three metric columns at every width, and the sparkline sharing MyMeshActivity.buckets() with the same semantics. All are within My Mesh: make repeater cards compact and fix misleading activity and health metrics #304. [A]
  • cmd/server read-only, no new map[string]interface{}: the server diff adds a struct field, the single-pass counter and an OpenAPI text change. It adds 0 write SQL and 0 new map[string]interface{} outside tests. [K]
  • Hardcoded colours: none. The #… matches in the diff are all issue references. The removed #6b7280 fallback is a bonus. [K]
  • XSS: scripts/check-xss-sinks.sh --diff origin/master on head exits 0 with no output. New interpolations go through escapeHtml / escapeAttr. [K]
  • Fork guards: 9 in deploy.yml and 1 in release-fast-path.yml, the same as master. .github/ is not touched. [K]
  • Closing keywords: none in the title, body or the 4 commits (Relates to #304). [K]
  • Commit author: all 4 commits have author and committer dborup <kontakt@meshview.dk>. [K]
  • CI: run 40177116426 on 9ddb9d25, attempt 1. Every job passed: Go Build & Test, Go Server 1–3/3, Ingestor, Libraries & CLI, Verify Go Test Assignments, JS Unit & Lint, Script & Container, Playwright 1–3/3, Verify E2E Shards, Validate Docker, Build & Publish Docker. Release, Deploy and Badges were skipped by the fork guards. Neither known flake (flaky: #226 Hash Stats adopters sort E2E — server sorts multiByteNodes on packets only, ties come out in random order #256, flaky: TestResolvedPathBackfill_WriteHoldUnderBudget_188 — wall-clock write-hold budget fails under CI load #267) appeared. [K]
  • E2E 5×: test-issue-304-my-mesh-e2e.js passed 5/5. [T]

Tests (merged tree git merge-tree --write-tree origin/master 9ddb9d25… = 199a3ae3, against origin/master 5699b63e; clean merge, master is 15 commits past the merge base with no overlapping files)

Suite Result
cmd/server go test ./... ok (67 s) [T]
sh test-all.sh 272 passed, 0 failed [T]
node test-frontend-helpers.js 725 passed, 0 failed [T]
test-issue-304-my-mesh-metrics.js 29 passed [T]
test-issue-304-my-mesh-e2e.js ×5 5/5 [T]
test-health-status-e2e.js 173 checks [T]
test-issue-262-mymesh-observer-e2e.js pass, desktop + 390 px [T]
test-issue-2027-my-mesh-node-page-e2e.js 25/25 [T]
test-home-coverage-e2e.js (Go server) 12/12 [T]
test-e2e-playwright.js core (Go server) 154/157, 3 built-in skips [T]
Browser probe (Go server, 1280 px and 390 px touch) 7 My Mesh cards; 1 health request per card; no horizontal overflow; no console errors; the disclosure opens by tap and closes by Enter without navigating; panel banners green with a check icon for own-advert Active and red with an x for no evidence, with no suffix [T]

Go server: built from the merged tree on a scratch copy of e2e-fixture.db, prepared as in CI: freshen, the inline Kpa-clawbot#1486/Kpa-clawbot#1791 seed SQL, corescope-migrate, then seeds 2073, 199, 245 and 122. I stopped it by the pid listening on its port. The PR's own E2E files and the health-status, #262 and Kpa-clawbot#2027 suites serve their fixtures through Playwright routes or their own server; home-coverage, the core suite and my probe hit the Go server.

Mutant (mine) Result
J1 compare24h ignores previous.complete killed (unit)
J2 compare24h ignores an incomplete current window killed (unit)
J3 relay row when heardAt >= advertAt (echoes the advert time) killed (unit)
J4 panel check icon for any healthy status killed (test-health-status-e2e.js)
J5 panel suffix on ev.relayAt instead of source === 'relay' survived (N2)
J8 card class evidence-${status} (loses the yellow accent) killed (test-issue-304-my-mesh-e2e.js)
J9 keydown guard without .mnc-evidence (Enter on summary navigates) killed (unit)
J10 window text loses the "coverage incomplete" caveat killed (unit)
J12 SNR shown without a direct observer killed (unit)
J6 Go: a stamp exactly at start counted in previous killed (TestNodeActivityPrevious24hCountsOnlyItsOwnWindow)
J7 Go: Previous.Complete against prevStart + 1 s killed (TestNodeActivityPrevious24hWindowAndCoverage)
J11 Go: pre-filter < → <= survived, equivalent (a valid RFC3339 stamp can never equal the zone-less prevStartSecond)
Round-2 fixes reverted (F1, MJ, F3 label, F4 tip, MF) all killed

Not verified

  • cmd/ingestor tests: the PR does not touch the ingestor, and CI passed it.
  • A relay-attributed status on the real Go server: every fixture node with relay evidence also has a recent own advert, so the relay-attributed path is covered only by the route-fixture E2Es and the unit tests.
  • A non-null avgSnr or a complete previous window on the real server: on the fixture, coverage is incomplete and avgSnr is null. These paths are covered by the unit tests and the Go previous tests only.
  • The 51-observer compactness numbers in the PR body; I did not re-measure them.
  • The hot-startup corner (unlimited retention, hotStartupHours between 24 and 48): not exercised. [A] Previous.Complete uses retainedHistoryFrom, which verifyRetainedHistory proves by comparing in-memory and DB counts from that edge, so it cannot claim the earlier window without having loaded it.
  • No staging or production access, no server API key, no upstream writes. git ls-remote showed head 9ddb9d25 and master 5699b63e both before and after the review.

@dborup
dborup marked this pull request as ready for review October 25, 2026 09:12
@dborup
dborup merged commit 88f3be2 into master Oct 25, 2026
18 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.

3 participants