From 48045b4f083169b6609860b5b84172204b3b2217 Mon Sep 17 00:00:00 2001 From: Eric Date: Fri, 18 Sep 2026 11:20:49 +0200 Subject: [PATCH 1/3] fix: persist spinner state so a reconnecting client sees it correctly Outbox.send_dict previously excluded "spinner" messages from persistence alongside one-off "ui" toasts. But spinner visibility is a *state* (like camera position or background color), not a one-shot notification: if a client's WebSocket connection drops mid-load and reconnects, it needs to see the CURRENT spinner state, not silently miss the "stop" it never received while disconnected and get stuck showing a spinner forever. Workspace.start_spinner/stop_spinner now also pass a stable obj_id="spinner" so each call overwrites the same persisted slot instead of piling up as separate history entries a reconnect could replay out of order. Co-Authored-By: Claude Sonnet 5 --- src/compas_threejs/viewer/outbox.py | 10 +++- src/compas_threejs/viewer/workspace.py | 8 ++++ tests/test_compas_pb_compatibility.py | 5 +- tests/test_spinner_persistence.py | 63 ++++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 tests/test_spinner_persistence.py diff --git a/src/compas_threejs/viewer/outbox.py b/src/compas_threejs/viewer/outbox.py index 4dd5362..dd7aaa3 100644 --- a/src/compas_threejs/viewer/outbox.py +++ b/src/compas_threejs/viewer/outbox.py @@ -52,7 +52,15 @@ def send_dict(self, message: dict, *, workspace_id: str = "main", remove_key=Non binary_data = compas_pb.pb_dump_bts(message) dispatch = message.get("dispatch", "") is_remove = dispatch == "handle_geometry" and message.get("type") == "remove" - persist = dispatch not in ("ui", "spinner") and not is_remove + # "ui" messages are one-off toasts - a client reconnecting later shouldn't see a + # stale one replayed. "spinner" is the opposite: it's a *state* (loading or not), + # same idea as camera position/background color above - if a client's connection + # drops mid-load and reconnects, it needs to see the CURRENT spinner state (still + # loading, or long since finished), not silently miss the "stop" it never received + # while disconnected and get stuck showing a spinner forever. See Workspace. + # start_spinner/stop_spinner, which pass the stable obj_id="spinner" that makes + # each call overwrite the previous one instead of piling up as separate entries. + persist = dispatch != "ui" and not is_remove self.send_bytes(binary_data, obj_id or "", persist=persist, workspace_id=workspace_id, remove_key=remove_key) def forget(self, key, *, workspace_id: str = "main"): diff --git a/src/compas_threejs/viewer/workspace.py b/src/compas_threejs/viewer/workspace.py index a4199ac..d3708d8 100644 --- a/src/compas_threejs/viewer/workspace.py +++ b/src/compas_threejs/viewer/workspace.py @@ -593,9 +593,16 @@ def start_spinner(self, message: Optional[str] = None): message : str, optional A short description to display alongside the spinner, e.g. "Loading model...". """ + # obj_id="spinner" - a fixed, stable key (see Outbox.send_dict's own docstring) - so + # this overwrites the *same* persisted slot every call rather than each start/stop + # piling up as its own separate history entry. Combined with "spinner" no longer + # being excluded from persistence (see Outbox.send_dict), a client that reconnects + # mid-load (or right after) always replays whatever the CURRENT spinner state + # actually is, instead of only ever seeing whichever call happened to reach it live. self.app.outbox.send_dict( {"dispatch": "spinner", "visible": True, "message": message}, workspace_id=self.workspace_id, + obj_id="spinner", ) def stop_spinner(self): @@ -603,6 +610,7 @@ def stop_spinner(self): self.app.outbox.send_dict( {"dispatch": "spinner", "visible": False}, workspace_id=self.workspace_id, + obj_id="spinner", ) def open_in_browser(self): diff --git a/tests/test_compas_pb_compatibility.py b/tests/test_compas_pb_compatibility.py index 8685f8f..ac8d87c 100644 --- a/tests/test_compas_pb_compatibility.py +++ b/tests/test_compas_pb_compatibility.py @@ -54,7 +54,10 @@ def test_outbox_command_roundtrip(self): binary_data, object_id, persist, workspace_id, remove_key, broadcast = outbox._queue[0] self.assertEqual(compas_pb.pb_load_bts(binary_data), command) self.assertEqual(object_id, "") - self.assertFalse(persist) + # "spinner" messages are persisted (see test_spinner_persistence.py) - unlike a + # one-off "ui" toast, spinner visibility is a *state*, so a reconnecting client + # must see its current value rather than never finding out it was ever hidden. + self.assertTrue(persist) self.assertEqual(workspace_id, "main") self.assertIsNone(remove_key) self.assertTrue(broadcast) diff --git a/tests/test_spinner_persistence.py b/tests/test_spinner_persistence.py new file mode 100644 index 0000000..561c6a5 --- /dev/null +++ b/tests/test_spinner_persistence.py @@ -0,0 +1,63 @@ +import asyncio +import unittest + +from compas_threejs.viewer.app import App +from compas_threejs.viewer.server import AppServer + + +class SpinnerPersistenceTest(unittest.TestCase): + """ + A client whose WebSocket connection drops mid-load and reconnects must see the + CURRENT spinner state on connect (loading or not), not silently miss whichever + start/stop call happened to be live at the exact moment it was disconnected and + get stuck showing a spinner forever - the bug this fix addresses. Two pieces: + Outbox.send_dict must actually persist "spinner" messages (it used to exclude them, + same as one-off "ui" toasts, which is right for a toast but wrong for a state), and + start_spinner/stop_spinner must use a stable obj_id so each call overwrites the + same persisted slot rather than piling up as separate history entries a reconnect + could replay out of order. + """ + + def test_send_dict_persists_spinner_but_not_ui_messages(self): + # No server running - Outbox.send_bytes queues rather than broadcasting (see + # test_object_lifecycle_dispatch.py's own note on this), which lets the queued + # (obj_id, persist) pair be inspected directly without a live event loop. + app = App() + + app.outbox.send_dict( + {"dispatch": "spinner", "visible": True}, workspace_id="test", obj_id="spinner" + ) + app.outbox.send_dict({"dispatch": "ui", "text": "hello"}, workspace_id="test") + + persist_by_obj_id = {entry[1]: entry[2] for entry in app.outbox._queue} + self.assertTrue(persist_by_obj_id["spinner"]) + self.assertFalse(persist_by_obj_id[""]) # the "ui" message's obj_id defaults to "" + + def test_workspace_start_and_stop_spinner_use_the_same_stable_obj_id(self): + app = App() + + app.main.start_spinner("Loading...") + app.main.stop_spinner() + + obj_ids = [entry[1] for entry in app.outbox._queue] + self.assertEqual(obj_ids, ["spinner", "spinner"]) + + def test_broadcast_overwrites_the_persisted_spinner_entry_instead_of_piling_up(self): + # Exercises AppServer.broadcast directly - persistence bookkeeping only happens + # there (Outbox.send_bytes just schedules a call to it onto the event loop), and + # it runs fine with no connected clients (persist bookkeeping happens before the + # "any clients?" check), so no live server/WebSocket is needed here either. + server = AppServer() + + asyncio.run(server.broadcast(b"start", obj_id="spinner", persist=True, workspace_id="test")) + self.assertEqual(dict(server.workspace_states["test"]), {"spinner": b"start"}) + + asyncio.run(server.broadcast(b"stop", obj_id="spinner", persist=True, workspace_id="test")) + + # Still exactly one persisted entry, holding the LATEST state - not two entries + # (one stale) that a reconnecting client could replay in the wrong order. + self.assertEqual(dict(server.workspace_states["test"]), {"spinner": b"stop"}) + + +if __name__ == "__main__": + unittest.main() From 84114403b120eb51c34ab8af09b462a007442d89 Mon Sep 17 00:00:00 2001 From: Eric Date: Wed, 23 Sep 2026 08:12:32 +0200 Subject: [PATCH 2/3] docs: add changelog entry for spinner persistence fix Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa7a824..9383276 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Fixed the spinner getting stuck after a WebSocket reconnect: spinner messages are now persisted (under a stable `obj_id="spinner"`) like other viewer state, so a reconnecting client sees the current spinner state instead of missing a "stop" it never received. + ### Removed From 613d5d215893204ae031352e0734d32e8ae9d1df Mon Sep 17 00:00:00 2001 From: Eric Date: Wed, 23 Sep 2026 08:13:09 +0200 Subject: [PATCH 3/3] style: apply ruff format Co-Authored-By: Claude Opus 5.5 --- tests/test_spinner_persistence.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_spinner_persistence.py b/tests/test_spinner_persistence.py index 561c6a5..fcbab8e 100644 --- a/tests/test_spinner_persistence.py +++ b/tests/test_spinner_persistence.py @@ -24,9 +24,7 @@ def test_send_dict_persists_spinner_but_not_ui_messages(self): # (obj_id, persist) pair be inspected directly without a live event loop. app = App() - app.outbox.send_dict( - {"dispatch": "spinner", "visible": True}, workspace_id="test", obj_id="spinner" - ) + app.outbox.send_dict({"dispatch": "spinner", "visible": True}, workspace_id="test", obj_id="spinner") app.outbox.send_dict({"dispatch": "ui", "text": "hello"}, workspace_id="test") persist_by_obj_id = {entry[1]: entry[2] for entry in app.outbox._queue}