mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-02 07:53:41 +08:00
mcp: fix get_code returning ambiguous with 1 match + cold-start project detection
Three bugs fixed in handle_get_code_snippet:
Bug A (root cause): Tiers 1-3 all use WHERE project = ?1 AND ... in SQL.
When project param is NULL, SQLite binds NULL and the comparison is always
false — all exact/suffix/name lookups silently return 0 rows.
Bug B: Tier 4 fuzzy search found 1 result but snippet_suggestions() always
sets status=ambiguous regardless of count.
Bug C: Dedup block with cand_count==1 after dedup fell through to ambiguous
instead of resolving.
Fixes:
1. extract_project_from_qn(): scans each dot-prefix of the QN and tests for
a matching ~/.cache/codebase-memory-mcp/{prefix}.db file (O(n), ~5-15
access() calls). Returns longest matching prefix — the QN is
self-describing so this works even on cold start (no prior search call).
Single malloc+memcpy pattern: best_end offset avoids repeated strdup.
2. handle_get_code_snippet: when project param is NULL, calls
extract_project_from_qn(qn) and opens the correct DB via resolve_store.
Falls back to srv->current_project if no DB found. Assigns result into
project (was NULL) so all existing free(project) exit paths own the memory.
3. Tier 4 fuzzy: fuzzy_count==1 now resolves directly (build_snippet_response)
instead of calling snippet_suggestions.
4. Dedup block: cand_count==1 after dedup now resolves directly.
Tests added (3 new, total 2190):
- get_code_no_project_uses_open_store_tier1: after search_graph opens a
store, get_code without project resolves via Tier 1 exact QN + eff_project
- get_code_single_fuzzy_result_resolves_not_ambiguous: wrong-prefix QN forces
Tier 4 fuzzy; single result must not return status=ambiguous
- get_code_cold_start_parses_project_from_qn: fresh server, no prior call,
extract_project_from_qn finds the DB and resolves the symbol
Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
This commit is contained in:
+100
-6
@@ -816,6 +816,54 @@ static const char *project_db_path(const char *project, char *buf, size_t bufsz)
|
||||
return buf;
|
||||
}
|
||||
|
||||
/* ── QN project extraction ─────────────────────────────────────── */
|
||||
|
||||
/* Try to identify the project prefix of a qualified name by scanning each
|
||||
* dot-separated prefix and checking if a matching DB file exists.
|
||||
* Returns a heap-allocated project name (caller must free), or NULL if no
|
||||
* matching DB is found. Cost: one access() call per dot in the QN (~5-10). */
|
||||
static char *extract_project_from_qn(const char *qn) {
|
||||
if (!qn) return NULL;
|
||||
const char *home = getenv("HOME");
|
||||
if (!home) return NULL;
|
||||
|
||||
/* Scan each dot-separated prefix of the QN and test if a matching DB file
|
||||
* exists. Walk left-to-right so the last hit is the longest (most
|
||||
* specific) match. Record only the winning offset to do a single strdup
|
||||
* at the end — avoids repeated alloc/free on multi-dot project names. */
|
||||
size_t qn_len = strlen(qn);
|
||||
char *candidate = malloc(qn_len + 1);
|
||||
if (!candidate) return NULL;
|
||||
memcpy(candidate, qn, qn_len + 1);
|
||||
|
||||
size_t best_end = 0; /* length of the longest matching prefix found */
|
||||
char db_path[1024];
|
||||
const char *home_val = home;
|
||||
|
||||
for (size_t i = 0; i < qn_len; i++) {
|
||||
if (candidate[i] == '.') {
|
||||
candidate[i] = '\0';
|
||||
snprintf(db_path, sizeof(db_path),
|
||||
"%s/.cache/codebase-memory-mcp/%s.db", home_val, candidate);
|
||||
if (access(db_path, F_OK) == 0) {
|
||||
best_end = i; /* length of this prefix */
|
||||
}
|
||||
candidate[i] = '.';
|
||||
}
|
||||
}
|
||||
|
||||
char *result = NULL;
|
||||
if (best_end > 0) {
|
||||
result = malloc(best_end + 1);
|
||||
if (result) {
|
||||
memcpy(result, qn, best_end);
|
||||
result[best_end] = '\0';
|
||||
}
|
||||
}
|
||||
free(candidate);
|
||||
return result; /* NULL if no matching DB found; caller frees */
|
||||
}
|
||||
|
||||
/* ── Store resolution ──────────────────────────────────────────── */
|
||||
|
||||
/* Open the right project's .db file for query tools.
|
||||
@@ -2513,6 +2561,24 @@ static char *handle_get_code_snippet(cbm_mcp_server_t *srv, const char *args) {
|
||||
char *qn = cbm_mcp_get_string_arg(args, "qualified_name");
|
||||
char *project = cbm_mcp_get_string_arg(args, "project");
|
||||
cbm_store_t *store = resolve_store(srv, project);
|
||||
/* When no project param given, try to parse the project prefix from the
|
||||
* qualified name by checking for a matching .db file. This is Option C:
|
||||
* the QN is self-describing, so we can always open the right store even on
|
||||
* a cold start (no prior search_code_graph call).
|
||||
* Falls back to the currently-open store's project as a secondary option. */
|
||||
const char *eff_project = project;
|
||||
if (!eff_project && qn) {
|
||||
/* Option C: QN is self-describing — try to find the project prefix by
|
||||
* checking for a matching .db file. assign into project so the
|
||||
* existing free(project) calls at every exit path own the memory. */
|
||||
project = extract_project_from_qn(qn);
|
||||
if (project) {
|
||||
eff_project = project;
|
||||
store = resolve_store(srv, project); /* open the correct DB */
|
||||
} else if (srv->current_project && srv->current_project[0]) {
|
||||
eff_project = srv->current_project; /* fallback: last-used project */
|
||||
}
|
||||
}
|
||||
bool auto_resolve = cbm_mcp_get_bool_arg(args, "auto_resolve");
|
||||
bool include_neighbors = cbm_mcp_get_bool_arg(args, "include_neighbors");
|
||||
int cfg_max_lines = cbm_config_get_int(srv->config, CBM_CONFIG_SNIPPET_MAX_LINES,
|
||||
@@ -2539,7 +2605,7 @@ static char *handle_get_code_snippet(cbm_mcp_server_t *srv, const char *args) {
|
||||
|
||||
/* Tier 1: Exact QN match */
|
||||
cbm_node_t node = {0};
|
||||
int rc = cbm_store_find_node_by_qn(store, project, qn, &node);
|
||||
int rc = cbm_store_find_node_by_qn(store, eff_project, qn, &node);
|
||||
if (rc == CBM_STORE_OK) {
|
||||
char *result =
|
||||
build_snippet_response(srv, &node, NULL /*exact*/, include_neighbors, NULL, 0,
|
||||
@@ -2554,7 +2620,7 @@ static char *handle_get_code_snippet(cbm_mcp_server_t *srv, const char *args) {
|
||||
/* Tier 2: QN suffix match */
|
||||
cbm_node_t *suffix_nodes = NULL;
|
||||
int suffix_count = 0;
|
||||
cbm_store_find_nodes_by_qn_suffix(store, project, qn, &suffix_nodes, &suffix_count);
|
||||
cbm_store_find_nodes_by_qn_suffix(store, eff_project, qn, &suffix_nodes, &suffix_count);
|
||||
if (suffix_count == 1) {
|
||||
copy_node(&suffix_nodes[0], &node);
|
||||
cbm_store_free_nodes(suffix_nodes, suffix_count);
|
||||
@@ -2570,7 +2636,7 @@ static char *handle_get_code_snippet(cbm_mcp_server_t *srv, const char *args) {
|
||||
/* Tier 3: Short name match */
|
||||
cbm_node_t *name_nodes = NULL;
|
||||
int name_count = 0;
|
||||
cbm_store_find_nodes_by_name(store, project, qn, &name_nodes, &name_count);
|
||||
cbm_store_find_nodes_by_name(store, eff_project, qn, &name_nodes, &name_count);
|
||||
if (name_count == 1) {
|
||||
copy_node(&name_nodes[0], &node);
|
||||
cbm_store_free_nodes(name_nodes, name_count);
|
||||
@@ -2610,8 +2676,22 @@ static char *handle_get_code_snippet(cbm_mcp_server_t *srv, const char *args) {
|
||||
cbm_store_free_nodes(suffix_nodes, suffix_count);
|
||||
cbm_store_free_nodes(name_nodes, name_count);
|
||||
|
||||
/* Auto-resolve: pick best candidate by degree */
|
||||
if (auto_resolve && cand_count >= 2 && cand_count <= 2) {
|
||||
/* Single candidate after dedup — resolve immediately, not ambiguous */
|
||||
if (cand_count == 1) {
|
||||
copy_node(&candidates[0], &node);
|
||||
free_node_contents(&candidates[0]);
|
||||
free(candidates);
|
||||
char *result = build_snippet_response(srv, &node, "name", include_neighbors, NULL, 0,
|
||||
max_lines, snippet_mode);
|
||||
free_node_contents(&node);
|
||||
free(qn);
|
||||
free(project);
|
||||
free(snippet_mode);
|
||||
return result;
|
||||
}
|
||||
|
||||
/* Auto-resolve: pick best candidate by degree when 2+ candidates */
|
||||
if (auto_resolve && cand_count >= 2) {
|
||||
/* Find best: highest total degree, prefer non-test files */
|
||||
int best_idx = 0;
|
||||
int best_deg = -1;
|
||||
@@ -2687,7 +2767,7 @@ static char *handle_get_code_snippet(cbm_mcp_server_t *srv, const char *args) {
|
||||
|
||||
/* Use search with name pattern for fuzzy matching */
|
||||
cbm_search_params_t params = {0};
|
||||
params.project = project;
|
||||
params.project = eff_project;
|
||||
params.name_pattern = search_name;
|
||||
params.limit = 5;
|
||||
params.min_degree = -1;
|
||||
@@ -2705,6 +2785,20 @@ static char *handle_get_code_snippet(cbm_mcp_server_t *srv, const char *args) {
|
||||
int fuzzy_count = search_out.count;
|
||||
cbm_store_search_free(&search_out);
|
||||
|
||||
/* Single fuzzy result — resolve immediately rather than reporting ambiguous */
|
||||
if (fuzzy_count == 1) {
|
||||
copy_node(&fuzzy[0], &node);
|
||||
free_node_contents(&fuzzy[0]);
|
||||
free(fuzzy);
|
||||
char *result = build_snippet_response(srv, &node, "fuzzy", include_neighbors, NULL, 0,
|
||||
max_lines, snippet_mode);
|
||||
free_node_contents(&node);
|
||||
free(qn);
|
||||
free(project);
|
||||
free(snippet_mode);
|
||||
return result;
|
||||
}
|
||||
|
||||
char *result = snippet_suggestions(qn, fuzzy, fuzzy_count);
|
||||
for (int i = 0; i < fuzzy_count; i++) {
|
||||
free_node_contents(&fuzzy[i]);
|
||||
|
||||
@@ -1188,6 +1188,129 @@ TEST(prefix_collision_session_is_substring_of_project) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* ── 16. get_code NULL-project regression tests ─────────── */
|
||||
|
||||
/* Bug: Tier 1-3 use WHERE project = ?1, so they return nothing when project
|
||||
* is NULL (SQL NULL comparison is always false). Fix: eff_project falls back
|
||||
* to srv->current_project when the caller omits the project param.
|
||||
*
|
||||
* Test: after search_graph opens a store, get_code with no project param
|
||||
* should resolve via Tier 1 exact QN match. */
|
||||
TEST(get_code_no_project_uses_open_store_tier1) {
|
||||
/* Create a file DB with one node */
|
||||
char db_path[1024];
|
||||
snprintf(db_path, sizeof(db_path), "%s/.cache/codebase-memory-mcp/_tc_gc_proj_.db",
|
||||
getenv("HOME"));
|
||||
cbm_store_t *s = cbm_store_open_path(db_path);
|
||||
ASSERT_NOT_NULL(s);
|
||||
cbm_store_upsert_project(s, "_tc_gc_proj_", "/tmp");
|
||||
cbm_node_t n = {.project = "_tc_gc_proj_", .label = "Function",
|
||||
.name = "tc_resolve_fn",
|
||||
.qualified_name = "_tc_gc_proj_.src.tc_resolve_fn",
|
||||
.file_path = "src/tc_resolve_fn.c"};
|
||||
cbm_store_upsert_node(s, &n);
|
||||
cbm_store_close(s);
|
||||
|
||||
/* Create server; call search_graph to open the store (sets current_project) */
|
||||
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
|
||||
ASSERT_NOT_NULL(srv);
|
||||
char *sr = cbm_mcp_handle_tool(srv, "search_graph",
|
||||
"{\"project\":\"_tc_gc_proj_\",\"name_pattern\":\"tc_resolve_fn\",\"limit\":1}");
|
||||
ASSERT_NOT_NULL(sr);
|
||||
free(sr);
|
||||
|
||||
/* get_code with no project param — eff_project must fall back to current_project */
|
||||
char *gr = cbm_mcp_handle_tool(srv, "get_code",
|
||||
"{\"qualified_name\":\"_tc_gc_proj_.src.tc_resolve_fn\"}");
|
||||
ASSERT_NOT_NULL(gr);
|
||||
/* Must NOT be ambiguous — Tier 1 exact QN should resolve via eff_project */
|
||||
ASSERT_NULL(strstr(gr, "\"ambiguous\""));
|
||||
/* Must contain the function name in the response */
|
||||
ASSERT_NOT_NULL(strstr(gr, "tc_resolve_fn"));
|
||||
free(gr);
|
||||
|
||||
cbm_mcp_server_free(srv);
|
||||
(void)unlink(db_path);
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* Bug: Tier 4 fuzzy search finding exactly 1 result returned status=ambiguous.
|
||||
* Fix: when fuzzy_count == 1, resolve immediately instead of calling
|
||||
* snippet_suggestions which always sets status=ambiguous. */
|
||||
TEST(get_code_single_fuzzy_result_resolves_not_ambiguous) {
|
||||
/* Create a file DB with one node */
|
||||
char db_path[1024];
|
||||
snprintf(db_path, sizeof(db_path), "%s/.cache/codebase-memory-mcp/_tc_gc_fuzzy_.db",
|
||||
getenv("HOME"));
|
||||
cbm_store_t *s = cbm_store_open_path(db_path);
|
||||
ASSERT_NOT_NULL(s);
|
||||
cbm_store_upsert_project(s, "_tc_gc_fuzzy_", "/tmp");
|
||||
cbm_node_t n = {.project = "_tc_gc_fuzzy_", .label = "Function",
|
||||
.name = "tc_unique_fuzzy_fn",
|
||||
.qualified_name = "_tc_gc_fuzzy_.src.tc_unique_fuzzy_fn",
|
||||
.file_path = "src/tc_unique_fuzzy_fn.c"};
|
||||
cbm_store_upsert_node(s, &n);
|
||||
cbm_store_close(s);
|
||||
|
||||
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
|
||||
ASSERT_NOT_NULL(srv);
|
||||
/* Open the store via search_graph so current_project is set */
|
||||
char *sr = cbm_mcp_handle_tool(srv, "search_graph",
|
||||
"{\"project\":\"_tc_gc_fuzzy_\",\"name_pattern\":\"tc_unique_fuzzy_fn\",\"limit\":1}");
|
||||
ASSERT_NOT_NULL(sr);
|
||||
free(sr);
|
||||
|
||||
/* QN with a wrong prefix — Tiers 1-3 will miss, Tier 4 fuzzy finds 1 by name */
|
||||
char *gr = cbm_mcp_handle_tool(srv, "get_code",
|
||||
"{\"qualified_name\":\"wrong.prefix.tc_unique_fuzzy_fn\"}");
|
||||
ASSERT_NOT_NULL(gr);
|
||||
/* Must NOT be ambiguous — single fuzzy result should auto-resolve */
|
||||
ASSERT_NULL(strstr(gr, "\"ambiguous\""));
|
||||
/* Must contain the function name */
|
||||
ASSERT_NOT_NULL(strstr(gr, "tc_unique_fuzzy_fn"));
|
||||
free(gr);
|
||||
|
||||
cbm_mcp_server_free(srv);
|
||||
(void)unlink(db_path);
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* Option C: cold-start test — no prior search_code_graph call.
|
||||
* extract_project_from_qn() must find the DB by scanning dot-prefixes of the
|
||||
* QN, so get_code works even when srv->current_project is unset. */
|
||||
TEST(get_code_cold_start_parses_project_from_qn) {
|
||||
char db_path[1024];
|
||||
snprintf(db_path, sizeof(db_path), "%s/.cache/codebase-memory-mcp/_tc_gc_cold_.db",
|
||||
getenv("HOME"));
|
||||
cbm_store_t *s = cbm_store_open_path(db_path);
|
||||
ASSERT_NOT_NULL(s);
|
||||
cbm_store_upsert_project(s, "_tc_gc_cold_", "/tmp");
|
||||
cbm_node_t n = {.project = "_tc_gc_cold_", .label = "Function",
|
||||
.name = "tc_cold_fn",
|
||||
.qualified_name = "_tc_gc_cold_.src.tc_cold_fn",
|
||||
.file_path = "src/tc_cold_fn.c"};
|
||||
cbm_store_upsert_node(s, &n);
|
||||
cbm_store_close(s);
|
||||
|
||||
/* Fresh server — no prior tool calls, srv->current_project is unset */
|
||||
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
|
||||
ASSERT_NOT_NULL(srv);
|
||||
|
||||
/* get_code with no project — must parse "_tc_gc_cold_" from the QN */
|
||||
char *gr = cbm_mcp_handle_tool(srv, "get_code",
|
||||
"{\"qualified_name\":\"_tc_gc_cold_.src.tc_cold_fn\"}");
|
||||
ASSERT_NOT_NULL(gr);
|
||||
/* Cold-start Option C: must resolve, not return ambiguous or not-found */
|
||||
ASSERT_NULL(strstr(gr, "\"ambiguous\""));
|
||||
ASSERT_NULL(strstr(gr, "\"error\""));
|
||||
ASSERT_NOT_NULL(strstr(gr, "tc_cold_fn"));
|
||||
free(gr);
|
||||
|
||||
cbm_mcp_server_free(srv);
|
||||
(void)unlink(db_path);
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* ── Suite registration ──────────────────────────────────── */
|
||||
|
||||
SUITE(tool_consolidation) {
|
||||
@@ -1268,4 +1391,8 @@ SUITE(tool_consolidation) {
|
||||
RUN_TEST(prefix_collision_longer_name_with_dot_not_dep);
|
||||
RUN_TEST(prefix_collision_completely_different_project);
|
||||
RUN_TEST(prefix_collision_session_is_substring_of_project);
|
||||
/* get_code NULL-project regression */
|
||||
RUN_TEST(get_code_no_project_uses_open_store_tier1);
|
||||
RUN_TEST(get_code_single_fuzzy_result_resolves_not_ambiguous);
|
||||
RUN_TEST(get_code_cold_start_parses_project_from_qn);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user