mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
perf(renderer): index GitHub resume lookups (#13698)
* perf(renderer): index GitHub resume lookups * perf(renderer): skip the resume cache sweep publication when nothing changed Every eviction helper already returns its input reference untouched when it evicts nothing, so the opening sweep in refreshAllGitHub can compare identities and return the previous state. Window resume then stops waking every store subscriber on a no-op sweep. Folds in the remaining unique optimization from #13711, which this PR supersedes on the lookup-indexing side. Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil
Orca
@@ -0,0 +1,66 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { Repo } from '../../../../shared/types'
|
||||
import { getGitHubRepoLookupIndex } from './github-repo-lookup-index'
|
||||
|
||||
function makeRepo(id: string, path: string): Repo {
|
||||
return { id, path, displayName: id, badgeColor: 'blue', addedAt: 1, kind: 'git' }
|
||||
}
|
||||
|
||||
describe('getGitHubRepoLookupIndex', () => {
|
||||
it('retains the first duplicate after indexing later rows', () => {
|
||||
const first = makeRepo('duplicate', '/first')
|
||||
const second = makeRepo('duplicate', '/second')
|
||||
const later = makeRepo('later', '/later')
|
||||
const lookup = getGitHubRepoLookupIndex([first, second, later])
|
||||
|
||||
expect(lookup.findById('duplicate')).toBe(first)
|
||||
expect(lookup.findById('later')).toBe(later)
|
||||
expect(lookup.findById('duplicate')).toBe(first)
|
||||
})
|
||||
|
||||
it('matches Array.find ordering for combined ID and path lookups', () => {
|
||||
const pathMatch = makeRepo('other', '/target')
|
||||
const idMatch = makeRepo('target', '/other')
|
||||
const lookup = getGitHubRepoLookupIndex([pathMatch, idMatch])
|
||||
|
||||
expect(lookup.findByIdOrPath('target', '/target')).toBe(pathMatch)
|
||||
})
|
||||
|
||||
it('scans each repo once across repeated misses', () => {
|
||||
let idReads = 0
|
||||
const repos = Array.from({ length: 32 }, (_, index) => {
|
||||
const repo = makeRepo(`repo-${index}`, `/repo-${index}`)
|
||||
return Object.defineProperty(repo, 'id', {
|
||||
configurable: true,
|
||||
enumerable: true,
|
||||
get: () => {
|
||||
idReads += 1
|
||||
return `repo-${index}`
|
||||
}
|
||||
})
|
||||
})
|
||||
const lookup = getGitHubRepoLookupIndex(repos)
|
||||
|
||||
expect(lookup.findById('missing-first')).toBeUndefined()
|
||||
expect(lookup.findById('missing-second')).toBeUndefined()
|
||||
expect(idReads).toBe(repos.length)
|
||||
})
|
||||
|
||||
it('stops after a sparse first-row match', () => {
|
||||
let idReads = 0
|
||||
const repos = Array.from({ length: 32 }, (_, index) => {
|
||||
const repo = makeRepo(`repo-${index}`, `/repo-${index}`)
|
||||
return Object.defineProperty(repo, 'id', {
|
||||
configurable: true,
|
||||
enumerable: true,
|
||||
get: () => {
|
||||
idReads += 1
|
||||
return `repo-${index}`
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
expect(getGitHubRepoLookupIndex(repos).findById('repo-0')).toBe(repos[0])
|
||||
expect(idReads).toBe(1)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,72 @@
|
||||
import type { Repo } from '../../../../shared/types'
|
||||
|
||||
type IndexedRepo = {
|
||||
index: number
|
||||
repo: Repo
|
||||
}
|
||||
|
||||
export type GitHubRepoLookupIndex = {
|
||||
findById: (repoId: string) => Repo | undefined
|
||||
findByPath: (repoPath: string) => Repo | undefined
|
||||
findByIdOrPath: (repoId: string | undefined, repoPath: string) => Repo | undefined
|
||||
}
|
||||
|
||||
// Why: repo identity/path updates replace this array, while weak ownership avoids retaining superseded snapshots.
|
||||
const lookupByRepos = new WeakMap<readonly Repo[], GitHubRepoLookupIndex>()
|
||||
const EMPTY_REPOS: readonly Repo[] = []
|
||||
|
||||
export function getGitHubRepoLookupIndex(
|
||||
repos: readonly Repo[] | undefined
|
||||
): GitHubRepoLookupIndex {
|
||||
const repoRows = repos ?? EMPTY_REPOS
|
||||
const cached = lookupByRepos.get(repoRows)
|
||||
if (cached) {
|
||||
return cached
|
||||
}
|
||||
|
||||
const byId = new Map<string, IndexedRepo>()
|
||||
const byPath = new Map<string, IndexedRepo>()
|
||||
let indexedCount = 0
|
||||
|
||||
const scanUntil = (matches: (repoId: string, repoPath: string) => boolean): Repo | undefined => {
|
||||
while (indexedCount < repoRows.length) {
|
||||
const index = indexedCount
|
||||
const repo = repoRows[index]
|
||||
indexedCount += 1
|
||||
const repoId = repo.id
|
||||
const repoPath = repo.path
|
||||
if (!byId.has(repoId)) {
|
||||
byId.set(repoId, { index, repo })
|
||||
}
|
||||
if (!byPath.has(repoPath)) {
|
||||
byPath.set(repoPath, { index, repo })
|
||||
}
|
||||
if (matches(repoId, repoPath)) {
|
||||
return repo
|
||||
}
|
||||
}
|
||||
return undefined
|
||||
}
|
||||
|
||||
const lookup: GitHubRepoLookupIndex = {
|
||||
findById: (repoId) => byId.get(repoId)?.repo ?? scanUntil((id) => id === repoId),
|
||||
findByPath: (repoPath) =>
|
||||
byPath.get(repoPath)?.repo ?? scanUntil((_id, path) => path === repoPath),
|
||||
findByIdOrPath: (repoId, repoPath) => {
|
||||
if (!repoId) {
|
||||
return lookup.findByPath(repoPath)
|
||||
}
|
||||
const idMatch = byId.get(repoId)
|
||||
const pathMatch = byPath.get(repoPath)
|
||||
if (idMatch || pathMatch) {
|
||||
if (!pathMatch || (idMatch && idMatch.index < pathMatch.index)) {
|
||||
return idMatch?.repo
|
||||
}
|
||||
return pathMatch.repo
|
||||
}
|
||||
return scanUntil((id, path) => id === repoId || path === repoPath)
|
||||
}
|
||||
}
|
||||
lookupByRepos.set(repoRows, lookup)
|
||||
return lookup
|
||||
}
|
||||
@@ -5425,6 +5425,447 @@ describe('createGitHubSlice.refreshGitHubForWorktreeIfStale', () => {
|
||||
describe('createGitHubSlice.refreshAllGitHub', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
resetRemoteRuntimeMocks()
|
||||
})
|
||||
|
||||
it('publishes no store update when the cache sweep changes nothing', () => {
|
||||
const store = createTestStore()
|
||||
store.setState({
|
||||
repos: [{ id: 'repo-1', path: '/repo', name: 'repo', kind: 'git' }],
|
||||
groupBy: 'repo',
|
||||
worktreeCardProperties: ['comment'],
|
||||
rightSidebarOpen: false,
|
||||
worktreesByRepo: { 'repo-1': [makePRRefreshWorktree()] }
|
||||
} as unknown as Partial<AppState>)
|
||||
let publications = 0
|
||||
const unsubscribe = store.subscribe(() => {
|
||||
publications += 1
|
||||
})
|
||||
|
||||
store.getState().refreshAllGitHub()
|
||||
unsubscribe()
|
||||
|
||||
expect(publications).toBe(0)
|
||||
})
|
||||
|
||||
it('still clears populated comments when no workspace refresh is needed', () => {
|
||||
const store = createTestStore()
|
||||
store.setState({
|
||||
commentsCache: { cached: { data: [], fetchedAt: 1 } },
|
||||
groupBy: 'repo',
|
||||
worktreeCardProperties: ['comment'],
|
||||
rightSidebarOpen: false
|
||||
} as unknown as Partial<AppState>)
|
||||
let publications = 0
|
||||
const unsubscribe = store.subscribe(() => {
|
||||
publications += 1
|
||||
})
|
||||
|
||||
store.getState().refreshAllGitHub()
|
||||
unsubscribe()
|
||||
|
||||
expect(store.getState().commentsCache).toEqual({})
|
||||
expect(publications).toBe(1)
|
||||
})
|
||||
|
||||
it('skips repo and worktree identity reads when no GitHub decoration is visible', () => {
|
||||
const store = createTestStore()
|
||||
const repoId = 'repo-idle'
|
||||
let repoIdentityReads = 0
|
||||
let worktreeRepoIdentityReads = 0
|
||||
const repo = { id: repoId, path: '/idle', name: 'idle', kind: 'git' as const }
|
||||
const worktree = makePRRefreshWorktree({
|
||||
id: 'wt-idle',
|
||||
repoId,
|
||||
path: '/idle/worktrees/idle',
|
||||
branch: 'feature/idle'
|
||||
})
|
||||
Object.defineProperty(repo, 'id', {
|
||||
configurable: true,
|
||||
enumerable: true,
|
||||
get: () => {
|
||||
repoIdentityReads += 1
|
||||
return repoId
|
||||
}
|
||||
})
|
||||
Object.defineProperty(worktree, 'repoId', {
|
||||
configurable: true,
|
||||
enumerable: true,
|
||||
get: () => {
|
||||
worktreeRepoIdentityReads += 1
|
||||
return repoId
|
||||
}
|
||||
})
|
||||
store.setState({
|
||||
repos: [repo],
|
||||
groupBy: 'repo',
|
||||
worktreeCardProperties: ['comment'],
|
||||
rightSidebarOpen: false,
|
||||
commentsCache: { cached: { data: [], fetchedAt: 1 } },
|
||||
worktreesByRepo: { [repoId]: [worktree] }
|
||||
} as unknown as Partial<AppState>)
|
||||
repoIdentityReads = 0
|
||||
worktreeRepoIdentityReads = 0
|
||||
|
||||
store.getState().refreshAllGitHub()
|
||||
|
||||
expect(store.getState().commentsCache).toEqual({})
|
||||
expect(repoIdentityReads).toBe(0)
|
||||
expect(worktreeRepoIdentityReads).toBe(0)
|
||||
expect(mockApi.gh.enqueuePRRefresh).not.toHaveBeenCalled()
|
||||
expect(mockApi.gh.issue).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('bounds stale PR repo identity reads to a constant amount per repo and worktree', () => {
|
||||
const store = createTestStore()
|
||||
const repoCount = 128
|
||||
let repoIdentityReads = 0
|
||||
const repoIds = Array.from({ length: repoCount }, (_, index) => `repo-${index}`)
|
||||
const repos = repoIds.map((repoId) => {
|
||||
const repo = { id: repoId, path: `/${repoId}`, name: repoId, kind: 'git' as const }
|
||||
return Object.defineProperty(repo, 'id', {
|
||||
configurable: true,
|
||||
enumerable: true,
|
||||
get: () => {
|
||||
repoIdentityReads += 1
|
||||
return repoId
|
||||
}
|
||||
})
|
||||
})
|
||||
const worktreesByRepo = Object.fromEntries(
|
||||
repoIds.map((repoId, index) => [
|
||||
repoId,
|
||||
[
|
||||
makePRRefreshWorktree({
|
||||
id: `wt-${index}`,
|
||||
repoId,
|
||||
path: `/${repoId}/worktrees/feature`,
|
||||
branch: `feature/${index}`,
|
||||
lastActivityAt: index
|
||||
})
|
||||
]
|
||||
])
|
||||
)
|
||||
store.setState({
|
||||
repos,
|
||||
groupBy: 'repo',
|
||||
worktreeCardProperties: ['pr'],
|
||||
rightSidebarOpen: false,
|
||||
worktreesByRepo
|
||||
} as unknown as Partial<AppState>)
|
||||
repoIdentityReads = 0
|
||||
|
||||
store.getState().refreshAllGitHub()
|
||||
|
||||
expect(repoIdentityReads).toBeLessThanOrEqual(repoCount * 5)
|
||||
expect(mockApi.gh.enqueuePRRefresh).toHaveBeenCalledTimes(5)
|
||||
})
|
||||
|
||||
it('stops repo indexing after a sparse enabled match', () => {
|
||||
const store = createTestStore()
|
||||
const repoCount = 128
|
||||
let repoIdentityReads = 0
|
||||
const repos = Array.from({ length: repoCount }, (_, index) => {
|
||||
const repoId = `repo-${index}`
|
||||
const repo = { id: repoId, path: `/${repoId}`, name: repoId, kind: 'git' as const }
|
||||
return Object.defineProperty(repo, 'id', {
|
||||
configurable: true,
|
||||
enumerable: true,
|
||||
get: () => {
|
||||
repoIdentityReads += 1
|
||||
return repoId
|
||||
}
|
||||
})
|
||||
})
|
||||
store.setState({
|
||||
repos,
|
||||
groupBy: 'repo',
|
||||
worktreeCardProperties: ['pr'],
|
||||
rightSidebarOpen: false,
|
||||
worktreesByRepo: {
|
||||
'repo-0': [
|
||||
makePRRefreshWorktree({
|
||||
id: 'wt-sparse',
|
||||
repoId: 'repo-0',
|
||||
path: '/repo-0/worktrees/feature',
|
||||
branch: 'feature/sparse'
|
||||
})
|
||||
]
|
||||
}
|
||||
} as unknown as Partial<AppState>)
|
||||
repoIdentityReads = 0
|
||||
|
||||
store.getState().refreshAllGitHub()
|
||||
|
||||
expect(repoIdentityReads).toBeLessThanOrEqual(5)
|
||||
expect(mockApi.gh.enqueuePRRefresh).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('does not restart repo indexing for repeated missing IDs', () => {
|
||||
const store = createTestStore()
|
||||
const repoCount = 128
|
||||
let repoIdentityReads = 0
|
||||
const repos = Array.from({ length: repoCount }, (_, index) => {
|
||||
const repoId = `repo-${index}`
|
||||
const repo = { id: repoId, path: `/${repoId}`, name: repoId, kind: 'git' as const }
|
||||
return Object.defineProperty(repo, 'id', {
|
||||
configurable: true,
|
||||
enumerable: true,
|
||||
get: () => {
|
||||
repoIdentityReads += 1
|
||||
return repoId
|
||||
}
|
||||
})
|
||||
})
|
||||
const missingWorktrees = Array.from({ length: repoCount }, (_, index) =>
|
||||
makePRRefreshWorktree({
|
||||
id: `wt-missing-${index}`,
|
||||
repoId: `missing-${index}`,
|
||||
branch: `feature/missing-${index}`
|
||||
})
|
||||
)
|
||||
store.setState({
|
||||
repos,
|
||||
groupBy: 'repo',
|
||||
worktreeCardProperties: ['pr'],
|
||||
rightSidebarOpen: false,
|
||||
worktreesByRepo: { missing: missingWorktrees }
|
||||
} as unknown as Partial<AppState>)
|
||||
repoIdentityReads = 0
|
||||
|
||||
store.getState().refreshAllGitHub()
|
||||
|
||||
expect(repoIdentityReads).toBeLessThanOrEqual(repoCount)
|
||||
expect(mockApi.gh.enqueuePRRefresh).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps runtime PR dispatch identity reads linear in PR-status grouping', async () => {
|
||||
runtimeEnvironmentCall.mockResolvedValue({
|
||||
id: 'rpc-linear-pr',
|
||||
ok: true,
|
||||
result: makePR({ number: 12 }),
|
||||
_meta: { runtimeId: 'remote-runtime' }
|
||||
})
|
||||
const store = createTestStore()
|
||||
const repoCount = 128
|
||||
let repoIdentityReads = 0
|
||||
let worktreeIdentityReads = 0
|
||||
const repos = Array.from({ length: repoCount }, (_, index) => {
|
||||
const repoId = `runtime-repo-${index}`
|
||||
const repoPath = `/runtime/repo-${index}`
|
||||
const repo = {
|
||||
id: repoId,
|
||||
path: repoPath,
|
||||
name: repoId,
|
||||
kind: 'git' as const,
|
||||
executionHostId: 'runtime:env-1'
|
||||
}
|
||||
Object.defineProperty(repo, 'id', {
|
||||
configurable: true,
|
||||
enumerable: true,
|
||||
get: () => {
|
||||
repoIdentityReads += 1
|
||||
return repoId
|
||||
}
|
||||
})
|
||||
return Object.defineProperty(repo, 'path', {
|
||||
configurable: true,
|
||||
enumerable: true,
|
||||
get: () => {
|
||||
repoIdentityReads += 1
|
||||
return repoPath
|
||||
}
|
||||
})
|
||||
})
|
||||
const worktreesByRepo = Object.fromEntries(
|
||||
repos.map((repo, index) => {
|
||||
const worktreeId = `runtime-wt-${index}`
|
||||
const worktree = makePRRefreshWorktree({
|
||||
id: worktreeId,
|
||||
repoId: repo.id,
|
||||
path: `${repo.path}/worktrees/feature`,
|
||||
branch: `feature/${index}`,
|
||||
linkedPR: 12,
|
||||
lastActivityAt: index
|
||||
})
|
||||
Object.defineProperty(worktree, 'id', {
|
||||
configurable: true,
|
||||
enumerable: true,
|
||||
get: () => {
|
||||
worktreeIdentityReads += 1
|
||||
return worktreeId
|
||||
}
|
||||
})
|
||||
return [repo.id, [worktree]]
|
||||
})
|
||||
)
|
||||
store.setState({
|
||||
settings: { activeRuntimeEnvironmentId: 'env-1' } as AppState['settings'],
|
||||
repos,
|
||||
groupBy: 'pr-status',
|
||||
worktreeCardProperties: ['comment'],
|
||||
worktreesByRepo
|
||||
} as unknown as Partial<AppState>)
|
||||
repoIdentityReads = 0
|
||||
worktreeIdentityReads = 0
|
||||
|
||||
store.getState().refreshAllGitHub()
|
||||
|
||||
await vi.waitFor(() => expect(Object.keys(store.getState().prCache)).toHaveLength(repoCount))
|
||||
expect(runtimeEnvironmentCall).toHaveBeenCalledTimes(repoCount)
|
||||
expect(repoIdentityReads).toBeLessThanOrEqual(repoCount * 20)
|
||||
expect(worktreeIdentityReads).toBeLessThanOrEqual(repoCount * 5)
|
||||
})
|
||||
|
||||
it('keeps runtime issue dispatch repo identity reads linear', async () => {
|
||||
runtimeEnvironmentCall.mockResolvedValue({
|
||||
id: 'rpc-linear-issue',
|
||||
ok: true,
|
||||
result: null,
|
||||
_meta: { runtimeId: 'remote-runtime' }
|
||||
})
|
||||
const store = createTestStore()
|
||||
const repoCount = 128
|
||||
let repoIdentityReads = 0
|
||||
const repos = Array.from({ length: repoCount }, (_, index) => {
|
||||
const repoId = `issue-repo-${index}`
|
||||
const repoPath = `/runtime/issues-${index}`
|
||||
const repo = {
|
||||
id: repoId,
|
||||
path: repoPath,
|
||||
name: repoId,
|
||||
kind: 'git' as const,
|
||||
executionHostId: 'runtime:env-1'
|
||||
}
|
||||
Object.defineProperty(repo, 'id', {
|
||||
configurable: true,
|
||||
enumerable: true,
|
||||
get: () => {
|
||||
repoIdentityReads += 1
|
||||
return repoId
|
||||
}
|
||||
})
|
||||
return Object.defineProperty(repo, 'path', {
|
||||
configurable: true,
|
||||
enumerable: true,
|
||||
get: () => {
|
||||
repoIdentityReads += 1
|
||||
return repoPath
|
||||
}
|
||||
})
|
||||
})
|
||||
const worktreesByRepo = Object.fromEntries(
|
||||
repos.map((repo, index) => [
|
||||
repo.id,
|
||||
[
|
||||
makePRRefreshWorktree({
|
||||
id: `issue-wt-${index}`,
|
||||
repoId: repo.id,
|
||||
path: `${repo.path}/worktrees/feature`,
|
||||
linkedIssue: index + 1
|
||||
})
|
||||
]
|
||||
])
|
||||
)
|
||||
store.setState({
|
||||
settings: { activeRuntimeEnvironmentId: 'env-1' } as AppState['settings'],
|
||||
repos,
|
||||
groupBy: 'repo',
|
||||
worktreeCardProperties: ['issue'],
|
||||
worktreesByRepo
|
||||
} as unknown as Partial<AppState>)
|
||||
repoIdentityReads = 0
|
||||
|
||||
store.getState().refreshAllGitHub()
|
||||
|
||||
await vi.waitFor(() => expect(Object.keys(store.getState().issueCache)).toHaveLength(repoCount))
|
||||
expect(runtimeEnvironmentCall).toHaveBeenCalledTimes(repoCount)
|
||||
expect(repoIdentityReads).toBeLessThanOrEqual(repoCount * 15)
|
||||
})
|
||||
|
||||
it('keeps the first repo owner when duplicate IDs span hosts', () => {
|
||||
const store = createTestStore()
|
||||
const repoId = 'duplicate-repo'
|
||||
const firstRepo = {
|
||||
id: repoId,
|
||||
path: '/first',
|
||||
name: 'first',
|
||||
kind: 'git' as const,
|
||||
connectionId: 'first',
|
||||
executionHostId: 'ssh:first'
|
||||
}
|
||||
const secondRepo = {
|
||||
id: repoId,
|
||||
path: '/second',
|
||||
name: 'second',
|
||||
kind: 'git' as const,
|
||||
connectionId: 'second',
|
||||
executionHostId: 'ssh:second'
|
||||
}
|
||||
const laterRepo = {
|
||||
id: 'later-repo',
|
||||
path: '/later',
|
||||
name: 'later',
|
||||
kind: 'git' as const
|
||||
}
|
||||
store.setState({
|
||||
repos: [firstRepo, secondRepo, laterRepo],
|
||||
groupBy: 'repo',
|
||||
worktreeCardProperties: ['pr'],
|
||||
rightSidebarOpen: false,
|
||||
sshConnectionStates: new Map([
|
||||
['first', { status: 'connected' }],
|
||||
['second', { status: 'connected' }]
|
||||
]),
|
||||
worktreesByRepo: {
|
||||
first: [
|
||||
makePRRefreshWorktree({
|
||||
id: 'wt-duplicate-first',
|
||||
repoId,
|
||||
path: '/first/worktrees/feature',
|
||||
branch: 'feature/duplicate-first'
|
||||
})
|
||||
],
|
||||
middle: [
|
||||
makePRRefreshWorktree({
|
||||
id: 'wt-later',
|
||||
repoId: laterRepo.id,
|
||||
path: '/later/worktrees/feature',
|
||||
branch: 'feature/later'
|
||||
})
|
||||
],
|
||||
last: [
|
||||
makePRRefreshWorktree({
|
||||
id: 'wt-duplicate-last',
|
||||
repoId,
|
||||
path: '/first/worktrees/last',
|
||||
branch: 'feature/duplicate-last'
|
||||
})
|
||||
]
|
||||
}
|
||||
} as unknown as Partial<AppState>)
|
||||
|
||||
store.getState().refreshAllGitHub()
|
||||
|
||||
const duplicateCandidates = mockApi.gh.enqueuePRRefresh.mock.calls
|
||||
.map(([call]) => call.candidate)
|
||||
.filter((candidate) => candidate.repoId === repoId)
|
||||
expect(duplicateCandidates).toHaveLength(2)
|
||||
expect(duplicateCandidates).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
repoPath: '/first',
|
||||
connectionId: 'first',
|
||||
executionHostId: 'ssh:first'
|
||||
}),
|
||||
expect.objectContaining({
|
||||
repoPath: '/first',
|
||||
connectionId: 'first',
|
||||
executionHostId: 'ssh:first'
|
||||
})
|
||||
])
|
||||
)
|
||||
expect(runtimeEnvironmentCall).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('refreshes stale PR data when source control is the visible PR surface', () => {
|
||||
|
||||
@@ -76,6 +76,7 @@ import {
|
||||
} from '../../../../shared/task-source-context'
|
||||
import { normalizeGitHubPRForBranchOutcome } from '../../../../shared/github-pr-for-branch-outcome'
|
||||
import { restoreReactionOnSubject, setReactionOnSubject } from '@/lib/pr-comment-reactions'
|
||||
import { getGitHubRepoLookupIndex } from './github-repo-lookup-index'
|
||||
|
||||
// ─── ProjectV2 cache types ────────────────────────────────────────────
|
||||
// Why: separate from CacheEntry<T> — project-view has a single GraphQL source (no issue/PR fallback) and a distinct error union.
|
||||
@@ -125,7 +126,7 @@ function getRuntimeRepoTarget(
|
||||
if (target.kind !== 'environment') {
|
||||
return null
|
||||
}
|
||||
const repo = state.repos.find((candidate) => candidate.path === repoPath)
|
||||
const repo = getGitHubRepoLookupIndex(state.repos).findByPath(repoPath)
|
||||
return repo ? { target, repo } : null
|
||||
}
|
||||
|
||||
@@ -249,9 +250,9 @@ function findRepoForGitHubOwner(
|
||||
repoId: string | undefined,
|
||||
repoPath: string
|
||||
): Repo | undefined {
|
||||
return (state.repos ?? []).find((candidate) =>
|
||||
repoId ? candidate.id === repoId || candidate.path === repoPath : candidate.path === repoPath
|
||||
)
|
||||
return state.repos
|
||||
? getGitHubRepoLookupIndex(state.repos).findByIdOrPath(repoId, repoPath)
|
||||
: undefined
|
||||
}
|
||||
|
||||
function getGitHubFocusedRepoOwnerHostId(
|
||||
@@ -962,13 +963,7 @@ function getPRChecksCacheTtl(entry: CacheEntry<PRCheckDetail[]> | undefined): nu
|
||||
}
|
||||
|
||||
function findWorktreeById(state: AppState, worktreeId: string): Worktree | null {
|
||||
for (const worktrees of Object.values(state.worktreesByRepo)) {
|
||||
const worktree = worktrees.find((w) => w.id === worktreeId)
|
||||
if (worktree) {
|
||||
return worktree
|
||||
}
|
||||
}
|
||||
return null
|
||||
return getWorktreeLookupIndex(state).byId.get(worktreeId)?.first ?? null
|
||||
}
|
||||
|
||||
type WorktreeLookupEntry = {
|
||||
@@ -981,9 +976,12 @@ type WorktreeLookupIndex = {
|
||||
repoHostIdsByRepoId: Map<string, Set<string>>
|
||||
}
|
||||
|
||||
const EMPTY_WORKTREES_BY_REPO: AppState['worktreesByRepo'] = {}
|
||||
const EMPTY_WORKTREE_REPOS: AppState['repos'] = []
|
||||
|
||||
function buildWorktreeLookupIndex(state: AppState): WorktreeLookupIndex {
|
||||
const byId = new Map<string, WorktreeLookupEntry>()
|
||||
for (const worktrees of Object.values(state.worktreesByRepo)) {
|
||||
for (const worktrees of Object.values(state.worktreesByRepo ?? EMPTY_WORKTREES_BY_REPO)) {
|
||||
for (const worktree of worktrees) {
|
||||
const worktreeId = worktree.id
|
||||
const existing = byId.get(worktreeId)
|
||||
@@ -1007,11 +1005,29 @@ function buildWorktreeLookupIndex(state: AppState): WorktreeLookupIndex {
|
||||
return { byId, repoHostIdsByRepoId }
|
||||
}
|
||||
|
||||
// Why: worktree/owner updates replace these snapshots, while weak ownership avoids retaining superseded state.
|
||||
const worktreeLookupIndexes = new WeakMap<
|
||||
AppState['worktreesByRepo'],
|
||||
{ repos: AppState['repos']; index: WorktreeLookupIndex }
|
||||
>()
|
||||
|
||||
function getWorktreeLookupIndex(state: AppState): WorktreeLookupIndex {
|
||||
const worktreesByRepo = state.worktreesByRepo ?? EMPTY_WORKTREES_BY_REPO
|
||||
const repos = state.repos ?? EMPTY_WORKTREE_REPOS
|
||||
const cached = worktreeLookupIndexes.get(worktreesByRepo)
|
||||
if (cached && cached.repos === repos) {
|
||||
return cached.index
|
||||
}
|
||||
const index = buildWorktreeLookupIndex(state)
|
||||
worktreeLookupIndexes.set(worktreesByRepo, { repos, index })
|
||||
return index
|
||||
}
|
||||
|
||||
function findUniqueWorktreeById(
|
||||
state: AppState,
|
||||
worktreeId: string,
|
||||
executionHostId?: string,
|
||||
lookupIndex = buildWorktreeLookupIndex(state)
|
||||
lookupIndex = getWorktreeLookupIndex(state)
|
||||
): Worktree | null {
|
||||
const match = lookupIndex.byId.get(worktreeId)?.unique ?? null
|
||||
// Why: metadata persistence is keyed only by worktree id; an id owned by two hosts is non-unique so destructive clears fail closed.
|
||||
@@ -1130,9 +1146,10 @@ function shouldApplyBranchMismatchedLinkedPRClear(args: {
|
||||
function buildPRRefreshCandidate(
|
||||
state: AppState,
|
||||
worktree: Worktree,
|
||||
repoPath?: string
|
||||
repoPath?: string,
|
||||
repoOverride?: Repo
|
||||
): GitHubPRRefreshCandidate | null {
|
||||
const repo = state.repos.find((r) => r.id === worktree.repoId)
|
||||
const repo = repoOverride ?? getGitHubRepoLookupIndex(state.repos).findById(worktree.repoId)
|
||||
if (!repo) {
|
||||
return null
|
||||
}
|
||||
@@ -2989,9 +3006,10 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
||||
},
|
||||
|
||||
fetchPRForBranch: async (repoPath, branch, options): Promise<PRInfo | null> => {
|
||||
const repo = get().repos?.find((candidate) =>
|
||||
options?.repoId ? candidate.id === options.repoId : candidate.path === repoPath
|
||||
)
|
||||
const repoLookup = getGitHubRepoLookupIndex(get().repos)
|
||||
const repo = options?.repoId
|
||||
? repoLookup.findById(options.repoId)
|
||||
: repoLookup.findByPath(repoPath)
|
||||
const repoId = options?.repoId ?? repo?.id
|
||||
const requestSettings = settingsForGitHubRepoOwner(get().settings, repo)
|
||||
const cacheKey = prCacheKey(
|
||||
@@ -4415,22 +4433,33 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
||||
|
||||
refreshAllGitHub: () => {
|
||||
// Clear comments cache; evict stale entries to bound long-session growth across repos/branches.
|
||||
set((s) => ({
|
||||
commentsCache: {},
|
||||
prCache: evictStaleEntries(s.prCache),
|
||||
issueCache: evictStaleEntries(s.issueCache),
|
||||
checksCache: evictStaleEntries(s.checksCache),
|
||||
workItemsCache: evictStaleEntries(s.workItemsCache),
|
||||
projectViewCache: evictStaleEntries(s.projectViewCache),
|
||||
prRefreshStates: pruneExpiredPRRefreshStates(s.prRefreshStates)
|
||||
}))
|
||||
set((s) => {
|
||||
const next = {
|
||||
commentsCache: Object.keys(s.commentsCache).length === 0 ? s.commentsCache : {},
|
||||
prCache: evictStaleEntries(s.prCache),
|
||||
issueCache: evictStaleEntries(s.issueCache),
|
||||
checksCache: evictStaleEntries(s.checksCache),
|
||||
workItemsCache: evictStaleEntries(s.workItemsCache),
|
||||
projectViewCache: evictStaleEntries(s.projectViewCache),
|
||||
prRefreshStates: pruneExpiredPRRefreshStates(s.prRefreshStates)
|
||||
}
|
||||
// Why: each eviction helper returns its input untouched when nothing changed, so an
|
||||
// unchanged sweep can return `s` and avoid waking every subscriber on window resume.
|
||||
return next.commentsCache === s.commentsCache &&
|
||||
next.prCache === s.prCache &&
|
||||
next.issueCache === s.issueCache &&
|
||||
next.checksCache === s.checksCache &&
|
||||
next.workItemsCache === s.workItemsCache &&
|
||||
next.projectViewCache === s.projectViewCache &&
|
||||
next.prRefreshStates === s.prRefreshStates
|
||||
? s
|
||||
: next
|
||||
})
|
||||
|
||||
// Why: don't prune prRequestGenerations here — deleting a live generation makes its response look stale.
|
||||
|
||||
// Only re-fetch PR/issue entries that are already stale — skip fresh ones
|
||||
const state = get()
|
||||
const now = Date.now()
|
||||
const stalePRCandidates: { candidate: GitHubPRRefreshCandidate; score: number }[] = []
|
||||
const cardProps = state.worktreeCardProperties ?? []
|
||||
const rawCardProps = cardProps as readonly string[]
|
||||
const shouldRefreshIssues = shouldRefreshIssueDecorations(state)
|
||||
@@ -4442,10 +4471,17 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
||||
(state.settings?.experimentalNewWorktreeCardStyle === true
|
||||
? cardProps.includes('status')
|
||||
: cardProps.includes('pr') || rawCardProps.includes('ci'))
|
||||
if (!shouldRefreshPRs && !shouldRefreshIssues) {
|
||||
return
|
||||
}
|
||||
|
||||
const now = Date.now()
|
||||
const stalePRCandidates: { candidate: GitHubPRRefreshCandidate; score: number }[] = []
|
||||
const repoLookup = getGitHubRepoLookupIndex(state.repos)
|
||||
|
||||
for (const worktrees of Object.values(state.worktreesByRepo)) {
|
||||
for (const wt of worktrees) {
|
||||
const repo = state.repos.find((r) => r.id === wt.repoId)
|
||||
const repo = repoLookup.findById(wt.repoId)
|
||||
if (!repo) {
|
||||
continue
|
||||
}
|
||||
@@ -4463,7 +4499,7 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
||||
)
|
||||
const prEntry = state.prCache[prKey]
|
||||
if (!prEntry || now - prEntry.fetchedAt >= CACHE_TTL) {
|
||||
const candidate = buildPRRefreshCandidate(state, wt)
|
||||
const candidate = buildPRRefreshCandidate(state, wt, undefined, repo)
|
||||
if (candidate) {
|
||||
stalePRCandidates.push({
|
||||
candidate,
|
||||
|
||||
Reference in New Issue
Block a user