Skip to content

The MCOS decoder warns about the blocks it stopped reading and the variables its confidence gate skipped - #122

Merged
ww-mw merged 3 commits into
mainfrom
mcos-decoder-reports-what-it-stopped-reading
Oct 16, 2026
Merged

ww-mw merged 3 commits into
mainfrom
mcos-decoder-reports-what-it-stopped-reading

Conversation

@ww-mw

@ww-mw ww-mw commented Oct 16, 2026

Copy link
Copy Markdown
Member

Three commits, one per item. The two that change src/ regenerate dist/ (rm -rf dist && npm run build); C-228's commit adds tests only, and a rebuild at it leaves dist/ identical. npm run verify is green at each commit.

C-260: the MCOS decoder reports the blocks it stopped reading and the variables its gate skipped (80a6e02)

The ruling (DECIDED 2026-10-16, group E, theme 1, damaged input): "warn and show what was read; never refuse the file and never crash."

The recommendation it adopts: "(a), using the existing 'part-unreadable' code. It follows ParseWarning.ts's own rule, keeps the deliberate stop and skip behaviour (and the tests that pin it) intact, and needs no new warning code." Option (a): "Add an optional warnings?: ParseWarning[] to decodeMcosVariables and pass it through from attachMcosDecoded, MatNode.fromParsed and ModelNode.fromParsed. Push 'part-unreadable' ... once per skipped variable, naming it."

What was wrong. Both of the decoder's deliberate refusals were silent.

  • When a property count could not be right (over the 1000-property cap, or overrunning its segment), readBlocks stopped. buildProperties then read every block past the stop as blocks[idx] || [], so every object past the damage showed only its class defaults, as if they were its own values. Nothing said so.
  • The confidence gate in decodeMcosVariables continued past a variable whose handle names an object the table does not hold, or objects of another class than it declares. Nothing said so.

Root cause. The decoder had no warnings channel. decodeMcosVariables returned only a Map, and attachMcosDecoded (reached through NodeRegistry's hook from MatNode, ModelNode and MatlabVariableNode) had nowhere to put a report.

Fix. The stop and the refusal are unchanged; only the report is new.

  • McosParser.ts: export interface McosDecodeIssue { refused: boolean; reason: string }, and decodeMcosVariables(bytes, variables, issues?). The decode context records the objects whose type-2 property block was not read (buildProperties) and the strings whose type-1 block was not read (stringPayloadWords). Sets, not counters, because one object can be reached twice. They start fresh for each variable. A partial decode is recorded after the string-payload read. The gate records a refusal naming the first bad id (its handle names object 99999, and the object table has N rows) or the class found (its objects are of class Simulink.Parameter, not Simulink.Signal when the variable declares Simulink.Signal, or ... names class N, and the class table has M rows).
  • An id of 0 stays silent. MATLAB writes the null object for a handle that saved no state; the scope measured two such Simulink.ModelWorkspace handles in the corpus. A handle that mixes 0 with a bad id still warns.
  • attachMcosDecoded(blob, vars, warnings?, rootPath?) walks with each object's MATLAB path (s.f, s(2).f, c{3}, 1-based, as collectUndecoded spells them) and pushes one part-unreadable per affected variable, part = the path: the MATLAB object "<path>" was decoded in part: <reason>. or ... could not be decoded, so its properties are not shown: <reason>.
  • The NodeRegistry hook forwards both parameters. MatNode.fromParsed and ModelNode.fromParsed pass parsed.warnings.
  • A dictionary's encoded value (hex, or cdata in a text dictionary) records its messages through a new EncodedValue.recordPartialDecode / takePartialDecodes, beside recordDecodeFailure. SlddNode.parse drains it before and after each entry, with part = the entry name. recordDecodeFailure could not be reused: its "shown as not decoded" is false for a value decoded in part.

Deviations from the scope, with reasons.

  1. Two clauses, not one. A string with no type-1 block is not "shown with its class defaults", so it gets its own: "N of its strings' text could not be read, so those strings are shown without it".
  2. The sets start fresh per variable, not as a snapshot of one shared set. With one shared set, an object two variables share would be reported only for the first.
  3. A 4th hook parameter, rootPath: a dictionary stream's value is unnamed, so without it the message would say the MATLAB object "".
  4. The refusal says "its properties are not shown" rather than the scope's "shown as an opaque object", which was false until C-229 (below). It is true before and after it.

Tests. New file test/mcosDecodeWarnings.test.ts, 50 tests, each through the real decoder or parseMat/parseSlx + MatNode/ModelNode.fromParsed, or a dictionary through session.addDataSource or ingest. 13 fail before the change, each with expected [] to deeply equal [warning ...]: there was no sink, so no warning (fail-before, against 8e33408):

  • busArray.mat: a count over the cap, and a count that overruns its segment. One warning, part buses, and the properties are still ['Alignment'].
  • Param.mat: an id past the table; a mixed [0, 99999] handle; a class lie; a class id past the class table; the decoder's own record (refused, and absent from the result).
  • strings_nested.mat: mixStruct.p and mixCell{1}; and mixStruct(1).p in a struct array.
  • mcosfix.slx: a workspace object whose handle is past the table.
  • strings.mat: the type-1 count over the cap warns once per string.
  • A binary dictionary's hex stream, and a text dictionary's cdata of it: one warning naming the entry, and the stream written back as read.

The other 37 are controls and pass before and after: ids [0] say nothing; lost bytes that no object addresses say nothing; a stale partial decode is not reported against the next dictionary; and a sweep of 38 fixture files (.mat, .slx, .sldd; v7.3 is HDF5 and refused by design) opens with no MCOS warning. Mutations: dropping SlddNode's before-entry drain fails the stale test; dropping the id-0 exemption fails 3.

On real files. The scope measured the would-warn count with an instrumented copy of dist: 0 over 137 core fixture and parity-artifact files, and 0 over 335 opened corpus files (339,895 variables decoded). The only ids out of range were the two id-0 handles above.

Existing tests changed: none. mcosParser.test.ts:276-325 and :799-818, mcosBlockSegmentEnd.test.ts and nestedMcos.test.ts:465-520, which pin the stop and the refusal, call without a sink and are untouched and green. So are the workspace-cut sweeps in parseWarnings.test.ts (cut.warnings toEqual []), whose cuts land before the blob.

C-228: which objects count as not read, wherever the block reader stopped (2adffc0)

The ruling: group E, theme 1, as above.

The recommendation it adopts: "(a), built as one change with C-229 because they share the sink." Option (a): "Add an optional trailing warnings sink (the SlddNode.parse shape) to decodeMcosVariables, attachMcosDecoded and the NodeRegistry hook. Push one part-unreadable for each variable that has an object whose block index lies past the cut. This is honest and keeps the pinned stop."

What was wrong. readBlocks (if (nProps > 1000 || p + nProps * 12 > to) break;) ends the block table at a bad count, and buildProperties gives every object past the cut its class defaults with no warning. Same root cause as C-260: no warnings channel.

Fix. The counter that C-228 asks for landed with C-260's sink in 80a6e02: unreadBlocks in buildProperties (blockIdx !== 0 && !blocks[blockIdx]), one per variable, recorded as refused: false. This commit checks each scope point against that and changes no src. It adds the tests that pin what the counter means. rm -rf dist && npm run build at this commit leaves dist/ byte-identical, so the commit has no dist.

Tests. A new describe in test/mcosDecodeWarnings.test.ts, 7 tests, through decodeMcosVariables with an issues map or through parseMat + MatNode.fromParsed:

  1. Control: pristine busArray.mat records nothing, and its BusElement names are [[a],[x,y],[p,q,r]].
  2. mcosBlockSegmentEnd.test.ts's type-2 patches, a segment that ends 1-3 bytes into a count, and the control 4 bytes short: each gives {refused: false, "3 of its objects' property blocks ..."}, and the decode is still ['Alignment'] with 3 elements.
  3. The type-1 variant of those patches: it records the gate's refusal, its handle names object 1, and the object table has 0 rows, and buses is absent.
  4. A header that ends the type-2 segment before its last block, with no count to stop on: one warning, part buses, 1 object. buses(3).Elements(3) loses its Name.
  5. A count that overruns its segment in a nested object's block: 3 objects. The BusElements under buses(3) count for buses, and the Bus objects' own blocks are read.
  6. An object that one handle names twice (ids [1,1]) is counted once.
  7. Each segment counts only the objects that use it. In strings_nested.mat a lost type-1 block is reported for mixCell{2} and mixStruct.s only, and a lost type-2 block for mixStruct.p only.

Fail-before. Against 8e33408 (no issues map): 6 of the 7 fail, with expected undefined to deeply equal {refused: false, ...} or expected [] to deeply equal [warning]. The pristine control passes. With only C-228's half removed from the head (both .add calls deleted): 5 of the 7 fail, plus C-260's three block-reader tests and its two dictionary tests; the control and the type-1 gate test pass. Mutations, each caught: a counter in place of the Set fails test 6; counting at depth 0 only fails the 3 nested tests; letting block 0 count fails C-260's "lost bytes that no object addresses say nothing".

Deviation from the scope, with the reason. The scope's test 2 said the type-1 variant "records nothing for buses". It does not. To end the type-1 segment at the table's end, the patch has to set header words 4, 5 and 6 to the table's length, which empties the object table, so the gate refuses buses. Test 3 asserts what really happens. Test 7 covers what the scope meant: lost type-1 bytes say nothing about an object that uses no type-1 block.

Existing tests changed: none. mcosParser.test.ts:799-818, which calls defaults-only "the honest answer", and mcosBlockSegmentEnd.test.ts are untouched and green; the stop they pin is unchanged.

C-229: a refused object of a known class is the plain opaque, not an empty typed node (abc6fe1)

The ruling: group E, theme 1, as above.

The recommendation it adopts: "(b), done with C-228's sink. The empty 1x1 Parameter is the misleading part." Option (b): "Do (a) [warn], and also let modelOpaqueMcosVariable see the refusal. A refused known class then returns null, which gives the plain opaque with the right class and icon, instead of a 1x1 typed node with an empty Value."

What was wrong. When the gate refused a variable, modelOpaqueMcosVariable could not tell a refusal from having no blob at all. For a class the data model knows, it called buildTypedNodeFromMcos with an empty bag. Param.mat with a bad handle showed a ParameterNode with Value [ ] and DataType auto, a 1x1 Parameter that is not the one in the file, and copying it wrote that empty Parameter ({_array_class: 'Simulink.Parameter', ... _elements: [{_properties: {}}]}). A class lie showed an empty SignalNode.

Root cause. The decode result carried no refused state to the node layer. A refused variable and a variable with no decode looked the same to modelOpaqueMcosVariable.

Fix. The warning half is C-260's, unchanged.

  • mcosDecodedTable.ts: a WeakSet<MatVariable> beside the decode table, with setMcosRefused(variable, boolean) / mcosRefused(variable).
  • attachMcosDecoded sets it for each variable whose issue is refused: true, and clears it for every other one, including the detach attachMcosDecoded(null, [variable]).
  • modelOpaqueMcosVariable: if (!decoded && mcosRefused(variable)) return null;, ahead of buildTypedNodeFromMcos. The callers' existing null arms build the plain opaque (_createOpaque, which keeps _rawBytes and _matVar): MatNode through parseMatVariable, ModelNode through addWorkspaceEntry. So the row is <1x1 Simulink.Parameter>, with its class, its icon and its bytes.
  • Not refusals, and unchanged: ids [0] (the null object), no blob to decode against, and a decode in part. A nested object was already the opaque without a decode.

Deviation from the scope (an addition), with the reason. The refused set is read from issues before the walk starts. The walk deletes each issue the first time it meets it (a host's tree can hold one variable object in two places), so setting the flag during the walk would clear it on the second visit and show that variable typed. A test pins this.

Tests. A new describe in test/mcosDecodeWarnings.test.ts, 13 tests, through parseMat/parseSlx and MatNode/ModelNode.fromParsed, or the real attachMcosDecoded + modelOpaqueMcosVariable:

  • Param.mat with ids [99999], [1.5] and [-1] (3 tests): the row's class is exactly MatlabVariableNode, with _isOpaque, _opaqueClassName Simulink.Parameter, _matVar the variable and _rawBytes its bytes. Its icon (wsParameters), displayValue, dataType and children equal those of a no-decode twin built by parseMatVariable. One part-unreadable, part Param.
  • A class lie (declared Simulink.Signal): the opaque of Simulink.Signal, not a SignalNode, and the warning names Simulink.Parameter.
  • mcosfix.slx: the refused workspace row is the opaque, and every other row has the class it had in the pristine parse.
  • Nested (mixStruct.p): the opaque, with a warning at part mixStruct.p.
  • A copy of the refused row (serializeValue) equals the twin's and contains no Simulink.Parameter.
  • Controls: the pristine file is a ParameterNode with Value 42; ids [0] keep the typed node with no warning; a variable never attached, and one attached with a null blob, are typed nodes; a decode in part keeps its node class and mcosRefused is false.
  • A refusal lasts only as long as the attach that found it: refused gives null, the detach gives a ParameterNode, and a re-attach with the real handle gives Value 42.
  • A variable a tree holds twice is refused both times, and said once.

Fail-before. With HEAD's mcosTypedNode.ts (the new table module present so that the import resolves): 8 of the 13 fail, each a typed node where the opaque was expected (expected ParameterNode to not be an instance of ParameterNode, expected SignalNode ..., the node class: expected ParameterNode to be MatlabVariableNode, refused: expected ParameterNode to be null); the nested case and 4 controls pass. Mutations, each caught: a per-step issue?.refused fails "twice"; never clearing fails "lasts only as long"; counting every issue as a refusal fails the partial control (after it was made to assert mcosRefused(buses) === false); no display check fails 8.

Existing tests changed: none. mcosParser.test.ts:290-325 ("never guesses"), nestedMcos.test.ts:494-525 and mcosTypedNode.test.ts:278 are untouched and green.

Left out, and why

  • The short-struct warning inside an MCOS property. C-260's recommendation says to do it "in the same change as agenda.md:74 (the short-struct warning not reaching MCOS properties)". That is Record a .mat payload of a type with no width as not decoded, give a struct array rows only for the elements it holds, and report a compressed record too short for its tag #91's leftover, not a group E item, and the scope leaves it out unless trivial. This PR does not take it: collectUndecoded walks only top-level variables, so reaching a struct inside an MCOS property would mean walking decoded object properties, which is a change of its own and not part of this ruling.
  • A blob that does not parse at all (extractCells or parseMetaTable returns null) stays silent, as the scope says. Nothing is decoded from it, and no object is shown with invented values.
  • The 1000-property cap is unchanged.
  • No vscode change is needed: part-unreadable already renders generically there. Only the usual pin bump follows a release.
  • MATLAB was not needed: theme 1, and only damaged blobs reach these paths.

Gate, at the head (abc6fe1)

  • rm -rf dist && npm run build: rc 0, and the tree stays clean (git status --porcelain is empty).
  • npm run verify: rc 0. Typecheck, build, smoke, test (239 files passed, 1 skipped; 7,999 tests passed, 26 skipped, of 8,025), check:pack (571 files, dist only), check:leak, and check:browser (156 modules reachable from the barrel, none importing a node: builtin).
  • At each commit: 7,979 passed at 80a6e02 and 7,986 at 2adffc0 (26 skipped each time). The suite grows by the new tests only: 50, then 7, then 13.

Implements the maintainer's 2026-10-16 group E ruling.

ww-mw added 3 commits October 16, 2026 15:52
… part

The MCOS decoder stops at a property count that cannot be right, and its
confidence gate refuses a variable whose handle names an object the table
does not hold or objects of another class. Both are deliberate and stay,
but both were silent: an object past the damage showed only its class
defaults as if they were its own values, and a refused variable showed
nothing, with no warning anywhere.

decodeMcosVariables now takes an optional issues map and records, per
variable, a refusal (with the bad id or the class found) or a partial
decode (how many of its objects' property blocks, or how many strings'
text, could not be read). The null object, id 0, which MATLAB writes for a
handle that saved no state, stays silent. attachMcosDecoded turns each
issue into one part-unreadable warning named by the object's MATLAB path
(s.f, s(2).f, c{3}). MatNode and ModelNode pass their file's warnings. A
dictionary's encoded value, hex or cdata, records its messages through a
new EncodedValue.recordPartialDecode, which SlddNode.parse collects per
entry, as it collects decode failures.

The decode itself is unchanged, and so is every test that pins the stop
and the refusal. New tests in test/mcosDecodeWarnings.test.ts cover each
case, and every MATLAB-written object fixture still opens with no MCOS
warning.
…lock reader stopped

The previous commit's partial-decode warning counts the objects that
address a property block the reader did not read. It does not count the
times the reader stopped. These tests pin what that means. Each one goes
through the decoder's own issues map or through MatNode.fromParsed.

- A type-2 segment that ends 1 to 3 bytes into a property count gives
  the same record as a count over the cap: 3 of buses' objects. So does
  the control that ends 4 bytes short. These are the patches that
  mcosBlockSegmentEnd.test.ts uses, and the decode it pins is unchanged.
- A header that ends the segment before its last block, so there is no
  count to stop on, is still reported: 1 object.
- A nested object counts toward the variable that reaches it. A
  BusElement under buses(3) loses its Name and is counted for buses,
  both through the header case above and through an overrunning count
  in a nested block (3 objects).
- An object that one handle names twice is counted once.
- Each segment counts only the objects that use it. In
  strings_nested.mat, lost type-1 blocks are reported for the two
  strings alone, and a lost Parameter block for mixStruct.p alone.
- The type-1 variant of mcosBlockSegmentEnd's patches is not a partial
  decode. To end the type-1 segment at the table's end it has to empty
  the object table, so what is recorded is the gate's refusal.

Tests only. The counter already landed with the sink in the previous
commit, so src and dist are unchanged. A rebuild leaves dist identical.
…an empty typed node

When the MCOS confidence gate refuses a variable (its handle names an object
the table does not hold, or objects of another class than it declares), it
has no decode. For a class the data model knows, modelOpaqueMcosVariable
then built the typed node from no properties at all: Param.mat with a bad
handle showed a ParameterNode with Value "[ ]" and DataType "auto", a 1x1
Parameter that is not the one in the file, and a copy of it wrote that empty
Parameter. A class lie showed an empty SignalNode.

attachMcosDecoded now records each refused variable in a WeakSet beside the
decode table (setMcosRefused / mcosRefused), and clears it for every other
variable, the detach included. modelOpaqueMcosVariable returns null for a
refused variable with no decode, so MatNode and ModelNode build the plain
opaque: "<1x1 Simulink.Parameter>", with its class, its icon and its bytes.
The warning for the refusal is the one C-260 added, and is unchanged.

Not refusals, and unchanged: a handle that is only the null object (ids [0],
as MATLAB writes a handle that saved no state), no blob to decode against,
and a decode in part. Nested objects were already the opaque without a
decode. No existing test changes; the new ones are in
test/mcosDecodeWarnings.test.ts.
@ww-mw
ww-mw merged commit f16de1a into main Oct 16, 2026
4 checks passed
@ww-mw
ww-mw deleted the mcos-decoder-reports-what-it-stopped-reading branch October 16, 2026 20:44
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