From 55427991d5029c65d504873505b01638bc8a4119 Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Thu, 23 Apr 2026 22:55:36 +0200 Subject: [PATCH 1/2] fix: honor branch anchor destination on split-to-merge flows Branch-level `@anchor(... true: (...), false: (...))` metadata must preserve both the origin and destination sides for direct split-to-merge shortcut flows. The builder applied only the branch origin side in several shortcut paths. With no destination override, the flow fell back to the default destination side and the next describe changed the branch anchor. Pass the same branch anchor as both origin and destination override for split-to-merge shortcuts, matching the existing behavior for branches that have a first statement. Tests cover true and false branch destination sides surviving a builder roundtrip. --- mdl/executor/cmd_microflows_anchor_if_test.go | 54 +++++++++++++++++++ .../cmd_microflows_builder_control.go | 18 ++++--- 2 files changed, 66 insertions(+), 6 deletions(-) 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) } } From fd86c98d0aa2703fe0b544d6a09e4e3fae35be78 Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Sun, 26 Apr 2026 23:56:43 +0200 Subject: [PATCH 2/2] test: add bug-test reproducer for split-to-merge anchor destination fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an MDL script under mdl-examples/bug-tests/ exercising IF-without-else and IF-with-empty-then patterns where the branch anchor `to:` side must survive a describe → exec → describe cycle. Co-Authored-By: Claude Opus 4.7 --- .../310-anchor-split-to-merge-destination.mdl | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 mdl-examples/bug-tests/310-anchor-split-to-merge-destination.mdl 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; +/