Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optional slot mappings #7171

Merged
merged 12 commits into from
Nov 6, 2020
16 changes: 16 additions & 0 deletions changelog/7068.improvement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
Slot mappings for [Forms](forms.mdx) in the domain are now optional. If you do not
provide any slot mappings as part of the domain, you need to provide
[custom slot mappings](forms.mdx#custom-slot-mappings) through a custom action.
A form without slot mappings is specified as follows:

```rasa-yaml
forms:
my_form:
# no mappings
```

The action for [forms](forms.mdx) can now be overridden by defining a custom action
with the same name as the form. This can be used to keep using the deprecated
Rasa Open Source `FormAction` which is implemented within the Rasa SDK. Note that it is
**not** recommended to override the form action for anything else than using the
deprecated Rasa SDK `FormAction`.
9 changes: 9 additions & 0 deletions docs/docs/default-actions.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,12 @@ their message.
This action undoes the last user-bot interaction. It can be triggered by the user
by sending a "/back" message to the assistant if the [RulePolicy](./policies.mdx#rule-policy) is configured.
|
## Form Action

By default Rasa Open Source uses `FormAction` for processing any
[form logic](forms.mdx). You can override this default action with a custom action by
adding a custom action with the form's name to the domain.
Overriding the default action for forms should **only** be used during the process of
migrating from Rasa Open Source 1 to 2. In this case you can override the default
action to instruct Rasa Open Source to use the deprecated `FormAction` which is part of
the Rasa SDK.
17 changes: 17 additions & 0 deletions docs/docs/migration-guide.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,23 @@ class RestaurantFormValidator(FormValidationAction):
return {"cuisine": None}
```

You can also migrate forms from Rasa SDK to Rasa Open Source 2 iteratively. You can for
example migrate one form to the Rasa Open Source 2 implementation while continue using
the deprecated Rasa SDK implementation for another form. To continue to use
the deprecated Rasa SDK `FormAction`s, add a custom action with the name of your form to your domain. Note that you should complete the migration as soon as possible as the deprecated `FormAction`
will be removed from the Rasa SDK in Rasa Open Source 3.

```yaml-rasa title="domain.yml"
actions:
# Adding a custom action for a form will
# instruct Rasa Open Source to use the
# deprecated Rasa SDK implementation of forms.
- my_form

forms:
my_form:
```

See the [forms](./forms.mdx) documentation for more details.

### Response Selectors
Expand Down
48 changes: 24 additions & 24 deletions rasa/core/actions/action.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,17 +124,7 @@ def action_for_name(
if action_name not in domain.action_names:
domain.raise_action_not_found_exception(action_name)

should_use_form_action = (
action_name in domain.form_names and domain.slot_mapping_for_form(action_name)
)

return action_from_name(
action_name,
action_endpoint,
domain.user_actions_and_forms,
should_use_form_action,
domain.retrieval_intents,
)
return action_from_name(action_name, domain, action_endpoint)


def is_retrieval_action(action_name: Text, retrieval_intents: List[Text]) -> bool:
Expand All @@ -158,30 +148,40 @@ def is_retrieval_action(action_name: Text, retrieval_intents: List[Text]) -> boo


def action_from_name(
name: Text,
action_endpoint: Optional[EndpointConfig],
user_actions: List[Text],
should_use_form_action: bool = False,
retrieval_intents: Optional[List[Text]] = None,
name: Text, domain: Domain, action_endpoint: Optional[EndpointConfig]
) -> "Action":
"""Return an action instance for the name."""
"""Retrieves an action by its name.

Args:
name: The name of the action.
domain: The current model domain.
action_endpoint: The endpoint to execute custom actions.

Returns:
The instantiated action.
"""
defaults = {a.name(): a for a in default_actions(action_endpoint)}

if name in defaults and name not in user_actions:
if name in defaults and name not in domain.user_actions_and_forms:
return defaults[name]
elif name.startswith(UTTER_PREFIX) and is_retrieval_action(
name, retrieval_intents or []

if name.startswith(UTTER_PREFIX) and is_retrieval_action(
name, domain.retrieval_intents
):
return ActionRetrieveResponse(name)
elif name.startswith(UTTER_PREFIX):

if name.startswith(UTTER_PREFIX):
return ActionUtterTemplate(name)
elif should_use_form_action:

is_form = name in domain.form_names
# Users can override the form by defining an action with the same name as the form
user_overrode_form_action = is_form and name in domain.user_actions
if is_form and not user_overrode_form_action:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    if is_form and not user_overrode_form_action:

is equivalent to (X and not (X and Y)):

    if is_form and not (is_form and name in domain.user_actions):

which in turn is equivalent to (X and not Y):

    if is_form and name not in domain.user_actions:

So then maybe we could just write:

    if name in domain.form_names and name not in domain.user_actions:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assigned names to the single checks to make their meaning more explicit.

Developers might wonder what name not in domain.user_actions means while user_overrode_form_action conveys way more information about this. What do you think?

from rasa.core.actions.forms import FormAction

return FormAction(name, action_endpoint)
else:
return RemoteAction(name, action_endpoint)

return RemoteAction(name, action_endpoint)


def create_bot_utterance(message: Dict[Text, Any]) -> BotUttered:
Expand Down
4 changes: 1 addition & 3 deletions rasa/core/actions/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -554,9 +554,7 @@ async def _ask_for_slot(
logger.debug(f"Request next slot '{slot_name}'")

action_to_ask_for_next_slot = action.action_from_name(
self._name_of_utterance(domain, slot_name),
self.action_endpoint,
domain.user_actions,
self._name_of_utterance(domain, slot_name), domain, self.action_endpoint
)
events_to_ask_for_next_slot = await action_to_ask_for_next_slot.run(
output_channel, nlg, tracker, domain
Expand Down
8 changes: 3 additions & 5 deletions rasa/core/actions/two_stage_fallback.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,7 @@ async def _ask_affirm(
domain: Domain,
) -> List[Event]:
affirm_action = action.action_from_name(
ACTION_DEFAULT_ASK_AFFIRMATION_NAME,
self._action_endpoint,
domain.user_actions,
ACTION_DEFAULT_ASK_AFFIRMATION_NAME, domain, self._action_endpoint
)

return await affirm_action.run(output_channel, nlg, tracker, domain)
Expand All @@ -70,7 +68,7 @@ async def _ask_rephrase(
domain: Domain,
) -> List[Event]:
rephrase = action.action_from_name(
ACTION_DEFAULT_ASK_REPHRASE_NAME, self._action_endpoint, domain.user_actions
ACTION_DEFAULT_ASK_REPHRASE_NAME, domain, self._action_endpoint
)

return await rephrase.run(output_channel, nlg, tracker, domain)
Expand Down Expand Up @@ -138,7 +136,7 @@ async def _give_up(
domain: Domain,
) -> List[Event]:
fallback = action.action_from_name(
ACTION_DEFAULT_FALLBACK_NAME, self._action_endpoint, domain.user_actions
ACTION_DEFAULT_FALLBACK_NAME, domain, self._action_endpoint
)

return await fallback.run(output_channel, nlg, tracker, domain)
Expand Down
70 changes: 48 additions & 22 deletions rasa/shared/core/domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class Domain:

@classmethod
def empty(cls) -> "Domain":
return cls([], [], [], {}, [], [])
return cls([], [], [], {}, [], {})

@classmethod
def load(cls, paths: Union[List[Union[Path, Text]], Text, Path]) -> "Domain":
Expand Down Expand Up @@ -507,13 +507,14 @@ def __init__(
self.intent_properties = self.collect_intent_properties(
intents, self.entities, self.roles, self.groups
)
self.overriden_default_intents = self._collect_overridden_default_intents(
self.overridden_default_intents = self._collect_overridden_default_intents(
intents
)

self.forms: Dict[Text, Any] = {}
self.form_names: List[Text] = []
self._initialize_forms(forms)
self.form_names, self.forms, overridden_form_actions = self._initialize_forms(
forms
)
action_names += overridden_form_actions

self.slots = slots
self.templates = templates
Expand All @@ -528,7 +529,11 @@ def __init__(
# includes all actions (custom, utterance, default actions and forms)
self.action_names = (
self._combine_user_with_default_actions(self.user_actions)
+ self.form_names
+ [
form_name
for form_name in self.form_names
if form_name not in self._custom_actions
]
+ self.action_texts
)

Expand Down Expand Up @@ -569,34 +574,55 @@ def _collect_overridden_default_intents(
}
return sorted(intent_names & set(rasa.shared.core.constants.DEFAULT_INTENTS))

def _initialize_forms(self, forms: Union[Dict[Text, Any], List[Text]]) -> None:
"""Initialize the domain's `self.form` and `self.form_names` attributes.
@staticmethod
def _initialize_forms(
forms: Union[Dict[Text, Any], List[Text]]
) -> Tuple[List[Text], Dict[Text, Any], List[Text]]:
"""Retrieves the initial values for the Domain's form fields.

Args:
forms: Form names (if forms are a list) or a form dictionary. Forms
provided in dictionary format have the form names as keys, and either
empty dictionaries as values, or objects containing
`SlotMapping`s.

Returns:
The form names, a mapping of form names and slot mappings, and custom
actions.
Returning custom actions for each forms means that Rasa Open Source should
not use the default `FormAction` for the forms, but rather a custom action
for it. This can e.g. be used to run the deprecated Rasa Open Source 1
`FormAction` which is implemented in the Rasa SDK.
"""
if not forms:
# empty dict or empty list
return
elif isinstance(forms, dict):
if isinstance(forms, dict):
# dict with slot mappings
self.forms = forms
self.form_names = list(forms.keys())
elif isinstance(forms, list) and isinstance(forms[0], str):
# list of form names
self.forms = {form_name: {} for form_name in forms}
self.form_names = forms
else:
return list(forms.keys()), forms, []

if isinstance(forms, list) and (not forms or isinstance(forms[0], str)):
# list of form names (Rasa Open Source 1 format)
rasa.shared.utils.io.raise_warning(
f"The `forms` section in the domain needs to contain a dictionary. "
f"Instead found an object of type '{type(forms)}'.",
"The `forms` section in the domain used the old Rasa Open Source 1 "
"list format to define forms. Rasa Open Source will be configured to "
"use the deprecated `FormAction` within the Rasa SDK. If you want to "
"use the new Rasa Open Source 2 `FormAction` adapt your `forms` "
"section as described in the documentation. Support for the "
"deprecated `FormAction` in the Rasa SDK will be removed in Rasa Open "
"Source 3.0.",
Comment on lines +604 to +610
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"The `forms` section in the domain used the old Rasa Open Source 1 "
"list format to define forms. Rasa Open Source will be configured to "
"use the deprecated `FormAction` within the Rasa SDK. If you want to "
"use the new Rasa Open Source 2 `FormAction` adapt your `forms` "
"section as described in the documentation. Support for the "
"deprecated `FormAction` in the Rasa SDK will be removed in Rasa Open "
"Source 3.0.",
"The `forms` section in the domain used the old Rasa Open Source 1.0 "
"list format to define forms. Rasa Open Source will be configured to "
"use the deprecated `FormAction` within the Rasa SDK. If you want to "
"use the new Rasa Open Source 2.0 `FormAction` adapt your `forms` "
"section as described in the documentation. Support for the "
"deprecated `FormAction` in the Rasa SDK will be removed in Rasa Open "
"Source 3.0.",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need the .0? It was not only part of Rasa Open Source 1.0, but of all Rasa Open Source 1.x versions, right? Same for 2.0: It's part of 2.x

docs=rasa.shared.constants.DOCS_URL_FORMS,
category=FutureWarning,
)
return forms, {form_name: {} for form_name in forms}, forms

rasa.shared.utils.io.raise_warning(
f"The `forms` section in the domain needs to contain a dictionary. "
f"Instead found an object of type '{type(forms)}'.",
docs=rasa.shared.constants.DOCS_URL_FORMS,
)

return [], {}, []

def __hash__(self) -> int:
"""Returns a unique hash for the domain."""
self_as_dict = self.as_dict()
self_as_dict[
KEY_INTENTS
Expand Down Expand Up @@ -1032,7 +1058,7 @@ def _transform_intents_for_file(self) -> List[Union[Text, Dict[Text, Any]]]:
for intent_name, intent_props in intent_properties.items():
if (
intent_name in rasa.shared.core.constants.DEFAULT_INTENTS
and intent_name not in self.overriden_default_intents
and intent_name not in self.overridden_default_intents
):
# Default intents should be not dumped with the domain
continue
Expand Down
9 changes: 3 additions & 6 deletions tests/core/actions/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -1101,12 +1101,9 @@ async def test_ask_for_slot(
monkeypatch.setattr(action, action.action_from_name.__name__, action_from_name)

form = FormAction("my_form", endpoint_config)
domain = Domain.from_dict(domain)
await form._ask_for_slot(
Domain.from_dict(domain),
None,
None,
slot_name,
DialogueStateTracker.from_events("dasd", []),
domain, None, None, slot_name, DialogueStateTracker.from_events("dasd", [])
)

action_from_name.assert_called_once_with(expected_action, endpoint_config, ANY)
action_from_name.assert_called_once_with(expected_action, domain, endpoint_config)
38 changes: 32 additions & 6 deletions tests/core/test_actions.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import List
from typing import List, Text

import pytest
from aioresponses import aioresponses
Expand Down Expand Up @@ -693,31 +693,57 @@ async def test_action_default_ask_rephrase(
]


def test_get_form_action():
@pytest.mark.parametrize(
"slot_mapping",
[
"""
my_slot:
- type:from_text
""",
"",
],
)
def test_get_form_action(slot_mapping: Text):
form_action_name = "my_business_logic"
domain = Domain.from_yaml(
f"""
actions:
- my_action
forms:
{form_action_name}:
my_slot:
- type: from_text
{slot_mapping}
"""
)

actual = action.action_for_name(form_action_name, domain, None)
assert isinstance(actual, FormAction)


def test_get_form_action_without_slot_mapping():
def test_get_form_action_with_rasa_open_source_1_forms():
form_action_name = "my_business_logic"
with pytest.warns(FutureWarning):
domain = Domain.from_yaml(
f"""
actions:
- my_action
forms:
- {form_action_name}
"""
)

actual = action.action_for_name(form_action_name, domain, None)
assert isinstance(actual, RemoteAction)


def test_overridden_form_action():
form_action_name = "my_business_logic"
domain = Domain.from_yaml(
f"""
actions:
- my_action
forms:
- {form_action_name}
forms:
{form_action_name}:
"""
)

Expand Down
Loading