fix(nbd): fix graph orientation to show goals at root, prerequisites as leaves

Roots are now tickets with no dependents (nobody depends on them —
top-level goals), and the ASCII tree traverses dependency edges so
prerequisites appear indented beneath the goal that needs them.

JSON edges are now {from: dependent, to: dependency} rather than
{from: blocker, to: blocked}.

All graph-related unit and integration tests updated to match the
new semantics.

Closes #668150

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
quotesdb
Elijah Voigt 3 months ago
parent 380cfee6ca
commit 6d40671f28

@ -1,7 +1,7 @@
+++ +++
title = "Fix graph orientation: show goals at root, prerequisites as leaves" title = "Fix graph orientation: show goals at root, prerequisites as leaves"
priority = 7 priority = 7
status = "todo" status = "done"
ticket_type = "bug" ticket_type = "bug"
dependencies = [] dependencies = []
+++ +++

@ -348,16 +348,17 @@ pub fn print_diff(old: &Ticket, new: &Ticket) {
/// Format the full dependency forest as an ASCII tree string. /// Format the full dependency forest as an ASCII tree string.
/// ///
/// Roots (tickets with no in-graph dependencies) appear at column 0, sorted /// Roots (tickets that no other ticket depends on — top-level goals) appear
/// by priority descending. Each root's dependents (tickets that list it as a /// at column 0, sorted by priority descending. Each root's dependencies
/// dependency) are indented below using box-drawing characters: /// (prerequisites it needs completed first) are indented below using
/// box-drawing characters:
/// ///
/// ```text /// ```text
/// a3f9c2 [todo] Fix login bug /// a3f9c2 [todo] Fix login bug
/// ├── b7d41e [in_progress] Add rate limiting /// ├── b7d41e [in_progress] Add rate limiting
/// │ └── c9e823 [todo] Write tests /// │ └── c9e823 [todo] Write tests
/// └── d1f302 [done] Update docs /// └── d1f302 [done] Update docs
/// e4a781 [todo] New feature (no deps) /// e4a781 [todo] New feature (no prereqs)
/// ``` /// ```
/// ///
/// Nodes that appear in multiple subtrees are marked `*` on subsequent /// Nodes that appear in multiple subtrees are marked `*` on subsequent
@ -378,8 +379,8 @@ pub fn print_graph(graph: &TicketGraph<'_>) {
/// Format the subtree rooted at `root_id` as an ASCII tree string. /// Format the subtree rooted at `root_id` as an ASCII tree string.
/// ///
/// Renders `root_id` and every ticket that transitively depends on it /// Renders `root_id` and every ticket it transitively depends on
/// (via dependent edges), using the same box-drawing format as /// (via dependency edges), using the same box-drawing format as
/// [`format_graph`]. Returns an empty string when `root_id` is not in the /// [`format_graph`]. Returns an empty string when `root_id` is not in the
/// graph. /// graph.
pub fn format_subtree(graph: &TicketGraph<'_>, root_id: &str) -> String { pub fn format_subtree(graph: &TicketGraph<'_>, root_id: &str) -> String {
@ -398,7 +399,7 @@ pub fn print_subtree(graph: &TicketGraph<'_>, root_id: &str) {
// ── Internal graph helpers ──────────────────────────────────────────────────── // ── Internal graph helpers ────────────────────────────────────────────────────
/// Recursively render a single node and its dependents into `out`. /// Recursively render a single node and its dependencies into `out`.
/// ///
/// Parameters: /// Parameters:
/// - `prefix` — the indentation printed before `connector` on this node's line. /// - `prefix` — the indentation printed before `connector` on this node's line.
@ -429,11 +430,11 @@ fn render_node(
// Extract the node data we need, cloning so we can drop the borrow before // Extract the node data we need, cloning so we can drop the borrow before
// recursing (the recursive call needs a mutable `visited`). // recursing (the recursive call needs a mutable `visited`).
let (status_s, title, dependents) = match graph.get_node(id) { let (status_s, title, dependencies) = match graph.get_node(id) {
Some(node) => { Some(node) => {
let s = status_str(&node.ticket.status); let s = status_str(&node.ticket.status);
let t = node.ticket.title.clone(); let t = node.ticket.title.clone();
let d: Vec<String> = node.dependents.iter().map(|s| s.to_string()).collect(); let d: Vec<String> = node.dependencies.iter().map(|s| s.to_string()).collect();
(s, t, d) (s, t, d)
} }
None => return, None => return,
@ -444,8 +445,8 @@ fn render_node(
&format!("{prefix}{connector}{id} [{status_s}] {title}"), &format!("{prefix}{connector}{id} [{status_s}] {title}"),
); );
let n = dependents.len(); let n = dependencies.len();
for (i, dep_id) in dependents.iter().enumerate() { for (i, dep_id) in dependencies.iter().enumerate() {
let is_last = i == n - 1; let is_last = i == n - 1;
// The child's connector on its own line. // The child's connector on its own line.
let child_connector = if is_last { "└── " } else { "├── " }; let child_connector = if is_last { "└── " } else { "├── " };

@ -10,12 +10,12 @@
//! - A is a **dependency** of B (A must be done before B) //! - A is a **dependency** of B (A must be done before B)
//! - B is a **dependent** of A (B is waiting on A) //! - B is a **dependent** of A (B is waiting on A)
//! //!
//! The ASCII tree is rendered with **roots** (tickets with no in-graph //! The ASCII tree is rendered with **roots** (tickets that no other ticket
//! dependencies) at the top and their **dependents** indented below — showing //! depends on — the top-level goals) at the top and their **dependencies**
//! "what is blocked by this ticket?". //! indented below — showing "what does this ticket need to be done first?".
//! //!
//! JSON edges use `{"from": <blocker>, "to": <blocked>}` — i.e. the blocking //! JSON edges use `{"from": <dependent>, "to": <dependency>}` — i.e. the
//! ticket is `from` and the waiting ticket is `to`. //! ticket that is waiting is `from` and the prerequisite ticket is `to`.
use std::collections::{HashMap, HashSet}; use std::collections::{HashMap, HashSet};
@ -48,18 +48,19 @@ pub struct GraphNode<'a> {
/// ///
/// Only tickets present in the graph are listed; dangling references in /// Only tickets present in the graph are listed; dangling references in
/// [`Ticket::dependencies`] are silently ignored. /// [`Ticket::dependencies`] are silently ignored.
///
/// These are the visual "children" in the ASCII dependency tree — the
/// prerequisites that must be completed before this ticket.
pub dependencies: Vec<&'a str>, pub dependencies: Vec<&'a str>,
/// IDs of tickets that list this ticket as a dependency (reverse edges). /// IDs of tickets that list this ticket as a dependency (reverse edges).
///
/// These are the visual "children" in the ASCII dependency tree.
pub dependents: Vec<&'a str>, pub dependents: Vec<&'a str>,
} }
/// A directed dependency graph built from a flat list of tickets. /// A directed dependency graph built from a flat list of tickets.
/// ///
/// Build with [`TicketGraph::build`]. Roots (entry points) are returned by /// Build with [`TicketGraph::build`]. Roots (top-level goals) are returned by
/// [`TicketGraph::roots`]. Use [`TicketGraph::subtree`] to extract the IDs /// [`TicketGraph::roots`]. Use [`TicketGraph::subtree`] to extract the IDs
/// reachable from a specific ticket via its dependents, and /// reachable from a specific ticket via its dependencies, and
/// [`TicketGraph::to_json_value`] for machine-readable output. /// [`TicketGraph::to_json_value`] for machine-readable output.
pub struct TicketGraph<'a> { pub struct TicketGraph<'a> {
/// Nodes keyed by ticket ID. /// Nodes keyed by ticket ID.
@ -128,17 +129,18 @@ impl<'a> TicketGraph<'a> {
TicketGraph { nodes, ids } TicketGraph { nodes, ids }
} }
/// Return tickets with no in-graph dependencies, sorted by priority descending. /// Return tickets that no other ticket depends on, sorted by priority descending.
/// ///
/// These are the entry points for the ASCII dependency tree renderer. /// These are the top-level goals for the ASCII dependency tree renderer.
/// A ticket is a root when its `dependencies` list (after filtering to /// A ticket is a root when its `dependents` list is empty — nothing in the
/// in-graph tickets only) is empty. /// graph is waiting on it, so it represents an end goal rather than a
/// prerequisite.
pub fn roots(&self) -> Vec<&'a Ticket> { pub fn roots(&self) -> Vec<&'a Ticket> {
let mut roots: Vec<&'a Ticket> = self let mut roots: Vec<&'a Ticket> = self
.ids .ids
.iter() .iter()
.filter_map(|id| self.nodes.get(id)) .filter_map(|id| self.nodes.get(id))
.filter(|node| node.dependencies.is_empty()) .filter(|node| node.dependents.is_empty())
.map(|node| node.ticket) .map(|node| node.ticket)
.collect(); .collect();
roots.sort_by(|a, b| b.priority.cmp(&a.priority)); roots.sort_by(|a, b| b.priority.cmp(&a.priority));
@ -150,13 +152,13 @@ impl<'a> TicketGraph<'a> {
self.nodes.get(id) self.nodes.get(id)
} }
/// Return all ticket IDs reachable from `root_id` via dependent edges, /// Return all ticket IDs reachable from `root_id` via dependency edges,
/// in depth-first order, including `root_id` itself. /// in depth-first order, including `root_id` itself.
/// ///
/// "Reachable via dependents" means: `root_id`, plus every ticket that /// "Reachable via dependencies" means: `root_id`, plus every ticket that
/// depends on `root_id`, plus every ticket that depends on those, and so /// `root_id` depends on, plus every ticket those depend on, and so on.
/// on. This answers "what tickets are blocked (directly or transitively) /// This answers "what tickets does `root_id` need (directly or
/// by `root_id`?". /// transitively)?".
/// ///
/// Cycles are handled by a visited set — each ID appears at most once. /// Cycles are handled by a visited set — each ID appears at most once.
/// Returns an empty `Vec` when `root_id` is not in the graph. /// Returns an empty `Vec` when `root_id` is not in the graph.
@ -174,8 +176,8 @@ impl<'a> TicketGraph<'a> {
/// Each node includes `id`, `title`, `status`, `priority`, and /// Each node includes `id`, `title`, `status`, `priority`, and
/// `dependencies` (only in-graph dependency IDs). /// `dependencies` (only in-graph dependency IDs).
/// ///
/// Each edge is `{"from": <blocker_id>, "to": <blocked_id>}` — meaning /// Each edge is `{"from": <dependent_id>, "to": <dependency_id>}` — meaning
/// the ticket identified by `from` must be completed before `to` can start. /// the ticket identified by `from` depends on (must wait for) `to`.
pub fn to_json_value(&self) -> serde_json::Value { pub fn to_json_value(&self) -> serde_json::Value {
let nodes: Vec<serde_json::Value> = self let nodes: Vec<serde_json::Value> = self
.ids .ids
@ -192,14 +194,14 @@ impl<'a> TicketGraph<'a> {
}) })
.collect(); .collect();
// Edges point from the blocking ticket to the blocked ticket. // Edges point from the dependent (waiting) ticket to the dependency (prerequisite).
let edges: Vec<serde_json::Value> = self let edges: Vec<serde_json::Value> = self
.ids .ids
.iter() .iter()
.filter_map(|id| self.nodes.get(id)) .filter_map(|id| self.nodes.get(id))
.flat_map(|node| { .flat_map(|node| {
let from = node.ticket.id.as_str(); let from = node.ticket.id.as_str();
node.dependents node.dependencies
.iter() .iter()
.map(move |&to| serde_json::json!({ "from": from, "to": to })) .map(move |&to| serde_json::json!({ "from": from, "to": to }))
}) })
@ -211,7 +213,7 @@ impl<'a> TicketGraph<'a> {
/// Serialise the subtree rooted at `root_id` as a JSON object. /// Serialise the subtree rooted at `root_id` as a JSON object.
/// ///
/// Same structure as [`to_json_value`] but limited to nodes and edges /// Same structure as [`to_json_value`] but limited to nodes and edges
/// within the subtree reachable from `root_id` via dependent edges. /// within the subtree reachable from `root_id` via dependency edges.
/// Returns an empty `{"nodes":[],"edges":[]}` object when `root_id` is /// Returns an empty `{"nodes":[],"edges":[]}` object when `root_id` is
/// not in the graph. /// not in the graph.
pub fn to_subtree_json_value(&self, root_id: &str) -> serde_json::Value { pub fn to_subtree_json_value(&self, root_id: &str) -> serde_json::Value {
@ -246,7 +248,7 @@ impl<'a> TicketGraph<'a> {
.filter_map(|id| self.nodes.get(id)) .filter_map(|id| self.nodes.get(id))
.flat_map(|node| { .flat_map(|node| {
let from = node.ticket.id.as_str(); let from = node.ticket.id.as_str();
node.dependents node.dependencies
.iter() .iter()
.copied() .copied()
.filter(|&to| reachable.contains(to)) .filter(|&to| reachable.contains(to))
@ -260,10 +262,10 @@ impl<'a> TicketGraph<'a> {
// ── Private helpers ─────────────────────────────────────────────────────────── // ── Private helpers ───────────────────────────────────────────────────────────
/// Depth-first traversal following `dependents` edges. /// Depth-first traversal following `dependencies` edges.
/// ///
/// Visits `id` and recursively visits each of its dependents (tickets that /// Visits `id` and recursively visits each of its dependencies (tickets that
/// depend on it). A `visited` set prevents revisiting nodes, making the /// `id` depends on). A `visited` set prevents revisiting nodes, making the
/// function safe even when the data contains dependency cycles. /// function safe even when the data contains dependency cycles.
fn dfs_subtree<'a>( fn dfs_subtree<'a>(
nodes: &HashMap<&'a str, GraphNode<'a>>, nodes: &HashMap<&'a str, GraphNode<'a>>,
@ -277,8 +279,8 @@ fn dfs_subtree<'a>(
result.push(id); result.push(id);
if let Some(node) = nodes.get(id) { if let Some(node) = nodes.get(id) {
// Clone the list to avoid holding an immutable borrow while we recurse. // Clone the list to avoid holding an immutable borrow while we recurse.
let dependents: Vec<&'a str> = node.dependents.clone(); let dependencies: Vec<&'a str> = node.dependencies.clone();
for dep_id in dependents { for dep_id in dependencies {
dfs_subtree(nodes, dep_id, visited, result); dfs_subtree(nodes, dep_id, visited, result);
} }
} }

@ -1068,7 +1068,7 @@ mod graph {
assert!(ids.contains(&"bbbbbb")); assert!(ids.contains(&"bbbbbb"));
} }
/// When B depends on A, only A is a root. /// When B depends on A, only B is a root (B is the goal; A is a prerequisite).
#[test] #[test]
fn roots_with_chain() { fn roots_with_chain() {
let a = make_ticket("aaaaaa", &[]); let a = make_ticket("aaaaaa", &[]);
@ -1077,7 +1077,7 @@ mod graph {
let graph = TicketGraph::build(&tickets); let graph = TicketGraph::build(&tickets);
let roots = graph.roots(); let roots = graph.roots();
assert_eq!(roots.len(), 1); assert_eq!(roots.len(), 1);
assert_eq!(roots[0].id, "aaaaaa"); assert_eq!(roots[0].id, "bbbbbb");
} }
/// Roots are sorted by priority descending. /// Roots are sorted by priority descending.
@ -1095,7 +1095,7 @@ mod graph {
} }
/// `subtree` on a linear chain A → B → C (B depends on A, C depends on B) /// `subtree` on a linear chain A → B → C (B depends on A, C depends on B)
/// returns all three IDs when starting from A. /// returns all three IDs when starting from C (the top-level goal).
#[test] #[test]
fn subtree_linear_chain() { fn subtree_linear_chain() {
let a = make_ticket("aaaaaa", &[]); let a = make_ticket("aaaaaa", &[]);
@ -1104,14 +1104,14 @@ mod graph {
let tickets = vec![a, b, c]; let tickets = vec![a, b, c];
let graph = TicketGraph::build(&tickets); let graph = TicketGraph::build(&tickets);
let sub = graph.subtree("aaaaaa"); let sub = graph.subtree("cccccc");
assert_eq!(sub.len(), 3, "subtree should include all three tickets"); assert_eq!(sub.len(), 3, "subtree should include all three tickets");
assert!(sub.contains(&"aaaaaa")); assert!(sub.contains(&"aaaaaa"));
assert!(sub.contains(&"bbbbbb")); assert!(sub.contains(&"bbbbbb"));
assert!(sub.contains(&"cccccc")); assert!(sub.contains(&"cccccc"));
} }
/// `subtree` on a leaf node (no dependents) returns just that ID. /// `subtree` on a leaf node (no dependencies) returns just that ID.
#[test] #[test]
fn subtree_leaf() { fn subtree_leaf() {
let a = make_ticket("aaaaaa", &[]); let a = make_ticket("aaaaaa", &[]);
@ -1119,8 +1119,8 @@ mod graph {
let tickets = vec![a, b]; let tickets = vec![a, b];
let graph = TicketGraph::build(&tickets); let graph = TicketGraph::build(&tickets);
let sub = graph.subtree("bbbbbb"); let sub = graph.subtree("aaaaaa");
assert_eq!(sub, vec!["bbbbbb"]); assert_eq!(sub, vec!["aaaaaa"]);
} }
/// `subtree` on an unknown ID returns an empty vec. /// `subtree` on an unknown ID returns an empty vec.
@ -1169,9 +1169,9 @@ mod graph {
assert!(node_ids.contains(&"bbbbbb")); assert!(node_ids.contains(&"bbbbbb"));
assert!(node_ids.contains(&"cccccc")); assert!(node_ids.contains(&"cccccc"));
// Every edge should have "from" (blocker) = "aaaaaa". // Every edge should have "to" (dependency/prerequisite) = "aaaaaa".
for edge in edges { for edge in edges {
assert_eq!(edge["from"].as_str().unwrap(), "aaaaaa"); assert_eq!(edge["to"].as_str().unwrap(), "aaaaaa");
} }
} }
@ -1189,16 +1189,20 @@ mod graph {
fn to_subtree_json_value_scoped() { fn to_subtree_json_value_scoped() {
let a = make_ticket("aaaaaa", &[]); let a = make_ticket("aaaaaa", &[]);
let b = make_ticket("bbbbbb", &["aaaaaa"]); // depends on a let b = make_ticket("bbbbbb", &["aaaaaa"]); // depends on a
let c = make_ticket("cccccc", &[]); // unrelated root let c = make_ticket("cccccc", &[]); // unrelated
let tickets = vec![a, b, c]; let tickets = vec![a, b, c];
let graph = TicketGraph::build(&tickets); let graph = TicketGraph::build(&tickets);
let json = graph.to_subtree_json_value("aaaaaa"); // Starting from bbbbbb (the goal): subtree includes bbbbbb + its dependency aaaaaa.
let json = graph.to_subtree_json_value("bbbbbb");
let nodes = json["nodes"].as_array().unwrap(); let nodes = json["nodes"].as_array().unwrap();
let node_ids: Vec<&str> = nodes.iter().map(|n| n["id"].as_str().unwrap()).collect(); let node_ids: Vec<&str> = nodes.iter().map(|n| n["id"].as_str().unwrap()).collect();
assert!(node_ids.contains(&"aaaaaa"), "root should be included"); assert!(node_ids.contains(&"bbbbbb"), "root should be included");
assert!(node_ids.contains(&"bbbbbb"), "dependent should be included"); assert!(
node_ids.contains(&"aaaaaa"),
"dependency should be included"
);
assert!( assert!(
!node_ids.contains(&"cccccc"), !node_ids.contains(&"cccccc"),
"unrelated ticket should be excluded" "unrelated ticket should be excluded"
@ -1254,8 +1258,8 @@ mod display_graph {
assert!(!out.contains("└──"), "should have no branch connectors"); assert!(!out.contains("└──"), "should have no branch connectors");
} }
/// A two-ticket chain (B depends on A) renders A at the top level and B /// A two-ticket chain (B depends on A) renders B at the top level (the goal)
/// indented below it with `└──`. /// and A indented below it with `└──` (the prerequisite).
#[test] #[test]
fn two_ticket_chain() { fn two_ticket_chain() {
let a = make_ticket("aaaaaa", &[]); let a = make_ticket("aaaaaa", &[]);
@ -1264,26 +1268,27 @@ mod display_graph {
let graph = TicketGraph::build(&tickets); let graph = TicketGraph::build(&tickets);
let out = format_graph(&graph); let out = format_graph(&graph);
// A should appear before B. // B should appear before A (B is the goal, A is the prerequisite).
let pos_a = out.find("aaaaaa").expect("aaaaaa should appear"); let pos_a = out.find("aaaaaa").expect("aaaaaa should appear");
let pos_b = out.find("bbbbbb").expect("bbbbbb should appear"); let pos_b = out.find("bbbbbb").expect("bbbbbb should appear");
assert!( assert!(
pos_a < pos_b, pos_b < pos_a,
"root (aaaaaa) should appear before dependent (bbbbbb)" "goal (bbbbbb) should appear before prerequisite (aaaaaa)"
); );
// B's line should use the └── connector. // A's line should use the └── connector.
assert!(out.contains("└──"), "last (only) child should use └──"); assert!(out.contains("└──"), "last (only) child should use └──");
assert!(!out.contains("├──"), "only child should not use ├──"); assert!(!out.contains("├──"), "only child should not use ├──");
} }
/// When a root has two dependents, the first uses `├──` and the last `└──`. /// When a goal depends on two prerequisites, the first uses `├──` and the last `└──`.
#[test] #[test]
fn branching_parent() { fn branching_goal() {
let root = make_ticket("aaaaaa", &[]); let a = make_ticket("aaaaaa", &[]);
let b = make_ticket("bbbbbb", &["aaaaaa"]); let b = make_ticket("bbbbbb", &[]);
let c = make_ticket("cccccc", &["aaaaaa"]); // cccccc depends on both aaaaaa and bbbbbb, so it renders with two children.
let tickets = vec![root, b, c]; let c = make_ticket("cccccc", &["aaaaaa", "bbbbbb"]);
let tickets = vec![a, b, c];
let graph = TicketGraph::build(&tickets); let graph = TicketGraph::build(&tickets);
let out = format_graph(&graph); let out = format_graph(&graph);
@ -1291,7 +1296,7 @@ mod display_graph {
assert!(out.contains("└──"), "last child should use └──"); assert!(out.contains("└──"), "last child should use └──");
} }
/// `format_subtree` for a root only includes that root and its dependents, /// `format_subtree` for a goal includes that goal and its dependencies,
/// not unrelated tickets. /// not unrelated tickets.
#[test] #[test]
fn subtree_excludes_unrelated() { fn subtree_excludes_unrelated() {
@ -1301,9 +1306,10 @@ mod display_graph {
let tickets = vec![a, b, c]; let tickets = vec![a, b, c];
let graph = TicketGraph::build(&tickets); let graph = TicketGraph::build(&tickets);
let out = format_subtree(&graph, "aaaaaa"); // Starting from bbbbbb (the goal): renders bbbbbb and its dependency aaaaaa.
assert!(out.contains("aaaaaa"), "root should be present"); let out = format_subtree(&graph, "bbbbbb");
assert!(out.contains("bbbbbb"), "dependent should be present"); assert!(out.contains("bbbbbb"), "goal should be present");
assert!(out.contains("aaaaaa"), "dependency should be present");
assert!(!out.contains("cccccc"), "unrelated ticket should be absent"); assert!(!out.contains("cccccc"), "unrelated ticket should be absent");
} }
@ -1329,7 +1335,7 @@ mod display_graph {
let tickets = vec![a, b]; let tickets = vec![a, b];
let graph = TicketGraph::build(&tickets); let graph = TicketGraph::build(&tickets);
// format_subtree drives rendering from "aaaaaa"; when it tries to // format_subtree drives rendering from "aaaaaa"; when it tries to
// revisit "aaaaaa" via bbbbbb's dependent edge, it should mark with *. // revisit "aaaaaa" via bbbbbb's dependency edge, it should mark with *.
let out = format_subtree(&graph, "aaaaaa"); let out = format_subtree(&graph, "aaaaaa");
assert!( assert!(
out.contains(" *"), out.contains(" *"),

@ -1757,8 +1757,8 @@ fn graph_all_no_deps() {
); );
} }
/// When B depends on A, `nbd graph` shows A at the top level and B indented /// When B depends on A, `nbd graph` shows B at the top level (the goal) and A
/// below it with `└──`. /// indented below it with `└──` (the prerequisite).
#[test] #[test]
fn graph_chain() { fn graph_chain() {
let env = TestEnv::new(); let env = TestEnv::new();
@ -1769,21 +1769,24 @@ fn graph_chain() {
assert!(output.status.success()); assert!(output.status.success());
let stdout = String::from_utf8(output.stdout).unwrap(); let stdout = String::from_utf8(output.stdout).unwrap();
// A should appear before B. // B should appear before A (B is the goal, A is the prerequisite).
let pos_a = stdout.find(&id_a).expect("id_a not found"); let pos_a = stdout.find(&id_a).expect("id_a not found");
let pos_b = stdout.find(&id_b).expect("id_b not found"); let pos_b = stdout.find(&id_b).expect("id_b not found");
assert!(pos_a < pos_b, "root (A) should appear before dependent (B)"); assert!(
pos_b < pos_a,
"goal (B) should appear before prerequisite (A)"
);
// B's line should use `└──`. // A's line should use `└──`.
let line_b = stdout.lines().find(|l| l.contains(&id_b)).unwrap(); let line_a = stdout.lines().find(|l| l.contains(&id_a)).unwrap();
assert!( assert!(
line_b.contains("└──"), line_a.contains("└──"),
"child should use └── connector: {line_b}" "prerequisite should use └── connector: {line_a}"
); );
} }
/// `nbd graph <id>` for ticket A shows A and its dependents but not unrelated /// `nbd graph <id>` for ticket B (which depends on A) shows B and its
/// tickets. /// dependency A, but not unrelated tickets.
#[test] #[test]
fn graph_single_ticket_subtree() { fn graph_single_ticket_subtree() {
let env = TestEnv::new(); let env = TestEnv::new();
@ -1791,12 +1794,12 @@ fn graph_single_ticket_subtree() {
let id_b = env.create(&["--title", "Beta", "--deps", &id_a]); let id_b = env.create(&["--title", "Beta", "--deps", &id_a]);
let id_c = env.create(&["--title", "Gamma"]); // unrelated let id_c = env.create(&["--title", "Gamma"]); // unrelated
let output = env.run(&["graph", &id_a]); let output = env.run(&["graph", &id_b]);
assert!(output.status.success()); assert!(output.status.success());
let stdout = String::from_utf8(output.stdout).unwrap(); let stdout = String::from_utf8(output.stdout).unwrap();
assert!(stdout.contains(&id_a), "root should be in subtree"); assert!(stdout.contains(&id_b), "goal should be in subtree");
assert!(stdout.contains(&id_b), "dependent should be in subtree"); assert!(stdout.contains(&id_a), "dependency should be in subtree");
assert!( assert!(
!stdout.contains(&id_c), !stdout.contains(&id_c),
"unrelated ticket should be absent: {stdout}" "unrelated ticket should be absent: {stdout}"
@ -1832,12 +1835,17 @@ fn graph_json_output() {
node_ids.contains(&id_b.as_str()), node_ids.contains(&id_b.as_str()),
"nodes should include id_b" "nodes should include id_b"
); );
assert_eq!(edges.len(), 1, "should have exactly one edge (A blocks B)"); assert_eq!(
assert_eq!(edges[0]["from"].as_str().unwrap(), id_a); edges.len(),
assert_eq!(edges[0]["to"].as_str().unwrap(), id_b); 1,
"should have exactly one edge (B depends on A)"
);
assert_eq!(edges[0]["from"].as_str().unwrap(), id_b);
assert_eq!(edges[0]["to"].as_str().unwrap(), id_a);
} }
/// `nbd graph <id> --json` returns only nodes and edges reachable from <id>. /// `nbd graph <id> --json` returns only nodes and edges reachable from <id>
/// via dependency edges (the goal and its transitive prerequisites).
#[test] #[test]
fn graph_json_subtree() { fn graph_json_subtree() {
let env = TestEnv::new(); let env = TestEnv::new();
@ -1845,15 +1853,16 @@ fn graph_json_subtree() {
let id_b = env.create(&["--title", "Beta", "--deps", &id_a]); let id_b = env.create(&["--title", "Beta", "--deps", &id_a]);
let id_c = env.create(&["--title", "Gamma"]); // unrelated let id_c = env.create(&["--title", "Gamma"]); // unrelated
let output = env.run(&["graph", &id_a, "--json"]); // Starting from id_b (the goal): subtree includes id_b and its dependency id_a.
let output = env.run(&["graph", &id_b, "--json"]);
assert!(output.status.success()); assert!(output.status.success());
let stdout = String::from_utf8(output.stdout).unwrap(); let stdout = String::from_utf8(output.stdout).unwrap();
let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap(); let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap();
let nodes = parsed["nodes"].as_array().unwrap(); let nodes = parsed["nodes"].as_array().unwrap();
let node_ids: Vec<&str> = nodes.iter().map(|n| n["id"].as_str().unwrap()).collect(); let node_ids: Vec<&str> = nodes.iter().map(|n| n["id"].as_str().unwrap()).collect();
assert!(node_ids.contains(&id_a.as_str()));
assert!(node_ids.contains(&id_b.as_str())); assert!(node_ids.contains(&id_b.as_str()));
assert!(node_ids.contains(&id_a.as_str()));
assert!( assert!(
!node_ids.contains(&id_c.as_str()), !node_ids.contains(&id_c.as_str()),
"unrelated ticket should be excluded from JSON subtree" "unrelated ticket should be excluded from JSON subtree"

Loading…
Cancel
Save