From 58761b6542fc776b2f548bcbb68d3f0ff8676ae9 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Tue, 5 May 2026 20:50:47 -0400 Subject: [PATCH] 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> --- cmd/init.go | 2 +- cmd/merge.go | 2 +- cmd/modify.go | 11 +- cmd/push.go | 2 +- cmd/rebase.go | 6 +- cmd/submit.go | 4 +- cmd/sync.go | 4 +- cmd/utils.go | 265 +++++++++++++++++++++------- cmd/utils_test.go | 20 +-- cmd/view.go | 23 ++- internal/tui/stackview/data.go | 134 +++++++++----- internal/tui/stackview/data_test.go | 10 +- 12 files changed, 344 insertions(+), 139 deletions(-) diff --git a/cmd/init.go b/cmd/init.go index f625814..3db867a 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -320,7 +320,7 @@ func runInit(cfg *config.Config, opts *initOptions) error { } } } else { - syncStackPRs(cfg, latestStack) + _ = syncStackPRs(cfg, latestStack) } if err := stack.Save(gitDir, sf); err != nil { diff --git a/cmd/merge.go b/cmd/merge.go index 8a298e2..dc314ce 100644 --- a/cmd/merge.go +++ b/cmd/merge.go @@ -41,7 +41,7 @@ func runMerge(cfg *config.Config, target string) error { currentBranch := result.CurrentBranch // Sync PR state from GitHub so merge status is up to date. - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) // Persist the refreshed PR state. stack.SaveNonBlocking(result.GitDir, result.StackFile) diff --git a/cmd/modify.go b/cmd/modify.go index 143bffb..6c25c96 100644 --- a/cmd/modify.go +++ b/cmd/modify.go @@ -64,7 +64,7 @@ func runModify(cfg *config.Config) error { currentBranch := result.CurrentBranch // Load branch data for the TUI - viewNodes := stackview.LoadBranchNodes(cfg, s, currentBranch) + viewNodes := stackview.LoadBranchNodes(cfg, s, currentBranch, result.PRDetails) // Reverse so index 0 = top of stack (matching visual order) reversed := make([]stackview.BranchNode, len(viewNodes)) @@ -283,8 +283,15 @@ func checkModifyPreconditions(cfg *config.Config) (*loadStackResult, error) { return nil, ErrSilent } + // Show loading indicator while syncing PRs + fmt.Fprintf(cfg.Err, "Loading stack...") + // Sync PR state and check merge queue - syncStackPRs(cfg, s) + prDetails := syncStackPRs(cfg, s) + result.PRDetails = prDetails + + fmt.Fprintf(cfg.Err, "\r\033[2K") + if err := modify.CheckNoMergeQueuePRs(cfg, s); err != nil { return nil, ErrSilent } diff --git a/cmd/push.go b/cmd/push.go index a0b809b..40c63e7 100644 --- a/cmd/push.go +++ b/cmd/push.go @@ -78,7 +78,7 @@ func runPush(cfg *config.Config, opts *pushOptions) error { return ErrSilent } // Sync PR state to detect merged/queued PRs before pushing. - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) merged := s.MergedBranches() if len(merged) > 0 { diff --git a/cmd/rebase.go b/cmd/rebase.go index cfed5af..4e16158 100644 --- a/cmd/rebase.go +++ b/cmd/rebase.go @@ -176,7 +176,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error { branchesToRebase[0].Branch, branchesToRebase[len(branchesToRebase)-1].Branch) // Sync PR state before rebase so we can detect merged PRs. - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) branchNames := make([]string, 0, len(s.Branches)) for _, b := range s.Branches { @@ -349,7 +349,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error { updateBaseSHAs(s) - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) stack.SaveNonBlocking(gitDir, sf) @@ -540,7 +540,7 @@ func continueRebase(cfg *config.Config, gitDir string) error { updateBaseSHAs(s) - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) stack.SaveNonBlocking(gitDir, sf) diff --git a/cmd/submit.go b/cmd/submit.go index 18849f2..3c8397e 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -108,7 +108,7 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error { } // Sync PR state to detect merged/queued PRs before pushing. - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) // Resolve remote for pushing remote, err := pickRemote(cfg, currentBranch, opts.remote) @@ -189,7 +189,7 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error { // Update base commit hashes and sync PR state updateBaseSHAs(s) - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) if err := stack.Save(gitDir, sf); err != nil { return handleSaveError(cfg, err) diff --git a/cmd/sync.go b/cmd/sync.go index 866105c..b003114 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -134,7 +134,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error { cfg.Printf("Rebasing stack ...") // Sync PR state to detect merged PRs before rebasing. - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) // Save original refs so we can restore on conflict. // Merged branches that no longer exist locally have no ref to @@ -311,7 +311,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error { // --- Step 5: Sync PR state --- cfg.Printf("") cfg.Printf("Syncing PRs ...") - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) // Report PR status for each branch for _, b := range s.Branches { diff --git a/cmd/utils.go b/cmd/utils.go index 09e95e5..7ec0d3f 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -6,6 +6,7 @@ import ( "net/url" "strconv" "strings" + "sync" "github.com/AlecAivazis/survey/v2/terminal" "github.com/cli/go-gh/v2/pkg/prompter" @@ -21,13 +22,13 @@ var ErrSilent = &ExitError{Code: 1} // Typed exit errors for programmatic detection by scripts and agents. var ( - ErrNotInStack = &ExitError{Code: 2} // branch/stack not found - ErrConflict = &ExitError{Code: 3} // rebase conflict - ErrAPIFailure = &ExitError{Code: 4} // GitHub API error - ErrInvalidArgs = &ExitError{Code: 5} // invalid arguments or flags - ErrDisambiguate = &ExitError{Code: 6} // multiple stacks/remotes, can't auto-select - ErrRebaseActive = &ExitError{Code: 7} // rebase already in progress - ErrLockFailed = &ExitError{Code: 8} // could not acquire stack file lock + ErrNotInStack = &ExitError{Code: 2} // branch/stack not found + ErrConflict = &ExitError{Code: 3} // rebase conflict + ErrAPIFailure = &ExitError{Code: 4} // GitHub API error + ErrInvalidArgs = &ExitError{Code: 5} // invalid arguments or flags + ErrDisambiguate = &ExitError{Code: 6} // multiple stacks/remotes, can't auto-select + ErrRebaseActive = &ExitError{Code: 7} // rebase already in progress + ErrLockFailed = &ExitError{Code: 8} // could not acquire stack file lock ErrStacksUnavailable = &ExitError{Code: 9} // stacked PRs not available for this repository ErrModifyRecovery = &ExitError{Code: 10} // modify session interrupted, recovery required ) @@ -91,6 +92,7 @@ type loadStackResult struct { StackFile *stack.StackFile Stack *stack.Stack CurrentBranch string + PRDetails map[string]*github.PRDetails } // 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 } // syncStackPRs discovers and updates pull request metadata for branches in a stack. +// It also collects PRDetails for each branch, returned as a map keyed by branch name. +// The returned map is consumed by LoadBranchNodes to avoid redundant API calls. // // When the stack has a remote ID, the stack API is the source of truth: the // 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 // 3. Tracked PR (merged) — skip; the merged state is final. // // The transient Queued flag is also populated from the API response. -func syncStackPRs(cfg *config.Config, s *stack.Stack) { +// +// API calls for different branches are made concurrently to reduce latency. +func syncStackPRs(cfg *config.Config, s *stack.Stack) map[string]*github.PRDetails { client, err := cfg.GitHubClient() if err != nil { - return + return nil } // When the stack has a remote ID, the stack API is the source of truth. if s.ID != "" { - if syncStackPRsFromRemote(client, s) { - return + if details, ok := syncStackPRsFromRemote(client, s); ok { + return details } } // No remote stack (or remote sync failed) — local discovery. + // Each branch is processed concurrently; results are collected and applied sequentially. + type branchResult struct { + index int + pullRequest *stack.PullRequestRef + queued bool + details *github.PRDetails + skip bool // true means keep existing data, don't update + } + + results := make([]branchResult, len(s.Branches)) + + // Fetch PR data for all branches concurrently using a WaitGroup for + // completion and a semaphore channel to cap the number of in-flight + // API requests (see maxAPIConcurrency). + var wg sync.WaitGroup + sem := make(chan struct{}, maxAPIConcurrency) + for i := range s.Branches { - b := &s.Branches[i] + b := s.Branches[i] if b.IsMerged() { + results[i] = branchResult{index: i, skip: true} + // Provide PRDetails for merged branches from existing tracked PR + if b.PullRequest != nil && b.PullRequest.Number != 0 { + results[i].details = &github.PRDetails{ + Number: b.PullRequest.Number, + State: "MERGED", + URL: b.PullRequest.URL, + Merged: true, + } + } continue } - if b.PullRequest != nil && b.PullRequest.Number != 0 { - // Tracked PR — refresh its state. - pr, err := client.FindPRByNumber(b.PullRequest.Number) - if err != nil { - continue // API error — keep existing tracked PR - } - if pr == nil { - // PR not found — clear stale ref and fall through - // to the open-PR lookup below. - b.PullRequest = nil - b.Queued = false - } else { - b.PullRequest = &stack.PullRequestRef{ - Number: pr.Number, - ID: pr.ID, - URL: pr.URL, - Merged: pr.Merged, + wg.Add(1) + go func(idx int, branch stack.BranchRef) { + defer wg.Done() + + // Acquire a semaphore slot to limit concurrent API calls. + sem <- struct{}{} + defer func() { <-sem }() + + res := branchResult{index: idx} + + trackedResolved := false + if branch.PullRequest != nil && branch.PullRequest.Number != 0 { + // Tracked PR — refresh its state. + pr, err := client.FindPRByNumber(branch.PullRequest.Number) + if err != nil { + // API error — keep existing tracked PR + res.skip = true + res.details = prDetailsFromTracked(branch.PullRequest) + results[idx] = res + return } - b.Queued = pr.IsQueued() - - // If the PR was closed (not merged), remove the association - // so we fall through to the open-PR lookup below. - if pr.State == "CLOSED" { - b.PullRequest = nil - b.Queued = false - } else { - continue + if pr != nil && pr.State != "CLOSED" { + // PR is open or merged — keep it + res.pullRequest = &stack.PullRequestRef{ + Number: pr.Number, + ID: pr.ID, + URL: pr.URL, + Merged: pr.Merged, + } + res.queued = pr.IsQueued() + res.details = prDetailsFromPR(pr) + results[idx] = res + trackedResolved = true } + // Otherwise PR not found or closed — fall through to open-PR lookup } - } - // No tracked PR (or just cleared) — only adopt OPEN PRs to avoid - // picking up stale merged/closed PRs from a previous use of this - // branch name. - pr, err := client.FindPRForBranch(b.Branch) - if err != nil || pr == nil { + if trackedResolved { + return + } + + // No tracked PR (or cleared) — only adopt OPEN PRs. + pr, err := client.FindPRForBranch(branch.Branch) + if err != nil || pr == nil { + results[idx] = res + return + } + res.pullRequest = &stack.PullRequestRef{ + Number: pr.Number, + ID: pr.ID, + URL: pr.URL, + } + res.queued = pr.IsQueued() + // FindPRForBranch only returns OPEN PRs + res.details = &github.PRDetails{ + Number: pr.Number, + State: "OPEN", + URL: pr.URL, + IsDraft: pr.IsDraft, + Merged: false, + IsQueued: pr.IsQueued(), + } + results[idx] = res + }(i, b) + } + wg.Wait() + + // Apply results sequentially to preserve deterministic behavior. + details := make(map[string]*github.PRDetails) + for _, res := range results { + if res.details != nil { + details[s.Branches[res.index].Branch] = res.details + } + if res.skip { continue } - b.PullRequest = &stack.PullRequestRef{ - Number: pr.Number, - ID: pr.ID, - URL: pr.URL, + b := &s.Branches[res.index] + if res.pullRequest != nil { + b.PullRequest = res.pullRequest + b.Queued = res.queued + } else if !b.IsMerged() { + // Clear if we didn't find anything (and original was cleared during discovery) + if b.PullRequest != nil && res.pullRequest == nil { + b.PullRequest = nil + b.Queued = false + } } - b.Queued = pr.IsQueued() + } + + return details +} + +// maxAPIConcurrency limits the number of concurrent API calls to avoid hitting secondary rate limits. +const maxAPIConcurrency = 6 + +// prDetailsFromPR builds PRDetails from a PullRequest returned by FindPRByNumber. +func prDetailsFromPR(pr *github.PullRequest) *github.PRDetails { + if pr == nil { + return nil + } + return &github.PRDetails{ + Number: pr.Number, + State: pr.State, + URL: pr.URL, + IsDraft: pr.IsDraft, + Merged: pr.Merged, + IsQueued: pr.IsQueued(), + } +} + +// prDetailsFromTracked builds minimal PRDetails from a tracked PullRequestRef. +func prDetailsFromTracked(ref *stack.PullRequestRef) *github.PRDetails { + if ref == nil { + return nil + } + state := "OPEN" + if ref.Merged { + state = "MERGED" + } + return &github.PRDetails{ + Number: ref.Number, + State: state, + URL: ref.URL, + Merged: ref.Merged, } } // syncStackPRsFromRemote uses the stack API to sync PR state. The remote // stack's PR list is the source of truth — PRs stay associated even if -// closed. Returns true if the sync succeeded, false if we should fall -// back to local discovery (e.g. stack not found remotely, API error). -func syncStackPRsFromRemote(client github.ClientOps, s *stack.Stack) bool { +// closed. Returns the PRDetails map and true if sync succeeded, or nil and +// false if we should fall back to local discovery. +func syncStackPRsFromRemote(client github.ClientOps, s *stack.Stack) (map[string]*github.PRDetails, bool) { stacks, err := client.ListStacks() if err != nil { - return false + return nil, false } // Find our stack in the remote list. @@ -333,20 +446,47 @@ func syncStackPRsFromRemote(client github.ClientOps, s *stack.Stack) bool { } } if remotePRNumbers == nil { - return false + return nil, false } - // Fetch each remote PR's details and index by head branch name. + // Fetch each remote PR concurrently. Results are written to an ordered + // slice (one slot per PR number) so that when we build the branch map + // below, later entries win on duplicate HeadRefNames — matching the + // sequential behavior of the old code. + prResults := make([]*github.PullRequest, len(remotePRNumbers)) + + var wg sync.WaitGroup + sem := make(chan struct{}, maxAPIConcurrency) // limits concurrent API calls + + for i, num := range remotePRNumbers { + wg.Add(1) + go func(idx, prNum int) { + defer wg.Done() + + // Acquire a semaphore slot to limit concurrent API calls. + sem <- struct{}{} + defer func() { <-sem }() + + pr, err := client.FindPRByNumber(prNum) + if err != nil || pr == nil { + return + } + // Each goroutine writes to its own index — no lock needed. + prResults[idx] = pr + }(i, num) + } + wg.Wait() + + // Build map sequentially to preserve order semantics. prByBranch := make(map[string]*github.PullRequest, len(remotePRNumbers)) - for _, num := range remotePRNumbers { - pr, err := client.FindPRByNumber(num) - if err != nil || pr == nil { - continue + for _, pr := range prResults { + if pr != nil { + prByBranch[pr.HeadRefName] = pr } - prByBranch[pr.HeadRefName] = pr } - // Match remote PRs to local branches. + // Match remote PRs to local branches and collect PRDetails. + details := make(map[string]*github.PRDetails) for i := range s.Branches { b := &s.Branches[i] pr, ok := prByBranch[b.Branch] @@ -360,9 +500,10 @@ func syncStackPRsFromRemote(client github.ClientOps, s *stack.Stack) bool { Merged: pr.Merged, } b.Queued = pr.IsQueued() + details[b.Branch] = prDetailsFromPR(pr) } - return true + return details, true } // updateBaseSHAs refreshes the Base and Head SHAs for all active branches diff --git a/cmd/utils_test.go b/cmd/utils_test.go index 2f9c7b4..59d7a28 100644 --- a/cmd/utils_test.go +++ b/cmd/utils_test.go @@ -264,7 +264,7 @@ func TestSyncStackPRs_NoTrackedPR_OnlyAdoptsOpenPRs(t *testing.T) { }, } - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) collectOutput(cfg, outR, errR) // Branch should still have no PR tracked. @@ -292,7 +292,7 @@ func TestSyncStackPRs_NoTrackedPR_AdoptsOpenPR(t *testing.T) { }, } - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) collectOutput(cfg, outR, errR) require.NotNil(t, s.Branches[0].PullRequest) @@ -329,7 +329,7 @@ func TestSyncStackPRs_TrackedPR_DetectsMerge(t *testing.T) { }, } - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) collectOutput(cfg, outR, errR) require.NotNil(t, s.Branches[0].PullRequest) @@ -367,7 +367,7 @@ func TestSyncStackPRs_MergedBranch_StaysMerged(t *testing.T) { }, } - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) collectOutput(cfg, outR, errR) require.NotNil(t, s.Branches[0].PullRequest) @@ -412,7 +412,7 @@ func TestSyncStackPRs_ClosedPR_ReplacedByOpenPR(t *testing.T) { }, } - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) collectOutput(cfg, outR, errR) require.NotNil(t, s.Branches[0].PullRequest) @@ -449,7 +449,7 @@ func TestSyncStackPRs_TrackedOpenPR_UpdatesQueued(t *testing.T) { }, } - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) collectOutput(cfg, outR, errR) assert.True(t, s.Branches[0].Queued) @@ -487,7 +487,7 @@ func TestSyncStackPRs_ClosedPR_NoReplacement_ClearsPR(t *testing.T) { }, } - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) collectOutput(cfg, outR, errR) assert.Nil(t, s.Branches[0].PullRequest) @@ -524,7 +524,7 @@ func TestSyncStackPRs_RemoteStack_UsesStackAPI(t *testing.T) { }, } - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) collectOutput(cfg, outR, errR) // b1 should be tracked with open PR @@ -561,7 +561,7 @@ func TestSyncStackPRs_RemoteStack_ClosedPRStaysAssociated(t *testing.T) { }, } - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) collectOutput(cfg, outR, errR) // PR should still be associated (not cleared), because the stack API says it's part of the stack. @@ -590,7 +590,7 @@ func TestSyncStackPRs_RemoteStack_FallsBackOnAPIError(t *testing.T) { }, } - syncStackPRs(cfg, s) + _ = syncStackPRs(cfg, s) collectOutput(cfg, outR, errR) // Should have fallen back to local discovery and found the open PR. diff --git a/cmd/view.go b/cmd/view.go index 8ff3a35..8bb52fd 100644 --- a/cmd/view.go +++ b/cmd/view.go @@ -50,10 +50,21 @@ func runView(cfg *config.Config, opts *viewOptions) error { s := result.Stack currentBranch := result.CurrentBranch + // Show loading indicator for interactive TUI mode. + showingLoader := false + if !opts.asJSON && !opts.short && cfg.IsInteractive() { + fmt.Fprintf(cfg.Err, "Loading stack...") + showingLoader = true + } + // Sync PR state and save (best-effort). - syncStackPRs(cfg, s) + prDetails := syncStackPRs(cfg, s) stack.SaveNonBlocking(gitDir, sf) + if showingLoader { + fmt.Fprintf(cfg.Err, "\r\033[2K") + } + if opts.asJSON { return viewJSON(cfg, s, currentBranch) } @@ -62,7 +73,7 @@ func runView(cfg *config.Config, opts *viewOptions) error { return viewShort(cfg, s, currentBranch) } - return viewFull(cfg, s, currentBranch) + return viewFull(cfg, s, currentBranch, prDetails) } func viewShort(cfg *config.Config, s *stack.Stack, currentBranch string) error { @@ -222,17 +233,17 @@ func shortPRSuffix(cfg *config.Config, b stack.BranchRef, host, owner, repo stri return fmt.Sprintf(" %s", colorFn(prNum)) } -func viewFull(cfg *config.Config, s *stack.Stack, currentBranch string) error { +func viewFull(cfg *config.Config, s *stack.Stack, currentBranch string, prDetails map[string]*ghapi.PRDetails) error { if !cfg.IsInteractive() { return viewFullStatic(cfg, s, currentBranch) } - return viewFullTUI(cfg, s, currentBranch) + return viewFullTUI(cfg, s, currentBranch, prDetails) } -func viewFullTUI(cfg *config.Config, s *stack.Stack, currentBranch string) error { +func viewFullTUI(cfg *config.Config, s *stack.Stack, currentBranch string, prDetails map[string]*ghapi.PRDetails) error { // Load enriched data for all branches - nodes := stackview.LoadBranchNodes(cfg, s, currentBranch) + nodes := stackview.LoadBranchNodes(cfg, s, currentBranch, prDetails) // Reverse nodes so index 0 = top of stack (matches visual order) reversed := make([]stackview.BranchNode, len(nodes)) diff --git a/internal/tui/stackview/data.go b/internal/tui/stackview/data.go index cd51706..3f3924e 100644 --- a/internal/tui/stackview/data.go +++ b/internal/tui/stackview/data.go @@ -1,6 +1,8 @@ package stackview import ( + "sync" + "github.com/github/gh-stack/internal/config" "github.com/github/gh-stack/internal/git" ghapi "github.com/github/gh-stack/internal/github" @@ -24,65 +26,109 @@ type BranchNode struct { FilesExpanded bool } +// maxGitConcurrency limits the number of concurrent git subprocess spawns. +const maxGitConcurrency = 4 + // LoadBranchNodes populates branch display data from a stack. -func LoadBranchNodes(cfg *config.Config, s *stack.Stack, currentBranch string) []BranchNode { - client, clientErr := cfg.GitHubClient() +// prDetails, when non-nil, provides pre-fetched PR data from syncStackPRs, +// avoiding redundant GitHub API calls. +func LoadBranchNodes(cfg *config.Config, s *stack.Stack, currentBranch string, prDetails map[string]*ghapi.PRDetails) []BranchNode { + // Fall back to API-based PR fetching if no pre-fetched details are available. + var client ghapi.ClientOps + var clientErr error + if prDetails == nil { + client, clientErr = cfg.GitHubClient() + } + + // Pre-compute base branches (reads s.Branches, so must happen before goroutines). + type branchInput struct { + ref stack.BranchRef + baseBranch string + isCurrent bool + } + inputs := make([]branchInput, len(s.Branches)) + for i, b := range s.Branches { + inputs[i] = branchInput{ + ref: b, + baseBranch: s.ActiveBaseBranch(b.Branch), + isCurrent: b.Branch == currentBranch, + } + } nodes := make([]BranchNode, len(s.Branches)) - for i, b := range s.Branches { - baseBranch := s.ActiveBaseBranch(b.Branch) + // Load branch data concurrently using a WaitGroup for completion and a + // semaphore channel to cap the number of in-flight git subprocesses + // (see maxGitConcurrency). + var wg sync.WaitGroup + sem := make(chan struct{}, maxGitConcurrency) - node := BranchNode{ - Ref: b, - IsCurrent: b.Branch == currentBranch, - BaseBranch: baseBranch, - IsLinear: true, - } + for i, inp := range inputs { + wg.Add(1) + go func(idx int, inp branchInput) { + defer wg.Done() - // Check linearity (is base an ancestor of this branch?) - if isAncestor, err := git.IsAncestor(baseBranch, b.Branch); err == nil { - node.IsLinear = isAncestor - } + // Acquire a semaphore slot to limit concurrent git calls. + sem <- struct{}{} + defer func() { <-sem }() - // Use the merge-base (fork point) as the diff anchor so that we - // only show changes introduced on this branch. Without this, a - // diverged base (e.g. local main ahead of the branch's fork point) - // would inflate the diff with unrelated files. - diffBase := baseBranch - if mb, err := git.MergeBase(baseBranch, b.Branch); err == nil { - diffBase = mb - } + node := BranchNode{ + Ref: inp.ref, + IsCurrent: inp.isCurrent, + BaseBranch: inp.baseBranch, + IsLinear: true, + } - // Fetch commit range - if commits, err := git.LogRange(diffBase, b.Branch); err == nil { - node.Commits = commits - } + // Check linearity (is base an ancestor of this branch?) + if isAncestor, err := git.IsAncestor(inp.baseBranch, inp.ref.Branch); err == nil { + node.IsLinear = isAncestor + } - // Compute per-file diff stats from local git - if files, err := git.DiffStatFiles(diffBase, b.Branch); err == nil { - node.FilesChanged = files - for _, f := range files { - node.Additions += f.Additions - node.Deletions += f.Deletions + // Use the merge-base (fork point) as the diff anchor so that we + // only show changes introduced on this branch. Without this, a + // diverged base (e.g. local main ahead of the branch's fork point) + // would inflate the diff with unrelated files. + diffBase := inp.baseBranch + if mb, err := git.MergeBase(inp.baseBranch, inp.ref.Branch); err == nil { + diffBase = mb + } + + // Fetch commit range + if commits, err := git.LogRange(diffBase, inp.ref.Branch); err == nil { + node.Commits = commits } - } - // Fetch enriched PR details. - // Only adopt the result if it matches our tracked PR or is OPEN. - // This prevents showing stale merged/closed PR details when a - // branch name was reused from a previously merged PR. - if clientErr == nil { - if pr, err := client.FindPRDetailsForBranch(b.Branch); err == nil && pr != nil { - tracked := b.PullRequest != nil && b.PullRequest.Number == pr.Number - if tracked || pr.State == "OPEN" { - node.PR = pr + // Compute per-file diff stats from local git + if files, err := git.DiffStatFiles(diffBase, inp.ref.Branch); err == nil { + node.FilesChanged = files + for _, f := range files { + node.Additions += f.Additions + node.Deletions += f.Deletions + } + } + + // Use pre-fetched PR details if available, otherwise fall back to API. + if prDetails != nil { + if details, ok := prDetails[inp.ref.Branch]; ok { + tracked := inp.ref.PullRequest != nil && inp.ref.PullRequest.Number == details.Number + if tracked || details.State == "OPEN" { + node.PR = details + } + } + } else if clientErr == nil { + if pr, err := client.FindPRDetailsForBranch(inp.ref.Branch); err == nil && pr != nil { + tracked := inp.ref.PullRequest != nil && inp.ref.PullRequest.Number == pr.Number + if tracked || pr.State == "OPEN" { + node.PR = pr + } } } - } - nodes[i] = node + // Each goroutine writes to its own index — no lock needed. + nodes[idx] = node + }(i, inp) } + wg.Wait() return nodes } diff --git a/internal/tui/stackview/data_test.go b/internal/tui/stackview/data_test.go index 88e69fa..3abbd8d 100644 --- a/internal/tui/stackview/data_test.go +++ b/internal/tui/stackview/data_test.go @@ -48,7 +48,7 @@ func TestLoadBranchNodes_UsesMergeBaseForDivergedBranch(t *testing.T) { // Ensure no real GitHub API calls cfg.GitHubClientOverride = &ghapi.MockClient{} - nodes := LoadBranchNodes(cfg, s, "feature") + nodes := LoadBranchNodes(cfg, s, "feature", nil) require.Len(t, nodes, 1) // Diff must be computed from merge-base, not from "main" directly. @@ -93,7 +93,7 @@ func TestLoadBranchNodes_LinearBranchStillUsesMergeBase(t *testing.T) { defer errW.Close() cfg.GitHubClientOverride = &ghapi.MockClient{} - nodes := LoadBranchNodes(cfg, s, "other") + nodes := LoadBranchNodes(cfg, s, "other", nil) require.Len(t, nodes, 1) assert.Equal(t, "main-tip-sha", diffBase) @@ -134,7 +134,7 @@ func TestLoadBranchNodes_IgnoresStaleMergedPRDetails(t *testing.T) { }, } - nodes := LoadBranchNodes(cfg, s, "other") + nodes := LoadBranchNodes(cfg, s, "other", nil) require.Len(t, nodes, 1) assert.Nil(t, nodes[0].PR, "stale merged PR should not be adopted") @@ -179,7 +179,7 @@ func TestLoadBranchNodes_ShowsTrackedMergedPRDetails(t *testing.T) { }, } - nodes := LoadBranchNodes(cfg, s, "other") + nodes := LoadBranchNodes(cfg, s, "other", nil) require.Len(t, nodes, 1) require.NotNil(t, nodes[0].PR, "tracked merged PR should be shown") @@ -217,7 +217,7 @@ func TestLoadBranchNodes_ShowsOpenPRDetails(t *testing.T) { }, } - nodes := LoadBranchNodes(cfg, s, "other") + nodes := LoadBranchNodes(cfg, s, "other", nil) require.Len(t, nodes, 1) require.NotNil(t, nodes[0].PR, "OPEN PR should be shown")