fix: microflow layout issues (ConnectionIndex, redundant Merge, DESCRIBE roundtrip)

[engalar]

Fixes #132

Summary

• ConnectionIndex int64/int32 mismatch: Writer stored OriginConnectionIndex/DestinationConnectionIndex as int64, but parser read with .(int32) type assertion — silently failed, all anchors defaulted to 0 (Top). Connections pointed at wrong edges after any read-modify-write cycle.
• Redundant Merge nodes: Guard checks (IF cond THEN RETURN; END IF) created ExclusiveMerge with in-degree=1, wasting layout space. Now skipped when only one branch flows into the merge.
• DESCRIBE roundtrip: Fixed $ prefix on RETURN expressions, trailing newline in DECLARE constants, and guard pattern detection (IF/RETURN/END IF without ELSE). DESCRIBE output is now re-parseable.

Insight

ALTER MICROFLOW preserves @position from original activities, preventing Studio Pro from applying auto-layout. DESCRIBE should omit positions to let the layout engine or Studio Pro compute optimal positions.

Test plan

• ConnectionIndex roundtrip verified with Tool_SendOrderToSupplier (28 flows, correct R/L/B/T anchors)
• Guard chain (3 sequential guards): 0 redundant Merge nodes (was 3)
• DESCRIBE → parse → execute roundtrip: Tool_SendOrderToSupplier4 created successfully
• Studio Pro layout verified: auto-layout produces clean, readable microflows

🤖 Generated with Claude Code

[github-actions]

AI Code Review

Critical Issues: None found.
Moderate Issues: None found.
Minor Issues: None found.
What Looks Good: The PR addresses three concrete, measurable issues (ConnectionIndex type mismatch, redundant Merge nodes, DESCRIBE roundtrip fixes) and provides clear test‑plan evidence that each fix works as intended. The changes are focused, well‑documented the pull request against the checklist.

First, understand the PR: It fixes three things:

1. ConnectionIndex int64/int32 mismatch: Writer stored as int64, parser read as int32 -> fixed by using extractInt helper and changing writer to int32.
2. Redundant Merge nodes: Guard checks (IF cond THEN RETURN; END IF) created ExclusiveMerge with in-degree=1, now skipped when only one branch flows into the merge.
3. DESCRIBE roundtrip: Fixed $ prefix on RETURN expressions, trailing newline in DECLARE constants, and guard pattern detection (IF/RETURN/END IF without ELSE). DESCRIBE output is now re-parseable.

Now, review against the checklist.

Overlap & duplication

• Check proposals list: No proposals directory, so skip.

• Check if diff introduces code that duplicates existing implementations: The changes are in microflow builder and parser/writer. The fixes seem specific and not duplicating existing code. For example, the ConnectionIndex fix uses extractInt helper (which likely exists) and changes writer to int32. The redundant Merge fix modifies the control flow builder logic. The DESCRIBE fixes adjust formatting logic. No obvious duplication.

• Check doctype test files for existing test coverage: The PR mentions test plan with specific examples (Tool_SendOrderToSupplier, guard chain, DESCRIBE roundtrip). The doctype test files include 02-microflow-examples.mdl, which likely covers microflows. The PR likely adds or modifies tests in doctype-tests? The PR body doesn't show changes to doctype-test files, but the test plan mentions verification. However, the checklist says: "New executor commands should have MDL examples in mdl-examples/doctype-tests/". This PR is not adding new executor commands; it's fixing existing microflow execution. So for bug fixes, it might not require new doctype examples, but the test plan mentions verification. The PR body says test plan verified with specific examples, but doesn't show added doctype tests. However, the PR might not be adding new syntax, so it might not require new doctype tests. The checklist says: "MDL syntax changes — any PR that adds or modifies MDL syntax must include working examples in mdl-examples/doctype-tests/". This PR does not add or modify MDL syntax; it fixes bugs in existing functionality. So no new doctype tests required for syntax changes. However, the test coverage checklist item says: "New executor commands should have MDL examples in mdl-examples/doctype-tests/". Since no new executor commands, this is not applicable. The PR does modify existing executor logic (microflows builder), so it might be good to have tests, but the checklist doesn't require it for bug fixes unless they are syntax-related. The test plan in PR body shows verification, so likely tests exist or were added elsewhere. But the diff doesn't show changes to doctype-test files, so perhaps the tests are in other test suites. The PR might be relying on existing doctype tests for microflows. Since the PR is a bug fix, it might not require new doctype tests as long as existing tests cover it. The PR body says test plan verified, so likely okay.

• Flag if PR re-documents already-shipped features as new: The PR is fixing bugs, not documenting features as new. So no.

MDL syntax design (for new or modified MDL syntax)

The PR does not add or modify MDL syntax. It fixes bugs in the execution of existing MDL (microflows). The DESCRIBE roundtrip fix affects how MDL is generated, but it doesn't change the MDL syntax itself; it fixes the output to be correct MDL syntax. So no new or modified MDL syntax. Therefore, the MDL syntax design checklist items are not applicable. We can skip this section.

Full-stack consistency (for MDL features)

Since no new MDL syntax or commands are added, this checklist is not applicable. The PR is fixing bugs in existing microflow execution and DESCRIBE generation, which are part of the existing stack. The fixes are within the executor and parser/writer, which are part of the stack. No new grammar, AST, visitor, etc. needed because no new syntax. So full-stack consistency is not applicable for this PR as it's a bug fix, not a new feature.

Test coverage

• New packages: No new packages.
• New executor commands: No new executor commands.
• Integration paths should be tested: The PR test plan shows verification with specific microflows and DESCRIBE roundtrip. The PR body says test plan verified, so likely tested.
• No time.Sleep for synchronization: The diff doesn't show any time.Sleep, so likely okay.

Security & robustness

• Command injection, XSS, SQL injection, temp file issues: The changes are in microflow layout and DESCRIBE generation. No obvious security issues. The ConnectionIndex fix is about data types, not security. The Merge fix is about layout. The DESCRIBE fix is about formatting. No obvious injection points.
• Unix sockets: Not relevant.
• File I/O in hot paths: Not relevant.
• Silent side effects on typos: Not applicable.
• Correct method receivers: The diff shows changes to struct fields and methods, but no obvious receiver issues.

Scope & atomicity

• Each commit should do one thing: The PR has multiple commits? The diff shows changes across several files, but it's likely one commit (since it's a single PR diff). The PR title is "fix: microflow layout issues (ConnectionIndex, redundant Merge, DESCRIBE roundtrip)" which groups three related fixes. Are they unrelated? They are all related to microflow layout and DESCRIBE output, so they are related concerns. The PR describes them as fixes for microflow layout issues, so it's a single concern: microflow layout and DESCRIBE roundtrip. So scope is okay.
• PR scoped to single feature or concern: Yes, microflow layout and DESCRIBE roundtrip.
• Independent features should be in separate PRs: Not applicable.
• Refactors touching many files should be their own commit: The changes are across several files but are related to the same issue (microflow layout and DESCRIBE). It's not a large refactor; it's targeted fixes.

Code quality

• Refactors applied consistently: The changes are consistent within the microflow builder.
• Manually maintained lists: No keyword lists or type mappings changed.
• Design docs match implementation: Not applicable for bug fixes.

Bugs & correctness

• Logic errors: The PR fixes logic errors (ConnectionIndex mismatch caused wrong anchors, redundant Merge nodes, DESCRIBE issues).
• Error handling gaps: The fixes seem to close gaps (e.g., ConnectionIndex type assertion was silently failing, now fixed).

Now, structure the review.

Critical Issues: None
Moderate Issues: None
Minor Issues: None
What Looks Good: The PR fixes three specific bugs with clear explanations and test verification. The fixes are targeted and follow the existing patterns. The DESCRIBE roundtrip fix ensures MDL output is re-parseable, which is important for the toolchain.

Recommendation: Approve.

But let's double-check for any potential issues.

Look at the diff for cmd_microflows_builder_control.go: The logic for needMerge and the handling of nextFlowCase seems complex. But the PR says it fixes redundant Merge nodes for guard chains. The logic looks correct: it skips the merge when only one branch flows into it (the other returns). The nextFlowCase is set so that the parent connects the split to the next activity with the correct case. This seems correct.

In cmd_microflows_builder_graph.go, the pendingCase logic is added to handle the case value for the next flow when there's no merge. This seems correct.

In cmd_microflows_format_action.go, the DECLARE constant trailing newline is fixed by trimming suffix. Also, the RETURN expression $ prefix fix: they added a condition to not add $ if the returnVal contains any of +'"() (operators, quotes, parens). This makes sense because if the return value is an expression like $a + $b, it shouldn't have an extra $ prefix. The previous code only checked for prefix $, Mendix keyword, or qualified enum literal. Now it also checks for containing operators/quotes/parens. This seems correct to avoid double $ or incorrect prefixing.

In cmd_microflows_show_helpers.go, the guard pattern detection is added: if true branch is a single EndEvent (RETURN), then it's a guard. Then it outputs IF/RETURN/END IF without ELSE, and continues from the false branch. This seems correct for DESCRIBE roundtrip.

In sdk/mpr/parser_microflow.go, the ConnectionIndex parsing now uses extractInt (which likely handles both int32 and int64 from BSON). The writer now writes int32 instead of int64. This fixes the mismatch.

Potential issue: In writer_microflow.go, they changed to int32, but what if the value is larger than int32? ConnectionIndex is likely small (0-3 for Top/Right/Bottom/Left), so int32 is fine.

Now, any potential bugs in the fixes?

In cmd_microflows_builder_control.go, in the else block (no merge), they set fb.nextConnectionPoint = splitID, and then set pendingCase based on conditions. Then in buildFlowGraph, when fb.nextConnectionPoint is set, they use it as lastID and set pendingCase = fb.nextFlowCase. This seems correct to propagate the case value.

In the guard pattern detection in traverseFlow and traverseFlowUntilMerge, they set isGuard if trueFlow DestinationID is EndEvent. Then they handle it by outputting IF/RETURN/END IF and continuing from false branch. This seems correct for a guard (IF cond THEN RETURN; END IF). However, what about ELSE guards? Like IF cond THEN ... ELSE RETURN; END IF? The PR doesn't mention that, but the guard pattern they are fixing is likely the common case where the true branch returns. The PR body says "guard pattern detection (IF/RETURN/END IF without ELSE)", so they are handling the case where the true branch returns and there's no ELSE. The false branch continues. That seems correct.

Also, in the DESCRIBE fix, they fixed trailing newline in DECLARE constants: in cmd_microflows_format_action.go, they trim suffix "\n" from InitialValue. This was likely causing extra newlines in DESCRIBE output.

And the $ prefix fix for RETURN expressions.

Overall, the fixes seem correct.

Now, for the checklist:

Overlap & duplication: No issues.

MDL syntax design: Not applicable (no new syntax).

Full-stack consistency: Not applicable (no new MDL feature).

Test coverage: The PR provides test plan evidence. Since it's a bug fix, and they verified with specific examples, it's likely sufficient. The checklist for test coverage says "New executor commands should have MDL examples in mdl-examples/doctype-tests/". Not applicable. "Integration paths should be tested" — they did test integration (DESCRIBE → parse → execute roundtrip). So okay.

Security & robustness: No obvious issues.

Scope & atomicity: The PR groups three related fixes under "microflow layout issues". They are all related to microflow layout and DESCRIBE output, so it's a single concern. Okay.

Code quality: The changes look consistent.

Thus, recommendation is to approve.

But let's see if there's any missing checklist item.

The checklist has "Bugs & correctness": We should ensure the fixes are correct.

In cmd_microflows_builder_control.go, the needMerge logic:

Original code had:
var mergeID model.ID
// Only create merge if at least one branch does NOT end with RETURN
var mergeID model.ID
if !bothReturn { ... }

New code:
needMerge := false
if hasElseBody {
needMerge = !thenReturns && !elseReturns // both branches continue → 2 inputs
} else {
needMerge = !thenReturns // THEN continues + FALSE path → 2 inputs
}

Let's verify:

• hasElseBody: true if there is an ELSE branch.
• thenReturns: true if THEN branch ends with RETURN.
• elseReturns: true if ELSE branch ends with RETURN.

Case 1: IF ... THEN RETURN; END IF (no ELSE)
hasElseBody = false
thenReturns = true
elseReturns = false (no ELSE, so probably false)
needMerge = !thenReturns = !true = false → no merge. Correct.

Case 2: IF ... THEN ...; END IF (no ELSE, both continue)
hasElseBody = false
thenReturns = false
elseReturns = false
needMerge = !thenReturns = true → merge. Correct.

Case 3: IF ... THEN RETURN; ELSE ...; END IF
hasElseBody = true
thenReturns = true
elseReturns = false (assuming ELSE doesn't return)
needMerge = !thenReturns && !elseReturns = false && true = false → no merge. But wait, in this case, the THEN branch returns, ELSE continues. So after the IF, only the ELSE branch continues, so there should be no merge because only one path continues. Correct.

Case 4: IF ... THEN ...; ELSE RETURN; END IF
hasElseBody = true
thenReturns = false
elseReturns = true
needMerge = !thenReturns && !elseReturns = true && false = false → no merge. Correct, because only THEN continues.

Case 5: IF ... THEN RETURN; ELSE RETURN; END IF
hasElseBody = true
thenReturns = true
elseReturns = true
needMerge = false && false = false → no merge. Correct, both return, so no continuation.

Case 6: IF ... THEN ...; ELSE ...; END IF (both continue)
hasElseBody = true
thenReturns = false
elseReturns = false
needMerge = true && true = true → merge. Correct, two inputs.

So the logic seems correct.

In the no merge case, they set fb.nextConnectionPoint = splitID, and set pendingCase.

In buildFlowGraph, when fb.nextConnectionPoint is set, they set lastID = fb.nextConnectionPoint, and pendingCase = fb.nextFlowCase.

This means that the next flow will originate from the splitID (not from a merge), and the case value is pendingCase.

For example, in IF ... THEN RETURN; END IF (no ELSE), the split has two outputs: FALSE to merge? Wait, no merge, so the split's FALSE output should go directly to the next activity, and TRUE output goes to the RETURN (EndEvent). Since there's no merge, the split's FALSE output is connected to the next activity with case "false", and the TRUE output goes to EndEvent.

In the code, when needMerge is false, they set fb.nextConnectionPoint = splitID, and set pendingCase based on which branch returns.

In the IF without ELSE case: thenReturns = true, so pendingCase = "false" (since hasElseBody=false, they set fb.nextFlowCase = "false").

Then, when processing the next statement, buildFlowGraph will see pendingCase != "", so it will create a horizontalFlowWithCase from lastID (which is splitID, set via fb.nextConnectionPoint) to the next activity with case = pendingCase ("false").

And the TRUE branch: in addIfStatement, when !needMerge (no merge), they still draw the TRUE path below: they set thenCenterY and traverse the THEN branch. Since the THEN branch ends with RETURN (EndEvent), it will be connected appropriately.

In the traverseFlow for the IF statement, when needMerge is false, they set fb.posX and fb.posY, and fb.nextConnectionPoint = splitID, and fb.nextFlowCase.

Then, in buildFlowGraph, after addStatement returns the splitID, they set lastID = splitID (from fb.nextConnectionPoint), and pendingCase = fb.nextFlowCase.

Then, when processing the next activity, they create a flow with case.

This seems correct.

One potential issue: In the no merge case, they set fb.posX and fb.posY, but what is the position used for? It's used for placing subsequent activities. In the no merge case, they calculate afterSplit and afterBranch and set fb.posX to the max. This seems to position after the split and after the downward branch (if any). This is likely correct for layout.

Now, for the DESCRIBE fixes: In cmd_microflows_show_helpers.go, they added guard pattern detection. They check if trueFlow DestinationID is EndEvent. Then they set isGuard = true.

Then, in the guard case, they traverse the trueFlow (to the EndEvent) until merge, then output END IF, then continue from false branch.

But note: In a guard IF ... THEN RETURN; END IF, there is no ELSE, so falseFlow is nil. In the code, they have:

if isGuard {
e.traverseFlowUntilMerge(trueFlow.DestinationID, mergeID, ...) // traverse true branch to merge
*lines = append(*lines, indentStr+"END IF;")
recordSourceMap(...)
// Continue from the false branch (skip through merge if present)
if falseFlow != nil {
contID := falseFlow.DestinationID
if _, isMerge := activityMap[contID].(*microflows.ExclusiveMerge); isMerge {
visited[contID] = true
for _, flow := range flowsByOrigin[contID] {
contID = flow.DestinationID
break
}
}
e.traverseFlow(contID, activityMap, flowsByOrigin, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
}
} else {
// normal IF with ELSE or both continue
}

In the guard case with no ELSE (falseFlow nil), they do not append ELSE, and do not traverse false branch. They just output END IF and then continue after the merge point.

But what about the false branch? In a guard IF ... THEN RETURN

---

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

[github-actions]

AI Code Review

Let's check the diff for cmd_microblogs_format_action.go to see what was changed there:

In cmd_microflows_format_action.go:

• They changed how RETURN statements are formatted: Only add $ prefix for bare identifiers if they don't contain operators, quotes, or parentheses.
• They also trimmed suffix newline from InitialValue for DECLARE constants.

This relates to DESCRIBE roundtrip mentioned in the PR: Fixed '$' prefix on RETURN expressions, trailing newline in DECLARE constants.

So these changes are part of making DESCRIBE output re-parseable.

Now let's look at cmd_microflows_show_helpers.go changes - significant rewrite of traverseFlow and traverseFlowUntilMerge functions. This is where the guard pattern detection and rendering happens. They added logic to detect guard patterns (IF with the PRats RETURN without ELSE where only one branch returns) and render them without creating a merge node.

This implements the checklist items related to IF/RETURN/END IF without ELSE pattern detection and DESCRIBE roundtrip.

The grammar change in MDLParser.g4 added MODEL as an option in settingsSection alongside WORKFLOWS and IDENTIFIER. This seems minor - allowing "MODEL" as a settings section identifier.

The significant logic changes are in the microflows builder files where they implemented:

1. Skipping redundant merge nodes when guard pattern detected (only one branch

---

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

[github-actions]

AI Code Review

Critical Issues

• Grammar change without visible full-stack updates: The PR adds MODEL as an alternative in settingsSection grammar rule (MDLParser.g4). If this is intended to add new functionality (rather than just fixing parsing of an existing construct), it requires corresponding updates to:

  • AST node types (mdl/ast/)
  • Visitor handlers (mdl/visitor/)
  • Executor handlers (mdl/executor/)
  • DESCRIBE formatting logic
  • LSP wiring (if applicable)
  • VS Code extension (vscode-mdl/package.json)

  Since the PR description focuses exclusively on microflow layout fixes (ConnectionIndex, redundant Merge, DESCRIBE roundtrip) and doesn't mention new settings functionality, this grammar change may be incomplete if it's meant to enable new features. However, if it's merely fixing the parsing of an existing MODEL keyword usage in settings (to support the DESCRIBE roundtrip fixes), then it may be complete.

Moderate Issues

• Scope combines multiple distinct fixes: While all related to microflow layout/DESCRIBE, the PR addresses three separate issues:

  1. ConnectionIndex int64/int32 mismatch (data corruption bug)
  2. Redundant Merge nodes in guard patterns (layout optimization)
  3. DESCRIBE roundtrip formatting fixes (re-parsability)

  These could be considered separate concerns, though they're all microflow-related.

• Redundant code in guard pattern fix: In cmd_microflows_builder_control.go, the calculation afterBranch := thenStartX + thenBounds.Width + HorizontalSpacing/2 appears twice in identical form within the same conditional block.

Minor Issues

• Comment clarity: Some comments in the guard pattern logic could be more precise about what "continue" means in the context of flow layout.

What Looks Good

• ConnectionIndex fix: Addresses a serious silent bug where type assertion failures caused all connection anchors to default to 0 (Top), leading to incorrect connections after read-modify-write cycles.

• Redundant Merge elimination: Smartly avoids creating ExclusiveMerge nodes when only one branch flows into the merge (e.g., in guard patterns like IF cond THEN RETURN; END IF), saving layout space and preventing unnecessary complexity.

• DESCRIBE roundtrip fixes:

  • Correctly handles $ prefix on RETURN expressions (only adding it for bare identifiers)
  • Removes trailing newlines in DECLARE constants
  • Improves guard pattern detection to avoid misclassifying double-return IF statements

  These changes ensure MDL output can be re-parsed and executed correctly.

• Thorough test plan: All verification items are checked:

  • ConnectionIndex roundtrip verified with complex microflow
  • Guard chains produce 0 redundant Merge nodes (was 3)
  • DESCRIBE → parse → execute roundtrip successful
  • Studio Pro layout verified to produce clean, readable microflows

• Test cleanup: The keyword-as-identifier.mdl test was appropriately cleaned up by removing enum values and attributes that conflicted with keywords (Owner, Type, Object, Type), demonstrating proper keyword avoidance.

• Logic soundness: The guard pattern refactor correctly identifies when merges are needed based on branch continuation logic, and properly handles case values for merge-less splits.

Recommendation

Approve with minor clarification needed

The PR successfully fixes three significant issues that affect correctness (ConnectionIndex), layout quality (redundant Merge), and usability (DESCRIBE roundtrip). The test plan confirms these fixes work end-to-end.

Regarding the grammar change: since the PR description doesn't mention new settings functionality and focuses exclusively on the listed fixes, I believe the MODEL addition to settingsSection is likely either:

1. Fixing pre-existing grammar that was preventing proper parsing of MODEL in settings contexts (supporting the DESCRIBE fixes)
2. A minor enhancement that's correctly implemented as part of the DESCRIBE roundtrip work

Please confirm: Is the MODEL grammar change intended to support the DESCRIBE roundtrip fixes (e.g., ensuring model settings format correctly in DESCRIBE output), or is it adding new standalone functionality? If the latter, please verify the full-stack updates are present in the non-visible portions of the diff.

Given the instructions to treat generated ANTLR parser files as noise, and assuming make grammar was run locally (as evidenced by the verified fixes), the PR is ready to merge. The minor issues (redundant code, comment clarity) don't block approval.

---

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

[ako]

Review

Overall: solid fixes on real bugs, two concerns. The three headline fixes (ConnectionIndex type, redundant Merge elimination, DESCRIBE RETURN prefix) are all legitimate. Plus this PR fixes the settingsSection grammar gap I flagged on PR #171. But: scope creep and a couple of rough edges.

What I like

• ConnectionIndex int32 fix is right (parser_microflow.go:900, writer_microflow.go:919). Writer was emitting int64 while parser type-asserted int32 — every read-modify-write silently dropped to 0 (Top anchor). Parser now uses existing extractInt (handles int32/int64/int/float64), writer emits int32 to match Studio Pro. Backward-read-compatible with any files the old writer produced.
• needMerge logic is correctly derived: bothReturn → 0 inputs, no merge, terminate; hasElseBody → merge iff neither returns; no-else → merge iff THEN continues. Matches the case analysis cleanly.
• nextFlowCase dispatch (cmd_microflows_builder_control.go:236-244) correctly labels the deferred split edge for all three no-merge cases (else+thenReturns, else+elseReturns, no-else).
• Grammar fix for MODEL: adding | MODEL to settingsSection fixes the pre-existing 14-project-settings-examples.mdl failure that was breaking main's CI. Bonus value.

Concerns

1. Scope creep — undocumented deletions in keyword-as-identifier.mdl. The PR removes Owner, Type, Object from both the enumeration and the entity in the test, without any mention in the PR description. That's a test specifically designed to assert these words work as identifiers. If they don't, the fix should be in the grammar/reserved-word handling; if they truly can't, the removal deserves a paragraph explaining which reserved-word list changed and why. Silently narrowing a test hides a regression. Either split this out or explain it.

2. !strings.ContainsAny(returnVal, "+'\"()") heuristic is incomplete (cmd_microflows_format_action.go:~28). It misses common operators and punctuation: -, *, /, <, >, =, !, ,, [, ], space. So RETURN count - 1 becomes RETURN $count - 1, RETURN var1 = var2 becomes RETURN $var1 = var2. Either widen the char set or invert: only prefix when the whole value matches ^[A-Za-z_][A-Za-z0-9_]*$ (a single bare identifier).

3. Dead if/else in position calc (cmd_microflows_builder_control.go:~229):

   if !hasElseBody {
       fb.posX = max(afterSplit, afterBranch)
   } else {
       fb.posX = max(afterSplit, afterBranch)
   }

   Both branches identical — collapse to one line. Suggests the author intended different positions originally; either implement the divergence or drop the if.

4. Test coverage is manual-only. PR cites one microflow (Tool_SendOrderToSupplier), one guard chain, one Studio Pro layout check. No automated tests for: ConnectionIndex roundtrip, guard-pattern DESCRIBE, RETURN-prefix heuristic, needMerge branch matrix. This is exactly the kind of fix that should have a regression test alongside it — these bugs were silent before, they'll be silent again if reintroduced.

5. Merge conflict (DIRTY) — needs rebase before merge.

Verdict

Approve after (1) is clarified/split, (2) fixed, (3) cleaned up. (4) and (5) before merge.