Skip to content

fix(map): use the configured tile provider everywhere (#332) - #344

Merged
dborup merged 3 commits into
masterfrom
codex/issue-332-tile-provider-fallback
Oct 7, 2026
Merged

dborup merged 3 commits into
masterfrom
codex/issue-332-tile-provider-fallback

Conversation

@adminopenclaw8-sketch

Copy link
Copy Markdown
Collaborator

Relates to #332

Keyless CARTO raster tiles have been stamped API KEY REQUIRED since August 2026, and this fork has no CARTO basemap-key plumbing (#13 proposed one and was closed). Several maps still built their tile URL — and their attribution credit — from a hard-coded CARTO template instead of the operator's configured dark/light provider, so they rendered watermarked tiles even on an instance configured for Esri/OpenTopoMap.

Plan (as implemented)

  1. Write the failing tests first: a unit suite over the registry, getTileUrl(), the two customizer call sites and the two map resolvers, plus a Playwright E2E that records every tile request a page makes.
  2. Make map-tile-providers.js the single source of truth with a new MC_getTileSpec(type) resolver, and move the built-in fallback off CARTO.
  3. Route every remaining map through it, and give the one registry-less page (the standalone geo-filter builder) the same non-CARTO baseline.
  4. Update the two existing tests that pinned the old CARTO defaults, and wire both new tests into test-all.sh / the Playwright job.

What changed

public/map-tile-providers.js

  • The built-in fallback defaults become osm-dark / osm-standard instead of carto-dark / carto-light. OSM Standard is the keyless baseline: no token, tiles to zoom 19, and it is already the unconditional provider for the home mini-map, node-reach, packet-path and area maps — so this default contacts no third party the app did not already contact. A configured map.tiles.darkDefault / lightDefault still takes priority, so an operator who wants CARTO keeps it.
  • Those two ids are now registered regardless of provider gating. Without that, a config that disables everything leaves REGISTRY without the active id, every caller falls through to a URL of its own — which is how the hard-coded CARTO templates spread in the first place — and the default would not be selectable in the layer picker, so a user who switched away could not switch back. enabled still governs every other style of a provider.
  • New MC_getTileSpec(type) returns { id, url, attribution, refUrl, maxZoom, invertFilter } for 'dark' / 'light'. url is always a string template (lazy url functions are invoked) and attribution always comes from the resolved provider.

public/roles.js

  • TILE_DARK / TILE_LIGHT are no longer CARTO templates. They are the last-resort values used when the registry has not loaded at all (standalone pages, unit tests); the legacy tiles.dark / map.tiles.darkUrl config overrides still apply.
  • getTileUrl() honours the configured provider in light mode too. Before this it returned TILE_LIGHT unconditionally, so map.tiles.lightDefault was ignored on every consumer of the helper — the node-detail inset map and the analytics map among them. The legacy accessor path (MC_TILE_PROVIDERS + MC_get*TileProvider without getTileSpec) is kept and now also resolves light.
  • getActiveTileProvider() resolves both themes, so the node-detail map credits the configured light provider instead of a generic fallback. A light style carries no invertFilter, so nothing about the rendering changes.

public/customize-v2.js — both geo-filter preview maps (tab inset and full-screen modal) resolve the configured light provider through a shared _cv2PreviewTile() helper. They stay light-styled whatever the app theme is, which is why they ask for 'light' in both themes.

public/live.js, public/map.js — the tile resolvers prefer MC_getTileSpec, and their url + attribution fallbacks no longer name CARTO. The old bodies hard-coded CARTO three times each: the light-mode url fallback, the reg['carto-dark'] registry fallback and the © OpenStreetMap © CartoDB credit.

public/geofilter-builder.html — this standalone page loads neither roles.js nor the registry, so it has nothing to consult. It gets the same keyless OSM baseline plus a #map .leaflet-tile-pane invert filter (the registry's own INVERT_CSS string) so it keeps the dark map it had under CARTO dark_all.

config.example.json — defaults updated, with a comment explaining why CARTO is no longer the default and that its styles remain available.

cmd/server is untouched: map.tiles already reaches the client verbatim, so no Go change was needed.

Browser check

test-issue-332-no-carto-tiles-e2e.js drives Chromium over every surface the issue names — node detail, the main map, Live, Areas, and both customizer geo-filter previews — in both themes, plus the standalone builder. It records every off-origin image request and asserts two things per surface: zero requests to basemaps.cartocdn.com, and at least one request to the host the configured provider should produce — so "0 CARTO requests" cannot pass by the map simply failing to load. Off-origin scripts and stylesheets (Leaflet, Chart.js) are allowed through unchanged; only tile images are stubbed with a 1×1 PNG, since the point is which URL the page asks for and CI has no business reaching a tile CDN.

Verified four ways against a local server on the committed e2e-fixture.db, prepared as the Playwright job does:

server frontend config.json result
this branch darkDefault: esri-darkgray-labels, lightDefault: opentopomap 11/11 pass — 0 CARTO requests; dark maps hit server.arcgisonline.com, light maps tile.opentopomap.org
origin/master same 4 fail — node detail (light) 20 CARTO requests, both customizer previews 6, the builder 18
this branch none (the default install) 11/11 pass — every map hits tile.openstreetmap.org
origin/master none 9 fail — map 15 CARTO requests, Live 18, node detail 20, customizer 6, builder 18, in both themes

The CI job runs the no-config variant, which is the default-install case the issue describes.

Tests

test-issue-332-tile-provider-fallback.js — 43 assertions in a pure vm sandbox, registered in test-all.sh. It runs the real production call sites rather than grepping: the two customizer L.tileLayer(...) statements and both map resolvers are extracted from source and executed against a spy, so re-hard-coding a URL in either place fails the suite. Coverage: the fallback defaults per theme; the defaults staying resolvable with every optional provider disabled; a configured default still winning; MC_getTileSpec url/attribution/refUrl/maxZoom/invertFilter and function-valued urls; getTileUrl() for carto-dark, carto-voyager-dark, positron-dark, esri, carto-light, carto-voyager, opentopomap, usgs-topo, usgs-imagery, maptiler-upgraded OSM and Stamen; four fallback paths including a malformed style; the legacy accessor shape for both themes; getActiveTileProvider() in both themes; and a static guard that the only basemaps.cartocdn.com literal left under public/ is inside _getCartoBase().

13 mutants, one per requirement and then some, all killed: CARTO defaults restored; the always-register guarantee dropped; getTileSpec ignoring its type; getTileSpec leaking a function-valued url; getTileUrl() short-circuiting light to TILE_LIGHT; TILE_DARK/TILE_LIGHT back to CARTO; getActiveTileProvider() nulling in light mode; the customizer previews re-hard-coded; the preview helper asking for 'dark'; the CartoDB attribution fallback restored in live.js; map.js skipping MC_getTileSpec; the legacy branch using the wrong accessor per theme; and the builder back on CARTO.

Two existing tests pinned the behaviour this issue changes and were updated, with the reason recorded inline:

  • test-issue-1420-tile-providers.js — the fallback-id expectations, the "default registry contains only Carto" premise, the OSM gating assertion (the two fallback styles survive osm.enabled: false), and the registry-rebuild test (now observes a token-gated Stamen style instead). 39/39.
  • test-issue-1470-node-tile-helper.js — getActiveTileProvider returns null in light mode becomes returns the LIGHT provider in light mode, and now also asserts the light style carries no invert filter and follows the selected light provider. 6/6.

Local runs: sh test-all.sh 228/228 files green; go test ./... green in all 18 modules; node scripts/check-css-vars.js clean (no colours added — the one new CSS declaration is a filter); scripts/check-xss-sinks.sh --diff origin/master clean; npx eslint public/*.js 0 errors; fork guards unchanged (9 in deploy.yml, 1 in release-fast-path.yml). Affected Playwright suites green against the local fixture server: the new #332 E2E, test-e2e-playwright.js (132/135, 3 skipped — identical to origin/master), map modal/lifecycle/pagination/mobile/controls, live layout/fullscreen/GPU animations, node-reach and node-reach-coverage, route-map a11y, analytics subtab deeplinks.

Notes

  • CARTO is not removed. Its five styles stay registered and selectable, and darkDefault / lightDefault still accept them — this changes only what an unconfigured instance resolves to. A CARTO basemap-key path remains possible later; _getCartoBase() is still the single place a CARTO URL is built.
  • osm.enabled: false no longer removes osm-standard / osm-dark from the registry, since they are the built-in defaults. The flag's only effect was registry membership (_getOsmUrl() never read it), so nothing else about the provider changes.
  • Out of scope, unfiled: nodes.js _applyTilesToNodeMap() still hard-codes maxZoom: 18, so a provider capped lower (OpenTopoMap at 17, USGS at 16) over-zooms into blank tiles on the node-detail inset map. Pre-existing and independent of this change.
  • Out of scope, unfiled: the existing esri-darkgray-labels style declares maxZoom: 19, but the Esri Canvas service serves real imagery only to z16 and a blank tile above it (verified by probing z14/16/17/18/19 over land). That is why the fallback here is OSM rather than the Esri canvas.
  • public/geofilter-builder.html styles its chrome with hard-coded hex colours rather than CSS variables. Pre-existing across the whole file and left alone.

🤖 Generated with Claude Code

…vider

Keyless CARTO raster tiles have been stamped "API KEY REQUIRED" since
August 2026, and this fork has no CARTO basemap-key plumbing. Several
maps still built their tile URL from a hard-coded CARTO template instead
of the operator's configured dark/light provider, so they rendered
watermarked tiles even on an instance configured for Esri/OpenTopoMap.

- map-tile-providers.js: the built-in fallback defaults become the
  keyless OSM baseline ('osm-dark' / 'osm-standard') instead of
  'carto-dark' / 'carto-light'. Those two ids are now registered
  regardless of provider gating, so the active id always resolves and
  stays selectable in the layer picker. New MC_getTileSpec(type) is the
  single resolver for url + attribution + refUrl + maxZoom +
  invertFilter, per theme.
- roles.js: TILE_DARK / TILE_LIGHT are no longer CARTO templates, and
  getTileUrl() honours the configured provider in LIGHT mode too — it
  previously returned TILE_LIGHT unconditionally, ignoring
  map.tiles.lightDefault. getActiveTileProvider() resolves both themes,
  so the node-detail inset map credits the configured light provider.
- customize-v2.js: both geo-filter preview maps resolve the configured
  light provider through a shared _cv2PreviewTile() helper.
- live.js / map.js: the tile resolvers go through MC_getTileSpec, and
  their url + attribution fallbacks no longer name CARTO.
- geofilter-builder.html: this standalone page has no registry, so it
  uses the same keyless OSM baseline plus a tile-pane invert filter to
  keep its dark look.
- config.example.json: defaults updated, with a note on why CARTO is no
  longer the default.

Relates to #332

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@adminopenclaw8-sketch

Copy link
Copy Markdown
Collaborator Author

Rapport — CS-Macmini PR#344 #332 — head aac9445

Status: Implemented and verified — every map now takes its tile URL and attribution from the configured dark/light provider, the fallback is non-CARTO, 43 unit assertions and an 11-step browser check are green, 13/13 mutants killed, CI all green on the first run (3 jobs pass, 3 skipped by the fork guard).

Evidence tags: [T] test run, [A] assertion/code reference, [K] killed mutant.

Requirements

# Requirement Test (red before → green after) Mutant
1 roles.js getTileUrl() takes its URL from the configured provider, with TILE_DARK/TILE_LIGHT no longer CARTO [T] test-issue-332-tile-provider-fallback.js M3 — 9 provider cases (carto-dark, carto-voyager-dark, positron-dark, esri, carto-light, carto-voyager, opentopomap, usgs-topo, usgs-imagery) + maptiler-upgraded OSM + Stamen; red before on 6 of the 9 (light mode returned TILE_LIGHT unconditionally) [A] public/roles.js _activeTileStyle() / getTileUrl() [K] getTileUrl short-circuits light to TILE_LIGHT; [K] TILE_DARK/TILE_LIGHT back to CARTO
2 Customizer preview maps take URL + attribution from the provider [T] M4 — 4 assertions; the two production L.tileLayer(...) statements are extracted from customize-v2.js and executed against a spy, so re-hard-coding either fails; red before (_cv2PreviewTile did not exist) [A] public/customize-v2.js _cv2PreviewTile() [K] previews re-hard-code CARTO; [K] preview helper asks for 'dark'
3 live.js attribution fallback is not CARTO [T] M5 — 10 assertions across live.js _liveResolveTile and map.js _resolveTileUrl, both extracted and run; red before (© OpenStreetMap © CartoDB) [A] public/live.js, public/map.js [K] CartoDB attribution fallback restored; [K] map.js skips MC_getTileSpec; [K] legacy branch uses the wrong accessor per theme
4 Fallback with no provider configured is non-CARTO; CARTO kept only behind a key [T] M1 — 4 assertions on the fallback id, its URL, its presence in REGISTRY with every optional provider disabled, and a configured default still winning; red before (carto-dark/carto-light) [A] map-tile-providers.js DEFAULT_ID = 'osm-dark', DEFAULT_ID_LIGHT = 'osm-standard'. No key path exists in this fork (#13 closed), so the fallback is unconditionally non-CARTO; CARTO styles stay registered and selectable via darkDefault/lightDefault [K] defaults back to carto-dark/carto-light; [K] always-register guarantee dropped
5 Single resolver so no caller invents a URL [T] M2 — 6 assertions on MC_getTileSpec(type): per-theme id, string url from a lazy url function, provider attribution, invertFilter, maxZoom, no CARTO credit for a non-CARTO provider [A] map-tile-providers.js getTileSpec() [K] getTileSpec ignores type; [K] getTileSpec leaks a function-valued url
6 Browser check: 0 requests to basemaps.cartocdn.com from node detail, the map, Live, Areas and the customizer previews [T] test-issue-332-no-carto-tiles-e2e.js — 11 steps, both themes, plus the standalone builder. Table below. [K] geofilter-builder back to CARTO (static guard M6)
7 No hard-coded CARTO literal left [T] M6 — scans every public/**/*.{js,html}; the only basemaps.cartocdn.com occurrence allowed is inside _getCartoBase(). Red before on 5 lines (roles.js ×2, customize-v2.js ×2, geofilter-builder.html) [K] geofilter-builder back to CARTO

Unit suite: 43 passed, 0 failed [T]. Mutants: 13 applied, 13 killed, 0 survivors [T]. One mutant (legacy branch, wrong accessor per theme) survived the first round and a test was added to close that gap before re-running.

Browser check — recorded tile requests

Chromium, local server on the committed e2e-fixture.db prepared as the Playwright job does. Off-origin tile images are stubbed with a 1×1 PNG and their URLs recorded; off-origin scripts/stylesheets (Leaflet, Chart.js) load unchanged. Each surface asserts both 0 CARTO requests and at least one request to the expected provider host, so a map that fails to load cannot pass.

server frontend config.json result
this branch darkDefault: esri-darkgray-labels, lightDefault: opentopomap 11/11 pass — 0 CARTO; dark maps hit server.arcgisonline.com, light maps tile.opentopomap.org [T]
origin/master same 4 fail — node detail (light) 20 CARTO requests, customizer previews 6, builder 18 [T]
this branch none (default install) 11/11 pass — every map hits tile.openstreetmap.org [T]
origin/master none 9 fail — map 15, Live 18, node detail 20, customizer 6, builder 18, in both themes [T]

The CI job runs the no-config variant, which is the default-install case the issue describes. Areas was already non-CARTO on master (it uses plain OSM) and is asserted for regression only.

Existing tests updated

Two tests pinned the behaviour this issue changes; both were updated with the reason recorded inline, not deleted.

  • test-issue-1420-tile-providers.js — fallback-id expectations, the "default registry contains only Carto" premise, the OSM gating assertion (the two fallback styles survive osm.enabled: false), and the registry-rebuild test, which now observes a token-gated Stamen style. 39/39 [T]
  • test-issue-1470-node-tile-helper.js — getActiveTileProvider returns null in light mode becomes returns the LIGHT provider in light mode, and additionally asserts the light style carries no invert filter and follows the selected light provider. 6/6 [T]

CI per job

Run 37596269519, no re-runs needed.

Job Result Duration Note
✅ Go Build & Test pass 24m39s sh test-all.sh 228/228 files; test-issue-332-tile-provider-fallback.js 43 passed, 0 failed [T]. Includes the XSS --diff gate, the CSS-variable lint and the eslint no-undef step.
🎭 Playwright E2E Tests pass 21m59s test-issue-332-no-carto-tiles-e2e.js 11 passed, 0 failed [T] — 0 CARTO requests from map, Live, node detail, Areas and the customizer previews in both themes, plus the standalone builder. Run with no config.json, i.e. against the built-in non-CARTO fallback.
🏗️ Build & Publish Docker Image pass 48s local image build only (no GHCR push on a fork)
📦 Release Artifacts skipping — fork guard
📝 Publish Badges & Summary skipping — fork guard
🚀 Deploy Staging skipping — fork guard

The known-flaky #271 test did not fail, so no job was re-run.

Local gates

Gate Result
sh test-all.sh 228/228 files green [T]
go test ./... green in all 18 modules [T]
scripts/check-xss-sinks.sh --diff origin/master clean, exit 0 [T]
node scripts/check-css-vars.js clean, 0 undefined — no colours added; the one new CSS declaration is a filter [T]
npx eslint public/*.js (eslint@8) 0 errors (88 pre-existing warnings, unchanged) [T]
cmd/server read-only untouched — map.tiles already reaches the client verbatim [A]
new map[string]interface{} outside tests none added [A]
Fork guards 9 in deploy.yml, 1 in release-fast-path.yml — unchanged [T]
Affected Playwright suites new #332 E2E; test-e2e-playwright.js 132/135, 3 skipped — identical to origin/master; map modal/lifecycle/pagination/mobile/controls; live layout/fullscreen/GPU animations; node-reach and node-reach-coverage; route-map a11y; analytics subtab deeplinks — all green [T]

Residuals

  • osm.enabled: false no longer removes osm-standard / osm-dark from the registry, because they are the built-in defaults — the active id must always resolve and must stay selectable in the layer picker. The flag's only prior effect was registry membership (_getOsmUrl() never read it), and OSM tiles were already fetched unconditionally by six other maps, so no new third party is contacted. Documented in config.example.json and in the code comment.
  • Out of scope, unfiled: nodes.js _applyTilesToNodeMap() hard-codes maxZoom: 18, so a provider capped lower (OpenTopoMap 17, USGS 16) over-zooms into blank tiles on the node-detail inset map. Pre-existing.
  • Out of scope, unfiled: the existing esri-darkgray-labels style declares maxZoom: 19, but the Esri Canvas service serves real imagery only to z16 and a blank tile above it (probed at z14/16/17/18/19 over land). That is why the fallback here is OSM rather than the Esri canvas.
  • Out of scope: public/geofilter-builder.html styles its chrome with hard-coded hex colours instead of CSS variables. Pre-existing across the whole file; the only thing added there is the tile URL, its attribution and a tile-pane filter.
  • CARTO is not removed. Its five styles stay registered and selectable, _getCartoBase() remains the single place a CARTO URL is built, and a basemap-key path can be added later without touching any call site.

@dborup-agent

Copy link
Copy Markdown
Collaborator

Review — CS-pve-agent1 PR#344 — head aac9445

Dom: REQUEST CHANGES

The core of the PR is right. With a non-CARTO provider configured, I measured 0 requests to basemaps.cartocdn.com across 20 map surfaces in both themes. Master makes 135 of them with the same config, and 392 with no config. The fallback is keyless OSM, not CARTO. The new unit suite is red on master and green on head.

Two things block. First, the change of default dark style introduces a dark-mode regression on a map the PR doesn't touch: the analytics subpath minimap. Second, the E2E misses two of the surfaces the review brief lists. Packet-path and the analytics minimap are not covered, and the "Areas" step asserts nothing. Both fixes are small.

Evidence tags: [T] test or browser run by me, [A] code reading or analysis, [K] mutant run by me.

Findings

# Sev Where Finding Evidence
1 Medium (blocking) public/analytics.js:2524 Dark-mode regression. The Analytics → Subpaths detail minimap does L.tileLayer(getTileUrl()) and never applies the provider's invertFilter. The new default dark style is osm-dark, which is the light OSM Standard template plus an invert filter. On a default install in dark mode, this map now renders the un-inverted light OSM map. On master it rendered CARTO dark_all, which is natively dark. Every other map applies the filter. Fix: replace the line with window._applyTilesToNodeMap(map) (packet-path, area-nodes, node-reach and rx-coverage already do this), and add the surface to the E2E. [T] No-config server, dark theme. #subpathMap .leaflet-tile-pane has computed filter: none and tiles from tile.openstreetmap.org. On the same server, #/map, Live, node detail, packet-path and area map all have invert(1) hue-rotate(180deg)…. Master, same page: CARTO tiles, so natively dark. [A] The line is unchanged by the PR; the regression comes from DEFAULT_ID = 'osm-dark'.
2 Medium (blocking) test-issue-332-no-carto-tiles-e2e.js:152-161 E2E coverage gap. (a) There is no packet-path map step and no analytics subpath step. (b) The "Areas" step is vacuous. The Analytics → Areas tab mounts no Leaflet map, and the step runs with requireHit: false, so it passes on 0 tiles. The Areas map that does exist is AreaNodesMap (Position-gaps), and the test never opens it. [K] Mutant E: the subpath minimap takes MC_TILE_PROVIDERS['carto-dark'].url(), so no literal. It survives both the #332 unit suite (43/43) and the #332 E2E (11/11). Only my own network check caught it (6 CARTO requests). [T] Opening AreaNodesMap.open(...) and PacketPathMap.open(hash) directly works fine on head (0 CARTO), so these are easy to add.
3 Low public/map-tile-providers.js:59-60 Attribution on the new default is wrong for a keyless install. osm-standard / osm-dark credit © OpenStreetMap contributors, Maps © Mapbox/Thunderforest/MapTiler. Without an osm.provider token none of those three services is used. The string predates the PR, but #332 promotes it to the default that every unconfigured instance shows. Suggest resolving it like _getOsmUrl(), so plain OSM gets © OpenStreetMap contributors and the vendor credit appears only when that vendor is configured. [T] The #/map attribution on the no-config server reads Leaflet | © OpenStreetMap contributors, Maps © Mapbox/T….
4 Nit public/map-tile-providers.js:24, config.example.json:97 The comment and _comment_defaults say "tiles to zoom 19", but both OSM styles declare maxZoom: 18, and MC_getTileSpec returns 18. [A]
5 Nit public/customize-v2.js:1640-1641 The tile-picker still falls back to the ids 'carto-dark' / 'carto-light' when the accessors are missing. It is unreachable in practice (the registry check returns early first), but it is the last CARTO default in the frontend. [A]
6 Info map-tile-providers.js:103-104 Behaviour change: osm.enabled: false no longer hides OSM. Every instance's layer picker gains two entries labelled "OSM Standard", one light and one dark. The trade-off is reasonable and documented. I'm flagging it only because it is visible to operators. [A], [T]

Answers to the review brief

  1. Network check (non-CARTO provider configured). [T] I ran my own Playwright script on a local server with darkDefault: esri-darkgray-labels, lightDefault: opentopomap, and both carto.enabled: true and opentopomap.enabled: true. It records every request of every resource type, not only images. Tile images are stubbed with a 1×1 PNG. Result: 0 CARTO requests. The surfaces were: map, Live, node detail, Areas (AreaNodesMap), analytics subpath minimap, packet-path map, customizer geo-filter inset and modal (each in dark and light), plus geofilter-builder.html and area-map.html.

    • Dark maps hit server.arcgisonline.com.
    • Light maps hit tile.opentopomap.org.
    • The customizer previews hit OpenTopoMap in both themes, which matches their deliberate light-only styling.

    Master with the same config made 135 CARTO requests across 9 surfaces. These were node detail (light), Areas (light), the subpath minimap (light), packet-path (light), both customizer previews (both themes), and the builder. That matches the issue.

  2. Fallback with no provider configured. [T] Dark is osm-dark and light is osm-standard, both on {s}.tile.openstreetmap.org. Dark is inverted via INVERT_CSS. Measured in the browser on a server with no config.json: 0 CARTO requests across all 20 surfaces. Master made 392 CARTO requests across 17 of them. So the fallback is not keyless CARTO. [A] Caveat: a browser that once explicitly picked a CARTO style keeps it via localStorage. That is a deliberate user choice, which is fine. Note, though, that the customizer's "Reset all" clears only mc-dark-tile-provider and not mc-light-tile-provider. That predates the PR.

  3. Tests. [T] test-issue-332-tile-provider-fallback.js passes 43/0 on head and fails 9 passed / 34 failed on master. It covers getTileUrl() for carto-dark, carto-voyager-dark, positron-dark, esri, carto-light, carto-voyager, opentopomap, usgs-topo, usgs-imagery, maptiler-upgraded OSM and Stamen, plus four fallback paths. [T] test-issue-332-no-carto-tiles-e2e.js passes 11/0 against my head server with no config. Mutants are listed below.

  4. Regression: dark/light switching and attribution. [T] I toggled #darkModeToggle twice (light → dark → light) on #/map, #/live and node detail. Without config, each map switches between OSM with no filter and OSM with the invert filter. With the Esri/OpenTopo config, each switches between OpenTopoMap and Esri with no filter. Master behaves the same, except it uses CARTO. Attribution comes from the configured provider: Tiles © Esri, the OpenTopoMap CC-BY-SA credit, and the OSM credit (wording issue in F3). There is no CartoDB credit left anywhere on head. Regression found: F1 (analytics subpath minimap in dark mode).

  5. XSS gate and colours. [T] bash scripts/check-xss-sinks.sh --diff origin/master exits 0. node scripts/check-css-vars.js reports 0 undefined. [A] The added lines contain no hex or rgb colours (every # match is an issue number). The one new CSS rule is a filter.

Standing checks

Tests and mutants

All local runs were on the merged tree. git merge-tree --write-tree origin/master aac9445a… gives tree d7daec98, which is identical to head's tree because head is based on 6444294d.

Suite Result
go test -timeout 40m ./... in cmd/server ok github.com/corescope/server (1345 s) [T]
go test -timeout 40m ./... in cmd/ingestor 1 failure: TestBackfillPathJSONAsync, "OpenStore blocked for 4.94s". This is a wall-clock assertion, and it failed while the server suite was running in parallel on the same host. Run in isolation it passes 3/3 on head and 1/1 on master. The PR changes no Go code, and the CI ingestor job is green on this head. It is not one of the listed flaky tests (#256, #267), but it is the same timing-under-load class. [T]
sh test-all.sh 228/228 files green [T]. A first run made in parallel with the Go suites had 1 failure in test-channels-client-state-152.js (R4-3, a timing assertion). That test passes 5/5 in isolation and on master, and channels.js is untouched by the PR.
node test-frontend-helpers.js 709 passed, 0 failed [T]
#332 unit / Kpa-clawbot#1420 / Kpa-clawbot#1470 / Kpa-clawbot#1614 43/0 · 39/0 · 6/0 · 3/0 [T]
#332 E2E against a no-config local server 11/0 [T]
Related E2Es against a local server on e2e-fixture.db, prepared as in CI (freshen, seed SQL, corescope-migrate, then seeds 2073, 199 and 245) test-map-modal-fluid 10/0, test-issue-123-map-lifecycle 15/0, test-map-nodes-pagination 3/0, test-issue-1236-map-mobile 3/3, test-issue-1329-map-controls-accordion 5/5, test-live-layout-1178-1179 10/0, test-live-fullscreen-1572 9/0, test-pr-1490-live-map-gpu-animations 14/0, test-node-reach OK, test-issue-1374-route-map-a11y 20/0, test-issue-205-analytics-subtab-deeplinks 15/0, test-customize-theme 8/8 [T]. test-node-reach-coverage SKIPs here because clientRxCoverage is off on this config.
test-e2e-playwright.js (run with fail-fast disabled in a scratch copy) head: 131 pass, 1 fail, 7 skip. master: 131 pass, 1 fail, 7 skip, identical. The one failure is "Version info lives on Perf dashboard" (#navStats wait timeout). It fails the same way on master, so it comes from my local environment, not the PR. [T]

Mutants (my own; each was applied to a scratch copy of the merged tree):

Mutant Result
A — analytics subpath minimap hard-codes the CARTO light_all literal [K] killed by #332 M6 (static guard)
B — _cv2PreviewTile() resolves MC_TILE_PROVIDERS['carto-light'] (no literal) [K] killed by #332 M4 (3 failures)
C — only DEFAULT_ID_LIGHT reverted to 'carto-light' [K] killed by #332 M1 and by Kpa-clawbot#1420 (6 failures)
D — _applyTilesToNodeMap takes carto-dark from the registry in dark mode (no literal) [K] survives the #332 unit suite. Killed by Kpa-clawbot#1470 and by the #332 E2E ([dark] node detail: 20 request(s) to basemaps.cartocdn.com)
E — analytics subpath minimap takes carto-dark from the registry (no literal) SURVIVES the #332 unit suite (43/0) and the #332 E2E (11/0). Caught only by my own network check (6 CARTO requests). This is the gap in F2.

Not verified

  • Real tile rendering. Every third-party tile image was stubbed. The dark-mode verdict in F1 rests on the computed filter and the tile URL, not on looking at real OSM pixels.
  • Token-based OSM providers (MapTiler, Thunderforest, Mapbox), Stamen with a token, and a CARTO enterprise domain. These were tested only through the unit suite.
  • The home mini-map (home.js:527) did not render in my runs because no "my node" was claimed. It and area-map.html hard-code OSM, so they ignore the configured provider and are never inverted. Neither is CARTO, and both predate the PR. A follow-up could route them through _applyTilesToNodeMap too.
  • Docker image, Firefox/Safari, mobile viewports, and staging or prod (out of scope).
  • The OSM tile usage policy as it applies to a default install. A default install now sends every user's tiles to the OSMF servers, via the deprecated {s}. subdomains. This is a policy question for the operator, not a defect in this PR.

Head was checked with git ls-remote before and after the review: aac9445a965227313131a8808042fd32a6dc5457 both times.

dborup added 2 commits October 7, 2026 16:40
Round-2 review findings on #344 (#332).

F1 (blocking) — dark-mode regression. The Analytics -> Subpaths detail
minimap did `L.tileLayer(getTileUrl())` and never applied the provider's
invertFilter. #332 makes `osm-dark` the dark default, and that style is
the LIGHT OSM template plus an invert filter, so on a default install in
dark mode this map rendered an un-inverted light basemap inside a dark
page. It now goes through the one shared helper every other inset map
uses (nodes.js `_applyTilesToNodeMap`), which resolves the configured
provider for the current theme and applies its invertFilter to that
map's own tile pane. It was the only `L.tileLayer(getTileUrl())` call
left in the frontend.

F2 (blocking) — E2E coverage gaps. Added steps for the Analytics ->
Subpaths minimap and the packet-path modal, and replaced the vacuous
"Areas tab" step: that tab mounts no Leaflet map at all (its
position-gap rows are not clickable there), so the step passed on zero
tiles. The Areas map that exists is AreaNodesMap, from the Position-Fix
Coverage Gaps tool. Every map step now also asserts that the resolved
provider's invertFilter reaches the tile pane -- read from the page, so
it holds for any configured provider.

F3 — attribution on the new default. `osm-standard` / `osm-dark`
credited "Maps (c) Mapbox/Thunderforest/MapTiler" unconditionally, but a
keyless install uses none of those three. The credit is now resolved the
same way `_getOsmUrl()` picks the URL: plain OSM by default, the one
configured vendor when a token is set.

F4 — the comment and config.example.json said "tiles to zoom 19"; both
OSM styles declare maxZoom 18.

F5 — the tile picker's last-resort ids were 'carto-dark' / 'carto-light'.
They now come from the registry's own exported defaults.

Relates to #332
@adminopenclaw8-sketch

Copy link
Copy Markdown
Collaborator Author

Rapport — CS-Macmini PR#344 runde-2 — head 26beef7

Review feedback addressed (commit 26beef7e). origin/master (e8e99c4a) merged in as 4b3b5334 first; the fixes are the single commit on top. PR stays draft.

Evidence tags: [T] test or browser run by me, [A] code reading, [K] killed mutant.

1. F1 (blocking) — dark-mode regression on the Analytics → Subpaths minimap — fixed

public/analytics.js now routes the minimap through the one shared helper instead of resolving tiles itself:

const map = L.map('subpathMap', { zoomControl: false, attributionControl: false });
if (typeof window._applyTilesToNodeMap === 'function') {
  window._applyTilesToNodeMap(map);
} else { /* loud warn + keyless OSM */ }

That is the same call the node-detail inset, node-reach, packet-path, area-nodes and rx-coverage maps already make, so there is no second resolver: the helper picks the provider for the current theme, carries its attribution and refUrl, and applies its invertFilter to that map's own tile pane.

I swept the frontend for other call sites missing it. L.tileLayer(getTileUrl()) existed in exactly one place — this one. [A] The remaining L.tileLayer(...) statements are: the five helper call sites' unreachable else fallbacks (plain OSM, pre-existing idiom), map.js/live.js (both go through MC_getTileSpec), the two customizer previews (_cv2PreviewTile), the layer control inside map-tile-providers.js, and home.js / area-map.html / geofilter-builder.html, which hard-code OSM and have no registry — pre-existing, listed as residuals in round 1 and unchanged here.

Verified in a real browser on a no-config server: [T] #subpathMap .leaflet-tile-pane computes filter: invert(1) hue-rotate(180deg) brightness(0.9) contrast(1.05) in dark mode and none in light, with tiles from tile.openstreetmap.org in both.

2. F2 (blocking) — E2E coverage gaps — fixed

test-issue-332-no-carto-tiles-e2e.js goes 11 → 15 steps:

  • New: Analytics → Subpaths minimap (both themes). Clicks route rows until one has ≥2 located nodes (which is what makes renderSubpathDetail() mount #subpathMap), and asserts loudly if none of the first 15 rows mounts it — so a fixture change cannot silently turn the step into a no-op.
  • New: packet-path map (both themes). Reached the way a shared link reaches it: #/packets/<hash>?viewPath=1. The hash is discovered at runtime from /api/packets + /api/packets/<hash>/path, so the step does not depend on one fixture row surviving a freshen-fixture.sh.
  • Areas step fixed. You were right that it was vacuous: the Analytics → Areas tab mounts no Leaflet map at all — its position-gap rows carry no data-area-key, so openAreaMap() is never reachable from there [A] (analytics.js gapsRowHtml vs position-gaps.js:157). The step now targets the Areas map that does exist, AreaNodesMap, on #/tools/position-gaps, with requireHit: true. The committed fixture has positionGaps: [] and estimatedNodes: [] [T], so there is no row to click; the step drives the modal's own public entry point AreaNodesMap.open(label, points) with two synthetic points. The production tile-attach path is identical either way — only the markers come from the test.
  • New assertion on every map step: checkFilter() reads window.MC_getTileSpec(theme).invertFilter from the page and asserts the tile pane's computed filter matches — present when the provider declares one, absent when it does not. It is read from the page rather than hard-coded, so it holds for any operator config, and it is what makes F1 catchable in the browser.

The test can fail. Four mutants, each run against both suites:

Mutant Unit suite #332 E2E
A — subpath minimap back to L.tileLayer(getTileUrl()), i.e. the pre-fix code [K] 4 failures [K] [dark] … declares invertFilter "invert(1) …" but #subpathMap .leaflet-tile-pane computes filter "none"
B — your mutant E: subpath minimap takes MC_TILE_PROVIDERS['carto-dark'].url(), no literal [K] 5 failures [K] 2 steps, 3 request(s) to basemaps.cartocdn.com
C — one hard-coded CARTO URL in packet-path-map.js (https://{s}.basemaps.cartocdn.com/dark_all/{z}/{x}/{y}.png) [K] M6 static guard [K] 2 steps, 18 request(s) to basemaps.cartocdn.com
D — area-nodes-map.js takes MC_TILE_PROVIDERS['carto-light'].url(), no literal survives (57/0) [K] 2 steps, 8 request(s) to basemaps.cartocdn.com

Mutant B is the one that survived everything in round 1; D is the Areas-shaped equivalent and is killed only by the repaired Areas step. [K]

3. F3 — attribution on the new default is now correct for a keyless install — fixed

osm-standard / osm-dark credited © OpenStreetMap contributors, Maps © Mapbox/Thunderforest/MapTiler unconditionally. The credit is now resolved exactly the way _getOsmUrl() picks the URL, in a new _getOsmAttribution(), and is re-resolved on every MC_initTileRegistry() so config arriving after parse time still applies:

  • no osm.provider + osm.token → © OpenStreetMap contributors
  • maptiler + token → © OpenStreetMap contributors, Tiles © MapTiler (likewise Thunderforest, Mapbox — one vendor only)
  • a provider with no token → plain OSM, because the tiles still come from the OSMF servers

It is resolved into a plain string at registry-build time, so every reader stays unchanged (getTileSpec, the layer control, _applyTilesToNodeMap, the map.js/live.js legacy branches). [A]

Browser, no-config server: [T] the #/map attribution control now reads Leaflet | © OpenStreetMap contributors in both themes (was … , Maps © Mapbox/T…). With darkDefault: esri-darkgray-labels / lightDefault: opentopomap it reads Leaflet | Tiles © Esri and the OpenTopoMap CC-BY-SA credit [T].

4. F4 — zoom text corrected to 18

public/map-tile-providers.js:24 and config.example.json map.tiles._comment_defaults both said "tiles to zoom 19"; both OSM styles declare maxZoom: 18. Both now say 18, and a test ties the documented number to the style's real maxZoom, so they cannot drift apart again. No other "zoom 19" text referred to the OSM default [T].

5. F5 — last CARTO id fallback removed

_renderTileProviderSelector() fell back to the ids 'carto-dark' / 'carto-light'. It now uses window.MC_DARK_TILE_DEFAULT / the newly exported window.MC_LIGHT_TILE_DEFAULT. There is no hard-coded carto-dark/carto-light id left anywhere in public/, and a test guards that [T].

F6 (info) — acknowledged, no change

osm.enabled: false no longer hides OSM, because the two OSM styles are the built-in defaults and the active id must always resolve and stay selectable. It is documented in the code comment and in config.example.json, and it is listed as a residual in the PR body. Agreed it is operator-visible; I left the trade-off as is rather than reintroducing an unresolvable active id.

Tests

New assertions in test-issue-332-tile-provider-fallback.js: 43 → 57, all red before the fix and green after [T].

Section What it pins Red before
M7 (F1) Executes the production renderSubpathDetail() with the production _applyTilesToNodeMap() on top of the real registry + roles.js, against a Leaflet mock whose maps each own their tile pane. 4 cases: dark default → OSM url + invert filter on its own pane; light default → no filter; configured Esri dark → server.arcgisonline.com, no filter, Tiles © Esri; configured OpenTopoMap light. Plus a guard that analytics.js has no inline tile resolver left. 4 of 5 [T]
M8 (F3) 6 cases over MC_getTileSpec: keyless (both themes) credits OSM and names none of the three vendors; each of maptiler/thunderforest/mapbox credits itself and nobody else; provider-without-token credits plain OSM. 6 of 6 [T]
M9 (F4) The documented zoom in the JS comment and in config.example.json equals osm-standard's declared maxZoom and what MC_getTileSpec returns. 1 of 1 [T]
M10 (F5) Runs the extracted _renderTileProviderSelector() with a registry present but the MC_get*TileProvider accessors absent — the exact branch that used to fall back — and asserts no carto-* option is preselected. Plus a no-literal guard. 2 of 2 [T]

Mutants for the non-blocking findings as well — 8 total, 8 killed, 0 survivors [T]:

Mutant Killed by
A, B (F1/F2, above) M7 + E2E
C, D (F2, above) M6 / E2E
E — vendor credit back to unconditional Mapbox/Thunderforest/MapTiler [K] M8, 6 failures
F — JS comment back to "zoom 19" [K] M9
G — config.example.json back to "zoom 19" [K] M9
H — picker ids back to 'carto-dark' / 'carto-light' [K] M10, 2 failures

Local gates

Gate Result
node test-issue-332-tile-provider-fallback.js 57 passed, 0 failed [T]
sh test-all.sh 231/231 files green [T]
node test-frontend-helpers.js 709 passed, 0 failed [T]
#332 E2E, no config.json (what CI runs) 15 passed, 0 failed [T]
#332 E2E, darkDefault: esri-darkgray-labels / lightDefault: opentopomap 15 passed, 0 failed — dark hits server.arcgisonline.com, light tile.opentopomap.org, and checkFilter correctly expects no filter from the natively-dark Esri style [T]
#1420 / #1470 / #1614 tile suites 39/0 · 6/0 · 3/0 [T]
scripts/check-xss-sinks.sh --diff origin/master clean, exit 0 [T]
node scripts/check-css-vars.js 0 undefined (2179 refs, 84 files) [T]
npx eslint public/*.js 0 errors, 88 pre-existing warnings, unchanged [T]
Go suites git diff origin/master...HEAD -- cmd/ internal/ is empty — this round changes no Go at all [T]; the CI Go job is green on this head
Affected E2Es against a local server on e2e-fixture.db, prepared as the CI job does test-customize-theme-e2e 8/8, test-analytics-subtab-deeplinks-205 121/0, test-map-modal-fluid-e2e 10/0, test-issue-123-map-lifecycle-e2e 15/0, test-issue-1236-map-mobile-e2e 3/3, test-issue-1329-map-controls-accordion-e2e 5/5, test-issue-1374-route-map-a11y-e2e 20/0, test-live-layout-1178-1179-e2e 10/0, test-live-fullscreen-1572-e2e 9/0, test-pr-1490-live-map-gpu-animations-e2e 14/0, test-node-reach-e2e OK, test-area-nodes-map 14/0, test-packet-path-map 76/0, test-position-gaps-tool 25/0, test-analytics-distance-view-path 8/0 — all green [T]. test-node-reach-coverage-e2e SKIPs here (clientRxCoverage off on this config).

CI per job

Run 37641463819, attempt 1, head 26beef7e. No re-runs needed — neither #271 nor #301 fired.

Job Result Duration Note
✅ Go Build & Test pass 23m29s log: ✅ #332: 57 passed, 0 failed, then 231 passed, 0 failed (231 files) · All tests passed [T]
🎭 Playwright E2E Tests pass 25m55s log: #332 no-CARTO E2E: 15 passed, 0 failed, with the three new surfaces green in both themes [T]
🏗️ Build & Publish Docker Image pass 48s local image build only (no GHCR push on a fork)
📦 Release Artifacts skipped — fork guard
🚀 Deploy Staging skipped — fork guard
📝 Publish Badges & Summary skipped — fork guard

Notes and residuals

  • The else branch added in analytics.js keeps the established idiom of the other four call sites (warn + keyless OSM) and is unreachable on the shipped page, since nodes.js registers the helper at parse time and loads before analytics.js in index.html [A]. Like those four, it does not apply an invert filter — if that fallback is ever reached there is no registry to read one from.
  • Still out of scope and unfiled, unchanged from round 1: _applyTilesToNodeMap hard-codes maxZoom: 18, so a provider capped lower (OpenTopoMap 17, USGS 16) over-zooms into blank tiles on every inset map — the subpath minimap now inherits that too, in exchange for the filter it was missing. home.js and area-map.html hard-code OSM and ignore the configured provider. The customizer's "Reset all" clears mc-dark-tile-provider but not mc-light-tile-provider.
  • Not verified: real tile pixels (every third-party tile image is stubbed with a 1×1 PNG, so the dark-mode verdict rests on the computed filter plus the requested URL), token-based OSM providers in a browser (unit-tested only), Firefox/Safari, mobile viewports, staging and prod.

@dborup-agent

Copy link
Copy Markdown
Collaborator

Review — CS-pve-agent3 PR#344 — head 26beef7

Dom: APPROVE med nits

This is an independent re-review of round 2. All five round-1 findings (F1–F5) are fixed, and the fixes hold up under my own mutants. Across 51 routes, both themes and two configs, the browser sweep saw 0 requests of any resource type to basemaps.cartocdn.com. The same sweep on master saw 234 per theme. The Analytics → Subpaths minimap now carries the provider's invertFilter in dark mode and no filter in light mode. All remaining items are nits: one untested CSS rule and some stale comments and PR text.

Evidence tags: [T] test or browser run by me, [A] code reading, [K] mutant run by me.

Findings

# Sev Where Finding Evidence
1 Nit test-issue-332-no-carto-tiles-e2e.js (builder step), public/geofilter-builder.html:28 The builder's new #map .leaflet-tile-pane { filter: invert(…) } rule has no test. It is what keeps the builder dark now that the keyless OSM tiles are light. Deleting the rule passes the unit suite (57/0) and the E2E (15/0). Suggest a computed-filter assertion in the builder step, as checkFilter already does for the in-app maps. [K] mutant K4 survives both suites
2 Nit public/map-tile-providers.js:4,7 and :24-26 Stale or inaccurate comments. The file header still says "Carto (default)" and "→ 'carto-dark' / 'carto-light'". The new comment says OSM "is already the unconditional provider for the home mini-map, node-reach, packet-path and area maps". On master, node-reach, packet-path and area-nodes already went through _applyTilesToNodeMap, which used the configured provider (CARTO by default). OSM was only their unreachable else fallback. Only home.js and area-map.html hard-code OSM. The "no new third party" conclusion still roughly holds because of those two, but the stated reason is wrong. [A] git show origin/master:public/packet-path-map.js L483-487, node-reach-map.js L48-54
3 Nit PR description The body still describes round 1: "11/11 pass", "43 assertions", and no mention of the analytics.js change, _getOsmAttribution() or the 15-step E2E. Worth refreshing before merge, because reviewers and the merge commit see it first. [A]
4 Info public/nodes.js:133 and others These residuals are unchanged and acknowledged in the round-2 report. They are out of scope and not blocking. (a) _applyTilesToNodeMap hard-codes maxZoom: 18. (b) home.js and area-map.html ignore the configured provider. (c) "Reset all" doesn't clear mc-light-tile-provider. (d) osm.enabled: false no longer hides the two OSM default styles (F6). [A]

Answers to the brief

1. F1 (dark mode): fixed.

  • [A] I grepped every L.tileLayer( under public/ on the merged tree, and none bypasses the provider path where a registry exists:
    • The subpath minimap (analytics.js:2545) now calls window._applyTilesToNodeMap(map). That is the same helper the node-detail inset, node-reach, packet-path, area-nodes and rx-coverage maps use. It applies provider.invertFilter to the map's own tile pane.
    • map.js and live.js resolve through MC_getTileSpec, then MC_applyTileFilter().
    • The two customizer previews use _cv2PreviewTile(). They are light-only by design, as on master.
    • The remaining literals are all OSM, never CARTO. They are the unreachable else fallbacks of the helper call sites, plus home.js, area-map.html and geofilter-builder.html.
    • There is no L.tileLayer(getTileUrl()) left.
  • [T] In a browser on a no-config server, #subpathMap .leaflet-tile-pane computes invert(1) hue-rotate(180deg) brightness(0.9) contrast(1.05) in dark mode and none in light mode. Packet-path, node detail, #/map and the AreaNodesMap behave the same way.
    • Light stub tiles render dark in a dark-mode screenshot of the minimap and light in light mode.
    • With darkDefault: esri-darkgray-labels, every one of those panes computes none, which is correct because Esri is natively dark.

2. F2 (E2E): fixed, and the test can fail.

  • [A] The steps for the packet-path map and Subpaths exist, in both themes:

    • The packet-path step opens #/packets/<hash>?viewPath=1, with the hash discovered at runtime.
    • The Subpaths step clicks route rows until #subpathMap mounts. If none of the first 15 rows mounts it, the step fails loudly.
  • [A] The Areas step now drives AreaNodesMap.open() with requireHit: true and checkFilter, so it is no longer vacuous. Synthetic points are fair here, because the fixture has positionGaps: [] and the tile-attach path is the production one.

  • [T] Results:

    Tree Config Result
    Merged No config 15/0
    Merged EXPECT_TILE_HOST_* with an Esri/OpenTopo config 15/0
    Master No config 0/15 (every step reports CARTO requests)
  • [K] The mutants with one hard-coded CARTO URL are caught (K1, K6 below).

3. F3–F5: fixed.

  • F3 [T]: With no config, the attribution reads Leaflet | © OpenStreetMap contributors on #/map and in the customizer modal, in both themes. With the Esri/OpenTopo config, it reads Tiles © Esri and the OpenTopoMap CC-BY-SA credit.
  • F3 [A]: _getOsmAttribution() mirrors the _getOsmUrl() branch exactly: the same provider && token guard, toLowerCase(), and the same three vendors. An unknown vendor or a missing token falls back to plain OSM for both the URL and the credit. It is re-resolved on every MC_initTileRegistry().
  • F4 [A]/[T]: No "zoom 19" text remains in public/ or config.example.json, and M9 ties the documented value to maxZoom.
  • F5 [A]: _renderTileProviderSelector() falls back to MC_DARK_TILE_DEFAULT / MC_LIGHT_TILE_DEFAULT.
    • The only remaining carto-dark / carto-light / cartocdn occurrences in public/ are the BASE_STYLES keys, _getCartoBase(), and comments (see nit 2).
    • No code falls back to a CARTO id.

4. Browser check, dark and light: 0 CARTO requests.

  • [T] My own Playwright sweep listened on context.on('request'), so it saw every resource type, not only images. Third-party images were stubbed.

  • It covered:

    • every nav and #/tools/* link (discovered at runtime);
    • all 20 Analytics tabs;
    • node detail, the Subpaths minimap, the packet-path map and AreaNodesMap;
    • the customizer geo-filter inset and modal;
    • geofilter-builder.html, area-map.html and geofilter-docs.html.

    That is 51 page loads per theme.

    Server Theme CARTO requests Tile hosts
    Head, no config dark 0 tile.openstreetmap.org 259
    Head, no config light 0 tile.openstreetmap.org 259
    Head, Esri/OpenTopo config dark 0 server.arcgisonline.com 156, tile.opentopomap.org 30 (customizer previews, light by design), tile.openstreetmap.org 73
    Head, Esri/OpenTopo config light 0 tile.opentopomap.org 186, tile.openstreetmap.org 73
    Master, no config dark 234 —
    Master, no config light 234 —
  • [T] I attributed the 73 OSM requests per page. All of them come from the two registry-less standalone pages, geofilter-builder.html and area-map.html. No in-app map ignores the configured provider.

Standing checks

  • Acceptance criteria. Both items in the issue have a test that is red before and green after.
  • Behaviour beyond the purpose. None, beyond what was documented in round 1.
    • getActiveTileProvider() now also resolves in light mode. Its only consumer is _applyTilesToNodeMap (nodes.js:131), and light styles carry no invertFilter or refUrl, so the rendering is unchanged. [A]
    • The subpath minimap still has attributionControl: false, as on master. [A]
  • cmd/server read-only, no new map[string]interface{}. git diff origin/master <merged tree> -- cmd/ internal/ is empty, and 0 added map[string]interface{}. [T]
  • Colours. There are no added hex or rgb values in public/. The one new CSS rule is a filter. scripts/check-css-vars.js reports 0 undefined. [T]
  • XSS gate. scripts/check-xss-sinks.sh --diff origin/master exits 0, run in a detached scratch worktree at head. [T]
  • Fork guards. 9 in deploy.yml and 1 in release-fast-path.yml, the same as master. [T]
  • Closing keywords. None in the PR body or the three commits, and closingIssuesReferences is empty. [T]
  • Commit author. dborup <kontakt@meshview.dk> as author and committer on aac9445a, 4b3b5334 and 26beef7e. [T]
  • CI, run 37641463819 attempt 1 on 26beef7e. [T]

Tests and mutants

git merge-tree --write-tree origin/master 26beef7e… (origin/master = 987c220d) gives tree 5f3bc24f with no conflicts. All local runs used a git archive of that tree in scratch.

Suite Result
go test ./... in cmd/server ok (923 s) [T]
go test ./... in cmd/ingestor ok (1326 s), run after the server suite finished [T]
sh test-all.sh 232/233 files green. The 1 failure is test-channels-client-state-152.js N1, a timing assertion that failed while the Go suite ran in parallel. It passed 3/3 in isolation on the merged tree and 1/1 on master, and channels.js is untouched. This is the same class the round-1 review saw. [T]
node test-frontend-helpers.js 709 passed, 0 failed [T]
#332 unit / Kpa-clawbot#1420 / Kpa-clawbot#1470 / Kpa-clawbot#1614 57/0 · 39/0 · 6/0 · 3/0 [T]
#332 E2E, merged tree 15/0 with no config, and 15/0 with the Esri/OpenTopo config and EXPECT_TILE_HOST_* [T]
#332 E2E, master 0/15 [T]
Affected E2Es against a local server on e2e-fixture.db (freshen, seed SQL, corescope-migrate, seeds 2073 and 199) map-modal-fluid 10/0, issue-123-map-lifecycle 15/0, map-nodes-pagination 3/0, issue-1236-map-mobile 3/3, issue-1329-map-controls-accordion 5/5, live-layout-1178-1179 10/0, live-fullscreen-1572 9/0, pr-1490-live-map-gpu-animations 14/0, node-reach OK, issue-1374-route-map-a11y 20/0, issue-205-analytics-subtab-deeplinks 15/0, customize-theme 8/8, a11y-1705-subpath-hop-prefix PASS, issue-1705-subpath-contrast PASS. node-reach-coverage and rx-coverage-mobile-nav SKIP (clientRxCoverage is off in the fixture). [T]

My own mutants. Each was applied to a copy of the merged tree, served by its own local server, and run against the #332 unit suite and the #332 E2E (plus Kpa-clawbot#1420/Kpa-clawbot#1470/Kpa-clawbot#1614):

Mutant Unit suites #332 E2E
K1: one hard-coded CARTO dark_all URL returned by live.js _liveResolveTile(dark) [K] M5 ×4 + M6 static guard [K] [dark] live: 18 request(s) to basemaps.cartocdn.com
K6: one hard-coded CARTO light_all URL in place of the helper call in the subpath minimap [K] M6 + M7 ×4 [K] both Subpaths steps, 3 request(s) to basemaps.cartocdn.com
K5: map.js dark path takes MC_TILE_PROVIDERS['carto-dark'].url() (no literal) survives (57/0) [K] [dark] map: 15 request(s) to basemaps.cartocdn.com
K3: _applyTilesToNodeMap stops applying invertFilter [K] M7 + Kpa-clawbot#1470 [K] 4 dark steps (node detail, Subpaths, packet-path, Areas) via checkFilter
K2: getTileSpec('light') ignores the configured light provider [K] M2/M3 (several) survives with no config (default = configured); [K] with the Esri/OpenTopo config, 8 steps
K4: delete the builder's invert rule survives survives → nit 1

Not verified

  • Real tile pixels. Every third-party image was stubbed with a light 256 px PNG. The dark-mode verdict rests on the computed filter, the requested URL and screenshots of the stub tiles.
  • Token-based OSM vendors (MapTiler, Thunderforest, Mapbox), Stamen with a token, and a CARTO enterprise domain in a browser. These are covered only by the unit suite.
  • The home mini-map (no claimed node in the fixture), Firefox/Safari, mobile viewports, the Docker image, and staging or prod (out of scope).
  • The full test-e2e-playwright.js locally. I relied on the green CI job for it.
  • I seeded 2073 and 199 as the brief asked. CI also seeds 245, which none of the tile tests depend on.

Head was checked with git ls-remote before and after the review: 26beef7e45fc7614db34487324aae14823f48a3d both times.

@dborup
dborup marked this pull request as ready for review October 7, 2026 17:47
@dborup
dborup merged commit 650218d into master Oct 7, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants