fix: quiet unsupported dated supplements

[IceCodeNew]

Follow-up to #75.

Addresses the dated-supplement observability finding in:

• feat: add Japan JMA forecast capability #75 (comment)

Introduces a specific UnsupportedForecastDateError for the expected capability mismatch. LoggedWeatherContextProvider records that case as an INFO-level skip with the target forecast date instead of a failed WARNING, while real WeatherContextError and unexpected provider failures retain their existing warning behavior. Secondary duration/logging failures cannot replace the original unsupported-date exception. Dated-capable supplements such as JMA continue to run for explicit forecast dates.

Verification:

• 788 tests passed with branch coverage
• line coverage 99.82% (increased from 99.78%)
• prek run --all-files

Summary by CodeRabbit

• Bug Fixes

  • Improved handling when weather providers cannot support a requested forecast date.
  • These cases are now recorded as skipped rather than failed, with the original error preserved for callers.
  • Added clearer logging details, including the requested date and reason for skipping.
  • Ensured forecast-date errors remain available even if secondary logging or telemetry encounters an issue.

• Tests

  • Added coverage for unsupported forecast dates, skip logging, and error propagation.

[coderabbitai]

Warning

Review limit reached

@IceCodeNew, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 433e086a-889f-45c1-8119-338d42cf67ad

📥 Commits

Reviewing files that changed from the base of the PR and between 3f99470 and 99dcd80.

📒 Files selected for processing (2)

• tests/test_weather_context.py
• weather_briefing/weather_context.py

📝 Walkthrough

Walkthrough

The change adds UnsupportedForecastDateError, raises it when providers cannot fetch explicit forecast dates, and makes LoggedWeatherContextProvider log these cases as skipped while re-raising the original error. Tests cover skip logging and failures in secondary logging paths.

Changes

Forecast date handling

Layer / File(s)	Summary
Forecast date error contract weather_briefing/weather_context.py	Adds UnsupportedForecastDateError and raises it when fetch_for_date is unsupported or non-callable.
Skip logging and validation weather_briefing/weather_context.py, tests/test_weather_context.py	Logs unsupported forecast dates as skipped at info level, avoids failure logging, and verifies the original exception survives logging failures.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• IceCodeNew/weather-briefing#22: Earlier date-aware fetching changes in the same weather-context capability and logging paths.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title matches the main change: handling unsupported dated supplements by quieting them as skips.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/quiet-undated-supplement-skip

---

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.78%. Comparing base (baf5ca8) to head (99dcd80).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master      #79   +/-   ##
=======================================
  Coverage   99.78%   99.78%           
=======================================
  Files          45       45           
  Lines        8394     8427   +33     
  Branches      488      490    +2     
=======================================
+ Hits         8376     8409   +33     
  Misses         13       13           
  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 (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 37 rules

✅ REVIEW.md

1. UnsupportedForecastDateError logging can mask exception ✓ Resolved 📘 Rule violation ☼ Reliability

Description

The new except UnsupportedForecastDateError handler performs secondary operations
(_LOGGER.info(...) and _elapsed_milliseconds(...)) without guarding against failures, so an
exception in those operations could replace the original UnsupportedForecastDateError. This
violates the requirement to preserve the original business exception when secondary error-handling
operations fail.

Code

weather_briefing/weather_context.py[R186-192]

+        except UnsupportedForecastDateError:
+            _LOGGER.info(
+                "Weather API call skipped provider=%s duration_ms=%d reason=unsupported-forecast-date",
+                self._name,
+                _elapsed_milliseconds(started_at),
+            )
+            raise

Relevance

⭐⭐⭐ High

PR #51 explicitly changed code to preserve original exception when secondary failure handling
errors.

PR-#51

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2152132 requires that failures in secondary error-handling operations (like
logging/metrics) must not replace the original caught business exception. The new `except
UnsupportedForecastDateError: block calls _LOGGER.info(...) and _elapsed_milliseconds(...)`
before re-raising, with no suppression or try/except around those secondary operations.

Rule 2152132: Preserve original business exception when secondary error-handling operations fail
weather_briefing/weather_context.py[186-192]

Agent prompt

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

## Issue description
The `except UnsupportedForecastDateError` path performs secondary operations (logging and duration calculation) that could theoretically raise and mask the original `UnsupportedForecastDateError`, violating the requirement to preserve the primary exception.

## Issue Context
This PR adds a new `except UnsupportedForecastDateError:` block in `LoggedWeatherContextProvider.fetch()` which logs and then re-raises. To ensure the original exception is always the one propagated, secondary operations should be best-effort.

## Fix Focus Areas
- weather_briefing/weather_context.py[186-192]

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

---

2. Skip log omits date ✓ Resolved 🐞 Bug ◔ Observability

Description

LoggedWeatherContextProvider.fetch logs UnsupportedForecastDateError skips without including the
requested forecast_date, which makes it difficult to correlate which dated request triggered the
skip when multiple forecast runs occur. This is an observability regression in the new logging
branch (behavior is otherwise unchanged).

Code

weather_briefing/weather_context.py[R186-191]

+        except UnsupportedForecastDateError:
+            _LOGGER.info(
+                "Weather API call skipped provider=%s duration_ms=%d reason=unsupported-forecast-date",
+                self._name,
+                _elapsed_milliseconds(started_at),
+            )

Relevance

⭐⭐ Medium

No clear historical precedent on including request parameters (forecast_date) in skip logs; logging
patterns vary.

PR-#19

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The provider wrapper receives forecast_date but the new INFO skip log line only includes provider
name, duration, and a static reason, so logs alone cannot identify which date was being requested
when the skip occurred.

weather_briefing/weather_context.py[174-180]
weather_briefing/weather_context.py[183-192]

Agent prompt

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

## Issue description
`LoggedWeatherContextProvider.fetch()` logs a new INFO-level "skipped" line for `UnsupportedForecastDateError`, but it does not include the requested `forecast_date`. This makes it harder to debug or correlate skip events to a particular dated forecast run.

## Issue Context
The `forecast_date` is available in the `fetch()` method signature and is what differentiates the unsupported capability case from normal current-weather calls.

## Fix Focus Areas
- weather_briefing/weather_context.py[174-192]

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

---

[IceCodeNew]

/agentic_review

[qodo-code-review]

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

[qodo-code-review]

PR Summary by Qodo

Fix: log unsupported forecast-date providers as INFO skips

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Introduce a dedicated UnsupportedForecastDateError for forecast-date capability mismatches.
• Log unsupported forecast-date requests as INFO “skipped”, while preserving existing WARNING
 failures.
• Add tests for skip logging and for preserving the primary exception under secondary failures.

Diagram

sequenceDiagram
participant Client
participant Logged as LoggedWeatherContextProvider
participant Fetch as fetch_weather_context
participant Prov as Weather Provider
participant Log as "weather_context logger"

Client->>Logged: fetch(lat, lon, forecast_date)
Logged->>Fetch: fetch_weather_context(Prov, ...)
Fetch-->>Logged: UnsupportedForecastDateError
Logged->>Log: INFO "call skipped" (forecast_date)
Logged-->>Client: re-raise UnsupportedForecastDateError

Fetch-->>Logged: WeatherContextError / Exception
Logged->>Log: WARNING "call failed"

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Pre-check dated capability and return a “skipped” result (no exception)

• ➕ Avoids exception-driven control flow for a known/expected mismatch
• ➕ Makes “skip” an explicit output state
• ➖ Requires changing the public contract (callers currently expect exceptions)
• ➖ Harder to thread through existing fallback/provider composition without broader refactors

2. Use an error code/flag on WeatherContextError instead of a subclass

• ➕ Avoids new exception type proliferation
• ➕ Centralizes classification in one error type
• ➖ Weaker typing/clarity at catch sites (relies on inspecting fields/strings)
• ➖ Easy for secondary failures to accidentally overwrite or misclassify the condition

Recommendation: The PR’s approach (a dedicated UnsupportedForecastDateError plus explicit catch/log-as-skip) is the best fit: it keeps the existing exception-based contract while making the expected capability mismatch distinguishable from real provider failures. Wrapping the skip log in suppress(Exception) is also appropriate to ensure secondary telemetry/logging issues cannot mask the primary unsupported-date error.

Files changed (2) +68 / -2

Bug fix (1) +16 / -2

weather_context.pyDifferentiate unsupported forecast-date requests and log them as skips +16/-2

Differentiate unsupported forecast-date requests and log them as skips

• Introduces UnsupportedForecastDateError as a specific WeatherContextError subtype for dated-capability mismatches. LoggedWeatherContextProvider now catches this error to log an INFO "skipped" event (best-effort, suppressing secondary failures) and re-raises, while other WeatherContextError and unexpected exceptions retain WARNING behavior. The provider capability boundary (fetch_weather_context) now raises UnsupportedForecastDateError when forecast_date is requested from an undated provider.

weather_briefing/weather_context.py

Tests (1) +52 / -0

test_weather_context.pyAdd coverage for unsupported forecast-date skip logging +52/-0

Add coverage for unsupported forecast-date skip logging

• Imports logging and adds tests asserting that UnsupportedForecastDateError is logged as an INFO-level skip (including forecast_date and reason) rather than a WARNING failure. Adds parametrized coverage ensuring secondary failures in elapsed-time calculation or skip logging do not replace the original unsupported-date exception.

tests/test_weather_context.py

[qodo-code-review]

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

[IceCodeNew]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 99dcd80

[qodo-code-review]

Code review by qodo was updated up to the latest commit 99dcd80