From 76d3c10da2a23b85012ca1ad2d53ee51dd82368a Mon Sep 17 00:00:00 2001 From: Hui Date: Wed, 30 Sep 2026 12:20:23 -0400 Subject: [PATCH] fix(workflows): split expression operators across any whitespace The evaluator matched word operators by their surrounding spaces (" or ", " and ", " in ", " not in ", a leading "not "), so an operator next to a newline or tab was never split on. A condition wrapped after its operator in YAML keeps that line break -- a `|` block keeps every one, and `>` keeps the break before a more-indented continuation line -- so `{{ inputs.skip_review or\n inputs.scope == 'docs' }}` was resolved as a single dot path, came back None, and an `if` step silently took its `else` branch. Workflow validation accepted it. Collapse each run of whitespace outside quoted strings to one space before parsing, as Jinja2 treats any whitespace between tokens alike. Quoted operands keep their text exactly. The condition gate had pinned the old behaviour: it withheld the "wrap the expression" correction for `inputs.x == 1\nand inputs.name == 'abc'` because the wrapped form read False. Wrapping now repairs it, so the case moves from MULTI_POSITION_UNFIXABLE to a test that the correction is offered and evaluates True. Assisted-by: Claude Code (model: Claude Opus 5.5, autonomous) Co-Authored-By: Claude Opus 5.5 --- src/specify_cli/workflows/expressions.py | 36 +++++++++++- tests/test_workflows.py | 58 +++++++++++++++++++ tests/unit/test_condition_expression_block.py | 23 ++++++-- 3 files changed, 112 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/workflows/expressions.py b/src/specify_cli/workflows/expressions.py index 2dc474a140..e2cd9ce215 100644 --- a/src/specify_cli/workflows/expressions.py +++ b/src/specify_cli/workflows/expressions.py @@ -578,6 +578,40 @@ def _is_wrapped_in_parens(text: str) -> bool: return False +def _collapse_whitespace(text: str) -> str: + """Strip *text* and turn each run of whitespace outside a quoted string into + one space. + + The operator scans below match word operators by their surrounding spaces + (``" or "``, ``" not in "``, ``expr.startswith("not ")``), so an operator + next to a newline or tab was never found. A condition wrapped across lines + in YAML keeps those newlines -- a ``|`` block scalar keeps every one, and a + ``>`` folded scalar keeps the break before a more-indented continuation + line -- so ``{{ inputs.a or\\n inputs.b }}`` was resolved as one dot path, + came back ``None``, and read false with no error. Jinja2 treats any + whitespace between tokens alike; so does this, while quoted operands keep + their text exactly. + """ + out: list[str] = [] + quote: str | None = None + pending_space = False + for ch in text.strip(): + if quote is not None: + out.append(ch) + if ch == quote: + quote = None + elif ch.isspace(): + pending_space = True + else: + if pending_space: + out.append(" ") + pending_space = False + if ch in ("'", '"'): + quote = ch + out.append(ch) + return "".join(out) + + def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any: """Evaluate a simple expression against the namespace. @@ -589,7 +623,7 @@ def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any: - Pipe filters: ``| default('...')``, ``| join(', ')``, ``| contains('...')``, ``| from_json``, ``| map('...')`` - String and numeric literals """ - expr = expr.strip() + 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 diff --git a/tests/test_workflows.py b/tests/test_workflows.py index acfdaa0b3b..25c2a79eb1 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1030,6 +1030,64 @@ def test_parenthesised_grouping(self): assert evaluate_expression("{{ 'a(b' }}", ctx) == "a(b" assert evaluate_expression("{{ ('(') }}", ctx) == "(" + @pytest.mark.parametrize( + ("expression", "expected"), + [ + ("{{ inputs.b or\n inputs.a }}", True), + ("{{ inputs.a and\n inputs.c }}", True), + ("{{ inputs.b\nor inputs.a }}", True), + ("{{ inputs.a\tand inputs.c }}", True), + ("{{ not\n inputs.b }}", True), + ("{{ 'x' in\n inputs.tags }}", True), + ("{{ 'z' not\n in inputs.tags }}", True), + ("{{ 'z' not in\n inputs.tags }}", True), + ("{{ (inputs.b or\r\n inputs.a) and inputs.c }}", True), + ], + ) + def test_operators_separated_by_any_whitespace(self, expression, expected): + """Word operators are found across newlines and tabs, as in Jinja2. + + The operator scans match ``" or "`` and friends by their spaces, so an + operator next to a line break was never split on: the whole expression + was looked up as one dot path and came back ``None`` -- a false + condition with no error. + """ + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext( + inputs={"a": True, "b": False, "c": True, "mode": "fast", "tags": ["x", "y"]} + ) + assert evaluate_expression(expression, ctx) is expected + + def test_condition_wrapped_across_lines_in_yaml(self): + """A long condition broken after its operator keeps the line break in + YAML (the continuation line is more indented, so ``>`` does not fold + it), and still has to evaluate as written.""" + from specify_cli.workflows.expressions import evaluate_condition + from specify_cli.workflows.base import StepContext + + step = yaml.safe_load( + "condition: >-\n" + " {{ inputs.skip_review or\n" + " inputs.scope == 'docs' }}\n" + ) + assert "\n" in step["condition"] + + ctx = StepContext(inputs={"skip_review": False, "scope": "docs"}) + assert evaluate_condition(step["condition"], ctx) is True + + def test_whitespace_inside_quoted_operand_is_kept(self): + """Collapsing whitespace between tokens leaves string literals alone.""" + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"title": "two spaces", "text": "tab\there"}) + assert evaluate_expression("{{ inputs.title == 'two spaces' }}", ctx) is True + assert evaluate_expression("{{ inputs.title == 'two spaces' }}", ctx) is False + assert evaluate_expression("{{ inputs.text\n == 'tab\there' }}", ctx) is True + assert evaluate_expression("{{ 'a or b' }}", ctx) == "a or b" + def test_list_indexing(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 f13a830526..591d7d11a4 100644 --- a/tests/unit/test_condition_expression_block.py +++ b/tests/unit/test_condition_expression_block.py @@ -560,10 +560,6 @@ def test_incomplete_operand_covers_every_operator_the_evaluator_splits_on(): ("inputs.a === inputs.b", "is not a name the evaluator can resolve"), ("bogus == 'x'", "is not one of the namespace roots"), ("inputs.payload | from_json()", "the evaluator rejects it"), - # `_find_top_level` matches " and " with literal spaces, so a newline before - # the keyword is not an operator: the wrapped form evaluates False where the - # same expression with a space evaluates True. - ("inputs.x == 1\nand inputs.name == 'abc'", "is not a name the evaluator can resolve"), ] @@ -578,6 +574,25 @@ def test_gates_inspect_every_position_not_just_the_first(step_cls, condition, ex assert expected in errors[0] +@pytest.mark.parametrize("step_cls", STEP_CLASSES) +def test_a_condition_wrapped_across_lines_gets_a_paste_ready_correction(step_cls): + """A line break before ``and`` no longer hides the operator. + + The evaluator used to match " and " with literal spaces, so the wrapped form + of this condition read False where the one-line form read True, and the gate + had to withhold the correction. It now splits on any whitespace, so wrapping + repairs the condition and the correction is offered. + """ + condition = "inputs.x == 1\nand inputs.name == 'abc'" + config = {"id": "s1", "condition": condition, "then": [], "steps": []} + errors = [e for e in step_cls().validate(config) if "'condition'" in e] + + assert len(errors) == 1 + assert "Wrap the expression" in errors[0] + ctx = StepContext(inputs={"x": 1, "name": "abc"}) + assert evaluate_condition("{{ " + condition + " }}", ctx) is True + + @pytest.mark.parametrize( "text,unbalanced", [