MycroftAI / mycroft-core

Mycroft Core, the Mycroft Artificial Intelligence platform.
https://mycroft.ai
Apache License 2.0
6.51k stars 1.27k forks source link

Add 'utterances' to message emitted to skill intent handler #2997

Closed NeonDaniel closed 2 years ago

NeonDaniel commented 2 years ago

Description

Adds utterances to message.data that is passed to skill intent handlers. This allows intent handlers to use all transcriptions for STT modules that return alternative transcriptions which may be useful if extracting numbers or other entities that may be present in alternate transcriptions. This also lets intent handlers utilize the same data that is now passed to converse() (as of #2813 ).

How to test

This PR should have no breaking changes or affect behavior in any existing skills. Intent handler methods should now be able to reference the list message.data["utterances"] which will contain message.data["utterance"]

Contributor license agreement signed?

CLA [X] (Whether you have signed a CLA - Contributor Licensing Agreement

pep8speaks commented 2 years ago

Hello @NeonDaniel! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2021-09-10 05:04:15 UTC
devops-mycroft commented 2 years ago

Hello, @NeonDaniel, thank you for helping with the Mycroft project! We welcome everyone into the community and greatly appreciate your help as we work to build an AI for Everyone.

To protect yourself, the project, and users of Mycroft technologies we require a Contributor Licensing Agreement (CLA) before accepting any code contribution. This agreement makes it crystal clear that along with your code you are offering a license to use it within the confines of this project. You retain ownership of the code, this is just a license.

Please visit https://mycroft.ai/cla to initiate this one-time signing. Thank you!

devops-mycroft commented 2 years ago

Voight Kampff Integration Test Succeeded (Results)

codecov-commenter commented 2 years ago

Codecov Report

Merging #2997 (10f1e12) into dev (170ebc4) will decrease coverage by 0.04%. The diff coverage is 61.68%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #2997      +/-   ##
==========================================
- Coverage   52.85%   52.80%   -0.05%     
==========================================
  Files         123      123              
  Lines       11044    11106      +62     
==========================================
+ Hits         5837     5865      +28     
- Misses       5207     5241      +34     
Impacted Files Coverage Δ
mycroft/client/enclosure/__main__.py 0.00% <0.00%> (ø)
mycroft/client/enclosure/mark1/__init__.py 0.00% <0.00%> (ø)
mycroft/client/text/text_client.py 0.00% <0.00%> (ø)
mycroft/messagebus/send_func.py 38.46% <0.00%> (-4.40%) :arrow_down:
mycroft/skills/intent_service.py 67.72% <0.00%> (-0.31%) :arrow_down:
mycroft/filesystem/__init__.py 80.00% <62.50%> (-8.89%) :arrow_down:
mycroft/api/__init__.py 80.76% <66.66%> (-0.09%) :arrow_down:
mycroft/client/speech/hotword_factory.py 70.70% <81.25%> (+0.18%) :arrow_up:
mycroft/util/file_utils.py 94.05% <83.33%> (-0.74%) :arrow_down:
mycroft/skills/event_scheduler.py 68.50% <85.71%> (+0.29%) :arrow_up:
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4bce534...10f1e12. Read the comment docs.

devs-mycroft commented 2 years ago

Hello, @NeonDaniel, thank you for helping with the Mycroft project! We welcome everyone into the community and greatly appreciate your help as we work to build an AI for Everyone.

To protect yourself, the project, and users of Mycroft technologies we require a Contributor Licensing Agreement (CLA) before accepting any code contribution. This agreement makes it crystal clear that along with your code you are offering a license to use it within the confines of this project. You retain ownership of the code, this is just a license.

Please visit https://mycroft.ai/cla to initiate this one-time signing. Thank you!

NeonDaniel commented 2 years ago

In terms of this actual PR, it looks like a good addition to me. My only question would be whether we should expand the comment slightly, maybe including something like:

# Add back original list of utterances for intent handlers
# match.intent_data only includes utterance version with the highest confidence

?

Makes sense to me; updating that now

NeonDaniel commented 2 years ago

Anything I need to do about the CLA Needed label? (or anything else RE this PR)

krisgesling commented 2 years ago

Nope, I was just waiting for everything to pass.

Will need to look into the CLA bot again, there must be something else going on...

Thanks Daniel!