Address review findings on the learned-modes store

Flatten the store to {href: [codes]}. One href carries one LEARNABLE
rule, so keying the codes by the rule's supported field too let the
write side (rule.supported_field) and both read sides (the module-level
SUPPORTED_FIELD) disagree the moment a rule used a different field --
codes learned and persisted, then never offered.

The options flow's reset step read the persisted value raw in the
entry-not-loaded branch, so malformed data aborted the one screen that
can clear it; route it through LearnedModes like every other reader.
For the same reason forget_learned_modes() now persists whenever the
entry carries a record, not only when the in-memory store had one: a
record _coerce rejected at startup exists only on the entry.

custom_components/localthings/config_flow.py

@@ -61,6 +61,7 @@ from .const import (
     PROBE_PORT_RANGE,
     SERVICE_WRITE_RESOURCE,
 )
+from .learned import LearnedModes
 
 _TEXT = TextSelector(TextSelectorConfig(type=TextSelectorType.TEXT))
 _MULTILINE = TextSelector(TextSelectorConfig(type=TextSelectorType.TEXT, multiline=True))
@@ -881,11 +882,12 @@ class LocalThingsOptionsFlow(config_entries.OptionsFlow):
         learned = (
             coord.learned_snapshot()
             if coord is not None
-            else self.config_entry.data.get(CONF_LEARNED_MODES) or {}
-        )
-        codes = sorted(
-            {code for fields in learned.values() for value in fields.values() for code in value}
+            # Through LearnedModes, not the raw entry value: this step is
+            # the one screen that can clear a malformed persisted record,
+            # so it must not be the one screen that trips over it.
+            else LearnedModes(self.config_entry.data.get(CONF_LEARNED_MODES)).snapshot()
         )
+        codes = sorted({code for codes in learned.values() for code in codes})
 
         if user_input is not None:
             if coord is not None:

custom_components/localthings/const.py

@@ -38,7 +38,7 @@ CONF_DEVICE_TYPE = "device_type"
 # in the same resource's supportedModes (issue #327). Stored on the entry
 # rather than kept in memory so a mode the device only names while it is
 # active survives a restart -- see learned.py. Shape:
-# {actual_href: {supported_field: [code, ...]}}.
+# {actual_href: [code, ...]}.
 CONF_LEARNED_MODES = "learned_modes"
 
 # Options-flow key: whether learned modes are remembered and offered.

custom_components/localthings/coordinator.py

@@ -40,7 +40,7 @@ from .const import (
     DTLS_LOCAL_PORT_BASE,
     SUMMARY_INTERVAL_S,
 )
-from .learned import SUPPORTED_FIELD, LearnedModes
+from .learned import LearnedModes
 from .observe import GRACE_PERIOD_S, MODE_OBSERVE, MODE_POLL, ObserveManager
 from .registry import CAPABILITIES
 from .registry.adapter import flatten
@@ -322,7 +322,7 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
     def learning_enabled(self) -> bool:
         return bool(self._entry.options.get(CONF_LEARN_MODES, DEFAULT_LEARN_MODES))
 
-    def learned_modes(self, actual_href: str, field: str = SUPPORTED_FIELD) -> list[str]:
+    def learned_modes(self, actual_href: str) -> list[str]:
         """Codes learned for `actual_href`, or [] while the option is off.
 
         Gating the read here rather than only the write is what makes the
@@ -331,17 +331,22 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
         (the options flow's reset step is for that)."""
         if not self.learning_enabled:
             return []
-        return self._learned.codes(actual_href, field)
+        return self._learned.codes(actual_href)
 
-    def learned_snapshot(self) -> dict[str, dict[str, list[str]]]:
+    def learned_snapshot(self) -> dict[str, list[str]]:
         """Everything learned, option state ignored -- for diagnostics and
         the options flow, both of which need to show what is remembered
         even when it isn't currently being offered."""
         return self._learned.snapshot()
 
     def forget_learned_modes(self) -> None:
-        """Drop every learned mode, here and on the entry."""
-        if self._learned.clear():
+        """Drop every learned mode, here and on the entry.
+
+        Persists even when the in-memory store was already empty: a record
+        _coerce rejected at startup exists only on the entry, and this is
+        the one control that can clear it."""
+        self._learned.clear()
+        if self._entry.data.get(CONF_LEARNED_MODES):
             self._persist_learned()
 
     def _canonical_href(self, actual: str) -> str:

custom_components/localthings/learned.py

@@ -59,28 +59,25 @@ def _codes(value) -> list[str]:
     return []
 
 
-def _coerce(stored) -> dict[str, dict[str, list[str]]]:
+def _coerce(stored) -> dict[str, list[str]]:
     """Restore the persisted map, dropping anything that isn't the shape
     this module writes. It round-trips through the config entry as plain
     JSON, and a hand-edited .storage file shouldn't be able to crash
     setup."""
-    restored: dict[str, dict[str, list[str]]] = {}
     if not isinstance(stored, dict):
-        return restored
-    for href, fields in stored.items():
-        if not isinstance(href, str) or not isinstance(fields, dict):
-            continue
-        for field, codes in fields.items():
-            if not isinstance(field, str):
-                continue
-            if valid := [c for c in _codes(codes) if c]:
-                restored.setdefault(href, {})[field] = valid
+        return {}
+    restored = {}
+    for href, codes in stored.items():
+        if isinstance(href, str) and (valid := [c for c in _codes(codes) if c]):
+            restored[href] = valid
     return restored
 
 
 class LearnedModes:
     """Per-device store of learned codes, keyed by actual (on-the-wire)
     href so two subdevices of one composite appliance learn separately.
+    One href carries one LEARNABLE rule, so the rule's fields are how a rep
+    is read, never part of the key.
 
     Mutated from whichever thread applied the update (the DTLS reader for
     an OBSERVE notify, an executor thread for a poll -- see
@@ -111,7 +108,7 @@ class LearnedModes:
         if not supported:
             return False
         with self._lock:
-            known = self._learned.get(actual_href, {}).get(rule.supported_field, [])
+            known = self._learned.get(actual_href, [])
             new = [
                 code
                 for code in _codes(rep.get(rule.current_field))
@@ -119,24 +116,17 @@ class LearnedModes:
             ]
             if not new:
                 return False
-            self._learned.setdefault(actual_href, {})[rule.supported_field] = [*known, *new]
+            self._learned[actual_href] = [*known, *new]
         return True
 
-    def codes(self, actual_href: str, field: str = SUPPORTED_FIELD) -> list[str]:
+    def codes(self, actual_href: str) -> list[str]:
         with self._lock:
-            return list(self._learned.get(actual_href, {}).get(field, ()))
+            return list(self._learned.get(actual_href, ()))
 
-    def snapshot(self) -> dict[str, dict[str, list[str]]]:
+    def snapshot(self) -> dict[str, list[str]]:
         with self._lock:
-            return {
-                href: {f: list(c) for f, c in fields.items()}
-                for href, fields in self._learned.items()
-            }
+            return {href: list(codes) for href, codes in self._learned.items()}
 
-    def clear(self) -> bool:
-        """Forget everything; True when there was something to forget."""
+    def clear(self) -> None:
         with self._lock:
-            if not self._learned:
-                return False
             self._learned = {}
-        return True

tests/localthings/test_config_flow.py

@@ -1013,12 +1013,22 @@ async def test_learned_modes_option_can_be_turned_off(hass: HomeAssistant) -> No
     assert entry.options[CONF_LEARN_MODES] is False
 
 
-async def test_forget_learned_modes_clears_the_entry(hass: HomeAssistant) -> None:
+@pytest.mark.parametrize(
+    ("stored", "listed"),
+    [
+        ({"/mode/convenient/vs/0": ["Quiet"]}, "Quiet"),
+        # Malformed -- nothing writes this shape, but a hand-edited
+        # .storage can hold it, and this step is the one screen that can
+        # clear it, so it must not be the one screen that trips over it.
+        ({"/mode/convenient/vs/0": None}, "(none)"),
+    ],
+)
+async def test_forget_learned_modes_clears_the_entry(hass: HomeAssistant, stored, listed) -> None:
     """The reset step works on an unloaded entry too, by dropping the
     persisted copy directly -- that's all a reload would restore from."""
     entry = MockConfigEntry(
         domain=DOMAIN,
-        data={**ENTRY_DATA, CONF_LEARNED_MODES: {"/mode/convenient/vs/0": {"f": ["Quiet"]}}},
+        data={**ENTRY_DATA, CONF_LEARNED_MODES: stored},
         unique_id=f"localthings_{MOCK_SERIAL}",
     )
     entry.add_to_hass(hass)
@@ -1028,7 +1038,7 @@ async def test_forget_learned_modes_clears_the_entry(hass: HomeAssistant) -> Non
         result["flow_id"], user_input={"next_step_id": "forget_learned_modes"}
     )
     assert result["type"] == FlowResultType.FORM
-    assert result["description_placeholders"] == {"codes": "Quiet"}
+    assert result["description_placeholders"] == {"codes": listed}
 
     result = await hass.config_entries.options.async_configure(result["flow_id"], user_input={})
 

tests/test_airconditioner_artik051_krac.py

@@ -64,7 +64,7 @@ class _FakeCoordinator:
     async def async_send_command(self, bound, payload):
         self.commands.append((bound, payload))
 
-    def learned_modes(self, href, field=None):
+    def learned_modes(self, href):
         # Nothing learned in this stub -- issue #327's store lives on the
         # real coordinator; climate._supported unions it in.
         return []

tests/test_airconditioner_tp1x_rac_01001_fan.py

@@ -48,7 +48,7 @@ class _FakeCoordinator:
     async def async_send_command(self, bound, payload):
         self.commands.append((bound, payload))
 
-    def learned_modes(self, href, field=None):
+    def learned_modes(self, href):
         # Nothing learned in this stub -- issue #327's store lives on the
         # real coordinator; climate._supported unions it in.
         return []

tests/test_climate_ac_modes.py

@@ -119,7 +119,7 @@ def test_fac_bora_wind_strength_codes_fit_the_standard_scale():
             # canonical view is just the raw snapshot (issue #177).
             return self.last_resources
 
-        def learned_modes(self, href, field=None):
+        def learned_modes(self, href):
             # Nothing learned in this stub -- issue #327's store lives on
             # the real coordinator; climate._supported unions it in.
             return []

tests/test_learned_modes.py

@@ -104,9 +104,10 @@ def test_two_subdevices_learn_separately():
     [
         None,
         "not-a-dict",
-        {"/mode/convenient/vs/0": "not-a-dict"},
-        {"/mode/convenient/vs/0": {SUPPORTED_FIELD: [""]}},
-        {"/mode/convenient/vs/0": {SUPPORTED_FIELD: [1, 2]}},
+        {"/mode/convenient/vs/0": None},
+        {"/mode/convenient/vs/0": [""]},
+        {"/mode/convenient/vs/0": [1, 2]},
+        {"/mode/convenient/vs/0": {"x.com.samsung.da.supportedModes": ["Quiet"]}},
     ],
 )
 def test_malformed_stored_data_restores_as_empty(stored):
@@ -116,15 +117,14 @@ def test_malformed_stored_data_restores_as_empty(stored):
 
 
 def test_well_formed_stored_data_restores():
-    learned = LearnedModes({CONVENIENT: {SUPPORTED_FIELD: ["Quiet"]}})
+    learned = LearnedModes({CONVENIENT: ["Quiet"]})
     assert learned.codes(CONVENIENT) == ["Quiet"]
 
 
-def test_clear_reports_whether_there_was_anything_to_forget():
-    learned = LearnedModes({CONVENIENT: {SUPPORTED_FIELD: ["Quiet"]}})
-    assert learned.clear() is True
+def test_clear_forgets_everything():
+    learned = LearnedModes({CONVENIENT: ["Quiet"]})
+    learned.clear()
     assert learned.snapshot() == {}
-    assert learned.clear() is False
 
 
 def test_every_learnable_href_is_one_climate_resolves():
@@ -215,14 +215,14 @@ async def test_learning_persists_onto_the_config_entry(hass: HomeAssistant):
     coordinator._observe.apply(CONVENIENT, QUIET_REP, source="poll")
     await _flush(hass)
 
-    assert entry.data[CONF_LEARNED_MODES] == {CONVENIENT: {SUPPORTED_FIELD: ["Quiet"]}}
+    assert entry.data[CONF_LEARNED_MODES] == {CONVENIENT: ["Quiet"]}
 
 
 async def test_a_learned_preset_survives_a_restart(hass: HomeAssistant):
     """The point of persisting: the unit only names Quiet while it is in
     Quiet, so a restart in any other mode would otherwise lose it until
     someone reached for the physical remote again."""
-    entry = _entry(hass, data={CONF_LEARNED_MODES: {CONVENIENT: {SUPPORTED_FIELD: ["Quiet"]}}})
+    entry = _entry(hass, data={CONF_LEARNED_MODES: {CONVENIENT: ["Quiet"]}})
     _, entity = await _climate(hass, entry)
 
     # Nothing applied this run -- the fixture's own rep says Off, and its
@@ -255,7 +255,7 @@ async def test_the_option_turns_off_both_halves(hass: HomeAssistant):
     nothing already learned is offered."""
     entry = _entry(
         hass,
-        data={CONF_LEARNED_MODES: {CONVENIENT: {SUPPORTED_FIELD: ["Smart"]}}},
+        data={CONF_LEARNED_MODES: {CONVENIENT: ["Smart"]}},
         options={CONF_LEARN_MODES: False},
     )
     coordinator, entity = await _climate(hass, entry)
@@ -266,12 +266,12 @@ async def test_the_option_turns_off_both_halves(hass: HomeAssistant):
     assert "quiet" not in entity.preset_modes
     # Kept, not discarded -- turning the option back on restores it, and
     # nothing new was written while it was off.
-    assert coordinator.learned_snapshot() == {CONVENIENT: {SUPPORTED_FIELD: ["Smart"]}}
-    assert entry.data[CONF_LEARNED_MODES] == {CONVENIENT: {SUPPORTED_FIELD: ["Smart"]}}
+    assert coordinator.learned_snapshot() == {CONVENIENT: ["Smart"]}
+    assert entry.data[CONF_LEARNED_MODES] == {CONVENIENT: ["Smart"]}
 
 
 async def test_forgetting_clears_the_store_and_the_entry(hass: HomeAssistant):
-    entry = _entry(hass, data={CONF_LEARNED_MODES: {CONVENIENT: {SUPPORTED_FIELD: ["Quiet"]}}})
+    entry = _entry(hass, data={CONF_LEARNED_MODES: {CONVENIENT: ["Quiet"]}})
     coordinator, entity = await _climate(hass, entry)
 
     coordinator.forget_learned_modes()
@@ -281,6 +281,20 @@ async def test_forgetting_clears_the_store_and_the_entry(hass: HomeAssistant):
     assert entry.data[CONF_LEARNED_MODES] == {}
 
 
+async def test_forgetting_clears_a_record_the_store_rejected(hass: HomeAssistant):
+    """A malformed persisted record is dropped on restore, so the store is
+    empty while the entry still holds it. Forget has to reach it anyway --
+    it's the only control that can."""
+    entry = _entry(hass, data={CONF_LEARNED_MODES: {CONVENIENT: None}})
+    coordinator, _ = await _climate(hass, entry)
+    assert coordinator.learned_snapshot() == {}
+
+    coordinator.forget_learned_modes()
+    await _flush(hass)
+
+    assert entry.data[CONF_LEARNED_MODES] == {}
+
+
 async def test_diagnostics_report_what_was_learned(
     hass: HomeAssistant,
     enable_custom_integrations,
@@ -298,8 +312,5 @@ async def test_diagnostics_report_what_was_learned(
 
     diag = await async_get_config_entry_diagnostics(hass, cast(Any, entry))
 
-    assert diag["learned_modes"] == {
-        "enabled": True,
-        "codes": {CONVENIENT: {SUPPORTED_FIELD: ["Quiet"]}},
-    }
+    assert diag["learned_modes"] == {"enabled": True, "codes": {CONVENIENT: ["Quiet"]}}
     assert diag["resources"][CONVENIENT][SUPPORTED_FIELD] == ADVERTISED