From f63f40805565bba7dc4dea1d75ec655bb3d9dd7d Mon Sep 17 00:00:00 2001 From: Markus Date: Wed, 14 Oct 2026 15:57:58 +0200 Subject: [PATCH 01/12] refactor: derive filter metadata from one table and add a syntax-error hook Preparation for a grammar-based engine; behavior-neutral for the hand-written engine. - filters.py: FILTER_TABLE (function, argument form, usage text) is the single source; _REGISTERED_FILTERS, _ZERO_ARG_FILTERS and the "expected one of ..." message are derived from it. The message text is byte-identical to the previous literal in engine_current. - engine contract: _syntax_error(text) returns a parse-error message or None. engine_current has no syntax layer and always returns None. - diagnostics.py: prefers _syntax_error over the leaf walk in _unresolvable_term and after the incomplete-operand check in _wrapping_would_not_repair, so a parser's message (with position) reaches the condition diagnostics. No test changes; 1150 passed in the expression and condition test files. Assisted-by: OpenCode (model: claude-opus-5.5, autonomous) --- .../workflows/expressions/diagnostics.py | 17 ++++ .../workflows/expressions/engine.py | 2 + .../workflows/expressions/engine_current.py | 24 ++++- .../workflows/expressions/filters.py | 91 +++++++++++++------ 4 files changed, 102 insertions(+), 32 deletions(-) diff --git a/src/specify_cli/workflows/expressions/diagnostics.py b/src/specify_cli/workflows/expressions/diagnostics.py index b99bb51039..db12433760 100644 --- a/src/specify_cli/workflows/expressions/diagnostics.py +++ b/src/specify_cli/workflows/expressions/diagnostics.py @@ -18,6 +18,7 @@ _first_unclosable_block, _has_incomplete_operand, _is_single_expression, + _syntax_error, ) from .template import _split_top_level, _strip_stray_delimiters @@ -261,6 +262,15 @@ def _unresolvable_term(text: str) -> str | None: if not stripped: return "an operand is empty" + # A grammar-based engine can name the parse fault directly, position and + # character included. Prefer that over the leaf walk: text that does not + # parse has no leaves, and reporting "no unresolvable term" for it would + # tell the author their expression is fine. ``None`` for the hand-written + # engine, which resolves unknown shapes to ``None`` instead. + syntax = _syntax_error(stripped) + if syntax is not None: + return syntax + for leaf in _collect_leaves(stripped): reason = _unresolvable_leaf(leaf) if reason is not None: @@ -285,6 +295,13 @@ def _wrapping_would_not_repair(core: str) -> str | None: return "its brackets do not balance" if _has_incomplete_operand(core): return "an operator in it is missing an operand" + # After the incomplete-operand check, not before: the four "missing an + # operand" shapes are also parse failures, and the specific message is the + # better one for them. Everything left that does not parse (arithmetic, + # stray characters, a second literal) gets the engine's own message. + syntax = _syntax_error(core) + if syntax is not None: + return syntax unresolvable = _unresolvable_term(core) if unresolvable is not None: return unresolvable diff --git a/src/specify_cli/workflows/expressions/engine.py b/src/specify_cli/workflows/expressions/engine.py index 023decb145..1857ecba77 100644 --- a/src/specify_cli/workflows/expressions/engine.py +++ b/src/specify_cli/workflows/expressions/engine.py @@ -19,6 +19,7 @@ _has_incomplete_operand, _is_single_expression, _leaf_sink, + _syntax_error, ) __all__ = [ @@ -31,4 +32,5 @@ "_has_incomplete_operand", "_is_single_expression", "_leaf_sink", + "_syntax_error", ] diff --git a/src/specify_cli/workflows/expressions/engine_current.py b/src/specify_cli/workflows/expressions/engine_current.py index e3c74eda70..beef2e682c 100644 --- a/src/specify_cli/workflows/expressions/engine_current.py +++ b/src/specify_cli/workflows/expressions/engine_current.py @@ -15,6 +15,7 @@ from typing import Any from .filters import ( + _EXPECTED_FILTERS, _REGISTERED_FILTERS, _ZERO_ARG_FILTERS, _filter_contains, @@ -50,6 +51,7 @@ "_has_incomplete_operand", "_is_single_expression", "_leaf_sink", + "_syntax_error", ] @@ -133,11 +135,9 @@ def _apply_filter(value: Any, filter_expr: str, namespace: dict[str, Any]) -> An # genuinely unknown filter name, so the message names the real problem # instead of calling a known filter "unknown". name = leading.group(0) if leading else filter_expr - expected = ( - "expected one of default or default('x'), join('sep'), " - "map('attr'), contains('s'), split('sep'), from_json, upper, " - "lower, length, or to_json" - ) + # Derived from the filter table, so the message lists exactly what is + # registered instead of a second copy of the set. + expected = _EXPECTED_FILTERS if name in _REGISTERED_FILTERS: raise ValueError( f"filter '{name}' used in an unsupported form (got " @@ -546,3 +546,17 @@ def _has_incomplete_operand(text: str) -> bool: return _find_top_level(stripped, "|") != -1 and any( not segment.strip() for segment in _split_top_level(stripped, "|") ) + + +def _syntax_error(text: str) -> str | None: + """A parse-error message for *text*, or ``None``. + + This engine has no syntax layer: an unknown shape falls through to the + dot-path resolution and becomes ``None``, and the diagnostics describe that + through ``_collect_leaves`` and ``_has_incomplete_operand``. Grammar-based + engines return the parser's complaint here, naming the character and column + instead of classifying the text by heuristics; the diagnostics prefer it + when it is present. Always ``None`` here, so the hook changes nothing for + this engine. + """ + return None diff --git a/src/specify_cli/workflows/expressions/filters.py b/src/specify_cli/workflows/expressions/filters.py index 864b7280dd..67f330a4a4 100644 --- a/src/specify_cli/workflows/expressions/filters.py +++ b/src/specify_cli/workflows/expressions/filters.py @@ -10,27 +10,10 @@ import json from collections.abc import Callable +from dataclasses import dataclass from typing import Any -# The filters the expression evaluator recognizes. Used to tell a -# *registered* filter used in an unsupported form (e.g. `| join` with no -# argument) apart from a genuinely unknown filter name, so each raises an -# error that names the real problem. -_REGISTERED_FILTERS: tuple[str, ...] = ( - "default", - "join", - "map", - "contains", - "from_json", - "upper", - "lower", - "split", - "length", - "to_json", -) - - # -- Custom filters ------------------------------------------------------- def _filter_default(value: Any, default_value: Any = "") -> Any: @@ -277,15 +260,69 @@ def _check_json_keys(value: Any) -> None: stack.extend(item) +# -- The filter table ------------------------------------------------------ +# +# One entry per filter: the function, the call forms it accepts, and the usage +# text the error messages advertise. ``_REGISTERED_FILTERS``, ``_ZERO_ARG_FILTERS`` +# and the "expected one of ..." sentence are derived from this table, so a new +# filter is one row here — not a row here, a branch in the dispatcher and a +# second edit to the message. The table order is the order the message lists. + +@dataclass(frozen=True) +class FilterSpec: + """One pipe filter: its function and the call forms it accepts. + + ``argument``: the filter takes exactly one parenthesised argument + (``join('sep')``). ``bare``: the unparenthesised form is also valid + (``default`` alone). Zero-argument filters (``from_json``, ``upper``, ...) + have ``argument=False`` and are only valid bare — the strictness that turns + ``from_json()``, ``from_json)`` and ``from_json extra`` into errors naming + the filter instead of a silent passthrough. + """ + + function: Callable[..., Any] + argument: bool + bare: bool = False + usage: str = "" + + +FILTER_TABLE: dict[str, FilterSpec] = { + "default": FilterSpec( + _filter_default, argument=True, bare=True, usage="default or default('x')" + ), + "join": FilterSpec(_filter_join, argument=True, usage="join('sep')"), + "map": FilterSpec(_filter_map, argument=True, usage="map('attr')"), + "contains": FilterSpec(_filter_contains, argument=True, usage="contains('s')"), + "split": FilterSpec(_filter_split, argument=True, usage="split('sep')"), + "from_json": FilterSpec(_filter_from_json, argument=False, usage="from_json"), + "upper": FilterSpec(_filter_upper, argument=False, usage="upper"), + "lower": FilterSpec(_filter_lower, argument=False, usage="lower"), + "length": FilterSpec(_filter_length, argument=False, usage="length"), + "to_json": FilterSpec(_filter_to_json, argument=False, usage="to_json"), +} + + +# The filters the expression evaluator recognizes. Used to tell a +# *registered* filter used in an unsupported form (e.g. `| join` with no +# argument) apart from a genuinely unknown filter name, so each raises an +# error that names the real problem. +_REGISTERED_FILTERS: tuple[str, ...] = tuple(FILTER_TABLE) + + +# The sentence every filter-wiring error ends with, derived from the table so +# adding a filter cannot leave the message listing the old set. +_EXPECTED_FILTERS = ( + "expected one of " + + ", ".join(spec.usage for spec in list(FILTER_TABLE.values())[:-1]) + + ", or " + + list(FILTER_TABLE.values())[-1].usage +) + + # Filters that take no arguments and tolerate no trailing tokens. Keyed by name -# so ``_apply_filter`` can recognize a mis-wired form of any of them from the -# leading filter name alone, instead of each needing its own branch. ``default`` -# is deliberately absent: it is the one filter that is valid both bare and with -# an argument, so it is dispatched by both of the branches below. +# so a dispatcher can recognize a mis-wired form of any of them from the +# leading filter name alone. ``default`` is deliberately absent: it is the one +# filter that is valid both bare and with an argument. _ZERO_ARG_FILTERS: dict[str, Callable[[Any], Any]] = { - "from_json": _filter_from_json, - "upper": _filter_upper, - "lower": _filter_lower, - "length": _filter_length, - "to_json": _filter_to_json, + name: spec.function for name, spec in FILTER_TABLE.items() if not spec.argument } From 2b83c8e90ec81db505b0fba05c5433f874325788 Mon Sep 17 00:00:00 2001 From: Markus Date: Wed, 14 Oct 2026 15:58:23 +0200 Subject: [PATCH 02/12] feat(workflows): parse workflow expressions with a Lark LALR grammar Replace the hand-written block-body parser with a Lark (LALR) grammar and select it in engine.py. The expression language is unchanged (#5060). - LALR parser with keyword terminal priority. - The ambiguous-precedence guards are a grammar property: a filter chain only applies to an operator-free head, so a pipe after a comparison or after `not` is a parse error ("ambiguous filter precedence"). - The PATH terminal is built from namespace._PLAIN_SEGMENT / _INDEXED_SEGMENT, so path syntax has one definition; the existing drift test now covers the grammar too. - _collect_leaves / _evaluator_rejects are a single static tree walk; the probe run and _leaf_sink are gone from the contract. - _syntax_error returns the parser's message with column, which the condition diagnostics use. - Filters dispatch through FILTER_TABLE. Deliberate test changes, each with its reason in the test: - [1,2,3][1] and an unclosed quote in a condition raise ValueError instead of silently yielding None / False. - Trailing tokens after a filter name, and a comparison after a filter chain, are parse errors ("invalid expression") instead of dispatcher messages; still ValueError. - Three diagnostics fixture rows expect the parser's message. - The probe-only tests are replaced (leaf-walk transparency of groups) or removed (_leaf_sink); imports of engine internals are dropped. Assisted-by: OpenCode (model: claude-opus-5.5, autonomous) Assisted-by: GitHub Copilot CLI (model: Claude Sonnet 5.5, supervised) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pyproject.toml | 1 + .../workflows/expressions/__init__.py | 4 +- .../workflows/expressions/engine.py | 4 +- .../workflows/expressions/engine_lark_a2.py | 561 ++++++++++++++++++ tests/test_workflows.py | 66 ++- tests/unit/test_condition_expression_block.py | 76 ++- 6 files changed, 638 insertions(+), 74 deletions(-) create mode 100644 src/specify_cli/workflows/expressions/engine_lark_a2.py diff --git a/pyproject.toml b/pyproject.toml index 7a0404f84c..e1cc5428a8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,6 +15,7 @@ dependencies = [ "json5>=0.13.0", "mcp>=2.2.0,<3.0.0", "pydantic>=2.13.0,<3.0.0", + "lark>=1.3", ] [project.scripts] diff --git a/src/specify_cli/workflows/expressions/__init__.py b/src/specify_cli/workflows/expressions/__init__.py index befee1157e..4a86ebd7de 100644 --- a/src/specify_cli/workflows/expressions/__init__.py +++ b/src/specify_cli/workflows/expressions/__init__.py @@ -7,7 +7,7 @@ This package is split by responsibility: - ``namespace``/``filters``/``operators``/``template`` — shared leaves; -- ``engine_current`` — the hand-written evaluator behind the contract; +- ``engine_lark_a2`` — the Lark (LALR) evaluator behind the contract; - ``engine`` — the selection point (the one line that swaps the engine); - ``api`` — the public evaluation entry points; - ``diagnostics`` — condition validation and remediation. @@ -36,7 +36,6 @@ _collect_leaves, _evaluator_rejects, _has_incomplete_operand, - _leaf_sink, ) from .filters import _REGISTERED_FILTERS from .template import _strip_stray_delimiters @@ -57,7 +56,6 @@ "_has_incomplete_operand", "_has_unbalanced_bracket", "_has_unbalanced_quote", - "_leaf_sink", "_strip_stray_delimiters", "_unresolvable_term", ] diff --git a/src/specify_cli/workflows/expressions/engine.py b/src/specify_cli/workflows/expressions/engine.py index 1857ecba77..d6a6f91f57 100644 --- a/src/specify_cli/workflows/expressions/engine.py +++ b/src/specify_cli/workflows/expressions/engine.py @@ -9,7 +9,7 @@ from __future__ import annotations -from .engine_current import ( +from .engine_lark_a2 import ( _COMPARISON_OPERATORS, _WORD_OPERATORS, _collect_leaves, @@ -18,7 +18,6 @@ _first_unclosable_block, _has_incomplete_operand, _is_single_expression, - _leaf_sink, _syntax_error, ) @@ -31,6 +30,5 @@ "_first_unclosable_block", "_has_incomplete_operand", "_is_single_expression", - "_leaf_sink", "_syntax_error", ] diff --git a/src/specify_cli/workflows/expressions/engine_lark_a2.py b/src/specify_cli/workflows/expressions/engine_lark_a2.py new file mode 100644 index 0000000000..789360c16f --- /dev/null +++ b/src/specify_cli/workflows/expressions/engine_lark_a2.py @@ -0,0 +1,561 @@ +"""Variant A2: Lark (LALR) as the parser, used the way the library is meant to be. + +The Earley spike (``engine_lark.py``) emulated ``engine_current``'s internals: +an if-chain filter dispatcher rebuilt from strings, the probe walk with its +leaf sink, and a grammar that accepted shapes the current parser refuses. This +engine answers the question A/C left open -- what a grammar-based parser costs +and buys when it is not written to mimic the old code: + +* **The precedence guards are in the grammar, not in a post-check.** A filter + chain is only valid where the current parser allows one: the head is a plain + operand, and the chain is a whole expression or sits in parentheses, a list + element or a filter argument. ``count > limit | default(5)``, ``not x | + default(1)`` and ``x | default(0) > 5`` do not parse at all; the first two + keep the "ambiguous filter precedence" message through the offending ``|`` + token. +* **The path terminal is derived from ``namespace``.** ``_PLAIN_SEGMENT`` and + ``_INDEXED_SEGMENT`` are the one definition of a path segment; widening + either rebuilds the parser (the cache is keyed on the patterns) and reaches + evaluation and validation together. No second copy to drift. +* **The contract primitives are static.** ``_collect_leaves`` walks the parse + tree for ``path`` nodes -- there is no probe run, no context variable, no + placeholder namespace. ``_evaluator_rejects`` checks filter nodes against + ``FILTER_TABLE``. ``_has_incomplete_operand`` classifies the parse failure: + end of text or an operator token means a missing operand, anything else is a + syntax error and reported with its position through ``_syntax_error``. +* **Template level unchanged.** The shared quote-aware scanner still decides + what is one block and what is text; only the block body is parsed here. + +Everything else -- namespace, filters, comparison semantics, diagnostics -- +is the shared package, unchanged. +""" + +from __future__ import annotations + +import re +from functools import lru_cache +from typing import Any + +from lark import Lark, Token, Transformer, v_args +from lark.exceptions import UnexpectedCharacters, UnexpectedInput, UnexpectedToken + +from . import namespace +from .filters import FILTER_TABLE, _EXPECTED_FILTERS +from .namespace import _resolve_dot_path +from .operators import _safe_compare, _safe_membership +from .template import ( + _collapse_whitespace, + _find_block_close, + _first_unclosable_block, + _is_single_expression, +) + + +__all__ = [ + "_COMPARISON_OPERATORS", + "_WORD_OPERATORS", + "_collect_leaves", + "_evaluate_template", + "_evaluator_rejects", + "_first_unclosable_block", + "_has_incomplete_operand", + "_is_single_expression", + "_syntax_error", +] + + +# Order matters -- multi-char operators first. The contract constant the +# diagnostics and the tests read; the grammar below implements the same set +# (``CMP_OP`` and the word terminals), so a new operator needs both lines. +_COMPARISON_OPERATORS = ("!=", "==", ">=", "<=", ">", "<", " not in ", " in ") + +_WORD_OPERATORS = tuple( + op for op in (" or ", " and ") + _COMPARISON_OPERATORS if op.startswith(" ") +) + + +def _path_terminal_pattern(plain_pattern: str, indexed_pattern: str) -> str: + """The ``PATH`` regex, built from the namespace's own segment definitions. + + ``_PLAIN_SEGMENT`` (``^[\\w-]+$``) and ``_INDEXED_SEGMENT`` + (``^([\\w-]+)\\[(-?\\d+)\\]$``) are stripped of their anchors and combined: + every segment is a plain or an indexed one. The *first* segment's first + character excludes digits so ``123`` keeps lexing as a number; that is the + only difference from the namespace pattern, and it is why this derivation + reads the pattern instead of re-describing the shape. + + ``_INDEXED_SEGMENT`` is read through ``namespace`` at call time, so the + parser cache below rebuilds when it is widened -- the drift guard + ``test_gate_reads_the_shared_indexed_segment_definition`` rests on that. + """ + plain = plain_pattern.strip("^$") + indexed = indexed_pattern.strip("^$") + later = f"(?:{indexed}|{plain})" + first = f"(?:{indexed}|[^\\W\\d]{plain[:-1]}*)" + return f"{first}(?:\\.{later})*" + + +def _grammar(path_pattern: str) -> str: + return r""" +?start: filtered + | expr + +filtered: operand ("|" filter)+ + +?expr: or_expr + +?or_expr: and_expr + | or_expr OR and_expr -> or_op + +?and_expr: not_expr + | and_expr AND not_expr -> and_op + +?not_expr: NOT not_expr -> not_op + | comparison + +?comparison: operand + | comparison CMP_OP operand -> cmp + +?operand: string + | number + | list_lit + | path + | group + +group: "(" start ")" + +string: STRING +number: NUMBER +path: PATH + +list_lit: "[" list_body? "]" +list_body: [start] ("," [start])* + +?filter: filter_bare + | filter_call + +filter_bare: FILTER_NAME +filter_call: FILTER_NAME "(" [start ("," start)*] ")" + +CMP_OP.2: "!=" | "==" | ">=" | "<=" | ">" | "<" | /(? Lark: + """Build the parser for one pair of namespace patterns, once. + + Keyed on the patterns (not a single cached instance) so widening + ``namespace._INDEXED_SEGMENT`` builds a parser that accepts the wider form + instead of serving a stale one. + """ + return Lark( + _grammar(_path_terminal_pattern(plain_pattern, indexed_pattern)), + parser="lalr", + maybe_placeholders=False, + ) + + +def _parser() -> Lark: + return _parser_for( + namespace._PLAIN_SEGMENT.pattern, namespace._INDEXED_SEGMENT.pattern + ) + + +@v_args(inline=True) +class _BuildAst(Transformer): + """Turn the parse tree into the same kind of tuples the other engines walk. + + One small method per grammar rule; aliases (``-> cmp``, ``-> filter_call``) + name the shape, and anonymous terminals (``"|"``, ``"("``, ``","``) are + filtered out by Lark, so the children are exactly the operands. + """ + + def string(self, token: Token) -> tuple[str, str]: + return ("lit", str(token)[1:-1]) + + def number(self, token: Token) -> tuple[str, Any]: + text = str(token) + return ("lit", float(text) if "." in text else int(text)) + + def path(self, token: Token) -> tuple[str, str]: + return ("path", str(token)) + + def group(self, inner: Any) -> Any: + return inner + + def list_body(self, *items: Any) -> tuple[Any, ...]: + return items + + def list_lit(self, body: tuple[Any, ...] | None = None) -> tuple[str, tuple[Any, ...]]: + return ("list", body or ()) + + def filter_bare(self, name: Token) -> tuple[str, str, tuple[Any, ...], bool]: + return ("filter", str(name), (), False) + + def filter_call(self, name: Token, *args: Any) -> tuple[str, str, tuple[Any, ...], bool]: + return ("filter", str(name), args, True) + + def filtered(self, head: Any, *filters: Any) -> tuple[str, Any, tuple[Any, ...]]: + return ("pipe", head, filters) + + def cmp(self, left: Any, op: Token, right: Any) -> tuple[str, ...]: + return ("cmp", " ".join(str(op).split()), left, right) + + def or_op(self, left: Any, _op: Token, right: Any) -> tuple[str, Any, Any]: + return ("or", left, right) + + def and_op(self, left: Any, _op: Token, right: Any) -> tuple[str, Any, Any]: + return ("and", left, right) + + def not_op(self, _op: Token, inner: Any) -> tuple[str, Any]: + return ("not", inner) + + +def _parse(text: str) -> Any: + """Parse one block body into the tuple AST. Raises ``lark`` errors.""" + return _BuildAst().transform(_parser().parse(text)) + + +def _parse_error_message(text: str, exc: UnexpectedInput) -> str: + """The author-facing message for a parse failure of *text*. + + The pipe case is the one carried-over semantic: a ``|`` where an operand + was expected is the shape the current evaluator refuses as an ambiguous + filter precedence, so it gets the same wording. Everything else names the + offending character and column. + """ + token = getattr(exc, "token", None) + if token is not None and token.value == "|" and getattr(token, "column", 1) > 1: + return ( + f"ambiguous filter precedence in '{text}': a filter cannot be an " + f"operand of an operator. Filter the operand in its own expression " + f"instead." + ) + if token is not None and token.type == "$END": + return f"invalid expression '{text}': unexpected end of text" + if isinstance(exc, UnexpectedCharacters): + char = getattr(exc, "char", "?") + column = getattr(exc, "column", "?") + return f"invalid expression '{text}': unexpected character '{char}' at column {column}" + if token is not None and token.value: + column = getattr(token, "column", "?") + return f"invalid expression '{text}': unexpected '{token.value}' at column {column}" + return f"invalid expression '{text}'" + + +def _syntax_error(text: str) -> str | None: + """The parser's complaint about *text*, or ``None`` when it parses. + + The diagnostics ask this before walking leaves: text that does not parse + has no leaves, and "no unresolvable term" would tell the author their + expression is fine when the evaluator will refuse it. + """ + collapsed = _collapse_whitespace(text) + if not collapsed: + return None + try: + _parse(collapsed) + except UnexpectedInput as exc: + return _parse_error_message(collapsed, exc) + return None + + +# -- Evaluation --------------------------------------------------------------- + +def _apply_filter( + value: Any, filt: tuple[str, str, tuple[Any, ...], bool], namespace_: dict[str, Any] +) -> Any: + """Apply one ``("filter", name, args, has_parens)`` node to *value*. + + Arguments are evaluated here, before the table lookup, so a leaf in an + argument is visible to the diagnostics even when the filter name is wrong. + The accepted call forms and the error wording come from ``FILTER_TABLE`` + and``_EXPECTED_FILTERS``: the same table the message lists, so adding a + filter cannot leave the two disagreeing. + """ + _, name, arg_nodes, has_parens = filt + args = [_eval(node, namespace_) for node in arg_nodes] + display = f"{name}(...)" if has_parens else name + + spec = FILTER_TABLE.get(name) + if spec is None: + raise ValueError( + f"unknown filter '{name}': {_EXPECTED_FILTERS} (got '| {display}')" + ) + if not spec.argument: + # A zero-argument filter tolerates no parentheses and no trailing + # tokens; `from_json()` and `from_json('x')` are the same mistake. + if has_parens: + raise ValueError( + f"{name}: expected '| {name}' with no arguments or trailing " + f"tokens, got '| {display}'" + ) + return spec.function(value) + if has_parens and len(args) == 1: + return spec.function(value, args[0]) + if not has_parens and spec.bare: + return spec.function(value) + raise ValueError( + f"filter '{name}' used in an unsupported form (got '| {display}'): " + f"{_EXPECTED_FILTERS}" + ) + + +def _eval(node: Any, namespace_: dict[str, Any]) -> Any: + """Evaluate a tuple AST node against *namespace_*.""" + tag = node[0] + if tag == "lit": + return node[1] + if tag == "path": + text = node[1] + # A bare word that is a literal keyword is a value, not a name; the + # dot/bracket guard keeps `inputs.true` a path. + if _is_literal_path(text): + lowered = text.lower() + if lowered == "true": + return True + if lowered == "false": + return False + return None + return _resolve_dot_path(namespace_, text) + if tag == "list": + return [_eval(item, namespace_) for item in node[1]] + if tag == "pipe": + value = _eval(node[1], namespace_) + for filt in node[2]: + value = _apply_filter(value, filt, namespace_) + return value + if tag == "not": + return not bool(_eval(node[1], namespace_)) + + if tag == "cmp": + op = node[1] + left = _eval(node[2], namespace_) + right = _eval(node[3], namespace_) + if op == "==": + return left == right + if op == "!=": + return left != right + if op == "in": + return _safe_membership(left, right, negate=False) + if op == "not in": + return _safe_membership(left, right, negate=True) + return _safe_compare(left, right, op) + + left = _eval(node[1], namespace_) + right = _eval(node[2], namespace_) + if tag == "or": + return bool(left) or bool(right) + if tag == "and": + return bool(left) and bool(right) + raise ValueError(f"unsupported expression node {tag!r}") + + +def _evaluate_simple_expression(expr: str, namespace_: dict[str, Any]) -> Any: + """Evaluate the body of one ``{{ }}`` block against *namespace_*. + + A syntax error is raised as ``ValueError``, the type the evaluator already + uses for a malformed expression, rather than leaking the parser's own + exception. The message names the character and column. + """ + collapsed = _collapse_whitespace(expr) + if not collapsed: + return None + try: + ast = _parse(collapsed) + except UnexpectedInput as exc: + raise ValueError(_parse_error_message(collapsed, exc)) from exc + return _eval(ast, namespace_) + + +def _interpolate_expressions(template: str, namespace_: dict[str, Any]) -> str: + """Substitute every top-level ``{{ ... }}`` block, quote-aware. + + The template level is the shared scanner, unchanged: only the body handed + to ``_evaluate_simple_expression`` goes through the parser. + """ + out: list[str] = [] + i = 0 + n = len(template) + while i < n: + start = template.find("{{", i) + if start == -1: + out.append(template[i:]) + break + out.append(template[i:start]) + close = _find_block_close(template, start) + if close == -1: + raw_close = template.find("}}", start + 2) + if raw_close == -1: + out.append(template[start:]) + break + close = raw_close + val = _evaluate_simple_expression(template[start + 2:close].strip(), namespace_) + out.append(str(val) if val is not None else "") + i = close + 2 + return "".join(out) + + +def _evaluate_template(template: str, namespace_: dict[str, Any]) -> Any: + """Evaluate *template*: a single block stays typed, text is interpolated.""" + stripped = template.strip() + if _is_single_expression(stripped): + return _evaluate_simple_expression(stripped[2:-2].strip(), namespace_) + return _interpolate_expressions(template, namespace_) + + +# -- Contract primitives -------------------------------------------------------- + +_LITERAL_WORDS = ("true", "false", "none", "null") + + +def _is_literal_path(text: str) -> bool: + """True for a bare literal keyword, which the grammar lexes as a path. + + The evaluator resolves names, not literals; the diagnostics must report the + same names the evaluator will look up, so both ask this one condition. + A dot or bracket keeps ``inputs.true`` a path. + """ + return "." not in text and "[" not in text and text.lower() in _LITERAL_WORDS + + +def _visit(node: Any, paths: list[str] | None = None, filters: list[Any] | None = None) -> None: + """Walk *node* in evaluation order, collecting paths and filter nodes. + + One traversal serves both contract primitives: ``_collect_leaves`` asks for + the names (``paths``), ``_evaluator_rejects`` asks for the filters to check + against the table (``filters``). Parents are transparent -- both sides of + ``and``/``or``, every list element and every filter argument is reached. + Text that does not parse has neither names nor filters; ``_syntax_error`` + explains why separately. + """ + tag = node[0] + if tag == "path": + if paths is not None and not _is_literal_path(node[1]): + paths.append(node[1]) + return + if tag == "list": + for item in node[1]: + _visit(item, paths, filters) + elif tag == "pipe": + _visit(node[1], paths, filters) + for filt in node[2]: + if filters is not None: + filters.append(filt) + for arg in filt[2]: + _visit(arg, paths, filters) + elif tag == "not": + _visit(node[1], paths, filters) + elif tag in ("and", "or"): + _visit(node[1], paths, filters) + _visit(node[2], paths, filters) + elif tag == "cmp": + _visit(node[2], paths, filters) + _visit(node[3], paths, filters) + + +def _collect_leaves(text: str) -> list[str]: + """Every name *text* hands to the evaluator as a path to resolve. + + Walks the parse tree rather than re-running the evaluation: no probe, no + placeholder namespace, no state to arm. + """ + collapsed = _collapse_whitespace(text) + if not collapsed: + return [] + try: + ast = _parse(collapsed) + except UnexpectedInput: + return [] + leaves: list[str] = [] + _visit(ast, paths=leaves) + return leaves + + +def _check_filter(filt: tuple[str, str, tuple[Any, ...], bool]) -> str | None: + """Why *filt* would be refused at evaluation time, or ``None``.""" + _, name, args, has_parens = filt + spec = FILTER_TABLE.get(name) + if spec is None: + return f"unknown filter '{name}'" + if not spec.argument: + if has_parens: + return f"{name}: expected '| {name}' with no arguments or trailing tokens" + return None + if has_parens and len(args) == 1: + return None + if not has_parens and spec.bare: + return None + return f"filter '{name}' used in an unsupported form" + + +def _evaluator_rejects(text: str) -> str | None: + """The evaluator's own complaint about how *text* wires its filters. + + Static: the parse tree is checked against ``FILTER_TABLE`` -- the same + table the dispatcher uses -- so a filter used under an unknown name or in + an unsupported form is reported by the code that will run it, without a + probe evaluation. Text that does not parse is not a filter-wiring fault; + ``_syntax_error`` covers it. + """ + collapsed = _collapse_whitespace(text) + if not collapsed: + return None + try: + ast = _parse(collapsed) + except UnexpectedInput: + return None + filters: list[Any] = [] + _visit(ast, filters=filters) + for filt in filters: + problem = _check_filter(filt) + if problem is not None: + return problem + return None + + +_OPERATOR_TOKENS = frozenset({"CMP_OP", "AND", "OR", "NOT"}) + +# A word operator at the very start of the text. The LALR lexer only offers +# CMP_OP/AND/OR/NOT where the grammar can take them, so without a left operand +# `in`/`and`/`or` lex as paths and the failure lands on the *next* token; the +# leading word is what identifies the missing operand. `not x` is valid, so +# only `not in` counts. +_LEADING_WORD_OPERATOR = re.compile(r"^(?:and|or|in)\b|^not\s+in\b") + + +def _has_incomplete_operand(text: str) -> bool: + """True when an operator in *text* is missing an operand on either side. + + Reads the parse failure rather than restating the operator table: text + that ends where an operand was expected (``inputs.a ==``, ``inputs.a and``, + ``inputs.x |``) fails at the ``$END`` token; a leading word operator + (``in inputs.a``, ``and inputs.x``) fails right behind the word. A failure + anywhere else (arithmetic, a stray character, a second literal) is a + syntax error, not an incomplete operand, and is reported by + ``_syntax_error`` instead. + """ + stripped = text.strip() + if not stripped: + return True + try: + _parse(_collapse_whitespace(stripped)) + except UnexpectedToken as exc: + if exc.token.type == "$END": + return True + if str(exc.token.type) in _OPERATOR_TOKENS: + return True + return bool(_LEADING_WORD_OPERATOR.match(stripped)) + except UnexpectedInput: + return False + return False diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 2910f8b8ad..a1e8920e8a 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -451,16 +451,22 @@ def test_list_literal_followed_by_index_is_not_misparsed_as_one_literal(self): same shape as a genuine single list literal. Naively stripping the 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. + resolving the index. It must not be misclassified as one literal. + + The grammar-based engine (A2) refuses the shape at the parser, with + the position in the message; the hand-written engine fell through to + unresolvable (``None``). Both avoid the wrong-looking list; the parse + error is the louder signal, so this test pins it. """ + 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="invalid expression"): + evaluate_expression("{{ [1,2,3][1] }}", ctx) + with pytest.raises(ValueError, match="invalid expression"): + 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. @@ -604,28 +610,31 @@ def test_filter_from_json_non_string_raises(self): def test_filter_from_json_rejects_malformed_forms(self): # `from_json` is strict: no arguments and no trailing tokens. Every - # mis-wired form — parenthesized, accidental arg, or trailing - # garbage — must raise rather than silently fall through to the - # unknown-filter path and return the unparsed value. + # mis-wired form must raise rather than silently fall through to the + # unknown-filter path and return the unparsed value. Parenthesized + # forms reach the strict dispatcher; trailing garbage after the filter + # name is refused by the parser instead (A2: the grammar, not a regex, + # is the strictness), so those carry the parse message. import pytest from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext ctx = StepContext(steps={"emit": {"output": {"stdout": '{"a": 1}'}}}) - bad_forms = ( + for bad in ( "from_json()", "from_json('x')", "from_json ()", "from_json ('x')", - "from_json)", - "from_json extra", - "from_json 'x'", - ) - for bad in bad_forms: + ): with pytest.raises(ValueError, match="from_json: expected"): evaluate_expression( "{{ steps.emit.output.stdout | " + bad + " }}", ctx ) + for bad in ("from_json)", "from_json extra", "from_json 'x'"): + with pytest.raises(ValueError, match="invalid expression"): + evaluate_expression( + "{{ steps.emit.output.stdout | " + bad + " }}", ctx + ) def test_filter_upper_and_lower(self): from specify_cli.workflows.expressions import evaluate_expression @@ -907,20 +916,26 @@ def test_split_with_extra_argument_reports_unsupported_form(self): def test_zero_arg_filters_reject_miswired_forms(self): # The strict no-argument branch is shared by from_json/upper/lower/ - # length/to_json. Every mis-wired form — parenthesized, accidental arg, - # or trailing garbage — must raise naming the filter, rather than - # silently falling through to the unknown-filter path. + # length/to_json. Every mis-wired form must raise naming the filter + # rather than silently falling through to the unknown-filter path. + # Parenthesized forms reach the strict dispatcher; trailing garbage + # after the name is a parse error (A2). import pytest from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext ctx = StepContext(inputs={"s": "hi", "items": ["a"]}) for fname in ("from_json", "upper", "lower", "length", "to_json"): - for bad in (fname + "()", fname + "('x')", fname + ")", fname + " extra"): + for bad in (fname + "()", fname + "('x')"): with pytest.raises(ValueError, match=fname + ": expected"): evaluate_expression( "{{ inputs.s | " + bad + " }}", ctx ) + for bad in (fname + ")", fname + " extra"): + with pytest.raises(ValueError, match="invalid expression"): + evaluate_expression( + "{{ inputs.s | " + bad + " }}", ctx + ) def test_filter_unknown_name_raises(self): # An unregistered filter name must fail loudly rather than silently @@ -1071,22 +1086,21 @@ def test_registered_filter_unsupported_form_raises(self): evaluate_expression("{{ inputs.tags | contains }}", ctx) def test_filter_call_with_trailing_tokens_fails_loudly(self): - # A trailing operator/token after a filter's closing paren must not be - # silently discarded (the parser used an unanchored regex). It must - # fall through to the "unsupported form" ValueError, like the from_json - # branch's strict trailing-token handling. + # A trailing operator/token after a filter comes out as a parse error + # rather than being silently discarded. The grammar makes the filter + # chain a complete expression: a comparison after it and garbage after + # its closing paren both fail to parse (A2 — previously the hand-written + # parser routed these to an "unsupported form" ValueError instead). import pytest from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext - # A comparison after a filter (binds looser than the pipe) was dropped, - # so `default('7') > '5'` silently returned '7'. - with pytest.raises(ValueError, match="unsupported form"): + with pytest.raises(ValueError, match="invalid expression"): evaluate_expression( "{{ inputs.missing | default('7') > '5' }}", StepContext(inputs={}) ) # Trailing garbage after a valid filter call. - with pytest.raises(ValueError, match="unsupported form"): + with pytest.raises(ValueError, match="invalid expression"): evaluate_expression( "{{ inputs.tags | join(',') extra }}", StepContext(inputs={"tags": ["a", "b"]}), diff --git a/tests/unit/test_condition_expression_block.py b/tests/unit/test_condition_expression_block.py index 0f20431edc..d95e3728a6 100644 --- a/tests/unit/test_condition_expression_block.py +++ b/tests/unit/test_condition_expression_block.py @@ -19,14 +19,13 @@ _has_incomplete_operand, _unresolvable_term, _collect_leaves, - _leaf_sink, _evaluator_rejects, _strip_stray_delimiters, _COMPARISON_OPERATORS, _WORD_OPERATORS, format_condition_remediation, ) -from specify_cli.workflows.expressions import engine_current, namespace +from specify_cli.workflows.expressions import namespace from specify_cli.workflows.step.do_while import DoWhileStep from specify_cli.workflows.step.if_then import IfThenStep from specify_cli.workflows.step.while_loop import WhileStep @@ -418,16 +417,19 @@ def test_wrapping_whitespace_would_invert_the_condition(): assert evaluate_condition("{{ }}", ctx) is False -def test_wrapping_an_open_quote_inverts_the_condition(): +def test_wrapping_an_open_quote_is_not_a_repair(): """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 unwrapped form interpolates verbatim into non-empty text, so the + condition reads true. The "wrapped" form is an expression the evaluator + refuses: the grammar-based engine (A2) raises a parse error for the + unterminated quote instead of the hand-written engine's silent ``False``. + Either way, pasting the "correction" does not repair the condition. """ 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="invalid expression"): + evaluate_condition("{{ inputs.name == 'abc }}", ctx) @pytest.mark.parametrize( @@ -556,9 +558,13 @@ def test_incomplete_operand_covers_every_operator_the_evaluator_splits_on(): ("inputs.f(]", "brackets do not balance"), ("inputs.items | truncate", "the evaluator rejects it"), ("inputs.tags | join", "used in an unsupported form"), - ('he said "hi" then left', "is not a name the evaluator can resolve"), - ("inputs.count+1", "is not a valid path segment"), - ("inputs.a === inputs.b", "is not a name the evaluator can resolve"), + # The grammar-based engine (A2) names the position instead of classifying + # the text: `he said ...` fails at the second word, `count+1` at the `+`. + # Previously: "is not a name the evaluator can resolve" / "is not a valid + # path segment". + ('he said "hi" then left', "unexpected 'said'"), + ("inputs.count+1", "unexpected character '+' at column 13"), + ("inputs.a === inputs.b", "unexpected character '=' at column 12"), ("bogus == 'x'", "is not one of the namespace roots"), ("inputs.payload | from_json()", "the evaluator rejects it"), ] @@ -869,23 +875,20 @@ def test_indexing_an_always_mapping_root_still_loses_the_correction(condition): assert evaluate_condition("{{ " + condition + " }}", ctx) is False -# --- what the leaf sink rests on ---------------------------------------------- +# --- what the leaf walk rests on ---------------------------------------------- -def test_the_sink_is_off_outside_a_probe(): - """A normal evaluation must not pay for, or be observed by, the gate.""" - assert _leaf_sink.get() is None +def test_the_group_is_transparent_to_the_leaf_walk(): + """The gate's operands are the names the evaluator's tree contains. - evaluate_expression("{{ inputs.name }}", StepContext(inputs={"name": "x"})) - - assert _leaf_sink.get() is None - - -def test_the_sink_is_cleared_even_when_the_probe_raises(): - """`_collect_leaves` swallows probe errors; it must still reset the var.""" - _collect_leaves("inputs.tags | nosuchfilter") - - assert _leaf_sink.get() is None + A parenthesised group is not itself a name: its operands are the leaves. + The old probe got this from the evaluator unwrapping the group; the + parse-tree walk (A2) gets it from the grammar, so the assertion holds + without touching engine internals. + """ + grouped = "(inputs.a or inputs.b) and inputs.c" + assert _collect_leaves(grouped) == ["inputs.a", "inputs.b", "inputs.c"] + assert expressions._unresolvable_term(grouped) is None def test_both_sides_of_a_boolean_are_reported(): @@ -970,8 +973,8 @@ def test_a_literal_never_reaches_the_resolver(): # The point of this refactor is that widening what the evaluator accepts reaches # the validation gate for free. That is easy to claim and easy to lose: a second # copy of the grammar in the gate keeps every existing test green while silently -# reintroducing the drift. These two pin the wiring by moving the evaluator's own -# definitions and asserting the gate follows. +# reintroducing the drift. This pins the wiring by moving the evaluator's own +# definition and asserting the gate follows. def test_gate_reads_the_shared_indexed_segment_definition(monkeypatch): @@ -982,6 +985,11 @@ def test_gate_reads_the_shared_indexed_segment_definition(monkeypatch): optional minus sign and digits only. Widening it there and nowhere else must be enough; if the gate keeps its own copy of the shape (as `_PATH_SEGMENT` used to), this fails. + + With the grammar-based engine (A2) this also pins that the parser's `PATH` + terminal is *derived* from `namespace`: the cache is keyed on the patterns, + so the widened definition rebuilds the parser instead of serving a stale + one. A grammar that hard-coded the segment shape would fail here. """ path = "steps.tasks.output.task_list[+1].file" assert expressions._unresolvable_term(path) is not None @@ -990,19 +998,3 @@ def test_gate_reads_the_shared_indexed_segment_definition(monkeypatch): namespace, "_INDEXED_SEGMENT", re.compile(r"^([\w-]+)\[([+-]?\d+)\]$") ) assert expressions._unresolvable_term(path) is None - - -def test_gate_reports_the_leaves_the_evaluator_actually_reached(monkeypatch): - """The gate's operands come from the evaluator's own walk, not a second parse. - - What `_evaluate_simple_expression` treats as a leaf is what the gate checks. - Unwrapping a parenthesised group takes that group off the list, so the gate - stops checking it, with no change to the gate itself. - """ - grouped = "(inputs.a or inputs.b) and inputs.c" - assert expressions._unresolvable_term(grouped) is None - - # Stop the evaluator unwrapping the group and it becomes a leaf again, so - # the gate goes back to reporting it as a name it cannot resolve. - monkeypatch.setattr(engine_current, "_is_wrapped_in_parens", lambda text: False) - assert expressions._unresolvable_term(grouped) is not None From 5ffbb7136141985bfbdf881277dc8c3e65be8bfb Mon Sep 17 00:00:00 2001 From: Markus Date: Thu, 29 Oct 2026 11:41:23 +0100 Subject: [PATCH 03/12] fix(workflows): short-circuit and/or in the Lark expression engine The Lark engine evaluated both operands of `and`/`or`, which regressed the behavior added in #4923: a guard such as `inputs.payload != '' and (inputs.payload | from_json | length)` ran `from_json` on an empty string again. Return as soon as the left operand decides the result. Validation walks the parse tree, so skipped operands are still checked there. Assisted-by: GitHub Copilot CLI (model: Claude Sonnet 5.5, supervised) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/workflows/expressions/engine_lark_a2.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/specify_cli/workflows/expressions/engine_lark_a2.py b/src/specify_cli/workflows/expressions/engine_lark_a2.py index 789360c16f..033ad3fc7f 100644 --- a/src/specify_cli/workflows/expressions/engine_lark_a2.py +++ b/src/specify_cli/workflows/expressions/engine_lark_a2.py @@ -353,6 +353,12 @@ def _eval(node: Any, namespace_: dict[str, Any]) -> Any: return _safe_compare(left, right, op) left = _eval(node[1], namespace_) + # Skip the right operand when the left one decides, so a guard protects it. + # Validation walks the parse tree instead, so it still visits both sides. + if tag == "or" and bool(left): + return True + if tag == "and" and not bool(left): + return False right = _eval(node[2], namespace_) if tag == "or": return bool(left) or bool(right) From 255c6cc8f7c7b0fde0ff31e2d4baab585e20719a Mon Sep 17 00:00:00 2001 From: Markus Date: Thu, 29 Oct 2026 11:47:46 +0100 Subject: [PATCH 04/12] refactor(workflows): remove the hand-written expression engine The Lark engine is the only engine behind engine.py, so delete engine_current.py and everything that only it used: the `_leaf_sink` export, `_split_top_level_commas`, `_is_single_list_literal`, `_is_wrapped_in_parens`, `_EXPR_PATTERN` and `_ZERO_ARG_FILTERS`. Rename engine_lark_a2 to engine_lark, pin `lark>=1.3.1,<2.0.0`, and drop the prototype wording from docstrings and test comments. Assisted-by: GitHub Copilot CLI (model: Claude Sonnet 5.5, supervised) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pyproject.toml | 2 +- .../workflows/expressions/__init__.py | 2 +- .../workflows/expressions/engine.py | 2 +- .../workflows/expressions/engine_current.py | 562 ------------------ .../{engine_lark_a2.py => engine_lark.py} | 11 +- .../workflows/expressions/filters.py | 13 +- .../workflows/expressions/template.py | 101 +--- tests/test_workflows.py | 14 +- tests/unit/test_condition_expression_block.py | 11 +- .../unit/test_expression_package_boundary.py | 6 +- 10 files changed, 25 insertions(+), 699 deletions(-) delete mode 100644 src/specify_cli/workflows/expressions/engine_current.py rename src/specify_cli/workflows/expressions/{engine_lark_a2.py => engine_lark.py} (97%) diff --git a/pyproject.toml b/pyproject.toml index e1cc5428a8..6af69b2e1e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,7 +15,7 @@ dependencies = [ "json5>=0.13.0", "mcp>=2.2.0,<3.0.0", "pydantic>=2.13.0,<3.0.0", - "lark>=1.3", + "lark>=1.3.1,<2.0.0", ] [project.scripts] diff --git a/src/specify_cli/workflows/expressions/__init__.py b/src/specify_cli/workflows/expressions/__init__.py index 4a86ebd7de..9aa68442f4 100644 --- a/src/specify_cli/workflows/expressions/__init__.py +++ b/src/specify_cli/workflows/expressions/__init__.py @@ -7,7 +7,7 @@ This package is split by responsibility: - ``namespace``/``filters``/``operators``/``template`` — shared leaves; -- ``engine_lark_a2`` — the Lark (LALR) evaluator behind the contract; +- ``engine_lark`` — the Lark (LALR) evaluator behind the contract; - ``engine`` — the selection point (the one line that swaps the engine); - ``api`` — the public evaluation entry points; - ``diagnostics`` — condition validation and remediation. diff --git a/src/specify_cli/workflows/expressions/engine.py b/src/specify_cli/workflows/expressions/engine.py index d6a6f91f57..ba45bcc00d 100644 --- a/src/specify_cli/workflows/expressions/engine.py +++ b/src/specify_cli/workflows/expressions/engine.py @@ -9,7 +9,7 @@ from __future__ import annotations -from .engine_lark_a2 import ( +from .engine_lark import ( _COMPARISON_OPERATORS, _WORD_OPERATORS, _collect_leaves, diff --git a/src/specify_cli/workflows/expressions/engine_current.py b/src/specify_cli/workflows/expressions/engine_current.py deleted file mode 100644 index beef2e682c..0000000000 --- a/src/specify_cli/workflows/expressions/engine_current.py +++ /dev/null @@ -1,562 +0,0 @@ -"""The current hand-written workflow expression engine. - -This module is the engine behind the ``engine.py`` contract. It owns the -grammar walk (``_evaluate_simple_expression``), the filter dispatcher -(``_apply_filter``), template interpolation, and the probe/primitives the -condition diagnostics consume. It imports the shared leaf modules -(``namespace``, ``filters``, ``operators``, ``template``) and is imported only -by ``engine.py`` and the package facade. -""" - -from __future__ import annotations - -import re -from contextvars import ContextVar -from typing import Any - -from .filters import ( - _EXPECTED_FILTERS, - _REGISTERED_FILTERS, - _ZERO_ARG_FILTERS, - _filter_contains, - _filter_default, - _filter_join, - _filter_map, - _filter_split, -) -from .namespace import _NAMESPACE_ROOTS, _resolve_dot_path -from .operators import _safe_compare, _safe_membership -from .template import ( - _collapse_whitespace, - _find_block_close, - _find_top_level, - _first_unclosable_block, - _is_single_expression, - _is_single_list_literal, - _is_wrapped_in_parens, - _split_top_level, - _split_top_level_commas, -) - - -# The contract this engine fulfills. ``engine.py`` re-exports exactly these; -# anything else here is an internal of this engine. -__all__ = [ - "_COMPARISON_OPERATORS", - "_WORD_OPERATORS", - "_collect_leaves", - "_evaluate_template", - "_evaluator_rejects", - "_first_unclosable_block", - "_has_incomplete_operand", - "_is_single_expression", - "_leaf_sink", - "_syntax_error", -] - - -def _apply_filter(value: Any, filter_expr: str, namespace: dict[str, Any]) -> Any: - """Apply a single pipe filter segment to *value*. - - *filter_expr* is one link of a filter chain — the text between two - top-level ``|`` separators, already stripped (e.g. ``map('name')``, - ``default('x')``, ``from_json``). Returns the filtered value so the caller - can feed it into the next link. - - Raises ``ValueError`` on any mis-wired or unknown filter rather than - silently returning *value* unchanged: a passthrough would turn a mistyped - or unsupported filter into a wrong result with no signal. - """ - # Zero-argument filters are strict: they take no arguments and tolerate no - # trailing tokens. Match on the leading filter name and require the whole - # filter to be exactly that name, so every mis-wired form (`from_json()`, - # `from_json('x')`, `from_json)`, `from_json extra`, and the same for - # `upper`/`lower`/`length`/`to_json`) fails loudly instead of silently - # falling through to the unknown-filter path. - leading = re.match(r"\w+", filter_expr) - if leading and leading.group(0) in _ZERO_ARG_FILTERS: - fname = leading.group(0) - if filter_expr != fname: - raise ValueError( - f"{fname}: expected '| {fname}' with no arguments or " - f"trailing tokens, got '| {filter_expr}'" - ) - return _ZERO_ARG_FILTERS[fname](value) - - # Parse filter name and argument. Use fullmatch (not match) so trailing - # tokens after the closing paren — e.g. a comparison/boolean operator that - # binds looser than the pipe, as in ``count | default(0) > 5`` — are not - # silently discarded but fall through to the "unsupported form" ValueError - # below, mirroring the strict trailing-token handling of the from_json - # branch above. The greedy ``.+`` still handles literal ``)`` and ``|`` - # inside quoted args. - filter_match = re.fullmatch(r"(\w+)\((.+)\)", filter_expr) - # A multi-argument call is not a supported form: every filter here takes - # exactly one argument, and the whole captured argument text was handed to - # ``_evaluate_simple_expression`` as ONE expression. "1, 2" is not a valid - # expression, so it evaluated to None -- making ``default(1, 2)`` return - # None (silently wrong) and ``join(",", "extra")`` raise a message blaming - # the separator rather than the extra argument. Fall through to the - # unsupported-form error below instead, which names the filter and lists - # the accepted forms. - # - # Use ``_find_top_level``, the same scanner the operator splitting uses: it - # skips commas inside quotes AND inside nested brackets, so a single - # argument that happens to contain a comma still works -- ``join(", ")``, - # ``default("a, b")``, and the list literals the evaluator supports - # (``default([1, 2])``). - # - # List literals are the only container form ``_evaluate_simple_expression`` - # implements; a mapping such as ``{"a": 1}`` has no branch there and falls - # through to dot-path resolution, which yields ``None``. The scanner does - # skip commas inside braces too, so nothing here changes if that ever gains - # support -- but do not read this comment as a promise that it exists. - if filter_match and _find_top_level(filter_match.group(2), ",") != -1: - filter_match = None - if filter_match: - fname = filter_match.group(1) - farg = _evaluate_simple_expression(filter_match.group(2).strip(), namespace) - if fname == "default": - return _filter_default(value, farg) - if fname == "join": - return _filter_join(value, farg) - if fname == "map": - return _filter_map(value, farg) - if fname == "contains": - return _filter_contains(value, farg) - if fname == "split": - return _filter_split(value, farg) - # Filter without args - if filter_expr == "default": - return _filter_default(value) - # No recognized filter matched. Fail loudly rather than silently returning - # the unfiltered value. Distinguish a *registered* filter used in an - # unsupported form (e.g. `| join` or `| map` with no argument) from a - # genuinely unknown filter name, so the message names the real problem - # instead of calling a known filter "unknown". - name = leading.group(0) if leading else filter_expr - # Derived from the filter table, so the message lists exactly what is - # registered instead of a second copy of the set. - expected = _EXPECTED_FILTERS - if name in _REGISTERED_FILTERS: - raise ValueError( - f"filter '{name}' used in an unsupported form (got " - f"'| {filter_expr}'): {expected}" - ) - raise ValueError( - f"unknown filter '{name}': {expected} (got '| {filter_expr}')" - ) - - -# Order matters -- multi-char operators first, so "!=" is not split as "!" + "=". -# Shared with the remediation check so a validator cannot drift from what the -# evaluator will actually split on. -_COMPARISON_OPERATORS = ("!=", "==", ">=", "<=", ">", "<", " not in ", " in ") - -# Set only while `_collect_leaves` probes an expression; None everywhere else, so -# a normal evaluation costs one `.get()`. A ContextVar rather than a module global -# so concurrent probes cannot append into each other's list. -_leaf_sink: ContextVar[list[str] | None] = ContextVar("_leaf_sink", default=None) - - -def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any: - """Evaluate a simple expression against the namespace. - - Supports: - - Dot-path access: ``steps.specify.output.file`` - - Comparisons: ``==``, ``!=``, ``>``, ``<``, ``>=``, ``<=`` - - 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 - """ - expr = _collapse_whitespace(expr) - - # String literal — only when the WHOLE expression is one quoted string, - # i.e. the opening quote's matching close is the final character. Checking - # startswith/endswith alone would also grab `'a' == 'b'` and strip it to the - # garbage `a' == 'b`; a genuine single literal short-circuits here so quoted - # strings containing `|` or operator keywords are not mis-parsed downstream. - if expr[:1] in ("'", '"') and expr.find(expr[0], 1) == len(expr) - 1: - return expr[1:-1] - - # A parenthesised group. The operator scans below deliberately skip over - # bracketed text so an operator inside a quoted or nested operand is not - # split on -- which also means nothing ever looked inside a group that - # wraps the WHOLE expression. `(a or b) and c` split at the top-level - # `and`, then evaluated `(a or b)` as a dot path, found no such key, and - # returned None: the `or` was never evaluated and the whole thing read - # false. Unwrap here so grouping means what it says. - if _is_wrapped_in_parens(expr): - return _evaluate_simple_expression(expr[1:-1], namespace) - - # Handle pipe filters. Detect the pipe at the top level only, so a literal - # '|' inside a quoted operand (e.g. `inputs.x == 'a|b'`) or nested brackets is - # not mistaken for a filter separator — mirroring the operator parsing below. - # Filters chain left-to-right: `list | map('name') | join(', ')` feeds each - # filter's result into the next, so `map` (which yields a list) can be - # rendered by `join`. Splitting only at the first pipe would hand the whole - # tail to one filter and mangle any later `|`. - pipe_idx = _find_top_level(expr, "|") - if pipe_idx != -1: - segments = _split_top_level(expr, "|") - # The pipe is detected before the operators below, so a filter written on - # the right-hand operand of a comparison was applied to the comparison's - # BOOLEAN RESULT instead: `count > limit | default(5)` evaluated - # `count > limit` first and then `default` on the bool, which is a no-op, - # so the expression silently returned the comparison against the - # *unfiltered* operand. This is the mirror of a filter followed by a - # comparison (`default('7') > '5'`), which this module already refuses - # rather than guessing at the intended precedence. Refuse both the same - # way, so an ambiguous expression is reported instead of quietly - # producing the answer the author did not ask for. - head = segments[0].strip() - # Unary ``not`` is a leading prefix, not an infix token, so it has no - # surrounding space for the scan below to match -- it has to be checked - # the same way the parser itself does (``expr.startswith("not ")``). - # Without this, ``not inputs.missing | default(1)`` still evaluated - # ``not inputs.missing`` first and applied the filter to that boolean, - # which is the exact mis-binding this guard exists to reject. - # (A ``not`` that follows ``and``/``or`` is already caught by those - # tokens below.) - _ambiguous_op = "not" if head.startswith("not ") else None - if _ambiguous_op is None: - for _op in ("!=", "==", ">=", "<=", ">", "<", " not in ", " in ", - " or ", " and "): - if _find_top_level(head, _op) != -1: - _ambiguous_op = _op.strip() - break - if _ambiguous_op is not None: - raise ValueError( - f"ambiguous filter precedence in '{expr}': " - f"'| {segments[1].strip()}' would apply to the result of " - f"'{head}', not to an operand of '{_ambiguous_op}'. Filter the " - f"operand in its own expression instead." - ) - value = _evaluate_simple_expression(head, namespace) - sink = _leaf_sink.get() - for segment in segments[1:]: - if sink is None: - value = _apply_filter(value, segment.strip(), namespace) - continue - # Probing. A filter handed a placeholder can raise on it -- from_json - # on a mapping is the common one -- and letting that end the walk - # hides every leaf further along the chain, which is the one thing - # this collection exists to report: `inputs.blob | from_json | - # contains(bogus)` recorded `inputs.blob` and stopped, so `bogus` - # was never offered to _unresolvable_leaf. _apply_filter evaluates a - # filter's argument before it can raise on the value, so the leaves - # of the failing segment are already recorded when we get here. - # Carry a fresh placeholder so the next filter sees the same kind of - # unknown the namespace hands out. Real evaluation is untouched: the - # sink is armed only by _collect_leaves, and _evaluator_rejects runs - # its own probe without it, so a mis-wired filter is still reported. - try: - value = _apply_filter(value, segment.strip(), namespace) - except Exception: # noqa: BLE001 - probe values, not the author's text - value = _ProbeNamespace() - return value - - # Boolean operators — parse 'or' first (lower precedence) so that - # 'a or b and c' is evaluated as 'a or (b and c)'. Splits are quote/bracket - # aware so a keyword inside a quoted operand (e.g. the 'and' in - # 'read and write') is not mistaken for an operator. - or_idx = _find_top_level(expr, " or ") - if or_idx != -1: - left = _evaluate_simple_expression(expr[:or_idx].strip(), namespace) - # Validation probes must still visit operands that runtime can skip. - if bool(left) and not isinstance(namespace, _ProbeNamespace): - return True - right = _evaluate_simple_expression(expr[or_idx + 4:].strip(), namespace) - return bool(left) or bool(right) - - and_idx = _find_top_level(expr, " and ") - if and_idx != -1: - left = _evaluate_simple_expression(expr[:and_idx].strip(), namespace) - if not bool(left) and not isinstance(namespace, _ProbeNamespace): - return False - right = _evaluate_simple_expression(expr[and_idx + 5:].strip(), namespace) - return bool(left) and bool(right) - - if expr.startswith("not "): - inner = _evaluate_simple_expression(expr[4:].strip(), namespace) - return not bool(inner) - - # Comparison operators (order matters — check multi-char ops first). Split at - # the first top-level occurrence so an operator inside a quoted operand is - # ignored. - for op in _COMPARISON_OPERATORS: - op_idx = _find_top_level(expr, op) - if op_idx != -1: - left = _evaluate_simple_expression(expr[:op_idx].strip(), namespace) - right = _evaluate_simple_expression(expr[op_idx + len(op):].strip(), namespace) - if op == "==": - return left == right - if op == "!=": - return left != right - if op == ">": - return _safe_compare(left, right, ">") - if op == "<": - return _safe_compare(left, right, "<") - if op == ">=": - return _safe_compare(left, right, ">=") - if op == "<=": - return _safe_compare(left, right, "<=") - if op == " in ": - return _safe_membership(left, right, negate=False) - if op == " not in ": - return _safe_membership(left, right, negate=True) - - # Numeric literal - try: - if "." in expr: - return float(expr) - return int(expr) - except (ValueError, TypeError): - pass - - # Boolean literal - if expr.lower() == "true": - return True - if expr.lower() == "false": - return False - - # Null - if expr.lower() in ("none", "null"): - return None - - # List literal (simple) - if _is_single_list_literal(expr): - inner = expr[1:-1].strip() - if not inner: - return [] - items = [ - _evaluate_simple_expression(i.strip(), namespace) - for i in _split_top_level_commas(inner) - # Drop empty segments from trailing/leading/double commas ([1, 2,] -> - # [1, 2], not [1, 2, None]). An intentional empty-string element - # ('') strips to "''" (truthy), so ['', 'a'] is preserved. - if i.strip() - ] - return items - - # Variable reference (dot-path). This is the one place a substring stops being - # grammar and becomes a name to resolve, so it is where a probe can learn what - # the evaluator will actually look up. Literals have all returned above. - sink = _leaf_sink.get() - if sink is not None: - sink.append(expr) - return _resolve_dot_path(namespace, expr) - - -def _interpolate_expressions(template: str, namespace: dict[str, Any]) -> str: - """Substitute every top-level ``{{ ... }}`` block in *template*, quote-aware. - - Walks the template and, for each block, finds the closing ``}}`` that lies - outside string literals -- the same quote-scanning used by - ``_is_single_expression``. This keeps a literal ``}}`` inside a string - argument (e.g. ``| default('}}')``) from prematurely closing a block. - - ``_EXPR_PATTERN.sub`` cannot do this: its non-greedy body stops at the first - ``}}`` regardless of quoting, so in a multi-expression template any block - whose argument contains a literal ``}}`` is captured truncated and mis-parsed - (raising ``ValueError`` from the filter parser). #3208/#3228 fixed exactly - this for the single-expression fast path but left the interpolation path on - the old regex. - """ - out: list[str] = [] - i = 0 - n = len(template) - while i < n: - start = template.find("{{", i) - if start == -1: - out.append(template[i:]) - break - out.append(template[i:start]) - # Scan for the block-closing ``}}`` that is outside any string literal. - close = _find_block_close(template, start) - if close == -1: - # No quote-aware close. Two sub-cases, both kept identical to the old - # regex so a malformed template is never silently hidden: - # * a raw ``}}`` still exists in the tail (e.g. an unbalanced quote - # in a filter arg swallowed the real delimiter) -- fall back to - # that first raw ``}}`` and evaluate, letting the parser surface - # a ValueError just as ``_EXPR_PATTERN.sub`` would have. - # * no ``}}`` at all -- a genuinely unterminated ``{{``; leave the - # tail verbatim, again matching the regex (which cannot match). - raw_close = template.find("}}", start + 2) - if raw_close == -1: - out.append(template[start:]) - break - close = raw_close - val = _evaluate_simple_expression(template[start + 2:close].strip(), namespace) - out.append(str(val) if val is not None else "") - i = close + 2 - return "".join(out) - - -def _evaluate_template(template: str, namespace: dict[str, Any]) -> Any: - """Evaluate *template* against an already-built *namespace*. - - If the whole template is one ``{{ ... }}`` block, returns the raw value - (preserving type). Otherwise, interpolates each block inline and returns a - string. Split out of ``evaluate_expression`` so the API layer owns only - namespace construction and the engine contract exposes the whole decision. - - The single-expression fast path must fire only when the whole template is - one ``{{ ... }}`` block. Neither ``fullmatch`` nor a match-span check on - ``_EXPR_PATTERN`` can decide this reliably: the non-greedy body stops at the - first ``}}``, so ``fullmatch`` over-expands ``"{{ a }} {{ b }}"`` to garbage - (returning ``None`` and bypassing interpolation, issue #3208), while a span - check trips over a literal ``}}`` inside a string argument such as - ``{{ inputs.text | contains('}}') }}`` and mis-routes it to interpolation - (coercing its typed return to ``str``). ``_is_single_expression`` scans for a - block-closing ``}}`` outside string literals, so both cases resolve - correctly. - """ - stripped = template.strip() - if _is_single_expression(stripped): - return _evaluate_simple_expression(stripped[2:-2].strip(), namespace) - - # Multi-expression: interpolate each block inline. Uses a quote-aware scan - # (not ``_EXPR_PATTERN.sub``) so a literal ``}}`` inside a string argument - # in any block does not close that block early -- matching the handling the - # single-expression path above already got in #3208/#3228. - return _interpolate_expressions(template, namespace) - - -class _ProbeNamespace(dict): - """Namespace for the parse probe: every root exists, every leaf is absent. - - Enough for ``_evaluate_simple_expression`` to walk the grammar without needing - real inputs. Deliberately *not* resolving leaves to a sentinel value: a probe - that answers every lookup also answers ``inputs.count+1``, which is the - malformed shape the probe is meant to expose. - - The probes also pass one as the outer namespace, which tells - ``_evaluate_simple_expression`` to evaluate both operands of ``and``/``or`` - instead of short-circuiting, so a leaf or filter in a skipped operand is - still checked. - """ - - def __missing__(self, key: str) -> "_ProbeNamespace": # noqa: UP037 # pragma: no cover - return _ProbeNamespace() - - -def _evaluator_rejects(text: str) -> str | None: - """The evaluator's own complaint about how *text* is wired, or ``None``. - - Structural checks cannot establish that a core is parseable -- four rounds of - review found a new shape each time -- so this asks the evaluator. It reports - only the two failures ``_apply_filter`` raises about the expression itself: an - unknown filter name, and a registered filter used in an unsupported form. - - Anything else a probe run raises is about the probe's placeholder values, not - the author's text. ``steps.emit.output.stdout | from_json`` is valid against a - string output and is exercised in ``tests/test_workflows.py``; the probe hands - ``from_json`` a dict and it raises, so treating every error as a rejection - withheld a correction from a perfectly good condition. - """ - try: - _evaluate_simple_expression( - text, _ProbeNamespace({root: _ProbeNamespace() for root in _NAMESPACE_ROOTS}) - ) - except ValueError as exc: - message = str(exc) - # Every error _apply_filter raises about the filter *expression* quotes the - # segment back as `got '| ...'`. Its value errors instead name the type they - # received, which under a probe is the placeholder, not anything the author - # wrote -- treating those as rejections withheld corrections from valid - # conditions such as `steps.emit.output.stdout | from_json`. - if "got '| " in message: - return message.split(":", 1)[0] - except Exception: # noqa: BLE001 - probe values, not the author's text - return None - return None - - -def _collect_leaves(text: str) -> list[str]: - """Every substring *text* hands to the evaluator as a name to resolve. - - Runs the same probe ``_evaluator_rejects`` uses, with the leaf sink armed. - Literals never reach the dot-path resolution, and operands, filter arguments - and list elements all do -- probe evaluation visits both sides of - ``or``/``and`` eagerly rather than short-circuiting, so a leaf is - recorded whatever the other side is worth. - - A probe run can still raise on its own placeholder values, which is what - ``_evaluator_rejects`` sorts out. The leaves seen before that point are real - -- the evaluator reached them -- so they are kept rather than discarded: - ``inputs.tags | join(bogus)`` records ``bogus`` and only then trips over the - placeholder handed to ``join``. - """ - leaves: list[str] = [] - token = _leaf_sink.set(leaves) - try: - _evaluate_simple_expression( - text, _ProbeNamespace({root: _ProbeNamespace() for root in _NAMESPACE_ROOTS}) - ) - except Exception: # noqa: BLE001 - probe values, reported by _evaluator_rejects - pass - finally: - _leaf_sink.reset(token) - return leaves - - -# The operators the evaluator delimits with spaces; derived so the check cannot -# drift from _COMPARISON_OPERATORS. -_WORD_OPERATORS = tuple( - op for op in (" or ", " and ") + _COMPARISON_OPERATORS if op.startswith(" ") -) - - -def _has_incomplete_operand(text: str) -> bool: - """True when an operator in *text* is missing an operand on either side. - - Splits on **every** top-level occurrence rather than the first. Checking only - the first is the same defect this module exists to reject one level up: it let - ``inputs.a == inputs.b ==`` through, because the leading ``==`` has operands on - both sides and the scan stopped there. - - Reads ``_COMPARISON_OPERATORS`` from the evaluator rather than restating it, so - the check cannot drift from what ``_evaluate_simple_expression`` splits on. - """ - stripped = text.strip() - if not stripped: - return True - - # `not x` is a valid prefix form; `and x` and `or x` are not, and none of the - # three is valid alone or trailing. The keyword scans below use bare words - # because a leading operator has no space in front of it to match on. - if stripped in ("and", "or", "not") or stripped.endswith(" not"): - return True - # Word operators lose their delimiting space at the ends of a stripped core, so - # a trailing "not in" or a leading "and" needs matching without it. Derived from - # the evaluator's own table rather than restated. - for op in _WORD_OPERATORS: - if stripped.endswith(op.rstrip()) or stripped.startswith(op.lstrip()): - return True - - for op in (" or ", " and ") + _COMPARISON_OPERATORS: - if _find_top_level(stripped, op) == -1: - continue - if any(not segment.strip() for segment in _split_top_level(stripped, op)): - return True - - return _find_top_level(stripped, "|") != -1 and any( - not segment.strip() for segment in _split_top_level(stripped, "|") - ) - - -def _syntax_error(text: str) -> str | None: - """A parse-error message for *text*, or ``None``. - - This engine has no syntax layer: an unknown shape falls through to the - dot-path resolution and becomes ``None``, and the diagnostics describe that - through ``_collect_leaves`` and ``_has_incomplete_operand``. Grammar-based - engines return the parser's complaint here, naming the character and column - instead of classifying the text by heuristics; the diagnostics prefer it - when it is present. Always ``None`` here, so the hook changes nothing for - this engine. - """ - return None diff --git a/src/specify_cli/workflows/expressions/engine_lark_a2.py b/src/specify_cli/workflows/expressions/engine_lark.py similarity index 97% rename from src/specify_cli/workflows/expressions/engine_lark_a2.py rename to src/specify_cli/workflows/expressions/engine_lark.py index 033ad3fc7f..6543128b1d 100644 --- a/src/specify_cli/workflows/expressions/engine_lark_a2.py +++ b/src/specify_cli/workflows/expressions/engine_lark.py @@ -1,13 +1,10 @@ -"""Variant A2: Lark (LALR) as the parser, used the way the library is meant to be. +"""Lark (LALR) engine for workflow expressions. -The Earley spike (``engine_lark.py``) emulated ``engine_current``'s internals: -an if-chain filter dispatcher rebuilt from strings, the probe walk with its -leaf sink, and a grammar that accepted shapes the current parser refuses. This -engine answers the question A/C left open -- what a grammar-based parser costs -and buys when it is not written to mimic the old code: +The grammar replaces the former hand-written parser; the expression language +is unchanged. Only the block body is parsed here: * **The precedence guards are in the grammar, not in a post-check.** A filter - chain is only valid where the current parser allows one: the head is a plain + chain is only valid where the language allows one: the head is a plain operand, and the chain is a whole expression or sits in parentheses, a list element or a filter argument. ``count > limit | default(5)``, ``not x | default(1)`` and ``x | default(0) > 5`` do not parse at all; the first two diff --git a/src/specify_cli/workflows/expressions/filters.py b/src/specify_cli/workflows/expressions/filters.py index 67f330a4a4..22a13d6977 100644 --- a/src/specify_cli/workflows/expressions/filters.py +++ b/src/specify_cli/workflows/expressions/filters.py @@ -263,8 +263,8 @@ def _check_json_keys(value: Any) -> None: # -- The filter table ------------------------------------------------------ # # One entry per filter: the function, the call forms it accepts, and the usage -# text the error messages advertise. ``_REGISTERED_FILTERS``, ``_ZERO_ARG_FILTERS`` -# and the "expected one of ..." sentence are derived from this table, so a new +# text the error messages advertise. ``_REGISTERED_FILTERS`` and the +# "expected one of ..." sentence are derived from this table, so a new # filter is one row here — not a row here, a branch in the dispatcher and a # second edit to the message. The table order is the order the message lists. @@ -317,12 +317,3 @@ class FilterSpec: + ", or " + list(FILTER_TABLE.values())[-1].usage ) - - -# Filters that take no arguments and tolerate no trailing tokens. Keyed by name -# so a dispatcher can recognize a mis-wired form of any of them from the -# leading filter name alone. ``default`` is deliberately absent: it is the one -# filter that is valid both bare and with an argument. -_ZERO_ARG_FILTERS: dict[str, Callable[[Any], Any]] = { - name: spec.function for name, spec in FILTER_TABLE.items() if not spec.argument -} diff --git a/src/specify_cli/workflows/expressions/template.py b/src/specify_cli/workflows/expressions/template.py index 652fe76976..dad28019b6 100644 --- a/src/specify_cli/workflows/expressions/template.py +++ b/src/specify_cli/workflows/expressions/template.py @@ -7,10 +7,6 @@ from __future__ import annotations -import re - - -_EXPR_PATTERN = re.compile(r"\{\{(.+?)\}\}") def _is_single_expression(stripped: str) -> bool: @@ -21,8 +17,7 @@ def _is_single_expression(stripped: str) -> bool: argument contains a literal ``{{`` or ``}}`` (e.g. ``{{ inputs.text | contains('}}') }}``) on the typed fast path, while ``{{ a }} {{ b }}`` and ``{{ a }}{{ b }}`` are correctly seen as - multi-expression. Mirrors the quote handling in - ``_split_top_level_commas``. + multi-expression. A regex span check cannot decide this: the pattern's non-greedy body stops at the first ``}}``, so a literal ``}}`` inside a string argument would be @@ -123,40 +118,6 @@ def _split_top_level(text: str, sep: str) -> list[str]: start += idx + len(sep) -def _split_top_level_commas(text: str) -> list[str]: - """Split *text* on commas that are not inside quotes or nested brackets. - - Used for list-literal elements so a quoted element containing a comma - (e.g. ``["a, b", "c"]``) is not split mid-string, and nested lists/calls - (e.g. ``[[1, 2], 3]``) are kept intact. - """ - parts: list[str] = [] - buf: list[str] = [] - quote: str | None = None - depth = 0 - for ch in text: - if quote is not None: - buf.append(ch) - if ch == quote: - quote = None - elif ch in ("'", '"'): - quote = ch - buf.append(ch) - elif ch in "([{": - depth += 1 - buf.append(ch) - elif ch in ")]}": - depth = max(0, depth - 1) - buf.append(ch) - elif ch == "," and depth == 0: - parts.append("".join(buf)) - buf = [] - else: - buf.append(ch) - parts.append("".join(buf)) - return parts - - def _find_top_level(text: str, token: str) -> int: """Return the index of the first occurrence of *token* in *text* that lies outside any quoted string or nested bracket, or ``-1`` if there is none. @@ -186,66 +147,6 @@ def _find_top_level(text: str, token: str) -> int: return -1 -def _is_single_list_literal(expr: str) -> bool: - """Return ``True`` only when *expr* is exactly one bracketed list - literal -- the opening ``[`` closes at the FINAL character, not partway - through the string. - - ``expr.startswith("[") and expr.endswith("]")`` alone also matches a - list literal immediately followed by an index suffix, e.g. - ``[1,2,3][1]`` (meant as "index 1 of [1,2,3]", i.e. 2). Naively - stripping the outer brackets from that string yields the garbage - ``1,2,3][1``, which then silently evaluates to ``[1, 2, None]`` instead - of raising or resolving the index -- the same "grabs the wrong span" - failure mode the string-literal check above guards against, just never - given the same treatment for brackets. - """ - if not (expr.startswith("[") and expr.endswith("]")): - return False - quote: str | None = None - depth = 0 - n = len(expr) - for i, ch in enumerate(expr): - if quote is not None: - if ch == quote: - quote = None - continue - if ch in ("'", '"'): - quote = ch - elif ch in "([{": - depth += 1 - elif ch in ")]}": - depth -= 1 - if depth == 0: - return i == n - 1 - return False - - -def _is_wrapped_in_parens(text: str) -> bool: - """True when *text* is one parenthesised group, brackets and all. - - ``(a or b)`` is; ``(a) and (b)`` is not, because the opening paren closes - before the end. Quote-aware, so ``('(')`` does not count its own literal. - """ - if not (text.startswith("(") and text.endswith(")")): - return False - quote: str | None = None - depth = 0 - for index, ch in enumerate(text): - if quote is not None: - if ch == quote: - quote = None - elif ch in ("'", '"'): - quote = ch - elif ch == "(": - depth += 1 - elif ch == ")": - depth -= 1 - if depth == 0: - return index == len(text) - 1 - return False - - def _collapse_whitespace(text: str) -> str: """Strip *text* and turn each run of whitespace outside a quoted string into one space. diff --git a/tests/test_workflows.py b/tests/test_workflows.py index a1e8920e8a..443045d949 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -453,10 +453,10 @@ def test_list_literal_followed_by_index_is_not_misparsed_as_one_literal(self): silently evaluates to ``[1, 2, None]`` instead of raising or resolving the index. It must not be misclassified as one literal. - The grammar-based engine (A2) refuses the shape at the parser, with - the position in the message; the hand-written engine fell through to - unresolvable (``None``). Both avoid the wrong-looking list; the parse - error is the louder signal, so this test pins it. + The grammar-based engine refuses the shape at the parser, with + the position in the message; the former hand-written engine fell + through to unresolvable (``None``). Both avoid the wrong-looking list; + the parse error is the louder signal, so this test pins it. """ import pytest from specify_cli.workflows.expressions import evaluate_expression @@ -613,7 +613,7 @@ def test_filter_from_json_rejects_malformed_forms(self): # mis-wired form must raise rather than silently fall through to the # unknown-filter path and return the unparsed value. Parenthesized # forms reach the strict dispatcher; trailing garbage after the filter - # name is refused by the parser instead (A2: the grammar, not a regex, + # name is refused by the parser instead (the grammar, not a regex, # is the strictness), so those carry the parse message. import pytest from specify_cli.workflows.expressions import evaluate_expression @@ -919,7 +919,7 @@ def test_zero_arg_filters_reject_miswired_forms(self): # length/to_json. Every mis-wired form must raise naming the filter # rather than silently falling through to the unknown-filter path. # Parenthesized forms reach the strict dispatcher; trailing garbage - # after the name is a parse error (A2). + # after the name is a parse error. import pytest from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext @@ -1089,7 +1089,7 @@ def test_filter_call_with_trailing_tokens_fails_loudly(self): # A trailing operator/token after a filter comes out as a parse error # rather than being silently discarded. The grammar makes the filter # chain a complete expression: a comparison after it and garbage after - # its closing paren both fail to parse (A2 — previously the hand-written + # its closing paren both fail to parse (previously the hand-written # parser routed these to an "unsupported form" ValueError instead). import pytest from specify_cli.workflows.expressions import evaluate_expression diff --git a/tests/unit/test_condition_expression_block.py b/tests/unit/test_condition_expression_block.py index d95e3728a6..3df4638647 100644 --- a/tests/unit/test_condition_expression_block.py +++ b/tests/unit/test_condition_expression_block.py @@ -422,8 +422,8 @@ def test_wrapping_an_open_quote_is_not_a_repair(): The unwrapped form interpolates verbatim into non-empty text, so the condition reads true. The "wrapped" form is an expression the evaluator - refuses: the grammar-based engine (A2) raises a parse error for the - unterminated quote instead of the hand-written engine's silent ``False``. + refuses: the parser raises an error for the unterminated quote + instead of the former hand-written engine's silent ``False``. Either way, pasting the "correction" does not repair the condition. """ ctx = StepContext(inputs={"name": "Bob"}) @@ -558,7 +558,7 @@ def test_incomplete_operand_covers_every_operator_the_evaluator_splits_on(): ("inputs.f(]", "brackets do not balance"), ("inputs.items | truncate", "the evaluator rejects it"), ("inputs.tags | join", "used in an unsupported form"), - # The grammar-based engine (A2) names the position instead of classifying + # The grammar-based engine names the position instead of classifying # the text: `he said ...` fails at the second word, `count+1` at the `+`. # Previously: "is not a name the evaluator can resolve" / "is not a valid # path segment". @@ -882,8 +882,7 @@ def test_the_group_is_transparent_to_the_leaf_walk(): """The gate's operands are the names the evaluator's tree contains. A parenthesised group is not itself a name: its operands are the leaves. - The old probe got this from the evaluator unwrapping the group; the - parse-tree walk (A2) gets it from the grammar, so the assertion holds + The parse-tree walk gets this from the grammar, so the assertion holds without touching engine internals. """ grouped = "(inputs.a or inputs.b) and inputs.c" @@ -986,7 +985,7 @@ def test_gate_reads_the_shared_indexed_segment_definition(monkeypatch): be enough; if the gate keeps its own copy of the shape (as `_PATH_SEGMENT` used to), this fails. - With the grammar-based engine (A2) this also pins that the parser's `PATH` + With the grammar-based engine this also pins that the parser's `PATH` terminal is *derived* from `namespace`: the cache is keyed on the patterns, so the widened definition rebuilds the parser instead of serving a stale one. A grammar that hard-coded the segment shape would fail here. diff --git a/tests/unit/test_expression_package_boundary.py b/tests/unit/test_expression_package_boundary.py index 8202ffe31f..b9804dc03d 100644 --- a/tests/unit/test_expression_package_boundary.py +++ b/tests/unit/test_expression_package_boundary.py @@ -1,8 +1,8 @@ """Boundary test: only ``engine.py`` and the package facade see an engine. Runtime and validation must reach the evaluator through the ``engine`` contract, -never by importing ``engine_current`` (or a future ``engine_lark``) directly, or -a swapped engine would leave one of them on the old implementation. This pins +never by importing ``engine_lark`` (or any future engine) directly, or a +swapped engine would leave one of them on the old implementation. This pins the import boundary the package layout is built around. """ @@ -57,5 +57,5 @@ def test_the_engine_module_really_is_the_selection_point(): def test_the_boundary_check_detects_a_direct_engine_import(): - direct = "from .engine_current import _evaluate_template\n" + direct = "from .engine_lark import _evaluate_template\n" assert _imports_engine_implementation(direct, filename="diagnostics.py") From fc10f73ccce38f1f3d6c869138c0138af5b60011 Mon Sep 17 00:00:00 2001 From: Markus Date: Thu, 29 Oct 2026 13:10:04 +0100 Subject: [PATCH 05/12] fix(workflows): restore expression forms and messages lost in the Lark engine Review of the Lark engine against the former hand-written engine found inputs that now fail or report the wrong problem: - Number literals accepted by int()/float() before (`.5`, `5.`, `+5`, `1_000`, `1.5e3`) failed to lex. `1e3` stays invalid, as before. - `a == not b` no longer parsed; `not` is accepted on the right of a comparison again and negates only its operand. - Filters evaluated their arguments before checking the call form, so a failing argument hid a misused or unknown filter. The documented order (form first) is restored. - Condition validation reported an operator after a filter chain (`items | length > 0`) as "missing an operand", and `a || b` as "ambiguous filter precedence". The parser's expected-token set now separates the two. Documented as deliberate and pinned by tests: - All comparison operators share one precedence level, left to right (`a == b > 0` is `(a == b) > 0`). The former engine's scan order gave them accidental priorities. - Prose in `{{ }}` and path keys outside `[\w-]` are syntax errors instead of a None lookup. Assisted-by: GitHub Copilot CLI (model: Claude Opus 5.5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/reference/workflows.md | 6 +- .../workflows/expressions/engine_lark.py | 89 +++++++--- tests/test_workflows.py | 154 +++++++++++++++++- tests/unit/test_condition_expression_block.py | 31 +++- workflows/ARCHITECTURE.md | 8 +- 5 files changed, 254 insertions(+), 34 deletions(-) diff --git a/docs/reference/workflows.md b/docs/reference/workflows.md index cb4bc1fa96..a08592ea12 100644 --- a/docs/reference/workflows.md +++ b/docs/reference/workflows.md @@ -1204,7 +1204,7 @@ Notes on the newer filters: - **`to_json` output is deterministic.** Object keys are sorted and non-ASCII characters are left as-is rather than escaped, so the same value always serializes to the same bytes. That buys reproducibility only — it does **not** make the result safe to pass through a shell, because [interpolation adds no quoting or escaping](#interpolation-and-shell-safety). Do not interpolate unconstrained JSON into a `run` field. - **`to_json` rejects non-finite floats.** `NaN`, `Infinity`, and `-Infinity` raise a `ValueError` naming `to_json` instead of serializing to bare tokens. None of the three is valid JSON, so emitting them would hand a standards-compliant downstream parser a string it must reject. - **`to_json` requires string mapping keys.** JSON objects have string keys, so a mapping with any other key type — `{1: "a"}`, `{True: "a"}` — raises a `ValueError` naming the key type instead. Left to `json.dumps`, an integer key would be coerced to `"1"` and collide with an existing `"1"` key, while mixed key types would fail `sort_keys` with an ordering `TypeError` reported only as "not JSON-serializable". -- **Comparing filtered counts requires parentheses.** The parser splits on the top-level `|` before looking for operators, so an unparenthesized trailing comparison such as `{{ inputs.items | length > 0 }}` raises a `ValueError`. Parenthesize the filtered operand to compare its result: `{{ (inputs.items | length) > 0 }}`. For a threshold of at least two: +- **Comparing filtered counts requires parentheses.** A filter chain cannot be an operand of an operator, so an unparenthesized trailing comparison such as `{{ inputs.items | length > 0 }}` raises an "ambiguous filter precedence" `ValueError`. Parenthesize the filtered operand to compare its result: `{{ (inputs.items | length) > 0 }}`. For a threshold of at least two: ```yaml condition: "{{ (inputs.items | length) >= 2 }}" @@ -1218,6 +1218,10 @@ Notes on the newer filters: `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. +Operators bind from loosest to tightest as `or`, `and`, `not`, then the comparisons. All comparisons (`==`, `!=`, `>`, `<`, `>=`, `<=`, `in`, `not in`) share one level and are evaluated left to right; none has priority over another. `{{ a == b > 0 }}` therefore means `{{ (a == b) > 0 }}`. To combine comparisons, use parentheses or `and`/`or`. + +The body of every `{{ }}` block must be a valid expression. Text that is not one, such as prose (`{{ some text }}`), a key with characters other than letters, digits, `_` and `-` (`{{ inputs.weird:key }}`), or arithmetic (`{{ count + 1 }}`), raises a `ValueError` naming the offending token. Numbers may be written as `5`, `-5`, `+5`, `1_000`, `0.5`, `.5`, `5.`, or `1.5e3`. + Example: ```yaml diff --git a/src/specify_cli/workflows/expressions/engine_lark.py b/src/specify_cli/workflows/expressions/engine_lark.py index 6543128b1d..ebbbbc9da5 100644 --- a/src/specify_cli/workflows/expressions/engine_lark.py +++ b/src/specify_cli/workflows/expressions/engine_lark.py @@ -1,7 +1,16 @@ """Lark (LALR) engine for workflow expressions. -The grammar replaces the former hand-written parser; the expression language -is unchanged. Only the block body is parsed here: +The grammar replaces the former hand-written parser. Only the block body is +parsed here. The language is the same with these deliberate differences, all +documented in ``workflows/ARCHITECTURE.md``: + +* All comparison operators share one precedence level and group left to right + (``a == b > 0`` is ``(a == b) > 0``). The former engine's operator scan order + gave them accidental priorities; parenthesize to combine comparisons. +* Text that is not an expression -- prose inside ``{{ }}``, path keys with + characters outside ``[\\w-]`` -- is a syntax error instead of a ``None`` lookup. + +The rest of this docstring describes how the engine is built: * **The precedence guards are in the grammar, not in a post-check.** A filter chain is only valid where the language allows one: the head is a plain @@ -111,7 +120,10 @@ def _grammar(path_pattern: str) -> str: | comparison ?comparison: operand - | comparison CMP_OP operand -> cmp + | comparison CMP_OP cmp_rhs -> cmp + +?cmp_rhs: operand + | NOT cmp_rhs -> not_op ?operand: string | number @@ -141,7 +153,7 @@ def _grammar(path_pattern: str) -> str: FILTER_NAME: /[A-Za-z_][\w-]*/ PATH: /""" + path_pattern + r"""/ STRING: /'[^']*'/ | /"[^"]*"/ -NUMBER: /-?\d+(\.\d+)?/ +NUMBER: /[+-]?(?:(?:\d(?:_?\d)*\.(?:\d(?:_?\d)*)?|\.\d(?:_?\d)*)(?:[eE][+-]?\d(?:_?\d)*)?|\d(?:_?\d)*)/ %import common.WS %ignore WS """ @@ -226,18 +238,34 @@ def _parse(text: str) -> Any: def _parse_error_message(text: str, exc: UnexpectedInput) -> str: """The author-facing message for a parse failure of *text*. - The pipe case is the one carried-over semantic: a ``|`` where an operand - was expected is the shape the current evaluator refuses as an ambiguous - filter precedence, so it gets the same wording. Everything else names the - offending character and column. + Filter precedence is the one carried-over semantic. A filter chain cannot + be an operand of an operator, in either direction: a ``|`` after an + operator's operand (``count > limit | default(5)``), or an operator after a + filter chain (``items | length > 0``). Both get the same wording. Everything + else names the offending character and column. """ token = getattr(exc, "token", None) - if token is not None and token.value == "|" and getattr(token, "column", 1) > 1: + expected = set(getattr(exc, "expected", ()) or ()) + expects_operand = bool(expected & _OPERAND_EXPECTED) + if token is not None and token.type == "VBAR" and not expects_operand: return ( f"ambiguous filter precedence in '{text}': a filter cannot be an " f"operand of an operator. Filter the operand in its own expression " f"instead." ) + if ( + token is not None + and str(token.type) in _OPERATOR_TOKENS + and "VBAR" in expected + and not expects_operand + ): + operator = " ".join(str(token.value).split()) + return ( + f"ambiguous filter precedence in '{text}': '{operator}' follows a " + f"filter chain, and a filter chain cannot be an operand of an " + f"operator; parenthesize the filtered operand, as in " + f"'(inputs.items | length) > 0'" + ) if token is not None and token.type == "$END": return f"invalid expression '{text}': unexpected end of text" if isinstance(exc, UnexpectedCharacters): @@ -274,14 +302,15 @@ def _apply_filter( ) -> Any: """Apply one ``("filter", name, args, has_parens)`` node to *value*. - Arguments are evaluated here, before the table lookup, so a leaf in an - argument is visible to the diagnostics even when the filter name is wrong. - The accepted call forms and the error wording come from ``FILTER_TABLE`` - and``_EXPECTED_FILTERS``: the same table the message lists, so adding a - filter cannot leave the two disagreeing. + The call form is checked against ``FILTER_TABLE`` before any argument is + evaluated, so an unknown or misused filter is reported as such even when + an argument would itself fail. The diagnostics walk the parse tree for + argument leaves, so they do not need the arguments evaluated first. The + error wording comes from ``FILTER_TABLE`` and ``_EXPECTED_FILTERS``: the + same table the message lists, so adding a filter cannot leave the two + disagreeing. """ _, name, arg_nodes, has_parens = filt - args = [_eval(node, namespace_) for node in arg_nodes] display = f"{name}(...)" if has_parens else name spec = FILTER_TABLE.get(name) @@ -298,8 +327,8 @@ def _apply_filter( f"tokens, got '| {display}'" ) return spec.function(value) - if has_parens and len(args) == 1: - return spec.function(value, args[0]) + if has_parens and len(arg_nodes) == 1: + return spec.function(value, _eval(arg_nodes[0], namespace_)) if not has_parens and spec.bare: return spec.function(value) raise ValueError( @@ -529,6 +558,13 @@ def _evaluator_rejects(text: str) -> str | None: _OPERATOR_TOKENS = frozenset({"CMP_OP", "AND", "OR", "NOT"}) +# Terminals the parser expects exactly where an operand (``PATH``) or a filter +# name (``FILTER_NAME``) is missing. An operator token that fails while one of +# these is expected lacks an operand; one that fails after a filter chain +# (where only ``|``, ``(``, ``)`` and the like are expected) is a filter +# precedence mistake instead. +_OPERAND_EXPECTED = frozenset({"PATH", "FILTER_NAME"}) + # A word operator at the very start of the text. The LALR lexer only offers # CMP_OP/AND/OR/NOT where the grammar can take them, so without a left operand # `in`/`and`/`or` lex as paths and the failure lands on the *next* token; the @@ -542,11 +578,13 @@ def _has_incomplete_operand(text: str) -> bool: Reads the parse failure rather than restating the operator table: text that ends where an operand was expected (``inputs.a ==``, ``inputs.a and``, - ``inputs.x |``) fails at the ``$END`` token; a leading word operator - (``in inputs.a``, ``and inputs.x``) fails right behind the word. A failure - anywhere else (arithmetic, a stray character, a second literal) is a - syntax error, not an incomplete operand, and is reported by - ``_syntax_error`` instead. + ``inputs.x |``) fails at the ``$END`` token; an operator or ``|`` where an + operand or a filter name was expected (``inputs.a == == inputs.b``, + ``inputs.a || inputs.b``) fails on that token; a leading word operator + (``in inputs.a``, ``and inputs.x``) fails right behind the word. An operator + after a filter chain (``inputs.items | length > 0``) has both operands and + is a filter precedence mistake, reported by ``_syntax_error``, as is a + failure anywhere else (arithmetic, a stray character, a second literal). """ stripped = text.strip() if not stripped: @@ -554,10 +592,11 @@ def _has_incomplete_operand(text: str) -> bool: try: _parse(_collapse_whitespace(stripped)) except UnexpectedToken as exc: - if exc.token.type == "$END": - return True - if str(exc.token.type) in _OPERATOR_TOKENS: + token_type = str(exc.token.type) + if token_type == "$END": return True + if token_type in _OPERATOR_TOKENS or token_type == "VBAR": + return bool(set(exc.expected) & _OPERAND_EXPECTED) return bool(_LEADING_WORD_OPERATOR.match(stripped)) except UnexpectedInput: return False diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 443045d949..bd1936f13e 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1088,14 +1088,15 @@ def test_registered_filter_unsupported_form_raises(self): def test_filter_call_with_trailing_tokens_fails_loudly(self): # A trailing operator/token after a filter comes out as a parse error # rather than being silently discarded. The grammar makes the filter - # chain a complete expression: a comparison after it and garbage after - # its closing paren both fail to parse (previously the hand-written - # parser routed these to an "unsupported form" ValueError instead). + # chain a complete expression: a comparison after it is reported as a + # filter precedence mistake, and garbage after its closing paren fails + # to parse (previously the hand-written parser routed both to an + # "unsupported form" ValueError instead). import pytest from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext - with pytest.raises(ValueError, match="invalid expression"): + with pytest.raises(ValueError, match="ambiguous filter precedence"): evaluate_expression( "{{ inputs.missing | default('7') > '5' }}", StepContext(inputs={}) ) @@ -1662,6 +1663,151 @@ def test_boolean_precedence_without_parentheses(self): # Explicit grouping overrides the default and flips the result. assert evaluate_expression("{{ (inputs.a or inputs.b) and inputs.c }}", ctx) is False + def test_comparisons_share_one_level_and_group_left_to_right(self): + """No comparison operator has priority over another. + + The former engine's operator scan order made ``==``/``!=`` bind looser + than ``<``/``>``/``in`` and grouped repeated comparisons from the right. + The grammar puts every comparison on one level, left-associative, so + ``a == b > 0`` is ``(a == b) > 0``. Each case is checked against its + explicit grouping and against the other grouping, which gives a + different result, so the assertion fails if the precedence moves. + """ + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext( + inputs={"enabled": True, "count": 5, "mode": "x", "tags": ["x"]} + ) + cases = [ + ("inputs.enabled == inputs.count > 0", + "(inputs.enabled == inputs.count) > 0", + "inputs.enabled == (inputs.count > 0)"), + ("true == inputs.mode in inputs.tags", + "(true == inputs.mode) in inputs.tags", + "true == (inputs.mode in inputs.tags)"), + ("inputs.enabled != inputs.count >= 10", + "(inputs.enabled != inputs.count) >= 10", + "inputs.enabled != (inputs.count >= 10)"), + ("1 < 2 < 3", "(1 < 2) < 3", "1 < (2 < 3)"), + ("3 > 2 > 1", "(3 > 2) > 1", "3 > (2 > 1)"), + ] + for plain, left, right in cases: + result = evaluate_expression("{{ " + plain + " }}", ctx) + assert result == evaluate_expression("{{ " + left + " }}", ctx), plain + assert result != evaluate_expression("{{ " + right + " }}", ctx), plain + # Comparisons still bind tighter than not/and/or. + assert evaluate_expression("{{ not inputs.count > 9 }}", ctx) is True + assert evaluate_expression( + "{{ inputs.count > 9 or inputs.mode == 'x' }}", ctx + ) is True + + def test_number_literal_forms(self): + """Every number form the former engine read through ``int()``/``float()``. + + A ``.`` makes a float (exponents need one too, as before); a sign and + digit-group underscores are allowed. + """ + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"ratio": 0.7}) + for text, expected in [ + ("5", 5), ("-5", -5), ("+5", 5), ("1_000", 1000), + ("0.5", 0.5), (".5", 0.5), ("-.5", -0.5), ("5.", 5.0), + ("1.5e3", 1500.0), (".5E-1", 0.05), ("1_000.5", 1000.5), + ]: + result = evaluate_expression("{{ " + text + " }}", ctx) + assert result == expected and type(result) is type(expected), text + assert evaluate_expression("{{ inputs.ratio > .5 }}", ctx) is True + assert evaluate_expression("{{ [.5, +1] }}", ctx) == [0.5, 1] + + def test_malformed_number_literals_are_refused(self): + """Forms ``int()``/``float()`` refuse (or, for ``1e3``, the former engine + never read as a number) do not lex as one either.""" + import pytest + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + for text in ("1__0", "1_", "1e3", "5..", "."): + with pytest.raises(ValueError, match="invalid expression"): + evaluate_expression("{{ " + text + " }}", StepContext(inputs={})) + + def test_not_on_the_right_of_a_comparison(self): + """``a == not b`` compares against the negation, as before.""" + import pytest + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"a": False, "b": True}) + assert evaluate_expression("{{ inputs.a == not inputs.b }}", ctx) is True + assert evaluate_expression("{{ inputs.a != not inputs.b }}", ctx) is False + assert evaluate_expression("{{ inputs.b == not not inputs.b }}", ctx) is True + # The negation covers only its operand; `and` still splits the whole. + assert evaluate_expression( + "{{ inputs.a == not inputs.b and inputs.a }}", ctx + ) is False + with pytest.raises(ValueError, match="unexpected end of text"): + evaluate_expression("{{ inputs.a == not }}", ctx) + # A filter after it is still a filter precedence mistake. + with pytest.raises(ValueError, match="ambiguous filter precedence"): + evaluate_expression("{{ inputs.a == not inputs.b | default(1) }}", ctx) + + def test_filter_form_is_checked_before_arguments_are_evaluated(self): + """A misused or unknown filter is reported as such, even when evaluating + its argument would itself raise.""" + import pytest + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"s": "hi", "n": None}) + with pytest.raises(ValueError, match=r"upper: expected '\| upper' with no arguments"): + evaluate_expression("{{ inputs.s | upper(inputs.n | split(',')) }}", ctx) + with pytest.raises(ValueError, match="filter 'join' used in an unsupported form"): + evaluate_expression("{{ inputs.s | join(inputs.n | upper, 1) }}", ctx) + with pytest.raises(ValueError, match="unknown filter 'nope'"): + evaluate_expression("{{ inputs.s | nope(inputs.n | upper) }}", ctx) + # A correctly used filter still evaluates its argument and surfaces its + # error. + with pytest.raises(ValueError, match="upper: expected a string value"): + evaluate_expression("{{ inputs.s | join(inputs.n | upper) }}", ctx) + + def test_operator_after_a_filter_chain_is_a_filter_precedence_error(self): + """Both directions of a filter used as an operator's operand get the + filter-precedence message; a doubled ``|`` does not.""" + import pytest + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"items": [1], "s": "hi", "t": True, "a": 1, "b": 2}) + for text in ( + "inputs.items | length > 0", + "inputs.s | contains('h') and inputs.t", + "(inputs.s | upper) | lower == 'hi'", + ): + with pytest.raises(ValueError, match="ambiguous filter precedence"): + evaluate_expression("{{ " + text + " }}", ctx) + assert evaluate_expression("{{ (inputs.items | length) > 0 }}", ctx) is True + with pytest.raises(ValueError) as excinfo: + evaluate_expression("{{ inputs.a || inputs.b }}", ctx) + assert "ambiguous filter precedence" not in str(excinfo.value) + assert "unexpected '|' at column 11" in str(excinfo.value) + + def test_text_that_is_not_an_expression_is_refused(self): + """Prose inside ``{{ }}`` and path keys outside ``[\\w-]`` are syntax + errors. The former engine looked them up as paths and rendered the + miss as an empty string or ``None``.""" + import pytest + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"weird:key": "v", "ok-key": "v"}) + with pytest.raises(ValueError, match="invalid expression 'some text'"): + evaluate_expression("a {{ some text }} b", ctx) + with pytest.raises(ValueError, match="unexpected character ':'"): + evaluate_expression("{{ inputs.weird:key }}", ctx) + assert evaluate_expression("{{ inputs.ok-key }}", ctx) == "v" + def test_item_and_fan_in_namespace(self): """The ``item`` and ``fan_in`` roots resolve inside fan-out/fan-in, like ``inputs``/``steps``/``context``. Only the diagnostics probed them.""" diff --git a/tests/unit/test_condition_expression_block.py b/tests/unit/test_condition_expression_block.py index 3df4638647..6c4d20375a 100644 --- a/tests/unit/test_condition_expression_block.py +++ b/tests/unit/test_condition_expression_block.py @@ -547,6 +547,25 @@ def test_incomplete_operand_covers_every_operator_the_evaluator_splits_on(): assert _has_incomplete_operand("inputs.a" + op + "inputs.b") is False, op +@pytest.mark.parametrize( + "text,incomplete", + [ + ("inputs.items | length > 0", False), + ("inputs.s | contains('h') and inputs.t", False), + ("(inputs.x | upper) | lower == 'x'", False), + ("inputs.a || inputs.b", True), + ("inputs.a == == inputs.b", True), + ("| upper", True), + ], +) +def test_incomplete_operand_tells_a_filter_chain_from_a_missing_operand( + text, incomplete +): + """An operator token only means a missing operand where an operand was + expected; after a filter chain it is a filter precedence mistake.""" + assert _has_incomplete_operand(text) is incomplete + + # Copilot round 3: the first two gates each inspected only one position. These pin # every-position scanning, both ends, and bracket-type matching. MULTI_POSITION_UNFIXABLE = [ @@ -559,14 +578,22 @@ def test_incomplete_operand_covers_every_operator_the_evaluator_splits_on(): ("inputs.items | truncate", "the evaluator rejects it"), ("inputs.tags | join", "used in an unsupported form"), # The grammar-based engine names the position instead of classifying - # the text: `he said ...` fails at the second word, `count+1` at the `+`. + # the text: `he said ...` fails at the second word, `count+1` at the `+1` + # (a signed number literal). # Previously: "is not a name the evaluator can resolve" / "is not a valid # path segment". ('he said "hi" then left', "unexpected 'said'"), - ("inputs.count+1", "unexpected character '+' at column 13"), + ("inputs.count+1", "unexpected '+1' at column 13"), ("inputs.a === inputs.b", "unexpected character '=' at column 12"), ("bogus == 'x'", "is not one of the namespace roots"), ("inputs.payload | from_json()", "the evaluator rejects it"), + # An operator after a filter chain has both operands: it is a filter + # precedence mistake, not a missing operand. A doubled `|` or operator is + # the missing operand. + ("inputs.items | length > 0", "ambiguous filter precedence"), + ("inputs.s | contains('h') and inputs.t", "ambiguous filter precedence"), + ("inputs.a || inputs.b", "missing an operand"), + ("inputs.a == == inputs.b", "missing an operand"), ] diff --git a/workflows/ARCHITECTURE.md b/workflows/ARCHITECTURE.md index 1ce065778a..5664d323cf 100644 --- a/workflows/ARCHITECTURE.md +++ b/workflows/ARCHITECTURE.md @@ -192,7 +192,7 @@ Workflow definitions use Jinja2-like `{{ expression }}` syntax for dynamic value | Comparisons | `==`, `!=`, `>`, `<`, `>=`, `<=` | `{{ count > 5 }}` | | Boolean logic | `and`, `or`, `not` | `{{ items and status == 'ok' }}` | | Membership | `in`, `not in` | `{{ 'error' not in status }}` | -| Literals | strings, numbers, booleans, lists | `{{ true }}`, `{{ [1, 2] }}` | +| Literals | strings, numbers (`5`, `-5`, `+5`, `1_000`, `0.5`, `.5`, `5.`, `1.5e3`), booleans, `none`/`null`, lists | `{{ true }}`, `{{ [1, 2] }}` | | Filter: `default` | `{{ val \| default('fallback') }}` | Fallback for `None` or an empty string | | Filter: `join` | `{{ list \| join(', ') }}` | Join list elements | | Filter: `contains` | `{{ text \| contains('sub') }}` | Substring/membership check | @@ -206,13 +206,17 @@ Workflow definitions use Jinja2-like `{{ expression }}` syntax for dynamic value **Single expressions** (`{{ expr }}` only) return typed values. **Mixed templates** (`"text {{ expr }} more"`) return interpolated strings. +**Operator precedence.** From loosest to tightest: `or`, `and`, `not`, then the comparisons (`==`, `!=`, `>`, `<`, `>=`, `<=`, `in`, `not in`). All comparisons share **one** level and group left to right, so no comparison has priority over another: `{{ a == b > 0 }}` is `{{ (a == b) > 0 }}`, and `{{ 1 < 2 < 3 }}` is `{{ (1 < 2) < 3 }}`. The former hand-written engine gave comparisons accidental priorities through its operator scan order; parenthesize, or join comparisons with `and`/`or`, instead of relying on either reading. A `not` on the right of a comparison negates only its operand: `{{ a == not b }}`. + +**Only expressions parse.** The body of every `{{ }}` block is parsed by a grammar, so text that is not an expression is a `ValueError` naming the offending token and column: prose such as `{{ some text }}`, path keys with characters outside letters, digits, `_` and `-` (`{{ inputs.weird:key }}`), arithmetic, or two adjacent operands. The former engine looked such text up as a path and rendered the miss as an empty string or `None`. Conditions are checked at validation time; other fields fail when the step runs. + **Filter argument strictness (new filters).** The five filters added in #4766 validate their supported inputs and raise `ValueError` naming the problem rather than leaking a Python `TypeError`/`AttributeError`: `upper`/`lower` accept only strings, `split` requires a string value and a non-empty string separator, `length` accepts only lists and strings, and `to_json` requires a JSON-serializable value whose mapping keys are strings, and additionally rejects non-finite floats (`NaN`, `Infinity`, `-Infinity`), which `json.dumps` would otherwise emit as bare tokens that are not valid JSON. Keys are checked before serialization because `json.dumps` would otherwise coerce `1` to `"1"` (colliding with an existing `"1"` key), while `sort_keys=True` raises an ordering `TypeError` on mixed key types — both surfacing as a generic "not JSON-serializable" that hides the authoring mistake. Coercion is deliberately rejected for these filters: a type mismatch almost always means the pipeline is wired to the wrong variable, and a coerced result (e.g. `"3"` for an int) would hide that. Arity is checked before the argument expression is evaluated, so a filter used with the wrong number of arguments — `| upper('x')`, `| split` with no separator, `| split(',', 1)`, `| join` bare — is reported as a known filter misused, distinct from an entirely unknown name. The older filters are more permissive and are unchanged: `join` stringifies unsupported value shapes and elements, and `map`/`contains` return fallback values for unsupported inputs rather than raising. **`to_json` output is deterministic.** It pins `sort_keys=True` and `ensure_ascii=False`, so the same value always serializes to the same bytes regardless of dict insertion order or platform, and non-ASCII text is not `\uXXXX`-escaped. This buys reproducibility only — it does **not** make the result safe to pass through a shell, because expression interpolation adds no quoting or escaping. See [Interpolation and shell safety](../docs/reference/workflows.md#interpolation-and-shell-safety) before interpolating JSON into a `run` field. -**Filters and comparisons.** The parser splits on the top-level `|` before looking for operators, so an unparenthesized trailing comparison such as `{{ inputs.items | length > 0 }}` raises a `ValueError`. Parenthesize the filtered operand to compare its result: `{{ (inputs.items | length) > 0 }}`. For a threshold of at least two: +**Filters and comparisons.** A filter chain cannot be an operand of an operator, so an unparenthesized trailing comparison such as `{{ inputs.items | length > 0 }}` raises an "ambiguous filter precedence" `ValueError`, as does a filter on an operator's operand (`{{ count > limit | default(5) }}`). Parenthesize the filtered operand to compare its result: `{{ (inputs.items | length) > 0 }}`. For a threshold of at least two: ```yaml condition: "{{ (inputs.items | length) >= 2 }}" From d17ae133f671972140e6dd2871939bb1868bca40 Mon Sep 17 00:00:00 2001 From: Markus Date: Fri, 30 Oct 2026 01:43:53 +0100 Subject: [PATCH 06/12] chore(deps): refresh security audit snapshot for lark Regenerated with the command documented in CONTRIBUTING.md so the dependency-audit job finds lark in the committed snapshot. The command upgrades, so a few unrelated pins moved to their latest patch releases. Assisted-by: GitHub Copilot (model: Claude Sonnet 5.5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/security-audit-requirements.txt | 314 ++++++++++++------------ 1 file changed, 161 insertions(+), 153 deletions(-) diff --git a/.github/security-audit-requirements.txt b/.github/security-audit-requirements.txt index 23c40ba85d..fd2718d3f1 100644 --- a/.github/security-audit-requirements.txt +++ b/.github/security-audit-requirements.txt @@ -360,6 +360,10 @@ jsonschema-specifications==2025.9.1 \ --hash=sha256:98802fee3a11ee76ecaca44429fda8a41bff98b00a0f2838151b113f210cc6fe \ --hash=sha256:b540987f239e745613c7a9176f3edb72b832a4ac465cf02712288397832b5e8d # via jsonschema +lark==1.3.1 \ + --hash=sha256:b426a7a6d6d53189d318f2b6236ab5d6429eaf09259f1ca33eb716eed10d2905 \ + --hash=sha256:c629b661023a014c37da873b4ff58a817398d12635d3bbb2c5a03be7fe5d1e12 + # via specify-cli (pyproject.toml) markdown-it-py==4.2.0 \ --hash=sha256:04a21681d6fbb623de53f6f364d352309d4094dd4194040a10fd51833e418d49 \ --hash=sha256:9f7ebbcd14fe59494226453aed97c1070d83f8d24b6fc3a3bcf9a38092641c4a @@ -376,9 +380,9 @@ mdurl==0.1.2 \ --hash=sha256:84008a41e51615a49fc9966191ff91509e3c40b939176e643fd50a5c2196b8f8 \ --hash=sha256:bb413d29f5eea38f31dd4754dd7377d4465116fb207585f97bf925588687c1ba # via markdown-it-py -opentelemetry-api==1.45.1 \ - --hash=sha256:aa38ed19bcc084ba42782a73255b3582283eced7ad6dddbd6695189e69adfb75 \ - --hash=sha256:b31553efa588ae44bc306f863c785c5333a9ecc091248c6ee68b4b6c87fdedfb +opentelemetry-api==1.45.2 \ + --hash=sha256:2cbd737377e710ee740d261c94e93fa0f08e6d037c18f17513cc0a2d94479a34 \ + --hash=sha256:d7b0008e21e5fd61cbe7a7f34ca3b6905a78ab677d9bc395e299359c75cda6b4 # via mcp packaging==26.3 \ --hash=sha256:94edc256424af38762eb31306eed28beb9f0efc50a8837492c9d6fd6004aed79 \ @@ -400,151 +404,155 @@ pycparser==3.12 ; implementation_name != 'PyPy' and platform_python_implementati --hash=sha256:a076f3f489b39b43ed13563a1dafe42447445bec2e294154ec5c57fcdda4c85e \ --hash=sha256:f03387877eab82ce01bbdfd0ba7252ee55a90b1c708d5129e2ce915475cf1db6 # via cffi -pydantic==2.14.1 \ - --hash=sha256:9195d967ec791692a04438115466764fb8b9a27b31f14a760437694f40d6b454 \ - --hash=sha256:94f478203dd03404682a1ada216965651dd74b1d2d5ffd62e00e0837caab5c26 +pydantic==2.14.2 \ + --hash=sha256:7efbf7507b937931cd32a5755acb00ea1cf5ccb1d4e8067a5850d5069e382118 \ + --hash=sha256:fb7b2db6cefe8414dd612edeede49c4c433acb0432b0f92b95024e45f66c505d # via # specify-cli (pyproject.toml) # mcp # mcp-types -pydantic-core==2.50.1 \ - --hash=sha256:0036473f5583e6a60e50b8b21651511564277a3f05cc5dab8cf579f552cd5f6c \ - --hash=sha256:0048b6dddc8ef4b64fccaad878bd143b0c3882ea9936279dc11d613f6b7dd1bc \ - --hash=sha256:009634b83993777ddcd69cad0ffcace43dabde692109528e35f0fde91e386a8b \ - --hash=sha256:028e2f212273d4a39b1ec1e0de8166b1165a65fc0f1111452a9d94fc7c625c63 \ - --hash=sha256:062e891facce5ca296a1c37098e5e466780457f86413894b399f0cf22934f769 \ - --hash=sha256:06e01fbbfdb9be777b316a71b6c49efaf4a08b615d0a98d678cda3023f79d019 \ - --hash=sha256:06ead20d39ffd6f2f6f2a8f8a6de67ff8bb1b4f14a8a30e058502514ee2ac685 \ - --hash=sha256:0b3a6f334c6a2345ca15318ff894502a90012536404b37c844a976c76c846e0b \ - --hash=sha256:0c003c3b7f49debb893d2d85ae099ac5959c9839e2f330fadb1fcdf7a6594482 \ - --hash=sha256:131059670f1d2444269b8585cb888963994871932447c08b39ac6a51fcfef658 \ - --hash=sha256:132529c83901437ff642f585216831bf5fd7a91df66829907e155192ead62498 \ - --hash=sha256:17e722e156d0444ecaefbe640bdb60928752bf2013e2b7a11cdb099aaae19bec \ - --hash=sha256:1a9006395dece0e32e704c315eff8a00bede494f6108546cfc5539c89fef4f9a \ - --hash=sha256:1c8632d4ac04e6f91128fca584b3a8a507d81604c24eeaaad00d4be42765c32b \ - --hash=sha256:1c96fd793b73d1b92e65570132505498fe7b21eaef73cdf74e67e5dfba7ac9e4 \ - --hash=sha256:1cf41f1ae3fa155cf167a72689ad044bcc1e3c97e064123677149bdfb5dafc4a \ - --hash=sha256:1deeacb112d14d3f4fcb16b165f7dbaf76c70ba6e82f37ba042bdab51970a0b8 \ - --hash=sha256:1ef800dd7d85bcdadf4c3076e4c94e43939493558a3b69a1ea830c706d4617bb \ - --hash=sha256:1fa4c8bc12c1354c5550c0c35c1852c8c1901e89e06561724e03f8d0342e1f87 \ - --hash=sha256:2005207aafe1231315718bf6ed5d064a7300fb4772754af35ee72fc68159492e \ - --hash=sha256:23923ab9292c40da026330b1ecf4dc2618c8e86e0422e5d1fbf50d94d64ca4f8 \ - --hash=sha256:23edad659e8dbd8ca7e4e877fe6c81573abbdf215bd25a68b53e1272f58b80c7 \ - --hash=sha256:2ab756b72bd5054e4c7ef3ded331b35786cbd3cf931531a508f79a9537517064 \ - --hash=sha256:2cbd1b75b09e976ed0d6b6ca297675632ca35df86130088457cdc60ef36970ae \ - --hash=sha256:2cf91809d0721ab81592ba67bea7694821679c10b1a2e3c3460082b286c1918a \ - --hash=sha256:2df1ff41884de2bc4b307bafd7c40a691094fad2ff8e767e5b45a319257bcf4e \ - --hash=sha256:2eb75304506894a281d346220a4f7481a1b8729577c5ed2a05395991966a8396 \ - --hash=sha256:2eedf82ee4753cdab8e50044c6bd569577eebc3859b11fecf4eb9223761ff966 \ - --hash=sha256:30ddf019d082c117b5d309e5b86710c2a78909907ec1a9381feec3eec02eca0b \ - --hash=sha256:325c23f3e35cfbf0fe3486fa5f7260d1e45885173002d30a28ca019994124255 \ - --hash=sha256:32fad3a91e51b6d2039c572db04a5a873260b399f6bd62c3552671fa7a4a2899 \ - --hash=sha256:36c426eac0af8d1529ff8467e612b933346caec1fdc0d774f78f67a1a11e16c1 \ - --hash=sha256:3a5fce22f1e87d181e924e12da7d81cfe031fb3881a5ddf26ad28f141756ca43 \ - --hash=sha256:3aa9de446b793de2beb6fa2d9d0961803126c4e2a99c2f25ab59b9fd6ea125c0 \ - --hash=sha256:3e46a9eb0a0901dd6275e6b06ac3a464885ef350ec4121fe486869de8053e4bb \ - --hash=sha256:3fde4fdc6487a58d944ca87cf5adc95d5f266e872c19599f5f4c0a8a1b1f9f9f \ - --hash=sha256:409e0ea40ec30d9158f33574fd758e689f6045a0f2596701828c27816ca9687d \ - --hash=sha256:40f523349960fa30f3ea51404308ff50f9997a90df639590f47a057c1f32b415 \ - --hash=sha256:41bc8237121bd8dc8d888dfd6279fc166ffc88c1f1bf3a8bf00869680533ca4c \ - --hash=sha256:42b54c2c90ad348b5e3a85e03e715d572c1fde357ef104cdfe3b03b697a404ea \ - --hash=sha256:455a773617b5913bf5c20d0692e5787b119e52c4d40ea644ca31f5758fd31be2 \ - --hash=sha256:45b11cac094aa25725581d9304eee93c9028516b9ea80dd9e175e13a5a2c840e \ - --hash=sha256:45c6266d071c241f2a168d45bf8c54344f0effce35e7e6b73afdec11f3687568 \ - --hash=sha256:46b3301d3b5c886f77de7546e47274a5842c622ea2020b8c6524c6b66913b4a6 \ - --hash=sha256:476f6ed8e43cd1e0b460920e23571700872b284e77331cb30c4faf459cf48a4b \ - --hash=sha256:48569b0ade9edfbe065cad1d700175546592aebbb42f02adcebcc26e75b896fe \ - --hash=sha256:49c2cbb2397fe4d0987e84606e691af6cb87bc0ee1bd3e7b737f7e10b4c142f9 \ - --hash=sha256:4a53d13cdfbedbfa87f08b83c1a0a5efcc767d785a4b41934fa9cb672670493a \ - --hash=sha256:4be846f55c9477f5f3ddde8f2ce941137e16862a56d018ed885d422bb6ae02f2 \ - --hash=sha256:4df197990c15b5a37c5a277d131d9f2c67de6133f2e5dafd80d9bba4b99f46f9 \ - --hash=sha256:4e834f6a8e4ff772dcc34f58ef5504147a3ea5b0f4eeb13b0f8eb2ca75ac57f1 \ - --hash=sha256:57f51b31ff826e2859120cf4737c5a758a48d96f3e97da40ccee1796d58078ff \ - --hash=sha256:5958c72adb417c39b12ac87525ac60b0d73315fcdc59e21f44ee4a5e2512c9ef \ - --hash=sha256:5dfe41f232befddb9c4377f6cfc702b51595e2d78ed082672adf8758d2c4619f \ - --hash=sha256:5f3cae32fc46121f787cb2486de9cf95a8bf72aec5cc78f64c606fa1735a6ef5 \ - --hash=sha256:64f6047f62a6c5ae08d0a6afb035667aa2d97c3d20d69762e034c5ea144d92a5 \ - --hash=sha256:6a733778df2f7087ec1100ed0b41533e4f3001976e99570fa34f57c66e7f8e3e \ - --hash=sha256:6b20a4bffabdad0db2927ac034ae3b8a681b1f7a0182f3e60b479ad2fde21ebb \ - --hash=sha256:6dbcbee53bf17196a7f745aa9bf5a9603953a1e365b1f020be3207c676a3e7c4 \ - --hash=sha256:6ed4f3cef55164b026fefb41341b7754cc6b624c75dfe7142d2ecceb5ad21c87 \ - --hash=sha256:704075d10b74f2f3c6e15407c696d88701df35fc8953f434a431add0d0074db0 \ - --hash=sha256:739dc730e6be3bd5ec2f4ab5cfc7eb047cc45fc1497b3bafec74ff2ed07df597 \ - --hash=sha256:7456d699b13954e9c0164dcb267250a10ae0dfb03e6e26d6796ab0d46e189c84 \ - --hash=sha256:756d669f04e62ec4148ecfe22be6a4484d9b1181a6ef32e205ebfd200540858b \ - --hash=sha256:7689580e72a642ab5ec64d5f55b2e33636fa43b4ebe63c0c2c965ef307c7d1aa \ - --hash=sha256:76e2e83fa6ec8cdc972d438dafc2522b3a47bee4ec0ae668b29cfb1977ab5242 \ - --hash=sha256:7816e98acc08119dc0f340ab167048ecc54126316330c1f0caf7c6756c88e28f \ - --hash=sha256:79490e33c4c0fcb933bbbcfc3a62184d8803b99f535863dfbb925e1bcb6945ad \ - --hash=sha256:7f476456ac2bb0d937f75191494a09c83a30765fea4f70f3b404942fe25f6cdf \ - --hash=sha256:844b869f118e22a41a091bdcedda8a71bc1b0f62c38d1a0c3211cece47e1d8fc \ - --hash=sha256:84bc765b282a9d5b7fe0348b8648904f25a6a04b2139da52b1dd30c8ac3a2c8f \ - --hash=sha256:84f34323a61a365b4e9295de6028474754829aaddd59c7bf1a040e7487ef8f3c \ - --hash=sha256:8812592c85d0edf423f10eadcef42716d71e8219085ad9e85b775057b7306133 \ - --hash=sha256:88e492e8b9d0312e7dc13667c30222abf284dc3b79b5302b3607b41a5784ce61 \ - --hash=sha256:8a6791afa2245e6c6b180122d105941644f5bd410bb18623b408808cc41a3102 \ - --hash=sha256:8b4c3df25bd323bf1d36a648d563cf1fc69d717451569927151bdad7cad07a77 \ - --hash=sha256:8daa7ee75245d43ad7d747e5c9ecc1b1d06552f72b14887e9276f787d57375f4 \ - --hash=sha256:93ba4e9d8210d941c200431a56b2c0400b131865947903937ed3ec5404307d2e \ - --hash=sha256:94845ff54dc5193f228cab81b2662a04bfbb892e95bdc15edf7399000ce57d54 \ - --hash=sha256:94be440c03fede26969a5ce75468e0e6a9927a1b46d9b679ee8adc1b057b0350 \ - --hash=sha256:9572c1369e9c9da2d64a7b7992c786d90ff295abc93964cfe3125e4290768070 \ - --hash=sha256:983a662de2571cb2502fc8ff47b6770b03d025d2eb314c92f77b3f07c74720ed \ - --hash=sha256:992c3514ec891fa7858099183e4d64e6bd5a5d4ff452fae29df22faa77a006bb \ - --hash=sha256:99ba9bc2b8062ea0c326a990f7f00e6530c23579de66dd246e72c4cafef950a5 \ - --hash=sha256:9d1bed94af6a63835461f3cf7502058eb166c58c4778e11d0f433cfb1bd69e19 \ - --hash=sha256:9e4472072de0137ee0d8e72d6620e85939c271d2f90f6bbb4b15c24638b79f92 \ - --hash=sha256:a27c09d86600f1bf2fe3f37e1ae697faf3143931c09322cd799da94deee923b5 \ - --hash=sha256:a29a061fec0b4e2d714f277e70a3a18125ecff803f2fea6eade2f2e53711d112 \ - --hash=sha256:a3cda0e538208e5d722bbf3698b24f19c0a7d05bc8d5f8a7f9b121ea7fa243d9 \ - --hash=sha256:a44101320cfe99432db74237545a63057dc7a88dfe792cbcad0647f2af56cb81 \ - --hash=sha256:a4aaaa791bdae1c972a7e81765f4f3571c926b8e0b9b6e47346499fb80079665 \ - --hash=sha256:a51eee75939cf811ac09b278745a6cee7dc873ccfbc8b9af3cc88fe4b7ce25b5 \ - --hash=sha256:a7c58106de36ac6a56314182958de20db8d3a29dfd5db527192cc754e4f8e7fb \ - --hash=sha256:aa8224f10880d9bf1b5993988ba153d42a8b4f3f4f511f93b1f09c93ff613c72 \ - --hash=sha256:acbf31f37c53a5ac0c34706c80b4f5107ba20b05fdd3816124bf236ef0c57dd2 \ - --hash=sha256:adc06d218a1cadfd2ec4628424d7d79ce4eba69c2965e7e7b55106f0da5208c8 \ - --hash=sha256:ae28183297fb0d2b8dc46a1f01d51f5e45825fc5afe76a835a6cb7fb34821295 \ - --hash=sha256:b0135bcdcaa0f23573f286e4cb5e0fd2962700964ed13df085b85f2b97aeab9e \ - --hash=sha256:b087b1c5be7ac687cf22eabfe4b6b608d40df23610651e93611e1f49118baf84 \ - --hash=sha256:b0d955195bbbe489ad343fcc956eacea9357b79cb22192c66cacdefcbc14b32f \ - --hash=sha256:b281a3b0f0822618fe5e3e0d8a2048b6356b14388505dc9374ccffeb69989713 \ - --hash=sha256:b6d0c2183008c188e19f4906d426b293bdc4f67ab17df8e180fe16cda208fa71 \ - --hash=sha256:bbce99252ba3167b2b6277f1829d5bf4b43b754524bddf7f944707c3db7d2253 \ - --hash=sha256:bc1f08f68dac9f9e83845a8039880aba2ab553eb9b2259c3243a313182c253fe \ - --hash=sha256:bc94f474417604bd383d2cd445d071b07dd55fedceed3ce33407bf1fcc107290 \ - --hash=sha256:bed5163e03b98bc1fa2eb05d74c63d9c5c95d8ed6254985481640fbf5e237dea \ - --hash=sha256:c17799a62c142d61b8a3c51752a7cbc87fe2ad4ccfab10e628a77b405075c662 \ - --hash=sha256:c18db21573bd2c6489f9a544b7499f0df2853958c568e5e783536ee1f690af41 \ - --hash=sha256:c3ede305158e75510be50869b319550ab072008c13d64d4ab1e094fb286b6f44 \ - --hash=sha256:c516cc5367ca3448995d42cb994bf3f4c9002d2a7c22eac9622551269ad1b807 \ - --hash=sha256:c531166c42ea7bdfecc8c50049581f05dd1993b09cc7c52bb36a14e96deaec7d \ - --hash=sha256:c73622ef819328873b53109ee4f77ceb598bffedd02daf916102be3228866b78 \ - --hash=sha256:c8dce1f1e0e5358b682a6ad3fa5e31b31d4560997b8e61417e9217c8d60f8a0c \ - --hash=sha256:cb57f304525a5e3c13333b772bf9a473f36326e9c821b2e8e1b2fd36f80ae2c3 \ - --hash=sha256:ce8c25ca38cc0e3d7753ba180808de2c0c8cb24eae0df64491e40921454e9831 \ - --hash=sha256:ceff0acc940be2715bd6ad17b24c0e5304abf44f6efd0f81ee8499e640f9dc86 \ - --hash=sha256:cf356f70551d40374eaffb1aa63f1eb6d2006681cbd7a9faea173ce0f4dd7cd2 \ - --hash=sha256:d2d82aa62521c55ddfb000ae70f88cdd8de974078f6024e821dfe5addd0c818f \ - --hash=sha256:d32f3acc081cc3923386d88f422cde8892335e95f034e0104bb4cf9310d9915f \ - --hash=sha256:d4193206b6587047437f6f11d7e776df23e1c1e23af2a54d9347275614791e10 \ - --hash=sha256:d5c0e32fdbce7f1e8ef4d11f655694bf5f4175c757a9f1dc2be09b8864e5bcf5 \ - --hash=sha256:d5e062c01286d861fd6a1c4ff6e063547b3e713067f2df033c0ff97ac2ca006b \ - --hash=sha256:d8f9e8a6c4ab04b78d61f78627370d834eb004b2869dcb28cfffa647b4ea1980 \ - --hash=sha256:d939de9c82e2126f7f48a7e658f8a85ed46d57662d53f44c49b8895fe94a3eb7 \ - --hash=sha256:de531ce1e2a3364e8767878b58f4ff728a434b4fde089781fe30b1e08e2396e0 \ - --hash=sha256:e50d7b94baac6c7d09927fa5ca5800a0c7ee5015c7fcff65beb3a1931b5a6e09 \ - --hash=sha256:e5faeaee74a57d32b3ab3aebad2e348f06d3ba946fc5d28c1728455f00a3d13a \ - --hash=sha256:e6f0cc1bb9900dc558960894adeb30b0c083366fc1d69b856209fb2ca5c36fe5 \ - --hash=sha256:e8e1d6ce820aa23317e8209a86bd65a540973c12dc7552b48a4f6c8e9926815e \ - --hash=sha256:ed1e728b39a383c81035b2459cfcb35d99dfb01f7d6ebe3a913bc1cc5b81e459 \ - --hash=sha256:ee6db2fbed51a7991302e8fac498cd67e336246026d0dfa84cf5166ce1412760 \ - --hash=sha256:efbecf43d321f7b9281441f1f213f7c21c66988b0e06c2730ba13ed47a46bb08 \ - --hash=sha256:f2c634642694e6a0dad2ab1d375589fa671fd442edd5caf7d9737b8f6ca22906 \ - --hash=sha256:f3377c8c2b3ce898423c5e5dd94c7982e30aa7717a7e6ab2470b9de364963709 \ - --hash=sha256:f5187624823423e1d1b82b1072ac41dc837389e18d3d0572cc19bbee46cd550a \ - --hash=sha256:f77ac30b19221cd9bd3fcfa3d4614eff93140d0572ab730cded17b64adca05f3 \ - --hash=sha256:fe90228920fd8ff2be62622b6bb8a2b11acd65046d50c6b130614b5879605a20 +pydantic-core==2.50.2 \ + --hash=sha256:01eb0282d0b3a065d55956d1be9ce398c46db3dd6d277a6cb2358c8aa8c53179 \ + --hash=sha256:04cf672960b8c4fc28bb159c8cdd101fbd311c520237a5c0360448b851adfb41 \ + --hash=sha256:0b94a515f44f1ebcb25395bcdba506c4cdaf6d3fcb0466b9658521b41e5ff057 \ + --hash=sha256:0d090ae76d5b09c003600c7779b3adcf3d1b49268f616c5c51d844e54c8e318d \ + --hash=sha256:126f158474346f457c0759c29c1a6334acd5f7f99dd272bb27d19314f3d81b4d \ + --hash=sha256:150f6e101f575c21a4ecfed1be1c05bb89a0ad49cb9c846ed377583bd71b32c2 \ + --hash=sha256:16ec8e07c37f02dcf68939cc7f202ba9a148cc155ee2acf22d337e10b24c2cb7 \ + --hash=sha256:18f008ef58cd30782f81b6e0119e20b7e884edcefbc2d370162ebe033f6e5c99 \ + --hash=sha256:196b87b48b2695426920e8470d20f9e99f019ebd86f15808aa018b68086840cd \ + --hash=sha256:1a5e859f5a8a6d834b0a13acaa0eaddf1a9b4b015fcda13bab7b471e18299d32 \ + --hash=sha256:1ac5b99bc7b0a68ec64f5f75a6828a7bab5535c8df0cdc07901deb50e0815910 \ + --hash=sha256:1b0e8dc3932cf4e9ed5727570325cacc716b9409ffc794a221c0ccc72e8eb775 \ + --hash=sha256:1b9f652e17a13bbc09b45f74c3c326cca72059ce4dcdef687fcb04389db413a9 \ + --hash=sha256:1db9cab6197e797824f0479dd2ae5fd2e272aa61c61b7faa5a6e450ca7a0d3b7 \ + --hash=sha256:1fdf5b52ed22f1404c2433d0b46df2d86d9befb742d0b9b4042a2325c43cdb97 \ + --hash=sha256:2115d7ca6d58554af8a14da3dd32d361a4fe999fae52f115a6155679b5b00e72 \ + --hash=sha256:21a902e546032c82b7275552345a2342d862e0541aea7226c91067c396e68b3f \ + --hash=sha256:21ea3b00f1e87c66f3f9e92f9f6fec20495ac0e60d7d2e68ba1fc49739d7cf40 \ + --hash=sha256:23385c4626f8b5a7792aa381ea573d9432cfbf4c21375a0a5eed2a4fb64e0e58 \ + --hash=sha256:240fec32a74af0979f7253532859b3fd209ad5eea822416bb010929dba445c3d \ + --hash=sha256:24120c644322a35b5eb765416ac8442214c45689e255f5fc6e761e7e7c743c8b \ + --hash=sha256:26b778d4ebb75f181d36220ff0f54a513fc46f053d9b09ce79993c2f00bc45cc \ + --hash=sha256:2d4a043c820363359e40ac96270e1915e5376e116be1f59cee0701775de3e376 \ + --hash=sha256:31a1fb3ffbd1195b24490f671d12623dd4c9bfa56d6af0a299fa43b516134f5b \ + --hash=sha256:33c6800ab9563adaa0d7893e9cd8f5b972ef1e6a23df70498e05c24f3fcec5e1 \ + --hash=sha256:3435c443da7510a6de1a30c68beae338852a11f0440cc8320ea756a162f38dbe \ + --hash=sha256:36917e29c0b815d6b940bc91560fcf453b5b85d55c579796373f0ea11d1c111a \ + --hash=sha256:36d8b9725684ca3bf658b90a101d335fc8cae405bb22196b7b5ef41344913756 \ + --hash=sha256:3939538e640c94ab779a05f2d45dafedd01d65d93e1290797f3ac5b59fcb6455 \ + --hash=sha256:3b1bb3950e0794a795c9344d818d5b476189b0c9c9755e8eb8fde87c3acee17a \ + --hash=sha256:3bfe12243b3ded517c6734d91f127489e2f51bf95cb533b53130fe360333db80 \ + --hash=sha256:3fbcd402e4b607d4b8de5d4591215f31f042af03f9d09e58f693a6221e32833f \ + --hash=sha256:41ed8abeffda6a880207fdc0cd1343a448cec3cc6b38b53f730c0745f745292d \ + --hash=sha256:43a8fd56f3c681f3cda2d5c842ad28e88241df742dfcbe8fd1dd437a3056e1f6 \ + --hash=sha256:48e944091b7bff233920bed8a60b0ed6c96c73d3c5836bf51c4b9a4bfb58e4ca \ + --hash=sha256:4ac996e42a93d80122a23edce44e02c6b9c4913f5c6198a077da31475341a39a \ + --hash=sha256:4e9392ddf67fe97ab9889f796d468c9908a8754b8b6c7520eb03a65012b9a1e7 \ + --hash=sha256:4ea01f30e119f1ec7a2034e85c40f491d469896aef02cd6436fd9292fe1be3fb \ + --hash=sha256:4eb54c25e3463d73e1e4ed439ffc45d1d7e590df40bc7bb06f11a906afd996c3 \ + --hash=sha256:4ee64baeab403b9de9c92846c5c1abec28ffb046cbc023bfae059897841f88af \ + --hash=sha256:523fe6574aea50f456cbc301c1cdddcbf237939f7657f9af99b4f0f71942b59c \ + --hash=sha256:5299296ff5cafcb69851fd652de8f9268a0bb036bcbedbfcaa94cf743baf3986 \ + --hash=sha256:53bd25e8ab54c02bb4c55d4848c1d36f75f3b9a7356e83179fe27794d9e9e6c4 \ + --hash=sha256:57bccd3e1128736c792094da1feac6da46ec3feb1957ef649a9b3e4f8de6fa04 \ + --hash=sha256:596921489390a002d6a5e303384a0f1f98384f8bb550fbf38a1aafa6093da0f9 \ + --hash=sha256:5ab5738c39b25b5dc5a721270e2269a2f5e86e614375e6c07ee6caab8cbc0f2f \ + --hash=sha256:5c12a5a080570a91bbcbf6d10db008b80e3f5dea015ef2f0137bbec70f24f3f3 \ + --hash=sha256:5d3cd8be71c4da3c9b42cc92f17e5e3036bdca98bf18555a0e072c3fe87abbb4 \ + --hash=sha256:61e13100b30d8d25fc0d1f0297014b32941bf0975a3584bd08f3adebb68d1ead \ + --hash=sha256:6258754c473b3045a7aa30b833a8a331a1ab36c6147cb230789861d80bb26f83 \ + --hash=sha256:6a438cb8a3f87f2c49e82e43fb19511d4d5043f53e0787dc4df937ccb904373a \ + --hash=sha256:6c25b8826be85714b27dafe908d3461e1b09e3e31c0279a7029040c832c781ef \ + --hash=sha256:6d47b2d0e00e030e0ebdb08115a37da7294e7a6d196d134a77ed7ee6c142284b \ + --hash=sha256:6e731bf5a5e8e25398544d7709e39068ec4230b0177e4db7fa955692fd415ba3 \ + --hash=sha256:6eb6ff07964f39e33c6f57fefe939514e76b8c0d4a694e5663e2e04e80cc1798 \ + --hash=sha256:6f5a432c6c1ef18a1a02a04b3616b8d3f2f88b13faa46caae1ac6559ab202234 \ + --hash=sha256:6ffb4dce3ecb2cd563aa30f9eb5088a1cf1f4f688b90cbb374fa96e49fc50b02 \ + --hash=sha256:701bc55d08c22f981973ca4f6fdf4ea102ee1ee3a21b818b66fdb161ea711a4a \ + --hash=sha256:713d7c33c0e231e3786eb0ec95885814dd65d5f3a09754399b6acbf5e9365f11 \ + --hash=sha256:71615edeaf1ae70f5f3f558426072eede0168b7e5190fd9633878e29a0b85732 \ + --hash=sha256:783abdfac21247d33bec40aa683d26bba30d61655c40de38b2decc4797aabf92 \ + --hash=sha256:7c539d452cba22f250103c7d1c00cbe9038eb705e657476e701c43374efd9f2e \ + --hash=sha256:7c6e1bd6cadae13958e52897ef020e85c9916a95917149fca463bb0c0421a7c6 \ + --hash=sha256:7f9821dfc0c473c96ad55ce4189c7cb4f9c94e7d4f563afb874173243dec1162 \ + --hash=sha256:823b222c4fc861a82c650998a6117ecbd150cc71bf8b78f9a31bf3f48c456108 \ + --hash=sha256:83d6f42ae6eba44548a1a3d4f900a5b1500fdd14f09f04754a8800ab0d544432 \ + --hash=sha256:8431ecb131489cbdaf6f51f7ade47475f73aff4f1912552f75ba19a6cfe233a8 \ + --hash=sha256:848984e231190ae49bc8b5229e64d2a6fba4231dc6046e1b6b28ddc060bc7601 \ + --hash=sha256:86e7abd24a67f61a4bfe8df059b67a2bdd8bf33705178e98463195d140bd3780 \ + --hash=sha256:8772c2c0091e6fc2d0499151793d31bf811826986b0727052935a95104845544 \ + --hash=sha256:8835566b3e94026da3ce2249f9ee35c9d2ebe4cd131f6a39bfc82708d3010a46 \ + --hash=sha256:89213368eef163350af37127939ace099010e8102757905c3e329f1175950ed8 \ + --hash=sha256:91238117cd296352f54f24117ff5a227bdea428865dba1244aa0cdd1424c0bc2 \ + --hash=sha256:91315aea7adb51821d4a3378ede953bfdd76e3bf71551de4a82d7638cef7dc02 \ + --hash=sha256:91c92856b846aa690ea6239ffd6c5003c5e3e296a69095570bf4efee5bfa0bdc \ + --hash=sha256:928a44ad1e21a7e923eb924d601605f0694cd187d2ce5d40bf7ad76650f4627d \ + --hash=sha256:92acf02c770b9d5c4e9a3e8d64e9d95ae26456a86ebb7b7cf64a97f899e5871b \ + --hash=sha256:92ada606285dab054e40848a951f583c537cde1e54701ea1305ec0096877012d \ + --hash=sha256:93bedc68c4138088ea3c042189c01d2945f530f0dcc36e1ea02df8418bd4f1cd \ + --hash=sha256:93eb41d626d9a2749a678823c70bba8c25033cef474ae8001fd7621adbbd8437 \ + --hash=sha256:95838be125133fa314753f8cc5168f7c41e3a9dd5eef5383dbbbeed511bd4853 \ + --hash=sha256:9667637665acba166a7dbbc786f14c50512d3a01f7199e0e1a13b929e92d7c7a \ + --hash=sha256:98722314e12a8e4fa346e0afd3fff2098b5732875ff45592b450a7b589b43c4f \ + --hash=sha256:99e1d482138a71902415ba375bb4627679a5fefd3bd07803c3a8f37632f4981d \ + --hash=sha256:9a87744e683d9a13f60b7fa5242c17f8d283ffa94ad727cb19063b48c3d34960 \ + --hash=sha256:9a8a62281fd9c6a373b3fd3d473685e6838d43341eecde5f47729748e2c11ee3 \ + --hash=sha256:9b01a092955c1c7530196ad526755cff4f2d11f37ca9a6fc9ebe37ca49a48977 \ + --hash=sha256:9b2383933b1dd7a8c93e02f99ebfd305f1286057172483b8e6898bed3fef69d3 \ + --hash=sha256:9e7460abce637f90e37f8c443559fbd687c38884d05cb69ef265e634945f4324 \ + --hash=sha256:9eff570402175c7d5fff8497d5a9c05580664a79bb66c07b8f205d17f69e580b \ + --hash=sha256:9fcde11462e0faab4c2a2ae8cd4516f7dd8239eec583bd88c5524d8064ad2d0c \ + --hash=sha256:a1b4d60bad57d6e0e7cd30cc4d1dc646d258dd6dfb7935ce76c7dd72dd1e98b0 \ + --hash=sha256:a3317630e2791f56071b82f89bb71624146125e4311e6643f708c6ba744b5ac5 \ + --hash=sha256:a53d5f11cd6d15569ba3e53f3498077f0d5e633f6ec604d2ea613e895017ebee \ + --hash=sha256:a55c1163e829a08707b4a92c8ee02a5a3a3847ced74aa4da94e3e975ffe1ec63 \ + --hash=sha256:a59db03b80a156a6fbeae41096866d1cb89cdf31031ba44c768a804538880636 \ + --hash=sha256:a786cf782ebb2e45a43c7118d75d452aac982b79577dfe804d8dbf2878f0b589 \ + --hash=sha256:a88851e8e332588cb8cced8b5318d1163c7d0bb87830aa8dcafbc4ae9602b73c \ + --hash=sha256:adec010c824290d4ca87047e8d8bfa4f85ac29f545f326cd5321696805d311ed \ + --hash=sha256:aff31602e8518594e6f6bc670cdb27e37cb7dbeb2d5d0b76a7707b70fe951ddf \ + --hash=sha256:b0b25d647036972c2951059046cb498f72b7af5a2def27bf8e5f44c3067ab7dd \ + --hash=sha256:b0e6a2152706d84f7da8b9d34dcecc7e9bde678af32c40bd5b405f427ae0fbdb \ + --hash=sha256:b1adf2f37521b578c438f6ec61d1c1d0c20b076863ddd5d8b6f3ab658857db3f \ + --hash=sha256:b29b7c5adf4b2be7c941f7f76432aa3269cd5052ff8610376b8ece24d7b02c51 \ + --hash=sha256:b2dd65864aad38dbcc57c3d0e2c978d4286e063378137d8fc0c9fcd558943285 \ + --hash=sha256:b381574614245f18fdb2d6d1aa8f149e225869c47b5ee7f81812de8977817314 \ + --hash=sha256:b3e681208e2d39fde13fbea449ef2736a85ec1ea2afa41e9193a8c536e71dafe \ + --hash=sha256:b5675a12a7b67063fe3ca187e735b0463cf13fabc10166ae4d4b2f775fd0632d \ + --hash=sha256:b583ddeee6a6bb5bbc04462167c68034913221139d52d9f155ca6d1637a2fdcf \ + --hash=sha256:b62c89545d21d930c333208fd548e7b4f262228ba3a322fd75c987a75ab11f31 \ + --hash=sha256:b7abcfaf66c1dc14c5ed928c349614f223c972a8b021b4be9548dbea6c817371 \ + --hash=sha256:b958c217c6e56bef261f516e4caf5d490b079ad6a428752afada5a764cca6c71 \ + --hash=sha256:bb6d49f00e2babd40af4889d6b9f4bf50879f522dd4e2e22cb705cac5e0c5e8e \ + --hash=sha256:bd1fe69056a19c377a6c6de2c1240fb8676ba0512a969965efb486510c5e0f1f \ + --hash=sha256:bd6ecae08c0d871cf93b6f0d4ee76b06c65be44ea22311981a022b2f1948cf9d \ + --hash=sha256:bf7f99a0e6616cea754e2334d129091f3260d011369d9b8c658522bfb2427619 \ + --hash=sha256:c1b65a42afc73b39e6ae908ec7b9c3b9898676a7b15ce4c66d4965f7071007c8 \ + --hash=sha256:c324a7e0c0012e3974053f81a270d8f1aa23227571f4ddd3254c911bb356459b \ + --hash=sha256:cb8d2d7b07cbe2d7488de7e53671f836c4f4a8032dcda079df33dc1b7e2397c0 \ + --hash=sha256:cc488daac19e6f5292180fac5e0ef7efffabeef4e096af9a17a75a7d3d6781f6 \ + --hash=sha256:d18d56c1ec63b304f4bb3c7db7e0d311e61cca1b1c944590a4b13ac39b2ca92b \ + --hash=sha256:d3d049b557875f9d15230eec8a731749a7a873d8ed6a051903b05bdf46e0e65f \ + --hash=sha256:d684ff87d00de2f6abcbbfa9b0fdce8b513dd799a900809dcfa7eb62ad69832a \ + --hash=sha256:d71a9a9352acbcb5c73d39dd003383599428067b6eda5cf7c5d335a7a0da37e1 \ + --hash=sha256:d79aaa468193c1bed6b0560d3c5c9f9e369e3efb0b128b38a2537e2d51d42773 \ + --hash=sha256:d93c066a38f83376b5bc020e563666784d1b680a70d6ceb625fdb541af7a312e \ + --hash=sha256:e2e6240a0a5ad847017b3d65187877fdb7a11d5a58d43c30736f9f523478913b \ + --hash=sha256:e3d824b6152e1c0fd58dc6266f2de470eb7b2286135294f5f7cd26a0f0693738 \ + --hash=sha256:e5c801bdf3cc92274b8fc2c7023c8751e5ae81260c009ac558ff55ce712a2d45 \ + --hash=sha256:e80c47314c7233e7105a7efcfffd89d56e479e558189d1959eeeaf2639a27680 \ + --hash=sha256:ea71b94a2eb5d2fc102926321f56d2ee8eddfad2673eadb1d23e9f287a492715 \ + --hash=sha256:eec187d2876abb6a74203efbe2aa796455c1809334acf4e739dde35da8cfb69f \ + --hash=sha256:efad0bdc67d3b119c3bab6b94d341a27333c2bb89ed75dfa182a4a06cd0ea329 \ + --hash=sha256:f078e276848bc23d1f92f46fc88d5b9cbb824b9acc60d9e07116620e502e16a9 \ + --hash=sha256:f07c965a8aa6d62145f122641f292129984f39a3e3697585b19298a1488287db \ + --hash=sha256:f142cd663d1148ed32993ed9a58e5c67ea312f446a01a6c13a956945aafc9f23 \ + --hash=sha256:f47c1349c3b4274aeb18459d54244881a5c0dee0fdccedffca45704ba5c9e0a0 \ + --hash=sha256:f5ba7ef10ce82737e173fa95cafa1c8210918f7e653715b2c7a7a31877a74848 \ + --hash=sha256:f73930dacad5e611fe019e18df6f53c30629b1843d038d303b8dec86a32493bd \ + --hash=sha256:f81df206b937d0dca5efb38c25fc99037d82dbcd3740e34c3f81997d2322ac1c \ + --hash=sha256:fd9aa71a800c87c0f7ab53fa38009169a23a82c685989626ca74a4bd483e1b67 # via pydantic pygments==2.21.0 \ --hash=sha256:2363c69b61c4a97c838da3b130dcd6468f4848992b21a82f2a63ec34377137d9 \ @@ -556,9 +564,9 @@ pyjwt==2.15.1 \ --hash=sha256:42d59d631f7768a1028a64c7ff581a9bf7519804daf91fc5b6c56e30eec5e193 \ --hash=sha256:4f259e80cdfb6b3fc18a7de51fd1ef9ec79652f25019bae68975ca2468a34df8 # via mcp -pytest==9.1.1 \ - --hash=sha256:1088fbde8f2b49d95a549a195707afa7a76a3ce9bcadc26b6d71f0ffda5fe313 \ - --hash=sha256:37a86b45efb9a47a61a36449063e8e18d0cab3161329fc099eb21783169c4f0c +pytest==9.2.0 \ + --hash=sha256:07acdcf88ec174f907dc01af70bf52d49b014bbab03faeeade31c96b18c2e588 \ + --hash=sha256:97c4f3b1f3c7414aa36465e09b8220f1d03627993bd51b1282e193753c80557b # via # specify-cli (pyproject.toml) # pytest-cov @@ -825,13 +833,13 @@ shellingham==1.5.4 \ --hash=sha256:7ecfff8f2fd72616f7481040475a65b2bf8af90a56c89140852d1120324e8686 \ --hash=sha256:8dbca0739d487e5bd35ab3ca4b36e11c4078f3a234bfce294b0a0291363404de # via typer -sse-starlette==3.5.0 \ - --hash=sha256:3e6e1070df3f0f5d9cea81496de92dbb72f6721871d99748ece67441dd8b7997 \ - --hash=sha256:75de713aa8a9441513cc283220826da079d982770965b951e9437720e8bafdb2 +sse-starlette==3.6.0 \ + --hash=sha256:623151d6c20dc0eebf1b3379c27faae91fff883bac7ab6665f16f46440a88b3a \ + --hash=sha256:e4dfa4d237d2712cb2b4d4918eda9b457cd6b44bbecdc838af75de836565bafd # via mcp -starlette==1.9.0 \ - --hash=sha256:3d931de246ed870e35ce605974440816ef895e592f255d1396cfdf191d4c4318 \ - --hash=sha256:fd3f159936e882d06c1108f84688f7cda2cf85fc6a592a50edbd175e882c2ddc +starlette==1.10.0 \ + --hash=sha256:478c0ab3b4ced6e82719849c118fa406f0278a3499c1a898cd15cd31bdc38ce4 \ + --hash=sha256:b8399098d4c1e3522734f7235dc79b1fc967f96cf139c81e0fa30479e0685f9b # via # mcp # sse-starlette From 39992610c87245a7bb3b676a4c09b333584066c3 Mon Sep 17 00:00:00 2001 From: Markus Date: Fri, 30 Oct 2026 01:43:53 +0100 Subject: [PATCH 07/12] fix(workflows): name misplaced filter chains from the parse tree `inputs.mode not 'done'` was reported as an ambiguous filter precedence mistake although it has no filter. The message was inferred from the terminals the parser expected at the failure, and `|` is expected after any plain operand, not only after a filter chain. The grammar now parses `|` as a postfix operator that binds tightest, so a filter chain is a node wherever it appears. `_parse` refuses a tree in which an operator has such a node as a direct operand and names the leftmost one. Parentheses keep their group node, so `(items | length) > 0` stays valid. Accepted inputs and their ASTs are unchanged; malformed text without a misplaced chain now gets the plain positional syntax error. Assisted-by: GitHub Copilot (model: Claude Opus 5.5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../workflows/expressions/engine_lark.py | 147 ++++++++++++------ tests/unit/test_condition_expression_block.py | 73 +++++++++ 2 files changed, 171 insertions(+), 49 deletions(-) diff --git a/src/specify_cli/workflows/expressions/engine_lark.py b/src/specify_cli/workflows/expressions/engine_lark.py index ebbbbc9da5..621f57cfea 100644 --- a/src/specify_cli/workflows/expressions/engine_lark.py +++ b/src/specify_cli/workflows/expressions/engine_lark.py @@ -12,13 +12,15 @@ The rest of this docstring describes how the engine is built: -* **The precedence guards are in the grammar, not in a post-check.** A filter - chain is only valid where the language allows one: the head is a plain - operand, and the chain is a whole expression or sits in parentheses, a list - element or a filter argument. ``count > limit | default(5)``, ``not x | - default(1)`` and ``x | default(0) > 5`` do not parse at all; the first two - keep the "ambiguous filter precedence" message through the offending ``|`` - token. +* **The grammar models filter chains everywhere; ``_parse`` rejects the + misplaced ones.** ``|`` is a postfix operator that binds tighter than any + other, so ``count > limit | default(5)``, ``not x | default(1)`` and + ``x | default(0) > 5`` all parse, and the tree shows exactly which operator + has a filter chain as its operand. ``_parse`` refuses such a tree before + anyone sees it, naming the operator, so the message comes from the tree's + structure and not from guessing at the parser's failure state. A chain is + valid as a whole expression, in parentheses, as a list element or as a + filter argument. * **The path terminal is derived from ``namespace``.** ``_PLAIN_SEGMENT`` and ``_INDEXED_SEGMENT`` are the one definition of a path segment; widening either rebuilds the parser (the cache is keyed on the patterns) and reaches @@ -42,7 +44,7 @@ from functools import lru_cache from typing import Any -from lark import Lark, Token, Transformer, v_args +from lark import Lark, Token, Transformer, Tree, v_args from lark.exceptions import UnexpectedCharacters, UnexpectedInput, UnexpectedToken from . import namespace @@ -103,10 +105,7 @@ def _path_terminal_pattern(plain_pattern: str, indexed_pattern: str) -> str: def _grammar(path_pattern: str) -> str: return r""" -?start: filtered - | expr - -filtered: operand ("|" filter)+ +?start: expr ?expr: or_expr @@ -119,12 +118,15 @@ def _grammar(path_pattern: str) -> str: ?not_expr: NOT not_expr -> not_op | comparison -?comparison: operand +?comparison: postfix | comparison CMP_OP cmp_rhs -> cmp -?cmp_rhs: operand +?cmp_rhs: postfix | NOT cmp_rhs -> not_op +?postfix: operand + | postfix "|" filter -> filtered + ?operand: string | number | list_lit @@ -214,8 +216,11 @@ def filter_bare(self, name: Token) -> tuple[str, str, tuple[Any, ...], bool]: def filter_call(self, name: Token, *args: Any) -> tuple[str, str, tuple[Any, ...], bool]: return ("filter", str(name), args, True) - def filtered(self, head: Any, *filters: Any) -> tuple[str, Any, tuple[Any, ...]]: - return ("pipe", head, filters) + def filtered(self, head: Any, filt: Any) -> tuple[str, Any, tuple[Any, ...]]: + # The grammar nests a chain to the left; flatten it into one pipe. + if head[0] == "pipe": + return ("pipe", head[1], head[2] + (filt,)) + return ("pipe", head, (filt,)) def cmp(self, left: Any, op: Token, right: Any) -> tuple[str, ...]: return ("cmp", " ".join(str(op).split()), left, right) @@ -230,42 +235,86 @@ def not_op(self, _op: Token, inner: Any) -> tuple[str, Any]: return ("not", inner) +class _FilterPrecedenceError(Exception): + """A filter chain is the operand of an operator. + + ``operator`` is the operator token. ``chain_first`` is true when the chain + comes before the operator (``items | length > 0``) and false when the + operator comes first (``count > limit | default(5)``, ``not x | f``). + """ + + def __init__(self, operator: Token, chain_first: bool) -> None: + super().__init__(str(operator)) + self.operator = operator + self.chain_first = chain_first + + +_OPERATOR_NODES = frozenset({"cmp", "and_op", "or_op", "not_op"}) + + +def _misplaced_filter_chains(tree: Tree) -> list[_FilterPrecedenceError]: + """Every operator in *tree* that has a filter chain as a direct operand. + + Parentheses keep their ``group`` node, so ``(items | length) > 0`` has a + group operand, not a chain. + """ + found: list[_FilterPrecedenceError] = [] + for node in tree.iter_subtrees(): + if node.data not in _OPERATOR_NODES: + continue + operator = node.children[0] if node.data == "not_op" else node.children[1] + for index, child in enumerate(node.children): + if isinstance(child, Tree) and child.data == "filtered": + chain_first = node.data != "not_op" and index == 0 + found.append(_FilterPrecedenceError(operator, chain_first)) + return found + + def _parse(text: str) -> Any: - """Parse one block body into the tuple AST. Raises ``lark`` errors.""" - return _BuildAst().transform(_parser().parse(text)) + """Parse one block body into the tuple AST. + + Raises ``lark`` errors for text the grammar does not accept, and + ``_FilterPrecedenceError`` for the leftmost operator that has a filter + chain as an operand. Every caller parses through here, so no caller sees + such a tree. + """ + tree = _parser().parse(text) + misplaced = _misplaced_filter_chains(tree) + if misplaced: + raise min(misplaced, key=lambda err: err.operator.start_pos) + return _BuildAst().transform(tree) + + +# What a failed ``_parse`` raises. +_PARSE_ERRORS = (UnexpectedInput, _FilterPrecedenceError) -def _parse_error_message(text: str, exc: UnexpectedInput) -> str: +def _parse_error_message( + text: str, exc: UnexpectedInput | _FilterPrecedenceError +) -> str: """The author-facing message for a parse failure of *text*. Filter precedence is the one carried-over semantic. A filter chain cannot - be an operand of an operator, in either direction: a ``|`` after an + be an operand of an operator, in either direction: a ``|`` on an operator's operand (``count > limit | default(5)``), or an operator after a - filter chain (``items | length > 0``). Both get the same wording. Everything - else names the offending character and column. + filter chain (``items | length > 0``). Everything else names the offending + character and column. """ - token = getattr(exc, "token", None) - expected = set(getattr(exc, "expected", ()) or ()) - expects_operand = bool(expected & _OPERAND_EXPECTED) - if token is not None and token.type == "VBAR" and not expects_operand: - return ( - f"ambiguous filter precedence in '{text}': a filter cannot be an " - f"operand of an operator. Filter the operand in its own expression " - f"instead." - ) - if ( - token is not None - and str(token.type) in _OPERATOR_TOKENS - and "VBAR" in expected - and not expects_operand - ): - operator = " ".join(str(token.value).split()) + if isinstance(exc, _FilterPrecedenceError): + if not exc.chain_first: + return ( + f"ambiguous filter precedence in '{text}': a filter cannot be an " + f"operand of an operator. Filter the operand in its own expression " + f"instead." + ) + operator = " ".join(str(exc.operator).split()) return ( f"ambiguous filter precedence in '{text}': '{operator}' follows a " f"filter chain, and a filter chain cannot be an operand of an " f"operator; parenthesize the filtered operand, as in " f"'(inputs.items | length) > 0'" ) + token = getattr(exc, "token", None) if token is not None and token.type == "$END": return f"invalid expression '{text}': unexpected end of text" if isinstance(exc, UnexpectedCharacters): @@ -290,7 +339,7 @@ def _syntax_error(text: str) -> str | None: return None try: _parse(collapsed) - except UnexpectedInput as exc: + except _PARSE_ERRORS as exc: return _parse_error_message(collapsed, exc) return None @@ -405,7 +454,7 @@ def _evaluate_simple_expression(expr: str, namespace_: dict[str, Any]) -> Any: return None try: ast = _parse(collapsed) - except UnexpectedInput as exc: + except _PARSE_ERRORS as exc: raise ValueError(_parse_error_message(collapsed, exc)) from exc return _eval(ast, namespace_) @@ -507,7 +556,7 @@ def _collect_leaves(text: str) -> list[str]: return [] try: ast = _parse(collapsed) - except UnexpectedInput: + except _PARSE_ERRORS: return [] leaves: list[str] = [] _visit(ast, paths=leaves) @@ -545,7 +594,7 @@ def _evaluator_rejects(text: str) -> str | None: return None try: ast = _parse(collapsed) - except UnexpectedInput: + except _PARSE_ERRORS: return None filters: list[Any] = [] _visit(ast, filters=filters) @@ -560,9 +609,8 @@ def _evaluator_rejects(text: str) -> str | None: # Terminals the parser expects exactly where an operand (``PATH``) or a filter # name (``FILTER_NAME``) is missing. An operator token that fails while one of -# these is expected lacks an operand; one that fails after a filter chain -# (where only ``|``, ``(``, ``)`` and the like are expected) is a filter -# precedence mistake instead. +# these is expected lacks an operand; one that fails elsewhere (``not`` right +# after an operand, as in ``inputs.mode not 'done'``) is a plain syntax error. _OPERAND_EXPECTED = frozenset({"PATH", "FILTER_NAME"}) # A word operator at the very start of the text. The LALR lexer only offers @@ -582,9 +630,10 @@ def _has_incomplete_operand(text: str) -> bool: operand or a filter name was expected (``inputs.a == == inputs.b``, ``inputs.a || inputs.b``) fails on that token; a leading word operator (``in inputs.a``, ``and inputs.x``) fails right behind the word. An operator - after a filter chain (``inputs.items | length > 0``) has both operands and - is a filter precedence mistake, reported by ``_syntax_error``, as is a - failure anywhere else (arithmetic, a stray character, a second literal). + with a filter chain as an operand (``inputs.items | length > 0``) has both + operands and is a filter precedence mistake, and any other failure + (arithmetic, a stray character, a second literal) is a syntax error; both + are reported by ``_syntax_error``. """ stripped = text.strip() if not stripped: @@ -598,6 +647,6 @@ def _has_incomplete_operand(text: str) -> bool: if token_type in _OPERATOR_TOKENS or token_type == "VBAR": return bool(set(exc.expected) & _OPERAND_EXPECTED) return bool(_LEADING_WORD_OPERATOR.match(stripped)) - except UnexpectedInput: + except _PARSE_ERRORS: return False return False diff --git a/tests/unit/test_condition_expression_block.py b/tests/unit/test_condition_expression_block.py index 6c4d20375a..ca4c420ceb 100644 --- a/tests/unit/test_condition_expression_block.py +++ b/tests/unit/test_condition_expression_block.py @@ -556,6 +556,8 @@ def test_incomplete_operand_covers_every_operator_the_evaluator_splits_on(): ("inputs.a || inputs.b", True), ("inputs.a == == inputs.b", True), ("| upper", True), + ("inputs.items | length >", True), + ("inputs.mode not 'done'", False), ], ) def test_incomplete_operand_tells_a_filter_chain_from_a_missing_operand( @@ -566,6 +568,77 @@ def test_incomplete_operand_tells_a_filter_chain_from_a_missing_operand( assert _has_incomplete_operand(text) is incomplete +# Review of #5171: `not` right after an operand was reported as a filter +# precedence mistake, because the message was inferred from the terminals the +# parser expected and `|` is expected there too. The grammar now parses filter +# chains everywhere and the tree names the operator that has one as operand. +@pytest.mark.parametrize( + "expr,near", + [ + ("inputs.mode not 'done'", "unexpected 'not' at column 13"), + ("(inputs.a not inputs.b)", "unexpected 'not' at column 11"), + ("[1, inputs.a not 2]", "unexpected 'not' at column 14"), + ("inputs.a == inputs.b not 'x'", "unexpected 'not' at column 22"), + ], +) +def test_a_malformed_expression_without_a_filter_gets_no_filter_advice(expr, near): + with pytest.raises(ValueError) as info: + evaluate_expression("{{ " + expr + " }}", StepContext()) + + assert "filter precedence" not in str(info.value) + assert near in str(info.value) + + +@pytest.mark.parametrize("step_cls", STEP_CLASSES) +def test_a_condition_without_a_filter_is_not_told_to_parenthesize_one(step_cls): + config = {"id": "s1", "condition": "inputs.mode not 'done'", "then": [], "steps": []} + errors = [e for e in step_cls().validate(config) if "'condition'" in e] + + assert len(errors) == 1 + assert "filter" not in errors[0] + assert "unexpected 'not'" in errors[0] + + +@pytest.mark.parametrize( + "expr,message", + [ + ("(inputs.items | length > 0)", "'>' follows a filter chain"), + ("[inputs.items | length and inputs.ok]", "'and' follows a filter chain"), + ("inputs.x | default(inputs.items | length > 0)", "'>' follows a filter chain"), + ("(inputs.n > inputs.limit | default(5))", "a filter cannot be an operand"), + ("[1, not inputs.x | default(1)]", "a filter cannot be an operand"), + ("inputs.a == not inputs.b | default(1)", "a filter cannot be an operand"), + # Two misplaced chains: the leftmost operator is named. + ( + "inputs.items | length == inputs.a and inputs.b > inputs.c | length", + "'==' follows a filter chain", + ), + ], +) +def test_filter_precedence_is_refused_at_any_nesting_depth(expr, message): + with pytest.raises(ValueError, match="ambiguous filter precedence") as info: + evaluate_expression("{{ " + expr + " }}", StepContext()) + + assert message in str(info.value) + + +@pytest.mark.parametrize( + "expr,value", + [ + ("(inputs.items | length) > 1", True), + ("1 < (inputs.items | length)", True), + ("[inputs.items | length, 1]", [2, 1]), + ("inputs.items | join(',') | upper", "A,B"), + ("(inputs.items | join(',')) | upper", "A,B"), + ("not (inputs.items | length)", False), + ], +) +def test_a_parenthesized_or_whole_filter_chain_still_evaluates(expr, value): + ctx = StepContext(inputs={"items": ["a", "b"]}) + + assert evaluate_expression("{{ " + expr + " }}", ctx) == value + + # Copilot round 3: the first two gates each inspected only one position. These pin # every-position scanning, both ends, and bracket-type matching. MULTI_POSITION_UNFIXABLE = [ From 8c6ed22e2879608f5b75e8f9dac6cd88c837ee11 Mon Sep 17 00:00:00 2001 From: Markus Date: Sat, 31 Oct 2026 13:21:55 +0100 Subject: [PATCH 08/12] fix(workflows): name the operator in filter-precedence errors and correct validation docs The operator-first filter-precedence error now names the offending operator and suggests parenthesizing the filtered operand, matching the chain-first message. ARCHITECTURE.md no longer claims condition expressions are syntax-checked at validation time; validators check only delimiters and layout. Assisted-by: GitHub Copilot (model: Claude Sonnet 5.5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/workflows/expressions/engine_lark.py | 6 +++--- tests/unit/test_condition_expression_block.py | 7 ++++--- workflows/ARCHITECTURE.md | 4 ++-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/specify_cli/workflows/expressions/engine_lark.py b/src/specify_cli/workflows/expressions/engine_lark.py index 621f57cfea..6de86af6c4 100644 --- a/src/specify_cli/workflows/expressions/engine_lark.py +++ b/src/specify_cli/workflows/expressions/engine_lark.py @@ -301,13 +301,13 @@ def _parse_error_message( character and column. """ if isinstance(exc, _FilterPrecedenceError): + operator = " ".join(str(exc.operator).split()) if not exc.chain_first: return ( f"ambiguous filter precedence in '{text}': a filter cannot be an " - f"operand of an operator. Filter the operand in its own expression " - f"instead." + f"operand of '{operator}'; parenthesize the filtered operand, " + f"as in '(inputs.limit | default(5))'" ) - operator = " ".join(str(exc.operator).split()) return ( f"ambiguous filter precedence in '{text}': '{operator}' follows a " f"filter chain, and a filter chain cannot be an operand of an " diff --git a/tests/unit/test_condition_expression_block.py b/tests/unit/test_condition_expression_block.py index ca4c420ceb..6be3819a15 100644 --- a/tests/unit/test_condition_expression_block.py +++ b/tests/unit/test_condition_expression_block.py @@ -605,9 +605,10 @@ def test_a_condition_without_a_filter_is_not_told_to_parenthesize_one(step_cls): ("(inputs.items | length > 0)", "'>' follows a filter chain"), ("[inputs.items | length and inputs.ok]", "'and' follows a filter chain"), ("inputs.x | default(inputs.items | length > 0)", "'>' follows a filter chain"), - ("(inputs.n > inputs.limit | default(5))", "a filter cannot be an operand"), - ("[1, not inputs.x | default(1)]", "a filter cannot be an operand"), - ("inputs.a == not inputs.b | default(1)", "a filter cannot be an operand"), + ("(inputs.n > inputs.limit | default(5))", "an operand of '>'"), + ("[1, not inputs.x | default(1)]", "an operand of 'not'"), + ("inputs.a == not inputs.b | default(1)", "an operand of 'not'"), + ("inputs.count > inputs.limit | default(5)", "an operand of '>'; parenthesize"), # Two misplaced chains: the leftmost operator is named. ( "inputs.items | length == inputs.a and inputs.b > inputs.c | length", diff --git a/workflows/ARCHITECTURE.md b/workflows/ARCHITECTURE.md index 5664d323cf..0a160567f6 100644 --- a/workflows/ARCHITECTURE.md +++ b/workflows/ARCHITECTURE.md @@ -208,7 +208,7 @@ Workflow definitions use Jinja2-like `{{ expression }}` syntax for dynamic value **Operator precedence.** From loosest to tightest: `or`, `and`, `not`, then the comparisons (`==`, `!=`, `>`, `<`, `>=`, `<=`, `in`, `not in`). All comparisons share **one** level and group left to right, so no comparison has priority over another: `{{ a == b > 0 }}` is `{{ (a == b) > 0 }}`, and `{{ 1 < 2 < 3 }}` is `{{ (1 < 2) < 3 }}`. The former hand-written engine gave comparisons accidental priorities through its operator scan order; parenthesize, or join comparisons with `and`/`or`, instead of relying on either reading. A `not` on the right of a comparison negates only its operand: `{{ a == not b }}`. -**Only expressions parse.** The body of every `{{ }}` block is parsed by a grammar, so text that is not an expression is a `ValueError` naming the offending token and column: prose such as `{{ some text }}`, path keys with characters outside letters, digits, `_` and `-` (`{{ inputs.weird:key }}`), arithmetic, or two adjacent operands. The former engine looked such text up as a path and rendered the miss as an empty string or `None`. Conditions are checked at validation time; other fields fail when the step runs. +**Only expressions parse.** The body of every `{{ }}` block is parsed by a grammar, so text that is not an expression is a `ValueError` naming the offending token and column: prose such as `{{ some text }}`, path keys with characters outside letters, digits, `_` and `-` (`{{ inputs.weird:key }}`), arithmetic, or two adjacent operands. The former engine looked such text up as a path and rendered the miss as an empty string or `None`. Validation of `if`/`while`/`do-while` conditions checks only the `{{ }}` delimiters and layout, not the expression inside: `{{ inputs.count + 1 }}` passes validation and fails with the `ValueError` when the step runs, as do all other fields. **Filter argument strictness (new filters).** The five filters added in #4766 validate their supported inputs and raise `ValueError` naming the problem rather than leaking a Python `TypeError`/`AttributeError`: `upper`/`lower` accept only strings, `split` requires a string value and a non-empty string separator, `length` accepts only lists and strings, and `to_json` requires a JSON-serializable value whose mapping keys are strings, and additionally rejects non-finite floats (`NaN`, `Infinity`, `-Infinity`), which `json.dumps` would otherwise emit as bare tokens that are not valid JSON. Keys are checked before serialization because `json.dumps` would otherwise coerce `1` to `"1"` (colliding with an existing `"1"` key), while `sort_keys=True` raises an ordering `TypeError` on mixed key types — both surfacing as a generic "not JSON-serializable" that hides the authoring mistake. Coercion is deliberately rejected for these filters: a type mismatch almost always means the pipeline is wired to the wrong variable, and a coerced result (e.g. `"3"` for an int) would hide that. Arity is checked before the argument expression is evaluated, so a filter used with the wrong number of arguments — `| upper('x')`, `| split` with no separator, `| split(',', 1)`, `| join` bare — is reported as a known filter misused, distinct from an entirely unknown name. @@ -216,7 +216,7 @@ The older filters are more permissive and are unchanged: `join` stringifies unsu **`to_json` output is deterministic.** It pins `sort_keys=True` and `ensure_ascii=False`, so the same value always serializes to the same bytes regardless of dict insertion order or platform, and non-ASCII text is not `\uXXXX`-escaped. This buys reproducibility only — it does **not** make the result safe to pass through a shell, because expression interpolation adds no quoting or escaping. See [Interpolation and shell safety](../docs/reference/workflows.md#interpolation-and-shell-safety) before interpolating JSON into a `run` field. -**Filters and comparisons.** A filter chain cannot be an operand of an operator, so an unparenthesized trailing comparison such as `{{ inputs.items | length > 0 }}` raises an "ambiguous filter precedence" `ValueError`, as does a filter on an operator's operand (`{{ count > limit | default(5) }}`). Parenthesize the filtered operand to compare its result: `{{ (inputs.items | length) > 0 }}`. For a threshold of at least two: +**Filters and comparisons.** A filter chain cannot be an operand of an operator, so an unparenthesized trailing comparison such as `{{ inputs.items | length > 0 }}` raises an "ambiguous filter precedence" `ValueError`, as does a filter on an operator's operand (`{{ count > limit | default(5) }}`); the message names the operator. Parenthesize the filtered operand to compare its result: `{{ (inputs.items | length) > 0 }}`, `{{ count > (limit | default(5)) }}`. For a threshold of at least two: ```yaml condition: "{{ (inputs.items | length) >= 2 }}" From 23faa2dd92622d9d0a7e22eba80b42f9ba0aa191 Mon Sep 17 00:00:00 2001 From: Markus Date: Sun, 1 Nov 2026 07:20:00 +0100 Subject: [PATCH 09/12] refactor(workflows): fold engine_lark into engine and reject word operators as paths Move the Lark expression engine directly into engine.py now that engine_current.py is gone, and remove the temporary engine-swap boundary test. Exclude the reserved word operators (and, or, not, in) from the first segment of PATH so they never lex as variable names when an operand is expected, and include Lark's VisitError in _PARSE_ERRORS so int()/float() conversion errors in _BuildAst.number surface as ValueError. Assisted-by: Pi Coding Agent (model: gemini-4-argon, supervised) --- .../workflows/expressions/__init__.py | 3 +- .../workflows/expressions/engine.py | 643 ++++++++++++++++- .../workflows/expressions/engine_lark.py | 652 ------------------ tests/test_workflows.py | 5 +- tests/unit/test_condition_expression_block.py | 8 + .../unit/test_expression_package_boundary.py | 61 -- 6 files changed, 642 insertions(+), 730 deletions(-) delete mode 100644 src/specify_cli/workflows/expressions/engine_lark.py delete mode 100644 tests/unit/test_expression_package_boundary.py diff --git a/src/specify_cli/workflows/expressions/__init__.py b/src/specify_cli/workflows/expressions/__init__.py index 9aa68442f4..d69b2878e2 100644 --- a/src/specify_cli/workflows/expressions/__init__.py +++ b/src/specify_cli/workflows/expressions/__init__.py @@ -7,8 +7,7 @@ This package is split by responsibility: - ``namespace``/``filters``/``operators``/``template`` — shared leaves; -- ``engine_lark`` — the Lark (LALR) evaluator behind the contract; -- ``engine`` — the selection point (the one line that swaps the engine); +- ``engine`` — the Lark (LALR) expression parser and evaluator; - ``api`` — the public evaluation entry points; - ``diagnostics`` — condition validation and remediation. diff --git a/src/specify_cli/workflows/expressions/engine.py b/src/specify_cli/workflows/expressions/engine.py index ba45bcc00d..9bcf862310 100644 --- a/src/specify_cli/workflows/expressions/engine.py +++ b/src/specify_cli/workflows/expressions/engine.py @@ -1,26 +1,63 @@ -"""Selection point for the workflow expression engine. +"""Lark (LALR) engine for workflow expressions. -Every runtime use (``api.py``) and every validation use (``diagnostics.py``) -imports the evaluator only through this module, so a single line here switches -both together and they cannot drift onto different engines. The names below are -the full engine contract: an engine is not plugged in until it provides all of -them, not just ``_evaluate_template``. +The grammar replaces the former hand-written parser. Only the block body is +parsed here. The language is the same with these deliberate differences, all +documented in ``workflows/ARCHITECTURE.md``: + +* All comparison operators share one precedence level and group left to right + (``a == b > 0`` is ``(a == b) > 0``). The former engine's operator scan order + gave them accidental priorities; parenthesize to combine comparisons. +* Text that is not an expression -- prose inside ``{{ }}``, path keys with + characters outside ``[\\w-]`` -- is a syntax error instead of a ``None`` lookup. + +The rest of this docstring describes how the engine is built: + +* **The grammar models filter chains everywhere; ``_parse`` rejects the + misplaced ones.** ``|`` is a postfix operator that binds tighter than any + other, so ``count > limit | default(5)``, ``not x | default(1)`` and + ``x | default(0) > 5`` all parse, and the tree shows exactly which operator + has a filter chain as its operand. ``_parse`` refuses such a tree before + anyone sees it, naming the operator, so the message comes from the tree's + structure and not from guessing at the parser's failure state. A chain is + valid as a whole expression, in parentheses, as a list element or as a + filter argument. +* **The path terminal is derived from ``namespace``.** ``_PLAIN_SEGMENT`` and + ``_INDEXED_SEGMENT`` are the one definition of a path segment; widening + either rebuilds the parser (the cache is keyed on the patterns) and reaches + evaluation and validation together. No second copy to drift. +* **The contract primitives are static.** ``_collect_leaves`` walks the parse + tree for ``path`` nodes -- there is no probe run, no context variable, no + placeholder namespace. ``_evaluator_rejects`` checks filter nodes against + ``FILTER_TABLE``. ``_has_incomplete_operand`` classifies the parse failure: + end of text or an operator token means a missing operand, anything else is a + syntax error and reported with its position through ``_syntax_error``. +* **Template level unchanged.** The shared quote-aware scanner still decides + what is one block and what is text; only the block body is parsed here. + +Everything else -- namespace, filters, comparison semantics, diagnostics -- +is the shared package, unchanged. """ from __future__ import annotations -from .engine_lark import ( - _COMPARISON_OPERATORS, - _WORD_OPERATORS, - _collect_leaves, - _evaluate_template, - _evaluator_rejects, +from functools import lru_cache +from typing import Any + +from lark import Lark, Token, Transformer, Tree, v_args +from lark.exceptions import UnexpectedCharacters, UnexpectedInput, UnexpectedToken, VisitError + +from . import namespace +from .filters import FILTER_TABLE, _EXPECTED_FILTERS +from .namespace import _resolve_dot_path +from .operators import _safe_compare, _safe_membership +from .template import ( + _collapse_whitespace, + _find_block_close, _first_unclosable_block, - _has_incomplete_operand, _is_single_expression, - _syntax_error, ) + __all__ = [ "_COMPARISON_OPERATORS", "_WORD_OPERATORS", @@ -32,3 +69,581 @@ "_is_single_expression", "_syntax_error", ] + + +# Order matters -- multi-char operators first. The contract constant the +# diagnostics and the tests read; the grammar below implements the same set +# (``CMP_OP`` and the word terminals), so a new operator needs both lines. +_COMPARISON_OPERATORS = ("!=", "==", ">=", "<=", ">", "<", " not in ", " in ") + +_WORD_OPERATORS = tuple( + op for op in (" or ", " and ") + _COMPARISON_OPERATORS if op.startswith(" ") +) + + +def _path_terminal_pattern(plain_pattern: str, indexed_pattern: str) -> str: + """The ``PATH`` regex, built from the namespace's own segment definitions. + + ``_PLAIN_SEGMENT`` (``^[\\w-]+$``) and ``_INDEXED_SEGMENT`` + (``^([\\w-]+)\\[(-?\\d+)\\]$``) are stripped of their anchors and combined: + every segment is a plain or an indexed one. The *first* segment excludes a + leading digit (so ``123`` keeps lexing as a number) and the reserved word + operators (``and``, ``or``, ``not``, ``in``, so they never lex as a path + where an operand is expected); that is the only difference from the + namespace pattern, and it is why this derivation reads the pattern instead + of re-describing the shape. + + ``_INDEXED_SEGMENT`` is read through ``namespace`` at call time, so the + parser cache below rebuilds when it is widened -- the drift guard + ``test_gate_reads_the_shared_indexed_segment_definition`` rests on that. + """ + plain = plain_pattern.strip("^$") + indexed = indexed_pattern.strip("^$") + later = f"(?:{indexed}|{plain})" + first = f"(?:{indexed}|(?!(?:and|or|not|in)(?![\\w-]))[^\\W\\d]{plain[:-1]}*)" + return f"{first}(?:\\.{later})*" + + +def _grammar(path_pattern: str) -> str: + return r""" +?start: expr + +?expr: or_expr + +?or_expr: and_expr + | or_expr OR and_expr -> or_op + +?and_expr: not_expr + | and_expr AND not_expr -> and_op + +?not_expr: NOT not_expr -> not_op + | comparison + +?comparison: postfix + | comparison CMP_OP cmp_rhs -> cmp + +?cmp_rhs: postfix + | NOT cmp_rhs -> not_op + +?postfix: operand + | postfix "|" filter -> filtered + +?operand: string + | number + | list_lit + | path + | group + +group: "(" start ")" + +string: STRING +number: NUMBER +path: PATH + +list_lit: "[" list_body? "]" +list_body: [start] ("," [start])* + +?filter: filter_bare + | filter_call + +filter_bare: FILTER_NAME +filter_call: FILTER_NAME "(" [start ("," start)*] ")" + +CMP_OP.2: "!=" | "==" | ">=" | "<=" | ">" | "<" | /(? Lark: + """Build the parser for one pair of namespace patterns, once. + + Keyed on the patterns (not a single cached instance) so widening + ``namespace._INDEXED_SEGMENT`` builds a parser that accepts the wider form + instead of serving a stale one. + """ + return Lark( + _grammar(_path_terminal_pattern(plain_pattern, indexed_pattern)), + parser="lalr", + maybe_placeholders=False, + ) + + +def _parser() -> Lark: + return _parser_for( + namespace._PLAIN_SEGMENT.pattern, namespace._INDEXED_SEGMENT.pattern + ) + + +@v_args(inline=True) +class _BuildAst(Transformer): + """Turn the parse tree into the same kind of tuples the other engines walk. + + One small method per grammar rule; aliases (``-> cmp``, ``-> filter_call``) + name the shape, and anonymous terminals (``"|"``, ``"("``, ``","``) are + filtered out by Lark, so the children are exactly the operands. + """ + + def string(self, token: Token) -> tuple[str, str]: + return ("lit", str(token)[1:-1]) + + def number(self, token: Token) -> tuple[str, Any]: + text = str(token) + return ("lit", float(text) if "." in text else int(text)) + + def path(self, token: Token) -> tuple[str, str]: + return ("path", str(token)) + + def group(self, inner: Any) -> Any: + return inner + + def list_body(self, *items: Any) -> tuple[Any, ...]: + return items + + def list_lit(self, body: tuple[Any, ...] | None = None) -> tuple[str, tuple[Any, ...]]: + return ("list", body or ()) + + def filter_bare(self, name: Token) -> tuple[str, str, tuple[Any, ...], bool]: + return ("filter", str(name), (), False) + + def filter_call(self, name: Token, *args: Any) -> tuple[str, str, tuple[Any, ...], bool]: + return ("filter", str(name), args, True) + + def filtered(self, head: Any, filt: Any) -> tuple[str, Any, tuple[Any, ...]]: + # The grammar nests a chain to the left; flatten it into one pipe. + if head[0] == "pipe": + return ("pipe", head[1], head[2] + (filt,)) + return ("pipe", head, (filt,)) + + def cmp(self, left: Any, op: Token, right: Any) -> tuple[str, ...]: + return ("cmp", " ".join(str(op).split()), left, right) + + def or_op(self, left: Any, _op: Token, right: Any) -> tuple[str, Any, Any]: + return ("or", left, right) + + def and_op(self, left: Any, _op: Token, right: Any) -> tuple[str, Any, Any]: + return ("and", left, right) + + def not_op(self, _op: Token, inner: Any) -> tuple[str, Any]: + return ("not", inner) + + +class _FilterPrecedenceError(Exception): + """A filter chain is the operand of an operator. + + ``operator`` is the operator token. ``chain_first`` is true when the chain + comes before the operator (``items | length > 0``) and false when the + operator comes first (``count > limit | default(5)``, ``not x | f``). + """ + + def __init__(self, operator: Token, chain_first: bool) -> None: + super().__init__(str(operator)) + self.operator = operator + self.chain_first = chain_first + + +_OPERATOR_NODES = frozenset({"cmp", "and_op", "or_op", "not_op"}) + + +def _misplaced_filter_chains(tree: Tree) -> list[_FilterPrecedenceError]: + """Every operator in *tree* that has a filter chain as a direct operand. + + Parentheses keep their ``group`` node, so ``(items | length) > 0`` has a + group operand, not a chain. + """ + found: list[_FilterPrecedenceError] = [] + for node in tree.iter_subtrees(): + if node.data not in _OPERATOR_NODES: + continue + operator = node.children[0] if node.data == "not_op" else node.children[1] + for index, child in enumerate(node.children): + if isinstance(child, Tree) and child.data == "filtered": + chain_first = node.data != "not_op" and index == 0 + found.append(_FilterPrecedenceError(operator, chain_first)) + return found + + +def _parse(text: str) -> Any: + """Parse one block body into the tuple AST. + + Raises ``lark`` errors for text the grammar does not accept, and + ``_FilterPrecedenceError`` for the leftmost operator that has a filter + chain as an operand. Every caller parses through here, so no caller sees + such a tree. + """ + tree = _parser().parse(text) + misplaced = _misplaced_filter_chains(tree) + if misplaced: + raise min(misplaced, key=lambda err: err.operator.start_pos) + return _BuildAst().transform(tree) + + +# What a failed ``_parse`` raises. ``VisitError`` wraps ``ValueError`` from +# ``int()``/``float()`` inside ``_BuildAst.number`` (e.g. an integer exceeding +# Python's 4300-digit string conversion limit). +_PARSE_ERRORS = (UnexpectedInput, _FilterPrecedenceError, VisitError) + + +def _parse_error_message( + text: str, exc: UnexpectedInput | _FilterPrecedenceError | VisitError +) -> str: + """The author-facing message for a parse failure of *text*. + + Filter precedence is the one carried-over semantic. A filter chain cannot + be an operand of an operator, in either direction: a ``|`` on an + operator's operand (``count > limit | default(5)``), or an operator after a + filter chain (``items | length > 0``). Everything else names the offending + character and column. + """ + if isinstance(exc, VisitError): + return f"invalid expression '{text}': {exc.orig_exc}" + if isinstance(exc, _FilterPrecedenceError): + operator = " ".join(str(exc.operator).split()) + if not exc.chain_first: + return ( + f"ambiguous filter precedence in '{text}': a filter cannot be an " + f"operand of '{operator}'; parenthesize the filtered operand, " + f"as in '(inputs.limit | default(5))'" + ) + return ( + f"ambiguous filter precedence in '{text}': '{operator}' follows a " + f"filter chain, and a filter chain cannot be an operand of an " + f"operator; parenthesize the filtered operand, as in " + f"'(inputs.items | length) > 0'" + ) + token = getattr(exc, "token", None) + if token is not None and token.type == "$END": + return f"invalid expression '{text}': unexpected end of text" + if isinstance(exc, UnexpectedCharacters): + char = getattr(exc, "char", "?") + column = getattr(exc, "column", "?") + return f"invalid expression '{text}': unexpected character '{char}' at column {column}" + if token is not None and token.value: + column = getattr(token, "column", "?") + return f"invalid expression '{text}': unexpected '{token.value}' at column {column}" + return f"invalid expression '{text}'" + + +def _syntax_error(text: str) -> str | None: + """The parser's complaint about *text*, or ``None`` when it parses. + + The diagnostics ask this before walking leaves: text that does not parse + has no leaves, and "no unresolvable term" would tell the author their + expression is fine when the evaluator will refuse it. + """ + collapsed = _collapse_whitespace(text) + if not collapsed: + return None + try: + _parse(collapsed) + except _PARSE_ERRORS as exc: + return _parse_error_message(collapsed, exc) + return None + + +# -- Evaluation --------------------------------------------------------------- + +def _apply_filter( + value: Any, filt: tuple[str, str, tuple[Any, ...], bool], namespace_: dict[str, Any] +) -> Any: + """Apply one ``("filter", name, args, has_parens)`` node to *value*. + + The call form is checked against ``FILTER_TABLE`` before any argument is + evaluated, so an unknown or misused filter is reported as such even when + an argument would itself fail. The diagnostics walk the parse tree for + argument leaves, so they do not need the arguments evaluated first. The + error wording comes from ``FILTER_TABLE`` and ``_EXPECTED_FILTERS``: the + same table the message lists, so adding a filter cannot leave the two + disagreeing. + """ + _, name, arg_nodes, has_parens = filt + display = f"{name}(...)" if has_parens else name + + spec = FILTER_TABLE.get(name) + if spec is None: + raise ValueError( + f"unknown filter '{name}': {_EXPECTED_FILTERS} (got '| {display}')" + ) + if not spec.argument: + # A zero-argument filter tolerates no parentheses and no trailing + # tokens; `from_json()` and `from_json('x')` are the same mistake. + if has_parens: + raise ValueError( + f"{name}: expected '| {name}' with no arguments or trailing " + f"tokens, got '| {display}'" + ) + return spec.function(value) + if has_parens and len(arg_nodes) == 1: + return spec.function(value, _eval(arg_nodes[0], namespace_)) + if not has_parens and spec.bare: + return spec.function(value) + raise ValueError( + f"filter '{name}' used in an unsupported form (got '| {display}'): " + f"{_EXPECTED_FILTERS}" + ) + + +def _eval(node: Any, namespace_: dict[str, Any]) -> Any: + """Evaluate a tuple AST node against *namespace_*.""" + tag = node[0] + if tag == "lit": + return node[1] + if tag == "path": + text = node[1] + # A bare word that is a literal keyword is a value, not a name; the + # dot/bracket guard keeps `inputs.true` a path. + if _is_literal_path(text): + lowered = text.lower() + if lowered == "true": + return True + if lowered == "false": + return False + return None + return _resolve_dot_path(namespace_, text) + if tag == "list": + return [_eval(item, namespace_) for item in node[1]] + if tag == "pipe": + value = _eval(node[1], namespace_) + for filt in node[2]: + value = _apply_filter(value, filt, namespace_) + return value + if tag == "not": + return not bool(_eval(node[1], namespace_)) + + if tag == "cmp": + op = node[1] + left = _eval(node[2], namespace_) + right = _eval(node[3], namespace_) + if op == "==": + return left == right + if op == "!=": + return left != right + if op == "in": + return _safe_membership(left, right, negate=False) + if op == "not in": + return _safe_membership(left, right, negate=True) + return _safe_compare(left, right, op) + + left = _eval(node[1], namespace_) + # Skip the right operand when the left one decides, so a guard protects it. + # Validation walks the parse tree instead, so it still visits both sides. + if tag == "or" and bool(left): + return True + if tag == "and" and not bool(left): + return False + right = _eval(node[2], namespace_) + if tag == "or": + return bool(left) or bool(right) + if tag == "and": + return bool(left) and bool(right) + raise ValueError(f"unsupported expression node {tag!r}") + + +def _evaluate_simple_expression(expr: str, namespace_: dict[str, Any]) -> Any: + """Evaluate the body of one ``{{ }}`` block against *namespace_*. + + A syntax error is raised as ``ValueError``, the type the evaluator already + uses for a malformed expression, rather than leaking the parser's own + exception. The message names the character and column. + """ + collapsed = _collapse_whitespace(expr) + if not collapsed: + return None + try: + ast = _parse(collapsed) + except _PARSE_ERRORS as exc: + raise ValueError(_parse_error_message(collapsed, exc)) from exc + return _eval(ast, namespace_) + + +def _interpolate_expressions(template: str, namespace_: dict[str, Any]) -> str: + """Substitute every top-level ``{{ ... }}`` block, quote-aware. + + The template level is the shared scanner, unchanged: only the body handed + to ``_evaluate_simple_expression`` goes through the parser. + """ + out: list[str] = [] + i = 0 + n = len(template) + while i < n: + start = template.find("{{", i) + if start == -1: + out.append(template[i:]) + break + out.append(template[i:start]) + close = _find_block_close(template, start) + if close == -1: + raw_close = template.find("}}", start + 2) + if raw_close == -1: + out.append(template[start:]) + break + close = raw_close + val = _evaluate_simple_expression(template[start + 2:close].strip(), namespace_) + out.append(str(val) if val is not None else "") + i = close + 2 + return "".join(out) + + +def _evaluate_template(template: str, namespace_: dict[str, Any]) -> Any: + """Evaluate *template*: a single block stays typed, text is interpolated.""" + stripped = template.strip() + if _is_single_expression(stripped): + return _evaluate_simple_expression(stripped[2:-2].strip(), namespace_) + return _interpolate_expressions(template, namespace_) + + +# -- Contract primitives -------------------------------------------------------- + +_LITERAL_WORDS = ("true", "false", "none", "null") + + +def _is_literal_path(text: str) -> bool: + """True for a bare literal keyword, which the grammar lexes as a path. + + The evaluator resolves names, not literals; the diagnostics must report the + same names the evaluator will look up, so both ask this one condition. + A dot or bracket keeps ``inputs.true`` a path. + """ + return "." not in text and "[" not in text and text.lower() in _LITERAL_WORDS + + +def _visit(node: Any, paths: list[str] | None = None, filters: list[Any] | None = None) -> None: + """Walk *node* in evaluation order, collecting paths and filter nodes. + + One traversal serves both contract primitives: ``_collect_leaves`` asks for + the names (``paths``), ``_evaluator_rejects`` asks for the filters to check + against the table (``filters``). Parents are transparent -- both sides of + ``and``/``or``, every list element and every filter argument is reached. + Text that does not parse has neither names nor filters; ``_syntax_error`` + explains why separately. + """ + tag = node[0] + if tag == "path": + if paths is not None and not _is_literal_path(node[1]): + paths.append(node[1]) + return + if tag == "list": + for item in node[1]: + _visit(item, paths, filters) + elif tag == "pipe": + _visit(node[1], paths, filters) + for filt in node[2]: + if filters is not None: + filters.append(filt) + for arg in filt[2]: + _visit(arg, paths, filters) + elif tag == "not": + _visit(node[1], paths, filters) + elif tag in ("and", "or"): + _visit(node[1], paths, filters) + _visit(node[2], paths, filters) + elif tag == "cmp": + _visit(node[2], paths, filters) + _visit(node[3], paths, filters) + + +def _collect_leaves(text: str) -> list[str]: + """Every name *text* hands to the evaluator as a path to resolve. + + Walks the parse tree rather than re-running the evaluation: no probe, no + placeholder namespace, no state to arm. + """ + collapsed = _collapse_whitespace(text) + if not collapsed: + return [] + try: + ast = _parse(collapsed) + except _PARSE_ERRORS: + return [] + leaves: list[str] = [] + _visit(ast, paths=leaves) + return leaves + + +def _check_filter(filt: tuple[str, str, tuple[Any, ...], bool]) -> str | None: + """Why *filt* would be refused at evaluation time, or ``None``.""" + _, name, args, has_parens = filt + spec = FILTER_TABLE.get(name) + if spec is None: + return f"unknown filter '{name}'" + if not spec.argument: + if has_parens: + return f"{name}: expected '| {name}' with no arguments or trailing tokens" + return None + if has_parens and len(args) == 1: + return None + if not has_parens and spec.bare: + return None + return f"filter '{name}' used in an unsupported form" + + +def _evaluator_rejects(text: str) -> str | None: + """The evaluator's own complaint about how *text* wires its filters. + + Static: the parse tree is checked against ``FILTER_TABLE`` -- the same + table the dispatcher uses -- so a filter used under an unknown name or in + an unsupported form is reported by the code that will run it, without a + probe evaluation. Text that does not parse is not a filter-wiring fault; + ``_syntax_error`` covers it. + """ + collapsed = _collapse_whitespace(text) + if not collapsed: + return None + try: + ast = _parse(collapsed) + except _PARSE_ERRORS: + return None + filters: list[Any] = [] + _visit(ast, filters=filters) + for filt in filters: + problem = _check_filter(filt) + if problem is not None: + return problem + return None + + +_OPERATOR_TOKENS = frozenset({"CMP_OP", "AND", "OR", "NOT"}) + +# Terminals the parser expects exactly where an operand (``PATH``) or a filter +# name (``FILTER_NAME``) is missing. An operator token that fails while one of +# these is expected lacks an operand; one that fails elsewhere (``not`` right +# after an operand, as in ``inputs.mode not 'done'``) is a plain syntax error. +_OPERAND_EXPECTED = frozenset({"PATH", "FILTER_NAME"}) + + +def _has_incomplete_operand(text: str) -> bool: + """True when an operator in *text* is missing an operand on either side. + + Reads the parse failure rather than restating the operator table: text + that ends where an operand was expected (``inputs.a ==``, ``inputs.a and``, + ``inputs.x |``) fails at the ``$END`` token; an operator or ``|`` where an + operand or a filter name was expected (``inputs.a == == inputs.b``, + ``inputs.a || inputs.b``, ``and inputs.x``, ``inputs.a == and``) fails on + that token. An operator with a filter chain as an operand + (``inputs.items | length > 0``) has both operands and is a filter + precedence mistake, and any other failure (arithmetic, a stray character, + a second literal) is a syntax error; both are reported by ``_syntax_error``. + """ + stripped = text.strip() + if not stripped: + return True + try: + _parse(_collapse_whitespace(stripped)) + except UnexpectedToken as exc: + token_type = str(exc.token.type) + if token_type == "$END": + return True + if token_type in _OPERATOR_TOKENS or token_type == "VBAR": + return bool(set(exc.expected) & _OPERAND_EXPECTED) + return False + except _PARSE_ERRORS: + return False + return False diff --git a/src/specify_cli/workflows/expressions/engine_lark.py b/src/specify_cli/workflows/expressions/engine_lark.py deleted file mode 100644 index 6de86af6c4..0000000000 --- a/src/specify_cli/workflows/expressions/engine_lark.py +++ /dev/null @@ -1,652 +0,0 @@ -"""Lark (LALR) engine for workflow expressions. - -The grammar replaces the former hand-written parser. Only the block body is -parsed here. The language is the same with these deliberate differences, all -documented in ``workflows/ARCHITECTURE.md``: - -* All comparison operators share one precedence level and group left to right - (``a == b > 0`` is ``(a == b) > 0``). The former engine's operator scan order - gave them accidental priorities; parenthesize to combine comparisons. -* Text that is not an expression -- prose inside ``{{ }}``, path keys with - characters outside ``[\\w-]`` -- is a syntax error instead of a ``None`` lookup. - -The rest of this docstring describes how the engine is built: - -* **The grammar models filter chains everywhere; ``_parse`` rejects the - misplaced ones.** ``|`` is a postfix operator that binds tighter than any - other, so ``count > limit | default(5)``, ``not x | default(1)`` and - ``x | default(0) > 5`` all parse, and the tree shows exactly which operator - has a filter chain as its operand. ``_parse`` refuses such a tree before - anyone sees it, naming the operator, so the message comes from the tree's - structure and not from guessing at the parser's failure state. A chain is - valid as a whole expression, in parentheses, as a list element or as a - filter argument. -* **The path terminal is derived from ``namespace``.** ``_PLAIN_SEGMENT`` and - ``_INDEXED_SEGMENT`` are the one definition of a path segment; widening - either rebuilds the parser (the cache is keyed on the patterns) and reaches - evaluation and validation together. No second copy to drift. -* **The contract primitives are static.** ``_collect_leaves`` walks the parse - tree for ``path`` nodes -- there is no probe run, no context variable, no - placeholder namespace. ``_evaluator_rejects`` checks filter nodes against - ``FILTER_TABLE``. ``_has_incomplete_operand`` classifies the parse failure: - end of text or an operator token means a missing operand, anything else is a - syntax error and reported with its position through ``_syntax_error``. -* **Template level unchanged.** The shared quote-aware scanner still decides - what is one block and what is text; only the block body is parsed here. - -Everything else -- namespace, filters, comparison semantics, diagnostics -- -is the shared package, unchanged. -""" - -from __future__ import annotations - -import re -from functools import lru_cache -from typing import Any - -from lark import Lark, Token, Transformer, Tree, v_args -from lark.exceptions import UnexpectedCharacters, UnexpectedInput, UnexpectedToken - -from . import namespace -from .filters import FILTER_TABLE, _EXPECTED_FILTERS -from .namespace import _resolve_dot_path -from .operators import _safe_compare, _safe_membership -from .template import ( - _collapse_whitespace, - _find_block_close, - _first_unclosable_block, - _is_single_expression, -) - - -__all__ = [ - "_COMPARISON_OPERATORS", - "_WORD_OPERATORS", - "_collect_leaves", - "_evaluate_template", - "_evaluator_rejects", - "_first_unclosable_block", - "_has_incomplete_operand", - "_is_single_expression", - "_syntax_error", -] - - -# Order matters -- multi-char operators first. The contract constant the -# diagnostics and the tests read; the grammar below implements the same set -# (``CMP_OP`` and the word terminals), so a new operator needs both lines. -_COMPARISON_OPERATORS = ("!=", "==", ">=", "<=", ">", "<", " not in ", " in ") - -_WORD_OPERATORS = tuple( - op for op in (" or ", " and ") + _COMPARISON_OPERATORS if op.startswith(" ") -) - - -def _path_terminal_pattern(plain_pattern: str, indexed_pattern: str) -> str: - """The ``PATH`` regex, built from the namespace's own segment definitions. - - ``_PLAIN_SEGMENT`` (``^[\\w-]+$``) and ``_INDEXED_SEGMENT`` - (``^([\\w-]+)\\[(-?\\d+)\\]$``) are stripped of their anchors and combined: - every segment is a plain or an indexed one. The *first* segment's first - character excludes digits so ``123`` keeps lexing as a number; that is the - only difference from the namespace pattern, and it is why this derivation - reads the pattern instead of re-describing the shape. - - ``_INDEXED_SEGMENT`` is read through ``namespace`` at call time, so the - parser cache below rebuilds when it is widened -- the drift guard - ``test_gate_reads_the_shared_indexed_segment_definition`` rests on that. - """ - plain = plain_pattern.strip("^$") - indexed = indexed_pattern.strip("^$") - later = f"(?:{indexed}|{plain})" - first = f"(?:{indexed}|[^\\W\\d]{plain[:-1]}*)" - return f"{first}(?:\\.{later})*" - - -def _grammar(path_pattern: str) -> str: - return r""" -?start: expr - -?expr: or_expr - -?or_expr: and_expr - | or_expr OR and_expr -> or_op - -?and_expr: not_expr - | and_expr AND not_expr -> and_op - -?not_expr: NOT not_expr -> not_op - | comparison - -?comparison: postfix - | comparison CMP_OP cmp_rhs -> cmp - -?cmp_rhs: postfix - | NOT cmp_rhs -> not_op - -?postfix: operand - | postfix "|" filter -> filtered - -?operand: string - | number - | list_lit - | path - | group - -group: "(" start ")" - -string: STRING -number: NUMBER -path: PATH - -list_lit: "[" list_body? "]" -list_body: [start] ("," [start])* - -?filter: filter_bare - | filter_call - -filter_bare: FILTER_NAME -filter_call: FILTER_NAME "(" [start ("," start)*] ")" - -CMP_OP.2: "!=" | "==" | ">=" | "<=" | ">" | "<" | /(? Lark: - """Build the parser for one pair of namespace patterns, once. - - Keyed on the patterns (not a single cached instance) so widening - ``namespace._INDEXED_SEGMENT`` builds a parser that accepts the wider form - instead of serving a stale one. - """ - return Lark( - _grammar(_path_terminal_pattern(plain_pattern, indexed_pattern)), - parser="lalr", - maybe_placeholders=False, - ) - - -def _parser() -> Lark: - return _parser_for( - namespace._PLAIN_SEGMENT.pattern, namespace._INDEXED_SEGMENT.pattern - ) - - -@v_args(inline=True) -class _BuildAst(Transformer): - """Turn the parse tree into the same kind of tuples the other engines walk. - - One small method per grammar rule; aliases (``-> cmp``, ``-> filter_call``) - name the shape, and anonymous terminals (``"|"``, ``"("``, ``","``) are - filtered out by Lark, so the children are exactly the operands. - """ - - def string(self, token: Token) -> tuple[str, str]: - return ("lit", str(token)[1:-1]) - - def number(self, token: Token) -> tuple[str, Any]: - text = str(token) - return ("lit", float(text) if "." in text else int(text)) - - def path(self, token: Token) -> tuple[str, str]: - return ("path", str(token)) - - def group(self, inner: Any) -> Any: - return inner - - def list_body(self, *items: Any) -> tuple[Any, ...]: - return items - - def list_lit(self, body: tuple[Any, ...] | None = None) -> tuple[str, tuple[Any, ...]]: - return ("list", body or ()) - - def filter_bare(self, name: Token) -> tuple[str, str, tuple[Any, ...], bool]: - return ("filter", str(name), (), False) - - def filter_call(self, name: Token, *args: Any) -> tuple[str, str, tuple[Any, ...], bool]: - return ("filter", str(name), args, True) - - def filtered(self, head: Any, filt: Any) -> tuple[str, Any, tuple[Any, ...]]: - # The grammar nests a chain to the left; flatten it into one pipe. - if head[0] == "pipe": - return ("pipe", head[1], head[2] + (filt,)) - return ("pipe", head, (filt,)) - - def cmp(self, left: Any, op: Token, right: Any) -> tuple[str, ...]: - return ("cmp", " ".join(str(op).split()), left, right) - - def or_op(self, left: Any, _op: Token, right: Any) -> tuple[str, Any, Any]: - return ("or", left, right) - - def and_op(self, left: Any, _op: Token, right: Any) -> tuple[str, Any, Any]: - return ("and", left, right) - - def not_op(self, _op: Token, inner: Any) -> tuple[str, Any]: - return ("not", inner) - - -class _FilterPrecedenceError(Exception): - """A filter chain is the operand of an operator. - - ``operator`` is the operator token. ``chain_first`` is true when the chain - comes before the operator (``items | length > 0``) and false when the - operator comes first (``count > limit | default(5)``, ``not x | f``). - """ - - def __init__(self, operator: Token, chain_first: bool) -> None: - super().__init__(str(operator)) - self.operator = operator - self.chain_first = chain_first - - -_OPERATOR_NODES = frozenset({"cmp", "and_op", "or_op", "not_op"}) - - -def _misplaced_filter_chains(tree: Tree) -> list[_FilterPrecedenceError]: - """Every operator in *tree* that has a filter chain as a direct operand. - - Parentheses keep their ``group`` node, so ``(items | length) > 0`` has a - group operand, not a chain. - """ - found: list[_FilterPrecedenceError] = [] - for node in tree.iter_subtrees(): - if node.data not in _OPERATOR_NODES: - continue - operator = node.children[0] if node.data == "not_op" else node.children[1] - for index, child in enumerate(node.children): - if isinstance(child, Tree) and child.data == "filtered": - chain_first = node.data != "not_op" and index == 0 - found.append(_FilterPrecedenceError(operator, chain_first)) - return found - - -def _parse(text: str) -> Any: - """Parse one block body into the tuple AST. - - Raises ``lark`` errors for text the grammar does not accept, and - ``_FilterPrecedenceError`` for the leftmost operator that has a filter - chain as an operand. Every caller parses through here, so no caller sees - such a tree. - """ - tree = _parser().parse(text) - misplaced = _misplaced_filter_chains(tree) - if misplaced: - raise min(misplaced, key=lambda err: err.operator.start_pos) - return _BuildAst().transform(tree) - - -# What a failed ``_parse`` raises. -_PARSE_ERRORS = (UnexpectedInput, _FilterPrecedenceError) - - -def _parse_error_message( - text: str, exc: UnexpectedInput | _FilterPrecedenceError -) -> str: - """The author-facing message for a parse failure of *text*. - - Filter precedence is the one carried-over semantic. A filter chain cannot - be an operand of an operator, in either direction: a ``|`` on an - operator's operand (``count > limit | default(5)``), or an operator after a - filter chain (``items | length > 0``). Everything else names the offending - character and column. - """ - if isinstance(exc, _FilterPrecedenceError): - operator = " ".join(str(exc.operator).split()) - if not exc.chain_first: - return ( - f"ambiguous filter precedence in '{text}': a filter cannot be an " - f"operand of '{operator}'; parenthesize the filtered operand, " - f"as in '(inputs.limit | default(5))'" - ) - return ( - f"ambiguous filter precedence in '{text}': '{operator}' follows a " - f"filter chain, and a filter chain cannot be an operand of an " - f"operator; parenthesize the filtered operand, as in " - f"'(inputs.items | length) > 0'" - ) - token = getattr(exc, "token", None) - if token is not None and token.type == "$END": - return f"invalid expression '{text}': unexpected end of text" - if isinstance(exc, UnexpectedCharacters): - char = getattr(exc, "char", "?") - column = getattr(exc, "column", "?") - return f"invalid expression '{text}': unexpected character '{char}' at column {column}" - if token is not None and token.value: - column = getattr(token, "column", "?") - return f"invalid expression '{text}': unexpected '{token.value}' at column {column}" - return f"invalid expression '{text}'" - - -def _syntax_error(text: str) -> str | None: - """The parser's complaint about *text*, or ``None`` when it parses. - - The diagnostics ask this before walking leaves: text that does not parse - has no leaves, and "no unresolvable term" would tell the author their - expression is fine when the evaluator will refuse it. - """ - collapsed = _collapse_whitespace(text) - if not collapsed: - return None - try: - _parse(collapsed) - except _PARSE_ERRORS as exc: - return _parse_error_message(collapsed, exc) - return None - - -# -- Evaluation --------------------------------------------------------------- - -def _apply_filter( - value: Any, filt: tuple[str, str, tuple[Any, ...], bool], namespace_: dict[str, Any] -) -> Any: - """Apply one ``("filter", name, args, has_parens)`` node to *value*. - - The call form is checked against ``FILTER_TABLE`` before any argument is - evaluated, so an unknown or misused filter is reported as such even when - an argument would itself fail. The diagnostics walk the parse tree for - argument leaves, so they do not need the arguments evaluated first. The - error wording comes from ``FILTER_TABLE`` and ``_EXPECTED_FILTERS``: the - same table the message lists, so adding a filter cannot leave the two - disagreeing. - """ - _, name, arg_nodes, has_parens = filt - display = f"{name}(...)" if has_parens else name - - spec = FILTER_TABLE.get(name) - if spec is None: - raise ValueError( - f"unknown filter '{name}': {_EXPECTED_FILTERS} (got '| {display}')" - ) - if not spec.argument: - # A zero-argument filter tolerates no parentheses and no trailing - # tokens; `from_json()` and `from_json('x')` are the same mistake. - if has_parens: - raise ValueError( - f"{name}: expected '| {name}' with no arguments or trailing " - f"tokens, got '| {display}'" - ) - return spec.function(value) - if has_parens and len(arg_nodes) == 1: - return spec.function(value, _eval(arg_nodes[0], namespace_)) - if not has_parens and spec.bare: - return spec.function(value) - raise ValueError( - f"filter '{name}' used in an unsupported form (got '| {display}'): " - f"{_EXPECTED_FILTERS}" - ) - - -def _eval(node: Any, namespace_: dict[str, Any]) -> Any: - """Evaluate a tuple AST node against *namespace_*.""" - tag = node[0] - if tag == "lit": - return node[1] - if tag == "path": - text = node[1] - # A bare word that is a literal keyword is a value, not a name; the - # dot/bracket guard keeps `inputs.true` a path. - if _is_literal_path(text): - lowered = text.lower() - if lowered == "true": - return True - if lowered == "false": - return False - return None - return _resolve_dot_path(namespace_, text) - if tag == "list": - return [_eval(item, namespace_) for item in node[1]] - if tag == "pipe": - value = _eval(node[1], namespace_) - for filt in node[2]: - value = _apply_filter(value, filt, namespace_) - return value - if tag == "not": - return not bool(_eval(node[1], namespace_)) - - if tag == "cmp": - op = node[1] - left = _eval(node[2], namespace_) - right = _eval(node[3], namespace_) - if op == "==": - return left == right - if op == "!=": - return left != right - if op == "in": - return _safe_membership(left, right, negate=False) - if op == "not in": - return _safe_membership(left, right, negate=True) - return _safe_compare(left, right, op) - - left = _eval(node[1], namespace_) - # Skip the right operand when the left one decides, so a guard protects it. - # Validation walks the parse tree instead, so it still visits both sides. - if tag == "or" and bool(left): - return True - if tag == "and" and not bool(left): - return False - right = _eval(node[2], namespace_) - if tag == "or": - return bool(left) or bool(right) - if tag == "and": - return bool(left) and bool(right) - raise ValueError(f"unsupported expression node {tag!r}") - - -def _evaluate_simple_expression(expr: str, namespace_: dict[str, Any]) -> Any: - """Evaluate the body of one ``{{ }}`` block against *namespace_*. - - A syntax error is raised as ``ValueError``, the type the evaluator already - uses for a malformed expression, rather than leaking the parser's own - exception. The message names the character and column. - """ - collapsed = _collapse_whitespace(expr) - if not collapsed: - return None - try: - ast = _parse(collapsed) - except _PARSE_ERRORS as exc: - raise ValueError(_parse_error_message(collapsed, exc)) from exc - return _eval(ast, namespace_) - - -def _interpolate_expressions(template: str, namespace_: dict[str, Any]) -> str: - """Substitute every top-level ``{{ ... }}`` block, quote-aware. - - The template level is the shared scanner, unchanged: only the body handed - to ``_evaluate_simple_expression`` goes through the parser. - """ - out: list[str] = [] - i = 0 - n = len(template) - while i < n: - start = template.find("{{", i) - if start == -1: - out.append(template[i:]) - break - out.append(template[i:start]) - close = _find_block_close(template, start) - if close == -1: - raw_close = template.find("}}", start + 2) - if raw_close == -1: - out.append(template[start:]) - break - close = raw_close - val = _evaluate_simple_expression(template[start + 2:close].strip(), namespace_) - out.append(str(val) if val is not None else "") - i = close + 2 - return "".join(out) - - -def _evaluate_template(template: str, namespace_: dict[str, Any]) -> Any: - """Evaluate *template*: a single block stays typed, text is interpolated.""" - stripped = template.strip() - if _is_single_expression(stripped): - return _evaluate_simple_expression(stripped[2:-2].strip(), namespace_) - return _interpolate_expressions(template, namespace_) - - -# -- Contract primitives -------------------------------------------------------- - -_LITERAL_WORDS = ("true", "false", "none", "null") - - -def _is_literal_path(text: str) -> bool: - """True for a bare literal keyword, which the grammar lexes as a path. - - The evaluator resolves names, not literals; the diagnostics must report the - same names the evaluator will look up, so both ask this one condition. - A dot or bracket keeps ``inputs.true`` a path. - """ - return "." not in text and "[" not in text and text.lower() in _LITERAL_WORDS - - -def _visit(node: Any, paths: list[str] | None = None, filters: list[Any] | None = None) -> None: - """Walk *node* in evaluation order, collecting paths and filter nodes. - - One traversal serves both contract primitives: ``_collect_leaves`` asks for - the names (``paths``), ``_evaluator_rejects`` asks for the filters to check - against the table (``filters``). Parents are transparent -- both sides of - ``and``/``or``, every list element and every filter argument is reached. - Text that does not parse has neither names nor filters; ``_syntax_error`` - explains why separately. - """ - tag = node[0] - if tag == "path": - if paths is not None and not _is_literal_path(node[1]): - paths.append(node[1]) - return - if tag == "list": - for item in node[1]: - _visit(item, paths, filters) - elif tag == "pipe": - _visit(node[1], paths, filters) - for filt in node[2]: - if filters is not None: - filters.append(filt) - for arg in filt[2]: - _visit(arg, paths, filters) - elif tag == "not": - _visit(node[1], paths, filters) - elif tag in ("and", "or"): - _visit(node[1], paths, filters) - _visit(node[2], paths, filters) - elif tag == "cmp": - _visit(node[2], paths, filters) - _visit(node[3], paths, filters) - - -def _collect_leaves(text: str) -> list[str]: - """Every name *text* hands to the evaluator as a path to resolve. - - Walks the parse tree rather than re-running the evaluation: no probe, no - placeholder namespace, no state to arm. - """ - collapsed = _collapse_whitespace(text) - if not collapsed: - return [] - try: - ast = _parse(collapsed) - except _PARSE_ERRORS: - return [] - leaves: list[str] = [] - _visit(ast, paths=leaves) - return leaves - - -def _check_filter(filt: tuple[str, str, tuple[Any, ...], bool]) -> str | None: - """Why *filt* would be refused at evaluation time, or ``None``.""" - _, name, args, has_parens = filt - spec = FILTER_TABLE.get(name) - if spec is None: - return f"unknown filter '{name}'" - if not spec.argument: - if has_parens: - return f"{name}: expected '| {name}' with no arguments or trailing tokens" - return None - if has_parens and len(args) == 1: - return None - if not has_parens and spec.bare: - return None - return f"filter '{name}' used in an unsupported form" - - -def _evaluator_rejects(text: str) -> str | None: - """The evaluator's own complaint about how *text* wires its filters. - - Static: the parse tree is checked against ``FILTER_TABLE`` -- the same - table the dispatcher uses -- so a filter used under an unknown name or in - an unsupported form is reported by the code that will run it, without a - probe evaluation. Text that does not parse is not a filter-wiring fault; - ``_syntax_error`` covers it. - """ - collapsed = _collapse_whitespace(text) - if not collapsed: - return None - try: - ast = _parse(collapsed) - except _PARSE_ERRORS: - return None - filters: list[Any] = [] - _visit(ast, filters=filters) - for filt in filters: - problem = _check_filter(filt) - if problem is not None: - return problem - return None - - -_OPERATOR_TOKENS = frozenset({"CMP_OP", "AND", "OR", "NOT"}) - -# Terminals the parser expects exactly where an operand (``PATH``) or a filter -# name (``FILTER_NAME``) is missing. An operator token that fails while one of -# these is expected lacks an operand; one that fails elsewhere (``not`` right -# after an operand, as in ``inputs.mode not 'done'``) is a plain syntax error. -_OPERAND_EXPECTED = frozenset({"PATH", "FILTER_NAME"}) - -# A word operator at the very start of the text. The LALR lexer only offers -# CMP_OP/AND/OR/NOT where the grammar can take them, so without a left operand -# `in`/`and`/`or` lex as paths and the failure lands on the *next* token; the -# leading word is what identifies the missing operand. `not x` is valid, so -# only `not in` counts. -_LEADING_WORD_OPERATOR = re.compile(r"^(?:and|or|in)\b|^not\s+in\b") - - -def _has_incomplete_operand(text: str) -> bool: - """True when an operator in *text* is missing an operand on either side. - - Reads the parse failure rather than restating the operator table: text - that ends where an operand was expected (``inputs.a ==``, ``inputs.a and``, - ``inputs.x |``) fails at the ``$END`` token; an operator or ``|`` where an - operand or a filter name was expected (``inputs.a == == inputs.b``, - ``inputs.a || inputs.b``) fails on that token; a leading word operator - (``in inputs.a``, ``and inputs.x``) fails right behind the word. An operator - with a filter chain as an operand (``inputs.items | length > 0``) has both - operands and is a filter precedence mistake, and any other failure - (arithmetic, a stray character, a second literal) is a syntax error; both - are reported by ``_syntax_error``. - """ - stripped = text.strip() - if not stripped: - return True - try: - _parse(_collapse_whitespace(stripped)) - except UnexpectedToken as exc: - token_type = str(exc.token.type) - if token_type == "$END": - return True - if token_type in _OPERATOR_TOKENS or token_type == "VBAR": - return bool(set(exc.expected) & _OPERAND_EXPECTED) - return bool(_LEADING_WORD_OPERATOR.match(stripped)) - except _PARSE_ERRORS: - return False - return False diff --git a/tests/test_workflows.py b/tests/test_workflows.py index e98bd67814..53efe34df9 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1729,7 +1729,7 @@ def test_malformed_number_literals_are_refused(self): from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext - for text in ("1__0", "1_", "1e3", "5..", "."): + for text in ("1__0", "1_", "1e3", "5..", ".", "9" * 5000): with pytest.raises(ValueError, match="invalid expression"): evaluate_expression("{{ " + text + " }}", StepContext(inputs={})) @@ -1807,6 +1807,9 @@ def test_text_that_is_not_an_expression_is_refused(self): with pytest.raises(ValueError, match="unexpected character ':'"): evaluate_expression("{{ inputs.weird:key }}", ctx) assert evaluate_expression("{{ inputs.ok-key }}", ctx) == "v" + for text in ("and", "or", "in", "not in", "inputs.a == and", "inputs.a and or inputs.b"): + with pytest.raises(ValueError, match="invalid expression"): + evaluate_expression("{{ " + text + " }}", ctx) def test_item_and_fan_in_namespace(self): """The ``item`` and ``fan_in`` roots resolve inside fan-out/fan-in, like diff --git a/tests/unit/test_condition_expression_block.py b/tests/unit/test_condition_expression_block.py index 6be3819a15..08b70adf39 100644 --- a/tests/unit/test_condition_expression_block.py +++ b/tests/unit/test_condition_expression_block.py @@ -555,9 +555,17 @@ def test_incomplete_operand_covers_every_operator_the_evaluator_splits_on(): ("(inputs.x | upper) | lower == 'x'", False), ("inputs.a || inputs.b", True), ("inputs.a == == inputs.b", True), + ("inputs.a == and", True), + ("inputs.a and or inputs.b", True), + ("and", True), + ("or", True), + ("in", True), + ("not in", True), + ("not", True), ("| upper", True), ("inputs.items | length >", True), ("inputs.mode not 'done'", False), + ("9" * 5000, False), ], ) def test_incomplete_operand_tells_a_filter_chain_from_a_missing_operand( diff --git a/tests/unit/test_expression_package_boundary.py b/tests/unit/test_expression_package_boundary.py deleted file mode 100644 index b9804dc03d..0000000000 --- a/tests/unit/test_expression_package_boundary.py +++ /dev/null @@ -1,61 +0,0 @@ -"""Boundary test: only ``engine.py`` and the package facade see an engine. - -Runtime and validation must reach the evaluator through the ``engine`` contract, -never by importing ``engine_lark`` (or any future engine) directly, or a -swapped engine would leave one of them on the old implementation. This pins -the import boundary the package layout is built around. -""" - -import ast -from pathlib import Path - -from specify_cli.workflows import expressions - -# Modules allowed to name an engine implementation. ``engine.py`` is the -# selection point; ``__init__.py`` may expose test-only internals. -_ENGINE_IMPORT_ALLOWED = {"engine.py", "__init__.py"} - - -def _imports_engine_implementation(source: str, filename: str = "") -> bool: - """True when *source* imports a module whose final component is ``engine_*``.""" - tree = ast.parse(source, filename=filename) - for node in ast.walk(tree): - if isinstance(node, ast.ImportFrom): - module = (node.module or "").split(".")[-1] - names = [alias.name.split(".")[-1] for alias in node.names] - if module.startswith("engine_") or any( - name.startswith("engine_") for name in names - ): - return True - elif isinstance(node, ast.Import): - if any( - alias.name.split(".")[-1].startswith("engine_") - for alias in node.names - ): - return True - return False - - -def test_only_the_engine_module_and_facade_import_an_engine_implementation(): - package_dir = Path(expressions.__file__).parent - offenders = [ - path.name - for path in sorted(package_dir.glob("*.py")) - if path.name not in _ENGINE_IMPORT_ALLOWED - and _imports_engine_implementation( - path.read_text(encoding="utf-8"), filename=str(path) - ) - ] - assert offenders == [] - - -def test_the_engine_module_really_is_the_selection_point(): - """Guard against a vacuous boundary: ``engine.py`` must import an engine.""" - package_dir = Path(expressions.__file__).parent - engine_source = (package_dir / "engine.py").read_text(encoding="utf-8") - assert _imports_engine_implementation(engine_source, filename="engine.py") - - -def test_the_boundary_check_detects_a_direct_engine_import(): - direct = "from .engine_lark import _evaluate_template\n" - assert _imports_engine_implementation(direct, filename="diagnostics.py") From 7d8e4df740abf43d8a112f8f0072b90b24bad280 Mon Sep 17 00:00:00 2001 From: Markus Date: Sun, 1 Nov 2026 16:01:51 +0100 Subject: [PATCH 10/12] docs(workflows): warn that path keys outside [\w-] no longer resolve The docs said the former engine looked up a key such as `{{ inputs.weird:key }}` as a path and rendered the miss as an empty string or `None`. When the key existed, the lookup resolved to its value, so a workflow that uses such a key worked before and now fails with a ValueError at run time. Correct the claim in workflows/ARCHITECTURE.md, the engine docstring and the test docstring, and add a compatibility note to the workflow reference: rename the key and its references, since quoted-key lookup is not supported and validation may not catch these expressions before a run. Assisted-by: Pi Coding Agent (model: claude-opus-5.5, autonomous) --- docs/reference/workflows.md | 2 ++ src/specify_cli/workflows/expressions/engine.py | 3 ++- tests/test_workflows.py | 4 ++-- workflows/ARCHITECTURE.md | 2 +- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/reference/workflows.md b/docs/reference/workflows.md index f0bb1c2b1f..6ebd712014 100644 --- a/docs/reference/workflows.md +++ b/docs/reference/workflows.md @@ -1298,6 +1298,8 @@ Operators bind from loosest to tightest as `or`, `and`, `not`, then the comparis The body of every `{{ }}` block must be a valid expression. Text that is not one, such as prose (`{{ some text }}`), a key with characters other than letters, digits, `_` and `-` (`{{ inputs.weird:key }}`), or arithmetic (`{{ count + 1 }}`), raises a `ValueError` naming the offending token. Numbers may be written as `5`, `-5`, `+5`, `1_000`, `0.5`, `.5`, `5.`, or `1.5e3`. +> **Compatibility:** Earlier releases could resolve an expression such as `{{ inputs.weird:key }}` when the key existed; this expression now fails at runtime. Rename keys you control (for example, `weird:key` to `weird_key`) and update their references. Quoted-key lookup is not supported. Workflow validation may not detect these expressions before execution. + Example: ```yaml diff --git a/src/specify_cli/workflows/expressions/engine.py b/src/specify_cli/workflows/expressions/engine.py index 9bcf862310..bfe77f0cee 100644 --- a/src/specify_cli/workflows/expressions/engine.py +++ b/src/specify_cli/workflows/expressions/engine.py @@ -8,7 +8,8 @@ (``a == b > 0`` is ``(a == b) > 0``). The former engine's operator scan order gave them accidental priorities; parenthesize to combine comparisons. * Text that is not an expression -- prose inside ``{{ }}``, path keys with - characters outside ``[\\w-]`` -- is a syntax error instead of a ``None`` lookup. + characters outside ``[\\w-]`` -- is a syntax error instead of a path lookup, + which could resolve an existing key. The rest of this docstring describes how the engine is built: diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 53efe34df9..9d776a3193 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1795,8 +1795,8 @@ def test_operator_after_a_filter_chain_is_a_filter_precedence_error(self): def test_text_that_is_not_an_expression_is_refused(self): """Prose inside ``{{ }}`` and path keys outside ``[\\w-]`` are syntax - errors. The former engine looked them up as paths and rendered the - miss as an empty string or ``None``.""" + errors. The former engine looked them up as paths and could resolve an + existing key.""" import pytest from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext diff --git a/workflows/ARCHITECTURE.md b/workflows/ARCHITECTURE.md index 0a160567f6..dafef1736b 100644 --- a/workflows/ARCHITECTURE.md +++ b/workflows/ARCHITECTURE.md @@ -208,7 +208,7 @@ Workflow definitions use Jinja2-like `{{ expression }}` syntax for dynamic value **Operator precedence.** From loosest to tightest: `or`, `and`, `not`, then the comparisons (`==`, `!=`, `>`, `<`, `>=`, `<=`, `in`, `not in`). All comparisons share **one** level and group left to right, so no comparison has priority over another: `{{ a == b > 0 }}` is `{{ (a == b) > 0 }}`, and `{{ 1 < 2 < 3 }}` is `{{ (1 < 2) < 3 }}`. The former hand-written engine gave comparisons accidental priorities through its operator scan order; parenthesize, or join comparisons with `and`/`or`, instead of relying on either reading. A `not` on the right of a comparison negates only its operand: `{{ a == not b }}`. -**Only expressions parse.** The body of every `{{ }}` block is parsed by a grammar, so text that is not an expression is a `ValueError` naming the offending token and column: prose such as `{{ some text }}`, path keys with characters outside letters, digits, `_` and `-` (`{{ inputs.weird:key }}`), arithmetic, or two adjacent operands. The former engine looked such text up as a path and rendered the miss as an empty string or `None`. Validation of `if`/`while`/`do-while` conditions checks only the `{{ }}` delimiters and layout, not the expression inside: `{{ inputs.count + 1 }}` passes validation and fails with the `ValueError` when the step runs, as do all other fields. +**Only expressions parse.** The body of every `{{ }}` block is parsed by a grammar, so text that is not an expression is a `ValueError` naming the offending token and column: prose such as `{{ some text }}`, path keys with characters outside letters, digits, `_` and `-` (`{{ inputs.weird:key }}`), arithmetic, or two adjacent operands. The former engine looked such text up as a path: an existing key such as `inputs.weird:key` could resolve successfully, while a miss rendered as an empty string or `None`. Validation of `if`/`while`/`do-while` conditions checks only the `{{ }}` delimiters and layout, not the expression inside: `{{ inputs.count + 1 }}` passes validation and fails with the `ValueError` when the step runs, as do all other fields. **Filter argument strictness (new filters).** The five filters added in #4766 validate their supported inputs and raise `ValueError` naming the problem rather than leaking a Python `TypeError`/`AttributeError`: `upper`/`lower` accept only strings, `split` requires a string value and a non-empty string separator, `length` accepts only lists and strings, and `to_json` requires a JSON-serializable value whose mapping keys are strings, and additionally rejects non-finite floats (`NaN`, `Infinity`, `-Infinity`), which `json.dumps` would otherwise emit as bare tokens that are not valid JSON. Keys are checked before serialization because `json.dumps` would otherwise coerce `1` to `"1"` (colliding with an existing `"1"` key), while `sort_keys=True` raises an ordering `TypeError` on mixed key types — both surfacing as a generic "not JSON-serializable" that hides the authoring mistake. Coercion is deliberately rejected for these filters: a type mismatch almost always means the pipeline is wired to the wrong variable, and a coerced result (e.g. `"3"` for an int) would hide that. Arity is checked before the argument expression is evaluated, so a filter used with the wrong number of arguments — `| upper('x')`, `| split` with no separator, `| split(',', 1)`, `| join` bare — is reported as a known filter misused, distinct from an entirely unknown name. From a9fe9f7950a21c03f8faca939f37c8c4e13ab5a3 Mon Sep 17 00:00:00 2001 From: Markus Date: Sun, 1 Nov 2026 16:01:56 +0100 Subject: [PATCH 11/12] test(workflows): pin the integer limit in the number-literal test The 5,000-digit literal was refused only because of Python's default limit on integer string conversion. With PYTHONINTMAXSTRDIGITS=0 the literal parses, and test_malformed_number_literals_are_refused failed. Move the case into its own test, which sets a known limit and restores the previous one. Assisted-by: Pi Coding Agent (model: claude-opus-5.5, autonomous) --- tests/test_workflows.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 9d776a3193..55fbb3b982 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1729,10 +1729,29 @@ def test_malformed_number_literals_are_refused(self): from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext - for text in ("1__0", "1_", "1e3", "5..", ".", "9" * 5000): + for text in ("1__0", "1_", "1e3", "5..", "."): with pytest.raises(ValueError, match="invalid expression"): evaluate_expression("{{ " + text + " }}", StepContext(inputs={})) + def test_integer_conversion_error_is_reported_as_invalid_expression(self): + """The integer conversion limit is reported as an expression error.""" + import sys + + import pytest + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + previous_limit = sys.get_int_max_str_digits() + limit = sys.int_info.str_digits_check_threshold + try: + sys.set_int_max_str_digits(limit) + with pytest.raises(ValueError, match="invalid expression"): + evaluate_expression( + "{{ " + "9" * (limit + 1) + " }}", StepContext(inputs={}) + ) + finally: + sys.set_int_max_str_digits(previous_limit) + def test_not_on_the_right_of_a_comparison(self): """``a == not b`` compares against the negation, as before.""" import pytest From c5b3bb382bb171c652ee2c628bcb12db9d8c969f Mon Sep 17 00:00:00 2001 From: Markus Date: Sun, 1 Nov 2026 16:02:00 +0100 Subject: [PATCH 12/12] fix(workflows): evaluate long and/or chains without exhausting the stack The Lark engine built its AST with Lark's recursive Transformer, which uses about two Python frames per tree level, and nested `a or b or c` as `((a or b) or c)`. Building, evaluating and validating the tree all recursed once per operator, so a condition of about 500 terms or 500 nested parentheses raised RecursionError. That includes `true or ...`, which the former engine returned after its first operand, and the validation of a bare condition of that length, which raised instead of reporting the missing braces. Build the AST with Transformer_NonRecursive and flatten each and/or chain into one node, as filter chains already are. Evaluation loops over the operands and stops at the first one that decides the result; validation visits all of them. Chains and parentheses no longer depend on the recursion limit (tested to 5,000). `not` chains, comparison chains and nested lists keep the former engine's limit of about 1,000 levels. Assisted-by: Pi Coding Agent (model: claude-opus-5.5, autonomous) --- .../workflows/expressions/engine.py | 57 ++++++++++--------- tests/test_workflows.py | 32 +++++++++++ tests/unit/test_condition_expression_block.py | 10 ++++ 3 files changed, 73 insertions(+), 26 deletions(-) diff --git a/src/specify_cli/workflows/expressions/engine.py b/src/specify_cli/workflows/expressions/engine.py index bfe77f0cee..18665c30a2 100644 --- a/src/specify_cli/workflows/expressions/engine.py +++ b/src/specify_cli/workflows/expressions/engine.py @@ -44,7 +44,7 @@ from functools import lru_cache from typing import Any -from lark import Lark, Token, Transformer, Tree, v_args +from lark import Lark, Token, Transformer_NonRecursive, Tree, v_args from lark.exceptions import UnexpectedCharacters, UnexpectedInput, UnexpectedToken, VisitError from . import namespace @@ -185,12 +185,15 @@ def _parser() -> Lark: @v_args(inline=True) -class _BuildAst(Transformer): - """Turn the parse tree into the same kind of tuples the other engines walk. - - One small method per grammar rule; aliases (``-> cmp``, ``-> filter_call``) - name the shape, and anonymous terminals (``"|"``, ``"("``, ``","``) are - filtered out by Lark, so the children are exactly the operands. +class _BuildAst(Transformer_NonRecursive): + """Turn the parse tree into the tuple AST that ``_eval`` and ``_visit`` walk. + + Non-recursive, and each ``and``/``or`` chain becomes one node, so a long + chain or deep parentheses do not exhaust Python's call stack while the tree + is built, evaluated or validated. One small method per grammar rule; + aliases (``-> cmp``, ``-> filter_call``) name the shape, and anonymous + terminals (``"|"``, ``"("``, ``","``) are filtered out by Lark, so the + children are exactly the operands. """ def string(self, token: Token) -> tuple[str, str]: @@ -227,11 +230,15 @@ def filtered(self, head: Any, filt: Any) -> tuple[str, Any, tuple[Any, ...]]: def cmp(self, left: Any, op: Token, right: Any) -> tuple[str, ...]: return ("cmp", " ".join(str(op).split()), left, right) - def or_op(self, left: Any, _op: Token, right: Any) -> tuple[str, Any, Any]: - return ("or", left, right) + def or_op(self, left: Any, _op: Token, right: Any) -> tuple[str, tuple[Any, ...]]: + # As for ``filtered``: the grammar nests a chain to the left; flatten it + # so ``_eval`` loops over the operands instead of recursing per operator. + operands = left[1] if left[0] == "or" else (left,) + return ("or", operands + (right,)) - def and_op(self, left: Any, _op: Token, right: Any) -> tuple[str, Any, Any]: - return ("and", left, right) + def and_op(self, left: Any, _op: Token, right: Any) -> tuple[str, tuple[Any, ...]]: + operands = left[1] if left[0] == "and" else (left,) + return ("and", operands + (right,)) def not_op(self, _op: Token, inner: Any) -> tuple[str, Any]: return ("not", inner) @@ -433,18 +440,16 @@ def _eval(node: Any, namespace_: dict[str, Any]) -> Any: return _safe_membership(left, right, negate=True) return _safe_compare(left, right, op) - left = _eval(node[1], namespace_) - # Skip the right operand when the left one decides, so a guard protects it. - # Validation walks the parse tree instead, so it still visits both sides. - if tag == "or" and bool(left): - return True - if tag == "and" and not bool(left): - return False - right = _eval(node[2], namespace_) - if tag == "or": - return bool(left) or bool(right) - if tag == "and": - return bool(left) and bool(right) + if tag in ("and", "or"): + # Left to right, stopping at the first operand that decides the result, + # so a guard protects the rest: a true operand decides `or`, a false one + # `and`. Validation walks the parse tree instead, so it still visits + # every operand. + decides = tag == "or" + for operand in node[1]: + if bool(_eval(operand, namespace_)) is decides: + return decides + return not decides raise ValueError(f"unsupported expression node {tag!r}") @@ -521,7 +526,7 @@ def _visit(node: Any, paths: list[str] | None = None, filters: list[Any] | None One traversal serves both contract primitives: ``_collect_leaves`` asks for the names (``paths``), ``_evaluator_rejects`` asks for the filters to check - against the table (``filters``). Parents are transparent -- both sides of + against the table (``filters``). Parents are transparent -- every operand of ``and``/``or``, every list element and every filter argument is reached. Text that does not parse has neither names nor filters; ``_syntax_error`` explains why separately. @@ -544,8 +549,8 @@ def _visit(node: Any, paths: list[str] | None = None, filters: list[Any] | None elif tag == "not": _visit(node[1], paths, filters) elif tag in ("and", "or"): - _visit(node[1], paths, filters) - _visit(node[2], paths, filters) + for operand in node[1]: + _visit(operand, paths, filters) elif tag == "cmp": _visit(node[2], paths, filters) _visit(node[3], paths, filters) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 55fbb3b982..a3d4d8531e 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -416,6 +416,38 @@ def test_boolean_required_operand_still_raises(self, expression): "{{ " + expression + " }}", StepContext(inputs={"payload": ""}) ) + def test_long_boolean_chains_and_deep_parentheses_evaluate(self): + """A long ``and``/``or`` chain or deep parentheses must not exhaust the + call stack. A chain stops at the first deciding operand, so the failing + operands after it are never evaluated.""" + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"flag": True, "payload": ""}) + count = 1000 + failing = "(inputs.payload | from_json)" + assert evaluate_expression( + "{{ true or " + " or ".join([failing] * count) + " }}", ctx + ) is True + assert evaluate_expression( + "{{ false and " + " and ".join([failing] * count) + " }}", ctx + ) is False + assert evaluate_expression( + "{{ " + " or ".join(["false"] * count) + " or inputs.flag }}", ctx + ) is True + assert evaluate_expression( + "{{ " + " and ".join(["inputs.flag"] * count) + " }}", ctx + ) is True + assert evaluate_expression( + "{{ " + "(" * count + "inputs.flag" + ")" * count + " }}", ctx + ) is True + # An operand the chain reaches still fails the evaluation. + with pytest.raises(ValueError, match="from_json: invalid JSON"): + evaluate_expression( + "{{ " + " or ".join(["false"] * count) + " or " + failing + " }}", + ctx, + ) + def test_list_literal_preserves_quoted_commas(self): from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext diff --git a/tests/unit/test_condition_expression_block.py b/tests/unit/test_condition_expression_block.py index 08b70adf39..976143bb3a 100644 --- a/tests/unit/test_condition_expression_block.py +++ b/tests/unit/test_condition_expression_block.py @@ -1007,6 +1007,16 @@ def test_both_sides_of_a_boolean_are_reported(): assert _unresolvable_term("false and bogus") is not None +def test_a_long_boolean_chain_is_validated_without_exhausting_the_stack(): + """Validation walks every operand of a long chain, the last one included.""" + chain = " or ".join(["inputs.flag"] * 1000) + assert len(_collect_leaves(chain)) == 1000 + assert _unresolvable_term(chain + " or bogus") is not None + errors = IfThenStep().validate({"id": "s1", "condition": chain, "then": []}) + assert len(errors) == 1 + assert "Wrap the expression" in errors[0] + + @pytest.mark.parametrize("template", ["true or ({})", "false and ({})", "true or (false and ({}))"]) @pytest.mark.parametrize("filtered", ["inputs.x | nosuchfilter", "inputs.x | join", "inputs.x | length(1)"]) def test_filter_probe_checks_short_circuited_operands(template, filtered):