Make notifier radar actually animate, and split the flag that never meant what it said (#184) #185

Merged

claude-bot merged 1 commit from fix/notifier-radar-animation into main

2026-08-21 03:36:09 +00:00

claude-bot commented

2026-08-19 21:36:07 +00:00

Contributor

Fixes #184. Targeted at v2.0.0-rc3. Based on main at the rc2 commit.

Three bugs, one code path

animated=True was a no-op — _fetch_radar accepted the flag and never read it, always returning a static PNG. The "animated radar" option has never animated.
Mislabelled as GIF everywhere — cached as image/gif, attached to Discord as radar.gif, base64'd into Signal as data:image/gif. PNG bytes wearing a GIF label.
radar_animated gated attachment, not animation — turning "animated radar" off silently removed the radar image entirely.

Why it needed more than a content-type fix

NOAA cannot animate. Its MapServer exposes no timeInfo and its capabilities are Map,Query,Data — it serves "now" only, with no historical frames to sweep. RainViewer can (~13 past frames at 10-min cadence) and was already a dependency — but used as a single fixed tile at z6, which is coarse and badly framed: a location sits wherever it falls in its tile, and St. Louis lands at fraction 0.93 across its own, hard against the edge.

Source	Resolution	Framing
NOAA still (before)	0.72 km/px	centred
RainViewer 1 tile @ z6/256 (before)	1.91 km/px	wherever it lands
RainViewer 3x3 stitched @ z7/512 (now)	0.48 km/px	centred

3x3, not 2x2: a location can sit near a tile corner, and only a 3x3 grid guarantees margin for a centred crop in every direction. The same stitcher backs the static fallback, so the off-centre framing defect is fixed for stills too — not just animations.

Measurements that shaped the design

Sequential tile fetching was disqualifying: 5.1s for one frame's 9 tiles → ~40s per animation on the alert dispatch path. Fetched concurrently: 72 requests in 3.0s, producing a 404 KiB GIF whose frames genuinely differ.
CPU cost is ~71ms for compositing + GIF encode. Unlike #128's radial decode, this does not meaningfully block the shared event loop, which is what makes in-process assembly acceptable here.

Format has to travel with the bytes

Because an animated request can silently fall back to a still, no caller can infer the format from the animated argument it passed. _fetch_radar now returns a RadarImage carrying the format actually produced, and every call site labels from it. The cache-hit path sniffs magic bytes rather than trusting the requested format — the disk backend doesn't persist content types, so an animated-key entry written during a RainViewer outage would otherwise be served mislabelled.

Migration 0037

Adds radar_enabled, backfilled from each row's own radar_animated. That is the only value preserving existing behaviour — a constant default would have switched radar on for every channel that had deliberately opted out. server_default exists only so ADD COLUMN succeeds on a populated table, and is dropped after the backfill so the ORM default governs new rows.

Verification

Independently re-run, not taken on trust:

ruff check app/ tests/ — clean
Bulk (SQLite) tier: 1001 passed, 14 deselected
Postgres tier against a throwaway container: 14 passed, including the 0037 backfill test (a radar_animated=False row must land on radar_enabled=False)

Worth knowing

Pillow collapses runs of byte-identical consecutive frames even with optimize=False — 6 identical frames become n_frames=1. Consequence: on a genuinely clear day every frame composites identically and the "animation" is a single-frame GIF. Correctly labelled either way, and there is nothing to animate, so this is benign — but the multi-frame test deliberately varies tiles per frame so it tests the #184 regression rather than Pillow's dedup.
Attachment size grows ~72 KiB → ~404 KiB. Fine for Discord, Signal, Matrix and Pushover's limits, but it is a real increase in outbound bytes per alert; radar_animation_frames and radar_animation_crop_px are configurable to tune it down.
72 tile requests per build against a free public API. The 5-minute radar cache absorbs repeats, but frame count multiplies request count — documented in the config comments.
Scope beyond the issue as filed: the same mislabelling was also fixed in Discord's /radar and Matrix's !radar commands, and in the dashboard/public radar endpoints, which hardcoded media_type="image/png" while capable of serving GIF bytes. Those endpoints remain deliberately un-composited (no basemap) per #182, since they render under a Leaflet tile layer.

Fixes #184. Targeted at **v2.0.0-rc3**. Based on `main` at the rc2 commit. ## Three bugs, one code path 1. **`animated=True` was a no-op** — `_fetch_radar` accepted the flag and never read it, always returning a static PNG. The "animated radar" option has never animated. 2. **Mislabelled as GIF everywhere** — cached as `image/gif`, attached to Discord as `radar.gif`, base64'd into Signal as `data:image/gif`. PNG bytes wearing a GIF label. 3. **`radar_animated` gated attachment, not animation** — turning "animated radar" off silently removed the radar image entirely. ## Why it needed more than a content-type fix **NOAA cannot animate.** Its MapServer exposes no `timeInfo` and its capabilities are `Map,Query,Data` — it serves "now" only, with no historical frames to sweep. **RainViewer can** (~13 past frames at 10-min cadence) and was already a dependency — but used as a single fixed tile at z6, which is coarse *and* badly framed: a location sits wherever it falls in its tile, and St. Louis lands at fraction **0.93** across its own, hard against the edge. | Source | Resolution | Framing | |---|---:|---| | NOAA still (before) | 0.72 km/px | centred | | RainViewer 1 tile @ z6/256 (before) | 1.91 km/px | wherever it lands | | **RainViewer 3x3 stitched @ z7/512 (now)** | **0.48 km/px** | **centred** | **3x3, not 2x2**: a location can sit near a tile corner, and only a 3x3 grid guarantees margin for a centred crop in every direction. The same stitcher backs the static fallback, so the off-centre framing defect is fixed for stills too — not just animations. ## Measurements that shaped the design - **Sequential tile fetching was disqualifying**: 5.1s for one frame's 9 tiles → ~40s per animation on the alert dispatch path. Fetched concurrently: **72 requests in 3.0s**, producing a 404 KiB GIF whose frames genuinely differ. - **CPU cost is ~71ms** for compositing + GIF encode. Unlike #128's radial decode, this does not meaningfully block the shared event loop, which is what makes in-process assembly acceptable here. ## Format has to travel with the bytes Because an animated request can silently fall back to a still, no caller can infer the format from the `animated` argument it passed. `_fetch_radar` now returns a `RadarImage` carrying the format actually produced, and every call site labels from it. The **cache-hit path sniffs magic bytes** rather than trusting the requested format — the disk backend doesn't persist content types, so an animated-key entry written during a RainViewer outage would otherwise be served mislabelled. ## Migration 0037 Adds `radar_enabled`, backfilled from each row's **own** `radar_animated`. That is the only value preserving existing behaviour — a constant default would have switched radar **on** for every channel that had deliberately opted out. `server_default` exists only so `ADD COLUMN` succeeds on a populated table, and is dropped after the backfill so the ORM default governs new rows. ## Verification Independently re-run, not taken on trust: - `ruff check app/ tests/` — clean - Bulk (SQLite) tier: **1001 passed, 14 deselected** - Postgres tier against a throwaway container: **14 passed**, including the 0037 backfill test (a `radar_animated=False` row must land on `radar_enabled=False`) ## Worth knowing - **Pillow collapses runs of byte-identical consecutive frames** even with `optimize=False` — 6 identical frames become `n_frames=1`. Consequence: on a genuinely clear day every frame composites identically and the "animation" is a single-frame GIF. Correctly labelled either way, and there is nothing to animate, so this is benign — but the multi-frame test deliberately varies tiles per frame so it tests the #184 regression rather than Pillow's dedup. - **Attachment size grows ~72 KiB → ~404 KiB.** Fine for Discord, Signal, Matrix and Pushover's limits, but it is a real increase in outbound bytes per alert; `radar_animation_frames` and `radar_animation_crop_px` are configurable to tune it down. - **72 tile requests per build** against a free public API. The 5-minute radar cache absorbs repeats, but frame count multiplies request count — documented in the config comments. - Scope beyond the issue as filed: the same mislabelling was also fixed in Discord's `/radar` and Matrix's `!radar` commands, and in the dashboard/public radar endpoints, which hardcoded `media_type="image/png"` while capable of serving GIF bytes. Those endpoints remain deliberately **un-composited** (no basemap) per #182, since they render under a Leaflet tile layer.

claude-bot added 1 commit

2026-08-19 21:36:07 +00:00

Make notifier radar actually animate, and split the flag that never meant what it said (#184 )

CI / test (pull_request) Successful in 4m9s

Details

6f12520a0f

Three bugs, one code path.

get_radar_image(animated=True) — what every notifier passes — was a no-op.
_fetch_radar accepted the flag and never read it, always returning a static
PNG. The result was then cached as image/gif, attached to Discord as
radar.gif, and base64'd into Signal as data:image/gif. PNG bytes wearing a
GIF label, and an "animated radar" option that had never animated.

The NOAA source cannot animate: its MapServer exposes no timeInfo and serves
"now" only, so there are no historical frames to sweep. RainViewer can — it
publishes ~13 past frames at 10-minute cadence. It was already a dependency,
used as a single fixed tile at zoom 6, which is both coarse (1.91 km/px) and
badly framed: a location sits wherever it falls inside its tile, and St.
Louis lands at fraction 0.93 across its own, hard against the edge.

Animation now stitches a 3x3 tile grid at z7/512 and crops a window centred
on the location. 3x3 rather than 2x2 because a location can sit near a tile
corner, and only a 3x3 grid guarantees margin for a centred crop in every
direction. The result is 0.48 km/px and correctly framed — sharper than the
NOAA still it replaces. The same stitcher backs the static fallback, so the
off-centre framing defect is fixed for stills too.

Tiles are fetched concurrently: sequentially this cost 5.1s per frame, which
would have put ~40s on the alert dispatch path and disqualified the whole
approach. Compositing and GIF encode measure ~71ms, so unlike #128's radial
decode this does not meaningfully block the shared event loop.

Because an animated request can silently fall back to a still, bytes alone
can no longer tell a caller what it got. _fetch_radar now returns a
RadarImage carrying the format actually produced, and every call site labels
from that instead of guessing — including the cache-hit path, which sniffs
magic bytes rather than trusting the requested format, since the disk
backend does not persist content types.

radar_animated never gated animation. It gated whether radar was attached at
all, so turning "animated radar" off silently removed the image entirely.
Migration 0037 adds radar_enabled and backfills it from each row's own
radar_animated, which is the only value that preserves existing behaviour: a
constant default would have switched radar on for every channel that had
deliberately opted out.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

claude-bot referenced this pull request

2026-08-19 21:41:04 +00:00

Notifier radar: make "animated" actually animate, and split the flag that doesn't mean what it says #184

claude-bot merged commit a66f70c2d1 into main

2026-08-21 03:36:09 +00:00

claude-bot deleted branch fix/notifier-radar-animation

2026-08-21 03:36:10 +00:00

claude-bot referenced this pull request from a commit

2026-08-21 03:36:11 +00:00

Merge pull request 'Make notifier radar actually animate, and split the flag that never meant what it said (#184)' (#185) from fix/notifier-radar-animation into main

claude-bot referenced this pull request

2026-08-21 03:43:35 +00:00