fix(analytics): treat the distance index's 202 as a transient building state

[dborup]

Relates to #120

Summary

The Analytics → Distance tab no longer breaks while the lazy distance index is building.

/api/analytics/distance answers 202 Accepted with Retry-After: 5 and {"status":"building",…} until the index is ready (cmd/server/routes.go, handleAnalyticsDistance). This caused two problems:

• Cached placeholder. api() cached that 202 body for the analytics TTL, so the placeholder was served back for minutes.
• Error message. renderDistanceTab() read data.summary.totalHops from it and showed "Failed to load distance analytics: Cannot read properties of undefined (reading 'totalHops')".

Now the tab shows a building state, retries on the server's interval, and renders the data when the 200 arrives. The server side is unchanged.

Upstream reference (read only): Kpa-clawbot/CoreScope#2051.

Plan and design

Autonomous run, so the plan is written here instead of waiting for approval (AGENTS.md rule 5).

1. Test first (commit 810df7d9). The new test loads the real public/app.js and public/analytics.js in a vm, with a fake fetch, a stubbed api() and a fake clock. The only production change in this commit is a test hook, window._analyticsRenderDistanceTab, next to the other tab hooks.
2. Fix (commit ab35ccf9).

public/app.js, api()

• A 202 is never written to _apiCache. A 200 caches as before, and the 503 warm-up retry loop is untouched.
• For a 202 with a valid Retry-After (a positive integer of seconds), the value is attached to the returned object as a non-enumerable retryAfterSeconds. The JSON body as served is unchanged, and no other caller sees a new field.

public/analytics.js, renderDistanceTab()

• Building state. A {status:"building"} body without summary renders "Building the distance index…" (role="status") with the retry delay. It no longer reads data.summary.
• Retry delay. The Retry-After header is used first, then the body's retry_after_seconds, then 5 s. The delay is clamped to 1–30 s.
• Generation counter. _distanceGen is bumped by every render, by switching away from the tab (tab-bar handler) and by destroy(). A response or retry from an older generation does nothing. Every bump also clears the timer, so at most one retry chain exists per active Distance view.

Where the fork differs from upstream

• Upstream uses only the body's retry_after_seconds. The issue asks for Retry-After, so the header is honoured and the body is only a fallback.
• Upstream cancels the timer, but a response that is still in flight can land after navigation. Here the generation guard makes it inert, as the issue's out-of-order criterion requires.

Config and customizer (AGENTS.md rule 8). The 5 s fallback and the 1–30 s clamp are hard-coded. They only pace a one-off retry, so they are not customizer material.

Acceptance criteria

Criterion	Status	Evidence
Never insert a 202 into _apiCache	Met	Test "a 202 body is not cached" (second call reaches the server)
Clear "building distance index" state instead of throwing	Met	Unit test; browser shows the building state, then data
Honour a valid Retry-After, bounded fallback otherwise	Met	Tests for header, body and 5 s fallback, malformed/absent values, clamps 30 s and 1 s
At most one retry chain per active Distance view	Met	Test: 4 renders while building, then a retry fire, leave exactly 1 timer
Navigation cancels or invalidates timers and responses; an older generation cannot overwrite	Met	Tests for destroy, tab switch (plus a check that the tab bar calls _leaveDistanceTab()), and an older 202 arriving after a newer 200
A later 200 replaces the building state and caches normally	Met	Tests "202 then 200" and "a 200 is still cached"
503 warm-up handling intact	Met	test-1659-analytics-warmup.js 7/0, same as master
Tests: 202 → 200, cache, Retry-After, navigation/re-entry, out of order, no unhandled rejections	Met	13 cases; any unhandled rejection fails the case

Tests

node test-issue-120-distance-building.js

• Commit 810df7d9 (master behaviour plus the test hook): 3 passed, 10 failed. The failures:
  • the 202 is cached;
  • the tab shows Cannot read properties of undefined (reading 'totalHops');
  • no retry is scheduled;
  • a late response overwrites a newer render or a left view.
• This branch: 13 passed, 0 failed.

Mutation checks (each run against the fixed code, then restored)

Mutant	Result
No generation check after await	3 failed
Render does not supersede the previous one	2 failed
api() caches 202 again	1 failed
No generation check inside the timer callback	Survives, as expected. Every generation bump already clears the timer, so the check is belt-and-braces.

Related suites, same results on master 85bfee49 and this branch

Suite	master	branch
test-1659-analytics-warmup.js	7/0	7/0
test-app-api-inflight-cleanup-rejection.js	0 failed	0 failed
test-analytics-distance-view-path.js	8/0	8/0
test-analytics-table-ids-unique.js	4/0	4/0
test-analytics-wardriving-tab.js	18/0	18/0
test-analytics-foreign-traffic-tab.js	12/0	12/0
test-analytics-areas-tab.js	19/0	19/0
test-analytics-channels-integration.js	23 passed / 1 failed	23 passed / 1 failed
test-frontend-helpers.js	705 passed / 2 failed	705 passed / 2 failed
test-packet-filter.js	92/0	92/0
test-aging.js	19/0	19/0

The failures in the channels-integration and frontend-helpers rows are pre-existing, and neither file is in the CI unit step.

Static checks

• scripts/check-xss-sinks.sh --diff origin/master: exit 0.
• eslint@8 --quiet public/*.js: 0 errors.

Browser (Playwright with Chromium, a fresh local server on the migrated and freshened test-fixtures/e2e-fixture.db, 1440×900, opening #/analytics?tab=distance)

Run	Distance responses	What the page shows	Page errors
This branch	202, 200	The building state, then "1,808 Total Hops Analyzed…"	none
master	202 only	"Failed to load distance analytics: Cannot read properties of undefined (reading 'totalHops')"; nothing retries	—

Leaflet's CDN is blocked in this sandbox. That L is not defined error is ignored in both runs.

Performance. Not a hot path. There is one extra branch per api() response and one timer at most while building.

Not verified

• No staging test. The building window on a production-sized database lasts longer, but the logic is the same.
• Only 1440×900 was driven in the browser. The building state is plain text in the existing content area.

Overlap with my other open PRs

• PR fix(packets): empty observer/type selections on Clear Filters #132 (issue fix(packets): Clear Filters must reset observer and type selection state #121) also adds one line each to .github/workflows/deploy.yml (unit step) and test-all.sh.
• The insertions are at different places, so no conflict is expected. There is no other shared file.

🤖 Generated with Claude Code

https://claude.ai/code/session_019TcZHooUiiknVWbECVWzk8

---

Generated by Claude Code

[dborup]

Independent review of ab35ccf9

Verdict: APPROVE with nits. This is a recommendation only; merging is the owner's call.

Reviewed head: ab35ccf9928b2b084ab9a66bc2675e89b57048a2 (unchanged before and after the review). Work done on a git archive of the PR head, commit A (810df7d9) and origin/master (85bfee49).

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

Findings

1. P3, test gap. The generation guard in the catch branch (public/analytics.js:3099) is not covered.
   • The mutant that removes if (gen !== _distanceGen) return; in catch survives, with 13/13 green [F].
   • Failure without the guard: the user switches from Distance to another tab while /analytics/distance is in flight, and the request fails (500 or network). "Failed to load distance analytics" then lands in the new tab. That is exactly the issue's criterion that an older generation must not overwrite a newer tab.
   • The code is correct, so this is only a test gap. Not reproduced in a browser, because the guard is present.
   • Suggestion: one test case where a request from an outdated generation rejects.
2. nit. parseInt is lenient in api() (public/app.js:199). Retry-After: 5abc and 2.9 become 5 and 2 and count as "valid". This is harmless because the value is clamped to 1–30 s, and it mirrors the existing 503 path. An HTTP date gives NaN and falls back correctly [F], confirmed in the browser with Retry-After: abc.
3. nit. The retry timer calls renderDistanceTab(el) directly, not renderTab('distance') (public/analytics.js:3017-3020). It therefore skips renderTab's post-processing: assignAnalyticsTableIds, the mobile scroll wrappers and ?section= scrolling.
   • Today this has no effect, because the Distance tables use class="data-table", not analytics-table [F].
   • It becomes fragile if the tab later gets analytics-table.
4. Observation, not a defect. Now that a 202 is no longer cached, the theme-refresh re-render at page load sends a second distance request about 300 ms after the first while the index is building [F] (browser log: 202 @~200ms, 202 @~500ms). This is cheap on the server: TriggerDistanceIndexBuild returns at once while distLazyBuilding is set (cmd/server/store.go:4636).

Metadata

• [F] Both commits (810df7d9, ab35ccf9) have author and committer dborup <kontakt@meshview.dk>.
• [F] Base is origin/master 85bfee49. merge-tree against master has no conflicts.
• [F] No closing keywords ("Relates to fix(analytics): treat lazy distance-index 202 responses as transient #120"); closingIssuesReferences = 0; no @mentions; no full upstream URLs (only Kpa-clawbot/CoreScope#2051 in code format).
• [F] .github/workflows/deploy.yml: one added line (node test-issue-120-distance-building.js) in the unit step. No Kpa-clawbot lines, triggers, permissions or jobs are touched.
• [F] Overlap with batch PR fix(packets): empty observer/type selections on Clear Filters #132 (issue fix(packets): Clear Filters must reset observer and type selection state #121): both touch deploy.yml and test-all.sh, on different lines. A sequential merge-tree master → fix(packets): empty observer/type selections on Clear Filters #132 → fix(analytics): treat the distance index's 202 as a transient building state #133 has no conflicts.
• [F] CI on ab35ccf9 is complete: Go, Playwright and Docker are SUCCESS; the rest are SKIPPED.
• [F] Commit A changes a single production line, the test hook window._analyticsRenderDistanceTab. The test file is unchanged from A to the head.

Acceptance criteria (issue #120)

Criterion	Result
Never insert a 202 into _apiCache	Met [F]: code, test, and mutant M1 is caught
Clear building state instead of a throw	Met [F]: the browser shows "Building the distance index… Retrying in Ns." (role="status"). Master shows Cannot read properties of undefined (reading 'totalHops')
Honour a valid Retry-After, bounded fallback otherwise	Met [F]: in the browser, RA=2 retries every ~2 s; no header and abc give 5 s; 999 is clamped to 30 s. Mutants M5 and M6 are caught
At most one retry chain per active Distance view	Met [F]: test, mutant M8 is caught, and the browser's retry interval is even
Navigation cancels timers and responses; an older generation cannot overwrite	Met [F] in code and in the browser. After a tab switch and after a route change to #/nodes, there are 0 distance requests over 12 s and nothing is written. Mutants M2, M3 and M4 are caught. The catch branch is not tested (finding 1)
A later 200 replaces the building state and is cached normally	Met [F]: the browser shows building, then "1,808 Total Hops Analyzed…"; test "a 200 is still cached"
503 warm-up is intact	Met [F]: test-1659-analytics-warmup.js 7/0 on master and the head; the 503 loop is untouched in the diff
No console errors or unhandled rejections	Met [F]: 0 page errors and 0 unhandledrejection in every browser run

Test-first and mutants

• [F] test-issue-120-distance-building.js: commit A gives 3 passed, 10 failed. Master plus the test file gives 2/11, because the hook is missing. The head gives 13/0.
• [F] Eight mutants, seven caught:

Mutant	Result
M1 api() caches 202 again	1 fail
M2 no generation check after await	3 fail
M3 tab switch does not leave Distance	1 fail
M4 destroy() does not leave Distance	1 fail
M5 header ignored (body only)	2 fail
M6 no 30 s clamp	1 fail
M8 a new render does not supersede the previous one	2 fail
M7 no generation guard in catch	survives (finding 1)

Suites run locally (master vs head, same result on both)

• [F] test-1659-analytics-warmup.js 7/0 · test-app-api-inflight-cleanup-rejection.js PASS · test-analytics-distance-view-path.js 8/0 · test-analytics-table-ids-unique.js 4/0 · test-analytics-wardriving-tab.js 18/0 · test-analytics-foreign-traffic-tab.js 12/0 · test-analytics-areas-tab.js 19/0 · test-packet-filter.js 92/0 · test-aging.js 19/0.
• [F] test-analytics-channels-integration.js 23/1 and test-frontend-helpers.js 705/2. The failing test names are the same on master and the head, so the failures are pre-existing.

Browser

Run with Playwright/Chromium against a local Go server on a freshened and migrated test-fixtures/e2e-fixture.db.

• [F] Fresh server, real backend, 1440×900: the head gets 202 (Retry-After: 5), then 200, and shows the data. Master gets only 202 and shows the error text, with no retry.
• [F] Slow build, simulated with route interception (202 for 9 s, then the real backend):
  • RA=2: retries at about 2.5 / 4.5 / 6.6 / 8.6 s, data at 10.6 s.
  • No header: 5 s.
  • abc: 5 s.
  • 999: "Retrying in 30s".
  • 375×900 with RA=2: building, then data.
• [F] Tab switch (Distance → RF) while building: the RF content stays put for 12 s, there are 0 new distance requests, and a new chain works on re-entry. Route change to #/nodes: 0 requests and no Distance content.

Performance and security

• [F] Not a hot path. The change is one branch per api() response and at most one timer.
• [F] The building HTML is static text plus an integer. The error path still uses esc(e.message). There are no new unbounded data structures and no Go, API or DB changes, so mode=ro and the map[string]interface{} rule are not affected.
• The hard-coded #ff6b6b in the error path already exists on master and is not touched.

Not verified

• I did not run scripts/check-xss-sinks.sh --diff origin/master or eslint myself [T].
• The full test-e2e-playwright.js. Locally it stops (fail-fast) on "Version info lives on Perf dashboard", identically on master and the head, so it is my environment. I rely on CI's green Playwright job.
• Staging, and a really long index build on a production-sized database. The long build was only simulated with route interception. The PR's "Not verified" list mentions staging. It says only 1440×900 was driven, which I have supplemented with 375 px.