From 44881a06764cb172b8211622673b67bc4d4bed27 Mon Sep 17 00:00:00 2001 From: Andrew Studnicky Date: Thu, 6 Aug 2026 08:24:23 -0400 Subject: [PATCH] fix(configlink): order candidates canonically before the collector cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit collect_config_entries and collect_code_entries fill fixed-capacity arrays and stop at max_out. They walk gbuf label indexes in insertion order, which under parallel extraction is worker-merge order and varies run to run, so on any repository with more candidates than the cap the surviving set — and therefore the emitted CONFIGURES edges — depended on thread scheduling. Sort candidates by a pure content key (qualified_name, file_path, start_line, name) before the cap applies, so the surviving set is a function of the inputs alone. Tie-breaks are content-only: node ids are handed out in merge order and belong in no canonical comparator. Also log configlinker.truncated when a collector fills to capacity, so the cap reports itself instead of dropping candidates silently. Measured on a 4191-file TypeScript corpus (12940 candidates against the 8192 code cap): before, 3-61 CONFIGURES edges differed per run against a single-threaded reference and no two multi-threaded runs agreed; after, the single-threaded reference and four multi-threaded runs are byte-identical. Corpora below the cap were already stable, and all divergence was CONFIGURES. Signed-off-by: Andrew Studnicky --- src/pipeline/pass_configlink.c | 87 +++++++++++++++++++++++++-- tests/test_configlink.c | 107 +++++++++++++++++++++++++++++++++ 2 files changed, 190 insertions(+), 4 deletions(-) diff --git a/src/pipeline/pass_configlink.c b/src/pipeline/pass_configlink.c index 341847c40..34c90bdf3 100644 --- a/src/pipeline/pass_configlink.c +++ b/src/pipeline/pass_configlink.c @@ -60,6 +60,67 @@ static bool is_dep_section(const char *s) { /* ── Strategy 1: Config Key → Code Symbol ───────────────────────── */ +/* Canonical candidate order (determinism). Both collectors below fill a + * fixed-capacity array and stop at max_out; the label indexes they walk are + * gbuf insertion order = parallel-extraction merge order, which varies run to + * run. On a repo with more candidates than the cap, sorting by a pure content + * key first is what keeps the surviving set — and therefore the emitted + * CONFIGURES edges — a function of the inputs rather than of worker + * scheduling. Tie-breaks stay content-only: node ids are handed out in merge + * order, so an id tie-break belongs in no canonical comparator. */ +enum { + CANON_CMP_LESS = -1, /* qsort: left sorts before right */ + CANON_CMP_GREATER = 1, /* qsort: left sorts after right */ + CANON_CAP_BUF = 32 /* decimal rendering of a cap value */ +}; + +static int cmp_node_ptr_canonical(const void *pa, const void *pb) { + const cbm_gbuf_node_t *a = *(const cbm_gbuf_node_t *const *)pa; + const cbm_gbuf_node_t *b = *(const cbm_gbuf_node_t *const *)pb; + const char *qa = a->qualified_name ? a->qualified_name : ""; + const char *qb = b->qualified_name ? b->qualified_name : ""; + int r = strcmp(qa, qb); + if (r != 0) { + return r; + } + const char *fa = a->file_path ? a->file_path : ""; + const char *fb = b->file_path ? b->file_path : ""; + r = strcmp(fa, fb); + if (r != 0) { + return r; + } + if (a->start_line != b->start_line) { + return a->start_line < b->start_line ? CANON_CMP_LESS : CANON_CMP_GREATER; + } + const char *na = a->name ? a->name : ""; + const char *nb = b->name ? b->name : ""; + return strcmp(na, nb); +} + +/* A filled-to-capacity collector dropped candidates; say so rather than + * truncating silently. */ +static void log_candidate_truncation(const char *side, int cap) { + char cap_buf[CANON_CAP_BUF]; + snprintf(cap_buf, sizeof(cap_buf), "%d", cap); + cbm_log_info("configlinker.truncated", "side", side, "cap", cap_buf); +} + +/* Heap copy of `nodes` sorted by cmp_node_ptr_canonical. Returns NULL (and + * leaves the caller on the unsorted borrowed array) only on allocation + * failure, which degrades determinism but never correctness. */ +static const cbm_gbuf_node_t **canonical_node_copy(const cbm_gbuf_node_t *const *nodes, int count) { + if (!nodes || count <= 0) { + return NULL; + } + const cbm_gbuf_node_t **sorted = malloc((size_t)count * sizeof(*sorted)); + if (!sorted) { + return NULL; + } + memcpy(sorted, nodes, (size_t)count * sizeof(*sorted)); + qsort(sorted, (size_t)count, sizeof(*sorted), cmp_node_ptr_canonical); + return sorted; +} + typedef struct { int64_t node_id; char normalized[CBM_SZ_256]; @@ -70,6 +131,10 @@ typedef struct { static int collect_config_entries(const cbm_gbuf_node_t *const *vars, int var_count, config_entry_t *out, int max_out) { int n = 0; + const cbm_gbuf_node_t **sorted = canonical_node_copy(vars, var_count); + if (sorted) { + vars = sorted; + } for (int i = 0; i < var_count && n < max_out; i++) { if (!cbm_has_config_extension(vars[i]->file_path)) { continue; @@ -102,6 +167,10 @@ static int collect_config_entries(const cbm_gbuf_node_t *const *vars, int var_co snprintf(out[n].name, sizeof(out[n].name), "%s", vars[i]->name); n++; } + if (n == max_out) { + log_candidate_truncation("config", max_out); + } + free((void *)sorted); return n; } @@ -124,22 +193,32 @@ static int collect_code_entries(cbm_gbuf_t *gb, code_entry_t *out, int max_out) continue; } + /* Canonical order before the cap — see cmp_node_ptr_canonical. The cap + * spans the whole label list, so a later label can be cut mid-group; + * sorting per group keeps that cut a pure function of content. */ + const cbm_gbuf_node_t **sorted = canonical_node_copy(nodes, count); + const cbm_gbuf_node_t *const *scan = sorted ? sorted : nodes; + for (int i = 0; i < count && n < max_out; i++) { - if (cbm_has_config_extension(nodes[i]->file_path)) { + if (cbm_has_config_extension(scan[i]->file_path)) { continue; } char norm[CBM_SZ_256]; - int tokens = cbm_normalize_config_key(nodes[i]->name, norm, sizeof(norm)); + int tokens = cbm_normalize_config_key(scan[i]->name, norm, sizeof(norm)); if (tokens == 0 || norm[0] == '\0') { continue; } - out[n].node_id = nodes[i]->id; + out[n].node_id = scan[i]->id; snprintf(out[n].normalized, sizeof(out[n].normalized), "%s", norm); n++; } - /* gbuf data is borrowed — no free */ + /* gbuf data is borrowed — only the sorted copy is owned */ + free((void *)sorted); + } + if (n == max_out) { + log_candidate_truncation("code", max_out); } return n; } diff --git a/tests/test_configlink.c b/tests/test_configlink.c index 1147f2785..1b1ea216a 100644 --- a/tests/test_configlink.c +++ b/tests/test_configlink.c @@ -290,6 +290,110 @@ TEST(configlink_file_ref_no_false_positive) { PASS(); } +/* ── Determinism: candidate truncation must not depend on insert order ── */ + +static int cl_cmp_row(const void *pa, const void *pb) { + return strcmp(*(const char *const *)pa, *(const char *const *)pb); +} + +/* Collect the CONFIGURES edge set as a sorted, newline-joined + * "src_qn|tgt_qn" fingerprint. Caller frees. */ +static char *configures_fingerprint(cbm_gbuf_t *gb) { + const cbm_gbuf_edge_t **edges = NULL; + int count = 0; + cbm_gbuf_find_edges_by_type(gb, "CONFIGURES", &edges, &count); + + char **rows = calloc((size_t)(count > 0 ? count : 1), sizeof(*rows)); + if (!rows) { + return NULL; + } + int n = 0; + for (int i = 0; i < count; i++) { + const cbm_gbuf_node_t *s = cbm_gbuf_find_by_id(gb, edges[i]->source_id); + const cbm_gbuf_node_t *t = cbm_gbuf_find_by_id(gb, edges[i]->target_id); + if (!s || !t || !s->qualified_name || !t->qualified_name) { + continue; + } + size_t len = strlen(s->qualified_name) + strlen(t->qualified_name) + 2; + rows[n] = malloc(len); + if (!rows[n]) { + break; + } + snprintf(rows[n], len, "%s|%s", s->qualified_name, t->qualified_name); + n++; + } + qsort(rows, (size_t)n, sizeof(*rows), cl_cmp_row); + + size_t total = 1; + for (int i = 0; i < n; i++) { + total += strlen(rows[i]) + 1; + } + char *out = calloc(total, 1); + if (out) { + for (int i = 0; i < n; i++) { + strcat(out, rows[i]); + strcat(out, "\n"); + } + } + for (int i = 0; i < n; i++) { + free(rows[i]); + } + free(rows); + return out; +} + +/* Build a gbuf whose code-candidate count exceeds the collector's internal + * cap, inserting the candidates either forwards or backwards. */ +enum { CL_DET_CANDIDATES = 9000 }; /* > the 8192 code-entry cap */ + +static cbm_gbuf_t *build_oversized_corpus(bool reverse) { + cbm_gbuf_t *gb = cbm_gbuf_new("test", "/tmp/test"); + + /* One config key every candidate can match on: "max_connections". */ + cbm_gbuf_upsert_node(gb, "Variable", "max_connections", "test.config.max_connections", + "config.toml", 0, 0, NULL); + + for (int k = 0; k < CL_DET_CANDIDATES; k++) { + int i = reverse ? (CL_DET_CANDIDATES - 1 - k) : k; + char name[CBM_SZ_64]; + char qn[CBM_SZ_128]; + char path[CBM_SZ_64]; + snprintf(name, sizeof(name), "getMaxConnections%04d", i); + snprintf(qn, sizeof(qn), "test.mod%04d.%s", i, name); + snprintf(path, sizeof(path), "mod%04d.go", i); + cbm_gbuf_upsert_node(gb, "Function", name, qn, path, 0, 0, NULL); + } + return gb; +} + +/* REGRESSION: collect_config_entries/collect_code_entries fill fixed-capacity + * arrays and stop at the cap, walking gbuf label indexes in insertion order — + * which under parallel extraction is worker-merge order and varies run to run. + * Sorting candidates canonically before the cap is what keeps the surviving + * set, and therefore the emitted CONFIGURES edges, a pure function of the + * inputs. This test stands in for the scheduling variance by feeding the same + * corpus in two insertion orders: the emitted edge set must be identical. */ +TEST(configlink_candidate_truncation_is_order_independent) { + cbm_gbuf_t *fwd = build_oversized_corpus(false); + run_configlink(fwd, "test", NULL); + char *fp_fwd = configures_fingerprint(fwd); + + cbm_gbuf_t *rev = build_oversized_corpus(true); + run_configlink(rev, "test", NULL); + char *fp_rev = configures_fingerprint(rev); + + ASSERT_TRUE(fp_fwd != NULL); + ASSERT_TRUE(fp_rev != NULL); + ASSERT_TRUE(fp_fwd[0] != '\0'); /* the cap must actually have been exercised */ + ASSERT_STR_EQ(fp_fwd, fp_rev); + + free(fp_fwd); + free(fp_rev); + cbm_gbuf_free(fwd); + cbm_gbuf_free(rev); + PASS(); +} + /* ── Suite ───────────────────────────────────────────────────────── */ SUITE(configlink) { @@ -305,4 +409,7 @@ SUITE(configlink) { /* Strategy 3: File Path → Reference */ RUN_TEST(configlink_file_ref_no_false_positive); + + /* Determinism */ + RUN_TEST(configlink_candidate_truncation_is_order_independent); }