fix: report price health from the cache instead of re-fetching every 5 minutes

[johanzander]

Summary

• The price health check made a live Nordpool service call every 5 minutes for data that changes once a day — 288 calls/day — so a single transient HA 500 produced a near-daily "recovered from an earlier issue" banner.
• It now reports from PriceManager's own today-cache, and probes the source only when that cache is cold.

Root cause

PriceManager.check_health() delegated unconditionally to price_source.perform_health_check(), and every remote source implements that as a live fetch of today's prices — official_nordpool_source.py:207-220, and identically in octopus_energy_source.py:257 and entsoe_source.py:204. It calls the source directly, bypassing PriceManager's cache.

refresh_health_check runs on CronTrigger(minute="*/5") (backend/app.py:442) and reaches the price source via health_check.py:372, so that probe fired 288 times a day. HealthRecoveryTracker (#239) fires a banner on every ERROR→OK transition, so one transient upstream 500 — the known-transient failure documented in #583 — flipped the component ERROR at one check and OK at the next, five minutes later. The reporter saw the banner most days for a system that held today's prices throughout and never missed an optimization.

Fix

Holding today's prices already answers the only question this check asks — can we optimize? So check_health() reports OK from the cache when it is warm, and probes the source only when it is cold: startup, date rollover, or clear_cache() after a settings/provider change. A cold probe that fails is still ERROR, unchanged — without prices the system genuinely cannot optimize, and that is what the banner should mean.

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 (raised by code review).

Scope: local. PriceSource.perform_health_check()'s contract is untouched and no source implementation changed — the fix lands at the single site that already owns the cache. No parameter, flag, fallback, extra trigger or second construction site was added; the diff removes redundant work rather than routing around it.

The mypy annotations in price_manager.py are #614's changed-files gate pulling this file's legacy backlog into scope (annotations only, no behaviour change).

Documentation check: docs/agents/bess-knowledge.md has no health-check section, and docs/SOFTWARE_DESIGN.md's "Health Monitoring" section describes the sensor severity model (determine_health_status, is_required/required_methods) which this change does not touch. Nothing to update.

Test plan

• ./scripts/quality-check.sh passes locally (fast suite 2191 passed, Black, Ruff, mypy, frontend, ESLint). The one remaining gate error is pre-existing and unrelated: permissions.ask is missing Bash(gh api) in committed .claude/settings.json.
• .venv/bin/pytest -m slow — 554 passed, 8 skipped.
• Observed live against the real backend + mock-HA stack (docker-compose.ci.yml, scenario dated to today so Nordpool date-anchored calls resolve), counting actual service calls via mock-HA's /mock/service_log:
  • With the fix: startup issues 3 calls (today ×2, tomorrow ×1); 3 subsequent POST /api/system-health/recheck calls issued 0 further nordpool.get_prices_for_date calls. /api/system-health reports Electricity Price Data: OK → "96 prices available for today".
  • Control (fix disabled in the same running stack, container restarted): the same 3 rechecks issued 3 more get_prices_for_date calls — one live call per health check, exactly the reported behaviour.

Evidence the test discriminates

• Written RED first: assert source.probe_count == 0 failed as assert 3 == 0 — three check_health() calls, three live probes for prices already held.
• After the fix: 13 passed in test_price_manager.py.
• The live control run above is the same mutation at the system level: 3 rechecks → 3 calls without the fix, 0 with it.

Outcome-level coverage

• test_health_check_reports_from_cache_without_probing_the_source pins the outcome that matters (zero live source calls when the cache is warm), not the shape of the returned dict.
• test_health_check_probes_the_source_when_today_is_not_cached and test_health_check_reports_error_when_the_cold_probe_fails pin that the cold path still probes and still reports ERROR — both passed before the fix and must keep passing, which is what stops this becoming "never probe".
• No fixture/golden/VPP-baseline changes: the diff touches no DP, intent classification, or control mapping, so no R == P scenario applies.

Closes #662

🤖 Generated with Claude Code

https://claude.ai/code/session_01Kf5wtkJiPQQ5tJnmfxQA3j

[bess-agent]

@claude-bot review

[bess-manager-claude-bot]

Review: PR #667

Matches root cause of #662: Yes. Issue #662 identifies that perform_health_check() on every price source does a live fetch (official_nordpool_source.py:207-220, same pattern in octopus_energy_source.py/entsoe_source.py), and refresh_health_check calls it every 5 minutes (backend/app.py:442), turning transient upstream 500s into daily error/recovery banners. The fix (price_manager.py's new _cached_today_prices() + check_health() short-circuit) reports OK from the warm cache and only probes the source when cold — exactly the proposed fix in the issue.

Would the test catch a regression? Yes, and it's outcome-based per docs/agents/testing.md. test_health_check_reports_from_cache_without_probing_the_source asserts source.probe_count == 0 across 3 check_health() calls after warming the cache — this fails loudly (3 == 0) if the fix regresses. test_health_check_probes_the_source_when_today_is_not_cached and test_health_check_reports_error_when_the_cold_probe_fails pin that a cold cache still probes and still reports ERROR, guarding against the fix silently becoming "never probe." The PR body documents the test was written RED first and backs it with an actual live mock-HA run (3 rechecks → 0 extra nordpool.get_prices_for_date calls with the fix, 3 without) — real evidence, not just unit coverage.

Scope: Minimal and correctly assessed as local. _cached_today_prices() is a small shared helper used by both get_price_data() and check_health() (avoiding the two notions of "cache is warm" drifting apart — a legitimate reason to touch get_price_data() too, not scope creep). No new parameter/flag/fallback/second construction site was added — this is a direct fix at the site that already owns the cache, not a workaround. Confirmed via git grep that every caller of get_price_data() passes only today/tomorrow/None, so the changed cache-lookup condition (target_date == time_utils.today() before calling _cached_today_prices(), vs. the old self._today_date == target_date) has no reachable behavior difference. The mypy annotations are pre-existing-file cleanup pulled in by #614's changed-files ratchet, as stated, and are typing-only.

Rule compliance: No Optional[x], no hasattr/getattr fallbacks, no new production classes, no exception-string matching, no hardcoded entity IDs. CountingSource in the test file extends the existing MockSource test double — a test fixture, not a production/architectural class, so it doesn't trigger the "no new classes" architecture rule in spirit. The multi-line comment above the check_health() cache short-circuit explains why (redundant probing turned transient failures into daily banners, referencing #662) rather than what, consistent with the comments rule.

No blockers found. APPROVE.