From 89842aa7bdf398da38e0a925e5ca733e25a79c90 Mon Sep 17 00:00:00 2001 From: Jacob Chapel Date: Tue, 16 Dec 2025 16:22:05 -0800 Subject: [PATCH] fix(storage): calculate blocked/ready counts in memory store GetStatistics (#587) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MemoryStorage.GetStatistics() was returning 0 for blocked_issues and ready_issues in no-db mode. The SQLite implementation correctly calculated these based on dependencies, but the memory store only counted explicit status values. Changes: - Reuse getOpenBlockers() helper for dependency-based blocked count - Calculate ready_issues as open issues with no open blockers - Add tombstone handling (exclude from TotalIssues) - Add AverageLeadTime calculation for parity with SQLite - Add EpicsEligibleForClosure calculation - Add tests for blocked/ready counts, epics, and tombstones 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- internal/storage/memory/memory.go | 88 +++++++++++- internal/storage/memory/memory_test.go | 192 +++++++++++++++++++++++++ 2 files changed, 275 insertions(+), 5 deletions(-) diff --git a/internal/storage/memory/memory.go b/internal/storage/memory/memory.go index 4d475baa..b73e3fe2 100644 --- a/internal/storage/memory/memory.go +++ b/internal/storage/memory/memory.go @@ -1202,26 +1202,104 @@ func (m *MemoryStorage) GetStatistics(ctx context.Context) (*types.Statistics, e m.mu.RLock() defer m.mu.RUnlock() - stats := &types.Statistics{ - TotalIssues: len(m.issues), - } + stats := &types.Statistics{} + // First pass: count by status for _, issue := range m.issues { switch issue.Status { case types.StatusOpen: stats.OpenIssues++ case types.StatusInProgress: stats.InProgressIssues++ - case types.StatusBlocked: - stats.BlockedIssues++ case types.StatusClosed: stats.ClosedIssues++ + case types.StatusTombstone: + stats.TombstoneIssues++ } } + // TotalIssues excludes tombstones (matches SQLite behavior) + stats.TotalIssues = stats.OpenIssues + stats.InProgressIssues + stats.ClosedIssues + + // Second pass: calculate blocked and ready issues based on dependencies + // An issue is blocked if it has open blockers (uses same logic as GetBlockedIssues) + for id, issue := range m.issues { + // Only consider non-closed, non-tombstone issues for blocking + if issue.Status == types.StatusClosed || issue.Status == types.StatusTombstone { + continue + } + + blockers := m.getOpenBlockers(id) + if len(blockers) > 0 { + stats.BlockedIssues++ + } else if issue.Status == types.StatusOpen { + // Ready = open issues with no open blockers + stats.ReadyIssues++ + } + } + + // Calculate average lead time (hours from created to closed) + var totalLeadTime float64 + var closedCount int + for _, issue := range m.issues { + if issue.Status == types.StatusClosed && issue.ClosedAt != nil { + leadTime := issue.ClosedAt.Sub(issue.CreatedAt).Hours() + totalLeadTime += leadTime + closedCount++ + } + } + if closedCount > 0 { + stats.AverageLeadTime = totalLeadTime / float64(closedCount) + } + + // Calculate epics eligible for closure + stats.EpicsEligibleForClosure = m.countEpicsEligibleForClosure() + return stats, nil } +// countEpicsEligibleForClosure returns the count of non-closed epics where all children are closed +func (m *MemoryStorage) countEpicsEligibleForClosure() int { + // Build a map of epic -> children using parent-child dependencies + epicChildren := make(map[string][]string) + for _, deps := range m.dependencies { + for _, dep := range deps { + if dep.Type == types.DepParentChild { + // dep.IssueID is the child, dep.DependsOnID is the parent + epicChildren[dep.DependsOnID] = append(epicChildren[dep.DependsOnID], dep.IssueID) + } + } + } + + count := 0 + for epicID, children := range epicChildren { + epic, exists := m.issues[epicID] + if !exists { + continue + } + // Only consider non-closed epics + if epic.IssueType != types.TypeEpic || epic.Status == types.StatusClosed { + continue + } + // Check if all children are closed + if len(children) == 0 { + continue + } + allClosed := true + for _, childID := range children { + child, exists := m.issues[childID] + if !exists || child.Status != types.StatusClosed { + allClosed = false + break + } + } + if allClosed { + count++ + } + } + return count +} + // Dirty tracking func (m *MemoryStorage) GetDirtyIssues(ctx context.Context) ([]string, error) { m.mu.RLock() diff --git a/internal/storage/memory/memory_test.go b/internal/storage/memory/memory_test.go index 41d6e811..ecbe1063 100644 --- a/internal/storage/memory/memory_test.go +++ b/internal/storage/memory/memory_test.go @@ -757,6 +757,198 @@ func TestStatistics(t *testing.T) { } } +func TestStatistics_BlockedAndReadyCounts(t *testing.T) { + store := setupTestMemory(t) + defer store.Close() + + ctx := context.Background() + closedAt := time.Now() + + // Create issues: + // - blocker: open issue that blocks others + // - blocked1: open issue blocked by blocker + // - blocked2: in_progress issue blocked by blocker + // - ready1: open issue with no blockers + // - ready2: open issue "blocked" by a closed issue (should be ready) + // - closedBlocker: closed issue that doesn't block + blocker := &types.Issue{Title: "Blocker", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + blocked1 := &types.Issue{Title: "Blocked Open", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + blocked2 := &types.Issue{Title: "Blocked InProgress", Status: types.StatusInProgress, Priority: 1, IssueType: types.TypeTask} + ready1 := &types.Issue{Title: "Ready 1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + ready2 := &types.Issue{Title: "Ready 2 (closed blocker)", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + closedBlocker := &types.Issue{Title: "Closed Blocker", Status: types.StatusClosed, Priority: 1, IssueType: types.TypeTask, ClosedAt: &closedAt} + + for _, issue := range []*types.Issue{blocker, blocked1, blocked2, ready1, ready2, closedBlocker} { + if err := store.CreateIssue(ctx, issue, "test"); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + } + + // Close the closedBlocker properly + if err := store.CloseIssue(ctx, closedBlocker.ID, "Done", "test"); err != nil { + t.Fatalf("CloseIssue failed: %v", err) + } + + // Add blocking dependencies + // blocked1 is blocked by blocker (open) + if err := store.AddDependency(ctx, &types.Dependency{ + IssueID: blocked1.ID, + DependsOnID: blocker.ID, + Type: types.DepBlocks, + CreatedAt: time.Now(), + CreatedBy: "test", + }, "test"); err != nil { + t.Fatalf("AddDependency failed: %v", err) + } + + // blocked2 is blocked by blocker (open) + if err := store.AddDependency(ctx, &types.Dependency{ + IssueID: blocked2.ID, + DependsOnID: blocker.ID, + Type: types.DepBlocks, + CreatedAt: time.Now(), + CreatedBy: "test", + }, "test"); err != nil { + t.Fatalf("AddDependency failed: %v", err) + } + + // ready2 is "blocked" by closedBlocker (closed, so doesn't actually block) + if err := store.AddDependency(ctx, &types.Dependency{ + IssueID: ready2.ID, + DependsOnID: closedBlocker.ID, + Type: types.DepBlocks, + CreatedAt: time.Now(), + CreatedBy: "test", + }, "test"); err != nil { + t.Fatalf("AddDependency failed: %v", err) + } + + stats, err := store.GetStatistics(ctx) + if err != nil { + t.Fatalf("GetStatistics failed: %v", err) + } + + // Expected: + // - BlockedIssues: 2 (blocked1 and blocked2) + // - ReadyIssues: 3 (blocker, ready1, ready2 - all open with no open blockers) + if stats.BlockedIssues != 2 { + t.Errorf("Expected 2 blocked issues, got %d", stats.BlockedIssues) + } + if stats.ReadyIssues != 3 { + t.Errorf("Expected 3 ready issues, got %d", stats.ReadyIssues) + } + + // Verify other counts are correct + if stats.TotalIssues != 6 { + t.Errorf("Expected 6 total issues, got %d", stats.TotalIssues) + } + if stats.OpenIssues != 4 { + t.Errorf("Expected 4 open issues, got %d", stats.OpenIssues) + } + if stats.InProgressIssues != 1 { + t.Errorf("Expected 1 in-progress issue, got %d", stats.InProgressIssues) + } + if stats.ClosedIssues != 1 { + t.Errorf("Expected 1 closed issue, got %d", stats.ClosedIssues) + } +} + +func TestStatistics_EpicsEligibleForClosure(t *testing.T) { + store := setupTestMemory(t) + defer store.Close() + + ctx := context.Background() + closedAt := time.Now() + + // Create an epic with two children, both closed + epic1 := &types.Issue{Title: "Epic 1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeEpic} + child1 := &types.Issue{Title: "Child 1", Status: types.StatusClosed, Priority: 1, IssueType: types.TypeTask, ClosedAt: &closedAt} + child2 := &types.Issue{Title: "Child 2", Status: types.StatusClosed, Priority: 1, IssueType: types.TypeTask, ClosedAt: &closedAt} + + // Create an epic with one open child (not eligible) + epic2 := &types.Issue{Title: "Epic 2", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeEpic} + child3 := &types.Issue{Title: "Child 3", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + + for _, issue := range []*types.Issue{epic1, child1, child2, epic2, child3} { + if err := store.CreateIssue(ctx, issue, "test"); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + } + + // Close the children properly + for _, child := range []*types.Issue{child1, child2} { + if err := store.CloseIssue(ctx, child.ID, "Done", "test"); err != nil { + t.Fatalf("CloseIssue failed: %v", err) + } + } + + // Add parent-child dependencies + for _, dep := range []*types.Dependency{ + {IssueID: child1.ID, DependsOnID: epic1.ID, Type: types.DepParentChild, CreatedAt: time.Now(), CreatedBy: "test"}, + {IssueID: child2.ID, DependsOnID: epic1.ID, Type: types.DepParentChild, CreatedAt: time.Now(), CreatedBy: "test"}, + {IssueID: child3.ID, DependsOnID: epic2.ID, Type: types.DepParentChild, CreatedAt: time.Now(), CreatedBy: "test"}, + } { + if err := store.AddDependency(ctx, dep, "test"); err != nil { + t.Fatalf("AddDependency failed: %v", err) + } + } + + stats, err := store.GetStatistics(ctx) + if err != nil { + t.Fatalf("GetStatistics failed: %v", err) + } + + // Only epic1 should be eligible (all children closed) + if stats.EpicsEligibleForClosure != 1 { + t.Errorf("Expected 1 epic eligible for closure, got %d", stats.EpicsEligibleForClosure) + } +} + +func TestStatistics_TombstonesExcludedFromTotal(t *testing.T) { + store := setupTestMemory(t) + defer store.Close() + + ctx := context.Background() + deletedAt := time.Now() + + // Create 2 regular issues and 1 tombstone + issues := []*types.Issue{ + {Title: "Open Issue", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask}, + {Title: "Closed Issue", Status: types.StatusClosed, Priority: 1, IssueType: types.TypeTask, ClosedAt: &deletedAt}, + {Title: "Tombstone Issue", Status: types.StatusTombstone, Priority: 1, IssueType: types.TypeTask, DeletedAt: &deletedAt, DeletedBy: "test"}, + } + + for _, issue := range issues { + if err := store.CreateIssue(ctx, issue, "test"); err != nil { + t.Fatalf("CreateIssue failed: %v", err) + } + } + + // Close the closed issue properly + if err := store.CloseIssue(ctx, issues[1].ID, "Done", "test"); err != nil { + t.Fatalf("CloseIssue failed: %v", err) + } + + stats, err := store.GetStatistics(ctx) + if err != nil { + t.Fatalf("GetStatistics failed: %v", err) + } + + // Tombstone should be excluded from total but counted separately + if stats.TotalIssues != 2 { + t.Errorf("Expected 2 total issues (excluding tombstone), got %d", stats.TotalIssues) + } + if stats.TombstoneIssues != 1 { + t.Errorf("Expected 1 tombstone issue, got %d", stats.TombstoneIssues) + } + if stats.OpenIssues != 1 { + t.Errorf("Expected 1 open issue, got %d", stats.OpenIssues) + } + if stats.ClosedIssues != 1 { + t.Errorf("Expected 1 closed issue, got %d", stats.ClosedIssues) + } +} + func TestConfigOperations(t *testing.T) { store := setupTestMemory(t) defer store.Close()