Skip to content

fix: refresh empty change-object actions#340

Open
hjotha wants to merge 2 commits intomendixlabs:mainfrom
hjotha:submit/builder-empty-change-refresh-in-client
Open

fix: refresh empty change-object actions#340
hjotha wants to merge 2 commits intomendixlabs:mainfrom
hjotha:submit/builder-empty-change-refresh-in-client

Conversation

@hjotha
Copy link
Copy Markdown
Contributor

@hjotha hjotha commented Apr 27, 2026

Summary

  • sets RefreshInClient when rebuilding change $Object; with no member assignments
  • keeps explicit member-change actions unchanged
  • adds a builder regression test for the empty change-object case

Why

An empty, non-committing ChangeObjectAction without refresh is invalid in Studio Pro and reports CE0032. Existing models can use empty change activities as refresh-only actions, and the MDL form currently has no explicit refresh modifier for change statements, so the builder needs to preserve validity by inferring refresh for empty changes.

Validation

  • make build
  • make lint-go
  • make test

Closes #339.
Related to #332.

Symptom: executing MDL that describes an empty change-object activity could create a model that fails Studio Pro validation with CE0032 because the action had no items and did not commit the object.

Root cause: the MDL syntax represents an empty change action as `change $Object;`, but the builder always wrote RefreshInClient=false for change-object actions. For an empty non-committing change, Studio Pro requires the refresh flag to make the activity valid.

Fix: set RefreshInClient=true when building a change-object action with no member changes. Change actions with explicit member assignments keep their previous refresh behavior.

Tests: add a builder regression test that constructs an empty change statement and asserts that the resulting ChangeObjectAction refreshes in client.
Symptom: the empty change-object fix relies on RefreshInClient=true even though the CE0032 text mentions only missing items and commit state.

Root cause: the Studio Pro validator accepts refresh-only empty changes, but that behavior is not obvious from public docs or the CE0032 wording.

Fix: add an executable bug-test fixture for issue mendixlabs#339 and document the mx check behavior directly beside the builder inference.

Tests: `./bin/mxcli check mdl-examples/bug-tests/339-empty-change-refresh-in-client.mdl` and `go test ./mdl/executor -run TestEmptyChangeObjectRefreshesInClient`.
@hjotha
Copy link
Copy Markdown
Contributor Author

hjotha commented Apr 27, 2026

Addressed the review follow-up in 8d5cc94:

  • added mdl-examples/bug-tests/339-empty-change-refresh-in-client.mdl documenting the mx check behavior for the CE0032 case
  • added an inline builder comment explaining why RefreshInClient=true is valid for an empty non-committing change action even though the CE0032 text only mentions items/commit
  • rechecked the fixture with ./bin/mxcli check mdl-examples/bug-tests/339-empty-change-refresh-in-client.mdl
  • reran go test ./mdl/executor -run TestEmptyChangeObjectRefreshesInClient

I am keeping the separate concern of preserving explicit RefreshInClient on change actions with member assignments out of this PR; that needs a syntax/design decision for a change ... refresh modifier rather than being bundled into the CE0032 fix.

@github-actions
Copy link
Copy Markdown

AI Code Review

Critical Issues

None found.

Moderate Issues

None found.

Minor Issues

  • The bug test file is placed in mdl-examples/bug-tests/ directory, but this directory doesn't appear in the existing doctype test file list. While not incorrect, it would be helpful to confirm if this is the intended location for bug-specific tests (as opposed to the existing pattern of .tests.mdl files in the main mdl-examples/ directory). However, since the test runs and passes, this is minimal.

What Looks Good

  • Minimal, focused change: The PR addresses exactly one issue (empty change-object actions requiring client refresh) without modifying unrelated code.
  • Correct logic: Sets RefreshInClient: len(s.Changes) == 0 only for empty changes, preserving existing behavior for non-empty changes.
  • Comprehensive testing:
    • Adds an executable MDL bug test (mdl-examples/bug-tests/339-empty-change-refresh-in-client.mdl) that validates the fix against Studio Pro via mx check.
    • Adds a unit test (mdl/executor/bugfix_regression_test.go) that directly verifies the BSON output.
    • Both tests follow the project's validation approach (build → lint → test).
  • Clear documentation: The code change includes explanatory comments referencing the CE0032 error and Studio Pro behavior.
  • No syntax changes: Since this is a behavioral fix (not new MDL syntax), it correctly avoids unnecessary grammar/AST/visitor/LSP updates.
  • Atomic scope: All changes relate solely to the empty change-object refresh behavior - no unrelated modifications.
  • Follows existing patterns: The fix aligns with how other microflow actions handle validity constraints in the executor.

Recommendation

Approve. The PR correctly resolves the reported issue with appropriate tests and minimal, well-documented code changes. 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.

Empty change-object microflow actions can be written as invalid CE0032 activities

2 participants