This repository has been archived by the owner on Sep 8, 2024. It is now read-only.
Refactor intent service #2599
Merged
Refactor intent service #2599
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
forslund
added
the
Status: Work in progress
devs-mycroft
added
the
CLA: Yes
forslund
force-pushed
the
refactor/intent-service
branch
from
6c6a61c
to
acf750b
Compare
|
Voight Kampff Integration Test Failed (Results) |
1 similar comment
|
Voight Kampff Integration Test Failed (Results) |
forslund
force-pushed
the
refactor/intent-service
branch
from
acf750b
to
e4e1415
Compare
|
Voight Kampff Integration Test Failed (Results) |
|
Voight Kampff Integration Test Failed (Results) |
|
Voight Kampff Integration Test Succeeded (Results) |
forslund
removed
the
Status: Work in progress
forslund
force-pushed
the
refactor/intent-service
branch
from
e4e1415
to
6e2c9f8
Compare
|
Voight Kampff Integration Test Failed (Results) |
|
Voight Kampff Integration Test Succeeded (Results) |
|
Voight Kampff Integration Test Failed (Results) |
forslund
force-pushed
the
refactor/intent-service
branch
from
6e2c9f8
to
1a91a32
Compare
|
Voight Kampff Integration Test Failed (Results) |
forslund
force-pushed
the
refactor/intent-service
branch
from
1a91a32
to
b593fb3
Compare
|
Voight Kampff Integration Test Failed (Results) |
1 similar comment
|
Voight Kampff Integration Test Failed (Results) |
krisgesling
suggested changes
Aug 10, 2020
|
Good catches all over. Will update the PR during the day |
forslund
force-pushed
the
refactor/intent-service
branch
from
b593fb3
to
ecaa3ae
Compare
|
Voight Kampff Integration Test Succeeded (Results) |
forslund
force-pushed
the
refactor/intent-service
branch
2 times, most recently
from
a4f4d18
to
d84956b
Compare
krisgesling
reviewed
Aug 11, 2020
|
Voight Kampff Integration Test Succeeded (Results) |
If no range is provided it defaults to 0-100 to be backwards compatible
Simplify the handle_utterance into a list of intent matching functions run in order until a match is found. The resolution order is - Converse - Padatious High Confidence - Adapt - Fallback High priority - Padatious Medium Confidence - Fallback Medium priority - Padatious Last ditch effore - Fallback Low priority This collects the many parts of where intent is handled into a single location handling the entire flow. The idea is that, in the end, all the skill calling should be done from this method. The main intent parsers does this but the converse and fallback still calls directly.
This issue was properly fixed in Adapt 0.3.5.
forslund
force-pushed
the
refactor/intent-service
branch
from
5d3a296
to
336dd70
Compare
- Move setting original utterance to the respective intent service - Remove botch limiting the intent service to a single STT hypothesis
- imports - All docstring - AdaptService method warnings
- Add missing docstrings - fix short variable names - restructure return code
forslund
force-pushed
the
refactor/intent-service
branch
from
336dd70
to
8089411
Compare
|
Voight Kampff Integration Test Failed (Results) |
|
Voight Kampff Integration Test Succeeded (Results) |
|
Voight Kampff Integration Test Succeeded (Results) |
forslund
force-pushed
the
refactor/intent-service
branch
from
54380d1
to
8a8caa5
Compare
|
Voight Kampff Integration Test Succeeded (Results) |
ken-mycroft
approved these changes
Sep 22, 2020
ken-mycroft
approved these changes
Sep 22, 2020
ken-mycroft
approved these changes
Sep 22, 2020
krisgesling
approved these changes
Sep 23, 2020
|
Thanks again for this huge clean up Ake, it's fantastic work! I'm going to leave the commits unsquashed as they each represent a significant change and are already well structured. |
forslund
removed
the
Status: Work in progress
Merged
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The intent handling has been spread out over a bunch of files and this sort of collects it back into a single location makes the entire flow viewable from a single handler for the old
recognizer_loop:utterancemessage.The resolution order is the same as before only expressed as a single list instead of a series of checks and fallbacks interlaced.
This collects the many parts of where intent is handled into a single
location handling the entire flow.
The idea is that, in the end, all the skill calling should be done from
this method. The main intent parsers does this but the converse and
fallback still calls directly.
Future work: Converse and fallback still triggers handlers directly.
How to test
Best way to test this is to make sure the voight kampff tests are still passing.
Contributor license agreement signed?
CLA [ Yes ]