diff --git a/cmd/server/hop_disambig_e2e_test.go b/cmd/server/hop_disambig_e2e_test.go new file mode 100644 index 00000000..6e4e289c --- /dev/null +++ b/cmd/server/hop_disambig_e2e_test.go @@ -0,0 +1,242 @@ +package main + +import ( + "encoding/json" + "fmt" + "testing" + "time" +) + +// End-to-end fixture test for issue #1201 sub-task 4. +// +// Builds a *PacketStore with multi-candidate-prefix nodes (intentional 1-byte +// prefix collisions across continents) and asserts that the top-hops ranking +// produced by buildDistanceIndex honors the resolver's neighbor-affinity +// choice, NOT the misresolution interpretations that would survive without +// context. +// +// Mutation-test sentinel: this test MUST fail if any call site that feeds +// per-tx context to the hop resolver is reverted to `nil`. Reproduce by +// replacing the `setContext(buildHopContextPubkeys(tx, pm))` call inside +// buildDistanceIndex (cmd/server/store.go, in the per-tx loop) with +// `setContext(nil)` and re-running this test — it fails with a "CA↔CA hop +// missing, saw 72dddd→8acccc (Berlin↔Berlin)" assertion. See PR body for +// the full mutation log. +// +// Fixture layout (no real handles — generic placeholders only): +// Prefix "72" (4 candidates, all repeaters with GPS): +// - 72aa… SLO-CA (35.30, -120.70) obsCount=5 +// - 72bb… LA-CA (34.05, -118.25) obsCount=5 +// - 72cc… NYC (40.70, -74.00) obsCount=5 +// - 72dd… Berlin (52.50, 13.40) obsCount=200 ← would win tier-3 +// Prefix "8a" (3 candidates): +// - 8aaa… SF-CA (37.00, -120.50) obsCount=5 +// - 8abb… CA-other (36.50, -119.50) obsCount=5 +// - 8acc… Berlin (52.60, 13.50) obsCount=200 ← would win tier-3 +// +// Sender: CA repeater at (36.0, -120.0), pubkey "ccc…". +// Observer: CA repeater at (36.2, -120.2), pubkey "dddd…". +// +// Affinity graph: strong edges sender↔72aa and sender↔8aaa +// (count ≥ affinityMinObservations, recent timestamps). +// +// 50 synthetic transmissions, all with path ["72","8a"]. With per-tx context +// piped through (sender pubkey is added by buildHopContextPubkeys), tier 1 +// picks the CA candidates. Without it, tier 3 picks the Berlin candidates +// and the Berlin↔Berlin hop (~11 km — under 300 km cap) becomes the only +// surviving hop. The test asserts the inverse: CA↔CA hop present, no +// Berlin pubkeys appear in distHops. + +const ( + t1201Sender = "ccccccccccccccc1" + t1201Observer = "dddddddddddddddd" + + t1201_72aa = "72aaaaaaaaaaaaaa" // SLO + t1201_72bb = "72bbbbbbbbbbbbbb" // LA + t1201_72cc = "72cccccccccccccc" // NYC + t1201_72dd = "72dddddddddddddd" // Berlin + + t1201_8aaa = "8aaaaaaaaaaaaaaa" // SF + t1201_8abb = "8abbbbbbbbbbbbbb" // CA-other + t1201_8acc = "8acccccccccccccc" // Berlin +) + +type t1201Node struct { + pk string + lat, lon float64 + obsCount int +} + +func t1201InsertNode(t *testing.T, db *DB, n t1201Node) { + t.Helper() + // NOTE: `obsCount` is written to the `advert_count` column. That column + // is what resolveWithContext reads (via nodeInfo.ObservationCount / + // betterByObsCount) as the tier-3 popularity tiebreak. If the tier-3 + // source column ever changes (e.g. observations.packet_count), the + // "Berlin would win tier-3" premise of this fixture weakens silently — + // update both this insert and the candidate scoring assertions. + _, err := db.conn.Exec( + `INSERT INTO nodes (public_key, name, role, lat, lon, last_seen, first_seen, advert_count) VALUES (?, ?, 'repeater', ?, ?, ?, '2026-01-01T00:00:00Z', ?)`, + n.pk, "node-"+n.pk[:4], n.lat, n.lon, "2026-05-01T00:00:00Z", n.obsCount, + ) + if err != nil { + t.Fatalf("insert node %s: %v", n.pk, err) + } +} + +// TestTopHopsRespectsContextAcrossAllCallSites is the end-to-end regression +// sentinel for issue #1201. See file-header docblock for design. +func TestTopHopsRespectsContextAcrossAllCallSites(t *testing.T) { + db := setupTestDB(t) + + // Insert all repeater nodes with GPS + observation counts. + nodes := []t1201Node{ + {t1201Sender, 36.0, -120.0, 50}, + {t1201Observer, 36.2, -120.2, 60}, + + {t1201_72aa, 35.30, -120.70, 5}, + {t1201_72bb, 34.05, -118.25, 5}, + {t1201_72cc, 40.70, -74.00, 5}, + {t1201_72dd, 52.50, 13.40, 200}, // would win tier-3 without context + + {t1201_8aaa, 37.00, -120.50, 5}, + {t1201_8abb, 36.50, -119.50, 5}, + {t1201_8acc, 52.60, 13.50, 200}, // would win tier-3 without context + } + for _, n := range nodes { + t1201InsertNode(t, db, n) + } + + // Insert observer row (referenced by observations via observer_idx). + if _, err := db.conn.Exec( + `INSERT INTO observers (id, name, last_seen, first_seen, packet_count) VALUES (?, ?, ?, '2026-01-01T00:00:00Z', 100)`, + t1201Observer, "obs-ca", "2026-05-01T00:00:00Z", + ); err != nil { + t.Fatal(err) + } + + // Insert 50 transmissions, each with path ["72","8a"], sender pubkey + // embedded in decoded_json (read by buildHopContextPubkeys via ParsedDecoded). + // Wrapped in a single BEGIN/COMMIT — shaves wall time on slow CI runners. + decoded, _ := json.Marshal(map[string]interface{}{"pubKey": t1201Sender, "type": "data"}) + pathJSON := `["72","8a"]` + baseTime := time.Date(2026, 5, 1, 0, 0, 0, 0, time.UTC) + tx, err := db.conn.Begin() + if err != nil { + t.Fatalf("begin tx: %v", err) + } + for i := 0; i < 50; i++ { + ts := baseTime.Add(time.Duration(i) * time.Minute).Format(time.RFC3339) + hash := fmt.Sprintf("hash1201_%03d", i) + res, err := tx.Exec( + `INSERT INTO transmissions (raw_hex, hash, first_seen, route_type, payload_type, decoded_json) VALUES (?, ?, ?, 1, 1, ?)`, + "AA", hash, ts, string(decoded), + ) + if err != nil { + _ = tx.Rollback() + t.Fatal(err) + } + txID, _ := res.LastInsertId() + if _, err := tx.Exec( + `INSERT INTO observations (transmission_id, observer_idx, snr, rssi, path_json, timestamp) VALUES (?, 1, 12.0, -90, ?, ?)`, + txID, pathJSON, baseTime.Add(time.Duration(i)*time.Minute).Unix(), + ); err != nil { + _ = tx.Rollback() + t.Fatal(err) + } + } + if err := tx.Commit(); err != nil { + t.Fatalf("commit tx: %v", err) + } + + // Build store and seed graph BEFORE Load() — Load calls buildDistanceIndex + // which reads s.graph; if it's nil, tier 1 is skipped. + store := NewPacketStore(db, nil) + g := NewNeighborGraph() + // Strong sender↔72aa and sender↔8aaa edges (count well above + // affinityMinObservations, recent timestamp). + now := time.Now() + for i := 0; i < 100; i++ { + g.upsertEdge(t1201Sender, t1201_72aa, "72", t1201Observer, nil, now.Add(-time.Duration(i)*time.Minute)) + g.upsertEdge(t1201Sender, t1201_8aaa, "8a", t1201Observer, nil, now.Add(-time.Duration(i)*time.Minute)) + } + // Weaker sender↔Berlin edges so even if someone weakens the ratio guard, + // the CA candidates still dominate by 100× — and the Berlin counts in + // node table don't bleed through. + for i := 0; i < 2; i++ { + g.upsertEdge(t1201Sender, t1201_72dd, "72", t1201Observer, nil, now.Add(-time.Duration(i)*time.Hour)) + } + store.graph = g + + if err := store.Load(); err != nil { + t.Fatalf("Load: %v", err) + } + + // Inspect precomputed distance index. + store.mu.RLock() + hops := make([]distHopRecord, len(store.distHops)) + copy(hops, store.distHops) + store.mu.RUnlock() + + if len(hops) == 0 { + t.Fatal("buildDistanceIndex produced zero hops; expected at least the CA↔CA leg") + } + + // Assertion 1: CA↔CA hop between 72aa (SLO) and 8aaa (SF) must appear. + pairHas := func(h *distHopRecord, a, b string) bool { + return (h.FromPk == a && h.ToPk == b) || (h.FromPk == b && h.ToPk == a) + } + var sawCAPair bool + for i := range hops { + if pairHas(&hops[i], t1201_72aa, t1201_8aaa) { + sawCAPair = true + break + } + } + if !sawCAPair { + // Surface what we did see so failure is debuggable. + seen := []string{} + for i := range hops { + seen = append(seen, fmt.Sprintf("%s→%s@%.1fkm", hops[i].FromPk[:6], hops[i].ToPk[:6], hops[i].Dist)) + if i >= 5 { + seen = append(seen, "…") + break + } + } + t.Fatalf("expected CA↔CA hop (72aa↔8aaa) in distHops; saw %v", seen) + } + + // Assertion 2: no hop should reference Berlin pubkeys. The Berlin↔Berlin + // pair is the misresolution-only outcome that emerges when context is + // dropped; its presence proves a regression at one of the call sites. + // Note: 72cc (NYC) is omitted from this guard — its obsCount=5 would + // never win the tier-3 obsCount-200 fight against Berlin, so checking + // for it was redundant defense. Berlin pubkeys carry the signal. + berlinPKs := map[string]bool{ + t1201_72dd: true, + t1201_8acc: true, + } + for i := range hops { + if berlinPKs[hops[i].FromPk] || berlinPKs[hops[i].ToPk] { + t.Fatalf("misresolution hop leaked into distHops: %s→%s dist=%.1fkm (any call site dropped context?)", + hops[i].FromPk, hops[i].ToPk, hops[i].Dist) + } + } + + // Assertion 3: top-hop max distance must be consistent with CA geometry, + // well under the continent-spanning misresolution range. + maxDist := 0.0 + for i := range hops { + if hops[i].Dist > maxDist { + maxDist = hops[i].Dist + } + } + // SLO→SF ≈ 190 km; LA→SF ≈ 560 km (>300 cap → dropped). Cap should + // keep max well under 300. We drop the lower-bound "suspiciously small" + // floor: the >300 ceiling carries the misresolution signal on its own, + // and a tight floor would false-fire if a future cap tightening or + // fixture tweak legitimately shrinks the surviving CA↔CA leg. + if maxDist > 300 { + t.Fatalf("top-hop max distance %.1fkm exceeds 300km cap — resolver picked continent-spanning candidate", maxDist) + } +} diff --git a/cmd/server/hop_disambig_tier1_test.go b/cmd/server/hop_disambig_tier1_test.go new file mode 100644 index 00000000..d8cfd7cc --- /dev/null +++ b/cmd/server/hop_disambig_tier1_test.go @@ -0,0 +1,204 @@ +package main + +import ( + "testing" + "time" +) + +// Regression coverage for the hop disambiguator's tier-1 (neighbor affinity) +// path of pm.resolveWithContext. Issue #1201: tier 1 is the strongest +// disambiguation signal but was untested by any test we shipped — only +// upstream tests (that predate the context-plumbing fix in #1198) exercised +// it. These tests pin tier-1 behavior so any future refactor that disables +// tier 1, reorders priorities, or drops the Ambiguous-edge guard will fail. +// +// Naming convention for fixture pubkeys: lowercase hex placeholders only; +// no real observer/operator handles (per AGENTS.md PII rules). + +// ─── helpers ─────────────────────────────────────────────────────────────────── + +// seedAffinity adds n observations of an edge between obsPK and candPK at +// recent timestamps. Count ≥ affinityMinObservations is required for tier 1 +// to consider an edge. +func seedAffinity(g *NeighborGraph, obsPK, candPK, prefix, observer string, n int) { + now := time.Now() + for i := 0; i < n; i++ { + g.upsertEdge(obsPK, candPK, prefix, observer, nil, now.Add(-time.Duration(i)*time.Minute)) + } +} + +// Standard fixture shared by most tier-1 tests: two "72" candidates and +// (when needed) an anchor pubkey co-located with candY. candX is far +// (Seattle), candY is near LA — so geo proximity to anchor picks candY +// unless tier-1 fires for candX. +var tier1StdNodes = []nodeInfo{ + {PublicKey: "72aaaaaaaaaa", Role: "repeater", Name: "candX", HasGPS: true, Lat: 47.6, Lon: -122.3}, // Seattle (far) + {PublicKey: "72bbbbbbbbbb", Role: "repeater", Name: "candY", HasGPS: true, Lat: 34.05, Lon: -118.25}, // LA (near anchor) + {PublicKey: "ffeeeeeeeeee", Role: "repeater", Name: "anchor", HasGPS: true, Lat: 34.1, Lon: -118.3}, +} + +const tier1Anchor = "ffeeeeeeeeee" + +// ─── sub-task 1: tier-1 explicit tests (table-driven) ────────────────────────── + +// TestResolveWithContext_Tier1 collapses what were five near-identical +// per-branch functions into one table-driven test. Each row exercises +// exactly one tier-1 branch (strong-pick X, strong-pick Y, ambiguous-skip, +// tier-1-beats-tier-2, fall-throughs). Adding a new tier-1 case is a +// one-line addition. +// +// Mirror-pair rows (StrongAffinityPicksX / PicksY) prevent a "tier-1 always +// returns first candidate" tautology — the score MUST be consulted because +// flipping the weights flips the winner. +func TestResolveWithContext_Tier1(t *testing.T) { + type seed struct { + obsPK, candPK, prefix string + count int + } + cases := []struct { + name string + nodes []nodeInfo + ctxPK string + useNilGraph bool // skip graph entirely (tests `graph != nil` guard) + seeds []seed // tier-1 affinity seeds + markAmbiguous [2]string // if non-empty pair, mark that edge ambiguous + extraGraphSeed *seed // seed unrelated to ctxPK (empty-for-context fixture) + wantName string + wantMethod string + }{ + { + name: "StrongAffinityPicksX", + nodes: []nodeInfo{{PublicKey: "72aaaaaaaaaa", Role: "repeater", Name: "candX", HasGPS: true, Lat: 35.3, Lon: -120.7}, {PublicKey: "72bbbbbbbbbb", Role: "repeater", Name: "candY", HasGPS: true, Lat: 34.0, Lon: -118.2}}, + ctxPK: "ccccccccccc1", + seeds: []seed{{"ccccccccccc1", "72aaaaaaaaaa", "72", 100}, {"ccccccccccc1", "72bbbbbbbbbb", "72", 1}}, + wantName: "candX", + wantMethod: "neighbor_affinity", + }, + { + name: "StrongAffinityPicksY", + nodes: []nodeInfo{{PublicKey: "72aaaaaaaaaa", Role: "repeater", Name: "candX", HasGPS: true, Lat: 35.3, Lon: -120.7}, {PublicKey: "72bbbbbbbbbb", Role: "repeater", Name: "candY", HasGPS: true, Lat: 34.0, Lon: -118.2}}, + ctxPK: "ccccccccccc1", + seeds: []seed{{"ccccccccccc1", "72aaaaaaaaaa", "72", 1}, {"ccccccccccc1", "72bbbbbbbbbb", "72", 100}}, + wantName: "candY", + wantMethod: "neighbor_affinity", + }, + { + // Strong edge to candX exists but is flagged Ambiguous → tier 1 + // must skip it and tier 2 (geo) picks candY (near anchor). + name: "AmbiguousEdgeSkipsToTier2", + nodes: tier1StdNodes, + ctxPK: tier1Anchor, + seeds: []seed{{tier1Anchor, "72aaaaaaaaaa", "72", 100}}, + markAmbiguous: [2]string{tier1Anchor, "72aaaaaaaaaa"}, + wantName: "candY", + wantMethod: "geo_proximity", + }, + { + // candX is far (affinity), candY is geo-close. Tier 1 firing + // → candX wins. Sentinel for "geo branch hit first" regressions. + name: "BeatsTier2WhenBothSignal", + nodes: tier1StdNodes, + ctxPK: tier1Anchor, + seeds: []seed{{tier1Anchor, "72aaaaaaaaaa", "72", 100}}, + wantName: "candX", + wantMethod: "neighbor_affinity", + }, + { + // Graph is non-nil but has no edges involving the context. + // Tier 1 must short-circuit; tier 2 picks candY. + name: "EmptyGraphFallsThrough", + nodes: tier1StdNodes, + ctxPK: tier1Anchor, + extraGraphSeed: &seed{"aaaaaaaaaaa1", "aaaaaaaaaaa2", "aa", 10}, + wantName: "candY", + wantMethod: "geo_proximity", + }, + { + // Graph is nil — `graph != nil` short-circuit; tier 2 decides. + name: "NilGraphFallsThrough", + nodes: tier1StdNodes, + ctxPK: tier1Anchor, + useNilGraph: true, + wantName: "candY", + wantMethod: "geo_proximity", + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + pm := buildPrefixMap(tc.nodes) + var g *NeighborGraph + if !tc.useNilGraph { + g = NewNeighborGraph() + for _, s := range tc.seeds { + seedAffinity(g, s.obsPK, s.candPK, s.prefix, "obs1", s.count) + } + if tc.extraGraphSeed != nil { + s := *tc.extraGraphSeed + seedAffinity(g, s.obsPK, s.candPK, s.prefix, "obs1", s.count) + } + if tc.markAmbiguous[0] != "" { + // Use the public helper rather than mutating + // *NeighborEdge fields returned from AllEdges() — + // hardens the test against any future change that + // makes AllEdges() return copies. + if !g.MarkAmbiguous(tc.markAmbiguous[0], tc.markAmbiguous[1], true) { + t.Fatalf("MarkAmbiguous(%s,%s): edge not found", tc.markAmbiguous[0], tc.markAmbiguous[1]) + } + } + } + + r, method, _ := pm.resolveWithContext("72", []string{tc.ctxPK}, g) + if r == nil { + t.Fatal("expected non-nil candidate") + } + if r.Name != tc.wantName { + t.Fatalf("name: want %s got %s (method=%s)", tc.wantName, r.Name, method) + } + if method != tc.wantMethod { + t.Fatalf("method: want %s got %s", tc.wantMethod, method) + } + }) + } +} + +// TestResolveWithContext_Tier1_ScoresTooCloseFallsThrough: best.score is +// below affinityConfidenceRatio × runner-up.score (the ratio guard at the +// end of the tier-1 block in resolveWithContext). Resolver must fall +// through to tier 2. +// +// This case is kept SEPARATE from the table above because it asserts an +// extra invariant the others don't: the returned `score` field MUST be 0 +// (tier-2 geo path returns score=0 in store.go). Pinning score==0 makes +// the test fail loudly if affinityConfidenceRatio is ever lowered to a +// value (≤1.25) where the 10/8 count ratio would actually clear tier 1 — +// at that point the resolver would return a non-zero affinity score and +// this assertion catches it, even before the wantMethod string check. +func TestResolveWithContext_Tier1_ScoresTooCloseFallsThrough(t *testing.T) { + pm := buildPrefixMap(tier1StdNodes) + g := NewNeighborGraph() + // Both above affinityMinObservations, but within 3× of each other → + // ratio guard fails, fall-through expected. + seedAffinity(g, tier1Anchor, "72aaaaaaaaaa", "72", "obs1", 10) + seedAffinity(g, tier1Anchor, "72bbbbbbbbbb", "72", "obs1", 8) + + r, method, score := pm.resolveWithContext("72", []string{tier1Anchor}, g) + if r == nil { + t.Fatal("expected non-nil candidate") + } + // Direct pin on score==0: catches a lowered affinityConfidenceRatio + // constant that would let 10/8 clear the ratio guard and return a + // non-zero affinity score. + if score != 0 { + t.Fatalf("expected tier-2 fall-through (score==0); got score=%f via %s — affinityConfidenceRatio (%v) may have been lowered to admit a 1.25× ratio", + score, method, affinityConfidenceRatio) + } + if method == "neighbor_affinity" { + t.Fatalf("tier 1 must fall through when scores are too close (< %v ratio); got method=%s", + affinityConfidenceRatio, method) + } + if r.Name != "candY" { + t.Fatalf("expected tier-2 geo to pick candY; got %s via %s", r.Name, method) + } +} diff --git a/cmd/server/neighbor_graph.go b/cmd/server/neighbor_graph.go index 4e597180..780dfc1f 100644 --- a/cmd/server/neighbor_graph.go +++ b/cmd/server/neighbor_graph.go @@ -115,6 +115,27 @@ func (g *NeighborGraph) AllEdges() []*NeighborEdge { return out } +// MarkAmbiguous flips the Ambiguous flag on the edge between pubkeyA and +// pubkeyB (key direction-agnostic) to the supplied value. Returns true if +// the edge existed and was updated. +// +// This helper exists so tests don't have to mutate *NeighborEdge fields +// returned from AllEdges()/Neighbors() — those mutations work today only +// because the map stores pointers, which is a hidden coupling. Routing +// the flip through a method makes the intent explicit and lets the graph +// take its own write-lock. +func (g *NeighborGraph) MarkAmbiguous(pubkeyA, pubkeyB string, ambiguous bool) bool { + g.mu.Lock() + defer g.mu.Unlock() + key := makeEdgeKey(strings.ToLower(pubkeyA), strings.ToLower(pubkeyB)) + e, ok := g.edges[key] + if !ok { + return false + } + e.Ambiguous = ambiguous + return true +} + // IsStale returns true if the graph cache has expired. func (g *NeighborGraph) IsStale() bool { g.mu.RLock()