Explorer radar loop: no basemap, and frames composite cumulatively instead of replacing #182

Closed
opened 2026-08-19 18:13:31 +00:00 by claude-bot · 3 comments
Contributor

Reported against v2.0.0-rc1 on dev while reviewing the Explorer (#140). Two defects, one shared root cause, plus a third latent bug found while tracing.

1. Radar frames have no basemap

radar._fetch_noaa_static (app/services/radar.py:121) requests transparent=true from the NOAA ArcGIS MapServer, so every captured PNG is reflectivity on a fully transparent background. Nothing composites geography underneath at any point.

This was invisible until now because the dashboard and public pages render the radar as a Leaflet ImageOverlay on top of an OSM tile layer — the browser supplies the basemap. The stored PNG itself never had one. The Explorer's still strip and loop player show the raw image with no Leaflet under it, so the echoes float on blank page background.

Affects every server-side radar image: the #85 alert-time snapshot, #130 periodic frames, the #140 loop, and all notifier attachments.

2. Loop frames stack instead of replacing

radar.assemble_gif (app/services/radar.py:466-476) converts frames to RGBA and saves the GIF without specifying a disposal method. Pillow defaults to disposal 0 ("do not dispose"), so each frame's transparent pixels leave the previous frame's echoes visible. The loop accumulates every scan on top of every other, which is exactly the reported symptom — progression is unreadable.

Reproduced with a 3-frame synthetic GIF through the real assemble_gif code path:

frame 0: blobs visible at x-offsets [0]
frame 1: blobs visible at x-offsets [0, 20]
frame 2: blobs visible at x-offsets [0, 20, 40]

The SPC outlook GIF uses this same assembler and is unaffected, because IEM's plot-220 maps are fully opaque — which is why this survived review.

3. (Separate, latent) animated=True is a no-op, and mislabels content type

get_radar_image(..., animated=True) — what every notifier calls — caches the result with content_type="image/gif" (radar.py:88), but _fetch_radar (radar.py:99-105) ignores the animated parameter entirely and unconditionally returns a static PNG from _fetch_noaa_static (or a RainViewer PNG tile on fallback). So notifier radar attachments are PNG bytes labelled as GIF, and the "animated radar" channel option has never produced an animation.

Not fixed as part of this issue unless it falls out naturally — filed here so it isn't lost.

Fix

Compositing each frame onto an opaque basemap resolves 1 and 2 together: opaque frames cannot accumulate, so the disposal problem disappears by construction (belt-and-braces: set disposal=2 anyway).

Verified that Esri's ArcGIS export endpoints accept the identical bbox/bboxSR/imageSR/size parameters as the NOAA radar export, so the basemap is pixel-aligned by construction — no tile stitching, no reprojection, one extra HTTP GET. Probed live and confirmed 600x600 PNGs returned from Canvas/World_Dark_Gray_Base, Canvas/World_Light_Gray_Base, World_Topo_Map, and Reference/World_Boundaries_and_Places.

Flattening onto an opaque background also shrinks the GIF substantially (a synthetic 3-frame test went 2612 → 344 bytes) because an opaque palette needs no transparency index.

Decisions taken

Question Decision
Basemap style Dark Gray Canvas + World_Boundaries_and_Places reference overlay — reflectivity reads best against it and it matches the app's dark theme
Where to composite At assembly/serve time, not capture time — the bbox is fully derived from the location's lat/lon and the BBOX_PADDING constant, so a basemap fetched today aligns with a frame captured months ago. This retroactively fixes all 13 months of already-captured transparent history, which capture-time compositing could not.
Scope Explorer (loop + still strip) + notifier attachments. Dashboard/public radar endpoints are deliberately excluded — they already render under a Leaflet tile layer and a baked-in basemap would double up.

Work

  • Basemap fetch + composite helper, with the basemap cached per bbox through the existing media storage abstraction (#131) — it never changes, so fetch once per location.
  • Composite in the loop assembly path (event_radar_loop._assemble) and the still-frame serve path (/media/alert-radar/...).
  • Set disposal=2 in assemble_gif regardless, so a future transparent-frame caller can't reintroduce the smear.
  • Composite for notifier attachments via get_radar_image.
  • Settings flag + basemap service URLs in config; fail-soft to the current bare-radar behaviour if the basemap fetch fails (a missing basemap must never cost us the radar image).
  • Tests: cumulative-overlay regression (assert frame N does not contain frame N-1's echoes), composite alignment, basemap-fetch-failure fallback.

Blocks the v2.0.0 final release — this is a visible defect in the headline feature of the release.

Reported against v2.0.0-rc1 on dev while reviewing the Explorer (#140). Two defects, one shared root cause, plus a third latent bug found while tracing. ## 1. Radar frames have no basemap `radar._fetch_noaa_static` (`app/services/radar.py:121`) requests `transparent=true` from the NOAA ArcGIS MapServer, so every captured PNG is reflectivity on a **fully transparent background**. Nothing composites geography underneath at any point. This was invisible until now because the dashboard and public pages render the radar as a Leaflet `ImageOverlay` on top of an OSM tile layer — the browser supplies the basemap. The stored PNG itself never had one. The Explorer's still strip and loop player show the raw image with no Leaflet under it, so the echoes float on blank page background. Affects **every server-side radar image**: the #85 alert-time snapshot, #130 periodic frames, the #140 loop, and all notifier attachments. ## 2. Loop frames stack instead of replacing `radar.assemble_gif` (`app/services/radar.py:466-476`) converts frames to RGBA and saves the GIF without specifying a disposal method. Pillow defaults to disposal `0` ("do not dispose"), so each frame's transparent pixels leave the **previous frame's echoes visible**. The loop accumulates every scan on top of every other, which is exactly the reported symptom — progression is unreadable. Reproduced with a 3-frame synthetic GIF through the real `assemble_gif` code path: ``` frame 0: blobs visible at x-offsets [0] frame 1: blobs visible at x-offsets [0, 20] frame 2: blobs visible at x-offsets [0, 20, 40] ``` The SPC outlook GIF uses this same assembler and is unaffected, because IEM's plot-220 maps are fully opaque — which is why this survived review. ## 3. (Separate, latent) `animated=True` is a no-op, and mislabels content type `get_radar_image(..., animated=True)` — what **every notifier** calls — caches the result with `content_type="image/gif"` (`radar.py:88`), but `_fetch_radar` (`radar.py:99-105`) ignores the `animated` parameter entirely and unconditionally returns a static PNG from `_fetch_noaa_static` (or a RainViewer PNG tile on fallback). So notifier radar attachments are **PNG bytes labelled as GIF**, and the "animated radar" channel option has never produced an animation. Not fixed as part of this issue unless it falls out naturally — filed here so it isn't lost. ## Fix Compositing each frame onto an **opaque** basemap resolves 1 and 2 together: opaque frames cannot accumulate, so the disposal problem disappears by construction (belt-and-braces: set `disposal=2` anyway). Verified that Esri's ArcGIS export endpoints accept the **identical** `bbox`/`bboxSR`/`imageSR`/`size` parameters as the NOAA radar export, so the basemap is pixel-aligned by construction — no tile stitching, no reprojection, one extra HTTP GET. Probed live and confirmed 600x600 PNGs returned from `Canvas/World_Dark_Gray_Base`, `Canvas/World_Light_Gray_Base`, `World_Topo_Map`, and `Reference/World_Boundaries_and_Places`. Flattening onto an opaque background also shrinks the GIF substantially (a synthetic 3-frame test went 2612 → 344 bytes) because an opaque palette needs no transparency index. ## Decisions taken | Question | Decision | |---|---| | Basemap style | **Dark Gray Canvas** + `World_Boundaries_and_Places` reference overlay — reflectivity reads best against it and it matches the app's dark theme | | Where to composite | **At assembly/serve time, not capture time** — the bbox is fully derived from the location's `lat`/`lon` and the `BBOX_PADDING` constant, so a basemap fetched today aligns with a frame captured months ago. This retroactively fixes all 13 months of already-captured transparent history, which capture-time compositing could not. | | Scope | **Explorer (loop + still strip) + notifier attachments.** Dashboard/public radar endpoints are deliberately excluded — they already render under a Leaflet tile layer and a baked-in basemap would double up. | ## Work - [ ] Basemap fetch + composite helper, with the basemap cached per bbox through the existing media storage abstraction (#131) — it never changes, so fetch once per location. - [ ] Composite in the loop assembly path (`event_radar_loop._assemble`) and the still-frame serve path (`/media/alert-radar/...`). - [ ] Set `disposal=2` in `assemble_gif` regardless, so a future transparent-frame caller can't reintroduce the smear. - [ ] Composite for notifier attachments via `get_radar_image`. - [ ] Settings flag + basemap service URLs in config; fail-soft to the current bare-radar behaviour if the basemap fetch fails (a missing basemap must never cost us the radar image). - [ ] Tests: cumulative-overlay regression (assert frame N does not contain frame N-1's echoes), composite alignment, basemap-fetch-failure fallback. Blocks the v2.0.0 final release — this is a visible defect in the headline feature of the release.
Author
Contributor

Implemented on fix/radar-basemap-and-loop-disposal (not yet committed)

All six work items done. Changes are in the working tree pending review — nothing committed or pushed yet, and CHANGELOG.md is untouched.

What landed

New app/services/basemap.py — fetches the two Esri layers, caches each indefinitely per quantized (lat, lon) under basemaps/{lat}_{lon}_{layer}.png, and composites. Fail-soft is unconditional: the disable setting, a fetch failure, a malformed PNG, or any Pillow error all return the original radar bytes unchanged rather than raising. Nothing on the notification path can lose its radar image to a basemap problem.

radar.assemble_gif — now saves disposal=2, with the reasoning in the docstring. Belt-and-braces: opaque post-composite frames already fix the smear structurally, but a future caller passing transparent frames can't silently reintroduce it.

radar.get_radar_image — gained with_basemap: bool = False, and _cache_key now incorporates the variant (KLSX_gif_basemap.gif) so composited and bare images can never collide in the cache. with_basemap=True passed at the 6 notifier call sites only (discord_bot ×2, matrix_bot ×2, pushover, signal); sms/email/webhook don't call it, and webex uses Iowa State's warning-polygon image instead. api/alerts.py and api/locations.py deliberately left at the default False.

event_radar_loop_assemble/get_event_radar_loop take lat/lon threaded from the caller (no DB query inside the assembler); NULL coordinates fall through to bare assembly. api/media.py — both still routes join to Location and composite.

Public-page path checked and in scope: public.py:670 links /media/alert-radar/{id}.png and public_alert.html:140 renders it as a plain <img>, not a Leaflet overlay — so compositing there is correct with no double-up risk.

Defect found and fixed in review

The first implementation cached the base and reference layers pre-flattened into one basemap, then composited the radar over both — which buried the boundaries and place labels underneath the echoes, contradicting this issue's stated base → radar → reference order.

It matters precisely where it hurts most: the labels a reader needs are the ones under the storm. Measured against a real St. Louis reflectivity frame, the two orderings differ across ~1.3% of pixels, and those pixels are exactly the state line, "Cape Girardeau" and "Missouri" — all completely swallowed in the wrong order, all legible in the right one.

Fixed by caching the two layers separately (get_basemap_layers returns (base, reference)) so the radar composites between them. Returning None unless both layers are available is deliberate: half a basemap — echoes with no labels, or labels with no land — is worse than the plain radar frame.

Added test_composite_on_basemap_draws_reference_labels_over_the_radar to pin the order, since no existing assertion (opacity, alignment, caching) could catch an ordering mistake.

Verification

  • Cumulative-overlay regression test confirmed to actually fail without disposal=2 and pass with it, using the exact blob-offset scenario from the original repro — not a smoke test.
  • ruff check app/ tests/ — clean.
  • Full suite on the dev server: 994 passed, 13 deselected (postgres tier), 0 failures. Dev server working tree restored to clean afterward.

Note its .venv had a stale Pillow 11.0.0 that tripped an unrelated GIF palette bug; it is now pinned to 12.3.0 to match requirements.txt. Worth knowing if that box is used for other test runs.

Still open

  • The animated=True no-op / GIF-mislabelling bug (item 3 above) is not fixed — still a separate decision.
  • Not committed; no changelog entry yet.
  • Since this lands after v2.0.0-rc1, whether to cut an rc2 and soak it or go straight to final is an open release call.
## Implemented on `fix/radar-basemap-and-loop-disposal` (not yet committed) All six work items done. Changes are in the working tree pending review — nothing committed or pushed yet, and `CHANGELOG.md` is untouched. ### What landed **New `app/services/basemap.py`** — fetches the two Esri layers, caches each indefinitely per quantized `(lat, lon)` under `basemaps/{lat}_{lon}_{layer}.png`, and composites. Fail-soft is unconditional: the disable setting, a fetch failure, a malformed PNG, or any Pillow error all return the original radar bytes unchanged rather than raising. Nothing on the notification path can lose its radar image to a basemap problem. **`radar.assemble_gif`** — now saves `disposal=2`, with the reasoning in the docstring. Belt-and-braces: opaque post-composite frames already fix the smear structurally, but a future caller passing transparent frames can't silently reintroduce it. **`radar.get_radar_image`** — gained `with_basemap: bool = False`, and `_cache_key` now incorporates the variant (`KLSX_gif_basemap.gif`) so composited and bare images can never collide in the cache. `with_basemap=True` passed at the 6 notifier call sites only (`discord_bot` ×2, `matrix_bot` ×2, `pushover`, `signal`); `sms`/`email`/`webhook` don't call it, and `webex` uses Iowa State's warning-polygon image instead. `api/alerts.py` and `api/locations.py` deliberately left at the default `False`. **`event_radar_loop`** — `_assemble`/`get_event_radar_loop` take `lat`/`lon` threaded from the caller (no DB query inside the assembler); NULL coordinates fall through to bare assembly. **`api/media.py`** — both still routes join to `Location` and composite. **Public-page path checked and in scope:** `public.py:670` links `/media/alert-radar/{id}.png` and `public_alert.html:140` renders it as a plain `<img>`, not a Leaflet overlay — so compositing there is correct with no double-up risk. ### Defect found and fixed in review The first implementation cached the base and reference layers **pre-flattened into one basemap**, then composited the radar over both — which buried the boundaries and place labels *underneath* the echoes, contradicting this issue's stated base → radar → reference order. It matters precisely where it hurts most: the labels a reader needs are the ones under the storm. Measured against a real St. Louis reflectivity frame, the two orderings differ across ~1.3% of pixels, and those pixels are exactly the state line, "Cape Girardeau" and "Missouri" — all completely swallowed in the wrong order, all legible in the right one. Fixed by caching the two layers **separately** (`get_basemap_layers` returns `(base, reference)`) so the radar composites between them. Returning `None` unless *both* layers are available is deliberate: half a basemap — echoes with no labels, or labels with no land — is worse than the plain radar frame. Added `test_composite_on_basemap_draws_reference_labels_over_the_radar` to pin the order, since no existing assertion (opacity, alignment, caching) could catch an ordering mistake. ### Verification - **Cumulative-overlay regression test** confirmed to actually fail without `disposal=2` and pass with it, using the exact blob-offset scenario from the original repro — not a smoke test. - `ruff check app/ tests/` — clean. - Full suite on the dev server: **994 passed, 13 deselected** (postgres tier), 0 failures. Dev server working tree restored to clean afterward. Note its `.venv` had a stale Pillow 11.0.0 that tripped an unrelated GIF palette bug; it is now pinned to 12.3.0 to match `requirements.txt`. Worth knowing if that box is used for other test runs. ### Still open - The `animated=True` no-op / GIF-mislabelling bug (item 3 above) is **not** fixed — still a separate decision. - Not committed; no changelog entry yet. - Since this lands after `v2.0.0-rc1`, whether to cut an rc2 and soak it or go straight to final is an open release call.
Author
Contributor

Shipped in v2.0.0-rc2, soaking on dev

Merged via #183 (50921a4), tagged v2.0.0-rc2, image git.rhoving.com/rbrooks/weatherbot:2.0.0-rc2 published 2026-08-19 20:48 UTC.

Deployed to docker-test (iac-repo 2b6eb3d bumps the host_vars pin). Play recap: ok=24 changed=4 failed=0 unreachable=0, health gate passed. Container running and healthy; startup clean with no errors, no new migrations to apply.

The play also converged a pending systemd Restart=on-failure change from iac-repo #277 that docker-test had not picked up since that landed — unrelated to this fix, but it rode along, so noting it here.

Items 1 and 2 of this issue are now resolved and in an RC. Item 3 (animated=True) remains open and is tracked separately — see the linked follow-up issue. Leaving this issue open until rc2 has been seen rendering a real event on dev, since that is the actual verification that the loop now reads as motion rather than a smear.

## Shipped in v2.0.0-rc2, soaking on dev Merged via #183 (`50921a4`), tagged `v2.0.0-rc2`, image `git.rhoving.com/rbrooks/weatherbot:2.0.0-rc2` published 2026-08-19 20:48 UTC. Deployed to `docker-test` (iac-repo `2b6eb3d` bumps the host_vars pin). Play recap: `ok=24 changed=4 failed=0 unreachable=0`, health gate passed. Container running and healthy; startup clean with no errors, no new migrations to apply. The play also converged a pending systemd `Restart=on-failure` change from iac-repo #277 that `docker-test` had not picked up since that landed — unrelated to this fix, but it rode along, so noting it here. Items 1 and 2 of this issue are now resolved and in an RC. **Item 3 (`animated=True`) remains open** and is tracked separately — see the linked follow-up issue. Leaving this issue open until rc2 has been seen rendering a real event on dev, since that is the actual verification that the loop now reads as motion rather than a smear.
Author
Contributor

Verified against real weather — both fixes confirmed

This issue was held open pending live verification rather than synthetic fixtures. That is now done, on v2.0.0-rc4 on dev, against an active MCS over north Alabama.

Setup. A temporary dev location at Huntsville, AL (34.630, -86.428 — WFO HUN, radar KHTX) placed under a sustained system producing repeated severe and flash-flood warnings for 2+ hours. It captured a Flash Flood Warning and began accumulating radar frames on the normal 5-minute cycle. Frames were then assembled through the real code path (basemap.composite_on_basemapradar.assemble_gif at EVENT_LOOP_FRAME_DURATION_MS).

Result — all three properties confirmed on live data:

Property Evidence
Basemap present Composited frames are mode=RGB, 49.8 KB bare → 244 KB composited. State lines, rivers and city labels (Huntsville, Chattanooga, Decatur, Nashville, Birmingham) all legible.
Reference layer over the radar Place labels and the TN/AL state line remain readable across active echoes rather than being buried — the layer-order fix holds on real reflectivity, not just the constructed test.
No cumulative smear Each frame shows only its own scan. Frame 2 does not carry frame 1's echoes. This is the defect this issue was filed for.
Genuine motion 15.94% of bytes differ between consecutive frames; the MCS core visibly translates east and the northern cell develops.

Worth stating plainly: the earlier soak attempt produced zero frames, which turned out to be a regression rc3 introduced (#184, fixed in rc4) rather than anything wrong with this issue's fix. This verification is on rc4 and is the first time the loop has been exercised end to end on real weather.

Caveat on scope

The frames were assembled off-host through the same functions the service uses, because invoking get_event_radar_loop inside the container required database credentials I deliberately did not read. So what is verified is the compositing and GIF assembly — the actual subject of this issue. The surrounding event_radar_loop plumbing (window derivation, caching, staleness rebuild) remains covered only by its unit tests and was not exercised here.

I consider items 1 and 2 of this issue closed. The remaining animated=True work tracked on #184 is unaffected by this and still needs a radar-capable notification channel on dev.

## Verified against real weather — both fixes confirmed This issue was held open pending live verification rather than synthetic fixtures. That is now done, on `v2.0.0-rc4` on dev, against an active MCS over north Alabama. **Setup.** A temporary dev location at Huntsville, AL (34.630, -86.428 — WFO `HUN`, radar `KHTX`) placed under a sustained system producing repeated severe and flash-flood warnings for 2+ hours. It captured a Flash Flood Warning and began accumulating radar frames on the normal 5-minute cycle. Frames were then assembled through the **real** code path (`basemap.composite_on_basemap` → `radar.assemble_gif` at `EVENT_LOOP_FRAME_DURATION_MS`). **Result — all three properties confirmed on live data:** | Property | Evidence | |---|---| | Basemap present | Composited frames are `mode=RGB`, 49.8 KB bare → 244 KB composited. State lines, rivers and city labels (Huntsville, Chattanooga, Decatur, Nashville, Birmingham) all legible. | | Reference layer **over** the radar | Place labels and the TN/AL state line remain readable across active echoes rather than being buried — the layer-order fix holds on real reflectivity, not just the constructed test. | | **No cumulative smear** | Each frame shows only its own scan. Frame 2 does not carry frame 1's echoes. This is the defect this issue was filed for. | | Genuine motion | 15.94% of bytes differ between consecutive frames; the MCS core visibly translates east and the northern cell develops. | Worth stating plainly: the earlier soak attempt produced **zero** frames, which turned out to be a regression rc3 introduced (#184, fixed in rc4) rather than anything wrong with this issue's fix. This verification is on rc4 and is the first time the loop has been exercised end to end on real weather. ### Caveat on scope The frames were assembled off-host through the same functions the service uses, because invoking `get_event_radar_loop` inside the container required database credentials I deliberately did not read. So what is verified is the **compositing and GIF assembly** — the actual subject of this issue. The surrounding `event_radar_loop` plumbing (window derivation, caching, staleness rebuild) remains covered only by its unit tests and was not exercised here. I consider items 1 and 2 of this issue closed. The remaining `animated=True` work tracked on #184 is unaffected by this and still needs a radar-capable notification channel on dev.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rbrooks/WeatherBot#182
No description provided.