fix(#1199): 6 deferred quality items from PR #1198 r2 review (#1200)

Red commit: 75563ce (CI run: pending — pushed at branch open)

Follows up PR #1198 round-2 adversarial review (issue #1199). Six
robustness / perf-hot-path / maintenance items, one commit per logical
change. Stacked on top of `fix/issue-1197` (PR #1198) — base must move
to `master` after #1198 merges.

| # | Item | Commit(s) | Discipline |
|---|---|---|---|
| 1 | Brittle static-grep regex → go/parser AST walk in
`resolve_context_callsites_test.go` | 33d80b6 (RED) → 450236d (GREEN) |
red→green |
| 2 | `computeAnalyticsTopology` double-pass filter → materialize
`filteredTxs` once | 00005f6 | refactor |
| 3 | `BenchmarkBuildAggregateHopContextPubkeys` baseline + tiny smoke
test | b520048 | net-new bench/test |
| 4 | `hopResolverPerTx` CONCURRENCY doc — single-goroutine invariant |
155ff07 | doc-only |
| 5 | `schemaDegradationLogged` package-level `sync.Map` → PacketStore
field | 75563ce (RED) → 7dbf193 (GREEN) | red→green |
| 6 | `buildHopContextPubkeys` `out` slice cap hint (`make([]string, 0,
16)`) | 2040962 | refactor |

Items 2 & 6 are pure refactors — no test files modified for items 2 & 6
(per AGENTS.md exemption rule). Existing tests stay green and unaltered.

Item 4 is doc-only (CONCURRENCY: comment); no behavior change.

Item 3 adds a bench + a smoke assertion for the aggregate helper that
previously had no coverage. Local arm64 baseline: ~72ms/op, 130k allocs
at 5k txs.

Items 1 & 5 follow red→green: 33d80b6 demonstrates the regex blindspot
via a synthetic AST-detectable input the regex misses; 75563ce
demonstrates per-store log dedup leaks across instances. Both flips
visible in branch history.

Full `go test ./cmd/server/...` runs clean post-amend.

Fixes #1199

---------

Co-authored-by: openclaw-bot <bot@openclaw.local>

cmd/server/hop_context_bench_test.go

@@ -43,3 +43,100 @@ func BenchmarkBuildHopContextPubkeys(b *testing.B) {
 		_ = buildHopContextPubkeys(tx, pm)
 	}
 }
+
+// BenchmarkBuildAggregateHopContextPubkeys exercises the aggregate context
+// builder at the hot scale called out by #1197 (subpath/topology bulk
+// aggregations): ~5k txs sharing a node pool of ~50 prefixes. The aggregate
+// builder unions per-tx contexts with its own dedupe map; this benchmark
+// gives us a baseline so a future regression (e.g. accidental O(n²) dedupe)
+// shows up immediately. No assertion threshold yet — see #1199 item 3.
+func BenchmarkBuildAggregateHopContextPubkeys(b *testing.B) {
+	const numNodes = 50
+	const numTxs = 5000
+
+	nodes := make([]nodeInfo, 0, numNodes)
+	for i := 0; i < numNodes; i++ {
+		nodes = append(nodes, nodeInfo{
+			PublicKey:        fmt.Sprintf("%012x", i*0x101010101),
+			Role:             "repeater",
+			Name:             fmt.Sprintf("N%d", i),
+			ObservationCount: i * 3,
+			Lat:              37.0 + float64(i)*0.01,
+			Lon:              -122.0 - float64(i)*0.01,
+			HasGPS:           true,
+		})
+	}
+	pm := buildPrefixMap(nodes)
+
+	txs := make([]*StoreTx, 0, numTxs)
+	for i := 0; i < numTxs; i++ {
+		hops := []string{
+			nodes[(i+1)%numNodes].PublicKey[:6],
+			nodes[(i+3)%numNodes].PublicKey[:6],
+			nodes[(i+5)%numNodes].PublicKey[:6],
+			nodes[(i+7)%numNodes].PublicKey[:6],
+			nodes[(i+9)%numNodes].PublicKey[:6],
+			nodes[(i+11)%numNodes].PublicKey[:6],
+		}
+		pathJSON, _ := json.Marshal(hops)
+		decoded, _ := json.Marshal(map[string]interface{}{
+			"pubKey": fmt.Sprintf("cc%010x", i),
+		})
+		txs = append(txs, &StoreTx{
+			PathJSON:    string(pathJSON),
+			DecodedJSON: string(decoded),
+			ObserverID:  fmt.Sprintf("dd%010x", i%32),
+		})
+	}
+
+	b.ReportAllocs()
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		_ = buildAggregateHopContextPubkeys(txs, pm)
+	}
+}
+
+// TestBuildAggregateHopContextPubkeysSmoke is a tiny correctness anchor for
+// the aggregate helper: union over per-tx contexts, deduped. Lives next to
+// the benchmark so the file ships an assertion (preflight gate). See #1199
+// item 3.
+func TestBuildAggregateHopContextPubkeysSmoke(t *testing.T) {
+	pm := buildPrefixMap([]nodeInfo{{PublicKey: "aabbccddeeff"}})
+	d1, _ := json.Marshal(map[string]interface{}{"pubKey": "1111111111"})
+	d2, _ := json.Marshal(map[string]interface{}{"pubKey": "2222222222"})
+	d3, _ := json.Marshal(map[string]interface{}{"pubKey": "1111111111"}) // dup
+	txs := []*StoreTx{
+		{DecodedJSON: string(d1)},
+		{DecodedJSON: string(d2)},
+		{DecodedJSON: string(d3)},
+	}
+	got := buildAggregateHopContextPubkeys(txs, pm)
+	if len(got) != 2 {
+		t.Fatalf("expected 2 deduped pubkeys, got %d (%v)", len(got), got)
+	}
+	// Content assertion — proves dedup actually keeps the right pubkeys
+	// (not just any 2). Without this the test would pass even if dedup
+	// returned, e.g., one pubkey twice or two unrelated pubkeys. See
+	// #1199 r1 review (adv #1).
+	wantSet := map[string]bool{"1111111111": true, "2222222222": true}
+	gotSet := map[string]bool{}
+	for _, pk := range got {
+		gotSet[pk] = true
+	}
+	for pk := range wantSet {
+		if !gotSet[pk] {
+			t.Fatalf("expected pubkey %q in deduped result, got %v", pk, got)
+		}
+	}
+	for pk := range gotSet {
+		if !wantSet[pk] {
+			t.Fatalf("unexpected pubkey %q in deduped result, got %v", pk, got)
+		}
+	}
+	if buildAggregateHopContextPubkeys(nil, pm) != nil {
+		t.Fatalf("nil tx slice must yield nil")
+	}
+	if buildAggregateHopContextPubkeys(txs, nil) != nil {
+		t.Fatalf("nil prefix map must yield nil")
+	}
+}

cmd/server/resolve_context_callsites_test.go

@@ -1,25 +1,111 @@
 package main
 
 import (
-	"os"
+	"bytes"
+	"fmt"
+	"go/ast"
+	"go/parser"
+	"go/printer"
+	"go/token"
 	"path/filepath"
-	"regexp"
+	"sort"
 	"strings"
 	"testing"
 )
 
-// TestAllResolveWithContextCallSitesPassNonNilContext is a static grep gate
-// against #1197: every call to pm.resolveWithContext(...) in production code
-// (any non-test *.go file under cmd/server/) must pass a non-nil context
-// argument. Reverting any one call site to `nil` would silently re-introduce
-// the regression #1197 is meant to prevent.
+// minPMResolveWithContextCallSites is a floor on how many production-code
+// call sites of `pm.resolveWithContext(...)` the AST walker must find. If
+// the selector matcher is accidentally narrowed (e.g. typo in the receiver
+// name, or refactor that renames the method) the count will drop below the
+// floor and the test will fail loudly instead of silently passing with
+// zero offenders. Bump this if legitimate call sites are added/removed.
+const minPMResolveWithContextCallSites = 3
+
+// nilContextOffender describes a `pm.resolveWithContext(x, nil, ...)` call
+// site found in production code. file is the source filename, line the 1-based
+// line number, text a stable rendering of arg2 (always "nil" today, but kept
+// for future expansion to other forbidden expressions).
+type nilContextOffender struct {
+	file string
+	line int
+	text string
+}
+
+// findPMResolveNilContextOffenders walks one parsed *ast.File and returns
+// every call site of `pm.resolveWithContext(...)` whose second argument is
+// the identifier `nil`. The selector receiver is constrained to the literal
+// identifier `pm` to prevent the matcher from accidentally firing on
+// unrelated types that happen to expose a `resolveWithContext` method.
 //
-// Scope rationale: the original gate only scanned store.go and missed
-// routes.go:1428 (handleNodePaths) which still passed `nil`. Extending the
-// scope to all production *.go files in cmd/server/ closes that hole.
+// Returns offenders, total matched call sites (including non-offenders), and
+// any first-encountered error (currently unused, reserved for future
+// expansion). totalCallSites is reported separately so callers can enforce
+// a floor — see minPMResolveWithContextCallSites.
+func findPMResolveNilContextOffenders(fset *token.FileSet, file *ast.File, filename string) (offenders []nilContextOffender, totalCallSites int) {
+	ast.Inspect(file, func(n ast.Node) bool {
+		ce, ok := n.(*ast.CallExpr)
+		if !ok {
+			return true
+		}
+		sel, ok := ce.Fun.(*ast.SelectorExpr)
+		if !ok || sel.Sel == nil || sel.Sel.Name != "resolveWithContext" {
+			return true
+		}
+		// Constrain receiver to the literal identifier `pm`. This prevents
+		// drive-by matches on `foo.resolveWithContext(...)` for any other
+		// type. See #1199 review (adv #3).
+		recv, ok := sel.X.(*ast.Ident)
+		if !ok || recv.Name != "pm" {
+			return true
+		}
+		if len(ce.Args) < 2 {
+			return true
+		}
+		totalCallSites++
+		arg2 := ce.Args[1]
+		id, ok := arg2.(*ast.Ident)
+		if !ok || id.Name != "nil" {
+			return true
+		}
+		pos := fset.Position(ce.Pos())
+		offenders = append(offenders, nilContextOffender{
+			file: filename,
+			line: pos.Line,
+			text: renderExpr(fset, arg2),
+		})
+		return true
+	})
+	return offenders, totalCallSites
+}
+
+// renderExpr round-trips an ast.Expr back to source text via go/printer so
+// the failure message names the real expression (e.g. `nil`, `getCtx()`,
+// `someVar`) instead of an ast type tag. Falls back to a Go-syntax
+// description if printing fails.
+func renderExpr(fset *token.FileSet, e ast.Expr) string {
+	var buf bytes.Buffer
+	if err := printer.Fprint(&buf, fset, e); err != nil {
+		return fmt.Sprintf("<unprintable %T: %v>", e, err)
+	}
+	return buf.String()
+}
+
+// TestAllResolveWithContextCallSitesPassNonNilContext is a static AST-based
+// gate against #1197/#1199: every call to pm.resolveWithContext(...) in
+// production code (any non-test *.go file under cmd/server/) must pass a
+// non-nil context as the second argument. Reverting any one call site to
+// `nil` would silently re-introduce the regression #1197 is meant to prevent.
+//
+// History: the original gate (issue #1197) was a regex grep that split on
+// the first comma. Issue #1199 (item 1) showed that input like
+// `pm.resolveWithContext(getHop(a, b), nil, graph)` slipped past — the regex
+// captured `b)` as arg2. Same hazard for any gofmt-induced multi-line
+// reflow. This test now uses go/parser to walk the AST: arg2 is the SECOND
+// formal argument by position, robust against nesting and formatting.
 //
 // Allowed exceptions: callers that must pass nil (currently none in
-// production code) should be enumerated in `allowedNilCallers` below.
+// production code) should be enumerated in `allowedNilCallers` below by
+// "<file>:<line>".
 func TestAllResolveWithContextCallSitesPassNonNilContext(t *testing.T) {
 	allowedNilCallers := map[string]bool{
 		// "<file>:<line>": true,
@@ -30,64 +116,160 @@ func TestAllResolveWithContextCallSitesPassNonNilContext(t *testing.T) {
 		t.Fatalf("glob *.go: %v", err)
 	}
 
-	// Match: resolveWithContext(<arg1>, <arg2>, ...) — capture arg2.
-	re := regexp.MustCompile(`resolveWithContext\s*\(\s*([^,]+?)\s*,\s*([^,]+?)\s*,`)
-
-	var offenders []string
+	var offenders []nilContextOffender
 	totalCallSites := 0
 	scannedFiles := 0
+	fset := token.NewFileSet()
 	for _, f := range files {
 		// Skip *_test.go (unit tests legitimately pass nil for fixture-driven
 		// behavior) and the test scaffold itself.
 		if strings.HasSuffix(f, "_test.go") {
 			continue
 		}
-		body, err := os.ReadFile(f)
+		af, err := parser.ParseFile(fset, f, nil, parser.SkipObjectResolution)
 		if err != nil {
-			t.Fatalf("read %s: %v", f, err)
+			t.Fatalf("parse %s: %v", f, err)
 		}
 		scannedFiles++
-		src := string(body)
-		matches := re.FindAllStringSubmatchIndex(src, -1)
-		for _, m := range matches {
-			totalCallSites++
-			full := src[m[0]:m[1]]
-			arg2 := strings.TrimSpace(src[m[4]:m[5]])
-			if arg2 != "nil" {
+		fileOffenders, fileTotal := findPMResolveNilContextOffenders(fset, af, f)
+		totalCallSites += fileTotal
+		for _, o := range fileOffenders {
+			key := fmt.Sprintf("%s:%d", o.file, o.line)
+			if allowedNilCallers[key] {
 				continue
 			}
-			line := 1 + strings.Count(src[:m[0]], "\n")
-			site := f + ":" + itoa(line)
-			if allowedNilCallers[site] {
-				continue
-			}
-			offenders = append(offenders, site+" — "+full)
+			offenders = append(offenders, o)
 		}
 	}
 
 	if scannedFiles == 0 {
 		t.Fatalf("no production *.go files scanned — test scaffold broken")
 	}
-	if totalCallSites == 0 {
-		t.Fatalf("no resolveWithContext call sites found across %d files — test scaffold broken", scannedFiles)
+	if totalCallSites < minPMResolveWithContextCallSites {
+		t.Fatalf("found only %d pm.resolveWithContext call site(s) across %d files "+
+			"(floor is %d) — selector matcher likely too narrow, or call sites were "+
+			"removed without updating the floor",
+			totalCallSites, scannedFiles, minPMResolveWithContextCallSites)
 	}
 	if len(offenders) > 0 {
+		sort.Slice(offenders, func(i, j int) bool {
+			if offenders[i].file != offenders[j].file {
+				return offenders[i].file < offenders[j].file
+			}
+			return offenders[i].line < offenders[j].line
+		})
+		var lines []string
+		for _, o := range offenders {
+			lines = append(lines, fmt.Sprintf("%s:%d — arg2=%s", o.file, o.line, o.text))
+		}
 		t.Fatalf("found %d call site(s) of pm.resolveWithContext that pass nil context "+
 			"(re-introduces regression #1197 — must pass non-nil contextPubkeys):\n  %s",
-			len(offenders), strings.Join(offenders, "\n  "))
+			len(offenders), strings.Join(lines, "\n  "))
 	}
 }
 
-func itoa(i int) string {
-	if i == 0 {
-		return "0"
-	}
-	var b [20]byte
-	pos := len(b)
-	for i > 0 {
-		pos--
-		b[pos] = byte('0' + i%10)
-		i /= 10
-	}
-	return string(b[pos:])
+// TestFindPMResolveNilContextOffenders_SelfTest is the anti-tautology guard
+// for the AST walker (#1199 r1 kent MF-1). The deleted regex blindspot test
+// served the same purpose for the old regex matcher: if the matcher quietly
+// stops detecting violations, the production gate above will pass vacuously.
+// This test feeds the walker a synthetic Go source string with a known mix
+// of clean and violating call sites and asserts the walker flags exactly
+// the violators — no more, no less.
+//
+// If the walker is broken (e.g. selector predicate inverted, arg2 index
+// off-by-one, nil-Ident check removed), this test fails. If the walker's
+// selector is broadened (e.g. accepts any receiver), the negative cases for
+// `other.resolveWithContext(h, nil, g)` and `Foo.resolveWithContext(h, nil, g)`
+// will start being flagged and the assertion below will fail.
+func TestFindPMResolveNilContextOffenders_SelfTest(t *testing.T) {
+	src := `package fake
+
+func _() {
+	var pm *prefixMap
+	var other *prefixMap
+	var h string
+	var ctx []string
+	var g interface{}
+
+	// CLEAN — must NOT be flagged.
+	pm.resolveWithContext(h, ctx, g)
+	pm.resolveWithContext(getHop("a", "b"), ctx, g)
+
+	// VIOLATING — must be flagged.
+	pm.resolveWithContext(h, nil, g)
+	pm.resolveWithContext(getHop("a", "b"), nil, g)
+
+	// NON-pm receiver — must NOT be flagged (selector constrained to pm).
+	other.resolveWithContext(h, nil, g)
+	Foo{}.resolveWithContext(h, nil, g)
+
+	// Different method name — must NOT be flagged.
+	pm.resolveSomethingElse(h, nil, g)
+}
+
+func getHop(a, b string) string { return a + b }
+
+type prefixMap struct{}
+func (p *prefixMap) resolveWithContext(h string, ctx []string, g interface{}) {}
+func (p *prefixMap) resolveSomethingElse(h string, ctx []string, g interface{}) {}
+
+type Foo struct{}
+func (Foo) resolveWithContext(h string, ctx []string, g interface{}) {}
+`
+
+	fset := token.NewFileSet()
+	af, err := parser.ParseFile(fset, "synthetic.go", src, parser.SkipObjectResolution)
+	if err != nil {
+		t.Fatalf("parse synthetic source: %v", err)
+	}
+
+	offenders, totalCallSites := findPMResolveNilContextOffenders(fset, af, "synthetic.go")
+
+	// Expect 4 pm.resolveWithContext call sites total (2 clean + 2 nil),
+	// of which 2 are nil-context offenders. The two non-pm receivers and
+	// the resolveSomethingElse call MUST be ignored.
+	const wantTotal = 4
+	const wantOffenders = 2
+	if totalCallSites != wantTotal {
+		t.Errorf("totalCallSites = %d, want %d (selector should match pm.resolveWithContext only)",
+			totalCallSites, wantTotal)
+	}
+	if len(offenders) != wantOffenders {
+		t.Errorf("len(offenders) = %d, want %d", len(offenders), wantOffenders)
+		for _, o := range offenders {
+			t.Logf("  offender: %s:%d arg2=%s", o.file, o.line, o.text)
+		}
+	}
+
+	// Both flagged offenders must render arg2 as the literal text "nil"
+	// (proves renderExpr is round-tripping ast → source, not returning a
+	// type tag like "*ast.Ident").
+	for _, o := range offenders {
+		if o.text != "nil" {
+			t.Errorf("offender at line %d: arg2 text = %q, want %q", o.line, o.text, "nil")
+		}
+	}
+}
+
+// TestRenderExprRoundTripsSource is a focused assertion that renderExpr
+// uses go/printer (not %T) — guards against regressing exprText back to
+// the dead-branch state that always returned the type name "*ast.Ident".
+func TestRenderExprRoundTripsSource(t *testing.T) {
+	cases := []struct{ src, want string }{
+		{"nil", "nil"},
+		{"ctx", "ctx"},
+		{`getHop("a", "b")`, `getHop("a", "b")`},
+		{"foo.bar", "foo.bar"},
+	}
+	fset := token.NewFileSet()
+	for _, tc := range cases {
+		expr, err := parser.ParseExprFrom(fset, "expr.go", tc.src, parser.SkipObjectResolution)
+		if err != nil {
+			t.Fatalf("parse %q: %v", tc.src, err)
+		}
+		got := renderExpr(fset, expr)
+		if got != tc.want {
+			t.Errorf("renderExpr(%q) = %q, want %q", tc.src, got, tc.want)
+		}
+	}
 }

cmd/server/schema_degradation_per_store_test.go

@@ -0,0 +1,43 @@
+package main
+
+import (
+	"bytes"
+	"log"
+	"strings"
+	"testing"
+)
+
+// TestSchemaDegradationLogIsPerStore asserts that two independent
+// PacketStore instances both emit a schema-degradation warning for the
+// same message. With the (pre-#1199) package-level sync.Map, the second
+// instance silently swallows the warning — that is order-dependent test
+// pollution and is exactly what item 5/6 of #1199 calls out.
+//
+// RED: today, only the first store logs; the second is suppressed by the
+// package-level sentinel. GREEN follow-up moves the sentinel to a
+// PacketStore field so each instance has a fresh dedupe set.
+func TestSchemaDegradationLogIsPerStore(t *testing.T) {
+	var buf bytes.Buffer
+	prev := log.Writer()
+	prevFlags := log.Flags()
+	log.SetOutput(&buf)
+	log.SetFlags(0)
+	t.Cleanup(func() {
+		log.SetOutput(prev)
+		log.SetFlags(prevFlags)
+	})
+
+	const msg = "test-schema-degradation-marker-1199"
+
+	s1 := &PacketStore{}
+	s2 := &PacketStore{}
+	s1.logSchemaDegradationOnce(msg)
+	s2.logSchemaDegradationOnce(msg)
+
+	hits := strings.Count(buf.String(), msg)
+	if hits != 2 {
+		t.Fatalf("expected 2 log emissions (one per PacketStore), got %d. "+
+			"package-level sentinel pollutes across instances — move to a "+
+			"struct field. log buffer:\n%s", hits, buf.String())
+	}
+}

cmd/server/store.go

@@ -173,6 +173,9 @@ type PacketStore struct {
 	nodeCache     []nodeInfo
 	nodePM        *prefixMap
 	nodeCacheTime time.Time
+	// Per-store dedupe set for one-shot schema-degradation warnings. Field
+	// (not package-level) so each test gets a fresh state — see #1199 item 5.
+	schemaDegradationLogged sync.Map
 	// Precomputed subpath index: raw comma-joined hops → occurrence count.
 	// Built during Load(), incrementally updated on ingest. Avoids full
 	// packet iteration at query time (O(unique_subpaths) vs O(total_packets)).
@@ -3440,7 +3443,7 @@ func buildHopContextPubkeys(tx *StoreTx, pm *prefixMap) []string {
 		return nil
 	}
 	seen := make(map[string]struct{}, 16)
-	var out []string
+	out := make([]string, 0, 16)
 	add := func(pk string) {
 		if pk == "" {
 			return
@@ -3537,6 +3540,14 @@ func buildAggregateHopContextPubkeys(txs []*StoreTx, pm *prefixMap) []string {
 // by all per-tx distance/topology loops to avoid 4× duplicate closure
 // definitions and per-tx map allocation. See #1197 (adversarial r1 #7,
 // carmack r1 #3).
+//
+// CONCURRENCY: NOT safe for concurrent use. The returned closures share
+// mutable captured state — `contextPubkeys` is reassigned by setContext and
+// read by resolveHop, and `hopCache` is mutated by both (resolveHop writes
+// on miss, setContext clears wholesale). Callers MUST invoke both functions
+// from a single goroutine for the lifetime of the (resolveHop, setContext)
+// pair. If a future caller fans out per-tx work across goroutines, allocate
+// a fresh resolver pair per goroutine. See #1199 item 4.
 func (s *PacketStore) hopResolverPerTx(pm *prefixMap) (resolveHop func(string) *nodeInfo, setContext func([]string)) {
 	hopCache := make(map[string]*nodeInfo, 16)
 	var contextPubkeys []string
@@ -4908,13 +4919,12 @@ type nodeInfo struct {
 	ObservationCount int // count of advertisements/observations; used for tier-3 tiebreak in resolveWithContext
 }
 
-// schemaDegradationLogged tracks one-shot schema degradation warnings so we
-// don't spam the log on every getAllNodes() call. See #1197 (adversarial r1
-// #10).
-var schemaDegradationLogged sync.Map
+// schemaDegradationLogged is now a PacketStore field (see type definition) so
+// each store/test instance has a fresh dedupe set. Issue #1199 item 5: the
+// prior package-level sync.Map silently suppressed re-emission across tests.
 
 func (s *PacketStore) logSchemaDegradationOnce(msg string) {
-	if _, loaded := schemaDegradationLogged.LoadOrStore(msg, true); !loaded {
+	if _, loaded := s.schemaDegradationLogged.LoadOrStore(msg, true); !loaded {
 		log.Printf("[store] schema-degradation: %s", msg)
 	}
 }
@@ -5303,6 +5313,31 @@ func (s *PacketStore) computeAnalyticsTopology(region string, window TimeWindow)
 	allNodes, pm := s.getCachedNodesAndPM()
 	_ = allNodes // only pm is needed for topology
 
+	// Materialize the filtered tx slice ONCE — both the context-build pass
+	// and the main aggregation pass need the same window+region predicate.
+	// Two scans of s.packets re-running identical predicates is wasteful at
+	// the 30k+ packet hot-path scale (#1199 item 2). One filter, two passes
+	// over the result.
+	filteredTxs := make([]*StoreTx, 0, len(s.packets))
+	for _, tx := range s.packets {
+		if !window.Includes(tx.FirstSeen) {
+			continue
+		}
+		if regionObs != nil {
+			match := false
+			for _, obs := range tx.Observations {
+				if regionObs[obs.ObserverID] {
+					match = true
+					break
+				}
+			}
+			if !match {
+				continue
+			}
+		}
+		filteredTxs = append(filteredTxs, tx)
+	}
+
 	// Pre-pass: build the full hop-disambiguation context from all in-window
 	// txs BEFORE any resolveHop call. The earlier shape — populating
 	// contextPubkeys lazily during the main scan and reading it from a
@@ -5314,22 +5349,7 @@ func (s *PacketStore) computeAnalyticsTopology(region string, window TimeWindow)
 	var contextPubkeys []string
 	{
 		seen := make(map[string]struct{}, 64)
-		for _, tx := range s.packets {
-			if !window.Includes(tx.FirstSeen) {
-				continue
-			}
-			if regionObs != nil {
-				match := false
-				for _, obs := range tx.Observations {
-					if regionObs[obs.ObserverID] {
-						match = true
-						break
-					}
-				}
-				if !match {
-					continue
-				}
-			}
+		for _, tx := range filteredTxs {
 			for _, pk := range buildHopContextPubkeys(tx, pm) {
 				if _, ok := seen[pk]; ok {
 					continue
@@ -5358,28 +5378,11 @@ func (s *PacketStore) computeAnalyticsTopology(region string, window TimeWindow)
 	observerMap := map[string]string{} // observer_id → observer_name
 	perObserver := map[string]map[string]*struct{ minDist, maxDist, count int }{}
 
-	for _, tx := range s.packets {
-		if !window.Includes(tx.FirstSeen) {
-			continue
-		}
+	for _, tx := range filteredTxs {
 		hops := txGetParsedPath(tx)
 		if len(hops) == 0 {
 			continue
 		}
-		if regionObs != nil {
-			match := false
-			for _, obs := range tx.Observations {
-				if regionObs[obs.ObserverID] {
-					match = true
-					break
-				}
-			}
-			if !match {
-				continue
-			}
-		}
-
-		// Hop-disambiguation context was assembled in the pre-pass above (#1197).
 
 		n := len(hops)
 		hopCounts[n]++