fix: outlier filter not working properly

Author: bramstroker

custom_components/powercalc/sensors/energy.py

@@ -58,6 +58,16 @@
 _LOGGER = logging.getLogger(__name__)
 
 
+def _numeric_state_value(state: State | None) -> float | None:
+    """Return the numeric value of a state, or None when it is not a usable number."""
+    if state is None or state.state in (STATE_UNKNOWN, STATE_UNAVAILABLE):
+        return None
+    try:
+        return float(state.state)
+    except TypeError, ValueError:
+        return None
+
+
 def create_energy_sensor(
     hass: HomeAssistant,
     sensor_config: ConfigType,
@@ -290,27 +300,86 @@ def __init__(
             max_z_score=3.5,
             max_expected_step=sensor_config.get(CONF_ENERGY_FILTER_OUTLIER_MAX, 1000),
         )
+        self._last_accepted_value: float | None = None
+        self._last_rejected_value: float | None = None
 
     def _integrate_on_state_change(self, *args: Any, **kwargs: Any) -> None:  # noqa: ANN401
-        """Override to add outlier filtering."""
-
-        new_state: State | None = kwargs.get("new_state")
-        if new_state is None and args:
-            last_arg = args[-1]
-            if isinstance(last_arg, State):
-                new_state = last_arg
-
-        if self._filter_outliers and new_state is not None:
-            valid_state = new_state.state not in (STATE_UNKNOWN, STATE_UNAVAILABLE)
-            if valid_state and not self._outlier_filter.accept(float(new_state.state)):
-                _LOGGER.debug(
-                    "%s: Rejecting power value %s as outlier for energy integration",
-                    self.entity_id,
-                    new_state.state,
-                )
-                return
-
-        super()._integrate_on_state_change(*args, **kwargs)
+        """Override to add outlier filtering.
+
+        Simply skipping integration when an outlier arrives as the ``new_state`` is not
+        enough: the energy sensor integrates over consecutive states, and depending on the
+        integration method the outlier also contributes when it is the *old* state of the
+        following event. With the default ``left`` Riemann method the contribution of a
+        state change is ``old_state * elapsed_time``, so a rejected spike still leaks into
+        the total on the next update. Instead of skipping, we substitute any outlier reading
+        with the last accepted value wherever it appears (as old and as new state), so it can
+        never affect the energy total regardless of the integration method.
+        """
+        if not self._filter_outliers:
+            super()._integrate_on_state_change(*args, **kwargs)
+            return
+
+        arg_list = list(args)
+        state_positions = [index for index, value in enumerate(arg_list) if isinstance(value, State)]
+
+        # The integration sensor passes the states positionally (old_state, new_state being
+        # the last two). Sanitize old_state first, then new_state, since processing new_state
+        # updates the tracking used to detect the outlier as a subsequent old_state.
+        if len(state_positions) >= 2:
+            old_index = state_positions[-2]
+            arg_list[old_index] = self._replace_outlier_state(arg_list[old_index])
+        if state_positions:
+            new_index = state_positions[-1]
+            arg_list[new_index] = self._sanitize_new_state(arg_list[new_index])
+
+        super()._integrate_on_state_change(*arg_list, **kwargs)
+
+    def _schedule_max_sub_interval_exceeded_if_state_is_numeric(self, source_state: State | None) -> None:
+        """Prevent a rejected outlier from being integrated as the assumed constant value.
+
+        When ``max_sub_interval`` is configured (always the case for powercalc energy sensors)
+        the integration sensor keeps integrating the last known source state until a new state
+        change arrives. Substitute the outlier with the last accepted value so this fallback
+        does not leak the spike either.
+        """
+        if self._filter_outliers:
+            source_state = self._replace_outlier_state(source_state)
+        super()._schedule_max_sub_interval_exceeded_if_state_is_numeric(source_state)
+
+    def _sanitize_new_state(self, state: State | None) -> State | None:
+        """Feed a new state through the outlier filter, substituting rejected outliers."""
+        value = _numeric_state_value(state)
+        if value is None:
+            return state
+
+        if self._outlier_filter.accept(value):
+            self._last_accepted_value = value
+            self._last_rejected_value = None
+            return state
+
+        self._last_rejected_value = value
+        _LOGGER.debug(
+            "%s: Rejecting power value %s as outlier for energy integration",
+            self.entity_id,
+            state.state if state else value,
+        )
+        return self._replace_outlier_state(state)
+
+    def _replace_outlier_state(self, state: State | None) -> State | None:
+        """Replace a state holding the last rejected outlier value with the last accepted value."""
+        if state is None or self._last_rejected_value is None or self._last_accepted_value is None:
+            return state
+        if _numeric_state_value(state) != self._last_rejected_value:
+            return state
+        return State(
+            state.entity_id,
+            str(self._last_accepted_value),
+            state.attributes,
+            last_changed=state.last_changed,
+            last_reported=state.last_reported,
+            last_updated=state.last_updated,
+            context=state.context,
+        )
 
     @property
     def extra_state_attributes(self) -> dict[str, str] | None:

tests/sensors/test_energy.py

@@ -2,6 +2,7 @@
 import logging
 from unittest.mock import patch
 
+from freezegun.api import FrozenDateTimeFactory
 from homeassistant.components.sensor import ATTR_STATE_CLASS, SensorStateClass
 from homeassistant.components.utility_meter.sensor import SensorDeviceClass
 from homeassistant.const import (
@@ -528,3 +529,48 @@ async def test_outlier_filtering(hass: HomeAssistant, caplog: pytest.LogCaptureF
         ],
     )
     assert "Rejecting power value 7000000 as outlier for energy integration" in caplog.text
+
+
+async def test_outlier_filtering_does_not_leak_energy(
+    hass: HomeAssistant,
+    caplog: pytest.LogCaptureFixture,
+    freezer: FrozenDateTimeFactory,
+) -> None:
+    """Regression test for https://github.com/bramstroker/homeassistant-powercalc/issues/4279.
+
+    With the default `left` Riemann integration method the harmful contribution of an outlier
+    is not on its own state change (it becomes the new_state), but on the *following* state
+    change when it acts as the old_state. Ensure the spike never leaks into the energy total.
+    """
+    caplog.set_level(logging.DEBUG)
+
+    power_sensor_id = "sensor.test_power"
+    await run_powercalc_setup(
+        hass,
+        {
+            CONF_NAME: "Test",
+            CONF_POWER_SENSOR_ID: power_sensor_id,
+            CONF_ENERGY_FILTER_OUTLIER_ENABLED: True,
+        },
+    )
+
+    async def advance_and_set(value: str) -> None:
+        freezer.tick(timedelta(seconds=15))
+        await set_states(hass, [(power_sensor_id, value, {ATTR_UNIT_OF_MEASUREMENT: "W"})])
+
+    # Warm up the filter with a stable but slightly varied ~4W baseline (non-zero MAD)
+    for value in ["4.4", "4.1", "4.6", "4.2", "4.5", "4.4", "4.5", "4.1", "4.0", "4.2"]:
+        await advance_and_set(value)
+
+    energy_before = float(hass.states.get("sensor.test_energy").state)
+
+    # Spike to an unrealistic value and back to the baseline
+    await advance_and_set("6553.5")
+    await advance_and_set("4.0")
+
+    energy_after = float(hass.states.get("sensor.test_energy").state)
+
+    assert "Rejecting power value 6553.5 as outlier for energy integration" in caplog.text
+    # The spike (6553.5W integrated over 15s ~= 0.027 kWh) must not have leaked into the total.
+    # Only the ~4W baseline over the two intervals (~0.00003 kWh) may have accrued.
+    assert energy_after - energy_before < 0.001