Skip to content

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

Open
Studnicky wants to merge 1 commit into
DeusData:mainfrom
Studnicky:fix/configlink-deterministic-candidate-order
Open

fix(configlink): order candidates canonically before the collector cap#1470
Studnicky wants to merge 1 commit into
DeusData:mainfrom
Studnicky:fix/configlink-deterministic-candidate-order

Conversation

@Studnicky

Copy link
Copy Markdown

Fixes #1468

What does this PR do?

collect_config_entries and collect_code_entries fill fixed-capacity arrays and stop at max_out, walking gbuf label indexes in insertion order — which under parallel extraction is worker-merge order and varies run to run. On a repository with more candidates than the cap, the surviving set, and therefore the emitted CONFIGURES edges, depended on thread scheduling.

This sorts candidates by a pure content key (qualified_name, file_path, start_line, name) before the cap applies. Tie-breaks are content-only — node ids are handed out in merge order and belong in no canonical comparator.

It also logs configlinker.truncated when a collector fills to capacity, so the cap reports itself rather than dropping candidates silently.

Why

Indexing the same tree twice produced a different CONFIGURES edge set each time, and a different set from the single-threaded reference — which breaks the deterministic-output property established in #921, makes A/B comparisons noisy, and churns incremental artifacts.

This is the recipe #998 called for ("sort candidates by (score, qn) instead of arrival order"), applied to the pass that did not get it.

Testing

4191-file TypeScript corpus, --mode moderate, fresh CBM_CACHE_DIR per run, comparing sorted (source_qn, type, target_qn) triples:

ST vs MT MT vs MT
before 3–61 edges differ every run all differ
after identical across 4 runs identical
  • All divergence before the change was CONFIGURES — 86 edges across 3 runs, of 414 total.
  • Non-CONFIGURES edges are byte-identical before and after (70393), so the blast radius is contained to this pass.
  • A corpus below the cap (2871 candidates) was already deterministic, which is what identifies the cap as the mechanism.

New test configlink_candidate_truncation_is_order_independent builds a corpus exceeding the code-entry cap and runs the pass over it twice, inserting candidates forwards and then backwards; the emitted CONFIGURES set must match. It fails on an unpatched tree (7 passed, 1 failed) and passes with the change (8 passed). The configlinker.truncated log fires during the test, confirming the cap is genuinely exercised rather than the assertion passing vacuously.

scripts/test.sh: 438 passed across graph_buffer configlink simhash semantic parallel pipeline. The full suite matches the pre-change baseline exactly — the two failures (cli, py_lsp_scale) reproduce identically on an unpatched tree.

scripts/lint.sh: no new findings. pass_configlink.c reports the same 10 pre-existing items as before the change.

Risks

This changes which CONFIGURES edges are produced — 416 → 465 on the test corpus — because a canonical cut necessarily differs from an arbitrary one. That is inherent to any deterministic fix here.

The alternative, removing the caps so nothing is truncated, is not a drop-in: config_entries and code_entries are stack arrays (CBM_SZ_8K × 264 bytes plus CBM_SZ_4K × 520 bytes, ~4.3 MB of frame already), so lifting the bound would need heap allocation and a look at the O(config × code) match loop. Happy to take that direction instead if you would prefer it.

Scope is limited to pass_configlink.c; no other pass, the store format, or the MCP surface is touched.

Links

Checklist

  • Every commit is signed off (git commit -s)
  • Tests pass locally
  • Lint passes
  • New behavior is covered by a test (reproduce-first: the test is red on an unpatched tree)

collect_config_entries and collect_code_entries fill fixed-capacity arrays
and stop at max_out. They walk gbuf label indexes in insertion order, which
under parallel extraction is worker-merge order and varies run to run, so on
any repository with more candidates than the cap the surviving set — and
therefore the emitted CONFIGURES edges — depended on thread scheduling.

Sort candidates by a pure content key (qualified_name, file_path, start_line,
name) before the cap applies, so the surviving set is a function of the inputs
alone. Tie-breaks are content-only: node ids are handed out in merge order and
belong in no canonical comparator.

Also log configlinker.truncated when a collector fills to capacity, so the cap
reports itself instead of dropping candidates silently.

Measured on a 4191-file TypeScript corpus (12940 candidates against the 8192
code cap): before, 3-61 CONFIGURES edges differed per run against a
single-threaded reference and no two multi-threaded runs agreed; after, the
single-threaded reference and four multi-threaded runs are byte-identical.
Corpora below the cap were already stable, and all divergence was CONFIGURES.

Signed-off-by: Andrew Studnicky <a.j.studnicky@gmail.com>
@Studnicky
Studnicky requested a review from DeusData as a code owner August 6, 2026 12:30
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

  • It will not be closed for inactivity. No stale bot touches pull requests here.
  • It may still sit a while before a human reads it. That is on us, not on you.
  • Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

  • Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
  • Get CI green, or say which failures you believe are pre-existing.
  • Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
  • Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

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.

pass_configlink: CONFIGURES edges are nondeterministic when candidates exceed the collector caps

1 participant