replayer plan snapshot cleanup

.changeset/eighty-crabs-shave.md

@@ -0,0 +1,5 @@
+---
+'@viamrobotics/motion-tools': patch
+---
+
+Keep a plan's snapshots with the plan when another one is removed

src/lib/plugins/MotionPlanReplayer/MotionPlanReplayerUI.svelte

@@ -97,7 +97,9 @@
 			</div>
 		{/if}
 
-		{#each ctx.plans as plan, i (plan.name)}
+		<!-- Keyed by id: only the upload path rejects a duplicate name, and a repeated key throws
+		`each_key_duplicate` in production builds as well as dev. -->
+		{#each ctx.plans as plan, i (plan.id)}
 			{@const isActive = ctx.activePlanIndex === i}
 			<div
 				class={[

src/lib/plugins/MotionPlanReplayer/__tests__/MotionPlanReplayerUI.svelte.spec.ts

@@ -0,0 +1,46 @@
+import '@testing-library/jest-dom/vitest'
+import { render, screen } from '@testing-library/svelte'
+import userEvent from '@testing-library/user-event'
+import { describe, expect, it, vi } from 'vitest'
+
+import ReplayerUIHarness from './__fixtures__/ReplayerUIHarness.svelte'
+
+// The real panel seeds its position from `useThrelte().dom`, which the global `@threlte/core` mock
+// has no field for, so it throws on mount outside a Canvas.
+vi.mock('$lib/components/overlay/FloatingPanel.svelte', async () => {
+	const MockFloatingPanel = await import('./__fixtures__/MockFloatingPanel.svelte')
+	return { default: MockFloatingPanel.default }
+})
+
+// The `$lib` barrel re-exports `App.svelte` and pulls the whole Threlte component tree in with it.
+// `DashboardPortal` is only a `Portal`, already mocked globally to a passthrough.
+vi.mock('$lib', async () => {
+	const MockDashboardPortal = await import('./__fixtures__/MockDashboardPortal.svelte')
+	return { DashboardPortal: MockDashboardPortal.default }
+})
+
+// useToast requires a `provideToast` ancestor; nothing here checks toast content.
+vi.mock('@viamrobotics/prime-core', async (importOriginal) => ({
+	...(await importOriginal<typeof import('@viamrobotics/prime-core')>()),
+	useToast: () => vi.fn(),
+}))
+
+describe('MotionPlanReplayerUI', () => {
+	// The store spec's duplicate-name case pins the id generator and renders nothing, so this is the
+	// only test that constrains the `{#each}` key.
+	it('renders two plans that share a name as distinct rows', async () => {
+		const user = userEvent.setup()
+		render(ReplayerUIHarness, {
+			props: {
+				plans: [
+					{ name: 'same.json', content: 'content-a' },
+					{ name: 'same.json', content: 'content-b' },
+				],
+			},
+		})
+
+		await user.click(screen.getByRole('radio', { name: 'Motion Plan Replayer' }))
+
+		expect(screen.getAllByRole('button', { name: 'Remove plan' })).toHaveLength(2)
+	})
+})

src/lib/plugins/MotionPlanReplayer/__tests__/__fixtures__/MockDashboardPortal.svelte

@@ -0,0 +1,11 @@
+<script lang="ts">
+	import type { Snippet } from 'svelte'
+
+	interface Props {
+		children: Snippet
+	}
+
+	const { children }: Props = $props()
+</script>
+
+{@render children()}

src/lib/plugins/MotionPlanReplayer/__tests__/__fixtures__/MockFloatingPanel.svelte

@@ -0,0 +1,33 @@
+<script lang="ts">
+	import type { Snippet } from 'svelte'
+
+	interface Props {
+		title?: string
+		isOpen?: boolean
+		children: Snippet
+	}
+
+	// eslint-disable-next-line svelte/no-unused-props -- accepts extra FloatingPanel props
+	let {
+		title = '',
+		isOpen = $bindable(false),
+		children,
+	}: Props & Record<string, unknown> = $props()
+</script>
+
+<div data-testid="floating-panel">
+	{#if title}
+		<h3>{title}</h3>
+	{/if}
+
+	<button
+		aria-label="Close panel"
+		onclick={() => (isOpen = false)}
+	>
+		close
+	</button>
+
+	{#if isOpen}
+		{@render children()}
+	{/if}
+</div>

src/lib/plugins/MotionPlanReplayer/__tests__/__fixtures__/ReplayerHarness.svelte

@@ -0,0 +1,31 @@
+<script lang="ts">
+	/**
+	 * `provideMotionPlanReplayer` reads the world and the relationship registry off Svelte context,
+	 * which only a `setContext` during init can supply, so a spec cannot stand it up in an
+	 * `$effect.root`.
+	 */
+
+	import type { World } from 'koota'
+
+	import { untrack } from 'svelte'
+
+	import { provideWorld, useWorld } from '$lib/ecs'
+	import { provideRelationships } from '$lib/hooks/useRelationships.svelte'
+
+	import type { MotionPlanReplayerContext } from '../../useMotionPlanReplayer.svelte'
+
+	import { provideMotionPlanReplayer } from '../../useMotionPlanReplayer.svelte'
+
+	interface Props {
+		/** Handed the live context and the world it draws into, once, during init. */
+		onReady: (ctx: MotionPlanReplayerContext, world: World) => void
+	}
+
+	const { onReady }: Props = $props()
+
+	provideWorld()
+	provideRelationships()
+	// `untrack`: the context and the world are both stable, so reading `onReady` once at init is
+	// the intent rather than the stale capture the compiler otherwise warns about.
+	untrack(() => onReady(provideMotionPlanReplayer(), useWorld()))
+</script>

src/lib/plugins/MotionPlanReplayer/__tests__/__fixtures__/ReplayerUIHarness.svelte

@@ -0,0 +1,27 @@
+<script lang="ts">
+	/**
+	 * `provideMotionPlanReplayer` publishes to a module-level singleton rather than to Svelte
+	 * context, so it only has to run before `MotionPlanReplayerUI` mounts, not be its ancestor.
+	 */
+
+	import { untrack } from 'svelte'
+
+	import { provideWorld } from '$lib/ecs'
+	import { provideRelationships } from '$lib/hooks/useRelationships.svelte'
+
+	import MotionPlanReplayerUI from '../../MotionPlanReplayerUI.svelte'
+	import { type PlanEntry, provideMotionPlanReplayer } from '../../useMotionPlanReplayer.svelte'
+
+	interface Props {
+		plans?: PlanEntry[]
+	}
+
+	const { plans }: Props = $props()
+
+	provideWorld()
+	provideRelationships()
+	// `untrack`, matching `MotionPlanReplayer.svelte`: `plans` seeds the store once at mount.
+	provideMotionPlanReplayer(untrack(() => plans))
+</script>
+
+<MotionPlanReplayerUI />