Skip to content

Commit 94bded8

Browse files
committed
fixup: close PR #263 review gaps
- mergeStatementAnnotations: add WhileStmt case (was falling through to default: nil, so @caption on a WHILE was silently dropped). - applyAnnotations: add LoopedActivity case — LOOP and WHILE activities can now carry the captions they already parse. - Add TestLoopCaptionPreserved, TestWhileLoopCaptionPreserved, and TestInheritanceSplitCaptionApplied to cover the gaps ako flagged (loop and InheritanceSplit caption paths were previously untested). - Add `mdl-examples/bug-tests/263-nested-caption-preservation.mdl` reproducer per CLAUDE.md checklist.
1 parent 1a202e5 commit 94bded8

3 files changed

Lines changed: 144 additions & 0 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
-- ============================================================================
2+
-- Bug #263: Preserve decision/loop captions across nested control flow
3+
-- ============================================================================
4+
--
5+
-- Symptom (before fix):
6+
-- `@caption` on an outer IF/LOOP/WHILE was being overwritten by the inner
7+
-- IF/LOOP's caption because `pendingAnnotations` was shared mutable state
8+
-- across recursive addStatement calls. Annotations attached to the outer
9+
-- split ended up bound to the inner split, and the outer split inherited
10+
-- whatever caption the inner statement carried.
11+
--
12+
-- After fix:
13+
-- addIfStatement / addLoopStatement / addWhileStatement snapshot + clear
14+
-- `pendingAnnotations` before recursing, then re-apply to their own activity
15+
-- after it's created. The WHILE case also gained explicit handling in
16+
-- mergeStatementAnnotations (previously fell through to `default: nil`).
17+
--
18+
-- Usage:
19+
-- mxcli exec mdl-examples/bug-tests/263-nested-caption-preservation.mdl -p app.mpr
20+
-- Open in Studio Pro — each split/loop displays its own caption, not
21+
-- inherited from a nested statement.
22+
-- ============================================================================
23+
24+
create module BugTest263;
25+
26+
create microflow BugTest263.MF_NestedCaptions (
27+
$S: string
28+
)
29+
returns boolean as $ok
30+
begin
31+
declare $ok boolean = false;
32+
33+
@caption 'String not empty?'
34+
if $S != empty then
35+
@caption 'Right format?'
36+
if isMatch($S, 'x') then
37+
return true;
38+
else
39+
return false;
40+
end if;
41+
else
42+
return false;
43+
end if;
44+
end;
45+
/

mdl/executor/cmd_microflows_builder_annotations.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ func getStatementAnnotations(stmt ast.MicroflowStatement) *ast.ActivityAnnotatio
3737
return s.Annotations
3838
case *ast.LoopStmt:
3939
return s.Annotations
40+
case *ast.WhileStmt:
41+
return s.Annotations
4042
case *ast.LogStmt:
4143
return s.Annotations
4244
case *ast.CallMicroflowStmt:
@@ -138,6 +140,12 @@ func (fb *flowBuilder) applyAnnotations(activityID model.ID, ann *ast.ActivityAn
138140
if ann.Caption != "" {
139141
activity.Caption = ann.Caption
140142
}
143+
case *microflows.LoopedActivity:
144+
// LOOP / WHILE activities can carry a caption just like
145+
// splits and action activities.
146+
if ann.Caption != "" {
147+
activity.Caption = ann.Caption
148+
}
141149
}
142150

143151
break

mdl/executor/cmd_microflows_builder_annotations_test.go

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,3 +219,94 @@ func TestIfAnnotationStaysWithCorrectSplit(t *testing.T) {
219219
t.Errorf("inner note destination: got %q, want %q (inner split)", innerNoteDestID, innerSplit.ID)
220220
}
221221
}
222+
223+
// TestLoopCaptionPreserved covers the loop caption case — previously untested
224+
// per PR review. The fix for the outer-IF caption contamination bug also applied
225+
// the same snapshot/restore pattern to addLoopStatement and addWhileStatement.
226+
func TestLoopCaptionPreserved(t *testing.T) {
227+
innerReturn := &ast.ReturnStmt{Value: &ast.LiteralExpr{Value: true, Kind: ast.LiteralBoolean}}
228+
loop := &ast.LoopStmt{
229+
LoopVariable: "item",
230+
ListVariable: "items",
231+
Body: []ast.MicroflowStatement{innerReturn},
232+
Annotations: &ast.ActivityAnnotations{Caption: "Process each item"},
233+
}
234+
235+
fb := &flowBuilder{
236+
posX: 100,
237+
posY: 100,
238+
spacing: HorizontalSpacing,
239+
varTypes: map[string]string{"items": "List of MyMod.Item"},
240+
declaredVars: map[string]string{"items": "List of MyMod.Item"},
241+
}
242+
fb.buildFlowGraph([]ast.MicroflowStatement{loop}, nil)
243+
244+
var loops []*microflows.LoopedActivity
245+
for _, obj := range fb.objects {
246+
if l, ok := obj.(*microflows.LoopedActivity); ok {
247+
loops = append(loops, l)
248+
}
249+
}
250+
251+
if len(loops) != 1 {
252+
t.Fatalf("expected 1 LoopedActivity, got %d", len(loops))
253+
}
254+
if loops[0].Caption != "Process each item" {
255+
t.Errorf("loop caption: got %q, want %q", loops[0].Caption, "Process each item")
256+
}
257+
}
258+
259+
// TestWhileLoopCaptionPreserved — same coverage for the WHILE shape.
260+
func TestWhileLoopCaptionPreserved(t *testing.T) {
261+
whileStmt := &ast.WhileStmt{
262+
Condition: &ast.BinaryExpr{
263+
Left: &ast.VariableExpr{Name: "n"},
264+
Operator: "<",
265+
Right: &ast.LiteralExpr{Value: int64(10), Kind: ast.LiteralInteger},
266+
},
267+
Body: []ast.MicroflowStatement{
268+
&ast.ReturnStmt{Value: &ast.LiteralExpr{Value: true, Kind: ast.LiteralBoolean}},
269+
},
270+
Annotations: &ast.ActivityAnnotations{Caption: "Until n >= 10"},
271+
}
272+
273+
fb := &flowBuilder{
274+
posX: 100,
275+
posY: 100,
276+
spacing: HorizontalSpacing,
277+
varTypes: map[string]string{"n": "Integer"},
278+
declaredVars: map[string]string{"n": "Integer"},
279+
}
280+
fb.buildFlowGraph([]ast.MicroflowStatement{whileStmt}, nil)
281+
282+
var loops []*microflows.LoopedActivity
283+
for _, obj := range fb.objects {
284+
if l, ok := obj.(*microflows.LoopedActivity); ok {
285+
loops = append(loops, l)
286+
}
287+
}
288+
289+
if len(loops) != 1 {
290+
t.Fatalf("expected 1 LoopedActivity (WHILE), got %d", len(loops))
291+
}
292+
if loops[0].Caption != "Until n >= 10" {
293+
t.Errorf("while caption: got %q, want %q", loops[0].Caption, "Until n >= 10")
294+
}
295+
}
296+
297+
// TestInheritanceSplitCaptionApplied — InheritanceSplit is not produced by the
298+
// executor builder (only parsed from BSON for roundtrip), but applyAnnotations
299+
// gained an InheritanceSplit case in the fix. Test the applicator directly.
300+
func TestInheritanceSplitCaptionApplied(t *testing.T) {
301+
split := &microflows.InheritanceSplit{}
302+
split.ID = "inh-split-1"
303+
304+
fb := &flowBuilder{}
305+
fb.objects = append(fb.objects, split)
306+
307+
fb.applyAnnotations(split.ID, &ast.ActivityAnnotations{Caption: "Customer type?"})
308+
309+
if split.Caption != "Customer type?" {
310+
t.Errorf("inheritance split caption: got %q, want %q", split.Caption, "Customer type?")
311+
}
312+
}

0 commit comments

Comments
 (0)