fix: stream auto-reconnect and preserve scene entities during disconnect

[Devin T. Currie (DTCurrie)]

Closes #584

• Add retryStream utility with exponential backoff (1s–30s)
• Wrap streamEntityChanges and streamSceneChanges in useDrawService with retryStream so the draw backend reconnects automatically after errors
• Guard entity cleanup in useGeometries and usePointclouds: when all queries disappear (transient disconnect), preserve existing ECS entities so the scene doesn't go blank; only destroy entities when the partID changes or other queries remain active

(authored by claude)

[changeset-bot]

🦋 Changeset detected

Latest commit: 4e925fb

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-823/
Built to branch gh-pages at 2026-07-17 18:57 UTC. Preview will be ready when the GitHub Pages deployment is complete.

[Devin T. Currie (DTCurrie)]

Claude (@claude) adversarial review

[claude]

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

---

Adversarial Review

• Read CLAUDE.md and gather context
• Read all changed files in full
• Run git diff origin/main...HEAD
• Analyze and post inline comments
• Post review summary

---

Summary

The retryStream utility is well-structured and the test coverage for the happy-path and backoff mechanics is solid. The preserve-on-disconnect guard in useGeometries/usePointclouds has sound logic. However there are two correctness bugs — one critical — that would make the reconnect behavior worse than the original error-and-stop approach in some real failure scenarios.

---

Critical

Stale entity maps after reconnect (comment)
streamEntityChanges wraps retryStream but doesn't clear transformEntities / drawingEntities before retrying. When the stream reconnects the server re-sends ADDED for all live entities, but processTransformEvent and processDrawingEvent both guard with if (!map.has(uuid)) — so every re-bootstrapped entity is silently dropped. Any mutation, removal, or re-creation that occurred while offline is invisible to the client permanently. The onRetry callback needs to destroy all existing ECS entities and clear both maps before the next run attempt.

---

High

sleep accumulates abort listeners (comment)
{ once: true } only auto-removes the listener when 'abort' fires. When the timer fires first, the listener stays registered on the signal for the lifetime of the connection. With a 30 s max delay and a long outage this can accumulate O(outage_seconds / 30) dangling listeners. Suggestion included.

---

Medium

No CONNECTING state during retries (comment)
After the first error, the UI flips to DISCONNECTED and stays there until the first message arrives on the new stream. There's no indication the client is actively retrying. The initial connection correctly uses CONNECTING; retries should too.

streamSceneChanges errors are silent (comment)
No onRetry is passed, so scene-stream failures never update connectionStatus. The entity stream can show "Connected" while scene metadata is silently failing and retrying.

---

Low / Nits

Preserve-logic condition (comment) — logic is correct but non-obvious; a one-line comment on the if would prevent future misreads.

Missing test cases (comment 1, comment 2) — MAX_DELAY_MS cap is untested; pre-aborted signal is untested; 'stops retrying' test name conflicts with the fact that onRetry is still called once.

No jitter — pure exponential backoff risks thundering-herd if multiple clients reconnect simultaneously after a server restart. Low risk for the current single-robot use case but worth noting.

No tests for the useGeometries/usePointclouds preserve logic — the ECS preservation behavior is the most user-visible part of this PR and has zero unit test coverage.

[claude]

Missing test: delay cap at MAX_DELAY_MS (30 s) is untested.

The exponential doubling is exercised up to 2 s but never hits the 30 s ceiling. Worth one parameterized test that runs enough failures to saturate the cap and verifies onRetry stops receiving larger values.

Also missing: a test for what happens when signal is already aborted before retryStream is called (the while (!signal.aborted) check on the first iteration). The current suite only aborts during an in-flight operation or backoff.

[Devin T. Currie (DTCurrie)]

Claude (@claude) add these tests

[claude]

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

---

• Read CLAUDE.md and test file context
• Read retry-stream implementation
• Add test for MAX_DELAY_MS cap (30s ceiling)
• Add test for pre-aborted signal
• Commit and push

---

Both missing tests have been added in commit 4e925fb:

caps delay at MAX_DELAY_MS (30s) and does not double beyond it — runs 6 consecutive failures to drive the backoff through 1s → 2s → 4s → 8s → 16s → 30s (capped), then asserts the 6th onRetry call received 30_000 rather than 32_000.

does not call run when signal is already aborted before retryStream is called — pre-aborts the controller before passing the signal, asserts run is never invoked (tests the while (!signal.aborted) guard on first entry).
· Branch

[claude]

Claude finished Micheal Parks (@micheal-parks)'s task in 3m 3s —— View job

---

Analysis: resetQueriesOnDisconnect vs hook-level preserve logic

• Read CLAUDE.md and gather context
• Read usePointclouds.svelte.ts and useGeometries.svelte.ts
• Fetch resetQueriesOnDisconnect from viam-svelte-sdk
• Evaluate the approach

---

The suggestion is sound — here's a full breakdown.

What resetQueriesOnDisconnect does

In @viamrobotics/viam-svelte-sdk, when a part disconnects the SDK calls:

await queryClient.resetQueries({ queryKey: ['viam-svelte-sdk', 'partID', partID] })

This wipes query.data → undefined for all queries on that part. That's what triggers the "all queries disappear" path in useGeometries / usePointclouds. With resetQueriesOnDisconnect: false, query.data stays populated, so the $effect tracking that data never sees a change and the ECS entities naturally remain.

SDK status

PR 152 in viamrobotics/viam-svelte-sdk merged on 2026-07-01 and shipped as v1.2.3. The project currently depends on v1.2.2 (installed). resetQueriesOnDisconnect is not available yet in the installed version, so any PR implementing this approach requires a package bump.

Trade-offs

resetQueriesOnDisconnect: false (Michael's suggestion)

• ✅ Single option, set once in the provider — no per-hook preserve logic
• ✅ Hooks stay simple; future hooks work correctly for free
• ✅ Uses an SDK feature designed exactly for this use case
• ⚠️ This is a global option: ALL queries for the part keep stale data on disconnect (arm positions, camera frames, etc.), not just geometry/pointcloud. For display-only data that's fine; for anything action-driven it means stale state is shown until reconnect
• ⚠️ @viamrobotics/motion-tools is a library — consumers set up their own ViamProvider. Setting resetQueriesOnDisconnect: false correctly would need to be documented or threaded through Visualizer props, otherwise library consumers silently get the blank-scene behavior back

Hook-level preserve logic (current PR)

• ✅ Self-contained: works regardless of consumer's ViamProvider configuration
• ✅ Only geometry/pointcloud entities are preserved — no unintended stale state elsewhere
• ❌ Has to be replicated in every hook that manages ECS entities from queries (fragile, as noted)
• ❌ The condition (queryPartID !== currentPartID || anyQueriesActive) is subtle and easy to break

Recommendation

Michael's approach is architecturally cleaner. For the standalone dev app, bumping to 1.2.3 and passing options={{ resetQueriesOnDisconnect: false }} to ViamProvider in +layout.svelte is the right move. For the library use case, either:

1. Thread resetQueriesOnDisconnect through the Visualizer props and document the recommended value, or
2. Keep the hook-level guard as a safe default for consumers who don't set the option

Without addressing the library surface, this is a partial fix: the standalone app works, but any third-party consumer of @viamrobotics/motion-tools who sets up their own ViamProvider with defaults gets blank scenes on disconnect.

---

[Devin T. Currie (DTCurrie)]

What resetQueriesOnDisconnect does

In @viamrobotics/viam-svelte-sdk, when a part disconnects the SDK calls:

await queryClient.resetQueries({ queryKey: ['viam-svelte-sdk', 'partID', partID] })

This wipes query.data → undefined for all queries on that part. That's what triggers the "all queries disappear" path in useGeometries / usePointclouds. With resetQueriesOnDisconnect: false, query.data stays populated, so the $effect tracking that data never sees a change and the ECS entities naturally remain.

SDK status

PR 152 in viamrobotics/viam-svelte-sdk merged on 2026-07-01 and shipped as v1.2.3. The project currently depends on v1.2.2 (installed). resetQueriesOnDisconnect is not available yet in the installed version, so any PR implementing this approach requires a package bump.

Trade-offs

resetQueriesOnDisconnect: false (Michael's suggestion)

• ✅ Single option, set once in the provider — no per-hook preserve logic
• ✅ Hooks stay simple; future hooks work correctly for free
• ✅ Uses an SDK feature designed exactly for this use case
• ⚠️ This is a global option: ALL queries for the part keep stale data on disconnect (arm positions, camera frames, etc.), not just geometry/pointcloud. For display-only data that's fine; for anything action-driven it means stale state is shown until reconnect
• ⚠️ @viamrobotics/motion-tools is a library — consumers set up their own ViamProvider. Setting resetQueriesOnDisconnect: false correctly would need to be documented or threaded through Visualizer props, otherwise library consumers silently get the blank-scene behavior back

Hook-level preserve logic (current PR)

• ✅ Self-contained: works regardless of consumer's ViamProvider configuration
• ✅ Only geometry/pointcloud entities are preserved — no unintended stale state elsewhere
• ❌ Has to be replicated in every hook that manages ECS entities from queries (fragile, as noted)
• ❌ The condition (queryPartID !== currentPartID || anyQueriesActive) is subtle and easy to break

Recommendation

Michael's approach is architecturally cleaner. For the standalone dev app, bumping to 1.2.3 and passing options={{ resetQueriesOnDisconnect: false }} to ViamProvider in +layout.svelte is the right move. For the library use case, either:

1. Thread resetQueriesOnDisconnect through the Visualizer props and document the recommended value, or
2. Keep the hook-level guard as a safe default for consumers who don't set the option

Without addressing the library surface, this is a partial fix: the standalone app works, but any third-party consumer of @viamrobotics/motion-tools who sets up their own ViamProvider with defaults gets blank scenes on disconnect.

Claude (@claude) lets make these changes. I think we go with option 2 for now.

[claude]

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

---

Implementing resetQueriesOnDisconnect changes (Option 2)

• Read CLAUDE.md and gather context
• Read relevant files (+layout.svelte, package.json, changed source files)
• Verified resetQueriesOnDisconnect API — options={{ resetQueriesOnDisconnect: false }} on ViamProvider
• Bump @viamrobotics/svelte-sdk to v1.2.3 and install
• Set resetQueriesOnDisconnect: false in ViamProvider in +layout.svelte
• Fix critical bug: clear stale entity maps in onRetry before reconnect
• Fix memory leak: remove abort listener when timer fires in sleep
• Add CONNECTING status during retries
• Fix silent streamSceneChanges errors
• Run pnpm lint:eslint ✅, pnpm check ✅ (browser tests require Playwright install, pre-existing env constraint)
• Commit and push

---

Changes made

package.json + pnpm-lock.yaml — bumped @viamrobotics/svelte-sdk from 1.2.2 → 1.2.3

src/routes/+layout.svelte — added options={{ resetQueriesOnDisconnect: false }} to ViamProvider. With this, query data stays populated across disconnects in the standalone dev app, so useGeometries / usePointclouds never see empty data and their ECS entities remain naturally. The hook-level preserve logic stays in place as a safe default for library consumers who configure their own ViamProvider.

src/lib/retry-stream.ts — fixed sleep() memory leak. { once: true } only removes the listener when 'abort' fires; if the timer fires first the listener was orphaned for the lifetime of the connection. Now the timer handler explicitly calls removeEventListener before resolving.

src/lib/plugins/DrawService/useDrawService.svelte.ts:

• Extracted clearEntities() helper that destroys all transform/drawing ECS entities and clears both maps + serverRelationships.
• onRetry for streamEntityChanges now sets connectionStatus = CONNECTING (was DISCONNECTED) and calls clearEntities(). Without the clear, the server's ADDED re-bootstrap after reconnect was silently dropped for every UUID still in the maps — leaving the client permanently stale after any reconnect where entities changed on the server while offline.
• Added onRetry warn log to streamSceneChanges so scene-stream failures are no longer silently swallowed.

[Devin T. Currie (DTCurrie)]

Claude (@claude) resolve or hide outdated comments

[claude]

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

---

Resolved outdated review comments

• Read CLAUDE.md
• List review threads on PR fix: stream auto-reconnect and preserve scene entities during disconnect #823
• Resolve threads for issues fixed in commit 7120c0e
• Leave open any threads still relevant

Resolved (6 threads):

• Memory leak in sleep — fixed
• Stale entity maps after reconnect — fixed (clearEntities() in onRetry)
• Silent streamSceneChanges errors — fixed (warn log added)
• No CONNECTING state during retries — fixed
• Preserve-logic condition comment — addressed (comment block added above the if)
• Micheal Parks (@micheal-parks)'s resetQueriesOnDisconnect suggestion — addressed (SDK bumped to 1.2.3, options={{ resetQueriesOnDisconnect: false }} set in +layout.svelte)

Still open (2 threads):

• Missing test for MAX_DELAY_MS cap (30 s ceiling untested)
• Misleading test name ('stops retrying when signal is aborted' — onRetry IS called once, test name implies otherwise)

[viam-overwatch]

Hey Devin T. Currie (@DTCurrie) — this PR has been approved and CI has been green for 4+ business days. Ready to merge?

Auto-comment from overwatch. Will not re-nudge for 7 days.

[claude]

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

---

• Read test file and rename the test for clarity
• Rename committed to branch (commit 6b82207)
• Verified: file content on remote branch matches the rename

Done

Renamed 'stops retrying when signal is aborted' → 'calls onRetry but does not retry run when signal is aborted during backoff' on line 46 of src/lib/__tests__/retry-stream.spec.ts.

The new name accurately reflects what the test asserts: onRetry is called once (before the backoff sleep) but run is not called a second time because the abort fires during the backoff window.

[claude]

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

---

• Read CLAUDE.md and gather context
• Read retry-stream.ts and retry-stream.spec.ts
• Add MAX_DELAY_MS cap test
• Add pre-aborted signal test
• Run tests — 8/8 passed
• Commit and push

Done

Added two tests to src/lib/__tests__/retry-stream.spec.ts (commit 4e925fb):

'caps delay at MAX_DELAY_MS (30s) and does not double beyond it' — runs run through 6 failures advancing fake timers through the full exponential sequence (1s→2s→4s→8s→16s→30s). Asserts that the 5th onRetry call receives 16_000 and the 6th receives 30_000 (not 32_000), proving the Math.min(delay * 2, MAX_DELAY_MS) cap works.

'does not call run when signal is already aborted before retryStream is called' — aborts the controller before calling retryStream, then asserts run was never called. Covers the while (!signal.aborted) guard on the first iteration.