Skip to content

Commit a24bf4f

Browse files
committed
Use the API PR title/body for existing PRs and fix new-PR defaults
For existing PRs, the submit TUI showed a commit/template-derived draft instead of the pull request's real title and body. Fetch the actual title and body and render them in the read-only card: - open/draft/queued (tracked) and adopted-open PRs now carry title/body through the existing batch sync (added the fields to the GraphQL queries and PRDetails — no extra round trips), and - merged branches (which skip the live refresh) are filled in by a targeted enrichment step run only when the submit TUI opens. Also align the new-PR defaults with the non-TUI submit's defaultPRTitleBody: - Title: the commit subject only when the branch has exactly one commit, otherwise the humanized branch name (was: the oldest commit's subject even for multi-commit branches). - Description: the PR template, else the single commit's body, else empty (removed the bulleted commit-subject list for multi-commit branches).
1 parent cc60343 commit a24bf4f

7 files changed

Lines changed: 120 additions & 44 deletions

File tree

cmd/submit.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error {
194194
// auto-generated titles and bodies (today's behavior).
195195
var drafts map[string]*submitview.PRDraft
196196
if cfg.IsInteractive() && !opts.auto {
197-
collected, cancelled, tuiErr := collectPRDrafts(cfg, s, currentBranch, prDetails, templateContent)
197+
collected, cancelled, tuiErr := collectPRDrafts(cfg, client, s, currentBranch, prDetails, templateContent)
198198
if tuiErr != nil {
199199
cfg.Errorf("failed to run the submit editor: %s", tuiErr)
200200
return ErrSilent
@@ -255,7 +255,11 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error {
255255
// returns the per-branch overrides, whether the user cancelled, and any error.
256256
// When the stack contains no branches without a PR, it skips the TUI and
257257
// returns nil drafts so the normal push/relink path runs.
258-
func collectPRDrafts(cfg *config.Config, s *stack.Stack, currentBranch string, prDetails map[string]*github.PRDetails, templateContent string) (map[string]*submitview.PRDraft, bool, error) {
258+
func collectPRDrafts(cfg *config.Config, client github.ClientOps, s *stack.Stack, currentBranch string, prDetails map[string]*github.PRDetails, templateContent string) (map[string]*submitview.PRDraft, bool, error) {
259+
// Fill in the real title/description for existing PRs that were synced
260+
// without them (e.g. merged branches) so the read-only cards show API data.
261+
enrichPRContent(client, prDetails)
262+
259263
fmt.Fprintf(cfg.Err, "Loading stack...")
260264
viewNodes := stackview.LoadBranchNodes(cfg, s, currentBranch, prDetails)
261265
fmt.Fprintf(cfg.Err, "\r\033[2K")

cmd/submit_tui_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func TestCollectPRDrafts_SkipsWhenNoNewBranches(t *testing.T) {
7676
"b2": {Number: 2, State: "OPEN"},
7777
}
7878

79-
drafts, cancelled, err := collectPRDrafts(cfg, s, "b1", prDetails, "")
79+
drafts, cancelled, err := collectPRDrafts(cfg, nil, s, "b1", prDetails, "")
8080
require.NoError(t, err)
8181
assert.False(t, cancelled)
8282
assert.Nil(t, drafts, "no NEW branches means the TUI is skipped and drafts are nil")

cmd/utils.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,8 @@ func syncStackPRs(cfg *config.Config, s *stack.Stack) map[string]*github.PRDetai
412412
Number: pr.Number,
413413
State: "OPEN",
414414
URL: pr.URL,
415+
Title: pr.Title,
416+
Body: pr.Body,
415417
IsDraft: pr.IsDraft,
416418
Merged: false,
417419
IsQueued: pr.IsQueued(),
@@ -458,6 +460,8 @@ func prDetailsFromPR(pr *github.PullRequest) *github.PRDetails {
458460
Number: pr.Number,
459461
State: pr.State,
460462
URL: pr.URL,
463+
Title: pr.Title,
464+
Body: pr.Body,
461465
IsDraft: pr.IsDraft,
462466
Merged: pr.Merged,
463467
IsQueued: pr.IsQueued(),
@@ -481,6 +485,35 @@ func prDetailsFromTracked(ref *stack.PullRequestRef) *github.PRDetails {
481485
}
482486
}
483487

488+
// enrichPRContent fills in the Title and Body of any existing PR whose details
489+
// were built without them (e.g. merged branches, which skip the live refresh in
490+
// syncStackPRs). It is used before the submit TUI renders an existing PR's
491+
// read-only card so it shows the real PR title and description. PRs that already
492+
// have a title (the common open/draft/queued case) are left untouched.
493+
func enrichPRContent(client github.ClientOps, details map[string]*github.PRDetails) {
494+
if client == nil {
495+
return
496+
}
497+
var wg sync.WaitGroup
498+
sem := make(chan struct{}, maxAPIConcurrency)
499+
for _, d := range details {
500+
if d == nil || d.Number == 0 || strings.TrimSpace(d.Title) != "" {
501+
continue
502+
}
503+
wg.Add(1)
504+
go func(d *github.PRDetails) {
505+
defer wg.Done()
506+
sem <- struct{}{}
507+
defer func() { <-sem }()
508+
if pr, err := client.FindPRByNumber(d.Number); err == nil && pr != nil {
509+
d.Title = pr.Title
510+
d.Body = pr.Body
511+
}
512+
}(d)
513+
}
514+
wg.Wait()
515+
}
516+
484517
// syncStackPRsFromRemote uses the stack API to sync PR state. The remote
485518
// stack's PR list is the source of truth — PRs stay associated even if
486519
// closed. Returns the PRDetails map and true if sync succeeded, or nil and

cmd/utils_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -810,3 +810,25 @@ func TestEnsureLocalTrunk_CreateFails(t *testing.T) {
810810
assert.Error(t, err)
811811
assert.Contains(t, err.Error(), "could not create local trunk branch main")
812812
}
813+
814+
func TestEnrichPRContent(t *testing.T) {
815+
calls := 0
816+
client := &github.MockClient{
817+
FindPRByNumberFn: func(number int) (*github.PullRequest, error) {
818+
calls++
819+
return &github.PullRequest{Number: number, Title: "Fetched title", Body: "Fetched body"}, nil
820+
},
821+
}
822+
details := map[string]*github.PRDetails{
823+
"merged": {Number: 10, State: "MERGED"}, // missing title -> fetched
824+
"open": {Number: 11, State: "OPEN", Title: "Has it"}, // already has a title -> skipped
825+
"nonum": {Number: 0, State: "OPEN"}, // no number -> skipped
826+
}
827+
828+
enrichPRContent(client, details)
829+
830+
assert.Equal(t, 1, calls, "only the title-less PR with a number is fetched")
831+
assert.Equal(t, "Fetched title", details["merged"].Title)
832+
assert.Equal(t, "Fetched body", details["merged"].Body)
833+
assert.Equal(t, "Has it", details["open"].Title, "PRs that already have a title are untouched")
834+
}

internal/github/github.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ type PullRequest struct {
2929
Number int `graphql:"number"`
3030
State string `graphql:"state"`
3131
URL string `graphql:"url"`
32+
Title string `graphql:"title"`
33+
Body string `graphql:"body"`
3234
HeadRefName string `graphql:"headRefName"`
3335
BaseRefName string `graphql:"baseRefName"`
3436
IsDraft bool `graphql:"isDraft"`
@@ -101,6 +103,8 @@ func (c *Client) FindPRForBranch(branch string) (*PullRequest, error) {
101103
ID string `graphql:"id"`
102104
Number int `graphql:"number"`
103105
URL string `graphql:"url"`
106+
Title string `graphql:"title"`
107+
Body string `graphql:"body"`
104108
BaseRefName string `graphql:"baseRefName"`
105109
IsDraft bool `graphql:"isDraft"`
106110
MergeQueueEntry *MergeQueueEntry `graphql:"mergeQueueEntry"`
@@ -130,6 +134,8 @@ func (c *Client) FindPRForBranch(branch string) (*PullRequest, error) {
130134
ID: n.ID,
131135
Number: n.Number,
132136
URL: n.URL,
137+
Title: n.Title,
138+
Body: n.Body,
133139
BaseRefName: n.BaseRefName,
134140
IsDraft: n.IsDraft,
135141
MergeQueueEntry: n.MergeQueueEntry,
@@ -279,6 +285,8 @@ type PRDetails struct {
279285
Number int
280286
State string // OPEN, CLOSED, MERGED
281287
URL string
288+
Title string
289+
Body string
282290
IsDraft bool
283291
Merged bool
284292
IsQueued bool
@@ -342,6 +350,8 @@ func (c *Client) FindPRByNumber(number int) (*PullRequest, error) {
342350
Number int `graphql:"number"`
343351
State string `graphql:"state"`
344352
URL string `graphql:"url"`
353+
Title string `graphql:"title"`
354+
Body string `graphql:"body"`
345355
HeadRefName string `graphql:"headRefName"`
346356
BaseRefName string `graphql:"baseRefName"`
347357
IsDraft bool `graphql:"isDraft"`
@@ -371,6 +381,8 @@ func (c *Client) FindPRByNumber(number int) (*PullRequest, error) {
371381
Number: n.Number,
372382
State: n.State,
373383
URL: n.URL,
384+
Title: n.Title,
385+
Body: n.Body,
374386
HeadRefName: n.HeadRefName,
375387
BaseRefName: n.BaseRefName,
376388
IsDraft: n.IsDraft,

internal/tui/submitview/data.go

Lines changed: 25 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -40,61 +40,53 @@ func DeriveState(node stackview.BranchNode) BranchState {
4040
return StateNew
4141
}
4242

43-
// PrefillTitle returns the default PR title for a node: the subject of the
44-
// branch's first (oldest) commit when it has any commits, otherwise the
45-
// humanized branch name. git.LogRange returns commits newest-first, so the
46-
// oldest commit — the one that established the branch — is the last element.
43+
// PrefillTitle returns the default PR title for a new branch: the subject of its
44+
// single commit when the branch has exactly one commit, otherwise the humanized
45+
// branch name. This mirrors the non-TUI submit's defaultPRTitleBody.
4746
func PrefillTitle(node stackview.BranchNode) string {
48-
if n := len(node.Commits); n > 0 {
49-
if subject := strings.TrimSpace(node.Commits[n-1].Subject); subject != "" {
47+
if commits := node.Commits; len(commits) == 1 {
48+
if subject := strings.TrimSpace(commits[0].Subject); subject != "" {
5049
return subject
5150
}
5251
}
5352
return humanize(node.Ref.Branch)
5453
}
5554

56-
// PrefillDescription returns the default PR description following the spec's
57-
// priority order: the repo PR template if one exists, otherwise the single
58-
// commit body, otherwise a bulleted list of commit subjects for multi-commit
59-
// branches. The attribution footer is appended later, at submit time.
55+
// PrefillDescription returns the default PR description for a new branch: the
56+
// repo PR template if one exists, otherwise the body of the branch's single
57+
// commit, otherwise empty. Multi-commit branches with no template get an empty
58+
// description (no bulleted commit list). This mirrors the non-TUI submit. The
59+
// attribution footer is appended later, at submit time.
6060
func PrefillDescription(node stackview.BranchNode, template string) string {
6161
if t := strings.TrimSpace(template); t != "" {
6262
return t
6363
}
64-
65-
commits := node.Commits
66-
switch {
67-
case len(commits) == 1:
64+
if commits := node.Commits; len(commits) == 1 {
6865
return strings.TrimSpace(commits[0].Body)
69-
case len(commits) > 1:
70-
var b strings.Builder
71-
// List oldest commit first so the body reads like a changelog.
72-
for i := len(commits) - 1; i >= 0; i-- {
73-
subject := strings.TrimSpace(commits[i].Subject)
74-
if subject == "" {
75-
continue
76-
}
77-
b.WriteString("- ")
78-
b.WriteString(subject)
79-
b.WriteString("\n")
80-
}
81-
return strings.TrimSpace(b.String())
82-
default:
83-
return ""
8466
}
67+
return ""
8568
}
8669

8770
// NewSubmitNodes builds the per-branch UI state for the submit TUI from loaded
88-
// branch display data. NEW branches default to included; every node's title and
89-
// description are prefilled. New PRs default to ready for review; the per-PR
90-
// draft toggle starts off. The prefill snapshots are retained for edit
91-
// detection.
71+
// branch display data. NEW branches default to included; new PRs have their
72+
// title and description prefilled from commits and the PR template, while
73+
// existing PRs show their real title and description fetched from the API. New
74+
// PRs default to ready for review; the per-PR draft toggle starts off. The
75+
// prefill snapshots are retained for edit detection.
9276
func NewSubmitNodes(nodes []stackview.BranchNode, template string) []SubmitNode {
9377
out := make([]SubmitNode, len(nodes))
9478
for i, n := range nodes {
9579
state := DeriveState(n)
9680
title := PrefillTitle(n)
9781
desc := PrefillDescription(n, template)
82+
// Existing PRs render their real title/description from the API instead
83+
// of the commit/template-derived draft used for new PRs.
84+
if state != StateNew && n.PR != nil {
85+
desc = n.PR.Body
86+
if t := strings.TrimSpace(n.PR.Title); t != "" {
87+
title = t
88+
}
89+
}
9890
out[i] = SubmitNode{
9991
BranchNode: n,
10092
State: state,

internal/tui/submitview/data_test.go

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -102,11 +102,11 @@ func TestPrefillTitle(t *testing.T) {
102102
assert.Equal(t, "Add auth middleware", PrefillTitle(n))
103103
})
104104

105-
t.Run("multiple commits use the first (oldest) commit subject", func(t *testing.T) {
106-
// git.LogRange returns commits newest-first, so the last element is the
107-
// oldest — the commit that established the branch.
105+
t.Run("multiple commits humanize the branch name", func(t *testing.T) {
106+
// Only a single-commit branch uses the commit subject; multi-commit
107+
// branches default to the humanized branch name (matches non-TUI submit).
108108
n := node("feat/auth-middleware", commit("Polish middleware", ""), commit("Add auth middleware", ""))
109-
assert.Equal(t, "Add auth middleware", PrefillTitle(n))
109+
assert.Equal(t, "feat/auth middleware", PrefillTitle(n))
110110
})
111111

112112
t.Run("zero commits humanize branch name", func(t *testing.T) {
@@ -131,11 +131,10 @@ func TestPrefillDescription(t *testing.T) {
131131
assert.Equal(t, "Detailed body\nsecond line", PrefillDescription(n, ""))
132132
})
133133

134-
t.Run("multi commit lists subjects oldest first", func(t *testing.T) {
135-
// LogRange returns newest first; the body should read oldest first.
134+
t.Run("multi commit with no template is empty", func(t *testing.T) {
135+
// No bulleted commit list — multi-commit branches default to empty.
136136
n := node("feat/a", commit("newest", ""), commit("middle", ""), commit("oldest", ""))
137-
got := PrefillDescription(n, "")
138-
assert.Equal(t, "- oldest\n- middle\n- newest", got)
137+
assert.Equal(t, "", PrefillDescription(n, ""))
139138
})
140139

141140
t.Run("no commits no template is empty", func(t *testing.T) {
@@ -167,6 +166,20 @@ func TestNewSubmitNodes(t *testing.T) {
167166
assert.False(t, got[1].Included)
168167
}
169168

169+
func TestNewSubmitNodes_ExistingPRUsesAPIContent(t *testing.T) {
170+
// An existing PR shows its real title/body from the API, not the
171+
// commit-subject / template prefill used for new PRs.
172+
nodes := []stackview.BranchNode{
173+
withPR(node("feat/b", commit("commit subject", "commit body")),
174+
&ghapi.PRDetails{Number: 2, State: "OPEN", Title: "Real PR title", Body: "Real PR body"}),
175+
}
176+
got := NewSubmitNodes(nodes, "## Template")
177+
assert.Len(t, got, 1)
178+
assert.Equal(t, StateOpen, got[0].State)
179+
assert.Equal(t, "Real PR title", got[0].Title, "existing PR uses the API title")
180+
assert.Equal(t, "Real PR body", got[0].Description, "existing PR uses the API body, not the template")
181+
}
182+
170183
func TestCounts(t *testing.T) {
171184
nodes := []SubmitNode{
172185
{State: StateNew, Included: true},

0 commit comments

Comments
 (0)