Repository navigation
feat(wework): redesign split task workbench - #2611
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds split-pane workbench state, rendering, drag-and-drop, persistence, pane-specific actions, visibility-aware sessions, updated desktop automation, and end-to-end coverage. ChangesSplit workbench
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Sidebar
participant PaneStack
participant WorkbenchLayout
participant WorkbenchPane
Sidebar->>PaneStack: Dispatch sidebar pane drag data
PaneStack->>WorkbenchLayout: Split or move pane
WorkbenchLayout-->>PaneStack: Persist layout state
PaneStack->>WorkbenchPane: Render focused and visible pane
WorkbenchPane-->>PaneStack: Render pane-specific header actions
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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.
Actionable comments posted: 5
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/layout/DesktopWorkbenchMain.tsx (1)
3936-3952: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winApply
paneVisibleto the panel root visibility.When
openistrueandpaneVisibleisfalse, the root remainsaria-hidden="false", usespointer-events-auto, and keeps its height. Apply the hidden, non-interactive state at theBottomWorkspacePanelroot while preserving cached content.🤖 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/DesktopWorkbenchMain.tsx` around lines 3936 - 3952, Update BottomWorkspacePanel’s root visibility handling so paneVisible=false applies the hidden, non-interactive state even when open=true: set the root’s aria-hidden, pointer-events, and height behavior from paneVisible while preserving the mounted/cached content. Ensure the caller’s paneVisible prop from DesktopWorkbenchMain continues to drive this behavior.
🧹 Nitpick comments (9)
wework/src/e2e/automation.ts (1)
365-383: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAlign interaction targeting with snapshot filtering.
Inactive split panes remain rendered and can contain duplicate controls. Filter them from
findRenderedDesktopControlElementsso interaction commands target the same surface thatdesktopControlSnapshotexposes.🤖 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/e2e/automation.ts` around lines 365 - 383, Update findRenderedDesktopControlElements to exclude elements where desktopControlElementInInactiveSurface returns true, so interaction targeting matches the filtering used by desktopControlSnapshot. Preserve the existing selector lookup and rendered-element filtering while removing controls from inactive split panes.wework/src/components/layout/workbenchSplitLayout.test.ts (1)
95-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a size assertion when a group closes from a three-way split.
This test asserts only the child count after
closeWorkbenchGroup. It does not assert the resultingsizes. A size-index defect inremoveGrouptherefore passes unnoticed. Assert the normalizedsizesarray here.💚 Proposed assertion
const closed = closeWorkbenchGroup(third, third.focusedGroupId) expect(closed.root.type === 'split' ? closed.root.children : []).toHaveLength(2) + expect(closed.root.type === 'split' ? closed.root.sizes : []).toEqual([50, 50])🤖 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/workbenchSplitLayout.test.ts` around lines 95 - 106, Add an assertion in the “flattens same-orientation splits and closes groups cleanly” test after closeWorkbenchGroup verifies the resulting normalized sizes array, alongside the existing child-count assertion, so removeGroup size-index defects are detected.wework/src/components/layout/workbenchPaneStack.tsx (4)
445-527: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated
labelsobject.The same 17-key
labelsobject is written twice, at Lines 450-471 and Lines 505-526. The two copies must stay in sync manually. The object is also rebuilt on every render.Build it once with
useMemokeyed ont, then pass the same value to both consumers.As per coding guidelines: "Before adding code, search for and reuse existing components, services, utilities, and patterns; extract shared logic instead of duplicating it."
♻️ Proposed change
+ const labels = useMemo<WorkbenchLayoutLabels>( + () => ({ + paneMenu: t('workbench.split_pane_menu', '分屏操作'), + splitLeft: t('workbench.split_left', '向左分屏'), + // ...remaining keys + }), + [t] + )Then replace both inline
labels={{ ... }}props withlabels={labels}.🤖 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/workbenchPaneStack.tsx` around lines 445 - 527, Extract the duplicated 17-key labels object into a useMemo value keyed by t within the component containing WorkbenchSplitActionsPortal and WorkbenchLayoutRenderer. Replace both inline labels props with the shared labels value, preserving all existing translation keys and fallback text.Source: Coding guidelines
91-103: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the oversized components.
SplitWorkbenchPaneStackspans roughly 440 lines andWorkbenchPaneGroupspans roughly 218 lines.WorkbenchLayoutRendereralso carries 20 props. This exceeds the project limit and makes the drag, focus, persistence, and rendering concerns hard to change independently.Extract the sidebar drag listeners and the layout mutation callbacks into dedicated hooks, for example
useWorkbenchSidebarPaneDropanduseWorkbenchLayoutActions. Group the renderer props into a single context or options object.As per coding guidelines: "functions should remain focused, preferably under 50 lines."
Also applies to: 784-825
🤖 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/workbenchPaneStack.tsx` around lines 91 - 103, Refactor SplitWorkbenchPaneStack and WorkbenchPaneGroup into focused units under the project’s function-size guideline by extracting sidebar drag listeners into a useWorkbenchSidebarPaneDrop hook and layout mutation callbacks into a useWorkbenchLayoutActions hook. Consolidate WorkbenchLayoutRenderer’s large prop list into a single context or options object, while preserving existing drag, focus, persistence, and rendering behavior.Source: Coding guidelines
336-386: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMove
onPaneFocusout of thesetLayoutupdaters.
closePane,closeGroup, andfocusGroupcallqueueMicrotask(() => onPaneFocus(...))inside the state updater. React treats updaters as pure functions and may invoke them more than once, including in StrictMode development.onPaneFocustriggersopenRuntimeTask, so a double invocation issues a duplicate navigation.Compute the next state first, then schedule the callback outside the updater.
♻️ Proposed shape for `closeGroup`
const closeGroup = useCallback( (groupId: string) => { if (focusedViewGroupId === groupId) setFocusedViewGroupId(null) + const next = closeWorkbenchGroup(layout, groupId) + const nextPane = next.focusedPaneKey ? resolveCachedPane(next.focusedPaneKey) : null + setLayout(next) + if (nextPane) queueMicrotask(() => onPaneFocus(nextPane)) - setLayout(current => { - const next = closeWorkbenchGroup(current, groupId) - const nextPaneKey = next.focusedPaneKey - const nextPane = nextPaneKey ? resolveCachedPane(nextPaneKey) : null - if (nextPane) queueMicrotask(() => onPaneFocus(nextPane)) - return next - }) }, - [focusedViewGroupId, onPaneFocus, resolveCachedPane, setLayout] + [focusedViewGroupId, layout, onPaneFocus, resolveCachedPane] )🤖 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/workbenchPaneStack.tsx` around lines 336 - 386, Move the onPaneFocus scheduling out of the setLayout updater in closePane, closeGroup, and focusGroup. Compute and retain the resulting state and focused pane from the updater, then queue the callback once after setLayout completes, preserving the existing pane-selection behavior without triggering side effects from the pure updater.
159-172: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReturn the existing layout when pruning makes no changes.
DesktopWorkbenchMainmemoizesruntimePaneKeys, sovalidRuntimeKeysis not recreated on every render and this effect does not loop from prop identity.pruneWorkbenchLayoutstill allocates new state and node objects for a no-op prune, causing an unnecessary render andlocalStoragewrite when the runtime-key list changes.🤖 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/workbenchPaneStack.tsx` around lines 159 - 172, The runtime-key pruning effect in DesktopWorkbenchMain should preserve the existing layout when pruneWorkbenchLayout makes no changes. Update pruneWorkbenchLayout to return the original layout object and node references for a no-op prune, so the setLayout call avoids unnecessary renders and localStorage writes while still returning the fallback layout when pruning yields no valid layout.wework/src/components/layout/SidebarSortableList.tsx (1)
105-139: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConfirm the synchronous
handledmutation contract.
handleDragEndreadsdragEndData.handledimmediately afterdispatchWorkbenchSidebarPaneDragEnd. This only works becausewindow.dispatchEventruns listeners synchronously andSplitWorkbenchPaneStackmutates the samedetailobject. Any future listener that defers its decision (for example insidequeueMicrotaskorsetTimeout) will silently fall through to internal reordering and produce a duplicate move.Add a short comment that documents this requirement so the contract survives refactoring.
♻️ Proposed comment
+ // Listeners run synchronously, so a split drop target can claim this drag by + // setting `handled` on the shared detail object before dispatch returns. dispatchWorkbenchSidebarPaneDragEnd(dragEndData) if (dragEndData.handled) return🤖 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/SidebarSortableList.tsx` around lines 105 - 139, Document the synchronous handled-mutation contract at the dispatchWorkbenchSidebarPaneDragEnd call in handleDragEnd: explain that listeners must mutate the shared dragEndData.handled value during synchronous window.dispatchEvent processing, because the immediate check determines whether internal reordering proceeds. Keep the comment short and do not change behavior.wework/src/components/layout/workbenchPaneStack.module.css (1)
5-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDo not drive production styling from
data-testidvalues.These four rules select elements by their test identifiers. Test ids are non-semantic hooks. A test refactor that renames
open-code-server-titlebar-buttonoropen-local-workspace-picker-buttonsilently breaks the compact split header without failing any test.Add a dedicated attribute for this purpose, for example
data-pane-header-control="workspace-picker", and select on that instead.🤖 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/workbenchPaneStack.module.css` around lines 5 - 22, Replace the data-testid selectors in the compact-header rules within the stylesheet with a dedicated semantic pane-header control attribute, such as data-pane-header-control, using distinct values for the workspace titlebar, code-server button, workspace location label, and local workspace picker. Update the corresponding component markup to add these attributes while preserving the existing responsive styling behavior.wework/src/components/layout/workbenchPaneStack.test.tsx (1)
288-326: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestore the
document.elementsFromPointspy after this test.
vi.spyOn(document, 'elementsFromPoint')at Line 300 is never restored. The stub persists for every later test in this file and returns a detached element from a finished test. The stub is currently inert becausehandleSidebarDragMovecheckssidebarDragActiveReffirst, but a future change to the drag path makes the later tests depend on stale state.Capture the spy and restore it, or restore it in an
afterEach.♻️ Proposed change
const target = await screen.findByTestId(/^drop:.+:right$/) - vi.spyOn(document, 'elementsFromPoint').mockReturnValue([target]) + const elementsFromPoint = vi.spyOn(document, 'elementsFromPoint').mockReturnValue([target]) + onTestFinished(() => elementsFromPoint.mockRestore())Import
onTestFinishedfromvitest, or add anafterEach(() => vi.restoreAllMocks())and move thebeforeAllstubs intobeforeEach.🤖 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/workbenchPaneStack.test.tsx` around lines 288 - 326, Restore the document.elementsFromPoint spy created in the “accepts a task dragged from the sidebar onto a split edge” test after that test completes, using a captured spy restore or the file’s test cleanup mechanism. Ensure later tests do not retain the detached target element while preserving the existing drag behavior.
🤖 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/e2e/desktop/scenarios/split-workbench.scenario.mjs`:
- Around line 361-368: Remove the localized text constraint from the right
drop-target wait in wework/e2e/desktop/scenarios/split-workbench.scenario.mjs
lines 361-368, relying on the right-position and data-over selectors. Also
remove the localized aria-label constraint at lines 506-512 and rely on
aria-pressed="true" instead.
In `@wework/src/components/layout/DesktopSidebar.tsx`:
- Around line 2590-2593: Replace the duplicated runtime pane-key template
literal with getWorkbenchPaneKey in all three DesktopSidebar.tsx sites: lines
2590-2593, 3719-3722, and 4120-4123. Pass currentRuntimeTask with
workspace.deviceId and task.taskId, and currentProject: null, preserving each
callback’s existing title handling.
In `@wework/src/components/layout/workbenchPaneStack.tsx`:
- Around line 387-404: Update handleDragEnd to notify the parent via onPaneFocus
after moving the pane, matching the sidebar drop handler’s behavior. Use the
dragged pane key or resulting pane identity as required by the existing
callback, and preserve the current layout update and focus-group handling.
In `@wework/src/components/layout/workbenchSplitLayout.ts`:
- Around line 413-425: Update removeGroup to preserve each child’s original size
before filtering removes the target group, matching the approach used by
pruneNode and removeEmptyGroups. Build the surviving children together with
their corresponding node.sizes entries, then normalize those captured sizes
after recursive removal so later siblings retain their original pane sizes.
In `@wework/src/e2e/automation.ts`:
- Around line 1422-1426: Update the select-control handling around
findRenderedDesktopControlElement to first detect an undefined result and throw
a missing-element error for command.selector; only perform the HTMLSelectElement
type check after an element is found, preserving the existing wrong-type error
for non-select elements.
---
Outside diff comments:
In `@wework/src/components/layout/DesktopWorkbenchMain.tsx`:
- Around line 3936-3952: Update BottomWorkspacePanel’s root visibility handling
so paneVisible=false applies the hidden, non-interactive state even when
open=true: set the root’s aria-hidden, pointer-events, and height behavior from
paneVisible while preserving the mounted/cached content. Ensure the caller’s
paneVisible prop from DesktopWorkbenchMain continues to drive this behavior.
---
Nitpick comments:
In `@wework/src/components/layout/SidebarSortableList.tsx`:
- Around line 105-139: Document the synchronous handled-mutation contract at the
dispatchWorkbenchSidebarPaneDragEnd call in handleDragEnd: explain that
listeners must mutate the shared dragEndData.handled value during synchronous
window.dispatchEvent processing, because the immediate check determines whether
internal reordering proceeds. Keep the comment short and do not change behavior.
In `@wework/src/components/layout/workbenchPaneStack.module.css`:
- Around line 5-22: Replace the data-testid selectors in the compact-header
rules within the stylesheet with a dedicated semantic pane-header control
attribute, such as data-pane-header-control, using distinct values for the
workspace titlebar, code-server button, workspace location label, and local
workspace picker. Update the corresponding component markup to add these
attributes while preserving the existing responsive styling behavior.
In `@wework/src/components/layout/workbenchPaneStack.test.tsx`:
- Around line 288-326: Restore the document.elementsFromPoint spy created in the
“accepts a task dragged from the sidebar onto a split edge” test after that test
completes, using a captured spy restore or the file’s test cleanup mechanism.
Ensure later tests do not retain the detached target element while preserving
the existing drag behavior.
In `@wework/src/components/layout/workbenchPaneStack.tsx`:
- Around line 445-527: Extract the duplicated 17-key labels object into a
useMemo value keyed by t within the component containing
WorkbenchSplitActionsPortal and WorkbenchLayoutRenderer. Replace both inline
labels props with the shared labels value, preserving all existing translation
keys and fallback text.
- Around line 91-103: Refactor SplitWorkbenchPaneStack and WorkbenchPaneGroup
into focused units under the project’s function-size guideline by extracting
sidebar drag listeners into a useWorkbenchSidebarPaneDrop hook and layout
mutation callbacks into a useWorkbenchLayoutActions hook. Consolidate
WorkbenchLayoutRenderer’s large prop list into a single context or options
object, while preserving existing drag, focus, persistence, and rendering
behavior.
- Around line 336-386: Move the onPaneFocus scheduling out of the setLayout
updater in closePane, closeGroup, and focusGroup. Compute and retain the
resulting state and focused pane from the updater, then queue the callback once
after setLayout completes, preserving the existing pane-selection behavior
without triggering side effects from the pure updater.
- Around line 159-172: The runtime-key pruning effect in DesktopWorkbenchMain
should preserve the existing layout when pruneWorkbenchLayout makes no changes.
Update pruneWorkbenchLayout to return the original layout object and node
references for a no-op prune, so the setLayout call avoids unnecessary renders
and localStorage writes while still returning the fallback layout when pruning
yields no valid layout.
In `@wework/src/components/layout/workbenchSplitLayout.test.ts`:
- Around line 95-106: Add an assertion in the “flattens same-orientation splits
and closes groups cleanly” test after closeWorkbenchGroup verifies the resulting
normalized sizes array, alongside the existing child-count assertion, so
removeGroup size-index defects are detected.
In `@wework/src/e2e/automation.ts`:
- Around line 365-383: Update findRenderedDesktopControlElements to exclude
elements where desktopControlElementInInactiveSurface returns true, so
interaction targeting matches the filtering used by desktopControlSnapshot.
Preserve the existing selector lookup and rendered-element filtering while
removing controls from inactive split panes.
🪄 Autofix
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 Plus
Run ID: 93339214-493b-4a8f-86b4-0cf1594cf4d2
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
wework/e2e/desktop/checkpoints.mjswework/e2e/desktop/modules/shared.mjswework/e2e/desktop/modules/window-attachment-flows.mjswework/e2e/desktop/run-checkpoints.mjswework/e2e/desktop/scenarios/split-workbench.scenario.mjswework/package.jsonwework/src/components/layout/DesktopSidebar.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/SidebarSortableList.tsxwework/src/components/layout/workbenchPaneDrag.tswework/src/components/layout/workbenchPaneIdentity.tswework/src/components/layout/workbenchPaneStack.module.csswework/src/components/layout/workbenchPaneStack.test.tsxwework/src/components/layout/workbenchPaneStack.tsxwework/src/components/layout/workbenchSplitLayout.test.tswework/src/components/layout/workbenchSplitLayout.tswework/src/components/layout/workspace-panels/BottomWorkspacePanel.tsxwework/src/components/layout/workspace-panels/RightWorkspacePanel.tsxwework/src/components/layout/workspace-panels/WorkspacePanelActions.tsxwework/src/components/topnav/TitlebarActionsPortal.tsxwework/src/e2e/automation.tswework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/workbenchContextTypes.tswework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.json
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/e2e/automation.ts`:
- Around line 369-380: Update findActiveDesktopControlElements to return only
elements that pass desktopControlElementInInactiveSurface, removing the fallback
to the original elements when no active matches exist. Also update
desktopControlElementInInactiveSurface so a workspace-tab root matching
[data-workspace-tab-content][aria-hidden="true"] is treated as inactive, not
only its descendants.
- Around line 358-368: Update desktopControlElementInInactiveSurface and its use
in findActiveDesktopControlElements to treat elements inside inactive
data-workspace-tab-portal-owner wrappers as inactive, before applying the
existing surface checks and fallback selection. Preserve active portal controls
and existing inactive workbench behavior.
🪄 Autofix
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 Plus
Run ID: cc61e536-1a59-437d-955c-b2dceed4a065
📒 Files selected for processing (10)
wework/e2e/desktop/checkpoints.mjswework/e2e/desktop/modules/shared.mjswework/package.jsonwework/src/components/layout/DesktopSidebar.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/workspace-panels/RightWorkspacePanel.tsxwework/src/e2e/automation.tswework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.json
🚧 Files skipped from review as they are similar to previous changes (9)
- wework/e2e/desktop/checkpoints.mjs
- wework/e2e/desktop/modules/shared.mjs
- wework/src/i18n/locales/en/common.json
- wework/package.json
- wework/src/components/layout/DesktopWorkbenchLayout.test.tsx
- wework/src/components/layout/workspace-panels/RightWorkspacePanel.tsx
- wework/src/components/layout/DesktopSidebar.tsx
- wework/src/i18n/locales/zh-CN/common.json
- wework/src/components/layout/DesktopWorkbenchMain.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/composer/ModelSelector.tsx`:
- Around line 152-156: Update the useEffect that handles disabled and open state
to call closeMenu() synchronously instead of scheduling it with
window.setTimeout. Preserve the existing guard and dependency array so the menu
closes immediately when disabled becomes true, preventing its actions from
remaining available.
🪄 Autofix
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 Plus
Run ID: 5668c910-f5e0-4f21-99e3-7f806acba4a6
📒 Files selected for processing (3)
wework/src/components/chat/composer/ModelSelector.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/workbenchPaneStack.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- wework/src/components/layout/DesktopWorkbenchMain.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
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/layout/DesktopWorkbenchMain.tsx (1)
714-814: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftSplit these oversized modified files into cohesive modules.
These modified files exceed the 1000-line limit. Extract the independent responsibilities before merge.
wework/src/components/layout/DesktopWorkbenchMain.tsx#L714-L814: extract pane header, workspace-panel, and pane-session integration.wework/src/components/layout/useWorkbenchPaneSession.ts#L151-L154: extract transcript hydration and refresh coordination from message and composer operations.wework/src/features/workbench/WorkbenchProvider.test.tsx#L8116-L8167: move runtime-pane transcript tests into focused test files.wework/src/components/chat/ScrollableMessageArea.tsx#L207-L268: extract scroll snapshot and suspension coordination into a dedicated hook.As per coding guidelines: “Favor cohesive modules, explicit interfaces, and standard practices; split files over 1000 lines.”
🤖 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/DesktopWorkbenchMain.tsx` around lines 714 - 814, Split the oversized files into cohesive modules without changing behavior: in wework/src/components/layout/DesktopWorkbenchMain.tsx#L714-L814, extract the DesktopWorkbenchPane header, workspace-panel, and pane-session integration responsibilities; in wework/src/components/layout/useWorkbenchPaneSession.ts#L151-L154, extract transcript hydration and refresh coordination from message/composer operations; in wework/src/features/workbench/WorkbenchProvider.test.tsx#L8116-L8167, move runtime-pane transcript tests into focused test files; and in wework/src/components/chat/ScrollableMessageArea.tsx#L207-L268, extract scroll snapshot and suspension coordination into a dedicated hook. Preserve existing interfaces and behavior while keeping each module cohesive.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/layout/useWorkbenchPaneSession.ts`:
- Around line 642-695: Update the visible-pane refresh effect around
loadRuntimeTranscriptForPaneRef so cleanup aborts the active hydration and
releases rebuildingTranscriptRef/current state. Track each refresh with a
per-request generation token, and only allow the matching request to clear
rebuilding state so an older cancelled request cannot affect a newer refresh.
Preserve identity checks and add a regression test covering hide, show, hide,
and show before the first refresh resolves.
---
Outside diff comments:
In `@wework/src/components/layout/DesktopWorkbenchMain.tsx`:
- Around line 714-814: Split the oversized files into cohesive modules without
changing behavior: in
wework/src/components/layout/DesktopWorkbenchMain.tsx#L714-L814, extract the
DesktopWorkbenchPane header, workspace-panel, and pane-session integration
responsibilities; in
wework/src/components/layout/useWorkbenchPaneSession.ts#L151-L154, extract
transcript hydration and refresh coordination from message/composer operations;
in wework/src/features/workbench/WorkbenchProvider.test.tsx#L8116-L8167, move
runtime-pane transcript tests into focused test files; and in
wework/src/components/chat/ScrollableMessageArea.tsx#L207-L268, extract scroll
snapshot and suspension coordination into a dedicated hook. Preserve existing
interfaces and behavior while keeping each module cohesive.
🪄 Autofix
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 Plus
Run ID: 6ab5684d-ae52-43be-852b-58de3bbfa719
📒 Files selected for processing (6)
wework/src/components/chat/ScrollableMessageArea.test.tsxwework/src/components/chat/ScrollableMessageArea.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/useWorkbenchPaneSession.tswework/src/components/layout/workbenchPaneStack.tsxwework/src/features/workbench/WorkbenchProvider.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- wework/src/components/layout/workbenchPaneStack.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/MessageList.tsx`:
- Around line 375-389: Update the cleanup around messageVirtualizer and
cacheConversationVirtualMeasurements to read
virtualMeasurementPersistenceEnabled from a ref containing the latest prop
value, preventing a stale enabled closure from writing during a true-to-false
transition. Keep the existing guards and snapshot behavior, and add a regression
test covering rerendering with persistence enabled, disabling it, then
unmounting.
🪄 Autofix
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 Plus
Run ID: 49ea81e3-21a9-4546-990e-b06b714a0716
📒 Files selected for processing (4)
wework/src/components/chat/MessageList.tsxwework/src/components/chat/MessageList.virtualization.test.tsxwework/src/components/chat/ScrollableMessageArea.tsxwework/src/components/layout/useWorkbenchPaneSession.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- wework/src/components/chat/ScrollableMessageArea.tsx
- wework/src/components/layout/useWorkbenchPaneSession.ts
0221d3f to
ae4c3d5
Compare
4dce00c to
c80d8ed
Compare
c80d8ed to
f3364d5
Compare
变更概述
性能
验证
WorkbenchProvider.test.tsx:159/159DesktopWorkbenchLayout.test.tsx:184/184ai:verify:创建两个真实 Codex 任务并完成拖入分屏、独立标题栏、环境隔离、单任务聚焦、恢复与关闭回收验证核心交互结果