From e5f3aab02e1c8bdb3d3e04f6b52d78d741e1241f Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 28 Aug 2026 19:03:07 +0200 Subject: [PATCH 1/2] feat(pipeline): index-time importance scoring (weighted degree) Score every Function/Method/Class node during indexing and persist the result as a numeric "importance" key inside the node's existing properties_json: importance = sqrt(num_refs) * priv * generic * distinct * test_penalty num_refs is the incoming CALLS + USAGE degree. The multipliers demote private (leading-underscore) and generic names (defined in >= 5 distinct files), promote distinctive identifiers (snake_case or camelCase, len >= 8), and demote test scaffolding -- symbols in a test file per the graph's own cbm_is_test_path() classifier, or targets of an incoming TESTS edge. Weighted degree only; a transitive (PageRank) refinement is deliberately not built here until it can be shown to beat this on a fixed judgment set. No schema change and no CBM_INDEX_FORMAT_VERSION bump: the score lives in the existing properties_json TEXT column, so no index is forced to rebuild and indexes written by older builds simply lack the key. Three failure modes this shape invites, all of them silent, are addressed directly: * Cost. The generic-name multiplier needs |{files a name is defined in}|. Computing that per NODE with a pairwise file-path scan is O(k^3) in a same-name group of size k -- ruinous on real corpora, where a single Java name group reaches k > 4000. Here the count is computed once per distinct NAME and memoized, and file paths are deduplicated through a hash set, so total distinct-file work is linear in the graph. Measured cold full index of a 666 MB Java corpus, same -O2 binary: 85 s with the pass disabled, 80 s with this implementation, 481 s with the per-node shape. * Registration. PREDUMP_PASS_COUNT is now derived from the pass table with sizeof instead of being hand-written. A hand-written count that lags an appended entry silently skips the last-registered pass with every test still green. * Incremental. The incremental post-pass sequence runs the pass too, over a graph rehydrated from the store whose nodes already carry the key. The write-back overwrites an existing key in place instead of appending a second one, which would otherwise corrupt properties_json into {"importance":1.0,...,"importance":2.0}. tests/test_importance.c covers the formula and binds all three failure modes; the complexity suite gains a GATED counter asserting the pass's same-name-group work stays linear (~2x, not ~4x) under a corpus doubling. Signed-off-by: Martin Vogel Co-authored-by: petercoxphoto <20230727+petercoxphoto@users.noreply.github.com> --- Makefile.cbm | 3 +- src/pipeline/pass_importance.c | 396 +++++++++++++++++ src/pipeline/pipeline.c | 22 +- src/pipeline/pipeline_incremental.c | 14 + src/pipeline/pipeline_internal.h | 25 ++ tests/test_complexity.c | 44 ++ tests/test_importance.c | 641 ++++++++++++++++++++++++++++ tests/test_main.c | 2 + 8 files changed, 1142 insertions(+), 5 deletions(-) create mode 100644 src/pipeline/pass_importance.c create mode 100644 tests/test_importance.c diff --git a/Makefile.cbm b/Makefile.cbm index b71fa6ac9..dc92e1703 100644 --- a/Makefile.cbm +++ b/Makefile.cbm @@ -354,6 +354,7 @@ PIPELINE_SRCS = \ src/pipeline/pass_similarity.c \ src/pipeline/pass_semantic_edges.c \ src/pipeline/pass_complexity.c \ + src/pipeline/pass_importance.c \ src/pipeline/pass_cross_repo.c \ src/pipeline/artifact.c \ src/pipeline/pass_ensemble_routing.c \ @@ -560,7 +561,7 @@ TEST_DISCOVER_SRCS = \ TEST_GRAPH_BUFFER_SRCS = tests/test_graph_buffer.c -TEST_PIPELINE_SRCS = tests/test_registry.c tests/test_pipeline.c tests/test_cross_repo.c tests/test_fqn.c tests/test_route_canon.c tests/test_path_alias.c tests/test_configlink.c tests/test_infrascan.c tests/test_worker_pool.c tests/test_parallel.c tests/test_index_resilience.c tests/test_index_format.c tests/test_call_reference_contract.c tests/repro/repro_call_scope_usages.c tests/repro/repro_call_argument_usages.c tests/repro/repro_reference_precision.c tests/repro/repro_lexical_binding_precision.c tests/repro/repro_call_argument_matrix_a.c tests/repro/repro_call_argument_matrix_b.c tests/repro/repro_call_node_behaviors.c tests/repro/repro_language_registry.c tests/repro/repro_call_node_manifest.c tests/repro/repro_lsp_ordered_signatures.c tests/repro/repro_lsp_ordered_local.c tests/repro/repro_ts_overload_return_chains.c tests/repro/repro_harness_cleanup.c tests/repro/repro_runner_filter.c +TEST_PIPELINE_SRCS = tests/test_registry.c tests/test_pipeline.c tests/test_importance.c tests/test_cross_repo.c tests/test_fqn.c tests/test_route_canon.c tests/test_path_alias.c tests/test_configlink.c tests/test_infrascan.c tests/test_worker_pool.c tests/test_parallel.c tests/test_index_resilience.c tests/test_index_format.c tests/test_call_reference_contract.c tests/repro/repro_call_scope_usages.c tests/repro/repro_call_argument_usages.c tests/repro/repro_reference_precision.c tests/repro/repro_lexical_binding_precision.c tests/repro/repro_call_argument_matrix_a.c tests/repro/repro_call_argument_matrix_b.c tests/repro/repro_call_node_behaviors.c tests/repro/repro_language_registry.c tests/repro/repro_call_node_manifest.c tests/repro/repro_lsp_ordered_signatures.c tests/repro/repro_lsp_ordered_local.c tests/repro/repro_ts_overload_return_chains.c tests/repro/repro_harness_cleanup.c tests/repro/repro_runner_filter.c TEST_WATCHER_SRCS = tests/test_watcher.c diff --git a/src/pipeline/pass_importance.c b/src/pipeline/pass_importance.c new file mode 100644 index 000000000..a727ddee0 --- /dev/null +++ b/src/pipeline/pass_importance.c @@ -0,0 +1,396 @@ +/* + * pass_importance.c — Index-time per-symbol importance score (weighted degree). + * + * Predump pass. Computes an importance score for every Function/Method/Class + * node and stores it as a numeric "importance" key inside the node's EXISTING + * properties_json blob — no new column, no schema change, and therefore no + * CBM_INDEX_FORMAT_VERSION bump. Indexes written by older builds simply lack + * the key; every consumer must tolerate its absence. + * + * Model (weighted degree, Aider-repomap shaped): + * + * importance = sqrt(num_refs) * priv * generic * distinct * test_penalty + * + * num_refs incoming CALLS + USAGE edges. 0 -> sqrt(0) = 0. + * priv 0.1 name is private (leading underscore) + * generic 0.1 the name is defined in >= 5 DISTINCT files + * distinct 10 name is snake_case or camelCase AND len >= 8 + * test_penalty + * 0.1 the symbol is test scaffolding — either the target of an + * incoming TESTS edge (a production helper exercised by a + * test), or it lives in a test file per the graph's own + * canonical cbm_is_test_path() classifier (the same one + * pass_tests uses). + * + * Transitive importance (PageRank) is deliberately NOT built here: weighted + * degree is what this pass claims, and a transitive refinement has to earn + * itself against a fixed judgment set before it ships. + * + * ── Cost ───────────────────────────────────────────────────────────── + * The generic-name multiplier needs |{ file a name is defined in }|. The + * obvious shape — for each node, walk its whole same-name group and + * pairwise-compare file paths — is O(k^2) per node and therefore O(k^3) per + * same-name group of size k. That is not a theoretical concern: on a large + * Java corpus a single name group (`toString`, k≈4.6k) costs ~2 minutes, and + * a handful of such names alone multiplies total index time several-fold. + * + * This implementation is linear in the graph instead: + * - the distinct-file count is computed ONCE PER DISTINCT NAME and memoized + * (g_imp_name_counts), so a group of size k is scanned once, not k times; + * - within a group, file paths are deduplicated through a hash set rather + * than pairwise strcmp. + * Total distinct-file work is therefore Σ over distinct names of k = O(N). + * Both gbuf lookups it rests on (find_by_name, find_edges_by_target_type) are + * hash-indexed, so the per-node loop is O(1) amortised per node. + * + * g_importance_name_visits counts same-name-group member visits and is GATED + * in the complexity suite: it must stay linear (ratio ~2) under a corpus + * doubling. A regression to the per-node scan lands at ~4 and fails the gate. + * + * ── Ordering (load-bearing) ────────────────────────────────────────── + * Registered LAST among the predump passes, and last in the incremental + * post-pass sequence, so every edge type the score reads (CALLS, USAGE from + * extraction; TESTS from pass_tests) already exists. A pass ordered earlier + * would silently score everything at num_refs = 0 with no test penalty. + */ +#include "foundation/constants.h" +#include "pipeline/pipeline.h" +#include "pipeline/pipeline_internal.h" +#include "graph_buffer/graph_buffer.h" +#include "foundation/hash_table.h" +#include "foundation/log.h" +#include "foundation/compat.h" +#include "cbm.h" + +#include +#include +#include +#include +#include +#include +#include +#include + +/* Work counters — deltas are read by the complexity suite's linearity gate. + * Never reset here: callers snapshot before/after so nested runs compose. */ +_Atomic uint64_t g_importance_nodes = 0; +_Atomic uint64_t g_importance_name_visits = 0; + +enum { + CBM_IMPORTANCE_GENERIC_MIN_FILES = 5, /* name defined in >= N files -> generic */ + CBM_IMPORTANCE_DISTINCT_MIN_LEN = 8, /* distinctive-identifier length floor */ +}; +static const double CBM_IMPORTANCE_PRIV_MUL = 0.1; +static const double CBM_IMPORTANCE_GENERIC_MUL = 0.1; +static const double CBM_IMPORTANCE_DISTINCT_MUL = 10.0; +static const double CBM_IMPORTANCE_TEST_MUL = 0.1; + +/* The node labels that carry an importance score. File/Module and other + * non-symbol nodes are deliberately excluded. */ +static const char *const CBM_IMPORTANCE_LABELS[] = {"Function", "Method", "Class"}; +enum { CBM_IMPORTANCE_LABEL_COUNT = 3 }; + +/* Int → string for structured logging (thread-safe ring buffer). */ +static const char *itoa_imp(uint64_t val) { + enum { RING = 2, MASK = 1 }; + static CBM_TLS char bufs[RING][CBM_SZ_32]; + static CBM_TLS int idx = 0; + int i = idx; + idx = (idx + 1) & MASK; + snprintf(bufs[i], sizeof(bufs[i]), "%llu", (unsigned long long)val); + return bufs[i]; +} + +static bool name_is_private(const char *name) { + return name != NULL && name[0] == '_'; +} + +/* Distinctive = (snake_case OR camelCase) AND length >= 8. snake_case is an + * embedded '_'; camelCase is a lower->upper "hump". A plain len>=8 lowercase + * word (no '_', no hump) is NOT distinctive. */ +static bool name_is_distinctive(const char *name) { + if (!name) { + return false; + } + size_t len = strlen(name); + if (len < (size_t)CBM_IMPORTANCE_DISTINCT_MIN_LEN) { + return false; + } + if (strchr(name, '_') != NULL) { + return true; /* snake_case */ + } + for (size_t i = 1; i < len; i++) { + if (islower((unsigned char)name[i - 1]) && isupper((unsigned char)name[i])) { + return true; /* camelCase hump */ + } + } + return false; +} + +/* ── JSON property write-back ───────────────────────────────────────── + * + * properties_json is a flat object here (same assumption the sibling passes' + * json_get_int/json_get_bool helpers make), but the value scanner below is + * written to survive nested values so a future richer blob cannot be + * truncated by an in-place overwrite. */ + +/* Return a pointer just past the JSON value beginning at p (first non-space + * char of the value). NULL if the value is unterminated/malformed. */ +static const char *imp_value_end(const char *p) { + if (*p == '"') { + p++; + while (*p) { + if (*p == '\\' && p[1]) { + p += 2; + continue; + } + if (*p == '"') { + return p + 1; + } + p++; + } + return NULL; + } + if (*p == '{' || *p == '[') { + int depth = 0; + bool in_str = false; + while (*p) { + if (in_str) { + if (*p == '\\' && p[1]) { + p += 2; + continue; + } + if (*p == '"') { + in_str = false; + } + } else if (*p == '"') { + in_str = true; + } else if (*p == '{' || *p == '[') { + depth++; + } else if (*p == '}' || *p == ']') { + depth--; + if (depth == 0) { + return p + 1; + } + } + p++; + } + return NULL; + } + while (*p && *p != ',' && *p != '}' && *p != ']') { + p++; + } + return p; +} + +/* Find `"key"` where it is genuinely in KEY position: preceded (modulo + * whitespace) by '{' or ',' and followed (modulo whitespace) by ':'. A bare + * strstr would also match the text inside a string VALUE. Returns a pointer + * to the opening quote, or NULL. */ +static const char *imp_find_key(const char *json, const char *key) { + char pat[CBM_SZ_64]; + int n = snprintf(pat, sizeof(pat), "\"%s\"", key); + if (n < 0 || (size_t)n >= sizeof(pat)) { + return NULL; + } + const char *p = json; + while ((p = strstr(p, pat)) != NULL) { + const char *before = p; + while (before > json && isspace((unsigned char)before[-1])) { + before--; + } + char prev = (before > json) ? before[-1] : '\0'; + const char *after = p + n; + while (isspace((unsigned char)*after)) { + after++; + } + if ((prev == '{' || prev == ',') && *after == ':') { + return p; + } + p += n; + } + return NULL; +} + +/* Write the numeric "importance" key into a node's properties JSON object. + * + * IDEMPOTENT BY CONTRACT. The incremental path rehydrates nodes straight out + * of the store (cbm_gbuf_load_from_db), and those nodes ALREADY carry an + * "importance" key from the previous run. A pure append would emit + * {"importance":1.0,...,"importance":2.0} — silent property corruption that + * no build or schema check would catch. So: overwrite an existing key in + * place, append only when the key is genuinely absent. + * + * A properties_json that is not a JSON object is left untouched. */ +void cbm_pipeline_importance_append_prop(cbm_gbuf_node_t *node, double score) { + if (!node) { + return; + } + const char *old = node->properties_json ? node->properties_json : "{}"; + size_t olen = strlen(old); + if (olen < 2 || old[olen - 1] != '}') { + return; /* not a JSON object — leave untouched */ + } + + char val[CBM_SZ_32]; + int vn = snprintf(val, sizeof(val), "%.6f", score); + if (vn < 0 || (size_t)vn >= sizeof(val)) { + return; + } + + const char *k = imp_find_key(old, "importance"); + if (k) { + const char *colon = strchr(k, ':'); + if (!colon) { + return; /* imp_find_key guarantees one, but never trust a raw scan */ + } + const char *v = colon + 1; + while (isspace((unsigned char)*v)) { + v++; + } + const char *vend = imp_value_end(v); + if (!vend) { + return; + } + size_t head = (size_t)(v - old); + size_t tail = strlen(vend); + char *neu = malloc(head + (size_t)vn + tail + 1); + if (!neu) { + return; + } + memcpy(neu, old, head); + memcpy(neu + head, val, (size_t)vn); + memcpy(neu + head + (size_t)vn, vend, tail + 1); + free(node->properties_json); + node->properties_json = neu; + return; + } + + char frag[CBM_SZ_64]; + int fn = snprintf(frag, sizeof(frag), "%s\"importance\":%s}", (olen == 2) ? "" : ",", val); + if (fn < 0 || (size_t)fn >= sizeof(frag)) { + return; + } + char *neu = malloc(olen - 1 + (size_t)fn + 1); + if (!neu) { + return; + } + memcpy(neu, old, olen - 1); /* copy without the trailing '}' */ + memcpy(neu + olen - 1, frag, (size_t)fn + 1); + free(node->properties_json); + node->properties_json = neu; +} + +/* ── Distinct-file counting (the linear core) ─────────────────────────── */ + +typedef struct { + CBMHashTable *counts; /* name -> (void *)(intptr_t)(distinct_files + 1) */ + CBMHashTable *paths; /* scratch file-path set, cleared between names */ +} imp_name_index_t; + +/* Distinct files a name is DEFINED in. Counts distinct file paths, not + * distinct nodes — two defs of one name in the same file count once. + * Computed once per distinct name; every later node in the group is a hash + * hit. Name keys are borrowed from the gbuf nodes, which outlive the pass. */ +static int imp_distinct_file_count(const cbm_gbuf_t *gb, const char *name, imp_name_index_t *ix) { + if (!name || !*name || !ix || !ix->counts) { + return 0; + } + void *cached = cbm_ht_get(ix->counts, name); + if (cached) { + return (int)((intptr_t)cached - 1); + } + + const cbm_gbuf_node_t **nodes = NULL; + int count = 0; + int distinct = 0; + if (cbm_gbuf_find_by_name(gb, name, &nodes, &count) == 0 && count > 0) { + atomic_fetch_add_explicit(&g_importance_name_visits, (uint64_t)count, memory_order_relaxed); + if (ix->paths) { + cbm_ht_clear(ix->paths); + for (int i = 0; i < count; i++) { + const char *fp = nodes[i]->file_path; + if (!fp || cbm_ht_has(ix->paths, fp)) { + continue; + } + cbm_ht_set(ix->paths, fp, (void *)(intptr_t)1); + distinct++; + } + } else { + /* Scratch set unavailable (allocation failure): the generic-name + * multiplier degrades to "never generic" rather than falling back + * to a pairwise scan — a silent quadratic is worse than a missing + * 0.1 multiplier. */ + distinct = 0; + } + } + cbm_ht_set(ix->counts, name, (void *)(intptr_t)(distinct + 1)); + return distinct; +} + +static int incoming_edge_count(const cbm_gbuf_t *gb, int64_t id, const char *type) { + const cbm_gbuf_edge_t **edges = NULL; + int ne = 0; + if (cbm_gbuf_find_edges_by_target_type(gb, id, type, &edges, &ne) != 0) { + return 0; + } + return ne; +} + +void cbm_pipeline_pass_importance(cbm_pipeline_ctx_t *ctx) { + if (!ctx || !ctx->gbuf) { + return; + } + cbm_gbuf_t *gb = ctx->gbuf; + + /* Modest reservations, deliberately not sized from node_count: the memo + * holds one entry per DISTINCT name, far fewer than nodes, and reserving + * buckets for every node would cost hundreds of MB of transient peak on a + * multi-million-node graph to save a handful of amortised rehashes. */ + imp_name_index_t ix = { + .counts = cbm_ht_create(CBM_SZ_4K), + .paths = cbm_ht_create(CBM_SZ_256), + }; + if (!ix.counts) { + cbm_ht_free(ix.paths); + cbm_log_error("pass.importance", "msg", "name_index_alloc_failed"); + return; + } + + uint64_t updated = 0; + for (int li = 0; li < CBM_IMPORTANCE_LABEL_COUNT; li++) { + const cbm_gbuf_node_t **nodes = NULL; + int count = 0; + if (cbm_gbuf_find_by_label(gb, CBM_IMPORTANCE_LABELS[li], &nodes, &count) != 0) { + continue; + } + for (int i = 0; i < count; i++) { + cbm_gbuf_node_t *n = (cbm_gbuf_node_t *)nodes[i]; + + int num_refs = + incoming_edge_count(gb, n->id, "CALLS") + incoming_edge_count(gb, n->id, "USAGE"); + double score = sqrt((double)num_refs); + + if (name_is_private(n->name)) { + score *= CBM_IMPORTANCE_PRIV_MUL; + } + if (imp_distinct_file_count(gb, n->name, &ix) >= CBM_IMPORTANCE_GENERIC_MIN_FILES) { + score *= CBM_IMPORTANCE_GENERIC_MUL; + } + if (name_is_distinctive(n->name)) { + score *= CBM_IMPORTANCE_DISTINCT_MUL; + } + if (cbm_is_test_path(n->file_path) || incoming_edge_count(gb, n->id, "TESTS") > 0) { + score *= CBM_IMPORTANCE_TEST_MUL; + } + + cbm_pipeline_importance_append_prop(n, score); + updated++; + } + } + + cbm_ht_free(ix.paths); + cbm_ht_free(ix.counts); + atomic_fetch_add_explicit(&g_importance_nodes, updated, memory_order_relaxed); + cbm_log_info("pass.importance", "symbols", itoa_imp(updated)); +} diff --git a/src/pipeline/pipeline.c b/src/pipeline/pipeline.c index 0c1d5eee4..091d8c665 100644 --- a/src/pipeline/pipeline.c +++ b/src/pipeline/pipeline.c @@ -946,6 +946,9 @@ static void predump_complexity(cbm_pipeline_ctx_t *ctx) { static void predump_ensemble(cbm_pipeline_ctx_t *ctx) { cbm_pipeline_pass_ensemble_routing(ctx); } +static void predump_importance(cbm_pipeline_ctx_t *ctx) { + cbm_pipeline_pass_importance(ctx); +} static void run_predump_passes(cbm_pipeline_t *p, cbm_pipeline_ctx_t *ctx) { static const struct { @@ -953,12 +956,23 @@ static void run_predump_passes(cbm_pipeline_t *p, cbm_pipeline_ctx_t *ctx) { const char *name; bool moderate_only; /* true = skip in fast mode */ } passes[] = { - {predump_deco, "decorator_tags", false}, {predump_cfg, "configlink", false}, - {predump_route, "route_match", false}, {predump_ensemble, "ensemble_routing", false}, - {predump_sim, "similarity", true}, {predump_sem, "semantic_edges", true}, + {predump_deco, "decorator_tags", false}, + {predump_cfg, "configlink", false}, + {predump_route, "route_match", false}, + {predump_ensemble, "ensemble_routing", false}, + {predump_sim, "similarity", true}, + {predump_sem, "semantic_edges", true}, {predump_complexity, "complexity", false}, + /* Importance runs LAST: it reads CALLS/USAGE (extraction) and TESTS + * (pass_tests, which run_post_extraction runs before this loop), so + * every edge type its score depends on already exists here. */ + {predump_importance, "importance", false}, }; - enum { PREDUMP_PASS_COUNT = 7 }; + /* Derived from the table, never hand-written. A hand-written count that + * lags a newly appended entry silently skips the LAST pass while every + * test stays green — exactly the failure this expression makes + * impossible. */ + enum { PREDUMP_PASS_COUNT = (int)(sizeof(passes) / sizeof(passes[0])) }; struct timespec t; for (int i = 0; i < PREDUMP_PASS_COUNT && !check_cancel(p); i++) { /* "moderate_only" passes (similarity/semantic edges) run in FULL, diff --git a/src/pipeline/pipeline_incremental.c b/src/pipeline/pipeline_incremental.c index fca1c8f06..5de6bfd6a 100644 --- a/src/pipeline/pipeline_incremental.c +++ b/src/pipeline/pipeline_incremental.c @@ -1456,6 +1456,20 @@ static int run_postpasses(cbm_pipeline_ctx_t *ctx, cbm_file_info_t *changed_file return rc < 0 ? rc : CBM_NOT_FOUND; } } + + /* Importance last, in EVERY mode — same ordering rule as the full path: + * it reads CALLS/USAGE (re-extraction, above) and TESTS (pass_tests, first + * in this function). The graph here is the rehydrated whole-project buffer, + * so nodes outside the changed set already carry an "importance" key from + * the previous run; cbm_pipeline_importance_append_prop overwrites it in + * place rather than appending a duplicate. */ + cbm_clock_gettime(CLOCK_MONOTONIC, &t); + cbm_pipeline_pass_importance(ctx); + cbm_log_info("pass.timing", "pass", "incr_importance", "elapsed_ms", + itoa_buf((int)elapsed_ms(t))); + if (cbm_pipeline_check_cancel(ctx)) { + return CBM_NOT_FOUND; + } return 0; } /* Publish the test-only legacy partial result through the same atomic diff --git a/src/pipeline/pipeline_internal.h b/src/pipeline/pipeline_internal.h index 754fa4ffa..a25573733 100644 --- a/src/pipeline/pipeline_internal.h +++ b/src/pipeline/pipeline_internal.h @@ -630,6 +630,31 @@ int cbm_pipeline_pass_semantic_edges(cbm_pipeline_ctx_t *ctx); * cycles (recursive). Runs on the graph buffer before the dump. */ void cbm_pipeline_pass_complexity(cbm_pipeline_ctx_t *ctx); +/* Pre-dump pass: per-symbol importance score (weighted degree). + * importance = sqrt(num_refs) * priv * generic * distinct * test_penalty + * Stored as a numeric "importance" key inside the node's EXISTING + * properties_json — no schema change and no index-format bump; indexes written + * by older builds simply lack the key and consumers must tolerate its absence. + * MUST run after pass_tests and after CALLS/USAGE extraction — a pass ordered + * earlier would see zero TESTS edges and num_refs = 0 everywhere. It is + * therefore registered last in run_predump_passes and last in the incremental + * post-pass sequence. */ +void cbm_pipeline_pass_importance(cbm_pipeline_ctx_t *ctx); + +/* Write the numeric "importance" key into a node's properties JSON object. + * IDEMPOTENT: an existing key is overwritten in place, never appended twice — + * the incremental path re-scores nodes rehydrated from the store that already + * carry the key. Non-static so the idempotence contract is directly testable. */ +void cbm_pipeline_importance_append_prop(cbm_gbuf_node_t *node, double score); + +/* Work counters for the importance pass (pass_importance.c). Deltas are read + * by the complexity suite's linearity gate; never reset by the pass itself, so + * nested/repeated runs compose. g_importance_name_visits counts same-name-group + * member visits during distinct-file counting — the quantity that goes + * superlinear if the per-distinct-name memoization is ever lost. */ +extern _Atomic uint64_t g_importance_nodes; +extern _Atomic uint64_t g_importance_name_visits; + /* ── Env URL scanner (pass_envscan.c) ────────────────────────────── */ typedef struct { diff --git a/tests/test_complexity.c b/tests/test_complexity.c index 1fe68a71e..d9c651ff9 100644 --- a/tests/test_complexity.c +++ b/tests/test_complexity.c @@ -353,6 +353,8 @@ typedef struct { uint64_t tail_lookups; uint64_t tail_candidates; uint64_t fallback_rows; + uint64_t imp_nodes; /* symbols scored by the importance pass */ + uint64_t imp_name_visits; /* same-name-group members visited by that pass */ double wall_s; } CxMetrics; @@ -373,6 +375,8 @@ static int cx_run(const char *root, const char *db_path, CxMetrics *out) { uint64_t tl0 = atomic_load_explicit(&g_lsp_tail_lookups, memory_order_relaxed); uint64_t tc0 = atomic_load_explicit(&g_lsp_tail_candidates, memory_order_relaxed); uint64_t fb0 = cbm_pp_lsp_linear_fallback_rows(); + uint64_t in0 = atomic_load_explicit(&g_importance_nodes, memory_order_relaxed); + uint64_t iv0 = atomic_load_explicit(&g_importance_name_visits, memory_order_relaxed); atomic_store_explicit(&g_cx_pass_count, 0, memory_order_relaxed); cbm_log_set_sink_ex(cx_pass_sink, CBM_LOG_SINK_TEE); @@ -407,6 +411,9 @@ static int cx_run(const char *root, const char *db_path, CxMetrics *out) { out->tail_lookups = atomic_load_explicit(&g_lsp_tail_lookups, memory_order_relaxed) - tl0; out->tail_candidates = atomic_load_explicit(&g_lsp_tail_candidates, memory_order_relaxed) - tc0; out->fallback_rows = cbm_pp_lsp_linear_fallback_rows() - fb0; + out->imp_nodes = atomic_load_explicit(&g_importance_nodes, memory_order_relaxed) - in0; + out->imp_name_visits = + atomic_load_explicit(&g_importance_name_visits, memory_order_relaxed) - iv0; cbm_store_t *s = cbm_store_open_path(db_path); if (!s) { @@ -717,9 +724,46 @@ TEST(complexity_throughput_report_written) { PASS(); } +/* Importance scoring must stay linear in the graph. Its generic-name + * multiplier needs |{files a name is defined in}|; computing that per NODE + * instead of once per distinct NAME makes a same-name group of size k cost + * O(k^3) — measured on a large Java corpus as a multi-minute cost for a + * handful of names. Under replication the same-name groups grow with the copy + * count, so the per-node shape lands at ~4x per doubling here while the + * memoized shape stays at ~2x. Gated on the counter ratio, never on wall time + * (O9). This counter is GATED, not merely recorded: the whole point of the + * work is that this quantity must not go superlinear. */ +TEST(complexity_importance_scoring_is_linear) { + if (cx_measure_pair() != 0) { + FAIL("failed to build/run the complexity corpus pair"); + } + const CxMetrics *a = &g_cx_base; + const CxMetrics *b = &g_cx_doubled; + + printf(" imp_nodes %llu -> %llu imp_name_visits %llu -> %llu\n", + (unsigned long long)a->imp_nodes, (unsigned long long)b->imp_nodes, + (unsigned long long)a->imp_name_visits, (unsigned long long)b->imp_name_visits); + + /* Non-vacuous, twice over: the pass must have run at all (a miscounted + * PREDUMP_PASS_COUNT would silently skip it and zero both counters), and + * the base leg must have produced enough work for a ratio to mean + * anything. A zero here is a wiring defect to fix, never a pass. */ + ASSERT_GT((long long)a->imp_nodes, 0); + ASSERT_GT((long long)a->imp_name_visits, (long long)CX_MIN_BASE_WORK); + + double node_r = cx_ratio((double)b->imp_nodes, (double)a->imp_nodes); + double visit_r = cx_ratio((double)b->imp_name_visits, (double)a->imp_name_visits); + printf(" imp_nodes ratio %.2f imp_name_visits ratio %.2f (linear ~2, per-node ~4)\n", + node_r, visit_r); + ASSERT_TRUE(node_r >= CX_RATIO_LO && node_r <= CX_RATIO_HI); + ASSERT_TRUE(visit_r <= CX_RATIO_HI); + PASS(); +} + SUITE(complexity) { RUN_TEST(complexity_replicated_modules_scale_linearly); RUN_TEST(complexity_perfile_registry_work_is_linear); + RUN_TEST(complexity_importance_scoring_is_linear); RUN_TEST(complexity_shared_package_growth_stays_linear); RUN_TEST(complexity_throughput_report_written); } diff --git a/tests/test_importance.c b/tests/test_importance.c new file mode 100644 index 000000000..9452a49e9 --- /dev/null +++ b/tests/test_importance.c @@ -0,0 +1,641 @@ +/* + * test_importance.c — index-time per-symbol importance score (pass_importance.c). + * + * Formula under test: + * importance = sqrt(num_refs) * priv * generic * distinct * test_penalty + * + * Three of these tests exist to catch failure modes that are SILENT — they + * produce a wrong index with a fully green build: + * + * - importance_pass_actually_runs_in_full_pipeline + * binds the predump registration. A hand-written PREDUMP_PASS_COUNT that + * lags the pass table skips the LAST-registered pass and nothing else + * notices. This test fails loudly if the score never lands. + * - importance_rehydrated_node_keeps_exactly_one_key + * - importance_second_index_run_keeps_exactly_one_key + * bind the idempotent write-back. The incremental path rehydrates nodes + * that already carry "importance"; a plain append yields + * {"importance":1.0,...,"importance":2.0} — property corruption, not a + * build failure. + * - importance_distinct_file_count_is_linear_in_group_size + * binds the cost shape: the distinct-file count must be computed once + * per distinct NAME, not once per node. The per-node shape is O(k^3) in + * a same-name group and is measurably disqualifying on real corpora. + */ +#include "test_framework.h" +#include "test_helpers.h" + +#include "pipeline/pipeline.h" +#include "pipeline/pipeline_internal.h" +#include "graph_buffer/graph_buffer.h" +#include "store/store.h" +#include "foundation/compat.h" +#include "foundation/log.h" + +#include +#include +#include +#include +#include +#include + +/* ── Shared helpers ───────────────────────────────────────────────────── */ + +/* Parse the numeric value of "importance" out of a properties_json blob. + * Returns a sentinel (-999.0) when the key is absent — callers that care about + * presence assert on imp_key_count() instead. */ +static double imp_value(const char *json) { + if (!json) { + return -999.0; + } + const char *p = strstr(json, "\"importance\":"); + if (!p) { + return -999.0; + } + return strtod(p + strlen("\"importance\":"), NULL); +} + +/* How many times the key appears. The duplicate-key corruption this suite + * guards against shows up here as 2. */ +static int imp_key_count(const char *json) { + if (!json) { + return 0; + } + int n = 0; + const char *p = json; + while ((p = strstr(p, "\"importance\":")) != NULL) { + n++; + p += strlen("\"importance\":"); + } + return n; +} + +static void imp_run_pass(cbm_gbuf_t *gb) { + atomic_int cancelled = 0; + cbm_pipeline_ctx_t ctx = { + .project_name = "test-proj", + .repo_path = "/tmp/test", + .gbuf = gb, + .cancelled = &cancelled, + }; + cbm_pipeline_pass_importance(&ctx); +} + +/* Every stored node of the given label must carry exactly `expect` copies of + * the key. Returns the number of nodes checked, or -1 on a store error. */ +static int imp_check_label(cbm_store_t *s, const char *project, const char *label, int expect) { + cbm_node_t *nodes = NULL; + int count = 0; + if (cbm_store_find_nodes_by_label(s, project, label, &nodes, &count) != CBM_STORE_OK) { + return -1; + } + int bad = 0; + for (int i = 0; i < count; i++) { + if (imp_key_count(nodes[i].properties_json) != expect) { + bad++; + printf(" %s node '%s' props=%s\n", label, nodes[i].name ? nodes[i].name : "?", + nodes[i].properties_json ? nodes[i].properties_json : "(null)"); + } + } + cbm_store_free_nodes(nodes, count); + return bad == 0 ? count : -1; +} + +/* ── Registration: the pass must actually execute ─────────────────────── */ + +/* A miscounted PREDUMP_PASS_COUNT silently drops the last-registered pass. + * The score is the only observable, so assert on the score. */ +TEST(importance_pass_actually_runs_in_full_pipeline) { + char *tmp = th_mktempdir("cbm_imp_run"); + if (!tmp) { + FAIL("tmpdir"); + } + char root[512]; + snprintf(root, sizeof(root), "%s", tmp); /* th_mktempdir returns a static buffer */ + + ASSERT_EQ(th_write_file(TH_PATH(root, "main.py"), + "def helper():\n return 1\n\n" + "def caller():\n return helper() + helper()\n\n" + "class Widget:\n def render(self):\n return helper()\n"), + 0); + + char db_path[600]; + snprintf(db_path, sizeof(db_path), "%s/graph.db", root); + cbm_pipeline_t *p = cbm_pipeline_new(root, db_path, CBM_MODE_FULL); + ASSERT_NOT_NULL(p); + ASSERT_EQ(cbm_pipeline_run(p), 0); + char project[256]; + snprintf(project, sizeof(project), "%s", cbm_pipeline_project_name(p)); + cbm_pipeline_free(p); + + cbm_store_t *s = cbm_store_open_path(db_path); + ASSERT_NOT_NULL(s); + + /* Non-vacuous: the corpus really produced symbols of each kind. */ + int fc = imp_check_label(s, project, "Function", 1); + int mc = imp_check_label(s, project, "Method", 1); + int cc = imp_check_label(s, project, "Class", 1); + printf(" scored Function=%d Method=%d Class=%d\n", fc, mc, cc); + ASSERT_GT(fc, 0); + ASSERT_GT(mc, 0); + ASSERT_GT(cc, 0); + + /* File nodes are deliberately excluded from scoring. */ + cbm_node_t *files = NULL; + int filec = 0; + ASSERT_EQ(cbm_store_find_nodes_by_label(s, project, "File", &files, &filec), CBM_STORE_OK); + for (int i = 0; i < filec; i++) { + ASSERT_EQ(imp_key_count(files[i].properties_json), 0); + } + cbm_store_free_nodes(files, filec); + + cbm_store_close(s); + th_rmtree(root); + PASS(); +} + +/* ── Formula ──────────────────────────────────────────────────────────── */ + +TEST(importance_base_is_sqrt_of_incoming_refs) { + cbm_gbuf_t *gb = cbm_gbuf_new("test-proj", "/tmp/test"); + ASSERT_NOT_NULL(gb); + + int64_t target = + cbm_gbuf_upsert_node(gb, "Function", "target", "pkg.target", "pkg/main.go", 1, 1, "{}"); + int64_t c1 = cbm_gbuf_upsert_node(gb, "Function", "c1", "pkg.c1", "pkg/main.go", 2, 2, "{}"); + int64_t c2 = cbm_gbuf_upsert_node(gb, "Function", "c2", "pkg.c2", "pkg/main.go", 3, 3, "{}"); + int64_t c3 = cbm_gbuf_upsert_node(gb, "Function", "c3", "pkg.c3", "pkg/main.go", 4, 4, "{}"); + int64_t lonely = + cbm_gbuf_upsert_node(gb, "Function", "lonely", "pkg.lonely", "pkg/main.go", 5, 5, "{}"); + ASSERT_GT(target, 0); + ASSERT_GT(lonely, 0); + + cbm_gbuf_insert_edge(gb, c1, target, "CALLS", "{}"); + cbm_gbuf_insert_edge(gb, c2, target, "CALLS", "{}"); + cbm_gbuf_insert_edge(gb, c3, target, "USAGE", "{}"); /* spans BOTH edge types */ + + imp_run_pass(gb); + + const cbm_gbuf_node_t *tn = cbm_gbuf_find_by_id(gb, target); + ASSERT_NOT_NULL(tn); + ASSERT_EQ(imp_key_count(tn->properties_json), 1); + ASSERT_FLOAT_EQ(imp_value(tn->properties_json), sqrt(3.0), 1e-6); + + /* num_refs == 0 -> sqrt(0) == 0, still written. */ + const cbm_gbuf_node_t *ln = cbm_gbuf_find_by_id(gb, lonely); + ASSERT_NOT_NULL(ln); + ASSERT_EQ(imp_key_count(ln->properties_json), 1); + ASSERT_FLOAT_EQ(imp_value(ln->properties_json), 0.0, 1e-9); + + cbm_gbuf_free(gb); + PASS(); +} + +TEST(importance_private_names_are_demoted) { + cbm_gbuf_t *gb = cbm_gbuf_new("test-proj", "/tmp/test"); + ASSERT_NOT_NULL(gb); + + int64_t priv = + cbm_gbuf_upsert_node(gb, "Function", "_run", "pkg._run", "pkg/main.go", 1, 1, "{}"); + int64_t pub = cbm_gbuf_upsert_node(gb, "Function", "run", "pkg.run", "pkg/main.go", 2, 2, "{}"); + for (int i = 0; i < 4; i++) { + char name[CBM_SZ_32]; + char qn[CBM_SZ_64]; + snprintf(name, sizeof(name), "p%d", i); + snprintf(qn, sizeof(qn), "pkg.p%d", i); + int64_t c = + cbm_gbuf_upsert_node(gb, "Function", name, qn, "pkg/main.go", 10 + i, 10 + i, "{}"); + cbm_gbuf_insert_edge(gb, c, priv, "CALLS", "{}"); + snprintf(name, sizeof(name), "q%d", i); + snprintf(qn, sizeof(qn), "pkg.q%d", i); + c = cbm_gbuf_upsert_node(gb, "Function", name, qn, "pkg/main.go", 20 + i, 20 + i, "{}"); + cbm_gbuf_insert_edge(gb, c, pub, "CALLS", "{}"); + } + + imp_run_pass(gb); + + ASSERT_FLOAT_EQ(imp_value(cbm_gbuf_find_by_id(gb, priv)->properties_json), sqrt(4.0) * 0.1, + 1e-6); + ASSERT_FLOAT_EQ(imp_value(cbm_gbuf_find_by_id(gb, pub)->properties_json), sqrt(4.0), 1e-6); + + cbm_gbuf_free(gb); + PASS(); +} + +/* Generic = defined in >= 5 DISTINCT FILES. Two defs of one name in the same + * file count once, so the boundary is tested on files, not on node count. */ +TEST(importance_generic_names_are_demoted_on_distinct_files) { + cbm_gbuf_t *gb = cbm_gbuf_new("test-proj", "/tmp/test"); + ASSERT_NOT_NULL(gb); + + /* "generic": 5 distinct files -> demoted. */ + int64_t generic = 0; + for (int i = 0; i < 5; i++) { + char qn[CBM_SZ_64]; + char file[CBM_SZ_64]; + snprintf(qn, sizeof(qn), "pkg%d.generic", i); + snprintf(file, sizeof(file), "pkg%d/main.go", i); + int64_t id = cbm_gbuf_upsert_node(gb, "Function", "generic", qn, file, 1, 1, "{}"); + if (i == 0) { + generic = id; + } + } + /* "narrow": 4 distinct files, but 6 nodes — two duplicates share a file, + * so the DISTINCT count is 4 and it stays undemoted. */ + int64_t narrow = 0; + for (int i = 0; i < 4; i++) { + char qn[CBM_SZ_64]; + char file[CBM_SZ_64]; + snprintf(qn, sizeof(qn), "n%d.narrow", i); + snprintf(file, sizeof(file), "n%d/main.go", i); + int64_t id = cbm_gbuf_upsert_node(gb, "Function", "narrow", qn, file, 1, 1, "{}"); + if (i == 0) { + narrow = id; + } + } + cbm_gbuf_upsert_node(gb, "Function", "narrow", "n0.narrow_b", "n0/main.go", 9, 9, "{}"); + cbm_gbuf_upsert_node(gb, "Function", "narrow", "n1.narrow_b", "n1/main.go", 9, 9, "{}"); + + for (int i = 0; i < 9; i++) { + char name[CBM_SZ_32]; + char qn[CBM_SZ_64]; + snprintf(name, sizeof(name), "z%d", i); + snprintf(qn, sizeof(qn), "z.z%d", i); + int64_t c = cbm_gbuf_upsert_node(gb, "Function", name, qn, "z/main.go", i, i, "{}"); + cbm_gbuf_insert_edge(gb, c, i < 4 ? generic : narrow, "CALLS", "{}"); + } + + imp_run_pass(gb); + + /* generic: sqrt(4) * 0.1; narrow: sqrt(5), undemoted. */ + ASSERT_FLOAT_EQ(imp_value(cbm_gbuf_find_by_id(gb, generic)->properties_json), sqrt(4.0) * 0.1, + 1e-6); + ASSERT_FLOAT_EQ(imp_value(cbm_gbuf_find_by_id(gb, narrow)->properties_json), sqrt(5.0), 1e-6); + + cbm_gbuf_free(gb); + PASS(); +} + +TEST(importance_distinctive_names_are_promoted) { + cbm_gbuf_t *gb = cbm_gbuf_new("test-proj", "/tmp/test"); + ASSERT_NOT_NULL(gb); + + /* snake_case >= 8, camelCase >= 8, and a plain long lowercase word that is + * NOT distinctive (no '_', no hump). */ + int64_t snake = cbm_gbuf_upsert_node(gb, "Function", "load_user_profile", + "pkg.load_user_profile", "pkg/a.go", 1, 1, "{}"); + int64_t camel = cbm_gbuf_upsert_node(gb, "Function", "loadUserProfile", "pkg.loadUserProfile", + "pkg/b.go", 1, 1, "{}"); + int64_t plain = + cbm_gbuf_upsert_node(gb, "Function", "aggregate", "pkg.aggregate", "pkg/c.go", 1, 1, "{}"); + int64_t shortnm = + cbm_gbuf_upsert_node(gb, "Function", "get_x", "pkg.get_x", "pkg/d.go", 1, 1, "{}"); + + const int64_t targets[] = {snake, camel, plain, shortnm}; + for (int t = 0; t < 4; t++) { + char name[CBM_SZ_32]; + char qn[CBM_SZ_64]; + snprintf(name, sizeof(name), "cc%d", t); + snprintf(qn, sizeof(qn), "z.cc%d", t); + int64_t c = cbm_gbuf_upsert_node(gb, "Function", name, qn, "z/main.go", t, t, "{}"); + cbm_gbuf_insert_edge(gb, c, targets[t], "CALLS", "{}"); + } + + imp_run_pass(gb); + + ASSERT_FLOAT_EQ(imp_value(cbm_gbuf_find_by_id(gb, snake)->properties_json), 1.0 * 10.0, 1e-6); + ASSERT_FLOAT_EQ(imp_value(cbm_gbuf_find_by_id(gb, camel)->properties_json), 1.0 * 10.0, 1e-6); + ASSERT_FLOAT_EQ(imp_value(cbm_gbuf_find_by_id(gb, plain)->properties_json), 1.0, 1e-6); + ASSERT_FLOAT_EQ(imp_value(cbm_gbuf_find_by_id(gb, shortnm)->properties_json), 1.0, 1e-6); + + cbm_gbuf_free(gb); + PASS(); +} + +/* Test scaffolding is demoted two ways: living in a test file (the graph's own + * cbm_is_test_path classifier) or being the target of a TESTS edge. */ +TEST(importance_test_scaffolding_is_demoted) { + cbm_gbuf_t *gb = cbm_gbuf_new("test-proj", "/tmp/test"); + ASSERT_NOT_NULL(gb); + + int64_t in_test_file = cbm_gbuf_upsert_node(gb, "Function", "fixture", "t.fixture", + "pkg/thing_test.go", 1, 1, "{}"); + int64_t tests_target = + cbm_gbuf_upsert_node(gb, "Function", "helper", "pkg.helper", "pkg/util.go", 1, 1, "{}"); + int64_t plain = + cbm_gbuf_upsert_node(gb, "Function", "worker", "pkg.worker", "pkg/util.go", 2, 2, "{}"); + /* A production file whose NAME merely contains "test" must NOT be demoted. */ + int64_t testutil = cbm_gbuf_upsert_node(gb, "Function", "spawn", "pkg.spawn", + "pkg/testutil_helpers.go", 1, 1, "{}"); + + const int64_t targets[] = {in_test_file, tests_target, plain, testutil}; + for (int t = 0; t < 4; t++) { + char name[CBM_SZ_32]; + char qn[CBM_SZ_64]; + snprintf(name, sizeof(name), "d%d", t); + snprintf(qn, sizeof(qn), "z.d%d", t); + int64_t c = cbm_gbuf_upsert_node(gb, "Function", name, qn, "z/main.go", t, t, "{}"); + cbm_gbuf_insert_edge(gb, c, targets[t], "CALLS", "{}"); + } + int64_t tfn = cbm_gbuf_upsert_node(gb, "Function", "TestHelper", "t.TestHelper", + "pkg/util_test.go", 5, 5, "{}"); + cbm_gbuf_insert_edge(gb, tfn, tests_target, "TESTS", "{}"); + + imp_run_pass(gb); + + ASSERT_FLOAT_EQ(imp_value(cbm_gbuf_find_by_id(gb, in_test_file)->properties_json), 1.0 * 0.1, + 1e-6); + ASSERT_FLOAT_EQ(imp_value(cbm_gbuf_find_by_id(gb, tests_target)->properties_json), 1.0 * 0.1, + 1e-6); + ASSERT_FLOAT_EQ(imp_value(cbm_gbuf_find_by_id(gb, plain)->properties_json), 1.0, 1e-6); + ASSERT_FLOAT_EQ(imp_value(cbm_gbuf_find_by_id(gb, testutil)->properties_json), 1.0, 1e-6); + + cbm_gbuf_free(gb); + PASS(); +} + +/* ── Idempotent write-back ────────────────────────────────────────────── */ + +TEST(importance_append_prop_overwrites_instead_of_duplicating) { + cbm_gbuf_t *gb = cbm_gbuf_new("test-proj", "/tmp/test"); + ASSERT_NOT_NULL(gb); + int64_t id = cbm_gbuf_upsert_node(gb, "Function", "f", "p.f", "p/a.go", 1, 1, + "{\"loop_depth\":2,\"recursive\":false}"); + cbm_gbuf_node_t *n = (cbm_gbuf_node_t *)cbm_gbuf_find_by_id(gb, id); + ASSERT_NOT_NULL(n); + + cbm_pipeline_importance_append_prop(n, 1.5); + ASSERT_EQ(imp_key_count(n->properties_json), 1); + ASSERT_FLOAT_EQ(imp_value(n->properties_json), 1.5, 1e-6); + + cbm_pipeline_importance_append_prop(n, 42.25); + printf(" after second write: %s\n", n->properties_json); + ASSERT_EQ(imp_key_count(n->properties_json), 1); + ASSERT_FLOAT_EQ(imp_value(n->properties_json), 42.25, 1e-6); + + /* Sibling keys survive the in-place overwrite, on both sides of the key. */ + ASSERT_TRUE(strstr(n->properties_json, "\"loop_depth\":2") != NULL); + ASSERT_TRUE(strstr(n->properties_json, "\"recursive\":false") != NULL); + + /* A key that is NOT last: overwrite must not truncate the tail. */ + free(n->properties_json); + n->properties_json = strdup("{\"importance\":0.000000,\"tail\":7}"); + ASSERT_NOT_NULL(n->properties_json); + cbm_pipeline_importance_append_prop(n, 3.0); + ASSERT_EQ(imp_key_count(n->properties_json), 1); + ASSERT_FLOAT_EQ(imp_value(n->properties_json), 3.0, 1e-6); + ASSERT_TRUE(strstr(n->properties_json, "\"tail\":7") != NULL); + + /* A string VALUE that merely contains the key text is not a key. */ + free(n->properties_json); + n->properties_json = strdup("{\"doc\":\"see \\\"importance\\\": below\"}"); + ASSERT_NOT_NULL(n->properties_json); + cbm_pipeline_importance_append_prop(n, 2.0); + ASSERT_FLOAT_EQ(imp_value(n->properties_json), 2.0, 1e-6); + ASSERT_TRUE(strstr(n->properties_json, "\"doc\":") != NULL); + + /* Empty object and a non-object blob. */ + free(n->properties_json); + n->properties_json = strdup("{}"); + ASSERT_NOT_NULL(n->properties_json); + cbm_pipeline_importance_append_prop(n, 0.5); + ASSERT_STR_EQ(n->properties_json, "{\"importance\":0.500000}"); + + free(n->properties_json); + n->properties_json = strdup("not json"); + ASSERT_NOT_NULL(n->properties_json); + cbm_pipeline_importance_append_prop(n, 9.0); + ASSERT_STR_EQ(n->properties_json, "not json"); + + cbm_gbuf_free(gb); + PASS(); +} + +/* Running the pass twice over the same graph — the shape the incremental path + * produces when it re-scores an already-scored rehydrated buffer. */ +TEST(importance_pass_is_idempotent_across_reruns) { + cbm_gbuf_t *gb = cbm_gbuf_new("test-proj", "/tmp/test"); + ASSERT_NOT_NULL(gb); + int64_t t = + cbm_gbuf_upsert_node(gb, "Function", "target", "pkg.target", "pkg/main.go", 1, 1, "{}"); + int64_t c = + cbm_gbuf_upsert_node(gb, "Function", "caller", "pkg.caller", "pkg/main.go", 2, 2, "{}"); + cbm_gbuf_insert_edge(gb, c, t, "CALLS", "{}"); + + imp_run_pass(gb); + char first[CBM_SZ_512]; + snprintf(first, sizeof(first), "%s", cbm_gbuf_find_by_id(gb, t)->properties_json); + imp_run_pass(gb); + const char *second = cbm_gbuf_find_by_id(gb, t)->properties_json; + + printf(" run1=%s run2=%s\n", first, second); + ASSERT_EQ(imp_key_count(second), 1); + ASSERT_STR_EQ(first, second); /* re-scoring is a fixed point */ + + cbm_gbuf_free(gb); + PASS(); +} + +/* The literal incremental shape: index, rehydrate the stored graph exactly as + * pipeline_incremental.c does, re-run the pass. Nodes arrive already carrying + * the key — an append-only write-back yields two. */ +TEST(importance_rehydrated_node_keeps_exactly_one_key) { + char *tmp = th_mktempdir("cbm_imp_rehy"); + if (!tmp) { + FAIL("tmpdir"); + } + char root[512]; + snprintf(root, sizeof(root), "%s", tmp); /* th_mktempdir returns a static buffer */ + + ASSERT_EQ(th_write_file(TH_PATH(root, "main.py"), + "def helper():\n return 1\n\n" + "def caller():\n return helper() + helper()\n"), + 0); + char db_path[600]; + snprintf(db_path, sizeof(db_path), "%s/graph.db", root); + cbm_pipeline_t *p = cbm_pipeline_new(root, db_path, CBM_MODE_FULL); + ASSERT_NOT_NULL(p); + ASSERT_EQ(cbm_pipeline_run(p), 0); + char project[256]; + snprintf(project, sizeof(project), "%s", cbm_pipeline_project_name(p)); + cbm_pipeline_free(p); + + cbm_gbuf_t *gb = cbm_gbuf_new(project, root); + ASSERT_NOT_NULL(gb); + ASSERT_EQ(cbm_gbuf_load_from_db(gb, db_path, project), 0); + + /* Precondition: the rehydrated nodes already carry the key. Without this + * the test could pass while proving nothing. */ + const cbm_gbuf_node_t **fns = NULL; + int fc = 0; + ASSERT_EQ(cbm_gbuf_find_by_label(gb, "Function", &fns, &fc), 0); + ASSERT_GT(fc, 0); + for (int i = 0; i < fc; i++) { + ASSERT_EQ(imp_key_count(fns[i]->properties_json), 1); + } + + imp_run_pass(gb); + + ASSERT_EQ(cbm_gbuf_find_by_label(gb, "Function", &fns, &fc), 0); + ASSERT_GT(fc, 0); + for (int i = 0; i < fc; i++) { + if (imp_key_count(fns[i]->properties_json) != 1) { + printf(" duplicate key on '%s': %s\n", fns[i]->name, fns[i]->properties_json); + } + ASSERT_EQ(imp_key_count(fns[i]->properties_json), 1); + } + + cbm_gbuf_free(gb); + th_rmtree(root); + PASS(); +} + +/* Pass-timing observer: which importance-pass call sites fired. */ +static atomic_int g_saw_incr_importance = 0; +static atomic_int g_saw_full_importance = 0; +static void imp_pass_sink(const char *line) { + if (!line || !strstr(line, "pass.timing")) { + return; + } + if (strstr(line, "pass=incr_importance")) { + atomic_fetch_add_explicit(&g_saw_incr_importance, 1, memory_order_relaxed); + } else if (strstr(line, "pass=importance")) { + atomic_fetch_add_explicit(&g_saw_full_importance, 1, memory_order_relaxed); + } +} + +/* End-to-end, on the incremental route that actually rehydrates the stored + * graph (LEGACY_PARTIAL — production's other incremental route, closure + * repair, works on a delta buffer and never sees pre-scored nodes). Index, + * edit a file, index again: the run must be observed on that route, the pass + * must have run on it, and every stored symbol must carry exactly one key. An + * append-only write-back produces two here. */ +TEST(importance_second_index_run_keeps_exactly_one_key) { + char *tmp = th_mktempdir("cbm_imp_incr"); + if (!tmp) { + FAIL("tmpdir"); + } + char root[512]; + snprintf(root, sizeof(root), "%s", tmp); /* th_mktempdir returns a static buffer */ + + ASSERT_EQ(th_write_file(TH_PATH(root, "a.py"), "def helper():\n return 1\n"), 0); + ASSERT_EQ( + th_write_file(TH_PATH(root, "b.py"), "import a\n\ndef caller():\n return a.helper()\n"), + 0); + + char db_path[600]; + snprintf(db_path, sizeof(db_path), "%s/graph.db", root); + + atomic_store(&g_saw_incr_importance, 0); + atomic_store(&g_saw_full_importance, 0); + cbm_log_set_sink_ex(imp_pass_sink, CBM_LOG_SINK_TEE); + + cbm_pipeline_t *p1 = cbm_pipeline_new(root, db_path, CBM_MODE_FULL); + ASSERT_NOT_NULL(p1); + ASSERT_EQ(cbm_pipeline_run(p1), 0); + char project[256]; + snprintf(project, sizeof(project), "%s", cbm_pipeline_project_name(p1)); + cbm_pipeline_free(p1); + + /* Edit one file so the second run has real work but stays incremental. */ + ASSERT_EQ(th_write_file(TH_PATH(root, "b.py"), + "import a\n\ndef caller():\n return a.helper() + a.helper()\n"), + 0); + + cbm_pipeline_incremental_test_force_legacy_partial_once(); + cbm_pipeline_t *p2 = cbm_pipeline_new(root, db_path, CBM_MODE_FULL); + ASSERT_NOT_NULL(p2); + ASSERT_EQ(cbm_pipeline_run(p2), 0); + cbm_incremental_route_t route = cbm_pipeline_incremental_test_last_route(); + cbm_pipeline_free(p2); + cbm_log_set_sink(NULL); + + /* Route proof: without it a silent fall-back to a full reindex would make + * the duplicate-key assertion below vacuous. */ + ASSERT_EQ((int)route, (int)CBM_INCREMENTAL_ROUTE_LEGACY_PARTIAL); + + int incr = atomic_load(&g_saw_incr_importance); + int full = atomic_load(&g_saw_full_importance); + printf(" importance pass runs: full=%d incremental=%d\n", full, incr); + ASSERT_GT(full, 0); /* first run scored via the predump table */ + /* The second run must have re-scored too — through whichever path it took. + * If it was NOT incremental the duplicate-key trap is untested, so say so + * rather than passing quietly. */ + if (incr == 0) { + th_rmtree(root); + FAIL("second run did not re-score on the incremental path — trap untested"); + } + + cbm_store_t *s = cbm_store_open_path(db_path); + ASSERT_NOT_NULL(s); + int fc = imp_check_label(s, project, "Function", 1); + printf(" functions with exactly one importance key: %d\n", fc); + ASSERT_GT(fc, 0); + cbm_store_close(s); + + th_rmtree(root); + PASS(); +} + +/* ── Cost shape ───────────────────────────────────────────────────────── */ + +/* The distinct-file count must be computed once per distinct NAME. Computing + * it per NODE makes a same-name group of size k cost O(k^3) — measurably + * disqualifying on real corpora. Assert on the pass's own work counter, not + * on wall time: doubling a same-name group must roughly double the work. */ +TEST(importance_distinct_file_count_is_linear_in_group_size) { + enum { K = 200 }; + + uint64_t visits[2]; + for (int leg = 0; leg < 2; leg++) { + int k = (leg == 0) ? K : 2 * K; + cbm_gbuf_t *gb = cbm_gbuf_new("test-proj", "/tmp/test"); + ASSERT_NOT_NULL(gb); + for (int i = 0; i < k; i++) { + char qn[CBM_SZ_64]; + char file[CBM_SZ_64]; + snprintf(qn, sizeof(qn), "pkg%d.toString", i); + snprintf(file, sizeof(file), "pkg%d/a.java", i); + ASSERT_GT(cbm_gbuf_upsert_node(gb, "Method", "toString", qn, file, 1, 1, "{}"), 0); + } + uint64_t v0 = atomic_load_explicit(&g_importance_name_visits, memory_order_relaxed); + imp_run_pass(gb); + visits[leg] = atomic_load_explicit(&g_importance_name_visits, memory_order_relaxed) - v0; + cbm_gbuf_free(gb); + } + + double ratio = (visits[0] > 0) ? (double)visits[1] / (double)visits[0] : 0.0; + printf(" name visits k=%d -> %llu, k=%d -> %llu (ratio %.2f; linear ~2, per-node ~4)\n", K, + (unsigned long long)visits[0], 2 * K, (unsigned long long)visits[1], ratio); + + /* Non-vacuous: the group really was scanned. */ + ASSERT_GT((long long)visits[0], 0); + /* Memoized: one scan of the whole group, so visits == k exactly. */ + ASSERT_EQ((long long)visits[0], (long long)K); + ASSERT_EQ((long long)visits[1], (long long)(2 * K)); + ASSERT_TRUE(ratio <= 2.75); + PASS(); +} + +/* ── Suite ────────────────────────────────────────────────────────────── */ + +SUITE(importance) { + /* Registration — catches a miscounted PREDUMP_PASS_COUNT. */ + RUN_TEST(importance_pass_actually_runs_in_full_pipeline); + + /* Formula. */ + RUN_TEST(importance_base_is_sqrt_of_incoming_refs); + RUN_TEST(importance_private_names_are_demoted); + RUN_TEST(importance_generic_names_are_demoted_on_distinct_files); + RUN_TEST(importance_distinctive_names_are_promoted); + RUN_TEST(importance_test_scaffolding_is_demoted); + + /* Idempotent write-back — the incremental duplicate-key trap. */ + RUN_TEST(importance_append_prop_overwrites_instead_of_duplicating); + RUN_TEST(importance_pass_is_idempotent_across_reruns); + RUN_TEST(importance_rehydrated_node_keeps_exactly_one_key); + RUN_TEST(importance_second_index_run_keeps_exactly_one_key); + + /* Cost shape. */ + RUN_TEST(importance_distinct_file_count_is_linear_in_group_size); +} diff --git a/tests/test_main.c b/tests/test_main.c index 0e478ccad..2d57f5468 100644 --- a/tests/test_main.c +++ b/tests/test_main.c @@ -768,6 +768,7 @@ extern void suite_discover(void); extern void suite_graph_buffer(void); extern void suite_registry(void); extern void suite_pipeline(void); +extern void suite_importance(void); extern void suite_pipeline_semantic_manifest_repro(void); extern void suite_cross_repo(void); extern void suite_index_resilience(void); @@ -1041,6 +1042,7 @@ int main(int argc, char **argv) { /* Pipeline (M8) */ RUN_SELECTED_SUITE(registry); RUN_SELECTED_SUITE(pipeline); + RUN_SELECTED_SUITE(importance); RUN_SELECTED_SUITE(index_format); RUN_SELECTED_SUITE(pipeline_semantic_manifest_repro); RUN_SELECTED_SUITE(call_reference_contract); From 396ac348b5abe0e1a7c6cc7bb9d604e8ee76cefc Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 28 Aug 2026 20:56:33 +0200 Subject: [PATCH 2/2] fix(pipeline): score importance over the staging store on the delta route The closure-delta incremental route scored importance off a graph that does not contain the project's edges, and persisted the result. cbm_delta_preseed fills that route's gbuf with PROXY nodes -- id, label, name, qualified name, file path, and nothing else. Only the re-extracted files carry real edges; every inbound edge from an unchanged file is snapshotted and re-linked later, inside cbm_delta_patch. Scoring in run_postpasses therefore read an in-degree of zero for symbols with hundreds of real callers, and because the repaired files' rows are purged and re-inserted, cbm_delta_patch wrote those zeros to disk. Measured on a fixture whose `helper` has 25 callers: full index 5.000000, after one closure-delta re-index 0.000000. closure_try_plan runs first, so this is the preferred warm path, not an edge case. The comment at the call site claimed a rehydrated whole-project buffer; that is true only of the legacy-partial route, and it is now stated correctly. The fix rescores in SQL over the staging store, immediately after cbm_delta_patch has inserted the new nodes and re-linked the inbound edges -- the first moment on this route where the complete graph is queryable. Loading the full graph into RAM instead would have given back the point of the delta route. Rescoring is project-wide, not limited to the changed files. Distinct-files- per-name and in-degree are both GLOBAL inputs: a body-only edit in one file can change the score of a symbol defined in a file that did not change. A scoped rescore would leave those stale. TWO ROUTES, ONE RULE. The scoring rule and the JSON write-back are not reimplemented in SQL. cbm_pipeline_importance_score() and cbm_pipeline_importance_set_prop() are now single definitions that both routes call -- the SQL side reaches them through two SQLite user-defined functions, so the same C code runs either way and the rule cannot drift. SQL contributes only the three aggregates, each an indexed GROUP BY. What remains free is the input GATHERING, and tests bind that directly: a full index and a closure-delta incremental index converging on the same tree must produce identical scores for every symbol. Both new tests assert the route was actually taken, so neither can pass by silently running some other path. Cost, measured on a 666 MB Java corpus: cold full index unchanged at 83 s (the recompute does not run on that path); warm closure-delta re-index 20 s without the recompute, 26 s with it. The delta route keeps its advantage over a full reindex (26 s vs 83 s). Signed-off-by: Martin Vogel --- src/pipeline/pass_importance.c | 242 ++++++++++++++++++++----- src/pipeline/pipeline_incremental.c | 57 ++++-- src/pipeline/pipeline_internal.h | 38 +++- tests/test_importance.c | 270 ++++++++++++++++++++++++++++ 4 files changed, 548 insertions(+), 59 deletions(-) diff --git a/src/pipeline/pass_importance.c b/src/pipeline/pass_importance.c index a727ddee0..c544d0a10 100644 --- a/src/pipeline/pass_importance.c +++ b/src/pipeline/pass_importance.c @@ -60,7 +60,9 @@ #include "foundation/hash_table.h" #include "foundation/log.h" #include "foundation/compat.h" +#include "store/store.h" #include "cbm.h" +#include "sqlite3.h" #include #include @@ -75,6 +77,7 @@ * Never reset here: callers snapshot before/after so nested runs compose. */ _Atomic uint64_t g_importance_nodes = 0; _Atomic uint64_t g_importance_name_visits = 0; +_Atomic uint64_t g_importance_store_rows = 0; enum { CBM_IMPORTANCE_GENERIC_MIN_FILES = 5, /* name defined in >= N files -> generic */ @@ -212,37 +215,39 @@ static const char *imp_find_key(const char *json, const char *key) { return NULL; } -/* Write the numeric "importance" key into a node's properties JSON object. +/* Produce a copy of `json` carrying the numeric "importance" key set to + * `score`, or NULL if `json` is not a JSON object (caller leaves it alone) or + * on allocation failure. Caller owns the result. * - * IDEMPOTENT BY CONTRACT. The incremental path rehydrates nodes straight out - * of the store (cbm_gbuf_load_from_db), and those nodes ALREADY carry an - * "importance" key from the previous run. A pure append would emit - * {"importance":1.0,...,"importance":2.0} — silent property corruption that - * no build or schema check would catch. So: overwrite an existing key in - * place, append only when the key is genuinely absent. + * IDEMPOTENT BY CONTRACT. Nodes reach this function already carrying an + * "importance" key on every re-scoring route: the legacy-partial incremental + * path rehydrates them via cbm_gbuf_load_from_db, and the store-level + * recompute reads them straight out of SQLite. A pure append would emit + * {"importance":1.0,...,"importance":2.0} — silent property corruption that no + * build or schema check would catch. So: overwrite an existing key in place, + * append only when the key is genuinely absent. * - * A properties_json that is not a JSON object is left untouched. */ -void cbm_pipeline_importance_append_prop(cbm_gbuf_node_t *node, double score) { - if (!node) { - return; - } - const char *old = node->properties_json ? node->properties_json : "{}"; + * SINGLE DEFINITION. Both scoring routes (the in-memory gbuf pass and the + * SQL-level store recompute) write through this one function, so the JSON + * shape cannot drift between them. */ +char *cbm_pipeline_importance_set_prop(const char *json, double score) { + const char *old = json ? json : "{}"; size_t olen = strlen(old); if (olen < 2 || old[olen - 1] != '}') { - return; /* not a JSON object — leave untouched */ + return NULL; /* not a JSON object — leave untouched */ } char val[CBM_SZ_32]; int vn = snprintf(val, sizeof(val), "%.6f", score); if (vn < 0 || (size_t)vn >= sizeof(val)) { - return; + return NULL; } const char *k = imp_find_key(old, "importance"); if (k) { const char *colon = strchr(k, ':'); if (!colon) { - return; /* imp_find_key guarantees one, but never trust a raw scan */ + return NULL; /* imp_find_key guarantees one, but never trust a raw scan */ } const char *v = colon + 1; while (isspace((unsigned char)*v)) { @@ -250,37 +255,77 @@ void cbm_pipeline_importance_append_prop(cbm_gbuf_node_t *node, double score) { } const char *vend = imp_value_end(v); if (!vend) { - return; + return NULL; } size_t head = (size_t)(v - old); size_t tail = strlen(vend); char *neu = malloc(head + (size_t)vn + tail + 1); if (!neu) { - return; + return NULL; } memcpy(neu, old, head); memcpy(neu + head, val, (size_t)vn); memcpy(neu + head + (size_t)vn, vend, tail + 1); - free(node->properties_json); - node->properties_json = neu; - return; + return neu; } char frag[CBM_SZ_64]; int fn = snprintf(frag, sizeof(frag), "%s\"importance\":%s}", (olen == 2) ? "" : ",", val); if (fn < 0 || (size_t)fn >= sizeof(frag)) { - return; + return NULL; } char *neu = malloc(olen - 1 + (size_t)fn + 1); if (!neu) { - return; + return NULL; } memcpy(neu, old, olen - 1); /* copy without the trailing '}' */ memcpy(neu + olen - 1, frag, (size_t)fn + 1); + return neu; +} + +/* gbuf-node convenience wrapper around cbm_pipeline_importance_set_prop. */ +void cbm_pipeline_importance_append_prop(cbm_gbuf_node_t *node, double score) { + if (!node) { + return; + } + char *neu = cbm_pipeline_importance_set_prop(node->properties_json, score); + if (!neu) { + return; + } free(node->properties_json); node->properties_json = neu; } +/* ── The scoring rule ───────────────────────────────────────────────── + * + * SINGLE DEFINITION, deliberately. There are two routes that score nodes — + * the in-memory gbuf pass (full index, legacy-partial incremental) and the + * SQL-level recompute over the staging store (closure-delta incremental) — + * and they gather their inputs very differently. Only the GATHERING differs; + * the rule itself lives here alone, so the two routes cannot drift apart in + * what a score means. The equivalence test in tests/test_importance.c guards + * the remaining freedom: that both gatherings produce the same inputs for the + * same graph. */ +double cbm_pipeline_importance_score(const cbm_importance_inputs_t *in) { + if (!in) { + return 0.0; + } + double score = sqrt((double)(in->num_refs > 0 ? in->num_refs : 0)); + if (name_is_private(in->name)) { + score *= CBM_IMPORTANCE_PRIV_MUL; + } + if (in->name_distinct_files >= CBM_IMPORTANCE_GENERIC_MIN_FILES) { + score *= CBM_IMPORTANCE_GENERIC_MUL; + } + if (name_is_distinctive(in->name)) { + score *= CBM_IMPORTANCE_DISTINCT_MUL; + } + if (cbm_is_test_path(in->file_path) || in->tests_target) { + score *= CBM_IMPORTANCE_TEST_MUL; + } + return score; +} + /* ── Distinct-file counting (the linear core) ─────────────────────────── */ typedef struct { @@ -367,24 +412,16 @@ void cbm_pipeline_pass_importance(cbm_pipeline_ctx_t *ctx) { for (int i = 0; i < count; i++) { cbm_gbuf_node_t *n = (cbm_gbuf_node_t *)nodes[i]; - int num_refs = - incoming_edge_count(gb, n->id, "CALLS") + incoming_edge_count(gb, n->id, "USAGE"); - double score = sqrt((double)num_refs); - - if (name_is_private(n->name)) { - score *= CBM_IMPORTANCE_PRIV_MUL; - } - if (imp_distinct_file_count(gb, n->name, &ix) >= CBM_IMPORTANCE_GENERIC_MIN_FILES) { - score *= CBM_IMPORTANCE_GENERIC_MUL; - } - if (name_is_distinctive(n->name)) { - score *= CBM_IMPORTANCE_DISTINCT_MUL; - } - if (cbm_is_test_path(n->file_path) || incoming_edge_count(gb, n->id, "TESTS") > 0) { - score *= CBM_IMPORTANCE_TEST_MUL; - } - - cbm_pipeline_importance_append_prop(n, score); + /* Gather only — the rule itself lives in one place. */ + cbm_importance_inputs_t in = { + .name = n->name, + .file_path = n->file_path, + .num_refs = incoming_edge_count(gb, n->id, "CALLS") + + incoming_edge_count(gb, n->id, "USAGE"), + .name_distinct_files = imp_distinct_file_count(gb, n->name, &ix), + .tests_target = incoming_edge_count(gb, n->id, "TESTS") > 0, + }; + cbm_pipeline_importance_append_prop(n, cbm_pipeline_importance_score(&in)); updated++; } } @@ -394,3 +431,126 @@ void cbm_pipeline_pass_importance(cbm_pipeline_ctx_t *ctx) { atomic_fetch_add_explicit(&g_importance_nodes, updated, memory_order_relaxed); cbm_log_info("pass.importance", "symbols", itoa_imp(updated)); } + +/* ── Store-level recompute (closure-delta incremental) ──────────────── + * + * WHY THIS EXISTS. The closure-delta route never materialises the project + * graph in RAM: cbm_delta_preseed fills the delta gbuf with *proxy* nodes + * (id, label, name, qn, file_path — no edges, empty properties), and only the + * re-extracted files' symbols carry real edges. Scoring there reads a + * near-zero in-degree for a symbol referenced hundreds of times project-wide, + * and cbm_delta_patch then persists exactly those fresh rows. Loading the full + * graph to fix that would give back the point of the delta route, so the + * recompute runs where the complete graph already exists: in SQL, over the + * staging store, AFTER cbm_delta_patch has inserted the new nodes and + * re-linked the snapshotted inbound edges. + * + * WHY IT CANNOT DRIFT FROM THE IN-MEMORY PASS. The scoring rule and the JSON + * write-back are not reimplemented here. They are registered as SQLite + * user-defined functions that call cbm_pipeline_importance_score() and + * cbm_pipeline_importance_set_prop() — the same C code the gbuf pass runs. + * SQL supplies only the three aggregates (in-degree, TESTS presence, distinct + * files per name), each one indexed GROUP BY. What remains free is the + * GATHERING, and the full-vs-incremental equivalence test guards that. + * + * The whole project is rescored, not just the changed files: distinct-files- + * per-name is a global input, so editing one file can legitimately move a name + * across the generic-name threshold for every other definition of that name. A + * scoped recompute would leave those stale — and would fail the equivalence + * test, correctly. */ + +/* UDF: cbm_imp_score(name, file_path, num_refs, name_files, tests_target). */ +static void sql_importance_score(sqlite3_context *c, int argc, sqlite3_value **argv) { + if (argc != 5) { + sqlite3_result_error(c, "cbm_imp_score arity", -1); + return; + } + cbm_importance_inputs_t in = { + .name = (const char *)sqlite3_value_text(argv[0]), + .file_path = (const char *)sqlite3_value_text(argv[1]), + .num_refs = sqlite3_value_int(argv[2]), + .name_distinct_files = sqlite3_value_int(argv[3]), + .tests_target = sqlite3_value_int(argv[4]) != 0, + }; + sqlite3_result_double(c, cbm_pipeline_importance_score(&in)); +} + +/* UDF: cbm_imp_set(properties, score) → properties with "importance" set. + * A non-object blob yields NULL; the UPDATE's COALESCE then leaves that row + * untouched — the same tolerance the in-memory writer has. */ +static void sql_importance_set(sqlite3_context *c, int argc, sqlite3_value **argv) { + if (argc != 2) { + sqlite3_result_error(c, "cbm_imp_set arity", -1); + return; + } + const char *props = (const char *)sqlite3_value_text(argv[0]); + char *neu = cbm_pipeline_importance_set_prop(props, sqlite3_value_double(argv[1])); + if (!neu) { + sqlite3_result_null(c); + return; + } + sqlite3_result_text(c, neu, -1, SQLITE_TRANSIENT); + free(neu); +} + +/* The aggregate CTE is MATERIALIZED on purpose: it reads `nodes` while the + * UPDATE writes `nodes`, and materialising settles that ordering explicitly + * rather than relying on the planner to choose it. */ +static const char *const kImportanceRecomputeSQL = + "WITH agg AS MATERIALIZED (" + " SELECT n.id AS id," + " cbm_imp_score(n.name, n.file_path, COALESCE(d.refs, 0)," + " COALESCE(f.files, 0), COALESCE(d.tests, 0)) AS score" + " FROM nodes n" + " LEFT JOIN (SELECT target_id AS id," + " SUM(CASE WHEN type IN ('CALLS','USAGE') THEN 1 ELSE 0 END) AS refs," + " SUM(CASE WHEN type = 'TESTS' THEN 1 ELSE 0 END) AS tests" + " FROM edges WHERE project = ?1 GROUP BY target_id) d ON d.id = n.id" + " LEFT JOIN (SELECT name, COUNT(DISTINCT file_path) AS files" + " FROM nodes WHERE project = ?1 GROUP BY name) f ON f.name = n.name" + " WHERE n.project = ?1 AND n.label IN ('Function','Method','Class')" + ")" + "UPDATE nodes SET properties = COALESCE(cbm_imp_set(properties, agg.score), properties)" + " FROM agg WHERE nodes.id = agg.id"; + +int cbm_pipeline_importance_recompute_store(cbm_store_t *store, const char *project) { + if (!store || !project) { + return -1; + } + sqlite3 *db = cbm_store_get_db(store); + if (!db) { + return -1; + } + if (sqlite3_create_function(db, "cbm_imp_score", 5, SQLITE_UTF8 | SQLITE_DETERMINISTIC, NULL, + sql_importance_score, NULL, NULL) != SQLITE_OK || + sqlite3_create_function(db, "cbm_imp_set", 2, SQLITE_UTF8 | SQLITE_DETERMINISTIC, NULL, + sql_importance_set, NULL, NULL) != SQLITE_OK) { + cbm_log_error("importance.store_udf_failed", "err", sqlite3_errmsg(db)); + return -1; + } + if (cbm_store_begin(store) != CBM_STORE_OK) { + return -1; + } + sqlite3_stmt *st = NULL; + if (sqlite3_prepare_v2(db, kImportanceRecomputeSQL, -1, &st, NULL) != SQLITE_OK) { + cbm_log_error("importance.store_prepare_failed", "err", sqlite3_errmsg(db)); + cbm_store_rollback(store); + return -1; + } + sqlite3_bind_text(st, 1, project, -1, SQLITE_TRANSIENT); + int step_rc = sqlite3_step(st); + sqlite3_finalize(st); + if (step_rc != SQLITE_DONE) { + cbm_log_error("importance.store_step_failed", "err", sqlite3_errmsg(db)); + cbm_store_rollback(store); + return -1; + } + int changed = sqlite3_changes(db); + if (cbm_store_commit(store) != CBM_STORE_OK) { + return -1; + } + uint64_t rows = (uint64_t)(changed > 0 ? changed : 0); + atomic_fetch_add_explicit(&g_importance_store_rows, rows, memory_order_relaxed); + cbm_log_info("pass.importance_store", "symbols", itoa_imp(rows)); + return 0; +} diff --git a/src/pipeline/pipeline_incremental.c b/src/pipeline/pipeline_incremental.c index 5de6bfd6a..4ff89aa42 100644 --- a/src/pipeline/pipeline_incremental.c +++ b/src/pipeline/pipeline_incremental.c @@ -1410,9 +1410,15 @@ static int run_extract_resolve(cbm_pipeline_ctx_t *ctx, cbm_file_info_t *changed } } -/* Run post-extraction passes (tests, decorator tags, configlink). */ +/* Run post-extraction passes (tests, decorator tags, configlink). + * + * score_importance: false on the closure-delta route, whose ctx->gbuf is a + * PROXY buffer — cbm_delta_preseed fills it with id/label/name/qn/file_path + * and no project-wide edges, so an in-memory score there would read a + * near-zero in-degree and cbm_delta_patch would persist it. That route scores + * in SQL instead, after the patch; see cbm_pipeline_importance_recompute_store. */ static int run_postpasses(cbm_pipeline_ctx_t *ctx, cbm_file_info_t *changed_files, int ci, - const char *project) { + const char *project, bool score_importance) { struct timespec t; cbm_clock_gettime(CLOCK_MONOTONIC, &t); @@ -1457,16 +1463,22 @@ static int run_postpasses(cbm_pipeline_ctx_t *ctx, cbm_file_info_t *changed_file } } - /* Importance last, in EVERY mode — same ordering rule as the full path: - * it reads CALLS/USAGE (re-extraction, above) and TESTS (pass_tests, first - * in this function). The graph here is the rehydrated whole-project buffer, - * so nodes outside the changed set already carry an "importance" key from - * the previous run; cbm_pipeline_importance_append_prop overwrites it in - * place rather than appending a duplicate. */ - cbm_clock_gettime(CLOCK_MONOTONIC, &t); - cbm_pipeline_pass_importance(ctx); - cbm_log_info("pass.timing", "pass", "incr_importance", "elapsed_ms", - itoa_buf((int)elapsed_ms(t))); + /* Importance last, in EVERY mode — same ordering rule as the full path: it + * reads CALLS/USAGE (re-extraction, above) and TESTS (pass_tests, first in + * this function). + * + * ONLY on the legacy-partial route, where ctx->gbuf really is the whole + * project rehydrated by cbm_gbuf_load_from_db — those nodes already carry + * an "importance" key from the previous run, and the write-back overwrites + * it in place rather than appending a duplicate. The closure-delta route + * passes score_importance = false and rescores in SQL after its patch, + * because its buffer holds proxies without project-wide edges. */ + if (score_importance) { + cbm_clock_gettime(CLOCK_MONOTONIC, &t); + cbm_pipeline_pass_importance(ctx); + cbm_log_info("pass.timing", "pass", "incr_importance", "elapsed_ms", + itoa_buf((int)elapsed_ms(t))); + } if (cbm_pipeline_check_cancel(ctx)) { return CBM_NOT_FOUND; } @@ -2158,7 +2170,7 @@ static int run_closure_delta(cbm_pipeline_t *p, const char *db_path, const char phase_rc = cbm_pipeline_check_cancel(&ctx); } if (phase_rc == 0) { - phase_rc = run_postpasses(&ctx, changed_files, ci, project); + phase_rc = run_postpasses(&ctx, changed_files, ci, project, false); } if (ctx.return_type_table) { for (int i = 0; i < ctx.return_type_table->count; i++) { @@ -2183,6 +2195,23 @@ static int run_closure_delta(cbm_pipeline_t *p, const char *db_path, const char } cbm_log_info("delta.patch_done", "elapsed_ms", itoa_buf((int)elapsed_ms(t))); + /* Importance, in SQL, over the now-complete staging graph. This is the + * FIRST point on this route where the whole project is queryable: the + * patch has just inserted the repaired files' nodes AND re-linked the + * snapshotted inbound edges, so in-degree is finally correct. Scoring + * earlier — in run_postpasses, off the proxy buffer — would persist a + * near-zero score for any heavily-referenced symbol in a changed file. + * A failure here is a delta-route failure: `result` still holds + * FORCE_FULL_REINDEX, so the run degrades to a correct full rebuild + * rather than publishing wrong scores. */ + cbm_clock_gettime(CLOCK_MONOTONIC, &t); + if (cbm_pipeline_importance_recompute_store(staging, project) != 0) { + cbm_log_error("delta.err", "phase", "importance_recompute"); + goto out; + } + cbm_log_info("pass.timing", "pass", "delta_importance_store", "elapsed_ms", + itoa_buf((int)elapsed_ms(t))); + /* Coverage: previous failure rows for files not re-extracted + this * run's fresh entries — same merge the legacy tail performs. */ cbm_file_error_t *run_errs = NULL; @@ -2713,7 +2742,7 @@ int cbm_pipeline_run_incremental(cbm_pipeline_t *p, const char *db_path, cbm_fil phase_rc = cbm_pipeline_check_cancel(&ctx); } if (phase_rc == 0) { - phase_rc = run_postpasses(&ctx, changed_files, ci, project); + phase_rc = run_postpasses(&ctx, changed_files, ci, project, true); } /* Free ObjectScript tables built by pass_calls during run_extract_resolve. */ diff --git a/src/pipeline/pipeline_internal.h b/src/pipeline/pipeline_internal.h index a25573733..e686ac3b7 100644 --- a/src/pipeline/pipeline_internal.h +++ b/src/pipeline/pipeline_internal.h @@ -641,12 +641,39 @@ void cbm_pipeline_pass_complexity(cbm_pipeline_ctx_t *ctx); * post-pass sequence. */ void cbm_pipeline_pass_importance(cbm_pipeline_ctx_t *ctx); -/* Write the numeric "importance" key into a node's properties JSON object. - * IDEMPOTENT: an existing key is overwritten in place, never appended twice — - * the incremental path re-scores nodes rehydrated from the store that already - * carry the key. Non-static so the idempotence contract is directly testable. */ +/* Gathered inputs for one symbol. Each scoring route fills this its own way + * (gbuf lookups, or SQL aggregates) and then calls the ONE rule below. */ +typedef struct { + const char *name; + const char *file_path; + int num_refs; /* incoming CALLS + USAGE */ + int name_distinct_files; /* distinct files the NAME is defined in */ + bool tests_target; /* has an incoming TESTS edge */ +} cbm_importance_inputs_t; + +/* THE scoring rule — single definition, shared by every route, so the two + * gathering strategies cannot drift apart in what a score means. */ +double cbm_pipeline_importance_score(const cbm_importance_inputs_t *in); + +/* Produce a copy of `json` with the numeric "importance" key set to `score`, + * or NULL when `json` is not a JSON object or allocation fails. Caller owns + * the result. IDEMPOTENT: an existing key is overwritten in place, never + * appended twice — every re-scoring route sees nodes that already carry it. + * Single definition, shared by the in-memory and SQL writers alike. */ +char *cbm_pipeline_importance_set_prop(const char *json, double score); + +/* gbuf-node convenience wrapper around cbm_pipeline_importance_set_prop. */ void cbm_pipeline_importance_append_prop(cbm_gbuf_node_t *node, double score); +/* Recompute importance for an ENTIRE project directly in a store, in SQL. + * Used by the closure-delta incremental route, whose in-RAM graph is a proxy + * buffer with no project-wide edges — scoring there would persist a near-zero + * in-degree for heavily-referenced symbols in the changed files. Must be + * called on the STAGING store AFTER cbm_delta_patch has merged nodes and + * re-linked inbound edges. Returns 0 on success; the caller treats failure as + * a delta-route failure and falls back to a full rebuild. */ +int cbm_pipeline_importance_recompute_store(cbm_store_t *store, const char *project); + /* Work counters for the importance pass (pass_importance.c). Deltas are read * by the complexity suite's linearity gate; never reset by the pass itself, so * nested/repeated runs compose. g_importance_name_visits counts same-name-group @@ -654,6 +681,9 @@ void cbm_pipeline_importance_append_prop(cbm_gbuf_node_t *node, double score); * superlinear if the per-distinct-name memoization is ever lost. */ extern _Atomic uint64_t g_importance_nodes; extern _Atomic uint64_t g_importance_name_visits; +/* Rows rescored by the SQL-level store recompute. Lets a test prove the + * closure-delta route actually took that path instead of passing vacuously. */ +extern _Atomic uint64_t g_importance_store_rows; /* ── Env URL scanner (pass_envscan.c) ────────────────────────────── */ diff --git a/tests/test_importance.c b/tests/test_importance.c index 9452a49e9..afaf41a3c 100644 --- a/tests/test_importance.c +++ b/tests/test_importance.c @@ -101,6 +101,97 @@ static int imp_check_label(cbm_store_t *s, const char *project, const char *labe return bad == 0 ? count : -1; } +/* ── Store-level score readout (for the closure-delta tests) ──────────── */ + +typedef struct { + char **qns; + double *scores; + int count; +} ImpScoreSet; + +static void imp_scores_free(ImpScoreSet *set) { + for (int i = 0; i < set->count; i++) { + free(set->qns[i]); + } + free(set->qns); + free(set->scores); + set->qns = NULL; + set->scores = NULL; + set->count = 0; +} + +/* Collect (qualified_name -> importance) for every scored label in a store. */ +static int imp_collect_scores(const char *db_path, const char *project, ImpScoreSet *out) { + static const char *const kLabels[] = {"Function", "Method", "Class"}; + memset(out, 0, sizeof(*out)); + cbm_store_t *s = cbm_store_open_path(db_path); + if (!s) { + return -1; + } + int rc = 0; + for (int li = 0; li < 3; li++) { + cbm_node_t *nodes = NULL; + int n = 0; + if (cbm_store_find_nodes_by_label(s, project, kLabels[li], &nodes, &n) != CBM_STORE_OK) { + rc = -1; + break; + } + char **qns = realloc(out->qns, (size_t)(out->count + n) * sizeof(*qns)); + double *sc = realloc(out->scores, (size_t)(out->count + n) * sizeof(*sc)); + if (n > 0 && (!qns || !sc)) { + cbm_store_free_nodes(nodes, n); + rc = -1; + break; + } + if (qns) { + out->qns = qns; + } + if (sc) { + out->scores = sc; + } + for (int i = 0; i < n; i++) { + out->qns[out->count] = strdup(nodes[i].qualified_name ? nodes[i].qualified_name : ""); + out->scores[out->count] = imp_value(nodes[i].properties_json); + out->count++; + } + cbm_store_free_nodes(nodes, n); + } + cbm_store_close(s); + return rc; +} + +static double imp_score_of(const ImpScoreSet *set, const char *qn_suffix) { + for (int i = 0; i < set->count; i++) { + size_t l = strlen(set->qns[i]); + size_t k = strlen(qn_suffix); + if (l >= k && strcmp(set->qns[i] + (l - k), qn_suffix) == 0) { + return set->scores[i]; + } + } + return -999.0; +} + +/* Write a fixture whose `helper` in hot.py is called from `callers` other + * files — the inbound edges that only exist project-wide, never in a delta + * buffer. `tag` lets the caller perturb hot.py to force a re-parse. */ +static int imp_write_hot_fixture(const char *root, int callers, const char *tag) { + char body[256]; + snprintf(body, sizeof(body), "%sdef helper():\n return 1\n", tag); + if (th_write_file(TH_PATH(root, "hot.py"), body) != 0) { + return -1; + } + for (int i = 0; i < callers; i++) { + char rel[64]; + char src[192]; + snprintf(rel, sizeof(rel), "c%d.py", i); + snprintf(src, sizeof(src), "import hot\n\ndef use%d():\n return hot.helper()\n", i); + if (th_write_file(TH_PATH(root, rel), src) != 0) { + return -1; + } + } + return 0; +} + /* ── Registration: the pass must actually execute ─────────────────────── */ /* A miscounted PREDUMP_PASS_COUNT silently drops the last-registered pass. @@ -577,6 +668,181 @@ TEST(importance_second_index_run_keeps_exactly_one_key) { PASS(); } +/* ── Closure-delta route: the graph is only complete in the store ─────── */ + +/* REGRESSION. `helper` lives in hot.py and is called from 25 other files. Edit + * hot.py and re-index: the closure-delta route purges hot.py's rows, rebuilds + * them from a PROXY buffer that holds no project-wide edges, and persists what + * it computed. Scoring off that buffer sees in-degree 0 and stores 0.0 for a + * symbol with 25 real callers. The fix rescores in SQL after cbm_delta_patch + * has re-linked the snapshotted inbound edges. */ +TEST(importance_closure_delta_keeps_referenced_symbol_sane) { + enum { CALLERS = 25 }; + char *tmp = th_mktempdir("cbm_imp_delta"); + if (!tmp) { + FAIL("tmpdir"); + } + char root[512]; + snprintf(root, sizeof(root), "%s", tmp); + char *tmp2 = th_mktempdir("cbm_imp_deltadb"); + if (!tmp2) { + th_rmtree(root); + FAIL("tmpdir2"); + } + char dbdir[512]; + snprintf(dbdir, sizeof(dbdir), "%s", tmp2); + + ASSERT_EQ(imp_write_hot_fixture(root, CALLERS, ""), 0); + + char db_path[600]; + snprintf(db_path, sizeof(db_path), "%s/graph.db", dbdir); + cbm_pipeline_t *p1 = cbm_pipeline_new(root, db_path, CBM_MODE_FULL); + ASSERT_NOT_NULL(p1); + ASSERT_EQ(cbm_pipeline_run(p1), 0); + char project[256]; + snprintf(project, sizeof(project), "%s", cbm_pipeline_project_name(p1)); + cbm_pipeline_free(p1); + + ImpScoreSet before = {0}; + ASSERT_EQ(imp_collect_scores(db_path, project, &before), 0); + double full_score = imp_score_of(&before, "helper"); + printf(" full-index helper score %.6f (expect sqrt(%d) = %.6f)\n", full_score, CALLERS, + sqrt((double)CALLERS)); + /* Non-vacuous: the callers really produced edges. If this is 0 the fixture + * stopped exercising the bug and the test below would prove nothing. */ + ASSERT_TRUE(full_score > 1.0); + imp_scores_free(&before); + + /* Touch hot.py so the delta route has exactly one file to repair. */ + ASSERT_EQ(imp_write_hot_fixture(root, CALLERS, "# touched\n"), 0); + + uint64_t sql0 = atomic_load_explicit(&g_importance_store_rows, memory_order_relaxed); + cbm_pipeline_t *p2 = cbm_pipeline_new(root, db_path, CBM_MODE_FULL); + ASSERT_NOT_NULL(p2); + ASSERT_EQ(cbm_pipeline_run(p2), 0); + cbm_incremental_route_t route = cbm_pipeline_incremental_test_last_route(); + cbm_pipeline_free(p2); + uint64_t sql_rows = atomic_load_explicit(&g_importance_store_rows, memory_order_relaxed) - sql0; + + /* Route proof — without it a fall-back to a full reindex would repair the + * score for the wrong reason and the test would pass vacuously. */ + printf(" route=%d (closure_repair=%d) sql_rescored_rows=%llu\n", (int)route, + (int)CBM_INCREMENTAL_ROUTE_CLOSURE_REPAIR, (unsigned long long)sql_rows); + ASSERT_EQ((int)route, (int)CBM_INCREMENTAL_ROUTE_CLOSURE_REPAIR); + + ImpScoreSet after = {0}; + ASSERT_EQ(imp_collect_scores(db_path, project, &after), 0); + double delta_score = imp_score_of(&after, "helper"); + printf(" after closure-delta helper score %.6f\n", delta_score); + imp_scores_free(&after); + + th_rmtree(root); + th_rmtree(dbdir); + /* THE SYMPTOM, asserted before any mechanism guard: the graph is unchanged, + * so the score must be unchanged. Scoring off the proxy buffer yields 0.0. */ + ASSERT_FLOAT_EQ(delta_score, full_score, 1e-6); + /* Mechanism guard second, so a regression fails on the wrong score rather + * than on bookkeeping. */ + ASSERT_GT((long long)sql_rows, 0); + PASS(); +} + +/* EQUIVALENCE. A full index and an incremental index that converge on the same + * tree must produce the same scores — for every symbol, not just the hot one. + * Same directory and therefore the same project name and qualified names, so + * the two score maps are directly comparable. */ +TEST(importance_full_and_incremental_converge_on_same_scores) { + enum { CALLERS = 12 }; + char *tmp = th_mktempdir("cbm_imp_equiv"); + if (!tmp) { + FAIL("tmpdir"); + } + char root[512]; + snprintf(root, sizeof(root), "%s", tmp); + char *tmp2 = th_mktempdir("cbm_imp_equivdb"); + if (!tmp2) { + th_rmtree(root); + FAIL("tmpdir2"); + } + char dbdir[512]; + snprintf(dbdir, sizeof(dbdir), "%s", tmp2); + + /* v1: every caller but c0 calls helper. c0 imports hot but does not. */ + ASSERT_EQ(imp_write_hot_fixture(root, CALLERS, ""), 0); + ASSERT_EQ(th_write_file(TH_PATH(root, "c0.py"), "import hot\n\ndef use0():\n return 1\n"), + 0); + ASSERT_EQ(th_write_file(TH_PATH(root, "extra.py"), + "import hot\n\ndef load_user_profile():\n return hot.helper()\n"), + 0); + + char db_incr[600]; + snprintf(db_incr, sizeof(db_incr), "%s/incr.db", dbdir); + cbm_pipeline_t *p1 = cbm_pipeline_new(root, db_incr, CBM_MODE_FULL); + ASSERT_NOT_NULL(p1); + ASSERT_EQ(cbm_pipeline_run(p1), 0); + char project[256]; + snprintf(project, sizeof(project), "%s", cbm_pipeline_project_name(p1)); + cbm_pipeline_free(p1); + + /* v2: c0's BODY starts calling helper. Body-only, so c0's surface is + * unchanged and the closure planner accepts the delta — but the in-degree + * of `helper`, which lives in the UNCHANGED hot.py, goes up by one. That + * is the case a changed-files-only rescore gets wrong: the symbol whose + * score moved is not in the changed set at all. */ + ASSERT_EQ(th_write_file(TH_PATH(root, "c0.py"), + "import hot\n\ndef use0():\n return hot.helper()\n"), + 0); + + uint64_t sql0 = atomic_load_explicit(&g_importance_store_rows, memory_order_relaxed); + cbm_pipeline_t *p2 = cbm_pipeline_new(root, db_incr, CBM_MODE_FULL); + ASSERT_NOT_NULL(p2); + ASSERT_EQ(cbm_pipeline_run(p2), 0); + cbm_incremental_route_t route = cbm_pipeline_incremental_test_last_route(); + cbm_pipeline_free(p2); + uint64_t sql_rows = atomic_load_explicit(&g_importance_store_rows, memory_order_relaxed) - sql0; + printf(" route=%d (closure_repair=%d) sql_rescored_rows=%llu\n", (int)route, + (int)CBM_INCREMENTAL_ROUTE_CLOSURE_REPAIR, (unsigned long long)sql_rows); + ASSERT_EQ((int)route, (int)CBM_INCREMENTAL_ROUTE_CLOSURE_REPAIR); + + /* Same tree, indexed cold into a separate database. */ + char db_full[600]; + snprintf(db_full, sizeof(db_full), "%s/full.db", dbdir); + cbm_pipeline_t *p3 = cbm_pipeline_new(root, db_full, CBM_MODE_FULL); + ASSERT_NOT_NULL(p3); + ASSERT_EQ(cbm_pipeline_run(p3), 0); + cbm_pipeline_free(p3); + + ImpScoreSet incr = {0}; + ImpScoreSet full = {0}; + ASSERT_EQ(imp_collect_scores(db_incr, project, &incr), 0); + ASSERT_EQ(imp_collect_scores(db_full, project, &full), 0); + printf(" scored symbols: incremental=%d full=%d\n", incr.count, full.count); + + int mismatches = 0; + for (int i = 0; i < full.count; i++) { + double a = full.scores[i]; + double b = imp_score_of(&incr, full.qns[i]); + if (!(fabs(a - b) < 1e-6)) { + mismatches++; + if (mismatches <= 5) { + printf(" MISMATCH %s: full=%.6f incremental=%.6f\n", full.qns[i], a, b); + } + } + } + int count_full = full.count; + int count_incr = incr.count; + imp_scores_free(&incr); + imp_scores_free(&full); + th_rmtree(root); + th_rmtree(dbdir); + + ASSERT_GT(count_full, 5); /* non-vacuous: there really are symbols to compare */ + ASSERT_EQ(count_incr, count_full); + ASSERT_EQ(mismatches, 0); /* the symptom */ + ASSERT_GT((long long)sql_rows, 0); /* mechanism guard, asserted second */ + PASS(); +} + /* ── Cost shape ───────────────────────────────────────────────────────── */ /* The distinct-file count must be computed once per distinct NAME. Computing @@ -636,6 +902,10 @@ SUITE(importance) { RUN_TEST(importance_rehydrated_node_keeps_exactly_one_key); RUN_TEST(importance_second_index_run_keeps_exactly_one_key); + /* Closure-delta route — the store-level recompute. */ + RUN_TEST(importance_closure_delta_keeps_referenced_symbol_sane); + RUN_TEST(importance_full_and_incremental_converge_on_same_scores); + /* Cost shape. */ RUN_TEST(importance_distinct_file_count_is_linear_in_group_size); }