Skip to content

Commit 689050f

Browse files
author
Your Name
committed
fix(mcp): get_impact resolves Class over Constructor for accurate blast radius
When a Class and its Constructor share the same name (common in C#/Java), get_impact previously picked the Constructor (which has 0 incoming CALLS), yielding empty blast radius results for any class query. Now mirrors trace_call_path's disambiguation logic: - Prefers Class node over same-named Constructor/Method - Expands through DEFINES_METHOD edges to get all method node IDs - Runs BFS from each method and merges results (dedup by closest hop) - Caps at 30 methods per class (vs trace's 5) for comprehensive coverage - Improved affected_processes matching: checks d=1 caller names too Tested on a 26K-node C# monolith: 'UserService' went from 0 callers to 16 direct callers, 19 total affected, HIGH risk, 20 affected processes.
1 parent 6a45196 commit 689050f

1 file changed

Lines changed: 128 additions & 7 deletions

File tree

src/mcp/mcp.c

Lines changed: 128 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1259,26 +1259,131 @@ static char *handle_get_impact(cbm_mcp_server_t *srv, const char *args) {
12591259
return r;
12601260
}
12611261

1262-
/* Pick best node (prefer Function/Method) */
1262+
/* Pick best node: prefer Class over Constructor when both share the same name.
1263+
* This mirrors the disambiguation logic in trace_call_path so that impact
1264+
* analysis on a class name (e.g. "UserService") resolves to the Class node
1265+
* and then fans out through DEFINES_METHOD to all its methods. Previously
1266+
* this picked the Constructor/Method first, which has 0 callers. */
12631267
int best = 0;
1268+
bool has_class = false;
1269+
int class_idx = -1;
12641270
for (int i = 0; i < node_count; i++) {
1265-
if (nodes[i].label && (strcmp(nodes[i].label, "Function") == 0 ||
1266-
strcmp(nodes[i].label, "Method") == 0)) {
1271+
const char *lbl = nodes[i].label;
1272+
if (lbl && (strcmp(lbl, "Class") == 0 || strcmp(lbl, "Interface") == 0)) {
1273+
has_class = true;
1274+
if (class_idx < 0) class_idx = i;
1275+
}
1276+
}
1277+
/* Look for a non-constructor Function/Method (skip if same name as Class) */
1278+
bool found_callable = false;
1279+
for (int i = 0; i < node_count; i++) {
1280+
const char *lbl = nodes[i].label;
1281+
if (lbl && (strcmp(lbl, "Function") == 0 || strcmp(lbl, "Method") == 0)) {
1282+
if (has_class) continue; /* skip constructor */
12671283
best = i;
1284+
found_callable = true;
12681285
break;
12691286
}
12701287
}
1288+
if (!found_callable && class_idx >= 0) {
1289+
best = class_idx;
1290+
}
1291+
1292+
/* Resolve start IDs: if target is a Class/Interface, expand through
1293+
* DEFINES_METHOD edges to get all method node IDs for BFS. */
1294+
int64_t *start_ids = NULL;
1295+
int start_id_count = 0;
1296+
bool is_class_like = false;
1297+
const char *best_label = nodes[best].label;
1298+
if (best_label &&
1299+
(strcmp(best_label, "Class") == 0 || strcmp(best_label, "Interface") == 0)) {
1300+
is_class_like = true;
1301+
}
1302+
1303+
if (is_class_like) {
1304+
cbm_edge_t *dm_edges = NULL;
1305+
int dm_count = 0;
1306+
cbm_store_find_edges_by_source_type(store, nodes[best].id, "DEFINES_METHOD",
1307+
&dm_edges, &dm_count);
1308+
if (dm_count > 0) {
1309+
/* For impact we use all methods (unlike trace which caps at 5) */
1310+
int use_count = dm_count > 30 ? 30 : dm_count;
1311+
start_ids = malloc((size_t)use_count * sizeof(int64_t));
1312+
for (int i = 0; i < use_count; i++) {
1313+
start_ids[i] = dm_edges[i].target_id;
1314+
}
1315+
start_id_count = use_count;
1316+
}
1317+
for (int i = 0; i < dm_count; i++) {
1318+
free((void *)dm_edges[i].project);
1319+
free((void *)dm_edges[i].type);
1320+
free((void *)dm_edges[i].properties_json);
1321+
}
1322+
free(dm_edges);
1323+
if (start_id_count == 0) {
1324+
start_ids = malloc(sizeof(int64_t));
1325+
start_ids[0] = nodes[best].id;
1326+
start_id_count = 1;
1327+
}
1328+
} else {
1329+
start_ids = malloc(sizeof(int64_t));
1330+
start_ids[0] = nodes[best].id;
1331+
start_id_count = 1;
1332+
}
12711333

12721334
yyjson_mut_obj_add_strcpy(doc, root, "target", target);
12731335
yyjson_mut_obj_add_strcpy(doc, root, "direction", direction);
12741336
yyjson_mut_obj_add_strcpy(doc, root, "file_path",
12751337
nodes[best].file_path ? nodes[best].file_path : "");
12761338
yyjson_mut_obj_add_int(doc, root, "line", nodes[best].start_line);
12771339

1278-
/* BFS with full depth */
1340+
/* BFS from each start ID and merge results. For classes this fans out
1341+
* through all methods, giving a true blast radius instead of 0. */
12791342
const char *call_types[] = {"CALLS", "HTTP_CALLS", "ASYNC_CALLS", "USAGE"};
12801343
cbm_traverse_result_t tr = {0};
1281-
cbm_store_bfs(store, nodes[best].id, bfs_dir, call_types, 4, max_depth, 200, &tr);
1344+
1345+
if (start_id_count == 1) {
1346+
cbm_store_bfs(store, start_ids[0], bfs_dir, call_types, 4, max_depth, 200, &tr);
1347+
} else {
1348+
/* Multi-method BFS: run from each method, collect unique visited nodes */
1349+
cbm_traverse_result_t *subs = calloc((size_t)start_id_count, sizeof(*subs));
1350+
int total_visited = 0;
1351+
for (int s = 0; s < start_id_count; s++) {
1352+
cbm_store_bfs(store, start_ids[s], bfs_dir, call_types, 4, max_depth,
1353+
200, &subs[s]);
1354+
total_visited += subs[s].visited_count;
1355+
}
1356+
/* Merge into tr: allocate worst-case, then dedup by node id */
1357+
if (total_visited > 0) {
1358+
tr.visited = malloc((size_t)total_visited * sizeof(cbm_node_hop_t));
1359+
tr.visited_count = 0;
1360+
for (int s = 0; s < start_id_count; s++) {
1361+
for (int v = 0; v < subs[s].visited_count; v++) {
1362+
int64_t vid = subs[s].visited[v].node.id;
1363+
/* Check for duplicate (same node already in tr) */
1364+
bool dup = false;
1365+
for (int e = 0; e < tr.visited_count; e++) {
1366+
if (tr.visited[e].node.id == vid) {
1367+
/* Keep the one with smaller hop (closer = more impacted) */
1368+
if (subs[s].visited[v].hop < tr.visited[e].hop)
1369+
tr.visited[e].hop = subs[s].visited[v].hop;
1370+
dup = true;
1371+
break;
1372+
}
1373+
}
1374+
if (!dup && tr.visited_count < total_visited) {
1375+
tr.visited[tr.visited_count] = subs[s].visited[v];
1376+
tr.visited_count++;
1377+
}
1378+
}
1379+
}
1380+
}
1381+
/* Free sub-traversals (but NOT their visited[].node fields — we moved them) */
1382+
for (int s = 0; s < start_id_count; s++) {
1383+
free(subs[s].edges);
1384+
}
1385+
free(subs);
1386+
}
12821387

12831388
/* Group by depth */
12841389
yyjson_mut_val *d1_arr = yyjson_mut_arr(doc);
@@ -1332,15 +1437,30 @@ static char *handle_get_impact(cbm_mcp_server_t *srv, const char *args) {
13321437
yyjson_mut_obj_add_strcpy(doc, summary, "d3", d3_label);
13331438
yyjson_mut_obj_add_val(doc, root, "summary", summary);
13341439

1335-
/* Affected processes */
1440+
/* Affected processes — match by checking if any BFS-visited node name
1441+
* appears in the process label, OR if the target name itself appears.
1442+
* This catches processes that flow through the target's methods. */
13361443
{
13371444
cbm_process_info_t *procs = NULL;
13381445
int pcount = 0;
13391446
cbm_store_list_processes(store, project, &procs, &pcount);
13401447
yyjson_mut_val *paff = yyjson_mut_arr(doc);
13411448
int pc = 0;
13421449
for (int pi = 0; pi < pcount && pc < 20; pi++) {
1343-
if (procs[pi].label && target && strstr(procs[pi].label, target)) {
1450+
if (!procs[pi].label) continue;
1451+
bool match = false;
1452+
/* Check target name */
1453+
if (target && strstr(procs[pi].label, target)) match = true;
1454+
/* Check BFS-visited node names (d=1 callers are most likely) */
1455+
if (!match) {
1456+
for (int v = 0; v < tr.visited_count && !match; v++) {
1457+
if (tr.visited[v].hop == 1 && tr.visited[v].node.name &&
1458+
strstr(procs[pi].label, tr.visited[v].node.name)) {
1459+
match = true;
1460+
}
1461+
}
1462+
}
1463+
if (match) {
13441464
yyjson_mut_val *pitem = yyjson_mut_obj(doc);
13451465
yyjson_mut_obj_add_strcpy(doc, pitem, "label", procs[pi].label);
13461466
yyjson_mut_obj_add_int(doc, pitem, "step_count", procs[pi].step_count);
@@ -1356,6 +1476,7 @@ static char *handle_get_impact(cbm_mcp_server_t *srv, const char *args) {
13561476
yyjson_mut_doc_free(doc);
13571477
cbm_store_traverse_free(&tr);
13581478
cbm_store_free_nodes(nodes, node_count);
1479+
free(start_ids);
13591480
free(target); free(project); free(direction);
13601481
char *result = cbm_mcp_text_result(json, false);
13611482
free(json);

0 commit comments

Comments
 (0)