rbrooks/WeatherBot

Watch

Fork 0

Decouple historical alert-snapshot capture from public-page gating #129

New issue

Closed

opened 2026-07-27 20:06:40 +00:00 by claude-bot · 2 comments

claude-bot commented

2026-07-27 20:06:40 +00:00

Contributor

Parent: #22.

nws_alert_snapshots is the only table holding the full official alert body (description, instruction, area_desc, hazards, geometry, affected_zones, vtec) — the raw material the Explorer needs. Rows are created in app/services/public_alerts.py:117 (persist_alert_snapshot), reached only via get_or_create_public_alert_page.

Actual gating (corrected after reading the code)

The original text of this issue said capture required PUBLIC_ALERT_PAGES_ENABLED and Location.public_enabled. That was wrong in one direction and understated in another. What the code actually does:

settings.public_alert_pages_enabled gates capture (app/config.py:253, default False) — checked in _ensure_public_alert_page (alert_processor.py:116) and again in get_or_create_public_alert_page (public_alerts.py:212). This part was right.
Location.public_enabled does NOT gate capture. get_or_create_public_alert_page never inspects it — a page and token are created for every matched location when the flag is on. public_enabled is enforced at serve time only (app/api/public.py:611 for /p/{token}, and in the feed routes). So this was wrong.
Capture is coupled to channel dispatch, which is the bigger gap. In _dispatch_to_locations, _ensure_public_alert_page is called from exactly two places: line 281, only when a page already exists, and line 351, inside the per-channel dispatch loop. _process_cancel is gated the same way (has_channel_record or has_existing_page, line 406).

The consequence: a location with no channel subscriptions never gets a snapshot at all, even with PUBLIC_ALERT_PAGES_ENABLED=true. The dashboard record is still written unconditionally (_record_and_dispatch(..., dashboard_only=True), line 292), so such a location accrues sent_alerts rows — event, severity, headline, times — and nothing else. Dashboard-only locations are precisely the ones whose history is currently emptiest.

Change

Capture a snapshot for every matched location on the dashboard-record path, independent of both the public-pages flag and channel dispatch. Keep public_alert_pages_enabled and Location.public_enabled governing only exposure — who may read a snapshot and whether a /p/{token} page is minted.

Capture and publication become independent concerns, which is what they always should have been.

Tasks

New fail-soft _capture_alert_snapshot helper, savepoint-wrapped like _ensure_public_alert_page, called for every matched location in _dispatch_to_locations before the channel loop.
Same for _process_cancel, so CAN messages are captured for every location with records rather than only channel-dispatched ones — a cancel is part of the lifecycle the timeline needs.
Thread the resulting snapshot into get_or_create_public_alert_page via its existing snapshot= parameter so the page path does not redundantly re-persist.
Leave public_alert_pages / public_tokens creation gated exactly as today.
Handle the (location_id, lifecycle_id, content_hash) unique-constraint race between overlapping poll runs quietly — the row exists either way, and logging a traceback every poll would be noise.
Confirm identical re-polls still dedup on content_hash and do not create rows.
Measure row growth per alert per location; post the numbers here.
Tests: snapshot written for a location with no channel subscriptions and public pages disabled; no public token minted in that case; repeated identical polls produce exactly one row.

Why this is first

History not captured is gone forever. Every day this is unshipped is a day the Explorer will have nothing to show for — and today the gap is total for dashboard-only locations.

Parent: #22. `nws_alert_snapshots` is the only table holding the full official alert body (`description`, `instruction`, `area_desc`, `hazards`, `geometry`, `affected_zones`, `vtec`) — the raw material the Explorer needs. Rows are created in `app/services/public_alerts.py:117` (`persist_alert_snapshot`), reached only via `get_or_create_public_alert_page`. ## Actual gating (corrected after reading the code) The original text of this issue said capture required `PUBLIC_ALERT_PAGES_ENABLED` **and** `Location.public_enabled`. That was wrong in one direction and understated in another. What the code actually does: 1. **`settings.public_alert_pages_enabled` gates capture** (`app/config.py:253`, default **False**) — checked in `_ensure_public_alert_page` (`alert_processor.py:116`) and again in `get_or_create_public_alert_page` (`public_alerts.py:212`). This part was right. 2. **`Location.public_enabled` does NOT gate capture.** `get_or_create_public_alert_page` never inspects it — a page and token are created for every matched location when the flag is on. `public_enabled` is enforced at *serve* time only (`app/api/public.py:611` for `/p/{token}`, and in the feed routes). So this was wrong. 3. **Capture is coupled to channel dispatch, which is the bigger gap.** In `_dispatch_to_locations`, `_ensure_public_alert_page` is called from exactly two places: line 281, only when a page *already* exists, and line 351, **inside the per-channel dispatch loop**. `_process_cancel` is gated the same way (`has_channel_record or has_existing_page`, line 406). The consequence: **a location with no channel subscriptions never gets a snapshot at all**, even with `PUBLIC_ALERT_PAGES_ENABLED=true`. The dashboard record is still written unconditionally (`_record_and_dispatch(..., dashboard_only=True)`, line 292), so such a location accrues `sent_alerts` rows — event, severity, headline, times — and nothing else. Dashboard-only locations are precisely the ones whose history is currently emptiest. ## Change Capture a snapshot for **every matched location** on the dashboard-record path, independent of both the public-pages flag and channel dispatch. Keep `public_alert_pages_enabled` and `Location.public_enabled` governing only **exposure** — who may read a snapshot and whether a `/p/{token}` page is minted. Capture and publication become independent concerns, which is what they always should have been. ## Tasks - [ ] New fail-soft `_capture_alert_snapshot` helper, savepoint-wrapped like `_ensure_public_alert_page`, called for every matched location in `_dispatch_to_locations` before the channel loop. - [ ] Same for `_process_cancel`, so CAN messages are captured for every location with records rather than only channel-dispatched ones — a cancel is part of the lifecycle the timeline needs. - [ ] Thread the resulting snapshot into `get_or_create_public_alert_page` via its existing `snapshot=` parameter so the page path does not redundantly re-persist. - [ ] Leave `public_alert_pages` / `public_tokens` creation gated exactly as today. - [ ] Handle the `(location_id, lifecycle_id, content_hash)` unique-constraint race between overlapping poll runs quietly — the row exists either way, and logging a traceback every poll would be noise. - [ ] Confirm identical re-polls still dedup on `content_hash` and do not create rows. - [ ] Measure row growth per alert per location; post the numbers here. - [ ] Tests: snapshot written for a location with **no channel subscriptions** and public pages disabled; no public token minted in that case; repeated identical polls produce exactly one row. ## Why this is first History not captured is gone forever. Every day this is unshipped is a day the Explorer will have nothing to show for — and today the gap is total for dashboard-only locations.

claude-bot added this to the v2.0.0 - Historical Weather Event Explorer milestone

2026-07-27 20:06:40 +00:00

claude-bot added the

v2.0.0

type:feature

labels

2026-07-27 20:06:40 +00:00

claude-bot referenced this issue

2026-07-27 20:09:21 +00:00

Historical Weather Event Explorer #22

claude-bot commented

2026-07-27 20:34:29 +00:00

Author

Contributor

Picking this up on branch feat/history-capture-ungate.

Reading the code first turned up two corrections to this issue as originally filed, now folded into the body above:

Location.public_enabled does not gate capture — get_or_create_public_alert_page never checks it. It gates serving only.
Capture is reached only from the per-channel dispatch loop (alert_processor.py:351) or when a page already exists (:281). So a location with no channel subscriptions gets no snapshot at all, even with PUBLIC_ALERT_PAGES_ENABLED=true. That is a larger gap than "the flag is off by default", and it hits dashboard-only locations hardest.

Picking this up on branch `feat/history-capture-ungate`. Reading the code first turned up two corrections to this issue as originally filed, now folded into the body above: - `Location.public_enabled` does **not** gate capture — `get_or_create_public_alert_page` never checks it. It gates serving only. - Capture is reached only from the **per-channel dispatch loop** (`alert_processor.py:351`) or when a page already exists (`:281`). So a location with no channel subscriptions gets no snapshot at all, even with `PUBLIC_ALERT_PAGES_ENABLED=true`. That is a larger gap than "the flag is off by default", and it hits dashboard-only locations hardest.

rbrooks referenced this issue from a commit

2026-07-27 23:04:38 +00:00

Capture alert snapshots independent of public pages (#129)

claude-bot referenced this issue from a pull request that will close it,

2026-07-27 23:04:58 +00:00

Capture alert snapshots independent of public pages (#129) #143

claude-bot closed this issue

2026-07-27 23:15:30 +00:00

claude-bot referenced this issue from a commit

2026-07-27 23:15:30 +00:00

Merge pull request 'Capture alert snapshots independent of public pages (#129)' (#143) from feat/history-capture-ungate into main

claude-bot commented

2026-07-27 23:16:15 +00:00

Author

Contributor

Done in #143 (merged to main).

Shipped: _capture_alert_snapshot — fail-soft, savepoint-wrapped — now runs for every matched location on the dashboard path, before and independent of channel dispatch, and on the Cancel path for every location holding records. The snapshot is threaded into get_or_create_public_alert_page via its existing snapshot= parameter so the page path does not re-persist. IntegrityError on the (location_id, lifecycle_id, content_hash) constraint is caught and logged at debug, since overlapping polls can race and the row exists either way. Public page and token creation remain gated on public_alert_pages_enabled — capture and exposure are now separate concerns.

Tests: new tests/test_history_capture.py covers capture with no channel subscriptions and public pages disabled (asserting zero PublicAlertPage/PublicToken rows), dedup across three identical polls, append on a CON content change, and capture on Cancel.

Verified locally in Docker against every CI gate: ruff clean, compileall OK, 783 SQLite-tier tests passed, alembic upgrade head clean through 0030 on a fresh Postgres 16, 4 Postgres-tier tests passed. CI on the PR was green in 4m30s.

Two follow-ups, neither blocking:

Row-growth measurement is still open. It could not be measured meaningfully from tests — it needs production traffic. Worth checking nws_alert_snapshots row count and table size after a week of real polling, especially before #133 settles the 13-month horizon, since this change raises the write rate for any deployment that had dashboard-only locations.
test_quiet_hours_suppress_channel_send_but_keep_dashboard_record now emits a RuntimeWarning (unawaited coroutine): it passes a MagicMock session, and _capture_alert_snapshot has no early return, so it reaches begin_nested() on the mock and the failure is swallowed fail-soft. Production behaviour is correct and covered. An identical warning already exists in test_webhook_ssrf. One-line mock fix if it becomes annoying.

Done in #143 (merged to `main`). **Shipped:** `_capture_alert_snapshot` — fail-soft, savepoint-wrapped — now runs for every matched location on the dashboard path, before and independent of channel dispatch, and on the Cancel path for every location holding records. The snapshot is threaded into `get_or_create_public_alert_page` via its existing `snapshot=` parameter so the page path does not re-persist. `IntegrityError` on the `(location_id, lifecycle_id, content_hash)` constraint is caught and logged at debug, since overlapping polls can race and the row exists either way. Public page and token creation remain gated on `public_alert_pages_enabled` — capture and exposure are now separate concerns. **Tests:** new `tests/test_history_capture.py` covers capture with no channel subscriptions and public pages disabled (asserting zero `PublicAlertPage`/`PublicToken` rows), dedup across three identical polls, append on a CON content change, and capture on Cancel. **Verified locally in Docker against every CI gate:** ruff clean, `compileall` OK, 783 SQLite-tier tests passed, `alembic upgrade head` clean through `0030` on a fresh Postgres 16, 4 Postgres-tier tests passed. CI on the PR was green in 4m30s. **Two follow-ups, neither blocking:** 1. **Row-growth measurement is still open.** It could not be measured meaningfully from tests — it needs production traffic. Worth checking `nws_alert_snapshots` row count and table size after a week of real polling, especially before #133 settles the 13-month horizon, since this change raises the write rate for any deployment that had dashboard-only locations. 2. `test_quiet_hours_suppress_channel_send_but_keep_dashboard_record` now emits a `RuntimeWarning` (unawaited coroutine): it passes a `MagicMock` session, and `_capture_alert_snapshot` has no early return, so it reaches `begin_nested()` on the mock and the failure is swallowed fail-soft. Production behaviour is correct and covered. An identical warning already exists in `test_webhook_ssrf`. One-line mock fix if it becomes annoying.

claude-bot referenced this issue

2026-07-27 23:21:36 +00:00

Capture radar snapshots for every alert, with an independent retention window #130

claude-bot referenced this issue

2026-07-28 03:40:47 +00:00

CD deploys only to the dev host; the live instance is 6 days behind and manually managed #154

claude-bot referenced this issue

2026-07-28 06:12:58 +00:00

Archive ENTLN lightning clusters (#132) #156

claude-bot referenced this issue

2026-07-28 14:26:25 +00:00

Lightning cluster archive #132