Repository navigation
The MCOS decoder warns about the blocks it stopped reading and the variables its confidence gate skipped - #122
Merged
Conversation
… 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three commits, one per item. The two that change
src/regeneratedist/(rm -rf dist && npm run build); C-228's commit adds tests only, and a rebuild at it leavesdist/identical.npm run verifyis 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.
readBlocksstopped.buildPropertiesthen read every block past the stop asblocks[idx] || [], so every object past the damage showed only its class defaults, as if they were its own values. Nothing said so.decodeMcosVariablescontinued 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.
decodeMcosVariablesreturned only a Map, andattachMcosDecoded(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 }, anddecodeMcosVariables(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.Signalwhen the variable declaresSimulink.Signal, or... names class N, and the class table has M rows).Simulink.ModelWorkspacehandles 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, ascollectUndecodedspells them) and pushes onepart-unreadableper 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>.parsed.warnings.EncodedValue.recordPartialDecode/takePartialDecodes, besiderecordDecodeFailure. SlddNode.parse drains it before and after each entry, withpart= the entry name.recordDecodeFailurecould not be reused: its "shown as not decoded" is false for a value decoded in part.Deviations from the scope, with reasons.
stringwith 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".rootPath: a dictionary stream's value is unnamed, so without it the message would saythe MATLAB object "".Tests. New file
test/mcosDecodeWarnings.test.ts, 50 tests, each through the real decoder orparseMat/parseSlx+MatNode/ModelNode.fromParsed, or a dictionary throughsession.addDataSourceoringest. 13 fail before the change, each withexpected [] to deeply equal [warning ...]: there was no sink, so no warning (fail-before, against8e33408):buses, and the properties are still['Alignment'].[0, 99999]handle; a class lie; a class id past the class table; the decoder's own record (refused, and absent from the result).mixStruct.pandmixCell{1}; andmixStruct(1).pin a struct array.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-325and:799-818,mcosBlockSegmentEnd.test.tsandnestedMcos.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 inparseWarnings.test.ts(cut.warningstoEqual[]), 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
warningssink (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, andbuildPropertiesgives 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:
unreadBlocksinbuildProperties(blockIdx !== 0 && !blocks[blockIdx]), one per variable, recorded asrefused: false. This commit checks each scope point against that and changes nosrc. It adds the tests that pin what the counter means.rm -rf dist && npm run buildat this commit leavesdist/byte-identical, so the commit has nodist.Tests. A new describe in
test/mcosDecodeWarnings.test.ts, 7 tests, throughdecodeMcosVariableswith an issues map or throughparseMat+MatNode.fromParsed:[[a],[x,y],[p,q,r]].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.its handle names object 1, and the object table has 0 rows, andbusesis absent.buses, 1 object.buses(3).Elements(3)loses its Name.buses(3)count forbuses, and the Bus objects' own blocks are read.[1,1]) is counted once.mixCell{2}andmixStruct.sonly, and a lost type-2 block formixStruct.ponly.Fail-before. Against
8e33408(no issues map): 6 of the 7 fail, withexpected undefined to deeply equal {refused: false, ...}orexpected [] to deeply equal [warning]. The pristine control passes. With only C-228's half removed from the head (both.addcalls 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", andmcosBlockSegmentEnd.test.tsare 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,
modelOpaqueMcosVariablecould not tell a refusal from having no blob at all. For a class the data model knows, it calledbuildTypedNodeFromMcoswith an empty bag. Param.mat with a bad handle showed aParameterNodewith Value[ ]and DataTypeauto, 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 emptySignalNode.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: aWeakSet<MatVariable>beside the decode table, withsetMcosRefused(variable, boolean)/mcosRefused(variable).attachMcosDecodedsets it for each variable whose issue isrefused: true, and clears it for every other one, including the detachattachMcosDecoded(null, [variable]).modelOpaqueMcosVariable:if (!decoded && mcosRefused(variable)) return null;, ahead ofbuildTypedNodeFromMcos. The callers' existing null arms build the plain opaque (_createOpaque, which keeps_rawBytesand_matVar): MatNode throughparseMatVariable, ModelNode throughaddWorkspaceEntry. So the row is<1x1 Simulink.Parameter>, with its class, its icon and its bytes.[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
issuesbefore 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, throughparseMat/parseSlxandMatNode/ModelNode.fromParsed, or the realattachMcosDecoded+modelOpaqueMcosVariable:[99999],[1.5]and[-1](3 tests): the row's class is exactlyMatlabVariableNode, with_isOpaque,_opaqueClassNameSimulink.Parameter,_matVarthe variable and_rawBytesits bytes. Its icon (wsParameters), displayValue, dataType and children equal those of a no-decode twin built byparseMatVariable. Onepart-unreadable, partParam.Simulink.Signal): the opaque ofSimulink.Signal, not aSignalNode, and the warning namesSimulink.Parameter.mixStruct.p): the opaque, with a warning at partmixStruct.p.serializeValue) equals the twin's and contains noSimulink.Parameter.ParameterNodewith 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 andmcosRefusedis false.ParameterNode, and a re-attach with the real handle gives Value 42.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-stepissue?.refusedfails "twice"; never clearing fails "lasts only as long"; counting every issue as a refusal fails the partial control (after it was made to assertmcosRefused(buses) === false); no display check fails 8.Existing tests changed: none.
mcosParser.test.ts:290-325("never guesses"),nestedMcos.test.ts:494-525andmcosTypedNode.test.ts:278are untouched and green.Left out, and why
collectUndecodedwalks 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.extractCellsorparseMetaTablereturns null) stays silent, as the scope says. Nothing is decoded from it, and no object is shown with invented values.part-unreadablealready renders generically there. Only the usual pin bump follows a release.Gate, at the head (abc6fe1)
rm -rf dist && npm run build: rc 0, and the tree stays clean (git status --porcelainis 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 anode:builtin).Implements the maintainer's 2026-10-16 group E ruling.