[aarjang]

Fixes #5071.

Problem

A chained comparison such as {{ 0 <= inputs.n <= 10 }} does not behave like a range check. On the Lark engine (after #5171) the documented left-to-right grouping evaluates it as (0 <= inputs.n) <= 10, so with n = 50 the first pair yields True and _safe_compare(True, 10, "<=") answers True — an if/while step silently takes the wrong branch for every out-of-range value. Authors reading the expression as Python or Jinja2 (where a <= b <= c means a <= b and b <= c) get the opposite of what they wrote, with no error.

Fix (option 1, as chosen by the maintainer in the PR discussion)

Python/Jinja2 chained-comparison semantics in engine.py, grammar unchanged:

• _BuildAst.cmp now flattens the grammar's left-nested comparisons into one ("chain", operands, ops) node — the same flattening and/or and filter chains already use.
• _eval evaluates a chain pairwise, left to right: each shared operand is evaluated once, and evaluation stops at the first false pair, so a later operand is only reached when every pair before it held (the same guard behavior as and; a reached operand's errors still fail the run).
• Explicit parentheses end a chain: _BuildAst.group keeps a parenthesized comparison as a single grouped operand, so {{ (0 <= inputs.n) <= 10 }} compares the grouped boolean with 10 — exactly Python's own reading of that expression.
• Validation is untouched in behavior: _visit walks every chain operand, so _collect_leaves / _evaluator_rejects still see all names and filters (as they do for and/or).

docs/workflows/expressions.md and the engine docstring now document chaining instead of left-to-right grouping.

Tests & evidence

• test_comparisons_chain_like_python — every case asserted against Python's own result for the same expression, including mixed-operator chains, a four-operand chain, and the parenthesized forms. Fails on main (e.g. 0 <= 50 <= 10 evaluates True there), passes on this branch.
• test_chained_comparison_short_circuits_like_and — a failing pair skips a later operand whose filter would raise; a reached operand still raises. Fails on main, passes here.
• test_chained_comparison_evaluates_the_shared_operand_once — pins the single evaluation of the shared operand (guards against a naive a <= b and b <= c rewrite); passes on both, kept as a contract.
• Full suite: 14,601 passed locally; the only failures are pre-existing on a clean main checkout of this macOS machine — the 11 known composed-template script-parity flakes (each passes in isolation), plus tests/specify_cli/bundles/test_references.py::test_community_step_is_not_treated_as_bundled (fails identically on clean main) and test_unknown_reference_errors_online (full-suite only; passes in isolation). None touch the expressions package.

Notes

• The branch was rebuilt on current main (the original patch targeted engine_current.py, which feat(workflows): parse workflow expressions with a Lark LALR grammar #5171 removed), hence the force-push.
• This also addresses the earlier Copilot finding about eager evaluation: operands after a failing pair are no longer evaluated.

AI disclosure

This PR was developed with Claude Code (model: Claude Fable 5), human-directed and reviewed, on behalf of @aarjang. Commits are authored by @aarjang and carry an Assisted-by trailer.

[Copilot]

🟡 Changes recommended

Chained comparisons eagerly evaluate operands after a false comparison, so an otherwise skipped JSON parse can raise ValueError and fail the workflow.

1 open issue

• Chained comparisons eagerly evaluate operands after a false comparison · New

What changed in this PR

This PR addresses #5071 by making Spec Kit workflow expressions evaluate chained comparisons pairwise, correcting range checks that could select the wrong workflow branch.

Changes:

• Scan top-level comparison operators left to right, respecting quoted strings and brackets.
• Compare adjacent operands with an implicit and.
• Add regression coverage for range checks, mixed operators, boolean precedence, and quoted operator text.

AI disclosure: GitHub Copilot (model: unknown; autonomous) reviewed the changes and suggested a fix on behalf of the requester.

File	Description
tests/​test_workflows.py	Adds chained-comparison regression cases.
src/​specify_cli/​workflows/​expressions/​engine_current.py	Adds comparison-chain scanning and pairwise evaluation.

_{Review effort: Balanced (auto)}

[mnriem]

Please address Copilot feedback

[aarjang]

Re-checked against main after #5171 landed: the bug in #5071 still reproduces on the Lark engine. With n = 50, {{ 0 <= inputs.n <= 10 }} is True — now via the documented left-to-right grouping ((0 <= 50) <= 10 → True <= 10, which _safe_compare answers True), so an if/while step still silently takes the wrong branch. The explicit form {{ 0 <= inputs.n and inputs.n <= 10 }} answers False, as expected.

This PR's diff targets the removed hand-written engine, hence the conflict. Before I rework it against the grammar, which direction do you want?

1. Chain comparisons pairwise like Python/Jinja2 (what this PR implemented): 0 <= n <= 10 means 0 <= n and n <= 10. Matches the issue's expectation and Jinja2 semantics, but changes the engine's documented "comparisons group left to right" rule.
2. Reject an unparenthesized comparison chain as a syntax error, with a message pointing at the parenthesized or and form. Keeps the documented grouping for parenthesized text, and fits the engine's fail-loudly direction — a bool compared against a number is almost never what the author meant.

I will implement whichever you prefer, with regression tests either way. The Copilot finding on the previous diff (eager operand evaluation after a decided pair) will be addressed in the rework regardless — the new engine's short-circuit style for and/or is the pattern to follow.

AI disclosure: this comment and the analysis behind it were produced with Claude Code (model: Claude Fable 5, default reasoning effort, human-supervised), posted on behalf of @aarjang, who reviewed it before posting.

[mnriem]

Let's go with option 1: pairwise comparisons with Python-style short-circuiting. Python evaluates 0 <= n <= 10 as 0 <= n and n <= 10, except that the middle operand is evaluated only once. It also stops at the first false comparison rather than evaluating all operands up front. Please preserve explicit parentheses: (0 <= n) <= 10 is a different expression, and Python itself returns True for n = 50 because True behaves numerically as 1. Please update the documented grouping rule and include regression coverage for range checks, short-circuiting, and explicit parentheses.

AI disclosure: Posted on behalf of @mnriem using GitHub Copilot (model: GPT-6.1 Sol, interactive mode, human-supervised). AI assessed Python's comparison semantics and drafted this response; @mnriem approved the wording for posting.

[Copilot]

🟡 Changes recommended

The new chain and group nodes lack diagnostic regression tests confirming that validation detects unknown names and filters in operands skipped at runtime.

2 open issues

• Chained comparisons eagerly evaluate operands after a false comparison
• Add diagnostic tests for chain and group operands · New

_{Review effort: Balanced (auto)}

[aarjang]

Reworked as option 1 in a324625 — the branch is rebuilt on current main against the Lark engine (the old patch targeted the removed engine_current.py, hence the force-push), and the PR body is updated to match.

• Semantics — a OP b OP c is now a OP b and b OP c: the shared operand is evaluated once, evaluation stops at the first false pair (so a later operand is only reached when every pair before it held — same guard behavior as and, and this also resolves the earlier eager-evaluation finding), and explicit parentheses end a chain: (0 <= n) <= 10 compares the grouped boolean, exactly as Python reads it.
• Implementation — grammar unchanged; cmp nodes flatten into one chain node (the same flattening and/or and filter chains use), group keeps a parenthesized comparison as a single operand, and validation still walks every operand.
• Evidence — test_comparisons_chain_like_python (every case asserted against Python's own result) and test_chained_comparison_short_circuits_like_and fail on main and pass here; test_chained_comparison_evaluates_the_shared_operand_once pins the single evaluation of the shared operand. Full suite green locally apart from the 11 known composed-template parity flakes on this machine (identical on clean main).

AI disclosure: This comment and the commit were drafted with Claude Code (model: Claude Fable 5), human-directed and reviewed, on behalf of @aarjang.

[Copilot]

🔵 Needs a closer look

The new chain/group static-validation branches lack contract tests covering unresolved operands, skipped invalid filters, parenthesized comparisons, and valid counterparts.

2 open issues

• Chained comparisons eagerly evaluate operands after a false comparison
• Add diagnostic tests for chain and group operands

_{Review effort: Balanced (auto)}

[mnriem]

Please address Copilot feedback