Pass slots to error messages instead of IDs [rework] (#109410)

Co-authored-by: tetele <tm.sandu@gmail.com>

homeassistant/components/conversation/__init__.py

@@ -349,7 +349,7 @@ async def websocket_hass_agent_debug(
                 },
                 # Slot values that would be received by the intent
                 "slots": {  # direct access to values
-                    entity_key: entity.value
+                    entity_key: entity.text or entity.value
                     for entity_key, entity in result.entities.items()
                 },
                 # Extra slot details, such as the originally matched text

homeassistant/components/conversation/default_agent.py

@@ -1,4 +1,5 @@
 """Standard conversation implementation for Home Assistant."""
+
 from __future__ import annotations
 
 import asyncio
@@ -264,9 +265,11 @@ class DefaultAgent(AbstractConversationAgent):
             _LOGGER.debug(
                 "Recognized intent '%s' for template '%s' but had unmatched: %s",
                 result.intent.name,
+                (
                     result.intent_sentence.text
                     if result.intent_sentence is not None
-                else "",
+                    else ""
+                ),
                 result.unmatched_entities_list,
             )
             error_response_type, error_response_args = _get_unmatched_response(result)
@@ -285,7 +288,8 @@ class DefaultAgent(AbstractConversationAgent):
 
         # Slot values to pass to the intent
         slots = {
-            entity.name: {"value": entity.value} for entity in result.entities_list
+            entity.name: {"value": entity.value, "text": entity.text or entity.value}
+            for entity in result.entities_list
         }
 
         try:
@@ -474,9 +478,11 @@ class DefaultAgent(AbstractConversationAgent):
                     for entity_name, entity_value in recognize_result.entities.items()
                 },
                 # First matched or unmatched state
-                "state": template.TemplateState(self.hass, state1)
+                "state": (
+                    template.TemplateState(self.hass, state1)
                     if state1 is not None
-                else None,
+                    else None
+                ),
                 "query": {
                     # Entity states that matched the query (e.g, "on")
                     "matched": [
@@ -734,7 +740,7 @@ class DefaultAgent(AbstractConversationAgent):
 
             if not entity:
                 # Default name
-                entity_names.append((state.name, state.name, context))
+                entity_names.append((state.name, state.entity_id, context))
                 continue
 
             if entity.aliases:
@@ -742,10 +748,10 @@ class DefaultAgent(AbstractConversationAgent):
                     if not alias.strip():
                         continue
 
-                    entity_names.append((alias, alias, context))
+                    entity_names.append((alias, state.entity_id, context))
 
             # Default name
-            entity_names.append((state.name, state.name, context))
+            entity_names.append((state.name, state.entity_id, context))
 
         # Expose all areas
         areas = ar.async_get(self.hass)
@@ -785,7 +791,7 @@ class DefaultAgent(AbstractConversationAgent):
         if device_area is None:
             return None
 
-        return {"area": device_area.id}
+        return {"area": {"value": device_area.id, "text": device_area.name}}
 
     def _get_error_text(
         self,

homeassistant/components/conversation/manifest.json

@@ -7,5 +7,5 @@
   "integration_type": "system",
   "iot_class": "local_push",
   "quality_scale": "internal",
-  "requirements": ["hassil==1.6.0", "home-assistant-intents==2024.2.2"]
+  "requirements": ["hassil==1.6.1", "home-assistant-intents==2024.2.2"]
 }

homeassistant/helpers/intent.py

@@ -1,4 +1,5 @@
 """Module to coordinate user intentions."""
+
 from __future__ import annotations
 
 import asyncio
@@ -401,17 +402,21 @@ class ServiceIntentHandler(IntentHandler):
         hass = intent_obj.hass
         slots = self.async_validate_slots(intent_obj.slots)
 
-        name: str | None = slots.get("name", {}).get("value")
+        name_slot = slots.get("name", {})
-        if name == "all":
+        entity_id: str | None = name_slot.get("value")
+        entity_name: str | None = name_slot.get("text")
+        if entity_id == "all":
             # Don't match on name if targeting all entities
-            name = None
+            entity_id = None
 
         # Look up area first to fail early
-        area_name = slots.get("area", {}).get("value")
+        area_slot = slots.get("area", {})
+        area_id = area_slot.get("value")
+        area_name = area_slot.get("text")
         area: area_registry.AreaEntry | None = None
-        if area_name is not None:
+        if area_id is not None:
             areas = area_registry.async_get(hass)
-            area = areas.async_get_area(area_name) or areas.async_get_area_by_name(
+            area = areas.async_get_area(area_id) or areas.async_get_area_by_name(
                 area_name
             )
             if area is None:
@@ -431,7 +436,7 @@ class ServiceIntentHandler(IntentHandler):
         states = list(
             async_match_states(
                 hass,
-                name=name,
+                name=entity_id,
                 area=area,
                 domains=domains,
                 device_classes=device_classes,
@@ -442,8 +447,8 @@ class ServiceIntentHandler(IntentHandler):
         if not states:
             # No states matched constraints
             raise NoStatesMatchedError(
-                name=name,
+                name=entity_name or entity_id,
-                area=area_name,
+                area=area_name or area_id,
                 domains=domains,
                 device_classes=device_classes,
             )

homeassistant/package_constraints.txt

@@ -26,7 +26,7 @@ ha-av==10.1.1
 ha-ffmpeg==3.1.0
 habluetooth==2.4.0
 hass-nabucasa==0.76.0
-hassil==1.6.0
+hassil==1.6.1
 home-assistant-bluetooth==1.12.0
 home-assistant-frontend==20240202.0
 home-assistant-intents==2024.2.2

requirements_all.txt

@@ -1025,7 +1025,7 @@ hass-nabucasa==0.76.0
 hass-splunk==0.1.1
 
 # homeassistant.components.conversation
-hassil==1.6.0
+hassil==1.6.1
 
 # homeassistant.components.jewish_calendar
 hdate==0.10.4

requirements_test_all.txt

@@ -830,7 +830,7 @@ habluetooth==2.4.0
 hass-nabucasa==0.76.0
 
 # homeassistant.components.conversation
-hassil==1.6.0
+hassil==1.6.1
 
 # homeassistant.components.jewish_calendar
 hdate==0.10.4

tests/components/conversation/snapshots/test_init.ambr

@@ -1397,7 +1397,7 @@
           'name': dict({
             'name': 'name',
             'text': 'my cool light',
-            'value': 'my cool light',
+            'value': 'light.kitchen',
           }),
         }),
         'intent': dict({
@@ -1422,7 +1422,7 @@
           'name': dict({
             'name': 'name',
             'text': 'my cool light',
-            'value': 'my cool light',
+            'value': 'light.kitchen',
           }),
         }),
         'intent': dict({
@@ -1498,7 +1498,7 @@
         'sentence_template': '[tell me] how many {on_off_domains:domain} (is|are) {on_off_states:state} [in <area>]',
         'slots': dict({
           'area': 'kitchen',
-          'domain': 'light',
+          'domain': 'lights',
           'state': 'on',
         }),
         'source': 'builtin',
@@ -1572,7 +1572,7 @@
           'name': dict({
             'name': 'name',
             'text': 'test light',
-            'value': 'test light',
+            'value': 'light.demo_1234',
           }),
         }),
         'intent': dict({
@@ -1581,7 +1581,7 @@
         'match': True,
         'sentence_template': '[<numeric_value_set>] <name> brightness [to] <brightness>',
         'slots': dict({
-          'brightness': 100,
+          'brightness': '100%',
           'name': 'test light',
         }),
         'source': 'builtin',
@@ -1604,7 +1604,7 @@
           'name': dict({
             'name': 'name',
             'text': 'test light',
-            'value': 'test light',
+            'value': 'light.demo_1234',
           }),
         }),
         'intent': dict({

tests/components/conversation/test_default_agent.py

@@ -1,4 +1,5 @@
 """Test for the default agent."""
+
 from collections import defaultdict
 from unittest.mock import AsyncMock, patch
 
@@ -85,8 +86,10 @@ async def test_exposed_areas(
     entity_registry: er.EntityRegistry,
 ) -> None:
     """Test that all areas are exposed."""
-    area_kitchen = area_registry.async_get_or_create("kitchen")
+    area_kitchen = area_registry.async_get_or_create("kitchen_id")
-    area_bedroom = area_registry.async_get_or_create("bedroom")
+    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")
 
     entry = MockConfigEntry()
     entry.add_to_hass(hass)
@@ -122,6 +125,9 @@ async def test_exposed_areas(
 
     # All is well for the exposed kitchen light
     assert result.response.response_type == intent.IntentResponseType.ACTION_DONE
+    assert result.response.intent is not None
+    assert result.response.intent.slots["area"]["value"] == area_kitchen.id
+    assert result.response.intent.slots["area"]["text"] == area_kitchen.normalized_name
 
     # Bedroom has no exposed entities
     result = await conversation.async_converse(
@@ -195,7 +201,8 @@ async def test_unexposed_entities_skipped(
     entity_registry: er.EntityRegistry,
 ) -> None:
     """Test that unexposed entities are skipped in exposed areas."""
-    area_kitchen = area_registry.async_get_or_create("kitchen")
+    area_kitchen = area_registry.async_get_or_create("kitchen_id")
+    area_kitchen = area_registry.async_update(area_kitchen.id, name="kitchen")
 
     # Both lights are in the kitchen
     exposed_light = entity_registry.async_get_or_create("light", "demo", "1234")
@@ -224,6 +231,9 @@ async def test_unexposed_entities_skipped(
 
     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["area"]["value"] == area_kitchen.id
+    assert result.response.intent.slots["area"]["text"] == area_kitchen.normalized_name
 
     # Only one light should be returned
     hass.states.async_set(exposed_light.entity_id, "on")
@@ -314,8 +324,10 @@ async def test_device_area_context(
     turn_on_calls = async_mock_service(hass, "light", "turn_on")
     turn_off_calls = async_mock_service(hass, "light", "turn_off")
 
-    area_kitchen = area_registry.async_get_or_create("Kitchen")
+    area_kitchen = area_registry.async_get_or_create("kitchen_id")
-    area_bedroom = area_registry.async_get_or_create("Bedroom")
+    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")
 
     # Create 2 lights in each area
     area_lights = defaultdict(list)
@@ -363,13 +375,14 @@ async def test_device_area_context(
     assert result.response.response_type == intent.IntentResponseType.ACTION_DONE
     assert result.response.intent is not None
     assert result.response.intent.slots["area"]["value"] == area_kitchen.id
+    assert result.response.intent.slots["area"]["text"] == area_kitchen.normalized_name
 
     # Verify only kitchen lights were targeted
     assert {s.entity_id for s in result.response.matched_states} == {
-        e.entity_id for e in area_lights["kitchen"]
+        e.entity_id for e in area_lights[area_kitchen.id]
     }
     assert {c.data["entity_id"][0] for c in turn_on_calls} == {
-        e.entity_id for e in area_lights["kitchen"]
+        e.entity_id for e in area_lights[area_kitchen.id]
     }
     turn_on_calls.clear()
 
@@ -386,13 +399,14 @@ async def test_device_area_context(
     assert result.response.response_type == intent.IntentResponseType.ACTION_DONE
     assert result.response.intent is not None
     assert result.response.intent.slots["area"]["value"] == area_bedroom.id
+    assert result.response.intent.slots["area"]["text"] == area_bedroom.normalized_name
 
     # Verify only bedroom lights were targeted
     assert {s.entity_id for s in result.response.matched_states} == {
-        e.entity_id for e in area_lights["bedroom"]
+        e.entity_id for e in area_lights[area_bedroom.id]
     }
     assert {c.data["entity_id"][0] for c in turn_on_calls} == {
-        e.entity_id for e in area_lights["bedroom"]
+        e.entity_id for e in area_lights[area_bedroom.id]
     }
     turn_on_calls.clear()
 
@@ -409,13 +423,14 @@ async def test_device_area_context(
     assert result.response.response_type == intent.IntentResponseType.ACTION_DONE
     assert result.response.intent is not None
     assert result.response.intent.slots["area"]["value"] == area_bedroom.id
+    assert result.response.intent.slots["area"]["text"] == area_bedroom.normalized_name
 
     # Verify only bedroom lights were targeted
     assert {s.entity_id for s in result.response.matched_states} == {
-        e.entity_id for e in area_lights["bedroom"]
+        e.entity_id for e in area_lights[area_bedroom.id]
     }
     assert {c.data["entity_id"][0] for c in turn_off_calls} == {
-        e.entity_id for e in area_lights["bedroom"]
+        e.entity_id for e in area_lights[area_bedroom.id]
     }
     turn_off_calls.clear()
 
@@ -463,7 +478,8 @@ async def test_error_no_device_in_area(
     hass: HomeAssistant, init_components, area_registry: ar.AreaRegistry
 ) -> None:
     """Test error message when area is missing a device/entity."""
-    area_registry.async_get_or_create("kitchen")
+    area_kitchen = area_registry.async_get_or_create("kitchen_id")
+    area_kitchen = area_registry.async_update(area_kitchen.id, name="kitchen")
     result = await conversation.async_converse(
         hass, "turn on missing entity in the kitchen", None, Context(), None
     )
@@ -482,7 +498,7 @@ async def test_error_no_domain(
     """Test error message when no devices/entities exist for a domain."""
 
     # We don't have a sentence for turning on all fans
-    fan_domain = MatchEntity(name="domain", value="fan", text="")
+    fan_domain = MatchEntity(name="domain", value="fan", text="fans")
     recognize_result = RecognizeResult(
         intent=Intent("HassTurnOn"),
         intent_data=IntentData([]),
@@ -513,7 +529,8 @@ async def test_error_no_domain_in_area(
     hass: HomeAssistant, init_components, area_registry: ar.AreaRegistry
 ) -> None:
     """Test error message when no devices/entities for a domain exist in an area."""
-    area_registry.async_get_or_create("kitchen")
+    area_kitchen = area_registry.async_get_or_create("kitchen_id")
+    area_kitchen = area_registry.async_update(area_kitchen.id, name="kitchen")
     result = await conversation.async_converse(
         hass, "turn on the lights in the kitchen", None, Context(), None
     )
@@ -526,13 +543,11 @@ async def test_error_no_domain_in_area(
     )
 
 
-async def test_error_no_device_class(
+async def test_error_no_device_class(hass: HomeAssistant, init_components) -> None:
-    hass: HomeAssistant, init_components, area_registry: ar.AreaRegistry
-) -> None:
     """Test error message when no entities of a device class exist."""
 
     # We don't have a sentence for opening all windows
-    window_class = MatchEntity(name="device_class", value="window", text="")
+    window_class = MatchEntity(name="device_class", value="window", text="windows")
     recognize_result = RecognizeResult(
         intent=Intent("HassTurnOn"),
         intent_data=IntentData([]),
@@ -563,7 +578,8 @@ async def test_error_no_device_class_in_area(
     hass: HomeAssistant, init_components, area_registry: ar.AreaRegistry
 ) -> None:
     """Test error message when no entities of a device class exist in an area."""
-    area_registry.async_get_or_create("bedroom")
+    area_bedroom = area_registry.async_get_or_create("bedroom_id")
+    area_bedroom = area_registry.async_update(area_bedroom.id, name="bedroom")
     result = await conversation.async_converse(
         hass, "open bedroom windows", None, Context(), None
     )
@@ -600,7 +616,8 @@ async def test_no_states_matched_default_error(
     hass: HomeAssistant, init_components, area_registry: ar.AreaRegistry
 ) -> None:
     """Test default response when no states match and slots are missing."""
-    area_registry.async_get_or_create("kitchen")
+    area_kitchen = area_registry.async_get_or_create("kitchen_id")
+    area_kitchen = area_registry.async_update(area_kitchen.id, name="kitchen")
 
     with patch(
         "homeassistant.components.conversation.default_agent.intent.async_handle",
@@ -629,9 +646,9 @@ async def test_empty_aliases(
     entity_registry: er.EntityRegistry,
 ) -> None:
     """Test that empty aliases are not added to slot lists."""
-    area_kitchen = area_registry.async_get_or_create("kitchen")
+    area_kitchen = area_registry.async_get_or_create("kitchen_id")
-    assert area_kitchen.id is not None
+    area_kitchen = area_registry.async_update(area_kitchen.id, name="kitchen")
-    area_registry.async_update(area_kitchen.id, aliases={" "})
+    area_kitchen = area_registry.async_update(area_kitchen.id, aliases={" "})
 
     entry = MockConfigEntry()
     entry.add_to_hass(hass)
@@ -643,11 +660,16 @@ async def test_empty_aliases(
     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, aliases={" "}
+        kitchen_light.entity_id,
+        device_id=kitchen_device.id,
+        name="kitchen light",
+        aliases={" "},
     )
     hass.states.async_set(
-        kitchen_light.entity_id, "on", attributes={ATTR_FRIENDLY_NAME: "kitchen light"}
+        kitchen_light.entity_id,
+        "on",
+        attributes={ATTR_FRIENDLY_NAME: kitchen_light.name},
     )
 
     with patch(
@@ -665,16 +687,16 @@ async def test_empty_aliases(
         assert slot_lists.keys() == {"area", "name"}
         areas = slot_lists["area"]
         assert len(areas.values) == 1
-        assert areas.values[0].value_out == "kitchen"
+        assert areas.values[0].value_out == area_kitchen.id
+        assert areas.values[0].text_in.text == area_kitchen.normalized_name
 
         names = slot_lists["name"]
         assert len(names.values) == 1
-        assert names.values[0].value_out == "kitchen light"
+        assert names.values[0].value_out == kitchen_light.entity_id
+        assert names.values[0].text_in.text == kitchen_light.name
 
 
-async def test_all_domains_loaded(
+async def test_all_domains_loaded(hass: HomeAssistant, init_components) -> None:
-    hass: HomeAssistant, init_components, area_registry: ar.AreaRegistry
-) -> None:
     """Test that sentences for all domains are always loaded."""
 
     # light domain is not loaded