mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-08 08:53:43 +08:00
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 <ATHundt@gmail.com>
This commit is contained in:
+12
-1
@@ -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;
|
||||
|
||||
+37
-34
@@ -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);
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user