Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(ui): make eval section header draggable #2637

Merged
merged 18 commits into from
Dec 11, 2024
Merged
Changes from 1 commit
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
Prev Previous commit
Next Next commit
review comments
  • Loading branch information
gtarpenning committed Dec 3, 2024
commit 71a11824203e1369d67c74e72d9430b10d6a4dd3
Original file line number Diff line number Diff line change
@@ -77,9 +77,6 @@ export const CompareEvaluationsPage: React.FC<
export const CompareEvaluationsPageContent: React.FC<
CompareEvaluationsPageProps
> = props => {
const [selectedCallIdsOrdered, setSelectedCallIdsOrdered] = React.useState<
string[]
>([]);
const [comparisonDimensions, setComparisonDimensions] =
React.useState<ComparisonDimensionsType | null>(null);

@@ -105,10 +102,6 @@ export const CompareEvaluationsPageContent: React.FC<
[comparisonDimensions]
);

React.useEffect(() => {
setSelectedCallIdsOrdered(props.evaluationCallIds);
}, [props.evaluationCallIds, setSelectedCallIdsOrdered]);

if (props.evaluationCallIds.length === 0) {
return <div>No evaluations to compare</div>;
}
@@ -120,8 +113,6 @@ export const CompareEvaluationsPageContent: React.FC<
initialEvaluationCallIds={props.evaluationCallIds}
selectedMetrics={props.selectedMetrics}
setSelectedMetrics={props.setSelectedMetrics}
selectedCallIdsOrdered={selectedCallIdsOrdered}
setSelectedCallIdsOrdered={setSelectedCallIdsOrdered}
comparisonDimensions={comparisonDimensions ?? undefined}
onEvaluationCallIdsUpdate={props.onEvaluationCallIdsUpdate}
setComparisonDimensions={setComparisonDimensionsAndClearInputDigest}
Original file line number Diff line number Diff line change
@@ -10,14 +10,14 @@ import {ComparisonDimensionsType} from './ecpState';

const CompareEvaluationsContext = React.createContext<{
state: EvaluationComparisonState;
setSelectedCallIdsOrdered: React.Dispatch<React.SetStateAction<string[]>>;
setComparisonDimensions: React.Dispatch<
React.SetStateAction<ComparisonDimensionsType | null>
>;
setSelectedInputDigest: React.Dispatch<React.SetStateAction<string | null>>;
setSelectedMetrics: (newModel: Record<string, boolean>) => void;
addEvaluationCall: (newCallId: string) => void;
removeEvaluationCall: (callId: string) => void;
setEvaluationCallOrder: (newCallIdOrder: string[]) => void;
} | null>(null);

export const useCompareEvaluationsState = () => {
@@ -32,8 +32,6 @@ export const CompareEvaluationsProvider: React.FC<{
entity: string;
project: string;
initialEvaluationCallIds: string[];
selectedCallIdsOrdered: string[];
setSelectedCallIdsOrdered: React.Dispatch<React.SetStateAction<string[]>>;
selectedMetrics: Record<string, boolean> | null;
setSelectedMetrics: (newModel: Record<string, boolean>) => void;

@@ -48,8 +46,6 @@ export const CompareEvaluationsProvider: React.FC<{
entity,
project,
initialEvaluationCallIds,
setSelectedCallIdsOrdered,
selectedCallIdsOrdered,
selectedMetrics,
setSelectedMetrics,
onEvaluationCallIdsUpdate,
@@ -71,7 +67,6 @@ export const CompareEvaluationsProvider: React.FC<{
entity,
project,
evaluationCallIds,
selectedCallIdsOrdered,
comparisonDimensions,
selectedInputDigest,
selectedMetrics ?? undefined
@@ -83,30 +78,30 @@ export const CompareEvaluationsProvider: React.FC<{
}
return {
state: initialState.result,
setSelectedCallIdsOrdered,
setComparisonDimensions,
setSelectedInputDigest,
setSelectedMetrics,
addEvaluationCall: (newCallId: string) => {
const newEvaluationCallIds = [...evaluationCallIds, newCallId];
setEvaluationCallIds(newEvaluationCallIds);
setSelectedCallIdsOrdered(newEvaluationCallIds);
onEvaluationCallIdsUpdate(newEvaluationCallIds);
},
removeEvaluationCall: (callId: string) => {
const newEvaluationCallIds = evaluationCallIds.filter(
id => id !== callId
);
setEvaluationCallIds(newEvaluationCallIds);
setSelectedCallIdsOrdered(newEvaluationCallIds);
onEvaluationCallIdsUpdate(newEvaluationCallIds);
},
setEvaluationCallOrder: (newCallIdOrder: string[]) => {
setEvaluationCallIds(newCallIdOrder);
onEvaluationCallIdsUpdate(newCallIdOrder);
},
};
}, [
initialState.loading,
initialState.result,
setEvaluationCallIds,
setSelectedCallIdsOrdered,
evaluationCallIds,
onEvaluationCallIdsUpdate,
setComparisonDimensions,
Original file line number Diff line number Diff line change
@@ -24,8 +24,8 @@ export type EvaluationComparisonState = {
selectedInputDigest?: string;
// The selected metrics to display
selectedMetrics?: Record<string, boolean>;
// The order of the evaluation calls, the first call is always the baseline
selectedCallIdsOrdered: string[];
// Ordered call Ids
evaluationCallIdsOrdered: string[];
};

export type ComparisonDimensionsType = Array<{
@@ -43,12 +43,14 @@ export const useEvaluationComparisonState = (
entity: string,
project: string,
evaluationCallIds: string[],
selectedCallIdsOrdered: string[],
comparisonDimensions?: ComparisonDimensionsType,
selectedInputDigest?: string,
selectedMetrics?: Record<string, boolean>
): Loadable<EvaluationComparisonState> => {
const data = useEvaluationComparisonData(entity, project, evaluationCallIds);
const orderedCallIds = useMemo(() => {
return getCallIdsOrderedForQuery(evaluationCallIds);
}, [evaluationCallIds]);
const data = useEvaluationComparisonData(entity, project, orderedCallIds);

const value = useMemo(() => {
if (data.result == null || data.loading) {
@@ -94,34 +96,35 @@ export const useEvaluationComparisonState = (
data: data.result,
comparisonDimensions: newComparisonDimensions,
selectedInputDigest,
selectedCallIdsOrdered,
selectedMetrics,
evaluationCallIdsOrdered: evaluationCallIds,
},
};
}, [
data.result,
data.loading,
selectedCallIdsOrdered,
comparisonDimensions,
selectedInputDigest,
selectedMetrics,
evaluationCallIds,
]);

return value;
};

/**
* Should use this over keys of `state.data.evaluationCalls` because it ensures the baseline
* evaluation call is first.
*/
export const getOrderedCallIds = (state: EvaluationComparisonState) => {
return state.selectedCallIdsOrdered.filter(
callId => state.data.evaluationCalls[callId] != null
);
return state.evaluationCallIdsOrdered;
};

export const getBaselineCallId = (state: EvaluationComparisonState) => {
return state.selectedCallIdsOrdered[0];
return getOrderedCallIds(state)[0];
};

/**
* Sort call IDs to ensure consistent order for memoized query params
*/
const getCallIdsOrderedForQuery = (callIds: string[]) => {
return Array.from(callIds).sort();
};

/**
@@ -147,3 +150,29 @@ const moveItemToFront = <T>(arr: T[], item: T): T[] => {
}
return arr;
};

export const getOrderedEvalsWithNewBaseline = (
evaluationCallIds: string[],
newBaselineCallId: string
) => {
return moveItemToFront(evaluationCallIds, newBaselineCallId);
};

export const swapEvaluationCalls = (
evaluationCallIds: string[],
ndx1: number,
ndx2: number
): string[] => {
return swapArrayItems(evaluationCallIds, ndx1, ndx2);
};

const swapArrayItems = <T>(arr: T[], ndx1: number, ndx2: number): T[] => {
if (ndx1 < 0 || ndx2 < 0 || ndx1 >= arr.length || ndx2 >= arr.length) {
throw new Error('Index out of bounds');
}
const newArr = [...arr];
const from = newArr[ndx1];
newArr[ndx1] = newArr[ndx2];
newArr[ndx2] = from;
return newArr;
};
Original file line number Diff line number Diff line change
@@ -18,6 +18,7 @@ export type EvaluationComparisonData = {
};

// EvaluationCalls are the specific calls of an evaluation.
// The visual order of the evaluation calls is determined by the order of the keys.
evaluationCalls: {
[callId: string]: EvaluationCall;
};
Original file line number Diff line number Diff line change
@@ -24,30 +24,27 @@ import {
} from '../../../wfReactInterface/wfDataModelHooksInterface';
import {useCompareEvaluationsState} from '../../compareEvaluationsContext';
import {STANDARD_PADDING} from '../../ecpConstants';
import {EvaluationComparisonState, getOrderedCallIds} from '../../ecpState';
import {
EvaluationComparisonState,
getOrderedCallIds,
swapEvaluationCalls,
} from '../../ecpState';
import {HorizontalBox} from '../../Layout';
import {useDragDropReorder} from './dragUtils';
import {EvaluationDefinition, VerticalBar} from './EvaluationDefinition';

export const ComparisonDefinitionSection: React.FC<{
state: EvaluationComparisonState;
}> = props => {
const {setSelectedCallIdsOrdered} = useCompareEvaluationsState();
const {setEvaluationCallOrder} = useCompareEvaluationsState();

const reorderItems = useCallback(
(fromIndex: number, toIndex: number) => {
setSelectedCallIdsOrdered(prev => {
if (prev == null) {
return prev;
}
const newOrder = [...prev];
const from = newOrder[fromIndex];
newOrder[fromIndex] = newOrder[toIndex];
newOrder[toIndex] = from;
return newOrder;
});
const currentOrder = getOrderedCallIds(props.state);
const newOrder = swapEvaluationCalls(currentOrder, fromIndex, toIndex);
setEvaluationCallOrder(newOrder);
},
[setSelectedCallIdsOrdered]
[setEvaluationCallOrder, props.state]
);

const {makeDragSourceCallbackRef, onDragOver, onDrop, onDragEnd} =
@@ -56,7 +53,9 @@ export const ComparisonDefinitionSection: React.FC<{
dropzonePadding: 8,
});

const callIds = getOrderedCallIds(props.state);
const callIds = useMemo(() => {
return getOrderedCallIds(props.state);
}, [props.state]);

return (
<DragDropProvider>
@@ -160,9 +159,9 @@ const AddEvaluationButton: React.FC<{

const evalsNotComparing = useMemo(() => {
return calls.result.filter(
call => !props.state.selectedCallIdsOrdered.includes(call.callId)
call => !getOrderedCallIds(props.state).includes(call.callId)
);
}, [calls.result, props.state.selectedCallIdsOrdered]);
}, [calls.result, props.state]);

const [menuOptions, setMenuOptions] =
useState<CallSchema[]>(evalsNotComparing);
Original file line number Diff line number Diff line change
@@ -29,7 +29,12 @@ import {
EVAL_DEF_HEIGHT,
STANDARD_BORDER,
} from '../../ecpConstants';
import {EvaluationComparisonState, getBaselineCallId} from '../../ecpState';
import {
EvaluationComparisonState,
getBaselineCallId,
getOrderedCallIds,
getOrderedEvalsWithNewBaseline,
} from '../../ecpState';
import {HorizontalBox} from '../../Layout';
import {DragHandleIcon} from './dragUtils';

@@ -39,7 +44,7 @@ export const EvaluationDefinition: React.FC<{
ndx: number;
onDragEnd?: (ctx: DragDropState, e: React.DragEvent) => void;
}> = props => {
const {removeEvaluationCall, setSelectedCallIdsOrdered} =
const {removeEvaluationCall, setEvaluationCallOrder} =
useCompareEvaluationsState();

const menuOptions = useMemo(() => {
@@ -48,19 +53,12 @@ export const EvaluationDefinition: React.FC<{
key: 'add-to-baseline',
content: 'Set as baseline',
onClick: () => {
setSelectedCallIdsOrdered(prev => {
if (prev == null) {
return prev;
}
const index = prev.indexOf(props.callId);
if (index === 0) {
return prev;
}
const newOrder = [...prev];
newOrder.splice(index, 1);
newOrder.unshift(props.callId);
return newOrder;
});
const currentOrder = getOrderedCallIds(props.state);
const newOrder = getOrderedEvalsWithNewBaseline(
currentOrder,
props.callId
);
setEvaluationCallOrder(newOrder);
},
disabled: props.callId === getBaselineCallId(props.state),
},
@@ -73,12 +71,7 @@ export const EvaluationDefinition: React.FC<{
disabled: Object.keys(props.state.data.evaluationCalls).length === 1,
},
];
}, [
props.callId,
props.state,
removeEvaluationCall,
setSelectedCallIdsOrdered,
]);
}, [props.callId, props.state, removeEvaluationCall, setEvaluationCallOrder]);

const partRef = useMemo(() => ({id: `${props.ndx}`}), [props.ndx]);

Original file line number Diff line number Diff line change
@@ -32,6 +32,7 @@ import {
} from '../../ecpConstants';
import {
EvaluationComparisonState,
getBaselineCallId,
getOrderedCallIds,
getOrderedModelRefs,
} from '../../ecpState';
Loading