RasaHQ/rasa-sdk

Next slot asked not consistent when returning to a form in unhappy path

nbeuchat opened this issue ยท 12 comments

Rasa version: 2.2.1

Rasa SDK version: 2.2.0

Python version: 3.7.9

Operating system (windows, osx, ...): Ubuntu

Issue:
I have a form with a FormValidationAction which overrides the required_slots method. I have a rule for an unhappy path when user ask why we need this info. The bot answers the question and then comes back to the form directly (see rules below).

The problem is that although the slot has not been filled, the next question asked is not the same (ie: not the first unfilled slot in the list of slots returned by required_slots). It even seems that it is not using the list from the required_slots but the one from the domain.

This gives rise to weird conversations like the one below:

image

In this example, I would expect the bot to ask for the user_contact again. Also, has_pet being set to False, the has_pet_type question should not even be asked at all (see the required_slots function below).

Content of domain file (domain.yml):

forms:
  form_preferences:
    ...
    has_pet:
    - intent: deny
      type: from_intent
      value: false
    - entity: animal
      type: from_entity
    - intent: affirm
      type: from_intent
      value: true
    has_pet_type:
    - entity: animal
      type: from_entity
    user_contact:
    - entity: phone_number
      type: from_entity
    - entity: email
      type: from_entity

Contents of actions.py:

class ValidatePreferencesForm(FormValidationAction):
    def name(self) -> Text:
        return "validate_form_preferences"

    async def required_slots(
        self,
        slots_mapped_in_domain: List[Text],
        dispatcher: CollectingDispatcher,
        tracker: Tracker,
        domain: DomainDict,
    ) -> Optional[List[Text]]:
        # TODO: do not override slots_mapped_in_domain once Rasa X overriding issue is addressed
        # See: https://github.com/RasaHQ/rasa/issues/7460

        slots_mapped_in_domain = [
            ...
            "has_pet",
            "has_pet_type",
            "user_contact",
        ]

        if tracker.get_slot("has_pet") is not True:
            pop_from_list(slots_mapped_in_domain, "has_pet_type")

        return slots_mapped_in_domain

Rules

- rule: Activate preferences form
  steps:
  - action: action_introduce_form_preferences
  - action: form_preferences
  - active_loop: form_preferences
 
- rule: Explain within form preferences
  condition:
  - active_loop: form_preferences
  steps:
  - intent: explain
  - action: action_explain
  - action: form_preferences

Sorry @wochinge, I think I found one more issue with the forms ๐Ÿ˜‰

Thanks for the issue, @ArjaanBuijk will get back to you about it soon!

You may find help in the docs and the forum, too ๐Ÿค—

@wochinge @ArjaanBuijk do you have any insights on this one? I'd be happy to dig in the Rasa code and do a PR if you can give me some pointers as to where to get started.

Thanks! ๐Ÿ™‚

The issue is this line:
We currently only call the validate(...) if there is something to validate. After returning from an unhappy path we don't try to extract any slots and hence (in the past) didn't require validating slots.

As the custom action for validating things is also used to determine the slots to request, we need to call it anyway after returning from an unhappy path. We should in my opinion not call validate(...) as this will automatically reject if no slots were extracted but instead validate_slots (might make sense to rename this then to reflects its broader meaning).

Thanks for documenting this issue so well ๐Ÿ™Œ๐Ÿป Does my explanation and potential fix make sense @nbeuchat ?

Thanks for your input @wochinge! That's clear what the issue is now ๐Ÿ™‚

Would something like this fix the issue?

if needs_validation:
            logger.debug(f"Validating user input '{tracker.latest_message}'.")
            return await self.validate(tracker, domain, output_channel, nlg)
else:
            return await self.validate_slots({}, tracker, domain, output_channel, nlg)

where basically we use the validate_slots function with nothing to be validated. I guess that fixes the issue described here but would it have other side-effects?

What would you rename this function to?

@wochinge do you think that would fix the issue? Should I open a PR for this or are there other considerations?

@nbeuchat Yes, that looks great! Would you mind creating a PR with a test? Ideally even against 2.2.x so we can cut a micro immediately

and maybe also add a comment why we still call self.validate_slots in this case ๐Ÿ™Œ๐Ÿป

Sure, will do! I'm flying at the moment but will definitely do that in the next days

Thanks!

stale commented

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.