mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-10-23 04:01:16 +00:00
fix(1838): fold non-transport routes into scope-stats Unscoped (#1842)
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 <bot@corescope.dev>
This commit is contained in:
1 parent
d60188e481
commit
59cf5130d1
4 files changed
+52
-11
No files matched your search
+24
-2
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
+11
-4
@@ -4474,7 +4474,10 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _analyticsData =
|
||||
// Fix 5: write static frame only once
|
||||
if (!el.querySelector('#scopes-cards')) {
|
||||
el.innerHTML =
|
||||
'<h3 style="margin:0 0 12px">Scope Statistics</h3>' +
|
||||
'<h3 style="margin:0 0 4px">Scope Statistics</h3>' +
|
||||
'<p class="text-muted" style="margin:0 0 12px;font-size:0.85em">' +
|
||||
'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.' +
|
||||
'</p>' +
|
||||
'<div style="margin-bottom:12px">' +
|
||||
['1h', '24h', '7d'].map(function(v) {
|
||||
return '<button class="tab-btn' + (selectedWindow === v ? ' active' : '') + '" data-win="' + v + '">' + v + '</button>';
|
||||
@@ -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 '<div class="stat-card"><div class="stat-value">' + c.value + '</div>' +
|
||||
|
||||
Reference in new issue
Block a user