Repository navigation
Threads default to min(4, cores): PIT back from hours to minutes, 100 % again - #245
Merged
Merged
Conversation
bernardladenthin
had a problem deploying
to
maven-central
October 26, 2026 15:24 — with 
GitHub Actions
Failure
GitHub Actions
Failure
bernardladenthin
had a problem deploying
to
maven-central
October 26, 2026 15:24 — with
GitHub Actions
Failure
GitHub Actions
Failure
|
Caution Review failedThe pull request is closed. No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Set and propagate the default thread count srcmorph/src/main/java/net/ladenthin/srcmorph/config/*, srcmorph-maven-plugin/src/main/java/net/ladenthin/maven/srcmorph/mojo/*, srcmorph-maven-plugin/README.md, CHANGELOG.md |
AiGenerationConfig.DEFAULT_THREADS is now the smaller of four and the available processor count. DEFAULT_LLAMA_THREADS and the Maven goals use this value. The README and changelog document the default. |
Align thread configuration and test model paths srcmorph/src/test/java/net/ladenthin/srcmorph/NativeLlamaAvailability.java, srcmorph/src/test/java/net/ladenthin/srcmorph/config/*, srcmorph/src/test/java/net/ladenthin/srcmorph/indexer/*, srcmorph/src/test/java/net/ladenthin/srcmorph/provider/*, CHANGELOG.md |
Real-model tests use NativeLlamaAvailability.THREADS. Other tests use explicit thread values of 7. A test checks that resolveModelPath leaves a NUL-containing path unchanged. |
Bound PIT step runtime .github/workflows/publish.yml |
The PIT mutation-testing step has a 60-minute timeout. Its Maven invocation is unchanged. |
Priority:
Estimated code review effort: 2 (Simple) | ~12 minutes
Change: Bug fix
Suggested reviewers: claude
Merge Risk: 
90ead
-
No actionable merge-blocking risk remains from the reviewed changes.
Security architecture risk: 90ead
-
The changes adjust default CPU usage and bound validation time without expanding access or weakening release gates. No material security risk was identified in the reviewed changes.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- observed — The changed resource policy affects inference thread parameters used by existing core and Maven execution paths. The workflow change is confined to the PIT test step, before publication preparation.
Trust Boundaries and Controls
- observed — Explicit thread configuration remains a caller-controlled resource setting: both configuration setters assign the supplied integer directly. The processor-aware default does not introduce validation or enforce a CPU quota, and these setter semantics predate the PR.
-
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 15 files. (3 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly identifies the main change: reducing the default thread count to min(4, cores) to restore practical PIT runtime and coverage. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 15 files. (3 skipped: 3 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
The first main run past its tests since llama 6.0.0 (40316163607) sat in the PIT step for over two hours until it was cancelled. Two real-model tests used threads(8) on the 4-vCPU runner. ggml synchronises its threads on spinning barriers, so more threads than cores is orders of magnitude slower: - on four visible cores (docker --cpuset-cpus=0-3) the same test class took 644.9 s with 8 threads and 7.8 s with 4; - in CI one generation took 431 s and PIT's coverage pass 904 s, against 41 s locally; - PIT re-runs these tests for every mutant they cover. The production default had the same flaw: 8 threads for a model definition on every four-core machine. - AiGenerationConfig.DEFAULT_THREADS = min(4, cores), formerly 8. The fallback default SrcMorphConfiguration.DEFAULT_LLAMA_THREADS (formerly 2) and the four mojos' srcmorph.llama.threads (formerly defaultValue "2") now take the same value. - The real-model tests and the knob sweep use it via NativeLlamaAvailability.THREADS. - The PIT step gets a 60-minute timeout. - AiModelDefinitionSupport.hasRoot's InvalidPathException branch, the one mutant without coverage, is tested with a NUL-character path. - Config tests that set threads to 4 now set 7. 4 is the new default, so a removed setter call would no longer have been noticed (PIT's survivor in AiModelDefinitionSupport.toConfig). Verified on four visible cores: - mvn verify with requireNativeLlama: 669 + 39 + 32 tests green; - reactor-wide PIT: 843/843, 16/16, 63/63, 10.6 min in all (coverage 34 s). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AJGh28Gu8pLWYbTCqG1LZf
bernardladenthin
force-pushed
the
fix/real-model-test-threads
branch
from
October 26, 2026 17:17
90eadef to
4b7fc3f
Compare
bernardladenthin
had a problem deploying
to
maven-central
October 26, 2026 17:17 — with
GitHub Actions
Failure
GitHub Actions
Failure
bernardladenthin
had a problem deploying
to
startgate
October 26, 2026 17:17 — with
GitHub Actions
Error
GitHub Actions
Error
bernardladenthin
had a problem deploying
to
maven-central
October 26, 2026 17:17 — with
GitHub Actions
Failure
GitHub Actions
Failure
|
This branch had an error being deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The first
mainrun since llama 6.0.0 to pass its tests (40316163607) sat in Run PIT mutation tests for over two hours until it was cancelled. On 1 September the same step took 7 minutes.Cause: two real-model tests ran llama.cpp with
threads(8)on the 4-vCPU runner. ggml synchronises its threads on spinning barriers, so more threads than cores is not a little slower but orders of magnitude slower: every barrier waits for a thread the OS has descheduled. PIT re-runs these tests for every mutant they cover.LlamaCppJniAiGenerationProviderTeston 4 visible cores (docker --cpuset-cpus=0-3)generate_realProvider_returnsNonEmptyResponse)It went unnoticed locally because Docker throttles CPU time but leaves all host cores visible.
The production default had the same flaw: 8 threads for a model definition on every four-core machine. As agreed, the default is now
min(4, cores):AiGenerationConfig.DEFAULT_THREADS(was 8);SrcMorphConfiguration.DEFAULT_LLAMA_THREADS(was 2);srcmorph.llama.threads(wasdefaultValue = "2").A configuration that sets
threadsexplicitly is unchanged. The plugin README's table and the CHANGELOG (Changed) say so.Also in this PR:
NativeLlamaAvailability.THREADS).timeout-minutes: 60, so a regression cannot burn the six-hour job limit again.AiModelDefinitionSupport.hasRootreturnstruefor an unparsable path, and no test covered it (842/843). It is now tested with a NUL-character path.threadsto4now set7. 4 is the new default, so a removed setter call would no longer be noticed; that was PIT's survivor inAiModelDefinitionSupport.toConfig.Verified on 4 visible cores (
maven:3.9-eclipse-temurin-25):mvn verify -Dnet.ladenthin.srcmorph.requireNativeLlama=true: 669 + 39 + 32 tests, BUILD SUCCESS.Not yet seen on GitHub: the run of this PR.
🤖 Generated with Claude Code
https://claude.ai/code/session_01AJGh28Gu8pLWYbTCqG1LZf
Summary by CodeRabbit