From be82cacbe2f0f02734ec780f5b605ea66e44189f Mon Sep 17 00:00:00 2001 From: Andrew Hundt Date: Fri, 26 Jun 2026 19:08:19 -0400 Subject: [PATCH] feat(search): rank project symbols above dependencies; make it tunable The dep-last tiebreak in cbm_store_search only fired for glob searches (params->project_pattern), so the common prefix-match path (params->project, which includes proj.dep.*) let a dependency symbol outrank the project's own symbol of the same name -- the "Path" concern (a stdlib Path fronting the user's own Path). - store.c: hoist dep-last into one clause applied consistently across ALL sort modes (relevance/degree/calls/linkrank/name) whenever a search is scoped to a project; gated on the new param. - store.h: cbm_search_params_t::disable_dep_ranking (default false = deps rank last; inverted sense so {0}-init preserves the safe default). - mcp.c: search_disable_dep_ranking config key (bool, default false) wired through srv->config and documented in the search_graph schema, following the existing CBM_CONFIG_* pattern. - test_tool_consolidation.c: store_prefix_ranks_project_above_dep (default ranks project above dep) and store_prefix_disable_dep_ranking_lets_dep_win (pure relevance when disabled). Green: tool_consolidation 87 passed. Full suite unchanged except the 2 known B1 incremental failures. Signed-off-by: Andrew Hundt --- src/mcp/mcp.c | 13 ++++- src/store/store.c | 71 +++++++++++++------------ src/store/store.h | 6 +++ tests/test_tool_consolidation.c | 91 +++++++++++++++++++++++++++++++++ 4 files changed, 146 insertions(+), 35 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 4e562d1a4..2a8b0a84c 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -99,6 +99,11 @@ static void add_pagerank_val(yyjson_mut_doc *doc, yyjson_mut_val *obj, double v) #define CBM_DEFAULT_SEARCH_LIMIT 50 #define CBM_CONFIG_SEARCH_LIMIT "search_limit" +/* Default: rank dependency sub-project symbols (proj.dep.*) LAST so a stdlib + * symbol like 'Path' never fronts the user's own 'Path'. Tunable off via config + * key "search_disable_dep_ranking" (true = pure relevance, deps may rank high). */ +#define CBM_CONFIG_SEARCH_DISABLE_DEP_RANKING "search_disable_dep_ranking" + /* Default max source lines returned by get_code_snippet. * Set to 0 for unlimited. Prevents huge functions from consuming tokens. * Configurable via config key "snippet_max_lines". */ @@ -371,7 +376,8 @@ static const tool_def_t TOOLS[] = { "label/file_path='', degree=0. Saves tokens.\"}," "\"include_dependencies\":{\"type\":\"boolean\",\"default\":true,\"description\":\"Include " "symbols from dependency sub-projects (marked source=dependency in results). Set false to " - "scope to project code only.\"}," + "scope to project code only. When true, project symbols rank above dependency symbols by " + "default (config key search_disable_dep_ranking=true reverts to pure relevance).\"}," "\"exclude\":{\"type\":\"array\",\"items\":{\"type\":\"string\"},\"description\":\"Glob " "patterns for file paths to exclude from results (e.g. [\\\"tests/**\\\",\\\"scripts/**\\\"])." "\"}}}"}, @@ -2724,6 +2730,11 @@ static char *handle_search_graph(cbm_mcp_server_t *srv, const char *args) { params.degree_mode = srv->config ? cbm_config_get(srv->config, "degree_mode", NULL) : NULL; + /* Dep-ranking: default false (deps rank last). Config key + * search_disable_dep_ranking=true → pure relevance (deps may rank high). */ + params.disable_dep_ranking = srv->config + ? cbm_config_get_bool(srv->config, CBM_CONFIG_SEARCH_DISABLE_DEP_RANKING, false) + : false; params.limit = effective_limit; params.offset = offset; params.min_degree = min_degree; diff --git a/src/store/store.c b/src/store/store.c index b26dfd052..755a75b34 100644 --- a/src/store/store.c +++ b/src/store/store.c @@ -2693,61 +2693,64 @@ int cbm_store_search(cbm_store_t *s, const cbm_search_params_t *params, cbm_sear const char *id_col = has_degree_filter ? "id" : "n.id"; const char *pr_col = has_degree_filter ? "pr_rank" : "pr_rank"; char order_limit[CBM_SZ_256]; + + /* Dep-last: rank the project's own symbols above dependency sub-project + * symbols (proj.dep.*) whenever a search is scoped to a project (prefix + * match OR glob). Applied as a tiebreak right after the primary metric + * across ALL sort modes, so a stdlib symbol like 'Path' never outranks the + * user's own 'Path' on equal merit (#18, the Path concern). Previously this + * only fired for glob (project_pattern) searches; the common prefix path + * (params->project) omitted it, letting deps win the name/id tiebreak. + * Empty string when unscoped (no project) — preserves prior behavior. */ + const char *proj_col = has_degree_filter ? "project" : "n.project"; + bool scope_has_project = (params->project != NULL || params->project_pattern != NULL); + char dep_last[CBM_SZ_128]; + if (scope_has_project && !params->disable_dep_ranking) { + snprintf(dep_last, sizeof(dep_last), + "CASE WHEN %s LIKE '%%.dep.%%' THEN 1 ELSE 0 END, ", proj_col); + } else { + dep_last[0] = '\0'; + } + if (use_pagerank) { - /* Relevance sort: PageRank DESC, then dep-last, then name for stability */ - if (params->project_pattern) { - const char *proj_col = has_degree_filter ? "project" : "n.project"; - snprintf(order_limit, sizeof(order_limit), - " ORDER BY %s DESC, " - "CASE WHEN %s LIKE '%%.dep.%%' THEN 1 ELSE 0 END, %s, %s" - " LIMIT %d OFFSET %d", - pr_col, proj_col, name_col, id_col, limit, offset); - } else { - snprintf(order_limit, sizeof(order_limit), - " ORDER BY %s DESC, %s, %s LIMIT %d OFFSET %d", - pr_col, name_col, id_col, limit, offset); - } + /* Relevance sort: PageRank DESC, dep-last, name, id (stable pagination). */ + snprintf(order_limit, sizeof(order_limit), + " ORDER BY %s DESC, %s%s, %s LIMIT %d OFFSET %d", + pr_col, dep_last, name_col, id_col, limit, offset); } else if (params->sort_by && strcmp(params->sort_by, "degree") == 0) { snprintf(order_limit, sizeof(order_limit), - " ORDER BY (in_deg + out_deg) DESC, %s, %s LIMIT %d OFFSET %d", - name_col, id_col, limit, offset); + " ORDER BY (in_deg + out_deg) DESC, %s%s, %s LIMIT %d OFFSET %d", + dep_last, name_col, id_col, limit, offset); } else if (params->sort_by && strcmp(params->sort_by, "calls") == 0) { if (has_degree_table && !has_degree_filter) { /* nd.* only accessible when not wrapped by the degree-filter subquery */ snprintf(order_limit, sizeof(order_limit), - " ORDER BY COALESCE(nd.calls_in + nd.calls_out, 0) DESC, %s, %s" + " ORDER BY COALESCE(nd.calls_in + nd.calls_out, 0) DESC, %s%s, %s" " LIMIT %d OFFSET %d", - name_col, id_col, limit, offset); + dep_last, name_col, id_col, limit, offset); } else { /* Fallback: no precomputed calls data, or query wrapped by degree * filter (nd alias out of scope) — use total degree */ snprintf(order_limit, sizeof(order_limit), - " ORDER BY (in_deg + out_deg) DESC, %s, %s LIMIT %d OFFSET %d", - name_col, id_col, limit, offset); + " ORDER BY (in_deg + out_deg) DESC, %s%s, %s LIMIT %d OFFSET %d", + dep_last, name_col, id_col, limit, offset); } } else if (params->sort_by && strcmp(params->sort_by, "linkrank") == 0) { if (has_degree_table && !has_degree_filter) { snprintf(order_limit, sizeof(order_limit), - " ORDER BY COALESCE(nd.linkrank_in, 0) DESC, %s, %s LIMIT %d OFFSET %d", - name_col, id_col, limit, offset); + " ORDER BY COALESCE(nd.linkrank_in, 0) DESC, %s%s, %s LIMIT %d OFFSET %d", + dep_last, name_col, id_col, limit, offset); } else { /* Fallback: no precomputed linkrank — use total degree */ snprintf(order_limit, sizeof(order_limit), - " ORDER BY (in_deg + out_deg) DESC, %s, %s LIMIT %d OFFSET %d", - name_col, id_col, limit, offset); + " ORDER BY (in_deg + out_deg) DESC, %s%s, %s LIMIT %d OFFSET %d", + dep_last, name_col, id_col, limit, offset); } } else { - /* name sort (explicit or fallback) */ - if (params->project_pattern) { - const char *proj_col = has_degree_filter ? "project" : "n.project"; - snprintf(order_limit, sizeof(order_limit), - " ORDER BY CASE WHEN %s LIKE '%%.dep.%%' THEN 1 ELSE 0 END, %s, %s" - " LIMIT %d OFFSET %d", - proj_col, name_col, id_col, limit, offset); - } else { - snprintf(order_limit, sizeof(order_limit), " ORDER BY %s, %s LIMIT %d OFFSET %d", - name_col, id_col, limit, offset); - } + /* name sort (explicit or fallback): dep-last, name, id. */ + snprintf(order_limit, sizeof(order_limit), + " ORDER BY %s%s, %s LIMIT %d OFFSET %d", + dep_last, name_col, id_col, limit, offset); } strncat(sql, order_limit, sizeof(sql) - strlen(sql) - 1); diff --git a/src/store/store.h b/src/store/store.h index 5a173db6f..99a3901f6 100644 --- a/src/store/store.h +++ b/src/store/store.h @@ -122,6 +122,12 @@ typedef struct { const char *sort_by; /* "relevance" / "name" / "degree" / "calls" / "linkrank", NULL = relevance */ const char *degree_mode; /* "weighted" / "unweighted" / "calls_only", NULL = unweighted */ bool case_sensitive; + /* Ranking: when true, dependency sub-project symbols (proj.dep.*) are NOT + * demoted below the project's own symbols — pure relevance order applies. + * Default false (zero-init): deps rank LAST so a stdlib symbol like 'Path' + * never fronts the user's own 'Path'. Tunable via MCP config key + * "search_disable_dep_ranking" (search_graph). */ + bool disable_dep_ranking; const char **exclude_labels; /* NULL-terminated array, or NULL */ const char **exclude_paths; /* NULL-terminated array of glob patterns to exclude by file_path */ } cbm_search_params_t; diff --git a/tests/test_tool_consolidation.c b/tests/test_tool_consolidation.c index c6ebf8fb8..e74cf5576 100644 --- a/tests/test_tool_consolidation.c +++ b/tests/test_tool_consolidation.c @@ -1051,6 +1051,95 @@ TEST(store_prefix_match_includes_deps) { PASS(); } +/* #18 RED: project-over-dependency source ranking in the COMMON prefix-match + * path (project="myapp", which includes myapp.dep.*). A dependency symbol must + * NOT outrank the project's own symbol of the same name — the "Path" concern + * (a python-stdlib Path must not be front-of-line over the user's own Path). + * + * The store already has a dep-last tiebreak, but it ONLY fires for + * params->project_pattern (glob). The common prefix path sets params->project, + * so today deps are NOT demoted here → a dep inserted first (lower id) wins the + * name/id tiebreak and ranks above the project symbol. This test must FAIL until + * dep-last is extended to the prefix-match path. */ +TEST(store_prefix_ranks_project_above_dep) { + cbm_store_t *s = cbm_store_open_memory(); + ASSERT_NOT_NULL(s); + cbm_store_upsert_project(s, "myapp", "/tmp/myapp"); + cbm_store_upsert_project(s, "myapp.dep.stdlib", "/tmp/stdlib"); + + /* Insert the DEPENDENCY symbol FIRST so it gets the lower id — under the + * current (no-dep-last-in-prefix-path) ordering it wins the id tiebreak. */ + cbm_node_t nd = {.project = "myapp.dep.stdlib", .label = "Class", .name = "Path", + .qualified_name = "myapp.dep.stdlib.Path", .file_path = "stdlib/path.py"}; + cbm_store_upsert_node(s, &nd); + cbm_node_t np = {.project = "myapp", .label = "Class", .name = "Path", + .qualified_name = "myapp.Path", .file_path = "src/path.py"}; + cbm_store_upsert_node(s, &np); + + cbm_search_params_t params = {0}; + params.project = "myapp"; /* prefix match: includes myapp.dep.* */ + params.limit = 10; + cbm_search_output_t out = {0}; + cbm_store_search(s, ¶ms, &out); + ASSERT_GTE(out.count, 2); + + /* The PROJECT 'Path' must rank above the DEPENDENCY 'Path'. */ + int proj_idx = -1, dep_idx = -1; + for (int i = 0; i < out.count; i++) { + if (strcmp(out.results[i].node.name, "Path") != 0) continue; + if (strcmp(out.results[i].node.project, "myapp") == 0) proj_idx = i; + if (strcmp(out.results[i].node.project, "myapp.dep.stdlib") == 0) dep_idx = i; + } + ASSERT_NEQ(proj_idx, -1); + ASSERT_NEQ(dep_idx, -1); + ASSERT_TRUE(proj_idx < dep_idx); /* project before dependency */ + + cbm_store_search_free(&out); + cbm_store_close(s); + PASS(); +} + +/* #38: the dep-last ranking is tunable. With disable_dep_ranking=true the store + * applies PURE relevance order — a dep symbol may rank above the project's own. + * This makes the ranking a parameter (config key search_disable_dep_ranking) + * rather than a hard-coded decision, per the project's meta-param conventions. */ +TEST(store_prefix_disable_dep_ranking_lets_dep_win) { + cbm_store_t *s = cbm_store_open_memory(); + ASSERT_NOT_NULL(s); + cbm_store_upsert_project(s, "myapp", "/tmp/myapp"); + cbm_store_upsert_project(s, "myapp.dep.stdlib", "/tmp/stdlib"); + cbm_node_t nd = {.project = "myapp.dep.stdlib", .label = "Class", .name = "Path", + .qualified_name = "myapp.dep.stdlib.Path", .file_path = "stdlib/path.py"}; + cbm_store_upsert_node(s, &nd); + cbm_node_t np = {.project = "myapp", .label = "Class", .name = "Path", + .qualified_name = "myapp.Path", .file_path = "src/path.py"}; + cbm_store_upsert_node(s, &np); + + cbm_search_params_t params = {0}; + params.project = "myapp"; + params.disable_dep_ranking = true; /* pure relevance — no dep demotion */ + params.limit = 10; + cbm_search_output_t out = {0}; + cbm_store_search(s, ¶ms, &out); + ASSERT_GTE(out.count, 2); + + /* With dep-ranking disabled and equal relevance, the dep (lower id, inserted + * first) wins the id tiebreak → ranks above the project symbol. */ + int proj_idx = -1, dep_idx = -1; + for (int i = 0; i < out.count; i++) { + if (strcmp(out.results[i].node.name, "Path") != 0) continue; + if (strcmp(out.results[i].node.project, "myapp") == 0) proj_idx = i; + if (strcmp(out.results[i].node.project, "myapp.dep.stdlib") == 0) dep_idx = i; + } + ASSERT_NEQ(proj_idx, -1); + ASSERT_NEQ(dep_idx, -1); + ASSERT_TRUE(dep_idx < proj_idx); /* dep before project (ranking disabled) */ + + cbm_store_search_free(&out); + cbm_store_close(s); + PASS(); +} + /* Bug 2 complement: exact match should NOT include deps. */ TEST(store_exact_match_excludes_deps) { cbm_store_t *s = cbm_store_open_memory(); @@ -2095,6 +2184,8 @@ SUITE(tool_consolidation) { /* Dep search bug regressions */ RUN_TEST(dep_search_explicit_dep_project_name); RUN_TEST(store_prefix_match_includes_deps); + RUN_TEST(store_prefix_ranks_project_above_dep); + RUN_TEST(store_prefix_disable_dep_ranking_lets_dep_win); RUN_TEST(store_exact_match_excludes_deps); RUN_TEST(is_dep_project_cross_project_detection); RUN_TEST(e2e_dep_search_returns_project_and_dep_results);