Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions mdl-examples/bug-tests/310-anchor-split-to-merge-destination.mdl
Original file line number Diff line number Diff line change
@@ -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;
/
54 changes: 54 additions & 0 deletions mdl/executor/cmd_microflows_anchor_if_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 12 additions & 6 deletions mdl/executor/cmd_microflows_builder_control.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}
Expand Down