mirror of
https://github.com/github/spec-kit.git
synced 2026-07-07 06:35:06 +08:00
address review: execute loop body with original IDs
Instead of namespacing step IDs for execution and copying results back, execute the loop body with original (unprefixed) step IDs so results naturally land at the right keys. Snapshot previous iteration results to namespaced keys (parent:child:N) for history only. This fixes multi-step loop bodies where step B references step A's output within the same iteration — previously step B would see stale data until the copy-back ran after the entire iteration.
This commit is contained in:
@@ -672,27 +672,22 @@ class WorkflowEngine:
|
||||
for _loop_iter in range(max_iters - 1):
|
||||
if not evaluate_condition(condition, context):
|
||||
break
|
||||
# Snapshot iteration-0 results under a
|
||||
# namespaced key before the first overwrite.
|
||||
if _loop_iter == 0:
|
||||
for ns in result.next_steps:
|
||||
orig = ns.get("id")
|
||||
if orig and orig in context.steps:
|
||||
ns_key = f"{step_id}:{orig}:0"
|
||||
context.steps[ns_key] = context.steps[orig]
|
||||
state.step_results[ns_key] = context.steps[orig]
|
||||
# Namespace nested step IDs per iteration
|
||||
iter_steps = []
|
||||
original_ids = {}
|
||||
# Snapshot current results under namespaced
|
||||
# keys for per-iteration history before they
|
||||
# are overwritten by the next iteration.
|
||||
for ns in result.next_steps:
|
||||
ns_copy = dict(ns)
|
||||
if "id" in ns_copy:
|
||||
orig = ns_copy["id"]
|
||||
ns_copy["id"] = f"{step_id}:{orig}:{_loop_iter + 1}"
|
||||
original_ids[ns_copy["id"]] = orig
|
||||
iter_steps.append(ns_copy)
|
||||
orig = ns.get("id")
|
||||
if orig and orig in context.steps:
|
||||
ns_key = f"{step_id}:{orig}:{_loop_iter}"
|
||||
context.steps[ns_key] = context.steps[orig]
|
||||
state.step_results[ns_key] = context.steps[orig]
|
||||
# Execute body with original step IDs so
|
||||
# results land at the unprefixed keys. Both
|
||||
# inter-step references within the body and
|
||||
# the loop condition naturally see the latest
|
||||
# values without a copy-back.
|
||||
self._execute_steps(
|
||||
iter_steps, context, state, registry,
|
||||
result.next_steps, context, state, registry,
|
||||
step_offset=-1,
|
||||
)
|
||||
if state.status in (
|
||||
@@ -701,15 +696,6 @@ class WorkflowEngine:
|
||||
RunStatus.ABORTED,
|
||||
):
|
||||
return
|
||||
# Copy namespaced results back to unprefixed
|
||||
# keys so loop conditions see updated values.
|
||||
# Only after a fully completed iteration —
|
||||
# partial results from paused/failed steps
|
||||
# must not overwrite the unprefixed key.
|
||||
for namespaced, orig in original_ids.items():
|
||||
if namespaced in context.steps:
|
||||
context.steps[orig] = context.steps[namespaced]
|
||||
state.step_results[orig] = context.steps[namespaced]
|
||||
|
||||
# Fan-out: execute nested step template per item with unique IDs
|
||||
if step_type == "fan-out":
|
||||
|
||||
@@ -1943,10 +1943,9 @@ steps:
|
||||
assert state.status == RunStatus.COMPLETED
|
||||
# The unprefixed key should reflect the latest iteration's result.
|
||||
assert state.step_results["attempt"]["output"]["stdout"] == "done"
|
||||
# Namespaced iteration-1 result should also exist.
|
||||
assert "retry-loop:attempt:1" in state.step_results
|
||||
# Iteration-0 history preserved under namespaced key.
|
||||
assert "retry-loop:attempt:0" in state.step_results
|
||||
assert state.step_results["retry-loop:attempt:0"]["output"]["stdout"] != "done"
|
||||
# Counter should be 2 (iteration 0 + iteration 1), not 5.
|
||||
assert counter_file.read_text(encoding="utf-8").strip() == "2"
|
||||
|
||||
@@ -2042,10 +2041,9 @@ steps:
|
||||
assert counter_file.read_text(encoding="utf-8").strip() == "3"
|
||||
# Unprefixed key holds the last iteration's result.
|
||||
assert state.step_results["tick"]["output"]["stdout"] == "pending"
|
||||
# Namespaced keys for all iterations exist.
|
||||
# Namespaced history keys for previous iterations exist.
|
||||
assert "retry-loop:tick:0" in state.step_results
|
||||
assert "retry-loop:tick:1" in state.step_results
|
||||
assert "retry-loop:tick:2" in state.step_results
|
||||
|
||||
def test_do_while_loop_runs_to_max_when_condition_stays_true(self, project_dir):
|
||||
"""Do-while loop must still run to max_iterations when the condition
|
||||
|
||||
Reference in New Issue
Block a user