fix(workflows): evaluate exponent numbers and chained indexing, reject unsupported syntax

docs/reference/workflows.md

@@ -1292,6 +1292,8 @@ Notes on the newer filters:
   condition: "{{ inputs.items | length }}"   # 0 is False, any non-zero count is True
   ```
 
+- **Unsupported syntax raises, it does not resolve to `None`.** Text the evaluator cannot parse — a backslash-escaped quote in a string literal (`'it\'s'`; quote such a literal with the other quote type, `"it's"`), or a word operator without a space on both sides (`'x'and inputs.b`) — raises a `ValueError` naming the unsupported syntax at run time, instead of being looked up as a variable, resolving to `None`, and silently steering a condition. A well-formed path that merely finds nothing (`inputs.missing`) still resolves to `None`: missing data is not a syntax fault. Numeric literals accept the exponent form (`1e3`), both as literals and when a numeric string is coerced in a comparison, and indexed paths may chain (`matrix[0][1]`).
+
 `and` and `or` return `true` or `false` and evaluate their right side only when the left side does not decide the result: `and` skips it when the left side is false, and `or` skips it when the left side is true. A condition can therefore guard a filter that would fail on an empty value. `{{ inputs.payload != '' and (inputs.payload | from_json | length) }}` is false for an empty optional `payload` instead of failing the run on invalid JSON. When the right side is evaluated, its errors still fail the run.
 
 Example:

src/specify_cli/workflows/expressions/engine_current.py

@@ -23,7 +23,7 @@
     _filter_map,
     _filter_split,
 )
-from .namespace import _NAMESPACE_ROOTS, _resolve_dot_path
+from .namespace import _NAMESPACE_ROOTS, _is_path_segment, _resolve_dot_path
 from .operators import _safe_compare, _safe_membership
 from .template import (
     _collapse_whitespace,
@@ -168,7 +168,11 @@ def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any:
     - Boolean operators: ``and``, ``or``, ``not``
     - ``in``, ``not in``
     - Pipe filters: ``| default('...')``, ``| join(', ')``, ``| contains('...')``, ``| from_json``, ``| map('...')``, ``| split(',')``, ``| upper``, ``| lower``, ``| length``, ``| to_json``
-    - String and numeric literals
+    - String and numeric literals (including the exponent form ``1e3``)
+
+    Text that is none of the above and is not a resolvable variable path
+    raises ``ValueError`` naming the unsupported syntax, rather than silently
+    resolving to ``None``.
     """
     expr = _collapse_whitespace(expr)
 
@@ -307,13 +311,20 @@ def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any:
             if op == " not in ":
                 return _safe_membership(left, right, negate=True)
 
-    # Numeric literal
+    # Numeric literal. int() first so integer text stays an int; float() then
+    # admits the decimal and exponent forms ("1.5", "1e3"), which the previous
+    # "." check missed -- `1e3` was looked up as a variable and became None
+    # (#5073). Only digit-bearing text may become a float, so the words
+    # float() also accepts ("inf", "nan") keep resolving as variable names.
     try:
-        if "." in expr:
-            return float(expr)
         return int(expr)
     except (ValueError, TypeError):
         pass
+    if any(ch.isdigit() for ch in expr):
+        try:
+            return float(expr)
+        except (ValueError, TypeError):
+            pass
 
     # Boolean literal
     if expr.lower() == "true":
@@ -346,6 +357,34 @@ def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any:
     sink = _leaf_sink.get()
     if sink is not None:
         sink.append(expr)
+    # Text that is not a resolvable variable path used to be looked up as a
+    # name anyway, resolve to None, and silently hand a condition the wrong
+    # answer -- `'it\'s'`, `'x'and inputs.b` (#5073). Raise at run time
+    # instead, naming the text, the way mis-wired filters already do. Probe
+    # runs are exempt: the leaf is recorded above, and the validation gate
+    # owns the authoring diagnostics (`_unresolvable_leaf`), so validation
+    # messages do not change.
+    if (
+        expr
+        and not isinstance(namespace, _ProbeNamespace)
+        and not all(_is_path_segment(seg.strip()) for seg in expr.split("."))
+    ):
+        hint = ""
+        if re.search(r"['\"](?:and|or|in|not)\b|\b(?:and|or|in|not)['\"]", expr):
+            hint = (
+                "; the word operators and/or/in/not need a space on both "
+                "sides to be recognized"
+            )
+        elif "'" in expr or '"' in expr:
+            hint = (
+                "; a string literal must be one wholly quoted string -- "
+                "backslash-escaped quotes are not supported, so quote a "
+                "literal containing a quote with the other quote type"
+            )
+        raise ValueError(
+            f"unsupported expression syntax: {expr!r} is not a supported "
+            f"literal, operator expression, or variable path{hint}"
+        )
     return _resolve_dot_path(namespace, expr)
 
 

src/specify_cli/workflows/expressions/namespace.py

@@ -16,7 +16,12 @@
 # against it, and the condition gate below reuses it rather than describing the
 # same shape a second time, so widening what indexing accepts cannot leave the
 # evaluator and the gate disagreeing.
-_INDEXED_SEGMENT = re.compile(r"^([\w-]+)\[(-?\d+)\]$")
+_INDEXED_SEGMENT = re.compile(r"^([\w-]+)((?:\[-?\d+\])+)$")
+
+# The integer tokens inside an indexed segment's bracket suffix. Parsed from the
+# matched text rather than a capture group so a widened _INDEXED_SEGMENT (the
+# wiring test swaps in one with a different group shape) still yields indices.
+_INDEX_TOKEN = re.compile(r"-?\d+")
 
 _PLAIN_SEGMENT = re.compile(r"^[\w-]+$")
 
@@ -25,23 +30,28 @@ def _resolve_dot_path(obj: Any, path: str) -> Any:
     """Resolve a dotted path like ``steps.specify.output.file`` against *obj*.
 
     Supports dict key access and list indexing, including the negative form
-    Python and Jinja2 both accept (e.g., ``task_list[0]``, ``task_list[-1]``).
+    Python and Jinja2 both accept (e.g., ``task_list[0]``, ``task_list[-1]``)
+    and the chained form (``matrix[0][1]``), which used to fall outside
+    ``_INDEXED_SEGMENT`` and be looked up as the literal key ``"matrix[0][1]"``
+    (#5073).
     """
     parts = path.split(".")
     current = obj
     for part in parts:
-        # Handle list indexing: name[0], name[-1]
+        # Handle list indexing: name[0], name[-1], name[0][1]
         idx_match = _INDEXED_SEGMENT.match(part)
         if idx_match:
-            key, idx = idx_match.group(1), int(idx_match.group(2))
+            key = idx_match.group(1)
             if isinstance(current, dict):
                 current = current.get(key)
             else:
                 return None
-            if isinstance(current, list) and -len(current) <= idx < len(current):
-                current = current[idx]
-            else:
-                return None
+            for token in _INDEX_TOKEN.findall(part[len(key):]):
+                idx = int(token)
+                if isinstance(current, list) and -len(current) <= idx < len(current):
+                    current = current[idx]
+                else:
+                    return None
         elif isinstance(current, dict):
             current = current.get(part)
         else:

src/specify_cli/workflows/expressions/operators.py

@@ -10,12 +10,25 @@
 
 
 def _coerce_number(value: Any) -> Any:
-    """Return *value* as int/float if it is a numeric string, else unchanged."""
+    """Return *value* as int/float if it is a numeric string, else unchanged.
+
+    ``int()`` first so integer text stays an int; ``float()`` then admits the
+    decimal and exponent forms (``"1.5"``, ``"1e3"``), matching what workflow
+    input coercion accepts as a number (``WorkflowEngine._coerce_input``), so
+    the same text cannot be a number as an input but a string in a comparison
+    (#5073). ``float()`` also accepts the words ``"inf"``/``"nan"``; those
+    carry no digit and stay ordinary strings here.
+    """
     if isinstance(value, str):
         try:
-            return float(value) if "." in value else int(value)
+            return int(value)
         except ValueError:
-            return value
+            pass
+        if any(ch.isdigit() for ch in value):
+            try:
+                return float(value)
+            except ValueError:
+                return value
     return value
 
 

tests/test_workflows.py

@@ -452,15 +452,19 @@ def test_list_literal_followed_by_index_is_not_misparsed_as_one_literal(self):
         outer brackets from ``[1,2,3][1]`` yields ``1,2,3][1``, which then
         silently evaluates to ``[1, 2, None]`` instead of raising or
         resolving the index. It must not be misclassified as one literal;
-        falling through to unresolvable (``None``) is safe, unlike silently
-        returning a wrong-looking list.
+        since #5073 the fall-through raises as unsupported syntax, which is
+        safe, unlike silently returning a wrong-looking list.
         """
+        import pytest
+
         from specify_cli.workflows.expressions import evaluate_expression
         from specify_cli.workflows.base import StepContext
 
         ctx = StepContext()
-        assert evaluate_expression("{{ [1,2,3][1] }}", ctx) is None
-        assert evaluate_expression("{{ [1,2][0] }}", ctx) is None
+        with pytest.raises(ValueError, match="unsupported expression syntax"):
+            evaluate_expression("{{ [1,2,3][1] }}", ctx)
+        with pytest.raises(ValueError, match="unsupported expression syntax"):
+            evaluate_expression("{{ [1,2][0] }}", ctx)
         # Genuine list literals -- including ones a bracket-depth scan must
         # still recognize as ending exactly at the final character -- are
         # unaffected.
@@ -504,6 +508,78 @@ def test_operator_splitting_is_quote_aware(self):
         assert evaluate_expression("{{ inputs.a == 9 or inputs.b == 2 }}", plain) is True
         assert evaluate_expression("{{ inputs.missing | default('a and b') }}", plain) == "a and b"
 
+    def test_exponent_numeric_literals_evaluate_as_numbers(self):
+        """#5073 case 1: `1e3` is a number, as in Python/Jinja2 and as workflow
+        input coercion already accepts. It used to be looked up as a variable
+        and become None, and the numeric string "1e3" stayed a string in
+        comparisons while coercing to 1000 as an input."""
+        from specify_cli.workflows.expressions import (
+            evaluate_condition,
+            evaluate_expression,
+        )
+        from specify_cli.workflows.base import StepContext
+
+        ctx = StepContext(inputs={"n": 5, "s": "1e3"})
+        assert evaluate_expression("{{ 1e3 }}", ctx) == 1000.0
+        assert evaluate_expression("{{ 2.5e-1 }}", ctx) == 0.25
+        assert evaluate_condition("{{ inputs.n < 1e3 }}", ctx) is True
+        # A numeric string in the exponent form coerces in comparisons the way
+        # plain "10" already does (_coerce_number).
+        assert evaluate_condition("{{ inputs.s > 100 }}", ctx) is True
+        # The words float() also accepts are still variable names, not numbers.
+        assert evaluate_expression("{{ inf }}", ctx) is None
+        assert evaluate_expression("{{ nan }}", ctx) is None
+        # Plain integers stay ints.
+        assert evaluate_expression("{{ 10 }}", ctx) == 10
+        assert isinstance(evaluate_expression("{{ 10 }}", ctx), int)
+
+    def test_chained_indexing_resolves_like_jinja(self):
+        """#5073 case 3: `matrix[0][1]` walks both indices. It used to fall
+        outside the one-index segment pattern and be looked up as the literal
+        key "matrix[0][1]", returning None."""
+        from specify_cli.workflows.expressions import evaluate_expression
+        from specify_cli.workflows.base import StepContext
+
+        ctx = StepContext(
+            steps={"plan": {"output": {"matrix": [["a", "b"], ["c", "d"]]}}}
+        )
+        assert evaluate_expression("{{ steps.plan.output.matrix[0] }}", ctx) == ["a", "b"]
+        assert evaluate_expression("{{ steps.plan.output.matrix[0][1] }}", ctx) == "b"
+        assert evaluate_expression("{{ steps.plan.output.matrix[-1][0] }}", ctx) == "c"
+        # An out-of-range index is missing data, not syntax: still None.
+        assert evaluate_expression("{{ steps.plan.output.matrix[0][9] }}", ctx) is None
+        assert evaluate_expression("{{ steps.plan.output.matrix[9][0] }}", ctx) is None
+
+    def test_unsupported_syntax_raises_instead_of_silent_none(self):
+        """#5073 cases 2 and 4: text the evaluator does not parse used to be
+        looked up as a variable path, resolve to None, and silently give a
+        condition the wrong answer. It now raises a ValueError naming the
+        unsupported syntax, the way mis-wired filters already do."""
+        import pytest
+
+        from specify_cli.workflows.expressions import evaluate_expression
+        from specify_cli.workflows.base import StepContext
+
+        ctx = StepContext(inputs={"name": "it's", "a": "x", "b": True})
+        for template in (
+            r"{{ 'it\'s' }}",                     # escaped quote in a literal
+            r"{{ inputs.name == 'it\'s' }}",
+            "{{ inputs.a=='x'and inputs.b }}",    # word operator without spaces
+            "{{ 'x'in inputs.a }}",
+        ):
+            with pytest.raises(ValueError, match="unsupported expression syntax"):
+                evaluate_expression(template, ctx)
+
+        # The error's advice works: the other quote type spells the literal.
+        assert evaluate_expression("{{ \"it's\" }}", ctx) == "it's"
+        assert evaluate_expression("{{ inputs.name == \"it's\" }}", ctx) is True
+        # Properly spaced operators are untouched...
+        assert evaluate_expression("{{ inputs.a == 'x' and inputs.b }}", ctx) is True
+        # ...and an unknown but well-formed name still resolves to None, not an
+        # error: missing data is not a syntax fault.
+        assert evaluate_expression("{{ inputs.missing }}", ctx) is None
+        assert evaluate_expression("{{ bogus.path[3] }}", ctx) is None
+
     def test_pipe_detection_is_quote_aware(self):
         from specify_cli.workflows.expressions import evaluate_expression
         from specify_cli.workflows.base import StepContext

tests/unit/test_condition_expression_block.py

@@ -421,13 +421,18 @@ def test_wrapping_whitespace_would_invert_the_condition():
 def test_wrapping_an_open_quote_inverts_the_condition():
     """Why the unbalanced-quote case gets advice instead of a suggestion.
 
-    The raw-close fallback evaluates a truncated comparison and yields the string
-    "False", which evaluate_condition then reads as the `false` keyword. Pasting
-    the "correction" flips the condition rather than repairing it.
+    The raw-close fallback evaluates a truncated comparison, whose dangling
+    `'abc` operand used to resolve to None and yield the string "False", which
+    evaluate_condition then read as the `false` keyword -- pasting the
+    "correction" flipped the condition rather than repairing it. Since #5073
+    the truncated operand raises as unsupported syntax instead of silently
+    inverting; the advice (rather than a paste-ready suggestion) is still
+    right, because wrapping the text as written cannot repair it.
     """
     ctx = StepContext(inputs={"name": "Bob"})
     assert evaluate_condition("{{ inputs.name == 'abc", ctx) is True
-    assert evaluate_condition("{{ inputs.name == 'abc }}", ctx) is False
+    with pytest.raises(ValueError, match="unsupported expression syntax"):
+        evaluate_condition("{{ inputs.name == 'abc }}", ctx)
 
 
 @pytest.mark.parametrize(
@@ -655,7 +660,8 @@ def test_the_probe_reports_what_the_evaluator_reports():
         # _resolve_dot_path keys on [w-]+, so a key literally named "2bad" resolves.
         ("inputs.2bad", False),
         ("inputs.tags[foo]", True),
-        ("inputs.matrix[0][1]", True),
+        # Chained indexing is part of the indexed-segment shape since #5073.
+        ("inputs.matrix[0][1]", False),
         # Round 7: an operand one level down, which the single-term gate never saw.
         ("inputs.a === inputs.b", True),
         ("bogus", True),
@@ -711,12 +717,10 @@ def test_filter_wiring_errors_are_still_rejections(condition):
         ("42", True),
         ("3.14", True),
         ("-7", True),
-        # `1e3` has no "." so the evaluator calls int() on it, which fails; it then
-        # falls through to a path lookup and resolves to None. The gate used to
-        # decide this for itself with a float() test that accepted `1e3`, and the
-        # correction turned a truthy condition false. Now the evaluator reaches the
-        # dot-path resolution with `1e3` and reports it, so the two cannot disagree.
-        ("1e3", False),
+        # `1e3` is a numeric literal since #5073 -- the evaluator's float()
+        # branch accepts the exponent form, and the gate reads that straight
+        # off the evaluator's walk, so the two cannot disagree.
+        ("1e3", True),
         ("'one'", True),
         ('"one"', True),
         # Two literals, not one: the evaluator requires the opening quote's match to
@@ -745,7 +749,6 @@ def test_literal_handling_comes_from_the_evaluator(condition, resolvable):
         # to None however the index is written.
         "inputs[0]",
         "steps[1]",
-        "1e3",
         "'a' 'b'",
     ],
 )