Highlight experiments with errors (#2072)

* implement basic errors functionality

* add partial errors

* refactor errors in experiments webview

* pull out type

* fix test

* use pre-wrap for tooltip (inserts line breaks)

* use cell contents component

* add tooltip test in tree

* add test for error handling in extract columns

* update tests after merging main

.vscode/settings.json

@@ -15,6 +15,7 @@
     "conda",
     "datapoint",
     "datapoints",
+    "Decoratable",
     "dvcignore",
     "DVCLIVE",
     "DVCPATH",

extension/src/experiments/columns/extract.test.ts

@@ -0,0 +1,21 @@
+import { extractColumns } from './extract'
+
+describe('extractColumns', () => {
+  it('should handle concatenating errors', () => {
+    const data = {
+      metrics: {
+        'summary.json': {
+          error: { msg: 'metrics file is busted', type: 'fatal' }
+        }
+      },
+      params: {
+        'params.yaml': {
+          error: { msg: 'this is also broken', type: 'impossible' }
+        }
+      }
+    }
+
+    const { error } = extractColumns(data)
+    expect(error).toStrictEqual('metrics file is busted\nthis is also broken')
+  })
+})

extension/src/experiments/columns/extract.ts

@@ -1,25 +1,45 @@
-import { Deps, ExperimentFields, ValueTreeRoot } from '../../cli/reader'
+import {
+  Deps,
+  ExperimentFields,
+  ValueTreeOrError,
+  ValueTreeRoot
+} from '../../cli/reader'
 import { shortenForLabel } from '../../util/string'
 import {
   DepColumns,
   Experiment,
   MetricOrParamColumns
 } from '../webview/contract'
 
+const extractFileMetricsOrParams = (
+  acc: { columns: MetricOrParamColumns; errors: string[] },
+  file: string,
+  dataOrError: ValueTreeOrError
+) => {
+  const data = dataOrError?.data
+  const error = dataOrError?.error?.msg
+  if (error) {
+    acc.errors.push(error)
+  }
+  if (!data) {
+    return
+  }
+  acc.columns[file] = data
+}
+
 const extractMetricsOrParams = (
-  columns?: ValueTreeRoot
-): MetricOrParamColumns | undefined => {
-  if (!columns) {
+  valueTreeRoot?: ValueTreeRoot
+): { columns: MetricOrParamColumns; errors: string[] } | undefined => {
+  if (!valueTreeRoot) {
     return
   }
-  const acc: MetricOrParamColumns = {}
+  const acc: { columns: MetricOrParamColumns; errors: string[] } = {
+    columns: {},
+    errors: []
+  }
 
-  for (const [file, dataOrError] of Object.entries(columns)) {
-    const data = dataOrError?.data
-    if (!data) {
-      continue
-    }
-    acc[file] = data
+  for (const [file, dataOrError] of Object.entries(valueTreeRoot)) {
+    extractFileMetricsOrParams(acc, file, dataOrError)
   }
 
   return acc
@@ -46,15 +66,34 @@ const extractDeps = (
   return acc
 }
 
-export const extractColumns = (
-  experiment: ExperimentFields,
-  branch?: Experiment
-): {
+type Columns = {
+  error?: string
   deps: DepColumns | undefined
   metrics: MetricOrParamColumns | undefined
   params: MetricOrParamColumns | undefined
-} => ({
-  deps: extractDeps(experiment.deps, branch),
-  metrics: extractMetricsOrParams(experiment.metrics),
-  params: extractMetricsOrParams(experiment.params)
-})
+}
+
+export const extractColumns = (
+  experiment: ExperimentFields,
+  branch?: Experiment
+): Columns => {
+  const metricsData = extractMetricsOrParams(experiment.metrics)
+  const paramsData = extractMetricsOrParams(experiment.params)
+
+  const error = [
+    ...(metricsData?.errors || []),
+    ...(paramsData?.errors || [])
+  ].join('\n')
+
+  const columns: Columns = {
+    deps: extractDeps(experiment.deps, branch),
+    metrics: metricsData?.columns,
+    params: paramsData?.columns
+  }
+
+  if (error) {
+    columns.error = error
+  }
+
+  return columns
+}

extension/src/experiments/index.ts

@@ -16,7 +16,7 @@ import { ExperimentsData } from './data'
 import { askToDisableAutoApplyFilters } from './toast'
 import { Experiment, ColumnType, TableData } from './webview/contract'
 import { WebviewMessages } from './webview/messages'
-import { DecorationProvider } from './model/filterBy/decorationProvider'
+import { DecorationProvider } from './model/decorationProvider'
 import { ResourceLocator } from '../resourceLocator'
 import { AvailableCommands, InternalCommands } from '../commands/internal'
 import { ExperimentsOutput } from '../cli/reader'
@@ -425,7 +425,8 @@ export class Experiments extends BaseRepository<TableData> {
   private notifyChanged(data?: ExperimentsOutput) {
     this.decorationProvider.setState(
       this.experiments.getLabels(),
-      this.experiments.getLabelsToDecorate()
+      this.experiments.getLabelsToDecorate(),
+      this.experiments.getErrors()
     )
     this.experimentsChanged.fire(data)
     this.notifyColumnsChanged()

extension/src/experiments/model/collect.ts

@@ -1,4 +1,9 @@
-import { ThemeIcon, TreeItemCollapsibleState, Uri } from 'vscode'
+import {
+  MarkdownString,
+  ThemeIcon,
+  TreeItemCollapsibleState,
+  Uri
+} from 'vscode'
 import omit from 'lodash.omit'
 import { ExperimentType } from '.'
 import { ExperimentsAccumulator } from './accumulator'
@@ -29,6 +34,7 @@ export type ExperimentItem = {
   collapsibleState: TreeItemCollapsibleState
   type: ExperimentType
   iconPath: ThemeIcon | Uri | Resource
+  tooltip: MarkdownString | undefined
 }
 
 type ExperimentsObject = { [sha: string]: ExperimentFieldsOrError }
@@ -40,8 +46,12 @@ export const isCheckpoint = (
 
 const getExperimentId = (
   sha: string,
-  experimentsFields: ExperimentFields
+  experimentsFields?: ExperimentFields
 ): string => {
+  if (!experimentsFields) {
+    return sha
+  }
+
   const { name, checkpoint_tip } = experimentsFields
 
   if (isCheckpoint(checkpoint_tip, sha)) {
@@ -115,7 +125,10 @@ const transformColumns = (
   experimentFields: ExperimentFields,
   branch?: Experiment
 ) => {
-  const { metrics, params, deps } = extractColumns(experimentFields, branch)
+  const { error, metrics, params, deps } = extractColumns(
+    experimentFields,
+    branch
+  )
 
   if (metrics) {
     experiment.metrics = metrics
@@ -126,21 +139,28 @@ const transformColumns = (
   if (deps) {
     experiment.deps = deps
   }
+  if (error) {
+    experiment.error = error
+  }
 }
 
 const transformExperimentData = (
   id: string,
-  experimentFields: ExperimentFields,
+  experimentFieldsOrError: ExperimentFieldsOrError,
   label: string | undefined,
   sha?: string,
   displayNameOrParent?: string,
   logicalGroupName?: string,
   branch?: Experiment
 ): Experiment => {
+  const data = experimentFieldsOrError.data || {}
+
+  const error = experimentFieldsOrError.error
+
   const experiment = {
     id,
     label,
-    ...omit(experimentFields, Object.values(ColumnType))
+    ...omit(data, Object.values(ColumnType))
   } as Experiment
 
   if (displayNameOrParent) {
@@ -155,7 +175,11 @@ const transformExperimentData = (
     experiment.sha = sha
   }
 
-  transformColumns(experiment, experimentFields, branch)
+  if (error) {
+    experiment.error = error.msg
+  }
+
+  transformColumns(experiment, data, branch)
 
   return experiment
 }
@@ -172,7 +196,8 @@ const transformExperimentOrCheckpointData = (
 } => {
   const experimentFields = experimentData.data
   if (!experimentFields) {
-    return { experiment: undefined }
+    const error = experimentData?.error?.msg
+    return { experiment: { error, id: sha, label: shortenForLabel(sha) } }
   }
 
   const checkpointTipId = getCheckpointTipId(
@@ -186,7 +211,7 @@ const transformExperimentOrCheckpointData = (
     checkpointTipId,
     experiment: transformExperimentData(
       id,
-      experimentFields,
+      experimentData,
       shortenForLabel(sha),
       sha,
       getDisplayNameOrParent(sha, branchSha, experimentsObject),
@@ -254,14 +279,8 @@ const collectFromBranchesObject = (
   for (const [sha, { baseline, ...experimentsObject }] of Object.entries(
     branchesObject
   )) {
-    const experimentFields = baseline.data
-    if (!experimentFields) {
-      continue
-    }
-
-    const name = experimentFields.name as string
-
-    const branch = transformExperimentData(name, experimentFields, name, sha)
+    const name = baseline.data?.name || sha
+    const branch = transformExperimentData(name, baseline, name, sha)
 
     if (branch) {
       collectFromExperimentsObject(acc, experimentsObject, sha, branch)
@@ -278,10 +297,11 @@ export const collectExperiments = (
   const { workspace, ...branchesObject } = data
   const workspaceId = 'workspace'
 
-  const workspaceFields = workspace.baseline.data
-  const workspaceBaseline = workspaceFields
-    ? transformExperimentData(workspaceId, workspaceFields, workspaceId)
-    : undefined
+  const workspaceBaseline = transformExperimentData(
+    workspaceId,
+    workspace.baseline,
+    workspaceId
+  )
 
   const acc = new ExperimentsAccumulator(workspaceBaseline)
 

extension/src/experiments/model/filterBy/decorationProvider.ts → extension/src/experiments/model/decorationProvider.ts

@@ -7,7 +7,7 @@ import {
   Uri,
   window
 } from 'vscode'
-import { Disposable } from '../../../class/dispose'
+import { Disposable } from '../../class/dispose'
 
 export const getDecoratableUri = (label: string): Uri =>
   Uri.from({ path: label, scheme: 'dvc.experiments' })
@@ -16,6 +16,10 @@ export class DecorationProvider
   extends Disposable
   implements FileDecorationProvider
 {
+  private static DecorationError: FileDecoration = {
+    color: new ThemeColor('errorForeground')
+  }
+
   private static DecorationFiltered: FileDecoration = {
     color: new ThemeColor('gitDecoration.ignoredResourceForeground'),
     tooltip: 'Filtered'
@@ -24,6 +28,7 @@ export class DecorationProvider
   public readonly onDidChangeFileDecorations: Event<Uri[]>
   private readonly decorationsChanged: EventEmitter<Uri[]>
 
+  private errors = new Set<string>()
   private filtered = new Set<string>()
 
   constructor(decorationsChanged?: EventEmitter<Uri[]>) {
@@ -38,19 +43,27 @@ export class DecorationProvider
   }
 
   public provideFileDecoration(uri: Uri): FileDecoration | undefined {
+    if (this.errors.has(uri.fsPath)) {
+      return DecorationProvider.DecorationError
+    }
     if (this.filtered.has(uri.fsPath)) {
       return DecorationProvider.DecorationFiltered
     }
   }
 
-  public setState(labels: string[], filtered: Set<string>) {
+  public setState(
+    labels: string[],
+    filtered: Set<string>,
+    errors: Set<string>
+  ) {
     const urisToUpdate: Uri[] = []
 
     for (const label of labels) {
       urisToUpdate.push(getDecoratableUri(label))
     }
 
     this.filtered = filtered
+    this.errors = errors
     this.decorationsChanged.fire(urisToUpdate)
   }
 }

extension/src/experiments/model/index.ts

@@ -310,6 +310,14 @@ export class ExperimentsModel extends ModelWithPersistence {
     ]
   }
 
+  public getErrors() {
+    return new Set(
+      this.getCombinedList()
+        .filter(({ error }) => error)
+        .map(({ label }) => label)
+    )
+  }
+
   public getExperimentsWithCheckpoints(): ExperimentWithCheckpoints[] {
     return this.getExperiments().map(experiment => {
       const checkpoints = this.checkpointsByTip