fix(workflows): list-literal expression ignores trailing/empty commas#3631
Merged
mnriem merged 1 commit intoJul 22, 2026
Merged
Conversation
A workflow list-literal expression with a trailing (or leading/double) comma —
'{{ [1, 2,] }}' — evaluated to [1, 2, None]: _split_top_level_commas returns a
trailing empty segment, which _evaluate_simple_expression resolves as an empty
dot-path to None. That silently widens membership tests and renders a stray
None in joins. Python and Jinja2 both tolerate trailing commas.
Drop whitespace-empty segments from the comprehension. An intentional
empty-string element ('') survives because its segment strips to "''" (truthy),
so ['', 'a'] is preserved. Completes the quoted-comma handling from github#3134.
Test: [1, 2,] and [1,, 2] -> [1, 2]; ['', 'a'] -> ['', 'a'] (fails before:
trailing None).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes list-literal parsing so empty comma segments no longer produce spurious None elements.
Changes:
- Filters whitespace-empty list segments.
- Preserves explicit empty-string elements.
- Adds regression coverage for trailing and repeated commas.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/workflows/expressions.py |
Ignores empty list-literal segments. |
tests/test_workflows.py |
Adds list parsing regression tests. |
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: Medium
Collaborator
|
Thank you! |
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.
What
A workflow list-literal expression with a trailing comma evaluates to a list with a spurious
None:_split_top_level_commas("1, 2,")returns['1', ' 2', '']; the empty segment flows through_evaluate_simple_expression(''), which resolves as an empty dot-path toNone. Leading/double commas hit the same path. This silently widens membership tests ({{ x in [a, b,] }}) and renders a strayNonein joins. Python and Jinja2 both tolerate trailing commas.Fix
Drop whitespace-empty segments from the comprehension (
if i.strip()). An intentional empty-string element is preserved, because its segment strips to"''"(truthy) —['', 'a']still yields['', 'a']. This completes the quoted-comma handling added in #3134.Test
test_list_literal_ignores_trailing_and_empty_commas:[1, 2,]and[1,, 2]→[1, 2];['', 'a']→['', 'a']— fails before (trailingNone), passes after.🤖 Written with the assistance of Claude Code (AI). Bug self-found; fix/tests verified locally (fail-before / pass-after), ruff clean.