Skip to content

graph_buffer: duplicate-edge property merge is last-write-wins, so edge attributes depend on worker merge order #1469

Description

@Studnicky

On a duplicate edge key, cbm_gbuf_insert_edge overwrites the stored property blob unconditionally. Because per-worker edge buffers merge in worker-slot order, which strategy's attributes survive is a function of thread scheduling. The edge set stays stable; its attributes flicker run to run.

Cause

The dedup key excludes the attributes it needs to arbitrate:

snprintf(buf, bufsz, "%lld:%lld:%s", (long long)src, (long long)tgt, type);

confidence, strategy, via and candidates live only in properties_json. On a key hit:

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) {
        free(existing->properties_json);
        existing->properties_json = heap_strdup(properties_json);
    }
    return existing->id;
}

Duplicates carrying different attributes genuinely occur: pass_calls mints CALLS edges from both LSP resolution and registry-textual matching, with different confidence and strategy. pass_parallel.c states the intended precedence — "LSP-resolved calls take precedence over registry-textual matching" — but last-write-wins only honours it when the LSP edge happens to arrive second.

Since file→worker assignment comes from an atomic work-stealing queue and worker buffers merge by slot index rather than by content, arrival order is scheduler-dependent.

Impact

An mcp__* consumer reading CALLS confidence can see a different value for the same commit on two indexes of the same tree. It also means the stated LSP-over-textual precedence is not actually enforced.

This is invisible to tests/repro/repro_parallel_determinism.c, which compares (source_qn, type, target_qn) triples and so cannot observe attribute churn.

Suggested fix

Replace the overwrite with a total order over the two candidates, making the merge commutative and associative and therefore independent of arrival order — e.g. higher confidence wins, with a deterministic tie-break between equals. That also makes the documented LSP-over-textual precedence hold regardless of which lands first.

The node path already works this way: cbm_gbuf_upsert_node picks the survivor of a same-QN collision by a canonical content rule "so the pick is commutative and scheduling-free" (#923). This is the same idea applied to edge attributes.

Environment: v0.9.0 (9b9b6ff), darwin-arm64, built from source.

Related: #923, #998.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions