Fix stale weather radar: per-refresh cache-buster + invalidate-on-show (#111)#120
Merged
Conversation
Per-refresh cache-buster on the radar tile URL to defeat CDN/HTTP caching (root cause of stale NEXRAD); opt-in via setRefreshInterval, with ADR-0004 addendum and a refresh-distinctness regression test.
… radar tiles (#111) The NEXRAD radar overlay showed stale precipitation: invalidateCache() clears only the LOCAL disk cache each refresh, but the re-fetch hits the same cache-buster-less z/x/y URL, which a CDN/proxy between CAMP and IEM can answer from its own edge cache with an older tile. Disk invalidation alone (#99) does not defeat that intermediary cache. Add an opt-in per-refresh URL cache-buster to CachedTileLoader: a wall-clock- seeded token (cross-session freshness) advanced with a strict-monotonic guard each refresh, appended as ?t=<token> to the network URL only (disk path stays z/x/y.png). Enabled from MapTiles::setRefreshInterval (refreshing layers only, so static OSM/WMTS URLs are unchanged) and bumped in onRefreshTimer before the re-fetch. Defensive fix: correct regardless of whether CDN caching is the sole mechanism. IEM endpoint verified to tolerate an unknown ?t= param (HTTP 200, identical bytes). Tests: withCacheBust ?/&-join + distinctness, strict per-cycle token advance on refresh, and static-layer opt-out (token stays 0). All 8 refresh tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…18 (#111) ADR-0004 recorded IEM NEXRAD N0Q as the radar tile provider and asserted as a Consequence that 'refresh yields a genuinely fresh frame' — a claim #111 disproves (it ignored CDN/proxy caching between client and origin). The provider choice was always a tile-path stopgap (NOAA serves radar only via WMS, which camp2 can't yet consume), not settled architecture. Per workspace ADR-0012, correcting/softening an accepted ADR's Consequences requires a new/superseding ADR rather than an in-place edit. Operator decision (2026-06-22): retire the radar-provider decision from the ADR set entirely and track the intended end state (NOAA nowCOAST via WMS) as camp#118, which carries the IEM-stopgap rationale forward so discoverability is preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…blank-on-reject (#111 review) Pre-push review suggestions on the #111 cache-buster: - enableCacheBusting() now no-ops when already enabled, so a second setRefreshInterval(>0) cannot re-seed the token BELOW a value already advanced by bumpCacheBust() (its +1 guard can sit above wall-clock). Unreachable today (one setRefreshInterval per layer) but makes the invariant robust. - Document that a tile server rejecting the unknown ?t= param would degrade to a blank tile (CachedFileLoader drops a failed reply) — safe (never stale). IEM tolerates ?t= today (verified HTTP 200, identical bytes). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…int is never stale (#111) The cache-buster fixes the periodic refresh, but the 5-min timer is the only thing that invalidates the disk cache — so enabling the radar layer shortly after launch served a PREVIOUS session's on-disk tiles (e.g. yesterday's radar) until the next refresh. Operator requirement: never show yesterday's frame, not even briefly. Override MapTiles::itemChange: on the ItemVisibleHasChanged→true transition, a refreshing layer (gated on cacheBustToken() != 0, i.e. radar only) drops its disk cache and bumps the cache-buster, so the load() from the ensuing paint fetches a fresh, CDN-distinct frame. The brief gap shows blank, never a stale frame. Static OSM/WMTS basemaps keep their cache on show (token stays 0). Tests: becoming visible strictly advances the token on a refreshing layer; a static layer's visibility toggle does not bust. All 10 refresh tests pass. The broader mid-session safeguard (a displayed frame that AGES when fetches stop) is filed separately as camp#119. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…Round-2 must-fix) Round-2 review caught that itemChange invalidate-on-show bumped the cache-buster and dropped the disk cache but never rebuilt the tile set — so the already-built Tile objects (a prior show, or the ctor-seeded zoom-0 tile loaded at token 0) were not re-fetched, re-showing the stale frame on off→on until the next 5-min refresh. The token bump only affects loads issued LATER; without setLayout() no new load is issued. Extract a shared refreshTiles() (bump + invalidate + setLayout rebuild) called by both onRefreshTimer() and itemChange()-on-show, so the two paths cannot drift. The on-show path now delivers a genuinely fresh frame. Tests: BecomingVisibleRebuildsTilesNotJustToken counts destruction of the pre-show Tile objects (robust to allocator pointer reuse) — it fails against the token-only bug and passes with the rebuild. Static-layer test now also asserts no tiles are destroyed on show (invalidate-on-show branch gated on a non-zero token). Tightened that test's comment to match its assertion. Noted the rapid-toggle re-download tradeoff (freshness guard deferred to camp#119). 11 refresh tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Remove a latent risk flagged in review: tile_loader_ was an uninitialized raw pointer read by the itemChange guard. Safe today (the ctor assigns it before any paint/setLayout, and itemChange dispatches virtually only on a live object), but default-initializing to nullptr removes the trap if construction order ever changes. Matches the sibling pointers (refresh_timer_, wmts_capabilites_). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #111
Problem
The NEXRAD weather-radar overlay (#99) showed stale precipitation — it never advanced to the latest IEM mosaic, and enabling it after launch could re-show a previous session's frame.
Two distinct staleness paths, both fixed here:
invalidateCache()clears only the local disk cache each 5-min cycle; the re-fetch then hits the same cache-buster-less{z}/{x}/{y}URL, which a CDN/proxy between CAMP and IEM can answer from its own edge cache with an older tile. Disk invalidation alone (Add auto-refreshing weather radar tile overlay (stacked layer on the map system) #99) does not defeat that intermediary cache.Fix
CachedTileLoader): an opt-in, wall-clock-seeded token (cross-session freshness) advanced with a strict-monotonic guard each refresh, appended as?t=<token>to the network URL only (disk path staysz/x/y.png). Enabled fromsetRefreshInterval(refreshing layers only → static OSM/WMTS URLs untouched). Defensive: correct whether or not CDN caching is the sole mechanism. IEM verified to tolerate the param (HTTP 200, identical bytes).MapTiles::itemChange): when a refreshing layer becomes visible, run the sharedrefreshTiles()(bump + invalidate +setLayoutrebuild) so the first paint re-fetches a fresh frame — brief blank, never a stale one. A shared helper keeps the timer and on-show paths from drifting.ADR-0004 removed
ADR-0004 recorded IEM as the radar provider and asserted "refresh yields a genuinely fresh frame" — a claim this bug disproves, and the provider choice was always a tile-path stopgap (NOAA serves radar only via WMS, which camp2 can't yet consume). Per workspace ADR-0012, correcting an accepted ADR's Consequences requires a new/superseding ADR rather than an in-place edit; by operator decision the radar-provider decision is retired from the ADR set and tracked forward as #118 (which carries the IEM-stopgap rationale).
Follow-ups filed
Tests
test/test_map_tiles_refresh.cpp(11 tests, all pass):withCacheBustjoin/distinctness, strict per-cycle token advance, static-layer opt-out, becoming-visible rebuilds the tile set (counts pre-show Tile destruction — fails against a token-only fix), and static-layer-no-refresh-on-show.Authored-By:
Claude Code AgentModel:
Claude Opus 4.8