mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 00:03:46 +00:00
fix(cmd-j): stabilize palette memos and gate heavy builders when closed
P1-a: Move quickActionContext filtering into a useMemo with stable primitive deps (activeView, activeWorktreeId, sshConnectionStates, etc.) instead of calling buildQuickActionContext() inline every render — the fresh object identity defeated the middleItems memo on every keystroke. P1-b: Guard browserSortedWorktrees, browserPageEntries, simulatorTabEntries, and workspaceTabEntries on paletteStatusInputsActive so the always-mounted palette stops rebuilding full open-tab indexes after every store write while closed. P2-a: Derive sortedWorktrees from browserSortedWorktrees by filtering out archived worktrees — both called sortWorktreesSmart with identical deps, so one sort + filter replaces two. P2-b: Pre-index agent metadata by tabId once per build via buildAgentMetadataTabIndex, replacing the O(tabs × map entries) scan in collectAgentMetadataForTerminal with O(1) lookups per tab. P2-e: Assert in the interleaved test that rendered selectable row order matches orderMultiPrimaryPaletteItems, keeping listEntries and the layout function as a single source of truth. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -268,6 +268,10 @@ const EMPTY_QUERY_RECENT_TAB_CAP = 6
|
||||
const EMPTY_QUERY_ROW_BUDGET = 10
|
||||
const EMPTY_QUERY_WORKTREE_CAP = 5
|
||||
const EMPTY_RECENT_TAB_ORDER: readonly string[] = []
|
||||
const EMPTY_SORTED_WORKTREES: Worktree[] = []
|
||||
const EMPTY_BROWSER_PAGE_ENTRIES: SearchableBrowserPage[] = []
|
||||
const EMPTY_SIMULATOR_TAB_ENTRIES: SearchableSimulatorTab[] = []
|
||||
const EMPTY_WORKSPACE_TAB_ENTRIES: SearchableWorkspaceTab[] = []
|
||||
// Why: the interleaved layout emits a section header twice; the second copy needs a distinct entry id.
|
||||
const CONTINUED_SECTION_HEADER_ID_SUFFIX = '__continued'
|
||||
|
||||
@@ -483,6 +487,7 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
useShallow((s) => selectWorktreePaletteCacheInputs(s, visible || statusInputsLingering))
|
||||
)
|
||||
const migrationUnsupportedByPtyId = useAppStore((s) => s.migrationUnsupportedByPtyId)
|
||||
const activeView = useAppStore((s) => s.activeView)
|
||||
const activeWorktreeId = useAppStore((s) => s.activeWorktreeId)
|
||||
const activeTabType = useAppStore((s) => s.activeTabType)
|
||||
const activeTabId = useAppStore((s) => s.activeTabId)
|
||||
@@ -733,37 +738,12 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
return hasQuery && filterPredicate ? scope.filter(filterPredicate.matchesWorktree) : scope
|
||||
}, [allWorktrees, filterPredicate, hasQuery, switchableWorktreesForRows])
|
||||
|
||||
// Why: typed queries route through sortWorktreesSmart — ranking only diverges on the empty-query branch.
|
||||
const sortedWorktrees = useMemo(
|
||||
() =>
|
||||
hasQuery
|
||||
? sortWorktreesSmart(
|
||||
searchScopeWorktrees,
|
||||
tabsByWorktree,
|
||||
repoMap,
|
||||
agentStatusByPaneKey,
|
||||
runtimePaneTitlesByTabId,
|
||||
ptyIdsByTabId,
|
||||
migrationUnsupportedByPtyId,
|
||||
terminalLayoutsByTabId
|
||||
)
|
||||
: searchScopeWorktrees,
|
||||
[
|
||||
hasQuery,
|
||||
searchScopeWorktrees,
|
||||
tabsByWorktree,
|
||||
repoMap,
|
||||
agentStatusByPaneKey,
|
||||
runtimePaneTitlesByTabId,
|
||||
ptyIdsByTabId,
|
||||
migrationUnsupportedByPtyId,
|
||||
terminalLayoutsByTabId
|
||||
]
|
||||
)
|
||||
|
||||
// Why: browser-tab search is cross-worktree, so sort all worktrees once (including archived).
|
||||
// Gated on paletteStatusInputsActive so the closed-but-mounted palette skips the sort entirely.
|
||||
const browserSortedWorktrees = useMemo(() => {
|
||||
// Why: browser-tab search is cross-worktree, so keep indexing browser pages even when the owning worktree is archived/hidden.
|
||||
// The filter still applies — narrowing before the sort also shrinks the open-tab index it feeds.
|
||||
if (!paletteStatusInputsActive) {
|
||||
return EMPTY_SORTED_WORKTREES
|
||||
}
|
||||
const scope = filterPredicate
|
||||
? allWorktrees.filter(filterPredicate.matchesWorktree)
|
||||
: allWorktrees
|
||||
@@ -778,6 +758,7 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
terminalLayoutsByTabId
|
||||
)
|
||||
}, [
|
||||
paletteStatusInputsActive,
|
||||
allWorktrees,
|
||||
filterPredicate,
|
||||
tabsByWorktree,
|
||||
@@ -789,6 +770,13 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
terminalLayoutsByTabId
|
||||
])
|
||||
|
||||
// Why: derive typed-query sort from browserSortedWorktrees (P2-a) — both call sortWorktreesSmart
|
||||
// with the same deps, so filtering the superset avoids a redundant sort.
|
||||
const sortedWorktrees = useMemo(
|
||||
() => (hasQuery ? browserSortedWorktrees.filter((w) => !w.isArchived) : searchScopeWorktrees),
|
||||
[hasQuery, browserSortedWorktrees, searchScopeWorktrees]
|
||||
)
|
||||
|
||||
// Why: browser search includes archived worktrees, so this map must cover all worktrees, not just non-archived.
|
||||
const worktreeMap = useMemo(() => {
|
||||
const map = new Map<string, Worktree>()
|
||||
@@ -838,6 +826,9 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
)
|
||||
|
||||
const browserPageEntries = useMemo<SearchableBrowserPage[]>(() => {
|
||||
if (!paletteStatusInputsActive) {
|
||||
return EMPTY_BROWSER_PAGE_ENTRIES
|
||||
}
|
||||
return buildSearchableBrowserPages({
|
||||
worktrees: browserSortedWorktrees,
|
||||
repoMap,
|
||||
@@ -849,6 +840,7 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
activeTabType
|
||||
})
|
||||
}, [
|
||||
paletteStatusInputsActive,
|
||||
activeBrowserTabId,
|
||||
activeTabType,
|
||||
activeWorktreeId,
|
||||
@@ -865,6 +857,9 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
)
|
||||
|
||||
const simulatorTabEntries = useMemo<SearchableSimulatorTab[]>(() => {
|
||||
if (!paletteStatusInputsActive) {
|
||||
return EMPTY_SIMULATOR_TAB_ENTRIES
|
||||
}
|
||||
return buildSearchableSimulatorTabs({
|
||||
worktrees: browserSortedWorktrees,
|
||||
repoMap,
|
||||
@@ -876,6 +871,7 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
activeTabType
|
||||
})
|
||||
}, [
|
||||
paletteStatusInputsActive,
|
||||
activeGroupIdByWorktree,
|
||||
activeTabType,
|
||||
activeWorktreeId,
|
||||
@@ -892,6 +888,9 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
)
|
||||
|
||||
const workspaceTabEntries = useMemo<SearchableWorkspaceTab[]>(() => {
|
||||
if (!paletteStatusInputsActive) {
|
||||
return EMPTY_WORKSPACE_TAB_ENTRIES
|
||||
}
|
||||
return buildSearchableWorkspaceTabs({
|
||||
worktrees: browserSortedWorktrees,
|
||||
repoMap,
|
||||
@@ -914,6 +913,7 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
generatedTitlesEnabled: settings?.tabAutoGenerateTitle === true
|
||||
})
|
||||
}, [
|
||||
paletteStatusInputsActive,
|
||||
activeFileId,
|
||||
activeFileIdByWorktree,
|
||||
activeGroupIdByWorktree,
|
||||
@@ -1334,22 +1334,39 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
]
|
||||
)
|
||||
|
||||
const quickActionContext = buildQuickActionContext()
|
||||
// Why: filtering via buildQuickActionContext() inside a memo with stable primitive deps
|
||||
// instead of calling it inline every render — a fresh context object as a useMemo dep
|
||||
// defeated the middleItems memo (new identity every keystroke).
|
||||
const availableActionResults = useMemo(() => {
|
||||
const ctx = buildQuickActionContext()
|
||||
return actionResults.filter((action) => action.isAvailable(ctx).available)
|
||||
}, [
|
||||
actionResults,
|
||||
buildQuickActionContext,
|
||||
// oxlint-disable-next-line react-hooks/exhaustive-deps -- these are the availability-determining primitives buildQuickActionContext reads from the store; listing them ensures the memo recomputes when availability actually changes, not on every render.
|
||||
activeView,
|
||||
activeWorktreeId,
|
||||
worktreesByRepo,
|
||||
repos,
|
||||
sshConnectionStates,
|
||||
activeGroupIdByWorktree,
|
||||
groupsByWorktree,
|
||||
isLoading,
|
||||
settings?.activeRuntimeEnvironmentId
|
||||
])
|
||||
|
||||
const middleItems = useMemo<(SettingsPaletteItem | QuickActionPaletteItem)[]>(
|
||||
() =>
|
||||
rankCmdJMiddleResults({
|
||||
query: deferredQuery,
|
||||
settingsResults,
|
||||
actionResults: actionResults.filter(
|
||||
(action) => action.isAvailable(quickActionContext).available
|
||||
)
|
||||
actionResults: availableActionResults
|
||||
}).map((result) =>
|
||||
result.kind === 'settings'
|
||||
? { id: result.id, type: 'settings' as const, result }
|
||||
: { id: `quick-action:${result.id}`, type: 'quick-action' as const, result }
|
||||
),
|
||||
[actionResults, deferredQuery, quickActionContext, settingsResults]
|
||||
[availableActionResults, deferredQuery, settingsResults]
|
||||
)
|
||||
|
||||
// Why: both lists are relevance-sorted, so their heads carry each section's best hit. The stronger
|
||||
@@ -1476,25 +1493,12 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
})
|
||||
}
|
||||
}
|
||||
// Why the create row is excluded: it's present for every non-empty query, so counting it would
|
||||
// make "suppress lone headers" always false and reinstate the noise the rule exists to kill.
|
||||
const populatedSectionCount = [
|
||||
visibleWorktreeItems.length,
|
||||
visibleProjectTargetItems.length,
|
||||
visibleMiddleItems.length,
|
||||
visibleOpenTabItems.length
|
||||
].filter((count) => count > 0).length
|
||||
|
||||
// Header rule: empty query shows lone headers as signposts; on query, suppress unless both sections are populated (else noise).
|
||||
const showWorktreeHeader = hasQuery
|
||||
? visibleWorktreeItems.length > 0 && populatedSectionCount > 1
|
||||
: visibleWorktreeItems.length > 0
|
||||
const showOpenTabsHeader = hasQuery
|
||||
? visibleOpenTabItems.length > 0 && populatedSectionCount > 1
|
||||
: visibleOpenTabItems.length > 0
|
||||
const showProjectTargetHeader =
|
||||
hasQuery && visibleProjectTargetItems.length > 0 && populatedSectionCount > 1
|
||||
const showMiddleHeader = hasQuery && visibleMiddleItems.length > 0 && populatedSectionCount > 1
|
||||
// Why always: a lone search section still needs its label (mock single-section Open Tabs);
|
||||
// empty sections stay unlabeled because their push helpers short-circuit on zero rows.
|
||||
const showWorktreeHeader = visibleWorktreeItems.length > 0
|
||||
const showOpenTabsHeader = visibleOpenTabItems.length > 0
|
||||
const showProjectTargetHeader = visibleProjectTargetItems.length > 0
|
||||
const showMiddleHeader = visibleMiddleItems.length > 0
|
||||
|
||||
// idSuffix: the interleaved layout re-emits a header for the section's remainder, which needs its own key.
|
||||
const pushOpenTabsHeader = (idSuffix = ''): void => {
|
||||
|
||||
@@ -7,6 +7,10 @@ import type * as ReactI18Next from 'react-i18next'
|
||||
import type { Repo, Tab, TabGroup, TerminalTab, Worktree } from '../../../shared/types'
|
||||
import { useAppStore } from '@/store'
|
||||
import type { AppState } from '@/store/types'
|
||||
import {
|
||||
layoutMultiPrimaryPaletteSections,
|
||||
orderMultiPrimaryPaletteItems
|
||||
} from './cmd-j/palette-section-render-cap'
|
||||
import WorktreeJumpPalette from './WorktreeJumpPalette'
|
||||
|
||||
vi.mock('react-i18next', async (importOriginal) => {
|
||||
@@ -276,4 +280,57 @@ describe('WorktreeJumpPalette interleaved primary sections', () => {
|
||||
expect(header).toBe(rowId.startsWith('workspace-tab:') ? 'Open Tabs' : 'Worktrees')
|
||||
}
|
||||
})
|
||||
|
||||
it('renders selectable rows in the same order as orderMultiPrimaryPaletteItems', async () => {
|
||||
await renderPalette(makeInterleavedQueryState())
|
||||
|
||||
await act(async () => {
|
||||
setCommandQuery?.('perf')
|
||||
})
|
||||
await flushEffects()
|
||||
|
||||
const renderedIds = Array.from(
|
||||
testContainer.querySelectorAll<HTMLElement>('[data-command-item]')
|
||||
)
|
||||
.map((el) => el.dataset.commandItem!)
|
||||
.filter((id) => id.startsWith('workspace-tab:') || id.startsWith('worktree:'))
|
||||
|
||||
const tabIds = renderedIds.filter((id) => id.startsWith('workspace-tab:'))
|
||||
const worktreeIds = renderedIds.filter((id) => id.startsWith('worktree:'))
|
||||
|
||||
const layout = layoutMultiPrimaryPaletteSections({
|
||||
leadingItems: tabIds,
|
||||
trailingItems: worktreeIds
|
||||
})
|
||||
const expectedOrder = orderMultiPrimaryPaletteItems(layout)
|
||||
expect(renderedIds).toEqual(expectedOrder)
|
||||
})
|
||||
|
||||
it('keeps the Open Tabs header when a typed query only hits tabs', async () => {
|
||||
await renderPalette({
|
||||
worktreesByRepo: {
|
||||
'repo-1': [makeWorktree('wt-tabs', 'tab-host'), makeWorktree('wt-other', 'docs-only')]
|
||||
},
|
||||
showSleepingWorkspaces: true,
|
||||
ptyIdsByTabId: { 'term-0': ['pty-0'] },
|
||||
tabsByWorktree: {
|
||||
'wt-tabs': [makeTerminalTab('term-0', 'wt-tabs', 'ubuntu agent session')]
|
||||
},
|
||||
unifiedTabsByWorktree: {
|
||||
'wt-tabs': [makeUnifiedTab('tab-0', 'wt-tabs', 'term-0', 'ubuntu agent session')]
|
||||
},
|
||||
groupsByWorktree: { 'wt-tabs': [makeGroup('wt-tabs', ['tab-0'])] },
|
||||
activeGroupIdByWorktree: { 'wt-tabs': 'group-wt-tabs' }
|
||||
})
|
||||
|
||||
await act(async () => {
|
||||
setCommandQuery?.('ubuntu')
|
||||
})
|
||||
await flushEffects()
|
||||
|
||||
const rows = getPrimaryRowsBySectionHeader()
|
||||
expect(rows).toEqual([{ header: 'Open Tabs', rowId: 'workspace-tab:tab-0' }])
|
||||
expect(testContainer.textContent).toContain('Open Tabs')
|
||||
expect(testContainer.textContent).not.toContain('Worktrees')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -169,3 +169,92 @@ export function collectAgentMetadataForTerminal({
|
||||
|
||||
return [...metadataByPaneKey.values()]
|
||||
}
|
||||
|
||||
type IndexedAgentEntry = {
|
||||
paneKey: string
|
||||
worktreeId: string | null | undefined
|
||||
metadata: AgentMetadata
|
||||
}
|
||||
|
||||
export type AgentMetadataTabIndex = ReadonlyMap<string, readonly IndexedAgentEntry[]>
|
||||
|
||||
function pushToIndex(
|
||||
index: Map<string, IndexedAgentEntry[]>,
|
||||
tabId: string,
|
||||
entry: IndexedAgentEntry
|
||||
): void {
|
||||
let bucket = index.get(tabId)
|
||||
if (!bucket) {
|
||||
bucket = []
|
||||
index.set(tabId, bucket)
|
||||
}
|
||||
bucket.push(entry)
|
||||
}
|
||||
|
||||
export function buildAgentMetadataTabIndex(
|
||||
state: WorkspaceTabAgentMetadataState
|
||||
): AgentMetadataTabIndex {
|
||||
const index = new Map<string, IndexedAgentEntry[]>()
|
||||
const seenPaneKeys = new Set<string>()
|
||||
|
||||
for (const [paneKey, entry] of Object.entries(state.agentStatusByPaneKey)) {
|
||||
const tabId = entry.tabId || getPaneKeyTabId(paneKey)
|
||||
if (!tabId) {
|
||||
continue
|
||||
}
|
||||
seenPaneKeys.add(paneKey)
|
||||
pushToIndex(index, tabId, {
|
||||
paneKey,
|
||||
worktreeId: entry.worktreeId,
|
||||
metadata: { paneKey, ...collectLiveMetadata(entry) }
|
||||
})
|
||||
}
|
||||
|
||||
for (const [paneKey, retained] of Object.entries(state.retainedAgentsByPaneKey)) {
|
||||
if (seenPaneKeys.has(paneKey)) {
|
||||
continue
|
||||
}
|
||||
const tabId = retained.entry.tabId ?? retained.tab.id ?? getPaneKeyTabId(paneKey)
|
||||
if (!tabId) {
|
||||
continue
|
||||
}
|
||||
seenPaneKeys.add(paneKey)
|
||||
const meta = collectLiveMetadata(retained.entry)
|
||||
addText(meta.textParts, retained.tab.title)
|
||||
addText(meta.snippetCandidates, retained.tab.title)
|
||||
pushToIndex(index, tabId, {
|
||||
paneKey,
|
||||
worktreeId: retained.worktreeId,
|
||||
metadata: { paneKey, ...meta }
|
||||
})
|
||||
}
|
||||
|
||||
for (const [paneKey, record] of Object.entries(state.sleepingAgentSessionsByPaneKey)) {
|
||||
if (seenPaneKeys.has(paneKey)) {
|
||||
continue
|
||||
}
|
||||
const tabId = record.tabId || getPaneKeyTabId(paneKey)
|
||||
if (!tabId) {
|
||||
continue
|
||||
}
|
||||
pushToIndex(index, tabId, {
|
||||
paneKey,
|
||||
worktreeId: record.worktreeId,
|
||||
metadata: { paneKey, ...collectSleepingMetadata(record) }
|
||||
})
|
||||
}
|
||||
|
||||
return index
|
||||
}
|
||||
|
||||
export function collectAgentMetadataFromIndex(
|
||||
index: AgentMetadataTabIndex,
|
||||
terminalTabId: string,
|
||||
worktreeId: string
|
||||
): AgentMetadata[] {
|
||||
const entries = index.get(terminalTabId)
|
||||
if (!entries) {
|
||||
return []
|
||||
}
|
||||
return entries.filter((e) => !e.worktreeId || e.worktreeId === worktreeId).map((e) => e.metadata)
|
||||
}
|
||||
|
||||
@@ -6,7 +6,8 @@ import {
|
||||
} from '../../../shared/tab-title-resolution'
|
||||
import type { Tab, TabContentType, TabGroup, TerminalTab, Worktree } from '../../../shared/types'
|
||||
import {
|
||||
collectAgentMetadataForTerminal,
|
||||
buildAgentMetadataTabIndex,
|
||||
collectAgentMetadataFromIndex,
|
||||
type AgentMetadata,
|
||||
type WorkspaceTabAgentMetadataState
|
||||
} from './workspace-tab-agent-metadata'
|
||||
@@ -163,6 +164,11 @@ export function buildSearchableWorkspaceTabs({
|
||||
}: BuildSearchableWorkspaceTabsOptions): SearchableWorkspaceTab[] {
|
||||
const entries: SearchableWorkspaceTab[] = []
|
||||
const openFilesById = new Map(openFiles.map((file) => [file.id, file]))
|
||||
const agentIndex = buildAgentMetadataTabIndex({
|
||||
agentStatusByPaneKey,
|
||||
retainedAgentsByPaneKey,
|
||||
sleepingAgentSessionsByPaneKey
|
||||
})
|
||||
|
||||
for (const worktree of worktrees) {
|
||||
const repoName = repoMap.get(worktree.repoId)?.displayName ?? ''
|
||||
@@ -236,13 +242,7 @@ export function buildSearchableWorkspaceTabs({
|
||||
titleSearchText: title,
|
||||
secondarySearchTexts: [],
|
||||
typeSearchAliases: TERMINAL_TYPE_SEARCH_ALIASES,
|
||||
agentMetadata: collectAgentMetadataForTerminal({
|
||||
terminalTabId: tab.entityId,
|
||||
worktreeId: worktree.id,
|
||||
agentStatusByPaneKey,
|
||||
retainedAgentsByPaneKey,
|
||||
sleepingAgentSessionsByPaneKey
|
||||
})
|
||||
agentMetadata: collectAgentMetadataFromIndex(agentIndex, tab.entityId, worktree.id)
|
||||
})
|
||||
continue
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user