Skip to content

Commit 8c6ed22

Browse files
MarkusCopilot
andcommitted
fix(workflows): name the operator in filter-precedence errors and correct validation docs
The operator-first filter-precedence error now names the offending operator and suggests parenthesizing the filtered operand, matching the chain-first message. ARCHITECTURE.md no longer claims condition expressions are syntax-checked at validation time; validators check only delimiters and layout. Assisted-by: GitHub Copilot (model: Claude Sonnet 5.5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3999261 commit 8c6ed22

3 files changed

Lines changed: 9 additions & 8 deletions

File tree

‎src/specify_cli/workflows/expressions/engine_lark.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -301,13 +301,13 @@ def _parse_error_message(
301301
character and column.
302302
"""
303303
if isinstance(exc, _FilterPrecedenceError):
304+
operator = " ".join(str(exc.operator).split())
304305
if not exc.chain_first:
305306
return (
306307
f"ambiguous filter precedence in '{text}': a filter cannot be an "
307-
f"operand of an operator. Filter the operand in its own expression "
308-
f"instead."
308+
f"operand of '{operator}'; parenthesize the filtered operand, "
309+
f"as in '(inputs.limit | default(5))'"
309310
)
310-
operator = " ".join(str(exc.operator).split())
311311
return (
312312
f"ambiguous filter precedence in '{text}': '{operator}' follows a "
313313
f"filter chain, and a filter chain cannot be an operand of an "

‎tests/unit/test_condition_expression_block.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -605,9 +605,10 @@ def test_a_condition_without_a_filter_is_not_told_to_parenthesize_one(step_cls):
605605
("(inputs.items | length > 0)", "'>' follows a filter chain"),
606606
("[inputs.items | length and inputs.ok]", "'and' follows a filter chain"),
607607
("inputs.x | default(inputs.items | length > 0)", "'>' follows a filter chain"),
608-
("(inputs.n > inputs.limit | default(5))", "a filter cannot be an operand"),
609-
("[1, not inputs.x | default(1)]", "a filter cannot be an operand"),
610-
("inputs.a == not inputs.b | default(1)", "a filter cannot be an operand"),
608+
("(inputs.n > inputs.limit | default(5))", "an operand of '>'"),
609+
("[1, not inputs.x | default(1)]", "an operand of 'not'"),
610+
("inputs.a == not inputs.b | default(1)", "an operand of 'not'"),
611+
("inputs.count > inputs.limit | default(5)", "an operand of '>'; parenthesize"),
611612
# Two misplaced chains: the leftmost operator is named.
612613
(
613614
"inputs.items | length == inputs.a and inputs.b > inputs.c | length",

‎workflows/ARCHITECTURE.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,15 +208,15 @@ Workflow definitions use Jinja2-like `{{ expression }}` syntax for dynamic value
208208

209209
**Operator precedence.** From loosest to tightest: `or`, `and`, `not`, then the comparisons (`==`, `!=`, `>`, `<`, `>=`, `<=`, `in`, `not in`). All comparisons share **one** level and group left to right, so no comparison has priority over another: `{{ a == b > 0 }}` is `{{ (a == b) > 0 }}`, and `{{ 1 < 2 < 3 }}` is `{{ (1 < 2) < 3 }}`. The former hand-written engine gave comparisons accidental priorities through its operator scan order; parenthesize, or join comparisons with `and`/`or`, instead of relying on either reading. A `not` on the right of a comparison negates only its operand: `{{ a == not b }}`.
210210

211-
**Only expressions parse.** The body of every `{{ }}` block is parsed by a grammar, so text that is not an expression is a `ValueError` naming the offending token and column: prose such as `{{ some text }}`, path keys with characters outside letters, digits, `_` and `-` (`{{ inputs.weird:key }}`), arithmetic, or two adjacent operands. The former engine looked such text up as a path and rendered the miss as an empty string or `None`. Conditions are checked at validation time; other fields fail when the step runs.
211+
**Only expressions parse.** The body of every `{{ }}` block is parsed by a grammar, so text that is not an expression is a `ValueError` naming the offending token and column: prose such as `{{ some text }}`, path keys with characters outside letters, digits, `_` and `-` (`{{ inputs.weird:key }}`), arithmetic, or two adjacent operands. The former engine looked such text up as a path and rendered the miss as an empty string or `None`. Validation of `if`/`while`/`do-while` conditions checks only the `{{ }}` delimiters and layout, not the expression inside: `{{ inputs.count + 1 }}` passes validation and fails with the `ValueError` when the step runs, as do all other fields.
212212

213213
**Filter argument strictness (new filters).** The five filters added in #4766 validate their supported inputs and raise `ValueError` naming the problem rather than leaking a Python `TypeError`/`AttributeError`: `upper`/`lower` accept only strings, `split` requires a string value and a non-empty string separator, `length` accepts only lists and strings, and `to_json` requires a JSON-serializable value whose mapping keys are strings, and additionally rejects non-finite floats (`NaN`, `Infinity`, `-Infinity`), which `json.dumps` would otherwise emit as bare tokens that are not valid JSON. Keys are checked before serialization because `json.dumps` would otherwise coerce `1` to `"1"` (colliding with an existing `"1"` key), while `sort_keys=True` raises an ordering `TypeError` on mixed key types — both surfacing as a generic "not JSON-serializable" that hides the authoring mistake. Coercion is deliberately rejected for these filters: a type mismatch almost always means the pipeline is wired to the wrong variable, and a coerced result (e.g. `"3"` for an int) would hide that. Arity is checked before the argument expression is evaluated, so a filter used with the wrong number of arguments — `| upper('x')`, `| split` with no separator, `| split(',', 1)`, `| join` bare — is reported as a known filter misused, distinct from an entirely unknown name.
214214

215215
The older filters are more permissive and are unchanged: `join` stringifies unsupported value shapes and elements, and `map`/`contains` return fallback values for unsupported inputs rather than raising.
216216

217217
**`to_json` output is deterministic.** It pins `sort_keys=True` and `ensure_ascii=False`, so the same value always serializes to the same bytes regardless of dict insertion order or platform, and non-ASCII text is not `\uXXXX`-escaped. This buys reproducibility only — it does **not** make the result safe to pass through a shell, because expression interpolation adds no quoting or escaping. See [Interpolation and shell safety](../docs/reference/workflows.md#interpolation-and-shell-safety) before interpolating JSON into a `run` field.
218218

219-
**Filters and comparisons.** A filter chain cannot be an operand of an operator, so an unparenthesized trailing comparison such as `{{ inputs.items | length > 0 }}` raises an "ambiguous filter precedence" `ValueError`, as does a filter on an operator's operand (`{{ count > limit | default(5) }}`). Parenthesize the filtered operand to compare its result: `{{ (inputs.items | length) > 0 }}`. For a threshold of at least two:
219+
**Filters and comparisons.** A filter chain cannot be an operand of an operator, so an unparenthesized trailing comparison such as `{{ inputs.items | length > 0 }}` raises an "ambiguous filter precedence" `ValueError`, as does a filter on an operator's operand (`{{ count > limit | default(5) }}`); the message names the operator. Parenthesize the filtered operand to compare its result: `{{ (inputs.items | length) > 0 }}`, `{{ count > (limit | default(5)) }}`. For a threshold of at least two:
220220

221221
```yaml
222222
condition: "{{ (inputs.items | length) >= 2 }}"

0 commit comments

Comments
 (0)