Skip to content

Commit c57bcfc

Browse files
committed
fix: batch wisp checks during export
1 parent a54ef8d commit c57bcfc

5 files changed

Lines changed: 157 additions & 22 deletions

File tree

cmd/bd/where_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,9 @@ func TestResolveWhereBeadsDir_ReturnsEmptyWithoutWorkspace(t *testing.T) {
113113
}
114114

115115
func TestResolveWhereBeadsDir_UsesInitializedDBPath(t *testing.T) {
116+
saveAndRestoreGlobals(t)
117+
ensureCleanGlobalState(t)
118+
116119
originalDBPath := dbPath
117120
originalCmdCtx := cmdCtx
118121
defer func() {
@@ -150,6 +153,9 @@ func TestResolveWhereBeadsDir_UsesInitializedDBPath(t *testing.T) {
150153
})
151154

152155
dbPath = dbDir
156+
t.Setenv("BEADS_DIR", "")
157+
t.Setenv("BEADS_DB", dbDir)
158+
t.Setenv("BD_DB", "")
153159

154160
if got := resolveWhereBeadsDir(nil); !utils.PathsEqual(got, beadsDir) {
155161
t.Fatalf("resolveWhereBeadsDir(nil) = %q, want %q", got, beadsDir)

internal/storage/dolt/wisp_set_scoped_test.go

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,3 +316,139 @@ func TestGetIssuesByIDsInTx_SmallInputLargeWispTable(t *testing.T) {
316316
t.Fatalf("read tx: %v", err)
317317
}
318318
}
319+
320+
// TestExportHydrationHelpers_SmallInputLargeWispTable covers the bulk
321+
// hydration helpers used by `bd export`. These helpers must partition input IDs
322+
// with a scoped WispIDSetInTx query; falling back to per-ID IsActiveWispInTx
323+
// checks is correct but becomes painfully slow against remote Dolt servers.
324+
func TestExportHydrationHelpers_SmallInputLargeWispTable(t *testing.T) {
325+
store, cleanup := setupTestStore(t)
326+
defer cleanup()
327+
328+
ctx, cancel := testContext(t)
329+
defer cancel()
330+
331+
blocker := &types.Issue{
332+
ID: "export-hydrate-blocker",
333+
Title: "blocker",
334+
Status: types.StatusOpen,
335+
Priority: 2,
336+
IssueType: types.TypeTask,
337+
}
338+
if err := store.CreateIssue(ctx, blocker, "tester"); err != nil {
339+
t.Fatalf("create blocker: %v", err)
340+
}
341+
342+
perm := &types.Issue{
343+
ID: "export-hydrate-perm",
344+
Title: "perm",
345+
Status: types.StatusOpen,
346+
Priority: 2,
347+
IssueType: types.TypeTask,
348+
}
349+
if err := store.CreateIssue(ctx, perm, "tester"); err != nil {
350+
t.Fatalf("create perm: %v", err)
351+
}
352+
if _, err := store.AddIssueComment(ctx, perm.ID, "tester", "perm comment"); err != nil {
353+
t.Fatalf("add perm comment: %v", err)
354+
}
355+
if err := store.AddDependency(ctx, &types.Dependency{
356+
IssueID: perm.ID,
357+
DependsOnID: blocker.ID,
358+
Type: types.DepBlocks,
359+
}, "tester"); err != nil {
360+
t.Fatalf("add perm dependency: %v", err)
361+
}
362+
363+
target := &types.Issue{
364+
Title: "target wisp",
365+
Status: types.StatusOpen,
366+
Priority: 2,
367+
IssueType: types.TypeTask,
368+
Ephemeral: true,
369+
}
370+
if err := store.CreateIssue(ctx, target, "tester"); err != nil {
371+
t.Fatalf("create target wisp: %v", err)
372+
}
373+
if _, err := store.AddIssueComment(ctx, target.ID, "tester", "target comment"); err != nil {
374+
t.Fatalf("add target comment: %v", err)
375+
}
376+
if err := store.AddDependency(ctx, &types.Dependency{
377+
IssueID: target.ID,
378+
DependsOnID: blocker.ID,
379+
Type: types.DepBlocks,
380+
}, "tester"); err != nil {
381+
t.Fatalf("add target dependency: %v", err)
382+
}
383+
384+
const noiseCount = 20
385+
noiseIDs := make([]string, 0, noiseCount)
386+
for i := 0; i < noiseCount; i++ {
387+
iss := &types.Issue{
388+
Title: fmt.Sprintf("noise wisp %d", i),
389+
Status: types.StatusOpen,
390+
Priority: 2,
391+
IssueType: types.TypeTask,
392+
Ephemeral: true,
393+
}
394+
if err := store.CreateIssue(ctx, iss, "tester"); err != nil {
395+
t.Fatalf("create noise %d: %v", i, err)
396+
}
397+
if _, err := store.AddIssueComment(ctx, iss.ID, "tester", fmt.Sprintf("noise comment %d", i)); err != nil {
398+
t.Fatalf("add noise comment %d: %v", i, err)
399+
}
400+
noiseIDs = append(noiseIDs, iss.ID)
401+
}
402+
403+
input := []string{perm.ID, target.ID}
404+
405+
if err := store.withReadTx(ctx, func(tx *sql.Tx) error {
406+
commentMap, err := issueops.GetCommentsForIssuesInTx(ctx, tx, input)
407+
if err != nil {
408+
return fmt.Errorf("GetCommentsForIssuesInTx: %w", err)
409+
}
410+
if got := len(commentMap[perm.ID]); got != 1 {
411+
t.Errorf("perm comments: got %d, want 1", got)
412+
}
413+
if got := len(commentMap[target.ID]); got != 1 {
414+
t.Errorf("target comments: got %d, want 1", got)
415+
}
416+
417+
commentCounts, err := issueops.GetCommentCountsInTx(ctx, tx, input)
418+
if err != nil {
419+
return fmt.Errorf("GetCommentCountsInTx: %w", err)
420+
}
421+
if got := commentCounts[perm.ID]; got != 1 {
422+
t.Errorf("perm comment count: got %d, want 1", got)
423+
}
424+
if got := commentCounts[target.ID]; got != 1 {
425+
t.Errorf("target comment count: got %d, want 1", got)
426+
}
427+
428+
depMap, err := issueops.GetDependencyRecordsForIssuesInTx(ctx, tx, input)
429+
if err != nil {
430+
return fmt.Errorf("GetDependencyRecordsForIssuesInTx: %w", err)
431+
}
432+
if got := len(depMap[perm.ID]); got != 1 {
433+
t.Errorf("perm dependencies: got %d, want 1", got)
434+
}
435+
if got := len(depMap[target.ID]); got != 1 {
436+
t.Errorf("target dependencies: got %d, want 1", got)
437+
}
438+
439+
for _, n := range noiseIDs {
440+
if _, leaked := commentMap[n]; leaked {
441+
t.Errorf("comments leaked noise wisp %q", n)
442+
}
443+
if _, leaked := commentCounts[n]; leaked {
444+
t.Errorf("comment counts leaked noise wisp %q", n)
445+
}
446+
if _, leaked := depMap[n]; leaked {
447+
t.Errorf("dependencies leaked noise wisp %q", n)
448+
}
449+
}
450+
return nil
451+
}); err != nil {
452+
t.Fatalf("read tx: %v", err)
453+
}
454+
}

internal/storage/issueops/bulk_ops.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -109,15 +109,14 @@ func GetCommentsForIssuesInTx(ctx context.Context, tx *sql.Tx, issueIDs []string
109109

110110
result := make(map[string][]*types.Comment)
111111

112-
// Partition IDs by wisp status.
113-
var wispIDs, permIDs []string
114-
for _, id := range issueIDs {
115-
if IsActiveWispInTx(ctx, tx, id) {
116-
wispIDs = append(wispIDs, id)
117-
} else {
118-
permIDs = append(permIDs, id)
119-
}
112+
// Partition IDs by wisp status with one scoped query instead of one
113+
// existence query per ID. This path is used by export and can receive
114+
// thousands of IDs against a remote Dolt server.
115+
wispSet, err := WispIDSetInTx(ctx, tx, issueIDs)
116+
if err != nil {
117+
return nil, fmt.Errorf("get comments for issues: build wisp set: %w", err)
120118
}
119+
wispIDs, permIDs := partitionByWispSet(issueIDs, wispSet)
121120

122121
if len(permIDs) > 0 {
123122
if err := getCommentsForIDsInto(ctx, tx, "comments", permIDs, result); err != nil {

internal/storage/issueops/comments.go

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,11 @@ func GetCommentCountsInTx(ctx context.Context, tx *sql.Tx, issueIDs []string) (m
5353

5454
result := make(map[string]int)
5555

56-
var wispIDs, permIDs []string
57-
for _, id := range issueIDs {
58-
if IsActiveWispInTx(ctx, tx, id) {
59-
wispIDs = append(wispIDs, id)
60-
} else {
61-
permIDs = append(permIDs, id)
62-
}
56+
wispSet, err := WispIDSetInTx(ctx, tx, issueIDs)
57+
if err != nil {
58+
return nil, fmt.Errorf("get comment counts: build wisp set: %w", err)
6359
}
60+
wispIDs, permIDs := partitionByWispSet(issueIDs, wispSet)
6461

6562
for _, pair := range []struct {
6663
table string

internal/storage/issueops/dependency_queries.go

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,11 @@ func GetDependencyRecordsForIssuesInTx(ctx context.Context, tx *sql.Tx, issueIDs
4242

4343
result := make(map[string][]*types.Dependency)
4444

45-
var wispIDs, permIDs []string
46-
for _, id := range issueIDs {
47-
if IsActiveWispInTx(ctx, tx, id) {
48-
wispIDs = append(wispIDs, id)
49-
} else {
50-
permIDs = append(permIDs, id)
51-
}
45+
wispSet, err := WispIDSetInTx(ctx, tx, issueIDs)
46+
if err != nil {
47+
return nil, fmt.Errorf("get dependency records: build wisp set: %w", err)
5248
}
49+
wispIDs, permIDs := partitionByWispSet(issueIDs, wispSet)
5350

5451
for _, pair := range []struct {
5552
table string

0 commit comments

Comments
 (0)