fix: fail loudly when a fan-out 'items' expression does not resolve to a list#2957
Open
doquanghuy wants to merge 1 commit into
Open
fix: fail loudly when a fan-out 'items' expression does not resolve to a list#2957doquanghuy wants to merge 1 commit into
doquanghuy wants to merge 1 commit into
Conversation
Contributor
Author
|
@mnriem when you have a moment, would appreciate a review — happy to adjust anything. |
…o a list A non-list result from the items expression is a wiring error (the template did not resolve to a collection); silently fanning out over zero items hides it until a confusing downstream failure. Fail the step with an error naming the expression instead. An explicit empty list remains valid input. Fixes github#2956
485c977 to
317d347
Compare
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.
Description
Fixes #2956.
FanOutStep.executesilently coerced any non-listitemsresult to[], so a mis-wired or unresolvableitems:expression produced a vacuous zero-instance run with no error anywhere near the cause. This PR makes the step fail loudly instead, with an error naming the expression and the resolved type. An explicit empty list remains valid input (legitimately-empty data still completes withitem_count: 0).The previous behavior was pinned by
test_execute_non_list_items_resolves_empty; that test is updated to the fail-loud contract, and a newtest_execute_empty_list_items_is_validlocks the empty-list-stays-valid distinction. If silent-empty was intentional for some use case, happy to rework this as an opt-in knob (allow_empty:/strict:) with fail-loud as default — see the issue for that alternative.The failure output keeps the step's four output keys (
items/max_concurrency/step_template/item_count) so downstream readers of a failed run's state don't hit missing keys.Testing
uv sync && uv run pytest— 208 passed (the new non-list test fails red against currentmain, passes with the fix; verified both directions)uvx ruff check src/— cleanuv run specify --helpAI Disclosure
Code, tests, and this description were authored with AI assistance (Claude), driven by a real-pipeline failure investigation; everything was verified by running the repo's test suite and ruff locally in both red (unfixed) and green (fixed) directions.