refactor(provider): put the JNI provider's pure half on the PIT gate, and close TODO #51

[bernardladenthin]

Summary

LlamaCppJniAiGenerationProvider is the one production class where a defect has actually reached users, and it was not on targetClasses. That is why the gate read a stable 775/775 straight through the 1.1.0-era dry_penalty_last_n regression and through its fix: a class that generates no mutants cannot move the number, whether the tests are adequate or not. TODO #51 asked to fix that, and — before committing to it — to measure, because the pure logic is also exercised by model-backed tests and PIT re-runs every test covering a mutated line.

Both are done. The measurement's answer is not the one the TODO guessed.

Why extraction, and not just excludedTestClasses

Checked rather than assumed. Putting the provider on the gate unchanged and excluding the model-backed test classes does not work: model()'s mutants would then have no coverage at all, and a NO_COVERAGE mutant fails a threshold-100 gate exactly like a survivor. Only a class whose every line is covered model-free can go on the gate — so the split is forced, not a matter of taste.

provider.LlamaCppJniProviderSupport (new) takes everything that does not touch the native handle: buildInferenceParameters, buildChatTemplateKwargs, warnOnTruncatedAnswer, logPromptCacheReuse, the static lazyMode/cacheType resolvers and their two known*Values renderers. The provider keeps model(), generate(), chatResponse(), generateWithTimings(), close() — and loses its LOGGER, since it no longer logs anything itself.

The boundary is "touches the native handle", and it is load-bearing: a pure method moved back into the provider silently drops off the gate; an impure one moved into the support class reds it. CLAUDE.md now says so.

The tests split the same way. Only two of the old test class's cases were model-bound (generate_realProvider_*, generateWithTimings_realProvider_*, both behind NativeLlamaAvailability.assumeAvailable()); every test of the pure methods already built its provider over LlamaCppJniConfig.builder("/does/not/exist.gguf"), since model() is lazy. So the pure paths already had full model-free coverage — they just had no mutants to prove it.

The measurement

	mutations	killed	wall clock
before	807	807 (100 %)	11:34 min
with the split	830	825 (99 %)	15:57 min
with the survivors killed	830	830 (100 %)	12:14 min

So the extension costs ~40 s for 23 mutations, and excludedTestClasses — which the TODO expected to be necessary — is not used. It would remove a test class from mutant matching for every target class, not just this one, and the number says it buys nothing. The 15:57 reading in the middle is not the cost of the split: PIT exhausts every covering test for a mutant that never dies, while a killed one stops at the first failure, so survivors inflate the clock. Worth stating because I misreported that figure once before re-running.

The five survivors — all real gaps, none equivalent

• known{CacheType,LazyMode}Values, known.length() > 0 → >= 0. The renderer emits its separator from the second value on; with the mutant the message reads expected one of: , f32, …. Both rejection tests asserted containsString("f32, f16, bf16, q8_0"), which sits in the middle of the list and cannot see a stray leading separator. Each now also anchors on "expected one of: <first value>".
• config.seed() >= 0, boundary and negation. The seed had no model-free coverage at all. Two tests: an unconfigured run sends no seed (the sentinel is -1 = "random per request"; forwarding it would pin every run to seed −1 and destroy the randomness upstream provides), and seed 0 is sent — 0 is a legitimate seed, not a second sentinel, so the guard must be >= 0. Same shape as the penalty-window pair already there.
• config.drySequenceBreakers().isEmpty(), negated. An empty list means "keep the binding's default set", not "clear it". One test per branch.

Test plan

• Affected unit / integration tests pass locally — 658 tests, 0 failures, 0 skipped (654 before; the 4 new ones are the seed and sequence-breaker pairs). Identical count across the split, so nothing was lost in the move.
• mvn -f srcmorph/pom.xml test-compile pitest:mutationCoverage → 830/830 killed, 100 %, 0 mutations with no coverage, test strength 100 %, BUILD SUCCESS
• spotless:apply run; working tree clean afterwards
• CI is green on this branch — it cannot be: main deliberately pins net.ladenthin:llama:5.2.0, which Central does not carry yet, so resolution fails before the first test compiles. That is unchanged by this PR and is tracked separately. To verify locally, install jllama main as 5.2.0 first (versions:set -DnewVersion=5.2.0 → install -pl llama -am → git checkout -- .), which is how the numbers above were produced.
• Docs updated — CLAUDE.md 807 → 830 plus the boundary rule and the measurement; TODO.md's entry deleted rather than annotated, per that file's own header.

Related issues / PRs

Closes TODO #51. Supersedes the abandoned claude/pit-provider-measure probe branch, which took the "gate the provider as-is and measure" route — that branch's approach is the one refuted above, and it can be deleted once this lands.

Checklist

• I have read CONTRIBUTING.md and CODE_OF_CONDUCT.md
• My commits follow Conventional Commits
• No security-sensitive changes — pure refactor plus tests; no production behaviour changes, every moved method is byte-identical apart from its new home

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH

---

Generated by Claude Code

[claude]

Excellent Javadoc. This explanation of why the split exists along the "touches the native handle" boundary is both precise and load-bearing. The history of the dry_penalty_last_n regression (1.1.0/1.1.1) passing a 775/775 gate is a perfect concrete example of why mutation testing alone cannot replace proper architectural boundaries.

[claude]

Strong test design: pinning the boundary from multiple angles. The empty list test confirms an empty configured set does NOT get forwarded (preserving the binding's default), while the configured test asserts it does reach the request. This pattern is repeated throughout for seed, penalty windows, and enable_thinking — each test name explicitly documents what boundary it verifies and why the guard matters.

[claude]

This test and its comment are documenting exactly why the refactoring matters: the native binding rejects negative penalty windows, so forwarding a -1 sentinel kills every generation before a token is produced. The dry_penalty_last_n regression of 1.1.0/1.1.1 did exactly this, and the gate read 775/775 straight through it because the provider wasn't on targetClasses. This extraction puts the guard code itself under mutation testing where it belongs.

[claude]

✓ Correctly added to targetClasses for PIT mutation testing at 100% threshold. The new class holds the guardcode that prevented the 1.1.0/1.1.1 regression from being caught, so its presence on the gate directly closes the coverage gap that existed before.

[claude]

Code Review: APPROVED ✓

This is a well-executed refactor that closes a real gap in mutation testing coverage while maintaining strict code quality standards.

Strengths

Architecture & Design

• Clean separation along a load-bearing boundary: "touches the native handle" (see LlamaCppJniProviderSupport Javadoc lines 24–45). This is not a stylistic choice; it's the only way to get pure-logic methods under the mutation gate without dragging in 36 model-loading test cases (~22.9 s each).
• Extraction of 23 new mutations to gate (807 → 830), all killed at 100%, adds ~40 s total runtime — excellent ROI.

Test Coverage (530 lines, 4 new test methods)

• Boundary conditions thoroughly pinned from multiple angles:
  • Seed: 0 is a valid seed (not sentinel -1 for "random per request")
  • Penalty windows: must guard against -1 forward (binding rejects negatives)
  • Sequence breakers: empty list means "keep binding default", not "clear to empty"
  • Enable thinking: unset (null) vs false vs true — each is a different configuration
• Regression locked in: the dry_penalty_last_n bug from 1.1.0/1.1.1 directly motivated this split. That bug killed every generation before a token was produced, but the gate read 775/775 straight through it because the provider wasn't on targetClasses. The new tests and extracted class prevent that class of defect from hiding again.

Documentation

• Javadoc in LlamaCppJniProviderSupport is exemplary: explains the history (1.1.0/1.1.1 regression), the constraint (PIT reruns every covering test for each mutant), and the trade-off reasoning.
• CLAUDE.md updated with the boundary rule and measurement data, not just numbers.
• Test names + inline comments document why each boundary matters.
• TODO Document llama dependency in README #51 fully retired (deleted, not annotated).

Code Quality

• Byte-identical extraction — moved code is unchanged except location
• All existing tests pass (654 → 658 after the 4 new support-class tests)
• No behavioural changes; pure refactor
• @ToString.Exclude annotations correct (support class detail, native handle noise)
• Package-private support class is correct (only the provider and tests need it)

Observations (not blockers)

1. The measurement data (11:34 min → 15:57 min with split → 12:14 min with survivors killed) proves that the TODO's initial assumption ("excludedTestClasses might be necessary") was wrong. This is valuable evidence that the design decision was correct.

2. The five survivors identified in the PR summary are all real gaps, not equivalent mutants:

   • knownCacheTypeValues() / knownLazyModeValues() separator logic (boundary > 0)
   • seed >= 0 (0 is valid, -1 is sentinel)
   • drySequenceBreakers().isEmpty() (empty = keep default)
     Each has a new test pinning the boundary.

3. LlamaCppJniProviderSupport is package-private by design. The static enum-resolver methods are only called from the provider's model() during lazy load (which still touches the native handle, so it stays in the provider). This is correct.

Summary

This PR directly closes a defect in the mutation-testing gate that allowed the dry_penalty_last_n regression to ship twice. The refactoring is minimal (moving pure methods), well-tested (530 new LOC), and improves the architecture by properly separating concerns. No security, correctness, or performance regressions.