feat: @anchor annotation for microflow sequence flow endpoints

[hjotha]

Summary

Implements the @anchor(from: X, to: Y) annotation proposed in #275 so that DESCRIBE MICROFLOW output and mxcli exec roundtrips preserve each SequenceFlow's connection side — previously dropped on describe and always re-derived from the visual direction on exec.

Syntax

Simple form — for non-split statements:

@anchor(from: right, to: left)
log info node 'App' 'hello';

Each side is independently optional; missing sides fall back to the builder default so existing scripts are unchanged.

Combined form — for IF, carries the incoming side plus per-branch outgoing sides:

@anchor(to: top, true: (from: right, to: left), false: (from: bottom, to: top))
if condition then
    ...
else
    ...
end if;

Values: top (0), right (1), bottom (2), left (3) — matching the BSON OriginConnectionIndex / DestinationConnectionIndex.

Ancillary fixes

Enforcing the roundtrip invariant exposed three pre-existing bugs in the describer that would have blocked the annotation from surviving describe → check:

1. Expression string literals double-escaped backslashes. isMatch($x, '^\d+$') round-tripped as '^\\d+$', then '^\\\\d+$', etc. New quoteExpressionLiteral escapes only what the lexer requires (\n, \r, \t, trailing \, backslash followed by an escape letter, and apostrophe) and passes regex-escape sequences through verbatim.
2. Loop describe output omitted begin. Grammar requires loop $X in $Y begin ... end loop; but the describer wrote the header directly before the body. It happened to parse before because every body statement's leading keyword was accepted. With @anchor landing before the first body statement, the parser saw @position(...) right after the loop header and failed with cascade errors.
3. @annotation / @caption emitted raw control characters. Free annotations used a home-grown escape that only doubled apostrophes; multiline @annotation text (as in MxAdmin.CreateMendixVersionFromString) was then rejected by the parser on re-execute. Switched to mdlQuote.

Implementation highlights

• AST: ast.AnchorSide, ast.FlowAnchors; ActivityAnnotations gains Anchor, TrueBranchAnchor, FalseBranchAnchor (plus IteratorAnchor / BodyTailAnchor reserved for a follow-up on LOOP/WHILE internal flows).
• Grammar: ANCHOR, TOP, BOTTOM lexer tokens; anchorSide production + nested annotationParenValue for the split form.
• Visitor: populates anchor fields from annotation contexts with the same keyword tolerance as the rest of the MDL visitor.
• Builder: applyUserAnchors helper; mergeStatementAnnotations carries anchor fields into pendingAnnotations; buildFlowGraph and addIfStatement apply them per the user's spec. Guard-pattern IFs (no else, then returns) use new nextFlowAnchor plumbing so the deferred split→nextActivity flow also honours @anchor(false: ...).
• Describer: emitAnchorAnnotation outputs the simple form for non-split activities; split activities delegate to emitSplitAnchorAnnotation. Branch detection reuses findBranchFlows so all CaseValue variants (ExpressionCase, EnumerationCase, BooleanCase, value + pointer) are covered.

Test coverage

• Grammar / visitor: 5 cases (simple syntax, partial, 4 sides, split form, absence-is-nil).
• Builder: 7 cases (override, default preserved, partial override, per-branch IF anchor, guard-pattern IF carry-through).
• Describer: 6 cases (simple form, split form with each CaseValue variant, no-flows skip, expression literal escape, loop begin emission).
• Regression: cmd_microflows_anchor_if_test.go pins the attempt-Fix batch of reported issues (#18, #19, #23, #25, #26, #27, #28) #35 scenario (anchor inside ELSE branch + return with to: top).

All tests run with go test ./... green (29 packages, 0 failures).

E2E verification

Control Centre (Mendix 9.24, ~200 microflows). 5 representative microflows including the attempt-#35 repro:

• Apps.GetOrCreateMendixVersionFromString — 0 anchor drifts
• RepositoryServiceIntegration.GetDisplayVersion — 0 drifts
• MxAdmin.CreateMendixVersionFromString — 0 drifts; also validates multiline @annotation with '' and \r\n survives roundtrip
• AcademyIntegration.GetOrCreateCertificate — guard-pattern IF, 0 drifts
• Administration.ChangePassword — 0 drifts

All 5 pass mxcli check on the describe output AND round-trip describe → exec → describe with byte-exact anchor preservation.

Closes

#275

[github-actions]

AI Code Review

Critical Issues

None found.

Moderate Issues

None found.

Minor Issues

• The PR description mentions that IteratorAnchor and BodyTailAnchor are "reserved for a follow-up on LOOP/WHILE internal flows" but these fields are actually initialized in the AST (in ActivityAnnotations struct). While not harmful, it would be cleaner to either:

  1. Initialize them as nil and document they're for future use, or
  2. Add them in a separate follow-up PR
     However, since they're just nil pointers and don't affect current functionality, this is very minor.

• In cmd_microflows_show_helpers.go, the emitObjectAnnotations function has a comment that says "// @anchor (emit whenever attached flows exist, for roundtrip fidelity)" but the actual implementation is several lines below. This is just a minor comment placement issue.

What Looks Good

1. Complete full-stack implementation: The feature properly flows through grammar → AST → visitor → executor → backend → describer → LSP
2. Thorough test coverage: Includes unit tests for builder, describer, roundtrip scenarios, and specific bug fixes
3. Backward compatibility: Existing scripts remain unchanged as missing anchor sides fall back to builder defaults
4. Bug fixes: The implementation uncovered and fixed three pre-existing bugs in the describer:
   • Expression string literal double-escaping
   • Missing begin in loop describe output
   • Raw control characters in @annotation/@caption
5. Roundtrip fidelity: E2E verification shows byte-exact anchor preservation across describe → exec → describe cycles
6. Proper escaping: New quoteExpressionLiteral function correctly handles Mendix expression strings while preserving regex escape sequences
7. Split statement support: Properly handles the complex case of IF statements with per-branch anchors in both simple and guard-pattern forms
8. Clear documentation: PR includes comprehensive examples and motivation
9. LSP integration: Keywords properly added to completion suggestions

Recommendation

Approve. The PR successfully implements the @anchor annotation feature with full pipeline integration, comprehensive testing, and necessary bug fixes. All checklist items are satisfied with only minor nitpicks that don't block approval.

---

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

[hjotha]

Follow-up commit 3bb8126 addresses both minor notes from the AI review:

• IteratorAnchor / BodyTailAnchor — the grammar now accepts @anchor(iterator: (...), tail: (...)) on LOOP/WHILE statements and the visitor populates both fields. The builder intentionally does NOT translate them into SequenceFlows: Studio Pro rejects loop→body and body→loop edges with CE0709 ("Sequence flow is not accepted by origin or destination") because the iterator icon is drawn implicitly from the LoopedActivity geometry. Keeping the grammar slot reserved means scripts stay forward-compatible if a future Mendix version ever starts supporting these edges. The describer gains a emitLoopAnchorAnnotation helper that will render the combined form the moment such flows appear, but is a no-op on current projects.
• Misplaced @anchor comment — relocated to sit directly above the actual emitAnchorAnnotation call in emitObjectAnnotations.

Verified against a fresh Mendix 11.9 project: mxcli exec for the updated bug-test script followed by mxcli docker check reports 0 errors, and describe → exec → describe round-trips cleanly.

[ako]

Code Review — feat: @anchor annotation for microflow sequence flow endpoints

Overview

Solid, well-motivated feature. The @anchor(from: X, to: Y) annotation closes a real gap: Studio Pro records which side of each activity box a SequenceFlow attaches to, but the describer was silently discarding that information, causing diagram re-layout on every describe → exec roundtrip. The three ancillary bug fixes (double-escape, missing begin, raw control chars in @annotation) are legitimate pre-existing issues that the roundtrip requirement exposed, and bundling them is appropriate.

E2E coverage (5 real-world microflows, 0 anchor drifts, mxcli check passing) is the right validation approach for this kind of change.

---

Issues

1. Package-level global state for flow maps (design concern)

var (
    currentFlowsByOrigin map[model.ID][]*microflows.SequenceFlow
    currentFlowsByDest   map[model.ID][]*microflows.SequenceFlow
)

The comment says "Not ideal, but keeps the blast radius confined." This is honest, but the two globals are a data race hazard: captureDescribeParallel in cmd_catalog.go spawns concurrent describe goroutines that all go through formatMicroflowActivities, which installs these maps. Two concurrent describe calls will clobber each other's maps mid-traversal. Consider passing the maps as fields on the ExecContext's Executor (they're per-traversal state, not per-session), or threading them directly into traverseFlow and emitObjectAnnotations. The current approach is safe for the current calling pattern but is a footgun if that ever changes.

2. Duplicate assertion in TestBuilder_AnchorInsideElseBranch

// Lines 368–375 — identical predicate, different error messages
if !hasFlow(oc.Flows, AnchorBottom, AnchorTop) {
    t.Errorf("expected split→log flow with Bottom→Top...")
}
if !hasFlow(oc.Flows, AnchorBottom, AnchorTop) {
    t.Errorf("expected log→return flow with Bottom→Top inside else branch...")
}

Both checks test the same condition. The second check would need (AnchorBottom, AnchorTop) to appear at least twice — but hasFlow returns true on the first match, so the second assertion passes trivially once one such flow exists. The test doesn't actually verify that two distinct Bottom→Top flows were created. This appears to be an oversight from the development process.

3. Dead code in TestBuilder_IfBranchAnchorOverrides

if ec, ok := f.CaseValue.(ast.Expression); ok {
    _ = ec
}
switch cv := f.CaseValue.(type) {
case nil:
    // skip
default:
    _ = cv
}

These twelve lines do nothing and should be removed. The actual verification proceeds via the GetValue() string interface check below them.

4. LSP completion labels are misleading

{Label: "TOP",    Kind: ..., Detail: "Query keyword"},
{Label: "BOTTOM", Kind: ..., Detail: "Query keyword"},
{Label: "ANCHOR", Kind: ..., Detail: "Query keyword"},

TOP/BOTTOM/ANCHOR are anchor-side / annotation keywords, not SQL query keywords. A completion detail of "Annotation keyword" or "Flow annotation" would be accurate and less confusing to users who encounter them in completion lists.

5. quoteExpressionLiteral edge case: \ followed by an unrecognised character

In the inner default path of the backslash handler:

b.WriteByte('\\')
continue

The continue re-enters the for i++ loop, so s[i+1] is processed on the next iteration. This means \d outputs \d — which is correct for regex roundtrip. But the function comment says "any other backslash-prefixed sequence is passed through unchanged." The implementation actually passes \ through and then emits the following character as plain, which is the same thing but the comment could be clearer. Worth a note: this is not a bug.

---

Minor observations

• emitAnchorAnnotation picks outgoing[0] / incoming[0] for non-split activities. For well-formed microflows each non-split activity has exactly one incoming and one outgoing flow (or zero at boundaries), so this is correct. Worth a comment that this is a single-flow assumption.
• emitLoopAnchorAnnotation builds innerIDs from loop.ObjectCollection.Objects but ObjectCollection can be nil — the nil guard is there (if loop.ObjectCollection != nil), so this is fine.
• AnchorSideUnset = -1 as a sentinel for "use builder default" is clean and avoids nil pointer overhead. The values 0–3 matching OriginConnectionIndex/DestinationConnectionIndex directly in the BSON is a nice property.
• Grammar slot reservation for @anchor(iterator: ..., tail: ...) on LOOP/WHILE with deliberate no-op in the builder is well-documented and forward-safe.

---

Summary

The core implementation is correct and well-tested. Two actionable items before merging:

1. The package-level flow maps are a data race risk with concurrent describe (via captureDescribeParallel). If concurrent calls are possible in the current call graph, this should be fixed now; if they're currently sequential, add a comment and track it as a known risk.
2. The duplicate assertion in TestBuilder_AnchorInsideElseBranch should be fixed — it leaves a gap in test coverage for the exact scenario the test is named after.

The dead test code and LSP label mismatch are minor, easy to fix before merge.

[hjotha]

@ako thanks for the thorough review. Pushed 0279dde addressing every item:

1. Package-level flow-map globals (data race hazard) — removed. flowsByOrigin and flowsByDest are now explicit parameters threaded through traverseFlow, traverseFlowUntilMerge, traverseLoopBody, emitLoopBody, emitActivityStatement, and emitObjectAnnotations. The two describer entry points in cmd_microflows_show.go no longer install/restore package state. The legacy Executor.traverseFlow wrapper passes nil for flowsByDest so @anchor emission stays suppressed for pre-existing test callers. Added TestFormatMicroflowActivities_Concurrent_NoRace: 24 goroutines describe two distinct microflows in parallel and each worker's output is verified for per-flow anchor correctness. Passes under go test -race ./mdl/executor/.

2. Duplicate assertion in TestBuilder_AnchorInsideElseBranch — fixed. The second hasFlow check was identical and would pass trivially; replaced with a new countFlows helper that asserts exactly two distinct Bottom→Top flows exist (split→log and log→return), which is what the test was actually trying to prove.

3. Dead code in TestBuilder_IfBranchAnchorOverrides — removed. The 12-line no-op type-assert block fell through to a GetValue() interface check that never matches (neither EnumerationCase nor the other variants expose that method), so the whole assertion path was relying on a fallback. Replaced the inspection with the describer's own findBranchFlows helper, which matches every CaseValue variant uniformly.

4. LSP completion labels — fixed. Moved TOP/BOTTOM/ANCHOR out of the OQL / QUERY KEYWORDS lexer section into a new ANCHOR ANNOTATION KEYWORDS section, and taught cmd/gen-completions/main.go to map it to Flow annotation keyword. The regenerated cmd/mxcli/lsp_completions_gen.go now reads Detail: "Flow annotation keyword" for all three.

5. quoteExpressionLiteral doc comment — clarified. Reworded the unrecognised-backslash-follower note: the backslash is emitted as-is and the follower is handled by the next loop iteration via the default arm — byte-identical to passthrough but walked separately.

Also picked up the codex review:

• Added three parser/visitor regression tests in mdl/visitor/visitor_anchor_test.go that feed real MDL text with @anchor(iterator: ..., tail: ...) through Build() and assert IteratorAnchor / BodyTailAnchor are populated (loop-both, while-both, loop-iterator-only).
• Restructured the bug-test file into two clearly labeled sections: Section A (roundtrip-preserving — flat @anchor + IF split) and Section B (parse-only forward-compatibility — LOOP/WHILE iterator/tail). The usage text at the top describes each section's guarantee explicitly.

Validation: go test ./... passes, go test -race ./mdl/executor/ passes (including the new concurrent test), go build -tags integration ./... builds clean, Go lint passes, fresh Mendix 11.9 project + mxcli exec of the updated bug-test + mxcli docker check reports 0 errors, and Section A describe → exec → describe is bit-exact.