From 45b619cd8c97b033db4c909298b651fb32a390ca Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 20:35:51 +0000 Subject: [PATCH 1/3] refactor(provider): split the JNI provider's pure half out so PIT can reach it TODO #51. The mutation gate could not cover LlamaCppJniAiGenerationProvider, and the reason was structural rather than a missing test: PIT re-runs every test covering a mutated line, and while this logic sat next to model() the only way to put any of it on targetClasses was to put model() there too -- a ~100-line ModelParameters chain whose sole exerciser is LlamaCppJniKnobSweepTest, 36 cases that each load a GGUF. Measured here: that class takes 22.9 s, and the provider test another 29.8 s. Excluding those tests instead does not work, which is worth recording because it is the obvious first idea: model()'s mutants would then have no coverage at all, and a NO_COVERAGE mutant fails a threshold-100 gate exactly like a survivor. The only arrangement that works is one where the gated class is covered end to end by tests that need no model -- hence a split along "touches the native handle", not along any tidier conceptual line. Moved to the new LlamaCppJniProviderSupport (config + prompt support in, no native handle): buildInferenceParameters, buildChatTemplateKwargs, warnOnTruncatedAnswer, logPromptCacheReuse, the static lazyMode/cacheType resolvers and their two known*Values renderers, plus the five constants only they use. The provider keeps model(), generate(), chatResponse(), generateWithTimings() and close(), and delegates. Two consequences worth knowing rather than discovering later: * The provider no longer logs anything itself and lost its LOGGER; every log line this component emits now comes from LlamaCppJniProviderSupport. The ListAppender assertions follow that move. * lazyMode/cacheType were reachable only from model(). Checked before moving them that the same-named calls in LlamaCppJniConfigFactory are builder setters on the config, not these resolvers, so the move has exactly one caller. The test class is split the same way: LlamaCppJniProviderSupportTest holds the 20 model-free cases, LlamaCppJniAiGenerationProviderTest keeps the two real-provider ones behind NativeLlamaAvailability.assumeAvailable(). Every support case builds its config over "/does/not/exist.gguf" -- not a shortcut but the contract the class exists to keep, since model() is lazy and none of these methods touches it. Verified: 654 tests, 0 failures -- the same count as before the split, so nothing was dropped in the move. Baseline PIT for comparison, on this machine: 807/807 killed, 100%, 11:34 min, 1300 test runs. excludedTestClasses is deliberately NOT set yet. An exclusion removes that class from mutant matching for every target class, not just this one, so at 807/807 it could create survivors elsewhere. Measure what the extraction alone buys first; add exclusions only if the runtime demands it. That is what the TODO entry's "measure before committing to it" asks for, and the measurement is still running. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH --- srcmorph/pom.xml | 1 + .../LlamaCppJniAiGenerationProvider.java | 314 +------------ .../provider/LlamaCppJniProviderSupport.java | 308 ++++++++++++ .../LlamaCppJniAiGenerationProviderTest.java | 387 --------------- .../LlamaCppJniProviderSupportTest.java | 441 ++++++++++++++++++ 5 files changed, 766 insertions(+), 685 deletions(-) create mode 100644 srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniProviderSupport.java create mode 100644 srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniProviderSupportTest.java diff --git a/srcmorph/pom.xml b/srcmorph/pom.xml index 755a7b2..54c40e5 100644 --- a/srcmorph/pom.xml +++ b/srcmorph/pom.xml @@ -720,6 +720,7 @@ SPDX-License-Identifier: Apache-2.0 net.ladenthin.srcmorph.config.SrcMorphConfiguration net.ladenthin.srcmorph.provider.LlamaCppJniConfig net.ladenthin.srcmorph.provider.LlamaCppJniConfigFactory + net.ladenthin.srcmorph.provider.LlamaCppJniProviderSupport net.ladenthin.srcmorph.engine.EngineSupport net.ladenthin.srcmorph.engine.GenerateResult net.ladenthin.srcmorph.engine.CalibrationReport diff --git a/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProvider.java b/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProvider.java index 328fa16..e4dd002 100644 --- a/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProvider.java +++ b/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProvider.java @@ -4,30 +4,20 @@ package net.ladenthin.srcmorph.provider; import java.io.IOException; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Objects; import lombok.ToString; import net.ladenthin.llama.LlamaModel; -import net.ladenthin.llama.args.CacheType; import net.ladenthin.llama.args.FlashAttn; -import net.ladenthin.llama.args.LazyMode; import net.ladenthin.llama.args.ReasoningFormat; import net.ladenthin.llama.json.ChatResponseParser; -import net.ladenthin.llama.parameters.InferenceParameters; import net.ladenthin.llama.parameters.ModelParameters; import net.ladenthin.llama.value.ChatResponse; -import net.ladenthin.llama.value.Pair; import net.ladenthin.llama.value.Timings; -import net.ladenthin.llama.value.Usage; import net.ladenthin.srcmorph.document.AiGenerationRequest; import net.ladenthin.srcmorph.prompt.AiPromptSupport; import net.ladenthin.srcmorph.support.Java8CompatibilityHelper; import org.jspecify.annotations.Nullable; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * {@link AiGenerationProvider} implementation backed by the {@code net.ladenthin:llama} @@ -42,11 +32,6 @@ @ToString public final class LlamaCppJniAiGenerationProvider implements AiGenerationProvider, AutoCloseable { - private static final Logger LOGGER = LoggerFactory.getLogger(LlamaCppJniAiGenerationProvider.class); - - /** OpenAI finish reason meaning "the token budget ran out", as opposed to {@code "stop"}. */ - private static final String FINISH_REASON_LENGTH = "length"; - private final LlamaCppJniConfig config; // Native llama.cpp model handle — its toString prints native pointer / internal @@ -58,26 +43,18 @@ public final class LlamaCppJniAiGenerationProvider implements AiGenerationProvid private @Nullable LlamaModel model; private final AiPromptSupport promptSupport; - private final AiCompletionParser completionParser = new AiCompletionParser(); - private final ChatResponseParser chatResponseParser = new ChatResponseParser(); - private final Java8CompatibilityHelper compatibilityHelper = new Java8CompatibilityHelper(); /** - * Fixed llama.cpp server slot every request is pinned to. The model is loaded once and reused - * for all files; pinning each request to the same slot lets the prompt cache - * ({@code cache_prompt}) reuse the shared prompt-template prefix's KV across files instead of - * re-prefilling it for every file. Reuse is exact, so generated output is unchanged. + * Everything this provider does that does not touch the native handle. Split out so the + * mutation gate can reach it: see {@link LlamaCppJniProviderSupport}'s own javadoc for why + * the boundary runs where it does. */ - private static final int REUSE_SLOT_ID = 0; - - /** Chat-template kwarg controlling Qwen-style thinking (ignored by non-Qwen templates). */ - private static final String ENABLE_THINKING_KWARG = "enable_thinking"; - - /** Chat-template kwarg for the gpt-oss reasoning-effort level (ignored by non-gpt-oss templates). */ - private static final String REASONING_EFFORT_KWARG = "reasoning_effort"; + @ToString.Exclude + private final LlamaCppJniProviderSupport support; - /** Number of chat-template kwargs put into the map (used for presizing). */ - private static final int CHAT_TEMPLATE_KWARG_COUNT = 2; + private final AiCompletionParser completionParser = new AiCompletionParser(); + private final ChatResponseParser chatResponseParser = new ChatResponseParser(); + private final Java8CompatibilityHelper compatibilityHelper = new Java8CompatibilityHelper(); /** * Creates a new {@link LlamaCppJniAiGenerationProvider}; the GGUF model is loaded lazily on the first generate(...) call. @@ -88,45 +65,14 @@ public final class LlamaCppJniAiGenerationProvider implements AiGenerationProvid public LlamaCppJniAiGenerationProvider(final LlamaCppJniConfig config, final AiPromptSupport promptSupport) { this.config = Objects.requireNonNull(config, "config"); this.promptSupport = Objects.requireNonNull(promptSupport, "promptSupport"); - } - - /** - * Builds the chat-template kwargs passed to - * {@link net.ladenthin.llama.parameters.ModelParameters#setChatTemplateKwargs}. - * - *

Both kwargs are opt-in: an entry is written only when the user actually configured it, so a - * chat template that has never heard of the kwarg is not handed it. That matters because - * llama.cpp's Jinja layer has been moving unknown kwargs from "silently ignored" toward "warned - * about" -- while {@code enable_thinking} was a plain {@code boolean} defaulting to {@code true}, - * every run sent it, and the only way to stop the noise was to set the knob to {@code false}, - * which means something else entirely.

- * - *

Package-private so it can be pinned without loading a GGUF; {@link #model()} is the only - * production caller.

- * - * @return the kwargs to send; empty when neither knob is configured - */ - Map buildChatTemplateKwargs() { - final Map chatTemplateKwargs = - new HashMap<>(compatibilityHelper.hashMapCapacityFor(CHAT_TEMPLATE_KWARG_COUNT)); - // Qwen-style thinking. Unset (null) omits the kwarg so the model's own template default applies. - final Boolean enableThinking = config.chatTemplateEnableThinking(); - if (enableThinking != null) { - chatTemplateKwargs.put(ENABLE_THINKING_KWARG, String.valueOf(enableThinking.booleanValue())); - } - // gpt-oss honors reasoning_effort; non-gpt-oss chat templates ignore it. An empty - // configured value omits the kwarg so the model's own template default applies. - if (!compatibilityHelper.isBlank(config.reasoningEffort())) { - chatTemplateKwargs.put(REASONING_EFFORT_KWARG, config.reasoningEffort()); - } - return chatTemplateKwargs; + this.support = new LlamaCppJniProviderSupport(this.config, this.promptSupport); } /** Loads the GGUF model on first use and caches it for subsequent calls. */ private LlamaModel model() { LlamaModel current = model; if (current == null) { - final Map chatTemplateKwargs = buildChatTemplateKwargs(); + final Map chatTemplateKwargs = support.buildChatTemplateKwargs(); final ModelParameters modelParameters = new ModelParameters() .setModel(config.modelPath()) .setCtxSize(config.contextSize()) @@ -170,7 +116,7 @@ private LlamaModel model() { // load time, which matters here because a run loads one model per model group and // calibrate preflights every model in turn. Empty leaves the binding/native-build default. if (!compatibilityHelper.isBlank(config.lazyMode())) { - modelParameters.setLazyMode(lazyMode(config.lazyMode())); + modelParameters.setLazyMode(LlamaCppJniProviderSupport.lazyMode(config.lazyMode())); } // Flash Attention. Emitted only when asked for: llama.cpp's own default is `auto`, so // NOT emitting the option leaves it to decide per backend and model, which is what an @@ -183,10 +129,10 @@ private LlamaModel model() { // KV-cache quantization. Set independently of each other, but note that a quantized V cache // generally needs Flash Attention above -- llama.cpp refuses the combination otherwise. if (!compatibilityHelper.isBlank(config.cacheTypeK())) { - modelParameters.setCacheTypeK(cacheType("cacheTypeK", config.cacheTypeK())); + modelParameters.setCacheTypeK(LlamaCppJniProviderSupport.cacheType("cacheTypeK", config.cacheTypeK())); } if (!compatibilityHelper.isBlank(config.cacheTypeV())) { - modelParameters.setCacheTypeV(cacheType("cacheTypeV", config.cacheTypeV())); + modelParameters.setCacheTypeV(LlamaCppJniProviderSupport.cacheType("cacheTypeV", config.cacheTypeV())); } // Prefill sizing. The binding's own default for both is 0, which llama.cpp reads as // "decide for me", so 0 is not a meaningful user value and the guard is > 0. @@ -216,91 +162,11 @@ private LlamaModel model() { return current; } - /** - * Resolves a configured {@code --tensor-read-lazy} value to the binding's enum. - * - *

Matched case-insensitively against the CLI strings the enum itself declares - * ({@code off}, {@code auto}, {@code on}), so this never drifts from the binding. An - * unrecognised value is rejected rather than silently ignored: it is always a configuration - * typo, and dropping it would hand the user a run that quietly did not do what was asked.

- * - *

Package-private rather than private so it can be unit-tested without loading a model: it is - * otherwise reachable only from the lazy {@code model()} path, which needs a real GGUF.

- * - * @param value the configured value; must not be blank (callers guard on that) - * @return the matching mode - * @throws IllegalArgumentException if no mode declares that CLI string - */ - static LazyMode lazyMode(final String value) { - for (final LazyMode mode : LazyMode.values()) { - if (mode.getArgValue().equalsIgnoreCase(value)) { - return mode; - } - } - throw new IllegalArgumentException( - "Invalid lazyMode value: \"" + value + "\" (expected one of: " + knownLazyModeValues() + ")"); - } - - /** - * Resolves a configured {@code --cache-type-k} / {@code --cache-type-v} value to the binding's enum. - * - *

Same contract as {@link #lazyMode(String)}: matched case-insensitively against the CLI - * strings the enum itself declares, so a cache type upstream adds is accepted for free, and an - * unrecognised value is rejected rather than dropped. The knob name is passed in so the message names - * the element the user actually wrote.

- * - * @param knobName the configuration element name, for the error message - * @param value the configured value; must not be blank (callers guard on that) - * @return the matching cache type - * @throws IllegalArgumentException if no cache type declares that CLI string - */ - static CacheType cacheType(final String knobName, final String value) { - for (final CacheType type : CacheType.values()) { - if (type.getArgValue().equalsIgnoreCase(value)) { - return type; - } - } - throw new IllegalArgumentException( - "Invalid " + knobName + " value: \"" + value + "\" (expected one of: " + knownCacheTypeValues() + ")"); - } - - /** - * Renders the accepted KV-cache types for an error message. - * - * @return the CLI strings the binding's enum declares, comma-separated in declaration order - */ - private static String knownCacheTypeValues() { - final StringBuilder known = new StringBuilder(); - for (final CacheType type : CacheType.values()) { - if (known.length() > 0) { - known.append(", "); - } - known.append(type.getArgValue()); - } - return known.toString(); - } - - /** - * Renders the accepted {@code --tensor-read-lazy} values for an error message. - * - * @return the CLI strings the binding's enum declares, comma-separated in declaration order - */ - private static String knownLazyModeValues() { - final StringBuilder known = new StringBuilder(); - for (final LazyMode mode : LazyMode.values()) { - if (known.length() > 0) { - known.append(", "); - } - known.append(mode.getArgValue()); - } - return known.toString(); - } - @Override public String generate(final AiGenerationRequest request) throws IOException { final ChatResponse response = chatResponse(request); - warnOnTruncatedAnswer(request, response); - logPromptCacheReuse(request, response); + support.warnOnTruncatedAnswer(request, response); + support.logPromptCacheReuse(request, response); return completionParser.parseCompletion(response.getFirstContent()); } @@ -316,73 +182,7 @@ public String generate(final AiGenerationRequest request) throws IOException { * @return the parsed response */ private ChatResponse chatResponse(final AiGenerationRequest request) { - return chatResponseParser.parseResponse(model().chatComplete(buildInferenceParameters(request))); - } - - /** - * Warns when the model stopped because it ran out of output budget rather than because it was done. - * - *

{@code maxOutputTokens} defaults to 128. A summary that hits that ceiling is cut off mid-sentence - * and written to the {@code .ai.md} exactly like a complete one — the run stays green and the - * index silently degrades. The signal was already in the response this provider parses; it was simply - * discarded along with the rest of it.

- * - *

Compared against the literal {@code "length"}, deliberately not against - * {@code net.ladenthin.llama.value.StopReason}. Those are two different vocabularies: - * {@code getFinishReason()} is OpenAI's ({@code stop} / {@code length} / {@code tool_calls}), while - * {@code StopReason} maps llama.cpp's own {@code stop_type} ({@code eos} / {@code word} / - * {@code limit}). {@code StopReason.fromStopType("length")} returns {@code NONE} — a silent - * wrong answer, not a compile error.

- * - *

Package-private so the {@code "length"}-versus-{@code "stop"} decision can be driven from a - * hand-built {@link ChatResponse}, with no model: the trap this method documents is a silent one, - * so it needs a test that runs on every platform rather than only where a GGUF is present.

- * - * @param request the request, for the file name in the message - * @param response the parsed response - */ - void warnOnTruncatedAnswer(final AiGenerationRequest request, final ChatResponse response) { - if (response.getChoices().isEmpty()) { - return; - } - if (!FINISH_REASON_LENGTH.equals(response.getChoices().get(0).getFinishReason())) { - return; - } - LOGGER.warn( - "Generated text for {} was cut off at the {}-token output budget, not finished by the model." - + " The .ai.md will end mid-thought; raise maxOutputTokens for this model, or shorten" - + " the prompt.", - request.sourceFile(), - config.maxOutputTokens()); - } - - /** - * Logs, per generation, how much of the prompt the KV cache served. - * - *

{@code srcmorph:calibrate} reports this for its own probe runs, but an indexing run — the - * one that actually pays {@code swaFull}'s KV-memory surcharge, file after file — had no - * visibility at all. At {@code DEBUG} because it is one line per file.

- * - *

Package-private for the same reason as {@link #warnOnTruncatedAnswer}: a hand-built - * {@link ChatResponse} is enough to drive it.

- * - * @param request the request, for the file name - * @param response the parsed response - */ - void logPromptCacheReuse(final AiGenerationRequest request, final ChatResponse response) { - if (!LOGGER.isDebugEnabled()) { - return; - } - // No null guard: ChatResponse.getUsage() is declared non-null, and SpotBugs flags a check on it - // as dead code (RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE). Unlike getTimings(), which the - // timings path does have to guard. - final Usage usage = response.getUsage(); - LOGGER.debug( - "{}: {} of {} prompt token(s) served from the KV cache, {} generated", - request.sourceFile(), - usage.getCachedTokens(), - usage.getPromptTokens(), - usage.getCompletionTokens()); + return chatResponseParser.parseResponse(model().chatComplete(support.buildInferenceParameters(request))); } // Narrows away the interface's checked IOException: the chat path (chatComplete + parse) throws only @@ -412,88 +212,6 @@ public AiGenerationTimings generateWithTimings(final AiGenerationRequest request timings.getPredictedPerSecond()); } - /** - * Builds the immutable {@link InferenceParameters} for the given request from the resolved config. - * - *

Package-private rather than private so the sentinel guards below can be tested without a - * model: the method touches no llama state (the model is loaded lazily, on the first generate), - * so a test can build a provider over a nonexistent path and inspect what would be sent. Given - * that an unguarded negative window silently killed the provider for two releases, that guard - * needs a test which does not depend on a GGUF being present.

- * - * @param request the generation request - * @return the inference parameters - */ - InferenceParameters buildInferenceParameters(final AiGenerationRequest request) { - // Static instructions go in the SYSTEM message (byte-identical across files, so its KV - // prefix is reused by cache_prompt); the variable file name + source go in the USER message. - final String systemPrompt = promptSupport.systemPrompt(request.promptKey()); - final String userMessage = promptSupport.userMessage(request.sourceFile(), request.sourceText()); - - final List> messages = new ArrayList<>(); - messages.add(new Pair<>("user", userMessage)); - - // InferenceParameters uses immutable withers: each with* returns a new instance, so the - // whole request is built as a single chain. - final InferenceParameters baseParameters = new InferenceParameters("") - .withMessages(systemPrompt, messages) - .withTemperature(config.temperature()) - .withNPredict(config.maxOutputTokens()) - .withTopP(config.topP()) - .withTopK(config.topK()) - .withMinP(config.minP()) - .withTopNSigma(config.topNSigma()) - .withRepeatPenalty(config.repeatPenalty()) - // Cap harmony analysis (reasoning) tokens so a runaway chain-of-thought cannot - // starve the final answer; -1 (default) = unrestricted, so behaviour is unchanged. - .withReasoningBudgetTokens(config.reasoningBudgetTokens()) - // DRY (Don't Repeat Yourself) repetition suppression; multiplier 0.0 (default) = off, - // so the base/allowed-length knobs have no effect unless opted in. The DRY penalty - // WINDOW is deliberately not in this chain -- see penaltyScopedParameters below. - .withDryMultiplier(config.dryMultiplier()) - .withDryBase(config.dryBase()) - .withDryAllowedLength(config.dryAllowedLength()) - .withStopStrings(config.stopStrings().toArray(new String[0])) - // Keep the shared prompt-template prefix warm in the KV cache and reuse it across - // files (pinned to one slot); only the differing source is re-prefilled. - // Reuse is exact -> output unchanged. - .withCachePrompt(config.cachePrompt()) - .withSlotId(REUSE_SLOT_ID); - - // Pin the RNG seed only when explicitly configured (>= 0). Upstream's default is a random seed - // per request, so an unconfigured run keeps exactly the behaviour it had; a configured one makes - // the generated body stable for a given machine and configuration, which is what turns a - // re-index into a reviewable diff. Not bit-reproducibility -- see AiGenerationConfig.DEFAULT_SEED. - final InferenceParameters seededParameters = - config.seed() >= 0 ? baseParameters.withSeed(config.seed()) : baseParameters; - - // The two penalty windows -- the one the repeat penalty acts on, and DRY's own -- are forwarded - // only when configured (>= 0), so an unconfigured run keeps llama.cpp's own window; 0 is - // meaningful (disables the penalty), which is why the guard is >= 0 rather than > 0. - // - // The guard is not an optimisation, it is the only thing keeping the provider alive. The binding - // REJECTS a negative window outright (IllegalArgumentException, because llama.cpp b10273 dropped - // "-1 = context size"), and both defaults are -1. An unguarded forward therefore kills EVERY - // generation before a token is produced, whatever the rest of the configuration says. That is - // not hypothetical: dryPenaltyLastN was forwarded unguarded and did exactly that in 1.1.0 and - // 1.1.1. It survived because "DRY is off by default, so the window cannot matter" is true of the - // window's *effect* and false of the setter's *validation* -- the wither rejects the value - // whether or not DRY is active. Keep both guards, and add one for any future knob whose sentinel - // is negative. - final InferenceParameters repeatScopedParameters = - config.repeatLastN() >= 0 ? seededParameters.withRepeatLastN(config.repeatLastN()) : seededParameters; - final InferenceParameters penaltyScopedParameters = config.dryPenaltyLastN() >= 0 - ? repeatScopedParameters.withDryPenaltyLastN(config.dryPenaltyLastN()) - : repeatScopedParameters; - - // Only override the DRY sequence breakers when explicitly configured; an empty list keeps - // the binding/model default set instead of clearing it. - return config.drySequenceBreakers().isEmpty() - ? penaltyScopedParameters - : penaltyScopedParameters.withDrySequenceBreakers( - config.drySequenceBreakers().toArray(new String[0])); - } - @Override public void close() { final LlamaModel current = model; diff --git a/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniProviderSupport.java b/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniProviderSupport.java new file mode 100644 index 0000000..83e81ef --- /dev/null +++ b/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniProviderSupport.java @@ -0,0 +1,308 @@ +// SPDX-FileCopyrightText: 2026 Bernard Ladenthin +// +// SPDX-License-Identifier: Apache-2.0 +package net.ladenthin.srcmorph.provider; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import lombok.ToString; +import net.ladenthin.llama.args.CacheType; +import net.ladenthin.llama.args.LazyMode; +import net.ladenthin.llama.parameters.InferenceParameters; +import net.ladenthin.llama.value.ChatResponse; +import net.ladenthin.llama.value.Pair; +import net.ladenthin.llama.value.Usage; +import net.ladenthin.srcmorph.document.AiGenerationRequest; +import net.ladenthin.srcmorph.prompt.AiPromptSupport; +import net.ladenthin.srcmorph.support.Java8CompatibilityHelper; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Everything {@link LlamaCppJniAiGenerationProvider} does that does not touch the JNI binding's + * native side: turning a {@link AiGenerationRequest} plus a {@link LlamaCppJniConfig} into + * {@link InferenceParameters}, resolving configured CLI strings to the binding's enums, and + * reporting on a {@link ChatResponse} that has already come back. + * + *

Why this is a class of its own. It is not a style preference; it is what makes the + * mutation gate reach these lines at all. PIT re-runs every test covering a mutated line, and while + * this logic sat next to {@code model()} the only way to put any of it on {@code targetClasses} was + * to put {@code model()} there too -- a ~100-line {@code ModelParameters} chain whose sole exerciser + * is {@code LlamaCppJniKnobSweepTest}, 36 cases that each load a GGUF. Excluding those tests instead + * does not work either: {@code model()}'s mutants then have no coverage at all, and a + * {@code NO_COVERAGE} mutant fails a threshold-100 gate exactly like a survivor. Separating the pure + * half is the only arrangement where the gated class is covered end to end by tests that need no + * model, which is why the split runs along "touches the native handle" and not along any tidier + * conceptual line.

+ * + *

That gap was not hypothetical. The {@code dry_penalty_last_n} regression in 1.1.0/1.1.1 killed + * every generation before a token was produced, and the gate read a stable 775/775 straight through + * it and through its fix -- because the class generated no mutants, neither the defect nor the tests + * that now cover it could move the number. Stability was never evidence of anything here.

+ * + *

Members are package-private: the provider is the only production caller, and the tests live in + * this package. {@code toString} is generated by Lombok for build-log diagnostics.

+ */ +@ToString +final class LlamaCppJniProviderSupport { + + private static final Logger LOGGER = LoggerFactory.getLogger(LlamaCppJniProviderSupport.class); + + /** OpenAI finish reason meaning "the token budget ran out", as opposed to {@code "stop"}. */ + private static final String FINISH_REASON_LENGTH = "length"; + + /** + * Fixed llama.cpp server slot every request is pinned to. The model is loaded once and reused + * for all files; pinning each request to the same slot lets the prompt cache + * ({@code cache_prompt}) reuse the shared prompt-template prefix's KV across files instead of + * re-prefilling it for every file. Reuse is exact, so generated output is unchanged. + */ + private static final int REUSE_SLOT_ID = 0; + + /** Chat-template kwarg controlling Qwen-style thinking (ignored by non-Qwen templates). */ + private static final String ENABLE_THINKING_KWARG = "enable_thinking"; + + /** Chat-template kwarg for the gpt-oss reasoning-effort level (ignored by non-gpt-oss templates). */ + private static final String REASONING_EFFORT_KWARG = "reasoning_effort"; + + /** Number of chat-template kwargs put into the map (used for presizing). */ + private static final int CHAT_TEMPLATE_KWARG_COUNT = 2; + + private final LlamaCppJniConfig config; + private final AiPromptSupport promptSupport; + private final Java8CompatibilityHelper compatibilityHelper = new Java8CompatibilityHelper(); + + /** + * Creates the support object for one provider instance. + * + * @param config llama.cpp configuration + * @param promptSupport prompt lookup used to render request prompts + */ + LlamaCppJniProviderSupport(final LlamaCppJniConfig config, final AiPromptSupport promptSupport) { + this.config = Objects.requireNonNull(config, "config"); + this.promptSupport = Objects.requireNonNull(promptSupport, "promptSupport"); + } + + /** + * Builds the chat-template kwargs passed to + * {@link net.ladenthin.llama.parameters.ModelParameters#setChatTemplateKwargs}. + * + *

Both kwargs are opt-in: an entry is written only when the user actually configured it, so a + * chat template that has never heard of the kwarg is not handed it. That matters because + * llama.cpp's Jinja layer has been moving unknown kwargs from "silently ignored" toward "warned + * about" -- while {@code enable_thinking} was a plain {@code boolean} defaulting to {@code true}, + * every run sent it, and the only way to stop the noise was to set the knob to {@code false}, + * which means something else entirely.

+ * + * @return the kwargs to send; empty when neither knob is configured + */ + Map buildChatTemplateKwargs() { + final Map chatTemplateKwargs = + new HashMap<>(compatibilityHelper.hashMapCapacityFor(CHAT_TEMPLATE_KWARG_COUNT)); + // Qwen-style thinking. Unset (null) omits the kwarg so the model's own template default applies. + final Boolean enableThinking = config.chatTemplateEnableThinking(); + if (enableThinking != null) { + chatTemplateKwargs.put(ENABLE_THINKING_KWARG, String.valueOf(enableThinking.booleanValue())); + } + // gpt-oss honors reasoning_effort; non-gpt-oss chat templates ignore it. An empty + // configured value omits the kwarg so the model's own template default applies. + if (!compatibilityHelper.isBlank(config.reasoningEffort())) { + chatTemplateKwargs.put(REASONING_EFFORT_KWARG, config.reasoningEffort()); + } + return chatTemplateKwargs; + } + + /** + * Resolves a configured {@code --lazy-mode} value to the binding's enum. + * + *

Matched case-insensitively against the CLI strings the enum itself declares + * ({@code off}, {@code auto}, {@code on}), so this never drifts from the binding. An + * unrecognised value is rejected rather than silently ignored: it is always a configuration + * typo, and dropping it would hand the user a run that quietly did not do what was asked.

+ * + * @param value the configured value; must not be blank (callers guard on that) + * @return the matching mode + * @throws IllegalArgumentException if no mode declares that CLI string + */ + static LazyMode lazyMode(final String value) { + for (final LazyMode mode : LazyMode.values()) { + if (mode.getArgValue().equalsIgnoreCase(value)) { + return mode; + } + } + throw new IllegalArgumentException( + "Invalid lazyMode value: \"" + value + "\" (expected one of: " + knownLazyModeValues() + ")"); + } + + /** + * Resolves a configured {@code --cache-type-k} / {@code --cache-type-v} value to the binding's enum. + * + *

Same contract as {@link #lazyMode(String)}: matched case-insensitively against the CLI + * strings the enum itself declares, so a cache type upstream adds is accepted for free, and an + * unrecognised value is rejected rather than dropped. The knob name is passed in so the message names + * the element the user actually wrote.

+ * + * @param knobName the configuration element name, for the error message + * @param value the configured value; must not be blank (callers guard on that) + * @return the matching cache type + * @throws IllegalArgumentException if no cache type declares that CLI string + */ + static CacheType cacheType(final String knobName, final String value) { + for (final CacheType type : CacheType.values()) { + if (type.getArgValue().equalsIgnoreCase(value)) { + return type; + } + } + throw new IllegalArgumentException( + "Invalid " + knobName + " value: \"" + value + "\" (expected one of: " + knownCacheTypeValues() + ")"); + } + + /** + * Renders the accepted KV-cache types for an error message. + * + * @return the CLI strings the binding's enum declares, comma-separated in declaration order + */ + private static String knownCacheTypeValues() { + final StringBuilder known = new StringBuilder(); + for (final CacheType type : CacheType.values()) { + if (known.length() > 0) { + known.append(", "); + } + known.append(type.getArgValue()); + } + return known.toString(); + } + + /** + * Renders the accepted {@code --lazy-mode} values for an error message. + * + * @return the CLI strings the binding's enum declares, comma-separated in declaration order + */ + private static String knownLazyModeValues() { + final StringBuilder known = new StringBuilder(); + for (final LazyMode mode : LazyMode.values()) { + if (known.length() > 0) { + known.append(", "); + } + known.append(mode.getArgValue()); + } + return known.toString(); + } + + /** + * Warns when the model stopped because the output budget ran out rather than because it was done. + * + * @param request the request whose source file is named in the warning + * @param response the completed chat response + */ + void warnOnTruncatedAnswer(final AiGenerationRequest request, final ChatResponse response) { + if (response.getChoices().isEmpty()) { + return; + } + if (!FINISH_REASON_LENGTH.equals(response.getChoices().get(0).getFinishReason())) { + return; + } + LOGGER.warn( + "Generated text for {} was cut off at the {}-token output budget, not finished by the model." + + " The .ai.md will end mid-thought; raise maxOutputTokens for this model, or shorten" + + " the prompt.", + request.sourceFile(), + config.maxOutputTokens()); + } + + /** + * Reports how much of the prompt was served from the KV cache, at debug level. + * + * @param request the request whose source file is named in the line + * @param response the completed chat response + */ + void logPromptCacheReuse(final AiGenerationRequest request, final ChatResponse response) { + if (!LOGGER.isDebugEnabled()) { + return; + } + // No null guard: ChatResponse.getUsage() is declared non-null, and SpotBugs flags a check on it + // as dead code (RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE). Unlike getTimings(), which the + // timings path does have to guard. + final Usage usage = response.getUsage(); + LOGGER.debug( + "{}: {} of {} prompt token(s) served from the KV cache, {} generated", + request.sourceFile(), + usage.getCachedTokens(), + usage.getPromptTokens(), + usage.getCompletionTokens()); + } + + /** + * Builds the per-request inference parameters from the configuration and the request. + * + * @param request the request to render + * @return the parameters to hand to the binding + */ + InferenceParameters buildInferenceParameters(final AiGenerationRequest request) { + // Static instructions go in the SYSTEM message (byte-identical across files, so its KV + // prefix is reused by cache_prompt); the variable file name + source go in the USER message. + final String systemPrompt = promptSupport.systemPrompt(request.promptKey()); + final String userMessage = promptSupport.userMessage(request.sourceFile(), request.sourceText()); + final List> messages = new ArrayList<>(); + messages.add(new Pair<>("user", userMessage)); + // InferenceParameters uses immutable withers: each with* returns a new instance, so the + // whole request is built as a single chain. + final InferenceParameters baseParameters = new InferenceParameters("") + .withMessages(systemPrompt, messages) + .withTemperature(config.temperature()) + .withNPredict(config.maxOutputTokens()) + .withTopP(config.topP()) + .withTopK(config.topK()) + .withMinP(config.minP()) + .withTopNSigma(config.topNSigma()) + .withRepeatPenalty(config.repeatPenalty()) + // Cap harmony analysis (reasoning) tokens so a runaway chain-of-thought cannot + // starve the final answer; -1 (default) = unrestricted, so behaviour is unchanged. + .withReasoningBudgetTokens(config.reasoningBudgetTokens()) + // DRY (Don't Repeat Yourself) repetition suppression; multiplier 0.0 (default) = off, + // so the base/allowed-length knobs have no effect unless opted in. The DRY penalty + // WINDOW is deliberately not in this chain -- see penaltyScopedParameters below. + .withDryMultiplier(config.dryMultiplier()) + .withDryBase(config.dryBase()) + .withDryAllowedLength(config.dryAllowedLength()) + .withStopStrings(config.stopStrings().toArray(new String[0])) + // Keep the shared prompt-template prefix warm in the KV cache and reuse it across + // files (pinned to one slot); only the differing source is re-prefilled. + // Reuse is exact -> output unchanged. + .withCachePrompt(config.cachePrompt()) + .withSlotId(REUSE_SLOT_ID); + // Pin the RNG seed only when explicitly configured (>= 0). Upstream's default is a random seed + // per request, so an unconfigured run keeps exactly the behaviour it had; a configured one makes + // the generated body stable for a given machine and configuration, which is what turns a + // re-index into a reviewable diff. Not bit-reproducibility -- see AiGenerationConfig.DEFAULT_SEED. + final InferenceParameters seededParameters = + config.seed() >= 0 ? baseParameters.withSeed(config.seed()) : baseParameters; + // The two penalty windows -- the one the repeat penalty acts on, and DRY's own -- are forwarded + // only when configured (>= 0), so an unconfigured run keeps llama.cpp's own window; 0 is + // meaningful (disables the penalty), which is why the guard is >= 0 rather than > 0. + // + // The guard is not an optimisation, it is the only thing keeping the provider alive. The binding + // REJECTS a negative window outright (IllegalArgumentException, because llama.cpp b10273 dropped + // "-1 = context size"), and both defaults are -1. An unguarded forward therefore kills EVERY + // generation before a token is produced, whatever the rest of the configuration says. That is + // not hypothetical: dryPenaltyLastN was forwarded unguarded and did exactly that in 1.1.0 and + // 1.1.1. It survived because "DRY is off by default, so the window cannot matter" is true of the + // window's *effect* and false of the setter's *validation* -- the wither rejects the value + // whether or not DRY is active. Keep both guards, and add one for any future knob whose sentinel + // is negative. + final InferenceParameters repeatScopedParameters = + config.repeatLastN() >= 0 ? seededParameters.withRepeatLastN(config.repeatLastN()) : seededParameters; + final InferenceParameters penaltyScopedParameters = config.dryPenaltyLastN() >= 0 + ? repeatScopedParameters.withDryPenaltyLastN(config.dryPenaltyLastN()) + : repeatScopedParameters; + // Only override the DRY sequence breakers when explicitly configured; an empty list keeps + // the binding/model default set instead of clearing it. + return config.drySequenceBreakers().isEmpty() + ? penaltyScopedParameters + : penaltyScopedParameters.withDrySequenceBreakers( + config.drySequenceBreakers().toArray(new String[0])); + } +} diff --git a/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProviderTest.java b/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProviderTest.java index d1c9c5e..dd7635e 100644 --- a/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProviderTest.java +++ b/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProviderTest.java @@ -3,34 +3,18 @@ // SPDX-License-Identifier: Apache-2.0 package net.ladenthin.srcmorph.provider; -import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.MatcherAssert.assertThat; -import ch.qos.logback.classic.Level; -import ch.qos.logback.classic.Logger; -import ch.qos.logback.classic.spi.ILoggingEvent; -import ch.qos.logback.core.read.ListAppender; import java.nio.file.Paths; -import java.util.Collections; -import java.util.Map; -import net.ladenthin.llama.args.CacheType; -import net.ladenthin.llama.args.LazyMode; -import net.ladenthin.llama.value.ChatChoice; -import net.ladenthin.llama.value.ChatMessage; -import net.ladenthin.llama.value.ChatResponse; -import net.ladenthin.llama.value.Usage; import net.ladenthin.srcmorph.CommonTestFixtures; import net.ladenthin.srcmorph.NativeLlamaAvailability; import net.ladenthin.srcmorph.document.AiGenerationRequest; import net.ladenthin.srcmorph.document.AiMdHeader; import net.ladenthin.srcmorph.document.AiMdHeaderCodec; import net.ladenthin.srcmorph.prompt.AiPromptSupport; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import org.slf4j.LoggerFactory; public class LlamaCppJniAiGenerationProviderTest { @@ -132,375 +116,4 @@ public void generateWithTimings_realProvider_reportsEngineTimingsThatScaleWithPr } } // - - @Test - public void lazyMode_mapsEveryDeclaredCliString() { - // Every mode the binding declares must resolve; a new upstream mode is then covered for free. - for (final LazyMode mode : LazyMode.values()) { - assertThat(LlamaCppJniAiGenerationProvider.lazyMode(mode.getArgValue()), is(mode)); - } - } - - @Test - public void lazyMode_isCaseInsensitive() { - assertThat(LlamaCppJniAiGenerationProvider.lazyMode("ON"), is(LazyMode.ON)); - assertThat(LlamaCppJniAiGenerationProvider.lazyMode("Auto"), is(LazyMode.AUTO)); - } - - @Test - public void lazyMode_rejectsUnknownValueAndNamesTheAcceptedOnes() { - // A typo must fail loud rather than be dropped, and the message must name what is accepted. - final IllegalArgumentException thrown = Assertions.assertThrows( - IllegalArgumentException.class, () -> LlamaCppJniAiGenerationProvider.lazyMode("lazy")); - assertThat(thrown.getMessage(), containsString("lazy")); - assertThat(thrown.getMessage(), containsString("off, auto, on")); - } - - @Test - public void cacheType_mapsEveryDeclaredCliString() { - // Every cache type the binding declares must resolve; a type upstream adds is covered for free. - for (final CacheType type : CacheType.values()) { - assertThat(LlamaCppJniAiGenerationProvider.cacheType("cacheTypeK", type.getArgValue()), is(type)); - } - } - - @Test - public void cacheType_isCaseInsensitive() { - assertThat(LlamaCppJniAiGenerationProvider.cacheType("cacheTypeK", "Q8_0"), is(CacheType.Q8_0)); - assertThat(LlamaCppJniAiGenerationProvider.cacheType("cacheTypeV", "F16"), is(CacheType.F16)); - } - - @Test - public void cacheType_rejectsUnknownValueAndNamesBothTheKnobAndTheAcceptedOnes() { - // A typo must fail loud rather than be dropped. The knob name is in the message because the same - // resolver serves cacheTypeK and cacheTypeV -- without it the user cannot tell which one is wrong. - final IllegalArgumentException thrown = Assertions.assertThrows( - IllegalArgumentException.class, () -> LlamaCppJniAiGenerationProvider.cacheType("cacheTypeV", "q3_k")); - assertThat(thrown.getMessage(), containsString("cacheTypeV")); - assertThat(thrown.getMessage(), containsString("q3_k")); - assertThat(thrown.getMessage(), containsString("f32, f16, bf16, q8_0")); - } - - // - - /** - * Builds a provider over a path that does not exist. The model is loaded lazily on the first - * generate, and {@code buildInferenceParameters} never touches it, so these tests need no GGUF and - * no native library -- which is the point: the regression they guard shipped twice while every - * model-free gate stayed green. - */ - private static LlamaCppJniAiGenerationProvider providerWith(final LlamaCppJniConfig config) { - return new LlamaCppJniAiGenerationProvider( - config, new AiPromptSupport(CommonTestFixtures.createFilePromptDefinitions())); - } - - /** - * The regression itself. Both penalty windows default to {@code -1}, and the binding rejects any - * negative window outright ({@code IllegalArgumentException}) because llama.cpp b10273 dropped - * "{@code -1} = context size". Forwarding one unguarded therefore throws before a single token is - * produced -- which is exactly what {@code dry_penalty_last_n} did in 1.1.0 and 1.1.1. - */ - @Test - public void buildInferenceParameters_defaultConfig_doesNotThrowOnTheNegativeSentinels() { - // arrange - final LlamaCppJniConfig defaults = - LlamaCppJniConfig.builder("/does/not/exist.gguf").build(); - - // act / assert - Assertions.assertDoesNotThrow(() -> providerWith(defaults).buildInferenceParameters(request("class A {}"))); - } - - /** A sentinel means "say nothing", so llama.cpp keeps its own window -- it must not be sent as -1. */ - @Test - public void buildInferenceParameters_defaultConfig_sendsNeitherPenaltyWindow() { - // arrange - final LlamaCppJniConfig defaults = - LlamaCppJniConfig.builder("/does/not/exist.gguf").build(); - - // act - final String json = providerWith(defaults) - .buildInferenceParameters(request("class A {}")) - .toString(); - - // assert - assertThat(json, not(containsString(PARAM_DRY_PENALTY_LAST_N))); - assertThat(json, not(containsString(PARAM_REPEAT_LAST_N))); - } - - /** - * {@code 0} is a real value, not a second sentinel: it disables the penalty. A guard written as - * {@code > 0} would swallow it, so this pins the boundary from the other side. - */ - @Test - public void buildInferenceParameters_zeroWindows_sendsBothAsZero() { - // arrange - final LlamaCppJniConfig zeroed = LlamaCppJniConfig.builder("/does/not/exist.gguf") - .dryPenaltyLastN(0) - .repeatLastN(0) - .build(); - - // act - final String json = providerWith(zeroed) - .buildInferenceParameters(request("class A {}")) - .toString(); - - // assert - assertThat(json, containsString(PARAM_DRY_PENALTY_LAST_N)); - assertThat(json, containsString(PARAM_REPEAT_LAST_N)); - } - - /** A configured positive window reaches the request unchanged. */ - @Test - public void buildInferenceParameters_configuredWindows_arePassedThrough() { - // arrange - final LlamaCppJniConfig configured = LlamaCppJniConfig.builder("/does/not/exist.gguf") - .dryPenaltyLastN(64) - .repeatLastN(128) - .build(); - - // act - final String json = providerWith(configured) - .buildInferenceParameters(request("class A {}")) - .toString(); - - // assert -- distinct values, so a transposition of the two guards fails rather than cancelling out - assertThat(json, containsString(PARAM_DRY_PENALTY_LAST_N + ": 64")); - assertThat(json, containsString(PARAM_REPEAT_LAST_N + ": 128")); - } - - // - - // - - /** Builds a response carrying just the one field each of these two helpers reads. */ - private static ChatResponse responseWith(final String finishReason, final Usage usage) { - final ChatChoice choice = new ChatChoice(0, new ChatMessage("assistant", "body"), finishReason); - return new ChatResponse("id", Collections.singletonList(choice), usage, null, "{}"); - } - - private static ListAppender attachAppender(final Level level) { - final ListAppender appender = new ListAppender<>(); - appender.start(); - final Logger logger = (Logger) LoggerFactory.getLogger(LlamaCppJniAiGenerationProvider.class); - logger.setLevel(level); - logger.addAppender(appender); - return appender; - } - - private static void detachAppender(final ListAppender appender) { - ((Logger) LoggerFactory.getLogger(LlamaCppJniAiGenerationProvider.class)).detachAppender(appender); - appender.stop(); - } - - /** - * The whole point of the feature: {@code length} means the model hit the output budget and stopped - * mid-sentence, so the {@code .ai.md} it produced is incomplete and the user has to be told. - */ - @Test - public void warnOnTruncatedAnswer_finishReasonLength_warnsAndNamesTheFileAndTheBudget() { - // arrange - final ListAppender appender = attachAppender(Level.WARN); - try { - final LlamaCppJniAiGenerationProvider provider = - providerWith(LlamaCppJniConfig.builder("/does/not/exist.gguf") - .maxOutputTokens(128) - .build()); - - // act - provider.warnOnTruncatedAnswer(request("class A {}"), responseWith("length", new Usage(1, 1))); - - // assert - assertThat(appender.list.size(), is(1)); - final String message = appender.list.get(0).getFormattedMessage(); - assertThat(message, containsString("Test.java")); - assertThat(message, containsString("128")); - } finally { - detachAppender(appender); - } - } - - /** - * A normal completion must stay silent. This is the assertion that pins the {@code "length"} - * literal: {@code StopReason.fromStopType("length")} is {@code NONE}, so comparing against the - * wrong vocabulary would make the warning fire never or always -- silently, either way. - */ - @Test - public void warnOnTruncatedAnswer_finishReasonStop_saysNothing() { - // arrange - final ListAppender appender = attachAppender(Level.WARN); - try { - final LlamaCppJniAiGenerationProvider provider = providerWith( - LlamaCppJniConfig.builder("/does/not/exist.gguf").build()); - - // act - provider.warnOnTruncatedAnswer(request("class A {}"), responseWith("stop", new Usage(1, 1))); - - // assert - assertThat(appender.list.isEmpty(), is(true)); - } finally { - detachAppender(appender); - } - } - - /** No choices means nothing to judge; reading {@code get(0)} anyway would throw. */ - @Test - public void warnOnTruncatedAnswer_noChoices_saysNothing() { - // arrange - final ListAppender appender = attachAppender(Level.WARN); - try { - final LlamaCppJniAiGenerationProvider provider = providerWith( - LlamaCppJniConfig.builder("/does/not/exist.gguf").build()); - final ChatResponse empty = new ChatResponse("id", Collections.emptyList(), new Usage(1, 1), null, "{}"); - - // act - provider.warnOnTruncatedAnswer(request("class A {}"), empty); - - // assert - assertThat(appender.list.isEmpty(), is(true)); - } finally { - detachAppender(appender); - } - } - - /** The cache line reports all three counts, so a transposition of two of them is visible. */ - @Test - public void logPromptCacheReuse_debugEnabled_reportsCachedTotalAndGenerated() { - // arrange - final ListAppender appender = attachAppender(Level.DEBUG); - try { - final LlamaCppJniAiGenerationProvider provider = providerWith( - LlamaCppJniConfig.builder("/does/not/exist.gguf").build()); - - // act -- distinct values so each lands in its own placeholder - provider.logPromptCacheReuse(request("class A {}"), responseWith("stop", new Usage(70, 11, 33))); - - // assert - assertThat(appender.list.size(), is(1)); - final String message = appender.list.get(0).getFormattedMessage(); - assertThat(message, containsString("33 of 70 prompt token(s)")); - assertThat(message, containsString("11 generated")); - } finally { - detachAppender(appender); - } - } - - /** At INFO the line must not be built at all -- it is one log line per indexed file. */ - @Test - public void logPromptCacheReuse_debugDisabled_saysNothing() { - // arrange - final ListAppender appender = attachAppender(Level.INFO); - try { - final LlamaCppJniAiGenerationProvider provider = providerWith( - LlamaCppJniConfig.builder("/does/not/exist.gguf").build()); - - // act - provider.logPromptCacheReuse(request("class A {}"), responseWith("stop", new Usage(70, 11, 33))); - - // assert - assertThat(appender.list.isEmpty(), is(true)); - } finally { - detachAppender(appender); - } - } - - // - - /** - * The point of the tri-state. While {@code chatTemplateEnableThinking} was a plain - * {@code boolean} defaulting to {@code true}, every run handed {@code enable_thinking} to the - * chat template -- including templates that have never heard of it, which llama.cpp's Jinja - * layer has been moving from "silently ignored" toward "warned about". Unset must now mean - * "say nothing", so the template's own default applies. - * - *

The sibling kwarg is deliberately not asserted away here: {@code reasoningEffort} does - * default to {@code "low"} and is therefore still sent by a default run, but unlike - * {@code enable_thinking} it has an escape that means exactly "unset" -- the empty string -- so a - * non-gpt-oss user can switch it off without picking a value that says something else. That is a - * documented choice, not the same defect; the next test pins that escape.

- */ - @Test - public void buildChatTemplateKwargs_defaultConfig_doesNotSendEnableThinking() { - // arrange - final LlamaCppJniConfig defaults = - LlamaCppJniConfig.builder("/does/not/exist.gguf").build(); - - // act - final Map kwargs = providerWith(defaults).buildChatTemplateKwargs(); - - // assert - assertThat(kwargs.containsKey(KWARG_ENABLE_THINKING), is(false)); - } - - /** The other half of "unset means unset": a blank reasoning effort omits its kwarg too. */ - @Test - public void buildChatTemplateKwargs_blankReasoningEffort_sendsNoKwargAtAll() { - // arrange - final LlamaCppJniConfig blank = LlamaCppJniConfig.builder("/does/not/exist.gguf") - .reasoningEffort("") - .build(); - - // act - final Map kwargs = providerWith(blank).buildChatTemplateKwargs(); - - // assert - assertThat(kwargs.isEmpty(), is(true)); - } - - /** - * {@code true} is a real configured value, not a second spelling of "unset". A guard written as - * "send it only when false" would swallow it, so this pins the boundary from the other side -- - * the same shape as the {@code 0}-versus-{@code -1} penalty-window pair above. - */ - @Test - public void buildChatTemplateKwargs_thinkingSetToTrue_isStillSent() { - // arrange - final LlamaCppJniConfig enabled = LlamaCppJniConfig.builder("/does/not/exist.gguf") - .chatTemplateEnableThinking(Boolean.TRUE) - .build(); - - // act - final Map kwargs = providerWith(enabled).buildChatTemplateKwargs(); - - // assert - assertThat(kwargs.get(KWARG_ENABLE_THINKING), is("true")); - } - - /** The Gemma-4 case the knob exists for: suppress the thinking block at the Jinja level. */ - @Test - public void buildChatTemplateKwargs_thinkingSetToFalse_isSentAsFalse() { - // arrange - final LlamaCppJniConfig disabled = LlamaCppJniConfig.builder("/does/not/exist.gguf") - .chatTemplateEnableThinking(Boolean.FALSE) - .build(); - - // act - final Map kwargs = providerWith(disabled).buildChatTemplateKwargs(); - - // assert - assertThat(kwargs.get(KWARG_ENABLE_THINKING), is("false")); - } - - /** - * The second kwarg was already opt-in (blank omits it); pinned here so the extraction of - * {@code buildChatTemplateKwargs} out of {@code model()} cannot drop it unnoticed. - */ - @Test - public void buildChatTemplateKwargs_reasoningEffortConfigured_isSentAlongside() { - // arrange - final LlamaCppJniConfig configured = LlamaCppJniConfig.builder("/does/not/exist.gguf") - .reasoningEffort("high") - .chatTemplateEnableThinking(Boolean.FALSE) - .build(); - - // act - final Map kwargs = providerWith(configured).buildChatTemplateKwargs(); - - // assert - assertThat(kwargs.get(KWARG_REASONING_EFFORT), is("high")); - assertThat(kwargs.get(KWARG_ENABLE_THINKING), is("false")); - } - - // - - // } diff --git a/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniProviderSupportTest.java b/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniProviderSupportTest.java new file mode 100644 index 0000000..d4e017f --- /dev/null +++ b/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniProviderSupportTest.java @@ -0,0 +1,441 @@ +// SPDX-FileCopyrightText: 2026 Bernard Ladenthin +// +// SPDX-License-Identifier: Apache-2.0 +package net.ladenthin.srcmorph.provider; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.MatcherAssert.assertThat; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import java.nio.file.Paths; +import java.util.Collections; +import java.util.Map; +import net.ladenthin.llama.args.CacheType; +import net.ladenthin.llama.args.LazyMode; +import net.ladenthin.llama.value.ChatChoice; +import net.ladenthin.llama.value.ChatMessage; +import net.ladenthin.llama.value.ChatResponse; +import net.ladenthin.llama.value.Usage; +import net.ladenthin.srcmorph.CommonTestFixtures; +import net.ladenthin.srcmorph.document.AiGenerationRequest; +import net.ladenthin.srcmorph.document.AiMdHeader; +import net.ladenthin.srcmorph.document.AiMdHeaderCodec; +import net.ladenthin.srcmorph.prompt.AiPromptSupport; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; + +/** + * Model-free tests for {@link LlamaCppJniProviderSupport}. + * + *

Every case here builds its configuration over a path that does not exist. That is not a + * shortcut, it is the contract this class exists to keep: none of the support methods touches the + * native handle, so all of them can be pinned without a GGUF -- which is what lets the mutation + * gate cover them at threshold 100 without dragging the model-loading suites into every mutant.

+ */ +public class LlamaCppJniProviderSupportTest { + + /** + * JSON key of the DRY penalty window as the binding renders it. Quoted so the assertions cannot + * accidentally match a different key that merely contains this one as a substring. + */ + private static final String PARAM_DRY_PENALTY_LAST_N = "\"dry_penalty_last_n\""; + + /** JSON key of the repeat-penalty window, quoted for the same reason. */ + private static final String PARAM_REPEAT_LAST_N = "\"repeat_last_n\""; + + /** Chat-template kwarg key for Qwen-style thinking, as the provider spells it. */ + private static final String KWARG_ENABLE_THINKING = "enable_thinking"; + + /** Chat-template kwarg key for the gpt-oss reasoning-effort level. */ + private static final String KWARG_REASONING_EFFORT = "reasoning_effort"; + + private static final AiMdHeader HEADER = new AiMdHeader( + "Test.java", + AiMdHeaderCodec.HEADER_VERSION_1_0, + "00000000", + "2026-03-18T00:00:00Z", + "2026-03-18T00:00:00Z", + "0.1.0-SNAPSHOT", + "0.0.0", + AiMdHeaderCodec.NODE_TYPE_FILE); + + private static AiGenerationRequest request(final String source) { + return new AiGenerationRequest(CommonTestFixtures.PROMPT_KEY_FILE_BODY, Paths.get("Test.java"), source, HEADER); + } + + @Test + public void lazyMode_mapsEveryDeclaredCliString() { + // Every mode the binding declares must resolve; a new upstream mode is then covered for free. + for (final LazyMode mode : LazyMode.values()) { + assertThat(LlamaCppJniProviderSupport.lazyMode(mode.getArgValue()), is(mode)); + } + } + + @Test + public void lazyMode_isCaseInsensitive() { + assertThat(LlamaCppJniProviderSupport.lazyMode("ON"), is(LazyMode.ON)); + assertThat(LlamaCppJniProviderSupport.lazyMode("Auto"), is(LazyMode.AUTO)); + } + + @Test + public void lazyMode_rejectsUnknownValueAndNamesTheAcceptedOnes() { + // A typo must fail loud rather than be dropped, and the message must name what is accepted. + final IllegalArgumentException thrown = Assertions.assertThrows( + IllegalArgumentException.class, () -> LlamaCppJniProviderSupport.lazyMode("lazy")); + assertThat(thrown.getMessage(), containsString("lazy")); + assertThat(thrown.getMessage(), containsString("off, auto, on")); + } + + @Test + public void cacheType_mapsEveryDeclaredCliString() { + // Every cache type the binding declares must resolve; a type upstream adds is covered for free. + for (final CacheType type : CacheType.values()) { + assertThat(LlamaCppJniProviderSupport.cacheType("cacheTypeK", type.getArgValue()), is(type)); + } + } + + @Test + public void cacheType_isCaseInsensitive() { + assertThat(LlamaCppJniProviderSupport.cacheType("cacheTypeK", "Q8_0"), is(CacheType.Q8_0)); + assertThat(LlamaCppJniProviderSupport.cacheType("cacheTypeV", "F16"), is(CacheType.F16)); + } + + @Test + public void cacheType_rejectsUnknownValueAndNamesBothTheKnobAndTheAcceptedOnes() { + // A typo must fail loud rather than be dropped. The knob name is in the message because the same + // resolver serves cacheTypeK and cacheTypeV -- without it the user cannot tell which one is wrong. + final IllegalArgumentException thrown = Assertions.assertThrows( + IllegalArgumentException.class, () -> LlamaCppJniProviderSupport.cacheType("cacheTypeV", "q3_k")); + assertThat(thrown.getMessage(), containsString("cacheTypeV")); + assertThat(thrown.getMessage(), containsString("q3_k")); + assertThat(thrown.getMessage(), containsString("f32, f16, bf16, q8_0")); + } + + // + + /** + * Builds a provider over a path that does not exist. The model is loaded lazily on the first + * generate, and {@code buildInferenceParameters} never touches it, so these tests need no GGUF and + * no native library -- which is the point: the regression they guard shipped twice while every + * model-free gate stayed green. + */ + private static LlamaCppJniProviderSupport providerWith(final LlamaCppJniConfig config) { + return new LlamaCppJniProviderSupport( + config, new AiPromptSupport(CommonTestFixtures.createFilePromptDefinitions())); + } + + /** + * The regression itself. Both penalty windows default to {@code -1}, and the binding rejects any + * negative window outright ({@code IllegalArgumentException}) because llama.cpp b10273 dropped + * "{@code -1} = context size". Forwarding one unguarded therefore throws before a single token is + * produced -- which is exactly what {@code dry_penalty_last_n} did in 1.1.0 and 1.1.1. + */ + @Test + public void buildInferenceParameters_defaultConfig_doesNotThrowOnTheNegativeSentinels() { + // arrange + final LlamaCppJniConfig defaults = + LlamaCppJniConfig.builder("/does/not/exist.gguf").build(); + + // act / assert + Assertions.assertDoesNotThrow(() -> providerWith(defaults).buildInferenceParameters(request("class A {}"))); + } + + /** A sentinel means "say nothing", so llama.cpp keeps its own window -- it must not be sent as -1. */ + @Test + public void buildInferenceParameters_defaultConfig_sendsNeitherPenaltyWindow() { + // arrange + final LlamaCppJniConfig defaults = + LlamaCppJniConfig.builder("/does/not/exist.gguf").build(); + + // act + final String json = providerWith(defaults) + .buildInferenceParameters(request("class A {}")) + .toString(); + + // assert + assertThat(json, not(containsString(PARAM_DRY_PENALTY_LAST_N))); + assertThat(json, not(containsString(PARAM_REPEAT_LAST_N))); + } + + /** + * {@code 0} is a real value, not a second sentinel: it disables the penalty. A guard written as + * {@code > 0} would swallow it, so this pins the boundary from the other side. + */ + @Test + public void buildInferenceParameters_zeroWindows_sendsBothAsZero() { + // arrange + final LlamaCppJniConfig zeroed = LlamaCppJniConfig.builder("/does/not/exist.gguf") + .dryPenaltyLastN(0) + .repeatLastN(0) + .build(); + + // act + final String json = providerWith(zeroed) + .buildInferenceParameters(request("class A {}")) + .toString(); + + // assert + assertThat(json, containsString(PARAM_DRY_PENALTY_LAST_N)); + assertThat(json, containsString(PARAM_REPEAT_LAST_N)); + } + + /** A configured positive window reaches the request unchanged. */ + @Test + public void buildInferenceParameters_configuredWindows_arePassedThrough() { + // arrange + final LlamaCppJniConfig configured = LlamaCppJniConfig.builder("/does/not/exist.gguf") + .dryPenaltyLastN(64) + .repeatLastN(128) + .build(); + + // act + final String json = providerWith(configured) + .buildInferenceParameters(request("class A {}")) + .toString(); + + // assert -- distinct values, so a transposition of the two guards fails rather than cancelling out + assertThat(json, containsString(PARAM_DRY_PENALTY_LAST_N + ": 64")); + assertThat(json, containsString(PARAM_REPEAT_LAST_N + ": 128")); + } + + // + + // + + /** Builds a response carrying just the one field each of these two helpers reads. */ + private static ChatResponse responseWith(final String finishReason, final Usage usage) { + final ChatChoice choice = new ChatChoice(0, new ChatMessage("assistant", "body"), finishReason); + return new ChatResponse("id", Collections.singletonList(choice), usage, null, "{}"); + } + + private static ListAppender attachAppender(final Level level) { + final ListAppender appender = new ListAppender<>(); + appender.start(); + final Logger logger = (Logger) LoggerFactory.getLogger(LlamaCppJniProviderSupport.class); + logger.setLevel(level); + logger.addAppender(appender); + return appender; + } + + private static void detachAppender(final ListAppender appender) { + ((Logger) LoggerFactory.getLogger(LlamaCppJniProviderSupport.class)).detachAppender(appender); + appender.stop(); + } + + /** + * The whole point of the feature: {@code length} means the model hit the output budget and stopped + * mid-sentence, so the {@code .ai.md} it produced is incomplete and the user has to be told. + */ + @Test + public void warnOnTruncatedAnswer_finishReasonLength_warnsAndNamesTheFileAndTheBudget() { + // arrange + final ListAppender appender = attachAppender(Level.WARN); + try { + final LlamaCppJniProviderSupport provider = providerWith(LlamaCppJniConfig.builder("/does/not/exist.gguf") + .maxOutputTokens(128) + .build()); + + // act + provider.warnOnTruncatedAnswer(request("class A {}"), responseWith("length", new Usage(1, 1))); + + // assert + assertThat(appender.list.size(), is(1)); + final String message = appender.list.get(0).getFormattedMessage(); + assertThat(message, containsString("Test.java")); + assertThat(message, containsString("128")); + } finally { + detachAppender(appender); + } + } + + /** + * A normal completion must stay silent. This is the assertion that pins the {@code "length"} + * literal: {@code StopReason.fromStopType("length")} is {@code NONE}, so comparing against the + * wrong vocabulary would make the warning fire never or always -- silently, either way. + */ + @Test + public void warnOnTruncatedAnswer_finishReasonStop_saysNothing() { + // arrange + final ListAppender appender = attachAppender(Level.WARN); + try { + final LlamaCppJniProviderSupport provider = providerWith( + LlamaCppJniConfig.builder("/does/not/exist.gguf").build()); + + // act + provider.warnOnTruncatedAnswer(request("class A {}"), responseWith("stop", new Usage(1, 1))); + + // assert + assertThat(appender.list.isEmpty(), is(true)); + } finally { + detachAppender(appender); + } + } + + /** No choices means nothing to judge; reading {@code get(0)} anyway would throw. */ + @Test + public void warnOnTruncatedAnswer_noChoices_saysNothing() { + // arrange + final ListAppender appender = attachAppender(Level.WARN); + try { + final LlamaCppJniProviderSupport provider = providerWith( + LlamaCppJniConfig.builder("/does/not/exist.gguf").build()); + final ChatResponse empty = new ChatResponse("id", Collections.emptyList(), new Usage(1, 1), null, "{}"); + + // act + provider.warnOnTruncatedAnswer(request("class A {}"), empty); + + // assert + assertThat(appender.list.isEmpty(), is(true)); + } finally { + detachAppender(appender); + } + } + + /** The cache line reports all three counts, so a transposition of two of them is visible. */ + @Test + public void logPromptCacheReuse_debugEnabled_reportsCachedTotalAndGenerated() { + // arrange + final ListAppender appender = attachAppender(Level.DEBUG); + try { + final LlamaCppJniProviderSupport provider = providerWith( + LlamaCppJniConfig.builder("/does/not/exist.gguf").build()); + + // act -- distinct values so each lands in its own placeholder + provider.logPromptCacheReuse(request("class A {}"), responseWith("stop", new Usage(70, 11, 33))); + + // assert + assertThat(appender.list.size(), is(1)); + final String message = appender.list.get(0).getFormattedMessage(); + assertThat(message, containsString("33 of 70 prompt token(s)")); + assertThat(message, containsString("11 generated")); + } finally { + detachAppender(appender); + } + } + + /** At INFO the line must not be built at all -- it is one log line per indexed file. */ + @Test + public void logPromptCacheReuse_debugDisabled_saysNothing() { + // arrange + final ListAppender appender = attachAppender(Level.INFO); + try { + final LlamaCppJniProviderSupport provider = providerWith( + LlamaCppJniConfig.builder("/does/not/exist.gguf").build()); + + // act + provider.logPromptCacheReuse(request("class A {}"), responseWith("stop", new Usage(70, 11, 33))); + + // assert + assertThat(appender.list.isEmpty(), is(true)); + } finally { + detachAppender(appender); + } + } + + // + + /** + * The point of the tri-state. While {@code chatTemplateEnableThinking} was a plain + * {@code boolean} defaulting to {@code true}, every run handed {@code enable_thinking} to the + * chat template -- including templates that have never heard of it, which llama.cpp's Jinja + * layer has been moving from "silently ignored" toward "warned about". Unset must now mean + * "say nothing", so the template's own default applies. + * + *

The sibling kwarg is deliberately not asserted away here: {@code reasoningEffort} does + * default to {@code "low"} and is therefore still sent by a default run, but unlike + * {@code enable_thinking} it has an escape that means exactly "unset" -- the empty string -- so a + * non-gpt-oss user can switch it off without picking a value that says something else. That is a + * documented choice, not the same defect; the next test pins that escape.

+ */ + @Test + public void buildChatTemplateKwargs_defaultConfig_doesNotSendEnableThinking() { + // arrange + final LlamaCppJniConfig defaults = + LlamaCppJniConfig.builder("/does/not/exist.gguf").build(); + + // act + final Map kwargs = providerWith(defaults).buildChatTemplateKwargs(); + + // assert + assertThat(kwargs.containsKey(KWARG_ENABLE_THINKING), is(false)); + } + + /** The other half of "unset means unset": a blank reasoning effort omits its kwarg too. */ + @Test + public void buildChatTemplateKwargs_blankReasoningEffort_sendsNoKwargAtAll() { + // arrange + final LlamaCppJniConfig blank = LlamaCppJniConfig.builder("/does/not/exist.gguf") + .reasoningEffort("") + .build(); + + // act + final Map kwargs = providerWith(blank).buildChatTemplateKwargs(); + + // assert + assertThat(kwargs.isEmpty(), is(true)); + } + + /** + * {@code true} is a real configured value, not a second spelling of "unset". A guard written as + * "send it only when false" would swallow it, so this pins the boundary from the other side -- + * the same shape as the {@code 0}-versus-{@code -1} penalty-window pair above. + */ + @Test + public void buildChatTemplateKwargs_thinkingSetToTrue_isStillSent() { + // arrange + final LlamaCppJniConfig enabled = LlamaCppJniConfig.builder("/does/not/exist.gguf") + .chatTemplateEnableThinking(Boolean.TRUE) + .build(); + + // act + final Map kwargs = providerWith(enabled).buildChatTemplateKwargs(); + + // assert + assertThat(kwargs.get(KWARG_ENABLE_THINKING), is("true")); + } + + /** The Gemma-4 case the knob exists for: suppress the thinking block at the Jinja level. */ + @Test + public void buildChatTemplateKwargs_thinkingSetToFalse_isSentAsFalse() { + // arrange + final LlamaCppJniConfig disabled = LlamaCppJniConfig.builder("/does/not/exist.gguf") + .chatTemplateEnableThinking(Boolean.FALSE) + .build(); + + // act + final Map kwargs = providerWith(disabled).buildChatTemplateKwargs(); + + // assert + assertThat(kwargs.get(KWARG_ENABLE_THINKING), is("false")); + } + + /** + * The second kwarg was already opt-in (blank omits it); pinned here so the extraction of + * {@code buildChatTemplateKwargs} out of {@code model()} cannot drop it unnoticed. + */ + @Test + public void buildChatTemplateKwargs_reasoningEffortConfigured_isSentAlongside() { + // arrange + final LlamaCppJniConfig configured = LlamaCppJniConfig.builder("/does/not/exist.gguf") + .reasoningEffort("high") + .chatTemplateEnableThinking(Boolean.FALSE) + .build(); + + // act + final Map kwargs = providerWith(configured).buildChatTemplateKwargs(); + + // assert + assertThat(kwargs.get(KWARG_REASONING_EFFORT), is("high")); + assertThat(kwargs.get(KWARG_ENABLE_THINKING), is("false")); + } + + // + + // +} From 131f9e994e2b6fe26d5db40d213f207e9404f24b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 20:55:24 +0000 Subject: [PATCH 2/3] test(provider): kill the five mutants the new gate scope exposed Putting LlamaCppJniProviderSupport on targetClasses turned 807/807 into 825/830: the class contributes 23 mutations and five of them survived. They are real test gaps, not equivalent mutants, and every one of them was invisible before -- which is the whole argument of TODO #51: while the class generated no mutants, the gate number could not move whether the tests were adequate or not. What survived, and what now kills it: * 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: ". * `config.seed() >= 0`, both the boundary and the 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", and 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 own default set", not "clear it", so an unconfigured run must send nothing; a configured one must arrive. Two tests, one per branch. 658 tests, 0 failures. The PIT re-run confirming 830/830 is next; this commit is the test work, not the confirmation. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH --- .../LlamaCppJniProviderSupportTest.java | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniProviderSupportTest.java b/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniProviderSupportTest.java index d4e017f..981fafb 100644 --- a/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniProviderSupportTest.java +++ b/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniProviderSupportTest.java @@ -49,6 +49,12 @@ public class LlamaCppJniProviderSupportTest { /** JSON key of the repeat-penalty window, quoted for the same reason. */ private static final String PARAM_REPEAT_LAST_N = "\"repeat_last_n\""; + /** JSON key of the RNG seed, quoted for the same reason. */ + private static final String PARAM_SEED = "\"seed\""; + + /** JSON key of the DRY sequence breakers, quoted for the same reason. */ + private static final String PARAM_DRY_SEQUENCE_BREAKERS = "\"dry_sequence_breakers\""; + /** Chat-template kwarg key for Qwen-style thinking, as the provider spells it. */ private static final String KWARG_ENABLE_THINKING = "enable_thinking"; @@ -90,6 +96,10 @@ public void lazyMode_rejectsUnknownValueAndNamesTheAcceptedOnes() { IllegalArgumentException.class, () -> LlamaCppJniProviderSupport.lazyMode("lazy")); assertThat(thrown.getMessage(), containsString("lazy")); assertThat(thrown.getMessage(), containsString("off, auto, on")); + // Anchored to the "expected one of: " prefix on purpose: the renderer emits its separator + // only from the second value on, and a guard written as >= 0 instead of > 0 would prepend a + // stray ", " that a containsString on the middle of the list cannot see. + assertThat(thrown.getMessage(), containsString("expected one of: off")); } @Test @@ -115,6 +125,8 @@ public void cacheType_rejectsUnknownValueAndNamesBothTheKnobAndTheAcceptedOnes() assertThat(thrown.getMessage(), containsString("cacheTypeV")); assertThat(thrown.getMessage(), containsString("q3_k")); assertThat(thrown.getMessage(), containsString("f32, f16, bf16, q8_0")); + // Same anchor as lazyMode above, for the same separator boundary. + assertThat(thrown.getMessage(), containsString("expected one of: f32")); } // @@ -204,6 +216,83 @@ public void buildInferenceParameters_configuredWindows_arePassedThrough() { assertThat(json, containsString(PARAM_REPEAT_LAST_N + ": 128")); } + /** + * The seed sentinel is {@link net.ladenthin.srcmorph.config.AiGenerationConfig#DEFAULT_SEED} (-1), + * meaning "random per request". Forwarding it would pin every run to seed -1 and silently destroy + * the randomness upstream provides, so an unconfigured run must send nothing at all. + */ + @Test + public void buildInferenceParameters_defaultConfig_sendsNoSeed() { + // arrange + final LlamaCppJniConfig defaults = + LlamaCppJniConfig.builder("/does/not/exist.gguf").build(); + + // act + final String json = providerWith(defaults) + .buildInferenceParameters(request("class A {}")) + .toString(); + + // assert + assertThat(json, not(containsString(PARAM_SEED))); + } + + /** + * {@code 0} is a legitimate seed, not a second sentinel, so the guard has to be {@code >= 0}. Pins + * that boundary from the other side, exactly as the penalty windows do above -- a guard written as + * {@code > 0} would silently drop a run the user pinned to seed 0. + */ + @Test + public void buildInferenceParameters_zeroSeed_isStillSent() { + // arrange + final LlamaCppJniConfig zeroSeed = + LlamaCppJniConfig.builder("/does/not/exist.gguf").seed(0).build(); + + // act + final String json = providerWith(zeroSeed) + .buildInferenceParameters(request("class A {}")) + .toString(); + + // assert + assertThat(json, containsString(PARAM_SEED + ": 0")); + } + + /** + * An empty breaker list means "keep the binding's own default set", not "clear it". Sending an empty + * array would replace a sensible default with nothing, which is a behaviour change rather than a + * no-op -- hence the guard, and hence this test. + */ + @Test + public void buildInferenceParameters_defaultConfig_sendsNoDrySequenceBreakers() { + // arrange + final LlamaCppJniConfig defaults = + LlamaCppJniConfig.builder("/does/not/exist.gguf").build(); + + // act + final String json = providerWith(defaults) + .buildInferenceParameters(request("class A {}")) + .toString(); + + // assert + assertThat(json, not(containsString(PARAM_DRY_SEQUENCE_BREAKERS))); + } + + /** A configured breaker list reaches the request. */ + @Test + public void buildInferenceParameters_configuredDrySequenceBreakers_arePassedThrough() { + // arrange + final LlamaCppJniConfig configured = LlamaCppJniConfig.builder("/does/not/exist.gguf") + .drySequenceBreakers(Collections.singletonList("\\n")) + .build(); + + // act + final String json = providerWith(configured) + .buildInferenceParameters(request("class A {}")) + .toString(); + + // assert + assertThat(json, containsString(PARAM_DRY_SEQUENCE_BREAKERS)); + } + // // From 44ce8d54763a6cb6c43a4e36c60368c8d18b792e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 21:08:39 +0000 Subject: [PATCH 3/3] docs: close TODO #51 with the measurement it asked for The entry asked for a class on the PIT gate and, before committing to it, a measurement -- because the pure logic was covered by model-backed tests and PIT re-runs every test covering a mutated line. Both are done, and the answer is not the one the entry guessed. Measured on one machine, same suite, three runs: 807 mutations / 807 killed / 11:34 min before 830 mutations / 825 killed / 15:57 min with the split, five survivors 830 mutations / 830 killed / 12:14 min with the survivors killed So the extension costs about 40 s for 23 mutations, and `excludedTestClasses` -- which the entry 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 here. 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. CLAUDE.md now carries the boundary rule rather than only the outcome, because the split is load-bearing and easy to undo by accident: a pure method moved back into the provider silently drops off the gate, and an impure one moved into the support class reds it. The rule is "touches the native handle". TODO.md's entry is deleted rather than annotated, per that file's own header. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH --- CLAUDE.md | 26 +++++++++++++++++++++++++- TODO.md | 14 -------------- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9afde52..e067a97 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -533,12 +533,36 @@ See [`../workspace/policies/ci-test-diagnostics.md`](../workspace/policies/ci-te See [`../workspace/policies/pit-mutation-testing.md`](../workspace/policies/pit-mutation-testing.md). Run PIT with the lifecycle prefix. Reactor-wide (what CI does): `mvn test-compile org.pitest:pitest-maven:mutationCoverage`; or scoped to one module with -`-f srcmorph/pom.xml`. All three modules gate at `mutationThreshold` 100 — `srcmorph` (807 mutations), +`-f srcmorph/pom.xml`. All three modules gate at `mutationThreshold` 100 — `srcmorph` (830 mutations), `srcmorph-maven-plugin` (62, the five mojo classes) and `srcmorph-cli` (16). The CLI's `Main.main(String[])` is the one documented exclusion: it is the process entry point, and the `smoke-fatjar` release-gating job already runs the real `java -jar` artifact and asserts `Main#run end.` in its output, which is an end-to-end check a unit mutant cannot reach. +**`LlamaCppJniAiGenerationProvider` is split so the gate can reach it, and the split is load-bearing.** +Everything the provider does that does not touch the native handle lives in +`provider.LlamaCppJniProviderSupport`, which is on the gate; the provider itself is not. That is not +tidiness. PIT re-runs every test covering a mutated line, and while the pure logic sat next to +`model()` the only way to gate any of it was to gate `model()` too -- a ~100-line `ModelParameters` +chain whose sole exerciser is `LlamaCppJniKnobSweepTest`, 36 cases that each load a GGUF (22.9 s). +Excluding those tests instead does not work: `model()`'s mutants would then have no coverage, and a +`NO_COVERAGE` mutant fails a threshold-100 gate exactly like a survivor. **Keep the boundary at +"touches the native handle"** -- moving a pure method back into the provider silently drops it off +the gate, and moving an impure one into the support class reds it. + +The extension paid for itself immediately: the class contributed 23 mutations and **five survived**, +all real gaps (the two `known*Values` separator boundaries, both `seed` guards, and the +`drySequenceBreakers` branch). None could have been seen before, because a class that generates no +mutants cannot move the number -- which is exactly why the gate read a stable 775/775 straight +through the 1.1.0-era `dry_penalty_last_n` regression and through its fix. + +Cost, measured on one machine rather than estimated: 11:34 min at 807 mutations before, **12:14 min +at 830 after** -- about 40 s for 23 mutations. `excludedTestClasses` was considered and is **not** +used: it would remove a test class from mutant matching for *every* target class, not just this one, +and the measurement says it is not needed. (The intermediate 15:57 min reading was inflated by the +five survivors: PIT exhausts every covering test for a mutant that never dies, while a killed one +stops at the first failure.) + **Two classes are permanently off the gate, and this is worth not re-litigating.** Both have survivors that are *equivalent mutants*, unkillable through the public API rather than merely untested. `document.AiMdHeaderCodec`: the colon guard in `read` is reached only after diff --git a/TODO.md b/TODO.md index 78d4ae1..ef782cf 100644 --- a/TODO.md +++ b/TODO.md @@ -10,20 +10,6 @@ everything below is genuinely still open. ## Open -- **Put `provider.LlamaCppJniAiGenerationProvider` on the PIT gate.** It is the one production class - where a defect has actually reached users, and it is *not* on the `targetClasses` list — which is - why the gate stayed at 775/775 through the 1.1.0-era `dry_penalty_last_n` regression and through - its fix: the class generates no mutants, so neither the bug nor the tests that now cover it move - the number. Do not read that stability as reassurance; PIT structurally could not have caught this. - It is now worth adding, because the class finally has model-free coverage of the parts that matter - (`buildInferenceParameters`, `warnOnTruncatedAnswer`, `logPromptCacheReuse`, `lazyMode`, - `cacheType`). **Measure before committing to it**: the real-model tests take ~4-16 s each, and PIT - re-runs every test covering a mutated line, so mutants in `model()` — the long `ModelParameters` - chain — could make the run far slower than the current few minutes. If it does, the answer is - probably `excludedTestClasses` for the real-model tests plus mutants restricted to the pure paths, - not dropping the idea. Deliberately out of scope for 1.2.0: it is a build-time question, not a - correctness one. - - **The sixteen GPU classifier fat jars are verified structurally, never launched.** Since 1.2.0 `.github/verify-classifier-fatjars.sh` asserts each is the artifact its name claims (one jar per classifier, a native for the promised OS/arch, a native set that differs from the default jar's, so