make motion utils reusable

[Devin T. Currie (DTCurrie)]

Moves the frame-system reconstruction out of the MotionPlanReplayer plugin into $lib/motion, with no behavior change. Stacks on #913, against base branch fix/mimic-joints.

The replayer builds a drawable chain out of RDK's frame_system dump. Nothing about that reconstruction is specific to a plan dump: it reads frames plus parents, and anything that can produce those can be drawn. Relocating it first means the behavior fixes stacked above this PR are readable as behavior fixes rather than as noise inside a rename.

This PR must not merge without #918. The reason is the last entry under ### Why?.

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. This PR: Move the shared plan kinematics into $lib/motion
5. Drive plan joints by RDK's schema order (match rdk joint numbering #918)
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

• MotionPlanReplayer/build-frame-descriptors.ts moves to motion/frameDescriptors.ts, and MotionPlanReplayer/model-joint-columns.ts to motion/jointColumns.ts. Both specs move with them. The fixtures stay under the plugin, because plan-to-snapshots.spec.ts still reads them from there.
• buildFrameDescriptors, buildFrameContexts and buildDescriptors take a new FrameSystemJson (frames plus parents) instead of ParsedPlan. ParsedPlan is structurally a superset, so the replayer passes its parsed plan straight in and no call site changes.
• RawFrame and FrameSystemJson are declared in frameDescriptors.ts. parse-plan.ts keeps its zod-derived equivalent, which is what validates the wire shape, but no longer exports it under the name RawFrame.
• computeJointPose and jointValueAt move out of plan-to-snapshots.ts into a new motion/jointPose.ts, along with a TrajectoryStep alias that descriptorToTransform now uses.
• parse-plan.ts, plan-to-snapshots.ts and world-state-obstacles.ts are repointed at the new paths.
• The three diagnostics inside the moved code say [motion] rather than [MotionPlanReplayer]. The three in code that did not move keep their old prefix.

Nothing consumer-facing moves: plugins/index.ts still exports transformBytesToSnapshots, useMotionPlanReplayer and ResolvePlanSnapshots with the same signatures. $lib/motion is in no barrel and no package.json export path, so none of it is reachable from outside the package.

Why?

Why split the move from the fixes that follow it?

build-frame-descriptors.ts has two real bugs in it, joint columns read in declaration order rather than RDK's schema order, and geometry that fails to decode, and both are fixed in the PRs above this one. Landing a fix inside a rename means a reviewer has to tell "this line moved" from "this line changed" by eye, across a 400 line file. Doing the move alone first makes git pair the files as renames and reduces the next three PRs to the lines that actually changed.

Why FrameSystemJson rather than keeping ParsedPlan?

ParsedPlan also carries trajectory, goals, worldState and obstaclesInWorldFrame, none of which this file reads. Depending on it meant $lib/motion would import from $lib/plugins, inverting the layering. Because TypeScript is structural, narrowing the parameter costs nothing at the call sites.

Why a separate jointPose.ts instead of leaving the kinematics in plan-to-snapshots.ts?

plan-to-snapshots.ts is about assembling Snapshot messages. computeJointPose and jointValueAt are the kinematics underneath that, and nothing in them knows a plan is being replayed: they turn one joint value into one pose. Separating them is what lets the PRs above this one run the same kinematics for a previewed move rather than a replayed plan.

Why do half the diagnostics keep the old prefix?

Because half the code did not move. plan-dropper.ts, useMotionPlanReplayer.svelte.ts and world-state-obstacles.ts stayed in the plugin and are structurally replayer only: a failed host resolvePlanSnapshots callback, a loadPlan error, and a world_state key that only a plan dump carries. Relabeling those to [motion] would cost attribution for nothing, and would read as ambiguous once a second feature in this stack starts emitting into the same console.

Why can this not merge without #918?

It relocates the joint column mapper without changing its behavior, so #913's constraint travels with it: the mapper numbers model.joints in declaration order, which is the wrong order for a branching model such as a two finger gripper. #918 replaces that numbering with the schema walk, and jointColumns.ts is byte identical from #918 to the top of the stack.

Testing

Ran pnpm exec vitest --run: 70 test files, 712 tests, all passing, which is the same tuple as on fix/mimic-joints. pnpm exec svelte-check --tsconfig ./tsconfig.json reports 0 errors and 0 warnings.

This PR adds and removes no tests by design. The spec diff contains no added or removed it or test call site, and both branches carry 70 spec files and 616 test call sites, so the delta is zero by construction rather than by coincidence. If the relocation changed behavior, an existing test should be the thing that says so.

An unchanged count is necessary but not sufficient, since a behavior change nothing covers would not move it. The stronger check is a normalized diff, substituting the renamed identifiers in frameDescriptors.ts back to their old spellings and diffing that against build-frame-descriptors.ts. What survives is the header docblock, the RawFrame and FrameSystemJson declarations that replace the ParsedPlan import, and one console.warn that prettier now fits on a single line because the prefix got shorter. No executable statement differs. jointColumns.ts is byte identical to model-joint-columns.ts, computeJointPose moves with its body unchanged, and jointValueAt is the old inline block with mimic destructured out, which no input distinguishes.

[changeset-bot]

🦋 Changeset detected

Latest commit: c8e86b9

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
Preview removed because the pull request was closed.
2026-08-13 18:46 UTC

[Devin T. Currie (DTCurrie)]

Claude (@claude) review

[claude]

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

---

Review

• Read CLAUDE.md and repository conventions
• Read changed files in full
• Run git diff against base branch (origin/fix/mimic-joints)
• Analyze code correctness, API design, patterns, tests
• Post inline comments
• Post review summary

---

Clean refactoring. The structural reasoning is sound and the implementation is faithful to the PR description. Notes by severity:

Nit

jointPose.ts line 35 — the // offset is in the column's unit… comment sits between the JSDoc block and the function signature. Per the repo's code-comments.md, inline rationale belongs next to the code it explains, not floating before the declaration. Suggestion posted inline: move it as an inline comment on the return mimic ?… line where mimic.offset appears.

frameDescriptors.spec.ts lines 7–10 — fixtures are reached via ../../plugins/MotionPlanReplayer/__tests__/__fixtures__/… (relative) while parsePlan is imported via $lib/… (alias). Not a defect, but the spec is now coupled to a directory two levels away. Comment posted; deferring to the fixture-pass you'd do post-stack is fine.

Observations (not blocking)

RawFrame dual definition. frameDescriptors.ts declares export interface RawFrame { frame_type: string; frame: unknown } and parse-plan.ts holds an identical but unexported zod-derived type. TypeScript's structural typing catches drift at compile time — if RawFrameSchema gains a field, ParsedPlan['frames'] stops being assignable to FrameSystemJson['frames'] and the build breaks. Good design; just noting it so the next reader doesn't wonder why two definitions exist.

jointPose.ts has no dedicated tests. computeJointPose and jointValueAt are tested indirectly via the snapshot specs. Now that they live in a reusable module, direct unit tests would make the contract explicit (particularly the mimic composition and the translational/rotational branch). The PR description defers this intentionally and the count check holds — flagging as a follow-up candidate rather than a blocker.

JointFrameDescriptor not re-exported from jointPose.ts. computeJointPose's parameter type lives in frameDescriptors.ts, so callers that need the type import it from there directly. This is fine — the type belongs to the descriptor layer — but worth knowing when #922 extends usage.

Correctness

• ParsedPlan is a structural superset of FrameSystemJson; no cast is needed at buildFrameDescriptors(plan) in parsedPlanToSnapshots. TypeScript confirms this at build time.
• Module-level quat / vec3 singletons in jointPose.ts are safe: the functions are synchronous, so there is no re-entrant read of a partially-mutated value.
• jointColumns.ts is byte-identical to model-joint-columns.ts — confirmed by the empty diff hunk.
• Console prefix change ([MotionPlanReplayer] → [motion]) is applied only to the three diagnostics in moved code; the three in plan-dropper.ts, useMotionPlanReplayer.svelte.ts, and world-state-obstacles.ts keep their old prefix.

One nit on the comment placement; everything else is correct.

[claude]

The // 'offset' is in... line floats between the JSDoc block and the function signature, which is an unusual position per the repo's code-comments.md rule ("inline rationale next to the code it explains"). The note is about mimic.offset specifically, so it reads better as an inline comment on the return line where it matters:

Suggested change

	// `offset` is in the column's unit, radians or mm, not degrees like the sibling `min`/`max`.
	export const jointValueAt = (
	descriptor: JointFrameDescriptor,
	stepInputs: TrajectoryStep
	): number => {
	const column = stepInputs[descriptor.componentName]?.[descriptor.jointIndex] ?? 0
	const { mimic } = descriptor
	// `offset` is in the column's unit (radians or mm), not degrees like the sibling `min`/`max`.
	return mimic ? mimic.multiplier * column + mimic.offset : column
	}

[claude]

The fixtures use relative paths (../../plugins/MotionPlanReplayer/__tests__/__fixtures__/...) while parsePlan uses an absolute alias ($lib/plugins/MotionPlanReplayer/parse-plan). Mixing is fine functionally, but if the fixtures were co-located under $lib/motion/__tests__/__fixtures__/ the spec would be self-contained and the cross-directory relative hops would disappear. Not blocking — just noting it for the refactor stack if there's ever a dedicated fixture pass.