mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-09-10 15:06:08 +00:00
efitenandClaude Opus 5
f081f91b88
Fixes #1904. ## The bug `buildPathHopIndex` reassigned `s.byPathHop` to a fresh map and refilled it from every packet's raw `path_json` hops: ```go func (s *PacketStore) buildPathHopIndex() { s.byPathHop = make(map[string][]*StoreTx, 4096) for _, tx := range s.packets { addTxToPathHopIndex(s.byPathHop, tx) // raw hops only } ... } ``` `byPathHop` carries two kinds of key, though: those raw wire hops, and the resolved full pubkeys fed per observation by `indexResolvedPathHops`. The pubkey strings behind the second kind are retained nowhere — #800 replaced the per-`StoreTx` `ResolvedPath` field with a hash-only membership index (`resolvedPubkeyIndex` stores FNV hashes, not strings) — so the rebuild could not reproduce them and dropped them. All three call sites run post-load: `LoadChunked` (`chunked_load.go:459`), the background fill loader (`store.go:1573`), and the deferred startup build (`index_ready_1008.go:177`). The `resolved_path` branch of the chunk scan populates the index and is then silently undone a few hundred lines later, while the `resolved_path IS NULL` fallback right beside it is explicitly documented as "byNode ONLY — the resolved_path/path-hop indexes must NOT be populated here". The two branches disagreed about who owns the index. Consequence: after a cold start every lookup keyed by a node's full pubkey missed, so `relay_count_1h/24h`, `last_relayed`, `unscoped_relay_count_24h`, `transported_scopes` (#1751) and the usefulness Traffic axis all read zero until live ingestion slowly refilled the index. ## Evidence Fixture built from live data: 2512 nodes, 17,056 transmissions, 528,891 observations, 123,057 of them carrying a non-NULL `resolved_path`. ``` before [store] Built path-hop index: 2924 unique keys /api/nodes → 0 of 2000 nodes with transported_scopes 0 with relay_count_24h > 0 after [store] Built path-hop index: 3881 unique keys (172181 resolved-hop entries retained) /api/nodes → 726 with transported_scopes 741 with relay_count_24h > 0 ``` The 957 extra keys are the full pubkeys. ## The change `retainResolvedPathHops` re-merges the pre-rebuild map's entries that the raw-hop pass cannot reproduce. Entries are carried over **only for transmissions still in `s.packets`**. That filter is load-bearing rather than defensive. `removeTxFromPathHopIndex` strips raw hops only — it derives them from `txGetParsedPath` — and its companion `removeFromResolvedPubkeyIndex` cleans the hash index, not `byPathHop`. So evicted transmissions linger under their resolved keys, and the wipe this PR removes was the only thing that ever cleared them. Filtering on liveness keeps the index bounded by the eviction policy instead of converting that gap into a permanent leak. `TestBuildPathHopIndex_DropsResolvedHopsOfEvictedTx_1904` pins it. (The eviction gap itself is pre-existing and outside this change: between rebuilds, an evicted transmission still stays referenced under its resolved keys. Filed separately.) ## Perf `O(entries in prev)` with one scratch map reused across keys (`clear()` per key, the same idiom as `hopsSeen`), plus one `map[*StoreTx]struct{}` over `s.packets` for the liveness check. It runs only where `buildPathHopIndex` already ran — cold load and background-fill completion — never on an ingest or request path. Measured on the fixture above: index build stayed within the same `LoadChunked` step, 15.2s total for 17k transmissions / 527k observations. Memory: the retained entries point at transmissions already held by `s.packets`, so no `StoreTx` is kept alive beyond eviction; the cost is map/slice overhead for keys that the feature is supposed to have. ## Tests `cmd/server/pathhop_rebuild_1904_test.go`, red before / green after: 1. `TestBuildPathHopIndex_RetainsResolvedHops_1904` — a resolved full-pubkey key survives the rebuild alongside the raw hop. 2. `TestBuildPathHopIndex_DropsResolvedHopsOfEvictedTx_1904` — a resolved key whose transmission is no longer in `s.packets` is dropped, and the now-empty key is not left behind. 3. `TestBuildPathHopIndex_NoDuplicateOnRepeatedBuild_1904` — building twice does not double-append (`indexResolvedPathHops` dedups within a call, not across the several observations of one transmission, so `prev` can legitimately contain duplicates). ``` cd cmd/server && go test ./... ok github.com/corescope/server 85.5s go vet ./... clean ``` Frontend and ingestor suites are untouched by this change (Go server only, no `public/` files). ## Interaction with #1903 Both touch `byPathHop` semantics, so I verified them composed on the same fixture. With #1904 alone the resolved keys come back and #1902's prefix collision is plainly visible again (51% of 1-byte prefix groups reporting an identical scope set). With both: ``` f79616 BE repeater ['#be','#de','#eu','#nl'] relay24h=542 f752c2 DE/NRW repeater ['#de','#de-nw'] relay24h=343 f788ad BE repeater none relay24h=383 ``` Identical-set prefix groups fall to 8%, relay counts stay intact, and each node's scopes match what its own `resolved_path` rows say. The two changes are independent and compose cleanly. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>