Skip to content

Commit aaa8da1

Browse files
committed
Speed up PR mutation responses
1 parent 53b2acb commit aaa8da1

3 files changed

Lines changed: 54 additions & 7 deletions

File tree

internal/sdk/additional_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,49 @@ func TestDetailContext_UsesContextAwareCheckPR(t *testing.T) {
588588
}
589589
}
590590

591+
func TestPRMutations_DoNotRequireCheckPRContextForBranchDurability(t *testing.T) {
592+
db := newFakeDB()
593+
db.seedItem(fakeItem{
594+
ID: "w-1",
595+
Title: "Fix bug",
596+
Status: "claimed",
597+
ClaimedBy: "alice",
598+
PostedBy: "bob",
599+
EffortLevel: "medium",
600+
})
601+
602+
c := New(ClientConfig{
603+
DB: db,
604+
RigHandle: "alice",
605+
Mode: "pr",
606+
CheckPRContext: func(context.Context, string) string {
607+
t.Fatal("CheckPRContext() should not be called on the mutation hot path")
608+
return ""
609+
},
610+
CheckPR: func(string) string {
611+
t.Fatal("CheckPR() should not be called on the mutation hot path")
612+
return ""
613+
},
614+
})
615+
616+
result, err := c.Unclaim("w-1")
617+
if err != nil {
618+
t.Fatalf("Unclaim() error = %v", err)
619+
}
620+
if result.Detail == nil || result.Detail.Item == nil {
621+
t.Fatal("expected mutation detail item")
622+
}
623+
if result.Detail.Item.Status != "open" {
624+
t.Fatalf("status = %q, want open", result.Detail.Item.Status)
625+
}
626+
if result.Detail.Branch == "" {
627+
t.Fatal("expected branch to persist when branch diverges from main")
628+
}
629+
if result.Detail.Delta == "" {
630+
t.Fatal("expected non-empty delta for branch mutation")
631+
}
632+
}
633+
591634
func TestDetail_UsesSingleJoinedQuery(t *testing.T) {
592635
var calls int
593636
db := &queryOnlyDB{

internal/sdk/mutate.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package sdk
22

33
import (
44
"bytes"
5-
"context"
65
"fmt"
76
"io"
87
"strings"
@@ -158,9 +157,10 @@ func (c *Client) mutatePRResult(wantedID, branch, mainStatus string) *MutationRe
158157
detail.Actions = commons.AvailableTransitions(item, c.rigHandle)
159158
detail.Delta = commons.ComputeDelta(mainStatus, item.Status, true)
160159
}
161-
if branch != "" {
162-
detail.PRURL = c.checkPRContext(context.Background(), branch)
163-
}
160+
// Skip PR lookup on mutation responses. On large repos, finding an existing
161+
// PR requires paging through upstream pulls and can dominate mutation
162+
// latency even though the branch write has already succeeded. Detail reads
163+
// still resolve PRURL on demand.
164164
if branch != "" && c.BranchURL != nil {
165165
detail.BranchURL = c.BranchURL(branch)
166166
}

internal/sdk/sdk_test.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1836,11 +1836,15 @@ func TestBranchActions_PRMode_WithPR(t *testing.T) {
18361836
})
18371837

18381838
// Claim creates a branch with a delta.
1839-
result, err := c.Claim("w-1")
1840-
if err != nil {
1839+
if _, err := c.Claim("w-1"); err != nil {
18411840
t.Fatalf("Claim: %v", err)
18421841
}
1843-
d := result.Detail
1842+
// Detail lookups still resolve the existing PR and should suppress
1843+
// submit_pr from branch actions.
1844+
d, err := c.Detail("w-1")
1845+
if err != nil {
1846+
t.Fatalf("Detail: %v", err)
1847+
}
18441848
// PR mode + delta + existing PR → discard only
18451849
if len(d.BranchActions) != 1 {
18461850
t.Fatalf("expected 1 branch action, got %v", d.BranchActions)

0 commit comments

Comments
 (0)