Repository: RasaHQ/rasa
Pull request: Rules states should not include the initial values of slots.
**Proposed changes**:
Fixes #7450 

When calculating tracker states from rules we were including the initial value of slots.
This meant that the rule only matched when a slot had that initial value.
This change omits initial values of slots from the rule states.

**Status (please check what you already did)**:
- [x] added some tests for the functionality
- [ ] updated the documentation
- [x] updated the changelog (please check [changelog](https://github.com/RasaHQ/rasa/tree/main/changelog) for instructions)
- [x] reformat files using `black` (please check [Readme](https://github.com/RasaHQ/rasa#code-style) for instructions)


Problem statement:
Rule applicability depends on slot values when initial_value is used
**Rasa version**: 2.1.2

**Rasa SDK version** (if used & relevant):

**Rasa X version** (if used & relevant):

**Python version**: 3.8.5

**Operating system** (windows, osx, ...): Mac

**Issue**:
When using `initial_value` for some slot `s` whose value influences the conversation, rules that aren't written as depending on a specific value of `s` are still constructed such that they are conditioned on the exact value used for `initial_value`. Then, these rules can't be applied in situations where `s` has a different value.

Example:
```yml
intents:
- i1

slots:
  s_cat1:
    type: categorical
    values:
      - v1
      - v2
    initial_value: v1

actions:
- action_1

rules:
- rule: rule not conditioned on slot value
  steps:
  - intent: i1
  - action: action_1
  - slot_was_set:
    - s_cat1: v2

policies:
  - name: RulePolicy 
```

where `action_1` simply sets the slot to a different value:
```python
class Action1(Action):
    def name(self) -> Text:
        return "action_1"

    async def run(self, dispatcher, tracker, domain):
        return [SlotSet("s_cat1", "v2")]
```

after training and testing like this:
```bash
rasa train core
rasa test core -s tests/test_story.yml
```

leads to failed predictions in a test story because the rule couldn't be applied where the slot had a value different from the initial one:
```yml
  steps:
  - intent: i1
  - action: action_1
  - slot_was_set:
    - s_cat1: v2
  - intent: i1
  - action: action_1  # predicted: action_default_fallback
  - action: action_listen  # predicted: action_default_fallback
```

Hints from issue discussion:

Commit message: Merge branch 'main' into 7450-rule_applicability_depends_on_slots_initial_value

Patch to review:
diff --git a/changelog/7450.bugfix.md b/changelog/7450.bugfix.md
new file mode 100644
index 000000000000..c09ead50a69c
--- /dev/null
+++ b/changelog/7450.bugfix.md
@@ -0,0 +1,2 @@
+Rule tracker states no longer include the initial value of slots.
+Rules now only require slot values when explicitly stated in the rule.
diff --git a/rasa/core/featurizers/tracker_featurizers.py b/rasa/core/featurizers/tracker_featurizers.py
index 033747c7927b..fee19c97283d 100644
--- a/rasa/core/featurizers/tracker_featurizers.py
+++ b/rasa/core/featurizers/tracker_featurizers.py
@@ -51,17 +51,20 @@ def __init__(
         self.state_featurizer = state_featurizer
 
     @staticmethod
-    def _create_states(tracker: DialogueStateTracker, domain: Domain) -> List[State]:
+    def _create_states(
+        tracker: DialogueStateTracker, domain: Domain, omit_unset_slots: bool = False,
+    ) -> List[State]:
         """Create states for the given tracker.
 
         Args:
             tracker: a :class:`rasa.core.trackers.DialogueStateTracker`
             domain: a :class:`rasa.shared.core.domain.Domain`
+            omit_unset_slots: If `True` do not include the initial values of slots.
 
         Returns:
             a list of states
         """
-        return tracker.past_states(domain)
+        return tracker.past_states(domain, omit_unset_slots=omit_unset_slots)
 
     def _featurize_states(
         self,
@@ -127,13 +130,17 @@ def _remove_user_text_if_intent(trackers_as_states: List[List[State]]) -> None:
                     del state[USER][TEXT]
 
     def training_states_actions_and_entities(
-        self, trackers: List[DialogueStateTracker], domain: Domain
+        self,
+        trackers: List[DialogueStateTracker],
+        domain: Domain,
+        omit_unset_slots: bool = False,
     ) -> Tuple[List[List[State]], List[List[Text]], List[List[Dict[Text, Any]]]]:
         """Transforms list of trackers to lists of states, actions and entity data.
 
         Args:
             trackers: The trackers to transform
             domain: The domain
+            omit_unset_slots: If `True` do not include the initial values of slots.
 
         Returns:
             A tuple of list of states, list of actions and list of entity data.
@@ -143,13 +150,17 @@ def training_states_actions_and_entities(
         )
 
     def training_states_and_actions(
-        self, trackers: List[DialogueStateTracker], domain: Domain
+        self,
+        trackers: List[DialogueStateTracker],
+        domain: Domain,
+        omit_unset_slots: bool = False,
     ) -> Tuple[List[List[State]], List[List[Text]]]:
         """Transforms list of trackers to lists of states and actions.
 
         Args:
             trackers: The trackers to transform
             domain: The domain
+            omit_unset_slots: If `True` do not include the initial values of slots.
 
         Returns:
             A tuple of list of states and list of actions.
@@ -158,7 +169,9 @@ def training_states_and_actions(
             trackers_as_states,
             trackers_as_actions,
             _,
-        ) = self.training_states_actions_and_entities(trackers, domain)
+        ) = self.training_states_actions_and_entities(
+            trackers, domain, omit_unset_slots=omit_unset_slots
+        )
         return trackers_as_states, trackers_as_actions
 
     def featurize_trackers(
@@ -332,13 +345,17 @@ class FullDialogueTrackerFeaturizer(TrackerFeaturizer):
     """
 
     def training_states_actions_and_entities(
-        self, trackers: List[DialogueStateTracker], domain: Domain
+        self,
+        trackers: List[DialogueStateTracker],
+        domain: Domain,
+        omit_unset_slots: bool = False,
     ) -> Tuple[List[List[State]], List[List[Text]], List[List[Dict[Text, Any]]]]:
         """Transforms list of trackers to lists of states, actions and entity data.
 
         Args:
             trackers: The trackers to transform
             domain: The domain
+            omit_unset_slots: If `True` do not include the initial values of slots.
 
         Returns:
             A tuple of list of states, list of actions and list of entity data.
@@ -358,7 +375,9 @@ def training_states_actions_and_entities(
             disable=rasa.shared.utils.io.is_logging_disabled(),
         )
         for tracker in pbar:
-            states = self._create_states(tracker, domain)
+            states = self._create_states(
+                tracker, domain, omit_unset_slots=omit_unset_slots
+            )
 
             delete_first_state = False
             actions = []
@@ -476,13 +495,17 @@ def _hash_example(
         return hash((frozen_states, frozen_actions))
 
     def training_states_actions_and_entities(
-        self, trackers: List[DialogueStateTracker], domain: Domain
+        self,
+        trackers: List[DialogueStateTracker],
+        domain: Domain,
+        omit_unset_slots: bool = False,
     ) -> Tuple[List[List[State]], List[List[Text]], List[List[Dict[Text, Any]]]]:
         """Transforms list of trackers to lists of states, actions and entity data.
 
         Args:
             trackers: The trackers to transform
             domain: The domain
+            omit_unset_slots: If `True` do not include the initial values of slots.
 
         Returns:
             A tuple of list of states, list of actions and list of entity data.
@@ -506,7 +529,9 @@ def training_states_actions_and_entities(
             disable=rasa.shared.utils.io.is_logging_disabled(),
         )
         for tracker in pbar:
-            states = self._create_states(tracker, domain)
+            states = self._create_states(
+                tracker, domain, omit_unset_slots=omit_unset_slots
+            )
 
             states_length_for_action = 0
             entity_data = {}
diff --git a/rasa/core/policies/rule_policy.py b/rasa/core/policies/rule_policy.py
index 9a85ed9b4311..50082f3a66e4 100644
--- a/rasa/core/policies/rule_policy.py
+++ b/rasa/core/policies/rule_policy.py
@@ -597,7 +597,9 @@ def train(
         (
             rule_trackers_as_states,
             rule_trackers_as_actions,
-        ) = self.featurizer.training_states_and_actions(rule_trackers, domain)
+        ) = self.featurizer.training_states_and_actions(
+            rule_trackers, domain, omit_unset_slots=True
+        )
 
         rules_lookup = self._create_lookup_from_states(
             rule_trackers_as_states, rule_trackers_as_actions
diff --git a/rasa/shared/core/domain.py b/rasa/shared/core/domain.py
index 1d9eec351ce3..b995cb7c7f2f 100644
--- a/rasa/shared/core/domain.py
+++ b/rasa/shared/core/domain.py
@@ -1002,17 +1002,22 @@ def _get_user_sub_state(
 
     @staticmethod
     def _get_slots_sub_state(
-        tracker: "DialogueStateTracker",
+        tracker: "DialogueStateTracker", omit_unset_slots: bool = False,
     ) -> Dict[Text, Union[Text, Tuple[float]]]:
-        """Set all set slots with the featurization of the stored value
+        """Set all set slots with the featurization of the stored value.
+
         Args:
             tracker: dialog state tracker containing the dialog so far
+            omit_unset_slots: If `True` do not include the initial values of slots.
+
         Returns:
             a dictionary mapping slot names to their featurization
         """
         slots = {}
         for slot_name, slot in tracker.slots.items():
             if slot is not None and slot.as_feature():
+                if omit_unset_slots and not slot.has_been_set:
+                    continue
                 if slot.value == rasa.shared.core.constants.SHOULD_NOT_BE_SET:
                     slots[slot_name] = rasa.shared.core.constants.SHOULD_NOT_BE_SET
                 elif any(slot.as_feature()):
@@ -1060,11 +1065,22 @@ def _clean_state(state: State) -> State:
             if sub_state
         }
 
-    def get_active_states(self, tracker: "DialogueStateTracker") -> State:
-        """Return a bag of active states from the tracker state."""
+    def get_active_states(
+        self, tracker: "DialogueStateTracker", omit_unset_slots: bool = False,
+    ) -> State:
+        """Return a bag of active states from the tracker state.
+
+        Args:
+            tracker: dialog state tracker containing the dialog so far
+            omit_unset_slots: If `True` do not include the initial values of slots.
+
+        Returns `State` containing all active states.
+        """
         state = {
             rasa.shared.core.constants.USER: self._get_user_sub_state(tracker),
-            rasa.shared.core.constants.SLOTS: self._get_slots_sub_state(tracker),
+            rasa.shared.core.constants.SLOTS: self._get_slots_sub_state(
+                tracker, omit_unset_slots=omit_unset_slots
+            ),
             rasa.shared.core.constants.PREVIOUS_ACTION: self._get_prev_action_sub_state(
                 tracker
             ),
@@ -1075,14 +1091,23 @@ def get_active_states(self, tracker: "DialogueStateTracker") -> State:
         return self._clean_state(state)
 
     def states_for_tracker_history(
-        self, tracker: "DialogueStateTracker"
+        self, tracker: "DialogueStateTracker", omit_unset_slots: bool = False,
     ) -> List[State]:
-        """Array of states for each state of the trackers history."""
+        """Array of states for each state of the trackers history.
+
+        Args:
+            tracker: dialog state tracker containing the dialog so far
+            omit_unset_slots: If `True` do not include the initial values of slots.
+
+        Returns A `State` for each prior tracker.
+        """
         return [
-            self.get_active_states(tr) for tr in tracker.generate_all_prior_trackers()
+            self.get_active_states(tr, omit_unset_slots=omit_unset_slots)
+            for tr in tracker.generate_all_prior_trackers()
         ]
 
     def slots_for_entities(self, entities: List[Dict[Text, Any]]) -> List[SlotSet]:
+        """Return a list of of SlotSet events from a list of entities."""
         if self.store_entities_as_slots:
             slot_events = []
             for s in self.slots:
diff --git a/rasa/shared/core/generator.py b/rasa/shared/core/generator.py
index 994ee52fedaf..093e61d11d2a 100644
--- a/rasa/shared/core/generator.py
+++ b/rasa/shared/core/generator.py
@@ -84,7 +84,18 @@ def from_events(
             tracker.update(e)
         return tracker
 
-    def past_states_for_hashing(self, domain: Domain) -> Deque[FrozenState]:
+    def past_states_for_hashing(
+        self, domain: Domain, omit_unset_slots: bool = False,
+    ) -> Deque[FrozenState]:
+        """Generate and cache the past states of this tracker based on the history.
+
+        Args:
+            domain: a :class:`rasa.shared.core.domain.Domain`
+            omit_unset_slots: If `True` do not include the initial values of slots.
+
+        Returns:
+            A list of states
+        """
         # we need to make sure this is the same domain, otherwise things will
         # go south. but really, the same tracker shouldn't be used across
         # domains
@@ -93,7 +104,7 @@ def past_states_for_hashing(self, domain: Domain) -> Deque[FrozenState]:
         # if don't have it cached, we use the domain to calculate the states
         # from the events
         if self._states_for_hashing is None:
-            states = super().past_states(domain)
+            states = super().past_states(domain, omit_unset_slots=omit_unset_slots)
             self._states_for_hashing = deque(
                 self.freeze_current_state(s) for s in states
             )
@@ -107,8 +118,21 @@ def _unfreeze_states(frozen_states: Deque[FrozenState]) -> List[State]:
             for frozen_state in frozen_states
         ]
 
-    def past_states(self, domain: Domain) -> List[State]:
-        states_for_hashing = self.past_states_for_hashing(domain)
+    def past_states(
+        self, domain: Domain, omit_unset_slots: bool = False,
+    ) -> List[State]:
+        """Generate the past states of this tracker based on the history.
+
+        Args:
+            domain: a :class:`rasa.shared.core.domain.Domain`
+            omit_unset_slots: If `True` do not include the initial values of slots.
+
+        Returns:
+            A list of states
+        """
+        states_for_hashing = self.past_states_for_hashing(
+            domain, omit_unset_slots=omit_unset_slots
+        )
         return self._unfreeze_states(states_for_hashing)
 
     def clear_states(self) -> None:
diff --git a/rasa/shared/core/slots.py b/rasa/shared/core/slots.py
index a1b5b2a0a277..37e95cc06f39 100644
--- a/rasa/shared/core/slots.py
+++ b/rasa/shared/core/slots.py
@@ -45,11 +45,12 @@ def __init__(
                 influence the predictions of the dialogue polices.
         """
         self.name = name
-        self.value = initial_value
+        self._value = initial_value
         self.initial_value = initial_value
         self._value_reset_delay = value_reset_delay
         self.auto_fill = auto_fill
         self.influence_conversation = influence_conversation
+        self._has_been_set = False
 
     def feature_dimensionality(self) -> int:
         """How many features this single slot creates.
@@ -98,7 +99,25 @@ def _as_feature(self) -> List[float]:
         )
 
     def reset(self) -> None:
+        """Resets the slot's value to the initial value."""
         self.value = self.initial_value
+        self._has_been_set = False
+
+    @property
+    def value(self) -> Any:
+        """Get the slot's value."""
+        return self._value
+
+    @value.setter
+    def value(self, value: Any) -> None:
+        """Set the slot's value."""
+        self._value = value
+        self._has_been_set = True
+
+    @property
+    def has_been_set(self) -> bool:
+        """Indicates if the slot's value has been set."""
+        return self._has_been_set
 
     def __str__(self) -> Text:
         return f"{self.__class__.__name__}({self.name}: {self.value})"
diff --git a/rasa/shared/core/trackers.py b/rasa/shared/core/trackers.py
index 0e06d4ce0dee..ec7b7af806a9 100644
--- a/rasa/shared/core/trackers.py
+++ b/rasa/shared/core/trackers.py
@@ -273,16 +273,21 @@ def freeze_current_state(state: State) -> FrozenState:
             }.items()
         )
 
-    def past_states(self, domain: Domain) -> List[State]:
+    def past_states(
+        self, domain: Domain, omit_unset_slots: bool = False,
+    ) -> List[State]:
         """Generate the past states of this tracker based on the history.
 
         Args:
             domain: a :class:`rasa.shared.core.domain.Domain`
+            omit_unset_slots: If `True` do not include the initial values of slots.
 
         Returns:
-            a list of states
+            A list of states
         """
-        return domain.states_for_tracker_history(self)
+        return domain.states_for_tracker_history(
+            self, omit_unset_slots=omit_unset_slots
+        )
 
     def change_loop_to(self, loop_name: Optional[Text]) -> None:
         """Set the currently active loop.
@@ -783,9 +788,9 @@ def _reset_slots(self) -> None:
 
     def _set_slot(self, key: Text, value: Any) -> None:
         """Set the value of a slot if that slot exists."""
-
         if key in self.slots:
-            self.slots[key].value = value
+            slot = self.slots[key]
+            slot.value = value
         else:
             logger.error(
                 f"Tried to set non existent slot '{key}'. Make sure you "
diff --git a/tests/core/policies/test_rule_policy.py b/tests/core/policies/test_rule_policy.py
index 70975bff8cd1..d33358825fbc 100644
--- a/tests/core/policies/test_rule_policy.py
+++ b/tests/core/policies/test_rule_policy.py
@@ -659,7 +659,6 @@ def test_all_policy_attributes_are_persisted(tmpdir: Path):
 async def test_rule_policy_finetune(
     tmp_path: Path, trained_rule_policy: RulePolicy, trained_rule_policy_domain: Domain
 ):
-
     trained_rule_policy.persist(tmp_path)
 
     loaded_policy = RulePolicy.load(tmp_path, should_finetune=True)
@@ -701,7 +700,6 @@ async def test_rule_policy_finetune(
 async def test_rule_policy_contradicting_rule_finetune(
     tmp_path: Path, trained_rule_policy: RulePolicy, trained_rule_policy_domain: Domain
 ):
-
     trained_rule_policy.persist(tmp_path)
 
     loaded_policy = RulePolicy.load(tmp_path, should_finetune=True)
@@ -922,6 +920,113 @@ async def test_predict_form_action_if_multiple_turns():
     assert_predicted_action(prediction, domain, form_name, is_no_user_prediction=True)
 
 
+async def test_predict_slot_initial_value_not_required_in_rule():
+    domain = Domain.from_yaml(
+        """
+intents:
+- i1
+actions:
+- action1
+slots:
+  s_cat1:
+    type: categorical
+    values:
+      - v1
+      - v2
+    initial_value: v1
+"""
+    )
+
+    rule = DialogueStateTracker.from_events(
+        "slot rule",
+        evts=[
+            ActionExecuted(RULE_SNIPPET_ACTION_NAME),
+            ActionExecuted(ACTION_LISTEN_NAME),
+            UserUttered(intent={"name": "i1"}),
+            ActionExecuted("action1"),
+            SlotSet("s_cat1", "v2"),
+            ActionExecuted(ACTION_LISTEN_NAME),
+        ],
+        domain=domain,
+        slots=domain.slots,
+    )
+    rule.is_rule_tracker = True
+
+    policy = RulePolicy()
+    policy.train([rule], domain, RegexInterpreter())
+
+    form_conversation = DialogueStateTracker.from_events(
+        "slot rule test",
+        evts=[
+            ActionExecuted(ACTION_LISTEN_NAME),
+            UserUttered(intent={"name": "i1"}),
+            ActionExecuted("action1"),
+            SlotSet("s_cat1", "v2"),
+            ActionExecuted(ACTION_LISTEN_NAME),
+            UserUttered(intent={"name": "i1"}),
+        ],
+        domain=domain,
+        slots=domain.slots,
+    )
+
+    prediction = policy.predict_action_probabilities(
+        form_conversation, domain, RegexInterpreter()
+    )
+    assert_predicted_action(prediction, domain, "action1")
+
+
+async def test_predict_slot_with_initial_slot_matches_rule():
+    domain = Domain.from_yaml(
+        """
+intents:
+- i1
+actions:
+- action1
+slots:
+  s_cat1:
+    type: categorical
+    values:
+      - v1
+      - v2
+    initial_value: v1
+"""
+    )
+
+    rule = DialogueStateTracker.from_events(
+        "slot rule",
+        evts=[
+            ActionExecuted(RULE_SNIPPET_ACTION_NAME),
+            SlotSet("s_cat1", "v1"),
+            UserUttered(intent={"name": "i1"}),
+            ActionExecuted(ACTION_LISTEN_NAME),
+            ActionExecuted("action1"),
+            ActionExecuted(ACTION_LISTEN_NAME),
+        ],
+        domain=domain,
+        slots=domain.slots,
+    )
+    rule.is_rule_tracker = True
+
+    policy = RulePolicy()
+    policy.train([rule], domain, RegexInterpreter())
+
+    form_conversation = DialogueStateTracker.from_events(
+        "slot rule test",
+        evts=[
+            # We are in an active form
+            ActionExecuted(ACTION_LISTEN_NAME),
+            UserUttered(intent={"name": "i1"}),
+        ],
+        domain=domain,
+        slots=domain.slots,
+    )
+
+    prediction = policy.predict_action_probabilities(
+        form_conversation, domain, RegexInterpreter()
+    )
+    assert_predicted_action(prediction, domain, "action1")
+
+
 async def test_predict_action_listen_after_form():
     form_name = "some_form"
 
diff --git a/tests/shared/core/test_slots.py b/tests/shared/core/test_slots.py
index 92aa44d5f19f..295e0cee98d2 100644
--- a/tests/shared/core/test_slots.py
+++ b/tests/shared/core/test_slots.py
@@ -96,6 +96,18 @@ def test_serialization(self, influence_conversation: bool):
         assert isinstance(slot, slot_type)
         assert recreated.persistence_info() == persistence_info
 
+    @pytest.mark.parametrize("influence_conversation", [True, False])
+    def test_slot_has_been_set(
+        self, influence_conversation: bool, value_feature_pair: Tuple[Any, List[float]]
+    ):
+        slot = self.create_slot(influence_conversation)
+        assert not slot.has_been_set
+        value, _ = value_feature_pair
+        slot.value = value
+        assert slot.has_been_set
+        slot.reset()
+        assert not slot.has_been_set
+
 
 class TestTextSlot(SlotTestCollection):
     def create_slot(self, influence_conversation: bool) -> Slot:


Code review comments:
diff_hunk: @@ -922,6 +920,113 @@ async def test_predict_form_action_if_multiple_turns():
     assert_predicted_action(prediction, domain, form_name, is_no_user_prediction=True)
 
 
+async def test_predict_slot_initial_value_not_required_in_rule():
+    domain = Domain.from_yaml(
+        """
+intents:
+- i1
+actions:
+- action1
+slots:
+  s_cat1:
+    type: categorical
+    values:
+      - v1
+      - v2
+    initial_value: v1
+"""
+    )
+
+    rule = DialogueStateTracker.from_events(
+        "slot rule",
+        evts=[
+            ActionExecuted(RULE_SNIPPET_ACTION_NAME),
+            ActionExecuted(ACTION_LISTEN_NAME),
+            UserUttered(intent={"name": "i1"}),
+            ActionExecuted("action1"),
+            SlotSet("s_cat1", "v2"),
+            ActionExecuted(ACTION_LISTEN_NAME),
+        ],
+        domain=domain,
+        slots=domain.slots,
+    )
+    rule.is_rule_tracker = True
+
+    policy = RulePolicy()
+    policy.train([rule], domain, RegexInterpreter())
+
+    form_conversation = DialogueStateTracker.from_events(
+        "slot rule test",
+        evts=[
+            ActionExecuted(ACTION_LISTEN_NAME),
+            UserUttered(intent={"name": "i1"}),
path: tests/core/policies/test_rule_policy.py
text: @user1:
Are these two events needed? I think they can be removed...

@author:
the `UserUttered` and the `action1`? They are not needed to test this fix, that is true. I was actually modelling it on your example in the issue to make it more realistic, but maybe better to have it more succinct.

@user1:
Yep, I meant those two. I know, my example wasn't the best, but let's remove them from this test to not confuse anyone else with unnecessary stuff.

@author:
ok I've reduced it to what I think is the smallest real example.