Skip to content

fix(mcp): emit trace evidence columns in header order #1703

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rudi193-cmd wants to merge 1 commit into
base: main
Choose a base branch
from
+163 −20
Open

fix(mcp): emit trace evidence columns in header order #1703

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 50 additions & 20 deletions src/mcp/mcp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5954,25 +5954,33 @@ 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)) {
continue;
}
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";
}
if (include_tests) {
cols[ncols++] = "test";
}
if (include_evidence) {
cols[ncols++] = "strategy";
cols[ncols++] = "confidence";
}
if (data_flow) {
cols[ncols++] = "args";
}
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -6782,15 +6810,17 @@ 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);
}
}
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);
}
Expand Down
113 changes: 113 additions & 0 deletions tests/test_mcp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
Loading