From 3587bbc87943099c4237621e810b79ab9b303e86 Mon Sep 17 00:00:00 2001 From: jbolor21 <86250273+jbolor21@users.noreply.github.com> Date: Thu, 24 Sep 2026 19:00:48 -0700 Subject: [PATCH 01/17] adding initial custom fields for targets --- frontend/e2e/config.spec.ts | 26 +- frontend/e2e/numeric-controls.spec.ts | 35 +- .../Config/CreateTargetDialog.styles.ts | 19 + .../Config/CreateTargetDialog.test.tsx | 581 +++++++++++++++++- .../components/Config/CreateTargetDialog.tsx | 481 +++++++++------ .../Parameters/ParameterField.styles.ts | 7 + .../components/Parameters/ParameterField.tsx | 77 ++- .../Parameters/parameterForm.test.ts | 55 ++ tests/unit/backend/test_target_service.py | 50 ++ 9 files changed, 1093 insertions(+), 238 deletions(-) diff --git a/frontend/e2e/config.spec.ts b/frontend/e2e/config.spec.ts index 4c1670e1a9..feccb6467a 100644 --- a/frontend/e2e/config.spec.ts +++ b/frontend/e2e/config.spec.ts @@ -398,7 +398,7 @@ test.describe("Create Target Dialog", () => { await expect(page.getByText("OpenAIChatTarget")).toBeVisible(); }); - test("should show validation errors for empty required fields", async ({ page }) => { + test("should require an endpoint when identity authentication is selected", async ({ page }) => { await page.route(/\/api\/targets/, async (route) => { await route.fulfill(mockTargetsList([])); }); @@ -409,25 +409,27 @@ 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" ); + + // API-key mode can use the target's configured environment defaults. + await expect(createBtn).toBeEnabled(); + + // Identity authentication needs an explicit endpoint so the target can + // validate the Azure host and request the correct credential. + 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 bfc784347f..48e1c17e63 100644 --- a/frontend/e2e/numeric-controls.spec.ts +++ b/frontend/e2e/numeric-controls.spec.ts @@ -46,11 +46,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, @@ -164,22 +176,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..780690c619 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,62 @@ 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, + 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, + 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 +291,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 +488,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 +506,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 +644,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 +907,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 +916,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 +955,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 +984,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 +993,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 +1015,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 +1095,300 @@ 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"); + 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: { + 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"); + 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: { + 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(); + + fireEvent.change(screen.getByLabelText(/Http Request/), { + target: { value: "POST /chat HTTP/1.1\\r\\n\\r\\n{PROMPT}" }, + }); + + 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: "POST /chat HTTP/1.1\\r\\n\\r\\n{PROMPT}", + }, + }); + }); + }); + + 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 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..a155075dad 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,57 @@ 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 endpointParameter = targetTypeEntry?.parameters.find((parameter) => parameter.name === 'endpoint') + const modelNameParameter = targetTypeEntry?.parameters.find((parameter) => parameter.name === 'model_name') + const underlyingModelParameter = targetTypeEntry?.parameters.find( + (parameter) => parameter.name === 'underlying_model' || parameter.name === 'underlying_model_name', + ) + const apiKeyParameter = targetTypeEntry?.parameters.find((parameter) => parameter.name === 'api_key') + const customFunctionsParameter = targetTypeEntry?.parameters.find( + (parameter) => parameter.name === 'custom_functions', + ) + const customFunctionsReason = customFunctionsParameter + ? getTargetParameterPolicy(targetType, customFunctionsParameter.name)?.reason + : null + const metadataUnavailableForSelection = targetType !== '' && !targetTypeEntry + const hasEndpointField = endpointParameter !== undefined + || (metadataUnavailableForSelection && !isRoundRobin) + const hasModelNameField = modelNameParameter !== undefined + || (metadataUnavailableForSelection && !isRoundRobin) + const hasUnderlyingModelField = underlyingModelParameter !== undefined + || (metadataUnavailableForSelection && !isRoundRobin) + const hasApiKeyField = apiKeyParameter !== undefined + || (metadataUnavailableForSelection && !isRoundRobin) 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 = Boolean(endpointParameter?.required) + || (isIdentity && hasEndpointField) + || (metadataUnavailableForSelection && 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 +442,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 +496,47 @@ 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, - } - 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 + const params: Record = { ...(metadataParams.parameters ?? {}) } + 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 +606,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 +791,173 @@ 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) => ( + 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('') + }} + > + + + + + )} - {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) => ( + setParameterValues((current) => ({ + ...current, + [name]: value, + }))} + /> + ))} + {customFunctionsReason && ( + + + Custom Functions: {customFunctionsReason} + + + )} +
+
+ )} )} @@ -878,7 +991,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/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..5f1d5303fb 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,6 +22,10 @@ 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. */ @@ -43,14 +49,28 @@ export default function ParameterField({ value, disabled, onChange, + label: labelOverride, + showDefaultHint = false, allowEmptyList = false, showRequiredError = false, testIdPrefix = 'param', }: 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] + .filter((part): part is string => Boolean(part)) + .join(' ') if (kind === 'structured') { const current = isStructuredParameterFormValue(value) ? value : { type: '', values: {} } @@ -58,7 +78,7 @@ export default function ParameterField({ <>