Skip to content

feat(carbon): only warn about a short carbon forecast when it matters (#5964) - #5965

Open
springfall2008 wants to merge 1 commit into
mainfrom
feat/carbon-warn-thin-coverage-5964
Open

springfall2008 wants to merge 1 commit into
mainfrom
feat/carbon-warn-thin-coverage-5964

Conversation

@springfall2008

Copy link
Copy Markdown
Owner

This is an automated draft PR generated from issue #5964 — a maintainer should review it before merging.

Fixes #5964

Summary

The carbon component polls the Carbon Intensity API at most every ~4 hours, and each poll gets the 48 hours the API publishes. Between polls the forecast's reach therefore drops below a 48-hour plan on every cycle. fetch_carbon_intensity() warned whenever any plan minute had to be replicated, so it logged a Warn: on almost every 5-minute cycle for everyone with carbon enabled. This PR follows both of the reporter's suggestions:

  • At fetch time (CarbonAPI.fetch_carbon_data()): a successful fetch whose forecast reaches less than CARBON_FETCH_SHORT_HOURS (47h) ahead logs one Warn: Carbon API: Forecast for postcode … only reaches N hours ahead…. That is the case fix(carbon): cope with the carbon forecast running out before the plan ends #4958 was meant to flag (the API publishing short when its upstream forecast is late), and it now warns once per fetch instead of once per cycle. The 1h margin allows for a fresh fetch whose last slot ends a little before now+48h because slots fall on half-hour boundaries.
  • Per cycle (fetch_carbon_intensity()): the replication message is still logged so it stays visible for debugging, but it is only a Warn: once the forecast covers less than CARBON_WARN_COVERAGE_HOURS (24h) of the plan. That happens after about a day of failed polls, or when a short API response is already near its end.

Both thresholds are named constants in const.py.

Testing

  • ./run_pre_commit (from coverage/): all 14 hooks passed.
  • tools/triage_test.sh carbon, with the fix: 32 passed, 0 failed.
  • Same module with the source changes stashed (carbon.py, fetch.py, const.py) and only the new tests in place: 30 passed, 2 failed. The two failures are the new behaviour tests:
    • fetch_short_warns: a 15h API response must warn once at fetch time. Without the fix there is no fetch-time warning.
    • intensity_warn_thin: a forecast with 44h left (the normal state 4h after a poll) must log a plain message, not a Warn:, and one with 12h left must still warn. Without the fix the 44h case logged Warn: Carbon intensity forecast only covers 44.0 hours…, which is the reporter's symptom.
  • The new fetch_full_quiet test (a full 48h fetch logs no warning) passes with and without the fix. That is intended: it stops the fetch-time check from over-warning, rather than reproducing the bug.

Both tests fail without the fix and pass with it.

Notes

  • A genuinely short API response under 24h now produces both the one-off fetch-time warning and the per-cycle warning. That is deliberate, because at that point the plan really is running on replicated data for most of its horizon.
  • The command-line harness test_carbon_api() (carbon.py, # pragma: no cover) still works out its own forward coverage and prints its own note at < 48h. I left it as it is. It is a human-facing diagnostic with a stricter threshold, and sharing the calculation with the component would change a no-cover path for little gain. Running it now also shows the new component log line when the API is short.
  • No separate "API call failed and cached coverage is thin" message was added. Failed fetches already log a Warn:, and if failures continue the per-cycle message turns into a warning once coverage falls under 24h, which covers that case.
  • Blast radius: GitNexus impact() on fetch_carbon_intensity and fetch_carbon_data showed LOW risk, with one direct caller each (fetch_sensor_data and CarbonAPI.run). detect_changes showed low risk and no affected processes.

🤖 Generated with Claude Code

…#5964)

The per-cycle replication warning fired on every 5-minute plan between the
component's ~4-hourly polls, because a 48h forecast's reach decays below a
48h plan as it ages. Warn once per fetch when the API itself publishes short,
and only escalate the per-cycle message to a warning when less than 24h of
the plan is covered.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@springfall2008 springfall2008 self-assigned this Nov 3, 2026
@springfall2008 springfall2008 added BOT_REVIEW Trigger an autotriage BOT_CLEANUP Trigger: bot should address PR review feedback and CI failures, then commit and push labels Nov 3, 2026

@springfall2008 springfall2008 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated comment from the triage bot.

Reviewed at head add863ea (add863eaf6669631b429576696b8e3cff1c6672a) — no issues found: 0 inline findings, 0 in the body.

What was checked:

  • Read the enclosing functions in full: fetch_carbon_data() (apps/predbat/carbon.py:39-117), carbon_replicate() (apps/predbat/fetch.py:2794) and fetch_carbon_intensity() (apps/predbat/fetch.py:4307).
  • Coverage semantics: carbon_replicate() keys replicated minutes relative to now (its docstring and body confirm), so min(carbon_replicated) / 60 at apps/predbat/fetch.py:4327 is hours from now to the first uncovered minute — the CARBON_WARN_COVERAGE_HOURS gate means what the message claims, and the reported log pattern (coverage decaying 44.8 → 44.1 between polls, back to ~47.9 after one) is exactly the state that now logs at INFO instead of Warn:.
  • Threshold margin: date_from is minute-truncated and the API's slots sit on the half-hour grid, so the worst reach of a healthy fetch is ~47.5–48.0h ahead — CARBON_FETCH_SHORT_HOURS = 47 cannot fire on a normal response; the 1h margin covers grid snapping and the minute truncation. The per-cycle decay between polls (~4h window) never gets near 47 either way.
  • Degenerate cases: max() runs on a list guarded by if collected_data:; the to strings round-trip through this code's own strftime(TIME_FORMAT_HA) so strptime_aware cannot raise there; the first point starts at date_from (= now), so max to is always ≥ now+30min and hours_ahead cannot go negative. Both datetimes are timezone-aware. Dict truthiness (if carbon_replicated:) is unchanged from the old code.
  • Frequency: the fetch-time warning sits behind the existing 4-hour cache gate (carbon.py:44), so it fires once per successful poll as the body says; a genuinely short response under 24h legitimately surfaces both warnings (documented as deliberate).
  • Tests: the 3 new sub-tests are registered in test_carbon()'s registry (the module is already in TEST_REGISTRY); the PR body's red/green run (fetch_short_warns and intensity_warn_thin fail with the source change stashed) covers both new behaviours, and fetch_full_quiet intentionally passes both ways. The helpers used (MockCarbonAPI.log_messages, create_aiohttp_mock_session/response, run_async) all exist; _test_intensity_warn_only_when_thin restores forecast_minutes/log/get_state_wrapper in a finally, and the pop path correctly un-sets instance attributes that were class-level methods, so no cross-test leak. Docstrings on all six new helpers/tests pass interrogate.
  • Blast radius: GitNexus impact() on fetch_carbon_intensity = LOW with one direct caller (fetch_sensor_data → plan_once → update_pred); fetch_carbon_data has one caller (the component's run() loop). Logging-only change — no plan/prediction surface, so no kernel-parity concern, no golden case moves, no new PredBat state for the debug dump, no CONFIG_ITEMS entry needed (internal thresholds, not user settings), and nothing under docs/ references the old message text.

@springfall2008 springfall2008 removed the BOT_REVIEW Trigger an autotriage label Nov 3, 2026
@gcoan
gcoan marked this pull request as ready for review November 3, 2026 23:56
@springfall2008

Copy link
Copy Markdown
Owner Author

Automated cleanup check from the triage bot — nothing for a maintainer to act on.

Nothing outstanding at head add863ea:

  • Branch is already up to date with main (merge was a no-op).
  • No inline review threads; the one review on this head reported 0 findings.
  • CI: pre-commit and kernel-binaries pass. owner-gate fails only with "Waiting for springfall2008 to apply the owner-approved label, enable auto-merge or approve the PR". It clears once the owner approves, so there is no code change to make.

Nothing pushed.

@springfall2008 springfall2008 removed the BOT_CLEANUP Trigger: bot should address PR review feedback and CI failures, then commit and push label Nov 4, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reduce number of warnings from "Carbon intensity forecast"

1 participant