Bypass NLU pipeline when message is /intent or /intent + entities - [ENG 286] #12480
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
There was a problem hiding this comment.
Good job so far! Can you also add the changelog file?
If we really wanted to do this right we should probably also address the following:
- This change is currently difficult to test, because the
RegexMessageHandleris still in the graph. We should remove the RegexMessageHandler from the graph ultimately, but this also involves touching the default recipe etc, because all NLU calls currently target this component as the execution target if I remember correctly. - We'd need to take a deeper look whether the http interpreters should actually also be covered by the regex parsing. This could be relevant if we remove the RegexMessageParser from the graph and then the http interpreters might not have any regex message parsing at all anymore. I am not really familiar with them
Skip sequential integration tests on release tag
Thanks a lot @twerkmeister, your review was very helpful. Is it better that I create a separate task to handle the comments that you said here? |
|
@varunshankar given that |
twerkmeister
approved these changes
Jun 20, 2023
Comment on lines
+117
to
+136
| with mock.patch( | ||
| "rasa.core.processor.MessageProcessor._parse_message_with_graph" | ||
| ) as mocked_function: | ||
| # Case1: message has intent and entities explicitly set. | ||
| message = UserMessage('/greet{"name": "boy"}') | ||
| parsed = await default_processor.parse_message(message) | ||
| assert parsed["intent"][INTENT_NAME_KEY] == "greet" | ||
| assert parsed["entities"][0]["entity"] == "name" | ||
| mocked_function.assert_not_called() | ||
|
|
||
| # Case2: Normal user message. | ||
| parse_data = { | ||
| "text": "mocked", | ||
| "intent": {"name": None, "confidence": 0.0}, | ||
| "entities": [], | ||
| } | ||
| mocked_function.return_value = parse_data | ||
| message = UserMessage("hi hello how are you?") | ||
| parsed = await default_processor.parse_message(message) | ||
| mocked_function.assert_called() |
There was a problem hiding this comment.
This is really smart, I didn't know something like this was possible 😎. Very cool!
There was a problem hiding this comment.
Thanks Thomas, means a lot 😄
|
🚀 A preview of the docs have been deployed at the following URL: https://12480--rasahq-docs-rasa-v2.netlify.app/docs/rasa |
souvikg10
pushed a commit
that referenced
this pull request
Jun 26, 2023
…ENG 286] (#12480) * Bypass NLU pipeline when a message is /intent or /intent + entities
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Proposed changes:
Status (please check what you already did):
black(please check Readme for instructions)