feat(studio): Prompts FF and list UI#450
Open
steramae-nvidia wants to merge 2 commits into
Open
Conversation
Signed-off-by: Sean Teramae <steramae@nvidia.com>
📝 WalkthroughWalkthroughAdds a feature-flagged Prompts workspace route and table UI, plus shared Lucide icon test queries and updated icon assertions. ChangesPrompts workspace flow
Lucide icon test queries
Sequence Diagram(s)sequenceDiagram
participant User
participant PROMPTS_ENABLED
participant WorkspaceSideNav
participant promptsRoutes
participant PromptsListRoute
participant PromptsDataView
participant useModelsListPrompts
participant DeleteConfirmationModal
participant useModelsDeletePrompt
PROMPTS_ENABLED-->>WorkspaceSideNav: enables Prompts nav item
PROMPTS_ENABLED-->>promptsRoutes: enables prompts route objects
User->>WorkspaceSideNav: selects Prompts
WorkspaceSideNav->>promptsRoutes: navigates to workspace prompts href
promptsRoutes->>PromptsListRoute: lazy-loads page
PromptsListRoute->>PromptsDataView: renders prompts table
PromptsDataView->>useModelsListPrompts: fetches prompts
useModelsListPrompts-->>PromptsDataView: returns rows
User->>PromptsDataView: clicks Delete
PromptsDataView->>DeleteConfirmationModal: opens confirmation
User->>DeleteConfirmationModal: confirms delete
DeleteConfirmationModal->>PromptsDataView: triggers delete handler
PromptsDataView->>useModelsDeletePrompt: mutateAsync(prompt)
useModelsDeletePrompt-->>PromptsDataView: refetches prompts
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
web/packages/studio/src/components/dataViews/PromptsDataView/index.tsx (1)
21-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse type-only React imports and avoid
React.namespace types here.This segment mixes runtime/type imports and
React.ReactNode/React.ComponentProps; switch toimport type+ direct type names for guideline compliance and cleaner TS output.
As per coding guidelines,web/**/*.{ts,tsx}: Use import type for type-only imports in TypeScript.Proposed patch
-import { ComponentProps, FC, useCallback, useMemo, useState } from 'react'; +import { useCallback, useMemo, useState } from 'react'; +import type { ComponentProps, FC, ReactNode } from 'react'; @@ export interface PromptsDataViewProps { workspace: string; - emptyStateActions?: React.ReactNode; + emptyStateActions?: ReactNode; attributes?: { - Stack?: React.ComponentProps<typeof Stack>; + Stack?: ComponentProps<typeof Stack>; }; }🤖 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/PromptsDataView/index.tsx` around lines 21 - 29, Use type-only React imports in PromptsDataView and stop referencing React namespace types directly. Update the imports in the PromptsDataView component to separate runtime hooks from type-only symbols, and replace React.ReactNode/React.ComponentProps with direct type names in PromptsDataViewProps. Keep Stack’s prop typing intact by using the imported type names directly so the file follows the import type guideline and produces cleaner TS output.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/env/.env.dev.local.sample`:
- Line 34: The env sample currently quotes the boolean flag value, which
triggers dotenv-linter. Update the VITE_FF_PROMPTS_ENABLED entry in the env
sample to use an unquoted false value, keeping the rest of the file unchanged so
it matches dotenv expectations.
In `@web/packages/studio/src/components/dataViews/PromptsDataView/index.tsx`:
- Around line 81-88: The delete flow in handleDeletePrompt currently passes
promptToDelete.name directly into deletePromptMutation.mutateAsync without
checking that name exists. Add a guard in handleDeletePrompt to return early
when promptToDelete.name is missing, and only build the delete payload after
validating the prompt object. Keep the fix localized to handleDeletePrompt and
the deletePromptMutation call so invalid delete requests are never sent.
- Around line 65-70: The current filtering in PromptsDataView only applies to
the already paginated `prompts` array, so search results outside the fetched
page remain hidden while `totalCount` still reflects the unfiltered server
total. Update the `useMemo`/search flow in `PromptsDataView` so the search term
is applied before pagination or at the data source level, and make sure the
count used by the pager reflects the filtered result set instead of
`pagination?.total_results`.
---
Nitpick comments:
In `@web/packages/studio/src/components/dataViews/PromptsDataView/index.tsx`:
- Around line 21-29: Use type-only React imports in PromptsDataView and stop
referencing React namespace types directly. Update the imports in the
PromptsDataView component to separate runtime hooks from type-only symbols, and
replace React.ReactNode/React.ComponentProps with direct type names in
PromptsDataViewProps. Keep Stack’s prop typing intact by using the imported type
names directly so the file follows the import type guideline and produces
cleaner TS output.
🪄 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: f4ab5dce-3017-4ff8-88d3-969152ae6ac6
📒 Files selected for processing (15)
services/studio/src/nmp/studio/env_mappings.pyweb/packages/studio/env/.env.dev.local.sampleweb/packages/studio/env/.env.fastapiweb/packages/studio/src/components/dataViews/DataDesignerJobsDataView/index.tsxweb/packages/studio/src/components/dataViews/PromptsDataView/index.tsxweb/packages/studio/src/constants/constants.tsxweb/packages/studio/src/constants/environment.tsweb/packages/studio/src/constants/featureFlags/featureFlags.tsweb/packages/studio/src/constants/routes.tsweb/packages/studio/src/routes/PromptsListRoute/index.tsxweb/packages/studio/src/routes/WorkspaceLayout/WorkspaceSideNav.tsxweb/packages/studio/src/routes/groups/index.tsweb/packages/studio/src/routes/groups/promptsRoutes.tsxweb/packages/studio/src/routes/index.tsxweb/packages/studio/src/routes/utils.ts
Comment on lines
+65
to
+70
| const filteredPrompts = useMemo(() => { | ||
| if (!searchBar) return prompts; | ||
| return prompts.filter((prompt: Prompt) => | ||
| prompt.name?.toLowerCase().includes(searchBar.toLowerCase()) | ||
| ); | ||
| }, [prompts, searchBar]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# Inspect the component structure and the pagination/search flow around the flagged lines.
git ls-files 'web/packages/studio/src/components/dataViews/PromptsDataView/*' && \
wc -l web/packages/studio/src/components/dataViews/PromptsDataView/index.tsx && \
ast-grep outline web/packages/studio/src/components/dataViews/PromptsDataView/index.tsx --view expandedRepository: NVIDIA-NeMo/nemo-platform
Length of output: 624
🏁 Script executed:
sed -n '1,260p' web/packages/studio/src/components/dataViews/PromptsDataView/index.tsxRepository: NVIDIA-NeMo/nemo-platform
Length of output: 7190
🏁 Script executed:
# Find where prompts are fetched and whether search is server-side or client-side.
rg -n "searchBar|page|pagination|useQuery|useInfiniteQuery|offset|limit|cursor|total" web/packages/studio/src/components/dataViews/PromptsDataView/index.tsxRepository: NVIDIA-NeMo/nemo-platform
Length of output: 620
🏁 Script executed:
# Inspect nearby code if the previous output is large.
sed -n '1,220p' web/packages/studio/src/components/dataViews/PromptsDataView/index.tsxRepository: NVIDIA-NeMo/nemo-platform
Length of output: 7025
🏁 Script executed:
sed -n '1,140p' web/packages/studio/src/components/dataViews/PromptsDataView/index.tsxRepository: NVIDIA-NeMo/nemo-platform
Length of output: 4557
🏁 Script executed:
sed -n '140,240p' web/packages/studio/src/components/dataViews/PromptsDataView/index.tsxRepository: NVIDIA-NeMo/nemo-platform
Length of output: 2808
🏁 Script executed:
sed -n '1,140p' web/packages/studio/src/components/dataViews/PromptsDataView/index.tsxRepository: NVIDIA-NeMo/nemo-platform
Length of output: 4557
🏁 Script executed:
sed -n '140,240p' web/packages/studio/src/components/dataViews/PromptsDataView/index.tsxRepository: NVIDIA-NeMo/nemo-platform
Length of output: 2808
🏁 Script executed:
# Inspect how StudioDataView uses data, searchField, and totalCount.
ast-grep outline web/packages/common/src/components/DataView/StudioDataView.tsx --view expandedRepository: NVIDIA-NeMo/nemo-platform
Length of output: 1120
🏁 Script executed:
fd 'StudioDataView.tsx' web packages . -aRepository: NVIDIA-NeMo/nemo-platform
Length of output: 481
🏁 Script executed:
fd -a 'StudioDataView.tsx' web . | headRepository: NVIDIA-NeMo/nemo-platform
Length of output: 481
🏁 Script executed:
fd -a 'StudioDataView.tsx' . | head -20Repository: NVIDIA-NeMo/nemo-platform
Length of output: 322
🏁 Script executed:
fd -a 'useStudioDataViewState.ts' . | head -20Repository: NVIDIA-NeMo/nemo-platform
Length of output: 163
🏁 Script executed:
fd -a 'StudioDataView.tsx' . | head -20Repository: NVIDIA-NeMo/nemo-platform
Length of output: 322
🏁 Script executed:
fd -a 'useStudioDataViewState.ts' . | head -20Repository: NVIDIA-NeMo/nemo-platform
Length of output: 163
🏁 Script executed:
fd -a 'StudioDataView.tsx' . | head -20Repository: NVIDIA-NeMo/nemo-platform
Length of output: 322
🏁 Script executed:
fd -a 'useStudioDataViewState.ts' . | head -20Repository: NVIDIA-NeMo/nemo-platform
Length of output: 163
Move search before pagination
Search only filters the fetched page, while totalCount still uses pagination?.total_results. That makes matches on other pages invisible and leaves the pager showing unfiltered totals.
🤖 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/PromptsDataView/index.tsx`
around lines 65 - 70, The current filtering in PromptsDataView only applies to
the already paginated `prompts` array, so search results outside the fetched
page remain hidden while `totalCount` still reflects the unfiltered server
total. Update the `useMemo`/search flow in `PromptsDataView` so the search term
is applied before pagination or at the data source level, and make sure the
count used by the pager reflects the filtered result set instead of
`pagination?.total_results`.
Comment on lines
+81
to
+88
| const handleDeletePrompt = async () => { | ||
| if (!promptToDelete) return false; | ||
|
|
||
| try { | ||
| await deletePromptMutation.mutateAsync({ | ||
| workspace, | ||
| name: promptToDelete.name, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Guard delete payload when name is missing.
mutateAsync is called with name: promptToDelete.name without validating name; this can send an invalid delete request.
Proposed patch
const handleDeletePrompt = async () => {
- if (!promptToDelete) return false;
+ if (!promptToDelete?.name) {
+ toast.error('Prompt name is missing');
+ return false;
+ }
try {
await deletePromptMutation.mutateAsync({
workspace,
name: promptToDelete.name,
});📝 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.
Suggested change
| const handleDeletePrompt = async () => { | |
| if (!promptToDelete) return false; | |
| try { | |
| await deletePromptMutation.mutateAsync({ | |
| workspace, | |
| name: promptToDelete.name, | |
| }); | |
| const handleDeletePrompt = async () => { | |
| if (!promptToDelete?.name) { | |
| toast.error('Prompt name is missing'); | |
| return false; | |
| } | |
| try { | |
| await deletePromptMutation.mutateAsync({ | |
| workspace, | |
| name: promptToDelete.name, | |
| }); |
🤖 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/PromptsDataView/index.tsx`
around lines 81 - 88, The delete flow in handleDeletePrompt currently passes
promptToDelete.name directly into deletePromptMutation.mutateAsync without
checking that name exists. Add a guard in handleDeletePrompt to return early
when promptToDelete.name is missing, and only build the delete payload after
validating the prompt object. Keep the fix localized to handleDeletePrompt and
the deletePromptMutation call so invalid delete requests are never sent.
Contributor
|
Signed-off-by: Sean Teramae <steramae@nvidia.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/packages/common/src/tests/lucideIconQueries.ts (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a type-only import for
Matcher.
Matcheris type-only here; import it withimport typeto satisfy TS/lint rules and avoid value import emission.Proposed fix
-import { buildQueries, Matcher } from '`@testing-library/react`'; +import { buildQueries } from '`@testing-library/react`'; +import type { Matcher } from '`@testing-library/react`';As per coding guidelines, “Use
import typefor type-only 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/common/src/tests/lucideIconQueries.ts` at line 4, The `lucideIconQueries` test file is importing `Matcher` as a value even though it is only used as a type. Update the existing import near `buildQueries` to use a type-only import for `Matcher`, keeping the runtime import for `buildQueries` unchanged so the TypeScript/lint rule is satisfied and no unnecessary value import is emitted.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/common/src/tests/lucideIconQueries.ts`:
- Line 4: The `lucideIconQueries` test file is importing `Matcher` as a value
even though it is only used as a type. Update the existing import near
`buildQueries` to use a type-only import for `Matcher`, keeping the runtime
import for `buildQueries` unchanged so the TypeScript/lint rule is satisfied and
no unnecessary value import is emitted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9bd53c6d-7b41-47d0-9b90-42bf9b99be2b
📒 Files selected for processing (5)
web/packages/common/src/components/StatusBadge/StatusBadge.test.tsxweb/packages/common/src/tests/customQueries.tsweb/packages/common/src/tests/lucideIconQueries.tsweb/packages/studio/src/components/Layouts/NavigationDrawer/index.test.tsxweb/packages/studio/src/routes/SafeSynthesizerJobDetailsRoute/components/JobDetailsPanel.test.tsx
✅ Files skipped from review due to trivial changes (1)
- web/packages/studio/src/components/Layouts/NavigationDrawer/index.test.tsx
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 by CodeRabbit