Skip to content
Merged
Show file tree
Hide file tree
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
53 changes: 45 additions & 8 deletions apps/opik-frontend/src/hooks/useEvaluationSuiteSavePayload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ interface BuildPayloadOptions {
tags?: string[];
changeDescription?: string;
override?: boolean;
baseVersionOverride?: string;
}

interface UseEvaluationSuiteSavePayloadOptions {
Expand Down Expand Up @@ -45,7 +46,12 @@ export function useEvaluationSuiteSavePayload({
const queryClient = useQueryClient();

const buildPayload = useCallback(
({ tags, changeDescription, override = false }: BuildPayloadOptions) => {
({
tags,
changeDescription,
override = false,
baseVersionOverride,
}: BuildPayloadOptions) => {
const state = useEvaluationSuiteDraftStore.getState();

if (!suite) {
Expand All @@ -54,12 +60,8 @@ export function useEvaluationSuiteSavePayload({
);
}

const baseVersion = suite.latest_version?.id ?? "";
if (!baseVersion) {
throw new Error(
"Base version is missing. The evaluation suite may not have been fully loaded.",
);
}
const baseVersion =
baseVersionOverride ?? suite.latest_version?.id ?? null;

const isEvaluationSuite = suite.type === DATASET_TYPE.EVALUATION_SUITE;

Expand Down Expand Up @@ -142,5 +144,40 @@ export function useEvaluationSuiteSavePayload({
[suiteId, suite, versionEvaluators, queryClient],
);

return { buildPayload };
const hasNoVersion = !suite?.latest_version?.id;

const buildInitialVersionPayload = useCallback(
({ tags, changeDescription }: BuildPayloadOptions) => {
const state = useEvaluationSuiteDraftStore.getState();
const isEvaluationSuite = suite?.type === DATASET_TYPE.EVALUATION_SUITE;

let evaluators: Evaluator[] | undefined;
const executionPolicy = state.executionPolicy ?? undefined;

if (isEvaluationSuite && state.suiteAssertions !== null) {
const originalEvaluator = versionEvaluators[0];
evaluators = [packAssertions(state.suiteAssertions, originalEvaluator)];
}

return {
datasetId: suiteId,
payload: {
added_items: [] as DatasetItem[],
edited_items: [] as Partial<DatasetItem>[],
deleted_ids: [] as string[],
base_version: null,
tags,
change_description: changeDescription,
...(evaluators !== undefined && { evaluators }),
...(executionPolicy !== undefined && {
execution_policy: executionPolicy,
}),
},
override: true,
};
},
[suiteId, suite, versionEvaluators],
);

return { buildPayload, buildInitialVersionPayload, hasNoVersion };
}
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,61 @@ const AddEditEvaluationSuiteDialog = ({
[assertions, runsPerItem, passThreshold, changesMutate],
);

const uploadItems = useCallback(
(datasetId: string, onDone: () => void) => {
if (isCsvUploadEnabled && csvFile) {
createItemsFromCsvMutate(
{ datasetId, csvFile },
{
onSuccess: () => {
toast({
title: "CSV upload accepted",
description:
"Your CSV file is being processed in the background. Items will appear automatically when ready. If you don't see them, try refreshing the page.",
});
},
onError: (error: unknown) => {
console.error("Error uploading CSV file:", error);
const errorMessage =
(
error as { response?: { data?: { errors?: string[] } } }
).response?.data?.errors?.join(", ") ||
(error as { message?: string }).message ||
"Failed to upload CSV file";
toast({
title: "Error uploading CSV file",
description: errorMessage,
variant: "destructive",
});
},
onSettled: onDone,
},
);
} else if (!isCsvUploadEnabled && csvData) {
createItemsMutate(
{
datasetId,
workspaceName,
datasetItems: csvData.map((row) => ({
data: row,
source: DATASET_ITEM_SOURCE.manual,
})),
},
{ onSettled: onDone },
);
}
},
[
isCsvUploadEnabled,
csvFile,
csvData,
createItemsFromCsvMutate,
createItemsMutate,
workspaceName,
toast,
],
);

const onCreateSuccessHandler = useCallback(
(newDataset: Dataset) => {
const navigateToDataset = () => {
Expand All @@ -206,75 +261,24 @@ const AddEditEvaluationSuiteDialog = ({
}

if (hasValidCsvFile && newDataset.id) {
const applyThenNavigate = () => {
applyEvaluationCriteria(newDataset.id, navigateToDataset);
const uploadThenNavigate = () => {
uploadItems(newDataset.id, navigateToDataset);
};

if (isCsvUploadEnabled && csvFile) {
// CSV mode: Upload CSV file directly to backend
createItemsFromCsvMutate(
{
datasetId: newDataset.id,
csvFile,
},
{
onSuccess: () => {
toast({
title: "CSV upload accepted",
description:
"Your CSV file is being processed in the background. Items will appear automatically when ready. If you don't see them, try refreshing the page.",
});
},
onError: (error: unknown) => {
console.error("Error uploading CSV file:", error);
const errorMessage =
(
error as { response?: { data?: { errors?: string[] } } }
).response?.data?.errors?.join(", ") ||
(error as { message?: string }).message ||
"Failed to upload CSV file";
toast({
title: "Error uploading CSV file",
description: errorMessage,
variant: "destructive",
});
},
onSettled: applyThenNavigate,
},
);
} else if (!isCsvUploadEnabled && csvData) {
// JSON mode: Send parsed JSON data
createItemsMutate(
{
datasetId: newDataset.id,
workspaceName,
datasetItems: csvData.map((row) => ({
data: row,
source: DATASET_ITEM_SOURCE.manual,
})),
},
{
onSettled: applyThenNavigate,
},
);
}
// Apply evaluation criteria first (creates the initial version),
// then upload items. This matches the Python SDK order and avoids
// a backend rejection when items create a version before criteria.
applyEvaluationCriteria(newDataset.id, uploadThenNavigate);
} else {
// No CSV — wait for evaluation criteria before navigating
applyEvaluationCriteria(newDataset.id, navigateToDataset);
}
},
[
applyEvaluationCriteria,
uploadItems,
hasValidCsvFile,
isCsvUploadEnabled,
csvFile,
csvData,
createItemsFromCsvMutate,
createItemsMutate,
workspaceName,
onDatasetCreated,
setOpen,
toast,
],
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,12 @@ function EvaluationSuiteItemsPage(): React.ReactElement {
);
const latestVersionData = versionsData?.content?.[0];
const versionEvaluators = latestVersionData?.evaluators ?? [];
const { buildPayload } = useEvaluationSuiteSavePayload({
suiteId,
suite,
versionEvaluators,
});
const { buildPayload, buildInitialVersionPayload, hasNoVersion } =
useEvaluationSuiteSavePayload({
suiteId,
suite,
versionEvaluators,
});

const effectiveAssertions = useEffectiveSuiteAssertions(suiteId);

Expand Down Expand Up @@ -179,23 +180,64 @@ function EvaluationSuiteItemsPage(): React.ReactElement {
},
});

const onSaveSuccess = async (version?: { id?: string }) => {
setAddVersionDialogOpen(false);
showSuccessToast(version?.id);
await Promise.all([
queryClient.invalidateQueries({
queryKey: ["dataset-items", { datasetId: suiteId }],
}),
queryClient.invalidateQueries({
queryKey: ["dataset-versions"],
}),
queryClient.invalidateQueries({
queryKey: ["dataset", { datasetId: suiteId }],
}),
]);
clearDraft();
};

const handleSaveChanges = (tags?: string[], changeDescription?: string) => {
if (changesMutation.isPending) return;

if (hasNoVersion) {
// Two-step save: create initial metadata version first, then apply
// item changes on top of it. The backend requires base_version=null
// with no items for the first version.
changesMutation.mutate(
buildInitialVersionPayload({ tags, changeDescription }),
{
onSuccess: (initialVersion) => {
const itemPayload = buildPayload({
Comment on lines 200 to +211
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When hasNoVersion the second buildPayload call omits tags/changeDescription — should we pass them so the final version preserves user metadata?

Finding type: Logical Bugs | Severity: 🔴 High


Want Baz to fix this for you? Activate Fixer

Other fix methods

Fix in Cursor

Prompt for AI Agents:

Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/v2/pages/EvaluationSuiteItemsPage/EvaluationSuiteItemsPage.tsx
around lines 200 to 225, the handleSaveChanges function (the hasNoVersion branch) builds
an itemPayload with buildPayload({ baseVersionOverride: initialVersion?.id }) but does
not pass the tags and changeDescription from the Add Version dialog. Refactor this by
calling buildPayload({ baseVersionOverride: initialVersion?.id, tags, changeDescription
}) so the second mutation includes the same metadata, ensuring the final version
preserves tags and changeDescription provided by the user. Keep the existing
conflict/onError handling the same.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit 4864e55 addressed this comment by passing the tags and changeDescription into the second buildPayload call in the hasNoVersion flow, so the final version preserves the metadata from the Add Version dialog and the existing conflict handling remains unchanged.

baseVersionOverride: initialVersion?.id,
});

const hasItemChanges =
itemPayload.payload.added_items.length > 0 ||
itemPayload.payload.edited_items.length > 0 ||
itemPayload.payload.deleted_ids.length > 0;

if (!hasItemChanges) {
onSaveSuccess(initialVersion);
return;
}

changesMutation.mutate(itemPayload, {
onSuccess: onSaveSuccess,
onError: (error) => {
if ((error as AxiosError).response?.status === 409) {
setPendingVersionData({ tags, changeDescription });
}
},
});
},
},
);
return;
}

changesMutation.mutate(buildPayload({ tags, changeDescription }), {
onSuccess: async (version) => {
setAddVersionDialogOpen(false);
showSuccessToast(version?.id);
await Promise.all([
queryClient.invalidateQueries({
queryKey: ["dataset-items", { datasetId: suiteId }],
}),
queryClient.invalidateQueries({
queryKey: ["dataset-versions"],
}),
]);
clearDraft();
},
onSuccess: onSaveSuccess,
onError: (error) => {
if ((error as AxiosError).response?.status === 409) {
setPendingVersionData({ tags, changeDescription });
Expand All @@ -211,19 +253,9 @@ function EvaluationSuiteItemsPage(): React.ReactElement {
buildPayload({ ...pendingVersionData, override: true }),
{
onSuccess: async (version) => {
setAddVersionDialogOpen(false);
setOverrideDialogOpen(false);
setPendingVersionData(null);
showSuccessToast(version?.id);
await Promise.all([
queryClient.invalidateQueries({
queryKey: ["dataset-items", { datasetId: suiteId }],
}),
queryClient.invalidateQueries({
queryKey: ["dataset-versions"],
}),
]);
clearDraft();
await onSaveSuccess(version);
},
},
);
Expand Down
Loading