Skip to content

Commit fea266f

Browse files
DeusDataDeusData
committed
fix(gbuf): canonical collision winner honors the upsert refresh contract
The deterministic same-QN collision rule picked the lexicographically smallest (label, file_path, start_line, name), which silently inverted the documented upsert/merge semantics: re-upserting a QN with updated fields kept the OLD content (gbuf_upsert_updates, gbuf_upsert_same_qn_updates_all_fields, gbuf_merge_overlapping_qns all red on every platform). Replace it with a single mixed-direction total order applied in both cbm_gbuf_upsert_node and merge_update_existing: smallest file_path, then LARGEST start_line, then largest name/label. Still a pure content function of the two candidates (commutative, associative, scheduling-independent — multi-threaded runs stay byte-identical, verified by the xfs MT x4 fingerprint guard), and line-descending within a file restores the classic contract: a later definition in the same file replaces the earlier one, and a full tie is the same entity refreshed in place. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent da75d1e commit fea266f

1 file changed

Lines changed: 47 additions & 64 deletions

File tree

‎src/graph_buffer/graph_buffer.c‎

Lines changed: 47 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -663,45 +663,35 @@ int64_t cbm_gbuf_upsert_node(cbm_gbuf_t *gb, const char *label, const char *name
663663
(strcmp(existing->label, "Project") == 0 || strcmp(existing->label, "Folder") == 0)) {
664664
return existing->id;
665665
}
666-
/* Same-QN arrival. Two distinct cases:
667-
*
668-
* 1) The SAME source entity re-upserted (identity fields match) —
669-
* refresh in place, exactly as before.
670-
*
671-
* 2) A DIFFERENT source entity sharing the QN. In C a struct, a
672-
* function and a macro can all carry one name and the QN scheme
673-
* does not encode the entity kind, so they collide here. The old
674-
* code let the LAST arrival overwrite — under parallel extraction
675-
* the merge order varies run to run, so WHICH entity survived
676-
* (label/name/lines/props) flickered (xfs: Function-node count
677-
* 4998 vs 5015 across two runs), and every order-sensitive
678-
* consumer downstream (semantic vectors, LSH, near-threshold
679-
* scores) inherited the flicker. Pick the survivor by a canonical
680-
* CONTENT rule instead — lexicographically smallest
681-
* (label, file_path, start_line, name) — so the outcome is a pure
682-
* function of the colliding entities, not of scheduling. Exactly
683-
* one entity is dropped, as one always was; kind-disambiguated
684-
* QNs (the real cure) are tracked as a follow-up. */
685-
bool same_entity = existing->label && label && strcmp(existing->label, label) == 0 &&
686-
existing->file_path && file_path &&
687-
strcmp(existing->file_path, file_path) == 0 &&
688-
existing->start_line == start_line && existing->name && name &&
689-
strcmp(existing->name, name) == 0;
690-
if (!same_entity) {
691-
int c = strcmp(label ? label : "", existing->label ? existing->label : "");
692-
if (c == 0) {
693-
c = strcmp(file_path ? file_path : "",
694-
existing->file_path ? existing->file_path : "");
695-
}
696-
if (c == 0) {
697-
c = start_line - existing->start_line;
698-
}
699-
if (c == 0) {
700-
c = strcmp(name ? name : "", existing->name ? existing->name : "");
701-
}
702-
if (c >= 0) {
703-
return existing->id; /* existing entity is the canonical winner */
704-
}
666+
/* Same-QN arrival: distinct source entities can share a QN (C: a
667+
* struct, a function and a macro with one name), and the same entity
668+
* can be re-upserted with fresh content. The old code let the LAST
669+
* arrival overwrite — under parallel extraction the merge order
670+
* varies run to run, so WHICH entity survived flickered (xfs:
671+
* Function-node count 4998 vs 5015 across two runs) and every
672+
* order-sensitive consumer downstream inherited it. Pick the
673+
* survivor by a canonical CONTENT rule instead, a pure function of
674+
* the two candidates: smallest file_path, then LARGEST start_line,
675+
* then largest name/label (a mixed-direction composite is still a
676+
* total order, so the pick is commutative and scheduling-free).
677+
* Line-descending within a file keeps the classic upsert contract —
678+
* a later definition in the same file (macro redefinition, refresh
679+
* of the same entity with new content) replaces the earlier one,
680+
* deterministically, because intra-file arrival order is fixed. A
681+
* full tie is the same entity re-upserted → refresh in place.
682+
* Kind-disambiguated QNs (the real cure) remain a follow-up. */
683+
int c = strcmp(file_path ? file_path : "", existing->file_path ? existing->file_path : "");
684+
if (c == 0) {
685+
c = existing->start_line - start_line;
686+
}
687+
if (c == 0) {
688+
c = strcmp(existing->name ? existing->name : "", name ? name : "");
689+
}
690+
if (c == 0) {
691+
c = strcmp(existing->label ? existing->label : "", label ? label : "");
692+
}
693+
if (c > 0) {
694+
return existing->id; /* existing entity is the canonical winner */
705695
}
706696
/* Update in-place. name/properties are strdup'd BEFORE freeing old ones
707697
* (callers may pass existing->name as an argument). label/file_path are
@@ -1206,32 +1196,25 @@ static void merge_update_existing(cbm_gbuf_t *dst, cbm_gbuf_node_t *existing,
12061196
(strcmp(existing->label, "Project") == 0 || strcmp(existing->label, "Folder") == 0);
12071197
if (!module_on_container) {
12081198
/* Canonical collision winner (determinism) — mirrors
1209-
* cbm_gbuf_upsert_node. Distinct source entities can share a QN (C:
1210-
* struct/function/macro with one name); unconditional "src wins" made
1211-
* the survivor depend on worker merge order, flickering the node set
1212-
* (and every downstream consumer) run to run. Same entity → refresh;
1213-
* different entity → keep the lexicographically smallest
1214-
* (label, file_path, start_line, name). */
1215-
bool same_entity = existing->label && sn->label &&
1216-
strcmp(existing->label, sn->label) == 0 && existing->file_path &&
1217-
sn->file_path && strcmp(existing->file_path, sn->file_path) == 0 &&
1218-
existing->start_line == sn->start_line && existing->name && sn->name &&
1219-
strcmp(existing->name, sn->name) == 0;
1220-
bool sn_wins = true;
1221-
if (!same_entity) {
1222-
int c = strcmp(sn->label ? sn->label : "", existing->label ? existing->label : "");
1223-
if (c == 0) {
1224-
c = strcmp(sn->file_path ? sn->file_path : "",
1225-
existing->file_path ? existing->file_path : "");
1226-
}
1227-
if (c == 0) {
1228-
c = sn->start_line - existing->start_line;
1229-
}
1230-
if (c == 0) {
1231-
c = strcmp(sn->name ? sn->name : "", existing->name ? existing->name : "");
1232-
}
1233-
sn_wins = c < 0;
1199+
* cbm_gbuf_upsert_node exactly. Distinct source entities can share a
1200+
* QN (C: struct/function/macro with one name); unconditional "src
1201+
* wins" made the survivor depend on worker merge order, flickering
1202+
* the node set (and every downstream consumer) run to run. Winner =
1203+
* smallest file_path, then LARGEST start_line, then largest
1204+
* name/label — one total order, commutative, scheduling-free; a full
1205+
* tie is the same entity → refresh from src. */
1206+
int c = strcmp(sn->file_path ? sn->file_path : "",
1207+
existing->file_path ? existing->file_path : "");
1208+
if (c == 0) {
1209+
c = existing->start_line - sn->start_line;
1210+
}
1211+
if (c == 0) {
1212+
c = strcmp(existing->name ? existing->name : "", sn->name ? sn->name : "");
1213+
}
1214+
if (c == 0) {
1215+
c = strcmp(existing->label ? existing->label : "", sn->label ? sn->label : "");
12341216
}
1217+
bool sn_wins = c <= 0;
12351218
if (sn_wins) {
12361219
/* Keep the secondary indexes consistent when the surviving
12371220
* label/name changes (the old code left the node listed under its

0 commit comments

Comments
 (0)