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

Merged
claude-bot merged 1 commit from fix/incomplete-feed-guards into main 2026-07-28 01:47:45 +00:00
Contributor

Closes #146. Closes #147.

Two independent routes to a false all-clear, taken together because they are the same defect: treating "absent" as "ended" when the evidence cannot support it. Both land in the alert-clearing decision, so splitting them would mean two passes over the same code.

#146 — truncation looked like success

fetch_alerts_for_states followed pagination.next but stopped at a hard 10-page cap and still returned an ordinary list, indistinguishable from a complete fetch. Every alert past the cap was then cleared as "disappeared from the feed".

The comment directly above that loop said pagination existed precisely to stop this happening at page 1. The cap reintroduced it at page 10. Worse, the trigger is feed size, so it fires during large multi-state outbreaks — exactly when the dropped alerts matter most.

Fix. Fetches return an AlertFeed carrying a complete flag, set False when pages remained at the cap. It subclasses list, so every existing caller and test works unchanged (len, indexing, iteration, is None). process_alerts still dispatches the alerts it did receive — they are real — but skips the clearing pass entirely, because absence from a partial feed proves nothing.

The cap moves 10 → 50, far above any plausible legitimate feed for the handful of states one install monitors, so it reads as the runaway guard it is. Hitting it now logs at ERROR instead of passing silently.

#147 — expiry swept on an unverifiable timestamp

expire_alerts_job cleared purely on stored expires, with no feed cross-check — unlike _clear_missing_alerts, which also requires the alert to be absent from the live feed.

That asymmetry is unsafe exactly when the poller is unhealthy. A failed fetch makes poll_alerts_job skip the cycle (#40), and an alert extended by NWS (VTEC EXT) has its expires refreshed only by that same poll. So the job acts unilaterally on data it cannot verify, at the moment that data is least trustworthy.

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

Tests

tests/test_incomplete_feed_guards.py, 8 tests, each guard with a control so it cannot silently stop legitimate all-clears:

  • AlertFeed completeness defaults
  • feed marked incomplete when pages remain at the cap, and pagination stops there
  • control: pagination ending naturally → complete is True
  • incomplete feed → clearing skipped, nothing cleared
  • control: same empty feed trusted → does clear
  • expiry sweep skipped when the last poll is stale
  • expiry sweep skipped when no poll has ever succeeded
  • control: fresh poll → does clear

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.

Verification

Step Result
ruff check . passed
python -m compileall app OK
SQLite bulk tier 805 passed
alembic upgrade head on fresh Postgres 16 clean
Postgres integration tier 4 passed

Relationship to #145

#145 fixed supersession — a hazard continuing under a different product. These two fix incomplete evidence. All three end in the same symptom, by different routes; none subsumes another.

🤖 Generated with Claude Code

Closes #146. Closes #147. Two independent routes to a false all-clear, taken together because they are the same defect: **treating "absent" as "ended" when the evidence cannot support it.** Both land in the alert-clearing decision, so splitting them would mean two passes over the same code. ## #146 — truncation looked like success `fetch_alerts_for_states` followed `pagination.next` but stopped at a hard 10-page cap and still returned an ordinary list, indistinguishable from a complete fetch. Every alert past the cap was then cleared as "disappeared from the feed". The comment directly above that loop said pagination existed precisely to stop this happening at page 1. The cap reintroduced it at page 10. Worse, the trigger is **feed size**, so it fires during large multi-state outbreaks — exactly when the dropped alerts matter most. **Fix.** Fetches return an `AlertFeed` carrying a `complete` flag, set False when pages remained at the cap. It subclasses `list`, so every existing caller and test works unchanged (`len`, indexing, iteration, `is None`). `process_alerts` still **dispatches** the alerts it did receive — they are real — but skips the clearing pass entirely, because absence from a partial feed proves nothing. The cap moves 10 → 50, far above any plausible legitimate feed for the handful of states one install monitors, so it reads as the runaway guard it is. Hitting it now logs at ERROR instead of passing silently. ## #147 — expiry swept on an unverifiable timestamp `expire_alerts_job` cleared purely on stored `expires`, with no feed cross-check — unlike `_clear_missing_alerts`, which also requires the alert to be absent from the live feed. That asymmetry is unsafe exactly when the poller is unhealthy. A failed fetch makes `poll_alerts_job` skip the cycle (#40), and an alert extended by NWS (VTEC `EXT`) has its `expires` refreshed *only* by that same poll. So the job acts unilaterally on data it cannot verify, at the moment that data is least trustworthy. **Fix.** Gate on poll freshness via a new `observability.latest_alert_poll_success` helper, reusing the existing `heartbeat_stale_minutes` threshold rather than adding a knob. No recent successful poll — **including none ever recorded**, which must read as "not fresh" rather than "fine" — means skip the sweep and log why, while still delivering all-clears already decided by a healthy path. ## Tests `tests/test_incomplete_feed_guards.py`, 8 tests, each guard with a **control** so it cannot silently stop legitimate all-clears: - `AlertFeed` completeness defaults - feed marked incomplete when pages remain at the cap, and pagination stops there - **control:** pagination ending naturally → `complete is True` - incomplete feed → clearing skipped, nothing cleared - **control:** same empty feed trusted → does clear - expiry sweep skipped when the last poll is stale - expiry sweep skipped when no poll has ever succeeded - **control:** fresh poll → does clear 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. ## Verification | Step | Result | |---|---| | `ruff check .` | passed | | `python -m compileall app` | OK | | SQLite bulk tier | 805 passed | | `alembic upgrade head` on fresh Postgres 16 | clean | | Postgres integration tier | 4 passed | ## Relationship to #145 #145 fixed supersession — a hazard continuing under a different product. These two fix incomplete evidence. All three end in the same symptom, by different routes; none subsumes another. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Don't clear alerts on incomplete information (#146, #147)
All checks were successful
CI / test (pull_request) Successful in 4m18s
00194ed6cd
Two independent routes to a false all-clear, both from treating "absent" as
"ended" when the evidence could not support it.

#146 -- fetch_alerts_for_states followed pagination.next but stopped at a hard
10-page cap and still returned an ordinary list, indistinguishable from a
complete fetch. Every alert past the cap was then cleared as "disappeared from
the feed". The comment above that loop said pagination existed precisely to
stop that happening at page 1; the cap reintroduced it at page 10. Worse, the
trigger is feed size, so it fires during large outbreaks -- exactly when the
dropped alerts matter most.

Fetches now return an AlertFeed (a list subclass, so every existing caller and
test is unaffected) carrying a `complete` flag, set False when pages remained
at the cap. process_alerts still dispatches the alerts it did receive -- they
are real -- but skips the clearing pass entirely, because absence from a
partial feed proves nothing. The cap itself moves 10 -> 50, far above any
plausible legitimate feed, so it reads as the runaway guard it is; hitting it
now logs at ERROR instead of passing silently.

#147 -- expire_alerts_job cleared purely on the stored expires timestamp with
no feed cross-check, unlike _clear_missing_alerts which also requires the
alert to be absent from the live feed. That is unsafe exactly when the poller
is unhealthy: a failed fetch makes poll_alerts_job skip the cycle, and an
alert extended by NWS has its expires refreshed only by that same poll, so the
job acts unilaterally on data it cannot verify.

It now gates on poll freshness via a new observability.latest_alert_poll_success
helper, using the existing heartbeat_stale_minutes threshold. No recent
successful poll -- including none ever recorded -- means skip the sweep and
log why, while still delivering all-clears already decided by a healthy path.

Both new suites carry controls (complete feed still clears; fresh poller still
expires) so the guards cannot silently stop legitimate all-clears.
test_expire_alerts_job_sends_lifted_once_across_repeated_runs now seeds a
healthy poll record: its subject is lifted-notification dedup, not freshness.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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!150
No description provided.