diff --git a/frontend/src/components/Chat/TargetBadge.styles.ts b/frontend/src/components/Chat/TargetBadge.styles.ts index afd94aacb8..a879666d9d 100644 --- a/frontend/src/components/Chat/TargetBadge.styles.ts +++ b/frontend/src/components/Chat/TargetBadge.styles.ts @@ -80,4 +80,13 @@ export const useTargetBadgeStyles = makeStyles({ overflowX: 'auto', boxSizing: 'border-box', }, + /** A single inner target entry in the tooltip's Inner Targets section. */ + innerTargetItem: { + display: 'flex', + flexDirection: 'column', + gap: tokens.spacingVerticalXXS, + padding: `${tokens.spacingVerticalXXS} ${tokens.spacingHorizontalXS}`, + backgroundColor: tokens.colorNeutralBackground2, + borderRadius: tokens.borderRadiusSmall, + }, }) diff --git a/frontend/src/components/Chat/TargetBadge.test.tsx b/frontend/src/components/Chat/TargetBadge.test.tsx index bf53f81f56..172fbb3566 100644 --- a/frontend/src/components/Chat/TargetBadge.test.tsx +++ b/frontend/src/components/Chat/TargetBadge.test.tsx @@ -73,4 +73,45 @@ describe('TargetBadge', () => { ).not.toThrow() expect(screen.getByTestId('target-badge')).toHaveTextContent('TextTarget') }) + + it('shows count in display name for RoundRobinTarget with inner targets', () => { + const rrTarget: TargetInstance = { + target_registry_name: 'rr_test', + target_type: 'RoundRobinTarget', + model_name: 'gpt-4o', + inner_targets: [ + { target_registry_name: 'a', target_type: 'OpenAIChatTarget', model_name: 'gpt-4o' }, + { target_registry_name: 'b', target_type: 'OpenAIChatTarget', model_name: 'gpt-4o' }, + { target_registry_name: 'c', target_type: 'OpenAIChatTarget', model_name: 'gpt-4o' }, + ], + } + render( + + + + ) + const badge = screen.getByTestId('target-badge') + expect(badge).toHaveTextContent('RoundRobinTarget (gpt-4o ×3)') + }) + + it('prefers underlying_model_name over model_name for RoundRobinTarget badge', () => { + const rrTarget: TargetInstance = { + target_registry_name: 'rr_mixed', + target_type: 'RoundRobinTarget', + model_name: 'gpt-4o-japan-nilfilter', + underlying_model_name: 'gpt-4o', + inner_targets: [ + { target_registry_name: 'a', target_type: 'OpenAIChatTarget', model_name: 'deploy-1' }, + { target_registry_name: 'b', target_type: 'OpenAIChatTarget', model_name: 'deploy-2' }, + ], + } + render( + + + + ) + const badge = screen.getByTestId('target-badge') + // Should show the underlying model, not the deployment name + expect(badge).toHaveTextContent('RoundRobinTarget (gpt-4o ×2)') + }) }) diff --git a/frontend/src/components/Chat/TargetBadge.tsx b/frontend/src/components/Chat/TargetBadge.tsx index eb57cbb5b4..aee8be493c 100644 --- a/frontend/src/components/Chat/TargetBadge.tsx +++ b/frontend/src/components/Chat/TargetBadge.tsx @@ -33,9 +33,23 @@ function formatParams(params?: Record | null): string { export default function TargetBadge({ target }: TargetBadgeProps) { const styles = useTargetBadgeStyles() - const displayName = target.model_name - ? `${target.target_type} (${target.model_name})` - : target.target_type + const innerTargets = target.inner_targets ?? [] + const isRoundRobin = innerTargets.length > 0 + + // For RoundRobinTarget, prefer underlying_model_name because inner targets share + // the same underlying model but may have different deployment names (model_name). + // For regular targets, use model_name as before. + const badgeModel = isRoundRobin + ? (target.underlying_model_name ?? target.model_name) + : target.model_name + const displayName = isRoundRobin + ? badgeModel + ? `${target.target_type} (${badgeModel} ×${innerTargets.length})` + : `${target.target_type} (×${innerTargets.length})` + : target.model_name + ? `${target.target_type} (${target.model_name})` + : target.target_type + const showUnderlying = target.underlying_model_name && target.model_name && @@ -47,6 +61,9 @@ export default function TargetBadge({ target }: TargetBadgeProps) { const outputModalities = target.capabilities?.supported_output_modalities ?? [] const params = formatParams(target.target_specific_params) + // Extract weights from params so we can show them next to each inner target + const weights = target.target_specific_params?.weights as number[] | undefined + const tooltipContent = (
@@ -91,6 +108,28 @@ export default function TargetBadge({ target }: TargetBadgeProps) {
)} + {/* Inner targets section — only shown for composite targets like RoundRobinTarget */} + {isRoundRobin && ( +
+ Inner Targets ({innerTargets.length}) + {innerTargets.map((inner, idx) => ( +
+ + #{idx + 1}{weights?.[idx] != null ? ` (weight: ${weights[idx]})` : ''} + + + {inner.target_type} + {inner.model_name ? ` — ${inner.model_name}` : ''} + + {inner.endpoint && ( + + {inner.endpoint} + + )} +
+ ))} +
+ )} {params && (
Parameters diff --git a/frontend/src/components/Config/CreateTargetDialog.styles.ts b/frontend/src/components/Config/CreateTargetDialog.styles.ts index 67fdb2aa2f..f63ca604d7 100644 --- a/frontend/src/components/Config/CreateTargetDialog.styles.ts +++ b/frontend/src/components/Config/CreateTargetDialog.styles.ts @@ -14,4 +14,19 @@ export const useCreateTargetDialogStyles = makeStyles({ overflowWrap: 'anywhere', wordBreak: 'break-word', }, + /** Container for the list of selected inner targets in the RoundRobin form. */ + selectedTargetsList: { + display: 'flex', + flexDirection: 'column', + gap: tokens.spacingVerticalXS, + }, + /** A single row in the selected targets list: target name + weight + remove button. */ + selectedTargetRow: { + display: 'flex', + alignItems: 'center', + gap: tokens.spacingHorizontalS, + padding: `${tokens.spacingVerticalXS} ${tokens.spacingHorizontalS}`, + backgroundColor: tokens.colorNeutralBackground2, + borderRadius: tokens.borderRadiusSmall, + }, }) diff --git a/frontend/src/components/Config/CreateTargetDialog.test.tsx b/frontend/src/components/Config/CreateTargetDialog.test.tsx index 69b7b65ab2..c825c32ac5 100644 --- a/frontend/src/components/Config/CreateTargetDialog.test.tsx +++ b/frontend/src/components/Config/CreateTargetDialog.test.tsx @@ -2,6 +2,7 @@ import { render, screen, waitFor, fireEvent } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { FluentProvider, webLightTheme } from "@fluentui/react-components"; import CreateTargetDialog from "./CreateTargetDialog"; +import { parseWeight, MAX_WEIGHT } from "./weightValidation"; import { targetsApi } from "@/services/api"; jest.mock("@/services/api", () => ({ @@ -28,6 +29,72 @@ async function selectTargetType( await user.selectOptions(select, value); } +describe("parseWeight", () => { + it("rejects empty input", () => { + expect(parseWeight("")).toEqual({ ok: false, error: "Weight is required" }); + }); + + it("rejects decimals like '2.5' (parseInt would silently truncate to 2)", () => { + expect(parseWeight("2.5")).toEqual({ + ok: false, + error: "Weight must be a whole number", + }); + }); + + it("rejects scientific notation like '1e10' (parseInt would silently return 1)", () => { + expect(parseWeight("1e10")).toEqual({ + ok: false, + error: "Weight must be a whole number", + }); + }); + + it("rejects negatives", () => { + expect(parseWeight("-3")).toEqual({ + ok: false, + error: "Weight must be a whole number", + }); + }); + + it("rejects whitespace and trailing characters", () => { + expect(parseWeight(" 5")).toEqual({ + ok: false, + error: "Weight must be a whole number", + }); + expect(parseWeight("5x")).toEqual({ + ok: false, + error: "Weight must be a whole number", + }); + }); + + it("rejects 0 with a 'must be at least 1' error (no silent revert)", () => { + expect(parseWeight("0")).toEqual({ + ok: false, + error: "Weight must be at least 1", + }); + }); + + it(`rejects values above MAX_WEIGHT (${MAX_WEIGHT})`, () => { + expect(parseWeight(String(MAX_WEIGHT + 1))).toEqual({ + ok: false, + error: `Weight must be at most ${MAX_WEIGHT}`, + }); + expect(parseWeight("99999999999")).toEqual({ + ok: false, + error: `Weight must be at most ${MAX_WEIGHT}`, + }); + }); + + it("accepts boundary values 1 and MAX_WEIGHT", () => { + expect(parseWeight("1")).toEqual({ ok: true, value: 1 }); + expect(parseWeight(String(MAX_WEIGHT))).toEqual({ ok: true, value: MAX_WEIGHT }); + }); + + it("accepts typical integer weights", () => { + expect(parseWeight("7")).toEqual({ ok: true, value: 7 }); + expect(parseWeight("42")).toEqual({ ok: true, value: 42 }); + }); +}); + describe("CreateTargetDialog", () => { const defaultProps = { open: true, @@ -361,7 +428,7 @@ describe("CreateTargetDialog", () => { }); }); - it("should show generic error for non-Error exceptions", async () => { + it("should surface string throws verbatim via toApiError", async () => { const user = userEvent.setup(); mockedTargetsApi.createTarget.mockRejectedValue("string error"); @@ -381,7 +448,7 @@ describe("CreateTargetDialog", () => { await user.click(screen.getByText("Create Target")); await waitFor(() => { - expect(screen.getByText("Failed to create target")).toBeInTheDocument(); + expect(screen.getByText("string error")).toBeInTheDocument(); }); }); @@ -797,4 +864,414 @@ describe("CreateTargetDialog", () => { expect(mockedTargetsApi.createTarget).not.toHaveBeenCalled(); }); + + it("should show target picker when RoundRobinTarget is selected", async () => { + const user = userEvent.setup(); + + render( + + + + ); + + await selectTargetType(user, "RoundRobinTarget"); + + // Endpoint field should NOT be visible for RoundRobin + expect( + screen.queryByPlaceholderText("https://your-resource.openai.azure.com/") + ).not.toBeInTheDocument(); + + // Add Target dropdown should be visible + expect(screen.getByText("Add Target")).toBeInTheDocument(); + }); + + it("should disable Create button when fewer than 2 inner targets are selected for RoundRobin", async () => { + const user = userEvent.setup(); + + render( + + + + ); + + await selectTargetType(user, "RoundRobinTarget"); + + const createButton = screen.getByText("Create Target").closest("button"); + expect(createButton).toBeDisabled(); + }); + + it("filters duplicate-by-identifier-hash targets out of the picker once one is selected", async () => { + const user = userEvent.setup(); + + // openai_a and openai_a_alias share an identifier_hash — they resolve to the + // same backend config, so once one is picked the other should disappear from + // the dropdown. openai_b has a different hash and stays. + render( + + + + ); + + await selectTargetType(user, "RoundRobinTarget"); + + // Before selecting anything: all three are eligible. + const select = screen.getByText("Select a target to add...").closest("select")!; + expect(select.querySelector('option[value="openai_a"]')).not.toBeNull(); + expect(select.querySelector('option[value="openai_a_alias"]')).not.toBeNull(); + expect(select.querySelector('option[value="openai_b"]')).not.toBeNull(); + + // Pick openai_a. + await user.selectOptions(select, "openai_a"); + + // openai_a_alias (same hash) should now be filtered out, openai_b stays. + expect(select.querySelector('option[value="openai_a_alias"]')).toBeNull(); + expect(select.querySelector('option[value="openai_b"]')).not.toBeNull(); + }); + + it("applies underlying_model_name → model_name fallback when filtering compatible targets", async () => { + const user = userEvent.setup(); + + // foundry_a has no underlying_model_name but model_name='DeepSeek-R1' — the + // backend treats its effective underlying model as 'DeepSeek-R1' via the + // TARGET_EVAL_PARAM_FALLBACKS fallback. foundry_b also lacks underlying_model_name + // but has model_name='Gemini', which is a different effective underlying model. + // foundry_c is a true match: same effective underlying model as foundry_a. + render( + + + + ); + + await selectTargetType(user, "RoundRobinTarget"); + const select = screen.getByText("Select a target to add...").closest("select")!; + await user.selectOptions(select, "foundry_a"); + + // foundry_b's effective underlying model ('Gemini') differs from foundry_a's + // ('DeepSeek-R1') once the model_name fallback is applied, so it must be + // filtered out — the backend would reject the pair with HTTP 400. + expect(select.querySelector('option[value="foundry_b"]')).toBeNull(); + // foundry_c shares the effective underlying model and stays eligible. + expect(select.querySelector('option[value="foundry_c"]')).not.toBeNull(); + }); + + it("surfaces the backend error detail when target creation fails", async () => { + const user = userEvent.setup(); + + // Simulate an axios error with an RFC 7807 detail body — this is what the + // backend returns when, for example, RoundRobinTarget rejects an incompatible + // pair the frontend filter missed. + const axiosError = Object.assign(new Error("Request failed with status code 400"), { + isAxiosError: true, + response: { + status: 400, + data: { + detail: + "Behavioral parameter 'underlying_model_name' differs across targets: target 0 has 'DeepSeek-R1', target 1 has 'gemini-2.0-flash'.", + }, + }, + }); + mockedTargetsApi.createTarget.mockRejectedValueOnce(axiosError); + + render( + + + + ); + + await selectTargetType(user, "RoundRobinTarget"); + const select = screen.getByText("Select a target to add...").closest("select")!; + await user.selectOptions(select, "a"); + await user.selectOptions(select, "b"); + + await user.click(screen.getByText("Create Target")); + + await waitFor(() => { + // The backend's detail (the actual validation message) should be shown to + // the user, not the generic "Request failed with status code 400". + expect(screen.getByText(/Behavioral parameter/)).toBeInTheDocument(); + }); + expect(screen.queryByText(/Request failed with status code 400/)).not.toBeInTheDocument(); + }); + + // =========================================================================== + // RoundRobinTarget weight input validation + // =========================================================================== + + /** + * Helper: render the dialog with two compatible inner targets already + * pickable, then select both — leaves the form in the state where every + * remaining concern is the weight inputs. + */ + async function renderWithTwoRoundRobinTargetsSelected(): Promise<{ + user: ReturnType; + weightInputs: HTMLInputElement[]; + }> { + const user = userEvent.setup(); + render( + + + + ); + await selectTargetType(user, "RoundRobinTarget"); + const select = screen.getByText("Select a target to add...").closest("select")!; + await user.selectOptions(select, "a"); + await user.selectOptions(select, "b"); + // Wait for both weight inputs to actually render — under load, React + // updates can lag behind the userEvent returns and queries return 0 or 1 + // result, causing downstream assertions to operate on stale state. + await waitFor( + () => { + expect(screen.getAllByLabelText(/Weight for /)).toHaveLength(2); + }, + { timeout: 10000 }, + ); + const weightInputs = screen.getAllByLabelText(/Weight for /) as HTMLInputElement[]; + return { user, weightInputs }; + } + + // Note: exhaustive validation of decimal, scientific notation, negative, + // zero, and out-of-range cases is covered by the parseWeight unit tests + // above. These integration tests only verify the UI wiring: that invalid + // input surfaces an alert + disables Create, and that valid input round + // trips through createTarget as parsed ints. + + it("shows an alert and disables Create when a weight is invalid", async () => { + const { user, weightInputs } = await renderWithTwoRoundRobinTargetsSelected(); + + // Use fireEvent.change to bypass HTML5 step="1" constraint that + // userEvent.type would respect. We specifically want to verify our JS + // validation catches values that bypass browser-level checks. + fireEvent.change(weightInputs[0], { target: { value: "2.5" } }); + + // Re-query state under waitFor — under heavy load React commits can lag + // behind fireEvent's return, and stale references won't reflect updates. + await waitFor( + () => { + const inputs = screen.getAllByLabelText(/Weight for /) as HTMLInputElement[]; + expect(inputs[0].value).toBe("2.5"); + expect(inputs[0].getAttribute("aria-invalid")).toBe("true"); + }, + { timeout: 10000 }, + ); + + expect(screen.getByText("Weight must be a whole number")).toBeInTheDocument(); + expect(screen.getByText("Create Target").closest("button")).toBeDisabled(); + + // Pressing Enter inside the weight input must not bypass the disabled + // button and submit the form. + await user.click(screen.getByText("Create Target")); + expect(mockedTargetsApi.createTarget).not.toHaveBeenCalled(); + }, 30000); + + it("submits parsed integer weights when all inputs are valid", async () => { + mockedTargetsApi.createTarget.mockResolvedValueOnce({ + target_registry_name: "rr", + } as unknown as Awaited>); + + const { user, weightInputs } = await renderWithTwoRoundRobinTargetsSelected(); + fireEvent.change(weightInputs[0], { target: { value: "7" } }); + fireEvent.change(weightInputs[1], { target: { value: "42" } }); + + // Wait until both inputs reflect the new values and Create is enabled + // (which only happens once both weights have flushed to state and parsed + // successfully). + await waitFor( + () => { + const inputs = screen.getAllByLabelText(/Weight for /) as HTMLInputElement[]; + expect(inputs[0].value).toBe("7"); + expect(inputs[1].value).toBe("42"); + expect(screen.getByText("Create Target").closest("button")).not.toBeDisabled(); + }, + { timeout: 10000 }, + ); + + await user.click(screen.getByText("Create Target")); + + await waitFor( + () => { + expect(mockedTargetsApi.createTarget).toHaveBeenCalledTimes(1); + }, + { timeout: 10000 }, + ); + const call = mockedTargetsApi.createTarget.mock.calls[0][0]; + expect(call.type).toBe("RoundRobinTarget"); + expect(call.params?.weights).toEqual([7, 42]); + }, 30000); + + it("removes a selected inner target when its delete button is clicked", async () => { + const { user } = await renderWithTwoRoundRobinTargetsSelected(); + + // Both targets show up as selected rows. + expect(screen.getAllByLabelText(/Weight for /)).toHaveLength(2); + + await user.click(screen.getByLabelText("Remove a")); + + await waitFor( + () => { + expect(screen.getAllByLabelText(/Weight for /)).toHaveLength(1); + }, + { timeout: 10000 }, + ); + // With only one inner target left, Create is disabled (needs >= 2). + expect(screen.getByText("Create Target").closest("button")).toBeDisabled(); + }, 30000); + + it("submit-time guards reject invalid weights even if the disabled button is bypassed", async () => { + // Re-validating at submit time defends against pressing Enter inside the + // weight input, which submits the form regardless of the button's disabled + // state. We exercise that branch directly by submitting the form element. + const { weightInputs } = await renderWithTwoRoundRobinTargetsSelected(); + + fireEvent.change(weightInputs[0], { target: { value: "2.5" } }); + await waitFor( + () => { + expect(screen.getByText("Create Target").closest("button")).toBeDisabled(); + }, + { timeout: 10000 }, + ); + + // Find the form (the dialog wraps the fields in a
) and dispatch + // submit directly to simulate Enter-key submission bypassing the button. + const form = weightInputs[0].closest("form")!; + fireEvent.submit(form); + + // Error surfaces in the dialog's top-level MessageBar, and the API is not + // called. + await waitFor( + () => { + expect( + screen.getByText(/Invalid weight for "a": Weight must be a whole number\./), + ).toBeInTheDocument(); + }, + { timeout: 10000 }, + ); + expect(mockedTargetsApi.createTarget).not.toHaveBeenCalled(); + }, 30000); + + it("submit-time guard rejects submission with fewer than 2 selected inner targets", async () => { + // Same bypass scenario as above, but for the "need at least 2" guard. + const { user, weightInputs } = await renderWithTwoRoundRobinTargetsSelected(); + await user.click(screen.getByLabelText("Remove b")); + await waitFor( + () => { + expect(screen.getAllByLabelText(/Weight for /)).toHaveLength(1); + }, + { timeout: 10000 }, + ); + + const form = weightInputs[0].closest("form")!; + fireEvent.submit(form); + + await waitFor( + () => { + expect(screen.getByText("Please select at least 2 targets.")).toBeInTheDocument(); + }, + { timeout: 10000 }, + ); + expect(mockedTargetsApi.createTarget).not.toHaveBeenCalled(); + }, 30000); }); diff --git a/frontend/src/components/Config/CreateTargetDialog.tsx b/frontend/src/components/Config/CreateTargetDialog.tsx index 7cd1e66363..ba2b13b4c7 100644 --- a/frontend/src/components/Config/CreateTargetDialog.tsx +++ b/frontend/src/components/Config/CreateTargetDialog.tsx @@ -1,4 +1,4 @@ -import { useState } from 'react' +import { useState, useEffect, useMemo } from 'react' import { Dialog, DialogSurface, @@ -20,11 +20,15 @@ import { MessageBar, MessageBarBody, } from '@fluentui/react-components' +import { DeleteRegular } from '@fluentui/react-icons' import { targetsApi } from '@/services/api' +import { toApiError } from '@/services/errors' +import type { TargetInstance } from '@/types' import { useCreateTargetDialogStyles } from './CreateTargetDialog.styles' +import { MAX_WEIGHT, parseWeight } from './weightValidation' interface TargetTypeConfig { - readonly kind: 'openai' | 'azureml' + readonly kind: 'openai' | 'azureml' | 'roundrobin' readonly supportsEntra: boolean } @@ -36,6 +40,7 @@ const TARGET_TYPE_CONFIG: Record = { OpenAITTSTarget: { kind: 'openai', supportsEntra: true }, OpenAIResponseTarget: { kind: 'openai', supportsEntra: true }, AzureMLChatTarget: { kind: 'azureml', supportsEntra: true }, + RoundRobinTarget: { kind: 'roundrobin', supportsEntra: false }, } const SUPPORTED_TARGET_TYPES = Object.keys(TARGET_TYPE_CONFIG) @@ -78,9 +83,62 @@ interface CreateTargetDialogProps { open: boolean onClose: () => void onCreated: () => void + /** Existing targets, passed from the parent to avoid a redundant API call. */ + existingTargets?: TargetInstance[] } -export default function CreateTargetDialog({ open, onClose, onCreated }: CreateTargetDialogProps) { +/** State for one selected inner target in the RoundRobinTarget form. */ +interface SelectedInnerTarget { + readonly registryName: string + /** + * Raw text the user has typed into the weight input. May be transiently + * invalid (empty, "2.5", "99999999999", etc.) — the canonical numeric weight + * is derived on demand via {@link parseWeight}. Keeping the raw string as + * the single source of truth avoids the "user types 0 and the field + * silently reverts" UX bug. + */ + weightInput: string +} + +/** + * Resolve the effective underlying model for compatibility checks. + * + * Mirrors the backend's TARGET_EVAL_PARAM_FALLBACKS rule: when + * underlying_model_name is empty (null, undefined, or empty string), fall back + * to model_name. Without this fallback, two targets with the same model_name + * but no underlying_model_name would compare as compatible on the frontend + * even though they resolve to different actual models on the backend (or vice + * versa), so the user would only see the failure as an opaque HTTP 400. + * + * Keep in sync with TARGET_EVAL_PARAM_FALLBACKS in + * pyrit/models/identifiers/evaluation_identifier.py — the + * TestFrontendBackendCompatibilitySync test guards against drift. + */ +function effectiveUnderlyingModel(t: TargetInstance): string | null { + return t.underlying_model_name || t.model_name || null +} + +/** + * Check if two targets are compatible for grouping in a RoundRobinTarget. + * + * Must match the behavioral params that RoundRobinTarget validates on the backend: + * same target_type + TARGET_EVAL_PARAMS (underlying_model_name, temperature, top_p), + * with the underlying_model_name → model_name fallback from TARGET_EVAL_PARAM_FALLBACKS. + * + * NOTE: model_name itself is intentionally NOT compared directly — inner targets can + * have different deployment names as long as the underlying model is the same. + * Keep this in sync with RoundRobinTarget._validate_behavioral_consistency. + */ +function isCompatible(a: TargetInstance, b: TargetInstance): boolean { + return ( + a.target_type === b.target_type && + effectiveUnderlyingModel(a) === effectiveUnderlyingModel(b) && + (a.temperature ?? null) === (b.temperature ?? null) && + (a.top_p ?? null) === (b.top_p ?? null) + ) +} + +export default function CreateTargetDialog({ open, onClose, onCreated, existingTargets }: CreateTargetDialogProps) { const styles = useCreateTargetDialogStyles() const [targetType, setTargetType] = useState('') const [endpoint, setEndpoint] = useState('') @@ -97,7 +155,14 @@ export default function CreateTargetDialog({ open, onClose, onCreated }: CreateT const [error, setError] = useState(null) const [fieldErrors, setFieldErrors] = useState<{ targetType?: string; endpoint?: string }>({}) + // --- RoundRobin-specific state --- + // The list of targets available for selection (fetched once when dialog opens). + const [availableTargets, setAvailableTargets] = useState([]) + // Targets the user has picked for the RoundRobinTarget, with their weights. + const [selectedInnerTargets, setSelectedInnerTargets] = useState([]) + const targetConfig = TARGET_TYPE_CONFIG[targetType] + const isRoundRobin = targetConfig?.kind === 'roundrobin' const isAzureML = targetConfig?.kind === 'azureml' const isOpenAi = targetConfig?.kind === 'openai' const supportsEntra = targetConfig?.supportsEntra ?? false @@ -115,6 +180,66 @@ export default function CreateTargetDialog({ open, onClose, onCreated }: CreateT })() const showEntraEndpointError = entraEndpointError !== null + // Fetch the available targets when the dialog opens with RoundRobin selected. + // If the parent already passed targets, use those to avoid a redundant API call. + useEffect(() => { + if (!open || !isRoundRobin) return + if (existingTargets && existingTargets.length > 0) { + setAvailableTargets(existingTargets) + return + } + let cancelled = false + targetsApi.listTargets(200).then((res) => { + if (!cancelled) setAvailableTargets(res.items) + }).catch(() => { + // Ignore fetch errors — the list will just be empty + }) + return () => { cancelled = true } + }, [open, isRoundRobin, existingTargets]) + + // Compute which targets are eligible to be added next, based on compatibility + // with the first selected target. We also exclude RoundRobinTargets (no nesting), + // already-selected targets, and any target whose identifier_hash matches one already + // selected (different registry names that resolve to the same backend config). + const eligibleTargets = useMemo(() => { + // Targets the user has already selected — exclude by registry name AND by hash so + // aliases pointing at the same underlying endpoint don't show up as separate options. + const selectedNames = new Set(selectedInnerTargets.map((t) => t.registryName)) + const selectedHashes = new Set( + selectedInnerTargets + .map((sel) => availableTargets.find((t) => t.target_registry_name === sel.registryName)?.identifier_hash) + .filter((h): h is string => Boolean(h)), + ) + const candidates = availableTargets.filter( + (t) => + t.target_type !== 'RoundRobinTarget' && + !selectedNames.has(t.target_registry_name) && + !(t.identifier_hash && selectedHashes.has(t.identifier_hash)), + ) + // If nothing is selected yet, all non-RRT candidates are eligible + if (selectedInnerTargets.length === 0) return candidates + // Otherwise, filter to only targets compatible with the first one + const firstSelected = availableTargets.find( + (t) => t.target_registry_name === selectedInnerTargets[0].registryName, + ) + if (!firstSelected) return candidates + return candidates.filter((t) => isCompatible(firstSelected, t)) + }, [availableTargets, selectedInnerTargets]) + + const addInnerTarget = (registryName: string) => { + setSelectedInnerTargets((prev) => [...prev, { registryName, weightInput: '1' }]) + } + + const removeInnerTarget = (registryName: string) => { + setSelectedInnerTargets((prev) => prev.filter((t) => t.registryName !== registryName)) + } + + const setInnerTargetWeightInput = (registryName: string, weightInput: string) => { + setSelectedInnerTargets((prev) => + prev.map((t) => (t.registryName === registryName ? { ...t, weightInput } : t)), + ) + } + const resetForm = () => { setTargetType('') setEndpoint('') @@ -129,6 +254,7 @@ export default function CreateTargetDialog({ open, onClose, onCreated }: CreateT setRepetitionPenalty('1.0') setError(null) setFieldErrors({}) + setSelectedInnerTargets([]) } const handleClose = () => { @@ -137,6 +263,48 @@ export default function CreateTargetDialog({ open, onClose, onCreated }: CreateT } const handleSubmit = async () => { + // For RoundRobinTarget, validation is different: we need ≥2 selected targets, not endpoint + if (isRoundRobin) { + if (selectedInnerTargets.length < 2) { + setError('Please select at least 2 targets.') + return + } + // Re-validate every weight at submit time. The Submit button's disabled + // state usually catches this, but pressing Enter inside the weight input + // triggers the form's onSubmit handler, bypassing the button. + const parsedWeights: number[] = [] + for (const t of selectedInnerTargets) { + const parsed = parseWeight(t.weightInput) + if (!parsed.ok) { + setError(`Invalid weight for "${t.registryName}": ${parsed.error}.`) + return + } + parsedWeights.push(parsed.value) + } + + setSubmitting(true) + setError(null) + + try { + await targetsApi.createTarget({ + type: 'RoundRobinTarget', + params: { + target_registry_names: selectedInnerTargets.map((t) => t.registryName), + weights: parsedWeights, + }, + }) + resetForm() + onCreated() + } catch (err) { + // Surface the backend's RFC 7807 `detail` (e.g. RoundRobinTarget validation + // messages) rather than the generic axios "Request failed with status code 400". + setError(toApiError(err).detail) + } finally { + setSubmitting(false) + } + return + } + const errors: { targetType?: string; endpoint?: string } = {} if (!targetType) errors.targetType = 'Please select a target type' if (!endpoint) errors.endpoint = 'Please provide an endpoint URL' @@ -174,14 +342,13 @@ export default function CreateTargetDialog({ open, onClose, onCreated }: CreateT params, ...(isEntra ? { auth_mode: 'entra' as const } : {}), }) + resetForm() onCreated() } catch (err) { - if (err instanceof Error) { - setError(err.message) - } else { - setError('Failed to create target') - } + // Surface the backend's RFC 7807 `detail` (e.g. RoundRobinTarget validation + // messages) rather than the generic axios "Request failed with status code 400". + setError(toApiError(err).detail) } finally { setSubmitting(false) } @@ -223,6 +390,106 @@ export default function CreateTargetDialog({ open, onClose, onCreated }: CreateT + {/* === RoundRobinTarget form: select existing targets === */} + {isRoundRobin && ( + <> + + + + + {selectedInnerTargets.length > 0 && ( +
+ +
+ {selectedInnerTargets.map((sel) => { + const target = availableTargets.find( + (t) => t.target_registry_name === sel.registryName, + ) + const weightParse = parseWeight(sel.weightInput) + const weightError = weightParse.ok ? null : weightParse.error + return ( +
+
+
+ + {target?.target_registry_name ?? sel.registryName} + {target?.model_name ? ` (${target.model_name})` : ''} + +
+ + + setInnerTargetWeightInput(sel.registryName, data.value) + } + /> +
+
+ {weightError && ( + + {weightError} + + )} +
+
+ ) + })} +
+
+ )} + + )} + + {/* === Standard target form fields (hidden for RoundRobin) === */} + {!isRoundRobin && ( + <> )} + {/* Close the !isRoundRobin conditional wrapper */} + + )} + + {!isRoundRobin && ( + )} @@ -362,7 +635,14 @@ export default function CreateTargetDialog({ open, onClose, onCreated }: CreateT diff --git a/frontend/src/components/Config/TargetConfig.tsx b/frontend/src/components/Config/TargetConfig.tsx index 80e3419c84..d9b858d3b4 100644 --- a/frontend/src/components/Config/TargetConfig.tsx +++ b/frontend/src/components/Config/TargetConfig.tsx @@ -135,6 +135,7 @@ export default function TargetConfig({ activeTarget, onSetActiveTarget }: Target open={dialogOpen} onClose={() => setDialogOpen(false)} onCreated={handleTargetCreated} + existingTargets={targets} />
) diff --git a/frontend/src/components/Config/TargetTable.styles.ts b/frontend/src/components/Config/TargetTable.styles.ts index 906db7578c..23ce7d25c6 100644 --- a/frontend/src/components/Config/TargetTable.styles.ts +++ b/frontend/src/components/Config/TargetTable.styles.ts @@ -72,4 +72,10 @@ export const useTargetTableStyles = makeStyles({ helpHeader: { cursor: 'help', }, + /** Sub-row for inner targets of a RoundRobinTarget — visually indented with a + * lighter background so it's clear these are children, not standalone targets. */ + innerTargetRow: { + backgroundColor: tokens.colorNeutralBackground2, + opacity: 0.85, + }, }) diff --git a/frontend/src/components/Config/TargetTable.test.tsx b/frontend/src/components/Config/TargetTable.test.tsx index 51289614ec..17408d6d3c 100644 --- a/frontend/src/components/Config/TargetTable.test.tsx +++ b/frontend/src/components/Config/TargetTable.test.tsx @@ -344,4 +344,57 @@ describe('TargetTable', () => { expect(screen.queryByText('Filter by type:')).not.toBeInTheDocument() }) + + it('should show expand button for RoundRobinTarget with inner targets', () => { + const rrTarget: TargetInstance = { + target_registry_name: 'rr_gpt4o', + target_type: 'RoundRobinTarget', + model_name: 'gpt-4o', + target_specific_params: { weights: [1, 1] }, + inner_targets: [ + { + target_registry_name: 'inner_a', + target_type: 'OpenAIChatTarget', + endpoint: 'https://a.openai.azure.com', + model_name: 'gpt-4o', + }, + { + target_registry_name: 'inner_b', + target_type: 'OpenAIChatTarget', + endpoint: 'https://b.openai.azure.com', + model_name: 'gpt-4o', + }, + ], + } + + render( + + + + ) + + // Expand button should be present + const expandButton = screen.getByLabelText('Expand inner targets') + expect(expandButton).toBeInTheDocument() + + // Inner targets are not visible before expanding + expect(screen.queryByText('https://a.openai.azure.com')).not.toBeInTheDocument() + + // Click to expand + fireEvent.click(expandButton) + + // Inner targets should now be visible + expect(screen.getByText('https://a.openai.azure.com')).toBeInTheDocument() + expect(screen.getByText('https://b.openai.azure.com')).toBeInTheDocument() + }) + + it('should not show expand button for regular targets', () => { + render( + + + + ) + + expect(screen.queryByLabelText('Expand inner targets')).not.toBeInTheDocument() + }) }) diff --git a/frontend/src/components/Config/TargetTable.tsx b/frontend/src/components/Config/TargetTable.tsx index ba7b224d4e..79d1df3777 100644 --- a/frontend/src/components/Config/TargetTable.tsx +++ b/frontend/src/components/Config/TargetTable.tsx @@ -1,4 +1,4 @@ -import { useState, useMemo, forwardRef } from 'react' +import React, { useState, useMemo, forwardRef } from 'react' import { Table, TableHeader, @@ -26,6 +26,8 @@ import { MathFormulaRegular, WrenchRegular, ArrowHookUpLeftRegular, + ChevronRightRegular, + ChevronDownRegular, } from '@fluentui/react-icons' import type { TargetInstance } from '../../types' import { useTargetTableStyles } from './TargetTable.styles' @@ -191,9 +193,72 @@ function CapabilityCells({ target }: { target: TargetInstance }) { ) } +/** Render expandable sub-rows for a RoundRobinTarget's inner targets. + * Reused by both the active target summary and the main table. */ +function InnerTargetRows({ parentKey, innerTargets, weights }: { + parentKey: string + innerTargets: TargetInstance[] + weights: number[] | undefined +}) { + const styles = useTargetTableStyles() + return ( + <> + {innerTargets.map((inner, idx) => ( + + + #{idx + 1} + + + {inner.target_type} + + + + + + + {inner.endpoint || '—'} + + + + + + + + + + + + {weights?.[idx] != null ? `weight: ${weights[idx]}` : '—'} + + + + ))} + + ) +} + export default function TargetTable({ targets, activeTarget, onSetActiveTarget }: TargetTableProps) { const styles = useTargetTableStyles() const [typeFilter, setTypeFilter] = useState('') + // Tracks which RoundRobinTarget rows are expanded to show inner targets. + // We use a Set of target_registry_name strings — when a name is in the set, + // that row's sub-rows are visible. + const [expandedRows, setExpandedRows] = useState>(new Set()) + + const toggleExpanded = (registryName: string) => { + setExpandedRows((prev) => { + const next = new Set(prev) + if (next.has(registryName)) { + next.delete(registryName) + } else { + next.add(registryName) + } + return next + }) + } + + const hasInnerTargets = (target: TargetInstance): boolean => + (target.inner_targets ?? []).length > 0 const targetTypes = useMemo( () => Array.from(new Set(targets.map(t => t.target_type))).sort(), @@ -218,7 +283,18 @@ export default function TargetTable({ targets, activeTarget, onSetActiveTarget } }>Active - {activeTarget.target_type} +
+ {hasInnerTargets(activeTarget) && ( +
@@ -241,6 +317,14 @@ export default function TargetTable({ targets, activeTarget, onSetActiveTarget } + {/* Expandable sub-rows for the active target summary */} + {expandedRows.has(activeTarget.target_registry_name) && activeTarget.inner_targets && ( + + )} )} @@ -305,51 +389,80 @@ export default function TargetTable({ targets, activeTarget, onSetActiveTarget } - {filteredTargets.map((target) => ( - - - {isActive(target) ? ( - }> - Active - - ) : ( - + {filteredTargets.map((target) => { + const expanded = expandedRows.has(target.target_registry_name) + const expandable = hasInnerTargets(target) + // Extract weights from target_specific_params so we can show per-inner-target weight + const weights = target.target_specific_params?.weights as number[] | undefined + + return ( + + + + {isActive(target) ? ( + }> + Active + + ) : ( + + )} + + +
+ {/* Chevron in the Type column keeps the action column aligned */} + {expandable && ( +
+ + + + + + + {target.endpoint || '—'} + + + + + + + + + + + + {formatParams(target.target_specific_params) || '—'} + + + + + {/* Sub-rows for each inner target, visible when the parent row is expanded */} + {expanded && target.inner_targets && ( + )} - - - {target.target_type} - - - - - - - {target.endpoint || '—'} - - - - - - - - - - - - {formatParams(target.target_specific_params) || '—'} - - - - ))} + + ) + })} diff --git a/frontend/src/components/Config/weightValidation.ts b/frontend/src/components/Config/weightValidation.ts new file mode 100644 index 0000000000..b42baabef7 --- /dev/null +++ b/frontend/src/components/Config/weightValidation.ts @@ -0,0 +1,35 @@ +/** + * Helpers for validating per-target round-robin weight inputs. + * + * Lives in its own module (rather than inline in CreateTargetDialog) so the + * pure-function parser can be exported and unit-tested without breaking React + * Fast Refresh on the dialog component. + */ + +// Cap for per-target round-robin weights. RoundRobinTarget itself accepts any +// positive int, but in the UI a typo like 99999999999 is almost certainly not +// what the user meant — and the backend round-robin scheduler allocates work +// roughly in proportion to weights, so values above a few hundred have no +// practical effect anyway. Tune up if a real use case needs it. +export const MAX_WEIGHT = 1000 + +/** Result of parsing a weight input string. */ +export type WeightParse = { ok: true; value: number } | { ok: false; error: string } + +/** + * Strict weight parser used for both inline validation and submit-time validation. + * + * Rejects empty input, non-integers (including ``2.5`` and scientific notation + * like ``1e10`` that ``parseInt`` would otherwise silently truncate), values + * below 1, and values above {@link MAX_WEIGHT}. The returned ``error`` is + * shown to the user verbatim. + */ +export function parseWeight(raw: string): WeightParse { + if (raw === '') return { ok: false, error: 'Weight is required' } + // Anchored digits-only regex rejects '2.5', '1e10', '-3', whitespace, etc. + if (!/^\d+$/.test(raw)) return { ok: false, error: 'Weight must be a whole number' } + const value = parseInt(raw, 10) + if (value < 1) return { ok: false, error: 'Weight must be at least 1' } + if (value > MAX_WEIGHT) return { ok: false, error: `Weight must be at most ${MAX_WEIGHT}` } + return { ok: true, value } +} diff --git a/frontend/src/types/index.ts b/frontend/src/types/index.ts index f3834dca67..1c6dcc283e 100644 --- a/frontend/src/types/index.ts +++ b/frontend/src/types/index.ts @@ -80,6 +80,11 @@ export interface TargetInstance { max_requests_per_minute?: number | null capabilities?: TargetCapabilitiesInfo | null target_specific_params?: Record | null + /** Inner targets for composite targets like RoundRobinTarget. */ + inner_targets?: TargetInstance[] | null + /** ComponentIdentifier content hash. Targets with the same hash resolve to the + * same backend configuration and are treated as duplicates for RoundRobinTarget grouping. */ + identifier_hash?: string | null } export interface TargetListResponse { diff --git a/pyrit/backend/mappers/target_mappers.py b/pyrit/backend/mappers/target_mappers.py index caf045aa17..b7f715aca0 100644 --- a/pyrit/backend/mappers/target_mappers.py +++ b/pyrit/backend/mappers/target_mappers.py @@ -8,6 +8,7 @@ from pyrit.backend.models.targets import TargetCapabilitiesInfo, TargetInstance from pyrit.prompt_target import PromptTarget from pyrit.prompt_target.common.target_capabilities import CapabilityName, TargetCapabilities +from pyrit.prompt_target.round_robin_target import RoundRobinTarget # Capability flag names that should never be surfaced as identifier-level params: # they are sourced from `target_obj.capabilities` instead. @@ -77,15 +78,57 @@ def target_object_to_instance(target_registry_name: str, target_obj: PromptTarge extra = {k: v for k, v in params.items() if k not in extracted_keys and v is not None} combined_specific = {**extra, **explicit_specific} or None + inner_targets = _build_inner_targets(target_obj) + + # For composite targets (RoundRobinTarget), hoist model_name from inner targets + # only when ALL inner targets share the same deployment name. When they differ + # (e.g. "gpt-4o-japan-nilfilter" vs "pyrit-github-gpt4"), show "—" for + # consistency with how other targets display model_name. + model_name = params.get("model_name") or None + underlying_model_name = params.get("underlying_model_name") or None + if model_name is None and inner_targets: + inner_models = {t.model_name for t in inner_targets} + model_name = inner_models.pop() if len(inner_models) == 1 else None + if underlying_model_name is None and inner_targets: + inner_underlying = {t.underlying_model_name for t in inner_targets} + underlying_model_name = inner_underlying.pop() if len(inner_underlying) == 1 else None + return TargetInstance( target_registry_name=target_registry_name, target_type=identifier.class_name, endpoint=params.get("endpoint") or None, - model_name=params.get("model_name") or None, - underlying_model_name=params.get("underlying_model_name") or None, + model_name=model_name, + underlying_model_name=underlying_model_name, temperature=params.get("temperature"), top_p=params.get("top_p"), max_requests_per_minute=params.get("max_requests_per_minute"), capabilities=_target_capabilities_to_info(target_obj.capabilities), target_specific_params=combined_specific, + inner_targets=inner_targets, + identifier_hash=identifier.hash, ) + + +def _build_inner_targets(target_obj: PromptTarget) -> list[TargetInstance] | None: + """ + Build inner target DTOs for composite targets like RoundRobinTarget. + + For non-composite targets, returns None. For RoundRobinTarget, recursively + maps each inner target into a TargetInstance using the inner target's + ``unique_name`` as the registry name (inner targets may not be independently + registered in the registry, so we derive the name from the identifier). + + Args: + target_obj: The domain PromptTarget object. + + Returns: + A list of inner TargetInstance DTOs, or None for non-composite targets. + """ + if not isinstance(target_obj, RoundRobinTarget): + return None + + inner_instances: list[TargetInstance] = [] + for inner_target in target_obj._targets: + inner_name = inner_target.get_identifier().unique_name + inner_instances.append(target_object_to_instance(inner_name, inner_target)) + return inner_instances diff --git a/pyrit/backend/models/targets.py b/pyrit/backend/models/targets.py index 944fbc358f..9e5ce0c56d 100644 --- a/pyrit/backend/models/targets.py +++ b/pyrit/backend/models/targets.py @@ -64,6 +64,10 @@ class TargetInstance(BaseModel): max_requests_per_minute: int | None = Field(None, description="Maximum requests per minute") capabilities: TargetCapabilitiesInfo = Field(..., description="Structured snapshot of target capabilities") target_specific_params: dict[str, Any] | None = Field(None, description="Additional target-specific parameters") + inner_targets: list["TargetInstance"] | None = Field( + None, description="Inner targets for composite targets like RoundRobinTarget" + ) + identifier_hash: str | None = Field(None, description="ComponentIdentifier content hash for duplicate detection") class TargetListResponse(BaseModel): diff --git a/pyrit/backend/services/target_service.py b/pyrit/backend/services/target_service.py index 3995213075..66e5ccf9d3 100644 --- a/pyrit/backend/services/target_service.py +++ b/pyrit/backend/services/target_service.py @@ -30,6 +30,7 @@ from pyrit.prompt_target import PromptTarget from pyrit.prompt_target.azure_ml_chat_target import AzureMLChatTarget from pyrit.prompt_target.openai.openai_target import OpenAITarget +from pyrit.prompt_target.round_robin_target import RoundRobinTarget from pyrit.registry.object_registries import TargetRegistry logger = logging.getLogger(__name__) @@ -259,24 +260,106 @@ async def create_target_async(self, *, request: CreateTargetRequest) -> TargetIn - Entra ID auth is requested but the target type does not support it; - Entra ID auth is requested for an OpenAI target or AzureMLChatTarget but the endpoint is not valid (not managed by correct hosts); - - If auth_mode='api_key' is set for a target but no key is supplied + - If auth_mode='api_key' is set for a target but no key is supplied; + - For RoundRobinTarget: if target_registry_names are missing, any name + is not found, or inner targets fail compatibility checks. """ target_class = self._get_target_class(target_type=request.type) - # Copy params so we can modify values (eg api_key) without changing request.params. - params: dict[str, Any] = dict(request.params) - - if request.auth_mode == "entra": - params = self._apply_entra_auth(target_class=target_class, target_type=request.type, params=params) + # RoundRobinTarget needs special handling: the user passes registry names + # of existing targets, and we resolve them to live objects. + if request.type == "RoundRobinTarget": + target_obj = self._create_round_robin_target(params=dict(request.params)) else: - self._validate_api_key_auth(target_class=target_class, params=params) + # Copy params so we can modify values (eg api_key) without changing request.params. + params: dict[str, Any] = dict(request.params) + + if request.auth_mode == "entra": + params = self._apply_entra_auth(target_class=target_class, target_type=request.type, params=params) + else: + self._validate_api_key_auth(target_class=target_class, params=params) + + target_obj = target_class(**params) - target_obj = target_class(**params) self._registry.register_instance(target_obj) target_registry_name = target_obj.get_identifier().unique_name return self._build_instance_from_object(target_registry_name=target_registry_name, target_obj=target_obj) + def _create_round_robin_target(self, *, params: dict[str, Any]) -> RoundRobinTarget: + """ + Resolve registry names to target objects and create a RoundRobinTarget. + + Targets resolving to the same ``ComponentIdentifier.hash`` are deduplicated + before construction (mirroring ``TargetInitializer._auto_group_targets``) + so duplicate registry aliases for the same underlying endpoint do not + produce a rotation that hits one target twice. If fewer than 2 distinct + targets remain after dedup, a ``ValueError`` is raised. + + The RoundRobinTarget constructor validates all compatibility requirements + (same class, same configuration, same behavioral params, ≥2 targets). + + Args: + params: Must contain ``target_registry_names`` (list of registry name + strings). May contain ``weights`` (list of positive ints) of the + same length as ``target_registry_names``; weights for deduped + entries are dropped along with their target. + + Returns: + A new RoundRobinTarget wrapping the resolved (deduped) targets. + + Raises: + ValueError: If fewer than 2 names are supplied, a name is not found + in the registry, weights length does not match, dedup leaves + fewer than 2 distinct targets, or the RoundRobinTarget + constructor rejects the combination. + """ + registry_names: list[str] = params.get("target_registry_names", []) + if len(registry_names) < 2: + raise ValueError("RoundRobinTarget requires at least 2 target_registry_names in params.") + + raw_weights: list[int] | None = params.get("weights") or None + if raw_weights is not None and len(raw_weights) != len(registry_names): + raise ValueError( + f"weights length ({len(raw_weights)}) must match target_registry_names length ({len(registry_names)})." + ) + + # Deduplicate by ComponentIdentifier hash: two registry entries that + # resolve to the same identifier (same endpoint, model, api_version, etc.) + # would just hit the same target twice in the rotation. This mirrors the + # dedup in TargetInitializer._auto_group_targets so user-driven and + # auto-grouped flows behave the same. + seen_hashes: set[str | None] = set() + resolved_targets: list[PromptTarget] = [] + resolved_weights: list[int] = [] + duplicates: list[str] = [] + for idx, name in enumerate(registry_names): + target_obj = self._registry.get_instance_by_name(name) + if target_obj is None: + raise ValueError(f"Target '{name}' not found in the registry.") + target_hash = target_obj.get_identifier().hash + if target_hash in seen_hashes: + duplicates.append(name) + logger.debug(f"Skipping duplicate target '{name}' (hash {target_hash}) in RoundRobinTarget creation") + continue + seen_hashes.add(target_hash) + resolved_targets.append(target_obj) + if raw_weights is not None: + resolved_weights.append(raw_weights[idx]) + + if len(resolved_targets) < 2: + raise ValueError( + f"RoundRobinTarget requires at least 2 distinct targets, but the provided names " + f"resolved to {len(resolved_targets)} unique target(s) after deduplication. " + f"Duplicate names skipped: {duplicates}. Please select targets with different " + f"endpoints or configurations." + ) + + weights = resolved_weights if raw_weights is not None else None + + # The constructor validates same-class, same-config, behavioral consistency, etc. + return RoundRobinTarget(targets=resolved_targets, weights=weights) + @staticmethod def _apply_entra_auth(*, target_class: type, target_type: str, params: dict[str, Any]) -> dict[str, Any]: """ diff --git a/pyrit/setup/initializers/components/targets.py b/pyrit/setup/initializers/components/targets.py index 017bb873bf..098c2997cd 100644 --- a/pyrit/setup/initializers/components/targets.py +++ b/pyrit/setup/initializers/components/targets.py @@ -713,8 +713,25 @@ def _auto_group_targets(self) -> None: if len(members) < 2: continue - member_names = [name for name, _ in members] - member_targets = [target for _, target in members] + # Deduplicate: targets with identical ComponentIdentifier hashes have + # the exact same config (endpoint, model, api_version, etc.) so including + # both in a round-robin just wastes a rotation slot. Keep the first + # occurrence of each hash. + seen_hashes: set[str | None] = set() + unique_members: list[tuple[str, PromptTarget]] = [] + for name, target in members: + target_hash = target.get_identifier().hash + if target_hash in seen_hashes: + logger.debug(f"Skipping duplicate target '{name}' (hash {target_hash}) in auto-group for key {key}") + continue + seen_hashes.add(target_hash) + unique_members.append((name, target)) + + if len(unique_members) < 2: + continue + + member_names = [name for name, _ in unique_members] + member_targets = [target for _, target in unique_members] try: rr_target = RoundRobinTarget(targets=member_targets) diff --git a/tests/unit/backend/test_mappers.py b/tests/unit/backend/test_mappers.py index 2d6e4da37c..b5df06aa09 100644 --- a/tests/unit/backend/test_mappers.py +++ b/tests/unit/backend/test_mappers.py @@ -1129,6 +1129,11 @@ def test_maps_target_with_identifier(self) -> None: assert result.endpoint == "http://test" assert result.model_name == "gpt-4" assert result.temperature == 0.7 + # identifier_hash is auto-populated by the ComponentIdentifier validator and + # surfaced on the DTO so the frontend can dedupe targets that resolve to the + # same underlying configuration. + assert result.identifier_hash is not None + assert result.identifier_hash == mock_identifier.hash def test_no_endpoint_returns_none(self) -> None: """Test that missing endpoint returns None.""" @@ -1491,6 +1496,134 @@ def test_target_configuration_excluded_from_target_specific_params(self) -> None assert result.target_specific_params["reasoning_effort"] == "high" +class TestTargetObjectToInstanceRoundRobin: + """Tests for target_object_to_instance with RoundRobinTarget.""" + + def test_round_robin_populates_inner_targets(self) -> None: + """Inner targets list is populated for RoundRobinTarget objects.""" + from pyrit.prompt_target.round_robin_target import RoundRobinTarget + + # Build a mock RoundRobinTarget with two inner targets + rr = MagicMock(spec=RoundRobinTarget) + rr.capabilities = TargetCapabilities(supports_multi_turn=True) + + inner_a = MagicMock(spec=PromptTarget) + inner_a.capabilities = TargetCapabilities() + inner_a.get_identifier.return_value = ComponentIdentifier( + class_name="OpenAIChatTarget", + class_module="pyrit.prompt_target", + params={"endpoint": "https://a.openai.azure.com", "model_name": "gpt-4o"}, + ) + + inner_b = MagicMock(spec=PromptTarget) + inner_b.capabilities = TargetCapabilities() + inner_b.get_identifier.return_value = ComponentIdentifier( + class_name="OpenAIChatTarget", + class_module="pyrit.prompt_target", + params={"endpoint": "https://b.openai.azure.com", "model_name": "gpt-4o"}, + ) + + rr._targets = [inner_a, inner_b] + rr.get_identifier.return_value = ComponentIdentifier( + class_name="RoundRobinTarget", + class_module="pyrit.prompt_target.round_robin_target", + params={"weights": [1, 1]}, + ) + + result = target_object_to_instance("rr-1", rr) + + assert result.target_type == "RoundRobinTarget" + assert result.inner_targets is not None + assert len(result.inner_targets) == 2 + assert result.inner_targets[0].endpoint == "https://a.openai.azure.com" + assert result.inner_targets[1].endpoint == "https://b.openai.azure.com" + + def test_round_robin_hoists_model_name_when_all_inner_targets_match(self) -> None: + """model_name is hoisted only when all inner targets share the same deployment name.""" + from pyrit.prompt_target.round_robin_target import RoundRobinTarget + + rr = MagicMock(spec=RoundRobinTarget) + rr.capabilities = TargetCapabilities() + + inner_a = MagicMock(spec=PromptTarget) + inner_a.capabilities = TargetCapabilities() + inner_a.get_identifier.return_value = ComponentIdentifier( + class_name="OpenAIChatTarget", + class_module="pyrit.prompt_target", + params={"model_name": "gpt-4o", "underlying_model_name": "gpt-4o"}, + ) + + inner_b = MagicMock(spec=PromptTarget) + inner_b.capabilities = TargetCapabilities() + inner_b.get_identifier.return_value = ComponentIdentifier( + class_name="OpenAIChatTarget", + class_module="pyrit.prompt_target", + params={"model_name": "gpt-4o", "underlying_model_name": "gpt-4o"}, + ) + + rr._targets = [inner_a, inner_b] + rr.get_identifier.return_value = ComponentIdentifier( + class_name="RoundRobinTarget", + class_module="pyrit.prompt_target.round_robin_target", + params={"weights": [1, 1]}, + ) + + result = target_object_to_instance("rr-2", rr) + + assert result.model_name == "gpt-4o" + assert result.underlying_model_name == "gpt-4o" + + def test_round_robin_omits_model_name_when_inner_targets_differ(self) -> None: + """model_name is None when inner targets have different deployment names.""" + from pyrit.prompt_target.round_robin_target import RoundRobinTarget + + rr = MagicMock(spec=RoundRobinTarget) + rr.capabilities = TargetCapabilities() + + inner_a = MagicMock(spec=PromptTarget) + inner_a.capabilities = TargetCapabilities() + inner_a.get_identifier.return_value = ComponentIdentifier( + class_name="OpenAIChatTarget", + class_module="pyrit.prompt_target", + params={"model_name": "deploy-japan", "underlying_model_name": "gpt-4o"}, + ) + + inner_b = MagicMock(spec=PromptTarget) + inner_b.capabilities = TargetCapabilities() + inner_b.get_identifier.return_value = ComponentIdentifier( + class_name="OpenAIChatTarget", + class_module="pyrit.prompt_target", + params={"model_name": "deploy-us", "underlying_model_name": "gpt-4o"}, + ) + + rr._targets = [inner_a, inner_b] + rr.get_identifier.return_value = ComponentIdentifier( + class_name="RoundRobinTarget", + class_module="pyrit.prompt_target.round_robin_target", + params={"weights": [1, 1]}, + ) + + result = target_object_to_instance("rr-3", rr) + + # model_name should be None since deployments differ + assert result.model_name is None + # underlying_model_name should still be hoisted (they all share gpt-4o) + assert result.underlying_model_name == "gpt-4o" + + def test_non_round_robin_has_no_inner_targets(self) -> None: + """Regular targets return None for inner_targets.""" + target_obj = MagicMock(spec=PromptTarget) + target_obj.capabilities = TargetCapabilities() + target_obj.get_identifier.return_value = ComponentIdentifier( + class_name="TextTarget", + class_module="pyrit.prompt_target", + ) + + result = target_object_to_instance("t-1", target_obj) + + assert result.inner_targets is None + + # ============================================================================ # Converter Mapper Tests # ============================================================================ diff --git a/tests/unit/backend/test_target_service.py b/tests/unit/backend/test_target_service.py index b996029218..b86467eb88 100644 --- a/tests/unit/backend/test_target_service.py +++ b/tests/unit/backend/test_target_service.py @@ -598,6 +598,169 @@ async def test_create_text_target_api_key_mode_skips_validation(self, sqlite_ins assert result.target_type == "TextTarget" +class TestCreateRoundRobinTarget: + """Tests for creating RoundRobinTarget via the service.""" + + async def test_create_round_robin_target_resolves_registry_names(self, sqlite_instance) -> None: + """RoundRobinTarget creation resolves registry names to live target objects.""" + service = TargetService() + + # Register two mock targets in the registry to serve as inner targets. + # We mock the RoundRobinTarget constructor because it does deep validation + # (same class, multi-turn, editable history) that requires real compatible + # targets. The service's job is to resolve registry names and pass them + # through — the constructor validation is tested in RoundRobinTarget's own tests. + mock_a = MagicMock() + mock_a.get_identifier.return_value = _mock_target_identifier( + class_name="OpenAIChatTarget", endpoint="https://a.openai.azure.com", model_name="gpt-4o" + ) + mock_b = MagicMock() + mock_b.get_identifier.return_value = _mock_target_identifier( + class_name="OpenAIChatTarget", endpoint="https://b.openai.azure.com", model_name="gpt-4o" + ) + service._registry.register_instance(mock_a, name="target-a") + service._registry.register_instance(mock_b, name="target-b") + + # Patch RoundRobinTarget so the constructor returns a mock that behaves + # like a registered target (has get_identifier, capabilities, etc.) + mock_rr = MagicMock() + mock_rr.get_identifier.return_value = ComponentIdentifier( + class_name="RoundRobinTarget", + class_module="pyrit.prompt_target.round_robin_target", + params={"weights": [2, 1]}, + ) + mock_rr._targets = [mock_a, mock_b] + + with patch( + "pyrit.backend.services.target_service.RoundRobinTarget", + return_value=mock_rr, + ) as mock_rr_cls: + rr_request = CreateTargetRequest( + type="RoundRobinTarget", + params={ + "target_registry_names": ["target-a", "target-b"], + "weights": [2, 1], + }, + ) + + result = await service.create_target_async(request=rr_request) + + # Verify the constructor was called with the resolved targets and weights + mock_rr_cls.assert_called_once_with(targets=[mock_a, mock_b], weights=[2, 1]) + assert result.target_type == "RoundRobinTarget" + + async def test_create_round_robin_target_fewer_than_2_raises(self, sqlite_instance) -> None: + """RoundRobinTarget with fewer than 2 registry names raises ValueError.""" + service = TargetService() + + rr_request = CreateTargetRequest( + type="RoundRobinTarget", + params={"target_registry_names": ["only-one"]}, + ) + + with pytest.raises(ValueError, match="at least 2"): + await service.create_target_async(request=rr_request) + + async def test_create_round_robin_target_unknown_name_raises(self, sqlite_instance) -> None: + """RoundRobinTarget with a non-existent registry name raises ValueError.""" + service = TargetService() + + rr_request = CreateTargetRequest( + type="RoundRobinTarget", + params={"target_registry_names": ["does-not-exist-a", "does-not-exist-b"]}, + ) + + with pytest.raises(ValueError, match="not found"): + await service.create_target_async(request=rr_request) + + async def test_create_round_robin_target_deduplicates_identical_targets(self, sqlite_instance) -> None: + """Targets that resolve to the same identifier hash are deduplicated, and + the corresponding weights are dropped alongside them.""" + service = TargetService() + + # mock_a and mock_a_alias share the same identifier params, so their + # ComponentIdentifier.hash is identical — they should dedupe to one entry. + identifier_a = _mock_target_identifier( + class_name="OpenAIChatTarget", endpoint="https://a.openai.azure.com", model_name="gpt-4o" + ) + mock_a = MagicMock() + mock_a.get_identifier.return_value = identifier_a + mock_a_alias = MagicMock() + mock_a_alias.get_identifier.return_value = identifier_a + + mock_b = MagicMock() + mock_b.get_identifier.return_value = _mock_target_identifier( + class_name="OpenAIChatTarget", endpoint="https://b.openai.azure.com", model_name="gpt-4o" + ) + + service._registry.register_instance(mock_a, name="target-a") + service._registry.register_instance(mock_a_alias, name="target-a-alias") + service._registry.register_instance(mock_b, name="target-b") + + mock_rr = MagicMock() + mock_rr.get_identifier.return_value = ComponentIdentifier( + class_name="RoundRobinTarget", + class_module="pyrit.prompt_target.round_robin_target", + params={"weights": [3, 1]}, + ) + mock_rr._targets = [mock_a, mock_b] + + with patch( + "pyrit.backend.services.target_service.RoundRobinTarget", + return_value=mock_rr, + ) as mock_rr_cls: + rr_request = CreateTargetRequest( + type="RoundRobinTarget", + params={ + "target_registry_names": ["target-a", "target-a-alias", "target-b"], + "weights": [3, 2, 1], + }, + ) + + await service.create_target_async(request=rr_request) + + # The duplicate alias and its weight (2) should be dropped. + mock_rr_cls.assert_called_once_with(targets=[mock_a, mock_b], weights=[3, 1]) + + async def test_create_round_robin_target_all_duplicates_raises(self, sqlite_instance) -> None: + """If dedup leaves fewer than 2 distinct targets, raise a clear error.""" + service = TargetService() + + identifier = _mock_target_identifier( + class_name="OpenAIChatTarget", endpoint="https://a.openai.azure.com", model_name="gpt-4o" + ) + mock_a = MagicMock() + mock_a.get_identifier.return_value = identifier + mock_a_alias = MagicMock() + mock_a_alias.get_identifier.return_value = identifier + + service._registry.register_instance(mock_a, name="target-a") + service._registry.register_instance(mock_a_alias, name="target-a-alias") + + rr_request = CreateTargetRequest( + type="RoundRobinTarget", + params={"target_registry_names": ["target-a", "target-a-alias"]}, + ) + + with pytest.raises(ValueError, match="at least 2 distinct targets"): + await service.create_target_async(request=rr_request) + + async def test_create_round_robin_target_weights_length_mismatch_raises(self, sqlite_instance) -> None: + """Mismatched weights length raises before any registry lookups.""" + service = TargetService() + + rr_request = CreateTargetRequest( + type="RoundRobinTarget", + params={ + "target_registry_names": ["a", "b", "c"], + "weights": [1, 2], + }, + ) + + with pytest.raises(ValueError, match="weights length"): + await service.create_target_async(request=rr_request) + + class TestTargetServiceSingleton: """Tests for get_target_service singleton function.""" @@ -615,3 +778,48 @@ def test_get_target_service_returns_same_instance(self) -> None: service1 = get_target_service() service2 = get_target_service() assert service1 is service2 + + +class TestFrontendBackendCompatibilitySync: + """Guard against drift between frontend isCompatible() and backend TARGET_EVAL_PARAMS. + + The frontend pre-filters the Create RoundRobinTarget dropdown using a hardcoded + set of fields (target_type + TARGET_EVAL_PARAMS). If the backend adds a new + behavioral param, this test fails and reminds the developer to update + frontend/src/components/Config/CreateTargetDialog.tsx → isCompatible(). + """ + + def test_target_eval_params_match_frontend_iscompatible(self) -> None: + """TARGET_EVAL_PARAMS must be exactly {underlying_model_name, temperature, top_p}. + + If this fails, someone added or removed a param from TARGET_EVAL_PARAMS. + Update the frontend isCompatible() function in CreateTargetDialog.tsx + to check the same fields, then update the expected set here. + """ + from pyrit.models import TARGET_EVAL_PARAMS + + expected = {"underlying_model_name", "temperature", "top_p"} + assert expected == TARGET_EVAL_PARAMS, ( + f"TARGET_EVAL_PARAMS changed to {TARGET_EVAL_PARAMS}. " + f"Update the frontend isCompatible() in CreateTargetDialog.tsx to match, " + f"then update this test's expected set." + ) + + def test_target_eval_param_fallbacks_match_frontend(self) -> None: + """TARGET_EVAL_PARAM_FALLBACKS must match the fallback rule implemented in + the frontend effectiveUnderlyingModel() helper in CreateTargetDialog.tsx. + + If this fails, someone added or changed a fallback. Update + effectiveUnderlyingModel() (and any sibling resolvers) in + CreateTargetDialog.tsx so the frontend pre-filter agrees with what the + backend RoundRobinTarget._validate_behavioral_consistency check accepts, + then update this test's expected dict. + """ + from pyrit.models import TARGET_EVAL_PARAM_FALLBACKS + + expected = {"underlying_model_name": "model_name"} + assert expected == TARGET_EVAL_PARAM_FALLBACKS, ( + f"TARGET_EVAL_PARAM_FALLBACKS changed to {TARGET_EVAL_PARAM_FALLBACKS}. " + f"Update effectiveUnderlyingModel() in CreateTargetDialog.tsx to match, " + f"then update this test's expected dict." + ) diff --git a/tests/unit/setup/test_targets_initializer.py b/tests/unit/setup/test_targets_initializer.py index 52f275867d..964c7789fb 100644 --- a/tests/unit/setup/test_targets_initializer.py +++ b/tests/unit/setup/test_targets_initializer.py @@ -822,6 +822,48 @@ async def test_auto_group_three_targets_same_model(self) -> None: # Should have 3 inner targets (gpt4o, gpt4o2, unsafe_chat) assert len(rr._targets) == 3 + async def test_auto_group_deduplicates_identical_targets(self) -> None: + """Test that targets with identical config (same endpoint+model+key) are deduplicated in auto-grouping.""" + from pyrit.prompt_target import RoundRobinTarget + + # Set GPT4O_1 and GPT4O_2, but also set a third env var block that points + # to the SAME endpoint+key+model as GPT4O_1. This simulates the real-world + # case where .env has overlapping entries. + os.environ.update(self.GPT4O_1_ENV) + os.environ.update(self.GPT4O_2_ENV) + os.environ.update(self.UNSAFE1_ENV) + + # Register a duplicate: same endpoint, key, model as UNSAFE1 but under a different registry name. + # We do this by directly registering a target with the same params. + from pyrit.prompt_target import OpenAIChatTarget + + dup_target = OpenAIChatTarget( + endpoint="https://unsafe1.openai.azure.com", + api_key="key1", + model_name="gpt-4o", + underlying_model="gpt-4o", + ) + registry = TargetRegistry.get_registry_singleton() + + init = TargetInitializer() + await init.initialize_async() + + # Register the duplicate after init so it's in the registry with a distinct name. + registry.register_instance(dup_target, name="unsafe1_duplicate") + init._registered_names.append("unsafe1_duplicate") + + # Re-run auto-grouping (clear existing RR first) + existing_rr = registry.get_instance_by_name("OpenAIChatTarget_gpt-4o_rr") + if existing_rr: + registry._instances.pop("OpenAIChatTarget_gpt-4o_rr", None) + init._auto_group_targets() + + rr = registry.get_instance_by_name("OpenAIChatTarget_gpt-4o_rr") + assert rr is not None + assert isinstance(rr, RoundRobinTarget) + # Should have 3, not 4 — the duplicate should be deduplicated + assert len(rr._targets) == 3 + class TestGetBehavioralKey: """Tests for _get_behavioral_key helper function."""