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

Remove spurious collision-related code after ultrathink review

Browse Source 
After 2 weeks of collision/stale-data fixes, reviewed all changes to identify
spurious code that is no longer needed after content-hash resolution was implemented.

**Removed:**
1. countReferences() function from collision.go (lines 274-328)
   - Was used for reference-count based collision scoring
   - Completely unused after switching to content-hash based resolution (commit 2e87329)
   - Still exists in duplicates.go for deduplication (different use case)

2. ReferenceScore field from CollisionDetail struct
   - Marked as DEPRECATED but never removed
   - No longer used by ScoreCollisions() which now uses content hashing

3. TestCountReferences and TestCountReferencesWordBoundary tests
   - Tested the now-deleted countReferences() function
   - No longer relevant

**Fixed:**
- Updated CheckpointWAL comments to remove misleading "staleness detection" claim
  - Staleness detection uses metadata (last_import_time), NOT file mtime
  - CheckpointWAL is still valuable for data persistence and WAL size reduction
  - Comments now accurately reflect actual benefits

**Verified:**
- All tests pass (internal/storage/sqlite)
- Content-hash collision resolution still works correctly
- No behavioral changes, just cleanup

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Steve Yegge
2025-10-28 20:03:04 -07:00
parent a51a9297d6
commit b0d28bbdbd
5 changed files with 25 additions and 232 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
internal/importer/importer.go
View File

@@ -111,8 +111,7 @@ func ImportIssues(ctx context.Context, dbPath string, store storage.Storage, iss
return nil, err
}
// Checkpoint WAL to update main .db file timestamp
// This ensures staleness detection sees the database as fresh
// Checkpoint WAL to ensure data persistence and reduce WAL file size
if err := sqliteStore.CheckpointWAL(ctx); err != nil {
// Non-fatal - just log warning
fmt.Fprintf(os.Stderr, "Warning: failed to checkpoint WAL: %v\n", err)

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

@@ -31,7 +31,6 @@ type CollisionDetail struct {
IncomingIssue *types.Issue // The issue from the import file
ExistingIssue *types.Issue // The issue currently in the database
ConflictingFields []string // List of field names that differ
ReferenceScore int // Number of references to this issue (for scoring) - DEPRECATED
RemapIncoming bool // If true, remap incoming; if false, remap existing
}
@@ -271,62 +270,6 @@ func ScoreCollisions(ctx context.Context, s *SQLiteStorage, collisions []*Collis
return nil
}
// countReferences counts how many times an issue ID is referenced
// Returns: text mentions + dependency references
func countReferences(issueID string, allIssues []*types.Issue, allDeps map[string][]*types.Dependency) (int, error) {
count := 0
// Count text mentions in all issues' text fields
// Use word boundary regex to match exact IDs (e.g., "bd-10" but not "bd-100")
pattern := fmt.Sprintf(`\b%s\b`, regexp.QuoteMeta(issueID))
re, err := regexp.Compile(pattern)
if err != nil {
return 0, fmt.Errorf("failed to compile regex for %s: %w", issueID, err)
}
for _, issue := range allIssues {
// Skip counting references in the issue itself
if issue.ID == issueID {
continue
}
// Count mentions in description
count += len(re.FindAllString(issue.Description, -1))
// Count mentions in design
count += len(re.FindAllString(issue.Design, -1))
// Count mentions in notes
count += len(re.FindAllString(issue.Notes, -1))
// Count mentions in acceptance criteria
count += len(re.FindAllString(issue.AcceptanceCriteria, -1))
}
// Count dependency references
// An issue can be referenced as either IssueID or DependsOnID
for _, deps := range allDeps {
for _, dep := range deps {
// Skip self-references
if dep.IssueID == issueID && dep.DependsOnID == issueID {
continue
}
// Count if this issue is the source (IssueID)
if dep.IssueID == issueID {
count++
}
// Count if this issue is the target (DependsOnID)
if dep.DependsOnID == issueID {
count++
}
}
}
return count, nil
}
// deduplicateIncomingIssues removes content-duplicate issues within the incoming batch
// Returns deduplicated slice, keeping the first issue ID (lexicographically) for each unique content
func deduplicateIncomingIssues(issues []*types.Issue) []*types.Issue {

189
internal/storage/sqlite/collision_test.go
View File

@@ -435,90 +435,6 @@ func intPtr(i int) *int {
return &i
}
func TestCountReferences(t *testing.T) {
allIssues := []*types.Issue{
{
ID: "bd-1",
Title: "Issue 1",
Description: "This mentions bd-2 and bd-3",
Design: "Design mentions bd-2 twice: bd-2 and bd-2",
Notes: "Notes mention bd-3",
},
{
ID: "bd-2",
Title: "Issue 2",
Description: "This mentions bd-1",
},
{
ID: "bd-3",
Title: "Issue 3",
Description: "No mentions here",
},
{
ID: "bd-10",
Title: "Issue 10",
Description: "This has bd-100 but not bd-10 itself",
},
}
allDeps := map[string][]*types.Dependency{
"bd-1": {
{IssueID: "bd-1", DependsOnID: "bd-2", Type: types.DepBlocks},
},
"bd-2": {
{IssueID: "bd-2", DependsOnID: "bd-3", Type: types.DepBlocks},
},
}
tests := []struct {
name string
issueID string
expectedCount int
}{
{
name: "bd-1 - one text mention, one dependency",
issueID: "bd-1",
// Text: bd-2's description mentions bd-1 (1)
// Deps: bd-1 → bd-2 (1)
expectedCount: 2,
},
{
name: "bd-2 - multiple text mentions, two dependencies",
issueID: "bd-2",
// Text: bd-1's description mentions bd-2 (1) + bd-1's design mentions bd-2 three times (3) = 4
// (design has: "mentions bd-2" + "bd-2 and" + "bd-2")
// Deps: bd-1 → bd-2 (1) + bd-2 → bd-3 (1) = 2
expectedCount: 6,
},
{
name: "bd-3 - some text mentions, one dependency",
issueID: "bd-3",
// Text: bd-1's description (1) + bd-1's notes (1) = 2
// Deps: bd-2 → bd-3 (1)
expectedCount: 3,
},
{
name: "bd-10 - no mentions (bd-100 doesn't count)",
issueID: "bd-10",
// Text: bd-100 in bd-10's description doesn't match \bbd-10\b = 0
// Deps: none = 0
expectedCount: 0,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
count, err := countReferences(tt.issueID, allIssues, allDeps)
if err != nil {
t.Fatalf("countReferences failed: %v", err)
}
if count != tt.expectedCount {
t.Errorf("expected count %d, got %d", tt.expectedCount, count)
}
})
}
}
func TestScoreCollisions(t *testing.T) {
// Create temporary database
tmpDir, err := os.MkdirTemp("", "score-collision-test-*")
@@ -607,28 +523,24 @@ func TestScoreCollisions(t *testing.T) {
// Create collision details (simulated)
collisions := []*CollisionDetail{
{
ID: "bd-1",
IncomingIssue: issue1,
ExistingIssue: issue1,
ReferenceScore: 0, // Will be calculated
ID: "bd-1",
IncomingIssue: issue1,
ExistingIssue: issue1,
},
{
ID: "bd-2",
IncomingIssue: issue2,
ExistingIssue: issue2,
ReferenceScore: 0, // Will be calculated
ID: "bd-2",
IncomingIssue: issue2,
ExistingIssue: issue2,
},
{
ID: "bd-3",
IncomingIssue: issue3,
ExistingIssue: issue3,
ReferenceScore: 0, // Will be calculated
ID: "bd-3",
IncomingIssue: issue3,
ExistingIssue: issue3,
},
{
ID: "bd-4",
IncomingIssue: issue4,
ExistingIssue: issue4,
ReferenceScore: 0, // Will be calculated
ID: "bd-4",
IncomingIssue: issue4,
ExistingIssue: issue4,
},
}
@@ -656,67 +568,6 @@ func TestScoreCollisions(t *testing.T) {
}
}
func TestCountReferencesWordBoundary(t *testing.T) {
// Test that word boundaries work correctly
allIssues := []*types.Issue{
{
ID: "bd-1",
Description: "bd-10 and bd-100 and bd-1 and bd-11",
},
{
ID: "bd-10",
Description: "bd-1 and bd-100",
},
}
allDeps := map[string][]*types.Dependency{}
tests := []struct {
name string
issueID string
expectedCount int
description string
}{
{
name: "bd-1 exact match",
issueID: "bd-1",
expectedCount: 2, // bd-10's desc mentions bd-1 (1) + bd-1's desc mentions bd-1 (1) = 2
// Wait, bd-1's desc shouldn't count itself
// So: bd-10's desc mentions bd-1 (1)
},
{
name: "bd-10 exact match",
issueID: "bd-10",
expectedCount: 1, // bd-1's desc mentions bd-10 (1)
},
{
name: "bd-100 exact match",
issueID: "bd-100",
expectedCount: 2, // bd-1's desc (1) + bd-10's desc (1)
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
count, err := countReferences(tt.issueID, allIssues, allDeps)
if err != nil {
t.Fatalf("countReferences failed: %v", err)
}
// Adjust expected based on actual counting logic
// countReferences skips the issue itself
expected := tt.expectedCount
if tt.issueID == testIssueBD1 {
expected = 1 // only bd-10's description
}
if count != expected {
t.Errorf("expected count %d, got %d", expected, count)
}
})
}
}
func TestReplaceIDReferences(t *testing.T) {
tests := []struct {
name string
@@ -843,33 +694,31 @@ func TestRemapCollisions(t *testing.T) {
// Create collisions (incoming issues with same IDs as DB but different content)
collision1 := &CollisionDetail{
ID: "bd-2",
ID: "bd-2",
ExistingIssue: dbIssue2,
IncomingIssue: &types.Issue{
ID: "bd-2",
Title: "Collision 2 (has fewer references)",
Title: "Collision 2",
Description: "This is different content",
Status: types.StatusOpen,
Priority: 1,
IssueType: types.TypeTask,
},
RemapIncoming: true, // Incoming will be remapped
ReferenceScore: 2, // Fewer references
RemapIncoming: true, // Incoming will be remapped
}
collision2 := &CollisionDetail{
ID: "bd-3",
ID: "bd-3",
ExistingIssue: dbIssue3,
IncomingIssue: &types.Issue{
ID: "bd-3",
Title: "Collision 3 (has more references)",
Title: "Collision 3",
Description: "Different content for bd-3",
Status: types.StatusOpen,
Priority: 1,
IssueType: types.TypeTask,
},
RemapIncoming: true, // Incoming will be remapped
ReferenceScore: 5, // More references
RemapIncoming: true, // Incoming will be remapped
}
collisions := []*CollisionDetail{collision1, collision2}
@@ -903,7 +752,7 @@ func TestRemapCollisions(t *testing.T) {
if remappedIssue2 == nil {
t.Fatalf("remapped issue %s not found", newID2)
}
if remappedIssue2.Title != "Collision 2 (has fewer references)" {
if remappedIssue2.Title != "Collision 2" {
t.Errorf("unexpected title for remapped issue: %s", remappedIssue2.Title)
}

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

@@ -2301,9 +2301,11 @@ func (s *SQLiteStorage) UnderlyingConn(ctx context.Context) (*sql.Conn, error) {
}
// CheckpointWAL checkpoints the WAL file to flush changes to the main database file.
// This updates the main .db file's modification time, which is important for staleness detection.
// In WAL mode, writes go to the -wal file, leaving the main .db file untouched.
// Checkpointing flushes the WAL to the main database file.
// Checkpointing:
// - Ensures data persistence by flushing WAL to main database
// - Reduces WAL file size
// - Makes database safe for backup/copy operations
func (s *SQLiteStorage) CheckpointWAL(ctx context.Context) error {
_, err := s.db.ExecContext(ctx, "PRAGMA wal_checkpoint(FULL)")
return err