feat: Nationwide Weather Discussions with AI Summarization #293
Conversation
…fetching (PMD, SWO, QPF)
…when Nationwide is selected
Orinks
left a comment
Orinks
left a comment
There was a problem hiding this comment.
Review Notes
🔴 Remove from the PR
progress.txt is a development artifact (agent progress log) that shouldn't be committed to the repository. Please remove it from the branch and add it to .gitignore if needed.
🟡 Duplicate CPC scraping code
CPC scraping methods were added to both national_discussion_scraper.py AND the new national_discussion_service.py. Since NationalForecastHandler now uses NationalDiscussionService (not the scraper), the CPC additions to the scraper appear orphaned. Consider removing the CPC methods from the scraper to avoid maintaining duplicate code.
🟡 New httpx.Client per request
Both _make_request and _make_html_request create a new httpx.Client() for every single HTTP call. Since fetch_all_discussions makes many sequential requests, reusing a single client (or using self._client) would be more efficient and allow connection pooling.
🟡 Near-identical _make_request / _make_html_request
These two methods are ~90% identical — the only difference is .json() vs .text and the headers. Consider consolidating into one method with a parameter for response type.
✅ What looks good
- Clean tabbed dialog structure with proper accessibility labels (
name=on all controls) - Good test coverage across service, dialog, and routing
- Hurricane season gating for NHC tab is a nice touch
- Caching with configurable TTL and force_refresh
- Routing logic in main_window is clean
- Background threading for data loading with wx.CallAfter
Overall the feature is well-structured. The progress.txt needs to go, and the scraper duplication should be cleaned up.
Orinks
left a comment
Orinks
left a comment
There was a problem hiding this comment.
Review Notes
🔴 Remove progress.txt from the PR
progress.txt is a development artifact (agent progress log) that should not be committed to the repository. Please remove it from the branch and add it to .gitignore if needed.
🟡 Duplicate CPC scraping code
CPC scraping methods were added to both national_discussion_scraper.py AND the new national_discussion_service.py. Since NationalForecastHandler now uses NationalDiscussionService (not the scraper), the CPC additions to the scraper appear orphaned. Consider removing the CPC methods from the scraper to avoid maintaining duplicate code.
🟡 New httpx.Client per request
Both _make_request and _make_html_request create a new httpx.Client() for every single HTTP call. Since fetch_all_discussions makes many sequential requests, reusing a single client would be more efficient and allow connection pooling.
🟡 Near-identical _make_request / _make_html_request
These two methods are ~90% identical. Consider consolidating into one method with a parameter for response type.
✅ What looks good
- Clean tabbed dialog structure with proper accessibility labels (name= on all controls)
- Good test coverage across service, dialog, and routing
- Hurricane season gating for NHC tab is a nice touch
- Caching with configurable TTL and force_refresh
- Background threading for data loading with wx.CallAfter
- Routing logic in main_window is clean
Overall the feature is well-structured. The progress.txt needs to go, and the scraper duplication should be cleaned up.
Development artifact from Antfarm workflow. Also noted: CPC scraping is duplicated between national_discussion_service.py and the existing national_discussion_scraper.py — consolidation deferred to follow-up.
…summaries - Add 'Show Nationwide location' checkbox to General tab in settings dialog - Save/load nationwide visibility via LocationManager.set_show_nationwide() - When Nationwide is selected, fetch and display national discussion summaries instead of regular weather data - Current conditions shows Short Range Forecast + SPC Day 1 Outlook - Forecast field shows Extended Forecast + CPC 6-10 and 8-14 Day Outlooks - Regular location weather display remains unchanged
Cover missing lines in _make_request, _make_html_request, _fetch_latest_product, _fetch_product_text, classify methods, fetch_wpc/spc/qpf/nhc/cpc_discussions, _extract_nhc_outlook_text, _extract_cpc_outlook_text, and services __init__.py lazy imports. Targets 80%+ diff-cover on changed lines.
- Add show_nationwide_location field to AppSettings model - Fix settings_dialog to read from config settings instead of non-existent config_manager.location_manager - Simplify Nationwide detection in main_window to direct name check
get_all_locations now checks show_nationwide_location setting and injects or filters the Nationwide location accordingly. Previously the setting was saved but never acted upon.
- set_current_location now handles Nationwide (dynamically injected, not in config.locations) by creating the Location inline - Nationwide discussions only fetch with auto/nws data source; shows helpful error message for other sources - Clears stale weather data when switching to Nationwide
- get_all_locations filters out Nationwide for non-NWS sources - Settings checkbox disabled and unchecked for incompatible sources - Removed error message path since Nationwide won't appear
NWS API returns generic productName for all PMD/SWO products. Classification now uses WMO header codes from the product text (PMDSPD=short_range, PMDEPD=medium_range, PMDET*=extended, SWODY1/2/3=day1/2/3). Fetch text first, then classify. Falls back to keyword matching for compatibility.
- Tab labels now show full names: WPC (Weather Prediction Center), etc. - Show informative loading message while fetching discussions - Clear stale data immediately when switching to Nationwide
Tabs are now hidden when all their discussions are empty, unavailable, or errored. Replaces the hurricane-season-only NHC check with a general approach for all tabs.
NHC tab and its controls are now only created during hurricane season. All loading/error handlers guard NHC references behind the _nhc_available flag.
CPC index pages are map galleries with no discussion text. The actual prognostic discussions live at fxus06.html which covers both 6-10 and 8-14 day outlooks in a single document.
Both the main window summary and the discussions dialog now use a single cached service instance. The service has a 1-hour cache TTL, so opening the dialog after viewing the summary reuses the already-fetched data instantly.
The CPC 6-10 and 8-14 day discussions are a single document (fxus06.html). Replaced two duplicate fields with one unified 'CPC 6-10 & 8-14 Day Outlook' field in both the dialog and main window summary. Eliminates redundant HTTP request.
Adds 'Summarize with AI' button that sends the currently selected tab's discussion text to AIExplainer for a plain language summary. Uses the same AI infrastructure as the regular discussion dialog. Button is disabled when no OpenRouter API key is configured.
AIExplainer uses async explain_afd(), not sync explain_discussion(). Switch from thread to app.run_async() and use result.text from the response object.
- Config manager tests account for Nationwide being auto-injected - Discussion service tests use WMO header codes in mock text - CPC tests use consolidated 'outlook' key instead of two separate keys - Classify tests cover both WMO codes and keyword fallbacks
Orinks
changed the title
1 participant
Summary
Adds a Nationwide pseudo-location that provides access to national weather discussions from multiple NWS centers, with optional AI-powered summarization.
Closes #292
Features
Technical Details
fxus06.html(consolidated 6-10 & 8-14 day outlook)get_all_locations()dynamically injects/filters Nationwide based on setting + data sourceKey Files
src/accessiweather/services/national_discussion_service.py— new servicesrc/accessiweather/ui/dialogs/nationwide_discussion_dialog.py— new dialogsrc/accessiweather/ui/main_window.py— Nationwide routing + discussion summariessrc/accessiweather/config/locations.py— Nationwide injection/filteringsrc/accessiweather/ui/dialogs/settings_dialog.py— settings checkboxsrc/accessiweather/models/config.py—show_nationwide_locationfieldtests/test_national_discussion_service.py— 96 tests for service layertests/test_config_manager.py— updated for Nationwide injection