Skip to content

Commit ac3c2fc

Browse files
committed
fix: stop nested splits at parent merge joins
Symptom: describe could treat a nested split's downstream common tail as the nested merge while the parent branch was supposed to stop at an earlier merge. The emitted MDL moved shared continuation into an else branch, which could leave return-valued microflows with a path lacking a return. Root cause: nested traversal trusted the precomputed split merge even when that join was reachable only after the parent merge. The empty-then swap also ran against that over-wide join. Fix: resolve nested split merges against the parent merge when the computed join is downstream, and only apply the empty-then swap when the resolved nested merge is the active parent merge. Tests: added synthetic regression coverage for downstream-vs-local nested merge resolution and ran make build && make test.
1 parent 3e384be commit ac3c2fc

2 files changed

Lines changed: 99 additions & 1 deletion

File tree

mdl/executor/cmd_microflows_show_helpers.go

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -783,10 +783,11 @@ func traverseFlowUntilMerge(
783783
nestedMergeID := splitMergeMap[currentID]
784784

785785
trueFlow, falseFlow := findBranchFlows(flows)
786+
nestedMergeID = resolveNestedMergeID(nestedMergeID, mergeID, trueFlow, falseFlow, flowsByOrigin)
786787

787788
// Empty-then swap: negate when true branch is empty but false branch has content.
788789
// Skip when both branches go directly to merge (both empty).
789-
if trueFlow != nil && falseFlow != nil && nestedMergeID != "" {
790+
if trueFlow != nil && falseFlow != nil && nestedMergeID != "" && nestedMergeID == mergeID {
790791
if trueFlow.DestinationID == nestedMergeID && falseFlow.DestinationID != nestedMergeID {
791792
stmt = negateIfCondition(stmt)
792793
trueFlow, falseFlow = falseFlow, trueFlow
@@ -938,6 +939,63 @@ func continueAfterNestedSplitJoin(
938939
traverseFlowUntilMerge(ctx, joinID, parentMergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
939940
}
940941

942+
func resolveNestedMergeID(
943+
nestedMergeID model.ID,
944+
parentMergeID model.ID,
945+
trueFlow *microflows.SequenceFlow,
946+
falseFlow *microflows.SequenceFlow,
947+
flowsByOrigin map[model.ID][]*microflows.SequenceFlow,
948+
) model.ID {
949+
if nestedMergeID != "" && parentMergeID != "" && nestedMergeID != parentMergeID &&
950+
canReachNode(parentMergeID, nestedMergeID, flowsByOrigin, make(map[model.ID]bool)) {
951+
for _, flow := range []*microflows.SequenceFlow{trueFlow, falseFlow} {
952+
if flow == nil {
953+
continue
954+
}
955+
if flow.DestinationID == parentMergeID ||
956+
canReachNode(flow.DestinationID, parentMergeID, flowsByOrigin, make(map[model.ID]bool)) {
957+
return parentMergeID
958+
}
959+
}
960+
}
961+
if nestedMergeID != "" || parentMergeID == "" {
962+
return nestedMergeID
963+
}
964+
for _, flow := range []*microflows.SequenceFlow{trueFlow, falseFlow} {
965+
if flow == nil {
966+
continue
967+
}
968+
if canReachNode(flow.DestinationID, parentMergeID, flowsByOrigin, make(map[model.ID]bool)) {
969+
return parentMergeID
970+
}
971+
}
972+
return ""
973+
}
974+
975+
func canReachNode(
976+
currentID model.ID,
977+
targetID model.ID,
978+
flowsByOrigin map[model.ID][]*microflows.SequenceFlow,
979+
visited map[model.ID]bool,
980+
) bool {
981+
if currentID == "" {
982+
return false
983+
}
984+
if currentID == targetID {
985+
return true
986+
}
987+
if visited[currentID] {
988+
return false
989+
}
990+
visited[currentID] = true
991+
for _, flow := range findNormalFlows(flowsByOrigin[currentID]) {
992+
if canReachNode(flow.DestinationID, targetID, flowsByOrigin, visited) {
993+
return true
994+
}
995+
}
996+
return false
997+
}
998+
941999
// traverseLoopBody traverses activities inside a loop body.
9421000
// When sourceMap is non-nil, it also records line ranges for each activity node.
9431001
func traverseLoopBody(

mdl/executor/cmd_microflows_traverse_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,46 @@ func TestTraverseFlow_NestedTerminalGuardBranchSuppressesEmptyOuterElse(t *testi
654654
}
655655
}
656656

657+
func TestResolveNestedMergeID_UsesParentMergeBeforeDownstreamJoin(t *testing.T) {
658+
flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{
659+
mkID("parent_merge"): {mkFlow("parent_merge", "downstream_join")},
660+
}
661+
trueFlow := mkFlow("nested_split", "parent_merge")
662+
falseFlow := mkFlow("nested_split", "false_branch")
663+
664+
got := resolveNestedMergeID(
665+
mkID("downstream_join"),
666+
mkID("parent_merge"),
667+
trueFlow,
668+
falseFlow,
669+
flowsByOrigin,
670+
)
671+
672+
if got != mkID("parent_merge") {
673+
t.Fatalf("nested split used downstream join %q, want parent merge %q", got, mkID("parent_merge"))
674+
}
675+
}
676+
677+
func TestResolveNestedMergeID_KeepsIndependentNestedJoin(t *testing.T) {
678+
flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{
679+
mkID("nested_join"): {mkFlow("nested_join", "parent_merge")},
680+
}
681+
trueFlow := mkFlow("nested_split", "true_branch")
682+
falseFlow := mkFlow("nested_split", "nested_join")
683+
684+
got := resolveNestedMergeID(
685+
mkID("nested_join"),
686+
mkID("parent_merge"),
687+
trueFlow,
688+
falseFlow,
689+
flowsByOrigin,
690+
)
691+
692+
if got != mkID("nested_join") {
693+
t.Fatalf("nested split merge changed to %q, want local nested join %q", got, mkID("nested_join"))
694+
}
695+
}
696+
657697
// =============================================================================
658698
// collectErrorHandlerStatements
659699
// =============================================================================

0 commit comments

Comments
 (0)