fix: preserve nested loop body flow traversal

[hjotha]

Part of #332.

Summary

• merge parent-level body flows into loop-local traversal maps
• collect nested loop body object IDs recursively
• add a regression for local loop-boundary flows plus parent-level continuations sharing origins

Closes #328

Validation

• go test ./mdl/executor -run TestTraverseFlow_LoopBodyMergesParentFlowsForExistingOrigin
• make build
• make lint-go
• make test

[github-actions]

AI Code Review

We are given a PR titled "fix: preserve nested loop body flow traversal"

The PR summary says:

• merge parent-level body flows into loop-local traversal maps
• collect nested loop body object IDs recursively
• add a regression for local loop-boundary flows plus parent-level continuations sharing origins

The changes are in two files:

1. mdl/executor/cmd_microflows_show_helpers.go
2. mdl/executor/cmd_microflows_traverse_test.go

We are to review against the provided checklist.

First, note that the PR does not introduce any new MDL syntax (it's a fix to the executor's traversal logic for loops). Therefore, we skip the MDL syntax design section.

Now, let's go through the checklist:

Overlap & duplication

• The PR does not add any new proposals (we are told there is no proposals directory).
• We check for duplicated code: The changes are in the executor helpers and add a test. The changes are about collecting loop object IDs recursively and using that to merge parent flows. This does not duplicate existing functionality but rather fixes a bug in the traversal for nested loops.
• We check doctype test files: The PR adds a new test in mdl/executor/cmd_microflows_traverse_test.go (a regression test). There is no indication that this test duplicates an existing one.
• The PR does not re-document already-shipped features as new.

Full-stack consistency (for MDL features)

Since this is a bug fix and not a new MDL feature, we do not expect changes to the grammar, AST, visitor, etc. The changes are confined to the executor. Therefore, we skip the full-stack consistency check for MDL features.

Test coverage

• The PR adds a new test: TestTraverseFlow_LoopBodyMergesParentFlowsForExistingOrigin in mdl/executor/cmd_microflows_traverse_test.go.
• The test is a regression test for the specific issue.
• The test does not use time.Sleep for synchronization (it uses a direct function call and checks the output).
• The test is in the executor package, which is an integration path (not just a helper).

Security & robustness

• The changes are in the microflow traversal logic. There is no obvious security issue (like command injection, XSS, etc.) because we are dealing with internal data structures.
• No Unix sockets or file I/O in hot paths are introduced.
• No silent side effects on typos: the changes are about correctly identifying loop objects and merging flows.
• Method receivers: The changes are in helper functions, so no issue with receivers.

Scope & atomicity

• The PR is focused on fixing the traversal of nested loops in microflows. It does one thing: fix the bug where parent-level flows were not being merged into nested loop bodies when there was an existing local loop-boundary flow.
• The PR description does not have an "and" between unrelated items.

Code quality

• The changes are refactorings of the loop traversal logic to handle nested loops correctly. They are applied consistently in the two places (origin and destination maps) and in the test.
• There are no manually maintained lists that are flagged as maintenance risks.
• The design (as per the comments in the code) matches the implementation.

Bugs & correctness

• The fix addresses a specific bug: when a nested loop has a local loop-boundary flow (i.e., a flow that stays within the loop body) and there is also a parent-level flow that continues from the same origin, the parent-level flow was not being merged in. The fix collects all object IDs in the loop (including nested loops) and then uses that set to decide whether to merge parent flows.
• The test case is designed to catch this bug and the fix passes it.
• Error handling: The changes do not introduce new error paths that are not handled.

Now, let's look at the changes in detail:

In mdl/executor/cmd_microflows_show_helpers.go:

• We added a call to collectLoopObjectIDs(loop.ObjectCollection) to get a set of all object IDs in the loop (including nested loops).
• Then, when checking if an originID or destID is in the loop, we use loopObjectIDs[originID] (or loopObjectIDs[destID]) instead of checking against loopActivityMap (which only had the immediate objects).
• We also changed the way we add flows: instead of setting the slice to the flows (which would replace any existing ones), we now append each flow if it's missing (using appendSequenceFlowIfMissing). This avoids duplicates and also merges multiple flows from different sources (like parent-level and local) for the same origin/destination.
• We added helper functions: collectLoopObjectIDs, collectLoopObjectIDsInto, appendSequenceFlowIfMissing, sequenceFlowIdentity, and preferLoopBodyStart.

In the test file:

• We added a test that sets up a nested loop scenario where:

  • The outer loop has a body: setup -> fetch -> (then the loop continues to the end)

  • The nested loop (inside the outer loop's body) has a body: nested-log -> nested-tail -> (and then back to the nested loop? Actually, the nested loop has a flow from nested-log to nested, and then from nested-tail to nested? Wait, the test sets up:
    nested loop:
    Objects: [nested-log, nested-tail]
    Flows: [nested-log -> nested] (note: nested is the loop activity itself? Actually, the loop activity is the LoopedActivity, so the flow from nested-log to the loop activity? But note: the loop activity has an ID of "nested".)
    Then, the outer loop has:
    Objects: [nestedLoop, setup, fetch]
    Flows: [setup->fetch, fetch->loop] (so the loop activity is the outer loop, with ID "loop")
    And then the parent flows (from the outer loop's parent, which is the main flow) are:
    start->loop, loop->end, fetch->nested, nested->loop, nested-log->nested-tail, nested-tail->nested.

    The key point:
    - The nested loop has a local flow: nested-log -> nested (which is the loop activity) and nested-tail -> nested.
    - But there is also a parent flow: fetch -> nested (which is the nested loop activity) from the outer loop's body.

    The bug was that when processing the nested loop, the parent flow (fetch->nested) was not being merged into the nested loop's flow set because the nested loop's immediate object collection did not contain the "fetch" activity (it's in the outer loop). The fix collects all object IDs in the nested loop (including the nested loop's own objects and any nested loops inside it, but note: the nested loop in this test does not have a nested loop inside it? Actually, the nested loop in the test is the inner loop, and it has no further nesting). However, the fix collects the object IDs of the nested loop's ObjectCollection, which in the test only has the two log activities. So why does it work?

    Actually, the fix changes the condition from:
    if _, inLoop := loopActivityMap[originID]; inLoop {
    to:
    if loopObjectIDs[originID] {
    where loopObjectIDs is built by recursively collecting all object IDs in the loop's ObjectCollection (including any nested loops inside).

    In the test, the nested loop (inner) does not have any nested loops inside it, so loopObjectIDs for the inner loop only has the two log activities. Therefore, the parent flow (fetch->nested) would still not be included because "fetch" is not in the inner loop's object set.

    Wait, let me re-read the test: the nested loop is the inner loop, and the parent flow we are concerned about is from "fetch" (which is in the outer loop's body) to the inner loop (activity ID "nested"). The inner loop's ObjectCollection does not contain "fetch", so the condition loopObjectIDs[originID] for originID="fetch" would be false.

    But note: the test is named TestTraverseFlow_LoopBodyMergesParentFlowsForExistingOrigin and the comment says:
    // add a regression for local loop-boundary flows plus parent-level continuations sharing origins

    And the test sets up:
    // This local loop-boundary flow gives "fetch" an existing
    // local origin entry. Parent-level body flows with the same
    // origin must still be merged in.

    The local loop-boundary flow in the outer loop is: "fetch" -> "loop" (the outer loop activity). But wait, the outer loop's flow is set up as:
    Fls: []*microflows.SequenceFlow{
    mkFlow("setup", "fetch"),
    mkFlow("fetch", "loop"), // this is the local loop-boundary flow for the outer loop?
    }

    However, the test is about the nested loop. The nested loop is the inner loop. The parent-level flow for the inner loop is the flow from the outer loop's body to the inner loop: that is, the flow from "fetch" to "nested" (the inner loop activity).

    The bug was that when processing the inner loop, we were only looking at the inner loop's immediate ObjectCollection (which does not contain "fetch") to decide whether to include parent flows. So we were missing the parent flow from "fetch" to "nested".

    The fix changes the condition to use the recursively collected object IDs of the loop. But note: the inner loop does not have "fetch" in its ObjectCollection (nor in any nested loop inside it, because there isn't one). So why would the fix work?

    Let me look at the code again:

    We have:
    loopObjectIDs := collectLoopObjectIDs(loop.ObjectCollection)

    And then we use that to check if an originID is in the loop.

    However, the inner loop's ObjectCollection does not contain "fetch", so loopObjectIDs for the inner loop does not have "fetch".

    But wait: the test also sets up a flow in the outer loop:
    mkFlow("fetch", "loop") // meaning from fetch to the outer loop activity (ID "loop")

    And then in the parent flows (which are the flows from the outer loop's parent, i.e., the main flow) we have:
    mkID("fetch"): {mkFlow("fetch", "nested")},

    So the parent flow for the inner loop is from "fetch" to "nested".

    The fix in the code for the inner loop is to check if the originID (which is "fetch") is in the set of object IDs of the inner loop (recursively collected). It is not.

    However, note that the test also has:
    mkID("nested"): {mkFlow("nested", "loop")}, // this is a flow from the nested loop activity to the outer loop activity?
    ... and others.

    And then the test expects to see:
    "log info node 'Synthetic' 'setup';",
    "log info node 'Synthetic' 'fetch';",
    "loop $role in $roles",
    ...

    The key is that the test is actually testing the outer loop? Let me read the test again:

    We have:
    outerLoop := &microflows.LoopedActivity{ ... } // ID "loop"
    nestedLoop := &microflows.LoopedActivity{ ... } // ID "nested"

    And then we build:
    activityMap:
    "start": StartEvent
    "loop": outerLoop
    "end": EndEvent
    flowsByOrigin:
    "start": [start->loop]
    "loop": [loop->end]
    "fetch": [fetch->nested] // note: fetch is an activity in the outer loop's body
    "nested": [nested->loop] // note: nested is the inner loop activity
    "nested-log": [nested-log->nested-tail]
    "nested-tail": [nested-tail->nested]

    The traverseFlow is called from the start activity.

    The flow from start goes to the outer loop (activity "loop"). Then we enter the outer loop's body.

    In the outer loop's body, we have:
    Objects: [nestedLoop, setup, fetch]
    Flows: [setup->fetch, fetch->loop] // note: fetch->loop is a flow that goes back to the outer loop activity (so it's a loop-boundary flow for the outer loop)

    But wait, the outer loop's body also contains the nested loop (as an object). So when we are processing the outer loop's body, we will encounter the nested loop object.

    When we process the nested loop object (which is a LoopedActivity), we then call emitLoopBody on it.

    Inside emitLoopBody for the nested loop (inner loop):
    We build loopActivityMap from the inner loop's ObjectCollection: which has the two log activities.
    Then we compute loopObjectIDs by recursively collecting from the inner loop's ObjectCollection (which only has the two log activities, and they are not LoopedActivity so no further recursion).

     Then we look at flowsByOrigin (which is the parent flows, i.e., the flows from the outer loop's body to the inner loop and within the inner loop? Actually, note: the flowsByOrigin passed to emitLoopBody is the same as the one passed to the outer loop's emitLoopBody? 
    

    Actually, the outer loop's emitLoopBody is called with:
    flowsByOrigin: the one we built in the outer loop's emitLoopBody?

    Let me trace:

    The outer loop's emitLoopBody (for the outer loop) is called from the main traverseFlow when we hit the outer loop activity.

    In the outer loop's emitLoopBody:
    We build loopActivityMap for the outer loop's ObjectCollection: [nestedLoop, setup, fetch]
    Then we compute loopObjectIDs for the outer loop by recursively collecting:
    - nestedLoop: is a LoopedActivity -> we recursively collect its ObjectCollection (the two log activities)
    - setup: not a loop
    - fetch: not a loop
    So loopObjectIDs for the outer loop will include:
    nestedLoop.ID, setup.ID, fetch.ID, and the two log activities from the inner loop.

     Then, when processing parent flows (flowsByOrigin) for the outer loop, we check for each originID if it is in loopObjectIDs (the outer loop's set).
    
     The parent flows for the outer loop (passed in from the main traverseFlow) are:
       start: [start->loop]   -> originID = start -> not in outer loop's ObjectCollection? (start is not in the outer loop's body) -> skip
       loop: [loop->end]      -> originID = loop -> is in the outer loop's ObjectCollection? (the loop activity itself is not in the ObjectCollection of the outer loop's body? Actually, the outer loop's activity is the LoopedActivity, and its ObjectCollection does not contain itself) -> so skip? 
       fetch: [fetch->nested] -> originID = fetch -> is in the outer loop's ObjectCollection? (yes, because fetch is in the outer loop's ObjectCollection) -> so we add this flow to the outer loop's loopFlowsByOrigin for originID=fetch.
       nested: [nested->loop] -> originID = nested -> is in the outer loop's ObjectCollection? (yes, because the nested loop activity is in the outer loop's ObjectCollection) -> so we add this flow.
       nested-log: [nested-log->nested-tail] -> originID = nested-log -> is in the outer loop's ObjectCollection? (yes, because we collected it from the inner loop) -> so we add.
       nested-tail: [nested-tail->nested] -> originID = nested-tail -> is in the outer loop's ObjectCollection? (yes) -> so we add.
    
     Then, when building the flow graph for the outer loop's body, we have:
       loopFlowsByOrigin for fetch: [fetch->nested]   (from parent) and also we have the local flows? 
       The outer loop's own ObjectCollection flows: [setup->fetch, fetch->loop]
    
     So for originID=fetch, we have two flows: 
       - the local one: fetch->loop
       - the parent one: fetch->nested
    
     Then, when we build the flow graph for the outer loop's body, we will have both.
    

    Now, when we traverse the outer loop's body, we start at the activity with zero incoming flow (in the outer loop's body). We compute incomingCount for the outer loop's body.

    The outer loop's body activities: nestedLoop, setup, fetch.

    Incoming flows (from the outer loop's body flow graph, which includes both local and parent flows that we merged):
    setup:
    from?
    local: none?
    parent: none?
    Actually, we have:
    setup->fetch (local) -> so fetch gets an incoming from setup.
    fetch->loop (local) -> loop activity (which is not in the body) -> ignore for body traversal?
    fetch->nested (parent) -> nested loop activity (which is in the body) -> so nestedLoop gets an incoming from fetch.
    fetch:
    from setup (local) -> one incoming
    nestedLoop:
    from fetch (parent) -> one incoming
    nested-log:
    from?
    parent: nested-log->nested-tail (so nested-tail gets an incoming from nested-log)
    nested-tail:
    from nested-log (parent) and from?
    parent: nested-tail->nested (so nested gets an incoming from nested-tail)

    But wait, we also have the inner loop's own flows?
    The inner loop's ObjectCollection flows: [nested-log->nested] (note: the test sets: Flows: []*microflows.SequenceFlow{ mkFlow("nested-log", "nested"), } for the inner loop? Actually, no: the test sets for the inner loop:
    ObjectCollection: &microflows.MicroflowObjectCollection

---

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

[github-actions]

AI Code Review

Review Summary

Critical Issues: None
Moderate Issues: None
Minor Issues: None
What Looks Good:

• The PR adds a comprehensive bug test (MDL script) and unit test covering the nested loop flow traversal issue
• The fix correctly merges parent-level flows into loop-local traversal maps for nested loop descendants
• Changes are minimal, focused, and follow existing code patterns
• Test validation commands are provided and pass

Recommendation: Approve

Detailed Findings

Overlap & Duplication

• No existing proposals or code duplication found for this specific nested loop flow bug (DESCRIBE MICROFLOW can omit loop-body continuation flows stored in the parent graph #328)
• The bug test file is new and targets the exact symptom described in DESCRIBE MICROFLOW can omit loop-body continuation flows stored in the parent graph #328
• Changes are isolated to microflow traversal logic without overlapping existing functionality

MDL Syntax Design

• Not applicable: PR contains no MDL syntax changes (only executor fixes and tests)
• No grammar, AST, or visitor modifications required

Full-Stack Consistency

• Not applicable: No new MDL feature added
• Fix lives entirely in executor layer (cmd_microflows_show_helpers.go) with appropriate test coverage
• No LSP/DESCRIBE roundtrip modifications needed since DESCRIBE behavior is fixed via executor

Test Coverage

• ✅ New bug test: mdl-examples/bug-tests/328-describer-nested-loop-body-flows.mdl (executable MDL script)
• ✅ New unit test: TestTraverseFlow_LoopBodyMergesParentFlowsForExistingOrigin in cmd_microflows_traverse_test.go
• ✅ Tests validate both MDL roundtrip (describe → exec → describe) and low-level traversal logic
• ✅ No time.sleep used; relies on deterministic flow verification

Security & Robustness

• ✅ No file I/O, sockets, or external interactions in hot paths
• ✅ No silent side effects (changes only affect flow traversal logic)
• ✅ Method receivers unchanged; no mutation concerns
• ✅ Input validation preserved (nil checks in new helpers)

Scope & Atomicity

• ✅ Single concern: fix nested loop body flow preservation in DESCRIBE MICROFLOW
• ✅ All changes directly serve this goal (test + executor fix)
• ✅ No unrelated modifications bundled

Code Quality

• ✅ Refactor applied consistently: extracted collectLoopObjectIDs and helpers
• ✅ No duplicated logic; replaced manual loops with reusable functions
• ✅ Clear variable names (loopObjectIDs, appendSequenceFlowIfMissing)
• ✅ Deterministic map iteration via key-based comparison in sequenceFlowIdentity

Conclusion

The PR correctly addresses bug #328 by ensuring parent-level continuation flows are merged into loop-local traversal maps for nested loop bodies. The solution is minimal, well-tested, and maintains existing code quality standards. No checklist violations found. Ready for merge.

---

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

[ako]

Review: fix: preserve nested loop body flow traversal

Summary: Correct, well-targeted fix. The root cause (short-circuiting parent-flow merging when a local loop-boundary flow already existed for the same origin) is directly addressed, and the recursive descent via collectLoopObjectIDs correctly handles the nested-loop case the old loopActivityMap lookup missed.

Core fix correctness

Old guard:

if _, inLoop := loopActivityMap[originID]; inLoop {
    if _, exists := loopFlowsByOrigin[originID]; !exists {
        loopFlowsByOrigin[originID] = flows
    }
}

The !exists short-circuit silently dropped parent-level continuation flows for any origin that already had a local loop-boundary flow. The new code merges both using appendSequenceFlowIfMissing. ✓

The same fix is correctly applied to the flowsByDest side too. ✓

collectLoopObjectIDs — recursive descent is necessary

The old code used loopActivityMap (immediate loop objects only). The new loopObjectIDs descends into nested LoopedActivity.ObjectCollection objects, which is needed so parent-level flows for nested loop body activities are also merged. The recursion is straightforward and handles nil ObjectCollections. ✓

preferLoopBodyStart — deterministic start selection

Replacing the previous non-deterministic break with a position-based tiebreaker is a good improvement. One edge: if two activities have exactly the same X/Y position AND both have zero incoming flows, the winner still depends on map iteration order. This is an acceptable edge case (same-position activities are a user authoring issue), but a secondary tiebreaker on ID string would make it fully deterministic.

sequenceFlowIdentity — composite key gap

return fmt.Sprintf("%s>%s:%t:%d:%d", flow.OriginID, flow.DestinationID, flow.IsErrorHandler, flow.OriginConnectionIndex, flow.DestinationConnectionIndex)

The composite fallback key (used when flow.ID == "") does not include CaseValue. If a split emits two flows from the same origin to the same destination with different case values but without IDs (possible in test-constructed graphs), one would be incorrectly deduplicated. In production data every flow has a UUID so this only affects test helpers — but since test flows are constructed without IDs in this very file, it's worth noting. Adding :%v for CaseValue would close the gap.

Test coverage

The test graph is well-designed — adversarial storage order (nested loop listed first), local boundary flow on fetch to trigger the original bug, and parent-level continuation flow from fetch to nested. The ordering assertions (setup before fetch before nested loop) catch the regression.

One gap: the test verifies that nested and nested-tail appear in the output, but not their relative order inside the nested loop body. A position check similar to the outer body assertions would catch a future inner-body ordering regression.

Bug test script

Covers a realistic nested-loop microflow with outer and inner body activities. The describe → exec → describe fixpoint requirement is documented in the header. ✓

Verdict

Approve. The fix is correct and the recursive descent approach is the right solution. The two minor items (same-position tiebreaker and CaseValue in composite key) are low-priority but easy wins.

[github-actions]

AI Code Review

Critical Issues

None found.

Moderate Issues

None found.

Minor Issues

• The bug test file (mdl-examples/bug-tests/328-describer-nested-loop-body-flows.mdl) includes usage instructions as comments but lacks automated validation of the DESCRIBE output. While the checklist requires bug tests to reproduce the issue, the test currently only sets up the microflow and relies on manual execution of describe microflow for verification. An automated check (e.g., via a test that runs the DESCRIBE command and validates output) would strengthen the test, though the unit test in cmd_microflows_traverse_test.go covers the core logic.
• The helper function preferLoopBodyStart uses position-based ordering to select the loop body start. While functional, this is somewhat fragile if Mendix changes activity positioning logic. However, since it mirrors existing logic in the codebase (see similar usage elsewhere in the describer), this is acceptable.

What Looks Good

• The fix correctly addresses the root cause: merging parent-level body flows into loop-local traversal maps for nested loop descendants, ensuring continuations aren't ignored when local boundary flows exist.
• Refactoring findSplitMergePoints into findSplitMergePointsForGraph improves code reuse and clarity, allowing loop bodies to leverage the same split/merge detection as top-level microflows.
• New unit tests comprehensively validate the fix:
  • TestTraverseFlow_LoopBodyMergesParentFlowsForExistingOrigin tests the exact scenario from the bug report (local boundary flow + parent-level continuation).
  • TestTraverseFlow_LoopBodyEmitsStructuredIfElse ensures structured control flow (if/else) still works correctly within loop bodies after the refactor.
• Changes are narrowly scoped to the DESCRIBE MICROFLOW traversal logic with no unrelated modifications.
• The bug test file provides clear reproduction steps and expected behavior, satisfying the checklist requirement for bug tests to include MDL scripts in mdl-examples/bug-tests/.

Recommendation

Approve the PR. The changes are correct, well-tested, and follow project conventions. Minor test improvement suggestions do not block merging.

---

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

[github-actions]

AI Code Review

What Looks Good

• Well-targeted bug fix: The changes precisely address the issue where DESCRIBE MICROFLOW omitted nested loop body continuation flows when parent-level flows shared origins with local loop-boundary flows.
• Comprehensive test coverage: Added both an MDL test script (mdl-examples/bug-tests/328-describer-nested-loop-body-flows.mdl) and a unit test (TestTraverseFlow_LoopBodyMergesParentFlowsForExistingOrigin) that validates the fix including activity ordering and presence.
• Clean refactoring: Extracted helper functions (collectLoopObjectIDs, appendSequenceFlowIfMissing, sequenceFlowIdentity, preferLoopBodyStart) improve readability and reusability without changing behavior.
• Recursive handling: Properly collects nested loop body object IDs recursively to handle arbitrarily deep nesting.
• Flow deduplication: Implements robust flow identity checking to avoid duplicates when merging parent-level flows into loop-local maps.
• Maintains existing behavior: The fix enhances rather than rewrites the traversal logic, preserving all existing functionality while closing the gap.

Recommendation

Approve. This is a well-scoped bug fix that:

• Addresses a specific symptom described in issue DESCRIBE MICROFLOW can omit loop-body continuation flows stored in the parent graph #328
• Includes appropriate test coverage at both MDL and unit levels
• Makes minimal, focused changes to the executor logic
• Follows existing code patterns and conventions
• Requires no MDL syntax changes or pipeline modifications (as it's fixing existing DESCRIBE MICROFLOW behavior)
• Passes all validation checks mentioned in the PR (go test, make build, make lint-go, make test)

The fix correctly merges parent-level body flows into loop-local traversal maps for origins in the loop body, resolving the issue where nested loop bodies would collapse into single statements during DESCRIBE → EXECUTE → DESCRIBE roundtrips.

---

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

[github-actions]

AI Code Review

Critical Issues

• None found

Moderate Issues

• None found

Minor Issues

• None found

What Looks Good

• The PR correctly addresses bug DESCRIBE MICROFLOW can omit loop-body continuation flows stored in the parent graph #328 by improving loop body flow traversal in the microflow describer
• Changes are focused and atomic: refactoring split/merge point logic, rewriting emitLoopBody to properly merge parent-level flows for nested loops, and adding comprehensive tests
• Test coverage is strong:
  • Added regression test script in mdl-examples/bug-tests/ (328-describer-nested-loop-body-flows.mdl)
  • Added two unit tests in cmd_microflows_traverse_test.go covering the specific fix and structured if/else preservation in loops
  • Tests avoid time.Sleep and use proper synchronization
• Code quality improvements:
  • Extracted findSplitMergePointsForGraph for reuse
  • Added helper functions (collectLoopObjectIDs, appendSequenceFlowIfMissing, etc.) with clear purposes
  • Improved deduplication logic for sequence flows using stable identities
  • Maintained existing function signatures and behavior for callers
• No MDL syntax changes were made, so syntax design and full-stack consistency checks are not applicable
• PR follows atomicity principle: single bug fix with related refactoring and tests
• No security concerns identified in the traversal logic changes

Recommendation
Approve the PR. The changes are well-tested, focused, and resolve the reported issue without introducing regressions. The test additions provide adequate coverage for the fix and related edge cases.

---

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

[hjotha]

Review follow-up status on the current head:

• Included CaseValue in the synthetic sequence-flow identity fallback used by tests/ID-less flows.
• Added deterministic loop-body start tie-breaking by ID for equal positions.

Local validation after the update was green:

make build
make test
make lint-go