Keep binding in reversed evaluation order (#1065)

* Keep binding in reversed evaluation order

* Treat rebound names as new

* Add test for the default order

* Optimise binding reorder

* Update lib/livebook/runtime/evaluator.ex

Co-authored-by: José Valim <jose.valim@dashbit.co>

* Apply review comments

Co-authored-by: José Valim <jose.valim@dashbit.co>

lib/livebook/runtime/evaluator.ex

@@ -322,6 +322,7 @@ defmodule Livebook.Runtime.Evaluator do
     {result_context, result, code_error} =
       case eval(code, context.binding, context.env) do
         {:ok, value, binding, env} ->
+          binding = reorder_binding(binding, context.binding)
           result_context = %{binding: binding, env: env, id: random_id()}
           result = {:ok, value}
           {result_context, result, nil}
@@ -454,6 +455,24 @@ defmodule Livebook.Runtime.Evaluator do
   defp code_error?(%CompileError{}), do: true
   defp code_error?(_error), do: false
 
+  defp reorder_binding(binding, prev_binding) do
+    # We keep the order of existing binding entries and move the new
+    # ones to the beginning
+
+    binding_map = Map.new(binding)
+
+    unchanged_binding =
+      Enum.filter(prev_binding, fn {key, prev_val} ->
+        val = binding_map[key]
+        :erts_debug.same(val, prev_val)
+      end)
+
+    unchanged_binding
+    |> Enum.reduce(binding_map, fn {key, _}, acc -> Map.delete(acc, key) end)
+    |> Map.to_list()
+    |> Kernel.++(unchanged_binding)
+  end
+
   # Adapted from https://github.com/elixir-lang/elixir/blob/1c1654c88adfdbef38ff07fc30f6fbd34a542c07/lib/iex/lib/iex/evaluator.ex#L355-L372
   # TODO: Remove else branch once we depend on the versions below
   if System.otp_release() >= "25" and Version.match?(System.version(), "~> 1.14-dev") do

test/livebook/runtime/evaluator_test.exs

@@ -310,6 +310,27 @@ defmodule Livebook.Runtime.EvaluatorTest do
     end
   end
 
+  describe "binding order" do
+    test "keeps binding in evaluation order, starting from most recent", %{evaluator: evaluator} do
+      Evaluator.evaluate_code(evaluator, "b = 1", :code_1)
+      Evaluator.evaluate_code(evaluator, "a = 1", :code_2, :code_1)
+      Evaluator.evaluate_code(evaluator, "c = 1", :code_3, :code_2)
+      Evaluator.evaluate_code(evaluator, "x = 1", :code_4, :code_3)
+
+      {:ok, %{binding: binding}} = Evaluator.fetch_evaluation_context(evaluator, :code_4)
+      assert [:x, :c, :a, :b] == Enum.map(binding, &elem(&1, 0))
+    end
+
+    test "treats rebound names as new", %{evaluator: evaluator} do
+      Evaluator.evaluate_code(evaluator, "b = 1", :code_1)
+      Evaluator.evaluate_code(evaluator, "a = 1", :code_2, :code_1)
+      Evaluator.evaluate_code(evaluator, "b = 2", :code_3, :code_2)
+
+      {:ok, %{binding: binding}} = Evaluator.fetch_evaluation_context(evaluator, :code_3)
+      assert [:b, :a] == Enum.map(binding, &elem(&1, 0))
+    end
+  end
+
   # Helpers
 
   # Some of the code passed to Evaluator above is expected