diff --git a/src/cli/cli.c b/src/cli/cli.c index 1428e13ef..afd45b405 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -554,12 +554,16 @@ static const char skill_content[] = "MATCH (f:Function) WHERE f.name =~ '.*Handler.*' RETURN f.name, f.file_path\n" "MATCH (a)-[r:CALLS]->(b) WHERE a.name = 'main' RETURN b.name\n" "```\n" + "Examples are non-exhaustive guidance. Write problem-specific Cypher for effective, " + "computationally efficient results; explicit labels/properties and exploratory LIMIT are " + "optional optimizations.\n" "\n" "## Gotchas\n" "1. `search_graph(relationship=\"HTTP_CALLS\")` filters nodes by degree — " "use `query_graph` with Cypher to see actual edges.\n" - "2. `query_graph` output is capped by query_max_output_bytes; add LIMIT or set " - "max_output_bytes=0.\n" + "2. `query_graph` output is capped by query_max_output_bytes. Use LIMIT when it helps query " + "efficiency during exploration; omit it when the full result is necessary, and set " + "max_output_bytes=0 only when uncapped output is appropriate.\n" "3. `trace_path` works best with exact names — use `search_graph(name_pattern=...)` first.\n" "4. `direction=\"outbound\"` returns callees only; use `direction=\"both\"` for callers too.\n" "5. Results default to search_limit (50 unless configured); check `has_more` and use " @@ -576,11 +580,17 @@ static const char codex_instructions_content[] = "auto_index_limit\n" "- `trace_path` — trace who calls a function or what it calls\n" "- `get_code` — read function source code by qualified_name\n" - "- `query_graph` — run Cypher queries for complex patterns\n" + "- `query_graph` — write problem-specific Cypher for effective, computationally efficient " + "structural answers; examples and LIMIT are optional guidance\n" "- `get_architecture` — high-level summary after `_hidden_tools` reveal or " "`CBM_TOOL_MODE=classic`\n" "\n" - "Prefer graph tools over grep for structural code discovery.\n"; + "Prefer graph tools over grep for structural code discovery.\n" + "\n" + "If a sandbox blocks an MCP or CLI operation because it crosses a shell or filesystem " + "boundary, " + "retry that operation with escalation instead of treating the block as a project failure. " + "MCP approval and shell sandbox authorization are separate.\n"; /* Old skill names — cleaned up during install to remove stale directories. */ static const char *old_skill_names[] = { @@ -1392,7 +1402,8 @@ static const char agent_instructions_content[] = "1. `search_graph` — find functions, classes, routes, variables by pattern\n" "2. `trace_path` — trace who calls a function or what it calls\n" "3. `get_code` — read specific function/class source code by qualified_name\n" - "4. `query_graph` — run Cypher queries for complex patterns\n" + "4. `query_graph` — write problem-specific Cypher for effective, computationally efficient " + "structural answers; examples and LIMIT are optional guidance\n" "5. `get_architecture` — high-level summary after `_hidden_tools` reveal or " "`CBM_TOOL_MODE=classic`\n" "\n" diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index df974378b..eb43530d9 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -1117,21 +1117,26 @@ static const tool_def_t TOOLS[] = { "signature. Internal fp/sp/bt indexing fields are never returned.\"}}}"}, {"query_graph", "Query graph", - "Execute a Cypher query against the knowledge graph for complex multi-hop patterns, " - "aggregations, and cross-service analysis. Row scan and output bytes are capped by default " - "(config keys query_max_rows and query_max_output_bytes). Set max_output_bytes=0 for " - "unlimited output bytes or add LIMIT. " - "Dependency sub-project symbols (proj.dep.*) are tagged source:dependency; to rank your own " - "project's symbols above them, ORDER BY CASE WHEN n.project LIKE '%.dep.%' THEN 1 ELSE 0 END.", + "Core compositional tool; use it early for bespoke structural questions. Create new, " + "effective, computationally efficient custom Cypher for multi-hop paths, aggregates/hotspots, " + "arbitrary predicates, cross-service links, or graph=\"missed\". Non-exhaustive examples: " + "MATCH (f:Function)-[:CALLS]->(g) RETURN f.name,g.name LIMIT 20; " + "MATCH (f:File) RETURN f.file_path,count(*). Any valid query shape is allowed; explicit " + "labels/properties and LIMIT are optional efficiency aids. Server caps query_max_rows and " + "query_max_output_bytes; raise only when needed. Dependency symbols use proj.dep.* and " + "source:dependency; rank primary symbols first with " + "ORDER BY CASE WHEN n.project LIKE '%.dep.%' THEN 1 ELSE 0 END.", "{\"type\":\"object\",\"properties\":{\"query\":{\"type\":\"string\",\"description\":\"Cypher " "query\"},\"project\":{\"type\":\"string\",\"description\":\"Indexed project name. Omit to " "use the MCP server project derived from server CWD.\"},\"max_rows\":{\"type\":\"integer\"," "\"description\":\"Scan-level row limit. Omit to use query_max_rows config. Set 0 to use " "the implementation ceiling. Note: limits nodes scanned, not rows returned. For output size, " - "use max_output_bytes or add LIMIT to your Cypher query.\"},\"max_output_bytes\":{\"type\":" + "set max_output_bytes; an optional Cypher LIMIT can reduce computation and output.\"}," + "\"max_output_bytes\":{\"type\":" "\"integer\",\"description\":\"Max response size in bytes (configurable via " "query_max_output_bytes config key). Set to 0 for unlimited. When exceeded, returns " - "truncated=true with total_bytes and hint to add LIMIT.\"},\"graph\":{\"type\":\"string\"," + "truncated=true with total_bytes and optional ways to narrow the query or raise the cap.\"}," + "\"graph\":{\"type\":\"string\"," "\"enum\":[\"code\",\"missed\"],\"default\":\"code\",\"description\":\"Query the code " "graph or the best-effort graph of files not fully indexed.\"},\"format\":{\"type\":\"string\"," "\"enum\":[\"toon\",\"json\"],\"default\":\"toon\"}},\"required\":[\"query\"]}"}, @@ -1398,6 +1403,12 @@ static void emit_tool(yyjson_mut_doc *doc, yyjson_mut_val *tools, const tool_def } static bool is_streamlined_default_tool(const char *name) { + /* query_graph is intentionally a core tool, not merely an advanced escape + * hatch. It composes labels, properties, relationships, predicates and + * aggregations into new solutions without requiring a dedicated MCP + * tool/schema for every structural code question. Its description asks + * callers to create effective, computationally efficient custom Cypher; + * examples and optimization guidance are explicitly non-binding. */ return name && (strcmp(name, "search_graph") == 0 || strcmp(name, "query_graph") == 0 || strcmp(name, "search_code") == 0 || @@ -5216,13 +5227,21 @@ static char *semantic_query_type_error_response(void) { } static bool run_semantic_query(yyjson_mut_doc *doc, yyjson_mut_val *root, const char *args, - cbm_store_t *store, const char *project, int limit) { + cbm_store_t *store, const char *project, int limit, + bool *out_present, int *out_count) { cbm_vector_result_t *vresults = NULL; int vcount = 0; + bool present = false; bool type_error = - run_semantic_query_core(args, store, project, limit, &vresults, &vcount, NULL); - if (project && cbm_store_derived_view_is_stale( - store, project, CBM_STORE_DERIVED_VIEW_SEMANTIC_EDGES)) { + run_semantic_query_core(args, store, project, limit, &vresults, &vcount, &present); + if (out_present) { + *out_present = present; + } + if (out_count) { + *out_count = vcount; + } + if (present && project && + cbm_store_derived_view_is_stale(store, project, CBM_STORE_DERIVED_VIEW_SEMANTIC_EDGES)) { add_stale_derived_view_warning( doc, root, CBM_STORE_DERIVED_VIEW_SEMANTIC_EDGES, "semantic_edges derived view is stale; semantic_results may be stale."); @@ -5384,7 +5403,7 @@ static char *append_semantic_query_to_json(const char *base_json, const char *ar } yyjson_mut_doc_set_root(mdoc, root); - bool sq_type_error = run_semantic_query(mdoc, root, args, store, project, limit); + bool sq_type_error = run_semantic_query(mdoc, root, args, store, project, limit, NULL, NULL); if (sq_type_error) { if (type_error) { *type_error = true; @@ -5831,6 +5850,15 @@ static char *handle_search_graph(cbm_mcp_server_t *srv, const char *args) { true); } + /* A semantic-only request must not silently prepend an unrelated unfiltered + * graph search. Besides misleading callers, that legacy JSON behavior added + * an unnecessary O(graph-result-page) scan to the requested vector lookup. + * This mirrors the compact TOON path above. */ + bool has_graph_filters = label || name_pattern || qn_pattern || unified_pattern || + file_pattern || relationship || exclude_entry_points || + min_degree != CBM_NOT_FOUND || max_degree != CBM_NOT_FOUND; + bool semantic_only = cbm_mcp_has_arg(args, "semantic_query") && !has_graph_filters; + cbm_search_output_t out = {0}; cbm_store_overlay_node_view_summary_t overlay_summary = {0}; bool overlay_ready_for_nodes = @@ -5844,10 +5872,12 @@ static char *handle_search_graph(cbm_mcp_server_t *srv, const char *args) { (sort_by && (strcmp(sort_by, "degree") == 0 || strcmp(sort_by, "calls") == 0 || strcmp(sort_by, "linkrank") == 0)); bool overlay_search_used = overlay_ready_for_nodes; - if (overlay_search_used) { - cbm_store_search_overlay_view(store, ¶ms, &out); - } else { - cbm_store_search(store, ¶ms, &out); + if (!semantic_only) { + if (overlay_search_used) { + cbm_store_search_overlay_view(store, ¶ms, &out); + } else { + cbm_store_search(store, ¶ms, &out); + } } yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL); @@ -5986,7 +6016,10 @@ static char *handle_search_graph(cbm_mcp_server_t *srv, const char *args) { /* Semantic (vector) search: append semantic_results if semantic_query * array was provided. Returns true on type error (non-array value). */ - bool sq_type_error = run_semantic_query(doc, root, args, store, project, limit); + bool sq_present = false; + int semantic_count = 0; + bool sq_type_error = + run_semantic_query(doc, root, args, store, project, limit, &sq_present, &semantic_count); if (sq_type_error) { for (int pi = 0; pi < props_doc_count; pi++) { yyjson_doc_free(props_docs[pi]); @@ -6000,6 +6033,12 @@ static char *handle_search_graph(cbm_mcp_server_t *srv, const char *args) { free_string_array(exclude); return semantic_query_type_error_response(); } + if (semantic_only && sq_present && semantic_count == 0) { + yyjson_mut_obj_add_val(doc, root, "semantic_results", yyjson_mut_arr(doc)); + yyjson_mut_obj_add_str(doc, root, "hint", + "No semantic matches. semantic_query needs a moderate/full index; " + "try broader or fewer keywords."); + } char *json = yy_doc_to_str(doc); /* Property docs are zero-copy referenced by the mut doc — they must @@ -6226,7 +6265,8 @@ static char *handle_query_graph(cbm_mcp_server_t *srv, const char *args) { snprintf(trunc_json, sizeof(trunc_json), "{\"truncated\":true,\"total_bytes\":%lu," "\"rows_returned\":%d," - "\"hint\":\"Add LIMIT to your Cypher query\"}", + "\"hint\":\"Narrow returned fields, add LIMIT when appropriate, or raise " + "max_output_bytes\"}", (unsigned long)json_len, total_rows); char *result_text = cbm_mcp_text_result(trunc_json, false); free(json); diff --git a/tests/test_cli.c b/tests/test_cli.c index 06864f4c4..307d38433 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -730,6 +730,10 @@ TEST(cli_codex_instructions) { ASSERT_NOT_NULL(instr); ASSERT(strstr(instr, "Codebase Knowledge Graph") != NULL); ASSERT(strstr(instr, "trace_path") != NULL); + ASSERT(strstr(instr, "effective, computationally efficient") != NULL); + ASSERT(strstr(instr, "examples and LIMIT are optional guidance") != NULL); + ASSERT(strstr(instr, "retry that operation with escalation") != NULL); + ASSERT(strstr(instr, "MCP approval and shell sandbox authorization are separate") != NULL); PASS(); } diff --git a/tests/test_mcp.c b/tests/test_mcp.c index e64544d52..09f34aed4 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -2445,6 +2445,56 @@ TEST(tool_search_graph_semantic_query_warns_on_stale_semantic_view) { PASS(); } +TEST(tool_search_graph_semantic_only_json_does_not_return_unfiltered_nodes) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *st = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(st); + + const char *proj = "semantic-only-json"; + ASSERT_EQ(cbm_store_upsert_project(st, proj, "/tmp/semantic-only-json"), CBM_STORE_OK); + cbm_mcp_server_set_project(srv, proj); + + /* A graph node without a semantic vector proves the handler does not + * silently substitute an unrelated unfiltered graph search when the + * semantic-only request has no matches. */ + cbm_node_t unrelated = {.project = proj, + .label = "Function", + .name = "unrelated_ranked_function", + .qualified_name = "semantic.only.unrelated_ranked_function", + .file_path = "src/unrelated.c"}; + ASSERT_GT(cbm_store_upsert_node(st, &unrelated), 0); + + char *resp = cbm_mcp_server_handle( + srv, "{\"jsonrpc\":\"2.0\",\"id\":481,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"search_graph\"," + "\"arguments\":{\"project\":\"semantic-only-json\"," + "\"semantic_query\":[\"transport\",\"lifecycle\"],\"format\":\"json\"}}}"); + ASSERT_NOT_NULL(resp); + char *inner = extract_text_content(resp); + ASSERT_NOT_NULL(inner); + + yyjson_doc *doc = yyjson_read(inner, strlen(inner), 0); + ASSERT_NOT_NULL(doc); + yyjson_val *root = yyjson_doc_get_root(doc); + yyjson_val *results = yyjson_obj_get(root, "results"); + yyjson_val *semantic_results = yyjson_obj_get(root, "semantic_results"); + ASSERT_NOT_NULL(results); + ASSERT_TRUE(yyjson_is_arr(results)); + ASSERT_EQ(yyjson_arr_size(results), 0); + ASSERT_NOT_NULL(semantic_results); + ASSERT_TRUE(yyjson_is_arr(semantic_results)); + ASSERT_EQ(yyjson_arr_size(semantic_results), 0); + ASSERT_NOT_NULL(yyjson_obj_get(root, "hint")); + ASSERT_NULL(strstr(inner, "unrelated_ranked_function")); + + yyjson_doc_free(doc); + free(inner); + free(resp); + cbm_mcp_server_free(srv); + PASS(); +} + /* MCP discovery probes must return valid lists, not -32601 Method-not-found: * clients like Cline call these on connect and * resources/list + prompts/list + resources/templates/list on connect and @@ -10077,6 +10127,7 @@ SUITE(mcp) { RUN_TEST(tool_search_graph_query_uses_search_limit_config); RUN_TEST(tool_search_graph_query_rejects_bad_semantic_query); RUN_TEST(tool_search_graph_semantic_query_warns_on_stale_semantic_view); + RUN_TEST(tool_search_graph_semantic_only_json_does_not_return_unfiltered_nodes); RUN_TEST(tool_search_graph_blocks_internal_fields_and_compacts_json_properties); RUN_TEST(tool_output_byte_budgets); RUN_TEST(mcp_discovery_methods_return_supported_lists); diff --git a/tests/test_token_reduction.c b/tests/test_token_reduction.c index d55d58d45..e2279e2a5 100644 --- a/tests/test_token_reduction.c +++ b/tests/test_token_reduction.c @@ -771,6 +771,8 @@ TEST(query_graph_max_output_bytes_truncates) { /* Response should indicate truncation */ ASSERT_NOT_NULL(strstr(resp, "\"truncated\":true")); + ASSERT_NOT_NULL(strstr(resp, "Narrow returned fields, add LIMIT when appropriate, or raise " + "max_output_bytes")); /* Response body should be near the byte limit */ ASSERT_TRUE(strlen(resp) <= 2048); /* some slack for metadata */ diff --git a/tests/test_tool_consolidation.c b/tests/test_tool_consolidation.c index 151946090..9f9bee8ce 100644 --- a/tests/test_tool_consolidation.c +++ b/tests/test_tool_consolidation.c @@ -544,6 +544,36 @@ TEST(default_tool_autoindex_description_is_precise) { PASS(); } +TEST(query_graph_description_explains_compositional_value) { + char *streamlined = cbm_mcp_tools_list(NULL); + ASSERT_NOT_NULL(streamlined); + ASSERT_NOT_NULL(strstr(streamlined, "Core compositional tool; use it early")); + ASSERT_NOT_NULL( + strstr(streamlined, "Create new, effective, computationally efficient custom Cypher")); + ASSERT_NOT_NULL(strstr(streamlined, "Non-exhaustive examples")); + ASSERT_NOT_NULL(strstr(streamlined, "Any valid query shape is allowed")); + ASSERT_NOT_NULL(strstr(streamlined, "multi-hop paths")); + ASSERT_NOT_NULL(strstr(streamlined, "aggregates/hotspots")); + ASSERT_NOT_NULL(strstr(streamlined, "LIMIT are optional efficiency aids")); + free(streamlined); + + /* query_graph is serialized from the same canonical definition in both + * modes; this guards the user-facing value contract as well as schema parity. */ + char *saved_mode = save_tool_mode(); + cbm_setenv("CBM_TOOL_MODE", "classic", 1); + char *classic = cbm_mcp_tools_list(NULL); + restore_tool_mode(saved_mode); + ASSERT_NOT_NULL(classic); + ASSERT_NOT_NULL(strstr(classic, "Core compositional tool; use it early")); + ASSERT_NOT_NULL( + strstr(classic, "Create new, effective, computationally efficient custom Cypher")); + ASSERT_NOT_NULL(strstr(classic, "Non-exhaustive examples")); + ASSERT_NOT_NULL(strstr(classic, "Any valid query shape is allowed")); + free(classic); + + PASS(); +} + TEST(revealed_trace_path_parameter_contract) { char *saved_mode = save_tool_mode(); cbm_unsetenv("CBM_TOOL_MODE"); @@ -2918,6 +2948,7 @@ SUITE(tool_consolidation) { RUN_TEST(streamlined_reveal_covers_classic_capabilities); RUN_TEST(streamlined_core_parameter_contract); RUN_TEST(default_tool_autoindex_description_is_precise); + RUN_TEST(query_graph_description_explains_compositional_value); RUN_TEST(revealed_trace_path_parameter_contract); RUN_TEST(revealed_advanced_tool_schema_matches_handlers); /* Dispatch */