expire_alerts_job clears on timestamp alone, without any feed cross-check

claude-bot commented

2026-07-28 00:37:05 +00:00

Contributor

expire_alerts_job (app/services/scheduler.py:94-119) clears alerts on the stored timestamp alone:

select(SentAlert)
  .where(SentAlert.expires != None)
  .where(SentAlert.expires < now)
  .where(SentAlert.cleared_at == None)

_clear_missing_alerts requires an alert to be both past its expiry and absent from active_matches before clearing it. This job has no such cross-check, so the two paths disagree about what "expired" means.

That asymmetry matters when the feed and the stored timestamp disagree:

fetch_alerts_for_states returns None during an NWS outage and process_alerts correctly skips the cycle (#40) — but this job keeps running every 5 minutes and keeps clearing on stale timestamps, firing all-clears nobody can verify.
An alert extended by NWS (VTEC EXT) has its expires refreshed by the poll path — but if the poll is failing or lagging, this job can clear it in the window before the extension is seen.

The job's docstring says it exists for the case where "the poll cycle missed the cancellation window", which is reasonable — but a poll cycle that is failing is exactly when its timestamps are least trustworthy, and that is precisely when this job acts unilaterally.

Fix

Gate the job on recent poll health before clearing. The heartbeat/freshness machinery already exists (product_freshness, heartbeat_watchdog_job, settings.heartbeat_stale_minutes): if the last successful alert poll is stale, skip the expiry sweep rather than clearing blind. Optionally require the alert to have been absent from the last successful poll's match set, to match _clear_missing_alerts semantics exactly.

Same family as #145 and the pagination-truncation issue: all three end in a false all-clear by a different route.

`expire_alerts_job` (`app/services/scheduler.py:94-119`) clears alerts on the stored timestamp alone: ```python select(SentAlert) .where(SentAlert.expires != None) .where(SentAlert.expires < now) .where(SentAlert.cleared_at == None) ``` `_clear_missing_alerts` requires an alert to be **both** past its expiry **and** absent from `active_matches` before clearing it. This job has no such cross-check, so the two paths disagree about what "expired" means. That asymmetry matters when the feed and the stored timestamp disagree: - `fetch_alerts_for_states` returns `None` during an NWS outage and `process_alerts` correctly **skips the cycle** (`#40`) — but this job keeps running every 5 minutes and keeps clearing on stale timestamps, firing all-clears nobody can verify. - An alert extended by NWS (VTEC `EXT`) has its `expires` refreshed by the poll path — but if the poll is failing or lagging, this job can clear it in the window before the extension is seen. The job's docstring says it exists for the case where "the poll cycle missed the cancellation window", which is reasonable — but a poll cycle that is *failing* is exactly when its timestamps are least trustworthy, and that is precisely when this job acts unilaterally. ## Fix Gate the job on recent poll health before clearing. The heartbeat/freshness machinery already exists (`product_freshness`, `heartbeat_watchdog_job`, `settings.heartbeat_stale_minutes`): if the last successful alert poll is stale, skip the expiry sweep rather than clearing blind. Optionally require the alert to have been absent from the last successful poll's match set, to match `_clear_missing_alerts` semantics exactly. ## Related Same family as #145 and the pagination-truncation issue: all three end in a false all-clear by a different route.

claude-bot added the

bug

area:notifications

labels

2026-07-28 00:37:05 +00:00

claude-bot referenced this issue

2026-07-28 00:38:19 +00:00

False all-clear when NWS replaces an alert (watch -> advisory/warning) #145

rbrooks referenced this issue from a commit

2026-07-28 00:48:06 +00:00

Suppress false all-clears when a hazard is superseded (#145)

claude-bot referenced this issue

2026-07-28 00:48:28 +00:00

Suppress false all-clears when a hazard is superseded (#145) #149

claude-bot referenced this issue

2026-07-28 00:59:08 +00:00

False all-clear when NWS replaces an alert (watch -> advisory/warning) #145

rbrooks referenced this issue from a commit

2026-07-28 01:37:13 +00:00

Don't clear alerts on incomplete information (#146, #147)

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

2026-07-28 01:37:35 +00:00

Don't clear alerts on incomplete information (#146, #147) #150

claude-bot closed this issue

2026-07-28 01:47:46 +00:00

claude-bot referenced this issue from a commit

2026-07-28 01:47:46 +00:00

Merge pull request 'Don't clear alerts on incomplete information (#146, #147)' (#150) from fix/incomplete-feed-guards into main

claude-bot commented

2026-07-28 02:08:26 +00:00

Author

Contributor

Fixed in #150 (merged). CI green.

expire_alerts_job now gates on poll freshness via a new observability.latest_alert_poll_success helper, reusing the existing heartbeat_stale_minutes threshold rather than adding another knob. No recent successful poll — including none ever recorded, which reads as "not fresh" rather than "fine" — means skip the sweep and log why, while still delivering all-clears already decided by a healthy path.

One existing test changed: test_expire_alerts_job_sends_lifted_once_across_repeated_runs now seeds a healthy poll record. Its subject is lifted-notification dedup, not freshness, so the precondition is declared rather than the assertion weakened.

Verified locally in Docker: ruff, 805 SQLite-tier tests, migrations on Postgres 16, 4 Postgres-tier tests. Includes a control (fresh poller → still expires).

Not done here: making the job require the alert to have been absent from the last successful poll's match set, which would match _clear_missing_alerts semantics exactly. The freshness gate covers the dangerous case (acting while blind); exact parity would need the poll to persist its match set, which is a larger change and did not seem worth it yet.

Fixed in #150 (merged). CI green. `expire_alerts_job` now gates on poll freshness via a new `observability.latest_alert_poll_success` helper, reusing the existing `heartbeat_stale_minutes` threshold rather than adding another knob. No recent successful poll — **including none ever recorded**, which reads as "not fresh" rather than "fine" — means skip the sweep and log why, while still delivering all-clears already decided by a healthy path. One existing test changed: `test_expire_alerts_job_sends_lifted_once_across_repeated_runs` now seeds a healthy poll record. Its subject is lifted-notification dedup, not freshness, so the precondition is declared rather than the assertion weakened. Verified locally in Docker: ruff, 805 SQLite-tier tests, migrations on Postgres 16, 4 Postgres-tier tests. Includes a control (fresh poller → still expires). Not done here: making the job require the alert to have been absent from the last successful poll's match set, which would match `_clear_missing_alerts` semantics exactly. The freshness gate covers the dangerous case (acting while blind); exact parity would need the poll to persist its match set, which is a larger change and did not seem worth it yet.

claude-bot referenced this issue

2026-07-28 03:14:35 +00:00

Webex all-clear 400s in an endless retry loop; response body not logged #148

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

Rows
Columns

expire_alerts_job clears on timestamp alone, without any feed cross-check #147

Fix

Related