fix(energy): floor the request window to whole UTC hours - #284
Closed
andrew-blake wants to merge 6 commits into
Closed
fix(energy): floor the request window to whole UTC hours#284andrew-blake wants to merge 6 commits into
andrew-blake wants to merge 6 commits into
Conversation
sensor_ata.py carried "Only created for devices where outdoor sensor detected during capability discovery" from the original gated implementation (#53). The sensor has been always-created since, and the two lines below the comment in the same description say so. docs/entities.md carried the matching "(if available)", contradicted by its own line 102. The hazard is not today's behaviour, which is correct. has_outdoor_temp_sensor still exists as a runtime discovery flag, so the comment reads as an invitation to reinstate should_create_fn on it - gating creation on a transient value, which would drop the entity for any unit idle at setup. Also names which concern the ATW keep-filter serves: the sensors are gated in sensor_atw.py, and that filter only keeps fake values out of the cache. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The server sums only the samples at or after "from", and both energy call sites computed it as now - 48h with no flooring. The oldest bucket was therefore a shrinking partial hour: every poll asked for a smaller slice of it than the last, so the value came back lower and the decrease guard logged it as "possible API issue". Measured on prod 2026-08-25: all 79 such warnings hit the bucket exactly 48h old, at :16 and :46 past the hour - the 30-minute energy interval. Kukja's 2026-08-23 06:00 bucket holds 0.567 kWh, read as 0.433 at :16 (prorata predicts 44/60) and 0.133 at :46 (14/60, observed 0.235). Re-requesting it floored returned 0.567 again. No data was wrong: the guard already kept the larger value, and nothing was double-counted or lost. This removes ~4 warnings/hour/unit that had been framing our own window arithmetic as a vendor fault - 76% of prod's melcloudhome log volume. The window computation moves to EnergyTrackerBase so the reason lives in one place rather than being duplicated at both call sites. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The report endpoints stamp points in the unit's own zone (ADR-022), so "is UTC right here?" is the obvious question about the hour floor. It is: measured 2026-08-25 07:51 UTC, a Europe/London (+1) and a Europe/Skopje (+2) unit both reported newest bucket 07:00 for an identical window, and pushing "to" three hours past either unit's local wall-clock surfaced no newer bucket, ruling out clipping as the explanation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The energy request format ("%Y-%m-%d %H:%M") carries no offset marker, so
strftime drops the tzinfo without complaint. A caller passing a
local-aware datetime would query a window shifted by its offset, and
nothing - not the request, not the response, not the decrease guard -
would report anything wrong.
Both current callers pass datetime.now(UTC), so this changes no behaviour
today. It closes the path where a future one does not. A naive datetime
cannot be rescued by astimezone and raises instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ADR-023's "what tilts it that way" paragraph and the mock's INTEMP_DATASETS comment both justified themselves by citing a source this repo should not reference. Removing the citation alone would have left the ADR asserting a deliberate assumption with no reason given, next to a paragraph arguing the other way, so the argument is rebuilt from facts checkable in a captured response: - the report's dataset ids are exactly the measure names the per-measure telemetry endpoint used, zone-2 names included - a single-zone unit still receives *_zone1 and *_boiler series, so the server is not filtering datasets by device capability on any visible axis The counter-argument gains the consequence that follows honestly from that: if the unfiltered set is simply fixed at eight, zone-2 series never arrive for anyone. The ROOM_TEMPERATURE_ZONE1 / SET_TEMPERATURE_ZONE1 label keys cited on the other side are observable in comfort-graph responses. This is a weaker case than the one it replaces, and deliberately reads that way. No claim here rests on anything but a recorded response. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## fix/report-timestamps-device-local #284 +/- ##
======================================================================
+ Coverage 93.22% 93.25% +0.03%
======================================================================
Files 44 44
Lines 2922 2937 +15
======================================================================
+ Hits 2724 2739 +15
Misses 198 198
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
…n on failure Diagnostics carried the reading and any error, but never when a poll last completed, so "the endpoint has nothing newer" and "we have not asked recently" were indistinguishable from outside. outdoor_temp_last_poll_at is stamped on every path and read against outdoor_temp_recorded_at. A failed poll resets the 30-minute timer and keeps the previous value, and logged only at debug. A unit failing every poll was therefore identical to an idle one at the level anyone runs in production. Entering a failure streak now warns once, naming the unit and that the sensor holds its old value; recovery logs once at info. Neither repeats while the state persists. outdoor_temp_last_error is cleared by any call that did not raise, including one that returned no reading, so it answers "did this raise", not "did this find anything". The new field is what separates those. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
Split into #286 (the energy window fix) and #287 (outdoor-temperature poll diagnostics). No single title covered both concerns. #286 carries the three energy commits and is the half already deployed and soaked on real hardware. #287 carries the diagnostics field and the failure-streak warning, plus the stale gating-comment cleanup, and has not run on a real installation. The ADR-023 provenance commit that was sitting here has moved to #283, which already amends that ADR. |
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
1 participant
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.
Summary
Sits on #283 and needs nothing from it; the documentation correction here amends ADR-023, which #275 introduces. Energy requests floor the
frombound to the hour and normalise both bounds to UTC, so the oldest hourly bucket is always requested whole: the server sums only the samples at or afterfrom, and a bound inside the hour returns a partial slice of that bucket which shrinks as the poll clock advances. The measurements and the UTC-label evidence are in the docstring ofEnergyTrackerBase._energy_window. Two further commits make the outdoor-temperature poll observable from outside: diagnostics gain the time of the last poll, and a unit whose poll starts failing says so once. The rest correct comments and documentation that no longer describe the code.Key changes
EnergyTrackerBase._energy_windowcomputes the request window, floorsfromto the hour, and converts both bounds withastimezone(UTC). A naivenowraises. The ATA and ATW trackers both take their window from it.sensor_ata.pyanddocs/entities.md: the ATA outdoor-temperature sensor is created for every unit, and both now say so.telemetry_tracker.py: a clause namingsensor_atw.pyas where the zone-2 and boiler sensors are gated, so the request filter here reads as the cache-hygiene measure it is.outdoor_temp_last_poll_at, stamped on every path. Read againstoutdoor_temp_recorded_atit separates "the endpoint has nothing newer" from "we have not asked recently".outdoor_temp_last_erroranswers neither, being cleared by any call that did not raise, including one that returned no reading.What changes for users
Risks accepted
/report/v1/on the same account labels its points in the unit's own zone (ADR-022). If some units label these buckets locally at an offset that is not a whole number of hours, the floored bound lands mid-bucket and their warnings persist. That persistence is the detection; the undo is a revert of the two energy commits.AI Disclosure
Testing
make pre-commit: all hooks passed.make test-api: 395 passed, 1 skipped, 1 xfailed, 16 deselected.make test-integration: 259 passed, 1 warning.make test-e2e: 16 passed, 397 deselected.tests/integration/test_coordinator_energy.py: the energy request'sfrombound is floored to the hour and the window only ever widens, and a local-awarenowyields the same window as its UTC equivalent while a naive one raises. Intests/integration/test_outdoor_temperature_sensor.py: a failing outdoor-temperature poll warns exactly once per failure streak._energy_windowdocstring..pyand.jsonfiles verified byte-identical by per-file SHA-256 to the branch as it stood at that commit.