From 8b6845fd6aa8d8dc6967357a250a6ae656202036 Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Sat, 25 Apr 2026 10:01:47 +0200 Subject: [PATCH 1/3] fix: pair microflow splits with nearest merge Symptom: sequential if-without-else structures could be described with the continuation of the first split nested inside that split. Root cause: split/merge pairing selected an arbitrary common reachable merge from map iteration, so a later downstream merge could be chosen before the immediate branch convergence. Fix: rank common merge candidates by nearest branch distance, then total distance and ID, while ignoring non-normal flows for structural pairing. Tests: add unit coverage for nearest-merge selection and for keeping the continuation after a sequential split at top level. --- mdl/executor/cmd_microflows_show.go | 118 ++++++++++++++++--- mdl/executor/cmd_microflows_traverse_test.go | 116 ++++++++++++++++++ 2 files changed, 220 insertions(+), 14 deletions(-) diff --git a/mdl/executor/cmd_microflows_show.go b/mdl/executor/cmd_microflows_show.go index cdd9486d..7a72e87c 100644 --- a/mdl/executor/cmd_microflows_show.go +++ b/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(ctx, flow.DestinationID, flowsByOrigin, activityMap)) } - return "" + return selectNearestCommonMerge(activityMap, branchDistances) } // collectReachableNodes collects all nodes reachable from a starting node. @@ -830,4 +821,103 @@ 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( + ctx *ExecContext, + startID model.ID, + flowsByOrigin map[model.ID][]*microflows.SequenceFlow, + activityMap map[model.ID]microflows.MicroflowObject, +) map[model.ID]int { + _ = ctx + _ = activityMap + + 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 --- diff --git a/mdl/executor/cmd_microflows_traverse_test.go b/mdl/executor/cmd_microflows_traverse_test.go index f924ddd1..8c920272 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,121 @@ func TestTraverseFlow_IfElse(t *testing.T) { } } +func TestFindMergeForSplit_ChoosesNearestMergeBeforeDownstreamIf(t *testing.T) { + activityMap := map[model.ID]microflows.MicroflowObject{ + mkID("logo_split"): µflows.ExclusiveSplit{ + BaseMicroflowObject: mkObj("logo_split"), + SplitCondition: µflows.ExpressionSplitCondition{Expression: "$Logo != empty"}, + }, + mkID("logo_then"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("logo_then")}, + Action: µflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'App'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "logo"}}}, + }, + mkID("logo_merge"): µflows.ExclusiveMerge{BaseMicroflowObject: mkObj("logo_merge")}, + mkID("after_logo"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("after_logo")}, + Action: µflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'App'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "after logo"}}}, + }, + mkID("cover_split"): µflows.ExclusiveSplit{ + BaseMicroflowObject: mkObj("cover_split"), + SplitCondition: µflows.ExpressionSplitCondition{Expression: "$Cover != empty"}, + }, + mkID("cover_then"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("cover_then")}, + Action: µflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'App'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "cover"}}}, + }, + mkID("cover_merge"): µflows.ExclusiveMerge{BaseMicroflowObject: mkObj("cover_merge")}, + } + + flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{ + mkID("logo_split"): { + mkBranchFlow("logo_split", "logo_then", µflows.ExpressionCase{Expression: "true"}), + mkBranchFlow("logo_split", "logo_merge", µflows.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", µflows.ExpressionCase{Expression: "true"}), + mkBranchFlow("cover_split", "cover_merge", µflows.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"): µflows.StartEvent{BaseMicroflowObject: mkObj("start")}, + mkID("logo_split"): µflows.ExclusiveSplit{ + BaseMicroflowObject: mkObj("logo_split"), + SplitCondition: µflows.ExpressionSplitCondition{Expression: "$Logo != empty"}, + }, + mkID("logo_then"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("logo_then")}, + Action: µflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'App'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "logo"}}}, + }, + mkID("logo_merge"): µflows.ExclusiveMerge{BaseMicroflowObject: mkObj("logo_merge")}, + mkID("after_logo"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("after_logo")}, + Action: µflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'App'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "after logo"}}}, + }, + mkID("cover_split"): µflows.ExclusiveSplit{ + BaseMicroflowObject: mkObj("cover_split"), + SplitCondition: µflows.ExpressionSplitCondition{Expression: "$Cover != empty"}, + }, + mkID("cover_then"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("cover_then")}, + Action: µflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'App'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "cover"}}}, + }, + mkID("cover_merge"): µflows.ExclusiveMerge{BaseMicroflowObject: mkObj("cover_merge")}, + mkID("end"): µflows.EndEvent{BaseMicroflowObject: mkObj("end")}, + } + + flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{ + mkID("start"): {mkFlow("start", "logo_split")}, + mkID("logo_split"): { + mkBranchFlow("logo_split", "logo_then", µflows.ExpressionCase{Expression: "true"}), + mkBranchFlow("logo_split", "logo_merge", µflows.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", µflows.ExpressionCase{Expression: "true"}), + mkBranchFlow("cover_split", "cover_merge", µflows.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 // ============================================================================= From e9a84f184c9c34e10e3fdc275e0252c5f28e7180 Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Mon, 27 Apr 2026 03:10:40 +0200 Subject: [PATCH 2/3] test: add bug-test reproducer for nearest split-merge pairing 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 sequential if-without-else blocks. The describe → exec → describe fixpoint confirms the first split is paired with its nearest merge, keeping the ifs as siblings rather than nesting the continuation inside the first if's body. Co-Authored-By: Claude Opus 4.7 --- ...escriber-pair-split-with-nearest-merge.mdl | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 mdl-examples/bug-tests/326-describer-pair-split-with-nearest-merge.mdl diff --git a/mdl-examples/bug-tests/326-describer-pair-split-with-nearest-merge.mdl b/mdl-examples/bug-tests/326-describer-pair-split-with-nearest-merge.mdl new file mode 100644 index 00000000..da437382 --- /dev/null +++ b/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; +/ From dc6231e67907d91661c5aecbc0e407a442741cb9 Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Mon, 27 Apr 2026 08:29:38 +0200 Subject: [PATCH 3/3] refactor: drop unused collectReachableDistances parameters The ctx and activityMap parameters were carried over from a sibling helper but never used by the BFS traversal. Removes them and the matching `_ = ctx; _ = activityMap` discard lines, addressing AI review feedback on PR #327. Co-Authored-By: Claude Opus 4.7 --- mdl/executor/cmd_microflows_show.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/mdl/executor/cmd_microflows_show.go b/mdl/executor/cmd_microflows_show.go index 7a72e87c..47a4963e 100644 --- a/mdl/executor/cmd_microflows_show.go +++ b/mdl/executor/cmd_microflows_show.go @@ -788,7 +788,7 @@ func findMergeForSplit( branchDistances := make([]map[model.ID]int, 0, len(flows)) for _, flow := range flows { - branchDistances = append(branchDistances, collectReachableDistances(ctx, flow.DestinationID, flowsByOrigin, activityMap)) + branchDistances = append(branchDistances, collectReachableDistances(flow.DestinationID, flowsByOrigin)) } return selectNearestCommonMerge(activityMap, branchDistances) @@ -825,14 +825,9 @@ func collectReachableNodes( // branch start to every reachable node. Error handler flows are excluded because // they do not participate in split/merge structural pairing. func collectReachableDistances( - ctx *ExecContext, startID model.ID, flowsByOrigin map[model.ID][]*microflows.SequenceFlow, - activityMap map[model.ID]microflows.MicroflowObject, ) map[model.ID]int { - _ = ctx - _ = activityMap - distances := map[model.ID]int{} type queueItem struct { id model.ID