Skip to content

Commit 6bb1846

Browse files
hjothamendixhjothamendix
committed
fix: preserve anchors from no-merge branch tails
Symptom: a nested IF whose continuing branch skips a local merge could lose its @anchor metadata when the parent IF connected that tail to its merge or next statement. In affected models this changed branch layout and could make Studio Pro report variables as out of scope after round-trip. Root cause: addIfStatement only consumed pending branch metadata when nextFlowCase was non-empty. No-merge continuations can carry only nextFlowAnchor, so those anchors were ignored. Fix: treat pending anchors as pending flow metadata even without a case value, and route their origin/destination sides through shared helper functions. Tests: added a synthetic nested no-merge IF regression that asserts the branch tail preserves a bottom-to-top anchor when it connects to the parent merge, and ran make build plus make test.
1 parent 02f4d95 commit 6bb1846

3 files changed

Lines changed: 123 additions & 32 deletions

File tree

‎mdl/executor/cmd_microflows_builder_control.go‎

Lines changed: 48 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -137,17 +137,20 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
137137
fb.flows = append(fb.flows, flow)
138138
} else {
139139
var flow *microflows.SequenceFlow
140-
if pendingThenCase != "" {
140+
originAnchor := prevThenAnchor
141+
destAnchor := thisAnchor
142+
if pendingThenCase != "" || pendingThenAnchor != nil {
143+
originAnchor, destAnchor = pendingFlowAnchors(prevThenAnchor, pendingThenAnchor, thisAnchor)
141144
flow = newHorizontalFlowWithCase(lastThenID, actID, pendingThenCase)
142-
if pendingThenAnchor != nil {
143-
prevThenAnchor = pendingThenAnchor
145+
if pendingThenCase == "" {
146+
flow = newHorizontalFlow(lastThenID, actID)
144147
}
145148
pendingThenCase = ""
146149
pendingThenAnchor = nil
147150
} else {
148151
flow = newHorizontalFlow(lastThenID, actID)
149152
}
150-
applyUserAnchors(flow, prevThenAnchor, thisAnchor)
153+
applyUserAnchors(flow, originAnchor, destAnchor)
151154
fb.flows = append(fb.flows, flow)
152155
}
153156
prevThenAnchor = thisAnchor
@@ -171,15 +174,18 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
171174
if !thenReturns && needMerge {
172175
if lastThenID != "" {
173176
var flow *microflows.SequenceFlow
174-
if pendingThenCase != "" {
177+
originAnchor := prevThenAnchor
178+
destAnchor := (*ast.FlowAnchors)(nil)
179+
if pendingThenCase != "" || pendingThenAnchor != nil {
180+
originAnchor, destAnchor = pendingFlowAnchors(prevThenAnchor, pendingThenAnchor, nil)
175181
flow = newHorizontalFlowWithCase(lastThenID, mergeID, pendingThenCase)
176-
if pendingThenAnchor != nil {
177-
prevThenAnchor = pendingThenAnchor
182+
if pendingThenCase == "" {
183+
flow = newHorizontalFlow(lastThenID, mergeID)
178184
}
179185
} else {
180186
flow = newHorizontalFlow(lastThenID, mergeID)
181187
}
182-
applyUserAnchors(flow, prevThenAnchor, nil)
188+
applyUserAnchors(flow, originAnchor, destAnchor)
183189
fb.flows = append(fb.flows, flow)
184190
} else {
185191
// Empty THEN body - connect split directly to merge with true case
@@ -214,17 +220,20 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
214220
fb.flows = append(fb.flows, flow)
215221
} else {
216222
var flow *microflows.SequenceFlow
217-
if pendingElseCase != "" {
223+
originAnchor := prevElseAnchor
224+
destAnchor := thisAnchor
225+
if pendingElseCase != "" || pendingElseAnchor != nil {
226+
originAnchor, destAnchor = pendingFlowAnchors(prevElseAnchor, pendingElseAnchor, thisAnchor)
218227
flow = newHorizontalFlowWithCase(lastElseID, actID, pendingElseCase)
219-
if pendingElseAnchor != nil {
220-
prevElseAnchor = pendingElseAnchor
228+
if pendingElseCase == "" {
229+
flow = newHorizontalFlow(lastElseID, actID)
221230
}
222231
pendingElseCase = ""
223232
pendingElseAnchor = nil
224233
} else {
225234
flow = newHorizontalFlow(lastElseID, actID)
226235
}
227-
applyUserAnchors(flow, prevElseAnchor, thisAnchor)
236+
applyUserAnchors(flow, originAnchor, destAnchor)
228237
fb.flows = append(fb.flows, flow)
229238
}
230239
prevElseAnchor = thisAnchor
@@ -248,16 +257,18 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
248257
if !elseReturns && needMerge {
249258
if lastElseID != "" {
250259
flow := newUpwardFlow(lastElseID, mergeID)
251-
if pendingElseCase != "" {
252-
flow.CaseValue = microflows.EnumerationCase{
253-
BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
254-
Value: pendingElseCase,
255-
}
256-
if pendingElseAnchor != nil {
257-
prevElseAnchor = pendingElseAnchor
260+
originAnchor := prevElseAnchor
261+
destAnchor := (*ast.FlowAnchors)(nil)
262+
if pendingElseCase != "" || pendingElseAnchor != nil {
263+
originAnchor, destAnchor = pendingFlowAnchors(prevElseAnchor, pendingElseAnchor, nil)
264+
if pendingElseCase != "" {
265+
flow.CaseValue = microflows.EnumerationCase{
266+
BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
267+
Value: pendingElseCase,
268+
}
258269
}
259270
}
260-
applyUserAnchors(flow, prevElseAnchor, nil)
271+
applyUserAnchors(flow, originAnchor, destAnchor)
261272
fb.flows = append(fb.flows, flow)
262273
}
263274
}
@@ -329,17 +340,20 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
329340
fb.flows = append(fb.flows, flow)
330341
} else {
331342
var flow *microflows.SequenceFlow
332-
if pendingThenCase != "" {
343+
originAnchor := prevThenAnchor
344+
destAnchor := thisAnchor
345+
if pendingThenCase != "" || pendingThenAnchor != nil {
346+
originAnchor, destAnchor = pendingFlowAnchors(prevThenAnchor, pendingThenAnchor, thisAnchor)
333347
flow = newHorizontalFlowWithCase(lastThenID, actID, pendingThenCase)
334-
if pendingThenAnchor != nil {
335-
prevThenAnchor = pendingThenAnchor
348+
if pendingThenCase == "" {
349+
flow = newHorizontalFlow(lastThenID, actID)
336350
}
337351
pendingThenCase = ""
338352
pendingThenAnchor = nil
339353
} else {
340354
flow = newHorizontalFlow(lastThenID, actID)
341355
}
342-
applyUserAnchors(flow, prevThenAnchor, thisAnchor)
356+
applyUserAnchors(flow, originAnchor, destAnchor)
343357
fb.flows = append(fb.flows, flow)
344358
}
345359
prevThenAnchor = thisAnchor
@@ -363,16 +377,18 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
363377
if !thenReturns && needMerge {
364378
if lastThenID != "" {
365379
flow := newUpwardFlow(lastThenID, mergeID)
366-
if pendingThenCase != "" {
367-
flow.CaseValue = microflows.EnumerationCase{
368-
BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
369-
Value: pendingThenCase,
370-
}
371-
if pendingThenAnchor != nil {
372-
prevThenAnchor = pendingThenAnchor
380+
originAnchor := prevThenAnchor
381+
destAnchor := (*ast.FlowAnchors)(nil)
382+
if pendingThenCase != "" || pendingThenAnchor != nil {
383+
originAnchor, destAnchor = pendingFlowAnchors(prevThenAnchor, pendingThenAnchor, nil)
384+
if pendingThenCase != "" {
385+
flow.CaseValue = microflows.EnumerationCase{
386+
BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
387+
Value: pendingThenCase,
388+
}
373389
}
374390
}
375-
applyUserAnchors(flow, prevThenAnchor, nil)
391+
applyUserAnchors(flow, originAnchor, destAnchor)
376392
fb.flows = append(fb.flows, flow)
377393
} else {
378394
// Empty THEN body - connect split directly to merge going down and back up

‎mdl/executor/cmd_microflows_builder_flows.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,20 @@ func applyUserAnchors(flow *microflows.SequenceFlow, origin *ast.FlowAnchors, de
191191
}
192192
}
193193

194+
func branchDestinationAnchor(branchAnchor, stmtAnchor *ast.FlowAnchors) *ast.FlowAnchors {
195+
if branchAnchor != nil && branchAnchor.To != ast.AnchorSideUnset {
196+
return branchAnchor
197+
}
198+
return stmtAnchor
199+
}
200+
201+
func pendingFlowAnchors(previousAnchor, pendingAnchor, stmtAnchor *ast.FlowAnchors) (*ast.FlowAnchors, *ast.FlowAnchors) {
202+
if pendingAnchor == nil {
203+
return previousAnchor, stmtAnchor
204+
}
205+
return pendingAnchor, branchDestinationAnchor(pendingAnchor, stmtAnchor)
206+
}
207+
194208
// lastStmtIsReturn reports whether execution of a body is guaranteed to terminate
195209
// (via RETURN or RAISE ERROR) on every path — i.e. control can never fall off the
196210
// end of the body into the parent flow.

‎mdl/executor/cmd_microflows_builder_no_merge_test.go‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,58 @@ func TestBuildFlowGraph_NoMergeIfElseContinuesFromBranchTail(t *testing.T) {
4646
}
4747
}
4848

49+
func TestBuildFlowGraph_NestedNoMergeTailCarriesAnchorToParentMerge(t *testing.T) {
50+
anchoredTail := &ast.LogStmt{
51+
Level: ast.LogInfo,
52+
Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "nested else tail"},
53+
Annotations: &ast.ActivityAnnotations{
54+
Anchor: &ast.FlowAnchors{From: ast.AnchorSideBottom, To: ast.AnchorSideTop},
55+
},
56+
}
57+
body := []ast.MicroflowStatement{
58+
&ast.IfStmt{
59+
Condition: &ast.VariableExpr{Name: "Outer"},
60+
ThenBody: []ast.MicroflowStatement{
61+
&ast.IfStmt{
62+
Condition: &ast.VariableExpr{Name: "Inner"},
63+
ThenBody: []ast.MicroflowStatement{
64+
&ast.ReturnStmt{Value: &ast.LiteralExpr{Kind: ast.LiteralBoolean, Value: true}},
65+
},
66+
ElseBody: []ast.MicroflowStatement{anchoredTail},
67+
},
68+
},
69+
ElseBody: []ast.MicroflowStatement{
70+
&ast.LogStmt{Level: ast.LogInfo, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "outer else"}},
71+
},
72+
},
73+
&ast.LogStmt{Level: ast.LogInfo, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "after outer"}},
74+
}
75+
76+
fb := &flowBuilder{
77+
posX: 100,
78+
posY: 100,
79+
spacing: HorizontalSpacing,
80+
declaredVars: map[string]string{"Outer": "Boolean", "Inner": "Boolean"},
81+
measurer: &layoutMeasurer{},
82+
}
83+
oc := fb.buildFlowGraph(body, &ast.MicroflowReturnType{Type: ast.DataType{Kind: ast.TypeBoolean}})
84+
85+
tailID := findLogActivityIDByMessage(t, oc, "nested else tail")
86+
for _, flow := range oc.Flows {
87+
if flow.OriginID != tailID {
88+
continue
89+
}
90+
if _, ok := objectByID(oc, flow.DestinationID).(*microflows.ExclusiveMerge); !ok {
91+
continue
92+
}
93+
if flow.OriginConnectionIndex != AnchorBottom || flow.DestinationConnectionIndex != AnchorTop {
94+
t.Fatalf("nested no-merge tail anchor = from %d to %d, want bottom/top", flow.OriginConnectionIndex, flow.DestinationConnectionIndex)
95+
}
96+
return
97+
}
98+
t.Fatal("expected nested no-merge tail to connect to the parent merge")
99+
}
100+
49101
func findLogActivityIDByMessage(t *testing.T, oc *microflows.MicroflowObjectCollection, message string) model.ID {
50102
t.Helper()
51103
for _, obj := range oc.Objects {
@@ -65,6 +117,15 @@ func findLogActivityIDByMessage(t *testing.T, oc *microflows.MicroflowObjectColl
65117
return ""
66118
}
67119

120+
func objectByID(oc *microflows.MicroflowObjectCollection, id model.ID) microflows.MicroflowObject {
121+
for _, obj := range oc.Objects {
122+
if obj.GetID() == id {
123+
return obj
124+
}
125+
}
126+
return nil
127+
}
128+
68129
func hasSequenceFlow(flows []*microflows.SequenceFlow, originID, destinationID model.ID) bool {
69130
for _, flow := range flows {
70131
if flow.OriginID == originID && flow.DestinationID == destinationID {

0 commit comments

Comments
 (0)