refactoring parser.correctness and adding context to check_correctnessand and check_parse_correctness error messages

Author: telmomenezes

src/hyperbase/builders.py

@@ -6,7 +6,7 @@
 
 from hyperbase.constants import ATOM_ENCODE_TABLE
 from hyperbase.hyperedge import Atom, Hyperedge, UniqueAtom
-from hyperbase.parsers.parse_result import ParseResult
+from hyperbase.parsers.result import ParseResult
 
 
 def str_to_atom(s: str) -> str:

src/hyperbase/cli/repl.py

@@ -25,16 +25,16 @@
 from hyperbase.constants import EdgeType
 from hyperbase.hyperedge import Atom, Hyperedge
 from hyperbase.parsers import Parser, get_parser, list_parsers
-from hyperbase.parsers.badness import badness_check
-from hyperbase.parsers.parse_result import ParseResult
-from hyperbase.parsers.repl_api import (
+from hyperbase.parsers.correctness import check_parse_correctness
+from hyperbase.parsers.repl import (
     CommandHandler,
     DiagnosticsProvider,
     PostResultHook,
     PreResultHook,
     ReplContext,
     StatsProvider,
 )
+from hyperbase.parsers.result import ParseResult
 
 SETTINGS_FILE = Path.home() / ".hyperbase_repl_settings.json"
 
@@ -2001,7 +2001,7 @@ def _render_badness(self, ctx: ReplContext) -> None:
         _edge = hedge(ctx.edge)
         if _edge is None:
             return
-        badness_errors = badness_check(_edge, ctx.tokens)
+        badness_errors = check_parse_correctness(_edge, ctx.tokens)
 
         self.console.print()
         if not badness_errors:

src/hyperbase/correctness.py

@@ -10,15 +10,19 @@
     from hyperbase.hyperedge import Atom, Hyperedge
 
 
-def check_correctness(edge: Hyperedge) -> dict[Hyperedge, list[tuple[str, str]]]:
-    """Check correctness of a hyperedge, returning errors keyed by subedge."""
+def check_correctness(edge: Hyperedge) -> dict[Hyperedge, list[tuple[str, str, int]]]:
+    """Check correctness of a hyperedge, returning errors keyed by subedge.
+
+    Each error is ``(code, message, severity)``. Correctness failures are hard
+    grammar violations, so they all carry severity ``0`` (the most serious).
+    """
     if edge.atom:
         return _check_atom(edge)  # type: ignore[arg-type]
     return _check_edge(edge)
 
 
-def _check_atom(atom: Atom) -> dict[Hyperedge, list[tuple[str, str]]]:
-    output: dict[Hyperedge, list[tuple[str, str]]] = {}
+def _check_atom(atom: Atom) -> dict[Hyperedge, list[tuple[str, str, int]]]:
+    output: dict[Hyperedge, list[tuple[str, str, int]]] = {}
     errors: list[tuple[str, str]] = []
 
     at = atom.mtype()
@@ -30,16 +34,16 @@ def _check_atom(atom: Atom) -> dict[Hyperedge, list[tuple[str, str]]]:
         EdgeType.TRIGGER,
         EdgeType.CONJUNCTION,
     }:
-        errors.append(("bad-atom-type", f"{at} is not a valid atom type"))
+        errors.append(("bad-atom-type", f"atom '{atom}' has invalid type {at}"))
 
     if len(errors) > 0:
-        output[atom] = errors
+        output[atom] = [(code, msg, 0) for code, msg in errors]
 
     return output
 
 
-def _check_edge(edge: Hyperedge) -> dict[Hyperedge, list[tuple[str, str]]]:
-    output: dict[Hyperedge, list[tuple[str, str]]] = {}
+def _check_edge(edge: Hyperedge) -> dict[Hyperedge, list[tuple[str, str, int]]]:
+    output: dict[Hyperedge, list[tuple[str, str, int]]] = {}
     errors: list[tuple[str, str]] = []
 
     ct = edge[0].mtype()
@@ -51,40 +55,54 @@ def _check_edge(edge: Hyperedge) -> dict[Hyperedge, list[tuple[str, str]]]:
         EdgeType.TRIGGER,
         EdgeType.CONJUNCTION,
     }:
-        errors.append(("conn-bad-type", f"connector has incorrect type: {ct}"))
+        errors.append(
+            (
+                "conn-bad-type",
+                f"connector '{edge[0]}' of '{edge}' has incorrect type: {ct}",
+            )
+        )
     # check if modifier structure is correct
     if ct == EdgeType.MODIFIER:
         if len(edge) != 2:
-            errors.append(("mod-1-arg", "modifiers can only have one argument"))
+            errors.append(
+                ("mod-1-arg", f"modifier edge '{edge}' can only have one argument")
+            )
     # check if builder structure is correct
     elif ct == EdgeType.BUILDER:
         if len(edge) != 3:
-            errors.append(("build-2-args", "builders can only have two arguments"))
+            errors.append(
+                ("build-2-args", f"builder edge '{edge}' can only have two arguments")
+            )
         for arg in edge[1:]:
             at = arg.mtype()
             if at != EdgeType.CONCEPT:
-                e = f"builder argument {arg!s} has incorrect type: {at}"
+                e = f"builder argument '{arg}' of '{edge}' has incorrect type: {at}"
                 errors.append(("build-arg-bad-type", e))
     # check if trigger structure is correct
     elif ct == EdgeType.TRIGGER:
         if len(edge) != 2:
-            errors.append(("trig-1-arg", "triggers can only have one arguments"))
+            errors.append(
+                ("trig-1-arg", f"trigger edge '{edge}' can only have one argument")
+            )
         for arg in edge[1:]:
             at = arg.mtype()
             if at not in {EdgeType.CONCEPT, EdgeType.RELATION}:
-                e = f"trigger argument {arg!s} has incorrect type: {at}"
+                e = f"trigger argument '{arg}' of '{edge}' has incorrect type: {at}"
                 errors.append(("trig-bad-arg-type", e))
     # check if predicate structure is correct
     elif ct == EdgeType.PREDICATE:
         for arg in edge[1:]:
             at = arg.mtype()
             if at not in {EdgeType.CONCEPT, EdgeType.RELATION, EdgeType.SPECIFIER}:
-                e = f"predicate argument {arg!s} has incorrect type: {at}"
+                e = f"predicate argument '{arg}' of '{edge}' has incorrect type: {at}"
                 errors.append(("pred-arg-bad-type", e))
     # check if conjunction structure is correct
     elif ct == EdgeType.CONJUNCTION and len(edge) < 3:
         errors.append(
-            ("conj-2-args-min", "conjunctions must have at least two arguments")
+            (
+                "conj-2-args-min",
+                f"conjunction edge '{edge}' must have at least two arguments",
+            )
         )
 
     # check argrole counts
@@ -98,8 +116,8 @@ def _check_edge(edge: Hyperedge) -> dict[Hyperedge, list[tuple[str, str]]]:
                             errors.append(
                                 (
                                     "pred-bad-arg-role",
-                                    f"{ar} is not a valid argument role "
-                                    "for connector of type P",
+                                    f"'{ar}' is not a valid argument role for "
+                                    f"predicate connector '{edge[0]}' in '{edge}'",
                                 )
                             )
                 elif ct == EdgeType.BUILDER:
@@ -108,16 +126,17 @@ def _check_edge(edge: Hyperedge) -> dict[Hyperedge, list[tuple[str, str]]]:
                             errors.append(
                                 (
                                     "build-bad-arg-role",
-                                    f"{ar} is not a valid argument role "
-                                    "for connector of type B",
+                                    f"'{ar}' is not a valid argument role for "
+                                    f"builder connector '{edge[0]}' in '{edge}'",
                                 )
                             )
 
                 if len(ars) != len(edge) - 1:
                     errors.append(
                         (
                             "bad-num-argroles",
-                            "number of argroles must match number of arguments",
+                            f"number of argroles on connector '{edge[0]}' must match "
+                            f"number of arguments in '{edge}'",
                         )
                     )
 
@@ -127,24 +146,140 @@ def _check_edge(edge: Hyperedge) -> dict[Hyperedge, list[tuple[str, str]]]:
                         errors.append(
                             (
                                 f"argrole-{role}-1-max",
-                                f"argrole {role} can only be used once",
+                                f"argrole '{role}' can only be used once in '{edge}'",
                             )
                         )
             else:
                 errors.append(
                     (
                         "no-argroles",
-                        "Connectors of type P or B must have argument roles",
+                        f"connector '{edge[0]}' of '{edge}' (type P or B) must have "
+                        "argument roles",
                     )
                 )
         except RuntimeError:
             # malformed edges are detected elsewhere
             pass
 
     if len(errors) > 0:
-        output[edge] = errors
+        output[edge] = [(code, msg, 0) for code, msg in errors]
 
     for subedge in edge:
         output.update(check_correctness(subedge))
 
     return output
+
+
+def check_structural_quality(
+    edge: Hyperedge,
+) -> dict[Hyperedge, list[tuple[str, str, int]]]:
+    """Soft, edge-only quality checks on argroles, modifiers and junctions.
+
+    Unlike :func:`check_correctness` (hard grammar rules), these flag
+    structures that are valid but unidiomatic. The third tuple value is a
+    severity (lower is worse): ``2`` for argrole problems, ``3`` for modifier
+    and junction issues.
+    """
+    errors: dict[Hyperedge, list[tuple[str, str, int]]] = {}
+
+    def _visit(current_edge: Hyperedge) -> None:
+        if not current_edge or current_edge.atom:
+            return
+
+        current_errors: list[tuple[str, str, int]] = []
+
+        # Argrole checks
+        try:
+            ars = current_edge.argroles()
+            ar_counts: Counter[str] = Counter()
+            for ar in ars:
+                if ar not in "masox":
+                    current_errors.append(
+                        (
+                            "bad-argrole",
+                            f"Bad argument role '{ar}' on connector "
+                            f"'{current_edge[0]}' in '{current_edge}'. "
+                            "Should be one of 'masox'.",
+                            2,
+                        )
+                    )
+                ar_counts[ar] += 1
+        except Exception:
+            pass
+
+        # Modifier checks
+        try:
+            connector_type = current_edge[0].type()
+            if len(current_edge) >= 2:
+                target_mt = current_edge[1].mt
+                if connector_type in {"Ma", "Md", "Mq", "Mp"} and target_mt != "C":
+                    current_errors.append(
+                        (
+                            f"bad-{connector_type.lower()}-target",
+                            f"Modifier '{current_edge}' of type '{connector_type}' "
+                            "should only be applied to concepts (type 'C'), but its "
+                            f"target '{current_edge[1]}' has type '{target_mt}'.",
+                            3,
+                        )
+                    )
+                elif connector_type == "Mm" and target_mt != "P":
+                    current_errors.append(
+                        (
+                            "bad-mm-target",
+                            f"Modifier '{current_edge}' of type 'Mm' should only be "
+                            "applied to predicates (type 'P'), but its target "
+                            f"'{current_edge[1]}' has type '{target_mt}'.",
+                            3,
+                        )
+                    )
+                elif connector_type == "Mn" and target_mt not in {"C", "P"}:
+                    current_errors.append(
+                        (
+                            "bad-mn-target",
+                            f"Modifier '{current_edge}' of type 'Mn' should only be "
+                            "applied to concepts (type 'C') or predicates (type 'P'), "
+                            f"but its target '{current_edge[1]}' has type "
+                            f"'{target_mt}'.",
+                            3,
+                        )
+                    )
+        except Exception:
+            pass
+
+        # Junction checks
+        try:
+            if current_edge[0].mt == "J":
+                types = {child.mt for child in current_edge[1:]}
+                if str(current_edge[0]) == ":/J/.":
+                    if types != {"R"} and types != {"C"} and types != {"C", "R"}:
+                        current_errors.append(
+                            (
+                                "bad-colon-junction-types",
+                                f"Arguments of junction '{current_edge}' should "
+                                "ideally be of either type 'R' or 'C'.",
+                                3,
+                            )
+                        )
+                else:
+                    if len(types) > 1 and types != {"C", "R"}:
+                        current_errors.append(
+                            (
+                                "bad-junction-types",
+                                f"Arguments of junction '{current_edge}' should "
+                                "ideally be all of the same type or a combination of "
+                                "'R' and 'C' types.",
+                                3,
+                            )
+                        )
+        except Exception:
+            pass
+
+        if current_errors:
+            errors[current_edge] = current_errors
+
+        for child in current_edge:
+            _visit(child)
+
+    if edge:
+        _visit(edge)
+    return errors

src/hyperbase/hyperedge.py

@@ -428,7 +428,7 @@ def arguments_with_role(self, argrole: str) -> list[Hyperedge]:
                 edges.append(self[pos + 1])
         return edges
 
-    def check_correctness(self) -> dict[Hyperedge, list[tuple[str, str]]]:
+    def check_correctness(self) -> dict[Hyperedge, list[tuple[str, str, int]]]:
         from hyperbase.correctness import check_correctness
 
         return check_correctness(self)

src/hyperbase/loaders.py

@@ -7,7 +7,7 @@
 
 from hyperbase.builders import hedge
 from hyperbase.hyperedge import Hyperedge
-from hyperbase.parsers.parse_result import ParseResult
+from hyperbase.parsers.result import ParseResult
 
 
 def load_edges(

src/hyperbase/parsers/__init__.py

@@ -1,9 +1,9 @@
 from importlib.metadata import EntryPoint, entry_points
 from typing import Any
 
-from hyperbase.parsers.parse_result import ParseResult
 from hyperbase.parsers.parser import Parser
-from hyperbase.parsers.repl_api import ReplContext
+from hyperbase.parsers.repl import ReplContext
+from hyperbase.parsers.result import ParseResult
 
 
 def list_parsers() -> dict[str, EntryPoint]: