From 1493467c6313eba9c98335001ee3958f8bd22cdb Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Wed, 22 Jul 2026 20:16:20 +0000 Subject: [PATCH] Fix AI energy level platform-flip bug found in Opus review The switch and select shared a stub-time asymmetry: only the select had a `not rep` carve-out, so an unfetched-stub rep at the moment platforms are set up (entity creation runs once, ever) would instantiate a Select, while flatten() re-evaluates exists_fn every poll against live data -- once the resource populated to a single-level list, the switch's exists_fn would win instead and feed the already-created Select a bool through their shared 'ai_energy_level' key, which isn't a valid select option. Dropped the stub carve-out from the select's exists_fn so both sides require real, populated data to decide the platform -- on a device that stubs this cold-tier href on its very first poll, the entity now simply doesn't appear until a reload, instead of appearing as the wrong widget type. Added tests for the missing/empty-list supportedAiLevel shapes on both widgets and a regression test locking in the stub behavior. --- .../registry/capabilities/common.py | 18 ++++++- tests/test_common_capabilities.py | 47 ++++++++++++++++--- 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/custom_components/localthings/registry/capabilities/common.py b/custom_components/localthings/registry/capabilities/common.py index 7a490a1..ef181fc 100644 --- a/custom_components/localthings/registry/capabilities/common.py +++ b/custom_components/localthings/registry/capabilities/common.py @@ -277,6 +277,22 @@ AI_ENERGY_LEVEL = Capability( href='/energy/ailevel/vs/0', poll_tier='cold', entities=( + # No `not rep` stub carve-out on either side, unlike most exists_fn + # gates in this file -- entity creation only ever runs once, against + # whichever snapshot happens to be current the moment platforms are + # set up (see entity._is_included / __init__.py's + # async_config_entry_first_refresh-before-forward-entry-setups + # ordering), while flatten() re-evaluates exists_fn every poll + # against live data. Both descriptors share key='ai_energy_level', + # so if a stub carve-out let one of them win at setup time while the + # other wins once real data lands, flatten() would feed the + # instantiated entity a value shaped for the other platform (e.g. a + # bool into a Select). Requiring real, populated data on both sides + # keeps the entity-creation decision and the live-value decision in + # permanent agreement -- the cost is this entity doesn't appear + # until a reload if the device's very first poll stubs this + # cold-tier href, the same reload already required to fix which + # platform got picked in that case. SwitchDesc(key='ai_energy_level', field='aiLevel', name='AI energy level', icon='mdi:leaf', entity_category='config', @@ -289,7 +305,7 @@ AI_ENERGY_LEVEL = Capability( entity_category='config', options=_ai_energy_level_options, exists_fn=lambda rep, resources: ( - not rep or len(_ai_energy_supported_levels(rep)) > 1), + len(_ai_energy_supported_levels(rep)) > 1), write_fn=_ai_energy_level_write), ), ) diff --git a/tests/test_common_capabilities.py b/tests/test_common_capabilities.py index e852809..fdd1150 100644 --- a/tests/test_common_capabilities.py +++ b/tests/test_common_capabilities.py @@ -166,10 +166,15 @@ class TestAiEnergyLevelSwitch: desc = self._desc() assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': '1'}, {}) is False + def test_hidden_when_supported_level_missing(self): + desc = self._desc() + assert desc.exists_fn({'aiLevel': '1'}, {}) is False + + def test_hidden_when_supported_level_empty_list(self): + desc = self._desc() + assert desc.exists_fn({'aiLevel': '0', 'supportedAiLevel': []}, {}) is False + def test_hidden_on_empty_stub_rep(self): - """Unlike the select, the switch requires a confirmed single-entry - list -- an unfetched stub defaults to the select branch instead (see - TestAiEnergyLevelSelect.test_shown_for_empty_stub_rep).""" desc = self._desc() assert desc.exists_fn({}, {}) is False @@ -201,11 +206,17 @@ class TestAiEnergyLevelSelect: assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': ['1', '2']}, {}) is True assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': ['1']}, {}) is False - def test_shown_for_empty_stub_rep(self): - """An empty {} rep is /device/0's not-yet-fetched-stub carve-out -- - defaults to the select so sub-polls can populate it either way.""" + def test_hidden_when_supported_level_is_non_list_scalar(self): desc = self._desc() - assert desc.exists_fn({}, {}) is True + assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': '12'}, {}) is False + + def test_hidden_when_supported_level_missing(self): + desc = self._desc() + assert desc.exists_fn({'aiLevel': '1'}, {}) is False + + def test_hidden_when_supported_level_empty_list(self): + desc = self._desc() + assert desc.exists_fn({'aiLevel': '0', 'supportedAiLevel': []}, {}) is False def test_no_translation_key(self): """aiLevel's values are plain digits that render fine untranslated @@ -232,6 +243,28 @@ class TestAiEnergyLevelSelect: assert body == {'aiLevel': '2'} +class TestAiEnergyLevelStubDoesNotDecideThePlatform: + """Issue found in review: entity *creation* runs once, against whatever + snapshot is current when platforms are set up (see + __init__.py's async_config_entry_first_refresh-before-forward-entry-setups + ordering), while flatten() re-evaluates exists_fn every poll against live + data. Both descriptors share key='ai_energy_level' (see adapter._key), so + if a stub carve-out let one of them win at setup time while the other + wins once real data lands, flatten() would feed the already-instantiated + entity a value shaped for the other platform (e.g. a bool into a Select + expecting a string option). Neither side gets a `not rep` carve-out, so + an unfetched stub can never win entity creation for either platform -- + the entity simply doesn't appear until a reload happens with real data, + same as any other exists_fn-gated entity in this codebase that's unlucky + on first-poll timing, instead of appearing as the wrong widget type.""" + + def test_neither_widget_exists_on_empty_stub_rep(self): + switch = _ai_energy_level_desc('SwitchDesc') + select = _ai_energy_level_desc('SelectDesc') + assert switch.exists_fn({}, {}) is False + assert select.exists_fn({}, {}) is False + + class TestSelfCheckError: """Self-check diagnostic error list -- surfaced on hardware that reports x.com.samsung.da.error, joined into a single display string."""