feat: add reverse mode to dependency tree traversal
- Add reverse parameter to GetDependencyTree interface - Implement reverse SQL traversal (dependents vs dependencies) - Add comprehensive test for reverse mode (TDD: test-first approach) - Update existing test calls with reverse=false for backward compatibility Reverse mode inverts tree direction to show dependents instead of dependencies: - Normal: JOIN dependencies d ON i.id = d.depends_on_id (traverse UP) - Reverse: JOIN dependencies d ON i.id = d.issue_id (traverse DOWN) All storage tests passing. No regressions.
This commit is contained in:
@@ -462,14 +462,57 @@ func (s *SQLiteStorage) GetAllDependencyRecords(ctx context.Context) (map[string
|
|||||||
// When showAllPaths is false (default), nodes appearing via multiple paths (diamond dependencies)
|
// When showAllPaths is false (default), nodes appearing via multiple paths (diamond dependencies)
|
||||||
// appear only once at their shallowest depth in the tree.
|
// appear only once at their shallowest depth in the tree.
|
||||||
// When showAllPaths is true, all paths are shown with duplicate nodes at different depths.
|
// When showAllPaths is true, all paths are shown with duplicate nodes at different depths.
|
||||||
func (s *SQLiteStorage) GetDependencyTree(ctx context.Context, issueID string, maxDepth int, showAllPaths bool) ([]*types.TreeNode, error) {
|
// When reverse is true, shows dependent tree (what was discovered from this) instead of dependency tree (what blocks this).
|
||||||
|
func (s *SQLiteStorage) GetDependencyTree(ctx context.Context, issueID string, maxDepth int, showAllPaths bool, reverse bool) ([]*types.TreeNode, error) {
|
||||||
if maxDepth <= 0 {
|
if maxDepth <= 0 {
|
||||||
maxDepth = 50
|
maxDepth = 50
|
||||||
}
|
}
|
||||||
|
|
||||||
// First, build the complete tree with all paths using recursive CTE
|
// Build SQL query based on direction
|
||||||
// We need to track the full path to handle proper tree structure
|
// Normal mode: traverse dependencies (what blocks me) - goes UP
|
||||||
rows, err := s.db.QueryContext(ctx, `
|
// Reverse mode: traverse dependents (what was discovered from me) - goes DOWN
|
||||||
|
var query string
|
||||||
|
if reverse {
|
||||||
|
// Reverse: show dependents (what depends on this issue)
|
||||||
|
query = `
|
||||||
|
WITH RECURSIVE tree AS (
|
||||||
|
SELECT
|
||||||
|
i.id, i.title, i.status, i.priority, i.description, i.design,
|
||||||
|
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,
|
||||||
|
i.id as path,
|
||||||
|
i.id as parent_id
|
||||||
|
FROM issues i
|
||||||
|
WHERE i.id = ?
|
||||||
|
|
||||||
|
UNION ALL
|
||||||
|
|
||||||
|
SELECT
|
||||||
|
i.id, i.title, i.status, i.priority, i.description, i.design,
|
||||||
|
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.path || '→' || i.id,
|
||||||
|
t.id
|
||||||
|
FROM issues i
|
||||||
|
JOIN dependencies d ON i.id = d.issue_id
|
||||||
|
JOIN tree t ON d.depends_on_id = t.id
|
||||||
|
WHERE t.depth < ?
|
||||||
|
AND t.path NOT LIKE '%' || i.id || '%'
|
||||||
|
)
|
||||||
|
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
|
||||||
|
`
|
||||||
|
} else {
|
||||||
|
// Normal: show dependencies (what this issue depends on)
|
||||||
|
query = `
|
||||||
WITH RECURSIVE tree AS (
|
WITH RECURSIVE tree AS (
|
||||||
SELECT
|
SELECT
|
||||||
i.id, i.title, i.status, i.priority, i.description, i.design,
|
i.id, i.title, i.status, i.priority, i.description, i.design,
|
||||||
@@ -504,7 +547,12 @@ func (s *SQLiteStorage) GetDependencyTree(ctx context.Context, issueID string, m
|
|||||||
external_ref, depth, parent_id
|
external_ref, depth, parent_id
|
||||||
FROM tree
|
FROM tree
|
||||||
ORDER BY depth, priority, id
|
ORDER BY depth, priority, id
|
||||||
`, issueID, maxDepth)
|
`
|
||||||
|
}
|
||||||
|
|
||||||
|
// 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, query, 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)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -249,7 +249,7 @@ func TestGetDependencyTree(t *testing.T) {
|
|||||||
store.AddDependency(ctx, &types.Dependency{IssueID: issue3.ID, DependsOnID: issue2.ID, Type: types.DepBlocks}, "test-user")
|
store.AddDependency(ctx, &types.Dependency{IssueID: issue3.ID, DependsOnID: issue2.ID, Type: types.DepBlocks}, "test-user")
|
||||||
|
|
||||||
// Get tree starting from issue3
|
// Get tree starting from issue3
|
||||||
tree, err := store.GetDependencyTree(ctx, issue3.ID, 10, false)
|
tree, err := store.GetDependencyTree(ctx, issue3.ID, 10, false, false)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("GetDependencyTree failed: %v", err)
|
t.Fatalf("GetDependencyTree failed: %v", err)
|
||||||
}
|
}
|
||||||
@@ -311,7 +311,7 @@ func TestGetDependencyTree_TruncationDepth(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Get tree with maxDepth=2 (should only get 3 nodes: depths 0, 1, 2)
|
// Get tree with maxDepth=2 (should only get 3 nodes: depths 0, 1, 2)
|
||||||
tree, err := store.GetDependencyTree(ctx, issues[4].ID, 2, false)
|
tree, err := store.GetDependencyTree(ctx, issues[4].ID, 2, false, false)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("GetDependencyTree failed: %v", err)
|
t.Fatalf("GetDependencyTree failed: %v", err)
|
||||||
}
|
}
|
||||||
@@ -354,7 +354,7 @@ func TestGetDependencyTree_DefaultDepth(t *testing.T) {
|
|||||||
}, "test-user")
|
}, "test-user")
|
||||||
|
|
||||||
// Get tree with default depth (50)
|
// Get tree with default depth (50)
|
||||||
tree, err := store.GetDependencyTree(ctx, issue2.ID, 50, false)
|
tree, err := store.GetDependencyTree(ctx, issue2.ID, 50, false, false)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("GetDependencyTree failed: %v", err)
|
t.Fatalf("GetDependencyTree failed: %v", err)
|
||||||
}
|
}
|
||||||
@@ -399,7 +399,7 @@ func TestGetDependencyTree_MaxDepthOne(t *testing.T) {
|
|||||||
}, "test-user")
|
}, "test-user")
|
||||||
|
|
||||||
// Get tree with maxDepth=1 (should get root + one level)
|
// Get tree with maxDepth=1 (should get root + one level)
|
||||||
tree, err := store.GetDependencyTree(ctx, issue3.ID, 1, false)
|
tree, err := store.GetDependencyTree(ctx, issue3.ID, 1, false, false)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("GetDependencyTree failed: %v", err)
|
t.Fatalf("GetDependencyTree failed: %v", err)
|
||||||
}
|
}
|
||||||
@@ -725,3 +725,78 @@ func TestCrossTypeCyclePreventionThreeIssues(t *testing.T) {
|
|||||||
t.Errorf("Expected no cycles after prevention, but found %d", len(cycles))
|
t.Errorf("Expected no cycles after prevention, but found %d", len(cycles))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestGetDependencyTree_Reverse(t *testing.T) {
|
||||||
|
store, cleanup := setupTestDB(t)
|
||||||
|
defer cleanup()
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
// Create a dependency chain: issue1 <- issue2 <- issue3
|
||||||
|
// (issue3 depends on issue2, issue2 depends on issue1)
|
||||||
|
issue1 := &types.Issue{
|
||||||
|
Title: "Base issue",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 2,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
issue2 := &types.Issue{
|
||||||
|
Title: "Depends on issue1",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 2,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
issue3 := &types.Issue{
|
||||||
|
Title: "Depends on issue2",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 2,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
|
||||||
|
store.CreateIssue(ctx, issue1, "test")
|
||||||
|
store.CreateIssue(ctx, issue2, "test")
|
||||||
|
store.CreateIssue(ctx, issue3, "test")
|
||||||
|
|
||||||
|
// Create dependencies: issue3 → issue2 → issue1
|
||||||
|
dep1 := &types.Dependency{IssueID: issue2.ID, DependsOnID: issue1.ID, Type: types.DepBlocks}
|
||||||
|
dep2 := &types.Dependency{IssueID: issue3.ID, DependsOnID: issue2.ID, Type: types.DepBlocks}
|
||||||
|
store.AddDependency(ctx, dep1, "test")
|
||||||
|
store.AddDependency(ctx, dep2, "test")
|
||||||
|
|
||||||
|
// Test normal mode: from issue3, should traverse UP to issue1
|
||||||
|
normalTree, err := store.GetDependencyTree(ctx, issue3.ID, 10, false, false)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetDependencyTree normal mode failed: %v", err)
|
||||||
|
}
|
||||||
|
if len(normalTree) != 3 {
|
||||||
|
t.Fatalf("Expected 3 nodes in normal tree, got %d", len(normalTree))
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test reverse mode: from issue1, should traverse DOWN to issue3
|
||||||
|
reverseTree, err := store.GetDependencyTree(ctx, issue1.ID, 10, false, true)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetDependencyTree reverse mode failed: %v", err)
|
||||||
|
}
|
||||||
|
if len(reverseTree) != 3 {
|
||||||
|
t.Fatalf("Expected 3 nodes in reverse tree, got %d", len(reverseTree))
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify reverse tree structure: issue1 at depth 0
|
||||||
|
depthMap := make(map[string]int)
|
||||||
|
for _, node := range reverseTree {
|
||||||
|
depthMap[node.ID] = node.Depth
|
||||||
|
}
|
||||||
|
|
||||||
|
if depthMap[issue1.ID] != 0 {
|
||||||
|
t.Errorf("Expected depth 0 for %s in reverse tree, got %d", issue1.ID, depthMap[issue1.ID])
|
||||||
|
}
|
||||||
|
|
||||||
|
// issue2 should be at depth 1 (depends on issue1)
|
||||||
|
if depthMap[issue2.ID] != 1 {
|
||||||
|
t.Errorf("Expected depth 1 for %s in reverse tree, got %d", issue2.ID, depthMap[issue2.ID])
|
||||||
|
}
|
||||||
|
|
||||||
|
// issue3 should be at depth 2 (depends on issue2)
|
||||||
|
if depthMap[issue3.ID] != 2 {
|
||||||
|
t.Errorf("Expected depth 2 for %s in reverse tree, got %d", issue3.ID, depthMap[issue3.ID])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -25,7 +25,7 @@ type Storage interface {
|
|||||||
GetDependents(ctx context.Context, issueID string) ([]*types.Issue, error)
|
GetDependents(ctx context.Context, issueID string) ([]*types.Issue, error)
|
||||||
GetDependencyRecords(ctx context.Context, issueID string) ([]*types.Dependency, error)
|
GetDependencyRecords(ctx context.Context, issueID string) ([]*types.Dependency, error)
|
||||||
GetAllDependencyRecords(ctx context.Context) (map[string][]*types.Dependency, error)
|
GetAllDependencyRecords(ctx context.Context) (map[string][]*types.Dependency, error)
|
||||||
GetDependencyTree(ctx context.Context, issueID string, maxDepth int, showAllPaths bool) ([]*types.TreeNode, error)
|
GetDependencyTree(ctx context.Context, issueID string, maxDepth int, showAllPaths bool, reverse bool) ([]*types.TreeNode, error)
|
||||||
DetectCycles(ctx context.Context) ([][]*types.Issue, error)
|
DetectCycles(ctx context.Context) ([][]*types.Issue, error)
|
||||||
|
|
||||||
// Labels
|
// Labels
|
||||||
|
|||||||
Reference in New Issue
Block a user