No reviewers
Labels
No labels
area:ai
area:ci-cd
area:notifications
area:observability
area:public-pages
backlog
bug
duplicate
enhancement
help wanted
invalid
question
type:decision
type:feature
type:infra
type:maintenance
type:security
v1.0.1
v1.1.0
v1.2.0
v1.3.0
v2.0.0
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
rbrooks/WeatherBot!150
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/incomplete-feed-guards"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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_statesfollowedpagination.nextbut 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
AlertFeedcarrying acompleteflag, set False when pages remained at the cap. It subclasseslist, so every existing caller and test works unchanged (len, indexing, iteration,is None).process_alertsstill 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_jobcleared purely on storedexpires, 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_jobskip the cycle (#40), and an alert extended by NWS (VTECEXT) has itsexpiresrefreshed 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_successhelper, reusing the existingheartbeat_stale_minutesthreshold 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:AlertFeedcompleteness defaultscomplete is TrueOne existing test changed:
test_expire_alerts_job_sends_lifted_once_across_repeated_runsnow seeds a healthy poll record. Its subject is lifted-notification dedup, not freshness, so the precondition is declared rather than the assertion weakened.Verification
ruff check .python -m compileall appalembic upgrade headon fresh Postgres 16Relationship 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