[halindrome]

Summary

Run supervised index workers (cli --index-worker) at below-normal CPU priority so a long full index can't starve interactive CLI/MCP queries.

Refs #2834

During a full index the parallel extraction pass saturates every core. One-shot codebase-memory-mcp cli <tool> queries are CPU-bound at start-up, and measurement showed their wall time doubling during an index while their CPU time stayed flat — run-queue contention, not a lock (same-project and other-project queries behaved identically). Nothing in src/ lowered worker priority before this change.

Change

• cbm_lower_cpu_priority() (src/foundation/platform.c/.h): POSIX setpriority(PRIO_PROCESS, 0, 10); Windows BELOW_NORMAL_PRIORITY_CLASS. It never raises priority: an already-nicer process, or one in the IDLE / BELOW_NORMAL class, is left alone. Plain nice only — not macOS background QoS, which would pin indexing to efficiency cores and throttle its I/O.
• run_cli (src/main.c) calls it once in the index-worker role, before any indexing thread exists, so every pipeline thread inherits it. Failure is logged at info and ignored.

Acceptance criteria

1. A process started in the index-worker role runs at nice >= 10 on POSIX (below-normal priority class on Windows) before it creates indexing threads.
2. A process that is already nicer than 10 is never raised.
3. Failing to lower priority never fails the index; it is logged once.
4. Non-worker roles (daemon, MCP stdio, one-shot CLI) keep their priority unchanged.
5. Existing index-supervisor / worker suites stay green.

Testing

• New platform_lower_cpu_priority_nices_the_process (tests/test_platform.c): forks a child, calls the helper, and reports getpriority through a pipe. It checks two things:
  • A normal-priority child ends at nice >= 10.
  • A child started nicer than that (15, or 19) is left where it was.
  • Red with the helper stubbed, and red with the never-raise branch removed; green with the change. Skipped on Windows.
• scripts/test.sh --suites platform,index_supervisor,index_resilience,worker_pool,cli: 442 passed, 1 pre-existing skip, 0 failed.
• Windows branch not compiled locally (no Windows toolchain here).

🤖 Generated with Claude Code

https://claude.ai/code/session_01CDDUoWEZMNZ9fLvzp2pPGT

[halindrome]

QA round 1 for this change. It ran on the fork PR (halindrome#4) before the commits were squashed into 349d80c, so any commit SHAs below refer to the pre-squash history. The QA was performed by Claude Opus 5.5 (Anthropic) in Claude Code.

QA Round 1

Contract: #2834 + PR acceptance criteria (CPU-priority half only; the per-call executable hash is out of scope)

Contract Verification

#	Criterion	Verdict	Evidence
1	cli --index-worker runs at nice >= 10 (BELOW_NORMAL on Windows) before indexing threads are created	✅ pass	src/main.c:905-911: run_cli calls cbm_lower_cpu_priority() inside if (index_worker), before cbm_mcp_server_new or any indexing dispatch. The only earlier thread is the parent-death watchdog (main.c:565), which is not a pipeline thread.
2	An already-nicer process is never raised	⚠️ partial	POSIX: met. platform.c:744-748 returns early when getpriority() >= 10, and it checks errno correctly. Windows: not met, see Finding 1.
3	A failure to lower priority never fails the index; it is logged once and ignored	✅ pass	main.c:908-910 logs index.worker.priority lowered=false once and has no early return.
4	Non-worker roles keep their priority	✅ pass	The only call site is main.c:908, gated on index_worker.
5	macOS uses plain nice only (no background QoS)	✅ pass	The code uses only setpriority. It makes no setiopolicy_np, pthread_set_qos_class_self_np or PRIO_DARWIN_BG call.
6	Existing suites stay green; a new test proves the helper lowers priority	⚠️ partial	The new test platform_lower_cpu_priority_nices_the_process (tests/test_platform.c:1448-1479) exists and is registered. Lenses were told not to run the suite, so "stays green" is not verified by this round. The test's red-without-change claim depends on the environment, see Finding 3.

Finding 1 — Windows path can raise an IDLE-class worker to BELOW_NORMAL

[claude:contract-security|regression-edges] · minor · contract · confirmed · src/foundation/platform.c:739-742

The POSIX branch checks the current nice value before changing it. The Windows branch calls SetPriorityClass(GetCurrentProcess(), BELOW_NORMAL_PRIORITY_CLASS) without checking GetPriorityClass() first. Child processes inherit the IDLE class, so a daemon an operator deliberately started at idle priority (start /low) has its workers raised to BELOW_NORMAL. That breaks criterion 2 and the "never raises an already-nicer process" promise in platform.h:221-228. The impact in practice is small: it only happens on Windows when the operator chose idle priority, and the result is still below normal.

Expected: return early when the current class is already IDLE_PRIORITY_CLASS or BELOW_NORMAL_PRIORITY_CLASS.

Finding 2 — No test covers the worker-role wiring

[claude:test-quality] · minor · regression · confirmed · src/main.c:902-911

The new test exercises the helper on its own. Removing the 6-line block in run_cli, or moving it after thread creation, fails no test, so criterion 1 has no test. This is a coverage gap, not a behaviour defect, and it may be acceptable to defer to a follow-up.

Expected: some test fails if the index-worker role stops lowering its priority.

Finding 3 — New test passes vacuously under an already-niced runner; the "never raise" branch is never exercised

[claude:test-quality] · minor · regression · confirmed · tests/test_platform.c:1448-1479

The test asserts only returned true and nice >= 10 afterwards. When the runner is already at nice >= 10, an empty helper body passes. No leg starts above 10 and checks that the nice value is unchanged.

Expected: record the starting nice and assert a strict change when it starts below 10. Add a leg that starts above 10 and asserts the value is unchanged.

Summary

Severity	Confirmed
Critical	0
Major	0
Minor	3

Panel: contract-security, regression-edges, test-quality. All 3 lenses landed on claude-opus-5-5 with cmm+ctx navigation, and the working tree was unchanged.

Observations

None.

---

SAST review not applicable

Target default has security_stage: false in the qa-cycle config. No CI
security stage is wired for this target, so no SAST/SCA delta is computed.

---

QA performed by Claude Code (claude-opus-5-5), manager + 3-lens panel

Dispositions (orchestrator, Step 3B)

• Fixed: Windows path could raise an IDLE-class worker. cbm_lower_cpu_priority now leaves IDLE / BELOW_NORMAL processes untouched.
• Fixed: the test could pass vacuously under an already-niced runner. The child now also starts nicer than the helper's level (15, or 19 if already ≥15) and asserts the helper leaves it there. Red-first check: with the "never raise" early return disabled, the test fails at tests/test_platform.c:1485 (report[2] == 0). It passes with the code restored.
• Not fixed (carried forward): no test covers the worker-role wiring in run_cli. A wiring test would need to spawn a real index worker; that costs more than the one-line call it guards.

Verification

CC=/usr/bin/clang CXX=/usr/bin/clang++ SDKROOT=… scripts/test.sh --suites "platform index_supervisor index_resilience worker_pool" → 70 passed, 0 failed (exit 0). This is narrower than CI's default leg; the full default scripts/test.sh stops at contract Step 0t in this local environment, which unmodified main also fails.

QA-Fix-Commit: 704a208

---

Posted by the developer account (QA-agent token unavailable). Review by Claude Opus 5.5 (Anthropic) in Claude Code via qa-panel.

[halindrome]

QA round 2 for this change. It ran on the fork PR (halindrome#4) before the commits were squashed into 349d80c, so any commit SHAs below refer to the pre-squash history. The QA was performed by Claude Opus 5.5 (Anthropic) in Claude Code.

QA Round 2

Scope: PR #4 (perf/index-worker-priority → main). This round re-reviews the full diff with round 1's fix commit 704a208 applied (Windows never-raise for the IDLE class, plus the test's never-raise leg). Panel: contract-security, regression-edges, test-quality, all on claude-opus-5-5.

Contract Verification

Contract: upstream issue #2834 + PR acceptance criteria.

#	Criterion	Verdict	Evidence
1	The index-worker role (cli --index-worker) runs at nice >= 10 on POSIX (below-normal priority class on Windows) before any indexing thread exists	✅ pass	src/main.c run_cli: cbm_lower_cpu_priority() is called inside if (index_worker) before cbm_index_set_worker_role_options and tool dispatch. The helper (src/foundation/platform.c) calls setpriority(PRIO_PROCESS,0,10) on POSIX and SetPriorityClass(BELOW_NORMAL) on Windows. The only threads started earlier are the watchdog and monitor threads, which mostly sleep.
2	Priority is never raised for a process that is already nicer than 10	✅ pass	POSIX returns early when errno==0 && current >= 10. Windows returns early for IDLE/BELOW_NORMAL (round-1 fix). The test's never-raise leg checks this.
3	If lowering priority fails, it is logged once and the index continues	✅ pass	main.c: a false return only calls cbm_log_info("index.worker.priority","lowered","false"). It runs once per process.
4	Non-worker roles keep their priority	✅ pass	There is one call site, gated by index_worker. search_code finds no other caller.
5	macOS uses plain nice only (no background QoS)	✅ pass	The code only calls setpriority. There is no setiopolicy_np or pthread_set_qos_class call.
6	Existing suites stay green; a new test proves the helper (red without the change)	✅ pass	New test platform_lower_cpu_priority_nices_the_process is registered in SUITE(platform). The read-only lenses rated this partial because they cannot run tests. The orchestrator ran scripts/test.sh --suites "platform index_supervisor index_resilience worker_pool": 70 passed, 0 failed.

Finding 1 — [claude] No test pins the index-worker wiring: removing or moving the main.c call fails nothing

• Severity: minor · Status: confirmed · Relevance: regression · Category: test-gap
• Location: src/main.c:905-910
• Description: The new test covers cbm_lower_cpu_priority() on its own. No test checks that a cli --index-worker process actually runs at nice >= 10, or that the call happens before the indexing threads are created.
• Evidence: Deleting the call at lines 908-910, or moving it after thread creation (nice is per-thread on Linux), would leave every test green.
• Impact: A later refactor of run_cli could silently drop this behaviour. The lens noted that covering it needs a spawned-worker harness, which is larger than this 6-line wiring, so a follow-up ticket fits better than a change to this PR.
• Expected: Some test fails if the worker role stops lowering its priority.

Finding 2 — [claude] Helper test passes vacuously when the test runner is already at nice >= 10

• Severity: minor · Status: confirmed · Relevance: regression · Category: test-gap
• Location: tests/test_platform.c:1458-1486
• Description: The first leg asserts report[1] >= CBM_LOW_CPU_PRIORITY_NICE. If the suite runs under nice -n 10 or higher, the forked child inherits that nice and the helper takes its early return. Both legs then pass even if setpriority were removed.
• Impact: This depends on the environment. Only a CI runner that is already niced loses the red-without-change guarantee.
• Expected: The test checks that the nice actually changed from a starting value below 10, or skips with a message when the runner is already at nice >= 10.

Summary

Severity	Confirmed
Critical	0
Major	0
Minor	2

No blocking findings. Both minors are test-coverage gaps. Neither is on code that round 1 introduced.

Observations

None.

Fix review: <filled in by main after Step 3B>

---

SAST review not applicable

Target default has security_stage: false in the qa-cycle config. No CI
security stage is wired for this target, so no SAST/SCA delta is computed.

---

QA performed by Claude Code (claude-opus-5-5), manager + 3-lens panel

Dispositions (orchestrator, Step 3B)

No code changed this round. Both minors were declined as known test limits:

• Worker-wiring test: already carried forward from round 1. A test would need to spawn a real index worker.
• Helper test under an already-niced runner: a non-root process cannot lower its nice value, so the first leg cannot force a starting nice below 10. The round-1 second leg starts the child at nice 15 or 19 and asserts the helper leaves it there. It is shown to fail when the "never raise" branch is removed.

Cycle outcome: clean of critical and major findings after round 2, and every contract criterion passes. Not approved: no QA-agent token is available, and approval is the human reviewer's call.

---

Posted by the developer account (QA-agent token unavailable). Review by Claude Opus 5.5 (Anthropic) in Claude Code via qa-panel.

[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]

Thank you, @halindrome -- this is a clean, well-scoped change. Lowering the priority once, inside the index-worker role, covers every path that spawns a worker (index_repository from MCP and the CLI, the crash-retry and single-thread fallback rounds, and the daemon's background re-index), and the never-raise rule plus the forked-child test make the contract easy to trust. We also re-checked the graph side: indexing the same corpus (redis) with and without the change gives identical nodes and edges, and with the change the worker really runs at nice 10 while the CLI and daemon keep their priority.

Two small things we did on our side so it can land:

1. We merged current main into the branch (a plain merge commit; your commit is untouched). main had moved on in src/main.c since your base, in a different part of run_cli, so this only re-runs CI against the current tree.
2. We changed "Closes perf(cli): every one-shot cli call hashes the whole executable; under a concurrent index this dominates query latency #2834" to "Refs perf(cli): every one-shot cli call hashes the whole executable; under a concurrent index this dominates query latency #2834" in the description, because perf(cli): every one-shot cli call hashes the whole executable; under a concurrent index this dominates query latency #2834 also covers the per-call executable-hash cost, which stays open as its own piece of work.

If you push again, please pull first so the merge commit is not lost. We will merge as soon as the required checks are green. Thanks again!

[DeusData]

Merged in 7a9b1cb. Thank you very much, @halindrome! One nice-10 call in the worker role covers every place we spawn an index worker, so supervised indexing now stays out of the way of the editor and the agent without any change to what gets indexed - the graph bench on redis came out identical (same nodes, edges and set hashes). That is exactly the kind of low-risk, measurable improvement we like to land.

As noted on the PR, #2834 stays open for the per-call executable-hash cost, which is its own piece of work. Thanks again for the careful, well-scoped change.