Merge pull request #594

drazvan · web-flow · commit bb7e7c41398d · 2024-07-04T12:04:05.000+03:00
Add more information to Colang syntax errors
diff --git a/nemoguardrails/actions/action_dispatcher.py b/nemoguardrails/actions/action_dispatcher.py
@@ -236,7 +236,17 @@ async def execute_action(
                     raise e
 
                 except Exception as e:
-                    log.warning(f"Error while execution {action_name}: {e}")
+                    filtered_params = {
+                        k: v
+                        for k, v in params.items()
+                        if k not in ["state", "events", "llm"]
+                    }
+                    log.warning(
+                        "Error while execution '%s' with parameters '%s': %s",
+                        action_name,
+                        filtered_params,
+                        e,
+                    )
                     log.exception(e)
 
         return None, "failed"
diff --git a/nemoguardrails/actions/v2_x/generation.py b/nemoguardrails/actions/v2_x/generation.py
@@ -653,7 +653,10 @@ async def generate_value(
 
         log.info("Generated value for $%s: %s", var_name, value)
 
-        return literal_eval(value)
+        try:
+            return literal_eval(value)
+        except Exception:
+            raise Exception(f"Invalid LLM response: `{value}`")
 
     @action(name="GenerateFlowAction", is_system_action=True, execute_async=True)
     async def generate_flow(
diff --git a/nemoguardrails/colang/v2_x/lang/expansion.py b/nemoguardrails/colang/v2_x/lang/expansion.py
@@ -55,49 +55,69 @@ def expand_elements(
         elements_changed = False
         new_elements: List[ElementType] = []
         for element in elements:
-            expanded_elements: List[ElementType] = []
-            if isinstance(element, SpecOp):
-                if element.op == "send":
-                    expanded_elements = _expand_send_element(element)
-                elif element.op == "match":
-                    expanded_elements = _expand_match_element(element)
-                elif element.op == "start":
-                    expanded_elements = _expand_start_element(element)
-                elif element.op == "stop":
-                    expanded_elements = _expand_stop_element(element)
-                elif element.op == "activate":
-                    expanded_elements = _expand_activate_element(element)
-                elif element.op == "await":
-                    expanded_elements = _expand_await_element(element)
-            elif isinstance(element, Assignment):
-                expanded_elements = _expand_assignment_stmt_element(element)
-            elif isinstance(element, While):
-                expanded_elements = _expand_while_stmt_element(element, flow_configs)
-            elif isinstance(element, If):
-                expanded_elements = _expand_if_element(element, flow_configs)
-                elements_changed = True  # Makes sure to update continue/break elements
-            elif isinstance(element, When):
-                expanded_elements = _expand_when_stmt_element(element, flow_configs)
-                elements_changed = True  # Makes sure to update continue/break elements
-            elif isinstance(element, Continue):
-                if element.label is None and continue_break_labels is not None:
-                    element.label = continue_break_labels[0]
-            elif isinstance(element, Break):
-                if element.label is None and continue_break_labels is not None:
-                    element.label = continue_break_labels[1]
-
-            if len(expanded_elements) > 0:
-                # Map new elements to source
-                for expanded_element in expanded_elements:
-                    if isinstance(expanded_element, Element) and isinstance(
-                        element, Element
-                    ):
-                        expanded_element._source = element._source
-                # Add new elements
-                new_elements.extend(expanded_elements)
-                elements_changed = True
-            else:
-                new_elements.extend([element])
+            try:
+                expanded_elements: List[ElementType] = []
+                if isinstance(element, SpecOp):
+                    if element.op == "send":
+                        expanded_elements = _expand_send_element(element)
+                    elif element.op == "match":
+                        expanded_elements = _expand_match_element(element)
+                    elif element.op == "start":
+                        expanded_elements = _expand_start_element(element)
+                    elif element.op == "stop":
+                        expanded_elements = _expand_stop_element(element)
+                    elif element.op == "activate":
+                        expanded_elements = _expand_activate_element(element)
+                    elif element.op == "await":
+                        expanded_elements = _expand_await_element(element)
+                elif isinstance(element, Assignment):
+                    expanded_elements = _expand_assignment_stmt_element(element)
+                elif isinstance(element, While):
+                    expanded_elements = _expand_while_stmt_element(
+                        element, flow_configs
+                    )
+                elif isinstance(element, If):
+                    expanded_elements = _expand_if_element(element, flow_configs)
+                    elements_changed = (
+                        True  # Makes sure to update continue/break elements
+                    )
+                elif isinstance(element, When):
+                    expanded_elements = _expand_when_stmt_element(element, flow_configs)
+                    elements_changed = (
+                        True  # Makes sure to update continue/break elements
+                    )
+                elif isinstance(element, Continue):
+                    if element.label is None and continue_break_labels is not None:
+                        element.label = continue_break_labels[0]
+                elif isinstance(element, Break):
+                    if element.label is None and continue_break_labels is not None:
+                        element.label = continue_break_labels[1]
+
+                if len(expanded_elements) > 0:
+                    # Map new elements to source
+                    for expanded_element in expanded_elements:
+                        if isinstance(expanded_element, Element) and isinstance(
+                            element, Element
+                        ):
+                            expanded_element._source = element._source
+                    # Add new elements
+                    new_elements.extend(expanded_elements)
+                    elements_changed = True
+                else:
+                    new_elements.extend([element])
+
+            except Exception as e:
+                error = "Error"
+                if e.args[0]:
+                    error = e.args[0]
+
+                if hasattr(element, "_source") and element._source:
+                    # TODO: Resolve source line to Colang file level
+                    raise ColangSyntaxError(
+                        error + f" on source line {element._source.line}"
+                    )
+                else:
+                    raise ColangSyntaxError(error)
 
         elements = new_elements
     return elements
@@ -476,7 +496,7 @@ def _expand_await_element(
             )
         else:
             raise ColangSyntaxError(
-                f"Unsupported spec type '{type(element.spec)}', element '{element.spec.name}' on line {element._source.line}"
+                f"Unsupported spec type '{type(element.spec)}', element '{element.spec.name}'"
             )
     else:
         # Element group
@@ -631,7 +651,7 @@ def _expand_activate_element(
         else:
             # It's an UMIM event
             raise ColangSyntaxError(
-                f"Only flows can be activated but not '{element.spec.spec_type}', element '{element.spec.name}' on line {element._source.line}!"
+                f"Only flows can be activated but not '{element.spec.spec_type}', element '{element.spec.name}'"
             )
     elif isinstance(element.spec, dict):
         # Multiple match elements
diff --git a/nemoguardrails/colang/v2_x/lang/parser.py b/nemoguardrails/colang/v2_x/lang/parser.py
@@ -137,27 +137,20 @@ def _contains_exclude_from_llm_tag(self, content: str) -> bool:
 
 
 def parse_colang_file(
-    _filename: str, content: str, include_source_mapping: bool = True
+    filename: str, content: str, include_source_mapping: bool = True
 ) -> dict:
     """Parse the content of a .co."""
 
     colang_parser = ColangParser(include_source_mapping=include_source_mapping)
     result = colang_parser.parse_content(content, print_tokens=False)
 
-    # flows = []
-    # for flow_data in result["flows"]:
-    #     # elements = parse_flow_elements(items)
-    #     # TODO: extract the source code here
-    #     source_code = ""
-    #     flows.append(
-    #         {
-    #             "id": flow_data["name"],
-    #             "elements": flow_data["elements"],
-    #             "source_code": source_code,
-    #         }
-    #     )
-
-    data = {"flows": result["flows"], "import_paths": result.get("import_paths", [])}
+    for flow in result["flows"]:
+        flow["file_info"]["name"] = filename
+
+    data = {
+        "flows": result["flows"],
+        "import_paths": result.get("import_paths", []),
+    }
 
     return data
 
diff --git a/nemoguardrails/colang/v2_x/lang/utils.py b/nemoguardrails/colang/v2_x/lang/utils.py
@@ -26,3 +26,10 @@ def dataclass_to_dict(obj: Any) -> Any:
         return {k: dataclass_to_dict(v) for k, v in obj.items()}
     else:
         return obj
+
+
+def format_colang_parsing_error_message(exception, colang_content):
+    """Improves readability of Colang error messages."""
+    line = colang_content.splitlines()[exception.line - 1]
+    marker = " " * (exception.column - 1) + "^"
+    return f"{exception}:\n{line}\n{marker}"
diff --git a/nemoguardrails/colang/v2_x/runtime/eval.py b/nemoguardrails/colang/v2_x/runtime/eval.py
@@ -183,7 +183,7 @@ def eval_expression(expr: str, context: dict) -> Any:
 
         return result
     except Exception as e:
-        raise ColangValueError(f"Error evaluating '{expr}'") from e
+        raise ColangValueError(f"Error evaluating '{expr}', {e}")
 
 
 def _create_regex(pattern: str) -> re.Pattern:
diff --git a/nemoguardrails/colang/v2_x/runtime/flows.py b/nemoguardrails/colang/v2_x/runtime/flows.py
@@ -372,6 +372,9 @@ class FlowConfig:
     # The actual source code, if available
     source_code: Optional[str] = None
 
+    # The name of the source code file
+    source_file: Optional[str] = None
+
     @property
     def loop_id(self) -> Optional[str]:
         """Return the interaction loop id if set."""
diff --git a/nemoguardrails/colang/v2_x/runtime/runtime.py b/nemoguardrails/colang/v2_x/runtime/runtime.py
@@ -27,6 +27,7 @@
 from nemoguardrails.colang import parse_colang_file
 from nemoguardrails.colang.runtime import Runtime
 from nemoguardrails.colang.v2_x.lang.colang_ast import Decorator, Flow
+from nemoguardrails.colang.v2_x.lang.utils import format_colang_parsing_error_message
 from nemoguardrails.colang.v2_x.runtime.errors import (
     ColangRuntimeError,
     ColangSyntaxError,
@@ -82,38 +83,44 @@ async def _add_flows_action(self, state: "State", **args: dict) -> List[str]:
                 version="2.x",
                 include_source_mapping=True,
             )
-        except Exception as e:
-            log.warning("Failed parsing a generated flow\n%s\n%s", flow_content, e)
-            return []
-            # Alternatively, we could through an exceptions
-            # raise ColangRuntimeError(f"Could not parse the generated Colang code! {ex}")
-
-        added_flows: List[str] = []
-        for flow in parsed_flow["flows"]:
-            if flow.name in state.flow_configs:
-                log.warning("Flow '%s' already exists! Not loaded!", flow.name)
-                break
-
-            flow_config = FlowConfig(
-                id=flow.name,
-                elements=expand_elements(flow.elements, state.flow_configs),
-                decorators=convert_decorator_list_to_dictionary(flow.decorators),
-                parameters=flow.parameters,
-                return_members=flow.return_members,
-                source_code=flow.source_code,
-            )
 
-            # Print out expanded flow elements
-            # json.dump(flow_config, sys.stdout, indent=4, cls=EnhancedJsonEncoder)
+            added_flows: List[str] = []
+            for flow in parsed_flow["flows"]:
+                if flow.name in state.flow_configs:
+                    log.warning("Flow '%s' already exists! Not loaded!", flow.name)
+                    break
+
+                flow_config = FlowConfig(
+                    id=flow.name,
+                    elements=expand_elements(flow.elements, state.flow_configs),
+                    decorators=convert_decorator_list_to_dictionary(flow.decorators),
+                    parameters=flow.parameters,
+                    return_members=flow.return_members,
+                    source_code=flow.source_code,
+                )
+
+                # Alternatively, we could through an exceptions
+                # raise ColangRuntimeError(f"Could not parse the generated Colang code! {ex}")
 
-            initialize_flow(state, flow_config)
+                # Print out expanded flow elements
+                # json.dump(flow_config, sys.stdout, indent=4, cls=EnhancedJsonEncoder)
 
-            # Add flow config to state.flow_configs
-            state.flow_configs.update({flow.name: flow_config})
+                initialize_flow(state, flow_config)
 
-            added_flows.append(flow.name)
+                # Add flow config to state.flow_configs
+                state.flow_configs.update({flow.name: flow_config})
 
-        return added_flows
+                added_flows.append(flow.name)
+
+            return added_flows
+
+        except Exception as e:
+            log.warning(
+                "Failed parsing a generated flow\n%s\n%s",
+                flow_content,
+                format_colang_parsing_error_message(e, flow_content),
+            )
+            return []
 
     async def _remove_flows_action(self, state: "State", **args: dict) -> None:
         log.info("Start RemoveFlowsAction! %s", args)
@@ -499,7 +506,7 @@ async def process_events(
                         run_to_completion(state, new_event)
                         new_event = None
                     except Exception as e:
-                        log.warning("Colang error!", exc_info=True)
+                        log.warning("Colang runtime error!", exc_info=True)
                         new_event = Event(
                             name="ColangError",
                             arguments={
@@ -708,6 +715,7 @@ def create_flow_configs_from_flow_list(flows: List[Flow]) -> Dict[str, FlowConfi
             parameters=flow.parameters,
             return_members=flow.return_members,
             source_code=flow.source_code,
+            source_file=flow.file_info["name"],
         )
 
         if config.is_override:
diff --git a/nemoguardrails/colang/v2_x/runtime/statemachine.py b/nemoguardrails/colang/v2_x/runtime/statemachine.py
@@ -49,6 +49,7 @@
 from nemoguardrails.colang.v2_x.lang.expansion import expand_elements
 from nemoguardrails.colang.v2_x.runtime.errors import (
     ColangRuntimeError,
+    ColangSyntaxError,
     ColangValueError,
 )
 from nemoguardrails.colang.v2_x.runtime.eval import (
@@ -87,9 +88,15 @@ def initialize_state(state: State) -> None:
 
     state.flow_states = dict()
 
-    # TODO: Think about where to put this
-    for flow_config in state.flow_configs.values():
-        initialize_flow(state, flow_config)
+    try:
+        # TODO: Think about where to put this
+        for flow_config in state.flow_configs.values():
+            initialize_flow(state, flow_config)
+    except Exception as e:
+        if e.args[0]:
+            raise ColangSyntaxError(e.args[0] + f" (flow `{flow_config.id}`)")
+        else:
+            raise ColangSyntaxError() from e
 
     # Create main flow state first
     main_flow_config = state.flow_configs["main"]
@@ -920,9 +927,16 @@ def _advance_head_front(state: State, heads: List[FlowHead]) -> List[FlowHead]:
                     actionable_heads.append(head)
         except Exception as e:
             # In case there were any runtime error the flow will be aborted (fail)
+            source_line = "unknown"
+            element = flow_config.elements[head.position]
+            if hasattr(element, "_source") and element._source:
+                source_line = str(element._source.line)
             log.warning(
-                "Colang error: Flow '%s' failed due to runtime exception!",
+                "Flow '%s' failed on line %s (%s) due to Colang runtime exception: %s",
                 flow_state.flow_id,
+                source_line,
+                flow_config.source_file,
+                e,
                 exc_info=True,
             )
             colang_error_event = Event(
diff --git a/nemoguardrails/rails/llm/config.py b/nemoguardrails/rails/llm/config.py
@@ -24,6 +24,7 @@
 
 from nemoguardrails.colang import parse_colang_file, parse_flow_elements
 from nemoguardrails.colang.v2_x.lang.colang_ast import Flow
+from nemoguardrails.colang.v2_x.lang.utils import format_colang_parsing_error_message
 from nemoguardrails.colang.v2_x.runtime.errors import ColangParsingError
 
 log = logging.getLogger(__name__)
@@ -636,12 +637,14 @@ def _parse_colang_files_recursively(
 
         with open(current_path, "r", encoding="utf-8") as f:
             try:
+                content = f.read()
                 _parsed_config = parse_colang_file(
-                    current_file, content=f.read(), version=colang_version
+                    current_file, content=content, version=colang_version
                 )
             except Exception as e:
                 raise ColangParsingError(
-                    f"Error while parsing Colang file: {current_path}"
+                    f"Error while parsing Colang file: {current_path}\n"
+                    + format_colang_parsing_error_message(e, content)
                 ) from e
 
             # We join only the "import_paths" field in the config for now
diff --git a/tests/colang/parser/v2_x/test_basic.py b/tests/colang/parser/v2_x/test_basic.py
