From ba5942fa502f6f30afd0197f4330c7f8c558af4a Mon Sep 17 00:00:00 2001 From: Andrey Vasilevsky Date: Tue, 30 Jun 2026 16:03:09 -0700 Subject: [PATCH 1/4] fix(pr-review): use structural is_test marker, not just name heuristics codegraph_pr_context classified a caller as a test only when its name matched test_*/*_test* or its path contained /tests/. Idiomatic Rust unit tests live in `#[cfg(test)] mod tests` with descriptive names (e.g. `weighted_mean_l2_math`, `loads_potion_f32_no_weights`), so they were invisible and every function they cover was reported as "0 coverage". The indexer already records `is_test` on each function node (from the language's #[test]/@Test marker; helpers.rs:112). Read it via a new node_props::is_test accessor and prefer it over the name/path heuristics, which remain as a fallback for languages that don't populate the marker. Co-Authored-By: Claude Opus 4.8 --- crates/codegraph-server/src/domain/node_props.rs | 8 ++++++++ crates/codegraph-server/src/mcp/server.rs | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/crates/codegraph-server/src/domain/node_props.rs b/crates/codegraph-server/src/domain/node_props.rs index 06a406b..ef1739e 100644 --- a/crates/codegraph-server/src/domain/node_props.rs +++ b/crates/codegraph-server/src/domain/node_props.rs @@ -109,3 +109,11 @@ pub(crate) fn is_public(node: &Node) -> bool { .or_else(|| node.properties.get_bool("exported")) .unwrap_or_else(|| matches!(visibility(node), "public" | "pub")) } + +/// Whether the node is a test function, as recorded at index time from the +/// language's test marker (`#[test]`/`#[cfg(test)]`, `@Test`, etc.). This is +/// the structural signal; callers should prefer it over name heuristics, which +/// miss idiomatic test names (Rust `#[cfg(test)] mod tests { fn descriptive() }`). +pub(crate) fn is_test(node: &Node) -> bool { + node.properties.get_bool("is_test").unwrap_or(false) +} diff --git a/crates/codegraph-server/src/mcp/server.rs b/crates/codegraph-server/src/mcp/server.rs index ce46b85..3a776a6 100644 --- a/crates/codegraph-server/src/mcp/server.rs +++ b/crates/codegraph-server/src/mcp/server.rs @@ -4104,7 +4104,13 @@ impl McpServer { if let Ok(caller) = graph.get_node(caller_id) { let cname = crate::domain::node_props::name(caller); let cfile = caller.properties.get_string("path").unwrap_or(""); - let is_test = cname.to_lowercase().starts_with("test_") + // Prefer the structural is_test marker recorded at index time + // (#[test]/#[cfg(test)], @Test, …); fall back to name/path + // heuristics only for languages that don't populate it. The + // heuristics alone miss idiomatic Rust tests with descriptive + // names inside `#[cfg(test)] mod tests`. + let is_test = crate::domain::node_props::is_test(caller) + || cname.to_lowercase().starts_with("test_") || cname.to_lowercase().contains("_test") || cfile.contains("/tests/") || cfile.contains("/test_"); @@ -4132,7 +4138,11 @@ impl McpServer { // #87: Test gap — function has no test callers. // Skip functions that ARE tests (they don't need // their own coverage) and trivial getters/setters. - let fn_is_test = func_name.to_lowercase().starts_with("test_") + let fn_is_test = graph + .get_node(*node_id) + .ok() + .is_some_and(|n| crate::domain::node_props::is_test(n)) + || func_name.to_lowercase().starts_with("test_") || func_name.to_lowercase().contains("_test") || changed_rel[idx].contains("/tests/") || changed_rel[idx].contains("_test."); From 6e707f1c01b45ccaad36423cc7fb4a7708dc6970 Mon Sep 17 00:00:00 2001 From: Andrey Vasilevsky Date: Tue, 30 Jun 2026 16:17:59 -0700 Subject: [PATCH 2/4] fix(parser): persist is_test on Rust function nodes (partial) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ir_to_graph wrote is_async/is_static/is_abstract but not is_test, so the graph never carried the marker the visitor already computes. Add it, plus a regression test that a #[test] fn with a descriptive name inside `#[cfg(test)] mod tests` sets is_test. NOTE: insufficient on its own — end-to-end pr_context still misreports coverage; is_test is not reaching the queried nodes in the real indexing pipeline (cause not yet isolated; not a cache artifact). Co-Authored-By: Claude Opus 4.8 --- crates/codegraph-rust/src/mapper.rs | 3 ++- crates/codegraph-rust/src/visitor.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/crates/codegraph-rust/src/mapper.rs b/crates/codegraph-rust/src/mapper.rs index c36ccc6..935909f 100644 --- a/crates/codegraph-rust/src/mapper.rs +++ b/crates/codegraph-rust/src/mapper.rs @@ -71,7 +71,8 @@ pub fn ir_to_graph( .with("line_end", func.line_end as i64) .with("is_async", func.is_async) .with("is_static", func.is_static) - .with("is_abstract", func.is_abstract); + .with("is_abstract", func.is_abstract) + .with("is_test", func.is_test); // Add complexity metrics if available if let Some(ref complexity) = func.complexity { diff --git a/crates/codegraph-rust/src/visitor.rs b/crates/codegraph-rust/src/visitor.rs index 0665dd9..8adc743 100644 --- a/crates/codegraph-rust/src/visitor.rs +++ b/crates/codegraph-rust/src/visitor.rs @@ -1320,6 +1320,33 @@ fn test_something() {} assert!(visitor.functions[0].is_test); } + #[test] + fn test_visitor_test_fn_inside_cfg_test_mod() { + // The idiomatic Rust unit-test shape: a #[test] fn with a descriptive + // (non-`test_`) name inside `#[cfg(test)] mod tests`. This is what the + // PR-review coverage analysis was missing. + let source = r#" +fn weighted_mean_l2() {} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn weighted_mean_l2_math() { + weighted_mean_l2(); + } +} +"#; + let visitor = parse_and_visit(source); + let t = visitor + .functions + .iter() + .find(|f| f.name == "weighted_mean_l2_math") + .expect("nested test fn should be visited"); + assert!(t.is_test, "#[test] fn inside `mod tests` must set is_test"); + } + #[test] fn test_visitor_visibility_modifiers() { let source = r#" From 791d5b1ae1deb6f51b228014eec062e2c6235215 Mon Sep 17 00:00:00 2001 From: Andrey Vasilevsky Date: Tue, 30 Jun 2026 17:11:53 -0700 Subject: [PATCH 3/4] fix(pr-review): proportionate risk + count example/bench callers as coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two calibration fixes to codegraph_pr_context: - Risk was `total_direct > 20 || untested > 5 → high`, so any sizeable PR tripped "high" purely on caller count — even when every change was body-only and broke nothing. Base risk instead on `breaking_callers` (callers of signature-changed functions) and the untested *ratio* (untested / functions_touched). Surface `breaking_callers` in JSON, the message, and the blast-radius line. - Callers under `examples/` and `benches/` exercise a function at runtime but were counted as breakable production callers, inflating blast radius and leaving example-driven code flagged as untested. Count them as coverage instead. Co-Authored-By: Claude Opus 4.8 --- crates/codegraph-server/src/mcp/server.rs | 56 +++++++++++++++++------ 1 file changed, 43 insertions(+), 13 deletions(-) diff --git a/crates/codegraph-server/src/mcp/server.rs b/crates/codegraph-server/src/mcp/server.rs index 3a776a6..e244e72 100644 --- a/crates/codegraph-server/src/mcp/server.rs +++ b/crates/codegraph-server/src/mcp/server.rs @@ -4046,6 +4046,10 @@ impl McpServer { let mut all_tests = Vec::new(); let mut untested_functions = Vec::new(); let mut total_direct = 0usize; + // Callers of signature-changed functions — the ones that can + // actually break. Body-only changes don't break callers, so + // they shouldn't drive the risk level or blast radius. + let mut breaking_callers = 0usize; let mut all_affected_files = std::collections::HashSet::new(); let mut func_details = Vec::new(); @@ -4114,8 +4118,15 @@ impl McpServer { || cname.to_lowercase().contains("_test") || cfile.contains("/tests/") || cfile.contains("/test_"); - - if is_test { + // Callers under examples/ (and doctests) exercise the + // function at runtime — count them as coverage, not as + // breakable production callers. This is what covers code + // driven only by eval/example harnesses. + let is_exercising = !is_test + && (cfile.contains("/examples/") + || cfile.contains("/benches/")); + + if is_test || is_exercising { has_test_caller = true; all_tests.push(serde_json::json!({ "test": cname, "file": cfile, "covers": func_name, @@ -4134,6 +4145,9 @@ impl McpServer { } } total_direct += caller_count as usize; + if change_type == "signature_changed" { + breaking_callers += caller_count as usize; + } // #87: Test gap — function has no test callers. // Skip functions that ARE tests (they don't need @@ -4292,10 +4306,29 @@ impl McpServer { drop(graph); - // Risk level - let risk_level = if total_direct > 20 || untested_functions.len() > 5 { + // Functions actually touched by this PR (denominator for ratios). + let total_functions: u64 = file_impacts + .iter() + .map(|f| f["functions_changed"].as_u64().unwrap_or(0)) + .sum(); + + // Risk is driven by what can actually break — callers of + // signature-changed functions — and by the share of touched + // functions left untested, NOT by the raw caller count (which + // body-only changes inflate, e.g. a widely-called helper whose + // body changed but signature didn't). + let untested_ratio = if total_functions > 0 { + untested_functions.len() as f64 / total_functions as f64 + } else { + 0.0 + }; + let risk_level = if breaking_callers > 25 + || (untested_functions.len() > 5 && untested_ratio > 0.5) + { "high" - } else if total_direct > 5 || untested_functions.len() > 2 { + } else if breaking_callers > 8 + || (untested_functions.len() > 2 && untested_ratio > 0.25) + { "medium" } else { "low" @@ -4315,11 +4348,6 @@ impl McpServer { commit_prefix, primary_module ); - let total_functions: u64 = file_impacts - .iter() - .map(|f| f["functions_changed"].as_u64().unwrap_or(0)) - .sum(); - let mut result = serde_json::json!({ "base_branch": base, "changed_files": changed_rel.len(), @@ -4327,6 +4355,7 @@ impl McpServer { "lines_removed": lines_removed, "functions_touched": total_functions, "direct_callers": total_direct, + "breaking_callers": breaking_callers, "related_tests": unique_tests.len(), "untested_functions": untested_functions.len(), "affected_modules": affected_modules, @@ -4334,9 +4363,9 @@ impl McpServer { "commit_hint": commit_hint, "files": file_impacts, "message": format!( - "PR changes {} files (+{}/-{}, {} functions). {} direct callers, {} tests, {} untested. Risk: {}.", + "PR changes {} files (+{}/-{}, {} functions). {} direct callers ({} breaking), {} tests, {} untested. Risk: {}.", changed_rel.len(), lines_added, lines_removed, total_functions, - total_direct, unique_tests.len(), untested_functions.len(), risk_level, + total_direct, breaking_callers, unique_tests.len(), untested_functions.len(), risk_level, ), }); @@ -4395,9 +4424,10 @@ impl McpServer { if total_direct > 0 { md.push_str(&format!( - "### Blast radius\n{} direct caller{} affected", + "### Blast radius\n{} direct caller{} affected ({} breaking)", total_direct, if total_direct == 1 { "" } else { "s" }, + breaking_callers, )); if !affected_modules.is_empty() { let mods: Vec = affected_modules From bec92b144d0316cabccef207a09aef857375b258 Mon Sep 17 00:00:00 2001 From: Andrey Vasilevsky Date: Tue, 30 Jun 2026 17:35:21 -0700 Subject: [PATCH 4/4] chore: bump version to 0.19.1 (PR-review calibration fixes) Co-Authored-By: Claude Opus 4.8 --- Cargo.lock | 6 +++--- Cargo.toml | 2 +- mcp-package/package.json | 2 +- mcp-package/server.json | 4 ++-- vscode/package.json | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6fd2335..c3ebff9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -782,7 +782,7 @@ dependencies = [ [[package]] name = "codegraph-harness" -version = "0.19.0" +version = "0.19.1" dependencies = [ "anyhow", "clap", @@ -890,7 +890,7 @@ dependencies = [ [[package]] name = "codegraph-memory" -version = "0.19.0" +version = "0.19.1" dependencies = [ "anyhow", "bincode", @@ -1073,7 +1073,7 @@ dependencies = [ [[package]] name = "codegraph-server" -version = "0.19.0" +version = "0.19.1" dependencies = [ "clap", "codegraph", diff --git a/Cargo.toml b/Cargo.toml index 922e756..0830fdf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,7 +60,7 @@ members = [ ] [workspace.package] -version = "0.19.0" +version = "0.19.1" edition = "2021" license = "Apache-2.0" repository = "https://github.com/codegraph-ai/codegraph" diff --git a/mcp-package/package.json b/mcp-package/package.json index 335d570..b61d40a 100644 --- a/mcp-package/package.json +++ b/mcp-package/package.json @@ -1,6 +1,6 @@ { "name": "@astudioplus/codegraph-mcp", - "version": "0.19.0", + "version": "0.19.1", "mcpName": "io.github.codegraph-ai/codegraph", "description": "CodeGraph MCP server — cross-language code intelligence with 42 tools, 38 languages", "author": "Andrey Vasilevsky ", diff --git a/mcp-package/server.json b/mcp-package/server.json index b3a01b3..6568818 100644 --- a/mcp-package/server.json +++ b/mcp-package/server.json @@ -6,12 +6,12 @@ "url": "https://github.com/codegraph-ai/CodeGraph", "source": "github" }, - "version": "0.19.0", + "version": "0.19.1", "packages": [ { "registryType": "npm", "identifier": "@astudioplus/codegraph-mcp", - "version": "0.19.0", + "version": "0.19.1", "transport": { "type": "stdio" }, diff --git a/vscode/package.json b/vscode/package.json index 8628768..128d2bc 100644 --- a/vscode/package.json +++ b/vscode/package.json @@ -2,7 +2,7 @@ "name": "codegraph", "displayName": "CodeGraph", "description": "Cross-language code intelligence powered by graph analysis", - "version": "0.19.0", + "version": "0.19.1", "publisher": "aStudioPlus", "author": "Andrey Vasilevsky ", "license": "Apache-2.0",