Repository navigation
feat(carbon): only warn about a short carbon forecast when it matters (#5964) - #5965
Open
springfall2008 wants to merge 1 commit into
Open
springfall2008 wants to merge 1 commit into
springfall2008 wants to merge 1 commit into
Conversation
…#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
commented
Nov 3, 2026
springfall2008
left a comment
Owner
Author
There was a problem hiding this comment.
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) andfetch_carbon_intensity()(apps/predbat/fetch.py:4307). - Coverage semantics:
carbon_replicate()keys replicated minutes relative to now (its docstring and body confirm), somin(carbon_replicated) / 60atapps/predbat/fetch.py:4327is hours from now to the first uncovered minute — theCARBON_WARN_COVERAGE_HOURSgate 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 ofWarn:. - Threshold margin:
date_fromis 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 = 47cannot 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 byif collected_data:; thetostrings round-trip through this code's ownstrftime(TIME_FORMAT_HA)sostrptime_awarecannot raise there; the first point starts atdate_from(= now), so maxtois always ≥ now+30min andhours_aheadcannot 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 inTEST_REGISTRY); the PR body's red/green run (fetch_short_warnsandintensity_warn_thinfail with the source change stashed) covers both new behaviours, andfetch_full_quietintentionally passes both ways. The helpers used (MockCarbonAPI.log_messages,create_aiohttp_mock_session/response,run_async) all exist;_test_intensity_warn_only_when_thinrestoresforecast_minutes/log/get_state_wrapperin afinally, 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()onfetch_carbon_intensity= LOW with one direct caller (fetch_sensor_data→plan_once→update_pred);fetch_carbon_datahas one caller (the component'srun()loop). Logging-only change — no plan/prediction surface, so no kernel-parity concern, no golden case moves, no newPredBatstate for the debug dump, no CONFIG_ITEMS entry needed (internal thresholds, not user settings), and nothing underdocs/references the old message text.
gcoan
marked this pull request as ready for review
November 3, 2026 23:56
Owner
Author
|
Automated cleanup check from the triage bot — nothing for a maintainer to act on. Nothing outstanding at head
Nothing pushed. |
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
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 aWarn:on almost every 5-minute cycle for everyone with carbon enabled. This PR follows both of the reporter's suggestions:CarbonAPI.fetch_carbon_data()): a successful fetch whose forecast reaches less thanCARBON_FETCH_SHORT_HOURS(47h) ahead logs oneWarn: 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.fetch_carbon_intensity()): the replication message is still logged so it stays visible for debugging, but it is only aWarn:once the forecast covers less thanCARBON_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(fromcoverage/): all 14 hooks passed.tools/triage_test.sh carbon, with the fix: 32 passed, 0 failed.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 aWarn:, and one with 12h left must still warn. Without the fix the 44h case loggedWarn: Carbon intensity forecast only covers 44.0 hours…, which is the reporter's symptom.fetch_full_quiettest (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
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.Warn:, and if failures continue the per-cycle message turns into a warning once coverage falls under 24h, which covers that case.impact()onfetch_carbon_intensityandfetch_carbon_datashowed LOW risk, with one direct caller each (fetch_sensor_dataandCarbonAPI.run).detect_changesshowed low risk and no affected processes.🤖 Generated with Claude Code