From e3121919af4ca086562958d31d860ffe55c22ff8 Mon Sep 17 00:00:00 2001 From: lluchini Date: Thu, 9 Apr 2020 12:03:33 -0300 Subject: [PATCH 01/14] The default action 'action_session_start' wasn't able to receive the metadata from the message sent, RASA when initiating a new session it sends a blank tracker with no information whatsoever, ignoring the metadata into the message. Adjusted to add the event sessionStart to tracker events and with it the metadata from the message. --- rasa/core/processor.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/rasa/core/processor.py b/rasa/core/processor.py index 7fcfb5123c8e..60cbd4ed55e9 100644 --- a/rasa/core/processor.py +++ b/rasa/core/processor.py @@ -36,6 +36,7 @@ ReminderScheduled, SlotSet, UserUttered, + SessionStarted ) from rasa.core.interpreter import ( INTENT_MESSAGE_PREFIX, @@ -175,7 +176,7 @@ async def _update_tracker_session( ) async def get_tracker_with_session_start( - self, sender_id: Text, output_channel: Optional[OutputChannel] = None + self, sender_id: Text, output_channel: Optional[OutputChannel] = None, metadata: Optional[Dict] = None ) -> Optional[DialogueStateTracker]: """Get tracker for `sender_id` or create a new tracker for `sender_id`. @@ -193,6 +194,9 @@ async def get_tracker_with_session_start( if not tracker: return None + if metadata: + tracker.events.append(SessionStarted(metadata=metadata)) + await self._update_tracker_session(tracker, output_channel) return tracker @@ -233,7 +237,7 @@ async def log_message( # we have a Tracker instance for each user # which maintains conversation state tracker = await self.get_tracker_with_session_start( - message.sender_id, message.output_channel + message.sender_id, message.output_channel, message.metadata ) if tracker: From b53cdb1b59e12bce5652a269aaf8324002be9b35 Mon Sep 17 00:00:00 2001 From: lluchini Date: Thu, 9 Apr 2020 12:13:14 -0300 Subject: [PATCH 02/14] Updating doc string for: Method: get_tracker_with_session_start Parameter: metadata --- rasa/core/processor.py | 1 + 1 file changed, 1 insertion(+) diff --git a/rasa/core/processor.py b/rasa/core/processor.py index 60cbd4ed55e9..f1ba40691c42 100644 --- a/rasa/core/processor.py +++ b/rasa/core/processor.py @@ -183,6 +183,7 @@ async def get_tracker_with_session_start( If a new tracker is created, `action_session_start` is run. Args: + metadata: Data sent from client associated with the incoming user message. output_channel: Output channel associated with the incoming user message. sender_id: Conversation ID for which to fetch the tracker. From 09260e318ea7bfc6846aedb8a2557aea54aeae16 Mon Sep 17 00:00:00 2001 From: lluchini Date: Thu, 9 Apr 2020 15:06:44 -0300 Subject: [PATCH 03/14] PEP8 - line too long adjust. --- rasa/core/processor.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rasa/core/processor.py b/rasa/core/processor.py index f1ba40691c42..78f4dc1cb8ec 100644 --- a/rasa/core/processor.py +++ b/rasa/core/processor.py @@ -176,7 +176,9 @@ async def _update_tracker_session( ) async def get_tracker_with_session_start( - self, sender_id: Text, output_channel: Optional[OutputChannel] = None, metadata: Optional[Dict] = None + self, sender_id: Text, + output_channel: Optional[OutputChannel] = None, + metadata: Optional[Dict] = None ) -> Optional[DialogueStateTracker]: """Get tracker for `sender_id` or create a new tracker for `sender_id`. From 376e90f29b15867d8e84d8f2de2b1bfae9ba0830 Mon Sep 17 00:00:00 2001 From: lluchini Date: Thu, 9 Apr 2020 17:18:04 -0300 Subject: [PATCH 04/14] Appended test to improved functionality passed black formatter pytype doesn't work on Win10. --- rasa/core/processor.py | 7 ++++--- tests/core/test_actions.py | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/rasa/core/processor.py b/rasa/core/processor.py index 78f4dc1cb8ec..300d1325ced6 100644 --- a/rasa/core/processor.py +++ b/rasa/core/processor.py @@ -36,7 +36,7 @@ ReminderScheduled, SlotSet, UserUttered, - SessionStarted + SessionStarted, ) from rasa.core.interpreter import ( INTENT_MESSAGE_PREFIX, @@ -176,9 +176,10 @@ async def _update_tracker_session( ) async def get_tracker_with_session_start( - self, sender_id: Text, + self, + sender_id: Text, output_channel: Optional[OutputChannel] = None, - metadata: Optional[Dict] = None + metadata: Optional[Dict] = None, ) -> Optional[DialogueStateTracker]: """Get tracker for `sender_id` or create a new tracker for `sender_id`. diff --git a/tests/core/test_actions.py b/tests/core/test_actions.py index af42403ad261..5a5dbb4737d9 100644 --- a/tests/core/test_actions.py +++ b/tests/core/test_actions.py @@ -510,6 +510,21 @@ async def test_action_session_start_without_slots( assert events == [SessionStarted(), ActionExecuted(ACTION_LISTEN_NAME)] +async def test_action_session_start_with_metada( + default_channel: CollectingOutputChannel, + template_nlg: TemplatedNaturalLanguageGenerator, + template_sender_tracker: DialogueStateTracker, + default_domain: Domain, +): + template_sender_tracker.events.append( + SessionStarted(metadata={"metadataTestKey": "metadataTestValue"}) + ) + + await test_action_session_start_without_slots( + default_channel, template_nlg, template_sender_tracker, default_domain + ) + + @pytest.mark.parametrize( "session_config, expected_events", [ From e4b3e6a504ae1bf70c0857fd54b07363d8f0fedf Mon Sep 17 00:00:00 2001 From: lluchini Date: Fri, 10 Apr 2020 09:04:22 -0300 Subject: [PATCH 05/14] Adding changelog. --- changelog/5574.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/5574.bugfix.rst diff --git a/changelog/5574.bugfix.rst b/changelog/5574.bugfix.rst new file mode 100644 index 000000000000..29aaf42a6aa7 --- /dev/null +++ b/changelog/5574.bugfix.rst @@ -0,0 +1 @@ +Fixed issue where metadata was not being sent with the tracker from session_start event when calling action server. \ No newline at end of file From c41c084fdd0a6947af0f2945a6dc51d279a3bb38 Mon Sep 17 00:00:00 2001 From: lluchini <61990017+lluchini@users.noreply.github.com> Date: Thu, 16 Apr 2020 16:36:10 -0300 Subject: [PATCH 06/14] Update tests/core/test_actions.py Co-Authored-By: Alexander Khizov --- tests/core/test_actions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/test_actions.py b/tests/core/test_actions.py index 5a5dbb4737d9..955e4da011a4 100644 --- a/tests/core/test_actions.py +++ b/tests/core/test_actions.py @@ -510,7 +510,7 @@ async def test_action_session_start_without_slots( assert events == [SessionStarted(), ActionExecuted(ACTION_LISTEN_NAME)] -async def test_action_session_start_with_metada( +async def test_action_session_start_with_metadata( default_channel: CollectingOutputChannel, template_nlg: TemplatedNaturalLanguageGenerator, template_sender_tracker: DialogueStateTracker, From 0bd3d3b4f638fd89106e71555a228cd3996320ea Mon Sep 17 00:00:00 2001 From: lluchini Date: Thu, 16 Apr 2020 16:41:14 -0300 Subject: [PATCH 07/14] Adjust suggested by degiz (https://github.com/degiz) https://github.com/RasaHQ/rasa/pull/5611 --- rasa/core/processor.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/rasa/core/processor.py b/rasa/core/processor.py index 300d1325ced6..ec57d5bf92cb 100644 --- a/rasa/core/processor.py +++ b/rasa/core/processor.py @@ -150,7 +150,10 @@ async def predict_next(self, sender_id: Text) -> Optional[Dict[Text, Any]]: } async def _update_tracker_session( - self, tracker: DialogueStateTracker, output_channel: OutputChannel + self, + tracker: DialogueStateTracker, + output_channel: OutputChannel, + metadata: Optional[Dict] = None, ) -> None: """Check the current session in `tracker` and update it if expired. @@ -159,6 +162,7 @@ async def _update_tracker_session( restart are considered). Args: + metadata: Data sent from client associated with the incoming user message. tracker: Tracker to inspect. output_channel: Output channel for potential utterances in a custom `ActionSessionStart`. @@ -168,6 +172,9 @@ async def _update_tracker_session( f"Starting a new session for conversation ID '{tracker.sender_id}'." ) + if metadata: + tracker.events.append(SessionStarted(metadata=metadata)) + await self._run_action( action=self._get_action(ACTION_SESSION_START_NAME), tracker=tracker, @@ -198,10 +205,7 @@ async def get_tracker_with_session_start( if not tracker: return None - if metadata: - tracker.events.append(SessionStarted(metadata=metadata)) - - await self._update_tracker_session(tracker, output_channel) + await self._update_tracker_session(tracker, output_channel, metadata) return tracker From f9bb4589296e4ffd4b7d5317998a876f4f7692b0 Mon Sep 17 00:00:00 2001 From: lluchini Date: Fri, 17 Apr 2020 08:50:47 -0300 Subject: [PATCH 08/14] Most high level (in terms of structure) code we've modified get_tracker_with_session_start(). So the unit tests that covers this change to call this method with metadata and assert on the results. --- tests/core/test_processor.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/core/test_processor.py b/tests/core/test_processor.py index d67a3f07be2e..db00f7c5769b 100644 --- a/tests/core/test_processor.py +++ b/tests/core/test_processor.py @@ -485,6 +485,34 @@ async def test_update_tracker_session( ] +# noinspection PyProtectedMember +async def test_update_tracker_session_with_metadata( + default_channel: CollectingOutputChannel, + default_processor: MessageProcessor, + monkeypatch: MonkeyPatch, +): + sender_id = uuid.uuid4().hex + tracker = default_processor.tracker_store.get_or_create_tracker(sender_id) + + # patch `_has_session_expired()` so the `_update_tracker_session()` call actually + # does something + monkeypatch.setattr(default_processor, "_has_session_expired", lambda _: True) + + metadata = {"metadataTestKey": "metadataTestValue"} + + await default_processor._update_tracker_session(tracker, default_channel, metadata) + + # the save is not called in _update_tracker_session() + default_processor._save_tracker(tracker) + + # inspect tracker events and make sure SessionStarted event is present and has metadata. + tracker = default_processor.tracker_store.retrieve(sender_id) + session_event_idx = tracker.events.index(SessionStarted()) + session_event_metadata = tracker.events[session_event_idx].metadata + + assert metadata == session_event_metadata + + # noinspection PyProtectedMember async def test_update_tracker_session_with_slots( default_channel: CollectingOutputChannel, From d633cf696970ebb9cd24b5a5c53cbc516f6ce9c4 Mon Sep 17 00:00:00 2001 From: lluchini Date: Fri, 17 Apr 2020 09:07:54 -0300 Subject: [PATCH 09/14] DeepSource identified a patern break, wasn't me but, I fixed anyway. --- rasa/core/processor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rasa/core/processor.py b/rasa/core/processor.py index ec57d5bf92cb..6af5bf7a7cdf 100644 --- a/rasa/core/processor.py +++ b/rasa/core/processor.py @@ -304,8 +304,8 @@ def predict_next_action( max_confidence_index, self.action_endpoint ) logger.debug( - f"Predicted next action '{action.name()}' with confidence " - f"{action_confidences[max_confidence_index]:.2f}." + f"Predicted next action '{action.name()}' with the confidence " + f"of {action_confidences[max_confidence_index]:.2f}." ) return action, policy, action_confidences[max_confidence_index] From 2a3e2debbfbefae94ac7aca148e7b733817847fc Mon Sep 17 00:00:00 2001 From: lluchini Date: Fri, 17 Apr 2020 09:24:00 -0300 Subject: [PATCH 10/14] DeepSource identified a patern break, wasn't me but, I fixed anyway. --- rasa/core/processor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rasa/core/processor.py b/rasa/core/processor.py index 6af5bf7a7cdf..4f1c74264e4e 100644 --- a/rasa/core/processor.py +++ b/rasa/core/processor.py @@ -304,7 +304,7 @@ def predict_next_action( max_confidence_index, self.action_endpoint ) logger.debug( - f"Predicted next action '{action.name()}' with the confidence " + f"Predicted next action \"{action.name()}\" with the confidence " f"of {action_confidences[max_confidence_index]:.2f}." ) return action, policy, action_confidences[max_confidence_index] From afc0263ff656f5fb00e078a390f77520640ff178 Mon Sep 17 00:00:00 2001 From: lluchini Date: Fri, 17 Apr 2020 09:38:39 -0300 Subject: [PATCH 11/14] The deep source is faling without reason, it's a false positive. --- rasa/core/processor.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rasa/core/processor.py b/rasa/core/processor.py index 4f1c74264e4e..a57845680bd3 100644 --- a/rasa/core/processor.py +++ b/rasa/core/processor.py @@ -303,10 +303,12 @@ def predict_next_action( action = self.domain.action_for_index( max_confidence_index, self.action_endpoint ) + logger.debug( - f"Predicted next action \"{action.name()}\" with the confidence " - f"of {action_confidences[max_confidence_index]:.2f}." + f"Predicted next action '{action.name()}' with confidence " + f"{action_confidences[max_confidence_index]:.2f}." ) + return action, policy, action_confidences[max_confidence_index] @staticmethod From 26b1cadad69db90c3dda31903064596106bc6786 Mon Sep 17 00:00:00 2001 From: lluchini <61990017+lluchini@users.noreply.github.com> Date: Fri, 17 Apr 2020 13:33:39 -0300 Subject: [PATCH 12/14] Update changelog/5574.bugfix.rst Co-Authored-By: Alexander Khizov --- changelog/5574.bugfix.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/changelog/5574.bugfix.rst b/changelog/5574.bugfix.rst index 29aaf42a6aa7..b5e4d5ae3abd 100644 --- a/changelog/5574.bugfix.rst +++ b/changelog/5574.bugfix.rst @@ -1 +1,3 @@ -Fixed issue where metadata was not being sent with the tracker from session_start event when calling action server. \ No newline at end of file +Fixed an issue that happened when metadata is passed in a new session. + +Now the metadata is correctly passed to the ActionSessionStart. From 4f9f05b6803287942e9350bd61d119128bc8cf43 Mon Sep 17 00:00:00 2001 From: lluchini <61990017+lluchini@users.noreply.github.com> Date: Fri, 17 Apr 2020 13:33:49 -0300 Subject: [PATCH 13/14] Update tests/core/test_processor.py Co-Authored-By: Alexander Khizov --- tests/core/test_processor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/test_processor.py b/tests/core/test_processor.py index db00f7c5769b..be736c667c06 100644 --- a/tests/core/test_processor.py +++ b/tests/core/test_processor.py @@ -510,7 +510,7 @@ async def test_update_tracker_session_with_metadata( session_event_idx = tracker.events.index(SessionStarted()) session_event_metadata = tracker.events[session_event_idx].metadata - assert metadata == session_event_metadata + assert session_event_metadata == metadata # noinspection PyProtectedMember From 2dadeb6ccd2d72b41409b034248505fc16bcf9db Mon Sep 17 00:00:00 2001 From: lluchini Date: Fri, 17 Apr 2020 13:35:24 -0300 Subject: [PATCH 14/14] Removed by sugestion of degiz (https://github.com/degiz) https://github.com/RasaHQ/rasa/pull/5611/files/afc0263ff656f5fb00e078a390f77520640ff178 --- tests/core/test_actions.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/tests/core/test_actions.py b/tests/core/test_actions.py index 955e4da011a4..af42403ad261 100644 --- a/tests/core/test_actions.py +++ b/tests/core/test_actions.py @@ -510,21 +510,6 @@ async def test_action_session_start_without_slots( assert events == [SessionStarted(), ActionExecuted(ACTION_LISTEN_NAME)] -async def test_action_session_start_with_metadata( - default_channel: CollectingOutputChannel, - template_nlg: TemplatedNaturalLanguageGenerator, - template_sender_tracker: DialogueStateTracker, - default_domain: Domain, -): - template_sender_tracker.events.append( - SessionStarted(metadata={"metadataTestKey": "metadataTestValue"}) - ) - - await test_action_session_start_without_slots( - default_channel, template_nlg, template_sender_tracker, default_domain - ) - - @pytest.mark.parametrize( "session_config, expected_events", [