fix: Chain iterations after nested loop expansion (gt-zn35j)
Fixes a bug where outer iteration chaining was lost when nested loops were expanded. The problem was: 1. chainLoopIterations set: outer.iter2.inner depends on outer.iter1.inner 2. ApplyLoops recursively expanded nested loops 3. outer.iter2.inner became outer.iter2.inner.iter1.work, etc. 4. The dependency was lost (referenced non-existent ID) The fix: - Move recursive ApplyLoops BEFORE chaining - Add chainExpandedIterations that finds iteration boundaries by ID prefix - Works with variable step counts per iteration (nested loops expand differently) Now outer.iter2's first step correctly depends on outer.iter1's LAST step, ensuring sequential execution of outer iterations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -12,6 +12,7 @@ package formula
|
|||||||
import (
|
import (
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"strings"
|
||||||
)
|
)
|
||||||
|
|
||||||
// ApplyLoops expands loop bodies in a formula's steps.
|
// ApplyLoops expands loop bodies in a formula's steps.
|
||||||
@@ -102,9 +103,17 @@ func expandLoop(step *Step) ([]*Step, error) {
|
|||||||
result = append(result, iterSteps...)
|
result = append(result, iterSteps...)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Chain iterations: each iteration depends on previous
|
// Recursively expand any nested loops FIRST (gt-zn35j)
|
||||||
if len(step.Loop.Body) > 0 && step.Loop.Count > 1 {
|
var err error
|
||||||
result = chainLoopIterations(result, step.Loop.Body, step.Loop.Count)
|
result, err = ApplyLoops(result)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
|
// THEN chain iterations on the expanded result
|
||||||
|
// This must happen AFTER recursive expansion so we chain the final steps
|
||||||
|
if step.Loop.Count > 1 {
|
||||||
|
result = chainExpandedIterations(result, step.ID, step.Loop.Count)
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// Conditional loop: expand once with loop metadata
|
// Conditional loop: expand once with loop metadata
|
||||||
@@ -126,11 +135,14 @@ func expandLoop(step *Step) ([]*Step, error) {
|
|||||||
firstStep.Labels = append(firstStep.Labels, fmt.Sprintf("loop:%s", string(loopJSON)))
|
firstStep.Labels = append(firstStep.Labels, fmt.Sprintf("loop:%s", string(loopJSON)))
|
||||||
}
|
}
|
||||||
|
|
||||||
result = iterSteps
|
// Recursively expand any nested loops (gt-zn35j)
|
||||||
|
result, err = ApplyLoops(iterSteps)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Recursively expand any nested loops in the result (gt-zn35j)
|
return result, nil
|
||||||
return ApplyLoops(result)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// expandLoopIteration expands a single iteration of a loop.
|
// expandLoopIteration expands a single iteration of a loop.
|
||||||
@@ -271,6 +283,46 @@ func chainLoopIterations(steps []*Step, body []*Step, count int) []*Step {
|
|||||||
return steps
|
return steps
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// chainExpandedIterations chains iterations AFTER nested loop expansion (gt-zn35j).
|
||||||
|
// Unlike chainLoopIterations, this handles variable step counts per iteration
|
||||||
|
// by finding iteration boundaries via ID prefix matching.
|
||||||
|
func chainExpandedIterations(steps []*Step, loopID string, count int) []*Step {
|
||||||
|
if len(steps) == 0 || count < 2 {
|
||||||
|
return steps
|
||||||
|
}
|
||||||
|
|
||||||
|
// Find the first and last step index of each iteration
|
||||||
|
// Iteration N has steps with ID prefix: {loopID}.iter{N}.
|
||||||
|
iterFirstIdx := make(map[int]int) // iteration -> index of first step
|
||||||
|
iterLastIdx := make(map[int]int) // iteration -> index of last step
|
||||||
|
|
||||||
|
for i, s := range steps {
|
||||||
|
for iter := 1; iter <= count; iter++ {
|
||||||
|
prefix := fmt.Sprintf("%s.iter%d.", loopID, iter)
|
||||||
|
if strings.HasPrefix(s.ID, prefix) {
|
||||||
|
if _, found := iterFirstIdx[iter]; !found {
|
||||||
|
iterFirstIdx[iter] = i
|
||||||
|
}
|
||||||
|
iterLastIdx[iter] = i
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Chain: first step of iteration N+1 depends on last step of iteration N
|
||||||
|
for iter := 2; iter <= count; iter++ {
|
||||||
|
firstIdx, hasFirst := iterFirstIdx[iter]
|
||||||
|
prevLastIdx, hasPrevLast := iterLastIdx[iter-1]
|
||||||
|
|
||||||
|
if hasFirst && hasPrevLast {
|
||||||
|
lastStepID := steps[prevLastIdx].ID
|
||||||
|
steps[firstIdx].Needs = appendUnique(steps[firstIdx].Needs, lastStepID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return steps
|
||||||
|
}
|
||||||
|
|
||||||
// ApplyBranches wires fork-join dependency patterns.
|
// ApplyBranches wires fork-join dependency patterns.
|
||||||
// For each branch rule:
|
// For each branch rule:
|
||||||
// - All branch steps depend on the 'from' step
|
// - All branch steps depend on the 'from' step
|
||||||
|
|||||||
@@ -668,3 +668,66 @@ func TestApplyLoops_ThreeLevelNesting(t *testing.T) {
|
|||||||
t.Errorf("Last step ID wrong: %s", result[7].ID)
|
t.Errorf("Last step ID wrong: %s", result[7].ID)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestApplyLoops_NestedLoopsOuterChaining(t *testing.T) {
|
||||||
|
// Verify that outer iterations are chained AFTER nested loop expansion.
|
||||||
|
// outer.iter2's first step should depend on outer.iter1's LAST step.
|
||||||
|
steps := []*Step{
|
||||||
|
{
|
||||||
|
ID: "outer",
|
||||||
|
Title: "Outer loop",
|
||||||
|
Loop: &LoopSpec{
|
||||||
|
Count: 2,
|
||||||
|
Body: []*Step{
|
||||||
|
{
|
||||||
|
ID: "inner",
|
||||||
|
Title: "Inner loop",
|
||||||
|
Loop: &LoopSpec{
|
||||||
|
Count: 2,
|
||||||
|
Body: []*Step{
|
||||||
|
{ID: "work", Title: "Do work"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
result, err := ApplyLoops(steps)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ApplyLoops failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Should have 4 steps
|
||||||
|
if len(result) != 4 {
|
||||||
|
t.Fatalf("Expected 4 steps, got %d", len(result))
|
||||||
|
}
|
||||||
|
|
||||||
|
// Expected order:
|
||||||
|
// 0: outer.iter1.inner.iter1.work
|
||||||
|
// 1: outer.iter1.inner.iter2.work (depends on above via inner chaining)
|
||||||
|
// 2: outer.iter2.inner.iter1.work (should depend on step 1 via outer chaining!)
|
||||||
|
// 3: outer.iter2.inner.iter2.work (depends on above via inner chaining)
|
||||||
|
|
||||||
|
// Verify outer chaining: step 2 should depend on step 1
|
||||||
|
step2 := result[2]
|
||||||
|
if step2.ID != "outer.iter2.inner.iter1.work" {
|
||||||
|
t.Fatalf("Step 2 ID wrong: %s", step2.ID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// This is the key assertion: outer.iter2's first step must depend on
|
||||||
|
// outer.iter1's last step (outer.iter1.inner.iter2.work)
|
||||||
|
expectedDep := "outer.iter1.inner.iter2.work"
|
||||||
|
found := false
|
||||||
|
for _, need := range step2.Needs {
|
||||||
|
if need == expectedDep {
|
||||||
|
found = true
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !found {
|
||||||
|
t.Errorf("outer.iter2 first step should depend on outer.iter1 last step.\n"+
|
||||||
|
"Expected Needs to contain %q, got %v", expectedDep, step2.Needs)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user