Skip to content

Commit 6292989

Browse files
johanzanderclaude
andauthored
fix: report price health from the cache instead of re-fetching every 5 minutes (#667)
PriceManager.check_health() delegated unconditionally to price_source.perform_health_check(), and every source implements that as a live fetch of today's prices — bypassing PriceManager's own cache. With refresh_health_check on a */5 cron, that is 288 live Nordpool service calls a day for data that changes once a day. The cost is not just the calls. A single transient HA 500 flipped the "Electricity Price Data" component ERROR, and the next check five minutes later flipped it back, which HealthRecoveryTracker turned into a "recovered from an earlier issue" banner. Users saw one most days, for a system that had today's prices cached throughout and never missed an optimization. Holding today's prices already answers the only question this check asks, so report OK from the cache and probe the source only when the cache is cold — startup, date rollover, or clear_cache() after a settings or provider change. A cold probe that fails is still ERROR: without prices the system genuinely cannot optimize. get_price_data()'s today branch now goes through the same _cached_today_prices() helper, so the two notions of "the cache is warm" cannot drift apart. Mypy annotations in price_manager.py are the changed-files gate from #614 pulling this file's legacy backlog into scope; they are annotations only, no behaviour change. Closes #662 Claude-Session: https://claude.ai/code/session_01Kf5wtkJiPQQ5tJnmfxQA3j Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0dd7199 commit 6292989

3 files changed

Lines changed: 159 additions & 26 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
2222

2323
### Fixed
2424

25+
- **A brief Nordpool hiccup no longer produces a daily "recovered from an earlier issue" notice** — the health check re-fetched today's prices every five minutes instead of using the ones already held. ([#662](https://github.com/johanzander/bess-manager/issues/662))
2526
- **Growatt VPP now lets the inverter and BMS sleep through a long idle at minimum SoC** — an empty battery was still held under continuous remote control, which nothing was protecting. ([#592](https://github.com/johanzander/bess-manager/issues/592))
2627
- **A tiny solar surplus is no longer planned as an export the inverter will absorb** — below the export the plan can express, the battery charged anyway and ran fuller than planned, spilling the difference later. ([#630](https://github.com/johanzander/bess-manager/issues/630))
2728
- **The setup wizard no longer locks you out of an inverter platform it failed to auto-detect** — every platform stays selectable, and a re-scan keeps the one you picked. ([#621](https://github.com/johanzander/bess-manager/issues/621))

core/bess/price_manager.py

Lines changed: 53 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import logging
88
from datetime import date, datetime, timedelta
9+
from typing import Any
910

1011
from . import time_utils
1112
from .exceptions import PriceDataUnavailableError, SystemConfigurationError
@@ -134,7 +135,7 @@ class HomeAssistantSource(PriceSource):
134135

135136
def __init__(
136137
self,
137-
ha_controller,
138+
ha_controller: Any,
138139
vat_multiplier: float,
139140
entity: str,
140141
) -> None:
@@ -197,7 +198,7 @@ def get_prices_for_date(self, target_date: date) -> list:
197198
message=f"Failed to get price data for {target_date}: {e}",
198199
) from e
199200

200-
def _fetch_sensor_attributes(self, entity_id: str):
201+
def _fetch_sensor_attributes(self, entity_id: str) -> dict | None:
201202
"""Fetch attributes from the specified Nordpool sensor entity.
202203
203204
Args:
@@ -214,11 +215,14 @@ def _fetch_sensor_attributes(self, entity_id: str):
214215
if not response or "attributes" not in response:
215216
return None
216217

217-
return response["attributes"]
218+
attributes: dict = response["attributes"]
219+
return attributes
218220
except Exception:
219221
return None
220222

221-
def _extract_prices_for_date(self, sensor_attributes, target_date, sensor_name):
223+
def _extract_prices_for_date(
224+
self, sensor_attributes: dict | None, target_date: date, sensor_name: str
225+
) -> list | None:
222226
"""Extract prices for a specific date from sensor attributes."""
223227
if not sensor_attributes:
224228
return None
@@ -251,7 +255,9 @@ def _extract_prices_for_date(self, sensor_attributes, target_date, sensor_name):
251255
except Exception:
252256
return None
253257

254-
def _parse_raw_data_for_date(self, raw_data, target_date):
258+
def _parse_raw_data_for_date(
259+
self, raw_data: list | None, target_date: date
260+
) -> list | None:
255261
"""Parse raw data and return prices if it matches target_date."""
256262
if not raw_data or not isinstance(raw_data, list) or not raw_data:
257263
return None
@@ -290,7 +296,7 @@ def _parse_raw_data_for_date(self, raw_data, target_date):
290296
except Exception:
291297
return None
292298

293-
def _handle_dst_transitions(self, prices):
299+
def _handle_dst_transitions(self, prices: list) -> list:
294300
"""Handle DST transitions for quarterly resolution (92-100 periods).
295301
296302
Nordpool provides quarterly prices (15-minute intervals):
@@ -312,7 +318,9 @@ def _handle_dst_transitions(self, prices):
312318
message=f"Unexpected price count: {len(prices)} periods. Expected 92-100 for quarterly resolution with DST."
313319
)
314320

315-
def _get_sensor_diagnostic_info(self, sensor_data, sensor_name):
321+
def _get_sensor_diagnostic_info(
322+
self, sensor_data: dict | None, sensor_name: str
323+
) -> str:
316324
"""Get simple diagnostic information about sensor data availability."""
317325
if not sensor_data:
318326
return "no data"
@@ -415,12 +423,12 @@ def __init__(
415423
self._logger = logging.getLogger(__name__)
416424

417425
# Cache for today's prices
418-
self._today_prices = None
419-
self._today_date = None
426+
self._today_prices: list[dict[str, Any]] | None = None
427+
self._today_date: date | None = None
420428

421429
# Cache for tomorrow's prices
422-
self._tomorrow_prices = None
423-
self._tomorrow_date = None
430+
self._tomorrow_prices: list[dict[str, Any]] | None = None
431+
self._tomorrow_date: date | None = None
424432

425433
def clear_cache(self) -> None:
426434
"""Clear cached price data.
@@ -474,8 +482,10 @@ def get_price_data(self, target_date: date | None = None) -> list:
474482
target_date = time_utils.today()
475483

476484
# Use cached values for today if available
477-
if self._today_date == target_date and self._today_prices is not None:
478-
return self._today_prices
485+
if target_date == time_utils.today():
486+
cached_today = self._cached_today_prices()
487+
if cached_today is not None:
488+
return cached_today
479489

480490
# Use cached values for tomorrow if available
481491
if self._tomorrow_date == target_date and self._tomorrow_prices is not None:
@@ -698,6 +708,12 @@ def log_price_information(self, title: str | None = None) -> None:
698708
except Exception as e:
699709
self._logger.warning(f"Failed to log price information: {e}")
700710

711+
def _cached_today_prices(self) -> list | None:
712+
"""Return today's cached price entries, or None if the cache is cold."""
713+
if self._today_date == time_utils.today() and self._today_prices is not None:
714+
return self._today_prices
715+
return None
716+
701717
def check_health(self) -> list:
702718
"""Check price management capabilities."""
703719

@@ -710,6 +726,30 @@ def check_health(self) -> list:
710726
"last_run": datetime.now().isoformat(),
711727
}
712728

729+
# Today's prices already in hand answer the only question this check
730+
# asks — can we optimize? Every source implements perform_health_check()
731+
# as a live fetch of today, so probing again would re-request data that
732+
# changes once a day, on a five-minute schedule. That is what turned a
733+
# transient upstream failure into a daily error/recovery banner (#662).
734+
# A cold cache (startup, date rollover, or clear_cache() after a
735+
# settings or provider change) still probes for real, below.
736+
cached_today = self._cached_today_prices()
737+
if cached_today is not None:
738+
price_check.update(
739+
{
740+
"status": "OK",
741+
"checks": [
742+
{
743+
"name": "Electricity Prices",
744+
"status": "OK",
745+
"error": None,
746+
"value": f"{len(cached_today)} prices available for today",
747+
}
748+
],
749+
}
750+
)
751+
return [price_check]
752+
713753
# Get health check from price source
714754
try:
715755
source_health = self.price_source.perform_health_check()

core/bess/tests/unit/test_price_manager.py

Lines changed: 105 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@
22
Test the PriceManager implementation.
33
"""
44

5-
from datetime import timedelta
5+
from datetime import date, timedelta
66
from unittest.mock import MagicMock, patch
77

88
from core.bess import time_utils
9+
from core.bess.exceptions import PriceDataUnavailableError
910
from core.bess.price_manager import HomeAssistantSource, MockSource, PriceManager
1011

1112

12-
def test_direct_price_initialization():
13+
def test_direct_price_initialization() -> None:
1314
"""Test initialization with direct prices."""
1415
mock_source = MockSource([1.0, 2.0, 3.0, 4.0])
1516
pm = PriceManager(
@@ -36,7 +37,7 @@ def test_direct_price_initialization():
3637
assert pm.get_sell_prices() == pm.sell_prices
3738

3839

39-
def test_spot_multiplier_applied_to_buy_price():
40+
def test_spot_multiplier_applied_to_buy_price() -> None:
4041
"""Multiplicative spot adjustment must apply before markup/VAT (Luminus-style contracts)."""
4142
mock_source = MockSource([1.0])
4243
pm = PriceManager(
@@ -57,7 +58,7 @@ def test_spot_multiplier_applied_to_buy_price():
5758
assert pm.sell_prices[0] == expected_sell_price
5859

5960

60-
def test_spot_multiplier_defaults_to_no_adjustment():
61+
def test_spot_multiplier_defaults_to_no_adjustment() -> None:
6162
"""Omitting spot_multiplier/export_spot_multiplier must reproduce the additive-only formula."""
6263
mock_source = MockSource([1.0])
6364
pm = PriceManager(
@@ -73,7 +74,7 @@ def test_spot_multiplier_defaults_to_no_adjustment():
7374
assert pm.sell_prices[0] == 1.0 + 0.2
7475

7576

76-
def test_controller_price_fetching():
77+
def test_controller_price_fetching() -> None:
7778
"""Test price fetching from controller."""
7879
mock_controller = MagicMock()
7980

@@ -98,7 +99,7 @@ def test_controller_price_fetching():
9899
for m in [0, 15, 30, 45]
99100
]
100101

101-
def mock_api_request(method, path):
102+
def mock_api_request(method: str, path: str) -> dict | None:
102103
if "sensor.nordpool_kwh_se4_sek_2_10_025" in path:
103104
# Return both today and tomorrow data for the same entity
104105
return {
@@ -156,7 +157,7 @@ def mock_api_request(method, path):
156157
assert tomorrow_prices[0]["sellPrice"] == tomorrow_expected_sell_price
157158

158159

159-
def test_mock_source():
160+
def test_mock_source() -> None:
160161
"""Test using a MockSource."""
161162
mock_source = MockSource([1.0, 2.0, 3.0, 4.0])
162163

@@ -184,7 +185,7 @@ def test_mock_source():
184185
assert today_prices[0]["sellPrice"] == expected_sell_price
185186

186187

187-
def test_home_assistant_source_vat_parameter():
188+
def test_home_assistant_source_vat_parameter() -> None:
188189
"""Test that the VAT multiplier parameter in HomeAssistantSource works correctly."""
189190
mock_controller = MagicMock()
190191

@@ -201,7 +202,7 @@ def test_home_assistant_source_vat_parameter():
201202
}
202203
)
203204

204-
def mock_api_request(method, path):
205+
def mock_api_request(method: str, path: str) -> dict | None:
205206
if "sensor.nordpool_kwh_se4_sek_2_10_025" in path:
206207
return {"attributes": {"raw_today": raw_today_data}}
207208
return None
@@ -229,7 +230,7 @@ def mock_api_request(method, path):
229230
assert round(prices_custom[0], 4) == round(2.0 / 1.20, 4) # ~1.6667
230231

231232

232-
def test_get_available_prices_today_only():
233+
def test_get_available_prices_today_only() -> None:
233234
"""Should return today's prices at quarterly resolution when tomorrow unavailable."""
234235
mock_source = MockSource(
235236
test_prices=[0.5] * 96
@@ -257,7 +258,7 @@ def test_get_available_prices_today_only():
257258
assert all(b == buy[0] for b in buy)
258259

259260

260-
def test_get_available_prices_today_and_tomorrow():
261+
def test_get_available_prices_today_and_tomorrow() -> None:
261262
"""Should return today + tomorrow at quarterly resolution when both available."""
262263
mock_source = MockSource(
263264
test_prices=[0.5] * 96
@@ -290,7 +291,7 @@ def test_get_available_prices_today_and_tomorrow():
290291
assert all(b == pm._calculate_buy_price(0.6) for b in buy[96:])
291292

292293

293-
def test_get_available_prices_returns_full_arrays_from_midnight():
294+
def test_get_available_prices_returns_full_arrays_from_midnight() -> None:
294295
"""Should return quarterly arrays starting from 00:00 (not current time)."""
295296
mock_source = MockSource(test_prices=[0.5] * 96)
296297
pm = PriceManager(
@@ -325,7 +326,7 @@ def test_get_available_prices_returns_full_arrays_from_midnight():
325326
assert buy[56] != buy[57] # Different quarters have different prices
326327

327328

328-
def test_get_available_prices_returns_tuple():
329+
def test_get_available_prices_returns_tuple() -> None:
329330
"""Should return a tuple of (buy_prices, sell_prices)."""
330331
mock_source = MockSource(
331332
test_prices=[0.5] * 96
@@ -351,3 +352,94 @@ def test_get_available_prices_returns_tuple():
351352
assert isinstance(sell, list)
352353
assert len(buy) == 96
353354
assert len(sell) == 96
355+
356+
357+
class CountingSource(MockSource):
358+
"""MockSource that records how often it is fetched and probed.
359+
360+
Both counters are the health check's real cost: every probe is a live
361+
service call to Home Assistant (#662).
362+
"""
363+
364+
def __init__(self, test_prices: list, fetch_fails: bool = False) -> None:
365+
super().__init__(test_prices)
366+
self.fetch_count = 0
367+
self.probe_count = 0
368+
self.fetch_fails = fetch_fails
369+
370+
def get_prices_for_date(self, target_date: date) -> list:
371+
self.fetch_count += 1
372+
if self.fetch_fails:
373+
raise PriceDataUnavailableError(
374+
date=target_date, message="Nordpool service call failed"
375+
)
376+
return self.test_prices
377+
378+
def perform_health_check(self) -> dict:
379+
self.probe_count += 1
380+
if self.fetch_fails:
381+
return {
382+
"status": "ERROR",
383+
"checks": [
384+
{
385+
"name": "CountingSource",
386+
"status": "ERROR",
387+
"error": "Nordpool service call failed",
388+
}
389+
],
390+
}
391+
return super().perform_health_check()
392+
393+
394+
def _counting_price_manager(source: CountingSource) -> PriceManager:
395+
return PriceManager(
396+
price_source=source,
397+
markup_rate=0.0,
398+
vat_multiplier=1.0,
399+
additional_costs=0.0,
400+
tax_reduction=0.0,
401+
area="SE4",
402+
)
403+
404+
405+
def test_health_check_reports_from_cache_without_probing_the_source() -> None:
406+
"""Holding today's prices leaves nothing for a live call to prove.
407+
408+
The health check runs every 5 minutes and the source probe is a live Home
409+
Assistant service call, so re-proving data that changes once a day is what
410+
turned a transient upstream 500 into a daily error/recovery banner (#662).
411+
"""
412+
source = CountingSource([1.0] * 96)
413+
pm = _counting_price_manager(source)
414+
415+
pm.get_today_prices()
416+
fetches_after_warmup = source.fetch_count
417+
418+
for _ in range(3):
419+
result = pm.check_health()
420+
421+
assert result[0]["status"] == "OK"
422+
assert source.probe_count == 0
423+
assert source.fetch_count == fetches_after_warmup
424+
425+
426+
def test_health_check_probes_the_source_when_today_is_not_cached() -> None:
427+
"""A cold cache has nothing to report from, so the probe must still happen."""
428+
source = CountingSource([1.0] * 96)
429+
pm = _counting_price_manager(source)
430+
431+
result = pm.check_health()
432+
433+
assert result[0]["status"] == "OK"
434+
assert source.probe_count == 1
435+
436+
437+
def test_health_check_reports_error_when_the_cold_probe_fails() -> None:
438+
"""Without today's prices the system genuinely cannot optimize — still ERROR."""
439+
source = CountingSource([1.0] * 96, fetch_fails=True)
440+
pm = _counting_price_manager(source)
441+
442+
result = pm.check_health()
443+
444+
assert result[0]["status"] == "ERROR"
445+
assert source.probe_count == 1

0 commit comments

Comments
 (0)