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

IntentTEDPolicy: rasa interactive updates #8906

Merged
merged 10 commits into from
Jun 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/8421.improvement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Print an information about `action_unlikely_intent` when in gets predicted in the interactive mode.
4 changes: 1 addition & 3 deletions rasa/core/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -637,9 +637,7 @@ def _collect_action_executed_predictions(
# that something else than the form was supposed to run.
predicted_action = action.name()

predicted_warning = action_executed_eval_store.predicted_warning(
predicted_action
)
predicted_warning = action_executed_eval_store.predicted_warning(predicted_action)
if not predicted_warning:
action_executed_eval_store.add_to_store(
action_predictions=[predicted_action], action_targets=[expected_action]
Expand Down
26 changes: 23 additions & 3 deletions rasa/core/training/interactive.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
LOOP_REJECTED,
REQUESTED_SLOT,
LOOP_INTERRUPTED,
ACTION_UNLIKELY_INTENT_NAME,
)
from rasa.core import run, utils
import rasa.core.train
Expand All @@ -60,7 +61,12 @@
UserUtteranceReverted,
)
import rasa.core.interpreter
from rasa.shared.constants import INTENT_MESSAGE_PREFIX, DEFAULT_SENDER_ID, UTTER_PREFIX
from rasa.shared.constants import (
INTENT_MESSAGE_PREFIX,
DEFAULT_SENDER_ID,
UTTER_PREFIX,
DOCS_URL_POLICIES,
)
from rasa.shared.core.trackers import EventVerbosity, DialogueStateTracker
from rasa.shared.core.training_data import visualization
from rasa.shared.core.training_data.visualization import (
Expand Down Expand Up @@ -1130,9 +1136,23 @@ async def _validate_action(

Returns `True` if the prediction is correct, `False` otherwise."""

question = questionary.confirm(f"The bot wants to run '{action_name}', correct?")
if action_name == ACTION_UNLIKELY_INTENT_NAME:
question = questionary.confirm(
f"The bot wants to run '{action_name}' "
f"to indicate that the last user message was unexpected "
f"at this point in the conversation. "
f"Check out IntentTEDPolicy ({DOCS_URL_POLICIES}/#intent-ted-policy) "
f"to learn more."
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to ask the user here to press Y/N to continue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what we've decided to do, yes (to keep it consistent with the rest)

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok, then maybe it should be explicitly stated? Unless questionary takes care of this 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO this should instead be written in docs with the explanation what action_unlikely_intent is and how it works. We can also write it here but I am afraid the message will be too long in this case.

Copy link
Member

Choose a reason for hiding this comment

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

Ah I meant maybe it's not obvious what keys to press from the question and that should be explicitly stated, thats all 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, got it! Actually, questionary handles that and shows the available options automatically :)

)
else:
question = questionary.confirm(
f"The bot wants to run '{action_name}', correct?"
)

is_correct = await _ask_questions(question, conversation_id, endpoint)
is_correct = (
await _ask_questions(question, conversation_id, endpoint)
or action_name == ACTION_UNLIKELY_INTENT_NAME
Copy link
Member

Choose a reason for hiding this comment

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

Does this line imply that regardless of user answer to the query, the action is always correct for action_unlikely_intent?

Copy link
Contributor Author

@alwx alwx Jun 23, 2021

Choose a reason for hiding this comment

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

It means that the choice (Y/N) doesn't affect what's gonna be happening next.

)

if not is_correct:
action_name, is_new_action = await _request_action_from_user(
Expand Down
85 changes: 84 additions & 1 deletion tests/core/training/test_interactive.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from collections import deque
from pathlib import Path
from typing import Any, Dict, List, Text, Tuple, Callable
from tests.conftest import AsyncMock

import mock
import pytest
Expand All @@ -16,7 +17,11 @@
import rasa.utils.io
from rasa.core.actions import action
from rasa.core.training import interactive
from rasa.shared.constants import INTENT_MESSAGE_PREFIX, DEFAULT_SENDER_ID
from rasa.shared.constants import (
INTENT_MESSAGE_PREFIX,
DEFAULT_SENDER_ID,
DOCS_URL_POLICIES,
)
from rasa.shared.core.constants import ACTION_LISTEN_NAME
from rasa.shared.core.domain import Domain
from rasa.shared.core.events import BotUttered, ActionExecuted, UserUttered
Expand Down Expand Up @@ -733,6 +738,84 @@ def test_retry_on_error_success(monkeypatch: MonkeyPatch):
m.assert_called_once_with("export_path", 1, a=2)


@pytest.mark.parametrize(
"action_name, question, is_marked_as_correct, sent_action_name",
[
(
"action_unlikely_intent",
f"The bot wants to run 'action_unlikely_intent' "
f"to indicate that the last user message was unexpected "
f"at this point in the conversation. "
f"Check out IntentTEDPolicy ({DOCS_URL_POLICIES}/#intent-ted-policy) "
f"to learn more.",
True,
"action_unlikely_intent",
),
(
"action_unlikely_intent",
f"The bot wants to run 'action_unlikely_intent' "
f"to indicate that the last user message was unexpected "
f"at this point in the conversation. "
f"Check out IntentTEDPolicy ({DOCS_URL_POLICIES}/#intent-ted-policy) "
f"to learn more.",
False,
"action_unlikely_intent",
),
(
"action_test",
"The bot wants to run 'action_test', correct?",
True,
"action_test",
),
(
"action_test",
"The bot wants to run 'action_test', correct?",
False,
"action_another_one",
),
],
)
async def test_correct_question_for_action_name_was_asked(
monkeypatch: MonkeyPatch,
mock_endpoint: EndpointConfig,
action_name: Text,
question: Text,
is_marked_as_correct: bool,
sent_action_name: Text,
):
conversation_id = "conversation_id"
policy = "policy"
tracker = DialogueStateTracker.from_events("some_sender", [])

monkeypatch.setattr(
interactive,
"retrieve_tracker",
AsyncMock(return_value=tracker.current_state()),
)
monkeypatch.setattr(
interactive, "_ask_questions", AsyncMock(return_value=is_marked_as_correct)
)
monkeypatch.setattr(
interactive,
"_request_action_from_user",
AsyncMock(return_value=("action_another_one", False,)),
)

mocked_send_action = AsyncMock()
monkeypatch.setattr(interactive, "send_action", mocked_send_action)

mocked_confirm = Mock(return_value=None)
monkeypatch.setattr(interactive.questionary, "confirm", mocked_confirm)
wochinge marked this conversation as resolved.
Show resolved Hide resolved

# validate the action and make sure that the correct question was asked
await interactive._validate_action(
action_name, policy, 1.0, [], mock_endpoint, conversation_id
)
mocked_confirm.assert_called_once_with(question)
args, kwargs = mocked_send_action.call_args_list[-1]
assert args[2] == sent_action_name


def test_retry_on_error_three_retries(monkeypatch: MonkeyPatch):
monkeypatch.setattr(interactive.questionary, "confirm", QuestionaryConfirmMock(3))

Expand Down