Improve validity check when input changes during evaluation (#651)

lib/livebook/session/data.ex

@@ -61,7 +61,8 @@ defmodule Livebook.Session.Data do
           evaluation_snapshot: snapshot() | nil,
           evaluation_time_ms: integer() | nil,
           number_of_evaluations: non_neg_integer(),
-          bound_to_input_ids: MapSet.t(Cell.id())
+          bound_to_input_ids: MapSet.t(Cell.id()),
+          bound_input_readings: input_reading()
         }
 
   @type cell_bin_entry :: %{
@@ -98,6 +99,8 @@ defmodule Livebook.Session.Data do
   #
   @type snapshot :: {deps_snapshot :: term(), bound_inputs_snapshot :: term()}
 
+  @type input_reading :: {input_name :: String.t(), input_value :: String.t()}
+
   # Note that all operations carry the pid of whatever
   # process originated the operation. Some operations
   # like :apply_cell_delta and :report_cell_revision
@@ -417,7 +420,8 @@ defmodule Livebook.Session.Data do
     with {:ok, %Cell.Elixir{} = cell, _section} <-
            Notebook.fetch_cell_and_section(data.notebook, id),
          {:ok, %Cell.Input{} = input_cell, _section} <-
-           Notebook.fetch_cell_and_section(data.notebook, input_id) do
+           Notebook.fetch_cell_and_section(data.notebook, input_id),
+         false <- MapSet.member?(data.cell_infos[cell.id].bound_to_input_ids, input_cell.id) do
       data
       |> with_actions()
       |> bind_input(cell, input_cell)
@@ -816,7 +820,7 @@ defmodule Livebook.Session.Data do
     |> Enum.join("\n")
   end
 
-  defp finish_cell_evaluation({data, _} = data_actions, cell, section, metadata) do
+  defp finish_cell_evaluation(data_actions, cell, section, metadata) do
     data_actions
     |> update_cell_info!(cell.id, fn info ->
       %{
@@ -824,9 +828,10 @@ defmodule Livebook.Session.Data do
         | evaluation_status: :ready,
           evaluation_time_ms: metadata.evaluation_time_ms,
           number_of_evaluations: info.number_of_evaluations + 1,
-          # After finished evaluation, take latest snapshot of bound inputs
+          # After finished evaluation, take the snapshot of read inputs
           evaluation_snapshot:
-            {elem(info.evaluation_snapshot, 0), bound_inputs_snapshot(data, cell)}
+            {elem(info.evaluation_snapshot, 0),
+             input_readings_snapshot(info.bound_input_readings)}
       }
     end)
     |> set_section_info!(section.id, evaluating_cell_id: nil)
@@ -923,7 +928,8 @@ defmodule Livebook.Session.Data do
               evaluation_status: :evaluating,
               evaluation_digest: nil,
               evaluation_snapshot: info.snapshot,
-              bound_to_input_ids: MapSet.new()
+              bound_to_input_ids: MapSet.new(),
+              bound_input_readings: []
           }
         end)
         |> set_section_info!(section.id, evaluating_cell_id: id, evaluation_queue: ids)
@@ -937,7 +943,11 @@ defmodule Livebook.Session.Data do
   defp bind_input(data_actions, cell, input_cell) do
     data_actions
     |> update_cell_info!(cell.id, fn info ->
-      %{info | bound_to_input_ids: MapSet.put(info.bound_to_input_ids, input_cell.id)}
+      %{
+        info
+        | bound_to_input_ids: MapSet.put(info.bound_to_input_ids, input_cell.id),
+          bound_input_readings: [{input_cell.name, input_cell.value} | info.bound_input_readings]
+      }
     end)
   end
 
@@ -1204,6 +1214,7 @@ defmodule Livebook.Session.Data do
       evaluation_time_ms: nil,
       number_of_evaluations: 0,
       bound_to_input_ids: MapSet.new(),
+      bound_input_readings: [],
       snapshot: {:initial, :initial},
       evaluation_snapshot: nil
     }
@@ -1370,18 +1381,20 @@ defmodule Livebook.Session.Data do
   defp bound_inputs_snapshot(data, cell) do
     %{bound_to_input_ids: bound_to_input_ids} = data.cell_infos[cell.id]
 
-    if Enum.empty?(bound_to_input_ids) do
-      :initial
-    else
-      for(
-        section <- data.notebook.sections,
-        cell <- section.cells,
-        is_struct(cell, Cell.Input),
-        cell.id in bound_to_input_ids,
-        do: {cell.name, cell.value}
-      )
-      |> :erlang.phash2()
-    end
+    for(
+      section <- data.notebook.sections,
+      cell <- section.cells,
+      is_struct(cell, Cell.Input),
+      cell.id in bound_to_input_ids,
+      do: {cell.name, cell.value}
+    )
+    |> input_readings_snapshot()
+  end
+
+  defp input_readings_snapshot([]), do: :initial
+
+  defp input_readings_snapshot(name_value_pairs) do
+    name_value_pairs |> Enum.sort() |> :erlang.phash2()
   end
 
   defp update_validity({data, _} = data_actions) do

test/livebook/session/data_test.exs

@@ -2150,6 +2150,33 @@ defmodule Livebook.Session.DataTest do
               }, _actions} = Data.apply_operation(data, operation)
     end
 
+    test "if bound input value changes during cell evaluation, the cell is marked as stale afterwards" do
+      data =
+        data_after_operations!([
+          {:insert_section, self(), 0, "s1"},
+          {:insert_cell, self(), "s1", 0, :input, "c1"},
+          {:insert_cell, self(), "s1", 1, :elixir, "c2"},
+          {:set_runtime, self(), NoopRuntime.new()},
+          {:queue_cell_evaluation, self(), "c2"},
+          {:add_cell_evaluation_response, self(), "c2", @eval_resp, @eval_meta},
+          # Make the Elixir cell evaluating
+          {:queue_cell_evaluation, self(), "c2"},
+          # Bind the input (effectively read the current value)
+          {:bind_input, self(), "c2", "c1"},
+          # Change the input value, while the cell is evaluating
+          {:set_cell_attributes, self(), "c1", %{value: "stuff"}}
+        ])
+
+      operation = {:add_cell_evaluation_response, self(), "c2", @eval_resp, @eval_meta}
+
+      assert {:ok,
+              %{
+                cell_infos: %{
+                  "c2" => %{validity_status: :stale}
+                }
+              }, _} = Data.apply_operation(data, operation)
+    end
+
     test "adds evaluation time to the response" do
       data =
         data_after_operations!([