From 6d0da78370d203cfe2ae267612c4f9792e26afbe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonatan=20K=C5=82osko?= <jonatanklosko@gmail.com>
Date: Tue, 22 Jun 2021 15:06:16 +0200
Subject: [PATCH] Make it possible to mark evaluating cell as stale upfront
 (#378)

* Make it possible to mark evaluating cell as stale upfront

* Show evaluation time on stale cells as well
---
 lib/livebook/session/data.ex                  | 11 +++++--
 .../live/session_live/cell_component.ex       |  7 +++--
 test/livebook/session/data_test.exs           | 31 +++++++++++++++++--
 3 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/lib/livebook/session/data.ex b/lib/livebook/session/data.ex
index f049899a5..b059c307f 100644
--- a/lib/livebook/session/data.ex
+++ b/lib/livebook/session/data.ex
@@ -606,7 +606,6 @@ defmodule Livebook.Session.Data do
   defp finish_cell_evaluation(data_actions, cell, section, metadata) do
     data_actions
     |> set_cell_info!(cell.id,
-      validity_status: :evaluated,
       evaluation_status: :ready,
       evaluation_time_ms: metadata.evaluation_time_ms
     )
@@ -666,7 +665,15 @@ defmodule Livebook.Session.Data do
             data_actions
             |> set!(notebook: Notebook.update_cell(data.notebook, id, &%{&1 | outputs: []}))
             |> update_cell_info!(id, fn info ->
-              %{info | evaluation_status: :evaluating, evaluation_digest: nil}
+              %{
+                info
+                | evaluation_status: :evaluating,
+                  evaluation_digest: nil,
+                  # During evaluation notebook changes may invalidate the cell,
+                  # so we mark it as up-to-date straight away and possibly mark
+                  # it as stale during evaluation
+                  validity_status: :evaluated
+              }
             end)
             |> set_section_info!(section.id, evaluating_cell_id: id, evaluation_queue: ids)
             |> add_action({:start_evaluation, cell, section})
diff --git a/lib/livebook_web/live/session_live/cell_component.ex b/lib/livebook_web/live/session_live/cell_component.ex
index 8edfb8eef..d07170f9a 100644
--- a/lib/livebook_web/live/session_live/cell_component.ex
+++ b/lib/livebook_web/live/session_live/cell_component.ex
@@ -378,8 +378,11 @@ defmodule LivebookWeb.SessionLive.CellComponent do
     )
   end
 
-  defp render_cell_status(:stale, _, _) do
-    render_status_indicator("Stale", "bg-yellow-200", change_indicator: true)
+  defp render_cell_status(:stale, _, evaluation_time_ms) do
+    render_status_indicator("Stale", "bg-yellow-200",
+      change_indicator: true,
+      tooltip: evaluated_label(evaluation_time_ms)
+    )
   end
 
   defp render_cell_status(:aborted, _, _) do
diff --git a/test/livebook/session/data_test.exs b/test/livebook/session/data_test.exs
index 1169bd388..70894cbf6 100644
--- a/test/livebook/session/data_test.exs
+++ b/test/livebook/session/data_test.exs
@@ -1003,12 +1003,11 @@ defmodule Livebook.Session.DataTest do
               }, []} = Data.apply_operation(data, operation)
     end
 
-    test "marks the cell as evaluated" do
+    test "marks the cell as ready" 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_runtime, self(), NoopRuntime.new()},
           {:queue_cell_evaluation, self(), "c1"}
         ])
@@ -1018,11 +1017,37 @@ defmodule Livebook.Session.DataTest do
 
       assert {:ok,
               %{
-                cell_infos: %{"c1" => %{validity_status: :evaluated, evaluation_status: :ready}},
+                cell_infos: %{"c1" => %{evaluation_status: :ready}},
                 section_infos: %{"s1" => %{evaluating_cell_id: nil, evaluation_queue: []}}
               }, []} = Data.apply_operation(data, operation)
     end
 
+    test "preserves validity status" 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_runtime, self(), NoopRuntime.new()},
+          # Evaluate the first cell
+          {:queue_cell_evaluation, self(), "c1"},
+          {:add_cell_evaluation_response, self(), "c1", {:ok, [1, 2, 3]},
+           %{evaluation_time_ms: 10}},
+          # Start evaluating the second cell
+          {:queue_cell_evaluation, self(), "c2"},
+          # Remove the first cell, marking the second as stale
+          {:delete_cell, self(), "c1"}
+        ])
+
+      operation =
+        {:add_cell_evaluation_response, self(), "c2", {:ok, [1, 2, 3]}, %{evaluation_time_ms: 10}}
+
+      assert {:ok,
+              %{
+                cell_infos: %{"c2" => %{validity_status: :stale}}
+              }, []} = Data.apply_operation(data, operation)
+    end
+
     test "marks next queued cell in this section as evaluating if there is one" do
       data =
         data_after_operations!([