preview lifecycle

[Devin T. Currie (DTCurrie)]

Adds usePreviewMove, the hook that runs a move preview end to end: send the builtin motion service's plan DoCommand, rebuild the kinematics from frameSystemConfig, spawn the ghost set, and drive it from a TrajectoryPlayer. Stacks on #928. Nothing mounts it yet; the panel in #908 is the consumer.

The lifecycle is where this feature's ordering hazards live, so it lands on its own rather than inside the panel. A spec that calls the hook directly can suspend a request at its await and move the goal, swap the frame system, or close the panel underneath it, which is not something a rendered panel makes reachable.

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. 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. This PR: Run a previewed plan's lifecycle
17. Add move preview to the MoveFrame plugin (Motion plan preview #908)
18. Fill in the frames between planned waypoints (interpolation #930)

Frontend

• usePreviewMove owns the whole sequence: reset, request, parsePlanResult, frameSystemToPlanFrames into buildFrameDescriptors, createForwardKinematics, spawnPreviewGhosts, render step 0, and expose status, message, trajectory, plannedSteps and player.
• It takes world and frames as arguments rather than calling useWorld() and useFrames(), and tears down in an $effect cleanup rather than onDestroy, matching useMoveGhosts.
• resetPreview is the single teardown path: bump generation, abort inFlight, clearPreviewGhosts, drop both step arrays, player.reset(). Its three callers (clear, settle, and the top of requestPreview) differ only in the status and message they leave behind.
• PreviewStatus separates already-at-goal from error, and the two failure branches carry distinct messages: no descriptors at all versus descriptors with nothing a ghost can be made of.
• requestPreview returns without touching status when the client, the service name or the staged goal is missing. The caller is expected to gate the action on all three; nothing has failed.
• ghosts is a plain Map held outside $state, filled in place by spawnPreviewGhosts, so it stays the one handle teardown has across the await. trajectory and playbackFrames are $state.raw: they are replaced wholesale and only playbackFrames.length is read reactively.
• frameIntervalMs divides PREVIEW_DURATION_MS by playbackFrames.length - 1 and floors the result at MIN_FRAME_MS.
• useFrames.svelte.ts publishes the raw frameSystemConfig reply as FramesContext.parts, and exports the FramesContext interface so the hook can name what it is handed.

Why?

Why both an AbortController and a generation counter?

They cover different halves of the same race. inFlight.abort() stops the RPC, which is what you want once the user has dragged the gizmo somewhere else: planning is not cheap on the machine, and a plan for an abandoned goal is work nobody will look at. What abort cannot cover is a response that resolved before it landed. Aborting does nothing to a promise that has already settled, so the continuation past the await still runs, and every line after it writes the state a panel arms Execute preview from.

generation is what that continuation checks. resetPreview bumps it, requestPreview captures it into mine before the request goes out, and the success path returns early when the two no longer match. Without it, clear() leaves status at idle and then a late resolution walks it up to ready, so the panel goes from nothing pending to offering a plan for a goal the user has already replaced, with an execute button behind it.

The catch carries the same guard, for a reason of its own: an aborted request rejects, and reporting a cancellation the user caused as a failed plan is worse than saying nothing. The same is true of a genuine planning failure that belongs to a goal nobody is asking about any more.

Why is the frame system read before the await rather than after?

frames.parts is a getter over a query that refetches on every config revision, so the frame system really can be replaced while a plan is in flight. Reading it afterwards builds the descriptors from kinematics the plan was never computed against. Nothing throws: forward kinematics runs, the ghosts appear, and they stand somewhere the machine has never been, with nothing on screen to say the two halves disagree. Reading it alongside worldState and constraints makes the request's whole input set one snapshot.

Discarding a plan when the frame system changes is the caller's job, not the hook's, which is why invalidateOn is documented as every input the plan was computed from and not just the goal.

Why does FramesContext grow a parts field when it already exposes current?

current is Transform[]: the flattened frames, each with a pose, a parent and a physical object, and no kinematics anywhere. Running forward kinematics in the browser needs the model JSON, and FrameSystemConfig.kinematics on the raw reply is the only place it survives. That is the same route frameSystemToPlanFrames was written against in #923.

The field carries a caveat worth knowing before anything else reads it: the query is disabled in build mode and a disabled query keeps its last data, so a non-empty parts does not mean live. current can meanwhile have merged in or fallen back to config frames. The two can disagree, and parts is the one that has not been merged with anything.

Why is an empty ghost set its own error rather than folded into the empty-descriptor one?

Because they fail for different reasons and only one of them is about the frame system being missing. spawnPreviewGhosts ghosts a descriptor only when it is static, carries geometry, is not hidden, and hangs under a joint this plan actually moves. Joints and geometry-less mounts are most of a descriptor set, so a frame system can produce a hundred descriptors and no ghosts at all. Checking only descriptors.length would report ready with a live scrubber and an armed execute button over an empty scene, and the message would blame a frame system that is plainly there.

Why is "already at the target" not an error?

Because RDK answered, and answered correctly. It seeds a trajectory with the start configuration, so a satisfied goal comes back as two bit-identical steps rather than as an empty plan or a failure. Drawing it would be a scrub between two identical configurations. Giving it its own status lets the panel present it as information, and settle runs the same reset behind it, so trajectory is empty and there is nothing to execute.

Why keep trajectory and playbackFrames apart when they hold the same steps?

trajectory is what the planner said and the only thing execute may ever be handed. playbackFrames is what the scrubber walks. Today applyPlayback calls waypointFrames, which hands back the planned array itself, so the separation buys nothing observable. It stops being free at #930, where interpolatedFrames replaces it and playbackFrames starts holding configurations RDK never planned. Executing those would ask the machine to run through poses no planner checked. Splitting the fields now makes that swap a one-line change in applyPlayback with no way for the interpolated frames to reach execute.

Why does playback pace off a duration rather than a frame rate?

A trajectory carries no timing at all. RDK returns joint configurations and says nothing about how long the move takes, so no frame rate is more correct than any other. "The whole preview takes about four seconds" is at least a consistent claim, and it keeps a two-waypoint plan and a two-hundred-waypoint one comparable to watch. MIN_FRAME_MS floors the interval at 16 ms, so a very dense plan runs longer than the target instead of asking a display for frames it cannot show. The divisor is playbackFrames.length - 1 because playback covers transitions, not frames.

Testing

pnpm exec vitest --run passes 1096 tests across 83 files, up 18 tests and one file from the base branch. pnpm exec svelte-check reports 0 errors and 0 warnings.

previewMoveHarness.svelte.ts holds every doCommand open and never answers on its own: each call parks in pending with its resolve, reject and signal, and the spec decides when, and whether, it comes back. That is what makes the ordering cases real interleavings rather than sequential fakes. The request is genuinely suspended at its await when invalidate() fires, when setParts([]) swaps the frame system, and when dispose() unmounts the hook. The hook runs under a bare $effect.root with createWorld(), which is what taking world and frames as arguments is for.

Each guard in the hook has a test standing on it. The mine !== generation check after the await is held by discards the answer instead of arming the panel with it; the copy of it in the catch by keeps a discarded request’s failure off the panel; inFlight.abort() by cancels the request rather than letting it run to completion, which reads signal.aborted directly; reading frames.parts above the await by draws the plan through the kinematics it was requested with; and the trailing renderStep(0) by places them at step 0 rather than leaving them at the origin, since player.reset() deliberately does not call onStep.

The real gate on drawing is one assertion: moves the ghosts it drew is the only test in the file that fails when applyPreviewStep is reduced to a no-op. Everything else asserts ghost counts and statuses, which a preview that draws every ghost in the wrong place satisfies perfectly well. returns to exactly those poses when scrubbed back to the start covers the other axis, and also pins the direction of the index, since playing the plan backwards lands somewhere else.

What the fixtures are protecting:

• ARM is built out of plan.json's own left-arm model config, on the identity that a part's kinematics is the same ModelConfigJSON a plan dump nests under frame.model. The descriptors under test are a real six-joint arm rather than a stub shaped to pass.
• SHAPELESS is the counterweight: links and joints, no geometry anywhere. It is the only way to reach a large descriptor set and zero ghosts, and so the only way the two error branches can be told apart. They run as an it.each where each case's pattern has to reject the other case's message, so collapsing them onto one string fails.
• PLAN_REPLY's two steps are deliberately different configurations, or isAlreadyAtGoal would divert most of the file. AT_GOAL is the same configuration twice, which is exactly what RDK returns for a goal already met.
• passes through the world state and constraints the panel parsed is the only case that builds its own moveOptions. On the harness default of two undefineds, forwarded and silently dropped look identical, so no other test here can tell them apart.
• The afterEach calling destroy() is load-bearing rather than hygiene. Koota's world pool is 16 and only destroy returns one, so a spec that leaks worlds starves whatever runs later in the same browser context.

The two panel-closed cases are not duplicates of each other: one unmounts before any ghost exists and one after, and only the second gives teardown something to actually clear. Both matter because the world outlives the panel and a preview ghost carries no Name and no ChildOf, which puts a leaked one out of reach of every sweep in the codebase short of a page reload.

One thing this file cannot check: that trajectory and playbackFrames are genuinely distinct. waypointFrames returns the planned array itself, so today they are the same reference and every assertion about one holds for the other. The test that separates them arrives with interpolatedFrames in #930.

[changeset-bot]

🦋 Changeset detected

Latest commit: 914aa57

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-929/
Built to branch gh-pages at 2026-08-13 18:29 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 4m 51s —— View job

---

Review

• Read CLAUDE.md and rules
• Read all changed files
• Analyze diff vs base branch
• Post inline comments for line-level findings
• Post review summary

---

The lifecycle design is solid. The generation + abort dual-guard is correct and well-motivated: abort stops the machine work, generation discards a settled promise that beat the abort. The frames.parts snapshot before the await is the right call for the reason given. The harness design — parking every doCommand in pending until the spec decides — is what makes these ordering tests real interleavings rather than sequential fakes. All five guards in the hook have a test standing on them.

Correctness

Teardown asymmetry — the cleanup $effect (line 269) bumps generation, aborts, and clears ghosts, but does not reset trajectory, playbackFrames, message, or status. Once the root is destroyed nothing can observe those slots, so this is safe today. But resetPreview already assembles the full teardown sequence, and calling it in the cleanup would keep the two paths in sync if teardown responsibilities grow (ref-counted world, external subscribers). Inline comment →

frameName() post-await — frameName() is read after the await to build the already-at-goal message (line 221), and again in the catch (line 254). The generation guard only fires when invalidateOn changes; frameName can change independently without bumping generation, so the message could name the wrong frame. The invalidateOn doc comment says "world state, constraints, service, frame system" but does not name frameName. Inline comment →

Tests

trajectory compared to raw fixture — expect(h.preview.trajectory).toEqual(PLAN_REPLY.plan) compares TrajectoryStep[] against JsonValue[] (line 286). parsePlanResult returns the validated array as-is today, so the assertion holds, but the types don't force it. Asserting against a typed expected value ([{ 'left-arm': [0, 0, 0, 0, 0, 0] }, { 'left-arm': [1, 0, 0, 0, 0, 0] }] satisfies TrajectoryStep[]) would keep the test meaningful if the parse step ever transforms steps. Inline comment →

moveOptions() throwing — there's no test for the case where moveOptions() throws (malformed JSON, as the PR description notes). The catch handles it correctly, but the test file covers every other guard. Worth a single case, particularly to verify the error message is surfaced rather than swallowed.

Nits

• Curly apostrophe in spec line 154 — suggestion posted inline. Fix →
• Math.max(1, playbackFrames.length - 1) guard on line 113 silently saves a negative denominator that would otherwise pass through Math.max(MIN_FRAME_MS, ...) — worth a one-liner comment. Suggestion →
• as unknown as MotionClient in the harness (line 76) — the doCommand variable is already typed to MotionClient['doCommand'], but the cast suppresses any new required properties on MotionClient. A structural { doCommand: MotionClient['doCommand'] } type would be narrower. Inline comment →

[claude]

Typographic apostrophe (curly ') in the test description. This is valid JS but may cause trouble if copy-pasted into a terminal or matched with it.only.

Suggested change

	it('keeps a discarded request’s failure off the panel', async () => {
	it("keeps a discarded request's failure off the panel", async () => {

[claude]

The cleanup does not reset trajectory, playbackFrames, message, or status. That is correct — once the $effect.root is destroyed nothing can observe them — but it means teardown is not symmetric with resetPreview. If the cleanup responsibilities ever widen (e.g. a ref-counted world), resetPreview() here would be safer than another diverging copy of the teardown sequence.

[claude]

frameName() is read after the await. If frameName changes after the plan was sent but is not included in invalidateOn, the already-at-goal message names the current frame rather than the one the plan was computed for. The generation guard only fires if invalidateOn changes, not frameName independently.

This is a caller-contract issue rather than a hook bug, and is worth documenting on invalidateOn:

/**
 * Every input the plan was computed from, not just the goal: world state, constraints, service,
 * frame system, **and frame name**. A change to any discards the plan rather than leaving a ghost
 * of a stale problem.
 */
invalidateOn: () => unknown

[claude]

The Math.max(1, ...) guard is needed because playbackFrames starts at [] and 0 - 1 = -1 would make PREVIEW_DURATION_MS / -1 negative, passing straight through Math.max(MIN_FRAME_MS, ...). Worth a brief comment since the shape of the guard (length - 1) makes it easy to miss:

Suggested change

	const frameIntervalMs = $derived(
	// `length - 1` counts transitions, not frames. Guard against 0 or 1 frames (denominator ≤ 0).
	Math.max(MIN_FRAME_MS, PREVIEW_DURATION_MS / Math.max(1, playbackFrames.length - 1))

[claude]

h.preview.trajectory is TrajectoryStep[] but PLAN_REPLY.plan is JsonValue[]. The assertion works today because the values happen to match structurally, but toEqual will fail if parsePlanResult ever wraps or transforms the steps. Consider asserting trajectory against the expected typed value rather than the raw fixture:

[claude]

as unknown as MotionClient sidesteps the type checker here, which is understandable since only doCommand is needed. The doCommand variable above it is typed to MotionClient['doCommand'], which catches signature drift, but the cast still suppresses any MotionClient required properties added later. A minimal structural type ({ doCommand: MotionClient['doCommand'] }) would catch that without needing the full client shape.