fix: lay out microflows written without @position so they can be read - #1155
Merged
ako merged 6 commits intoSep 22, 2026
Merged
Conversation
A note was offset 100px from its target's CENTRE. That is above an activity, and inside any box taller than 200px: a note on a loop was drawn on top of the loop's own body. The connector was written with connection indices 0/0, so it was attached to the top of the note rather than the side facing the element. The note now goes above the target's top edge (unchanged for an activity: same y as before), and the connector runs from the note's bottom to the element's top. The describer shares defaultAnnotationGeometry, so `position:` is still omitted exactly when it matches what a rewrite re-derives. A model written by an earlier mxcli stores the old connector sides, so the first re-exec of a flow that has notes writes it once; the second reports Unchanged. Part of mendixlabs#1154. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…a merge Horizontal space the layout spent on nothing (MF_LayoutGuards in mdl-examples/bug-tests/microflow-layout-rows-lanes-cases.mdl shows the first): - A guard - `if X then ...; return; end if` - draws its branch in the lane BELOW the main line and ends it there, yet the next element was placed past the branch's far end: 370px from the split, where two activities stand 40px apart. The main line now resumes in the column the branch starts in, and the lane records how far it is taken (layout_lanes.go). Only an element that reaches down into that lane - another decision, a loop box, an activity with an error handler - waits for it. - mergeX added HorizontalSpacing/2 to a branch width that is already edge to edge: 120px before every merge. It is now the same 40px as between activities. - With the merge that close, a branch ending in RETURN has to be measured with the end event it draws (measureBranch), or the merge lands on it. - HorizontalSpacing is a centre-to-centre pitch, and measureStatements added it on top of both widths: two activities measured 400 wide and occupy 280, so every IF sized from that put its merge a whole activity too far right per statement. gapBetween tightens only the pairs whose arithmetic is exact and keeps the generous gap elsewhere - a merge short of its branch is worse than one long. - A loop box keeps the same 40px from its neighbours as an activity; it used to touch them because the cursor treated the box as activity-wide. The measurer mirrors the lanes: a run's width is max(main line, lane), not a sum. Part of mendixlabs#1154. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The builder lays the happy path left to right and never stops: every top-level statement advances posX, nothing moves down. A flow of 40 activities is one row from x=360 to x=6600 - four screens of sideways scrolling for something 60px tall (MF_LayoutLong in mdl-examples/bug-tests/microflow-layout-rows-lanes-cases.mdl). Past MaxRowWidth (2880px, two Studio Pro canvases) the next top-level statement now starts a new row. - A row's depth is not known when it begins: a loop box is fitted AFTER its body (250 estimated against 310 built) and a note adds its own height. So wrapRow only takes a first position and separateRows measures the finished rows once and moves them apart to RowGap. An earlier version estimated the height instead; a control experiment showed its test passing with the estimate removed, so it is gone. - The flow joining two rows leaves the BOTTOM of the last element and arrives on TOP of the first, so it runs through the band between the rows. Right-to-left it was drawn back across the row it left; bottom-to-left still came in over the first activities of the new row. An element with a note above it keeps the left side. - RowGap is 90: Studio Pro prints the output variable and its type under an activity, and at 40 that text touched the row below. - A tail that finishes within RowOverhang stays on its row rather than becoming a three-element row underneath. A RETURN never starts a row. - A statement with @position is never moved (ADR-0008); the row re-anchors to it. - Only the whole-microflow builder wraps; loop bodies and branches do not. - Whether a tail fits is asked of the measurer, which is why this follows the commit that makes it measure a run by what it occupies. Part of mendixlabs#1154. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The anchor pair on an enum case flow is not geometry: 0e69243 made it the storage for CASE order, one distinct pair per case, because stored flow order does not survive serialization. Past the third pair the table runs out of sides a drawing would choose - case 4 leaves the split's LEFT corner, cases 5-8 arrive on TOP of their activity, 9+ on its far side - so a 7-case split is a tangle of lines crossing each other and the activities between them. For four or more cases the lines now leave in three groups: the upper third from the split's top corner, the middle from its right, the lower from its bottom, all arriving on the left, and mirrored onto the closing merge. Branches are stacked top to bottom in case order, so no two lines cross. The side is checked against where the branch actually stands, so a branch moved with @position never gets a line that leaves away from it. DESCRIBE reads the order as the pair's rank, then the branch's Y. The table already ranks top/right/bottom-to-left as 0/1/2, so this is one reading path: a model written with one pair per case never reaches the tie-break and reads exactly as before, and a split of up to three cases is written exactly as before. Deliberate trade, pinned by TestHandPlacedCaseBranchesAreReadInCanvasOrder: with 4+ cases whose branches a user has re-ordered on the canvas with @position, DESCRIBE lists them top to bottom rather than as typed. CASE branches have no run-time order, and describe -> exec remains a fixed point. maxEnumSplitBranches stays at 16 (the table's length) although grouped lines no longer need the table past its third entry; lifting the limit is a separate change. A flow with a 4+ case split written by an earlier mxcli is rewritten once on re-exec. Part of mendixlabs#1154. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- `mxcli syntax microflow.layout`, the write-microflows skill (reference/control-flow.md) and the docs-site page say what happens when @position is left out, and why a few hand-placed statements in an otherwise automatic flow are what produces overlaps. - mdl-examples/bug-tests/microflow-layout-rows-lanes-cases.mdl: three flows to open in Studio Pro (a 40-activity line, three guards, a 7-case split). - One finding: nothing automatic sees this class, so measure; an anchor that looks like layout may be storage; do not estimate what can be measured afterwards. Part of mendixlabs#1154. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
AI Code ReviewReview SummaryCritical IssuesNone found. Moderate IssuesNone found. Minor IssuesNone found. What Looks Good
RecommendationApprove the PR. The changes are well-tested, correctly implement the layout improvements for microflows without Automated review via OpenRouter (Nemotron Super 120B) — workflow source |
This was referenced Sep 22, 2026
# Conflicts: # CHANGELOG.md
ako
added a commit
to tgolembiewski/mxcli
that referenced
this pull request
Sep 22, 2026
02c's five flows predate named `merge` / `join`, so nothing in the file exercises a backward jump, an error handler rejoining the main flow, or crossed branches — the shapes a nested IF cannot spell, and the ones with no coverage when the layout algorithm changes. Pattern 6 (CX_SYNC_FactoryTelemetry) covers all of them in one flow: 37 elements, 4 merges, 11 joins — a retry whose handler jumps BACKWARD to a merge above the call, three error handlers joining forward, an inner and an outer split whose FALSE arms land on the same merge, a fall-through handler, and a funnel where three paths end. Measured on a Mendix 11.14.0 app: mendixlabs#1155 lays it out at 2995x540 against 4790x350 on current main — same 37 elements, flow identical apart from geometry. It is also a repro for two gaps left in mendixlabs#923, both of which reproduce on the pre-mendixlabs#1155 build: a one-inbound merge that `exec` writes and the next `exec` deletes, and a flow that goes from describing faithfully (0 warnings) to "must not be re-executed" (2 warnings) after a single describe -> exec. Evidence in mendixlabs#923. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AI Code ReviewLet me review this PR against the checklist. The PR is about fixing microflow layout for flows written without First, I'll check the PR description and commits to understand what's being changed. The PR body says:
Now let me go through the review checklist: Overlap & duplication
MDL syntax design (for new or modified MDL syntax)The PR states: "no MDL syntax change" - so this section should not apply. Let me verify:
Full-stack consistency (for MDL features)Since there are no MDL syntax changes (just bug fixes to existing layout logic), this section doesn't apply. The PR is fixing how existing MDL is interpreted/layoutted, not adding new MDL features. Test coverage
Security & robustness
Scope & atomicity
Code quality
Bugs & correctness
Now let me look at specific code changes to verify correctness: From the diff, I can see:
Let me check if there are any issues:
Now let me check against the checklist items more specifically: Overlap & duplication
MDL syntax design
Full-stack consistency
Test coverage
Security & robustness
Scope & atomicity
Code quality
Bugs & correctness
One thing I notice: in func (fb *flowBuilder) wrapRow(noteAbove bool) {
next := fb.rowBottom() + RowGap + ActivityHeight/2
fb.posX = fb.row.startX
fb.posY = next
fb.baseY = next
fb.row.firstObject = len(fb.objects)
fb.row.pendingWrapEdge = true
fb.row.wrapNoteAbove = noteAbove
fb.row.starts = append(fb.row.starts, fb.row.firstObject)
}This sets fb.posY to the bottom of the current row plus RowGap plus ActivityHeight/2. In func (fb *flowBuilder) takeWrapAnchors() (origin, destination int, ok bool) {
// ...
if flow.OriginConnectionIndex != AnchorBottom || flow.DestinationConnectionIndex != AnchorTop {
t.Fatalf("wrap edge (%d,%d)->(%d,%d) anchored %d->%d, want bottom(%d)->top(%d)",
from.X, from.Y, to.X, to.Y,
flow.OriginConnectionIndex, flow.DestinationConnectionIndex, AnchorBottom, AnchorTop)
}This validates that wrap edges go from bottom to top, which matches the comment. In func (m *layoutMeasurer) measureBranch(stmts []ast.MicroflowStatement) Bounds {
b := m.measureStatements(stmts)
if !lastStmtIsReturn(stmts) {
return b
}
if b.Width == 0 {
// A bare RETURN: the end event stands where the first activity would.
b.Width = ActivityWidth/2 + EventSize/2
} else {
b.Width += HorizontalSpacing - ActivityWidth/2 + EventSize/2
}
return b
}This seems correct: for a RETURN at the end of a branch, we need to account for the end event that gets drawn after the last activity. The func gapBetween(prev, next ast.MicroflowStatement) int {
// Distance from prev's measured right edge to the centre the builder gives next.
var toNextCentre int
switch prev.(type) {
case *ast.IfStmt:
// After an IF, posX = (measured right edge) + HorizontalSpacing/2.
toNextCentre = HorizontalSpacing / 2
case *ast.EnumSplitStmt, *ast.InheritanceSplitStmt:
return HorizontalSpacing
default:
// A simple activity, or a loop box: after either, the builder centres the
// next element one activity-gap plus half an activity past the right edge.
// A simple activity: centre-to-centre pitch, less its own right half.
toNextCentre = HorizontalSpacing - ActivityWidth/2
}
// How far next reaches left of the centre it is given.
var leftHalf int
switch next.(type) {
case *ast.IfStmt, *ast.EnumSplitStmt:
leftHalf = SplitWidth / 2
default:
// An activity — and a loop box, whose left edge the builder now puts where
// an activity's would be.
leftHalf = ActivityWidth / 2
}
---
*Automated review via OpenRouter (Nemotron Super 120B) — [workflow source](https://github.com/mendixlabs/mxcli/blob/main/.github/workflows/ai-review.yml)* |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1154
What
The default layout of a microflow written without
@position. Geometry and connection sides only: no MDL syntax change, no change to what a flow does, and a statement carrying@positionis never moved.Reproduce with
mdl-examples/bug-tests/microflow-layout-rows-lanes-cases.mdl(three flows, any blank app), thendescribe microflow:MF_LayoutLong, 40 activitiesMF_LayoutGuards, three guardsMF_LayoutCases, 7 cases@anchor(to: top)Also: a note is placed above its element's top edge instead of 100 px above its centre (inside any loop box), with the connector bottom → top; a merge stands 40 px after its branch instead of 120; loop boxes get the same gap as activities.
Commits
One concern each; each builds and passes
go test ./mdl/executor/.layout_lanes.go), merge gap, returning branch measured with its end event, run measured by what it occupieslayout_rows.go); rows are separated by measuring what was built, not by estimatingdescribereading the case order from pair rank, then the branch's Ymxcli syntax microflow.layout, write-microflows reference, docs-site, CHANGELOG, the bug-test script, one findingFor the reviewer
describe → execwith this build writes it once and the second isUnchanged. With the script above: model written by v0.23.0, re-executed with this branch —MF_LayoutLongandMF_LayoutGuardsUnchanged,MF_LayoutCasesReplacedthenUnchanged.describetext is identical before and after.@position,describelists them top to bottom rather than as typed. Pinned byTestHandPlacedCaseBranchesAreReadInCanvasOrder.maxEnumSplitBranchesstays at 16; lifting it is a separate change.Testing
make build,make test,make lint-go,make check-mdl,make check-findings,make check-wiki-pages,scripts/check-tunnel-deps.sh,scripts/check-skill-mdl.sh(skills, docs-site),mdbook build docs-site,govulncheck: pass.make test-integrationwithmx11.12.1 on macOS: passes. (It needs a shortTMPDIRthere; with the default onemx create-projectfails withPathTooLongExceptionand the roundtrip tests reportokwithout having created a project.)mx check0 errors after executing the script;describe → execof its three flows reportsUnchanged.