fix(web): keep the saved-view restore notice honest about what was lost#881
Open
aelefebv wants to merge 2 commits into
Open
fix(web): keep the saved-view restore notice honest about what was lost#881aelefebv wants to merge 2 commits into
aelefebv wants to merge 2 commits into
Conversation
The heavy applier wrapped the whole restore in try/finally with no catch,
and the scene-call guard rethrows, so one value the wasm deserializer
could not accept aborted the loop. Everything after it was dropped:
global contrast/gamma, T/C/Z, and the camera — which is deliberately the
LAST step. The user was still told the view had loaded, so they sat
looking at their own framing believing it was the author's.
This is not limited to hand-corrupted input. Colormap, blend mode and
render mode are closed serde enums, and the decoder only fills in absent
keys, so a link written by a newer build carries an unknown variant
straight to a command. A share link outliving a deploy is ordinary for
this viewer.
Each scene mutation now goes through one `dispatch` seam that contains a
refusal to that single command, so a stale enum costs its own setting and
the steps after it still land. Whatever was refused is collected and
named in a loading-banner warning ("Could not apply 2 settings from this
view: channel colormap (x2). Everything else was restored."), so a
partial restore can never read as a complete one. A document command the
local engine refused is no longer broadcast to peers.
A fatal error (a trapped or borrow-poisoned instance) still aborts: it
cannot apply anything, and the notice then says the restore stopped
rather than claiming the rest landed.
Tests cover a view carrying an unrecognized colormap landing the camera
and the Z/T/C indices, the unbroadcast refused layout, and the dead-engine
abort with its unfinished-restore notice.
Review of the previous commit found two ways the end-of-apply notice could still say something untrue. The camera step reads live presence before writing the camera back. That read was wrapped in a catch that recorded a skipped setting for anything it threw. But `export_presence` returns a plain string with no refusal path of its own, so it throws only when the wasm instance is gone — the case the apply is supposed to abort on. Instead the apply finished, and the notice told the user everything else had been restored by an engine that could no longer restore anything. The read now goes through the same `dispatch` seam as the writes, so one place decides that fatal aborts, and through `guardedSceneCall`, so the session can raise its engine banner. A layout the recipient does not have was dropped with only a console warning. Paired with any refused command, the user got one warning naming the refusal and ending "Everything else was restored." while the author's layout had gone missing too. The fallback now joins the same skipped list, so the sentence stays true rather than being softened. Both are covered by tests that fail against the previous commit.
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
1 participant
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.
Closes
lucida-52q.The problem
One unrecognised value in a saved view aborted the entire restore partway through. Because the camera was imported last, the usual result was that annotations came back but the viewpoint silently did not — and nothing told you anything had been dropped. The restore reported success.
The fix
Each restore step now goes through a dispatch seam that classifies what happened. An incompatible or recoverable value skips that one command and the restore continues; a genuinely fatal one still aborts. Either way the notice says what was skipped, so a partial restore looks partial.
Two further silent paths were closed in review: the camera import's own presence check now runs through the same seam, and the unknown-layout fallback reports through the shared skip path instead of dropping the step quietly.
How it was verified
Independent review, with the behaviour covered by tests in
applier.test.ts. Gates under Node 24 (matching CI'slts/*): full web suite, lint,tsc -b, andvite buildclean, run again in integration staging on top of the other changes in this batch.🤖 Generated with Claude Code