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

[Noor-ul-ain001]

What

evaluate_expression returned None for any template containing two or more {{ }} blocks with no surrounding literal text, e.g. "{{ context.run_id }} {{ inputs.issue }}". Downstream this surfaced as the literal string "None" reaching commands (the reporter hit "no issue was provided" despite issue: "23" being in the run inputs).

Fixes #3208.

Root cause

The single-expression fast path used _EXPR_PATTERN.fullmatch(), but fullmatch defeats the pattern's non-greedy (.+?) body. For "{{ a }} {{ b }}" it still matches, expanding the body to capture everything between the first {{ and the last }} — i.e. "a }} {{ b". That garbage body fails dot-path resolution and returns None directly, bypassing the sub() interpolation path that would have resolved each expression separately.

Template	fullmatch	Result
{{ inputs.issue }}	matches, body inputs.issue	✅ correct
bash x "{{ inputs.issue }}"	no match (text prefix) → sub()	✅ correct
{{ a }} {{ b }}	matches, body "a }} {{ b"	❌ None

How

Guard the fast path on stripped.count("{{") == 1 so only genuine single-expression templates take the typed-return path; multi-expression templates fall through to the existing sub() interpolation, which is already correct. Single-expression type preservation is unchanged.

Tests

Added two regression tests to TestExpressions:

• test_multi_expression_no_surrounding_text — "{{ context.run_id }} {{ inputs.issue }}" → "47c5eb4b 23"
• test_multi_expression_adjacent_no_separator — "{{ inputs.a }}{{ inputs.b }}" → "foobar"

Verified both fail on main (return None) and pass with the fix. All 31 TestExpressions tests pass; ruff clean. (The 5 symlink-test failures on my local Windows run pre-exist on main — a symlink-privilege limitation unrelated to this change; they pass on Linux CI.)

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes a workflow expression-evaluator edge case where templates containing multiple {{ ... }} blocks (and no non-whitespace surrounding text) could incorrectly take the single-expression fullmatch() fast path and return None, causing downstream commands to receive the literal string "None".

Changes:

• Adjust evaluate_expression() to only use the typed single-expression fast path when the template is truly a single expression, otherwise fall back to interpolation.
• Add regression tests covering multi-expression templates with whitespace-only separation and back-to-back expressions.

Show a summary per file

File	Description
src/specify_cli/workflows/expressions.py	Guards the typed fullmatch() fast path so multi-expression templates interpolate correctly instead of collapsing to None.
tests/test_workflows.py	Adds regression tests to ensure multi-expression templates interpolate as expected.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 2/2 changed files
• Comments generated: 2
• Review effort level: Low

[mnriem]

Please address Copilot feedback

[Copilot]

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Low

[mnriem]

Please address Copilot feedback. If not applicable, please explain why. And please resolve test & lint errors

[Copilot]

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Low

[mnriem]

Please address Copilot feedback. If not applicable, please explain why

[Noor-ul-ain001]

Addressed the Copilot feedback in 53c899a.

Main finding (the substantive one): the stripped.count("{{") == 1 guard misclassified a single expression whose string argument contains a literal {{ — e.g. {{ inputs.text | contains('{{') }} — as multi-expression. It then fell through to sub() interpolation and coerced the typed return (bool/int/list) to a string, breaking the type preservation the docstring promises. I confirmed the regression locally ('True' instead of True).

Fix: replaced the count heuristic with a match-span guard:

match = _EXPR_PATTERN.match(stripped)
if match and match.end() == len(stripped):
    return _evaluate_simple_expression(match.group(1).strip(), namespace)

The pattern's non-greedy body stops at the first }}, so:

• a two-block template ({{ a }} {{ b }}, {{ a }}{{ b }}, {{ a }} text {{ b }}) leaves a trailing }}, fails the span check, and falls through to interpolation → fixes [Bug]: expression evaluator returns None for multi-expression templates without surrounding text #3208;
• a lone expression — including one with a literal {{ inside a string literal — matches to the end and keeps its typed value.

This is strictly more correct than both the original fullmatch (broke #3208) and the count guard (broke literal-brace single expressions), and it sidesteps the {{ 'a {{ b }}' }} concern from the first comment as well.

Added a regression test test_single_expression_with_literal_braces_preserves_type asserting {{ inputs.text | contains('{{') }} returns boolean True.

Test-file findings (docstring wording + indentation) were already handled in the earlier autofix commits.

Tests/lint: all 34 TestExpressions tests pass; ruff clean. The 5 symlink-test failures are a pre-existing Windows symlink-privilege limitation (they fail identically on the base commit without this change and pass on Linux CI) — unrelated to this PR.

[Copilot]

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 2
• Review effort level: Low

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.