Skip to content

Read a string in a cell as a string, in the binary dictionary too - #37

Merged
ww-mw merged 2 commits into
mainfrom
cell-string-binary-read
Sep 18, 2026
Merged

ww-mw merged 2 commits into
mainfrom
cell-string-binary-read

Conversation

@ww-mw

@ww-mw ww-mw commented Sep 18, 2026

Copy link
Copy Markdown
Member

{"a"} in a compressed-binary .sldd displayed as {<1x1 string>} — the summary form, so the text was not on screen at all and the cell offered no editor. The text dictionary showed {"a"} the whole time, which is how it was reported: the same value read two ways depending on which format it had been saved in.

A string is not a JSON value in the XML channel, it is an object with a saveobj payload, and BinarySlddParser nests one in three places — parseEntryValue (an entry's own value), parsePropContent (a struct field or an object property), parseCellElement (a cell element). The first two tested Class="string"; the third never did, so the element fell through to the generic nested-object tail and became an object of class string whose text sat in the saveobj bag, where no formatter looks.

The display was the visible tip. Nothing decoded that bag back, so rebuilding the chunk wrote the element out as an empty <Element Class="string"> and the text was gone from the file — silent data loss on any save of a dictionary that held one, with no edit to it required.

Fixed by lifting string out of the object tail in parseCellElement, the same two lines its two siblings already had.

How the shape was settled

test/parity/matlab/probe_cell_string.m, against R2027a: a cell element holding an object is a classless <Element> wrapping the object's own <Element Class="...">, one level deeper than it looks — so the tail is right for every other class and the fix had to be a lift, not a replacement. Its two fixtures (cellstr_text.sldd, cellstr_binary.sldd) are one dictionary in both flavours, which lets the channels be asserted against each other rather than each against its own literal.

Tests

  • binaryWriteBackGate.test.ts — reads MATLAB's own bytes, compares both channels, and pins that an untouched rebuild keeps all six saveobj payloads and reopens as the values MATLAB wrote; two entries are asserted byte-identical to MATLAB's chunk.
  • binarySlddValues.test.ts — the reader-level regression, failing before the fix.
  • cellElementShape.test.ts — three assertions that had pinned {<1x1 string>} as a known limitation of the binary channel, blamed on a .mat MCOS payload, now assert both channels agree. An .sldd carries no MCOS blob and the write side was already byte-correct, so the limitation bullet they cited now says so.

Full npm run verify green: 4868 tests.

Two neighbours recorded, neither touched

  • We write Dimension="1*1" on a 1x1 cell's <P Name="Value"> where MATLAB omits the attribute. Save churn, not data loss; it round-trips, and it predates this defect.
  • MATLAB build 27.1.0.3393633 no longer emits _fields in a text dictionary, and StructNode.parse builds a scalar struct's field rows from _fields alone — so a struct in a current-MATLAB text .sldd shows no field row at all. Wider blast radius than this defect and not fixed here; measured with a nine-struct probe and written up in DESIGN.md.

`{"a"}` in a compressed-binary .sldd displayed as `{<1x1 string>}`: the summary
form, so the text was not merely mis-typed, it was not on screen at all and the
cell offered no editor. The text dictionary showed `{"a"}` the whole time, which
is how this was reported -- the same value read two ways depending on which
format it had been saved in.

A `string` is not a JSON value in the XML channel, it is an object with a
`saveobj` payload, and BinarySlddParser nests one in three places:
parseEntryValue for an entry's own value, parsePropContent for a struct field or
an object property, and parseCellElement for a cell element. The first two tested
`Class="string"` and called parseStringValue; the third never did, so the element
fell through to the generic nested-object tail and became an object of class
`string` whose text sat in the saveobj bag, where no formatter looks. One rule,
three paths, two of them right.

The display was the visible tip. Nothing decoded that bag back, so rebuilding the
chunk wrote the element out as an empty `<Element Class="string">` and the text
was GONE from the file -- silent data loss on any save of a dictionary that held
one, with no edit to it required. Re-serializing an untouched MATLAB-authored
dictionary with the fix stashed loses all four string-bearing entries.

Fixed by lifting `string` out of the object tail, the same two lines its two
siblings already had. probe_cell_string.m settles the shape the branch keys on,
which is a level deeper than it looks: a cell element holding an object is a
CLASSLESS <Element> wrapping the object's own <Element Class="...">, so the tail
was right for every other class and the fix had to be a lift rather than a
replacement. Its two fixtures are one dictionary in both flavours, which is what
lets the channels be asserted against EACH OTHER rather than each against its own
literal.

Three tests in cellElementShape.test.ts had pinned `{<1x1 string>}` as a known
limitation of the binary channel, blamed on the undecoded MCOS payload of a .mat.
That was wrong twice over -- an .sldd carries no MCOS blob, and the write side was
already byte-correct -- so the limitation bullet it cited now says so.

Two neighbours the same measurement turned up, both recorded in DESIGN.md and
neither touched here: we write `Dimension="1*1"` on a 1x1 cell where MATLAB omits
the attribute (save churn, round-trips correctly, predates this), and MATLAB has
stopped emitting `_fields` in a text dictionary, which is the only thing
StructNode.parse builds a scalar struct's field rows from.
A value that could not be read and was destroyed on save now reads and
round-trips, and three PRs merged since v1.22.0 ride along, so this is a minor
bump. The release tag has to match this field, and the pin in
data-explorer-vscode's package.json cannot move until the tag exists.

package-lock.json is left alone deliberately, as every bump from 1.14.0 on has
been: its `version` field is cosmetic for a git dependency, and a global version
substitution in that file has corrupted an unrelated package's entry before.
@ww-mw
ww-mw merged commit fac37a1 into main Sep 18, 2026
1 check passed
@ww-mw
ww-mw deleted the cell-string-binary-read branch September 18, 2026 15:48
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