From 77b25553e3bb7d74434f1cb70f661fdfb8084b7d Mon Sep 17 00:00:00 2001 From: Michael Hansen Date: Thu, 1 Feb 2024 14:10:24 -0600 Subject: [PATCH] Migrate to new intent error response keys (#109269) --- .../components/conversation/default_agent.py | 128 +++++++++--------- .../components/conversation/manifest.json | 2 +- homeassistant/package_constraints.txt | 2 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- .../conversation/snapshots/test_init.ambr | 18 +-- .../conversation/test_default_agent.py | 110 +++++++++++++-- 7 files changed, 178 insertions(+), 86 deletions(-) diff --git a/homeassistant/components/conversation/default_agent.py b/homeassistant/components/conversation/default_agent.py index c91199352137f6..73c177a31504f6 100644 --- a/homeassistant/components/conversation/default_agent.py +++ b/homeassistant/components/conversation/default_agent.py @@ -12,22 +12,15 @@ from typing import IO, Any from hassil.expression import Expression, ListReference, Sequence -from hassil.intents import ( - Intents, - ResponseType, - SlotList, - TextSlotList, - WildcardSlotList, -) +from hassil.intents import Intents, SlotList, TextSlotList, WildcardSlotList from hassil.recognize import ( MISSING_ENTITY, RecognizeResult, - UnmatchedEntity, UnmatchedTextEntity, recognize_all, ) from hassil.util import merge_dict -from home_assistant_intents import get_intents, get_languages +from home_assistant_intents import ErrorKey, get_intents, get_languages import yaml from homeassistant import core, setup @@ -259,7 +252,7 @@ async def async_process(self, user_input: ConversationInput) -> ConversationResu return _make_error_result( language, intent.IntentResponseErrorCode.NO_INTENT_MATCH, - self._get_error_text(ResponseType.NO_INTENT, lang_intents), + self._get_error_text(ErrorKey.NO_INTENT, lang_intents), conversation_id, ) @@ -273,9 +266,7 @@ async def async_process(self, user_input: ConversationInput) -> ConversationResu else "", result.unmatched_entities_list, ) - error_response_type, error_response_args = _get_unmatched_response( - result.unmatched_entities - ) + error_response_type, error_response_args = _get_unmatched_response(result) return _make_error_result( language, intent.IntentResponseErrorCode.NO_VALID_TARGETS, @@ -325,7 +316,7 @@ async def async_process(self, user_input: ConversationInput) -> ConversationResu return _make_error_result( language, intent.IntentResponseErrorCode.FAILED_TO_HANDLE, - self._get_error_text(ResponseType.HANDLE_ERROR, lang_intents), + self._get_error_text(ErrorKey.HANDLE_ERROR, lang_intents), conversation_id, ) except intent.IntentUnexpectedError: @@ -333,7 +324,7 @@ async def async_process(self, user_input: ConversationInput) -> ConversationResu return _make_error_result( language, intent.IntentResponseErrorCode.UNKNOWN, - self._get_error_text(ResponseType.HANDLE_ERROR, lang_intents), + self._get_error_text(ErrorKey.HANDLE_ERROR, lang_intents), conversation_id, ) @@ -795,7 +786,7 @@ def _make_intent_context( def _get_error_text( self, - response_type: ResponseType, + error_key: ErrorKey, lang_intents: LanguageIntents | None, **response_args, ) -> str: @@ -803,7 +794,7 @@ def _get_error_text( if lang_intents is None: return _DEFAULT_ERROR_TEXT - response_key = response_type.value + response_key = error_key.value response_str = ( lang_intents.error_responses.get(response_key) or _DEFAULT_ERROR_TEXT ) @@ -916,59 +907,72 @@ def _make_error_result( return ConversationResult(response, conversation_id) -def _get_unmatched_response( - unmatched_entities: dict[str, UnmatchedEntity], -) -> tuple[ResponseType, dict[str, Any]]: - error_response_type = ResponseType.NO_INTENT - error_response_args: dict[str, Any] = {} +def _get_unmatched_response(result: RecognizeResult) -> tuple[ErrorKey, dict[str, Any]]: + """Get key and template arguments for error when there are unmatched intent entities/slots.""" - if unmatched_name := unmatched_entities.get("name"): - # Unmatched device or entity - assert isinstance(unmatched_name, UnmatchedTextEntity) - error_response_type = ResponseType.NO_ENTITY - error_response_args["entity"] = unmatched_name.text + # Filter out non-text and missing context entities + unmatched_text: dict[str, str] = { + key: entity.text.strip() + for key, entity in result.unmatched_entities.items() + if isinstance(entity, UnmatchedTextEntity) and entity.text != MISSING_ENTITY + } - elif unmatched_area := unmatched_entities.get("area"): - # Unmatched area - assert isinstance(unmatched_area, UnmatchedTextEntity) - error_response_type = ResponseType.NO_AREA - error_response_args["area"] = unmatched_area.text + if unmatched_area := unmatched_text.get("area"): + # area only + return ErrorKey.NO_AREA, {"area": unmatched_area} - return error_response_type, error_response_args + # Area may still have matched + matched_area: str | None = None + if matched_area_entity := result.entities.get("area"): + matched_area = matched_area_entity.text.strip() + + if unmatched_name := unmatched_text.get("name"): + if matched_area: + # device in area + return ErrorKey.NO_ENTITY_IN_AREA, { + "entity": unmatched_name, + "area": matched_area, + } + + # device only + return ErrorKey.NO_ENTITY, {"entity": unmatched_name} + + # Default error + return ErrorKey.NO_INTENT, {} def _get_no_states_matched_response( no_states_error: intent.NoStatesMatchedError, -) -> tuple[ResponseType, dict[str, Any]]: - """Return error response type and template arguments for error.""" - if not ( - no_states_error.area - and (no_states_error.device_classes or no_states_error.domains) - ): - # Device class and domain must be paired with an area for the error - # message. - return ResponseType.NO_INTENT, {} - - error_response_args: dict[str, Any] = {"area": no_states_error.area} - - # Check device classes first, since it's more specific than domain +) -> tuple[ErrorKey, dict[str, Any]]: + """Return key and template arguments for error when intent returns no matching states.""" + + # Device classes should be checked before domains if no_states_error.device_classes: - # No exposed entities of a particular class in an area. - # Example: "close the bedroom windows" - # - # Only use the first device class for the error message - error_response_args["device_class"] = next(iter(no_states_error.device_classes)) - - return ResponseType.NO_DEVICE_CLASS, error_response_args - - # No exposed entities of a domain in an area. - # Example: "turn on lights in kitchen" - assert no_states_error.domains - # - # Only use the first domain for the error message - error_response_args["domain"] = next(iter(no_states_error.domains)) - - return ResponseType.NO_DOMAIN, error_response_args + device_class = next(iter(no_states_error.device_classes)) # first device class + if no_states_error.area: + # device_class in area + return ErrorKey.NO_DEVICE_CLASS_IN_AREA, { + "device_class": device_class, + "area": no_states_error.area, + } + + # device_class only + return ErrorKey.NO_DEVICE_CLASS, {"device_class": device_class} + + if no_states_error.domains: + domain = next(iter(no_states_error.domains)) # first domain + if no_states_error.area: + # domain in area + return ErrorKey.NO_DOMAIN_IN_AREA, { + "domain": domain, + "area": no_states_error.area, + } + + # domain only + return ErrorKey.NO_DOMAIN, {"domain": domain} + + # Default error + return ErrorKey.NO_INTENT, {} def _collect_list_references(expression: Expression, list_names: set[str]) -> None: diff --git a/homeassistant/components/conversation/manifest.json b/homeassistant/components/conversation/manifest.json index 89dd880f69ea93..ea0a11ae657b69 100644 --- a/homeassistant/components/conversation/manifest.json +++ b/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.1.29"] + "requirements": ["hassil==1.6.0", "home-assistant-intents==2024.2.1"] } diff --git a/homeassistant/package_constraints.txt b/homeassistant/package_constraints.txt index c5cea22795ae91..1b47b2693b0b38 100644 --- a/homeassistant/package_constraints.txt +++ b/homeassistant/package_constraints.txt @@ -29,7 +29,7 @@ hass-nabucasa==0.76.0 hassil==1.6.0 home-assistant-bluetooth==1.12.0 home-assistant-frontend==20240131.0 -home-assistant-intents==2024.1.29 +home-assistant-intents==2024.2.1 httpx==0.26.0 ifaddr==0.2.0 janus==1.0.0 diff --git a/requirements_all.txt b/requirements_all.txt index 4b5bcad5d769bf..949c35cc3f6b93 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1062,7 +1062,7 @@ holidays==0.41 home-assistant-frontend==20240131.0 # homeassistant.components.conversation -home-assistant-intents==2024.1.29 +home-assistant-intents==2024.2.1 # homeassistant.components.home_connect homeconnect==0.7.2 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 390bce7559d411..0ce1d0a33dccdf 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -858,7 +858,7 @@ holidays==0.41 home-assistant-frontend==20240131.0 # homeassistant.components.conversation -home-assistant-intents==2024.1.29 +home-assistant-intents==2024.2.1 # homeassistant.components.home_connect homeconnect==0.7.2 diff --git a/tests/components/conversation/snapshots/test_init.ambr b/tests/components/conversation/snapshots/test_init.ambr index 23dab0902a9cca..468f3215cb7457 100644 --- a/tests/components/conversation/snapshots/test_init.ambr +++ b/tests/components/conversation/snapshots/test_init.ambr @@ -339,7 +339,7 @@ 'speech': dict({ 'plain': dict({ 'extra_data': None, - 'speech': 'An unexpected error occurred while handling the intent', + 'speech': 'An unexpected error occurred', }), }), }), @@ -379,7 +379,7 @@ 'speech': dict({ 'plain': dict({ 'extra_data': None, - 'speech': 'An unexpected error occurred while handling the intent', + 'speech': 'An unexpected error occurred', }), }), }), @@ -519,7 +519,7 @@ 'speech': dict({ 'plain': dict({ 'extra_data': None, - 'speech': 'Sorry, I am not aware of any device or entity called late added alias', + 'speech': 'Sorry, I am not aware of any device called late added alias', }), }), }), @@ -539,7 +539,7 @@ 'speech': dict({ 'plain': dict({ 'extra_data': None, - 'speech': 'Sorry, I am not aware of any device or entity called kitchen light', + 'speech': 'Sorry, I am not aware of any device called kitchen light', }), }), }), @@ -679,7 +679,7 @@ 'speech': dict({ 'plain': dict({ 'extra_data': None, - 'speech': 'Sorry, I am not aware of any device or entity called late added light', + 'speech': 'Sorry, I am not aware of any device called late added light', }), }), }), @@ -759,7 +759,7 @@ 'speech': dict({ 'plain': dict({ 'extra_data': None, - 'speech': 'Sorry, I am not aware of any device or entity called kitchen light', + 'speech': 'Sorry, I am not aware of any device called kitchen light', }), }), }), @@ -779,7 +779,7 @@ 'speech': dict({ 'plain': dict({ 'extra_data': None, - 'speech': 'Sorry, I am not aware of any device or entity called my cool light', + 'speech': 'Sorry, I am not aware of any device called my cool light', }), }), }), @@ -919,7 +919,7 @@ 'speech': dict({ 'plain': dict({ 'extra_data': None, - 'speech': 'Sorry, I am not aware of any device or entity called kitchen light', + 'speech': 'Sorry, I am not aware of any device called kitchen light', }), }), }), @@ -969,7 +969,7 @@ 'speech': dict({ 'plain': dict({ 'extra_data': None, - 'speech': 'Sorry, I am not aware of any device or entity called renamed light', + 'speech': 'Sorry, I am not aware of any device called renamed light', }), }), }), diff --git a/tests/components/conversation/test_default_agent.py b/tests/components/conversation/test_default_agent.py index b992b0086d755c..d7182aa3c2f982 100644 --- a/tests/components/conversation/test_default_agent.py +++ b/tests/components/conversation/test_default_agent.py @@ -2,6 +2,7 @@ from collections import defaultdict from unittest.mock import AsyncMock, patch +from hassil.recognize import Intent, IntentData, MatchEntity, RecognizeResult import pytest from homeassistant.components import conversation @@ -430,8 +431,8 @@ async def test_device_area_context( ) -async def test_error_missing_entity(hass: HomeAssistant, init_components) -> None: - """Test error message when entity is missing.""" +async def test_error_no_device(hass: HomeAssistant, init_components) -> None: + """Test error message when device/entity is missing.""" result = await conversation.async_converse( hass, "turn on missing entity", None, Context(), None ) @@ -440,11 +441,11 @@ async def test_error_missing_entity(hass: HomeAssistant, init_components) -> Non assert result.response.error_code == intent.IntentResponseErrorCode.NO_VALID_TARGETS assert ( result.response.speech["plain"]["speech"] - == "Sorry, I am not aware of any device or entity called missing entity" + == "Sorry, I am not aware of any device called missing entity" ) -async def test_error_missing_area(hass: HomeAssistant, init_components) -> None: +async def test_error_no_area(hass: HomeAssistant, init_components) -> None: """Test error message when area is missing.""" result = await conversation.async_converse( hass, "turn on the lights in missing area", None, Context(), None @@ -458,10 +459,60 @@ async def test_error_missing_area(hass: HomeAssistant, init_components) -> None: ) -async def test_error_no_exposed_for_domain( +async def test_error_no_device_in_area( hass: HomeAssistant, init_components, area_registry: ar.AreaRegistry ) -> None: - """Test error message when no entities for a domain are exposed in an area.""" + """Test error message when area is missing a device/entity.""" + area_registry.async_get_or_create("kitchen") + result = await conversation.async_converse( + hass, "turn on missing entity in the kitchen", 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"] + == "Sorry, I am not aware of any device called missing entity in the kitchen area" + ) + + +async def test_error_no_domain( + hass: HomeAssistant, init_components, area_registry: ar.AreaRegistry +) -> None: + """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="") + recognize_result = RecognizeResult( + intent=Intent("HassTurnOn"), + intent_data=IntentData([]), + entities={"domain": fan_domain}, + entities_list=[fan_domain], + ) + + with patch( + "homeassistant.components.conversation.default_agent.recognize_all", + return_value=[recognize_result], + ): + result = await conversation.async_converse( + hass, "turn on the fans", 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"] + == "Sorry, I am not aware of any fan" + ) + + +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") result = await conversation.async_converse( hass, "turn on the lights in the kitchen", None, Context(), None @@ -475,10 +526,43 @@ async def test_error_no_exposed_for_domain( ) -async def test_error_no_exposed_for_device_class( +async def test_error_no_device_class( hass: HomeAssistant, init_components, area_registry: ar.AreaRegistry ) -> None: - """Test error message when no entities of a device class are exposed in an area.""" + """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="") + recognize_result = RecognizeResult( + intent=Intent("HassTurnOn"), + intent_data=IntentData([]), + entities={"device_class": window_class}, + entities_list=[window_class], + ) + + with patch( + "homeassistant.components.conversation.default_agent.recognize_all", + return_value=[recognize_result], + ): + result = await conversation.async_converse( + hass, "open the windows", 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"] + == "Sorry, I am not aware of any window" + ) + + +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") result = await conversation.async_converse( hass, "open bedroom windows", None, Context(), None @@ -492,8 +576,8 @@ async def test_error_no_exposed_for_device_class( ) -async def test_error_match_failure(hass: HomeAssistant, init_components) -> None: - """Test response with complete match failure.""" +async def test_error_no_intent(hass: HomeAssistant, init_components) -> None: + """Test response with an intent match failure.""" with patch( "homeassistant.components.conversation.default_agent.recognize_all", return_value=[], @@ -506,6 +590,10 @@ async def test_error_match_failure(hass: HomeAssistant, init_components) -> None assert ( result.response.error_code == intent.IntentResponseErrorCode.NO_INTENT_MATCH ) + assert ( + result.response.speech["plain"]["speech"] + == "Sorry, I couldn't understand that" + ) async def test_no_states_matched_default_error( @@ -601,5 +689,5 @@ async def test_all_domains_loaded( assert result.response.error_code == intent.IntentResponseErrorCode.NO_VALID_TARGETS assert ( result.response.speech["plain"]["speech"] - == "Sorry, I am not aware of any device or entity called test light" + == "Sorry, I am not aware of any device called test light" )