Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions frontend/src/components/TwoLevelDropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export interface SelectedItem {
itemName: string;
subItemName: string;
subItemSecondaryName?: string;
runId?: string;
}

export interface DropdownSubItem {
Expand All @@ -91,6 +92,7 @@ export interface DropdownSubItem {
export interface DropdownItem {
name: string;
subItems: DropdownSubItem[];
id?: string;
}

interface DropdownButtonProps {
Expand Down Expand Up @@ -163,6 +165,7 @@ function DropdownSubMenu(props: DropdownSubMenuProps) {
itemName: item.name,
subItemName: subItem.name,
subItemSecondaryName: subItem.secondaryName,
runId: item.id,
});
setSubDropdownActive(-1);
}}
Expand Down
63 changes: 62 additions & 1 deletion frontend/src/components/viewers/MetricsDropdown.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ describe('MetricsDropdown', () => {
itemName: 'run1',
subItemName: 'execution1',
subItemSecondaryName: 'artifact1',
runId: '1',
},
},
{
Expand Down Expand Up @@ -403,7 +404,7 @@ describe('MetricsDropdown', () => {
});
});

it('Dropdown initially loaded with selected artifact', async () => {
it('Dropdown initially loaded with selected artifact (display_name fallback, no runId)', async () => {
const newSelectedArtifacts: SelectedArtifact[] = [
{
selectedItem: {
Expand Down Expand Up @@ -550,4 +551,64 @@ describe('MetricsDropdown', () => {
expect(getHtmlViewerConfigSpy).toHaveBeenLastCalledWith([freshLinkedArtifact], undefined);
});
});

it('Dropdown initially loaded with selected artifact resolved via runId', async () => {
const getHtmlViewerConfigSpy = vi.spyOn(metricsVisualizations, 'getHtmlViewerConfig');
getHtmlViewerConfigSpy.mockResolvedValue([]);

const artifactFromRunA = newMockLinkedArtifact(10, 'shared-artifact');
const artifactFromRunB = newMockLinkedArtifact(20, 'shared-artifact');

const duplicateNameRunArtifacts: RunArtifact[] = [
{
run: { run_id: 'run-id-A', display_name: 'duplicate-run' },
executionArtifacts: [
{
execution: newMockExecution(10, 'execution1'),
linkedArtifacts: [artifactFromRunA],
},
],
},
{
run: { run_id: 'run-id-B', display_name: 'duplicate-run' },
executionArtifacts: [
{
execution: newMockExecution(20, 'execution1'),
linkedArtifacts: [artifactFromRunB],
},
],
},
];

const selectedArtifactsWithRunId: SelectedArtifact[] = [
{ selectedItem: { itemName: '', subItemName: '' } },
{
linkedArtifact: artifactFromRunB,
selectedItem: {
itemName: 'duplicate-run', // matches both runs' display_name
subItemName: 'execution1',
subItemSecondaryName: 'shared-artifact',
runId: 'run-id-B', // disambiguates to run B
},
},
];

render(
<CommonTestWrapper>
<MetricsDropdown
filteredRunArtifacts={duplicateNameRunArtifacts}
metricsTab={MetricsType.HTML}
selectedArtifacts={selectedArtifactsWithRunId}
updateSelectedArtifacts={updateSelectedArtifactsSpy}
/>
</CommonTestWrapper>,
);
await TestUtils.flushPromises();

// Must resolve to run B's artifact (ID 20), not run A's (ID 10).
await waitFor(() => {
expect(getHtmlViewerConfigSpy).toHaveBeenLastCalledWith([artifactFromRunB], undefined);
});
expect(getHtmlViewerConfigSpy).not.toHaveBeenCalledWith([artifactFromRunA], undefined);
});
Comment thread
pnaik1 marked this conversation as resolved.
});
94 changes: 46 additions & 48 deletions frontend/src/components/viewers/MetricsDropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import React, { useMemo } from 'react';
import React from 'react';
import { color, commonCss, fontsize, zIndex } from 'src/Css';
import { queryKeys } from 'src/hooks/queryKeys';
import { classes, stylesheet } from 'typestyle';
Expand All @@ -37,6 +37,7 @@ import { useQuery } from '@tanstack/react-query';
import { errorToMessage, logger } from 'src/lib/Utils';
import {
metricsTypeToString,
COMPARE_PANEL_COUNT,
ExecutionArtifact,
MetricsType,
RunArtifact,
Expand All @@ -51,6 +52,10 @@ const css = stylesheet({
rightCell: {
borderLeft: `3px solid ${color.divider}`,
},
middleCell: {
borderLeft: `3px solid ${color.divider}`,
borderRight: `3px solid ${color.divider}`,
},
Comment thread
pnaik1 marked this conversation as resolved.
cell: {
borderCollapse: 'collapse',
padding: '1rem',
Expand Down Expand Up @@ -93,28 +98,22 @@ export default function MetricsDropdown(props: MetricsDropdownProps) {
namespace,
} = props;

const selectedArtifactsForDisplay = useMemo(
() =>
selectedArtifacts.map((selectedArtifact) => ({
selectedItem: selectedArtifact.selectedItem,
linkedArtifact: getLinkedArtifactFromSelectedItem(
filteredRunArtifacts,
selectedArtifact.selectedItem,
),
})),
[filteredRunArtifacts, selectedArtifacts],
if (selectedArtifacts.length !== COMPARE_PANEL_COUNT) {
logger.error(
`MetricsDropdown expected ${COMPARE_PANEL_COUNT} panels but received ${selectedArtifacts.length}.`,
);
return null;
}

const linkedArtifacts = selectedArtifacts.map((selectedArtifact) =>
getLinkedArtifactFromSelectedItem(filteredRunArtifacts, selectedArtifact.selectedItem),
);

const metricsTabText = metricsTypeToString(metricsTab);
const updateSelectedItemAndArtifact = (panelIndex: number, selectedItem: SelectedItem): void => {
const linkedArtifact = getLinkedArtifactFromSelectedItem(filteredRunArtifacts, selectedItem);
const nextSelectedArtifacts = selectedArtifactsForDisplay.map((selectedArtifact, index) =>
index === panelIndex
? {
selectedItem,
linkedArtifact,
}
: selectedArtifact,
const nextSelectedArtifacts = selectedArtifacts.map((artifact, index) =>
index === panelIndex ? { selectedItem, linkedArtifact } : artifact,
);
updateSelectedArtifacts(nextSelectedArtifacts);
};
Expand All @@ -128,34 +127,30 @@ export default function MetricsDropdown(props: MetricsDropdownProps) {
<table>
<tbody>
<tr>
<td className={classes(css.cell, css.leftCell)}>
<TwoLevelDropdown
title={`Choose a first ${metricsTabText} artifact`}
items={dropdownItems}
selectedItem={selectedArtifactsForDisplay[0].selectedItem}
setSelectedItem={updateSelectedItemAndArtifact.bind(null, 0)}
/>
<VisualizationPanelItem
metricsTab={metricsTab}
metricsTabText={metricsTabText}
linkedArtifact={selectedArtifactsForDisplay[0].linkedArtifact}
namespace={namespace}
/>
</td>
<td className={classes(css.cell, css.rightCell)}>
<TwoLevelDropdown
title={`Choose a second ${metricsTabText} artifact`}
items={dropdownItems}
selectedItem={selectedArtifactsForDisplay[1].selectedItem}
setSelectedItem={updateSelectedItemAndArtifact.bind(null, 1)}
/>
<VisualizationPanelItem
metricsTab={metricsTab}
metricsTabText={metricsTabText}
linkedArtifact={selectedArtifactsForDisplay[1].linkedArtifact}
namespace={namespace}
/>
</td>
{selectedArtifacts.map((selectedArtifact, panelIndex) => (
<td
key={panelIndex}
className={classes(
css.cell,
panelIndex === 0 && css.leftCell,
panelIndex === selectedArtifacts.length - 1 && css.rightCell,
panelIndex > 0 && panelIndex < selectedArtifacts.length - 1 && css.middleCell,
)}
>
<TwoLevelDropdown
title={`Choose a ${panelIndex === 0 ? 'first' : panelIndex === 1 ? 'second' : `panel ${panelIndex + 1}`} ${metricsTabText} artifact`}
items={dropdownItems}
selectedItem={selectedArtifact.selectedItem}
setSelectedItem={updateSelectedItemAndArtifact.bind(null, panelIndex)}
/>
<VisualizationPanelItem
metricsTab={metricsTab}
metricsTabText={metricsTabText}
linkedArtifact={linkedArtifacts[panelIndex]}
namespace={namespace}
/>
</td>
))}
</tr>
</tbody>
</table>
Expand Down Expand Up @@ -317,6 +312,7 @@ function getDropdownItems(filteredRunArtifacts: RunArtifact[]) {
dropdownItems.push({
name: runName,
subItems,
id: runArtifact.run.run_id,
} as DropdownItem);
}
}
Expand All @@ -328,8 +324,10 @@ function getLinkedArtifactFromSelectedItem(
filteredRunArtifacts: RunArtifact[],
selectedItem: SelectedItem,
): LinkedArtifact | undefined {
const filteredRunArtifact = filteredRunArtifacts.find(
(runArtifact) => runArtifact.run.display_name === selectedItem.itemName,
const filteredRunArtifact = filteredRunArtifacts.find((runArtifact) =>
selectedItem.runId
? runArtifact.run.run_id === selectedItem.runId
: runArtifact.run.display_name === selectedItem.itemName,
);

const executionArtifact = filteredRunArtifact?.executionArtifacts.find((executionArtifact) => {
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/lib/v2/CompareUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import { stylesheet } from 'typestyle';
import { RuntimeParameters } from 'src/pages/NewRunV2';
import { V2beta1Run } from 'src/apisv2beta1/run';

export const COMPARE_PANEL_COUNT = 2;

export const compareCss = stylesheet({
smallRelativeContainer: {
position: 'relative',
Expand Down
55 changes: 28 additions & 27 deletions frontend/src/pages/CompareV2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {
import CompareTable, { CompareTableProps } from 'src/components/CompareTable';
import {
compareCss,
COMPARE_PANEL_COUNT,
ExecutionArtifact,
FullArtifactPathMap,
getScalarTableProps,
Expand Down Expand Up @@ -263,9 +264,9 @@ const createSelectedArtifactArray = (count: number): SelectedArtifact[] => {
};

const createInitialSelectedArtifactsMap = () => ({
[MetricsType.CONFUSION_MATRIX]: createSelectedArtifactArray(2),
[MetricsType.HTML]: createSelectedArtifactArray(2),
[MetricsType.MARKDOWN]: createSelectedArtifactArray(2),
[MetricsType.CONFUSION_MATRIX]: createSelectedArtifactArray(COMPARE_PANEL_COUNT),
[MetricsType.HTML]: createSelectedArtifactArray(COMPARE_PANEL_COUNT),
[MetricsType.MARKDOWN]: createSelectedArtifactArray(COMPARE_PANEL_COUNT),
});

const createInitialRocCurveSelectionState = (): RocCurveSelectionState => ({
Expand All @@ -283,39 +284,39 @@ const areSelectedArtifactsEqual = (
currentArtifacts.every((currentArtifact, index) => {
const nextArtifact = nextArtifacts[index];
return (
currentArtifact.selectedItem.runId === nextArtifact.selectedItem.runId &&
currentArtifact.selectedItem.itemName === nextArtifact.selectedItem.itemName &&
currentArtifact.selectedItem.subItemName === nextArtifact.selectedItem.subItemName &&
currentArtifact.linkedArtifact?.artifact.getId() ===
nextArtifact.linkedArtifact?.artifact.getId()
);
});

// Ensure that the two-panel selected artifacts are present in the selected valid run list.
const getVerifiedTwoPanelSelection = (
const getVerifiedPanelSelection = (
runArtifacts: RunArtifact[],
selectedArtifacts: SelectedArtifact[],
) => {
const artifactsPresent: boolean[] = new Array(2).fill(false);
for (const runArtifact of runArtifacts) {
const runName = runArtifact.run.display_name;
if (runName === selectedArtifacts[0].selectedItem.itemName) {
artifactsPresent[0] = true;
}
if (runName === selectedArtifacts[1].selectedItem.itemName) {
artifactsPresent[1] = true;
): SelectedArtifact[] => {
const validRunIds = new Set(
runArtifacts.map((r) => r.run.run_id).filter((id): id is string => id !== undefined),
);
const validRunDisplayNames = new Set(
runArtifacts.map((runArtifact) => runArtifact.run.display_name),
);
return selectedArtifacts.map((selectedArtifact) => {
const { runId, itemName } = selectedArtifact.selectedItem;
if (runId) {
if (validRunIds.has(runId)) {
return selectedArtifact;
}
if (validRunDisplayNames.has(itemName)) {
return { selectedItem: { ...selectedArtifact.selectedItem, runId: undefined } };
}
return { selectedItem: { itemName: '', subItemName: '' } };
}
}

return selectedArtifacts.map((selectedArtifact, index) => {
if (artifactsPresent[index]) {
if (validRunDisplayNames.has(itemName)) {
return selectedArtifact;
}
return {
selectedItem: {
itemName: '',
subItemName: '',
},
};
return { selectedItem: { itemName: '', subItemName: '' } };
});
};

Expand All @@ -329,15 +330,15 @@ const reconcileSelectedArtifactsMap = (

const nextSelectedArtifactsMap = {
...currentSelectedArtifactsMap,
[MetricsType.CONFUSION_MATRIX]: getVerifiedTwoPanelSelection(
[MetricsType.CONFUSION_MATRIX]: getVerifiedPanelSelection(
metricsArtifactData.confusionMatrixRunArtifacts,
currentSelectedArtifactsMap[MetricsType.CONFUSION_MATRIX],
),
[MetricsType.HTML]: getVerifiedTwoPanelSelection(
[MetricsType.HTML]: getVerifiedPanelSelection(
metricsArtifactData.htmlRunArtifacts,
currentSelectedArtifactsMap[MetricsType.HTML],
),
[MetricsType.MARKDOWN]: getVerifiedTwoPanelSelection(
[MetricsType.MARKDOWN]: getVerifiedPanelSelection(
metricsArtifactData.markdownRunArtifacts,
currentSelectedArtifactsMap[MetricsType.MARKDOWN],
),
Expand Down
Loading