fixup: PR #276 review feedback (ako + codex)

Addresses the substantive and minor notes from both reviews on PR #276.

ako review:

- **[substantive] Remove package-level currentFlowsByOrigin/currentFlowsByDest
  globals**. These were a data race under captureDescribeParallel, which
  spawns concurrent describe goroutines. flowsByOrigin/flowsByDest are now
  explicit parameters on traverseFlow / traverseFlowUntilMerge /
  traverseLoopBody / emitLoopBody / emitActivityStatement /
  emitObjectAnnotations. The legacy Executor.traverseFlow wrapper passes
  nil for flowsByDest (preserves prior @anchor-suppressed behaviour for
  pre-existing callers and tests). Added TestFormatMicroflowActivities_
  Concurrent_NoRace: 24 goroutines run formatMicroflowActivities in
  parallel on two distinct microflows and each worker's output is
  checked for correct per-flow @anchor content. Passes under -race.

- **Duplicate assertion in TestBuilder_AnchorInsideElseBranch**. The
  second hasFlow(AnchorBottom, AnchorTop) duplicated the first and
  trivially passed once any match existed. Replaced with countFlows()
  to assert two distinct Bottom→Top flows (split→log and log→return).

- **Dead code in TestBuilder_IfBranchAnchorOverrides**. The 12-line
  no-op type-assert block always fell through to a GetValue-based
  interface assertion that fails (EnumerationCase has no GetValue
  method). Replaced the whole post-build inspection with findBranchFlows
  — the describer's own helper that handles every CaseValue variant.

- **LSP completion labels "Query keyword"** on TOP/BOTTOM/ANCHOR.
  Moved the anchor tokens out of the OQL/QUERY section into a new
  "ANCHOR ANNOTATION KEYWORDS" section in MDLLexer.g4 and taught
  gen-completions to map it to "Flow annotation keyword".

- **quoteExpressionLiteral doc comment clarity**. Reworded the note
  about backslash followers that are not recognised escape letters: the
  backslash is emitted as-is and the follower is processed by the next
  loop iteration via the default arm — byte-identical to passthrough
  but walked separately.

codex review:

- **Parser/visitor regression tests for @anchor(iterator: ..., tail: ...)**.
  visitor_anchor_test.go gains three tests that feed real MDL text
  through Build() and assert IteratorAnchor / BodyTailAnchor fields:
  loop with both iterator+tail, while with both, loop with iterator-only
  (tail stays nil). The existing executor-side tests only used
  synthetic AST values.

- **Clarify bug-test file wording**. The usage note said describe output
  "must include the @anchor(...) lines below verbatim" but the LOOP/WHILE
  section immediately below says those are parse-only. Restructured into
  two labeled sections: Section A = roundtrip-preserving (flat @anchor +
  IF split), Section B = parse-only forward-compatibility (LOOP/WHILE
  iterator/tail). Usage text now 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.
- On a fresh Mendix 11.9 project: mxcli exec of the updated bug-test
  script followed by mxcli docker check reports 0 errors.
- describe → exec → describe is bit-exact for Section A microflows.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: hjothamendix

cmd/gen-completions/main.go

@@ -304,6 +304,8 @@ func normalizeCategoryName(raw string) string {
 		return "Connection keyword"
 	case strings.Contains(raw, "OQL"), strings.Contains(raw, "QUERY"):
 		return "Query keyword"
+	case strings.Contains(raw, "ANCHOR"):
+		return "Flow annotation keyword"
 	case strings.Contains(raw, "MICROFLOW"):
 		return "Microflow keyword"
 	case strings.Contains(raw, "PAGE"), strings.Contains(raw, "WIDGET"):

cmd/mxcli/lsp_completions_gen.go

@@ -21,8 +21,24 @@
 --
 -- Usage:
 --   mxcli exec mdl-examples/bug-tests/anchor-sequence-flow-annotation.mdl -p app.mpr
---   Then: mxcli describe microflow BugTestAnchor.MF_CustomAnchors -p app.mpr
---   The describe output must include the @anchor(...) lines below verbatim.
+--   Then describe each microflow below with `mxcli describe microflow ...`.
+--
+--   The file has two sections with different roundtrip guarantees:
+--
+--     A) Roundtrip-preserving examples (flat @anchor + IF split form) — the
+--        describe output MUST include the @anchor(...) lines verbatim.
+--        Executing describe's output back into the project is bit-exact.
+--
+--     B) Parse-only forward-compatibility examples (LOOP / WHILE iterator
+--        and tail sides) — the grammar accepts @anchor(iterator: ..., tail: ...)
+--        but the builder intentionally does NOT emit those SequenceFlows today
+--        (see the note at the top of section B). Describe therefore will NOT
+--        echo iterator/tail back; the annotation is accepted so existing
+--        scripts keep parsing if Mendix ever starts recording those edges.
+-- ============================================================================
+
+-- ============================================================================
+-- Section A — roundtrip-preserving examples
 -- ============================================================================
 
 create module BugTestAnchor;
@@ -60,16 +76,18 @@ end;
 /
 
 -- ============================================================================
--- LOOP / WHILE body anchors
+-- Section B — parse-only forward-compatibility examples (LOOP / WHILE)
 -- ============================================================================
 -- The grammar accepts @anchor(iterator: (...), tail: (...)) on LOOP and
 -- WHILE statements so authoring tools can forward-propagate the intent, but
 -- the builder deliberately does NOT translate them into SequenceFlows.
 -- Mendix rejects edges between a LoopedActivity and its body statements with
 -- CE0709 "Sequence flow is not accepted by origin or destination" — the
 -- iterator icon is drawn implicitly from the LoopedActivity geometry.
--- Keeping the grammar slot reserved means existing scripts continue to parse
--- if a future Mendix version starts supporting these edges.
+--
+-- NOTE: describe will not echo `iterator: (...)` / `tail: (...)` in the
+-- output for the two microflows below. That is expected — see the usage
+-- text above.
 
 create entity BugTestAnchor.Item (
   Label: String(100)

mdl-examples/bug-tests/anchor-sequence-flow-annotation.mdl

@@ -53,6 +53,17 @@ func hasFlow(flows []anchorFlow, origin, dest int) bool {
 	return false
 }
 
+// countFlows counts how many flows have the given anchor pair.
+func countFlows(flows []anchorFlow, origin, dest int) int {
+	n := 0
+	for _, f := range flows {
+		if f.OriginIdx == origin && f.DestIdx == dest {
+			n++
+		}
+	}
+	return n
+}
+
 func TestBuilder_AnchorInsideElseBranch(t *testing.T) {
 	// Reproduces the pattern from attempt #35:
 	//   if cond then { set ... }
@@ -88,14 +99,13 @@ func TestBuilder_AnchorInsideElseBranch(t *testing.T) {
 
 	oc := buildWithAnchors(body)
 
-	// The else-first-statement flow must land on the log's top anchor.
-	if !hasFlow(oc.Flows, AnchorBottom, AnchorTop) {
-		t.Errorf("expected split→log flow with Bottom→Top (from user's own @anchor), got %+v", oc.Flows)
-	}
-	// The log→return flow inside the else branch must emit from=bottom
-	// (previous statement's From) and to=top (return's own To).
-	if !hasFlow(oc.Flows, AnchorBottom, AnchorTop) {
-		t.Errorf("expected log→return flow with Bottom→Top inside else branch, got %+v", oc.Flows)
+	// Two distinct Bottom→Top flows must exist:
+	//   1. split → log   (from the user's @anchor on the log statement)
+	//   2. log   → return (propagating the log's From=Bottom and the return's To=Top)
+	// A single hasFlow check would pass with just one match, so count explicitly
+	// to pin the regression — see ako review note on TestBuilder_AnchorInsideElseBranch.
+	if got := countFlows(oc.Flows, AnchorBottom, AnchorTop); got != 2 {
+		t.Errorf("expected 2 Bottom→Top flows (split→log and log→return), got %d: %+v", got, oc.Flows)
 	}
 }
 

mdl/executor/cmd_microflows_anchor_if_test.go

@@ -134,57 +134,22 @@ func TestBuilder_IfBranchAnchorOverrides(t *testing.T) {
 	fb := &flowBuilder{posX: 100, posY: 100, spacing: HorizontalSpacing}
 	oc := fb.buildFlowGraph(body, nil)
 
-	// Scan for the split→activity "true" and "false" flows and check their
-	// OriginConnectionIndex matches the per-branch anchors.
-	var trueFlow, falseFlow bool
-	for _, f := range oc.Flows {
-		if ec, ok := f.CaseValue.(ast.Expression); ok {
-			_ = ec
-		}
-		switch cv := f.CaseValue.(type) {
-		case nil:
-			// skip
-		default:
-			_ = cv
-		}
-		// The split's outgoing flows carry an EnumerationCase with "true" / "false".
-		if cv, ok := f.CaseValue.(interface{ GetValue() string }); ok {
-			switch cv.GetValue() {
-			case "true":
-				if f.OriginConnectionIndex == AnchorTop && f.DestinationConnectionIndex == AnchorLeft {
-					trueFlow = true
-				}
-			case "false":
-				if f.OriginConnectionIndex == AnchorBottom && f.DestinationConnectionIndex == AnchorTop {
-					falseFlow = true
-				}
-			}
-		}
-	}
-
-	// Fallback if the CaseValue interface doesn't expose GetValue — just check
-	// that AT LEAST one flow with Top origin and one with Bottom origin exist.
-	if !trueFlow {
-		for _, f := range oc.Flows {
-			if f.OriginConnectionIndex == AnchorTop && f.DestinationConnectionIndex == AnchorLeft {
-				trueFlow = true
-				break
-			}
-		}
-	}
-	if !falseFlow {
-		for _, f := range oc.Flows {
-			if f.OriginConnectionIndex == AnchorBottom && f.DestinationConnectionIndex == AnchorTop {
-				falseFlow = true
-				break
-			}
-		}
-	}
-
-	if !trueFlow {
-		t.Error("expected a split outgoing flow with origin=Top, destination=Left (true branch anchor)")
-	}
-	if !falseFlow {
-		t.Error("expected a split outgoing flow with origin=Bottom, destination=Top (false branch anchor)")
+	// Identify the TRUE/FALSE split outgoing flows via the describer's own
+	// helper so we match every CaseValue variant the builder can emit
+	// (ExpressionCase, EnumerationCase — value and pointer forms, BooleanCase).
+	trueF, falseF := findBranchFlows(oc.Flows)
+	if trueF == nil {
+		t.Fatalf("expected a TRUE split flow, got none among %+v", oc.Flows)
+	}
+	if falseF == nil {
+		t.Fatalf("expected a FALSE split flow, got none among %+v", oc.Flows)
+	}
+	if trueF.OriginConnectionIndex != AnchorTop || trueF.DestinationConnectionIndex != AnchorLeft {
+		t.Errorf("true branch anchors: got from=%d to=%d, want Top(%d)/Left(%d)",
+			trueF.OriginConnectionIndex, trueF.DestinationConnectionIndex, AnchorTop, AnchorLeft)
+	}
+	if falseF.OriginConnectionIndex != AnchorBottom || falseF.DestinationConnectionIndex != AnchorTop {
+		t.Errorf("false branch anchors: got from=%d to=%d, want Bottom(%d)/Top(%d)",
+			falseF.OriginConnectionIndex, falseF.DestinationConnectionIndex, AnchorBottom, AnchorTop)
 	}
 }

mdl/executor/cmd_microflows_builder_anchor_test.go

@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: Apache-2.0
+
+// Tests that concurrent describe calls do not race on the describer's flow-map
+// state. The describer used to install package-level currentFlowsByOrigin /
+// currentFlowsByDest maps for the duration of a describe; two goroutines
+// running formatMicroflowActivities in parallel would clobber each other's
+// maps mid-traversal.
+//
+// This test drives formatMicroflowActivities from many goroutines at once.
+// It is primarily useful under `go test -race`, where the previous global-
+// state implementation reported a data race on currentFlowsByOrigin /
+// currentFlowsByDest. After threading the maps as explicit parameters the
+// race disappears; the test also asserts per-goroutine output integrity.
+package executor
+
+import (
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/mendixlabs/mxcli/model"
+	"github.com/mendixlabs/mxcli/sdk/microflows"
+)
+
+func TestFormatMicroflowActivities_Concurrent_NoRace(t *testing.T) {
+	// Build two distinct microflows whose activities are anchored to
+	// different sides. If the two describe calls share state, one will
+	// emit the other's anchor keyword.
+	mfA := mkRaceMicroflow("mfa-start", "mfa-log", "mfa-end", AnchorRight)
+	mfB := mkRaceMicroflow("mfb-start", "mfb-log", "mfb-end", AnchorBottom)
+
+	e := newTestExecutor()
+	ctx := e.newExecContext(t.Context())
+
+	const workersPerFlow = 12
+
+	var wg sync.WaitGroup
+	resultsA := make([]string, workersPerFlow)
+	resultsB := make([]string, workersPerFlow)
+	for i := 0; i < workersPerFlow; i++ {
+		wg.Add(2)
+		go func(i int) {
+			defer wg.Done()
+			resultsA[i] = strings.Join(formatMicroflowActivities(ctx, mfA, nil, nil), "\n")
+		}(i)
+		go func(i int) {
+			defer wg.Done()
+			resultsB[i] = strings.Join(formatMicroflowActivities(ctx, mfB, nil, nil), "\n")
+		}(i)
+	}
+	wg.Wait()
+
+	wantA := "@anchor(from: right, to: left)"
+	wantB := "@anchor(from: bottom, to: left)"
+	for i, got := range resultsA {
+		if !strings.Contains(got, wantA) {
+			t.Errorf("worker %d (A) missing %q in output:\n%s", i, wantA, got)
+		}
+		if strings.Contains(got, wantB) {
+			t.Errorf("worker %d (A) unexpectedly contains %q from flow B:\n%s", i, wantB, got)
+		}
+	}
+	for i, got := range resultsB {
+		if !strings.Contains(got, wantB) {
+			t.Errorf("worker %d (B) missing %q in output:\n%s", i, wantB, got)
+		}
+		if strings.Contains(got, wantA) {
+			t.Errorf("worker %d (B) unexpectedly contains %q from flow A:\n%s", i, wantA, got)
+		}
+	}
+}
+
+// mkRaceMicroflow builds a minimal microflow (Start → Log → End) whose
+// sequence-flow origin index matches the given anchorSide, so concurrent
+// describe outputs are distinguishable by their @anchor line.
+func mkRaceMicroflow(startID, logID, endID string, originSide int) *microflows.Microflow {
+	start := &microflows.StartEvent{BaseMicroflowObject: mkObj(startID)}
+	log := &microflows.ActionActivity{
+		BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj(logID)},
+		Action: &microflows.LogMessageAction{
+			LogLevel:    "Info",
+			LogNodeName: "'App'",
+			MessageTemplate: &model.Text{
+				Translations: map[string]string{"en_US": "hi"},
+			},
+		},
+	}
+	end := &microflows.EndEvent{BaseMicroflowObject: mkObj(endID)}
+
+	// Flow start→log carries the distinguishing originSide on the LOG side
+	// (where @anchor(from: ...) is emitted).
+	flowSL := &microflows.SequenceFlow{
+		OriginID:                   mkID(startID),
+		DestinationID:              mkID(logID),
+		OriginConnectionIndex:      AnchorRight,
+		DestinationConnectionIndex: AnchorLeft,
+	}
+	flowLE := &microflows.SequenceFlow{
+		OriginID:                   mkID(logID),
+		DestinationID:              mkID(endID),
+		OriginConnectionIndex:      originSide,
+		DestinationConnectionIndex: AnchorLeft,
+	}
+
+	return &microflows.Microflow{
+		Name: "MF_" + logID,
+		ObjectCollection: &microflows.MicroflowObjectCollection{
+			Objects: []microflows.MicroflowObject{start, log, end},
+			Flows:   []*microflows.SequenceFlow{flowSL, flowLE},
+		},
+	}
+}

mdl/executor/cmd_microflows_describe_concurrent_test.go

@@ -176,9 +176,12 @@ func mendixFunctionName(name string) string {
 //   - Backslashes followed by one of the recognised escape letters (n/r/t/\/')
 //     are doubled so the visitor's unquoteString preserves them — without this,
 //     the source literal `\\n` would come back as a real newline on reparse.
-//   - Any other backslash-prefixed sequence (e.g. `\d`, `\w`, `\p{...}` inside
-//     regexes) is passed through unchanged so describe→exec roundtrips of
-//     Mendix regular-expression arguments stay bit-exact.
+//   - For any other backslash-prefixed byte (e.g. `\d`, `\w`, `\p{...}` inside
+//     regexes) the backslash is emitted as-is and the follower is written by the
+//     next loop iteration via the default arm, so the two bytes end up in the
+//     output unchanged. This keeps Mendix regular-expression escape sequences
+//     bit-exact across describe→exec roundtrips; the output is byte-identical
+//     to passthrough even though the implementation walks the bytes separately.
 //
 // This is narrower than mdlQuote (used for @annotation / @caption text where
 // the AST value is a plain string): mdlQuote unconditionally doubles every

mdl/executor/cmd_microflows_helpers.go

@@ -681,11 +681,10 @@ func formatMicroflowActivities(
 	// Build annotation map for @annotation emission
 	annotationsByTarget := buildAnnotationsByTarget(mf.ObjectCollection)
 
-	// Install flow maps for @anchor emission during traversal.
-	restore := setDescriberFlowMaps(flowsByOrigin, flowsByDest)
-	defer restore()
-
-	traverseFlow(ctx, startID, activityMap, flowsByOrigin, splitMergeMap, visited, entityNames, microflowNames, &lines, 0, nil, 0, annotationsByTarget)
+	// flowsByOrigin / flowsByDest are threaded into traverseFlow so @anchor
+	// emission is per-call — no package-level globals, safe under concurrent
+	// describe (e.g. captureDescribeParallel).
+	traverseFlow(ctx, startID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, &lines, 0, nil, 0, annotationsByTarget)
 
 	return lines
 }
@@ -741,11 +740,7 @@ func formatMicroflowActivitiesWithSourceMap(
 	// Build annotation map for @annotation emission
 	annotationsByTarget := buildAnnotationsByTarget(mf.ObjectCollection)
 
-	// Install flow maps for @anchor emission during traversal.
-	restore := setDescriberFlowMaps(flowsByOrigin, flowsByDest)
-	defer restore()
-
-	traverseFlow(ctx, startID, activityMap, flowsByOrigin, splitMergeMap, visited, entityNames, microflowNames, &lines, 0, sourceMap, headerLineCount, annotationsByTarget)
+	traverseFlow(ctx, startID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, &lines, 0, sourceMap, headerLineCount, annotationsByTarget)
 
 	return lines
 }