Skip to content

Commit 07c2434

Browse files
committed
fix for inflated diff counts when base branch has been updated since stack init
1 parent 5c1fe16 commit 07c2434

File tree

2 files changed

+110
-10
lines changed

2 files changed

+110
-10
lines changed

internal/tui/stackview/data.go

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ type BranchNode struct {
1313
IsCurrent bool
1414
IsLinear bool // whether history is linear with base branch
1515
BaseBranch string
16-
Commits []git.CommitInfo // commits unique to this branch (base..head)
17-
FilesChanged []git.FileDiffStat // per-file diff stats
16+
Commits []git.CommitInfo // commits unique to this branch (base..head)
17+
FilesChanged []git.FileDiffStat // per-file diff stats
1818
PR *ghapi.PRDetails
1919
Additions int
2020
Deletions int
@@ -45,15 +45,13 @@ func LoadBranchNodes(cfg *config.Config, s *stack.Stack, currentBranch string) [
4545
node.IsLinear = isAncestor
4646
}
4747

48-
// For merged branches, use the merge-base (fork point) as the diff
49-
// anchor since the base branch has moved past the merge point and
50-
// a two-dot diff would show nothing after a squash merge.
51-
isMerged := b.IsMerged()
48+
// Use the merge-base (fork point) as the diff anchor so that we
49+
// only show changes introduced on this branch. Without this, a
50+
// diverged base (e.g. local main ahead of the branch's fork point)
51+
// would inflate the diff with unrelated files.
5252
diffBase := baseBranch
53-
if isMerged {
54-
if mb, err := git.MergeBase(baseBranch, b.Branch); err == nil {
55-
diffBase = mb
56-
}
53+
if mb, err := git.MergeBase(baseBranch, b.Branch); err == nil {
54+
diffBase = mb
5755
}
5856

5957
// Fetch commit range
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
package stackview
2+
3+
import (
4+
"testing"
5+
6+
"github.com/github/gh-stack/internal/config"
7+
"github.com/github/gh-stack/internal/git"
8+
ghapi "github.com/github/gh-stack/internal/github"
9+
"github.com/github/gh-stack/internal/stack"
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
func TestLoadBranchNodes_UsesMergeBaseForDivergedBranch(t *testing.T) {
15+
// Scenario: local main has diverged from the branch's history.
16+
// Without merge-base, diff would be computed against main directly,
17+
// inflating the file count with unrelated changes.
18+
s := &stack.Stack{
19+
Trunk: stack.BranchRef{Branch: "main"},
20+
Branches: []stack.BranchRef{{Branch: "feature"}},
21+
}
22+
23+
var diffBase string
24+
restore := git.SetOps(&git.MockOps{
25+
IsAncestorFn: func(ancestor, descendant string) (bool, error) {
26+
// main is NOT an ancestor of feature (diverged)
27+
return false, nil
28+
},
29+
MergeBaseFn: func(a, b string) (string, error) {
30+
return "abc123", nil
31+
},
32+
LogRangeFn: func(base, head string) ([]git.CommitInfo, error) {
33+
return []git.CommitInfo{{SHA: "def456", Subject: "only commit"}}, nil
34+
},
35+
DiffStatFilesFn: func(base, head string) ([]git.FileDiffStat, error) {
36+
diffBase = base
37+
return []git.FileDiffStat{
38+
{Path: "file1.go", Additions: 5, Deletions: 2},
39+
{Path: "file2.go", Additions: 3, Deletions: 1},
40+
}, nil
41+
},
42+
})
43+
defer restore()
44+
45+
cfg, outW, errW := config.NewTestConfig()
46+
defer outW.Close()
47+
defer errW.Close()
48+
// Ensure no real GitHub API calls
49+
cfg.GitHubClientOverride = &ghapi.MockClient{}
50+
51+
nodes := LoadBranchNodes(cfg, s, "feature")
52+
53+
require.Len(t, nodes, 1)
54+
// Diff must be computed from merge-base, not from "main" directly.
55+
assert.Equal(t, "abc123", diffBase, "diff should use merge-base as base, not the branch name")
56+
assert.Len(t, nodes[0].FilesChanged, 2)
57+
assert.Equal(t, 8, nodes[0].Additions)
58+
assert.Equal(t, 3, nodes[0].Deletions)
59+
assert.False(t, nodes[0].IsLinear)
60+
}
61+
62+
func TestLoadBranchNodes_LinearBranchStillUsesMergeBase(t *testing.T) {
63+
// When base IS an ancestor (linear history), merge-base returns the
64+
// base tip, so behavior is unchanged.
65+
s := &stack.Stack{
66+
Trunk: stack.BranchRef{Branch: "main"},
67+
Branches: []stack.BranchRef{{Branch: "feature"}},
68+
}
69+
70+
var diffBase string
71+
restore := git.SetOps(&git.MockOps{
72+
IsAncestorFn: func(ancestor, descendant string) (bool, error) {
73+
return true, nil
74+
},
75+
MergeBaseFn: func(a, b string) (string, error) {
76+
// For linear history, merge-base returns the base tip
77+
return "main-tip-sha", nil
78+
},
79+
LogRangeFn: func(base, head string) ([]git.CommitInfo, error) {
80+
return nil, nil
81+
},
82+
DiffStatFilesFn: func(base, head string) ([]git.FileDiffStat, error) {
83+
diffBase = base
84+
return []git.FileDiffStat{
85+
{Path: "only.go", Additions: 1, Deletions: 0},
86+
}, nil
87+
},
88+
})
89+
defer restore()
90+
91+
cfg, outW, errW := config.NewTestConfig()
92+
defer outW.Close()
93+
defer errW.Close()
94+
cfg.GitHubClientOverride = &ghapi.MockClient{}
95+
96+
nodes := LoadBranchNodes(cfg, s, "other")
97+
98+
require.Len(t, nodes, 1)
99+
assert.Equal(t, "main-tip-sha", diffBase)
100+
assert.Len(t, nodes[0].FilesChanged, 1)
101+
assert.True(t, nodes[0].IsLinear)
102+
}

0 commit comments

Comments
 (0)