Exclude parent tasks with open subtasks from next candidates

Amolith created

Change summary

src/cmd/next.rs   |  18 ++++++
src/score.rs      |  73 ++++++++++++++++++++++----
tests/cli_next.rs | 130 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 208 insertions(+), 13 deletions(-)

Detailed changes

src/cmd/next.rs 🔗

@@ -1,6 +1,7 @@
 use anyhow::{bail, Result};
 use comfy_table::presets::NOTHING;
 use comfy_table::Table;
+use std::collections::HashSet;
 use std::path::Path;
 
 use crate::db;
@@ -41,7 +42,22 @@ pub fn run(root: &Path, mode_str: &str, verbose: bool, limit: usize, json: bool)
         .query_map([], |r| Ok((r.get(0)?, r.get(1)?)))?
         .collect::<rusqlite::Result<_>>()?;
 
-    let scored = score::rank(&open_tasks, &edges, mode, limit);
+    // Parents with at least one open subtask are not actionable work
+    // units — exclude them from candidates while keeping them in the
+    // graph for downstream scoring.
+    let mut parent_stmt =
+        conn.prepare("SELECT DISTINCT parent FROM tasks WHERE parent != '' AND status = 'open'")?;
+    let parents_with_open_children: HashSet<String> = parent_stmt
+        .query_map([], |r| r.get(0))?
+        .collect::<rusqlite::Result<_>>()?;
+
+    let scored = score::rank(
+        &open_tasks,
+        &edges,
+        &parents_with_open_children,
+        mode,
+        limit,
+    );
 
     if scored.is_empty() {
         if json {

src/score.rs 🔗

@@ -114,11 +114,14 @@ fn downstream(
 /// # Arguments
 /// * `open_tasks` — all open tasks (id, title, priority, effort)
 /// * `blocker_edges` — `(task_id, blocker_id)` pairs among open tasks
+/// * `exclude` — task IDs to exclude from candidates (still counted in
+///   downstream scores). Used to filter parent tasks that have open subtasks.
 /// * `mode` — scoring strategy
 /// * `limit` — maximum number of results to return
 pub fn rank(
     open_tasks: &[(String, String, i32, i32)],
     blocker_edges: &[(String, String)],
+    exclude: &HashSet<String>,
     mode: Mode,
     limit: usize,
 ) -> Vec<ScoredTask> {
@@ -157,10 +160,11 @@ pub fn rank(
         }
     }
 
-    // Find ready tasks: open tasks with no open blockers.
+    // Find ready tasks: open tasks with no open blockers, excluding
+    // parent tasks that still have open subtasks.
     let ready: Vec<&TaskNode> = nodes
         .values()
-        .filter(|n| !blocked_by.contains_key(&n.id))
+        .filter(|n| !blocked_by.contains_key(&n.id) && !exclude.contains(&n.id))
         .collect();
 
     // Score each ready task.
@@ -218,7 +222,7 @@ mod tests {
     #[test]
     fn single_task_no_deps() {
         let tasks = vec![task("a", "Alpha", 1, 1)];
-        let result = rank(&tasks, &[], Mode::Impact, 5);
+        let result = rank(&tasks, &[], &HashSet::new(), Mode::Impact, 5);
 
         assert_eq!(result.len(), 1);
         assert_eq!(result[0].id, "a");
@@ -234,7 +238,7 @@ mod tests {
         // A is ready (blocks B), B is blocked.
         let tasks = vec![task("a", "Blocker", 2, 2), task("b", "Blocked", 1, 1)];
         let edges = vec![edge("b", "a")];
-        let result = rank(&tasks, &edges, Mode::Impact, 5);
+        let result = rank(&tasks, &edges, &HashSet::new(), Mode::Impact, 5);
 
         // Only A is ready.
         assert_eq!(result.len(), 1);
@@ -255,7 +259,7 @@ mod tests {
             task("c", "Leaf", 1, 1),
         ];
         let edges = vec![edge("b", "a"), edge("c", "b")];
-        let result = rank(&tasks, &edges, Mode::Impact, 5);
+        let result = rank(&tasks, &edges, &HashSet::new(), Mode::Impact, 5);
 
         assert_eq!(result.len(), 1);
         assert_eq!(result[0].id, "a");
@@ -275,7 +279,7 @@ mod tests {
             task("c", "Sink", 1, 1),
         ];
         let edges = vec![edge("c", "a"), edge("c", "b")];
-        let result = rank(&tasks, &edges, Mode::Impact, 5);
+        let result = rank(&tasks, &edges, &HashSet::new(), Mode::Impact, 5);
 
         assert_eq!(result.len(), 2);
         // Both A and B see C as downstream.
@@ -296,8 +300,8 @@ mod tests {
         ];
         let edges = vec![edge("b", "a")];
 
-        let impact = rank(&tasks, &edges, Mode::Impact, 5);
-        let effort = rank(&tasks, &edges, Mode::Effort, 5);
+        let impact = rank(&tasks, &edges, &HashSet::new(), Mode::Impact, 5);
+        let effort = rank(&tasks, &edges, &HashSet::new(), Mode::Effort, 5);
 
         // Impact: (3.0 + 1.0) * 3.0 / 3.0 = 4.0
         assert!((impact[0].score - 4.0).abs() < f64::EPSILON);
@@ -309,7 +313,7 @@ mod tests {
     fn effort_mode_prefers_low_effort() {
         // Two standalone tasks: A is high-effort, B is low-effort. Same priority.
         let tasks = vec![task("a", "Heavy", 2, 3), task("b", "Light", 2, 1)];
-        let result = rank(&tasks, &[], Mode::Effort, 5);
+        let result = rank(&tasks, &[], &HashSet::new(), Mode::Effort, 5);
 
         assert_eq!(result.len(), 2);
         // B should rank higher (low effort).
@@ -324,13 +328,13 @@ mod tests {
             task("b", "B", 2, 2),
             task("c", "C", 3, 3),
         ];
-        let result = rank(&tasks, &[], Mode::Impact, 2);
+        let result = rank(&tasks, &[], &HashSet::new(), Mode::Impact, 2);
         assert_eq!(result.len(), 2);
     }
 
     #[test]
     fn empty_input() {
-        let result = rank(&[], &[], Mode::Impact, 5);
+        let result = rank(&[], &[], &HashSet::new(), Mode::Impact, 5);
         assert!(result.is_empty());
     }
 
@@ -338,8 +342,53 @@ mod tests {
     fn stable_sort_by_id() {
         // Two tasks with identical scores should sort by id.
         let tasks = vec![task("b", "Second", 2, 2), task("a", "First", 2, 2)];
-        let result = rank(&tasks, &[], Mode::Impact, 5);
+        let result = rank(&tasks, &[], &HashSet::new(), Mode::Impact, 5);
         assert_eq!(result[0].id, "a");
         assert_eq!(result[1].id, "b");
     }
+
+    #[test]
+    fn excluded_tasks_not_candidates() {
+        // A and B are both standalone ready tasks, but A is excluded
+        // (simulating a parent with open subtasks).
+        let tasks = vec![task("a", "Parent", 1, 1), task("b", "Leaf", 2, 2)];
+        let exclude: HashSet<String> = ["a".to_string()].into();
+        let result = rank(&tasks, &[], &exclude, Mode::Impact, 5);
+
+        assert_eq!(result.len(), 1);
+        assert_eq!(result[0].id, "b");
+    }
+
+    #[test]
+    fn excluded_task_still_counted_in_downstream() {
+        // A blocks B (the excluded parent). B blocks C.
+        // A is ready. B is excluded but its downstream weight should still
+        // flow through the graph — A should see both B and C downstream.
+        let tasks = vec![
+            task("a", "Root", 2, 2),
+            task("b", "Parent", 1, 1),
+            task("c", "Leaf", 2, 2),
+        ];
+        let edges = vec![edge("b", "a"), edge("c", "b")];
+        let exclude: HashSet<String> = ["b".to_string()].into();
+        let result = rank(&tasks, &edges, &exclude, Mode::Impact, 5);
+
+        // Only A is ready (B is blocked and also excluded, C is blocked).
+        assert_eq!(result.len(), 1);
+        assert_eq!(result[0].id, "a");
+        // Downstream still includes B and C.
+        assert_eq!(result[0].total_unblocked, 2);
+    }
+
+    #[test]
+    fn parent_with_all_children_closed_remains_candidate() {
+        // A standalone task that would be in the open_tasks list but NOT
+        // in the exclude set (because all its children are closed, the
+        // caller wouldn't put it there). Verify it's still a candidate.
+        let tasks = vec![task("a", "Parent done kids", 1, 1)];
+        let result = rank(&tasks, &[], &HashSet::new(), Mode::Impact, 5);
+
+        assert_eq!(result.len(), 1);
+        assert_eq!(result[0].id, "a");
+    }
 }

tests/cli_next.rs 🔗

@@ -213,3 +213,133 @@ fn next_ignores_closed_tasks() {
     assert_eq!(results.len(), 1);
     assert_eq!(results[0]["id"].as_str().unwrap(), a);
 }
+
+#[test]
+fn next_excludes_parent_with_open_subtasks() {
+    let tmp = init_tmp();
+    let parent = create_task(&tmp, "Parent task", "high", "low");
+    // Create a subtask under the parent.
+    let out = td()
+        .args([
+            "--json",
+            "create",
+            "Child task",
+            "-p",
+            "medium",
+            "-e",
+            "medium",
+            "--parent",
+            &parent,
+        ])
+        .current_dir(&tmp)
+        .output()
+        .unwrap();
+    let child: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap();
+    let child_id = child["id"].as_str().unwrap().to_string();
+
+    let out = td()
+        .args(["--json", "next"])
+        .current_dir(&tmp)
+        .output()
+        .unwrap();
+    let v: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap();
+    let results = v.as_array().unwrap();
+    let ids: Vec<&str> = results.iter().map(|r| r["id"].as_str().unwrap()).collect();
+
+    // Parent should be excluded; only the child subtask should appear.
+    assert!(!ids.contains(&parent.as_str()), "parent should be excluded");
+    assert!(
+        ids.contains(&child_id.as_str()),
+        "child should be a candidate"
+    );
+}
+
+#[test]
+fn next_includes_parent_when_all_subtasks_closed() {
+    let tmp = init_tmp();
+    let parent = create_task(&tmp, "Parent task", "high", "low");
+    let out = td()
+        .args([
+            "--json",
+            "create",
+            "Child task",
+            "-p",
+            "medium",
+            "-e",
+            "medium",
+            "--parent",
+            &parent,
+        ])
+        .current_dir(&tmp)
+        .output()
+        .unwrap();
+    let child: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap();
+    let child_id = child["id"].as_str().unwrap().to_string();
+
+    // Close the subtask.
+    td().args(["done", &child_id])
+        .current_dir(&tmp)
+        .assert()
+        .success();
+
+    let out = td()
+        .args(["--json", "next"])
+        .current_dir(&tmp)
+        .output()
+        .unwrap();
+    let v: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap();
+    let results = v.as_array().unwrap();
+    let ids: Vec<&str> = results.iter().map(|r| r["id"].as_str().unwrap()).collect();
+
+    // Parent should reappear as a candidate once all children are closed.
+    assert!(
+        ids.contains(&parent.as_str()),
+        "parent should be a candidate"
+    );
+}
+
+#[test]
+fn next_nested_parents_excluded_at_each_level() {
+    let tmp = init_tmp();
+    // grandparent → parent → child (nested subtasks)
+    let gp = create_task(&tmp, "Grandparent", "high", "low");
+    let out = td()
+        .args([
+            "--json", "create", "Parent", "-p", "medium", "-e", "medium", "--parent", &gp,
+        ])
+        .current_dir(&tmp)
+        .output()
+        .unwrap();
+    let p: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap();
+    let p_id = p["id"].as_str().unwrap().to_string();
+
+    let out = td()
+        .args([
+            "--json", "create", "Child", "-p", "low", "-e", "low", "--parent", &p_id,
+        ])
+        .current_dir(&tmp)
+        .output()
+        .unwrap();
+    let c: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap();
+    let c_id = c["id"].as_str().unwrap().to_string();
+
+    let out = td()
+        .args(["--json", "next"])
+        .current_dir(&tmp)
+        .output()
+        .unwrap();
+    let v: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap();
+    let results = v.as_array().unwrap();
+    let ids: Vec<&str> = results.iter().map(|r| r["id"].as_str().unwrap()).collect();
+
+    // Both grandparent and parent are excluded; only the leaf child appears.
+    assert!(
+        !ids.contains(&gp.as_str()),
+        "grandparent should be excluded"
+    );
+    assert!(!ids.contains(&p_id.as_str()), "parent should be excluded");
+    assert!(
+        ids.contains(&c_id.as_str()),
+        "leaf child should be a candidate"
+    );
+}