Skip to content

feat: mxcli layout --only-overlapping, and domain-model positions are top-left corners - #1480

Closed
stagknee wants to merge 3 commits into
mendixlabs:mainfrom
stagknee:feat/layout-only-overlapping
Closed

stagknee wants to merge 3 commits into
mendixlabs:mainfrom
stagknee:feat/layout-only-overlapping

Conversation

@stagknee

Copy link
Copy Markdown
Contributor

Closes #1479.

There are three commits, and each one stands on its own.

1. fix: view, external and contract-imported entities use the grid placement

  • create view entity (cmd_entities.go) and create external entity / create external entities from (cmd_odata.go) used the old X: 100 + n*150, Y: 100 row. So did contract imports (cmd_contract.go).
  • They now take dmlayout.GridSlot(existing + created), so the count includes the entities the module already holds. An explicit @Position is still honoured.
  • Tests are in mdl/executor/entity_default_position_test.go, and there is a findings record.

2. feat: mxcli layout --only-overlapping [--dry-run]

  • dmlayout.ResolveOverlaps(dm) returns an OverlapPlan{Pairs, Moves}.
    • Boxes use the same boxSize estimates as Plan.
    • Entities are taken in model (creation) order. An entity stays if it clears every entity kept before it and every annotation; otherwise it moves.
    • Each entity that moves goes to the nearest free point, found by searching outward in 10px square rings. The new spot keeps a 40px gutter to every other box and never goes left of or above the origin.
    • Annotations are obstacles and never move. Their box is a generous estimate, because their origin convention is not verified.
  • An entity that collides with nothing keeps its exact position, and a second run moves nothing.
  • The command reuses the existing layout plumbing: --module, --dry-run and the Marketplace skip. The docs section is in docs-site/src/tools/domain-model-layout.md.

3. fix: an entity's position is the top-left corner of its box, not its centre

  • dmlayout's package comment said a position is the box's centre and cited RelativeMiddlePoint. That is the microflow-object field. A domain-model entity stores Location, and Studio Pro draws it as the top-left corner. Plan added half a box to every position, so a tall box stacked above a short one could run into it.
  • Plan, entityRect and nearestFree now measure from the corner, and the docs line saying "the coordinate is the box's centre" is corrected.
  • How it was verified, in Studio Pro 11.12.5:
    • Entities created at the same Y with 4, 30 and 45 attributes share one top edge, and entities at the same X share one left edge.
    • Three relocated entities land exactly where their stored Location, read as a top-left corner, puts them.
    • Measured box size, in model units: about 150 wide for short names, and about 34 + 14.7 × attributes tall. The boxSize estimates of 160 wide and 34 + 16 × attributes stay, because they are slightly generous.
  • Tests:
    • TestResolveOverlaps_TopLeftConventionFixture sizes boxes independently with the measured geometry rather than boxSize, so it can't simply agree with the code's own convention.
    • TestPlan_TallEntityAboveShortOneDoesNotOverlap.
    • Both failed before the fix. There is also a findings record.

Evidence

  • Studio Pro 11.12.5, scratch module with 16 entities. Attribute counts are 2 to 50, and the module includes a view entity and an entity with an explicit @Position far away. Entities were created with no position.

    • Before: boxes overlapped, a 22-attribute entity into the one below it and a 45-attribute entity into a view entity.
    • --dry-run: listed 4 collisions and 4 moves.
    • The real run: moved 4 of 16 entities and kept the other 12 exactly, including the far-away pinned one.
    • A second run: "no overlapping entities".
    • Studio Pro after the run: no overlaps.

    The first version of commit 2 assumed a centred position. In Studio Pro it still left two overlaps, and that is how commit 3 was found.

  • Unit tests: all 26 in mdl/dmlayout, plus the cmd/mxcli layout tests, pass on Windows. go vet and gofmt are clean.

  • Linux (golang:1.26.6-bookworm, ANTLR 4.13.2, as in CI): make grammar build test check-mdl check-findings check-conformance check-migration-reference lint-go all pass.

🤖 Generated with Claude Code

stagknee and others added 3 commits October 13, 2026 12:57
…ment

CREATE ENTITY places an entity without an @position on dmlayout.GridSlot, but
view entities, external entities and entities imported from a contract kept
the old single-row formula, whose 150px step is narrower than an entity box.
All three now take the next grid slot after the entities the module already
holds; the contract import also counted only its own entities before.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… collide

The default grid has a fixed row pitch, but a box is as tall as its attribute
list, so an entity with many attributes runs into the one below it. The flag
moves just the entities whose estimated boxes (or an annotation) collide, to
the nearest free spot, and keeps every other position. Deterministic and
idempotent: a second run reports no overlapping entities.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…centre

dmlayout treated an entity's Location as the centre of its box (citing
RelativeMiddlePoint, which is the microflow-object field). Studio Pro 11.12.5
draws an entity's Location as the box's top-left corner: entities created at
the same Y with 4, 30 and 45 attributes share one top edge. Plan therefore
placed every box half its size lower and further right than planned, and a
tall box ran into a short one under it; the overlap check missed the same
collisions. Both now measure from the corner.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

AI Code Review

Critical Issues

None found.

Moderate Issues

None found.

Minor Issues

  • In cmd/mxcli/cmd_layout.go, the Example section has a minor formatting quirk where the closing backtick appears on the same line as the last command example:

    mxcli layout -p app.mpr --module Sales --only-overlapping --dry-run`,
    

    While technically correct (it's ending the Example string), it visually looks like the backtick belongs to the command. Consider moving it to the next line for clarity, though this is purely cosmetic.

  • The annotationRect function in overlap.go contains a comment indicating it intentionally covers both possible Location conventions (centre and top-left) because the annotation origin convention isn't verified. While understandable given the uncertainty, this represents a slight imprecision in obstacle modeling. However, the comment adequately explains the rationale, and the approach errs on the side of caution (generously treating annotations as larger obstacles), which is acceptable for correctness.

What Looks Good

  • Atomic commits: Each commit addresses a single concern (positioning fixes for specific entity types, new --only-overlapping flag, position convention fix).
  • Thorough testing:
    • New comprehensive test suite for overlap resolution (overlap_test.go)
    • Executor-level tests verifying grid slot positioning for view/external/contract entities (entity_default_position_test.go)
    • CLI tests for the new layout flag (cmd_layout_test.go)
    • Existing layout tests updated to reflect top-left corner convention
  • Validation evidence:
    • Explicit Studio Pro 11.12.5 validation described
    • Before/after behavior demonstrated with overlap scenarios
    • Idempotence verified (second run produces no changes)
  • Documentation:
    • User-facing documentation updated in docs-site/src/tools/domain-model-layout.md
    • Clear explanation of --only-overlapping behavior with examples
    • Correction of position convention documentation
  • Consistency:
    • Positioning logic now uniformly uses dmlayout.GridSlot() across entity creation paths
    • Layout algorithm correctly respects top-left corner convention throughout
    • Reuses existing CLI plumbing (--module, --dry-run, Marketplace handling)
  • Findings records: Two properly formatted findings records added to document the fixed issues
  • Idempotence: The --only-overlapping implementation correctly produces no changes on subsequent runs when no overlaps exist

Recommendation

Approve. The PR successfully addresses the layout overlap issues and position convention problems with thorough testing, clear documentation, and proper validation. The implementation follows project conventions and maintains backward compatibility where appropriate (explicit @Position annotations are still honored). The minor formatting and comment issues noted do not affect correctness or warrant blocking the merge.


Automated review via OpenRouter (Nemotron Super 120B) — workflow source

@stagknee

Copy link
Copy Markdown
Contributor Author

Superseded: #1479 landed in main as 7402dd1 (via #1503), including the top-left Location convention and layout --only-overlapping, plus placement of new entities clear of tall ones. Closing this one — thanks!

@stagknee stagknee closed this Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant