diff --git a/Cargo.lock b/Cargo.lock index 4541120..ab033bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2703,6 +2703,7 @@ dependencies = [ "criterion", "fastembed", "globset", + "ignore", "rayon", "rpg-core", "rpg-parser", diff --git a/crates/rpg-mcp/src/tools.rs b/crates/rpg-mcp/src/tools.rs index 1299fdb..e4408bd 100644 --- a/crates/rpg-mcp/src/tools.rs +++ b/crates/rpg-mcp/src/tools.rs @@ -2938,6 +2938,18 @@ impl RpgServer { .clone() .unwrap_or_else(|| "length".to_string()); + let excluded_paths = if !params.ignore_rpgignore.unwrap_or(false) { + let ignore_path = self.project_root.join(".rpgignore"); + let (gitignore, err) = ignore::gitignore::Gitignore::new(&ignore_path); + if err.is_none() || ignore_path.exists() { + Some(gitignore) + } else { + None + } + } else { + None + }; + let config = rpg_nav::cycles::CycleConfig { max_cycles, max_cycle_length: params.max_cycle_length.unwrap_or(20), @@ -2947,62 +2959,11 @@ impl RpgServer { include_files: params.include_files.unwrap_or(true), cross_file_only, cross_area_only, - ignore_rpgignore: params.ignore_rpgignore.unwrap_or(false), - project_root: Some(self.project_root.clone()), + excluded_paths, }; let report = rpg_nav::cycles::detect_cycles(graph, &config); - // TOON-optimized output: https://github.com/toon-format/toon - - let mut output = format!("{}# Circular Dependencies\n\n", notice); - - // Summary: inline object (compact TOON) - // Format: cycles: {total: N, entities: N, files: N, areas: N, cross_file: N, cross_area: N} - output.push_str(&format!( - "cycles: {{total: {}, entities: {}, files: {}, areas: {}, cross_file: {}, cross_area: {}}}\n\n", - report.cycle_count, - report.entities_in_cycles, - report.files_in_cycles, - report.areas_in_cycles, - report.cross_file_count, - report.cross_area_count - )); - - if report.cycle_count > 0 { - // Length distribution: compact key=value pairs - output.push_str(&format!( - "length_dist: len2={} len3={} len4={} len5+={}\n\n", - report.length_distribution.length_2, - report.length_distribution.length_3, - report.length_distribution.length_4, - report.length_distribution.length_5_plus - )); - - // Area breakdown: TOON tabular format - // Format: area_breakdown[N]{area,cycles,len2,len3,len4+,files}: - // AreaName,cycles,len2,len3,len4+,files - if !report.area_breakdown.is_empty() { - output.push_str(&format!( - "area_breakdown[{}]{{area,cycles,len2,len3,len4+,files}}:\n", - report.area_breakdown.len() - )); - for area in &report.area_breakdown { - output.push_str(&format!( - " {},{},{},{},{},{}\n", - area.area, - area.cycle_count, - area.length_2, - area.length_3, - area.length_4_plus, - area.file_count - )); - } - output.push('\n'); - } - } - - // If no filters applied, show available areas and next step let has_filters = params.max_cycles.is_some() || params.min_cycle_length.unwrap_or(2) > 2 || params.area.is_some() @@ -3010,104 +2971,32 @@ impl RpgServer { || params.cross_area_only.unwrap_or(false) || params.sort_by.is_some(); - // Option 1: TOON-ify available_areas with count - if !has_filters { - // Only show areas that have cycles (from area_breakdown, sorted by cycle count). - // Showing all hierarchy areas including cycle-free ones is misleading noise. - let areas: Vec = report - .area_breakdown - .iter() - .map(|ab| ab.area.clone()) - .collect(); - if !areas.is_empty() { - output.push_str(&format!("available_areas[{}]: ", areas.len())); - output.push_str(&format!("{}\n\n", areas.join(","))); - } - output.push_str( - "---\nnext_step: Use area/max_cycles/cross_file_only/cross_area_only to filter\n", - ); - return Ok(output); - } - - // Option 2: Show active filters when filters are applied - output.push_str(&format!( - "filters: {{area: {}, max_cycles: {}, cross_file: {}, cross_area: {}}}\n\n", - params.area.as_deref().unwrap_or("-"), - params - .max_cycles - .map(|n| n.to_string()) - .unwrap_or_else(|| "-".to_string()), - cross_file_only, - params.cross_area_only.unwrap_or(false) - )); - - // Cycles: TOON tabular with file:entity format - // Format: cycles[N]{chain,len,files}: - // file:entity->file:entity->file:entity,len=N,files=file1,file2 - let display_cycles: Vec<_> = report.cycles.iter().take(max_cycles).collect(); - - // Handle edge case: max_cycles = 0 - if max_cycles == 0 { - output.push_str("cycles[0]: (none requested)\n"); + let filter_summary = if has_filters { + Some(format!( + "area: {}, max_cycles: {}, cross_file: {}, cross_area: {}", + params.area.as_deref().unwrap_or("-"), + params + .max_cycles + .map(|n| n.to_string()) + .unwrap_or_else(|| "-".to_string()), + cross_file_only, + params.cross_area_only.unwrap_or(false) + )) } else { - output.push_str(&format!( - "cycles[{}]{{chain,len,files}}:\n", - display_cycles.len() - )); - - for cycle in &display_cycles { - // Convert chain to file:entity format for clarity - let chain: Vec = cycle - .cycle - .iter() - .map(|id| { - if let Some(entity) = graph.entities.get(id) { - let filename = entity - .file - .file_name() - .map(|n| n.to_string_lossy().to_string()) - .unwrap_or_else(|| "unknown".to_string()); - format!("{}:{}", filename, entity.name) - } else { - id.clone() - } - }) - .collect(); - let chain_str = chain.join("->"); - - // Extract just filenames from full paths - let files_str: Vec = cycle - .files - .iter() - .map(|f| { - std::path::Path::new(f) - .file_name() - .map(|n| n.to_string_lossy().to_string()) - .unwrap_or_else(|| f.clone()) - }) - .collect(); - let files_display = files_str.join(","); - - output.push_str(&format!( - " {},len={},{}\n", - chain_str, cycle.length, files_display - )); - } - } - - // Only show "...and N more" if max_cycles > 0 and there are actually more cycles - if max_cycles > 0 && report.cycle_count > max_cycles { - output.push_str(&format!( - "\n... and {} more. Use max_cycles to limit.\n", - report.cycle_count - max_cycles - )); - } + None + }; - output.push_str( - "\n---\nnext_step: Use area/max_cycles/cross_file_only/cross_area_only to filter\n", - ); + let opts = rpg_nav::toon::CycleReportOptions { + has_filters, + max_cycles, + filter_summary, + }; - Ok(output) + Ok(format!( + "{}{}", + notice, + rpg_nav::toon::format_cycle_report(&report, graph, &opts) + )) } } diff --git a/crates/rpg-nav/Cargo.toml b/crates/rpg-nav/Cargo.toml index fec44c0..3979b69 100644 --- a/crates/rpg-nav/Cargo.toml +++ b/crates/rpg-nav/Cargo.toml @@ -15,6 +15,7 @@ embeddings = ["dep:fastembed"] rpg-core.workspace = true anyhow.workspace = true globset.workspace = true +ignore.workspace = true strsim.workspace = true toon-format.workspace = true serde.workspace = true diff --git a/crates/rpg-nav/src/cycles.rs b/crates/rpg-nav/src/cycles.rs index 7e09bc7..1e1cd0b 100644 --- a/crates/rpg-nav/src/cycles.rs +++ b/crates/rpg-nav/src/cycles.rs @@ -9,26 +9,6 @@ use rpg_core::graph::{EdgeKind, RPGraph}; use serde::Serialize; use std::collections::{HashMap, HashSet}; -fn glob_match(pattern: &str, path: &str) -> bool { - if pattern.contains('*') { - let parts: Vec<&str> = pattern.split('*').collect(); - let mut pos = 0; - for part in parts { - if part.is_empty() { - continue; - } - if let Some(i) = path[pos..].find(part) { - pos += i + part.len(); - } else { - return false; - } - } - true - } else { - path.contains(pattern) - } -} - /// Edge kinds that represent dependency relationships (not structural containment). const DEPENDENCY_EDGE_KINDS: &[EdgeKind] = &[ EdgeKind::Imports, @@ -76,10 +56,9 @@ pub struct CycleConfig { pub cross_file_only: bool, /// Only show cycles that span multiple hierarchy areas. pub cross_area_only: bool, - /// Ignore .rpgignore rules. - pub ignore_rpgignore: bool, - /// Project root path for .rpgignore lookup. - pub project_root: Option, + /// Paths to exclude (loaded from .rpgignore or other source by the caller). + /// Cycles where ALL entities match an excluded path are removed. + pub excluded_paths: Option, } impl Default for CycleConfig { @@ -93,8 +72,7 @@ impl Default for CycleConfig { include_files: true, cross_file_only: false, cross_area_only: false, - ignore_rpgignore: false, - project_root: None, + excluded_paths: None, } } } @@ -205,35 +183,19 @@ pub fn detect_cycles(graph: &RPGraph, config: &CycleConfig) -> CycleReport { // Deduplicate cycles (same cycle can be found starting from different nodes) let mut cycles = deduplicate_cycles(cycles); - // Filter based on .rpgignore if not ignored - if !config.ignore_rpgignore - && let Some(ref project_root) = config.project_root - { - let rpgignore_path = project_root.join(".rpgignore"); - if rpgignore_path.exists() - && let Ok(patterns) = std::fs::read_to_string(&rpgignore_path) - { - let patterns: Vec = patterns - .lines() - .filter(|l| !l.trim().is_empty() && !l.trim().starts_with('#')) - .map(|l| l.trim().to_string()) - .collect(); - - if !patterns.is_empty() { - cycles.retain(|cycle| { - cycle.cycle.iter().any(|entity_id| { - if let Some(entity) = graph.entities.get(entity_id) { - let file_path = entity.file.display().to_string(); - !patterns - .iter() - .any(|p| glob_match(p, &file_path) || file_path.contains(p)) - } else { - true - } - }) - }); - } - } + // Filter out cycles where ALL entities are in excluded paths + if let Some(ref gitignore) = config.excluded_paths { + cycles.retain(|cycle| { + cycle.cycle.iter().any(|entity_id| { + if let Some(entity) = graph.entities.get(entity_id) { + !gitignore + .matched_path_or_any_parents(&entity.file, false) + .is_ignore() + } else { + true // keep cycles with unknown entities + } + }) + }); } // Filter by hierarchy area if specified (supports comma-separated multiple areas) diff --git a/crates/rpg-nav/src/toon.rs b/crates/rpg-nav/src/toon.rs index 65f6f06..a35783a 100644 --- a/crates/rpg-nav/src/toon.rs +++ b/crates/rpg-nav/src/toon.rs @@ -854,6 +854,160 @@ pub fn format_health_report(report: &HealthReport) -> String { output } +// --------------------------------------------------------------------------- +// Cycle report output +// --------------------------------------------------------------------------- + +use crate::cycles::CycleReport; + +/// Options controlling how the cycle report is rendered. +pub struct CycleReportOptions { + /// Whether any filters were applied (area, max_cycles, cross_file, etc.). + pub has_filters: bool, + /// Maximum cycles to display when filters are active. + pub max_cycles: usize, + /// Active filter description (e.g., "area: Navigation, max_cycles: 10, ..."). + pub filter_summary: Option, +} + +/// Format a cycle report as TOON for LLM consumption. +/// +/// When `has_filters` is false, shows a summary with available areas and a +/// next-step hint. When filters are active, shows the matching cycles in +/// TOON tabular format with entity chain details. +pub fn format_cycle_report( + report: &CycleReport, + graph: &RPGraph, + opts: &CycleReportOptions, +) -> String { + let mut output = String::from("# Circular Dependencies\n\n"); + + // Summary: inline object (compact TOON) + output.push_str(&format!( + "cycles: {{total: {}, entities: {}, files: {}, areas: {}, cross_file: {}, cross_area: {}}}\n\n", + report.cycle_count, + report.entities_in_cycles, + report.files_in_cycles, + report.areas_in_cycles, + report.cross_file_count, + report.cross_area_count + )); + + if report.cycle_count > 0 { + // Length distribution + output.push_str(&format!( + "length_dist: len2={} len3={} len4={} len5+={}\n\n", + report.length_distribution.length_2, + report.length_distribution.length_3, + report.length_distribution.length_4, + report.length_distribution.length_5_plus + )); + + // Area breakdown + if !report.area_breakdown.is_empty() { + output.push_str(&format!( + "area_breakdown[{}]{{area,cycles,len2,len3,len4+,files}}:\n", + report.area_breakdown.len() + )); + for area in &report.area_breakdown { + output.push_str(&format!( + " {},{},{},{},{},{}\n", + area.area, + area.cycle_count, + area.length_2, + area.length_3, + area.length_4_plus, + area.file_count + )); + } + output.push('\n'); + } + } + + // Unfiltered: show available areas and next-step hint + if !opts.has_filters { + let areas: Vec = report + .area_breakdown + .iter() + .map(|ab| ab.area.clone()) + .collect(); + if !areas.is_empty() { + output.push_str(&format!("available_areas[{}]: ", areas.len())); + output.push_str(&format!("{}\n\n", areas.join(","))); + } + output.push_str( + "---\nnext_step: Use area/max_cycles/cross_file_only/cross_area_only to filter\n", + ); + return output; + } + + // Filtered: show active filters and cycle details + if let Some(ref summary) = opts.filter_summary { + output.push_str(&format!("filters: {{{}}}\n\n", summary)); + } + + let display_cycles: Vec<_> = report.cycles.iter().take(opts.max_cycles).collect(); + + if opts.max_cycles == 0 { + output.push_str("cycles[0]: (none requested)\n"); + } else { + output.push_str(&format!( + "cycles[{}]{{chain,len,files}}:\n", + display_cycles.len() + )); + + for cycle in &display_cycles { + let chain: Vec = cycle + .cycle + .iter() + .map(|id| { + if let Some(entity) = graph.entities.get(id) { + let filename = entity + .file + .file_name() + .map(|n| n.to_string_lossy().to_string()) + .unwrap_or_else(|| "unknown".to_string()); + format!("{}:{}", filename, entity.name) + } else { + id.clone() + } + }) + .collect(); + let chain_str = chain.join("->"); + + let files_str: Vec = cycle + .files + .iter() + .map(|f| { + std::path::Path::new(f) + .file_name() + .map(|n| n.to_string_lossy().to_string()) + .unwrap_or_else(|| f.clone()) + }) + .collect(); + let files_display = files_str.join(","); + + output.push_str(&format!( + " {},len={},{}\n", + chain_str, cycle.length, files_display + )); + } + } + + if opts.max_cycles > 0 && report.cycle_count > opts.max_cycles { + output.push_str(&format!( + "\n... and {} more. Use max_cycles to limit.\n", + report.cycle_count - opts.max_cycles + )); + } + + output.push_str( + "\n---\nnext_step: Use area/max_cycles/cross_file_only/cross_area_only to filter\n", + ); + + output +} + // --------------------------------------------------------------------------- // Tests // ---------------------------------------------------------------------------