Repository navigation
Resolve FeatureIds for contour plates and profile members - #412
Merged
Merged
Conversation
axisrow
commented
Oct 26, 2026
axisrow
left a comment
axisrow
left a comment
Collaborator
Author
There was a problem hiding this comment.
Review of PR #412 (FeatureIds for contour plates and profile members)
Verdict: Approved. No correctness bugs found.
Highlights verified in the diff vs master:
- BCVIEW02 to BCVIEW03 with the old magic refused is the right call for the added per-element face tables; the round-trip, byte-flip and shared-blob tests were updated with correct offsets, and the snapshot token now covers both tables, so metadata changes invalidate the token even when mesh blobs are shared.
- Admission (admit_face_table) is fail-closed: table length must equal the blob face count, roles unique, bounded and charset-restricted, qualified-length bounds enforced, and the scene-wide metadata budget is separate from mesh bytes.
- Kernel roles survive evaluation: PlateMesh carries a face-id table (rectangle members now emit member: roles per F06), and SolidMesh/SceneMesh preserve the tables through operations, rotations and scene conversion, while mesh dedup still keys on the blob digest alone.
- The topology reconstruction names only from authored wall-role incidence: exactly-three-face vertices, ambiguity refuses the whole element instead of picking a mesh index, grouped walls without point mappings stay unnamed, clipped original features dangle - all matching the #411 contract.
- Parsing and lookup are strict: fid2 only with profile bodies, canonical decimals, bounded indices, cut faces only; numeric addresses stay forbidden.
- Picks and snaps honor include_backfaces, EPS, ranking and truncation; snaps expose named vertices only; unnamed bodies keep the interim mesh-ray path and references stay skipped, as documented.
Notes (no action required): (1) the PR depends on the #411 contract; integrate that first, as the description itself says. (2) In snap(), a named element queried with snap kinds other than vertex now yields no candidates and is not listed in skipped (previously non-box elements were always skipped) - confirm that is the intended skipped semantics. (3) MEMBER_EDGES and MEMBER_VERTICES are exported but unused by the engine in this PR; fine as contract surface.
axisrow
marked this pull request as ready for review
October 26, 2026 13:04
Blogbotana
approved these changes
Oct 26, 2026
11 tasks done
Blogbotana
added a commit
that referenced
this pull request
Nov 1, 2026
The platform no longer knows steel. The steel half of layer-engine (the hooks behind Shape::Discipline and Change::Discipline, the steel readers, writers and definitions, the read-back discipline, the authoring facade and its tests) moves into steel::layer, and the Cargo edge flips: steel depends on beamcraft-layer-engine, no platform crate depends on steel, and steel is kind `discipline` in the F08 table. The registry starts empty; both apps call steel::layer::install() at start (the window reads layers through session). The remaining branching on the kind goes too. steel::engine::part::PartRecord chooses between the per-kind pairs (copy, P11 copy, array, patch with re-evaluation, quantity record), so the CLI copy, array and update-element adapters drop CopiedElement, UpdatedElement and Target. The engine's queries ask ElementKind::is_part and is_linear_member instead of matching Plate, Beam or Column. Stored bytes, refusal codes and command output are unchanged. Left on purpose, with reasons in docs/design/element-interface.md section 8: no platform trait and no ElementPatch split (the UpdateElement/v2 fingerprint), one SCHEMA_DESCRIPTOR, and the /World/Steel prim paths and F06 tokens, which are format, not behaviour. The layer-engine move and the flip are axisrow's series from their fork (axisrow/BeamCraft #394, #398-#402, #404, #407, #411, #412), ported onto master.
Blogbotana
added a commit
that referenced
this pull request
Nov 1, 2026
The platform no longer knows steel. The steel half of layer-engine (the hooks behind Shape::Discipline and Change::Discipline, the steel readers, writers and definitions, the read-back discipline, the authoring facade and its tests) moves into steel::layer, and the Cargo edge flips: steel depends on beamcraft-layer-engine, no platform crate depends on steel, and steel is kind `discipline` in the F08 table. The registry starts empty; both apps call steel::layer::install() at start (the window reads layers through session). The remaining branching on the kind goes too. steel::engine::part::PartRecord chooses between the per-kind pairs (copy, P11 copy, array, patch with re-evaluation, quantity record), so the CLI copy, array and update-element adapters drop CopiedElement, UpdatedElement and Target. The engine's queries ask ElementKind::is_part and is_linear_member instead of matching Plate, Beam or Column. Stored bytes, refusal codes and command output are unchanged. Left on purpose, with reasons in docs/design/element-interface.md section 8: no platform trait and no ElementPatch split (the UpdateElement/v2 fingerprint), one SCHEMA_DESCRIPTOR, and the /World/Steel prim paths and F06 tokens, which are format, not behaviour. The layer-engine move and the flip are axisrow's series from their fork (axisrow/BeamCraft #394, #398-#402, #404, #407, #411, #412), ported onto master.
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.
What this changes
Contour plates, holed/cut plates and profile members previously fell back to an element-only ray hit or could not answer point picks and snaps. Preserve kernel face roles through solid evaluation, scene snapshots and BCVIEW03 bundles, and resolve their semantic faces, original edges and vertices. Ray/point picks and vertex snaps now return resolvable FeatureIds; surface lookup preserves holes and grouped curved walls.
Fixes #190. Depends on the naming/lookup contract in #411; integrate that docs-only PR first. No merge is requested automatically.
How you verified it
Never-crash
Checked blob reads and bounded topology counts; per-query hit retention stays bounded. Semantic metadata has a scene-wide transport budget. No new unsafe or production unwrap/expect/panic.
Provenance
Contributor-original MPL-2.0 code, specification and synthetic geometry. No external assets, commercial-source material or new dependencies.
Anything a reviewer should look at first
BCVIEW03 requires matching producers/consumers. Stored model formats are unchanged. Operation-created edges/vertices and grouped approximation samples without authored point mappings remain unaddressable under the explicit F06 rules. Non-box member axis snaps remain unsupported; unnamed bodies retain the explicit interim mesh-ray fallback. Keep this PR a draft while the contract dependency and required checks are pending.