fix: keep GetSnapshotWithError supported, caller-owned checkpoints, reusable layout worker - #27
Merged
Merged
Conversation
GetSnapshotWithError was added in this unreleased cycle and deprecated again before any release. Keep it as the supported checked capture method; only the context-free GetSnapshot and LoadSnapshot stay deprecated. Name those two explicitly in the docs and record the deprecations in the changelog. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SyRBcc1W1QuigZPCFx26zm
#26 let commit-snapshot take ownership of any snapshot returned by the public createHistorySnapshot, to skip a second clone. That contradicts the contract documented in docs/studio-performance.md (supplied checkpoints are mutable and caller-owned): a consumer that mutated a checkpoint after committing it silently changed what undo restored. createHistorySnapshot is copied on commit again. Ownership transfer is now an internal, non-exported mechanism used only for the editor's own drag/gesture checkpoint, which is never exposed, so the editor keeps the single-clone gain. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SyRBcc1W1QuigZPCFx26zm
The worker path from #25 created, initialized, and terminated a new ELK worker for every layout, so each layout paid 200-400 ms of engine startup and small graphs were slower than on the main thread. If the worker could not load (404, worker-src CSP, cross-origin asset URL), every layout failed with only a console error. - One worker per URL is reused, including for concurrent requests (elk-api tags each message with an id), and released after 60 s idle. - If the worker cannot start or load, the request falls back to the bundled engine on the main thread and the URL is not retried this page session. A worker that crashes after succeeding is discarded and recreated next time. - A 30 s timeout still rejects without falling back, discarding the worker, since rerunning a stuck graph on the main thread would freeze the UI. Verified in Chromium against the real elk worker: initial plus three manual layouts used a single worker with no errors; a broken worker URL was tried once, fell back, and layouts kept working. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SyRBcc1W1QuigZPCFx26zm
rendis
added a commit
that referenced
this pull request
Oct 5, 2026
fix: keep GetSnapshotWithError supported, caller-owned checkpoints, reusable layout worker
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes three problems that came from combining #25 and #26. All three should be fixed before the next release.
1. Keep
GetSnapshotWithErrorsupported (docs only)ExQuantumMachine.GetSnapshotWithErrorwas added during the current unreleased cycle (#22/#23). #25 deprecated it before any release shipped it.experimental/machine.go: theDeprecated:notice is replaced by a note pointing toGetSnapshotContextfor callers that need cancellable lock waiting or callback reentry detection.docs/runtime.md,docs/instrumentation.md: these said "context-free snapshot methods are deprecated", which also coveredGetSnapshotWithError. They now name onlyGetSnapshotandLoadSnapshot.CHANGELOG.md: adds aDeprecatedsection, because Handle snapshot errors and run Studio layout in workers #25's deprecations were not recorded there.2. Public history checkpoints stay caller-owned (Studio)
In #26,
commit-snapshottook ownership of any snapshot returned by the publiccreateHistorySnapshot, to skip a second clone. That breaks the contract #25 documented indocs/studio-performance.md: supplied checkpoints are mutable and caller-owned.Reproduction on current
main: create a snapshot, commit it, mutatesnapshot.machineConfig.id, then undo. Undo restores the mutated value.The fix:
createHistorySnapshotis copied on commit again.state/historyOwnership.ts, which is not re-exported from the package.Tests:
mainand passes here.3. Reusable layout worker with a fallback (Studio)
In #25 every layout created, initialized and terminated a new ELK worker. Each layout spent 200–400 ms starting the 1.6 MB engine, so small graphs were slower end to end than on the main thread. And if the worker could not load (404,
worker-srcCSP, cross-origin asset URL), every layout failed and only logged aconsole.error.autoLayoutWorker.test.tswas rewritten with 10 tests. They cover worker reuse, falling back onerrorandmessageerror, a throwingWorkerconstructor, a missingWorkerglobal, a crash after success, timeout, ELK errors, and idle release.Checked in Chromium against the real ELK worker served by the Vite dev server:
Validation
go build,go vet,go test ./...lint,typecheck,test(330 + 3 tests),build, Stryker dry-run