Skip to content

Commit 18ed490

Browse files
committed
Make merged #289 training code pass SpotBugs + PIT (keep pure @builder)
The #289 merge introduced the first Lombok-@Builder-heavy class and it had not cleared the strict quality gates: SpotBugs (13 findings), all fixed while keeping class-level @builder: - 10x MRC_METHOD_RETURNS_CONSTANT on Lombok's synthetic $default$*() methods: @Builder.Default emits these, and they are NOT tagged @lombok.Generated even with lombok.addLombokGeneratedAnnotation=true (a Lombok limitation), so SpotBugs cannot auto-skip them. Suppressed for the $default$* methods only. - IMC_IMMATURE_CLASS_NO_TOSTRING: add @tostring (generated, @lombok.Generated). - IMC_IMMATURE_CLASS_WRONG_FIELD_ORDER: move the static MAPPER above the instance fields. - RCN_REDUNDANT_NULLCHECK in LlamaTrainer: the @NullMarked default treated the native finetuneNative() return as @nonnull; mark it @nullable (JNI can return null on success), making the `error != null` guard meaningful. PIT (was 99% < 100% threshold): - Optimizer.getNativeValue() showed as NO_COVERAGE because its only test lived in TrainingParametersTest (package parameters.*), outside PIT's args.*/value.*/ exception.*/json.* targetTests. Move it to a dedicated args.OptimizerTest so PIT runs it. Score back to 100% (249/249). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rt1paYztGJ2AKUuBuAGDXE
1 parent d64a365 commit 18ed490

5 files changed

Lines changed: 48 additions & 46 deletions

File tree

llama/spotbugs-exclude.xml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,4 +605,21 @@ SPDX-License-Identifier: MIT
605605
<Method name="synthesize"/>
606606
</Match>
607607

608+
<!--
609+
TrainingParameters uses Lombok @Builder with @Builder.Default on its schedule/optimizer
610+
fields. For each defaulted field Lombok emits a synthetic static `$default$<field>()` method
611+
that simply returns the default constant, which fb-contrib flags as
612+
MRC_METHOD_RETURNS_CONSTANT. Unlike Lombok's accessor/constructor output, these
613+
`$default$*` methods are NOT tagged with @lombok.Generated even when
614+
`lombok.addLombokGeneratedAnnotation = true` (a long-standing Lombok limitation), so
615+
SpotBugs cannot skip them automatically the way it does the rest of the generated members.
616+
The methods are pure generated boilerplate — returning a constant is exactly their purpose —
617+
so the finding is suppressed for these synthetic methods only.
618+
-->
619+
<Match>
620+
<Class name="net.ladenthin.llama.parameters.TrainingParameters"/>
621+
<Bug pattern="MRC_METHOD_RETURNS_CONSTANT"/>
622+
<Method name="~\$default\$.*"/>
623+
</Match>
624+
608625
</FindBugsFilter>

llama/src/main/java/net/ladenthin/llama/LlamaTrainer.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import net.ladenthin.llama.exception.LlamaException;
99
import net.ladenthin.llama.loader.LlamaLoader;
1010
import net.ladenthin.llama.parameters.TrainingParameters;
11+
import org.jspecify.annotations.Nullable;
1112

1213
/**
1314
* In-process fine-tuning entry point, wrapping llama.cpp's ggml-opt training path
@@ -60,5 +61,8 @@ public static void finetune(Path model, String trainingText, Path output, int ep
6061
.build());
6162
}
6263

63-
private static native String finetuneNative(String configJson);
64+
// The native layer returns null on success and a non-empty error message on failure, so the
65+
// return is genuinely @Nullable — without this the module's @NullMarked default would treat it
66+
// as @NonNull and SpotBugs would flag the `error != null` guard as a redundant null check.
67+
private static native @Nullable String finetuneNative(String configJson);
6468
}

llama/src/main/java/net/ladenthin/llama/parameters/TrainingParameters.java

Lines changed: 6 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.nio.file.Path;
1010
import lombok.Builder;
1111
import lombok.Getter;
12+
import lombok.ToString;
1213
import net.ladenthin.llama.args.Optimizer;
1314
import org.jspecify.annotations.Nullable;
1415

@@ -23,8 +24,13 @@
2324
*/
2425
@Builder
2526
@Getter
27+
@ToString
2628
public final class TrainingParameters {
2729

30+
// Jackson mapper for JSON serialization. Static field declared before the instance fields
31+
// to satisfy fb-contrib's IMC_IMMATURE_CLASS_WRONG_FIELD_ORDER (static members come first).
32+
private static final ObjectMapper MAPPER = new ObjectMapper();
33+
2834
// Base GGUF model to fine-tune.
2935
private final Path modelPath;
3036

@@ -81,45 +87,6 @@ public final class TrainingParameters {
8187
@Builder.Default
8288
private final int nUbatch = 0;
8389

84-
private static final ObjectMapper MAPPER = new ObjectMapper();
85-
86-
// Explicit all-args constructor used by the Lombok-generated builder. Declared explicitly (rather
87-
// than letting @Builder synthesize the package-private one) so Javadoc sees a real constructor and
88-
// does not emit the "use of default constructor, which does not provide a comment" warning; it is
89-
// private, so it is not part of the public API and is not doclint-checked.
90-
private TrainingParameters(
91-
Path modelPath,
92-
@Nullable String trainingText,
93-
@Nullable Path trainingFile,
94-
Path outputPath,
95-
int epochs,
96-
float learningRate,
97-
float lrMin,
98-
float decayEpochs,
99-
float weightDecay,
100-
Optimizer optimizer,
101-
int nCtx,
102-
int nGpuLayers,
103-
float valSplit,
104-
int nBatch,
105-
int nUbatch) {
106-
this.modelPath = modelPath;
107-
this.trainingText = trainingText;
108-
this.trainingFile = trainingFile;
109-
this.outputPath = outputPath;
110-
this.epochs = epochs;
111-
this.learningRate = learningRate;
112-
this.lrMin = lrMin;
113-
this.decayEpochs = decayEpochs;
114-
this.weightDecay = weightDecay;
115-
this.optimizer = optimizer;
116-
this.nCtx = nCtx;
117-
this.nGpuLayers = nGpuLayers;
118-
this.valSplit = valSplit;
119-
this.nBatch = nBatch;
120-
this.nUbatch = nUbatch;
121-
}
122-
12390
/**
12491
* Serialize this configuration to the JSON object the native fine-tuning layer expects.
12592
*
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// SPDX-FileCopyrightText: 2026 Bernard Ladenthin <bernard.ladenthin@gmail.com>
2+
//
3+
// SPDX-License-Identifier: MIT
4+
5+
package net.ladenthin.llama.args;
6+
7+
import static org.hamcrest.MatcherAssert.assertThat;
8+
import static org.hamcrest.Matchers.is;
9+
10+
import org.junit.jupiter.api.Test;
11+
12+
/** Model-free tests for the {@link Optimizer} native-value mapping. */
13+
class OptimizerTest {
14+
15+
@Test
16+
void nativeValuesMatchGgml() {
17+
assertThat(Optimizer.ADAMW.getNativeValue(), is(0));
18+
assertThat(Optimizer.SGD.getNativeValue(), is(1));
19+
}
20+
}

llama/src/test/java/net/ladenthin/llama/parameters/TrainingParametersTest.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,4 @@ void customValuesSerialize() throws Exception {
7070
// training_text is omitted when a corpus file is given.
7171
assertThat(node.has("training_text"), is(false));
7272
}
73-
74-
@Test
75-
void optimizerNativeValuesMatchGgml() {
76-
assertThat(Optimizer.ADAMW.getNativeValue(), is(0));
77-
assertThat(Optimizer.SGD.getNativeValue(), is(1));
78-
}
7973
}

0 commit comments

Comments
 (0)