From 66bd377e0e9a362b0cb7653666a35f911d79c497 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 07:08:27 +0000 Subject: [PATCH] fix: name the model in the flashAttn refusal so the message is not a constant SpotBugs' WEM_WEAK_EXCEPTION_MESSAGING fires on a throw whose entire message is a compile-time constant, which is what the provider-side flashAttn guard did. That reds main. The fix is not a suppression, because the finding is pointing at something real: this guard runs on the direct-API path, where there is no aiDefinition key to name -- the plan-time guard in EngineSupport prefixes that one -- so a caller running several models was told what is wrong but not which configuration to change. The message now leads with the model path, matching the shape EngineSupport already uses. The knob-sweep assertion moves from equality to containsString and also pins that the model path appears, so the prefix cannot be dropped again silently. Why it was not caught before the merge: spotbugs:check is bound to verify, and the runs used here were mvn test and mvn -P release package, both of which stop earlier. Verified this time with the two commands the code-style job runs verbatim (spotless:check and spotbugs:check over all three modules, 0 bugs) plus the 81 tests that touch the message. --- .../provider/LlamaCppJniAiGenerationProvider.java | 9 ++++++++- .../srcmorph/provider/LlamaCppJniKnobSweepTest.java | 8 +++++++- 2 files changed, 15 insertions(+), 2 deletions(-) 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 541c6054..4b8479e4 100644 --- a/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProvider.java +++ b/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProvider.java @@ -175,7 +175,14 @@ private LlamaModel model() { // llama.version alone will NOT surface this: the guard keeps throwing and the knob keeps // looking broken, so the bump checklist has to name it. See TODO.md. if (config.flashAttn()) { - throw new IllegalArgumentException(FLASH_ATTN_UNSUPPORTED_MESSAGE); + // The model path is not decoration: this guard fires on the direct-API path, where + // there is no aiDefinition key to name (the plan-time guard in EngineSupport prefixes + // that one), so without it a caller running several models is told what is wrong but + // not which configuration to change. It also keeps the message out of + // WEM_WEAK_EXCEPTION_MESSAGING, which SpotBugs raises on a throw whose whole message + // is a compile-time constant. + throw new IllegalArgumentException( + "model '" + config.modelPath() + "': " + FLASH_ATTN_UNSUPPORTED_MESSAGE); } // 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. diff --git a/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniKnobSweepTest.java b/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniKnobSweepTest.java index d1787ebb..96f34c8e 100644 --- a/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniKnobSweepTest.java +++ b/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniKnobSweepTest.java @@ -3,6 +3,7 @@ // 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.notNullValue; import static org.hamcrest.MatcherAssert.assertThat; @@ -217,7 +218,12 @@ public void flashAttn_isRefusedRatherThanSilentlyDropped() { try (LlamaCppJniAiGenerationProvider provider = new LlamaCppJniAiGenerationProvider(config, promptSupport)) { final IllegalArgumentException thrown = Assertions.assertThrows(IllegalArgumentException.class, () -> provider.generate(request())); - assertThat(thrown.getMessage(), is(LlamaCppJniAiGenerationProvider.FLASH_ATTN_UNSUPPORTED_MESSAGE)); + // containsString, not equality: the provider prefixes the model it is refusing for, so a + // caller running several models can tell which configuration to change. + assertThat( + thrown.getMessage(), + containsString(LlamaCppJniAiGenerationProvider.FLASH_ATTN_UNSUPPORTED_MESSAGE)); + assertThat(thrown.getMessage(), containsString(NativeLlamaAvailability.modelPath())); } }