Skip to content

Complete loops on steps add, and remove empty loops with their closer - #98

Closed
jbedient-kizen wants to merge 2 commits into
feat/author-loops-from-a-specfrom
feat/steps-add-completes-loops
Closed

jbedient-kizen wants to merge 2 commits into
feat/author-loops-from-a-specfrom
feat/steps-add-completes-loops

Conversation

@jbedient-kizen

Copy link
Copy Markdown
Contributor

Stacked on #97; merge #96 and #97 first, then retarget this to main.

Problem

Kizen's builder creates a loop's next_or_finish and its item variable together with the loop, but steps add refused a loop, so a one-step edit had to go through a whole-automation spec. And removing a loop needed --cascade, which also deleted every step after it.

Solution

steps add of a loop inserts the loop, its closer and its item variable in one plan. The item variable goes at the end of the variable steps, named <array>_item (then _item_2, _3 when taken), the name the UI gives. It's always created, never chosen or reused, so a steps add spec that sets step_loop.item_variable is refused. The array variable must already exist. The parent's existing children go after the closer, so they still run once, after the loop, as in the UI. A wrong type, nesting past three levels and one array at two levels are refused by the graph checks every steps verb already runs.

steps remove of a loop with an empty body removes its closer with it and moves the steps after it up. A loop with a body is still refused.

Testing

Local checks after merging main: lint, format, typecheck, pytest (2981 passed, 4 skipped, 1 xfailed) and the CLI-tree diff, passed: lint format typecheck test extra_checks.

Live on a test tenant, on a scratch automation: steps add --dry-run of a loop reported the loop, closer, item variable arr_item and the adopted child; applied, show had them in the right places and roundtrip --execute → drift: []. With a body step added, an activated run completed three body passes and then the step after the loop. A second loop over the same array got arr_item_2. steps remove of the empty second loop removed it and its closer and reparented the next step; removing the first loop with a body was refused, and after its body step went, removing it worked. Each step ended with drift: []. Drift tier: 109 passed, 4 failed, all also failing on main (schema snapshot predates the 2026-10-26 release).

Design notes / tradeoffs

build_wire_loop and insert_loop sit beside build_wire_step and insert_step. The spec path (create/update --spec-file) stays explicit, for the diff reason in #97. Rejected: keeping the refusal, which sends a one-step edit through a whole-automation spec. The fiddly part is placing the item variable last among the variable steps while keeping variables first, including when the loop's own parent is the last variable step; tests cover that case.

Overlaps: as #96 and #97, plus tools/automations.py, tools/steps.py and docs/specs/automation-step.md.

Kizen's builder creates a loop's next_or_finish and its item variable
together, but 'steps add' refused a loop and sent a one-step edit
through a whole-automation spec. It now adds all three: the item
variable is always created, named '<array>_item', and the parent's
children follow the closer so they still run once, after the loop.

Removing a loop needed --cascade, which also deleted every step after
it. A loop with an empty body now goes with its closer and the steps
after it move up.
@jbedient-kizen

Copy link
Copy Markdown
Contributor Author

Included in the 0.4.0 release PR #102; merge that instead.

@sangyeT sangyeT 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.

Reviewed build_wire_loop / insert_loop (closer and _item variable created together, item step placed after the last initialize_variable, parent's children follow the closer) and the empty-loop removal that takes the closer with it. The undeclared-array case correctly yields no item and fails validation rather than guessing. Approving.

@jbedient-kizen

Copy link
Copy Markdown
Contributor Author

Merged to main through the 0.4.0 release PR #102 (39beea1), released as v0.4.0. This PR's head is on main; GitHub couldn't mark it merged because its base was the stacked branch.

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.

2 participants