From 3600afa5968fc44665ceefe685cf71b106c0fe4b Mon Sep 17 00:00:00 2001 From: Alex Ray Date: Wed, 12 Aug 2026 15:25:05 -0700 Subject: [PATCH] feat(studio): run guardrail checks against an unsaved draft config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Test and Validate tab could only exercise the saved config, so verifying an edit meant saving it first — publishing a half-tested change and writing a config version the user did not want to keep. Add a Draft/Saved run-target control. A Draft run sends the merged form state inline via the /checks endpoint's `config` field, which the service already supports, and resolves the request model from the draft. No backend change is needed. Runs now record what produced them: `is_draft` in place of a config version, and a snapshot of the activated guardrails, so a result keeps describing the config that ran even after the config changes. Both fields are optional, so records written earlier render unchanged. Extract useDraftRailsConfig so the Configuration and checks tabs share one definition of the draft rather than deriving it twice. Signed-off-by: Alex Ray --- .../guardrail-checks/guardrailChecks.test.ts | 69 +++++++++++++++++++ .../api/guardrail-checks/guardrailChecks.ts | 58 +++++++++++++--- .../studio/src/api/guardrail-checks/hooks.ts | 12 ++-- .../studio/src/api/guardrail-checks/types.ts | 20 ++++++ .../RailStatusTab.test.tsx | 30 ++++++++ .../RailStatusTab.tsx | 7 +- .../RunHistoryTab.test.tsx | 37 ++++++++++ .../RunHistoryTab.tsx | 30 ++++++-- .../railLabels.ts | 11 +-- .../studio/src/mocks/handlers/guardrails.ts | 9 ++- .../GuardrailTestCasesEditor.tsx | 43 +++++++++++- .../GuardrailChecksTab/index.test.tsx | 63 ++++++++++++++++- .../guardrails/GuardrailChecksTab/index.tsx | 6 ++ .../guardrails/GuardrailConfigTab/index.tsx | 12 +--- .../GuardrailForm/useDraftRailsConfig.ts | 40 +++++++++++ 15 files changed, 402 insertions(+), 45 deletions(-) create mode 100644 web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RunHistoryTab.test.tsx create mode 100644 web/packages/studio/src/routes/guardrails/GuardrailForm/useDraftRailsConfig.ts diff --git a/web/packages/studio/src/api/guardrail-checks/guardrailChecks.test.ts b/web/packages/studio/src/api/guardrail-checks/guardrailChecks.test.ts index 3f42d68e63..964333f74f 100644 --- a/web/packages/studio/src/api/guardrail-checks/guardrailChecks.test.ts +++ b/web/packages/studio/src/api/guardrail-checks/guardrailChecks.test.ts @@ -133,6 +133,75 @@ describe('runGuardrailCheck', () => { ); expect(recordedCheckRequests).toHaveLength(0); }); + + it('snapshots the saved config coverage onto the run', async () => { + const { run } = await runGuardrailCheck(WORKSPACE, snapshot('benign-greeting')); + + // cfg-1 declares two input and two output flows, none of which the registry + // recognizes — so each falls back to its raw name. + expect(run.activated_guardrails?.map((g) => g.label)).toEqual([ + 'check pii', + 'check toxicity', + 'mask pii output', + 'check output facts', + ]); + expect(run.is_draft).toBeUndefined(); + }); +}); + +describe('runGuardrailCheck against a draft', () => { + /** A draft that differs from cfg-1 in both its model and its rails. */ + const DRAFT: RailsConfig = { + models: [{ type: 'main', engine: 'openai', model: 'gpt-4o-draft' }], + instructions: [{ type: 'general', content: 'Be extremely cautious.' }], + rails: { input: { flows: ['jailbreak detection'] } }, + }; + + it('sends the draft inline instead of referencing the saved config by id', async () => { + await runGuardrailCheck(WORKSPACE, snapshot('benign-greeting'), DRAFT); + + expect(recordedCheckRequests).toEqual([ + { + model: 'gpt-4o-draft', + messages: [{ role: 'user', content: 'Hello there' }], + guardrails: { config: DRAFT }, + }, + ]); + // Never both: the service's validator would silently discard config_ids. + expect(recordedCheckRequests[0]?.guardrails).not.toHaveProperty('config_ids'); + }); + + it('records the run as a draft with no config version', async () => { + const { run } = await runGuardrailCheck(WORKSPACE, snapshot('benign-greeting'), DRAFT); + + expect(run.is_draft).toBe(true); + expect(run.config_version).toBeUndefined(); + expect(getMockGuardrailCheck('benign-greeting')?.data.runs).toEqual([run]); + }); + + it("snapshots the draft's coverage, not the saved config's", async () => { + const { run } = await runGuardrailCheck(WORKSPACE, snapshot('benign-greeting'), DRAFT); + + const labels = run.activated_guardrails?.map((g) => g.label); + expect(labels).toContain('Jailbreak Detection'); + expect(labels).not.toContain('check pii'); + }); + + it("overrides a check's stored guardrails rather than silently ignoring the draft", async () => { + const check = snapshot('benign-greeting'); + check.data.guardrails = { config_ids: ['some-other-config'] }; + + await runGuardrailCheck(WORKSPACE, check, DRAFT); + + expect(recordedCheckRequests[0]?.guardrails).toEqual({ config: DRAFT }); + }); + + it('rejects a draft with no usable model before calling /checks', async () => { + await expect( + runGuardrailCheck(WORKSPACE, snapshot('benign-greeting'), { models: [] }) + ).rejects.toThrow("Guardrail config 'pii-filter' has no usable model to run checks against."); + expect(recordedCheckRequests).toHaveLength(0); + }); }); describe('runGuardrailChecks', () => { diff --git a/web/packages/studio/src/api/guardrail-checks/guardrailChecks.ts b/web/packages/studio/src/api/guardrail-checks/guardrailChecks.ts index 145498f8fb..d3c87a7033 100644 --- a/web/packages/studio/src/api/guardrail-checks/guardrailChecks.ts +++ b/web/packages/studio/src/api/guardrail-checks/guardrailChecks.ts @@ -25,6 +25,9 @@ import { type GuardrailChecksPage, type RunRecord, } from '@studio/api/guardrail-checks/types'; +// Layering wrinkle: this reaches into the components layer. Deliberate — it keeps one +// definition of guardrail identity shared with the config editor's detector catalog. +import { getActivatedGuardrails } from '@studio/components/sidePanels/GuardrailCheckDetailSidePanel/railLabels'; // --------------------------------------------------------------------------- // Query keys @@ -183,18 +186,32 @@ export function resolveConfigModel(config: RailsConfig | undefined, configLabel: return chosen.model; } +/** What the run targeted, for stamping onto the record. */ +export interface RunRecordContext { + /** Parent config db_version. Omitted for a draft run — a draft has no version. */ + configVersion?: number; + /** True when the run targeted an unsaved draft. */ + isDraft?: boolean; + /** The config that actually ran, for the activated-guardrail snapshot. */ + config?: RailsConfig; +} + /** Map a /checks response to a persisted run record. */ export function responseToRunRecord( response: GuardrailCheckResponse, runAt: string, - configVersion?: number + context: RunRecordContext = {} ): RunRecord { return { run_at: runAt, status: response.status, rails_status: response.rails_status, config_ids: response.guardrails_data?.config_ids, - config_version: configVersion, + config_version: context.configVersion, + ...(context.isDraft ? { is_draft: true } : {}), + // Resolved now, not at render time: the config that produced this run may be edited + // (or, for a draft, cease to exist) before anyone opens the result panel. + activated_guardrails: getActivatedGuardrails(context.config, response.rails_status), }; } @@ -213,10 +230,15 @@ export function executeGuardrailCheck( * * Takes the check entity directly (rather than re-fetching by name) because checks are * child entities — name-based lookups must be scoped by `parent`, which the entity carries. + * + * `draftConfig` runs the check against an unsaved edit instead of the saved config: it is + * sent inline (the /checks endpoint accepts a whole RailsConfig, not just an id) and the + * resulting record is marked as a draft run with no config version. */ export async function runGuardrailCheck( workspace: string, - check: GuardrailCheckEntity + check: GuardrailCheckEntity, + draftConfig?: RailsConfig ): Promise<{ entity: GuardrailCheckEntity; run: RunRecord }> { if (!check.parent) { throw new Error( @@ -227,19 +249,32 @@ export async function runGuardrailCheck( const configEntity = await entitiesGetEntityById(check.parent); // A guardrail_config entity nests the rails config under `data.data` // (`data` also carries the config's description). - const configData = (configEntity.data as { data?: RailsConfig }).data; - const model = resolveConfigModel(configData, configEntity.name); + const savedData = (configEntity.data as { data?: RailsConfig }).data; + // Still fetched on the draft path: `persistRun` needs the entity, and its name is the + // label in error messages. + const effectiveConfig = draftConfig ?? savedData; + const model = resolveConfigModel(effectiveConfig, configEntity.name); const request: GuardrailCheckRequest = { model, messages: check.data.messages, - // The check references its config by name (the /checks endpoint resolves config_ids to - // `workspace/name`), unless the check carries explicit guardrails options. - guardrails: check.data.guardrails ?? { config_ids: [configEntity.name] }, + // A draft has no id to reference, so it travels whole. Never both: the service's + // validator nulls out `config_ids` when `config` is an object, which would make the + // request's meaning non-obvious from the wire. + // + // Draft wins over a check's stored `guardrails`: the user explicitly chose the target, + // and silently running something else would be worse than ignoring the override. + guardrails: draftConfig + ? { config: draftConfig } + : (check.data.guardrails ?? { config_ids: [configEntity.name] }), }; const response = await executeGuardrailCheck(workspace, request); - const run = responseToRunRecord(response, new Date().toISOString(), configEntity.db_version); + const run = responseToRunRecord(response, new Date().toISOString(), { + configVersion: draftConfig ? undefined : configEntity.db_version, + isDraft: Boolean(draftConfig), + config: effectiveConfig, + }); const entity = await persistRun(workspace, check, run); @@ -272,12 +307,13 @@ async function persistRun( /** Batch execution — backs the "Re-run N Tests" action. Failures are captured per check. */ export function runGuardrailChecks( workspace: string, - checks: GuardrailCheckEntity[] + checks: GuardrailCheckEntity[], + draftConfig?: RailsConfig ): Promise> { return Promise.all( checks.map(async (check) => { try { - const { run } = await runGuardrailCheck(workspace, check); + const { run } = await runGuardrailCheck(workspace, check, draftConfig); return { name: check.name, run }; } catch (error) { return { diff --git a/web/packages/studio/src/api/guardrail-checks/hooks.ts b/web/packages/studio/src/api/guardrail-checks/hooks.ts index aa64eb9257..7f9ae94b55 100644 --- a/web/packages/studio/src/api/guardrail-checks/hooks.ts +++ b/web/packages/studio/src/api/guardrail-checks/hooks.ts @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import type { EntitiesListEntitiesParams } from '@nemo/sdk/generated/platform/schema'; +import type { EntitiesListEntitiesParams, RailsConfig } from '@nemo/sdk/generated/platform/schema'; import { createGuardrailCheck, type CreateGuardrailCheckInput, @@ -158,7 +158,7 @@ export type UseRunGuardrailCheckOptions = Omit< UseMutationOptions< Awaited>, Error, - { workspace: string; check: GuardrailCheckEntity } + { workspace: string; check: GuardrailCheckEntity; draftConfig?: RailsConfig } >, 'mutationFn' >; @@ -167,7 +167,8 @@ export type UseRunGuardrailCheckOptions = Omit< export const useRunGuardrailCheck = (options?: UseRunGuardrailCheckOptions) => useMutation({ ...options, - mutationFn: ({ workspace, check }) => runGuardrailCheck(workspace, check), + mutationFn: ({ workspace, check, draftConfig }) => + runGuardrailCheck(workspace, check, draftConfig), onSuccess: (...args) => { const [, variables] = args; invalidateGuardrailChecksCaches(variables.workspace, variables.check.name); @@ -179,7 +180,7 @@ export type UseRunGuardrailChecksOptions = Omit< UseMutationOptions< Awaited>, Error, - { workspace: string; checks: GuardrailCheckEntity[] } + { workspace: string; checks: GuardrailCheckEntity[]; draftConfig?: RailsConfig } >, 'mutationFn' >; @@ -188,7 +189,8 @@ export type UseRunGuardrailChecksOptions = Omit< export const useRunGuardrailChecks = (options?: UseRunGuardrailChecksOptions) => useMutation({ ...options, - mutationFn: ({ workspace, checks }) => runGuardrailChecks(workspace, checks), + mutationFn: ({ workspace, checks, draftConfig }) => + runGuardrailChecks(workspace, checks, draftConfig), onSuccess: (...args) => { const [, variables] = args; invalidateGuardrailChecksCaches(variables.workspace); diff --git a/web/packages/studio/src/api/guardrail-checks/types.ts b/web/packages/studio/src/api/guardrail-checks/types.ts index 8148b8e98f..b3e4aa13e1 100644 --- a/web/packages/studio/src/api/guardrail-checks/types.ts +++ b/web/packages/studio/src/api/guardrail-checks/types.ts @@ -21,6 +21,19 @@ export type GuardrailCheckMessage = GuardrailCheckRequest['messages'][number]; /** Per-rail verdict map returned by the /checks endpoint. */ export type RailsStatus = GuardrailCheckResponse['rails_status']; +/** + * One guardrail a config declared at run time, and whether it actually reported a + * verdict. Snapshotted onto each run: the config that produced a run is gone the + * moment the user edits again, so deriving this at render time would describe + * coverage that never ran. + */ +export interface ActivatedGuardrail { + /** Dedupe identity (detector key, else the friendly label) and the only safe React key. */ + id: string; + label: string; + active: boolean; +} + /** One execution of a check against /checks, recorded on the check's history. */ export type RunRecord = { /** ISO 8601 timestamp of when the run completed. */ @@ -33,6 +46,13 @@ export type RunRecord = { config_ids?: string[]; /** The parent config's db_version at run time, for honest history across config edits. */ config_version?: number; + /** + * Set when the run targeted an unsaved draft rather than the saved config. Mutually + * exclusive with `config_version` — a draft has no version to stamp. + */ + is_draft?: boolean; + /** Guardrail coverage of the config that ran. Absent on records written before this existed. */ + activated_guardrails?: ActivatedGuardrail[]; }; /** Studio-owned payload stored in a guardrail_checks entity's `data`. */ diff --git a/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RailStatusTab.test.tsx b/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RailStatusTab.test.tsx index 6f54f4d2d7..08ebfa756f 100644 --- a/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RailStatusTab.test.tsx +++ b/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RailStatusTab.test.tsx @@ -41,4 +41,34 @@ describe('RailStatusTab', () => { expect(screen.getByText('No runs yet.')).toBeInTheDocument(); expect(screen.getByText('Activated Guardrails')).toBeInTheDocument(); }); + + // The config that produced a run may have been edited since — or, for a draft, never saved. + it("prefers the run's own coverage snapshot over the current config", () => { + render( + + ); + + expect(screen.getByText('Jailbreak Detection')).toBeInTheDocument(); + expect(screen.queryByText('Acme Guard')).not.toBeInTheDocument(); + }); + + it('falls back to the current config for a run recorded before snapshots existed', () => { + render( + + ); + + expect(screen.getAllByText('Acme Guard')).toHaveLength(2); + }); }); diff --git a/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RailStatusTab.tsx b/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RailStatusTab.tsx index 5bff37e662..699a614490 100644 --- a/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RailStatusTab.tsx +++ b/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RailStatusTab.tsx @@ -32,7 +32,12 @@ export const RailStatusTab: FC = ({ latestRun, configData }) // Not gated on a run: the guardrails below come from the config, so a check // that has never run still shows its declared coverage, all of it inactive. const railEntries = Object.entries(latestRun?.rails_status ?? {}); - const guardrails = getActivatedGuardrails(configData, latestRun?.rails_status); + // The snapshot describes the config that actually ran — which for a draft no longer + // exists, and for a saved run may since have been edited. Deriving from `configData` + // is the fallback for runs recorded before snapshots existed, and for checks with no + // runs at all (where declared coverage is still worth showing). + const guardrails = + latestRun?.activated_guardrails ?? getActivatedGuardrails(configData, latestRun?.rails_status); return ( diff --git a/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RunHistoryTab.test.tsx b/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RunHistoryTab.test.tsx new file mode 100644 index 0000000000..3d2323982d --- /dev/null +++ b/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RunHistoryTab.test.tsx @@ -0,0 +1,37 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import type { RunRecord } from '@studio/api/guardrail-checks/types'; +import { RunHistoryTab } from '@studio/components/sidePanels/GuardrailCheckDetailSidePanel/RunHistoryTab'; +import { render, screen } from '@testing-library/react'; + +const run = (overrides: Partial): RunRecord => ({ + run_at: '2026-04-12T11:05:00.000Z', + status: 'success', + rails_status: {}, + ...overrides, +}); + +describe('RunHistoryTab', () => { + it('labels a saved run with the config version it ran against', () => { + render(); + + expect(screen.getByText('v3')).toBeInTheDocument(); + expect(screen.queryByText('Unsaved draft')).not.toBeInTheDocument(); + }); + + it('marks a draft run instead of showing a version', () => { + render(); + + expect(screen.getByText('Unsaved draft')).toBeInTheDocument(); + expect(screen.queryByText(/^v\d+$/)).not.toBeInTheDocument(); + }); + + // Records written before either field existed must still render. + it('shows no origin badge for a record carrying neither field', () => { + render(); + + expect(screen.queryByText('Unsaved draft')).not.toBeInTheDocument(); + expect(screen.queryByText(/^v\d+$/)).not.toBeInTheDocument(); + }); +}); diff --git a/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RunHistoryTab.tsx b/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RunHistoryTab.tsx index 273312f04a..78663314a5 100644 --- a/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RunHistoryTab.tsx +++ b/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RunHistoryTab.tsx @@ -13,6 +13,30 @@ export interface RunHistoryTabProps { readonly runs: RunRecord[]; } +/** + * What a run was measured against: a saved config version, or an unsaved draft. + * + * A draft has no version to show, and showing nothing would read as "old record" + * rather than "not a saved config" — so it gets its own marker. + */ +const RunOriginBadge: FC<{ readonly run: RunRecord }> = ({ run }) => { + if (run.is_draft) { + return ( + + Unsaved draft + + ); + } + if (run.config_version === undefined) { + return null; + } + return ( + + v{run.config_version} + + ); +}; + /** * Every recorded run of a check, newest first. * @@ -47,11 +71,7 @@ export const RunHistoryTab: FC = ({ runs }) => { - {run.config_version !== undefined && ( - - v{run.config_version} - - )} + diff --git a/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/railLabels.ts b/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/railLabels.ts index a16fa33632..c14aeee02c 100644 --- a/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/railLabels.ts +++ b/web/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/railLabels.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import type { RailsConfig } from '@nemo/sdk/generated/platform/schema'; -import type { RailsStatus } from '@studio/api/guardrail-checks/types'; +import type { ActivatedGuardrail, RailsStatus } from '@studio/api/guardrail-checks/types'; import { detectorMeta, listConfiguredDetectors, @@ -71,13 +71,8 @@ const collectConfigFlows = (data: RailsConfig | undefined): string[] => { ]; }; -/** One guardrail the config declares, and whether it actually ran in a given run. */ -export interface ActivatedGuardrail { - /** The dedupe identity from [[guardrailId]], and the only safe React key: labels can collide. */ - id: string; - label: string; - active: boolean; -} +// Declared alongside RunRecord — it is persisted on each run, not just rendered. +export type { ActivatedGuardrail }; /** * Identity for deduping a guardrail: its detector key when the flow registry diff --git a/web/packages/studio/src/mocks/handlers/guardrails.ts b/web/packages/studio/src/mocks/handlers/guardrails.ts index 8e88c3c14c..267e7231d4 100644 --- a/web/packages/studio/src/mocks/handlers/guardrails.ts +++ b/web/packages/studio/src/mocks/handlers/guardrails.ts @@ -281,9 +281,16 @@ export const guardrailsHandlers = [ (message) => typeof message.content === 'string' && /\d{3}-\d{2}-\d{4}/.test(message.content) ); + // Mirrors the service's two addressing modes: a whole RailsConfig runs the rails it + // declares, an id runs the saved config's. Without this a draft that changes its + // rails would be indistinguishable from one that didn't. + const inline = body.guardrails?.config; + const inlineRail = typeof inline === 'object' ? inline.rails?.input?.flows?.[0] : undefined; + const railName = inlineRail ?? 'check pii'; return HttpResponse.json({ status: blocked ? 'blocked' : 'success', - rails_status: { 'check pii': { status: blocked ? 'blocked' : 'success' } }, + rails_status: { [railName]: { status: blocked ? 'blocked' : 'success' } }, + // The real service hardcodes this to null; the echo is a mock-only convenience. guardrails_data: { config_ids: body.guardrails?.config_ids }, }); } diff --git a/web/packages/studio/src/routes/guardrails/GuardrailChecksTab/GuardrailTestCasesEditor.tsx b/web/packages/studio/src/routes/guardrails/GuardrailChecksTab/GuardrailTestCasesEditor.tsx index f44fa68432..889b6f9946 100644 --- a/web/packages/studio/src/routes/guardrails/GuardrailChecksTab/GuardrailTestCasesEditor.tsx +++ b/web/packages/studio/src/routes/guardrails/GuardrailChecksTab/GuardrailTestCasesEditor.tsx @@ -5,7 +5,7 @@ import { getErrorMessage } from '@nemo/common/src/api/common/utils'; import { LoadingButton } from '@nemo/common/src/components/LoadingButton'; import { useToast } from '@nemo/common/src/providers/toast/useToast'; import type { RailsConfig } from '@nemo/sdk/generated/platform/schema'; -import { Button, Flex, Stack, Tabs, Text } from '@nvidia/foundations-react-core'; +import { Button, Flex, SegmentedControl, Stack, Tabs, Text } from '@nvidia/foundations-react-core'; import { useCreateGuardrailCheck, useRunGuardrailChecks } from '@studio/api/guardrail-checks/hooks'; import type { GuardrailCheckEntity } from '@studio/api/guardrail-checks/types'; import { GuardrailChecksDataView } from '@studio/components/dataViews/GuardrailChecksDataView'; @@ -17,14 +17,21 @@ import { GuardrailTestCard } from '@studio/routes/guardrails/GuardrailChecksTab/ import { getGuardrailChecksSubTabRoute } from '@studio/routes/utils'; import { useRequiredPathParams } from '@studio/util/hooks/useRequiredPathParams'; import { ListChecks, Plus, Settings } from 'lucide-react'; -import { type FC, useCallback, useRef, useState } from 'react'; +import { type FC, useCallback, useEffect, useRef, useState } from 'react'; import { Link } from 'react-router'; +/** Which config a run is dispatched against. */ +type RunTarget = 'draft' | 'saved'; + interface GuardrailTestCasesEditorProps { readonly workspace: string; readonly configId: string; /** The config's rails, used by the result panel to list guardrail coverage. */ readonly configData: RailsConfig | undefined; + /** Whether the Configuration tab holds unsaved edits. Gates the Draft run target. */ + readonly isDirty: boolean; + /** Server config with live form edits applied — what a Draft run sends inline. */ + readonly draftConfig: RailsConfig; readonly checks: GuardrailCheckEntity[]; /** Which sub-tab to show. The route owns this; an unknown segment redirects upstream. */ readonly subTab: GuardrailChecksSubTab; @@ -34,6 +41,8 @@ export const GuardrailTestCasesEditor: FC = ({ workspace, configId, configData, + isDirty, + draftConfig, checks, subTab, }) => { @@ -44,6 +53,14 @@ export const GuardrailTestCasesEditor: FC = ({ const flushersRef = useRef(new Map Promise>()); const [isFlushing, setIsFlushing] = useState(false); + // Null means "follow the form": a pristine form can only mean Saved, and saving from the + // Configuration tab flips the target back on its own. Only an explicit choice is stored. + const [targetOverride, setTargetOverride] = useState(null); + const runTarget: RunTarget = isDirty ? (targetOverride ?? 'draft') : 'saved'; + + // A choice made for one config must not follow the user to the next one. + useEffect(() => setTargetOverride(null), [configId]); + const runMutation = useRunGuardrailChecks({ onSuccess: (results) => { const errors = results.filter((r): r is { name: string; error: Error } => 'error' in r); @@ -87,7 +104,11 @@ export const GuardrailTestCasesEditor: FC = ({ const fresh = await Promise.all( checks.map((check) => flushersRef.current.get(check.name)?.() ?? Promise.resolve(check)) ); - runMutation.mutate({ workspace, checks: fresh }); + runMutation.mutate({ + workspace, + checks: fresh, + draftConfig: runTarget === 'draft' ? draftConfig : undefined, + }); } finally { setIsFlushing(false); } @@ -111,6 +132,22 @@ export const GuardrailTestCasesEditor: FC = ({ Guardrail Test Cases + setTargetOverride(value as RunTarget)} + items={[ + { + value: 'draft', + children: 'Draft', + disabled: !isDirty, + // Only useful as an explanation while it is unselectable. + title: isDirty ? undefined : 'No unsaved changes to test', + }, + { value: 'saved', children: 'Saved' }, + ]} + /> diff --git a/web/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.test.tsx b/web/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.test.tsx index ad6204f4c9..3aa19c3974 100644 --- a/web/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.test.tsx +++ b/web/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.test.tsx @@ -18,7 +18,11 @@ import { } from '@studio/routes/guardrails/GuardrailChecksTab/constants'; import { GuardrailConfigTab } from '@studio/routes/guardrails/GuardrailConfigTab'; import { GuardrailDetailRoute } from '@studio/routes/guardrails/GuardrailDetailRoute'; -import { getGuardrailChecksRoute, getGuardrailChecksSubTabRoute } from '@studio/routes/utils'; +import { + getGuardrailChecksRoute, + getGuardrailChecksSubTabRoute, + getGuardrailConfigRoute, +} from '@studio/routes/utils'; import { XL_SELECTOR_TIMEOUT } from '@studio/tests/util/constants'; import { renderRoute, screen } from '@studio/tests/util/render'; import userEvent from '@testing-library/user-event'; @@ -223,6 +227,63 @@ describe('GuardrailChecksTab', () => { }); }); + describe('run target', () => { + /** Dirty the form via the real Configuration tab, then return to Test and Validate. */ + const dirtyThenOpenChecks = async () => { + const user = userEvent.setup(); + renderChecks('pii-filter', getGuardrailConfigRoute(WORKSPACE, 'pii-filter')); + await user.type( + await screen.findByRole( + 'textbox', + { name: 'General instruction' }, + { timeout: XL_SELECTOR_TIMEOUT } + ), + 'Be extremely cautious.' + ); + await user.click(screen.getByRole('tab', { name: 'Test and Validate' })); + await screen.findByText('Guardrail Test Cases', undefined, { timeout: XL_SELECTOR_TIMEOUT }); + return user; + }; + + it('offers only the saved config while the form is pristine', async () => { + renderChecks('pii-filter'); + await screen.findByText('Guardrail Test Cases', undefined, { timeout: XL_SELECTOR_TIMEOUT }); + + expect(screen.getByRole('radio', { name: 'Saved' })).toBeChecked(); + expect(screen.getByRole('radio', { name: 'Draft' })).toBeDisabled(); + }); + + it('selects Draft once there are unsaved edits and sends the config inline', async () => { + const user = await dirtyThenOpenChecks(); + + expect(screen.getByRole('radio', { name: 'Draft' })).toBeChecked(); + + await user.click(screen.getByRole('button', { name: /Run 2 Tests/ })); + await screen.findByText('Ran 2 test(s) successfully', undefined, { + timeout: XL_SELECTOR_TIMEOUT, + }); + + const [sent] = recordedCheckRequests; + expect(sent?.guardrails).not.toHaveProperty('config_ids'); + const inline = sent?.guardrails?.config; + expect(typeof inline === 'object' ? inline.instructions : undefined).toContainEqual( + expect.objectContaining({ content: expect.stringContaining('Be extremely cautious.') }) + ); + }); + + it('runs against the saved config when the user picks Saved on a dirty form', async () => { + const user = await dirtyThenOpenChecks(); + + await user.click(screen.getByRole('radio', { name: 'Saved' })); + await user.click(screen.getByRole('button', { name: /Run 2 Tests/ })); + await screen.findByText('Ran 2 test(s) successfully', undefined, { + timeout: XL_SELECTOR_TIMEOUT, + }); + + expect(recordedCheckRequests[0]?.guardrails).toEqual({ config_ids: ['pii-filter'] }); + }); + }); + it('shows an error state when the checks cannot be loaded', async () => { server.use(http.get(CHECKS_URL, () => new HttpResponse(null, { status: 500 }))); diff --git a/web/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.tsx b/web/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.tsx index e1b80f25c4..b9905a1b25 100644 --- a/web/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.tsx +++ b/web/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.tsx @@ -12,6 +12,7 @@ import { isGuardrailChecksSubTab, } from '@studio/routes/guardrails/GuardrailChecksTab/constants'; import { GuardrailTestCasesEditor } from '@studio/routes/guardrails/GuardrailChecksTab/GuardrailTestCasesEditor'; +import { useDraftRailsConfig } from '@studio/routes/guardrails/GuardrailForm/useDraftRailsConfig'; import { getGuardrailChecksSubTabRoute } from '@studio/routes/utils'; import { useRequiredPathParams } from '@studio/util/hooks/useRequiredPathParams'; import type { FC } from 'react'; @@ -23,6 +24,9 @@ export const GuardrailChecksTab: FC = () => { const subTab = useParams()[ROUTE_PARAMS.guardrailChecksSubTab]; const isValidSubTab = isGuardrailChecksSubTab(subTab); + // The unsaved edits from the Configuration tab; both tabs sit inside GuardrailFormProvider. + const { isDirty, draftConfig } = useDraftRailsConfig(); + const { data: config, isPending: isConfigPending } = useGuardrailsGetGuardrailConfig( workspace, guardrailConfigName, @@ -84,6 +88,8 @@ export const GuardrailChecksTab: FC = () => { workspace={workspace} configId={config.id} configData={config.data} + isDirty={isDirty} + draftConfig={draftConfig} checks={checksPage.data} subTab={subTab} /> diff --git a/web/packages/studio/src/routes/guardrails/GuardrailConfigTab/index.tsx b/web/packages/studio/src/routes/guardrails/GuardrailConfigTab/index.tsx index 5578908e08..d4b6b96382 100644 --- a/web/packages/studio/src/routes/guardrails/GuardrailConfigTab/index.tsx +++ b/web/packages/studio/src/routes/guardrails/GuardrailConfigTab/index.tsx @@ -12,23 +12,15 @@ import { GeneralSection } from '@studio/routes/guardrails/GuardrailConfigTab/Gen import { LlmSection } from '@studio/routes/guardrails/GuardrailConfigTab/LlmSection'; import { PipelineSection } from '@studio/routes/guardrails/GuardrailConfigTab/PipelineSection'; import { RawConfigSection } from '@studio/routes/guardrails/GuardrailConfigTab/RawConfigSection'; -import { - applyFormToConfig, - type GuardrailFormValues, -} from '@studio/routes/guardrails/GuardrailForm/formModel'; +import { useDraftRailsConfig } from '@studio/routes/guardrails/GuardrailForm/useDraftRailsConfig'; import { useGuardrailForm } from '@studio/routes/guardrails/GuardrailForm/useGuardrailForm'; import { Shield } from 'lucide-react'; import type { FC } from 'react'; -import { useFormContext, useWatch } from 'react-hook-form'; export const GuardrailConfigTab: FC = () => { const { config } = useGuardrailForm(); - const { control } = useFormContext(); - // Fields all have string defaults, so watched values are never undefined at runtime. - const values = useWatch({ control }) as GuardrailFormValues; - // Read-only sections reflect live edits: server data with the form applied. - const data = applyFormToConfig(config.data, values); + const { draftConfig: data } = useDraftRailsConfig(); const rails = data.rails; const modelCount = data.models?.length ?? 0; const railCount = countRails(data); diff --git a/web/packages/studio/src/routes/guardrails/GuardrailForm/useDraftRailsConfig.ts b/web/packages/studio/src/routes/guardrails/GuardrailForm/useDraftRailsConfig.ts new file mode 100644 index 0000000000..4febbf6b51 --- /dev/null +++ b/web/packages/studio/src/routes/guardrails/GuardrailForm/useDraftRailsConfig.ts @@ -0,0 +1,40 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import type { RailsConfig } from '@nemo/sdk/generated/platform/schema'; +import { + applyFormToConfig, + type GuardrailFormValues, +} from '@studio/routes/guardrails/GuardrailForm/formModel'; +import { useGuardrailForm } from '@studio/routes/guardrails/GuardrailForm/useGuardrailForm'; +import { useFormContext, useWatch } from 'react-hook-form'; + +export interface DraftRailsConfig { + /** Whether the form holds edits not yet saved to the server. */ + isDirty: boolean; + /** + * Server config with the current form values applied. Equal to the saved config + * when the form is pristine — callers that need "the draft, or nothing" should + * gate on {@link isDirty}. + */ + draftConfig: RailsConfig; +} + +/** + * The config as the user currently sees it: server data with live form edits merged in. + * + * Both tabs read this. The Configuration tab renders it; the checks tab sends it to + * /checks when the run target is Draft. Deriving it twice would let a run exercise + * something different from what the editor displays. + */ +export const useDraftRailsConfig = (): DraftRailsConfig => { + const { config } = useGuardrailForm(); + const { + control, + formState: { isDirty }, + } = useFormContext(); + // Fields all have string defaults, so watched values are never undefined at runtime. + const values = useWatch({ control }) as GuardrailFormValues; + + return { isDirty, draftConfig: applyFormToConfig(config.data, values) }; +};