[08/10] refactor: separate weather adapters

[IceCodeNew]

Summary

• separate weather contracts, context conversion, provider composition, and regional adapters
• move Open-Meteo metadata validation beside the weather adapter
• migrate capability checks and tests to owning modules
• keep thin compatibility exports for the existing application layer

Dependency

Based on #95 because weather metadata migration follows the delivery-owned reference split. Merge as step 08 after #95.

Verification

• prek run --all-files
• 889 tests passed
• line coverage: 99.85%
• branch coverage: 99.55%

Summary by CodeRabbit

• New Features

  • Added unified weather provider support for Open-Meteo, QWeather, Japan, and Singapore forecasts.
  • Added optional air-quality, pollen, lifestyle, and forecast-date information.
  • Added fallback providers, error handling, and conversion of weather results into documents.
  • Added validation for weather responses and reference data.

• Compatibility

  • Existing weather-related imports remain supported after the internal module reorganization.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR organizes weather contracts, provider adapters, composition, document conversion, and reference data under weather_briefing/weather. Legacy modules re-export the new API, while capability wiring and tests use the updated module boundaries and validate additional malformed-response cases.

Changes

Weather package extraction

Layer / File(s)	Summary
Core weather contracts and orchestration weather_briefing/weather/base.py	Defines shared errors, provider protocols, logging and fallback wrappers, dated-fetch dispatch, and helper utilities.
Regional provider error contract weather_briefing/weather/regional_errors.py	Adds regional provider errors and sanitized exception classification.
JMA and NEA regional adapters weather_briefing/weather/jma.py, weather_briefing/weather/nea.py	Adds Japan forecast and Singapore nowcast providers with validation, parsing, normalization, and regional error handling.
Open-Meteo adapter and reference data weather_briefing/weather/open_meteo.py, weather_briefing/weather/open_meteo_reference.py	Adds weather, air-quality, and allergen enrichment plus validated weather-code metadata loading.
QWeather adapter weather_briefing/weather/qweather.py	Adds JWT authentication, forecast retrieval, optional lifestyle and air-quality enrichment, validation, and formatting.
Composition and document conversion weather_briefing/weather/composition.py, weather_briefing/weather/documents.py	Adds air-quality supplementation and snapshot-to-document conversion.
Package facade and compatibility wiring weather_briefing/weather/__init__.py, weather_briefing/weather_context.py, weather_briefing/regional_weather.py, weather_briefing/reference_data.py, weather_briefing/capabilities.py	Defines public exports, preserves legacy re-exports, relocates reference data, and rewires capability imports.
Validation and import updates tests/test_*.py	Updates import and monkeypatch paths and adds malformed Open-Meteo/QWeather response coverage.
Estimated code review effort: 4 (Complex)	~60 minutes

Possibly related PRs

• IceCodeNew/weather-briefing#22: Touches the dated fetch_weather_context interfaces relocated by this PR.
• IceCodeNew/weather-briefing#75: Overlaps with the JMA provider and parsing helpers moved into weather/jma.py.
• IceCodeNew/weather-briefing#85: Touches the Open-Meteo weather-code reference loader relocated by this PR.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 49.25% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the refactor that separates weather adapters and related weather modules.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/weather-refactor-07-weather

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[IceCodeNew]

/agentic_review

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.82%. Comparing base (83f7128) to head (b9c7d30).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff            @@
##           master      #99    +/-   ##
========================================
  Coverage   99.81%   99.82%            
========================================
  Files          76       86    +10     
  Lines        9358     9470   +112     
  Branches      554      563     +9     
========================================
+ Hits         9341     9453   +112     
  Misses         12       12            
  Partials        5        5            

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 46 rules

✅ REVIEW.md

1. QWeather JSON unchecked 🐞 Bug ☼ Reliability

Description

QWeatherProvider.fetch() calls .get(...) on response.json() results without validating they are
dicts; if QWeather returns a non-object JSON root, an AttributeError escapes both the optional
indices handler and the outer fetch handler (which do not catch AttributeError), bypassing
WeatherContextError and breaking fallback.

Code

weather_briefing/weather/qweather.py[R137-143]

+            weather_response.raise_for_status()
+            weather_payload = weather_response.json()
+            if weather_payload.get("code") != "200":
+                raise WeatherContextError(
+                    "QWeather returned a non-success weather status "
+                    f"code={_safe_api_status(weather_payload.get('code'))}"
+                )

Relevance

⭐⭐⭐ High

Repo has accepted JSON-boundary validation to prevent AttributeError from .get on non-dict payloads.

PR-#98
PR-#24

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The code calls .get on weather_payload and parsed_indices_payload without type checks; if
either payload is not a dict, .get triggers AttributeError, and the exception handlers shown do
not include AttributeError, so it propagates past the intended WeatherContextError boundary.

weather_briefing/weather/qweather.py[124-143]
weather_briefing/weather/qweather.py[167-185]
weather_briefing/weather/qweather.py[218-224]
PR-#98

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`QWeatherProvider.fetch()` assumes `response.json()` returns a dict and immediately uses `.get(...)`. If the JSON root is not an object (array/string/etc.), `.get` raises `AttributeError`, which is not caught by either the outer `fetch()` exception handler or the inner optional indices handler, causing unexpected crashes instead of raising `WeatherContextError` (and preventing fallback).

## Issue Context
This affects both:
- required weather payload parsing (`weather_payload.get(...)`)
- optional indices enrichment (`parsed_indices_payload.get(...)`)

## Fix Focus Areas
- Validate that `weather_payload` is a mapping (e.g., `isinstance(weather_payload, dict)` or `_is_string_keyed_dict`) before using `.get`, otherwise raise `WeatherContextError` / `_QWeatherResponseError`.
- In the optional indices block, similarly validate `parsed_indices_payload` (or catch `AttributeError`) so malformed responses only disable enrichment rather than aborting the whole provider.
- file: weather_briefing/weather/qweather.py[124-163]
- file: weather_briefing/weather/qweather.py[167-205]
- file: weather_briefing/weather/qweather.py[218-224]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Open-Meteo current unchecked 🐞 Bug ☼ Reliability

Description

In OpenMeteoProvider._fetch_air_quality_and_allergen(), payload["current"] is assumed to be a dict;
if it is not, _parse_allergen() will raise AttributeError on current.get(...) and the exception is
not caught, aborting the whole weather fetch instead of skipping optional enrichment.

Code

weather_briefing/weather/open_meteo.py[R183-207]

+            payload = response.json()
+            if forecast_date is None:
+                air_quality_values: dict[str, Any] = payload["current"]
+                allergen_values = air_quality_values
+            else:
+                hourly = payload["hourly"]
+                if not _is_string_keyed_dict(hourly):
+                    raise TypeError("hourly air quality must be an object")
+                air_quality_values, allergen_values = _open_meteo_daily_peak_values(hourly, pollen_types)
+        except (httpx.HTTPError, KeyError, TypeError, ValueError) as exc:
+            _LOGGER.warning(
+                "Weather API optional call failed provider=open-meteo operation=air-quality reason=%s",
+                _safe_provider_error(exc),
+            )
+            return None, None
+        allergen = None
+        if pollen_types:
+            try:
+                allergen = self._parse_allergen(allergen_values, payload, pollen_types)
+            except ReferenceDataError as exc:
+                _LOGGER.warning(
+                    "Weather API optional enrichment failed provider=open-meteo operation=allergen reason=%s",
+                    type(exc).__name__,
+                )
+        time_kind = AirQualityTimeKind.FORECAST if forecast_date is not None else AirQualityTimeKind.OBSERVATION

Relevance

⭐⭐⭐ High

Team previously accepted guarding response.json() shape to avoid uncaught AttributeError and
preserve fallback behavior.

PR-#98

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The enrichment call assigns payload["current"] without checking it is a mapping, then passes it to
_parse_allergen, which unconditionally uses current.get(...); a non-dict current will raise
AttributeError that is not covered by the surrounding exception handling.

weather_briefing/weather/open_meteo.py[183-208]
weather_briefing/weather/open_meteo.py[246-256]
PR-#98

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`OpenMeteoProvider._fetch_air_quality_and_allergen()` assumes the Open‑Meteo air-quality response contains a mapping at `payload["current"]`. If `current` is not a dict (e.g., list/string), `_parse_allergen()` calls `.get()` on it and raises `AttributeError`, which is not handled and aborts `OpenMeteoProvider.fetch()`.

## Issue Context
This code path is meant to be *optional enrichment* (air quality/allergen). It should never crash the overall weather fetch due to a malformed enrichment payload.

## Fix Focus Areas
- Validate `payload`/`payload["current"]` shape before use and convert invalid shapes into a handled failure (e.g., raise `TypeError`/`_OpenMeteoResponseError` so the existing `except (httpx.HTTPError, KeyError, TypeError, ValueError)` returns `(None, None)`).
- file: weather_briefing/weather/open_meteo.py[183-207]
- file: weather_briefing/weather/open_meteo.py[246-256]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Monkeypatches open_meteo import-site ✓ Resolved 📘 Rule violation ▣ Testability

Description

Two tests monkeypatch symbols through weather_briefing.weather.open_meteo even though those
symbols are defined in other modules (open_meteo_weather_code_descriptions in
weather_briefing.weather.open_meteo_reference and pollen_type_names in
weather_briefing.allergen). This violates the requirement to patch at the defining module rather
than an import/re-export site, making the tests more brittle to refactors.

Code

tests/test_weather_context.py[R556-560]

def test_open_meteo_weather_code_lookup_uses_cached_loader_boundary(monkeypatch) -> None:
    monkeypatch.setattr(
-        "weather_briefing.weather_context.open_meteo_weather_code_descriptions",
+        "weather_briefing.weather.open_meteo.open_meteo_weather_code_descriptions",
        lambda: {53: "Reloaded description"},
    )

Relevance

⭐⭐⭐ High

Team previously accepted changing monkeypatch targets from import-site bindings to defining modules
to reduce brittleness.

PR-#95

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

In test_open_meteo_weather_code_lookup_uses_cached_loader_boundary, the monkeypatch targets
weather_briefing.weather.open_meteo.open_meteo_weather_code_descriptions, but the function is
actually defined in weather_briefing.weather.open_meteo_reference and only imported into
open_meteo, which is precisely the kind of import-site patching the rule forbids. Similarly,
test_open_meteo_allergen_reference_failure_keeps_air_quality patches pollen_type_names through
weather_briefing.weather.open_meteo even though pollen_type_names is defined in
weather_briefing.allergen and merely imported into open_meteo, again matching the disallowed
pattern of patching a re-export instead of the defining module.

Rule 2274647: Patch behavior at its defining module, not where it is imported
tests/test_weather_context.py[556-560]
weather_briefing/weather/open_meteo.py[13-27]
weather_briefing/weather/open_meteo_reference.py[12-35]
tests/test_weather_context.py[2022-2025]
weather_briefing/weather/open_meteo.py[13-21]
weather_briefing/allergen.py[70-77]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Tests are monkeypatching `open_meteo_weather_code_descriptions` and `pollen_type_names` via the `weather_briefing.weather.open_meteo` import/re-export site even though those symbols are defined in `weather_briefing.weather.open_meteo_reference` and `weather_briefing.allergen`, respectively.

## Issue Context
Compliance requires monkeypatching/mocking to target the symbol at its defining module path rather than where it is imported or re-exported, because patching import sites is brittle under refactors and re-exports.

## Fix Focus Areas
- tests/test_weather_context.py[556-563]
- tests/test_weather_context.py[2001-2028]
- weather_briefing/weather/open_meteo.py[13-27]
- weather_briefing/weather/open_meteo.py[384-392]
- weather_briefing/weather/open_meteo.py[142-157]
- weather_briefing/weather/open_meteo_reference.py[12-35]
- weather_briefing/allergen.py[70-77]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Capabilities imports heavy facade ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

CapabilityProviderSet and weather_briefing.regional_weather import the
weather_briefing.weather facade inside execution/import paths for provider-neutral or regional
re-export symbols, which triggers eager import of all weather providers the first time those
branches/modules are reached. This creates avoidable first-call/first-import overhead and couples
otherwise thin, provider-agnostic composition/shims to unrelated provider initialization and
potential initialization-time failures, even though the neutral symbols live in
weather_briefing/weather/base.py.

Code

weather_briefing/capabilities.py[R76-79]

        if self.air_quality is None:
-            from .weather_context import WeatherContextError
+            from .weather import WeatherContextError

            raise WeatherContextError("Weather source did not provide air quality; configure AQICN_API_TOKEN")

Relevance

⭐⭐⭐ High

Team has accepted changes to avoid import-time side effects/validation; decoupling facade imports
fits that pattern.

PR-#88
PR-#86

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The cited imports target the weather facade (weather/__init__.py), which in turn imports
provider modules such as open_meteo and qweather, so touching the facade causes those providers
to be imported even when only neutral or regional symbols are needed. The evidence notes that
open_meteo performs import-time reference-data/code metadata validation/loading and qweather
loads localization tables at import time; both operations read packaged JSON resources, meaning
these side effects (latency and possible resource/initialization errors) are incurred on first
import/first execution of those code paths due to Python’s import caching semantics.

weather_briefing/capabilities.py[65-90]
weather_briefing/capabilities.py[119-129]
weather_briefing/weather/init.py[1-24]
weather_briefing/weather/open_meteo.py[24-27]
weather_briefing/weather/qweather.py[24-31]
weather_briefing/data/resources.py[18-30]
weather_briefing/regional_weather.py[1-9]
weather_briefing/localization.py[82-86]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`weather_briefing/capabilities.py` performs deferred imports from `.weather` (the facade) for provider-neutral symbols (`WeatherContextError`, `fetch_weather_context`), and `weather_briefing/regional_weather.py` imports `weather_briefing.weather` to re-export regional providers. Importing the facade executes `weather/__init__.py`, which eagerly imports all providers (e.g., Open‑Meteo and QWeather) and triggers their import-time initialization, creating unnecessary coupling and first-use/first-import overhead.

## Issue Context
- The main cost/failure risk is on the first time per process that the relevant branch/module is hit (Python import caching prevents repeating the work), but it is still unnecessary and can cause unrelated failures when only neutral/regional symbols are desired.
- Provider-neutral definitions (`WeatherContextError`, `fetch_weather_context`) live in `weather_briefing/weather/base.py`.
- The providers brought in transitively by the facade do import-time reference-data initialization (Open‑Meteo code validation/loading and QWeather localization table load), which reads packaged JSON resources.
- `regional_weather.py` is intended as a thin compatibility/re-export layer, but importing the facade makes it pay unrelated startup cost.

## Fix Focus Areas
- weather_briefing/capabilities.py[65-90]
- weather_briefing/capabilities.py[119-129]
- weather_briefing/regional_weather.py[1-17]

## Suggested fix
- In `weather_briefing/capabilities.py`, replace imports from `.weather` with imports from `.weather.base` for provider-neutral symbols:
 - `from .weather import WeatherContextError` � `from .weather.base import WeatherContextError`
 - `from .weather import fetch_weather_context` � `from .weather.base import fetch_weather_context`
 - Similarly update the `fetch_all()` import site to use `.weather.base`.
- In `weather_briefing/regional_weather.py`, avoid importing the facade and instead import only what the module re-exports:
 - `from .weather.jma import JMA_LANGUAGE_SUPPORT, JMAJapanForecastProvider`
 - `from .weather.nea import NEA_LANGUAGE_SUPPORT, NEASingaporeNowcastProvider`
 - `from .weather.regional_errors import RegionalWeatherProviderError`
 - Keep `__all__` the same.

This should keep behavior identical while avoiding facade side effects and unwanted provider initialization.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Enrichment errors logged generically ✓ Resolved 🐞 Bug ◔ Observability

Description

Both OpenMeteoProvider._fetch_air_quality_and_allergen() and QWeatherProvider.fetch() now raise
TypeError with safe, specific contract messages when optional enrichment payload shapes are invalid,
but their handlers log failures via _safe_provider_error(exc), collapsing these schema violations to
the generic "TypeError". Because these code paths intentionally skip optional enrichment and return
empty values ((None, None) or lifestyle_advice == ()), the loss of the safe message makes the
warning logs non-actionable for diagnosing why enrichment was dropped.

Code

weather_briefing/weather/open_meteo.py[R184-189]

+            if not _is_string_keyed_dict(payload):
+                raise TypeError("air-quality response must be an object")
+            if forecast_date is None:
+                air_quality_values = payload["current"]
+                if not _is_string_keyed_dict(air_quality_values):
+                    raise TypeError("current air quality must be an object")

Relevance

⭐⭐ Medium

Team prefers non-sensitive logging by type/classification; adding messages may conflict, though safe
contract text could be allowed.

PR-#84
PR-#98

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

In OpenMeteoProvider._fetch_air_quality_and_allergen(), the code performs explicit shape checks on
the root payload and its current field and raises TypeError with code-defined contract messages,
yet the exception handler logs reason=%s using _safe_provider_error(exc), which only returns the
exception type name and thus discards the message; the accompanying test codifies that invalid
shapes should be handled by silently returning (None, None), making log specificity the primary
diagnostic signal. In QWeatherProvider.fetch(), the new indices validation similarly raises
TypeError for response-shape contract violations, but the handler only logs str(exc) for
_QWeatherResponseError while routing all other exceptions—including these TypeErrors—through
_safe_provider_error(), reducing them to reason=TypeError; the updated test even expects
reason=TypeError, demonstrating the current (but unhelpful) behavior.

weather_briefing/weather/open_meteo.py[183-201]
weather_briefing/weather/base.py[168-171]
tests/test_weather_context.py[663-681]
weather_briefing/weather/qweather.py[184-213]
tests/test_weather_context.py[1350-1377]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Optional enrichment parsing in both `OpenMeteoProvider._fetch_air_quality_and_allergen()` and `QWeatherProvider.fetch()` raises `TypeError` with safe, specific contract messages for invalid upstream payload shapes, but the surrounding exception handlers log failures via `_safe_provider_error(exc)`, collapsing these violations to the generic `reason=TypeError` and losing the actionable message.

## Issue Context
These enrichment paths are intentionally non-fatal: OpenMeteo returns `(None, None)` when air_quality/allergen enrichment fails, and QWeather returns a snapshot with `lifestyle_advice == ()` when indices parsing fails. Because execution continues successfully, operators primarily rely on the warning logs to understand which contract boundary failed; tests currently codify the silent-skip behavior and (for QWeather) even assert `reason=TypeError`, so improving the logged reason requires adjusting both implementation and the relevant expectation.

## Fix Focus Areas
- weather_briefing/weather/open_meteo.py[183-201]
- weather_briefing/weather/qweather.py[192-213]
- tests/test_weather_context.py[1350-1377]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Previous review results

Review updated until commit b9c7d30 ⚖️ Balanced

Results up to commit 3a9f3a0 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Capabilities imports heavy facade ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

CapabilityProviderSet and weather_briefing.regional_weather import the
weather_briefing.weather facade inside execution/import paths for provider-neutral or regional
re-export symbols, which triggers eager import of all weather providers the first time those
branches/modules are reached. This creates avoidable first-call/first-import overhead and couples
otherwise thin, provider-agnostic composition/shims to unrelated provider initialization and
potential initialization-time failures, even though the neutral symbols live in
weather_briefing/weather/base.py.

Code

weather_briefing/capabilities.py[R76-79]

        if self.air_quality is None:
-            from .weather_context import WeatherContextError
+            from .weather import WeatherContextError

            raise WeatherContextError("Weather source did not provide air quality; configure AQICN_API_TOKEN")

Relevance

⭐⭐⭐ High

Team has accepted changes to avoid import-time side effects/validation; decoupling facade imports
fits that pattern.

PR-#88
PR-#86

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The cited imports target the weather facade (weather/__init__.py), which in turn imports
provider modules such as open_meteo and qweather, so touching the facade causes those providers
to be imported even when only neutral or regional symbols are needed. The evidence notes that
open_meteo performs import-time reference-data/code metadata validation/loading and qweather
loads localization tables at import time; both operations read packaged JSON resources, meaning
these side effects (latency and possible resource/initialization errors) are incurred on first
import/first execution of those code paths due to Python’s import caching semantics.

weather_briefing/capabilities.py[65-90]
weather_briefing/capabilities.py[119-129]
weather_briefing/weather/init.py[1-24]
weather_briefing/weather/open_meteo.py[24-27]
weather_briefing/weather/qweather.py[24-31]
weather_briefing/data/resources.py[18-30]
weather_briefing/regional_weather.py[1-9]
weather_briefing/localization.py[82-86]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`weather_briefing/capabilities.py` performs deferred imports from `.weather` (the facade) for provider-neutral symbols (`WeatherContextError`, `fetch_weather_context`), and `weather_briefing/regional_weather.py` imports `weather_briefing.weather` to re-export regional providers. Importing the facade executes `weather/__init__.py`, which eagerly imports all providers (e.g., Open‑Meteo and QWeather) and triggers their import-time initialization, creating unnecessary coupling and first-use/first-import overhead.

## Issue Context
- The main cost/failure risk is on the first time per process that the relevant branch/module is hit (Python import caching prevents repeating the work), but it is still unnecessary and can cause unrelated failures when only neutral/regional symbols are desired.
- Provider-neutral definitions (`WeatherContextError`, `fetch_weather_context`) live in `weather_briefing/weather/base.py`.
- The providers brought in transitively by the facade do import-time reference-data initialization (Open‑Meteo code validation/loading and QWeather localization table load), which reads packaged JSON resources.
- `regional_weather.py` is intended as a thin compatibility/re-export layer, but importing the facade makes it pay unrelated startup cost.

## Fix Focus Areas
- weather_briefing/capabilities.py[65-90]
- weather_briefing/capabilities.py[119-129]
- weather_briefing/regional_weather.py[1-17]

## Suggested fix
- In `weather_briefing/capabilities.py`, replace imports from `.weather` with imports from `.weather.base` for provider-neutral symbols:
 - `from .weather import WeatherContextError` � `from .weather.base import WeatherContextError`
 - `from .weather import fetch_weather_context` � `from .weather.base import fetch_weather_context`
 - Similarly update the `fetch_all()` import site to use `.weather.base`.
- In `weather_briefing/regional_weather.py`, avoid importing the facade and instead import only what the module re-exports:
 - `from .weather.jma import JMA_LANGUAGE_SUPPORT, JMAJapanForecastProvider`
 - `from .weather.nea import NEA_LANGUAGE_SUPPORT, NEASingaporeNowcastProvider`
 - `from .weather.regional_errors import RegionalWeatherProviderError`
 - Keep `__all__` the same.

This should keep behavior identical while avoiding facade side effects and unwanted provider initialization.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Results up to commit c16e5b2 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Monkeypatches open_meteo import-site ✓ Resolved 📘 Rule violation ▣ Testability

Description

Two tests monkeypatch symbols through weather_briefing.weather.open_meteo even though those
symbols are defined in other modules (open_meteo_weather_code_descriptions in
weather_briefing.weather.open_meteo_reference and pollen_type_names in
weather_briefing.allergen). This violates the requirement to patch at the defining module rather
than an import/re-export site, making the tests more brittle to refactors.

Code

tests/test_weather_context.py[R556-560]

def test_open_meteo_weather_code_lookup_uses_cached_loader_boundary(monkeypatch) -> None:
    monkeypatch.setattr(
-        "weather_briefing.weather_context.open_meteo_weather_code_descriptions",
+        "weather_briefing.weather.open_meteo.open_meteo_weather_code_descriptions",
        lambda: {53: "Reloaded description"},
    )

Relevance

⭐⭐⭐ High

Team previously accepted changing monkeypatch targets from import-site bindings to defining modules
to reduce brittleness.

PR-#95

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

In test_open_meteo_weather_code_lookup_uses_cached_loader_boundary, the monkeypatch targets
weather_briefing.weather.open_meteo.open_meteo_weather_code_descriptions, but the function is
actually defined in weather_briefing.weather.open_meteo_reference and only imported into
open_meteo, which is precisely the kind of import-site patching the rule forbids. Similarly,
test_open_meteo_allergen_reference_failure_keeps_air_quality patches pollen_type_names through
weather_briefing.weather.open_meteo even though pollen_type_names is defined in
weather_briefing.allergen and merely imported into open_meteo, again matching the disallowed
pattern of patching a re-export instead of the defining module.

Rule 2274647: Patch behavior at its defining module, not where it is imported
tests/test_weather_context.py[556-560]
weather_briefing/weather/open_meteo.py[13-27]
weather_briefing/weather/open_meteo_reference.py[12-35]
tests/test_weather_context.py[2022-2025]
weather_briefing/weather/open_meteo.py[13-21]
weather_briefing/allergen.py[70-77]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Tests are monkeypatching `open_meteo_weather_code_descriptions` and `pollen_type_names` via the `weather_briefing.weather.open_meteo` import/re-export site even though those symbols are defined in `weather_briefing.weather.open_meteo_reference` and `weather_briefing.allergen`, respectively.

## Issue Context
Compliance requires monkeypatching/mocking to target the symbol at its defining module path rather than where it is imported or re-exported, because patching import sites is brittle under refactors and re-exports.

## Fix Focus Areas
- tests/test_weather_context.py[556-563]
- tests/test_weather_context.py[2001-2028]
- weather_briefing/weather/open_meteo.py[13-27]
- weather_briefing/weather/open_meteo.py[384-392]
- weather_briefing/weather/open_meteo.py[142-157]
- weather_briefing/weather/open_meteo_reference.py[12-35]
- weather_briefing/allergen.py[70-77]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Results up to commit 35d616f ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Enrichment errors logged generically ✓ Resolved 🐞 Bug ◔ Observability

Description

Both OpenMeteoProvider._fetch_air_quality_and_allergen() and QWeatherProvider.fetch() now raise
TypeError with safe, specific contract messages when optional enrichment payload shapes are invalid,
but their handlers log failures via _safe_provider_error(exc), collapsing these schema violations to
the generic "TypeError". Because these code paths intentionally skip optional enrichment and return
empty values ((None, None) or lifestyle_advice == ()), the loss of the safe message makes the
warning logs non-actionable for diagnosing why enrichment was dropped.

Code

weather_briefing/weather/open_meteo.py[R184-189]

+            if not _is_string_keyed_dict(payload):
+                raise TypeError("air-quality response must be an object")
+            if forecast_date is None:
+                air_quality_values = payload["current"]
+                if not _is_string_keyed_dict(air_quality_values):
+                    raise TypeError("current air quality must be an object")

Relevance

⭐⭐ Medium

Team prefers non-sensitive logging by type/classification; adding messages may conflict, though safe
contract text could be allowed.

PR-#84
PR-#98

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

In OpenMeteoProvider._fetch_air_quality_and_allergen(), the code performs explicit shape checks on
the root payload and its current field and raises TypeError with code-defined contract messages,
yet the exception handler logs reason=%s using _safe_provider_error(exc), which only returns the
exception type name and thus discards the message; the accompanying test codifies that invalid
shapes should be handled by silently returning (None, None), making log specificity the primary
diagnostic signal. In QWeatherProvider.fetch(), the new indices validation similarly raises
TypeError for response-shape contract violations, but the handler only logs str(exc) for
_QWeatherResponseError while routing all other exceptions—including these TypeErrors—through
_safe_provider_error(), reducing them to reason=TypeError; the updated test even expects
reason=TypeError, demonstrating the current (but unhelpful) behavior.

weather_briefing/weather/open_meteo.py[183-201]
weather_briefing/weather/base.py[168-171]
tests/test_weather_context.py[663-681]
weather_briefing/weather/qweather.py[184-213]
tests/test_weather_context.py[1350-1377]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Optional enrichment parsing in both `OpenMeteoProvider._fetch_air_quality_and_allergen()` and `QWeatherProvider.fetch()` raises `TypeError` with safe, specific contract messages for invalid upstream payload shapes, but the surrounding exception handlers log failures via `_safe_provider_error(exc)`, collapsing these violations to the generic `reason=TypeError` and losing the actionable message.

## Issue Context
These enrichment paths are intentionally non-fatal: OpenMeteo returns `(None, None)` when air_quality/allergen enrichment fails, and QWeather returns a snapshot with `lifestyle_advice == ()` when indices parsing fails. Because execution continues successfully, operators primarily rely on the warning logs to understand which contract boundary failed; tests currently codify the silent-skip behavior and (for QWeather) even assert `reason=TypeError`, so improving the logged reason requires adjusting both implementation and the relevant expectation.

## Fix Focus Areas
- weather_briefing/weather/open_meteo.py[183-201]
- weather_briefing/weather/qweather.py[192-213]
- tests/test_weather_context.py[1350-1377]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Results up to commit b9c7d30 ⚖️ Balanced

No changes from previous review

[IceCodeNew]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[qodo-code-review]

PR Summary by Qodo

Refactor weather providers into dedicated weather package with compat exports

✨ Enhancement 🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

AI Description

• Split monolithic weather context into contracts, adapters, composition, and document conversion
 modules.
• Harden provider logging and response-shape validation to avoid leaking untrusted details.
• Keep compatibility re-exports and update tests to the new import locations.

Diagram

graph TD
  A["Application / tests"] --> B["weather_briefing.weather (facade)"] --> C["Provider adapters"] --> X{{"External weather APIs"}}
  B --> D["base (contracts + fallback + logging)"]
  B --> E["composition (AQ supplement)"]
  B --> F["documents (snapshot_to_documents)"]
  C --> G["Open-Meteo reference"] --> H[("Packaged reference data")]
  subgraph Legend
    direction LR
    _m["Python module"] ~~~ _e{{"External API"}} ~~~ _d[("Data")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep `weather_context.py` as orchestrator with internal submodules

• ➕ Minimizes import churn and maintains a single public file for users
• ➕ Allows incremental extraction without adding a new top-level package
• ➖ Continues to concentrate responsibilities and encourages tight coupling
• ➖ Harder to enforce ownership boundaries and keep provider-specific logic isolated

2. Introduce provider plugin registry (entrypoints/config-driven)

• ➕ Decouples provider selection from code and enables easier extension
• ➕ Can support dynamic enable/disable per deployment
• ➖ More complexity than needed for the current provider set
• ➖ Harder to type-check and test deterministically without additional scaffolding

3. Extract a shared HTTP/validation utility layer

• ➕ Reduces duplication of response-shape checks and safe error classification
• ➕ Encourages consistent error semantics across providers
• ➖ Risk of over-abstracting provider-specific contracts
• ➖ May slow future provider-specific evolution if utilities become too generic

Recommendation: The chosen approach (new weather/ package with a facade plus thin compatibility re-exports) is the best tradeoff for this refactor step: it creates clear ownership boundaries per provider, centralizes provider-neutral contracts/logging in base.py, and limits downstream breakage. The alternatives either preserve the monolith structure or add unnecessary indirection for the current scope.

Files changed (19) +1619 / -1354

Enhancement (2) +225 / -0

__init__.pyAdd weather package facade re-exporting contracts, providers, and helpers +46/-0

Add weather package facade re-exporting contracts, providers, and helpers

• Introduces 'weather_briefing.weather' as the primary import surface for weather contracts, providers, composition utilities, and snapshot conversion, including '__all__' for stable exports.

weather_briefing/weather/init.py

base.pyIntroduce provider-neutral contracts, fallback orchestration, and safe logging +179/-0

Introduce provider-neutral contracts, fallback orchestration, and safe logging

• Adds protocols for current/dated providers, shared 'fetch_weather_context' capability boundary, fallback provider composition, and 'LoggedWeatherContextProvider' with non-sensitive error logging (exception class name only). Also includes shared helpers for safe error categorization and payload shape guards.

weather_briefing/weather/base.py

Bug fix (2) +818 / -0

open_meteo.pyAdd Open-Meteo adapter with stricter payload validation and safe optional enrichment +397/-0

Add Open-Meteo adapter with stricter payload validation and safe optional enrichment

• Ports the Open-Meteo provider into its own module, adds explicit object/array shape checks for weather and air-quality payloads, and logs optional enrichment failures with safe, code-defined reasons. Uses 'open_meteo_reference' for weather-code descriptions.

weather_briefing/weather/open_meteo.py

qweather.pyAdd QWeather adapter with response-shape validation and safer failure modes +421/-0

Add QWeather adapter with response-shape validation and safer failure modes

• Ports QWeather authentication and provider logic into its own module and adds explicit validation that responses are objects and that 'daily' fields are arrays. Treats lifestyle indices as optional: invalid shapes log warnings and yield empty advice instead of failing the whole fetch.

weather_briefing/weather/qweather.py

Refactor (10) +468 / -1333

capabilities.pyRoute capability composition through new weather facade boundary +4/-4

Route capability composition through new weather facade boundary

• Replaces imports of 'WeatherContextError' and 'fetch_weather_context' from 'weather_context' to 'weather_briefing.weather', keeping capability behavior but aligning with the new ownership split.

weather_briefing/capabilities.py

reference_data.pyRetain reference_data compatibility while delegating Open-Meteo metadata +2/-34

Retain reference_data compatibility while delegating Open-Meteo metadata

• Removes the in-module Open-Meteo weather-code validation/caching logic and re-exports 'open_meteo_weather_code_descriptions' from 'weather.open_meteo_reference'. Keeps 'ReferenceDataError' and other packaged-data exports stable.

weather_briefing/reference_data.py

regional_weather.pyConvert regional_weather into compatibility re-export module +17/-243

Convert regional_weather into compatibility re-export module

• Replaces the previous in-file NEA/JMA provider implementations with re-exports from the new 'weather' package facade, preserving the public API while relocating adapter code.

weather_briefing/regional_weather.py

composition.pyExtract air-quality supplementing composition policy +62/-0

Extract air-quality supplementing composition policy

• Moves 'AirQualitySupplementingWeatherProvider' into a dedicated module and routes composition through the provider boundary in 'base.fetch_weather_context'. Preserves existing behavior around optional AQ supplementation.

weather_briefing/weather/composition.py

documents.pyExtract snapshot-to-source-documents conversion +49/-0

Extract snapshot-to-source-documents conversion

• Moves 'snapshot_to_documents' out of the former monolith into a focused module, keeping the formatting and localization behaviors while aligning with the new package structure.

weather_briefing/weather/documents.py

jma.pyAdd JMA regional adapter module +151/-0

Add JMA regional adapter module

• Introduces the Japan Meteorological Agency provider adapter and related parsing helpers, using shared regional error classification and preserving the prior response validation/normalization behavior.

weather_briefing/weather/jma.py

nea.pyAdd NEA Singapore nowcast adapter module +97/-0

Add NEA Singapore nowcast adapter module

• Introduces the Singapore NEA two-hour nowcast provider and associated payload parsing helpers, with shared safe error classification and unchanged snapshot semantics.

weather_briefing/weather/nea.py

open_meteo_reference.pyMove Open-Meteo weather-code metadata validation beside provider +35/-0

Move Open-Meteo weather-code metadata validation beside provider

• Adds a cached loader that validates packaged Open-Meteo weather-code descriptions and exposes a read-only mapping, relocating the logic from 'reference_data' to weather-owned code.

weather_briefing/weather/open_meteo_reference.py

regional_errors.pyCentralize regional provider error type and safe error classifier +16/-0

Centralize regional provider error type and safe error classifier

• Introduces 'RegionalWeatherProviderError' and 'safe_regional_error' shared by NEA/JMA adapters to ensure consistent contract errors and non-sensitive error reporting.

weather_briefing/weather/regional_errors.py

weather_context.pyReduce weather_context to compatibility re-exports +35/-1052

Reduce weather_context to compatibility re-exports

• Replaces the former all-in-one implementation with re-exports from 'weather_briefing.weather', preserving the legacy import path while moving logic to dedicated modules.

weather_briefing/weather_context.py

Tests (5) +108 / -21

test_capabilities.pyUpdate capability tests to import WeatherContextError from new facade +1/-1

Update capability tests to import WeatherContextError from new facade

• Switches test imports from 'weather_context' to 'weather_briefing.weather' for 'WeatherContextError' to reflect the new module layout.

tests/test_capabilities.py

test_languages.pyPoint language support tests at new weather facade exports +1/-1

Point language support tests at new weather facade exports

• Moves 'OPEN_METEO_LANGUAGE_SUPPORT' and 'QWEATHER_LANGUAGE_SUPPORT' imports from 'weather_context' to 'weather_briefing.weather'. No behavior changes; aligns tests with new API surface.

tests/test_languages.py

test_reference_data.pyMigrate Open-Meteo reference-data tests to weather-owned module +2/-4

Migrate Open-Meteo reference-data tests to weather-owned module

• Updates imports and monkeypatch targets to use 'weather.open_meteo_reference' instead of 'reference_data' for Open-Meteo weather-code descriptions. Keeps validation expectations unchanged while moving ownership.

tests/test_reference_data.py

test_regional_weather.pyRewire regional provider tests to split NEA/JMA modules and facade +7/-3

Rewire regional provider tests to split NEA/JMA modules and facade

• Updates imports so high-level providers come from 'weather_briefing.weather', while internal parsing helpers are imported from 'weather.jma' and 'weather.nea'. Reflects the new per-region adapter modules.

tests/test_regional_weather.py

test_weather_context.pyExpand tests for safe logging and response-shape validation under new modules +97/-12

Expand tests for safe logging and response-shape validation under new modules

• Repoints imports to 'weather_briefing.weather' and the provider-specific modules. Adds/updates tests to ensure invalid Open-Meteo/QWeather payload shapes are rejected or treated as optional enrichments, and verifies failure logs no longer include sensitive/untrusted exception details.

tests/test_weather_context.py

[qodo-code-review]

Code review by qodo was updated up to the latest commit 3a9f3a0

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

weather_briefing/weather/open_meteo.py (1)

26-26: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Document the Open-Meteo reference-data warm-up.

The module imports and calls open_meteo_weather_code_descriptions(), so a missing/malformed open_meteo_weather_codes.json can raise ReferenceDataError before any Open-Meteo provider is used. Add a short comment on line 26 stating this is intentional fail-fast reference-data validation.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@weather_briefing/weather/open_meteo.py` at line 26, Add a short comment
immediately above the module-level call to
open_meteo_weather_code_descriptions() explaining that it intentionally warms up
and fail-fast validates the Open-Meteo reference data before provider use,
allowing missing or malformed data to raise ReferenceDataError during import.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@weather_briefing/weather/base.py`:
- Around line 78-84: Update the WeatherContextError handling around the logger
call to stop passing exc as the reason value, which implicitly logs its
provider-supplied message. Replace it with a stable, non-sensitive error
category while preserving the existing provider name and duration fields.

---

Nitpick comments:
In `@weather_briefing/weather/open_meteo.py`:
- Line 26: Add a short comment immediately above the module-level call to
open_meteo_weather_code_descriptions() explaining that it intentionally warms up
and fail-fast validates the Open-Meteo reference data before provider use,
allowing missing or malformed data to raise ReferenceDataError during import.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 46c1ab66-d7bd-4c76-918a-16bde8c1a53f

📥 Commits

Reviewing files that changed from the base of the PR and between b6983cc and 3a9f3a0.

📒 Files selected for processing (19)

• tests/test_capabilities.py
• tests/test_languages.py
• tests/test_reference_data.py
• tests/test_regional_weather.py
• tests/test_weather_context.py
• weather_briefing/capabilities.py
• weather_briefing/reference_data.py
• weather_briefing/regional_weather.py
• weather_briefing/weather/__init__.py
• weather_briefing/weather/base.py
• weather_briefing/weather/composition.py
• weather_briefing/weather/documents.py
• weather_briefing/weather/jma.py
• weather_briefing/weather/nea.py
• weather_briefing/weather/open_meteo.py
• weather_briefing/weather/open_meteo_reference.py
• weather_briefing/weather/qweather.py
• weather_briefing/weather/regional_errors.py
• weather_briefing/weather_context.py

[IceCodeNew]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit c16e5b2

[IceCodeNew]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit c16e5b2

[IceCodeNew]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 35d616f

[IceCodeNew]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit b9c7d30

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_weather_context.py`:
- Around line 663-694: Update
test_open_meteo_current_enrichment_rejects_non_object_payload to use neutral
synthetic coordinates, preferably the existing fixture values 1 and 2, when
calling _fetch_air_quality_and_allergen; leave the mocked payload and assertions
unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ac74aa74-6f3f-4285-92c4-52340bb4d801

📥 Commits

Reviewing files that changed from the base of the PR and between 3a9f3a0 and b9c7d30.

📒 Files selected for processing (19)

• tests/test_capabilities.py
• tests/test_languages.py
• tests/test_reference_data.py
• tests/test_regional_weather.py
• tests/test_weather_context.py
• weather_briefing/capabilities.py
• weather_briefing/reference_data.py
• weather_briefing/regional_weather.py
• weather_briefing/weather/__init__.py
• weather_briefing/weather/base.py
• weather_briefing/weather/composition.py
• weather_briefing/weather/documents.py
• weather_briefing/weather/jma.py
• weather_briefing/weather/nea.py
• weather_briefing/weather/open_meteo.py
• weather_briefing/weather/open_meteo_reference.py
• weather_briefing/weather/qweather.py
• weather_briefing/weather/regional_errors.py
• weather_briefing/weather_context.py

🚧 Files skipped from review as they are similar to previous changes (15)

• weather_briefing/weather/open_meteo_reference.py
• tests/test_languages.py
• weather_briefing/weather/init.py
• weather_briefing/regional_weather.py
• weather_briefing/weather/composition.py
• weather_briefing/weather/documents.py
• weather_briefing/weather/regional_errors.py
• tests/test_reference_data.py
• tests/test_regional_weather.py
• weather_briefing/weather/jma.py
• weather_briefing/weather/nea.py
• weather_briefing/weather/base.py
• weather_briefing/weather_context.py
• weather_briefing/weather/qweather.py
• weather_briefing/weather/open_meteo.py

[coderabbitai]

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Use synthetic coordinates in the mocked test.

The test passes the real location 39.9, 116.3, although the mocked response does not depend on geography. Replace it with neutral fixture values such as the existing 1, 2 inputs.

As per coding guidelines, “Never commit real credentials, locations, coordinates, private source URLs, generated content, or runtime state; use runtime configuration, ignored state paths, and public test data.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_weather_context.py` around lines 663 - 694, Update
test_open_meteo_current_enrichment_rejects_non_object_payload to use neutral
synthetic coordinates, preferably the existing fixture values 1 and 2, when
calling _fetch_air_quality_and_allergen; leave the mocked payload and assertions
unchanged.

Source: Coding guidelines

[qodo-code-review]

Code review by qodo was updated up to the latest commit b9c7d30