refactor: decouple tree construction from Newick serialization

Refactor phylogenetic algorithms (`neighbor_joining`, `upgma`) to return an explicit `Tree` struct instead of a serialized Newick string. This change makes serialization an explicit step for downstream consumers via the new public `Tree::to_newick()` method, decoupling tree construction from output formatting. The `siblings` module has also been moved to `siblings_old`.

src/obikmer2/src/cmd/phylo/mod.rs

@@ -5,7 +5,7 @@ use std::sync::Arc;
 
 use obikidxcache::index_cache::IndexCache;
 use obikindex::KmerIndex;
-use obikphylo::{Metrics, neighbor_joining_newick, upgma_newick};
+use obikphylo::{Metrics, neighbor_joining, upgma};
 use obisys::{Reporter, Stage};
 use tracing::info;
 
@@ -103,10 +103,11 @@ pub fn run(args: PhyloArgs) {
 
     // ── NJ tree ────────────────────────────────────────────────────────────────
     if args.nj {
-        let newick = neighbor_joining_newick(&result.matrix, &labels).unwrap_or_else(|e| {
+        let tree = neighbor_joining(&result.matrix, &labels).unwrap_or_else(|e| {
             eprintln!("error computing NJ tree: {e}");
             std::process::exit(1);
         });
+        let newick = tree.to_newick();
         let path = args.output.as_ref()
             .map(|p| format!("{}_nj.nwk", p.display()))
             .unwrap_or_else(|| "nj.nwk".into());
@@ -119,7 +120,7 @@ pub fn run(args: PhyloArgs) {
 
     // ── UPGMA tree ───────────────────────────────────────────────────────────────
     if args.upgma {
-        let newick = upgma_newick(&result.matrix, &labels);
+        let newick = upgma(&result.matrix, &labels).to_newick();
         let path = args.output.as_ref()
             .map(|p| format!("{}_upgma.nwk", p.display()))
             .unwrap_or_else(|| "upgma.nwk".into());

src/obikphylo/src/lib.rs

@@ -19,4 +19,4 @@ mod tree;
 // pub mod siblings; // temporarily disconnected — see module doc above.
 
 pub use distance::{DistanceMetric, DistanceOutput, Metrics};
-pub use tree::{neighbor_joining_newick, upgma_newick};
+pub use tree::{Tree, neighbor_joining, upgma};

src/obikphylo/src/tree/mod.rs

@@ -3,13 +3,20 @@
 //! shared Newick serialisation, instead of each algorithm hand-rolling its
 //! own Newick writer. No reader — nothing in this crate ever needs to parse
 //! a Newick string back in, so there is no `from_newick`.
+//!
+//! Two independent axes, deliberately kept from ever multiplying against
+//! each other: [`nj::neighbor_joining`]/[`upgma::upgma`] each only know how
+//! to build a [`Tree`] from their own algorithm's result — neither knows
+//! Newick exists. [`Tree::to_newick`] only knows how to walk an already-built
+//! `Tree` — it doesn't know NJ or UPGMA exist. A new algorithm or a new
+//! output format each plug in on their own side, at zero cost to the other.
 
 mod newick;
 mod nj;
 mod upgma;
 
-pub use nj::neighbor_joining_newick;
+pub use nj::neighbor_joining;
-pub use upgma::upgma_newick;
+pub use upgma::upgma;
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 struct NodeId(usize);
@@ -28,7 +35,7 @@ struct Node {
 /// algorithm (UPGMA's successive merges, NJ's already-built graph) needs:
 /// a node's parent is only known once *it* already exists.
 #[derive(Debug, Default)]
-struct Tree {
+pub struct Tree {
     nodes: Vec<Node>,
     root: Option<NodeId>,
 }
@@ -75,7 +82,7 @@ impl Tree {
     /// Serialise to Newick (terminated with `;`). Panics if
     /// [`set_root`](Tree::set_root) was never called — a tree with no
     /// designated root is a builder bug, not a recoverable input.
-    fn to_newick(&self) -> String {
+    pub fn to_newick(&self) -> String {
         let root = self.root.expect("Tree::to_newick: root not set");
         let mut out = newick::write_node(self, root);
         out.push(';');

src/obikphylo/src/tree/nj.rs

@@ -1,7 +1,7 @@
 //! Neighbor-Joining, via `speedytree` — converts its own tree type
-//! (`petgraph::graph::UnGraph<String, f64>`) into this crate's [`super::Tree`]
+//! (`petgraph::graph::UnGraph<String, f64>`) into this crate's [`super::Tree`].
-//! so its Newick output goes through the same writer as every other
+//! Knows nothing about Newick or any other output format — that's
-//! algorithm here.
+//! [`Tree::to_newick`]'s job, not this algorithm's.
 
 use ndarray::Array2;
 use obikindex::{OKIError, OKIResult};
@@ -11,8 +11,8 @@ use speedytree::{DistanceMatrix, Hybrid, NeighborJoiningSolver};
 use super::{NodeId, Tree};
 
 /// Compute a Neighbor-Joining tree from a symmetric `n×n` distance `matrix`
-/// (genomes in `labels` order), returning it as a Newick string.
+/// (genomes in `labels` order).
-pub fn neighbor_joining_newick(matrix: &Array2<f64>, labels: &[String]) -> OKIResult<String> {
+pub fn neighbor_joining(matrix: &Array2<f64>, labels: &[String]) -> OKIResult<Tree> {
     let n = labels.len();
     let rows: Vec<Vec<f64>> = (0..n).map(|i| (0..n).map(|j| matrix[[i, j]]).collect()).collect();
     let dm = DistanceMatrix::build(rows, labels.to_vec())
@@ -33,7 +33,7 @@ pub fn neighbor_joining_newick(matrix: &Array2<f64>, labels: &[String]) -> OKIRe
     let mut tree = Tree::new();
     let root_id = convert(&graph, root, root, &mut tree);
     tree.set_root(root_id);
-    Ok(tree.to_newick())
+    Ok(tree)
 }
 
 /// Recursively convert `node` (and everything below it, i.e. every

src/obikphylo/src/tree/upgma.rs

@@ -1,7 +1,7 @@
 //! UPGMA (average-linkage hierarchical clustering), via `kodama` — converts
 //! its dendrogram (a flat list of merge steps) into this crate's
-//! [`super::Tree`], bottom-up, so its Newick output goes through the same
+//! [`super::Tree`], bottom-up. Knows nothing about Newick or any other
-//! writer as every other algorithm here.
+//! output format — that's [`Tree::to_newick`]'s job, not this algorithm's.
 
 use kodama::{Method, linkage};
 use ndarray::Array2;
@@ -9,8 +9,8 @@ use ndarray::Array2;
 use super::Tree;
 
 /// Compute a UPGMA tree from a symmetric `n×n` distance `matrix` (genomes in
-/// `labels` order), returning it as a Newick string.
+/// `labels` order).
-pub fn upgma_newick(matrix: &Array2<f64>, labels: &[String]) -> String {
+pub fn upgma(matrix: &Array2<f64>, labels: &[String]) -> Tree {
     let n = labels.len();
     let mut condensed: Vec<f64> = Vec::with_capacity(n * (n - 1) / 2);
     for i in 0..n {
@@ -47,5 +47,5 @@ pub fn upgma_newick(matrix: &Array2<f64>, labels: &[String]) -> String {
     }
 
     tree.set_root(root.expect("at least one merge step for n >= 2 genomes"));
-    tree.to_newick()
+    tree
 }