mirror of
https://github.com/stablyai/orca.git
synced 2026-10-03 00:02:19 +00:00
perf(github-drawer): cache work-item details + collapse issue fetch (#1655)
Reopening a GitHub issue/PR drawer paid full IPC + `gh` startup latency on every open. Two changes here: 1. Module-level SWR cache in GitHubItemDialog.tsx keyed by (repoPath, issueSourcePreference, type, number). Reopening within 30s paints cached data instantly; older entries paint stale-then-refresh. Concurrent opens dedupe on a shared in-flight promise. Mutation handlers invalidate by (repo, type, number); a cache-generation counter prevents in-flight refetches from resurrecting stale data after a mid-flight invalidation. 2. Collapsed GraphQL query for issue details replaces 3 serial `gh` subprocesses (REST issue + REST comments + GraphQL participants) with one round-trip. Falls back to the legacy fan-out on any GraphQL error so historical contract is preserved. Cross-window invalidation rides a new `gh:workItemMutated` IPC broadcast that skips the originating sender (the source already updated its cache optimistically — re-broadcasting would race the optimistic write). `addIssueComment` now takes a `type` so the broadcast scopes correctly when a PR shares its number with an issue. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinjing
Orca
@@ -49,7 +49,7 @@ describe('getWorkItemDetails', () => {
|
||||
acquireMock.mockResolvedValue(undefined)
|
||||
})
|
||||
|
||||
it('passes row type into the lookup and fetches issue comments from the issue source', async () => {
|
||||
it('uses the collapsed GraphQL issue query as the hot path', async () => {
|
||||
getWorkItemMock.mockResolvedValueOnce({
|
||||
id: 'issue:923',
|
||||
type: 'issue',
|
||||
@@ -61,21 +61,84 @@ describe('getWorkItemDetails', () => {
|
||||
updatedAt: '2026-04-01T00:00:00Z',
|
||||
author: 'octocat'
|
||||
})
|
||||
getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
|
||||
ghExecFileAsyncMock
|
||||
.mockResolvedValueOnce({ stdout: JSON.stringify({ body: 'Issue body' }) })
|
||||
.mockResolvedValueOnce({ stdout: '[]' })
|
||||
getIssueOwnerRepoMock.mockResolvedValue({ owner: 'stablyai', repo: 'orca' })
|
||||
ghExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify({
|
||||
data: {
|
||||
repository: {
|
||||
issue: {
|
||||
body: 'Issue body',
|
||||
assignees: { nodes: [{ login: 'jinjing' }] },
|
||||
participants: {
|
||||
nodes: [{ login: 'octocat', avatarUrl: 'https://x/y', name: 'Octo Cat' }]
|
||||
},
|
||||
comments: {
|
||||
nodes: [
|
||||
{
|
||||
databaseId: 7,
|
||||
body: 'first',
|
||||
createdAt: '2026-04-01T00:00:00Z',
|
||||
url: 'https://github.com/stablyai/orca/issues/923#issuecomment-7',
|
||||
author: { login: 'octocat', avatarUrl: 'https://x/y' }
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
const details = await getWorkItemDetails('/repo-root', 923, 'issue')
|
||||
|
||||
expect(getWorkItemMock).toHaveBeenCalledWith('/repo-root', 923, 'issue')
|
||||
// Why: a single gh subprocess call replaces the previous REST + REST + GraphQL fan-out.
|
||||
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1)
|
||||
expect(ghExecFileAsyncMock.mock.calls[0][0][0]).toBe('api')
|
||||
expect(ghExecFileAsyncMock.mock.calls[0][0][1]).toBe('graphql')
|
||||
expect(details?.body).toBe('Issue body')
|
||||
expect(details?.assignees).toEqual(['jinjing'])
|
||||
expect(details?.comments).toHaveLength(1)
|
||||
expect(details?.comments[0].id).toBe(7)
|
||||
expect(details?.participants?.[0]?.login).toBe('octocat')
|
||||
})
|
||||
|
||||
it('falls back to REST + GraphQL when the collapsed issue query fails', async () => {
|
||||
getWorkItemMock.mockResolvedValueOnce({
|
||||
id: 'issue:923',
|
||||
type: 'issue',
|
||||
number: 923,
|
||||
title: 'Use upstream issues',
|
||||
state: 'open',
|
||||
url: 'https://github.com/stablyai/orca/issues/923',
|
||||
labels: [],
|
||||
updatedAt: '2026-04-01T00:00:00Z',
|
||||
author: 'octocat'
|
||||
})
|
||||
getIssueOwnerRepoMock.mockResolvedValue({ owner: 'stablyai', repo: 'orca' })
|
||||
// Collapsed GraphQL throws → fallback path picks up.
|
||||
ghExecFileAsyncMock
|
||||
.mockRejectedValueOnce(new Error('GraphQL error'))
|
||||
.mockResolvedValueOnce({ stdout: JSON.stringify({ body: 'Issue body' }) })
|
||||
.mockResolvedValueOnce({ stdout: '[]' })
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify({
|
||||
data: { repository: { issue: { participants: { nodes: [] } } } }
|
||||
})
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify({ data: {} })
|
||||
})
|
||||
|
||||
const details = await getWorkItemDetails('/repo-root', 923, 'issue')
|
||||
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
2,
|
||||
['api', '--cache', '60s', 'repos/stablyai/orca/issues/923'],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
3,
|
||||
['api', '--cache', '60s', 'repos/stablyai/orca/issues/923/comments?per_page=100'],
|
||||
{ cwd: '/repo-root' }
|
||||
)
|
||||
|
||||
@@ -32,6 +32,135 @@ const WORK_ITEM_PARTICIPANTS_QUERY = `query($owner: String!, $repo: String!, $nu
|
||||
}
|
||||
}`
|
||||
|
||||
// Why: a single GraphQL round-trip replaces three serial gh subprocesses on
|
||||
// the issue path (REST issue + REST comments + GraphQL participants). The
|
||||
// previous fan-out could spawn ~3 `gh` processes per drawer-open; this drops
|
||||
// it to one. We still fall back to the legacy REST+GraphQL path if the
|
||||
// collapsed query throws or returns missing data — see the strict-fallback
|
||||
// branch in getWorkItemDetails.
|
||||
const ISSUE_DETAILS_QUERY = `query($owner: String!, $repo: String!, $number: Int!) {
|
||||
repository(owner: $owner, name: $repo) {
|
||||
issue(number: $number) {
|
||||
body
|
||||
assignees(first: 50) { nodes { login } }
|
||||
participants(first: 100) {
|
||||
nodes { login avatarUrl(size: 48) ... on User { name } }
|
||||
}
|
||||
comments(first: 100) {
|
||||
nodes {
|
||||
databaseId
|
||||
body
|
||||
createdAt
|
||||
url
|
||||
author {
|
||||
login
|
||||
avatarUrl(size: 48)
|
||||
... on Bot { __typename }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}`
|
||||
|
||||
type GraphQLIssueDetailsResponse = {
|
||||
data?: {
|
||||
repository?: {
|
||||
issue?: {
|
||||
body?: string | null
|
||||
assignees?: { nodes?: { login?: string }[] }
|
||||
participants?: { nodes?: GitHubAssignableUser[] }
|
||||
comments?: {
|
||||
nodes?: {
|
||||
databaseId?: number | null
|
||||
body?: string | null
|
||||
createdAt?: string | null
|
||||
url?: string | null
|
||||
author?: {
|
||||
login?: string | null
|
||||
avatarUrl?: string | null
|
||||
__typename?: string
|
||||
} | null
|
||||
}[]
|
||||
}
|
||||
} | null
|
||||
} | null
|
||||
}
|
||||
errors?: { message?: string }[]
|
||||
}
|
||||
|
||||
async function getIssueDetailsViaGraphQL(
|
||||
repoPath: string,
|
||||
issueNumber: number
|
||||
): Promise<{
|
||||
body: string
|
||||
comments: PRComment[]
|
||||
assignees: string[]
|
||||
participants: GitHubAssignableUser[]
|
||||
} | null> {
|
||||
const ownerRepo = await getIssueOwnerRepo(repoPath)
|
||||
if (!ownerRepo) {
|
||||
return null
|
||||
}
|
||||
try {
|
||||
const { stdout } = await ghExecFileAsync(
|
||||
[
|
||||
'api',
|
||||
'graphql',
|
||||
'-f',
|
||||
`query=${ISSUE_DETAILS_QUERY}`,
|
||||
'-f',
|
||||
`owner=${ownerRepo.owner}`,
|
||||
'-f',
|
||||
`repo=${ownerRepo.repo}`,
|
||||
'-F',
|
||||
`number=${issueNumber}`
|
||||
],
|
||||
{ cwd: repoPath }
|
||||
)
|
||||
const parsed = JSON.parse(stdout) as GraphQLIssueDetailsResponse
|
||||
if (parsed.errors && parsed.errors.length > 0) {
|
||||
// Why: any partial GraphQL error (permissions, unknown field on a fork)
|
||||
// forces the strict REST fallback so the drawer never paints a half-built
|
||||
// shell. The fallback path's behavior is the historical contract.
|
||||
return null
|
||||
}
|
||||
const issue = parsed.data?.repository?.issue
|
||||
if (!issue) {
|
||||
return null
|
||||
}
|
||||
const comments: PRComment[] = (issue.comments?.nodes ?? [])
|
||||
.filter((c) => typeof c.databaseId === 'number')
|
||||
.map((c) => ({
|
||||
id: c.databaseId as number,
|
||||
author: c.author?.login ?? 'ghost',
|
||||
authorAvatarUrl: c.author?.avatarUrl ?? '',
|
||||
body: c.body ?? '',
|
||||
createdAt: c.createdAt ?? '',
|
||||
url: c.url ?? '',
|
||||
isBot: c.author?.__typename === 'Bot'
|
||||
}))
|
||||
const assignees = (issue.assignees?.nodes ?? [])
|
||||
.map((a) => a.login)
|
||||
.filter((login): login is string => Boolean(login))
|
||||
const participants: GitHubAssignableUser[] = (issue.participants?.nodes ?? [])
|
||||
.filter((u) => Boolean(u.login))
|
||||
.map((u) => ({
|
||||
login: u.login,
|
||||
name: u.name ?? null,
|
||||
avatarUrl: u.avatarUrl ?? ''
|
||||
}))
|
||||
return {
|
||||
body: issue.body ?? '',
|
||||
comments,
|
||||
assignees,
|
||||
participants
|
||||
}
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
function mergeGitHubUsers(users: GitHubAssignableUser[]): GitHubAssignableUser[] {
|
||||
const byLogin = new Map<string, GitHubAssignableUser>()
|
||||
for (const user of users) {
|
||||
@@ -404,8 +533,25 @@ export async function getWorkItemDetails(
|
||||
await acquire()
|
||||
try {
|
||||
if (item.type === 'issue') {
|
||||
// Why: fetch body/comments and GraphQL participants in parallel; the
|
||||
// mention-participant merge is a cheap local operation afterward.
|
||||
// Why: try the collapsed single-GraphQL path first — body, assignees,
|
||||
// participants, and comments all return in one round-trip. On any
|
||||
// failure (permissions, partial errors, non-GitHub remote), strictly
|
||||
// fall back to the legacy REST+GraphQL fan-out so historical behavior
|
||||
// is preserved. The GraphQL `participants` connection includes every
|
||||
// commenter, so we skip the extra `getMentionParticipants` aliased
|
||||
// user-hydration trip when the collapsed path succeeds.
|
||||
const collapsed = await getIssueDetailsViaGraphQL(repoPath, item.number)
|
||||
if (collapsed) {
|
||||
return {
|
||||
item,
|
||||
body: collapsed.body,
|
||||
comments: collapsed.comments,
|
||||
assignees: collapsed.assignees,
|
||||
participants: collapsed.participants
|
||||
}
|
||||
}
|
||||
// Why: fall back to body/comments and GraphQL participants in parallel;
|
||||
// the mention-participant merge is a cheap local operation afterward.
|
||||
const [{ body, comments, assignees }, participants] = await Promise.all([
|
||||
getIssueBodyAndComments(repoPath, item.number),
|
||||
getWorkItemParticipants(repoPath, item)
|
||||
|
||||
Reference in New Issue
Block a user