fix: pair microflow splits with nearest merge

[hjotha]

Part of #332.

Summary

• pair exclusive splits with the nearest common exclusive merge
• ignore error-handler flows when computing structural split/merge pairing
• add regression tests for sequential if-without-else continuation placement

Closes #326

Validation

• make build
• make lint-go
• make test

[github-actions]

AI Code Review

Critical Issues

None found.

Moderate Issues

None found.

Minor Issues

• Unused parameters: In collectReachableDistances, the parameters ctx and activityMap are marked as unused (_ = ctx; _ = activityMap). While harmless, removing them would improve clarity since they aren't used in the function body.
• Deterministic tie-breaking: In selectNearestCommonMerge, the final tie-breaker uses string(candidates[i].id) < string(candidates[j].id). While this ensures deterministic output, consider whether natural ID ordering (e.g., lexical) is sufficient or if a more semantic tie-breaker (e.g., positional in microflow) would be better. However, the current approach is acceptable for stability.

What Looks Good

• Algorithm improvement: The new distance-based approach correctly identifies the nearest common exclusive merge by measuring shortest-path distances per branch, fixing the original issue where splits were incorrectly paired with downstream merges (e.g., in sequential ifs).
• Error-handler exclusion: The use of findNormalFlows to skip error-handler flows during traversal aligns with the requirement to ignore them for structural split/merge pairing.
• Test coverage: Two new tests validate:
  1. Nearest merge selection before downstream splits (TestFindMergeForSplit_ChoosesNearestMergeBeforeDownstreamIf).
  2. Continuation placement for sequential ifs-without-else (TestTraverseFlow_SequentialIfWithoutElseKeepsContinuationOutsideFirstIf), which directly addresses the regression mentioned in the PR.
• Code clarity: The refactored logic is well-commented and separates concerns (distance collection vs. merge selection). The sort criteria (minimizing max distance, then sum distance) is logical for identifying the "nearest" merge in a structural sense.
• No scope creep: Changes are focused exclusively on microflow split/merge pairing logic and its test coverage, with no unrelated modifications.

Recommendation

Approve. The PR correctly fixes the split/merge pairing issue with thoughtful algorithmic improvements, includes relevant regression tests, and maintains code quality. Minor cleanups (unused parameters) are trivial and do not block merging.

---

Note: No MDL syntax changes were introduced, so syntax design and full-stack consistency checks were not applicable.

---

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

[github-actions]

AI Code Review

Critical Issues

None found.

Moderate Issues

None found.

Minor Issues

• In mdl/executor/cmd_microflows_show.go, the function collectReachableDistances has unused parameters (ctx and activityMap) that are explicitly ignored with _ = ctx and _ = activityMap. While this is acceptable, removing them would clean up the signature if they're truly not needed for the algorithm.
• The bug test file's usage example shows mxcli -p app.mpr -c "describe microflow BugTest326.MF_SequentialIfs" which requires quotes around the qualified name due to the dot. This is correct MDL syntax, but worth noting for users.

What Looks Good

• The fix correctly implements nearest common merge selection using shortest-path distances per branch
• Error-handler flows are properly excluded from structural pairing via findNormalFlows
• Comprehensive test coverage added:
  • New bug test file (mdl-examples/bug-tests/326-describer-pair-split-with-nearest-merge.mdl) demonstrating the fix
  • Unit tests verifying the nearest merge selection and sequential if-continuation placement
• The changes are focused solely on the split/merge pairing logic without touching unrelated functionality
• Code is well-commented explaining the algorithm and rationale
• Maintains backward compatibility (still returns empty ID when no merge found)
• Follows existing patterns for bug tests and executor tests in the project

Recommendation

Approve. The PR correctly fixes issue #326 with appropriate tests and minimal, focused changes. No blockers identified.

---

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

[github-actions]

AI Code Review

What Looks Good

The PR correctly addresses issue #326 by fixing the split/merge pairing logic in the microflow describer:

• Root cause fix: Changed from finding any common downstream merge to finding the nearest common merge using shortest-path distance calculation
• Error handler exclusion: Properly excludes error-handler flows from structural pairing consideration via findNormalFlows()
• Algorithm improvement: Uses BFS to compute distances and selects merges with minimal max distance (then sum distance) for optimal pairing
• Test coverage:
  • Added bug test script showing the fixpoint behavior (describe → exec → describe)
  • Added unit tests verifying nearest merge selection and correct continuation placement
  • Tests cover the specific sequential if-without-else scenario mentioned in the issue
• Code quality:
  • Clean refactor with well-named helper functions (collectReachableDistances, selectNearestCommonMerge)
  • Deterministic tie-breaking in merge selection
  • No unnecessary changes to unrelated code

Recommendation

Approve the PR. The fix correctly implements the nearest-common-merge pairing logic while ignoring error-handler flows, includes appropriate test coverage, and maintains consistency with the project's architecture and coding standards. No changes to MDL syntax or full-stack feature wiring were needed since this is a behavioral fix to existing DESCRIBE functionality.

---

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