Skip to content

fix: drop empty else branch from describer output#305

Open
hjotha wants to merge 2 commits intomendixlabs:mainfrom
hjotha:audit/describer-empty-else-branch
Open

fix: drop empty else branch from describer output#305
hjotha wants to merge 2 commits intomendixlabs:mainfrom
hjotha:audit/describer-empty-else-branch

Conversation

@hjotha
Copy link
Copy Markdown
Contributor

@hjotha hjotha commented Apr 26, 2026

Part of #332.

Fixes #304.

Summary

DESCRIBE MICROFLOW emitted an else line for IF statements whose false flow jumps directly to the merge point. That produced a spurious empty branch in MDL output and broke describe/exec/describe equality.

Root cause

The split traversal printed else whenever a false flow existed, without checking whether the false flow was only the direct split-to-merge edge that represents "no else body".

Fix

Skip the else keyword when the false branch destination is the matching merge point. Explicit false-branch bodies are still emitted normally.

Tests

Added focused traversal coverage for an IF without an ELSE body.

Validation

  • make build
  • make test
  • make lint-go
  • make test-integration

Agentic Code Testing

  • No new authoring syntax.
  • Synthetic regression coverage exercises the describe output shape agents consume.

Test plan

  • Else-less IF does not emit a blank else.
  • Existing IF/ELSE traversal remains covered by the existing test suite.

When an ExclusiveSplit's FALSE flow jumps straight to the merge point, as the builder emits for `if X then ... end if` without an else body, the describer was still printing `else` followed by nothing.

That produced a spurious empty branch in MDL output. Re-parsing normalized the branch away again, but describe/exec/describe no longer stayed byte-identical.

Skip the `else` keyword when the FALSE flow's destination is the matching merge point. Explicit false-branch bodies are still emitted normally.

Tests cover traversal for an IF without an ELSE body.
@github-actions
Copy link
Copy Markdown

AI Code Review

What Looks Good

  • The fix correctly addresses the root cause by skipping the else branch when the false flow jumps directly to the merge point (indicating no else body exists), preventing spurious empty branches in MDL output.
  • The added test TestTraverseFlow_IfWithoutElse provides focused coverage for the IF-without-ELSE scenario and validates the fix.
  • The change is minimal, localized, and maintains backward compatibility for existing IF/ELSE traversal (as noted in validation).
  • The PR follows scope and atomicity principles: it addresses a single bug fix with a clear test and no unrelated changes.
  • Validation steps (make build, make test, make lint-go, make test-integration) and agentic code testing are completed.

Recommendation

Approve. The PR resolves the issue without introducing new risks, adheres to project conventions, and includes adequate test coverage. No changes are required.


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

Adds an MDL script under mdl-examples/bug-tests/ that exercises the
describe → exec → describe fixpoint for IF-without-else, plus a negative
control with an explicit else body.

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 correctly addresses bug DESCRIBE MICROFLOW emits an empty else branch for if-without-else #304 by modifying the microflow traverser to skip emitting an empty else branch when the false flow jumps directly to the merge point.
  • The fix is minimal and targeted, only affecting the specific case that caused the spurious else output.
  • Test coverage is adequate:
    • Added a bug test script (mdl-examples/bug-tests/304-describer-empty-else-branch.mdl) that verifies both the fix (IF without else) and a negative control (IF with else preserved).
    • Added a unit test (mdl/executor/cmd_microflows_traverse_test.go) specifically for the IF-without-else scenario.
  • The PR follows the project's validation process: all make targets (build, test, lint-go, test-integration) pass.
  • The change maintains backward compatibility: explicit else bodies are still emitted correctly.
  • The fix aligns with the described root cause and produces a clean describe/exec/describe roundtrip.
  • No MDL syntax changes are involved, so syntax design and full-stack consistency checklists are not applicable.
  • The PR is scoped to a single bug fix with clear, atomic commits.

Recommendation

Approve the PR. It correctly resolves the reported issue with appropriate test coverage and maintains all existing functionality. No changes are requested.


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.

DESCRIBE MICROFLOW emits an empty else branch for if-without-else

2 participants