diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index bb5147c06..86cea7bde 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -5954,10 +5954,13 @@ static bool bfs_edge_evidence_for_hop(cbm_traverse_result_t *tr, int64_t hop_nod } /* TOON table for one trace direction: callees[N]{qn,hop,...} with optional - * risk / test / args columns. `name` is omitted (it is the qn's last - * segment); the per-item JSON key envelope was 84% of the legacy payload. */ + * risk / test / evidence / args columns. `name` is omitted (it is the qn's + * last segment); the per-item JSON key envelope was 84% of the legacy + * payload. include_evidence must be forwarded here — flat_trace used to + * drop it (#1542 leftover). */ static void bfs_to_toon_table(cbm_sb_t *sb, const char *key, cbm_traverse_result_t *tr, - bool risk_labels, bool include_tests, bool data_flow) { + bool risk_labels, bool include_tests, bool data_flow, + bool include_evidence) { int visible = 0; for (int i = 0; i < tr->visited_count; i++) { if (!include_tests && is_test_file(tr->visited[i].node.file_path)) { @@ -5965,7 +5968,8 @@ static void bfs_to_toon_table(cbm_sb_t *sb, const char *key, cbm_traverse_result } visible++; } - const char *cols[5] = {"qn", "hop"}; + /* Max: qn hop risk test strategy confidence args. */ + const char *cols[7] = {"qn", "hop"}; int ncols = 2; if (risk_labels) { cols[ncols++] = "risk"; @@ -5973,6 +5977,10 @@ static void bfs_to_toon_table(cbm_sb_t *sb, const char *key, cbm_traverse_result if (include_tests) { cols[ncols++] = "test"; } + if (include_evidence) { + cols[ncols++] = "strategy"; + cols[ncols++] = "confidence"; + } if (data_flow) { cols[ncols++] = "args"; } @@ -5992,6 +6000,23 @@ static void bfs_to_toon_table(cbm_sb_t *sb, const char *key, cbm_traverse_result if (include_tests) { cbm_tree_cell_bool(sb, test, false); } + if (include_evidence) { + const char *ev_class = NULL; + double ev_conf = -1.0; + if (bfs_edge_evidence_for_hop(tr, tr->visited[i].node.id, &ev_class, &ev_conf)) { + cbm_tree_cell_str(sb, ev_class ? ev_class : "", false); + if (ev_conf >= 0.0) { + cbm_tree_cell_real(sb, ev_conf, false); + } else { + cbm_tree_cell_str(sb, "-", false); + } + } else { + /* Root hop / non-CALLS: keep column count fixed, same "-" + * placeholders as bfs_to_tree_table. */ + cbm_tree_cell_str(sb, "-", false); + cbm_tree_cell_str(sb, "-", false); + } + } if (data_flow) { size_t alen = 0; const char *ea = bfs_edge_args_for_hop(tr, tr->visited[i].node.id, &alen); @@ -6337,20 +6362,9 @@ static yyjson_mut_val *bfs_to_tree_json(yyjson_mut_doc *doc, cbm_traverse_result if (risk_labels) { yyjson_mut_arr_add_str(doc, row, cbm_risk_label(cbm_hop_to_risk(tr->visited[i].hop))); } - if (data_flow) { - size_t alen = 0; - const char *ea = bfs_edge_args_for_hop(tr, tr->visited[i].node.id, &alen); - if (ea && alen > 0) { - yyjson_mut_val *av = yyjson_mut_rawn(doc, ea, alen); - if (av) { - yyjson_mut_arr_add_val(row, av); - } else { - yyjson_mut_arr_add_str(doc, row, ""); - } - } else { - yyjson_mut_arr_add_str(doc, row, ""); - } - } + /* Emit in header order: strategy, confidence, then args. Swapping these + * two blocks mislabeled every include_evidence+data_flow json row + * (#1542 leftover). */ if (include_evidence) { const char *ev_class = NULL; double ev_conf = -1.0; @@ -6370,6 +6384,20 @@ static yyjson_mut_val *bfs_to_tree_json(yyjson_mut_doc *doc, cbm_traverse_result yyjson_mut_arr_add_null(doc, row); } } + if (data_flow) { + size_t alen = 0; + const char *ea = bfs_edge_args_for_hop(tr, tr->visited[i].node.id, &alen); + if (ea && alen > 0) { + yyjson_mut_val *av = yyjson_mut_rawn(doc, ea, alen); + if (av) { + yyjson_mut_arr_add_val(row, av); + } else { + yyjson_mut_arr_add_str(doc, row, ""); + } + } else { + yyjson_mut_arr_add_str(doc, row, ""); + } + } yyjson_mut_arr_add_val(cur_rows, row); } yyjson_mut_obj_add_val(doc, leg, "groups", groups); @@ -6782,7 +6810,8 @@ static char *handle_trace_call_path(cbm_mcp_server_t *srv, const char *args) { if (do_outbound) { cbm_tree_scalar_int(&sb, "callees_total", out_total); if (flat_trace) { - bfs_to_toon_table(&sb, "callees", &view_out, risk_labels, include_tests, data_flow); + bfs_to_toon_table(&sb, "callees", &view_out, risk_labels, include_tests, data_flow, + include_evidence); } else { bfs_to_tree_table(&sb, "callees", &view_out, include_tests, include_evidence); } @@ -6790,7 +6819,8 @@ static char *handle_trace_call_path(cbm_mcp_server_t *srv, const char *args) { if (do_inbound) { cbm_tree_scalar_int(&sb, "callers_total", in_total); if (flat_trace) { - bfs_to_toon_table(&sb, "callers", &view_in, risk_labels, include_tests, data_flow); + bfs_to_toon_table(&sb, "callers", &view_in, risk_labels, include_tests, data_flow, + include_evidence); } else { bfs_to_tree_table(&sb, "callers", &view_in, include_tests, include_evidence); } diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 14d6d066f..2b2341653 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -3385,6 +3385,118 @@ TEST(tool_trace_path_evidence_is_opt_in_and_class_mapped) { PASS(); } +/* #1542 leftover: header order is strategy,confidence then args, but json + * used to emit args first; tree flat_trace (risk_labels || data_flow) used + * to call bfs_to_toon_table without include_evidence. Pin both: every row + * has len(cols)==len(row), and the strategy cell is the class not the args + * array. */ +TEST(tool_trace_path_evidence_columns_match_header_issue1542) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + cbm_store_t *st = cbm_mcp_server_store(srv); + const char *proj = "ev-order"; + cbm_mcp_server_set_project(srv, proj); + cbm_store_upsert_project(st, proj, "/tmp/ev-order"); + cbm_node_t caller = {.project = proj, + .label = "Function", + .name = "caller", + .qualified_name = "ev-order.src.caller", + .file_path = "src/a.c", + .start_line = 1, + .end_line = 5}; + cbm_node_t callee = {.project = proj, + .label = "Function", + .name = "target", + .qualified_name = "ev-order.src.target", + .file_path = "src/a.c", + .start_line = 10, + .end_line = 20}; + int64_t id_caller = cbm_store_upsert_node(st, &caller); + int64_t id_callee = cbm_store_upsert_node(st, &callee); + ASSERT_GT(id_caller, 0); + ASSERT_GT(id_callee, 0); + cbm_edge_t e = {.project = proj, + .source_id = id_caller, + .target_id = id_callee, + .type = "CALLS", + .properties_json = "{\"callee\":\"target\",\"confidence\":0.95," + "\"strategy\":\"lsp_trait_dispatch\",\"candidates\":1," + "\"args\":[\"x\"]}"}; + ASSERT_GT(cbm_store_insert_edge(st, &e), 0); + + /* json × data_flow × include_evidence: cols identity, not just count. */ + char *js = cbm_mcp_server_handle( + srv, "{\"jsonrpc\":\"2.0\",\"id\":94,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"trace_path\",\"arguments\":{\"function_name\":\"caller\"," + "\"project\":\"ev-order\",\"direction\":\"outbound\",\"include_evidence\":true," + "\"mode\":\"data_flow\",\"format\":\"json\"}}}"); + ASSERT_NOT_NULL(js); + char *js_txt = extract_text_content(js); + ASSERT_NOT_NULL(js_txt); + yyjson_doc *doc = yyjson_read(js_txt, strlen(js_txt), 0); + ASSERT_NOT_NULL(doc); + yyjson_val *callees = yyjson_obj_get(yyjson_doc_get_root(doc), "callees"); + ASSERT_NOT_NULL(callees); + yyjson_val *cols = yyjson_obj_get(callees, "cols"); + ASSERT_NOT_NULL(cols); + static const char *want[] = {"name", "hop", "strategy", "confidence", "args"}; + ASSERT_EQ((int)yyjson_arr_size(cols), 5); + for (int i = 0; i < 5; i++) { + ASSERT_STR_EQ(yyjson_get_str(yyjson_arr_get(cols, i)), want[i]); + } + yyjson_val *hop1 = NULL; + yyjson_val *groups = yyjson_obj_get(callees, "groups"); + ASSERT_NOT_NULL(groups); + size_t ng = yyjson_arr_size(groups); + for (size_t g = 0; g < ng; g++) { + yyjson_val *rows = yyjson_obj_get(yyjson_arr_get(groups, g), "rows"); + if (!rows) { + continue; + } + size_t nr = yyjson_arr_size(rows); + for (size_t r = 0; r < nr; r++) { + yyjson_val *row = yyjson_arr_get(rows, r); + yyjson_val *hop = row ? yyjson_arr_get(row, 1) : NULL; + if (hop && yyjson_get_int(hop) >= 1) { + hop1 = row; + break; + } + } + if (hop1) { + break; + } + } + ASSERT_NOT_NULL(hop1); + ASSERT_EQ((int)yyjson_arr_size(hop1), 5); + ASSERT_TRUE(yyjson_is_str(yyjson_arr_get(hop1, 2))); + ASSERT_STR_EQ(yyjson_get_str(yyjson_arr_get(hop1, 2)), "lsp"); + ASSERT_TRUE(yyjson_is_num(yyjson_arr_get(hop1, 3))); + ASSERT_TRUE(yyjson_is_arr(yyjson_arr_get(hop1, 4))); + yyjson_doc_free(doc); + free(js_txt); + free(js); + + /* tree × risk_labels × include_evidence used to drop evidence entirely + * because flat_trace routed through bfs_to_toon_table without the flag. */ + char *tree = cbm_mcp_server_handle( + srv, "{\"jsonrpc\":\"2.0\",\"id\":95,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"trace_path\",\"arguments\":{\"function_name\":\"caller\"," + "\"project\":\"ev-order\",\"direction\":\"outbound\",\"include_evidence\":true," + "\"risk_labels\":true}}}"); + ASSERT_NOT_NULL(tree); + char *tree_txt = extract_text_content(tree); + ASSERT_NOT_NULL(tree_txt); + ASSERT_NOT_NULL(strstr(tree_txt, "strategy")); + ASSERT_NOT_NULL(strstr(tree_txt, "confidence")); + ASSERT_NOT_NULL(strstr(tree_txt, "lsp")); + ASSERT_NOT_NULL(strstr(tree_txt, "0.95")); + ASSERT_NULL(strstr(tree_txt, "lsp_trait_dispatch")); + free(tree_txt); + free(tree); + + cbm_mcp_server_free(srv); + PASS(); +} + /* Reproduce-first (#887): the client-supplied `depth` on trace_call_path must be * clamped to the MCP ceiling (cbm_mcp_max_depth(), default 15). On origin/main * an MCP_MAX_DEPTH=15 constant was defined but never applied — `depth` flowed @@ -11188,6 +11300,7 @@ SUITE(mcp) { RUN_TEST(tool_trace_call_path_prefers_definition); RUN_TEST(trace_evidence_strategy_class_vocabulary_is_closed); RUN_TEST(tool_trace_path_evidence_is_opt_in_and_class_mapped); + RUN_TEST(tool_trace_path_evidence_columns_match_header_issue1542); RUN_TEST(tool_trace_call_path_depth_clamped); RUN_TEST(tool_trace_call_path_distinct_defs_not_over_unioned); RUN_TEST(tool_trace_call_path_dts_stub_unions_with_impl);