Use plot errors to display correct messages for missing plots (#3520)

extension/src/cli/dvc/contract.ts

@@ -100,7 +100,7 @@ export interface PlotsData {
   [path: string]: Plot[]
 }
 
-type PlotError = {
+export type PlotError = {
   name: string
   rev: string
   source?: string

extension/src/plots/errors/collect.test.ts

@@ -0,0 +1,137 @@
+import { join } from 'path'
+import { collectErrors, collectImageErrors } from './collect'
+import { EXPERIMENT_WORKSPACE_ID } from '../../cli/dvc/contract'
+
+describe('collectErrors', () => {
+  it('should remove errors that belong to revisions that have been fetched', () => {
+    const errors = collectErrors(
+      { data: {} },
+      [EXPERIMENT_WORKSPACE_ID],
+      [
+        {
+          msg: 'unexpected error',
+          name: 'fun::plot',
+          rev: EXPERIMENT_WORKSPACE_ID,
+          source: 'metrics.json',
+          type: 'unexpected'
+        }
+      ],
+      {}
+    )
+
+    expect(errors).toStrictEqual([])
+  })
+
+  it('should collect new errors', () => {
+    const newError = {
+      msg: 'Blue screen of death',
+      name: 'fun::plot',
+      rev: 'd7ad114',
+      source: 'metrics.json',
+      type: 'unexpected'
+    }
+
+    const errors = collectErrors(
+      {
+        data: {},
+        errors: [newError]
+      },
+      [EXPERIMENT_WORKSPACE_ID, 'd7ad114'],
+      [
+        {
+          msg: 'unexpected error',
+          name: 'fun::plot',
+          rev: EXPERIMENT_WORKSPACE_ID,
+          source: 'metrics.json',
+          type: 'unexpected'
+        }
+      ],
+      { d7ad114: 'main' }
+    )
+
+    expect(errors).toStrictEqual([{ ...newError, rev: 'main' }])
+  })
+})
+
+describe('collectImageErrors', () => {
+  it('should return undefined if there are no errors for the image', () => {
+    const error = collectImageErrors(
+      'misclassified.jpg',
+      EXPERIMENT_WORKSPACE_ID,
+      []
+    )
+    expect(error).toBeUndefined()
+  })
+
+  it('should collect a single error for an image', () => {
+    const path = join('training', 'plots', 'images', 'mispredicted.jpg')
+    const otherPath = 'other'
+
+    const errors = [
+      {
+        msg: `${otherPath} - file type error\nOnly JSON, YAML, CSV and TSV formats are supported.`,
+        name: otherPath,
+        rev: EXPERIMENT_WORKSPACE_ID,
+        source: otherPath,
+        type: 'PlotMetricTypeError'
+      },
+      {
+        msg: "Could not find provided field ('ste') in data fields ('step, test/acc').",
+        name: otherPath,
+        rev: EXPERIMENT_WORKSPACE_ID,
+        source: otherPath,
+        type: 'FieldNotFoundError'
+      },
+      {
+        msg: '',
+        name: path,
+        rev: EXPERIMENT_WORKSPACE_ID,
+        source: path,
+        type: 'FileNotFoundError'
+      },
+      {
+        msg: '',
+        name: path,
+        rev: 'main',
+        source: path,
+        type: 'FileNotFoundError'
+      }
+    ]
+
+    const error = collectImageErrors(path, EXPERIMENT_WORKSPACE_ID, errors)
+    expect(error).toStrictEqual(`FileNotFoundError: ${path} not found.`)
+  })
+
+  it('should concatenate errors together to give a single string', () => {
+    const path = join('training', 'plots', 'images', 'mispredicted.jpg')
+
+    const errors = [
+      {
+        msg: '',
+        name: path,
+        rev: EXPERIMENT_WORKSPACE_ID,
+        source: path,
+        type: 'FileNotFoundError'
+      },
+      {
+        msg: 'catastrophic error',
+        name: path,
+        rev: EXPERIMENT_WORKSPACE_ID,
+        source: path,
+        type: 'SomeError'
+      },
+      {
+        msg: '',
+        name: path,
+        rev: EXPERIMENT_WORKSPACE_ID,
+        source: path,
+        type: 'UNEXPECTEDERRRRROR'
+      }
+    ]
+
+    const error = collectImageErrors(path, EXPERIMENT_WORKSPACE_ID, errors)
+    expect(error).toStrictEqual(
+      `FileNotFoundError: ${path} not found.\nSomeError: catastrophic error\nUNEXPECTEDERRRRROR`
+    )
+  })
+})

extension/src/plots/errors/collect.ts

@@ -0,0 +1,53 @@
+import { PlotError, PlotsOutput } from '../../cli/dvc/contract'
+
+export const collectErrors = (
+  data: PlotsOutput,
+  revs: string[],
+  errors: PlotError[],
+  cliIdToLabel: { [id: string]: string }
+) => {
+  const existingErrors = errors.filter(
+    ({ rev }) => !revs.includes(cliIdToLabel[rev] || rev)
+  )
+  const newErrors = data?.errors || []
+
+  return [
+    ...existingErrors,
+    ...newErrors.map(error => {
+      const { rev } = error
+      return {
+        ...error,
+        rev: cliIdToLabel[rev] || rev
+      }
+    })
+  ]
+}
+
+const getMessage = (error: PlotError): string => {
+  const { msg, type, source } = error
+
+  if (type === 'FileNotFoundError' && source && !msg) {
+    return `${type}: ${source} not found.`
+  }
+  return [type, msg].filter(Boolean).join(': ')
+}
+
+export const collectImageErrors = (
+  path: string,
+  revision: string,
+  errors: PlotError[]
+): string | undefined => {
+  const msgs: string[] = []
+  for (const error of errors) {
+    if (error.rev === revision && error.name === path) {
+      const msg = getMessage(error)
+      msgs.push(msg)
+    }
+  }
+
+  if (msgs.length === 0) {
+    return undefined
+  }
+
+  return msgs.join('\n')
+}

extension/src/plots/errors/model.ts

@@ -0,0 +1,31 @@
+import { collectErrors, collectImageErrors } from './collect'
+import { Disposable } from '../../class/dispose'
+import { PlotError, PlotsOutputOrError } from '../../cli/dvc/contract'
+import { isDvcError } from '../../cli/dvc/reader'
+
+export class ErrorsModel extends Disposable {
+  private readonly dvcRoot: string
+
+  private errors: PlotError[] = []
+
+  constructor(dvcRoot: string) {
+    super()
+    this.dvcRoot = dvcRoot
+  }
+
+  public transformAndSet(
+    data: PlotsOutputOrError,
+    revs: string[],
+    cliIdToLabel: { [id: string]: string }
+  ) {
+    if (isDvcError(data)) {
+      return
+    }
+
+    this.errors = collectErrors(data, revs, this.errors, cliIdToLabel)
+  }
+
+  public getImageErrors(path: string, revision: string) {
+    return collectImageErrors(path, revision, this.errors)
+  }
+}

extension/src/plots/index.ts

@@ -3,6 +3,7 @@ import { Event, EventEmitter, Memento } from 'vscode'
 import { PlotsData as TPlotsData } from './webview/contract'
 import { WebviewMessages } from './webview/messages'
 import { PlotsData } from './data'
+import { ErrorsModel } from './errors/model'
 import { PlotsModel } from './model'
 import { collectEncodingElements, collectScale } from './paths/collect'
 import { PathsModel } from './paths/model'
@@ -31,6 +32,8 @@ export class Plots extends BaseRepository<TPlotsData> {
   private readonly paths: PathsModel
   private readonly data: PlotsData
 
+  private readonly errors: ErrorsModel
+
   private webviewMessages: WebviewMessages
 
   constructor(
@@ -43,8 +46,10 @@ export class Plots extends BaseRepository<TPlotsData> {
   ) {
     super(dvcRoot, webviewIcon)
 
+    this.errors = this.dispose.track(new ErrorsModel(this.dvcRoot))
+
     this.plots = this.dispose.track(
-      new PlotsModel(this.dvcRoot, experiments, workspaceState)
+      new PlotsModel(this.dvcRoot, experiments, this.errors, workspaceState)
     )
     this.paths = this.dispose.track(
       new PathsModel(this.dvcRoot, workspaceState)
@@ -213,9 +218,11 @@ export class Plots extends BaseRepository<TPlotsData> {
   private onDidUpdateData() {
     this.dispose.track(
       this.data.onDidUpdate(async ({ data, revs }) => {
+        const cliIdToLabel = this.plots.getCLIIdToLabel()
         await Promise.all([
           this.plots.transformAndSetPlots(data, revs),
-          this.paths.transformAndSet(data, revs, this.plots.getCLIIdToLabel())
+          this.paths.transformAndSet(data, revs, cliIdToLabel),
+          this.errors.transformAndSet(data, revs, cliIdToLabel)
         ])
         this.notifyChanged()
       })

extension/src/plots/model/index.test.ts

@@ -11,6 +11,7 @@ import { Experiments } from '../../experiments'
 import { PersistenceKey } from '../../persistence/constants'
 import { EXPERIMENT_WORKSPACE_ID } from '../../cli/dvc/contract'
 import { customPlotsOrderFixture } from '../../test/fixtures/expShow/base/customPlots'
+import { ErrorsModel } from '../errors/model'
 
 const mockedRevisions = [
   { displayColor: 'white', label: EXPERIMENT_WORKSPACE_ID },
@@ -41,6 +42,7 @@ describe('plotsModel', () => {
         getSelectedRevisions: mockedGetSelectedRevisions,
         isReady: () => Promise.resolve(undefined)
       } as unknown as Experiments,
+      { getImageErrors: () => undefined } as unknown as ErrorsModel,
       memento
     )
     jest.clearAllMocks()
@@ -64,6 +66,7 @@ describe('plotsModel', () => {
         getSelectedRevisions: mockedGetSelectedRevisions,
         isReady: () => Promise.resolve(undefined)
       } as unknown as Experiments,
+      { getImageErrors: () => undefined } as unknown as ErrorsModel,
       memento
     )
     expect(model.getCustomPlotsOrder()).toStrictEqual([

extension/src/plots/model/index.ts

@@ -52,11 +52,13 @@ import {
   MultiSourceVariations
 } from '../multiSource/collect'
 import { isDvcError } from '../../cli/dvc/reader'
+import { ErrorsModel } from '../errors/model'
 
 export type CustomCheckpointPlots = { [metric: string]: CheckpointPlot }
 
 export class PlotsModel extends ModelWithPersistence {
   private readonly experiments: Experiments
+  private readonly errors: ErrorsModel
 
   private nbItemsPerRowOrWidth: Record<PlotsSection, number>
   private height: Record<PlotsSection, PlotHeight>
@@ -77,10 +79,12 @@ export class PlotsModel extends ModelWithPersistence {
   constructor(
     dvcRoot: string,
     experiments: Experiments,
+    errors: ErrorsModel,
     workspaceState: Memento
   ) {
     super(dvcRoot, workspaceState)
     this.experiments = experiments
+    this.errors = errors
 
     this.nbItemsPerRowOrWidth = this.revive(
       PersistenceKey.PLOT_NB_ITEMS_PER_ROW_OR_WIDTH,
@@ -494,7 +498,9 @@ export class PlotsModel extends ModelWithPersistence {
 
     for (const revision of selectedRevisions) {
       const image = this.comparisonData?.[revision]?.[path]
+      const error = this.errors.getImageErrors(path, revision)
       pathRevisions.revisions[revision] = {
+        error,
         revision,
         url: image?.url
       }