From 1447dc74bb2bb4f139ed498d1dd857fb4764e945 Mon Sep 17 00:00:00 2001 From: Gabriel Kaszewski Date: Wed, 17 Jun 2026 10:48:59 +0200 Subject: [PATCH] =?UTF-8?q?refactor:=20extract=20module=5Fedges()=20to=20C?= =?UTF-8?q?odeGraph=20domain=20=E2=80=94=20removes=20duplication=20from=20?= =?UTF-8?q?Mermaid=20and=20D2=20renderers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/adapters/d2/src/d2_renderer.rs | 43 +--------- .../adapters/mermaid/src/mermaid_renderer.rs | 83 ++----------------- crates/domain/src/aggregates/code_graph.rs | 68 +++++++++++++++ crates/domain/tests/code_graph_tests.rs | 56 +++++++++++++ 4 files changed, 133 insertions(+), 117 deletions(-) diff --git a/crates/adapters/d2/src/d2_renderer.rs b/crates/adapters/d2/src/d2_renderer.rs index 032b300..4450851 100644 --- a/crates/adapters/d2/src/d2_renderer.rs +++ b/crates/adapters/d2/src/d2_renderer.rs @@ -92,49 +92,14 @@ fn render_type(graph: &CodeGraph) -> String { } fn render_module(graph: &CodeGraph) -> String { - use archlens_domain::RelationshipKind; - use std::collections::{HashMap, HashSet}; - let mut lines = Vec::new(); - let mut modules: HashSet = HashSet::new(); - let mut name_to_module: HashMap<&str, &str> = HashMap::new(); - for el in graph.elements() { - if let Some(m) = el.module() { - modules.insert(m.as_str().to_string()); - name_to_module.insert(el.qualified_name(), m.as_str()); - name_to_module.insert(el.name(), m.as_str()); - } + for module in graph.modules() { + let id = sanitize(module.as_str()); + lines.push(format!("{id}: {}", module.as_str())); } - for module in &modules { - let id = sanitize(module); - lines.push(format!("{id}: {module}")); - } - - let mut edges: HashSet<(String, String)> = HashSet::new(); - for rel in graph.relationships() { - if rel.kind() == RelationshipKind::Import { - continue; - } - // Direct module-to-module edge (from merged project deps) - if modules.contains(rel.source()) - && modules.contains(rel.target()) - && rel.source() != rel.target() - { - edges.insert((rel.source().to_string(), rel.target().to_string())); - continue; - } - let src_mod = name_to_module.get(rel.source()); - let tgt_mod = name_to_module.get(rel.target()); - if let (Some(s), Some(t)) = (src_mod, tgt_mod) - && s != t - { - edges.insert((s.to_string(), t.to_string())); - } - } - - for (src, tgt) in &edges { + for (src, tgt) in graph.module_edges().keys() { lines.push(format!("{} -> {}", sanitize(src), sanitize(tgt))); } diff --git a/crates/adapters/mermaid/src/mermaid_renderer.rs b/crates/adapters/mermaid/src/mermaid_renderer.rs index 33e0031..ffc9e63 100644 --- a/crates/adapters/mermaid/src/mermaid_renderer.rs +++ b/crates/adapters/mermaid/src/mermaid_renderer.rs @@ -1,4 +1,4 @@ -use std::collections::{HashMap, HashSet}; +use std::collections::HashSet; use archlens_domain::{ CodeElement, CodeGraph, DiagramLevel, DomainError, ModuleName, RelationshipKind, RenderOutput, @@ -146,85 +146,12 @@ impl MermaidRenderer { fn render_module_flowchart(&self, graph: &CodeGraph) -> String { let mut lines = vec!["graph TD".to_string()]; - let mut name_to_modules: HashMap<&str, HashSet<&str>> = HashMap::new(); - let mut file_to_module: HashMap = HashMap::new(); - let mut modules: HashSet = HashSet::new(); - - for element in graph.elements() { - if let Some(module) = element.module() { - // Index both bare name and qualified name for lookup - name_to_modules - .entry(element.name()) - .or_default() - .insert(module.as_str()); - name_to_modules - .entry(element.qualified_name()) - .or_default() - .insert(module.as_str()); - modules.insert(module.as_str().to_string()); - - let file_stem = std::path::Path::new(element.file_path().as_str()) - .file_stem() - .and_then(|s| s.to_str()) - .unwrap_or(""); - if !file_stem.is_empty() { - file_to_module.insert(file_stem.to_string(), module.as_str().to_string()); - } - } + for module in graph.modules() { + let m = module.as_str(); + lines.push(format!(" {m}[{m}]")); } - for module in &modules { - lines.push(format!(" {module}[{module}]")); - } - - let mut module_edges: HashMap<(String, String), usize> = HashMap::new(); - for rel in graph.relationships() { - match rel.kind() { - RelationshipKind::Import => { - let source_mod = file_to_module.get(rel.source()); - let target_top = rel.target().split('.').next().unwrap_or(""); - let target_mod = ModuleName::capitalize(target_top); - - if let Some(src) = source_mod - && modules.contains(&target_mod) - && *src != target_mod - { - *module_edges.entry((src.clone(), target_mod)).or_insert(0) += 1; - } - } - _ => { - if modules.contains(rel.source()) - && modules.contains(rel.target()) - && rel.source() != rel.target() - { - *module_edges - .entry((rel.source().to_string(), rel.target().to_string())) - .or_insert(0) += 1; - continue; - } - - let src_mods = name_to_modules.get(rel.source()); - let tgt_mods = name_to_modules.get(rel.target()); - - if let (Some(src_set), Some(tgt_set)) = (src_mods, tgt_mods) { - for src_mod in src_set { - if tgt_set.contains(src_mod) { - continue; - } - for tgt_mod in tgt_set { - if src_mod != tgt_mod { - *module_edges - .entry((src_mod.to_string(), tgt_mod.to_string())) - .or_insert(0) += 1; - } - } - } - } - } - } - } - - for ((source, target), count) in &module_edges { + for ((source, target), count) in &graph.module_edges() { let line = if self.show_weights { let label = if *count == 1 { "1 dep".to_string() diff --git a/crates/domain/src/aggregates/code_graph.rs b/crates/domain/src/aggregates/code_graph.rs index d34d6fb..bc1eafe 100644 --- a/crates/domain/src/aggregates/code_graph.rs +++ b/crates/domain/src/aggregates/code_graph.rs @@ -278,4 +278,72 @@ impl CodeGraph { relationships: filtered_relationships, } } + + /// Compute module-to-module edges with relationship counts. + /// + /// Handles three cases: + /// - Direct module-name edges injected by `merge_project_deps_as_module_edges` + /// - Type-level composition/inheritance relationships resolved to their modules + /// - Import relationships resolved via file-stem → module mapping + pub fn module_edges(&self) -> HashMap<(String, String), usize> { + let mut name_to_module: HashMap<&str, &str> = HashMap::new(); + let mut file_to_module: HashMap = HashMap::new(); + let mut modules: HashSet = HashSet::new(); + + for el in &self.elements { + if let Some(m) = el.module() { + modules.insert(m.as_str().to_string()); + name_to_module.insert(el.qualified_name(), m.as_str()); + name_to_module.insert(el.name(), m.as_str()); + + let file_stem = std::path::Path::new(el.file_path().as_str()) + .file_stem() + .and_then(|s| s.to_str()) + .unwrap_or_default() + .to_string(); + if !file_stem.is_empty() { + file_to_module.insert(file_stem, m.as_str().to_string()); + } + } + } + + let mut edges: HashMap<(String, String), usize> = HashMap::new(); + + for rel in &self.relationships { + match rel.kind() { + RelationshipKind::Import => { + let source_mod = file_to_module.get(rel.source()); + let target_top = rel.target().split('.').next().unwrap_or(""); + let target_mod = ModuleName::capitalize(target_top); + if let Some(src) = source_mod + && modules.contains(&target_mod) + && *src != target_mod + { + *edges.entry((src.clone(), target_mod)).or_insert(0) += 1; + } + } + _ => { + // Direct module-to-module edge (injected by merge_project_deps) + if modules.contains(rel.source()) + && modules.contains(rel.target()) + && rel.source() != rel.target() + { + *edges + .entry((rel.source().to_string(), rel.target().to_string())) + .or_insert(0) += 1; + continue; + } + let src_mod = name_to_module.get(rel.source()).copied(); + let tgt_mod = name_to_module.get(rel.target()).copied(); + if let (Some(s), Some(t)) = (src_mod, tgt_mod) + && s != t + { + *edges.entry((s.to_string(), t.to_string())).or_insert(0) += 1; + } + } + } + } + + edges + } } diff --git a/crates/domain/tests/code_graph_tests.rs b/crates/domain/tests/code_graph_tests.rs index 9c5a891..7346b55 100644 --- a/crates/domain/tests/code_graph_tests.rs +++ b/crates/domain/tests/code_graph_tests.rs @@ -299,3 +299,59 @@ fn graph_lists_unique_modules() { assert!(modules.iter().any(|m| m.as_str() == "Orders")); assert!(modules.iter().any(|m| m.as_str() == "Billing")); } + +#[test] +fn module_edges_aggregates_type_level_relationships_into_module_pairs() { + let mut graph = CodeGraph::new(); + graph.add_element(make_element("ServiceA", Some("App"))); + graph.add_element(make_element("ServiceB", Some("App"))); + graph.add_element(make_element("Order", Some("Domain"))); + graph.add_element(make_element("Product", Some("Domain"))); + + graph.add_relationship( + Relationship::new("ServiceA", "Order", RelationshipKind::Composition).unwrap(), + ); + graph.add_relationship( + Relationship::new("ServiceB", "Product", RelationshipKind::Composition).unwrap(), + ); + + let graph = graph.qualify(); + let edges = graph.module_edges(); + + assert_eq!(edges.len(), 1, "should have one cross-module edge pair"); + assert_eq!(edges[&("App".to_string(), "Domain".to_string())], 2); +} + +#[test] +fn module_edges_handles_direct_module_to_module_relationships() { + let mut graph = CodeGraph::new(); + graph.add_element(make_element("Order", Some("Domain"))); + graph.add_element(make_element("Service", Some("App"))); + // Direct module-name edge (injected by merge_project_deps) + graph.add_relationship( + Relationship::new("App", "Domain", RelationshipKind::Composition).unwrap(), + ); + + let graph = graph.qualify(); + let edges = graph.module_edges(); + + assert!( + edges.contains_key(&("App".to_string(), "Domain".to_string())), + "direct module edge should appear: {edges:?}" + ); +} + +#[test] +fn module_edges_excludes_intra_module_relationships() { + let mut graph = CodeGraph::new(); + graph.add_element(make_element("Service", Some("App"))); + graph.add_element(make_element("Helper", Some("App"))); + graph.add_relationship( + Relationship::new("Service", "Helper", RelationshipKind::Composition).unwrap(), + ); + + let graph = graph.qualify(); + let edges = graph.module_edges(); + + assert!(edges.is_empty(), "intra-module relationships should not produce edges"); +}