Conversation
Greptile SummaryRefactors user settings from a modal-based approach to a dedicated full-page experience at
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Sidebar
participant UserAvatarPopover
participant SettingsPage
participant API
User->>Sidebar: Click user avatar
Sidebar->>UserAvatarPopover: Open popover
UserAvatarPopover->>API: Fetch notifications
API-->>UserAvatarPopover: Return notifications
UserAvatarPopover-->>User: Show popover menu
User->>UserAvatarPopover: Click "User Settings"
UserAvatarPopover->>SettingsPage: Navigate to /chat/settings
SettingsPage->>API: Fetch user data, PATs, connectors
API-->>SettingsPage: Return data
SettingsPage-->>User: Display settings tabs
User->>SettingsPage: Modify settings
SettingsPage->>API: Save changes
API-->>SettingsPage: Confirm save
SettingsPage-->>User: Show success message
|
There was a problem hiding this comment.
3 issues found across 10 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="web/src/refresh-pages/SettingsPage.tsx">
<violation number="1" location="web/src/refresh-pages/SettingsPage.tsx:686">
P1: Direct mutation of state object. Instead of mutating `personalizationValues.memories` directly, the parent should expose a function to update memories (like `setMemories`) or the hook should accept the new memories as a parameter to `handleSavePersonalization`.</violation>
<violation number="2" location="web/src/refresh-pages/SettingsPage.tsx:924">
P1: Toggle state is not persisted. `toggleUseMemories` only updates local state but doesn't call `handleSavePersonalization`. The user's preference change will be lost on page refresh. Consider calling `handleSavePersonalization()` after toggling, similar to how other preference switches persist their changes.</violation>
</file>
<file name="web/src/sections/sidebar/UserAvatarPopover.tsx">
<violation number="1" location="web/src/sections/sidebar/UserAvatarPopover.tsx:63">
P2: Add error handling for promise rejection. The `logout()` call only has `.then()` but no `.catch()` to handle network errors or other failures, which could result in unhandled promise rejections.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 8 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="web/src/refresh-pages/SettingsPage.tsx">
<violation number="1" location="web/src/refresh-pages/SettingsPage.tsx:1249">
P2: `isSubmitting` is never set to true in the manual submit handler, so the button never enters a loading/disabled state and users can trigger multiple password-change requests while one is in flight. Set `setSubmitting(true)` before awaiting the request (and reset it in a finally block) to keep the button disabled during submission.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="web/tests/e2e/auth/pat_management.spec.ts">
<violation number="1" location="web/tests/e2e/auth/pat_management.spec.ts:35">
P2: This wait is no longer tied to the PAT page content; it can pass immediately because the Access Tokens tab text is already visible. Use a selector scoped to the page heading or a unique element on the PAT page so the test actually waits for navigation to finish.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
The tab is rendered as a link, not a button. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Subash-Mohan
left a comment
There was a problem hiding this comment.
Can’t understand much, but it looks very clean and good. One small nit: it would be better if we split the settings page into two. But I’m going to leave it to you. I’m approving the PR.
Ensures button is re-enabled even if the request fails. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The index is already constrained to >= 0 by all state setters. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
@cubic-dev-ai, please re-review this. |
@raunakab I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
4 issues found across 26 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="web/src/refresh-pages/SettingsPage.tsx">
<violation number="1" location="web/src/refresh-pages/SettingsPage.tsx:229">
P2: Keep the delete confirmation modal open when deletion fails; only close it on success so users can see the error and retry without reopening.
(Based on your team's feedback about keeping modals open on error to allow popups to render.) [FEEDBACK_USED]</violation>
<violation number="2" location="web/src/refresh-pages/SettingsPage.tsx:1105">
P2: Only close the revoke-token modal on successful deletion; keep it open on error so the popup can render and the user can retry.
(Based on your team's feedback about keeping modals open on error to allow popups to render.) [FEEDBACK_USED]</violation>
</file>
<file name="web/src/app/chat/components/input/ChatInputBar.tsx">
<violation number="1" location="web/src/app/chat/components/input/ChatInputBar.tsx:386">
P2: handleKeyDown should also check the user shortcut preference; otherwise users who disabled shortcuts can no longer submit messages that start with "/" because Enter is prevented.</violation>
<violation number="2" location="web/src/app/chat/components/input/ChatInputBar.tsx:390">
P2: Use sortedFilteredPrompts for keyboard selection so the Enter action matches the highlighted prompt order shown in the UI.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
handleKeyDown was intercepting Enter key when showPrompts was true, even if shortcuts were disabled. This prevented users with disabled shortcuts from submitting messages starting with '/'. Now checks shortcut_enabled preference to match Popover open condition. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add navigation to Chat Preferences tab where auto-scroll setting lives - Fix selector to find Switch by label text instead of aria-label - The Switch component doesn't have aria-label, so we locate the parent label with "Chat Auto-scroll" text and find the switch within Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Settings is no longer a modal, so Escape doesn't close it. Click "New Session" button to navigate back to chat. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The UI renders prompts using sortedFilteredPrompts (sorted by id), but the keyboard handler was using filteredPrompts (unsorted). This caused a mismatch where pressing Enter would select a different prompt than the one visually highlighted. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Description
Addresses: https://linear.app/onyx-app/project/user-settings-1646662d4e19.
Screenshots
General:
Chat Preferences:
Accounts & Access:
Connectors:
Summary by cubic
Refreshed and consolidated user settings into a dedicated page with a cleaner UI and simpler navigation. Settings now live at /chat/settings (linked from the avatar popover), with fixed notification counts, personalization save/toggle, improved logout errors, duplicate protection for prompt shortcuts, and correct message submission when shortcuts are disabled.
New Features
Refactors
Written for commit cad138d. Summary will update on new commits.