-
Notifications
You must be signed in to change notification settings - Fork 577
UN-3020 [FEAT] Agentic prompt studio frontend #1737
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds two frontend dependencies, integrates an optionally-loaded Agentic Prompt Studio plugin (runtime require, route, and sidebar injection), and refactors ListOfTools to extract DefaultCustomButtons, centralize exception handling, and add full-height tab styling. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Router
participant PluginLoader as AgenticPluginLoader
participant Sidebar
participant PluginModule as AgenticPromptStudio
Browser->>Router: app initialization / navigation
Router->>PluginLoader: attempt runtime require AgenticPromptStudio (try-catch)
PluginLoader-->>Router: module returned or error
alt plugin found
Router->>Sidebar: inject Agentic Prompt Studio menu item
Router->>PluginModule: register/mount route at /agentic-prompt-studio/*
Sidebar-->>Browser: render menu with plugin entry
Browser->>PluginModule: navigate to plugin route
else plugin absent
Router-->>Sidebar: render default menu
Sidebar-->>Browser: render menu without plugin entry
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx:
- Around line 244-256: The new submenu entry added when
agenticPromptStudioEnabled sets id: 1.2 which duplicates the existing Workflows
id and will cause React key conflicts; update the inserted object in the
data[0]?.subMenu?.splice(...) call to use a unique identifier (preferably a
string like "agentic-prompt-studio" for the id) and ensure any code that
references the numeric id is updated to handle the new string id; keep the rest
of the properties (title, path, active check using
window.location.pathname.startsWith(`/${orgName}/agentic-prompt-studio`))
unchanged.
🧹 Nitpick comments (1)
frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx (1)
244-264: Consider memoizing or extractingDefaultCustomButtons.
DefaultCustomButtonsis defined inside the component, creating a new function reference on every render. Since it accessesisImportLoading,setOpenImportTool, andhandleNewProjectBtnClickfrom closure, it cannot be extracted outside. Consider wrapping withuseCallbackoruseMemoto stabilize the reference, which can help prevent unnecessary re-renders inToolNavBar.♻️ Suggested refactor using useMemo
+import { useEffect, useState, useMemo } from "react"; -import { useEffect, useState } from "react"; ... - const DefaultCustomButtons = () => { - return ( + const DefaultCustomButtons = useMemo( + () => () => ( <Space gap={16}> <CustomButton type="default" icon={<ArrowDownOutlined />} onClick={() => setOpenImportTool(true)} loading={isImportLoading} > Import Project </CustomButton> <CustomButton type="primary" icon={<PlusOutlined />} onClick={handleNewProjectBtnClick} > New Project </CustomButton> </Space> - ); - }; + ), + [isImportLoading, handleNewProjectBtnClick] + );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
frontend/package.jsonfrontend/src/components/custom-tools/list-of-tools/ListOfTools.cssfrontend/src/components/custom-tools/list-of-tools/ListOfTools.jsxfrontend/src/components/navigations/side-nav-bar/SideNavBar.jsxfrontend/src/routes/useMainAppRoutes.js
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx (1)
frontend/src/hooks/useExceptionHandler.jsx (1)
useExceptionHandler(4-127)
frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx (1)
frontend/src/components/helpers/auth/RequireAuth.js (1)
orgName(33-33)
🔇 Additional comments (7)
frontend/src/routes/useMainAppRoutes.js (2)
53-57: LGTM!The plugin loading pattern is consistent with other plugins in this file (e.g., RequirePlatformAdmin, AppDeployments). The try-catch with an empty catch block is appropriate for optional plugins.
160-165: LGTM!The conditional route rendering follows the established pattern for optional plugins. The wildcard path (
agentic-prompt-studio/*) correctly allows for nested routing within the plugin.frontend/src/components/custom-tools/list-of-tools/ListOfTools.css (1)
10-27: LGTM!The flex-based tab layout styling correctly establishes full-height behavior for Ant Design tabs. The
overflow: hiddenon the content holder andheight: 100%cascading through the tab structure is a standard pattern for containing scrollable tab content.frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx (2)
28-28: LGTM!The
useExceptionHandlerhook is properly integrated and used consistently throughout the component for error handling in API calls.
340-357: LGTM!The extraction of
defaultContentimproves readability and prepares the component structure for potential future tab-based layouts (as suggested by the new CSS classes). The rendering logic is clean and maintainable.Also applies to: 370-370
frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx (1)
61-67: LGTM!The plugin detection pattern is consistent with other plugins in this file. Setting a boolean flag at module load time is appropriate for conditional feature enablement.
frontend/package.json (1)
41-41: Bothreact-diff-viewer-continued@^3.4.0andrecharts@^3.5.1exist on npm and have no known security vulnerabilities. The specified versions are valid and safe to use.
Resolved conflicts in SideNavBar.jsx by: - Keeping both agenticPromptStudioEnabled and manualReviewSettingsEnabled plugin checks - Adding new Settings menu functions (getSettingsMenuItems, getActiveSettingsKey, SettingsPopoverContent) - Preserving agentic prompt studio menu item addition
Changed id from 1.2 to 1.15 to avoid conflict with Workflows menu item. Resolves CodeRabbit review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx (1)
328-340: Implementation is consistent with existing patterns.The menu injection follows the same pattern as the app_deployment injection at line 325. The optional chaining handles the edge case when
datais empty.Minor concern: Both this and line 325 mutate
datain place, which could bemenureturned from an external hook. If that hook returns a stable reference, this mutation could affect other consumers or cause subtle re-render issues.Consider cloning before mutation to avoid side effects:
♻️ Optional defensive clone
const data = selectedProduct === "verticals" && menu === null ? [] - : menu || unstractMenuItems; + : JSON.parse(JSON.stringify(menu || unstractMenuItems));Alternatively, use a
useMemoto derive the final menu structure without mutating the source.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx (1)
frontend/src/components/helpers/auth/RequireAuth.js (1)
orgName(33-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx (1)
70-76: LGTM!The plugin detection pattern is consistent with other optional plugin loads in this file (e.g., lines 34-46, 62-68, 78-84). The approach correctly sets a boolean flag based on plugin availability.
- Renumbered Workflows from 1.2 to 1.3 - Set Agentic Prompt Studio to 1.2 - Maintains consistent single decimal numbering (1.1, 1.2, 1.3)
1. ListOfTools.jsx: Move DefaultCustomButtons component outside parent - Extracted component definition to module level - Added PropTypes for type safety - Pass required props from parent component - Improves performance by preventing component recreation on each render 2. SideNavBar.jsx: Replace window with globalThis - Changed all window.location references to globalThis.location - Improves portability and ES2020 compatibility
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx (1)
378-384: Inline arrow function inCustomButtonsprop creates new reference each render.While
DefaultCustomButtonswas correctly extracted to module level, wrapping it in an inline arrow function() => (...)creates a new function reference on every render. This partially negates the performance benefit and can cause unnecessary re-renders ofToolNavBarif it uses memoization.♻️ Consider using useCallback to stabilize the reference
+ import { useEffect, useState, useCallback } from "react"; ... + const renderCustomButtons = useCallback( + () => ( + <DefaultCustomButtons + setOpenImportTool={setOpenImportTool} + isImportLoading={isImportLoading} + handleNewProjectBtnClick={handleNewProjectBtnClick} + /> + ), + [isImportLoading, handleNewProjectBtnClick] + ); ... <ToolNavBar title={"Prompt Studio"} enableSearch onSearch={onSearch} searchList={listOfTools} setSearchList={setFilteredListOfTools} - CustomButtons={() => ( - <DefaultCustomButtons - setOpenImportTool={setOpenImportTool} - isImportLoading={isImportLoading} - handleNewProjectBtnClick={handleNewProjectBtnClick} - /> - )} + CustomButtons={renderCustomButtons} />Note:
handleNewProjectBtnClickwould also need to be wrapped inuseCallbackfor this to be fully effective.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsxfrontend/src/components/navigations/side-nav-bar/SideNavBar.jsx
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx (1)
frontend/src/hooks/useExceptionHandler.jsx (1)
useExceptionHandler(4-127)
frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx (1)
frontend/src/components/helpers/auth/RequireAuth.js (1)
orgName(33-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx (3)
70-76: LGTM!The plugin detection pattern is consistent with existing plugin detection blocks in this file (e.g., lines 34-46, 78-84). Using a flag set via try-catch on dynamic require is a clean approach for optional plugin availability.
191-202: Consistent use ofglobalThis.locationfor portability.The change from
window.locationtoglobalThis.locationfollows ES2020+ standards and improves portability across different JavaScript environments. The ID renumbering (1.3 for Workflows) accommodates the new Agentic Prompt Studio entry.
330-342: Agentic Prompt Studio menu injection is correctly implemented.The menu item is properly spliced at position 1 with ID 1.2, maintaining consistent ordering. The optional chaining guards against undefined submenu.
Note: The mutation pattern via
spliceis consistent with existing menu injections in this component (lines 18 and 28). The safety of this approach depends on whetherdatareferences a cached array from the plugin hook or a fresh array created each render. SinceunstractMenuItemsis constructed fresh per render andmenucomes from a React hook call, mutations should be safe under typical React hook patterns. However, the ultimate responsibility lies with the plugin'suseSideMenuhook implementation.frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx (3)
19-49: Good extraction ofDefaultCustomButtonsto module level.Extracting this component to module level with proper PropTypes prevents recreation on each render of
ListOfTools, addressing performance concerns. The component structure is clean and follows React conventions.
61-61: LGTM!Using the
handleExceptionhook follows the centralized exception handling pattern documented inuseExceptionHandler.jsx, ensuring consistent error handling across API calls in this component.
351-368: LGTM!The
defaultContentextraction improves readability by separating the ViewTools configuration from the render tree. This prepares the component for potential plugin-driven content variations.
vishnuszipstack
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Deepak-Kesavan check the sonar issues. if possible to resolve pls do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx`:
- Around line 370-379: The useCallback for CustomButtonsComponent closes over
handleNewProjectBtnClick but does not include it in the dependency array,
risking stale closures; either add handleNewProjectBtnClick to the dependency
array of the useCallback that defines CustomButtonsComponent, or stabilize
handleNewProjectBtnClick itself by wrapping it in useCallback and then include
that stable reference in the deps; ensure dependencies reflect
setOpenImportTool/isImportLoading/handleNewProjectBtnClick so
DefaultCustomButtons receives up-to-date props.
- Line 25: Replace the unsupported gap prop on the Ant Design Space component
with the supported size prop: find the Space usage in the ListOfTools.jsx
component (the JSX that currently reads <Space gap={16}>) and change it to use
<Space size={16}> so spacing works with Ant Design 5.5.1.
|



What
Why
How
Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
No
Database Migrations
N/A
Env Config
N/A
Relevant Docs
Related Issues or PRs
UN-3020
Dependencies Versions
Notes on Testing
Screenshots
Checklist
I have read and understood the Contribution Guidelines.