From 9f20a8787035a830abe30cd008698125095e2e64 Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Sun, 26 Apr 2026 17:27:12 +0200 Subject: [PATCH 1/4] 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 | 104 +++++++++++++++++++ 2 files changed, 174 insertions(+), 8 deletions(-) diff --git a/mdl/executor/cmd_microflows_show_helpers.go b/mdl/executor/cmd_microflows_show_helpers.go index 3286eae3..94dd555e 100644 --- a/mdl/executor/cmd_microflows_show_helpers.go +++ b/mdl/executor/cmd_microflows_show_helpers.go @@ -995,6 +995,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) @@ -1005,9 +1006,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) } } } @@ -1022,9 +1023,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) } } } @@ -1042,10 +1043,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 } } @@ -1058,6 +1061,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 ba60da11..8e9f4223 100644 --- a/mdl/executor/cmd_microflows_traverse_test.go +++ b/mdl/executor/cmd_microflows_traverse_test.go @@ -183,6 +183,110 @@ func TestTraverseFlow_IfWithoutElse(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 35d2218f462977f0bf48f53d3b340f3bb56027de Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Mon, 27 Apr 2026 03:11:46 +0200 Subject: [PATCH 2/4] 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; +/ From 68718a717ebfa1ef0a54809e6c52e66bf7a95072 Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Tue, 28 Apr 2026 13:57:05 +0200 Subject: [PATCH 3/4] fix: reuse structured traversal inside loop bodies Symptom: loop bodies that contained nested decision flows could describe the body as a flat sequence and lose the structured `else` / `end if` shape needed for a valid roundtrip. Root cause: loop-body traversal had a separate simplified walker that did not build a loop-local split-to-merge map before walking nested branches. Fix: extract split/merge discovery for an arbitrary graph and reuse the main structured traversal inside loop bodies. Tests: added synthetic loop-body coverage for an IF/ELSE split inside a loop object collection. --- mdl/executor/cmd_microflows_show.go | 17 +- mdl/executor/cmd_microflows_show_helpers.go | 163 +------------------ mdl/executor/cmd_microflows_traverse_test.go | 49 ++++++ 3 files changed, 67 insertions(+), 162 deletions(-) diff --git a/mdl/executor/cmd_microflows_show.go b/mdl/executor/cmd_microflows_show.go index 55e26373..3adf1fc0 100644 --- a/mdl/executor/cmd_microflows_show.go +++ b/mdl/executor/cmd_microflows_show.go @@ -844,16 +844,25 @@ func findSplitMergePoints( oc *microflows.MicroflowObjectCollection, activityMap map[model.ID]microflows.MicroflowObject, ) map[model.ID]model.ID { - result := make(map[model.ID]model.ID) - // Build flow graph for forward traversal flowsByOrigin := make(map[model.ID][]*microflows.SequenceFlow) for _, flow := range oc.Flows { flowsByOrigin[flow.OriginID] = append(flowsByOrigin[flow.OriginID], flow) } - // For each ExclusiveSplit, find its merge point - for _, obj := range oc.Objects { + return findSplitMergePointsForGraph(ctx, activityMap, flowsByOrigin) +} + +// findSplitMergePointsForGraph finds the corresponding merge point for each +// split in an already materialized flow graph. Nested traversals such as loop +// bodies use this because they do not have a top-level object collection. +func findSplitMergePointsForGraph( + ctx *ExecContext, + activityMap map[model.ID]microflows.MicroflowObject, + flowsByOrigin map[model.ID][]*microflows.SequenceFlow, +) map[model.ID]model.ID { + result := make(map[model.ID]model.ID) + for _, obj := range activityMap { if _, ok := obj.(*microflows.ExclusiveSplit); ok { splitID := obj.GetID() // Find merge by following both branches until they converge diff --git a/mdl/executor/cmd_microflows_show_helpers.go b/mdl/executor/cmd_microflows_show_helpers.go index 94dd555e..582aa9fb 100644 --- a/mdl/executor/cmd_microflows_show_helpers.go +++ b/mdl/executor/cmd_microflows_show_helpers.go @@ -811,164 +811,11 @@ func traverseLoopBody( headerLineCount int, annotationsByTarget map[model.ID][]string, ) { - if currentID == "" || visited[currentID] { - return - } - - obj := activityMap[currentID] - if obj == nil { - return - } - - visited[currentID] = true - - stmt := formatActivity(ctx, obj, entityNames, microflowNames) - indentStr := strings.Repeat(" ", indent) - - // Handle ExclusiveSplit (if/else) inside loop body - if _, isSplit := obj.(*microflows.ExclusiveSplit); isSplit { - startLine := len(*lines) + headerLineCount - - flows := flowsByOrigin[currentID] - mergeID := splitMergeMap[currentID] - - trueFlow, falseFlow := findBranchFlows(flows) - - // Empty-then swap: negate when true branch is empty but false branch has content. - if trueFlow != nil && falseFlow != nil && mergeID != "" { - if trueFlow.DestinationID == mergeID && falseFlow.DestinationID != mergeID { - stmt = negateIfCondition(stmt) - trueFlow, falseFlow = falseFlow, trueFlow - } - } - - if stmt != "" { - emitObjectAnnotations(obj, lines, indentStr, annotationsByTarget, flowsByOrigin, flowsByDest) - *lines = append(*lines, indentStr+stmt) - } - - // Guard pattern: true branch is a single EndEvent/BreakEvent/ContinueEvent - isGuard := false - if trueFlow != nil { - trueTarget := activityMap[trueFlow.DestinationID] - switch trueTarget.(type) { - case *microflows.EndEvent, *microflows.BreakEvent, *microflows.ContinueEvent: - isGuard = true - if falseFlow != nil { - falseTarget := activityMap[falseFlow.DestinationID] - switch falseTarget.(type) { - case *microflows.EndEvent, *microflows.BreakEvent, *microflows.ContinueEvent: - isGuard = false - } - } - } - } - - if isGuard { - // Emit true branch (the guard action) - traverseLoopBody(ctx, trueFlow.DestinationID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent+1, sourceMap, headerLineCount, annotationsByTarget) - *lines = append(*lines, indentStr+"end if;") - recordSourceMap(sourceMap, currentID, startLine, len(*lines)+headerLineCount-1) - - // Continue from the false branch - if falseFlow != nil { - contID := falseFlow.DestinationID - if _, isMerge := activityMap[contID].(*microflows.ExclusiveMerge); isMerge { - visited[contID] = true - for _, flow := range flowsByOrigin[contID] { - contID = flow.DestinationID - break - } - } - traverseLoopBody(ctx, contID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) - } - } else { - // Normal if/else - if trueFlow != nil { - traverseFlowUntilMerge(ctx, trueFlow.DestinationID, mergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent+1, sourceMap, headerLineCount, annotationsByTarget) - } - - if falseFlow != nil { - elseLineIdx := len(*lines) - *lines = append(*lines, indentStr+"else") - visitedFalseBranch := make(map[model.ID]bool) - for id := range visited { - visitedFalseBranch[id] = true - } - traverseFlowUntilMerge(ctx, falseFlow.DestinationID, mergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visitedFalseBranch, entityNames, microflowNames, lines, indent+1, sourceMap, headerLineCount, annotationsByTarget) - // Remove empty else block - if len(*lines) == elseLineIdx+1 { - *lines = (*lines)[:elseLineIdx] - } - } - - *lines = append(*lines, indentStr+"end if;") - recordSourceMap(sourceMap, currentID, startLine, len(*lines)+headerLineCount-1) - - // Continue after merge within the loop body - if mergeID != "" { - visited[mergeID] = true - nextFlows := flowsByOrigin[mergeID] - for _, flow := range nextFlows { - if _, inLoop := activityMap[flow.DestinationID]; inLoop { - traverseLoopBody(ctx, flow.DestinationID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) - } - } - } - } - return - } - - // Handle nested LoopedActivity specially - if loop, isLoop := obj.(*microflows.LoopedActivity); isLoop { - startLine := len(*lines) + headerLineCount - if stmt != "" { - emitObjectAnnotations(obj, lines, indentStr, annotationsByTarget, flowsByOrigin, flowsByDest) - *lines = append(*lines, indentStr+stmt) - } - - *lines = append(*lines, indentStr+"begin") - emitLoopBody(ctx, loop, flowsByOrigin, flowsByDest, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) - - *lines = append(*lines, indentStr+loopEndKeyword(loop)+";") - recordSourceMap(sourceMap, currentID, startLine, len(*lines)+headerLineCount-1) - - // Continue after the nested loop within the parent loop body - flows := flowsByOrigin[currentID] - for _, flow := range flows { - if _, inLoop := activityMap[flow.DestinationID]; inLoop { - traverseLoopBody(ctx, flow.DestinationID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) - } - } - return - } - - // Handle ExclusiveMerge — traverse through without outputting anything - if _, isMerge := obj.(*microflows.ExclusiveMerge); isMerge { - if isMergePairedWithSplit(currentID, splitMergeMap) { - return - } - flows := flowsByOrigin[currentID] - for _, flow := range flows { - if _, inLoop := activityMap[flow.DestinationID]; inLoop { - traverseLoopBody(ctx, flow.DestinationID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) - } - } - return - } - - // Regular activity - startLine := len(*lines) + headerLineCount - normalFlows := findNormalFlows(flowsByOrigin[currentID]) - emitActivityStatement(ctx, obj, stmt, flowsByOrigin, flowsByDest, activityMap, entityNames, microflowNames, lines, indentStr, annotationsByTarget) - recordSourceMap(sourceMap, currentID, startLine, len(*lines)+headerLineCount-1) - - // Follow normal (non-error-handler) outgoing flows within the loop body - for _, flow := range normalFlows { - if _, inLoop := activityMap[flow.DestinationID]; inLoop { - traverseLoopBody(ctx, flow.DestinationID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) - } - } + // Loop bodies can contain the same structured control flow as top-level + // microflows. Reuse the main traversal with a loop-local split/merge map so + // nested IF/ELSE blocks emit `else` / `end if;` correctly. + loopSplitMergeMap := findSplitMergePointsForGraph(ctx, activityMap, flowsByOrigin) + traverseFlow(ctx, currentID, activityMap, flowsByOrigin, flowsByDest, loopSplitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) } // emitLoopBody processes the inner objects of a LoopedActivity. diff --git a/mdl/executor/cmd_microflows_traverse_test.go b/mdl/executor/cmd_microflows_traverse_test.go index 8e9f4223..159a10c5 100644 --- a/mdl/executor/cmd_microflows_traverse_test.go +++ b/mdl/executor/cmd_microflows_traverse_test.go @@ -287,6 +287,55 @@ func TestTraverseFlow_LoopBodyMergesParentFlowsForExistingOrigin(t *testing.T) { } } +func TestTraverseFlow_LoopBodyEmitsStructuredIfElse(t *testing.T) { + split := µflows.ExclusiveSplit{ + BaseMicroflowObject: mkObj("split"), + SplitCondition: µflows.ExpressionSplitCondition{Expression: "$Item/IsActive"}, + } + trueLog := µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("true_log")}, + Action: µflows.LogMessageAction{ + LogLevel: "Info", + LogNodeName: "'App'", + MessageTemplate: &model.Text{ + Translations: map[string]string{"en_US": "active"}, + }, + }, + } + falseLog := µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("false_log")}, + Action: µflows.LogMessageAction{ + LogLevel: "Info", + LogNodeName: "'App'", + MessageTemplate: &model.Text{ + Translations: map[string]string{"en_US": "inactive"}, + }, + }, + } + merge := µflows.ExclusiveMerge{BaseMicroflowObject: mkObj("merge")} + loop := µflows.LoopedActivity{ + BaseMicroflowObject: mkObj("loop"), + ObjectCollection: µflows.MicroflowObjectCollection{ + Objects: []microflows.MicroflowObject{split, trueLog, falseLog, merge}, + Flows: []*microflows.SequenceFlow{ + mkBranchFlow("split", "true_log", µflows.ExpressionCase{Expression: "true"}), + mkBranchFlow("split", "false_log", µflows.ExpressionCase{Expression: "false"}), + mkFlow("true_log", "merge"), + mkFlow("false_log", "merge"), + }, + }, + } + + var lines []string + emitLoopBody(nil, loop, nil, nil, nil, nil, &lines, 0, nil, 0, nil) + out := strings.Join(lines, "\n") + for _, want := range []string{"if $Item/IsActive then", "else", "end if;"} { + if !strings.Contains(out, want) { + t.Fatalf("loop body should emit structured %q, got:\n%s", want, out) + } + } +} + // ============================================================================= // collectErrorHandlerStatements // ============================================================================= From e9a004cbd289777875810d41b0638fc14e83ba94 Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Thu, 30 Apr 2026 09:19:09 +0200 Subject: [PATCH 4/4] fix: include CaseValue in sequenceFlowIdentity composite key The composite fallback used when flow.ID is empty omitted CaseValue, so two split branches with the same origin/destination but different case values were collapsed by the loop-body merge. Production flows always have UUIDs so this only manifests in test-constructed graphs, but the fix is one extra %v in the format string and prevents the class of bug entirely. Addresses ako's review on PR #329. Co-Authored-By: Claude Opus 4.7 --- mdl/executor/cmd_microflows_show_helpers.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/mdl/executor/cmd_microflows_show_helpers.go b/mdl/executor/cmd_microflows_show_helpers.go index 582aa9fb..a2ac9805 100644 --- a/mdl/executor/cmd_microflows_show_helpers.go +++ b/mdl/executor/cmd_microflows_show_helpers.go @@ -942,6 +942,13 @@ func appendSequenceFlowIfMissing(flows []*microflows.SequenceFlow, candidate *mi return append(flows, candidate) } +// sequenceFlowIdentity returns a stable key used to deduplicate flows when +// merging the parent graph into a loop's local map. Production data always +// carries a UUID, so the ID branch is the common path. The composite fallback +// covers test helpers and any other call site that constructs flows without +// IDs — including CaseValue prevents two split branches with the same +// origin/destination but different case values from being mistakenly +// deduplicated. func sequenceFlowIdentity(flow *microflows.SequenceFlow) string { if flow == nil { return "" @@ -949,7 +956,7 @@ func sequenceFlowIdentity(flow *microflows.SequenceFlow) string { 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) + return fmt.Sprintf("%s>%s:%t:%d:%d:%v", flow.OriginID, flow.DestinationID, flow.IsErrorHandler, flow.OriginConnectionIndex, flow.DestinationConnectionIndex, flow.CaseValue) } func preferLoopBodyStart(candidate, current microflows.MicroflowObject) bool {