Fix review findings before 0.3.0rc3, and bump to 0.3.0rc3

README.md

@@ -17,13 +17,13 @@ tag. They depend on each other by version, so install the ones you use
 **and the packages they depend on** from the same tag:
 
 ```bash
-pip install "lambdafeedback-slr-parsing @ git+https://github.com/lambda-feedback/lambda-feedback-packages.git@v0.3.0-rc.2#subdirectory=slr_parsing"
-pip install "lambdafeedback-criteria @ git+https://github.com/lambda-feedback/lambda-feedback-packages.git@v0.3.0-rc.2#subdirectory=criteria"
-pip install "lambdafeedback-expression-parsing @ git+https://github.com/lambda-feedback/lambda-feedback-packages.git@v0.3.0-rc.2#subdirectory=expression_parsing"
-pip install "lambdafeedback-units @ git+https://github.com/lambda-feedback/lambda-feedback-packages.git@v0.3.0-rc.2#subdirectory=units"
+pip install "lambdafeedback-slr-parsing @ git+https://github.com/lambda-feedback/lambda-feedback-packages.git@v0.3.0-rc.3#subdirectory=slr_parsing"
+pip install "lambdafeedback-criteria @ git+https://github.com/lambda-feedback/lambda-feedback-packages.git@v0.3.0-rc.3#subdirectory=criteria"
+pip install "lambdafeedback-expression-parsing @ git+https://github.com/lambda-feedback/lambda-feedback-packages.git@v0.3.0-rc.3#subdirectory=expression_parsing"
+pip install "lambdafeedback-units @ git+https://github.com/lambda-feedback/lambda-feedback-packages.git@v0.3.0-rc.3#subdirectory=units"
 ```
 
-With Poetry: `lambdafeedback-units = { git = "https://github.com/lambda-feedback/lambda-feedback-packages.git", tag = "v0.3.0-rc.2", subdirectory = "units" }`, and likewise for each package.
+With Poetry: `lambdafeedback-units = { git = "https://github.com/lambda-feedback/lambda-feedback-packages.git", tag = "v0.3.0-rc.3", subdirectory = "units" }`, and likewise for each package.
 
 ## Packages
 

criteria/CHANGELOG.md

@@ -1,6 +1,6 @@
 # Changelog: lambdafeedback-criteria
 
-## 0.3.0 (unreleased; pre-release 0.3.0rc2, tag v0.3.0-rc.2)
+## 0.3.0 (unreleased; pre-release 0.3.0rc3, tag v0.3.0-rc.3)
 
 The first public release, after the extraction refactor. The import path is now `lambdafeedback.criteria` (was `criteria`). There was no 0.2.0 release: all four `lambdafeedback-*` packages share one version, and the first release is 0.3.0. Supports Python 3.12 to 3.14 (3.11 is not supported); `__version__` gives the installed version.
 
@@ -63,7 +63,7 @@ Behaviour differences to be aware of when adopting:
 ### Added
 
 - `build_quantity_criteria_parser(reserved_expressions, token_list=, productions=)` parses criteria on physical quantities (`response matches answer`, `dimension(response) matches dimension(answer)`, and `=`, `<=`, `>=`, `<`, `>` between quantities). Its defaults are `quantity_grammar.QUANTITY_TOKENS` / `QUANTITY_PRODUCTIONS`. It was `generate_criteria_parser` in compareExpressions' physical-quantity context, and its parse trees are unchanged. It takes the same arguments as `build_criteria_parser`, so either can fill the same slot. `reserved_expressions` isn't used yet: only `response` and `answer` scan as quantities.
-- `split_criteria(criteria)` splits a string of criteria on the commas outside `()`, `[]` and `{}`, and strips each criterion. It was `split_criteria` in compareExpressions' `evaluation.py`. Unlike that version, it raises `CriteriaSyntaxError` (new, a `CriteriaError` and `ValueError`) when a bracket is closed before it is opened, or left open. The old version stopped splitting at the first such bracket instead.
+- `split_criteria(criteria)` splits a string of criteria on the commas outside `()`, `[]` and `{}`, and strips each criterion. It was `split_criteria` in compareExpressions' `evaluation.py`. Any closing bracket closes any opening one, so a half-open interval keeps its comma: `response in [0, 1), response > 0` is two criteria. The old version counted each kind of bracket on its own and stopped splitting after `[0, 1)`, running the rest into one criterion. Unlike that version, it raises `CriteriaSyntaxError` (new, a `CriteriaError` and `ValueError`) when a bracket is closed when none is open, or left open. The old version stopped splitting at the first such bracket instead. It also raises `CriteriaSyntaxError` for an empty criterion (`response = answer,`, `a = b,, c = d` or an empty string), which the old version returned as `""` for the criteria parser to fail on.
 - `where_substitutions(substitutions)` gives `[(name, expression), ...]` for the substitution part of a `where` criterion (the second child of a `WHERE` node), with the expressions as text. It replaces the node-reading half of compareExpressions' `parse_where_substitutions`; the caller now parses the expressions, so this package doesn't need `expression_parsing`. The grammar also accepts other relations after `where` (`where x > 2`, `where x proportional to y`). compareExpressions' `where_substitution_nodes` returned the node itself for these, instead of a list, and this function raises `CriteriaSyntaxError` for them. A `SEPARATOR` that mixes in such a relation (`where a = 2; x > 3`) used to be read as substitutions, and also raises now.
 
 ### Changed

criteria/README.md

@@ -14,7 +14,7 @@ evaluation function.
 From git (not on PyPI yet):
 
 ```bash
-pip install "lambdafeedback-criteria @ git+https://github.com/lambda-feedback/lambda-feedback-packages.git@v0.3.0-rc.2#subdirectory=criteria"
+pip install "lambdafeedback-criteria @ git+https://github.com/lambda-feedback/lambda-feedback-packages.git@v0.3.0-rc.3#subdirectory=criteria"
 ```
 
 Install the packages it depends on (`slr_parsing`) from the same tag; see the [repository README](../README.md#install).

criteria/pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "poetry.core.masonry.api"
 
 [project]
 name = "lambdafeedback-criteria"
-version = "0.3.0rc2"
+version = "0.3.0rc3"
 description = "Criteria parser and criteria graphs that turn evaluations into feedback, for Lambda Feedback evaluation functions."
 readme = "README.md"
 license = "MIT"
@@ -25,7 +25,7 @@ classifiers = [
 ]
 requires-python = ">=3.12"
 dependencies = [
-    "lambdafeedback-slr-parsing (>=0.3.0rc2,<0.4.0)",
+    "lambdafeedback-slr-parsing (>=0.3.0rc3,<0.4.0)",
 ]
 
 [project.urls]

criteria/src/lambdafeedback/criteria/nodes.py

@@ -27,11 +27,6 @@
 
 NodeKind: TypeAlias = Literal["evaluation", "criterion", "output"]
 
-# Mermaid node shapes (opening, closing).
-EVALUATION_STYLE = ("([", "])")
-CRITERION_STYLE = ("[", "]")
-OUTPUT_STYLE = ("{{", "}}")
-
 
 class ResultLike(Protocol):
     """The part of a result object ``CriteriaGraph.export_feedback`` needs (``lf_toolkit.evaluation.Result`` fits)."""
@@ -91,7 +86,7 @@ class EvaluationNode(Node):
     """
 
     KIND: ClassVar[NodeKind | None] = "evaluation"
-    STYLE: ClassVar[tuple[str, str] | None] = EVALUATION_STYLE
+    STYLE: ClassVar[tuple[str, str] | None] = ("([", "])")
 
     evaluate: Evaluate | None = None
     replacement: str | None = None
@@ -119,7 +114,7 @@ class CriterionNode(Node):
     """An outcome of an evaluation, with the generator for its feedback text."""
 
     KIND: ClassVar[NodeKind | None] = "criterion"
-    STYLE: ClassVar[tuple[str, str] | None] = CRITERION_STYLE
+    STYLE: ClassVar[tuple[str, str] | None] = ("[", "]")
 
     feedback_string_generator: FeedbackStringGenerator = no_feedback
 
@@ -139,7 +134,7 @@ class OutputNode(Node):
     """A terminal node; output nodes have no outgoing edges."""
 
     KIND: ClassVar[NodeKind | None] = "output"
-    STYLE: ClassVar[tuple[str, str] | None] = OUTPUT_STYLE
+    STYLE: ClassVar[tuple[str, str] | None] = ("{{", "}}")
 
     def copy_into(self, graph: CriteriaGraph) -> OutputNode:
         return graph.add_output_node(self.label, self.summary, self.details)

criteria/src/lambdafeedback/criteria/render.py

@@ -6,7 +6,8 @@
 from collections.abc import Collection, Mapping
 from typing import TYPE_CHECKING, Any
 
-from .nodes import CRITERION_STYLE, EVALUATION_STYLE, OUTPUT_STYLE, EvaluationNode, Node
+from .errors import CriteriaGraphError
+from .nodes import EvaluationNode, Node
 
 if TYPE_CHECKING:
     from .graph import CriteriaGraph
@@ -52,20 +53,24 @@ def entry(node: Node, outgoing: bool = True) -> dict[str, Any]:
     )
 
 
+def _style(node: Node) -> tuple[str, str]:
+    """``node``'s mermaid shape (opening, closing): its kind's ``STYLE``."""
+    if node.STYLE is None:
+        raise CriteriaGraphError(f"Cannot display node {node.label} of type {type(node).__name__}.")
+    return node.STYLE
+
+
 def graph_to_mermaid(graph: CriteriaGraph) -> str:
     """Mermaid flowchart source for ``graph``; sufficiencies are drawn as dotted edges."""
-    node_groups: list[tuple[Mapping[str, Node], tuple[str, str]]] = [
-        (graph.evaluations, EVALUATION_STYLE),
-        (graph.criteria, CRITERION_STYLE),
-        (graph.outputs, OUTPUT_STYLE),
-    ]
-    keys = {label: f"N_{g}_{i}" for g, (nodes, _) in enumerate(node_groups) for i, label in enumerate(nodes)}
+    node_groups: list[Mapping[str, Node]] = [graph.evaluations, graph.criteria, graph.outputs]
+    keys = {label: f"N_{g}_{i}" for g, nodes in enumerate(node_groups) for i, label in enumerate(nodes)}
     lines = ["flowchart TD"]
     # Ordered de-duplication (dict keys), so the output doesn't depend on the hash seed.
     edges: dict[tuple[str, str], None] = {}
     sufficiencies: dict[tuple[str, str], None] = {}
-    for nodes, (opening, closing) in node_groups:
+    for nodes in node_groups:
         for label, node in nodes.items():
+            opening, closing = _style(node)
             lines.append(f'{keys[label]}{opening}"{_text(label)}<br/>---<br/>{_text(node.details)}"{closing}')
             edges.update(
                 dict.fromkeys((keys[e.source.label], keys[e.target.label]) for e in node.outgoing + node.incoming)

criteria/src/lambdafeedback/criteria/split.py

@@ -4,32 +4,40 @@
 
 from .errors import CriteriaSyntaxError
 
-_OPENING = {")": "(", "]": "[", "}": "{"}
-"""The opening bracket for each closing bracket."""
+_OPENING = frozenset("([{")
+_CLOSING = frozenset(")]}")
 
 
 def split_criteria(criteria: str) -> list[str]:
     """``criteria`` split on the commas outside brackets, with each criterion stripped.
 
-    Each kind of bracket (``()``, ``[]``, ``{}``) is counted on its own. Raises
-    :class:`CriteriaSyntaxError` if a bracket is closed before it is opened,
-    or left open.
+    Any closing bracket (``)``, ``]``, ``}``) closes any opening one, so
+    half-open intervals such as ``x in [0, 1)`` keep their comma. Raises
+    :class:`CriteriaSyntaxError` if a bracket is closed when none is open,
+    or left open, or if a criterion is empty (``a = b,`` or ``a = b,, c = d``).
     """
-    depth = dict.fromkeys(_OPENING.values(), 0)
+    depth = 0
     parts = []
     start = 0
     for index, char in enumerate(criteria):
-        if char in depth:
-            depth[char] += 1
-        elif char in _OPENING:
-            opening = _OPENING[char]
-            if depth[opening] == 0:
-                raise CriteriaSyntaxError(f"`{char}` at position {index} closes no `{opening}` in: {criteria}")
-            depth[opening] -= 1
-        elif char == "," and not any(depth.values()):
-            parts.append(criteria[start:index].strip())
+        if char in _OPENING:
+            depth += 1
+        elif char in _CLOSING:
+            if depth == 0:
+                raise CriteriaSyntaxError(f"`{char}` at position {index} closes no bracket in: {criteria}")
+            depth -= 1
+        elif char == "," and depth == 0:
+            parts.append(_criterion(criteria, start, index))
             start = index + 1
-    if any(depth.values()):
+    if depth:
         raise CriteriaSyntaxError(f"Unclosed bracket in: {criteria}")
-    parts.append(criteria[start:].strip())
+    parts.append(_criterion(criteria, start, len(criteria)))
     return parts
+
+
+def _criterion(criteria: str, start: int, end: int) -> str:
+    """``criteria[start:end]``, stripped; raises :class:`CriteriaSyntaxError` if that leaves nothing."""
+    criterion = criteria[start:end].strip()
+    if not criterion:
+        raise CriteriaSyntaxError(f"Empty criterion at position {start} in: {criteria}")
+    return criterion

criteria/tests/test_criteria.py

@@ -92,15 +92,23 @@ class TestSplitCriteria:
             ("response = f(x, y), response > 0", ["response = f(x, y)", "response > 0"]),
             ("response = [1, 2], response = {a, b}", ["response = [1, 2]", "response = {a, b}"]),
             ("response = f([x, {y, z}]), answer > 0", ["response = f([x, {y, z}])", "answer > 0"]),
-            ("response = answer,", ["response = answer", ""]),
-            ("", [""]),
+            ("response in [0, 1), response > 0", ["response in [0, 1)", "response > 0"]),
+            ("response in (0, 1], answer in (a, b]", ["response in (0, 1]", "answer in (a, b]"]),
         ],
     )
     def test_splits_on_commas_outside_brackets(self, criteria, expected):
         assert split_criteria(criteria) == expected
 
     @pytest.mark.parametrize(
-        "criteria", ["response = x), answer > 0", "response = (x, answer > 0", "response = (x], y"]
+        "criteria",
+        ["", "  ", "response = answer,", "response = answer, ", ", response > 0", "response = answer,, x > 0"],
+    )
+    def test_empty_criterion_raises(self, criteria):
+        with pytest.raises(CriteriaSyntaxError, match="Empty criterion"):
+            split_criteria(criteria)
+
+    @pytest.mark.parametrize(
+        "criteria", ["response = x), answer > 0", "response = (x, answer > 0", "response = [0, 1)), y", "f(x"]
     )
     def test_unbalanced_brackets_raise(self, criteria):
         with pytest.raises(CriteriaSyntaxError):
@@ -149,6 +157,16 @@ def test_mermaid_returns_string(self):
         g.add_evaluation_node("EVAL", "eval", "eval details")
         assert isinstance(g.to_mermaid(), str)
 
+    def test_mermaid_draws_each_kind_of_node_in_its_shape(self):
+        g = CriteriaGraph("root")
+        g.add_evaluation_node("EVAL", "eval", "eval details")
+        g.add_criterion_node("CRIT", "crit", "crit details")
+        g.add_output_node("OUT", "out", "out details")
+        lines = g.to_mermaid().split("\n\t")
+        assert 'N_0_0(["EVAL<br/>---<br/>eval details"])' in lines
+        assert 'N_1_0["CRIT<br/>---<br/>crit details"]' in lines
+        assert 'N_2_0{{"OUT<br/>---<br/>out details"}}' in lines
+
     def test_duplicate_evaluation_node_rejected(self):
         g = CriteriaGraph("root")
         g.add_evaluation_node("EVAL", "eval", "eval details")