diff --git a/extension/src/cli/dvc/constants.ts b/extension/src/cli/dvc/constants.ts index 8fd953e900..29f554eac3 100644 --- a/extension/src/cli/dvc/constants.ts +++ b/extension/src/cli/dvc/constants.ts @@ -28,9 +28,11 @@ export enum SubCommand { } export enum Flag { + ALL_COMMITS = '-A', FORCE = '-f', GRANULAR = '--granular', JSON = '--json', + NUM_COMMIT = '-n', OUTPUT_PATH = '-o', SUBDIRECTORY = '--subdir', SET_PARAM = '-S', diff --git a/extension/src/cli/dvc/reader.test.ts b/extension/src/cli/dvc/reader.test.ts index 255d79edd1..2bd84fc65e 100644 --- a/extension/src/cli/dvc/reader.test.ts +++ b/extension/src/cli/dvc/reader.test.ts @@ -72,7 +72,7 @@ describe('CliReader', () => { const cliOutput = await dvcReader.expShow(cwd) expect(cliOutput).toStrictEqual(expShowFixture) expect(mockedCreateProcess).toHaveBeenCalledWith({ - args: ['exp', 'show', JSON_FLAG], + args: ['exp', 'show', '-n', '3', JSON_FLAG], cwd, env: mockedEnv, executable: 'dvc' diff --git a/extension/src/cli/dvc/reader.ts b/extension/src/cli/dvc/reader.ts index 85af238081..9acc8c50cd 100644 --- a/extension/src/cli/dvc/reader.ts +++ b/extension/src/cli/dvc/reader.ts @@ -92,6 +92,7 @@ export interface PlotsOutput { } export const TEMP_PLOTS_DIR = join('.dvc', 'tmp', 'plots') +export const NUM_OF_COMMITS_TO_SHOW = '3' export const isDvcError = < T extends ExperimentsOutput | DataStatusOutput | PlotsOutput @@ -135,6 +136,8 @@ export class DvcReader extends DvcCli { Command.EXPERIMENT, SubCommand.SHOW, ...flags, + Flag.NUM_COMMIT, + NUM_OF_COMMITS_TO_SHOW, Flag.JSON ) if (isDvcError(output) || isEmpty(output)) { diff --git a/extension/src/experiments/index.ts b/extension/src/experiments/index.ts index 8f3a945c6a..0e14f2c1ca 100644 --- a/extension/src/experiments/index.ts +++ b/extension/src/experiments/index.ts @@ -363,6 +363,10 @@ export class Experiments extends BaseRepository { return this.experiments.getCheckpointsWithType(id) } + public getBranchExperiments(branch: Experiment) { + return this.experiments.getExperimentsByBranchForTree(branch) + } + public sendInitialWebviewData() { return this.webviewMessages.sendWebviewMessage() } diff --git a/extension/src/experiments/model/collect.ts b/extension/src/experiments/model/collect.ts index 537dd952f3..71188d8315 100644 --- a/extension/src/experiments/model/collect.ts +++ b/extension/src/experiments/model/collect.ts @@ -282,8 +282,14 @@ const collectFromBranchesObject = ( for (const [sha, { baseline, ...experimentsObject }] of Object.entries( branchesObject )) { - const name = baseline.data?.name || sha - const branch = transformExperimentData(name, baseline, name, sha) + let name = baseline.data?.name + let label = name + + if (!name) { + name = sha + label = shortenForLabel(name) + } + const branch = transformExperimentData(name, baseline, label, sha) if (branch) { collectFromExperimentsObject(acc, experimentsObject, sha, branch) diff --git a/extension/src/experiments/model/filterBy/collect.ts b/extension/src/experiments/model/filterBy/collect.ts index b1d2db811a..f71d6ade05 100644 --- a/extension/src/experiments/model/filterBy/collect.ts +++ b/extension/src/experiments/model/filterBy/collect.ts @@ -8,6 +8,11 @@ import { definedAndNonEmpty } from '../../../util/array' import { Experiment } from '../../webview/contract' export type ExperimentWithType = Experiment & { type: ExperimentType } +export type ExperimentAugmented = ExperimentWithType & { + hasChildren: boolean + selected?: boolean + starred: boolean +} export type FilteredCounts = { checkpoints?: number experiments: number diff --git a/extension/src/experiments/model/index.test.ts b/extension/src/experiments/model/index.test.ts index 3ed5f87580..fa98aaaf0d 100644 --- a/extension/src/experiments/model/index.test.ts +++ b/extension/src/experiments/model/index.test.ts @@ -145,7 +145,7 @@ describe('ExperimentsModel', () => { } }) - const experiments = model.getExperiments() + const experiments = model.getAllExperiments() const changed: string[] = [] for (const { deps, sha } of experiments) { diff --git a/extension/src/experiments/model/index.ts b/extension/src/experiments/model/index.ts index bef95bedcd..ba29ba6e27 100644 --- a/extension/src/experiments/model/index.ts +++ b/extension/src/experiments/model/index.ts @@ -10,6 +10,7 @@ import { collectExperiments, collectMutableRevisions } from './collect' import { collectFiltered, collectFilteredCounts, + ExperimentAugmented, ExperimentWithType } from './filterBy/collect' import { collectColoredStatus, collectSelected } from './status/collect' @@ -228,7 +229,7 @@ export class ExperimentsModel extends ModelWithPersistence { } public getMutableRevisions(hasCheckpoints: boolean) { - return collectMutableRevisions(this.getExperiments(), hasCheckpoints) + return collectMutableRevisions(this.getAllExperiments(), hasCheckpoints) } public getSelectedRevisions() { @@ -264,7 +265,7 @@ export class ExperimentsModel extends ModelWithPersistence { public getUnfilteredExperiments(filters = this.getFilters()) { const unfilteredExperiments = this.getSubRows( - this.getExperiments(), + this.getAllExperiments(), filters ) @@ -285,11 +286,7 @@ export class ExperimentsModel extends ModelWithPersistence { ) } - public getExperiments(): (ExperimentWithType & { - hasChildren: boolean - selected?: boolean - starred: boolean - })[] { + public getExperiments(): ExperimentAugmented[] { return [ { ...this.addDetails(this.workspace), @@ -299,22 +296,17 @@ export class ExperimentsModel extends ModelWithPersistence { ...this.branches.map(branch => { return { ...this.addDetails(branch), - hasChildren: false, + hasChildren: !!this.experimentsByBranch.get(branch.label), type: ExperimentType.BRANCH } - }), - ...this.getFlattenedExperiments().map(experiment => ({ - ...experiment, - hasChildren: definedAndNonEmpty( - this.checkpointsByTip.get(experiment.id) - ), - type: experiment.queued - ? ExperimentType.QUEUED - : ExperimentType.EXPERIMENT - })) + }) ] } + public getAllExperiments() { + return [...this.getExperiments(), ...this.getFlattenedExperiments()] + } + public getErrors() { return new Set( this.getCombinedList() @@ -325,7 +317,7 @@ export class ExperimentsModel extends ModelWithPersistence { public getExperimentsWithCheckpoints(): ExperimentWithCheckpoints[] { const experimentsWithCheckpoints: ExperimentWithCheckpoints[] = [] - for (const experiment of this.getExperiments()) { + for (const experiment of this.getAllExperiments()) { const { id, queued } = experiment if (queued) { continue @@ -342,7 +334,7 @@ export class ExperimentsModel extends ModelWithPersistence { } public getExperimentParams(id: string) { - const params = this.getExperiments().find( + const params = this.getAllExperiments().find( experiment => experiment.id === id )?.params @@ -416,6 +408,16 @@ export class ExperimentsModel extends ModelWithPersistence { ] } + public getExperimentsByBranchForTree(branch: Experiment) { + return this.getExperimentsByBranch(branch)?.map(experiment => ({ + ...experiment, + hasChildren: definedAndNonEmpty(this.checkpointsByTip.get(experiment.id)), + type: experiment.queued + ? ExperimentType.QUEUED + : ExperimentType.EXPERIMENT + })) + } + private getSubRows(experiments: Experiment[], filters = this.getFilters()) { return experiments .map(experiment => { @@ -514,6 +516,7 @@ export class ExperimentsModel extends ModelWithPersistence { const { coloredStatus, availableColors } = collectColoredStatus( this.getExperiments(), this.checkpointsByTip, + this.experimentsByBranch, this.coloredStatus, this.availableColors ) @@ -571,6 +574,7 @@ export class ExperimentsModel extends ModelWithPersistence { if (!hasKey(this.coloredStatus, id)) { return { ...experiment, + selected: false, starred } } diff --git a/extension/src/experiments/model/status/collect.test.ts b/extension/src/experiments/model/status/collect.test.ts index 1f57839546..48154385e5 100644 --- a/extension/src/experiments/model/status/collect.test.ts +++ b/extension/src/experiments/model/status/collect.test.ts @@ -13,17 +13,22 @@ describe('collectColoredStatus', () => { return mockExperiments } - it('should set new experiments to selected if there are less than 7', () => { - const experiments = buildMockExperiments(4) - const colors = copyOriginalColors() - - const { availableColors, coloredStatus } = collectColoredStatus( + const collectedColoredStatus = (experiments: Experiment[]) => + collectColoredStatus( experiments, new Map(), + new Map(), {}, copyOriginalColors() ) + it('should set new experiments to selected if there are less than 7', () => { + const experiments = buildMockExperiments(4) + const colors = copyOriginalColors() + + const { availableColors, coloredStatus } = + collectedColoredStatus(experiments) + expect(availableColors).toStrictEqual(colors.slice(4)) expect(coloredStatus).toStrictEqual({ exp1: colors[0], @@ -40,12 +45,8 @@ describe('collectColoredStatus', () => { ] as Experiment[] const colors = copyOriginalColors() - const { availableColors, coloredStatus } = collectColoredStatus( - experiments, - new Map(), - {}, - copyOriginalColors() - ) + const { availableColors, coloredStatus } = + collectedColoredStatus(experiments) expect(availableColors).toStrictEqual(colors.slice(1)) expect(coloredStatus).toStrictEqual({ @@ -57,12 +58,8 @@ describe('collectColoredStatus', () => { const experiments = buildMockExperiments(8) const colors = copyOriginalColors() - const { availableColors, coloredStatus } = collectColoredStatus( - experiments, - new Map(), - {}, - copyOriginalColors() - ) + const { availableColors, coloredStatus } = + collectedColoredStatus(experiments) expect(availableColors).toStrictEqual([]) expect(coloredStatus).toStrictEqual({ @@ -84,6 +81,7 @@ describe('collectColoredStatus', () => { const { availableColors, coloredStatus } = collectColoredStatus( experiments, new Map(), + new Map(), { exp2: 0, exp3: colors[2], @@ -107,6 +105,7 @@ describe('collectColoredStatus', () => { const { availableColors, coloredStatus } = collectColoredStatus( experiments, new Map(), + new Map(), { exp1: 0, exp10: colors[0], @@ -138,6 +137,7 @@ describe('collectColoredStatus', () => { const { availableColors, coloredStatus } = collectColoredStatus( experiments, new Map(), + new Map(), { exp9: colors[0] }, copyOriginalColors().slice(1) ) @@ -163,6 +163,7 @@ describe('collectColoredStatus', () => { const { availableColors, coloredStatus } = collectColoredStatus( experiments, new Map(), + new Map(), { exp4: colors[0], exp5: colors[1], @@ -198,6 +199,7 @@ describe('collectColoredStatus', () => { const { availableColors, coloredStatus } = collectColoredStatus( experiments, checkpointsByTip, + new Map(), {}, copyOriginalColors() ) @@ -231,6 +233,7 @@ describe('collectColoredStatus', () => { const { availableColors, coloredStatus } = collectColoredStatus( experiments, checkpointsByTip, + new Map(), { checkC1: colors[1], checkD2: colors[2], diff --git a/extension/src/experiments/model/status/collect.ts b/extension/src/experiments/model/status/collect.ts index 77d2da73fe..96b1b04b00 100644 --- a/extension/src/experiments/model/status/collect.ts +++ b/extension/src/experiments/model/status/collect.ts @@ -25,20 +25,24 @@ const collectStatus = ( if (!id || queued || hasKey(acc, id)) { return } + acc[id] = getStatus(acc, unassignColors) } const collectExistingStatuses = ( experiments: Experiment[], checkpointsByTip: Map, + experimentsByBranch: Map, previousStatus: ColoredStatus ) => { const existingStatuses: ColoredStatus = {} for (const experiment of [ ...experiments, + ...flattenMapValues(experimentsByBranch), ...flattenMapValues(checkpointsByTip) ]) { const { id } = experiment + if (!hasKey(previousStatus, id)) { continue } @@ -70,11 +74,17 @@ export const unassignColors = ( export const collectColoredStatus = ( experiments: Experiment[], checkpointsByTip: Map, + experimentsByBranch: Map, previousStatus: ColoredStatus, unassignedColors: Color[] ): { coloredStatus: ColoredStatus; availableColors: Color[] } => { + const flattenExperimentsByBranch = flattenMapValues(experimentsByBranch) const availableColors = unassignColors( - [...experiments, ...flattenMapValues(checkpointsByTip)], + [ + ...experiments, + ...flattenExperimentsByBranch, + ...flattenMapValues(checkpointsByTip) + ], previousStatus, unassignedColors ) @@ -82,10 +92,11 @@ export const collectColoredStatus = ( const coloredStatus = collectExistingStatuses( experiments, checkpointsByTip, + experimentsByBranch, previousStatus ) - for (const experiment of experiments) { + for (const experiment of [...experiments, ...flattenExperimentsByBranch]) { collectStatus(coloredStatus, experiment, availableColors) for (const checkpoint of checkpointsByTip.get(experiment.id) || []) { diff --git a/extension/src/experiments/model/tree.test.ts b/extension/src/experiments/model/tree.test.ts index 2e8e486f87..e8c114ba20 100644 --- a/extension/src/experiments/model/tree.test.ts +++ b/extension/src/experiments/model/tree.test.ts @@ -33,6 +33,7 @@ const { mockedExperiments, mockedGetDvcRoots, mockedGetExperiments, + mockedGetBranchExperiments, mockedGetCheckpoints } = buildMockedExperiments() @@ -334,6 +335,61 @@ describe('ExperimentsTree', () => { } ]) }) + + it('should return the branch experiments when the element is a branch', async () => { + const experimentsTree = new ExperimentsTree( + mockedExperiments, + mockedResourceLocator + ) + const getMockedUri = (name: string, color: string) => + Uri.file(join('path', 'to', 'resources', `${name}-${color}.svg`)) + const dvcRoot = '/dvc-root' + const branch = { + collapsibleState: 0, + description: 'f81f1b5', + dvcRoot, + hasChildren: true, + iconPath: getMockedUri('circle-filled', '#b180d7'), + id: 'f81f1b5', + label: 'f81f1b5', + queued: false, + tooltip: undefined, + type: ExperimentType.BRANCH + } + + const experimentsByBranch = [ + { + displayColor: undefined, + hasChildren: false, + id: 'exp-abcdef', + label: 'e350702', + running: false, + selected: false, + type: ExperimentType.EXPERIMENT + } + ] + mockedGetBranchExperiments.mockReturnValueOnce(experimentsByBranch) + + const children = await experimentsTree.getChildren(branch) + + expect(children).toStrictEqual([ + { + collapsibleState: 0, + command: { + arguments: [{ dvcRoot, id: 'exp-abcdef' }], + command: RegisteredCommands.EXPERIMENT_TOGGLE, + title: 'toggle' + }, + description: undefined, + dvcRoot, + iconPath: expect.anything(), + id: 'exp-abcdef', + label: 'e350702', + tooltip: undefined, + type: ExperimentType.EXPERIMENT + } + ]) + }) }) describe('getTreeItem', () => { diff --git a/extension/src/experiments/model/tree.ts b/extension/src/experiments/model/tree.ts index 6f4294f784..10e9af4369 100644 --- a/extension/src/experiments/model/tree.ts +++ b/extension/src/experiments/model/tree.ts @@ -9,6 +9,7 @@ import { Uri } from 'vscode' import { ExperimentType } from '.' +import { ExperimentAugmented } from './filterBy/collect' import { collectDeletable, ExperimentItem } from './collect' import { MAX_SELECTED_EXPERIMENTS } from './status' import { WorkspaceExperiments } from '../workspace' @@ -29,6 +30,7 @@ import { } from '../../commands/external' import { sum } from '../../util/math' import { Disposable } from '../../class/dispose' +import { Experiment } from '../webview/contract' export class ExperimentsTree extends Disposable @@ -120,6 +122,12 @@ export class ExperimentsTree return Promise.resolve(this.getExperiments(element)) } + if (element.type === ExperimentType.BRANCH) { + return Promise.resolve( + this.getExperimentsByBranch(element.dvcRoot, element) + ) + } + const { dvcRoot, id } = element return Promise.resolve(this.getCheckpoints(dvcRoot, id)) } @@ -168,27 +176,45 @@ export class ExperimentsTree return [] } + private formatExperiment(experiment: ExperimentAugmented, dvcRoot: string) { + return { + collapsibleState: experiment.hasChildren + ? this.getCollapsibleState(experiment.displayNameOrParent) + : TreeItemCollapsibleState.None, + command: { + arguments: [{ dvcRoot, id: experiment.id }], + command: RegisteredCommands.EXPERIMENT_TOGGLE, + title: 'toggle' + }, + description: experiment.displayNameOrParent, + dvcRoot, + iconPath: this.getExperimentIcon(experiment), + id: experiment.id, + label: experiment.label, + tooltip: this.getTooltip(experiment.error), + type: experiment.type + } + } + private getExperiments(dvcRoot: string): ExperimentItem[] { return this.experiments .getRepository(dvcRoot) .getExperiments() - .map(experiment => ({ - collapsibleState: experiment.hasChildren - ? this.getCollapsibleState(experiment.displayNameOrParent) - : TreeItemCollapsibleState.None, - command: { - arguments: [{ dvcRoot, id: experiment.id }], - command: RegisteredCommands.EXPERIMENT_TOGGLE, - title: 'toggle' - }, - description: experiment.displayNameOrParent, - dvcRoot, - iconPath: this.getExperimentIcon(experiment), - id: experiment.id, - label: experiment.label, - tooltip: this.getTooltip(experiment.error), - type: experiment.type - })) + .map(experiment => this.formatExperiment(experiment, dvcRoot)) + } + + private getExperimentsByBranch( + dvcRoot: string, + branch: Experiment + ): ExperimentItem[] { + return ( + this.experiments + .getRepository(dvcRoot) + .getBranchExperiments(branch) + ?.map(experiment => + this.formatExperiment(experiment as ExperimentAugmented, dvcRoot) + ) || [] + ) } private setExpanded(element: string | ExperimentItem, expanded: boolean) { diff --git a/extension/src/test/fixtures/expShow/rows.ts b/extension/src/test/fixtures/expShow/rows.ts index 5c5900d5c8..48107b9537 100644 --- a/extension/src/test/fixtures/expShow/rows.ts +++ b/extension/src/test/fixtures/expShow/rows.ts @@ -1474,6 +1474,7 @@ const data: Row[] = [ } }, queued: true, + selected: false, sha: '90aea7f2482117a55dfcadcdb901aaa6610fbbc9', starred: false, Created: '2020-12-29T15:25:27' diff --git a/extension/src/test/suite/experiments/index.test.ts b/extension/src/test/suite/experiments/index.test.ts index 2c88327cd5..5ab8e5b88e 100644 --- a/extension/src/test/suite/experiments/index.test.ts +++ b/extension/src/test/suite/experiments/index.test.ts @@ -79,7 +79,7 @@ suite('Experiments Test Suite', () => { }) describe('getExperiments', () => { - it('should return all existing experiments, the workspace and branch (HEAD revision)', async () => { + it('should return the workspace and branch (HEAD revision)', async () => { const { experiments } = buildExperiments(disposable) await experiments.isReady() @@ -88,13 +88,7 @@ suite('Experiments Test Suite', () => { expect(runs.map(experiment => experiment.label)).to.deep.equal([ 'workspace', - 'main', - '4fb124a', - '42b8736', - '1ba7bcd', - '489fd8b', - 'f0f9186', - '90aea7f' + 'main' ]) }) }) @@ -729,7 +723,7 @@ suite('Experiments Test Suite', () => { const queuedExperiment = '90aea7f2482117a55dfcadcdb901aaa6610fbbc9' const isExperimentSelected = (expId: string): boolean => - !!experimentsModel.getExperiments().find(({ id }) => id === expId) + !!experimentsModel.getAllExperiments().find(({ id }) => id === expId) ?.selected expect(isExperimentSelected(experimentToToggle), 'experiment is selected') @@ -893,7 +887,7 @@ suite('Experiments Test Suite', () => { const areExperimentsStarred = (expIds: string[]): boolean => expIds .map(expId => - experimentsModel.getExperiments().find(({ id }) => id === expId) + experimentsModel.getAllExperiments().find(({ id }) => id === expId) ) .every(exp => exp?.starred) diff --git a/extension/src/test/util/jest/index.ts b/extension/src/test/util/jest/index.ts index 0fca4bb223..eb33f97b2e 100644 --- a/extension/src/test/util/jest/index.ts +++ b/extension/src/test/util/jest/index.ts @@ -36,12 +36,14 @@ export const buildMockedExperiments = () => { const mockedGetFilter = jest.fn() const mockedGetSorts = jest.fn() const mockedGetSelectedRevisions = jest.fn() + const mockedGetBranchExperiments = jest.fn() const mockedExperiments = { columnsChanged: mockedColumnsChanged, experimentsChanged: mockedExperimentsChanged, getDvcRoots: mockedGetDvcRoots, getRepository: () => ({ + getBranchExperiments: mockedGetBranchExperiments, getCheckpoints: mockedGetCheckpoints, getChildColumns: mockedGetChildColumns, getExperiments: mockedGetExperiments, @@ -57,6 +59,7 @@ export const buildMockedExperiments = () => { mockedColumnsChanged, mockedExperiments, mockedExperimentsChanged, + mockedGetBranchExperiments, mockedGetCheckpoints, mockedGetChildColumns, mockedGetDvcRoots, diff --git a/webview/src/experiments/components/table/styles.module.scss b/webview/src/experiments/components/table/styles.module.scss index 62adbf8aa2..14a3234ada 100644 --- a/webview/src/experiments/components/table/styles.module.scss +++ b/webview/src/experiments/components/table/styles.module.scss @@ -221,19 +221,14 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding; bottom: 0; } -.rowGroup:last-child > .tr:last-child { - border-color: $row-bg-color; +.rowGroup:last-child { + & > .tr:last-child { + border-color: $row-bg-color; - &.rowSelected { - border-color: $row-bg-selected-color; + &.rowSelected { + border-color: $row-bg-selected-color; + } } -} - -.experiments { - box-sizing: border-box; - height: 100%; - display: flex; - flex-flow: column nowrap; .experimentGroup { &:last-child .tr:last-child { @@ -243,7 +238,16 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding; border-color: $row-bg-selected-color; } } + } +} +.experiments { + box-sizing: border-box; + height: 100%; + display: flex; + flex-flow: column nowrap; + + .experimentGroup { .nestedRow { .experimentCell { padding-left: $nested-row-padding;