Microflow/nanoflow create or modify as diff-then-patch: an unchanged definition writes nothing (#747)

[ako]

Closes #747. Part of #714 (plan item 4.2g, ADR-0012 decision 3).

What

create or modify microflow|nanoflow on an existing flow is now diff-then-patch on top of the #739 splice engine, not a whole-document rebuild:

1. The stored flow is described and the description parsed back, under the script's language version. That is the one rendering of a stored flow MDL already guarantees to re-parse, so both sides end up in the same AST and "the same definition" is a structural comparison. There is no second renderer (the mxcli diff reports false deletions (java-action calls, download, grant, geometry) on an UNMODIFIED describe dump — and leaks Go struct pointers into retrieve constraints mendixlabs/mxcli#997 lesson).
2. The header is compared with the rules the rebuild already used for "an absent clause keeps what is stored" (documentation, @excluded, @applyentityaccess, expose, URL, export level, concurrency).
3. Statements are matched by a longest common subsequence using declaredMatches. It compares whole statements, so signature and output variable both count. Each run of unmatched statements becomes an insert, replace or drop, aimed at the stored activity by the @position describe printed for it. An if that differs only inside its branches is diffed branch by branch, because branch activities are top-level graph nodes.
4. The operations go through alterFlowContext.apply (extracted from alter microflow in the first commit), so a derived patch gets the same fragment build, scope checks and splice as a hand-written alter.
5. An empty patch writes nothing and reports Unchanged microflow: …. A folder-only difference is a move.

The fallback (the choice the issue asked to record)

The splice cannot express some changes:

• a header change;
• a change inside a loop body or an error handler;
• a moved node;
• a change of annotations on a replaced or dropped activity;
• anything else the splice refuses.

For these:

• Under mdl 1; it is refused with the reason, and nothing is written. The message points to alter for activity changes, or to drop + create for a deliberate rebuild.
• Under mdl 0 the existing rebuild still runs, with a new warning Warning [MDL-V1-REBUILD]: … (a langver.Change, ADR-0011: a new refusal applies only under the header that opts into it).

I did not keep the rebuild under mdl 1 anywhere. The round-trip harness shows it lossless on none of the 29 PedApp flows (all 29 were on the #721 class A allowlist).

Design choices the ADRs did not settle

• Geometry the script leaves out is not a difference. A statement without @position/@curve/@anchor/@merge/@start matches the stored one wherever it is drawn (R1: change only what differs). Geometry that is stated must equal the stored geometry. A statement that differs only in geometry is refused as "moved", not replaced, because the splice places only new nodes and a silent non-move would be a meaningless form (R11). Captions, colours and notes are compared exactly.
• Where new nodes go. New statements are placed by the splice, and any @position they carry is ignored. describe afterwards shows where they actually went.
• Annotations on a replaced activity. The splice keeps the stored activity's notes and re-attaches them to the replacement. The declared statement must carry the same notes, which are then taken off it so the builder does not draw them twice (shared notes such as @annotation(id: n1) included). Free annotations are compared at flow level and kept out of statement matching.
• Drops run after inserts and replaces, so a stored activity whose output only a replaced statement read can be dropped. The alter scope check now skips activities an earlier operation removed (removedIDs).
• mdl 0 cannot state a list-of-one retrieve. describe prints a Custom range limit 1 as limit 1, which mdl 0 reads as the object range. Parsed as is, a headerless limit 1 would look unchanged against a stored list. correctAmbiguousRanges sets those parsed statements back to the list they stand for, keyed on output variable and @position. Describing under mdl 1 instead was tried and rejected: describe does not emit mdl 1 string escapes yet, so VAL_Feedback's '…\n' strings re-parse differently.
• MDL-V1-REBUILD is gated in the executor, not the visitor. Whether a statement needs the fallback depends on what is stored, so it is not in visitor.LanguageChanges() or the fmt --upgrade registry. A script upgraded to mdl 1; can meet the refusal at run time.

Allowlists shrunk

• mdl/roundtrip/allowlist_test.go: all 16 microflow and 13 nanoflow Round-trip harness: untracked describe → exec losses on the Studio Pro fixture (microflows, entities, pages, snippets, menus, roles, JS actions) #721 class A entries struck. 29 of 29 flows now keep GetPut and PutGet.
• mdl/executor/studiopro_roundtrip_test.go (studioProKnownLossy): the two nanoflow entries (ACT_Feedback_UploadImage, SUB_Feedback_GetOrCreate) now pass and are struck.

Test plan

Run locally in the worktree:

• make build, make lint
• go test ./mdl/executor/ ./mdl/backend/... ./mdl/upgrade/ ./mdl/visitor/ ./cmd/mxcli/testrunner/ ./cmd/mxcli/theme/
• go test -tags integration ./mdl/roundtrip/ (full package: PedApp harness plus new tests)
• go test -tags integration ./mdl/executor/ (full package) and ./mdl/backend/modelsdk/

New tests, all on Studio Pro-authored PedApp flows (mdl/roundtrip/flow_modify_test.go):

• UnchangedIsByteIdentical: VAL_Feedback describe → exec under mdl 0 and mdl 1. Unit bytes are identical, nothing is written, and the output says "Unchanged".
• InsertIsSpliced (the edited-flow control): one inserted statement is written. Every stored element keeps its $ID and type, all 10 merges survive (the rebuild left 5), and re-executing the new describe writes nothing.
• ReplaceKeepsSharedNote: a replace on PopulateUserAttributes keeps the one shared note on both activities. Only the replaced activity's subtree leaves.
• DropAndReplace: a drop plus a replace on SUB_Feedback_Sanitize under mdl 1.
• BranchEditIsSpliced: a message changed inside an if branch of VAL_Feedback. Only that activity is replaced and the merges are kept.
• UnspliceableChange: a moved node is refused under mdl 1 with nothing written. Under mdl 0 it is rebuilt with the MDL-V1-REBUILD warning.
• Unit: TestDeclaredMatches_OmittedGeometryIsNotADifference, TestLCSStatements_PairsTheUnchangedRuns.

Revert checks. Each change was reverted or stubbed, and the listed tests failed with the expected symptom:

• In-place path disabled (return false, nil): Unchanged (unit rewritten), Insert (merges 10 → 5, $IDs gone), Replace (parameter, end event and note $IDs gone) and Unspliceable (no refusal under mdl 1) all fail.
• Geometry wildcard off: the "no geometry at all" unit case fails.
• Note stripping off: the Replace test fails (the fragment with the duplicated note has no room, so it falls back to the rebuild).
• correctAmbiguousRanges off: TestPedAppRoundTrip_RetrieveRange fails (a headerless limit 1 leaves the stored list).
• If-branch recursion off: BranchEditIsSpliced is refused under mdl 1.
• Move refusal off: UnspliceableChange gets no refusal.
• removedIDs skip off, or drops not ordered last: DropAndReplace is refused with "$SanitizedPageName is still used by…".

Studio Pro verification: MCP tunnel to TestApp, Mendix 11.14.0 (--mcp http://localhost/mcp --mcp-dial host.docker.internal:7792):

• Unchanged describe → create or modify of Administration.ChangeMyPassword and Microflows.SplitMerge: Unchanged microflow: …. --mcp-trace shows no PED calls.
• Inserted log statement before the return of Microflows.SplitMerge (a merge-only, Studio Pro-drawn flow): Modified microflow: Microflows.SplitMerge (spliced: 1 inserted). It was sent as one ped_update_document, and the backend's ped_check_errors on the document came back clean. A rerun is refused by the live-document guard (7 objects live, 6 stored), which confirms the insert landed and nothing else changed count. The insert is still in the open, unsaved Studio Pro session.
• Replace and drop are not available over MCP yet (the MCP mutator refuses them), so those were verified on PedApp only.

Follow-ups (not in this PR)

• Splice inside loop bodies (needs mfmutator loop-relative coordinates) and inside error-handler bodies.
• Patch header changes (documentation, properties, parameters) in place instead of falling back.
• Honour or refuse an explicit @position on inserted statements, instead of ignoring it.
• describe under mdl 1 should emit mdl 1 string escapes. Then the stored side could always be read under the latest version, and correctAmbiguousRanges could go.

🤖 Generated with Claude Code

[ako]

Independent review (#747)

I pushed two fixes to the branch, each as its own commit. Each has a test that I watched fail before the fix went in.

1. e521e24: under mdl 1, a string change could report "Unchanged". The stored side was described (in mdl 0) and then re-parsed under the script's header. VAL_Feedback stores a line break, which describe prints as \n. Under mdl 1 that reads as a backslash followed by an n. A script that says exactly that then matched, and nothing was written. A fresh create of the same text stores the backslash, so this was a silent no-op of a stated value.
   • Fix: always describe and parse the stored side as mdl 0. The AST holds values, so it compares with a declared side parsed under any version. correctAmbiguousRanges now always applies.
   • Tests: new TestFlowModify_MDL1ReadsStoredStringsAsStored, which runs an mdl 0 control first. Three tests had relied on the misreading on VAL_Feedback: the mdl 1 legs of UnchangedIsByteIdentical, BranchEditIsSpliced and UnspliceableChange. Those legs now run on SUB_Feedback_SendToServer, which has merges and an error handler but no backslash. mdl 1 has no spelling for VAL_Feedback's line break, and fmt --upgrade --header refuses it too.
   • TestPedAppListActivitiesUnderMdl1 now also asserts that the mdl 1 description of its list-activity flow reports Unchanged. This shows that list operations described in call form under mdl 0 compare equal to the statement form.
2. c6b1f0e: a change inside a loop body replaced the whole loop. The PR text, the file comment and both skills say such a change is refused under mdl 1. In fact the splice replaced the loop: every inner node was rebuilt with new IDs. It is now refused under mdl 1 and falls back to the warned rebuild under mdl 0. A changed loop header is still a replace.
   • Test: TestFlowModify_LoopBodyChangeIsNotSpliced, with a control that a change after the loop is still spliced.
   • Checked over the MCP tunnel on TestApp ZzMxcliProbe_LoopSplit: unchanged under mdl 1 reports Unchanged, and a loop-body edit is refused. No PED calls were made in either case.

Probed without finding a defect. Each case was run on a PedApp copy. I checked it with mx check against a baseline (no new errors) and with a geometry-stripped describe diff (the intended result each time).

• SUB_Feedback_SendToServer:
  • replace inside an else branch
  • insert after a call that has an error handler joining a shared merge
  • insert after merge rejoin1
  • insert between two ifs
  • insert first in a then-branch
  • an error-handler body change, which is refused by the engine and rebuilt with a warning under mdl 0
• VAL_Feedback:
  • a nested if condition change, which is refused
  • drop one or both statements of a branch
  • insert before join
  • replace return, which is refused
  • replace declare
  • drop a whole if, which is refused
  • insert in unreachable position, which is rebuilt, as before this PR
• Nanoflow ACT_Feedback_UploadImage:
  • drop in a 4-deep branch
  • insert
  • replace of the annotated JS call (note kept)
  • message change

Ran:

• make build and make lint
• go test ./mdl/executor/ ./mdl/backend/...
• go test -tags integration ./mdl/roundtrip/ (full package, allowlist included)
• go test -tags integration ./mdl/executor/

Remaining (not blocking):

• mdl 1 cannot state VAL_Feedback's stored line break, so under mdl 1 that flow can never read as unchanged. This follows from the escape rule, not from the splice.
• The Studio Pro leftover the implementer reported in TestApp Microflows.SplitMerge is still there.

🤖 Generated with Claude Code