fix(rx-coverage): honour configured defaults and save an independent viewport

[dborup]

Relates to #124

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. 44c367bf: tests that reproduce the bug (red on master).
2. 1ab283a2: the fix in public/rx-coverage.js.

Claims in the issue, verified against master

• The page always opens at [51.0, 4.8], zoom 8.
• It never reads /api/config/map, a URL viewport or any saved view.
• It saves nothing.
• Its async responses can land after destroy().

Initial viewport (first valid source wins)

1. Explicit URL lat/lon/zoom in the hash.
2. This page's own localStorage['rx-coverage-view'].
3. /api/config/map (center, zoom).
4. The documented offline fallback 51.0, 4.8, zoom 8. That was the page's original start and the fork's home area, so offline behaviour does not change.

Validation. All three values must be present, numeric and in range (lat −90..90, lon −180..180, zoom 1..19). Anything invalid, partial or out of range falls through to the next source.

Saving. On moveend the page saves to rx-coverage-view only. It never reads or writes the main map's map-view, so the two pages do not recentre each other.

URL. syncHash() keeps days and rx and adds lat/lon/zoom once the map exists; rx is now URL-encoded. An observer-only rx= link (no explicit viewport) still fits that observer's coverage.

Stale async work. A generation counter is bumped on each mount and in destroy(). The config, observer extent, coverage and leaderboard responses and the delayed invalidateSize all check isLive(gen), so late responses from a destroyed or replaced mount do nothing.

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

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

• The offline fallback stays 51.0, 4.8, zoom 8 (this fork's area) instead of upstream's 37.6, -122.1, zoom 9.
• Stored and URL values must be complete and in range; upstream only parses them.
• rx is URL-encoded in the hash.
• The generation guard also covers observer extent and leaderboard responses, and a quick remount.
• The test uses the real module in a vm with a fake Leaflet and releases the fake fetches one by one, so the delayed-response cases are deterministic.

Acceptance criteria

Criterion	Status	Evidence (test no. in test-issue-124-rx-coverage-viewport.js)
Precedence 1: valid explicit URL viewport	Met	1
Precedence 2: valid rx-coverage-view	Met	2
Precedence 3: /api/config/map	Met	3
Precedence 4: documented offline fallback	Met	4
Read/write only rx-coverage-view, never map-view	Met	6
Invalid, partial or out-of-range values fall through safely	Met	5
days and rx preserved as the viewport changes	Met	7
Observer-only rx= link still fits the observer	Met	8
Delayed config/extent/coverage responses cannot affect a destroyed page	Met	9, 10 (destroy and quick remount)
Coverage filtering and leaderboard unchanged	Met	11 (identical request URLs)

Tests

test-issue-124-rx-coverage-viewport.js loads the real public/rx-coverage.js in a vm. It uses a fake Leaflet, fetches released one by one, fake storage, location and history, and a manual clock. It is registered in test-all.sh and the deploy.yml unit step.

	Passed	Failed
master	2	9
this branch	11	0

The 2 that pass on master are the offline-fallback and "requests unchanged" controls.

Other checks:

• test-rx-coverage-escape.js fails identically on master and on this branch (pre-existing, unrelated).
• scripts/check-xss-sinks.sh --diff: clean.
• eslint: clean.
• Workflow guard lines are unchanged.

Browser check

Local Chromium against a server on test-fixtures with clientRxCoverage.enabled and mapDefaults in a local config:

• The page opened at the configured default.
• Panning saved rx-coverage-view and left map-view untouched.
• A reload restored the saved view.
• An explicit URL lat/lon/zoom was honoured.

Perf

Not a hot path. There is one extra localStorage write per moveend, and a few integer comparisons in async callbacks.

Not verified

• On a deployment with real RX coverage data (staging). The local fixture has little coverage data.
• Touch devices.

Overlap with other open PRs

• .github/workflows/deploy.yml unit step and test-all.sh: one line each, next to lines from PR fix(packets): empty observer/type selections on Clear Filters #132 (fix(packets): Clear Filters must reset observer and type selection state #121), PR fix(analytics): treat the distance index's 202 as a transient building state #133 (fix(analytics): treat lazy distance-index 202 responses as transient #120), PR fix(live): wire every persisted view toggle before Live init awaits #135 (fix(live): wire persisted view toggles before initialization awaits #125) and the upcoming PRs for fix(live): recover the shared WebSocket from silent half-open connections #117 and ui: show the running CoreScope version in the navigation drawer #111.
• No other open PR touches public/rx-coverage.js.
• The branch was cut from 85bfee49, the master at the time. It merges cleanly with the current origin/master d264716c (checked with git merge-tree).
• Merge simulation of all 13 batch branches on origin/master, in issue order: the only conflicts are the test-registration lines in deploy.yml and test-all.sh, where several PRs add a line after the same anchor. Keep both lines; nothing else overlaps textually.

🤖 Generated with Claude Code

https://claude.ai/code/session_019TcZHooUiiknVWbECVWzk8

---

Generated by Claude Code

[dborup]

Independent review of 1ab283a2

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

Reviewed head: 1ab283a293f9816a9e670746081dd38d46e2f029 (unchanged before and after the review). The work was done on git archive trees of the head, of commit A 44c367bf (test-first) and of origin/master ad011021. The master tree had the new test copied in.

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

Findings

1. P3. After a pan across the antimeridian, the page rejects its own saved view and URL. See public/rx-coverage.js:56 (saveView), :277 (syncHash) and the validator at :32.
   • Leaflet's getCenter() is not wrapped, so a pan east past 180° stores lng: 193.36 in rx-coverage-view and writes lon=193.35938 to the hash.
   • validView rejects lon > 180. So on both remount and reload, the page drops back to /api/config/map: the user's view and their shareable link are lost.
   • Reproduced in Chromium [F]. I opened #/rx-coverage?lat=10&lon=178&zoom=5 and dragged 300 px. The hash and storage then held lon 193.36, and the remount and the reload both opened at 55.68, 12.57 z9 (the config).
   • This matters mainly for Pacific deployments. Fix: map.getCenter().wrap() in both places.
2. P3. The stale-async guards are mostly untested, although test 10 claims to cover them. See test-issue-124-rx-coverage-viewport.js:220-242.
   • Test 10 releases the old extent response while the new mount's map is still null, so the !map guard handles it, not the generation check.
   • It never releases an old coverage response, and it runs the old settle timer before destroy().
   • As a result these mutants survive [F]:
     • drawCoverage guard weakened back to destroyed (M10)
     • extent guard weakened (M11)
     • settle-timer guard removed (M16)
     • init() guard weakened to !destroyed (M18, init→destroy→init before MeshConfigReady resolves)
   • The head behaves correctly: probe tests P2, P3, P4 and P6 in my dir (probe-124.js) pass on the head and catch each of those mutants.
3. P3. Two validation claims are not pinned. [F]
   • "Partial falls through" is tested only with the zoom missing. Dropping the null/empty handling in validView (public/rx-coverage.js:29, M5) survives. Under that mutant, #/rx-coverage?lon=10&zoom=9 opens at lat 0, and a saved {"lat":null,...} is accepted. The head is correct (probe P1).
   • The rx URL-encoding the PR claims is not tested (M17 survives; probe P5 catches it). Pubkeys are hex, so there is no practical impact.
4. nit. A second viewport parser sits beside the shared parseViewportHash. See public/app.js:345 and public/rx-coverage.js:28-40.
   • The semantics diverge. Map and Live accept lat+lon without a zoom (defaulting to 12) and clamp zoom to 1..20. RX Coverage rejects both cases.
   • Issue fix(rx-coverage): honor configured defaults and save an independent viewport #124 requires the stricter behaviour, so the divergence itself is justified. Extending the shared helper with an option would have kept one implementation (AGENTS.md DRY).
   • The test harness also extracts parseViewportHash from app.js into the vm (test-issue-124-rx-coverage-viewport.js:85-87), but the page never calls it. That is dead harness code.
5. nit. The offline fallback applies only when the server is unreachable or its answer is invalid; the PR body does not mention this.
   • /api/config/map always returns a center. Without mapDefaults it returns 37.45, -122.0, zoom 9 (cmd/server/routes.go:880-887).
   • So an online deployment with no mapDefaults now opens RX Coverage in the SF Bay area instead of at 51.0, 4.8.
   • That matches the precedence in the acceptance criteria and the main map, but it is a visible change worth one line in the PR body [F].
   • The cfg.zoom == null ? 9 default (public/rx-coverage.js:50) can never be reached with this server.

Metadata

• Head at the start and at the end: 1ab283a2... [F].
• Commits in origin/master..head [F]. Author and committer of both are exactly dborup <kontakt@meshview.dk>:
  • 44c367bf test
  • 1ab283a2 fix
• Files changed [F]:
  • public/rx-coverage.js (+94/−13)
  • test-issue-124-rx-coverage-viewport.js (new, 254 lines)
  • test-all.sh (+1)
  • .github/workflows/deploy.yml (+1)
• The merge-base is 85bfee49, 4 commits behind ad011021. None of those commits touches rx-coverage.js. git merge-tree --write-tree origin/master 1ab283a2 gives a clean tree 3208622c [F].
• PR body [F]:
  • It uses "Relates to fix(rx-coverage): honor configured defaults and save an independent viewport #124", with no closing keyword, and closingIssuesReferences is empty.
  • It has no @mentions.
  • Upstream appears only as `Kpa-clawbot/CoreScope#2033` in code format, with no github.com/Kpa-clawbot URL.
• Workflow [F]: one line, node test-issue-124-rx-coverage-viewport.js, is added in the unit-test step. No github.repository == 'Kpa-clawbot/CoreScope' guard line, trigger, permission or job is changed.
• [F] CI on 1ab283a2 is complete: Go Build & Test, Playwright E2E and Docker are SUCCESS; the rest are SKIPPED.
• Tests unchanged between A and the head [F]: the head diff against A touches only public/rx-coverage.js.

Acceptance criteria (issue #124)

Criterion	Result
1. A valid URL lat/lon/zoom wins	Met [F]: unit test 1. Browser: ?days=14&lat=48.85&lon=2.35&zoom=10 with a saved view at 10,10 z5 opened at 48.85, 2.35 z10. Master opened at 51, 4.8 z8.
2. Valid rx-coverage-view next	Met [F]: unit test 2. Browser: after a drag, SPA nav away and back restored 55.949, 13.368 z9. Master reset to 51, 4.8.
3. /api/config/map next	Met [F]: unit test 3. Browser: fresh context opened at the configured 55.68, 12.57 z9. Master: 51, 4.8 z8.
4. Documented offline fallback	Met [F]: unit test 4 covers a failed fetch, a bad center, an out-of-range center and {}. See finding 5 for when it can apply.
Only rx-coverage-view, never map-view	Met [F]: unit test 6. Browser: the RX drag left map-view null. Moving the main map to 40,-3 z6 wrote only map-view, and RX Coverage then reopened at its own saved view.
Invalid, partial or out-of-range values fall through	Met [F]: unit test 5 and probe P1. Browser: ?lat=95&lon=2&zoom=10 fell through to the saved view. There is a test gap (finding 3) and an antimeridian self-rejection (finding 1).
days/rx preserved as the viewport changes	Met [F]: unit test 7. Browser: after the drag the hash was days=7&lat=...&lon=...&zoom=9. After a leaderboard click it was days=7&rx=<pk>&lat=...&lon=...&zoom=10.
Observer-only rx= link still fits	Met [F]: unit test 8. Browser, fresh context: ?rx=<pk> fitted to the seeded observer (56.187, 10.207 z10), the same as master.
Delayed config/extent/coverage responses cannot affect a destroyed page	Met in code [F]: probes P2 to P4 and P6 pass. Only partly pinned by the PR's tests (finding 2). Browser: an immediate navigate-away after mount showed no errors and left no #rxMap.
Coverage filtering and leaderboard unchanged	Met [F]: unit test 11 checks the exact request URLs. The leaderboard rendered 1 row on both master and the head. drawCoverage/loadBoard request construction is unchanged apart from the guards.

Test-first and mutants

New test: 2 pass and 9 fail on master ad011021 and on commit A. On the head, 11 pass and 0 fail [F]. The two that pass on master are the fallback control and the requests control, as the PR says.

Mutants were run against the head with test-issue-124-rx-coverage-viewport.js. I then re-ran the survivors against my probe file (probe-124.js, kept in my dir only). The original file was restored and shasum matches git show 1ab283a2:public/rx-coverage.js (7011dc53) [F].

#	Mutant	PR test	Probe
M1	URL precedence removed	caught (1, 7)
M2	saved view before URL	caught (1)
M3	skip saved view	caught (2, 5)
M4	zoom range check removed	caught (5)
M5	null/empty handling in validView removed	survived (gap)	caught P1
M6	save under map-view	caught (2, 5, 6)
M7	syncHash drops the viewport	caught (7)
M8	explicit view still fits the observer	caught (8)
M9	createMap generation guard removed	caught (9)
M10	coverage guard weakened to destroyed	survived (gap)	caught P3
M11	extent guard weakened to destroyed	survived (gap)	caught P2
M12	leaderboard guard weakened	caught (10)
M13	destroy() does not bump the generation	survived	equivalent: destroyed covers the time until the next init(), which bumps the generation itself
M14	moveend handler guard reduced to !map	survived	near-equivalent: the harness debounce is the identity. An old debounced call after a quick remount would only redo save/sync/draw for the new map
M15	moveend does not save	caught (6)
M16	settle-timer guard reduced to !map	survived (gap)	caught P4 (the old timer causes a second coverage draw on the new map)
M17	rx not encoded in the hash	survived (gap)	caught P5
M18	init() guard reduced to !destroyed	survived (gap)	caught P6 (the stale start() renders into the old container and fetches the leaderboard twice)

Suites run locally

Suite	master ad011021	head 1ab283a2
test-issue-124-rx-coverage-viewport.js	2 pass / 9 fail	11 / 0
probe-124.js (mine, P1 to P5)	1 / 4	6 / 0 (P1 to P6)
test-frontend-helpers.js	705 / 2 (favStar ×2)	705 / 2 (same two)
test-rx-coverage-escape.js	crashes: "could not locate row-builder end"	same crash (pre-existing)
test-rx-coverage-config-race.js	OK	OK
test-nav-first-load-fit.js / test-nav-dynamic-link-lifecycle.js / test-nav-priority-scheduler.js	23 / 21 / 15 pass	23 / 21 / 15 pass
test-privacy-page.js, test-node-reach-coverage-debounce.js	34 pass, OK	34 pass, OK
test-packet-filter.js / test-aging.js	92 / 19 pass	92 / 19 pass
test-rx-coverage-mobile-nav-e2e.js (Playwright)	3 / 0	3 / 0
test-e2e-playwright.js	6 pass, then fail-fast on "Version info lives on Perf dashboard" [K]	identical

• Go was not run: cmd/server and cmd/ingestor are untouched. node --check public/rx-coverage.js passes.
• The PR adds no Playwright E2E. It relies on the vm unit test and a manual browser check [T].

Browser

• Setup [F]:
  • The e2e-up.sh servers ran on :13700 (master) and :13701 (head), each with a local config.json (clientRxCoverage.enabled, mapDefaults 55.68, 12.57 z9).
  • The fixture DB has no client_receptions table, so I created the ingestor schema in my DB copies and seeded 20 synthetic receptions near 56.1, 10.1 for one observer.
  • Map instances were captured by wrapping L.map in an init script.
• Scenario scenario.js, run on both servers:
  • fresh load
  • real mouse drag
  • SPA nav away and back
  • main map moved independently
  • leaderboard row click
  • quick mount followed by navigate-away
  • fresh observer-only link
  • explicit URL plus a conflicting saved view
  • invalid URL plus reload
• Results are in scenario-master.json and scenario-head.json:
  • On master, every load opens at 51.0, 4.8 z8 and nothing is saved or synced.
  • On the head, every precedence step behaves as specified (see the AC table).
  • There were zero console or page errors on both.
• A separate dateline.js run reproduced finding 1.

Performance and security

• Not a hot path [F]. Each debounced moveend (200 ms) adds one localStorage.setItem and one history.replaceState. There is one /api/config/map fetch per mount, and only when there is no URL view or saved view. No per-item calls and no new collections. The generation counter is a single integer.
• Timers [F]: one 150 ms settle timer per mount, as before, now guarded by isLive(gen). No intervals were added.
• DOM [F]:
  • None of the added lines contains an HTML sink. The one moved innerHTML (leaderboard error) is static text.
  • The URL values reach setView only as validated finite numbers. rx is now encodeURIComponent-ed into the hash.
  • No hardcoded colours were added.
• No Go, no map[string]interface{} and no server writes: the backend is untouched.

Not verified

• Real RX coverage data (staging) and touch devices [T], and I did not check them either.
• Actual Leaflet map.remove() handler teardown for M14. I did not check whether Leaflet cancels the pending debounced call [A].
• scripts/check-xss-sinks.sh --diff and eslint were not run: they need a git checkout, and I made no git writes. I grepped the added lines for sinks by hand instead. The PR reports both as clean [T].
• The PR's "Not verified" section is honest as far as it goes. It omits finding 5 (online deployments without mapDefaults now open at the server default rather than 51.0, 4.8).

[dborup]

Review feedback addressed (commit 35f297b0)

1. Merge conflict (2935cc86). I merged origin/master (4ce26563) into the branch with a merge commit.
   • In .github/workflows/deploy.yml and test-all.sh, the fix(rx-coverage): honor configured defaults and save an independent viewport #124 line (test-issue-124-rx-coverage-viewport.js) and the fix(live): wire persisted view toggles before initialization awaits #125 line from fix(live): wire every persisted view toggle before Live init awaits #135 (test-issue-125-live-toggles-wiring.js) are both kept.
   • The github.repository == 'Kpa-clawbot/CoreScope' lines are identical to master.
   • git diff origin/master..35f297b0 touches the same 4 files as before.
2. Test gap for the stale guards (35f297b0). New test 12 in test-issue-124-rx-coverage-viewport.js remounts with a saved rx-coverage-view, so the new map and coverage layer exist at once. It then releases the old mount's observer-extent response and coverage response.
   • Guard mutant in fitToObserver (if (!isLive(gen) || !map) → if (!map)): the old test file gives 11 passed / 0 failed; the new one gives 11/1 ("the old observer extent fitted the new map").
   • Guard mutant in drawCoverage (if (!isLive(gen) || !covLayer) → if (!covLayer)): the old file gives 11/0; the new one gives 11/1 ("the old coverage response was drawn on the new layer (cleared 1, added 1)").
   • Head: 12 passed / 0 failed.
   • Test 10 stays green under both mutants, which confirms the gap was real.
3. Other suites on 35f297b0:
   • test-rx-coverage-config-race.js: OK.
   • test-issue-125-live-toggles-wiring.js: 4/0.
   • test-packet-filter.js: 92/0.
   • test-aging.js: 19/0.
   • test-frontend-helpers.js: 705/2, identical on master.
   • test-rx-coverage-escape.js: fails on master too (pre-existing).
   • scripts/check-xss-sinks.sh --diff origin/master: clean.

The other review findings (antimeridian wrap(), validation/rx encoding pins, the parser DRY nit, the fallback note) are not part of this round.

---

Generated by Claude Code