refactor: one identity-table builder, and a file load that returns identity

[gsdali]

Closes #7.

Bakeoff posted first, as ecosystem#43 requires, with the failure paths as the first question rather than a checklist item.

The design call: both shapes, because they are complementary

The issue offered a public builder or tables on CADLoadResult, and argued the second is better. It is better at the thing it is better at, and it cannot replace the first.

• Tables on CADLoadResult cannot serve a consumer that never produces one. CADViewportService.load(_:id:transform:), CADViewportService.loadShape(_:id:) and OCCTSwiftUX's ViewportService.loadShape all take an in-memory Shape. Nothing about a file load is involved, so shape 2 alone leaves all three needing a builder, which is how we got three copies.
• A public builder cannot remove the pairing hazard. A consumer handed (bodies, shapes) still pairs them, still has to notice the mismatch, and still has to decide what absent means. Three consumers reached three different answers.

They fix different halves: the builder removes duplicated construction, the load result removes duplicated pairing. Both landed.

What is new

• OCCTSwiftTools.ShapeIdentity: the Shape, its BRepGraph and all three identity tables. init(shape:graph:) uses a graph the caller holds, nil still meaning "tables without durable uids"; init(shape:) mints one, which is the convenience CADKit and UX each hand-rolled. The uid loop is written once, generic over [Shape], which was OCCTSwiftUX's shape rather than the older two copies' three repetitions of it.
• CADLoadResult.identity: [String: ShapeIdentity], keyed by ViewportBody.id, behind includeIdentity: Bool = false on load(from:format:progress:) and loadFromManifest(at:).

Both are additions: a new stored property with a default, and new defaulted parameters. No source break. Adding a defaulted parameter does change the mangled symbol for load, so anything holding a stale object file relinks rather than recompiles; this package is source-distributed with no library evolution, so that is a rebuild, not a break.

Why identity is off by default

BRepGraph.init serialises the whole shape to a BREP string. Measured against a 14-face, 36-edge solid: meshing 9.6ms, BRepGraph(shape:) 5.0ms, of which 3.8ms is that serialisation. OCCTDesignLoop calls load / loadFromManifest from seven non-picking sites (reprojection, batch render, parts extraction) and should not pay for it.

The failure-path divergences, and what the merged version does

Every one of them was on a failure path. The three copies agreed on every success path, which is the configuration one edit away from not agreeing.

Failure case	Tools	CADKit	UX	Merged
graph nil / build failed	uids == nil, shapes populated	same	same	same, kept
Shape has no faces	face table shapes: [], uids: []	same	same	same, kept, now tested
Edge-polyline-only branch	face table forced empty	builds all three in full	builds all three in full	general builder, see below
shapes/bodies count mismatch	not detected: load returned no tables	all identity wiped, bodyShapes and bodyGraphs too	every body's shape nil'd	cannot arise: identity keyed by body id inside the loader
That mismatch's effect on a pick	n/a	pick returns nil	pick returns unenriched whole-body ref	picks resolve normally
Body present, no identity entry	n/a	pick returns nil	unenriched ref	still CADKit's own call, now tested
Shape.fromFace returns nil	compactMap shortens, shifting ordinals	same	same	shared, unchanged, now in one place

The count-mismatch answer is none of the three. It is a symptom, visible only from outside, of a pairing CADFileLoader knows exactly: reloadRobustAndBridge appends to shapes on every input and to bodies only on success. Building identity there, keyed by body id, removes the pairing rather than guarding it. CADKit's guard was also implemented three times for one hazard: loadFile(from:id:) pre-detected it at the call site and addIdentity re-detected it, while rebuildIdentity's five-line wholesale wipe ran against dictionaries resetAllModelState() had emptied on the line above.

The miss-path lesson applied. Phase 3 found two systems disagreeing about whether a miss clears the selection; phase 4 found one acquiring the ability to decline. Here the two consumers disagreed about whether a mismatch drops the pick or falls back to a whole-body ref. That is a selection-mode decision, which the phase 2 bakeoff already ruled belongs above the resolver, so the builder does not decide it. Making identity always trustworthy removes the question from both.

One behaviour change, on a path no test could reach

The bridge's edge-polyline-only branch (taken when mesh(...) returns nil) used to substitute an empty FaceIdentityTable; it now builds the ordinary one. It was the only place any copy varied a table's content, it was asymmetric with the edge and vertex tables built in full on the same branch, and it is inert through picking either way because resolveFace bounds-checks against faceIndices, which is empty there.

Measured while checking whether it was testable: a wire, an edge and a lone vertex all mesh to an empty Mesh rather than to nil, so none of them reaches that branch. It fires only when meshing fails outright, the same condition that triggers the STL/IGES robust reload. It is now driven from tests through an internal edgePolylineOnlyBridge seam, the same treatment (and the same reason) bodyEntries already had in that file.

Also in here

shapeToBodyAndMetadata used to build all three identity tables and discard them. It now skips construction entirely: three fewer shape-map walks per body, on the path every load body takes.

Numbers, measured rather than estimated

Consistent rule throughout: statement lines, excluding blanks, comments and lone punctuation.

Copy	Raw lines	Statement lines
Tools, three private helpers	65	27
CADKit, three statics	51	27
UX, whole ShapeIdentity.swift	68	26

184 raw lines against 80 statements of construction logic. Length overstates by 2.3 times, milder than phases 2 through 4 (five, eight and 43-fold) because these really are near-identical copies rather than a small shared core wrapped in layer-appropriate code.

Correction to my own bakeoff. I posted 21/21/12 and a 3.4x factor from a hand count. Re-measuring mechanically gives 27/27/26: the hand count omitted signature lines, and undercounted UX by forgetting its Tables struct and memberwise init. The 2.3x above is the measured figure.

Actually deleted from this repo, mechanically counted from the diff:

• CADViewportService: 121 statement lines removed, 32 added, net 89. Of those 121, 27 are the verbatim copy of the Tools helpers, another 11 are addIdentity's near-copy of rebuildIdentity's install loop, and the rest is per-call-site table plumbing (four sites, each with three if let table { ... } blocks) that consolidated into one installIdentity(_:).
• CADFileLoader: 74 removed, 73 added. Roughly flat by design, since construction moved out to ShapeIdentity.swift (26 statement lines, 131 raw, mostly documentation) while the identity plumbing came in.

OCCTSwiftUX's copy becomes fully removable

Not touched here, it is another repo. It becomes deletable rather than half-deletable, which is the payoff for landing both shapes:

• ViewportService.loadFile reads result.identity instead of calling ShapeIdentity.tables(for:), asking for it with includeIdentity: true.
• ViewportService.loadShape calls OCCTSwiftTools.ShapeIdentity(shape:).
• buildTopology's paired flag and its shape = paired ? shapes[i] : nil have nothing left to guard, and the guard added in OCCTSwiftUX#30 can go with them.
• ShapeIdentity.Tables is replaced by OCCTSwiftTools.ShapeIdentity, whose three properties are named faces/edges/vertices rather than face/edge/vertex.

One thing to watch on that follow-up: UX currently discards the graph it builds per body, so anything later needing one rebuilds it. OCCTSwiftTools.ShapeIdentity retains it, which was OCCTSwiftCADKit's behaviour and the better of the two. A follow-up should be filed against OCCTSwiftUX rather than done from here.

Verification

$ swift build
Build complete! (1.45s)

$ swift build --build-tests
Build complete! (5.87s)

$ OCCT_SERIAL=1 swift test --parallel --num-workers 1
✔ Test run with 357 tests in 32 suites passed after 1.102 seconds.

Up from 343 in 30. 14 new tests, no deletions. t_sharedFaceBetweenShells_everyPickResolvesToCorrectSubShape is untouched and green.

New coverage goes to the cases the three copies disagreed about, which had none: nil graph, a shape with no faces, a lone vertex, the edge-polyline-only branch in all three of its states, identity keyed by body id, and identity being off by default.

swift-format lint --strict and swiftlint lint --strict both clean (0 violations in 38 files).

The pairing test was mutation-checked. Deliberately mispairing identity inside the loader fails t_identityIsKeyedByBodyIDAndPairedWithTheRightGeometry on its bounds assertion. Worth noting that the pick test alongside it did not catch the mutation: both fixture bodies have six faces and both resolve to some uid, so only the geometric assertion bites. That is why it compares bounding boxes rather than indices.

One thing in the issue that did not survive contact with the code

The issue treats CADKit's count-mismatch guard as the rule to preserve, and OCCTSwiftUX#30 as the fix that brought UX up to it. The guard is correct, but it was defending against a hazard that only exists because the loader declined to say which shape produced which body. Once the loader says so, the guard has no subject. Three implementations of it are gone rather than consolidated.

🤖 Generated with Claude Code

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (17 files)

• Sources/OCCTSwiftTools/ShapeIdentity.swift (new)
• Sources/OCCTSwiftTools/CADFileLoader.swift
• Sources/OCCTSwiftCADKit/CADViewportService.swift
• Tests/OCCTSwiftToolsTests/CADFileLoaderIdentityTests.swift (new)
• Tests/OCCTSwiftToolsTests/ShapeIdentityTests.swift (new)
• Tests/OCCTSwiftCADKitTests/SmokeTests.swift
• docs/CHANGELOG-OCCTSwiftCADKit.md
• docs/CHANGELOG-OCCTSwiftTools.md
• docs/index-OCCTSwiftTools.md
• docs/reference/CADFileLoader.md
• docs/reference/EdgeIdentityTable.md
• docs/reference/FaceIdentityTable.md
• docs/reference/README.md
• docs/reference/ShapeIdentity.md (new)
• docs/reference/VertexIdentityTable.md
• docs/spec/OCCTSwiftTools.md
• okf/components/OCCTSwiftCADKit.md
• okf/components/OCCTSwiftTools.md

---

_{Reviewed by nemotron-3-ultra-550b-a55b:free · Input: 155.5K · Output: 9.5K · Cached: 289.4K}