[mvanhorn]

Automatic refreshes of an existing committed index should leave CPU headroom while preserving throughput for a first automatic index and for explicit/manual indexing. This change carries an internal background-execution marker through daemon coordination and worker serialization, then applies the existing background worker policy in the pipeline. It adds no public MCP option or user-facing configuration key.

The final policy preserves CBM_WORKERS precedence and single-thread crash recovery. Surface construction also respects the total worker budget, including the calling thread, rather than allowing one extra active callback. Existing non-Git auto-index limits, watcher deduplication, and subprocess RSS isolation remain in place.

Fixes #1084

Validation:

• Deterministic surface-concurrency regression: RED twice at three callbacks for budget two; GREEN at two and serial override one; production-only revert reproduces RED.
• Final scoped macOS arm64 ASan/UBSan run: 44 targeted worker-policy, surface, and pipeline checks passed.
• The prepared fix files are unchanged after merging the current main baseline; fresh required GitHub CI validates the combined branch.
• Contributor history is preserved; maintainer commits carry the repository DCO sign-off.

[github-actions]

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.

[DeusData]

Welcome, and thank you for the PR 👋 — one quick, mechanical thing before review, so you are not left guessing at the red.

dco is failing because your commit is not signed off. c846ed7b ("fix: preserve CPU headroom during background indexing") carries no Signed-off-by trailer, and this repository enforces DCO on push.

The fix is to amend the commit with a sign-off and force-push your branch:

• git commit --amend -s --no-edit — -s appends the trailer using your configured user.name and user.email
• then force-push your own PR branch (with lease), naming the branch explicitly

If you end up with more than one commit, git rebase --signoff <base> does the whole branch at once.

Two things that will save you time when the rest of CI reports:

• test-msan is currently failing for everyone — it dies at the Docker image build step, before any test runs. If you see it red, it is ours, not yours.
• test-windows-guards has a standing red on test_daemon_stability.py this week, likewise unrelated to any diff.

I will review the change itself properly once it is signed off — background-indexing CPU headroom is a good thing to be looking at, and +209/-12 over 8 files is a reviewable size.

[DeusData]

Attributed your reds so you are not chasing nine separate things — there are really only two, and one of them is a genuinely interesting test-design point.

1. dco — still unsigned

c846ed7b still has no Signed-off-by trailer. Amend with -s and force-push your own branch (see my earlier note).

2. The sanitizer legs — your own new test, and worth thinking about

The failure is not spread across nine problems. On the macOS TSan leg:

mcp_auto_index_in_process_uses_background_worker_policy
  FAIL tests/test_mcp.c:13035: selected_workers == 2, expected cbm_default_worker_count(false) == 4

That is the test this PR adds, asserting:

ASSERT_EQ(selected_workers, cbm_default_worker_count(false));

against your production change:

workers = cbm_default_worker_count(!p || !p->background);

The assertion re-evaluates the helper rather than naming an expected number, so it silently assumes nothing between cbm_default_worker_count(false) and the value the pipeline actually selects can change the answer. The run says otherwise: the helper returned 4 while the pipeline chose 2. Something downstream — a clamp, a cap, or an environment-sensitive input — is intervening.

That is worth pinning down rather than papering over, because it is the same shape as a class of bug we have been clearing out of this suite all week: assertions whose verdict depends on the machine rather than the code. A test that compares a captured value against a live re-evaluation of a helper will agree on your laptop and disagree on a constrained runner, and neither result tells you whether the policy is right.

Two questions that should settle it:

• Is 2 the value your change intends on that runner, and the helper's 4 simply not the right comparison? Then assert the policy relationship directly, or assert against whatever the pipeline is actually given.
• Or is the pipeline clamping below your intended policy? Then the production change is not yet doing what the description says, and the test has correctly caught it.

Either way the test is doing its job — it failed rather than passing vacuously, which is more than a lot of new tests manage.

Not everything red is that

For completeness: the test-unix (ubuntu-latest) leg reports 2216 passed, 0 failed and failed at a later step, so that one is environmental rather than yours. And test-msan fails at the Docker image build with the suite skipped — a DNS resolution failure inside buildkit, entirely ours.

I have not reviewed the change on merit yet — I will once it is signed off and the worker-count question is resolved. Preserving CPU headroom during background indexing is a good thing to be fixing.

[DeusData]

Maintainer notice: please disregard comments from @adfjadfj16-a11y on this thread

@adfjadfj16-a11y is not a maintainer of this project and does not speak for it. That account has posted replies on 17 threads here written in the project's voice — promising merges, announcing that a case has been "escalated to the development team", asking to close issues, and in some threads replying as though it were the author of someone else's pull request. None of those were maintainer decisions, and none of them carried any weight.

@DeusData is the only account that gives a maintainer response on this repository. If a comment about the fate of your issue or pull request did not come from @DeusData, it is not a decision, however official it reads.

If you were waiting on something because of one of those comments — a promised merge, a review "immediately", a request to close your ticket — I am sorry. That was noise you had no way to identify as noise, and it should not have been on your thread. Your issue or PR is judged on its own merits, and I will answer it here myself.

Nothing in this notice reflects on your contribution. Thank you for your patience, and thank you for the work.

[DeusData]

The attribution I owed you on 3 September, with apologies that it took this long. Every red on your run 34091819685 is now accounted for, and none of them is a mystery:

job	what failed	whose
test-diag, test-lsan-macos, test-unix 2/3 (x64 + arm64)	suites fully green (7643 passed, 0 failed etc.), then LSan: 360 bytes in 9 allocations, stack autoindex_thread → cbm_pipeline_run → … → cbm_kind_in_set	ours, stale base: the in-process auto-index thread never freed its extraction bitset cache; fixed on main by e45d7051 (#2116) after your merge base. Your new test is simply the second thing to exercise that thread. Disappears on rebase.
test-tsan (macos-14, ubuntu x64, ubuntu arm64)	mcp_auto_index_in_process_uses_background_worker_policy: selected_workers == 2, expected cbm_default_worker_count(false) == 4 (3 vs 4 on the 4-core runners)	the PR's own test, assertion order: the test unsets CBM_WORKERS for the run, then restores the environment, and only then computes the expected value — which now sees the lane's CBM_WORKERS=4. Deterministic; here it fails the other way under CBM_WORKERS=1 (5, expected 1).

No infrastructure flake among them, and the Windows guard was green on your run.

On the change itself, which I read in full on your head and built and ran locally: I like it. Routing every automatic index — in-process auto-index, the supervised worker, the daemon's session auto-index and the watcher re-index — onto the existing "leave headroom" policy through one flag, while an explicit index_repository keeps every core and CBM_WORKERS / CBM_INDEX_SINGLE_THREAD keep their precedence, is the smallest shape that does the job, and reusing the incremental policy rather than inventing a second one is the right instinct. It changes only the worker count fed to the scheduler, never merge order or a work cap, so the graph is unaffected; no delay, no raw fopen, no new allocation site. pipeline 266/0 and daemon_application 52/0 pass here, and with the production hunks neutered your preserves_overrides and the two daemon_application assertions go red, so the tests bind.

Four things before it can merge:

1. Rebase onto main. The four LSan reds vanish with it. The conflicts are all same-anchor insertion drift against the opt-in resource-policy work (21ae0215) and #713's tests: keep both sides in pipeline.h / pipeline.c and the two test files, and in the three args builders (application_auto_index_args, application_background_index, index_run_supervised_path) chain your _background bool after main's policy call inside its guarded form.
2. Fix the assertion order in mcp_auto_index_in_process_uses_background_worker_policy: capture int expected = cbm_default_worker_count(false); inside the window where the variables are unset, before mcp_test_restore_env, and assert against that. Your pipeline_background_worker_policy_preserves_overrides already does it this way.
3. pipeline_background_policy_preserves_index_results is inert. setup_test_repo() writes about three files, below MIN_FILES_FOR_PARALLEL (50), so foreground and background both take the sequential path and the test stays green with the policy removed. Either give it a fixture past 50 files so it really compares an all-core index against a headroom index, or drop it and name the existing determinism test that covers the property.
4. The description overstates the coverage. Two all-core sites are still unrouted on a background run: the LSP-surface pass (lsp_surface.c, cbm_parallel_for with auto-detected workers) and incremental hashing (pipeline_incremental.c, hash_workers = cbm_default_worker_count(true)). Both have the pipeline in reach; route them through cbm_pipeline_worker_count too, or narrow the description to what is covered. Small naming nit while you are there: _cbm_background matches the existing _cbm_index_policy convention for hidden keys.

One product question I want to be explicit about rather than bury: with this, a user who connects with auto_index=true gets their first graph built on perf_cores-1 workers, so the first index takes a little longer in exchange for a responsive machine. I think that is the right trade and it is the thesis of the PR, but it is a visible change on first connect, and I will confirm it on our side before merging. Thank you for the careful set of tests and for waiting out nineteen days of silence you did nothing to earn.

[mvanhorn]

Thank you for the attribution, and for taking the time to sort every red into whose it was. That saved me chasing the LSan leak.

All four are done, rebased onto main in 01fba2c6 and the fixes in 7498e2b7.

1. Rebase. Resolved as you described: kept both sides in pipeline.h / pipeline.c and the two test files, and chained the background bool after main's policy call inside its guarded form in all three args builders. The LSan reds are gone with it.

2. Assertion order. expected is now captured inside the unset window, before mcp_test_restore_env, the way pipeline_background_worker_policy_preserves_overrides already did it.

3. The inert test. Gave it a real fixture rather than dropping it: 64 padding files put it past MIN_FILES_FOR_PARALLEL, and I also unset any inherited CBM_WORKERS / CBM_INDEX_SINGLE_THREAD, since a pin would have made both runs pick the same count and left the test inert for a second reason.

4. Coverage. Routed both sites you named, lsp_surface.c's cbm_parallel_for and pipeline_incremental.c's hash_workers, through cbm_pipeline_worker_count rather than narrowing the description; the pipeline is plumbed into cbm_pipeline_build_semantic_manifest for that. Took the naming nit too, so the hidden key is _cbm_background now.

One thing worth your eye in 4: closure_probe_surfaces previously passed NULL as the pipeline ctx deliberately, so file errors were not recorded twice. I set probe_ctx.pipeline around the call and clear it after, so the surface pass can read the worker policy without changing the error-recording behaviour. If you would rather it stayed NULL and the probe path kept all cores, say so and I will pull that hunk.

On the product question: I agree that is the right trade, and I think it is worth saying in the release note rather than only here, since a slower first index on auto_index=true is the kind of thing people notice and file.

666 passed, 4 skipped locally across the pipeline, mcp and daemon_application suites.

[DeusData]

Thank you for turning all four around so quickly. The assertion order, the 64-file fixture that finally makes the invariance test compare two different worker counts, and routing the two remaining all-core sites are all right, and I verified them on the merge result with today's main (e783f73d): build clean; pipeline 289, daemon_application 56 and mcp 321 passed; memory-core linter unchanged.

Your question about closure_probe_surfaces first, because the answer is that you were right and the hunk should stay. The surface builder reads the context for exactly one thing, the worker count, and the only function it forwards the context to, cbm_pipeline_result_acquire, reads nothing but ctx->spill, which probe_ctx never sets. So that call takes the same branch it took when you passed NULL, and no file error can be recorded through it, twice or at all. What would break the property is leaving probe_ctx.pipeline set for the struct's whole lifetime, because then cbm_parallel_extract above it would see a pipeline. Your set-and-clear around the one call is exactly the shape that avoids that.

Two things before it merges, and the first is the product question you and I discussed:

1. A project's first index runs at full width. We have decided the trade the other way round for the one case where it matters most. When someone connects and the project has no index yet, they are waiting for a graph that does not exist, and a first build one worker short is the slowest possible moment to make them wait. Once an index exists, a refresh really is background work, and leaving a core free is right. So _cbm_background should take effect only when a committed index for the project already exists; a project's very first index keeps every core even when it was started automatically, and watcher re-indexes and auto-indexes of an already-indexed project keep the headroom. The flag is honoured in two places today, handle_index_repository (reads _cbm_background, then cbm_pipeline_set_background) and the in-process autoindex_thread (sets it directly), so the check belongs there, or in one shared helper both call, as long as the two paths agree. For tests, a pair makes the rule visible: the in-process auto-index of a fresh project asserts full width, and the same auto-index of a project that is already indexed asserts headroom.
2. The lint job. src/pipeline/pipeline.c:207 still has two spaces before the trailing comment on the new bool background; field where the block wants one. I reproduced it with our own Homebrew clang-format 22.1.8, so it is a genuine violation and not the standalone clang-format-20 drift we have been caught by before; clang-format -i on that file fixes it and touches nothing else.

Everything else stands as reviewed. After those two, this merges. If the Windows guard job goes red on your next run, that is the daemon endpoint cold-start race on our side (#2275), not your change.

[mvanhorn]

Thanks, that trade makes sense. Both are done in 7a307ab.

1. _cbm_background now only applies when the project already has a committed index. There's one helper, project_has_committed_index(), and both handle_index_repository (checked after the final project name is set) and autoindex_thread go through it, so the two paths can't drift. A first index keeps every core even when auto-started. The tests come in pairs as you described: fresh in-process auto-index asserts full width and the already-indexed one asserts headroom, plus the same pair for the _cbm_background tool path.
2. The bool background; comment spacing at pipeline.c:207 is fixed. Homebrew clang-format 22.1.8 is clean on both source files.

make test-focused for mcp and pipeline: 613 passed.

[DeusData]

Thank you, @mvanhorn, for four quick and careful rounds on this. The first index keeping full width is exactly what was decided, and the review of 7a307ab is clean.

One last step before it can merge: main has moved a long way (about 215 commits), and the branch now conflicts in two places. Both are same-spot insertions:

• src/pipeline/pipeline.c: cbm_pipeline_export_error() (from fa22921) and your cbm_pipeline_set_background() both landed right after cbm_pipeline_get_resource_violation().
• src/mcp/mcp.c: index_root_owner_append() / index_root_owner_resolve() (from fix(mcp): reuse the project that owns a root on unnamed re-index (#2134) #2344, for index_repository without name silently creates a duplicate index #2134) and your project_has_committed_index() both landed right after project_db_is_servable().

Keeping both sides resolves the text in each case. One thing is worth a look while you're there: #2344 changed how handle_index_repository picks the project name for an unnamed re-index (it now reuses the project that owns the root). Your headroom decision reads the pipeline's project name at that same site, so project_has_committed_index() should be asked about the name that is actually being indexed.

Could you rebase onto current main? We'll re-verify the merge result and merge right after. Thanks again for your patience with this one!

[mvanhorn]

Rebased onto current main and pushed (26eece5). Both conflicts kept both sides: cbm_pipeline_export_error() and cbm_pipeline_set_background() now sit one after the other in pipeline.c, and index_root_owner_append() / index_root_owner_resolve() sit ahead of project_has_committed_index() in mcp.c.

On the unnamed re-index: handle_index_repository swaps in the root owner's name before cbm_pipeline_set_project_name(), and the headroom check reads cbm_pipeline_project_name(p) after that, so project_has_committed_index() is asked about the reused project, not the path-derived one. No change was needed there.

I ran make -f Makefile.cbm test-par on the rebased branch: 8327 passed, 0 failed, 10 skipped.

[DeusData]

Thanks for the rebase and for checking the reused project name in the headroom decision. I have read the October 3 results. The remaining verification of that updated head and landing decision are maintainer-side work; no further routine rebase is requested here.

[DeusData]

Thank you, @mvanhorn, for sticking with this through four careful rounds, and for checking the reused project name in the headroom decision after #2344 -- that was exactly the right thing to verify. We have now finished the maintainer-side verification of 26eece5 and pushed two commits on top of your branch so it can land without another round trip for you:

1. Merged current main. main had moved about a thousand commits, and the branch conflicted in six places, all same-spot insertions. Each one keeps both sides:

   • the supervised worker args carry main's doc_candidates setting and your _cbm_background flag;
   • autoindex_thread applies the doc_candidates choice first, then your committed-index headroom check;
   • the new "frozen" pipelines on main still force a single worker in the extraction and incremental paths, and every other pipeline uses your cbm_pipeline_worker_count();
   • in closure_probe_surfaces, main added an abort for a failed definition pass; it is kept, and your set-and-clear of probe_ctx.pipeline still wraps only the surface call.

2. An explicit index joins a running automatic job again. While verifying, we found one interaction that the earlier review rounds (ours included) missed. The daemon lets a new index_repository request join a job already running for the same project only when both requests have the same options, and it decides that by comparing the request arguments. Automatic jobs now carry _cbm_background, so a plain explicit index_repository issued while a watcher re-index or session auto-index of that project was running no longer matched, and was refused with "another index operation for this project is active with different options". Since _cbm_background only picks the worker count and never what gets indexed, the comparison now ignores it, the same way it already ignores default-valued options. A new daemon_application test holds an auto-index job open, checks that it carries the flag, and sends an explicit request for the same root: without the change it is refused as an options conflict, with it the request joins the running job.

What we checked on the result: the pipeline, mcp, daemon_application, incremental, pipeline_frozen, parallel, worker_pool and index_supervisor suites pass, and the lint checks are clean. We also ran the built binary on a 341-file C tree: a project's first index used all 18 workers on our machine even when it was started automatically, a background refresh of an already-indexed project used 5, an explicit re-index kept 18, and CBM_WORKERS still takes precedence over both. Nodes and every edge type matched across the two widths, except the semantic-similarity edges, which already vary slightly from run to run at any width.

Thanks again for a well-tested change, and for your patience with a fast-moving main.

[DeusData]

Thanks for the CPU-headroom work and for carrying it through the review rounds. I added two maintainer commits:

• 1d7ce9ba6: cap the surface-building callbacks at the total worker budget, including the calling thread. A deterministic regression reproduced three simultaneous callbacks with a budget of two; the fix holds that to two, and reverting it reproduces the failure. All 44 targeted macOS ASan/UBSan checks passed, including the serial override and output parity.
• 38a3ddb2: merge the repaired main baseline so fresh CI picks up the language-count and CodeQL fixes.

The agreed policy remains unchanged: the first automatic index keeps full-width throughput; background refreshes of an existing committed index leave CPU headroom, with explicit worker and single-thread overrides preserved. I am also correcting the description to reflect that final policy. Your original commits remain intact. Thanks again for the contribution.

[DeusData]

Thank you again for the CPU-headroom work. The October 21 update records the surface-budget correction and baseline refresh, with the agreed worker policy preserved. The next follow-up remains on our side; sorry for the delay, and I’ll follow up soon.

[DeusData]

Thank you again, @mvanhorn, for the CPU-headroom work and for your patience while main keeps moving. Since the October 21 refresh, #2737 reshaped how the daemon admits watcher and background index jobs, and it landed on the same lines as your automatic-job marker in src/daemon/application.c, so the branch no longer merged cleanly.

We pushed one merge commit on top of your branch (173e11a); your own commits are unchanged:

1. Merged current main. The one conflict keeps main's new admit/wait structure and carries your _cbm_background marker into it, so session auto-index and watcher re-index jobs still leave CPU headroom, and an explicit index_repository still joins a running automatic job. Against main, the branch's diff is exactly the change reviewed before, line for line.

Fresh CI will run on the new head. If you push to this branch again, please pull first so this merge is not lost. Thanks again for a careful, well-tested change.

[DeusData]

Thank you again, @mvanhorn, for the CPU-headroom work. The policy you built is the one we want, and because you routed every all-core site through one function, this follow-up is a single small change. While measuring the branch on real projects we found one interaction worth guarding, so we pushed one more commit on top of your branch; your own commits are unchanged.

1. db74648 fix(pipeline): keep background refreshes on the parallel route. A background refresh uses the performance cores minus one, which is a single worker on a machine with two or fewer performance cores. With one worker the indexer takes its sequential route, and on main today that route still builds a slightly different graph than the parallel one: on a Rust project we measured 49 fewer nodes, HANDLES edges dropping from 22 to 0, and about 380 fewer CALLS edges. That gap is ours and predates your change, but with the new policy small machines would have hit it on every automatic refresh, so a background refresh could quietly produce a different graph than a manual index of the same code. The background width is now floored at two whenever the foreground width is two or more. A machine whose foreground width is one keeps one, CBM_WORKERS and CBM_INDEX_SINGLE_THREAD keep their precedence, and larger machines keep exactly the headroom you chose. At two workers the graph matched the full-width graph exactly on all four projects we measured. Think of the floor as a graph-quality guard: it can go away once the sequential route builds the same graph as the parallel one. The worker-policy tests in the pipeline and mcp suites now expect the floored width, and a new pipeline test pins the width for one-core, two-core and larger machines.

If you push to this branch again, please pull first so this commit is not lost. Thanks again for a careful, well-tested change.

[DeusData]

Thanks again, @mvanhorn. Several other pull requests landed on main this evening, including changes in files this one also touches, so we merged current main into your branch once more (one signed-off merge commit, a9fceeb; your changes are untouched). That keeps the required checks honest against today's tree; we will merge once they are green. Please pull before pushing again.

[DeusData]

Thank you again, @mvanhorn, and sorry for one more push on your branch. When we merged current main in this evening, a test that main had just added (manifest-skip-locked, from 9f57edf) still called cbm_pipeline_build_semantic_manifest with the old signature, while your branch gives that function a leading pipeline parameter. The files merged without a textual conflict, but the combined tree did not compile. We pushed one line (4316e93) that passes NULL as the first argument, exactly like your own manifest-fail-closed test does. Your changes are untouched. Please pull before pushing again.

[DeusData]

Thank you for your patience, @mvanhorn. CI went green on this branch, but this evening's merges on main touched files this pull request also changes, so that green no longer covered today's main. We merged current main into the branch once more (one merge commit, acd0b5b; your changes are untouched) and will merge once the fresh checks are green. Please pull before pushing again.

[DeusData]

Thank you again, @mvanhorn, for your patience with this one. Two pull requests merged into main a few minutes ago (#2754 and #2090), and they touch src/pipeline/pipeline.h, which this pull request also changes. So we merged current main into your branch once more (one merge commit, 79d02a6; it merged cleanly and your own change is untouched), checked that the merged files still compile, and started a fresh CI run. We will merge as soon as the required checks are green. Please pull before pushing again so the merge commit is kept.

[DeusData]

Thank you for your patience, @mvanhorn. A pull request that merged into main a few minutes ago (#2769) changes src/pipeline/lsp_surface.c and pipeline.c, which this pull request also changes, so the CI run in progress here could no longer vouch for today's main. We merged current main into your branch once more (one merge commit, ed95814; it merged cleanly and your own change is untouched) and started a fresh run. We will merge once the required checks are green. Please pull before pushing again.

[DeusData]

Thank you for your patience, @mvanhorn. A pull request that merged into main a few minutes ago (#2600) changes src/mcp/mcp.c, which this pull request also changes, so the CI run in progress here could no longer vouch for today's main. We merged current main into your branch once more (one merge commit, 7854e1c; it merged cleanly and your own change is untouched) and started a fresh run. We will merge once the required checks are green. Please pull before pushing again.

[DeusData]

Thank you for your patience, @mvanhorn. A pull request that merged into main tonight (#2601) changes src/pipeline/pipeline_incremental.c, which this pull request also changes, so CI on this branch could no longer vouch for today's main. We merged current main into your branch once more (one merge commit, 88375be; it merged cleanly and your own change is untouched) and started a fresh run. We will merge once the required checks are green. Please pull before pushing again.