diff --git a/mdl-examples/bug-tests/310-anchor-split-to-merge-destination.mdl b/mdl-examples/bug-tests/310-anchor-split-to-merge-destination.mdl new file mode 100644 index 00000000..8af2612f --- /dev/null +++ b/mdl-examples/bug-tests/310-anchor-split-to-merge-destination.mdl @@ -0,0 +1,63 @@ +-- ============================================================================ +-- Bug #310: Branch anchor `to:` side dropped on direct split-to-merge flows +-- ============================================================================ +-- +-- Symptom (before fix): +-- When an IF had a branch anchor whose `to:` side wasn't the default Left +-- AND that branch body was empty (so the split connected straight to the +-- merge), the writer applied only the branch FROM side and let the +-- default destination (Left) overwrite the user-provided `to:` side. +-- Re-describing turned +-- @anchor(to: left, false: (from: bottom, to: top)) +-- into +-- @anchor(to: left, false: (from: bottom, to: left)) +-- silently changing the visual layout in Studio Pro on roundtrip. +-- +-- Root cause: +-- Several split-to-merge shortcuts called `applyUserAnchors` with the +-- branch anchor only as the origin override and `nil` as the destination +-- override. +-- +-- After fix: +-- `applyUserAnchors` now receives the branch anchor for both origin and +-- destination overrides on split-to-merge shortcuts, matching the +-- behavior used when a branch has a first statement. +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/310-anchor-split-to-merge-destination.mdl -p app.mpr +-- mxcli -p app.mpr -c "describe microflow BugTest310.MF_AnchorFalseBranch" +-- The output must preserve `false: (from: bottom, to: top)` on the +-- IF anchor and must be a fixpoint under describe → exec → describe. +-- ============================================================================ + +create module BugTest310; + +-- IF-without-ELSE with a non-default false-branch destination. +create microflow BugTest310.MF_AnchorFalseBranch ( + $value: integer +) +begin + @anchor(to: left, false: (from: bottom, to: top)) + if $value > 0 then + log info node 'BugTest310' 'inside'; + end if; + + log info node 'BugTest310' 'after'; +end; +/ + +-- IF with empty THEN body — true branch goes straight to merge with a +-- non-default destination side. +create microflow BugTest310.MF_AnchorEmptyThen ( + $value: integer +) +begin + @anchor(to: left, true: (from: right, to: top)) + if $value > 0 then + else + log info node 'BugTest310' 'else only'; + end if; + + log info node 'BugTest310' 'after'; +end; +/ diff --git a/mdl/executor/cmd_microflows_anchor_if_test.go b/mdl/executor/cmd_microflows_anchor_if_test.go index 137e400f..4caefbd7 100644 --- a/mdl/executor/cmd_microflows_anchor_if_test.go +++ b/mdl/executor/cmd_microflows_anchor_if_test.go @@ -109,6 +109,60 @@ func TestBuilder_AnchorInsideElseBranch(t *testing.T) { } } +// TestBuilder_AnchorFalseBranchTo_IfWithoutElse pins a regression: when the +// describer emits +// +// @anchor(to: left, true: (from: right, to: left), false: (from: bottom, to: top)) +// +// on an IF-without-ELSE split, the writer used to apply only the FROM side of +// the false-branch anchor to the split→merge flow, letting the default (Left) +// overwrite the intended `to: top`. Re-describing produced `false: (from: bottom, to: left)`. +func TestBuilder_AnchorFalseBranchTo_IfWithoutElse(t *testing.T) { + body := []ast.MicroflowStatement{ + &ast.IfStmt{ + Condition: &ast.LiteralExpr{Kind: ast.LiteralBoolean, Value: true}, + ThenBody: []ast.MicroflowStatement{ + &ast.LogStmt{Level: ast.LogInfo, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "inside"}}, + }, + Annotations: &ast.ActivityAnnotations{ + FalseBranchAnchor: &ast.FlowAnchors{From: ast.AnchorSideBottom, To: ast.AnchorSideTop}, + }, + }, + } + + oc := buildWithAnchors(body) + + // The split → merge false flow must land on merge's Top side, not the default Left. + if !hasFlow(oc.Flows, AnchorBottom, AnchorTop) { + t.Errorf("expected false branch split→merge flow Bottom→Top, got %+v", oc.Flows) + } +} + +// TestBuilder_AnchorTrueBranchTo_EmptyThenIfWithElse is the ELSE-branch mirror +// of the cluster B4 regression: an empty THEN body connecting straight to the +// merge must honor the trueBranchAnchor.To. +func TestBuilder_AnchorTrueBranchTo_EmptyThenIfWithElse(t *testing.T) { + body := []ast.MicroflowStatement{ + &ast.IfStmt{ + Condition: &ast.LiteralExpr{Kind: ast.LiteralBoolean, Value: true}, + ThenBody: nil, // empty THEN body + ElseBody: []ast.MicroflowStatement{ + &ast.LogStmt{Level: ast.LogInfo, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "else"}}, + }, + Annotations: &ast.ActivityAnnotations{ + TrueBranchAnchor: &ast.FlowAnchors{From: ast.AnchorSideRight, To: ast.AnchorSideTop}, + }, + }, + } + + oc := buildWithAnchors(body) + + // Empty THEN → merge: must honor the user's Right→Top anchor, not the default Right→Left. + if !hasFlow(oc.Flows, AnchorRight, AnchorTop) { + t.Errorf("expected true branch split→merge flow Right→Top, got %+v", oc.Flows) + } +} + func TestBuilder_AnchorToTopOnReturnPreservedInsideElse(t *testing.T) { // Minimal case: single-statement ELSE whose only statement is a RETURN // carrying @anchor(to: top). The flow from the split to that return's diff --git a/mdl/executor/cmd_microflows_builder_control.go b/mdl/executor/cmd_microflows_builder_control.go index 4841f912..e68fff3d 100644 --- a/mdl/executor/cmd_microflows_builder_control.go +++ b/mdl/executor/cmd_microflows_builder_control.go @@ -155,9 +155,11 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID { applyUserAnchors(flow, prevThenAnchor, nil) fb.flows = append(fb.flows, flow) } else { - // Empty THEN body - connect split directly to merge with true case + // Empty THEN body - connect split directly to merge with true case. + // Pass trueBranchAnchor as destination too so the @anchor(true: (..., to: Y)) + // from the describer round-trips into the merge side of the flow. flow := newHorizontalFlowWithCase(splitID, mergeID, "true") - applyUserAnchors(flow, trueBranchAnchor, nil) + applyUserAnchors(flow, trueBranchAnchor, trueBranchAnchor) fb.flows = append(fb.flows, flow) } } @@ -214,9 +216,11 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID { // This keeps the "do nothing" path straight and the "do something" path below if needMerge { - // FALSE path: connect split directly to merge horizontally + // FALSE path: connect split directly to merge horizontally. + // Pass falseBranchAnchor as destination too so @anchor(false: (..., to: Y)) + // round-trips through the merge side of the split-to-merge flow. flow := newHorizontalFlowWithCase(splitID, mergeID, "false") - applyUserAnchors(flow, falseBranchAnchor, nil) + applyUserAnchors(flow, falseBranchAnchor, falseBranchAnchor) fb.flows = append(fb.flows, flow) } // When !needMerge (thenReturns): FALSE flow is deferred — the parent will @@ -267,9 +271,11 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID { applyUserAnchors(flow, prevThenAnchor, nil) fb.flows = append(fb.flows, flow) } else { - // Empty THEN body - connect split directly to merge going down and back up + // Empty THEN body - connect split directly to merge going down and back up. + // Pass trueBranchAnchor as destination too so @anchor(true: (..., to: Y)) + // round-trips into the merge side of the flow. flow := newDownwardFlowWithCase(splitID, mergeID, "true") - applyUserAnchors(flow, trueBranchAnchor, nil) + applyUserAnchors(flow, trueBranchAnchor, trueBranchAnchor) fb.flows = append(fb.flows, flow) } }