Skip to content

Commit ba5942f

Browse files
anvansterclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 29ee815 commit ba5942f

2 files changed

Lines changed: 20 additions & 2 deletions

File tree

crates/codegraph-server/src/domain/node_props.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,3 +109,11 @@ pub(crate) fn is_public(node: &Node) -> bool {
109109
.or_else(|| node.properties.get_bool("exported"))
110110
.unwrap_or_else(|| matches!(visibility(node), "public" | "pub"))
111111
}
112+
113+
/// Whether the node is a test function, as recorded at index time from the
114+
/// language's test marker (`#[test]`/`#[cfg(test)]`, `@Test`, etc.). This is
115+
/// the structural signal; callers should prefer it over name heuristics, which
116+
/// miss idiomatic test names (Rust `#[cfg(test)] mod tests { fn descriptive() }`).
117+
pub(crate) fn is_test(node: &Node) -> bool {
118+
node.properties.get_bool("is_test").unwrap_or(false)
119+
}

crates/codegraph-server/src/mcp/server.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4104,7 +4104,13 @@ impl McpServer {
41044104
if let Ok(caller) = graph.get_node(caller_id) {
41054105
let cname = crate::domain::node_props::name(caller);
41064106
let cfile = caller.properties.get_string("path").unwrap_or("");
4107-
let is_test = cname.to_lowercase().starts_with("test_")
4107+
// Prefer the structural is_test marker recorded at index time
4108+
// (#[test]/#[cfg(test)], @Test, …); fall back to name/path
4109+
// heuristics only for languages that don't populate it. The
4110+
// heuristics alone miss idiomatic Rust tests with descriptive
4111+
// names inside `#[cfg(test)] mod tests`.
4112+
let is_test = crate::domain::node_props::is_test(caller)
4113+
|| cname.to_lowercase().starts_with("test_")
41084114
|| cname.to_lowercase().contains("_test")
41094115
|| cfile.contains("/tests/")
41104116
|| cfile.contains("/test_");
@@ -4132,7 +4138,11 @@ impl McpServer {
41324138
// #87: Test gap — function has no test callers.
41334139
// Skip functions that ARE tests (they don't need
41344140
// their own coverage) and trivial getters/setters.
4135-
let fn_is_test = func_name.to_lowercase().starts_with("test_")
4141+
let fn_is_test = graph
4142+
.get_node(*node_id)
4143+
.ok()
4144+
.is_some_and(|n| crate::domain::node_props::is_test(n))
4145+
|| func_name.to_lowercase().starts_with("test_")
41364146
|| func_name.to_lowercase().contains("_test")
41374147
|| changed_rel[idx].contains("/tests/")
41384148
|| changed_rel[idx].contains("_test.");

0 commit comments

Comments
 (0)