Skip to content

Commit f3c1004

Browse files
committed
fix: avoid dangling enum split merges for terminal branches
Symptom: enum splits whose branches all returned could produce invalid models with an unreachable merge; grouped enum cases could also create fragile direct duplicate split flows. Root cause: the builder created the main enum merge eagerly before knowing whether any branch continued, and grouped case values pointed directly to the same first branch activity. Fix: create the main merge lazily only for continuing branches, and route grouped enum values through a branch merge before the branch body. Tests: added an all-terminal enum split regression and ran make test.
1 parent 0eedb66 commit f3c1004

2 files changed

Lines changed: 65 additions & 12 deletions

File tree

mdl/executor/cmd_microflows_builder_actions.go

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -324,14 +324,20 @@ func (fb *flowBuilder) addEnumSplit(s *ast.EnumSplitStmt) model.ID {
324324
branchWidth = HorizontalSpacing / 2
325325
}
326326
mergeX := splitX + SplitWidth + HorizontalSpacing/2 + branchWidth + HorizontalSpacing/2
327-
merge := &microflows.ExclusiveMerge{
328-
BaseMicroflowObject: microflows.BaseMicroflowObject{
329-
BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
330-
Position: model.Point{X: mergeX, Y: centerY},
331-
Size: model.Size{Width: MergeSize, Height: MergeSize},
332-
},
327+
var merge *microflows.ExclusiveMerge
328+
ensureMerge := func() *microflows.ExclusiveMerge {
329+
if merge == nil {
330+
merge = &microflows.ExclusiveMerge{
331+
BaseMicroflowObject: microflows.BaseMicroflowObject{
332+
BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
333+
Position: model.Point{X: mergeX, Y: centerY},
334+
Size: model.Size{Width: MergeSize, Height: MergeSize},
335+
},
336+
}
337+
fb.objects = append(fb.objects, merge)
338+
}
339+
return merge
333340
}
334-
fb.objects = append(fb.objects, merge)
335341

336342
savedEndsWithReturn := fb.endsWithReturn
337343
allBranchesReturn := len(branches) > 0
@@ -353,7 +359,7 @@ func (fb *flowBuilder) addEnumSplit(s *ast.EnumSplitStmt) model.ID {
353359
fb.pendingAnnotations = nil
354360
}
355361
if lastID == "" {
356-
fb.addEnumSplitFlows(splitID, actID, br.values, i)
362+
fb.addGroupedEnumSplitFlows(splitID, actID, br.values, i, splitX+SplitWidth+HorizontalSpacing/4, branchY)
357363
} else {
358364
if pendingCase != "" {
359365
fb.flows = append(fb.flows, newHorizontalFlowWithCase(lastID, actID, pendingCase))
@@ -377,12 +383,12 @@ func (fb *flowBuilder) addEnumSplit(s *ast.EnumSplitStmt) model.ID {
377383
}
378384
allBranchesReturn = false
379385
if lastID == "" {
380-
fb.addEnumSplitFlows(splitID, merge.ID, br.values, i)
386+
fb.addGroupedEnumSplitFlows(splitID, ensureMerge().ID, br.values, i, splitX+SplitWidth+HorizontalSpacing/4, branchY)
381387
} else {
382388
if pendingCase != "" {
383-
fb.flows = append(fb.flows, newHorizontalFlowWithCase(lastID, merge.ID, pendingCase))
389+
fb.flows = append(fb.flows, newHorizontalFlowWithCase(lastID, ensureMerge().ID, pendingCase))
384390
} else {
385-
fb.flows = append(fb.flows, newHorizontalFlow(lastID, merge.ID))
391+
fb.flows = append(fb.flows, newHorizontalFlow(lastID, ensureMerge().ID))
386392
}
387393
}
388394
}
@@ -393,11 +399,28 @@ func (fb *flowBuilder) addEnumSplit(s *ast.EnumSplitStmt) model.ID {
393399
if allBranchesReturn {
394400
fb.endsWithReturn = true
395401
} else {
396-
fb.nextConnectionPoint = merge.ID
402+
fb.nextConnectionPoint = ensureMerge().ID
397403
}
398404
return splitID
399405
}
400406

407+
func (fb *flowBuilder) addGroupedEnumSplitFlows(originID, destinationID model.ID, values []string, order int, mergeX, mergeY int) {
408+
if len(values) <= 1 {
409+
fb.addEnumSplitFlows(originID, destinationID, values, order)
410+
return
411+
}
412+
branchMerge := &microflows.ExclusiveMerge{
413+
BaseMicroflowObject: microflows.BaseMicroflowObject{
414+
BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
415+
Position: model.Point{X: mergeX, Y: mergeY},
416+
Size: model.Size{Width: MergeSize, Height: MergeSize},
417+
},
418+
}
419+
fb.objects = append(fb.objects, branchMerge)
420+
fb.addEnumSplitFlows(originID, branchMerge.ID, values, order)
421+
fb.flows = append(fb.flows, newHorizontalFlow(branchMerge.ID, destinationID))
422+
}
423+
401424
func (fb *flowBuilder) addEnumSplitFlows(originID, destinationID model.ID, values []string, order int) {
402425
if len(values) == 0 {
403426
flow := newHorizontalFlow(originID, destinationID)

mdl/executor/cmd_microflows_builder_enum_split_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,3 +119,33 @@ func TestEnumSplitNestedEmptyThenBranchKeepsContinuationCase(t *testing.T) {
119119
}
120120
t.Fatal("nested empty-then enum branch must carry CaseValue=true to the enum merge")
121121
}
122+
123+
func TestEnumSplitAllBranchesReturnDoesNotCreateDanglingMerge(t *testing.T) {
124+
fb := &flowBuilder{
125+
spacing: HorizontalSpacing,
126+
measurer: &layoutMeasurer{},
127+
}
128+
129+
oc := fb.buildFlowGraph([]ast.MicroflowStatement{
130+
&ast.EnumSplitStmt{
131+
Variable: "Status",
132+
Cases: []ast.EnumSplitCase{
133+
{
134+
Value: "Open",
135+
Body: []ast.MicroflowStatement{&ast.ReturnStmt{}},
136+
},
137+
{
138+
Value: "Closed",
139+
Body: []ast.MicroflowStatement{&ast.ReturnStmt{}},
140+
},
141+
},
142+
ElseBody: []ast.MicroflowStatement{&ast.ReturnStmt{}},
143+
},
144+
}, nil)
145+
146+
for _, obj := range oc.Objects {
147+
if _, ok := obj.(*microflows.ExclusiveMerge); ok {
148+
t.Fatalf("all-terminal enum split created dangling merge %#v", obj.GetID())
149+
}
150+
}
151+
}

0 commit comments

Comments
 (0)