Engine should send notification about node status (#3729)

When nodes get invalidated in the cache, they have to be recomputed. Let the IDE know which of the nodes are pending by sending `Api.ExpressionUpdate.Payload.Pending` message. # Important Notes This PR introduces new `Api.ExpressionUpdate.Payload.Pending` message. This message is delivered before re-computation of nodes. Later `Api.ExpressionUpdate.Payload.Value` or other is sent to notify the IDE that a value for given node is available. Trivial implementation of of the `Api.ExpressionUpdate.Payload.Pending` message in the IDE is provided by this PR to (improperly) visualize pending node status - further improvements needed in follow up PRs.
enso-org · Sep 28, 2022 · 835ac05 · 835ac05
1 parent 7da4d61
commit 835ac05
Show file tree

Hide file tree

Showing 20 changed files with 897 additions and 239 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -372,6 +372,7 @@
 - [Invalidate module's IR cache if imported module changed][3703]
 - [Don't rename imported Main module that only imports names][3710]
 - [Distinguish static and instance methods][3740]
+- [Notify node status to the IDE][3729]
 
 [3227]: https://github.com/enso-org/enso/pull/3227
 [3248]: https://github.com/enso-org/enso/pull/3248
@@ -418,8 +419,9 @@
 [3658]: https://github.com/enso-org/enso/pull/3658
 [3671]: https://github.com/enso-org/enso/pull/3671
 [3696]: https://github.com/enso-org/enso/pull/3696
-[3696]: https://github.com/enso-org/enso/pull/3703
-[3696]: https://github.com/enso-org/enso/pull/3710
+[3703]: https://github.com/enso-org/enso/pull/3703
+[3710]: https://github.com/enso-org/enso/pull/3710
+[3729]: https://github.com/enso-org/enso/pull/3729
 [3740]: https://github.com/enso-org/enso/pull/3740
 
 # Enso 2.0.0-alpha.18 (2021-10-12)

diff --git a/app/gui/controller/engine-protocol/src/language_server/types.rs b/app/gui/controller/engine-protocol/src/language_server/types.rs
@@ -236,6 +236,11 @@ pub enum ExpressionUpdatePayload {
         message: String,
         trace:   Vec<ExpressionId>,
     },
+    #[serde(rename_all = "camelCase")]
+    Pending {
+        message:  Option<String>,
+        progress: Option<f64>,
+    },
 }
 
 

diff --git a/app/gui/src/presenter/graph/state.rs b/app/gui/src/presenter/graph/state.rs
@@ -299,7 +299,7 @@ impl Expressions {
 ///
 /// This structure keeps the information how the particular graph elements received from controllers
 /// are represented in the view. It also handles updates from the controllers and
-/// the view in `update_from_controller` and `update_from_view` respectively.  
+/// the view in `update_from_controller` and `update_from_view` respectively.
 #[derive(Clone, Debug, Default)]
 pub struct State {
     nodes:       RefCell<Nodes>,
@@ -492,6 +492,7 @@ impl<'a> ControllerChange<'a> {
             None | Some(Value) => None,
             Some(DataflowError { trace }) => Some((Kind::Dataflow, None, trace)),
             Some(Panic { message, trace }) => Some((Kind::Panic, Some(message), trace)),
+            Some(Pending { .. }) => None,
         }?;
         let propagated = if kind == Kind::Panic {
             let nodes = self.nodes.borrow();
@@ -681,15 +682,15 @@ impl<'a> ViewChange<'a> {
 
 impl<'a> ViewChange<'a> {
     /// If the connections does not already exist, it is created and corresponding to-be-created
-    /// Ast connection is returned.  
+    /// Ast connection is returned.
     pub fn create_connection(&self, connection: view::graph_editor::Edge) -> Option<AstConnection> {
         let source = connection.source()?;
         let target = connection.target()?;
         self.create_connection_from_endpoints(connection.id(), source, target)
     }
 
     /// If the connections with provided endpoints does not already exist, it is created and
-    /// corresponding to-be-created Ast connection is returned.  
+    /// corresponding to-be-created Ast connection is returned.
     pub fn create_connection_from_endpoints(
         &self,
         connection: ViewConnection,

diff --git a/docs/language-server/protocol-language-server.md b/docs/language-server/protocol-language-server.md
@@ -341,7 +341,7 @@ interface ExpressionUpdate {
 An information about the computed value.
 
 ```typescript
-type ExpressionUpdatePayload = Value | DatafalowError | Panic;
+type ExpressionUpdatePayload = Value | DatafalowError | Panic | Pending;
 
 /**
  * An empty payload. Indicates that the expression was computed to a value.
@@ -372,6 +372,22 @@ interface Panic {
    */
   trace: ExpressionId[];
 }
+
+/**
+ * Indicates the expression is currently being computed. Optionally it
+ * provides description and percentage (`0.0-1.0`) of completeness.
+ */
+interface Pending {
+  /**
+   * Optional message describing current operation.
+   */
+  message?: String;
+
+  /**
+   * Optional amount of already done work as a number between `0.0` to `1.0`.
+   */
+  progress?: Number;
+}
 ```
 
 ### `VisualisationConfiguration`

diff --git a/...anguage-server/src/main/scala/org/enso/languageserver/runtime/ContextEventsListener.scala b/...anguage-server/src/main/scala/org/enso/languageserver/runtime/ContextEventsListener.scala
@@ -247,6 +247,9 @@ final class ContextEventsListener(
       case Api.ExpressionUpdate.Payload.Value() =>
         ContextRegistryProtocol.ExpressionUpdate.Payload.Value
 
+      case Api.ExpressionUpdate.Payload.Pending(m, p) =>
+        ContextRegistryProtocol.ExpressionUpdate.Payload.Pending(m, p)
+
       case Api.ExpressionUpdate.Payload.DataflowError(trace) =>
         ContextRegistryProtocol.ExpressionUpdate.Payload.DataflowError(trace)
 

diff --git a/...guage-server/src/main/scala/org/enso/languageserver/runtime/ContextRegistryProtocol.scala b/...guage-server/src/main/scala/org/enso/languageserver/runtime/ContextRegistryProtocol.scala
@@ -161,6 +161,9 @@ object ContextRegistryProtocol {
       /** An information about computed expression. */
       case object Value extends Payload
 
+      case class Pending(message: Option[String], progress: Option[Double])
+          extends Payload;
+
       /** Indicates that the expression was computed to an error.
         *
         * @param trace the list of expressions leading to the root error.
@@ -186,6 +189,8 @@ object ContextRegistryProtocol {
 
         val Value = "Value"
 
+        val Pending = "Pending"
+
         val DataflowError = "DataflowError"
 
         val Panic = "Panic"
@@ -210,6 +215,13 @@ object ContextRegistryProtocol {
               .deepMerge(
                 Json.obj(CodecField.Type -> PayloadType.Panic.asJson)
               )
+
+          case m: Payload.Pending =>
+            Encoder[Payload.Pending]
+              .apply(m)
+              .deepMerge(
+                Json.obj(CodecField.Type -> PayloadType.Pending.asJson)
+              )
         }
 
       implicit val decoder: Decoder[Payload] =
@@ -223,6 +235,9 @@ object ContextRegistryProtocol {
 
             case PayloadType.Panic =>
               Decoder[Payload.Panic].tryDecode(cursor)
+
+            case PayloadType.Pending =>
+              Decoder[Payload.Pending].tryDecode(cursor)
           }
         }
     }

diff --git a/engine/polyglot-api/src/main/scala/org/enso/polyglot/runtime/Runtime.scala b/engine/polyglot-api/src/main/scala/org/enso/polyglot/runtime/Runtime.scala
@@ -367,6 +367,10 @@ object Runtime {
           new JsonSubTypes.Type(
             value = classOf[Payload.Panic],
             name  = "expressionUpdatePayloadPanic"
+          ),
+          new JsonSubTypes.Type(
+            value = classOf[Payload.Pending],
+            name  = "expressionUpdatePayloadPending"
           )
         )
       )
@@ -378,6 +382,11 @@ object Runtime {
           */
         case class Value() extends Payload
 
+        /** TBD
+          */
+        case class Pending(message: Option[String], progress: Option[Double])
+            extends Payload;
+
         /** Indicates that the expression was computed to an error.
           *
           * @param trace the list of expressions leading to the root error.

diff --git a/...ent-id-execution/src/main/java/org/enso/interpreter/instrument/IdExecutionInstrument.java b/...ent-id-execution/src/main/java/org/enso/interpreter/instrument/IdExecutionInstrument.java
@@ -205,9 +205,7 @@ private void onExpressionReturn(Object result, Node node, EventContext context)
         ExpressionValue expressionValue =
                 new ExpressionValue(
                         nodeId, result, resultType, cachedType, call, cachedCall, profilingInfo, false);
-        if (expressionValue.isTypeChanged() || expressionValue.isFunctionCallChanged()) {
-            syncState.setExpressionUnsync(nodeId);
-        }
+        syncState.setExpressionUnsync(nodeId);
         syncState.setVisualisationUnsync(nodeId);
 
         // Panics are not cached because a panic can be fixed by changing seemingly unrelated code,

diff --git a/...nstruments/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java b/...nstruments/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java
@@ -16,6 +16,7 @@
 import org.enso.polyglot.runtime.Runtime$Api$CreateContextResponse;
 import org.enso.polyglot.runtime.Runtime$Api$EditFileNotification;
 import org.enso.polyglot.runtime.Runtime$Api$ExecutionFailed;
+import org.enso.polyglot.runtime.Runtime$Api$ExpressionUpdates;
 import org.enso.polyglot.runtime.Runtime$Api$InitializedNotification;
 import org.enso.polyglot.runtime.Runtime$Api$MethodPointer;
 import org.enso.polyglot.runtime.Runtime$Api$OpenFileNotification;
@@ -35,6 +36,8 @@
 import scala.Option;
 import scala.collection.immutable.List;
 import scala.collection.immutable.Seq;
+import scala.collection.immutable.Set;
+import scala.collection.immutable.Set$;
 import scala.collection.immutable.Vector1;
 
 public class IncrementalUpdatesTest {
@@ -168,7 +171,7 @@ private static String extractPositions(String code, String chars, Map<Character,
 
     Function<Character, UUID> registerRegion = (ch) -> {
       int[] beginAndLength = pos.get(ch);
-      return metadata.addItem(beginAndLength[0], beginAndLength[1]);
+      return metadata.addItem(beginAndLength[0], beginAndLength[1], null);
     };
     // foo definition
     registerRegion.apply('&');
@@ -213,10 +216,10 @@ private static String extractPositions(String code, String chars, Map<Character,
       )
     );
 
-    assertSameElements(context.receiveNIgnoreStdLib(4, 10),
+    assertSameElements(context.receiveNIgnorePendingExpressionUpdates(4, 10, emptySet()),
       Response(requestId, new Runtime$Api$PushContextResponse(contextId)),
       TestMessages.update(contextId, mainFoo, exprType, new Runtime$Api$MethodPointer("Enso_Test.Test.Main", "Enso_Test.Test.Main", "foo")),
-      TestMessages.update(contextId, mainRes, ConstantsGen.NOTHING),
+      TestMessages.update(contextId, mainRes, ConstantsGen.NOTHING, false),
       context.executionComplete(contextId)
     );
     assertEquals(List.newBuilder().addOne(originalOutput), context.consumeOut());
@@ -230,10 +233,10 @@ private static String extractPositions(String code, String chars, Map<Character,
         new Runtime$Api$PushContextRequest(contextId, new Runtime$Api$StackItem$LocalCall(mainFoo))
       )
     );
-    assertSameElements(context.receiveN(4, 10),
+    assertSameElements(context.receiveNIgnorePendingExpressionUpdates(4, 10, emptySet()),
       Response(requestId, new Runtime$Api$PushContextResponse(contextId)),
-      TestMessages.update(contextId, fooX, exprType),
-      TestMessages.update(contextId, fooRes, exprType),
+      TestMessages.update(contextId, fooX, exprType, false),
+      TestMessages.update(contextId, fooRes, exprType, false),
       context.executionComplete(contextId)
     );
     assertEquals(List.newBuilder().addOne(originalOutput), context.consumeOut());
@@ -265,7 +268,7 @@ private static String extractPositions(String code, String chars, Map<Character,
         ),
         true
     )));
-    return context.receiveN(1, 10);
+    return context.receiveNIgnorePendingExpressionUpdates(1, 10, emptySet());
   }
 
   private List<Runtime$Api$Response> sendExpressionValue(String originalText, String newText) {
@@ -281,7 +284,7 @@ private static String extractPositions(String code, String chars, Map<Character,
       UUID.randomUUID(),
       newText
     )));
-    return context.receiveN(1, 10);
+    return context.receiveNIgnoreExpressionUpdates(1, 10);
   }
 
   private <T extends Node> T findLiteralNode(Class<T> type, Map<Class, java.util.List<Node>> nodes) {
@@ -295,6 +298,11 @@ private static void assertSameElements(List<Runtime$Api$Response> actual, Runtim
     assertEquals("Same size: " + actual, seq.length, actual.size());
     for (int i = 0; i < seq.length; i++) {
       var real = actual.drop(i).head();
+      if (real instanceof Runtime$Api$Response response) {
+        if (response.payload() instanceof Runtime$Api$ExpressionUpdates) {
+          continue;
+        }
+      }
       assertEquals("Check on #" + i, seq[i], real);
     }
   }
@@ -313,6 +321,10 @@ private static <T> Option<T> None() {
     return (Option<T>) scala.None$.MODULE$;
   }
 
+  private static <T> Set<T> emptySet() {
+    return Set$.MODULE$.empty();
+  }
+
   private static Runtime$Api$Request Request(UUID id, org.enso.polyglot.runtime.Runtime.ApiRequest request) {
     return org.enso.polyglot.runtime.Runtime$Api$Request$.MODULE$.apply(id, request);
   }