mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-10-23 15:14:01 +00:00
Fixes #1228 — geo-implausible neighbor-graph edges are rejected at build time. Red commit: `5a6d9660` — failing tests for 4 cases (reject SF↔Berlin, accept local CA, accept no-GPS endpoint, counter increments). Live CI run (latest commit): https://github.com/Kpa-clawbot/CoreScope/actions?query=branch%3Afix%2Fissue-1228 ## Why The disambiguator's tier-1 affinity graph is built blindly from path co-occurrence. On wide-geo MQTT deployments, a single bad hop disambiguation seeds an edge across geographically impossible distances (e.g. Bay Area ↔ Berlin), which then reinforces the same wrong resolution next time. Self-poisoning spiral. ## What changed - `upsertEdge` now consults a per-graph GPS index. When **both** endpoints have known GPS and their haversine distance exceeds the threshold, the edge is dropped and `NeighborGraph.RejectedEdgesGeoFar` (atomic) is incremented. - Either endpoint missing GPS ⇒ accept (no signal to reject), per acceptance criteria. - Threshold is configurable via `neighborGraph.maxEdgeKm` (default **500 km** — well above any plausible terrestrial LoRa hop, including satellite-assisted). 0 ⇒ use default; negative ⇒ disable the filter. Exposed via `Config.NeighborMaxEdgeKm()`. - New `BuildFromStoreWithOptions` carrying the threshold; `BuildFromStore` and `BuildFromStoreWithLog` are kept as thin wrappers. - Stats are surfaced under `GET /api/analytics/neighbor-graph` as `stats.rejected_edges_geo_far`. - All rejection logs PII-truncate pubkeys to 8 hex chars (public repo discipline). - `config.example.json` updated with the new field + comment. ## Follow-up #1229 (per-region scoped affinity graphs) depends on this landing first. --------- Co-authored-by: corescope-bot <bot@corescope.local>
This commit is contained in:
1 parent
30ff45ad34
commit
7179afcfde
5 files changed
+246
-15
No files matched your search
+15
-1
@@ -129,7 +129,8 @@ type ResolvedPathConfig struct {
|
||||
|
||||
// NeighborGraphConfig controls neighbor edge pruning.
|
||||
type NeighborGraphConfig struct {
|
||||
MaxAgeDays int `json:"maxAgeDays"` // edges older than this are pruned (default 5)
|
||||
MaxAgeDays int `json:"maxAgeDays"` // edges older than this are pruned (default 5)
|
||||
MaxEdgeKm float64 `json:"maxEdgeKm"` // geo-implausibility threshold (km); 0 = default 500; negative disables (#1228)
|
||||
}
|
||||
|
||||
// PacketStoreConfig controls in-memory packet store limits.
|
||||
@@ -185,6 +186,19 @@ func (c *Config) NeighborMaxAgeDays() int {
|
||||
return 5
|
||||
}
|
||||
|
||||
// NeighborMaxEdgeKm returns the geo-implausibility threshold in km.
|
||||
// 0 (unset) → DefaultMaxEdgeKm (500). Negative → 0 (filter disabled).
|
||||
// See issue #1228.
|
||||
func (c *Config) NeighborMaxEdgeKm() float64 {
|
||||
if c == nil || c.NeighborGraph == nil || c.NeighborGraph.MaxEdgeKm == 0 {
|
||||
return DefaultMaxEdgeKm
|
||||
}
|
||||
if c.NeighborGraph.MaxEdgeKm < 0 {
|
||||
return 0
|
||||
}
|
||||
return c.NeighborGraph.MaxEdgeKm
|
||||
}
|
||||
|
||||
type TimestampConfig struct {
|
||||
DefaultMode string `json:"defaultMode"` // "ago" | "absolute"
|
||||
Timezone string `json:"timezone"` // "local" | "utc"
|
||||
|
||||
+17
-10
@@ -6,6 +6,7 @@ import (
|
||||
"sort"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
|
||||
"github.com/gorilla/mux"
|
||||
@@ -66,10 +67,11 @@ type GraphEdge struct {
|
||||
}
|
||||
|
||||
type GraphStats struct {
|
||||
TotalNodes int `json:"total_nodes"`
|
||||
TotalEdges int `json:"total_edges"`
|
||||
AmbiguousEdges int `json:"ambiguous_edges"`
|
||||
AvgClusterSize float64 `json:"avg_cluster_size"`
|
||||
TotalNodes int `json:"total_nodes"`
|
||||
TotalEdges int `json:"total_edges"`
|
||||
AmbiguousEdges int `json:"ambiguous_edges"`
|
||||
AvgClusterSize float64 `json:"avg_cluster_size"`
|
||||
RejectedEdgesGeoFar uint64 `json:"rejected_edges_geo_far"` // edges dropped at build time by the geo-implausibility filter (#1228)
|
||||
}
|
||||
|
||||
// ─── Graph accessor on Server ──────────────────────────────────────────────────
|
||||
@@ -81,8 +83,12 @@ func (s *Server) getNeighborGraph() *NeighborGraph {
|
||||
|
||||
if s.neighborGraph == nil || s.neighborGraph.IsStale() {
|
||||
if s.store != nil {
|
||||
debugLog := s.cfg != nil && s.cfg.DebugAffinity
|
||||
s.neighborGraph = BuildFromStoreWithLog(s.store, debugLog)
|
||||
opts := BuildOptions{MaxEdgeKm: DefaultMaxEdgeKm}
|
||||
if s.cfg != nil {
|
||||
opts.EnableLog = s.cfg.DebugAffinity
|
||||
opts.MaxEdgeKm = s.cfg.NeighborMaxEdgeKm()
|
||||
}
|
||||
s.neighborGraph = BuildFromStoreWithOptions(s.store, opts)
|
||||
} else {
|
||||
s.neighborGraph = NewNeighborGraph()
|
||||
}
|
||||
@@ -347,10 +353,11 @@ func (s *Server) handleNeighborGraph(w http.ResponseWriter, r *http.Request) {
|
||||
Nodes: nodes,
|
||||
Edges: filteredEdges,
|
||||
Stats: GraphStats{
|
||||
TotalNodes: len(nodes),
|
||||
TotalEdges: len(filteredEdges),
|
||||
AmbiguousEdges: ambiguousCount,
|
||||
AvgClusterSize: avgCluster,
|
||||
TotalNodes: len(nodes),
|
||||
TotalEdges: len(filteredEdges),
|
||||
AmbiguousEdges: ambiguousCount,
|
||||
AvgClusterSize: avgCluster,
|
||||
RejectedEdgesGeoFar: atomic.LoadUint64(&graph.RejectedEdgesGeoFar),
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
"math"
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
)
|
||||
|
||||
@@ -87,6 +88,27 @@ type NeighborGraph struct {
|
||||
byNode map[string][]*NeighborEdge // pubkey → edges involving this node
|
||||
builtAt time.Time
|
||||
logFn func(prefix, msg string) // optional structured logging callback
|
||||
|
||||
// RejectedEdgesGeoFar counts edges dropped at build time because both
|
||||
// endpoints had GPS and their haversine distance exceeded the
|
||||
// configurable threshold (NeighborGraphConfig.MaxEdgeKm, default 500).
|
||||
// Accessed via sync/atomic. See issue #1228.
|
||||
RejectedEdgesGeoFar uint64
|
||||
|
||||
// maxEdgeKm is the geo-sanity threshold copied from config at build
|
||||
// time. 0 means "no limit" / filter disabled.
|
||||
maxEdgeKm float64
|
||||
|
||||
// nodeGeo maps lowercased pubkey → (lat, lon, hasGPS) for geo-sanity
|
||||
// checks during upsertEdge. Populated by the builder; empty for graphs
|
||||
// constructed via NewNeighborGraph directly (geo filter inert).
|
||||
nodeGeo map[string]nodeGeoInfo
|
||||
}
|
||||
|
||||
// nodeGeoInfo is the minimal geo slice cached on the graph for upsertEdge.
|
||||
type nodeGeoInfo struct {
|
||||
Lat, Lon float64
|
||||
HasGPS bool
|
||||
}
|
||||
|
||||
// NewNeighborGraph creates an empty graph.
|
||||
@@ -148,9 +170,20 @@ func (g *NeighborGraph) IsStale() bool {
|
||||
// BuildFromStore constructs the neighbor graph from all packets in the store.
|
||||
// The store's read-lock must NOT be held by the caller.
|
||||
func BuildFromStore(store *PacketStore) *NeighborGraph {
|
||||
return BuildFromStoreWithLog(store, false)
|
||||
return BuildFromStoreWithOptions(store, BuildOptions{MaxEdgeKm: DefaultMaxEdgeKm})
|
||||
}
|
||||
|
||||
// BuildOptions controls optional behavior of BuildFromStoreWithOptions.
|
||||
type BuildOptions struct {
|
||||
EnableLog bool // structured disambiguation logging
|
||||
MaxEdgeKm float64 // geo-sanity threshold; 0 disables the filter
|
||||
}
|
||||
|
||||
// DefaultMaxEdgeKm is the conservative built-in cap for the
|
||||
// geo-implausibility filter (issue #1228). 500 km is comfortably above any
|
||||
// plausible terrestrial LoRa hop (including satellite-relayed cases).
|
||||
const DefaultMaxEdgeKm = 500.0
|
||||
|
||||
// cachedToLower returns strings.ToLower(s), caching results to avoid
|
||||
// repeated allocations for the same pubkey string.
|
||||
func cachedToLower(cache map[string]string, s string) string {
|
||||
@@ -163,9 +196,16 @@ func cachedToLower(cache map[string]string, s string) string {
|
||||
}
|
||||
|
||||
// BuildFromStoreWithLog constructs the neighbor graph, optionally logging disambiguation decisions.
|
||||
// Kept for backward compatibility; new callers should use BuildFromStoreWithOptions.
|
||||
func BuildFromStoreWithLog(store *PacketStore, enableLog bool) *NeighborGraph {
|
||||
return BuildFromStoreWithOptions(store, BuildOptions{EnableLog: enableLog, MaxEdgeKm: DefaultMaxEdgeKm})
|
||||
}
|
||||
|
||||
// BuildFromStoreWithOptions constructs the neighbor graph with explicit options.
|
||||
func BuildFromStoreWithOptions(store *PacketStore, opts BuildOptions) *NeighborGraph {
|
||||
g := NewNeighborGraph()
|
||||
if enableLog {
|
||||
g.maxEdgeKm = opts.MaxEdgeKm
|
||||
if opts.EnableLog {
|
||||
g.logFn = func(prefix, msg string) {
|
||||
log.Printf("[affinity] resolve %s: %s", prefix, msg)
|
||||
}
|
||||
@@ -179,7 +219,16 @@ func BuildFromStoreWithLog(store *PacketStore, enableLog bool) *NeighborGraph {
|
||||
|
||||
// Build prefix map for candidate resolution.
|
||||
// Use cached nodes+PM (avoids DB call if cache is fresh).
|
||||
_, pm := store.getCachedNodesAndPM()
|
||||
allNodes, pm := store.getCachedNodesAndPM()
|
||||
|
||||
// Index node geo for upsertEdge geo-sanity checks (issue #1228).
|
||||
geo := make(map[string]nodeGeoInfo, len(allNodes))
|
||||
for _, n := range allNodes {
|
||||
geo[strings.ToLower(n.PublicKey)] = nodeGeoInfo{Lat: n.Lat, Lon: n.Lon, HasGPS: n.HasGPS}
|
||||
}
|
||||
g.mu.Lock()
|
||||
g.nodeGeo = geo
|
||||
g.mu.Unlock()
|
||||
|
||||
// Local cache for strings.ToLower — pubkeys are immutable and repeat
|
||||
// across hundreds of thousands of observations.
|
||||
@@ -267,6 +316,13 @@ func jsonUnmarshalFast(data string, v interface{}) error {
|
||||
|
||||
// upsertEdge adds/updates an edge between two fully-known pubkeys.
|
||||
func (g *NeighborGraph) upsertEdge(pubkeyA, pubkeyB, prefix, observer string, snr *float64, ts time.Time) {
|
||||
// Geo-sanity guard (issue #1228): if both endpoints have known GPS and
|
||||
// the haversine distance exceeds the configured threshold, drop the
|
||||
// edge. When either lacks GPS we have no signal and accept.
|
||||
if g.shouldRejectGeoFar(pubkeyA, pubkeyB) {
|
||||
atomic.AddUint64(&g.RejectedEdgesGeoFar, 1)
|
||||
return
|
||||
}
|
||||
key := makeEdgeKey(pubkeyA, pubkeyB)
|
||||
|
||||
g.mu.Lock()
|
||||
@@ -652,3 +708,40 @@ func (g *NeighborGraph) PruneOlderThan(cutoff time.Time) int {
|
||||
}
|
||||
return pruned
|
||||
}
|
||||
|
||||
// shouldRejectGeoFar reports whether the edge (a, b) is geographically
|
||||
// implausible under the configured threshold. Both endpoints must have known
|
||||
// GPS to trigger a rejection; if either lacks GPS the edge is accepted
|
||||
// (issue #1228 — "no signal to reject").
|
||||
//
|
||||
// All log output is PII-truncated to the first 8 hex chars of each pubkey.
|
||||
func (g *NeighborGraph) shouldRejectGeoFar(a, b string) bool {
|
||||
if g == nil || g.maxEdgeKm <= 0 || g.nodeGeo == nil {
|
||||
return false
|
||||
}
|
||||
if strings.HasPrefix(a, "prefix:") || strings.HasPrefix(b, "prefix:") {
|
||||
return false
|
||||
}
|
||||
ga, oka := g.nodeGeo[a]
|
||||
gb, okb := g.nodeGeo[b]
|
||||
if !oka || !okb || !ga.HasGPS || !gb.HasGPS {
|
||||
return false
|
||||
}
|
||||
d := haversineKm(ga.Lat, ga.Lon, gb.Lat, gb.Lon)
|
||||
if d <= g.maxEdgeKm {
|
||||
return false
|
||||
}
|
||||
// PII-truncated INFO log (8-char prefix max).
|
||||
log.Printf("[neighbor-graph] reject geo-far edge %s↔%s distance=%.0fkm threshold=%.0fkm",
|
||||
piiTruncPubkey(a), piiTruncPubkey(b), d, g.maxEdgeKm)
|
||||
return true
|
||||
}
|
||||
|
||||
// piiTruncPubkey returns at most the first 8 hex chars of a pubkey for log
|
||||
// output. The repo is public and observer/node pubkeys are PII-adjacent.
|
||||
func piiTruncPubkey(pk string) string {
|
||||
if len(pk) <= 8 {
|
||||
return pk
|
||||
}
|
||||
return pk[:8]
|
||||
}
|
||||
@@ -0,0 +1,116 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestBuildNeighborGraph_RejectsGeoFarEdge — RED test for #1228.
|
||||
//
|
||||
// Synthetic advert produces an edge between A (Bay Area) and B (Berlin).
|
||||
// Distance ≈ 9 100 km, well above any plausible terrestrial LoRa hop.
|
||||
// The geo-sanity filter must reject the edge at build time so the
|
||||
// affinity graph cannot self-reinforce a wrong disambiguation.
|
||||
func TestBuildNeighborGraph_RejectsGeoFarEdge(t *testing.T) {
|
||||
nodes := []nodeInfo{
|
||||
// A: San Francisco
|
||||
{Role: "repeater", PublicKey: "aaaa1111", Name: "A_SF", Lat: 37.77, Lon: -122.41, HasGPS: true},
|
||||
// B: Berlin
|
||||
{Role: "repeater", PublicKey: "bbbb2222", Name: "B_BE", Lat: 52.52, Lon: 13.40, HasGPS: true},
|
||||
// Observer with GPS at SF (won't affect A↔B edge under test)
|
||||
{Role: "repeater", PublicKey: "obs00001", Name: "Observer", Lat: 37.77, Lon: -122.41, HasGPS: true},
|
||||
}
|
||||
// ADVERT originated by A, path=["bbbb"] → builder creates edge A↔B
|
||||
// (originator ↔ path[0]). With geo sanity ON this edge must be dropped.
|
||||
tx := ngMakeTx(1, 4, ngFromNodeJSON("aaaa1111"), []*StoreObs{
|
||||
ngMakeObs("obs00001", `["bbbb"]`, nowStr, ngFloatPtr(-10)),
|
||||
})
|
||||
store := ngTestStore(nodes, []*StoreTx{tx})
|
||||
g := BuildFromStore(store)
|
||||
|
||||
for _, e := range g.AllEdges() {
|
||||
if (e.NodeA == "aaaa1111" && e.NodeB == "bbbb2222") ||
|
||||
(e.NodeA == "bbbb2222" && e.NodeB == "aaaa1111") {
|
||||
t.Fatalf("geo-implausible edge A(SF)↔B(Berlin) was not rejected: %+v", e)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestBuildNeighborGraph_AcceptsLocalEdge — A↔C within plausible LoRa range
|
||||
// (both in CA, ~100 km apart) must remain in the graph.
|
||||
func TestBuildNeighborGraph_AcceptsLocalEdge(t *testing.T) {
|
||||
nodes := []nodeInfo{
|
||||
{Role: "repeater", PublicKey: "aaaa1111", Name: "A_SF", Lat: 37.77, Lon: -122.41, HasGPS: true},
|
||||
{Role: "repeater", PublicKey: "cccc3333", Name: "C_SJ", Lat: 37.34, Lon: -121.89, HasGPS: true},
|
||||
{Role: "repeater", PublicKey: "obs00001", Name: "Observer", Lat: 37.77, Lon: -122.41, HasGPS: true},
|
||||
}
|
||||
tx := ngMakeTx(1, 4, ngFromNodeJSON("aaaa1111"), []*StoreObs{
|
||||
ngMakeObs("obs00001", `["cccc"]`, nowStr, ngFloatPtr(-10)),
|
||||
})
|
||||
store := ngTestStore(nodes, []*StoreTx{tx})
|
||||
g := BuildFromStore(store)
|
||||
|
||||
found := false
|
||||
for _, e := range g.AllEdges() {
|
||||
if (e.NodeA == "aaaa1111" && e.NodeB == "cccc3333") ||
|
||||
(e.NodeA == "cccc3333" && e.NodeB == "aaaa1111") {
|
||||
found = true
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Fatalf("local A↔C edge (~50km) must be accepted")
|
||||
}
|
||||
}
|
||||
|
||||
// TestBuildNeighborGraph_AcceptsEdgeWhenNoGPS — if either endpoint lacks GPS,
|
||||
// we have no signal to reject; the edge is accepted.
|
||||
func TestBuildNeighborGraph_AcceptsEdgeWhenNoGPS(t *testing.T) {
|
||||
nodes := []nodeInfo{
|
||||
// A has GPS (Berlin)
|
||||
{Role: "repeater", PublicKey: "aaaa1111", Name: "A", Lat: 52.52, Lon: 13.40, HasGPS: true},
|
||||
// D has no GPS
|
||||
{Role: "repeater", PublicKey: "dddd4444", Name: "D"}, // HasGPS = false
|
||||
{Role: "repeater", PublicKey: "obs00001", Name: "Observer"},
|
||||
}
|
||||
tx := ngMakeTx(1, 4, ngFromNodeJSON("aaaa1111"), []*StoreObs{
|
||||
ngMakeObs("obs00001", `["dddd"]`, nowStr, nil),
|
||||
})
|
||||
store := ngTestStore(nodes, []*StoreTx{tx})
|
||||
g := BuildFromStore(store)
|
||||
|
||||
found := false
|
||||
for _, e := range g.AllEdges() {
|
||||
if (e.NodeA == "aaaa1111" && e.NodeB == "dddd4444") ||
|
||||
(e.NodeA == "dddd4444" && e.NodeB == "aaaa1111") {
|
||||
found = true
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Fatalf("A(GPS)↔D(no-GPS) edge must be accepted (no signal to reject)")
|
||||
}
|
||||
}
|
||||
|
||||
// TestBuildNeighborGraph_RejectedCounterIncrements — every dropped edge bumps
|
||||
// the atomic counter surfaced by /api/analytics/neighbor-graph stats.
|
||||
func TestBuildNeighborGraph_RejectedCounterIncrements(t *testing.T) {
|
||||
nodes := []nodeInfo{
|
||||
{Role: "repeater", PublicKey: "aaaa1111", Name: "A_SF", Lat: 37.77, Lon: -122.41, HasGPS: true},
|
||||
{Role: "repeater", PublicKey: "bbbb2222", Name: "B_BE", Lat: 52.52, Lon: 13.40, HasGPS: true},
|
||||
{Role: "repeater", PublicKey: "obs00001", Name: "Observer", Lat: 37.77, Lon: -122.41, HasGPS: true},
|
||||
}
|
||||
// Two adverts each producing the far A↔B edge attempt → counter ≥ 2.
|
||||
txs := []*StoreTx{
|
||||
ngMakeTx(1, 4, ngFromNodeJSON("aaaa1111"), []*StoreObs{
|
||||
ngMakeObs("obs00001", `["bbbb"]`, nowStr, nil),
|
||||
}),
|
||||
ngMakeTx(2, 4, ngFromNodeJSON("aaaa1111"), []*StoreObs{
|
||||
ngMakeObs("obs00001", `["bbbb"]`, nowStr, nil),
|
||||
}),
|
||||
}
|
||||
store := ngTestStore(nodes, txs)
|
||||
g := BuildFromStore(store)
|
||||
got := atomic.LoadUint64(&g.RejectedEdgesGeoFar)
|
||||
if got < 2 {
|
||||
t.Fatalf("RejectedEdgesGeoFar = %d, want >= 2", got)
|
||||
}
|
||||
}
|
||||
+2
-1
@@ -234,7 +234,8 @@
|
||||
},
|
||||
"neighborGraph": {
|
||||
"maxAgeDays": 5,
|
||||
"_comment": "Neighbor edges older than this many days are pruned on startup and daily. Default: 5."
|
||||
"maxEdgeKm": 500,
|
||||
"_comment": "maxAgeDays: neighbor edges older than this many days are pruned on startup and daily. Default 5. maxEdgeKm: geo-implausibility filter — when both endpoints have GPS, edges with haversine distance > maxEdgeKm are rejected at build time to prevent disambiguator self-reinforcement on wide-geo MQTT deployments. Default 500 km (well above any plausible terrestrial LoRa hop). 0 ⇒ use default; set negative to disable the filter. Rejected count surfaces in /api/analytics/neighbor-graph stats. Issue #1228."
|
||||
},
|
||||
"batteryThresholds": {
|
||||
"lowMv": 3300,
|
||||
|
||||
Reference in new issue
Block a user