fix(analytics): recompute cached snapshots after the complete startup load

[dborup]

Relates to #116

Plan and design

The user asked for autonomous work, so the plan is written here instead of waiting for sign-off (AGENTS.md rule 5).

Commits:

1. 1a668004: tests. They do not compile on master (no StartupLoadDone, RecomputeNow, LastStartAt or ClockSkewEngine.Invalidate).
2. 0a463524: the fix.
3. 9996e08f: merge of origin/master (e829d1ea), which brings in fix(store): lazy distance build — drop the reset sync.Once, fix the lock order (#149) #151 (the fix(store): lazy distance build can crash (sync.Once reset during Do) and deadlock (lock order) #149 distance build lock fix). See "Sync with master" below.
4. 6d608a7a: gofmt of the startupLoadDone field block in store.go (review nit 5).
5. 9729a9be: test that a distance build drops region/area TTL results (review nit 4, mutant M8).

Sync with master (after #151)

origin/master e829d1ea was merged in with a merge commit; there were no conflicts. #151 replaced the sync.Once in the lazy distance build with the distLazyBuilding/distLazyBuilt state under distLazyMu, and moved the build into runDistanceIndexBuild, a loop that builds once more when the background load completed during a build.

Git placed this PR's hunk (clear distCache, then RecomputeNow() on the distance recomputer) inside that loop, after s.mu.Unlock() and before the distLazyMu section that sets distLazyBuilt = true. That is the right place:

• The build has already swapped in the new index, so the recompute reads it.
• It runs before the index reports built, so the handler never goes from 202 to an older snapshot or TTL result.
• It holds neither s.mu nor distLazyMu. computeAnalyticsDistance takes s.mu.RLock, and s.mu must never be taken while distLazyMu is held (fix(store): lazy distance build can crash (sync.Once reset during Do) and deadlock (lock order) #149). The lock-order guard does not follow calls into RecomputeNow, so this was checked by reading, and the fix(store): lazy distance build can crash (sync.Once reset during Do) and deadlock (lock order) #149 tests pass with -race -count=10.
• If a pass turns out stale and the loop builds again, the cache is cleared and the snapshot refreshed again for that pass.

Review finding 1 (P2, sync.Once reset while Do runs) is fixed on master by #151 and no longer applies to this branch.

Claims in the issue, verified against master

• main.go starts the recomputers right after FirstChunkReady. Start() computes on the partial store, and the next pass comes one interval (5 min) later.
• The RF/topology/channels gate used LoadComplete(), which flips at the end of the hot window, before loadBackgroundChunks fills the retention window.
• The lazy distance build did not refresh the distance snapshot.
• Reproduced locally, same database copy and config (retentionHours=720, hotStartupHours=1), after background load complete: 499/499:
  • master: /api/analytics/rf returns 200 with totalTransmissions: 213 (the hot-window snapshot), until the next 5-minute tick;
  • this branch: 499 right after the load.

Change

One-shot terminal signal. StartupLoadDone() (chunked_load.go) is closed by a deferred signalStartupLoadDone() when RunStartupLoad returns, on every path:

• success
• hotStartupHours=0
• retention disabled
• empty DB
• LoadChunked error
• background-fill failure

It is separate from LoadComplete() (end of the hot window) and from backgroundLoadDone/Failed. Those keep their health/coverage meaning: after a failed fill backgroundLoadDone stays false, but the signal still fires.

Dependent caches dropped before the signal closes:

• the hash-size info cache (15 s TTL)
• the clock-skew recompute throttle (30 s; new ClockSkewEngine.Invalidate)
• the region/window analytics TTL caches computed on the partial store

RecomputeNow() (analytics_recomputer.go). Runs a pass on the recomputer's own loop goroutine, so it never overlaps a periodic pass, and resets the ticker, so no periodic pass follows right behind it.

Post-load pass. StartAnalyticsRecomputers recomputes all nine current default-shape recomputers once, sequentially, after the signal:

1. rf, topology, channels (the gated ones)
2. distance, hash-collisions, hash-sizes, observers-clock-skew, nodes-clock-skew
3. roles, which reads the nodes-clock-skew snapshot, so it comes after it

One log line reports the per-recomputer durations.

Warm-up gate (Kpa-clawbot#1659). The gate is now the startup-load signal, sampled before each compute, so a pass that began on partial data never opens it. 503 + Retry-After and the 60 s force timeout are unchanged. A forced-open partial snapshot is replaced by the post-load pass.

Ungated endpoints. Their availability is unchanged (no new 503s); their snapshots are replaced right after the load.

Distance. Each pass of the lazy build (runDistanceIndexBuild) clears the distance TTL cache and refreshes the distance recomputer before reporting the index built, so the handler never goes from 202 to an older snapshot or to a region/area result computed from the older index.

How this differs from upstream Kpa-clawbot/CoreScope#2025

Upstream is read as a reference only; nothing was cherry-picked.

• Only the nine recomputers that exist in this fork (no retransmissions recomputer).
• The region/window analytics TTL caches (rfCache, topoCache, hashCache, collisionCache, chanCache, distCache, subpathCache, channels list) are also dropped at the signal. Upstream listed them as "not verified".
• The distance TTL cache is cleared on each lazy build.
• LastStartAt() exposes when a pass started, which the tests use to prove that passes start after the signal and run in the required order.

Acceptance criteria

Criterion	Status	Evidence
One-shot terminal signal, exactly once, on all startup-load exit paths	Met	TestStartupLoadDoneFiresOnEveryPath_116 (6 paths; a second signal is a no-op), TestStartupLoadDoneWaitsForBackgroundFill_116
Every current default-shape recomputer runs once after it	Met	TestEveryRecomputerRunsOnceAfterLoad_116 (all nine, exactly +1, started after the signal, gated first, roles after nodes-clock-skew)
Manual recomputation on the loop, resetting its ticker	Met	TestRecomputeNowRunsOnLoopAndResetsTicker_116 (no overlap, no immediate periodic pass, loop continues, returns on a stopped recomputer)
Gated endpoints cannot open on a pass that began before the signal; forced-open results replaced promptly	Met	TestWarmupGateIgnoresPassStartedBeforeLoad_116, TestForcedOpenSnapshotReplacedAfterLoad_116, end to end in TestGatedRFWaitsForBackgroundFill_116
Ungated endpoints keep current availability; no new 503	Met	TestGatedRFWaitsForBackgroundFill_116: hash-sizes, hash-collisions and roles return 200 during the load
Dependent caches/throttles invalidated before recompute	Met	TestStartupLoadDoneDropsDependentCaches_116
Distance snapshot refreshed before the index reports ready	Met	TestDistanceSnapshotRefreshedBeforeReady_116
Region/area distance results from the older index not served after ready	Met	TestDistanceBuildDropsRegionAreaCache_116
Background-load health/coverage semantics unchanged	Met	TestStartupLoadDoneFiresOnEveryPath_116 asserts backgroundLoadDone/Failed per path; existing RunStartupLoad/Kpa-clawbot#1690/Kpa-clawbot#1809 tests pass

Tests

• analytics_after_startup_load_116_test.go: 10 test functions (one a table test with 6 paths). They do not compile on master; on this branch all pass, including 10× under -race.
• New: TestDistanceBuildDropsRegionAreaCache_116. It holds the lazy build before it reads the dataset (the fix(store): lazy distance build can crash (sync.Once reset during Do) and deadlock (lock order) #149 distanceBuildHook gate) and caches SJC|, |BAY and SJC|BAY distance results from the index as it was before the build (0 hops). This models a request that passed the DistanceIndexBuilt() check just before an invalidation and cached its result after the startup signal had dropped the cache. After the build reports built, each key must return a fresh result equal to computeAnalyticsDistance on the built index (3 hops), not the cached map. Without the distCache reset in runDistanceIndexBuild it fails for all three keys (served the result cached from the pre-build index after the index reported built); TestDistanceSnapshotRefreshedBeforeReady_116 alone lets that mutant survive.
• Mutation check: 10 single-line mutations, all caught by named failing tests:
  • signal defer removed
  • hash-size cache not dropped
  • clock-skew throttle not reset
  • TTL caches not dropped
  • ticker not reset
  • gate not sampled before compute
  • post-load pass not started
  • gate back on LoadComplete
  • roles before nodes-clock-skew
  • distance not refreshed
  • distance TTL cache not cleared on build (added after review; M8 in the review)
• Changed existing test: TestAnalyticsRF_AfterFirstPassReturns200 now models "loaded" with signalStartupLoadDone() instead of setting loadComplete, because the gate's meaning changed as the issue requires. TestWarmup_GateBlockedUntilLoadComplete is unchanged; it passes its own gate function.
• go test -run "_116|Warmup|1659|RunStartupLoad|Recomputer|Distance|ClockSkew|StartupLoad|Chunked|1690|1809": ok.
• Full server -race suite (cd cmd/server && go test -race -count=1 ./...): ok github.com/corescope/server 1632.212s.
• After the sync: the fix(analytics): recompute cached snapshots after the complete startup load #116 tests, the fix(store): lazy distance build can crash (sync.Once reset during Do) and deadlock (lock order) #149 tests, the fix(store): lazy distance build can crash (sync.Once reset during Do) and deadlock (lock order) #149 lock-order and sync.Once guards and the Analytics cards show post-restart slice with 'All data' selected until multiple manual refreshes Kpa-clawbot/CoreScope#1659 warm-up tests with -race -count=10: ok. Full server suite with -race: see the latest follow-up comment.
• go vet is clean. store.go is gofmt-clean again. analytics_recomputer.go, analytics_warmup_1659.go and clock_skew.go were already not gofmt-clean on master; this PR's lines in them follow the surrounding style, and CI does not run gofmt.

Local run on a database copy

test-fixtures/e2e-fixture.db, freshened, with last_seen derived from first_seen; 499 transmissions, 214 in the last hour. Config: retentionHours=720, hotStartupHours=1.

[store] LoadChunked: 214 transmissions ... (DB total=214)
[store] background load complete: 499/499 packets in memory (coverage=100.0%)
[analytics-recompute] startup load done: recomputed 9 snapshots in 34ms (rf=1ms topology=17ms channels=1ms distance=1ms hash-collisions=5ms hash-sizes=9ms observers-clock-skew=0s nodes-clock-skew=0s roles=0s)

/api/analytics/rf → 200, totalTransmissions: 499. The master binary on the same copy gives 213.

Perf

• One extra compute per recomputer per process start, run sequentially so they do not all hold the store read lock at once.
• Ticker phases afterwards are offset by the post-load compute durations.
• The cache drops at the signal are map resets.

Not verified

• Timestamp and snapshot comparison on a large database copy, as the issue asks: the post-load recompute duration and lock behaviour at production size (100k+ transmissions). Only the 499-row fixture and synthetic tests were used here.
• Staging startup with a real hot/background split.

Overlap with other open PRs

• cmd/server/store.go: this PR changes runDistanceIndexBuild (one block) and one struct field. fix(store): preserve first_seen ordering when merging background chunks #137 (fix(store): preserve first_seen ordering when merging background chunks #114), fix(store): remove evicted transmissions from resolved byPathHop entries #144 (fix(store): remove evicted transmissions from resolved byPathHop entries #115) and fix(store): lazy distance build — drop the reset sync.Once, fix the lock order (#149) #151 (fix(store): lazy distance build can crash (sync.Once reset during Do) and deadlock (lock order) #149) are merged into master and included through the sync.
• cmd/server/chunked_load.go is changed only by this PR.
• The branch now contains origin/master e829d1ea.

Open review findings

• Finding 2 (P3): a tick that comes due during a RecomputeNow pass can still start a periodic pass right after it, because go.mod is go 1.22 and t.Reset does not drain t.C. Not changed here.
• Finding 3 (P3): the warm-up 503 window now spans the background fill, while the frontend retries for about 30 s. The trade-off the issue asks for; not changed here.
• Nit 4, M11 (stop closure without <-postLoadDone) and nit 6 (extra post-load pass when the load finishes before the recomputers start): not changed here.

🤖 Generated with Claude Code

https://claude.ai/code/session_019TcZHooUiiknVWbECVWzk8

---

Generated by Claude Code

[dborup]

Independent review of 0a463524

Verdict: APPROVE with nits. This is a recommendation only; merging is the owner's call. The fix does what issue #116 asks, and I reproduced it end to end. The one P2 below is a pre-existing crash on master. This PR makes its window a bit wider but does not introduce it. It should get its own issue rather than block this PR.

Reviewed head: 0a463524f0e5b553359e90b326099a887e35f6b6 (unchanged before and after the review). I worked on git archive trees of the head, of commit A (1a668004), of origin/master (5f493f1d), of the merge origin/master + head (tree e3e2bb4b), and of the stacked merge master → #137 → #144 → #145 (tree eaa12e48).

Labels: [F] freshly verified by me · [T] taken from the PR text · [A] assumption · [K] known limitation.

Findings

1. P2 (pre-existing, widened by this PR). The server crashes with fatal error: sync: unlock of unlocked mutex when the background fill resets distLazyOnce while a lazy distance build is still inside Do. cmd/server/store.go:1685 (s.distLazyOnce = sync.Once{} in loadBackgroundChunks), cmd/server/store.go:4694 (go s.distLazyOnce.Do(...)), cmd/server/store.go:4719 (new rc.RecomputeNow() inside that Do).
   • Scenario: during a restart, someone opens the Distance tab while the background fill is still running. The handler triggers the lazy build. Its Do is still running when loadBackgroundChunks overwrites the Once. doSlow's deferred o.m.Unlock() then runs on a fresh mutex, which is an unrecoverable runtime fatal that kills the process.
   • Reproduced on master [F]: I held the Do open with the existing distanceBuildHook while the fill completed (reviewer probe test, kept locally, not committed).
   • Reproduced on the head [F]: I held the new distance RecomputeNow open with a slow distance compute (reviewer probe test, kept locally, not committed).
   • What this PR changes: Do now also covers one full distance recompute, plus any wait behind a running distance pass. That compute needs s.mu.RLock, which the fill's final section holds, so the in-flight window grows by the distance compute time [F by reading; the timing effect is [A]].
   • Second effect, same root cause: even without the crash, a reset in that window is overwritten by distLazyBuilt = true for an index built before the fill finished. The handler only calls TriggerDistanceIndexBuild while !DistanceIndexBuilt(), so that stale index is never rebuilt.
   • Recommendation: open a follow-up issue. Do not reassign a sync.Once that may be running; for example, drop the Once and rely on distLazyBuilding, or add a build generation counter that the fill bumps.
2. P3. RecomputeNow can still be followed immediately by a periodic pass. cmd/server/analytics_recomputer.go:113, cmd/server/go.mod:3 (go 1.22).
   • Cause: with go 1.22 in go.mod, the runtime keeps the pre-1.23 timer semantics (asynctimerchan=1). A tick that comes due while the on-demand pass runs stays buffered in t.C, and t.Reset does not drain it.
   • Probe [F] (TestReviewProbe_StaleTickAfterRecomputeNow: interval 100 ms, on-demand pass 250 ms): a duplicate pass started about 0 µs after RecomputeNow returned. With GODEBUG=asynctimerchan=0 the same probe passes 3/3.
   • Impact: criterion 3 ("avoid an immediate duplicate pass") fails whenever a recomputer's tick falls inside its post-load or distance-build pass. That is rare at a 5 min interval but real.
   • Test gap: the PR's TestRecomputeNowRunsOnLoopAndResetsTicker_116 uses a 5 ms compute, so it cannot see this.
   • Fix: drain after the reset: select { case <-t.C: default: }.
3. P3. The warm-up 503 window now covers the whole background fill, but the frontend retries for only about 30 s. cmd/server/analytics_warmup_1659.go:77 (force-open after 60 s), public/app.js:171-183 on master (6 retries × Retry-After: 5 ≈ 30 s, then throw API 503).
   • Observed [F]: on a 51 K-transmission copy, RF and topology returned 503 + Retry-After: 5 for about 5 s during the fill, then 200 with the full store. Master served 200 with totalTransmissions: 212 right away.
   • Risk [A]: at production size, if the fill takes longer than about 30 s, opening Analytics right after a restart shows an error for RF, topology and channels until a reload. Before this PR it showed partial data.
   • Assessment: this is the trade-off the issue asks for. The 30 s vs 60 s mismatch comes from Analytics cards show post-restart slice with 'All data' selected until multiple manual refreshes Kpa-clawbot/CoreScope#1659. The PR's "Not verified" section does not mention this user-visible effect.
4. nit. Test gaps where mutants survived (details under Test-first and mutants):
   • M8, removing the distCache clear in the lazy build (store.go:4712-4714), survives. Under the current call graph it is almost equivalent: distLazyBuilt only goes false in loadBackgroundChunks, the handler answers 202 while not built, and the startup signal already drops distCache. The PR body still presents it as a feature.
   • M11, removing <-postLoadDone from the stop closure (analytics_recomputer.go:392), survives. The only consequence is a goroutine that briefly outlives stop().
5. nit. store.go is no longer gofmt-clean. cmd/server/store.go:516-521. The new startupLoadDone and startupLoadSignaled fields are aligned differently from the rest of the block. gofmt -l flags store.go on the head but not on master [F]. The PR body says the touched files "were already not gofmt-clean on master"; that is true for analytics_recomputer.go, analytics_warmup_1659.go and clock_skew.go, not for store.go.
6. nit. Extra post-load pass when the load finishes before StartAnalyticsRecomputers. analytics_recomputer.go:380. On the default fixture config (hot window off), the log shows the initial passes and then startup load done: recomputed 9 snapshots in 12ms right after them [F]. It costs one extra round per start (about 0.7 s at 51 K transmissions, see Performance). This is harmless, but "every recomputer runs once after the signal" becomes "twice" when the signal is already closed.

Metadata

• CI on 0a463524 is complete: Go Build & Test, Playwright E2E and Docker are SUCCESS; the rest are SKIPPED [F].

• Head at start 0a463524f0e5b553359e90b326099a887e35f6b6, at end the same (gh pr view 145 --json headRefOid) [F]. The PR is a draft.

• Commits in origin/master..head:

  • 1a668004 test: author and committer dborup <kontakt@meshview.dk> [F].
  • 0a463524 fix: author and committer dborup <kontakt@meshview.dk> [F].
  • Both carry a Co-Authored-By: Claude Opus 5.5 trailer.

• Merge base: d264716c. origin/master is now 5f493f1d; the 7 commits after the base (fix(analytics): treat lazy distance-index 202 responses as transient #120, fix(packets): Clear Filters must reset observer and type selection state #121) touch only frontend, JS tests, test-all.sh and deploy.yml, not cmd/. cmd/ in the merged tree is byte-identical to the head [F].

• git merge-tree --write-tree origin/master head: clean (e3e2bb4b) [F].

• Stacked master → fix(store): preserve first_seen ordering when merging background chunks #137 (1ca172d8) → fix(store): remove evicted transmissions from resolved byPathHop entries #144 (1477f579) → fix(analytics): recompute cached snapshots after the complete startup load #145: all three merges are clean (final tree eaa12e48) [F].

  • fix(store): preserve first_seen ordering when merging background chunks #137 changes loadChunk and fix(store): remove evicted transmissions from resolved byPathHop entries #144 changes eviction; fix(analytics): recompute cached snapshots after the complete startup load #145 changes TriggerDistanceIndexBuild and the struct. The hunks do not overlap.
  • On the stacked tree go vet is clean, and go test -race -run '_114|_115|_116|Warmup|1659|RunStartupLoad|Chunked|1690|1809|Evict' gives ok (38 s) [F].

• Files: 7 in cmd/server (+678/−46). deploy.yml is not touched [F].

• PR body [F]:

  • Relates to #116; no closing keyword; closingIssuesReferences: [].
  • No @mentions.
  • Upstream appears only as `Kpa-clawbot/CoreScope#2025` in code format, with no github.com/Kpa-clawbot URL.

Acceptance criteria (issue #116)

Criterion	Result
One-shot terminal signal fires exactly once on every startup-load exit path	Met [F]. defer s.signalStartupLoadDone() at chunked_load.go:210 is the first defer, so it runs last. The CompareAndSwap guard makes it one-shot. TestStartupLoadDoneFiresOnEveryPath_116 covers 6 paths plus a second call. Mutants M1 (defer removed) and M12 (signal before the fill) are caught.
Every current default-shape analytics recomputer runs once after that signal	Met for the nine in StartAnalyticsRecomputers [F]: recomputeWhenLoaded runs them in order rf, topology, channels, distance, hash-collisions, hash-sizes, observers-clock-skew, nodes-clock-skew, roles. Live log on the 51 K copy: recomputed 9 snapshots in 726ms. The /api/analytics/neighbor-graph response cache (Kpa-clawbot#1481) is not included; it reads s.graph from persisted neighbor_edges, not from the packet load, so it is unaffected in the normal case [A]. See nit 6 for the double pass.
Manual recompute runs on the loop and resets its ticker (no immediate duplicate)	Mostly met [F]. It runs on the loop goroutine, never overlaps, and resets the ticker. But a tick that comes due during the pass still triggers an immediate duplicate under go 1.22 timer semantics (finding 2).
Gated endpoints cannot open on a pass that began before the signal; force-open results replaced promptly	Met [F]. The gate is sampled before compute (analytics_recomputer.go:136), and the gate is now the startup signal instead of LoadComplete. Live, on the 51 K copy: RF 503 + Retry-After: 5 until the fill ended, then 200 with totalTransmissions: 51307. Master served 212 (hot window) as final and would keep doing so until the next 5 min tick. Mutants M2, M5 and M4 are caught.
Ungated endpoints keep their availability; no new 503	Met [F]. hash-sizes stayed 200 throughout the live startup poll. The test checks hash-sizes, hash-collisions and roles.
Dependent caches and throttles invalidated before recompute	Met [F]. The hash-size info cache, the clock-skew throttle (new ClockSkewEngine.Invalidate, clock_skew.go:225) and the region/window TTL caches are dropped before close. Mutants M6 and M9 are caught. The clock_skew.go change is in scope: Recompute returns early when the last run is under 30 s old (clock_skew.go:238), so without it the post-load nodes/observers-clock-skew pass could return the partial result.
Distance snapshot refreshed before the index reports ready	Met [F]. Live: distance 202 → 200. Mutant M7 is caught. See finding 1 for the widened pre-existing race in the same block.
Background-load health/coverage semantics unchanged	Met [F]. The table test asserts backgroundLoadDone/Failed per path, including a failed fill (done=false, failed=true, with the signal still fired). The existing RunStartupLoad, Kpa-clawbot#1690 and Kpa-clawbot#1809 tests pass.

Test-first and mutants

• Commit A adds only analytics_after_startup_load_116_test.go, and it does not compile on master [T]; go test of commit A fails to build [F]. The test file is byte-identical between A and the head [F].
• The only change to an existing test is analytics_warmup_1659_test.go: loadComplete.Store(true) becomes signalStartupLoadDone(). That is justified, because the meaning of the gate changed.
• Red on master behaviour [F]: I took the head, kept the new API, and reverted the fix (29 lines across 4 files). 8 of the 9 new tests go red:
  • RF during the background fill: 200, want 503
  • hash-sizes did not recompute after the startup load finished
  • the distance index reported built before the distance snapshot was refreshed
  • and others.
• TestStartupLoadDoneWaitsForBackgroundFill_116 passes vacuously on the revert, because the signal never fires. It is a guard against signalling too early and does catch M12.
• The same 9 tests with -race -count=10: ok in 31 s [F].

All mutants were run against the head with -run '_116|Warmup|1659|RunStartupLoad|Distance'. After each run the file was restored, and shasum matched git show 0a463524:<path> for all 4 files [F].

#	Mutant	Result
M1	remove defer s.signalStartupLoadDone()	caught (FiresOnEveryPath ×6, GatedRF)
M2	sample the warm-up gate after compute (old behaviour)	caught (WarmupGateIgnoresPassStartedBeforeLoad)
M3	remove t.Reset(r.interval)	caught (RecomputeNowRunsOnLoopAndResetsTicker)
M4	do not start the post-load pass	caught (EveryRecomputerRunsOnce, GatedRF, ForcedOpenReplaced)
M5	gate back on s.LoadComplete	caught (GatedRF, ForcedOpenReplaced)
M6	drop clockSkew.Invalidate()	caught (DropsDependentCaches)
M7	drop rc.RecomputeNow() in the lazy distance build	caught (DistanceSnapshotRefreshedBeforeReady)
M8	drop the distCache clear in the lazy distance build	survived; almost equivalent (nit 4)
M9	drop invalidateCachesFor(eviction) at the signal	caught (DropsDependentCaches)
M10	roles before nodes-clock-skew	caught (EveryRecomputerRunsOnce)
M11	stop closure does not wait on postLoadDone	survived; test gap (nit 4)
M12	signal before the background fill instead of deferred	caught (WaitsForBackgroundFill, GatedRF)

Suites run locally

Suite	Head	Master
cmd/server go test -race -count=1 -timeout 60m ./...	ok 896.3s (2002 tests listed) [F]	ok 758.1s (1993 tests listed) [F]
Read-only invariant tests (TestServerSourceHasNoCachedRWCalls, TestServerDBHasNoWriteMethods, TestServerDBConnIsReadOnly, TestPacketStoreHasNoMultibytePersistMethods)	PASS [F]	n/a
go vet .	clean [F]	n/a
gofmt -l on touched files	4 files flagged, including store.go [F]	3 files flagged; store.go clean [F]

• The known flake TestIssue1008_HandlerReturns503WhileSubpathIndexLoading did not fire in either run [F].
• The PR reports ok 1632s for the full race suite [T]; my run gave ok 896s. The difference is machine timing only.
• No JS or E2E suites were run: no frontend files changed, and the master drift since the base is frontend-only and does not touch cmd/.

Browser

Not applicable: there is no UI change. I checked the server side live with e2e-up.sh (head on 13780, master on 13781; both killed afterwards):

• Default config (hot window off), fixture: head and master both return 200 for all gated and ungated endpoints, totalTransmissions: 500, and distance 202 → 200.
• retentionHours=720, hotStartupHours=1, 51,409-transmission copy of the fixture (100 time-shifted replicas; 213 in the hot hour), poll every ~0.1 s from launch:
  • head: t=1.2s rf=503 Retry-After: 5, topology=503, hash-sizes=200 → t=6.3s rf=200 total=51307 → t=6.5s topology=200. The fill completed at +6 s.
  • master: t=0.37s rf=200 total=212 for the whole poll. After the fill, master still served rf totalTransmissions=212, hash-sizes total=159, channels activeChannels=15; the head served 51307, 40299 and 19.

Performance and security

Performance [F].

• On the 51 K copy the post-load round took 726 ms in total, run sequentially:

  Recomputer	Time
  rf	33 ms
  topology	350 ms
  channels	59 ms
  distance	0 ms
  hash-collisions	27 ms
  hash-sizes	204 ms
  observers-clock-skew	14 ms
  nodes-clock-skew	38 ms
  roles	0 ms

• The PR's own measurement covers 499 rows (34 ms) only [T].

• This is one extra round per process start. Because the passes run sequentially, a writer waits for at most one compute's read-lock hold at a time.

• Production scale (~1.6 M observations) is not measured, by the PR or by me. The PR's "Not verified" section says so.

• Cache drops are map resets, O(1).

Locking and deadlocks [F by reading].

• signalStartupLoadDone takes hashSizeInfoMu, clockSkew.mu and cacheMu→channelsCacheMu one after another, and never holds one across close. That order matches the existing invalidateCachesFor.
• TriggerDistanceIndexBuild releases cacheMu and analyticsRecomputerMu before RecomputeNow.
• RecomputeNow holds no lock while it waits, and it returns on r.stop.
• The stop closure closes stopPostLoad, stops each recomputer (each RecomputeNow wait unblocks through r.stop), then waits for the post-load goroutine. I found no cycle.

Goroutines and timers [F]. One post-load goroutine per StartAnalyticsRecomputers; it exits on the signal or on stop. RecomputeNow blocks until Start if the recomputer has not started yet. The only external caller that could see an unstarted recomputer is the distance build, and it only waits briefly while StartAnalyticsRecomputers is still starting them.

Other checks [F]. No new map[string]interface{} outside one test assertion. No DB writes, and the read-only invariant tests pass. No DOM or HTML surface.

Not verified

• Timings at production size (100 K+ transmissions, ~1.6 M observations), and how long the 503 window lasts there compared with the frontend's ~30 s retry budget (finding 3) [K].
• Staging startup with a real hot/background split [K]. No ssh by brief.
• How often the widened distance race (finding 1) is hit in practice. I only reproduced it deterministically with hooks.
• Frontend behaviour during a warm-up longer than 30 s. It was not exercised in a browser; this is based on reading public/app.js [A].
• Reviewer probe tests were kept locally only; nothing was committed.

[dborup]

Review feedback addressed (commit 9729a9be)

1. Synced with master (9996e08f): merged origin/master e829d1ea, which includes fix(store): lazy distance build — drop the reset sync.Once, fix the lock order (#149) #151 (fix(store): lazy distance build can crash (sync.Once reset during Do) and deadlock (lock order) #149), with a merge commit; no conflicts. The hunk that clears distCache and calls RecomputeNow() now sits inside the runDistanceIndexBuild loop, after s.mu.Unlock() and before the distLazyMu section that sets distLazyBuilt = true. It holds neither lock, so computeAnalyticsDistance (which takes s.mu.RLock) never runs under distLazyMu. The lock-order guard does not follow calls into RecomputeNow; this was checked by reading. Finding 1 (P2) is fixed on master by fix(store): lazy distance build — drop the reset sync.Once, fix the lock order (#149) #151.
2. Nit 5, gofmt (6d608a7a): the startupLoadDone field block in store.go is aligned again; gofmt -l store.go is clean.
3. Nit 4, mutant M8 (9729a9be): new TestDistanceBuildDropsRegionAreaCache_116. It holds the lazy build before it reads the dataset, caches SJC|, |BAY and SJC|BAY distance results from the older index (0 hops), releases the build and checks that none of those cached maps is served once the index reports built; each key must match the built index (3 hops). With the distCache reset removed it fails for all three keys.

Local runs on 9729a9be:

• -race -count=10 on the fix(analytics): recompute cached snapshots after the complete startup load #116 tests, the fix(store): lazy distance build can crash (sync.Once reset during Do) and deadlock (lock order) #149 tests, TestDistLazyMuNeverHeldWhileTakingStoreMu, TestLockOrderWalkerControls, TestNoSyncOnceIsReassigned, TestSyncOnceGuardControls and the Analytics cards show post-restart slice with 'All data' selected until multiple manual refreshes Kpa-clawbot/CoreScope#1659 warm-up tests: ok (142 s).
• cd cmd/server && go test -race -count=1 ./...: ok (620 s). The known flakes did not fire.
• go vet ./...: clean.

Not changed here: finding 2 (ticker tick buffered during RecomputeNow under go 1.22), finding 3 (503 window vs the frontend's ~30 s retry budget), M11 and nit 6. They are listed under "Open review findings" in the description.