diff --git a/CHANGELOG.md b/CHANGELOG.md index 40d5dbe..c5f43b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,9 +43,38 @@ The release procedure (prompt template and step-by-step instructions) lives in [ native** produces `Failed to parse model parameters` at load time and nothing earlier catches it. The knob sweep did — it is what surfaced the mismatch here. -- **`net.ladenthin:llama` 5.1.0 → 5.2.0-SNAPSHOT.** Deliberately a snapshot: the binding change this - release depends on is not yet published. Building srcmorph therefore requires that snapshot to be - resolvable, and CI stays red until it is — recorded here so nobody mistakes it for a regression. +- **`net.ladenthin:llama` 5.1.0 → 5.2.0.** Deliberately the *release* version, not `5.2.0-SNAPSHOT`: + this repo declares no `` element (the `distributionManagement/snapshotRepository` is + deploy-only), so the Central snapshot channel is not resolvable here at all — and a `-SNAPSHOT` + dependency on `main` would block every srcmorph release, since Central rejects one. The build + therefore stays red until `net.ladenthin:llama:5.2.0` is published and goes green with no further + edit the moment it is — recorded here so nobody mistakes it for a regression. + +- **BREAKING: `chatTemplateEnableThinking` is a tri-state; the `enable_thinking` kwarg is only sent + when it was actually set.** It used to be a plain `boolean` defaulting to `true`, so every run put + `enable_thinking` into the chat-template kwargs -- including runs whose chat template has never + heard of the kwarg, which llama.cpp's Jinja layer has been moving from "silently ignored" toward + "warned about". The only way to stop that noise was to set the knob to `false`, which means + something else entirely. + + The obvious phrasing -- "send it only when it differs from the template's default" -- is not + implementable: srcmorph would have to parse and evaluate the template to know that default, which + is exactly the work it delegates to the binding. So the rule is "send it only when the user + actually set it". + + `AiGenerationConfig`, `AiModelDefinition` and `LlamaCppJniConfig` (and its builder) now carry + `@Nullable Boolean` instead of `boolean`, `DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING` is `null` + (unset) instead of `true`, and the two `isChatTemplateEnableThinking()` getters were renamed to + `getChatTemplateEnableThinking()` to match the wrapper type. `true` and `false` are both still + forwarded verbatim -- unset is the only value that omits the kwarg. A configuration that never + mentioned the knob keeps working and simply stops sending it; one that set it explicitly is + unaffected. + + Guarded by four model-free tests over the extracted `buildChatTemplateKwargs()` -- including the + `true` case, so a future "send it only when false" shortcut fails rather than silently swallowing + a configured value. + +--- ## [1.2.0] - 2026-09-01 diff --git a/TODO.md b/TODO.md index 347d20f..8234988 100644 --- a/TODO.md +++ b/TODO.md @@ -55,24 +55,6 @@ recorded in git history and `crossrepostatus.md`, not here. not dropping the idea. Deliberately out of scope for 1.2.0: it is a build-time question, not a correctness one. -- **`enable_thinking` is sent unconditionally, including at its own default.** - `LlamaCppJniAiGenerationProvider.model()` always puts `enable_thinking` into - `chatTemplateKwargs`, at whatever `chatTemplateEnableThinking` says — and its default is `true`. - A model whose chat template does not know the kwarg gets it anyway; llama.cpp's Jinja layer has - been moving such unknown kwargs from "ignored" toward "warned about", so a default run emits noise - that the user did not ask for and cannot switch off without setting the knob to a value that means - something else. - - The obvious phrasing of the fix -- "send it only when it differs from the template's default" -- - is **not implementable**: srcmorph cannot know a template's default without parsing and evaluating - the template, which is exactly the work it delegates to the binding. The implementable fix is - "send it only when the user actually set it", and that needs the config field to become a tri-state - (`Boolean` rather than `boolean`, `null` = unset), which changes `AiGenerationConfig`, - `AiModelDefinition`, `LlamaCppJniConfig` and its builder, plus the plugin's `@Parameter`. That is a - public-API change, so it belongs in a minor release with the deprecation story written out, not in - a patch. **Was announced during the 1.2.0 audit cycle and never landed** -- recorded here rather - than left as a claim in a chat log. - - **`srcmorph:calibrate` reports only through the log.** `CalibrateEngine` builds a `CalibrationReport` and `CalibrateMojo` prints it as `INFO` lines. There is no machine-readable output, so the numbers a calibration run produces (prefill / decode throughput, chars per token per diff --git a/srcmorph-maven-plugin/README.md b/srcmorph-maven-plugin/README.md index 6a17cee..09dc954 100644 --- a/srcmorph-maven-plugin/README.md +++ b/srcmorph-maven-plugin/README.md @@ -442,7 +442,7 @@ below are the shipped values (`AiGenerationConfig.DEFAULT_*`). | `lazyMode` | *(empty)* | On-demand reading of lazy-loadable tensors (`--tensor-read-lazy`): `off`, `auto` or `on`; empty leaves the default. Trades resident memory for disk reads and shortens model load time; **requires mmap** | | `mainGpu` | `-1` | Primary GPU index (`--main-gpu`); `-1` = leave default. Matters on multi-GPU hosts (e.g. a Vulkan build enumerates every GPU) | | `devices` | *(empty)* | Explicit device selection (`--device`), comma-separated backend device names (e.g. `Vulkan1`); takes precedence over `mainGpu` | -| `chatTemplateEnableThinking` | `true` | Enable the chat template's thinking mode | +| `chatTemplateEnableThinking` | *(unset)* | Chat-template thinking mode (`enable_thinking` kwarg): `true` or `false` is forwarded verbatim, unset omits the kwarg so the model's own template default applies. Set `false` for Gemma 4 to keep thought tokens out of stored output | | `reasoningEffort` | `low` | gpt-oss harmony reasoning effort (`low`/`medium`/`high`); empty omits the kwarg (e.g. for non-gpt-oss models) | | `reasoningBudgetTokens` | `-1` | Cap on harmony reasoning tokens (`-1` = unrestricted) | | `dryMultiplier` | `0.0` | DRY repetition-penalty multiplier (`0.0` = disabled); the other `dry*` knobs only apply when this is `> 0` | diff --git a/srcmorph/pom.xml b/srcmorph/pom.xml index 686a400..c65519b 100644 --- a/srcmorph/pom.xml +++ b/srcmorph/pom.xml @@ -51,13 +51,13 @@ SPDX-License-Identifier: Apache-2.0 InferenceParameters, ModelParameters, ChatResponse/Timings/Pair, ChatResponseParser, ReasoningFormat) is untouched by 5.1.0. 5.2.0 is a BREAKING bump this module needs, not an optional one: the binding removed - ModelParameters.enableFlashAttn()/ModelFlag.FLASH_ATTN (--flash-attn takes a mandatory + ModelParameters.enableFlashAttn()/ModelFlag.FLASH_ATTN (the flash-attn flag takes a mandatory on|off|auto value since llama.cpp b10273, so the valueless flag emitted a broken argv) and renamed setTensorReadLazy/TensorReadLazyMode to setLazyMode/LazyMode (llama.cpp - b10731 renamed --tensor-read-lazy to --lazy-mode with no alias). The provider calls + b10731 renamed the tensor-read-lazy flag to lazy-mode with no alias). The provider calls both, so it cannot compile against 5.1.0 any more. This deliberately names the RELEASE, not 5.2.0-SNAPSHOT: a snapshot would need a - entry for the Central snapshot channel (this repo declares none -- the + entry for the Central snapshot channel (this repo declares none; the distributionManagement/snapshotRepository below is deploy-only) AND would block every srcmorph release, since Central rejects a release with a -SNAPSHOT dependency. The build therefore stays red until net.ladenthin:llama:5.2.0 is published, and goes green diff --git a/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiGenerationConfig.java b/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiGenerationConfig.java index 81cd0cc..fcf6de0 100644 --- a/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiGenerationConfig.java +++ b/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiGenerationConfig.java @@ -123,15 +123,24 @@ public AiGenerationConfig() { public static final float DEFAULT_REPEAT_PENALTY = 1.0f; /** - * Default setting for whether the model's chat-template thinking mode is enabled. + * Default setting for the model's chat-template thinking mode: {@code null}, meaning + * unset. * - *

When enabled, the model uses its own chat-template default for chain-of-thought - * reasoning. When disabled, {@link net.ladenthin.llama.parameters.ModelParameters#setChatTemplateKwargs} - * is called with {@code {"enable_thinking": "false"}} to suppress the thinking block at the - * Jinja template level — set to {@code false} for Gemma 4 definitions to prevent - * {@code <|channel>thought} tokens from leaking into stored output.

+ *

The value is a tri-state on purpose. While it was a plain {@code boolean} defaulting to + * {@code true}, every run put {@code enable_thinking} into + * {@link net.ladenthin.llama.parameters.ModelParameters#setChatTemplateKwargs} — including + * runs whose chat template has never heard of the kwarg, which llama.cpp's Jinja layer has been + * moving from "silently ignored" toward "warned about". There was no way to switch that noise + * off except by setting the knob to {@code false}, which means something else entirely.

+ * + *

"Send it only when it differs from the template's default" is not implementable here: + * srcmorph would have to parse and evaluate the template to know that default, which is exactly + * the work it delegates to the binding. So the rule is "send it only when the user actually set + * it": {@code null} omits the kwarg and the model's own chat-template default applies, while + * {@code true} and {@code false} are both forwarded verbatim. Set {@code false} for Gemma 4 + * definitions to keep {@code <|channel>thought} tokens out of stored output.

*/ - public static final boolean DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING = true; + public static final @Nullable Boolean DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING = null; /** * Default for whether llama.cpp prompt caching ({@code cache_prompt}) is enabled. @@ -387,7 +396,7 @@ public AiGenerationConfig() { private float minP = DEFAULT_MIN_P; private float topNSigma = DEFAULT_TOP_N_SIGMA; private float repeatPenalty = DEFAULT_REPEAT_PENALTY; - private boolean chatTemplateEnableThinking = DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING; + private @Nullable Boolean chatTemplateEnableThinking = DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING; private boolean cachePrompt = DEFAULT_CACHE_PROMPT; private boolean swaFull = DEFAULT_SWA_FULL; private int cacheReuse = DEFAULT_CACHE_REUSE; @@ -655,20 +664,26 @@ public void setTopNSigma(final float topNSigma) { } /** - * Returns whether the model's chat-template thinking mode is enabled. + * Returns the configured chat-template thinking mode, or {@code null} when it was never set. * - * @return {@code true} if chat-template thinking mode is enabled + * @return {@code true} or {@code false} when the knob was set, {@code null} when it is unset + * — in which case the {@code enable_thinking} kwarg is not sent at all and the + * model's own chat-template default applies + * @see #DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING */ - public boolean isChatTemplateEnableThinking() { + public @Nullable Boolean getChatTemplateEnableThinking() { return chatTemplateEnableThinking; } /** - * Sets whether the model's chat-template thinking mode is enabled. + * Sets the model's chat-template thinking mode. * - * @param chatTemplateEnableThinking {@code true} to enable chat-template thinking mode + * @param chatTemplateEnableThinking {@code true} or {@code false} to forward + * {@code enable_thinking} to the chat template, {@code null} to omit the kwarg entirely + * and leave the template's own default in place + * @see #DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING */ - public void setChatTemplateEnableThinking(final boolean chatTemplateEnableThinking) { + public void setChatTemplateEnableThinking(final @Nullable Boolean chatTemplateEnableThinking) { this.chatTemplateEnableThinking = chatTemplateEnableThinking; } diff --git a/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiModelDefinition.java b/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiModelDefinition.java index 3209963..474008e 100644 --- a/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiModelDefinition.java +++ b/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiModelDefinition.java @@ -51,7 +51,8 @@ public AiModelDefinition() { private float minP = AiGenerationConfig.DEFAULT_MIN_P; private float topNSigma = AiGenerationConfig.DEFAULT_TOP_N_SIGMA; private float repeatPenalty = AiGenerationConfig.DEFAULT_REPEAT_PENALTY; - private boolean chatTemplateEnableThinking = AiGenerationConfig.DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING; + private @Nullable Boolean chatTemplateEnableThinking = + AiGenerationConfig.DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING; private boolean cachePrompt = AiGenerationConfig.DEFAULT_CACHE_PROMPT; private boolean swaFull = AiGenerationConfig.DEFAULT_SWA_FULL; private int cacheReuse = AiGenerationConfig.DEFAULT_CACHE_REUSE; @@ -324,24 +325,26 @@ public void setTopNSigma(final float topNSigma) { } /** - * Returns whether the model's chat-template thinking mode is enabled. + * Returns this definition's chat-template thinking mode, or {@code null} when it was never set. * - * @return {@code true} to keep thinking enabled via the model's chat-template default; + * @return {@code true} or {@code false} when the knob was set, {@code null} when it is unset + * — the kwarg is then not sent and the model's own chat-template default applies; * defaults to {@link AiGenerationConfig#DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING} */ - public boolean isChatTemplateEnableThinking() { + public @Nullable Boolean getChatTemplateEnableThinking() { return chatTemplateEnableThinking; } /** - * Sets whether the model's chat-template thinking mode is enabled. + * Sets this definition's chat-template thinking mode. * * @param chatTemplateEnableThinking {@code false} passes * {@code enable_thinking=false} to * {@link net.ladenthin.llama.parameters.ModelParameters#setChatTemplateKwargs} to suppress - * chain-of-thought reasoning at the Jinja template level + * chain-of-thought reasoning at the Jinja template level, {@code true} passes it as + * {@code true}, and {@code null} omits the kwarg entirely */ - public void setChatTemplateEnableThinking(final boolean chatTemplateEnableThinking) { + public void setChatTemplateEnableThinking(final @Nullable Boolean chatTemplateEnableThinking) { this.chatTemplateEnableThinking = chatTemplateEnableThinking; } diff --git a/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiModelDefinitionSupport.java b/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiModelDefinitionSupport.java index 629aeaf..cc0e692 100644 --- a/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiModelDefinitionSupport.java +++ b/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiModelDefinitionSupport.java @@ -118,7 +118,7 @@ private static AiGenerationConfig toConfig(final AiModelDefinition definition) { config.setTopNSigma(definition.getTopNSigma()); config.setRepeatPenalty(definition.getRepeatPenalty()); config.setStopStrings(definition.getStopStrings()); - config.setChatTemplateEnableThinking(definition.isChatTemplateEnableThinking()); + config.setChatTemplateEnableThinking(definition.getChatTemplateEnableThinking()); config.setCachePrompt(definition.isCachePrompt()); config.setSwaFull(definition.isSwaFull()); config.setCacheReuse(definition.getCacheReuse()); 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 a85fec0..328fa16 100644 --- a/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProvider.java +++ b/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProvider.java @@ -90,18 +90,43 @@ public LlamaCppJniAiGenerationProvider(final LlamaCppJniConfig config, final AiP 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; + } + /** 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 = - new HashMap<>(compatibilityHelper.hashMapCapacityFor(CHAT_TEMPLATE_KWARG_COUNT)); - chatTemplateKwargs.put(ENABLE_THINKING_KWARG, String.valueOf(config.chatTemplateEnableThinking())); - // 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()); - } + final Map chatTemplateKwargs = buildChatTemplateKwargs(); final ModelParameters modelParameters = new ModelParameters() .setModel(config.modelPath()) .setCtxSize(config.contextSize()) diff --git a/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniConfig.java b/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniConfig.java index d7732a7..858742b 100644 --- a/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniConfig.java +++ b/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniConfig.java @@ -10,6 +10,7 @@ import lombok.ToString; import net.ladenthin.srcmorph.config.AiGenerationConfig; import net.ladenthin.srcmorph.support.ConvertToRecord; +import org.jspecify.annotations.Nullable; /** * Immutable configuration for the llama.cpp JNI provider. @@ -37,7 +38,7 @@ public final class LlamaCppJniConfig { private final float minP; private final float topNSigma; private final float repeatPenalty; - private final boolean chatTemplateEnableThinking; + private final @Nullable Boolean chatTemplateEnableThinking; private final boolean cachePrompt; private final boolean swaFull; private final int cacheReuse; @@ -224,11 +225,14 @@ public float repeatPenalty() { } /** - * Returns whether chat-template thinking mode is enabled. + * Returns the chat-template thinking mode, or {@code null} when it was never set. * - * @return {@code true} when chat-template thinking mode is enabled + * @return {@code true} or {@code false} when the knob was set, {@code null} when it is unset + * — the provider then omits the {@code enable_thinking} kwarg entirely and the + * model's own chat-template default applies + * @see AiGenerationConfig#DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING */ - public boolean chatTemplateEnableThinking() { + public @Nullable Boolean chatTemplateEnableThinking() { return chatTemplateEnableThinking; } @@ -489,7 +493,8 @@ public static final class Builder { private float minP = AiGenerationConfig.DEFAULT_MIN_P; private float topNSigma = AiGenerationConfig.DEFAULT_TOP_N_SIGMA; private float repeatPenalty = AiGenerationConfig.DEFAULT_REPEAT_PENALTY; - private boolean chatTemplateEnableThinking = AiGenerationConfig.DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING; + private @Nullable Boolean chatTemplateEnableThinking = + AiGenerationConfig.DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING; private boolean cachePrompt = AiGenerationConfig.DEFAULT_CACHE_PROMPT; private boolean swaFull = AiGenerationConfig.DEFAULT_SWA_FULL; private int cacheReuse = AiGenerationConfig.DEFAULT_CACHE_REUSE; @@ -621,12 +626,13 @@ public Builder repeatPenalty(final float repeatPenalty) { } /** - * Sets whether chat-template thinking mode is enabled. + * Sets the chat-template thinking mode. * - * @param chatTemplateEnableThinking whether chat-template thinking mode is enabled + * @param chatTemplateEnableThinking {@code true} or {@code false} to forward + * {@code enable_thinking} to the chat template, {@code null} to omit the kwarg * @return this builder */ - public Builder chatTemplateEnableThinking(final boolean chatTemplateEnableThinking) { + public Builder chatTemplateEnableThinking(final @Nullable Boolean chatTemplateEnableThinking) { this.chatTemplateEnableThinking = chatTemplateEnableThinking; return this; } diff --git a/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniConfigFactory.java b/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniConfigFactory.java index 8ea9d49..7126b29 100644 --- a/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniConfigFactory.java +++ b/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniConfigFactory.java @@ -43,7 +43,7 @@ public static LlamaCppJniConfig fromGenerationConfig(final AiGenerationConfig co .minP(config.getMinP()) .topNSigma(config.getTopNSigma()) .repeatPenalty(config.getRepeatPenalty()) - .chatTemplateEnableThinking(config.isChatTemplateEnableThinking()) + .chatTemplateEnableThinking(config.getChatTemplateEnableThinking()) .cachePrompt(config.isCachePrompt()) .swaFull(config.isSwaFull()) .cacheReuse(config.getCacheReuse()) diff --git a/srcmorph/src/test/java/net/ladenthin/srcmorph/config/AiModelDefinitionSupportTest.java b/srcmorph/src/test/java/net/ladenthin/srcmorph/config/AiModelDefinitionSupportTest.java index 22099a7..949eb57 100644 --- a/srcmorph/src/test/java/net/ladenthin/srcmorph/config/AiModelDefinitionSupportTest.java +++ b/srcmorph/src/test/java/net/ladenthin/srcmorph/config/AiModelDefinitionSupportTest.java @@ -98,7 +98,7 @@ public void getConfig_knownKey_propagatesAllFields() { assertThat(config.getThreads(), is(equalTo(4))); assertThat(config.getCharsPerToken(), is(equalTo(3))); assertThat(config.isWarnOnTrim(), is(false)); - assertThat(config.isChatTemplateEnableThinking(), is(false)); + assertThat(config.getChatTemplateEnableThinking(), is(false)); // topP/topK/repeatPenalty/stopStrings are propagated too — kills the void-call mutants // that would drop those setter calls from toConfig(). assertThat(config.getTopP(), is(equalTo(0.55f))); @@ -164,7 +164,7 @@ public void getConfig_defaultValues_matchAiGenerationConfigDefaults() { assertThat(config.getCharsPerToken(), is(equalTo(AiGenerationConfig.DEFAULT_CHARS_PER_TOKEN))); assertThat(config.getMaxInputChars(), is(equalTo(AiGenerationConfig.DEFAULT_MAX_INPUT_CHARS))); assertThat(config.isWarnOnTrim(), is(AiGenerationConfig.DEFAULT_WARN_ON_TRIM)); - assertThat(config.isChatTemplateEnableThinking(), is(AiGenerationConfig.DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING)); + assertThat(config.getChatTemplateEnableThinking(), is(AiGenerationConfig.DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING)); assertThat(config.isCachePrompt(), is(AiGenerationConfig.DEFAULT_CACHE_PROMPT)); assertThat(config.isSwaFull(), is(AiGenerationConfig.DEFAULT_SWA_FULL)); assertThat(config.getCacheReuse(), is(equalTo(AiGenerationConfig.DEFAULT_CACHE_REUSE))); 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 bbf4fa2..d1c9c5e 100644 --- a/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProviderTest.java +++ b/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProviderTest.java @@ -15,6 +15,7 @@ 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; @@ -44,6 +45,12 @@ public class LlamaCppJniAiGenerationProviderTest { /** 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, @@ -396,5 +403,104 @@ public void logPromptCacheReuse_debugDisabled_saysNothing() { } } + // + + /** + * 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")); + } + + // + // }