Skip to content

Commit a40bbcb

Browse files
fix(workflows): resolve negative list indices in expressions (#4416)
* fix(workflows): resolve negative list indices in expressions _resolve_dot_path matched only digits in the index bracket, so `task_list[-1]` never entered the indexing branch. It fell through to the dict lookup and asked for the literal key "task_list[-1]", which returns None — a template reaching for the last element of a step output rendered empty with no error, and a condition on it silently read false. Accept the negative form Python and Jinja2 both use, and bound the index from both ends so out-of-range still yields None rather than raising. * test(workflows): keep the shared-index gate test on a shape the regex still rejects
1 parent 8d3f64c commit a40bbcb

3 files changed

Lines changed: 31 additions & 10 deletions

File tree

‎src/specify_cli/workflows/expressions.py‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,28 +139,29 @@ def _filter_from_json(value: Any) -> Any:
139139
# against it, and the condition gate below reuses it rather than describing the
140140
# same shape a second time, so widening what indexing accepts cannot leave the
141141
# evaluator and the gate disagreeing.
142-
_INDEXED_SEGMENT = re.compile(r"^([\w-]+)\[(\d+)\]$")
142+
_INDEXED_SEGMENT = re.compile(r"^([\w-]+)\[(-?\d+)\]$")
143143

144144
_PLAIN_SEGMENT = re.compile(r"^[\w-]+$")
145145

146146

147147
def _resolve_dot_path(obj: Any, path: str) -> Any:
148148
"""Resolve a dotted path like ``steps.specify.output.file`` against *obj*.
149149
150-
Supports dict key access and list indexing (e.g., ``task_list[0]``).
150+
Supports dict key access and list indexing, including the negative form
151+
Python and Jinja2 both accept (e.g., ``task_list[0]``, ``task_list[-1]``).
151152
"""
152153
parts = path.split(".")
153154
current = obj
154155
for part in parts:
155-
# Handle list indexing: name[0]
156+
# Handle list indexing: name[0], name[-1]
156157
idx_match = _INDEXED_SEGMENT.match(part)
157158
if idx_match:
158159
key, idx = idx_match.group(1), int(idx_match.group(2))
159160
if isinstance(current, dict):
160161
current = current.get(key)
161162
else:
162163
return None
163-
if isinstance(current, list) and 0 <= idx < len(current):
164+
if isinstance(current, list) and -len(current) <= idx < len(current):
164165
current = current[idx]
165166
else:
166167
return None

‎tests/test_workflows.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,6 +1013,25 @@ def test_list_indexing(self):
10131013
result = evaluate_expression("{{ steps.tasks.output.task_list[0].file }}", ctx)
10141014
assert result == "a.md"
10151015

1016+
def test_negative_list_indexing(self):
1017+
"""``list[-1]`` resolves from the end, as Python and Jinja2 both do.
1018+
1019+
Without it the index silently fell through to a dict lookup for the
1020+
literal key ``"task_list[-1]"`` and produced ``None``, so a template
1021+
reaching for the last element rendered empty with no error.
1022+
"""
1023+
from specify_cli.workflows.expressions import evaluate_expression
1024+
from specify_cli.workflows.base import StepContext
1025+
1026+
ctx = StepContext(
1027+
steps={"tasks": {"output": {"task_list": [{"file": "a.md"}, {"file": "b.md"}]}}}
1028+
)
1029+
assert evaluate_expression("{{ steps.tasks.output.task_list[-1].file }}", ctx) == "b.md"
1030+
assert evaluate_expression("{{ steps.tasks.output.task_list[-2].file }}", ctx) == "a.md"
1031+
# Out of range in either direction stays None rather than raising.
1032+
assert evaluate_expression("{{ steps.tasks.output.task_list[-3] }}", ctx) is None
1033+
assert evaluate_expression("{{ steps.tasks.output.task_list[2] }}", ctx) is None
1034+
10161035
def test_context_run_id_resolves(self):
10171036
"""``{{ context.run_id }}`` resolves to ``StepContext.run_id``.
10181037

‎tests/unit/test_condition_expression_block.py‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -946,16 +946,17 @@ def test_a_literal_never_reaches_the_resolver():
946946
def test_gate_reads_the_shared_indexed_segment_definition(monkeypatch):
947947
"""Widening `_INDEXED_SEGMENT` alone must reach the gate.
948948
949-
`steps.…​.task_list[-1]` is rejected today because `_INDEXED_SEGMENT` — the
950-
one place `_resolve_dot_path` says what an index looks like — accepts digits
951-
only. Widening it there and nowhere else must be enough; if the gate keeps
952-
its own copy of the shape (as `_PATH_SEGMENT` used to), this fails.
949+
`steps.…​.task_list[+1]` is rejected today because `_INDEXED_SEGMENT` — the
950+
one place `_resolve_dot_path` says what an index looks like — accepts an
951+
optional minus sign and digits only. Widening it there and nowhere else must
952+
be enough; if the gate keeps its own copy of the shape (as `_PATH_SEGMENT`
953+
used to), this fails.
953954
"""
954-
path = "steps.tasks.output.task_list[-1].file"
955+
path = "steps.tasks.output.task_list[+1].file"
955956
assert expressions._unresolvable_term(path) is not None
956957

957958
monkeypatch.setattr(
958-
expressions, "_INDEXED_SEGMENT", re.compile(r"^([\w-]+)\[(-?\d+)\]$")
959+
expressions, "_INDEXED_SEGMENT", re.compile(r"^([\w-]+)\[([+-]?\d+)\]$")
959960
)
960961
assert expressions._unresolvable_term(path) is None
961962

0 commit comments

Comments
 (0)