Audit 3/3: the evidence - red-first tests for every finding

tests/unit/adapters/test_a_forecast_from_six_hours_ago_is_not_a_forecast.py

@@ -0,0 +1,135 @@
+"""forecast_hours[N] must mean "N hours from now"; the adapter must make that true.
+
+Every consumer slices WeatherData.forecast_hours positionally (thermal_layer
+forecast_hours[:3] is the cold-snap trigger; weather_layer [:24]; prediction_layer
+[:horizon]). The adapter used to append every entry the weather entity published, in
+its published order, including hours already past - so a stalled-but-"available"
+integration (unavailable never trips) could hold stale weather at index 0 and push a
+real cold snap outside every horizon.
+
+get_forecast() now drops hours that have already ended and sorts the rest. A forecast
+entirely in the past becomes empty, and the layers abstain when there is no forecast.
+"""
+
+from __future__ import annotations
+
+from datetime import timedelta
+from unittest.mock import MagicMock
+
+import pytest
+from homeassistant.util import dt as dt_util
+
+from custom_components.effektguard.adapters.weather_adapter import WeatherAdapter
+from custom_components.effektguard.const import CONF_WEATHER_ENTITY
+
+# The clock is read INSIDE each test (fixture below), never at module import: a module-level NOW
+# captured at collection time would diverge from the adapter's run-time clock under a frozen clock.
+
+
+@pytest.fixture
+def now():
+    return dt_util.utcnow()
+
+
+# A cold snap arriving within the hour, behind six hours of stale mild weather.
+STALE_LEADING_HOURS = [(-6, 5.0), (-5, 4.0), (-4, 3.0), (-3, 2.0), (-2, 1.0), (-1, 0.0)]
+THE_COLD_SNAP = [(0, -1.0), (1, -8.0), (2, -14.0), (3, -18.0)]
+
+
+def _adapter(now, hours: list[tuple[int, float]]) -> WeatherAdapter:
+    state = MagicMock()
+    state.state = "cloudy"
+    state.attributes = {
+        "temperature": -1.0,
+        "temperature_unit": "°C",
+        "forecast": [
+            {
+                "datetime": (now + timedelta(hours=offset)).isoformat(),
+                "temperature": temp,
+                "condition": "cloudy",
+            }
+            for offset, temp in hours
+        ],
+    }
+    hass = MagicMock()
+    hass.states.get.return_value = state
+    return WeatherAdapter(hass, {CONF_WEATHER_ENTITY: "weather.home"})
+
+
+@pytest.mark.asyncio
+async def test_the_first_forecast_hour_is_actually_in_the_future(now):
+    data = await _adapter(now, STALE_LEADING_HOURS + THE_COLD_SNAP).get_forecast()
+
+    first = data.forecast_hours[0]
+    hours_away = (first.datetime - now).total_seconds() / 3600
+
+    assert hours_away > -1.0, (
+        f"forecast_hours[0] is {hours_away:+.0f} hours from now, and reads {first.temperature:+.1f} "
+        f"C. Every layer slices this list positionally and treats index 0 as the next hour - so the "
+        f"cold-snap trigger was reading the weather from this morning."
+    )
+
+
+@pytest.mark.asyncio
+async def test_the_cold_snap_is_inside_the_three_hour_trigger_window(now):
+    """The whole point. thermal_layer reads forecast_hours[:3] to decide whether cold is coming."""
+    data = await _adapter(now, STALE_LEADING_HOURS + THE_COLD_SNAP).get_forecast()
+
+    next_three = [hour.temperature for hour in data.forecast_hours[:3]]
+
+    assert min(next_three) < -5.0, (
+        f"The next three forecast hours read {next_three} C, and a cold snap reaching -18 C arrives "
+        f"within the hour. Six hours of already-past weather were sitting at the front of the list, "
+        f"pushing the snap out of every horizon the layers look at."
+    )
+
+
+@pytest.mark.asyncio
+async def test_the_past_hours_are_dropped_entirely(now):
+    data = await _adapter(now, STALE_LEADING_HOURS + THE_COLD_SNAP).get_forecast()
+
+    assert len(data.forecast_hours) == len(THE_COLD_SNAP)
+    assert all((hour.datetime - now).total_seconds() / 3600 > -1.0 for hour in data.forecast_hours)
+
+
+@pytest.mark.asyncio
+async def test_the_hours_come_back_in_order(now):
+    """A positional read is meaningless on an unsorted list, and nothing guaranteed the order."""
+    shuffled = [THE_COLD_SNAP[2], THE_COLD_SNAP[0], THE_COLD_SNAP[3], THE_COLD_SNAP[1]]
+
+    data = await _adapter(now, shuffled).get_forecast()
+    times = [hour.datetime for hour in data.forecast_hours]
+
+    assert times == sorted(times)
+    assert data.forecast_hours[0].temperature == pytest.approx(-1.0)
+
+
+@pytest.mark.asyncio
+async def test_a_forecast_entirely_in_the_past_is_no_forecast_at_all(now):
+    """A stalled weather integration stays 'available' forever. It must not drive the pre-heat."""
+    data = await _adapter(now, STALE_LEADING_HOURS).get_forecast()
+
+    assert data is None, (
+        "Every hour this weather entity published has already passed - it has stalled, and its "
+        "entity is still 'available', so the existing unavailable-check never trips. Driving the "
+        "pre-heat on it means pre-heating for weather that has already happened. The layers already "
+        "abstain when there is no forecast, which is the correct behaviour here."
+    )
+
+
+@pytest.mark.asyncio
+async def test_a_healthy_forecast_is_untouched(now):
+    """The regression guard."""
+    data = await _adapter(now, THE_COLD_SNAP).get_forecast()
+
+    assert [hour.temperature for hour in data.forecast_hours] == pytest.approx(
+        [temp for _, temp in THE_COLD_SNAP]
+    )
+
+
+@pytest.mark.asyncio
+async def test_the_current_hour_is_kept(now):
+    """A period that began forty minutes ago is still the weather now, not a memory."""
+    data = await _adapter(now, [(0, -1.0), (1, -8.0)]).get_forecast()
+
+    assert len(data.forecast_hours) == 2

tests/unit/adapters/test_a_missing_price_is_not_a_free_hour.py

@@ -0,0 +1,102 @@
+"""A GE-Spot entry with no `value` must be dropped, never defaulted to a price.
+
+`_parse_periods` reads `float(item["value"])`: a missing `value` raises KeyError and the
+interval is dropped. It must never become `.get("value", 0.0)` - 0.0 is the cheapest
+possible price, so a data-less quarter would rank best of the day, classify VERY_CHEAP
+(PRICE_OFFSET_VERY_CHEAP is +4.0 C, aggressive pre-heating), and drive the pump hardest
+in the interval nobody sent a price for. Zero is also a real Nordic price, so a fabricated
+0.0 is indistinguishable from a genuinely free quarter after the fact.
+
+Dropped intervals are located by timestamp, so a gap means that quarter has no price and
+the price layer abstains; a wholly empty day trips the no-price-source path.
+"""
+
+from __future__ import annotations
+
+from datetime import timedelta
+from unittest.mock import MagicMock
+
+import pytest
+from homeassistant.util import dt as dt_util
+
+from custom_components.effektguard.adapters.gespot_adapter import GESpotAdapter
+from custom_components.effektguard.optimization.price_layer import (
+    PriceAnalyzer,
+    QuarterClassification,
+)
+
+
+def _adapter() -> GESpotAdapter:
+    return GESpotAdapter(MagicMock(), {"gespot_entity": "sensor.gespot"})
+
+
+def _raw_day(broken_at: int | None = None) -> list[dict[str, object]]:
+    """A realistic SE4 day. One entry may arrive without its `value` key."""
+    base = dt_util.now().replace(hour=0, minute=0, second=0, microsecond=0)
+    day: list[dict[str, object]] = []
+
+    for i in range(96):
+        item: dict[str, object] = {"time": (base + timedelta(minutes=15 * i)).isoformat()}
+        if i != broken_at:
+            item["value"] = 40.0 + 50.0 * (i % 24) / 24.0
+        day.append(item)
+
+    return day
+
+
+def test_a_good_day_parses():
+    """The precondition. If this fails, the parser is rejecting everything."""
+    periods = _adapter()._parse_periods(_raw_day())
+
+    assert len(periods) == 96
+    assert min(p.price for p in periods) >= 40.0
+
+
+def test_an_entry_with_no_price_is_dropped_not_invented():
+    """The interval has no price. That is not the same as a price of zero."""
+    periods = _adapter()._parse_periods(_raw_day(broken_at=50))
+
+    assert len(periods) == 95, (
+        "A GE-Spot entry with no `value` key was still turned into a price period. "
+        f"`.get('value', 0.0)` invented 0.0 for it - and 0.0 is the cheapest possible price."
+    )
+    assert all(p.price != 0.0 for p in periods), (
+        "A fabricated 0.0 öre survived into the parsed day. Zero is a REAL Nordic price (~100 h a "
+        "year per SE zone), so nothing downstream can ever tell it apart from a genuinely free "
+        "quarter."
+    )
+
+
+def test_a_quarter_with_no_data_is_not_the_best_quarter_of_the_day():
+    """The consequence, end to end: no data ranks as the cheapest hour there is."""
+    periods = _adapter()._parse_periods(_raw_day(broken_at=50))
+
+    classes = PriceAnalyzer().classify_quarterly_periods(periods)
+
+    assert QuarterClassification.VERY_CHEAP not in set(classes.values()) or all(
+        periods[i].price > 0.0 for i, c in classes.items() if c is QuarterClassification.VERY_CHEAP
+    ), (
+        "A quarter the adapter had no price for was classified VERY_CHEAP - the best quarter of the "
+        "day - because it was invented as 0.0. PRICE_OFFSET_VERY_CHEAP is +4.0 °C, 'aggressive "
+        "pre-heating'. The heat pump would be driven hardest in the interval nobody sent us a price "
+        "for."
+    )
+
+
+def test_a_day_where_every_price_is_missing_yields_no_day_at_all():
+    """The schema-change case: GE-Spot renames the key and every entry breaks.
+
+    All 96 intervals drop, `today` comes back empty, and the coordinator's no-price-source path
+    takes over: the price layer abstains entirely and a repair issue tells the user (F-123). That is
+    the correct outcome. The wrong one is 96 quarters of invented 0.0, every one of them VERY_CHEAP,
+    with the pump pre-heating aggressively around the clock.
+    """
+    broken = [{"time": item["time"]} for item in _raw_day()]
+
+    periods = _adapter()._parse_periods(broken)
+
+    assert periods == [], (
+        f"Every entry was missing its price and {len(periods)} periods came back anyway. If they "
+        f"are all invented zeros, every quarter of the day classifies VERY_CHEAP and the pump "
+        f"pre-heats aggressively, around the clock, on a day nobody sent us a single price for."
+    )

tests/unit/adapters/test_a_setpoint_is_not_a_measurement.py

@@ -0,0 +1,123 @@
+"""A NIBE room-temperature SETPOINT must not be discovered as the indoor MEASUREMENT.
+
+Discovery must reject `number.` entities for temperature keys (`_consider_candidate`
+requires `sensor.`). A `number.` is something the owner sets; a NIBE room setpoint is a
+`number.` with device_class temperature and unit C and can match the `room_temperature`
+pattern. Bound as the measurement it is silent and catastrophic: the target is read as
+the measurement with indoor_temp_valid=True, so the deviation from target is 0.0 forever,
+the comfort layer never corrects, and the 18 C safety floor (MIN_TEMP_LIMIT) never fires
+because it reads the same setpoint. Manual overrides bypass discovery, so a reading truly
+exposed as a `number.` can still be configured explicitly.
+"""
+
+from __future__ import annotations
+
+from unittest.mock import MagicMock
+
+import pytest
+
+from custom_components.effektguard.adapters.nibe_adapter import NibeAdapter
+from custom_components.effektguard.const import (
+    CONF_NIBE_ENTITY,
+    NIBE_DISCOVERY_PATTERNS,
+    NIBE_TEMPERATURE_KEYS,
+)
+
+
+def _adapter() -> NibeAdapter:
+    return NibeAdapter(MagicMock(), {CONF_NIBE_ENTITY: "number.offset"})
+
+
+def _consider(adapter: NibeAdapter, entity_id: str) -> dict[str, str]:
+    """Run the real discovery candidate check against one entity."""
+    adapter._entity_cache = {}
+    adapter._consider_candidate(
+        entity_id=entity_id,
+        device_class="temperature",
+        unit="°C",
+        rank=0,
+        ranks={},
+        claimed=set(),
+    )
+    return adapter._entity_cache
+
+
+def test_the_pattern_that_makes_this_reachable_is_still_there():
+    """The premise. `room_temperature` matches a setpoint's entity id just as well as a sensor's."""
+    assert "room_temperature" in NIBE_DISCOVERY_PATTERNS["indoor_temp"]
+    assert "indoor_temp" in NIBE_TEMPERATURE_KEYS
+
+
+@pytest.mark.parametrize(
+    "setpoint",
+    [
+        "number.nibe_room_temperature_setpoint_s1",
+        "number.f750_room_temperature_s1_47398",
+        "number.heatpump_room_temperature",
+    ],
+)
+def test_a_writable_setpoint_is_never_bound_as_the_indoor_measurement(setpoint):
+    cache = _consider(_adapter(), setpoint)
+
+    assert "indoor_temp" not in cache, (
+        f"Discovery bound {setpoint} - a WRITABLE setpoint, something the owner sets - as the "
+        f"indoor temperature MEASUREMENT. The target is then read as the measurement with "
+        f"indoor_temp_valid=True, so the deviation from target is exactly 0.0 forever, the comfort "
+        f"layer never corrects, and the 18 C safety floor can never fire because it is reading the "
+        f"same setpoint. A house at 12 C in January would report itself perfectly on target."
+    )
+
+
+@pytest.mark.parametrize(
+    ("entity_id", "key"),
+    [
+        ("sensor.nibe_bt50_room_temperature", "indoor_temp"),
+        ("sensor.nibe_bt1_outdoor_temperature", "outdoor_temp"),
+        ("sensor.nibe_bt25_supply_temperature", "supply_temp"),
+    ],
+)
+def test_a_real_sensor_is_still_discovered(entity_id, key):
+    """The regression guard. Do not break discovery while hardening it."""
+    cache = _consider(_adapter(), entity_id)
+
+    assert cache.get(key) == entity_id, (
+        f"{entity_id} is an ordinary temperature sensor and discovery no longer finds it as "
+        f"{key}. The domain rule must reject setpoints, not measurements."
+    )
+
+
+def test_every_temperature_key_is_protected_not_just_the_indoor_one():
+    """A setpoint bound as the SUPPLY temperature would drive weather compensation on a target."""
+    adapter = _adapter()
+
+    for key in NIBE_TEMPERATURE_KEYS:
+        patterns = NIBE_DISCOVERY_PATTERNS.get(key, [])
+        if not patterns:
+            continue
+        entity_id = f"number.nibe{patterns[0]}_setpoint"
+        cache = _consider(adapter, entity_id)
+
+        assert key not in cache, (
+            f"A `number.` entity matching the {key} pattern was bound as a {key} MEASUREMENT. "
+            f"Every temperature key reads a value the pump reports; none of them is something the "
+            f"owner sets."
+        )
+
+
+def test_the_write_target_still_has_to_be_a_number():
+    """The mirror-image rule, which this file already had. It must survive."""
+    adapter = _adapter()
+    adapter._entity_cache = {}
+    adapter._consider_candidate(
+        entity_id="sensor.nibe_heat_offset_s1_47011",
+        device_class=None,
+        unit=None,
+        rank=0,
+        ranks={},
+        claimed=set(),
+    )
+
+    assert "offset" not in adapter._entity_cache, (
+        "A `sensor.` was bound as the OFFSET write target. The write path calls number.set_value; "
+        "a sensor can never work."
+    )