Explorer radar loop: no basemap, and frames composite cumulatively instead of replacing #182
Labels
No labels
area:ai
area:ci-cd
area:notifications
area:observability
area:public-pages
backlog
bug
duplicate
enhancement
help wanted
invalid
question
type:decision
type:feature
type:infra
type:maintenance
type:security
v1.0.1
v1.1.0
v1.2.0
v1.3.0
v2.0.0
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
rbrooks/WeatherBot#182
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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) requeststransparent=truefrom 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
ImageOverlayon 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 disposal0("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_gifcode path: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=Trueis a no-op, and mislabels content typeget_radar_image(..., animated=True)— what every notifier calls — caches the result withcontent_type="image/gif"(radar.py:88), but_fetch_radar(radar.py:99-105) ignores theanimatedparameter 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=2anyway).Verified that Esri's ArcGIS export endpoints accept the identical
bbox/bboxSR/imageSR/sizeparameters 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 fromCanvas/World_Dark_Gray_Base,Canvas/World_Light_Gray_Base,World_Topo_Map, andReference/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
World_Boundaries_and_Placesreference overlay — reflectivity reads best against it and it matches the app's dark themelat/lonand theBBOX_PADDINGconstant, 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.Work
event_radar_loop._assemble) and the still-frame serve path (/media/alert-radar/...).disposal=2inassemble_gifregardless, so a future transparent-frame caller can't reintroduce the smear.get_radar_image.Blocks the v2.0.0 final release — this is a visible defect in the headline feature of the release.
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.mdis untouched.What landed
New
app/services/basemap.py— fetches the two Esri layers, caches each indefinitely per quantized(lat, lon)underbasemaps/{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 savesdisposal=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— gainedwith_basemap: bool = False, and_cache_keynow incorporates the variant (KLSX_gif_basemap.gif) so composited and bare images can never collide in the cache.with_basemap=Truepassed at the 6 notifier call sites only (discord_bot×2,matrix_bot×2,pushover,signal);sms/email/webhookdon't call it, andwebexuses Iowa State's warning-polygon image instead.api/alerts.pyandapi/locations.pydeliberately left at the defaultFalse.event_radar_loop—_assemble/get_event_radar_looptakelat/lonthreaded from the caller (no DB query inside the assembler); NULL coordinates fall through to bare assembly.api/media.py— both still routes join toLocationand composite.Public-page path checked and in scope:
public.py:670links/media/alert-radar/{id}.pngandpublic_alert.html:140renders 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_layersreturns(base, reference)) so the radar composites between them. ReturningNoneunless 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_radarto pin the order, since no existing assertion (opacity, alignment, caching) could catch an ordering mistake.Verification
disposal=2and pass with it, using the exact blob-offset scenario from the original repro — not a smoke test.ruff check app/ tests/— clean.Note its
.venvhad a stale Pillow 11.0.0 that tripped an unrelated GIF palette bug; it is now pinned to 12.3.0 to matchrequirements.txt. Worth knowing if that box is used for other test runs.Still open
animated=Trueno-op / GIF-mislabelling bug (item 3 above) is not fixed — still a separate decision.v2.0.0-rc1, whether to cut an rc2 and soak it or go straight to final is an open release call.Shipped in v2.0.0-rc2, soaking on dev
Merged via #183 (
50921a4), taggedv2.0.0-rc2, imagegit.rhoving.com/rbrooks/weatherbot:2.0.0-rc2published 2026-08-19 20:48 UTC.Deployed to
docker-test(iac-repo2b6eb3dbumps 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-failurechange from iac-repo #277 thatdocker-testhad 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.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-rc4on dev, against an active MCS over north Alabama.Setup. A temporary dev location at Huntsville, AL (34.630, -86.428 — WFO
HUN, radarKHTX) 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_gifatEVENT_LOOP_FRAME_DURATION_MS).Result — all three properties confirmed on live data:
mode=RGB, 49.8 KB bare → 244 KB composited. State lines, rivers and city labels (Huntsville, Chattanooga, Decatur, Nashville, Birmingham) all legible.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_loopinside 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 surroundingevent_radar_loopplumbing (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=Truework tracked on #184 is unaffected by this and still needs a radar-capable notification channel on dev.