diff --git a/.github/workflows/ci-model-regression.yml b/.github/workflows/ci-model-regression.yml index ca874cf62625..3ef275f175c1 100644 --- a/.github/workflows/ci-model-regression.yml +++ b/.github/workflows/ci-model-regression.yml @@ -643,7 +643,7 @@ jobs: # Get ID of the schedule workflow SCHEDULE_ID=$(curl -X GET -s -H 'Authorization: token ${{ secrets.GITHUB_TOKEN }}' -H "Accept: application/vnd.github.v3+json" \ "https://api.github.com/repos/${{ github.repository }}/actions/workflows" \ - | jq -r '.workflows[] | select(.name == "${{ github.workflow }}") | select(.path | test("schedule")) | .id') + | jq -r '.workflows[] | select(.name == "CI - Model Regression on schedule") | select(.path | test("schedule")) | .id') ARTIFACT_URL=$(curl -s -H 'Authorization: token ${{ secrets.GITHUB_TOKEN }}' -H "Accept: application/vnd.github.v3+json" \ "https://api.github.com/repos/${{ github.repository }}/actions/workflows/${SCHEDULE_ID}/runs?event=schedule&status=completed&branch=main&per_page=1" | jq -r .workflow_runs[0].artifacts_url) diff --git a/changelog/10606.bugfix.md b/changelog/10606.bugfix.md new file mode 100644 index 000000000000..4f0b55ee6df9 --- /dev/null +++ b/changelog/10606.bugfix.md @@ -0,0 +1,2 @@ +Fix `max_history` truncation in `AugmentedMemoizationPolicy` to preserve the most recent `UserUttered` event. +Previously, `AugmentedMemoizationPolicy` failed to predict next action after long sequences of actions (longer than `max_history`) because the policy did not have access to the most recent user message. diff --git a/rasa/core/policies/memoization.py b/rasa/core/policies/memoization.py index 977bc254ddac..1836f6bcff6c 100644 --- a/rasa/core/policies/memoization.py +++ b/rasa/core/policies/memoization.py @@ -22,6 +22,7 @@ from rasa.shared.core.generator import TrackerWithCachedStates from rasa.shared.utils.io import is_logging_disabled from rasa.core.constants import MEMOIZATION_POLICY_PRIORITY +from rasa.shared.core.constants import ACTION_LISTEN_NAME logger = logging.getLogger(__name__) @@ -286,12 +287,20 @@ class AugmentedMemoizationPolicy(MemoizationPolicy): """ @staticmethod - def _back_to_the_future( + def _strip_leading_events_until_action_executed( tracker: DialogueStateTracker, again: bool = False ) -> Optional[DialogueStateTracker]: - """Send Marty to the past to get - the new featurization for the future""" + """Truncates the tracker to begin at the next `ActionExecuted` event. + Args: + tracker: The tracker to truncate. + again: When true, truncate tracker at the second action. + Otherwise truncate to the first action. + + Returns: + The truncated tracker if there were actions present. + If none are found, returns `None`. + """ idx_of_first_action = None idx_of_second_action = None @@ -299,7 +308,6 @@ def _back_to_the_future( # we need to find second executed action for e_i, event in enumerate(applied_events): - # find second ActionExecuted if isinstance(event, ActionExecuted): if idx_of_first_action is None: idx_of_first_action = e_i @@ -317,19 +325,19 @@ def _back_to_the_future( if not events: return None - mcfly_tracker = tracker.init_copy() + truncated_tracker = tracker.init_copy() for e in events: - mcfly_tracker.update(e) + truncated_tracker.update(e) - return mcfly_tracker + return truncated_tracker - def _recall_using_delorean( + def _recall_using_truncation( self, old_states: List[State], tracker: DialogueStateTracker, domain: Domain, ) -> Optional[Text]: - """Applies to the future idea to change the past and get the new future. + """Attempts to match memorized states to progressively shorter trackers. - Recursively go to the past to correctly forget slots, - and then back to the future to recall. + This matching will iteratively remove prior slot setting events and + other actions, looking for the first matching memorized state sequence. Args: old_states: List of states. @@ -342,10 +350,12 @@ def _recall_using_delorean( logger.debug("Launch DeLorean...") # Truncate the tracker based on `max_history` - mcfly_tracker = _trim_tracker_by_max_history(tracker, self.max_history) - mcfly_tracker = self._back_to_the_future(mcfly_tracker) - while mcfly_tracker is not None: - states = self._prediction_states(mcfly_tracker, domain,) + truncated_tracker = _trim_tracker_by_max_history(tracker, self.max_history) + truncated_tracker = self._strip_leading_events_until_action_executed( + truncated_tracker + ) + while truncated_tracker is not None: + states = self._prediction_states(truncated_tracker, domain) if old_states != states: # check if we like new futures @@ -356,7 +366,9 @@ def _recall_using_delorean( old_states = states # go back again - mcfly_tracker = self._back_to_the_future(mcfly_tracker, again=True) + truncated_tracker = self._strip_leading_events_until_action_executed( + truncated_tracker, again=True + ) # No match found logger.debug(f"Current tracker state {old_states}") @@ -381,7 +393,7 @@ def recall( predicted_action_name = self._recall_states(states) if predicted_action_name is None: # let's try a different method to recall that tracker - return self._recall_using_delorean(states, tracker, domain,) + return self._recall_using_truncation(states, tracker, domain) else: return predicted_action_name @@ -391,12 +403,16 @@ def _get_max_applied_events_for_max_history( ) -> Optional[int]: """Computes the number of events in the tracker that correspond to max_history. + To ensure that the last user utterance is correctly included in the prediction + states, return the index of the most recent `action_listen` event occuring + before the tracker would be truncated according to the value of `max_history`. + Args: tracker: Some tracker holding the events max_history: The number of actions to count Returns: - The number of actions, as counted from the end of the event list, that should + The number of events, as counted from the end of the event list, that should be taken into accout according to the `max_history` setting. If all events should be taken into account, the return value is `None`. """ @@ -408,8 +424,8 @@ def _get_max_applied_events_for_max_history( num_events += 1 if isinstance(event, ActionExecuted): num_actions += 1 - if num_actions > max_history: - return num_events + if num_actions > max_history and event.action_name == ACTION_LISTEN_NAME: + return num_events return None diff --git a/tests/core/policies/test_memoization.py b/tests/core/policies/test_memoization.py index b2578d1acc55..5a34a3aa2b44 100644 --- a/tests/core/policies/test_memoization.py +++ b/tests/core/policies/test_memoization.py @@ -13,6 +13,7 @@ UserUttered, SlotSet, ) +from rasa.shared.core.constants import ACTION_LISTEN_NAME from rasa.shared.nlu.interpreter import RegexInterpreter @@ -50,6 +51,7 @@ def test_prediction(self, max_history): """ ) events = [ + ActionExecuted(ACTION_LISTEN_NAME), UserUttered(intent={"name": GREET_INTENT_NAME}), ActionExecuted(UTTER_GREET_ACTION), SlotSet("slot_1", True), @@ -58,16 +60,18 @@ def test_prediction(self, max_history): SlotSet("slot_3", True), ActionExecuted(UTTER_GREET_ACTION), ActionExecuted(UTTER_GREET_ACTION), + ActionExecuted(ACTION_LISTEN_NAME), UserUttered(intent={"name": GREET_INTENT_NAME}), ActionExecuted(UTTER_GREET_ACTION), SlotSet("slot_4", True), ActionExecuted(UTTER_BYE_ACTION), + ActionExecuted(ACTION_LISTEN_NAME), ] training_story = TrackerWithCachedStates.from_events( "training story", evts=events, domain=domain, slots=domain.slots, ) test_story = TrackerWithCachedStates.from_events( - "training story", events[:-1], domain=domain, slots=domain.slots, + "training story", events[:-2], domain=domain, slots=domain.slots, ) interpreter = RegexInterpreter() policy.train([training_story], domain, interpreter) @@ -117,11 +121,12 @@ def test_augmented_prediction(self, max_history): training_story = TrackerWithCachedStates.from_events( "training story", [ - ActionExecuted(UTTER_GREET_ACTION), + ActionExecuted(ACTION_LISTEN_NAME), UserUttered(intent={"name": GREET_INTENT_NAME}), ActionExecuted(UTTER_GREET_ACTION), SlotSet("slot_3", True), ActionExecuted(UTTER_BYE_ACTION), + ActionExecuted(ACTION_LISTEN_NAME), ], domain=domain, slots=domain.slots, @@ -129,15 +134,18 @@ def test_augmented_prediction(self, max_history): test_story = TrackerWithCachedStates.from_events( "test story", [ + ActionExecuted(ACTION_LISTEN_NAME), UserUttered(intent={"name": GREET_INTENT_NAME}), ActionExecuted(UTTER_GREET_ACTION), SlotSet("slot_1", False), ActionExecuted(UTTER_GREET_ACTION), ActionExecuted(UTTER_GREET_ACTION), + ActionExecuted(ACTION_LISTEN_NAME), UserUttered(intent={"name": GREET_INTENT_NAME}), ActionExecuted(UTTER_GREET_ACTION), SlotSet("slot_2", True), ActionExecuted(UTTER_GREET_ACTION), + ActionExecuted(ACTION_LISTEN_NAME), UserUttered(intent={"name": GREET_INTENT_NAME}), ActionExecuted(UTTER_GREET_ACTION), SlotSet("slot_3", True), @@ -157,3 +165,253 @@ def test_augmented_prediction(self, max_history): ] == UTTER_BYE_ACTION ) + + @pytest.mark.parametrize("max_history", [1, 2, 3, 4, None]) + def test_augmented_prediction_across_max_history_actions(self, max_history): + """Tests that the last user utterance is preserved in action states + even when the utterance occurs prior to `max_history` actions in the + past. + """ + policy = self.create_policy( + featurizer=MaxHistoryTrackerFeaturizer(max_history=max_history), priority=1 + ) + + GREET_INTENT_NAME = "greet" + UTTER_GREET_ACTION = "utter_greet" + UTTER_ACTION_1 = "utter_1" + UTTER_ACTION_2 = "utter_2" + UTTER_ACTION_3 = "utter_3" + UTTER_ACTION_4 = "utter_4" + UTTER_ACTION_5 = "utter_5" + UTTER_BYE_ACTION = "utter_goodbye" + domain = Domain.from_yaml( + f""" + intents: + - {GREET_INTENT_NAME} + actions: + - {UTTER_GREET_ACTION} + - {UTTER_ACTION_1} + - {UTTER_ACTION_2} + - {UTTER_ACTION_3} + - {UTTER_ACTION_4} + - {UTTER_ACTION_5} + - {UTTER_BYE_ACTION} + """ + ) + training_story = TrackerWithCachedStates.from_events( + "training story", + [ + ActionExecuted(ACTION_LISTEN_NAME), + UserUttered(intent={"name": GREET_INTENT_NAME}), + ActionExecuted(UTTER_ACTION_1), + ActionExecuted(UTTER_ACTION_2), + ActionExecuted(UTTER_ACTION_3), + ActionExecuted(UTTER_ACTION_4), + ActionExecuted(UTTER_ACTION_5), + ActionExecuted(UTTER_BYE_ACTION), + ActionExecuted(ACTION_LISTEN_NAME), + ], + domain=domain, + slots=domain.slots, + ) + test_story = TrackerWithCachedStates.from_events( + "test story", + [ + ActionExecuted(ACTION_LISTEN_NAME), + UserUttered(intent={"name": GREET_INTENT_NAME}), + ActionExecuted(UTTER_ACTION_1), + ActionExecuted(UTTER_ACTION_2), + ActionExecuted(UTTER_ACTION_3), + ActionExecuted(UTTER_ACTION_4), + ActionExecuted(UTTER_ACTION_5), + # ActionExecuted(UTTER_BYE_ACTION), + ], + domain=domain, + slots=domain.slots, + ) + interpreter = RegexInterpreter() + policy.train([training_story], domain, interpreter) + prediction = policy.predict_action_probabilities( + test_story, domain, interpreter + ) + assert ( + domain.action_names_or_texts[ + prediction.probabilities.index(max(prediction.probabilities)) + ] + == UTTER_BYE_ACTION + ) + + @pytest.mark.parametrize("max_history", [1, 2, 3, 4, None]) + def test_aug_pred_sensitive_to_intent_across_max_history_actions(self, max_history): + """Tests that only the most recent user utterance propagates to state + creation of following actions. + """ + policy = self.create_policy( + featurizer=MaxHistoryTrackerFeaturizer(max_history=max_history), priority=1 + ) + + GREET_INTENT_NAME = "greet" + GOODBYE_INTENT_NAME = "goodbye" + UTTER_GREET_ACTION = "utter_greet" + UTTER_ACTION_1 = "utter_1" + UTTER_ACTION_2 = "utter_2" + UTTER_ACTION_3 = "utter_3" + UTTER_ACTION_4 = "utter_4" + UTTER_ACTION_5 = "utter_5" + UTTER_BYE_ACTION = "utter_goodbye" + domain = Domain.from_yaml( + f""" + intents: + - {GREET_INTENT_NAME} + - {GOODBYE_INTENT_NAME} + actions: + - {UTTER_GREET_ACTION} + - {UTTER_ACTION_1} + - {UTTER_ACTION_2} + - {UTTER_ACTION_3} + - {UTTER_ACTION_4} + - {UTTER_ACTION_5} + - {UTTER_BYE_ACTION} + """ + ) + training_story = TrackerWithCachedStates.from_events( + "training story", + [ + ActionExecuted(ACTION_LISTEN_NAME), + UserUttered(intent={"name": GREET_INTENT_NAME}), + ActionExecuted(UTTER_ACTION_1), + ActionExecuted(UTTER_ACTION_2), + ActionExecuted(UTTER_ACTION_3), + ActionExecuted(UTTER_ACTION_4), + ActionExecuted(UTTER_ACTION_5), + ActionExecuted(UTTER_BYE_ACTION), + ActionExecuted(ACTION_LISTEN_NAME), + ], + domain=domain, + slots=domain.slots, + ) + interpreter = RegexInterpreter() + policy.train([training_story], domain, interpreter) + + test_story1 = TrackerWithCachedStates.from_events( + "test story", + [ + ActionExecuted(ACTION_LISTEN_NAME), + UserUttered(intent={"name": GOODBYE_INTENT_NAME}), + ActionExecuted(UTTER_BYE_ACTION), + ActionExecuted(ACTION_LISTEN_NAME), + UserUttered(intent={"name": GREET_INTENT_NAME}), + ActionExecuted(UTTER_ACTION_1), + ActionExecuted(UTTER_ACTION_2), + ActionExecuted(UTTER_ACTION_3), + ActionExecuted(UTTER_ACTION_4), + ActionExecuted(UTTER_ACTION_5), + # ActionExecuted(UTTER_BYE_ACTION), + ], + domain=domain, + slots=domain.slots, + ) + prediction1 = policy.predict_action_probabilities( + test_story1, domain, interpreter + ) + assert ( + domain.action_names_or_texts[ + prediction1.probabilities.index(max(prediction1.probabilities)) + ] + == UTTER_BYE_ACTION + ) + + test_story2_no_match_expected = TrackerWithCachedStates.from_events( + "test story", + [ + ActionExecuted(ACTION_LISTEN_NAME), + UserUttered(intent={"name": GREET_INTENT_NAME}), + ActionExecuted(UTTER_BYE_ACTION), + ActionExecuted(ACTION_LISTEN_NAME), + UserUttered(intent={"name": GOODBYE_INTENT_NAME}), + ActionExecuted(UTTER_ACTION_1), + ActionExecuted(UTTER_ACTION_2), + ActionExecuted(UTTER_ACTION_3), + ActionExecuted(UTTER_ACTION_4), + ActionExecuted(UTTER_ACTION_5), + # ActionExecuted(ACTION_LISTEN_NAME), + ], + domain=domain, + slots=domain.slots, + ) + + prediction2 = policy.predict_action_probabilities( + test_story2_no_match_expected, domain, interpreter + ) + assert ( + domain.action_names_or_texts[ + prediction2.probabilities.index(max(prediction2.probabilities)) + ] + == ACTION_LISTEN_NAME + ) + + @pytest.mark.parametrize("max_history", [1, 2, 3, 4, None]) + def test_aug_pred_without_intent(self, max_history): + """Tests memoization works for a memoized state sequence that does + not have a user utterance. + """ + policy = self.create_policy( + featurizer=MaxHistoryTrackerFeaturizer(max_history=max_history), priority=1 + ) + + GREET_INTENT_NAME = "greet" + GOODBYE_INTENT_NAME = "goodbye" + UTTER_GREET_ACTION = "utter_greet" + UTTER_ACTION_1 = "utter_1" + UTTER_ACTION_2 = "utter_2" + UTTER_ACTION_3 = "utter_3" + UTTER_ACTION_4 = "utter_4" + domain = Domain.from_yaml( + f""" + intents: + - {GREET_INTENT_NAME} + - {GOODBYE_INTENT_NAME} + actions: + - {UTTER_GREET_ACTION} + - {UTTER_ACTION_1} + - {UTTER_ACTION_2} + - {UTTER_ACTION_3} + - {UTTER_ACTION_4} + """ + ) + training_story = TrackerWithCachedStates.from_events( + "training story", + [ + ActionExecuted(UTTER_ACTION_3), + ActionExecuted(UTTER_ACTION_4), + ActionExecuted(ACTION_LISTEN_NAME), + ], + domain=domain, + slots=domain.slots, + ) + + interpreter = RegexInterpreter() + policy.train([training_story], domain, interpreter) + + test_story = TrackerWithCachedStates.from_events( + "test story", + [ + ActionExecuted(ACTION_LISTEN_NAME), + UserUttered(intent={"name": GREET_INTENT_NAME}), + ActionExecuted(UTTER_ACTION_1), + ActionExecuted(UTTER_ACTION_2), + ActionExecuted(UTTER_ACTION_3), + # ActionExecuted(UTTER_ACTION_4), + ], + domain=domain, + slots=domain.slots, + ) + prediction = policy.predict_action_probabilities( + test_story, domain, interpreter + ) + assert ( + domain.action_names_or_texts[ + prediction.probabilities.index(max(prediction.probabilities)) + ] + == UTTER_ACTION_4 + )