Hide errors from input nodes (#9633)

When a node has an error/warning/panic that exactly matches one of its input nodes, hide the message until the node is interacted with, showing an icon.

https://github.com/enso-org/enso/assets/1047859/4b1b5e3d-c236-40d7-a3e7-e6ab8182ecd5

# Important Notes
- New icon is used for panics.
- Opening circular menu now shifts any message out of the way, not just warnings.

app/gui2/shared/ast/index.ts

@@ -1,4 +1,5 @@
 import * as random from 'lib0/random'
+import { reachable } from '../util/data/graph'
 import type { ExternalId } from '../yjsModel'
 import type { Module } from './mutableModule'
 import type { SyncTokenId } from './token'
@@ -40,15 +41,10 @@ export function parentId(ast: Ast): AstId | undefined {
 
 /** Returns the given IDs, and the IDs of all their ancestors. */
 export function subtrees(module: Module, ids: Iterable<AstId>) {
-  const subtrees = new Set<AstId>()
-  for (const id of ids) {
-    let ast = module.tryGet(id)
-    while (ast != null && !subtrees.has(ast.id)) {
-      subtrees.add(ast.id)
-      ast = ast.parent()
-    }
-  }
-  return subtrees
+  return reachable(ids, (id) => {
+    const parent = module.tryGet(id)?.parent()
+    return parent ? [parent.id] : []
+  })
 }
 
 /** Returns the IDs of the ASTs that are not descendants of any others in the given set. */

app/gui2/shared/util/data/graph.ts

@@ -0,0 +1,17 @@
+/** Returns the set of all nodes transitively reachable from the given set of roots */
+export function reachable<T>(roots: Iterable<T>, edges: (node: T) => Iterable<T>): Set<T> {
+  const toVisit = [...new Set(roots)]
+  const result = new Set<T>()
+
+  let current: T | undefined
+  while ((current = toVisit.pop())) {
+    for (const nextReachable of edges(current)) {
+      if (!result.has(nextReachable)) {
+        result.add(nextReachable)
+        toVisit.push(nextReachable)
+      }
+    }
+  }
+
+  return result
+}

app/gui2/src/components/GraphEditor/GraphNode.vue

@@ -2,7 +2,11 @@
 import { nodeEditBindings } from '@/bindings'
 import CircularMenu from '@/components/CircularMenu.vue'
 import GraphNodeComment from '@/components/GraphEditor/GraphNodeComment.vue'
-import GraphNodeError from '@/components/GraphEditor/GraphNodeMessage.vue'
+import GraphNodeMessage, {
+  colorForMessageType,
+  iconForMessageType,
+  type MessageType,
+} from '@/components/GraphEditor/GraphNodeMessage.vue'
 import GraphNodeSelection from '@/components/GraphEditor/GraphNodeSelection.vue'
 import GraphVisualization from '@/components/GraphEditor/GraphVisualization.vue'
 import NodeWidgetTree, {
@@ -27,7 +31,7 @@ import { Rect } from '@/util/data/rect'
 import { Vec2 } from '@/util/data/vec2'
 import { displayedIconOf } from '@/util/getIconName'
 import { setIfUndefined } from 'lib0/map'
-import type { VisualizationIdentifier } from 'shared/yjsModel'
+import type { ExternalId, VisualizationIdentifier } from 'shared/yjsModel'
 import type { EffectScope } from 'vue'
 import { computed, effectScope, onScopeDispose, onUnmounted, ref, watch, watchEffect } from 'vue'
 
@@ -88,30 +92,67 @@ const rootNode = ref<HTMLElement>()
 const contentNode = ref<HTMLElement>()
 const nodeSize = useResizeObserver(rootNode)
 
-const error = computed(() => {
+function inputExternalIds() {
+  const externalIds = new Array<ExternalId>()
+  for (const inputId of graph.db.nodeDependents.reverseLookup(nodeId.value)) {
+    const externalId = graph.db.idToExternal(inputId)
+    if (externalId) {
+      externalIds.push(externalId)
+    }
+  }
+  return externalIds
+}
+
+function getPanic(id: ExternalId) {
+  const info = projectStore.computedValueRegistry.db.get(id)
+  return info?.payload.type === 'Panic' ? info.payload.message : undefined
+}
+
+function getDataflowError(id: ExternalId) {
+  return projectStore.dataflowErrors.lookup(id)?.value?.message
+}
+
+interface Message {
+  type: MessageType
+  text: string
+  alwaysShow: boolean
+}
+const availableMessage = computed<Message | undefined>(() => {
   const externalId = graph.db.idToExternal(nodeId.value)
-  if (!externalId) return
+  if (!externalId) return undefined
   const info = projectStore.computedValueRegistry.db.get(externalId)
   switch (info?.payload.type) {
     case 'Panic': {
-      return info.payload.message
+      const text = info.payload.message
+      const alwaysShow = !inputExternalIds().some((id) => getPanic(id) === text)
+      return { type: 'panic', text, alwaysShow } satisfies Message
     }
     case 'DataflowError': {
-      return projectStore.dataflowErrors.lookup(externalId)?.value?.message.split(' (at')[0]
+      const rawText = getDataflowError(externalId)
+      const text = rawText?.split(' (at')[0]
+      if (!text) return undefined
+      const alwaysShow = !inputExternalIds().some((id) => getDataflowError(id) === rawText)
+      return { type: 'error', text, alwaysShow } satisfies Message
+    }
+    case 'Value': {
+      const warning = info.payload.warnings?.value
+      if (!warning) return undefined
+      return {
+        type: 'warning',
+        text: 'Warning: ' + warning,
+        alwaysShow: false,
+      } satisfies Message
     }
     default:
       return undefined
   }
 })
 
-const warning = computed(() => {
-  const externalId = graph.db.idToExternal(nodeId.value)
-  if (!externalId) return
-  const info = projectStore.computedValueRegistry.db.get(externalId)
-  const warning = info?.payload.type === 'Value' ? info.payload.warnings?.value : undefined
-  if (!warning) return
-  return 'Warning: ' + warning!
-})
+const visibleMessage = computed(
+  () =>
+    (availableMessage.value?.alwaysShow || nodeHovered.value || selected.value) &&
+    availableMessage.value,
+)
 
 const nodeHovered = ref(false)
 
@@ -517,16 +558,18 @@ const documentation = computed<string | undefined>({
       />
     </div>
     <div class="statuses">
-      <SvgIcon v-if="warning" name="warning" />
+      <SvgIcon
+        v-if="availableMessage && !visibleMessage"
+        :name="iconForMessageType[availableMessage.type]"
+        :style="{ color: colorForMessageType[availableMessage.type] }"
+      />
     </div>
-    <GraphNodeError v-if="error" class="afterNode" :message="error" type="error" />
-    <GraphNodeError
-      v-if="warning && (nodeHovered || selected)"
-      class="afterNode warning"
+    <GraphNodeMessage
+      v-if="visibleMessage"
+      class="afterNode"
       :class="{ messageWithMenu: menuVisible }"
-      :message="warning"
-      icon="warning"
-      type="warning"
+      :message="visibleMessage.text"
+      :type="visibleMessage.type"
     />
     <svg class="bgPaths" :style="bgStyleVariables">
       <rect class="bgFill" />
@@ -721,10 +764,6 @@ const documentation = computed<string | undefined>({
   margin-top: 4px;
 }
 
-.messageWarning {
-  margin-top: 8px;
-}
-
 .messageWithMenu {
   left: 40px;
 }
@@ -739,7 +778,6 @@ const documentation = computed<string | undefined>({
   top: 0;
   right: 100%;
   margin-right: 8px;
-  color: var(--color-warning);
   transition: opacity 0.2s ease-in-out;
 }
 

app/gui2/src/components/GraphEditor/GraphNodeMessage.vue

@@ -1,35 +1,35 @@
 <script setup lang="ts">
 import SvgIcon from '@/components/SvgIcon.vue'
 import type { Icon } from '@/util/iconName'
-import { computed } from 'vue'
-
-/** The type of a message. */
-export type GraphNodeMessageType = 'error' | 'warning'
 
 const props = defineProps<{
   message: string
-  type: GraphNodeMessageType
-  icon?: Icon
+  type: MessageType
 }>()
-
-const icon = computed(() => iconForType[props.type])
 </script>
 
 <script lang="ts">
-const styleClassForType: Record<GraphNodeMessageType, string> = {
-  error: 'GraphNodeError',
-  warning: 'GraphNodeWarning',
-}
-
-const iconForType: Record<GraphNodeMessageType, Icon | undefined> = {
+/** The type of a message. */
+export type MessageType = 'error' | 'warning' | 'panic'
+export const iconForMessageType: Record<MessageType, Icon> = {
   error: 'error',
   warning: 'warning',
+  panic: 'panic',
+}
+export const colorForMessageType: Record<MessageType, string> = {
+  error: 'var(--color-error)',
+  warning: 'var(--color-warning)',
+  panic: 'var(--color-error)',
 }
 </script>
 
 <template>
-  <div class="GraphNodeMessage" :class="styleClassForType[props.type]">
-    <SvgIcon v-if="icon" class="icon" :name="icon" />
+  <div
+    class="GraphNodeMessage"
+    :class="props.type"
+    :style="{ backgroundColor: colorForMessageType[props.type] }"
+  >
+    <SvgIcon class="icon" :name="iconForMessageType[props.type]" />
     <div v-text="props.message"></div>
   </div>
 </template>
@@ -48,14 +48,6 @@ const iconForType: Record<GraphNodeMessageType, Icon | undefined> = {
   line-height: 20px;
 }
 
-.GraphNodeWarning {
-  background-color: #faa212;
-}
-
-.GraphNodeError {
-  background-color: #e85252;
-}
-
 .icon {
   margin: auto 0;
 }

app/gui2/src/stores/graph/__tests__/graphDatabase.test.ts

@@ -79,7 +79,7 @@ test('Reading graph from definition', () => {
   expect(Array.from(db.connections.lookup(id(3)))).toEqual([id(9)])
   // expect(db.getOutputPortIdentifier(id(2))).toBe('a')
   expect(db.getOutputPortIdentifier(id(3))).toBe('node1')
-  expect(Array.from(db.dependantNodes(asNodeId(id(4))))).toEqual([id(8), id(12)])
-  expect(Array.from(db.dependantNodes(asNodeId(id(8))))).toEqual([id(12)])
-  expect(Array.from(db.dependantNodes(asNodeId(id(12))))).toEqual([])
+  expect(Array.from(db.nodeDependents.lookup(asNodeId(id(4))))).toEqual([id(8)])
+  expect(Array.from(db.nodeDependents.lookup(asNodeId(id(8))))).toEqual([id(12)])
+  expect(Array.from(db.nodeDependents.lookup(asNodeId(id(12))))).toEqual([])
 })

app/gui2/src/stores/graph/graphDatabase.ts

@@ -161,6 +161,19 @@ export class GraphDb {
     return Array.from(this.connectionsFromBindings(info, alias, srcNode))
   })
 
+  nodeDependents = new ReactiveIndex(this.nodeIdToNode, (id) => {
+    const result = new Set<NodeId>()
+    const outputPorts = this.nodeOutputPorts.lookup(id)
+    for (const outputPort of outputPorts) {
+      const connectedPorts = this.connections.lookup(outputPort)
+      for (const port of connectedPorts) {
+        const portNode = this.getExpressionNodeId(port)
+        if (portNode != null) result.add(portNode)
+      }
+    }
+    return Array.from(result, (target) => [id, target])
+  })
+
   private *connectionsFromBindings(
     info: BindingInfo,
     alias: AstId,
@@ -254,32 +267,6 @@ export class GraphDb {
     )
   }
 
-  /**
-   * Get a list of all nodes that depend on given node. Includes transitive dependencies.
-   */
-  dependantNodes(id: NodeId): Set<NodeId> {
-    const toVisit = [id]
-    const result = new Set<NodeId>()
-
-    let currentNode: NodeId | undefined
-    while ((currentNode = toVisit.pop())) {
-      const outputPorts = this.nodeOutputPorts.lookup(currentNode)
-      for (const outputPort of outputPorts) {
-        const connectedPorts = this.connections.lookup(outputPort)
-        for (const port of connectedPorts) {
-          const portNode = this.getExpressionNodeId(port)
-          if (portNode == null) continue
-          if (!result.has(portNode)) {
-            result.add(portNode)
-            toVisit.push(portNode)
-          }
-        }
-      }
-    }
-
-    return result
-  }
-
   getMethodCallInfo(
     id: AstId,
   ):

app/gui2/src/stores/graph/index.ts

@@ -33,6 +33,7 @@ import { iteratorFilter } from 'lib0/iterator'
 import { defineStore } from 'pinia'
 import { SourceDocument } from 'shared/ast/sourceDocument'
 import type { ExpressionUpdate, StackItem } from 'shared/languageServerTypes'
+import { reachable } from 'shared/util/data/graph'
 import type {
   LocalUserActionOrigin,
   Origin,
@@ -662,7 +663,7 @@ export const useGraphStore = defineStore('graph', () => {
     // If source is placed after its new target, the nodes needs to be reordered.
     if (sourceIdx > targetIdx) {
       // Find all transitive dependencies of the moved target node.
-      const deps = db.dependantNodes(targetNodeId)
+      const deps = reachable([targetNodeId], (node) => db.nodeDependents.lookup(node))
 
       const dependantLines = new Set(
         Array.from(deps, (id) => db.nodeIdToNode.get(id)?.outerExpr.id),