From 59cf5130d19ad7055d1c2effa684b59e719e99a3 Mon Sep 17 00:00:00 2001 From: Kpa-clawbot Date: Thu, 9 Jul 2026 22:56:29 -0700 Subject: [PATCH] fix(1838): fold non-transport routes into scope-stats Unscoped (#1842) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1838 ## Problem `/api/scope-stats` reported 100% scoped whenever any region was configured. Reporter noticed on a scopeless instance that "unscoped" was always zero — the pie visual is misleading to operators deciding on `denyf *`. ## Root cause `cmd/server/db.go:22` restricted the entire scope-stats denominator to `route_type IN (0, 3)`. Per firmware `docs/packet_format.md § Route Types`: - `0` = `TRANSPORT_FLOOD` - `1` = `FLOOD` - `2` = `DIRECT` - `3` = `TRANSPORT_DIRECT` Only routes 0 and 3 carry `transport_code_1` (transport-level scope). Routes 1 and 2 are inherently unscoped by protocol. The existing SQL was correct for the "how many transport-scopable routes are actually scoped" question, but the denominator was silently promoted to "all traffic" in the UI. Bonus: the comment on `routeTypeTransportSQL` labelled routes 0+3 as "FLOOD (0) and DIRECT (3)" — wrong on both counts. ## Fix - `cmd/server/db.go` — corrected the `routeTypeTransportSQL` comment; added `routeTypeNonTransportSQL = "route_type IN (1, 2)"` alongside it. - `GetScopeStats` runs a second `COUNT(*)` over `route_type IN (1,2) AND first_seen >= ?` and folds that count into `Summary.Unscoped`. Same index path as the existing query — one extra scan per `/api/scope-stats` call (cached 30s per triage's carmack finding). - `public/analytics.js` — Scopes tab header explains the denominator (all observed transmissions) and which route types carry scope. Card notes now render `X% of all traffic` for Scoped/Unscoped and `X% of scoped` for Unknown Scope so the pie's denominator is explicit. ## TDD - Red: `5554ffe4` — extended `TestGetScopeStats` + `TestHandleScopeStats` with `route_type=1` and `route_type=2` rows and asserted `Unscoped = 3` (1 transport-NULL + 2 non-transport). Ran the tests and confirmed assertion failure (`Unscoped = 1, want 3`). - Green: `ebbb9253` — implementation + label copy. Full `go test ./cmd/server/...` passes (54s). ## Preflight overrides - check-branch-clean: justified — cross-stack fix by design (backend semantics change + matching frontend label copy). All 4 files are exactly the surface the triage comment identified. ## Verification - `go test ./cmd/server/...` — 54s, all pass. - Firmware confirmation: `firmware/docs/packet_format.md:20-24` (route type table). ## Files touched - `cmd/server/db.go` — comment fix + second COUNT query. - `cmd/server/db_test.go` — extended fixture. - `cmd/server/routes_test.go` — extended fixture + isolate from seed data. - `public/analytics.js` — labels and header copy. --------- Co-authored-by: corescope-bot --- cmd/server/db.go | 26 ++++++++++++++++++++++++-- cmd/server/db_test.go | 11 ++++++++--- cmd/server/routes_test.go | 11 +++++++++-- public/analytics.js | 15 +++++++++++---- 4 files changed, 52 insertions(+), 11 deletions(-) diff --git a/cmd/server/db.go b/cmd/server/db.go index c553b8e9..f2693894 100644 --- a/cmd/server/db.go +++ b/cmd/server/db.go @@ -17,10 +17,19 @@ import ( _ "modernc.org/sqlite" ) -// routeTypeTransport covers FLOOD (0) and DIRECT (3) route types — packets -// that carry transport-level scoping via Code1. +// routeTypeTransport covers TRANSPORT_FLOOD (0) and TRANSPORT_DIRECT (3) — +// the only route types that carry transport_code_1 (transport-level scope). +// Per firmware/docs/packet_format.md § Route Types: +// 0 = TRANSPORT_FLOOD, 1 = FLOOD, 2 = DIRECT, 3 = TRANSPORT_DIRECT. +// Routes 1 (FLOOD) and 2 (DIRECT) never carry a scope by protocol — they are +// inherently unscoped and are counted separately in GetScopeStats (#1838). const routeTypeTransportSQL = "route_type IN (0, 3)" +// routeTypeNonTransportSQL matches FLOOD (1) and DIRECT (2) — non-transport +// routes that carry no transport_code_1 and are therefore inherently unscoped +// per MeshCore protocol (#1838). +const routeTypeNonTransportSQL = "route_type IN (1, 2)" + // DB wraps a read-only connection to the MeshCore SQLite database. type DB struct { conn *sql.DB @@ -2790,6 +2799,19 @@ func (db *DB) GetScopeStats(window string) (*ScopeStatsResponse, error) { return nil, fmt.Errorf("scope summary query: %w", err) } + // #1838: non-transport routes (FLOOD=1, DIRECT=2) never carry + // transport_code_1 per MeshCore protocol, so they are inherently unscoped. + // Fold their count into Summary.Unscoped so the analytics denominator + // reflects total-observed-transmissions rather than only transport-eligible. + var nonTransportUnscoped int + if err := db.conn.QueryRow(` + SELECT COUNT(*) FROM transmissions + WHERE `+routeTypeNonTransportSQL+` AND first_seen >= ? + `, since).Scan(&nonTransportUnscoped); err != nil { + return nil, fmt.Errorf("scope non-transport count query: %w", err) + } + resp.Summary.Unscoped += nonTransportUnscoped + // Per-region counts (named regions only) rows, err := db.conn.Query(` SELECT scope_name, COUNT(*) AS cnt diff --git a/cmd/server/db_test.go b/cmd/server/db_test.go index 75e3bfe6..5ac18c08 100644 --- a/cmd/server/db_test.go +++ b/cmd/server/db_test.go @@ -2254,21 +2254,26 @@ func TestGetScopeStats(t *testing.T) { db.conn.Exec(`INSERT INTO transmissions (hash, first_seen, route_type, scope_name) VALUES ('b', ?, 0, '')`, now) // Transport unscoped (NULL) db.conn.Exec(`INSERT INTO transmissions (hash, first_seen, route_type, scope_name) VALUES ('c', ?, 0, NULL)`, now) - // Non-transport (should not count) + // Non-transport FLOOD (route_type=1) — inherently unscoped per MeshCore protocol (#1838) db.conn.Exec(`INSERT INTO transmissions (hash, first_seen, route_type, scope_name) VALUES ('d', ?, 1, NULL)`, now) + // Non-transport DIRECT (route_type=2) — inherently unscoped per MeshCore protocol (#1838) + db.conn.Exec(`INSERT INTO transmissions (hash, first_seen, route_type, scope_name) VALUES ('e', ?, 2, NULL)`, now) stats, err := db.GetScopeStats("24h") if err != nil { t.Fatalf("GetScopeStats: %v", err) } + // TransportTotal still counts only routes 0+3 (transport-code-carrying). if stats.Summary.TransportTotal != 3 { t.Errorf("TransportTotal = %d, want 3", stats.Summary.TransportTotal) } if stats.Summary.Scoped != 2 { t.Errorf("Scoped = %d, want 2", stats.Summary.Scoped) } - if stats.Summary.Unscoped != 1 { - t.Errorf("Unscoped = %d, want 1", stats.Summary.Unscoped) + // Unscoped now folds in non-transport (routes 1+2) — see #1838. + // 1 transport-NULL + 2 non-transport (routes 1 and 2) = 3. + if stats.Summary.Unscoped != 3 { + t.Errorf("Unscoped = %d, want 3 (1 transport-null + 2 non-transport)", stats.Summary.Unscoped) } if stats.Summary.UnknownScope != 1 { t.Errorf("UnknownScope = %d, want 1", stats.Summary.UnknownScope) diff --git a/cmd/server/routes_test.go b/cmd/server/routes_test.go index ed271584..8050ce0f 100644 --- a/cmd/server/routes_test.go +++ b/cmd/server/routes_test.go @@ -4180,6 +4180,11 @@ func TestHandleScopeStats(t *testing.T) { } srv.db.hasScopeName = true + // Clear seed transmissions so this test isolates scope-stats math. + if _, err := srv.db.conn.Exec(`DELETE FROM transmissions`); err != nil { + t.Fatalf("clear transmissions: %v", err) + } + now := time.Now().UTC().Format(time.RFC3339) // 2 scoped (known region), 1 unknown-scoped (empty string), 1 unscoped (NULL) rows := []struct { @@ -4191,6 +4196,8 @@ func TestHandleScopeStats(t *testing.T) { {"h2", "#belgium", 3}, {"h3", "", 0}, // transport-scoped, no region match {"h4_null", "", 0}, // will be inserted with NULL scope_name + {"h5_nt1", "", 1}, // non-transport FLOOD — inherently unscoped (#1838) + {"h6_nt2", "", 2}, // non-transport DIRECT — inherently unscoped (#1838) } for i, r := range rows { var scopeArg interface{} = r.scope @@ -4225,8 +4232,8 @@ func TestHandleScopeStats(t *testing.T) { if resp.Summary.Scoped != 3 { // 2 named + 1 unknown-scoped (empty string, non-NULL) t.Errorf("scoped = %d, want 3", resp.Summary.Scoped) } - if resp.Summary.Unscoped != 1 { - t.Errorf("unscoped = %d, want 1", resp.Summary.Unscoped) + if resp.Summary.Unscoped != 3 { // 1 transport-null + 2 non-transport routes 1,2 (#1838) + t.Errorf("unscoped = %d, want 3", resp.Summary.Unscoped) } if resp.Summary.UnknownScope != 1 { t.Errorf("unknownScope = %d, want 1", resp.Summary.UnknownScope) diff --git a/public/analytics.js b/public/analytics.js index 7650d380..0e1a4658 100644 --- a/public/analytics.js +++ b/public/analytics.js @@ -4474,7 +4474,10 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _analyticsData = // Fix 5: write static frame only once if (!el.querySelector('#scopes-cards')) { el.innerHTML = - '

Scope Statistics

' + + '

Scope Statistics

' + + '

' + + 'Denominator is all observed transmissions. Only TRANSPORT_FLOOD (0) and TRANSPORT_DIRECT (3) routes carry a scope; FLOOD (1) and DIRECT (2) are inherently unscoped per MeshCore protocol.' + + '

' + '
' + ['1h', '24h', '7d'].map(function(v) { return ''; @@ -4526,15 +4529,19 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _analyticsData = function updateData(d, w) { var s = d.summary; + // #1838: denominator = transport-carrying transmissions (route_type 0,3). + // Unscoped now includes non-transport routes (1,2) which are inherently + // unscoped by MeshCore protocol, so unscoped can exceed transportTotal. var total = s.transportTotal || 0; + var overall = (s.scoped || 0) + (s.unscoped || 0); // Summary cards var cardsEl = document.getElementById('scopes-cards'); if (cardsEl) { cardsEl.innerHTML = [ - { label: 'Transport Total', value: total.toLocaleString(), note: '' }, - { label: 'Scoped', value: s.scoped.toLocaleString(), note: pct(s.scoped, total) }, - { label: 'Unscoped', value: s.unscoped.toLocaleString(), note: pct(s.unscoped, total) }, + { label: 'Transport Total', value: total.toLocaleString(), note: 'routes 0,3 (carry scope)' }, + { label: 'Scoped', value: s.scoped.toLocaleString(), note: pct(s.scoped, overall) + ' of all traffic' }, + { label: 'Unscoped', value: s.unscoped.toLocaleString(), note: pct(s.unscoped, overall) + ' of all traffic' }, { label: 'Unknown Scope', value: s.unknownScope.toLocaleString(), note: pct(s.unknownScope, s.scoped) + ' of scoped' }, ].map(function(c) { return '
' + c.value + '
' +