Phase 2: usage-hour over-counting, HVAC accuracy, and the vanishing calendar

[bhaggs]

Phase 2 of the code review. Four independent problems, each with a regression test written first and confirmed failing.

H2 — downtime counted as runtime

Usage state was reconciled against the tracked entity only for brand-new filters (_last_usage_changed is None). But changes while HA is down raise no events, and async_track_state_change_event doesn't fire for states that already exist when the listener attaches. A fan that was on at shutdown kept accruing across the whole outage:

assert 600.16 == 1.0 ± 0.1              # displayed hours
downtime was committed to storage: 2160591.838692s   # ~25 days, permanently

The second one is the worse half — the bogus total was written to storage at the entity's next real state change.

Startup now always reconciles against live state and discards the gap. There's no evidence about what the entity did while nothing was watching, and crediting it was strictly worse than dropping it. This under-counts brief restarts; that's the conservative direction and a deliberate trade.

HVAC accuracy

Climate tracking counted hvac_mode, so a thermostat left on heat all winter accrued 24/7 while the furnace actually ran an hour a day — the single worst case for the main furnace-filter use case. Now prefers hvac_action when reported, falling back to the mode when not.

The derived "is running" flag is persisted alongside the raw state, because for climate entities the state string alone can't answer the question. Added without a store version bump: absence reads as None and falls back to the old rule. A bump without an async_migrate_func would make Store read the whole file as empty and lose the user's hours.

Since climate entities emit a state-change event for every attribute update, the handler now ignores changes that don't flip the running flag rather than writing storage each time the current temperature ticks.

H3 — every entity became a usage tracker

All seven entities receive the config-update signal, and the resubscribe path keyed off a field that was None for six of them. Changing the usage sensor made all seven subscribe and write the same store, racing read-modify-write on one file. Measured at 8 writes for one state change; now exactly 1.

H5 — the vanishing calendar

Guarded by a bare "already created" flag that only cleared when the last entry unloaded. Reloading the owning entry left the calendar permanently unavailable while other filters remained; deleting it lost the calendar entirely. Ownership is now tracked by entry id, released on unload, and re-homed onto a surviving filter on removal.

Worth flagging for review: HA does not delete the calendar on unload — it marks it unavailable with restored=True. My first version of this test asserted the entity merely existed and passed against the bug. The assertions check availability.

M7/M8 — thread safety, both directions

The dispatcher handlers were plain sync functions, so HA ran them in an executor thread — which is why they needed the thread-safe schedule_update_ha_state. They're now marked @callback and use async_write_ha_state. The async state-change handler had the inverse problem: a @callback decorator it should never have had, working only because HA checks iscoroutinefunction first.

Tests

39 total, up from 28. New: test_usage_tracking.py, test_calendar_ownership.py, and two fixtures (a filter active at shutdown, a climate-tracked furnace filter). The upgrade gate still passes, so v0.5.0 stores load with their hours intact.

Still open

The options flow still uses bespoke dispatchers rather than HA's reload pattern (Phase 3), and the efficiency items in Phase 4 — should_poll, delayed saves, midnight refresh.

🤖 Generated with Claude Code