Skip to content

Keep a complex cell member through a sibling edit, and a hand-edited Parameter shape through undo - #155

Merged
ww-mw merged 7 commits into
mainfrom
keep-what-was-read-through-sibling-edits-and-undo
Oct 19, 2026
Merged

ww-mw merged 7 commits into
mainfrom
keep-what-was-read-through-sibling-edits-and-undo

Conversation

@ww-mw

@ww-mw ww-mw commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

Two items from the group H follow-ups, one commit each (56b3da9, 659b5bf), plus a merge of main (16ae330) and four commits that add tests only (3abd651, 975caba, 42d92a2, e757474). Each item fixes a regression that a group H PR introduced: an edit or an undo rewrote data the file holds and nobody changed. Each item commit has dist regenerated (rm -rf dist && npm run build) and passed npm run verify on its own. The test-only commits change no src, dist or fixture, and dist stays clean when rebuilt at each.

Rulings: theme 1 (group F, 2026-10-18): "warn and show what was read; never refuse the file, never crash." D3 (save decisions, 2026-10-13): "Don't rewrite data nobody changed." B-25's own row: "undo gives what was read."

Commits

  • 56b3da9: B-36, the fix.
  • 659b5bf: B-37, the fix.
  • 16ae330: merge of main (f4f5c97).
  • 3abd651 (B-37, tests only): an undo gives a Parameter no Dimensions or Complexity the file does not hold.
  • 975caba (B-36, tests only): a complex member with a small imaginary part keeps its cell's literal editor.
  • 42d92a2 (B-36, tests only): a member with a negative imaginary part, or a char member, keeps its cell's literal editor.
  • e757474 (B-36, tests only): a member whose imaginary part is 0 takes no literal editor, whatever its real part or class.

B-36: a cell's complex member whose imaginary part is 0, through a sibling edit (56b3da9)

  • Ruling: D3, "Don't rewrite data nobody changed", and theme 1.

  • Wrong: a file's cell {complex(1,0), 5} shows {1+0i, 5} and offers its literal as an editor. If you retyped only the 5 ({1+0i, 6}), the save wrote the first member as a real 1, in both text and binary. This is a regression from Cell literals refuse unspaced arithmetic and read 0i as MATLAB does; VariantBank and VariantConfigurations follow MATLAB in every spelling #151: before it, the member stayed complex. Editing the 5 as an element already kept the member.

  • Root cause: Cell literals refuse unspaced arithmetic and read 0i as MATLAB does; VariantBank and VariantConfigurations follow MATLAB in every spelling #151 (B-02) made a cell literal read 1+0i as the real double MATLAB makes of it (zeroImaginaryAsReal). MatlabVariableNode._literalRoundTrips still said such a cell's literal was its value.

  • Fix: _literalRoundTrips now returns false when a member is a complex scalar whose imaginary part is 0 or -0 (new _isZeroImaginaryComplex). This also covers nested cells. Such a cell offers no editor for its literal, as a cell that shows an element as a summary offers none. setProperty('Value') refuses the literal with this reason: "This cell holds a complex element whose imaginary part is 0, which its text sets as a real number. Edit the element instead." The reason comes from _holdsZeroImaginaryElement, which looks into nested cells. For a .mat cell cut by the node ceiling, it looks into the ceiling twin. Any other refusal keeps the old message. A complex member whose imaginary part is not 0 reads back from the literal as itself, so that cell still takes its literal. An unchanged commit is still a no-op.

  • MATLAB truth (R2027a 27.1.0.3409468): test/fixtures/make_cell_zero_imaginary.m writes cell_zero_imaginary_binary.sldd and cell_zero_imaginary_text.sldd. Their entries are {complex(1,0),5}, {complex(2,-0),5}, {{complex(1,0)},5} and the control {complex(1,2),5}. cell_zero_imaginary.truth.json holds MATLAB's reading of those files. It also holds MATLAB's reading of core's saves after the element edits, made by make_cell_zero_imaginary_core.mjs. Every member reads complex, and the -0 is kept. The probes opened only files written under /tmp.

  • Tests: test/cellZeroImaginaryMember.test.ts (44 tests). It runs for both formats:

    • each file opens as written
    • each zero-imaginary cell offers no editor
    • retyping only the 5 is refused, and a save then reopen keeps the member complex, byte for byte as MATLAB wrote it
    • an unchanged commit records no undo step
    • the element edit 5 to 6 keeps the member as written
    • the nested cell refuses its literal too
    • the control still takes its literal and stays complex

    .mat cells (built with tools/matBytes) get these checks, each with node ceiling 0 and with no ceiling:

    • {complex(1,0), 5} and {{complex(1,0)}, 5} offer no editor and refuse their literal; {complex(1,2), 5} keeps its editor.
    • The imaginary part 0 at any real part and class (e757474): complex(0,0), complex(-0,0), complex(0,-0), complex(-0,-0), complex(-1,0), complex(Inf,0), and single and int16 complex(1,0). Each cell offers no editor and refuses its literal with the reason. With its 5 set to 6 as an element, the cell's copy is pasted into a text and a binary dictionary, saved and reopened. The member keeps its class (double, single, int16) and stays complex, and the copy holds the member as read. A cell cut by the node ceiling has no rows to edit, so its copy is saved as read. The test uses the member text shown and checks only that it ends +0i, so it does not pin how a -0 real part is shown.
    • Imaginary parts that are not 0 (975caba, 42d92a2): 0.5, 1.2246467991473532e-16 (exp(1i*pi)), 1e-16, 5e-324, NaN, 1-2i and 0-2i. Each cell keeps its editor, takes its literal with only the 5 retyped, and keeps the member double (complex).
    • A char member that reads like a zero-imaginary number, {'1+0i', 5} (42d92a2). The cell keeps its editor, takes {'1+0i', 6}, and the member stays a char.

    Mutants of _isZeroImaginaryComplex. Each passed the full suite before its pin commit. Each now fails its pin's 2 tests (node ceiling 0 and none) at valueEditable:

    • 975caba: Math.abs(im) < 1e-9, Math.abs(im) < 1, and !Number(im). Only the NaN control catches !Number(im).
    • 42d92a2: im <= 0 (caught by 1-2i), im === 0 || re === 0 (caught by 0-2i), and the _scalarType !== 'complex' guard dropped (caught by the char).
    • e757474: && re !== 0 (caught by complex(0,0), and by each -0 form alone), && re > 0 (caught by complex(0,0), and by complex(-1,0) alone), && Number.isFinite(re) (caught by complex(Inf,0)), and the complex guard limited to class double (caught by single, and by int16 alone). With the valueEditable assertion removed, the refusal assertion still catches each of the four.
    • Without the ceiling-twin branch, the "node ceiling 0" test fails.

    Fail before: with the fix's src (MatlabVariableNode.ts) reverted to main's at the head, 18 of 44 fail, each for the right reason: valueEditable was true, the reopened member was a real double 1 (or 2) where complex was expected, or the nested cell took its literal. They are the 14 dictionary tests, the 2 first .mat tests and the 2 zero-side .mat tests of e757474. The .mat tests were added after the fix, and they fail on main's source too. The 26 that pass are controls: opens as written, the unchanged commit, the element edit, the complex(1,2) control, the truth tests, and the not-0 and char .mat cells, which have an editor on main as well.

  • Existing tests changed: none. test/parity/matlab/m9_merged_saves.truth.json gained rows. The merged-save read-back requires every binary fixture in the tree to have been read back by MATLAB. So probe_m9_merged_reopen.m was run on the new fixture's 2 cases (a rename, and an older-save rename), and both read as expected. Their rows were added where a full re-run puts them, and a script checked that the result is the old truth plus those rows. No other row changed.

  • Left out: in a text dictionary the complex member itself takes no editor, because it is read from a MAT stream (the C-012 ruling, unchanged). Not checked, and the same as on main: whether a cell literal keeps a typed member's class, for example {int16(5), 1}.

B-37: a hand-edited Parameter's Dimensions and Complexity, through undo (659b5bf)

  • Ruling: theme 1, and B-25's own row: "undo gives what was read".

  • Wrong: a file whose numeric Dimensions or Complexity disagree with its Value is hand-edited (MATLAB refuses that state). Undo of a structural edit gave such a Parameter the Value's shape, not the file's. Take complex_binary.sldd with pRow's Dimensions set to [1 5]. Cut element 2 gives [1 2], and the undo gave [1 3], not [1 5], in the rows, the save and on reopen. Add, an element edit, and an edit of the Value row itself failed the same way, in both formats. So did Complexity written as 'real' on a complex row. This is a regression from Nodes keep what they read and check every spelling of a property: JSON re-reads, one-element arrays, struct fields, bus elements, Complexity, Parameter shape #152 for this hand-edited case. Before Nodes keep what they read and check every spelling of a property: JSON re-reads, one-element arrays, struct fields, bus elements, Complexity, Parameter shape #152 these edits did not touch the shape, so undo kept what was read. The undo of a Value typed on the Parameter was already correct (_restorerFor('Value')).

  • Root cause: the restorers of Add, removal and element edit restore the Value and then re-derive the shape from it (ParameterNode._followValueShape). They did not hold the shape as it was before the edit.

  • Fix:

    • BaseNode.childStructureRestorer(child) is a new hook beside childStructureChanged, null by default.
    • For its _valueNode, ParameterNode returns _shapeRestorer(). That closure puts back Dimensions and Complexity as they are now, each only where the file holds it. _restorerFor('Value') now reuses it, and its behaviour is unchanged.
    • childEdit.holdersRestorer(node) collects the restorer of every ancestor. addChildUndoable, removeChildUndoable and DataModel.editProperty capture it before the change and call it in undo, after the child is restored.
    • Redo still re-derives from the Value, as the edit did.
    • Simulink.Parameter.md describes this.
  • MATLAB truth: none. MATLAB refuses this state, and the ruling (undo gives what was read) is not a measurement. The MATLAB-written complex_binary.sldd is a control.

  • Tests: test/parameterShapeUndoAsRead.test.ts (38 tests). They drive the real session through applyStep (deleteNodeById, addChildTo, editProperty). Each case checks four things:

    • the file reads as written
    • after the edits, the shape is the Value's
    • each undo gives exactly the shape from before its step, and the last undo gives what was read, in the rows, the bag and the saved bytes (a binary reopen, or the text JSON)
    • redo gives the edit's shape again

    The binary pRow cases, each for Dimensions [1 5] and for Complexity 'real', are: cut, two cuts collapsing to a scalar, Add, Add then cut, element edit, every element set real, Value row typed, and typed on the Parameter. Also covered: binary pStruct with Dimensions [1 5] (add a field), the MATLAB-written file unchanged (undo still gives [1 3] complex), and text pDoubleRow for Dimensions [1 5] and Complexity 'complex'.

    A Parameter that holds neither Dimensions nor Complexity (3abd651): paramArray(1) of object_array_binary.sldd and of object_array_text.sldd. Type [1 2 3] on it, then make one of a cut, an Add, an element edit, an edit of the Value row, or a second typed Value. Undo it, then make it again. The save after the undo, and the save after the edit is made again, hold exactly the keys the file held, read from the saved bytes. That is 5 steps in 2 formats, 10 tests. They pin the "each only where the file holds it" of _shapeRestorer: with its hasOwnProperty filter dropped, the full suite passed before 3abd651, and now all 10 fail.

    Fail before: with the fix's src reverted, 23 of 38 fail, all at the undo assertion and for the right reason, for example [1 3] where [1 5] was expected. The 15 that pass are the 4 typed-on-Parameter controls, the MATLAB-written case, and the 10 neither-key cases, which pin the filter rather than the fix. Mutants: without the editProperty restorer call, 10 tests fail; without the Add call, 7; without the removal call, 6.

  • Existing tests changed: none.

  • Left out: nothing.

Gates (at the head, e757474)

  • rm -rf dist && npm run build leaves the tree clean.
  • npm run verify: exit 0. 305 test files passed, 1 skipped. 10910 tests passed, 27 skipped.
  • Opt-in, with DEX_READBACKS_CORPUS set: readbacks.truth and m9MergedSaves.truth, 46 of 46 passed.
  • Live DEX_MATLAB_CMD tests: not run. None of them edits a cell that holds a complex member or undoes a Parameter's structural edit, and src is unchanged since the item commits.
  • CI at e757474: build-and-test, CodeQL and both Analyze jobs passed (CI run 39235425919). GitHub says the PR is MERGEABLE / CLEAN.
  • The merge of main had one conflict, in the generated MatlabVariableNode.js.map. It was resolved by rebuilding dist from the merged source. origin/main is still f4f5c97, which the branch holds.

Seen, not changed here (same on main; possible follow-ups)

  • A cell holding a complex array member, {[1+0i 2+0i], 5} or {[1+2i 2+0i], 5}, offers its literal editor, and its own literal is then refused as "Invalid MATLAB expression". Nothing is written wrong, but the editor cannot take its own text.
  • complex(1, NaN) in a cell shows 1NaNi, with no sign. MATLAB shows 1.0000 + NaNi. The reading is ruled ("Binary Inf/NaN complex values", 2026-10-13); the cell literal's display of it is not. The tests do not pin that text.
  • complex(2, -0) shows 2+0i. Theme 2 may want MATLAB's -. The dictionary tests pin this existing display.
  • MatlabVariable.md (around lines 440-444) gives only summary elements as the reason a cell has no in-cell editor. It could also name the zero-imaginary case.

Mutants re-checked by the orchestrator at e757474

In a fresh clone, each applied alone to src, run against cellZeroImaginaryMember.test.ts and parameterShapeUndoAsRead.test.ts, then restored: B-36's Math.abs(im) < 1e-9, < 1, !Number(im), im <= 0, im === 0 || re === 0, && re !== 0, && re > 0, && isFinite(re), the complex guard dropped, and the guard limited to double each fail 2 tests; B-37 without the hasOwnProperty filter fails 10.

Implements the maintainer's rulings, as audited in group H (2026-10-19).

ww-mw added 7 commits October 19, 2026 04:08
… is edited

A cell a file holds as {complex(1,0), 5} shows its literal as {1+0i, 5}.
Since cell members follow MATLAB, a cell literal reads 1+0i as the real
double MATLAB makes of it. So with only the 5 retyped, {1+0i, 6} saved
the member nobody changed as a real 1, in a text and a binary dictionary
alike. Before that change the member stayed complex.

Under "do not rewrite data nobody changed" and theme 1, such a cell's
literal is not its value: _literalRoundTrips now says so for a cell that
holds a complex element whose imaginary part is 0 (or -0), itself or in
a nested cell. The cell offers no editor for its literal, as one that
shows an element as a summary offers none, and refuses the literal set
on it with a reason that says why. Its elements are edited one at a
time: the 5 set to 6 alone, the member kept as read. A complex member
with a non-zero imaginary part reads back from the literal as itself,
so that cell still takes its literal.

test/fixtures/cell_zero_imaginary_binary.sldd and _text.sldd are MATLAB
R2027a's (make_cell_zero_imaginary.m): {complex(1,0), 5},
{complex(2,-0), 5}, {{complex(1,0)}, 5} and the control {complex(1,2), 5}.
cell_zero_imaginary.truth.json is MATLAB's reading of them, and of core's
saves after the element edits (make_cell_zero_imaginary_core.mjs): every
member is still complex, the -0 kept. The new test drives both formats
and a .mat file, with and without its node ceiling.

The new binary fixture is in the merged-save read-back's tree, so MATLAB
was asked about its two merged saves too (probe_m9_merged_reopen.m on
just those): both read as expected. Their rows are added to
m9_merged_saves.truth.json as MATLAB wrote them; no other row changed.
After an edit of a Simulink.Parameter's Value, its Dimensions and Complexity
are set from the Value, as MATLAB sets them. The undo of an Add, a removal,
an element edit or an edit of the Value row set them again from the restored
Value. For a file MATLAB wrote that is what was read. A hand-edited file can
hold a numeric Dimensions or a Complexity that disagrees with its Value
(MATLAB refuses that state), and there the undo gave the Value's shape, not
the file's: pRow with [1 5] for its three elements, cut to two and undone,
came back [1 3] and was saved so.

Those undos now hold the two as they were before the edit and put them back
after the undone change is reported, as the undo of a Value typed on the
Parameter already did. A node above an edited one says what it sets from it
(BaseNode.childStructureRestorer, which only ParameterNode answers), and the
undo closures of childEdit's Add and removal and of the session's property
edit call it (childEdit.holdersRestorer). Redo sets them from the Value, as
the edit did.

test/parameterShapeUndoAsRead.test.ts drives the session on copies of
complex_binary.sldd and typed_mcos.sldd with one tag changed, in both
formats: each undo gives back the shape before it, the last one what was
read, shown and saved; redo gives the edit's shape again.
The only conflict was the generated sourcemap for MatlabVariableNode; dist was
rebuilt from the merged source.
…le does not hold

The undo of an edit of a Simulink.Parameter's Value puts back its Dimensions
and Complexity only where the file holds them (ParameterNode._shapeRestorer).
No test held that: with the filter dropped, so that both were put back whether
held or not, the whole suite still passed. Yet the undo then left both keys on
the Parameter, and the save wrote a Dimensions and a Complexity the file never
held, data nobody changed.

test/parameterShapeUndoAsRead.test.ts now opens object_array_binary.sldd and
object_array_text.sldd, whose paramArray(1) holds neither, types [1 2 3] on it,
then makes a cut, an Add, an element edit, an edit of the Value row or another
typed Value, undoes it and makes it again. The save after the undo and after
the edit made again holds the keys the file held, and no others. With the
filter dropped, all 10 cases fail.
…s literal editor

A cell whose complex member has an imaginary part of 0 offers no editor for its
literal (MatlabVariableNode._isZeroImaginaryComplex). The only control whose
imaginary part was not 0 was 2i, so a test of "near 0" in place of "0" still
passed the whole suite: below 1e-9 or below 1, or a test that also took NaN
for 0. Each took the editor from a cell whose literal reads back as itself,
such as {1+0.5i, 5}, or {-1+1.2246467991473532e-16i, 5} for exp(1i*pi).

test/cellZeroImaginaryMember.test.ts now builds .mat cells whose complex member
has the imaginary part 0.5, 1.2246467991473532e-16, 1e-16, 5e-324 (the least
double above 0) or NaN, with node ceiling 0 and with none. Each cell keeps its
editor, takes its literal with only the 5 retyped, and keeps the member complex.
Each of those mutants fails both cases, at the editor.
…mber, keeps its literal editor

A cell whose complex member has an imaginary part of 0 offers no editor for its
literal (MatlabVariableNode._isZeroImaginaryComplex). Every control so far had
a positive imaginary part and a non-zero real part, and none was a char. So
three changes to that test still passed the whole suite: an imaginary part
taken as 0 when it is 0 or below, a real part of 0 taken as 0 too, and the
check that the member is complex dropped. Each took the editor from a cell
whose literal reads back as itself, {1-2i, 5}, {0-2i, 5}, {0+2i, 5} or the
char {'1+0i', 5}, and refused its literal set with a reason that is false.

test/cellZeroImaginaryMember.test.ts now adds .mat cells whose complex member
is 1-2i or 0-2i to the controls, and a .mat cell {'1+0i', 5} whose first member
is a char. Each runs with node ceiling 0 and with none. Each cell keeps its
editor and takes its literal with only the 5 retyped, and the member keeps its
class. Each of those three changes fails both cases, at the editor.
…r at any real part or class

A cell whose complex member has an imaginary part of 0 offers no editor for its
literal (MatlabVariableNode._isZeroImaginaryComplex). Every such member in the
tests had a real part of 1 or 2, and was a double. So four changes to that test
still passed the whole suite: one that also asked for a real part other than 0,
one for a real part above 0, one for a finite real part, and one that took only
a double for complex. Each gave the literal editor back to a cell such as
{0+0i, 5}, {-1+0i, 5}, {Inf+0i, 5}, or a single or int16 {1+0i, 5}, and
retyping only the 5 then made the member nobody changed a real double.

test/cellZeroImaginaryMember.test.ts now builds .mat cells whose member is
complex(0, 0), with a -0 in either part or both, complex(-1, 0),
complex(Inf, 0), and single and int16 complex(1, 0), with node ceiling 0 and
with none. Each cell offers no editor and refuses its literal with the reason.
With its 5 set to 6 as an element, the cell copied into a text and a binary
dictionary, saved and opened again, keeps the member of its class and complex.
Each of the four changes fails both cases, at the editor.
@ww-mw
ww-mw merged commit 500885e into main Oct 19, 2026
4 checks passed
@ww-mw
ww-mw deleted the keep-what-was-read-through-sibling-edits-and-undo branch October 19, 2026 10:20
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.

1 participant