From 837eaac91c7976ff8a16130021e75a01810a8362 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Wed, 20 Sep 2023 11:59:28 +0200 Subject: [PATCH 01/20] improved typing of FlowLink --- rasa/shared/core/flows/flow.py | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/rasa/shared/core/flows/flow.py b/rasa/shared/core/flows/flow.py index e5b470ada4f3..41bd1dc1ef0d 100644 --- a/rasa/shared/core/flows/flow.py +++ b/rasa/shared/core/flows/flow.py @@ -1168,17 +1168,12 @@ def as_json(self) -> Any: return [link.as_json() for link in self.links] -class FlowLink(Protocol): +@dataclass +class FlowLink: """Represents a flow link.""" - @property - def target(self) -> Text: - """Returns the target of the flow link. - - Returns: - The target of the flow link. - """ - ... + target: str + """The id of the linked flow.""" def as_json(self) -> Any: """Returns the flow link as a dictionary. @@ -1202,11 +1197,9 @@ def from_json(link_config: Any) -> FlowLink: @dataclass -class IfFlowLink: +class IfFlowLink(FlowLink): """Represents the configuration of an if flow link.""" - target: Text - """The id of the linked flow.""" condition: Optional[Text] """The condition of the linked flow.""" @@ -1235,12 +1228,9 @@ def as_json(self) -> Dict[Text, Any]: @dataclass -class ElseFlowLink: +class ElseFlowLink(FlowLink): """Represents the configuration of an else flow link.""" - target: Text - """The id of the linked flow.""" - @staticmethod def from_json(link_config: Dict[Text, Any]) -> ElseFlowLink: """Used to read flow links from parsed YAML. @@ -1263,12 +1253,9 @@ def as_json(self) -> Dict[Text, Any]: @dataclass -class StaticFlowLink: +class StaticFlowLink(FlowLink): """Represents the configuration of a static flow link.""" - target: Text - """The id of the linked flow.""" - @staticmethod def from_json(link_config: Text) -> StaticFlowLink: """Used to read flow links from parsed YAML. From c7c9947094871fd355ffa34d560811d30a5ea1ba Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Wed, 20 Sep 2023 13:59:59 +0200 Subject: [PATCH 02/20] consistently using kwargs --- rasa/dialogue_understanding/patterns/cancel.py | 2 +- rasa/dialogue_understanding/patterns/clarify.py | 2 +- .../patterns/collect_information.py | 2 +- rasa/dialogue_understanding/patterns/completed.py | 2 +- .../patterns/continue_interrupted.py | 2 +- rasa/dialogue_understanding/patterns/correction.py | 2 +- .../stack/frames/flow_stack_frame.py | 8 ++++---- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/rasa/dialogue_understanding/patterns/cancel.py b/rasa/dialogue_understanding/patterns/cancel.py index fa1eda6316c4..76678285cfd5 100644 --- a/rasa/dialogue_understanding/patterns/cancel.py +++ b/rasa/dialogue_understanding/patterns/cancel.py @@ -59,7 +59,7 @@ def from_dict(data: Dict[str, Any]) -> CancelPatternFlowStackFrame: The created `DialogueStackFrame`. """ return CancelPatternFlowStackFrame( - data["frame_id"], + frame_id=data["frame_id"], step_id=data["step_id"], canceled_name=data["canceled_name"], canceled_frames=data["canceled_frames"], diff --git a/rasa/dialogue_understanding/patterns/clarify.py b/rasa/dialogue_understanding/patterns/clarify.py index 4a7b1df074f0..fdf41d79c247 100644 --- a/rasa/dialogue_understanding/patterns/clarify.py +++ b/rasa/dialogue_understanding/patterns/clarify.py @@ -50,7 +50,7 @@ def from_dict(data: Dict[str, Any]) -> ClarifyPatternFlowStackFrame: The created `DialogueStackFrame`. """ return ClarifyPatternFlowStackFrame( - data["frame_id"], + frame_id=data["frame_id"], step_id=data["step_id"], names=data["names"], clarification_options=data["clarification_options"], diff --git a/rasa/dialogue_understanding/patterns/collect_information.py b/rasa/dialogue_understanding/patterns/collect_information.py index f56c3bfe2c3d..c3d26e9cf852 100644 --- a/rasa/dialogue_understanding/patterns/collect_information.py +++ b/rasa/dialogue_understanding/patterns/collect_information.py @@ -49,7 +49,7 @@ def from_dict(data: Dict[str, Any]) -> CollectInformationPatternFlowStackFrame: ] return CollectInformationPatternFlowStackFrame( - data["frame_id"], + frame_id=data["frame_id"], step_id=data["step_id"], collect_information=data["collect_information"], rejections=rejections, diff --git a/rasa/dialogue_understanding/patterns/completed.py b/rasa/dialogue_understanding/patterns/completed.py index 5852be45c1d8..1392a403804a 100644 --- a/rasa/dialogue_understanding/patterns/completed.py +++ b/rasa/dialogue_understanding/patterns/completed.py @@ -34,7 +34,7 @@ def from_dict(data: Dict[str, Any]) -> CompletedPatternFlowStackFrame: The created `DialogueStackFrame`. """ return CompletedPatternFlowStackFrame( - data["frame_id"], + frame_id=data["frame_id"], step_id=data["step_id"], previous_flow_name=data["previous_flow_name"], ) diff --git a/rasa/dialogue_understanding/patterns/continue_interrupted.py b/rasa/dialogue_understanding/patterns/continue_interrupted.py index 1137408c8383..7a45f9e677b2 100644 --- a/rasa/dialogue_understanding/patterns/continue_interrupted.py +++ b/rasa/dialogue_understanding/patterns/continue_interrupted.py @@ -36,7 +36,7 @@ def from_dict(data: Dict[str, Any]) -> ContinueInterruptedPatternFlowStackFrame: The created `DialogueStackFrame`. """ return ContinueInterruptedPatternFlowStackFrame( - data["frame_id"], + frame_id=data["frame_id"], step_id=data["step_id"], previous_flow_name=data["previous_flow_name"], ) diff --git a/rasa/dialogue_understanding/patterns/correction.py b/rasa/dialogue_understanding/patterns/correction.py index e8626fa6ff20..7dbd5013e667 100644 --- a/rasa/dialogue_understanding/patterns/correction.py +++ b/rasa/dialogue_understanding/patterns/correction.py @@ -75,7 +75,7 @@ def from_dict(data: Dict[Text, Any]) -> CorrectionPatternFlowStackFrame: The created `DialogueStackFrame`. """ return CorrectionPatternFlowStackFrame( - data["frame_id"], + frame_id=data["frame_id"], step_id=data["step_id"], is_reset_only=data["is_reset_only"], corrected_slots=data["corrected_slots"], diff --git a/rasa/dialogue_understanding/stack/frames/flow_stack_frame.py b/rasa/dialogue_understanding/stack/frames/flow_stack_frame.py index ceeb4d5bfe39..20b7cfc6b4be 100644 --- a/rasa/dialogue_understanding/stack/frames/flow_stack_frame.py +++ b/rasa/dialogue_understanding/stack/frames/flow_stack_frame.py @@ -143,8 +143,8 @@ def from_dict(data: Dict[str, Any]) -> UserFlowStackFrame: The created `DialogueStackFrame`. """ return UserFlowStackFrame( - data["frame_id"], - data["flow_id"], - data["step_id"], - FlowStackFrameType.from_str(data.get("frame_type")), + frame_id=data["frame_id"], + flow_id=data["flow_id"], + step_id=data["step_id"], + frame_type=FlowStackFrameType.from_str(data.get("frame_type")), ) From f87bf7e74918759a154b0726e80c0baa1b7f1c2d Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Thu, 21 Sep 2023 09:01:03 +0200 Subject: [PATCH 03/20] implemented stack cleaning and test --- .../commands/cancel_flow_command.py | 4 +- .../commands/clean_stack_command.py | 67 +++++++ .../patterns/clean_stack.py | 96 +++++++++++ .../patterns/default_flows_for_patterns.yml | 13 ++ .../processor/command_processor.py | 47 ++++- rasa/shared/core/constants.py | 10 +- rasa/shared/core/flows/flow.py | 5 + tests/cdu/commands/test_command_processor.py | 163 ++++++++++++++++++ .../commands/test_stack_cleaning_command.py | 101 +++++++++++ 9 files changed, 499 insertions(+), 7 deletions(-) create mode 100644 rasa/dialogue_understanding/commands/clean_stack_command.py create mode 100644 rasa/dialogue_understanding/patterns/clean_stack.py create mode 100644 tests/cdu/commands/test_command_processor.py create mode 100644 tests/cdu/commands/test_stack_cleaning_command.py diff --git a/rasa/dialogue_understanding/commands/cancel_flow_command.py b/rasa/dialogue_understanding/commands/cancel_flow_command.py index 9a880111e7fe..125365142713 100644 --- a/rasa/dialogue_understanding/commands/cancel_flow_command.py +++ b/rasa/dialogue_understanding/commands/cancel_flow_command.py @@ -48,9 +48,9 @@ def select_canceled_frames(stack: DialogueStack) -> List[str]: The frames that were canceled.""" canceled_frames = [] # we need to go through the original stack dump in reverse order - # to find the frames that were canceled. we cancel everthing from + # to find the frames that were canceled. we cancel everything from # the top of the stack until we hit the user flow that was canceled. - # this will also cancel any patterns put ontop of that user flow, + # this will also cancel any patterns put on top of that user flow, # e.g. corrections. for frame in reversed(stack.frames): canceled_frames.append(frame.frame_id) diff --git a/rasa/dialogue_understanding/commands/clean_stack_command.py b/rasa/dialogue_understanding/commands/clean_stack_command.py new file mode 100644 index 000000000000..fa62f5e104ae --- /dev/null +++ b/rasa/dialogue_understanding/commands/clean_stack_command.py @@ -0,0 +1,67 @@ +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any, Dict, List + +import structlog + +from rasa.dialogue_understanding.commands import Command +from rasa.dialogue_understanding.patterns.clean_stack import CleanStackFlowStackFrame +from rasa.dialogue_understanding.stack.dialogue_stack import DialogueStack +from rasa.shared.core.constants import DIALOGUE_STACK_SLOT +from rasa.shared.core.events import Event, SlotSet +from rasa.shared.core.flows.flow import FlowsList +from rasa.shared.core.trackers import DialogueStateTracker +from rasa.dialogue_understanding.stack.utils import top_user_flow_frame + +structlogger = structlog.get_logger() + + +@dataclass +class CleanStackCommand(Command): + """A command to cancel the current flow.""" + + @classmethod + def command(cls) -> str: + """Returns the command type.""" + return "clean stack" + + @classmethod + def from_dict(cls, data: Dict[str, Any]) -> CleanStackCommand: + """Converts the dictionary to a command. + + Returns: + The converted dictionary. + """ + return CleanStackCommand() + + def run_command_on_tracker( + self, + tracker: DialogueStateTracker, + all_flows: FlowsList, + original_tracker: DialogueStateTracker, + ) -> List[Event]: + """Runs the command on the tracker. + + Args: + tracker: The tracker to run the command on. + all_flows: All flows in the assistant. + original_tracker: The tracker before any command was executed. + + Returns: + The events to apply to the tracker. + """ + + stack = DialogueStack.from_tracker(tracker) + original_stack = DialogueStack.from_tracker(original_tracker) + user_frame = top_user_flow_frame(original_stack) + current_flow = user_frame.flow(all_flows) if user_frame else None + + if not current_flow: + structlogger.debug( + "command_executor.skip_clean_stack.no_active_flow", command=self + ) + return [] + + stack.push(CleanStackFlowStackFrame()) + return [SlotSet(DIALOGUE_STACK_SLOT, stack.as_dict())] diff --git a/rasa/dialogue_understanding/patterns/clean_stack.py b/rasa/dialogue_understanding/patterns/clean_stack.py new file mode 100644 index 000000000000..ff7b744801c6 --- /dev/null +++ b/rasa/dialogue_understanding/patterns/clean_stack.py @@ -0,0 +1,96 @@ +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any, Dict, List, Optional + +import structlog +from rasa.dialogue_understanding.stack.dialogue_stack import ( + DialogueStack, +) +from rasa.dialogue_understanding.stack.frames import ( + PatternFlowStackFrame, + BaseFlowStackFrame, + UserFlowStackFrame, +) +from rasa.core.actions import action +from rasa.core.channels.channel import OutputChannel +from rasa.core.nlg.generator import NaturalLanguageGenerator +from rasa.dialogue_understanding.stack.frames.flow_stack_frame import FlowStackFrameType +from rasa.shared.constants import RASA_DEFAULT_FLOW_PATTERN_PREFIX +from rasa.shared.core.constants import DIALOGUE_STACK_SLOT, ACTION_CLEAN_STACK +from rasa.shared.core.domain import Domain +from rasa.shared.core.events import Event, SlotSet +from rasa.shared.core.flows.flow import END_STEP, ContinueFlowStep +from rasa.shared.core.trackers import DialogueStateTracker + + +structlogger = structlog.get_logger() + +FLOW_PATTERN_CLEAN_STACK_ID = RASA_DEFAULT_FLOW_PATTERN_PREFIX + "clean_stack" + + +@dataclass +class CleanStackFlowStackFrame(PatternFlowStackFrame): + """A pattern flow stack frame which cleans the stack after a bot update.""" + + flow_id: str = FLOW_PATTERN_CLEAN_STACK_ID + """The ID of the flow.""" + + @classmethod + def type(cls) -> str: + """Returns the type of the frame.""" + return "pattern_clean_stack" + + @staticmethod + def from_dict(data: Dict[str, Any]) -> CleanStackFlowStackFrame: + """Creates a `DialogueStackFrame` from a dictionary. + + Args: + data: The dictionary to create the `DialogueStackFrame` from. + + Returns: + The created `DialogueStackFrame`. + """ + return CleanStackFlowStackFrame( + frame_id=data["frame_id"], + step_id=data["step_id"], + ) + + +class ActionCleanStack(action.Action): + """Action which cancels a flow from the stack.""" + + def name(self) -> str: + """Return the flow name.""" + return ACTION_CLEAN_STACK + + async def run( + self, + output_channel: OutputChannel, + nlg: NaturalLanguageGenerator, + tracker: DialogueStateTracker, + domain: Domain, + metadata: Optional[Dict[str, Any]] = None, + ) -> List[Event]: + """Clean the stack.""" + stack = DialogueStack.from_tracker(tracker) + if not (top := stack.top()): + structlogger.warning("action.clean_stack.no_active_flow") + return [] + + if not isinstance(top, CleanStackFlowStackFrame): + structlogger.warning("action.clean_stack.no_cleaning_frame", top=top) + return [] + + new_frames = [] + # Set all frames to their end step, filter out any non-BaseFlowStackFrames + for frame in stack.frames: + if isinstance(frame, BaseFlowStackFrame): + frame.step_id = ContinueFlowStep.continue_step_for_id(END_STEP) + if isinstance(frame, UserFlowStackFrame): + # Making sure there are no "continue interrupts" triggered + frame.frame_type = FlowStackFrameType.REGULAR + new_frames.append(frame) + new_stack = DialogueStack.from_dict([frame.as_dict() for frame in new_frames]) + + return [SlotSet(DIALOGUE_STACK_SLOT, new_stack.as_dict())] diff --git a/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml b/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml index 93373798b4d3..8fe77c267ef1 100644 --- a/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml +++ b/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml @@ -32,6 +32,10 @@ responses: rephrase: True template: jinja + utter_inform_stack_cleanup_rasa: + - text: There has been an update to my code. I need to wrap up our running dialogue tasks and start from scratch. + metadata: + rephrase: True slots: confirm_correction: @@ -125,3 +129,12 @@ flows: action: action_listen next: "start" - id: "done" + + pattern_clean_stack: + description: flow used to clean the stack after a bot update + steps: + - id: "inform_user" + action: utter_inform_stack_cleanup + next: "run_cleanup" + - id: "run_cleanup" + action: action_clean_stack \ No newline at end of file diff --git a/rasa/dialogue_understanding/processor/command_processor.py b/rasa/dialogue_understanding/processor/command_processor.py index c9919900b448..fe1863f5ec7f 100644 --- a/rasa/dialogue_understanding/processor/command_processor.py +++ b/rasa/dialogue_understanding/processor/command_processor.py @@ -1,4 +1,4 @@ -from typing import List, Optional, Type +from typing import List, Optional, Type, Set, Tuple, Dict import structlog from rasa.dialogue_understanding.commands import ( @@ -9,6 +9,7 @@ SetSlotCommand, FreeFormAnswerCommand, ) +from rasa.dialogue_understanding.commands.clean_stack_command import CleanStackCommand from rasa.dialogue_understanding.patterns.collect_information import ( CollectInformationPatternFlowStackFrame, ) @@ -23,6 +24,7 @@ filled_slots_for_active_flow, top_flow_frame, ) +from rasa.shared.core.constants import FLOW_HASHES_SLOT from rasa.shared.core.events import Event, SlotSet from rasa.shared.core.flows.flow import ( FlowsList, @@ -95,6 +97,39 @@ def validate_state_of_commands(commands: List[Command]) -> None: assert sum(isinstance(c, CorrectSlotsCommand) for c in commands) <= 1 +def find_updated_flows(tracker: DialogueStateTracker, all_flows: FlowsList) -> Set[str]: + """Find the set of updated flows. + + Run through the current dialogue stack and compare the flow hashes of the + flows on the stack with those stored in the tracker. + + Args: + tracker: The tracker. + all_flows: All flows. + + Returns: + A set of flow ids of those flows that have changed + """ + stored_fingerprints = tracker.get_slot(FLOW_HASHES_SLOT) + dialogue_stack = DialogueStack.from_tracker(tracker) + + changed_flows = set() + for frame in dialogue_stack.frames: + if isinstance(frame, BaseFlowStackFrame): + flow = all_flows.flow_by_id(frame.flow_id) + if flow is None or ( + flow.id in stored_fingerprints + and flow.fingerprint() != stored_fingerprints[flow.id] + ): + changed_flows.add(frame.flow_id) + return changed_flows + + +def calculate_flow_fingerprints(all_flows: FlowsList) -> Dict[str, str]: + """Calculate fingerprints for all flows.""" + return {flow.id: flow.fingerprint() for flow in all_flows.underlying_flows} + + def execute_commands( tracker: DialogueStateTracker, all_flows: FlowsList ) -> List[Event]: @@ -113,6 +148,16 @@ def execute_commands( commands = clean_up_commands(commands, tracker, all_flows) + updated_flows = find_updated_flows(tracker, all_flows) + if updated_flows: + # Override commands + commands = [CleanStackCommand()] + + # always store current flow hashes + tracker.update_with_events( + [SlotSet(FLOW_HASHES_SLOT, calculate_flow_fingerprints(all_flows))], None + ) + events: List[Event] = [] # commands need to be reversed to make sure they end up in the right order diff --git a/rasa/shared/core/constants.py b/rasa/shared/core/constants.py index ed2d31b4cee5..f9991621b2a9 100644 --- a/rasa/shared/core/constants.py +++ b/rasa/shared/core/constants.py @@ -41,6 +41,7 @@ ACTION_CLARIFY_FLOWS = "action_clarify_flows" ACTION_CORRECT_FLOW_SLOT = "action_correct_flow_slot" ACTION_RUN_SLOT_REJECTIONS_NAME = "action_run_slot_rejections" +ACTION_CLEAN_STACK = "action_clean_stack" DEFAULT_ACTION_NAMES = [ @@ -62,6 +63,7 @@ ACTION_CORRECT_FLOW_SLOT, ACTION_CLARIFY_FLOWS, ACTION_RUN_SLOT_REJECTIONS_NAME, + ACTION_CLEAN_STACK, ] ACTION_SHOULD_SEND_DOMAIN = "send_domain" @@ -89,11 +91,9 @@ REQUESTED_SLOT = "requested_slot" DIALOGUE_STACK_SLOT = "dialogue_stack" RETURN_VALUE_SLOT = "return_value" +FLOW_HASHES_SLOT = "flow_hashes" -FLOW_SLOT_NAMES = [ - DIALOGUE_STACK_SLOT, - RETURN_VALUE_SLOT, -] +FLOW_SLOT_NAMES = [DIALOGUE_STACK_SLOT, RETURN_VALUE_SLOT, FLOW_HASHES_SLOT] # slots for knowledge base SLOT_LISTED_ITEMS = "knowledge_base_listed_objects" @@ -108,6 +108,8 @@ SLOT_LISTED_ITEMS, SLOT_LAST_OBJECT, SLOT_LAST_OBJECT_TYPE, + RETURN_VALUE_SLOT, + FLOW_HASHES_SLOT, } diff --git a/rasa/shared/core/flows/flow.py b/rasa/shared/core/flows/flow.py index 41bd1dc1ef0d..944788d2fa04 100644 --- a/rasa/shared/core/flows/flow.py +++ b/rasa/shared/core/flows/flow.py @@ -1,5 +1,6 @@ from __future__ import annotations +import functools from dataclasses import dataclass from enum import Enum from typing import Any, Dict, List, Optional, Protocol, Set, Text, runtime_checkable @@ -381,6 +382,10 @@ def get_collect_information_steps(self) -> List[CollectInformationFlowStep]: collect_information_steps.append(step) return collect_information_steps + def fingerprint(self) -> str: + """Create a fingerprint identifying this flow.""" + return rasa.shared.utils.io.deep_container_fingerprint(self.as_json()) + def step_from_json(flow_step_config: Dict[Text, Any]) -> FlowStep: """Used to read flow steps from parsed YAML. diff --git a/tests/cdu/commands/test_command_processor.py b/tests/cdu/commands/test_command_processor.py new file mode 100644 index 000000000000..1d3315c3b7b9 --- /dev/null +++ b/tests/cdu/commands/test_command_processor.py @@ -0,0 +1,163 @@ +import pytest + +from rasa.dialogue_understanding.commands import StartFlowCommand +from rasa.dialogue_understanding.processor.command_processor import ( + execute_commands, + find_updated_flows, +) +from rasa.dialogue_understanding.stack.dialogue_stack import DialogueStack +from rasa.dialogue_understanding.stack.frames import ( + UserFlowStackFrame, + PatternFlowStackFrame, +) +from rasa.shared.core.constants import FLOW_HASHES_SLOT +from rasa.shared.core.events import UserUttered +from rasa.shared.core.flows.flow import FlowsList +from rasa.shared.core.trackers import DialogueStateTracker +from rasa.shared.nlu.constants import COMMANDS +from tests.utilities import flows_from_str + + +@pytest.fixture +def all_flows() -> FlowsList: + return flows_from_str( + """ + flows: + foo: + steps: + - id: first_step + action: action_listen + bar: + steps: + - id: also_first_step + action: action_listen + """ + ) + + +start_foo_user_uttered = UserUttered( + "start foo", None, None, {COMMANDS: [StartFlowCommand("foo").as_dict()]} +) + +start_bar_user_uttered = UserUttered( + "start bar", None, None, {COMMANDS: [StartFlowCommand("bar").as_dict()]} +) + + +@pytest.fixture +def tracker(all_flows: FlowsList) -> DialogueStateTracker: + # Creates a useful tracker that has a started flow and the current flows hashed + tracker = DialogueStateTracker.from_events("test", evts=[start_foo_user_uttered]) + execute_commands(tracker, all_flows) + return tracker + + +def test_properly_prepared_tracker(tracker: DialogueStateTracker): + # flow hashes have been initialized + assert "foo" in tracker.get_slot(FLOW_HASHES_SLOT) + + # foo flow is on the stack + dialogue_stack = DialogueStack.from_tracker(tracker) + assert (top_frame := dialogue_stack.top()) + assert isinstance(top_frame, UserFlowStackFrame) + assert top_frame.flow_id == "foo" + + +def test_detects_no_changes_when_nothing_changed( + tracker: DialogueStateTracker, all_flows: FlowsList +): + assert find_updated_flows(tracker, all_flows) == set() + + +def test_detects_no_changes_for_not_started_flows( + tracker: DialogueStateTracker, +): + bar_changed_flows = flows_from_str( + """ + flows: + foo: + steps: + - id: first_step + action: action_listen + bar: + steps: + - id: also_first_step_BUT_CHANGED + action: action_listen + """ + ) + assert find_updated_flows(tracker, bar_changed_flows) == set() + + +change_cases = { + "step_id_changed": """ + flows: + foo: + steps: + - id: first_step_id_BUT_CHANGED + action: action_listen + bar: + steps: + - id: also_first_step + action: action_listen + """, + "action_changed": """ + flows: + foo: + steps: + - id: first_step_id + action: action_CHANGED + bar: + steps: + - id: also_first_step + action: action_listen + """, + "new_step": """ + flows: + foo: + steps: + - id: first_step_id + action: action_listen + next: second_step_id + - id: second_step_id + action: action_cool_stuff + bar: + steps: + - id: also_first_step + action: action_listen + """, + "flow_removed": """ + flows: + bar: + steps: + - id: also_first_step + action: action_listen + """, +} + + +@pytest.mark.parametrize("case, flow_yaml", list(change_cases.items())) +def test_detects_changes(case: str, flow_yaml: str, tracker: DialogueStateTracker): + all_flows = flows_from_str(flow_yaml) + assert find_updated_flows(tracker, all_flows) == {"foo"} + + +def test_starting_of_another_flow(tracker: DialogueStateTracker, all_flows: FlowsList): + """Tests that commands are not discarded when there is no change.""" + tracker.update_with_events([start_bar_user_uttered], None) + execute_commands(tracker, all_flows) + dialogue_stack = DialogueStack.from_tracker(tracker) + assert len(dialogue_stack.frames) == 2 + assert (top_frame := dialogue_stack.top()) + assert isinstance(top_frame, UserFlowStackFrame) + assert top_frame.flow_id == "bar" + + +def test_stack_cleaning_command_is_applied_on_changes(tracker: DialogueStateTracker): + all_flows = flows_from_str(change_cases["step_id_changed"]) + tracker.update_with_events([start_bar_user_uttered], None) + execute_commands(tracker, all_flows) + dialogue_stack = DialogueStack.from_tracker(tracker) + assert len(dialogue_stack.frames) == 2 + assert (top_frame := dialogue_stack.top()) + assert isinstance(top_frame, PatternFlowStackFrame) + assert top_frame.flow_id == "pattern_clean_stack" diff --git a/tests/cdu/commands/test_stack_cleaning_command.py b/tests/cdu/commands/test_stack_cleaning_command.py new file mode 100644 index 000000000000..d10bc553b42f --- /dev/null +++ b/tests/cdu/commands/test_stack_cleaning_command.py @@ -0,0 +1,101 @@ +import pytest + +from rasa.core.channels import CollectingOutputChannel +from rasa.core.nlg import TemplatedNaturalLanguageGenerator +from rasa.dialogue_understanding.commands.clean_stack_command import CleanStackCommand +from rasa.dialogue_understanding.patterns.clean_stack import ActionCleanStack +from rasa.dialogue_understanding.processor.command_processor import execute_commands +from rasa.dialogue_understanding.stack.dialogue_stack import DialogueStack +from rasa.dialogue_understanding.stack.frames import ( + UserFlowStackFrame, + PatternFlowStackFrame, +) +from rasa.shared.core.domain import Domain +from rasa.shared.core.events import SlotSet +from rasa.shared.core.flows.flow import ( + FlowsList, + START_STEP, + ContinueFlowStep, + END_STEP, +) +from rasa.shared.core.trackers import DialogueStateTracker +from tests.cdu.commands.test_command_processor import ( + tracker, + all_flows, + start_bar_user_uttered, + change_cases, +) +from tests.utilities import flows_from_str + + +def test_name_of_command(): + # names of commands should not change as they are part of persisted + # trackers + assert CleanStackCommand.command() == "clean stack" + + +def test_from_dict(): + assert CleanStackCommand.from_dict({}) == CleanStackCommand() + + +def test_run_command_on_tracker(tracker: DialogueStateTracker, all_flows: FlowsList): + command = CleanStackCommand() + events = command.run_command_on_tracker(tracker, all_flows, tracker) + assert len(events) == 1 + dialogue_stack_event = events[0] + assert isinstance(dialogue_stack_event, SlotSet) + assert dialogue_stack_event.key == "dialogue_stack" + assert len(dialogue_stack_event.value) == 2 + + frame = dialogue_stack_event.value[1] + assert frame["type"] == "pattern_clean_stack" + + +@pytest.fixture +def about_to_be_cleaned_tracker(tracker: DialogueStateTracker, all_flows: FlowsList): + tracker.update_with_events([start_bar_user_uttered], None) + execute_commands(tracker, all_flows) + changed_flows = flows_from_str(change_cases["step_id_changed"]) + execute_commands(tracker, changed_flows) + dialogue_stack = DialogueStack.from_tracker(tracker) + assert len(dialogue_stack.frames) == 3 + + foo_frame = dialogue_stack.frames[0] + assert isinstance(foo_frame, UserFlowStackFrame) + assert foo_frame.flow_id == "foo" + assert foo_frame.step_id == START_STEP + + bar_frame = dialogue_stack.frames[1] + assert isinstance(bar_frame, UserFlowStackFrame) + assert bar_frame.flow_id == "bar" + assert bar_frame.step_id == START_STEP + + stack_clean_frame = dialogue_stack.frames[2] + assert isinstance(stack_clean_frame, PatternFlowStackFrame) + assert stack_clean_frame.flow_id == "pattern_clean_stack" + assert stack_clean_frame.step_id == START_STEP + + return tracker + + +async def test_stack_cleaning_action(about_to_be_cleaned_tracker: DialogueStateTracker): + events = await ActionCleanStack().run( + CollectingOutputChannel(), + TemplatedNaturalLanguageGenerator({}), + about_to_be_cleaned_tracker, + Domain.empty(), + ) + about_to_be_cleaned_tracker.update_with_events(events, None) + + dialogue_stack = DialogueStack.from_tracker(about_to_be_cleaned_tracker) + assert len(dialogue_stack.frames) == 3 + + foo_frame = dialogue_stack.frames[0] + assert isinstance(foo_frame, UserFlowStackFrame) + assert foo_frame.flow_id == "foo" + assert foo_frame.step_id == ContinueFlowStep.continue_step_for_id(END_STEP) + + bar_frame = dialogue_stack.frames[1] + assert isinstance(bar_frame, UserFlowStackFrame) + assert bar_frame.flow_id == "bar" + assert bar_frame.step_id == ContinueFlowStep.continue_step_for_id(END_STEP) From f7a43918bbaad1e57bb670ea1a02255ad9dde744 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Fri, 22 Sep 2023 11:33:19 +0200 Subject: [PATCH 04/20] Revert "improved typing of FlowLink" This reverts commit 37d1b26ed3e3aa6fec11b3d2ac63ad9da6164be8. --- rasa/shared/core/flows/flow.py | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/rasa/shared/core/flows/flow.py b/rasa/shared/core/flows/flow.py index 944788d2fa04..2f32665fb0a5 100644 --- a/rasa/shared/core/flows/flow.py +++ b/rasa/shared/core/flows/flow.py @@ -1173,12 +1173,17 @@ def as_json(self) -> Any: return [link.as_json() for link in self.links] -@dataclass -class FlowLink: +class FlowLink(Protocol): """Represents a flow link.""" - target: str - """The id of the linked flow.""" + @property + def target(self) -> Text: + """Returns the target of the flow link. + + Returns: + The target of the flow link. + """ + ... def as_json(self) -> Any: """Returns the flow link as a dictionary. @@ -1202,9 +1207,11 @@ def from_json(link_config: Any) -> FlowLink: @dataclass -class IfFlowLink(FlowLink): +class IfFlowLink: """Represents the configuration of an if flow link.""" + target: Text + """The id of the linked flow.""" condition: Optional[Text] """The condition of the linked flow.""" @@ -1233,9 +1240,12 @@ def as_json(self) -> Dict[Text, Any]: @dataclass -class ElseFlowLink(FlowLink): +class ElseFlowLink: """Represents the configuration of an else flow link.""" + target: Text + """The id of the linked flow.""" + @staticmethod def from_json(link_config: Dict[Text, Any]) -> ElseFlowLink: """Used to read flow links from parsed YAML. @@ -1258,9 +1268,12 @@ def as_json(self) -> Dict[Text, Any]: @dataclass -class StaticFlowLink(FlowLink): +class StaticFlowLink: """Represents the configuration of a static flow link.""" + target: Text + """The id of the linked flow.""" + @staticmethod def from_json(link_config: Text) -> StaticFlowLink: """Used to read flow links from parsed YAML. From a0b95b38f5000d0dfa188b5d766c76941d076cdc Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 25 Sep 2023 10:11:31 +0200 Subject: [PATCH 05/20] improved and fixed tests relying on strict number indices --- .../featurizers/test_tracker_featurizer.py | 96 +++++++++++++++++-- 1 file changed, 88 insertions(+), 8 deletions(-) diff --git a/tests/core/featurizers/test_tracker_featurizer.py b/tests/core/featurizers/test_tracker_featurizer.py index c65bf18e1a60..6b55f5335a3a 100644 --- a/tests/core/featurizers/test_tracker_featurizer.py +++ b/tests/core/featurizers/test_tracker_featurizer.py @@ -22,6 +22,7 @@ ACTION_UNLIKELY_INTENT_NAME, USER, PREVIOUS_ACTION, + DEFAULT_ACTION_NAMES, ) from rasa.shared.core.events import ActionExecuted from rasa.shared.core.trackers import DialogueStateTracker @@ -179,14 +180,25 @@ def test_featurize_trackers_with_full_dialogue_tracker_featurizer( }, ] ] - assert actual_features is not None assert len(actual_features) == len(expected_features) for actual, expected in zip(actual_features, expected_features): assert compare_featurized_states(actual, expected) - expected_labels = np.array([[0, 21, 0, 18, 19, 0, 20]]) + expected_labels = np.array( + [ + [ + 0, + moodbot_domain.action_names_or_texts.index("utter_greet"), + 0, + moodbot_domain.action_names_or_texts.index("utter_cheer_up"), + moodbot_domain.action_names_or_texts.index("utter_did_that_help"), + 0, + moodbot_domain.action_names_or_texts.index("utter_goodbye"), + ] + ] + ) assert actual_labels is not None assert len(actual_labels) == 1 for actual, expected in zip(actual_labels, expected_labels): @@ -255,7 +267,19 @@ def test_trackers_ignore_action_unlikely_intent_with_full_dialogue_tracker_featu for actual, expected in zip(actual_features, expected_features): assert compare_featurized_states(actual, expected) - expected_labels = np.array([[0, 21, 0, 18, 19, 0, 20]]) + expected_labels = np.array( + [ + [ + 0, + moodbot_domain.action_names_or_texts.index("utter_greet"), + 0, + moodbot_domain.action_names_or_texts.index("utter_cheer_up"), + moodbot_domain.action_names_or_texts.index("utter_did_that_help"), + 0, + moodbot_domain.action_names_or_texts.index("utter_goodbye"), + ] + ] + ) assert actual_labels is not None assert len(actual_labels) == 1 for actual, expected in zip(actual_labels, expected_labels): @@ -324,7 +348,22 @@ def test_trackers_keep_action_unlikely_intent_with_full_dialogue_tracker_featuri for actual, expected in zip(actual_features, expected_features): assert compare_featurized_states(actual, expected) - expected_labels = np.array([[0, 9, 21, 0, 9, 18, 19, 0, 9, 20]]) + expected_labels = np.array( + [ + [ + 0, + moodbot_domain.action_names_or_texts.index("action_unlikely_intent"), + moodbot_domain.action_names_or_texts.index("utter_greet"), + 0, + moodbot_domain.action_names_or_texts.index("action_unlikely_intent"), + moodbot_domain.action_names_or_texts.index("utter_cheer_up"), + moodbot_domain.action_names_or_texts.index("utter_did_that_help"), + 0, + moodbot_domain.action_names_or_texts.index("action_unlikely_intent"), + moodbot_domain.action_names_or_texts.index("utter_goodbye"), + ] + ] + ) assert actual_labels is not None assert len(actual_labels) == 1 for actual, expected in zip(actual_labels, expected_labels): @@ -832,7 +871,19 @@ def test_featurize_trackers_with_max_history_tracker_featurizer( for actual, expected in zip(actual_features, expected_features): assert compare_featurized_states(actual, expected) - expected_labels = np.array([[0, 21, 0, 18, 19, 0, 20]]).T + expected_labels = np.array( + [ + [ + 0, + moodbot_domain.action_names_or_texts.index("utter_greet"), + 0, + moodbot_domain.action_names_or_texts.index("utter_cheer_up"), + moodbot_domain.action_names_or_texts.index("utter_did_that_help"), + 0, + moodbot_domain.action_names_or_texts.index("utter_goodbye"), + ] + ] + ).T assert actual_labels is not None assert actual_labels.shape == expected_labels.shape @@ -899,7 +950,15 @@ def test_featurize_trackers_ignore_action_unlikely_intent_max_history_featurizer for actual, expected in zip(actual_features, expected_features): assert compare_featurized_states(actual, expected) - expected_labels = np.array([[0, 21, 0]]).T + expected_labels = np.array( + [ + [ + 0, + moodbot_domain.action_names_or_texts.index("utter_greet"), + 0, + ] + ] + ).T assert actual_labels.shape == expected_labels.shape for actual, expected in zip(actual_labels, expected_labels): assert np.all(actual == expected) @@ -971,7 +1030,16 @@ def test_featurize_trackers_keep_action_unlikely_intent_max_history_featurizer( for actual, expected in zip(actual_features, expected_features): assert compare_featurized_states(actual, expected) - expected_labels = np.array([[0, 9, 21, 0]]).T + expected_labels = np.array( + [ + [ + 0, + moodbot_domain.action_names_or_texts.index("action_unlikely_intent"), + moodbot_domain.action_names_or_texts.index("utter_greet"), + 0, + ] + ] + ).T assert actual_labels is not None assert actual_labels.shape == expected_labels.shape for actual, expected in zip(actual_labels, expected_labels): @@ -1088,7 +1156,19 @@ def test_deduplicate_featurize_trackers_with_max_history_tracker_featurizer( for actual, expected in zip(actual_features, expected_features): assert compare_featurized_states(actual, expected) - expected_labels = np.array([[0, 21, 0, 18, 19, 0, 20]]).T + expected_labels = np.array( + [ + [ + 0, + moodbot_domain.action_names_or_texts.index("utter_greet"), + 0, + moodbot_domain.action_names_or_texts.index("utter_cheer_up"), + moodbot_domain.action_names_or_texts.index("utter_did_that_help"), + 0, + moodbot_domain.action_names_or_texts.index("utter_goodbye"), + ] + ] + ).T if not remove_duplicates: expected_labels = np.vstack([expected_labels] * 2) From cba979186eea5d17c58b39b5bee4d993a59163f0 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 25 Sep 2023 10:30:36 +0200 Subject: [PATCH 06/20] Only update flow hashes when they changed, added slot to tests --- .../processor/command_processor.py | 8 ++++---- tests/core/test_actions.py | 13 +++++++++---- tests/test_server.py | 2 ++ 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/rasa/dialogue_understanding/processor/command_processor.py b/rasa/dialogue_understanding/processor/command_processor.py index fe1863f5ec7f..8f029b263484 100644 --- a/rasa/dialogue_understanding/processor/command_processor.py +++ b/rasa/dialogue_understanding/processor/command_processor.py @@ -153,10 +153,10 @@ def execute_commands( # Override commands commands = [CleanStackCommand()] - # always store current flow hashes - tracker.update_with_events( - [SlotSet(FLOW_HASHES_SLOT, calculate_flow_fingerprints(all_flows))], None - ) + # store current flow hashes if they changed + new_hashes = calculate_flow_fingerprints(all_flows) + if new_hashes != (tracker.get_slot(FLOW_HASHES_SLOT) or {}): + tracker.update_with_events([SlotSet(FLOW_HASHES_SLOT, new_hashes)], None) events: List[Event] = [] diff --git a/tests/core/test_actions.py b/tests/core/test_actions.py index c240d065f775..85eecd683aa8 100644 --- a/tests/core/test_actions.py +++ b/tests/core/test_actions.py @@ -97,6 +97,8 @@ ACTION_EXTRACT_SLOTS, DIALOGUE_STACK_SLOT, RETURN_VALUE_SLOT, + ACTION_CLEAN_STACK, + FLOW_HASHES_SLOT, ) from rasa.shared.core.trackers import DialogueStateTracker from rasa.shared.exceptions import RasaException @@ -146,7 +148,7 @@ def test_domain_action_instantiation(): for action_name in domain.action_names_or_texts ] - assert len(instantiated_actions) == 21 + assert len(instantiated_actions) == 22 assert instantiated_actions[0].name() == ACTION_LISTEN_NAME assert instantiated_actions[1].name() == ACTION_RESTART_NAME assert instantiated_actions[2].name() == ACTION_SESSION_START_NAME @@ -165,9 +167,10 @@ def test_domain_action_instantiation(): assert instantiated_actions[15].name() == ACTION_CORRECT_FLOW_SLOT assert instantiated_actions[16].name() == ACTION_CLARIFY_FLOWS assert instantiated_actions[17].name() == ACTION_RUN_SLOT_REJECTIONS_NAME - assert instantiated_actions[18].name() == "my_module.ActionTest" - assert instantiated_actions[19].name() == "utter_test" - assert instantiated_actions[20].name() == "utter_chitchat" + assert instantiated_actions[18].name() == ACTION_CLEAN_STACK + assert instantiated_actions[19].name() == "my_module.ActionTest" + assert instantiated_actions[20].name() == "utter_test" + assert instantiated_actions[21].name() == "utter_chitchat" @pytest.mark.parametrize( @@ -248,6 +251,7 @@ async def test_remote_action_runs( "slots": { "name": None, REQUESTED_SLOT: None, + FLOW_HASHES_SLOT: None, SESSION_START_METADATA_SLOT: None, DIALOGUE_STACK_SLOT: None, RETURN_VALUE_SLOT: None, @@ -312,6 +316,7 @@ async def test_remote_action_logs_events( "slots": { "name": None, REQUESTED_SLOT: None, + FLOW_HASHES_SLOT: None, SESSION_START_METADATA_SLOT: None, DIALOGUE_STACK_SLOT: None, RETURN_VALUE_SLOT: None, diff --git a/tests/test_server.py b/tests/test_server.py index 8467693ed24e..f7df6c69fb6f 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -57,6 +57,7 @@ SESSION_START_METADATA_SLOT, DIALOGUE_STACK_SLOT, RETURN_VALUE_SLOT, + FLOW_HASHES_SLOT, ) from rasa.shared.core.domain import Domain, SessionConfig from rasa.shared.core.events import ( @@ -1117,6 +1118,7 @@ async def test_requesting_non_existent_tracker(rasa_app: SanicASGITestClient): assert content["slots"] == { "name": None, REQUESTED_SLOT: None, + FLOW_HASHES_SLOT: None, SESSION_START_METADATA_SLOT: None, DIALOGUE_STACK_SLOT: None, RETURN_VALUE_SLOT: None, From 75bad3685e7a61722dc97e8a7ce8f57dbe267bc6 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 25 Sep 2023 10:49:11 +0200 Subject: [PATCH 07/20] Code quality, moved fixtures to conftest --- .../processor/command_processor.py | 2 +- tests/cdu/commands/conftest.py | 43 +++++++++++++++++++ tests/cdu/commands/test_command_processor.py | 38 +--------------- .../commands/test_stack_cleaning_command.py | 2 - 4 files changed, 45 insertions(+), 40 deletions(-) create mode 100644 tests/cdu/commands/conftest.py diff --git a/rasa/dialogue_understanding/processor/command_processor.py b/rasa/dialogue_understanding/processor/command_processor.py index 8f029b263484..a25e39c2d594 100644 --- a/rasa/dialogue_understanding/processor/command_processor.py +++ b/rasa/dialogue_understanding/processor/command_processor.py @@ -1,4 +1,4 @@ -from typing import List, Optional, Type, Set, Tuple, Dict +from typing import List, Optional, Type, Set, Dict import structlog from rasa.dialogue_understanding.commands import ( diff --git a/tests/cdu/commands/conftest.py b/tests/cdu/commands/conftest.py new file mode 100644 index 000000000000..9e7437149042 --- /dev/null +++ b/tests/cdu/commands/conftest.py @@ -0,0 +1,43 @@ +import pytest + +from rasa.dialogue_understanding.commands import StartFlowCommand +from rasa.dialogue_understanding.processor.command_processor import execute_commands +from rasa.shared.core.events import UserUttered +from rasa.shared.core.flows.flow import FlowsList +from rasa.shared.core.trackers import DialogueStateTracker +from rasa.shared.nlu.constants import COMMANDS +from tests.utilities import flows_from_str + + +@pytest.fixture +def all_flows() -> FlowsList: + return flows_from_str( + """ + flows: + foo: + steps: + - id: first_step + action: action_listen + bar: + steps: + - id: also_first_step + action: action_listen + """ + ) + + +start_foo_user_uttered = UserUttered( + "start foo", None, None, {COMMANDS: [StartFlowCommand("foo").as_dict()]} +) + +start_bar_user_uttered = UserUttered( + "start bar", None, None, {COMMANDS: [StartFlowCommand("bar").as_dict()]} +) + + +@pytest.fixture +def tracker(all_flows: FlowsList) -> DialogueStateTracker: + # Creates a useful tracker that has a started flow and the current flows hashed + tracker = DialogueStateTracker.from_events("test", evts=[start_foo_user_uttered]) + execute_commands(tracker, all_flows) + return tracker diff --git a/tests/cdu/commands/test_command_processor.py b/tests/cdu/commands/test_command_processor.py index 1d3315c3b7b9..8b54056ccaf4 100644 --- a/tests/cdu/commands/test_command_processor.py +++ b/tests/cdu/commands/test_command_processor.py @@ -1,6 +1,5 @@ import pytest -from rasa.dialogue_understanding.commands import StartFlowCommand from rasa.dialogue_understanding.processor.command_processor import ( execute_commands, find_updated_flows, @@ -11,47 +10,12 @@ PatternFlowStackFrame, ) from rasa.shared.core.constants import FLOW_HASHES_SLOT -from rasa.shared.core.events import UserUttered from rasa.shared.core.flows.flow import FlowsList from rasa.shared.core.trackers import DialogueStateTracker -from rasa.shared.nlu.constants import COMMANDS +from tests.cdu.commands.conftest import start_bar_user_uttered from tests.utilities import flows_from_str -@pytest.fixture -def all_flows() -> FlowsList: - return flows_from_str( - """ - flows: - foo: - steps: - - id: first_step - action: action_listen - bar: - steps: - - id: also_first_step - action: action_listen - """ - ) - - -start_foo_user_uttered = UserUttered( - "start foo", None, None, {COMMANDS: [StartFlowCommand("foo").as_dict()]} -) - -start_bar_user_uttered = UserUttered( - "start bar", None, None, {COMMANDS: [StartFlowCommand("bar").as_dict()]} -) - - -@pytest.fixture -def tracker(all_flows: FlowsList) -> DialogueStateTracker: - # Creates a useful tracker that has a started flow and the current flows hashed - tracker = DialogueStateTracker.from_events("test", evts=[start_foo_user_uttered]) - execute_commands(tracker, all_flows) - return tracker - - def test_properly_prepared_tracker(tracker: DialogueStateTracker): # flow hashes have been initialized assert "foo" in tracker.get_slot(FLOW_HASHES_SLOT) diff --git a/tests/cdu/commands/test_stack_cleaning_command.py b/tests/cdu/commands/test_stack_cleaning_command.py index d10bc553b42f..b3881e8f1f16 100644 --- a/tests/cdu/commands/test_stack_cleaning_command.py +++ b/tests/cdu/commands/test_stack_cleaning_command.py @@ -20,8 +20,6 @@ ) from rasa.shared.core.trackers import DialogueStateTracker from tests.cdu.commands.test_command_processor import ( - tracker, - all_flows, start_bar_user_uttered, change_cases, ) From 2cf529a1138f5ee51444d7be82408358106cf9f8 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 25 Sep 2023 10:56:33 +0200 Subject: [PATCH 08/20] removed unused imports --- rasa/shared/core/flows/flow.py | 1 - tests/core/featurizers/test_tracker_featurizer.py | 1 - 2 files changed, 2 deletions(-) diff --git a/rasa/shared/core/flows/flow.py b/rasa/shared/core/flows/flow.py index 2f32665fb0a5..7528b55df0f0 100644 --- a/rasa/shared/core/flows/flow.py +++ b/rasa/shared/core/flows/flow.py @@ -1,6 +1,5 @@ from __future__ import annotations -import functools from dataclasses import dataclass from enum import Enum from typing import Any, Dict, List, Optional, Protocol, Set, Text, runtime_checkable diff --git a/tests/core/featurizers/test_tracker_featurizer.py b/tests/core/featurizers/test_tracker_featurizer.py index 6b55f5335a3a..2066a65d6cf4 100644 --- a/tests/core/featurizers/test_tracker_featurizer.py +++ b/tests/core/featurizers/test_tracker_featurizer.py @@ -22,7 +22,6 @@ ACTION_UNLIKELY_INTENT_NAME, USER, PREVIOUS_ACTION, - DEFAULT_ACTION_NAMES, ) from rasa.shared.core.events import ActionExecuted from rasa.shared.core.trackers import DialogueStateTracker From 416be840efcb2b89c7e304a222d32f7576946911 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 25 Sep 2023 11:05:18 +0200 Subject: [PATCH 09/20] Added explicit typing --- rasa/dialogue_understanding/processor/command_processor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rasa/dialogue_understanding/processor/command_processor.py b/rasa/dialogue_understanding/processor/command_processor.py index a25e39c2d594..2efb21fc11e9 100644 --- a/rasa/dialogue_understanding/processor/command_processor.py +++ b/rasa/dialogue_understanding/processor/command_processor.py @@ -110,7 +110,7 @@ def find_updated_flows(tracker: DialogueStateTracker, all_flows: FlowsList) -> S Returns: A set of flow ids of those flows that have changed """ - stored_fingerprints = tracker.get_slot(FLOW_HASHES_SLOT) + stored_fingerprints: Dict[str, str] = tracker.get_slot(FLOW_HASHES_SLOT) or {} dialogue_stack = DialogueStack.from_tracker(tracker) changed_flows = set() From 68af0a34624c71c5cda555e3c27c178a8e38a195 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 25 Sep 2023 13:12:02 +0200 Subject: [PATCH 10/20] improved tests --- data/test_trackers/tracker_moodbot.json | 1 + rasa/core/actions/action.py | 5 ++++- rasa/shared/core/constants.py | 9 ++++++--- rasa/shared/core/domain.py | 9 ++------- tests/shared/core/test_domain.py | 9 +++------ .../story_reader/test_yaml_story_reader.py | 13 +++++++++---- tests/shared/importers/test_rasa.py | 14 ++++++++++---- 7 files changed, 35 insertions(+), 25 deletions(-) diff --git a/data/test_trackers/tracker_moodbot.json b/data/test_trackers/tracker_moodbot.json index acdcac89a5cf..7fc6db7830d7 100644 --- a/data/test_trackers/tracker_moodbot.json +++ b/data/test_trackers/tracker_moodbot.json @@ -34,6 +34,7 @@ "followup_action": null, "slots": { "dialogue_stack": null, + "flow_hashes": null, "name": null, "requested_slot": null, "return_value": null, diff --git a/rasa/core/actions/action.py b/rasa/core/actions/action.py index 28258cfab877..dee8d54ccf25 100644 --- a/rasa/core/actions/action.py +++ b/rasa/core/actions/action.py @@ -57,6 +57,7 @@ ACTION_VALIDATE_SLOT_MAPPINGS, MAPPING_TYPE, SlotMappingType, + KNOWLEDGE_BASE_SLOT_NAMES, ) from rasa.shared.core.domain import Domain from rasa.shared.core.events import ( @@ -1292,7 +1293,9 @@ async def run( executed_custom_actions: Set[Text] = set() user_slots = [ - slot for slot in domain.slots if slot.name not in DEFAULT_SLOT_NAMES + slot + for slot in domain.slots + if slot.name not in DEFAULT_SLOT_NAMES | KNOWLEDGE_BASE_SLOT_NAMES ] for slot in user_slots: diff --git a/rasa/shared/core/constants.py b/rasa/shared/core/constants.py index f9991621b2a9..412722d9d055 100644 --- a/rasa/shared/core/constants.py +++ b/rasa/shared/core/constants.py @@ -101,13 +101,16 @@ SLOT_LAST_OBJECT_TYPE = "knowledge_base_last_object_type" DEFAULT_KNOWLEDGE_BASE_ACTION = "action_query_knowledge_base" +KNOWLEDGE_BASE_SLOT_NAMES = { + SLOT_LISTED_ITEMS, + SLOT_LAST_OBJECT, + SLOT_LAST_OBJECT_TYPE, +} + DEFAULT_SLOT_NAMES = { REQUESTED_SLOT, DIALOGUE_STACK_SLOT, SESSION_START_METADATA_SLOT, - SLOT_LISTED_ITEMS, - SLOT_LAST_OBJECT, - SLOT_LAST_OBJECT_TYPE, RETURN_VALUE_SLOT, FLOW_HASHES_SLOT, } diff --git a/rasa/shared/core/domain.py b/rasa/shared/core/domain.py index c105926a09b2..d697c3842a27 100644 --- a/rasa/shared/core/domain.py +++ b/rasa/shared/core/domain.py @@ -37,12 +37,12 @@ IGNORED_INTENTS, RESPONSE_CONDITION, ) -import rasa.shared.core.constants from rasa.shared.core.constants import ( ACTION_SHOULD_SEND_DOMAIN, SlotMappingType, MAPPING_TYPE, MAPPING_CONDITIONS, + KNOWLEDGE_BASE_SLOT_NAMES, ) from rasa.shared.exceptions import ( RasaException, @@ -1030,12 +1030,7 @@ def _add_knowledge_base_slots(self) -> None: ) ) slot_names = [slot.name for slot in self.slots] - knowledge_base_slots = [ - rasa.shared.core.constants.SLOT_LISTED_ITEMS, - rasa.shared.core.constants.SLOT_LAST_OBJECT, - rasa.shared.core.constants.SLOT_LAST_OBJECT_TYPE, - ] - for slot in knowledge_base_slots: + for slot in KNOWLEDGE_BASE_SLOT_NAMES: if slot not in slot_names: self.slots.append( TextSlot(slot, mappings=[], influence_conversation=False) diff --git a/tests/shared/core/test_domain.py b/tests/shared/core/test_domain.py index b70e1dc5b3a8..5e1d1e4ced66 100644 --- a/tests/shared/core/test_domain.py +++ b/tests/shared/core/test_domain.py @@ -27,6 +27,7 @@ DEFAULT_KNOWLEDGE_BASE_ACTION, ENTITY_LABEL_SEPARATOR, DEFAULT_ACTION_NAMES, + DEFAULT_SLOT_NAMES, ) from rasa.shared.core.domain import ( InvalidDomain, @@ -888,10 +889,9 @@ def test_domain_from_multiple_files(): "utter_default": [{"text": "default message"}], "utter_amazement": [{"text": "awesomness!"}], } - expected_slots = [ + expected_slots = list(DEFAULT_SLOT_NAMES) + [ "activate_double_simulation", "activate_simulation", - "dialogue_stack", "display_cure_method", "display_drum_cure_horns", "display_method_artwork", @@ -914,9 +914,6 @@ def test_domain_from_multiple_files(): "humbleSelectionManagement", "humbleSelectionStatus", "offers", - "requested_slot", - "return_value", - "session_started_metadata", ] domain_slots = [] @@ -930,7 +927,7 @@ def test_domain_from_multiple_files(): assert expected_responses == domain.responses assert expected_forms == domain.forms assert domain.session_config.session_expiration_time == 360 - assert expected_slots == sorted(domain_slots) + assert sorted(expected_slots) == sorted(domain_slots) def test_domain_warnings(domain: Domain): diff --git a/tests/shared/core/training_data/story_reader/test_yaml_story_reader.py b/tests/shared/core/training_data/story_reader/test_yaml_story_reader.py index 1bbb08bdd7d0..c51322b8065e 100644 --- a/tests/shared/core/training_data/story_reader/test_yaml_story_reader.py +++ b/tests/shared/core/training_data/story_reader/test_yaml_story_reader.py @@ -775,12 +775,17 @@ def test_generate_training_data_with_cycles(domain: Domain): # deterministic way but should always be 3 or 4 assert len(training_trackers) == 3 or len(training_trackers) == 4 - # if we have 4 trackers, there is going to be one example more for label 10 - num_tens = len(training_trackers) - 1 - # if new default actions are added the keys of the actions will be changed + # if we have 4 trackers, there is going to be one example more for utter_default + num_utter_default = len(training_trackers) - 1 all_label_ids = [id for ids in label_ids for id in ids] - assert Counter(all_label_ids) == {0: 6, 20: 3, 19: num_tens, 1: 2, 21: 1} + assert Counter(all_label_ids) == { + 0: 6, + domain.action_names_or_texts.index("utter_goodbye"): 3, + domain.action_names_or_texts.index("utter_default"): num_utter_default, + 1: 2, + domain.action_names_or_texts.index("utter_greet"): 1, + } def test_generate_training_data_with_unused_checkpoints(domain: Domain): diff --git a/tests/shared/importers/test_rasa.py b/tests/shared/importers/test_rasa.py index 88025312edf9..a4732aebf68c 100644 --- a/tests/shared/importers/test_rasa.py +++ b/tests/shared/importers/test_rasa.py @@ -14,6 +14,9 @@ SESSION_START_METADATA_SLOT, DIALOGUE_STACK_SLOT, RETURN_VALUE_SLOT, + FLOW_HASHES_SLOT, + DEFAULT_SLOT_NAMES, + REQUESTED_SLOT, ) from rasa.shared.core.domain import Domain from rasa.shared.core.slots import AnySlot @@ -30,11 +33,14 @@ def test_rasa_file_importer(project: Text): domain = importer.get_domain() assert len(domain.intents) == 7 + len(DEFAULT_INTENTS) - assert domain.slots == [ - AnySlot(DIALOGUE_STACK_SLOT, mappings=[{}]), - AnySlot(RETURN_VALUE_SLOT, mappings=[{}]), - AnySlot(SESSION_START_METADATA_SLOT, mappings=[{}]), + default_slots = [ + AnySlot(slot_name, mappings=[{}]) + for slot_name in DEFAULT_SLOT_NAMES + if slot_name != REQUESTED_SLOT ] + assert sorted(domain.slots, key=lambda s: s.name) == sorted( + default_slots, key=lambda s: s.name + ) assert domain.entities == [] assert len(domain.action_names_or_texts) == 6 + len(DEFAULT_ACTION_NAMES) From cc7e1a05ebce1e951ed2a546440a79a2a227f33d Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 25 Sep 2023 13:37:07 +0200 Subject: [PATCH 11/20] removed unused imports --- tests/shared/importers/test_rasa.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/shared/importers/test_rasa.py b/tests/shared/importers/test_rasa.py index a4732aebf68c..a739f1c71c97 100644 --- a/tests/shared/importers/test_rasa.py +++ b/tests/shared/importers/test_rasa.py @@ -11,10 +11,6 @@ from rasa.shared.core.constants import ( DEFAULT_ACTION_NAMES, DEFAULT_INTENTS, - SESSION_START_METADATA_SLOT, - DIALOGUE_STACK_SLOT, - RETURN_VALUE_SLOT, - FLOW_HASHES_SLOT, DEFAULT_SLOT_NAMES, REQUESTED_SLOT, ) From b03c781f6fe9de6611b2cae0268ba50851541c3c Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Thu, 28 Sep 2023 09:00:53 +0200 Subject: [PATCH 12/20] Update rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml Co-authored-by: Tom Bocklisch --- .../patterns/default_flows_for_patterns.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml b/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml index 654ce23135b7..b304ad21aed1 100644 --- a/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml +++ b/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml @@ -137,4 +137,4 @@ flows: action: utter_inform_stack_cleanup next: "run_cleanup" - id: "run_cleanup" - action: action_clean_stack \ No newline at end of file + action: action_clean_stack From 17d9ff688b0f5a0d6f500b028fc5efc91b0b2eaf Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Thu, 28 Sep 2023 09:01:24 +0200 Subject: [PATCH 13/20] Update rasa/dialogue_understanding/patterns/clean_stack.py Co-authored-by: Tom Bocklisch --- rasa/dialogue_understanding/patterns/clean_stack.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rasa/dialogue_understanding/patterns/clean_stack.py b/rasa/dialogue_understanding/patterns/clean_stack.py index ff7b744801c6..12a9e5794a77 100644 --- a/rasa/dialogue_understanding/patterns/clean_stack.py +++ b/rasa/dialogue_understanding/patterns/clean_stack.py @@ -39,7 +39,7 @@ class CleanStackFlowStackFrame(PatternFlowStackFrame): @classmethod def type(cls) -> str: """Returns the type of the frame.""" - return "pattern_clean_stack" + return FLOW_PATTERN_CLEAN_STACK_ID @staticmethod def from_dict(data: Dict[str, Any]) -> CleanStackFlowStackFrame: From c9fff34f4aa579d2b595d06b25c56553ce81b0ba Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Thu, 28 Sep 2023 09:08:07 +0200 Subject: [PATCH 14/20] applying and returning flow hash events --- rasa/dialogue_understanding/processor/command_processor.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rasa/dialogue_understanding/processor/command_processor.py b/rasa/dialogue_understanding/processor/command_processor.py index b85e657f26ea..eff191eb3303 100644 --- a/rasa/dialogue_understanding/processor/command_processor.py +++ b/rasa/dialogue_understanding/processor/command_processor.py @@ -155,10 +155,12 @@ def execute_commands( # store current flow hashes if they changed new_hashes = calculate_flow_fingerprints(all_flows) + flow_hash_events = [] if new_hashes != (tracker.get_slot(FLOW_HASHES_SLOT) or {}): - tracker.update_with_events([SlotSet(FLOW_HASHES_SLOT, new_hashes)], None) + flow_hash_events.append(SlotSet(FLOW_HASHES_SLOT, new_hashes)) + tracker.update_with_events(flow_hash_events, None) - events: List[Event] = [] + events: List[Event] = flow_hash_events # commands need to be reversed to make sure they end up in the right order # on the stack. e.g. if there multiple start flow commands, the first one From e17c6032fefb7656e32c16dddbae59c6d4a35a70 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Thu, 28 Sep 2023 09:11:11 +0200 Subject: [PATCH 15/20] added logging call for when flows were updated --- rasa/dialogue_understanding/processor/command_processor.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rasa/dialogue_understanding/processor/command_processor.py b/rasa/dialogue_understanding/processor/command_processor.py index eff191eb3303..ecc4e47c2135 100644 --- a/rasa/dialogue_understanding/processor/command_processor.py +++ b/rasa/dialogue_understanding/processor/command_processor.py @@ -151,6 +151,10 @@ def execute_commands( updated_flows = find_updated_flows(tracker, all_flows) if updated_flows: # Override commands + structlogger.debug( + "command_executor.running_flows_were_updated", + updated_flow_ids=updated_flows, + ) commands = [CleanStackCommand()] # store current flow hashes if they changed From d7857d03df2bc765575c32fb1c954cee4296c0e1 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Thu, 28 Sep 2023 09:14:53 +0200 Subject: [PATCH 16/20] made fingerprint of flows a cached property --- .../processor/command_processor.py | 4 ++-- rasa/shared/core/flows/flow.py | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/rasa/dialogue_understanding/processor/command_processor.py b/rasa/dialogue_understanding/processor/command_processor.py index ecc4e47c2135..66cacdb72ac2 100644 --- a/rasa/dialogue_understanding/processor/command_processor.py +++ b/rasa/dialogue_understanding/processor/command_processor.py @@ -119,7 +119,7 @@ def find_updated_flows(tracker: DialogueStateTracker, all_flows: FlowsList) -> S flow = all_flows.flow_by_id(frame.flow_id) if flow is None or ( flow.id in stored_fingerprints - and flow.fingerprint() != stored_fingerprints[flow.id] + and flow.fingerprint != stored_fingerprints[flow.id] ): changed_flows.add(frame.flow_id) return changed_flows @@ -127,7 +127,7 @@ def find_updated_flows(tracker: DialogueStateTracker, all_flows: FlowsList) -> S def calculate_flow_fingerprints(all_flows: FlowsList) -> Dict[str, str]: """Calculate fingerprints for all flows.""" - return {flow.id: flow.fingerprint() for flow in all_flows.underlying_flows} + return {flow.id: flow.fingerprint for flow in all_flows.underlying_flows} def execute_commands( diff --git a/rasa/shared/core/flows/flow.py b/rasa/shared/core/flows/flow.py index 63da8b617750..e6de628e229f 100644 --- a/rasa/shared/core/flows/flow.py +++ b/rasa/shared/core/flows/flow.py @@ -1,6 +1,7 @@ from __future__ import annotations from dataclasses import dataclass +from functools import cached_property from typing import ( Any, Dict, @@ -535,6 +536,11 @@ def steps(self) -> List[FlowStep]: """Returns the steps of the flow.""" return self.step_sequence.steps + @cached_property + def fingerprint(self) -> str: + """Create a fingerprint identifying this step sequence.""" + return rasa.shared.utils.io.deep_container_fingerprint(self.as_json()) + @dataclass class StepSequence: @@ -582,10 +588,6 @@ def first(self) -> Optional[FlowStep]: return None return self.child_steps[0] - def fingerprint(self) -> str: - """Create a fingerprint identifying this flow.""" - return rasa.shared.utils.io.deep_container_fingerprint(self.as_json()) - def step_from_json(flow_step_config: Dict[Text, Any]) -> FlowStep: """Used to read flow steps from parsed YAML. From 7a629f9bc7b0d7d070d0f2aa4699276b641f315f Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Thu, 28 Sep 2023 10:14:40 +0200 Subject: [PATCH 17/20] restructured command, pattern, and action to be more modular --- rasa/core/actions/action_clean_stack.py | 50 ++++++++++ ...mmand.py => handle_code_change_command.py} | 16 ++-- .../patterns/clean_stack.py | 96 ------------------- .../patterns/code_change.py | 42 ++++++++ .../patterns/default_flows_for_patterns.yml | 6 +- .../processor/command_processor.py | 6 +- tests/cdu/commands/test_command_processor.py | 3 +- ....py => test_handle_code_change_command.py} | 18 ++-- 8 files changed, 120 insertions(+), 117 deletions(-) create mode 100644 rasa/core/actions/action_clean_stack.py rename rasa/dialogue_understanding/commands/{clean_stack_command.py => handle_code_change_command.py} (79%) delete mode 100644 rasa/dialogue_understanding/patterns/clean_stack.py create mode 100644 rasa/dialogue_understanding/patterns/code_change.py rename tests/cdu/commands/{test_stack_cleaning_command.py => test_handle_code_change_command.py} (85%) diff --git a/rasa/core/actions/action_clean_stack.py b/rasa/core/actions/action_clean_stack.py new file mode 100644 index 000000000000..37a7161af43b --- /dev/null +++ b/rasa/core/actions/action_clean_stack.py @@ -0,0 +1,50 @@ +from __future__ import annotations + +from typing import Optional, Dict, Any, List + +from rasa.core.actions import action +from rasa.core.channels import OutputChannel +from rasa.core.nlg import NaturalLanguageGenerator +from rasa.dialogue_understanding.stack.dialogue_stack import DialogueStack +from rasa.dialogue_understanding.stack.frames import ( + BaseFlowStackFrame, + UserFlowStackFrame, +) +from rasa.dialogue_understanding.stack.frames.flow_stack_frame import FlowStackFrameType +from rasa.shared.core.constants import ACTION_CLEAN_STACK, DIALOGUE_STACK_SLOT +from rasa.shared.core.domain import Domain +from rasa.shared.core.events import Event, SlotSet +from rasa.shared.core.flows.flow import ContinueFlowStep, END_STEP +from rasa.shared.core.trackers import DialogueStateTracker + + +class ActionCleanStack(action.Action): + """Action which cancels a flow from the stack.""" + + def name(self) -> str: + """Return the flow name.""" + return ACTION_CLEAN_STACK + + async def run( + self, + output_channel: OutputChannel, + nlg: NaturalLanguageGenerator, + tracker: DialogueStateTracker, + domain: Domain, + metadata: Optional[Dict[str, Any]] = None, + ) -> List[Event]: + """Clean the stack.""" + stack = DialogueStack.from_tracker(tracker) + + new_frames = [] + # Set all frames to their end step, filter out any non-BaseFlowStackFrames + for frame in stack.frames: + if isinstance(frame, BaseFlowStackFrame): + frame.step_id = ContinueFlowStep.continue_step_for_id(END_STEP) + if isinstance(frame, UserFlowStackFrame): + # Making sure there are no "continue interrupts" triggered + frame.frame_type = FlowStackFrameType.REGULAR + new_frames.append(frame) + new_stack = DialogueStack.from_dict([frame.as_dict() for frame in new_frames]) + + return [SlotSet(DIALOGUE_STACK_SLOT, new_stack.as_dict())] diff --git a/rasa/dialogue_understanding/commands/clean_stack_command.py b/rasa/dialogue_understanding/commands/handle_code_change_command.py similarity index 79% rename from rasa/dialogue_understanding/commands/clean_stack_command.py rename to rasa/dialogue_understanding/commands/handle_code_change_command.py index fa62f5e104ae..c54bce685f17 100644 --- a/rasa/dialogue_understanding/commands/clean_stack_command.py +++ b/rasa/dialogue_understanding/commands/handle_code_change_command.py @@ -6,7 +6,7 @@ import structlog from rasa.dialogue_understanding.commands import Command -from rasa.dialogue_understanding.patterns.clean_stack import CleanStackFlowStackFrame +from rasa.dialogue_understanding.patterns.code_change import CodeChangeFlowStackFrame from rasa.dialogue_understanding.stack.dialogue_stack import DialogueStack from rasa.shared.core.constants import DIALOGUE_STACK_SLOT from rasa.shared.core.events import Event, SlotSet @@ -18,22 +18,22 @@ @dataclass -class CleanStackCommand(Command): - """A command to cancel the current flow.""" +class HandleCodeChangeCommand(Command): + """A that is executed when the flows have changed.""" @classmethod def command(cls) -> str: """Returns the command type.""" - return "clean stack" + return "handle code change" @classmethod - def from_dict(cls, data: Dict[str, Any]) -> CleanStackCommand: + def from_dict(cls, data: Dict[str, Any]) -> HandleCodeChangeCommand: """Converts the dictionary to a command. Returns: The converted dictionary. """ - return CleanStackCommand() + return HandleCodeChangeCommand() def run_command_on_tracker( self, @@ -59,9 +59,9 @@ def run_command_on_tracker( if not current_flow: structlogger.debug( - "command_executor.skip_clean_stack.no_active_flow", command=self + "handle_code_change_command.skip.no_active_flow", command=self ) return [] - stack.push(CleanStackFlowStackFrame()) + stack.push(CodeChangeFlowStackFrame()) return [SlotSet(DIALOGUE_STACK_SLOT, stack.as_dict())] diff --git a/rasa/dialogue_understanding/patterns/clean_stack.py b/rasa/dialogue_understanding/patterns/clean_stack.py deleted file mode 100644 index 12a9e5794a77..000000000000 --- a/rasa/dialogue_understanding/patterns/clean_stack.py +++ /dev/null @@ -1,96 +0,0 @@ -from __future__ import annotations - -from dataclasses import dataclass -from typing import Any, Dict, List, Optional - -import structlog -from rasa.dialogue_understanding.stack.dialogue_stack import ( - DialogueStack, -) -from rasa.dialogue_understanding.stack.frames import ( - PatternFlowStackFrame, - BaseFlowStackFrame, - UserFlowStackFrame, -) -from rasa.core.actions import action -from rasa.core.channels.channel import OutputChannel -from rasa.core.nlg.generator import NaturalLanguageGenerator -from rasa.dialogue_understanding.stack.frames.flow_stack_frame import FlowStackFrameType -from rasa.shared.constants import RASA_DEFAULT_FLOW_PATTERN_PREFIX -from rasa.shared.core.constants import DIALOGUE_STACK_SLOT, ACTION_CLEAN_STACK -from rasa.shared.core.domain import Domain -from rasa.shared.core.events import Event, SlotSet -from rasa.shared.core.flows.flow import END_STEP, ContinueFlowStep -from rasa.shared.core.trackers import DialogueStateTracker - - -structlogger = structlog.get_logger() - -FLOW_PATTERN_CLEAN_STACK_ID = RASA_DEFAULT_FLOW_PATTERN_PREFIX + "clean_stack" - - -@dataclass -class CleanStackFlowStackFrame(PatternFlowStackFrame): - """A pattern flow stack frame which cleans the stack after a bot update.""" - - flow_id: str = FLOW_PATTERN_CLEAN_STACK_ID - """The ID of the flow.""" - - @classmethod - def type(cls) -> str: - """Returns the type of the frame.""" - return FLOW_PATTERN_CLEAN_STACK_ID - - @staticmethod - def from_dict(data: Dict[str, Any]) -> CleanStackFlowStackFrame: - """Creates a `DialogueStackFrame` from a dictionary. - - Args: - data: The dictionary to create the `DialogueStackFrame` from. - - Returns: - The created `DialogueStackFrame`. - """ - return CleanStackFlowStackFrame( - frame_id=data["frame_id"], - step_id=data["step_id"], - ) - - -class ActionCleanStack(action.Action): - """Action which cancels a flow from the stack.""" - - def name(self) -> str: - """Return the flow name.""" - return ACTION_CLEAN_STACK - - async def run( - self, - output_channel: OutputChannel, - nlg: NaturalLanguageGenerator, - tracker: DialogueStateTracker, - domain: Domain, - metadata: Optional[Dict[str, Any]] = None, - ) -> List[Event]: - """Clean the stack.""" - stack = DialogueStack.from_tracker(tracker) - if not (top := stack.top()): - structlogger.warning("action.clean_stack.no_active_flow") - return [] - - if not isinstance(top, CleanStackFlowStackFrame): - structlogger.warning("action.clean_stack.no_cleaning_frame", top=top) - return [] - - new_frames = [] - # Set all frames to their end step, filter out any non-BaseFlowStackFrames - for frame in stack.frames: - if isinstance(frame, BaseFlowStackFrame): - frame.step_id = ContinueFlowStep.continue_step_for_id(END_STEP) - if isinstance(frame, UserFlowStackFrame): - # Making sure there are no "continue interrupts" triggered - frame.frame_type = FlowStackFrameType.REGULAR - new_frames.append(frame) - new_stack = DialogueStack.from_dict([frame.as_dict() for frame in new_frames]) - - return [SlotSet(DIALOGUE_STACK_SLOT, new_stack.as_dict())] diff --git a/rasa/dialogue_understanding/patterns/code_change.py b/rasa/dialogue_understanding/patterns/code_change.py new file mode 100644 index 000000000000..4a0ebf12ebeb --- /dev/null +++ b/rasa/dialogue_understanding/patterns/code_change.py @@ -0,0 +1,42 @@ +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any, Dict + +import structlog +from rasa.dialogue_understanding.stack.frames import ( + PatternFlowStackFrame, +) +from rasa.shared.constants import RASA_DEFAULT_FLOW_PATTERN_PREFIX + +structlogger = structlog.get_logger() + +FLOW_PATTERN_CODE_CHANGE_ID = RASA_DEFAULT_FLOW_PATTERN_PREFIX + "code_change" + + +@dataclass +class CodeChangeFlowStackFrame(PatternFlowStackFrame): + """A pattern flow stack frame which cleans the stack after a bot update.""" + + flow_id: str = FLOW_PATTERN_CODE_CHANGE_ID + """The ID of the flow.""" + + @classmethod + def type(cls) -> str: + """Returns the type of the frame.""" + return FLOW_PATTERN_CODE_CHANGE_ID + + @staticmethod + def from_dict(data: Dict[str, Any]) -> CodeChangeFlowStackFrame: + """Creates a `DialogueStackFrame` from a dictionary. + + Args: + data: The dictionary to create the `DialogueStackFrame` from. + + Returns: + The created `DialogueStackFrame`. + """ + return CodeChangeFlowStackFrame( + frame_id=data["frame_id"], + step_id=data["step_id"], + ) diff --git a/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml b/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml index b304ad21aed1..a8b62d8c15e7 100644 --- a/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml +++ b/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml @@ -32,7 +32,7 @@ responses: rephrase: True template: jinja - utter_inform_stack_cleanup_rasa: + utter_inform_code_change: - text: There has been an update to my code. I need to wrap up our running dialogue tasks and start from scratch. metadata: rephrase: True @@ -130,11 +130,11 @@ flows: next: "start" - id: "done" - pattern_clean_stack: + pattern_code_change: description: flow used to clean the stack after a bot update steps: - id: "inform_user" - action: utter_inform_stack_cleanup + action: utter_inform_code_change next: "run_cleanup" - id: "run_cleanup" action: action_clean_stack diff --git a/rasa/dialogue_understanding/processor/command_processor.py b/rasa/dialogue_understanding/processor/command_processor.py index 66cacdb72ac2..8e17f5e14c5c 100644 --- a/rasa/dialogue_understanding/processor/command_processor.py +++ b/rasa/dialogue_understanding/processor/command_processor.py @@ -9,7 +9,9 @@ SetSlotCommand, FreeFormAnswerCommand, ) -from rasa.dialogue_understanding.commands.clean_stack_command import CleanStackCommand +from rasa.dialogue_understanding.commands.handle_code_change_command import ( + HandleCodeChangeCommand, +) from rasa.dialogue_understanding.patterns.collect_information import ( CollectInformationPatternFlowStackFrame, ) @@ -155,7 +157,7 @@ def execute_commands( "command_executor.running_flows_were_updated", updated_flow_ids=updated_flows, ) - commands = [CleanStackCommand()] + commands = [HandleCodeChangeCommand()] # store current flow hashes if they changed new_hashes = calculate_flow_fingerprints(all_flows) diff --git a/tests/cdu/commands/test_command_processor.py b/tests/cdu/commands/test_command_processor.py index 8b54056ccaf4..e1b32d1b6c2b 100644 --- a/tests/cdu/commands/test_command_processor.py +++ b/tests/cdu/commands/test_command_processor.py @@ -1,5 +1,6 @@ import pytest +from rasa.dialogue_understanding.patterns.code_change import FLOW_PATTERN_CODE_CHANGE_ID from rasa.dialogue_understanding.processor.command_processor import ( execute_commands, find_updated_flows, @@ -124,4 +125,4 @@ def test_stack_cleaning_command_is_applied_on_changes(tracker: DialogueStateTrac assert len(dialogue_stack.frames) == 2 assert (top_frame := dialogue_stack.top()) assert isinstance(top_frame, PatternFlowStackFrame) - assert top_frame.flow_id == "pattern_clean_stack" + assert top_frame.flow_id == FLOW_PATTERN_CODE_CHANGE_ID diff --git a/tests/cdu/commands/test_stack_cleaning_command.py b/tests/cdu/commands/test_handle_code_change_command.py similarity index 85% rename from tests/cdu/commands/test_stack_cleaning_command.py rename to tests/cdu/commands/test_handle_code_change_command.py index b3881e8f1f16..f6a6afc2d06e 100644 --- a/tests/cdu/commands/test_stack_cleaning_command.py +++ b/tests/cdu/commands/test_handle_code_change_command.py @@ -2,8 +2,12 @@ from rasa.core.channels import CollectingOutputChannel from rasa.core.nlg import TemplatedNaturalLanguageGenerator -from rasa.dialogue_understanding.commands.clean_stack_command import CleanStackCommand -from rasa.dialogue_understanding.patterns.clean_stack import ActionCleanStack +from rasa.dialogue_understanding.commands.handle_code_change_command import ( + HandleCodeChangeCommand, +) +from rasa.core.actions.action_clean_stack import ActionCleanStack + +from rasa.dialogue_understanding.patterns.code_change import FLOW_PATTERN_CODE_CHANGE_ID from rasa.dialogue_understanding.processor.command_processor import execute_commands from rasa.dialogue_understanding.stack.dialogue_stack import DialogueStack from rasa.dialogue_understanding.stack.frames import ( @@ -29,15 +33,15 @@ def test_name_of_command(): # names of commands should not change as they are part of persisted # trackers - assert CleanStackCommand.command() == "clean stack" + assert HandleCodeChangeCommand.command() == "clean stack" def test_from_dict(): - assert CleanStackCommand.from_dict({}) == CleanStackCommand() + assert HandleCodeChangeCommand.from_dict({}) == HandleCodeChangeCommand() def test_run_command_on_tracker(tracker: DialogueStateTracker, all_flows: FlowsList): - command = CleanStackCommand() + command = HandleCodeChangeCommand() events = command.run_command_on_tracker(tracker, all_flows, tracker) assert len(events) == 1 dialogue_stack_event = events[0] @@ -46,7 +50,7 @@ def test_run_command_on_tracker(tracker: DialogueStateTracker, all_flows: FlowsL assert len(dialogue_stack_event.value) == 2 frame = dialogue_stack_event.value[1] - assert frame["type"] == "pattern_clean_stack" + assert frame["type"] == FLOW_PATTERN_CODE_CHANGE_ID @pytest.fixture @@ -70,7 +74,7 @@ def about_to_be_cleaned_tracker(tracker: DialogueStateTracker, all_flows: FlowsL stack_clean_frame = dialogue_stack.frames[2] assert isinstance(stack_clean_frame, PatternFlowStackFrame) - assert stack_clean_frame.flow_id == "pattern_clean_stack" + assert stack_clean_frame.flow_id == FLOW_PATTERN_CODE_CHANGE_ID assert stack_clean_frame.step_id == START_STEP return tracker From 586e1c7d9f1a4e2fb62c653b2cb996bdcad4e3bd Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Thu, 28 Sep 2023 10:23:15 +0200 Subject: [PATCH 18/20] adjusted flow texts to reflect conversation designer preferences --- .../patterns/default_flows_for_patterns.yml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml b/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml index a8b62d8c15e7..1a49392e27f9 100644 --- a/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml +++ b/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml @@ -1,39 +1,39 @@ version: "3.1" responses: utter_flow_continue_interrupted: - - text: Let's continue with the topic {{ context.previous_flow_name }}. + - text: "Let's continue with {{ context.previous_flow_name }}." metadata: rephrase: True template: jinja utter_corrected_previous_input: - - text: "Ok, I corrected the {{ context.corrected_slots.keys()|join(', ') }}." + - text: "Ok, I am updating {{ context.corrected_slots.keys()|join(', ') }} to {{ context.corrected_slots.values()|join(', ') respectively." metadata: rephrase: True template: jinja utter_flow_cancelled_rasa: - - text: Okay, stopping the flow {{ context.canceled_name }}. + - text: "Okay, stopping {{ context.canceled_name }}." metadata: rephrase: True template: jinja utter_can_do_something_else: - - text: "Is there anything else I can help you with?" + - text: "What else I can help you with?" metadata: rephrase: True utter_internal_error_rasa: - - text: Sorry, I'm having trouble understanding you right now. Please try again later. + - text: Sorry, I am having trouble with that. Please try again in a few minutes. utter_clarification_options_rasa: - - text: I'm not sure what you'd like to achieve. Do you want to {{context.clarification_options}}? + - text: "I can help, but I need more information. Which of these would you like to do: {{context.clarification_options}}?" metadata: rephrase: True template: jinja utter_inform_code_change: - - text: There has been an update to my code. I need to wrap up our running dialogue tasks and start from scratch. + - text: There has been an update to my code. I need to wrap up our running dialogue and start from scratch. metadata: rephrase: True From a46c52082d22c4629a605f69a76688fcbef6f4aa Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Thu, 28 Sep 2023 10:39:57 +0200 Subject: [PATCH 19/20] fixes for code quality, tests --- rasa/dialogue_understanding/processor/command_processor.py | 2 +- tests/cdu/commands/test_handle_code_change_command.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rasa/dialogue_understanding/processor/command_processor.py b/rasa/dialogue_understanding/processor/command_processor.py index 8e17f5e14c5c..e80a9c0998b2 100644 --- a/rasa/dialogue_understanding/processor/command_processor.py +++ b/rasa/dialogue_understanding/processor/command_processor.py @@ -161,7 +161,7 @@ def execute_commands( # store current flow hashes if they changed new_hashes = calculate_flow_fingerprints(all_flows) - flow_hash_events = [] + flow_hash_events: List[Event] = [] if new_hashes != (tracker.get_slot(FLOW_HASHES_SLOT) or {}): flow_hash_events.append(SlotSet(FLOW_HASHES_SLOT, new_hashes)) tracker.update_with_events(flow_hash_events, None) diff --git a/tests/cdu/commands/test_handle_code_change_command.py b/tests/cdu/commands/test_handle_code_change_command.py index f6a6afc2d06e..6ab56430d9a4 100644 --- a/tests/cdu/commands/test_handle_code_change_command.py +++ b/tests/cdu/commands/test_handle_code_change_command.py @@ -33,7 +33,7 @@ def test_name_of_command(): # names of commands should not change as they are part of persisted # trackers - assert HandleCodeChangeCommand.command() == "clean stack" + assert HandleCodeChangeCommand.command() == "handle code change" def test_from_dict(): From ff6c1fb0f68fe8a60c9eb80544ece3d79fbf4cff Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Fri, 29 Sep 2023 09:27:49 +0200 Subject: [PATCH 20/20] fixed default flow syntax --- .../patterns/default_flows_for_patterns.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml b/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml index 1a49392e27f9..79ec9ca351d1 100644 --- a/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml +++ b/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml @@ -7,7 +7,7 @@ responses: template: jinja utter_corrected_previous_input: - - text: "Ok, I am updating {{ context.corrected_slots.keys()|join(', ') }} to {{ context.corrected_slots.values()|join(', ') respectively." + - text: "Ok, I am updating {{ context.corrected_slots.keys()|join(', ') }} to {{ context.corrected_slots.values()|join(', ') }} respectively." metadata: rephrase: True template: jinja