From 2cad1f0ca04488f0efb6ed64176fe603a734be5c Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Sun, 26 Apr 2026 17:27:12 +0200 Subject: [PATCH 1/2] fix: preserve nested loop body flow traversal Symptom: describe could omit loop-body continuation activities when a loop stored boundary flows locally but stored the real body continuation in the parent graph. Root cause: loop traversal preferred the loop-local origin map whenever an origin already existed there, so parent-level flows for the same origin were not merged into the loop body. Nested loop descendants also did not receive the relevant parent-level body flows. Fix: collect loop body object IDs recursively, merge missing parent-level body flows into loop-local flow maps, and choose loop body starts from the merged flow graph. Tests: add a nested-loop traversal regression where local boundary flows and parent-level continuation flows share origins. --- mdl/executor/cmd_microflows_show_helpers.go | 78 ++++++++++++-- mdl/executor/cmd_microflows_traverse_test.go | 105 +++++++++++++++++++ 2 files changed, 175 insertions(+), 8 deletions(-) diff --git a/mdl/executor/cmd_microflows_show_helpers.go b/mdl/executor/cmd_microflows_show_helpers.go index 6c646abb..8fe21b63 100644 --- a/mdl/executor/cmd_microflows_show_helpers.go +++ b/mdl/executor/cmd_microflows_show_helpers.go @@ -831,6 +831,7 @@ func emitLoopBody( for _, loopObj := range loop.ObjectCollection.Objects { loopActivityMap[loopObj.GetID()] = loopObj } + loopObjectIDs := collectLoopObjectIDs(loop.ObjectCollection) // Build flow graph from the loop's own ObjectCollection flows loopFlowsByOrigin := make(map[model.ID][]*microflows.SequenceFlow) @@ -841,9 +842,9 @@ func emitLoopBody( } // Also include parent flows that originate from loop body objects (for backward compatibility) for originID, flows := range flowsByOrigin { - if _, inLoop := loopActivityMap[originID]; inLoop { - if _, exists := loopFlowsByOrigin[originID]; !exists { - loopFlowsByOrigin[originID] = flows + if loopObjectIDs[originID] { + for _, flow := range flows { + loopFlowsByOrigin[originID] = appendSequenceFlowIfMissing(loopFlowsByOrigin[originID], flow) } } } @@ -858,9 +859,9 @@ func emitLoopBody( } } for destID, flows := range flowsByDest { - if _, inLoop := loopActivityMap[destID]; inLoop { - if _, exists := loopFlowsByDest[destID]; !exists { - loopFlowsByDest[destID] = flows + if loopObjectIDs[destID] { + for _, flow := range flows { + loopFlowsByDest[destID] = appendSequenceFlowIfMissing(loopFlowsByDest[destID], flow) } } } @@ -878,10 +879,12 @@ func emitLoopBody( } } var firstID model.ID + var firstObj microflows.MicroflowObject for id, count := range incomingCount { - if count == 0 { + obj := loopActivityMap[id] + if count == 0 && preferLoopBodyStart(obj, firstObj) { firstID = id - break + firstObj = obj } } @@ -892,6 +895,65 @@ func emitLoopBody( } } +func collectLoopObjectIDs(oc *microflows.MicroflowObjectCollection) map[model.ID]bool { + result := make(map[model.ID]bool) + collectLoopObjectIDsInto(oc, result) + return result +} + +func collectLoopObjectIDsInto(oc *microflows.MicroflowObjectCollection, result map[model.ID]bool) { + if oc == nil { + return + } + for _, obj := range oc.Objects { + if obj == nil { + continue + } + result[obj.GetID()] = true + if loop, ok := obj.(*microflows.LoopedActivity); ok { + collectLoopObjectIDsInto(loop.ObjectCollection, result) + } + } +} + +func appendSequenceFlowIfMissing(flows []*microflows.SequenceFlow, candidate *microflows.SequenceFlow) []*microflows.SequenceFlow { + if candidate == nil { + return flows + } + candidateKey := sequenceFlowIdentity(candidate) + for _, flow := range flows { + if sequenceFlowIdentity(flow) == candidateKey { + return flows + } + } + return append(flows, candidate) +} + +func sequenceFlowIdentity(flow *microflows.SequenceFlow) string { + if flow == nil { + return "" + } + if flow.ID != "" { + return string(flow.ID) + } + return fmt.Sprintf("%s>%s:%t:%d:%d", flow.OriginID, flow.DestinationID, flow.IsErrorHandler, flow.OriginConnectionIndex, flow.DestinationConnectionIndex) +} + +func preferLoopBodyStart(candidate, current microflows.MicroflowObject) bool { + if candidate == nil { + return false + } + if current == nil { + return true + } + candidatePos := candidate.GetPosition() + currentPos := current.GetPosition() + if candidatePos.X != currentPos.X { + return candidatePos.X < currentPos.X + } + return candidatePos.Y < currentPos.Y +} + // isMergePairedWithSplit reports whether an ExclusiveMerge appears as the // matching end-of-branch point for some ExclusiveSplit recorded in // splitMergeMap (i.e., it closes an IF/ELSE block). Merges that aren't paired diff --git a/mdl/executor/cmd_microflows_traverse_test.go b/mdl/executor/cmd_microflows_traverse_test.go index f924ddd1..3cd8c3be 100644 --- a/mdl/executor/cmd_microflows_traverse_test.go +++ b/mdl/executor/cmd_microflows_traverse_test.go @@ -3,6 +3,7 @@ package executor import ( + "strings" "testing" "github.com/mendixlabs/mxcli/model" @@ -124,6 +125,110 @@ func TestTraverseFlow_IfElse(t *testing.T) { } } +func TestTraverseFlow_LoopBodyMergesParentFlowsForExistingOrigin(t *testing.T) { + e := newTestExecutor() + + logActivity := func(id, message string, x, y int) *microflows.ActionActivity { + return µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{ + BaseMicroflowObject: microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: mkID(id)}, + Position: model.Point{X: x, Y: y}, + }, + }, + Action: µflows.LogMessageAction{ + LogLevel: "Info", + LogNodeName: "'Synthetic'", + MessageTemplate: &model.Text{Translations: map[string]string{"en_US": message}}, + }, + } + } + + nestedLoop := µflows.LoopedActivity{ + BaseMicroflowObject: microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: mkID("nested")}, + Position: model.Point{X: 500, Y: 100}, + }, + LoopSource: µflows.IterableList{ + VariableName: "role", + ListVariableName: "roles", + }, + ObjectCollection: µflows.MicroflowObjectCollection{ + Objects: []microflows.MicroflowObject{ + logActivity("nested-log", "nested", 120, 80), + logActivity("nested-tail", "nested-tail", 320, 80), + }, + Flows: []*microflows.SequenceFlow{ + // Same pattern one level deeper: the loop-boundary flow is + // local, while the real body continuation is supplied by the + // parent graph that must be threaded into nested loops. + mkFlow("nested-log", "nested"), + }, + }, + } + outerLoop := µflows.LoopedActivity{ + BaseMicroflowObject: microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: mkID("loop")}, + }, + LoopSource: µflows.IterableList{ + VariableName: "item", + ListVariableName: "items", + }, + ObjectCollection: µflows.MicroflowObjectCollection{ + // Adversarial order: storage lists the nested loop first, but the + // flow graph and positions define the actual body order. + Objects: []microflows.MicroflowObject{ + nestedLoop, + logActivity("setup", "setup", 100, 100), + logActivity("fetch", "fetch", 300, 100), + }, + Flows: []*microflows.SequenceFlow{ + mkFlow("setup", "fetch"), + // This local loop-boundary flow gives "fetch" an existing + // local origin entry. Parent-level body flows with the same + // origin must still be merged in. + mkFlow("fetch", "loop"), + }, + }, + } + + activityMap := map[model.ID]microflows.MicroflowObject{ + mkID("start"): µflows.StartEvent{BaseMicroflowObject: mkObj("start")}, + mkID("loop"): outerLoop, + mkID("end"): µflows.EndEvent{BaseMicroflowObject: mkObj("end")}, + } + flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{ + mkID("start"): {mkFlow("start", "loop")}, + mkID("loop"): {mkFlow("loop", "end")}, + mkID("fetch"): {mkFlow("fetch", "nested")}, + mkID("nested"): {mkFlow("nested", "loop")}, + mkID("nested-log"): { + mkFlow("nested-log", "nested-tail"), + }, + mkID("nested-tail"): {mkFlow("nested-tail", "nested")}, + } + + var lines []string + e.traverseFlow(mkID("start"), activityMap, flowsByOrigin, nil, make(map[model.ID]bool), nil, nil, &lines, 0, nil, 0, nil) + + out := strings.Join(lines, "\n") + for _, want := range []string{ + "log info node 'Synthetic' 'setup';", + "log info node 'Synthetic' 'fetch';", + "loop $role in $roles", + "log info node 'Synthetic' 'nested';", + "log info node 'Synthetic' 'nested-tail';", + } { + if !strings.Contains(out, want) { + t.Fatalf("expected %q in output:\n%s", want, out) + } + } + if strings.Index(out, "setup") > strings.Index(out, "fetch") || + strings.Index(out, "fetch") > strings.Index(out, "loop $role in $roles") { + t.Fatalf("loop body emitted in wrong order:\n%s", out) + } +} + // ============================================================================= // collectErrorHandlerStatements // ============================================================================= From 77d439dd9c8c95efa8be219c7114f4010ec5aa47 Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Mon, 27 Apr 2026 03:11:46 +0200 Subject: [PATCH 2/2] test: add bug-test reproducer for nested loop body flow traversal fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an MDL script under mdl-examples/bug-tests/ exercising nested loops where the inner loop's body activities live in the parent microflow graph. The describe → exec → describe fixpoint confirms the describer merges parent-level body flows into the loop-local traversal map. Co-Authored-By: Claude Opus 4.7 --- .../328-describer-nested-loop-body-flows.mdl | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 mdl-examples/bug-tests/328-describer-nested-loop-body-flows.mdl diff --git a/mdl-examples/bug-tests/328-describer-nested-loop-body-flows.mdl b/mdl-examples/bug-tests/328-describer-nested-loop-body-flows.mdl new file mode 100644 index 00000000..68283ef3 --- /dev/null +++ b/mdl-examples/bug-tests/328-describer-nested-loop-body-flows.mdl @@ -0,0 +1,66 @@ +-- ============================================================================ +-- Bug #328: DESCRIBE MICROFLOW omitted nested loop body continuation flows +-- ============================================================================ +-- +-- Symptom (before fix): +-- Some microflow graphs store the loop boundary flow (last body activity → +-- loop iterator) in the loop's local object collection, while the real +-- body-to-body continuation flows are stored in the parent microflow +-- graph. When a body activity already had a local outgoing flow (the +-- boundary one), the describer ignored parent-level flows with the same +-- origin. Result: activities downstream of that origin disappeared from +-- the described loop body — most visible on nested loops, where the +-- inner body collapsed into one statement. +-- +-- Root cause: +-- `emitLoopBody` built a fresh `loopFlowsByOrigin` from the loop's local +-- object collection only, then short-circuited when an origin had a +-- local entry, never merging the equivalent parent-level flows. +-- +-- After fix: +-- The describer now merges parent-level body flows into the loop-local +-- traversal map for any origin that lives in the loop body, including +-- nested loop descendants collected recursively. +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/328-describer-nested-loop-body-flows.mdl -p app.mpr +-- mxcli -p app.mpr -c "describe microflow BugTest328.MF_NestedLoops" +-- The describe output must contain BOTH outer- and inner-loop body +-- activities and must be a fixpoint under describe → exec → describe. +-- ============================================================================ + +create module BugTest328; + +create entity BugTest328.Item ( + Name : string(100) +); +/ + +create entity BugTest328.Role ( + Name : string(100) +); +/ + +create microflow BugTest328.MF_NestedLoops ( + $Items: list of BugTest328.Item, + $Roles: list of BugTest328.Role +) +begin + log info node 'BugTest328' 'before outer'; + + loop $Item in $Items + begin + log info node 'BugTest328' 'outer head'; + + loop $Role in $Roles + begin + log info node 'BugTest328' 'inner body'; + log info node 'BugTest328' 'inner tail'; + end loop; + + log info node 'BugTest328' 'outer tail'; + end loop; + + log info node 'BugTest328' 'after outer'; +end; +/