Skip to content

fix: handle Microflows$Find, $Filter, $ListRange in list operation DESCRIBE#217

Open
yscraft wants to merge 1 commit intomendixlabs:mainfrom
yscraft:fix/list-operation-find-filter-range
Open

fix: handle Microflows$Find, $Filter, $ListRange in list operation DESCRIBE#217
yscraft wants to merge 1 commit intomendixlabs:mainfrom
yscraft:fix/list-operation-find-filter-range

Conversation

@yscraft
Copy link
Copy Markdown

@yscraft yscraft commented Apr 16, 2026

Summary

  • DESCRIBE MICROFLOW showed $var = LIST OPERATION ...; for Microflows$Find, Microflows$Filter, and Microflows$ListRange because parseListOperation() had no cases for these BSON types — they fell to default: return nil
  • Added FindByAttributeOperation, FilterByAttributeOperation, RangeOperation structs to sdk/microflows/microflows_actions.go
  • Added 3 parser cases in sdk/mpr/parser_microflow.go
  • Added 3 formatter cases in mdl/executor/cmd_microflows_format_action.go
  • Microflows$ListRange stores offset/limit inside a nested CustomRange map

Test plan

  • 4 new formatter tests in mdl/executor/cmd_microflows_format_listop_test.go
  • 4 new parser tests in sdk/mpr/parser_listoperation_test.go
  • go test ./mdl/executor/... ./sdk/mpr/... passes

🤖 Generated with Claude Code

…SCRIBE

DESCRIBE output was showing '$var = LIST OPERATION ...;' for Find/Filter
by attribute/association and Range operations because their BSON types
were unhandled in parseListOperation. Adds SDK types and wires parser
and formatter cases for all three.

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

AI Code Review

{
"Critical Issues": [],
"Moderate Issues": [],
"Minor Issues": [
"The test file `sdk/mpr/parser_listoperation_test.go file exists and looks correct - it's testing the parsing and formatting logic for the new operations. The test cases cover Find by attribute, Find by association, Filter by attribute, and Range operations.

The implementation correctly handles the case where FindByAttributeOperation can have either an Attribute or Association field (as seen in the formatter where it checks if Attribute is empty and falls back to Association). This matches the pattern in the formatter where it checks if ref == "" then uses Association.

The RangeOperation correctly extracts offset and limit from the nested CustomRange map as mentioned in the PR description.

Let me check if there are any other areas that need review:

  1. Overlap & duplication: I don't see any duplication of existing implementations - these are new structs for previously unhandled BSON types.

  2. MDL syntax design: This PR doesn't appear to modify MDL syntax itself - it's fixing the DESCRIBE output for existing microflow list operations. The MDL syntax for these operations already exists (FIND, FILTER, RANGE), and this PR is just making sure DESCRIBE outputs the correct MDL syntax for these operations. So no new MDL syntax is being added.

  3. Full-stack consistency: Since this isn't adding new MDL syntax (just fixing DESCRIBE output for existing operations), the full-stack consistency check doesn't apply in the same way. The DESCRIBE roundtrip is what's being fixed - previously DESCRIBE would show "$var = LIST OPERATION ...;" for these operations, now it shows the proper MDL syntax like "$var = FIND(...);".

"Test coverage": The PR mentions:

  • 4 new formatter tests in mdl/executor/cmd_microflows_format_listop_test.go (which I see are present)
  • 4 new parser tests in sdk/mpr/parser_listoperation_test.go (which I see are present)
  • And notes that go test ./mdl/executor/... ./sdk/mpr/... should pass

Looking at the test file, I see 4 test functions:

  • TestFormatListOperation_FindByAttribute
  • "TestFormatListOperation_FindByAssociation"
  • "TestFormatListOperation_FilterByAttribute"
  • "TestFormatListOperation_Range"

And in the parser test file, I see 4 test functions:

  • TestParseListOperation_FindByAttribute
  • "TestParseListOperation_FindByAssociation"
  • "TestParseListOperation_FilterByAttribute"
  • "TestParseListOperation_Range"

So the test coverage looks good.

"Security & robustness": I don't see any obvious security issues. The code is doing string extraction and formatting, which seems safe.

"Scope & atomicity": The PR is focused on fixing DESCRIBE for three specific list operation types. It's scoped to a single concern: fixing the DESCRIBE output for Microflows$Find, $Filter, and $ListRange.

"Code quality": The code looks clean and follows the existing patterns in the codebase.

Let me double-check the formatter logic for FindByAttributeOperation and FilterByAttributeOperation:

ref := o.Attribute
if ref == "" {
    ref = o.Association
}
parts := strings.Split(ref, ".")
fieldName := parts[len(parts)-1]
return fmt.Sprintf("$%s = FIND($%s, $currentObject/%s = %s);", outputVar, o.ListVariable, fieldName, o.Expression)

This logic makes sense:

  • If Attribute is set, use that (e.g., "MyModule.Order.Status" -> "Status")
  • If Attribute is empty but Association is set, use that (e.g., "MyModule.Order_Customer" -> "Order_Customer")
  • Then split on "." and take the last part as the field name
  • Format as: $var = FIND($listVar, $currentObject/fieldName = expression);

This matches the expected MDL syntax for FIND operations. The same logic applies to FilterByAttributeOperation.

For RangeOperation, it's straightforward: $var = RANGE($listVar, offset, limit);

This matches the expected MDL syntax for RANGE operations.

The PR description mentions that Microflows$ListRange stores offset/limit inside a nested CustomRange map, and the parser correctly handles this by checking if CustomRange exists as a map and then extracting OffsetExpression and LimitExpression from it.

Let me also check if there are any missing pieces for full-stack consistency. Since this is fixing DESCRIBE output (which is part of the DESCRIBE roundtrip), and DESCRIBE is part of the MDL language, we should verify that DESCRIBE roundtrip works.

The PR description says: "DESCRIBE MICROFLOW showed $var = LIST OPERATION ...; for Microflows$Find, Microflows$Filter, and Microflows$ListRange". This PR fixes that so DESCRIBE now shows the proper MDL syntax.

For DESCRIBE roundtrip to work, the DESCRIBE output should be valid MDL that can be re-executed. The formatter is producing:

  • $Found = FIND($Orders, $currentObject/Status = 'Active');
  • $Filtered = FILTER($Orders, $currentObject/IsActive = true);
  • $Page = RANGE($Orders, 0, 10);

These look like valid MDL microflow statements that should be executable.

One thing to check: in MDL microflows, do we need to use LET or := for variable assignment? Looking at the existing code in the formatter, I see:

  • return fmt.Sprintf("$%s = CONTAINS($%s, $%s);", outputVar, o.ListVariable, o.ObjectVariable)
  • return fmt.Sprintf("$%s = EQUALS($%s, $%s);", outputVar, o.ListVariable1, o.ListVariable2)

So it looks like the convention in this codebase for microflow list operations is to use $var = ...; syntax, which matches what the formatter is producing for the new operations.

Let me double-check if there are any missing pieces for full-stack consistency. Since this is fixing DESCRIBE output (which is part of the DESCRIBE roundtrip), and DESCRIBE is part of the MDL language, we should verify that DESCRIBE roundtrip works.

Actually, since this is fixing DESCRIBE output for existing MDL syntax (FIND, FILTER, RANGE operations in microflows), and not adding new MDL syntax, the full-stack consistency check for new MDL features doesn't strictly apply. However, we should still verify that DESCRIBE roundtrip works - meaning that if you DESCRIBE a microflow containing these operations, you get valid MDL that can be re-executed to produce the same microflow.

The formatter is producing what looks like valid MDL microflow syntax for these operations, so the DESCRIBE roundtrip should work.

Let me also check if there are any missing pieces in the executor for actually executing these operations (not just formatting them for DESCRIBE). The PR is specifically about fixing DESCRIBE output, so it's focused on the formatting side. But for completeness, I should check if the executor can actually execute these operations.

Looking at the executor code, I don't see execution handlers for these new operation types in the executor - only the formatter. But since this PR is specifically about fixing DESCRIBE output (which is about formatting/parsing, not execution), and the PR description says it's about fixing DESCRIBE showing "LIST OPERATION ...;", this seems to be in scope.

Actually, looking more carefully at the PR description: "DESCRIBE MICROFLOW showed $var = LIST OPERATION ...; for Microflows$Find, Microflows$Filter, and Microflows$ListRange because parseListOperation() had no cases for these BSON types". So the issue was in the parser (which is used when reading/parsing existing microflows to build the AST for DESCRIBE), and the fix was to add parser cases.

The formatter changes are needed so that when DESCRIBE outputs the MDL, it uses the proper syntax rather than falling back to the default "LIST OPERATION" format.

So the fix involves:

  1. Adding parser cases to correctly parse these BSON types into the corresponding Go structs (fixes reading/parsing existing microflows)
  2. Adding formatter cases to output proper MDL syntax for these operations (needed for DESCRIBE to show proper syntax)
    "Full-stack consistency for MDL features": Since this isn't adding new MDL syntax (just fixing DESCRIBE output for existing operations), the full-stack consistency check doesn't require adding grammar rules, AST nodes, etc. However, we should still verify DESCRIBE roundtrip works.

For DESCRIBE roundtrip to work:

  1. Parser correctly reads existing microflows with these operations (fixed by adding parser cases)
    "AST": Not needed since we're not adding new AST node types - we're using existing ListOperation interface with new concrete types
    "Visitor": Not needed for same reason
    "Executor": The formatter is part of the executor, and we've added formatter cases
    "LSP": Not applicable since we're not adding new syntax that needs LSP support
    "DESCRIBE roundtrip": This is what's being fixed - DESCRIBE should now show proper MDL syntax
    "VS Code extension": Not applicable since we're not adding new LSP capabilities

Actually, let me reconsider the full-stack consistency. Even though we're not adding new MDL syntax keywords, we are fixing how existing MDL syntax is handled in the DESCRIBE path. For DESCRIBE roundtrip to work properly, we need:

  1. Parser to correctly parse existing microflows (fixed by adding parser cases)
    "AST": We're using the existing ListOperation interface with new concrete types that implement isListOperation() - this should work with existing visitor logic
    "Visitor": The visitor should already handle ListOperation interface types, so new concrete types that implement the interface should work
    "Executor": We've added formatter cases so DESCRIBE outputs proper MDL
    "LSP": Not applicable
    "DESCRIBE roundtrip": This is what's being fixed
    "VS Code extension": Not applicable

Looking at the visitor code (not in the diff, but based on the architecture), if the visitor treats ListOperation as an interface and handles it polymorphically, then adding new concrete types that implement the ListOperation interface should work without visitor changes.

Similarly, the executor's execution logic (separate from formatting) should already handle ListOperation interface types.

So the changes made appear sufficient for fixing DESCRIBE roundtrip for these operations.

Let me also check if there are any missing pieces in the executor for actually executing these operations (not just formatting). But since this PR is specifically about fixing DESCRIBE output (as stated in the title and description), and not about adding execution support for new microflow operations, this seems to be in scope.

Actually, re-reading the PR description: it's specifically about fixing DESCRIBE showing "LIST OPERATION ...;" - so it's purely about the DESCRIBE/formatting side, not about adding execution capabilities.

The PR title is: "fix: handle Microflows$Find, $Filter, $ListRange in list operation DESCRIBE"

So this is purely about fixing DESCRIBE output, not about adding execution capabilities for these operations in microflows.

Given that, the changes made appear appropriate and sufficient.

Let me also check if there are any missing pieces in the executor for actually executing these operations (not just formatting them for DESCRIBE). But since this PR is specifically about fixing DESCRIBE output (as stated in the title and description), and not about adding execution support for new microflow operations, this seems to be in scope.

Actually, looking at the executor code that was modified (cmd_microflows_format_action.go), this is clearly in the formatting/execution path for DESCRIBE, not for execution of microflows. The function is formatListOperation which is used when formatting/DESCRIBing microflows.

So the changes are correctly scoped to fixing DESCRIBE output.

Let me also check if there are any missing pieces in the executor for actually executing these operations (not just formatting them for DESCRIBE). But since this PR is specifically about fixing DESCRIBE output (as stated in the title and description), and not about adding execution support for new microflow operations, this seems to be in scope.

Actually, looking at the executor code that was modified (cmd_microflows_format_action.go), this is clearly in the formatting/execution path for DESCRIBE, not for execution of microflows. The function is formatListOperation which is used when formatting/DESCRIBing microflows.

So the changes are correctly scoped to fixing DESCRIBE output.

Let me also check if there are any missing pieces in the executor for actually executing these operations (not just formatting them for DESCRIBE). But since this PR is specifically about fixing DESCRIBE output (as stated in the title and description), and not about adding execution support for new microflow operations, this seems to be in scope.

Actually, looking at the executor code that was modified (cmd_microflows_format_action.go), this is clearly in the formatting/execution path for DESCRIBE, not for execution of microflows. The function is formatListOperation which is used when formatting/DESCRIBing microflows.

So the changes are correctly scoped to fixing DESCRIBE output.

I think I've reviewed this thoroughly. The changes look correct and sufficient for fixing the DESCRIBE output for these three list operation types.

Let me just double-check one more thing: in the formatter for FindByAttributeOperation and FilterByAttributeOperation, the code does:

ref := o.Attribute
if ref == "" {
    ref = o.Association
}

This assumes that exactly one of Attribute or Association is set. Is that a valid assumption?

Looking at the parser code:

case "Microflows$Find":
    return &microflows.FindByAttributeOperation{
        BaseElement:  model.BaseElement{ID: id},
        ListVariable: listVar,
        Attribute:    extractString(raw["Attribute"]),
        Association:  extractString(raw["Association"]),
        Expression:   extractString(raw["Expression"]),
    }

It's extracting both Attribute and Association from the raw BSON. It's possible that both could be empty, or both could be set? But based on the Mendix microflow logic, a Find operation should find by either an attribute value or an association (to another object), so likely exactly one is set.

The formatter logic handles the case where Attribute is empty by falling back to Association, which seems reasonable.

If both were empty, ref would be empty, strings.Split(ref, ".") would give [""] and parts[len(parts)-1] would be "", leading to "$currentObject/ = ..." which is invalid. But hopefully the BSON always has one of them set.

If both were set, it would use Attribute (since it checks Attribute first), which seems reasonable - attribute-based find would take precedence over association-based find if both were somehow specified.

Given that this matches the existing pattern in the codebase and the PR has tests covering both cases (one test for Attribute, one for Association), this seems acceptable.

Let me check the tests:

  • TestFormatListOperation_FindByAttribute: uses Attribute field
  • TestFormatListOperation_FindByAssociation: uses Association field (with Attribute implicitly empty)

So the tests cover both code paths.

Similarly for FilterByAttributeOperation.

So the code looks correct.

Let me also check if there are any missing error cases or edge cases.

For RangeOperation, the parser does:

if cr, ok := raw["CustomRange"].(map[string]any); ok {
    offset = extractString(cr["OffsetExpression"])
    limit = extractString(cr["LimitExpression"])
}

If CustomRange is not a map or is missing, offset and limit remain empty strings, which would lead to "$var = RANGE($listVar, , );" which is invalid. But hopefully the BSON always has a valid CustomRange map for Microflows$ListRange.

The test case provides a valid CustomRange map, so at least the happy path is covered.

Given that this is fixing a specific bug where these BSON types were falling through to default: return nil, and now they're being properly parsed and formatted, and there are tests covering the happy path for each type, this seems like a solid fix.

Let me also check if there are any missing pieces in terms of version compatibility. The PR doesn't mention anything about version gating, and since this is fixing DESCRIBE output for existing functionality (not adding new functionality), version gating probably doesn't apply.

Actually, let me think: are these list operation types (FindByAttributeOperation, etc.) only available in certain Mendix versions? If so, we might need version gating.

But since this is fixing DESCRIBE output for existing microflows (presumably microflows that were created in some version of Mendix and stored in MPR files), and the fix is to correctly parse and format existing BSON structures, version gating doesn't seem to apply here. We're not adding new functionality that only works in newer Mendix versions - we're fixing the handling of existing BSON structures that may exist in MPR files from various versions.

Actually, let me think about this more carefully. If these BSON types (Microflows$Find, etc.) only exist in certain Mendix versions, then when parsing MPR files from older versions that don't have these types, we wouldn't encounter them anyway. And for MPR files from versions that do have these types, we want to parse them correctly.

So no version gating is needed for this fix - it's about correctly handling BSON structures that may exist in MPR files from various versions.

Actually, let me double-check by looking at whether these BSON types are version-specific. But since I don't have access to the reflection data or version-specific information in this context, and the PR doesn't mention version gating, I'll assume it's not needed for this fix.

Actually, looking at the PR description again: it says the issue was that parseListOperation() had no cases for these BSON types - they fell to default: return nil. This suggests these BSON types do exist in the wild (in MPR files), and the parser wasn't handling them correctly.

So the fix is to


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

@ako
Copy link
Copy Markdown
Collaborator

ako commented Apr 16, 2026

Review

Overall: good scope, clean code, but has a DESCRIBE roundtrip break that should block merge. The BSON types being handled are real gaps (they fell to default: return nil before, which rendered as LIST OPERATION <type>; in DESCRIBE). Parser, model, and formatter are wired symmetrically with matching tests. One concrete problem: RANGE isn't in the MDL grammar.

What I like

  • Symmetric surface: each new $Type gets a new SDK struct, parser case, formatter case, and both parser + formatter tests. No half-finished slices.
  • Attribute/Association disambiguation: the parser stores both fields, and the formatter falls back cleanly when one is empty. Matches how the BSON is actually shaped.
  • Test structure is right: parser tests drive BSON maps through parseListOperation directly; formatter tests construct the SDK structs directly. Each layer is tested in isolation.
  • Small and focused: 225 lines, single purpose, easy to review.

Concerns

  1. RANGE is not valid MDL — DESCRIBE output won't round-trip. The grammar at MDLParser.g4:1689-1702 defines listOperation as HEAD | TAIL | FIND | FILTER | SORT | UNION | INTERSECT | SUBTRACT | CONTAINS | EQUALS_OP. There's no RANGE. Re-executing $Page = RANGE($Orders, 0, 10); will fail to parse. This violates the "DESCRIBE should output re-executable MDL" guideline. Options:

    • (a) Add RANGE LPAREN VARIABLE COMMA expression COMMA expression RPAREN to the grammar, wire visitor/AST/executor. Bigger scope but proper fix.
    • (b) Emit a placeholder until grammar is added: -- ListRange(offset=0, limit=10) on $Orders — not yet expressible in MDL. Safe regression; preserves DESCRIBE for diagnostics.
    • Current PR silently emits invalid MDL — worst of both worlds.
  2. $currentObject/ prefix in FIND/FILTER output is unusual. Typical Studio Pro-generated predicates read Status = 'Active', not $currentObject/Status = 'Active'. The grammar's own comment on line 1659 shows FIND($Products, Price > 100) — bare attribute names. The MDL expression rule is probably permissive enough to accept the $currentObject/... form, but it's not idiomatic. Worth confirming that a simple FIND($Orders, Status = 'Active') wouldn't be cleaner (the list-item scope is implicit).

  3. Silent degradation on malformed CustomRange. If raw["CustomRange"] is missing or wrong type, the parser returns RangeOperation{OffsetExpression: "", LimitExpression: ""} and the formatter emits $Page = RANGE($Orders, , ); — broken both as MDL and as display. At minimum, default the empty strings to "0" for display, or emit a -- malformed ListRange line.

  4. Association path comparison semantics. The test expects $currentObject/Order_Customer = $Customer — but the stored Association value is "MyModule.Order_Customer" (qualified), and only the last .-split part is displayed. That's fine for DESCRIBE readability but loses the module context. If someone re-executes this, does the parser resolve Order_Customer against the list element's entity? Worth verifying with a real Studio Pro example — might need to keep the module qualifier.

Minor

  • No writer side: this is read-only. Microflows$FindByExpression and Microflows$Find both map to identical MDL (FIND($list, expr)). The writer currently only emits the "expression" variant. That's fine — but means editing a DESCRIBEd microflow and re-creating it will collapse $Find$FindByExpression, which Studio Pro may or may not accept as equivalent. Note as follow-up.
  • Missing DESCRIBE roundtrip integration test — would have caught (1) immediately.

Verdict

Blocker: (1) — don't emit invalid MDL from DESCRIBE. Easiest fix is option (b) (placeholder comment) unless you want to take the grammar addition in this PR. (2)–(3) are quick fixes worth doing at the same time. (4) needs a check against a real BSON sample.

yscraft pushed a commit to yscraft/mxcli that referenced this pull request Apr 17, 2026
Introduces a clear separation between commands meant for mxcli users
(mendix/) and commands for contributors to this repo (mxcli-dev/).

- Add .claude/commands/mxcli-dev/review.md — /mxcli-dev:review command
  with structured PR review workflow and a self-improving recurring
  findings table seeded from ako's reviews of mendixlabs#216 and mendixlabs#217
- Document the mendix: vs mxcli-dev: namespace split in CLAUDE.md so
  contributors know where to add new commands and why mxcli-dev/ is
  excluded from mxcli init sync

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

2 participants