diff --git a/rasa/cli/project_templates/dm2/data/flows.yml b/rasa/cli/project_templates/dm2/data/flows.yml index 319c4034e754..d4185318a853 100644 --- a/rasa/cli/project_templates/dm2/data/flows.yml +++ b/rasa/cli/project_templates/dm2/data/flows.yml @@ -13,7 +13,7 @@ flows: description: greet the user and ask how they are doing. cheer them up if needed. steps: - id: "0" - collect_information: good_mood + collect: good_mood description: "can be true or false" next: - if: good_mood @@ -30,13 +30,13 @@ flows: description: This flow recommends a restaurant steps: - id: "0" - collect_information: cuisine + collect: cuisine next: "1" - id: "1" - collect_information: price_range + collect: price_range next: "2" - id: "2" - collect_information: city + collect: city next: "3" - id: "3" action: utter_recommend_restaurant diff --git a/rasa/cli/project_templates/tutorial/data/flows.yml b/rasa/cli/project_templates/tutorial/data/flows.yml index aa081218dc1f..040a96b776f1 100644 --- a/rasa/cli/project_templates/tutorial/data/flows.yml +++ b/rasa/cli/project_templates/tutorial/data/flows.yml @@ -3,10 +3,10 @@ flows: description: This flow lets users send money to friends and family. steps: - id: "ask_recipient" - collect_information: recipient + collect: recipient next: "ask_amount" - id: "ask_amount" - collect_information: amount + collect: amount next: "transfer_successful" - id: "transfer_successful" action: utter_transfer_complete diff --git a/rasa/core/actions/action_run_slot_rejections.py b/rasa/core/actions/action_run_slot_rejections.py index ad55d4bf6069..3cbefe54173f 100644 --- a/rasa/core/actions/action_run_slot_rejections.py +++ b/rasa/core/actions/action_run_slot_rejections.py @@ -50,7 +50,7 @@ async def run( if not top_frame.rejections: return [] - slot_name = top_frame.collect_information + slot_name = top_frame.collect slot_instance = tracker.slots.get(slot_name) if slot_instance and not slot_instance.has_been_set: # this is the first time the assistant asks for the slot value, @@ -101,7 +101,7 @@ async def run( return [] # reset slot value that was initially filled with an invalid value - events.append(SlotSet(top_frame.collect_information, None)) + events.append(SlotSet(top_frame.collect, None)) if internal_error: utterance = "utter_internal_error_rasa" diff --git a/rasa/core/channels/chat.html b/rasa/core/channels/chat.html index 914c0bf16684..c7f299da3405 100644 --- a/rasa/core/channels/chat.html +++ b/rasa/core/channels/chat.html @@ -337,51 +337,67 @@

Happy chatting!

title: ${flow?.id ?? "No Flow"} --- flowchart TD -classDef collect_information stroke-width:1px +classDef collect stroke-width:1px classDef action fill:#FBFCFD,stroke:#A0B8CF classDef link fill:#f43 -classDef slot fill:#aaa +classDef slot fill:#e8f3db,stroke:#c5e1a5 +classDef endstep fill:#ccc,stroke:#444 classDef active stroke:#5A17ED,stroke-width:3px,stroke-dasharray: 5 3 `; - if (flow) { - flow.steps.forEach((step) => { - if (step.collect_information) { - var slotValue = slots[step.collect_information] - ? `'${slots[step.collect_information]}'` + function renderStepSequence(steps) { + var mermaidTextFragment = ""; + steps.forEach((step) => { + if (step.collect) { + var slotValue = slots[step.collect] + ? `'${slots[step.collect]}'` : "\uD83D\uDCAC"; - mermaidText += `${step.id}["${toHtmlEntities(keepShort(inject(step.collect_information, currentContext)))}\n${keepShort(slotValue)}"]:::collect_information\n`; + mermaidTextFragment += `${step.id}["${toHtmlEntities(keepShort(inject(step.collect, currentContext)))}\n${keepShort(slotValue)}"]:::collect\n`; } if (step.action) { - mermaidText += `${step.id}["${toHtmlEntities(keepShort(inject(step.action, currentContext)))}"]:::action\n`; + mermaidTextFragment += `${step.id}["${toHtmlEntities(keepShort(inject(step.action, currentContext)))}"]:::action\n`; } if (step.link) { - mermaidText += `${step.id}["\uD83D\uDD17 ${step.link}"]:::link\n`; + mermaidTextFragment += `${step.id}["\uD83D\uDD17 ${step.link}"]:::link\n`; } if (step.set_slots) { - mermaidText += `${step.id}["\uD83E\uDEA3 ${step.id}"]:::slot\n`; + mermaidTextFragment += `${step.id}["\uD83E\uDEA3 ${step.id}"]:::slot\n`; } if (step.id === activeStep) { - mermaidText += `class ${step.id} active\n`; + mermaidTextFragment += `class ${step.id} active\n`; } // if next is an id, then it is a link if (step.next && typeof step.next === "string") { - mermaidText += `${step.id} --> ${step.next}\n`; + mermaidTextFragment += `${step.id} --> ${step.next}\n`; } // if next is an array, then it is a list of conditions if (step.next && Array.isArray(step.next)) { step.next.forEach((condition) => { - if (condition.then) { - mermaidText += `${step.id} -->|${toHtmlEntities(inject(condition.if, currentContext))}| ${condition.then}\n`; + if (condition.then && typeof condition.then === "string") { + mermaidTextFragment += `${step.id} -->|${toHtmlEntities(inject(condition.if, currentContext))}| ${condition.then}\n`; + } + else if (condition.then) { + mermaidTextFragment += `${step.id} -->|${toHtmlEntities(inject(condition.if, currentContext))}| ${condition.then[0].id}\n`; + mermaidTextFragment += renderStepSequence(condition.then); } - if (condition.else) { - mermaidText += `${step.id} -->|else| ${condition.else}\n`; + + if (condition.else && typeof condition.else === "string") { + mermaidTextFragment += `${step.id} -->|else| ${condition.else}\n`; + } else if(condition.else) { + mermaidTextFragment += `${step.id} -->|else| ${condition.else[0].id}\n`; + mermaidTextFragment += renderStepSequence(condition.else); } }) } - }) + }); + mermaidTextFragment += `END["\uD83C\uDFC1 END"]:::endstep\n`; + return mermaidTextFragment; + } + + if (flow) { + mermaidText += renderStepSequence(flow.steps); } let flowEl = document.getElementById("flow"); console.log(mermaidText); diff --git a/rasa/core/policies/flow_policy.py b/rasa/core/policies/flow_policy.py index 2895458d30d1..99d2937d3159 100644 --- a/rasa/core/policies/flow_policy.py +++ b/rasa/core/policies/flow_policy.py @@ -68,7 +68,6 @@ from rasa.engine.storage.storage import ModelStorage from rasa.shared.core.domain import Domain from rasa.shared.core.generator import TrackerWithCachedStates -from rasa.shared.core.slots import Slot from rasa.shared.core.trackers import ( DialogueStateTracker, ) @@ -342,12 +341,18 @@ def _select_next_step_id( tracker=tracker, ) return None - if current.id != END_STEP: - # we've reached the end of the user defined steps in the flow. - # every flow should end with an end step, so we add it here. + if current.id == END_STEP: + # we are already at the very end of the flow. There is no next step. + return None + elif isinstance(current, LinkFlowStep): + # link steps don't have a next step, so we'll return the end step return END_STEP else: - # we are already at the very end of the flow. There is no next step. + structlogger.error( + "flow.step.failed_to_select_next_step", + step=current, + tracker=tracker, + ) return None def _select_next_step( @@ -372,23 +377,12 @@ def render_template_variables(text: str, context: Dict[Text, Any]) -> str: """Replace context variables in a text.""" return Template(text).render(context) - def _slot_for_collect_information(self, collect_information: Text) -> Slot: - """Find the slot for the collect information step.""" - for slot in self.domain.slots: - if slot.name == collect_information: - return slot - else: - raise FlowException( - f"Collect Information '{collect_information}' does not map to " - f"an existing slot." - ) - def _is_step_completed( self, step: FlowStep, tracker: "DialogueStateTracker" ) -> bool: """Check if a step is completed.""" if isinstance(step, CollectInformationFlowStep): - return tracker.get_slot(step.collect_information) is not None + return tracker.get_slot(step.collect) is not None else: return True @@ -522,9 +516,9 @@ def _reset_scoped_slots( isinstance(step, CollectInformationFlowStep) and step.reset_after_flow_ends ): - slot = tracker.slots.get(step.collect_information, None) + slot = tracker.slots.get(step.collect, None) initial_value = slot.initial_value if slot else None - events.append(SlotSet(step.collect_information, initial_value)) + events.append(SlotSet(step.collect, initial_value)) return events def _run_step( @@ -551,17 +545,17 @@ def _run_step( A result of running the step describing where to transition to. """ if isinstance(step, CollectInformationFlowStep): - structlogger.debug("flow.step.run.collect_information") + structlogger.debug("flow.step.run.collect") self.trigger_pattern_ask_collect_information( - step.collect_information, step.rejections, step.utter + step.collect, step.rejections, step.utter ) # reset the slot if its already filled and the collect information shouldn't # be skipped - slot = tracker.slots.get(step.collect_information, None) + slot = tracker.slots.get(step.collect, None) if slot and slot.has_been_set and step.ask_before_filling: - events = [SlotSet(step.collect_information, slot.initial_value)] + events = [SlotSet(step.collect, slot.initial_value)] else: events = [] @@ -683,14 +677,14 @@ def trigger_pattern_completed(self, current_frame: DialogueStackFrame) -> None: def trigger_pattern_ask_collect_information( self, - collect_information: str, + collect: str, rejections: List[SlotRejection], utter: str, ) -> None: """Trigger the pattern to ask for a slot value.""" self.dialogue_stack.push( CollectInformationPatternFlowStackFrame( - collect_information=collect_information, + collect=collect, utter=utter, rejections=rejections, ) diff --git a/rasa/core/processor.py b/rasa/core/processor.py index da2d2c886e56..0100b9597399 100644 --- a/rasa/core/processor.py +++ b/rasa/core/processor.py @@ -652,13 +652,9 @@ async def trigger_external_user_uttered( @staticmethod def _log_slots(tracker: DialogueStateTracker) -> None: # Log currently set slots - slot_values = "\n".join( - [f"\t{s.name}: {s.value}" for s in tracker.slots.values()] - ) - if slot_values.strip(): - structlogger.debug( - "processor.slots.log", slot_values=copy.deepcopy(slot_values) - ) + slots = {s.name: s.value for s in tracker.slots.values() if s.value is not None} + + structlogger.debug("processor.slots.log", slots=slots) def _check_for_unseen_features(self, parse_data: Dict[Text, Any]) -> None: """Warns the user if the NLU parse data contains unrecognized features. diff --git a/rasa/dialogue_understanding/commands/correct_slots_command.py b/rasa/dialogue_understanding/commands/correct_slots_command.py index 6c9a425ba084..2c80ed35e531 100644 --- a/rasa/dialogue_understanding/commands/correct_slots_command.py +++ b/rasa/dialogue_understanding/commands/correct_slots_command.py @@ -68,7 +68,7 @@ def are_all_slots_reset_only( ) -> bool: """Checks if all slots are reset only. - A slot is reset only if the `collect_information` step it gets filled by + A slot is reset only if the `collect` step it gets filled by has the `ask_before_filling` flag set to `True`. This means, the slot shouldn't be filled if the question isn't asked. @@ -83,10 +83,10 @@ def are_all_slots_reset_only( `True` if all slots are reset only, `False` otherwise. """ return all( - collect_information_step.collect_information not in proposed_slots - or collect_information_step.ask_before_filling + collect_step.collect not in proposed_slots + or collect_step.ask_before_filling for flow in all_flows.underlying_flows - for collect_information_step in flow.get_collect_information_steps() + for collect_step in flow.get_collect_steps() ) @staticmethod @@ -123,11 +123,11 @@ def find_earliest_updated_collect_info( # The way to get the exact set of slots would probably simulate the # flow forwards from the starting step. Given the current slots you # could chart the path to the current step id. - asked_collect_info_steps = flow.previous_collect_information_steps(step.id) + asked_collect_steps = flow.previous_collect_steps(step.id) - for collect_info_step in reversed(asked_collect_info_steps): - if collect_info_step.collect_information in updated_slots: - return collect_info_step + for collect_step in reversed(asked_collect_steps): + if collect_step.collect in updated_slots: + return collect_step return None def corrected_slots_dict(self, tracker: DialogueStateTracker) -> Dict[str, Any]: diff --git a/rasa/dialogue_understanding/commands/set_slot_command.py b/rasa/dialogue_understanding/commands/set_slot_command.py index c17bfea9c15f..b9f9de59ca7b 100644 --- a/rasa/dialogue_understanding/commands/set_slot_command.py +++ b/rasa/dialogue_understanding/commands/set_slot_command.py @@ -65,9 +65,9 @@ def run_command_on_tracker( if self.name not in slots_so_far: # only fill slots that belong to a collect infos that can be asked use_slot_fill = any( - step.collect_information == self.name and not step.ask_before_filling + step.collect == self.name and not step.ask_before_filling for flow in all_flows.underlying_flows - for step in flow.get_collect_information_steps() + for step in flow.get_collect_steps() ) if not use_slot_fill: diff --git a/rasa/dialogue_understanding/generator/command_prompt_template.jinja2 b/rasa/dialogue_understanding/generator/command_prompt_template.jinja2 index a909aec8825d..bb46fdcdf663 100644 --- a/rasa/dialogue_understanding/generator/command_prompt_template.jinja2 +++ b/rasa/dialogue_understanding/generator/command_prompt_template.jinja2 @@ -15,7 +15,7 @@ Here is what happened previously in the conversation: === {% if current_flow != None %} You are currently in the flow "{{ current_flow }}", which {{ current_flow.description }} -You have just asked the user for the slot "{{ collect_information }}"{% if collect_information_description %} ({{ collect_information_description }}){% endif %}. +You have just asked the user for the slot "{{ collect }}"{% if collect_description %} ({{ collect_description }}){% endif %}. {% if flow_slots|length > 0 %} Here are the slots of the currently active flow: diff --git a/rasa/dialogue_understanding/generator/llm_command_generator.py b/rasa/dialogue_understanding/generator/llm_command_generator.py index d4c4088c9d7a..d5ee1ac8bcc3 100644 --- a/rasa/dialogue_understanding/generator/llm_command_generator.py +++ b/rasa/dialogue_understanding/generator/llm_command_generator.py @@ -275,8 +275,8 @@ def create_template_inputs( if not flow.is_rasa_default_flow(): slots_with_info = [ - {"name": q.collect_information, "description": q.description} - for q in flow.get_collect_information_steps() + {"name": q.collect, "description": q.description} + for q in flow.get_collect_steps() if cls.is_extractable(q, tracker) ] result.append( @@ -294,12 +294,12 @@ def is_extractable( tracker: DialogueStateTracker, current_step: Optional[FlowStep] = None, ) -> bool: - """Check if the collect_information can be filled. + """Check if the `collect` can be filled. - A collect_information slot can only be filled if the slot exist - and either the collect_information has been asked already or the + A collect slot can only be filled if the slot exist + and either the collect has been asked already or the slot has been filled already.""" - slot = tracker.slots.get(q.collect_information) + slot = tracker.slots.get(q.collect) if slot is None: return False @@ -312,7 +312,7 @@ def is_extractable( or ( current_step is not None and isinstance(current_step, CollectInformationFlowStep) - and current_step.collect_information == q.collect_information + and current_step.collect == q.collect ) ) @@ -346,22 +346,22 @@ def render_template( if top_flow is not None: flow_slots = [ { - "name": q.collect_information, - "value": self.slot_value(tracker, q.collect_information), - "type": tracker.slots[q.collect_information].type_name, + "name": q.collect, + "value": self.slot_value(tracker, q.collect), + "type": tracker.slots[q.collect].type_name, "allowed_values": self.allowed_values_for_slot( - tracker.slots[q.collect_information] + tracker.slots[q.collect] ), "description": q.description, } - for q in top_flow.get_collect_information_steps() + for q in top_flow.get_collect_steps() if self.is_extractable(q, tracker, current_step) ] else: flow_slots = [] - collect_information, collect_information_description = ( - (current_step.collect_information, current_step.description) + collect, collect_description = ( + (current_step.collect, current_step.description) if isinstance(current_step, CollectInformationFlowStep) else (None, None) ) @@ -376,8 +376,8 @@ def render_template( "current_conversation": current_conversation, "flow_slots": flow_slots, "current_flow": top_flow.id if top_flow is not None else None, - "collect_information": collect_information, - "collect_information_description": collect_information_description, + "collect": collect, + "collect_description": collect_description, "user_message": latest_user_message, } diff --git a/rasa/dialogue_understanding/patterns/collect_information.py b/rasa/dialogue_understanding/patterns/collect_information.py index 6653f06a1e2d..2bda7588a622 100644 --- a/rasa/dialogue_understanding/patterns/collect_information.py +++ b/rasa/dialogue_understanding/patterns/collect_information.py @@ -8,7 +8,7 @@ from rasa.shared.core.flows.flow import SlotRejection FLOW_PATTERN_COLLECT_INFORMATION = ( - RASA_DEFAULT_FLOW_PATTERN_PREFIX + "ask_collect_information" + RASA_DEFAULT_FLOW_PATTERN_PREFIX + "collect_information" ) @@ -18,7 +18,7 @@ class CollectInformationPatternFlowStackFrame(PatternFlowStackFrame): flow_id: str = FLOW_PATTERN_COLLECT_INFORMATION """The ID of the flow.""" - collect_information: str = "" + collect: str = "" """The information that should be collected from the user. this corresponds to the slot that will be filled.""" utter: str = "" @@ -54,7 +54,7 @@ def from_dict(data: Dict[str, Any]) -> CollectInformationPatternFlowStackFrame: return CollectInformationPatternFlowStackFrame( data["frame_id"], step_id=data["step_id"], - collect_information=data["collect_information"], + collect=data["collect"], utter=data["utter"], rejections=rejections, ) diff --git a/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml b/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml index 2ca58f1afa33..917517f928ab 100644 --- a/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml +++ b/rasa/dialogue_understanding/patterns/default_flows_for_patterns.yml @@ -106,19 +106,19 @@ flows: - id: "1" action: utter_clarification_options_rasa - pattern_ask_collect_information: + pattern_collect_information: description: flow used to fill a slot steps: - id: "start" action: action_run_slot_rejections next: "validate" - id: "validate" - action: validate_{{context.collect_information}} + action: validate_{{context.collect}} next: - - if: "{{context.collect_information}} is not null" + - if: "{{context.collect}} is not null" then: "done" - - else: "ask_collect_information" - - id: "ask_collect_information" + - else: "ask_collect" + - id: "ask_collect" action: "{{context.utter}}" next: "listen" - id: "listen" diff --git a/rasa/dialogue_understanding/processor/command_processor.py b/rasa/dialogue_understanding/processor/command_processor.py index c9919900b448..4b7ecf7cc46f 100644 --- a/rasa/dialogue_understanding/processor/command_processor.py +++ b/rasa/dialogue_understanding/processor/command_processor.py @@ -163,7 +163,7 @@ def remove_duplicated_set_slots(events: List[Event]) -> List[Event]: return list(reversed(optimized_events)) -def get_current_collect_information( +def get_current_collect_step( dialogue_stack: DialogueStack, all_flows: FlowsList ) -> Optional[CollectInformationFlowStep]: """Get the current collect information if the conversation is currently in one. @@ -193,7 +193,7 @@ def get_current_collect_information( # for some reason only the collect information pattern step is on the stack # but no flow that triggered it. this should never happen. structlogger.warning( - "command_executor.get_current_collect information.no_flow_on_stack", + "command_executor.get_current_collect_step.no_flow_on_stack", stack=dialogue_stack, ) return None @@ -203,7 +203,7 @@ def get_current_collect_information( # this is a failure, if there is a frame, we should be able to get the # step from it structlogger.warning( - "command_executor.get_current_collect_information.no_step_for_frame", + "command_executor.get_current_collect_step.no_step_for_frame", frame=frame_that_triggered_collect_infos, ) return None @@ -216,7 +216,7 @@ def get_current_collect_information( # this should never happen as we only push collect information patterns # onto the stack if there is a collect information step structlogger.warning( - "command_executor.get_current_collect_information.step_not_collect_information", + "command_executor.get_current_collect_step.step_not_collect", step=step, ) return None @@ -246,14 +246,9 @@ def clean_up_commands( for command in commands: if isinstance(command, SetSlotCommand) and command.name in slots_so_far: - current_collect_info = get_current_collect_information( - dialogue_stack, all_flows - ) + current_collect_info = get_current_collect_step(dialogue_stack, all_flows) - if ( - current_collect_info - and current_collect_info.collect_information == command.name - ): + if current_collect_info and current_collect_info.collect == command.name: # not a correction but rather an answer to the current collect info clean_commands.append(command) continue diff --git a/rasa/dialogue_understanding/stack/utils.py b/rasa/dialogue_understanding/stack/utils.py index 144638e2b71e..44c1675b82ef 100644 --- a/rasa/dialogue_understanding/stack/utils.py +++ b/rasa/dialogue_understanding/stack/utils.py @@ -82,8 +82,8 @@ def filled_slots_for_active_flow( # frames, because they don't have slots. continue flow = frame.flow(all_flows) - for q in flow.previous_collect_information_steps(frame.step_id): - filled_slots.add(q.collect_information) + for q in flow.previous_collect_steps(frame.step_id): + filled_slots.add(q.collect) if isinstance(frame, UserFlowStackFrame): # as soon as we hit the first stack frame that is a "normal" diff --git a/rasa/engine/graph.py b/rasa/engine/graph.py index ad0ab228989c..7dbe3a519d83 100644 --- a/rasa/engine/graph.py +++ b/rasa/engine/graph.py @@ -3,9 +3,10 @@ import dataclasses from abc import ABC, abstractmethod from dataclasses import dataclass, field -import logging from typing import Any, Callable, Dict, List, Optional, Text, Type, Tuple, Union +import structlog + from rasa.engine.exceptions import ( GraphComponentException, GraphRunError, @@ -19,7 +20,7 @@ from rasa.shared.exceptions import InvalidConfigException, RasaException from rasa.shared.data import TrainingType -logger = logging.getLogger(__name__) +structlogger = structlog.get_logger() @dataclass @@ -392,10 +393,12 @@ def __init__( self._load_component() def _load_component(self, **kwargs: Any) -> None: - logger.debug( - f"Node '{self._node_name}' loading " - f"'{self._component_class.__name__}.{self._constructor_name}' " - f"and kwargs: '{kwargs}'." + structlogger.debug( + "graph.node.loading_component", + node_name=self._node_name, + clazz=self._component_class.__name__, + constructor=self._constructor_name, + kwargs=kwargs, ) constructor = getattr(self._component_class, self._constructor_name) @@ -417,8 +420,9 @@ def _load_component(self, **kwargs: Any) -> None: f"Error initializing graph component for node {self._node_name}." ) from e else: - logger.error( - f"Error initializing graph component for node {self._node_name}." + structlogger.error( + "graph.node.error_loading_component", + node_name=self._node_name, ) raise @@ -458,10 +462,14 @@ def __call__( node_name, node_output = i received_inputs[node_name] = node_output else: - logger.warning( - f"Node '{i}' was not resolved, there is no putput. " - f"Another component should have provided this as an " - f"output." + structlogger.warning( + "graph.node.input_not_resolved", + node_name=self._node_name, + input_name=i, + event_info=( + "Node input was not resolved, there is no putput. " + "Another component should have provided this as an output." + ), ) kwargs = {} @@ -487,9 +495,11 @@ def __call__( else: run_kwargs = kwargs - logger.debug( - f"Node '{self._node_name}' running " - f"'{self._component_class.__name__}.{self._fn_name}'." + structlogger.debug( + "graph.node.running_component", + node_name=self._node_name, + clazz=self._component_class.__name__, + fn=self._fn_name, ) try: @@ -504,8 +514,9 @@ def __call__( f"Error running graph component for node {self._node_name}." ) from e else: - logger.error( - f"Error running graph component for node {self._node_name}." + structlogger.error( + "graph.node.error_running_component", + node_name=self._node_name, ) raise @@ -516,9 +527,10 @@ def __call__( def _run_after_hooks(self, input_hook_outputs: List[Dict], output: Any) -> None: for hook, hook_data in zip(self._hooks, input_hook_outputs): try: - logger.debug( - f"Hook '{hook.__class__.__name__}.on_after_node' " - f"running for node '{self._node_name}'." + structlogger.debug( + "graph.node.hook.on_after_node", + node_name=self._node_name, + hook_name=hook.__class__.__name__, ) hook.on_after_node( node_name=self._node_name, @@ -536,9 +548,10 @@ def _run_before_hooks(self, received_inputs: Dict[Text, Any]) -> List[Dict]: input_hook_outputs = [] for hook in self._hooks: try: - logger.debug( - f"Hook '{hook.__class__.__name__}.on_before_node' " - f"running for node '{self._node_name}'." + structlogger.debug( + "graph.node.hook.on_before_node", + node_name=self._node_name, + hook_name=hook.__class__.__name__, ) hook_output = hook.on_before_node( node_name=self._node_name, diff --git a/rasa/shared/core/flows/flow.py b/rasa/shared/core/flows/flow.py index 47db88adfd26..4ea95e9858ad 100644 --- a/rasa/shared/core/flows/flow.py +++ b/rasa/shared/core/flows/flow.py @@ -1,8 +1,18 @@ from __future__ import annotations from dataclasses import dataclass -from typing import Any, Dict, List, Optional, Protocol, Set, Text, runtime_checkable - +from typing import ( + Any, + Dict, + Generator, + List, + Optional, + Protocol, + Set, + Text, + Union, + runtime_checkable, +) import structlog from rasa.shared.core.trackers import DialogueStateTracker @@ -18,6 +28,12 @@ structlogger = structlog.get_logger() +START_STEP = "START" + +END_STEP = "END" + +DEFAULT_STEPS = {END_STEP, START_STEP} + class UnreachableFlowStepException(RasaException): """Raised when a flow step is unreachable.""" @@ -37,6 +53,71 @@ def __str__(self) -> Text: ) +class MissingNextLinkException(RasaException): + """Raised when a flow step is missing a next link.""" + + def __init__(self, step: FlowStep, flow: Flow) -> None: + """Initializes the exception.""" + self.step = step + self.flow = flow + + def __str__(self) -> Text: + """Return a string representation of the exception.""" + return ( + f"Step '{self.step.id}' in flow '{self.flow.id}' is missing a `next`. " + f"As a last step of a branch, it is required to have one. " + ) + + +class ReservedFlowStepIdException(RasaException): + """Raised when a flow step is using a reserved id.""" + + def __init__(self, step: FlowStep, flow: Flow) -> None: + """Initializes the exception.""" + self.step = step + self.flow = flow + + def __str__(self) -> Text: + """Return a string representation of the exception.""" + return ( + f"Step '{self.step.id}' in flow '{self.flow.id}' is using the reserved id " + f"'{self.step.id}'. Please use a different id for your step." + ) + + +class MissingElseBranchException(RasaException): + """Raised when a flow step is missing an else branch.""" + + def __init__(self, step: FlowStep, flow: Flow) -> None: + """Initializes the exception.""" + self.step = step + self.flow = flow + + def __str__(self) -> Text: + """Return a string representation of the exception.""" + return ( + f"Step '{self.step.id}' in flow '{self.flow.id}' is missing an `else` " + f"branch. If a steps `next` statement contains an `if` it always " + f"also needs an `else` branch. Please add the missing `else` branch." + ) + + +class NoNextAllowedForLinkException(RasaException): + """Raised when a flow step has a next link but is not allowed to have one.""" + + def __init__(self, step: FlowStep, flow: Flow) -> None: + """Initializes the exception.""" + self.step = step + self.flow = flow + + def __str__(self) -> Text: + """Return a string representation of the exception.""" + return ( + f"Link step '{self.step.id}' in flow '{self.flow.id}' has a `next` but " + f"as a link step is not allowed to have one." + ) + + class UnresolvedFlowStepIdException(RasaException): """Raised when a flow step is referenced, but its id can not be resolved.""" @@ -193,7 +274,7 @@ class Flow: """The name of the flow.""" description: Optional[Text] """The description of the flow.""" - steps: List[FlowStep] + step_sequence: StepSequence """The steps of the flow.""" @staticmethod @@ -206,14 +287,54 @@ def from_json(flow_id: Text, flow_config: Dict[Text, Any]) -> Flow: Returns: The parsed flow. """ - steps = flow_config.get("steps") or [] + step_sequence = StepSequence.from_json(flow_config.get("steps")) + return Flow( id=flow_id, name=flow_config.get("name", ""), description=flow_config.get("description"), - steps=[step_from_json(step_config) for step_config in steps], + step_sequence=Flow.resolve_default_ids(step_sequence), ) + @staticmethod + def resolve_default_ids(step_sequence: StepSequence) -> StepSequence: + """Resolves the default ids of all steps in the sequence. + + If a step does not have an id, a default id is assigned to it based + on the type of the step and its position in the flow. + + Similarly, if a step doesn't have an explicit next assigned we resolve + the default next step id. + + Args: + step_sequence: The step sequence to resolve the default ids for. + + Returns: + The step sequence with the default ids resolved. + """ + # assign an index to all steps + for idx, step in enumerate(step_sequence.steps): + step.idx = idx + + def resolve_default_next(steps: List[FlowStep], is_root_sequence: bool) -> None: + for i, step in enumerate(steps): + if step.next.no_link_available(): + if i == len(steps) - 1: + # can't attach end to link step + if is_root_sequence and not isinstance(step, LinkFlowStep): + # if this is the root sequence, we need to add an end step + # to the end of the sequence. other sequences, e.g. + # in branches need to explicitly add a next step. + step.next.links.append(StaticFlowLink(target=END_STEP)) + else: + step.next.links.append(StaticFlowLink(target=steps[i + 1].id)) + for link in step.next.links: + if sub_steps := link.child_steps(): + resolve_default_next(sub_steps, is_root_sequence=False) + + resolve_default_next(step_sequence.child_steps, is_root_sequence=True) + return step_sequence + def as_json(self) -> Dict[Text, Any]: """Returns the flow as a dictionary. @@ -224,7 +345,7 @@ def as_json(self) -> Dict[Text, Any]: "id": self.id, "name": self.name, "description": self.description, - "steps": [step.as_json() for step in self.steps], + "steps": self.step_sequence.as_json(), } def readable_name(self) -> str: @@ -239,12 +360,43 @@ def validate(self) -> None: - whether all next links point to existing steps - whether all steps can be reached from the start step """ + self._validate_all_steps_next_property() self._validate_all_next_ids_are_availble_steps() self._validate_all_steps_can_be_reached() + self._validate_all_branches_have_an_else() + self._validate_not_using_buildin_ids() + + def _validate_not_using_buildin_ids(self) -> None: + """Validates that the flow does not use any of the build in ids.""" + for step in self.steps: + if step.id in DEFAULT_STEPS or step.id.startswith(CONTINUE_STEP_PREFIX): + raise ReservedFlowStepIdException(step, self) + + def _validate_all_branches_have_an_else(self) -> None: + """Validates that all branches have an else link.""" + for step in self.steps: + links = step.next.links + + has_an_if = any(isinstance(link, IfFlowLink) for link in links) + has_an_else = any(isinstance(link, ElseFlowLink) for link in links) + + if has_an_if and not has_an_else: + raise MissingElseBranchException(step, self) + + def _validate_all_steps_next_property(self) -> None: + """Validates that every step has a next link.""" + for step in self.steps: + if isinstance(step, LinkFlowStep): + # link steps can't have a next link! + if not step.next.no_link_available(): + raise NoNextAllowedForLinkException(step, self) + elif step.next.no_link_available(): + # all other steps should have a next link + raise MissingNextLinkException(step, self) def _validate_all_next_ids_are_availble_steps(self) -> None: """Validates that all next links point to existing steps.""" - available_steps = {step.id for step in self.steps} + available_steps = {step.id for step in self.steps} | DEFAULT_STEPS for step in self.steps: for link in step.next.links: if link.target not in available_steps: @@ -300,7 +452,7 @@ def first_step_in_flow(self) -> Optional[FlowStep]: return None return self.steps[0] - def previous_collect_information_steps( + def previous_collect_steps( self, step_id: Optional[str] ) -> List[CollectInformationFlowStep]: """Returns the collect informations asked before the given step. @@ -310,7 +462,7 @@ def previous_collect_information_steps( in the flow the order is not guaranteed to be exactly reverse. """ - def _previously_asked_collect_information( + def _previously_asked_collect( current_step_id: str, visited_steps: Set[str] ) -> List[CollectInformationFlowStep]: """Returns the collect informations asked before the given step. @@ -319,13 +471,13 @@ def _previously_asked_collect_information( """ current_step = self.step_by_id(current_step_id) - collect_informations: List[CollectInformationFlowStep] = [] + collects: List[CollectInformationFlowStep] = [] if not current_step: - return collect_informations + return collects if isinstance(current_step, CollectInformationFlowStep): - collect_informations.append(current_step) + collects.append(current_step) visited_steps.add(current_step.id) @@ -335,14 +487,12 @@ def _previously_asked_collect_information( continue if previous_step.id in visited_steps: continue - collect_informations.extend( - _previously_asked_collect_information( - previous_step.id, visited_steps - ) + collects.extend( + _previously_asked_collect(previous_step.id, visited_steps) ) - return collect_informations + return collects - return _previously_asked_collect_information(step_id or START_STEP, set()) + return _previously_asked_collect(step_id or START_STEP, set()) def is_handling_pattern(self) -> bool: """Returns whether the flow is handling a pattern.""" @@ -372,13 +522,65 @@ def is_rasa_default_flow(self) -> bool: """Test whether something is a rasa default flow.""" return self.id.startswith(RASA_DEFAULT_FLOW_PATTERN_PREFIX) - def get_collect_information_steps(self) -> List[CollectInformationFlowStep]: + def get_collect_steps(self) -> List[CollectInformationFlowStep]: """Return the collect information steps of the flow.""" - collect_information_steps = [] + collect_steps = [] for step in self.steps: if isinstance(step, CollectInformationFlowStep): - collect_information_steps.append(step) - return collect_information_steps + collect_steps.append(step) + return collect_steps + + @property + def steps(self) -> List[FlowStep]: + """Returns the steps of the flow.""" + return self.step_sequence.steps + + +@dataclass +class StepSequence: + child_steps: List[FlowStep] + + @staticmethod + def from_json(steps_config: List[Dict[Text, Any]]) -> StepSequence: + """Used to read steps from parsed YAML. + + Args: + steps_config: The parsed YAML as a dictionary. + + Returns: + The parsed steps. + """ + + flow_steps: List[FlowStep] = [step_from_json(config) for config in steps_config] + + return StepSequence(child_steps=flow_steps) + + def as_json(self) -> List[Dict[Text, Any]]: + """Returns the steps as a dictionary. + + Returns: + The steps as a dictionary. + """ + return [ + step.as_json() + for step in self.child_steps + if not isinstance(step, InternalFlowStep) + ] + + @property + def steps(self) -> List[FlowStep]: + """Returns the steps of the flow.""" + return [ + step + for child_step in self.child_steps + for step in child_step.steps_in_tree() + ] + + def first(self) -> Optional[FlowStep]: + """Returns the first step of the sequence.""" + if len(self.child_steps) == 0: + return None + return self.child_steps[0] def step_from_json(flow_step_config: Dict[Text, Any]) -> FlowStep: @@ -394,7 +596,7 @@ def step_from_json(flow_step_config: Dict[Text, Any]) -> FlowStep: return ActionFlowStep.from_json(flow_step_config) if "intent" in flow_step_config: return UserMessageStep.from_json(flow_step_config) - if "collect_information" in flow_step_config: + if "collect" in flow_step_config: return CollectInformationFlowStep.from_json(flow_step_config) if "link" in flow_step_config: return LinkFlowStep.from_json(flow_step_config) @@ -412,8 +614,10 @@ def step_from_json(flow_step_config: Dict[Text, Any]) -> FlowStep: class FlowStep: """Represents the configuration of a flow step.""" - id: Text + custom_id: Optional[Text] """The id of the flow step.""" + idx: int + """The index of the step in the flow.""" description: Optional[Text] """The description of the flow step.""" metadata: Dict[Text, Any] @@ -432,7 +636,10 @@ def _from_json(cls, flow_step_config: Dict[Text, Any]) -> FlowStep: The parsed flow step. """ return FlowStep( - id=flow_step_config["id"], + # the idx is set later once the flow is created that contains + # this step + idx=-1, + custom_id=flow_step_config.get("id"), description=flow_step_config.get("description"), metadata=flow_step_config.get("metadata", {}), next=FlowLinks.from_json(flow_step_config.get("next", [])), @@ -444,18 +651,31 @@ def as_json(self) -> Dict[Text, Any]: Returns: The flow step as a dictionary. """ - dump = { - "id": self.id, - "next": self.next.as_json(), - } + dump = {"next": self.next.as_json(), "id": self.id} + if self.description: dump["description"] = self.description if self.metadata: dump["metadata"] = self.metadata return dump + def steps_in_tree(self) -> Generator[FlowStep, None, None]: + """Returns the steps in the tree of the flow step.""" + yield self + yield from self.next.steps_in_tree() + + @property + def id(self) -> Text: + """Returns the id of the flow step.""" + return self.custom_id or self.default_id() -START_STEP = "__start__" + def default_id(self) -> str: + """Returns the default id of the flow step.""" + return f"{self.idx}_{self.default_id_postfix()}" + + def default_id_postfix(self) -> str: + """Returns the default id postfix of the flow step.""" + raise NotImplementedError() class InternalFlowStep(FlowStep): @@ -502,16 +722,14 @@ def __init__(self, start_step_id: Optional[Text]) -> None: links = [] super().__init__( - id=START_STEP, + idx=0, + custom_id=START_STEP, description=None, metadata={}, next=FlowLinks(links=links), ) -END_STEP = "__end__" - - @dataclass class EndFlowStep(InternalFlowStep): """Represents the configuration of an end to a flow.""" @@ -519,14 +737,15 @@ class EndFlowStep(InternalFlowStep): def __init__(self) -> None: """Initializes an end flow step.""" super().__init__( - id=END_STEP, + idx=0, + custom_id=END_STEP, description=None, metadata={}, next=FlowLinks(links=[]), ) -CONTINUE_STEP_PREFIX = "__next__" +CONTINUE_STEP_PREFIX = "NEXT:" @dataclass @@ -536,7 +755,8 @@ class ContinueFlowStep(InternalFlowStep): def __init__(self, next: str) -> None: """Initializes a continue-step flow step.""" super().__init__( - id=CONTINUE_STEP_PREFIX + next, + idx=0, + custom_id=CONTINUE_STEP_PREFIX + next, description=None, metadata={}, # The continue step links to the step that should be continued. @@ -587,6 +807,9 @@ def as_json(self) -> Dict[Text, Any]: dump["action"] = self.action return dump + def default_id_postfix(self) -> str: + return self.action + @dataclass class BranchFlowStep(FlowStep): @@ -614,6 +837,10 @@ def as_json(self) -> Dict[Text, Any]: dump = super().as_json() return dump + def default_id_postfix(self) -> str: + """Returns the default id postfix of the flow step.""" + return "branch" + @dataclass class LinkFlowStep(FlowStep): @@ -633,8 +860,6 @@ def from_json(cls, flow_step_config: Dict[Text, Any]) -> LinkFlowStep: The parsed flow step. """ base = super()._from_json(flow_step_config) - # Links are not allowed to have next step - base.next = FlowLinks(links=[]) return LinkFlowStep( link=flow_step_config.get("link", ""), **base.__dict__, @@ -650,6 +875,10 @@ def as_json(self) -> Dict[Text, Any]: dump["link"] = self.link return dump + def default_id_postfix(self) -> str: + """Returns the default id postfix of the flow step.""" + return f"link_{self.link}" + @dataclass class TriggerCondition: @@ -779,6 +1008,10 @@ def is_triggered(self, tracker: DialogueStateTracker) -> bool: for trigger_condition in self.trigger_conditions ) + def default_id_postfix(self) -> str: + """Returns the default id postfix of the flow step.""" + return "intent" + DEFAULT_LLM_CONFIG = { "_type": "openai", @@ -860,6 +1093,9 @@ def generate(self, tracker: DialogueStateTracker) -> Optional[Text]: ) return None + def default_id_postfix(self) -> str: + return "generate" + @dataclass class EntryPromptFlowStep(FlowStep, StepThatCanStartAFlow): @@ -961,6 +1197,10 @@ def is_triggered(self, tracker: DialogueStateTracker) -> bool: else: return False + def default_id_postfix(self) -> str: + """Returns the default id postfix of the flow step.""" + return "entry_prompt" + @dataclass class SlotRejection: @@ -1002,7 +1242,7 @@ def as_dict(self) -> Dict[Text, Any]: class CollectInformationFlowStep(FlowStep): """Represents the configuration of a collect information flow step.""" - collect_information: Text + collect: Text """The collect information of the flow step.""" utter: Text """The utterance that the assistant uses to ask for the slot.""" @@ -1025,9 +1265,9 @@ def from_json(cls, flow_step_config: Dict[Text, Any]) -> CollectInformationFlowS """ base = super()._from_json(flow_step_config) return CollectInformationFlowStep( - collect_information=flow_step_config.get("collect_information", ""), + collect=flow_step_config["collect"], utter=flow_step_config.get( - "utter", f"utter_ask_{flow_step_config['collect_information']}" + "utter", f"utter_ask_{flow_step_config['collect']}" ), ask_before_filling=flow_step_config.get("ask_before_filling", False), reset_after_flow_ends=flow_step_config.get("reset_after_flow_ends", True), @@ -1045,7 +1285,7 @@ def as_json(self) -> Dict[Text, Any]: The flow step as a dictionary. """ dump = super().as_json() - dump["collect_information"] = self.collect_information + dump["collect"] = self.collect dump["utter"] = self.utter dump["ask_before_filling"] = self.ask_before_filling dump["reset_after_flow_ends"] = self.reset_after_flow_ends @@ -1053,6 +1293,10 @@ def as_json(self) -> Dict[Text, Any]: return dump + def default_id_postfix(self) -> str: + """Returns the default id postfix of the flow step.""" + return f"collect_{self.collect}" + @dataclass class SetSlotsFlowStep(FlowStep): @@ -1092,6 +1336,10 @@ def as_json(self) -> Dict[Text, Any]: dump["set_slots"] = [{slot["key"]: slot["value"]} for slot in self.slots] return dump + def default_id_postfix(self) -> str: + """Returns the default id postfix of the flow step.""" + return "set_slots" + @dataclass class FlowLinks: @@ -1154,12 +1402,21 @@ def as_json(self) -> Any: return [link.as_json() for link in self.links] + def no_link_available(self) -> bool: + """Returns whether no link is available.""" + return len(self.links) == 0 + + def steps_in_tree(self) -> Generator[FlowStep, None, None]: + """Returns the steps in the tree of the flow links.""" + for link in self.links: + yield from link.steps_in_tree() + class FlowLink(Protocol): """Represents a flow link.""" @property - def target(self) -> Text: + def target(self) -> Optional[Text]: """Returns the target of the flow link. Returns: @@ -1187,13 +1444,48 @@ def from_json(link_config: Any) -> FlowLink: """ ... + def steps_in_tree(self) -> Generator[FlowStep, None, None]: + """Returns the steps in the tree of the flow link.""" + ... + + def child_steps(self) -> List[FlowStep]: + """Returns the child steps of the flow link.""" + ... + @dataclass -class IfFlowLink: +class BranchBasedLink: + target_reference: Union[Text, StepSequence] + """The id of the linked flow.""" + + def steps_in_tree(self) -> Generator[FlowStep, None, None]: + """Returns the steps in the tree of the flow link.""" + if isinstance(self.target_reference, StepSequence): + yield from self.target_reference.steps + + def child_steps(self) -> List[FlowStep]: + """Returns the child steps of the flow link.""" + if isinstance(self.target_reference, StepSequence): + return self.target_reference.child_steps + else: + return [] + + @property + def target(self) -> Optional[Text]: + """Returns the target of the flow link.""" + if isinstance(self.target_reference, StepSequence): + if first := self.target_reference.first(): + return first.id + else: + return None + else: + return self.target_reference + + +@dataclass +class IfFlowLink(BranchBasedLink): """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.""" @@ -1207,7 +1499,15 @@ def from_json(link_config: Dict[Text, Any]) -> IfFlowLink: Returns: The parsed flow link. """ - return IfFlowLink(target=link_config["then"], condition=link_config.get("if")) + if isinstance(link_config["then"], str): + return IfFlowLink( + target_reference=link_config["then"], condition=link_config.get("if") + ) + else: + return IfFlowLink( + target_reference=StepSequence.from_json(link_config["then"]), + condition=link_config.get("if"), + ) def as_json(self) -> Dict[Text, Any]: """Returns the flow link as a dictionary. @@ -1217,17 +1517,16 @@ def as_json(self) -> Dict[Text, Any]: """ return { "if": self.condition, - "then": self.target, + "then": self.target_reference.as_json() + if isinstance(self.target_reference, StepSequence) + else self.target_reference, } @dataclass -class ElseFlowLink: +class ElseFlowLink(BranchBasedLink): """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. @@ -1238,7 +1537,12 @@ def from_json(link_config: Dict[Text, Any]) -> ElseFlowLink: Returns: The parsed flow link. """ - return ElseFlowLink(target=link_config["else"]) + if isinstance(link_config["else"], str): + return ElseFlowLink(target_reference=link_config["else"]) + else: + return ElseFlowLink( + target_reference=StepSequence.from_json(link_config["else"]) + ) def as_json(self) -> Dict[Text, Any]: """Returns the flow link as a dictionary. @@ -1246,7 +1550,11 @@ def as_json(self) -> Dict[Text, Any]: Returns: The flow link as a dictionary. """ - return {"else": self.target} + return { + "else": self.target_reference.as_json() + if isinstance(self.target_reference, StepSequence) + else self.target_reference + } @dataclass @@ -1275,3 +1583,12 @@ def as_json(self) -> Text: The flow link as a dictionary. """ return self.target + + def steps_in_tree(self) -> Generator[FlowStep, None, None]: + """Returns the steps in the tree of the flow link.""" + # static links do not have any child steps + yield from [] + + def child_steps(self) -> List[FlowStep]: + """Returns the child steps of the flow link.""" + return [] diff --git a/rasa/utils/log_utils.py b/rasa/utils/log_utils.py index 5852a1ba3e45..5a530e391790 100644 --- a/rasa/utils/log_utils.py +++ b/rasa/utils/log_utils.py @@ -36,7 +36,7 @@ def _anonymizer( "text", "response_text", "user_text", - "slot_values", + "slots", "parse_data_text", "parse_data_entities", "prediction_events", diff --git a/rasa/validator.py b/rasa/validator.py index 6e2893472cf3..2afd89fe0d2f 100644 --- a/rasa/validator.py +++ b/rasa/validator.py @@ -241,7 +241,7 @@ def _utterances_used_in_flows(self) -> Set[str]: ): flow_utterances.add(step.action) if isinstance(step, CollectInformationFlowStep): - flow_utterances.add(UTTER_ASK_PREFIX + step.collect_information) + flow_utterances.add(UTTER_ASK_PREFIX + step.collect) return flow_utterances def verify_utterances_in_dialogues(self, ignore_warnings: bool = True) -> bool: diff --git a/tests/cdu/commands/test_cancel_flow_command.py b/tests/cdu/commands/test_cancel_flow_command.py index 0aa2ea74113a..6e902333cde9 100644 --- a/tests/cdu/commands/test_cancel_flow_command.py +++ b/tests/cdu/commands/test_cancel_flow_command.py @@ -72,7 +72,7 @@ def test_run_command_on_tracker(): assert dialogue_stack_dump[1]["type"] == "pattern_cancel_flow" assert dialogue_stack_dump[1]["flow_id"] == "pattern_cancel_flow" - assert dialogue_stack_dump[1]["step_id"] == "__start__" + assert dialogue_stack_dump[1]["step_id"] == "START" assert dialogue_stack_dump[1]["canceled_name"] == "foo flow" assert dialogue_stack_dump[1]["canceled_frames"] == ["some-frame-id"] @@ -84,7 +84,7 @@ def test_select_canceled_frames_cancels_patterns(): flow_id="foo", step_id="first_step", frame_id="some-frame-id" ), CollectInformationPatternFlowStackFrame( - collect_information="bar", frame_id="some-other-id" + collect="bar", frame_id="some-other-id" ), ] ) diff --git a/tests/cdu/commands/test_clarify_command.py b/tests/cdu/commands/test_clarify_command.py index 3dba7a707d1e..c317830eeba0 100644 --- a/tests/cdu/commands/test_clarify_command.py +++ b/tests/cdu/commands/test_clarify_command.py @@ -71,7 +71,7 @@ def test_run_command_ignores_non_existant_flows(): frame = dialogue_stack_dump.value[0] assert frame["type"] == "pattern_clarification" assert frame["flow_id"] == "pattern_clarification" - assert frame["step_id"] == "__start__" + assert frame["step_id"] == "START" assert frame["names"] == ["foo"] assert frame["clarification_options"] == "" diff --git a/tests/cdu/commands/test_correct_slots_command.py b/tests/cdu/commands/test_correct_slots_command.py index a0d5e8ef130d..bf8578d766c9 100644 --- a/tests/cdu/commands/test_correct_slots_command.py +++ b/tests/cdu/commands/test_correct_slots_command.py @@ -60,10 +60,10 @@ def test_run_command_on_tracker_correcting_previous_flow(): name: foo flow steps: - id: collect_foo - collect_information: foo + collect: foo next: collect_bar - id: collect_bar - collect_information: bar + collect: bar """ ) @@ -102,7 +102,7 @@ def test_run_command_on_tracker_correcting_previous_flow(): assert dialogue_stack_dump[1]["type"] == "pattern_correction" assert dialogue_stack_dump[1]["flow_id"] == "pattern_correction" - assert dialogue_stack_dump[1]["step_id"] == "__start__" + assert dialogue_stack_dump[1]["step_id"] == "START" assert dialogue_stack_dump[1]["corrected_slots"] == {"foo": "not-foofoo"} assert dialogue_stack_dump[1]["reset_flow_id"] == "my_flow" assert dialogue_stack_dump[1]["reset_step_id"] == "collect_foo" @@ -117,10 +117,10 @@ def test_run_command_on_tracker_correcting_current_flow(): name: foo flow steps: - id: collect_foo - collect_information: foo + collect: foo next: collect_bar - id: collect_bar - collect_information: bar + collect: bar """ ) @@ -159,7 +159,7 @@ def test_run_command_on_tracker_correcting_current_flow(): assert dialogue_stack_dump[1]["type"] == "pattern_correction" assert dialogue_stack_dump[1]["flow_id"] == "pattern_correction" - assert dialogue_stack_dump[1]["step_id"] == "__start__" + assert dialogue_stack_dump[1]["step_id"] == "START" assert dialogue_stack_dump[1]["corrected_slots"] == {"bar": "barbar"} assert dialogue_stack_dump[1]["reset_flow_id"] == "my_flow" assert dialogue_stack_dump[1]["reset_step_id"] == "collect_bar" @@ -174,10 +174,10 @@ def test_run_command_on_tracker_correcting_during_a_correction(): name: foo flow steps: - id: collect_foo - collect_information: foo + collect: foo next: collect_bar - id: collect_bar - collect_information: bar + collect: bar """ ) @@ -226,7 +226,7 @@ def test_run_command_on_tracker_correcting_during_a_correction(): assert dialogue_stack_dump[1]["type"] == "pattern_correction" assert dialogue_stack_dump[1]["flow_id"] == "pattern_correction" - assert dialogue_stack_dump[1]["step_id"] == "__start__" + assert dialogue_stack_dump[1]["step_id"] == "START" assert dialogue_stack_dump[1]["corrected_slots"] == {"bar": "barbar"} assert dialogue_stack_dump[1]["reset_flow_id"] == "my_flow" assert dialogue_stack_dump[1]["reset_step_id"] == "collect_bar" @@ -245,7 +245,7 @@ def test_index_for_correction_frame_handles_empty_stack(): def test_index_for_correction_handles_non_correction_pattern_at_the_top_of_stack(): top_flow_frame = CollectInformationPatternFlowStackFrame( - collect_information="foo", frame_id="some-other-id" + collect="foo", frame_id="some-other-id" ) stack = DialogueStack( frames=[ @@ -288,7 +288,7 @@ def test_end_previous_correction(): ) CorrectSlotsCommand.end_previous_correction(top_flow_frame, stack) # the previous pattern should be about to end - assert stack.frames[1].step_id == "__next____end__" + assert stack.frames[1].step_id == "NEXT:END" # make sure the user flow has not been modified assert stack.frames[0].step_id == "first_step" @@ -323,13 +323,13 @@ def test_find_earliest_updated_collect_info( name: foo flow steps: - id: collect_foo - collect_information: foo + collect: foo next: collect_bar - id: collect_bar - collect_information: bar + collect: bar next: collect_baz - id: collect_baz - collect_information: baz + collect: baz """ ) @@ -362,11 +362,11 @@ def test_are_all_slots_reset_only(proposed_slots: Dict[str, Any], expected: bool name: foo flow steps: - id: collect_foo - collect_information: foo + collect: foo ask_before_filling: true next: collect_bar - id: collect_bar - collect_information: bar + collect: bar """ ) assert ( diff --git a/tests/cdu/commands/test_error_command.py b/tests/cdu/commands/test_error_command.py index 187b4d37e98f..0b2ab94140b2 100644 --- a/tests/cdu/commands/test_error_command.py +++ b/tests/cdu/commands/test_error_command.py @@ -31,5 +31,5 @@ def test_run_command_on_tracker(): frame = dialogue_stack_event.value[0] assert frame["type"] == "pattern_internal_error" - assert frame["step_id"] == "__start__" + assert frame["step_id"] == "START" assert frame["flow_id"] == "pattern_internal_error" diff --git a/tests/cdu/commands/test_set_slot_command.py b/tests/cdu/commands/test_set_slot_command.py index 84e3867c2c0c..370ff3de20d5 100644 --- a/tests/cdu/commands/test_set_slot_command.py +++ b/tests/cdu/commands/test_set_slot_command.py @@ -48,10 +48,10 @@ def test_run_command_sets_slot_if_asked_for(): my_flow: steps: - id: collect_foo - collect_information: foo + collect: foo next: collect_bar - id: collect_bar - collect_information: bar + collect: bar """ ) @@ -84,11 +84,11 @@ def test_run_command_skips_set_slot_if_slot_was_not_asked_for(): my_flow: steps: - id: collect_foo - collect_information: foo + collect: foo next: collect_bar - id: collect_bar ask_before_filling: true - collect_information: bar + collect: bar """ ) @@ -122,10 +122,10 @@ def test_run_command_can_set_slots_before_asking(): my_flow: steps: - id: collect_foo - collect_information: foo + collect: foo next: collect_bar - id: collect_bar - collect_information: bar + collect: bar """ ) @@ -160,10 +160,10 @@ def test_run_command_can_set_slot_that_was_already_asked_in_the_past(): my_flow: steps: - id: collect_foo - collect_information: foo + collect: foo next: collect_bar - id: collect_bar - collect_information: bar + collect: bar """ ) @@ -199,10 +199,10 @@ def test_run_command_skips_setting_unknown_slot(): my_flow: steps: - id: collect_foo - collect_information: foo + collect: foo next: collect_bar - id: collect_bar - collect_information: bar + collect: bar """ ) diff --git a/tests/cdu/commands/test_start_flow_command.py b/tests/cdu/commands/test_start_flow_command.py index 2ac2895b44d1..628cc0273379 100644 --- a/tests/cdu/commands/test_start_flow_command.py +++ b/tests/cdu/commands/test_start_flow_command.py @@ -46,7 +46,7 @@ def test_run_command_on_tracker(): assert isinstance(dialogue_stack_dump, list) and len(dialogue_stack_dump) == 1 assert dialogue_stack_dump[0]["frame_type"] == "regular" assert dialogue_stack_dump[0]["flow_id"] == "foo" - assert dialogue_stack_dump[0]["step_id"] == "__start__" + assert dialogue_stack_dump[0]["step_id"] == "START" assert dialogue_stack_dump[0].get("frame_id") is not None @@ -88,7 +88,7 @@ def test_run_start_flow_that_is_already_on_the_stack(): "type": "flow", "frame_type": "regular", "flow_id": "foo", - "step_id": "__start__", + "step_id": "START", "frame_id": "test", } ], @@ -142,7 +142,7 @@ def test_run_start_flow_interrupting_existing_flow(): "type": "flow", "frame_type": "regular", "flow_id": "foo", - "step_id": "__start__", + "step_id": "START", "frame_id": "test", } ], @@ -161,7 +161,7 @@ def test_run_start_flow_interrupting_existing_flow(): assert isinstance(dialogue_stack_dump, list) and len(dialogue_stack_dump) == 2 assert dialogue_stack_dump[1]["frame_type"] == "interrupt" assert dialogue_stack_dump[1]["flow_id"] == "bar" - assert dialogue_stack_dump[1]["step_id"] == "__start__" + assert dialogue_stack_dump[1]["step_id"] == "START" assert dialogue_stack_dump[1].get("frame_id") is not None diff --git a/tests/cdu/stack/frames/test_flow_frame.py b/tests/cdu/stack/frames/test_flow_frame.py index 3cf75fc4c96d..b07940fa8e77 100644 --- a/tests/cdu/stack/frames/test_flow_frame.py +++ b/tests/cdu/stack/frames/test_flow_frame.py @@ -6,7 +6,13 @@ UserFlowStackFrame, FlowStackFrameType, ) -from rasa.shared.core.flows.flow import ActionFlowStep, Flow, FlowLinks, FlowsList +from rasa.shared.core.flows.flow import ( + ActionFlowStep, + Flow, + FlowLinks, + FlowsList, + StepSequence, +) def test_flow_frame_type(): @@ -49,7 +55,12 @@ def test_flow_stack_frame_type_from_str_none(): def test_flow_get_flow(): frame = UserFlowStackFrame(frame_id="test", flow_id="foo", step_id="bar") - flow = Flow(id="foo", steps=[], name="foo flow", description="foo flow description") + flow = Flow( + id="foo", + step_sequence=StepSequence(child_steps=[]), + name="foo flow", + description="foo flow description", + ) all_flows = FlowsList(flows=[flow]) assert frame.flow(all_flows) == flow @@ -59,7 +70,10 @@ def test_flow_get_flow_non_existant_id(): all_flows = FlowsList( flows=[ Flow( - id="foo", steps=[], name="foo flow", description="foo flow description" + id="foo", + step_sequence=StepSequence(child_steps=[]), + name="foo flow", + description="foo flow description", ) ] ) @@ -70,8 +84,9 @@ def test_flow_get_flow_non_existant_id(): def test_flow_get_step(): frame = UserFlowStackFrame(frame_id="test", flow_id="foo", step_id="my_step") step = ActionFlowStep( + idx=1, action="action_listen", - id="my_step", + custom_id="my_step", description=None, metadata={}, next=FlowLinks(links=[]), @@ -80,7 +95,7 @@ def test_flow_get_step(): flows=[ Flow( id="foo", - steps=[step], + step_sequence=StepSequence(child_steps=[step]), name="foo flow", description="foo flow description", ) @@ -94,7 +109,10 @@ def test_flow_get_step_non_existant_id(): all_flows = FlowsList( flows=[ Flow( - id="foo", steps=[], name="foo flow", description="foo flow description" + id="foo", + step_sequence=StepSequence(child_steps=[]), + name="foo flow", + description="foo flow description", ) ] ) @@ -107,7 +125,10 @@ def test_flow_get_step_non_existant_flow_id(): all_flows = FlowsList( flows=[ Flow( - id="foo", steps=[], name="foo flow", description="foo flow description" + id="foo", + step_sequence=StepSequence(child_steps=[]), + name="foo flow", + description="foo flow description", ) ] ) diff --git a/tests/cdu/stack/test_dialogue_stack.py b/tests/cdu/stack/test_dialogue_stack.py index a66e1aa93dc6..25769624eb0e 100644 --- a/tests/cdu/stack/test_dialogue_stack.py +++ b/tests/cdu/stack/test_dialogue_stack.py @@ -17,10 +17,10 @@ def test_dialogue_stack_from_dict(): }, { "type": "pattern_collect_information", - "collect_information": "foo", + "collect": "foo", "frame_id": "some-other-id", - "step_id": "__start__", - "flow_id": "pattern_ask_collect_information", + "step_id": "START", + "flow_id": "pattern_collect_information", "utter": "utter_ask_foo", }, ] @@ -32,7 +32,7 @@ def test_dialogue_stack_from_dict(): flow_id="foo", step_id="first_step", frame_id="some-frame-id" ) assert stack.frames[1] == CollectInformationPatternFlowStackFrame( - collect_information="foo", frame_id="some-other-id", utter="utter_ask_foo" + collect="foo", frame_id="some-other-id", utter="utter_ask_foo" ) @@ -48,7 +48,7 @@ def test_dialogue_stack_as_dict(): flow_id="foo", step_id="first_step", frame_id="some-frame-id" ), CollectInformationPatternFlowStackFrame( - collect_information="foo", + collect="foo", frame_id="some-other-id", utter="utter_ask_foo", ), @@ -65,10 +65,10 @@ def test_dialogue_stack_as_dict(): }, { "type": "pattern_collect_information", - "collect_information": "foo", + "collect": "foo", "frame_id": "some-other-id", - "step_id": "__start__", - "flow_id": "pattern_ask_collect_information", + "step_id": "START", + "flow_id": "pattern_collect_information", "rejections": None, "utter": "utter_ask_foo", }, @@ -100,7 +100,7 @@ def test_push_to_non_empty_stack(): flow_id="foo", step_id="first_step", frame_id="some-frame-id" ) pattern_frame = CollectInformationPatternFlowStackFrame( - collect_information="foo", frame_id="some-other-id" + collect="foo", frame_id="some-other-id" ) stack = DialogueStack(frames=[user_frame]) @@ -114,7 +114,7 @@ def test_push_to_index(): flow_id="foo", step_id="first_step", frame_id="some-frame-id" ) pattern_frame = CollectInformationPatternFlowStackFrame( - collect_information="foo", frame_id="some-other-id" + collect="foo", frame_id="some-other-id" ) stack = DialogueStack(frames=[user_frame]) @@ -154,7 +154,7 @@ def test_pop_frame(): flow_id="foo", step_id="first_step", frame_id="some-frame-id" ) pattern_frame = CollectInformationPatternFlowStackFrame( - collect_information="foo", frame_id="some-other-id" + collect="foo", frame_id="some-other-id" ) stack = DialogueStack(frames=[]) @@ -174,7 +174,7 @@ def test_top(): flow_id="foo", step_id="first_step", frame_id="some-frame-id" ) pattern_frame = CollectInformationPatternFlowStackFrame( - collect_information="foo", frame_id="some-other-id" + collect="foo", frame_id="some-other-id" ) stack = DialogueStack(frames=[]) @@ -193,7 +193,7 @@ def test_get_current_context(): flow_id="foo", step_id="first_step", frame_id="some-frame-id" ) pattern_frame = CollectInformationPatternFlowStackFrame( - collect_information="foo", frame_id="some-other-id", utter="utter_ask_foo" + collect="foo", frame_id="some-other-id", utter="utter_ask_foo" ) stack = DialogueStack(frames=[]) @@ -205,7 +205,7 @@ def test_get_current_context(): "frame_type": "regular", "step_id": "first_step", "type": "flow", - "collect_information": "foo", + "collect": "foo", "utter": "utter_ask_foo", "rejections": None, } diff --git a/tests/cdu/stack/test_utils.py b/tests/cdu/stack/test_utils.py index 984be9ffc3b7..4d7cf485522f 100644 --- a/tests/cdu/stack/test_utils.py +++ b/tests/cdu/stack/test_utils.py @@ -15,7 +15,7 @@ def test_top_flow_frame_ignores_pattern(): pattern_frame = CollectInformationPatternFlowStackFrame( - collect_information="foo", frame_id="some-other-id" + collect="foo", frame_id="some-other-id" ) user_frame = UserFlowStackFrame( flow_id="foo", step_id="first_step", frame_id="some-frame-id" @@ -32,7 +32,7 @@ def test_top_flow_frame_ignores_pattern(): def test_top_flow_frame_uses_pattern(): pattern_frame = CollectInformationPatternFlowStackFrame( - collect_information="foo", frame_id="some-other-id" + collect="foo", frame_id="some-other-id" ) user_frame = UserFlowStackFrame( flow_id="foo", step_id="first_step", frame_id="some-frame-id" @@ -51,7 +51,7 @@ def test_top_flow_frame_handles_empty(): def test_top_user_flow_frame(): pattern_frame = CollectInformationPatternFlowStackFrame( - collect_information="foo", frame_id="some-other-id" + collect="foo", frame_id="some-other-id" ) user_frame = UserFlowStackFrame( flow_id="foo", step_id="first_step", frame_id="some-frame-id" @@ -68,7 +68,7 @@ def test_top_user_flow_frame_handles_empty(): def test_user_flows_on_the_stack(): pattern_frame = CollectInformationPatternFlowStackFrame( - collect_information="foo", frame_id="some-other-id" + collect="foo", frame_id="some-other-id" ) user_frame = UserFlowStackFrame( flow_id="foo", step_id="first_step", frame_id="some-frame-id" @@ -94,13 +94,13 @@ def test_filled_slots_for_active_flow(): name: foo flow steps: - id: collect_foo - collect_information: foo + collect: foo next: collect_bar - id: collect_bar - collect_information: bar + collect: bar next: collect_baz - id: collect_baz - collect_information: baz + collect: baz """ ) @@ -120,13 +120,13 @@ def test_filled_slots_for_active_flow_handles_empty(): name: foo flow steps: - id: collect_foo - collect_information: foo + collect: foo next: collect_bar - id: collect_bar - collect_information: bar + collect: bar next: collect_baz - id: collect_baz - collect_information: baz + collect: baz """ ) @@ -142,13 +142,13 @@ def test_filled_slots_for_active_flow_skips_chitchat(): name: foo flow steps: - id: collect_foo - collect_information: foo + collect: foo next: collect_bar - id: collect_bar - collect_information: bar + collect: bar next: collect_baz - id: collect_baz - collect_information: baz + collect: baz """ ) @@ -169,24 +169,24 @@ def test_filled_slots_for_active_flow_only_collects_till_top_most_user_flow_fram name: foo flow steps: - id: collect_foo - collect_information: foo + collect: foo next: collect_bar - id: collect_bar - collect_information: bar + collect: bar next: collect_baz - id: collect_baz - collect_information: baz + collect: baz my_other_flow: name: foo flow steps: - id: collect_foo2 - collect_information: foo2 + collect: foo2 next: collect_bar2 - id: collect_bar2 - collect_information: bar2 + collect: bar2 next: collect_baz2 - id: collect_baz2 - collect_information: baz2 + collect: baz2 """ ) diff --git a/tests/core/actions/test_action_run_slot_rejections.py b/tests/core/actions/test_action_run_slot_rejections.py index c8e97d8521b2..9e535f2c9406 100644 --- a/tests/core/actions/test_action_run_slot_rejections.py +++ b/tests/core/actions/test_action_run_slot_rejections.py @@ -119,9 +119,9 @@ async def test_action_run_slot_rejections_top_frame_none_rejections( }, { "frame_id": "6Z7PSTRM", - "flow_id": "pattern_ask_collect_information", + "flow_id": "pattern_collect_information", "step_id": "start", - "collect_information": "payment_recipient", + "collect": "payment_recipient", "utter": "utter_ask_payment_recipient", "rejections": [], "type": "pattern_collect_information", @@ -167,9 +167,9 @@ async def test_action_run_slot_rejections_top_frame_slot_not_been_set( }, { "frame_id": "6Z7PSTRM", - "flow_id": "pattern_ask_collect_information", + "flow_id": "pattern_collect_information", "step_id": "start", - "collect_information": "recurrent_payment_type", + "collect": "recurrent_payment_type", "utter": "utter_ask_recurrent_payment_type", "rejections": [ { @@ -221,9 +221,9 @@ async def test_action_run_slot_rejections_run_success( }, { "frame_id": "6Z7PSTRM", - "flow_id": "pattern_ask_collect_information", + "flow_id": "pattern_collect_information", "step_id": "start", - "collect_information": "recurrent_payment_type", + "collect": "recurrent_payment_type", "utter": "utter_ask_recurrent_payment_type", "rejections": [ { @@ -285,9 +285,9 @@ async def test_action_run_slot_rejections_internal_error( }, { "frame_id": "6Z7PSTRM", - "flow_id": "pattern_ask_collect_information", + "flow_id": "pattern_collect_information", "step_id": "start", - "collect_information": "recurrent_payment_type", + "collect": "recurrent_payment_type", "utter": "utter_ask_recurrent_payment_type", "rejections": [ { @@ -346,9 +346,9 @@ async def test_action_run_slot_rejections_collect_missing_utter( }, { "frame_id": "6Z7PSTRM", - "flow_id": "pattern_ask_collect_information", + "flow_id": "pattern_collect_information", "step_id": "start", - "collect_information": "recurrent_payment_type", + "collect": "recurrent_payment_type", "utter": "utter_ask_recurrent_payment_type", "rejections": [ { @@ -404,9 +404,9 @@ async def test_action_run_slot_rejections_not_found_utter( }, { "frame_id": "6Z7PSTRM", - "flow_id": "pattern_ask_collect_information", + "flow_id": "pattern_collect_information", "step_id": "start", - "collect_information": "recurrent_payment_type", + "collect": "recurrent_payment_type", "utter": "utter_ask_recurrent_payment_type", "rejections": [ { @@ -462,9 +462,9 @@ async def test_action_run_slot_rejections_pass_multiple_rejection_checks( }, { "frame_id": "6Z7PSTRM", - "flow_id": "pattern_ask_collect_information", + "flow_id": "pattern_collect_information", "step_id": "start", - "collect_information": "payment_amount", + "collect": "payment_amount", "utter": "utter_ask_payment_amount", "rejections": [ { @@ -521,9 +521,9 @@ async def test_action_run_slot_rejections_fails_multiple_rejection_checks( }, { "frame_id": "6Z7PSTRM", - "flow_id": "pattern_ask_collect_information", + "flow_id": "pattern_collect_information", "step_id": "start", - "collect_information": "payment_amount", + "collect": "payment_amount", "utter": "utter_ask_payment_amount", "rejections": [ {