diff --git a/changelog/7529.bugfix.md b/changelog/7529.bugfix.md new file mode 100644 index 000000000000..08b96e79816e --- /dev/null +++ b/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. diff --git a/changelog/7529.removal.md b/changelog/7529.removal.md new file mode 100644 index 000000000000..c2b3ed35baa9 --- /dev/null +++ b/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. diff --git a/data/test_config/max_hist_config.yml b/data/test_config/max_hist_config.yml index 3e86d53b91f1..d62c8eb71684 100644 --- a/data/test_config/max_hist_config.yml +++ b/data/test_config/max_hist_config.yml @@ -1,5 +1,6 @@ policies: - name: MemoizationPolicy max_history: 5 + - name: RulePolicy - name: TEDPolicy max_history: 5 diff --git a/data/test_config/no_max_hist_config.yml b/data/test_config/no_max_hist_config.yml index 3d478a200115..033f91727f8d 100644 --- a/data/test_config/no_max_hist_config.yml +++ b/data/test_config/no_max_hist_config.yml @@ -1,3 +1,4 @@ policies: - name: MemoizationPolicy + - name: RulePolicy - name: TEDPolicy diff --git a/data/test_config/stack_config.yml b/data/test_config/stack_config.yml index 63c4a4509dae..0190f0c84d73 100644 --- a/data/test_config/stack_config.yml +++ b/data/test_config/stack_config.yml @@ -8,3 +8,4 @@ pipeline: # https://rasa.com/docs/rasa/policies policies: - name: MemoizationPolicy + - name: RulePolicy diff --git a/data/test_config/ted_random_seed.yaml b/data/test_config/ted_random_seed.yaml index 0bcf2dba2827..260d023163fb 100644 --- a/data/test_config/ted_random_seed.yaml +++ b/data/test_config/ted_random_seed.yaml @@ -3,3 +3,4 @@ policies: random_seed: 42 validation_split: 0 max_history: 5 +- name: RulePolicy diff --git a/data/test_domains/default_with_slots.yml b/data/test_domains/default_with_slots.yml index 3083ae158c06..58c59ed7d831 100644 --- a/data/test_domains/default_with_slots.yml +++ b/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 diff --git a/data/test_domains/default_with_slots_and_no_actions.yml b/data/test_domains/default_with_slots_and_no_actions.yml deleted file mode 100644 index d7234be0f866..000000000000 --- a/data/test_domains/default_with_slots_and_no_actions.yml +++ /dev/null @@ -1,34 +0,0 @@ -version: "2.0" - -# all hashtags are comments :) -intents: - - greet - - default - - goodbye - - affirm - - thank_you - - change_bank_details - - simple - - hello - - why - - next_intent - -entities: - - name - -slots: - name: - type: text - -responses: - utter_greet: - - text: "hey there {name}!" # {name} will be filled by slot (same name) or by custom action - utter_channel: - - text: "this is a default channel" - - text: "you're talking to me on slack!" # if you define channel-specific utterances, the bot will pick - channel: "slack" # from those when talking on that specific channel - utter_goodbye: - - text: "goodbye 😢" # multiple templates - bot will randomly pick one of them - - text: "bye bye 😢" - utter_default: # utterance sent by action_default_fallback - - text: "sorry, I didn't get that, can you rephrase it?" diff --git a/data/test_stories/stories_defaultdomain.md b/data/test_stories/stories_defaultdomain.md index 1690031b0eb7..9d7972b7ce0f 100644 --- a/data/test_stories/stories_defaultdomain.md +++ b/data/test_stories/stories_defaultdomain.md @@ -15,3 +15,7 @@ - utter_default * goodbye - utter_goodbye + +## simple_story_for_goodbye +* goodbye + - utter_goodbye diff --git a/data/test_trackers/tracker_moodbot.json b/data/test_trackers/tracker_moodbot.json index 045458a8ac68..115c56dcde12 100644 --- a/data/test_trackers/tracker_moodbot.json +++ b/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": [ { diff --git a/data/test_yaml_stories/stories_defaultdomain.yml b/data/test_yaml_stories/stories_defaultdomain.yml index 609613f3a5dc..86eecf6fa542 100644 --- a/data/test_yaml_stories/stories_defaultdomain.yml +++ b/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 diff --git a/docs/docs/nlu-training-data.mdx b/docs/docs/nlu-training-data.mdx index 22c20ea24875..682e6eca9603 100644 --- a/docs/docs/nlu-training-data.mdx +++ b/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. ::: diff --git a/rasa/core/agent.py b/rasa/core/agent.py index cb8407d0ea3c..cc2fd8abbc2e 100644 --- a/rasa/core/agent.py +++ b/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 ) diff --git a/rasa/core/policies/ensemble.py b/rasa/core/policies/ensemble.py index dfbce47c3b9e..d12eb094cf82 100644 --- a/rasa/core/policies/ensemble.py +++ b/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." ) diff --git a/rasa/model.py b/rasa/model.py index 556bc73f0a44..fd224eb2c2af 100644 --- a/rasa/model.py +++ b/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) diff --git a/rasa/shared/core/domain.py b/rasa/shared/core/domain.py index 493b6fa16136..d651e55ca2d4 100644 --- a/rasa/shared/core/domain.py +++ b/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]: diff --git a/rasa/shared/core/slots.py b/rasa/shared/core/slots.py index 6b6f9b9696d1..a43b45e184df 100644 --- a/rasa/shared/core/slots.py +++ b/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]: diff --git a/tests/conftest.py b/tests/conftest.py index 65984c7797b7..a02c31f8803b 100644 --- a/tests/conftest.py +++ b/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) diff --git a/tests/core/conftest.py b/tests/core/conftest.py index 527bb6d60ca2..8b4b3f0fb5e4 100644 --- a/tests/core/conftest.py +++ b/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", diff --git a/tests/core/test_actions.py b/tests/core/test_actions.py index bfce0a5bdc91..40c429057240 100644 --- a/tests/core/test_actions.py +++ b/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, }, diff --git a/tests/core/test_agent.py b/tests/core/test_agent.py index 4972c4300103..a26671c67232 100644 --- a/tests/core/test_agent.py +++ b/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( diff --git a/tests/core/test_processor.py b/tests/core/test_processor.py index a8558d8a1537..86cf704b82e9 100644 --- a/tests/core/test_processor.py +++ b/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( diff --git a/tests/core/test_training.py b/tests/core/test_training.py index 3757a0cfce02..ca3b7de32106 100644 --- a/tests/core/test_training.py +++ b/tests/core/test_training.py @@ -6,6 +6,7 @@ from _pytest.monkeypatch import MonkeyPatch from rasa.core.policies.memoization import MemoizationPolicy, OLD_DEFAULT_MAX_HISTORY +from rasa.core.policies.rule_policy import RulePolicy from rasa.shared.core.domain import Domain from rasa.core.interpreter import RasaNLUInterpreter from rasa.shared.nlu.interpreter import RegexInterpreter @@ -113,12 +114,12 @@ async def test_training_script_with_max_history_set(tmp_path: Path): additional_arguments={}, ) agent = Agent.load(tmpdir) + + expected_max_history = {FormPolicy: 2, RulePolicy: None} for policy in agent.policy_ensemble.policies: if hasattr(policy.featurizer, "max_history"): - if type(policy) == FormPolicy: - assert policy.featurizer.max_history == 2 - else: - assert policy.featurizer.max_history == 5 + expected_history = expected_max_history.get(type(policy), 5) + assert policy.featurizer.max_history == expected_history @pytest.mark.parametrize( @@ -181,7 +182,9 @@ async def test_trained_interpreter_passed_to_policies( ): from rasa.core.policies.ted_policy import TEDPolicy - policies_config = {"policies": [{"name": TEDPolicy.__name__}]} + policies_config = { + "policies": [{"name": TEDPolicy.__name__}, {"name": RulePolicy.__name__}] + } policy_train = Mock() monkeypatch.setattr(TEDPolicy, "train", policy_train) diff --git a/tests/shared/core/test_dialogues.py b/tests/shared/core/test_dialogues.py index a982635c3e5a..e8d4273c0d08 100644 --- a/tests/shared/core/test_dialogues.py +++ b/tests/shared/core/test_dialogues.py @@ -9,9 +9,9 @@ from rasa.shared.core.domain import Domain from rasa.core.tracker_store import InMemoryTrackerStore from tests.core.conftest import ( - DEFAULT_DOMAIN_PATH_WITH_SLOTS_AND_NO_ACTIONS, TEST_DIALOGUES, EXAMPLE_DOMAINS, + DEFAULT_DOMAIN_PATH_WITH_SLOTS, ) from tests.core.utilities import tracker_from_dialogue_file @@ -37,7 +37,7 @@ def test_inmemory_tracker_store(pair): def test_tracker_default(): - domain = Domain.load(DEFAULT_DOMAIN_PATH_WITH_SLOTS_AND_NO_ACTIONS) + domain = Domain.load(DEFAULT_DOMAIN_PATH_WITH_SLOTS) filename = "data/test_dialogues/default.json" tracker = tracker_from_dialogue_file(filename, domain) assert tracker.get_slot("name") == "Peter" @@ -45,7 +45,7 @@ def test_tracker_default(): def test_dialogue_from_parameters(): - domain = Domain.load(DEFAULT_DOMAIN_PATH_WITH_SLOTS_AND_NO_ACTIONS) + domain = Domain.load(DEFAULT_DOMAIN_PATH_WITH_SLOTS) filename = "data/test_dialogues/default.json" tracker = tracker_from_dialogue_file(filename, domain) serialised_dialogue = InMemoryTrackerStore.serialise_tracker(tracker) diff --git a/tests/shared/core/test_domain.py b/tests/shared/core/test_domain.py index 124bea433c65..c05077ac2d3f 100644 --- a/tests/shared/core/test_domain.py +++ b/tests/shared/core/test_domain.py @@ -21,6 +21,7 @@ SLOT_LAST_OBJECT_TYPE, DEFAULT_KNOWLEDGE_BASE_ACTION, ENTITY_LABEL_SEPARATOR, + DEFAULT_ACTION_NAMES, ) from rasa.shared.core.domain import ( InvalidDomain, @@ -35,11 +36,7 @@ ) from rasa.shared.core.trackers import DialogueStateTracker from rasa.shared.core.events import ActionExecuted, SlotSet, UserUttered -from tests.core.conftest import ( - DEFAULT_DOMAIN_PATH_WITH_SLOTS, - DEFAULT_DOMAIN_PATH_WITH_SLOTS_AND_NO_ACTIONS, - DEFAULT_STORIES_FILE, -) +from tests.core.conftest import DEFAULT_DOMAIN_PATH_WITH_SLOTS, DEFAULT_STORIES_FILE def test_slots_states_before_user_utterance(default_domain: Domain): @@ -65,7 +62,7 @@ async def test_create_train_data_no_history(default_domain: Domain): DEFAULT_STORIES_FILE, default_domain, augmentation_factor=0 ) - assert len(training_trackers) == 3 + assert len(training_trackers) == 4 (decoded, _) = featurizer.training_states_and_actions( training_trackers, default_domain ) @@ -96,7 +93,7 @@ async def test_create_train_data_with_history(default_domain: Domain): training_trackers = await training.load_data( DEFAULT_STORIES_FILE, default_domain, augmentation_factor=0 ) - assert len(training_trackers) == 3 + assert len(training_trackers) == 4 (decoded, _) = featurizer.training_states_and_actions( training_trackers, default_domain ) @@ -115,6 +112,8 @@ async def test_create_train_data_with_history(default_domain: Domain): '[{}, {"prev_action": {"action_name": "action_listen"}, "slots": {"name": [1.0]}, "user": {"entities": ["name"], "intent": "greet"}}, {"prev_action": {"action_name": "utter_greet"}, "slots": {"name": [1.0]}, "user": {"entities": ["name"], "intent": "greet"}}, {"prev_action": {"action_name": "action_listen"}, "slots": {"name": [1.0]}, "user": {"intent": "default"}}]', '[{}, {"prev_action": {"action_name": "action_listen"}, "slots": {"name": [1.0]}, "user": {"entities": ["name"], "intent": "greet"}}, {"prev_action": {"action_name": "utter_greet"}, "slots": {"name": [1.0]}, "user": {"entities": ["name"], "intent": "greet"}}]', '[{}, {"prev_action": {"action_name": "action_listen"}, "slots": {"name": [1.0]}, "user": {"entities": ["name"], "intent": "greet"}}]', + '[{}, {"prev_action": {"action_name": "action_listen"}, "user": {"intent": "goodbye"}}, {"prev_action": {"action_name": "utter_goodbye"}, "user": {"intent": "goodbye"}}]', + '[{}, {"prev_action": {"action_name": "action_listen"}, "user": {"intent": "goodbye"}}]', '[{}, {"prev_action": {"action_name": "action_listen"}, "user": {"intent": "greet"}}, {"prev_action": {"action_name": "utter_greet"}, "user": {"intent": "greet"}}, {"prev_action": {"action_name": "action_listen"}, "user": {"intent": "default"}}]', '[{}, {"prev_action": {"action_name": "action_listen"}, "user": {"intent": "greet"}}, {"prev_action": {"action_name": "utter_greet"}, "user": {"intent": "greet"}}]', '[{}, {"prev_action": {"action_name": "action_listen"}, "user": {"intent": "greet"}}]', @@ -182,16 +181,22 @@ def test_domain_from_template(): assert not domain.is_empty() assert len(domain.intents) == 10 + len(DEFAULT_INTENTS) - assert len(domain.action_names) == 15 + assert len(domain.action_names) == 16 -def test_avoid_action_repetition(): - domain = Domain.load(DEFAULT_DOMAIN_PATH_WITH_SLOTS) - domain_with_no_actions = Domain.load(DEFAULT_DOMAIN_PATH_WITH_SLOTS_AND_NO_ACTIONS) +def test_avoid_action_repetition(default_domain: Domain): + domain = Domain.from_yaml( + """ +actions: +- utter_greet + +responses: + utter_greet: + - text: "hi" + """ + ) - assert not domain.is_empty() and not domain_with_no_actions.is_empty() - assert len(domain.intents) == len(domain_with_no_actions.intents) - assert len(domain.action_names) == len(domain_with_no_actions.action_names) + assert len(domain.action_names) == len(DEFAULT_ACTION_NAMES) + 1 def test_utter_templates(): @@ -258,7 +263,8 @@ def test_domain_to_dict(): config: store_entities_as_slots: true entities: [] - forms: [] + forms: + some_form: intents: [] responses: utter_greet: @@ -266,7 +272,12 @@ def test_domain_to_dict(): session_config: carry_over_slots_to_new_session: true session_expiration_time: 60 - slots: {}""" + slots: + some_slot: + type: categorical + values: + - high + - low""" domain_as_dict = Domain.from_yaml(test_yaml).as_dict() @@ -274,7 +285,7 @@ def test_domain_to_dict(): "actions": ["action_save_world"], "config": {"store_entities_as_slots": True}, "entities": [], - "forms": {}, + "forms": {"some_form": None}, "intents": [], "e2e_actions": [], "responses": {"utter_greet": [{"text": "hey there!"}]}, @@ -282,7 +293,15 @@ def test_domain_to_dict(): "carry_over_slots_to_new_session": True, "session_expiration_time": 60, }, - "slots": {}, + "slots": { + "some_slot": { + "values": ["high", "low"], + "initial_value": None, + "auto_fill": True, + "influence_conversation": True, + "type": "rasa.shared.core.slots.CategoricalSlot", + } + }, } @@ -676,7 +695,8 @@ def test_domain_warnings(): # all other domain elements should be in `in_domain` diff for _type, elements in zip( - warning_types, [domain.user_actions, domain.intents, domain.entities] + warning_types, + [domain.user_actions + domain.form_names, domain.intents, domain.entities], ): assert set(domain_warnings[_type]["in_domain"]) == set(elements) @@ -684,8 +704,8 @@ def test_domain_warnings(): domain_warnings = domain.domain_warnings( intents=domain.intents, entities=domain.entities, - actions=domain.user_actions, - slots=[s.name for s in domain.slots], + actions=domain.user_actions + domain.form_names, + slots=[s.name for s in domain._user_slots], ) for diff_dict in domain_warnings.values(): @@ -694,22 +714,29 @@ def test_domain_warnings(): def test_unfeaturized_slot_in_domain_warnings(): # create empty domain - domain = Domain.empty() - - # add one unfeaturized and one text slot - unfeaturized_slot = TextSlot( - "unfeaturized_slot", "value1", influence_conversation=False + featurized_slot_name = "text_slot" + unfeaturized_slot_name = "unfeaturized_slot" + domain = Domain.from_dict( + { + "slots": { + featurized_slot_name: {"initial_value": "value2", "type": "text"}, + unfeaturized_slot_name: { + "type": "text", + "initial_value": "value1", + "influence_conversation": False, + }, + } + } ) - text_slot = TextSlot("text_slot", "value2") - domain.slots.extend([unfeaturized_slot, text_slot]) # ensure both are in domain - assert all(slot in domain.slots for slot in (unfeaturized_slot, text_slot)) + for slot in (featurized_slot_name, unfeaturized_slot_name): + assert slot in [slot.name for slot in domain.slots] # text slot should appear in domain warnings, unfeaturized slot should not in_domain_slot_warnings = domain.domain_warnings()["slot_warnings"]["in_domain"] - assert text_slot.name in in_domain_slot_warnings - assert unfeaturized_slot.name not in in_domain_slot_warnings + assert featurized_slot_name in in_domain_slot_warnings + assert unfeaturized_slot_name not in in_domain_slot_warnings def test_check_domain_sanity_on_invalid_domain(): @@ -874,11 +901,8 @@ def test_clean_domain_for_file(): assert cleaned == expected -def test_add_knowledge_base_slots(default_domain: Domain): - # don't modify default domain as it is used in other tests - test_domain = copy.deepcopy(default_domain) - - test_domain.action_names.append(DEFAULT_KNOWLEDGE_BASE_ACTION) +def test_not_add_knowledge_base_slots(): + test_domain = Domain.empty() slot_names = [s.name for s in test_domain.slots] @@ -886,7 +910,14 @@ def test_add_knowledge_base_slots(default_domain: Domain): assert SLOT_LAST_OBJECT not in slot_names assert SLOT_LAST_OBJECT_TYPE not in slot_names - test_domain.add_knowledge_base_slots() + +def test_add_knowledge_base_slots(): + test_domain = Domain.from_yaml( + f""" +actions: +- {DEFAULT_KNOWLEDGE_BASE_ACTION} + """ + ) slot_names = [s.name for s in test_domain.slots] diff --git a/tests/shared/core/test_trackers.py b/tests/shared/core/test_trackers.py index 3307e3b1677a..50d07251a4a8 100644 --- a/tests/shared/core/test_trackers.py +++ b/tests/shared/core/test_trackers.py @@ -640,6 +640,7 @@ async def test_tracker_dump_e2e_story(default_agent: Agent): "* greet: /greet", " - utter_greet", "* goodbye: /goodbye", + " - utter_goodbye", ] diff --git a/tests/shared/core/training_data/story_reader/test_common_story_reader.py b/tests/shared/core/training_data/story_reader/test_common_story_reader.py index fe76e8393b0b..c6926bb13e9e 100644 --- a/tests/shared/core/training_data/story_reader/test_common_story_reader.py +++ b/tests/shared/core/training_data/story_reader/test_common_story_reader.py @@ -163,8 +163,8 @@ async def test_generate_training_data_original_and_augmented_trackers( for t in training_trackers if not hasattr(t, "is_augmented") or not t.is_augmented ] - assert len(original_trackers) == 3 - assert len(training_trackers) <= 33 + assert len(original_trackers) == 4 + assert len(training_trackers) <= 34 @pytest.mark.parametrize( diff --git a/tests/test_model.py b/tests/test_model.py index fa8a12d98b76..52e97902cf95 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -51,7 +51,8 @@ FingerprintComparisonResult, ) from rasa.exceptions import ModelNotFound -from tests.core.conftest import DEFAULT_DOMAIN_PATH_WITH_MAPPING +from rasa.train import train_core, train_core_async +from tests.core.conftest import DEFAULT_DOMAIN_PATH_WITH_MAPPING, DEFAULT_STACK_CONFIG def test_get_latest_model(trained_rasa_model: str): @@ -478,6 +479,33 @@ def test_should_retrain( assert retrain.should_retrain_nlu() == fingerprint["retrain_nlu"] +async def test_should_not_retrain_core(default_domain_path: Text, tmp_path: Path): + # Don't use `default_stories_file` as checkpoints currently break fingerprinting + story_file = tmp_path / "simple_story.yml" + story_file.write_text( + """ +stories: +- story: test_story + steps: + - intent: greet + - action: utter_greet + """ + ) + trained_model = await train_core_async( + default_domain_path, DEFAULT_STACK_CONFIG, str(story_file), str(tmp_path) + ) + + importer = TrainingDataImporter.load_from_config( + DEFAULT_STACK_CONFIG, default_domain_path, training_data_paths=[str(story_file)] + ) + + new_fingerprint = await model.model_fingerprint(importer) + + result = model.should_retrain(new_fingerprint, trained_model, tmp_path) + + assert not result.should_retrain_core() + + def set_fingerprint( trained_rasa_model: Text, fingerprint: Fingerprint, tmp_path: Path ) -> Text: @@ -555,7 +583,6 @@ async def test_update_with_new_domain_preserves_domain( tmpdir: Path, domain_with_categorical_slot_path ): domain = Domain.load(domain_with_categorical_slot_path) - domain.setup_slots() core_directory = tmpdir / DEFAULT_CORE_SUBDIRECTORY_NAME core_directory.mkdir() diff --git a/tests/test_server.py b/tests/test_server.py index 94db125d673a..a04b201cd927 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -43,7 +43,11 @@ ) from rasa.train import TrainingResult from rasa.core.channels.slack import SlackBot -from rasa.shared.core.constants import ACTION_SESSION_START_NAME, ACTION_LISTEN_NAME +from rasa.shared.core.constants import ( + ACTION_SESSION_START_NAME, + ACTION_LISTEN_NAME, + REQUESTED_SLOT, +) from rasa.shared.core.domain import Domain, SessionConfig from rasa.shared.core.events import ( Event, @@ -1039,7 +1043,7 @@ async def test_requesting_non_existent_tracker(rasa_app: SanicASGITestClient): content = response.json() assert response.status == HTTPStatus.OK assert content["paused"] is False - assert content["slots"] == {"name": None} + assert content["slots"] == {"name": None, REQUESTED_SLOT: None} assert content["sender_id"] == "madeupid" assert content["events"] == [ {