Trigger plot updates whenever commit data changes

extension/src/plots/index.ts

@@ -225,6 +225,8 @@ export class Plots extends BaseRepository<TPlotsData> {
         this.dispose.untrack(waitForInitialExpData)
         waitForInitialExpData.dispose()
         this.data.setMetricFiles(experiments.getRelativeMetricsFiles())
+        const collectInitialIdShas = () => this.plots.removeStaleData()
+        collectInitialIdShas()
         this.setupExperimentsListener(experiments)
         void this.initializeData()
       })

extension/src/plots/model/collect.ts

@@ -468,3 +468,13 @@ export const collectImageUrl = (
 
   return url
 }
+
+export const collectIdShas = (experiments: Experiment[]) => {
+  const idShas: Record<string, string> = {}
+  for (const { id, sha } of experiments) {
+    if (sha) {
+      idShas[id] = sha
+    }
+  }
+  return idShas
+}

extension/src/plots/model/index.ts

@@ -9,7 +9,8 @@ import {
   collectCustomPlots,
   getCustomPlotId,
   collectOrderedRevisions,
-  collectImageUrl
+  collectImageUrl,
+  collectIdShas
 } from './collect'
 import { getRevisionFirstThreeColumns } from './util'
 import {
@@ -43,7 +44,6 @@ import {
   reorderObjectList,
   sameContents
 } from '../../util/array'
-import { removeMissingKeysFromObject } from '../../util/object'
 import { TemplateOrder } from '../paths/collect'
 import { PersistenceKey } from '../../persistence/constants'
 import { ModelWithPersistence } from '../../persistence/model'
@@ -66,6 +66,7 @@ export class PlotsModel extends ModelWithPersistence {
   private sectionCollapsed: SectionCollapsed
 
   private fetchedRevs = new Set<string>()
+  private idShas: { [id: string]: string } = {}
 
   private comparisonData: ComparisonData = {}
   private comparisonOrder: string[]
@@ -118,17 +119,18 @@ export class PlotsModel extends ModelWithPersistence {
   }
 
   public removeStaleData() {
-    const revisions = this.experiments.getRevisionIds()
-
-    this.comparisonData = removeMissingKeysFromObject(
-      revisions,
-      this.comparisonData
+    const idShas = collectIdShas(
+      this.experiments.getWorkspaceCommitsAndExperiments()
     )
 
-    this.revisionData = removeMissingKeysFromObject(
-      revisions,
-      this.revisionData
-    )
+    for (const id of Object.keys(this.idShas)) {
+      if (this.idShas[id] !== idShas[id]) {
+        this.deleteRevisionData(id)
+        this.fetchedRevs.delete(id)
+      }
+    }
+
+    this.idShas = idShas
   }
 
   public getCustomPlots(): CustomPlotsData | undefined {

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

@@ -43,6 +43,7 @@ import { isErrorItem } from '../../../tree'
 import { RegisteredCommands } from '../../../commands/external'
 import { REVISIONS } from '../../fixtures/plotsDiff'
 import * as FileSystem from '../../../fileSystem'
+import { experimentHasError } from '../../../cli/dvc/contract'
 
 suite('Plots Test Suite', () => {
   const disposable = Disposable.fn()
@@ -94,17 +95,31 @@ suite('Plots Test Suite', () => {
 
     it('should call plots diff with the commit whenever the current commit changes', async () => {
       const mockNow = getMockNow()
+      const noExperimentFixture = expShowFixtureWithoutErrors.map(exp => ({
+        ...exp,
+        experiments: null
+      }))
+
       const { data, experiments, mockPlotsDiff } = await buildPlots(
         disposable,
-        plotsDiffFixture
+        plotsDiffFixture,
+        noExperimentFixture
       )
+
       mockPlotsDiff.resetHistory()
 
-      const updatedExpShowFixture = expShowFixtureWithoutErrors.map(exp => {
+      const updatedExpShowFixture = noExperimentFixture.map(exp => {
         if (exp.rev === '53c3851f46955fa3e2b8f6e1c52999acc8c9ea77') {
+          if (experimentHasError(exp)) {
+            throw new Error('Experiment should not have error')
+          }
+
           return {
             ...exp,
-            experiments: null,
+            data: {
+              ...exp.data,
+              rev: '9235a02880a0372545e5f7f4d79a5d9eee6331ac'
+            },
             rev: '9235a02880a0372545e5f7f4d79a5d9eee6331ac'
           }
         }
@@ -127,7 +142,7 @@ suite('Plots Test Suite', () => {
         REVISIONS[0],
         REVISIONS[1]
       )
-    })
+    }).timeout(WEBVIEW_TEST_TIMEOUT)
 
     it('should remove the temporary plots directory on dispose', async () => {
       const { mockRemoveDir, plots } = await buildPlots(

extension/src/util/object.test.ts

@@ -1,51 +1,4 @@
-import { createTypedAccumulator, removeMissingKeysFromObject } from './object'
-
-describe('removeMissingKeysFromObject', () => {
-  it('should remove any keys that exist in the object but not the provided array', () => {
-    const expectedKeys = ['A', 'B', 'C', 'D']
-    const extendedObject = {
-      A: 1,
-      B: 2,
-      C: 3,
-      D: 4,
-      E: 5,
-      F: 6,
-      G: 7
-    }
-
-    expect(
-      removeMissingKeysFromObject(expectedKeys, extendedObject)
-    ).toStrictEqual({
-      A: 1,
-      B: 2,
-      C: 3,
-      D: 4
-    })
-  })
-
-  it('should not mutate the original object', () => {
-    const expectedKeys: string[] = []
-    const extendedObject = {
-      A: 1,
-      B: 2,
-      C: 3,
-      D: 4,
-      E: 5,
-      F: 6,
-      G: 7
-    }
-    const copyExtendedObject = { ...extendedObject }
-
-    const emptyObject = removeMissingKeysFromObject(
-      expectedKeys,
-      extendedObject
-    )
-
-    expect(emptyObject).toStrictEqual({})
-    expect(extendedObject).not.toStrictEqual({})
-    expect(extendedObject).toStrictEqual(copyExtendedObject)
-  })
-})
+import { createTypedAccumulator } from './object'
 
 describe('createTypedAccumulator', () => {
   it('should create a typed accumulator from an enum like object', () => {

extension/src/util/object.ts

@@ -2,21 +2,6 @@ export const hasKey = (maybeObject: unknown, key: string): boolean =>
   typeof maybeObject === 'object' &&
   Object.prototype.hasOwnProperty.call(maybeObject, key)
 
-export const removeMissingKeysFromObject = <
-  T extends { [key: string]: unknown }
->(
-  retainKeys: string[],
-  items: T
-): T => {
-  const copy = { ...items }
-  for (const key of Object.keys(copy)) {
-    if (!retainKeys.includes(key)) {
-      delete copy[key]
-    }
-  }
-  return copy
-}
-
 export const createTypedAccumulator = <T extends string>(
   enumLike: Record<string, T>
 ) => {