Conversation
…th` property with fallback for `displayName`.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe changes restructure the AI and MCP settings UI by introducing a configuration indicator component, simplifying the AI settings form to handle connections only with auto-save functionality, removing the dedicated MCP settings form, and refactoring the settings page to present a combined AI/AWS layout with unified mutation flows. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50–70 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
… adjust styles, and add conditional message for unconfigured state.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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 (2)
packages/react-ui/src/app/features/ai/ai-config-indicator.tsx (1)
8-26: LGTM! Clean and simple indicator component.The component follows React best practices with proper conditional rendering and i18n support. Consider adding an explicit return type for consistency with coding guidelines:
-const AiConfigIndicator = ({ enabled }: AiConfigIndicatorProps) => { +const AiConfigIndicator = ({ enabled }: AiConfigIndicatorProps): JSX.Element => {packages/react-ui/src/app/features/ai/ai-settings-form.tsx (1)
42-48: Potential issue:formin dependency array may cause unnecessary resets.The
formobject fromuseFormmaintains a stable reference, but including it in the dependency array is unconventional. If the reference ever changes, this effect would reset the form unexpectedly. Consider usingform.resetdirectly:useEffect(() => { const formValue: LocalForm = { connection: initialConnection ?? '', }; setInitialFormValue(formValue); form.reset(formValue); - }, [initialConnection, form]); + }, [initialConnection, form.reset]);Alternatively, if
formis confirmed stable, you can safely suppress the lint warning.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/react-ui/src/app/features/ai/ai-config-indicator.tsx(1 hunks)packages/react-ui/src/app/features/ai/ai-settings-form.tsx(1 hunks)packages/react-ui/src/app/features/ai/mcp-settings-form.tsx(0 hunks)packages/react-ui/src/app/routes/settings/ai/index.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- packages/react-ui/src/app/features/ai/mcp-settings-form.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Indentation: 2 spaces for TypeScript/JavaScript
Braces required for all control blocks, even single-line
One space between keywords and parentheses:if (condition) {
Use camelCase for variables and functions
Use PascalCase for classes and types
Use UPPER_SNAKE_CASE for constants
Use lowercase with hyphens for file names (e.g.,user-profile.ts)
Preferconstoverletorvarin TypeScript/JavaScript
Prefer arrow functions for callbacks and functional components in TypeScript/JavaScript
Files:
packages/react-ui/src/app/features/ai/ai-config-indicator.tsxpackages/react-ui/src/app/features/ai/ai-settings-form.tsxpackages/react-ui/src/app/routes/settings/ai/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx}: Use types and interfaces where appropriate in TypeScript
Use explicit return types for exported functions in TypeScript
Files:
packages/react-ui/src/app/features/ai/ai-config-indicator.tsxpackages/react-ui/src/app/features/ai/ai-settings-form.tsxpackages/react-ui/src/app/routes/settings/ai/index.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{tsx,ts}: Frontend tech stack must strictly use: React 18, Zustand, react-query v5, shadcn, and Axios withqspackage for query strings
Follow best practices for React hooks
Prefer small, composable components and extract helper functions where possible
Usecnutility to group tailwind classnames in React components
Files:
packages/react-ui/src/app/features/ai/ai-config-indicator.tsxpackages/react-ui/src/app/features/ai/ai-settings-form.tsxpackages/react-ui/src/app/routes/settings/ai/index.tsx
🧠 Learnings (3)
📚 Learning: 2025-11-25T10:22:44.853Z
Learnt from: CR
Repo: openops-cloud/openops PR: 0
File: .cursor/rules/coding-guidelines.mdc:0-0
Timestamp: 2025-11-25T10:22:44.853Z
Learning: Applies to **/*.{tsx,ts} : Follow best practices for React hooks
Applied to files:
packages/react-ui/src/app/features/ai/ai-settings-form.tsx
📚 Learning: 2025-11-25T10:22:44.853Z
Learnt from: CR
Repo: openops-cloud/openops PR: 0
File: .cursor/rules/coding-guidelines.mdc:0-0
Timestamp: 2025-11-25T10:22:44.853Z
Learning: Use the `react-ui` package for main application logic in the frontend
Applied to files:
packages/react-ui/src/app/features/ai/ai-settings-form.tsx
📚 Learning: 2025-11-25T10:22:44.853Z
Learnt from: CR
Repo: openops-cloud/openops PR: 0
File: .cursor/rules/coding-guidelines.mdc:0-0
Timestamp: 2025-11-25T10:22:44.853Z
Learning: Applies to **/*.{tsx,ts} : Frontend tech stack must strictly use: React 18, Zustand, react-query v5, shadcn, and Axios with `qs` package for query strings
Applied to files:
packages/react-ui/src/app/features/ai/ai-settings-form.tsx
🧬 Code graph analysis (1)
packages/react-ui/src/app/routes/settings/ai/index.tsx (8)
packages/react-ui/src/app/features/ai/lib/ai-form-utils.ts (3)
AiSettingsFormSchema(12-12)AI_SETTINGS_SAVED_SUCCESSFULLY_TOAST(14-18)MCP_SETTINGS_SAVED_SUCCESSFULLY_TOAST(26-30)packages/ui-components/src/ui/use-toast.tsx (1)
toast(213-213)packages/react-ui/src/app/lib/query-client.ts (1)
queryClient(3-3)packages/react-ui/src/app/constants/query-keys.ts (1)
QueryKeys(1-70)packages/shared/src/lib/common/application-error.ts (1)
ApplicationErrorParams(16-71)packages/react-ui/src/app/features/ai/ai-config-indicator.tsx (1)
AiConfigIndicator(28-28)packages/react-ui/src/app/features/ai/ai-settings-form.tsx (1)
AiSettingsForm(95-95)packages/shared/src/lib/app-connection/connections-utils.ts (1)
removeConnectionBrackets(6-16)
🔇 Additional comments (4)
packages/react-ui/src/app/features/ai/ai-settings-form.tsx (3)
57-66: VerifyonSaveis memoized by the parent to avoid debounce resets.The
debouncedSavefunction is recreated wheneveronSavechanges. If the parent component passes an inline callback, this will recreate the debounced function on every render, effectively breaking the debounce behavior.Ensure the parent wraps
onSaveinuseCallback, or consider using a ref to hold the latestonSave:+const onSaveRef = React.useRef(onSave); +useEffect(() => { + onSaveRef.current = onSave; +}, [onSave]); const debouncedSave = useMemo( () => debounce((connection: string, unchanged: boolean) => { if (!unchanged) { - onSave(connection); + onSaveRef.current(connection); setInitialFormValue({ connection }); } }, 300), - [onSave], + [], );
72-76: Good cleanup pattern for the debounced function.The cleanup effect properly cancels pending debounced calls when the component unmounts or when
debouncedSavechanges, preventing memory leaks and stale saves.
78-91: Form structure is appropriate for auto-save pattern.The
<form>wrapper without anonSubmithandler is acceptable here since saving is handled via the debounced effect. The structure integrates well with react-hook-form's Form component.packages/react-ui/src/app/routes/settings/ai/index.tsx (1)
89-157: Well-structured UI layout with proper loading states.The combined AI and MCP settings layout is clean, with appropriate Skeleton placeholders during loading and conditional guidance text when AI is not configured. The integration of
AiConfigIndicatorprovides clear visual feedback to users.
There was a problem hiding this comment.
Pull request overview
This PR updates the AI connections page with a new UI design. The changes streamline the configuration interface by consolidating AI and MCP settings into a single form component with automatic saving, removing the need for separate save/cancel/delete buttons.
Key Changes:
- Introduced automatic save-on-change functionality with debouncing for connection selections
- Added a visual status indicator showing whether OpenOps AI is enabled or disabled
- Consolidated MCP settings form into the main AI settings form component
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/react-ui/src/app/routes/settings/ai/index.tsx |
Restructured the AI settings page layout, integrated the new AI config indicator, and consolidated both AI and MCP configuration into unified interface with improved error handling |
packages/react-ui/src/app/features/ai/mcp-settings-form.tsx |
Removed the standalone MCP settings form component |
packages/react-ui/src/app/features/ai/ai-settings-form.tsx |
Refactored to support auto-save functionality with debouncing, removed manual save/cancel buttons, and made the component more generic to handle both AI and MCP connections |
packages/react-ui/src/app/features/ai/ai-config-indicator.tsx |
Added new component to display AI configuration status with visual indicators |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile OverviewGreptile SummaryThis PR refactors the AI settings page to consolidate AI and MCP tool configuration into a unified interface with improved UX. The changes introduce auto-save functionality with debouncing, remove manual save/cancel buttons, and add a visual indicator for AI configuration status. Key Changes:
Critical Issues Found:
Confidence Score: 2/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant AiSettingsPage
participant AiSettingsForm
participant ConnectionSelect
participant API
User->>AiSettingsPage: Configure AI connection
AiSettingsPage->>AiSettingsForm: Render form with initialConnection
AiSettingsForm->>ConnectionSelect: Display connection selector
User->>ConnectionSelect: Select connection
ConnectionSelect->>AiSettingsForm: field.onChange({{connections['name']}})
Note over ConnectionSelect,AiSettingsForm: Connection value includes brackets
AiSettingsForm->>AiSettingsForm: watchedConnection triggers useEffect
AiSettingsForm->>AiSettingsForm: debouncedSave(connection, isFormUnchanged)
AiSettingsForm->>AiSettingsPage: onSave(connection) after 300ms
Note over AiSettingsForm,AiSettingsPage: ⚠️ Brackets not stripped for AI
AiSettingsPage->>AiSettingsPage: handleSaveAi(connectionName)
AiSettingsPage->>API: saveAiSettings({connection: bracketed value})
Note over AiSettingsPage,API: ⚠️ May cause issues if backend expects plain name
API-->>AiSettingsPage: Success/Error
AiSettingsPage->>AiSettingsPage: refetch settings
User->>AiSettingsPage: Configure MCP connection
AiSettingsPage->>AiSettingsForm: Render MCP form
User->>ConnectionSelect: Select AWS connection
ConnectionSelect->>AiSettingsForm: field.onChange({{connections['aws-name']}})
AiSettingsForm->>AiSettingsPage: onSave(connection)
AiSettingsPage->>AiSettingsPage: handleSaveMcp(connectionName)
Note over AiSettingsPage: ✓ Brackets removed via removeConnectionBrackets
AiSettingsPage->>API: saveMcpSettings({connectionName: plain name})
API-->>AiSettingsPage: Success
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| }, | ||
| const isAiConfigured = !!aiSettings?.[0]?.connection; | ||
|
|
||
| const handleSaveAi = useCallback( |
There was a problem hiding this comment.
nitpick would call it handleSaveAiConfig and for mcp handleSaveMcpConfig
Co-authored-by: Alexandru-Dan Pop <alexandrudanpop@gmail.com>
…to feat/update-connections-page
|
Part of OPS-3164.
Available on UX
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.