fix: preserve decision/loop captions across nested control flow

hjotha · claude · hjothamendix · commit 48881e753dac · 2026-04-22T14:27:06.000+02:00
When an IF, LOOP, or WHILE statement carried an @caption or @annotation and contained other annotated statements in its body, the inner statement's annotations would overwrite fb.pendingAnnotations (shared builder state) before the outer loop in buildFlowGraph attached them to the right object. The outer split/loop then ended up with the wrong caption — or with the inner statement's caption applied to it. The fix snapshots & clears pendingAnnotations at the top of each compound statement handler, re-applies them to the split/loop/while activity right after it's created, and (for IF) moves applyAnnotations above the branch recursion so the split is decorated before the bodies are walked. Also extends applyAnnotations to recognise ExclusiveSplit and InheritanceSplit — on main these were only handled for ActionActivity, so @caption on a split was silently dropped even without nesting. Verified end-to-end against Apps.GetOrCreateMendixVersionFromString: describe -> exec -> describe now produces byte-identical @caption, @annotation, and @position output. Regression tests cover: - nested IFs with explicit @caption on each level - single IF @caption baseline - @annotation attachment targeting the correct split when nested Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
diff --git a/mdl/executor/cmd_microflows_builder_annotations.go b/mdl/executor/cmd_microflows_builder_annotations.go
@@ -116,8 +116,8 @@ func (fb *flowBuilder) applyAnnotations(activityID model.ID, ann *ast.ActivityAn
 				continue
 			}
 
-			// @caption, @color, and @excluded — only applicable to ActionActivity
-			if activity, ok := obj.(*microflows.ActionActivity); ok {
+			switch activity := obj.(type) {
+			case *microflows.ActionActivity:
 				if ann.Caption != "" {
 					activity.Caption = ann.Caption
 					activity.AutoGenerateCaption = false
@@ -128,6 +128,16 @@ func (fb *flowBuilder) applyAnnotations(activityID model.ID, ann *ast.ActivityAn
 				if ann.Excluded {
 					activity.Disabled = true
 				}
+			case *microflows.ExclusiveSplit:
+				// Splits carry a human-readable Caption (e.g. "Right format?")
+				// independent of the expression/rule being evaluated.
+				if ann.Caption != "" {
+					activity.Caption = ann.Caption
+				}
+			case *microflows.InheritanceSplit:
+				if ann.Caption != "" {
+					activity.Caption = ann.Caption
+				}
 			}
 
 			break
diff --git a/mdl/executor/cmd_microflows_builder_annotations_test.go b/mdl/executor/cmd_microflows_builder_annotations_test.go
@@ -0,0 +1,221 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package executor
+
+import (
+	"testing"
+
+	"github.com/mendixlabs/mxcli/mdl/ast"
+	"github.com/mendixlabs/mxcli/sdk/microflows"
+)
+
+// TestNestedIfPreservesCaptions is a regression test for the bug where
+// the outer IF's @caption would be overwritten by the inner IF's @caption
+// because pendingAnnotations is shared mutable state across recursive
+// addStatement calls.
+//
+// Before the fix:
+//   - outer ExclusiveSplit received caption "Right format?" (from inner IF)
+//   - inner ExclusiveSplit kept its condition expression as caption
+//   - inner IF's @annotation got attached to the outer split
+//
+// After the fix:
+//   - addIfStatement consumes its own pendingAnnotations right after
+//     creating its split, so outer and inner captions stay bound to the
+//     correct splits.
+func TestNestedIfPreservesCaptions(t *testing.T) {
+	// Build an AST equivalent to:
+	//   if $S != empty            @caption 'String not empty?'
+	//     if isMatch($S, 'x')     @caption 'Right format?'
+	//       return true
+	//     else
+	//       return false
+	//   else
+	//     return false
+	innerIf := &ast.IfStmt{
+		Condition: &ast.FunctionCallExpr{
+			Name:      "isMatch",
+			Arguments: []ast.Expression{&ast.VariableExpr{Name: "S"}, &ast.LiteralExpr{Value: "x", Kind: ast.LiteralString}},
+		},
+		ThenBody: []ast.MicroflowStatement{
+			&ast.ReturnStmt{Value: &ast.LiteralExpr{Value: true, Kind: ast.LiteralBoolean}},
+		},
+		ElseBody: []ast.MicroflowStatement{
+			&ast.ReturnStmt{Value: &ast.LiteralExpr{Value: false, Kind: ast.LiteralBoolean}},
+		},
+		Annotations: &ast.ActivityAnnotations{Caption: "Right format?"},
+	}
+	outerIf := &ast.IfStmt{
+		Condition: &ast.BinaryExpr{
+			Left:     &ast.VariableExpr{Name: "S"},
+			Operator: "!=",
+			Right:    &ast.LiteralExpr{Value: nil, Kind: ast.LiteralNull},
+		},
+		ThenBody: []ast.MicroflowStatement{innerIf},
+		ElseBody: []ast.MicroflowStatement{
+			&ast.ReturnStmt{Value: &ast.LiteralExpr{Value: false, Kind: ast.LiteralBoolean}},
+		},
+		Annotations: &ast.ActivityAnnotations{Caption: "String not empty?"},
+	}
+
+	fb := &flowBuilder{
+		posX:         100,
+		posY:         100,
+		spacing:      HorizontalSpacing,
+		varTypes:     map[string]string{"S": "String"},
+		declaredVars: map[string]string{"S": "String"},
+	}
+	fb.buildFlowGraph([]ast.MicroflowStatement{outerIf}, nil)
+
+	// Collect ExclusiveSplits with their captions. The outer split is created
+	// first, so objects[1] is the outer split (objects[0] is the StartEvent).
+	var splits []*microflows.ExclusiveSplit
+	for _, obj := range fb.objects {
+		if sp, ok := obj.(*microflows.ExclusiveSplit); ok {
+			splits = append(splits, sp)
+		}
+	}
+
+	if len(splits) != 2 {
+		t.Fatalf("expected 2 ExclusiveSplits, got %d", len(splits))
+	}
+
+	// Splits are appended in creation order: outer first (from outerIf),
+	// then inner (when recursion into ThenBody creates the nested IF's split).
+	outerSplit, innerSplit := splits[0], splits[1]
+
+	if outerSplit.Caption != "String not empty?" {
+		t.Errorf("outer split caption: got %q, want %q", outerSplit.Caption, "String not empty?")
+	}
+	if innerSplit.Caption != "Right format?" {
+		t.Errorf("inner split caption: got %q, want %q", innerSplit.Caption, "Right format?")
+	}
+}
+
+// TestIfCaptionWithoutNesting confirms a single IF with @caption still gets
+// the right caption after the fix (baseline sanity check).
+func TestIfCaptionWithoutNesting(t *testing.T) {
+	ifStmt := &ast.IfStmt{
+		Condition: &ast.BinaryExpr{
+			Left:     &ast.VariableExpr{Name: "S"},
+			Operator: "!=",
+			Right:    &ast.LiteralExpr{Value: nil, Kind: ast.LiteralNull},
+		},
+		ThenBody: []ast.MicroflowStatement{
+			&ast.ReturnStmt{Value: &ast.LiteralExpr{Value: true, Kind: ast.LiteralBoolean}},
+		},
+		ElseBody: []ast.MicroflowStatement{
+			&ast.ReturnStmt{Value: &ast.LiteralExpr{Value: false, Kind: ast.LiteralBoolean}},
+		},
+		Annotations: &ast.ActivityAnnotations{Caption: "String not empty?"},
+	}
+
+	fb := &flowBuilder{
+		posX:         100,
+		posY:         100,
+		spacing:      HorizontalSpacing,
+		varTypes:     map[string]string{"S": "String"},
+		declaredVars: map[string]string{"S": "String"},
+	}
+	fb.buildFlowGraph([]ast.MicroflowStatement{ifStmt}, nil)
+
+	for _, obj := range fb.objects {
+		if sp, ok := obj.(*microflows.ExclusiveSplit); ok {
+			if sp.Caption != "String not empty?" {
+				t.Errorf("split caption: got %q, want %q", sp.Caption, "String not empty?")
+			}
+			return
+		}
+	}
+	t.Fatal("no ExclusiveSplit found")
+}
+
+// TestIfAnnotationStaysWithCorrectSplit confirms @annotation on a nested IF
+// attaches to that IF's split, not to the outer one.
+func TestIfAnnotationStaysWithCorrectSplit(t *testing.T) {
+	innerIf := &ast.IfStmt{
+		Condition: &ast.FunctionCallExpr{
+			Name:      "isMatch",
+			Arguments: []ast.Expression{&ast.VariableExpr{Name: "S"}, &ast.LiteralExpr{Value: "x", Kind: ast.LiteralString}},
+		},
+		ThenBody: []ast.MicroflowStatement{
+			&ast.ReturnStmt{Value: &ast.LiteralExpr{Value: true, Kind: ast.LiteralBoolean}},
+		},
+		ElseBody: []ast.MicroflowStatement{
+			&ast.ReturnStmt{Value: &ast.LiteralExpr{Value: false, Kind: ast.LiteralBoolean}},
+		},
+		Annotations: &ast.ActivityAnnotations{
+			Caption:        "Right format?",
+			AnnotationText: "Inner IF note",
+		},
+	}
+	outerIf := &ast.IfStmt{
+		Condition: &ast.BinaryExpr{
+			Left:     &ast.VariableExpr{Name: "S"},
+			Operator: "!=",
+			Right:    &ast.LiteralExpr{Value: nil, Kind: ast.LiteralNull},
+		},
+		ThenBody: []ast.MicroflowStatement{innerIf},
+		ElseBody: []ast.MicroflowStatement{
+			&ast.ReturnStmt{Value: &ast.LiteralExpr{Value: false, Kind: ast.LiteralBoolean}},
+		},
+		Annotations: &ast.ActivityAnnotations{
+			Caption:        "String not empty?",
+			AnnotationText: "Outer IF note",
+		},
+	}
+
+	fb := &flowBuilder{
+		posX:         100,
+		posY:         100,
+		spacing:      HorizontalSpacing,
+		varTypes:     map[string]string{"S": "String"},
+		declaredVars: map[string]string{"S": "String"},
+	}
+	fb.buildFlowGraph([]ast.MicroflowStatement{outerIf}, nil)
+
+	var splits []*microflows.ExclusiveSplit
+	var annotations []*microflows.Annotation
+	for _, obj := range fb.objects {
+		switch o := obj.(type) {
+		case *microflows.ExclusiveSplit:
+			splits = append(splits, o)
+		case *microflows.Annotation:
+			annotations = append(annotations, o)
+		}
+	}
+
+	if len(splits) != 2 {
+		t.Fatalf("expected 2 splits, got %d", len(splits))
+	}
+	if len(annotations) != 2 {
+		t.Fatalf("expected 2 annotations, got %d", len(annotations))
+	}
+
+	outerSplit, innerSplit := splits[0], splits[1]
+
+	// AnnotationFlow links Annotation -> activity. Verify each flow points
+	// from the annotation with the expected text to the expected split.
+	var outerNoteDestID, innerNoteDestID string
+	for _, af := range fb.annotationFlows {
+		// Find the Annotation referenced by OriginID
+		for _, ann := range annotations {
+			if ann.ID != af.OriginID {
+				continue
+			}
+			switch ann.Caption {
+			case "Outer IF note":
+				outerNoteDestID = string(af.DestinationID)
+			case "Inner IF note":
+				innerNoteDestID = string(af.DestinationID)
+			}
+		}
+	}
+
+	if outerNoteDestID != string(outerSplit.ID) {
+		t.Errorf("outer note destination: got %q, want %q (outer split)", outerNoteDestID, outerSplit.ID)
+	}
+	if innerNoteDestID != string(innerSplit.ID) {
+		t.Errorf("inner note destination: got %q, want %q (inner split)", innerNoteDestID, innerSplit.ID)
+	}
+}
diff --git a/mdl/executor/cmd_microflows_builder_control.go b/mdl/executor/cmd_microflows_builder_control.go
@@ -62,6 +62,15 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
 	fb.objects = append(fb.objects, split)
 	splitID := split.ID
 
+	// Apply this IF's own annotations (e.g. @caption, @annotation) to the split
+	// BEFORE recursing into branch bodies. Otherwise a nested statement's annotations
+	// would overwrite fb.pendingAnnotations (shared state) and the outer loop in
+	// buildFlowGraph would then attach the wrong caption/annotation to this split.
+	if fb.pendingAnnotations != nil {
+		fb.applyAnnotations(splitID, fb.pendingAnnotations)
+		fb.pendingAnnotations = nil
+	}
+
 	// Calculate merge position (after the longest branch)
 	mergeX := splitX + SplitWidth + HorizontalSpacing/2 + branchWidth + HorizontalSpacing/2
 
@@ -251,6 +260,12 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
 // addLoopStatement creates a LOOP statement using LoopedActivity.
 // Layout: Auto-sizes the loop box to fit content with padding
 func (fb *flowBuilder) addLoopStatement(s *ast.LoopStmt) model.ID {
+	// Snapshot & clear this loop's own annotations so the body's recursive
+	// addStatement calls can't consume them. We re-apply to the loop activity
+	// after it's created below.
+	savedLoopAnnotations := fb.pendingAnnotations
+	fb.pendingAnnotations = nil
+
 	// First, measure the loop body to determine size
 	bodyBounds := fb.measurer.measureStatements(s.Body)
 
@@ -335,6 +350,11 @@ func (fb *flowBuilder) addLoopStatement(s *ast.LoopStmt) model.ID {
 	// This is how Mendix stores them - all flows at the microflow level
 	fb.flows = append(fb.flows, loopBuilder.flows...)
 
+	// Re-apply this loop's own annotations now that its activity exists.
+	if savedLoopAnnotations != nil {
+		fb.applyAnnotations(loop.ID, savedLoopAnnotations)
+	}
+
 	fb.posX += loopWidth + HorizontalSpacing
 
 	return loop.ID
@@ -343,6 +363,11 @@ func (fb *flowBuilder) addLoopStatement(s *ast.LoopStmt) model.ID {
 // addWhileStatement creates a WHILE loop using LoopedActivity with WhileLoopCondition.
 // Layout matches addLoopStatement but without iterator icon space.
 func (fb *flowBuilder) addWhileStatement(s *ast.WhileStmt) model.ID {
+	// Snapshot & clear this WHILE's own annotations so the body's recursive
+	// addStatement calls can't consume them (see addLoopStatement).
+	savedWhileAnnotations := fb.pendingAnnotations
+	fb.pendingAnnotations = nil
+
 	bodyBounds := fb.measurer.measureStatements(s.Body)
 
 	loopWidth := max(bodyBounds.Width+2*LoopPadding, MinLoopWidth)
@@ -402,6 +427,11 @@ func (fb *flowBuilder) addWhileStatement(s *ast.WhileStmt) model.ID {
 
 	fb.objects = append(fb.objects, loop)
 	fb.flows = append(fb.flows, loopBuilder.flows...)
+
+	if savedWhileAnnotations != nil {
+		fb.applyAnnotations(loop.ID, savedWhileAnnotations)
+	}
+
 	fb.posX += loopWidth + HorizontalSpacing
 
 	return loop.ID
