diff --git a/mdl/executor/cmd_microflows_show.go b/mdl/executor/cmd_microflows_show.go index 7ed24b7a..95b01dea 100644 --- a/mdl/executor/cmd_microflows_show.go +++ b/mdl/executor/cmd_microflows_show.go @@ -982,11 +982,13 @@ func findMergeForSplit( } branchDistances := make([]map[model.ID]int, 0, len(flows)) + branchStarts := make([]model.ID, 0, len(flows)) for _, flow := range flows { + branchStarts = append(branchStarts, flow.DestinationID) branchDistances = append(branchDistances, collectReachableDistances(flow.DestinationID, flowsByOrigin)) } - return selectNearestCommonJoin(activityMap, branchDistances) + return selectNearestCommonJoin(activityMap, flowsByOrigin, branchStarts, branchDistances) } // collectReachableDistances collects the shortest normal-flow distance from a @@ -1025,6 +1027,8 @@ func collectReachableDistances( func selectNearestCommonJoin( activityMap map[model.ID]microflows.MicroflowObject, + flowsByOrigin map[model.ID][]*microflows.SequenceFlow, + branchStarts []model.ID, branchDistances []map[model.ID]int, ) model.ID { if len(branchDistances) < 2 { @@ -1033,6 +1037,7 @@ func selectNearestCommonJoin( type candidate struct { id model.ID + reachCount int maxDistance int sumDistance int } @@ -1060,17 +1065,56 @@ func selectNearestCommonJoin( if common { candidates = append(candidates, candidate{ id: nodeID, + reachCount: len(branchDistances), maxDistance: maxDistance, sumDistance: sumDistance, }) } } + if len(candidates) == 0 { + byNode := map[model.ID]candidate{} + for _, distances := range branchDistances { + for nodeID, distance := range distances { + if !isSplitJoinCandidate(activityMap[nodeID]) { + continue + } + c := byNode[nodeID] + c.id = nodeID + c.reachCount++ + if distance > c.maxDistance { + c.maxDistance = distance + } + c.sumDistance += distance + byNode[nodeID] = c + } + } + for _, c := range byNode { + if c.reachCount >= 2 { + candidates = append(candidates, c) + } + } + } + + if len(candidates) == 0 { + return "" + } + + filtered := candidates[:0] + for _, candidate := range candidates { + if splitJoinCandidateDoesNotHaveDownstreamBypass(candidate.id, activityMap, flowsByOrigin, branchStarts) { + filtered = append(filtered, candidate) + } + } + candidates = filtered if len(candidates) == 0 { return "" } sort.Slice(candidates, func(i, j int) bool { + if candidates[i].reachCount != candidates[j].reachCount { + return candidates[i].reachCount > candidates[j].reachCount + } if candidates[i].maxDistance != candidates[j].maxDistance { return candidates[i].maxDistance < candidates[j].maxDistance } @@ -1083,6 +1127,88 @@ func selectNearestCommonJoin( return candidates[0].id } +func splitJoinCandidateDoesNotHaveDownstreamBypass( + candidateID model.ID, + activityMap map[model.ID]microflows.MicroflowObject, + flowsByOrigin map[model.ID][]*microflows.SequenceFlow, + branchStarts []model.ID, +) bool { + downstream := collectReachableNonTerminalObjects(candidateID, activityMap, flowsByOrigin) + if len(downstream) == 0 { + return true + } + for _, startID := range branchStarts { + if startID == candidateID { + continue + } + if reachesAnyObjectAvoiding(startID, downstream, candidateID, activityMap, flowsByOrigin, map[model.ID]bool{}) { + return false + } + } + return true +} + +func collectReachableNonTerminalObjects( + startID model.ID, + activityMap map[model.ID]microflows.MicroflowObject, + flowsByOrigin map[model.ID][]*microflows.SequenceFlow, +) map[model.ID]bool { + result := map[model.ID]bool{} + var walk func(model.ID) + visited := map[model.ID]bool{startID: true} + walk = func(currentID model.ID) { + if visited[currentID] { + return + } + visited[currentID] = true + if isNonTerminalMicroflowObject(activityMap[currentID]) { + result[currentID] = true + } + for _, flow := range findNormalFlows(flowsByOrigin[currentID]) { + walk(flow.DestinationID) + } + } + for _, flow := range findNormalFlows(flowsByOrigin[startID]) { + walk(flow.DestinationID) + } + return result +} + +func reachesAnyObjectAvoiding( + currentID model.ID, + targets map[model.ID]bool, + avoidID model.ID, + activityMap map[model.ID]microflows.MicroflowObject, + flowsByOrigin map[model.ID][]*microflows.SequenceFlow, + visited map[model.ID]bool, +) bool { + if currentID == "" || currentID == avoidID || visited[currentID] { + return false + } + if targets[currentID] { + return true + } + if !isNonTerminalMicroflowObject(activityMap[currentID]) { + return false + } + visited[currentID] = true + for _, flow := range findNormalFlows(flowsByOrigin[currentID]) { + if reachesAnyObjectAvoiding(flow.DestinationID, targets, avoidID, activityMap, flowsByOrigin, visited) { + return true + } + } + return false +} + +func isNonTerminalMicroflowObject(obj microflows.MicroflowObject) bool { + switch obj.(type) { + case nil, *microflows.StartEvent, *microflows.EndEvent, *microflows.ErrorEvent: + return false + default: + return true + } +} + func isSplitJoinCandidate(obj microflows.MicroflowObject) bool { switch obj.(type) { case nil, *microflows.StartEvent, *microflows.EndEvent: diff --git a/mdl/executor/cmd_microflows_show_helpers.go b/mdl/executor/cmd_microflows_show_helpers.go index 264ce323..a4a55206 100644 --- a/mdl/executor/cmd_microflows_show_helpers.go +++ b/mdl/executor/cmd_microflows_show_helpers.go @@ -668,12 +668,7 @@ func traverseFlow( emitObjectAnnotations(obj, lines, indentStr, annotationsByTarget, flowsByOrigin, flowsByDest, activityMap) emitEnumSplitStatement(ctx, currentID, mergeID, variable, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) recordSourceMap(sourceMap, currentID, startLine, len(*lines)+headerLineCount-1) - if mergeID != "" { - visited[mergeID] = true - for _, flow := range flowsByOrigin[mergeID] { - traverseFlow(ctx, flow.DestinationID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) - } - } + continueAfterSplitJoin(ctx, mergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) return } if stmt != "" { @@ -831,12 +826,7 @@ func traverseFlowUntilMerge( emitObjectAnnotations(obj, lines, indentStr, annotationsByTarget, flowsByOrigin, flowsByDest, activityMap) emitEnumSplitStatement(ctx, currentID, nestedMergeID, variable, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) recordSourceMap(sourceMap, currentID, startLine, len(*lines)+headerLineCount-1) - if nestedMergeID != "" && nestedMergeID != mergeID { - visited[nestedMergeID] = true - for _, flow := range flowsByOrigin[nestedMergeID] { - traverseFlowUntilMerge(ctx, flow.DestinationID, mergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) - } - } + continueAfterNestedSplitJoin(ctx, nestedMergeID, mergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) return } if stmt != "" { @@ -845,10 +835,11 @@ func traverseFlowUntilMerge( } trueFlow, falseFlow := findBranchFlows(flows) + nestedMergeID = resolveNestedMergeID(nestedMergeID, mergeID, trueFlow, falseFlow, flowsByOrigin) // Empty-then swap: negate when true branch is empty but false branch has content. // Skip when both branches go directly to merge (both empty). - if trueFlow != nil && falseFlow != nil && nestedMergeID != "" { + if trueFlow != nil && falseFlow != nil && nestedMergeID != "" && nestedMergeID == mergeID { if trueFlow.DestinationID == nestedMergeID && falseFlow.DestinationID != nestedMergeID { stmt = negateIfCondition(stmt) trueFlow, falseFlow = falseFlow, trueFlow @@ -1000,6 +991,63 @@ func continueAfterNestedSplitJoin( traverseFlowUntilMerge(ctx, joinID, parentMergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) } +func resolveNestedMergeID( + nestedMergeID model.ID, + parentMergeID model.ID, + trueFlow *microflows.SequenceFlow, + falseFlow *microflows.SequenceFlow, + flowsByOrigin map[model.ID][]*microflows.SequenceFlow, +) model.ID { + if nestedMergeID != "" && parentMergeID != "" && nestedMergeID != parentMergeID && + canReachNode(parentMergeID, nestedMergeID, flowsByOrigin, make(map[model.ID]bool)) { + for _, flow := range []*microflows.SequenceFlow{trueFlow, falseFlow} { + if flow == nil { + continue + } + if flow.DestinationID == parentMergeID || + canReachNode(flow.DestinationID, parentMergeID, flowsByOrigin, make(map[model.ID]bool)) { + return parentMergeID + } + } + } + if nestedMergeID != "" || parentMergeID == "" { + return nestedMergeID + } + for _, flow := range []*microflows.SequenceFlow{trueFlow, falseFlow} { + if flow == nil { + continue + } + if canReachNode(flow.DestinationID, parentMergeID, flowsByOrigin, make(map[model.ID]bool)) { + return parentMergeID + } + } + return "" +} + +func canReachNode( + currentID model.ID, + targetID model.ID, + flowsByOrigin map[model.ID][]*microflows.SequenceFlow, + visited map[model.ID]bool, +) bool { + if currentID == "" { + return false + } + if currentID == targetID { + return true + } + if visited[currentID] { + return false + } + visited[currentID] = true + for _, flow := range findNormalFlows(flowsByOrigin[currentID]) { + if canReachNode(flow.DestinationID, targetID, flowsByOrigin, visited) { + return true + } + } + return false +} + // traverseLoopBody traverses activities inside a loop body. // When sourceMap is non-nil, it also records line ranges for each activity node. func traverseLoopBody( @@ -1421,10 +1469,13 @@ func flowLooksLikeGuardContinuation( return false } // Builder-generated guard continuations sit on the split's horizontal - // centerline. This intentionally relies on mxcli's layout contract so a - // real branch that returns to a merge below the split is not collapsed into - // a guard-style continuation during describe. - return dest.GetPosition().Y == split.GetPosition().Y + // centerline and use the builder's horizontal split→tail flow. This + // intentionally relies on mxcli's layout/anchor contract so a real false + // branch whose activities happen to be aligned with the split is not + // collapsed into a guard-style continuation during describe. + return dest.GetPosition().Y == split.GetPosition().Y && + flow.OriginConnectionIndex == AnchorRight && + flow.DestinationConnectionIndex == AnchorLeft } // findErrorHandlerFlow returns the error handler flow from an activity's outgoing flows. diff --git a/mdl/executor/cmd_microflows_traverse_test.go b/mdl/executor/cmd_microflows_traverse_test.go index 9585fd86..75670107 100644 --- a/mdl/executor/cmd_microflows_traverse_test.go +++ b/mdl/executor/cmd_microflows_traverse_test.go @@ -493,6 +493,109 @@ func TestTraverseFlow_CommonActivityJoinKeepsTailOutsideBranches(t *testing.T) { } } +func TestTraverseFlow_EnumSplitPartialJoinKeepsSharedTailOutsideCases(t *testing.T) { + e := newTestExecutor() + + logAction := func(id, message string) *microflows.ActionActivity { + return µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj(id)}, + Action: µflows.LogMessageAction{ + LogLevel: "Info", + LogNodeName: "'Synthetic'", + MessageTemplate: &model.Text{Translations: map[string]string{"en_US": message}}, + }, + } + } + + activityMap := map[model.ID]microflows.MicroflowObject{ + mkID("start"): µflows.StartEvent{BaseMicroflowObject: mkObj("start")}, + mkID("kind_split"): µflows.ExclusiveSplit{ + BaseMicroflowObject: mkObj("kind_split"), + SplitCondition: µflows.ExpressionSplitCondition{Expression: "$Kind"}, + }, + mkID("case_a"): logAction("case_a", "case A"), + mkID("case_b"): logAction("case_b", "case B"), + mkID("case_c"): logAction("case_c", "case C terminal"), + mkID("shared_tail"): logAction("shared_tail", "shared tail"), + mkID("end"): µflows.EndEvent{BaseMicroflowObject: mkObj("end")}, + } + flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{ + mkID("start"): {mkFlow("start", "kind_split")}, + mkID("kind_split"): { + mkBranchFlow("kind_split", "case_a", microflows.EnumerationCase{Value: "A"}), + mkBranchFlow("kind_split", "case_b", microflows.EnumerationCase{Value: "B"}), + mkBranchFlow("kind_split", "case_c", microflows.EnumerationCase{Value: "C"}), + }, + mkID("case_a"): {mkFlow("case_a", "shared_tail")}, + mkID("case_b"): {mkFlow("case_b", "shared_tail")}, + mkID("case_c"): {mkFlow("case_c", "end")}, + mkID("shared_tail"): {mkFlow("shared_tail", "end")}, + } + + joinID := findMergeForSplit(nil, mkID("kind_split"), flowsByOrigin, activityMap) + if joinID != mkID("shared_tail") { + t.Fatalf("enum split paired with %q, want shared tail %q", joinID, mkID("shared_tail")) + } + + splitMergeMap := map[model.ID]model.ID{mkID("kind_split"): joinID} + 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") + if got := strings.Count(out, "shared tail"); got != 1 { + t.Fatalf("shared tail emitted %d times, want once:\n%s", got, out) + } + endCase := strings.Index(out, "end case;") + sharedTail := strings.Index(out, "shared tail") + if endCase == -1 || sharedTail == -1 || endCase > sharedTail { + t.Fatalf("shared tail must be emitted after enum split closes:\n%s", out) + } +} + +func TestFindMergeForSplit_SkipsJoinBypassedByNestedBranch(t *testing.T) { + activityMap := map[model.ID]microflows.MicroflowObject{ + mkID("outer_split"): µflows.ExclusiveSplit{ + BaseMicroflowObject: mkObj("outer_split"), + SplitCondition: µflows.ExpressionSplitCondition{Expression: "$HasExisting"}, + }, + mkID("inner_split"): µflows.ExclusiveSplit{ + BaseMicroflowObject: mkObj("inner_split"), + SplitCondition: µflows.ExpressionSplitCondition{Expression: "$CanUpdate"}, + }, + mkID("early_join"): µflows.ExclusiveMerge{BaseMicroflowObject: mkObj("early_join")}, + mkID("clear"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("clear")}, + Action: µflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'Synthetic'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "clear"}}}, + }, + mkID("shared_tail"): µflows.ExclusiveMerge{BaseMicroflowObject: mkObj("shared_tail")}, + mkID("retrieve"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("retrieve")}, + Action: µflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'Synthetic'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "retrieve"}}}, + }, + mkID("end"): µflows.EndEvent{BaseMicroflowObject: mkObj("end")}, + } + flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{ + mkID("outer_split"): { + mkBranchFlow("outer_split", "early_join", µflows.ExpressionCase{Expression: "true"}), + mkBranchFlow("outer_split", "inner_split", µflows.ExpressionCase{Expression: "false"}), + }, + mkID("inner_split"): { + mkBranchFlow("inner_split", "shared_tail", µflows.ExpressionCase{Expression: "true"}), + mkBranchFlow("inner_split", "early_join", µflows.ExpressionCase{Expression: "false"}), + }, + mkID("early_join"): {mkFlow("early_join", "clear")}, + mkID("clear"): {mkFlow("clear", "shared_tail")}, + mkID("shared_tail"): {mkFlow("shared_tail", "retrieve")}, + mkID("retrieve"): {mkFlow("retrieve", "end")}, + } + + joinID := findMergeForSplit(nil, mkID("outer_split"), flowsByOrigin, activityMap) + if joinID != mkID("shared_tail") { + t.Fatalf("outer split paired with %q, want downstream shared tail %q", joinID, mkID("shared_tail")) + } +} + func TestTraverseFlow_SequentialIfWithoutElseKeepsContinuationOutsideFirstIf(t *testing.T) { e := newTestExecutor() @@ -563,6 +666,9 @@ func TestTraverseFlow_SequentialIfWithoutElseKeepsContinuationOutsideFirstIf(t * func TestTraverseFlow_GuardBranchWithMultipleActivitiesKeepsContinuationOutsideElse(t *testing.T) { e := newTestExecutor() + falseFlow := mkBranchFlow("split", "tail_log", µflows.ExpressionCase{Expression: "false"}) + falseFlow.OriginConnectionIndex = AnchorRight + falseFlow.DestinationConnectionIndex = AnchorLeft activityMap := map[model.ID]microflows.MicroflowObject{ mkID("start"): µflows.StartEvent{BaseMicroflowObject: mkObj("start")}, @@ -585,7 +691,7 @@ func TestTraverseFlow_GuardBranchWithMultipleActivitiesKeepsContinuationOutsideE mkID("start"): {mkFlow("start", "split")}, mkID("split"): { mkBranchFlow("split", "then_log", µflows.ExpressionCase{Expression: "true"}), - mkBranchFlow("split", "tail_log", µflows.ExpressionCase{Expression: "false"}), + falseFlow, }, mkID("then_log"): {mkFlow("then_log", "then_return")}, mkID("tail_log"): {mkFlow("tail_log", "end")}, @@ -607,6 +713,47 @@ func TestTraverseFlow_GuardBranchWithMultipleActivitiesKeepsContinuationOutsideE } } +func TestTraverseFlow_TerminalFalseBranchIsNotGuardContinuation(t *testing.T) { + e := newTestExecutor() + + activityMap := map[model.ID]microflows.MicroflowObject{ + mkID("start"): µflows.StartEvent{BaseMicroflowObject: mkObj("start")}, + mkID("split"): µflows.ExclusiveSplit{ + BaseMicroflowObject: mkObj("split"), + SplitCondition: µflows.ExpressionSplitCondition{Expression: "$IsAllowed"}, + }, + mkID("then_log"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("then_log")}, + Action: µflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'App'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "allowed"}}}, + }, + mkID("then_return"): µflows.EndEvent{BaseMicroflowObject: mkObj("then_return")}, + mkID("false_log"): µ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": "blocked"}}}}, + mkID("false_return"): µflows.EndEvent{BaseMicroflowObject: mkObj("false_return")}, + } + + flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{ + mkID("start"): {mkFlow("start", "split")}, + mkID("split"): { + mkBranchFlow("split", "then_log", µflows.ExpressionCase{Expression: "true"}), + mkBranchFlow("split", "false_log", µflows.ExpressionCase{Expression: "false"}), + }, + mkID("then_log"): {mkFlow("then_log", "then_return")}, + mkID("false_log"): {mkFlow("false_log", "false_return")}, + } + + var lines []string + visited := make(map[model.ID]bool) + e.traverseFlow(mkID("start"), activityMap, flowsByOrigin, nil, visited, nil, nil, &lines, 0, nil, 0, nil) + + out := strings.Join(lines, "\n") + if !strings.Contains(out, "\nelse\n") { + t.Fatalf("terminal false branch must stay inside an explicit ELSE:\n%s", out) + } + if strings.Index(out, "blocked") < strings.Index(out, "else") { + t.Fatalf("false branch body was emitted outside ELSE:\n%s", out) + } +} + func TestTraverseFlow_NestedTerminalGuardBranchSuppressesEmptyOuterElse(t *testing.T) { e := newTestExecutor() @@ -654,6 +801,46 @@ func TestTraverseFlow_NestedTerminalGuardBranchSuppressesEmptyOuterElse(t *testi } } +func TestResolveNestedMergeID_UsesParentMergeBeforeDownstreamJoin(t *testing.T) { + flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{ + mkID("parent_merge"): {mkFlow("parent_merge", "downstream_join")}, + } + trueFlow := mkFlow("nested_split", "parent_merge") + falseFlow := mkFlow("nested_split", "false_branch") + + got := resolveNestedMergeID( + mkID("downstream_join"), + mkID("parent_merge"), + trueFlow, + falseFlow, + flowsByOrigin, + ) + + if got != mkID("parent_merge") { + t.Fatalf("nested split used downstream join %q, want parent merge %q", got, mkID("parent_merge")) + } +} + +func TestResolveNestedMergeID_KeepsIndependentNestedJoin(t *testing.T) { + flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{ + mkID("nested_join"): {mkFlow("nested_join", "parent_merge")}, + } + trueFlow := mkFlow("nested_split", "true_branch") + falseFlow := mkFlow("nested_split", "nested_join") + + got := resolveNestedMergeID( + mkID("nested_join"), + mkID("parent_merge"), + trueFlow, + falseFlow, + flowsByOrigin, + ) + + if got != mkID("nested_join") { + t.Fatalf("nested split merge changed to %q, want local nested join %q", got, mkID("nested_join")) + } +} + func TestTraverseFlow_LoopBodyUsesNestedAnnotationFlows(t *testing.T) { e := newTestExecutor()