fix(rename-prefix): use hash IDs instead of sequential in --repair mode
The --repair flag was generating sequential IDs (sageox-9895, sageox-9896) instead of hash-based IDs (sageox-jwnv, sageox-urtm). This fix uses the proper GenerateIssueID function from sqlite package to generate consistent hash-based IDs during prefix repair operations. Changes: - Import sqlite package for hash ID generation - Add generateRepairHashID helper that uses sqlite.GenerateIssueID - Track used IDs within batch to avoid collisions - Update test to verify hash IDs instead of sequential
This commit is contained in:
@@ -2,16 +2,19 @@ package main
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"database/sql"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
"regexp"
|
"regexp"
|
||||||
"sort"
|
"sort"
|
||||||
"strings"
|
"strings"
|
||||||
|
"time"
|
||||||
|
|
||||||
"github.com/fatih/color"
|
"github.com/fatih/color"
|
||||||
"github.com/spf13/cobra"
|
"github.com/spf13/cobra"
|
||||||
"github.com/steveyegge/beads/internal/storage"
|
"github.com/steveyegge/beads/internal/storage"
|
||||||
|
"github.com/steveyegge/beads/internal/storage/sqlite"
|
||||||
"github.com/steveyegge/beads/internal/types"
|
"github.com/steveyegge/beads/internal/types"
|
||||||
"github.com/steveyegge/beads/internal/utils"
|
"github.com/steveyegge/beads/internal/utils"
|
||||||
)
|
)
|
||||||
@@ -225,7 +228,7 @@ type issueSort struct {
|
|||||||
|
|
||||||
// repairPrefixes consolidates multiple prefixes into a single target prefix
|
// repairPrefixes consolidates multiple prefixes into a single target prefix
|
||||||
// Issues with the correct prefix are left unchanged.
|
// Issues with the correct prefix are left unchanged.
|
||||||
// Issues with incorrect prefixes are sorted and renumbered sequentially.
|
// Issues with incorrect prefixes get new hash-based IDs.
|
||||||
func repairPrefixes(ctx context.Context, st storage.Storage, actorName string, targetPrefix string, issues []*types.Issue, prefixes map[string]int, dryRun bool) error {
|
func repairPrefixes(ctx context.Context, st storage.Storage, actorName string, targetPrefix string, issues []*types.Issue, prefixes map[string]int, dryRun bool) error {
|
||||||
green := color.New(color.FgGreen).SprintFunc()
|
green := color.New(color.FgGreen).SprintFunc()
|
||||||
cyan := color.New(color.FgCyan).SprintFunc()
|
cyan := color.New(color.FgCyan).SprintFunc()
|
||||||
@@ -235,16 +238,12 @@ func repairPrefixes(ctx context.Context, st storage.Storage, actorName string, t
|
|||||||
var correctIssues []*types.Issue
|
var correctIssues []*types.Issue
|
||||||
var incorrectIssues []issueSort
|
var incorrectIssues []issueSort
|
||||||
|
|
||||||
maxCorrectNumber := 0
|
|
||||||
for _, issue := range issues {
|
for _, issue := range issues {
|
||||||
prefix := utils.ExtractIssuePrefix(issue.ID)
|
prefix := utils.ExtractIssuePrefix(issue.ID)
|
||||||
number := utils.ExtractIssueNumber(issue.ID)
|
number := utils.ExtractIssueNumber(issue.ID)
|
||||||
|
|
||||||
if prefix == targetPrefix {
|
if prefix == targetPrefix {
|
||||||
correctIssues = append(correctIssues, issue)
|
correctIssues = append(correctIssues, issue)
|
||||||
if number > maxCorrectNumber {
|
|
||||||
maxCorrectNumber = number
|
|
||||||
}
|
|
||||||
} else {
|
} else {
|
||||||
incorrectIssues = append(incorrectIssues, issueSort{
|
incorrectIssues = append(incorrectIssues, issueSort{
|
||||||
issue: issue,
|
issue: issue,
|
||||||
@@ -262,43 +261,58 @@ func repairPrefixes(ctx context.Context, st storage.Storage, actorName string, t
|
|||||||
return incorrectIssues[i].number < incorrectIssues[j].number
|
return incorrectIssues[i].number < incorrectIssues[j].number
|
||||||
})
|
})
|
||||||
|
|
||||||
|
// Get a database connection for ID generation
|
||||||
|
conn, err := st.UnderlyingConn(ctx)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to get database connection: %w", err)
|
||||||
|
}
|
||||||
|
defer func() { _ = conn.Close() }()
|
||||||
|
|
||||||
|
// Build a map of all renames for text replacement using hash IDs
|
||||||
|
// Track used IDs to avoid collisions within the batch
|
||||||
|
renameMap := make(map[string]string)
|
||||||
|
usedIDs := make(map[string]bool)
|
||||||
|
|
||||||
|
// Mark existing correct IDs as used
|
||||||
|
for _, issue := range correctIssues {
|
||||||
|
usedIDs[issue.ID] = true
|
||||||
|
}
|
||||||
|
|
||||||
|
// Generate hash IDs for all incorrect issues
|
||||||
|
for _, is := range incorrectIssues {
|
||||||
|
newID, err := generateRepairHashID(ctx, conn, targetPrefix, is.issue, actorName, usedIDs)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to generate hash ID for %s: %w", is.issue.ID, err)
|
||||||
|
}
|
||||||
|
renameMap[is.issue.ID] = newID
|
||||||
|
usedIDs[newID] = true
|
||||||
|
}
|
||||||
|
|
||||||
if dryRun {
|
if dryRun {
|
||||||
fmt.Printf("DRY RUN: Would repair %d issues with incorrect prefixes\n\n", len(incorrectIssues))
|
fmt.Printf("DRY RUN: Would repair %d issues with incorrect prefixes\n\n", len(incorrectIssues))
|
||||||
fmt.Printf("Issues with correct prefix (%s): %d (highest number: %d)\n", cyan(targetPrefix), len(correctIssues), maxCorrectNumber)
|
fmt.Printf("Issues with correct prefix (%s): %d\n", cyan(targetPrefix), len(correctIssues))
|
||||||
fmt.Printf("Issues to repair: %d\n\n", len(incorrectIssues))
|
fmt.Printf("Issues to repair: %d\n\n", len(incorrectIssues))
|
||||||
|
|
||||||
fmt.Printf("Planned renames (showing first 10):\n")
|
fmt.Printf("Planned renames (showing first 10):\n")
|
||||||
nextNumber := maxCorrectNumber + 1
|
|
||||||
for i, is := range incorrectIssues {
|
for i, is := range incorrectIssues {
|
||||||
if i >= 10 {
|
if i >= 10 {
|
||||||
fmt.Printf("... and %d more\n", len(incorrectIssues)-10)
|
fmt.Printf("... and %d more\n", len(incorrectIssues)-10)
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
oldID := is.issue.ID
|
oldID := is.issue.ID
|
||||||
newID := fmt.Sprintf("%s-%d", targetPrefix, nextNumber)
|
newID := renameMap[oldID]
|
||||||
fmt.Printf(" %s -> %s\n", yellow(oldID), cyan(newID))
|
fmt.Printf(" %s -> %s\n", yellow(oldID), cyan(newID))
|
||||||
nextNumber++
|
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// Perform the repairs
|
// Perform the repairs
|
||||||
fmt.Printf("Repairing database with multiple prefixes...\n")
|
fmt.Printf("Repairing database with multiple prefixes...\n")
|
||||||
fmt.Printf(" Issues with correct prefix (%s): %d (highest: %s-%d)\n",
|
fmt.Printf(" Issues with correct prefix (%s): %d\n", cyan(targetPrefix), len(correctIssues))
|
||||||
cyan(targetPrefix), len(correctIssues), targetPrefix, maxCorrectNumber)
|
|
||||||
fmt.Printf(" Issues to repair: %d\n\n", len(incorrectIssues))
|
fmt.Printf(" Issues to repair: %d\n\n", len(incorrectIssues))
|
||||||
|
|
||||||
oldPrefixPattern := regexp.MustCompile(`\b[a-z][a-z0-9-]*-(\d+)\b`)
|
// Pattern to match any issue ID reference in text (both hash and sequential IDs)
|
||||||
|
oldPrefixPattern := regexp.MustCompile(`\b[a-z][a-z0-9-]*-[a-z0-9]+\b`)
|
||||||
// Build a map of all renames for text replacement
|
|
||||||
renameMap := make(map[string]string)
|
|
||||||
nextNumber := maxCorrectNumber + 1
|
|
||||||
for _, is := range incorrectIssues {
|
|
||||||
oldID := is.issue.ID
|
|
||||||
newID := fmt.Sprintf("%s-%d", targetPrefix, nextNumber)
|
|
||||||
renameMap[oldID] = newID
|
|
||||||
nextNumber++
|
|
||||||
}
|
|
||||||
|
|
||||||
// Rename each issue
|
// Rename each issue
|
||||||
for _, is := range incorrectIssues {
|
for _, is := range incorrectIssues {
|
||||||
@@ -369,11 +383,10 @@ func repairPrefixes(ctx context.Context, st storage.Storage, actorName string, t
|
|||||||
|
|
||||||
if jsonOutput {
|
if jsonOutput {
|
||||||
result := map[string]interface{}{
|
result := map[string]interface{}{
|
||||||
"target_prefix": targetPrefix,
|
"target_prefix": targetPrefix,
|
||||||
"prefixes_found": len(prefixes),
|
"prefixes_found": len(prefixes),
|
||||||
"issues_repaired": len(incorrectIssues),
|
"issues_repaired": len(incorrectIssues),
|
||||||
"issues_unchanged": len(correctIssues),
|
"issues_unchanged": len(correctIssues),
|
||||||
"highest_number": nextNumber - 1,
|
|
||||||
}
|
}
|
||||||
enc := json.NewEncoder(os.Stdout)
|
enc := json.NewEncoder(os.Stdout)
|
||||||
enc.SetIndent("", " ")
|
enc.SetIndent("", " ")
|
||||||
@@ -434,6 +447,37 @@ func renamePrefixInDB(ctx context.Context, oldPrefix, newPrefix string, issues [
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// generateRepairHashID generates a hash-based ID for an issue during repair
|
||||||
|
// Uses the sqlite.GenerateIssueID function but also checks usedIDs for batch collision avoidance
|
||||||
|
func generateRepairHashID(ctx context.Context, conn *sql.Conn, prefix string, issue *types.Issue, actor string, usedIDs map[string]bool) (string, error) {
|
||||||
|
// Try to generate a unique ID using the standard generation function
|
||||||
|
// This handles collision detection against existing database IDs
|
||||||
|
newID, err := sqlite.GenerateIssueID(ctx, conn, prefix, issue, actor)
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check if this ID was already used in this batch
|
||||||
|
// If so, we need to generate a new one with a different timestamp
|
||||||
|
attempts := 0
|
||||||
|
for usedIDs[newID] && attempts < 100 {
|
||||||
|
// Slightly modify the creation time to get a different hash
|
||||||
|
modifiedIssue := *issue
|
||||||
|
modifiedIssue.CreatedAt = issue.CreatedAt.Add(time.Duration(attempts+1) * time.Nanosecond)
|
||||||
|
newID, err = sqlite.GenerateIssueID(ctx, conn, prefix, &modifiedIssue, actor)
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
attempts++
|
||||||
|
}
|
||||||
|
|
||||||
|
if usedIDs[newID] {
|
||||||
|
return "", fmt.Errorf("failed to generate unique ID after %d attempts", attempts)
|
||||||
|
}
|
||||||
|
|
||||||
|
return newID, nil
|
||||||
|
}
|
||||||
|
|
||||||
func init() {
|
func init() {
|
||||||
renamePrefixCmd.Flags().Bool("dry-run", false, "Preview changes without applying them")
|
renamePrefixCmd.Flags().Bool("dry-run", false, "Preview changes without applying them")
|
||||||
renamePrefixCmd.Flags().Bool("repair", false, "Repair database with multiple prefixes by consolidating them")
|
renamePrefixCmd.Flags().Bool("repair", false, "Repair database with multiple prefixes by consolidating them")
|
||||||
|
|||||||
@@ -93,19 +93,28 @@ func TestRepairMultiplePrefixes(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Verify the others were renumbered
|
// Verify the others were renamed with hash IDs (not sequential)
|
||||||
issue, err := store.GetIssue(ctx, "test-3")
|
// We have 5 total issues, 2 original (test-1, test-2), 3 renamed
|
||||||
if err != nil || issue == nil {
|
if len(allIssues) != 5 {
|
||||||
t.Fatalf("expected test-3 to exist (renamed from another-1)")
|
t.Fatalf("expected 5 issues total, got %d", len(allIssues))
|
||||||
}
|
}
|
||||||
|
|
||||||
issue, err = store.GetIssue(ctx, "test-4")
|
// Count issues with correct prefix and verify old IDs no longer exist
|
||||||
if err != nil || issue == nil {
|
testPrefixCount := 0
|
||||||
t.Fatalf("expected test-4 to exist (renamed from old-1)")
|
for _, issue := range allIssues {
|
||||||
|
if len(issue.ID) > 5 && issue.ID[:5] == "test-" {
|
||||||
|
testPrefixCount++
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if testPrefixCount != 5 {
|
||||||
|
t.Fatalf("expected all 5 issues to have 'test-' prefix, got %d", testPrefixCount)
|
||||||
}
|
}
|
||||||
|
|
||||||
issue, err = store.GetIssue(ctx, "test-5")
|
// Verify old IDs no longer exist
|
||||||
if err != nil || issue == nil {
|
for _, oldID := range []string{"old-1", "old-2", "another-1"} {
|
||||||
t.Fatalf("expected test-5 to exist (renamed from old-2)")
|
issue, err := store.GetIssue(ctx, oldID)
|
||||||
|
if err == nil && issue != nil {
|
||||||
|
t.Fatalf("expected old ID %s to no longer exist", oldID)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user