Merge pull request #1470 from Studnicky/fix/configlink-deterministic-candidate-order

fix(configlink): order candidates canonically before the collector cap

Author: DeusData

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;
 }

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);
 }