fix: use correct entity and context in service interceptor (#1349)

custom_components/adaptive_lighting/switch.py

@@ -1746,6 +1746,7 @@ class AdaptiveLightingManager:
         ]
 
         self._proactively_adapting_contexts: dict[str, str] = {}
+        self._context_cnt: int = 0
 
         try:
             self.listener_removers.append(
@@ -1809,6 +1810,16 @@ class AdaptiveLightingManager:
         for key in keys:
             self._proactively_adapting_contexts.pop(key)
 
+    def create_context(
+        self,
+        which: str = "default",
+        parent: Context | None = None,
+    ) -> Context:
+        """Create a context that identifies this integration."""
+        context = create_context("manager", which, self._context_cnt, parent=parent)
+        self._context_cnt += 1
+        return context
+
     def _separate_entity_ids(
         self,
         entity_ids: list[str],
@@ -2047,7 +2058,7 @@ class AdaptiveLightingManager:
                 assert set(skipped) == set(entity_ids)
                 return  # The call will be intercepted with the original data
             # Call light turn_on service for skipped entities
-            context = switch.create_context("skipped")
+            context = self.create_context("skipped")
             _LOGGER.debug(
                 "(5) _service_interceptor_turn_on_handler: calling `light.turn_on` with skipped='%s', service_data: '%s', context='%s'",
                 skipped,
@@ -2081,11 +2092,11 @@ class AdaptiveLightingManager:
         # `state_changed_event_listener`, however, this function is called
         # before that one.
         self.reset(*entity_ids, reset_manual_control=False)
-        for entity_id in entity_ids:
-            self.clear_proactively_adapting(entity_id)
+        for eid in entity_ids:
+            self.clear_proactively_adapting(eid)
 
         adaptation_data = await switch.prepare_adaptation_data(
-            entity_id,
+            entity_ids[0],
             transition,
         )
         if adaptation_data is None:

tests/test_switch.py

@@ -71,6 +71,7 @@ from homeassistant.components.adaptive_lighting.switch import (
     create_context,
     is_our_context,
     is_our_context_id,
+    short_hash,
 )
 from homeassistant.components.light import (
     ATTR_BRIGHTNESS,
@@ -2462,3 +2463,144 @@ def test_attributes_have_changed_light_mode_switch():
         new_attributes={ATTR_BRIGHTNESS: 128, ATTR_COLOR_TEMP_KELVIN: 4000},
         **kwargs_no_adapt,
     ), "RGB → color_temp should not be detected when adapt_color=False"
+
+
+# Regression tests for bugs found in PR #1348 by @protyposis
+# See: https://github.com/basnijholt/adaptive-lighting/pull/1348
+
+
+async def test_multi_light_intercept_prepares_adaptation_for_first_entity(hass):
+    """Test that adaptation data is prepared for the first entity, not the last.
+
+    Regression test for a bug where `entity_id` from a for-loop was used after
+    the loop ended, causing `prepare_adaptation_data` to be called with only
+    the last entity's ID instead of the first.
+
+    In `_service_interceptor_turn_on_single_light_handler`:
+    ```python
+    for entity_id in entity_ids:
+        self.clear_proactively_adapting(entity_id)
+
+    adaptation_data = await switch.prepare_adaptation_data(
+        entity_id,  # BUG: This uses the last entity_id from the loop!
+        transition,
+    )
+    ```
+
+    The adaptation data should be prepared for the first entity in the list since
+    that's the one whose service call is being intercepted and modified.
+
+    See: https://github.com/basnijholt/adaptive-lighting/pull/1348
+    """
+    switch, _ = await setup_lights_and_switch(hass, {CONF_INTERCEPT: True}, True)
+
+    # Turn off all lights first
+    await hass.services.async_call(
+        LIGHT_DOMAIN,
+        SERVICE_TURN_OFF,
+        {ATTR_ENTITY_ID: [ENTITY_LIGHT_1, ENTITY_LIGHT_2, ENTITY_LIGHT_3]},
+        blocking=True,
+    )
+    await hass.async_block_till_done()
+
+    # Mock prepare_adaptation_data to track which entity_id it's called with
+    original_prepare = switch.prepare_adaptation_data
+    called_with_entities = []
+
+    async def mock_prepare_adaptation_data(light, *args, **kwargs):
+        called_with_entities.append(light)
+        return await original_prepare(light, *args, **kwargs)
+
+    switch.prepare_adaptation_data = mock_prepare_adaptation_data
+
+    _mock_sun_light_settings(
+        switch,
+        {
+            ATTR_BRIGHTNESS_PCT: 67,
+            ATTR_COLOR_TEMP_KELVIN: 3448,
+            "force_rgb_color": False,
+        },
+    )
+
+    # Turn on multiple lights at once - this triggers the interceptor
+    await hass.services.async_call(
+        LIGHT_DOMAIN,
+        SERVICE_TURN_ON,
+        {ATTR_ENTITY_ID: [ENTITY_LIGHT_1, ENTITY_LIGHT_2]},
+        blocking=True,
+    )
+    await hass.async_block_till_done()
+
+    # The bug causes prepare_adaptation_data to be called with the LAST entity
+    # (ENTITY_LIGHT_2) instead of the FIRST entity (ENTITY_LIGHT_1)
+    assert len(called_with_entities) >= 1, "prepare_adaptation_data should be called"
+
+    # The first call should be for ENTITY_LIGHT_1 (the first entity in the list)
+    # since the intercepted service call will apply to all entities in entity_ids
+    # BUG: Currently this fails because entity_id is ENTITY_LIGHT_2 (the last one)
+    assert called_with_entities[0] == ENTITY_LIGHT_1, (
+        f"prepare_adaptation_data should be called with the first entity "
+        f"({ENTITY_LIGHT_1}), but was called with {called_with_entities[0]}. "
+        f"This indicates the bug where the last entity from the for-loop is used."
+    )
+
+
+async def test_skipped_lights_context_not_from_arbitrary_switch(hass):
+    """Test that context for skipped lights uses manager, not an arbitrary switch.
+
+    Regression test for a bug where the context for skipped lights was created
+    using `switch.create_context("skipped")` where `switch` was from the last
+    iteration of a for-loop, which had no relationship to the skipped lights.
+
+    The fix uses `self.create_context("skipped")` on the AdaptiveLightingManager
+    instead, which uses "manager" as the context name.
+
+    See: https://github.com/basnijholt/adaptive-lighting/pull/1348
+    """
+    # Setup two switches with different lights
+    lights, switch1, switch2 = await setup_proactive_multiple_lights_two_switches(hass)
+
+    # Turn on all three lights at once:
+    # - ENTITY_LIGHT_1 is in switch1
+    # - ENTITY_LIGHT_2 is in switch2
+    # - ENTITY_LIGHT_3 is not in any switch (will be skipped)
+    events = await _turn_on_and_track_event_contexts(
+        hass,
+        "test_skipped_context",
+        lights,
+        return_full_events=True,
+    )
+
+    # Find the skipped event (contains ":skpp:" in context)
+    skipped_events = [e for e in events if ":skpp:" in e.context.id]
+    assert (
+        len(skipped_events) == 1
+    ), f"Expected 1 skipped event, got {len(skipped_events)}"
+
+    skipped_event = skipped_events[0]
+    skipped_context_id = skipped_event.context.id
+
+    # Extract the name_hash from the context
+    # Context format: {timestamp}:{al}:{name_hash}:{which_short}:{index}
+    context_parts = skipped_context_id.split(":")
+    assert len(context_parts) >= 4, f"Unexpected context format: {skipped_context_id}"
+
+    # The context should still be recognized as ours
+    assert is_our_context_id(skipped_context_id), "Skipped context should be recognized"
+    assert is_our_context_id(
+        skipped_context_id,
+        "skipped",
+    ), "Skipped context should have 'skipped' marker"
+
+    # Verify the skipped lights are the ones not in any switch
+    assert skipped_event.data["service_data"][ATTR_ENTITY_ID] == [ENTITY_LIGHT_3]
+
+    # After the fix, the context should use "manager" as the name, not a switch name.
+    # The name_hash is the 3rd segment (index 2) in the context ID.
+    name_hash_in_context = context_parts[2]
+    expected_manager_hash = short_hash("manager")
+    assert name_hash_in_context == expected_manager_hash, (
+        f"Skipped context should use 'manager' hash ({expected_manager_hash}), "
+        f"but got {name_hash_in_context}. This indicates the context is still "
+        f"being created from an arbitrary switch instead of the manager."
+    )