Fix radar capture, which rc3 broke by changing what _fetch_radar returns (#184) #187

Merged

claude-bot merged 1 commit from fix/radar-image-callers into main

2026-08-21 05:46:20 +00:00

claude-bot commented

2026-08-21 05:36:25 +00:00

Contributor

Fixes a regression introduced by #185 (rc3). Targeted at v2.0.0-rc4.

What broke

#185 changed _fetch_radar to return a RadarImage instead of bytes, and updated the notifiers and API endpoints that consume it. It missed the two callers that write radar to disk:

app/services/radar_frames.py — periodic frame capture (#130)
app/services/alert_processor.py — alert-time snapshot (#85)

Both hand the value straight to storage.put() and len(), so every capture raised:

memoryview: a bytes-like object is required, not 'RadarImage'

Both call sites are deliberately fail-soft, so nothing surfaced — no alert was affected, nothing propagated, and the only trace was one warning line per alert per 5-minute cycle.

Net effect: rc3 silently disabled the radar history capture that rc2 had just fixed. No frames captured means no Explorer radar loop (#140) and no alert-time snapshot (#85).

How it was caught

By pointing a temporary dev location at live convection near Jackson, MS and watching zero frames accumulate against four active alerts over 41 minutes. Confirmed in the container logs:

2026-08-21 04:37:24 WARNING app.services.radar_frames Radar frame capture failed
  for lifecycle=NWS_KJAN_SV_W_0321 location=48f10aef-...:
  memoryview: a bytes-like object is required, not 'RadarImage'

Three release candidates and a fully green 1001-test suite did not.

Why the suite stayed green — the actual defect

tests/conftest.py:119 installs an autouse fixture that monkeypatches app.services.radar._fetch_radar to return None for every test in the suite. Tests that assert on capture substitute their own mock, each returning whatever shape it assumed at the time.

So no test anywhere ran the real _fetch_radar, and its contract could change underneath every caller without a single failure. The .data correction is the trivial part; this is the part worth fixing.

The fix

Both call sites use image.data, and pass image.content_type rather than a hardcoded image/png — the format is no longer knowable from the filename.
The mocks in test_radar_frames.py and test_radar_snapshot.py now return a real RadarImage, so they cannot silently drift from the real return type again.
New test_capture_uses_the_real_fetch_radar_return_type restores the real _fetch_radar — captured at module import, before the autouse fixture applies — and mocks only the HTTP layer beneath it, so the caller/callee contract is genuinely exercised. It asserts the bytes on disk are the image payload, not a repr of the wrapper.

Verification

The new test was confirmed to fail with the exact production error when the radar_frames.py fix is reverted, and pass with it — not trusted as a smoke test.
ruff check app/ tests/ — clean
Full suite: 1002 passed, 14 deselected

Note for the release

rc3 should not go to production. This needs an rc4, and the dev soak restarted — the Jackson MS location has captured no frames, so #182's loop fix still has no live verification.

Fixes a regression introduced by #185 (rc3). Targeted at **v2.0.0-rc4**. ## What broke #185 changed `_fetch_radar` to return a `RadarImage` instead of bytes, and updated the notifiers and API endpoints that consume it. It **missed the two callers that write radar to disk**: - `app/services/radar_frames.py` — periodic frame capture (#130) - `app/services/alert_processor.py` — alert-time snapshot (#85) Both hand the value straight to `storage.put()` and `len()`, so every capture raised: ``` memoryview: a bytes-like object is required, not 'RadarImage' ``` Both call sites are deliberately fail-soft, so nothing surfaced — no alert was affected, nothing propagated, and the only trace was one warning line per alert per 5-minute cycle. **Net effect: rc3 silently disabled the radar history capture that rc2 had just fixed.** No frames captured means no Explorer radar loop (#140) and no alert-time snapshot (#85). ## How it was caught By pointing a temporary dev location at live convection near Jackson, MS and watching **zero frames accumulate against four active alerts over 41 minutes**. Confirmed in the container logs: ``` 2026-08-21 04:37:24 WARNING app.services.radar_frames Radar frame capture failed for lifecycle=NWS_KJAN_SV_W_0321 location=48f10aef-...: memoryview: a bytes-like object is required, not 'RadarImage' ``` Three release candidates and a fully green 1001-test suite did not. ## Why the suite stayed green — the actual defect `tests/conftest.py:119` installs an **autouse fixture that monkeypatches `app.services.radar._fetch_radar` to return `None` for every test in the suite**. Tests that assert on capture substitute their own mock, each returning whatever shape it assumed at the time. So **no test anywhere ran the real `_fetch_radar`**, and its contract could change underneath every caller without a single failure. The `.data` correction is the trivial part; this is the part worth fixing. ## The fix 1. Both call sites use `image.data`, and pass `image.content_type` rather than a hardcoded `image/png` — the format is no longer knowable from the filename. 2. The mocks in `test_radar_frames.py` and `test_radar_snapshot.py` now return a real `RadarImage`, so they cannot silently drift from the real return type again. 3. New `test_capture_uses_the_real_fetch_radar_return_type` **restores the real `_fetch_radar`** — captured at module import, before the autouse fixture applies — and mocks only the HTTP layer beneath it, so the caller/callee contract is genuinely exercised. It asserts the bytes on disk are the image payload, not a repr of the wrapper. ## Verification - The new test was confirmed to **fail with the exact production error** when the `radar_frames.py` fix is reverted, and pass with it — not trusted as a smoke test. - `ruff check app/ tests/` — clean - Full suite: **1002 passed, 14 deselected** ## Note for the release rc3 should not go to production. This needs an rc4, and the dev soak restarted — the Jackson MS location has captured no frames, so **#182's loop fix still has no live verification**.

claude-bot added 1 commit

2026-08-21 05:36:26 +00:00

Fix radar capture, which rc3 broke by changing what _fetch_radar returns (#184 )

CI / test (pull_request) Successful in 6m38s

Details

1c5d4da710

rc3 made _fetch_radar return a RadarImage instead of bytes, and updated the
notifiers and API endpoints that consume it. It missed the two callers that
capture radar to disk:

  app/services/radar_frames.py   periodic frame capture (#130)
  app/services/alert_processor.py alert-time snapshot (#85)

Both passed the wrapper straight to storage.put() and len(), so every capture
raised "a bytes-like object is required, not 'RadarImage'". Both call sites
are deliberately fail-soft, so nothing surfaced: no alert was affected, no
error propagated, and the only trace was a warning line per alert per cycle.

The effect was that rc3 silently disabled the radar history capture that rc2
had just fixed — no frames, so no Explorer radar loop and no alert-time
snapshot. Caught on dev by pointing a temporary location at live convection
in Mississippi and watching zero frames accumulate against four active
alerts over 41 minutes.

The whole suite was green throughout, because conftest.py installs an autouse
fixture that stubs _fetch_radar out for EVERY test, and the tests that care
substitute their own mock returning whatever shape they assumed. Nothing
anywhere ran the real function, so the contract could change under all of
them silently.

So the fix is not just .data at the two call sites. Those mocks now return a
real RadarImage, and a new test restores the real _fetch_radar (captured at
import, before the autouse fixture applies) and mocks only the HTTP layer
beneath it, so the caller/callee contract is actually exercised. Verified it
fails with the exact production error when the fix is reverted.

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

claude-bot merged commit 5f95a55e0d into main

2026-08-21 05:46:20 +00:00

claude-bot deleted branch fix/radar-image-callers

2026-08-21 05:46:21 +00:00

claude-bot referenced this pull request from a commit

2026-08-21 05:46:21 +00:00

Merge pull request 'Fix radar capture, which rc3 broke by changing what _fetch_radar returns (#184)' (#187) from fix/radar-image-callers into main

claude-bot referenced this pull request

2026-08-21 05:58:12 +00:00

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