Repository navigation
Codex/wework device model updates - #1313
Conversation
# Conflicts: # wework/src/components/chat/ChatInput.test.tsx # wework/src/components/layout/DesktopWorkbenchLayout.test.tsx
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR hides the Gemini model family from model grouping and adds a flow to navigate from ProjectCreateDialog to Connections > Add Cloud Device (auto-opening the add dialog). It wires a new callback through DesktopSidebar and DesktopWorkbenchLayout, updates ConnectionsSettingsPage to accept an auto-open flag, and adds tests and i18n keys. ChangesHide Gemini Models from Selection
Cloud Device Settings Navigation Flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wework/src/components/chat/ChatInput.test.tsx (1)
1-2370: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftSplit this oversized test file before adding more cases
Line 1 onward: this file is now 2370 lines, which violates the repository file-size cap and is already difficult to maintain. Please move related test groups into smaller spec modules (e.g., model-selector, local-skill, project-work suites) and keep this file under 1000 lines.
As per coding guidelines,
**/*.{ts,tsx,js,jsx}: "File size limit: If a file exceeds 1000 lines, split it into multiple sub-modules".🤖 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 `@wework/src/components/chat/ChatInput.test.tsx` around lines 1 - 2370, The ChatInput.test.tsx file (describe('ChatInput')) has grown >1000 lines; split it into smaller spec modules by extracting related test groups (e.g., local-skill tests, model-selector tests, project-work tests, attachment/preview tests, composer-behavior tests) and keep helper components ControlledChatInput, projectChatControls, and projectWorkControls available as shared utilities. Create new files like ChatInput.local-skill.test.tsx, ChatInput.model-selector.test.tsx, ChatInput.project-work.test.tsx, ChatInput.attachments.test.tsx and move the corresponding tests (identify tests by their titles and test ids such as 'opens local skill autocomplete', 'opens the desktop model menu', 'opens project work menu', 'renders image preview' etc.) into those files; either export the helper functions from a new test-utils module or import them from a shared test helper file so each spec can reuse ControlledChatInput, projectChatControls, projectWorkControls, and keep the original imports (fireEvent/render/screen/userEvent). Ensure each new test file includes the same setup/teardown (afterEach restoring mocks, URL.createObjectURL) and run the test suite to confirm all tests still pass and the original ChatInput.test.tsx is reduced below the 1000-line limit.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.
Outside diff comments:
In `@wework/src/components/chat/ChatInput.test.tsx`:
- Around line 1-2370: The ChatInput.test.tsx file (describe('ChatInput')) has
grown >1000 lines; split it into smaller spec modules by extracting related test
groups (e.g., local-skill tests, model-selector tests, project-work tests,
attachment/preview tests, composer-behavior tests) and keep helper components
ControlledChatInput, projectChatControls, and projectWorkControls available as
shared utilities. Create new files like ChatInput.local-skill.test.tsx,
ChatInput.model-selector.test.tsx, ChatInput.project-work.test.tsx,
ChatInput.attachments.test.tsx and move the corresponding tests (identify tests
by their titles and test ids such as 'opens local skill autocomplete', 'opens
the desktop model menu', 'opens project work menu', 'renders image preview'
etc.) into those files; either export the helper functions from a new test-utils
module or import them from a shared test helper file so each spec can reuse
ControlledChatInput, projectChatControls, projectWorkControls, and keep the
original imports (fireEvent/render/screen/userEvent). Ensure each new test file
includes the same setup/teardown (afterEach restoring mocks,
URL.createObjectURL) and run the test suite to confirm all tests still pass and
the original ChatInput.test.tsx is reduced below the 1000-line limit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0b248835-8eb5-4861-8513-9430cb4e1eae
📒 Files selected for processing (9)
wework/src/components/chat/ChatInput.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/components/projects/ProjectCreateDialog.test.tsxwework/src/components/projects/ProjectCreateDialog.tsxwework/src/components/settings/ConnectionsSettingsPage.tsxwework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/lib/model-ui.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
wework/src/components/chat/ChatInput.test.tsx (1)
1073-1121: ⚡ Quick winAdd the hidden-selected-model edge case for Gemini filtering.
This test validates mixed-model visibility, but it doesn’t cover when
selectedModelis Gemini (hidden). Add a case asserting the menu still excludes Gemini and remains usable with visible families.As per coding guidelines, test files should cover edge cases and error conditions.
🤖 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 `@wework/src/components/chat/ChatInput.test.tsx` around lines 1073 - 1121, The test misses the edge case where the current selectedModel is a Gemini model that should be hidden; add a new test in ChatInput.test.tsx (using the existing pattern) that renders <ChatInput ... variant="desktop" projectChat={projectChatControls({ models: [gptModel, geminiModel], selectedModel: geminiModel, selectedModelOptions: {} })}>, opens the model menu via the model-selector-button, and asserts that model-family-gemini and model-option-overseas-gemini-3-pro are not rendered while visible families (e.g., model-family-gpt) remain present and the menu stays usable (no errors). Ensure you reuse the existing gptModel and geminiModel fixtures and assertions style to keep tests consistent.Source: Coding guidelines
wework/src/components/layout/DesktopSidebar.tsx (1)
762-1286: ⚖️ Poor tradeoffRefactor the DesktopSidebar function to stay within the preferred size limit.
The
DesktopSidebarfunction spans 524 lines (lines 762-1286), far exceeding the preferred 50-line limit specified in the coding guidelines. Consider extracting logical sections into separate components or hooks (e.g., sidebar state management, device status logic, project list rendering).🤖 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 `@wework/src/components/layout/DesktopSidebar.tsx` around lines 762 - 1286, DesktopSidebar is too large — extract logic into focused hooks/components: move state and storage logic (storageScope, projectsExpanded, chatsExpanded, expandedProjectIds, expandedTaskListIds and related useEffect writers/readers, plus getDesktopSidebarStorageKey/getDesktopSidebarStorageScope) into a new useDesktopSidebarState hook; move resize/window-controls logic using useResizableSidebar and reserveMacWindowControls into a small TopBar wrapper component; extract project rendering into a ProjectList component that receives projects, visibleExpandedProjectIds, visibleExpandedTaskListIds, handleToggleProject, handleToggleProjectTaskLimit and callbacks (onStartNewProjectChat, onArchiveProjectChats, onRemoveProject, setRenamingProject, onOpenTask, onArchiveTask, onRenameTask); extract chats rendering into ChatsList that uses sortedRecentTasks, canStartStandaloneChat, onStartStandaloneChat and RecentTaskRow; extract settings menu area (settingsMenuRef, settingsMenuOpen, DesktopSettingsMenu) and dialogs (DesktopSearchDialog, ProjectCreateDialog, TextInputDialog instances) into their own components or into the useDesktopSidebarState hook as needed; update DesktopSidebar to compose these new hooks/components and forward props (e.g., onRefreshDevices, onCreateProject, onRenameTask) so DesktopSidebar implementation becomes a thin renderer delegating to useDesktopSidebarState, ProjectList, ChatsList, TopBar, and SettingsMenu.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 `@wework/src/components/chat/ChatInput.test.tsx`:
- Around line 1814-1817: The test is selecting a branch by index which is flaky;
instead query and click the specific branch option by its visible text or
deterministic identifier. Replace the index-based click on the elements returned
from screen.findAllByTestId('project-branch-option') with a lookup like
screen.getByText('feature/chat') or a test id that encodes the branch name, then
call userEvent.click on that element and assert onCheckoutBranch was
calledWith('feature/chat'); update references in the test to use the new
selector instead of options[0].
---
Nitpick comments:
In `@wework/src/components/chat/ChatInput.test.tsx`:
- Around line 1073-1121: The test misses the edge case where the current
selectedModel is a Gemini model that should be hidden; add a new test in
ChatInput.test.tsx (using the existing pattern) that renders <ChatInput ...
variant="desktop" projectChat={projectChatControls({ models: [gptModel,
geminiModel], selectedModel: geminiModel, selectedModelOptions: {} })}>, opens
the model menu via the model-selector-button, and asserts that
model-family-gemini and model-option-overseas-gemini-3-pro are not rendered
while visible families (e.g., model-family-gpt) remain present and the menu
stays usable (no errors). Ensure you reuse the existing gptModel and geminiModel
fixtures and assertions style to keep tests consistent.
In `@wework/src/components/layout/DesktopSidebar.tsx`:
- Around line 762-1286: DesktopSidebar is too large — extract logic into focused
hooks/components: move state and storage logic (storageScope, projectsExpanded,
chatsExpanded, expandedProjectIds, expandedTaskListIds and related useEffect
writers/readers, plus getDesktopSidebarStorageKey/getDesktopSidebarStorageScope)
into a new useDesktopSidebarState hook; move resize/window-controls logic using
useResizableSidebar and reserveMacWindowControls into a small TopBar wrapper
component; extract project rendering into a ProjectList component that receives
projects, visibleExpandedProjectIds, visibleExpandedTaskListIds,
handleToggleProject, handleToggleProjectTaskLimit and callbacks
(onStartNewProjectChat, onArchiveProjectChats, onRemoveProject,
setRenamingProject, onOpenTask, onArchiveTask, onRenameTask); extract chats
rendering into ChatsList that uses sortedRecentTasks, canStartStandaloneChat,
onStartStandaloneChat and RecentTaskRow; extract settings menu area
(settingsMenuRef, settingsMenuOpen, DesktopSettingsMenu) and dialogs
(DesktopSearchDialog, ProjectCreateDialog, TextInputDialog instances) into their
own components or into the useDesktopSidebarState hook as needed; update
DesktopSidebar to compose these new hooks/components and forward props (e.g.,
onRefreshDevices, onCreateProject, onRenameTask) so DesktopSidebar
implementation becomes a thin renderer delegating to useDesktopSidebarState,
ProjectList, ChatsList, TopBar, and SettingsMenu.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b499a70c-1ff4-4357-9351-70fc91f3c25c
📒 Files selected for processing (9)
wework/src/components/chat/ChatInput.test.tsxwework/src/components/layout/DesktopSidebar.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/components/projects/ProjectCreateDialog.test.tsxwework/src/components/projects/ProjectCreateDialog.tsxwework/src/components/settings/ConnectionsSettingsPage.tsxwework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.json
✅ Files skipped from review due to trivial changes (2)
- wework/src/i18n/locales/en/common.json
- wework/src/i18n/locales/zh-CN/common.json
🚧 Files skipped from review as they are similar to previous changes (4)
- wework/src/components/projects/ProjectCreateDialog.test.tsx
- wework/src/components/settings/ConnectionsSettingsPage.tsx
- wework/src/components/layout/DesktopWorkbenchLayout.tsx
- wework/src/components/projects/ProjectCreateDialog.tsx
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🧹 Nitpick comments (2)
wework/src/components/chat/ChatInput.test.tsx (1)
1073-1121: ⚡ Quick winAdd the hidden-selected-model edge case for Gemini filtering.
This test validates mixed-model visibility, but it doesn’t cover when
selectedModelis Gemini (hidden). Add a case asserting the menu still excludes Gemini and remains usable with visible families.As per coding guidelines, test files should cover edge cases and error conditions.
🤖 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 `@wework/src/components/chat/ChatInput.test.tsx` around lines 1073 - 1121, The test misses the edge case where the current selectedModel is a Gemini model that should be hidden; add a new test in ChatInput.test.tsx (using the existing pattern) that renders <ChatInput ... variant="desktop" projectChat={projectChatControls({ models: [gptModel, geminiModel], selectedModel: geminiModel, selectedModelOptions: {} })}>, opens the model menu via the model-selector-button, and asserts that model-family-gemini and model-option-overseas-gemini-3-pro are not rendered while visible families (e.g., model-family-gpt) remain present and the menu stays usable (no errors). Ensure you reuse the existing gptModel and geminiModel fixtures and assertions style to keep tests consistent.Source: Coding guidelines
wework/src/components/layout/DesktopSidebar.tsx (1)
762-1286: ⚖️ Poor tradeoffRefactor the DesktopSidebar function to stay within the preferred size limit.
The
DesktopSidebarfunction spans 524 lines (lines 762-1286), far exceeding the preferred 50-line limit specified in the coding guidelines. Consider extracting logical sections into separate components or hooks (e.g., sidebar state management, device status logic, project list rendering).🤖 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 `@wework/src/components/layout/DesktopSidebar.tsx` around lines 762 - 1286, DesktopSidebar is too large — extract logic into focused hooks/components: move state and storage logic (storageScope, projectsExpanded, chatsExpanded, expandedProjectIds, expandedTaskListIds and related useEffect writers/readers, plus getDesktopSidebarStorageKey/getDesktopSidebarStorageScope) into a new useDesktopSidebarState hook; move resize/window-controls logic using useResizableSidebar and reserveMacWindowControls into a small TopBar wrapper component; extract project rendering into a ProjectList component that receives projects, visibleExpandedProjectIds, visibleExpandedTaskListIds, handleToggleProject, handleToggleProjectTaskLimit and callbacks (onStartNewProjectChat, onArchiveProjectChats, onRemoveProject, setRenamingProject, onOpenTask, onArchiveTask, onRenameTask); extract chats rendering into ChatsList that uses sortedRecentTasks, canStartStandaloneChat, onStartStandaloneChat and RecentTaskRow; extract settings menu area (settingsMenuRef, settingsMenuOpen, DesktopSettingsMenu) and dialogs (DesktopSearchDialog, ProjectCreateDialog, TextInputDialog instances) into their own components or into the useDesktopSidebarState hook as needed; update DesktopSidebar to compose these new hooks/components and forward props (e.g., onRefreshDevices, onCreateProject, onRenameTask) so DesktopSidebar implementation becomes a thin renderer delegating to useDesktopSidebarState, ProjectList, ChatsList, TopBar, and SettingsMenu.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 `@wework/src/components/chat/ChatInput.test.tsx`:
- Around line 1814-1817: The test is selecting a branch by index which is flaky;
instead query and click the specific branch option by its visible text or
deterministic identifier. Replace the index-based click on the elements returned
from screen.findAllByTestId('project-branch-option') with a lookup like
screen.getByText('feature/chat') or a test id that encodes the branch name, then
call userEvent.click on that element and assert onCheckoutBranch was
calledWith('feature/chat'); update references in the test to use the new
selector instead of options[0].
---
Nitpick comments:
In `@wework/src/components/chat/ChatInput.test.tsx`:
- Around line 1073-1121: The test misses the edge case where the current
selectedModel is a Gemini model that should be hidden; add a new test in
ChatInput.test.tsx (using the existing pattern) that renders <ChatInput ...
variant="desktop" projectChat={projectChatControls({ models: [gptModel,
geminiModel], selectedModel: geminiModel, selectedModelOptions: {} })}>, opens
the model menu via the model-selector-button, and asserts that
model-family-gemini and model-option-overseas-gemini-3-pro are not rendered
while visible families (e.g., model-family-gpt) remain present and the menu
stays usable (no errors). Ensure you reuse the existing gptModel and geminiModel
fixtures and assertions style to keep tests consistent.
In `@wework/src/components/layout/DesktopSidebar.tsx`:
- Around line 762-1286: DesktopSidebar is too large — extract logic into focused
hooks/components: move state and storage logic (storageScope, projectsExpanded,
chatsExpanded, expandedProjectIds, expandedTaskListIds and related useEffect
writers/readers, plus getDesktopSidebarStorageKey/getDesktopSidebarStorageScope)
into a new useDesktopSidebarState hook; move resize/window-controls logic using
useResizableSidebar and reserveMacWindowControls into a small TopBar wrapper
component; extract project rendering into a ProjectList component that receives
projects, visibleExpandedProjectIds, visibleExpandedTaskListIds,
handleToggleProject, handleToggleProjectTaskLimit and callbacks
(onStartNewProjectChat, onArchiveProjectChats, onRemoveProject,
setRenamingProject, onOpenTask, onArchiveTask, onRenameTask); extract chats
rendering into ChatsList that uses sortedRecentTasks, canStartStandaloneChat,
onStartStandaloneChat and RecentTaskRow; extract settings menu area
(settingsMenuRef, settingsMenuOpen, DesktopSettingsMenu) and dialogs
(DesktopSearchDialog, ProjectCreateDialog, TextInputDialog instances) into their
own components or into the useDesktopSidebarState hook as needed; update
DesktopSidebar to compose these new hooks/components and forward props (e.g.,
onRefreshDevices, onCreateProject, onRenameTask) so DesktopSidebar
implementation becomes a thin renderer delegating to useDesktopSidebarState,
ProjectList, ChatsList, TopBar, and SettingsMenu.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b499a70c-1ff4-4357-9351-70fc91f3c25c
📒 Files selected for processing (9)
wework/src/components/chat/ChatInput.test.tsxwework/src/components/layout/DesktopSidebar.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/components/projects/ProjectCreateDialog.test.tsxwework/src/components/projects/ProjectCreateDialog.tsxwework/src/components/settings/ConnectionsSettingsPage.tsxwework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.json
✅ Files skipped from review due to trivial changes (2)
- wework/src/i18n/locales/en/common.json
- wework/src/i18n/locales/zh-CN/common.json
🚧 Files skipped from review as they are similar to previous changes (4)
- wework/src/components/projects/ProjectCreateDialog.test.tsx
- wework/src/components/settings/ConnectionsSettingsPage.tsx
- wework/src/components/layout/DesktopWorkbenchLayout.tsx
- wework/src/components/projects/ProjectCreateDialog.tsx
🛑 Comments failed to post (1)
wework/src/components/chat/ChatInput.test.tsx (1)
1814-1817:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid index-based branch selection to prevent flaky assertions.
At Line 1815, clicking
options[0]couples the test to branch ordering. If ordering changes, this can fail while behavior is still correct. Select the intended branch option by visible text (or deterministic test id) before asserting checkout.🤖 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 `@wework/src/components/chat/ChatInput.test.tsx` around lines 1814 - 1817, The test is selecting a branch by index which is flaky; instead query and click the specific branch option by its visible text or deterministic identifier. Replace the index-based click on the elements returned from screen.findAllByTestId('project-branch-option') with a lookup like screen.getByText('feature/chat') or a test id that encodes the branch name, then call userEvent.click on that element and assert onCheckoutBranch was calledWith('feature/chat'); update references in the test to use the new selector instead of options[0].
Summary by CodeRabbit
New Features
Bug Fixes
Documentation