Alexa Intent: Use the 'id' field and expose nearest resolutions as variables #86709

AzonInc · 2023-01-26T11:31:26Z

Proposed change

Expose nearest Resolution (ID and Value) as Variables

Using the id value can be valuable if you don't want to use the id for entities but to process it in scripts for example. The ID Value is exposed as SlotName_ID variable.
In addition to the single value it could be useful to have the nearest resolution always available.

For example:

"TramStation": {
  "name": "TramStation",
  "value": "hauptbahnhof ost",
  "resolutions": {
    "resolutionsPerAuthority": [
    {
        "authority": "amzn1.er-authority.echo-sdk.amzn1.ask.skill.91b2d64a-aaa7-4390-bb66-a196e950d8ea.TramStation",
        "status": {
          "code": "ER_SUCCESS_MATCH"
        },
        "values": [
          {
            "value": {
              "name": "Hauptbahnhof Ost",
              "id": "80029079"
            }
          },
          {
            "value": {
              "name": "Hauptbahnhof West",
              "id": "80029080"
            }
          }
        ]
      }
    ]
  },
  "confirmationStatus": "NONE",
  "source": "USER"
}

In that case I wouldn't get the correct resolution, as it's more than one. So it defaults to the spoken value.
However the first resolution is always the one that fits best.
For that reason I exposed two more variables named "SlotName_NEAREST" and "SlotName_NEAREST_ID".
That way people can use it when they need it and can't rely on the default spoken value.

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes Alexa Intent Slot ID unavailable #86629
This PR is related to issue: Alexa Intent Slot ID unavailable #86629
Link to documentation pull request: Update alexa.intent.markdown home-assistant.io#27326

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

home-assistant · 2023-01-26T11:31:30Z

Hi AzonInc

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

home-assistant · 2023-01-26T11:31:35Z

Hey there @home-assistant/cloud, @ochlocracy, @jbouwh, mind taking a look at this pull request as it has been labeled with an integration (alexa) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of alexa can trigger bot actions by commenting:

@home-assistant close Closes the issue.
@home-assistant rename Awesome new title Change the title of the issue.
@home-assistant reopen Reopen the issue.
@home-assistant unassign alexa Removes the current integration label and assignees on the issue, add the integration domain after the command.

jbouwh · 2023-02-07T16:00:41Z

Please help me to understand why how this ID field would help. This is a part that I am not yet quite familiar with.
It would help if you could explain what steps I need to make use of this.

jbouwh

Please fix the errors, and tests. The PR code breaks the alexa integration code.

AzonInc · 2023-02-07T21:41:04Z

Please fix the errors, and tests. The PR code contains and breaks the alexa integration.

Sorry for that, I'm quite new and I'm not sure where these errors come from.
But you can see that I didn't change any dependencies - just a small change. Unfortunately I have no Idea how to fix that.

I rebased to get up with the latest commits - dunno if it's fixed now due to some work in progress in dev branch before.
Do you have an Idea what is causing that? Looks like this might be somewhat related.

AzonInc · 2023-02-07T21:46:42Z

Please help me to understand why how this ID field would help. This is a part that I am not yet quite familiar with. It would help if you could explain what steps I need to make use of this.

The ID field would help whenever we want to pass some custom IDs (for example train Station Location IDs) to Homeassistant Automations (or Node-RED Addon Flows). It was initially removed because people thought there is no use because they can generate their entity ids by the name. However there are people out there, that dont want to use the Slot Value ID as entity ID, but as a real ID for TV Apps or Train Station IDs.
That way it's not neccesary anymore to get the IDs somewhere else by Name if we could just serve them with Alexa.
It's easier to work with the IDs rather than collecting them later or comparing strings that could have special characters.

I hope I could explain it.

jbouwh · 2023-02-08T07:16:19Z

From what I see in the local tests they need to be fixed:

FAILED tests/components/alexa/test_intent.py::test_intent_request_with_slots - AssertionError: assert 500 == <HTTPStatus.OK: 200>
FAILED tests/components/alexa/test_intent.py::test_intent_request_with_slots_and_synonym_resolution - AssertionError: assert 500 == <HTTPStatus.OK: 200>
FAILED tests/components/alexa/test_intent.py::test_intent_request_with_slots_and_multi_synonym_resolution - AssertionError: assert 500 == <HTTPStatus.OK: 200>
FAILED tests/components/alexa/test_intent.py::test_intent_request_calling_service - AssertionError: assert 500 == <HTTPStatus.OK: 200>
FAILED tests/components/alexa/test_intent.py::test_intent_from_built_in_intent_library - AssertionError: assert 500 == <HTTPStatus.OK: 200>

Results (11.00s):
     182 passed
       5 failed
         - tests/components/alexa/test_intent.py:216 test_intent_request_with_slots
         - tests/components/alexa/test_intent.py:250 test_intent_request_with_slots_and_synonym_resolution
         - tests/components/alexa/test_intent.py:353 test_intent_request_with_slots_and_multi_synonym_resolution
         - tests/components/alexa/test_intent.py:481 test_intent_request_calling_service
         - tests/components/alexa/test_intent.py:553 test_intent_from_built_in_intent_library

The changed code seems not to be called during normal operation. So I do not really get when this code is used. Could you tell me step by step how to set up a test environment to be able to test/debug your code in real live.

AzonInc · 2023-02-08T13:39:39Z

I finally figured how to properly run the Tests and Test Enviroment. All Tests are passing now. I added two more Tests to cover all scenarios of the new variables.

AzonInc · 2023-02-10T19:09:34Z

I updated the branch to reflect the most recent changes to the tests file.

jbouwh · 2023-02-10T23:04:19Z

Can you answer my questions on how this is supposed to be used and especially how I can test your code, and decide if this would fit or not. This might save you a lot of time,

AzonInc · 2023-02-11T11:31:41Z

Scenareo:
Ask Alexa for the next Tram to "Hauptbahnhof West"
Ask Alexa for the next Tram to "Hauptbahnhof Ost"
Ask Alexa for the next Tram to "Hauptbahnhof" (Synonym for West)
Ask Alexa for the next Tram to "Bahnhof" (Synonym for West)

Problem:

Currently multiple resolutions are not supported, even when the first resolution is always the correct one, the spoken value is used instead. As there are always multiple resolutions for everything that contains "Hauptbahnhof" it's failing to resolve "Hauptbahnhof Ost" and "Hauptbahnhof West" even when there is no synonym set. Because Amazon is sending the two but the correct resolution on index 0.
Since the ID Support of the Alexa Integration got removed in the past, it's not possible to use the ID field of the Slot at all which is a pity.

Solution:
I kept that behaviour with the Code changes and added three new variables. So there is no breaking change.
One new Variable for the Slot ID: "SlotName"_ID
Two new Variables for the nearest resolution value and id: "SlotName"_NEAREST, "SlotName"_NEAREST_ID

Alexa Intent Setup:

Alexa Intent Slot Setup:

Homeassistant Intent:

TramIntentTest:
  action:
    - service: input_text.set_value
      data:
        value: "{{ TramStation_NEAREST_ID }}"
      target:
        entity_id: input_text.tram_destination
    - service: button.press
      data: {}
      target:
        entity_id: button.request_tram
  speech:
    type: plain
    text: Wait a Second, I'm looking for a tram to {{ TramStation_NEAREST }}

Entities:
Create one Input Button in HA with id: input_text.tram_destination
Create a Button in NodeRED (Companion Integration) with id: button.request_tram

Now you can ask Alexa for the next Tram to some Station. The Intent is setting the input value to the nerest resolution tram station id and press the button in nodered to start the flow.

AzonInc · 2023-05-09T21:14:23Z

Actually it could be way easier with only that one new Slot ID variable and a behaviour change which would lead to a breaking change tho.

I don't know why it is useful to prevent using the Slot resolutions when there is more than one possible resolution.
But maybe you can explain me the reasons behind the decision.

Let's say we have an Intent Configuration like this:
Utterance: Show me a route to {{ TrainStation}}
Slot Name: TrainStation
Slot Values: South Station, West Station, Apple Street and Junior Street

Behavior of the current implementation:

When there is no or more than one resolution, fall back to spoken Text

If I say "Alexa, show me a route to (West Street | West Station | Apple Street | Junior Street)", it will never use any of the resolutions bacause amazon is resolving them all as there are two values ending with street and two starting with west.
So with the current implementation it will always fall back to the spoken Text in my example.

Suggestion: New implementation

The nearest resolution will always be used, even when there is more than one.
Only fall back to the spoken Text if there is no resolution available.

As far as I have seen and tested, the first resolution has always been the correct one.
The new implementation however will be a breaking change for some people using the Slots as the resolved values might be a little bit different sometimes e.g. lowercase/uppercase/whitespace.

Let me know what you think about it.

jbouwh · 2023-05-10T06:48:36Z

Actually it could be way easier with only that one new Slot ID variable and a behaviour change which would lead to a breaking change tho.

I don't know why it is useful to prevent using the Slot resolutions when there is more than one possible resolution. But maybe you can explain me the reasons behind the decision.

Let's say we have an Intent Configuration like this: Utterance: Show me a route to {{ TrainStation}} Slot Name: TrainStation Slot Values: South Station, West Station, Apple Street and Junior Street

Behavior of the current implementation:
* When there is no or more than one resolution, fall back to spoken Text
If I say "Alexa, show me a route to (West Street | West Station | Apple Street | Junior Street)", it will never use any of the resolutions bacause amazon is resolving them all as there are two values ending with street and two starting with west. So with the current implementation it will always fall back to the spoken Text in my example.

Suggestion: New implementation
* The nearest resolution will always be used, even when there is more than one.

* Only fall back to the spoken Text if there is no resolution available.
As far as I have seen and tested, the first resolution has always been the correct one. The new implementation however will be a breaking change for some people using the Slots as the resolved values might be a little bit different sometimes e.g. lowercase/uppercase/whitespace.

Let me know what you think about it.

I have invest some time to understand what you are suggestion and what would break. If we break anything it would be nice if we could have some backwards compatibility and a deprecation warning (repair). Don't know if that is possible though.
If we can explain well enough what cases will break we could also explain this in the breaking changes release notes.

jbouwh · 2023-05-10T06:53:45Z

btw: i see that the merging of dev pushed a lot of non related commits. to solve this you can do:

git fetch upstream
git rebase -i upstream/dev (select the commits you want to keep, or omit the -i flag)
git push --force
So do not pull in dev in your branch.

jbouwh

Looks good, left one suggestion.

homeassistant/components/alexa/intent.py

AzonInc · 2023-05-10T08:26:38Z

Actually it could be way easier with only that one new Slot ID variable and a behaviour change which would lead to a breaking change tho.
I don't know why it is useful to prevent using the Slot resolutions when there is more than one possible resolution. But maybe you can explain me the reasons behind the decision.
Let's say we have an Intent Configuration like this: Utterance: Show me a route to {{ TrainStation}} Slot Name: TrainStation Slot Values: South Station, West Station, Apple Street and Junior Street
Behavior of the current implementation:
* When there is no or more than one resolution, fall back to spoken Text
If I say "Alexa, show me a route to (West Street | West Station | Apple Street | Junior Street)", it will never use any of the resolutions bacause amazon is resolving them all as there are two values ending with street and two starting with west. So with the current implementation it will always fall back to the spoken Text in my example.
Suggestion: New implementation
* The nearest resolution will always be used, even when there is more than one.

* Only fall back to the spoken Text if there is no resolution available.
As far as I have seen and tested, the first resolution has always been the correct one. The new implementation however will be a breaking change for some people using the Slots as the resolved values might be a little bit different sometimes e.g. lowercase/uppercase/whitespace.
Let me know what you think about it.
I have invest some time to understand what you are suggestion and what would break. If we break anything it would be nice if we could have some backwards compatibility and a deprecation warning (repair). Don't know if that is possible though. If we can explain well enough what cases will break we could also explain this in the breaking changes release notes.

We could add a deprecation warning now for a future release and introduce the changes in the future release.
Actually it's not a severe breaking change as the output is usually pretty much the same.
So for now we could just introduce the 3 new variables and a deprecation warning. And later remove two of the variables again and change the beahvior of the Slot Variable. However I don't think its possible to have a backwards compatibility. The only way I can imagine is renaming the variables and have one called SlotName_OLD.

homeassistant/components/alexa/intent.py

AzonInc · 2023-05-10T15:34:08Z

Now its only the most necessary changes

jbouwh · 2023-05-10T15:36:26Z

Now its only the most necessary changes

Looks good. Now we need to get the CI tests to work first. Needs to wait: https://www.githubstatus.com/

Expose nearest Resolution (ID and Value) as Variables

AzonInc · 2023-05-10T16:15:19Z

The previous CI run failed on one Job without any details and some others were skipped. Most probably due to the degraded performance.

jbouwh

Looks good 👍,
Thnx @AzonInc !

AzonInc · 2023-05-10T19:41:28Z

Looks good 👍, Thnx @AzonInc !

Cool :) Thanks a lot for your support and patience with me.

What would be the process for the other suggested logic change PR?
Also about the Breaking Change and Deprecation Warning

jbouwh · 2023-05-11T05:46:24Z

Looks good 👍, Thnx @AzonInc !

Cool :) Thanks a lot for your support and patience with me.

What would be the process for the other suggested logic change PR? Also about the Breaking Change and Deprecation Warning

Wel we plan to change the behavior and ask users to update there automations. The issue should only show when users have intents configured. But it should be clear wat a user should do to fix it, and we should check on that.
We could also add a config setting that temporary defaults to false and will default to true after deprecation. We show a warning if the value is false, ask users to set it. In that case the chance could be added as a non breaking change. The deprecation could be added later then, changing the default. And later removing the parameter.
We could just change it without an issue making it a breaking change. (I cannot tell how bad that could be, but may be that is fine)

It's up to you.

AzonInc requested review from ochlocracy and jbouwh as code owners January 26, 2023 11:31

home-assistant bot added cla-needed core has-tests integration: alexa new-feature labels Jan 26, 2023

home-assistant bot assigned ochlocracy and jbouwh Jan 26, 2023

home-assistant bot added Quality Scale: No score cla-recheck cla-signed and removed cla-recheck cla-needed labels Jan 26, 2023

jbouwh requested changes Feb 7, 2023

View reviewed changes

AzonInc force-pushed the alexa-intent-add-additional-slot-data branch from 354e45c to 6d1c77e Compare February 7, 2023 21:37

AzonInc requested review from jbouwh and removed request for ochlocracy February 7, 2023 22:31

jbouwh unassigned ochlocracy Feb 8, 2023

AzonInc force-pushed the alexa-intent-add-additional-slot-data branch from bad3871 to 6236fb5 Compare February 10, 2023 19:06

AzonInc force-pushed the alexa-intent-add-additional-slot-data branch from bcba0f8 to 78b1918 Compare May 8, 2023 22:03

jbouwh reviewed May 10, 2023

View reviewed changes

homeassistant/components/alexa/intent.py Outdated Show resolved Hide resolved

AzonInc force-pushed the alexa-intent-add-additional-slot-data branch from 78b1918 to 7a19e55 Compare May 10, 2023 09:12

jbouwh requested changes May 10, 2023

View reviewed changes

homeassistant/components/alexa/intent.py Outdated Show resolved Hide resolved

AzonInc force-pushed the alexa-intent-add-additional-slot-data branch from 7a19e55 to bcadf99 Compare May 10, 2023 14:24

jbouwh reviewed May 10, 2023

View reviewed changes

homeassistant/components/alexa/intent.py Outdated Show resolved Hide resolved

jbouwh reviewed May 10, 2023

View reviewed changes

homeassistant/components/alexa/intent.py Outdated Show resolved Hide resolved

jbouwh marked this pull request as ready for review May 10, 2023 15:35

jbouwh closed this May 10, 2023

jbouwh reopened this May 10, 2023

AzonInc added 7 commits May 10, 2023 16:13

Use the 'id' field and nearest resolutions

6adcb96

Expose nearest Resolution (ID and Value) as Variables

Add more specific type hints

711c24b

Change type definition of request

b281943

Add deprecation warning and remove variables

d63d52a

Remove deprecation warning & update tests

8e937c2

Fix wrong value assignment

fe9d0b1

revert future changes

a1f058f

AzonInc force-pushed the alexa-intent-add-additional-slot-data branch from 67ff50a to a1f058f Compare May 10, 2023 16:13

jbouwh approved these changes May 10, 2023

View reviewed changes

jbouwh merged commit 0f2caf8 into home-assistant:dev May 10, 2023
55 checks passed

github-actions bot locked and limited conversation to collaborators May 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alexa Intent: Use the 'id' field and expose nearest resolutions as variables #86709

Alexa Intent: Use the 'id' field and expose nearest resolutions as variables #86709

AzonInc commented Jan 26, 2023 •

edited by jbouwh

home-assistant bot commented Jan 26, 2023

home-assistant bot commented Jan 26, 2023

jbouwh commented Feb 7, 2023

jbouwh left a comment •

edited

AzonInc commented Feb 7, 2023 •

edited

AzonInc commented Feb 7, 2023 •

edited

jbouwh commented Feb 8, 2023

AzonInc commented Feb 8, 2023 •

edited

AzonInc commented Feb 10, 2023

jbouwh commented Feb 10, 2023

AzonInc commented Feb 11, 2023 •

edited

AzonInc commented May 9, 2023 •

edited

jbouwh commented May 10, 2023

jbouwh commented May 10, 2023

jbouwh left a comment

AzonInc commented May 10, 2023 •

edited

AzonInc commented May 10, 2023 •

edited

jbouwh commented May 10, 2023 •

edited

AzonInc commented May 10, 2023 •

edited

jbouwh left a comment

AzonInc commented May 10, 2023 •

edited

jbouwh commented May 11, 2023 •

edited

Alexa Intent: Use the 'id' field and expose nearest resolutions as variables #86709

Alexa Intent: Use the 'id' field and expose nearest resolutions as variables #86709

Conversation

AzonInc commented Jan 26, 2023 • edited by jbouwh

Proposed change

Type of change

Additional information

Checklist

home-assistant bot commented Jan 26, 2023

home-assistant bot commented Jan 26, 2023

jbouwh commented Feb 7, 2023

jbouwh left a comment • edited

Choose a reason for hiding this comment

AzonInc commented Feb 7, 2023 • edited

AzonInc commented Feb 7, 2023 • edited

jbouwh commented Feb 8, 2023

AzonInc commented Feb 8, 2023 • edited

AzonInc commented Feb 10, 2023

jbouwh commented Feb 10, 2023

AzonInc commented Feb 11, 2023 • edited

AzonInc commented May 9, 2023 • edited

jbouwh commented May 10, 2023

jbouwh commented May 10, 2023

jbouwh left a comment

Choose a reason for hiding this comment

AzonInc commented May 10, 2023 • edited

AzonInc commented May 10, 2023 • edited

jbouwh commented May 10, 2023 • edited

AzonInc commented May 10, 2023 • edited

jbouwh left a comment

Choose a reason for hiding this comment

AzonInc commented May 10, 2023 • edited

jbouwh commented May 11, 2023 • edited

AzonInc commented Jan 26, 2023 •

edited by jbouwh

jbouwh left a comment •

edited

AzonInc commented Feb 7, 2023 •

edited

AzonInc commented Feb 7, 2023 •

edited

AzonInc commented Feb 8, 2023 •

edited

AzonInc commented Feb 11, 2023 •

edited

AzonInc commented May 9, 2023 •

edited

AzonInc commented May 10, 2023 •

edited

AzonInc commented May 10, 2023 •

edited

jbouwh commented May 10, 2023 •

edited

AzonInc commented May 10, 2023 •

edited

AzonInc commented May 10, 2023 •

edited

jbouwh commented May 11, 2023 •

edited