diff --git a/src/graph_buffer/graph_buffer.c b/src/graph_buffer/graph_buffer.c index ca541e7d4..b173267ff 100644 --- a/src/graph_buffer/graph_buffer.c +++ b/src/graph_buffer/graph_buffer.c @@ -22,6 +22,10 @@ enum { GB_MIN_FOR_DEDUP = 2, /* need at least 2 vectors to sort+dedup */ GB_DEDUP_LOOKAHEAD = 1, /* compare current with next element */ }; + +/* Sentinel for an edge property blob carrying no parseable "confidence": any + * blob that states one outranks a blob that does not. */ +#define CBM_EDGE_CONF_ABSENT (-1.0) #include "graph_buffer/graph_buffer.h" #include // url_path extraction must match json_extract semantics #include "store/store.h" @@ -1029,6 +1033,53 @@ void cbm_gbuf_foreach_edge(const cbm_gbuf_t *gb, cbm_gbuf_edge_visitor_fn fn, vo /* ── Edge operations ─────────────────────────────────────────────── */ +/* Read "confidence": out of an edge property blob. Absent/unparseable + * reads as -1 so any edge that carries a confidence outranks one that does + * not. */ +static double edge_props_confidence(const char *props_json) { + if (!props_json) { + return CBM_EDGE_CONF_ABSENT; + } + static const char conf_key[] = "\"confidence\":"; + const char *p = strstr(props_json, conf_key); + if (!p) { + return CBM_EDGE_CONF_ABSENT; + } + return strtod(p + sizeof(conf_key) - SKIP_ONE, NULL); +} + +/* Decide whether an incoming property blob replaces the stored one on a + * duplicate edge key. + * + * confidence/strategy/via are not part of the edge key, and the same logical + * edge is legitimately minted by more than one strategy carrying different + * values — LSP resolution and registry-textual matching both produce CALLS + * edges. Per-worker edge buffers merge in worker-slot order, so any rule that + * depends on arrival order makes the surviving attributes a function of thread + * scheduling. + * + * This rule is a total order over the two candidates, hence commutative and + * associative — the outcome is independent of arrival order: + * 1. higher "confidence" wins, which also enforces the intended precedence + * that an LSP-resolved call outranks a textual match; + * 2. on equal confidence, the lexicographically greater blob wins, giving a + * deterministic choice between otherwise equal candidates. + * An empty or absent incoming blob never displaces stored properties. */ +static bool edge_props_should_replace(const char *existing_json, const char *incoming_json) { + if (!incoming_json || strcmp(incoming_json, "{}") == 0) { + return false; + } + if (!existing_json || strcmp(existing_json, "{}") == 0) { + return true; + } + double inc = edge_props_confidence(incoming_json); + double cur = edge_props_confidence(existing_json); + if (inc != cur) { + return inc > cur; + } + return strcmp(incoming_json, existing_json) > 0; +} + int64_t cbm_gbuf_insert_edge(cbm_gbuf_t *gb, int64_t source_id, int64_t target_id, const char *type, const char *properties_json) { if (!gb || !type) { @@ -1041,8 +1092,7 @@ int64_t cbm_gbuf_insert_edge(cbm_gbuf_t *gb, int64_t source_id, int64_t target_i cbm_gbuf_edge_t *existing = cbm_ht_get(gb->edge_by_key, key); if (existing) { - /* Merge properties (just replace for now) */ - if (properties_json && strcmp(properties_json, "{}") != 0) { + if (edge_props_should_replace(existing->properties_json, properties_json)) { free(existing->properties_json); existing->properties_json = heap_strdup(properties_json); } diff --git a/tests/test_graph_buffer.c b/tests/test_graph_buffer.c index 09a47af40..c4aa5c270 100644 --- a/tests/test_graph_buffer.c +++ b/tests/test_graph_buffer.c @@ -174,6 +174,88 @@ TEST(gbuf_edge_dedup) { PASS(); } +/* Properties on a deduped edge must not depend on arrival order. + * + * confidence/strategy/via are not part of the edge key, and the same logical + * CALLS edge is legitimately minted by two strategies (LSP resolution and + * registry-textual matching) carrying different values. Per-worker edge buffers + * merge in worker-slot order, so an arrival-order-dependent merge makes the + * stored attributes a function of thread scheduling. Insert the same pair of + * blobs in both orders; the stored properties must match. */ +TEST(gbuf_edge_props_merge_is_order_independent) { + const char *lsp = "{\"callee\":\"f\",\"confidence\":0.95,\"strategy\":\"lsp\"}"; + const char *txt = "{\"callee\":\"f\",\"confidence\":0.40,\"strategy\":\"registry\"}"; + + cbm_gbuf_t *fwd = cbm_gbuf_new("test", "/tmp"); + int64_t a1 = cbm_gbuf_upsert_node(fwd, "Function", "a", "pkg.a", "f.go", 1, 5, "{}"); + int64_t b1 = cbm_gbuf_upsert_node(fwd, "Function", "b", "pkg.b", "f.go", 6, 10, "{}"); + cbm_gbuf_insert_edge(fwd, a1, b1, "CALLS", lsp); + cbm_gbuf_insert_edge(fwd, a1, b1, "CALLS", txt); + + cbm_gbuf_t *rev = cbm_gbuf_new("test", "/tmp"); + int64_t a2 = cbm_gbuf_upsert_node(rev, "Function", "a", "pkg.a", "f.go", 1, 5, "{}"); + int64_t b2 = cbm_gbuf_upsert_node(rev, "Function", "b", "pkg.b", "f.go", 6, 10, "{}"); + cbm_gbuf_insert_edge(rev, a2, b2, "CALLS", txt); + cbm_gbuf_insert_edge(rev, a2, b2, "CALLS", lsp); + + const cbm_gbuf_edge_t **fe = NULL; + const cbm_gbuf_edge_t **re = NULL; + int fc = 0; + int rc = 0; + cbm_gbuf_find_edges_by_type(fwd, "CALLS", &fe, &fc); + cbm_gbuf_find_edges_by_type(rev, "CALLS", &re, &rc); + ASSERT_EQ(fc, 1); + ASSERT_EQ(rc, 1); + ASSERT_STR_EQ(fe[0]->properties_json, re[0]->properties_json); + + cbm_gbuf_free(fwd); + cbm_gbuf_free(rev); + PASS(); +} + +/* The order-independent winner is also the semantically right one: a + * higher-confidence discovery outranks a lower-confidence one regardless of + * which arrived first. */ +TEST(gbuf_edge_props_merge_prefers_higher_confidence) { + const char *lsp = "{\"callee\":\"f\",\"confidence\":0.95,\"strategy\":\"lsp\"}"; + const char *txt = "{\"callee\":\"f\",\"confidence\":0.40,\"strategy\":\"registry\"}"; + + cbm_gbuf_t *gb = cbm_gbuf_new("test", "/tmp"); + int64_t a = cbm_gbuf_upsert_node(gb, "Function", "a", "pkg.a", "f.go", 1, 5, "{}"); + int64_t b = cbm_gbuf_upsert_node(gb, "Function", "b", "pkg.b", "f.go", 6, 10, "{}"); + cbm_gbuf_insert_edge(gb, a, b, "CALLS", lsp); + cbm_gbuf_insert_edge(gb, a, b, "CALLS", txt); /* lower confidence, arrives last */ + + const cbm_gbuf_edge_t **edges = NULL; + int count = 0; + cbm_gbuf_find_edges_by_type(gb, "CALLS", &edges, &count); + ASSERT_EQ(count, 1); + ASSERT_TRUE(strstr(edges[0]->properties_json, "\"strategy\":\"lsp\"") != NULL); + + cbm_gbuf_free(gb); + PASS(); +} + +/* An empty incoming blob must never displace real stored properties. */ +TEST(gbuf_edge_props_merge_keeps_existing_on_empty) { + const char *lsp = "{\"callee\":\"f\",\"confidence\":0.95,\"strategy\":\"lsp\"}"; + + cbm_gbuf_t *gb = cbm_gbuf_new("test", "/tmp"); + int64_t a = cbm_gbuf_upsert_node(gb, "Function", "a", "pkg.a", "f.go", 1, 5, "{}"); + int64_t b = cbm_gbuf_upsert_node(gb, "Function", "b", "pkg.b", "f.go", 6, 10, "{}"); + cbm_gbuf_insert_edge(gb, a, b, "CALLS", lsp); + cbm_gbuf_insert_edge(gb, a, b, "CALLS", "{}"); + + const cbm_gbuf_edge_t **edges = NULL; + int count = 0; + cbm_gbuf_find_edges_by_type(gb, "CALLS", &edges, &count); + ASSERT_EQ(count, 1); + ASSERT_TRUE(strstr(edges[0]->properties_json, "\"strategy\":\"lsp\"") != NULL); + + cbm_gbuf_free(gb); + PASS(); +} + /* #768: two named imports from the same specifier (same source, same target * file) must produce two distinct IMPORTS edges, keyed apart by local_name -- * not collapse into one edge that silently drops whichever import lost the @@ -1067,6 +1149,11 @@ SUITE(graph_buffer) { RUN_TEST(gbuf_merge_into_store_preserves); RUN_TEST(gbuf_flush_skips_orphan_edges); + /* Edge property merge determinism */ + RUN_TEST(gbuf_edge_props_merge_is_order_independent); + RUN_TEST(gbuf_edge_props_merge_prefers_higher_confidence); + RUN_TEST(gbuf_edge_props_merge_keeps_existing_on_empty); + /* Shared ID tests */ RUN_TEST(gbuf_shared_ids_unique); RUN_TEST(gbuf_shared_ids_null_fallback);