From 961c8cc167bfbd4d18e1644a9044af2210a2e9f1 Mon Sep 17 00:00:00 2001 From: Michael Hansen Date: Tue, 13 Dec 2022 16:46:40 -0600 Subject: [PATCH] Update intent response (#83858) * Add language to conversation and intent response * Move language to intent response instead of speech * Extend intent response for voice MVP * Add tests for error conditions in conversation/process * Move intent response type data into "data" field * Move intent response error message back to speech * Remove "success" from intent response * Add id to target in intent response * target defaults to None * Update homeassistant/helpers/intent.py * Fix test * Return conversation_id and multiple targets * Clean up git mess * Fix linting errors * Fix more async_handle signatures * Separate conversation_id and IntentResponse * Add unknown error code * Add ConversationResult * Don't set domain on single entity * Language is required for intent response * Add partial_action_done * Default language in almond agent Co-authored-by: Paulus Schoutsen --- homeassistant/components/almond/__init__.py | 11 +- .../components/conversation/__init__.py | 94 ++++---- .../components/conversation/agent.py | 19 +- .../components/conversation/default_agent.py | 10 +- homeassistant/components/humidifier/intent.py | 2 + homeassistant/components/intent/__init__.py | 5 +- .../components/intent_script/__init__.py | 4 +- homeassistant/components/light/intent.py | 2 + .../components/shopping_list/intent.py | 6 +- homeassistant/helpers/intent.py | 69 ++++-- tests/components/conversation/test_init.py | 218 +++++++++++++----- tests/components/intent/test_init.py | 2 +- 12 files changed, 301 insertions(+), 141 deletions(-) diff --git a/homeassistant/components/almond/__init__.py b/homeassistant/components/almond/__init__.py index a391795df15..fe1ff65f932 100644 --- a/homeassistant/components/almond/__init__.py +++ b/homeassistant/components/almond/__init__.py @@ -291,9 +291,10 @@ class AlmondAgent(conversation.AbstractConversationAgent): context: Context, conversation_id: str | None = None, language: str | None = None, - ) -> intent.IntentResponse: + ) -> conversation.ConversationResult | None: """Process a sentence.""" response = await self.api.async_converse_text(text, conversation_id) + language = language or self.hass.config.language first_choice = True buffer = "" @@ -314,6 +315,8 @@ class AlmondAgent(conversation.AbstractConversationAgent): buffer += "," buffer += f" {message['title']}" - intent_result = intent.IntentResponse(language=language) - intent_result.async_set_speech(buffer.strip()) - return intent_result + intent_response = intent.IntentResponse(language=language) + intent_response.async_set_speech(buffer.strip()) + return conversation.ConversationResult( + response=intent_response, conversation_id=conversation_id + ) diff --git a/homeassistant/components/conversation/__init__.py b/homeassistant/components/conversation/__init__.py index dabd12968a8..54b69a7ad52 100644 --- a/homeassistant/components/conversation/__init__.py +++ b/homeassistant/components/conversation/__init__.py @@ -1,7 +1,6 @@ """Support for functionality to have conversations with Home Assistant.""" from __future__ import annotations -from http import HTTPStatus import logging import re from typing import Any @@ -16,7 +15,7 @@ from homeassistant.helpers import config_validation as cv, intent from homeassistant.helpers.typing import ConfigType from homeassistant.loader import bind_hass -from .agent import AbstractConversationAgent +from .agent import AbstractConversationAgent, ConversationResult from .default_agent import DefaultAgent, async_register _LOGGER = logging.getLogger(__name__) @@ -101,16 +100,14 @@ async def websocket_process( msg: dict[str, Any], ) -> None: """Process text.""" - connection.send_result( - msg["id"], - await _async_converse( - hass, - msg["text"], - msg.get("conversation_id"), - connection.context(msg), - msg.get("language"), - ), + result = await _async_converse( + hass, + msg["text"], + msg.get("conversation_id"), + connection.context(msg), + msg.get("language"), ) + connection.send_result(msg["id"], result.as_dict()) @websocket_api.websocket_command({"type": "conversation/agent/info"}) @@ -168,29 +165,15 @@ class ConversationProcessView(http.HomeAssistantView): async def post(self, request, data): """Send a request for processing.""" hass = request.app["hass"] + result = await _async_converse( + hass, + text=data["text"], + conversation_id=data.get("conversation_id"), + context=self.context(request), + language=data.get("language"), + ) - try: - intent_result = await _async_converse( - hass, - text=data["text"], - conversation_id=data.get("conversation_id"), - context=self.context(request), - language=data.get("language"), - ) - except intent.IntentError as err: - _LOGGER.error("Error handling intent: %s", err) - return self.json( - { - "success": False, - "error": { - "code": str(err.__class__.__name__).lower(), - "message": str(err), - }, - }, - status_code=HTTPStatus.INTERNAL_SERVER_ERROR, - ) - - return self.json(intent_result) + return self.json(result.as_dict()) async def _get_agent(hass: core.HomeAssistant) -> AbstractConversationAgent: @@ -207,37 +190,50 @@ async def _async_converse( conversation_id: str | None, context: core.Context, language: str | None = None, -) -> intent.IntentResponse: +) -> ConversationResult: """Process text and get intent.""" agent = await _get_agent(hass) if language is None: language = hass.config.language + result: ConversationResult | None = None + intent_response: intent.IntentResponse | None = None + try: - intent_result = await agent.async_process( - text, context, conversation_id, language - ) + result = await agent.async_process(text, context, conversation_id, language) except intent.IntentHandleError as err: # Match was successful, but target(s) were invalid - intent_result = intent.IntentResponse(language=language) - intent_result.async_set_error( + intent_response = intent.IntentResponse(language=language) + intent_response.async_set_error( intent.IntentResponseErrorCode.NO_VALID_TARGETS, str(err), ) except intent.IntentUnexpectedError as err: # Match was successful, but an error occurred while handling intent - intent_result = intent.IntentResponse(language=language) - intent_result.async_set_error( + intent_response = intent.IntentResponse(language=language) + intent_response.async_set_error( intent.IntentResponseErrorCode.FAILED_TO_HANDLE, str(err), ) - - if intent_result is None: - # Match was not successful - intent_result = intent.IntentResponse(language=language) - intent_result.async_set_error( - intent.IntentResponseErrorCode.NO_INTENT_MATCH, - "Sorry, I didn't understand that", + except intent.IntentError as err: + # Unknown error + intent_response = intent.IntentResponse(language=language) + intent_response.async_set_error( + intent.IntentResponseErrorCode.UNKNOWN, + str(err), ) - return intent_result + if result is None: + if intent_response is None: + # Match was not successful + intent_response = intent.IntentResponse(language=language) + intent_response.async_set_error( + intent.IntentResponseErrorCode.NO_INTENT_MATCH, + "Sorry, I didn't understand that", + ) + + result = ConversationResult( + response=intent_response, conversation_id=conversation_id + ) + + return result diff --git a/homeassistant/components/conversation/agent.py b/homeassistant/components/conversation/agent.py index d38c6acb936..0bd3f018589 100644 --- a/homeassistant/components/conversation/agent.py +++ b/homeassistant/components/conversation/agent.py @@ -2,11 +2,28 @@ from __future__ import annotations from abc import ABC, abstractmethod +from dataclasses import dataclass +from typing import Any from homeassistant.core import Context from homeassistant.helpers import intent +@dataclass +class ConversationResult: + """Result of async_process.""" + + response: intent.IntentResponse + conversation_id: str | None = None + + def as_dict(self) -> dict[str, Any]: + """Return result as a dict.""" + return { + "response": self.response.as_dict(), + "conversation_id": self.conversation_id, + } + + class AbstractConversationAgent(ABC): """Abstract conversation agent.""" @@ -30,5 +47,5 @@ class AbstractConversationAgent(ABC): context: Context, conversation_id: str | None = None, language: str | None = None, - ) -> intent.IntentResponse | None: + ) -> ConversationResult | None: """Process a sentence.""" diff --git a/homeassistant/components/conversation/default_agent.py b/homeassistant/components/conversation/default_agent.py index 2c92d5245b6..2e8e78d6f38 100644 --- a/homeassistant/components/conversation/default_agent.py +++ b/homeassistant/components/conversation/default_agent.py @@ -14,7 +14,7 @@ from homeassistant.core import callback from homeassistant.helpers import intent from homeassistant.setup import ATTR_COMPONENT -from .agent import AbstractConversationAgent +from .agent import AbstractConversationAgent, ConversationResult from .const import DOMAIN from .util import create_matcher @@ -116,7 +116,7 @@ class DefaultAgent(AbstractConversationAgent): context: core.Context, conversation_id: str | None = None, language: str | None = None, - ) -> intent.IntentResponse | None: + ) -> ConversationResult | None: """Process a sentence.""" intents = self.hass.data[DOMAIN] @@ -125,7 +125,7 @@ class DefaultAgent(AbstractConversationAgent): if not (match := matcher.match(text)): continue - return await intent.async_handle( + intent_response = await intent.async_handle( self.hass, DOMAIN, intent_type, @@ -135,4 +135,8 @@ class DefaultAgent(AbstractConversationAgent): language, ) + return ConversationResult( + response=intent_response, conversation_id=conversation_id + ) + return None diff --git a/homeassistant/components/humidifier/intent.py b/homeassistant/components/humidifier/intent.py index 57f42f58fe0..4d28cf5838c 100644 --- a/homeassistant/components/humidifier/intent.py +++ b/homeassistant/components/humidifier/intent.py @@ -1,4 +1,6 @@ """Intents for the humidifier integration.""" +from __future__ import annotations + import voluptuous as vol from homeassistant.const import ATTR_ENTITY_ID, ATTR_MODE, STATE_OFF diff --git a/homeassistant/components/intent/__init__.py b/homeassistant/components/intent/__init__.py index c43643b0244..c6ca9212c74 100644 --- a/homeassistant/components/intent/__init__.py +++ b/homeassistant/components/intent/__init__.py @@ -63,6 +63,7 @@ class IntentHandleView(http.HomeAssistantView): async def post(self, request, data): """Handle intent with name/data.""" hass = request.app["hass"] + language = hass.config.language try: intent_name = data["name"] @@ -73,11 +74,11 @@ class IntentHandleView(http.HomeAssistantView): hass, DOMAIN, intent_name, slots, "", self.context(request) ) except intent.IntentHandleError as err: - intent_result = intent.IntentResponse() + intent_result = intent.IntentResponse(language=language) intent_result.async_set_speech(str(err)) if intent_result is None: - intent_result = intent.IntentResponse() + intent_result = intent.IntentResponse(language=language) intent_result.async_set_speech("Sorry, I couldn't handle that") return self.json(intent_result) diff --git a/homeassistant/components/intent_script/__init__.py b/homeassistant/components/intent_script/__init__.py index 0090b1def9b..128c9332aeb 100644 --- a/homeassistant/components/intent_script/__init__.py +++ b/homeassistant/components/intent_script/__init__.py @@ -1,4 +1,6 @@ """Handle intents with scripts.""" +from __future__ import annotations + import copy import logging @@ -77,7 +79,7 @@ class ScriptIntentHandler(intent.IntentHandler): self.intent_type = intent_type self.config = config - async def async_handle(self, intent_obj): + async def async_handle(self, intent_obj: intent.Intent): """Handle the intent.""" speech = self.config.get(CONF_SPEECH) reprompt = self.config.get(CONF_REPROMPT) diff --git a/homeassistant/components/light/intent.py b/homeassistant/components/light/intent.py index 2ffd73a9792..e85602f763a 100644 --- a/homeassistant/components/light/intent.py +++ b/homeassistant/components/light/intent.py @@ -1,4 +1,6 @@ """Intents for the light integration.""" +from __future__ import annotations + import voluptuous as vol from homeassistant.const import ATTR_ENTITY_ID, SERVICE_TURN_ON diff --git a/homeassistant/components/shopping_list/intent.py b/homeassistant/components/shopping_list/intent.py index 4f5a39171b8..c709322e0b7 100644 --- a/homeassistant/components/shopping_list/intent.py +++ b/homeassistant/components/shopping_list/intent.py @@ -1,4 +1,6 @@ """Intents for the Shopping List integration.""" +from __future__ import annotations + from homeassistant.helpers import intent import homeassistant.helpers.config_validation as cv @@ -20,7 +22,7 @@ class AddItemIntent(intent.IntentHandler): intent_type = INTENT_ADD_ITEM slot_schema = {"item": cv.string} - async def async_handle(self, intent_obj): + async def async_handle(self, intent_obj: intent.Intent): """Handle the intent.""" slots = self.async_validate_slots(intent_obj.slots) item = slots["item"]["value"] @@ -38,7 +40,7 @@ class ListTopItemsIntent(intent.IntentHandler): intent_type = INTENT_LAST_ITEMS slot_schema = {"item": cv.string} - async def async_handle(self, intent_obj): + async def async_handle(self, intent_obj: intent.Intent): """Handle the intent.""" items = intent_obj.hass.data[DOMAIN].items[-5:] response = intent_obj.create_response() diff --git a/homeassistant/helpers/intent.py b/homeassistant/helpers/intent.py index f846db6d319..c0e965d6f13 100644 --- a/homeassistant/helpers/intent.py +++ b/homeassistant/helpers/intent.py @@ -221,12 +221,14 @@ class ServiceIntentHandler(IntentHandler): response = intent_obj.create_response() response.async_set_speech(self.speech.format(state.name)) - response.async_set_target( - IntentResponseTarget( - name=state.name, - type=IntentResponseTargetType.ENTITY, - id=state.entity_id, - ) + response.async_set_targets( + [ + IntentResponseTarget( + type=IntentResponseTargetType.ENTITY, + name=state.name, + id=state.entity_id, + ), + ], ) return response @@ -279,7 +281,7 @@ class Intent: @callback def create_response(self) -> IntentResponse: """Create a response.""" - return IntentResponse(self, language=self.language) + return IntentResponse(language=self.language, intent=self) class IntentResponseType(Enum): @@ -288,6 +290,9 @@ class IntentResponseType(Enum): ACTION_DONE = "action_done" """Intent caused an action to occur""" + PARTIAL_ACTION_DONE = "partial_action_done" + """Intent caused an action, but it could only be partially done""" + QUERY_ANSWER = "query_answer" """Response is an answer to a query""" @@ -307,6 +312,9 @@ class IntentResponseErrorCode(str, Enum): FAILED_TO_HANDLE = "failed_to_handle" """Unexpected error occurred while handling intent""" + UNKNOWN = "unknown" + """Error outside the scope of intent processing""" + class IntentResponseTargetType(str, Enum): """Type of target for an intent response.""" @@ -314,12 +322,14 @@ class IntentResponseTargetType(str, Enum): AREA = "area" DEVICE = "device" ENTITY = "entity" - OTHER = "other" + DOMAIN = "domain" + DEVICE_CLASS = "device_class" + CUSTOM = "custom" @dataclass class IntentResponseTarget: - """Main target of the intent response.""" + """Target of the intent response.""" name: str type: IntentResponseTargetType @@ -331,17 +341,19 @@ class IntentResponse: def __init__( self, + language: str, intent: Intent | None = None, - language: str | None = None, ) -> None: """Initialize an IntentResponse.""" + self.language = language self.intent = intent self.speech: dict[str, dict[str, Any]] = {} self.reprompt: dict[str, dict[str, Any]] = {} self.card: dict[str, dict[str, str]] = {} - self.language = language self.error_code: IntentResponseErrorCode | None = None - self.target: IntentResponseTarget | None = None + self.targets: list[IntentResponseTarget] = [] + self.success_targets: list[IntentResponseTarget] = [] + self.failed_targets: list[IntentResponseTarget] = [] if (self.intent is not None) and (self.intent.category == IntentCategory.QUERY): # speech will be the answer to the query @@ -392,9 +404,20 @@ class IntentResponse: self.async_set_speech(message) @callback - def async_set_target(self, target: IntentResponseTarget) -> None: - """Set response target.""" - self.target = target + def async_set_targets(self, targets: list[IntentResponseTarget]) -> None: + """Set response targets.""" + self.targets = targets + + @callback + def async_set_partial_action_done( + self, + success_targets: list[IntentResponseTarget], + failed_targets: list[IntentResponseTarget], + ) -> None: + """Set response targets.""" + self.response_type = IntentResponseType.PARTIAL_ACTION_DONE + self.success_targets = success_targets + self.failed_targets = failed_targets @callback def as_dict(self) -> dict[str, Any]: @@ -416,9 +439,19 @@ class IntentResponse: response_data["code"] = self.error_code.value else: # action done or query answer - response_data["target"] = ( - dataclasses.asdict(self.target) if self.target is not None else None - ) + response_data["targets"] = [ + dataclasses.asdict(target) for target in self.targets + ] + + if self.response_type == IntentResponseType.PARTIAL_ACTION_DONE: + # Add success/failed targets + response_data["success"] = [ + dataclasses.asdict(target) for target in self.success_targets + ] + + response_data["failed"] = [ + dataclasses.asdict(target) for target in self.failed_targets + ] response_dict["data"] = response_data diff --git a/tests/components/conversation/test_init.py b/tests/components/conversation/test_init.py index fc28c76b182..a2baeccc710 100644 --- a/tests/components/conversation/test_init.py +++ b/tests/components/conversation/test_init.py @@ -131,18 +131,97 @@ async def test_http_processing_intent(hass, hass_client, hass_admin_user): data = await resp.json() assert data == { - "response_type": "action_done", - "card": { - "simple": {"content": "You chose a Grolsch.", "title": "Beer ordered"} + "response": { + "response_type": "action_done", + "card": { + "simple": {"content": "You chose a Grolsch.", "title": "Beer ordered"} + }, + "speech": { + "plain": { + "extra_data": None, + "speech": "I've ordered a Grolsch!", + } + }, + "language": hass.config.language, + "data": {"targets": []}, }, - "speech": { - "plain": { - "extra_data": None, - "speech": "I've ordered a Grolsch!", + "conversation_id": None, + } + + +async def test_http_partial_action(hass, hass_client, hass_admin_user): + """Test processing intent via HTTP API with a partial completion.""" + + class TestIntentHandler(intent.IntentHandler): + """Test Intent Handler.""" + + intent_type = "TurnOffLights" + + async def async_handle(self, handle_intent: intent.Intent): + """Handle the intent.""" + response = handle_intent.create_response() + area = handle_intent.slots["area"]["value"] + response.async_set_targets( + [ + intent.IntentResponseTarget( + type=intent.IntentResponseTargetType.AREA, name=area, id=area + ) + ] + ) + + # Mark some targets as successful, others as failed + response.async_set_partial_action_done( + success_targets=[ + intent.IntentResponseTarget( + type=intent.IntentResponseTargetType.ENTITY, + name="light1", + id="light.light1", + ) + ], + failed_targets=[ + intent.IntentResponseTarget( + type=intent.IntentResponseTargetType.ENTITY, + name="light2", + id="light.light2", + ) + ], + ) + return response + + intent.async_register(hass, TestIntentHandler()) + + result = await async_setup_component( + hass, + "conversation", + { + "conversation": { + "intents": {"TurnOffLights": ["turn off the lights in the {area}"]} } }, - "language": hass.config.language, - "data": {"target": None}, + ) + assert result + + client = await hass_client() + resp = await client.post( + "/api/conversation/process", json={"text": "Turn off the lights in the kitchen"} + ) + + assert resp.status == HTTPStatus.OK + data = await resp.json() + + assert data == { + "response": { + "response_type": "partial_action_done", + "card": {}, + "speech": {}, + "language": hass.config.language, + "data": { + "targets": [{"type": "area", "id": "kitchen", "name": "kitchen"}], + "success": [{"type": "entity", "id": "light.light1", "name": "light1"}], + "failed": [{"type": "entity", "id": "light.light2", "name": "light2"}], + }, + }, + "conversation_id": None, } @@ -213,17 +292,22 @@ async def test_http_api(hass, init_components, hass_client): data = await resp.json() assert data == { - "card": {}, - "speech": {"plain": {"extra_data": None, "speech": "Turned kitchen on"}}, - "language": hass.config.language, - "response_type": "action_done", - "data": { - "target": { - "name": "kitchen", - "type": "entity", - "id": "light.kitchen", - } + "response": { + "card": {}, + "speech": {"plain": {"extra_data": None, "speech": "Turned kitchen on"}}, + "language": hass.config.language, + "response_type": "action_done", + "data": { + "targets": [ + { + "type": "entity", + "name": "kitchen", + "id": "light.kitchen", + }, + ] + }, }, + "conversation_id": None, } assert len(calls) == 1 @@ -243,18 +327,21 @@ async def test_http_api_no_match(hass, init_components, hass_client): data = await resp.json() assert data == { - "card": {}, - "speech": { - "plain": { - "extra_data": None, - "speech": "Sorry, I didn't understand that", + "response": { + "card": {}, + "speech": { + "plain": { + "extra_data": None, + "speech": "Sorry, I didn't understand that", + }, + }, + "language": hass.config.language, + "response_type": "error", + "data": { + "code": "no_intent_match", }, }, - "language": hass.config.language, - "response_type": "error", - "data": { - "code": "no_intent_match", - }, + "conversation_id": None, } @@ -270,18 +357,21 @@ async def test_http_api_no_valid_targets(hass, init_components, hass_client): data = await resp.json() assert data == { - "response_type": "error", - "card": {}, - "speech": { - "plain": { - "extra_data": None, - "speech": "Unable to find an entity called kitchen", + "response": { + "response_type": "error", + "card": {}, + "speech": { + "plain": { + "extra_data": None, + "speech": "Unable to find an entity called kitchen", + }, + }, + "language": hass.config.language, + "data": { + "code": "no_valid_targets", }, }, - "language": hass.config.language, - "data": { - "code": "no_valid_targets", - }, + "conversation_id": None, } @@ -306,18 +396,21 @@ async def test_http_api_handle_failure(hass, init_components, hass_client): data = await resp.json() assert data == { - "response_type": "error", - "card": {}, - "speech": { - "plain": { - "extra_data": None, - "speech": "Unexpected error turning on the kitchen light", - } - }, - "language": hass.config.language, - "data": { - "code": "failed_to_handle", + "response": { + "response_type": "error", + "card": {}, + "speech": { + "plain": { + "extra_data": None, + "speech": "Unexpected error turning on the kitchen light", + } + }, + "language": hass.config.language, + "data": { + "code": "failed_to_handle", + }, }, + "conversation_id": None, } @@ -345,7 +438,9 @@ async def test_custom_agent(hass, hass_client, hass_admin_user): calls.append((text, context, conversation_id, language)) response = intent.IntentResponse(language=language) response.async_set_speech("Test response") - return response + return conversation.ConversationResult( + response=response, conversation_id=conversation_id + ) conversation.async_set_agent(hass, MyAgent()) @@ -363,16 +458,19 @@ async def test_custom_agent(hass, hass_client, hass_admin_user): ) assert resp.status == HTTPStatus.OK assert await resp.json() == { - "response_type": "action_done", - "card": {}, - "speech": { - "plain": { - "extra_data": None, - "speech": "Test response", - } + "response": { + "response_type": "action_done", + "card": {}, + "speech": { + "plain": { + "extra_data": None, + "speech": "Test response", + } + }, + "language": "test-language", + "data": {"targets": []}, }, - "language": "test-language", - "data": {"target": None}, + "conversation_id": "test-conv-id", } assert len(calls) == 1 diff --git a/tests/components/intent/test_init.py b/tests/components/intent/test_init.py index ffea2a469a4..79fc2ea7480 100644 --- a/tests/components/intent/test_init.py +++ b/tests/components/intent/test_init.py @@ -54,7 +54,7 @@ async def test_http_handle_intent(hass, hass_client, hass_admin_user): }, "language": hass.config.language, "response_type": "action_done", - "data": {"target": None}, + "data": {"targets": []}, }