Skip to content

fix(semantic): sort deferred edges by (score desc, j) for deterministic admission (#998)#1014

Open
harshitaajoshi wants to merge 3 commits into
DeusData:mainfrom
harshitaajoshi:fix/semantic-edges-deterministic
Open

fix(semantic): sort deferred edges by (score desc, j) for deterministic admission (#998)#1014
harshitaajoshi wants to merge 3 commits into
DeusData:mainfrom
harshitaajoshi:fix/semantic-edges-deterministic

Conversation

@harshitaajoshi

Copy link
Copy Markdown

What does this PR do?

Fixes the ~50% SEMANTICALLY_RELATED edge churn across identical runs reported in #998. The root cause is cmp_deferred_edge_canonical sorting deferred edges by (i, c) where c is the candidate's LSH arrival rank, which varies with worker scheduling. This PR replaces the sort key with (i, score desc, j) so the per-node max_edges budget always admits the highest-scored edges first, with the partner func index (sorted by qualified_name in phase 1a) as a deterministic tie-break.

Checklist

  • Every commit is signed off (git commit -s) -- required, CI rejects
    unsigned commits (DCO, see CONTRIBUTING.md)
  • Tests pass locally (make -f Makefile.cbm test)
  • Lint passes (make -f Makefile.cbm lint-ci)
  • New behavior is covered by a test (reproduce-first for bug fixes)
    tests/repro/repro_parallel_determinism.c already guards this class of fix

…ic admission (DeusData#998)

Signed-off-by: Harshita Joshi <j.harshitaa06@gmail.com>
@harshitaajoshi harshitaajoshi requested a review from DeusData as a code owner July 10, 2026 22:00
@DeusData

Copy link
Copy Markdown
Owner

Thank you for digging into the admission ordering — before you invest more here, an important status update: the nondeterminism this PR targets no longer reproduces on current main.

I re-ran #998's measurement on main (db330bf): six full-mode indexes of the same flask checkout — three back-to-back plus CBM_WORKERS=2/8/16 variants — and all six produce a byte-identical SEMANTICALLY_RELATED edge set (129 edges, same SHA over the sorted src|dst fingerprint), versus the 74/59/126 churn from the issue. The deferred-admission rework noted in the file header (removing the budget's dependence on worker-completion order) appears to have resolved it; #998 is closed with the full evidence.

On the mechanism this PR proposes: the candidate rank c is assigned when LSH buckets are walked in ascending func index after the buckets are built sequentially in phase5c — so given a fixed funcs[] ordering, (i,c) is already a deterministic total order; the comparator change would alter which edges win ties but not remove any scheduling dependence. For it to be actionable we'd need a RED case on current main — a reproducible input where two identical runs differ. If you can produce one (the fingerprint procedure in #998's closing comment is cheap), I'll gladly re-triage; otherwise I'd suggest we close this once you've had a chance to verify on main.

Either way — thanks for engaging with one of the gnarlier corners of the pipeline; determinism reports and fixes are exactly the kind of contribution this project needs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants