fix(doctor): Preserve parent-child dependencies in child→parent check (GH#750)
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 <noreply@anthropic.com>
This commit is contained in:
@@ -180,11 +180,14 @@ func ChildParentDependencies(path string) error {
|
|||||||
}
|
}
|
||||||
defer db.Close()
|
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 := `
|
query := `
|
||||||
SELECT d.issue_id, d.depends_on_id
|
SELECT d.issue_id, d.depends_on_id, d.type
|
||||||
FROM dependencies d
|
FROM dependencies d
|
||||||
WHERE d.issue_id LIKE d.depends_on_id || '.%'
|
WHERE d.issue_id LIKE d.depends_on_id || '.%'
|
||||||
|
AND d.type IN ('blocks', 'conditional-blocks', 'waits-for')
|
||||||
`
|
`
|
||||||
rows, err := db.Query(query)
|
rows, err := db.Query(query)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -195,12 +198,13 @@ func ChildParentDependencies(path string) error {
|
|||||||
type badDep struct {
|
type badDep struct {
|
||||||
issueID string
|
issueID string
|
||||||
dependsOnID string
|
dependsOnID string
|
||||||
|
depType string
|
||||||
}
|
}
|
||||||
var badDeps []badDep
|
var badDeps []badDep
|
||||||
|
|
||||||
for rows.Next() {
|
for rows.Next() {
|
||||||
var d badDep
|
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)
|
badDeps = append(badDeps, d)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -210,10 +214,10 @@ func ChildParentDependencies(path string) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// Delete child→parent dependencies
|
// Delete child→parent blocking dependencies (preserving parent-child type)
|
||||||
for _, d := range badDeps {
|
for _, d := range badDeps {
|
||||||
_, err := db.Exec("DELETE FROM dependencies WHERE issue_id = ? AND depends_on_id = ?",
|
_, err := db.Exec("DELETE FROM dependencies WHERE issue_id = ? AND depends_on_id = ? AND type = ?",
|
||||||
d.issueID, d.dependsOnID)
|
d.issueID, d.dependsOnID, d.depType)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
fmt.Printf(" Warning: failed to remove %s→%s: %v\n", d.issueID, d.dependsOnID, err)
|
fmt.Printf(" Warning: failed to remove %s→%s: %v\n", d.issueID, d.dependsOnID, err)
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -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)
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -333,12 +333,14 @@ func CheckChildParentDependencies(path string) DoctorCheck {
|
|||||||
}
|
}
|
||||||
defer db.Close()
|
defer db.Close()
|
||||||
|
|
||||||
// Query for child→parent dependencies where issue_id starts with depends_on_id + "."
|
// Query for child→parent BLOCKING dependencies where issue_id starts with depends_on_id + "."
|
||||||
// This uses SQLite's LIKE pattern matching
|
// Only matches blocking types (blocks, conditional-blocks, waits-for) that cause deadlock.
|
||||||
|
// Excludes 'parent-child' type which is a legitimate structural hierarchy relationship.
|
||||||
query := `
|
query := `
|
||||||
SELECT d.issue_id, d.depends_on_id
|
SELECT d.issue_id, d.depends_on_id
|
||||||
FROM dependencies d
|
FROM dependencies d
|
||||||
WHERE d.issue_id LIKE d.depends_on_id || '.%'
|
WHERE d.issue_id LIKE d.depends_on_id || '.%'
|
||||||
|
AND d.type IN ('blocks', 'conditional-blocks', 'waits-for')
|
||||||
`
|
`
|
||||||
rows, err := db.Query(query)
|
rows, err := db.Query(query)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
Reference in New Issue
Block a user