From dd84e04078207c2798d566aedaf3f7c55971955d Mon Sep 17 00:00:00 2001 From: NgoQuocViet2001 Date: Thu, 3 Sep 2026 16:34:04 +0700 Subject: [PATCH 1/2] fix(workflows): evaluate parenthesised expressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The operator scans in _evaluate_simple_expression skip over bracketed text, so an operator inside a nested operand is never split on. Nothing then unwrapped a group spanning the whole expression: `(a or b) and c` split at the top-level `and`, evaluated `(a or b)` as a dot path, found no such key, and got None. The `or` was never evaluated and the expression read false. So adding parentheses to make precedence explicit — the usual reason to add them — silently inverted the result: `inputs.a or inputs.b and inputs.c` was true while `(inputs.a or inputs.b) and inputs.c` was false. A step gated on such a condition is skipped with nothing reported; the malformed-condition validators do not flag it, because the syntax is valid. Unwrap a group that spans the whole expression, quote-aware and only when the opening paren closes at the very end, so `(a) and (b)` and a literal paren inside a string are untouched. --- src/specify_cli/workflows/expressions.py | 35 ++++++++++++++++++++++++ tests/test_workflows.py | 27 ++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/src/specify_cli/workflows/expressions.py b/src/specify_cli/workflows/expressions.py index 5fb036ebf2..9f504d7b31 100644 --- a/src/specify_cli/workflows/expressions.py +++ b/src/specify_cli/workflows/expressions.py @@ -552,6 +552,31 @@ def _apply_filter(value: Any, filter_expr: str, namespace: dict[str, Any]) -> An _leaf_sink: ContextVar[list[str] | None] = ContextVar("_leaf_sink", default=None) +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 _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any: """Evaluate a simple expression against the namespace. @@ -573,6 +598,16 @@ def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any: 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. diff --git a/tests/test_workflows.py b/tests/test_workflows.py index d5869887fb..d00c3d0141 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1003,6 +1003,33 @@ def test_boolean_literal(self): assert evaluate_expression("{{ true }}", ctx) is True assert evaluate_expression("{{ false }}", ctx) is False + def test_parenthesised_grouping(self): + """A parenthesised group is evaluated, not read as a dot path. + + The operator scans skip bracketed text so an operator inside an + operand is not split on. Nothing unwrapped a group spanning the whole + expression, so ``(a or b) and c`` split at the top-level ``and`` and + then looked up ``(a or b)`` as a key, got ``None``, and read false -- + adding parentheses to make precedence explicit silently inverted the + result. + """ + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"a": True, "b": False, "c": True, "n": 5}) + + assert evaluate_expression("{{ (inputs.a or inputs.b) and inputs.c }}", ctx) is True + assert evaluate_expression("{{ (inputs.b or inputs.b) and inputs.c }}", ctx) is False + assert evaluate_expression("{{ (inputs.n) }}", ctx) == 5 + assert evaluate_expression("{{ (inputs.n > 1) }}", ctx) is True + assert evaluate_expression("{{ ((inputs.n)) }}", ctx) == 5 + # A group is still only unwrapped when it spans the whole expression. + assert evaluate_expression("{{ (inputs.a) and (inputs.b) }}", ctx) is False + assert evaluate_expression("{{ (inputs.n) | default(9) }}", ctx) == 5 + # A parenthesis inside a string literal is not a group. + assert evaluate_expression("{{ 'a(b' }}", ctx) == "a(b" + assert evaluate_expression("{{ ('(') }}", ctx) == "(" + def test_list_indexing(self): from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext From d407df6ac3fffda635116853071503e2cd2e38ea Mon Sep 17 00:00:00 2001 From: NgoQuocViet2001 Date: Tue, 29 Sep 2026 14:45:36 +0700 Subject: [PATCH 2/2] test(workflows): pin the gate to the evaluator's unwrapping instead of the old leaf --- tests/unit/test_condition_expression_block.py | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/tests/unit/test_condition_expression_block.py b/tests/unit/test_condition_expression_block.py index 528798d5e2..8f85650631 100644 --- a/tests/unit/test_condition_expression_block.py +++ b/tests/unit/test_condition_expression_block.py @@ -963,20 +963,14 @@ def test_gate_reads_the_shared_indexed_segment_definition(monkeypatch): 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. - If `_evaluate_simple_expression` stops treating something as a leaf — which - is what unwrapping a parenthesised group does — the gate stops checking it, - with no change to the gate itself. + 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 not None - - real = expressions._evaluate_simple_expression - - def unwrapping(expr, namespace): - stripped = expr.strip() - if stripped.startswith("(") and stripped.endswith(")"): - return unwrapping(stripped[1:-1], namespace) - return real(expr, namespace) - - monkeypatch.setattr(expressions, "_evaluate_simple_expression", unwrapping) 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(expressions, "_is_wrapped_in_parens", lambda text: False) + assert expressions._unresolvable_term(grouped) is not None