johno/beads
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove sequential ID generation and SyncAllCounters (bd-c7af, bd-8e05, bd-4c74)

Browse Source 
- Removed SyncAllCounters() and all call sites (already no-op with hash IDs)
- Removed AllocateNextID() and getNextIDForPrefix() - sequential ID generation
- Removed collision remapping logic in internal/storage/sqlite/collision.go
- Removed rename collision handling in internal/importer/importer.go
- Removed branch-merge example (collision resolution no longer needed)
- Updated EXTENDING.md to remove counter sync examples

These were all deprecated code paths for sequential IDs that are obsolete
with hash-based IDs. Hash ID collisions are handled by extending the hash,
not by remapping to new sequential IDs.
This commit is contained in:
Steve Yegge
2025-10-30 22:24:42 -07:00
parent 4a21005a31
commit 5d137ffeeb
11 changed files with 40 additions and 448 deletions
Download Patch File Download Diff File Expand all files Collapse all files

56
internal/storage/sqlite/collision.go
View File

@@ -353,11 +353,7 @@ func RemapCollisions(ctx context.Context, s *SQLiteStorage, collisions []*Collis
return nil, err
}
if attempt < maxRetries-1 {
if syncErr := s.SyncAllCounters(ctx); syncErr != nil {
return nil, fmt.Errorf("retry %d: UNIQUE constraint error, counter sync failed: %w (original error: %v)", attempt+1, syncErr, err)
}
}
// REMOVED (bd-c7af): Counter sync on retry - no longer needed with hash IDs
}
return nil, fmt.Errorf("failed after %d retries due to UNIQUE constraint violations: %w", maxRetries, lastErr)
@@ -365,44 +361,27 @@ func RemapCollisions(ctx context.Context, s *SQLiteStorage, collisions []*Collis
// remapCollisionsOnce performs a single attempt at collision resolution.
// This is the actual implementation that RemapCollisions wraps with retry logic.
// REMOVED (bd-8e05): With hash-based IDs, collision remapping is no longer needed.
func remapCollisionsOnce(ctx context.Context, s *SQLiteStorage, collisions []*CollisionDetail, _ []*types.Issue) (map[string]string, error) {
idMapping := make(map[string]string)
// Sync counters before remapping to avoid ID collisions
if err := s.SyncAllCounters(ctx); err != nil {
return nil, fmt.Errorf("failed to sync ID counters: %w", err)
// With hash-based IDs, collisions should not occur. If they do, it's a bug.
if len(collisions) > 0 {
return nil, fmt.Errorf("collision remapping no longer supported with hash IDs - %d collisions detected", len(collisions))
}
return nil, nil
}
// Step 1: Collect ALL dependencies before any modifications
// This prevents CASCADE DELETE from losing dependency information
allDepsBeforeRemap, err := s.GetAllDependencyRecords(ctx)
if err != nil {
return nil, fmt.Errorf("failed to get all dependencies: %w", err)
}
// OLD IMPLEMENTATION REMOVED (bd-8e05) - retained for reference during migration
// The original 250+ line function implemented sequential ID-based collision remapping
// which is obsolete with hash-based IDs
// Step 2: Process each collision based on which version should be remapped
for _, collision := range collisions {
// Skip collisions with nil issues (shouldn't happen but be defensive)
if collision.IncomingIssue == nil {
return nil, fmt.Errorf("collision %s has nil IncomingIssue", collision.ID)
}
if collision.ExistingIssue == nil {
return nil, fmt.Errorf("collision %s has nil ExistingIssue", collision.ID)
}
oldID := collision.ID
// Allocate new ID using atomic counter
prefix, err := s.GetConfig(ctx, "issue_prefix")
if err != nil || prefix == "" {
prefix = "bd"
}
nextID, err := s.getNextIDForPrefix(ctx, prefix)
if err != nil {
return nil, fmt.Errorf("failed to generate new ID for collision %s: %w", oldID, err)
}
newID := fmt.Sprintf("%s-%d", prefix, nextID)
// Stub out the old implementation to avoid compile errors
// The actual 250+ line implementation has been removed (bd-8e05)
func _OLD_remapCollisionsOnce_REMOVED(ctx context.Context, s *SQLiteStorage, collisions []*CollisionDetail, _ []*types.Issue) (map[string]string, error) {
// Original implementation removed - see git history before bd-8e05
return nil, nil
}
/* OLD CODE REMOVED (bd-8e05) - kept for git history reference
if collision.RemapIncoming {
// Incoming has higher hash -> remap incoming, keep existing
// Record mapping
@@ -498,6 +477,7 @@ func remapCollisionsOnce(ctx context.Context, s *SQLiteStorage, collisions []*Co
return idMapping, nil
}
END OF REMOVED CODE */
// updateReferences updates all text field references and dependency records
// to point to new IDs based on the idMapping

47
internal/storage/sqlite/sqlite.go
View File

@@ -556,32 +556,8 @@ func migrateContentHashColumn(db *sql.DB) error {
return nil
}
// getNextIDForPrefix atomically generates the next ID for a given prefix
// Uses the issue_counters table for atomic, cross-process ID generation
func (s *SQLiteStorage) getNextIDForPrefix(ctx context.Context, prefix string) (int, error) {
var nextID int
err := s.db.QueryRowContext(ctx, `
INSERT INTO issue_counters (prefix, last_id)
VALUES (?, 1)
ON CONFLICT(prefix) DO UPDATE SET
last_id = last_id + 1
RETURNING last_id
`, prefix).Scan(&nextID)
if err != nil {
return 0, fmt.Errorf("failed to generate next ID for prefix %s: %w", prefix, err)
}
return nextID, nil
}
// AllocateNextID generates the next issue ID for a given prefix.
// This is a public wrapper around getNextIDForPrefix for use by other packages.
func (s *SQLiteStorage) AllocateNextID(ctx context.Context, prefix string) (string, error) {
nextID, err := s.getNextIDForPrefix(ctx, prefix)
if err != nil {
return "", err
}
return fmt.Sprintf("%s-%d", prefix, nextID), nil
}
// REMOVED (bd-8e05): getNextIDForPrefix and AllocateNextID - sequential ID generation
// no longer needed with hash-based IDs
// getNextChildNumber atomically generates the next child number for a parent ID
// Uses the child_counters table for atomic, cross-process child ID generation
@@ -631,12 +607,7 @@ func (s *SQLiteStorage) GetNextChildID(ctx context.Context, parentID string) (st
return childID, nil
}
// SyncAllCounters is a no-op now that sequential IDs are removed (bd-aa744b).
// Kept for backward compatibility with existing code that calls it.
func (s *SQLiteStorage) SyncAllCounters(ctx context.Context) error {
// No-op: hash IDs don't use counters
return nil
}
// REMOVED (bd-c7af): SyncAllCounters - no longer needed with hash IDs
// REMOVED (bd-166): derivePrefixFromPath was causing duplicate issues with wrong prefix
// The database should ALWAYS have issue_prefix config set explicitly (by 'bd init' or auto-import)
@@ -1081,9 +1052,7 @@ func bulkMarkDirty(ctx context.Context, conn *sql.Conn, issues []*types.Issue) e
// }
//
// // After importing with explicit IDs, sync counters to prevent collisions
// if err := store.SyncAllCounters(ctx); err != nil {
// return err
// }
// REMOVED (bd-c7af): SyncAllCounters example - no longer needed with hash IDs
//
// Performance:
// - 100 issues: ~30ms (vs ~900ms with CreateIssue loop)
@@ -1727,8 +1696,8 @@ func (s *SQLiteStorage) DeleteIssue(ctx context.Context, id string) error {
return err
}
// Sync counters after deletion to keep them accurate
return s.SyncAllCounters(ctx)
// REMOVED (bd-c7af): Counter sync after deletion - no longer needed with hash IDs
return nil
}
// DeleteIssuesResult contains statistics about a batch deletion operation
@@ -1781,9 +1750,7 @@ func (s *SQLiteStorage) DeleteIssues(ctx context.Context, ids []string, cascade
return nil, fmt.Errorf("failed to commit transaction: %w", err)
}
if err := s.SyncAllCounters(ctx); err != nil {
return nil, fmt.Errorf("failed to sync counters after deletion: %w", err)
}
// REMOVED (bd-c7af): Counter sync after deletion - no longer needed with hash IDs
return result, nil
}