Matching duplicate named entities is now an error in Assist (#110050)

* Matching duplicate named entities is now an error

* Update snapshot

* Only use area id

homeassistant/components/conversation/default_agent.py

@@ -316,6 +316,20 @@ class DefaultAgent(AbstractConversationAgent):
                 ),
                 conversation_id,
             )
+        except intent.DuplicateNamesMatchedError as duplicate_names_error:
+            # Intent was valid, but two or more entities with the same name matched.
+            (
+                error_response_type,
+                error_response_args,
+            ) = _get_duplicate_names_matched_response(duplicate_names_error)
+            return _make_error_result(
+                language,
+                intent.IntentResponseErrorCode.NO_VALID_TARGETS,
+                self._get_error_text(
+                    error_response_type, lang_intents, **error_response_args
+                ),
+                conversation_id,
+            )
         except intent.IntentHandleError:
             # Intent was valid and entities matched constraints, but an error
             # occurred during handling.
@@ -753,7 +767,7 @@ class DefaultAgent(AbstractConversationAgent):
                     if not alias.strip():
                         continue
 
-                    entity_names.append((alias, state.name, context))
+                    entity_names.append((alias, alias, context))
 
             # Default name
             entity_names.append((state.name, state.name, context))
@@ -992,6 +1006,20 @@ def _get_no_states_matched_response(
     return ErrorKey.NO_INTENT, {}
 
 
+def _get_duplicate_names_matched_response(
+    duplicate_names_error: intent.DuplicateNamesMatchedError,
+) -> tuple[ErrorKey, dict[str, Any]]:
+    """Return key and template arguments for error when intent returns duplicate matches."""
+
+    if duplicate_names_error.area:
+        return ErrorKey.DUPLICATE_ENTITIES_IN_AREA, {
+            "entity": duplicate_names_error.name,
+            "area": duplicate_names_error.area,
+        }
+
+    return ErrorKey.DUPLICATE_ENTITIES, {"entity": duplicate_names_error.name}
+
+
 def _collect_list_references(expression: Expression, list_names: set[str]) -> None:
     """Collect list reference names recursively."""
     if isinstance(expression, Sequence):

homeassistant/components/intent/__init__.py

@@ -156,16 +156,18 @@ class GetStateIntentHandler(intent.IntentHandler):
         slots = self.async_validate_slots(intent_obj.slots)
 
         # Entity name to match
-        entity_name: str | None = slots.get("name", {}).get("value")
+        name_slot = slots.get("name", {})
+        entity_name: str | None = name_slot.get("value")
+        entity_text: str | None = name_slot.get("text")
 
         # 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: ar.AreaEntry | None = None
-        if area_name is not None:
+        if area_id is not None:
             areas = ar.async_get(hass)
-            area = areas.async_get_area(area_name) or areas.async_get_area_by_name(
+            area = areas.async_get_area(area_id)
-                area_name
-            )
             if area is None:
                 raise intent.IntentHandleError(f"No area named {area_name}")
 
@@ -205,6 +207,13 @@ class GetStateIntentHandler(intent.IntentHandler):
             intent_obj.assistant,
         )
 
+        if entity_name and (len(states) > 1):
+            # Multiple entities matched for the same name
+            raise intent.DuplicateNamesMatchedError(
+                name=entity_text or entity_name,
+                area=area_name or area_id,
+            )
+
         # Create response
         response = intent_obj.create_response()
         response.response_type = intent.IntentResponseType.QUERY_ANSWER

homeassistant/helpers/intent.py

@@ -155,6 +155,17 @@ class NoStatesMatchedError(IntentError):
         self.device_classes = device_classes
 
 
+class DuplicateNamesMatchedError(IntentError):
+    """Error when two or more entities with the same name matched."""
+
+    def __init__(self, name: str, area: str | None) -> None:
+        """Initialize error."""
+        super().__init__()
+
+        self.name = name
+        self.area = area
+
+
 def _is_device_class(
     state: State,
     entity: entity_registry.RegistryEntry | None,
@@ -318,8 +329,6 @@ def async_match_states(
         for state, entity in states_and_entities:
             if _has_name(state, entity, name):
                 yield state
-                break
-
     else:
         # Not filtered by name
         for state, _entity in states_and_entities:
@@ -416,9 +425,7 @@ class ServiceIntentHandler(IntentHandler):
         area: area_registry.AreaEntry | None = None
         if area_id is not None:
             areas = area_registry.async_get(hass)
-            area = areas.async_get_area(area_id) or areas.async_get_area_by_name(
+            area = areas.async_get_area(area_id)
-                area_name
-            )
             if area is None:
                 raise IntentHandleError(f"No area named {area_name}")
 
@@ -453,6 +460,13 @@ class ServiceIntentHandler(IntentHandler):
                 device_classes=device_classes,
             )
 
+        if entity_name and (len(states) > 1):
+            # Multiple entities matched for the same name
+            raise DuplicateNamesMatchedError(
+                name=entity_text or entity_name,
+                area=area_name or area_id,
+            )
+
         response = await self.async_handle_states(intent_obj, states, area)
 
         # Make the matched states available in the response

tests/components/conversation/snapshots/test_init.ambr

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

tests/components/conversation/test_default_agent.py

@@ -614,6 +614,115 @@ async def test_error_no_intent(hass: HomeAssistant, init_components) -> None:
         )
 
 
+async def test_error_duplicate_names(
+    hass: HomeAssistant, init_components, entity_registry: er.EntityRegistry
+) -> None:
+    """Test error message when multiple devices have the same name (or alias)."""
+    kitchen_light_1 = entity_registry.async_get_or_create("light", "demo", "1234")
+    kitchen_light_2 = entity_registry.async_get_or_create("light", "demo", "5678")
+
+    # Same name and alias
+    for light in (kitchen_light_1, kitchen_light_2):
+        light = entity_registry.async_update_entity(
+            light.entity_id,
+            name="kitchen light",
+            aliases={"overhead light"},
+        )
+        hass.states.async_set(
+            light.entity_id,
+            "off",
+            attributes={ATTR_FRIENDLY_NAME: light.name},
+        )
+
+    # Check name and alias
+    for name in ("kitchen light", "overhead light"):
+        # command
+        result = await conversation.async_converse(
+            hass, f"turn on {name}", None, Context(), None
+        )
+        assert result.response.response_type == intent.IntentResponseType.ERROR
+        assert (
+            result.response.error_code
+            == intent.IntentResponseErrorCode.NO_VALID_TARGETS
+        )
+        assert (
+            result.response.speech["plain"]["speech"]
+            == f"Sorry, there are multiple devices called {name}"
+        )
+
+        # question
+        result = await conversation.async_converse(
+            hass, f"is {name} on?", None, Context(), None
+        )
+        assert result.response.response_type == intent.IntentResponseType.ERROR
+        assert (
+            result.response.error_code
+            == intent.IntentResponseErrorCode.NO_VALID_TARGETS
+        )
+        assert (
+            result.response.speech["plain"]["speech"]
+            == f"Sorry, there are multiple devices called {name}"
+        )
+
+
+async def test_error_duplicate_names_in_area(
+    hass: HomeAssistant,
+    init_components,
+    area_registry: ar.AreaRegistry,
+    entity_registry: er.EntityRegistry,
+) -> None:
+    """Test error message when multiple devices have the same name (or alias)."""
+    area_kitchen = area_registry.async_get_or_create("kitchen_id")
+    area_kitchen = area_registry.async_update(area_kitchen.id, name="kitchen")
+
+    kitchen_light_1 = entity_registry.async_get_or_create("light", "demo", "1234")
+    kitchen_light_2 = entity_registry.async_get_or_create("light", "demo", "5678")
+
+    # Same name and alias
+    for light in (kitchen_light_1, kitchen_light_2):
+        light = entity_registry.async_update_entity(
+            light.entity_id,
+            name="kitchen light",
+            area_id=area_kitchen.id,
+            aliases={"overhead light"},
+        )
+        hass.states.async_set(
+            light.entity_id,
+            "off",
+            attributes={ATTR_FRIENDLY_NAME: light.name},
+        )
+
+    # Check name and alias
+    for name in ("kitchen light", "overhead light"):
+        # command
+        result = await conversation.async_converse(
+            hass, f"turn on {name} in {area_kitchen.name}", None, Context(), None
+        )
+        assert result.response.response_type == intent.IntentResponseType.ERROR
+        assert (
+            result.response.error_code
+            == intent.IntentResponseErrorCode.NO_VALID_TARGETS
+        )
+        assert (
+            result.response.speech["plain"]["speech"]
+            == f"Sorry, there are multiple devices called {name} in the {area_kitchen.name} area"
+        )
+
+        # question
+        result = await conversation.async_converse(
+            hass, f"is {name} on in the {area_kitchen.name}?", None, Context(), None
+        )
+        assert result.response.response_type == intent.IntentResponseType.ERROR
+        assert (
+            result.response.error_code
+            == intent.IntentResponseErrorCode.NO_VALID_TARGETS
+        )
+        assert (
+            result.response.speech["plain"]["speech"]
+            == f"Sorry, there are multiple devices called {name} in the {area_kitchen.name} area"
+        )
+
+
 async def test_no_states_matched_default_error(
     hass: HomeAssistant, init_components, area_registry: ar.AreaRegistry
 ) -> None:
@@ -794,3 +903,112 @@ async def test_same_named_entities_in_different_areas(
     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]
+
+    # Targeting a duplicate name should fail
+    result = await conversation.async_converse(
+        hass, "turn on overhead light", None, Context(), None
+    )
+    assert result.response.response_type == intent.IntentResponseType.ERROR
+
+    # Querying a duplicate name should also fail
+    result = await conversation.async_converse(
+        hass, "is the overhead light on?", None, Context(), None
+    )
+    assert result.response.response_type == intent.IntentResponseType.ERROR
+
+    # But we can still ask questions that don't rely on the name
+    result = await conversation.async_converse(
+        hass, "how many lights are on?", None, Context(), None
+    )
+    assert result.response.response_type == intent.IntentResponseType.QUERY_ANSWER
+
+
+async def test_same_aliased_entities_in_different_areas(
+    hass: HomeAssistant,
+    init_components,
+    area_registry: ar.AreaRegistry,
+    entity_registry: er.EntityRegistry,
+) -> None:
+    """Test that entities with the same alias (but different names) 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 alias, 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="kitchen overhead light",
+        aliases={"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="bedroom overhead light",
+        aliases={"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") == "overhead light"
+    assert result.response.intent.slots.get("name", {}).get("text") == "overhead light"
+    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") == "overhead light"
+    assert result.response.intent.slots.get("name", {}).get("text") == "overhead light"
+    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]
+
+    # Targeting a duplicate alias should fail
+    result = await conversation.async_converse(
+        hass, "turn on overhead light", None, Context(), None
+    )
+    assert result.response.response_type == intent.IntentResponseType.ERROR
+
+    # Querying a duplicate alias should also fail
+    result = await conversation.async_converse(
+        hass, "is the overhead light on?", None, Context(), None
+    )
+    assert result.response.response_type == intent.IntentResponseType.ERROR
+
+    # But we can still ask questions that don't rely on the alias
+    result = await conversation.async_converse(
+        hass, "how many lights are on?", None, Context(), None
+    )
+    assert result.response.response_type == intent.IntentResponseType.QUERY_ANSWER