From c33cab70207683ddbf352777c6953c85a307b02e Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 29 Apr 2026 20:37:23 -0400 Subject: [PATCH] fix(journal): wire weather refresh on config save; drop orphaned cache rows Two related gaps in the journal weather panel: 1. Saving locations via PUT /journal/config didn't trigger a weather fetch, so newly-entered sites had no cache row (or a stale one) until the user manually clicked the panel's refresh button. The panel rendered "two sites with empty values" against pre-existing cache rows that no longer matched what the user had configured. 2. get_cached_weather_rows returned every WeatherCache row for the user regardless of whether the location was still in journal_config. Briefing-era rows survived migration 0040 (which only deleted the briefing_config setting, not the cache table) and showed up as ghost tabs in the UI. Changes: - get_cached_weather_rows accepts an optional valid_keys filter; rows whose location_key is not in the set are excluded. - routes/journal.py: - put_config kicks off a background refresh_location_cache for any saved location with valid lat/lon. - GET /weather and POST /weather/refresh both pass valid_keys derived from the current config so orphaned rows don't surface. - services/journal_prep.py filters the weather section to currently- configured locations as well; uses a lazy import of get_journal_config to avoid a cycle (journal_scheduler imports journal_prep). 153 tests pass. Co-Authored-By: Claude Opus 4.7 --- src/fabledassistant/routes/journal.py | 50 +++++++++++++++++++- src/fabledassistant/services/journal_prep.py | 10 +++- src/fabledassistant/services/weather.py | 21 ++++++-- 3 files changed, 73 insertions(+), 8 deletions(-) diff --git a/src/fabledassistant/routes/journal.py b/src/fabledassistant/routes/journal.py index 8c4a6b5..462e840 100644 --- a/src/fabledassistant/routes/journal.py +++ b/src/fabledassistant/routes/journal.py @@ -66,6 +66,17 @@ async def _resolve_config(user_id: int) -> dict: return {**DEFAULT_JOURNAL_CONFIG, **config} +def _valid_location_keys(cfg: dict) -> set[str]: + """Keys in ``cfg.locations`` that have a usable lat/lon. Anything else + (orphaned cache rows, locations the user typed but didn't geocode) is + excluded so it can't render as a fake site in the UI.""" + locations = cfg.get("locations") or {} + return { + key for key, loc in locations.items() + if isinstance(loc, dict) and loc.get("lat") is not None and loc.get("lon") is not None + } + + @journal_bp.get("/config") @login_required async def get_config(): @@ -82,9 +93,41 @@ async def put_config(): return jsonify({"error": "config must be an object"}), 400 await set_setting(user_id, "journal_config", json.dumps(body)) await update_user_schedule(user_id) + + # Trigger a background weather refresh for any newly-saved location with + # valid lat/lon. Without this, the cache row for the location doesn't + # exist (or stays stale) until the user clicks the manual refresh button, + # so the journal weather panel renders empty for newly-entered sites. + valid_locs = [ + (key, loc) + for key, loc in (body.get("locations") or {}).items() + if isinstance(loc, dict) and loc.get("lat") is not None and loc.get("lon") is not None + ] + if valid_locs: + asyncio.create_task(_refresh_locations_in_background(user_id, valid_locs)) + return jsonify({"ok": True}) +async def _refresh_locations_in_background( + user_id: int, locations: list[tuple[str, dict]] +) -> None: + for key, loc in locations: + try: + await weather_svc.refresh_location_cache( + user_id=user_id, + location_key=key, + location_label=loc.get("label", key), + lat=loc["lat"], + lon=loc["lon"], + ) + except Exception: + logger.warning( + "Post-save weather refresh failed for user %d / %s", + user_id, key, exc_info=True, + ) + + @journal_bp.get("/today") @login_required async def get_today(): @@ -258,7 +301,9 @@ async def _refresh_stale_in_background(user_id: int, stale_keys: set[str]) -> No @login_required async def get_weather(): user_id = get_current_user_id() - rows = await weather_svc.get_cached_weather_rows(user_id) + cfg = await _resolve_config(user_id) + valid_keys = _valid_location_keys(cfg) + rows = await weather_svc.get_cached_weather_rows(user_id, valid_keys) temp_unit = await _journal_temp_unit(user_id) # Kick off a best-effort background refresh for stale rows so the next page @@ -318,7 +363,8 @@ async def refresh_weather(): ) except Exception: logger.warning("Failed to refresh weather for %s", key, exc_info=True) - rows = await weather_svc.get_cached_weather_rows(user_id) + valid_keys = _valid_location_keys(cfg) + rows = await weather_svc.get_cached_weather_rows(user_id, valid_keys) cards = [ card for row in rows if (card := weather_svc.parse_weather_card_data(row, temp_unit)) is not None diff --git a/src/fabledassistant/services/journal_prep.py b/src/fabledassistant/services/journal_prep.py index 73734ac..f1d9dcf 100644 --- a/src/fabledassistant/services/journal_prep.py +++ b/src/fabledassistant/services/journal_prep.py @@ -178,7 +178,15 @@ async def gather_daily_sections( sections["events"] = [] try: - weather_rows = await get_cached_weather_rows(user_id) + # Lazy import: journal_scheduler imports this module for prep generation, + # so a top-level import would cycle. + from fabledassistant.services.journal_scheduler import get_journal_config + cfg = await get_journal_config(user_id) + valid_weather_keys = { + key for key, loc in (cfg.get("locations") or {}).items() + if isinstance(loc, dict) and loc.get("lat") is not None and loc.get("lon") is not None + } + weather_rows = await get_cached_weather_rows(user_id, valid_weather_keys) sections["weather"] = [w.to_dict() for w in weather_rows] except Exception: logger.exception("daily_prep weather section failed for user %d", user_id) diff --git a/src/fabledassistant/services/weather.py b/src/fabledassistant/services/weather.py index 34f7eb3..46051a1 100644 --- a/src/fabledassistant/services/weather.py +++ b/src/fabledassistant/services/weather.py @@ -232,12 +232,23 @@ def parse_weather_card_data( } -async def get_cached_weather_rows(user_id: int) -> list: - """Return raw WeatherCache ORM rows for a user (for card parsing).""" +async def get_cached_weather_rows( + user_id: int, + valid_keys: set[str] | None = None, +) -> list: + """Return raw WeatherCache ORM rows for a user (for card parsing). + + If ``valid_keys`` is provided, only rows whose ``location_key`` is in the + set are returned. This is how callers drop orphaned cache rows whose + location is no longer in the user's ``journal_config.locations`` (e.g. + leftovers from the briefing era, or a location that's been removed). + Passing an empty set returns no rows. + """ async with async_session() as session: - result = await session.execute( - select(WeatherCache).where(WeatherCache.user_id == user_id) - ) + stmt = select(WeatherCache).where(WeatherCache.user_id == user_id) + if valid_keys is not None: + stmt = stmt.where(WeatherCache.location_key.in_(list(valid_keys))) + result = await session.execute(stmt) return list(result.scalars().all())