Skip to content

Commit 431b1f5

Browse files
skarimCopilot
andcommitted
Optimize view/modify TUI load time with parallel fetching
- Deduplicate API calls: syncStackPRs now returns PRDetails for LoadBranchNodes to reuse, eliminating redundant FindPRDetailsForBranch calls - Parallelize API calls in syncStackPRs (capped at 6 concurrent requests) - Parallelize git operations in LoadBranchNodes (capped at 4 concurrent) - Show "Loading stack..." indicator for interactive sessions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 11ab8c6 commit 431b1f5

5 files changed

Lines changed: 324 additions & 119 deletions

File tree

cmd/modify.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func runModify(cfg *config.Config) error {
6464
currentBranch := result.CurrentBranch
6565

6666
// Load branch data for the TUI
67-
viewNodes := stackview.LoadBranchNodes(cfg, s, currentBranch)
67+
viewNodes := stackview.LoadBranchNodes(cfg, s, currentBranch, result.PRDetails)
6868

6969
// Reverse so index 0 = top of stack (matching visual order)
7070
reversed := make([]stackview.BranchNode, len(viewNodes))
@@ -283,8 +283,15 @@ func checkModifyPreconditions(cfg *config.Config) (*loadStackResult, error) {
283283
return nil, ErrSilent
284284
}
285285

286+
// Show loading indicator while syncing PRs
287+
fmt.Fprintf(cfg.Err, "Loading stack...")
288+
286289
// Sync PR state and check merge queue
287-
syncStackPRs(cfg, s)
290+
prDetails := syncStackPRs(cfg, s)
291+
result.PRDetails = prDetails
292+
293+
fmt.Fprintf(cfg.Err, "\r\033[2K")
294+
288295
if err := modify.CheckNoMergeQueuePRs(cfg, s); err != nil {
289296
return nil, ErrSilent
290297
}

cmd/utils.go

Lines changed: 203 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"net/url"
77
"strconv"
88
"strings"
9+
"sync"
910

1011
"github.com/AlecAivazis/survey/v2/terminal"
1112
"github.com/cli/go-gh/v2/pkg/prompter"
@@ -21,13 +22,13 @@ var ErrSilent = &ExitError{Code: 1}
2122

2223
// Typed exit errors for programmatic detection by scripts and agents.
2324
var (
24-
ErrNotInStack = &ExitError{Code: 2} // branch/stack not found
25-
ErrConflict = &ExitError{Code: 3} // rebase conflict
26-
ErrAPIFailure = &ExitError{Code: 4} // GitHub API error
27-
ErrInvalidArgs = &ExitError{Code: 5} // invalid arguments or flags
28-
ErrDisambiguate = &ExitError{Code: 6} // multiple stacks/remotes, can't auto-select
29-
ErrRebaseActive = &ExitError{Code: 7} // rebase already in progress
30-
ErrLockFailed = &ExitError{Code: 8} // could not acquire stack file lock
25+
ErrNotInStack = &ExitError{Code: 2} // branch/stack not found
26+
ErrConflict = &ExitError{Code: 3} // rebase conflict
27+
ErrAPIFailure = &ExitError{Code: 4} // GitHub API error
28+
ErrInvalidArgs = &ExitError{Code: 5} // invalid arguments or flags
29+
ErrDisambiguate = &ExitError{Code: 6} // multiple stacks/remotes, can't auto-select
30+
ErrRebaseActive = &ExitError{Code: 7} // rebase already in progress
31+
ErrLockFailed = &ExitError{Code: 8} // could not acquire stack file lock
3132
ErrStacksUnavailable = &ExitError{Code: 9} // stacked PRs not available for this repository
3233
ErrModifyRecovery = &ExitError{Code: 10} // modify session interrupted, recovery required
3334
)
@@ -91,6 +92,7 @@ type loadStackResult struct {
9192
StackFile *stack.StackFile
9293
Stack *stack.Stack
9394
CurrentBranch string
95+
PRDetails map[string]*github.PRDetails
9496
}
9597

9698
// loadStack is the standard way to obtain a Stack for the current (or given)
@@ -233,6 +235,8 @@ func resolveStack(sf *stack.StackFile, branch string, cfg *config.Config) (*stac
233235
}
234236

235237
// syncStackPRs discovers and updates pull request metadata for branches in a stack.
238+
// It also collects PRDetails for each branch, returned as a map keyed by branch name.
239+
// The returned map is consumed by LoadBranchNodes to avoid redundant API calls.
236240
//
237241
// When the stack has a remote ID, the stack API is the source of truth: the
238242
// authoritative PR list is fetched from the server and matched to local
@@ -246,82 +250,191 @@ func resolveStack(sf *stack.StackFile, branch string, cfg *config.Config) (*stac
246250
// 3. Tracked PR (merged) — skip; the merged state is final.
247251
//
248252
// The transient Queued flag is also populated from the API response.
249-
func syncStackPRs(cfg *config.Config, s *stack.Stack) {
253+
//
254+
// API calls for different branches are made concurrently to reduce latency.
255+
func syncStackPRs(cfg *config.Config, s *stack.Stack) map[string]*github.PRDetails {
250256
client, err := cfg.GitHubClient()
251257
if err != nil {
252-
return
258+
return nil
253259
}
254260

255261
// When the stack has a remote ID, the stack API is the source of truth.
256262
if s.ID != "" {
257-
if syncStackPRsFromRemote(client, s) {
258-
return
263+
if details, ok := syncStackPRsFromRemote(client, s); ok {
264+
return details
259265
}
260266
}
261267

262268
// No remote stack (or remote sync failed) — local discovery.
269+
// Each branch is processed concurrently; results are collected and applied sequentially.
270+
type branchResult struct {
271+
index int
272+
pullRequest *stack.PullRequestRef
273+
queued bool
274+
details *github.PRDetails
275+
skip bool // true means keep existing data, don't update
276+
}
277+
278+
results := make([]branchResult, len(s.Branches))
279+
280+
// Fetch PR data for all branches concurrently using a WaitGroup for
281+
// completion and a semaphore channel to cap the number of in-flight
282+
// API requests (see maxAPIConcurrency).
283+
var wg sync.WaitGroup
284+
sem := make(chan struct{}, maxAPIConcurrency)
285+
263286
for i := range s.Branches {
264-
b := &s.Branches[i]
287+
b := s.Branches[i]
265288

266289
if b.IsMerged() {
290+
results[i] = branchResult{index: i, skip: true}
291+
// Provide PRDetails for merged branches from existing tracked PR
292+
if b.PullRequest != nil && b.PullRequest.Number != 0 {
293+
results[i].details = &github.PRDetails{
294+
Number: b.PullRequest.Number,
295+
State: "MERGED",
296+
URL: b.PullRequest.URL,
297+
Merged: true,
298+
}
299+
}
267300
continue
268301
}
269302

270-
if b.PullRequest != nil && b.PullRequest.Number != 0 {
271-
// Tracked PR — refresh its state.
272-
pr, err := client.FindPRByNumber(b.PullRequest.Number)
273-
if err != nil {
274-
continue // API error — keep existing tracked PR
275-
}
276-
if pr == nil {
277-
// PR not found — clear stale ref and fall through
278-
// to the open-PR lookup below.
279-
b.PullRequest = nil
280-
b.Queued = false
281-
} else {
282-
b.PullRequest = &stack.PullRequestRef{
283-
Number: pr.Number,
284-
ID: pr.ID,
285-
URL: pr.URL,
286-
Merged: pr.Merged,
303+
wg.Add(1)
304+
go func(idx int, branch stack.BranchRef) {
305+
defer wg.Done()
306+
307+
// Acquire a semaphore slot to limit concurrent API calls.
308+
sem <- struct{}{}
309+
defer func() { <-sem }()
310+
311+
res := branchResult{index: idx}
312+
313+
trackedResolved := false
314+
if branch.PullRequest != nil && branch.PullRequest.Number != 0 {
315+
// Tracked PR — refresh its state.
316+
pr, err := client.FindPRByNumber(branch.PullRequest.Number)
317+
if err != nil {
318+
// API error — keep existing tracked PR
319+
res.skip = true
320+
res.details = prDetailsFromTracked(branch.PullRequest)
321+
results[idx] = res
322+
return
287323
}
288-
b.Queued = pr.IsQueued()
289-
290-
// If the PR was closed (not merged), remove the association
291-
// so we fall through to the open-PR lookup below.
292-
if pr.State == "CLOSED" {
293-
b.PullRequest = nil
294-
b.Queued = false
295-
} else {
296-
continue
324+
if pr != nil && pr.State != "CLOSED" {
325+
// PR is open or merged — keep it
326+
res.pullRequest = &stack.PullRequestRef{
327+
Number: pr.Number,
328+
ID: pr.ID,
329+
URL: pr.URL,
330+
Merged: pr.Merged,
331+
}
332+
res.queued = pr.IsQueued()
333+
res.details = prDetailsFromPR(pr)
334+
results[idx] = res
335+
trackedResolved = true
297336
}
337+
// Otherwise PR not found or closed — fall through to open-PR lookup
298338
}
299-
}
300339

301-
// No tracked PR (or just cleared) — only adopt OPEN PRs to avoid
302-
// picking up stale merged/closed PRs from a previous use of this
303-
// branch name.
304-
pr, err := client.FindPRForBranch(b.Branch)
305-
if err != nil || pr == nil {
340+
if trackedResolved {
341+
return
342+
}
343+
344+
// No tracked PR (or cleared) — only adopt OPEN PRs.
345+
pr, err := client.FindPRForBranch(branch.Branch)
346+
if err != nil || pr == nil {
347+
results[idx] = res
348+
return
349+
}
350+
res.pullRequest = &stack.PullRequestRef{
351+
Number: pr.Number,
352+
ID: pr.ID,
353+
URL: pr.URL,
354+
}
355+
res.queued = pr.IsQueued()
356+
// FindPRForBranch only returns OPEN PRs
357+
res.details = &github.PRDetails{
358+
Number: pr.Number,
359+
State: "OPEN",
360+
URL: pr.URL,
361+
IsDraft: pr.IsDraft,
362+
Merged: false,
363+
IsQueued: pr.IsQueued(),
364+
}
365+
results[idx] = res
366+
}(i, b)
367+
}
368+
wg.Wait()
369+
370+
// Apply results sequentially to preserve deterministic behavior.
371+
details := make(map[string]*github.PRDetails)
372+
for _, res := range results {
373+
if res.details != nil {
374+
details[s.Branches[res.index].Branch] = res.details
375+
}
376+
if res.skip {
306377
continue
307378
}
308-
b.PullRequest = &stack.PullRequestRef{
309-
Number: pr.Number,
310-
ID: pr.ID,
311-
URL: pr.URL,
379+
b := &s.Branches[res.index]
380+
if res.pullRequest != nil {
381+
b.PullRequest = res.pullRequest
382+
b.Queued = res.queued
383+
} else if !b.IsMerged() {
384+
// Clear if we didn't find anything (and original was cleared during discovery)
385+
if b.PullRequest != nil && res.pullRequest == nil {
386+
b.PullRequest = nil
387+
b.Queued = false
388+
}
312389
}
313-
b.Queued = pr.IsQueued()
390+
}
391+
392+
return details
393+
}
394+
395+
// maxAPIConcurrency limits the number of concurrent API calls to avoid hitting secondary rate limits.
396+
const maxAPIConcurrency = 6
397+
398+
// prDetailsFromPR builds PRDetails from a PullRequest returned by FindPRByNumber.
399+
func prDetailsFromPR(pr *github.PullRequest) *github.PRDetails {
400+
if pr == nil {
401+
return nil
402+
}
403+
return &github.PRDetails{
404+
Number: pr.Number,
405+
State: pr.State,
406+
URL: pr.URL,
407+
IsDraft: pr.IsDraft,
408+
Merged: pr.Merged,
409+
IsQueued: pr.IsQueued(),
410+
}
411+
}
412+
413+
// prDetailsFromTracked builds minimal PRDetails from a tracked PullRequestRef.
414+
func prDetailsFromTracked(ref *stack.PullRequestRef) *github.PRDetails {
415+
if ref == nil {
416+
return nil
417+
}
418+
state := "OPEN"
419+
if ref.Merged {
420+
state = "MERGED"
421+
}
422+
return &github.PRDetails{
423+
Number: ref.Number,
424+
State: state,
425+
URL: ref.URL,
426+
Merged: ref.Merged,
314427
}
315428
}
316429

317430
// syncStackPRsFromRemote uses the stack API to sync PR state. The remote
318431
// stack's PR list is the source of truth — PRs stay associated even if
319-
// closed. Returns true if the sync succeeded, false if we should fall
320-
// back to local discovery (e.g. stack not found remotely, API error).
321-
func syncStackPRsFromRemote(client github.ClientOps, s *stack.Stack) bool {
432+
// closed. Returns the PRDetails map and true if sync succeeded, or nil and
433+
// false if we should fall back to local discovery.
434+
func syncStackPRsFromRemote(client github.ClientOps, s *stack.Stack) (map[string]*github.PRDetails, bool) {
322435
stacks, err := client.ListStacks()
323436
if err != nil {
324-
return false
437+
return nil, false
325438
}
326439

327440
// Find our stack in the remote list.
@@ -333,20 +446,47 @@ func syncStackPRsFromRemote(client github.ClientOps, s *stack.Stack) bool {
333446
}
334447
}
335448
if remotePRNumbers == nil {
336-
return false
449+
return nil, false
337450
}
338451

339-
// Fetch each remote PR's details and index by head branch name.
452+
// Fetch each remote PR concurrently. Results are written to an ordered
453+
// slice (one slot per PR number) so that when we build the branch map
454+
// below, later entries win on duplicate HeadRefNames — matching the
455+
// sequential behavior of the old code.
456+
prResults := make([]*github.PullRequest, len(remotePRNumbers))
457+
458+
var wg sync.WaitGroup
459+
sem := make(chan struct{}, maxAPIConcurrency) // limits concurrent API calls
460+
461+
for i, num := range remotePRNumbers {
462+
wg.Add(1)
463+
go func(idx, prNum int) {
464+
defer wg.Done()
465+
466+
// Acquire a semaphore slot to limit concurrent API calls.
467+
sem <- struct{}{}
468+
defer func() { <-sem }()
469+
470+
pr, err := client.FindPRByNumber(prNum)
471+
if err != nil || pr == nil {
472+
return
473+
}
474+
// Each goroutine writes to its own index — no lock needed.
475+
prResults[idx] = pr
476+
}(i, num)
477+
}
478+
wg.Wait()
479+
480+
// Build map sequentially to preserve order semantics.
340481
prByBranch := make(map[string]*github.PullRequest, len(remotePRNumbers))
341-
for _, num := range remotePRNumbers {
342-
pr, err := client.FindPRByNumber(num)
343-
if err != nil || pr == nil {
344-
continue
482+
for _, pr := range prResults {
483+
if pr != nil {
484+
prByBranch[pr.HeadRefName] = pr
345485
}
346-
prByBranch[pr.HeadRefName] = pr
347486
}
348487

349-
// Match remote PRs to local branches.
488+
// Match remote PRs to local branches and collect PRDetails.
489+
details := make(map[string]*github.PRDetails)
350490
for i := range s.Branches {
351491
b := &s.Branches[i]
352492
pr, ok := prByBranch[b.Branch]
@@ -360,9 +500,10 @@ func syncStackPRsFromRemote(client github.ClientOps, s *stack.Stack) bool {
360500
Merged: pr.Merged,
361501
}
362502
b.Queued = pr.IsQueued()
503+
details[b.Branch] = prDetailsFromPR(pr)
363504
}
364505

365-
return true
506+
return details, true
366507
}
367508

368509
// updateBaseSHAs refreshes the Base and Head SHAs for all active branches

0 commit comments

Comments
 (0)