diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d3c8be2..a0b9bf95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ All notable changes to this project will be documented in this file. - The tray text format dialog now warns you about unknown placeholders or unbalanced braces before saving, so a typo like `{tmp}` can't quietly end up in your tray tooltip. - Closing the Add or Edit Location dialog while a search or current-location detection is still running no longer risks a crash. - Weather history's "Today vs Yesterday" section now reads naturally when both days have the same temperature, instead of "is 0.0°F about the same than yesterday". +- Explain Weather now uses your effective unit preference, including automatic location-based units like Canadian kPa, instead of always prompting the AI with imperial current-condition values. ## [0.7.2] - 2026-05-30 diff --git a/src/accessiweather/ai_explainer_models.py b/src/accessiweather/ai_explainer_models.py index 09c83a5e..143ab973 100644 --- a/src/accessiweather/ai_explainer_models.py +++ b/src/accessiweather/ai_explainer_models.py @@ -83,6 +83,13 @@ class WeatherContext: pressure: float | None alerts: list[dict[str, Any]] forecast_summary: str | None = None + temperature_text: str | None = None + wind_speed_unit: str | None = None + wind_text: str | None = None + visibility_unit: str | None = None + visibility_text: str | None = None + pressure_unit: str | None = None + pressure_text: str | None = None local_time: str | None = None utc_time: str | None = None timezone: str | None = None @@ -100,7 +107,9 @@ def to_prompt_text(self) -> str: if self.time_of_day: parts.append(f"Time of Day: {self.time_of_day}") - if self.temperature is not None: + if self.temperature_text: + parts.append(f"Temperature: {self.temperature_text}") + elif self.temperature is not None: parts.append(f"Temperature: {self.temperature}°{self.temperature_unit}") if self.conditions: @@ -109,17 +118,29 @@ def to_prompt_text(self) -> str: if self.humidity is not None: parts.append(f"Humidity: {self.humidity}%") - if self.wind_speed is not None: - wind_info = f"Wind: {self.wind_speed} mph" + if self.wind_text: + wind_info = f"Wind: {self.wind_text}" + if self.wind_direction: + wind_info += f" from {self.wind_direction}" + parts.append(wind_info) + elif self.wind_speed is not None: + wind_unit = self.wind_speed_unit or "mph" + wind_info = f"Wind: {self.wind_speed} {wind_unit}" if self.wind_direction: wind_info += f" from {self.wind_direction}" parts.append(wind_info) - if self.visibility is not None: - parts.append(f"Visibility: {self.visibility} miles") - - if self.pressure is not None: - parts.append(f"Pressure: {self.pressure} inHg") + if self.visibility_text: + parts.append(f"Visibility: {self.visibility_text}") + elif self.visibility is not None: + visibility_unit = self.visibility_unit or "miles" + parts.append(f"Visibility: {self.visibility} {visibility_unit}") + + if self.pressure_text: + parts.append(f"Pressure: {self.pressure_text}") + elif self.pressure is not None: + pressure_unit = self.pressure_unit or "inHg" + parts.append(f"Pressure: {self.pressure} {pressure_unit}") if self.alerts: alert_texts = [] diff --git a/src/accessiweather/ai_explainer_prompting.py b/src/accessiweather/ai_explainer_prompting.py index 0058f4df..64b2a472 100644 --- a/src/accessiweather/ai_explainer_prompting.py +++ b/src/accessiweather/ai_explainer_prompting.py @@ -170,6 +170,13 @@ def _build_prompt( pressure=weather_data.get("pressure"), alerts=weather_data.get("alerts", []), forecast_summary=weather_data.get("forecast_summary"), + temperature_text=weather_data.get("temperature_text"), + wind_speed_unit=weather_data.get("wind_speed_unit"), + wind_text=weather_data.get("wind_text"), + visibility_unit=weather_data.get("visibility_unit"), + visibility_text=weather_data.get("visibility_text"), + pressure_unit=weather_data.get("pressure_unit"), + pressure_text=weather_data.get("pressure_text"), local_time=weather_data.get("local_time"), utc_time=weather_data.get("utc_time"), timezone=weather_data.get("timezone"), @@ -258,7 +265,11 @@ def _generate_cache_key(self, weather_data: dict[str, Any], location_name: str) key_parts = [ f"loc:{location_name}", f"temp:{weather_data.get('temperature')}", + f"temp_text:{weather_data.get('temperature_text')}", f"cond:{weather_data.get('conditions')}", + f"wind:{weather_data.get('wind_text') or weather_data.get('wind_speed')}", + f"visibility:{weather_data.get('visibility_text') or weather_data.get('visibility')}", + f"pressure:{weather_data.get('pressure_text') or weather_data.get('pressure')}", f"model:{self.get_effective_model()}", ] return "ai_explanation:" + ":".join(key_parts) diff --git a/src/accessiweather/ui/dialogs/explanation_dialog.py b/src/accessiweather/ui/dialogs/explanation_dialog.py index 4e90f743..3f9d6dc4 100644 --- a/src/accessiweather/ui/dialogs/explanation_dialog.py +++ b/src/accessiweather/ui/dialogs/explanation_dialog.py @@ -174,7 +174,13 @@ def _do_regenerate(): wx.CallAfter(self._on_regenerate_error, "No weather data available.") return - weather_dict = build_current_weather_payload(weather_data) + weather_dict = build_current_weather_payload( + weather_data, + temperature_unit_preference=getattr(settings, "temperature_unit", "both"), + location=location, + ) + if location: + add_location_time_context(weather_dict, location) loop = asyncio.new_event_loop() try: @@ -382,7 +388,11 @@ def generate_explanation(): ) # Build weather data dict from current conditions - weather_dict = build_current_weather_payload(weather_data) + weather_dict = build_current_weather_payload( + weather_data, + temperature_unit_preference=getattr(settings, "temperature_unit", "both"), + location=location, + ) # Add local time info for the location add_location_time_context(weather_dict, location) diff --git a/src/accessiweather/ui/dialogs/explanation_generation.py b/src/accessiweather/ui/dialogs/explanation_generation.py index c2e871dc..7360bdc7 100644 --- a/src/accessiweather/ui/dialogs/explanation_generation.py +++ b/src/accessiweather/ui/dialogs/explanation_generation.py @@ -8,6 +8,9 @@ from zoneinfo import ZoneInfo from ...ai_explainer import ExplanationStyle +from ...units import resolve_display_unit_system, resolve_temperature_unit_preference +from ...utils.temperature_utils import TemperatureUnit, format_temperature +from ...utils.unit_utils import format_pressure, format_visibility, format_wind_speed logger = logging.getLogger(__name__) @@ -29,18 +32,61 @@ def resolve_explanation_style(settings: Any) -> ExplanationStyle: return style_map.get(settings.ai_explanation_style, ExplanationStyle.STANDARD) -def build_current_weather_payload(weather_data: Any) -> dict[str, Any]: +def build_current_weather_payload( + weather_data: Any, + *, + temperature_unit_preference: str | None = "both", + location: Any | None = None, +) -> dict[str, Any]: """Build the AI explainer payload from current app weather data.""" current = weather_data.current + unit_pref = resolve_temperature_unit_preference(temperature_unit_preference, location) + unit_system = resolve_display_unit_system(temperature_unit_preference, location) + + temperature, temperature_unit = _resolve_temperature_for_prompt(current, unit_pref) weather_dict: dict[str, Any] = { - "temperature": current.temperature_f, - "temperature_unit": "F", + "temperature": temperature, + "temperature_unit": temperature_unit, + "temperature_text": _formatted_or_none( + format_temperature( + current.temperature_f, + unit=unit_pref, + temperature_c=current.temperature_c, + ) + ), "conditions": current.condition, "humidity": current.humidity, - "wind_speed": current.wind_speed_mph, + "wind_speed": _resolve_wind_speed_for_prompt(current, unit_pref, unit_system), + "wind_speed_unit": _resolve_wind_speed_unit_label(unit_pref, unit_system), + "wind_text": _formatted_or_none( + format_wind_speed( + current.wind_speed_mph, + unit=unit_pref, + wind_speed_kph=current.wind_speed_kph, + unit_system=unit_system, + ) + ), "wind_direction": current.wind_direction, - "visibility": current.visibility_miles, - "pressure": current.pressure_in, + "visibility": _resolve_visibility_for_prompt(current, unit_pref, unit_system), + "visibility_unit": _resolve_visibility_unit_label(unit_pref, unit_system), + "visibility_text": _formatted_or_none( + format_visibility( + current.visibility_miles, + unit=unit_pref, + visibility_km=current.visibility_km, + unit_system=unit_system, + ) + ), + "pressure": _resolve_pressure_for_prompt(current, unit_pref, unit_system), + "pressure_unit": _resolve_pressure_unit_label(unit_pref, unit_system), + "pressure_text": _formatted_or_none( + format_pressure( + current.pressure_in, + unit=unit_pref, + pressure_mb=current.pressure_mb, + unit_system=unit_system, + ) + ), "alerts": [], "forecast_periods": [], } @@ -67,6 +113,133 @@ def build_current_weather_payload(weather_data: Any) -> dict[str, Any]: return weather_dict +def _formatted_or_none(value: str) -> str | None: + return None if value == "N/A" else value + + +def _resolve_temperature_for_prompt( + current: Any, unit_pref: TemperatureUnit +) -> tuple[float | None, str]: + if current.temperature_f is None and current.temperature_c is not None: + temperature_f = (current.temperature_c * 9 / 5) + 32 + else: + temperature_f = current.temperature_f + + if current.temperature_c is None and current.temperature_f is not None: + temperature_c = (current.temperature_f - 32) * 5 / 9 + else: + temperature_c = current.temperature_c + + if unit_pref == TemperatureUnit.CELSIUS: + return temperature_c, "C" + if unit_pref == TemperatureUnit.BOTH: + return temperature_f, "F" + return temperature_f, "F" + + +def _resolve_wind_speed_for_prompt( + current: Any, unit_pref: TemperatureUnit, unit_system: Any | None +) -> float | None: + wind_speed_mph = current.wind_speed_mph + wind_speed_kph = current.wind_speed_kph + if wind_speed_mph is None and wind_speed_kph is not None: + wind_speed_mph = wind_speed_kph * 0.621371 + elif wind_speed_kph is None and wind_speed_mph is not None: + wind_speed_kph = wind_speed_mph * 1.60934 + + normalized_system = getattr(unit_system, "value", unit_system) + if normalized_system == "ca": + return wind_speed_kph + if normalized_system == "si": + return wind_speed_kph / 3.6 if wind_speed_kph is not None else None + if normalized_system in {"us", "uk"}: + return wind_speed_mph + if unit_pref == TemperatureUnit.CELSIUS: + return wind_speed_kph + return wind_speed_mph + + +def _resolve_wind_speed_unit_label(unit_pref: TemperatureUnit, unit_system: Any | None) -> str: + normalized_system = getattr(unit_system, "value", unit_system) + if normalized_system in {"us", "uk"}: + return "mph" + if normalized_system == "ca": + return "km/h" + if normalized_system == "si": + return "m/s" + if unit_pref == TemperatureUnit.CELSIUS: + return "km/h" + if unit_pref == TemperatureUnit.BOTH: + return "mph (km/h)" + return "mph" + + +def _resolve_visibility_for_prompt( + current: Any, unit_pref: TemperatureUnit, unit_system: Any | None +) -> float | None: + visibility_miles = current.visibility_miles + visibility_km = current.visibility_km + if visibility_miles is None and visibility_km is not None: + visibility_miles = visibility_km * 0.621371 + elif visibility_km is None and visibility_miles is not None: + visibility_km = visibility_miles * 1.60934 + + normalized_system = getattr(unit_system, "value", unit_system) + if normalized_system in {"us", "uk"}: + return visibility_miles + if normalized_system in {"ca", "si"}: + return visibility_km + if unit_pref == TemperatureUnit.CELSIUS: + return visibility_km + return visibility_miles + + +def _resolve_visibility_unit_label(unit_pref: TemperatureUnit, unit_system: Any | None) -> str: + normalized_system = getattr(unit_system, "value", unit_system) + if normalized_system in {"us", "uk"}: + return "mi" + if normalized_system in {"ca", "si"}: + return "km" + if unit_pref == TemperatureUnit.CELSIUS: + return "km" + if unit_pref == TemperatureUnit.BOTH: + return "mi (km)" + return "mi" + + +def _resolve_pressure_for_prompt( + current: Any, unit_pref: TemperatureUnit, unit_system: Any | None +) -> float | None: + pressure_in = current.pressure_in + pressure_mb = current.pressure_mb + if pressure_in is None and pressure_mb is not None: + pressure_in = pressure_mb / 33.8639 + elif pressure_mb is None and pressure_in is not None: + pressure_mb = pressure_in * 33.8639 + + normalized_system = getattr(unit_system, "value", unit_system) + if normalized_system == "ca": + return pressure_mb / 10 if pressure_mb is not None else None + if normalized_system in {"uk", "si"}: + return pressure_mb + if unit_pref == TemperatureUnit.CELSIUS: + return pressure_mb + return pressure_in + + +def _resolve_pressure_unit_label(unit_pref: TemperatureUnit, unit_system: Any | None) -> str: + normalized_system = getattr(unit_system, "value", unit_system) + if normalized_system == "ca": + return "kPa" + if normalized_system in {"uk", "si"}: + return "hPa" + if unit_pref == TemperatureUnit.CELSIUS: + return "hPa" + if unit_pref == TemperatureUnit.BOTH: + return "inHg (hPa)" + return "inHg" + + def add_location_time_context(weather_dict: dict[str, Any], location: Any) -> None: """Add UTC and local time context for the selected weather location.""" now_utc = datetime.now(UTC) diff --git a/tests/test_ai_explainer.py b/tests/test_ai_explainer.py index 31cb7775..396c73ec 100644 --- a/tests/test_ai_explainer.py +++ b/tests/test_ai_explainer.py @@ -290,6 +290,34 @@ def test_to_prompt_text_with_time_info(self): assert "America/New_York" in prompt_text assert "afternoon" in prompt_text + def test_to_prompt_text_prefers_preformatted_unit_strings(self): + """Prompt text should use the already-resolved display units when provided.""" + context = WeatherContext( + location="Toronto", + timestamp=datetime.now(UTC), + temperature=22.2, + temperature_unit="C", + conditions="Clear", + humidity=50, + wind_speed=16.1, + wind_direction="NW", + visibility=16.1, + pressure=101.8, + alerts=[], + temperature_text="22.2°C", + wind_text="16.1 km/h", + visibility_text="16.1 km", + pressure_text="101.76 kPa", + ) + prompt_text = context.to_prompt_text() + assert "Temperature: 22.2°C" in prompt_text + assert "Wind: 16.1 km/h from NW" in prompt_text + assert "Visibility: 16.1 km" in prompt_text + assert "Pressure: 101.76 kPa" in prompt_text + assert "mph" not in prompt_text + assert "miles" not in prompt_text + assert "inHg" not in prompt_text + # ============================================================================= # ExplanationStyle Tests diff --git a/tests/test_explanation_dialog.py b/tests/test_explanation_dialog.py index 4a7d3dd6..0b1e9b2b 100644 --- a/tests/test_explanation_dialog.py +++ b/tests/test_explanation_dialog.py @@ -12,6 +12,7 @@ import pytest from accessiweather.ai_explainer import ExplanationResult, ExplanationStyle +from accessiweather.ui.dialogs.explanation_generation import build_current_weather_payload # ============================================================================= # Fixtures @@ -59,6 +60,7 @@ def mock_app(): location_mock.latitude = 40.7128 location_mock.longitude = -74.0060 location_mock.timezone = "America/New_York" + location_mock.country_code = "US" app.config_manager.get_current_location.return_value = location_mock app.config_manager.get_settings.return_value = MagicMock( ai_model_preference="auto", @@ -66,16 +68,21 @@ def mock_app(): openrouter_api_key="test-api-key", custom_system_prompt=None, custom_instructions=None, + temperature_unit="fahrenheit", ) app.current_weather_data = MagicMock( current=MagicMock( temperature_f=72.0, + temperature_c=22.2, condition="Partly Cloudy", humidity=65, wind_speed_mph=10.0, + wind_speed_kph=16.1, wind_direction="NW", visibility_miles=10.0, + visibility_km=16.1, pressure_in=30.05, + pressure_mb=1017.6, ), forecast=MagicMock( periods=[ @@ -188,6 +195,46 @@ def test_build_weather_dict_with_forecast(self, mock_app): assert hasattr(first_period, "temperature") assert hasattr(first_period, "short_forecast") + def test_build_weather_payload_uses_metric_units_when_requested(self, mock_app): + """Current-condition prompt payload should honor an explicit metric preference.""" + weather_dict = build_current_weather_payload( + mock_app.current_weather_data, + temperature_unit_preference="celsius", + location=mock_app.config_manager.get_current_location(), + ) + + assert weather_dict["temperature"] == pytest.approx(22.2) + assert weather_dict["temperature_unit"] == "C" + assert weather_dict["temperature_text"] == "22.2°C" + assert weather_dict["wind_speed_unit"] == "km/h" + assert weather_dict["wind_text"] == "16.1 km/h" + assert weather_dict["visibility_unit"] == "km" + assert weather_dict["visibility_text"] == "16.1 km" + assert weather_dict["pressure_unit"] == "hPa" + assert weather_dict["pressure_text"] == "1017.60 hPa" + + def test_build_weather_payload_uses_auto_location_units(self, mock_app): + """Auto mode should resolve to the selected location's effective unit system.""" + location = mock_app.config_manager.get_current_location() + location.name = "Toronto, ON" + location.country_code = "CA" + + weather_dict = build_current_weather_payload( + mock_app.current_weather_data, + temperature_unit_preference="auto", + location=location, + ) + + assert weather_dict["temperature"] == pytest.approx(22.2) + assert weather_dict["temperature_unit"] == "C" + assert weather_dict["temperature_text"] == "22.2°C" + assert weather_dict["wind_speed_unit"] == "km/h" + assert weather_dict["wind_text"] == "16.1 km/h" + assert weather_dict["visibility_unit"] == "km" + assert weather_dict["visibility_text"] == "16.1 km" + assert weather_dict["pressure_unit"] == "kPa" + assert weather_dict["pressure_text"] == "101.76 kPa" + # ============================================================================= # Style Selection Tests