feat(studio): add guardrail Test and Validate tab (checks, runs, detail) - #831
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
5026a87 to
e86c1ac
Compare
a3dc362 to
a012ff1
Compare
41d3173 to
a869d48
Compare
a012ff1 to
439ef39
Compare
Add the Test and Validate tab to the guardrail detail view: a checks table (entity-store-backed test cases) with per-check status and a run action, a test-case editor, and a check-detail view showing per-rail run verdicts. Wires the checks and check-detail routes and the second detail tab. Signed-off-by: Alex Ray <alray@nvidia.com>
439ef39 to
d962340
Compare
📝 WalkthroughWalkthroughChangesGuardrail checks
Sequence Diagram(s)sequenceDiagram
participant User
participant GuardrailDetailRoute
participant GuardrailChecksTab
participant EntityStoreAPI
participant GuardrailTestCasesEditor
participant GuardrailTestCard
participant GuardrailResultsTable
User->>GuardrailDetailRoute: Select Test and Validate
GuardrailDetailRoute->>GuardrailChecksTab: Navigate to checks route
GuardrailChecksTab->>EntityStoreAPI: Load configuration and checks
EntityStoreAPI-->>GuardrailChecksTab: Return configuration and checks
GuardrailChecksTab->>GuardrailTestCasesEditor: Render loaded checks
User->>GuardrailTestCard: Edit or run test messages
GuardrailTestCard->>EntityStoreAPI: Persist test changes
GuardrailTestCasesEditor->>EntityStoreAPI: Run all checks
EntityStoreAPI-->>GuardrailResultsTable: Return run statuses
GuardrailResultsTable-->>User: Display inputs, outputs, and results
Possibly related PRs
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 (2)
web/packages/studio/src/routes/guardrails/GuardrailChecksTab/GuardrailTestCard.tsx (2)
43-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the type assertion.
The
as GuardrailCheckMessagecast hides any shape mismatch with the API message type. Type the mapper return and let the compiler check it, or narrowroleexplicitly.As per coding guidelines: "Use type assertions sparingly. Prefer type guards and narrowing 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/GuardrailChecksTab/GuardrailTestCard.tsx` around lines 43 - 45, Update toCheckMessages so its mapper returns GuardrailCheckMessage through contextual typing or an explicit return type, removing the as GuardrailCheckMessage assertion. If role or content types do not align, narrow or transform them explicitly so the compiler validates the API message shape.Source: Coding guidelines
59-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe sync effect never runs.
GuardrailTestCasesEditorrenders each card withkey={check.id}, so a mounted card never sees a newcheck.id. The reset is dead code, andcheck.data.messagesin the dependency array is misleading. If you want to ignore server refreshes to protect local drafts, drop the effect and state that in a comment. If you want resync, gate on message content instead.🤖 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/GuardrailChecksTab/GuardrailTestCard.tsx` around lines 59 - 66, Update the synchronization logic around GuardrailTestCard’s lastCheckId and useEffect: because key={check.id} remounts the card when the ID changes, either remove this dead effect and document that server refreshes are intentionally ignored to preserve local drafts, or gate resynchronization on changes to check.data.messages instead of check.id. Keep form.reset using toFormRows for any synchronization path retained.
🤖 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/routes/guardrails/GuardrailChecksTab/GuardrailMessageRow.tsx`:
- Around line 92-98: Update GuardrailMessageRow’s focusBody interval lifecycle
by storing the interval id in a ref, clearing any active interval during
component unmount via useEffect cleanup, and adding the required useEffect
import. Preserve the existing focus retry behavior while preventing callbacks
after unmount.
In
`@web/packages/studio/src/routes/guardrails/GuardrailChecksTab/GuardrailTestCard.tsx`:
- Around line 74-87: The persist callback currently reuses the stale
check.db_version during rapid successive saves. Update the GuardrailTestCard
save flow around persist and updateMutation so saves are serialized or queued,
and carry the latest expected_db_version forward from each mutation response
before sending the next patch, preserving all edits without
optimistic-concurrency failures.
In `@web/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.tsx`:
- Around line 30-35: Update the GuardrailChecksTab query-state handling around
isConfigPending, isChecksPending, config, and GuardrailTestCasesEditor to render
the established error state when either config or checks query fails. Do not
fall through to null for config failures, and do not substitute [] for failed
checks data; preserve normal rendering for successful queries. Add tests
covering both query failure paths.
---
Nitpick comments:
In
`@web/packages/studio/src/routes/guardrails/GuardrailChecksTab/GuardrailTestCard.tsx`:
- Around line 43-45: Update toCheckMessages so its mapper returns
GuardrailCheckMessage through contextual typing or an explicit return type,
removing the as GuardrailCheckMessage assertion. If role or content types do not
align, narrow or transform them explicitly so the compiler validates the API
message shape.
- Around line 59-66: Update the synchronization logic around GuardrailTestCard’s
lastCheckId and useEffect: because key={check.id} remounts the card when the ID
changes, either remove this dead effect and document that server refreshes are
intentionally ignored to preserve local drafts, or gate resynchronization on
changes to check.data.messages instead of check.id. Keep form.reset using
toFormRows for any synchronization path retained.
🪄 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: 29204279-6eb5-44fd-85cb-c9bfe1d70c5f
📒 Files selected for processing (13)
web/packages/common/src/components/ChatCompletionInput/index.tsxweb/packages/studio/src/components/dataViews/GuardrailChecksDataView/checkMessages.tsweb/packages/studio/src/components/dataViews/GuardrailChecksDataView/checkStatus.tsweb/packages/studio/src/constants/routes.tsweb/packages/studio/src/routes/groups/guardrailsRoutes.tsxweb/packages/studio/src/routes/guardrails/GuardrailChecksTab/GuardrailMessageRow.tsxweb/packages/studio/src/routes/guardrails/GuardrailChecksTab/GuardrailResultsTable.tsxweb/packages/studio/src/routes/guardrails/GuardrailChecksTab/GuardrailTestCard.tsxweb/packages/studio/src/routes/guardrails/GuardrailChecksTab/GuardrailTestCasesEditor.tsxweb/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.tsxweb/packages/studio/src/routes/guardrails/GuardrailDetailRoute/index.test.tsxweb/packages/studio/src/routes/guardrails/GuardrailDetailRoute/index.tsxweb/packages/studio/src/routes/utils.ts
|
Signed-off-by: Nicholas Kolean <nakolean@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
web/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.test.tsx (1)
27-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename
routestoTEST_ROUTES.
routesis immutable module-level configuration. UseSCREAMING_SNAKE_CASEfor constants. UpdaterenderChecksto passTEST_ROUTES.Proposed fix
-const routes = [ +const TEST_ROUTES = [ ... - routes, + routes: TEST_ROUTES,🤖 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/GuardrailChecksTab/index.test.tsx` around lines 27 - 47, Rename the module-level immutable routes configuration from routes to TEST_ROUTES, and update renderChecks to pass TEST_ROUTES to renderRoute while preserving the existing route definitions.Source: Coding guidelines
web/packages/studio/src/mocks/handlers/guardrails.ts (1)
4-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGroup external imports before internal imports.
web/packages/studio/src/mocks/handlers/guardrails.ts#L4-L10: Move themswimport into the external import group before@studioimports.web/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.tsx#L4-L12: Move the React type import into the external import group before@studioimports.web/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.test.tsx#L4-L15: Move themswandreact-router-domimports into the external import group 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/mocks/handlers/guardrails.ts` around lines 4 - 10, Reorder imports into external, internal, and relative groups: in web/packages/studio/src/mocks/handlers/guardrails.ts (lines 4-10), place the msw import before `@studio` imports; in web/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.tsx (lines 4-12), place the React type import before `@studio` imports; and in web/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.test.tsx (lines 4-15), place msw and react-router-dom imports before `@studio` imports. Preserve all imported symbols and behavior.Source: Coding guidelines
🤖 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 102-115: Update the guardrail checks GET handler to read the
request’s parent filter and return only entries from mockGuardrailChecks whose
parent matches it, while preserving accurate pagination counts for the filtered
result. Add a route test covering cfg-2 and assert that it returns no checks.
---
Nitpick comments:
In `@web/packages/studio/src/mocks/handlers/guardrails.ts`:
- Around line 4-10: Reorder imports into external, internal, and relative
groups: in web/packages/studio/src/mocks/handlers/guardrails.ts (lines 4-10),
place the msw import before `@studio` imports; in
web/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.tsx (lines
4-12), place the React type import before `@studio` imports; and in
web/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.test.tsx
(lines 4-15), place msw and react-router-dom imports before `@studio` imports.
Preserve all imported symbols and behavior.
In `@web/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.test.tsx`:
- Around line 27-47: Rename the module-level immutable routes configuration from
routes to TEST_ROUTES, and update renderChecks to pass TEST_ROUTES to
renderRoute while preserving the existing route definitions.
🪄 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: cd3923a0-66b8-44b6-a338-9737e9b7f569
📒 Files selected for processing (3)
web/packages/studio/src/mocks/handlers/guardrails.tsweb/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.test.tsxweb/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.tsx
Add the Test and Validate tab to the guardrail detail view: a checks table
(entity-store-backed test cases) with per-check status and a run action,
a test-case editor, and a check-detail view showing per-rail run verdicts.
Wires the checks and check-detail routes and the second detail tab.
Signed-off-by: Alex Ray alray@nvidia.com
Summary by CodeRabbit
New Features
Style