FIX initializer edit save failures

frontend/src/components/Initializers/AdditionalInitializers.test.tsx

@@ -1,6 +1,7 @@
 import { fireEvent, render, screen, within } from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 import { FluentProvider, webLightTheme } from '@fluentui/react-components'
+import { useState } from 'react'
 
 import type { AdditionalInitializerSetting, RegisteredInitializer } from '@/types'
 
@@ -138,7 +139,8 @@ describe('AdditionalInitializers', () => {
     registeredInitializers: [targetInitializer, scorerInitializer],
     creating: false,
     onAdd: jest.fn().mockResolvedValue(true),
-    onSave: jest.fn().mockResolvedValue(undefined),
+    onSave: jest.fn().mockResolvedValue(true),
+    onClearSaveError: jest.fn(),
     onApply: jest.fn().mockResolvedValue(undefined),
     onRemove: jest.fn().mockResolvedValue(undefined),
   }
@@ -238,11 +240,11 @@ describe('AdditionalInitializers', () => {
 
     await user.click(within(screen.getByTestId('initializer-row-additional-1')).getByRole('button', { name: 'Remove' }))
 
-    const dialog = await screen.findByRole('dialog')
+    const dialog = await screen.findByRole('dialog', { hidden: true })
     expect(within(dialog).getByText(/remove the/i)).toBeInTheDocument()
     expect(within(dialog).getByText('target')).toBeInTheDocument()
 
-    await user.click(within(dialog).getByRole('button', { name: 'Remove' }))
+    await user.click(within(dialog).getByRole('button', { name: 'Remove', hidden: true }))
 
     expect(defaultProps.onRemove).toHaveBeenCalledWith('additional-1')
   })
@@ -258,8 +260,8 @@ describe('AdditionalInitializers', () => {
 
     await user.click(within(screen.getByTestId('initializer-row-additional-1')).getByRole('button', { name: 'Remove' }))
 
-    const dialog = await screen.findByRole('dialog')
-    await user.click(within(dialog).getByRole('button', { name: 'Cancel' }))
+    const dialog = await screen.findByRole('dialog', { hidden: true })
+    await user.click(within(dialog).getByRole('button', { name: 'Cancel', hidden: true }))
 
     expect(defaultProps.onRemove).not.toHaveBeenCalled()
   })
@@ -337,6 +339,55 @@ describe('AdditionalInitializers', () => {
     expect(defaultProps.onAdd).toHaveBeenCalledWith('tagged_target', { tags: ['default', 'scorer'] })
   })
 
+  it('should keep the edit dialog open and show an inline error when save fails', async () => {
+    const user = userEvent.setup()
+    const onSave = jest.fn()
+    const onClearSaveError = jest.fn()
+
+    function TestComponent() {
+      const [saveErrors, setSaveErrors] = useState<Record<string, string>>({})
+
+      return (
+        <AdditionalInitializers
+          {...defaultProps}
+          saveErrors={saveErrors}
+          onSave={async (id, request) => {
+            onSave(id, request)
+            setSaveErrors({ [id]: 'Mock save failure' })
+            return false
+          }}
+          onClearSaveError={(id) => {
+            onClearSaveError(id)
+            setSaveErrors({})
+          }}
+        />
+      )
+    }
+
+    render(
+      <TestWrapper>
+        <TestComponent />
+      </TestWrapper>,
+    )
+
+    const row = screen.getByTestId('initializer-row-additional-1')
+    fireEvent.click(within(row).getByRole('button', { name: 'Edit' }))
+
+    const dialog = await screen.findByRole('dialog', {}, { timeout: 3000 })
+    await within(dialog).findByText('Edit target initializer')
+    const editor = within(dialog).getByTestId('param-tags')
+    fireEvent.change(editor, { target: { value: 'modified' } })
+    await user.click(await within(dialog).findByRole('button', { name: 'Save', hidden: true }))
+
+    expect(screen.getByRole('dialog', { hidden: true })).toBeInTheDocument()
+    expect(await within(dialog).findByRole('alert', { hidden: true })).toHaveTextContent('Mock save failure')
+    expect(editor).toHaveValue('modified')
+
+    await user.click(within(dialog).getByRole('button', { name: 'Cancel', hidden: true }))
+
+    expect(onClearSaveError).toHaveBeenCalledWith('additional-1')
+  })
+
   it('should hide the parameters editor and submit null for a no-parameter initializer', async () => {
     const user = userEvent.setup()
 

frontend/src/components/Initializers/AdditionalInitializers.tsx

@@ -27,10 +27,12 @@ interface AdditionalInitializersProps {
   registeredInitializers: RegisteredInitializer[]
   creating: boolean
   savingInitializerId?: string | null
+  saveErrors?: Record<string, string>
   applyingInitializerId?: string | null
   deletingInitializerId?: string | null
   onAdd: (initializerName: string, parameters: Record<string, unknown> | null) => Promise<boolean>
-  onSave: (id: string, request: UpdateAdditionalInitializerRequest) => Promise<void>
+  onSave: (id: string, request: UpdateAdditionalInitializerRequest) => Promise<boolean>
+  onClearSaveError: (id: string) => void
   onApply: (id: string, initializerName: string, parameters?: Record<string, unknown> | null) => Promise<void>
   onRemove: (id: string) => Promise<void>
 }
@@ -41,7 +43,9 @@ interface AdditionalInitializerCardProps {
   isSaving: boolean
   isApplying: boolean
   isDeleting: boolean
-  onSave: (id: string, request: UpdateAdditionalInitializerRequest) => Promise<void>
+  saveError?: string | null
+  onSave: (id: string, request: UpdateAdditionalInitializerRequest) => Promise<boolean>
+  onClearSaveError: (id: string) => void
   onApply: (id: string, initializerName: string, parameters?: Record<string, unknown> | null) => Promise<void>
   onRemove: (id: string) => Promise<void>
 }
@@ -52,7 +56,9 @@ function AdditionalInitializerCard({
   isSaving,
   isApplying,
   isDeleting,
+  saveError,
   onSave,
+  onClearSaveError,
   onApply,
   onRemove,
 }: AdditionalInitializerCardProps) {
@@ -62,8 +68,17 @@ function AdditionalInitializerCard({
   const isBusy = isSaving || isApplying || isDeleting
 
   const handleEditSubmit = async (parameters: Record<string, unknown> | null): Promise<void> => {
-    await onSave(item.id, { parameters, order_index: item.order_index ?? null })
-    setEditOpen(false)
+    const saved = await onSave(item.id, { parameters, order_index: item.order_index ?? null })
+    if (saved) {
+      setEditOpen(false)
+    }
+  }
+
+  const handleEditOpenChange = (open: boolean): void => {
+    setEditOpen(open)
+    if (!open) {
+      onClearSaveError(item.id)
+    }
   }
 
   return (
@@ -130,8 +145,9 @@ function AdditionalInitializerCard({
           initializer={initializer}
           initialParameters={item.parameters}
           submitting={isSaving}
+          externalError={saveError}
           onSubmit={handleEditSubmit}
-          onOpenChange={setEditOpen}
+          onOpenChange={handleEditOpenChange}
         />
       )}
     </div>
@@ -143,10 +159,12 @@ export default function AdditionalInitializers({
   registeredInitializers,
   creating,
   savingInitializerId = null,
+  saveErrors = {},
   applyingInitializerId = null,
   deletingInitializerId = null,
   onAdd,
   onSave,
+  onClearSaveError,
   onApply,
   onRemove,
 }: AdditionalInitializersProps) {
@@ -223,7 +241,9 @@ export default function AdditionalInitializers({
               isSaving={savingInitializerId === item.id}
               isApplying={applyingInitializerId === item.id}
               isDeleting={deletingInitializerId === item.id}
+              saveError={saveErrors[item.id] ?? null}
               onSave={onSave}
+              onClearSaveError={onClearSaveError}
               onApply={onApply}
               onRemove={onRemove}
             />

frontend/src/components/Initializers/AvailableInitializersDialog.test.tsx

@@ -50,7 +50,7 @@ describe('AvailableInitializersDialog', () => {
 
     await user.click(screen.getByRole('button', { name: /browse available initializers/i }))
 
-    const dialog = await screen.findByRole('dialog')
+    const dialog = await screen.findByRole('dialog', { hidden: true })
     const refreshRow = within(dialog).getByTestId('available-initializer-row-refresh_datasets')
     expect(within(refreshRow).getByText('Refreshes datasets.')).toBeInTheDocument()
     expect(within(refreshRow).getByText(/HF_TOKEN/)).toBeInTheDocument()
@@ -71,7 +71,7 @@ describe('AvailableInitializersDialog', () => {
 
     await user.click(screen.getByRole('button', { name: /browse available initializers/i }))
 
-    const dialog = await screen.findByRole('dialog')
+    const dialog = await screen.findByRole('dialog', { hidden: true })
     expect(within(dialog).getByText('No registered initializers were found.')).toBeInTheDocument()
   })
 })

frontend/src/components/Initializers/InitializerParametersDialog.test.tsx

@@ -95,7 +95,7 @@ describe('InitializerParametersDialog', () => {
 
     expect(screen.getByText('This initializer takes no parameters.')).toBeInTheDocument()
 
-    await user.click(screen.getByRole('button', { name: 'Add' }))
+    await user.click(screen.getByRole('button', { name: 'Add', hidden: true }))
 
     expect(onSubmit).toHaveBeenCalledWith(null)
   })
@@ -109,7 +109,7 @@ describe('InitializerParametersDialog', () => {
       </TestWrapper>,
     )
 
-    await user.click(screen.getByRole('button', { name: 'Add' }))
+    await user.click(screen.getByRole('button', { name: 'Add', hidden: true }))
 
     expect(await screen.findByRole('alert')).toHaveTextContent('label is required.')
     expect(onSubmit).not.toHaveBeenCalled()
@@ -126,7 +126,7 @@ describe('InitializerParametersDialog', () => {
 
     fireEvent.change(screen.getByTestId('param-days'), { target: { value: '7' } })
     fireEvent.change(screen.getByTestId('param-names'), { target: { value: 'x, y' } })
-    await user.click(screen.getByRole('button', { name: 'Add' }))
+    await user.click(screen.getByRole('button', { name: 'Add', hidden: true }))
 
     expect(onSubmit).toHaveBeenCalledWith({ days: 7, names: ['x', 'y'] })
   })
@@ -142,11 +142,26 @@ describe('InitializerParametersDialog', () => {
 
     await user.click(screen.getByTestId('param-flag'))
     await user.click(screen.getByTestId('param-tags-a'))
-    await user.click(screen.getByRole('button', { name: 'Add' }))
+    await user.click(screen.getByRole('button', { name: 'Add', hidden: true }))
 
     expect(onSubmit).toHaveBeenCalledWith(expect.objectContaining({ flag: true, tags: ['a'] }))
   })
 
+  it('toggles the intended multiselect option when clicking checkbox label text', async () => {
+    const user = userEvent.setup()
+    const onSubmit = jest.fn().mockResolvedValue(undefined)
+    render(
+      <TestWrapper>
+        <InitializerParametersDialog {...baseProps} onSubmit={onSubmit} initializer={allKindsInitializer} />
+      </TestWrapper>,
+    )
+
+    await user.click(screen.getByText('b'))
+    await user.click(screen.getByRole('button', { name: 'Add', hidden: true }))
+
+    expect(onSubmit).toHaveBeenCalledWith(expect.objectContaining({ tags: ['b'] }))
+  })
+
   it('unchecks a multiselect choice and picks a select value', async () => {
     const user = userEvent.setup()
     const onSubmit = jest.fn().mockResolvedValue(undefined)
@@ -159,7 +174,7 @@ describe('InitializerParametersDialog', () => {
     await user.click(screen.getByTestId('param-tags-a'))
     await user.click(screen.getByTestId('param-tags-a'))
     fireEvent.change(screen.getByTestId('param-level'), { target: { value: 'high' } })
-    await user.click(screen.getByRole('button', { name: 'Add' }))
+    await user.click(screen.getByRole('button', { name: 'Add', hidden: true }))
 
     expect(onSubmit).toHaveBeenCalledWith(expect.objectContaining({ level: 'high' }))
     expect(onSubmit.mock.calls[0][0]).not.toHaveProperty('tags')
@@ -221,4 +236,23 @@ describe('InitializerParametersDialog', () => {
 
     expect(screen.getByRole('alert')).toHaveTextContent('Server rejected the request.')
   })
+
+  it('prefers validation error over externalError', async () => {
+    const user = userEvent.setup()
+    const onSubmit = jest.fn().mockResolvedValue(undefined)
+    render(
+      <TestWrapper>
+        <InitializerParametersDialog
+          {...baseProps}
+          onSubmit={onSubmit}
+          initializer={requiredInitializer}
+          externalError="Server error"
+        />
+      </TestWrapper>,
+    )
+
+    await user.click(screen.getByRole('button', { name: 'Add', hidden: true }))
+
+    expect(await screen.findByRole('alert')).toHaveTextContent('label is required.')
+  })
 })

frontend/src/components/Initializers/InitializerParametersDialog.tsx

@@ -1,4 +1,4 @@
-import { useState } from 'react'
+import { useRef, useState } from 'react'
 import {
   Button,
   Checkbox,
@@ -52,6 +52,7 @@ export default function InitializerParametersDialog({
     getInitialFormValues(parameters, initialParameters),
   )
   const [error, setError] = useState<string | null>(null)
+  const submitInProgressRef = useRef(false)
 
   const acceptsParameters = parameters.length > 0
 
@@ -61,6 +62,15 @@ export default function InitializerParametersDialog({
   }
 
   const handleSubmit = async (): Promise<void> => {
+    submitInProgressRef.current = true
+    try {
+      await submitForm()
+    } finally {
+      submitInProgressRef.current = false
+    }
+  }
+
+  const submitForm = async (): Promise<void> => {
     if (!acceptsParameters) {
       setError(null)
       await onSubmit(null)
@@ -82,7 +92,15 @@ export default function InitializerParametersDialog({
   const submitLabel = mode === 'add' ? 'Add' : 'Save'
 
   return (
-    <Dialog open={open} onOpenChange={(_, data) => onOpenChange(data.open)}>
+    <Dialog
+      open={open}
+      onOpenChange={(_, data) => {
+        if (!data.open && submitInProgressRef.current) {
+          return
+        }
+        onOpenChange(data.open)
+      }}
+    >
       <DialogSurface>
         <DialogBody>
           <DialogTitle>{title}</DialogTitle>
@@ -173,6 +191,7 @@ function ParameterField({ parameter, value, disabled, onChange }: ParameterField
           {(parameter.choices ?? []).map((choice) => (
             <Checkbox
               key={choice}
+              id={`param-${parameter.name}-${choice}`}
               label={choice}
               checked={selected.includes(choice)}
               disabled={disabled}

frontend/src/components/Initializers/Initializers.test.tsx

@@ -237,6 +237,22 @@ describe('Initializers', () => {
     })
   })
 
+  it('should show save errors in the edit dialog and preserve edits', async () => {
+    const user = userEvent.setup()
+    mockedInitializersApi.updateAdditional.mockRejectedValue(new Error('Mock save failure'))
+    renderInitializers()
+
+    await screen.findByTestId('initializer-row-additional-1')
+    const dialog = await openDialogByButton(user, 'Edit', 'Edit scorer initializer')
+    const editor = within(dialog).getByTestId('param-tags')
+    fireEvent.change(editor, { target: { value: 'relaxed' } })
+    await user.click(await within(dialog).findByRole('button', { name: 'Save', hidden: true }))
+
+    expect(screen.getByRole('dialog', { hidden: true })).toBeInTheDocument()
+    expect(await within(dialog).findByRole('alert', { hidden: true })).toHaveTextContent('Mock save failure')
+    expect(editor).toHaveValue('relaxed')
+  })
+
   it('should apply an additional initializer', async () => {
     const user = userEvent.setup()
     renderInitializers()
@@ -276,8 +292,8 @@ describe('Initializers', () => {
     const row = await screen.findByTestId('initializer-row-additional-1')
     await user.click(within(row).getByRole('button', { name: 'Remove' }))
 
-    const dialog = await screen.findByRole('dialog')
-    await user.click(within(dialog).getByRole('button', { name: 'Remove' }))
+    const dialog = await screen.findByRole('dialog', { hidden: true })
+    await user.click(within(dialog).getByRole('button', { name: 'Remove', hidden: true }))
 
     await waitFor(() => {
       expect(mockedInitializersApi.deleteAdditional).toHaveBeenCalledWith('additional-1')