-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: introduce weather capability composition #71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| import pendulum | ||
| import pytest | ||
|
|
||
| from weather_briefing.air_quality import AirQualityError | ||
| from weather_briefing.capabilities import CapabilityName, CapabilityProviderSet, ProviderCapabilities | ||
| from weather_briefing.models import AirQualitySnapshot, AirQualityTimeKind, WeatherContextSnapshot | ||
| from weather_briefing.weather_context import WeatherContextError | ||
|
|
||
|
|
||
| def _weather(*, air_quality: AirQualitySnapshot | None = None) -> WeatherContextSnapshot: | ||
| return WeatherContextSnapshot( | ||
| source_id="weather:test", | ||
| source_name="Test weather", | ||
| source_url="https://example.invalid/weather", | ||
| observed_at=pendulum.datetime(2026, 7, 20, 8, tz="Asia/Singapore"), | ||
| weather_forecast=("forecast",), | ||
| air_quality=air_quality, | ||
| ) | ||
|
|
||
|
|
||
| def _air_quality() -> AirQualitySnapshot: | ||
| return AirQualitySnapshot( | ||
| source_id="air-quality:test", | ||
| source_name="Test air", | ||
| source_url="https://example.invalid/air", | ||
| effective_at=pendulum.datetime(2026, 7, 20, 8, tz="Asia/Singapore"), | ||
| time_kind=AirQualityTimeKind.OBSERVATION, | ||
| aqi=20, | ||
| aqi_display="20", | ||
| aqi_standard="Test", | ||
| pm25_aqi=None, | ||
| pm25_concentration=None, | ||
| pm25_unit=None, | ||
| category="good", | ||
| health_guidance="ok", | ||
| ) | ||
|
|
||
|
|
||
| def _metadata() -> ProviderCapabilities: | ||
| return ProviderCapabilities( | ||
| provider_id="test", | ||
| provider_name="Test", | ||
| capabilities=frozenset({CapabilityName.WEATHER}), | ||
| ) | ||
|
|
||
|
|
||
| def test_provider_capability_metadata_reports_support() -> None: | ||
| metadata = _metadata() | ||
|
|
||
| assert metadata.supports(CapabilityName.WEATHER) | ||
| assert not metadata.supports(CapabilityName.ALERTS) | ||
|
|
||
|
|
||
| async def test_capability_set_supplements_missing_current_air_quality() -> None: | ||
| class Weather: | ||
| async def fetch(self, latitude: float, longitude: float) -> WeatherContextSnapshot: | ||
| return _weather() | ||
|
|
||
| class Air: | ||
| async def fetch(self, latitude: float, longitude: float, timezone: str) -> AirQualitySnapshot: | ||
| assert timezone == "Asia/Singapore" | ||
| return _air_quality() | ||
|
|
||
| provider = CapabilityProviderSet( | ||
| weather=Weather(), | ||
| weather_metadata=_metadata(), | ||
| air_quality=Air(), | ||
| ) | ||
|
|
||
| snapshot = await provider.fetch(1, 2) | ||
|
|
||
| assert snapshot.air_quality == _air_quality() | ||
|
|
||
|
|
||
| async def test_capability_set_does_not_supplement_dated_context() -> None: | ||
| class Weather: | ||
| async def fetch(self, latitude: float, longitude: float) -> WeatherContextSnapshot: | ||
| raise AssertionError("dated fetch must use fetch_for_date") # pragma: no cover | ||
|
|
||
| async def fetch_for_date( | ||
| self, | ||
| latitude: float, | ||
| longitude: float, | ||
| forecast_date: pendulum.Date, | ||
| ) -> WeatherContextSnapshot: | ||
| assert forecast_date == pendulum.date(2026, 7, 21) | ||
| return _weather() | ||
|
|
||
| class FailingAir: | ||
| async def fetch(self, latitude: float, longitude: float, timezone: str) -> AirQualitySnapshot: | ||
| raise AssertionError("dated contexts must not use current air quality") # pragma: no cover | ||
|
|
||
| provider = CapabilityProviderSet( | ||
| weather=Weather(), | ||
| weather_metadata=_metadata(), | ||
| air_quality=FailingAir(), | ||
| ) | ||
|
|
||
| snapshot = await provider.fetch_for_date(1, 2, pendulum.date(2026, 7, 21)) | ||
|
|
||
| assert snapshot.air_quality is None | ||
|
|
||
|
|
||
| async def test_capability_set_requires_an_air_quality_capability_for_current_context() -> None: | ||
| class Weather: | ||
| async def fetch(self, latitude: float, longitude: float) -> WeatherContextSnapshot: | ||
| return _weather() | ||
|
|
||
| provider = CapabilityProviderSet(weather=Weather(), weather_metadata=_metadata()) | ||
|
|
||
| with pytest.raises(WeatherContextError, match="configure AQICN_API_TOKEN"): | ||
| await provider.fetch(1, 2) | ||
|
|
||
|
|
||
| async def test_capability_set_wraps_air_quality_provider_failure() -> None: | ||
| class Weather: | ||
| async def fetch(self, latitude: float, longitude: float) -> WeatherContextSnapshot: | ||
| return _weather() | ||
|
|
||
| class Air: | ||
| async def fetch(self, latitude: float, longitude: float, timezone: str) -> AirQualitySnapshot: | ||
| raise AirQualityError("failed") | ||
|
|
||
| provider = CapabilityProviderSet(weather=Weather(), weather_metadata=_metadata(), air_quality=Air()) | ||
|
|
||
| with pytest.raises(WeatherContextError, match="AQICN fallback failed"): | ||
| await provider.fetch(1, 2) | ||
|
|
||
|
|
||
| async def test_dated_context_requires_provider_support() -> None: | ||
| class Weather: | ||
| async def fetch(self, latitude: float, longitude: float) -> WeatherContextSnapshot: | ||
| return _weather() # pragma: no cover | ||
|
|
||
| provider = CapabilityProviderSet(weather=Weather(), weather_metadata=_metadata()) | ||
|
|
||
| with pytest.raises(WeatherContextError, match="does not support target forecast dates"): | ||
| await provider.fetch_for_date(1, 2, pendulum.date(2026, 7, 21)) | ||
|
|
||
|
|
||
| async def test_dated_context_rejects_non_callable_fetch_method() -> None: | ||
| class Weather: | ||
| fetch_for_date = 1 | ||
|
|
||
| async def fetch(self, latitude: float, longitude: float) -> WeatherContextSnapshot: | ||
| return _weather() # pragma: no cover | ||
|
|
||
| provider = CapabilityProviderSet(weather=Weather(), weather_metadata=_metadata()) | ||
|
|
||
| with pytest.raises(WeatherContextError, match="does not support target forecast dates"): | ||
| await provider.fetch_for_date(1, 2, pendulum.date(2026, 7, 21)) |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| """Composable provider capabilities and location-scoped context assembly.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from dataclasses import dataclass, replace | ||
| from enum import StrEnum | ||
| from typing import Protocol | ||
|
|
||
| import pendulum | ||
|
|
||
| from .air_quality import AirQualityError, AirQualityProvider | ||
| from .models import WeatherContextSnapshot | ||
| from .time_utils import datetime_timezone_specifier | ||
|
|
||
|
|
||
| class CapabilityName(StrEnum): | ||
| """Identify independently replaceable weather data capabilities.""" | ||
|
|
||
| WEATHER = "weather" | ||
| AIR_QUALITY = "air-quality" | ||
| ALLERGEN = "allergen" | ||
| LIFESTYLE = "lifestyle" | ||
| ALERTS = "alerts" | ||
| NOWCAST = "nowcast" | ||
|
|
||
|
|
||
| class ContextCapabilityProvider(Protocol): | ||
| """Fetch the normalized weather context supplied by a weather capability.""" | ||
|
|
||
| async def fetch( | ||
| self, | ||
| latitude: float, | ||
| longitude: float, | ||
| ) -> WeatherContextSnapshot: | ||
| """Fetch normalized context for a location.""" | ||
| ... | ||
|
|
||
|
|
||
| @dataclass(frozen=True, slots=True) | ||
| class ProviderCapabilities: | ||
| """Describe the capabilities exposed by one provider adapter.""" | ||
|
|
||
| provider_id: str | ||
| provider_name: str | ||
| capabilities: frozenset[CapabilityName] | ||
|
|
||
| def supports(self, capability: CapabilityName) -> bool: | ||
| """Return whether this provider exposes a capability.""" | ||
| return capability in self.capabilities | ||
|
|
||
|
|
||
| @dataclass(frozen=True, slots=True) | ||
|
qodo-code-review[bot] marked this conversation as resolved.
|
||
| class CapabilityProviderSet: | ||
| """Compose independent weather and air-quality capabilities.""" | ||
|
|
||
| weather: ContextCapabilityProvider | ||
| weather_metadata: ProviderCapabilities | ||
| air_quality: AirQualityProvider | None = None | ||
| air_quality_metadata: ProviderCapabilities | None = None | ||
|
|
||
| async def fetch( | ||
| self, | ||
| latitude: float, | ||
| longitude: float, | ||
| *, | ||
| forecast_date: pendulum.Date | None = None, | ||
| ) -> WeatherContextSnapshot: | ||
| """Fetch weather and fill a missing current air-quality capability.""" | ||
| snapshot = await _fetch_context(self.weather, latitude, longitude, forecast_date) | ||
| if snapshot.air_quality is not None or forecast_date is not None: | ||
| return snapshot | ||
| if self.air_quality is None: | ||
| from .weather_context import WeatherContextError | ||
|
|
||
| raise WeatherContextError("Weather source did not provide air quality; configure AQICN_API_TOKEN") | ||
| try: | ||
| air_quality = await self.air_quality.fetch( | ||
| latitude, | ||
| longitude, | ||
| datetime_timezone_specifier(snapshot.observed_at, context="Weather snapshot time"), | ||
| ) | ||
| except AirQualityError: | ||
| from .weather_context import WeatherContextError | ||
|
|
||
| raise WeatherContextError("Weather source did not provide air quality and AQICN fallback failed") from None | ||
| return replace(snapshot, air_quality=air_quality) | ||
|
|
||
| async def fetch_for_date( | ||
| self, | ||
| latitude: float, | ||
| longitude: float, | ||
| forecast_date: pendulum.Date, | ||
| ) -> WeatherContextSnapshot: | ||
| """Fetch a dated context through the composed capabilities.""" | ||
| return await self.fetch(latitude, longitude, forecast_date=forecast_date) | ||
|
|
||
|
|
||
| async def _fetch_context( | ||
|
qodo-code-review[bot] marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. Duplicated dated dispatch CapabilityProviderSet introduces _fetch_context() that re-implements the same “current vs forecast_date” dispatch already provided by weather_context.fetch_weather_context(), creating two sources of truth that can drift in behavior and error semantics. This is especially risky because both paths are exercised in production (service.py uses fetch_weather_context(), while CapabilityProviderSet uses _fetch_context()). Agent Prompt
|
||
| provider: ContextCapabilityProvider, | ||
| latitude: float, | ||
| longitude: float, | ||
| forecast_date: pendulum.Date | None, | ||
| ) -> WeatherContextSnapshot: | ||
| """Call providers that support either current or dated context.""" | ||
| if forecast_date is None: | ||
| return await provider.fetch(latitude, longitude) | ||
| from .weather_context import DatedWeatherContextProvider, WeatherContextError | ||
|
|
||
| if not isinstance(provider, DatedWeatherContextProvider): | ||
| raise WeatherContextError(f"{type(provider).__name__} does not support target forecast dates") | ||
| fetch_for_date = provider.fetch_for_date | ||
| if not callable(fetch_for_date): | ||
| raise WeatherContextError(f"{type(provider).__name__} does not support target forecast dates") | ||
| return await fetch_for_date(latitude, longitude, forecast_date) | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.