feat(notifications): improve AFD change summary with section extraction#453
Merged
feat(notifications): improve AFD change summary with section extraction#453
Conversation
…ate notifications fire The notification event path was calling _get_nws_forecast_and_discussion, which wrapped forecast + discussion in a single try/except. A transient forecast HTTP error (e.g. 503) caused the whole function to return (None, None, None), silently dropping discussion_issuance_time and preventing AFD update notifications from ever firing. Changes: - weather_client_nws.py: extract forecast fetch into its own try/except inside get_nws_forecast_and_discussion so discussion still returns on forecast failure. Add new get_nws_discussion_only() that fetches grid data + AFD without touching the forecast endpoint at all. - weather_client_base.py: get_notification_event_data now calls _get_nws_discussion_only instead of _get_nws_forecast_and_discussion, removing an unnecessary forecast fetch from the 60-second poll path and eliminating the forecast-failure silent suppression entirely. Added debug logging for issuance_time and discussion status at each check. - notification_event_manager.py: add debug log lines in _check_discussion_update for every early-return path (no issuance_time, first-run, unchanged, and updated) so failures are visible at DEBUG level. - tests: add tests/test_nws_afd_notification.py with 9 tests covering forecast failure isolation, discussion-only happy path, no-forecast-call assertion, and get_notification_event_data integration. Update test_split_notification_timers.py to mock _get_nws_discussion_only. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…scussion and get_nws_discussion_only Covers the previously uncovered `client=None` code paths in both NWS functions (lines 636-643, 650, 657, 709-714, 717, 722, 727) and the `_get_nws_discussion_only` delegation method in weather_client_base (line 1066). diff-cover now reports 100% on all changed lines. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace Linux-only `%-I` format specifier with `%I` + lstrip("0")
to avoid "Invalid format string" errors on Windows.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extract .WHAT HAS CHANGED... (priority 1) or .KEY MESSAGES... (priority 2) sections from NWS AFD text before falling back to the first new line. Truncation limit raised from 160 to 300 characters. Tests added covering all three paths and the priority ordering. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improves
summarize_discussion_changeto extract meaningful content from AFD updates:.WHAT HAS CHANGED...section (NWS includes this in many AFDs).KEY MESSAGES...sectionSummary truncated to 300 chars for toast readability.
Follow-up to #452.