Unify retention onto one 13-month horizon (#133) #157

Merged
claude-bot merged 1 commit from feat/unified-retention-horizon into main 2026-07-28 16:24:12 +00:00
Contributor

Closes #133. Follow-up measurement split out to #158. Follows the capture phase (#129, #130, #131, #132) in the v2.0.0 Historical Explorer epic (#22).

Grounded in measured production data, not the arithmetic estimates. Full measurement set is on #133; it changed several of the issue's premises, and this PR follows the measurements rather than the original task list.

Problem

Retention windows disagreed, so an older event rendered with holes. At 100 days a row still had its alert record (90 d) and its snapshot (180 d) but had already lost its AI summary (30 d) and its SPC context (30 d past retained_until) — history rotting in inconsistent, confusing ways exactly where the Explorer needs it whole.

Change

HISTORY_HORIZON_DAYS (400 ≈ 13 months) is now the single horizon every history table follows, chosen so a year-over-year comparison always has both sides. Each per-table RETENTION_* setting defaults to None, meaning "follow the horizon"; a number is a deliberate exception that must carry a reason. Env var names are unchanged, so a deployment that already pins one keeps exactly what it pinned.

Table Before After
sent_alerts 90 d on cleared_at horizon, on coalesce(cleared_at, expires, sent_at)
nws_alert_snapshots 180 d horizon
ai_summary_records / _attempts 30 d horizon
spc_outlook_pages (+ children) 30 d past retained_until horizon
alert_radar_frames 400 d (own window) horizon
notification_deliveries unbounded horizon, on updated_at
redirect_tokens unbounded horizon, on coalesce(last_used_at, created_at)
lightning_clusters 90 d 90 d — pinned, documented
public_tokens 30 d 30 d — pinned, documented
SPC radar disk cache 7 d 7 d — pinned, documented

Three decisions worth reviewing

The sent_alerts basis change is the substantive fix. Pruning on cleared_at meant a row that never got cleared was never pruned — and that is not an edge case: production measured 225 of 1,708 rows (13.2%) with cleared_at IS NULL, all of them immortal. An alert ends up uncleared whenever the clearing path doesn't complete (feed drops it, it's superseded, process restarts mid-cycle). The fallbacks answer "when did this stop mattering?" in descending order of trustworthiness: when we cleared it, else when NWS said it expired, else when we sent it. A long-running hazard with a future expires is still protected — coalesce picks the expiry, not the old sent_at.

product_freshness_samples stays outside the horizon, at its own inline 14 days. It is the largest table in production — 63 MB of a 101 MB database, 155,672 rows, 78% of it spc_fetch — but it is observability, not history the Explorer replays. Putting it on the horizon would cost roughly 1.8 GB for no Explorer benefit. Called out because #133 listed it as unpruned; it is in fact pruned at observability.py:108-110, just not via retention.py.

AI records move 30 d → horizon. The "large payloads" worry that motivated the short window isn't borne out — production holds 0 rows and 64 kB — and #23 (historical display of AI summaries) needs a summary to survive as long as the event it describes. Pin it again if payload size ever becomes real; the override exists.

Corrections to the issue's task list

Two of the four tables #133 listed as unbounded were already pruned: product_freshness_samples (14 d, inline) and sent_forecasts (by sent_date, in forecast_delivery.py:45). Only notification_deliveries and redirect_tokens genuinely weren't.

The suspicion that spc_outlook_pages.product_text was "likely the largest avoidable share of the growth" is disproved by measurement: 2,614 bytes per issuance, 18 kB across all pages. Truncating it would save nothing.

Safety

A horizon below 1 day is now rejected at config load, and negative per-table overrides are rejected too. Every history table follows the horizon, so a typo there is not a small misconfiguration — it is unrecoverable data loss the daily job would carry out silently.

This deploy cannot delete anything on its first run. Every changed window is longer, so those tables only retain more. The two newly-bounded tables are safe by inspection: redirect_tokens is empty, and notification_deliveries (32 rows) cascades from sent_alerts, whose oldest row is 92 days old — well inside a 400-day window. Verified separately that production's .env pins no retention keys, so the new defaults will actually take effect there rather than being silently overridden.

Verification

Step Result
ruff check . passed
python -m compileall app OK
SQLite bulk tier 853 passed (up from 848)

tests/test_retention.py covers each coalesce fallback independently, the future-expires long-running-hazard case, both newly-bounded tables (including that a recently used redirect token created long ago survives), and asserts the horizon invariant itself rather than leaving it to comments. tests/test_config.py covers the new validation.

Added one Postgres-tier test: _cleanup_notification_deliveries is bounded on its own window on the stated grounds that most rows leave via ON DELETE CASCADE from sent_alerts — a DB-level guarantee SQLite cannot verify, so the claim is now asserted where FKs are actually enforced rather than left as a comment the bulk tier would happily pass without.

Deliberately not claimed

#133's "size the result at 13 months" task is not done here — it is split out to #158. Production holds 3 snapshots, 31 radar frames and 0 lightning clusters (capture ungated only hours before this measurement), so there is no basis for a 13-month projection of the capture tables. #158 carries that, plus the sent_alerts bloat and radar_snapshot_path watch items, to be revisited after a real convective season. What could be measured is posted on #130 (radar frames ≈ 73–113 MB/location/year, below the 165 MB estimate) and #132 (still unmeasurable at 0 rows).

🤖 Generated with Claude Code

Closes #133. Follow-up measurement split out to #158. Follows the capture phase (#129, #130, #131, #132) in the v2.0.0 Historical Explorer epic (#22). **Grounded in measured production data, not the arithmetic estimates.** Full measurement set is on [#133](https://git.rhoving.com/rbrooks/WeatherBot/issues/133#issuecomment-18937); it changed several of the issue's premises, and this PR follows the measurements rather than the original task list. ## Problem Retention windows disagreed, so an older event rendered with holes. At 100 days a row still had its alert record (90 d) and its snapshot (180 d) but had already lost its AI summary (30 d) and its SPC context (30 d past `retained_until`) — history rotting in inconsistent, confusing ways exactly where the Explorer needs it whole. ## Change `HISTORY_HORIZON_DAYS` (400 ≈ 13 months) is now the single horizon every history table follows, chosen so a year-over-year comparison always has both sides. Each per-table `RETENTION_*` setting defaults to `None`, meaning "follow the horizon"; a number is a deliberate exception that must carry a reason. **Env var names are unchanged**, so a deployment that already pins one keeps exactly what it pinned. | Table | Before | After | |---|---|---| | `sent_alerts` | 90 d on `cleared_at` | horizon, on `coalesce(cleared_at, expires, sent_at)` | | `nws_alert_snapshots` | 180 d | horizon | | `ai_summary_records` / `_attempts` | 30 d | horizon | | `spc_outlook_pages` (+ children) | 30 d past `retained_until` | horizon | | `alert_radar_frames` | 400 d (own window) | horizon | | `notification_deliveries` | **unbounded** | horizon, on `updated_at` | | `redirect_tokens` | **unbounded** | horizon, on `coalesce(last_used_at, created_at)` | | `lightning_clusters` | 90 d | **90 d — pinned, documented** | | `public_tokens` | 30 d | **30 d — pinned, documented** | | SPC radar disk cache | 7 d | **7 d — pinned, documented** | ## Three decisions worth reviewing **The `sent_alerts` basis change is the substantive fix.** Pruning on `cleared_at` meant a row that never got cleared was never pruned — and that is not an edge case: production measured **225 of 1,708 rows (13.2%)** with `cleared_at IS NULL`, all of them immortal. An alert ends up uncleared whenever the clearing path doesn't complete (feed drops it, it's superseded, process restarts mid-cycle). The fallbacks answer "when did this stop mattering?" in descending order of trustworthiness: when we cleared it, else when NWS said it expired, else when we sent it. A long-running hazard with a *future* `expires` is still protected — `coalesce` picks the expiry, not the old `sent_at`. **`product_freshness_samples` stays outside the horizon, at its own inline 14 days.** It is the largest table in production — **63 MB of a 101 MB database**, 155,672 rows, 78% of it `spc_fetch` — but it is observability, not history the Explorer replays. Putting it on the horizon would cost roughly **1.8 GB** for no Explorer benefit. Called out because #133 listed it as unpruned; it is in fact pruned at [observability.py:108-110](app/services/observability.py#L108-L110), just not via `retention.py`. **AI records move 30 d → horizon.** The "large payloads" worry that motivated the short window isn't borne out — production holds 0 rows and 64 kB — and #23 (historical display of AI summaries) needs a summary to survive as long as the event it describes. Pin it again if payload size ever becomes real; the override exists. ## Corrections to the issue's task list Two of the four tables #133 listed as unbounded were **already pruned**: `product_freshness_samples` (14 d, inline) and `sent_forecasts` (by `sent_date`, in [forecast_delivery.py:45](app/services/forecast_delivery.py#L45)). Only `notification_deliveries` and `redirect_tokens` genuinely weren't. The suspicion that `spc_outlook_pages.product_text` was "likely the largest avoidable share of the growth" is **disproved by measurement**: 2,614 bytes per issuance, 18 kB across all pages. Truncating it would save nothing. ## Safety A horizon below 1 day is now rejected at config load, and negative per-table overrides are rejected too. Every history table follows the horizon, so a typo there is not a small misconfiguration — it is unrecoverable data loss the daily job would carry out silently. **This deploy cannot delete anything on its first run.** Every changed window is *longer*, so those tables only retain more. The two newly-bounded tables are safe by inspection: `redirect_tokens` is empty, and `notification_deliveries` (32 rows) cascades from `sent_alerts`, whose oldest row is 92 days old — well inside a 400-day window. Verified separately that production's `.env` pins **no** retention keys, so the new defaults will actually take effect there rather than being silently overridden. ## Verification | Step | Result | |---|---| | `ruff check .` | passed | | `python -m compileall app` | OK | | SQLite bulk tier | **853 passed** (up from 848) | `tests/test_retention.py` covers each `coalesce` fallback independently, the future-`expires` long-running-hazard case, both newly-bounded tables (including that a *recently used* redirect token created long ago survives), and asserts the horizon invariant itself rather than leaving it to comments. `tests/test_config.py` covers the new validation. Added one **Postgres-tier** test: `_cleanup_notification_deliveries` is bounded on its own window on the stated grounds that most rows leave via `ON DELETE CASCADE` from `sent_alerts` — a DB-level guarantee SQLite cannot verify, so the claim is now asserted where FKs are actually enforced rather than left as a comment the bulk tier would happily pass without. ## Deliberately not claimed **#133's "size the result at 13 months" task is not done here — it is split out to #158.** Production holds 3 snapshots, 31 radar frames and 0 lightning clusters (capture ungated only hours before this measurement), so there is no basis for a 13-month projection of the capture tables. #158 carries that, plus the `sent_alerts` bloat and `radar_snapshot_path` watch items, to be revisited after a real convective season. What *could* be measured is posted on #130 (radar frames ≈ 73–113 MB/location/year, below the 165 MB estimate) and #132 (still unmeasurable at 0 rows). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Unify retention onto one 13-month horizon (#133)
All checks were successful
CI / test (pull_request) Successful in 4m46s
af1d9ea047
Retention windows disagreed, so an older event rendered with holes: at 100 days
a row still had its alert record (90d) and its snapshot (180d) but had already
lost its AI summary (30d) and its SPC context (30d past retained_until). History
rotted in inconsistent, confusing ways precisely where the v2.0.0 Explorer needs
it whole.

HISTORY_HORIZON_DAYS (400 ~= 13 months) is now the single horizon every history
table follows, chosen so a year-over-year comparison always has both sides. Each
per-table RETENTION_* setting defaults to None meaning "follow the horizon";
a number is a deliberate exception that has to carry a reason. Env var names are
unchanged, so a deployment that already pins one keeps exactly what it pinned.

- sent_alerts now prunes on coalesce(cleared_at, expires, sent_at) rather than
  cleared_at alone. Pruning on cleared_at meant a row that never got cleared was
  never pruned, and that is not an edge case: production measured 225 of 1,708
  rows (13.2%) with cleared_at IS NULL, all of them immortal. The fallbacks
  answer "when did this stop mattering?" in descending order of trust -- when we
  cleared it, else when NWS said it expired, else when we sent it.
- notification_deliveries and redirect_tokens were genuinely unbounded and are
  now swept. redirect_tokens keys on coalesce(last_used_at, created_at) so a
  short link still circulating in someone's chat is not broken by its age.
- Deliberate exceptions, each documented: lightning_clusters at 90d (highest
  volume, fastest-decaying value, still unmeasured), public_tokens at 30d
  (credential hygiene, not history), radar_cache at 7d (a fetch cache).
  product_freshness_samples stays at its own inline 14 days -- it is
  observability rather than history, and measured 63 MB of a 101 MB database,
  so following the horizon would cost roughly 1.8 GB for no Explorer benefit.
- A horizon below 1 day is now rejected at config load. Every history table
  follows it, so a typo there is not a small misconfiguration -- it is
  unrecoverable data loss the daily job would carry out silently.

Two of the four tables this issue listed as unpruned turned out to be pruned
already (product_freshness_samples inline at 14d, sent_forecasts by sent_date),
and the suspicion that spc_outlook_pages.product_text was the largest avoidable
share of growth is disproved by measurement at 2,614 bytes per issuance.

Sizing the capture tables at 13 months is deliberately NOT claimed here. Prod
holds 3 snapshots, 31 radar frames and 0 lightning clusters -- capture ungated
only hours earlier -- so that task stays open on #133 pending a real convective
season rather than being closed with a projection dressed up as a measurement.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
claude-bot deleted branch feat/unified-retention-horizon 2026-07-28 16:24:12 +00:00
Sign in to join this conversation.
No reviewers
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!157
No description provided.