ATO 678 missing slot set event on a custom action filled slot #12314
Conversation
changelog/12314.improvement.md
Outdated
| @@ -0,0 +1 @@ | |||
| `SlotSet` events will be emitted when the value set by the custom action is the same as the existing value. | |||
There was a problem hiding this comment.
I'd give more context that this case needs to be handled to allow the AugMemo Policy to function properly.
There was a problem hiding this comment.
Sure, I've added this context to the changelog
There was a problem hiding this comment.
Regarding the failures in the CI - you will need to adapt this test to include the duplicate SlotSet event now that the code doesn't prevent this from being added to the tracker.
I'd also recommend going a bit beyond the existing unit test you added and creating a regression test that trains an agent that uses AugMemo policy in its config to check that the agent behaves as expected (similar to the scenario in the ticket description for example).
Co-authored-by: Anca Lita <27920906+ancalita@users.noreply.github.com>
…filled-slot' of github.com:RasaHQ/rasa into ATO-678-jpmc-missing-slot-set-event-on-a-custom-action-filled-slot
|
Kudos, SonarCloud Quality Gate passed!
|
vcidst
changed the title
| # Check that the bot asks for the type of RSA token | ||
| with aioresponses() as mocked: | ||
| mocked.post( | ||
| action_server_url, | ||
| payload={ | ||
| "events": [ | ||
| { | ||
| "event": "slot", | ||
| "name": "rsa_token", | ||
| "value": "unknown", | ||
| } | ||
| ], | ||
| "responses": [], | ||
| }, | ||
| ) | ||
|
|
||
| # Send the first message | ||
| await agent.handle_message( | ||
| _build_user_message(output_channel, "help me install a rsa token") | ||
| ) | ||
| await remote_action.run( | ||
| CollectingOutputChannel(), | ||
| TemplatedNaturalLanguageGenerator(domain.responses), | ||
| tracker, | ||
| domain, | ||
| ) | ||
|
|
||
| assert output_channel.messages[-1] == { | ||
| "recipient_id": SENDER, | ||
| "text": "What type of rsa token do you need?", | ||
| } |
There was a problem hiding this comment.
@ancalita Can you please help me figure out what's wrong here? The assertion fails with,
FAILED tests/regressions/test_action_extract_slots_12314.py::test_setting_slot_with_custom_action - AssertionError: assert {'recipient_i...type of None'} == {'recipient_i...do you need?'}
Omitting 1 identical items, use -vv to show
Differing items:
{'text': 'User has requested rsa token type of None'} != {'text': 'What type of rsa token do you need?'}
Full diff:
- {'recipient_id': 'sender', 'text': 'What type of rsa token do you need?'}
? ^ ---- ^^^ ^ ^^ -- ---
+ {'recipient_id': 'sender', 'text': 'User has requested rsa token type of None'}
? ^^^^^ ++++++++ ^ ^^^^^ ^^^
The bot instead uttered 'User has requested rsa token type of None' this happens when the custom action specified in domain.yaml is not run.
There was a problem hiding this comment.
I think what's needed is to specify to the agent its action_endpoint attribute:
agent.action_endpoint = endpoint
Then the agent can run the RemoteAction without you having to execute it in the test code. Hope that works 🤞🏻
There was a problem hiding this comment.
@ancalita I tried that it still fails, do you have any other ideas?
There was a problem hiding this comment.
Ah, I'm out of ideas, then maybe we can't test this without having a proper integration test with an instance of an action server running. I'd say this might require more effort, so if the manual tests validated the solution, let's not spend more time on the integration test, sorry for setting you on wild goose chase 😞
There was a problem hiding this comment.
@ancalita I think this was a great idea but I'm not sure at this point how to make this test work. Can I mark it as flaky and keep it there with some TODO comments?
There was a problem hiding this comment.
I think the flaky workflows have required status on main, so you'd have to comment out the failing assertions to prevent the test from failing. I also think that flaky marker doesn't describe this test well, because it always fails in current form 😄 Therefore I'd not add the flaky marker.
There was a problem hiding this comment.
@ancalita In that case, I've removed the regression tests from this PR. Can you please review it again?
There was a problem hiding this comment.
Sure, could you also please add a ticket in Jira maintenance epic for writing up an integration test with an instance of the action server, linking to this PR? 🙏🏻
There was a problem hiding this comment.
Yes, I've added a ticket here: https://rasahq.atlassian.net/browse/ATO-1046
|
🚀 A preview of the docs have been deployed at the following URL: https://12314--rasahq-docs-rasa-v2.netlify.app/docs/rasa |
vcidst
deleted the
ATO-678-jpmc-missing-slot-set-event-on-a-custom-action-filled-slot
branch
Proposed changes:
tracker.get(event.name) == event.valueexists?Status (please check what you already did):
black(please check Readme for instructions)