fix: Deduplicate nodes in bd dep tree for diamond dependencies
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 <noreply@anthropic.com>
This commit is contained in:
@@ -254,13 +254,16 @@ func (s *SQLiteStorage) GetAllDependencyRecords(ctx context.Context) (map[string
|
|||||||
return depsMap, nil
|
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) {
|
func (s *SQLiteStorage) GetDependencyTree(ctx context.Context, issueID string, maxDepth int) ([]*types.TreeNode, error) {
|
||||||
if maxDepth <= 0 {
|
if maxDepth <= 0 {
|
||||||
maxDepth = 50
|
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, `
|
rows, err := s.db.QueryContext(ctx, `
|
||||||
WITH RECURSIVE tree AS (
|
WITH RECURSIVE tree AS (
|
||||||
SELECT
|
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.acceptance_criteria, i.notes, i.issue_type, i.assignee,
|
||||||
i.estimated_minutes, i.created_at, i.updated_at, i.closed_at,
|
i.estimated_minutes, i.created_at, i.updated_at, i.closed_at,
|
||||||
i.external_ref,
|
i.external_ref,
|
||||||
0 as depth
|
0 as depth,
|
||||||
|
i.id as path,
|
||||||
|
i.id as parent_id
|
||||||
FROM issues i
|
FROM issues i
|
||||||
WHERE i.id = ?
|
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.acceptance_criteria, i.notes, i.issue_type, i.assignee,
|
||||||
i.estimated_minutes, i.created_at, i.updated_at, i.closed_at,
|
i.estimated_minutes, i.created_at, i.updated_at, i.closed_at,
|
||||||
i.external_ref,
|
i.external_ref,
|
||||||
t.depth + 1
|
t.depth + 1,
|
||||||
|
t.path || '→' || i.id,
|
||||||
|
t.id
|
||||||
FROM issues i
|
FROM issues i
|
||||||
JOIN dependencies d ON i.id = d.depends_on_id
|
JOIN dependencies d ON i.id = d.depends_on_id
|
||||||
JOIN tree t ON d.issue_id = t.id
|
JOIN tree t ON d.issue_id = t.id
|
||||||
WHERE t.depth < ?
|
WHERE t.depth < ?
|
||||||
|
AND t.path NOT LIKE '%' || i.id || '%'
|
||||||
)
|
)
|
||||||
SELECT * FROM tree
|
SELECT id, title, status, priority, description, design,
|
||||||
ORDER BY depth, priority
|
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)
|
`, issueID, maxDepth)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("failed to get dependency tree: %w", err)
|
return nil, fmt.Errorf("failed to get dependency tree: %w", err)
|
||||||
}
|
}
|
||||||
defer rows.Close()
|
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
|
var nodes []*types.TreeNode
|
||||||
|
|
||||||
for rows.Next() {
|
for rows.Next() {
|
||||||
var node types.TreeNode
|
var node types.TreeNode
|
||||||
var closedAt sql.NullTime
|
var closedAt sql.NullTime
|
||||||
var estimatedMinutes sql.NullInt64
|
var estimatedMinutes sql.NullInt64
|
||||||
var assignee sql.NullString
|
var assignee sql.NullString
|
||||||
var externalRef sql.NullString
|
var externalRef sql.NullString
|
||||||
|
var parentID string // Currently unused, but available for future parent relationship display
|
||||||
|
|
||||||
err := rows.Scan(
|
err := rows.Scan(
|
||||||
&node.ID, &node.Title, &node.Status, &node.Priority,
|
&node.ID, &node.Title, &node.Status, &node.Priority,
|
||||||
&node.Description, &node.Design, &node.AcceptanceCriteria,
|
&node.Description, &node.Design, &node.AcceptanceCriteria,
|
||||||
&node.Notes, &node.IssueType, &assignee, &estimatedMinutes,
|
&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 {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("failed to scan tree node: %w", err)
|
return nil, fmt.Errorf("failed to scan tree node: %w", err)
|
||||||
}
|
}
|
||||||
|
_ = parentID // Silence unused variable warning
|
||||||
|
|
||||||
if closedAt.Valid {
|
if closedAt.Valid {
|
||||||
node.ClosedAt = &closedAt.Time
|
node.ClosedAt = &closedAt.Time
|
||||||
@@ -327,6 +346,17 @@ func (s *SQLiteStorage) GetDependencyTree(ctx context.Context, issueID string, m
|
|||||||
|
|
||||||
node.Truncated = node.Depth == maxDepth
|
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)
|
nodes = append(nodes, &node)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user