feat(studio): standardize guardrails empty state and add ui-design skill - #1268
Open
aahunt-nv wants to merge 6 commits into
Open
feat(studio): standardize guardrails empty state and add ui-design skill#1268aahunt-nv wants to merge 6 commits into
aahunt-nv wants to merge 6 commits into
Conversation
Adds a ui-design agent skill under web/.agents/skills that routes UI changes to standardized references. First reference documents the EntityEmptyState empty-state pattern (ASTD-394). Signed-off-by: Aaron Hunt <aahunt@nvidia.com>
Add the shared EntityEmptyState component + entity registry in @nemo/common (first-use / no-results / error variants, centralized copy, CLI command and agent-prompt copy rows) and wire the guardrails list onto it. Scoped to /workspaces/:name/guardrails to verify in one place before migrating the remaining callsites (ASTD-394). Signed-off-by: Aaron Hunt <aahunt@nvidia.com>
…help Replace the custom copy rows in EntityEmptyState with a single KUI CodeSnippet (built-in copy) whose slotActions hosts a tiny SegmentedControl toggling between the NeMo CLI command and the Ask an Agent prompt. Update the ui-design empty-states reference to match. Signed-off-by: Aaron Hunt <aahunt@nvidia.com>
Wrap the SegmentedControl in a full-width Flex inside the CodeSnippet slotActions so the nemo CLI / Ask an Agent toggle left-aligns above the command, size it tiny, and align the ui-design reference label casing. Signed-off-by: Aaron Hunt <aahunt@nvidia.com>
EntityEmptyState now covers only first-use and no-results; the error variant is removed so failed loads keep routing through ErrorPanel with getErrorMessage(error), surfacing the real failure instead of hardcoded generic copy. Updates the GuardrailsDataView error branch, tests, and the ui-design empty-states reference to match. Signed-off-by: Aaron Hunt <aahunt@nvidia.com>
Contributor
|
Adds first-use and no-results stories for EntityEmptyState so the two governed empty-state variants render in isolation for visual review. Signed-off-by: Aaron Hunt <aahunt@nvidia.com>
aahunt-nv
marked this pull request as ready for review
August 12, 2026 23:30
Contributor
📝 WalkthroughWalkthroughAdds a shared ChangesEmpty-state standardization
Sequence Diagram(s)sequenceDiagram
participant GuardrailsRoute
participant GuardrailsDataView
participant EntityEmptyState
participant GuardrailCreateModal
GuardrailsRoute->>GuardrailsDataView: pass onCreate handler
GuardrailsDataView->>EntityEmptyState: pass entity and derived variant
EntityEmptyState->>GuardrailsRoute: invoke create callback
GuardrailsRoute->>GuardrailCreateModal: open creation modal
Suggested reviewers: Mergeability Score: 🔵 Low · up to The guardrails empty-state migration is localized and covered by passing checks, but the added guidance could lead to incorrect future integrations and the no-results API permits omitting its clear-filters action. The PR is mergeable with explicit owner awareness and follow-up. 🚥 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.
Actionable comments posted: 3
🧹 Nitpick comments (1)
web/packages/studio/src/components/dataViews/GuardrailsDataView/GuardrailsDataView.test.tsx (1)
25-36: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the
onCreatecallback.The helper supplies a mock, but the test checks only button presence. The test can pass if
GuardrailsDataViewignoresonCreateandEntityEmptyStateuses its registry fallback. Pass a spy, clickCreate guardrail config, and assert that the spy was called. The suppliedEntityEmptyStatecontract permits this fallback, so button presence alone does not prove callback wiring.Also applies to: 104-108
🤖 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/GuardrailsDataView/GuardrailsDataView.test.tsx` around lines 25 - 36, Update the GuardrailsDataView test helper and relevant test to use an explicit onCreate spy, click the “Create guardrail config” action, and assert the spy was called. Preserve the existing button-presence assertion while ensuring the test verifies GuardrailsDataView wires onCreate through to EntityEmptyState rather than relying on its registry fallback.
🤖 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/.agents/skills/ui-design/references/empty-states.md`:
- Around line 69-71: Update the createAction guidance in the empty-state
reference to use onCreate for imperative or modal-driven creation and omit to in
that case; do not instruct callsites to pass onClick, since
EmptyStateCreateAction does not define that field.
In
`@web/packages/common/src/components/EntityEmptyState/EntityEmptyState.test.tsx`:
- Line 9: Update the React import in the EntityEmptyState test to use a
type-only import for FC and ReactNode, since both symbols are used exclusively
as types.
In `@web/packages/common/src/components/EntityEmptyState/index.tsx`:
- Around line 23-34: Update EntityEmptyStateProps to a discriminated union keyed
by variant: require onClearFilters when variant is 'no-results', while keeping
it unavailable or optional for other variants. Preserve the existing onCreate
behavior and variant-specific constraints.
---
Nitpick comments:
In
`@web/packages/studio/src/components/dataViews/GuardrailsDataView/GuardrailsDataView.test.tsx`:
- Around line 25-36: Update the GuardrailsDataView test helper and relevant test
to use an explicit onCreate spy, click the “Create guardrail config” action, and
assert the spy was called. Preserve the existing button-presence assertion while
ensuring the test verifies GuardrailsDataView wires onCreate through to
EntityEmptyState rather than relying on its registry fallback.
🪄 Autofix
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: 32d3995f-8010-4de5-b380-f4b3397b7e11
📒 Files selected for processing (9)
web/.agents/skills/ui-design/SKILL.mdweb/.agents/skills/ui-design/references/empty-states.mdweb/packages/common/src/components/EntityEmptyState/EntityEmptyState.stories.tsxweb/packages/common/src/components/EntityEmptyState/EntityEmptyState.test.tsxweb/packages/common/src/components/EntityEmptyState/index.tsxweb/packages/common/src/components/EntityEmptyState/registry.tsweb/packages/studio/src/components/dataViews/GuardrailsDataView/GuardrailsDataView.test.tsxweb/packages/studio/src/components/dataViews/GuardrailsDataView/index.tsxweb/packages/studio/src/routes/guardrails/GuardrailsRoute/index.tsx
Comment on lines
+69
to
+71
| - `createAction?` — **omit** for entities with no in-app create flow (e.g. | ||
| Agents, Members). Use `to` for route navigation, `onClick` for imperative | ||
| flows. Renders as `<Button color="brand">`. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the imperative-create instruction.
EmptyStateCreateAction has no onClick field. Future registry entries that follow this instruction will fail type checking. Instruct callsites to pass onCreate and omit to for modal-driven creation.
🤖 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/.agents/skills/ui-design/references/empty-states.md` around lines 69 -
71, Update the createAction guidance in the empty-state reference to use
onCreate for imperative or modal-driven creation and omit to in that case; do
not instruct callsites to pass onClick, since EmptyStateCreateAction does not
define that field.
| import { ToastProvider } from '@nemo/common/src/providers/toast/ToastProvider'; | ||
| import { render, screen } from '@testing-library/react'; | ||
| import userEvent from '@testing-library/user-event'; | ||
| import { FC, ReactNode } from 'react'; |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a type-only React import.
FC and ReactNode are used only as types. Import them with import type.
🤖 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/common/src/components/EntityEmptyState/EntityEmptyState.test.tsx`
at line 9, Update the React import in the EntityEmptyState test to use a
type-only import for FC and ReactNode, since both symbols are used exclusively
as types.
Comment on lines
+23
to
+34
| export interface EntityEmptyStateProps { | ||
| entity: EntityKey; | ||
| variant: EntityEmptyStateVariant; | ||
| /** | ||
| * Overrides the registry create action's handler (e.g. opens a create modal). | ||
| * When omitted, a `createAction.to` route is navigated to instead. | ||
| * `first-use` only. | ||
| */ | ||
| onCreate?: () => void; | ||
| /** Clears the active filters/search. `no-results` only. */ | ||
| onClearFilters?: () => void; | ||
| className?: string; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Require onClearFilters for no-results.
The optional callback permits a no-results state with no required “Clear filters” action. Model the props as a discriminated union so variant: 'no-results' requires onClearFilters.
🤖 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/common/src/components/EntityEmptyState/index.tsx` around lines
23 - 34, Update EntityEmptyStateProps to a discriminated union keyed by variant:
require onClearFilters when variant is 'no-results', while keeping it
unavailable or optional for other variants. Preserve the existing onCreate
behavior and variant-specific constraints.
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
1 participant
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
Phase 1 of 2
Standardizes Studio empty states behind one shared primitive. Adds an
EntityEmptyStatecomponent + entity registry in@nemo/commonand wires the guardrails list onto it. Scoped intentionally to/workspaces/:name/guardrailsto verify the pattern in one place before migrating the remaining ~27 callsites.Also adds a
ui-designagent skill (underweb/.agents/skills/) whoseempty-statesreference documents the pattern.TableEmptyStatewith ad-hoc icon sizing and title-case copy, and the first-use state rendered no create CTA.Screenshots
first-useno-resultsRelated Issue
Linear: ASTD-394 (no linked GitHub issue).
Changes
EntityEmptyStatecomponent +ENTITY_EMPTY_STATESregistry in@nemo/common(first-use/no-resultsvariants; registry-driven copy, icon, CLI command, and skill prompt; "nemo CLI · Ask an Agent"CodeSnippet+SegmentedControlself-service help).GuardrailsDataViewonto it via the DataViewrenderEmptyState({ hasFiltersApplied, hasSearchApplied })hook; keeprenderErrorStateonErrorPanelwithgetErrorMessage(error).onCreateonGuardrailsRoute); drop the oldemptyStateActionsprop.ui-designskill +empty-statesreference.Type of Change
Quality Gates
Verification
Signed-off-by:traileruv run pre-commit run -apasses, or any blocked checks are identified belowTargeted validation:
pnpm --filter @nemo/common typecheck— passpnpm --filter nemo-studio-ui typecheck— passpnpm --filter @nemo/common test EntityEmptyState— pass (5 tests: first-use heading/subheading, CLI↔Agent toggle, create CTA, CTA omission, no-results clear-filters)pnpm --filter nemo-studio-ui test GuardrailsDataView— pass (2 files, 14 tests; incl.error-panelrenders on request failure)Common/EntityEmptyState):first-useandno-resultsrender as intended (see Screenshots).uv run pre-commit run -a— all code hooks pass (ruff, ruff format, ty typechecks, config-reference, uv-lock-check drift, UI lint-staged, copyright headers, plugin-import guard, merge-conflict). Two hooks fail only on local tooling and are unrelated to this web-only diff (no Helm orpyproject/uv.lockchanges; working tree unmodified by the run):helm-docs(binary not installed locally) anduv-lock(local uv 0.11.29 ≠ pinned 0.9.14; the separateuv-lock-checkdrift hook passes). CI runs both with correct tooling.