Merge pull request #356 from mbillow/claude/alert-read-action-guidance-2lwu5a

Fix stuck alarm_code: never merge /alarms/vs/0 onto stale cache

custom_components/localthings/observe.py

@@ -56,6 +56,26 @@ SUCCESS_FRACTION = 0.8
 PUSH_HEALTH_WINDOW_S = 60.0
 
 
+def _is_alarms_href(href: str) -> bool:
+    """True for /alarms/vs/<index> in any subdevice-translated shape --
+    the canonical MAIN form (/alarms/vs/0), an indexed subdevice's
+    renumbered instance (/alarms/vs/<key>), or a prefixed subdevice's
+    UUID-qualified form (/<uuid>/alarms/vs/0). `Subdevice.to_actual`
+    (registry/subdevices.py) only ever rewrites the trailing index
+    segment or prepends a prefix -- it never touches the 'alarms/vs'
+    stem -- so matching that fixed segment plus a wildcard tail catches
+    every shape without this module needing to be subdevice-aware.
+
+    See `ObserveManager.apply`'s use of this for why the href matters:
+    unlike most resources, /alarms/vs/0's `x.com.samsung.da.items` array
+    is a complete snapshot of every currently-active alarm, not a
+    possibly-partial field update -- so it must never be merged onto a
+    stale prior rep (issue #348).
+    """
+    head, _, _ = href.rpartition("/")
+    return head.endswith("/alarms/vs")
+
+
 class ObserveManager:
     """Per-device observe-mode state: mode, write-settle guard, and (later)
     subscription/staleness tracking. Pure sync logic — safe to call from
@@ -122,6 +142,20 @@ class ObserveManager:
         comes through, even though nothing about the device's actual
         supported options changed.
 
+        `_is_alarms_href` is the one exception to that merge (issue #348):
+        /alarms/vs/0's `items` array is always sent as a complete
+        snapshot of every currently-active alarm, never a partial delta
+        -- confirmed by a live `read_resource` GET returning `{}` (no
+        `items` key at all) the moment a washer's board actually clears
+        an alarm, which entity.py already documents as this resource's
+        normal no-alarm shape. Merging that `{}` onto the prior rep the
+        same way as everywhere else silently kept the stale `items`
+        entry forever: an absent key merges as "unchanged" everywhere
+        else, but on this href absent specifically means "cleared".
+        Every family that exposes an alarm sensor shares this href
+        (common.ALARMS, range_hood's own copy), so this is a full
+        replace for all of them, not a washer-specific carve-out.
+
         `apply()` is the sole path StateCache mutations flow through in
         this component (poll, sweep, and OBSERVE notify all funnel here),
         so `_cache_lock` serializes the read-then-write across those
@@ -149,7 +183,7 @@ class ObserveManager:
             self.log.debug("dropping %s update for %s (settling)", source, href)
             return False
         with self._cache_lock:
-            merged = {**(self.cache.get(href) or {}), **rep}
+            merged = dict(rep) if _is_alarms_href(href) else {**(self.cache.get(href) or {}), **rep}
             changed = self.cache.apply_rep(href, merged, source=source)
         # Outside the cache lock -- the hook takes locks of its own and
         # never reads the cache back. `source` is passed along rather than

tests/localthings/test_observe.py

@@ -58,6 +58,49 @@ def test_apply_merges_partial_update_onto_prior_rep():
     assert cached["x.com.samsung.da.supportedOptions"] == ["CV_FDR_WINE", "CV_FDR_MEAT"]
 
 
+def test_apply_fully_replaces_alarms_href_instead_of_merging():
+    """Regression test for issue #348: /alarms/vs/0's `items` array is a
+    complete snapshot of every currently-active alarm, not a partial field
+    update like /mode/vs/0 (issue #27). A washer's board reports a cleared
+    alarm by omitting `items` entirely -- a live read_resource GET showed
+    `{}` -- so merging that onto the prior rep (as every other href does)
+    left the stale ErrorCode_DC entry in the cache forever. This must
+    instead behave like a full replace: the empty rep wins outright."""
+    mgr = _manager()
+    active = {
+        "x.com.samsung.da.items": [
+            {"x.com.samsung.da.code": "ErrorCode_DC", "x.com.samsung.da.state": "Created"}
+        ]
+    }
+    mgr.apply("/alarms/vs/0", active, source="poll")
+    assert mgr.cache.get("/alarms/vs/0") == active
+
+    cleared = mgr.apply("/alarms/vs/0", {}, source="poll")
+
+    assert cleared is True
+    assert mgr.cache.get("/alarms/vs/0") == {}
+
+
+def test_apply_fully_replaces_alarms_href_for_subdevice_shapes():
+    """The same full-replace behavior must hold for both hrefs
+    `Subdevice.to_actual` can produce: an indexed subdevice renumbers only
+    the trailing '0' (/alarms/vs/1), and a prefixed one prepends a UUID
+    (/<uuid>/alarms/vs/0) -- neither ever touches the 'alarms/vs' stem
+    itself (registry/subdevices.py)."""
+    for href in ("/alarms/vs/1", "/6c2dff6d-ee5c-dad1-6a5e-000000000001/alarms/vs/0"):
+        mgr = _manager()
+        mgr.apply(
+            href,
+            {"x.com.samsung.da.items": [{"x.com.samsung.da.code": "ErrorCode_UB"}]},
+            source="poll",
+        )
+
+        cleared = mgr.apply(href, {}, source="poll")
+
+        assert cleared is True
+        assert mgr.cache.get(href) == {}
+
+
 def test_apply_drops_update_during_settle_window():
     mgr = _manager()
     mgr.cache.apply_rep("/oven/vs/0", {"a": 1}, source="seed")