diff --git a/doc/gui/0_gui.md b/doc/gui/0_gui.md index fafd4fb397..93307a96b7 100644 --- a/doc/gui/0_gui.md +++ b/doc/gui/0_gui.md @@ -463,7 +463,7 @@ Click "New Target" to open the creation dialog. Fill in: - **Target Type** (required): Select from `OpenAIChatTarget`, `OpenAICompletionTarget`, `OpenAIImageTarget`, `OpenAIVideoTarget`, `OpenAITTSTarget`, `OpenAIResponseTarget`, or `AzureMLChatTarget` - **Endpoint URL** (required): Your Azure OpenAI, OpenAI API, or Azure ML endpoint -- **Model / Deployment Name** (optional): e.g., `gpt-4o`, `dall-e-3`, `Llama-3.2-3B-Instruct` +- **Model / Deployment Name** (required): e.g., `gpt-4o`, `dall-e-3`, `Llama-3.2-3B-Instruct` - **API Key** (optional): Stored in memory only (not persisted to disk) For `AzureMLChatTarget`, additional fields are available: **Max New Tokens**, **Temperature**, **Top P**, and **Repetition Penalty**. diff --git a/frontend/e2e/config.spec.ts b/frontend/e2e/config.spec.ts index 4374477ca6..b0d048e8cd 100644 --- a/frontend/e2e/config.spec.ts +++ b/frontend/e2e/config.spec.ts @@ -456,8 +456,30 @@ test.describe("Create Target Dialog", () => { await expect(page.getByText("OpenAIChatTarget")).toBeVisible(); }); - test("should show validation errors for empty required fields", async ({ page }) => { - await page.route(/\/api\/targets/, async (route) => { + test("should require an endpoint and validate the identity host", async ({ page }) => { + await page.route(/\/api\/targets\/types(?:\?.*)?$/, async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + items: [ + { + target_type: "OpenAIChatTarget", + parameters: [ + { + name: "endpoint", + type_name: "str", + required: true, + default: null, + }, + ], + supported_auth_modes: ["api_key", "identity"], + }, + ], + }), + }); + }); + await page.route(/\/api\/targets(?:\?.*)?$/, async (route) => { await route.fulfill(mockTargetsList([])); }); @@ -467,25 +489,30 @@ test.describe("Create Target Dialog", () => { await page.getByRole("button", { name: /new target/i }).click(); await expect(page.getByText("Create New Target")).toBeVisible(); - // The Create Target button should be disabled when fields are empty - const createBtn = page.locator('[role="dialog"]').getByRole("button", { name: "Create Target" }); - await expect(createBtn).toBeDisabled(); - - // Fill only endpoint (no target type) — button should still be disabled - await page.locator('[role="dialog"]').getByPlaceholder("https://your-resource.openai.azure.com/").fill("https://test.com"); + const dialog = page.locator('[role="dialog"]'); + const createBtn = dialog.getByRole("button", { name: "Create Target" }); await expect(createBtn).toBeDisabled(); - // Clear endpoint, select type — button should still be disabled - await page.locator('[role="dialog"]').getByPlaceholder("https://your-resource.openai.azure.com/").fill(""); await selectTargetType( page, - page.locator('[role="dialog"]'), + dialog, "OpenAIChatTarget" ); + + // Every rendered endpoint field is required. + await expect(createBtn).toBeDisabled(); + await dialog.getByPlaceholder("https://your-resource.openai.azure.com/").fill( + "https://api.openai.com" + ); + await expect(createBtn).toBeEnabled(); + + // Identity authentication additionally requires a recognized Azure host. + await dialog.getByRole("radio", { name: /Identity-based/ }).click(); await expect(createBtn).toBeDisabled(); - // Fill both — button should be enabled - await page.locator('[role="dialog"]').getByPlaceholder("https://your-resource.openai.azure.com/").fill("https://test.com"); + await dialog.getByPlaceholder("https://your-resource.openai.azure.com/").fill( + "https://test.openai.azure.com" + ); await expect(createBtn).toBeEnabled(); }); }); diff --git a/frontend/e2e/numeric-controls.spec.ts b/frontend/e2e/numeric-controls.spec.ts index 58558548ba..22d3ee637d 100644 --- a/frontend/e2e/numeric-controls.spec.ts +++ b/frontend/e2e/numeric-controls.spec.ts @@ -51,11 +51,23 @@ async function mockNumericControlApis(page: Page): Promise { pagination: { limit: 200, has_more: false }, }, '/api/targets/types': { - items: ['AzureMLChatTarget', 'RoundRobinTarget'].map((targetType) => ({ - target_type: targetType, - parameters: [], - supported_auth_modes: ['api_key'], - })), + items: [ + { + target_type: 'AzureMLChatTarget', + parameters: [ + { name: 'max_new_tokens', type_name: 'int', required: false, default: '400' }, + { name: 'temperature', type_name: 'float', required: false, default: '1.0' }, + { name: 'top_p', type_name: 'float', required: false, default: '1.0' }, + { name: 'repetition_penalty', type_name: 'float', required: false, default: '1.0' }, + ], + supported_auth_modes: ['api_key'], + }, + { + target_type: 'RoundRobinTarget', + parameters: [], + supported_auth_modes: ['api_key'], + }, + ], }, '/api/labels': { source: 'attacks', labels: {} }, [`/api/scenarios/catalog/${SCENARIO.scenario_name}`]: SCENARIO, @@ -169,22 +181,29 @@ test('native scenario dataset and shared dynamic parameter controls step once', await expect(datasetSize).toHaveValue('') }) -test('native Azure ML numeric controls retain their configured steps and reset defaults', async ({ page }) => { +test('native Azure ML advanced controls retain steps and reset to default guidance', async ({ page }) => { await page.goto('/registry/targets') await page.getByRole('button', { name: /new target/i }).click() const dialog = page.getByRole('dialog') await dialog.getByRole('combobox', { name: 'Target Type' }).click() await page.getByRole('option', { name: /Implementation: AzureMLChatTarget/ }).click() + await dialog.getByText('Advanced settings').click() for (const [label, initial] of [['Max New Tokens', 400], ['Temperature', 1], ['Top P', 1], ['Repetition Penalty', 1]] as const) { - await expectNativeSteps(dialog.getByRole('spinbutton', { name: label, exact: true }), initial) + const input = dialog.getByRole('spinbutton', { name: label, exact: true }) + await expect(input).toHaveValue('') + await expect(input).toHaveAttribute('placeholder', `Defaults to ${initial === 1 ? '1.0' : initial}`) + await expectNativeSteps(input, initial) } await dialog.getByRole('spinbutton', { name: 'Temperature', exact: true }).fill('0.7') await dialog.getByRole('button', { name: 'Cancel', exact: true }).click() await page.getByRole('button', { name: /new target/i }).click() await dialog.getByRole('combobox', { name: 'Target Type' }).click() await page.getByRole('option', { name: /Implementation: AzureMLChatTarget/ }).click() - await expect(dialog.getByRole('spinbutton', { name: 'Temperature', exact: true })).toHaveValue('1.0') + await dialog.getByText('Advanced settings').click() + const resetTemperature = dialog.getByRole('spinbutton', { name: 'Temperature', exact: true }) + await expect(resetTemperature).toHaveValue('') + await expect(resetTemperature).toHaveAttribute('placeholder', 'Defaults to 1.0') }) test('native round-robin weights step once and preserve bounds and integer validation', async ({ page }) => { diff --git a/frontend/src/components/Config/CreateTargetDialog.styles.ts b/frontend/src/components/Config/CreateTargetDialog.styles.ts index dcceac2d81..a29a0878e2 100644 --- a/frontend/src/components/Config/CreateTargetDialog.styles.ts +++ b/frontend/src/components/Config/CreateTargetDialog.styles.ts @@ -95,6 +95,25 @@ export const useCreateTargetDialogStyles = makeStyles({ overflowWrap: 'anywhere', wordBreak: 'break-word', }, + advancedSettings: { + minWidth: 0, + border: `${tokens.strokeWidthThin} solid ${tokens.colorNeutralStroke2}`, + borderRadius: tokens.borderRadiusMedium, + backgroundColor: tokens.colorNeutralBackground2, + }, + advancedSettingsSummary: { + padding: `${tokens.spacingVerticalS} ${tokens.spacingHorizontalM}`, + color: tokens.colorNeutralForeground1, + fontWeight: tokens.fontWeightSemibold, + cursor: 'pointer', + }, + advancedSettingsFields: { + display: 'flex', + flexDirection: 'column', + minWidth: 0, + gap: tokens.spacingVerticalL, + padding: `0 ${tokens.spacingHorizontalM} ${tokens.spacingVerticalM}`, + }, selectedTargetsSection: { minWidth: 0, maxWidth: '100%', diff --git a/frontend/src/components/Config/CreateTargetDialog.test.tsx b/frontend/src/components/Config/CreateTargetDialog.test.tsx index 7bb983b360..dbc6ecb256 100644 --- a/frontend/src/components/Config/CreateTargetDialog.test.tsx +++ b/frontend/src/components/Config/CreateTargetDialog.test.tsx @@ -2,7 +2,7 @@ import { act, render, screen, waitFor, fireEvent, within } from "@testing-librar import userEvent from "@testing-library/user-event"; import { FluentProvider, webLightTheme } from "@fluentui/react-components"; import { makeTarget } from "@/test-utils/targetFixtures"; -import type { TargetTypeListResponse } from "@/types"; +import type { Parameter, TargetTypeListResponse } from "@/types"; import CreateTargetDialog from "./CreateTargetDialog"; import { parseWeight, MAX_WEIGHT } from "./weightValidation"; import { targetsApi } from "@/services/api"; @@ -17,47 +17,203 @@ jest.mock("@/services/api", () => ({ const mockedTargetsApi = targetsApi as jest.Mocked; +const OPENAI_COMMON_PARAMETERS: Parameter[] = [ + { + name: "model_name", + type_name: "str", + required: false, + default: null, + description: "The model or deployment name.", + }, + { + name: "endpoint", + type_name: "str", + required: false, + default: null, + description: "The target URL for the OpenAI service.", + }, + { + name: "api_key", + type_name: "str | Callable", + required: false, + default: null, + description: "The API key for accessing the OpenAI service.", + }, + { + name: "underlying_model", + type_name: "str", + required: false, + default: null, + description: "The underlying model name used for identification.", + }, +]; + +const AZURE_ML_COMMON_PARAMETERS: Parameter[] = [ + { + name: "endpoint", + type_name: "str", + required: false, + default: null, + description: "The endpoint URL for the deployed Azure ML model.", + }, + { + name: "api_key", + type_name: "str | Callable", + required: false, + default: null, + description: "The API key for accessing the Azure ML endpoint.", + }, + { + name: "model_name", + type_name: "str", + required: false, + default: "", + description: "The name of the deployed model.", + }, +]; + const TARGET_TYPES: TargetTypeListResponse = { items: [ { target_type: "AzureMLChatTarget", - parameters: [], + parameters: [ + ...AZURE_ML_COMMON_PARAMETERS, + { + name: "max_new_tokens", + type_name: "int", + required: false, + default: "400", + description: "The maximum number of tokens to generate in the response.", + }, + { + name: "temperature", + type_name: "float", + required: false, + default: "1.0", + description: "The temperature for generating diverse responses.", + }, + { + name: "top_p", + type_name: "float", + required: false, + default: "1.0", + description: "The top-p value for generating diverse responses.", + }, + { + name: "repetition_penalty", + type_name: "float", + required: false, + default: "1.0", + description: "The repetition penalty for generated responses.", + }, + ], supported_auth_modes: ["api_key", "identity"], description: "A prompt target for Azure Machine Learning chat endpoints.", }, { target_type: "OpenAIChatTarget", - parameters: [], + parameters: [ + { + name: "temperature", + type_name: "float", + required: false, + default: null, + description: "Controls the randomness of the response.", + }, + { + name: "top_p", + type_name: "float", + required: false, + default: null, + description: "Controls the diversity of the response.", + }, + { + name: "seed", + type_name: "int", + required: false, + default: null, + description: "Makes a best effort to sample deterministically.", + }, + { + name: "extra_body_parameters", + type_name: "dict[str, typing.Any]", + required: false, + default: null, + description: "Additional parameters to include in the request body.", + }, + { + name: "httpx_client_kwargs", + type_name: "dict[str, typing.Any]", + required: false, + default: null, + description: "Additional parameters for the HTTP client.", + }, + ...OPENAI_COMMON_PARAMETERS, + ], supported_auth_modes: ["api_key", "identity"], description: "Facilitates multimodal (image and text) input and text output generation.", }, { target_type: "OpenAICompletionTarget", - parameters: [], + parameters: [ + { + name: "stop", + type_name: "list[str]", + required: false, + default: null, + is_list: true, + description: "Sequences where generation should stop.", + }, + ...OPENAI_COMMON_PARAMETERS, + ], supported_auth_modes: ["api_key", "identity"], description: "A prompt target for OpenAI completion endpoints.", }, { target_type: "OpenAIImageTarget", - parameters: [], + parameters: OPENAI_COMMON_PARAMETERS, supported_auth_modes: ["api_key", "identity"], description: "A target for image generation or editing using OpenAI's image models.", }, { target_type: "OpenAIResponseTarget", - parameters: [], + parameters: [ + { + name: "reasoning_effort", + type_name: "str", + required: false, + default: null, + choices: ["none", "minimal", "low", "medium", "high", "xhigh"], + description: "Controls how much reasoning the model performs.", + }, + { + name: "fail_on_missing_function", + type_name: "bool", + required: false, + default: "False", + description: "Raise when the response calls an unknown function.", + }, + { + name: "custom_functions", + type_name: "dict[str, collections.abc.Callable]", + required: false, + default: null, + description: "Mapping of user-defined function names.", + }, + ...OPENAI_COMMON_PARAMETERS, + ], supported_auth_modes: ["api_key", "identity"], description: "Enables communication with endpoints that support the OpenAI Response API.", }, { target_type: "OpenAITTSTarget", - parameters: [], + parameters: OPENAI_COMMON_PARAMETERS, supported_auth_modes: ["api_key", "identity"], description: "A prompt target for OpenAI Text-to-Speech (TTS) endpoints.", }, { target_type: "OpenAIVideoTarget", - parameters: [], + parameters: OPENAI_COMMON_PARAMETERS, supported_auth_modes: ["api_key", "identity"], description: "OpenAI Video Target using the OpenAI SDK for video generation.", }, @@ -67,6 +223,65 @@ const TARGET_TYPES: TargetTypeListResponse = { supported_auth_modes: ["api_key"], description: "A prompt target that distributes requests across multiple inner targets using weighted round-robin selection.", }, + { + target_type: "HTTPTarget", + parameters: [ + { + name: "http_request", + type_name: "str", + required: true, + default: null, + multiline: true, + description: "The HTTP request template containing the prompt placeholder.", + }, + { + name: "use_tls", + type_name: "bool", + required: false, + default: "True", + description: "Whether to use TLS.", + }, + ], + supported_auth_modes: ["api_key"], + description: "Sends prompts through a raw HTTP request template.", + }, + { + target_type: "AzureBlobStorageTarget", + parameters: [ + { + name: "storage_url", + type_name: "str", + required: false, + default: null, + description: "The Azure Blob Storage service URL.", + }, + { + name: "sas_token", + type_name: "str", + required: false, + default: null, + sensitive: true, + identity_conflicting: true, + description: "The optional shared access signature.", + }, + ], + supported_auth_modes: ["api_key", "identity"], + description: "Stores prompts and responses in Azure Blob Storage.", + }, + { + target_type: "PlaywrightTarget", + parameters: [ + { + name: "interaction_func", + type_name: "InteractionFunction", + required: true, + default: null, + description: "The Python function used to interact with the page.", + }, + ], + supported_auth_modes: ["api_key"], + description: "Uses Playwright to interact with a web UI.", + }, ], }; @@ -79,6 +294,8 @@ const TARGET_DISPLAY_NAMES: Record = { OpenAITTSTarget: "OpenAI text to speech", OpenAIVideoTarget: "OpenAI video", RoundRobinTarget: "Weighted round robin", + HTTPTarget: "HTTPTarget", + AzureBlobStorageTarget: "AzureBlobStorageTarget", }; const TestWrapper: React.FC<{ children: React.ReactNode }> = ({ @@ -274,13 +491,15 @@ describe("CreateTargetDialog", () => { await openTargetTypePicker(); const options = screen.getAllByRole("option"); - expect(options).toHaveLength(8); - for (const entry of TARGET_TYPES.items) { + expect(options).toHaveLength(10); + for (const entry of TARGET_TYPES.items.filter((item) => item.target_type !== "PlaywrightTarget")) { const option = screen.getByRole("option", { name: new RegExp(`Implementation: ${entry.target_type}`), }); - expect(within(option).getByText(TARGET_DISPLAY_NAMES[entry.target_type])).toBeInTheDocument(); - expect(within(option).getByText(entry.target_type)).toBeInTheDocument(); + expect( + within(option).getAllByText(TARGET_DISPLAY_NAMES[entry.target_type]).length, + ).toBeGreaterThan(0); + expect(within(option).getByText(entry.target_type, { selector: "code" })).toBeInTheDocument(); expect(within(option).getByText(entry.description ?? "")).toBeInTheDocument(); } @@ -290,6 +509,9 @@ describe("CreateTargetDialog", () => { expect(within(screen.getByRole("option", { name: /Implementation: RoundRobinTarget/, })).getByText("Supported authentication: API key")).toBeInTheDocument(); + expect(screen.queryByRole("option", { + name: /Implementation: PlaywrightTarget/, + })).not.toBeInTheDocument(); }); it("should keep guidance for the selected target visible after the list closes", async () => { @@ -425,13 +647,13 @@ describe("CreateTargetDialog", () => { ); - // No type is chosen yet, so authentication option is not visible yet, but plain API Key input is. + // No type is chosen yet, so neither authentication nor API-key controls are visible. expect( screen.queryByRole("radio", { name: /Identity-based/ }) ).not.toBeInTheDocument(); expect( - screen.getByPlaceholderText("API key (stored in memory only)") - ).toBeInTheDocument(); + screen.queryByPlaceholderText("API key (stored in memory only)") + ).not.toBeInTheDocument(); // Selecting an identity-capable type should reveal the Authentication field. await selectTargetType("OpenAIChatTarget"); @@ -688,7 +910,8 @@ describe("CreateTargetDialog", () => { expect(link).toHaveAttribute("rel", "noopener noreferrer"); }); - it("should show field validation errors when submitting form without endpoint", async () => { + it("should require an endpoint when identity authentication is selected", async () => { + const user = userEvent.setup(); render( @@ -696,8 +919,8 @@ describe("CreateTargetDialog", () => { ); - // Select target type but leave endpoint empty await selectTargetType("OpenAIChatTarget"); + await user.click(screen.getByRole("radio", { name: /Identity-based/ })); // Submit via form (bypass disabled button by submitting the form directly) const form = screen.getByText("Create New Target").closest("form") ?? @@ -735,7 +958,7 @@ describe("CreateTargetDialog", () => { }); }); - it("should create AzureMLChatTarget with AzureML-specific params", async () => { + it("should omit untouched AzureML defaults so the constructor owns default behavior", async () => { const onCreated = jest.fn(); const user = userEvent.setup(); mockedTargetsApi.createTarget.mockResolvedValue(makeTarget({ @@ -764,7 +987,7 @@ describe("CreateTargetDialog", () => { const modelInput = screen.getByPlaceholderText("e.g. Llama-3.2-3B-Instruct"); fireEvent.change(modelInput, { target: { value: "Llama-3.2-3B-Instruct" } }); - // Submit (uses defaults for max_new_tokens, temperature, top_p, repetition_penalty) + // Submit without overriding the displayed constructor defaults. await user.click(screen.getByText("Create Target")); await waitFor(() => { @@ -773,17 +996,13 @@ describe("CreateTargetDialog", () => { params: { endpoint: "https://my-llama.eastus.inference.ml.azure.com/score", model_name: "Llama-3.2-3B-Instruct", - max_new_tokens: 400, - temperature: 1.0, - top_p: 1.0, - repetition_penalty: 1.0, }, }); expect(onCreated).toHaveBeenCalled(); }); }); - it("should show AzureML fields and hide OpenAI fields when AzureMLChatTarget selected", async () => { + it("should show AzureML metadata fields without unrelated OpenAI fields", async () => { render( @@ -799,8 +1018,27 @@ describe("CreateTargetDialog", () => { expect(screen.getByText("Top P")).toBeInTheDocument(); expect(screen.getByText("Repetition Penalty")).toBeInTheDocument(); - // OpenAI-specific fields should NOT be visible, but underlying model switch should be - expect(screen.getByRole("switch")).toBeInTheDocument(); + // Azure ML does not declare an underlying-model constructor parameter. + expect(screen.queryByRole("switch")).not.toBeInTheDocument(); + }); + + it("should show constructor defaults as editable guidance without setting the values", async () => { + render( + + + + ); + + await selectTargetType("AzureMLChatTarget"); + + const maxNewTokens = screen.getByLabelText("Max New Tokens"); + expect(maxNewTokens).toHaveValue(null); + expect(maxNewTokens).toHaveAttribute("placeholder", "Defaults to 400"); + expect(screen.getByText(/maximum number of tokens.*Defaults to 400\./i)).toBeInTheDocument(); + + const repetitionPenalty = screen.getByLabelText("Repetition Penalty"); + expect(repetitionPenalty).toHaveValue(null); + expect(repetitionPenalty).toHaveAttribute("placeholder", "Defaults to 1.0"); }); it("should send custom AzureML params when fields are modified", async () => { @@ -860,6 +1098,356 @@ describe("CreateTargetDialog", () => { }); }); + it("should render and submit OpenAI numeric parameters from target metadata", async () => { + const user = userEvent.setup(); + mockedTargetsApi.createTarget.mockResolvedValue(makeTarget({ + target_registry_name: "openai_chat_custom", + target_type: "OpenAIChatTarget", + })); + + render( + + + + ); + + await selectTargetType("OpenAIChatTarget"); + await user.click(screen.getByText("Advanced settings")); + + fireEvent.change(screen.getByPlaceholderText("https://your-resource.openai.azure.com/"), { + target: { value: "https://api.openai.com" }, + }); + fireEvent.change(screen.getByLabelText("Temperature"), { + target: { value: "0" }, + }); + fireEvent.change(screen.getByLabelText("Seed"), { + target: { value: "42" }, + }); + + expect(screen.getByLabelText("Extra Body Parameters")).toBeVisible(); + + await user.click(screen.getByText("Create Target")); + + await waitFor(() => { + expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + type: "OpenAIChatTarget", + params: { + endpoint: "https://api.openai.com", + temperature: 0, + seed: 42, + }, + }); + }); + }); + + it("should render and submit metadata choices and booleans", async () => { + const user = userEvent.setup(); + mockedTargetsApi.createTarget.mockResolvedValue(makeTarget({ + target_registry_name: "openai_response_custom", + target_type: "OpenAIResponseTarget", + })); + + render( + + + + ); + + await selectTargetType("OpenAIResponseTarget"); + await user.click(screen.getByText("Advanced settings")); + + fireEvent.change(screen.getByPlaceholderText("https://your-resource.openai.azure.com/"), { + target: { value: "https://api.openai.com" }, + }); + await user.selectOptions(screen.getByLabelText("Reasoning Effort"), "high"); + const failOnMissingFunction = screen.getByLabelText("Fail On Missing Function"); + expect(within(failOnMissingFunction).getByRole("option", { + name: "Use default (False)", + })).toBeInTheDocument(); + await user.selectOptions(failOnMissingFunction, "false"); + + expect(screen.queryByLabelText("Custom Functions")).not.toBeInTheDocument(); + + await user.click(screen.getByText("Create Target")); + + await waitFor(() => { + expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + type: "OpenAIResponseTarget", + params: { + endpoint: "https://api.openai.com", + reasoning_effort: "high", + fail_on_missing_function: false, + }, + }); + }); + }); + + it("should render and submit scalar list parameters from target metadata", async () => { + const user = userEvent.setup(); + mockedTargetsApi.createTarget.mockResolvedValue(makeTarget({ + target_registry_name: "openai_completion_custom", + target_type: "OpenAICompletionTarget", + })); + + render( + + + + ); + + await selectTargetType("OpenAICompletionTarget"); + await user.click(screen.getByText("Advanced settings")); + + fireEvent.change(screen.getByPlaceholderText("https://your-resource.openai.azure.com/"), { + target: { value: "https://api.openai.com" }, + }); + fireEvent.change(screen.getByLabelText("Stop"), { + target: { value: "END, DONE" }, + }); + + await user.click(screen.getByText("Create Target")); + + await waitFor(() => { + expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + type: "OpenAICompletionTarget", + params: { + endpoint: "https://api.openai.com", + stop: ["END", "DONE"], + }, + }); + }); + }); + + it("should preserve an explicitly selected empty list", async () => { + const user = userEvent.setup(); + mockedTargetsApi.createTarget.mockResolvedValue(makeTarget({ + target_registry_name: "openai_completion_empty_stop", + target_type: "OpenAICompletionTarget", + })); + + render( + + + + ); + + await selectTargetType("OpenAICompletionTarget"); + fireEvent.change(screen.getByPlaceholderText("https://your-resource.openai.azure.com/"), { + target: { value: "https://api.openai.com" }, + }); + await user.click(screen.getByText("Advanced settings")); + await user.click(screen.getByRole("checkbox", { name: "Use empty list for stop" })); + await user.click(screen.getByText("Create Target")); + + await waitFor(() => { + expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + type: "OpenAICompletionTarget", + params: { + endpoint: "https://api.openai.com", + stop: [], + }, + }); + }); + }); + + it("should parse and submit JSON-object target parameters", async () => { + const user = userEvent.setup(); + mockedTargetsApi.createTarget.mockResolvedValue(makeTarget({ + target_registry_name: "openai_chat_json", + target_type: "OpenAIChatTarget", + })); + + render( + + + + ); + + await selectTargetType("OpenAIChatTarget"); + fireEvent.change(screen.getByPlaceholderText("https://your-resource.openai.azure.com/"), { + target: { value: "https://api.openai.com" }, + }); + await user.click(screen.getByText("Advanced settings")); + + fireEvent.change(screen.getByLabelText("Extra Body Parameters"), { + target: { value: '{"reasoning":{"effort":"high"},"include":["usage"]}' }, + }); + fireEvent.change(screen.getByLabelText("Httpx Client Kwargs"), { + target: { value: '{"timeout":180}' }, + }); + + await user.click(screen.getByText("Create Target")); + + await waitFor(() => { + expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + type: "OpenAIChatTarget", + params: { + endpoint: "https://api.openai.com", + extra_body_parameters: { + reasoning: { effort: "high" }, + include: ["usage"], + }, + httpx_client_kwargs: { timeout: 180 }, + }, + }); + }); + }); + + it("should reject malformed or non-object JSON target parameters", async () => { + const user = userEvent.setup(); + + render( + + + + ); + + await selectTargetType("OpenAIChatTarget"); + await user.click(screen.getByText("Advanced settings")); + + const extraBody = screen.getByLabelText("Extra Body Parameters"); + fireEvent.change(extraBody, { + target: { value: '{"reasoning":' }, + }); + expect(screen.getByText("Extra Body Parameters must contain valid JSON.")).toBeVisible(); + + fireEvent.change(extraBody, { + target: { value: '["reasoning"]' }, + }); + expect(screen.getByText("Extra Body Parameters must be a JSON object.")).toBeVisible(); + + await user.click(screen.getByText("Create Target")); + + expect(mockedTargetsApi.createTarget).not.toHaveBeenCalled(); + }); + + it("should keep optional parameters in expandable advanced settings", async () => { + const user = userEvent.setup(); + + render( + + + + ); + + await selectTargetType("OpenAIResponseTarget"); + + const summary = screen.getByText("Advanced settings"); + const advancedSettings = summary.closest("details"); + expect(advancedSettings).not.toHaveAttribute("open"); + expect(screen.getByLabelText("Reasoning Effort")).not.toBeVisible(); + + await user.click(summary); + + expect(advancedSettings).toHaveAttribute("open"); + expect(screen.getByLabelText("Reasoning Effort")).toBeVisible(); + expect(screen.getByText(/Python callables cannot be configured in CopyRIT/)).toBeVisible(); + }); + + it("should create a metadata-supported target without a hardcoded form shape", async () => { + const user = userEvent.setup(); + mockedTargetsApi.createTarget.mockResolvedValue(makeTarget({ + target_registry_name: "raw_http_target", + target_type: "HTTPTarget", + })); + + render( + + + + ); + + await selectTargetType("HTTPTarget"); + + expect(screen.queryByLabelText("Endpoint URL")).not.toBeInTheDocument(); + expect(screen.queryByPlaceholderText("API key (stored in memory only)")).not.toBeInTheDocument(); + expect(screen.getByText("Create Target").closest("button")).toBeDisabled(); + + // A real raw HTTP request template needs actual line breaks between the + // request line, headers, and body — the backend parser splits on real + // newlines, not on literal "\r\n" characters. The http_request field + // must render as a multiline control (Textarea) for this to be entered. + const httpRequestTemplate = "POST /chat HTTP/1.1\nHost: example.com\nContent-Type: application/json\n\n{\"message\": \"{PROMPT}\"}"; + const httpRequestField = screen.getByLabelText(/Http Request/); + expect(httpRequestField.tagName).toBe("TEXTAREA"); + fireEvent.change(httpRequestField, { + target: { value: httpRequestTemplate }, + }); + + expect(screen.getByText("Create Target").closest("button")).toBeEnabled(); + await user.click(screen.getByText("Create Target")); + + await waitFor(() => { + expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + type: "HTTPTarget", + params: { + http_request: httpRequestTemplate, + }, + }); + }); + }); + + it("should support identity auth for targets without an endpoint or api_key parameter", async () => { + const user = userEvent.setup(); + + render( + + + + ); + + await selectTargetType("AzureBlobStorageTarget"); + + expect(screen.queryByLabelText("Endpoint URL")).not.toBeInTheDocument(); + expect(screen.queryByPlaceholderText("API key (stored in memory only)")).not.toBeInTheDocument(); + expect(screen.getByRole("radio", { name: /Identity-based/ })).toBeInTheDocument(); + + await user.click(screen.getByRole("radio", { name: /Identity-based/ })); + + expect(screen.getByText("Create Target").closest("button")).toBeEnabled(); + }); + + it("should mask the sas_token parameter and drop it once identity auth is selected", async () => { + const user = userEvent.setup(); + mockedTargetsApi.createTarget.mockResolvedValue(makeTarget({ + target_registry_name: "blob_target", + target_type: "AzureBlobStorageTarget", + })); + + render( + + + + ); + + await selectTargetType("AzureBlobStorageTarget"); + await user.click(screen.getByText("Advanced settings")); + + const sasTokenField = screen.getByLabelText(/Sas Token/); + expect(sasTokenField).toHaveAttribute("type", "password"); + + fireEvent.change(sasTokenField, { target: { value: "sv=2024&sig=secret" } }); + expect(sasTokenField).toHaveValue("sv=2024&sig=secret"); + + // Selecting identity-based auth must clear and disable sas_token so it + // can't silently override the chosen auth mode (SAS takes precedence + // over DefaultAzureCredential on the backend). + await user.click(screen.getByRole("radio", { name: /Identity-based/ })); + + expect(sasTokenField).toBeDisabled(); + expect(sasTokenField).toHaveValue(""); + + await user.click(screen.getByText("Create Target")); + + await waitFor(() => { + expect(mockedTargetsApi.createTarget).toHaveBeenCalledWith({ + type: "AzureBlobStorageTarget", + params: {}, + auth_mode: "identity", + }); + }); + }); + it("should reset form when dialog is closed via onOpenChange", () => { const onClose = jest.fn(); diff --git a/frontend/src/components/Config/CreateTargetDialog.tsx b/frontend/src/components/Config/CreateTargetDialog.tsx index db85659686..bc3e717f6c 100644 --- a/frontend/src/components/Config/CreateTargetDialog.tsx +++ b/frontend/src/components/Config/CreateTargetDialog.tsx @@ -25,6 +25,12 @@ import { Tooltip, } from '@fluentui/react-components' import { DeleteRegular } from '@fluentui/react-icons' + +import ParameterField from '@/components/Parameters/ParameterField' +import { + buildParametersFromForm, + type ParameterFormValue, +} from '@/components/Parameters/parameterForm' import { targetsApi } from '@/services/api' import { toApiError } from '@/services/errors' import type { TargetInstance, TargetTypeEntry } from '@/types' @@ -35,34 +41,33 @@ import { targetUnderlyingModelName, } from '@/utils/targetIdentity' import { useCreateTargetDialogStyles } from './CreateTargetDialog.styles' +import { + canConfigureTargetType, + getTargetParameterPolicy, + isMetadataDrivenTargetParameter, +} from './targetParameterPolicy' import { MAX_WEIGHT, parseWeight } from './weightValidation' -/** - * Form shape for each target type the dialog knows how to render. - * - * The dialog renders bespoke, type-specific forms (endpoint/model for OpenAI, - * extra sampling params for Azure ML, an inner-target picker for RoundRobin), - * so this map declares *which* types are renderable and *how*. The list of - * available types and their auth flags come from TargetRegistry metadata - * (`/targets/types`); this map only governs the form layout. Types the - * backend offers but that aren't in this map are simply not shown, and types in - * this map that the backend doesn't offer fall back to being listed anyway - * (e.g. when the type metadata fetch fails). - */ -type TargetFormShape = 'openai' | 'azureml' | 'roundrobin' - -const TARGET_FORM_SHAPES: Record = { - OpenAIChatTarget: 'openai', - OpenAICompletionTarget: 'openai', - OpenAIImageTarget: 'openai', - OpenAIVideoTarget: 'openai', - OpenAITTSTarget: 'openai', - OpenAIResponseTarget: 'openai', - AzureMLChatTarget: 'azureml', - RoundRobinTarget: 'roundrobin', -} +const FALLBACK_TARGET_TYPES = [ + 'OpenAIChatTarget', + 'OpenAICompletionTarget', + 'OpenAIImageTarget', + 'OpenAIVideoTarget', + 'OpenAITTSTarget', + 'OpenAIResponseTarget', + 'AzureMLChatTarget', + 'RoundRobinTarget', +] -const RENDERABLE_TARGET_TYPES = Object.keys(TARGET_FORM_SHAPES) +const FALLBACK_IDENTITY_TARGET_TYPES = new Set([ + 'OpenAIChatTarget', + 'OpenAICompletionTarget', + 'OpenAIImageTarget', + 'OpenAIVideoTarget', + 'OpenAITTSTarget', + 'OpenAIResponseTarget', + 'AzureMLChatTarget', +]) const TARGET_DISPLAY_NAMES: Record = { AzureMLChatTarget: 'Azure Machine Learning chat', @@ -75,7 +80,7 @@ const TARGET_DISPLAY_NAMES: Record = { RoundRobinTarget: 'Weighted round robin', } -const FALLBACK_TARGET_TYPE_ENTRIES: TargetTypeEntry[] = RENDERABLE_TARGET_TYPES.map((targetType) => ({ +const FALLBACK_TARGET_TYPE_ENTRIES: TargetTypeEntry[] = FALLBACK_TARGET_TYPES.map((targetType) => ({ target_type: targetType, parameters: [], supported_auth_modes: [], @@ -99,12 +104,27 @@ function getAuthDescription(authModes: TargetTypeEntry['supported_auth_modes']): } /** - * Fallback for whether a target type supports identity-based auth when the - * registry metadata hasn't loaded (or the fetch failed). Once the metadata is - * available it is authoritative; this only keeps the form usable offline / mid-load. + * Fallback for identity-based auth while registry metadata is unavailable. */ -function defaultSupportsIdentity(shape: TargetFormShape | undefined): boolean { - return shape === 'openai' || shape === 'azureml' +function defaultSupportsIdentity(targetType: string): boolean { + return FALLBACK_IDENTITY_TARGET_TYPES.has(targetType) +} + +function getParameterLabel(name: string): string { + return name + .split('_') + .map((word) => word.length === 1 ? word.toUpperCase() : `${word[0].toUpperCase()}${word.slice(1)}`) + .join(' ') +} + +function isParameterValueSet(value: ParameterFormValue | undefined): boolean { + if (typeof value === 'string') { + return value.trim().length > 0 + } + if (Array.isArray(value)) { + return value.length > 0 + } + return value !== undefined && value.type.length > 0 } // Mirrors backend's hostname-suffix check (list in target_service.py). @@ -207,13 +227,16 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT const [underlyingModel, setUnderlyingModel] = useState('') const [authMode, setAuthMode] = useState('api_key') const [apiKey, setApiKey] = useState('') - const [maxNewTokens, setMaxNewTokens] = useState('400') - const [temperature, setTemperature] = useState('1.0') - const [topP, setTopP] = useState('1.0') - const [repetitionPenalty, setRepetitionPenalty] = useState('1.0') + const [parameterValues, setParameterValues] = useState>({}) const [submitting, setSubmitting] = useState(false) const [error, setError] = useState(null) - const [fieldErrors, setFieldErrors] = useState<{ targetType?: string; endpoint?: string }>({}) + const [fieldErrors, setFieldErrors] = useState<{ + targetType?: string + endpoint?: string + modelName?: string + underlyingModel?: string + apiKey?: string + }>({}) // --- RoundRobin-specific state --- // The list of targets available for selection (fetched once when dialog opens). @@ -265,7 +288,7 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT }, [open]) const registeredTargetTypeOptions = useMemo(() => { - return targetTypeEntries.filter((entry) => entry.target_type in TARGET_FORM_SHAPES) + return targetTypeEntries.filter(canConfigureTargetType) }, [targetTypeEntries]) const typeMetadataAvailable = registeredTargetTypeOptions.length > 0 const typeMetadataUnavailable = typeMetadataStatus !== 'loading' && !typeMetadataAvailable @@ -273,20 +296,54 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT ? registeredTargetTypeOptions : FALLBACK_TARGET_TYPE_ENTRIES - const formShape = TARGET_FORM_SHAPES[targetType] - const isRoundRobin = formShape === 'roundrobin' - const isAzureML = formShape === 'azureml' - const isOpenAi = formShape === 'openai' + const isRoundRobin = targetType === 'RoundRobinTarget' + const isAzureML = targetType === 'AzureMLChatTarget' + const isOpenAi = targetType.startsWith('OpenAI') || targetType === 'RealtimeTarget' const targetTypeEntry = targetTypeByName.get(targetType) + const metadataDrivenParameters = useMemo( + () => targetTypeEntry?.parameters.filter( + (parameter) => isMetadataDrivenTargetParameter(targetType, parameter), + ) ?? [], + [targetType, targetTypeEntry], + ) + const requiredMetadataParameters = metadataDrivenParameters.filter((parameter) => parameter.required) + const optionalMetadataParameters = metadataDrivenParameters.filter((parameter) => !parameter.required) + const requiredMetadataParameterMissing = requiredMetadataParameters.some( + (parameter) => !isParameterValueSet(parameterValues[parameter.name]), + ) + const parameterByName = useMemo( + () => new Map(targetTypeEntry?.parameters.map((parameter) => [parameter.name, parameter]) ?? []), + [targetTypeEntry], + ) + const endpointParameter = parameterByName.get('endpoint') + const modelNameParameter = parameterByName.get('model_name') + const underlyingModelParameter = parameterByName.get('underlying_model') + ?? parameterByName.get('underlying_model_name') + const apiKeyParameter = parameterByName.get('api_key') + const customFunctionsParameter = parameterByName.get('custom_functions') + const customFunctionsReason = customFunctionsParameter + ? getTargetParameterPolicy(targetType, customFunctionsParameter.name)?.reason + : null + const metadataUnavailableForSelection = targetType !== '' && !targetTypeEntry + const hasField = (name: string) => parameterByName.has(name) + || (metadataUnavailableForSelection && !isRoundRobin) + const hasEndpointField = hasField('endpoint') + const hasModelNameField = hasField('model_name') + const hasUnderlyingModelField = hasField('underlying_model') || hasField('underlying_model_name') + const hasApiKeyField = hasField('api_key') const selectedTargetDisplayName = getTargetDisplayName(targetType) const selectedTargetAuthDescription = targetTypeEntry ? getAuthDescription(targetTypeEntry.supported_auth_modes) : null const supportsIdentity = targetTypeEntry ? targetTypeEntry.supported_auth_modes.includes('identity') - : defaultSupportsIdentity(formShape) + : defaultSupportsIdentity(targetType) const showAuthField = targetType !== '' && supportsIdentity const isIdentity = showAuthField && authMode === 'identity' + const endpointRequired = hasEndpointField + const modelNameRequired = Boolean(modelNameParameter?.required) + const underlyingModelRequired = Boolean(underlyingModelParameter?.required) + const apiKeyRequired = Boolean(apiKeyParameter?.required) && !isIdentity const identityEndpointError: string | null = (() => { if (!isIdentity || endpoint === '') return null if (isOpenAi && !isAzureOpenAiEndpoint(endpoint)) { @@ -382,10 +439,7 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT setUnderlyingModel('') setAuthMode('api_key') setApiKey('') - setMaxNewTokens('400') - setTemperature('1.0') - setTopP('1.0') - setRepetitionPenalty('1.0') + setParameterValues({}) setError(null) setFieldErrors({}) setSelectedInnerTargets([]) @@ -439,36 +493,55 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT return } - const errors: { targetType?: string; endpoint?: string } = {} + const errors: { + targetType?: string + endpoint?: string + modelName?: string + underlyingModel?: string + apiKey?: string + } = {} if (!targetType) errors.targetType = 'Please select a target type' - if (!endpoint) errors.endpoint = 'Please provide an endpoint URL' + if (endpointRequired && !endpoint) errors.endpoint = 'Please provide an endpoint URL' + if (modelNameRequired && !modelName) errors.modelName = 'Please provide a model name' + if (underlyingModelRequired && !underlyingModel) { + errors.underlyingModel = 'Please provide the underlying model' + } + if (apiKeyRequired && !apiKey) errors.apiKey = 'Please provide an API key' if (Object.keys(errors).length > 0) { setFieldErrors(errors) return } setFieldErrors({}) + const metadataParams = buildParametersFromForm(metadataDrivenParameters, parameterValues) + if (!metadataParams.ok) { + setError(metadataParams.error) + return + } + setSubmitting(true) setError(null) try { - const params: Record = { - endpoint, + const params: Record = { ...(metadataParams.parameters ?? {}) } + // Identity-based auth must be enforceable: strip any metadata-driven + // parameter (e.g. AzureBlobStorageTarget's sas_token) that would let the + // backend authenticate a different way than the selected identity. + if (isIdentity) { + for (const parameter of metadataDrivenParameters) { + if (parameter.identity_conflicting) delete params[parameter.name] + } } - if (modelName) params.model_name = modelName - if (!isIdentity && apiKey) params.api_key = apiKey - - if (hasDifferentUnderlying && underlyingModel) params.underlying_model = underlyingModel - - if (isAzureML) { - const parsedMaxNewTokens = parseInt(maxNewTokens, 10) - if (!isNaN(parsedMaxNewTokens)) params.max_new_tokens = parsedMaxNewTokens - const parsedTemperature = parseFloat(temperature) - if (!isNaN(parsedTemperature)) params.temperature = parsedTemperature - const parsedTopP = parseFloat(topP) - if (!isNaN(parsedTopP)) params.top_p = parsedTopP - const parsedRepetitionPenalty = parseFloat(repetitionPenalty) - if (!isNaN(parsedRepetitionPenalty)) params.repetition_penalty = parsedRepetitionPenalty + if (hasEndpointField && endpoint) params.endpoint = endpoint + if (hasModelNameField && modelName) params.model_name = modelName + if (hasApiKeyField && !isIdentity && apiKey) params.api_key = apiKey + + if ( + (underlyingModelRequired || hasDifferentUnderlying) + && underlyingModel + && hasUnderlyingModelField + ) { + params[underlyingModelParameter?.name ?? 'underlying_model'] = underlyingModel } await targetsApi.createTarget({ @@ -538,10 +611,11 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT const next = data.optionValue if (!next) return setTargetType(next) + setParameterValues({}) const nextEntry = targetTypeByName.get(next) const nextSupportsIdentity = nextEntry ? nextEntry.supported_auth_modes.includes('identity') - : defaultSupportsIdentity(TARGET_FORM_SHAPES[next]) + : defaultSupportsIdentity(next) if (!nextSupportsIdentity) { setAuthMode('api_key') } @@ -722,129 +796,194 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT {/* === Standard target form fields (hidden for RoundRobin) === */} {!isRoundRobin && ( <> - - setEndpoint(data.value)} - /> - - - - setModelName(data.value)} - /> - - -
- { - setHasDifferentUnderlying(data.checked) - if (!data.checked) setUnderlyingModel('') - }} - label="Underlying model differs from deployment name" - /> - - On Azure, the deployment name (e.g. my-gpt4-deployment) may differ from the actual model (e.g. gpt-4o). - -
- - {hasDifferentUnderlying && ( - - setUnderlyingModel(data.value)} - /> - - )} + {hasEndpointField && ( + + setEndpoint(data.value)} + /> + + )} - {isAzureML && ( - <> - - setMaxNewTokens(data.value)} - /> - + {hasModelNameField && ( + + setModelName(data.value)} + /> + + )} - - setTemperature(data.value)} - /> - + {hasUnderlyingModelField && !underlyingModelRequired && ( +
+ { + setHasDifferentUnderlying(data.checked) + if (!data.checked) setUnderlyingModel('') + }} + label="Underlying model differs from deployment name" + /> + + On Azure, the deployment name may differ from the actual model. + +
+ )} - - setTopP(data.value)} - /> - + {hasUnderlyingModelField && (underlyingModelRequired || hasDifferentUnderlying) && ( + + setUnderlyingModel(data.value)} + /> + + )} - - setRepetitionPenalty(data.value)} - /> - - - )} + {requiredMetadataParameters.map((parameter) => { + const identityConflict = isIdentity && Boolean(parameter.identity_conflicting) + return ( + setParameterValues((current) => ({ + ...current, + [name]: value, + }))} + /> + ) + })} - {showAuthField && ( - - { - const next = data.value as AuthMode - setAuthMode(next) - if (next === 'identity') setApiKey('') - }} - > - - - - - )} + {showAuthField && ( + + { + const next = data.value as AuthMode + setAuthMode(next) + if (next === 'identity') { + setApiKey('') + setParameterValues((current) => { + const cleared = { ...current } + for (const parameter of metadataDrivenParameters) { + if (parameter.identity_conflicting) delete cleared[parameter.name] + } + return cleared + }) + } + }} + > + + + + + )} - {showIdentityEndpointError && ( - - - {identityEndpointError} - - - )} + {showIdentityEndpointError && ( + + + {identityEndpointError} + + + )} - {!isIdentity && ( - - setApiKey(data.value)} - /> - - )} + {hasApiKeyField && !isIdentity && ( + + setApiKey(data.value)} + /> + + )} - {/* Close the !isRoundRobin conditional wrapper */} + {(optionalMetadataParameters.length > 0 || customFunctionsReason) && ( +
+ + Advanced settings + +
+ {optionalMetadataParameters.map((parameter) => { + const identityConflict = isIdentity && Boolean(parameter.identity_conflicting) + return ( + setParameterValues((current) => ({ + ...current, + [name]: value, + }))} + /> + ) + })} + {customFunctionsReason && ( + + + Custom Functions: {customFunctionsReason} + + + )} +
+
+ )} )} @@ -878,7 +1017,11 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT (isRoundRobin ? selectedInnerTargets.length < 2 || selectedInnerTargets.some((t) => !parseWeight(t.weightInput).ok) - : !endpoint || showIdentityEndpointError) + : (endpointRequired && !endpoint) || showIdentityEndpointError) + || (modelNameRequired && !modelName) + || (underlyingModelRequired && !underlyingModel) + || (apiKeyRequired && !apiKey) + || requiredMetadataParameterMissing } > {submitting ? 'Creating...' : 'Create Target'} diff --git a/frontend/src/components/Config/targetParameterPolicy.test.ts b/frontend/src/components/Config/targetParameterPolicy.test.ts new file mode 100644 index 0000000000..1b9ac0ae5f --- /dev/null +++ b/frontend/src/components/Config/targetParameterPolicy.test.ts @@ -0,0 +1,125 @@ +import type { Parameter, TargetTypeEntry } from '@/types' + +import { + canConfigureRequiredTargetParameter, + canConfigureTargetType, + getTargetParameterPolicy, + isMetadataDrivenTargetParameter, +} from './targetParameterPolicy' + +function makeParameter(overrides: Partial & { name: string }): Parameter { + return { + type_name: 'str', + required: false, + default: null, + choices: null, + is_list: false, + variants: null, + reference_type: null, + description: null, + ...overrides, + } +} + +function makeTargetType( + targetType: string, + parameters: Parameter[], +): TargetTypeEntry { + return { + target_type: targetType, + parameters, + supported_auth_modes: ['api_key'], + description: null, + } +} + +describe('target parameter ownership', () => { + it.each([ + ['api_key', 'authentication'], + ['auth_mode', 'authentication'], + ['endpoint', 'connection'], + ['model_name', 'connection'], + ['underlying_model', 'connection'], + ] as const)('assigns %s to the %s UI', (name, owner) => { + expect(getTargetParameterPolicy('OpenAIChatTarget', name)?.owner).toBe(owner) + }) + + it('assigns RoundRobin targets and weights only to the RoundRobin picker', () => { + expect(getTargetParameterPolicy('RoundRobinTarget', 'targets')?.owner).toBe('round_robin') + expect(getTargetParameterPolicy('RoundRobinTarget', 'weights')?.owner).toBe('round_robin') + expect(getTargetParameterPolicy('OpenAIChatTarget', 'weights')).toBeUndefined() + }) + + it('marks custom_functions unsupported with a visible reason', () => { + const policy = getTargetParameterPolicy('OpenAIResponseTarget', 'custom_functions') + expect(policy?.owner).toBe('unsupported') + expect(policy?.reason).toMatch(/Python callables cannot be configured/) + }) +}) + +describe('metadata-driven target parameters', () => { + it.each([ + makeParameter({ name: 'temperature', type_name: 'float' }), + makeParameter({ name: 'seed', type_name: 'int' }), + makeParameter({ name: 'enabled', type_name: 'bool' }), + makeParameter({ name: 'allowed_upload_directory', type_name: 'str | Path | None' }), + makeParameter({ name: 'working_directory', type_name: 'Path | str | None' }), + makeParameter({ name: 'mode', type_name: 'CustomEnum', choices: ['a', 'b'] }), + makeParameter({ name: 'stop', type_name: 'list[str]', is_list: true }), + makeParameter({ name: 'extra_body_parameters', type_name: 'dict[str, typing.Any]' }), + ])('renders $name from metadata', (parameter) => { + expect(isMetadataDrivenTargetParameter('OpenAIChatTarget', parameter)).toBe(true) + }) + + it.each([ + makeParameter({ name: 'callback', type_name: 'Callable' }), + makeParameter({ name: 'target', reference_type: 'target' }), + makeParameter({ + name: 'strategy', + variants: { fixed: [makeParameter({ name: 'count', type_name: 'int' })] }, + }), + makeParameter({ name: 'mixed_scalar', type_name: 'str | bytes | None' }), + makeParameter({ name: 'mixed', type_name: 'list[str | bytes]', is_list: true }), + ])('does not claim unsupported parameter $name is renderable', (parameter) => { + expect(isMetadataDrivenTargetParameter('ExampleTarget', parameter)).toBe(false) + }) +}) + +describe('target type configurability', () => { + it('accepts required metadata and specially owned parameters', () => { + const entry = makeTargetType('RoundRobinTarget', [ + makeParameter({ + name: 'targets', + type_name: 'list[str]', + is_list: true, + required: true, + reference_type: 'target', + }), + makeParameter({ name: 'weights', type_name: 'list[int]', is_list: true }), + ]) + expect(canConfigureTargetType(entry)).toBe(true) + }) + + it('rejects a target with an unsupported required Python object', () => { + const page = makeParameter({ + name: 'page', + type_name: 'Page', + required: true, + }) + expect(canConfigureRequiredTargetParameter('PlaywrightTarget', page)).toBe(false) + expect(canConfigureTargetType(makeTargetType('PlaywrightTarget', [page]))).toBe(false) + }) + + it('allows unsupported optional parameters without claiming to render them', () => { + const callback = makeParameter({ + name: 'callback', + type_name: 'Callable', + required: false, + }) + expect(canConfigureTargetType(makeTargetType('HTTPTarget', [ + makeParameter({ name: 'http_request', required: true }), + callback, + ]))).toBe(true) + expect(isMetadataDrivenTargetParameter('HTTPTarget', callback)).toBe(false) + }) +}) diff --git a/frontend/src/components/Config/targetParameterPolicy.ts b/frontend/src/components/Config/targetParameterPolicy.ts new file mode 100644 index 0000000000..b5622ba292 --- /dev/null +++ b/frontend/src/components/Config/targetParameterPolicy.ts @@ -0,0 +1,69 @@ +import { isJsonObjectParameter } from '@/components/Parameters/parameterForm' +import type { Parameter, TargetTypeEntry } from '@/types' + +export type TargetParameterOwner = 'authentication' | 'connection' | 'round_robin' | 'unsupported' + +export interface TargetParameterPolicy { + readonly owner: TargetParameterOwner + readonly reason?: string +} + +const TARGET_PARAMETER_POLICIES: Record = { + api_key: { owner: 'authentication' }, + auth_mode: { owner: 'authentication' }, + endpoint: { owner: 'connection' }, + model_name: { owner: 'connection' }, + underlying_model: { owner: 'connection' }, + underlying_model_name: { owner: 'connection' }, + custom_functions: { + owner: 'unsupported', + reason: 'Python callables cannot be configured in CopyRIT. Configure this target through the Python framework to use custom functions.', + }, +} + +const SCALAR_PARAMETER_TYPES = new Set(['Path', 'bool', 'float', 'int', 'str']) + +export function getTargetParameterPolicy( + targetType: string, + parameterName: string, +): TargetParameterPolicy | undefined { + if (targetType === 'RoundRobinTarget' && (parameterName === 'targets' || parameterName === 'weights')) { + return { owner: 'round_robin' } + } + return TARGET_PARAMETER_POLICIES[parameterName] +} + +export function isMetadataDrivenTargetParameter(targetType: string, parameter: Parameter): boolean { + if ( + getTargetParameterPolicy(targetType, parameter.name) + || parameter.reference_type + || parameter.variants + ) { + return false + } + if (parameter.choices?.length || isJsonObjectParameter(parameter)) { + return true + } + if (parameter.is_list) { + const elementType = /^list\[(.+)\]$/.exec(parameter.type_name)?.[1] + return elementType !== undefined && SCALAR_PARAMETER_TYPES.has(elementType) + } + return parameter.type_name + .split('|') + .map((part) => part.trim()) + .filter((part) => part !== 'None') + .every((part) => SCALAR_PARAMETER_TYPES.has(part)) +} + +export function canConfigureRequiredTargetParameter(targetType: string, parameter: Parameter): boolean { + const policy = getTargetParameterPolicy(targetType, parameter.name) + return (policy !== undefined && policy.owner !== 'unsupported') + || isMetadataDrivenTargetParameter(targetType, parameter) +} + +export function canConfigureTargetType(entry: TargetTypeEntry): boolean { + return entry.parameters.every( + (parameter) => !parameter.required + || canConfigureRequiredTargetParameter(entry.target_type, parameter), + ) +} diff --git a/frontend/src/components/Initializers/CustomInitializers.test.tsx b/frontend/src/components/Initializers/CustomInitializers.test.tsx index 48151b8ee4..63a1613d4d 100644 --- a/frontend/src/components/Initializers/CustomInitializers.test.tsx +++ b/frontend/src/components/Initializers/CustomInitializers.test.tsx @@ -53,7 +53,7 @@ describe('CustomInitializers', () => { await user.click(within(dialog).getByRole('button', { name: 'Remove' })) expect(defaultProps.onDelete).toHaveBeenCalledWith('custom_target') - expect(screen.getByRole('button', { name: 'second_target' })).toHaveAttribute('aria-current', 'page') + expect(await screen.findByRole('button', { name: 'second_target' })).toHaveAttribute('aria-current', 'page') }) it('should display stored source as read-only', () => { diff --git a/frontend/src/components/Parameters/ParameterField.styles.ts b/frontend/src/components/Parameters/ParameterField.styles.ts index 269a94772b..728ce29a31 100644 --- a/frontend/src/components/Parameters/ParameterField.styles.ts +++ b/frontend/src/components/Parameters/ParameterField.styles.ts @@ -19,6 +19,13 @@ export const useParameterFieldStyles = makeStyles({ minHeight: MINIMUM_TOUCH_TARGET_SIZE, }, }, + '& > textarea': { + minHeight: '6rem', + fontFamily: tokens.fontFamilyMonospace, + [TOUCH_INPUT_QUERY]: { + minHeight: '6rem', + }, + }, }, selectionControl: { ...mobileTouchTargetHeight, diff --git a/frontend/src/components/Parameters/ParameterField.tsx b/frontend/src/components/Parameters/ParameterField.tsx index b6c646d95f..74061b2b6c 100644 --- a/frontend/src/components/Parameters/ParameterField.tsx +++ b/frontend/src/components/Parameters/ParameterField.tsx @@ -3,6 +3,7 @@ import { Field, Input, Select, + Textarea, } from '@fluentui/react-components' import type { Parameter } from '@/types' @@ -12,6 +13,7 @@ import { getInitialFormValues, getParameterControlKind, isStructuredParameterFormValue, + parseJsonObjectFormValue, type ParameterFormValue, } from './parameterForm' @@ -20,12 +22,18 @@ export interface ParameterFieldProps { value: ParameterFormValue disabled: boolean onChange: (name: string, value: ParameterFormValue) => void + /** Optional presentation label when the parameter name should remain unchanged for submission. */ + label?: string + /** Present the declared default as guidance while keeping the field unset. */ + showDefaultHint?: boolean /** Let a list field distinguish an explicit empty list from an omitted value. */ allowEmptyList?: boolean /** Show required validation for a structured input whose variant is unset. */ showRequiredError?: boolean /** Prefix for `data-testid` attributes. Defaults to `'param'` (e.g. `param-`). */ testIdPrefix?: string + /** Extra guidance appended to the field's hint, e.g. explaining why it's disabled. */ + extraHint?: string } /** @@ -43,14 +51,29 @@ export default function ParameterField({ value, disabled, onChange, + label: labelOverride, + showDefaultHint = false, allowEmptyList = false, showRequiredError = false, testIdPrefix = 'param', + extraHint, }: ParameterFieldProps) { const styles = useParameterFieldStyles() const kind = getParameterControlKind(parameter) - const label = parameter.required ? `${parameter.name} *` : parameter.name + const label = parameter.required + ? `${labelOverride ?? parameter.name} *` + : labelOverride ?? parameter.name const testId = `${testIdPrefix}-${parameter.name}` + const defaultText = Array.isArray(parameter.default) + ? parameter.default.join(', ') + : parameter.default + const hasDisplayDefault = defaultText != null && defaultText !== '' + const defaultHint = showDefaultHint && hasDisplayDefault + ? `Defaults to ${defaultText}.` + : null + const descriptiveHint = [parameter.description, defaultHint, extraHint] + .filter((part): part is string => Boolean(part)) + .join(' ') if (kind === 'structured') { const current = isStructuredParameterFormValue(value) ? value : { type: '', values: {} } @@ -58,7 +81,7 @@ export default function ParameterField({ <>