From 2cceb3f194e75e2ee5e5b3e67eac1607b0d513cf Mon Sep 17 00:00:00 2001 From: Anton Standrik Date: Wed, 23 Sep 2026 12:41:59 +0300 Subject: [PATCH 1/2] fix(mcp): classify test nodes consistently in trace output Use test metadata and anchored path components for trace filtering, totals and test markers. Recognize .spec files and preserve filtering before pagination without pruning traversal. Add regression coverage for metadata, render modes and pagination. Refs #1593 Signed-off-by: Anton Standrik --- src/mcp/mcp.c | 59 +++++++------ tests/test_mcp.c | 221 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 252 insertions(+), 28 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index d6c7bd4686..479f2df7c5 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -8069,23 +8069,29 @@ static yyjson_doc *resolve_trace_edge_types(const char *args, const char *mode, return NULL; } -/* Check if a file path looks like a test file. The substring checks below - * only catch a tests/ directory nested under another path component - * (".../tests/foo"); a project-root-relative path like "tests/repro/foo.c" - * has no leading slash before "tests" and fell through undetected, leaking - * whole test subtrees into query_graph/trace_path results with the default - * include_tests=false (#1294). */ -static bool is_test_file(const char *path) { - if (!path) { - return false; +/* Path rules also cover legacy nodes with missing or false is_test metadata. */ +static bool trace_node_is_test(const cbm_node_t *node) { + const char *base = node->file_path; + if (base) { + for (const char *end = base; *end; end++) { + if (*end != '/' && *end != '\\') { + continue; + } + size_t length = (size_t)(end - base); + if ((length == SLEN("test") && strncmp(base, "test", length) == 0) || + (length == SLEN("tests") && strncmp(base, "tests", length) == 0) || + (length == SLEN("spec") && strncmp(base, "spec", length) == 0) || + (length == SLEN("__tests__") && strncmp(base, "__tests__", length) == 0)) { + return true; + } + base = end + SKIP_ONE; + } + if (strncmp(base, "test_", SLEN("test_")) == 0 || strstr(base, "_test.") || + strstr(base, ".test.") || strstr(base, ".spec.")) { + return true; + } } - return strstr(path, "/test") != NULL || strstr(path, "test_") != NULL || - strstr(path, "_test.") != NULL || strstr(path, "/tests/") != NULL || - strstr(path, "/spec/") != NULL || strstr(path, ".test.") != NULL || - strncmp(path, "tests/", SLEN("tests/")) == 0 || - strncmp(path, "test/", SLEN("test/")) == 0 || - strncmp(path, "spec/", SLEN("spec/")) == 0 || - strncmp(path, "__tests__/", SLEN("__tests__/")) == 0; + return node->properties_json && cbm_mcp_get_bool_arg(node->properties_json, "is_test"); } /* Filtering belongs before page-window calculation: hidden test rows must not @@ -8094,7 +8100,7 @@ static bool is_test_file(const char *path) { static void trace_filter_test_rows(cbm_traverse_result_t *tr) { int write_index = 0; for (int read_index = 0; read_index < tr->visited_count; read_index++) { - if (is_test_file(tr->visited[read_index].node.file_path)) { + if (trace_node_is_test(&tr->visited[read_index].node)) { cbm_node_free_fields(&tr->visited[read_index].node); continue; } @@ -8292,7 +8298,7 @@ static void bfs_to_toon_table(cbm_sb_t *sb, const char *key, cbm_traverse_result bool include_evidence, const trace_edge_context_t *edge_ctx) { int visible = 0; for (int i = 0; i < tr->visited_count; i++) { - if (!include_tests && is_test_file(tr->visited[i].node.file_path)) { + if (!include_tests && trace_node_is_test(&tr->visited[i].node)) { continue; } visible++; @@ -8315,8 +8321,7 @@ static void bfs_to_toon_table(cbm_sb_t *sb, const char *key, cbm_traverse_result } cbm_tree_table_header(sb, key, visible, cols, ncols); for (int i = 0; i < tr->visited_count; i++) { - const char *fp = tr->visited[i].node.file_path; - bool test = is_test_file(fp); + bool test = trace_node_is_test(&tr->visited[i].node); if (!include_tests && test) { continue; } @@ -8751,7 +8756,7 @@ static yyjson_mut_val *bfs_to_tree_json(yyjson_mut_doc *doc, cbm_traverse_result char *cur_group = NULL; bool have_group = false; for (int i = 0; i < tr->visited_count; i++) { - if (!include_tests && is_test_file(tr->visited[i].node.file_path)) { + if (!include_tests && trace_node_is_test(&tr->visited[i].node)) { continue; } const char *qn = @@ -8778,7 +8783,7 @@ static yyjson_mut_val *bfs_to_tree_json(yyjson_mut_doc *doc, cbm_traverse_result yyjson_mut_arr_add_str(doc, row, cbm_risk_label(cbm_hop_to_risk(tr->visited[i].hop))); } if (include_tests) { - yyjson_mut_arr_add_bool(doc, row, is_test_file(tr->visited[i].node.file_path)); + yyjson_mut_arr_add_bool(doc, row, trace_node_is_test(&tr->visited[i].node)); } const cbm_edge_info_t *predecessor = (data_flow || include_evidence) ? trace_predecessor_edge(edge_ctx, &tr->visited[i]) @@ -8845,7 +8850,7 @@ static void bfs_to_tree_table(cbm_sb_t *sb, const char *key, cbm_traverse_result const trace_edge_context_t *edge_ctx) { int visible = 0; for (int i = 0; i < tr->visited_count; i++) { - if (!include_tests && is_test_file(tr->visited[i].node.file_path)) { + if (!include_tests && trace_node_is_test(&tr->visited[i].node)) { continue; } visible++; @@ -8861,7 +8866,7 @@ static void bfs_to_tree_table(cbm_sb_t *sb, const char *key, cbm_traverse_result bool ordered_owned = ordered != NULL; int ordered_count = 0; for (int i = 0; i < tr->visited_count; i++) { - if (!include_tests && is_test_file(tr->visited[i].node.file_path)) { + if (!include_tests && trace_node_is_test(&tr->visited[i].node)) { continue; } if (ordered) { @@ -8895,7 +8900,7 @@ static void bfs_to_tree_table(cbm_sb_t *sb, const char *key, cbm_traverse_result cbm_tree_cell_str(sb, plen ? qn + plen + 1 : qn, true); cbm_tree_cell_int(sb, ordered[i].hop, false); if (include_tests) { - cbm_tree_cell_bool(sb, is_test_file(ordered[i].node.file_path), false); + cbm_tree_cell_bool(sb, trace_node_is_test(&ordered[i].node), false); } const char *ev_class = NULL; double ev_conf = -1.0; @@ -9383,13 +9388,13 @@ render_trace_output:; * the tool). Count with the same filter the emitters apply. */ int out_total = 0; for (int i = 0; i < tr_out.visited_count; i++) { - if (include_tests || !is_test_file(tr_out.visited[i].node.file_path)) { + if (include_tests || !trace_node_is_test(&tr_out.visited[i].node)) { out_total++; } } int in_total = 0; for (int i = 0; i < tr_in.visited_count; i++) { - if (include_tests || !is_test_file(tr_in.visited[i].node.file_path)) { + if (include_tests || !trace_node_is_test(&tr_in.visited[i].node)) { in_total++; } } diff --git a/tests/test_mcp.c b/tests/test_mcp.c index ffa9ea3a3b..67fb521232 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -2766,6 +2766,223 @@ TEST(tool_trace_totals_respect_test_filter_tests_root_subtree_issue1294) { PASS(); } +TEST(tool_trace_test_classification_issue1593) { + static const struct { + const char *path; + const char *properties; + bool is_test; + } cases[] = { + {"e2e/module.spec.ts", NULL, true}, + {"e2e/function.spec.ts", NULL, true}, + {"e2e/method.spec.ts", NULL, true}, + {"src/core.test.ts", NULL, true}, + {"tests/root.ts", NULL, true}, + {"test/root.ts", NULL, true}, + {"spec/root.ts", NULL, true}, + {"__tests__/root.ts", NULL, true}, + {"app/tests/nested.ts", NULL, true}, + {"app/test/nested.ts", NULL, true}, + {"app/spec/nested.ts", NULL, true}, + {"app/__tests__/nested.ts", NULL, true}, + {"tests\\root.ts", NULL, true}, + {"app\\__tests__\\nested.ts", NULL, true}, + {"src/test_core.py", NULL, true}, + {"src/core_test.c", NULL, true}, + {"src/main.ts", NULL, false}, + {"src/testimonials.ts", NULL, false}, + {"src/test_helpers/main.ts", NULL, false}, + {"src/my__tests__/main.ts", NULL, false}, + {"src/contest.ts", NULL, false}, + {"src/inline.rs", "{\"is_test\":true}", true}, + {"src/inline.rs", "{ \"is_test\" : true }", true}, + {NULL, "{\"is_test\":true}", true}, + {"src/main.ts", "{\"is_test\":false}", false}, + {"e2e/legacy.spec.ts", "{\"is_test\":false}", true}, + {"src/main.ts", "{\"is_test\":\"true\"}", false}, + {"src/main.ts", "{\"is_test\":1}", false}, + {"src/main.ts", "{\"nested\":{\"is_test\":true}}", false}, + {"src/main.ts", "null", false}, + {"src/main.ts", "{broken", false}, + {"e2e/broken.spec.ts", "{broken", true}, + {NULL, NULL, false}, + {"", "", false}, + }; + static const char *const labels[] = {"Module", "Function", "Method"}; + static const char *const directions[] = {"outbound", "inbound", "both"}; + static const char *const formats[] = {"json", "tree"}; + static const struct { + const char *args; + bool risk; + } views[] = {{"", false}, + {",\"risk_labels\":true", true}, + {",\"mode\":\"data_flow\"", false}, + {",\"mode\":\"data_flow\",\"risk_labels\":true,\"include_evidence\":true", true}}; + const size_t count = sizeof(cases) / sizeof(cases[0]); + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *store = cbm_mcp_server_store(srv); + const char *project = "trace-test-kinds"; + cbm_mcp_server_set_project(srv, project); + ASSERT_EQ(cbm_store_upsert_project(store, project, "/tmp/trace-test-kinds"), CBM_STORE_OK); + cbm_node_t hub = {.project = project, + .label = "Function", + .name = "hub", + .qualified_name = "trace-test-kinds.hub", + .file_path = "src/hub.ts"}; + int64_t hub_id = cbm_store_upsert_node(store, &hub); + ASSERT_GT(hub_id, 0); + size_t production_count = 0; + for (size_t i = 0; i < count; i++) { + char name[32], qn[64]; + snprintf(name, sizeof(name), "case_%02zu", i); + snprintf(qn, sizeof(qn), "%s.%s", project, name); + cbm_node_t node = {.project = project, + .label = labels[i % 3], + .name = name, + .qualified_name = qn, + .file_path = cases[i].path, + .properties_json = cases[i].properties}; + int64_t id = cbm_store_upsert_node(store, &node); + ASSERT_GT(id, 0); + for (int inbound = 0; inbound < 2; inbound++) { + cbm_edge_t edge = {.project = project, + .source_id = inbound ? id : hub_id, + .target_id = inbound ? hub_id : id, + .type = "CALLS", + .properties_json = + "{\"strategy\":\"unique_name\",\"confidence\":0.75," + "\"args\":[{\"i\":0,\"e\":\"value\"}]}"}; + ASSERT_GT(cbm_store_insert_edge(store, &edge), 0); + } + production_count += !cases[i].is_test; + } + + bool ok = true; + for (size_t d = 0; d < 3; d++) { + for (size_t f = 0; f < 2; f++) { + for (size_t v = 0; v < sizeof(views) / sizeof(views[0]); v++) { + for (int include_tests = 0; include_tests < 2; include_tests++) { + char args[512]; + snprintf(args, sizeof(args), + "{\"project\":\"%s\",\"function_name\":\"hub\",\"direction\":\"%s\"," + "\"depth\":1,\"limit\":100,\"max_output_tokens\":16000," + "\"format\":\"%s\",\"include_tests\":%s%s}", + project, directions[d], formats[f], include_tests ? "true" : "false", + views[v].args); + char *response = cbm_mcp_handle_tool(srv, "trace_path", args); + char *inner = response ? extract_text_content(response) : NULL; + yyjson_doc *doc = inner && f == 0 ? yyjson_read(inner, strlen(inner), 0) : NULL; + yyjson_val *root = doc ? yyjson_doc_get_root(doc) : NULL; + bool matches = inner && (f != 0 || doc); + size_t expected = include_tests ? count : production_count; + for (int inbound = 0; matches && inbound < 2; inbound++) { + if ((d == 0 && inbound) || (d == 1 && !inbound)) { + continue; + } + const char *leg = inbound ? "callers" : "callees"; + char total[64]; + snprintf(total, sizeof(total), "%s_total", leg); + yyjson_val *table = root ? yyjson_obj_get(root, leg) : NULL; + if (f == 0) { + yyjson_val *value = yyjson_obj_get(root, total); + matches = yyjson_is_int(value) && yyjson_get_uint(value) == expected; + } else { + snprintf(total, sizeof(total), "%s_total: %zu\n", leg, expected); + matches = strstr(inner, total) != NULL; + } + for (size_t i = 0; matches && i < count; i++) { + char name[32]; + snprintf(name, sizeof(name), "case_%02zu", i); + bool visible = include_tests || !cases[i].is_test; + if (f == 0) { + yyjson_val *row = trace_grouped_row_named(table, name, NULL); + matches = (row != NULL) == visible; + if (matches && visible && include_tests) { + yyjson_val *test = yyjson_arr_get(row, views[v].risk ? 3 : 2); + matches = yyjson_is_bool(test) && + yyjson_get_bool(test) == cases[i].is_test; + } + } else { + matches = (strstr(inner, name) != NULL) == visible; + if (matches && visible && include_tests) { + char row[96]; + snprintf(row, sizeof(row), "%s 1 %s%s", name, + views[v].risk ? "CRITICAL " : "", + cases[i].is_test ? "true" : "false"); + matches = strstr(inner, row) != NULL; + } + } + } + } + if (!matches && ok) { + fprintf(stderr, "trace classification mismatch: %s/%s view=%zu tests=%d\n", + directions[d], formats[f], v, include_tests); + } + ok = ok && matches; + yyjson_doc_free(doc); + free(inner); + free(response); + } + } + } + } + cbm_mcp_server_free(srv); + ASSERT_TRUE(ok); + PASS(); +} + +TEST(tool_trace_test_filter_preserves_walk_issue1593) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *store = cbm_mcp_server_store(srv); + const char *project = "trace-test-walk"; + cbm_mcp_server_set_project(srv, project); + ASSERT_EQ(cbm_store_upsert_project(store, project, "/tmp/trace-test-walk"), CBM_STORE_OK); + const char *names[] = {"hub", "hidden", "visible"}; + const char *paths[] = {"src/hub.ts", "e2e/login.spec.ts", "src/visible.ts"}; + int64_t ids[3]; + for (int i = 0; i < 3; i++) { + cbm_node_t node = {.project = project, + .label = "Function", + .name = names[i], + .qualified_name = names[i], + .file_path = paths[i]}; + ids[i] = cbm_store_upsert_node(store, &node); + ASSERT_GT(ids[i], 0); + if (i > 0) { + cbm_edge_t edge = { + .project = project, .source_id = ids[i - 1], .target_id = ids[i], .type = "CALLS"}; + ASSERT_GT(cbm_store_insert_edge(store, &edge), 0); + } + } + bool ok = true; + for (int depth = 1; depth <= 2; depth++) { + char args[256]; + snprintf(args, sizeof(args), + "{\"project\":\"%s\",\"function_name\":\"hub\",\"direction\":\"outbound\"," + "\"depth\":%d,\"limit\":1,\"format\":\"json\"}", + project, depth); + char *response = cbm_mcp_handle_tool(srv, "trace_path", args); + char *inner = response ? extract_text_content(response) : NULL; + yyjson_doc *doc = inner ? yyjson_read(inner, strlen(inner), 0) : NULL; + yyjson_val *root = doc ? yyjson_doc_get_root(doc) : NULL; + yyjson_val *leg = root ? yyjson_obj_get(root, "callees") : NULL; + yyjson_val *row = trace_grouped_row_named(leg, "visible", NULL); + ok = ok && doc && yyjson_is_obj(leg) && + yyjson_is_int(yyjson_obj_get(root, "callees_total")) && + yyjson_get_int(yyjson_obj_get(root, "callees_total")) == depth - 1 && + !yyjson_obj_get(root, "next_cursor") && + !trace_grouped_row_named(leg, "hidden", NULL) && + (depth == 1 ? row == NULL : row && yyjson_get_int(yyjson_arr_get(row, 1)) == 2); + yyjson_doc_free(doc); + free(inner); + free(response); + } + cbm_mcp_server_free(srv); + ASSERT_TRUE(ok); + PASS(); +} + /* SCC condensation (get_architecture aspect "cycles"): a 3-function CALLS * cycle A->B->C->A must be reported as one circular dependency of size 3 with * all three members; a separate acyclic chain (D->E) must NOT appear. The @@ -6484,7 +6701,7 @@ TEST(tool_trace_paging_filters_before_window_and_hashes_effective_args) { .label = "Function", .name = "hidden_test", .qualified_name = "trace-visible-page.hidden_test", - .file_path = "tests/hidden_test.c", + .file_path = "e2e/hidden.spec.ts", .start_line = 1, .end_line = 3}; cbm_node_t callee = {.project = project, @@ -20524,6 +20741,8 @@ SUITE(mcp) { RUN_TEST(tool_search_graph_grouped_dotless_qn_round_trips); RUN_TEST(tool_trace_totals_respect_test_filter); RUN_TEST(tool_trace_totals_respect_test_filter_tests_root_subtree_issue1294); + RUN_TEST(tool_trace_test_classification_issue1593); + RUN_TEST(tool_trace_test_filter_preserves_walk_issue1593); RUN_TEST(tool_get_architecture_cycles_detects_scc); RUN_TEST(tool_get_code_snippet_clips_whole_file_node); RUN_TEST(tool_get_code_snippet_omits_over_budget_whole_line); From 6e3d8c296f2f99375d9693e706b4818c72caad13 Mon Sep 17 00:00:00 2001 From: Anton Standrik Date: Sat, 26 Sep 2026 14:48:53 +0300 Subject: [PATCH 2/2] fix(mcp): use the shared test classifier in trace output Review on #2294 asked not to add a third test-path rule. trace_node_is_test() now asks cbm_is_test_path(), the rule that TESTS edges and importance already use, and still honours a stored boolean is_test=true. The prototype moves from pipeline_internal.h to pipeline.h, next to cbm_parse_hunks, so mcp.c can call it. The regression table follows the shared rule. Java *Test.java and Ruby *_spec.rb rows are added: the removed matcher kept them visible. Rows only that matcher handled are dropped: backslash paths, _test.c and my__tests__/. Refs #1593 Co-Authored-By: Claude Fable 5.1 Signed-off-by: Anton Standrik --- src/mcp/mcp.c | 25 +++---------------------- src/pipeline/pipeline.h | 8 ++++++++ src/pipeline/pipeline_internal.h | 3 --- tests/test_mcp.c | 6 ++---- 4 files changed, 13 insertions(+), 29 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 479f2df7c5..935c439a7a 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -8069,29 +8069,10 @@ static yyjson_doc *resolve_trace_edge_types(const char *args, const char *mode, return NULL; } -/* Path rules also cover legacy nodes with missing or false is_test metadata. */ +/* Same rule as TESTS edges and importance, plus the node's is_test property. */ static bool trace_node_is_test(const cbm_node_t *node) { - const char *base = node->file_path; - if (base) { - for (const char *end = base; *end; end++) { - if (*end != '/' && *end != '\\') { - continue; - } - size_t length = (size_t)(end - base); - if ((length == SLEN("test") && strncmp(base, "test", length) == 0) || - (length == SLEN("tests") && strncmp(base, "tests", length) == 0) || - (length == SLEN("spec") && strncmp(base, "spec", length) == 0) || - (length == SLEN("__tests__") && strncmp(base, "__tests__", length) == 0)) { - return true; - } - base = end + SKIP_ONE; - } - if (strncmp(base, "test_", SLEN("test_")) == 0 || strstr(base, "_test.") || - strstr(base, ".test.") || strstr(base, ".spec.")) { - return true; - } - } - return node->properties_json && cbm_mcp_get_bool_arg(node->properties_json, "is_test"); + return cbm_is_test_path(node->file_path) || + (node->properties_json && cbm_mcp_get_bool_arg(node->properties_json, "is_test")); } /* Filtering belongs before page-window calculation: hidden test rows must not diff --git a/src/pipeline/pipeline.h b/src/pipeline/pipeline.h index e5a672cb0b..1dc5e5bb4e 100644 --- a/src/pipeline/pipeline.h +++ b/src/pipeline/pipeline.h @@ -395,4 +395,12 @@ typedef struct { * Returns count written to out (capped at max_out). */ int cbm_parse_hunks(const char *output, cbm_changed_hunk_t *out, int max_out); +/* ── Test-path classifier (pass_tests.c) ────────────────────────── + * Public (unlike the rest of pipeline_internal.h) because trace_path + * (src/mcp/mcp.c) hides and marks test nodes with the same rule that TESTS + * edges and importance use. */ + +/* Check if a file path looks like a test file (language-agnostic). */ +bool cbm_is_test_path(const char *path); + #endif /* CBM_PIPELINE_H */ diff --git a/src/pipeline/pipeline_internal.h b/src/pipeline/pipeline_internal.h index e7782adeff..cd1f29fc91 100644 --- a/src/pipeline/pipeline_internal.h +++ b/src/pipeline/pipeline_internal.h @@ -285,9 +285,6 @@ bool cbm_import_symbol_fallback_allowed(CBMLanguage lang); /* Check if a file path is worth tracking for git history analysis. */ bool cbm_is_trackable_file(const char *path); -/* Check if a file path looks like a test file (language-agnostic). */ -bool cbm_is_test_path(const char *path); - /* Check if a function name looks like a test function (language-agnostic). */ bool cbm_is_test_func_name(const char *name); diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 67fb521232..84eefb5c81 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -2784,14 +2784,12 @@ TEST(tool_trace_test_classification_issue1593) { {"app/test/nested.ts", NULL, true}, {"app/spec/nested.ts", NULL, true}, {"app/__tests__/nested.ts", NULL, true}, - {"tests\\root.ts", NULL, true}, - {"app\\__tests__\\nested.ts", NULL, true}, {"src/test_core.py", NULL, true}, - {"src/core_test.c", NULL, true}, + {"src/FooTest.java", NULL, true}, + {"lib/user_spec.rb", NULL, true}, {"src/main.ts", NULL, false}, {"src/testimonials.ts", NULL, false}, {"src/test_helpers/main.ts", NULL, false}, - {"src/my__tests__/main.ts", NULL, false}, {"src/contest.ts", NULL, false}, {"src/inline.rs", "{\"is_test\":true}", true}, {"src/inline.rs", "{ \"is_test\" : true }", true},