Fix bd-88: import now reports unchanged issues correctly
When importing JSONL that matches the database exactly, import was reporting '0 created, 0 updated' which was confusing. Now it properly tracks and reports unchanged issues. Changes: - Added Unchanged field to ImportResult - Track unchanged issues separately from skipped/updated - Display unchanged count in import summary - Updated dry-run output to show unchanged count - Added test to verify correct reporting behavior Example output: 'Import complete: 0 created, 0 updated, 88 unchanged' Amp-Thread-ID: https://ampcode.com/threads/T-5dd4843e-9ce3-4fe0-9658-d2227b0a2e4e Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
@@ -141,8 +141,11 @@ Behavior:
|
||||
} else if !result.PrefixMismatch {
|
||||
fmt.Fprintf(os.Stderr, "No collisions detected.\n")
|
||||
}
|
||||
fmt.Fprintf(os.Stderr, "Would create %d new issues, update %d existing issues\n",
|
||||
result.Created, result.Updated)
|
||||
msg := fmt.Sprintf("Would create %d new issues, update %d existing issues", result.Created, result.Updated)
|
||||
if result.Unchanged > 0 {
|
||||
msg += fmt.Sprintf(", %d unchanged", result.Unchanged)
|
||||
}
|
||||
fmt.Fprintf(os.Stderr, "%s\n", msg)
|
||||
fmt.Fprintf(os.Stderr, "\nDry-run mode: no changes made\n")
|
||||
os.Exit(0)
|
||||
}
|
||||
@@ -177,6 +180,9 @@ Behavior:
|
||||
|
||||
// Print summary
|
||||
fmt.Fprintf(os.Stderr, "Import complete: %d created, %d updated", result.Created, result.Updated)
|
||||
if result.Unchanged > 0 {
|
||||
fmt.Fprintf(os.Stderr, ", %d unchanged", result.Unchanged)
|
||||
}
|
||||
if result.Skipped > 0 {
|
||||
fmt.Fprintf(os.Stderr, ", %d skipped", result.Skipped)
|
||||
}
|
||||
|
||||
100
cmd/bd/import_bug_test.go
Normal file
100
cmd/bd/import_bug_test.go
Normal file
@@ -0,0 +1,100 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/steveyegge/beads/internal/storage/sqlite"
|
||||
"github.com/steveyegge/beads/internal/types"
|
||||
)
|
||||
|
||||
// TestImportReturnsCorrectCounts reproduces bd-88
|
||||
// Import should report correct "created" count when importing new issues
|
||||
func TestImportReturnsCorrectCounts(t *testing.T) {
|
||||
// Create temporary database
|
||||
tmpDir, err := os.MkdirTemp("", "beads-test-")
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create temp dir: %v", err)
|
||||
}
|
||||
defer os.RemoveAll(tmpDir)
|
||||
|
||||
dbPath := filepath.Join(tmpDir, ".beads", "issues.db")
|
||||
if err := os.MkdirAll(filepath.Dir(dbPath), 0755); err != nil {
|
||||
t.Fatalf("Failed to create .beads dir: %v", err)
|
||||
}
|
||||
|
||||
// Initialize database
|
||||
store, err := sqlite.New(dbPath)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create store: %v", err)
|
||||
}
|
||||
defer store.Close()
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
// Create test issues to import
|
||||
issues := make([]*types.Issue, 0, 5)
|
||||
for i := 1; i <= 5; i++ {
|
||||
id := fmt.Sprintf("test-%d", i)
|
||||
issues = append(issues, &types.Issue{
|
||||
ID: id,
|
||||
Title: fmt.Sprintf("Test Issue %d", i),
|
||||
Description: "Test description",
|
||||
Status: types.StatusOpen,
|
||||
Priority: 2,
|
||||
IssueType: types.TypeTask,
|
||||
})
|
||||
}
|
||||
|
||||
// Import with default options
|
||||
opts := ImportOptions{
|
||||
ResolveCollisions: false,
|
||||
DryRun: false,
|
||||
SkipUpdate: false,
|
||||
Strict: false,
|
||||
}
|
||||
|
||||
result, err := importIssuesCore(ctx, dbPath, store, issues, opts)
|
||||
if err != nil {
|
||||
t.Fatalf("Import failed: %v", err)
|
||||
}
|
||||
|
||||
// Check that Created count matches
|
||||
if result.Created != len(issues) {
|
||||
t.Errorf("Expected Created=%d, got %d", len(issues), result.Created)
|
||||
}
|
||||
|
||||
// Verify issues are actually in the database
|
||||
for _, issue := range issues {
|
||||
retrieved, err := store.GetIssue(ctx, issue.ID)
|
||||
if err != nil {
|
||||
t.Errorf("Failed to get issue %s: %v", issue.ID, err)
|
||||
}
|
||||
if retrieved == nil {
|
||||
t.Errorf("Issue %s not found in database", issue.ID)
|
||||
}
|
||||
}
|
||||
|
||||
// Now test re-importing the same issues (idempotent case)
|
||||
result2, err := importIssuesCore(ctx, dbPath, store, issues, opts)
|
||||
if err != nil {
|
||||
t.Fatalf("Second import failed: %v", err)
|
||||
}
|
||||
|
||||
// bd-88: When reimporting unchanged issues, should report them as "Unchanged"
|
||||
if result2.Created != 0 {
|
||||
t.Errorf("Second import: expected Created=0, got %d", result2.Created)
|
||||
}
|
||||
if result2.Updated != 0 {
|
||||
t.Errorf("Second import: expected Updated=0, got %d", result2.Updated)
|
||||
}
|
||||
if result2.Unchanged != len(issues) {
|
||||
t.Errorf("Second import: expected Unchanged=%d, got %d", len(issues), result2.Unchanged)
|
||||
}
|
||||
|
||||
t.Logf("Second import: Created=%d, Updated=%d, Unchanged=%d, Skipped=%d",
|
||||
result2.Created, result2.Updated, result2.Unchanged, result2.Skipped)
|
||||
}
|
||||
@@ -165,6 +165,7 @@ type ImportOptions struct {
|
||||
type ImportResult struct {
|
||||
Created int // New issues created
|
||||
Updated int // Existing issues updated
|
||||
Unchanged int // Existing issues that matched exactly (idempotent)
|
||||
Skipped int // Issues skipped (duplicates, errors)
|
||||
Collisions int // Collisions detected
|
||||
IDMapping map[string]string // Mapping of remapped IDs (old -> new)
|
||||
@@ -317,7 +318,8 @@ func importIssuesCore(ctx context.Context, dbPath string, store storage.Storage,
|
||||
} else if opts.DryRun {
|
||||
// No collisions in dry-run mode
|
||||
result.Created = len(collisionResult.NewIssues)
|
||||
result.Updated = len(collisionResult.ExactMatches)
|
||||
// bd-88: ExactMatches are unchanged issues (idempotent), not updates
|
||||
result.Unchanged = len(collisionResult.ExactMatches)
|
||||
return result, nil
|
||||
}
|
||||
|
||||
@@ -362,15 +364,15 @@ func importIssuesCore(ctx context.Context, dbPath string, store storage.Storage,
|
||||
updates["external_ref"] = nil
|
||||
}
|
||||
|
||||
// bd-84: Only update if data actually changed (prevents timestamp churn)
|
||||
// bd-88: Only update if data actually changed (prevents timestamp churn)
|
||||
if issueDataChanged(existing, updates) {
|
||||
if err := sqliteStore.UpdateIssue(ctx, issue.ID, updates, "import"); err != nil {
|
||||
return nil, fmt.Errorf("error updating issue %s: %w", issue.ID, err)
|
||||
}
|
||||
result.Updated++
|
||||
} else {
|
||||
// Issue unchanged - count as skipped to avoid polluting JSONL with timestamp updates
|
||||
result.Skipped++
|
||||
// bd-88: Track unchanged issues separately for accurate reporting
|
||||
result.Unchanged++
|
||||
}
|
||||
} else {
|
||||
// New issue - check for duplicates in import batch
|
||||
|
||||
Reference in New Issue
Block a user