bd-ckej: fix orphan skip count mismatch on fresh import (#572)

* bd-ckej: fix orphan skip count mismatch on fresh import

When OrphanSkip mode is used during import and a child issue's parent doesn't
exist, the issue ID was cleared to '' but then regenerated anyway in
GenerateBatchIssueIDs, causing it to be created in the database. This resulted
in a count mismatch: JSONL had 824 issues but only 823 were in the database (one
orphan was counted but not created).

Fix: Filter out orphaned issues with empty IDs before batch creation and track
them in result.Skipped so the count stays accurate.

* test: add TestImportOrphanSkip_CountMismatch for bd-ckej

Adds comprehensive test that verifies orphaned issues are properly skipped
during import when orphan_handling=OrphanSkip and parent doesn't exist.

Also improves the fix to pre-filter orphaned issues before batch creation,
ensuring they're not inserted then have IDs cleared (preventing count
mismatches).

---------

Co-authored-by: Amp <amp@example.com>
This commit is contained in:
matt wilkie
2025-12-15 22:17:32 -07:00
committed by GitHub
parent bb3ca0c51d
commit cd3b0e30be
2 changed files with 151 additions and 9 deletions

View File

@@ -671,24 +671,62 @@ func upsertIssues(ctx context.Context, sqliteStore *sqlite.SQLiteStorage, issues
}
}
// Filter out orphaned issues if orphan_handling is set to skip (bd-ckej)
// Pre-filter before batch creation to prevent orphans from being created then ID-cleared
if opts.OrphanHandling == sqlite.OrphanSkip {
var filteredNewIssues []*types.Issue
for _, issue := range newIssues {
// Check if this is a hierarchical child whose parent doesn't exist
if strings.Contains(issue.ID, ".") {
lastDot := strings.LastIndex(issue.ID, ".")
parentID := issue.ID[:lastDot]
// Check if parent exists in either existing DB issues or in newIssues batch
var parentExists bool
for _, dbIssue := range dbIssues {
if dbIssue.ID == parentID {
parentExists = true
break
}
}
if !parentExists {
for _, newIssue := range newIssues {
if newIssue.ID == parentID {
parentExists = true
break
}
}
}
if !parentExists {
// Skip this orphaned issue
result.Skipped++
continue
}
}
filteredNewIssues = append(filteredNewIssues, issue)
}
newIssues = filteredNewIssues
}
// Batch create all new issues
// Sort by hierarchy depth to ensure parents are created before children
if len(newIssues) > 0 {
sort.Slice(newIssues, func(i, j int) bool {
depthI := strings.Count(newIssues[i].ID, ".")
depthJ := strings.Count(newIssues[j].ID, ".")
depthI := strings.Count(newIssues[i].ID, ".")
depthJ := strings.Count(newIssues[j].ID, ".")
if depthI != depthJ {
return depthI < depthJ // Shallower first
}
return newIssues[i].ID < newIssues[j].ID // Stable sort
return depthI < depthJ // Shallower first
}
return newIssues[i].ID < newIssues[j].ID // Stable sort
})
// Create in batches by depth level (max depth 3)
for depth := 0; depth <= 3; depth++ {
var batchForDepth []*types.Issue
for _, issue := range newIssues {
if strings.Count(issue.ID, ".") == depth {
batchForDepth = append(batchForDepth, issue)
var batchForDepth []*types.Issue
for _, issue := range newIssues {
if strings.Count(issue.ID, ".") == depth {
batchForDepth = append(batchForDepth, issue)
}
}
if len(batchForDepth) > 0 {

View File

@@ -1394,3 +1394,107 @@ func TestImportIssues_LegacyDeletionsConvertedToTombstones(t *testing.T) {
t.Errorf("Expected DeleteReason 'duplicate of test-xyz', got %q", tombstone.DeleteReason)
}
}
// TestImportOrphanSkip_CountMismatch verifies that orphaned issues are properly
// skipped during import and tracked in the result count (bd-ckej).
//
// Discovery recipe: Fresh clone >> bd init >> bd doctor --fix >> bd sync --import-only
// would show "Count mismatch: database has 823 issues, JSONL has 824" if orphaned
// child issues weren't properly filtered out (before the fix).
//
// The test imports issues where a child's parent doesn't exist in the database.
// With OrphanSkip mode, the child should be filtered out before creation.
func TestImportOrphanSkip_CountMismatch(t *testing.T) {
ctx := context.Background()
store, err := sqlite.New(ctx, ":memory:")
if err != nil {
t.Fatalf("Failed to create store: %v", err)
}
defer store.Close()
// Set prefix
if err := store.SetConfig(ctx, "issue_prefix", "test"); err != nil {
t.Fatalf("Failed to set prefix: %v", err)
}
now := time.Now()
// Prepare to import: normal issues + orphaned child
// The orphaned child has a parent (test-orphan) that doesn't exist in the database
issues := []*types.Issue{
{
ID: "test-new1",
Title: "Normal Issue",
Status: types.StatusOpen,
Priority: 2,
IssueType: types.TypeTask,
CreatedAt: now,
UpdatedAt: now,
},
{
ID: "test-orphan.1", // Child of non-existent parent
Title: "Orphaned Child",
Status: types.StatusOpen,
Priority: 2,
IssueType: types.TypeTask,
CreatedAt: now,
UpdatedAt: now,
},
{
ID: "test-new2",
Title: "Another Normal Issue",
Status: types.StatusOpen,
Priority: 2,
IssueType: types.TypeTask,
CreatedAt: now,
UpdatedAt: now,
},
}
// Import with OrphanSkip mode - parent doesn't exist
result, err := ImportIssues(ctx, "", store, issues, Options{
OrphanHandling: sqlite.OrphanSkip,
SkipPrefixValidation: true, // Allow explicit IDs during import
})
if err != nil {
t.Fatalf("Import failed: %v", err)
}
// Verify results:
// - 2 issues should be created (test-new1, test-new2)
// - 1 issue should be skipped (test-orphan.1 - no parent exists)
if result.Created != 2 {
t.Errorf("Expected 2 created issues, got %d", result.Created)
}
if result.Skipped != 1 {
t.Errorf("Expected 1 skipped issue (orphan), got %d", result.Skipped)
}
// Verify the orphan is NOT in the database
allIssues, err := store.SearchIssues(ctx, "", types.IssueFilter{})
if err != nil {
t.Fatalf("Failed to search issues: %v", err)
}
var orphanFound bool
for _, issue := range allIssues {
if issue.ID == "test-orphan.1" {
orphanFound = true
break
}
}
if orphanFound {
t.Error("Orphaned issue test-orphan.1 should not be in database")
}
// Verify normal issues ARE in the database
var count int
for _, issue := range allIssues {
if issue.ID == "test-new1" || issue.ID == "test-new2" {
count++
}
}
if count != 2 {
t.Errorf("Expected 2 normal issues in database, found %d", count)
}
}