fix(studio): refactor GuardrailsTable to DataView - #1043
Merged
Conversation
Signed-off-by: Nicholas Kolean <nakolean@gmail.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughChangesGuardrail results experience
Sequence Diagram(s)sequenceDiagram
participant GuardrailTestCasesEditor
participant ResultSummary
participant GuardrailChecksDataView
participant checkStatus
GuardrailTestCasesEditor->>ResultSummary: Render check status summary
GuardrailTestCasesEditor->>GuardrailChecksDataView: Pass guardrail checks
GuardrailChecksDataView->>checkStatus: Filter and rank verdicts
checkStatus-->>GuardrailChecksDataView: Return result buckets and sort order
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
web/packages/studio/src/components/dataViews/GuardrailChecksDataView/ResultSummary.tsx (1)
19-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
getResultFilterValuefor the counts.This duplicates the verdict bucketing in
checkStatus.ts. If the mapping changes there, the summary and the table will disagree.♻️ Proposed fix
-import { getLatestRunStatus } from '`@studio/components/dataViews/GuardrailChecksDataView/checkStatus`'; +import { + getLatestRunStatus, + getResultFilterValue, +} from '`@studio/components/dataViews/GuardrailChecksDataView/checkStatus`'; @@ for (const check of checks) { - const status = getLatestRunStatus(check); - if (status === 'success') { + const bucket = getResultFilterValue(getLatestRunStatus(check)); + if (bucket === 'success') { allowed += 1; - } else if (status === 'blocked') { + } else if (bucket === 'blocked') { guarded += 1; } else { notRun += 1; } }🤖 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/components/dataViews/GuardrailChecksDataView/ResultSummary.tsx` around lines 19 - 36, Update summarizeResults to classify each check using getResultFilterValue from checkStatus.ts instead of comparing getLatestRunStatus results directly. Increment allowed, guarded, or notRun according to the shared filter-value mapping, preserving the existing returned count shape.web/packages/studio/src/components/dataViews/GuardrailChecksDataView/index.test.tsx (1)
146-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for Result column sorting.
getResultSortRankand the sort branch inindex.tsx(Lines 77-85) have no coverage. Click the Result column header and assert the row order for ascending and descending.🤖 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/components/dataViews/GuardrailChecksDataView/index.test.tsx` around lines 146 - 167, Add a test near the existing filtering test that clicks the Result column header and verifies rows are ordered correctly in both ascending and descending directions. Exercise the getResultSortRank behavior and the sorting branch in the GuardrailChecksDataView component, using the existing rendered row text and established test utilities.web/packages/studio/src/components/dataViews/GuardrailChecksDataView/checkStatus.ts (1)
17-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueType the bucket values instead of
string.
getResultFilterValuereturnsstring, so consumers must cast (seeindex.tsxLine 63). A union type keeps the buckets checked at compile time and makesRESULT_SORT_RANKexhaustive.♻️ Proposed typing
+type ResultFilterValue = (typeof RESULT_FILTER_VALUES)[keyof typeof RESULT_FILTER_VALUES]; + /** Options for the "Result" single-select column filter. */ export const RESULT_FILTER_OPTIONS: FilterItem[] = [ @@ -export const getResultFilterValue = (status: Verdict | undefined): string => { +export const getResultFilterValue = (status: Verdict | undefined): ResultFilterValue => { @@ -const RESULT_SORT_RANK: Record<string, number> = { +const RESULT_SORT_RANK: Record<ResultFilterValue, number> = {As per coding guidelines: "Use
typefor unions, intersections, and computed types 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/components/dataViews/GuardrailChecksDataView/checkStatus.ts` around lines 17 - 53, Define a type alias for the three result bucket values represented by RESULT_FILTER_VALUES, and update getResultFilterValue to return that union instead of string. Type RESULT_SORT_RANK with the same bucket union (using an appropriate mapped or record type) so all buckets are compile-time checked and consumers no longer need casts.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.
Nitpick comments:
In
`@web/packages/studio/src/components/dataViews/GuardrailChecksDataView/checkStatus.ts`:
- Around line 17-53: Define a type alias for the three result bucket values
represented by RESULT_FILTER_VALUES, and update getResultFilterValue to return
that union instead of string. Type RESULT_SORT_RANK with the same bucket union
(using an appropriate mapped or record type) so all buckets are compile-time
checked and consumers no longer need casts.
In
`@web/packages/studio/src/components/dataViews/GuardrailChecksDataView/index.test.tsx`:
- Around line 146-167: Add a test near the existing filtering test that clicks
the Result column header and verifies rows are ordered correctly in both
ascending and descending directions. Exercise the getResultSortRank behavior and
the sorting branch in the GuardrailChecksDataView component, using the existing
rendered row text and established test utilities.
In
`@web/packages/studio/src/components/dataViews/GuardrailChecksDataView/ResultSummary.tsx`:
- Around line 19-36: Update summarizeResults to classify each check using
getResultFilterValue from checkStatus.ts instead of comparing getLatestRunStatus
results directly. Increment allowed, guarded, or notRun according to the shared
filter-value mapping, preserving the existing returned count shape.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3779c4fc-0fd1-47a3-9ce1-db6867babd76
📒 Files selected for processing (8)
web/packages/studio/src/components/dataViews/GuardrailChecksDataView/ResultIndicator.tsxweb/packages/studio/src/components/dataViews/GuardrailChecksDataView/ResultSummary.tsxweb/packages/studio/src/components/dataViews/GuardrailChecksDataView/checkStatus.tsweb/packages/studio/src/components/dataViews/GuardrailChecksDataView/index.test.tsxweb/packages/studio/src/components/dataViews/GuardrailChecksDataView/index.tsxweb/packages/studio/src/routes/guardrails/GuardrailChecksTab/GuardrailResultsTable.tsxweb/packages/studio/src/routes/guardrails/GuardrailChecksTab/GuardrailTestCasesEditor.tsxweb/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.test.tsx
💤 Files with no reviewable changes (1)
- web/packages/studio/src/routes/guardrails/GuardrailChecksTab/GuardrailResultsTable.tsx
Contributor
|
steramae-nvidia
approved these changes
Aug 3, 2026
Signed-off-by: Nicholas Kolean <nakolean@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests