fix: place a merge, a lane and the next statement where the measurer says

.claude/skills/fix-issue/findings/mdl-executor.jsonl

@@ -670,3 +670,4 @@
 {"area": "mdl/executor", "date": "2026-09-21", "symptom": "`create or modify entity` drops an attribute a LATER script added, silently. Reported shape: entity created in 01-domain-core.mdl, a calculated attribute added in 03-logic.mdl (its microflow does not exist until then); re-running slice 01 ALONE rebuilt the entity from its own statement and removed the attribute, with `Modified entity: ServiceCore.LithoSystem` as the only output. It surfaced two slices later as `[CE1613] \"The selected attribute 'ServiceCore.LithoSystem.OpenRequestCount' no longer exists.\" at Text 'dtOpen'` — an error naming the PAGE, never the script that removed the attribute. `mxcli check … -p app.mpr --references` said \"Check passed!\".", "ce": "CE1613", "rules": ["MDL087"], "cause": "Half the ask was already shipped and half was not, and the report could not tell them apart. exec's warning (droppedEntityMembers, findings #24, landed 320a304 two weeks before the report) DOES fire — measured on a real 11.6.6 project re-running the reporter's slice 01, it prints the attribute by name — so the reporter was on an older binary. What genuinely did not exist was the issue's second ask: `check` had no project-aware pass for member loss at all, so the one command that runs BEFORE anything is written was the silent one. Added CheckEntityMemberDrops (MDL087, warning) to cmd_check.go's catalog-backed tier, and refactored droppedEntityMembers to share its comparison.", "file": "`mdl/executor/validate_entity_member_drops.go` (new: entityMemberSet, droppedMembers, CheckEntityMemberDrops), `mdl/executor/cmd_entities.go` (droppedEntityMembers now delegates), `cmd/mxcli/cmd_check.go` (projectViolations)", "insight": "**Reproduce before theorising when the report predates a fix in the same area** — exec already printed the exact line the issue asks for, so reading the issue text alone leads either to 'already fixed, close it' or to reimplementing the shipped half. Running the reporter's own sequence against a real project separated the two halves in one command each, and the isolated-slice check printing `Check passed!` is what identified the actual gap. **A check-time twin of an exec-time warning must NOT be the same computation.** exec is per-statement because it is applying statements; check sees the whole script, so it has to be the NET effect — a script that rebuilds an entity and then `alter entity … add attribute`s the members back loses nothing, and that is the IDIOMATIC full-script order, so a per-statement port would warn on every correct script and be switched off within a day. **Intent has to be tracked, not inferred from the outcome**: `drop attribute` / `rename attribute` / `drop entity` produce the same before/after diff as the accident, and a pure diff cannot separate them. Both of those are separate controls, and the naive implementation fails each one specifically (measured: stubbing the net/intent logic fails TestMDL087_ExplicitRemovalIsSilent on 3 of 4 spellings while the positive test still passes — so the positive test alone proves nothing). **One comparison, two layers**: the audit system fields and an omitted `extends` were reported by exec and would have been missed by a second hand-written diff, which is why droppedEntityMembers was refactored onto the shared entityMemberSet rather than copied. An audit pseudo-type (`AutoOwner`) is a FLAG, not an attribute — exec `continue`s past it — so counting it as one makes a faithful restatement read as a drop.", "refs": ["ako/mxcli#562", "findings #24", "findings #13"]}
 {"area": "mdl/executor", "date": "2026-09-21", "symptom": "`retrieve $AccountList from Administration.Account sort by System.Language.Code asc;` — MDL that `mxcli describe` had just emitted — passed `mxcli check` and was refused by `mxcli exec`: \"sort by attribute 'System.Language.Code' does not belong to entity 'Administration.Account'\". Reported as a check/exec inconsistency (mendixlabs/mxcli#1152); the real defect is that the round trip cannot replay its own output for any sort over an association reached from an ANCESTOR.", "cause": "inferSortEntityRefSteps searched ONE domain model — the retrieved entity's own module — for associations whose parent was the retrieved entity ITSELF, and qualified the association it found with the retrieved entity's module. All three assumptions hold only when the hop starts on the retrieved entity in its own module. Administration.Account reaches System.Language through System.User_Language, declared on System.User and stored in the System module: parent is an ancestor, the domain model is another module's, and the qualified name carries THAT module. Rewritten as a generalization-chain walk that looks each ancestor up in its own module and qualifies the association with the module storing it; the destination end is matched with entityIsSubtypeOf rather than by equality, since an association may point at a specialization of the entity that declares the attribute.", "file": "`mdl/executor/cmd_microflows_builder_actions.go` (inferSortEntityRefSteps); tests `mdl/executor/cmd_microflows_sort_association_test.go`, `mdl/backend/modelsdk/microflow_retrievesort_test.go`; example `mdl-examples/bug-tests/microflow-1152-sort-over-association.mdl`", "insight": "**The second control is the one that pays.** Reverting the fix reproduces the refusal, which only proves the test fires. The control that taught something was building a binary that DERIVES the hop and does not WRITE it — exec succeeds and mxbuild 11.12.3 answers CE7247 \"Cannot sort on attribute 'System.Language.Code'. Attribute 'System.Language.Code' is not an attribute of entity 'Administration.Account'\" — the executor's refusal message almost word for word, from the other end of the pipeline. That is what fixes the qualified name as load-bearing: the stored EntityRefStep must read System.User_Language, and the pre-existing code would have written Administration.User_Language had it found anything at all. **Skip the theory that check is missing a rule**: check has no sort-attribute rule at all and resolves no hops, so it was never going to disagree with exec here — the inconsistency in the report is a symptom of the false refusal, not a second defect. **Known residue, stated because the round trip rests on it**: DESCRIBE emits only the attribute's qualified name, so where several associations reach one entity the replay picks the nearest ancestor's first and can silently land on the other hop. Spelling the hop needs grammar (sortColumn is qualifiedName|IDENTIFIER, no `/` path) and is a language change, not a fix."}
 {"area": "mdl/executor", "date": "2026-09-21", "symptom": "Follow-up to the sort-hop inference fix: with the hop derivable but not SAYABLE, `describe → exec` still silently changed the program wherever two associations reach the same entity. Measured on 11.12.3 with Order_ShipTo and Order_BillTo (both Order -> Address): a microflow sorting by the BILLING address came back sorting by the SHIPPING one, `mx check` 0 errors on both sides. Same for a page datasource's sort bar.", "cause": "DESCRIBE emitted only the sort attribute's qualified name and the reader never looked at the hop at all — `sortItemsFromRaw` read AttributeRef.Attribute and skipped AttributeRef.EntityRef, so the association was written and never read back. MDL had no spelling for it either (`sortColumn : (qualifiedName | IDENTIFIER)`). Closed end to end: sortColumn takes `qualifiedName (SLASH qualifiedName)*` (the shape MDLCatalog.g4 already uses for Association/Entity), SortColumnDef/OrderByItemV3 carry the hops, the executor resolves the NAMED association instead of inferring, both readers reconstruct EntityRef.Steps, both describers emit `Assoc/.../Attr`, and the page writers moved from attributeRefToGen to inputAttributeRefToGen. Inference stays as the fallback, so every script written before still works.", "file": "`mdl/grammar/domains/MDLPage.g4` (sortColumn) + `mdl/ast/ast_page.go`/`ast_page_v3.go` + `mdl/visitor/visitor_microflow_statements.go` (sortColumnHops) + `visitor_page_v3.go` + `mdl/executor/cmd_microflows_builder_actions.go` (resolveSortAssociationPath, lookupSortHop, entityChainModules) + `cmd_microflows_format_action.go` + `cmd_pages_builder_v3.go` (resolveAssociationAttributePathForEntity) + `cmd_pages_describe_datasource.go` (sortAttributeHops, sortColumnPath) + `mdl/backend/modelsdk/microflow_read_actions.go` (entityRefStepsFromRaw) + `widget_write.go` + `sdk/pages/pages_datasources.go` (GridSort.AttributeRefSteps)", "insight": "**The measurement that decides whether a lossy describer is worth a language change is a CONSTRUCTED one.** The corpus agrees with the inference rule by construction — every document mxcli itself wrote stores the association inference would have picked, so the round trip is a fixed point on everything to hand and looks faithful. The case that matters had to be built: two associations to one entity, then the stored hop edited to the one inference does NOT pick. Byte-patching the .mxunit is enough and takes a minute — `Order_ShipTo` and `Order_BillTo` are the same length, so a `sed` on the BSON needs no resize — and the replay flipped it back immediately. **Control on a binary that drops the hop, not just on one that reverts the fix**: reverting only proves the test fires, while dropping the hop gets mxbuild to say CE7247 \"Cannot sort on attribute … is not an attribute of entity …\" — the executor's own refusal message from the other end of the pipeline, which is what proves the EntityRef load-bearing rather than cosmetic. **Two reads were missing, not one**: the microflow reader and the page reader each drop the hop separately, and fixing only the half named in the report would have shipped a describer that emits the path for microflows and silently drops it for pages. **The strongest round-trip evidence is 'Unchanged'** — with identity preservation and write elision, replaying DESCRIBE output on a correct implementation elides the write entirely, so `Unchanged microflow: …` is a stronger result than any byte comparison."}
+{"area": "mdl/executor", "date": "2026-09-22", "symptom": "Elements of a generated microflow overlap wherever the shape needs a nested decision, a named `merge <label>` or a `case` with several branches (mendixlabs/mxcli#1158). Measured on mdl-examples/doctype-tests/02c-complex-layout-examples.mdl: 15 overlapping pairs in 3 of its 6 flows - an activity sitting on the merge that closes a split, a branch lane 50px inside the lane above it, and the first statement after a `merge` with its left edge on the merge's right edge. `mx check` reports 0 errors either way.", "cause": "Three fixed-size disagreements between layout.go's measurer and the builder, each harmless on its own and none caught by a test. (1) measureIfStatement/measureEnumSplitStatement stopped at `SplitWidth + HorizontalSpacing/2 + branchWidth + MergeSize`, which is the branch's extent plus a merge's width rather than the distance from the split's LEFT edge to the merge's RIGHT edge - 25px short. (2) `fb.posX = mergeX + HorizontalSpacing/2` (enum split) and `fb.posX += fb.spacing/2` (addMergeStatement) advance from the merge's CENTRE by what an activity's half-width needs; a merge is MergeSize=40 wide against ActivityWidth=120, so the gap came out 0. addIfStatement always used `mergeX + MergeSize + HorizontalSpacing/2` and was right. (3) `elseCenterY = centerY + thenH/2 + BranchGap + elseH/2` (and the same arithmetic stacking enum branches) treats a branch's measured HEIGHT as symmetric about its line, but a branch holding a nested IF hangs entirely below it: measured 160 tall, it occupies 30 above the line and 130 below, so the next lane landed 50px inside it.", "file": "`mdl/executor/layout.go` (splitWidthWithMerge, gapBetween), `mdl/executor/layout_lanes.go` (guardBranchInset, lowestBetween), `mdl/executor/cmd_microflows_builder_merge.go` (addMergeStatement), `mdl/executor/cmd_microflows_builder_actions.go` (addEnumSplit), `mdl/executor/cmd_microflows_builder_control.go` (addIfStatement)", "insight": "**A second number describing the same thing is a defect waiting for someone to tighten a constant.** These three were all present before the row/lane work and all hidden by slack in the spacing; removing the slack turned them into overlaps within a day. The lasting fix is not the arithmetic but the test shape: `TestMeasuredWidthMatchesWhatIsBuilt` asks the measurer and the builder the same question about the same run and compares, and `TestBuiltElementsDoNotOverlap` asserts the invariant directly, so the next constant that moves fails loudly instead of silently. **Reach for the real geometry over a model of it whenever the thing is already built** - the branch above a lane exists by the time the lane is placed (lowestBetween), exactly as separateRows already measures finished rows. Two traps when doing that: bound the range at BOTH ends, because a split's shared merge is appended among its branches and an open range measures it as part of whichever branch came before; and a control experiment must be run per fix - three of these tests passed with their fix reverted until the shapes were widened, because two elements can be vertically clear of each other simply by being in different columns.", "refs": ["mendixlabs/mxcli#1158", "mendixlabs/mxcli#1154", "mendixlabs/mxcli#1157"]}

CHANGELOG.md

@@ -6,6 +6,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ## [Unreleased]
 
+### Fixed
+
+- **Elements overlapped where a microflow's layout needed a nested decision, a named merge or a wide `case`** (mendixlabs/mxcli#1158). Three fixed-size disagreements between what `measureStatements` predicts and where the builder actually places things — every merge, every following statement and every branch lane is positioned from the first and drawn by the second, so a few pixels of disagreement is an overlap rather than a cosmetic difference. Measured on `mdl-examples/doctype-tests/02c-complex-layout-examples.mdl`: **15 overlapping pairs in 3 of its 6 flows, down to none** (`mx check` 0 errors before and after).
+
+  - **A split was measured to its branch, not to the merge that closes it** — 25 px short, because the merge is what a following statement has to clear. Nothing noticed while an extra half pitch was still being added on top of every split's width.
+  - **The advance past a merge came from its centre as though a merge were an activity's width.** Half a pitch is 80 px; a merge is 40 wide against an activity's 120, so the next element's left edge landed exactly on the merge's right edge. Both the `case` statement's own merge and `merge <label>` now clear the merge first and then leave the ordinary gap, which is what `if` has always done.
+  - **A branch lane was placed half the branch's measured HEIGHT below the line.** That is only right for content centred on its line: a branch holding a nested `if` hangs entirely below its own, so a lane placed on the measurement landed 50 px inside it. The branch above is already built when the next lane is placed, so its real extent is measured instead of modelled.
+
 ### Changed
 
 - **Microflows written without `@position` are laid out to be read, not just to be valid** (mendixlabs/mxcli#1154). Geometry and connection sides only: no MDL syntax changes, a statement carrying `@position` is never moved, and `describe` → `exec` still reports `Unchanged microflow`. Measured on a generated app of 41 microflows, none with an `@position`: the widest flow went from 6930×160 px on one row to 3220 px, with 0 overlapping elements and `mx check` at 0 errors before and after.

mdl/executor/cmd_microflows_builder_actions.go

@@ -475,6 +475,11 @@ func (fb *flowBuilder) addEnumSplit(s *ast.EnumSplitStmt) model.ID {
 		}
 	}
 
+	// The branch above's own objects, so a lane can be placed clear of what it really
+	// occupies. Bounded at both ends: the merge that closes the split is appended
+	// while the branches are being built, and it sits on the centre line, so an open
+	// range would measure it as part of whichever branch came before it.
+	prevBranchStart, prevBranchEnd := 0, 0
 	origins := enumSplitOriginAnchors(branchYs, centerY)
 	slot := func(i int) splitCaseSlot {
 		if origins == nil {
@@ -486,6 +491,15 @@ func (fb *flowBuilder) addEnumSplit(s *ast.EnumSplitStmt) model.ID {
 	savedEndsWithReturn := fb.endsWithReturn
 	allBranchesReturn := len(branches) > 0
 	for i, br := range branches {
+		if i > 0 {
+			// Clear of what the branch above actually occupies, not of half its
+			// measured height: a branch holding a nested IF hangs below its own line.
+			fallback := branchYs[i-1] + ActivityHeight/2
+			if below := fb.lowestBetween(prevBranchStart, prevBranchEnd, fallback) + BranchGap + branchHeights[i]/2; below > branchYs[i] {
+				branchYs[i] = below
+			}
+		}
+		prevBranchStart = len(fb.objects)
 		branchY := branchYs[i]
 		fb.posX = splitX + SplitWidth + HorizontalSpacing/2
 		fb.posY = branchY
@@ -554,6 +568,7 @@ func (fb *flowBuilder) addEnumSplit(s *ast.EnumSplitStmt) model.ID {
 				lastID = actID
 			}
 		}
+		prevBranchEnd = len(fb.objects)
 
 		if lastStmtIsReturn(br.body) {
 			continue
@@ -575,7 +590,7 @@ func (fb *flowBuilder) addEnumSplit(s *ast.EnumSplitStmt) model.ID {
 		}
 	}
 
-	fb.posX = mergeX + HorizontalSpacing/2
+	fb.posX = mergeX + MergeSize + HorizontalSpacing/2
 	fb.posY = centerY
 	fb.endsWithReturn = savedEndsWithReturn
 	if allBranchesReturn {

mdl/executor/cmd_microflows_builder_control.go

@@ -149,6 +149,7 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
 
 	if hasElseBody {
 		// IF WITH ELSE: TRUE path horizontal (happy path), FALSE path below
+		thenFirstObject := len(fb.objects)
 		fb.posX = thenStartX
 		fb.posY = centerY
 		fb.endsWithReturn = false
@@ -241,10 +242,15 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
 			fb.addPendingErrorHandlerFlowTo(mergeID)
 		}
 
-		// Process ELSE body (below the THEN path)
+		// Process ELSE body (below the THEN path), clear of what the THEN path
+		// actually occupies — see lowestSince: a THEN that contains a nested IF hangs
+		// below its own line, and half its measured height does not reach that far.
 		thenH := max(thenBounds.Height, ActivityHeight)
 		elseH := max(elseBounds.Height, ActivityHeight)
 		elseCenterY := centerY + thenH/2 + BranchGap + elseH/2
+		if built := fb.lowestBetween(thenFirstObject, -1, centerY+ActivityHeight/2) + BranchGap + elseH/2; built > elseCenterY {
+			elseCenterY = built
+		}
 		fb.posX = thenStartX
 		fb.posY = elseCenterY
 		fb.endsWithReturn = false

mdl/executor/cmd_microflows_builder_merge.go

@@ -109,7 +109,11 @@ func (fb *flowBuilder) addMergeStatement(s *ast.MergeStmt) model.ID {
 	// Adopt the declaration's position even when a forward `join` created the
 	// object earlier at whatever the cursor happened to be.
 	m.Position = model.Point{X: fb.posX, Y: fb.posY}
-	fb.posX += fb.spacing / 2
+	// A merge is MergeSize wide, not an activity's width, so half a pitch from its
+	// CENTRE put the next activity's edge exactly on the merge's. Clear the merge
+	// first, then leave the ordinary gap — the same arithmetic addIfStatement uses
+	// after the merge that closes a split.
+	fb.posX += MergeSize + fb.spacing/2
 	// A merge is a join point, not a terminator: whatever follows continues from
 	// it, so an end event is owed again even if the path that reached here
 	// arrived by `join`.