From d48826dc925869d1aa2a6e04ebca816e08a54f04 Mon Sep 17 00:00:00 2001 From: Amolith Date: Wed, 25 Feb 2026 22:07:33 +0000 Subject: [PATCH] Exclude parent tasks with open subtasks from next candidates --- src/cmd/next.rs | 18 ++++++- src/score.rs | 73 +++++++++++++++++++++----- tests/cli_next.rs | 130 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 208 insertions(+), 13 deletions(-) diff --git a/src/cmd/next.rs b/src/cmd/next.rs index 904a7616a6368ebc5aa46ef8861fcbcad4be39bb..fe9e082a349437f2715089c51c4e6f8f00165335 100644 --- a/src/cmd/next.rs +++ b/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::>()?; - 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 = parent_stmt + .query_map([], |r| r.get(0))? + .collect::>()?; + + let scored = score::rank( + &open_tasks, + &edges, + &parents_with_open_children, + mode, + limit, + ); if scored.is_empty() { if json { diff --git a/src/score.rs b/src/score.rs index 41a53e62cf9032625936416a2e8d7aca57c9bffb..cc9973c454930ce56753933aa2cef4e05b30d1c5 100644 --- a/src/score.rs +++ b/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, mode: Mode, limit: usize, ) -> Vec { @@ -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 = ["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 = ["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"); + } } diff --git a/tests/cli_next.rs b/tests/cli_next.rs index 31afe4b459bf38b59613478ede873c02d5091497..341e0f4de942d749eb41376137f63a4411a9139d 100644 --- a/tests/cli_next.rs +++ b/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" + ); +}