feat(assembly): warn when a joint primitive drives a placed part - #687
Merged
Conversation
kernelCAD has two conventions for a joint's `origin` and both are correct
within their own path:
.revolute() / .prismatic() / .ball() URDF — `origin` is the parent->child
FRAME OFFSET and the child link is modeled about its OWN origin.
forwardKinematics composes T(o) . M.
.mate() + partRef.connector(...) in-place — `origin` is a PIVOT POINT
and parts are modeled where they sit. composeChildTransform conjugates,
so pose 0 preserves the modeled position.
Mixing them is silent. Model a part in place — `box(...).translate(5, 15, 12)`,
the obvious thing to write — then drive it with `.revolute()`, and the engine
correctly applies link-frame semantics to in-place geometry: the child ships
displaced by the joint origin at EVERY pose, including 0. `evaluate` returns
ok: true with no diagnostics, and the downstream gates then score the
displaced solids — a swept-collision sweep reported clean across 0-90 deg on
a hinge that self-collides at 30 deg. A wrong green.
Add `assembly.joint.child-modeled-in-place` (warn). It fires only when both
hold, which is what keeps false positives down:
1. the joint origin is non-zero — at a zero origin the two conventions
agree exactly, so nothing can be displaced;
2. the child was positioned by the script, via `part(..., { at })` or a
placement transform on its own top-level shape.
A correctly-authored URDF link is modeled about its origin and placed by the
joint alone, so it trips neither. The hint names both exits rather than
assuming which convention the author wanted.
Wiring note: the gate runs BEFORE `validateAssemblyWithMates`'s no-mates early
return. An assembly built from joint primitives has zero mates by
construction, so a gate placed after that return never sees a joint-primitive
scene — which is precisely the case this checks. The first draft sat after it
and was dead code; the test at the bottom of the spec file drives the real
validator entry point so it cannot silently come unwired again.
Tests: 8, covering both positive forms and four false-positive controls
(URDF-authored link, zero origin, mate-based assembly, and a link BUILT from
translated primitives). Stubbing the gate to return [] fails exactly the 3
positive cases plus the wiring test and leaves all 4 controls green.
Catalogue count 247 -> 248; the two count assertions had stale titles saying
245, now matching their assertions.
w1ne
enabled auto-merge
August 25, 2026 15:26
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.
Follow-up to #686, which I closed as a misdiagnosis. This is the fix that case actually needed.
The problem
Two conventions for a joint's
origin, both correct within their own path:originmeans.revolute()/.prismatic()/.ball()T(o) . M.mate()+connector(...)T(o) . M . T(-o)Mixing them is silent. Model a part in place —
box(50, 10, 8).translate(5, 15, 12), the obvious thing to write — then drive it with.revolute(), and the engine correctly applies link-frame semantics to in-place geometry. The child ships displaced by the joint origin at every pose, including 0:evaluatereturnsok: truewith zero diagnostics. Worse, the downstream gates then score the displaced solids:swept-collisionreported a clean 0–90° sweep on a hinge that self-collides at 30° as authored. A wrong green is worse than a red.I hit this with the whole repository in front of me, misread it as a solver bug, and shipped a "fix" that broke the robotics stack. A customer with only the MCP tools has strictly less to go on.
The gate
assembly.joint.child-modeled-in-place(warn), fired only when both hold:part(..., { at })or a placement transform on its own top-level shape.A correctly-authored URDF link is modeled about its origin and placed by the joint alone, so it trips neither condition. The hint names both exits rather than assuming which convention the author meant.
Wiring
The gate runs before
validateAssemblyWithMates's no-mates early return. An assembly built from joint primitives has zero mates by construction, so anything placed after that return never sees a joint-primitive scene — exactly the case this checks. My first draft sat after it and was dead code; the last test in the spec drives the real validator entry point so it cannot silently come unwired again.(Worth noting separately: that early return is also why joint-primitive assemblies get so little validation, and is likely behind
review_cadreportingmechanism: broken/ orphan-part on a sound joint-primitive assembly.)Verification
return []fails exactly the 3 positive cases and the wiring test, leaving all 4 controls green.tests/tree: 3367 pass.src/suites: 2006 pass.tsc --noEmitclean. The one red file locally isviteReviewLiveEndpoint.test.ts—Cannot find module '../lightningcss.darwin-arm64.node', the known darwin-only env issue, unrelated and untouched by this change.