Skip to content

feat(workflows): add from_json expression filter#2961

Merged
mnriem merged 4 commits into
github:mainfrom
doquanghuy:feat/2960-from-json-filter
Jun 17, 2026
Merged

feat(workflows): add from_json expression filter#2961
mnriem merged 4 commits into
github:mainfrom
doquanghuy:feat/2960-from-json-filter

Conversation

@doquanghuy

Copy link
Copy Markdown
Contributor

Description

Fixes #2960.

Adds one arg-less pipe filter to the sandboxed expression evaluator:

items: "{{ steps.emit.output.stdout | from_json }}"

_filter_from_json parses a JSON string into its typed value (list/dict/scalar). Semantics are parse-or-raise: invalid JSON or non-string input raises a clear ValueError — never a silent passthrough, since a parse failure means the pipeline wiring is wrong and silence would hide it. The module docstring's filter list is updated; no other filter or evaluator behavior is touched.

This is the smallest unblock for feeding fan-out items: (or any condition/arg) from a step that computes a collection at runtime. It composes with — and doesn't preclude — a future declared-outputs: mechanism, which I'm proposing separately.

Testing

  • Ran existing tests with uv sync && uv run pytesttests/test_workflows.py 210 passed
  • Three new tests in TestExpressions (valid JSON → typed value; invalid JSON raises; non-string raises) — all three are red against current main, green with the filter (verified both directions)
  • uvx ruff check src/ — clean
  • Tested locally with uv run specify --help
  • Tested with a sample project (covered by the evaluator tests; exercised manually with the [Bug]: fan-out silently coerces a non-list 'items' to [] — zero-instance runs with no error #2956 repro workflow rewired through | from_json)

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (describe below)

Code, tests, and this description were authored with AI assistance (Claude); verified by running the repo's test suite and ruff locally in both red and green directions.

Step outputs captured as strings could never become typed values in
templates - the filter set was default/join/map/contains only, so e.g.
a fan-out items: could never consume a step's JSON stdout. Add an
arg-less from_json pipe filter with parse-or-raise semantics: invalid
JSON or non-string input raises a clear ValueError rather than passing
through silently.

Fixes github#2960
@doquanghuy doquanghuy requested a review from mnriem as a code owner June 12, 2026 17:29
@doquanghuy

Copy link
Copy Markdown
Contributor Author

@mnriem when you have a moment, would appreciate a review — happy to adjust anything.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new from_json pipe filter to the workflow expression evaluator so step outputs (typically strings like stdout) can be converted into typed Python values (dict/list/scalar) inside templates, enabling patterns like feeding fan-out.items from a runtime-emitted JSON string.

Changes:

  • Implement from_json filter in the sandboxed expression evaluator (json.loads with strict ValueError semantics on misuse).
  • Extend filter documentation in the evaluator docstring to include from_json.
  • Add unit tests validating successful parsing and failure modes (invalid JSON, non-string input).
Show a summary per file
File Description
src/specify_cli/workflows/expressions.py Adds the from_json filter and wires it into filter dispatch.
tests/test_workflows.py Adds 3 tests covering from_json success + error cases.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 2

Comment thread src/specify_cli/workflows/expressions.py Outdated
Comment thread tests/test_workflows.py Outdated
@mnriem

mnriem commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

doquanghuy added a commit to doquanghuy/spec-kit that referenced this pull request Jun 17, 2026
Address review (github#2961): from_json('x') and from_json() previously fell through to a silent passthrough of the unparsed value. Reject any parenthesized form with a clear error so mis-wired templates fail loudly. Rename test to ...parses_object (JSON under test is an object) and add coverage for the strict no-arguments behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@doquanghuy

Copy link
Copy Markdown
Contributor Author

@mnriem Thanks for the review — addressed the Copilot feedback:

  • from_json is now strict: any parenthesized form (from_json() or from_json('x')) raises a clear error instead of silently returning the unparsed value.
  • Renamed the mis-named test to test_filter_from_json_parses_object (the JSON under test is an object), and added coverage for the strict no-arguments behavior.

Full suite green; ruff check src/ clean. Ready for another look.

Address review (github#2961): from_json('x') and from_json() previously fell through to a silent passthrough of the unparsed value. Reject any parenthesized form with a clear error so mis-wired templates fail loudly. Rename test to ...parses_object (JSON under test is an object) and add coverage for the strict no-arguments behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

Comment thread src/specify_cli/workflows/expressions.py

@mnriem mnriem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address Copilot feedback

Address Copilot review: the user-facing filter references omitted the
newly added `from_json` filter. Add it to the ARCHITECTURE.md filter table
(with the `{{ steps.emit.output.stdout | from_json }}` example) and to the
filter enumerations in workflows/README.md and docs/reference/workflows.md
so the docs match the evaluator's capabilities.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@doquanghuy

Copy link
Copy Markdown
Contributor Author

@mnriem Pushed 0eba146 addressing the latest Copilot comment:

  • Documented the new from_json filter in the workflows/ARCHITECTURE.md filter table with the suggested example {{ steps.emit.output.stdout | from_json }}.
  • Preempted the same gap in the other two filter enumerations so all user-facing docs match the evaluator: workflows/README.md and docs/reference/workflows.md.

The earlier strictness fix (parenthesized forms like from_json() / from_json('x') raise instead of silently passing through) and the test rename are unchanged.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 5/5 changed files
  • Comments generated: 2

Comment thread src/specify_cli/workflows/expressions.py
Comment thread src/specify_cli/workflows/expressions.py Outdated
@mnriem

mnriem commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

… docstring

Address Copilot review:
- Strictness only rejected parenthesized forms, so typos like
  `| from_json)` or `| from_json extra` still fell through to the
  unknown-filter path and silently returned the unparsed value. Match on
  the leading filter token and require the whole filter to be exactly
  `from_json`, so every mis-wired form raises. Extend the rejection test to
  cover the trailing-token cases.
- The module docstring claimed "no imports", which is misleading now that
  the module imports `json`. Reword to state the actual sandbox guarantee:
  templates cannot do file I/O, import modules, or run arbitrary code.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@doquanghuy

Copy link
Copy Markdown
Contributor Author

@mnriem Pushed fd896a3 for the latest Copilot round:

  • Strictness now rejects trailing tokens, not just parens. Typos like | from_json) or | from_json extra previously fell through to the unknown-filter path and silently returned the unparsed value. The check now matches the leading filter token and requires the whole filter to be exactly from_json, so every mis-wired form raises. Extended the rejection test to cover the trailing-token cases (7 forms total).
  • Docstring fixed. No file I/O, no imports was misleading after the json import; reworded to state the actual sandbox guarantee (templates cannot do file I/O, import modules, or run arbitrary code).

Full tests/test_workflows.py green (211 passed).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 5/5 changed files
  • Comments generated: 0 new

@mnriem mnriem self-requested a review June 17, 2026 18:42
@mnriem mnriem merged commit affbf5e into github:main Jun 17, 2026
11 checks passed
@mnriem

mnriem commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: a from_json expression filter so step outputs can become typed values

3 participants