From d3e90225a049ebba4f9354f7deeb352b49ad5e03 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Fri, 26 Dec 2025 12:49:40 -0800 Subject: [PATCH] =?UTF-8?q?fix(doctor):=20Preserve=20parent-child=20depend?= =?UTF-8?q?encies=20in=20child=E2=86=92parent=20check=20(GH#750)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CheckChildParentDependencies detection and ChildParentDependencies fix were flagging ALL child→parent dependencies, including legitimate 'parent-child' type structural hierarchy relationships. Now only blocking types (blocks, conditional-blocks, waits-for) are detected as anti-patterns. The 'parent-child' type is a legitimate hierarchy marker used for: - Tracking parent-child relationships - Transitive block propagation (if parent blocked, children blocked) - Hierarchy visualization in external tools Changes: - Add type filter to SELECT queries (only blocking types) - Add type filter to DELETE statement (preserve parent-child) - Add regression test TestChildParentDependencies_PreservesParentChildType 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- cmd/bd/doctor/fix/validation.go | 16 ++++--- cmd/bd/doctor/fix/validation_test.go | 63 ++++++++++++++++++++++++++++ cmd/bd/doctor/validation.go | 6 ++- 3 files changed, 77 insertions(+), 8 deletions(-) diff --git a/cmd/bd/doctor/fix/validation.go b/cmd/bd/doctor/fix/validation.go index 1b5beb0d..6f278a76 100644 --- a/cmd/bd/doctor/fix/validation.go +++ b/cmd/bd/doctor/fix/validation.go @@ -180,11 +180,14 @@ func ChildParentDependencies(path string) error { } defer db.Close() - // Find child→parent dependencies where issue_id starts with depends_on_id + "." + // Find child→parent BLOCKING dependencies where issue_id starts with depends_on_id + "." + // Only matches blocking types (blocks, conditional-blocks, waits-for) that cause deadlock. + // Excludes 'parent-child' type which is a legitimate structural hierarchy relationship. query := ` - SELECT d.issue_id, d.depends_on_id + SELECT d.issue_id, d.depends_on_id, d.type FROM dependencies d WHERE d.issue_id LIKE d.depends_on_id || '.%' + AND d.type IN ('blocks', 'conditional-blocks', 'waits-for') ` rows, err := db.Query(query) if err != nil { @@ -195,12 +198,13 @@ func ChildParentDependencies(path string) error { type badDep struct { issueID string dependsOnID string + depType string } var badDeps []badDep for rows.Next() { var d badDep - if err := rows.Scan(&d.issueID, &d.dependsOnID); err == nil { + if err := rows.Scan(&d.issueID, &d.dependsOnID, &d.depType); err == nil { badDeps = append(badDeps, d) } } @@ -210,10 +214,10 @@ func ChildParentDependencies(path string) error { return nil } - // Delete child→parent dependencies + // Delete child→parent blocking dependencies (preserving parent-child type) for _, d := range badDeps { - _, err := db.Exec("DELETE FROM dependencies WHERE issue_id = ? AND depends_on_id = ?", - d.issueID, d.dependsOnID) + _, err := db.Exec("DELETE FROM dependencies WHERE issue_id = ? AND depends_on_id = ? AND type = ?", + d.issueID, d.dependsOnID, d.depType) if err != nil { fmt.Printf(" Warning: failed to remove %s→%s: %v\n", d.issueID, d.dependsOnID, err) } else { diff --git a/cmd/bd/doctor/fix/validation_test.go b/cmd/bd/doctor/fix/validation_test.go index 83196df1..a95bf510 100644 --- a/cmd/bd/doctor/fix/validation_test.go +++ b/cmd/bd/doctor/fix/validation_test.go @@ -138,3 +138,66 @@ func TestChildParentDependencies_FixesBadDeps(t *testing.T) { t.Errorf("Expected 2 dirty issues (unique issue_ids from removed deps), got %d", dirtyCount) } } + +// TestChildParentDependencies_PreservesParentChildType verifies that legitimate +// parent-child type dependencies are NOT removed (only blocking types are removed). +// Regression test for GitHub issue #750. +func TestChildParentDependencies_PreservesParentChildType(t *testing.T) { + // Set up test database with both 'blocks' and 'parent-child' type deps + dir := t.TempDir() + beadsDir := filepath.Join(dir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatal(err) + } + + dbPath := filepath.Join(beadsDir, "beads.db") + db, err := openDB(dbPath) + if err != nil { + t.Fatal(err) + } + + // Create schema with both 'blocks' (anti-pattern) and 'parent-child' (legitimate) deps + _, err = db.Exec(` + CREATE TABLE issues (id TEXT PRIMARY KEY); + CREATE TABLE dependencies (issue_id TEXT, depends_on_id TEXT, type TEXT); + CREATE TABLE dirty_issues (issue_id TEXT PRIMARY KEY); + INSERT INTO issues (id) VALUES ('bd-abc'), ('bd-abc.1'), ('bd-abc.2'); + INSERT INTO dependencies (issue_id, depends_on_id, type) VALUES + ('bd-abc.1', 'bd-abc', 'parent-child'), + ('bd-abc.2', 'bd-abc', 'parent-child'), + ('bd-abc.1', 'bd-abc', 'blocks'); + `) + if err != nil { + t.Fatal(err) + } + db.Close() + + // Run fix + err = ChildParentDependencies(dir) + if err != nil { + t.Fatalf("ChildParentDependencies failed: %v", err) + } + + // Verify only 'blocks' type was removed, 'parent-child' preserved + db, _ = openDB(dbPath) + defer db.Close() + + var blocksCount int + db.QueryRow("SELECT COUNT(*) FROM dependencies WHERE type = 'blocks'").Scan(&blocksCount) + if blocksCount != 0 { + t.Errorf("Expected 0 'blocks' dependencies after fix, got %d", blocksCount) + } + + var parentChildCount int + db.QueryRow("SELECT COUNT(*) FROM dependencies WHERE type = 'parent-child'").Scan(&parentChildCount) + if parentChildCount != 2 { + t.Errorf("Expected 2 'parent-child' dependencies preserved, got %d", parentChildCount) + } + + // Verify only 1 dirty issue (the one with 'blocks' dep removed) + var dirtyCount int + db.QueryRow("SELECT COUNT(*) FROM dirty_issues").Scan(&dirtyCount) + if dirtyCount != 1 { + t.Errorf("Expected 1 dirty issue, got %d", dirtyCount) + } +} diff --git a/cmd/bd/doctor/validation.go b/cmd/bd/doctor/validation.go index c484fc97..ca03c29f 100644 --- a/cmd/bd/doctor/validation.go +++ b/cmd/bd/doctor/validation.go @@ -333,12 +333,14 @@ func CheckChildParentDependencies(path string) DoctorCheck { } defer db.Close() - // Query for child→parent dependencies where issue_id starts with depends_on_id + "." - // This uses SQLite's LIKE pattern matching + // Query for child→parent BLOCKING dependencies where issue_id starts with depends_on_id + "." + // Only matches blocking types (blocks, conditional-blocks, waits-for) that cause deadlock. + // Excludes 'parent-child' type which is a legitimate structural hierarchy relationship. query := ` SELECT d.issue_id, d.depends_on_id FROM dependencies d WHERE d.issue_id LIKE d.depends_on_id || '.%' + AND d.type IN ('blocks', 'conditional-blocks', 'waits-for') ` rows, err := db.Query(query) if err != nil {