Experiment making echo updates a separate message

Author: jasongrout

packages/base/src/widget.ts

@@ -224,34 +224,28 @@ export class WidgetModel extends Backbone.Model {
     const method = data.method;
     switch (method) {
       case 'update':
+      case 'echo_update':
         this.state_change = this.state_change
           .then(() => {
-            // TODO: we can either combine state before replacing buffers, or we
-            // can introduce a new echo_state buffer path.
             const state = data.state;
             const buffer_paths = data.buffer_paths ?? [];
-            const buffers = msg.buffers?.slice(0,buffer_paths.length) ?? [];
+            const buffers = msg.buffers?.slice(0, buffer_paths.length) ?? [];
             utils.put_buffers(state, buffer_paths, buffers);
 
-            const echo_state = data.echo_state;
-            const echo_buffer_paths = data.echo_buffer_paths ?? [];
-            const echo_buffers = msg.buffers?.slice(buffer_paths.length) ?? [];
-            utils.put_buffers(echo_state, echo_buffer_paths, echo_buffers);
-
-            if (msg.parent_header && data.echo_state) {
+            if (msg.parent_header && method === 'echo_update') {
               const msgId = (msg.parent_header as any).msg_id;
               // we may have echos coming from other clients, we only care about
               // dropping echos for which we expected a reply
-              const expectedEcho = Object.keys(data.echo_state).filter(
-                (attrName) => this.expectedEchoMsgIds.has(attrName)
+              const expectedEcho = Object.keys(state).filter((attrName) =>
+                this.expectedEchoMsgIds.has(attrName)
               );
               expectedEcho.forEach((attrName: string) => {
                 // Skip echo messages until we get the reply we are expecting.
                 const isOldMessage =
                   this.expectedEchoMsgIds.get(attrName) !== msgId;
                 if (isOldMessage) {
                   // Ignore an echo update that comes before our echo.
-                  delete echo_state[attrName];
+                  delete state[attrName];
                 } else {
                   // we got our echo confirmation, so stop looking for it
                   this.expectedEchoMsgIds.delete(attrName);
@@ -263,14 +257,14 @@ export class WidgetModel extends Backbone.Model {
                       attrName
                     )
                   ) {
-                    delete echo_state[attrName];
+                    delete state[attrName];
                   }
                 }
               });
             }
             return (this.constructor as typeof WidgetModel)._deserialize_state(
               // Combine the state updates, with preference for kernel updates
-              { ...echo_state, ...state },
+              state,
               this.widget_manager
             );
           })

packages/schema/messages.md

@@ -292,35 +292,31 @@ The `data.state` and `data.buffer_paths` values are the same as in the `comm_ope
 
 See the [Model state](jupyterwidgetmodels.latest.md) documentation for the attributes of core Jupyter widgets.
 
-#### Synchronizing multiple frontends: `update` with `echo_state`
+#### Synchronizing multiple frontends: `echo_update`
 
-Starting with protocol version `2.1.0`, `update` messages from the kernel to the frontend can have optional `echo_state` and `echo_buffer_paths` attributes. These are analogous to `state` and `buffer_paths` attributes, but are for echoing state in messages from a frontend to the kernel back out to all the frontends.
+Starting with protocol version `2.1.0`, `echo_update` messages from the kernel to the frontend are optional update messages for echoing state in messages from a frontend to the kernel back out to all the frontends.
 
 ```
 {
   'comm_id' : 'u-u-i-d',
   'data' : {
-    'method': 'update',
+    'method': 'echo_update',
     'state': { <dictionary of widget state> },
     'buffer_paths': [ <list with paths corresponding to the binary buffers> ]
-    'echo_state': { <dictionary of widget state> },
-    'echo_buffer_paths': [ <list with paths corresponding to the binary buffers> ]
   }
 }
 ```
 
-The Jupyter comm protocol is asymmetric in how messages flow: messages flow from a single frontend to a single kernel, but messages are broadcast from the kernel to *all* frontends. In the widget protocol, if a frontend updates the value of a widget, the frontend does not have a way to directly notify other frontends about the state update. The `echo_state` and `echo_buffer_paths` optional attributes enable a kernel to broadcast out frontend updates to all frontends. This can also help resolve the race condition where the kernel and a frontend simultaneously send updates to each other since the frontend now knows the order of kernel updates.
+The Jupyter comm protocol is asymmetric in how messages flow: messages flow from a single frontend to a single kernel, but messages are broadcast from the kernel to *all* frontends. In the widget protocol, if a frontend updates the value of a widget, the frontend does not have a way to directly notify other frontends about the state update. The `echo_update` optional messages enable a kernel to broadcast out frontend updates to all frontends. This can also help resolve the race condition where the kernel and a frontend simultaneously send updates to each other since the frontend now knows the order of kernel updates.
 
-These attributes are intended to be used as follows:
+The `echo_update` messages enable a frontend to optimistically update its widget views to reflect its own changes that it knows the kernel will yet process. These messages are intended to be used as follows:
 1. A frontend model attribute is updated, and the frontend views are optimistically updated to reflect the attribute.
 2. The frontend queues an update message to the kernel and records the message id for the attribute.
-3. The frontend ignores updates to the attribute from the kernel contained in `echo_state` fields, until it gets an `echo_state` update corresponding to its own update (i.e., the [parent_header](https://jupyter-client.readthedocs.io/en/latest/messaging.html#parent-header) id matches the stored message id for the attribute). It also ignores `echo_state` updates if it has a pending attribute update to send to the kernel.
-
-Since the `echo_state` attributes are optional, and not all attributes may be echoed, it is important that only `echo_state` updates are ignored in the last step above, and `state` updates are always applied.
+3. The frontend ignores updates to the attribute from the kernel contained in `echo_update` messages until it gets an `echo_update` message corresponding to its own update of the attribute (i.e., the [parent_header](https://jupyter-client.readthedocs.io/en/latest/messaging.html#parent-header) id matches the stored message id for the attribute). It also ignores `echo_update` updates if it has a pending attribute update to send to the kernel. Once the frontend receives its own `echo_update` and does not have any more pending attribute updates to send to the kernel, it starts applying attribute updates from `echo_update` messages.
 
-For situations where sending back an echo update for an attribute is considered too expensive, we have implemented an opt-out mechanism in ipywidgets. A trait can have the `no_echo` metadata attribute to flag that the kernel should not send back an update to the frontends. We suggest other implementations implement a similar opt-out mechanism.
+Since the `echo_update` update messages are optional, and not all attribute updates may be echoed, it is important that only `echo_update` updates are ignored in the last step above, and `update` message updates are always applied.
 
-TODO: at this point, should we just make this an `'method': 'echo_update'` message. Then the entire message is ignored by older implementations, and processing is almost exactly the same for implementations that do implement it - they just have one or two small changes based on the message type.
+For attributes where sending back an `echo_update` is considered too expensive or unnecessary, we have implemented an opt-out mechanism in the ipywidgets package. A model trait can have the `no_echo` metadata attribute to flag that the kernel should not send an `echo_update` update for that attribute to the frontends. We suggest other implementations implement a similar opt-out mechanism.
 
 #### State requests: `request_state`
 

python/ipywidgets/ipywidgets/widgets/tests/test_set_state.py

@@ -90,12 +90,18 @@ def test_set_state_transformer():
     ))
     # Since the deserialize step changes the state, this should send an update
     assert w.comm.messages == [((), dict(
+        buffers=[],
+        data=dict(
+            buffer_paths=[],
+            method='echo_update',
+            state=dict(d=[True, False, True]),
+        ))),
+        ((), dict(
         buffers=[],
         data=dict(
             buffer_paths=[],
             method='update',
-            state=dict(),
-            echo_state=dict(d=[False, True, False]),
+            state=dict(d=[False, True, False]),
         )))]
 
 
@@ -117,15 +123,14 @@ def test_set_state_data_truncate():
         d={'data': data},
     ))
     # Get message for checking
-    assert len(w.comm.messages) == 1   # ensure we didn't get more than expected
-    msg = w.comm.messages[0]
+    assert len(w.comm.messages) == 2   # ensure we didn't get more than expected
+    msg = w.comm.messages[1]
     # Assert that the data update (truncation) sends an update
     buffers = msg[1].pop('buffers')
     assert msg == ((), dict(
         data=dict(
             method='update',
-            state=dict(),
-            echo_state=dict(d={}, a=True),
+            state=dict(d={}),
             buffer_paths=[['d', 'data']]
         )))
 
@@ -181,8 +186,8 @@ def test_set_state_cint_to_float():
         ci = 5.6
     ))
     # Ensure an update message gets produced
-    assert len(w.comm.messages) == 1
-    msg = w.comm.messages[0]
+    assert len(w.comm.messages) == 2
+    msg = w.comm.messages[1]
     data = msg[1]['data']
     assert data['method'] == 'update'
     assert data['state'] == {'ci': 5}
@@ -265,16 +270,13 @@ def _propagate_value(self, change):
     assert widget.value == 2
     assert widget.other == 11
 
-    # we expect only single state to be sent, i.e. the {'value': 42.0} state
-
-    # TODO: in this case, the echo_state key needs to be set, but we want the
-    # value to come from state. Since we do not want to duplicate the
-    # potentially large value, we just set the echo_state key to null. The
-    # frontend must rank the 'state' value higher than the echo_state value.
-    msg = {'method': 'update', 'state': {'value': 2.0, 'other': 11.0}, 'buffer_paths': [], 'echo_state': {'value': None}}
+    msg = {'method': 'echo_update', 'state': {'value': 42.0}, 'buffer_paths': []}
     call42 = mock.call(msg, buffers=[])
 
-    calls = [call42]
+    msg = {'method': 'update', 'state': {'value': 2.0, 'other': 11.0}, 'buffer_paths': []}
+    call2 = mock.call(msg, buffers=[])
+
+    calls = [call42, call2]
     widget._send.assert_has_calls(calls)
 
 
@@ -293,7 +295,7 @@ class ValueWidget(Widget):
     assert widget.value == 42
 
     # we expect this to be echoed
-    msg = {'method': 'update', 'state': {}, 'echo_state': {'value': 42.0}, 'buffer_paths': []}
+    msg = {'method': 'echo_update', 'state': {'value': 42.0}, 'buffer_paths': []}
     call42 = mock.call(msg, buffers=[])
 
     calls = [call42]
@@ -329,10 +331,14 @@ def _square(self, change):
 
     # we expect this to be echoed
     # note that only value is echoed, not square
-    msg = {'method': 'update', 'state': {'square': 64}, 'echo_state': {'value': 8.0}, 'buffer_paths': []}
+    msg = {'method': 'echo_update', 'state': {'value': 8.0}, 'buffer_paths': []}
     call = mock.call(msg, buffers=[])
+    
+    msg = {'method': 'update', 'state': {'square': 64}, 'buffer_paths': []}
+    call2 = mock.call(msg, buffers=[])
+
 
-    calls = [call]
+    calls = [call, call2]
     widget._send.assert_has_calls(calls)
 
 
@@ -363,5 +369,4 @@ class ValueWidget(Widget):
 
     # a regular set should sync to the frontend
     widget.value = 43
-    # TODO: the original test had a value in echo, which I think is incorrect - no value should be sent back to the frontend
     widget._send.assert_has_calls([mock.call({'method': 'update', 'state': {'value': 43.0}, 'buffer_paths': []}, buffers=[])])

python/ipywidgets/ipywidgets/widgets/widget.py

@@ -572,11 +572,9 @@ def set_state(self, sync_data):
             if echo_state:
                 echo_state, echo_buffer_paths, echo_buffers = _remove_buffers(echo_state)
                 msg = {
-                    'method': 'update',
-                    'state': {},
-                    'buffer_paths': [],
-                    'echo_state': echo_state,
-                    'echo_buffer_paths': echo_buffer_paths
+                    'method': 'echo_update',
+                    'state': echo_state,
+                    'buffer_paths': echo_buffer_paths,
                 }
                 self._send(msg, buffers=echo_buffers)