Skip to content

Region-scope visibility: Channels, Packets, and a new Scopes analytics tab - #1852

Open
dborup wants to merge 202 commits into
Kpa-clawbot:masterfrom
dborup:master
Open

Region-scope visibility: Channels, Packets, and a new Scopes analytics tab#1852
dborup wants to merge 202 commits into
Kpa-clawbot:masterfrom
dborup:master

Conversation

@dborup

@dborup dborup commented Jul 18, 2026

Copy link
Copy Markdown

Summary

Adds end-to-end visibility into MeshCore transport-region scopes (#dk, #dk-oj, etc.) across the UI — from individual messages/packets up to a full analytics dashboard — plus operational tooling (config hot-reload) and three correctness fixes found by cross-referencing real data against the actual MeshCore firmware source.

User-facing: scope on messages and packets

  • Channel messages (REST + live WebSocket) and the Packets-tab transport badge now show which region scope a message/packet carried, or "unknown" when transport-eligible but unresolved.
  • Packets-tab badge shows the resolved scope name directly in the label (not just on hover), with a max-width + ellipsis so long region names don't blow out the column.
  • Packets tab also gets a dedicated, sortable Scope column (between Type and Observer) so scoped traffic can be sorted/scanned directly instead of only spotted via the inline badge — wired into the existing column visibility toggle and mobile column-hiding. Returning visitors with a saved column-visibility preference (predating this column) now get new columns shown by default instead of silently missing them, a general fix to the column-prefs load path.

New: Scopes analytics tab

A full "Scopes" tab under Analytics, built out incrementally:

Section What it answers
Scope Statistics Existing summary/byRegion/time-series, now more accurate (see fixes below)
Channel Messages Of channel chat specifically (not all traffic), how much is scoped vs unscoped vs unknown
Region Utilization All-time diff between configured hashRegions and regions that have ever actually matched — surfaces how much of the region list is dead weight
Repeaters by Region Which repeaters have relayed traffic carrying each region scope
Bridge Repeaters Repeaters relaying for more than one region — the mesh's literal backbone nodes connecting separate regional communities
Nodes Running This Region Nodes whose OWN default_scope is a given region — as distinct from repeaters that merely relay it
Activity by Hour of Day Per-region heatmap of when during a typical day (UTC) each region is active
Scope Adoption by Channel Of the top 30 channels by volume, which ones actually use region scoping vs never

New: reload hashChannels/hashRegions without restarting (SIGHUP)

Adding a channel or region to config.json previously required a full ingestor restart to take effect — which also reset in-memory analytics state (Repeaters by Region, Bridge Repeaters, etc.) that takes real time to rebuild from live traffic. The ingestor now reloads hashChannels/hashRegions on SIGHUP (the standard Unix "reload config" convention) via an atomic key swap (cmd/ingestor/hot_reload.go), so config changes take effect without losing warm-up state. A malformed config.json during a live reload leaves the previous keys in place instead of blanking them. Documented in docs/deployment.md under "Reloading config changes without a restart (SIGHUP)".

Correctness fixes (found via real data on stg.meshview.dk, ~1100 configured hashRegions)

  1. HMAC collision on scope matching. matchScope (ingestor) returned the first configured region whose HMAC happened to match a packet's 16-bit transport_code_1, iterating a Go map in randomized order. With enough configured regions, an unrelated region can coincidentally collide (~len(regionKeys)/65534 per packet — birthday paradox). The same packet could resolve to different, sometimes-wrong region names across ingestor restarts. Now scans every configured region and only returns a name on an unambiguous single match; ambiguous matches report unknown-scoped rather than guessing.

  2. False repeater-scope attribution via ambiguous hash-prefix bucket. TransportedScopes (both the bulk computeRepeaterRelayInfoMap and per-node GetRepeaterRelayInfo) folded a full pubkey's path-hop packets together with its 1-byte raw-prefix fallback bucket — an existing, intentional approximation for relay-activity counts, which tolerates the ambiguity of a 1-byte hash matching many nodes. Applying that same fold to region-scope attribution let a repeater get credited with a scope from a packet it never actually carried — confirmed directly against the MeshCore firmware source (examples/simple_repeater/MyMesh.cpp allowPacketForward + RegionMap::findMatch): a repeater only relays a TRANSPORT_FLOOD/TRANSPORT_DIRECT packet when its own locally configured region matches, so the fold was asserting something the protocol itself wouldn't allow. Scope accumulation is now gated to the exact-key (unambiguous) match only; relay-activity counts keep the existing fallback.

  3. byPathHop bucket keys surfaced as fake repeaters. The 1-byte raw-prefix bucket keys (e.g. "b3") never correspond to a real nodes.public_key, but the first cut of "Repeaters by Region" iterated the relay-info map without filtering, so these bucket keys leaked into the UI as phantom "repeaters". Fixed by requiring role IN ('repeater','room') existence in the same lookup used to resolve display names.

Housekeeping

  • docs/deployment.md: brought in a pending restructuring of the deployment guide alongside the SIGHUP section.
  • Removed a stale duplicate git-tracked entry (docs/DEPLOYMENT.md vs docs/deployment.md) — two case-variant paths tracked as separate blobs, which collide into one file on case-insensitive filesystems (macOS/Windows), silently going stale on one side.
  • Gitignored cmd/server/prune-requests/, a runtime job-queue directory that shouldn't have been trackable in the first place.

Test plan

  • go test ./... (cmd/server, cmd/ingestor) — all green, including new regression tests for each fix (HMAC collision, prefix-bucket scope attribution, bucket-key filtering), each new analytics section, and the SIGHUP hot-reload path (hot_reload_test.go, including an end-to-end test sending a real SIGHUP to the test process)
  • Relevant frontend unit tests (test-channels-merge-1498-unit.js, test-channel-live-decrypt.js, test-issue-1849-trace-hashbytes.js) — all green
  • eslint public/*.js — 0 errors (pre-existing unused-var warnings only)
  • Verified live against real mesh data on stg.meshview.dk for every section, including manually recomputing HMAC matches to confirm the collision/false-attribution fixes against concrete real-world examples (e.g. a hyper-local scope showing repeaters hundreds of km away, traced back to the prefix-bucket bug and confirmed fixed)
  • Verified SIGHUP reload live: added a region to config.json on stg, sent kill -HUP to the ingestor, confirmed the new region matched without a restart
  • Verified the new Packets Scope column live: sortable, toggle-able, and confirmed it appears automatically for a browser with a pre-existing (pre-this-PR) saved column-visibility preference
  • Playwright e2e suite — not run locally (no Playwright in this environment); will run in CI

🤖 Generated with Claude Code

dborup added 12 commits July 17, 2026 14:20
Adds scope_name to GetChannelMessages (both SQLite and in-memory
store paths) and to the general packet response shape, then renders
it as "Scope: <name>" in the channel chat meta line so operators can
see which region scope a message was transported under.
The WS packet broadcast (IngestNewFromDB / IngestNewObservations in
cmd/server/store.go) builds its own map independent of the REST
response helpers, so scope_name was missing there even after it was
added to GetChannelMessages and txToMap. Adds it to both broadcast
paths and wires channels.js's live-append handler to read it, so
brand-new messages show their scope immediately instead of waiting
for the next periodic REST refresh.
transport_code_1 is only 16 bits, so with enough configured hashRegions
a different, unrelated region's HMAC can coincidentally also match a
packet's real code1 (expected rate ~= len(regionKeys)/65534 per
packet). matchScope used to return the first match found while
iterating the region-key map, whose order Go randomizes per process —
so an ambiguous packet's assigned scope was effectively a coin flip
that could even change across ingestor restarts.

Confirmed on stg.meshview.dk (1098 configured regions): a live GRP_TXT
message's code1=C417 matched both "#dk" (the sender's true region) and
"#dk1906" (an unrelated collider), and was observed resolving to
either name across sends.

matchScope now scans every configured region and only returns a name
when exactly one matches; two or more matches are reported as
unknown-scoped ("") rather than guessed, with a log line for operator
visibility. Doesn't fix the root collision-probability cause (that
needs fewer/coarser configured regions) but stops confidently
displaying a wrong single answer.
GetChannelMessages already returned an empty scope string for
transport-eligible packets whose region couldn't be determined (no
configured region matched, or matchScope now reports an HMAC
collision as unknown), but the UI treated empty scope the same as
"not applicable" and rendered nothing — indistinguishable from a
plain FLOOD/DIRECT message that never carries a scope at all.

Adds route_type to the channel-message payload (both SQLite and
in-memory paths, plus the decrypt-candidate and live WS paths) so the
frontend can tell "not transport-scoped" (routeType 1/2, no tag) apart
from "transport-scoped but unresolved" (routeType 0/3 with empty
scope, now shown as "Scope: unknown").
The packet detail pane already showed scope (with an "unknown scope"
fallback for empty scope_name), but the packets table itself gave no
at-a-glance signal — the existing transportBadge() "T" marker only
encoded route type.

transportBadge() now takes an optional scopeName argument: a resolved
region enriches the tooltip, an empty scope_name (transport-eligible
but unmatched/ambiguous) renders as "T?" with a distinct muted badge
style instead of the confident amber, so it's visually distinguishable
from a resolved scope without relying on color alone. Existing callers
that don't pass scopeName (live.js) are unaffected.

QueryGroupedPackets (SQLite + in-memory) didn't select scope_name at
all — added it, since the Packets tab defaults to the grouped view.
The transport badge previously only surfaced a known scope in the
title tooltip, requiring a hover. Now the label itself reads
"T·#region" so it's visible at a glance, matching the "T?" unknown
case which was already inline. Badge gets a max-width + ellipsis so
long region names (e.g. "#dk-trekantsomraadet") don't blow out the
Type column — full name stays in the title.
Adds configuredRegions/unusedRegions to /api/scope-stats: an all-time
(not window-scoped) diff between the operator's configured hashRegions
list and the set of scope_name values that have actually matched a
transmission still in retention. Surfaces how much of the region list
is dead weight — directly actionable evidence for pruning, which is
also the real fix for the HMAC-collision noise (fewer configured
regions -> lower birthday-collision probability per packet).

Server config now parses hashRegions (previously ingestor-only, same
config.json key) purely to read the configured names — no HMAC key
derivation happens server-side.

Frontend: a "Region Utilization" section on the Scopes tab shows
used/unused counts and a collapsible list of the unused region names.
Adds repeatersByRegion to /api/scope-stats: for every region that has
ever matched a transmission, which distinct repeaters/rooms have
relayed traffic carrying that scope. Sourced from the same 5-min
background-recomputed bulk relay-info cache the Nodes page already
uses (GetRepeaterRelayInfoMap / TransportedScopes, Kpa-clawbot#1751) — no new
expensive computation, just an inversion + name lookup.

Frontend renders a collapsible per-region repeater list (name links
to the node detail page) under a new "Repeaters by Region" section,
explicitly framed as a coverage/redundancy signal: a region carried
by only one repeater is a single point of failure for that area.
byPathHop indexes both full pubkeys and short hex-prefix "bucket" keys
used internally for ambiguous-hop resolution — GetRepeaterRelayInfoMap
returns TransportedScopes for every one of those keys indiscriminately.
The first deploy of repeatersByRegion iterated the raw map and fell
back to showing the bucket key itself when no name matched, so a
handful of short internal keys were counted as "repeaters" (stg showed
594 "repeaters" for #dk — every active repeater plus every 2-6 char
bucket key that ever touched a #dk packet).

GetNodeNamesByKeys is now GetRepeaterNamesByKeys and filters
`role IN ('repeater','room')` in the SQL itself, so a key only survives
if it's a real node — bucket keys never match a nodes.public_key row
and are dropped rather than falling back to the raw key.
Adds originatingNodesByRegion to /api/scope-stats: nodes whose OWN
default_scope (Kpa-clawbot#899) is a given region, complementing the existing
repeatersByRegion (transported_scopes) breakdown. The distinction
matters — a repeater can relay traffic for a region it isn't itself
configured with, so "who runs this region" and "who has carried this
region's traffic" are different, both useful questions.

Frontend: refactored the per-region collapsible-list rendering (used
by both breakdowns) into a shared renderRegionNodeGroups() helper
instead of duplicating the HTML-building logic, and added a "Nodes
Running This Region" section alongside "Repeaters by Region".
Root-caused via a real report: "Repeaters by Region" showed a hyper-
local scope (#dk-fyn-middelfart) as transported by repeaters spread
across the whole country, which shouldn't be possible — MeshCore
firmware only relays a TRANSPORT_FLOOD/DIRECT packet when the
repeater's OWN configured region matches the packet's transport code
(examples/simple_repeater/MyMesh.cpp allowPacketForward, gated by
RegionMap::findMatch against the repeater's local region list).
Confirmed against the meshcore-dev/MeshCore source.

The bug: both TransportedScopes computation paths (bulk
computeRepeaterRelayInfoMap and per-node GetRepeaterRelayInfo) fold a
full pubkey's byPathHop entries together with its matching 1-byte
raw-prefix bucket — an intentional, existing fallback for RelayCount/
LastRelayed ("this node is probably active") that tolerates the
1-byte hash's inherent ambiguity (any node sharing that first byte
gets folded in). Applying the SAME fold to TransportedScopes asserted
something far more specific than the ambiguous signal can support,
and something the protocol itself wouldn't allow.

Scope accumulation now only happens on the exact-key (resolved,
unambiguous) pass; RelayCount/LastRelayed/RelayActive keep the
prefix-bucket fold unchanged, since those remain intentionally
approximate. Added relayEntry.fromPrefix to carry this distinction
through the per-node path, and rewrote the test that had pinned the
old (incorrect) folding behavior.
# Conflicts:
#	public/packets.js
@Kpa-clawbot

Copy link
Copy Markdown
Owner

Review — PR #1852: Region-scope visibility

Verdict: solid PR. Two real correctness fixes with excellent test vectors, clean frontend surfacing. Findings below.

MAJOR

  1. rows.Scan error silently swalloweddb.go:1909 (GetChannelMessages): rows.Scan(scanArgs...) return value is unchecked (no if err :=). A schema mismatch (e.g. hasScopeName true but column dropped) silently produces zero-valued messages instead of failing visibly. The grouped-packets path at :640 has the same pattern (continue on error) but at least checks err. — (carmack)

MINOR

  1. normalizeRegionNames duplicates ingestor logic without sharingroutes.go:3456 re-implements the #-prefix + dedup logic from loadRegionKeys in cmd/ingestor/main.go. Divergence risk if one changes without the other. Consider extracting to a shared internal/regions package. — (dijkstra)

  2. CSS vertical-align: middle duplicatedstyle.css:1429 and :1435 both set vertical-align: middle on .badge-transport. The second declaration shadows the first with the same value. — (tufte)

  3. GetRepeaterNamesByKeys unbounded IN clausedb.go:2174: large deployments with thousands of repeaters build an unbounded IN (?,?,?...) list. SQLite handles this fine up to SQLITE_MAX_VARIABLE_NUMBER (default 999, raised in modern builds), but consider batching for safety. — (munger)

NIT

  1. ChannelMessageResp.Scope typed as interface{}types.go:937: *string would be more precise and avoid type-assertion surprises downstream. Same for RouteType*int. — (kent-beck)

  2. renderRegionNodeGroups inline stylesanalytics.js:4665-4680: margin/font-size via inline style= instead of CSS classes. Minor consistency gap with the rest of the file's pattern. — (tufte)

Overall: well-reasoned correctness fixes grounded in real collision data and firmware source. Test coverage is thorough — the ambiguous-collision regression test with real captured keys is exemplary. 0 BLOCKERs.

dborup added 2 commits July 18, 2026 12:04
Adds channelMessages to /api/scope-stats: the same scoped/unscoped/
unknown question as the main Summary, but restricted to payload_type=5
(channel chat) instead of all observed traffic. Most channel chat is
plain FLOOD rather than transport-scoped, so this can read very
differently from the all-traffic numbers — answers "how many of our
actual channel messages carry a region scope" directly instead of
requiring the reader to infer it from the broader stats.

New GetChannelMessageScopeStats() mirrors GetScopeStats' query shape
but scopes TotalMessages to ALL route types for payload_type=5 (not
just route_type 0/3), since restricting to transport routes would
answer a different question than "how many channel messages, period".

Frontend renders a small "Channel Messages" stat-card row under the
main summary cards, window-scoped like the rest of the tab.
Adds bridgeRepeaters to /api/scope-stats: RepeatersByRegion inverted
into pubkey -> regions, keeping only repeaters that have relayed
traffic for MORE than one region. These are the mesh's literal
backbone nodes connecting otherwise-separate regional communities —
losing one is a more consequential failure than losing a
single-region repeater.

Computed inline while building RepeatersByRegion (reuses the same
byRegion map and role-filtered names lookup, no extra queries).

Frontend renders a small table under "Repeaters by Region": repeater
name (linked to its node detail page), region count, and the region
list.
@Kpa-clawbot

Copy link
Copy Markdown
Owner

Review — PR #1852 (follow-up on delta since 40dc297)

New commits: c74a0052 (channel-messages breakdown), 1aed3ee5 (bridge-repeaters). +302 LOC across 5 files, 2 new focused table-driven tests. Clean, minimal, mirrors established patterns (RepeatersByRegion, GetScopeStats). No new SQL hot loops, no schema churn.

verdict: LGTM — merge-ready pending CI.

Findings

munger (backend, MINOR): handleScopeStats swallows GetChannelMessageScopeStats error silently (if chanStats, err := …; err == nil { … }). Same shape as existing sibling calls in the handler, but a hasScopeName=false server or a bad window will return channelMessages unset with no log breadcrumb. Consistent with existing surrounding code — non-blocking.

kent-beck (tests, LGTM): TestHandleScopeStats_ChannelMessagesExcludesOtherPayloadTypes isolates the payload_type=5 filter with a payload_type=4 ADVERT foil — precise and TDD-shaped. TestHandleScopeStats_BridgeRepeaters uses two nodes (bridge + single-region) as a discriminator, asserts region order deterministically, and includes an explicit negative check that SingleRegionNode is excluded. Both tests would fail on revert of the accompanying code. No tautology.

carmack (backend, LGTM): BridgeRepeaters inversion is O(regions × pks) piggybacking on the already-built byRegion map + names filter — no extra DB queries. Deterministic sort (count desc, name asc). sort.Strings(regions) inside the loop is cheap for expected small N.

dijkstra (correctness, LGTM): ChannelScopeStats.TotalMessages = transportTotal + nonTransportCount matches the doc-string ("ALL route types"). Unscoped += nonTransportCount fold is consistent with #1838's protocol reasoning (non-transport routes never carry a scope). Test coverage confirms both numerator and denominator behavior.

Auto-merge gate

  • mergeable: MERGEABLE
  • mergeStateStatus: BLOCKED (no CI checks reported yet on this head)
  • Polish: 0 BLOCKER / 1 MINOR
  • Tests: present ✓

Not merging — CI hasn't run/completed against 1aed3ee5. Author is community; if CI turns green and stays clean, safe to squash-merge.

dborup added 3 commits July 18, 2026 13:08
Repeaters by Region, Nodes Running This Region, and Bridge Repeaters
all cleared their entire section (heading included) when the data was
empty, which reads as "this feature doesn't exist" rather than "no
data yet" — especially confusing right after a restart, when the
neighbor graph and path resolution need a few minutes to catch back
up before repeater-level scope data repopulates. Now always renders
the heading + description with an explanatory empty-state message
instead of hiding.
Adds hourlyActivityByRegion to /api/scope-stats: each region's message
counts bucketed by hour-of-day (0-23 UTC), aggregated across every day
in the window — answers "when during a typical day is this region
active" rather than "how did volume change over the window" (that's
the existing chronological TimeSeries chart).

Frontend renders a compact heatmap: one row per region, 24 hour
columns, color intensity normalized per-row (each region's own busiest
hour) so a quiet region's daily shape stays visible next to a loud
one instead of being crushed toward zero.
Adds channelScopeAdoption to /api/scope-stats: the existing
ChannelMessages aggregate (scoped/unscoped/unknown for channel chat)
broken down PER CHANNEL — which specific channels (#test,
#wardriving, ...) actually use region scoping vs which never do.
Ordered by message volume, capped at the top 30 channels.

Frontend renders a compact table under "Channel Messages": channel
name, total messages, scoped count+%, unknown count.
@Kpa-clawbot

Copy link
Copy Markdown
Owner

Review — PR #1852 (delta since 1aed3ee)

New commits: 4f06102 (empty-state headings), 03abeba (hour-of-day heatmap), 064a9c8 (per-channel scope adoption).

Verdict: ship it. Clean, well-tested, no blockers.

Findings

  • (kent-beck, MINOR) TestHandleScopeStats_HourlyActivityByRegion handles the hour1==hour2 rollover edge case explicitly — nice. Both new endpoints land with red-first shaped table-driven tests. cmd/server/routes_test.go:4263-4405.
  • (carmack, MINOR) GetChannelScopeAdoption derives Unscoped = Total - Scoped - UnknownScope, but Total (COUNT(*)) includes rows outside route_type IN (0,3), while Scoped/UnknownScope sums are gated by that filter. Result: any non-transport payload_type=5 row silently lands in the "Unscoped" bucket. Matches existing ChannelMessageScopeStats shape so not a regression, but worth a _comment_-style code note if you want future readers to not re-derive it. cmd/server/db.go:3061-3110.
  • (tufte, MINOR) Adoption table renders Messages | Scoped | Unknown, silently omitting the Unscoped field the type carries. Either drop unscoped from the JSON or add the fourth column so the numbers reconcile without mental math. public/analytics.js:4590-4620.
  • (tufte, GOOD) Per-row normalization on the hour-of-day heatmap is the right call and it's explained inline — a quiet region's shape survives next to a loud one.
  • (dijkstra, GOOD) Empty-state headings + guidance strings (4f06102) are a real UX fix; "feature is missing" vs "no data yet after restart" is exactly the confusion this removes.

Non-blocking. Community PR — comment only per bot policy.

dborup and others added 3 commits July 18, 2026 14:33
channelKeys and regionKeys (derived from config.json's hashChannels/
hashRegions) were loaded exactly once in main() and held in plain
maps for the process lifetime — adding a new hashtag channel or
region required a full ingestor restart to take effect, which also
resets the in-memory relay/scope-attribution state built up since the
last restart (see the Repeaters-by-Region cold-start discussion).

Wraps both maps in atomic.Pointer-backed hotKeys (same pattern already
used for Config.NodeBlacklist, just actually wired to a trigger this
time) and adds a SIGHUP handler that re-reads config.json and
atomically swaps in freshly-derived keys:

    kill -HUP <ingestor-pid>

On a malformed config during reload, the previous keys are kept and
the error is logged — a bad edit must not blank out a working
ingestor. The MQTT message-handler closure now pulls a fresh snapshot
from the atomic pointers per message instead of closing over a static
map, so no other call sites needed to change.
Brings in a pending restructuring of the deployment doc, and adds a
section on reloading hashChannels/hashRegions via SIGHUP instead of a
full ingestor restart, which would otherwise reset in-memory analytics
state (Repeaters-by-Region, Bridge Repeaters, etc).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Git tracked docs/DEPLOYMENT.md and docs/deployment.md as two separate
blobs. On case-insensitive filesystems (macOS/Windows) only one can
exist on disk, so the uppercase entry silently went stale while the
lowercase one kept receiving edits — a case-sensitive checkout (e.g.
Linux CI) would have gotten both, one of them outdated. Keep only
docs/deployment.md, the one actually maintained.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@Kpa-clawbot

Copy link
Copy Markdown
Owner

Review — PR #1852 (delta since 064a9c8)

carmack: atomic.Pointer[map[...]] swap is correct — maps returned by loadChannelKeys/loadRegionKeys are freshly allocated and never mutated after Store, so readers holding a snapshot (hk.Channels()) are safe (cmd/ingestor/hot_reload.go:32-50). SIGHUP goroutine exit path (signal.Stop + close(done)) is race-free.

kent-beck: Three real tests, all fail without the implementation — TestHotKeys_ReloadPicksUpNewChannelsAndRegions, TestHotKeys_ReloadKeepsPreviousKeysOnParseError (asserts error + len unchanged), and TestStartSIGHUPReload_ActuallyReloadsOnSignal delivers a real SIGHUP to os.Getpid(). No tautologies. Malformed-config fallback and end-to-end signal delivery both covered.

munger: SIGHUP mid-message is benign — ingestBuffer.Submit captures the key snapshot at submit-time (main.go:190), so an in-flight handler uses the keys it was scheduled with; next submission sees the new snapshot. Docs match code: [hot-reload] log lines, fallback-on-error behavior, and the "only hashChannels/hashRegions" scope all accurately described in docs/deployment.md:187-207.

dijkstra: Pointer-swap is single atomic.Store, no torn read possible. No TOCTOU on config read — reload path is LoadConfig → derive maps → two independent stores; a reader between the two stores sees new channels + old regions transiently, which is acceptable (both fields evolve independently). Minor: loadChannelKeys/loadRegionKeys log-and-continue on per-key errors, so a partial reload can swap in a smaller map; documenting this trade-off would help operators, but not a blocker.

Verdict: polish-clean; awaiting operator merge decision (community PR).

dborup and others added 3 commits July 18, 2026 15:20
Region-scope was only visible as a small inline pill inside the Type
column (transportBadge). Split it into its own sortable "Scope" column
between Type and Observer, matching the same three states (region
name / unknown / not applicable for non-transport packets). Wired
into the existing column visibility toggle and mobile narrow-width
hide list.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Packets column-visibility toggle persists an explicit array of
visible column keys to localStorage. A returning visitor with a
pre-existing saved array (from before the Scope column existed) never
got it shown, since the load path used the saved array verbatim
instead of reconciling it against the current COL_DEFS. Missing keys
now default to visible (unless narrow-viewport-hidden), same as a
fresh visitor gets.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cmd/server/prune-requests/ is internal/prunequeue's on-disk job queue,
written next to the DB path at runtime — not source, shouldn't be
tracked.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@Kpa-clawbot

Copy link
Copy Markdown
Owner

Review — PR #1852 (delta since 18bc6fb7)

New commits: e4593e98 (Scope column), bcbcae1d (prefs backfill), 557d14e5 (gitignore).

tufte: Scope column is well-scoped (public/packets.js:2276,2311,2348). scopeCellHtml mirrors transportBadge's three-state semantics (dash / muted "unknown" / resolved name) and is escapeHtml-safe (public/app.js:50-54). .col-scope hidden on ≤640px (public/style.css:3009) — right call for a narrow-viewport secondary column. data-priority="4" matches the redundancy with the inline T-badge on Type.

carmack: Sort accessor added (public/packets.js:2710). Prefs-backfill loop (public/packets.js:2029-2035) mutates visibleCols in place but does not persist — next user-triggered save writes it back; benign but worth noting. .gitignore entry for cmd/server/prune-requests/ is a runtime queue dir, harmless.

kent-beck (MAJOR): Zero tests in this delta. Two changes need coverage under AGENTS.md § TDD:

  1. scopeCellHtml — net-new UI. Exemption allows test in same PR; none landed. DOM-grep against a static fixture would suffice.
  2. bcbcae1d prefs-backfill — this is a bug fix on existing UI (localStorage-persisted col visibility failing to surface new columns), which explicitly requires red→green (not exempt). Add a test that seeds localStorage['packets-visible-cols'] with a legacy set and asserts scope is added after init.

munger: Second-order: any future column added after bcbcae1d inherits the backfill for free — good. But defaultHidden for narrow viewports (packets.js:2023) is checked against the backfill loop's exclusion; adding a column that should be hidden on mobile still requires updating both lists. Not a blocker; document the pattern in a comment.

Verdict: MAJOR (missing prefs-backfill test). Community PR — leaving to operator. CI has not reported on 557d14e5 yet; not auto-mergeable.

dborup and others added 2 commits July 18, 2026 18:48
Per bot review on PR Kpa-clawbot#1852 (comment 5011643190): two changes in the
recent delta had zero test coverage, which the repo's TDD policy
(AGENTS.md § Test-First Development) requires for both net-new UI
helpers and bug fixes on existing UI.

- scopeCellHtml (public/app.js): unit tests for all three states
  (non-transport dash, resolved region, unresolved "unknown") plus an
  XSS-escaping check.
- Column-prefs backfill (public/packets.js): extracted the inline
  reconciliation logic into a standalone reconcileVisibleCols(),
  exposed via _packetsTestAPI like the rest of packets.js's pure
  helpers, and unit tested directly instead of only through the DOM.

Writing the test surfaced a real bug in the original backfill: it
couldn't distinguish "column didn't exist yet" from "user explicitly
unchecked this column" — both look identical as a missing key in the
saved array, so any existing column a user had hidden (e.g. Observer)
would get silently resurrected on next page load. Fixed by persisting
a second "known columns" baseline (packets-visible-cols-known)
alongside the visibility array: a missing key only gets backfilled as
newly-visible if it's also absent from the known-columns baseline: a
pre-existing key missing from the saved array is a deliberate user
choice and stays hidden. Falls back to the pre-Kpa-clawbot#1852 column list for
visitors whose saved prefs predate persisting the baseline at all.

Also registers scopeCellHtml in .eslintrc.json's cross-file globals
list (same treatment as transportBadge) — packets.js referencing it
was tripping no-undef since app.js and packets.js are separate
non-module scripts.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Working through the outstanding non-blocker findings across all five
automated review passes on the PR:

- MAJOR: GetChannelMessages' rows.Scan() error was silently discarded
  — a schema mismatch would produce zero-valued messages instead of a
  visible error. Now returns the error, matching sibling query loops.
- Extracted the hashRegions name-normalization rule (trim, "#"-prefix,
  dedupe) shared between cmd/ingestor's loadRegionKeys and cmd/server's
  region-utilization diff into a new internal/regions package, so the
  two can no longer drift apart on the rule independently.
- Batched GetRepeaterNamesByKeys' SQL IN (...) clause in chunks of 500
  — an unbounded clause risks SQLITE_MAX_VARIABLE_NUMBER on very large
  deployments' byPathHop candidate sets.
- handleScopeStats' remaining silently-swallowed err==nil branches
  (GetMatchedRegionNames, GetNodesByDefaultScope,
  GetChannelMessageScopeStats, GetChannelScopeAdoption) now log a WARN
  breadcrumb on failure instead of failing invisibly.
- Scope Adoption table was rendering 3 of the 4 ChannelScopeAdoption
  fields (Unscoped omitted) — the visible numbers didn't reconcile to
  the message total without doing the subtraction by hand. Added the
  column.
- Removed a duplicate `vertical-align: middle` declaration on
  .badge-transport (dead, not a behavior change).
- Deleted ChannelMessageResp — flagged for interface{} vs *string/*int
  typing, but turned out to be completely unused dead code; removing
  it resolves the finding more directly than retyping something
  nothing constructs.

Left two NIT/MINOR items as-is with reasoning:
- renderRegionNodeGroups' inline styles match the same pattern used in
  15+ other places in analytics.js — "fixing" only this one function
  would make it less consistent with the file, not more.
- TransmissionResp's interface{} fields (a different struct than the
  one just removed) are an established, actively-used pattern for
  nullable SQL-scanned values across that whole response type;
  retyping it is a much larger, unrelated refactor.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
dborup and others added 9 commits July 26, 2026 12:25
Found while live-testing Kpa-clawbot#1867 on stg: the server and ingestor are
separate processes started ~simultaneously by supervisor, sharing one
SQLite file. The server's PRAGMA-based column detection ran exactly
once at OpenDB(), and could fire before the ingestor's additive ALTER
TABLE migrations landed -- reproduced on a fresh stg deploy, where
configured_scope silently never appeared in the API until the
container was manually restarted.

detectSchemaWithRetry now re-scans a few more times on a short fixed
schedule after the first pass. detectSchema's booleans are monotonic
(never reset to false once found), so repeated calls merge safely.
Deliberately does not stop early on two agreeing scans -- a migration
landing between two poll points would make an early scan look stable
right before the real change arrives.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…routine master sync

Routine "sync branch with master before deploying" (git merge origin/master
into areas-meshguide-sync) fast-forwarded cleanly and, in doing so, silently
carried master's exclusion of this deployment-specific script back into
areas-meshguide-sync too -- fast-forward doesn't distinguish "master never
had this" from "master deliberately deletes this", so the branch meant to be
this script's home lost it entirely instead of just staying ahead of master.

Restored verbatim from the last commit that had it (e81f89d), including the
matching AREAS.md section describing it. Master must never carry this file;
areas-meshguide-sync must never lose it to a master sync again.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t the app reads)

Caught live on stg: after running the script for real, the one genuinely
new area it added (SE12) never showed up as scoped via /api/config/areas,
while all 14 "enriched" existing areas looked fine. Turned out those 14
already had a correct regionScopes array from earlier manual config work --
the script's regionScope (singular) write was dead data riding along
next to it. SE12 had no prior regionScopes, so the script's only write
landed on a field cmd/server/routes.go's handleConfigAreas never reads,
leaving it silently unlinked.

Now appends to regionScopes (preserving any other scopes already on the
area) and drops a stray regionScope key if a prior buggy run left one.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… key too

The previous fix only stopped the script from writing the wrong field
going forward. SE12 -- the one area actually created by today's buggy
run -- falls through both remaining code paths: it's not in CROSSWALK
(so the enrich step never touches it) and it already exists (so the
"add new area" step skips it as already present). Without an explicit
migration pass it would stay silently unlinked forever.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
New feature: every "ping" channel message gets recorded and scored,
producing records (farthest reach, most hops, widest simultaneous
spread, fastest full spread, most airtime-efficient) plus leaderboards
(top relay nodes, top first-hearer observers). Global, not scoped by
region/area, per dborup's request.

Architecture:
- Ingestor writes a lightweight ping_triggers row (tx_id, hash,
  channel_hash, sender, first_seen) at ingest time when a new CHAN
  transmission matches the same isPingTrigger logic the pong-reply
  feature already uses (mirrored, kept in sync by hand -- now a 3rd
  copy alongside public/channels.js and cmd/server/db.go).
- Server periodically (every 2 min) joins ping_triggers with the same
  GetPacketPath + LoRa-airtime-estimate logic behind View Path,
  computing the full snapshot in memory (no write access needed from
  the read-only server) and caching it, matching the steady-state
  recomputer pattern used throughout cmd/server.
- New GET /api/ping-scores endpoint + Ping Scores page (linked from
  Tools and from every pong reply bubble in Channels).

Schema change follows the dbschema.go INVARIANT from Kpa-clawbot#1321 exactly:
migration in Apply() (ingestor-only), assertion in AssertReady()
(server fatal-exits if ping_triggers is missing rather than silently
degrading) -- the same pattern Kpa-clawbot#1865/Kpa-clawbot#1867's configured_scope should
have used but didn't fully wire up (see the ping_scores.go doc
comment and today's earlier startup-race fix for the story).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Requested by dborup after seeing the highscore board start empty on
deploy. A new async migration (ping_triggers_backfill_v1, following the
existing tx_last_seen_backfill_v1 pattern) scans payload_type=5
transmissions for the ping trigger once, catching CHAN messages sent
before this feature existed -- they never went through
InsertTransmission's isNew detection hook, since that only fires for
brand-new transmissions going forward.

Pulled the migration body into a named backfillPingTriggers function
(unlike its inline siblings) so tests can call it directly instead of
faking the async marker/goroutine machinery.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two dborup-specific, already-used pieces held back from the generic
master branch:

- ops/meshguide-sync/sync_areas.py: fetches Danish region boundaries
  from meshguide.dk, useless for any other CoreScope deployment. This
  fast-forward merge brought it in again (areas-meshguide-sync had it
  restored after an earlier accidental deletion); excluding it here,
  same as every previous merge round.
- cmd/ingestor/db.go's ping_triggers_backfill_v1 async migration: a
  one-time historical scan that already ran and completed on stg
  (298 pings recorded from before the feature existed). Per dborup:
  other deployments merging from master didn't ask for a full-table
  CHAN-message scan on their own database. The ping-scores feature
  itself (detection, scoring, leaderboards, frontend) stays -- only
  the backfill-specific function + its tests are removed.

Both pieces remain intact on areas-meshguide-sync, where they've
already served their purpose.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cwichura flagged on PR Kpa-clawbot#1867 that nodes.configured_scope (from the
/neighbors OTA report) was stored as the observer's raw scope string
("dk") while default_scope and every other scope display use the
#-prefixed hashRegion form ("#dk"). Apply the same regions.Normalize
already used for default_scope, with "*" passed through unprefixed
since it's a protocol wildcard, not a region name.
dborup and others added 20 commits July 26, 2026 17:19
…-wins

SaarMesh-Bot flagged on Kpa-clawbot#1865 (PR Kpa-clawbot#1867 commit 631686a) that
UpdateNodeConfiguredScope stored the report timestamp raw and compared it
lexicographically. Real firmware emits microsecond+offset timestamps and
different observers may use "Z" or non-UTC offsets, so string comparison
can diverge from chronological order and let a stale report win.

normalizeReportTS parses RFC3339Nano/RFC3339 and stores canonical UTC
RFC3339, used for both storage and the comparison.
…clawbot#1865)

cwichura asked on PR Kpa-clawbot#1867 for a way to identify which observers have the
opt-in /neighbors firmware feature enabled, to help contact operators about
enabling it -- explicitly asking that the UI not "shame" observers that
lack it (non-PSRAM hardware can't send it; other MQTT uploaders haven't
been updated to include it).

Adds observers.last_neighbors_report_at, touched by the ingestor's new
TouchObserverNeighborsReport once per /neighbors MQTT message regardless
of whether any neighbor carried scope evidence. Added to dbschema's
AssertReady mustCol list from day one so the server can read it
unconditionally with no PRAGMA-detection race (Kpa-clawbot#1321 pattern).

Surfaced as a sortable "Neighbors" column on the observers list and a
stat card on the observer detail page -- both render a neutral dash/
"never" with an explanatory tooltip when absent, never a warning icon.
The Observer DB struct carried the field correctly, but handleObservers/
handleObserverDetail build a separate field-by-field ObserverResp DTO for
the actual JSON response, which never picked it up -- caught live on stg
(field missing from /api/observers entirely). Added handler-level tests
that decode the JSON response, since the existing DB-layer test for this
class of field didn't reach the DTO conversion at all.
…low-up)

Surfaces the observer's own firmware-reported zero-hop neighbor set --
ground truth from /neighbors, distinct from the packet-path-inferred
neighbor_edges graph. Requested by dborup after shipping the last-report
tracking: "should we show what neighbors it has under observer stat".

New observer_neighbors table (current-only snapshot, full replace on every
report so a dropped neighbor disappears rather than lingering). Guarded
against out-of-order reports by checking the report timestamp against the
already-touched observers.last_neighbors_report_at before replacing.

GET /api/observers/{id}/neighbors joins against nodes for name/role;
absence (never reported, or an unresolved pubkey) always renders neutrally
-- empty array not null, no warning icon -- consistent with the last
feature's no-shame requirement.
dborup's suggested "more ambitious use": flag firmware-confirmed neighbors
that our packet-path-inferred neighbor_edges graph has never seen adjacent
-- a diagnostic for coverage gaps / packet loss, not itself a fault.

GetObserverNeighbors now annotates each entry with seenViaPackets, checked
against neighbor_edges in both column directions (canonEdge orders
node_a<=node_b). Surfaced as a neutral "confirmed"/"not seen yet" column
on the Direct Neighbors panel, explaining the two possible causes in the
tooltip. This surfaces the mismatch only -- it does not yet feed the
path-hop disambiguator's own candidate scoring, which would be a separate,
larger change.
…1865 follow-up)

dborup spotted the raw /neighbors payload also carries snr and
heard_secs_ago per neighbor -- previously dropped entirely by
handleNeighborsReport's parsing loop. Both fields are present regardless
of scope-query status (responded or timeout).

New append-only observer_neighbor_metrics table, inspired by the existing
RF Health tab's observer_metrics pattern: one row per neighbor per
/neighbors report, pruned on the same 30-day MetricsRetentionDays
schedule. Deliberately no ordering guard on the write side (unlike the
current-only observer_neighbors snapshot) -- every report is valid
history at its own timestamp.

GET /api/observers/{id}/neighbors/{pubkey}/metrics serves the raw
(undownsampled) history. Frontend renders a per-row SNR sparkline on the
Direct Neighbors panel using the same lightweight inline-SVG technique as
the RF Health grid's noise-floor sparklines (no Chart.js), loaded async
after the panel paints.
dborup: "det ville være cool wih more history... jeg vil bare gerne se
mere" -- not more retention, more detail in the existing view.

- neighborSnrSparkline now labels min/max dB directly on the sparkline,
  readable without hovering.
- Clicking a sparkline (any row with data) opens a modal with a bigger
  Chart.js line chart showing both SNR and heard_secs_ago on dual y-axes,
  with native hover tooltips. Reuses the existing generic .modal-overlay/
  .modal CSS (previously only exercised by the channel-add modal).
- Metrics are cached per-pubkey when the sparkline loads, so opening the
  modal needs no second fetch.
- Click delegation is wired once at module load (not per-render), since
  renderDetail rewrites #obsDetailContent's innerHTML on every load.
borderColor was set to raw 'var(--accent)'/'var(--status-yellow)'
strings, which Canvas 2D (unlike SVG/DOM) can't resolve as a strokeStyle
-- Chart.js silently fell back to its default black for both lines,
making the SNR and Heard(s ago) series indistinguishable (spotted live
on stg). Switched to this page's existing CHART_COLORS literal-hex
palette, matching the pattern the other 4 charts on this page already
use. The small inline SVG sparkline was unaffected -- SVG is real DOM
and does resolve CSS custom properties, just Canvas doesn't.
SaarMesh-Bot flagged on PR Kpa-clawbot#1867 that the server's one-time detectSchema()
PRAGMA snapshot racing the ingestor's ALTER TABLE migrations isn't specific
to configured_scope -- it's a general property of every optional column,
and offered to send a fix. Built it ourselves instead.

hasResolvedPath/hasObsRawHex/hasScopeName/hasDefaultScope/
hasConfiguredScope/hasMultibyteSupCols/hasLastSeen are now schemaFlag
(atomic.Bool) methods instead of plain bools, self-healed by a background
ticker (healSchemaFlags) started once in OpenDB and stopped in Close().
Unlike an eager reprobe-on-read, the healer never nests inside another
caller's already-open *sql.Rows cursor, so it can't self-deadlock a
single-connection pool the way a naive "reprobe inside get()" version of
this fix did (caught immediately by the existing test suite hanging).
detectSchemaWithRetry's fixed 150ms budget is gone -- replaced by an
unbounded self-heal that catches the migration whenever it actually lands.

Every read site across db.go/store.go/chunked_load.go/main.go is now a
method call; test fixtures that previously force-set the old bool fields
now call .forceTrue() on the underlying schemaFlag.
… position-fix coverage gaps

New GET /api/analytics/areas endpoint and "Areas" analytics tab, built on
the configured drawn-polygon Areas (meshguide.dk sync), distinct from
hashRegion scope adoption:

- Density: node count + active/degraded/silent health + role mix per
  area, multi-membership via AreaKeysForPoint (a node in a sub-area also
  rolls up into its parent region).
- Bridge nodes: which nodes' packet-derived neighbor_edges reach into
  another area, ranked by how many other areas they touch -- distinct
  from the network-wide, area-unaware bridge_score.
- Position gaps: per area, real GPS fix vs. neighbor-centroid-estimated
  position, reusing nearestPositionedNeighbor (the same technique View
  Path's approx markers use) purely as an internal coverage signal.

30s TTL cache on the handler since position-gap computation calls
nearestPositionedNeighbor once per unpositioned node.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ort worst-first, collapse to top 10

dborup: the full 36-row dump buried the handful of areas with an actual
gap behind dozens already fully GPS-mapped (0% estimated). Sort by %
estimated descending and collapse to top 10 with a "Show all" toggle,
matching the existing pattern from the Wardriving tab's Top Senders /
Coverage by Observer sections.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ections in Areas tab

dborup asked to apply the same treatment given to Position-Fix Coverage
Gaps to the other two Areas sections. Node Density & Health now sorts
worst-health-first (% degraded+silent descending) instead of the API's
raw Total-desc order, so small struggling areas surface instead of
being buried under Europa/Danmark. Cross-Area Bridge Nodes keeps its
existing otherAreaCount-desc order (already most-interesting-first) and
just gets the same collapse-to-10 treatment. Factored the three
sections' collapsible-table rendering/wiring into shared helpers.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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