From 52438e0930ed32f70a23789f7d23d8b7e79619ba Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Sat, 1 Aug 2026 22:40:13 -0700 Subject: [PATCH 1/2] FIX Clarify target choices in the Add Target picker The Add Target dialog listed the eight selectable targets as bare implementation class names, so the similar OpenAI entries were impossible to tell apart before committing to one. Replace the native select with a Fluent Dropdown whose options show a human-friendly name, the backend catalog description, the implementation identifier as secondary detail, and the supported authentication modes. The same facts stay visible after the list closes in a "Selected target details" region, and the picker now reports catalog loading and catalog failure explicitly instead of silently falling back. All metadata comes from the existing /targets/catalog response; no new capabilities are claimed and no backend change is needed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c45e075-9e85-40b5-8352-fb0bcab7261c --- frontend/e2e/config.spec.ts | 136 ++++++++- .../Config/CreateTargetDialog.styles.ts | 52 ++++ .../Config/CreateTargetDialog.test.tsx | 257 ++++++++++++++---- .../components/Config/CreateTargetDialog.tsx | 178 ++++++++++-- 4 files changed, 548 insertions(+), 75 deletions(-) diff --git a/frontend/e2e/config.spec.ts b/frontend/e2e/config.spec.ts index b7b0eebdd9..e0731918d0 100644 --- a/frontend/e2e/config.spec.ts +++ b/frontend/e2e/config.spec.ts @@ -57,6 +57,57 @@ const RESPONSIVE_VIEWPORTS = [ { name: "desktop", width: 1280, height: 800 }, ] as const; +const TARGET_PICKER_CHOICES = [ + { + targetType: "AzureMLChatTarget", + displayName: "Azure Machine Learning chat", + description: "A prompt target for Azure Machine Learning chat endpoints.", + authModes: ["api_key", "identity"], + }, + { + targetType: "OpenAIChatTarget", + displayName: "OpenAI chat", + description: "Facilitates multimodal (image and text) input and text output generation.", + authModes: ["api_key", "identity"], + }, + { + targetType: "OpenAICompletionTarget", + displayName: "OpenAI text completion", + description: "A prompt target for OpenAI completion endpoints.", + authModes: ["api_key", "identity"], + }, + { + targetType: "OpenAIImageTarget", + displayName: "OpenAI image", + description: "A target for image generation or editing using OpenAI's image models.", + authModes: ["api_key", "identity"], + }, + { + targetType: "OpenAIResponseTarget", + displayName: "OpenAI Responses API", + description: "Enables communication with endpoints that support the OpenAI Response API.", + authModes: ["api_key", "identity"], + }, + { + targetType: "OpenAITTSTarget", + displayName: "OpenAI text to speech", + description: "A prompt target for OpenAI Text-to-Speech (TTS) endpoints.", + authModes: ["api_key", "identity"], + }, + { + targetType: "OpenAIVideoTarget", + displayName: "OpenAI video", + description: "OpenAI Video Target using the OpenAI SDK for video generation.", + authModes: ["api_key", "identity"], + }, + { + targetType: "RoundRobinTarget", + displayName: "Weighted round robin", + description: "A prompt target that distributes requests across multiple inner targets using weighted round-robin selection.", + authModes: ["api_key"], + }, +] as const; + async function routeResponsiveTargetData( page: Page, targets: FlatTarget[] @@ -109,6 +160,19 @@ async function goToConfig(page: Page) { await expect(page.getByText("Target Configuration")).toBeVisible({ timeout: 10000 }); } +async function selectTargetType( + page: Page, + dialog: Locator, + targetType: string +): Promise { + const picker = dialog.getByRole("combobox", { name: "Target Type" }); + await expect(picker).toBeEnabled(); + await picker.click(); + await page.getByRole("option", { + name: new RegExp(`Implementation: ${targetType}`), + }).click(); +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -208,6 +272,66 @@ test.describe("Target Configuration Page", () => { }); test.describe("Create Target Dialog", () => { + test("should make all target choices distinguishable and keyboard navigable", async ({ + page, + }) => { + await page.setViewportSize({ width: 390, height: 844 }); + await page.route(/\/api\/targets\/catalog(?:\?.*)?$/, async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + items: TARGET_PICKER_CHOICES.map((choice) => ({ + target_type: choice.targetType, + parameters: [], + supported_auth_modes: choice.authModes, + description: choice.description, + })), + }), + }); + }); + await page.route(/\/api\/targets(?:\?.*)?$/, async (route) => { + await route.fulfill(mockTargetsList([])); + }); + + await goToConfig(page); + await page.getByRole("button", { name: /new target/i }).click(); + + const dialog = page.getByRole("dialog"); + const picker = dialog.getByRole("combobox", { name: "Target Type" }); + await expect(picker).toBeEnabled(); + await picker.focus(); + await page.keyboard.press("Enter"); + + const listbox = page.getByRole("listbox"); + await expect(listbox).toBeVisible(); + await expect(listbox.getByRole("option")).toHaveCount(8); + for (const choice of TARGET_PICKER_CHOICES) { + const option = listbox.getByRole("option", { + name: new RegExp(`Implementation: ${choice.targetType}`), + }); + await expect(option).toContainText(choice.displayName); + await expect(option).toContainText(choice.targetType); + await expect(option).toContainText(choice.description); + } + + const listboxWidths = await listbox.evaluate((element) => ({ + clientWidth: element.clientWidth, + scrollWidth: element.scrollWidth, + })); + expect(listboxWidths.scrollWidth).toBeLessThanOrEqual(listboxWidths.clientWidth); + + await page.keyboard.press("End"); + await page.keyboard.press("Enter"); + await expect(picker).toContainText("Weighted round robin"); + const selectedDetails = dialog.getByRole("region", { + name: "Selected target details", + }); + await expect(selectedDetails).toContainText("RoundRobinTarget"); + await expect(selectedDetails).toContainText("weighted round-robin selection"); + await expect(selectedDetails).toContainText("Supported authentication: API key"); + }); + test("should create a target through the dialog", async ({ page }) => { let createdTarget: FlatTarget | null = null; @@ -242,7 +366,7 @@ test.describe("Create Target Dialog", () => { const dialog = page.locator('[role="dialog"]'); // Select target type - await dialog.locator("select").selectOption("OpenAIChatTarget"); + await selectTargetType(page, dialog, "OpenAIChatTarget"); // Fill endpoint await dialog.getByPlaceholder("https://your-resource.openai.azure.com/").fill("https://my-endpoint.openai.azure.com/"); @@ -280,7 +404,11 @@ test.describe("Create Target Dialog", () => { // Clear endpoint, select type — button should still be disabled await page.locator('[role="dialog"]').getByPlaceholder("https://your-resource.openai.azure.com/").fill(""); - await page.locator('[role="dialog"]').locator("select").selectOption("OpenAIChatTarget"); + await selectTargetType( + page, + page.locator('[role="dialog"]'), + "OpenAIChatTarget" + ); await expect(createBtn).toBeDisabled(); // Fill both — button should be enabled @@ -339,9 +467,9 @@ test.describe("Responsive Target Configuration", () => { if (viewport.name === "desktop") { expect(dialogBox.width).toBeLessThanOrEqual(640); } - await dialog.locator("select").first().selectOption("RoundRobinTarget"); + await selectTargetType(page, dialog, "RoundRobinTarget"); - const addTargetSelect = dialog.locator("select").nth(1); + const addTargetSelect = dialog.locator("select"); await addTargetSelect.selectOption(LONG_REGISTRY_NAME_A); await addTargetSelect.selectOption(LONG_REGISTRY_NAME_B); diff --git a/frontend/src/components/Config/CreateTargetDialog.styles.ts b/frontend/src/components/Config/CreateTargetDialog.styles.ts index 677c1d9b74..dcceac2d81 100644 --- a/frontend/src/components/Config/CreateTargetDialog.styles.ts +++ b/frontend/src/components/Config/CreateTargetDialog.styles.ts @@ -35,6 +35,58 @@ export const useCreateTargetDialogStyles = makeStyles({ maxWidth: '100%', }, }, + targetTypeListbox: { + maxHeight: 'min(28rem, 60vh)', + overflowY: 'auto', + }, + targetTypeOption: { + display: 'flex', + flexDirection: 'column', + width: '100%', + minWidth: 0, + gap: tokens.spacingVerticalXXS, + whiteSpace: 'normal', + }, + targetTypeOptionHeader: { + display: 'flex', + alignItems: 'baseline', + justifyContent: 'space-between', + flexWrap: 'wrap', + minWidth: 0, + gap: `${tokens.spacingVerticalXXS} ${tokens.spacingHorizontalS}`, + }, + targetTypeIdentifier: { + maxWidth: '100%', + padding: `0 ${tokens.spacingHorizontalXS}`, + overflowWrap: 'anywhere', + color: tokens.colorNeutralForeground3, + backgroundColor: tokens.colorNeutralBackground3, + borderRadius: tokens.borderRadiusSmall, + fontFamily: tokens.fontFamilyMonospace, + fontSize: tokens.fontSizeBase200, + }, + targetTypeDescription: { + display: 'block', + minWidth: 0, + color: tokens.colorNeutralForeground2, + overflowWrap: 'anywhere', + whiteSpace: 'normal', + }, + targetTypeAuth: { + display: 'block', + color: tokens.colorNeutralForeground3, + whiteSpace: 'normal', + }, + selectedTargetDetails: { + display: 'flex', + flexDirection: 'column', + minWidth: 0, + gap: tokens.spacingVerticalXS, + padding: `${tokens.spacingVerticalS} ${tokens.spacingHorizontalS}`, + backgroundColor: tokens.colorNeutralBackground2, + border: `${tokens.strokeWidthThin} solid ${tokens.colorNeutralStroke2}`, + borderRadius: tokens.borderRadiusMedium, + }, warningMessage: { width: '100%', }, diff --git a/frontend/src/components/Config/CreateTargetDialog.test.tsx b/frontend/src/components/Config/CreateTargetDialog.test.tsx index 70fa3ffb3d..8dc4233e19 100644 --- a/frontend/src/components/Config/CreateTargetDialog.test.tsx +++ b/frontend/src/components/Config/CreateTargetDialog.test.tsx @@ -1,7 +1,8 @@ -import { render, screen, waitFor, fireEvent } from "@testing-library/react"; +import { render, screen, waitFor, fireEvent, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { FluentProvider, webLightTheme } from "@fluentui/react-components"; import { makeTarget } from "@/test-utils/targetFixtures"; +import type { TargetCatalogResponse } from "@/types"; import CreateTargetDialog from "./CreateTargetDialog"; import { parseWeight, MAX_WEIGHT } from "./weightValidation"; import { targetsApi } from "@/services/api"; @@ -16,31 +17,112 @@ jest.mock("@/services/api", () => ({ const mockedTargetsApi = targetsApi as jest.Mocked; -// Representative target catalog covering the types the dialog renders. Mirrors -// the shape returned by GET /targets/catalog. -const TARGET_CATALOG = { +const TARGET_CATALOG: TargetCatalogResponse = { items: [ - { target_type: "OpenAIChatTarget", parameters: [], supported_auth_modes: ["api_key", "identity"] }, - { target_type: "OpenAIResponseTarget", parameters: [], supported_auth_modes: ["api_key", "identity"] }, - { target_type: "AzureMLChatTarget", parameters: [], supported_auth_modes: ["api_key", "identity"] }, - { target_type: "RoundRobinTarget", parameters: [], supported_auth_modes: ["api_key"] }, + { + target_type: "AzureMLChatTarget", + parameters: [], + supported_auth_modes: ["api_key", "identity"], + description: "A prompt target for Azure Machine Learning chat endpoints.", + }, + { + target_type: "OpenAIChatTarget", + parameters: [], + supported_auth_modes: ["api_key", "identity"], + description: "Facilitates multimodal (image and text) input and text output generation.", + }, + { + target_type: "OpenAICompletionTarget", + parameters: [], + supported_auth_modes: ["api_key", "identity"], + description: "A prompt target for OpenAI completion endpoints.", + }, + { + target_type: "OpenAIImageTarget", + parameters: [], + supported_auth_modes: ["api_key", "identity"], + description: "A target for image generation or editing using OpenAI's image models.", + }, + { + target_type: "OpenAIResponseTarget", + parameters: [], + supported_auth_modes: ["api_key", "identity"], + description: "Enables communication with endpoints that support the OpenAI Response API.", + }, + { + target_type: "OpenAITTSTarget", + parameters: [], + supported_auth_modes: ["api_key", "identity"], + description: "A prompt target for OpenAI Text-to-Speech (TTS) endpoints.", + }, + { + target_type: "OpenAIVideoTarget", + parameters: [], + supported_auth_modes: ["api_key", "identity"], + description: "OpenAI Video Target using the OpenAI SDK for video generation.", + }, + { + target_type: "RoundRobinTarget", + parameters: [], + supported_auth_modes: ["api_key"], + description: "A prompt target that distributes requests across multiple inner targets using weighted round-robin selection.", + }, ], }; +const TARGET_DISPLAY_NAMES: Record = { + AzureMLChatTarget: "Azure Machine Learning chat", + OpenAIChatTarget: "OpenAI chat", + OpenAICompletionTarget: "OpenAI text completion", + OpenAIImageTarget: "OpenAI image", + OpenAIResponseTarget: "OpenAI Responses API", + OpenAITTSTarget: "OpenAI text to speech", + OpenAIVideoTarget: "OpenAI video", + RoundRobinTarget: "Weighted round robin", +}; + const TestWrapper: React.FC<{ children: React.ReactNode }> = ({ children, }) => {children}; -/** - * Helper to select a target type from the native select element. - * Uses selectOptions from userEvent which works with native select. - */ -async function selectTargetType( - user: ReturnType, - value: string -) { - const select = screen.getByRole("combobox"); - await user.selectOptions(select, value); +// Fluent's Dropdown renders its listbox in a portal guarded by a focus +// modalizer. Under jsdom the popover only toggles via a direct click event, +// and the `aria-hidden` the modalizer puts on the dialog while the listbox is +// open is never restored once it closes, which would hide the rest of the form +// from role-based queries. Both behaviors are jsdom artifacts — real pointer +// and keyboard interaction is covered by e2e/config.spec.ts. +async function openTargetTypePicker(): Promise { + const picker = screen.getByRole("combobox", { name: /target type/i }); + await waitFor(() => { + expect(picker).toBeEnabled(); + }); + fireEvent.click(picker); + await screen.findByRole("listbox"); + return picker; +} + +function restoreDialogAccessibility(): void { + const dialog = document.querySelector('[role="dialog"]'); + if (!dialog) return; + const hiddenAncestors = document.querySelectorAll('[aria-hidden="true"]'); + for (const element of Array.from(hiddenAncestors)) { + if (element === dialog || element.contains(dialog)) { + element.removeAttribute("aria-hidden"); + } + } +} + +async function selectTargetType(value: string): Promise { + await openTargetTypePicker(); + fireEvent.click( + screen.getByRole("option", { + name: new RegExp(`Implementation: ${value}`), + }), + ); + await waitFor(() => { + expect(screen.queryByRole("listbox")).not.toBeInTheDocument(); + }); + restoreDialogAccessibility(); } describe("parseWeight", () => { @@ -118,9 +200,7 @@ describe("CreateTargetDialog", () => { beforeEach(() => { jest.clearAllMocks(); - mockedTargetsApi.listTargetCatalog.mockResolvedValue( - TARGET_CATALOG as unknown as Awaited>, - ); + mockedTargetsApi.listTargetCatalog.mockResolvedValue(TARGET_CATALOG); mockedTargetsApi.listTargets.mockResolvedValue({ items: [], pagination: { limit: 200, has_more: false, next_cursor: null, prev_cursor: null }, @@ -139,6 +219,84 @@ describe("CreateTargetDialog", () => { expect(screen.getByText("Cancel")).toBeInTheDocument(); }); + it("should show friendly names, catalog descriptions, implementation identifiers, and auth for all target types", async () => { + render( + + + + ); + + await openTargetTypePicker(); + + const options = screen.getAllByRole("option"); + expect(options).toHaveLength(8); + for (const entry of TARGET_CATALOG.items) { + 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).getByText(entry.description ?? "")).toBeInTheDocument(); + } + + expect(within(screen.getByRole("option", { + name: /Implementation: OpenAIChatTarget/, + })).getByText(/API key or Microsoft Entra ID/)).toBeInTheDocument(); + expect(within(screen.getByRole("option", { + name: /Implementation: RoundRobinTarget/, + })).getByText("Supported authentication: API key")).toBeInTheDocument(); + }); + + it("should keep guidance for the selected target visible after the list closes", async () => { + render( + + + + ); + + await selectTargetType("RoundRobinTarget"); + + expect(screen.getByRole("combobox", { name: /target type/i })).toHaveTextContent( + "Weighted round robin", + ); + const selectedDetails = screen.getByRole("region", { name: /selected target details/i }); + expect(selectedDetails).toHaveTextContent("RoundRobinTarget"); + expect(selectedDetails).toHaveTextContent("weighted round-robin selection"); + expect(selectedDetails).toHaveTextContent("Supported authentication: API key"); + }); + + it("should disable target selection while catalog details are loading", () => { + mockedTargetsApi.listTargetCatalog.mockReturnValue( + new Promise(() => {}), + ); + + render( + + + + ); + + expect(screen.getByText("Loading target details...")).toBeInTheDocument(); + expect(screen.getByRole("combobox", { name: /target type/i })).toBeDisabled(); + }); + + it("should keep all target types selectable and explain when catalog details fail to load", async () => { + mockedTargetsApi.listTargetCatalog.mockRejectedValueOnce(new Error("catalog unavailable")); + + render( + + + + ); + + expect(await screen.findByText(/Target details could not be loaded/)).toBeInTheDocument(); + await openTargetTypePicker(); + expect(screen.getAllByRole("option")).toHaveLength(8); + expect(screen.getByRole("option", { + name: /Azure Machine Learning chat.*Implementation: AzureMLChatTarget/, + })).toBeInTheDocument(); + }); + it("should not render when closed", () => { render( @@ -161,7 +319,6 @@ describe("CreateTargetDialog", () => { }); it("should hide the Authentication field until a target type is selected", async () => { - const user = userEvent.setup(); render( @@ -178,7 +335,7 @@ describe("CreateTargetDialog", () => { ).toBeInTheDocument(); // Selecting an identity-capable type should reveal the Authentication field. - await selectTargetType(user, "OpenAIChatTarget"); + await selectTargetType("OpenAIChatTarget"); expect( screen.getByRole("radio", { name: /Identity-based/ }) ).toBeInTheDocument(); @@ -214,7 +371,7 @@ describe("CreateTargetDialog", () => { ); // Select target type - await selectTargetType(user, "OpenAIChatTarget"); + await selectTargetType("OpenAIChatTarget"); // Fill the endpoint & model names const endpointInput = screen.getByPlaceholderText( @@ -255,7 +412,7 @@ describe("CreateTargetDialog", () => { ); // Select target type - await selectTargetType(user, "OpenAIChatTarget"); + await selectTargetType("OpenAIChatTarget"); // Fill endpoint & model names const endpointInput = screen.getByPlaceholderText( @@ -303,7 +460,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "OpenAIChatTarget"); + await selectTargetType("OpenAIChatTarget"); const endpointInput = screen.getByPlaceholderText( "https://your-resource.openai.azure.com/" @@ -341,7 +498,7 @@ describe("CreateTargetDialog", () => { ); // Select target type - await selectTargetType(user, "OpenAIChatTarget"); + await selectTargetType("OpenAIChatTarget"); // Fill endpoint const endpointInput = screen.getByPlaceholderText( @@ -371,7 +528,7 @@ describe("CreateTargetDialog", () => { ); // Select target type - await selectTargetType(user, "OpenAIChatTarget"); + await selectTargetType("OpenAIChatTarget"); // Fill endpoint — use fireEvent.change because userEvent.type truncates // URLs containing periods in FluentUI Input under jsdom. @@ -431,7 +588,6 @@ describe("CreateTargetDialog", () => { }); it("should show field validation errors when submitting form without endpoint", async () => { - const user = userEvent.setup(); render( @@ -440,7 +596,7 @@ describe("CreateTargetDialog", () => { ); // Select target type but leave endpoint empty - await selectTargetType(user, "OpenAIChatTarget"); + await selectTargetType("OpenAIChatTarget"); // Submit via form (bypass disabled button by submitting the form directly) const form = screen.getByText("Create New Target").closest("form") ?? @@ -464,7 +620,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "OpenAIChatTarget"); + await selectTargetType("OpenAIChatTarget"); const endpointInput = screen.getByPlaceholderText( "https://your-resource.openai.azure.com/" @@ -493,7 +649,7 @@ describe("CreateTargetDialog", () => { ); // Select AzureMLChatTarget type - await selectTargetType(user, "AzureMLChatTarget"); + await selectTargetType("AzureMLChatTarget"); // Fill endpoint const endpointInput = screen.getByPlaceholderText( @@ -527,7 +683,6 @@ describe("CreateTargetDialog", () => { }); it("should show AzureML fields and hide OpenAI fields when AzureMLChatTarget selected", async () => { - const user = userEvent.setup(); render( @@ -535,7 +690,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "AzureMLChatTarget"); + await selectTargetType("AzureMLChatTarget"); // AzureML-specific fields should be visible expect(screen.getByText("Max New Tokens")).toBeInTheDocument(); @@ -561,7 +716,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "AzureMLChatTarget"); + await selectTargetType("AzureMLChatTarget"); // Fill endpoint — use fireEvent.change because userEvent.type truncates // URLs containing periods in FluentUI Input under jsdom. @@ -641,7 +796,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "OpenAIChatTarget"); + await selectTargetType("OpenAIChatTarget"); const endpointInput = screen.getByPlaceholderText( "https://your-resource.openai.azure.com/" @@ -695,7 +850,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "OpenAIChatTarget"); + await selectTargetType("OpenAIChatTarget"); const endpointInput = screen.getByPlaceholderText( "https://your-resource.openai.azure.com/" @@ -732,7 +887,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "OpenAIChatTarget"); + await selectTargetType("OpenAIChatTarget"); const endpointInput = screen.getByPlaceholderText( "https://your-resource.openai.azure.com/" @@ -757,7 +912,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "OpenAIChatTarget"); + await selectTargetType("OpenAIChatTarget"); const endpointInput = screen.getByPlaceholderText( "https://your-resource.openai.azure.com/" @@ -784,7 +939,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "OpenAIChatTarget"); + await selectTargetType("OpenAIChatTarget"); const endpointInput = screen.getByPlaceholderText( "https://your-resource.openai.azure.com/" @@ -812,7 +967,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "AzureMLChatTarget"); + await selectTargetType("AzureMLChatTarget"); const endpointInput = screen.getByPlaceholderText( "https://your-model.region.inference.ml.azure.com/score" @@ -841,7 +996,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "AzureMLChatTarget"); + await selectTargetType("AzureMLChatTarget"); const endpointInput = screen.getByPlaceholderText( "https://your-model.region.inference.ml.azure.com/score" @@ -870,7 +1025,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "AzureMLChatTarget"); + await selectTargetType("AzureMLChatTarget"); const endpointInput = screen.getByPlaceholderText( "https://your-model.region.inference.ml.azure.com/score" @@ -892,7 +1047,6 @@ describe("CreateTargetDialog", () => { }); it("should show target picker when RoundRobinTarget is selected", async () => { - const user = userEvent.setup(); render( @@ -916,7 +1070,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "RoundRobinTarget"); + await selectTargetType("RoundRobinTarget"); // Endpoint field should NOT be visible for RoundRobin expect( @@ -928,7 +1082,6 @@ describe("CreateTargetDialog", () => { }); it("should disable Create button when fewer than 2 inner targets are selected for RoundRobin", async () => { - const user = userEvent.setup(); render( @@ -945,7 +1098,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "RoundRobinTarget"); + await selectTargetType("RoundRobinTarget"); const createButton = screen.getByText("Create Target").closest("button"); expect(createButton).toBeDisabled(); @@ -980,7 +1133,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "RoundRobinTarget"); + await selectTargetType("RoundRobinTarget"); const select = screen.getByText("Select a target to add...").closest("select"); expect(select).not.toBeNull(); if (!select) { @@ -1042,7 +1195,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "RoundRobinTarget"); + await selectTargetType("RoundRobinTarget"); // Before selecting anything: all three are eligible. const select = screen.getByText("Select a target to add...").closest("select")!; @@ -1094,7 +1247,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "RoundRobinTarget"); + await selectTargetType("RoundRobinTarget"); const select = screen.getByText("Select a target to add...").closest("select")!; await user.selectOptions(select, "foundry_a"); @@ -1146,7 +1299,7 @@ describe("CreateTargetDialog", () => { ); - await selectTargetType(user, "RoundRobinTarget"); + await selectTargetType("RoundRobinTarget"); const select = screen.getByText("Select a target to add...").closest("select")!; await user.selectOptions(select, "a"); await user.selectOptions(select, "b"); @@ -1196,7 +1349,7 @@ describe("CreateTargetDialog", () => { /> ); - await selectTargetType(user, "RoundRobinTarget"); + await selectTargetType("RoundRobinTarget"); const select = screen.getByText("Select a target to add...").closest("select")!; await user.selectOptions(select, "a"); await user.selectOptions(select, "b"); diff --git a/frontend/src/components/Config/CreateTargetDialog.tsx b/frontend/src/components/Config/CreateTargetDialog.tsx index 59869c9d86..24837c546b 100644 --- a/frontend/src/components/Config/CreateTargetDialog.tsx +++ b/frontend/src/components/Config/CreateTargetDialog.tsx @@ -7,12 +7,15 @@ import { DialogContent, DialogActions, Button, + Dropdown, Input, Label, Link, + Option, Radio, RadioGroup, Select, + Spinner, Switch, Text, tokens, @@ -61,7 +64,39 @@ const TARGET_FORM_SHAPES: Record = { const RENDERABLE_TARGET_TYPES = Object.keys(TARGET_FORM_SHAPES) +const TARGET_DISPLAY_NAMES: Record = { + AzureMLChatTarget: 'Azure Machine Learning chat', + OpenAIChatTarget: 'OpenAI chat', + OpenAICompletionTarget: 'OpenAI text completion', + OpenAIImageTarget: 'OpenAI image', + OpenAIResponseTarget: 'OpenAI Responses API', + OpenAITTSTarget: 'OpenAI text to speech', + OpenAIVideoTarget: 'OpenAI video', + RoundRobinTarget: 'Weighted round robin', +} + +const FALLBACK_TARGET_CATALOG_ENTRIES: TargetCatalogEntry[] = RENDERABLE_TARGET_TYPES.map((targetType) => ({ + target_type: targetType, + parameters: [], + supported_auth_modes: [], + description: null, +})) + type AuthMode = 'api_key' | 'identity' +type CatalogStatus = 'loading' | 'loaded' | 'error' + +function getTargetDisplayName(targetType: string): string { + return TARGET_DISPLAY_NAMES[targetType] ?? targetType +} + +function getAuthDescription(authModes: TargetCatalogEntry['supported_auth_modes']): string | null { + if (authModes.length === 0) return null + + const labels = authModes.map((mode) => ( + mode === 'identity' ? 'Microsoft Entra ID' : 'API key' + )) + return `Supported authentication: ${labels.join(' or ')}` +} /** * Fallback for whether a target type supports identity-based auth when the @@ -189,11 +224,25 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT // --- Catalog state --- // Available target types + their auth facts, fetched from the backend registry. const [catalogEntries, setCatalogEntries] = useState([]) + const [catalogStatus, setCatalogStatus] = useState('loading') const catalogByType = useMemo( () => new Map(catalogEntries.map((entry) => [entry.target_type, entry])), [catalogEntries], ) + // Reset the catalog back to its loading state whenever the dialog is opened, + // so a stale error/entries from a previous session isn't shown while the + // refetch is in flight. Adjusted during render (rather than in the effect + // below) to avoid a cascading render. + const [seenOpen, setSeenOpen] = useState(open) + if (open !== seenOpen) { + setSeenOpen(open) + if (open) { + setCatalogEntries([]) + setCatalogStatus('loading') + } + } + // Fetch the target catalog once when the dialog opens. The backend is the // authority on which types exist and which auth modes they support. useEffect(() => { @@ -201,29 +250,40 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT let cancelled = false targetsApi.listTargetCatalog() .then((res) => { - if (!cancelled) setCatalogEntries(res.items) + if (!cancelled) { + setCatalogEntries(res.items) + setCatalogStatus('loaded') + } }) .catch(() => { - // Ignore fetch errors — fall back to the locally-known renderable types. + if (!cancelled) { + setCatalogEntries([]) + setCatalogStatus('error') + } }) return () => { cancelled = true } }, [open]) - // The types offered in the dropdown: catalog types the dialog can render, - // preserving catalog order. Fall back to the locally-known types when the - // catalog hasn't loaded (or the fetch failed) so the form stays usable. - const targetTypeOptions = useMemo(() => { - const fromCatalog = catalogEntries - .map((entry) => entry.target_type) - .filter((type) => type in TARGET_FORM_SHAPES) - return fromCatalog.length > 0 ? fromCatalog : RENDERABLE_TARGET_TYPES + const catalogTargetTypeOptions = useMemo(() => { + return catalogEntries.filter((entry) => entry.target_type in TARGET_FORM_SHAPES) }, [catalogEntries]) + const catalogMetadataAvailable = catalogTargetTypeOptions.length > 0 + const catalogUnavailable = catalogStatus !== 'loading' && !catalogMetadataAvailable + const targetTypeOptions = catalogStatus === 'loading' + ? [] + : catalogMetadataAvailable + ? catalogTargetTypeOptions + : FALLBACK_TARGET_CATALOG_ENTRIES const formShape = TARGET_FORM_SHAPES[targetType] const isRoundRobin = formShape === 'roundrobin' const isAzureML = formShape === 'azureml' const isOpenAi = formShape === 'openai' const catalogEntry = catalogByType.get(targetType) + const selectedTargetDisplayName = getTargetDisplayName(targetType) + const selectedTargetAuthDescription = catalogEntry + ? getAuthDescription(catalogEntry.supported_auth_modes) + : null const supportsIdentity = catalogEntry ? catalogEntry.supported_auth_modes.includes('identity') : defaultSupportsIdentity(formShape) @@ -447,18 +507,39 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT )} + {catalogStatus === 'loading' && ( + + )} + + {catalogUnavailable && ( + + + Target details could not be loaded. You can still select a supported target type, + but its catalog description and authentication options are unavailable. + + + )} + - + {targetTypeOptions.map((entry) => { + const displayName = getTargetDisplayName(entry.target_type) + const authDescription = getAuthDescription(entry.supported_auth_modes) + const accessibleDescription = [ + displayName, + entry.description, + `Implementation: ${entry.target_type}`, + authDescription, + ].filter((value): value is string => Boolean(value)).join('. ') + + return ( + + ) + })} + + {targetType && ( +
+
+ {selectedTargetDisplayName} + {targetType} +
+ {catalogEntry?.description ? ( + + {catalogEntry.description} + + ) : ( + + Catalog details are unavailable for this target. + + )} + {selectedTargetAuthDescription && ( + + {selectedTargetAuthDescription} + + )} +
+ )} + {/* === RoundRobinTarget form: select existing targets === */} {isRoundRobin && ( <> From 72b05005f0239881a1e139b2741043e9add62817 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Tue, 4 Aug 2026 09:35:38 -0700 Subject: [PATCH 2/2] FIX Keep selected target visible after the picker loses focus Addresses review feedback on #2316. Passing `value={undefined}` while no target was selected made the Dropdown permanently uncontrolled: Fluent's `useIsControlled` freezes on the first render, so the `value` prop was ignored for the rest of the component's life. The trigger text then fell back to `getOptionsMatchingValue(...)`, which is empty once the listbox unmounts on blur, so the picker reverted to the "Select a target type" placeholder as soon as the user moved to Endpoint. Passing '' keeps it controlled from the first render. The placeholder still renders for the empty case, since the trigger uses `value || placeholder`. Adds a jsdom regression test and an assertion in the existing e2e creation flow; both were confirmed to fail without the one-line change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2c45e075-9e85-40b5-8352-fb0bcab7261c --- frontend/e2e/config.spec.ts | 5 +++++ .../Config/CreateTargetDialog.test.tsx | 22 +++++++++++++++++++ .../components/Config/CreateTargetDialog.tsx | 2 +- 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/frontend/e2e/config.spec.ts b/frontend/e2e/config.spec.ts index e0731918d0..e47b071db6 100644 --- a/frontend/e2e/config.spec.ts +++ b/frontend/e2e/config.spec.ts @@ -371,6 +371,11 @@ test.describe("Create Target Dialog", () => { // Fill endpoint await dialog.getByPlaceholder("https://your-resource.openai.azure.com/").fill("https://my-endpoint.openai.azure.com/"); + // The picker must keep showing the selection once focus moves to another field + await expect(dialog.getByRole("combobox", { name: "Target Type" })).toContainText( + "OpenAI chat" + ); + // Fill model name await dialog.getByPlaceholder("e.g. gpt-4o, my-deployment").fill("gpt-4o-test"); diff --git a/frontend/src/components/Config/CreateTargetDialog.test.tsx b/frontend/src/components/Config/CreateTargetDialog.test.tsx index 8dc4233e19..cb452f0fd0 100644 --- a/frontend/src/components/Config/CreateTargetDialog.test.tsx +++ b/frontend/src/components/Config/CreateTargetDialog.test.tsx @@ -265,6 +265,28 @@ describe("CreateTargetDialog", () => { expect(selectedDetails).toHaveTextContent("Supported authentication: API key"); }); + it("should keep the selected target displayed after focus moves to another field", async () => { + const user = userEvent.setup(); + + render( + + + + ); + + await selectTargetType("OpenAIChatTarget"); + + await user.click( + screen.getByPlaceholderText("https://your-resource.openai.azure.com/"), + ); + await user.keyboard("https://api.openai.com"); + restoreDialogAccessibility(); + + const picker = screen.getByRole("combobox", { name: /target type/i }); + expect(picker).toHaveTextContent("OpenAI chat"); + expect(picker).not.toHaveTextContent("Select a target type"); + }); + it("should disable target selection while catalog details are loading", () => { mockedTargetsApi.listTargetCatalog.mockReturnValue( new Promise(() => {}), diff --git a/frontend/src/components/Config/CreateTargetDialog.tsx b/frontend/src/components/Config/CreateTargetDialog.tsx index 24837c546b..6fa4afd62c 100644 --- a/frontend/src/components/Config/CreateTargetDialog.tsx +++ b/frontend/src/components/Config/CreateTargetDialog.tsx @@ -536,7 +536,7 @@ export default function CreateTargetDialog({ open, onClose, onCreated, existingT placeholder={catalogStatus === 'loading' ? 'Loading target types...' : 'Select a target type'} positioning={{ matchTargetSize: 'width' }} selectedOptions={targetType ? [targetType] : []} - value={targetType ? selectedTargetDisplayName : undefined} + value={targetType ? selectedTargetDisplayName : ''} onOptionSelect={(_, data) => { const next = data.optionValue if (!next) return