Fix AST bugs and enhance Action Flow control-node rendering

ROADMAP.md

@@ -80,12 +80,38 @@ implemented first).
 and renderers; possibly `LayoutLabel`/compartment tweaks.
 **Visual gate:** a documented requirement/part renders its note and full compartments.
 
-### View dynamics refinements
-
-- **Sequence View:** populate `LayoutActivation` execution bars; combined-fragment boxes
-  (alt/opt/loop); async/reply message styling.
-- **Action Flow View:** **fork/join** thick bars, **decision/merge** diamonds, accept/send
-  action shapes; optional **swim-lanes** via `LayoutBand`; item-flow edge annotations.
+### Action Flow View: control-node/successor AST correctness + fork/join/decision/merge shapes — delivered
+
+**Delivered.** `AstBuilder.VisitActionBodyItem` now synthesizes a `SysmlTransitionNode` for both
+combined-shape `actionBodyItem` alternatives — `initialNodeMember (actionTargetSuccessionMember)*`
+and `(sourceSuccessionMember)? actionBehaviorMember (actionTargetSuccessionMember)*` — reusing the
+`MultiNodeCapture` sentinel introduced for the State Transition View fix, so the compact `action
+a1; then a2;` idiom and multiple attached successions now resolve correctly (previously both the
+node and its successor were silently dropped). `VisitMergeNode`/`VisitDecisionNode`/
+`VisitJoinNode`/`VisitForkNode`/`VisitAcceptNode`/`VisitSendNode` build minimal `SysmlFeatureNode`s
+with `FeatureKeyword` values `"merge"`/`"decide"`/`"join"`/`"fork"`/`"accept"`/`"send"`, giving
+anonymous control nodes (the dominant idiom in the OMG training corpus) a synthesized `$<keyword>
+<n>` internal name so their successions still wire correctly.
+`ActionFlowViewLayoutStrategy` now renders fork/join as a `LayoutBadge(BadgeShape.HorizontalBar)`
+and decision/merge as a `LayoutBadge(BadgeShape.Diamond)`; accept/send keep the rounded-rectangle
+box shape (no pentagon primitive exists in the referenced `DemaConsulting.Rendering` package) but
+now show their own `Keyword` instead of the hard-coded `"action"` text. Ordinary-action rendering
+is unchanged. The CI/CD Pipeline gallery model (`docs/gallery/models/04-pipeline-action-flow.sysml`)
+was extended with a named `fork`/`join` pair and the compact succession idiom, and regenerated.
+
+**Explicitly deferred (separate future items, not delivered here):** a true pentagon accept/send
+shape (needs a `DemaConsulting.Rendering` package change); swim-lanes via `LayoutBand`; item-flow
+edge annotations (no item-flow/payload capture exists in the AST yet); `assignmentNode`/
+`terminateNode`/`ifNode`/`whileLoopNode`/`forLoopNode` AST support (still entirely unmodeled); an
+`actionUsage`-level (rather than `action def`-level) nested `actionBody` — `VisitActionUsage`
+still doesn't collect nested action-body children, so control nodes/successions nested inside a
+`action x : T { ... }` *usage* (as opposed to an `action def X { ... }` body) remain invisible;
+the Sequence View dynamics item below (different subsystem, separate branch).
+
+### Sequence View dynamics
+
+- Populate `LayoutActivation` execution bars; combined-fragment boxes (alt/opt/loop); async/reply
+  message styling.
 - **Sequence dynamic-view compatibility check (known limitation, carried over from "Dynamic
   (ad-hoc) views", done):** `DynamicViewSynthesizer`'s `--view-type sequence` pre-check accepts
   any target with at least one nested `message` usage (the cheap, necessary-but-not-sufficient
@@ -96,10 +122,9 @@ and renderers; possibly `LayoutLabel`/compartment tweaks.
   `DynamicViewSynthesizer` or surfacing `SequenceViewLayoutStrategy`'s own lifeline-resolution
   result back to the synthesizer.
 
-**Scope:** `SequenceViewLayoutStrategy`, `ActionFlowViewLayoutStrategy`, renderer shape
-primitives (bar, diamond, pentagon, note). `LayoutActivation`/`LayoutBand` already defined.
-**Visual gate:** sequence shows activation bars + a fragment; action flow shows a fork/join and
-a decision/merge with correct shapes.
+**Scope:** `SequenceViewLayoutStrategy`, renderer shape primitives (note). `LayoutActivation`
+already defined.
+**Visual gate:** sequence shows activation bars + a fragment.
 
 ### State Transition View: attached-transition states, entry/exit actions, inherited pseudostate features
 

docs/design/sysml2-tools-core/layout/internal/action-flow-view-layout-strategy.md

@@ -10,9 +10,11 @@ that have no successor, and successions drawn as dashed downward flow arrows.
 ##### Data Model
 
 `ActionFlowViewLayoutStrategy` has no instance state; all input arrives through the `BuildLayout`
-parameters. Layout constants (`MinActionWidth`, `CharWidthFactor`, `MarkerSize`, `MarkerBand`) are
-declared as `private const double` fields. A private `ActionItem` record carries each action with
-its computed box size; successions are carried as `(int From, int To)` index pairs.
+parameters. Layout constants (`MinActionWidth`, `CharWidthFactor`, `MarkerSize`, `MarkerBand`,
+`ControlNodeBadgeSize`) are declared as `private const double` fields. A private `ActionItem`
+record carries each action with its computed box size and a `Kind` field (`"action"` by default,
+or `"merge"`/`"decide"`/`"join"`/`"fork"`/`"accept"`/`"send"` for control nodes); successions are
+carried as `(int From, int To)` index pairs.
 
 ##### Key Methods
 
@@ -39,7 +41,28 @@ by this change — it is applied regardless of specificity. `CollectActions` gat
 qualified name fails `ExposeScopeResolver.IsInSubjectScope`; it then adds any additional action
 named only by a succession endpoint **unconditionally** (this second pass has no independent
 qualified name of its own to scope against, since it exists solely because a succession names it),
-building a name → index lookup.
+building a name → index lookup. Both the primary loop and `FindRoot`'s succession/action scoring
+recognize a 7-way `IsActionFlowKeyword` allow-list (`"action"`, `"merge"`, `"decide"`, `"join"`,
+`"fork"`, `"accept"`, `"send"`) rather than only `"action"`, so control-node-heavy action defs
+(e.g. one dominated by fork/join/decide/merge nodes) are still favored as root candidates; each
+collected item records its real `Kind` alongside its name.
+
+###### `ComputeActionSize(name, kind, theme)` and `MakeActionNode(item, rect)`
+
+`ComputeActionSize` sizes badge-kind items (`IsBadgeKind`: `"merge"`, `"decide"`, `"join"`,
+`"fork"`) to a fixed `ControlNodeBadgeSize` square, independent of the name's length; box-kind
+items (`"action"`, `"accept"`, `"send"`) keep the existing name-length-driven
+`MinActionWidth`/`CharWidthFactor` sizing. `MakeActionNode` is a router that replaces the former
+`MakeActionBox`: `"merge"`/`"decide"` map their placed rectangle's centre to a
+`LayoutBadge(CentreX, CentreY, Size, BadgeShape.Diamond, Label)`; `"fork"`/`"join"` map to a
+`LayoutBadge(..., BadgeShape.HorizontalBar, Label)`; `"accept"`/`"send"` and ordinary `"action"`
+items map to an unchanged `LayoutBox(BoxShape.RoundedRectangle, ...)`, except that `"accept"`/
+`"send"` set `Keyword` to their own kind instead of the hard-coded `"action"` text so the
+distinct node kind remains visible. For every kind, a badge or box `Label` derived from an
+AstBuilder-synthesized `$`-prefixed internal name (anonymous control nodes) is blanked to
+`string.Empty`, since synthetic internal identifiers are never meant to be user-visible. The
+ordinary-`"action"` rendering path is unchanged byte-for-byte from before this routing was
+introduced, since many existing gallery/test fixtures depend on today's plain-box behavior.
 
 ###### `ResolveSuccessions(root, index)`
 

docs/design/sysml2-tools-language/semantic/model/ast-builder.md

@@ -33,6 +33,13 @@ stack with `::` to form the fully-qualified name.
 | `VisitEntryActionMember` | `EntryActionMemberContext` | `SysmlFeatureNode` (`FeatureKeyword = "entry"`) |
 | `VisitDoActionMember` | `DoActionMemberContext` | `SysmlFeatureNode` (`FeatureKeyword = "do"`) |
 | `VisitExitActionMember` | `ExitActionMemberContext` | `SysmlFeatureNode` (`FeatureKeyword = "exit"`) |
+| `VisitActionBodyItem` | `ActionBodyItemContext` | `SysmlNode` or `MultiNodeCapture` (node + successions) |
+| `VisitMergeNode` | `MergeNodeContext` | `SysmlFeatureNode` (`FeatureKeyword = "merge"`) |
+| `VisitDecisionNode` | `DecisionNodeContext` | `SysmlFeatureNode` (`FeatureKeyword = "decide"`) |
+| `VisitJoinNode` | `JoinNodeContext` | `SysmlFeatureNode` (`FeatureKeyword = "join"`) |
+| `VisitForkNode` | `ForkNodeContext` | `SysmlFeatureNode` (`FeatureKeyword = "fork"`) |
+| `VisitAcceptNode` | `AcceptNodeContext` | `SysmlFeatureNode` (`FeatureKeyword = "accept"`) |
+| `VisitSendNode` | `SendNodeContext` | `SysmlFeatureNode` (`FeatureKeyword = "send"`) |
 
 `GetDeclaredName(IdentificationContext)` handles the three grammar alternatives:
 
@@ -304,6 +311,100 @@ the resulting feature node still registers as an (unnamed, unregistered) AST chi
 information is lost, but it is not itself a resolvable symbol. This scope boundary is intentional
 and matches the ROADMAP's framing of entry/do/exit action support as "minimal, non-behavioral."
 
+**Attached-succession action bodies and control-node features.** The `actionBodyItem` grammar
+rule has an analogous combined-shape problem to `stateBodyItem`: two of its alternatives attach
+a succession directly onto the immediately preceding node within the same alternative rather than
+exposing it as a separate `successionAsUsage` —
+`initialNodeMember (actionTargetSuccessionMember)*` (e.g. `first start; then off;`) and
+`(sourceSuccessionMember)? actionBehaviorMember (actionTargetSuccessionMember)*` (e.g. the compact
+`action a1; then a2;` idiom). Before `VisitActionBodyItem` existed, `AstBuilder` only ever
+visited the leading node of each alternative and silently dropped every attached succession, so
+the compact idiom resolved both action nodes but produced no succession edge linking them.
+
+`VisitActionBodyItem` dispatches all four alternatives explicitly:
+
+- `nonBehaviorBodyItem`, `guardedSuccessionMember` — passed straight through to `Visit(...)` (no
+  attached-succession shape applies).
+- `initialNodeMember (actionTargetSuccessionMember)*` — when one or more
+  `actionTargetSuccessionMember`s are attached, synthesizes a `SysmlTransitionNode` per entry
+  (via `BuildActionTargetSuccession`) sourced from the `qualifiedName` referenced by the
+  `initialNodeMember`; the bare `first start;` form (no attached succession) remains a no-op,
+  unchanged from today, since `ActionFlowViewLayoutStrategy` infers start/done markers from
+  succession topology rather than a declarative initial-marker concept.
+- `(sourceSuccessionMember)? actionBehaviorMember (actionTargetSuccessionMember)*` — visits
+  `actionBehaviorMember` (which delegates to the existing `actionNodeMember`/`behaviorUsageMember`
+  handling) to obtain the main node, then calls `BuildActionTargetSuccession` once per
+  `actionTargetSuccessionMember` entry, each producing a `SysmlTransitionNode` whose `Source` is
+  the main node's `Name`. When the optional leading `sourceSuccessionMember` is present (a bare
+  `then` immediately before the node, e.g. `action a; then fork f; ...` — the dominant real-world
+  idiom for wiring a control node into a flow) an additional *incoming* `SysmlTransitionNode` is
+  prepended, whose `Source` is `_actionBodyPreviousNodeName` and whose `Target` is the main node's
+  `Name`. The grammar's leading marker (`sourceSuccessionMember: THEN sourceSuccession`, where
+  `sourceSuccession`/`sourceEndMember`/`sourceEnd` carry no name token at all) is a pure marker —
+  its meaning is "this node's incoming edge comes from whatever immediately preceded it in the
+  same enclosing action body" — so its `Source` identity cannot be read off the grammar node
+  itself. It is instead resolved from `_actionBodyPreviousNodeName`, an `AstBuilder` instance
+  field maintained by `CollectActionBodyChildren` (a body-specific counterpart to the generic
+  `CollectChildren`, used only for action bodies) as it iterates an action body's
+  `actionBodyItem`s in source order, updating the tracked position after each item via
+  `DetermineFlowPositionName` (which resolves to the last synthesized transition's `Target` when
+  the item produced trailing successions, or the visited node's own `Name` otherwise). When no
+  previous position is known (e.g. the item is the first thing in the body), no incoming edge is
+  synthesized rather than fabricating a `Source` from nothing — this matches the safe,
+  no-op-by-default behavior applied elsewhere in this visitor. `CollectChildren` itself remains
+  untouched and continues to serve the state-body call sites, since only action bodies need this
+  order-sensitive bookkeeping.
+
+`BuildActionTargetSuccession` handles all three `actionTargetSuccession` grammar forms:
+unguarded `targetSuccession` (`sourceEndMember THEN connectorEndMember`), guarded
+`guardedTargetSuccession` (`if guardExpressionMember then connectorEndMember`, capturing the
+guard's expression text), and `defaultTargetSuccession` (`else then connectorEndMember`, which the
+grammar provides no guard expression for).
+
+When one or more attached successions (incoming, trailing, or both) are produced,
+`VisitActionBodyItem` returns the same `MultiNodeCapture` sentinel used by
+`VisitStateBodyItem`, wrapping the incoming transition (if any), the node, and its trailing
+succession(s) in that order; when none are produced, it returns the visited node/pass-through
+result directly.
+
+**Known follow-up gaps (out of scope for this fix).** Two structurally analogous
+leading-marker cases were discovered while fixing the above but deliberately left unfixed to keep
+this change surgical:
+
+- `nonBehaviorBodyItem`'s `(sourceSuccessionMember)? structureUsageMember` shape has the same
+  "leading `then` implies an implicit incoming edge" grammar structure, but there is no
+  `VisitNonBehaviorBodyItem`/`VisitStructureUsageMember` override at all today — it relies
+  entirely on ANTLR's default `VisitChildren` aggregation, which drops every child but the last.
+  This is a broader, pre-existing gap (no `MultiNodeCapture` handling exists there yet at all),
+  not merely a missing `sourceSuccessionMember` read.
+- `VisitStateBodyItem`'s `(sourceSuccessionMember)? behaviorUsageMember
+  (targetTransitionUsageMember)*` shape (State Transition View) has the identical unread-marker
+  problem this fix addresses for action bodies, and was not established as a working precedent to
+  copy — it has the same gap, latent and undetected because no current test exercises a leading
+  `then` before a `behaviorUsageMember` inside a state body.
+
+`VisitMergeNode`, `VisitDecisionNode`, `VisitJoinNode`, `VisitForkNode`, `VisitAcceptNode`, and
+`VisitSendNode` each delegate to a shared `BuildActionNodeFeature(usage, keyword)` helper that
+builds a **minimal** `SysmlFeatureNode` — `FeatureKeyword` set to
+`"merge"`/`"decide"`/`"join"`/`"fork"`/`"accept"`/`"send"` respectively, `Children` always empty.
+Unlike ordinary anonymous actions (which are left nameless), an anonymous control node is given a
+synthesized internal name of the form `$<keyword><n>` (via a monotonically increasing
+`_anonymousNodeCounter` field) rather than `null`. This is a deliberate deviation from the
+State Transition View precedent: anonymous fork/decide/send is the *dominant* real-world idiom in
+the OMG training corpus (e.g. `then fork;` immediately followed by several `then` successions),
+so leaving these nodes nameless would make it impossible to wire their successions or render a
+distinct badge for them. The synthetic name is never registered in the symbol table
+(`QualifiedName` stays `null`, so `$`-prefixed names never resolve and surface only as cosmetic
+"unresolved reference" warnings) and is blanked from rendered labels by
+`ActionFlowViewLayoutStrategy`; it exists purely as an internal succession-wiring mechanism. When
+the control node instead has an *explicitly declared* name (e.g. `fork buildFork;`), it is treated
+like any other named feature: `QualifiedName` is populated via the same `QualifyName` helper used
+by `BuildStateActionFeatureNode`, so it is registered in the symbol table and correctly subject to
+expose-scope filtering (`ExposeScopeResolver.IsInSubjectScope`) in
+`ActionFlowViewLayoutStrategy.CollectActions`.
+`assignmentNode`, `terminateNode`, `ifNode`, `whileLoopNode`, and `forLoopNode` remain
+intentionally unhandled — a pre-existing gap, not introduced by this change.
+
 ##### Error Handling
 
 Anonymous elements (null declared names) are silently skipped — visitor methods return `null`

docs/gallery/README.md

@@ -113,8 +113,12 @@ SVG: [`svg/ElevatorStateTransitionView.svg`](svg/ElevatorStateTransitionView.svg
 
 ## 4. Action Flow View — CI/CD Pipeline
 
-Shows actions arranged top-to-bottom by the layered layout pipeline,
-with a start node, a done node, and a quality-gate branch and join.
+Shows actions arranged top-to-bottom by the layered layout pipeline, with a start node, a done
+node, and a quality-gate branch and join. The `build` step fans out through a named `fork`
+(rendered as a horizontal-bar badge) into parallel `unitTest`/`securityScan` branches that rejoin
+through a named `join` (also a horizontal-bar badge) before `qualityGate`. The `checkout` step
+uses the compact `action checkout; then restoreDependencies;` idiom instead of a standalone
+`first ... then ...;` succession statement.
 
 Model: [`models/04-pipeline-action-flow.sysml`](models/04-pipeline-action-flow.sysml) ·
 SVG: [`svg/PipelineActionFlowView.svg`](svg/PipelineActionFlowView.svg)

docs/gallery/models/04-pipeline-action-flow.sysml

@@ -3,11 +3,29 @@ package CiCdPipeline {
     // A continuous-integration / deployment action flow with a quality gate branch.
     action def Pipeline {
         action checkout;
+        then restoreDependencies;
+
         action restoreDependencies;
         action build;
+        first restoreDependencies then build;
+        first build then buildFork;
+
+        fork buildFork;
+        then unitTest;
+        then securityScan;
+
         action unitTest;
+        then integrationTest;
+
         action integrationTest;
+        then testJoin;
+
         action securityScan;
+        then testJoin;
+
+        join testJoin;
+        then qualityGate;
+
         action qualityGate;
         action deployStaging;
         action smokeTest;
@@ -16,13 +34,6 @@ package CiCdPipeline {
         action notifySuccess;
         action notifyFailure;
 
-        first checkout then restoreDependencies;
-        first restoreDependencies then build;
-        first build then unitTest;
-        first build then securityScan;
-        first unitTest then integrationTest;
-        first integrationTest then qualityGate;
-        first securityScan then qualityGate;
         first qualityGate then deployStaging;
         first deployStaging then smokeTest;
         first smokeTest then deployProduction;