[dborup]

Adds a shareable #/regions?scope=... map inspired by the NashMesh Regions page and its Regions frontend, adapted to our existing route-derived transported_scopes_recent data. Positioned repeaters/rooms become markers; a selected scope with at least three points gets a dashed convex hull. The page explicitly says this is observed routing, not measured RF coverage or a region boundary.

This deliberately avoids the NashMesh resolved_path SQL, which would be empty on our current database. Node and scope limits are surfaced when a view may be incomplete; valid OTA scope spelling is preserved.

Validation: sh test-all.sh (235/235), both Go suites, and a targeted Chromium test with synthetic node data. No deployment.

[dborup-agent]

Rapport — CS-pve-agent3 PR#376 overtagelse — head cdc5672

Review feedback addressed (commit cdc56725)

1. No review findings were found. [K] PR Add observed relay Regions map #376 has zero reviews, review comments and issue comments, and nothing in the repo matches. I therefore fixed the items listed in the handover plus defects found by reading the page code. If a review exists elsewhere, point me to it.
2. Merge of origin/master (bcf263e1) done as a plain merge commit 2d16b720, no conflicts. [A]
3. Theme change not followed (found by me). The tile layer and dark invert filter were set once at init, so switching to light left dark-only inversion on the tiles. Now a data-theme observer re-resolves the configured provider through MC_getTileSpec (fix(maps): preserve legacy light tile URL overrides (#364) #366/fix(maps): honour the legacy dark tile URL, document raw-URL limits (#390) #399). [T]
4. Teardown. destroy() now disconnects the theme observer in addition to removing the Leaflet map (fix(ui): stop observers.js leaking delegated handlers on #app (#405) #419/fix(ui): stop compare/channels/live leaking handlers on #app (#428) #437). init() calls destroy() first so a re-entry cannot leak a map. [T]
5. Escaping. Node names, scopes and popups are built with createElement + textContent; no change needed. A new test injects <img onerror> as a node name and asserts no handler runs and no img is parsed. XSS gate scripts/check-xss-sinks.sh --diff: exit 0. [T]
6. Deep link and disclaimer. #/regions?scope= works and the page still states "observed routing, not a radio coverage measurement"; both asserted in the new E2E and the existing one. [T]
7. E2E sharding (ci: measure and shard Playwright E2E suites (#434) #436). test-regions-map-e2e.js added to scripts/e2e-shards.json (shard 3); --validate OK, 159 suites. [T]
8. PR description. No relevant open issue was clearly related (feat(scope-audit): evaluate transport-carrier view and observer-region filters #414 is only loosely adjacent), so no "Relates to" line was added. No closing keywords. [K]

Tests

• Red first: the theme test failed on the old code (invert(...) left on light theme) before the fix. [T]
• sh test-all.sh: 246/246. node test-frontend-helpers.js: 713/0. go test ./... passed in cmd/server and cmd/ingestor. [T]
• Local E2E against a local Go server with a CI-prepared copy of e2e-fixture.db: test-regions-map-e2e.js pass, test-nav-priority-1391-e2e.js 36/36, the Regions case in test-e2e-playwright.js pass. The same file then failed locally at "Packets type filter includes Group Data ("Group Data" message type missing from packet view window's "message type" filter Kpa-clawbot/CoreScope#1791)" (timeout), which I did not investigate. CI ran the full suite green. [T]
• Browser check in light and dark themes with screenshots: tiles, hull outline and disclaimer render correctly in both. [A]

Mutants (each killed)

• No theme observer: theme test fails.
• No disconnect() in destroy(): teardown test fails.
• Popup name via innerHTML: escaping test fails.

CI (run 38524238998)

All jobs success: Go Build & Test, Playwright E2E 1/3, 2/3, 3/3, Verify E2E Shards & Coverage, Validate Docker Image, Build & Publish Docker Image. Release, Deploy Staging and Badges were skipped. No flaky reruns needed.

Notes

• PR is not in draft state on GitHub (isDraft: false); I did not change it. No merge, ready or issue closing was done. No stg/prod access.

🤖 Generated with Claude Code

[dborup-agent]

Review — CS-pve-agent2 PR#376 — head cdc5672

Dom: APPROVE med nits

Evidence tags: [T] = I ran it, [A] = I read the code or diff, [K] = taken from the PR text or reports.

Scope note

#376 is the PR itself. There is no separate issue with an acceptance checklist, and the author reports zero earlier reviews [K]. So I checked the author's eight report items one by one against the diff rather than against an issue list. If a separate review or checklist exists, it was not available to me.

Findings

#	Sev	Finding	Evidence
1	nit	The tile-URL half of the theme test is vacuous. It only asserts the tile host when dark.url !== light.url. With the default provider both specs share one OSM URL. A mutant that never swaps the tile layer but still clears the invert filter survives (M6 below). Provider, attribution and maxZoom re-resolution on theme change is therefore untested. The code itself is correct.	[T]
2	nit	'steelblue' is a hardcoded colour fallback in public/regions-map.js (--accent fallback). AGENTS.md says no hardcoded colours. It is dead in practice because --accent is always defined. It came in with the first commit, not this one.	[A]
3	nit	The teardown test is not red before the fix. The old code had no observer, so there was nothing to leak and the test passes on it too. It is a guard for the new code (M2 kills it), not a red-to-green reproduction. The author's red-first claim holds only for the theme test, which I confirmed with M1 and M4.	[A][T]
4	nit	Collinear-point handling in convexHull (<= 0) is unpinned by tests (M7 survives). This is old code, outside this follow-up.	[T]
5	info	.catch(() => {}) on the 3 s waitForFunction in the theme test swallows the timeout. The following assertions still decide pass or fail, so it is harmless.	[A]

No blockers.

The author's report items

1. No findings found: consistent with the PR having no reviews. [K]
2. Merge of origin/master: plain merge, no conflicts. The merge-tree of the head against origin/master (bcf263e1) is clean. [T]
3. Theme change followed: the data-theme observer re-resolves the tile spec and the filter. Red on mutants M1, M4, M5. Partly vacuous for the URL part (finding 1). [T]
4. Teardown: destroy() disconnects the observer, and init() calls destroy() first. Killed by M2. See finding 3. [T]
5. Escaping: DOM is built with createElement and textContent. The <img onerror> name test is killed by M3. scripts/check-xss-sinks.sh --diff origin/master exits 0. [T]
6. Deep link and disclaimer: asserted in both the new E2E and the test-e2e-playwright.js case. [T]
7. Shard manifest: test-regions-map-e2e.js is registered in shard 3. [A]
8. No closing keywords in the PR body or any commit message. [T]

Hygiene checks

• Commit author and committer on all 4 commits: dborup <kontakt@meshview.dk>. [T]
• No Go files and no .github files changed, so cmd/server stays read-only and there are no new map[string]interface{}. Fork guards are unchanged at 9 in deploy.yml and 1 in release-fast-path.yml. [T]
• No scope creep or silent product decisions. The changes are the theme observer, teardown and re-init guard, tests, and the shard entry. The "Regions" nav link has no data-priority, so it lands in the overflow menu. That came from the earlier commits, and the nav-priority test covers it. [A]
• Head SHA via git ls-remote was identical before and after my run. [T]

Tests (merged tree: head merged onto origin/master bcf263e1)

• go test ./... in cmd/server: ok. In cmd/ingestor: ok. [T]
• sh test-all.sh: 246 passed, 0 failed. node test-frontend-helpers.js: 713 passed, 0 failed. [T]
• Local Go server on a fixture prepared as in CI: freshen, seed SQL, corescope-migrate, seeds 2073 and 199. [T]
  • test-regions-map-e2e.js: 4/4 pass.
  • test-nav-priority-1391-e2e.js: 36/36 pass.
  • The Regions case in test-e2e-playwright.js passes. The same file then fails fast at "Version info lives on Perf dashboard, not in navbar" (waits for #navStats, 10 s timeout). The identical failure occurs on a pure origin/master tree with the same fixture, so it is unrelated to this PR and local-environment related. CI ran the full suite green.
• CI run 38524238998: all jobs success. Release, Deploy Staging and Badges were skipped. No flaky test (flaky: #226 Hash Stats adopters sort E2E — server sorts multiByteNodes on packets only, ties come out in random order #256, flaky: TestResolvedPathBackfill_WriteHoldUnderBudget_188 — wall-clock write-hold budget fails under CI load #267) was involved. [T]

Mutants (my own, on public/regions-map.js)

Mutant	Result
M1 no theme observer	killed (theme test)
M2 no disconnect() in destroy()	killed (teardown test)
M3 popup name via innerHTML	killed (escaping test)
M4 tile filter set only once	killed (theme test)
M5 tile layer never swapped, early return	killed (theme test, via the filter)
M6 tile layer never swapped, filter still cleared	survived (finding 1)
M7 hull <= 0 changed to < 0	survived (finding 4, old code)
M8 validScope accepts :	killed (unit test)

Not verified

• Visual rendering of tiles and the hull in a real browser, and the author's light/dark screenshots. My coverage is DOM and E2E assertions only.
• Behaviour against real transported_scopes_recent data. The fixture has none, so every test uses a mocked /api/nodes.
• The fetchAllNodes cap and truncation messaging, and the antimeridian and polar hull cases.
• Any staging or production behaviour. I made no pushes or merges.