Merge pull request #3394 from jasongrout/echostate

Echo state updates in a backwards-compatible way

packages/base-manager/test/src/manager_test.ts

@@ -180,7 +180,7 @@ describe('ManagerBase', function () {
           },
         },
         metadata: {
-          version: '3.0.0',
+          version: '2.1.0',
         },
       });
       expect(model.comm).to.equal(comm);
@@ -243,7 +243,7 @@ describe('ManagerBase', function () {
         },
         buffers: [new DataView(new Uint8Array([1, 2, 3]).buffer)],
         metadata: {
-          version: '3.0.0',
+          version: '2.1.0',
         },
       });
       expect(model.comm).to.equal(comm);

packages/base/package.json

@@ -26,6 +26,7 @@
     "test:coverage": "npm run build:test && webpack --config test/webpack-cov.conf.js && karma start test/karma-cov.conf.js",
     "test:unit": "npm run test:unit:firefox && npm run test:unit:chrome",
     "test:unit:chrome": "npm run test:unit:default -- --browsers=Chrome",
+    "test:unit:chrome:debug": "npm run test:unit:default -- --browsers=Chrome --single-run=false",
     "test:unit:default": "npm run build:test && karma start test/karma.conf.js --log-level debug",
     "test:unit:firefox": "npm run test:unit:default -- --browsers=Firefox",
     "test:unit:firefox:headless": "npm run test:unit:default -- --browsers=FirefoxHeadless",

packages/base/src/version.ts

@@ -3,4 +3,4 @@
 
 export const JUPYTER_WIDGETS_VERSION = '2.0.0';
 
-export const PROTOCOL_VERSION = '3.0.0';
+export const PROTOCOL_VERSION = '2.1.0';

packages/base/src/widget.ts

@@ -115,7 +115,7 @@ export class WidgetModel extends Backbone.Model {
     attributes: Backbone.ObjectHash,
     options: IBackboneModelOptions
   ): void {
-    this.expectedEchoMsgIds = {};
+    this.expectedEchoMsgIds = new Map<string, string>();
     this.attrsToUpdate = new Set<string>();
 
     super.initialize(attributes, options);
@@ -224,32 +224,32 @@ export class WidgetModel extends Backbone.Model {
     const method = data.method;
     switch (method) {
       case 'update':
+      case 'echo_update':
         this.state_change = this.state_change
           .then(() => {
             const state = data.state;
-            const buffer_paths = data.buffer_paths || [];
-            const buffers = msg.buffers || [];
+            const buffer_paths = data.buffer_paths ?? [];
+            const buffers = msg.buffers?.slice(0, buffer_paths.length) ?? [];
             utils.put_buffers(state, buffer_paths, buffers);
-            if (msg.parent_header && data.echo) {
+
+            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 = data.echo.filter((attrName: string) =>
-                Object.keys(this.expectedEchoMsgIds).includes(attrName)
+              const expectedEcho = Object.keys(state).filter((attrName) =>
+                this.expectedEchoMsgIds.has(attrName)
               );
               expectedEcho.forEach((attrName: string) => {
-                // we don't care about the old messages, only the one send with the
-                // last msgId
+                // Skip echo messages until we get the reply we are expecting.
                 const isOldMessage =
-                  this.expectedEchoMsgIds[attrName] !== msgId;
+                  this.expectedEchoMsgIds.get(attrName) !== msgId;
                 if (isOldMessage) {
-                  // get rid of old updates
+                  // Ignore an echo update that comes before our echo.
                   delete state[attrName];
                 } else {
-                  // we got our confirmation, from now on we accept everything
-                  delete this.expectedEchoMsgIds[attrName];
-                  // except, we plan to send out a new state for this soon, so we will
-                  // also ignore the update for this property
+                  // we got our echo confirmation, so stop looking for it
+                  this.expectedEchoMsgIds.delete(attrName);
+                  // Start accepting echo updates unless we plan to send out a new state soon
                   if (
                     this._msg_buffer !== null &&
                     Object.prototype.hasOwnProperty.call(
@@ -263,6 +263,7 @@ export class WidgetModel extends Backbone.Model {
               });
             }
             return (this.constructor as typeof WidgetModel)._deserialize_state(
+              // Combine the state updates, with preference for kernel updates
               state,
               this.widget_manager
             );
@@ -498,8 +499,8 @@ export class WidgetModel extends Backbone.Model {
     }
   }
   rememberLastUpdateFor(msgId: string) {
-    [...this.attrsToUpdate].forEach((attrName) => {
-      this.expectedEchoMsgIds[attrName] = msgId;
+    this.attrsToUpdate.forEach((attrName) => {
+      this.expectedEchoMsgIds.set(attrName, msgId);
     });
     this.attrsToUpdate = new Set<string>();
   }
@@ -679,7 +680,7 @@ export class WidgetModel extends Backbone.Model {
   // keep track of the msg id for each attr for updates we send out so
   // that we can ignore old messages that we send in order to avoid
   // 'drunken' sliders going back and forward
-  private expectedEchoMsgIds: any;
+  private expectedEchoMsgIds: Map<string, string>;
   // because we don't know the attrs in _handle_status, we keep track of what we will send
   private attrsToUpdate: Set<string>;
 }

packages/schema/messages.md

@@ -292,27 +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
+#### Synchronizing multiple frontends: `echo_update`
 
-Starting with protocol version `3.0.0` the kernel can send a special update message back, to allow all connected frontends to be in sync with the kernel state. This allows multiple frontends to be connected to a single kernel but also resolves a possible out of sync situation when the kernel and a frontend send out an update message at the same time, causing both to think they have the latest state.
-
-In protocol version `3.0.0` the kernel is considered the single source of truth and is expected to send back to the frontends an update message that contains an extra list of keys to indicate which keys in the update are send back to the frontends as a reaction to an update received from a frontend.
+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': [ <list of keys for which the kernel is sending back the state>]
   }
 }
 ```
 
-In situations where a user does many changes to a widget on the frontend (e.g. moving a slider), the frontend will receive from the kernel many update messages (with the echo key set) from the kernel that can be considered old values. A frontend can choose to ignore all updates that are not originating from the last update it send to the kernel. This can be implemented by keeping track of the `msg_id` for each attribyte for which we send out an update message to the kernel, and ignoring all updates as a result from an `echo` for which the [`msg_id` of the parent header](https://jupyter-client.readthedocs.io/en/latest/messaging.html#parent-header) is not equal to `msg_id` we kept track of.
+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.
+
+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_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.
+
+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.
 
-For situations where sending back an echo update for a property is considered too expensive, we have implemented an opt-out mechanism in ipywidgets. A trait can have a `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.
+Implementation note: 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 `echo_update` metadata attribute set to `False` to flag that the kernel should never send an `echo_update` update for that attribute to the frontends. Additionally, we have a system-wide flag to disable echoing for all attributes via the environment variable `JUPYTER_WIDGETS_ECHO`. For ipywdgets 7.7, we default `JUPYTER_WIDGETS_ECHO` to off (disabling all echo messages) and in ipywidgets 8.0 we default `JUPYTER_WIDGETS_ECHO` to on (enabling echo messages).
 
 #### State requests: `request_state`
 

python/ipywidgets/ipywidgets/_version.py

@@ -3,7 +3,7 @@
 
 __version__ = '8.0.0b1'
 
-__protocol_version__ = '3.0.0'
+__protocol_version__ = '2.1.0'
 __control_protocol_version__ = '1.0.0'
 
 # These are *protocol* versions for each package, *not* npm versions. To check, look at each package's src/version.ts file for the protocol version the package implements.

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(d=[False, True, False]),
-            echo=['d'],
         )))]
 
 
@@ -117,16 +123,15 @@ 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(d={}, a=True),
-            buffer_paths=[['d', 'data']],
-            echo=['a', 'd'],
+            state=dict(d={}),
+            buffer_paths=[['d', 'data']]
         )))
 
     # Sanity:
@@ -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,11 +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
-    msg = {'method': 'update', 'state': {'value': 2.0, 'other': 11.0}, 'buffer_paths': [], 'echo': ['value']}
+    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)
 
 
@@ -288,7 +295,7 @@ class ValueWidget(Widget):
     assert widget.value == 42
 
     # we expect this to be echoed
-    msg = {'method': 'update', 'state': {'value': 42.0}, 'buffer_paths': [], 'echo': ['value']}
+    msg = {'method': 'echo_update', 'state': {'value': 42.0}, 'buffer_paths': []}
     call42 = mock.call(msg, buffers=[])
 
     calls = [call42]
@@ -324,17 +331,21 @@ def _square(self, change):
 
     # we expect this to be echoed
     # note that only value is echoed, not square
-    msg = {'method': 'update', 'state': {'square': 64, 'value': 8.0}, 'buffer_paths': [], 'echo': ['value']}
+    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)
 
 
 def test_no_echo():
-    # in cases where values coming fromt the frontend are 'heavy', we might want to opt out
+    # in cases where values coming from the frontend are 'heavy', we might want to opt out
     class ValueWidget(Widget):
-        value = Float().tag(sync=True, no_echo=True)
+        value = Float().tag(sync=True, echo_update=False)
 
     widget = ValueWidget(value=1)
     assert widget.value == 1
@@ -358,4 +369,4 @@ class ValueWidget(Widget):
 
     # a regular set should sync to the frontend
     widget.value = 43
-    widget._send.assert_has_calls([mock.call({'method': 'update', 'state': {'value': 43.0}, 'buffer_paths': [], 'echo': ['value']}, buffers=[])])
+    widget._send.assert_has_calls([mock.call({'method': 'update', 'state': {'value': 43.0}, 'buffer_paths': []}, buffers=[])])