Repository navigation
Commit 508f7cd
authored
feat(workflows): add upper, lower, split, length, and to_json expression filters (#4766)
* feat(workflows): add upper, lower, split, length, and to_json filters
Option A of #4614: the smallest high-value batch of expression filters
(length, split, upper, lower, to_json), leaving first, last, sort,
unique, and trim deferred until real usage justifies their semantics.
Each filter validates its own input types and raises ValueError naming
the problem rather than coercing, matching the strict argument handling
already used by join/map/from_json. Coercion is rejected because a type
mismatch almost always means the workflow is wired to the wrong
variable, and a coerced result (e.g. "3" for an int) would hide that;
it would also leak a cryptic TypeError/AttributeError past the
evaluator, which the engine does not wrap in a try/except, crashing the
whole run.
- upper/lower: str -> str, empty string valid, non-str -> ValueError
- split: str -> list[str], single split(sep) form only, "" -> [""],
both value and separator must be strings
- length: list and str -> int (Jinja2 behavior), mappings rejected so
the same expression cannot mean key count for one shape and value
count for another, empty -> 0, other types -> ValueError
- to_json: any serializable value via json.dumps(..., sort_keys=True,
ensure_ascii=False), None -> "null", non-serializable -> ValueError
to_json pins sort_keys and ensure_ascii so output is byte-stable
regardless of dict insertion order or platform, which is what makes a
to_json -> shell step -> from_json round trip safe.
The strict no-argument branch is generalized into _ZERO_ARG_FILTERS so
from_json, upper, lower, length, and to_json share it; mis-wired forms
(upper('x'), length extra, to_json()) raise naming the filter instead
of falling through to the unknown-filter path.
Existing tests that used length/upper as examples of unknown filters
are retargeted at a genuinely unknown name rather than deleted, and
test_registered_filters_unaffected now asserts all ten registered
filters. Behavior is documented in workflows/README.md,
workflows/ARCHITECTURE.md, and docs/reference/workflows.md.
Note: the issue's claim that length unlocks `{% if items | length > 0 %}`
does not hold. There is no `{% if %}` syntax in the evaluator, and the
pipe binds tighter than comparisons, so a trailing comparison after any
filter raises (pre-existing design, same as `count | default(0) > 5`).
Docs instead show condition: "{{ items | length }}" (0 is falsy).
Closes #4614
Assisted-by: opencode (model: mimo-v2.6-flash-free, autonomous)
* fix(workflows): address Copilot review feedback on the new filters
Seven findings from the Copilot review of #4766, addressed in code,
tests, and docs.
Define and test empty-separator errors: `split('')` reached str.split and
raised the bare `ValueError: empty separator`, naming neither the filter
nor the expression, and no test covered it. An empty separator has no
meaning in Python, so the filter now rejects it with a filter-specific
message alongside the existing value and separator type checks.
Scope strictness claims to the new filters: the docs said "every filter"
validates its inputs, which the older filters do not — `join` stringifies
unsupported shapes and elements, and `map`/`contains` return fallbacks
rather than raising. README and ARCHITECTURE now state the contract for
the five new filters and explicitly note that the older ones are
unchanged, so authors are not promised behavior the evaluator does not
provide.
Replace shell-safety claims with reproducibility: determinism from
`sort_keys`/`ensure_ascii` was described as making a `to_json` -> shell ->
from_json round trip "safe", which contradicts the repository's shell
safety model — interpolation adds no quoting or escaping, so JSON quotes
and metacharacters are still interpreted by the shell. All three
locations now claim reproducibility only and point at the
interpolation-and-shell-safety guidance.
Remove unsupported comparison guidance: "compare it before filtering"
was not a usable alternative, because the count does not exist until
`length` runs. The docs now say plainly that `{{ items | length > 0 }}`
is unreachable and that truthiness in a `condition:` is the supported
form.
Assisted-by: opencode (model: mimo-v2.6-flash-free, autonomous)
* test(workflows): limit to_json round-trip claim to the evaluator
The test comment promised a structured value could pass through a shell
step and be recovered, which the evaluator does not guarantee:
interpolation adds no quoting, so ShellStep would hand the JSON to
shell=True unescaped. The test only exercises to_json/from_json inside
the evaluator, so say that and point at the interpolation-and-shell-safety
section instead of promising a shell round trip.
Assisted-by: opencode (model: mimo-v2.6-flash-free, autonomous)
* fix(workflows): reject non-finite floats in to_json
json.dumps defaults to allow_nan=True, so to_json emitted bare NaN,
Infinity, and -Infinity. None of those is valid JSON: the filter promised
a JSON string and handed standards-compliant parsers something they must
reject. Pass allow_nan=False so non-finite floats take the same ValueError
path as any other unserializable value, and pin all three plus a nested
case with a regression test.
Documented alongside the existing to_json notes in README, ARCHITECTURE,
and the reference.
Assisted-by: opencode (model: mimo-v2.6-flash-free, autonomous)
* fix(workflows): validate to_json keys and filter arity before evaluating
Both findings from the latest review round hid the real authoring mistake
behind a less useful error.
to_json relied on sort_keys=True for determinism, so a mapping with
mixed key types raised an ordering TypeError that the generic handler
reported as "not JSON-serializable", while a mapping with a lone integer
key was silently coerced to "1" and could collide with an existing "1"
key. JSON objects have string keys, so keys are validated before
json.dumps is reached and the message names the key type instead. The
walk is iterative with a seen set, leaving circular structures for
json.dumps to report.
split(',', 1) evaluated the argument expression first. The evaluator has
no comma operator, so the fragment evaluated to None and the reported
error was "expected a string separator, got NoneType" -- the extra
argument never surfaced. Arity for the single-argument filters is now
counted on the top-level comma split, before evaluation, so the message
names the filter and the argument count.
Documented in workflows/README.md, workflows/ARCHITECTURE.md, and
docs/reference/workflows.md, with a regression test for each behavior.
Assisted-by: opencode (model: mimo-v2.6-flash-free, autonomous)
* docs(workflows): narrow the default fallback wording
Both filter tables said "empty values" / "None/empty", which reads as
though 0, false, [], and {} fall back too. _filter_default only checks
for None and the empty string, so a zero count comes back as 0. Spell
out exactly which values fall back in both tables, and say once in the
reference that falsy is not the same as empty here.
Assisted-by: opencode (model: mimo-v2.6-flash-free, autonomous)
* 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)
* test(workflows): correct two filter test comments
The upper test said non-ASCII passes through unchanged while asserting
that "café" becomes "CAFÉ": Unicode case mapping applies and nothing
is transliterated or lost. The split test called split the inverse of
join, which is not generally true when an element contains the
separator; the assertion converts a delimiter by composing split with
join.
Both were the two low-severity findings on the current head.
Assisted-by: opencode (model: mimo-v2.6-flash-free, autonomous)1 parent ae5ade7 commit 508f7cd
7 files changed
Lines changed: 597 additions & 26 deletions
File tree
- docs/reference
- src/specify_cli/workflows
- step/switch
- tests
- unit
- workflows
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
780 | 780 | | |
781 | 781 | | |
782 | 782 | | |
783 | | - | |
| 783 | + | |
| 784 | + | |
| 785 | + | |
| 786 | + | |
| 787 | + | |
| 788 | + | |
| 789 | + | |
| 790 | + | |
| 791 | + | |
| 792 | + | |
| 793 | + | |
| 794 | + | |
| 795 | + | |
| 796 | + | |
| 797 | + | |
| 798 | + | |
| 799 | + | |
| 800 | + | |
| 801 | + | |
| 802 | + | |
| 803 | + | |
| 804 | + | |
| 805 | + | |
| 806 | + | |
| 807 | + | |
| 808 | + | |
| 809 | + | |
| 810 | + | |
784 | 811 | | |
785 | 812 | | |
786 | 813 | | |
787 | 814 | | |
788 | 815 | | |
789 | 816 | | |
790 | 817 | | |
| 818 | + | |
| 819 | + | |
791 | 820 | | |
792 | 821 | | |
793 | 822 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
| 12 | + | |
12 | 13 | | |
13 | 14 | | |
14 | 15 | | |
| |||
23 | 24 | | |
24 | 25 | | |
25 | 26 | | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
26 | 32 | | |
27 | 33 | | |
28 | 34 | | |
| |||
130 | 136 | | |
131 | 137 | | |
132 | 138 | | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
133 | 295 | | |
134 | 296 | | |
135 | 297 | | |
| |||
464 | 626 | | |
465 | 627 | | |
466 | 628 | | |
467 | | - | |
468 | | - | |
469 | | - | |
470 | | - | |
471 | | - | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
472 | 635 | | |
473 | | - | |
474 | | - | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
475 | 639 | | |
476 | | - | |
| 640 | + | |
477 | 641 | | |
478 | 642 | | |
479 | | - | |
| 643 | + | |
480 | 644 | | |
481 | 645 | | |
482 | 646 | | |
| |||
519 | 683 | | |
520 | 684 | | |
521 | 685 | | |
| 686 | + | |
| 687 | + | |
522 | 688 | | |
523 | 689 | | |
524 | 690 | | |
| |||
530 | 696 | | |
531 | 697 | | |
532 | 698 | | |
533 | | - | |
| 699 | + | |
| 700 | + | |
534 | 701 | | |
535 | 702 | | |
536 | 703 | | |
| |||
620 | 787 | | |
621 | 788 | | |
622 | 789 | | |
623 | | - | |
| 790 | + | |
624 | 791 | | |
625 | 792 | | |
626 | 793 | | |
| |||
923 | 1090 | | |
924 | 1091 | | |
925 | 1092 | | |
926 | | - | |
927 | | - | |
| 1093 | + | |
| 1094 | + | |
928 | 1095 | | |
929 | 1096 | | |
930 | 1097 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
29 | 29 | | |
30 | 30 | | |
31 | 31 | | |
32 | | - | |
33 | | - | |
| 32 | + | |
| 33 | + | |
34 | 34 | | |
35 | 35 | | |
36 | 36 | | |
| |||
0 commit comments