feat(studio): add guardrail detail tabs with read-only Configuration viewer#830
feat(studio): add guardrail detail tabs with read-only Configuration viewer#830aray12 wants to merge 1 commit into
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
34a76bf to
a6b221b
Compare
5026a87 to
e86c1ac
Compare
…viewer Introduce the guardrail detail tab shell and a structured, read-only Configuration tab that renders the full RailsConfig schema (pipeline stages, detectors, models/prompting, behavior) instead of a raw JSON dump. Includes a flow-recognition registry and detector scope derivation. Signed-off-by: Alex Ray <alray@nvidia.com>
e86c1ac to
2f33320
Compare
📝 WalkthroughWalkthroughGuardrail details now use nested configuration routing, a structured configuration tab, persisted local drafts, reset/save actions, detector and flow interpretation helpers, and a PATCH mock for configuration updates. ChangesGuardrail configuration workflow
Sequence Diagram(s)sequenceDiagram
participant User
participant GuardrailConfigTab
participant GuardrailDraftProvider
participant GuardrailsAPI
User->>GuardrailConfigTab: edit general instruction
GuardrailConfigTab->>GuardrailDraftProvider: update draft
User->>GuardrailDraftProvider: save guardrail
GuardrailDraftProvider->>GuardrailsAPI: PATCH configuration
GuardrailsAPI-->>GuardrailDraftProvider: return updated configuration
GuardrailDraftProvider-->>GuardrailConfigTab: clear draft actions
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
web/packages/studio/src/routes/guardrails/GuardrailConfigTab/DraftTextAreaField.tsx (1)
7-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the prop contract readonly.
Mark
DraftTextAreaFieldPropsmembersreadonly. As per coding guidelines, usereadonlyfor immutable properties in TypeScript interfaces and types.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/studio/src/routes/guardrails/GuardrailConfigTab/DraftTextAreaField.tsx` around lines 7 - 15, Update the DraftTextAreaFieldProps interface so every property, including label, description, placeholder, value, and onChange, is marked readonly. Preserve the existing property types and optionality.Source: Coding guidelines
web/packages/studio/src/routes/guardrails/GuardrailDraft/useGuardrailDraft.ts (2)
7-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare the hook return type.
Expose the non-null context value as the exported hook contract. As per coding guidelines, exported public APIs require explicit return types.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/studio/src/routes/guardrails/GuardrailDraft/useGuardrailDraft.ts` around lines 7 - 13, Update the exported useGuardrailDraft hook to declare an explicit return type matching the non-null context value returned after the GuardrailDraftContext check. Preserve the existing provider validation and return behavior.Source: Coding guidelines
4-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep external imports before internal imports.
web/packages/studio/src/routes/guardrails/GuardrailDraft/useGuardrailDraft.ts#L4-L5: move the React import before the internal context import.web/packages/studio/src/routes/guardrails/GuardrailConfigTab/GeneralSection.tsx#L4-L12: placelucide-reactand React imports before@studioimports.web/packages/studio/src/routes/guardrails/GuardrailConfigTab/LlmSection.tsx#L4-L22: placelucide-reactand React imports before@studioimports.web/packages/studio/src/routes/guardrails/GuardrailConfigTab/BehaviorSection.tsx#L4-L9: placelucide-reactand React imports before@studioimports.web/packages/studio/src/routes/guardrails/GuardrailConfigTab/index.tsx#L4-L17: placelucide-reactand React imports before@studioimports.As per coding guidelines, “Group imports: external libraries, internal modules, relative imports in TypeScript.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/studio/src/routes/guardrails/GuardrailDraft/useGuardrailDraft.ts` around lines 4 - 5, Reorder imports so external libraries precede internal `@studio` modules: in web/packages/studio/src/routes/guardrails/GuardrailDraft/useGuardrailDraft.ts lines 4-5, move the React import before GuardrailDraftContext; apply the same ordering in web/packages/studio/src/routes/guardrails/GuardrailConfigTab/GeneralSection.tsx lines 4-12, LlmSection.tsx lines 4-22, BehaviorSection.tsx lines 4-9, and index.tsx lines 4-17, placing lucide-react and React imports before `@studio` imports.Source: Coding guidelines
web/packages/studio/src/routes/guardrails/GuardrailConfigTab/RawConfigSection.tsx (1)
20-30: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDefer raw JSON generation until expanded.
JSON.stringifyruns on every draft update even while collapsed. SinceGuardrailConfigTabalways mounts this section and textarea edits update the draft per keystroke, lazily mount or serialize the raw body only when the accordion is open.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/studio/src/routes/guardrails/GuardrailConfigTab/RawConfigSection.tsx` around lines 20 - 30, Update RawConfigSection so JSON.stringify(data, null, 2) is evaluated only after the raw-config AccordionItem is expanded, rather than on every draft update while collapsed. Use the accordion’s existing open-state mechanism to conditionally render or lazily compute the pre content, preserving the current JSON display when expanded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/packages/studio/src/mocks/handlers/guardrails.ts`:
- Around line 73-81: Update the PATCH handler for guardrail configs to merge the
parsed request body into the matched entry in mockGuardrailConfigs before
returning the response. Preserve the 404 behavior for unknown names, and ensure
subsequent GET/refetch requests observe the persisted merged configuration.
In
`@web/packages/studio/src/routes/guardrails/GuardrailConfigTab/BehaviorSection.tsx`:
- Around line 46-47: Update hasBehaviorContent to base visibility on content
that the BehaviorSection actually renders: count behavior fields and only treat
tracing as present when it produces rendered tracing fields or a badge. Ensure
empty tracing objects and disabled enable_content_capture values do not make the
behavior panel appear unless their disabled state is explicitly rendered.
In `@web/packages/studio/src/routes/guardrails/GuardrailConfigTab/detectors.ts`:
- Around line 116-120: Update SECRET_KEY and formatScalar so secret-bearing keys
with snake_case prefixes or suffixes, including nim_api_key, hf_token, and
access_token, are masked while existing exact-name matches remain supported. Add
regression cases covering these prefixed/suffixed keys and verify non-secret
keys remain rendered normally.
---
Nitpick comments:
In
`@web/packages/studio/src/routes/guardrails/GuardrailConfigTab/DraftTextAreaField.tsx`:
- Around line 7-15: Update the DraftTextAreaFieldProps interface so every
property, including label, description, placeholder, value, and onChange, is
marked readonly. Preserve the existing property types and optionality.
In
`@web/packages/studio/src/routes/guardrails/GuardrailConfigTab/RawConfigSection.tsx`:
- Around line 20-30: Update RawConfigSection so JSON.stringify(data, null, 2) is
evaluated only after the raw-config AccordionItem is expanded, rather than on
every draft update while collapsed. Use the accordion’s existing open-state
mechanism to conditionally render or lazily compute the pre content, preserving
the current JSON display when expanded.
In
`@web/packages/studio/src/routes/guardrails/GuardrailDraft/useGuardrailDraft.ts`:
- Around line 7-13: Update the exported useGuardrailDraft hook to declare an
explicit return type matching the non-null context value returned after the
GuardrailDraftContext check. Preserve the existing provider validation and
return behavior.
- Around line 4-5: Reorder imports so external libraries precede internal
`@studio` modules: in
web/packages/studio/src/routes/guardrails/GuardrailDraft/useGuardrailDraft.ts
lines 4-5, move the React import before GuardrailDraftContext; apply the same
ordering in
web/packages/studio/src/routes/guardrails/GuardrailConfigTab/GeneralSection.tsx
lines 4-12, LlmSection.tsx lines 4-22, BehaviorSection.tsx lines 4-9, and
index.tsx lines 4-17, placing lucide-react and React imports before `@studio`
imports.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c6632b2c-01dd-469f-a7e5-6ecd64fdf81f
📒 Files selected for processing (27)
web/packages/studio/src/constants/routes.tsweb/packages/studio/src/mocks/handlers/guardrails.tsweb/packages/studio/src/routes/groups/guardrailsRoutes.tsxweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/BehaviorSection.tsxweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/DetectorsSection.tsxweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/DraftTextAreaField.tsxweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/GeneralSection.tsxweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/LlmSection.tsxweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/PipelineSection.tsxweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/RawConfigSection.tsxweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/configPrimitives.tsxweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/detectors.test.tsweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/detectors.tsweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/flowRegistry.test.tsweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/flowRegistry.tsweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/index.tsxweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/instructions.test.tsweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/instructions.tsweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/sections.test.tsxweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/types.tsweb/packages/studio/src/routes/guardrails/GuardrailDetailRoute/index.test.tsxweb/packages/studio/src/routes/guardrails/GuardrailDetailRoute/index.tsxweb/packages/studio/src/routes/guardrails/GuardrailDraft/GuardrailDraftProvider.tsxweb/packages/studio/src/routes/guardrails/GuardrailDraft/GuardrailHeaderActions.tsxweb/packages/studio/src/routes/guardrails/GuardrailDraft/context.tsweb/packages/studio/src/routes/guardrails/GuardrailDraft/useGuardrailDraft.tsweb/packages/studio/src/routes/utils.ts
| http.patch( | ||
| `${PLATFORM_BASE_URL}/apis/guardrails/v2/workspaces/:workspace/configs/:name`, | ||
| async ({ params, request }) => { | ||
| const config = mockGuardrailConfigs.find((c) => c.name === params.name); | ||
| if (!config) return new HttpResponse(null, { status: 404 }); | ||
| const body = (await request.json()) as Partial<GuardrailConfig>; | ||
| return HttpResponse.json({ ...config, ...body }); | ||
| } | ||
| ), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
PATCH mock doesn't persist merged data — refetch after save returns stale config.
config is looked up by reference but the response is built from a new spread object; mockGuardrailConfigs is never updated. Any refetch (e.g. via invalidateQueries in GuardrailDraftProvider.save) will return pre-PATCH data, silently discarding the "saved" edit and weakening test coverage of the save flow.
🛠️ Proposed fix
http.patch(
`${PLATFORM_BASE_URL}/apis/guardrails/v2/workspaces/:workspace/configs/:name`,
async ({ params, request }) => {
const config = mockGuardrailConfigs.find((c) => c.name === params.name);
if (!config) return new HttpResponse(null, { status: 404 });
const body = (await request.json()) as Partial<GuardrailConfig>;
- return HttpResponse.json({ ...config, ...body });
+ Object.assign(config, body);
+ return HttpResponse.json(config);
}
),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| http.patch( | |
| `${PLATFORM_BASE_URL}/apis/guardrails/v2/workspaces/:workspace/configs/:name`, | |
| async ({ params, request }) => { | |
| const config = mockGuardrailConfigs.find((c) => c.name === params.name); | |
| if (!config) return new HttpResponse(null, { status: 404 }); | |
| const body = (await request.json()) as Partial<GuardrailConfig>; | |
| return HttpResponse.json({ ...config, ...body }); | |
| } | |
| ), | |
| http.patch( | |
| `${PLATFORM_BASE_URL}/apis/guardrails/v2/workspaces/:workspace/configs/:name`, | |
| async ({ params, request }) => { | |
| const config = mockGuardrailConfigs.find((c) => c.name === params.name); | |
| if (!config) return new HttpResponse(null, { status: 404 }); | |
| const body = (await request.json()) as Partial<GuardrailConfig>; | |
| Object.assign(config, body); | |
| return HttpResponse.json(config); | |
| } | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/packages/studio/src/mocks/handlers/guardrails.ts` around lines 73 - 81,
Update the PATCH handler for guardrail configs to merge the parsed request body
into the matched entry in mockGuardrailConfigs before returning the response.
Preserve the 404 behavior for unknown names, and ensure subsequent GET/refetch
requests observe the persisted merged configuration.
| const hasBehaviorContent = (data: RailsConfigOutput | undefined): boolean => | ||
| behaviorFields(data).length > 0 || Boolean(data?.tracing); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Avoid rendering an empty behavior panel.
tracing: {} or { enable_content_capture: false } passes this check but renders neither fields nor a badge. Base visibility on rendered tracing fields, or display the disabled capture state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@web/packages/studio/src/routes/guardrails/GuardrailConfigTab/BehaviorSection.tsx`
around lines 46 - 47, Update hasBehaviorContent to base visibility on content
that the BehaviorSection actually renders: count behavior fields and only treat
tracing as present when it produces rendered tracing fields or a badge. Ensure
empty tracing objects and disabled enable_content_capture values do not make the
behavior panel appear unless their disabled state is explicitly rendered.
| const SECRET_KEY = /^(api_key|secret|token|password)$/i; | ||
|
|
||
| const formatScalar = (key: string, value: string | number | boolean): string => { | ||
| if (SECRET_KEY.test(key)) return '••••••••'; | ||
| return String(value); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Mask prefixed secret keys.
nim_api_key, hf_token, and access_token do not match this exact-name regex and are rendered in plaintext. Match secret-bearing snake_case suffixes/prefixes and add regression cases.
Proposed fix
-const SECRET_KEY = /^(api_key|secret|token|password)$/i;
+const SECRET_KEY =
+ /(?:^|_)(?:api_?key|secret|token|password|private_?key)(?:$|_)/i;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const SECRET_KEY = /^(api_key|secret|token|password)$/i; | |
| const formatScalar = (key: string, value: string | number | boolean): string => { | |
| if (SECRET_KEY.test(key)) return '••••••••'; | |
| return String(value); | |
| const SECRET_KEY = | |
| /(?:^|_)(?:api_?key|secret|token|password|private_?key)(?:$|_)/i; | |
| const formatScalar = (key: string, value: string | number | boolean): string => { | |
| if (SECRET_KEY.test(key)) return '••••••••'; | |
| return String(value); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/packages/studio/src/routes/guardrails/GuardrailConfigTab/detectors.ts`
around lines 116 - 120, Update SECRET_KEY and formatScalar so secret-bearing
keys with snake_case prefixes or suffixes, including nim_api_key, hf_token, and
access_token, are masked while existing exact-name matches remain supported. Add
regression cases covering these prefixed/suffixed keys and verify non-secret
keys remain rendered normally.
|
Introduce the guardrail detail tab shell and a structured, read-only
Configuration tab that renders the full RailsConfig schema (pipeline
stages, detectors, models/prompting, behavior) instead of a raw JSON dump.
Includes a flow-recognition registry and detector scope derivation.
Signed-off-by: Alex Ray alray@nvidia.com
Summary by CodeRabbit