Merge branch 'master' into continuous_training

changelog/7529.bugfix.md

@@ -0,0 +1,3 @@
+Correctly fingerprint the default domain slots. Previously this led to the issue
+that `rasa train core` would always retrain the model even if the training data hasn't
+changed.

changelog/7529.removal.md

@@ -0,0 +1,4 @@
+`Domain.add_categorical_slot_default_value`, `Domain.add_requested_slot`
+and `Domain.add_knowledge_base_slots` are deprecated and will be removed in Rasa Open
+Source 3.0.0. Their internal versions are now called during the Domain creation.
+Calling them manually is no longer required.

data/test_config/max_hist_config.yml

@@ -1,5 +1,6 @@
 policies:
   - name: MemoizationPolicy
     max_history: 5
+  - name: RulePolicy
   - name: TEDPolicy
     max_history: 5

data/test_config/no_max_hist_config.yml

@@ -1,3 +1,4 @@
 policies:
   - name: MemoizationPolicy
+  - name: RulePolicy
   - name: TEDPolicy

data/test_config/stack_config.yml

@@ -8,3 +8,4 @@ pipeline:
 # https://rasa.com/docs/rasa/policies
 policies:
   - name: MemoizationPolicy
+  - name: RulePolicy

data/test_config/ted_random_seed.yaml

@@ -3,3 +3,4 @@ policies:
   random_seed: 42
   validation_split: 0
   max_history: 5
+- name: RulePolicy

data/test_domains/default_with_slots.yml

@@ -30,3 +30,9 @@ responses:
     - text: "bye bye 😢"
   utter_default:   # utterance sent by action_default_fallback
     - text: "sorry, I didn't get that, can you rephrase it?"
+
+forms:
+  some_form:
+    name:
+    - type: from_entity
+      entity: name

data/test_stories/stories_defaultdomain.md

@@ -15,3 +15,7 @@
     - utter_default
 * goodbye
     - utter_goodbye
+
+## simple_story_for_goodbye
+* goodbye
+    - utter_goodbye

data/test_trackers/tracker_moodbot.json

@@ -30,7 +30,7 @@
   "paused": false,
   "latest_event_time": 1517821726.211042,
   "followup_action": null,
-  "slots": {"name": null},
+  "slots": {"name": null, "requested_slot": null},
   "latest_input_channel": null,
   "events": [
     {

data/test_yaml_stories/stories_defaultdomain.yml

@@ -23,3 +23,8 @@ stories:
   - action: utter_default
   - intent: goodbye
   - action: utter_goodbye
+
+- story: simple_story_for_goodbye
+  steps:
+  - intent: goodbye
+  - action: utter_goodbye

docs/docs/nlu-training-data.mdx

@@ -113,7 +113,7 @@ that helps the model learn an association between intents/entities and inputs
 that fit the regular expression. 
 
 :::note Provide Training Examples
-The `RegexFeaturizer` provides features to the entity extractor, but it doesn't predict the entity directly. Include enough examples containing the regular expression so that the entity extractor can learn ot use the regular expression feature.
+The `RegexFeaturizer` provides features to the entity extractor, but it doesn't predict the entity directly. Include enough examples containing the regular expression so that the entity extractor can learn to use the regular expression feature.
 
 :::
 

rasa/core/agent.py

@@ -354,9 +354,6 @@ def __init__(
         self.domain = self._create_domain(domain)
         self.policy_ensemble = self._create_ensemble(policies)
 
-        if self.domain is not None:
-            self.domain.setup_slots()
-
         PolicyEnsemble.check_domain_ensemble_compatibility(
             self.policy_ensemble, self.domain
         )

rasa/core/policies/ensemble.py

@@ -801,10 +801,10 @@ def _check_policy_for_forms_available(
 
     if domain.form_names and not has_policy_for_forms:
         raise InvalidDomain(
-            "You have defined a form action, but haven't added the "
-            "FormPolicy to your policy ensemble. Either remove all "
-            "forms from your domain or exclude the FormPolicy from your "
-            "policy configuration."
+            "You have defined a form action, but have neither added the "
+            f"'{RulePolicy.__name__}' nor the '{FormPolicy.__name__}' (deprecated) to "
+            f"your policy ensemble. Either remove all forms from your domain or add "
+            f"the missing policy to your policy configuration."
         )
 
 

rasa/model.py

@@ -444,7 +444,7 @@ def move_model(source: Text, target: Text) -> bool:
 def should_retrain(
     new_fingerprint: Fingerprint,
     old_model: Text,
-    train_path: Text,
+    train_path: Union[Text, Path],
     force_training: bool = False,
 ) -> FingerprintComparisonResult:
     """Check which components of a model should be retrained.
@@ -580,7 +580,6 @@ async def update_model_with_new_domain(
     """
     model_path = Path(unpacked_model_path) / DEFAULT_CORE_SUBDIRECTORY_NAME
     domain = await importer.get_domain()
-    domain.setup_slots()
     domain.persist(model_path / DEFAULT_DOMAIN_PATH)
 
 

rasa/shared/core/domain.py

@@ -521,7 +521,6 @@ def __init__(
         )
         action_names += overridden_form_actions
 
-        self.slots = slots
         self.templates = templates
         self.action_texts = action_texts or []
         self.session_config = session_config
@@ -542,6 +541,10 @@ def __init__(
             + self.action_texts
         )
 
+        self._user_slots = copy.copy(slots)
+        self.slots = slots
+        self._add_default_slots()
+
         self.store_entities_as_slots = store_entities_as_slots
         self._check_domain_sanity()
 
@@ -684,13 +687,13 @@ def is_retrieval_intent_template(
         """
         return rasa.shared.nlu.constants.RESPONSE_IDENTIFIER_DELIMITER in template[0]
 
-    def setup_slots(self) -> None:
+    def _add_default_slots(self) -> None:
         """Sets up the default slots and slot values for the domain."""
-        self.add_requested_slot()
-        self.add_knowledge_base_slots()
-        self.add_categorical_slot_default_value()
+        self._add_requested_slot()
+        self._add_knowledge_base_slots()
+        self._add_categorical_slot_default_value()
 
-    def add_categorical_slot_default_value(self) -> None:
+    def _add_categorical_slot_default_value(self) -> None:
         """Add a default value to all categorical slots.
 
         All unseen values found for the slot will be mapped to this default value
@@ -699,7 +702,17 @@ def add_categorical_slot_default_value(self) -> None:
         for slot in [s for s in self.slots if isinstance(s, CategoricalSlot)]:
             slot.add_default_value()
 
-    def add_requested_slot(self) -> None:
+    def add_categorical_slot_default_value(self) -> None:
+        """See `_add_categorical_slot_default_value` for docstring."""
+        rasa.shared.utils.io.raise_deprecation_warning(
+            f"'{self.add_categorical_slot_default_value.__name__}' is deprecated and "
+            f"will be removed in Rasa Open Source 3.0.0. This method is now "
+            f"automatically called when the Domain is created which makes a manual "
+            f"call superfluous."
+        )
+        self._add_categorical_slot_default_value()
+
+    def _add_requested_slot(self) -> None:
         """Add a slot called `requested_slot` to the list of slots.
 
         The value of this slot will hold the name of the slot which the user
@@ -715,10 +728,20 @@ def add_requested_slot(self) -> None:
                 )
             )
 
-    def add_knowledge_base_slots(self) -> None:
-        """
-        Add slots for the knowledge base action to the list of slots, if the
-        default knowledge base action name is present.
+    def add_requested_slot(self) -> None:
+        """See `_add_categorical_slot_default_value` for docstring."""
+        rasa.shared.utils.io.raise_deprecation_warning(
+            f"'{self.add_requested_slot.__name__}' is deprecated and "
+            f"will be removed in Rasa Open Source 3.0.0. This method is now "
+            f"automatically called when the Domain is created which makes a manual "
+            f"call superfluous."
+        )
+        self._add_requested_slot()
+
+    def _add_knowledge_base_slots(self) -> None:
+        """Add slots for the knowledge base action to slots.
+
+        Slots are only added if the default knowledge base action name is present.
 
         As soon as the knowledge base action is not experimental anymore, we should
         consider creating a new section in the domain file dedicated to knowledge
@@ -743,9 +766,18 @@ def add_knowledge_base_slots(self) -> None:
                 if s not in slot_names:
                     self.slots.append(TextSlot(s, influence_conversation=False))
 
-    def index_for_action(self, action_name: Text) -> Optional[int]:
-        """Look up which action index corresponds to this action name."""
+    def add_knowledge_base_slots(self) -> None:
+        """See `_add_categorical_slot_default_value` for docstring."""
+        rasa.shared.utils.io.raise_deprecation_warning(
+            f"'{self.add_knowledge_base_slots.__name__}' is deprecated and "
+            f"will be removed in Rasa Open Source 3.0.0. This method is now "
+            f"automatically called when the Domain is created which makes a manual "
+            f"call superfluous."
+        )
+        self._add_knowledge_base_slots()
 
+    def index_for_action(self, action_name: Text) -> Optional[int]:
+        """Looks up which action index corresponds to this action name."""
         try:
             return self.action_names.index(action_name)
         except ValueError:
@@ -1040,9 +1072,12 @@ def compare_with_specification(self, path: Text) -> bool:
             return True
 
     def _slot_definitions(self) -> Dict[Any, Dict[str, Any]]:
-        return {slot.name: slot.persistence_info() for slot in self.slots}
+        # Only persist slots defined by the user. We add the default slots on the
+        # fly when loading the domain.
+        return {slot.name: slot.persistence_info() for slot in self._user_slots}
 
     def as_dict(self) -> Dict[Text, Any]:
+        """Returns serialized domain."""
         return {
             "config": {"store_entities_as_slots": self.store_entities_as_slots},
             SESSION_CONFIG_KEY: {
@@ -1252,7 +1287,7 @@ def _slots_for_domain_warnings(self) -> List[Text]:
         Excludes slots which aren't featurized.
         """
 
-        return [s.name for s in self.slots if s.influence_conversation]
+        return [s.name for s in self._user_slots if s.influence_conversation]
 
     @property
     def _actions_for_domain_warnings(self) -> List[Text]:

rasa/shared/core/slots.py

@@ -304,8 +304,15 @@ def add_default_value(self) -> None:
             )
 
     def persistence_info(self) -> Dict[Text, Any]:
+        """Returns serialized slot."""
         d = super().persistence_info()
-        d["values"] = self.values
+        d["values"] = [
+            value
+            for value in self.values
+            # Don't add default slot when persisting it.
+            # We'll re-add it on the fly when creating the domain.
+            if value != rasa.shared.core.constants.DEFAULT_CATEGORICAL_SLOT_VALUE
+        ]
         return d
 
     def _as_feature(self) -> List[float]:

tests/conftest.py

@@ -24,6 +24,7 @@
 from rasa.core.agent import Agent, load_agent
 from rasa.core.brokers.broker import EventBroker
 from rasa.core.channels import channel, RestInput
+from rasa.core.policies.rule_policy import RulePolicy
 from rasa.shared.core.domain import SessionConfig, Domain
 from rasa.shared.core.events import UserUttered
 from rasa.core.exporter import Exporter
@@ -72,7 +73,7 @@ async def _trained_default_agent(tmpdir_factory: TempdirFactory) -> Agent:
 
     agent = Agent(
         "data/test_domains/default_with_slots.yml",
-        policies=[AugmentedMemoizationPolicy(max_history=3)],
+        policies=[AugmentedMemoizationPolicy(max_history=3), RulePolicy()],
     )
 
     training_data = await agent.load_data(DEFAULT_STORIES_FILE)

tests/core/conftest.py

@@ -26,10 +26,6 @@
 
 DOMAIN_WITH_CATEGORICAL_SLOT = "data/test_domains/domain_with_categorical_slot.yml"
 
-DEFAULT_DOMAIN_PATH_WITH_SLOTS_AND_NO_ACTIONS = (
-    "data/test_domains/default_with_slots_and_no_actions.yml"
-)
-
 DEFAULT_DOMAIN_PATH_WITH_MAPPING = "data/test_domains/default_with_mapping.yml"
 
 DEFAULT_STORIES_FILE = "data/test_stories/stories_defaultdomain.md"
@@ -60,7 +56,6 @@
 
 EXAMPLE_DOMAINS = [
     DEFAULT_DOMAIN_PATH_WITH_SLOTS,
-    DEFAULT_DOMAIN_PATH_WITH_SLOTS_AND_NO_ACTIONS,
     DEFAULT_DOMAIN_PATH_WITH_MAPPING,
     "examples/formbot/domain.yml",
     "examples/moodbot/domain.yml",

tests/core/test_actions.py

@@ -48,6 +48,7 @@
     RULE_SNIPPET_ACTION_NAME,
     ACTIVE_LOOP,
     FOLLOWUP_ACTION,
+    REQUESTED_SLOT,
 )
 from rasa.shared.core.trackers import DialogueStateTracker
 from rasa.utils.endpoints import ClientResponseError, EndpointConfig
@@ -164,7 +165,7 @@ async def test_remote_action_runs(
                 "paused": False,
                 "latest_event_time": None,
                 FOLLOWUP_ACTION: "action_listen",
-                "slots": {"name": None},
+                "slots": {"name": None, REQUESTED_SLOT: None},
                 "events": [],
                 "latest_input_channel": None,
             },
@@ -219,7 +220,7 @@ async def test_remote_action_logs_events(
                 "paused": False,
                 FOLLOWUP_ACTION: ACTION_LISTEN_NAME,
                 "latest_event_time": None,
-                "slots": {"name": None},
+                "slots": {"name": None, REQUESTED_SLOT: None},
                 "events": [],
                 "latest_input_channel": None,
             },

tests/core/test_agent.py

@@ -242,7 +242,7 @@ def test_form_without_form_policy(policy_config: Dict[Text, List[Text]]):
             domain=Domain.from_dict({"forms": ["restaurant_form"]}),
             policies=PolicyEnsemble.from_dict(policy_config),
         )
-    assert "haven't added the FormPolicy" in str(execinfo.value)
+    assert "neither added the 'RulePolicy' nor the 'FormPolicy'" in str(execinfo.value)
 
 
 @pytest.mark.parametrize(

tests/core/test_processor.py

@@ -191,14 +191,13 @@ async def test_reminder_scheduled(
     # retrieve the updated tracker
     t = default_processor.tracker_store.retrieve(sender_id)
 
-    assert t.events[-5] == UserUttered("test")
-    assert t.events[-4] == ActionExecuted("action_schedule_reminder")
-    assert isinstance(t.events[-3], ReminderScheduled)
-    assert t.events[-2] == UserUttered(
+    assert t.events[1] == UserUttered("test")
+    assert t.events[2] == ActionExecuted("action_schedule_reminder")
+    assert isinstance(t.events[3], ReminderScheduled)
+    assert t.events[4] == UserUttered(
         f"{EXTERNAL_MESSAGE_PREFIX}remind",
         intent={INTENT_NAME_KEY: "remind", IS_EXTERNAL: True},
     )
-    assert t.events[-1] == ActionExecuted("action_listen")
 
 
 async def test_trigger_external_latest_input_channel(