Skip to content

Commit d6f100c

Browse files
hjothamendixhjothamendix
committed
fix: keep split continuations after structural joins
Symptom: describing nested split graphs could duplicate a shared continuation tail inside multiple branch bodies. Re-executing the MDL produced duplicate output-variable declarations and model consistency errors. Root cause: split-join discovery required a join to be reachable from every branch, so partially terminal enum splits lost their shared continuation. It also accepted early joins that nested branches could bypass before reaching the real downstream shared tail. Fix: allow nearest joins reached by multiple continuing branches when no all-branch join exists, continue enum splits through the same split-join helpers as IF splits, and reject join candidates that have downstream non-terminal objects reachable while bypassing the candidate. Tests: go test ./mdl/executor; make build; make lint-go; make test.
1 parent aa94970 commit d6f100c

3 files changed

Lines changed: 232 additions & 13 deletions

File tree

‎mdl/executor/cmd_microflows_show.go‎

Lines changed: 127 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -891,11 +891,13 @@ func findMergeForSplit(
891891
}
892892

893893
branchDistances := make([]map[model.ID]int, 0, len(flows))
894+
branchStarts := make([]model.ID, 0, len(flows))
894895
for _, flow := range flows {
896+
branchStarts = append(branchStarts, flow.DestinationID)
895897
branchDistances = append(branchDistances, collectReachableDistances(flow.DestinationID, flowsByOrigin))
896898
}
897899

898-
return selectNearestCommonJoin(activityMap, branchDistances)
900+
return selectNearestCommonJoin(activityMap, flowsByOrigin, branchStarts, branchDistances)
899901
}
900902

901903
// collectReachableDistances collects the shortest normal-flow distance from a
@@ -934,6 +936,8 @@ func collectReachableDistances(
934936

935937
func selectNearestCommonJoin(
936938
activityMap map[model.ID]microflows.MicroflowObject,
939+
flowsByOrigin map[model.ID][]*microflows.SequenceFlow,
940+
branchStarts []model.ID,
937941
branchDistances []map[model.ID]int,
938942
) model.ID {
939943
if len(branchDistances) < 2 {
@@ -942,6 +946,7 @@ func selectNearestCommonJoin(
942946

943947
type candidate struct {
944948
id model.ID
949+
reachCount int
945950
maxDistance int
946951
sumDistance int
947952
}
@@ -969,17 +974,56 @@ func selectNearestCommonJoin(
969974
if common {
970975
candidates = append(candidates, candidate{
971976
id: nodeID,
977+
reachCount: len(branchDistances),
972978
maxDistance: maxDistance,
973979
sumDistance: sumDistance,
974980
})
975981
}
976982
}
977983

984+
if len(candidates) == 0 {
985+
byNode := map[model.ID]candidate{}
986+
for _, distances := range branchDistances {
987+
for nodeID, distance := range distances {
988+
if !isSplitJoinCandidate(activityMap[nodeID]) {
989+
continue
990+
}
991+
c := byNode[nodeID]
992+
c.id = nodeID
993+
c.reachCount++
994+
if distance > c.maxDistance {
995+
c.maxDistance = distance
996+
}
997+
c.sumDistance += distance
998+
byNode[nodeID] = c
999+
}
1000+
}
1001+
for _, c := range byNode {
1002+
if c.reachCount >= 2 {
1003+
candidates = append(candidates, c)
1004+
}
1005+
}
1006+
}
1007+
1008+
if len(candidates) == 0 {
1009+
return ""
1010+
}
1011+
1012+
filtered := candidates[:0]
1013+
for _, candidate := range candidates {
1014+
if splitJoinCandidateDoesNotHaveDownstreamBypass(candidate.id, activityMap, flowsByOrigin, branchStarts) {
1015+
filtered = append(filtered, candidate)
1016+
}
1017+
}
1018+
candidates = filtered
9781019
if len(candidates) == 0 {
9791020
return ""
9801021
}
9811022

9821023
sort.Slice(candidates, func(i, j int) bool {
1024+
if candidates[i].reachCount != candidates[j].reachCount {
1025+
return candidates[i].reachCount > candidates[j].reachCount
1026+
}
9831027
if candidates[i].maxDistance != candidates[j].maxDistance {
9841028
return candidates[i].maxDistance < candidates[j].maxDistance
9851029
}
@@ -992,6 +1036,88 @@ func selectNearestCommonJoin(
9921036
return candidates[0].id
9931037
}
9941038

1039+
func splitJoinCandidateDoesNotHaveDownstreamBypass(
1040+
candidateID model.ID,
1041+
activityMap map[model.ID]microflows.MicroflowObject,
1042+
flowsByOrigin map[model.ID][]*microflows.SequenceFlow,
1043+
branchStarts []model.ID,
1044+
) bool {
1045+
downstream := collectReachableNonTerminalObjects(candidateID, activityMap, flowsByOrigin)
1046+
if len(downstream) == 0 {
1047+
return true
1048+
}
1049+
for _, startID := range branchStarts {
1050+
if startID == candidateID {
1051+
continue
1052+
}
1053+
if reachesAnyObjectAvoiding(startID, downstream, candidateID, activityMap, flowsByOrigin, map[model.ID]bool{}) {
1054+
return false
1055+
}
1056+
}
1057+
return true
1058+
}
1059+
1060+
func collectReachableNonTerminalObjects(
1061+
startID model.ID,
1062+
activityMap map[model.ID]microflows.MicroflowObject,
1063+
flowsByOrigin map[model.ID][]*microflows.SequenceFlow,
1064+
) map[model.ID]bool {
1065+
result := map[model.ID]bool{}
1066+
var walk func(model.ID)
1067+
visited := map[model.ID]bool{startID: true}
1068+
walk = func(currentID model.ID) {
1069+
if visited[currentID] {
1070+
return
1071+
}
1072+
visited[currentID] = true
1073+
if isNonTerminalMicroflowObject(activityMap[currentID]) {
1074+
result[currentID] = true
1075+
}
1076+
for _, flow := range findNormalFlows(flowsByOrigin[currentID]) {
1077+
walk(flow.DestinationID)
1078+
}
1079+
}
1080+
for _, flow := range findNormalFlows(flowsByOrigin[startID]) {
1081+
walk(flow.DestinationID)
1082+
}
1083+
return result
1084+
}
1085+
1086+
func reachesAnyObjectAvoiding(
1087+
currentID model.ID,
1088+
targets map[model.ID]bool,
1089+
avoidID model.ID,
1090+
activityMap map[model.ID]microflows.MicroflowObject,
1091+
flowsByOrigin map[model.ID][]*microflows.SequenceFlow,
1092+
visited map[model.ID]bool,
1093+
) bool {
1094+
if currentID == "" || currentID == avoidID || visited[currentID] {
1095+
return false
1096+
}
1097+
if targets[currentID] {
1098+
return true
1099+
}
1100+
if !isNonTerminalMicroflowObject(activityMap[currentID]) {
1101+
return false
1102+
}
1103+
visited[currentID] = true
1104+
for _, flow := range findNormalFlows(flowsByOrigin[currentID]) {
1105+
if reachesAnyObjectAvoiding(flow.DestinationID, targets, avoidID, activityMap, flowsByOrigin, visited) {
1106+
return true
1107+
}
1108+
}
1109+
return false
1110+
}
1111+
1112+
func isNonTerminalMicroflowObject(obj microflows.MicroflowObject) bool {
1113+
switch obj.(type) {
1114+
case nil, *microflows.StartEvent, *microflows.EndEvent, *microflows.ErrorEvent:
1115+
return false
1116+
default:
1117+
return true
1118+
}
1119+
}
1120+
9951121
func isSplitJoinCandidate(obj microflows.MicroflowObject) bool {
9961122
switch obj.(type) {
9971123
case nil, *microflows.StartEvent, *microflows.EndEvent:

‎mdl/executor/cmd_microflows_show_helpers.go‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -633,12 +633,7 @@ func traverseFlow(
633633
emitObjectAnnotations(obj, lines, indentStr, annotationsByTarget, flowsByOrigin, flowsByDest, activityMap)
634634
emitEnumSplitStatement(ctx, currentID, mergeID, variable, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
635635
recordSourceMap(sourceMap, currentID, startLine, len(*lines)+headerLineCount-1)
636-
if mergeID != "" {
637-
visited[mergeID] = true
638-
for _, flow := range flowsByOrigin[mergeID] {
639-
traverseFlow(ctx, flow.DestinationID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
640-
}
641-
}
636+
continueAfterSplitJoin(ctx, mergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
642637
return
643638
}
644639
if stmt != "" {
@@ -796,12 +791,7 @@ func traverseFlowUntilMerge(
796791
emitObjectAnnotations(obj, lines, indentStr, annotationsByTarget, flowsByOrigin, flowsByDest, activityMap)
797792
emitEnumSplitStatement(ctx, currentID, nestedMergeID, variable, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
798793
recordSourceMap(sourceMap, currentID, startLine, len(*lines)+headerLineCount-1)
799-
if nestedMergeID != "" && nestedMergeID != mergeID {
800-
visited[nestedMergeID] = true
801-
for _, flow := range flowsByOrigin[nestedMergeID] {
802-
traverseFlowUntilMerge(ctx, flow.DestinationID, mergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
803-
}
804-
}
794+
continueAfterNestedSplitJoin(ctx, nestedMergeID, mergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
805795
return
806796
}
807797
if stmt != "" {

‎mdl/executor/cmd_microflows_traverse_test.go‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,109 @@ func TestTraverseFlow_CommonActivityJoinKeepsTailOutsideBranches(t *testing.T) {
493493
}
494494
}
495495

496+
func TestTraverseFlow_EnumSplitPartialJoinKeepsSharedTailOutsideCases(t *testing.T) {
497+
e := newTestExecutor()
498+
499+
logAction := func(id, message string) *microflows.ActionActivity {
500+
return &microflows.ActionActivity{
501+
BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj(id)},
502+
Action: &microflows.LogMessageAction{
503+
LogLevel: "Info",
504+
LogNodeName: "'Synthetic'",
505+
MessageTemplate: &model.Text{Translations: map[string]string{"en_US": message}},
506+
},
507+
}
508+
}
509+
510+
activityMap := map[model.ID]microflows.MicroflowObject{
511+
mkID("start"): &microflows.StartEvent{BaseMicroflowObject: mkObj("start")},
512+
mkID("kind_split"): &microflows.ExclusiveSplit{
513+
BaseMicroflowObject: mkObj("kind_split"),
514+
SplitCondition: &microflows.ExpressionSplitCondition{Expression: "$Kind"},
515+
},
516+
mkID("case_a"): logAction("case_a", "case A"),
517+
mkID("case_b"): logAction("case_b", "case B"),
518+
mkID("case_c"): logAction("case_c", "case C terminal"),
519+
mkID("shared_tail"): logAction("shared_tail", "shared tail"),
520+
mkID("end"): &microflows.EndEvent{BaseMicroflowObject: mkObj("end")},
521+
}
522+
flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{
523+
mkID("start"): {mkFlow("start", "kind_split")},
524+
mkID("kind_split"): {
525+
mkBranchFlow("kind_split", "case_a", microflows.EnumerationCase{Value: "A"}),
526+
mkBranchFlow("kind_split", "case_b", microflows.EnumerationCase{Value: "B"}),
527+
mkBranchFlow("kind_split", "case_c", microflows.EnumerationCase{Value: "C"}),
528+
},
529+
mkID("case_a"): {mkFlow("case_a", "shared_tail")},
530+
mkID("case_b"): {mkFlow("case_b", "shared_tail")},
531+
mkID("case_c"): {mkFlow("case_c", "end")},
532+
mkID("shared_tail"): {mkFlow("shared_tail", "end")},
533+
}
534+
535+
joinID := findMergeForSplit(nil, mkID("kind_split"), flowsByOrigin, activityMap)
536+
if joinID != mkID("shared_tail") {
537+
t.Fatalf("enum split paired with %q, want shared tail %q", joinID, mkID("shared_tail"))
538+
}
539+
540+
splitMergeMap := map[model.ID]model.ID{mkID("kind_split"): joinID}
541+
var lines []string
542+
visited := make(map[model.ID]bool)
543+
e.traverseFlow(mkID("start"), activityMap, flowsByOrigin, splitMergeMap, visited, nil, nil, &lines, 0, nil, 0, nil)
544+
545+
out := strings.Join(lines, "\n")
546+
if got := strings.Count(out, "shared tail"); got != 1 {
547+
t.Fatalf("shared tail emitted %d times, want once:\n%s", got, out)
548+
}
549+
endCase := strings.Index(out, "end case;")
550+
sharedTail := strings.Index(out, "shared tail")
551+
if endCase == -1 || sharedTail == -1 || endCase > sharedTail {
552+
t.Fatalf("shared tail must be emitted after enum split closes:\n%s", out)
553+
}
554+
}
555+
556+
func TestFindMergeForSplit_SkipsJoinBypassedByNestedBranch(t *testing.T) {
557+
activityMap := map[model.ID]microflows.MicroflowObject{
558+
mkID("outer_split"): &microflows.ExclusiveSplit{
559+
BaseMicroflowObject: mkObj("outer_split"),
560+
SplitCondition: &microflows.ExpressionSplitCondition{Expression: "$HasExisting"},
561+
},
562+
mkID("inner_split"): &microflows.ExclusiveSplit{
563+
BaseMicroflowObject: mkObj("inner_split"),
564+
SplitCondition: &microflows.ExpressionSplitCondition{Expression: "$CanUpdate"},
565+
},
566+
mkID("early_join"): &microflows.ExclusiveMerge{BaseMicroflowObject: mkObj("early_join")},
567+
mkID("clear"): &microflows.ActionActivity{
568+
BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("clear")},
569+
Action: &microflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'Synthetic'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "clear"}}},
570+
},
571+
mkID("shared_tail"): &microflows.ExclusiveMerge{BaseMicroflowObject: mkObj("shared_tail")},
572+
mkID("retrieve"): &microflows.ActionActivity{
573+
BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("retrieve")},
574+
Action: &microflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'Synthetic'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "retrieve"}}},
575+
},
576+
mkID("end"): &microflows.EndEvent{BaseMicroflowObject: mkObj("end")},
577+
}
578+
flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{
579+
mkID("outer_split"): {
580+
mkBranchFlow("outer_split", "early_join", &microflows.ExpressionCase{Expression: "true"}),
581+
mkBranchFlow("outer_split", "inner_split", &microflows.ExpressionCase{Expression: "false"}),
582+
},
583+
mkID("inner_split"): {
584+
mkBranchFlow("inner_split", "shared_tail", &microflows.ExpressionCase{Expression: "true"}),
585+
mkBranchFlow("inner_split", "early_join", &microflows.ExpressionCase{Expression: "false"}),
586+
},
587+
mkID("early_join"): {mkFlow("early_join", "clear")},
588+
mkID("clear"): {mkFlow("clear", "shared_tail")},
589+
mkID("shared_tail"): {mkFlow("shared_tail", "retrieve")},
590+
mkID("retrieve"): {mkFlow("retrieve", "end")},
591+
}
592+
593+
joinID := findMergeForSplit(nil, mkID("outer_split"), flowsByOrigin, activityMap)
594+
if joinID != mkID("shared_tail") {
595+
t.Fatalf("outer split paired with %q, want downstream shared tail %q", joinID, mkID("shared_tail"))
596+
}
597+
}
598+
496599
func TestTraverseFlow_SequentialIfWithoutElseKeepsContinuationOutsideFirstIf(t *testing.T) {
497600
e := newTestExecutor()
498601

0 commit comments

Comments
 (0)