From 0360186d24bf2acc9c5908f37d1ded07103f38e6 Mon Sep 17 00:00:00 2001 From: JacobSampson Date: Mon, 8 Jun 2026 16:31:18 -0500 Subject: [PATCH] fix: resolve AttributeError and inaccurate lake coords in fishing app Bug 1 (APR-234): fishing_data._generate_plan_cached reconstructed WeatherResult from JSON with pressure_trend as a plain string, not the PressureTrend enum. plan_generator._build_conditions_summary then called .value on the string, raising AttributeError. Fix: after JSON deserialization, convert the string back to the enum and re-derive a PressureInterpretation so the conditions summary is complete. Bug 2 (APR-234): depth-contour dots on the lake map used hardcoded WRIGHT_COUNTY_LAKES coordinates that may not match the actual physical lake position. Fix: add a lake_coord_overrides parameter to build_lake_map; when a DNR API search result resolves a lake that already exists in WRIGHT_COUNTY_LAKES, fishing.py now extracts the authoritative DNR coordinates and passes them as overrides so the dot moves to the correct position. Adds regression tests covering all PressureTrend variants for the JSON round-trip and four tests for the coordinate-override path. Co-Authored-By: Claude Sonnet 4.6 --- app/components/dnr_map.py | 3 ++ app/components/fishing_data.py | 20 ++++++- app/pages/fishing.py | 9 ++++ tests/test_dnr_map_bathymetry.py | 39 ++++++++++++++ tests/test_plan_generator.py | 93 ++++++++++++++++++++++++++++++++ 5 files changed, 162 insertions(+), 2 deletions(-) diff --git a/app/components/dnr_map.py b/app/components/dnr_map.py index aced6da..a132f21 100644 --- a/app/components/dnr_map.py +++ b/app/components/dnr_map.py @@ -161,6 +161,7 @@ def build_lake_map( show_bathymetry: bool = False, species_zones: Optional[List[Dict[str, Any]]] = None, bathymetry_dir: Optional[Path] = None, + lake_coord_overrides: Optional[Dict[str, Tuple[float, float]]] = None, ) -> folium.Map: m = folium.Map( location=center, @@ -169,6 +170,8 @@ def build_lake_map( ) for name, (lat, lon, dow) in WRIGHT_COUNTY_LAKES.items(): + if lake_coord_overrides and name in lake_coord_overrides: + lat, lon = lake_coord_overrides[name] is_selected = name == selected_lake folium.CircleMarker( location=(lat, lon), diff --git a/app/components/fishing_data.py b/app/components/fishing_data.py index 48c11e4..5b2ed87 100644 --- a/app/components/fishing_data.py +++ b/app/components/fishing_data.py @@ -325,9 +325,25 @@ def _generate_plan_cached( ): import json as _json from onkia.plan_generator import generate_evening_plan as _gp - from onkia.weather import WeatherResult as _WR + from onkia.weather import WeatherResult as _WR, PressureTrend as _PT, PressureInterpretation as _PI from onkia.models import WaterTempPreference as _WTP - weather = _WR(**_json.loads(_weather_json)) + + _TREND_DEFAULTS = { + _PT.FALLING: ("Falling rapidly", "Fish feed aggressively before a front — best bite window!"), + _PT.RISING: ("Rising", "Fish may be less active — try deeper water and slower presentations."), + _PT.RISING_AFTER_DROP: ("Rising after a drop", "Fish resume feeding after pressure stabilizes — good action."), + _PT.HIGH_STEADY: ("High and steady", "High pressure = slower fishing. Focus on deep structure and live bait."), + _PT.STABLE: ("Stable", "Normal fish activity expected."), + } + + raw = _json.loads(_weather_json) + if raw.get("pressure_trend"): + trend = _PT(raw["pressure_trend"]) + raw["pressure_trend"] = trend + label, note = _TREND_DEFAULTS.get(trend, (trend.value, "")) + raw["pressure_interpretation"] = _PI(trend=trend, label=label, fishing_note=note) + + weather = _WR(**raw) pref_objs = [_WTP(**p) for p in _json.loads(_prefs_json)] survey = _json.loads(_survey_json) if _survey_json else None return _gp(weather, water_temp_f, pref_objs, survey, wind_dir_deg, start_hour, end_hour) diff --git a/app/pages/fishing.py b/app/pages/fishing.py index afb00a8..edf8409 100644 --- a/app/pages/fishing.py +++ b/app/pages/fishing.py @@ -316,11 +316,20 @@ def _build_depth_temp_chart( "color": SPECIES_COLORS.get(sp, "#457b9d"), }) +_dnr_coord_overrides = {} +for _r in st.session_state["dnr_search_results"]: + _rname = _r.get("name", "") + if _rname in WRIGHT_COUNTY_LAKES: + _coords = _r.get("point", {}).get("epsg:4326", []) + if len(_coords) >= 2: + _dnr_coord_overrides[_rname] = (float(_coords[1]), float(_coords[0])) + fmap = build_lake_map( selected_lake=st.session_state["selected_lake_name"], search_results=st.session_state["dnr_search_results"], show_bathymetry=show_bathymetry, species_zones=species_zones if species_zones else None, + lake_coord_overrides=_dnr_coord_overrides if _dnr_coord_overrides else None, ) map_data = st_folium(fmap, width="100%", height=400, returned_objects=["last_object_clicked"]) diff --git a/tests/test_dnr_map_bathymetry.py b/tests/test_dnr_map_bathymetry.py index 9b01906..ee46586 100644 --- a/tests/test_dnr_map_bathymetry.py +++ b/tests/test_dnr_map_bathymetry.py @@ -123,3 +123,42 @@ def test_lake_markers_still_present(self): m = build_lake_map() html = m._repr_html_() assert "Clearwater" in html or "Wright" in html + + +class TestLakeCoordOverrides: + def test_override_replaces_hardcoded_coordinates(self): + """Coordinate override must appear in the rendered HTML; hardcoded default must not.""" + # Clearwater hardcoded: (45.3052, -94.1184) + override_lat, override_lon = 45.9999, -94.7777 + m = build_lake_map( + lake_coord_overrides={"Clearwater": (override_lat, override_lon)}, + ) + html = m._repr_html_() + assert str(override_lat) in html + assert str(override_lon) in html + # The default hardcoded value should NOT appear (it was replaced) + assert "45.3052" not in html + + def test_non_overridden_lakes_keep_hardcoded_coords(self): + """Lakes without an override must still use their hardcoded coordinates.""" + # Clearwater (overridden) and Howard Lake (not overridden, lat=45.0618) + m = build_lake_map( + lake_coord_overrides={"Clearwater": (45.9999, -94.7777)}, + ) + html = m._repr_html_() + assert "45.0618" in html + + def test_none_override_uses_hardcoded_coords(self): + """Passing lake_coord_overrides=None must not replace any hardcoded coordinates.""" + m = build_lake_map(lake_coord_overrides=None) + html = m._repr_html_() + # Clearwater hardcoded coordinates must still appear + assert "45.3052" in html + assert "-94.1184" in html + + def test_empty_override_dict_uses_hardcoded_coords(self): + """An empty dict override must leave all hardcoded coords unchanged.""" + m = build_lake_map(lake_coord_overrides={}) + html = m._repr_html_() + assert "45.3052" in html + assert "-94.1184" in html diff --git a/tests/test_plan_generator.py b/tests/test_plan_generator.py index 22dd43b..831d38c 100644 --- a/tests/test_plan_generator.py +++ b/tests/test_plan_generator.py @@ -1,6 +1,7 @@ """Tests for onkia.plan_generator — evening fishing plan generator.""" from __future__ import annotations +import json from typing import Dict, List, Optional import pytest @@ -486,3 +487,95 @@ def test_cold_water_changes_plan(self): techniques_warm = [b.technique for b in plan_warm.blocks] techniques_cold = [b.technique for b in plan_cold.blocks] assert techniques_warm != techniques_cold or warm_species != cold_species + + +def _serialize_weather_like_fishing_page(weather: WeatherResult) -> str: + """Reproduce the JSON serialization performed by fishing.py before calling the + Streamlit-cached plan generator. pressure_trend is stored as its .value string + and pressure_interpretation is omitted entirely.""" + return json.dumps({ + "air_temp_f": weather.air_temp_f, + "pressure_hpa": weather.pressure_hpa, + "pressure_inhg": weather.pressure_inhg, + "wind_speed_mph": weather.wind_speed_mph, + "wind_direction_deg": weather.wind_direction_deg, + "cloud_cover_pct": weather.cloud_cover_pct, + "pressure_trend": weather.pressure_trend.value if weather.pressure_trend else None, + "source": weather.source, + "fallback_used": weather.fallback_used, + }) + + +def _deserialize_weather_like_cache_layer(weather_json: str) -> WeatherResult: + """Reproduce the fixed deserialization in fishing_data._generate_plan_cached.""" + raw = json.loads(weather_json) + if raw.get("pressure_trend"): + trend = PressureTrend(raw["pressure_trend"]) + raw["pressure_trend"] = trend + _TREND_DEFAULTS = { + PressureTrend.FALLING: ("Falling rapidly", "Fish feed aggressively before a front — best bite window!"), + PressureTrend.RISING: ("Rising", "Fish may be less active — try deeper water and slower presentations."), + PressureTrend.RISING_AFTER_DROP: ("Rising after a drop", "Fish resume feeding after pressure stabilizes — good action."), + PressureTrend.HIGH_STEADY: ("High and steady", "High pressure = slower fishing. Focus on deep structure and live bait."), + PressureTrend.STABLE: ("Stable", "Normal fish activity expected."), + } + label, note = _TREND_DEFAULTS.get(trend, (trend.value, "")) + raw["pressure_interpretation"] = PressureInterpretation(trend=trend, label=label, fishing_note=note) + return WeatherResult(**raw) + + +class TestWeatherJsonRoundTrip: + """Regression tests for the Streamlit cache JSON round-trip bug. + + Before the fix, fishing.py serialised pressure_trend as a plain string and + _generate_plan_cached reconstructed WeatherResult without converting it back + to the PressureTrend enum. plan_generator._build_conditions_summary then + called pressure_trend.value on the string, raising AttributeError. + """ + + @pytest.mark.parametrize("trend", list(PressureTrend)) + def test_round_trip_preserves_enum(self, trend): + weather = _make_weather(pressure_trend=trend) + recovered = _deserialize_weather_like_cache_layer( + _serialize_weather_like_fishing_page(weather) + ) + assert recovered.pressure_trend is trend + + @pytest.mark.parametrize("trend", list(PressureTrend)) + def test_round_trip_preserves_pressure_interpretation(self, trend): + weather = _make_weather(pressure_trend=trend) + recovered = _deserialize_weather_like_cache_layer( + _serialize_weather_like_fishing_page(weather) + ) + assert recovered.pressure_interpretation is not None + assert recovered.pressure_interpretation.trend is trend + assert recovered.pressure_interpretation.label + + @pytest.mark.parametrize("trend", list(PressureTrend)) + def test_generate_plan_does_not_raise_with_round_tripped_weather(self, trend): + """The AttributeError must not be raised after the fix.""" + weather = _make_weather(air_temp_f=68.0, pressure_trend=trend) + recovered = _deserialize_weather_like_cache_layer( + _serialize_weather_like_fishing_page(weather) + ) + plan = generate_evening_plan( + weather=recovered, + water_temp_f=68.0, + target_species_prefs=_TARGET_PREFS, + ) + assert plan.conditions_summary + assert trend.value in plan.conditions_summary + + def test_none_pressure_trend_round_trips_safely(self): + weather = _make_weather(pressure_trend=None) + recovered = _deserialize_weather_like_cache_layer( + _serialize_weather_like_fishing_page(weather) + ) + assert recovered.pressure_trend is None + assert recovered.pressure_interpretation is None + plan = generate_evening_plan( + weather=recovered, + water_temp_f=68.0, + target_species_prefs=_TARGET_PREFS, + ) + assert plan.conditions_summary