From 431792b633d6ce183423490e16e90537d81e35e7 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Tue, 14 Oct 2025 03:12:00 -0700 Subject: [PATCH] fix: Deduplicate nodes in bd dep tree for diamond dependencies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes bd-85 (GH-1): bd dep tree was showing duplicate nodes when multiple paths existed to the same issue (diamond dependencies). Changes: - Add path tracking in recursive CTE to detect cycles - Add cycle prevention via path LIKE check - Add Go-side deduplication using seen map - Show each node only once at its shallowest depth The fix maintains backward compatibility and passes all 37 tests. Created follow-up issues: - bd-164: Add visual indicators for multi-parent nodes - bd-165: Add --show-all-paths flag - bd-166: Make maxDepth configurable 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/storage/sqlite/dependencies.go | 44 +++++++++++++++++++++---- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/internal/storage/sqlite/dependencies.go b/internal/storage/sqlite/dependencies.go index 8cdfe1c5..95efe2a5 100644 --- a/internal/storage/sqlite/dependencies.go +++ b/internal/storage/sqlite/dependencies.go @@ -254,13 +254,16 @@ func (s *SQLiteStorage) GetAllDependencyRecords(ctx context.Context) (map[string return depsMap, nil } -// GetDependencyTree returns the full dependency tree +// GetDependencyTree returns the full dependency tree with deduplication +// When multiple paths lead to the same node (diamond dependencies), the node +// appears only once at its shallowest depth in the tree. func (s *SQLiteStorage) GetDependencyTree(ctx context.Context, issueID string, maxDepth int) ([]*types.TreeNode, error) { if maxDepth <= 0 { maxDepth = 50 } - // Use recursive CTE to build tree + // First, build the complete tree with all paths using recursive CTE + // We need to track the full path to handle proper tree structure rows, err := s.db.QueryContext(ctx, ` WITH RECURSIVE tree AS ( SELECT @@ -268,7 +271,9 @@ func (s *SQLiteStorage) GetDependencyTree(ctx context.Context, issueID string, m i.acceptance_criteria, i.notes, i.issue_type, i.assignee, i.estimated_minutes, i.created_at, i.updated_at, i.closed_at, i.external_ref, - 0 as depth + 0 as depth, + i.id as path, + i.id as parent_id FROM issues i WHERE i.id = ? @@ -279,37 +284,51 @@ func (s *SQLiteStorage) GetDependencyTree(ctx context.Context, issueID string, m i.acceptance_criteria, i.notes, i.issue_type, i.assignee, i.estimated_minutes, i.created_at, i.updated_at, i.closed_at, i.external_ref, - t.depth + 1 + t.depth + 1, + t.path || '→' || i.id, + t.id FROM issues i JOIN dependencies d ON i.id = d.depends_on_id JOIN tree t ON d.issue_id = t.id WHERE t.depth < ? + AND t.path NOT LIKE '%' || i.id || '%' ) - SELECT * FROM tree - ORDER BY depth, priority + SELECT id, title, status, priority, description, design, + acceptance_criteria, notes, issue_type, assignee, + estimated_minutes, created_at, updated_at, closed_at, + external_ref, depth, parent_id + FROM tree + ORDER BY depth, priority, id `, issueID, maxDepth) if err != nil { return nil, fmt.Errorf("failed to get dependency tree: %w", err) } defer rows.Close() + // Use a map to track nodes we've seen and deduplicate + // Key: issue ID, Value: minimum depth where we saw it + seen := make(map[string]int) var nodes []*types.TreeNode + for rows.Next() { var node types.TreeNode var closedAt sql.NullTime var estimatedMinutes sql.NullInt64 var assignee sql.NullString var externalRef sql.NullString + var parentID string // Currently unused, but available for future parent relationship display err := rows.Scan( &node.ID, &node.Title, &node.Status, &node.Priority, &node.Description, &node.Design, &node.AcceptanceCriteria, &node.Notes, &node.IssueType, &assignee, &estimatedMinutes, - &node.CreatedAt, &node.UpdatedAt, &closedAt, &externalRef, &node.Depth, + &node.CreatedAt, &node.UpdatedAt, &closedAt, &externalRef, + &node.Depth, &parentID, ) if err != nil { return nil, fmt.Errorf("failed to scan tree node: %w", err) } + _ = parentID // Silence unused variable warning if closedAt.Valid { node.ClosedAt = &closedAt.Time @@ -327,6 +346,17 @@ func (s *SQLiteStorage) GetDependencyTree(ctx context.Context, issueID string, m node.Truncated = node.Depth == maxDepth + // Deduplicate: only include a node the first time we see it (shallowest depth) + // Since we ORDER BY depth, priority, id - the first occurrence is at minimum depth + if prevDepth, exists := seen[node.ID]; exists { + // We've seen this node before at depth prevDepth + // Skip this duplicate occurrence + _ = prevDepth // Avoid unused variable warning + continue + } + + // Mark this node as seen at this depth + seen[node.ID] = node.Depth nodes = append(nodes, &node) }