mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-04 10:48:20 +00:00
fix: route the remaining all-core sites and fix the review findings
Addresses the four blockers from review: - Capture the expected worker count while CBM_WORKERS is still unset in mcp_auto_index_in_process_uses_background_worker_policy, so the assertion no longer reads the lane override restored just before it. - Give pipeline_background_policy_preserves_index_results a fixture past MIN_FILES_FOR_PARALLEL and drop any inherited CBM_WORKERS / CBM_INDEX_SINGLE_THREAD pin, so it really compares an all-core index against a headroom index instead of two sequential runs. - Route the two remaining all-core sites through cbm_pipeline_worker_count: the LSP surface pass and incremental manifest hashing. The pipeline is plumbed into cbm_pipeline_build_semantic_manifest for that. - Rename the hidden key _background to _cbm_background, matching the existing _cbm_index_policy convention, across every producer and consumer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du315CaKLufAYPEdEwcocq Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
This commit is contained in:
1 parent
2443e92bec
commit
9bfd5cbe60
9 files changed
+78
-34
No files matched your search
@@ -2201,7 +2201,7 @@ static char *application_auto_index_args(cbm_daemon_application_t *application,
|
||||
yyjson_mut_doc_set_root(document, root);
|
||||
bool encoded = yyjson_mut_obj_add_strcpy(document, root, "repo_path", root_path) &&
|
||||
application_index_args_add_policy(application, document, root) &&
|
||||
yyjson_mut_obj_add_bool(document, root, "_background", true);
|
||||
yyjson_mut_obj_add_bool(document, root, "_cbm_background", true);
|
||||
char *args = encoded ? yyjson_mut_write(document, 0, NULL) : NULL;
|
||||
yyjson_mut_doc_free(document);
|
||||
return args;
|
||||
@@ -3895,7 +3895,7 @@ static int application_background_index(cbm_daemon_application_t *application,
|
||||
yyjson_mut_doc_set_root(document, root);
|
||||
bool encoded = yyjson_mut_obj_add_strcpy(document, root, "repo_path", canonical_root) &&
|
||||
application_index_args_add_policy(application, document, root) &&
|
||||
yyjson_mut_obj_add_bool(document, root, "_background", true);
|
||||
yyjson_mut_obj_add_bool(document, root, "_cbm_background", true);
|
||||
char *default_project = cbm_project_name_from_path(canonical_root);
|
||||
bool custom_project =
|
||||
project_name[0] && (!default_project || strcmp(default_project, project_name) != 0);
|
||||
|
||||
+2
-2
@@ -11110,7 +11110,7 @@ static char *index_run_supervised_path(cbm_mcp_server_t *srv, const char *root_p
|
||||
yyjson_mut_doc_set_root(doc, root);
|
||||
if (!yyjson_mut_obj_add_strcpy(doc, root, "repo_path", root_path) ||
|
||||
!cbm_mcp_index_policy_add_to_args(doc, root, &policy) ||
|
||||
!yyjson_mut_obj_add_bool(doc, root, "_background", true)) {
|
||||
!yyjson_mut_obj_add_bool(doc, root, "_cbm_background", true)) {
|
||||
yyjson_mut_doc_free(doc);
|
||||
return NULL;
|
||||
}
|
||||
@@ -11499,7 +11499,7 @@ static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) {
|
||||
char *repo_path = cbm_mcp_get_string_arg(args, "repo_path");
|
||||
char *mode_str = cbm_mcp_get_string_arg(args, "mode");
|
||||
char *name_override = cbm_mcp_get_string_arg(args, "name");
|
||||
bool background = cbm_mcp_get_bool_arg(args, "_background");
|
||||
bool background = cbm_mcp_get_bool_arg(args, "_cbm_background");
|
||||
cbm_normalize_path_sep(repo_path);
|
||||
|
||||
if (!repo_path) {
|
||||
|
||||
@@ -231,8 +231,9 @@ int cbm_lsp_surface_build_rows(const cbm_pipeline_ctx_t *ctx, const char *projec
|
||||
.rows = rows,
|
||||
};
|
||||
atomic_init(&job.failed, false);
|
||||
int workers = cbm_pipeline_worker_count(ctx ? ctx->pipeline : NULL);
|
||||
cbm_parallel_for(file_count, surface_row_one, &job,
|
||||
(cbm_parallel_for_opts_t){.max_workers = 0, .force_pthreads = false});
|
||||
(cbm_parallel_for_opts_t){.max_workers = workers, .force_pthreads = false});
|
||||
if (atomic_load_explicit(&job.failed, memory_order_relaxed)) {
|
||||
cbm_store_free_lsp_surfaces(rows, file_count); /* untouched rows are all NULL */
|
||||
return -1;
|
||||
|
||||
@@ -2810,7 +2810,7 @@ static int cbm_pipeline_run_staged(cbm_pipeline_t *p) {
|
||||
rc = mode_promoted
|
||||
? cbm_pipeline_build_fresh_semantic_manifest(p, p->project_name, &baseline_manifest,
|
||||
&baseline_count)
|
||||
: cbm_pipeline_build_semantic_manifest(p->project_name, p->repo_path, files,
|
||||
: cbm_pipeline_build_semantic_manifest(p, p->project_name, p->repo_path, files,
|
||||
file_count, p->excluded_dirs, p->excluded_count,
|
||||
&p->git_ctx, p->userconfig, &baseline_manifest,
|
||||
&baseline_count);
|
||||
|
||||
@@ -413,9 +413,9 @@ static void *manifest_hash_worker(void *arg) {
|
||||
|
||||
enum { MANIFEST_PARALLEL_MIN_FILES = 64 };
|
||||
|
||||
int cbm_pipeline_build_semantic_manifest(const char *project, const char *repo_path,
|
||||
const cbm_file_info_t *files, int file_count,
|
||||
char **excluded_dirs, int excluded_count,
|
||||
int cbm_pipeline_build_semantic_manifest(const cbm_pipeline_t *p, const char *project,
|
||||
const char *repo_path, const cbm_file_info_t *files,
|
||||
int file_count, char **excluded_dirs, int excluded_count,
|
||||
const cbm_git_context_t *git_ctx,
|
||||
const cbm_userconfig_t *userconfig, cbm_file_hash_t **out,
|
||||
int *out_count) {
|
||||
@@ -456,7 +456,7 @@ int cbm_pipeline_build_semantic_manifest(const char *project, const char *repo_p
|
||||
}
|
||||
struct timespec t_hash;
|
||||
cbm_clock_gettime(CLOCK_MONOTONIC, &t_hash);
|
||||
int hash_workers = cbm_default_worker_count(true);
|
||||
int hash_workers = cbm_pipeline_worker_count(p);
|
||||
if (rc == 0 && file_count >= MANIFEST_PARALLEL_MIN_FILES && hash_workers > SKIP_ONE) {
|
||||
char (*shas)[CBM_SHA256_HEX_LEN + 1] = malloc((size_t)file_count * sizeof(*shas));
|
||||
int64_t *mtimes = calloc((size_t)file_count, sizeof(int64_t));
|
||||
@@ -634,9 +634,9 @@ int cbm_pipeline_build_fresh_semantic_manifest(cbm_pipeline_t *p, const char *pr
|
||||
&fresh_ignored_total);
|
||||
}
|
||||
if (rc == 0) {
|
||||
rc = cbm_pipeline_build_semantic_manifest(project, repo_path, fresh_files, fresh_file_count,
|
||||
fresh_excluded, fresh_excluded_count,
|
||||
&fresh_git_ctx, fresh_userconfig, out, out_count);
|
||||
rc = cbm_pipeline_build_semantic_manifest(
|
||||
p, project, repo_path, fresh_files, fresh_file_count, fresh_excluded,
|
||||
fresh_excluded_count, &fresh_git_ctx, fresh_userconfig, out, out_count);
|
||||
}
|
||||
cbm_set_user_lang_config(previous_userconfig);
|
||||
cbm_git_context_free(&fresh_git_ctx);
|
||||
@@ -1602,8 +1602,12 @@ static int closure_probe_surfaces(cbm_pipeline_t *p, const char *project,
|
||||
if (def_modules && def_starts) {
|
||||
defs = cbm_pxc_collect_all_defs(NULL, &probe_arena, cache, probe_files, probe_count,
|
||||
project, def_modules, &def_count, def_starts);
|
||||
rc = cbm_lsp_surface_build_rows(NULL, project, cache, probe_files, probe_count, defs,
|
||||
def_starts, out_rows, out_count);
|
||||
/* Extract left pipeline NULL so file errors are not recorded twice.
|
||||
* Surface rows only borrow it for the background worker policy. */
|
||||
probe_ctx.pipeline = p;
|
||||
rc = cbm_lsp_surface_build_rows(&probe_ctx, project, cache, probe_files, probe_count,
|
||||
defs, def_starts, out_rows, out_count);
|
||||
probe_ctx.pipeline = NULL;
|
||||
} else {
|
||||
rc = -1;
|
||||
}
|
||||
|
||||
@@ -808,9 +808,9 @@ int cbm_pipeline_run_incremental(cbm_pipeline_t *p, const char *db_path, cbm_fil
|
||||
#define CBM_SEMANTIC_INPUT_GLOBAL_CONFIG CBM_SEMANTIC_INPUT_PREFIX "global-extension-config-v1"
|
||||
#define CBM_SEMANTIC_INPUT_PROJECT_CONFIG CBM_SEMANTIC_INPUT_PREFIX "project-extension-config-v1"
|
||||
|
||||
int cbm_pipeline_build_semantic_manifest(const char *project, const char *repo_path,
|
||||
const cbm_file_info_t *files, int file_count,
|
||||
char **excluded_dirs, int excluded_count,
|
||||
int cbm_pipeline_build_semantic_manifest(const cbm_pipeline_t *p, const char *project,
|
||||
const char *repo_path, const cbm_file_info_t *files,
|
||||
int file_count, char **excluded_dirs, int excluded_count,
|
||||
const cbm_git_context_t *git_ctx,
|
||||
const cbm_userconfig_t *userconfig, cbm_file_hash_t **out,
|
||||
int *out_count);
|
||||
|
||||
@@ -1444,7 +1444,7 @@ static int app_fake_worker_start(void *opaque, const char *args_json, size_t mem
|
||||
atomic_store(&context->args_captured[worker->attempt], true);
|
||||
context->memory_budgets[worker->attempt] = memory_budget_bytes;
|
||||
context->background_requests[worker->attempt] =
|
||||
cbm_mcp_get_bool_arg(args_json, "_background");
|
||||
cbm_mcp_get_bool_arg(args_json, "_cbm_background");
|
||||
if (marker_file) {
|
||||
(void)snprintf(context->marker_paths[worker->attempt], APP_TEST_PATH_CAP, "%s",
|
||||
marker_file);
|
||||
|
||||
+4
-1
@@ -20057,6 +20057,9 @@ TEST(mcp_auto_index_in_process_uses_background_worker_policy) {
|
||||
if (cwd_ready) {
|
||||
(void)cbm_chdir(old_cwd);
|
||||
}
|
||||
/* Capture while CBM_WORKERS is still unset. Restoring first makes the
|
||||
* expected count follow the lane override instead of the headroom policy. */
|
||||
int expected = cbm_default_worker_count(false);
|
||||
mcp_test_restore_env(environment, sizeof(environment) / sizeof(environment[0]));
|
||||
bool cleaned = !cache_ready || th_rmtree(cache) == 0;
|
||||
|
||||
@@ -20068,7 +20071,7 @@ TEST(mcp_auto_index_in_process_uses_background_worker_policy) {
|
||||
ASSERT_TRUE(config_ready);
|
||||
ASSERT_TRUE(server_ready);
|
||||
ASSERT_TRUE(response_ready);
|
||||
ASSERT_EQ(selected_workers, cbm_default_worker_count(false));
|
||||
ASSERT_EQ(selected_workers, expected);
|
||||
ASSERT_TRUE(cleaned);
|
||||
PASS();
|
||||
}
|
||||
|
||||
+50
-14
@@ -161,32 +161,68 @@ TEST(pipeline_background_worker_policy_preserves_overrides) {
|
||||
|
||||
TEST(pipeline_background_policy_preserves_index_results) {
|
||||
ASSERT_EQ(setup_test_repo(), 0);
|
||||
/* 64 fillers plus setup_test_repo's three files exceed MIN_FILES_FOR_PARALLEL
|
||||
* (50). Below that threshold both policies take the sequential path, so the
|
||||
* comparison stays green even with the background policy removed. */
|
||||
for (int i = 0; i < 64; i++) {
|
||||
char name[32];
|
||||
char body[96];
|
||||
(void)snprintf(name, sizeof(name), "pad%02d.go", i);
|
||||
(void)snprintf(body, sizeof(body), "package main\n\nfunc Pad%02d() int {\n\treturn %d\n}\n",
|
||||
i, i);
|
||||
if (th_write_file(TH_PATH(g_tmpdir, name), body) != 0) {
|
||||
teardown_test_repo();
|
||||
FAIL("failed to pad fixture past MIN_FILES_FOR_PARALLEL");
|
||||
}
|
||||
}
|
||||
|
||||
/* An inherited CBM_WORKERS pin makes both runs use the same count. Drop it
|
||||
* so this compares an all-core index against a headroom index. */
|
||||
const char *old_workers = getenv("CBM_WORKERS");
|
||||
char *saved_workers = old_workers ? strdup(old_workers) : NULL;
|
||||
const char *old_single = getenv("CBM_INDEX_SINGLE_THREAD");
|
||||
char *saved_single = old_single ? strdup(old_single) : NULL;
|
||||
if ((old_workers && !saved_workers) || (old_single && !saved_single)) {
|
||||
free(saved_workers);
|
||||
free(saved_single);
|
||||
teardown_test_repo();
|
||||
FAIL("failed to save worker env");
|
||||
}
|
||||
cbm_unsetenv("CBM_WORKERS");
|
||||
cbm_unsetenv("CBM_INDEX_SINGLE_THREAD");
|
||||
|
||||
char foreground_db[512];
|
||||
char background_db[512];
|
||||
(void)snprintf(foreground_db, sizeof(foreground_db), "%s/foreground.db", g_tmpdir);
|
||||
(void)snprintf(background_db, sizeof(background_db), "%s/background.db", g_tmpdir);
|
||||
|
||||
cbm_pipeline_t *foreground =
|
||||
cbm_pipeline_new(g_tmpdir, foreground_db, CBM_MODE_FULL);
|
||||
cbm_pipeline_t *background =
|
||||
cbm_pipeline_new(g_tmpdir, background_db, CBM_MODE_FULL);
|
||||
ASSERT_NOT_NULL(foreground);
|
||||
ASSERT_NOT_NULL(background);
|
||||
cbm_pipeline_set_background(background, true);
|
||||
|
||||
int foreground_rc = cbm_pipeline_run(foreground);
|
||||
int background_rc = cbm_pipeline_run(background);
|
||||
cbm_pipeline_t *foreground = cbm_pipeline_new(g_tmpdir, foreground_db, CBM_MODE_FULL);
|
||||
cbm_pipeline_t *background = cbm_pipeline_new(g_tmpdir, background_db, CBM_MODE_FULL);
|
||||
int foreground_rc = -1;
|
||||
int background_rc = -1;
|
||||
int foreground_nodes = -1;
|
||||
int foreground_edges = -1;
|
||||
int background_nodes = -1;
|
||||
int background_edges = -1;
|
||||
cbm_pipeline_get_committed_counts(foreground, &foreground_nodes, &foreground_edges);
|
||||
cbm_pipeline_get_committed_counts(background, &background_nodes, &background_edges);
|
||||
bool created = foreground && background;
|
||||
if (created) {
|
||||
cbm_pipeline_set_background(background, true);
|
||||
foreground_rc = cbm_pipeline_run(foreground);
|
||||
background_rc = cbm_pipeline_run(background);
|
||||
cbm_pipeline_get_committed_counts(foreground, &foreground_nodes, &foreground_edges);
|
||||
cbm_pipeline_get_committed_counts(background, &background_nodes, &background_edges);
|
||||
}
|
||||
|
||||
cbm_pipeline_free(foreground);
|
||||
cbm_pipeline_free(background);
|
||||
teardown_test_repo();
|
||||
saved_workers ? cbm_setenv("CBM_WORKERS", saved_workers, 1) : cbm_unsetenv("CBM_WORKERS");
|
||||
saved_single ? cbm_setenv("CBM_INDEX_SINGLE_THREAD", saved_single, 1)
|
||||
: cbm_unsetenv("CBM_INDEX_SINGLE_THREAD");
|
||||
free(saved_workers);
|
||||
free(saved_single);
|
||||
|
||||
ASSERT_TRUE(created);
|
||||
ASSERT_EQ(foreground_rc, 0);
|
||||
ASSERT_EQ(background_rc, 0);
|
||||
ASSERT_GT(foreground_nodes, 0);
|
||||
@@ -5458,8 +5494,8 @@ TEST(pipeline_semantic_manifest_rejects_non_directory_root) {
|
||||
|
||||
cbm_file_hash_t *manifest = NULL;
|
||||
int manifest_count = -1;
|
||||
int rc = cbm_pipeline_build_semantic_manifest("manifest-fail-closed", root_path, NULL, 0, NULL,
|
||||
0, NULL, NULL, &manifest, &manifest_count);
|
||||
int rc = cbm_pipeline_build_semantic_manifest(NULL, "manifest-fail-closed", root_path, NULL, 0,
|
||||
NULL, 0, NULL, NULL, &manifest, &manifest_count);
|
||||
cbm_pipeline_free_semantic_manifest(manifest, manifest_count > 0 ? manifest_count : 0);
|
||||
th_rmtree(tmp);
|
||||
|
||||
|
||||
Reference in new issue
Block a user