Synchronize smart cell source before evaluation (#1164)

* Synchronize smart cell source before evaluation

* Sync JS view only if present

* Synchronize with smart cell only when started

* Update references to kino packages

* Update the Smart cells notebook

* Rename add dependnecy to add package

* Update changelog

CHANGELOG.md

@@ -43,6 +43,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Restructured cell insert buttons ([#1073](https://github.com/livebook-dev/livebook/pull/1073))
 - Allowed inserting images without specifying name in Markdown cells ([#1083](https://github.com/livebook-dev/livebook/pull/1083))
 - Unified authentication to always redirect to the initial URL ([#1104](https://github.com/livebook-dev/livebook/pull/1104) and [#1112](https://github.com/livebook-dev/livebook/pull/1112))
+- Erase outputs action to clear cell indicators ([#1160](https://github.com/livebook-dev/livebook/pull/1160))
 
 ### Removed
 

assets/js/hooks/cell.js

@@ -83,6 +83,11 @@ const Cell = {
     this.unsubscribeFromCellsEvents = globalPubSub.subscribe("cells", (event) =>
       this.handleCellsEvent(event)
     );
+
+    this.unsubscribeFromCellEvents = globalPubSub.subscribe(
+      `cells:${this.props.cellId}`,
+      (event) => this.handleCellEvent(event)
+    );
   },
 
   disconnected() {
@@ -93,6 +98,7 @@ const Cell = {
   destroyed() {
     this.unsubscribeFromNavigationEvents();
     this.unsubscribeFromCellsEvents();
+    this.unsubscribeFromCellEvents();
   },
 
   updated() {
@@ -114,6 +120,11 @@ const Cell = {
         "data-evaluation-digest",
         null
       ),
+      smartCellJSViewRef: getAttributeOrDefault(
+        this.el,
+        "data-smart-cell-js-view-ref",
+        null
+      ),
     };
   },
 
@@ -135,6 +146,12 @@ const Cell = {
     }
   },
 
+  handleCellEvent(event) {
+    if (event.type === "dispatch_queue_evaluation") {
+      this.handleDispatchQueueEvaluation(event.dispatch);
+    }
+  },
+
   handleElementFocused(focusableId, scroll) {
     if (this.props.cellId === focusableId) {
       this.isFocused = true;
@@ -325,6 +342,18 @@ const Cell = {
     }
   },
 
+  handleDispatchQueueEvaluation(dispatch) {
+    if (this.props.type === "smart" && this.props.smartCellJSViewRef) {
+      // Ensure the smart cell UI is reflected on the server, before the evaluation
+      globalPubSub.broadcast(`js_views:${this.props.smartCellJSViewRef}`, {
+        type: "sync",
+        callback: dispatch,
+      });
+    } else {
+      dispatch();
+    }
+  },
+
   handleLocationReport(client, report) {
     Object.entries(this.liveEditors).forEach(([tag, liveEditor]) => {
       if (

assets/js/hooks/js_view.js

@@ -1,5 +1,6 @@
 import { getAttributeOrThrow, parseInteger } from "../lib/attribute";
 import { isElementHidden, randomId, randomToken } from "../lib/utils";
+import { globalPubSub } from "../lib/pub_sub";
 import {
   getChannel,
   transportDecode,
@@ -58,6 +59,8 @@ const JSView = {
     this.childReadyPromise = null;
     this.childReady = false;
     this.initReceived = false;
+    this.syncCallbackQueue = [];
+    this.pongCallbackQueue = [];
 
     this.initTimeout = setTimeout(() => this.handleInitTimeout(), 2_000);
 
@@ -104,12 +107,22 @@ const JSView = {
       }
     );
 
+    const pongRef = this.channel.on(`pong:${this.props.ref}`, () => {
+      this.handleServerPong();
+    });
+
     this.unsubscribeFromChannelEvents = () => {
       this.channel.off(`init:${this.props.ref}:${this.id}`, initRef);
       this.channel.off(`event:${this.props.ref}`, eventRef);
       this.channel.off(`error:${this.props.ref}`, errorRef);
+      this.channel.off(`pong:${this.props.ref}`, pongRef);
     };
 
+    this.unsubscribeFromJSViewEvents = globalPubSub.subscribe(
+      `js_views:${this.props.ref}`,
+      (event) => this.handleJSViewEvent(event)
+    );
+
     this.channel.push(
       "connect",
       {
@@ -134,6 +147,8 @@ const JSView = {
 
     this.unsubscribeFromChannelEvents();
     this.channel.push("disconnect", { ref: this.props.ref });
+
+    this.unsubscribeFromJSViewEvents();
   },
 
   getProps() {
@@ -296,6 +311,9 @@ const JSView = {
         const { event, payload } = message;
         const raw = transportEncode([event, this.props.ref], payload);
         this.channel.push("event", raw);
+      } else if (message.type === "syncReply") {
+        this.pongCallbackQueue.push(this.syncCallbackQueue.shift());
+        this.channel.push("ping", { ref: this.props.ref });
       }
     }
   },
@@ -361,6 +379,21 @@ const JSView = {
 
     this.errorContainer.textContent = message;
   },
+
+  handleServerPong() {
+    const callback = this.pongCallbackQueue.shift();
+    callback();
+  },
+
+  handleJSViewEvent(event) {
+    if (event.type === "sync") {
+      // First, we invoke optional synchronization callback in the iframe,
+      // that may send any deferred UI changes to the server. Then, we
+      // do a ping to synchronize with the server
+      this.syncCallbackQueue.push(event.callback);
+      this.postMessage({ type: "sync" });
+    }
+  },
 };
 
 export default JSView;

assets/js/hooks/js_view/iframe.js

@@ -26,7 +26,7 @@ import { sha256Base64 } from "../../lib/utils";
 // (2): https://developer.mozilla.org/en-US/docs/Web/HTML/Element/iframe#attr-sandbox
 // (3): https://developer.mozilla.org/en-US/docs/Web/Security/Secure_Contexts
 
-const IFRAME_SHA256 = "4gyeA71Bpb4SGj2M0BUdT1jtk6wjUqOf6Q8wVYp7htc=";
+const IFRAME_SHA256 = "fA00WeO9LAvgpbMz9vKEU0WTr4Uk5bTt/BxKHdweEz8=";
 
 export function initializeIframeSource(iframe, iframePort) {
   const iframeUrl = getIframeUrl(iframePort);

assets/js/hooks/session.js

@@ -488,6 +488,14 @@ const Session = {
       this.setInsertMode(true);
     }
 
+    const evalButton = event.target.closest(
+      `[data-el-queue-cell-evaluation-button]`
+    );
+    if (evalButton) {
+      const cellId = evalButton.getAttribute("data-cell-id");
+      this.queueCellEvaluation(cellId);
+    }
+
     const hash = window.location.hash;
 
     if (hash) {
@@ -700,10 +708,16 @@ const Session = {
     }
   },
 
+  queueCellEvaluation(cellId) {
+    this.dispatchQueueEvaluation(() => {
+      this.pushEvent("queue_cell_evaluation", { cell_id: cellId });
+    });
+  },
+
   queueFocusedCellEvaluation() {
     if (this.focusedId && this.isCell(this.focusedId)) {
-      this.pushEvent("queue_cell_evaluation", {
-        cell_id: this.focusedId,
+      this.dispatchQueueEvaluation(() => {
+        this.pushEvent("queue_cell_evaluation", { cell_id: this.focusedId });
       });
     }
   },
@@ -714,8 +728,10 @@ const Session = {
         ? [this.focusedId]
         : [];
 
-    this.pushEvent("queue_full_evaluation", {
-      forced_cell_ids: forcedCellIds,
+    this.dispatchQueueEvaluation(() => {
+      this.pushEvent("queue_full_evaluation", {
+        forced_cell_ids: forcedCellIds,
+      });
     });
   },
 
@@ -724,13 +740,29 @@ const Session = {
       const sectionId = this.getSectionIdByFocusableId(this.focusedId);
 
       if (sectionId) {
-        this.pushEvent("queue_section_evaluation", {
-          section_id: sectionId,
+        this.dispatchQueueEvaluation(() => {
+          this.pushEvent("queue_section_evaluation", {
+            section_id: sectionId,
+          });
         });
       }
     }
   },
 
+  dispatchQueueEvaluation(dispatch) {
+    if (isEvaluable(this.focusedCellType())) {
+      // If an evaluable cell is focused, we forward the evaluation
+      // request to that cell, so it can synchronize itself before
+      // sending the request to the server
+      globalPubSub.broadcast(`cells:${this.focusedId}`, {
+        type: "dispatch_queue_evaluation",
+        dispatch,
+      });
+    } else {
+      dispatch();
+    }
+  },
+
   cancelFocusedCellEvaluation() {
     if (this.focusedId && this.isCell(this.focusedId)) {
       this.pushEvent("cancel_cell_evaluation", {

iframe/priv/static/iframe/v3.html

@@ -28,6 +28,7 @@
           importPromise: null,
           eventHandlers: {},
           eventQueue: [],
+          syncHandler: null,
         };
 
         function postMessage(message) {
@@ -75,6 +76,10 @@
               document.head.appendChild(linkEl);
             });
           },
+
+          handleSync(callback) {
+            state.syncHandler = callback;
+          },
         };
 
         window.addEventListener("message", (event) => {
@@ -128,6 +133,12 @@
             } else {
               state.eventQueue.push({ event, payload });
             }
+          } else if (message.type === "sync") {
+            Promise.resolve(state.syncHandler && state.syncHandler()).then(
+              () => {
+                postMessage({ type: "syncReply" });
+              }
+            );
           }
         }
 

lib/livebook.ex

@@ -30,11 +30,11 @@ defmodule Livebook do
 
       [
         %{
-          dependency: {:kino, "~> 0.6.0"},
+          dependency: {:kino, "~> 0.6.1"},
           description: "Interactive widgets for Livebook",
           name: "kino",
           url: "https://hex.pm/packages/kino",
-          version: "0.6.0"
+          version: "0.6.1"
         }
       ]
 

lib/livebook/notebook/explore/elixir_and_livebook.livemd

@@ -56,7 +56,7 @@ double-click on "Notebook dependencies and setup", add and run the code.
 
 ```elixir
 Mix.install([
-  {:kino, "~> 0.6.0"}
+  {:kino, "~> 0.6.1"}
 ])
 ```
 

lib/livebook/notebook/explore/intro_to_vega_lite.livemd

@@ -3,7 +3,7 @@
 ```elixir
 Mix.install([
   {:vega_lite, "~> 0.1.4"},
-  {:kino_vega_lite, "~> 0.1.0"}
+  {:kino_vega_lite, "~> 0.1.1"}
 ])
 ```
 

lib/livebook/notebook/explore/kino/chat_app.livemd

@@ -2,7 +2,7 @@
 
 ```elixir
 Mix.install([
-  {:kino, "~> 0.6.0"}
+  {:kino, "~> 0.6.1"}
 ])
 ```
 

lib/livebook/notebook/explore/kino/custom_kinos.livemd

@@ -2,7 +2,7 @@
 
 ```elixir
 Mix.install([
-  {:kino, "~> 0.6.0"}
+  {:kino, "~> 0.6.1"}
 ])
 ```
 

lib/livebook/notebook/explore/kino/intro_to_kino.livemd

@@ -2,7 +2,7 @@
 
 ```elixir
 Mix.install([
-  {:kino, "~> 0.6.0"}
+  {:kino, "~> 0.6.1"}
 ])
 ```
 

lib/livebook/notebook/explore/kino/pong.livemd

@@ -2,7 +2,7 @@
 
 ```elixir
 Mix.install([
-  {:kino, "~> 0.6.0"}
+  {:kino, "~> 0.6.1"}
 ])
 ```
 

lib/livebook/notebook/explore/kino/smart_cells.livemd

@@ -2,7 +2,7 @@
 
 ```elixir
 Mix.install([
-  {:kino, "~> 0.6.0"},
+  {:kino, "~> 0.6.1"},
   {:jason, "~> 1.3"}
 ])
 ```
@@ -90,9 +90,15 @@ defmodule KinoGuide.PrintCell do
         textEl.value = text;
       });
 
-      textEl.addEventListener("blur", (event) => {
+      textEl.addEventListener("change", (event) => {
         ctx.pushEvent("update_text", event.target.value);
       });
+
+      ctx.handleSync(() => {
+        // Synchronously invokes change listeners
+        document.activeElement &&
+          document.activeElement.dispatchEvent(new Event("change"));
+      });
     }
     """
   end
@@ -125,15 +131,21 @@ so that Livebook picks it up. Note that in practice we would put the
 Smart cell in a package and we would register it in `application.ex`
 when starting the application.
 
+Note that we register a synchronization handler on the client with
+`ctx.handleSync(() => ...)`. This optional handler is invoked before
+evaluation and it should flush any deferred UI changes to the server.
+In our example we listen to input's "change" event, which is only
+triggered on blur, so on synchronization we trigger it programmatically.
+
 <!-- livebook:{"break_markdown":true} -->
 
 Now let's try out the new cell! We already inserted one below, but you
 can add more with the <kbd>+ Smart</kbd> button.
 
-<!-- livebook:{"attrs":{"text":"something"},"kind":"Elixir.KinoGuide.PrintCell","livebook_object":"smart_cell"} -->
+<!-- livebook:{"attrs":{"text":"Hello!"},"kind":"Elixir.KinoGuide.PrintCell","livebook_object":"smart_cell"} -->
 
 ```elixir
-IO.puts("something")
+IO.puts("Hello!")
 ```
 
 Focus the Smart cell and click the "Source" icon. You should see the
@@ -329,13 +341,19 @@ defmodule KinoGuide.JSONConverterCell do
       const variableEl = ctx.root.querySelector(`[name="variable"]`);
       variableEl.value = payload.variable;
 
-      variableEl.addEventListener("blur", (event) => {
+      variableEl.addEventListener("change", (event) => {
         ctx.pushEvent("update_variable", event.target.value);
       });
 
       ctx.handleEvent("update_variable", (variable) => {
         variableEl.value = variable;
       });
+
+      ctx.handleSync(() => {
+        // Synchronously invokes change listeners
+        document.activeElement &&
+          document.activeElement.dispatchEvent(new Event("change"));
+      });
     }
     """
   end

lib/livebook/notebook/explore/kino/vm_introspection.livemd

@@ -2,8 +2,8 @@
 
 ```elixir
 Mix.install([
-  {:kino, "~> 0.6.0"},
-  {:kino_vega_lite, "~> 0.1.0"}
+  {:kino, "~> 0.6.1"},
+  {:kino_vega_lite, "~> 0.1.1"}
 ])
 
 alias VegaLite, as: Vl

lib/livebook/runtime.ex

@@ -411,7 +411,7 @@ defprotocol Livebook.Runtime do
   to the runtime owner whenever attrs and the generated source code
   change.
 
-    * `{:runtime_smart_cell_update, ref, attrs, source}`
+    * `{:runtime_smart_cell_update, ref, attrs, source, %{reevaluate: boolean()}}`
 
   The attrs are persisted and may be used to restore the smart cell
   state later. Note that for persistence they get serialized and

lib/livebook/runtime/elixir_standalone.ex

@@ -17,8 +17,8 @@ defmodule Livebook.Runtime.ElixirStandalone do
           server_pid: pid() | nil
         }
 
-  kino_vega_lite = %{name: "kino_vega_lite", dependency: {:kino_vega_lite, "~> 0.1.0"}}
-  kino_db = %{name: "kino_db", dependency: {:kino_db, "~> 0.1.0"}}
+  kino_vega_lite = %{name: "kino_vega_lite", dependency: {:kino_vega_lite, "~> 0.1.1"}}
+  kino_db = %{name: "kino_db", dependency: {:kino_db, "~> 0.1.1"}}
 
   @extra_smart_cell_definitions [
     %{

lib/livebook/session.ex

@@ -1001,6 +1001,18 @@ defmodule Livebook.Session do
     end
   end
 
+  def handle_info({:pong, {:smart_cell_evaluation, cell_id}, _info}, state) do
+    state =
+      with {:ok, cell, section} <- Notebook.fetch_cell_and_section(state.data.notebook, cell_id),
+           :evaluating <- state.data.cell_infos[cell.id].eval.status do
+        start_evaluation(state, cell, section)
+      else
+        _ -> state
+      end
+
+    {:noreply, state}
+  end
+
   def handle_info(_message, state), do: {:noreply, state}
 
   @impl true
@@ -1308,28 +1320,20 @@ defmodule Livebook.Session do
   end
 
   defp handle_action(state, {:start_evaluation, cell, section}) do
-    path =
-      case state.data.file do
-        nil -> ""
-        file -> file.path
-      end
+    info = state.data.cell_infos[cell.id]
 
-    file = path <> "#cell"
+    if is_struct(cell, Cell.Smart) and info.status == :started do
+      # We do a ping and start evaluation only once we get a reply,
+      # this way we make sure we received all relevant source changes
+      send(
+        cell.js_view.pid,
+        {:ping, self(), {:smart_cell_evaluation, cell.id}, %{ref: cell.js_view.ref}}
+      )
 
-    smart_cell_ref =
-      case cell do
-        %Cell.Smart{} -> cell.id
-        _ -> nil
-      end
-
-    opts = [file: file, smart_cell_ref: smart_cell_ref]
-
-    locator = {container_ref_for_section(section), cell.id}
-    base_locator = find_base_locator(state.data, cell, section)
-    Runtime.evaluate_code(state.data.runtime, cell.source, locator, base_locator, opts)
-
-    evaluation_digest = :erlang.md5(cell.source)
-    handle_operation(state, {:evaluation_started, self(), cell.id, evaluation_digest})
+      state
+    else
+      start_evaluation(state, cell, section)
+    end
   end
 
   defp handle_action(state, {:stop_evaluation, section}) do
@@ -1384,6 +1388,31 @@ defmodule Livebook.Session do
 
   defp handle_action(state, _action), do: state
 
+  defp start_evaluation(state, cell, section) do
+    path =
+      case state.data.file do
+        nil -> ""
+        file -> file.path
+      end
+
+    file = path <> "#cell"
+
+    smart_cell_ref =
+      case cell do
+        %Cell.Smart{} -> cell.id
+        _ -> nil
+      end
+
+    opts = [file: file, smart_cell_ref: smart_cell_ref]
+
+    locator = {container_ref_for_section(section), cell.id}
+    base_locator = find_base_locator(state.data, cell, section)
+    Runtime.evaluate_code(state.data.runtime, cell.source, locator, base_locator, opts)
+
+    evaluation_digest = :erlang.md5(cell.source)
+    handle_operation(state, {:evaluation_started, self(), cell.id, evaluation_digest})
+  end
+
   defp broadcast_operation(session_id, operation) do
     broadcast_message(session_id, {:operation, operation})
   end

lib/livebook_web/channels/js_view_channel.ex

@@ -39,6 +39,12 @@ defmodule LivebookWeb.JSViewChannel do
     {:noreply, socket}
   end
 
+  def handle_in("ping", %{"ref" => ref}, socket) do
+    pid = socket.assigns.ref_with_info[ref].pid
+    send(pid, {:ping, self(), nil, %{ref: ref}})
+    {:noreply, socket}
+  end
+
   def handle_in("disconnect", %{"ref" => ref}, socket) do
     socket =
       if socket.assigns.ref_with_info[ref].count == 1 do
@@ -76,6 +82,11 @@ defmodule LivebookWeb.JSViewChannel do
     {:noreply, socket}
   end
 
+  def handle_info({:pong, _, %{ref: ref}}, socket) do
+    push(socket, "pong:#{ref}", %{})
+    {:noreply, socket}
+  end
+
   def handle_info({:encoding_error, error, {:event, _event, _payload, %{ref: ref}}}, socket) do
     message = "Failed to serialize widget data, " <> error
     push(socket, "error:#{ref}", %{"message" => message})

lib/livebook_web/live/session_live/cell_component.ex

@@ -14,7 +14,8 @@ defmodule LivebookWeb.SessionLive.CellComponent do
       data-session-path={Routes.session_path(@socket, :page, @session_id)}
       data-evaluation-digest={get_in(@cell_view, [:eval, :evaluation_digest])}
       data-eval-validity={get_in(@cell_view, [:eval, :validity])}
-      data-js-empty={empty?(@cell_view.source_view)}>
+      data-js-empty={empty?(@cell_view.source_view)}
+      data-smart-cell-js-view-ref={smart_cell_js_view_ref(@cell_view)}>
       <%= render_cell(assigns) %>
     </div>
     """
@@ -272,8 +273,8 @@ defmodule LivebookWeb.SessionLive.CellComponent do
   defp cell_evaluation_button(%{status: :ready} = assigns) do
     ~H"""
     <button class="text-gray-600 hover:text-gray-800 focus:text-gray-800 flex space-x-1 items-center"
-      phx-click="queue_cell_evaluation"
-      phx-value-cell_id={@cell_id}>
+      data-el-queue-cell-evaluation-button
+      data-cell-id={@cell_id}>
       <.remix_icon icon="play-circle-fill" class="text-xl" />
       <span class="text-sm font-medium">
         <%= if(@validity == :evaluated, do: "Reevaluate", else: "Evaluate") %>
@@ -298,8 +299,8 @@ defmodule LivebookWeb.SessionLive.CellComponent do
   defp setup_cell_evaluation_button(%{status: :ready} = assigns) do
     ~H"""
     <button class="text-gray-600 hover:text-gray-800 focus:text-gray-800 flex space-x-1 items-center"
-      phx-click="queue_cell_evaluation"
-      phx-value-cell_id={@cell_id}>
+      data-el-queue-cell-evaluation-button
+      data-cell-id={@cell_id}>
       <%= if @validity == :fresh do %>
         <.remix_icon icon="play-circle-fill" class="text-xl" />
         <span class="text-sm font-medium">Setup</span>
@@ -388,10 +389,10 @@ defmodule LivebookWeb.SessionLive.CellComponent do
         </button>
       </span>
     <% else %>
-      <span class="tooltip top" data-tooltip="Add dependency (sp)">
+      <span class="tooltip top" data-tooltip="Add package (sp)">
         <%= live_patch to: Routes.session_path(@socket, :package_search, @session_id),
               class: "icon-button",
-              aria_label: "add dependency",
+              aria_label: "add package",
               role: "button",
               data_btn_package_search: true do %>
           <.remix_icon icon="play-list-add-line" class="text-xl" />
@@ -611,4 +612,7 @@ defmodule LivebookWeb.SessionLive.CellComponent do
   end
 
   defp evaluated_label(_time_ms), do: nil
+
+  defp smart_cell_js_view_ref(%{type: :smart, js_view: %{ref: ref}}), do: ref
+  defp smart_cell_js_view_ref(_cell_view), do: nil
 end

test/livebook/session_test.exs

@@ -676,6 +676,50 @@ defmodule Livebook.SessionTest do
 
       assert_receive {:editor_source, "content!"}
     end
+
+    test "pings the smart cell before evaluation to await all incoming messages" do
+      smart_cell = %{Notebook.Cell.new(:smart) | kind: "text", source: "1"}
+      notebook = %{Notebook.new() | sections: [%{Notebook.Section.new() | cells: [smart_cell]}]}
+      session = start_session(notebook: notebook)
+
+      runtime = connected_noop_runtime()
+      Session.set_runtime(session.pid, runtime)
+
+      send(
+        session.pid,
+        {:runtime_smart_cell_definitions, [%{kind: "text", name: "Text", requirement: nil}]}
+      )
+
+      Session.subscribe(session.id)
+
+      send(
+        session.pid,
+        {:runtime_smart_cell_started, smart_cell.id,
+         %{source: "1", js_view: %{pid: self(), ref: "ref"}, editor: nil}}
+      )
+
+      Session.queue_cell_evaluation(session.pid, smart_cell.id)
+
+      send(
+        session.pid,
+        {:runtime_evaluation_response, "setup", {:ok, ""}, %{evaluation_time_ms: 10}}
+      )
+
+      session_pid = session.pid
+      assert_receive {:ping, ^session_pid, metadata, %{ref: "ref"}}
+
+      # Update the source before replying to ping
+      send(
+        session.pid,
+        {:runtime_smart_cell_update, smart_cell.id, %{}, "2", %{reevaluate: false}}
+      )
+
+      send(session_pid, {:pong, metadata, %{ref: "ref"}})
+
+      cell_id = smart_cell.id
+      new_digest = :erlang.md5("2")
+      assert_receive {:operation, {:evaluation_started, ^session_pid, ^cell_id, ^new_digest}}
+    end
   end
 
   describe "find_base_locator/3" do