fix: persist spinner state so a reconnecting client sees it correctly

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
 
 

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"):

src/compas_threejs/viewer/workspace.py

@@ -593,16 +593,24 @@ 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):
         """Hides the loading spinner overlay in this workspace's frontend."""
         self.app.outbox.send_dict(
             {"dispatch": "spinner", "visible": False},
             workspace_id=self.workspace_id,
+            obj_id="spinner",
         )
 
     def open_in_browser(self):

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)

tests/test_spinner_persistence.py

@@ -0,0 +1,61 @@
+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()