From b48f81cd995395e4c649b62186b4215ed01393ee Mon Sep 17 00:00:00 2001 From: woops Date: Sat, 23 Oct 2021 19:33:52 +0200 Subject: [PATCH 01/11] Added automatic evaluation --- lib/livebook/notebook/cell/elixir.ex | 8 +++-- lib/livebook_web/live/session_live.ex | 8 ++++- .../live/session_live/cell_component.ex | 33 ++++++++++++++++++- .../elixir_cell_settings_component.ex | 20 +++++++++-- 4 files changed, 62 insertions(+), 7 deletions(-) diff --git a/lib/livebook/notebook/cell/elixir.ex b/lib/livebook/notebook/cell/elixir.ex index 6143daf8224..5dea8c5a166 100644 --- a/lib/livebook/notebook/cell/elixir.ex +++ b/lib/livebook/notebook/cell/elixir.ex @@ -6,7 +6,7 @@ defmodule Livebook.Notebook.Cell.Elixir do # It consists of text content that the user can edit # and produces some output once evaluated. - defstruct [:id, :source, :outputs, :disable_formatting] + defstruct [:id, :source, :outputs, :disable_formatting, :evaluate_automatically?] alias Livebook.Utils alias Livebook.Notebook.Cell @@ -15,7 +15,8 @@ defmodule Livebook.Notebook.Cell.Elixir do id: Cell.id(), source: String.t(), outputs: list(output()), - disable_formatting: boolean() + disable_formatting: boolean(), + evaluate_automatically?: boolean() } @typedoc """ @@ -49,7 +50,8 @@ defmodule Livebook.Notebook.Cell.Elixir do id: Utils.random_id(), source: "", outputs: [], - disable_formatting: false + disable_formatting: false, + evaluate_automatically?: false } end end diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index e9ec150f33f..2d166073c0c 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -810,6 +810,11 @@ defmodule LivebookWeb.SessionLive do {:noreply, push_event(socket, "location_report", report)} end + def handle_info({:queue_stale_component, %{"cell_view" => cell_view}}, socket) do + Session.queue_cell_evaluation(socket.assigns.session.pid, cell_view.id) + {:noreply, socket} + end + def handle_info(_message, socket), do: {:noreply, socket} defp handle_relative_path(socket, path) do @@ -1223,7 +1228,8 @@ defmodule LivebookWeb.SessionLive do validity_status: info.validity_status, evaluation_status: info.evaluation_status, evaluation_time_ms: info.evaluation_time_ms, - number_of_evaluations: info.number_of_evaluations + number_of_evaluations: info.number_of_evaluations, + evaluate_automatically?: cell.evaluate_automatically? } end diff --git a/lib/livebook_web/live/session_live/cell_component.ex b/lib/livebook_web/live/session_live/cell_component.ex index 4e1a6f73796..54a2c9c9e7c 100644 --- a/lib/livebook_web/live/session_live/cell_component.ex +++ b/lib/livebook_web/live/session_live/cell_component.ex @@ -1,6 +1,22 @@ defmodule LivebookWeb.SessionLive.CellComponent do use LivebookWeb, :live_component + def update(assigns, socket) do + if should_enqueue_stale_component?(assigns.cell_view) do + send(self(), {:queue_stale_component, %{"cell_view" => assigns.cell_view}}) + end + + { + :ok, + socket + |> assign( + cell_view: assigns.cell_view, + id: assigns.id, + session_id: assigns.session_id + ) + } + end + def render(assigns) do ~H"""
<.remix_icon icon="play-circle-fill" class="text-xl" /> - <%= if(@cell_view.validity_status == :evaluated, do: "Reevaluate", else: "Evaluate") %> + <%= render_evaluation_text(@cell_view) %> <% else %> @@ -500,4 +516,19 @@ defmodule LivebookWeb.SessionLive.CellComponent do
<%= @message %>
""" end + + defp render_evaluation_text(%{evaluate_automatically?: true}), do: "Evaluates Automatically" + + defp render_evaluation_text(%{validity_status: :evaluated}), do: "Reevaluate" + defp render_evaluation_text(_), do: "Evaluate" + + defp should_enqueue_stale_component?(%{ + evaluate_automatically?: true, + validity_status: :stale, + evaluation_status: :ready, + empty?: false + }), + do: true + + defp should_enqueue_stale_component?(_), do: false end diff --git a/lib/livebook_web/live/session_live/elixir_cell_settings_component.ex b/lib/livebook_web/live/session_live/elixir_cell_settings_component.ex index fac32884b5d..9900bca25cd 100644 --- a/lib/livebook_web/live/session_live/elixir_cell_settings_component.ex +++ b/lib/livebook_web/live/session_live/elixir_cell_settings_component.ex @@ -11,6 +11,7 @@ defmodule LivebookWeb.SessionLive.ElixirCellSettingsComponent do socket |> assign(assigns) |> assign_new(:disable_formatting, fn -> cell.disable_formatting end) + |> assign_new(:evaluate_automatically?, fn -> cell.evaluate_automatically? end) {:ok, socket} end @@ -29,6 +30,12 @@ defmodule LivebookWeb.SessionLive.ElixirCellSettingsComponent do label="Disable code formatting (when saving to file)" checked={@disable_formatting} />
+
+ <.switch_checkbox + name="evaluate_automatically" + label="Evaluate automatically" + checked={@evaluate_automatically?} /> +
<%= live_patch "Cancel", to: @return_to, class: "button button-outlined-gray" %> + <%= if @cell_view.reevaluate_automatically do %> + <%= live_patch to: Routes.session_path(@socket, :cell_settings, @session_id, @cell_view.id), + class: "text-gray-600 hover:text-gray-800 focus:text-gray-800 flex space-x-1 items-center" do %> + <.remix_icon icon="refresh-fill" class="text-xl" /> + + Reevaluates automatically + + <% end %> + <% else %> + + <% end %> <% else %>
""" end - - defp render_evaluation_text(%{reevaluate_automatically: true, validity_status: :stale}), - do: "Evaluates Automatically" - - defp render_evaluation_text(%{validity_status: :evaluated}), do: "Reevaluate" - defp render_evaluation_text(_), do: "Evaluate" end From b784098f6a10515529c70634b274fdd14b7a6566 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Tue, 26 Oct 2021 15:31:16 +0200 Subject: [PATCH 08/11] Evaluate automatically reevaluating cells after queuing --- lib/livebook/session/data.ex | 30 +++++-------- test/livebook/session/data_test.exs | 70 ++++++++++++++++++++++++++--- 2 files changed, 77 insertions(+), 23 deletions(-) diff --git a/lib/livebook/session/data.ex b/lib/livebook/session/data.ex index 79ab5b96b87..fbaa5f3fc01 100644 --- a/lib/livebook/session/data.ex +++ b/lib/livebook/session/data.ex @@ -1315,7 +1315,10 @@ defmodule Livebook.Session.Data do data_actions |> compute_snapshots() |> update_validity() + # After updating validity there may be new stale cells, so we check + # if any of them is configured for automatic reevaluation |> maybe_queue_reevaluating_cells() + |> maybe_evaluate_queued() end defp compute_snapshots({data, _} = data_actions) do @@ -1422,28 +1425,19 @@ defmodule Livebook.Session.Data do end defp maybe_queue_reevaluating_cells({data, _} = data_actions) do - stale_cell_ids = - for {cell_id, cell} <- data.cell_infos do - if cell.validity_status == :stale do - cell_id - end - end - |> Enum.reject(&is_nil/1) - |> MapSet.new() - - stale_cells = - Notebook.elixir_cells_with_section(data.notebook) - |> Enum.flat_map(fn {_, %{cells: cells} = section} -> - Enum.map(cells, fn cell -> {cell, section} end) - end) - |> Enum.filter(fn {cell, _} -> - MapSet.member?(stale_cell_ids, cell.id) && cell.reevaluate_automatically + cells_to_reeavaluete = + data.notebook + |> Notebook.elixir_cells_with_section() + |> Enum.filter(fn {cell, _section} -> + info = data.cell_infos[cell.id] + info.validity_status == :stale and cell.reevaluate_automatically end) data_actions - |> reduce(stale_cells, fn data_actions, {stale_cell, section} -> + |> reduce(cells_to_reeavaluete, fn data_actions, {cell, section} -> data_actions - |> queue_cell_evaluation(stale_cell, section) + |> queue_prerequisite_cells_evaluation(cell) + |> queue_cell_evaluation(cell, section) end) end end diff --git a/test/livebook/session/data_test.exs b/test/livebook/session/data_test.exs index a86e07aa3e0..97873c1388d 100644 --- a/test/livebook/session/data_test.exs +++ b/test/livebook/session/data_test.exs @@ -695,6 +695,29 @@ defmodule Livebook.Session.DataTest do }, _actions} = Data.apply_operation(data, operation) end + test "marks child automatically reevaluating cells for evaluation" do + data = + data_after_operations!([ + {:insert_section, self(), 0, "s1"}, + {:insert_cell, self(), "s1", 0, :elixir, "c1"}, + {:insert_cell, self(), "s1", 1, :elixir, "c2"}, + {:set_cell_attributes, self(), "c2", %{reevaluate_automatically: true}}, + # Evaluate both cells + {:set_runtime, self(), NoopRuntime.new()}, + {:queue_cell_evaluation, self(), "c1"}, + {:add_cell_evaluation_response, self(), "c1", @eval_resp, @eval_meta}, + {:queue_cell_evaluation, self(), "c2"}, + {:add_cell_evaluation_response, self(), "c2", @eval_resp, @eval_meta} + ]) + + operation = {:delete_cell, self(), "c1"} + + assert {:ok, + %{ + cell_infos: %{"c2" => %{evaluation_status: :evaluating}} + }, _actions} = Data.apply_operation(data, operation) + end + test "deleting a markdown cell does not change child cell validity" do data = data_after_operations!([ @@ -2073,6 +2096,37 @@ defmodule Livebook.Session.DataTest do }, []} = Data.apply_operation(data, operation) end + test "marks child automatically reevaluating cells for evaluation" do + data = + data_after_operations!([ + {:insert_section, self(), 0, "s1"}, + {:insert_cell, self(), "s1", 0, :elixir, "c1"}, + {:insert_cell, self(), "s1", 1, :elixir, "c2"}, + {:insert_cell, self(), "s1", 2, :elixir, "c3"}, + {:set_cell_attributes, self(), "c3", %{reevaluate_automatically: true}}, + # Evaluate all cells + {:set_runtime, self(), NoopRuntime.new()}, + {:queue_cell_evaluation, self(), "c1"}, + {:add_cell_evaluation_response, self(), "c1", @eval_resp, @eval_meta}, + {:queue_cell_evaluation, self(), "c2"}, + {:add_cell_evaluation_response, self(), "c2", @eval_resp, @eval_meta}, + {:queue_cell_evaluation, self(), "c3"}, + {:add_cell_evaluation_response, self(), "c3", @eval_resp, @eval_meta}, + # Queue the first cell again + {:queue_cell_evaluation, self(), "c1"} + ]) + + operation = {:add_cell_evaluation_response, self(), "c1", @eval_resp, @eval_meta} + + assert {:ok, + %{ + cell_infos: %{ + "c2" => %{evaluation_status: :evaluating}, + "c3" => %{evaluation_status: :queued} + } + }, _actions} = Data.apply_operation(data, operation) + end + test "adds evaluation time to the response" do data = data_after_operations!([ @@ -2947,24 +3001,30 @@ defmodule Livebook.Session.DataTest do }, _} = Data.apply_operation(data, operation) end - test "given 1 input with reevaluate_automatically setting on, addding a new cell enqueues the first" do + test "setting reevaluate_automatically on stale cell marks it for evaluation" do data = data_after_operations!([ {:insert_section, self(), 0, "s1"}, {:insert_cell, self(), "s1", 0, :elixir, "c1"}, + {:insert_cell, self(), "s1", 1, :elixir, "c2"}, + # Evaluate cells {:set_runtime, self(), NoopRuntime.new()}, - {:set_cell_attributes, self(), "c1", %{reevaluate_automatically: true}}, + {:queue_cell_evaluation, self(), "c1"}, + {:add_cell_evaluation_response, self(), "c1", @eval_resp, @eval_meta}, + {:queue_cell_evaluation, self(), "c2"}, + {:add_cell_evaluation_response, self(), "c2", @eval_resp, @eval_meta}, {:queue_cell_evaluation, self(), "c1"}, {:add_cell_evaluation_response, self(), "c1", @eval_resp, @eval_meta} ]) - operation = {:insert_cell, self(), "s1", 0, :elixir, "c2"} + attrs = %{reevaluate_automatically: true} + operation = {:set_cell_attributes, self(), "c2", attrs} assert {:ok, %{ cell_infos: %{ - "c1" => %{evaluation_status: :queued}, - "c2" => %{evaluation_status: :ready} + "c1" => %{evaluation_status: :ready}, + "c2" => %{evaluation_status: :evaluating} } }, _} = Data.apply_operation(data, operation) end From e0cab46263a5d28e249f8101958824a88db0952b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Tue, 26 Oct 2021 18:34:32 +0200 Subject: [PATCH 09/11] Always show evaluate button when cell is fresh --- lib/livebook_web/live/session_live/cell_component.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/livebook_web/live/session_live/cell_component.ex b/lib/livebook_web/live/session_live/cell_component.ex index f5401971eb2..3dbe57cd240 100644 --- a/lib/livebook_web/live/session_live/cell_component.ex +++ b/lib/livebook_web/live/session_live/cell_component.ex @@ -56,7 +56,7 @@ defmodule LivebookWeb.SessionLive.CellComponent do
<%= if @cell_view.evaluation_status == :ready do %> - <%= if @cell_view.reevaluate_automatically do %> + <%= if @cell_view.validity_status != :fresh and @cell_view.reevaluate_automatically do %> <%= live_patch to: Routes.session_path(@socket, :cell_settings, @session_id, @cell_view.id), class: "text-gray-600 hover:text-gray-800 focus:text-gray-800 flex space-x-1 items-center" do %> <.remix_icon icon="refresh-fill" class="text-xl" /> From 7560e1103ed5fee61cf270f28bca111f76835476 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Tue, 26 Oct 2021 18:51:32 +0200 Subject: [PATCH 10/11] Update icon --- lib/livebook_web/live/session_live/cell_component.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/livebook_web/live/session_live/cell_component.ex b/lib/livebook_web/live/session_live/cell_component.ex index 3dbe57cd240..e76367dd6af 100644 --- a/lib/livebook_web/live/session_live/cell_component.ex +++ b/lib/livebook_web/live/session_live/cell_component.ex @@ -59,7 +59,7 @@ defmodule LivebookWeb.SessionLive.CellComponent do <%= if @cell_view.validity_status != :fresh and @cell_view.reevaluate_automatically do %> <%= live_patch to: Routes.session_path(@socket, :cell_settings, @session_id, @cell_view.id), class: "text-gray-600 hover:text-gray-800 focus:text-gray-800 flex space-x-1 items-center" do %> - <.remix_icon icon="refresh-fill" class="text-xl" /> + <.remix_icon icon="check-line" class="text-xl" /> Reevaluates automatically From 26216dea9d44d9223281026b3c8457e9d463246c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Tue, 26 Oct 2021 20:35:47 +0200 Subject: [PATCH 11/11] Add test ensuring that fresh cells don't evaluate automatically --- test/livebook/session/data_test.exs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/livebook/session/data_test.exs b/test/livebook/session/data_test.exs index 97873c1388d..c7c7531f7b2 100644 --- a/test/livebook/session/data_test.exs +++ b/test/livebook/session/data_test.exs @@ -2127,6 +2127,29 @@ defmodule Livebook.Session.DataTest do }, _actions} = Data.apply_operation(data, operation) end + test "does not mark child automatically reevaluating cells for evaluation if they are fresh" do + data = + data_after_operations!([ + {:insert_section, self(), 0, "s1"}, + {:insert_cell, self(), "s1", 0, :elixir, "c1"}, + {:insert_cell, self(), "s1", 1, :elixir, "c2"}, + {:set_cell_attributes, self(), "c2", %{reevaluate_automatically: true}}, + # Evaluate all cells + {:set_runtime, self(), NoopRuntime.new()}, + {:queue_cell_evaluation, self(), "c1"} + ]) + + operation = {:add_cell_evaluation_response, self(), "c1", @eval_resp, @eval_meta} + + assert {:ok, + %{ + cell_infos: %{ + "c1" => %{evaluation_status: :ready, validity_status: :evaluated}, + "c2" => %{evaluation_status: :ready, validity_status: :fresh} + } + }, _actions} = Data.apply_operation(data, operation) + end + test "adds evaluation time to the response" do data = data_after_operations!([