mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
fix(cmd-j): re-rank Recent when terminal entities hydrate late (#13105)
* Fix Cmd+J recent order when terminal entities hydrate late Unified tabs can appear before tabsByWorktree entities on restore, which latched an all-IDLE ranking and buried blocked chats until reopen. Keep a provisional freeze, then re-capture once entities arrive. * Harden Cmd+J incomplete hydration re-rank latch Clear the provisional order latch when the ranked list goes empty so a brief tab wipe cannot freeze an empty Recent section, and assert that a user-moved selection survives the incomplete→complete re-rank. * Fix Cmd+J ordering to not compare focus ordinals across worktrees focusOrdinal is a per-worktree sequence, so comparing rows from different worktrees corrupts their relative order. Preserve input (positional) order instead. Also: refactor test helpers to use makePaneKey() utility for pane-key construction, and clarify a test description about CJK character handling in relevance scoring.
This commit is contained in:
@@ -6,6 +6,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type * as ReactI18Next from 'react-i18next'
|
||||
import type { Repo, Tab, TabGroup, TerminalTab, Worktree } from '../../../shared/types'
|
||||
import type { AgentStatusEntry, AgentStatusState } from '../../../shared/agent-status-types'
|
||||
import { makePaneKey } from '../../../shared/stable-pane-id'
|
||||
import { useAppStore } from '@/store'
|
||||
import type { AppState } from '@/store/types'
|
||||
import { emitCmdJRowIndexJump } from '@/lib/cmd-j-row-index-jump'
|
||||
@@ -431,7 +432,7 @@ function makeAgentEntry(
|
||||
prompt: '',
|
||||
updatedAt: stateStartedAt,
|
||||
stateStartedAt,
|
||||
paneKey: `${tabId}:${LEAF_ID}`,
|
||||
paneKey: makePaneKey(tabId, LEAF_ID),
|
||||
stateHistory: []
|
||||
}
|
||||
}
|
||||
@@ -767,11 +768,35 @@ describe('WorktreeJumpPalette recent chats & terminals', () => {
|
||||
expect(getCommandValue()).toBe(movedTo)
|
||||
})
|
||||
|
||||
it('re-ranks once when terminal entities hydrate after unified tabs', async () => {
|
||||
// Why split hydration: unified tabs can land before tabsByWorktree; without a re-capture every
|
||||
// row ranks IDLE. A deliberate second-row highlight must survive that one re-rank.
|
||||
const hydrated = makeRecentTabState({
|
||||
agentStatusByPaneKey: {
|
||||
[makePaneKey('term-alpha', LEAF_ID)]: makeAgentEntry('term-alpha', 'blocked', Date.now())
|
||||
},
|
||||
lastVisitedAtByWorktreeId: { 'wt-beta': Date.now() }
|
||||
})
|
||||
await renderPalette({ ...hydrated, tabsByWorktree: {} })
|
||||
expect(getTabRowIds()).toEqual(['tab-beta', 'tab-alpha'])
|
||||
const movedTo = `workspace-tab:${getTabRowIds()[1]}`
|
||||
await act(async () => {
|
||||
setCommandSelection?.(movedTo)
|
||||
})
|
||||
await flushEffects()
|
||||
await act(async () => {
|
||||
useAppStore.setState({ tabsByWorktree: hydrated.tabsByWorktree } as Partial<AppState>)
|
||||
})
|
||||
await flushEffects()
|
||||
expect(getTabRowIds()).toEqual(['tab-alpha', 'tab-beta'])
|
||||
expect(getCommandValue()).toBe(movedTo)
|
||||
})
|
||||
|
||||
it('ranks a blocked agent above a more recently visited idle tab', async () => {
|
||||
await renderPalette(
|
||||
makeRecentTabState({
|
||||
agentStatusByPaneKey: {
|
||||
[`term-alpha:${LEAF_ID}`]: makeAgentEntry('term-alpha', 'blocked', Date.now())
|
||||
[makePaneKey('term-alpha', LEAF_ID)]: makeAgentEntry('term-alpha', 'blocked', Date.now())
|
||||
},
|
||||
lastVisitedAtByWorktreeId: { 'wt-beta': Date.now() }
|
||||
})
|
||||
@@ -792,7 +817,7 @@ describe('WorktreeJumpPalette recent chats & terminals', () => {
|
||||
await act(async () => {
|
||||
useAppStore.setState({
|
||||
agentStatusByPaneKey: {
|
||||
[`term-alpha:${LEAF_ID}`]: makeAgentEntry('term-alpha', 'blocked', Date.now())
|
||||
[makePaneKey('term-alpha', LEAF_ID)]: makeAgentEntry('term-alpha', 'blocked', Date.now())
|
||||
}
|
||||
} as Partial<AppState>)
|
||||
})
|
||||
|
||||
@@ -1046,20 +1046,48 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
// send ⌘3 to the wrong row; dots keep updating, positions don't.
|
||||
const [recentTabOrder, setRecentTabOrder] = useState<readonly string[]>(EMPTY_RECENT_TAB_ORDER)
|
||||
const recentTabOrderCapturedRef = useRef(false)
|
||||
// Why: unified tabs can land before tabsByWorktree entities. A capture then ranks every chat as
|
||||
// IDLE; allow one re-capture when entities arrive, then freeze for good.
|
||||
const recentTabOrderAttentionReadyRef = useRef(false)
|
||||
// Terminal rows without a tabsByWorktree entity can't resolve attention yet (see orderRecent…).
|
||||
const recentOrderAttentionIncomplete = useMemo(() => {
|
||||
for (const item of openTabItems) {
|
||||
if (item.type !== 'workspace-tab' || item.result.contentType !== 'terminal') {
|
||||
continue
|
||||
}
|
||||
const worktree = worktreeMap.get(item.result.worktreeId)
|
||||
if (!worktree || worktree.isArchived || isCurrentOpenTabItem(item)) {
|
||||
continue
|
||||
}
|
||||
if (!terminalTabsById.has(item.result.entityId)) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}, [openTabItems, terminalTabsById, worktreeMap])
|
||||
// Why layout, not passive: a post-paint capture shows one frame of worktrees-only, which flashes
|
||||
// the list, renumbers ⌘1–6 under the user, and lets cmdk latch a worktree as the Enter target.
|
||||
useLayoutEffect(() => {
|
||||
if (!visible) {
|
||||
recentTabOrderCapturedRef.current = false
|
||||
recentTabOrderAttentionReadyRef.current = false
|
||||
autoSelectedItemIdRef.current = null
|
||||
setRecentTabOrder(EMPTY_RECENT_TAB_ORDER)
|
||||
return
|
||||
}
|
||||
// Why: the query and filter are cleared by the open effect below, which runs after this one —
|
||||
// capturing before that lands would freeze the *previous* session's filtered subset for good.
|
||||
if (recentTabOrderCapturedRef.current || hasQuery || query.length > 0 || filterActive) {
|
||||
if (hasQuery || query.length > 0 || filterActive) {
|
||||
return
|
||||
}
|
||||
// Fully frozen after an attention-ready capture; provisional freeze while entities still pending
|
||||
// so agent-status churn can't reshuffle under the cursor before the one-shot re-rank.
|
||||
if (recentTabOrderCapturedRef.current) {
|
||||
if (recentTabOrderAttentionReadyRef.current || recentOrderAttentionIncomplete) {
|
||||
return
|
||||
}
|
||||
// Incomplete → complete: fall through and re-capture with real attention ranks.
|
||||
}
|
||||
const order = orderRecentWorkspaceTabs({
|
||||
rows: recentTabRows,
|
||||
paneSources: recentTabPaneSources,
|
||||
@@ -1070,11 +1098,15 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
if (order.length === 0) {
|
||||
// Why: tabs can arrive after the palette opens (cold start, session restore, a late tab
|
||||
// mirror). Latching an empty snapshot would leave Recent dead — and digits inert — until
|
||||
// close+reopen, so stay unlatched; the stable empty ref keeps this a no-op re-render.
|
||||
// close+reopen. Also clear a provisional latch: incomplete→complete fallthrough can hit empty
|
||||
// if open tabs briefly vanish, and keeping captured would freeze an empty Recent forever.
|
||||
recentTabOrderCapturedRef.current = false
|
||||
recentTabOrderAttentionReadyRef.current = false
|
||||
setRecentTabOrder(EMPTY_RECENT_TAB_ORDER)
|
||||
return
|
||||
}
|
||||
recentTabOrderCapturedRef.current = true
|
||||
recentTabOrderAttentionReadyRef.current = !recentOrderAttentionIncomplete
|
||||
setRecentTabOrder(order)
|
||||
// Why: recents render above the worktrees, so a row auto-selected before they arrived is no
|
||||
// longer the list head — hand Enter back to the top, matching ⌘1. Untouched selections only:
|
||||
@@ -1089,6 +1121,7 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
hasQuery,
|
||||
lastVisitedAtByWorktreeId,
|
||||
query.length,
|
||||
recentOrderAttentionIncomplete,
|
||||
recentTabPaneSources,
|
||||
recentTabRows,
|
||||
visible
|
||||
|
||||
@@ -6,6 +6,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { useAppStore } from '@/store'
|
||||
import type { AppState } from '@/store/types'
|
||||
import type { AgentStatusEntry, AgentStatusState } from '../../../../shared/agent-status-types'
|
||||
import { makePaneKey } from '../../../../shared/stable-pane-id'
|
||||
import type { TerminalTab } from '../../../../shared/types'
|
||||
import {
|
||||
PaletteLiveStatusProvider,
|
||||
@@ -42,14 +43,14 @@ function makeAgentEntry(tabId: string, state: AgentStatusState): AgentStatusEntr
|
||||
prompt: '',
|
||||
updatedAt: Date.now(),
|
||||
stateStartedAt: Date.now(),
|
||||
paneKey: `${tabId}:${LEAF}`,
|
||||
paneKey: makePaneKey(tabId, LEAF),
|
||||
stateHistory: []
|
||||
}
|
||||
}
|
||||
|
||||
function setAgentState(state: AgentStatusState): void {
|
||||
useAppStore.setState((s) => ({
|
||||
agentStatusByPaneKey: { [`term-a:${LEAF}`]: makeAgentEntry('term-a', state) },
|
||||
agentStatusByPaneKey: { [makePaneKey('term-a', LEAF)]: makeAgentEntry('term-a', state) },
|
||||
agentStatusEpoch: s.agentStatusEpoch + 1
|
||||
}))
|
||||
}
|
||||
|
||||
@@ -79,7 +79,7 @@ describe('scorePaletteRelevance', () => {
|
||||
expect(worstPrimary).toBeLessThan(bestSecondary)
|
||||
})
|
||||
|
||||
it('treats a CJK character before the match as a word boundary', () => {
|
||||
it('treats a CJK letter before the match as mid-word, like a Latin letter', () => {
|
||||
expect(
|
||||
scorePaletteRelevance([{ text: '工作树perf', range: { start: 3, end: 7 }, tier: 0 }])
|
||||
).toBe(scorePaletteRelevance([{ text: 'aperf', range: { start: 1, end: 5 }, tier: 0 }]))
|
||||
|
||||
@@ -159,6 +159,26 @@ describe('orderRecentWorkspaceTabs', () => {
|
||||
).toEqual(['second', 'third', 'first'])
|
||||
})
|
||||
|
||||
it('keeps input order across worktrees instead of comparing their unrelated MRU ordinals', () => {
|
||||
// Callers pass worktree-grouped positional order; both worktrees are never-visited, so only
|
||||
// the per-worktree focus ordinals differ — beta's larger ordinal must not hoist it over alpha.
|
||||
const rows = [
|
||||
row('alpha-1', { worktreeId: 'wt-alpha', unifiedTabId: 'unified-alpha-1' }),
|
||||
row('alpha-2', { worktreeId: 'wt-alpha', unifiedTabId: 'unified-alpha-2' }),
|
||||
row('beta-1', { worktreeId: 'wt-beta', unifiedTabId: 'unified-beta-1' })
|
||||
]
|
||||
|
||||
expect(
|
||||
order(rows, sources([]), {
|
||||
focusedGroupTabRecency: new Map([
|
||||
['unified-alpha-1', 0],
|
||||
['unified-alpha-2', 1],
|
||||
['unified-beta-1', 5]
|
||||
])
|
||||
})
|
||||
).toEqual(['alpha-2', 'alpha-1', 'beta-1'])
|
||||
})
|
||||
|
||||
it('keeps input (positional) order when nothing else separates two rows', () => {
|
||||
const rows = [row('a', { worktreeId: 'wt-1' }), row('b', { worktreeId: 'wt-1' })]
|
||||
|
||||
|
||||
@@ -43,6 +43,7 @@ type RankedRow = {
|
||||
attentionTimestamp: number
|
||||
visitedAt: number | undefined
|
||||
focusOrdinal: number
|
||||
worktreeId: string
|
||||
}
|
||||
|
||||
/** Classes 1 (blocked/waiting) and 2 (freshly done) are the rows that want the user. */
|
||||
@@ -125,6 +126,11 @@ function compareRankedRows(a: RankedRow, b: RankedRow): number {
|
||||
}
|
||||
return b.visitedAt - a.visitedAt
|
||||
}
|
||||
// Why: focusOrdinal is a per-worktree sequence, so comparing it across worktrees interleaves
|
||||
// them arbitrarily. Keep input (positional) order instead.
|
||||
if (a.worktreeId !== b.worktreeId) {
|
||||
return 0
|
||||
}
|
||||
return b.focusOrdinal - a.focusOrdinal
|
||||
}
|
||||
|
||||
@@ -145,6 +151,7 @@ export function orderRecentWorkspaceTabs(inputs: RecentWorkspaceTabOrderInputs):
|
||||
attentionClass: attention.cls,
|
||||
attentionTimestamp: attention.attentionTimestamp,
|
||||
visitedAt: lastVisitedAtByWorktreeId[row.worktreeId],
|
||||
worktreeId: row.worktreeId,
|
||||
focusOrdinal:
|
||||
row.unifiedTabId === null
|
||||
? NO_FOCUS_ORDINAL
|
||||
|
||||
Reference in New Issue
Block a user