Assist fixes (#109889)

* Don't pass entity ids in hassil slot lists

* Use first completed response

* Add more tests

homeassistant/components/climate/intent.py

@@ -1,4 +1,5 @@
 """Intents for the client integration."""
+
 from __future__ import annotations
 
 import voluptuous as vol
@@ -36,24 +37,34 @@ class GetTemperatureIntent(intent.IntentHandler):
         if not entities:
             raise intent.IntentHandleError("No climate entities")
 
-        if "area" in slots:
+        name_slot = slots.get("name", {})
-            # Filter by area
+        entity_name: str | None = name_slot.get("value")
-            area_name = slots["area"]["value"]
+        entity_text: str | None = name_slot.get("text")
+
+        area_slot = slots.get("area", {})
+        area_id = area_slot.get("value")
+
+        if area_id:
+            # Filter by area and optionally name
+            area_name = area_slot.get("text")
 
             for maybe_climate in intent.async_match_states(
-                hass, area_name=area_name, domains=[DOMAIN]
+                hass, name=entity_name, area_name=area_id, domains=[DOMAIN]
             ):
                 climate_state = maybe_climate
                 break
 
             if climate_state is None:
-                raise intent.IntentHandleError(f"No climate entity in area {area_name}")
+                raise intent.NoStatesMatchedError(
+                    name=entity_text or entity_name,
+                    area=area_name or area_id,
+                    domains={DOMAIN},
+                    device_classes=None,
+                )
 
             climate_entity = component.get_entity(climate_state.entity_id)
-        elif "name" in slots:
+        elif entity_name:
             # Filter by name
-            entity_name = slots["name"]["value"]
-
             for maybe_climate in intent.async_match_states(
                 hass, name=entity_name, domains=[DOMAIN]
             ):
@@ -61,7 +72,12 @@ class GetTemperatureIntent(intent.IntentHandler):
                 break
 
             if climate_state is None:
-                raise intent.IntentHandleError(f"No climate entity named {entity_name}")
+                raise intent.NoStatesMatchedError(
+                    name=entity_name,
+                    area=None,
+                    domains={DOMAIN},
+                    device_classes=None,
+                )
 
             climate_entity = component.get_entity(climate_state.entity_id)
         else:

homeassistant/components/conversation/default_agent.py

@@ -223,22 +223,22 @@ class DefaultAgent(AbstractConversationAgent):
         # Check if a trigger matched
         if isinstance(result, SentenceTriggerResult):
             # Gather callback responses in parallel
-            trigger_responses = await asyncio.gather(
+            trigger_callbacks = [
-                *(
+                self._trigger_sentences[trigger_id].callback(
-                    self._trigger_sentences[trigger_id].callback(
+                    result.sentence, trigger_result
-                        result.sentence, trigger_result
-                    )
-                    for trigger_id, trigger_result in result.matched_triggers.items()
                 )
-            )
+                for trigger_id, trigger_result in result.matched_triggers.items()
+            ]
 
             # Use last non-empty result as response.
             #
             # There may be multiple copies of a trigger running when editing in
             # the UI, so it's critical that we filter out empty responses here.
             response_text: str | None = None
-            for trigger_response in trigger_responses:
+            for trigger_future in asyncio.as_completed(trigger_callbacks):
-                response_text = response_text or trigger_response
+                if trigger_response := await trigger_future:
+                    response_text = trigger_response
+                    break
 
             # Convert to conversation result
             response = intent.IntentResponse(language=language)
@@ -724,7 +724,12 @@ class DefaultAgent(AbstractConversationAgent):
             if async_should_expose(self.hass, DOMAIN, state.entity_id)
         ]
 
-        # Gather exposed entity names
+        # Gather exposed entity names.
+        #
+        # NOTE: We do not pass entity ids in here because multiple entities may
+        # have the same name. The intent matcher doesn't gather all matching
+        # values for a list, just the first. So we will need to match by name no
+        # matter what.
         entity_names = []
         for state in states:
             # Checked against "requires_context" and "excludes_context" in hassil
@@ -740,7 +745,7 @@ class DefaultAgent(AbstractConversationAgent):
 
             if not entity:
                 # Default name
-                entity_names.append((state.name, state.entity_id, context))
+                entity_names.append((state.name, state.name, context))
                 continue
 
             if entity.aliases:
@@ -748,12 +753,15 @@ class DefaultAgent(AbstractConversationAgent):
                     if not alias.strip():
                         continue
 
-                    entity_names.append((alias, state.entity_id, context))
+                    entity_names.append((alias, state.name, context))
 
             # Default name
-            entity_names.append((state.name, state.entity_id, context))
+            entity_names.append((state.name, state.name, context))
 
-        # Expose all areas
+        # Expose all areas.
+        #
+        # We pass in area id here with the expectation that no two areas will
+        # share the same name or alias.
         areas = ar.async_get(self.hass)
         area_names = []
         for area in areas.async_list_areas():

homeassistant/components/intent/__init__.py

@@ -1,4 +1,5 @@
 """The Intent integration."""
+
 from __future__ import annotations
 
 import logging
@@ -155,7 +156,7 @@ class GetStateIntentHandler(intent.IntentHandler):
         slots = self.async_validate_slots(intent_obj.slots)
 
         # Entity name to match
-        name: str | None = slots.get("name", {}).get("value")
+        entity_name: str | None = slots.get("name", {}).get("value")
 
         # Look up area first to fail early
         area_name = slots.get("area", {}).get("value")
@@ -186,7 +187,7 @@ class GetStateIntentHandler(intent.IntentHandler):
         states = list(
             intent.async_match_states(
                 hass,
-                name=name,
+                name=entity_name,
                 area=area,
                 domains=domains,
                 device_classes=device_classes,
@@ -197,7 +198,7 @@ class GetStateIntentHandler(intent.IntentHandler):
         _LOGGER.debug(
             "Found %s state(s) that matched: name=%s, area=%s, domains=%s, device_classes=%s, assistant=%s",
             len(states),
-            name,
+            entity_name,
             area,
             domains,
             device_classes,

homeassistant/helpers/intent.py

@@ -403,11 +403,11 @@ class ServiceIntentHandler(IntentHandler):
         slots = self.async_validate_slots(intent_obj.slots)
 
         name_slot = slots.get("name", {})
-        entity_id: str | None = name_slot.get("value")
+        entity_name: str | None = name_slot.get("value")
-        entity_name: str | None = name_slot.get("text")
+        entity_text: str | None = name_slot.get("text")
-        if entity_id == "all":
+        if entity_name == "all":
             # Don't match on name if targeting all entities
-            entity_id = None
+            entity_name = None
 
         # Look up area first to fail early
         area_slot = slots.get("area", {})
@@ -436,7 +436,7 @@ class ServiceIntentHandler(IntentHandler):
         states = list(
             async_match_states(
                 hass,
-                name=entity_id,
+                name=entity_name,
                 area=area,
                 domains=domains,
                 device_classes=device_classes,
@@ -447,7 +447,7 @@ class ServiceIntentHandler(IntentHandler):
         if not states:
             # No states matched constraints
             raise NoStatesMatchedError(
-                name=entity_name or entity_id,
+                name=entity_text or entity_name,
                 area=area_name or area_id,
                 domains=domains,
                 device_classes=device_classes,
@@ -455,6 +455,9 @@ class ServiceIntentHandler(IntentHandler):
 
         response = await self.async_handle_states(intent_obj, states, area)
 
+        # Make the matched states available in the response
+        response.async_set_states(matched_states=states, unmatched_states=[])
+
         return response
 
     async def async_handle_states(

tests/components/climate/test_intent.py

@@ -1,4 +1,5 @@
 """Test climate intents."""
+
 from collections.abc import Generator
 from unittest.mock import patch
 
@@ -135,8 +136,10 @@ async def test_get_temperature(
     # Add climate entities to different areas:
     # climate_1 => living room
     # climate_2 => bedroom
+    # nothing in office
     living_room_area = area_registry.async_create(name="Living Room")
     bedroom_area = area_registry.async_create(name="Bedroom")
+    office_area = area_registry.async_create(name="Office")
 
     entity_registry.async_update_entity(
         climate_1.entity_id, area_id=living_room_area.id
@@ -158,7 +161,7 @@ async def test_get_temperature(
         hass,
         "test",
         climate_intent.INTENT_GET_TEMPERATURE,
-        {"area": {"value": "Bedroom"}},
+        {"area": {"value": bedroom_area.name}},
     )
     assert response.response_type == intent.IntentResponseType.QUERY_ANSWER
     assert len(response.matched_states) == 1
@@ -179,6 +182,52 @@ async def test_get_temperature(
     state = response.matched_states[0]
     assert state.attributes["current_temperature"] == 22.0
 
+    # Check area with no climate entities
+    with pytest.raises(intent.NoStatesMatchedError) as error:
+        response = await intent.async_handle(
+            hass,
+            "test",
+            climate_intent.INTENT_GET_TEMPERATURE,
+            {"area": {"value": office_area.name}},
+        )
+
+    # Exception should contain details of what we tried to match
+    assert isinstance(error.value, intent.NoStatesMatchedError)
+    assert error.value.name is None
+    assert error.value.area == office_area.name
+    assert error.value.domains == {DOMAIN}
+    assert error.value.device_classes is None
+
+    # Check wrong name
+    with pytest.raises(intent.NoStatesMatchedError) as error:
+        response = await intent.async_handle(
+            hass,
+            "test",
+            climate_intent.INTENT_GET_TEMPERATURE,
+            {"name": {"value": "Does not exist"}},
+        )
+
+    assert isinstance(error.value, intent.NoStatesMatchedError)
+    assert error.value.name == "Does not exist"
+    assert error.value.area is None
+    assert error.value.domains == {DOMAIN}
+    assert error.value.device_classes is None
+
+    # Check wrong name with area
+    with pytest.raises(intent.NoStatesMatchedError) as error:
+        response = await intent.async_handle(
+            hass,
+            "test",
+            climate_intent.INTENT_GET_TEMPERATURE,
+            {"name": {"value": "Climate 1"}, "area": {"value": bedroom_area.name}},
+        )
+
+    assert isinstance(error.value, intent.NoStatesMatchedError)
+    assert error.value.name == "Climate 1"
+    assert error.value.area == bedroom_area.name
+    assert error.value.domains == {DOMAIN}
+    assert error.value.device_classes is None
+
 
 async def test_get_temperature_no_entities(
     hass: HomeAssistant,
@@ -216,19 +265,28 @@ async def test_get_temperature_no_state(
         climate_1.entity_id, area_id=living_room_area.id
     )
 
-    with patch("homeassistant.core.StateMachine.get", return_value=None), pytest.raises(
+    with (
-        intent.IntentHandleError
+        patch("homeassistant.core.StateMachine.get", return_value=None),
+        pytest.raises(intent.IntentHandleError),
     ):
         await intent.async_handle(
             hass, "test", climate_intent.INTENT_GET_TEMPERATURE, {}
         )
 
-    with patch(
+    with (
-        "homeassistant.core.StateMachine.async_all", return_value=[]
+        patch("homeassistant.core.StateMachine.async_all", return_value=[]),
-    ), pytest.raises(intent.IntentHandleError):
+        pytest.raises(intent.NoStatesMatchedError) as error,
+    ):
         await intent.async_handle(
             hass,
             "test",
             climate_intent.INTENT_GET_TEMPERATURE,
             {"area": {"value": "Living Room"}},
         )
+
+    # Exception should contain details of what we tried to match
+    assert isinstance(error.value, intent.NoStatesMatchedError)
+    assert error.value.name is None
+    assert error.value.area == "Living Room"
+    assert error.value.domains == {DOMAIN}
+    assert error.value.device_classes is None

tests/components/conversation/snapshots/test_init.ambr

@@ -1397,7 +1397,7 @@
           'name': dict({
             'name': 'name',
             'text': 'my cool light',
-            'value': 'light.kitchen',
+            'value': 'kitchen',
           }),
         }),
         'intent': dict({
@@ -1422,7 +1422,7 @@
           'name': dict({
             'name': 'name',
             'text': 'my cool light',
-            'value': 'light.kitchen',
+            'value': 'kitchen',
           }),
         }),
         'intent': dict({
@@ -1572,7 +1572,7 @@
           'name': dict({
             'name': 'name',
             'text': 'test light',
-            'value': 'light.demo_1234',
+            'value': 'test light',
           }),
         }),
         'intent': dict({
@@ -1604,7 +1604,7 @@
           'name': dict({
             'name': 'name',
             'text': 'test light',
-            'value': 'light.demo_1234',
+            'value': 'test light',
           }),
         }),
         'intent': dict({

tests/components/conversation/test_default_agent.py

@@ -101,7 +101,7 @@ async def test_exposed_areas(
     device_registry.async_update_device(kitchen_device.id, area_id=area_kitchen.id)
 
     kitchen_light = entity_registry.async_get_or_create("light", "demo", "1234")
-    entity_registry.async_update_entity(
+    kitchen_light = entity_registry.async_update_entity(
         kitchen_light.entity_id, device_id=kitchen_device.id
     )
     hass.states.async_set(
@@ -109,7 +109,7 @@ async def test_exposed_areas(
     )
 
     bedroom_light = entity_registry.async_get_or_create("light", "demo", "5678")
-    entity_registry.async_update_entity(
+    bedroom_light = entity_registry.async_update_entity(
         bedroom_light.entity_id, area_id=area_bedroom.id
     )
     hass.states.async_set(
@@ -206,14 +206,14 @@ async def test_unexposed_entities_skipped(
 
     # Both lights are in the kitchen
     exposed_light = entity_registry.async_get_or_create("light", "demo", "1234")
-    entity_registry.async_update_entity(
+    exposed_light = entity_registry.async_update_entity(
         exposed_light.entity_id,
         area_id=area_kitchen.id,
     )
     hass.states.async_set(exposed_light.entity_id, "off")
 
     unexposed_light = entity_registry.async_get_or_create("light", "demo", "5678")
-    entity_registry.async_update_entity(
+    unexposed_light = entity_registry.async_update_entity(
         unexposed_light.entity_id,
         area_id=area_kitchen.id,
     )
@@ -336,7 +336,9 @@ async def test_device_area_context(
             light_entity = entity_registry.async_get_or_create(
                 "light", "demo", f"{area.name}-light-{i}"
             )
-            entity_registry.async_update_entity(light_entity.entity_id, area_id=area.id)
+            light_entity = entity_registry.async_update_entity(
+                light_entity.entity_id, area_id=area.id
+            )
             hass.states.async_set(
                 light_entity.entity_id,
                 "off",
@@ -692,7 +694,7 @@ async def test_empty_aliases(
 
         names = slot_lists["name"]
         assert len(names.values) == 1
-        assert names.values[0].value_out == kitchen_light.entity_id
+        assert names.values[0].value_out == kitchen_light.name
         assert names.values[0].text_in.text == kitchen_light.name
 
 
@@ -713,3 +715,82 @@ async def test_all_domains_loaded(hass: HomeAssistant, init_components) -> None:
         result.response.speech["plain"]["speech"]
         == "Sorry, I am not aware of any device called test light"
     )
+
+
+async def test_same_named_entities_in_different_areas(
+    hass: HomeAssistant,
+    init_components,
+    area_registry: ar.AreaRegistry,
+    entity_registry: er.EntityRegistry,
+) -> None:
+    """Test that entities with the same name in different areas can be targeted."""
+    area_kitchen = area_registry.async_get_or_create("kitchen_id")
+    area_kitchen = area_registry.async_update(area_kitchen.id, name="kitchen")
+
+    area_bedroom = area_registry.async_get_or_create("bedroom_id")
+    area_bedroom = area_registry.async_update(area_bedroom.id, name="bedroom")
+
+    # Both lights have the same name, but are in different areas
+    kitchen_light = entity_registry.async_get_or_create("light", "demo", "1234")
+    kitchen_light = entity_registry.async_update_entity(
+        kitchen_light.entity_id,
+        area_id=area_kitchen.id,
+        name="overhead light",
+    )
+    hass.states.async_set(
+        kitchen_light.entity_id,
+        "off",
+        attributes={ATTR_FRIENDLY_NAME: kitchen_light.name},
+    )
+
+    bedroom_light = entity_registry.async_get_or_create("light", "demo", "5678")
+    bedroom_light = entity_registry.async_update_entity(
+        bedroom_light.entity_id,
+        area_id=area_bedroom.id,
+        name="overhead light",
+    )
+    hass.states.async_set(
+        bedroom_light.entity_id,
+        "off",
+        attributes={ATTR_FRIENDLY_NAME: bedroom_light.name},
+    )
+
+    # Target kitchen light
+    calls = async_mock_service(hass, "light", "turn_on")
+    result = await conversation.async_converse(
+        hass, "turn on overhead light in the kitchen", None, Context(), None
+    )
+    await hass.async_block_till_done()
+
+    assert len(calls) == 1
+    assert result.response.response_type == intent.IntentResponseType.ACTION_DONE
+    assert result.response.intent is not None
+    assert (
+        result.response.intent.slots.get("name", {}).get("value") == kitchen_light.name
+    )
+    assert (
+        result.response.intent.slots.get("name", {}).get("text") == kitchen_light.name
+    )
+    assert len(result.response.matched_states) == 1
+    assert result.response.matched_states[0].entity_id == kitchen_light.entity_id
+    assert calls[0].data.get("entity_id") == [kitchen_light.entity_id]
+
+    # Target bedroom light
+    calls.clear()
+    result = await conversation.async_converse(
+        hass, "turn on overhead light in the bedroom", None, Context(), None
+    )
+    await hass.async_block_till_done()
+
+    assert len(calls) == 1
+    assert result.response.response_type == intent.IntentResponseType.ACTION_DONE
+    assert result.response.intent is not None
+    assert (
+        result.response.intent.slots.get("name", {}).get("value") == bedroom_light.name
+    )
+    assert (
+        result.response.intent.slots.get("name", {}).get("text") == bedroom_light.name
+    )
+    assert len(result.response.matched_states) == 1
+    assert result.response.matched_states[0].entity_id == bedroom_light.entity_id
+    assert calls[0].data.get("entity_id") == [bedroom_light.entity_id]

tests/components/conversation/test_trigger.py

@@ -1,4 +1,7 @@
 """Test conversation triggers."""
+
+import logging
+
 import pytest
 import voluptuous as vol
 
@@ -70,7 +73,7 @@ async def test_if_fires_on_event(hass: HomeAssistant, calls, setup_comp) -> None
 
 
 async def test_response(hass: HomeAssistant, setup_comp) -> None:
-    """Test the firing of events."""
+    """Test the conversation response action."""
     response = "I'm sorry, Dave. I'm afraid I can't do that"
     assert await async_setup_component(
         hass,
@@ -100,6 +103,116 @@ async def test_response(hass: HomeAssistant, setup_comp) -> None:
     assert service_response["response"]["speech"]["plain"]["speech"] == response
 
 
+async def test_response_same_sentence(hass: HomeAssistant, calls, setup_comp) -> None:
+    """Test the conversation response action with multiple triggers using the same sentence."""
+    assert await async_setup_component(
+        hass,
+        "automation",
+        {
+            "automation": [
+                {
+                    "trigger": {
+                        "id": "trigger1",
+                        "platform": "conversation",
+                        "command": ["test sentence"],
+                    },
+                    "action": [
+                        # Add delay so this response will not be the first
+                        {"delay": "0:0:0.100"},
+                        {
+                            "service": "test.automation",
+                            "data_template": {"data": "{{ trigger }}"},
+                        },
+                        {"set_conversation_response": "response 2"},
+                    ],
+                },
+                {
+                    "trigger": {
+                        "id": "trigger2",
+                        "platform": "conversation",
+                        "command": ["test sentence"],
+                    },
+                    "action": {"set_conversation_response": "response 1"},
+                },
+            ]
+        },
+    )
+
+    service_response = await hass.services.async_call(
+        "conversation",
+        "process",
+        {"text": "test sentence"},
+        blocking=True,
+        return_response=True,
+    )
+    await hass.async_block_till_done()
+
+    # Should only get first response
+    assert service_response["response"]["speech"]["plain"]["speech"] == "response 1"
+
+    # Service should still have been called
+    assert len(calls) == 1
+    assert calls[0].data["data"] == {
+        "alias": None,
+        "id": "trigger1",
+        "idx": "0",
+        "platform": "conversation",
+        "sentence": "test sentence",
+        "slots": {},
+        "details": {},
+    }
+
+
+async def test_response_same_sentence_with_error(
+    hass: HomeAssistant, calls, setup_comp, caplog: pytest.LogCaptureFixture
+) -> None:
+    """Test the conversation response action with multiple triggers using the same sentence and an error."""
+    caplog.set_level(logging.ERROR)
+    assert await async_setup_component(
+        hass,
+        "automation",
+        {
+            "automation": [
+                {
+                    "trigger": {
+                        "id": "trigger1",
+                        "platform": "conversation",
+                        "command": ["test sentence"],
+                    },
+                    "action": [
+                        # Add delay so this will not finish first
+                        {"delay": "0:0:0.100"},
+                        {"service": "fake_domain.fake_service"},
+                    ],
+                },
+                {
+                    "trigger": {
+                        "id": "trigger2",
+                        "platform": "conversation",
+                        "command": ["test sentence"],
+                    },
+                    "action": {"set_conversation_response": "response 1"},
+                },
+            ]
+        },
+    )
+
+    service_response = await hass.services.async_call(
+        "conversation",
+        "process",
+        {"text": "test sentence"},
+        blocking=True,
+        return_response=True,
+    )
+    await hass.async_block_till_done()
+
+    # Should still get first response
+    assert service_response["response"]["speech"]["plain"]["speech"] == "response 1"
+
+    # Error should have been logged
+    assert "Error executing script" in caplog.text
+
+
 async def test_subscribe_trigger_does_not_interfere_with_responses(
     hass: HomeAssistant, setup_comp, hass_ws_client: WebSocketGenerator
 ) -> None:

tests/components/intent/test_init.py

@@ -1,4 +1,5 @@
 """Tests for Intent component."""
+
 import pytest
 
 from homeassistant.components.cover import SERVICE_OPEN_COVER
@@ -225,6 +226,30 @@ async def test_turn_on_multiple_intent(hass: HomeAssistant) -> None:
     assert call.data == {"entity_id": ["light.test_lights_2"]}
 
 
+async def test_turn_on_all(hass: HomeAssistant) -> None:
+    """Test HassTurnOn intent with "all" name."""
+    result = await async_setup_component(hass, "homeassistant", {})
+    result = await async_setup_component(hass, "intent", {})
+    assert result
+
+    hass.states.async_set("light.test_light", "off")
+    hass.states.async_set("light.test_light_2", "off")
+    calls = async_mock_service(hass, "light", SERVICE_TURN_ON)
+
+    await intent.async_handle(hass, "test", "HassTurnOn", {"name": {"value": "all"}})
+    await hass.async_block_till_done()
+
+    # All lights should be on now
+    assert len(calls) == 2
+    entity_ids = set()
+    for call in calls:
+        assert call.domain == "light"
+        assert call.service == "turn_on"
+        entity_ids.update(call.data.get("entity_id", []))
+
+    assert entity_ids == {"light.test_light", "light.test_light_2"}
+
+
 async def test_get_state_intent(
     hass: HomeAssistant,
     area_registry: ar.AreaRegistry,