match rdk joint numbering

[Devin T. Currie (DTCurrie)]

Numbers a model's joints the way RDK does, so a trajectory step drives the joint it was meant to. Stacks on #917.

modelJointColumns read model.joints in declaration order. RDK does not: NewModelWithMimics seeds a model's input schema by walking the model's own internal frame system breadth-first from its root, visiting each node's children in sorted order and skipping mimic frames while it numbers. The two agree only for a model whose links and joints happen to be declared down its own chain, which an xArm6 and every capture in this repo are. For anything branched, each joint is driven from a different joint's value and the arm folds through itself.

Stack

1. Read a plan model's output frame from where RDK writes it (read plan model output frame #910)
2. Geometry decode fixes (geometry decode fixes #912)
3. Mimic joint fixes (mimic joint fixes #913)
4. Move the shared plan kinematics into $lib/motion (make motion utils reusable #917)
5. This PR: Drive plan joints by RDK's schema order
6. Infer an untyped geometry from its dimensions (infer collisions #919)
7. Read mesh data in both the shapes RDK sends it (match rdk mesh data decoding #920)
8. Keep a plan's snapshots with the plan when another is removed (replayer plan snapshot cleanup #921)
9. Share trajectory playback between the replayer and the move panel (make trajectory playback reusable #922)
10. Draw a part's configured geometry even when it has a kinematic model (reconstructed flattened frames from rdk #923)
11. Place a plan's frames by running its kinematics on the client (forward kinematics for player #924)
12. Budget preview frames per joint unit (budget frame movement between waypoints #925)
13. Report a previewed collision as a warning about the move (handle preview collisions #926)
14. Ask RDK to check the start state before executing a previewed plan (add do command wiring for planning and execution #927)
15. Draw a previewed plan as ghost geometry (add preview ghosts #928)
16. Run a previewed plan's lifecycle (preview lifecycle #929)
17. Add move preview to the MoveFrame plugin (Motion plan preview #908)
18. Fill in the frames between planned waypoints (interpolation #930)

Frontend

• modelJointColumns(model, modelName) takes the whole model config rather than just its joints array, because the walk needs the links: a joint's parent is usually a link, so the chain cannot be reconstructed from the joints alone. It returns { order, columns } instead of a bare Map.
• order is every joint id in schema order, mimics included. columns is keyed by joint id, and a mimic's entry addresses its source's column plus the linear map to apply, unchanged from before.
• The walk builds one childrenOf map over links and joints together, sorts each sibling list, and visits breadth-first from MODEL_ROOT, the model's internal world. Numbering then runs over order, so a mimic shifts every joint below it rather than every joint declared after it.
• A node whose parent is not itself a declared node roots at MODEL_ROOT, matching buildModelFrameSystem. Only a parent cycle can now leave a joint unreached; those are appended in declaration order and warned about by name.
• nodeName maps both undefined and '' to undefined, and every id and parent read in this file goes through it. It is exported, because soleLeafOf needs the same filter.
• soleLeafOf in frameDescriptors.ts now filters ids and parents through nodeName. An unnamed node previously counted as a second unclaimed leaf and demoted a model that has a real sole leaf to "more than one".
• buildFrameContexts reads order.at(-1) when deciding where a model with no declared end effector hangs its tool, where it previously read model.joints.at(-1).
• ModelJson and ModelNodeJson are declared in jointColumns.ts. frameDescriptors.ts drops the local ModelNode it was declaring and types modelOf, soleLeafOf and modelOutputFrame against ModelJson instead of Record<string, unknown>.
• Two fixtures in frameDescriptors.spec.ts gained the links and parent fields a real model carries. They declared bare joints with no chain, which only passed because declaration order happened to be the answer.

Why?

Why is this one function and not two?

The two halves are separable to describe and not to implement. Skipping mimics is what makes the columns contiguous, and the walk is what decides who gets skipped past. Written apart, either one alone produces a wrong answer that looks right on RDK's own test models, because in both of them the mimic is the last joint and nothing shifts behind it.

Why does a node with an unknown parent root at the model rather than count as disconnected?

Because that is what RDK does, and the difference is a different set of columns rather than a warning. buildModelFrameSystem seeds its queue with every child whose parent is absent from the transforms it collected, and attaches those to fs.World(). So a joint parented to a name that does not exist is an ordinary root-level frame with a real position in the walk, and it sorts against the model's actual base. Reading it as disconnected instead pushed it to the end of the order.

I verified this rather than reasoning about it. The exact model this PR's test uses, given an output_frames so it clears RDK's single-end-effector check, builds through UnmarshalModelJSON and reports MoveableFrameNames() of [orphan attached]. The test asserts orphan first.

Why filter an empty id instead of treating it as a name?

LinkConfig.ID and JointConfig.ID are Go strings with no omitempty, so a node that declares no id arrives as "", not as a missing key. Left in the tree, every unnamed node collides on that one key and claims the others' children. In soleLeafOf the failure is quieter and worse: an unnamed node is unclaimed by definition, so it reads as a second leaf, and a model that has exactly one real leaf stops resolving its end effector and falls through to the last-joint rule instead. Both readers now go through nodeName, which is why it is exported rather than local.

Why does the end-effector fallback change at all?

That branch answers "what hangs off the last joint" for a model that declares no output frame, and model.joints.at(-1) is declaration order. The tool hangs off the joint the walk ends on. The branch is a floor rather than a live path, since a model with neither an output frame nor a single leaf is one RDK will not marshal, but the two readings disagree the moment a model branches and the old one had no way to be right.

Why keep a fallback for unreached joints when RDK refuses to build a cycle?

ErrCircularReference means the only shape that can strand a joint here cannot come off a real machine, so this is a floor under malformed input rather than a guess about a real one. It stays because the alternative is dropping the joint, which silently takes its entire subtree out of the drawing. Appending the strays in declaration order keeps them addressable and the warning names each one, the model, and the fact that their columns are a guess.

Testing

pnpm exec vitest --run passes 727 tests across 70 files, up 15 tests and no new files from the base branch. pnpm exec svelte-check reports 0 errors and 0 warnings.

None of the four captured plan dumps contains a mimic joint, and on all 29 model frames across them the walk order, the declaration order and RDK's own internal_fs agree. Nothing in this repo's fixtures renders differently, so all of the new coverage is synthetic and the fixtures are built to be shapes a machine could actually send.

RDK's answers here were taken from RDK rather than derived by reading it. A small Go program builds each fixture through UnmarshalModelJSON and prints MoveableFrameNames(), which is the schema order this function reproduces:

fixture	RDK
branched, declared out of order	[alpha_joint zeta_joint beta_joint]
the same with a mimic in one branch	[alpha_joint zeta_joint beta_joint], 3 DoF for 4 joints
a joint parented to an undeclared name	[orphan attached]
any of the above without output_frames	need exactly one end effector

That last row is why the invented models declare an output_frames this function never reads. Each has two leaves, and ParseConfig fails a two-leaf model with no declared end effector, so without the field they would be shapes no robot can produce. RDK also rejects a mimic joint that declares its own limits, which is worth knowing when writing one by hand.

The real gate is a model whose declaration order is not its chain order: two branches off a shared base, zeta declared before alpha, with a joint under each branch. gamma_joint is the one that earns its place. With a single joint below the last-sorted branch, a depth-first walk produces the same sequence and the fixture pins nothing; with two, a column index depends on breadth-versus-depth rather than only on the order array.

hangs the camera off the last joint of the walk, not the last one declared is the only test that separates order.at(-1) from model.joints.at(-1). Without it the entire frameDescriptors.ts half of this PR could be reverted with the whole suite still green: exactly one test reached that line, and its model produced an order of one element, where every reading agrees.

The pre-existing mimic tests were rewritten through a chain() helper that parents each joint to the one before it. That is what lets them keep their old expected values: it holds schema order and declaration order equal so the mimic rules are the only thing under test.

The rest of what the new fixtures pin, each of which a mutation could previously get away with:

• An unknown parent rooting at the model, rather than being pushed to the end of the order.
• Breadth-first rather than depth-first traversal, which is queue.push versus queue.unshift.
• nodeName's empty-string half, which had no coverage at all: it could be replaced with the identity function, or deleted and inlined, with nothing failing.
• The unreached-joint list keeping declaration order, and coming after everything the walk did reach.
• The warning text in full, so it cannot lose its [motion] prefix or the explanation that the columns are a guess.
• That a rooted-at-model joint produces no warning, since nothing about it was guessed.

[changeset-bot]

🦋 Changeset detected

Latest commit: 491f878

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@viamrobotics/motion-tools	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

PR Preview Action v1.8.1
🚀 View preview at https://viamrobotics.github.io/visualization/pr-preview/pr-918/
Built to branch gh-pages at 2026-08-13 18:47 UTC. Preview will be ready when the GitHub Pages deployment is complete.

[Devin T. Currie (DTCurrie)]

Claude (@claude) review

[claude]

Claude finished Devin T. Currie (@DTCurrie)'s task in 4s —— View job

---

Review

• Read CLAUDE.md and gather context
• Read modified files in full
• Diff against base branch
• Analyze correctness, tests, API design, and conventions
• Post review

---

Correctness ✅

The BFS implementation in modelJointColumns correctly reproduces RDK's NewModelWithMimics logic:

• Breadth-first, alphabetically sorted siblings — childrenOf.values() are sorted before the walk, matching buildModelFrameSystem's sorted-child traversal.
• Unknown parent roots at model — nodeName(node.parent) checked against declared; anything absent resolves to MODEL_ROOT = 'world', which is exactly what buildModelFrameSystem does when seeding unattached frames onto fs.World().
• Empty-string id/parent — nodeName correctly collapses '' and undefined to undefined. In soleLeafOf, the old node.parent ?? [] included '' as a claimed parent (since '' ?? [] returns '', not []), which meant a node with id: '' was never marked claimed and appeared as a spurious leaf. The new nodeName(node.parent) ?? [] closes that.
• Mimic resolution — resolveMimic correctly composes the linear maps (a = m₁(m₂c + o₂) + o₁) and detects cycles via visited. The two-pass column assignment (non-mimics first, mimics second) correctly produces contiguous indices even when a mimic sits in the middle of a branch.
• order includes mimics — documented on ModelJointColumns.order, and the end-effector fallback order.at(-1) is semantically correct: a mimic owns a frame, and soleLeafOf/modelOutputFrame already ruled out every cleaner path before reaching this branch.

One trace I checked by hand: the hangs the camera off the last joint of the walk, not the last one declared test — aux_joint sorts before gripper_rot alphabetically, so the walk visits aux_joint first. order.at(-1) is gripper_rot, whose first child in childMap is arm:extra_link (insertion order from parents). With model.joints.at(-1) (the old code), you'd get aux_joint, which has no children in childMap, leaving the camera parented to the bare model frame. The new code resolves correctly.

Tests ✅

Excellent coverage. Several design choices worth calling out positively:

• chain() helper keeps mimic tests orthogonal to ordering: by parenting each joint to the previous one, declaration order and schema order are identical, so the mimic rules are the only thing under test. This is exactly the right factoring.
• gamma_joint discriminator — the PR description explains why one joint below the last-sorted branch is the minimum fixture that separates BFS from DFS. The test finishes a whole breadth before descending directly pins queue.push vs queue.unshift.
• console.warn pinning — warning text is fully matched (not just stringContaining), so it can't silently lose its [motion] prefix or the explanation that columns are a guess.
• Orphan-produces-no-warning test — confirms that a joint rooting at the model via an unknown parent is not warned about, since nothing about its position was guessed. This is a meaningful semantic distinction from the cycle case.
• Fixture JSON files are now consumed as whole ModelJson objects rather than their .joints slices, which is a better-aligned test of the actual call site.

API Design

JointJson.id changed from id: string (required) to id?: string (optional, inherited from ModelNodeJson). This is a breaking type change for external consumers who import JointJson. However, jointColumns.ts is not re-exported at the package boundary (src/lib/index.ts has no reference to it), so the patch changeset bump is correct — no downstream published type surface changed.

The new nodeName export is necessary for soleLeafOf to call it across the module boundary. Exporting it is the minimal change; it doesn't need to be re-exported at the package level.

Nits (inline comments posted)

• chain() at line 19 and indices at line 24 both use ! non-null assertions on fields that JointJson now types as optional. Safe for all current callers, but the assertions will mask TypeScript errors if a future test passes a model with unnamed joints through either helper. No change required — just worth being aware of when extending these tests.

Summary

The algorithm is correct, well-tested, and the PR description's claim that "RDK's answers were taken from RDK rather than derived by reading it" is the right epistemic standard for this kind of reimplementation. The gamma_joint fixture is the critical one and it's there. Nothing blocks merge.

[claude]

Nit: joints[index - 1]!.id asserts non-null on a field that is now optional (JointJson extends ModelNodeJson where id?: string). Every current chain() call passes joints with explicit string ids, so this is safe in practice, but it's worth noting the gap — calling chain({ mimic: { joint: 'a' } }) (no id) would silently produce parent: undefined for the next joint, which nodeName then maps to MODEL_ROOT. Not a bug today; just fragile for future test additions.

[claude]

Same pattern: j.id! asserts non-null on an optional field. Safe because every caller of indices passes models whose joints have explicit ids, but the non-null assertion will mask a TypeScript error if a future test passes a model with unnamed joints through this helper.