feat: implement --max-depth flag for bd dep tree (closes #87, bd-3, bd-159)
- Add --max-depth/-d flag with default of 50 - Wire flag through to store.GetDependencyTree() - Add input validation (must be >= 1) - Show inline '… [truncated]' markers on truncated nodes - Update truncation warning to show actual depth used - Add comprehensive tests (truncation, default depth, boundary cases) - Update CLI docs and reference Thanks to @yashwanth-reddy909 for the initial implementation in PR #87. This commit completes the feature with full wiring, validation, tests, and docs. Amp-Thread-ID: https://ampcode.com/threads/T-c439b09c-cff2-48d9-8988-cf9353f0d32e Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
@@ -64,6 +64,7 @@
|
||||
{"id":"bd-156","title":"Refactor import logic to eliminate duplication between manual and auto-import","description":"The import logic is duplicated in two places:\n1. cmd/bd/import.go (manual 'bd import' command)\n2. cmd/bd/main.go:autoImportIfNewer() (auto-import after git pull)\n\nBoth have nearly identical code for:\n- Reading and parsing JSONL\n- Type-asserting store to *sqlite.SQLiteStorage (where we just fixed a bug twice)\n- Opening direct SQLite connection when using daemon mode\n- Detecting collisions with sqlite.DetectCollisions()\n- Scoring and remapping collisions\n- Importing issues, dependencies, and labels\n\n**Problems:**\n- Bugs must be fixed in two places (we just did this for daemon mode)\n- Features must be implemented twice\n- Tests must cover both code paths\n- Harder to maintain and keep in sync\n- Higher risk of divergence over time\n\n**Proposed solution:**\nExtract a shared function that handles the core import logic:\n\n```go\n// importIssues handles the core import logic used by both manual and auto-import\nfunc importIssues(ctx context.Context, dbPath string, store storage.Storage, \n issues []*types.Issue, opts ImportOptions) (*ImportResult, error) {\n // Handle SQLite store detection/creation for daemon mode\n // Detect collisions\n // Score and remap if needed\n // Import issues, dependencies, labels\n // Return result\n}\n```\n\nBoth import.go and autoImportIfNewer() would call this shared function with their specific options.\n\n**Benefits:**\n- Single source of truth for import logic\n- Bugs fixed once\n- Easier to test\n- Easier to extend with new import features\n- Less code overall","status":"closed","priority":2,"issue_type":"chore","created_at":"2025-10-18T17:07:06.007026-07:00","updated_at":"2025-10-18T18:35:11.753484-07:00","closed_at":"2025-10-18T17:11:20.280214-07:00"}
|
||||
{"id":"bd-157","title":"Complete auto-import refactoring to use shared importIssuesCore function","description":"The manual import command (bd import) was successfully refactored to use the shared importIssuesCore() function in import_shared.go, reducing code from 494 lines to 170 lines.\n\nHowever, autoImportIfNewer() in cmd/bd/main.go still has ~298 lines of duplicated import logic that should use the same shared function.\n\n**Current state:**\n- ✅ Manual import uses importIssuesCore() (commit 790233f)\n- ❌ Auto-import still has duplicated logic (lines 618-915 in main.go)\n\n**Duplication includes:**\n- SQLite store detection/creation for daemon mode (fixed in 790233f)\n- Collision detection with sqlite.DetectCollisions()\n- Scoring and remapping collisions\n- Importing issues (update existing, create new)\n- Importing dependencies\n- Importing labels\n\n**Benefits of completing this:**\n- Remove ~200 more lines of duplicated code\n- Ensure manual and auto-import have identical behavior\n- Future bug fixes only need to be made once\n- Easier to test and maintain\n\n**Implementation:**\nReplace lines 714-908 in autoImportIfNewer() with:\n```go\nopts := ImportOptions{\n ResolveCollisions: true, // Auto-import always resolves\n DryRun: false,\n SkipUpdate: false,\n Strict: false,\n}\nresult, err := importIssuesCore(ctx, dbPath, store, allIssues, opts)\n// Handle result and show remapping notification\n```\n\nThen update hash storage logic at the end.","status":"closed","priority":2,"issue_type":"chore","created_at":"2025-10-18T17:38:34.443872-07:00","updated_at":"2025-10-18T18:35:11.754006-07:00","closed_at":"2025-10-18T18:07:05.553928-07:00"}
|
||||
{"id":"bd-158","title":"Add .gitignore to prevent noisy untracked beads files","description":"When using beads, git status shows several untracked files in .beads/ directory: .beads/.gitignore, .beads/db.sqlite, daemon.pid and daemon.lock files. These should be added to the project's .gitignore to prevent noise.","status":"open","priority":2,"issue_type":"chore","created_at":"2025-10-18T18:27:16.424878-07:00","updated_at":"2025-10-18T18:35:11.754574-07:00"}
|
||||
{"id":"bd-159","title":"Implement --max-depth flag for bd dep tree","description":"PR #87 adds the flag but doesn't wire it through. Need to:\n1. Add flag definition in cmd/bd/dep.go\n2. Pass maxDepth to store.GetDependencyTree()\n3. Fix truncation warning to show actual depth used\n4. Add tests for truncation behavior (TestGetDependencyTree_TruncationDepth, TestGetDependencyTree_DefaultDepth)\n5. Update CLI docs/help\n\nDefault should remain 50. Keep using direct storage mode (no RPC needed for now).\n\nRelated: PR #87, bd-3","status":"closed","priority":2,"issue_type":"feature","created_at":"2025-10-19T08:31:15.473267-07:00","updated_at":"2025-10-19T08:55:21.266386-07:00","closed_at":"2025-10-19T08:55:21.266386-07:00"}
|
||||
{"id":"bd-16","title":"Add EXPLAIN QUERY PLAN tests for ready work query","description":"Verify that the hierarchical blocking query uses proper indexes and doesn't do full table scans.\n\n**Queries to analyze:**\n1. The recursive CTE (both base case and recursive case)\n2. The final SELECT with NOT EXISTS\n3. Impact of various filters (status, priority, assignee)\n\n**Implementation:**\nAdd test function that:\n- Runs EXPLAIN QUERY PLAN on GetReadyWork query\n- Parses output to verify no SCAN TABLE operations\n- Documents expected query plan in comments\n- Fails if query plan degrades\n\n**Benefits:**\n- Catch performance regressions in tests\n- Document expected query behavior\n- Ensure indexes are being used\n\nRelated to: bd-77 (composite index on depends_on_id, type)","status":"closed","priority":3,"issue_type":"task","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-18T18:35:11.755001-07:00","closed_at":"2025-10-18T12:47:44.284846-07:00"}
|
||||
{"id":"bd-17","title":"Make auto-flush debounce duration configurable","description":"flushDebounce is hardcoded to 5 seconds. Make it configurable via environment variable BEADS_FLUSH_DEBOUNCE (e.g., '500ms', '10s'). Current 5-second value is reasonable for interactive use, but CI/automated scenarios might want faster flush. Add getDebounceDuration() helper function. Located in cmd/bd/main.go:31.","status":"closed","priority":3,"issue_type":"feature","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-18T18:35:11.755588-07:00","closed_at":"2025-10-18T09:47:43.22126-07:00"}
|
||||
{"id":"bd-18","title":"Optimize auto-flush to use incremental updates","description":"Every flush exports ALL issues and ALL dependencies, even if only one issue changed. For large projects (1000+ issues), this could be expensive. Current approach guarantees consistency, which is fine for MVP, but future optimization could track which issues changed and use incremental updates. Located in cmd/bd/main.go:255-276.","status":"closed","priority":3,"issue_type":"feature","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-18T18:35:11.755965-07:00","closed_at":"2025-10-14T02:51:52.200141-07:00"}
|
||||
|
||||
@@ -1421,9 +1421,12 @@ Those issues probably have open blockers. Check:
|
||||
# See blocked issues
|
||||
bd blocked
|
||||
|
||||
# Show dependency tree
|
||||
# Show dependency tree (default max depth: 50)
|
||||
bd dep tree <issue-id>
|
||||
|
||||
# Limit tree depth to prevent deep traversals
|
||||
bd dep tree <issue-id> --max-depth 10
|
||||
|
||||
# Remove blocking dependency if needed
|
||||
bd dep remove <from-id> <to-id>
|
||||
```
|
||||
|
||||
@@ -178,8 +178,15 @@ var depTreeCmd = &cobra.Command{
|
||||
}
|
||||
|
||||
showAllPaths, _ := cmd.Flags().GetBool("show-all-paths")
|
||||
maxDepth, _ := cmd.Flags().GetInt("max-depth")
|
||||
|
||||
if maxDepth < 1 {
|
||||
fmt.Fprintf(os.Stderr, "Error: --max-depth must be >= 1\n")
|
||||
os.Exit(1)
|
||||
}
|
||||
|
||||
ctx := context.Background()
|
||||
tree, err := store.GetDependencyTree(ctx, args[0], 50, showAllPaths)
|
||||
tree, err := store.GetDependencyTree(ctx, args[0], maxDepth, showAllPaths)
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
|
||||
os.Exit(1)
|
||||
@@ -208,17 +215,19 @@ var depTreeCmd = &cobra.Command{
|
||||
for i := 0; i < node.Depth; i++ {
|
||||
indent += " "
|
||||
}
|
||||
fmt.Printf("%s→ %s: %s [P%d] (%s)\n",
|
||||
line := fmt.Sprintf("%s→ %s: %s [P%d] (%s)",
|
||||
indent, node.ID, node.Title, node.Priority, node.Status)
|
||||
if node.Truncated {
|
||||
line += " … [truncated]"
|
||||
hasTruncation = true
|
||||
}
|
||||
fmt.Println(line)
|
||||
}
|
||||
|
||||
if hasTruncation {
|
||||
yellow := color.New(color.FgYellow).SprintFunc()
|
||||
fmt.Printf("\n%s Warning: Tree truncated at depth 50 (safety limit)\n",
|
||||
yellow("⚠"))
|
||||
fmt.Printf("\n%s Warning: Tree truncated at depth %d (safety limit)\n",
|
||||
yellow("⚠"), maxDepth)
|
||||
}
|
||||
fmt.Println()
|
||||
},
|
||||
@@ -276,6 +285,7 @@ var depCyclesCmd = &cobra.Command{
|
||||
func init() {
|
||||
depAddCmd.Flags().StringP("type", "t", "blocks", "Dependency type (blocks|related|parent-child|discovered-from)")
|
||||
depTreeCmd.Flags().Bool("show-all-paths", false, "Show all paths to nodes (no deduplication for diamond dependencies)")
|
||||
depTreeCmd.Flags().IntP("max-depth", "d", 50, "Maximum tree depth to display (safety limit)")
|
||||
depCmd.AddCommand(depAddCmd)
|
||||
depCmd.AddCommand(depRemoveCmd)
|
||||
depCmd.AddCommand(depTreeCmd)
|
||||
|
||||
@@ -185,9 +185,13 @@ Visualize full dependency tree for an issue.
|
||||
|
||||
```bash
|
||||
bd dep tree issue-123
|
||||
|
||||
# Limit tree depth (default: 50)
|
||||
bd dep tree issue-123 --max-depth 10
|
||||
bd dep tree issue-123 -d 10
|
||||
```
|
||||
|
||||
Shows all dependencies and dependents in tree format.
|
||||
Shows all dependencies and dependents in tree format. Use `--max-depth` to limit traversal depth for very deep trees.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ package sqlite
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
@@ -234,6 +235,161 @@ func TestGetDependencyTree(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetDependencyTree_TruncationDepth(t *testing.T) {
|
||||
store, cleanup := setupTestDB(t)
|
||||
defer cleanup()
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
// Create a long chain: bd-5 → bd-4 → bd-3 → bd-2 → bd-1
|
||||
issues := make([]*types.Issue, 5)
|
||||
for i := 0; i < 5; i++ {
|
||||
issues[i] = &types.Issue{
|
||||
Title: fmt.Sprintf("Level %d", i),
|
||||
Status: types.StatusOpen,
|
||||
Priority: 1,
|
||||
IssueType: types.TypeTask,
|
||||
}
|
||||
err := store.CreateIssue(ctx, issues[i], "test-user")
|
||||
if err != nil {
|
||||
t.Fatalf("CreateIssue failed: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Link them in chain
|
||||
for i := 1; i < 5; i++ {
|
||||
err := store.AddDependency(ctx, &types.Dependency{
|
||||
IssueID: issues[i].ID,
|
||||
DependsOnID: issues[i-1].ID,
|
||||
Type: types.DepBlocks,
|
||||
}, "test-user")
|
||||
if err != nil {
|
||||
t.Fatalf("AddDependency failed: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Get tree with maxDepth=2 (should only get 3 nodes: depths 0, 1, 2)
|
||||
tree, err := store.GetDependencyTree(ctx, issues[4].ID, 2, false)
|
||||
if err != nil {
|
||||
t.Fatalf("GetDependencyTree failed: %v", err)
|
||||
}
|
||||
|
||||
if len(tree) != 3 {
|
||||
t.Fatalf("Expected 3 nodes with maxDepth=2, got %d", len(tree))
|
||||
}
|
||||
|
||||
// Check that last node is marked as truncated
|
||||
foundTruncated := false
|
||||
for _, node := range tree {
|
||||
if node.Depth == 2 && node.Truncated {
|
||||
foundTruncated = true
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
if !foundTruncated {
|
||||
t.Error("Expected node at depth 2 to be marked as truncated")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetDependencyTree_DefaultDepth(t *testing.T) {
|
||||
store, cleanup := setupTestDB(t)
|
||||
defer cleanup()
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
// Create a simple chain
|
||||
issue1 := &types.Issue{Title: "Level 0", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask}
|
||||
issue2 := &types.Issue{Title: "Level 1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask}
|
||||
|
||||
store.CreateIssue(ctx, issue1, "test-user")
|
||||
store.CreateIssue(ctx, issue2, "test-user")
|
||||
|
||||
store.AddDependency(ctx, &types.Dependency{
|
||||
IssueID: issue2.ID,
|
||||
DependsOnID: issue1.ID,
|
||||
Type: types.DepBlocks,
|
||||
}, "test-user")
|
||||
|
||||
// Get tree with default depth (50)
|
||||
tree, err := store.GetDependencyTree(ctx, issue2.ID, 50, false)
|
||||
if err != nil {
|
||||
t.Fatalf("GetDependencyTree failed: %v", err)
|
||||
}
|
||||
|
||||
if len(tree) != 2 {
|
||||
t.Fatalf("Expected 2 nodes, got %d", len(tree))
|
||||
}
|
||||
|
||||
// No truncation should occur
|
||||
for _, node := range tree {
|
||||
if node.Truncated {
|
||||
t.Error("Expected no truncation with default depth on short chain")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetDependencyTree_MaxDepthOne(t *testing.T) {
|
||||
store, cleanup := setupTestDB(t)
|
||||
defer cleanup()
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
// Create a chain: bd-3 → bd-2 → bd-1
|
||||
issue1 := &types.Issue{Title: "Level 2", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask}
|
||||
issue2 := &types.Issue{Title: "Level 1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask}
|
||||
issue3 := &types.Issue{Title: "Root", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask}
|
||||
|
||||
store.CreateIssue(ctx, issue1, "test-user")
|
||||
store.CreateIssue(ctx, issue2, "test-user")
|
||||
store.CreateIssue(ctx, issue3, "test-user")
|
||||
|
||||
store.AddDependency(ctx, &types.Dependency{
|
||||
IssueID: issue2.ID,
|
||||
DependsOnID: issue1.ID,
|
||||
Type: types.DepBlocks,
|
||||
}, "test-user")
|
||||
|
||||
store.AddDependency(ctx, &types.Dependency{
|
||||
IssueID: issue3.ID,
|
||||
DependsOnID: issue2.ID,
|
||||
Type: types.DepBlocks,
|
||||
}, "test-user")
|
||||
|
||||
// Get tree with maxDepth=1 (should get root + one level)
|
||||
tree, err := store.GetDependencyTree(ctx, issue3.ID, 1, false)
|
||||
if err != nil {
|
||||
t.Fatalf("GetDependencyTree failed: %v", err)
|
||||
}
|
||||
|
||||
// Should get root (depth 0) and one child (depth 1)
|
||||
if len(tree) != 2 {
|
||||
t.Fatalf("Expected 2 nodes with maxDepth=1, got %d", len(tree))
|
||||
}
|
||||
|
||||
// Check root is at depth 0 and not truncated
|
||||
rootFound := false
|
||||
for _, node := range tree {
|
||||
if node.ID == issue3.ID && node.Depth == 0 && !node.Truncated {
|
||||
rootFound = true
|
||||
}
|
||||
}
|
||||
if !rootFound {
|
||||
t.Error("Expected root at depth 0, not truncated")
|
||||
}
|
||||
|
||||
// Check child at depth 1 is truncated
|
||||
childTruncated := false
|
||||
for _, node := range tree {
|
||||
if node.Depth == 1 && node.Truncated {
|
||||
childTruncated = true
|
||||
}
|
||||
}
|
||||
if !childTruncated {
|
||||
t.Error("Expected child at depth 1 to be truncated")
|
||||
}
|
||||
}
|
||||
|
||||
func TestDetectCycles(t *testing.T) {
|
||||
store, cleanup := setupTestDB(t)
|
||||
defer cleanup()
|
||||
|
||||
Reference in New Issue
Block a user