fix(#2500): group_terminal must require the group node itself terminal

group_terminal checked only that every child was terminal, never the group
node's own state — so an empty group whose node is still Running returned
true (empty .all()), making a running node that has yet to append its
subgraph look already-finished. Now it requires the group node itself
terminal AND every child recursively terminal. Deciding when to settle a
group node to terminal once its children are done stays a scheduler concern;
this answers the dependents' question — is the whole group, node included,
finished. Adds group-node-pending-with-child-done + empty-running-group tests.
This commit is contained in:
atlas 2026-07-17 01:19:35 +02:00 committed by mara
commit 581737583e

View file

@ -210,12 +210,18 @@ impl<N> Graph<N> {
self.nodes.iter().filter(move |n| n.parent == Some(id))
}
/// A group is terminal once every node inside it (recursively) is terminal.
/// An empty group is terminal.
/// A group is terminal once the group node itself is terminal *and* every
/// node inside it (recursively) is terminal. The node's own state matters:
/// a group node still `Pending`/`Running` is not terminal even with no
/// children yet, since a running node may still append some. (Deciding when
/// to *settle* a group node to terminal once its children are all done is a
/// separate concern the scheduler owns.) An unknown id is not terminal.
#[must_use]
pub fn group_terminal(&self, id: NodeId) -> bool {
self.children(id)
.all(|child| child.state.is_terminal() && self.group_terminal(child.id))
let Some(node) = self.node(id) else {
return false;
};
node.state.is_terminal() && self.children(id).all(|child| self.group_terminal(child.id))
}
}
@ -242,16 +248,37 @@ mod tests {
assert_eq!(g.node(a).unwrap().parent, None);
}
fn set_state<N>(g: &mut Graph<N>, id: NodeId, state: State) {
let idx = g.nodes.iter().position(|n| n.id == id).unwrap();
g.nodes[idx].state = state;
}
#[test]
fn group_is_terminal_only_when_all_children_terminal() {
fn group_terminal_requires_the_group_node_and_all_children_terminal() {
let mut g: Graph<&str> = Graph::new();
let group = g.insert("group", vec![], None);
let child = g.insert("child", vec![], Some(group));
// Empty-below or pending child → not terminal.
// Both pending → not terminal.
assert!(!g.group_terminal(group));
// Mark the child done.
let idx = g.nodes.iter().position(|n| n.id == child).unwrap();
g.nodes[idx].state = State::Done;
// Child done, but the group node itself is still pending → NOT terminal:
// the group node's own state is load-bearing, not just its children.
set_state(&mut g, child, State::Done);
assert!(!g.group_terminal(group));
// Group node terminal too → the whole group is terminal.
set_state(&mut g, group, State::Done);
assert!(g.group_terminal(group));
}
#[test]
fn empty_running_group_is_not_terminal() {
// A running node with no children yet may still append some, so it must
// not read as terminal just because its child set is currently empty.
let mut g: Graph<&str> = Graph::new();
let group = g.insert("group", vec![], None);
set_state(&mut g, group, State::Running);
assert!(!g.group_terminal(group));
// Once it finishes (having grown no children), it is terminal.
set_state(&mut g, group, State::Done);
assert!(g.group_terminal(group));
}