Skip to content

fix: preserve annotations attached inside loop bodies#331

Open
hjotha wants to merge 2 commits intomendixlabs:mainfrom
hjotha:submit/preserve-loop-body-annotations
Open

fix: preserve annotations attached inside loop bodies#331
hjotha wants to merge 2 commits intomendixlabs:mainfrom
hjotha:submit/preserve-loop-body-annotations

Conversation

@hjotha
Copy link
Copy Markdown
Contributor

@hjotha hjotha commented Apr 27, 2026

Part of #332.

Summary

  • copy loop/while body annotation flows into the parent microflow graph
  • collect annotation captions recursively from nested loop object collections
  • merge loop-local annotation flows into loop body traversal
  • add regressions for annotated nested decisions and annotation flows stored inside loop object collections

Closes #330

Validation

  • go test ./mdl/executor -run 'TestLoopBodyIfAnnotationPromotedToParentFlows|TestTraverseFlow_LoopBodyUsesNestedAnnotationFlows'
  • make build
  • make lint-go
  • make test

@github-actions
Copy link
Copy Markdown

AI Code Review

What Looks Good

  • The PR correctly identifies and fixes the issue where annotations inside loop bodies weren't being preserved in the parent microflow graph
  • The test TestLoopBodyIfAnnotationPromotedToParentFlows properly validates the fix for an annotated nested decision inside a loop body
  • The changes are minimal and focused: adding two lines to propagate annotationFlows in loop/while statement handling, and refactoring annotation collection to be recursive for nested loops
  • The refactoring in cmd_microflows_show_helpers.go cleanly separates concerns with collectAnnotationCaptions and mergeAnnotationsByTarget functions
  • No new MDL syntax is introduced, so syntax design checks don't apply
  • The fix maintains backend abstraction compliance by working entirely within the executor layer
  • Test coverage is added for the specific bug scenario
  • The changes are scoped to a single concern (preserving loop body annotations)

Recommendation

Approve. The PR correctly fixes the annotation preservation issue in loop bodies with minimal, well-targeted changes and appropriate test coverage. All relevant checklist items are satisfied for this bug fix.


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

Symptom: annotations attached to statements inside loop or while bodies could disappear after describe/exec because nested annotation flows were not visible from the traversal context.

Root cause: loop body builders copied only sequence flows back to the parent builder, describe collected annotation captions only from top-level objects, and loop-local annotation flows were not merged into the annotation map used while traversing loop bodies.

Fix: promote nested loop annotation flows to the parent graph, collect annotation captions recursively from nested loop object collections, and merge loop-local annotations into loop body traversal.

Tests: add regression coverage for an annotated nested decision built inside a loop body and for annotation flows stored directly in a loop object collection.
@hjotha
Copy link
Copy Markdown
Contributor Author

hjotha commented Apr 27, 2026

Updated the branch to also cover annotation flows stored directly inside loop object collections. Validation after the update:

  • go test ./mdl/executor -run 'TestLoopBodyIfAnnotationPromotedToParentFlows|TestTraverseFlow_LoopBodyUsesNestedAnnotationFlows'
  • make build
  • make lint-go
  • make test

@hjotha hjotha force-pushed the submit/preserve-loop-body-annotations branch from 01d88e0 to 15605e0 Compare April 27, 2026 01:01
Adds an MDL script under mdl-examples/bug-tests/ exercising an
annotated IF nested inside a LOOP. After exec, the describe output
must contain `@annotation 'note on nested if'`, confirming the
builder propagates nested annotation flows to the parent graph and
the describer collects captions recursively.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

AI Code Review

Critical Issues

None found.

Moderate Issues

None found.

Minor Issues

None found.

What Looks Good

  • The PR adds a comprehensive bug test in mdl-examples/bug-tests/330-preserve-loop-body-annotations.mdl that reproduces the issue and validates the fix.
  • The unit tests (TestLoopBodyIfAnnotationPromotedToParentFlows and TestTraverseFlow_LoopBodyUsesNestedAnnotationFlows) properly cover the AST→BSON build path and the describer side.
  • The fix is minimal and focused:
    • In the builder (cmd_microflows_builder_control.go), annotation flows from loop bodies are now propagated to the parent graph for both LoopStmt and WhileStmt.
    • In the show helpers (cmd_microflows_show_helpers.go), annotation caption collection is made recursive to handle nested loop object collections, and loop-local annotations are merged into the body traversal.
  • The changes follow the existing patterns in the codebase (e.g., similar propagation of flows, use of helper functions for annotation handling).
  • No MDL syntax is modified, so full-stack consistency checks for new syntax are not applicable.
  • The PR is scoped to a single bug fix (annotations in loop bodies) with no unrelated changes.
  • Test coverage includes both unit tests and an MDL bug test, avoiding time.Sleep for synchronization.

Recommendation

approve - The PR correctly addresses the issue with minimal, well-tested changes that follow existing patterns. No blocking concerns identified.


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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Loop-body annotations can be lost during microflow describe/exec

2 participants