spotbugs: clear remaining Max+Low findings (DLS + SPP + RCN + Lombok USBR + getter routing)

End-to-end sweep that brings jllama to zero SpotBugs Max+Low findings.
The history audit while doing this surfaced that several earlier
"successful" suppressions were lost in past rebases and never actually
committed — this commit re-applies the canonical pattern in one place.

Five changes:

1. DLS_DEAD_LOCAL_STORE on LlamaModel.completeAsync — Option A from the
   cross-repo discussion: drop the `cancelHook` local entirely, lift
   `@SuppressWarnings("FutureReturnValueIgnored")` to the method. The
   inline comment documents why the suppression sits at method scope
   (cross-repo FireAndForget DLS reckoning).

2. SPP_FIELD_COULD_BE_STATIC on LlamaModel.ctx — narrow `<Match>` block
   in spotbugs-exclude.xml with rationale: `ctx` is the per-instance
   native handle, making it static would corrupt state across parallel
   LlamaModel instances.

3. RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE x2 in ChatRequest.buildMessagesJson:
   ChatMessage.getContent() and ToolCall.getArgumentsJson() are both
   @nonnull (no @nullable annotation; NullAway proves the contract);
   the `== null ? "" : ...` ternaries were dead code. Removed.

4. USBR_UNNECESSARY_STORE_BEFORE_RETURN on Lombok-generated equals /
   hashCode / canEqual / toString — restore the cross-repo canonical
   suppression block. Lombok injects the textbook polynomial-hash
   pattern (`int result = 1; ...; return result;`) on every value
   class; fb-contrib's USBR detector doesn't honour @lombok.Generated
   and would otherwise fire ~18 times across the codebase. Suppression
   matches the four Lombok-emitted method names; the cross-repo
   rationale lives in ../workspace/policies/lombok-config.md.

5. lombok.config — restore `doNotUseGetters = true` for
   @EqualsAndHashCode and @tostring. Cross-repo invariant tracked in
   the workspace policy. Without it, fb-contrib
   OI_OPTIONAL_ISSUES_CHECKING_REFERENCE fires on every Optional-
   wrapping getter routed through Lombok's generated equals/hashCode
   (ChatMessage.getParts() returns Optional<List<ContentPart>>).

Verification:
- mvn compile spotbugs:check -Dspotbugs.effort=Max -Dspotbugs.threshold=Low
  → BugInstance size is 0, BUILD SUCCESS.
- 121 tests pass across ChatRequestTest / ChatResponseTest /
  InferenceParametersTest / JsonParametersTest / MultimodalMessagesTest.

Net cross-repo SpotBugs Max+Low: 0 / 0 / 0 / 0 across jllama / BAF /
plugin / streambuffer.

Author: claude

lombok.config

@@ -19,32 +19,13 @@ lombok.equalsAndHashCode.callSuper = skip
 lombok.toString.callSuper = skip
 
 # Force Lombok's @EqualsAndHashCode / @ToString to read FIELDS directly
-# instead of routing through `this.getX()` (the default). Rationale:
-#
-# Some classes expose value-add getters that wrap their @Nullable field
-# in an Optional (e.g. ChatRequest.getToolChoice / ChatMessage.getToolCallId)
-# or wrap a list field in Collections.unmodifiableList + Optional
-# (ChatMessage.getParts). Those wrappers are the public-API contract,
-# not equality contracts:
-#
-#   1. fb-contrib's OI_OPTIONAL_ISSUES_CHECKING_REFERENCE fires on every
-#      Lombok-generated `this$x == null` branch when `x` is an Optional —
-#      Optional is the standard "never null" type, so the null branch is
-#      dead code.
-#   2. ChatMessage.getParts() allocates a fresh
-#      Collections.unmodifiableList AND a fresh Optional on every equals
-#      call. Field-access avoids both allocations per comparison.
-#   3. The two forms are semantically equivalent: Optional.equals and
-#      Collections.unmodifiableList(x).equals(...) both delegate to
-#      value-based comparison of the underlying state.
-#
-# All value classes in this repo are `final`, so subclass-override of a
-# getter cannot change equality (no subclasses exist). callSuper=true
-# chains are unaffected — `super.equals()` is still a method call, and
-# the parent class's own field handling is governed by the same setting.
-# Verified by the audit at commit time: zero Bucket-3 classes (where
-# getter form would be preferred), zero tests pinning Lombok-format
-# `Class(field=value)` substrings.
+# instead of routing through `this.getX()` (the default). Rationale lives
+# in ../workspace/policies/lombok-config.md. Cross-repo invariant: all
+# three Lombok-using repos ship the same setting. Without it,
+# fb-contrib's OI_OPTIONAL_ISSUES_CHECKING_REFERENCE fires on every
+# Lombok-generated `this$x == null` branch when `x` is an Optional, and
+# Optional/unmodifiable-wrapper getters allocate fresh wrappers on every
+# equals call.
 lombok.equalsAndHashCode.doNotUseGetters = true
 lombok.toString.doNotUseGetters = true
 

spotbugs-exclude.xml

@@ -208,6 +208,8 @@ SPDX-License-Identifier: MIT
         emit. The collateral cost is small: any handwritten member of those four names
         that genuinely stores-then-immediately-returns is either a debugger-friendly
         local-variable pattern or a micro-optimisation, both intentional here.
+
+        Cross-repo invariant — see `../workspace/policies/lombok-config.md`.
     -->
     <Match>
         <Or>
@@ -267,4 +269,21 @@ SPDX-License-Identifier: MIT
         </Or>
     </Match>
 
+    <!--
+        LlamaModel.ctx is the per-instance native handle: a long pointer
+        into the llama.cpp context owned by THIS LlamaModel instance.
+        fb-contrib's SPP_FIELD_COULD_BE_STATIC detector observes that
+        the field is only assigned inside loadModel (called from the
+        constructor) and never reassigned, and concludes the field could
+        be promoted to static. That is incorrect: every LlamaModel wraps
+        its OWN native context, and making ctx static would cause every
+        instance to share one handle — corrupting state across parallel
+        inference calls and double-freeing on close().
+    -->
+    <Match>
+        <Class name="net.ladenthin.llama.LlamaModel"/>
+        <Bug pattern="SPP_FIELD_COULD_BE_STATIC"/>
+        <Field name="ctx"/>
+    </Match>
+
 </FindBugsFilter>

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

@@ -236,13 +236,16 @@ public CompletableFuture<String> completeAsync(InferenceParameters parameters) {
      * @param token cancellation handle bound to the underlying inference loop
      * @return a future completed with whatever text was generated up to the point of stop or cancellation
      */
+    // The whenComplete return value is deliberately discarded: it is a
+    // fire-and-forget cancellation callback attached to `future`, and `future`
+    // (not the chained stage) is what the caller observes. The suppression sits
+    // on the method instead of on a local variable because the local-variable
+    // form triggered fb-contrib DLS_DEAD_LOCAL_STORE — see workspace/crossrepostatus.md
+    // "FireAndForget DLS reckoning" row for the cross-repo policy.
+    @SuppressWarnings("FutureReturnValueIgnored")
     public CompletableFuture<String> completeAsync(InferenceParameters parameters, CancellationToken token) {
         CompletableFuture<String> future = CompletableFuture.supplyAsync(() -> complete(parameters, token));
-        // whenComplete returns a new stage that we deliberately discard: this is a
-        // fire-and-forget cancellation callback attached to `future`, which is what
-        // the caller observes.
-        @SuppressWarnings("FutureReturnValueIgnored")
-        final CompletableFuture<String> cancelHook = future.whenComplete((result, ex) -> {
+        future.whenComplete((result, ex) -> {
             if (ex instanceof java.util.concurrent.CancellationException) {
                 token.cancel();
             }