From d0dd720316b47694b657f6da75318adf7dc4e001 Mon Sep 17 00:00:00 2001 From: Elijah Voigt Date: Sun, 22 Feb 2026 19:58:28 -0800 Subject: [PATCH] feat(nbd): add graph computation module (src/graph.rs) [9c9ebe] Implements TicketGraph<'a> with build(), roots(), subtree(), to_json_value(), and to_subtree_json_value(). Tracks both forward (dependencies) and reverse (dependents) edges. Cycle-safe DFS via visited set. No new crate dependencies. 14 unit tests covering empty graph, chains, branching, cycles, dangling refs, and JSON output. Co-Authored-By: Claude Sonnet 4.6 --- nbd/.nbd/tickets/9c9ebe.md | 76 ++++++++++ nbd/src/graph.rs | 283 +++++++++++++++++++++++++++++++++++++ nbd/src/main.rs | 1 + nbd/src/tests.rs | 192 +++++++++++++++++++++++++ 4 files changed, 552 insertions(+) create mode 100644 nbd/.nbd/tickets/9c9ebe.md create mode 100644 nbd/src/graph.rs diff --git a/nbd/.nbd/tickets/9c9ebe.md b/nbd/.nbd/tickets/9c9ebe.md new file mode 100644 index 0000000..85a2093 --- /dev/null +++ b/nbd/.nbd/tickets/9c9ebe.md @@ -0,0 +1,76 @@ ++++ +title = "Add graph computation module (src/graph.rs)" +priority = 5 +status = "done" +ticket_type = "feature" +dependencies = [] ++++ +Implement `src/graph.rs` — a module that builds a directed dependency graph from a flat list of tickets and provides the data structures needed by the ASCII renderer and JSON output. + +## Motivation + +The `nbd graph` command (see CLI ticket) needs to traverse `Ticket.dependencies` edges to produce an ordered, tree-structured representation of the dependency DAG. This module isolates that logic from I/O and rendering. + +## Data structures + +```rust +/// A node in the dependency graph. +pub struct GraphNode<'a> { + pub ticket: &'a Ticket, + /// Direct dependents (tickets that list this ticket as a dependency). + pub dependents: Vec<&'a str>, + /// Direct dependencies (tickets this ticket depends on). + pub dependencies: Vec<&'a str>, +} + +/// A directed dependency graph built from a flat list of tickets. +pub struct TicketGraph<'a> { + nodes: IndexMap<&'a str, GraphNode<'a>>, +} +``` + +## Functions to implement + +### `TicketGraph::build(tickets: &[Ticket]) -> TicketGraph` +- Constructs `nodes` map keyed by ticket ID. +- Iterates `ticket.dependencies` to populate both `dependencies` (forward) and `dependents` (reverse) edges. +- IDs in `dependencies` that do not correspond to a known ticket are silently ignored (dangling references are tolerated). + +### `TicketGraph::roots(&self) -> Vec<&Ticket>` +- Returns tickets with no dependencies (or whose dependencies are all outside the graph). +- Sorted by priority descending (same ordering as `list_tickets`). +- These become the starting points for the recursive ASCII tree renderer. + +### `TicketGraph::subtree(&self, root_id: &str) -> Vec<&str>` +- Returns all ticket IDs reachable from `root_id` by following `dependencies` edges in depth-first order, including `root_id` itself. +- Cycles: track visited set; stop recursion when an ID has been visited. This makes the function safe even if the data contains cycles. + +### `TicketGraph::to_json_value(&self) -> serde_json::Value` +- Returns an object like: + ```json + { + "nodes": [ + {"id": "a3f9c2", "title": "...", "status": "todo", "dependencies": ["b7d41e"]}, + ... + ], + "edges": [ + {"from": "a3f9c2", "to": "b7d41e"}, + ... + ] + } + ``` + +## Crate dependencies + +No new crates needed. If `IndexMap` insertion-order is useful, `indexmap` can be added — but a `HashMap` with a separate sorted `Vec<&str>` of keys also works. Prefer whatever avoids adding a new crate dependency. + +## Files touched +- `src/graph.rs` — new file, public module +- `src/main.rs` — `mod graph;` declaration + +## Tests (unit, in `src/tests.rs`) +- `build` with an empty slice returns an empty graph. +- `roots` returns only tickets with no in-graph dependencies. +- `subtree` returns the correct set of IDs for a linear chain. +- `subtree` does not infinite-loop when the data contains a cycle. +- `to_json_value` contains all expected IDs in `nodes` and all edges in `edges`. \ No newline at end of file diff --git a/nbd/src/graph.rs b/nbd/src/graph.rs new file mode 100644 index 0000000..0457612 --- /dev/null +++ b/nbd/src/graph.rs @@ -0,0 +1,283 @@ +//! Ticket dependency graph computation. +//! +//! Builds a directed graph from a flat list of tickets, tracking both forward +//! (dependency) and reverse (dependent) edges. Used by [`crate::display`] to +//! render an ASCII tree and by the `graph` CLI command for JSON output. +//! +//! ## Edge semantics +//! +//! If ticket B lists ticket A in its `dependencies`, then: +//! - A is a **dependency** of B (A must be done before B) +//! - B is a **dependent** of A (B is waiting on A) +//! +//! The ASCII tree is rendered with **roots** (tickets with no in-graph +//! dependencies) at the top and their **dependents** indented below — showing +//! "what is blocked by this ticket?". +//! +//! JSON edges use `{"from": , "to": }` — i.e. the blocking +//! ticket is `from` and the waiting ticket is `to`. + +use std::collections::{HashMap, HashSet}; + +use crate::ticket::{Status, Ticket}; + +// ── Internal helper ─────────────────────────────────────────────────────────── + +/// Return the canonical display string for a [`Status`] variant. +fn status_str(status: &Status) -> &'static str { + match status { + Status::Todo => "todo", + Status::InProgress => "in_progress", + Status::Done => "done", + Status::Closed => "closed", + } +} + +// ── Public types ────────────────────────────────────────────────────────────── + +/// A single node in the dependency graph. +/// +/// Holds a reference to the source [`Ticket`] and the IDs of both its +/// in-graph dependencies (forward edges) and its dependents (reverse edges). +pub struct GraphNode<'a> { + /// The ticket this node represents. + pub ticket: &'a Ticket, + /// IDs of in-graph tickets that this ticket depends on (forward edges). + /// + /// Only tickets present in the graph are listed; dangling references in + /// [`Ticket::dependencies`] are silently ignored. + pub dependencies: Vec<&'a str>, + /// 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>, +} + +/// A directed dependency graph built from a flat list of tickets. +/// +/// Build with [`TicketGraph::build`]. Roots (entry points) are returned by +/// [`TicketGraph::roots`]. Use [`TicketGraph::subtree`] to extract the IDs +/// reachable from a specific ticket via its dependents, and +/// [`TicketGraph::to_json_value`] for machine-readable output. +pub struct TicketGraph<'a> { + /// Nodes keyed by ticket ID. + nodes: HashMap<&'a str, GraphNode<'a>>, + /// Ticket IDs in the order they were inserted, for stable iteration. + ids: Vec<&'a str>, +} + +impl<'a> TicketGraph<'a> { + /// Build a graph from a slice of tickets. + /// + /// Each ticket in `tickets` becomes a node. Forward edges (`dependencies`) + /// and reverse edges (`dependents`) are both populated. References to IDs + /// not present in `tickets` are silently ignored. + /// + /// # Example + /// + /// ```rust,ignore + /// let graph = TicketGraph::build(&tickets); + /// for root in graph.roots() { + /// println!("{} – {}", root.id, root.title); + /// } + /// ``` + pub fn build(tickets: &'a [Ticket]) -> Self { + let mut nodes: HashMap<&'a str, GraphNode<'a>> = HashMap::with_capacity(tickets.len()); + let mut ids: Vec<&'a str> = Vec::with_capacity(tickets.len()); + + // First pass: create a node for every ticket. + for ticket in tickets { + let id: &'a str = ticket.id.as_str(); + nodes.insert( + id, + GraphNode { + ticket, + dependencies: Vec::new(), + dependents: Vec::new(), + }, + ); + ids.push(id); + } + + // Second pass: collect edges (avoids simultaneous mutable borrows). + // Each edge is (dependent_id, dependency_id). + let mut edges: Vec<(&'a str, &'a str)> = Vec::new(); + for ticket in tickets { + let ticket_id: &'a str = ticket.id.as_str(); + for dep_id_owned in &ticket.dependencies { + let dep_id: &str = dep_id_owned.as_str(); + // Only add an edge if the dependency exists in this graph. + if let Some((&stored_dep_id, _)) = nodes.get_key_value(dep_id) { + edges.push((ticket_id, stored_dep_id)); + } + } + } + + // Apply collected edges to both sides of each node. + for (dependent_id, dependency_id) in edges { + if let Some(node) = nodes.get_mut(dependent_id) { + node.dependencies.push(dependency_id); + } + if let Some(node) = nodes.get_mut(dependency_id) { + node.dependents.push(dependent_id); + } + } + + TicketGraph { nodes, ids } + } + + /// Return tickets with no in-graph dependencies, sorted by priority descending. + /// + /// These are the entry points for the ASCII dependency tree renderer. + /// A ticket is a root when its `dependencies` list (after filtering to + /// in-graph tickets only) is empty. + pub fn roots(&self) -> Vec<&'a Ticket> { + let mut roots: Vec<&'a Ticket> = self + .ids + .iter() + .filter_map(|id| self.nodes.get(id)) + .filter(|node| node.dependencies.is_empty()) + .map(|node| node.ticket) + .collect(); + roots.sort_by(|a, b| b.priority.cmp(&a.priority)); + roots + } + + /// Return the node for a ticket ID, or `None` if not present in the graph. + pub fn get_node(&self, id: &str) -> Option<&GraphNode<'a>> { + self.nodes.get(id) + } + + /// Return all ticket IDs reachable from `root_id` via dependent edges, + /// in depth-first order, including `root_id` itself. + /// + /// "Reachable via dependents" means: `root_id`, plus every ticket that + /// depends on `root_id`, plus every ticket that depends on those, and so + /// on. This answers "what tickets are blocked (directly or transitively) + /// by `root_id`?". + /// + /// 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. + pub fn subtree(&self, root_id: &str) -> Vec<&'a str> { + let mut result: Vec<&'a str> = Vec::new(); + let mut visited: HashSet<&'a str> = HashSet::new(); + if let Some((&stored_id, _)) = self.nodes.get_key_value(root_id) { + dfs_subtree(&self.nodes, stored_id, &mut visited, &mut result); + } + result + } + + /// Serialise the full graph as a JSON object with `nodes` and `edges`. + /// + /// Each node includes `id`, `title`, `status`, `priority`, and + /// `dependencies` (only in-graph dependency IDs). + /// + /// Each edge is `{"from": , "to": }` — meaning + /// the ticket identified by `from` must be completed before `to` can start. + pub fn to_json_value(&self) -> serde_json::Value { + let nodes: Vec = self + .ids + .iter() + .filter_map(|id| self.nodes.get(id)) + .map(|node| { + serde_json::json!({ + "id": node.ticket.id, + "title": node.ticket.title, + "status": status_str(&node.ticket.status), + "priority": node.ticket.priority, + "dependencies": node.dependencies, + }) + }) + .collect(); + + // Edges point from the blocking ticket to the blocked ticket. + let edges: Vec = self + .ids + .iter() + .filter_map(|id| self.nodes.get(id)) + .flat_map(|node| { + let from = node.ticket.id.as_str(); + node.dependents + .iter() + .map(move |&to| serde_json::json!({ "from": from, "to": to })) + }) + .collect(); + + serde_json::json!({ "nodes": nodes, "edges": edges }) + } + + /// Serialise the subtree rooted at `root_id` as a JSON object. + /// + /// Same structure as [`to_json_value`] but limited to nodes and edges + /// within the subtree reachable from `root_id` via dependent edges. + /// Returns an empty `{"nodes":[],"edges":[]}` object when `root_id` is + /// not in the graph. + pub fn to_subtree_json_value(&self, root_id: &str) -> serde_json::Value { + let reachable: HashSet<&'a str> = self.subtree(root_id).into_iter().collect(); + + let nodes: Vec = self + .ids + .iter() + .filter(|&&id| reachable.contains(id)) + .filter_map(|id| self.nodes.get(id)) + .map(|node| { + let deps: Vec<&str> = node + .dependencies + .iter() + .copied() + .filter(|&d| reachable.contains(d)) + .collect(); + serde_json::json!({ + "id": node.ticket.id, + "title": node.ticket.title, + "status": status_str(&node.ticket.status), + "priority": node.ticket.priority, + "dependencies": deps, + }) + }) + .collect(); + + let edges: Vec = self + .ids + .iter() + .filter(|&&id| reachable.contains(id)) + .filter_map(|id| self.nodes.get(id)) + .flat_map(|node| { + let from = node.ticket.id.as_str(); + node.dependents + .iter() + .copied() + .filter(|&to| reachable.contains(to)) + .map(move |to| serde_json::json!({ "from": from, "to": to })) + }) + .collect(); + + serde_json::json!({ "nodes": nodes, "edges": edges }) + } +} + +// ── Private helpers ─────────────────────────────────────────────────────────── + +/// Depth-first traversal following `dependents` edges. +/// +/// Visits `id` and recursively visits each of its dependents (tickets that +/// depend on it). A `visited` set prevents revisiting nodes, making the +/// function safe even when the data contains dependency cycles. +fn dfs_subtree<'a>( + nodes: &HashMap<&'a str, GraphNode<'a>>, + id: &'a str, + visited: &mut HashSet<&'a str>, + result: &mut Vec<&'a str>, +) { + if !visited.insert(id) { + return; + } + result.push(id); + if let Some(node) = nodes.get(id) { + // Clone the list to avoid holding an immutable borrow while we recurse. + let dependents: Vec<&'a str> = node.dependents.clone(); + for dep_id in dependents { + dfs_subtree(nodes, dep_id, visited, result); + } + } +} diff --git a/nbd/src/main.rs b/nbd/src/main.rs index e3a56fe..3dbbf84 100644 --- a/nbd/src/main.rs +++ b/nbd/src/main.rs @@ -6,6 +6,7 @@ mod display; mod filter; +mod graph; mod store; mod ticket; diff --git a/nbd/src/tests.rs b/nbd/src/tests.rs index 43d0cf5..fe9830b 100644 --- a/nbd/src/tests.rs +++ b/nbd/src/tests.rs @@ -1005,6 +1005,198 @@ mod filter { } } +// ── graph module ────────────────────────────────────────────────────────────── + +/// Tests for [`crate::graph`]. +mod graph { + use crate::graph::TicketGraph; + use crate::ticket::Ticket; + + /// Build a ticket with a given ID, title, and dependency list. + fn make_ticket(id: &str, deps: &[&str]) -> Ticket { + let mut t = Ticket::new(id.to_string(), format!("Ticket {id}")); + t.dependencies = deps.iter().map(|d| d.to_string()).collect(); + t + } + + /// `build` on an empty slice produces a graph with no nodes. + #[test] + fn build_empty() { + let graph = TicketGraph::build(&[]); + assert!(graph.roots().is_empty()); + assert!(graph.subtree("anything").is_empty()); + } + + /// Two tickets with no dependencies are both roots. + #[test] + fn roots_no_deps() { + let tickets = vec![make_ticket("aaaaaa", &[]), make_ticket("bbbbbb", &[])]; + let graph = TicketGraph::build(&tickets); + let roots = graph.roots(); + assert_eq!(roots.len(), 2); + let ids: Vec<&str> = roots.iter().map(|t| t.id.as_str()).collect(); + assert!(ids.contains(&"aaaaaa")); + assert!(ids.contains(&"bbbbbb")); + } + + /// When B depends on A, only A is a root. + #[test] + fn roots_with_chain() { + let a = make_ticket("aaaaaa", &[]); + let b = make_ticket("bbbbbb", &["aaaaaa"]); + let tickets = vec![a, b]; + let graph = TicketGraph::build(&tickets); + let roots = graph.roots(); + assert_eq!(roots.len(), 1); + assert_eq!(roots[0].id, "aaaaaa"); + } + + /// Roots are sorted by priority descending. + #[test] + fn roots_sorted_by_priority() { + let mut lo = make_ticket("aaaaaa", &[]); + lo.priority = 2; + let mut hi = make_ticket("bbbbbb", &[]); + hi.priority = 8; + let tickets = vec![lo, hi]; + let graph = TicketGraph::build(&tickets); + let roots = graph.roots(); + assert_eq!(roots[0].id, "bbbbbb", "highest priority root first"); + assert_eq!(roots[1].id, "aaaaaa"); + } + + /// `subtree` on a linear chain A → B → C (B depends on A, C depends on B) + /// returns all three IDs when starting from A. + #[test] + fn subtree_linear_chain() { + let a = make_ticket("aaaaaa", &[]); + let b = make_ticket("bbbbbb", &["aaaaaa"]); + let c = make_ticket("cccccc", &["bbbbbb"]); + let tickets = vec![a, b, c]; + let graph = TicketGraph::build(&tickets); + + let sub = graph.subtree("aaaaaa"); + assert_eq!(sub.len(), 3, "subtree should include all three tickets"); + assert!(sub.contains(&"aaaaaa")); + assert!(sub.contains(&"bbbbbb")); + assert!(sub.contains(&"cccccc")); + } + + /// `subtree` on a leaf node (no dependents) returns just that ID. + #[test] + fn subtree_leaf() { + let a = make_ticket("aaaaaa", &[]); + let b = make_ticket("bbbbbb", &["aaaaaa"]); + let tickets = vec![a, b]; + let graph = TicketGraph::build(&tickets); + + let sub = graph.subtree("bbbbbb"); + assert_eq!(sub, vec!["bbbbbb"]); + } + + /// `subtree` on an unknown ID returns an empty vec. + #[test] + fn subtree_unknown_id() { + let tickets = vec![make_ticket("aaaaaa", &[])]; + let graph = TicketGraph::build(&tickets); + assert!(graph.subtree("ffffff").is_empty()); + } + + /// `subtree` does not infinite-loop when the data contains a cycle. + #[test] + fn subtree_cycle_safe() { + let mut a = make_ticket("aaaaaa", &["bbbbbb"]); + a.dependencies = vec!["bbbbbb".to_string()]; + let mut b = make_ticket("bbbbbb", &["aaaaaa"]); + b.dependencies = vec!["aaaaaa".to_string()]; + let tickets = vec![a, b]; + let graph = TicketGraph::build(&tickets); + + // Must not hang; both IDs should appear exactly once. + let sub = graph.subtree("aaaaaa"); + assert!(sub.contains(&"aaaaaa")); + assert!(sub.contains(&"bbbbbb")); + assert_eq!(sub.len(), 2, "each ID should appear exactly once"); + } + + /// `to_json_value` includes all tickets as nodes and all in-graph edges. + #[test] + fn to_json_value_nodes_and_edges() { + let a = make_ticket("aaaaaa", &[]); + let b = make_ticket("bbbbbb", &["aaaaaa"]); + let c = make_ticket("cccccc", &["aaaaaa"]); + let tickets = vec![a, b, c]; + let graph = TicketGraph::build(&tickets); + + let json = graph.to_json_value(); + let nodes = json["nodes"].as_array().unwrap(); + let edges = json["edges"].as_array().unwrap(); + + assert_eq!(nodes.len(), 3, "all three tickets should be nodes"); + assert_eq!(edges.len(), 2, "two edges: aaaaaa→bbbbbb and aaaaaa→cccccc"); + + let node_ids: Vec<&str> = nodes.iter().map(|n| n["id"].as_str().unwrap()).collect(); + assert!(node_ids.contains(&"aaaaaa")); + assert!(node_ids.contains(&"bbbbbb")); + assert!(node_ids.contains(&"cccccc")); + + // Every edge should have "from" (blocker) = "aaaaaa". + for edge in edges { + assert_eq!(edge["from"].as_str().unwrap(), "aaaaaa"); + } + } + + /// `to_json_value` on an empty graph returns empty `nodes` and `edges`. + #[test] + fn to_json_value_empty() { + let graph = TicketGraph::build(&[]); + let json = graph.to_json_value(); + assert!(json["nodes"].as_array().unwrap().is_empty()); + assert!(json["edges"].as_array().unwrap().is_empty()); + } + + /// `to_subtree_json_value` limits nodes and edges to the reachable subtree. + #[test] + fn to_subtree_json_value_scoped() { + let a = make_ticket("aaaaaa", &[]); + let b = make_ticket("bbbbbb", &["aaaaaa"]); // depends on a + let c = make_ticket("cccccc", &[]); // unrelated root + let tickets = vec![a, b, c]; + let graph = TicketGraph::build(&tickets); + + let json = graph.to_subtree_json_value("aaaaaa"); + let nodes = json["nodes"].as_array().unwrap(); + 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"), "dependent should be included"); + assert!( + !node_ids.contains(&"cccccc"), + "unrelated ticket should be excluded" + ); + } + + /// Dangling dependency references (IDs not in the graph) are silently ignored. + #[test] + fn dangling_deps_ignored() { + let mut a = make_ticket("aaaaaa", &[]); + // "ffffff" does not exist in the graph. + a.dependencies = vec!["ffffff".to_string()]; + let tickets = vec![a]; + let graph = TicketGraph::build(&tickets); + + // aaaaaa has no in-graph dependencies, so it should be a root. + let roots = graph.roots(); + assert_eq!(roots.len(), 1); + assert_eq!(roots[0].id, "aaaaaa"); + + // The JSON should show an empty dependencies list. + let json = graph.to_json_value(); + let deps = json["nodes"][0]["dependencies"].as_array().unwrap(); + assert!(deps.is_empty()); + } +} + // ── display module ──────────────────────────────────────────────────────────── /// Tests for [`crate::display`].