Merge pull request #9475 from RasaHQ/fix-featurised-slots-in-rules

Fix #9302: Featurised slots sometimes get embedded as conditions in slot-agnostic rules.

Author: samsucik

changelog/9302.bugfix.md

@@ -0,0 +1,2 @@
+Fix rules not being applied when a featurised categorical slot has as one of its allowed
+values `none`, `NoNe`, `None` or a similar value.

rasa/shared/core/domain.py

@@ -1155,19 +1155,25 @@ def _get_slots_sub_state(
             omit_unset_slots: If `True` do not include the initial values of slots.
 
         Returns:
-            a dictionary mapping slot names to their featurization
+            a mapping of slot names to their featurization
         """
         slots = {}
         for slot_name, slot in tracker.slots.items():
+            # If the slot doesn't influence conversations, slot.as_feature() will return
+            # a result that evaluates to False, meaning that the slot shouldn't be
+            # included in featurised sub-states.
+            # Note that this condition checks if the slot itself is None. An unset slot
+            # will be a Slot object and its `value` attribute will be None.
             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()):
-                    # only add slot if some of the features are not zero
+                    # Only include slot in featurised sub-state if the slot is not
+                    # unset, i.e. is set to some actual value and has been successfully
+                    # featurized, and hence has at least one non-zero feature.
                     slots[slot_name] = tuple(slot.as_feature())
-
         return slots
 
     @staticmethod

rasa/shared/core/slots.py

@@ -343,9 +343,24 @@ def __init__(
         super().__init__(
             name, initial_value, value_reset_delay, auto_fill, influence_conversation
         )
-        self.values = [str(v).lower() for v in values] if values else []
+        if values and None in values:
+            rasa.shared.utils.io.raise_warning(
+                f"Categorical slot '{self.name}' has `null` listed as a possible value"
+                f" in the domain file, which translates to `None` in Python. This value"
+                f" is reserved for when the slot is not set, and should not be listed"
+                f" as a value in the slot's definition."
+                f" Rasa will ignore `null` as a possible value for the '{self.name}'"
+                f" slot. Consider changing this value in your domain file to, for"
+                f" example, `unset`, or provide the value explicitly as a string by"
+                f' using quotation marks: "null".',
+                category=UserWarning,
+            )
+        self.values = (
+            [str(v).lower() for v in values if v is not None] if values else []
+        )
 
     def add_default_value(self) -> None:
+        """Adds the special default value to the list of possible values."""
         values = set(self.values)
         if rasa.shared.core.constants.DEFAULT_CATEGORICAL_SLOT_VALUE not in values:
             self.values.append(
@@ -367,31 +382,36 @@ def persistence_info(self) -> Dict[Text, Any]:
     def _as_feature(self) -> List[float]:
         r = [0.0] * self.feature_dimensionality()
 
+        # Return the zero-filled array if the slot is unset (i.e. set to None).
+        # Conceptually, this is similar to the case when the featurisation process
+        # fails, hence the returned features here are the same as for that case.
+        if self.value is None:
+            return r
+
         try:
             for i, v in enumerate(self.values):
                 if v == str(self.value).lower():
                     r[i] = 1.0
                     break
             else:
-                if self.value is not None:
-                    if (
+                if (
+                    rasa.shared.core.constants.DEFAULT_CATEGORICAL_SLOT_VALUE
+                    in self.values
+                ):
+                    i = self.values.index(
                         rasa.shared.core.constants.DEFAULT_CATEGORICAL_SLOT_VALUE
-                        in self.values
-                    ):
-                        i = self.values.index(
-                            rasa.shared.core.constants.DEFAULT_CATEGORICAL_SLOT_VALUE
-                        )
-                        r[i] = 1.0
-                    else:
-                        rasa.shared.utils.io.raise_warning(
-                            f"Categorical slot '{self.name}' is set to a value "
-                            f"('{self.value}') "
-                            "that is not specified in the domain. "
-                            "Value will be ignored and the slot will "
-                            "behave as if no value is set. "
-                            "Make sure to add all values a categorical "
-                            "slot should store to the domain."
-                        )
+                    )
+                    r[i] = 1.0
+                else:
+                    rasa.shared.utils.io.raise_warning(
+                        f"Categorical slot '{self.name}' is set to a value "
+                        f"('{self.value}') "
+                        "that is not specified in the domain. "
+                        "Value will be ignored and the slot will "
+                        "behave as if no value is set. "
+                        "Make sure to add all values a categorical "
+                        "slot should store to the domain."
+                    )
         except (TypeError, ValueError):
             logger.exception("Failed to featurize categorical slot.")
             return r

tests/shared/core/test_slots.py

@@ -239,24 +239,28 @@ class TestCategoricalSlot(SlotTestCollection):
     def create_slot(self, influence_conversation: bool) -> Slot:
         return CategoricalSlot(
             "test",
-            values=[1, "two", "小于", {"three": 3}, None],
+            values=[1, "two", "小于", {"three": 3}, "nOnE", "None", "null"],
             influence_conversation=influence_conversation,
         )
 
-    @pytest.fixture(params=[{"a": "b"}, 2, True, "asd", "🌴"])
+    # None is a special value reserved for unset slots.
+    @pytest.fixture(params=[{"a": "b"}, 2, True, "asd", "🌴", None])
     def invalid_value(self, request: SubRequest) -> Any:
         return request.param
 
     @pytest.fixture(
         params=[
-            (None, [0, 0, 0, 0, 1]),
-            (1, [1, 0, 0, 0, 0]),
-            ("two", [0, 1, 0, 0, 0]),
-            ("小于", [0, 0, 1, 0, 0]),
-            ({"three": 3}, [0, 0, 0, 1, 0]),
+            (None, [0, 0, 0, 0, 0, 0, 0]),  # slot is unset
+            (1, [1, 0, 0, 0, 0, 0, 0]),
+            ("two", [0, 1, 0, 0, 0, 0, 0]),
+            ("小于", [0, 0, 1, 0, 0, 0, 0]),
+            ({"three": 3}, [0, 0, 0, 1, 0, 0, 0]),
+            ("nOnE", [0, 0, 0, 0, 1, 0, 0]),
+            ("None", [0, 0, 0, 0, 1, 0, 0]),  # same as for 'nOnE' (case insensivity)
+            ("null", [0, 0, 0, 0, 0, 0, 1]),
             (
                 rasa.shared.core.constants.DEFAULT_CATEGORICAL_SLOT_VALUE,
-                [0, 0, 0, 0, 0],
+                [0, 0, 0, 0, 0, 0, 0],
             ),
         ]
     )
@@ -268,28 +272,32 @@ class TestCategoricalSlotDefaultValue(SlotTestCollection):
     def create_slot(self, influence_conversation: bool) -> Slot:
         slot = CategoricalSlot(
             "test",
-            values=[1, "two", "小于", {"three": 3}, None],
+            values=[1, "two", "小于", {"three": 3}, "nOnE", "None", "null"],
             influence_conversation=influence_conversation,
         )
         slot.add_default_value()
         return slot
 
-    @pytest.fixture(params=[{"a": "b"}, 2, True, "asd", "🌴"])
+    # None is a special value reserved for unset slots.
+    @pytest.fixture(params=[{"a": "b"}, 2, True, "asd", "🌴", None])
     def invalid_value(self, request: SubRequest) -> Any:
         return request.param
 
     @pytest.fixture(
         params=[
-            (None, [0, 0, 0, 0, 1, 0]),
-            (1, [1, 0, 0, 0, 0, 0]),
-            ("two", [0, 1, 0, 0, 0, 0]),
-            ("小于", [0, 0, 1, 0, 0, 0]),
-            ({"three": 3}, [0, 0, 0, 1, 0, 0]),
+            (None, [0, 0, 0, 0, 0, 0, 0, 0]),  # slot is unset
+            (1, [1, 0, 0, 0, 0, 0, 0, 0]),
+            ("two", [0, 1, 0, 0, 0, 0, 0, 0]),
+            ("小于", [0, 0, 1, 0, 0, 0, 0, 0]),
+            ({"three": 3}, [0, 0, 0, 1, 0, 0, 0, 0]),
+            ("nOnE", [0, 0, 0, 0, 1, 0, 0, 0]),
+            ("None", [0, 0, 0, 0, 1, 0, 0, 0]),  # same as for 'nOnE' (case insensivity)
+            ("null", [0, 0, 0, 0, 0, 0, 1, 0]),
             (
                 rasa.shared.core.constants.DEFAULT_CATEGORICAL_SLOT_VALUE,
-                [0, 0, 0, 0, 0, 1],
+                [0, 0, 0, 0, 0, 0, 0, 1],
             ),
-            ("unseen value", [0, 0, 0, 0, 0, 1]),
+            ("unseen value", [0, 0, 0, 0, 0, 0, 0, 1]),
         ]
     )
     def value_feature_pair(self, request: SubRequest) -> Tuple[Any, List[float]]:
@@ -323,3 +331,14 @@ def test_exception_if_featurized(self):
 def test_raises_on_invalid_slot_type():
     with pytest.raises(InvalidSlotTypeException):
         Slot.resolve_by_type("foobar")
+
+
+def test_categorical_slot_ignores_none_value():
+    """Checks that None can't be added as a possible value for categorical slots."""
+    with pytest.warns(UserWarning) as records:
+        slot = CategoricalSlot(name="branch", values=["Berlin", None, "San Francisco"])
+
+    assert not ("none" in slot.values)
+
+    message_text = "Rasa will ignore `null` as a possible value for the 'branch' slot."
+    assert any(message_text in record.message.args[0] for record in records)