fix(workflows): apply chained expression filters left-to-right#3339
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes workflow expression evaluation so chained pipe filters are parsed and applied left-to-right, enabling canonical workflows like map(...) | join(...) without raising parsing errors.
Changes:
- Add
_split_top_level()to split filter chains on top-level|while respecting quotes/brackets. - Extract single-filter parsing/execution into
_apply_filter()and fold it across the chain left-to-right. - Add regression tests covering multi-link chains, later-link failures, and literal
|inside quoted operands/arguments.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/workflows/expressions.py |
Implements top-level pipe splitting and left-to-right filter folding via _split_top_level() + _apply_filter(). |
tests/test_workflows.py |
Adds tests validating correct chained-filter behavior and correct handling of ` |
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: 0
- Review effort level: Low
mnriem
requested changes
Jul 6, 2026
mnriem
left a comment
Collaborator
There was a problem hiding this comment.
Please address test & lint errors
The pipe-filter parser in `_evaluate_simple_expression` split the
expression only at the *first* top-level `|` and treated the whole
remainder as a single filter. So a filter chain like
`{{ inputs.rows | map('name') | join(', ') }}` handed
`map('name') | join(', ')` to one filter, where the `(\w+)\((.+)\)`
regex mangled it and raised `ValueError`.
This broke the canonical use of `map`: it returns a list, and `join`
is the only filter that renders a list to a string, so the two are
meant to be chained. Chaining was impossible for every registered
filter.
Split the pipe segments at the top level (quote/bracket aware, so a
literal `|` inside a quoted argument like `join(' | ')` is preserved)
and fold each filter over the running value. The single-filter logic
is extracted verbatim into `_apply_filter`, so all existing strict
handling (`from_json` arity, unsupported-form vs unknown-filter
messages) is unchanged and now applies to every link in the chain.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
853b7e9 to
636a472
Compare
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
mnriem
approved these changes
Jul 9, 2026
Collaborator
|
Thank you! |
kanfil
added a commit
to tikalk/agentic-sdlc-spec-kit
that referenced
this pull request
Jul 11, 2026
Upstream merge (30 commits, 3 releases 0.12.9-0.12.11): - invoke_separator parse-success fix (github#3304) - Windows Store python3 stub skip + _interpreter_runs() probe (github#3385) - SKILL.md frontmatter control char escape via yaml_quote() (github#3399) - chained expression filters left-to-right refactor (github#3339) - refresh_shared_templates preserves recovered files (github#3378) - Goose yaml skill placeholder resolution (github#3374) - bundled version pin enforcement (github#3377) - integration test home isolation (github#3144) - py: script type in command templates (github#3403) - configurable shell step timeout (github#3404) - find plans in nested spec directories (github#3405) - plan.md phase numbering fix (github#3416) - PowerShell -Number 0 honor via ContainsKey (github#3412) - workflow.yml non-string scalar validation (github#3421) - plan-template.md self-referencing path fix (github#3417) - pre-commit config + trailing whitespace cleanup (github#3430) - malformed URL error handling (github#3433/github#3435/github#3437) - agent-context nested plan.md discovery (github#3301) - community catalog additions (EARS, Figma) (github#3407/github#3408) 9 conflicts resolved: pyproject.toml, integrations/base.py, agents.py, forge/__init__.py, hermes/__init__.py, create-new-feature-branch.ps1, test_git_extension.py, test_base.py, test_integration_devin.py. Template-to-preset alignment: added py: script lines to 6 preset commands, removed stale Phase 1 agent context line from plan preset, fixed phase numbering. Pre-merge fix: wrapped bare make_typer import with fallback. Assisted-by: opencode (model: glm-5.2, autonomous)
kanfil
added a commit
to tikalk/agentic-sdlc-spec-kit
that referenced
this pull request
Jul 11, 2026
Upstream merge (30 commits, 3 releases 0.12.9-0.12.11): - invoke_separator parse-success fix (github#3304) - Windows Store python3 stub skip + _interpreter_runs() probe (github#3385) - SKILL.md frontmatter control char escape via yaml_quote() (github#3399) - chained expression filters left-to-right refactor (github#3339) - refresh_shared_templates preserves recovered files (github#3378) - Goose yaml skill placeholder resolution (github#3374) - bundled version pin enforcement (github#3377) - integration test home isolation (github#3144) - py: script type in command templates (github#3403) - configurable shell step timeout (github#3404) - find plans in nested spec directories (github#3405) - plan.md phase numbering fix (github#3416) - PowerShell -Number 0 honor via ContainsKey (github#3412) - workflow.yml non-string scalar validation (github#3421) - plan-template.md self-referencing path fix (github#3417) - pre-commit config + trailing whitespace cleanup (github#3430) - malformed URL error handling (github#3433/github#3435/github#3437) - agent-context nested plan.md discovery (github#3301) - community catalog additions (EARS, Figma) (github#3407/github#3408) 9 conflicts resolved: pyproject.toml, integrations/base.py, agents.py, forge/__init__.py, hermes/__init__.py, create-new-feature-branch.ps1, test_git_extension.py, test_base.py, test_integration_devin.py. Template-to-preset alignment: added py: script lines to 6 preset commands, removed stale Phase 1 agent context line from plan preset, fixed phase numbering. Pre-merge fix: wrapped bare make_typer import with fallback. Assisted-by: opencode (model: glm-5.2, autonomous)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Filter chaining is broken in the workflow expression evaluator.
_evaluate_simple_expressionsplits at the first top-level|and treats the entire remainder as one filter, so any expression with more than one filter raisesValueError.This makes the canonical
mapuse case impossible:mapreturns a list, andjoinis the only filter that renders a list to a string — the two are meant to be chained.A single filter (
| map('name')or| join(', ')alone) works, so chaining is the only broken axis — but it affects every registered filter.Root cause
For
map('name') | join(', '),filter_exprbecomes the full string and the greedy(.+)argument capture spans the second filter, so parsing fails.Fix
|inside a quoted argument such asjoin(' | ')or an operand like'a|b'is not treated as a separator)._apply_filterhelper, so all existing strict handling is preserved and now applies to every link in the chain:from_jsonarity/trailing-token strictnessTests
Added to
TestExpressions:test_chained_filters_apply_left_to_right—map → join, a three-linkmap → join → contains, anddefault → contains.test_chained_filter_error_in_later_link_raises— a mis-wired filter anywhere in the chain still fails loudly.test_pipe_in_quoted_arg_is_not_a_filter_separator— literal|inside quotes stays intact.Test-the-test verified: the two chaining tests fail without the source change and pass with it. Full
TestExpressions(37 tests) passes; the only failures intests/test_workflows.pyare the pre-existing Windows symlink-guard tests (unrelated, require elevation).🤖 Generated with Claude Code