Skip to content

FIX Clarify target choices in the Add Target picker - #2316

Open
romanlutz wants to merge 1 commit into
microsoft:mainfrom
romanlutz:romanlutz-clarify-target-choices
Open

FIX Clarify target choices in the Add Target picker#2316
romanlutz wants to merge 1 commit into
microsoft:mainfrom
romanlutz:romanlutz-clarify-target-choices

Conversation

@romanlutz

Copy link
Copy Markdown
Contributor

Description

The Add Target dialog listed its eight choices as raw implementation class names only (AzureMLChatTarget, OpenAIChatTarget, OpenAICompletionTarget, OpenAIImageTarget, OpenAIResponseTarget, OpenAITTSTarget, OpenAIVideoTarget, RoundRobinTarget). The five OpenAI entries are nearly indistinguishable from their names alone, and nothing in the selection UI explained what a target does or how it authenticates. A first-time user had to guess.

The backend already had everything needed to fix this. GET /targets/catalog returns a description per entry (sourced from metadata.class_description) plus supported_auth_modes, and TargetCatalogEntry.description was already declared in frontend/src/types/index.ts. target_service.py notes that deciding what to surface "is a presentation concern owned by the caller", so this is a frontend-only change with no API or backend work.

Approach: the bare native Select is replaced with a Fluent Dropdown. Each option now renders a human-friendly name, the catalog description, the implementation identifier as secondary detail, and the supported auth modes. Once a target is chosen, a "Selected target details" region keeps that same information visible so the user can confirm the choice before filling in parameters, rather than being left with just a class name.

Open picker showing all eight targets with descriptions

Before and after with RoundRobinTarget selected:

Before After
Before: only the class name is shown After: name, identifier, description and auth remain visible

Notes for reviewers:

  • No invented capabilities. Everything shown comes from the catalog response. Multi-turn and system-prompt behavior is deliberately not claimed, because the catalog does not represent it reliably. Auth text is derived solely from supported_auth_modes.
  • Display names and a fallback catalog are hardcoded in the component (TARGET_DISPLAY_NAMES, FALLBACK_TARGET_CATALOG_ENTRIES). The fallback only supplies the friendly label and identifier when the catalog request fails, so the picker degrades to roughly today's behavior plus a MessageBar rather than rendering an empty list. Descriptions are never faked.
  • Accessibility. Each option carries an aria-label of the form "<display>. <description>. Implementation: <TypeName>. Supported authentication: ...", which is what both the Jest and Playwright tests select on. That string is unambiguous across the similar OpenAI names.
  • Catalog reset on reopen uses the React "adjust state during render" pattern via a seenOpen state variable instead of a setState inside an effect, which the react-hooks/set-state-in-effect rule forbids.
  • The e2e/accessibility.spec.ts "mobile touch context" failures are pre-existing and unrelated (onboarding tour touch targets are 24px). I confirmed this by stashing these changes and reproducing the failures on unmodified main.

Tests and Documentation

CreateTargetDialog.test.tsx grows from 46 to 50 tests, covering the loading spinner, the catalog-unavailable message bar, description and auth rendering per option, and the selected-target details region. The existing creation, validation, and parameter tests were migrated to the new control.

Two jsdom-only quirks of the Fluent Dropdown when nested in a Fluent Dialog forced a test-helper change, and they are documented in a comment block above the helpers so the next person does not rediscover them:

  1. userEvent.click does not open the popover in that nesting (it works standalone); fireEvent.click does.
  2. The popover's focus modalizer sets aria-hidden="true" on the dialog while the listbox is open and never removes it, which hides the rest of the form from role-based queries. restoreDialogAccessibility() strips it after close.

Neither reproduces in a real browser, which the Playwright coverage confirms. e2e/config.spec.ts adds a test asserting all eight choices are present and distinguishable, and exercises keyboard navigation through the picker.

Results: Jest 41 suites / 877 tests pass. npm run lint and npm run type-check are clean. Playwright config.spec.ts passes 15/15.

JupyText: N/A, no files under doc/ changed.

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants