Skip to content

fix(cadence): read the top-level scope's property bag instead of skipping 8 fixed bytes - #231

Merged
valentinozegna merged 1 commit into
IntelligentElectron:mainfrom
vzegnameta:fix/hierarchy-top-scope-property-bag
Sep 22, 2026
Merged

valentinozegna merged 1 commit into
IntelligentElectron:mainfrom
vzegnameta:fix/hierarchy-top-scope-property-bag

Conversation

@vzegnameta

Copy link
Copy Markdown
Contributor

Closes #230.

What

TOP_LAYOUTS carries a padded flag that skips 8 bytes before the top-level occurrence
count. Those 8 bytes are not padding — they are a preamble whose trailing data is empty:
FF E4 5C 39 then a uint32 length of 0.

This replaces the flag with a readPreamble call, which consumes the same 8 bytes when the
trailing data is empty and 8 + len bytes when it is not. TOP_LAYOUTS drops from eight
entries to four, because the padded/unpadded axis was never a file-version difference — it
was an empty property bag versus a populated one.

-  if (top?.padded) reader.skip(8);
+  // The design's property bag, empty on most designs and sizeable on one that
+  // sets per-page reference ranges. Only the top-level scope carries it.
+  if (top) readPreamble(reader);

Why

The top-level scope, unlike a nested one, carries a preamble before its occurrence count,
and its trailing data holds the design's property bag. Capture writes a REFERENCE_RANGE_BAG
there when the design sets per-page reference ranges — the convention where page 3 numbers
its parts 300–399, page 4 400–499, and so on.

With a populated bag no entry in TOP_LAYOUTS can land on the occurrence count, so
parseHierarchyStream throws, parseDsnFile swallows it, and the design silently falls
back to the inline instance references. That fallback is precisely the behaviour #225 set
out to remove, and designs that set per-page reference ranges are re-annotated designs —
the ones whose occurrence refdes differ most from their inline copies. See #230 for the
full breakdown.

Effect

Measured over a corpus of 1,052 OrCAD Capture designs:

before after
Hierarchy stream parses 637 1,051
Falls back silently 414 0

The one design that still fails has a corrupt OLE container and never reached the
hierarchy parser.

Component recovery on one multi-page board in that corpus that sets per-page reference
ranges, checked against its assembly BOM:

before after
Components 76 336
ICs (U) 1 23
Connectors (J) 5 14

Before the fix the parser reported a single IC carrying a stale inline reference; the
occurrence tree names a different one, which is what the schematic and the BOM show.

Testing

  • npm test: 1,367 passed, 18 skipped, 0 failed (was 1,364 passed — three added cases).
    Every golden DAT comparison unchanged.
  • npm run type-check, npm run lint: clean.
  • All 15 Cadence fixtures still parse, with identical part and net counts. Ten of them
    carry the block with len = 0 — the case padded: true existed for — and read the same
    through readPreamble.
  • The synthetic layout tests are updated: the builder now writes the preamble framing
    unconditionally, as a real file does, and takes the trailing data as a parameter. The two
    padded cases become property-bag cases, plus one for a bag with a narrow auxiliary
    count and one for a bag with a wide occurrence count.

Not in scope

parseDsnFile catches a hierarchy parse failure and continues without a word, which is
what made this a silent wrong answer rather than an error. Raised separately in #230; it
needs a field on ParsedNetlist and that ripples into the universal-format schema, so it
does not belong in this patch.

…ping 8 fixed bytes

`TOP_LAYOUTS`'s `padded` flag skipped 8 bytes before the top-level occurrence
count. Those 8 bytes are a preamble whose trailing data is empty: `FF E4 5C 39`
then a `uint32` length of 0.

The top-level scope, unlike a nested one, carries a preamble before its
occurrence count, and its trailing data holds the design's property bag. Capture
writes a `REFERENCE_RANGE_BAG` there when the design sets per-page reference
ranges. With a populated bag the preamble is `8 + len` bytes, no entry in
`TOP_LAYOUTS` lands on the occurrence count, `parseHierarchyStream` throws, and
`parseDsnFile` silently falls back to the inline instance references -- the
behaviour IntelligentElectron#225 set out to remove.

Replacing the flag with `readPreamble` reads the same 8 bytes when the trailing
data is empty and `8 + len` when it is not, so `TOP_LAYOUTS` drops from eight
entries to four.

Over a corpus of 1052 OrCAD Capture designs, hierarchy streams that parse go
from 637 to 1051; the remaining one has a corrupt OLE container. All 15 Cadence
fixtures parse unchanged.
@vzegnameta

Copy link
Copy Markdown
Contributor Author

Full before/after regression over the corpus, parsing every design end to end on
pristine v1.11.0 and on this branch. 1,052 designs:

designs
Byte-identical component and net counts 953
Gained components (+1,841 components, +815 nets) 30
Net count decreased 68
OLE container unreadable on both sides 1

No design loses a component anywhere in the corpus. Component counts are
non-decreasing on all 1,051 parsable designs.

The 68 net-count decreases are worth spelling out, since "fewer nets" reads like a
regression and is not one. Decomposing every net on those 68 designs into pin sets and
matching each before-net against the after-nets it overlaps:

nets
Unchanged 19,784
Merged into a larger net 1,829
Split 19

Two mechanisms, both corrections:

The merges. Without the hierarchy stream the parser has no canonical net names, so one
net reached through several page-local aliases is counted several times. With the stream
parsing, the aliases resolve to one name and the pins land on one net. Pin-for-pin
conserved — on one design I checked exhaustively, 1,936 distinct pins before and 1,936
after, zero on either side only, and not one before-net split.

The splits. All 19 have the same shape: VSYS becomes VSYS and VSYS_29846433,
FOO_DP becomes FOO_DP and FOO_DP_8034320. Those suffixed names are not invented here
— they come out of the hierarchy stream, and per the note in net-assembly.ts they are
exactly what Cadence's own DAT export writes for two electrically distinct nets sharing a
display name. Merging them by name was the flat path being wrong; keeping them apart is
disambiguateCrossPageNets doing its job, which it cannot do at all when
canonicalNetNames is empty.

Pins appearing on more than one net across those designs goes from 60 to 48, so the
partition also gets marginally cleaner.

A second user-visible symptom of the same fallback

Worth recording because it is not obvious from the diff: the silent fallback also breaks
variant Do-Not-Stuff resolution.

applyVariantDns resolves CIS variant members through occurrenceRefdes, which
expandHierarchy can only produce from a parsed hierarchy stream. When the stream throws,
that map is empty, the variant store cannot resolve its members to reference designators,
and nearly every part comes back not-DNS.

On one affected design, querying its fabrication variant:

v1.11.0 this branch
Components 76 336
Parts flagged dns 8 122

The failure mode downstream is nastier than a wrong count, because dns is serialised as
dns || undefined. With the flag false everywhere the key is simply absent, so it looks
like the field was dropped from the response rather than computed as false — and since
dns is part of the component group key, do-not-stuff parts group together with stuffed
ones carrying the same MPN and value. include_dns=false then filters nothing. A reviewer
reading that output concludes every part is populated.

So this fix also restores per-part stuff state on any design that records variants, not
just reference designators.

Timing

No slowdown. On an affected design, three consecutive full parses: 1088/1252/1234 ms on
v1.11.0 against 971/1079/1101 ms on this branch — about 12% faster, because a failing
design currently walks all eight layouts deep before giving up, and this branch has four
layouts and succeeds on the first.

@valentinozegna

Copy link
Copy Markdown
Contributor

Merged as-is and released in v1.11.1. Thank you for the second pass on this, and for the corpus measurements that made the root cause unambiguous.

Validation before merging: the three new synthetic cases fail on the previous parser and pass here; every local Cadence design parses to byte-identical component and net counts on both; and a hunt across GitHub turned up a redistributable reproducer, the OpenCellular SDR board (CC BY 4.0), which is now the opencellular-sdr fixture with a regression test on its assembly BOM (#232). On the old parser that design lost 242 of the BOM's 1,133 references and reported 323 stale ones; here it reports all 1,133.

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.

Hierarchy stream: the padded top-layout flag is an empty preamble, so any design with a non-empty property bag silently falls back to inline references

2 participants