From 9442043f133f857a012d251f0de24445dc465779 Mon Sep 17 00:00:00 2001 From: Ako Date: Wed, 21 Oct 2026 13:00:34 +0000 Subject: [PATCH] fix: a splice making room leaves the flow's parameters where they are MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit shift moved every top-level node past the cut, and a parameter is a top-level object of the stored object collection: one if/else inserted into NF_Do moved `$Flag` from its derived (200, 53) to (539, 53), a longer replace then to (699, 53), and from then on describe printed a parameter @position no script stated. mxcli's derived parameter row is absolute and right of the start event, so almost every insert's cut falls left of it. A parameter is not on the line the room is made on. Studio Pro authors draw it above the start event — 63 of 71 in PedApp and JTSBootLogboek sit at the start's x, none within 60px of the flow line — and an insert in Studio Pro moves nothing. shifted() now leaves parameters out, and with it pinnedPast, so a stated parameter position no longer blocks a splice it is not in the way of. Fixes ako/mxcli#1547. Co-Authored-By: Claude Opus 5.5 --- ...6-10-21-splice-shift-moves-parameters.json | 1 + ...geometry-1547-splice-leaves-parameters.mdl | 35 +++++++++++++ mdl/backend/mfmutator/splice.go | 19 ++++--- .../mfmutator/splice_parameters_test.go | 50 +++++++++++++++++++ .../flow_modify_stated_positions_1508_test.go | 9 ++++ 5 files changed, 108 insertions(+), 6 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-backend/2026-10-21-splice-shift-moves-parameters.json create mode 100644 mdl-examples/bug-tests/flow-geometry-1547-splice-leaves-parameters.mdl create mode 100644 mdl/backend/mfmutator/splice_parameters_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-backend/2026-10-21-splice-shift-moves-parameters.json b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-21-splice-shift-moves-parameters.json new file mode 100644 index 0000000000..c7daa32352 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-21-splice-shift-moves-parameters.json @@ -0,0 +1 @@ +{"area": "mdl/backend", "date": "2026-10-21", "symptom": "A splice that makes room for an inserted fragment (create or modify / alter … insert) moved the flow's parameters along with the row: one if/else inserted into NF_Do moved $Flag from its derived (200, 53) to (539, 53), a longer replace then to (699, 53), and from then on describe printed a parameter @position no script stated. Inserting before the first statement of a two-parameter microflow moved $Count from (300, 53) to (410, 53).", "cause": "mfmutator shift (splice.go) moves every top-level node past the cut, and a Microflows$MicroflowParameter is a top-level object of the unit's object collection (the decoded model keeps parameters apart, so nothing in the executor sees them as nodes). mxcli's derived parameter row is absolute (200 + idx*pitch, 53), right of the start event, so almost every insert's cut falls left of it.", "file": "mdl/backend/mfmutator/splice.go (shifted)", "insight": "Measure the authored convention before deciding what 'the row' is: 63 of 71 Studio Pro-authored parameters (PedApp 19/20, JTSBootLogboek 44/51) sit exactly at the start event's x, all above every other node, none within 60px of the flow line — a parameter belongs to the flow, not to the stretch of line past an insert, and Studio Pro moves nothing on insert. The decoded model hides this: microflows.Microflow carries Parameters apart from ObjectCollection, so a probe that type-switches the collection for *MicroflowParameter does not compile, while the BSON the mutator walks has them inline. Excluding them in shifted also keeps them out of pinnedPast, so a stated parameter position can no longer block a splice it is not in the way of.", "refs": ["ako/mxcli#1547", "ako/mxcli#1508"]} diff --git a/mdl-examples/bug-tests/flow-geometry-1547-splice-leaves-parameters.mdl b/mdl-examples/bug-tests/flow-geometry-1547-splice-leaves-parameters.mdl new file mode 100644 index 0000000000..5b6f8b2c9b --- /dev/null +++ b/mdl-examples/bug-tests/flow-geometry-1547-splice-leaves-parameters.mdl @@ -0,0 +1,35 @@ +mdl 1; +-- ============================================================================ +-- Bug: a splice making room moved the flow's parameters along with the row +-- (ako/mxcli#1547) +-- ============================================================================ +-- +-- The second statement inserts a log before the first one, so the splice +-- makes room by moving everything after the start event along. Before the fix +-- that included the two parameters, which mxcli draws on a row above the flow +-- (x 200 and 300, y 53): both moved right, and from then on describe printed a +-- @position on each that no script ever stated, the row drifting further with +-- every insert. +-- +-- Fixed: a parameter is not on the line the room is made on, so it stays +-- where it is. Studio Pro authors draw a parameter above the start event, and +-- inserting an activity in Studio Pro moves nothing. +-- +-- Verify: exec, then +-- mxcli -p app.mpr -c "describe microflow MyFirstModule.ACT_ParamRow" +-- Expected: the parameters carry no @position (they are still on the derived +-- row), and the new log is first. +-- ============================================================================ + +create or modify microflow MyFirstModule.ACT_ParamRow ($Name: String, $Count: Integer) +begin + log info node 'Probe' 'first ' + $Name; + log info node 'Probe' 'second ' + toString($Count); +end; + +create or modify microflow MyFirstModule.ACT_ParamRow ($Name: String, $Count: Integer) +begin + log info node 'Probe' 'inserted'; + log info node 'Probe' 'first ' + $Name; + log info node 'Probe' 'second ' + toString($Count); +end; diff --git a/mdl/backend/mfmutator/splice.go b/mdl/backend/mfmutator/splice.go index b28ff3e354..c0ce3e42a1 100644 --- a/mdl/backend/mfmutator/splice.go +++ b/mdl/backend/mfmutator/splice.go @@ -881,10 +881,11 @@ func (m *Mutator) markRemoved(v any) { } // shift moves every top-level node past the cut (along axis ax, in the -// direction of s) by delta. The sweep keeps everything on either side of the -// cut exactly as it was relative to its neighbours, so only the flows that -// cross the cut get longer; a flow's control vectors are relative to its ends, -// so no curve changes. skip is a node that is about to be removed. +// direction of s) by delta, but the parameters (shifted). The sweep keeps +// everything on either side of the cut exactly as it was relative to its +// neighbours, so only the flows that cross the cut get longer; a flow's +// control vectors are relative to its ends, so no curve changes. skip is a +// node that is about to be removed. func (m *Mutator) shift(g *graph, ax axis, s int, cut float64, delta int, skip string) { for _, n := range g.order { if !shifted(n, ax, s, cut, skip) { @@ -895,9 +896,15 @@ func (m *Mutator) shift(g *graph, ax axis, s int, cut float64, delta int, skip s } } -// shifted reports whether shift moves n: a top-level node past the cut. +// shifted reports whether shift moves n: a top-level node past the cut that +// is not a parameter. A parameter is not on the line the room is made on: it +// belongs to the flow as a whole, and Studio Pro authors draw it above the +// start event (63 of 71 in PedApp and JTSBootLogboek at the start's x, none +// within 60px of the line). Moved, it drifted right with every insert and +// describe printed a @position the script never stated (ako/mxcli#1547). func shifted(n *node, ax axis, s int, cut float64, skip string) bool { - return n.loop == "" && n.id != skip && float64(s)*(float64(n.pos.get(ax))-cut) > 0 + return n.loop == "" && n.id != skip && n.typ != "Microflows$MicroflowParameter" && + float64(s)*(float64(n.pos.get(ax))-cut) > 0 } // pinnedPast returns the first node shift would move that is drawn at a point diff --git a/mdl/backend/mfmutator/splice_parameters_test.go b/mdl/backend/mfmutator/splice_parameters_test.go new file mode 100644 index 0000000000..96c2b74ee4 --- /dev/null +++ b/mdl/backend/mfmutator/splice_parameters_test.go @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: Apache-2.0 + +package mfmutator + +import ( + "testing" + + "github.com/mendixlabs/mxcli/model" +) + +// A parameter is not on the line a fragment is spliced into, so making room +// does not move it (ako/mxcli#1547). shift moved every top-level node past the +// cut, parameters included: one inserted if/else moved a nanoflow's `$Flag` +// from its derived (200, 53) to (539, 53), and from then on describe printed a +// @position the script never stated, the row drifting right with every insert. +// Studio Pro authors draw a parameter above the start event — 63 of 71 in +// PedApp and JTSBootLogboek sit at the start's x, none within 60px of the line +// — and an insert in Studio Pro moves nothing. The control is the activity +// after the gap, which does move. +func TestSplice_MakingRoomLeavesTheParameters(t *testing.T) { + positions := func(m *Mutator) map[string]point { + out := map[string]point{} + for _, n := range m.graph().order { + out[n.id] = n.pos + } + return out + } + objs := append(line(), obj("param", "Microflows$MicroflowParameter", 500, 53)) + for _, c := range []struct { + name string + splice func(m *Mutator) error + }{ + {"an insert", func(m *Mutator) error { return m.InsertAfter(model.ID(uid("a")), oneActivity()) }}, + {"a replace longer than what it replaces", func(m *Mutator) error { return m.Replace(model.ID(uid("a")), twoActivities()) }}, + } { + t.Run(c.name, func(t *testing.T) { + m, _ := newMutator(t, unit(objs, lineFlows())) + if err := c.splice(m); err != nil { + t.Fatal(err) + } + got := positions(m) + if p := got[uid("b")]; p.X <= 420 { + t.Fatalf("control: b is at x=%d, want it moved along past 420 to make room", p.X) + } + if p := got[uid("param")]; p != (point{500, 53}) { + t.Errorf("the parameter moved to (%d, %d), want it left at (500, 53)", p.X, p.Y) + } + }) + } +} diff --git a/mdl/executor/flow_modify_stated_positions_1508_test.go b/mdl/executor/flow_modify_stated_positions_1508_test.go index bb4a3cf40d..9ad10bf23c 100644 --- a/mdl/executor/flow_modify_stated_positions_1508_test.go +++ b/mdl/executor/flow_modify_stated_positions_1508_test.go @@ -160,6 +160,15 @@ func TestCreateOrModify_InsertDoesNotMoveStatedNodes1508(t *testing.T) { if v.objectAt(t, -500, 200, "Microflows$ActionActivity") == "" { t.Errorf("the declare the script draws at (-500, 200) moved:\n%s", describe()) } + // The parameter is not on the line the room was made on: it stays on + // its derived row (ako/mxcli#1547), where it moved to (539, 53). + for _, id := range v.order { + if o := v.objs[id]; afGet(o, "$Type") == "Microflows$MicroflowParameter" { + if x, y := afPoint(o); x != 200 || y != 53 { + t.Errorf("$Flag moved to (%d, %d), want it left at (200, 53):\n%s", x, y, describe()) + } + } + } split, merge, branch := nf1508Branch(t, v, "$Flag") if len(branch) != 2 { t.Fatalf("`if $Flag` has %d activities in its branch, want 2:\n%s", len(branch), describe())