fix: pair microflow splits with nearest merge

mdl-examples/bug-tests/326-describer-pair-split-with-nearest-merge.mdl

@@ -0,0 +1,55 @@
+-- ============================================================================
+-- Bug #326: Describer paired splits with downstream merge instead of nearest
+-- ============================================================================
+--
+-- Symptom (before fix):
+--   When a microflow had two sequential `if ... end if;` blocks, the
+--   describer could pair the FIRST split with the SECOND merge, nesting
+--   the continuation between the two ifs (and even the second if) inside
+--   the first if's body. A describe → exec → describe cycle then mutated
+--   the structure even though the original graph was valid:
+--     -- before fix, after first describe:
+--     if $Logo != empty then
+--       log info node 'App' 'logo';
+--       log info node 'App' 'after logo';   -- wrong: was outside first if
+--       if $Cover != empty then              -- wrong: was sibling, not nested
+--         log info node 'App' 'cover';
+--       end if;
+--     end if;
+--
+-- Root cause:
+--   Split/merge pairing picked any common downstream merge reachable from
+--   both branches, not the nearest one. Error-handler flows were included
+--   in the search, biasing it further.
+--
+-- After fix:
+--   `findMergeForSplit` now uses shortest-path distances per branch and
+--   selects the nearest common merge, ignoring error-handler flows for
+--   structural pairing.
+--
+-- Usage:
+--   mxcli exec mdl-examples/bug-tests/326-describer-pair-split-with-nearest-merge.mdl -p app.mpr
+--   mxcli -p app.mpr -c "describe microflow BugTest326.MF_SequentialIfs"
+--   The describe output must keep the two `if ... end if;` blocks as
+--   siblings (with the `log` between them at top level), and the
+--   describe → exec → describe cycle must be a fixpoint.
+-- ============================================================================
+
+create module BugTest326;
+
+create microflow BugTest326.MF_SequentialIfs (
+  $Logo: string,
+  $Cover: string
+)
+begin
+  if $Logo != empty then
+    log info node 'BugTest326' 'logo';
+  end if;
+
+  log info node 'BugTest326' 'after logo';
+
+  if $Cover != empty then
+    log info node 'BugTest326' 'cover';
+  end if;
+end;
+/

mdl/executor/cmd_microflows_show.go

@@ -781,26 +781,17 @@ func findMergeForSplit(
 	flowsByOrigin map[model.ID][]*microflows.SequenceFlow,
 	activityMap map[model.ID]microflows.MicroflowObject,
 ) model.ID {
-	flows := flowsByOrigin[splitID]
+	flows := findNormalFlows(flowsByOrigin[splitID])
 	if len(flows) < 2 {
 		return ""
 	}
 
-	// Follow each branch and collect all reachable nodes
-	branch0Nodes := collectReachableNodes(ctx, flows[0].DestinationID, flowsByOrigin, activityMap, make(map[model.ID]bool))
-	branch1Nodes := collectReachableNodes(ctx, flows[1].DestinationID, flowsByOrigin, activityMap, make(map[model.ID]bool))
-
-	// Find the first common node that is an ExclusiveMerge
-	// This is a simplification - we look for the first merge point reachable from both branches
-	for nodeID := range branch0Nodes {
-		if branch1Nodes[nodeID] {
-			if _, ok := activityMap[nodeID].(*microflows.ExclusiveMerge); ok {
-				return nodeID
-			}
-		}
+	branchDistances := make([]map[model.ID]int, 0, len(flows))
+	for _, flow := range flows {
+		branchDistances = append(branchDistances, collectReachableDistances(flow.DestinationID, flowsByOrigin))
 	}
 
-	return ""
+	return selectNearestCommonMerge(activityMap, branchDistances)
 }
 
 // collectReachableNodes collects all nodes reachable from a starting node.
@@ -830,4 +821,98 @@ func collectReachableNodes(
 	return result
 }
 
+// collectReachableDistances collects the shortest normal-flow distance from a
+// branch start to every reachable node. Error handler flows are excluded because
+// they do not participate in split/merge structural pairing.
+func collectReachableDistances(
+	startID model.ID,
+	flowsByOrigin map[model.ID][]*microflows.SequenceFlow,
+) map[model.ID]int {
+	distances := map[model.ID]int{}
+	type queueItem struct {
+		id       model.ID
+		distance int
+	}
+	queue := []queueItem{{id: startID}}
+
+	for len(queue) > 0 {
+		item := queue[0]
+		queue = queue[1:]
+
+		if previous, ok := distances[item.id]; ok && previous <= item.distance {
+			continue
+		}
+		distances[item.id] = item.distance
+
+		for _, flow := range findNormalFlows(flowsByOrigin[item.id]) {
+			queue = append(queue, queueItem{
+				id:       flow.DestinationID,
+				distance: item.distance + 1,
+			})
+		}
+	}
+
+	return distances
+}
+
+func selectNearestCommonMerge(
+	activityMap map[model.ID]microflows.MicroflowObject,
+	branchDistances []map[model.ID]int,
+) model.ID {
+	if len(branchDistances) < 2 {
+		return ""
+	}
+
+	type candidate struct {
+		id          model.ID
+		maxDistance int
+		sumDistance int
+	}
+	candidates := []candidate{}
+
+	for nodeID, firstDistance := range branchDistances[0] {
+		if _, ok := activityMap[nodeID].(*microflows.ExclusiveMerge); !ok {
+			continue
+		}
+
+		maxDistance := firstDistance
+		sumDistance := firstDistance
+		common := true
+		for _, distances := range branchDistances[1:] {
+			distance, ok := distances[nodeID]
+			if !ok {
+				common = false
+				break
+			}
+			if distance > maxDistance {
+				maxDistance = distance
+			}
+			sumDistance += distance
+		}
+		if common {
+			candidates = append(candidates, candidate{
+				id:          nodeID,
+				maxDistance: maxDistance,
+				sumDistance: sumDistance,
+			})
+		}
+	}
+
+	if len(candidates) == 0 {
+		return ""
+	}
+
+	sort.Slice(candidates, func(i, j int) bool {
+		if candidates[i].maxDistance != candidates[j].maxDistance {
+			return candidates[i].maxDistance < candidates[j].maxDistance
+		}
+		if candidates[i].sumDistance != candidates[j].sumDistance {
+			return candidates[i].sumDistance < candidates[j].sumDistance
+		}
+		return string(candidates[i].id) < string(candidates[j].id)
+	})
+
+	return candidates[0].id
+}
+
 // --- Executor method wrappers for callers in unmigrated code ---

mdl/executor/cmd_microflows_traverse_test.go

@@ -3,6 +3,7 @@
 package executor
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/mendixlabs/mxcli/model"
@@ -124,6 +125,121 @@ func TestTraverseFlow_IfElse(t *testing.T) {
 	}
 }
 
+func TestFindMergeForSplit_ChoosesNearestMergeBeforeDownstreamIf(t *testing.T) {
+	activityMap := map[model.ID]microflows.MicroflowObject{
+		mkID("logo_split"): &microflows.ExclusiveSplit{
+			BaseMicroflowObject: mkObj("logo_split"),
+			SplitCondition:      &microflows.ExpressionSplitCondition{Expression: "$Logo != empty"},
+		},
+		mkID("logo_then"): &microflows.ActionActivity{
+			BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("logo_then")},
+			Action:       &microflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'App'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "logo"}}},
+		},
+		mkID("logo_merge"): &microflows.ExclusiveMerge{BaseMicroflowObject: mkObj("logo_merge")},
+		mkID("after_logo"): &microflows.ActionActivity{
+			BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("after_logo")},
+			Action:       &microflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'App'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "after logo"}}},
+		},
+		mkID("cover_split"): &microflows.ExclusiveSplit{
+			BaseMicroflowObject: mkObj("cover_split"),
+			SplitCondition:      &microflows.ExpressionSplitCondition{Expression: "$Cover != empty"},
+		},
+		mkID("cover_then"): &microflows.ActionActivity{
+			BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("cover_then")},
+			Action:       &microflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'App'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "cover"}}},
+		},
+		mkID("cover_merge"): &microflows.ExclusiveMerge{BaseMicroflowObject: mkObj("cover_merge")},
+	}
+
+	flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{
+		mkID("logo_split"): {
+			mkBranchFlow("logo_split", "logo_then", &microflows.ExpressionCase{Expression: "true"}),
+			mkBranchFlow("logo_split", "logo_merge", &microflows.ExpressionCase{Expression: "false"}),
+		},
+		mkID("logo_then"):  {mkFlow("logo_then", "logo_merge")},
+		mkID("logo_merge"): {mkFlow("logo_merge", "after_logo")},
+		mkID("after_logo"): {mkFlow("after_logo", "cover_split")},
+		mkID("cover_split"): {
+			mkBranchFlow("cover_split", "cover_then", &microflows.ExpressionCase{Expression: "true"}),
+			mkBranchFlow("cover_split", "cover_merge", &microflows.ExpressionCase{Expression: "false"}),
+		},
+		mkID("cover_then"): {mkFlow("cover_then", "cover_merge")},
+	}
+
+	got := findMergeForSplit(nil, mkID("logo_split"), flowsByOrigin, activityMap)
+	if got != mkID("logo_merge") {
+		t.Fatalf("logo split paired with %q, want nearest merge %q", got, mkID("logo_merge"))
+	}
+}
+
+func TestTraverseFlow_SequentialIfWithoutElseKeepsContinuationOutsideFirstIf(t *testing.T) {
+	e := newTestExecutor()
+
+	activityMap := map[model.ID]microflows.MicroflowObject{
+		mkID("start"): &microflows.StartEvent{BaseMicroflowObject: mkObj("start")},
+		mkID("logo_split"): &microflows.ExclusiveSplit{
+			BaseMicroflowObject: mkObj("logo_split"),
+			SplitCondition:      &microflows.ExpressionSplitCondition{Expression: "$Logo != empty"},
+		},
+		mkID("logo_then"): &microflows.ActionActivity{
+			BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("logo_then")},
+			Action:       &microflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'App'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "logo"}}},
+		},
+		mkID("logo_merge"): &microflows.ExclusiveMerge{BaseMicroflowObject: mkObj("logo_merge")},
+		mkID("after_logo"): &microflows.ActionActivity{
+			BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("after_logo")},
+			Action:       &microflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'App'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "after logo"}}},
+		},
+		mkID("cover_split"): &microflows.ExclusiveSplit{
+			BaseMicroflowObject: mkObj("cover_split"),
+			SplitCondition:      &microflows.ExpressionSplitCondition{Expression: "$Cover != empty"},
+		},
+		mkID("cover_then"): &microflows.ActionActivity{
+			BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("cover_then")},
+			Action:       &microflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'App'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "cover"}}},
+		},
+		mkID("cover_merge"): &microflows.ExclusiveMerge{BaseMicroflowObject: mkObj("cover_merge")},
+		mkID("end"):         &microflows.EndEvent{BaseMicroflowObject: mkObj("end")},
+	}
+
+	flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{
+		mkID("start"): {mkFlow("start", "logo_split")},
+		mkID("logo_split"): {
+			mkBranchFlow("logo_split", "logo_then", &microflows.ExpressionCase{Expression: "true"}),
+			mkBranchFlow("logo_split", "logo_merge", &microflows.ExpressionCase{Expression: "false"}),
+		},
+		mkID("logo_then"):  {mkFlow("logo_then", "logo_merge")},
+		mkID("logo_merge"): {mkFlow("logo_merge", "after_logo")},
+		mkID("after_logo"): {mkFlow("after_logo", "cover_split")},
+		mkID("cover_split"): {
+			mkBranchFlow("cover_split", "cover_then", &microflows.ExpressionCase{Expression: "true"}),
+			mkBranchFlow("cover_split", "cover_merge", &microflows.ExpressionCase{Expression: "false"}),
+		},
+		mkID("cover_then"):  {mkFlow("cover_then", "cover_merge")},
+		mkID("cover_merge"): {mkFlow("cover_merge", "end")},
+	}
+
+	splitMergeMap := map[model.ID]model.ID{
+		mkID("logo_split"):  findMergeForSplit(nil, mkID("logo_split"), flowsByOrigin, activityMap),
+		mkID("cover_split"): findMergeForSplit(nil, mkID("cover_split"), flowsByOrigin, activityMap),
+	}
+	var lines []string
+	visited := make(map[model.ID]bool)
+	e.traverseFlow(mkID("start"), activityMap, flowsByOrigin, splitMergeMap, visited, nil, nil, &lines, 0, nil, 0, nil)
+
+	out := strings.Join(lines, "\n")
+	firstEndIf := strings.Index(out, "end if;")
+	afterLogo := strings.Index(out, "after logo")
+	if firstEndIf == -1 || afterLogo == -1 || firstEndIf > afterLogo {
+		t.Fatalf("continuation after first IF was emitted inside the IF:\n%s", out)
+	}
+	for _, line := range lines {
+		if strings.Contains(line, "after logo") && strings.HasPrefix(line, "  ") {
+			t.Fatalf("continuation after first IF must be top-level, got %q in:\n%s", line, out)
+		}
+	}
+}
+
 // =============================================================================
 // collectErrorHandlerStatements
 // =============================================================================