Merge pull request #327 from hjotha/submit/describer-nearest-split-merge

fix: pair microflow splits with nearest merge

Author: ako

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_builder.go

@@ -53,7 +53,7 @@ type flowBuilder struct {
 	microflowsCacheLoaded bool
 	nanoflowsCache        []*microflows.Nanoflow
 	nanoflowsCacheLoaded  bool
-	manualLoopBackTarget model.ID
+	manualLoopBackTarget  model.ID
 }
 
 // addError records a validation error during flow building.

mdl/executor/cmd_microflows_format_action.go

@@ -351,30 +351,30 @@ func formatAction(
 
 			stmt := fmt.Sprintf("retrieve $%s from %s", outputVar, entityName)
 
-		if dbSource.XPathConstraint != "" {
-			constraint := strings.TrimSpace(dbSource.XPathConstraint)
-			// XPath may contain multiple predicates like [a][b] or [a]\n[b].
-			// Split them and join with MDL 'and' so the parser sees
-			// separate xpathConstraint nodes.
-			if strings.HasPrefix(constraint, "[") && strings.HasSuffix(constraint, "]") {
-				// Split on "][" boundary (possibly separated by \n literals),
-				// then re-wrap each predicate.
-				inner := constraint[1 : len(constraint)-1]
-				// Normalise real newlines between predicates: ]\n[ → ][
-				inner = strings.ReplaceAll(inner, "]\n[", "][")
-				parts := strings.Split(inner, "][")
-				if len(parts) > 1 {
-					var wrapped []string
-					for _, p := range parts {
-						wrapped = append(wrapped, "["+strings.TrimSpace(p)+"]")
+			if dbSource.XPathConstraint != "" {
+				constraint := strings.TrimSpace(dbSource.XPathConstraint)
+				// XPath may contain multiple predicates like [a][b] or [a]\n[b].
+				// Split them and join with MDL 'and' so the parser sees
+				// separate xpathConstraint nodes.
+				if strings.HasPrefix(constraint, "[") && strings.HasSuffix(constraint, "]") {
+					// Split on "][" boundary (possibly separated by \n literals),
+					// then re-wrap each predicate.
+					inner := constraint[1 : len(constraint)-1]
+					// Normalise real newlines between predicates: ]\n[ → ][
+					inner = strings.ReplaceAll(inner, "]\n[", "][")
+					parts := strings.Split(inner, "][")
+					if len(parts) > 1 {
+						var wrapped []string
+						for _, p := range parts {
+							wrapped = append(wrapped, "["+strings.TrimSpace(p)+"]")
+						}
+						constraint = strings.Join(wrapped, "\n    ")
+					} else {
+						constraint = parts[0]
 					}
-					constraint = strings.Join(wrapped, "\n    ")
-				} else {
-					constraint = parts[0]
 				}
+				stmt += fmt.Sprintf("\n    where %s", constraint)
 			}
-			stmt += fmt.Sprintf("\n    where %s", constraint)
-		}
 
 			// Output SORT BY clause if present
 			if len(dbSource.Sorting) > 0 {

mdl/executor/cmd_microflows_show.go

@@ -867,60 +867,129 @@ func findSplitMergePoints(
 	return result
 }
 
-// findMergeForSplit finds the ExclusiveMerge where branches from a split converge.
+// findMergeForSplit finds the nearest node where branches from a split converge.
+// Studio Pro models often converge directly on the next activity instead of an
+// explicit ExclusiveMerge, so the join can be any executable microflow object.
 func findMergeForSplit(
 	ctx *ExecContext,
 	splitID model.ID,
 	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 selectNearestCommonJoin(activityMap, branchDistances)
 }
 
-// collectReachableNodes collects all nodes reachable from a starting node.
-func collectReachableNodes(
-	ctx *ExecContext,
+// 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 selectNearestCommonJoin(
 	activityMap map[model.ID]microflows.MicroflowObject,
-	visited map[model.ID]bool,
-) map[model.ID]bool {
-	result := make(map[model.ID]bool)
+	branchDistances []map[model.ID]int,
+) model.ID {
+	if len(branchDistances) < 2 {
+		return ""
+	}
 
-	var traverse func(id model.ID)
-	traverse = func(id model.ID) {
-		if visited[id] {
-			return
+	type candidate struct {
+		id          model.ID
+		maxDistance int
+		sumDistance int
+	}
+	candidates := []candidate{}
+
+	for nodeID, firstDistance := range branchDistances[0] {
+		if !isSplitJoinCandidate(activityMap[nodeID]) {
+			continue
 		}
-		visited[id] = true
-		result[id] = true
 
-		for _, flow := range flowsByOrigin[id] {
-			traverse(flow.DestinationID)
+		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,
+			})
 		}
 	}
 
-	traverse(startID)
-	return result
+	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
+}
+
+func isSplitJoinCandidate(obj microflows.MicroflowObject) bool {
+	switch obj.(type) {
+	case nil, *microflows.StartEvent, *microflows.EndEvent:
+		return false
+	default:
+		return true
+	}
 }
 
 // --- Executor method wrappers for callers in unmigrated code ---