fix: interpolate multi-expression templates instead of returning None (#3208)

src/specify_cli/workflows/expressions.py

@@ -146,6 +146,43 @@ def _build_namespace(context: Any) -> dict[str, Any]:
     return ns
 
 
+def _is_single_expression(stripped: str) -> bool:
+    """True when *stripped* is exactly one top-level ``{{ ... }}`` block.
+
+    Scans the block body for a ``}}`` that would close it early, ignoring any
+    braces inside string literals. This keeps a lone expression whose string
+    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``.
+
+    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
+    mistaken for the closing delimiter (issue #3208, follow-up review).
+    """
+    if not (stripped.startswith("{{") and stripped.endswith("}}")):
+        return False
+    inner = stripped[2:-2]
+    if not inner.strip():
+        return False
+    quote: str | None = None
+    i = 0
+    n = len(inner)
+    while i < n:
+        ch = inner[i]
+        if quote is not None:
+            if ch == quote:
+                quote = None
+        elif ch in ("'", '"'):
+            quote = ch
+        elif ch == "}" and i + 1 < n and inner[i + 1] == "}":
+            # A ``}}`` outside quotes closes the first block early.
+            return False
+        i += 1
+    return True
+
+
 def _split_top_level_commas(text: str) -> list[str]:
     """Split *text* on commas that are not inside quotes or nested brackets.
 
@@ -383,10 +420,21 @@ def evaluate_expression(template: str, context: Any) -> Any:
 
     namespace = _build_namespace(context)
 
-    # Single expression: return typed value
-    match = _EXPR_PATTERN.fullmatch(template.strip())
-    if match:
-        return _evaluate_simple_expression(match.group(1).strip(), namespace)
+    # Single expression: return typed value (preserving type).
+    #
+    # The 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: string interpolation
     def _replacer(m: re.Match[str]) -> str:

tests/test_workflows.py

@@ -226,6 +226,40 @@ def test_string_interpolation(self):
         result = evaluate_expression("Feature: {{ inputs.name }} done", ctx)
         assert result == "Feature: login done"
 
+    def test_multi_expression_no_surrounding_text(self):
+        """Two expressions with no surrounding literal text must interpolate each,
+        not collapse to None via the fullmatch fast path (#3208)."""
+        from specify_cli.workflows.expressions import evaluate_expression
+        from specify_cli.workflows.base import StepContext
+
+        ctx = StepContext(inputs={"issue": "23"}, run_id="47c5eb4b")
+        result = evaluate_expression(
+            "{{ context.run_id }} {{ inputs.issue }}", ctx
+        )
+        assert result == "47c5eb4b 23"
+
+    def test_multi_expression_adjacent_no_separator(self):
+        """Back-to-back expressions with no separator still interpolate (#3208)."""
+        from specify_cli.workflows.expressions import evaluate_expression
+        from specify_cli.workflows.base import StepContext
+
+        ctx = StepContext(inputs={"a": "foo", "b": "bar"})
+        result = evaluate_expression("{{ inputs.a }}{{ inputs.b }}", ctx)
+        assert result == "foobar"
+
+    def test_single_expression_with_literal_braces_preserves_type(self):
+        """A lone expression whose string argument contains a literal ``{{`` or ``}}``
+        must still take the typed fast path and return a bool, not a string
+        (the fix for #3208 must not coerce it to ``\"True\"``)."""
+        from specify_cli.workflows.expressions import evaluate_expression
+        from specify_cli.workflows.base import StepContext
+
+        ctx = StepContext(inputs={"text": "uses {{ jinja }} syntax"})
+        assert evaluate_expression("{{ inputs.text | contains('{{') }}", ctx) is True
+
+        ctx = StepContext(inputs={"text": "uses }} syntax"})
+        assert evaluate_expression("{{ inputs.text | contains('}}') }}", ctx) is True
+
     def test_comparison_equals(self):
         from specify_cli.workflows.expressions import evaluate_expression
         from specify_cli.workflows.base import StepContext