From d2e34b863b906b53828f98041546277fb7d9ff56 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Sat, 18 Oct 2025 11:52:57 -0700 Subject: [PATCH] Add --show-all-paths flag to bd dep tree Implements bd-9: Allow users to view all paths through diamond dependencies without deduplication. Useful for debugging complex dependency structures. Changes: - Added --show-all-paths flag to bd dep tree command - Updated GetDependencyTree interface to accept showAllPaths parameter - Modified deduplication logic to be conditional on flag - Updated tests to pass new parameter Amp-Thread-ID: https://ampcode.com/threads/T-43807dd5-8732-49ad-a839-cdb5dae70c35 Co-authored-by: Amp --- .beads/issues.jsonl | 6 ++-- cmd/bd/dep.go | 4 ++- internal/storage/sqlite/dependencies.go | 32 +++++++++++--------- internal/storage/sqlite/dependencies_test.go | 2 +- internal/storage/storage.go | 2 +- 5 files changed, 26 insertions(+), 20 deletions(-) diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 4ec8af4a..32a30c81 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -43,13 +43,13 @@ {"id":"bd-137","title":"Test A","description":"","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-17T23:06:59.59343-07:00","updated_at":"2025-10-18T09:57:28.091779-07:00","closed_at":"2025-10-17T23:06:59.740704-07:00","dependencies":[{"issue_id":"bd-137","depends_on_id":"bd-138","type":"blocks","created_at":"2025-10-17T23:06:59.668292-07:00","created_by":"daemon"}]} {"id":"bd-138","title":"Test B","description":"","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-17T23:06:59.626612-07:00","updated_at":"2025-10-18T09:57:28.095042-07:00","closed_at":"2025-10-17T23:06:59.744519-07:00"} {"id":"bd-139","title":"bd ready doesn't show epics/tasks ready to close when all children complete","description":"The 'bd ready' command doesn't show epics that have all children complete as ready work. Example: vc-30 (epic) blocks 4 children - 3 are closed, 1 is in_progress. The epic itself should be reviewable/closable but doesn't show in ready work. Similarly, vc-61 (epic, in_progress) depends on vc-48 (closed) but doesn't show as ready. Expected: epics with all dependencies satisfied should show as ready to review/close. Actual: 'bd ready' returns 'no ready work' even though multiple epics are completable.","acceptance_criteria":"bd ready shows epics/tasks that have all dependencies satisfied (even if status is in_progress), allowing user to review and close them","status":"closed","priority":0,"issue_type":"bug","created_at":"2025-10-18T00:04:41.811991-07:00","updated_at":"2025-10-18T09:57:28.095697-07:00","closed_at":"2025-10-18T00:20:36.188211-07:00"} -{"id":"bd-14","title":"Investigate vector/semantic search for issue discovery","description":"From GH issue #2 RFC discussion: Evaluate if vector/semantic search over issues would provide value for beads.\n\n**Use case:** Find semantically related issues (e.g., 'login broken' finds 'authentication failure', 'session expired').\n\n**Questions to answer:**\n1. What workflows would this enable that we can't do now?\n2. Is dataset size (typically 50-200 issues) large enough to benefit?\n3. Do structured features (deps, tags, types) already provide better relationships?\n4. What's the maintenance cost (embeddings, storage, recomputation)?\n\n**Alternatives to consider:**\n- Improve 'bd list' filtering with regex/boolean queries\n- Add 'bd related \u003cid\u003e' showing deps + mentions + same tags\n- Export to JSON and pipe to external AI tools\n\n**Decision:** Only implement if clear use case emerges. Don't add complexity for theoretical benefits.\n\n**Context:** Part of evaluating Turso RFC ideas (GH #2). Vector search was proposed but unclear if needed for typical beads usage.","status":"open","priority":3,"issue_type":"task","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-18T09:57:27.585303-07:00"} +{"id":"bd-14","title":"Investigate vector/semantic search for issue discovery","description":"From GH issue #2 RFC discussion: Evaluate if vector/semantic search over issues would provide value for beads.\n\n**Use case:** Find semantically related issues (e.g., 'login broken' finds 'authentication failure', 'session expired').\n\n**Questions to answer:**\n1. What workflows would this enable that we can't do now?\n2. Is dataset size (typically 50-200 issues) large enough to benefit?\n3. Do structured features (deps, tags, types) already provide better relationships?\n4. What's the maintenance cost (embeddings, storage, recomputation)?\n\n**Alternatives to consider:**\n- Improve 'bd list' filtering with regex/boolean queries\n- Add 'bd related \u003cid\u003e' showing deps + mentions + same tags\n- Export to JSON and pipe to external AI tools\n\n**Decision:** Only implement if clear use case emerges. Don't add complexity for theoretical benefits.\n\n**Context:** Part of evaluating Turso RFC ideas (GH #2). Vector search was proposed but unclear if needed for typical beads usage.","status":"closed","priority":3,"issue_type":"task","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-18T10:09:23.532858-07:00","closed_at":"2025-10-18T10:09:23.532858-07:00"} {"id":"bd-140","title":"CleanupStaleInstances() never called in production - orphaned claims accumulate","description":"The CleanupStaleInstances() method exists in storage layer but is never called in production code. This means dead executors leave orphaned claims that block work forever. Example: vc-106 claimed by executor that died 2 hours ago, still shows in_progress with execution_state record. Need to: 1) Add periodic cleanup to executor main loop (every 5 min?), 2) Make cleanup also release claimed issues (delete execution_state AND reset status to open), 3) Add comment explaining why released.","design":"Add background goroutine in executor that calls CleanupStaleInstances() every 5 minutes. When marking instance stopped, also query for all issues claimed by that instance and release them (delete execution_state, set status=open, add event comment).","acceptance_criteria":"Dead executors automatically release their claims within 5-10 minutes of going stale, issues return to open status and become available for re-execution","status":"closed","priority":0,"issue_type":"bug","created_at":"2025-10-18T00:24:57.920072-07:00","updated_at":"2025-10-18T09:57:28.096305-07:00","closed_at":"2025-10-18T02:09:05.05969-07:00"} {"id":"bd-141","title":"releaseIssueWithError() deletes execution_state but leaves status as in_progress","description":"When an executor hits an error and releases an issue via releaseIssueWithError(), it deletes the execution_state but leaves the issue status as in_progress. This means the issue drops out of ready work but has no active executor. Expected: releasing should reset status to open so the issue becomes available again. Current code in conversation.go just calls ReleaseIssue() which only deletes execution_state.","design":"Update releaseIssueWithError() to also update issue status back to open. Or create a new ReleaseAndReopen() method that does both atomically in a transaction.","acceptance_criteria":"Issues released due to errors automatically return to open status and show in bd ready","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-18T00:25:06.798843-07:00","updated_at":"2025-10-18T09:57:28.099963-07:00","closed_at":"2025-10-18T02:09:08.595764-07:00"} {"id":"bd-142","title":"Add 'bd stale' command to show orphaned claims and dead executors","description":"Need visibility into orphaned claims - issues stuck in_progress with execution_state but executor is dead/stopped. Add command to show: 1) All issues with execution_state where executor status=stopped or last_heartbeat \u003e threshold, 2) Executor instance details (when died, how long claimed), 3) Option to auto-release them. Makes manual recovery easier until auto-cleanup (bd-140) is implemented.","design":"Query: SELECT i.*, ei.status, ei.last_heartbeat FROM issues i JOIN issue_execution_state ies ON i.id = ies.issue_id JOIN executor_instances ei ON ies.executor_instance_id = ei.instance_id WHERE ei.status='stopped' OR ei.last_heartbeat \u003c NOW() - threshold. Add --release flag to auto-release all found issues.","acceptance_criteria":"bd stale shows orphaned claims, bd stale --release cleans them up","status":"closed","priority":1,"issue_type":"feature","created_at":"2025-10-18T00:25:16.530937-07:00","updated_at":"2025-10-18T09:57:28.141701-07:00","closed_at":"2025-10-18T02:09:12.529064-07:00"} {"id":"bd-143","title":"Bias ready work towards recent issues before oldest-first","description":"Currently 'bd ready' shows oldest issues first (by created_at). This can bury recently discovered work that might be more relevant. Propose a hybrid approach: show issues from the past 1-2 days first (sorted by priority), then fall back to oldest-first for older issues. This keeps fresh discoveries visible while still surfacing old forgotten work.","status":"closed","priority":2,"issue_type":"feature","created_at":"2025-10-18T09:31:15.036495-07:00","updated_at":"2025-10-18T09:57:28.105887-07:00","closed_at":"2025-10-18T09:35:55.084891-07:00"} {"id":"bd-144","title":"Fix nil pointer dereference in renumber command","description":"The 'bd renumber' command crashes with a nil pointer dereference at renumber.go:52 because store is nil. The command doesn't properly handle daemon/direct mode initialization like other commands do. Error occurs on both --dry-run and --force modes.","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-18T09:54:31.59912-07:00","updated_at":"2025-10-18T09:57:28.106373-07:00","closed_at":"2025-10-18T09:56:49.88701-07:00"} -{"id":"bd-15","title":"Add performance benchmarks document","description":"Document actual performance metrics with hyperfine tests","status":"open","priority":3,"issue_type":"task","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-18T09:57:27.587232-07:00","dependencies":[{"issue_id":"bd-15","depends_on_id":"bd-1","type":"parent-child","created_at":"2025-10-16T21:51:08.918095-07:00","created_by":"renumber"}]} +{"id":"bd-15","title":"Add performance benchmarks document","description":"Document actual performance metrics with hyperfine tests","status":"closed","priority":3,"issue_type":"task","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-18T10:09:23.532938-07:00","closed_at":"2025-10-18T10:09:23.532938-07:00","dependencies":[{"issue_id":"bd-15","depends_on_id":"bd-1","type":"parent-child","created_at":"2025-10-16T21:51:08.918095-07:00","created_by":"renumber"}]} {"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":"open","priority":3,"issue_type":"task","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-18T09:57:28.107143-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-18T09:57:27.593569-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-18T09:57:27.595342-07:00","closed_at":"2025-10-14T02:51:52.200141-07:00"} @@ -131,7 +131,7 @@ {"id":"bd-87","title":"Handle missing JSONL directory in findJSONLPath","description":"findJSONLPath() assumes the database directory exists. If someone runs bd init to create a new database but the .beads directory doesn't exist yet, the glob operations might fail silently. Add os.MkdirAll(dbDir, 0755) to ensure directory exists before globbing. Located in cmd/bd/main.go:188-201.","status":"closed","priority":2,"issue_type":"bug","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-18T09:57:27.822402-07:00","closed_at":"2025-10-14T02:51:52.199959-07:00"} {"id":"bd-88","title":"Add design/notes/acceptance_criteria fields to update command","description":"Currently bd update only supports status, priority, title, assignee. Add support for --design, --notes, --acceptance-criteria flags. This makes it easier to add detailed designs to issues after creation.","status":"closed","priority":2,"issue_type":"feature","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-18T09:57:27.827567-07:00","closed_at":"2025-10-16T10:07:34.00541-07:00"} {"id":"bd-89","title":"Fix import zero-value field handling","description":"Import uses zero-value checks (Priority != 0) to determine field updates. This prevents setting priority to 0 or clearing string fields. Export/import round-trip not fully idempotent for zero values. Consider JSON presence detection or explicit preserve-existing semantics. Location: cmd/bd/import.go:95-106","status":"closed","priority":2,"issue_type":"bug","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-18T09:57:27.82823-07:00","closed_at":"2025-10-14T02:51:52.198697-07:00"} -{"id":"bd-9","title":"Add --show-all-paths flag to bd dep tree","description":"Currently bd dep tree deduplicates nodes when multiple paths exist (diamond dependencies). Add optional --show-all-paths flag to display the full graph with all paths, showing duplicates. Useful for debugging complex dependency structures and understanding all relationships.","status":"open","priority":3,"issue_type":"feature","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-18T09:57:27.564134-07:00"} +{"id":"bd-9","title":"Add --show-all-paths flag to bd dep tree","description":"Currently bd dep tree deduplicates nodes when multiple paths exist (diamond dependencies). Add optional --show-all-paths flag to display the full graph with all paths, showing duplicates. Useful for debugging complex dependency structures and understanding all relationships.","status":"closed","priority":3,"issue_type":"feature","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-18T10:11:38.985862-07:00","closed_at":"2025-10-18T10:11:38.985862-07:00"} {"id":"bd-90","title":"Add --strict flag for dependency import failures","description":"Currently dependency import errors are warnings (logged to stderr, execution continues). Missing targets or cycles may indicate JSONL corruption. Add --strict flag to fail on any dependency errors for data integrity validation. Location: cmd/bd/import.go:159-164","status":"closed","priority":2,"issue_type":"feature","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-18T09:57:27.830423-07:00","closed_at":"2025-10-16T10:07:34.035752-07:00"} {"id":"bd-91","title":"Optimize reference updates to avoid loading all issues into memory","description":"In updateReferences(), we call SearchIssues with no filter to get ALL issues for updating references. For large databases (10k+ issues), this loads everything into memory. Options: 1) Use batched processing with LIMIT/OFFSET, 2) Use SQL UPDATE with REPLACE() directly, 3) Stream results instead of loading all at once. Located in collision.go:266","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-18T09:57:27.831676-07:00","closed_at":"2025-10-17T23:26:43.830137-07:00"} {"id":"bd-92","title":"Cache compiled regexes in replaceIDReferences for performance","description":"replaceIDReferences() compiles the same regex patterns on every call. With 100 issues and 10 ID mappings, that's 1000 regex compilations. Pre-compile regexes once and reuse. Can use a struct with compiled regex, placeholder, and newID. Located in collision.go:329. Estimated performance improvement: 10-100x for large batches.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-16T20:46:08.971822-07:00","updated_at":"2025-10-18T09:57:27.832549-07:00","closed_at":"2025-10-16T10:07:22.469891-07:00"} diff --git a/cmd/bd/dep.go b/cmd/bd/dep.go index 9f975024..fc07fad8 100644 --- a/cmd/bd/dep.go +++ b/cmd/bd/dep.go @@ -177,8 +177,9 @@ var depTreeCmd = &cobra.Command{ defer store.Close() } + showAllPaths, _ := cmd.Flags().GetBool("show-all-paths") ctx := context.Background() - tree, err := store.GetDependencyTree(ctx, args[0], 50) + tree, err := store.GetDependencyTree(ctx, args[0], 50, showAllPaths) if err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) os.Exit(1) @@ -274,6 +275,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)") depCmd.AddCommand(depAddCmd) depCmd.AddCommand(depRemoveCmd) depCmd.AddCommand(depTreeCmd) diff --git a/internal/storage/sqlite/dependencies.go b/internal/storage/sqlite/dependencies.go index 962bd951..1bdf1473 100644 --- a/internal/storage/sqlite/dependencies.go +++ b/internal/storage/sqlite/dependencies.go @@ -450,10 +450,11 @@ func (s *SQLiteStorage) GetAllDependencyRecords(ctx context.Context) (map[string return depsMap, nil } -// 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) { +// GetDependencyTree returns the full dependency tree with optional deduplication +// When showAllPaths is false (default), nodes appearing via multiple paths (diamond dependencies) +// appear only once at their shallowest depth in the tree. +// 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) { if maxDepth <= 0 { maxDepth = 50 } @@ -542,17 +543,20 @@ 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 - } + // Deduplicate only if showAllPaths is false + if !showAllPaths { + // 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 + // Mark this node as seen at this depth + seen[node.ID] = node.Depth + } nodes = append(nodes, &node) } diff --git a/internal/storage/sqlite/dependencies_test.go b/internal/storage/sqlite/dependencies_test.go index 9a901e39..f984bc41 100644 --- a/internal/storage/sqlite/dependencies_test.go +++ b/internal/storage/sqlite/dependencies_test.go @@ -206,7 +206,7 @@ func TestGetDependencyTree(t *testing.T) { store.AddDependency(ctx, &types.Dependency{IssueID: issue3.ID, DependsOnID: issue2.ID, Type: types.DepBlocks}, "test-user") // Get tree starting from issue3 - tree, err := store.GetDependencyTree(ctx, issue3.ID, 10) + tree, err := store.GetDependencyTree(ctx, issue3.ID, 10, false) if err != nil { t.Fatalf("GetDependencyTree failed: %v", err) } diff --git a/internal/storage/storage.go b/internal/storage/storage.go index f5306d8d..c9283531 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -24,7 +24,7 @@ type Storage interface { GetDependents(ctx context.Context, issueID string) ([]*types.Issue, error) GetDependencyRecords(ctx context.Context, issueID string) ([]*types.Dependency, error) GetAllDependencyRecords(ctx context.Context) (map[string][]*types.Dependency, error) - GetDependencyTree(ctx context.Context, issueID string, maxDepth int) ([]*types.TreeNode, error) + GetDependencyTree(ctx context.Context, issueID string, maxDepth int, showAllPaths bool) ([]*types.TreeNode, error) DetectCycles(ctx context.Context) ([][]*types.Issue, error) // Labels