trending/codebase-memory-mcp
1
0
Fork 0
mirror of https://github.com/DeusData/codebase-memory-mcp.git synced 2026-11-10 09:13:53 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(mcp): close the two seed-scoping recall gaps from review

Browse Source 
Three items from the #1372 review.

The Windows shard failed in the fixture, not the fix: `system("cd '<dir>' &&
git init …")` used POSIX single quotes, which cmd.exe cannot parse. Switched
to the portable `git -C "<dir>"` shape already used by wt_git in
tests/test_watcher.c, with identity, default branch and signing passed via -c
so the fixture no longer depends on the machine's global git config.

A changed file with hunks but no overlapping definition now keeps whole-file
seeding instead of dropping out. Import-only edits, module-level constants and
anything above the first definition land outside every definition's line range,
so scoping alone would have removed the file from the seed set entirely —
strictly worse recall than the behavior being replaced.

The overlap probe has to apply the same label filter as the seeding loop: a
Module node spans lines 1..EOF, so probing raw nodes reports an overlap for
every hunk and defeats the fallback. Extracting detect_is_seedable_label lets
both sites share one definition. The new regression test caught exactly this —
it failed with seed_symbols 0 until the probe was filtered.

A filled hunk buffer is now treated as a truncated diff: hunks past HUNK_CAP
are gone, so files captured only partially would still look scoped and silently
under-seed. Scoping is dropped for the whole request and the decision logged,
rather than under-reporting a large refactor.

Documented the mixed coordinate systems in the fetch block: base...HEAD hunks
are HEAD-side lines, worktree hunks are worktree-side, node lines come from the
indexed snapshot. They agree while the index is fresh; a stale index plus
earlier-in-file insertions can mis-scope, bounded by the zero-overlap fallback.

Signed-off-by: lishixiang <lishixiang@gmail.com>
This commit is contained in:
lishixiang
2026-07-31 20:51:55 +08:00
parent 2d94282772
commit 9235a23e61
2 changed files with 143 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/mcp/mcp.c
+53 -9
View File
@@ -9762,26 +9762,49 @@ bool cbm_detect_node_in_hunks(const cbm_node_t *node, const cbm_changed_hunk_t *
* "before" state, or the hunk fetch failed/was skipped), every non-container
* definition in the file is a seed — the previous, whole-file behavior — so
* this is a precision improvement, not a new failure mode. */
/* Structural container labels carry no CALLS edges and span the whole file, so
* they are never seeds. Shared by the seeding loop and the overlap probe. */
static bool detect_is_seedable_label(const char *lb) {
return lb && strcmp(lb, "File") != 0 && strcmp(lb, "Folder") != 0 &&
strcmp(lb, "Project") != 0 && strcmp(lb, "Module") != 0 && strcmp(lb, "Package") != 0 &&
strcmp(lb, "Section") != 0;
}
static void detect_collect_seeds(cbm_store_t *store, const char *project, const char *file,
const cbm_changed_hunk_t *hunks, int hunk_count, int64_t **seeds,
int *n, int *cap) {
cbm_node_t *nodes = NULL;
int ncount = 0;
cbm_store_find_nodes_by_file(store, project, file, &nodes, &ncount);
bool has_hunks_for_file = false;
bool scope_to_hunks = false;
for (int h = 0; h < hunk_count; h++) {
if (strcmp(hunks[h].path, file) == 0) {
has_hunks_for_file = true;
scope_to_hunks = true;
break;
}
}
/* A file can have hunks yet no SEEDABLE definition overlapping any of them:
* an import-only edit, a module-level constant, or a change above the first
* definition all land outside every definition's line range. Scoping would
* then drop the file from the seed set entirely — strictly worse recall
* than the whole-file behavior this replaces. Probe for an overlap first
* and keep whole-file seeding for that file when there is none.
*
* The probe must apply the same label filter as the seeding loop below:
* container nodes span the whole file (a Module node is lines 1..EOF), so
* counting them would report an overlap for every hunk and defeat the
* fallback entirely. */
if (scope_to_hunks) {
bool any_overlap = false;
for (int i = 0; i < ncount && !any_overlap; i++) {
any_overlap = detect_is_seedable_label(nodes[i].label) &&
cbm_detect_node_in_hunks(&nodes[i], hunks, hunk_count, file);
}
scope_to_hunks = any_overlap;
}
for (int i = 0; i < ncount; i++) {
const char *lb = nodes[i].label;
if (lb && strcmp(lb, "File") != 0 && strcmp(lb, "Folder") != 0 &&
strcmp(lb, "Project") != 0 && strcmp(lb, "Module") != 0 && strcmp(lb, "Package") != 0 &&
strcmp(lb, "Section") != 0) {
if (has_hunks_for_file &&
!cbm_detect_node_in_hunks(&nodes[i], hunks, hunk_count, file)) {
if (detect_is_seedable_label(nodes[i].label)) {
if (scope_to_hunks && !cbm_detect_node_in_hunks(&nodes[i], hunks, hunk_count, file)) {
continue;
}
if (*n >= *cap) {
@@ -10075,7 +10098,16 @@ static char *handle_detect_changes(cbm_mcp_server_t *srv, const char *args) {
* (see detect_collect_seeds). Best-effort: any failure here just leaves
* `hunks` empty and every file falls back to its previous whole-file
* seeding — this is a precision improvement, not a correctness
* dependency, so it is never treated as a request-level failure. */
* dependency, so it is never treated as a request-level failure.
*
* Coordinate systems: `base...HEAD` hunks carry HEAD-side line numbers,
* the worktree diff carries worktree-side ones, and node line ranges come
* from the indexed snapshot. These agree while the index is fresh — the
* watcher reindexes on HEAD movement and on a dirty tree — but a stale
* index combined with insertions earlier in the file shifts the node lines
* relative to the hunks and can mis-scope. The failure is bounded by
* detect_collect_seeds' zero-overlap fallback: a file whose definitions all
* miss reverts to whole-file seeding rather than dropping out. */
cbm_changed_hunk_t *hunks = NULL;
int hunk_count = 0;
if (want_symbols) {
@@ -10109,6 +10141,18 @@ static char *handle_detect_changes(cbm_mcp_server_t *srv, const char *args) {
enum { HUNK_CAP = 4096 };
hunks = safe_realloc(NULL, (size_t)HUNK_CAP * sizeof(cbm_changed_hunk_t));
hunk_count = cbm_parse_hunks(hbuf, hunks, HUNK_CAP);
/* A filled buffer means the diff was truncated: the hunks
* past the cap are gone, so files captured only partially
* would still look scoped and silently under-seed. Drop
* scoping for the whole request rather than under-report a
* large refactor — whole-file seeding is the safe side. */
if (hunk_count >= HUNK_CAP) {
cbm_log_info("detect_changes.hunks", "action", "scoping_disabled", "reason",
"hunk_cap_reached");
free(hunks);
hunks = NULL;
hunk_count = 0;
}
free(hbuf);
}
}
tests/test_mcp.c
+90 -7
View File
@@ -6192,15 +6192,22 @@ TEST(detect_changes_seeds_only_touched_symbol_issue1363) {
" return y\n"),
0);
/* `git -C` with double quotes, not `cd '<dir>' &&`: single quotes are not
* quoting characters for cmd.exe, and identity/branch/signing come from -c
* so the fixture does not depend on the machine's global git config. The
* assertions below read `base: main`, so pin init.defaultBranch. */
#define DC1363_GITCFG \
"-c user.name=t -c user.email=t@t.io -c init.defaultBranch=main -c commit.gpgsign=false"
char cmd[1200];
snprintf(cmd, sizeof(cmd),
"cd '%s' && git init -q && git config user.email t@t.com && "
"git config user.name t && git add -A && git commit -q -m init",
repo);
if (system(cmd) != 0) {
th_rmtree(repo);
FAIL("git fixture setup failed");
const char *steps[] = {"init -q", "add -A", "commit -q -m init"};
for (size_t s = 0; s < sizeof(steps) / sizeof(steps[0]); s++) {
snprintf(cmd, sizeof(cmd), "git -C \"%s\" " DC1363_GITCFG " %s", repo, steps[s]);
if (system(cmd) != 0) {
th_rmtree(repo);
FAIL("git fixture setup failed");
}
}
#undef DC1363_GITCFG
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
char idx_args[700];
@@ -6239,6 +6246,81 @@ TEST(detect_changes_seeds_only_touched_symbol_issue1363) {
PASS();
}
/* Recall guard for the zero-overlap case (#1363 review): an import-only edit
* changes lines that lie outside every definition's range. Scoping alone would
* drop the file from the seed set — worse recall than the whole-file behavior
* being replaced — so detect_collect_seeds falls back to whole-file seeding
* when a changed file has hunks but no definition overlapping any of them. */
TEST(detect_changes_zero_overlap_falls_back_issue1363) {
char repo[512];
snprintf(repo, sizeof(repo), "%s/cbm-detect-zero-overlap-XXXXXX", cbm_tmpdir());
if (!cbm_mkdtemp(repo)) {
FAIL("cbm_mkdtemp failed");
}
char src[600];
snprintf(src, sizeof(src), "%s/mod.py", repo);
/* Import on line 1 sits above both definitions. */
ASSERT_EQ(th_write_file(src, "import os\n"
"\n"
"\n"
"def foo():\n"
" return 1\n"
"\n"
"\n"
"def bar():\n"
" return 2\n"),
0);
#define DC1363B_GITCFG \
"-c user.name=t -c user.email=t@t.io -c init.defaultBranch=main -c commit.gpgsign=false"
char cmd[1200];
const char *steps[] = {"init -q", "add -A", "commit -q -m init"};
for (size_t s = 0; s < sizeof(steps) / sizeof(steps[0]); s++) {
snprintf(cmd, sizeof(cmd), "git -C \"%s\" " DC1363B_GITCFG " %s", repo, steps[s]);
if (system(cmd) != 0) {
th_rmtree(repo);
FAIL("git fixture setup failed");
}
}
#undef DC1363B_GITCFG
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
char idx_args[700];
snprintf(idx_args, sizeof(idx_args), "{\"repo_path\":\"%s\",\"mode\":\"full\"}", repo);
char *idx_resp = cbm_mcp_handle_tool(srv, "index_repository", idx_args);
ASSERT_NOT_NULL(idx_resp);
free(idx_resp);
/* Edit ONLY the import line — outside every definition's line range. */
ASSERT_EQ(th_write_file(src, "import os, sys\n"
"\n"
"\n"
"def foo():\n"
" return 1\n"
"\n"
"\n"
"def bar():\n"
" return 2\n"),
0);
char *project = cbm_project_name_from_path(repo);
ASSERT_NOT_NULL(project);
char dc_args[700];
snprintf(dc_args, sizeof(dc_args), "{\"project\":\"%s\",\"depth\":1}", project);
char *dc_resp = cbm_mcp_handle_tool(srv, "detect_changes", dc_args);
ASSERT_NOT_NULL(dc_resp);
/* Both definitions must survive: zero overlaps means no scoping for this
* file, not an empty seed set. */
ASSERT_NOT_NULL(strstr(dc_resp, "seed_symbols: 2\\n"));
free(dc_resp);
free(project);
cbm_mcp_server_free(srv);
th_rmtree(repo);
PASS();
}
TEST(tool_ingest_traces_basic) {
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
@@ -9959,6 +10041,7 @@ SUITE(mcp) {
RUN_TEST(tool_detect_changes_contained_commands_clean_up_error_and_success);
RUN_TEST(detect_changes_node_in_hunks_overlap_issue1363);
RUN_TEST(detect_changes_seeds_only_touched_symbol_issue1363);
RUN_TEST(detect_changes_zero_overlap_falls_back_issue1363);
RUN_TEST(tool_ingest_traces_basic);
RUN_TEST(tool_ingest_traces_empty);