From b024043feb58c97ec99c7f1817fdc6b3f20ccf83 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:07:20 -0400 Subject: [PATCH] refactor(tabs): delete unused copy-tab and merge-group store actions (#28836) * refactor(tabs): delete unused copy-tab and merge-group store actions copyUnifiedTabToGroup and mergeGroupIntoSibling had no callers outside the store and its tests. Tests that used the copy action as a setup step now create the duplicate tab directly, the same call the action made. Co-Authored-By: Claude Opus 5.5 * chore(tabs): follow the split-ratio file rename in the layout-writer baseline Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- config/renderer-layout-writer-baseline.txt | 2 +- .../ipc-events/runtime-file-position.test.ts | 6 +- ...b-session-focus-intent-visible-tab.test.ts | 2 +- .../tabs-pane-layout-operations.test.ts | 58 ----------- .../store/slices/tabs/create-tabs-slice.ts | 4 +- .../slices/tabs/tabs-secondary-actions.ts | 97 ------------------- .../store/slices/tabs/tabs-slice-contract.ts | 18 ---- .../slices/tabs/tabs-split-ratio-actions.ts | 43 ++++++++ tests/e2e/cli-file-open-position.spec.ts | 10 +- 9 files changed, 60 insertions(+), 180 deletions(-) delete mode 100644 src/renderer/src/store/slices/tabs/tabs-secondary-actions.ts create mode 100644 src/renderer/src/store/slices/tabs/tabs-split-ratio-actions.ts diff --git a/config/renderer-layout-writer-baseline.txt b/config/renderer-layout-writer-baseline.txt index 225a2c2f0ab..50775b2cb2b 100644 --- a/config/renderer-layout-writer-baseline.txt +++ b/config/renderer-layout-writer-baseline.txt @@ -95,8 +95,8 @@ 8 src/renderer/src/store/slices/tabs/tabs-label-actions.ts 10 src/renderer/src/store/slices/tabs/tabs-move-actions.ts 9 src/renderer/src/store/slices/tabs/tabs-reconciliation.ts -1 src/renderer/src/store/slices/tabs/tabs-secondary-actions.ts 4 src/renderer/src/store/slices/tabs/tabs-session-actions.ts +1 src/renderer/src/store/slices/tabs/tabs-split-ratio-actions.ts 5 src/renderer/src/store/slices/tabs/tabs-surface.ts 6 src/renderer/src/store/slices/terminal-orphan-helpers.ts 4 src/renderer/src/store/slices/terminal-tab-title-batch.ts diff --git a/src/renderer/src/hooks/ipc-events/runtime-file-position.test.ts b/src/renderer/src/hooks/ipc-events/runtime-file-position.test.ts index 2a61e0179ca..6aea9f8bfb0 100644 --- a/src/renderer/src/hooks/ipc-events/runtime-file-position.test.ts +++ b/src/renderer/src/hooks/ipc-events/runtime-file-position.test.ts @@ -173,7 +173,11 @@ it('scopes the floating reveal to its selected split pane', () => { if (!siblingGroupId) { throw new Error('Split missing') } - store.getState().copyUnifiedTabToGroup(tab.id, siblingGroupId) + store.getState().createUnifiedTab(FLOATING, tab.contentType, { + entityId: tab.entityId, + label: tab.label, + targetGroupId: siblingGroupId + }) store.getState().focusGroup(FLOATING, tab.groupId) position(fileId, { line: 300, column: 7 }, true) flushFrames() diff --git a/src/renderer/src/runtime/web-session-focus-intent-visible-tab.test.ts b/src/renderer/src/runtime/web-session-focus-intent-visible-tab.test.ts index 1434a8ef3d4..e39c43e279e 100644 --- a/src/renderer/src/runtime/web-session-focus-intent-visible-tab.test.ts +++ b/src/renderer/src/runtime/web-session-focus-intent-visible-tab.test.ts @@ -74,7 +74,7 @@ describe('resolveWebSessionVisibleTabId — grouped state is authoritative', () expect(resolveWebSessionVisibleTabId(state, WT)).toBe('tab-shown') }) - // Why: copyUnifiedTabToGroup duplicates contentType + entityId into another group, so a plain + // Why: opening a file into a second split group duplicates contentType + entityId, so a plain // array scan can return the background copy and drag group focus to it. it('prefers the active group copy when one entity exists in two split groups', () => { const background = tab({ diff --git a/src/renderer/src/store/slices/tabs-pane-layout-operations.test.ts b/src/renderer/src/store/slices/tabs-pane-layout-operations.test.ts index 0a05d9780b3..8ef18713edc 100644 --- a/src/renderer/src/store/slices/tabs-pane-layout-operations.test.ts +++ b/src/renderer/src/store/slices/tabs-pane-layout-operations.test.ts @@ -1,6 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import type * as AgentStatusModule from '@/lib/agent-status' -import { getDefaultUIState } from '../../../../shared/constants' import { createTabsSliceMockApi } from './tabs-slice-test-harness' import { createTestStore } from './store-test-helpers' @@ -131,63 +130,6 @@ describe('TabsSlice', () => { ).toEqual([tab.id]) }) - it('copies a unified tab into another group', () => { - const tab = store.getState().createUnifiedTab(WT, 'editor', { - id: 'file-a.ts', - executionHostId: 'runtime:host-b', - label: 'file-a.ts' - }) - const sourceGroupId = store.getState().groupsByWorktree[WT][0].id - const targetGroupId = store.getState().createEmptySplitGroup(WT, sourceGroupId, 'right') - expect(targetGroupId).toBeTruthy() - - const copied = store.getState().copyUnifiedTabToGroup(tab.id, targetGroupId!) - - expect(copied).not.toBeNull() - const state = store.getState() - expect(state.unifiedTabsByWorktree[WT]).toHaveLength(2) - expect( - state.groupsByWorktree[WT].find((group) => group.id === sourceGroupId)?.tabOrder - ).toEqual([tab.id]) - expect( - state.groupsByWorktree[WT].find((group) => group.id === targetGroupId)?.tabOrder - ).toEqual([copied!.id]) - expect(copied?.entityId).toBe(tab.entityId) - expect(copied?.executionHostId).toBe('runtime:host-b') - }) - - it('merges a group into its sibling', () => { - const setMock = vi.mocked(window.api.ui.set) - const t1 = store.getState().createUnifiedTab(WT, 'editor', { - id: 'file-a.ts', - label: 'file-a.ts' - }) - const sourceGroupId = store.getState().groupsByWorktree[WT][0].id - const targetGroupId = store.getState().createEmptySplitGroup(WT, sourceGroupId, 'right') - expect(targetGroupId).toBeTruthy() - store.getState().createUnifiedTab(WT, 'editor', { - id: 'file-b.ts', - label: 'file-b.ts', - targetGroupId: targetGroupId! - }) - store.getState().hydratePersistedUI(getDefaultUIState()) - setMock.mockClear() - - const mergedInto = store.getState().mergeGroupIntoSibling(WT, targetGroupId!) - - expect(mergedInto).toBe(sourceGroupId) - const state = store.getState() - expect(state.groupsByWorktree[WT]).toHaveLength(1) - expect(state.groupsByWorktree[WT][0].tabOrder).toEqual([t1.id, 'file-b.ts']) - expect(state.layoutByWorktree[WT]).toEqual({ type: 'leaf', groupId: sourceGroupId }) - expect(setMock).toHaveBeenCalledTimes(1) - expect(setMock).toHaveBeenCalledWith({ - featureInteractions: { - 'terminal-panes': expect.objectContaining({ interactionCount: 1 }) - } - }) - }) - it('drops a unified tab into another group and collapses an emptied source group', () => { const tab = store.getState().createUnifiedTab(WT, 'editor', { id: 'file-a.ts', diff --git a/src/renderer/src/store/slices/tabs/create-tabs-slice.ts b/src/renderer/src/store/slices/tabs/create-tabs-slice.ts index adbea2f1726..52bf7cff18d 100644 --- a/src/renderer/src/store/slices/tabs/create-tabs-slice.ts +++ b/src/renderer/src/store/slices/tabs/create-tabs-slice.ts @@ -9,7 +9,7 @@ import { createTabsLabelActions } from './tabs-label-actions' import { createTabsGroupActions } from './tabs-group-actions' import { createTabsMoveActions } from './tabs-move-actions' import { createTabsDropActions } from './tabs-drop-actions' -import { createTabsSecondaryActions } from './tabs-secondary-actions' +import { createTabsSplitRatioActions } from './tabs-split-ratio-actions' import { createTabsSessionActions } from './tabs-session-actions' export const createTabsSlice: StateCreator = (set, get) => ({ @@ -25,6 +25,6 @@ export const createTabsSlice: StateCreator = (set, ...createTabsGroupActions(set, get), ...createTabsMoveActions(set, get), ...createTabsDropActions(set, get), - ...createTabsSecondaryActions(set, get), + ...createTabsSplitRatioActions(set), ...createTabsSessionActions(set, get) }) diff --git a/src/renderer/src/store/slices/tabs/tabs-secondary-actions.ts b/src/renderer/src/store/slices/tabs/tabs-secondary-actions.ts deleted file mode 100644 index 3db74535500..00000000000 --- a/src/renderer/src/store/slices/tabs/tabs-secondary-actions.ts +++ /dev/null @@ -1,97 +0,0 @@ -import type { TabGroupLayoutNode } from '../../../../../shared/tab-types' -import type { TabsSlice, TabsSliceGet, TabsSliceSet } from './tabs-slice-contract' -import { findGroupAndWorktree, findTabAndWorktree } from '../tab-group-state' -import { - findSiblingGroupId, - updateSplitRatio -} from '../../../../../shared/workspace-layout/tab-group-layout-tree' - -export function createTabsSecondaryActions( - set: TabsSliceSet, - get: TabsSliceGet -): Pick { - return { - copyUnifiedTabToGroup: (tabId, targetGroupId, init) => { - const foundTab = findTabAndWorktree(get().unifiedTabsByWorktree, tabId) - const foundTarget = findGroupAndWorktree(get().groupsByWorktree, targetGroupId) - if (!foundTab || !foundTarget || foundTab.worktreeId !== foundTarget.worktreeId) { - return null - } - const { tab, worktreeId } = foundTab - return get().createUnifiedTab(worktreeId, tab.contentType, { - entityId: init?.entityId ?? tab.entityId, - executionHostId: tab.executionHostId, - label: init?.label ?? tab.label, - generatedLabel: init?.generatedLabel ?? tab.generatedLabel, - quickCommandLabel: init?.quickCommandLabel ?? tab.quickCommandLabel, - customLabel: init?.customLabel ?? tab.customLabel, - color: init?.color ?? tab.color, - isPinned: init?.isPinned ?? tab.isPinned, - id: init?.id, - targetGroupId - }) - }, - - mergeGroupIntoSibling: (worktreeId, groupId) => { - const state = get() - const groups = state.groupsByWorktree[worktreeId] ?? [] - const sourceGroup = groups.find((candidate) => candidate.id === groupId) - const layout = state.layoutByWorktree[worktreeId] - if (!sourceGroup || !layout || groups.length <= 1) { - return null - } - const targetGroupId = findSiblingGroupId(layout, groupId) - if (!targetGroupId) { - return null - } - - const orderedSourceTabs = (state.unifiedTabsByWorktree[worktreeId] ?? []).filter( - (tab) => tab.groupId === groupId - ) - for (const tabId of sourceGroup.tabOrder) { - const item = orderedSourceTabs.find((tab) => tab.id === tabId) - if (!item) { - continue - } - get().moveUnifiedTabToGroup(item.id, targetGroupId, { recordInteraction: false }) - } - get().closeEmptyGroup(worktreeId, groupId) - get().recordFeatureInteraction?.('terminal-panes') - return targetGroupId - }, - - setTabGroupSplitRatio: (worktreeId, nodePath, ratio) => { - set((state) => { - const currentLayout = state.layoutByWorktree[worktreeId] - if (!currentLayout) { - return state - } - // Why: an unchanged ratio must not mint fresh root state — every store - // subscriber wakes on the new reference (STA-3328). - let targetNode: TabGroupLayoutNode | undefined = currentLayout - for (const segment of nodePath.length > 0 ? nodePath.split('.') : []) { - targetNode = - targetNode && - targetNode.type === 'split' && - (segment === 'first' || segment === 'second') - ? targetNode[segment] - : undefined - } - if (!targetNode || targetNode.type !== 'split' || (targetNode.ratio ?? 0.5) === ratio) { - return state - } - return { - layoutByWorktree: { - ...state.layoutByWorktree, - // Why: split ratios belong to the tab-group model (not transient UI), so persist them for restores and multi-step group ops. - [worktreeId]: updateSplitRatio( - currentLayout, - nodePath.length > 0 ? nodePath.split('.') : [], - ratio - ) - } - } - }) - } - } -} diff --git a/src/renderer/src/store/slices/tabs/tabs-slice-contract.ts b/src/renderer/src/store/slices/tabs/tabs-slice-contract.ts index 63dfab374cd..444fe9a5882 100644 --- a/src/renderer/src/store/slices/tabs/tabs-slice-contract.ts +++ b/src/renderer/src/store/slices/tabs/tabs-slice-contract.ts @@ -136,24 +136,6 @@ export type TabsSlice = { splitDirection?: TabSplitDirection } ) => boolean - copyUnifiedTabToGroup: ( - tabId: string, - targetGroupId: string, - init?: Partial< - Pick< - Tab, - | 'id' - | 'entityId' - | 'label' - | 'generatedLabel' - | 'quickCommandLabel' - | 'customLabel' - | 'color' - | 'isPinned' - > - > - ) => Tab | null - mergeGroupIntoSibling: (worktreeId: string, groupId: string) => string | null setTabGroupSplitRatio: (worktreeId: string, nodePath: string, ratio: number) => void reconcileWorktreeTabModel: (worktreeId: string) => { renderableTabCount: number diff --git a/src/renderer/src/store/slices/tabs/tabs-split-ratio-actions.ts b/src/renderer/src/store/slices/tabs/tabs-split-ratio-actions.ts new file mode 100644 index 00000000000..191d105dc61 --- /dev/null +++ b/src/renderer/src/store/slices/tabs/tabs-split-ratio-actions.ts @@ -0,0 +1,43 @@ +import type { TabGroupLayoutNode } from '../../../../../shared/tab-types' +import type { TabsSlice, TabsSliceSet } from './tabs-slice-contract' +import { updateSplitRatio } from '../../../../../shared/workspace-layout/tab-group-layout-tree' + +export function createTabsSplitRatioActions( + set: TabsSliceSet +): Pick { + return { + setTabGroupSplitRatio: (worktreeId, nodePath, ratio) => { + set((state) => { + const currentLayout = state.layoutByWorktree[worktreeId] + if (!currentLayout) { + return state + } + // Why: an unchanged ratio must not mint fresh root state — every store + // subscriber wakes on the new reference (STA-3328). + let targetNode: TabGroupLayoutNode | undefined = currentLayout + for (const segment of nodePath.length > 0 ? nodePath.split('.') : []) { + targetNode = + targetNode && + targetNode.type === 'split' && + (segment === 'first' || segment === 'second') + ? targetNode[segment] + : undefined + } + if (!targetNode || targetNode.type !== 'split' || (targetNode.ratio ?? 0.5) === ratio) { + return state + } + return { + layoutByWorktree: { + ...state.layoutByWorktree, + // Why: split ratios belong to the tab-group model (not transient UI), so persist them for restores and multi-step group ops. + [worktreeId]: updateSplitRatio( + currentLayout, + nodePath.length > 0 ? nodePath.split('.') : [], + ratio + ) + } + } + }) + } + } +} diff --git a/tests/e2e/cli-file-open-position.spec.ts b/tests/e2e/cli-file-open-position.spec.ts index 471ea8300e7..df5ede359e6 100644 --- a/tests/e2e/cli-file-open-position.spec.ts +++ b/tests/e2e/cli-file-open-position.spec.ts @@ -133,9 +133,15 @@ test('CLI file positioning preserves background navigation and reveals the reque await orcaPage.evaluate( ({ id, split }) => { const state = window.__store?.getState() - if (!state?.copyUnifiedTabToGroup(split.tabId, split.siblingGroupId)) { - throw new Error('Editor tab was not copied') + const tab = state?.unifiedTabsByWorktree[id]?.find((item) => item.id === split.tabId) + if (!state || !tab) { + throw new Error('Editor tab missing') } + state.createUnifiedTab(id, tab.contentType, { + entityId: tab.entityId, + label: tab.label, + targetGroupId: split.siblingGroupId + }) state.focusGroup(id, split.groupId) }, { id: worktreeId, split }