Skip to content

Commit f662854

Browse files
fix(workflows): drop the duplicate multi-argument guard
main already rejects a multi-argument filter call before the argument expression is evaluated, reporting it as an unsupported form; this branch was 53 commits behind and did not have it. The guard added in 31928a4 sat behind that check and could never fire, so its message never appeared -- which is exactly what failed in CI. Keep main's guard. Pin split(','), the filter this PR adds, against the same error, since main's coverage predates it. Assisted-by: opencode (model: mimo-v2.6-flash-free, autonomous)
1 parent f532f5c commit f662854

3 files changed

Lines changed: 17 additions & 34 deletions

File tree

‎docs/reference/workflows.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -651,7 +651,7 @@ Available filters: `default`, `join`, `contains`, `map`, `from_json`, `to_json`,
651651

652652
Notes on the newer filters:
653653

654-
- **Types are validated, not coerced.** `upper` and `lower` accept strings only, `split` requires both a string value and a non-empty string separator, and `length` accepts lists and strings but rejects mappings. Anything else raises a `ValueError` naming the problem. Coercion is deliberately not performed: a type mismatch nearly always means the workflow is wired to the wrong variable, and a coerced result would hide that. A filter given the wrong number of arguments (`| upper('x')`, `| split` with no separator, `| split(',', 1)`) is reported as a known filter misused, which is distinct from an entirely unknown filter name; arity is checked before the argument expression is evaluated, so the extra argument is what gets named. The older filters (`join`, `map`, `contains`) are more permissive and unchanged: `join` stringifies unsupported values and `map`/`contains` return fallbacks rather than raising.
654+
- **Types are validated, not coerced.** `upper` and `lower` accept strings only, `split` requires both a string value and a non-empty string separator, and `length` accepts lists and strings but rejects mappings. Anything else raises a `ValueError` naming the problem. Coercion is deliberately not performed: a type mismatch nearly always means the workflow is wired to the wrong variable, and a coerced result would hide that. A filter given the wrong number of arguments (`| upper('x')`, `| split` with no separator, `| split(',', 1)`) is reported as a known filter misused, which is distinct from an entirely unknown filter name: a call carrying more than one argument falls through to that same unsupported-form error rather than being evaluated as a single expression. The older filters (`join`, `map`, `contains`) are more permissive and unchanged: `join` stringifies unsupported values and `map`/`contains` return fallbacks rather than raising.
655655
- **`to_json` output is deterministic.** Object keys are sorted and non-ASCII characters are left as-is rather than escaped, so the same value always serializes to the same bytes. That buys reproducibility only — it does **not** make the result safe to pass through a shell, because [interpolation adds no quoting or escaping](#interpolation-and-shell-safety). Do not interpolate unconstrained JSON into a `run` field.
656656
- **`to_json` rejects non-finite floats.** `NaN`, `Infinity`, and `-Infinity` raise a `ValueError` naming `to_json` instead of serializing to bare tokens. None of the three is valid JSON, so emitting them would hand a standards-compliant downstream parser a string it must reject.
657657
- **`to_json` requires string mapping keys.** JSON objects have string keys, so a mapping with any other key type — `{1: "a"}`, `{True: "a"}` — raises a `ValueError` naming the key type instead. Left to `json.dumps`, an integer key would be coerced to `"1"` and collide with an existing `"1"` key, while mixed key types would fail `sort_keys` with an ordering `TypeError` reported only as "not JSON-serializable".

‎src/specify_cli/workflows/expressions.py‎

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -291,14 +291,6 @@ def _check_json_keys(value: Any) -> None:
291291
"to_json": _filter_to_json,
292292
}
293293

294-
# Parenthesized filters that take exactly one argument. Used to report an
295-
# extra argument by name *before* the argument expression is evaluated:
296-
# _evaluate_simple_expression has no comma operator, so ``| split(',', 1)``
297-
# would otherwise hand split an unparseable fragment, which reads back as
298-
# "expected a string separator, got NoneType" -- the arity mistake, which is
299-
# what the author actually got wrong, never surfaces.
300-
_SINGLE_ARG_FILTERS = frozenset({"default", "join", "map", "contains", "split"})
301-
302294

303295
# -- Expression resolution ------------------------------------------------
304296

@@ -682,15 +674,7 @@ def _apply_filter(value: Any, filter_expr: str, namespace: dict[str, Any]) -> An
682674
filter_match = None
683675
if filter_match:
684676
fname = filter_match.group(1)
685-
farg_text = filter_match.group(2).strip()
686-
if fname in _SINGLE_ARG_FILTERS:
687-
arg_parts = _split_top_level_commas(farg_text)
688-
if len(arg_parts) > 1:
689-
raise ValueError(
690-
f"{fname}: expected exactly one argument, got "
691-
f"{len(arg_parts)}: '| {filter_expr}'"
692-
)
693-
farg = _evaluate_simple_expression(farg_text, namespace)
677+
farg = _evaluate_simple_expression(filter_match.group(2).strip(), namespace)
694678
if fname == "default":
695679
return _filter_default(value, farg)
696680
if fname == "join":

‎tests/test_workflows.py‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -853,29 +853,28 @@ def test_filter_to_json_rejects_non_string_keys(self):
853853
== '{"1": "a", "2": "b"}'
854854
)
855855

856-
def test_single_arg_filters_reject_extra_argument(self):
857-
# Arity is checked before the argument expression is evaluated.
858-
# split(',', 1) used to evaluate that fragment first, so the reported
859-
# error was "expected a string separator, got NoneType" -- the arity
860-
# mistake, which is what the author got wrong, stayed hidden behind a
861-
# type error.
856+
def test_split_with_extra_argument_reports_unsupported_form(self):
857+
# split is new in this PR, so main's multi-argument guard has to cover
858+
# it as well. Without that it evaluated the argument fragment first and
859+
# reported "expected a string separator, got NoneType" -- the extra
860+
# argument, which is what the author actually got wrong, never
861+
# surfaced. main already pins default() and join() the same way; this
862+
# pins the filter added here.
862863
import pytest
863864
from specify_cli.workflows.expressions import evaluate_expression
864865
from specify_cli.workflows.base import StepContext
865866

866-
ctx = StepContext(inputs={"s": "a,b,c", "n": 0})
867+
ctx = StepContext(inputs={"s": "a,b,c"})
867868
with pytest.raises(
868-
ValueError, match="split: expected exactly one argument, got 2"
869+
ValueError, match="filter 'split' used in an unsupported form"
869870
):
870871
evaluate_expression("{{ inputs.s | split(',', 1) }}", ctx)
871-
with pytest.raises(
872-
ValueError, match="default: expected exactly one argument, got 2"
873-
):
874-
evaluate_expression("{{ inputs.n | default(0, 'x') }}", ctx)
875-
# A comma inside quotes or brackets is still one argument.
876-
assert (
877-
evaluate_expression("{{ inputs.s | split(',') }}", ctx) == ["a", "b", "c"]
878-
)
872+
# A comma inside quotes is still a single argument.
873+
assert evaluate_expression("{{ inputs.s | split(',') }}", ctx) == [
874+
"a",
875+
"b",
876+
"c",
877+
]
879878

880879
def test_zero_arg_filters_reject_miswired_forms(self):
881880
# The strict no-argument branch is shared by from_json/upper/lower/

0 commit comments

Comments
 (0)