diff --git a/frontend/src/components/ScenarioPresets/ScenarioPresetEditor.test.tsx b/frontend/src/components/ScenarioPresets/ScenarioPresetEditor.test.tsx index 303e900888..d8dfccb70f 100644 --- a/frontend/src/components/ScenarioPresets/ScenarioPresetEditor.test.tsx +++ b/frontend/src/components/ScenarioPresets/ScenarioPresetEditor.test.tsx @@ -1,11 +1,22 @@ +import type { ReactNode } from 'react' + import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { FluentProvider, webLightTheme } from '@fluentui/react-components' -import { Link, MemoryRouter, Route, Routes } from 'react-router' +import { Link, MemoryRouter, Route, Routes, useNavigate } from 'react-router' +import * as scenarioConfigForm from '@/components/Scenarios/scenarioConfigForm' import { scenarioPresetsApi, scenariosApi } from '@/services/api' +import { deferred } from '@/test-utils/deferred' import { makeScenario } from '@/test-utils/scenarioFixtures' -import type { Parameter, ScenarioDatasetSettings, ScenarioPreset, ScenarioRunSizeEstimateResponse } from '@/types' +import type { + Parameter, + RegisteredScenario, + ScenarioDatasetSettings, + ScenarioPreset, + ScenarioPresetResponse, + ScenarioRunSizeEstimateResponse, +} from '@/types' import ScenarioPresetEditor from './ScenarioPresetEditor' @@ -42,9 +53,10 @@ const mockNavigate = jest.fn() jest.mock('react-router', () => ({ ...jest.requireActual('react-router'), - useNavigate: () => mockNavigate, + useNavigate: jest.fn(() => mockNavigate), })) +const mockUseNavigate = jest.mocked(useNavigate) const SCENARIO = makeScenario() const STORED_PRESET: ScenarioPreset = { @@ -62,6 +74,21 @@ function apiError(status: number, detail: string): unknown { } } +function TestWrapper({ children }: { children: ReactNode }) { + return {children} +} + +async function selectScenario(user: ReturnType, scenarioName: string): Promise { + await user.click(screen.getByRole('combobox', { name: 'Scenario' })) + await user.click(await screen.findByRole('option', { name: scenarioName, exact: true })) +} + +function submitPresetForm(): void { + const form = screen.getByRole('button', { name: /^(Save preset|Saving\.\.\.)$/ }).closest('form') + if (!form) throw new Error('Save preset must submit the preset form.') + fireEvent.submit(form) +} + /** Drives the shared dataset table's limit popover the same way its own suite does. */ async function editLimit(user: ReturnType, label: string, value: string): Promise { await user.click(await screen.findByRole('button', { name: `Edit ${label}` })) @@ -74,33 +101,36 @@ async function editLimit(user: ReturnType, label: string function renderCreate() { return render( - - - - } /> - - - , + + + } /> + Preset library} /> + + , + { wrapper: TestWrapper }, ) } function renderEdit(name = 'nightly_probe') { return render( - - - - } - /> - - - , + + + } + /> + + , + { wrapper: TestWrapper }, ) } beforeEach(() => { jest.clearAllMocks() + for (const mock of [mockGet, mockCreate, mockUpdate, mockListCatalog, mockGetScenario, mockEstimateRun]) { + mock.mockReset() + } + mockUseNavigate.mockReturnValue(mockNavigate) mockListCatalog.mockResolvedValue({ items: [SCENARIO], pagination: { has_more: false, next_cursor: null }, @@ -294,6 +324,266 @@ describe('ScenarioPresetEditor create mode', () => { }) }) +describe('ScenarioPresetEditor scenario selection ownership', () => { + const REPLACEMENT_SCENARIO = makeScenario({ + scenario_name: 'airt.multilingual', + all_techniques: ['multilingual'], + default_techniques: ['multilingual'], + supported_parameters: [ + { name: 'languages', type_name: 'list[str]', is_list: true, required: false, default: ['Spanish'] }, + ], + }) + const LATEST_SCENARIO = makeScenario({ + scenario_name: 'garak.prompt_inject', + all_techniques: ['prompt_inject'], + default_techniques: ['prompt_inject'], + }) + + beforeEach(() => { + mockListCatalog.mockResolvedValue({ + items: [SCENARIO, REPLACEMENT_SCENARIO, LATEST_SCENARIO], + pagination: { has_more: false, next_cursor: null }, + }) + }) + + async function renderSelectedCreate( + user: ReturnType, + ): Promise> { + const view = renderCreate() + await user.type(await screen.findByRole('textbox', { name: 'Name' }), 'nightly_probe') + await selectScenario(user, SCENARIO.scenario_name) + await waitFor(() => expect(screen.getByRole('button', { name: 'Save preset' })).toBeEnabled()) + return view + } + + it('disables Save during a pending replacement and saves only the replacement configuration', async () => { + const user = userEvent.setup() + await renderSelectedCreate(user) + await user.click(screen.getByTestId('technique-crescendo')) + const replacement = deferred() + mockGetScenario.mockReturnValueOnce(replacement.promise) + + await selectScenario(user, REPLACEMENT_SCENARIO.scenario_name) + + expect(screen.getByText('Loading scenario...')).toBeInTheDocument() + expect(screen.getByRole('combobox', { name: 'Scenario' })).toHaveValue(REPLACEMENT_SCENARIO.scenario_name) + const save = screen.getByRole('button', { name: 'Save preset' }) + expect(save).toBeDisabled() + await user.click(save) + expect(mockCreate).not.toHaveBeenCalled() + expect(mockNavigate).not.toHaveBeenCalled() + + await act(async () => { replacement.resolve(REPLACEMENT_SCENARIO) }) + expect(await screen.findByRole('textbox', { name: 'languages' })).toHaveValue('Spanish') + expect(save).toBeEnabled() + await user.click(save) + expect(mockCreate).toHaveBeenCalledWith({ + name: 'nightly_probe', + scenario_name: REPLACEMENT_SCENARIO.scenario_name, + }) + expect(mockNavigate).toHaveBeenCalledWith('/registry/scenario-presets') + }) + + it('guards a programmatic submit during pending replacement metadata', async () => { + const user = userEvent.setup() + await renderSelectedCreate(user) + const replacement = deferred() + mockGetScenario.mockReturnValueOnce(replacement.promise) + await selectScenario(user, REPLACEMENT_SCENARIO.scenario_name) + + submitPresetForm() + + expect(mockCreate).not.toHaveBeenCalled() + expect(mockUpdate).not.toHaveBeenCalled() + expect(mockNavigate).not.toHaveBeenCalled() + expect(screen.getByRole('alert')).toHaveTextContent('Wait for the selected scenario to finish loading.') + await act(async () => { replacement.resolve(REPLACEMENT_SCENARIO) }) + }) + + it('guards a programmatic submit before the selection event finishes rendering', async () => { + const user = userEvent.setup() + await renderSelectedCreate(user) + const replacement = deferred() + mockGetScenario.mockReturnValueOnce(replacement.promise) + await user.click(screen.getByRole('combobox', { name: 'Scenario' })) + const option = await screen.findByRole('option', { name: REPLACEMENT_SCENARIO.scenario_name, exact: true }) + let selectionStarted = false + const submitDuringSelection = (): void => { + selectionStarted = mockGetScenario.mock.calls.length === 2 + submitPresetForm() + } + document.addEventListener('click', submitDuringSelection, { once: true }) + try { + await user.click(option) + + expect(selectionStarted).toBe(true) + expect(mockCreate).not.toHaveBeenCalled() + expect(mockNavigate).not.toHaveBeenCalled() + } finally { + document.removeEventListener('click', submitDuringSelection) + await act(async () => { replacement.resolve(REPLACEMENT_SCENARIO) }) + } + }) + + it('rejects resolved metadata that does not match the selected identity', async () => { + const user = userEvent.setup() + await renderSelectedCreate(user) + mockGetScenario.mockResolvedValueOnce(SCENARIO) + + await selectScenario(user, REPLACEMENT_SCENARIO.scenario_name) + + expect(await screen.findByText(/metadata does not match the selected scenario/i)).toBeInTheDocument() + expect(screen.getByRole('combobox', { name: 'Scenario' })).toHaveValue(REPLACEMENT_SCENARIO.scenario_name) + expect(screen.getByRole('button', { name: 'Save preset' })).toBeDisabled() + submitPresetForm() + expect(mockCreate).not.toHaveBeenCalled() + expect(mockNavigate).not.toHaveBeenCalled() + }) + + it('keeps the latest selection and its edits when responses arrive in reverse order', async () => { + const user = userEvent.setup() + await renderSelectedCreate(user) + const older = deferred() + const latest = deferred() + mockGetScenario.mockReturnValueOnce(older.promise).mockReturnValueOnce(latest.promise) + + await selectScenario(user, REPLACEMENT_SCENARIO.scenario_name) + expect(screen.getByRole('combobox', { name: 'Scenario' })).toBeEnabled() + await selectScenario(user, LATEST_SCENARIO.scenario_name) + await act(async () => { latest.resolve(LATEST_SCENARIO) }) + await user.click(screen.getByRole('checkbox', { name: /^baseline$/i })) + await act(async () => { older.resolve(REPLACEMENT_SCENARIO) }) + + expect(screen.getByRole('combobox', { name: 'Scenario' })).toHaveValue(LATEST_SCENARIO.scenario_name) + expect(screen.getByRole('checkbox', { name: /^baseline$/i })).not.toBeChecked() + expect(screen.queryByRole('textbox', { name: 'languages' })).not.toBeInTheDocument() + const save = screen.getByRole('button', { name: 'Save preset' }) + expect(save).toBeEnabled() + await user.click(save) + expect(mockCreate).toHaveBeenCalledWith({ + name: 'nightly_probe', + scenario_name: LATEST_SCENARIO.scenario_name, + include_baseline: false, + }) + }) + + it.each(['resolve', 'reject'] as const)( + 'ignores an older request that %ss while the latest selection is pending', + async (outcome: 'resolve' | 'reject') => { + const user = userEvent.setup() + await renderSelectedCreate(user) + const older = deferred() + const latest = deferred() + mockGetScenario.mockReturnValueOnce(older.promise).mockReturnValueOnce(latest.promise) + + await selectScenario(user, REPLACEMENT_SCENARIO.scenario_name) + expect(screen.getByRole('combobox', { name: 'Scenario' })).toBeEnabled() + await selectScenario(user, LATEST_SCENARIO.scenario_name) + await act(async () => { + if (outcome === 'resolve') older.resolve(REPLACEMENT_SCENARIO) + else older.reject(apiError(503, 'Older scenario request failed.')) + }) + + expect(screen.getByText('Loading scenario...')).toBeInTheDocument() + expect(screen.getByRole('combobox', { name: 'Scenario' })).toHaveValue(LATEST_SCENARIO.scenario_name) + expect(screen.getByRole('button', { name: 'Save preset' })).toBeDisabled() + expect(screen.queryByText('Older scenario request failed.')).not.toBeInTheDocument() + submitPresetForm() + expect(mockCreate).not.toHaveBeenCalled() + await act(async () => { latest.resolve(LATEST_SCENARIO) }) + expect(screen.getByRole('button', { name: 'Save preset' })).toBeEnabled() + }, + ) + + it('keeps a failed selection and its error without saving the old configuration, then permits retry', async () => { + const user = userEvent.setup() + await renderSelectedCreate(user) + await user.type(screen.getByRole('textbox', { name: 'Description' }), 'Keep this draft.') + mockGetScenario.mockRejectedValueOnce(apiError(503, 'Replacement scenario is unavailable.')) + + await selectScenario(user, REPLACEMENT_SCENARIO.scenario_name) + + expect(await screen.findByText('Replacement scenario is unavailable.')).toBeInTheDocument() + expect(screen.getByRole('combobox', { name: 'Scenario' })).toHaveValue(REPLACEMENT_SCENARIO.scenario_name) + expect(screen.queryByText('Loading scenario...')).not.toBeInTheDocument() + expect(screen.getByRole('button', { name: 'Save preset' })).toBeDisabled() + submitPresetForm() + expect(screen.getByText('Replacement scenario is unavailable.')).toBeInTheDocument() + expect(mockCreate).not.toHaveBeenCalled() + expect(mockNavigate).not.toHaveBeenCalled() + + mockGetScenario.mockResolvedValueOnce(REPLACEMENT_SCENARIO) + await selectScenario(user, REPLACEMENT_SCENARIO.scenario_name) + expect(await screen.findByRole('textbox', { name: 'languages' })).toHaveValue('Spanish') + expect(screen.queryByText('Replacement scenario is unavailable.')).not.toBeInTheDocument() + await user.click(screen.getByRole('button', { name: 'Save preset' })) + expect(mockCreate).toHaveBeenCalledWith({ + name: 'nightly_probe', + scenario_name: REPLACEMENT_SCENARIO.scenario_name, + description: 'Keep this draft.', + }) + }) + + it('leaves pending replacement metadata behind when Cancel navigates to the library', async () => { + const user = userEvent.setup() + mockUseNavigate.mockImplementation( + jest.requireActual('react-router').useNavigate, + ) + await renderSelectedCreate(user) + const replacement = deferred() + mockGetScenario.mockReturnValueOnce(replacement.promise) + await selectScenario(user, REPLACEMENT_SCENARIO.scenario_name) + + await user.click(screen.getByRole('button', { name: 'Cancel' })) + expect(await screen.findByRole('heading', { name: 'Preset library' })).toBeInTheDocument() + await act(async () => { replacement.resolve(REPLACEMENT_SCENARIO) }) + + expect(screen.getByRole('heading', { name: 'Preset library' })).toBeInTheDocument() + expect(screen.queryByText('Loading scenario...')).not.toBeInTheDocument() + expect(mockCreate).not.toHaveBeenCalled() + expect(mockUpdate).not.toHaveBeenCalled() + }) + + it('does not apply replacement metadata after unmount', async () => { + const user = userEvent.setup() + const view = await renderSelectedCreate(user) + const replacement = deferred() + mockGetScenario.mockReturnValueOnce(replacement.promise) + const initializeConfig = jest.spyOn(scenarioConfigForm, 'initialScenarioConfigState') + try { + await selectScenario(user, REPLACEMENT_SCENARIO.scenario_name) + view.unmount() + await act(async () => { replacement.resolve(REPLACEMENT_SCENARIO) }) + + expect(initializeConfig).not.toHaveBeenCalled() + expect(mockCreate).not.toHaveBeenCalled() + expect(mockUpdate).not.toHaveBeenCalled() + expect(mockNavigate).not.toHaveBeenCalled() + } finally { + initializeConfig.mockRestore() + } + }) + + it('does not cancel an accepted save on unmount or accept a duplicate programmatic submit', async () => { + const user = userEvent.setup() + const view = await renderSelectedCreate(user) + const saving = deferred() + mockCreate.mockReturnValueOnce(saving.promise) + + await user.click(screen.getByRole('button', { name: 'Save preset' })) + expect(mockCreate).toHaveBeenCalledTimes(1) + expect(screen.getByRole('button', { name: 'Saving...' })).toBeDisabled() + expect(screen.getByRole('combobox', { name: 'Scenario' })).toBeDisabled() + submitPresetForm() + view.unmount() + await act(async () => { saving.resolve({ preset: STORED_PRESET, version: 'v1', issues: [] }) }) + + expect(mockCreate).toHaveBeenCalledTimes(1) + expect(mockNavigate).toHaveBeenCalledTimes(1) + expect(mockNavigate).toHaveBeenCalledWith('/registry/scenario-presets') + }) +}) + describe('ScenarioPresetEditor edit mode', () => { it('loads the stored preset and pins its name and scenario', async () => { renderEdit() @@ -319,6 +609,37 @@ describe('ScenarioPresetEditor edit mode', () => { expect(mockNavigate).toHaveBeenCalledWith('/registry/scenario-presets') }) + it('preserves stored list values when edit metadata finishes loading', async () => { + const user = userEvent.setup() + const metadata = deferred() + const languages = ['French', 'German'] + mockGet.mockResolvedValue({ + preset: { ...STORED_PRESET, scenario_params: { languages } }, + version: 'v1', + issues: [], + }) + mockGetScenario.mockReturnValueOnce(metadata.promise) + renderEdit() + + expect(await screen.findByRole('button', { name: 'Save preset' })).toBeDisabled() + await act(async () => { + metadata.resolve(makeScenario({ + supported_parameters: [ + { name: 'languages', type_name: 'list[str]', is_list: true, required: false, default: ['English'] }, + ], + })) + }) + expect(await screen.findByRole('textbox', { name: 'languages' })).toHaveValue('French, German') + await user.type(screen.getByRole('textbox', { name: 'Description' }), ' Updated.') + await user.click(screen.getByRole('button', { name: 'Save preset' })) + + expect(mockUpdate).toHaveBeenCalledWith( + STORED_PRESET.name, + expect.objectContaining({ techniques: STORED_PRESET.techniques, scenario_params: { languages } }), + 'v1', + ) + }) + it('reports a version conflict instead of navigating away', async () => { const user = userEvent.setup() mockUpdate.mockRejectedValue(apiError(409, 'The preset changed since it was loaded.')) diff --git a/frontend/src/components/ScenarioPresets/ScenarioPresetEditor.tsx b/frontend/src/components/ScenarioPresets/ScenarioPresetEditor.tsx index 29e207996a..2781b21118 100644 --- a/frontend/src/components/ScenarioPresets/ScenarioPresetEditor.tsx +++ b/frontend/src/components/ScenarioPresets/ScenarioPresetEditor.tsx @@ -1,4 +1,4 @@ -import { type FormEvent, useCallback, useEffect, useMemo, useState } from 'react' +import { type FormEvent, useCallback, useEffect, useMemo, useRef, useState } from 'react' import { Button, @@ -87,8 +87,11 @@ function ScenarioPresetEditorContent({ mode, presetName }: ScenarioPresetEditorC const [name, setName] = useState(mode === 'edit' ? decodedName : '') const [description, setDescription] = useState('') const [showInScanner, setShowInScanner] = useState(true) + const [selectedScenarioName, setSelectedScenarioName] = useState('') const [scenario, setScenario] = useState(null) const [scenarioLoading, setScenarioLoading] = useState(false) + const scenarioRequestSequence = useRef(0) + const [resolvedScenarioRequestSequence, setResolvedScenarioRequestSequence] = useState(null) const [config, setConfig] = useState(null) const [scenarioUnavailable, setScenarioUnavailable] = useState(null) const [datasetError, setDatasetError] = useState(null) @@ -123,6 +126,8 @@ function ScenarioPresetEditorContent({ mode, presetName }: ScenarioPresetEditorC setName(response.preset.name) setDescription(response.preset.description ?? '') setShowInScanner(response.preset.show_in_scanner !== false) + setSelectedScenarioName(response.preset.scenario_name) + setScenarioLoading(true) setStatus('ready') // The list endpoint omits run-size estimates, so the preset's scenario is @@ -135,9 +140,15 @@ function ScenarioPresetEditorContent({ mode, presetName }: ScenarioPresetEditorC if (cancelled) return setScenario(full) setConfig(presetToConfigState(full, response.preset)) + setResolvedScenarioRequestSequence(scenarioRequestSequence.current) } catch (scenarioErr) { if (cancelled) return setScenarioUnavailable(toApiError(scenarioErr).detail) + } finally { + if (!cancelled) { + setScenarioLoading(false) + setValidationError(null) + } } } catch (err) { if (cancelled) return @@ -150,26 +161,44 @@ function ScenarioPresetEditorContent({ mode, presetName }: ScenarioPresetEditorC void load() return () => { cancelled = true + scenarioRequestSequence.current += 1 } }, [decodedName, mode]) const handleScenarioChange = useCallback(async (nextScenarioName: string): Promise => { + const requestSequence = scenarioRequestSequence.current + 1 + scenarioRequestSequence.current = requestSequence + setSelectedScenarioName(nextScenarioName) + setScenario(null) + setConfig(null) setScenarioLoading(true) setSaveError(null) setValidationError(null) + setDatasetError(null) setDatasetProjection(null) setDatasetProjectionResult(null) try { const full = await scenariosApi.getScenario(nextScenarioName) + if (requestSequence !== scenarioRequestSequence.current) return + if (full.scenario_name !== nextScenarioName) { + setSaveError('Loaded metadata does not match the selected scenario. Select it again to retry.') + return + } setScenario(full) setConfig(initialScenarioConfigState(full)) + setResolvedScenarioRequestSequence(requestSequence) } catch (err) { + if (requestSequence !== scenarioRequestSequence.current) return setSaveError(toApiError(err).detail) } finally { - setScenarioLoading(false) + if (requestSequence === scenarioRequestSequence.current) { + setScenarioLoading(false) + setValidationError(null) + } } }, []) + const scenarioIsCurrent = scenario !== null && scenario.scenario_name === selectedScenarioName const datasetMetadata = useScenarioDatasets() const datasetBaseline = useMemo( () => (scenario @@ -199,7 +228,7 @@ function ScenarioPresetEditorContent({ mode, presetName }: ScenarioPresetEditorC [config, loaded, scenario], ) const scenarioName = scenario?.scenario_name - const projectionRequest = !scenarioLoading && builtConfig?.ok ? builtConfig.config : null + const projectionRequest = !scenarioLoading && scenarioIsCurrent && builtConfig?.ok ? builtConfig.config : null const projectionIsCurrent = datasetProjectionResult?.request === projectionRequest && datasetProjectionResult?.scenarioName === scenarioName const datasetProjectionLoading = Boolean(scenarioName && projectionRequest && !projectionIsCurrent) @@ -242,9 +271,16 @@ function ScenarioPresetEditorContent({ mode, presetName }: ScenarioPresetEditorC const handleSubmit = async (event: FormEvent): Promise => { event.preventDefault() - // While a newly picked scenario loads, `scenario` and `config` still hold the - // previous one, so saving now would store a preset for the wrong scenario. - if (saving || scenarioLoading) { + if (saving) { + return + } + // A programmatic submit can run before the selection's state updates commit. + const selectionPending = scenarioLoading + || (scenarioIsCurrent && resolvedScenarioRequestSequence !== scenarioRequestSequence.current) + if (selectionPending || !scenarioIsCurrent) { + setValidationError(selectionPending + ? 'Wait for the selected scenario to finish loading.' + : 'Select a scenario.') return } setSaveError(null) @@ -402,9 +438,9 @@ function ScenarioPresetEditorContent({ mode, presetName }: ScenarioPresetEditorC { if (data.optionValue) { @@ -436,7 +472,7 @@ function ScenarioPresetEditorContent({ mode, presetName }: ScenarioPresetEditorC {scenarioLoading && } - {scenario && config && ( + {scenarioIsCurrent && scenario && config && ( <> {saving ? 'Saving...' : 'Save preset'}