chore: make SpotBugs (effort=Max, threshold=Low) clean again

SpotBugs is already enabled at effort=Max/threshold=Low (fb-contrib + findsecbugs) and bound to the
verify phase — but the model-backed CI jobs run mvn test / mvn package, which never reach verify, so
the audit Tier-1–3 fixes (merged in #258) introduced 6 findings that went uncaught. Resolve all 6:

- ITC_INHERITANCE_TYPE_CHECKING (JsonParameters.withScalar): replace the instanceof Float +
  instanceof Double pair with a single `instanceof Number && !Double.isFinite(...)` check (also tidier).
- EI_EXPOSE_REP (ChatMessage.getToolCalls): return a fresh Collections.unmodifiableList view instead of
  the stored list, mirroring getParts — the internal list is never exposed.
- THROWS_METHOD_THROWS_RUNTIMEEXCEPTION x3 (LlamaModel.completeBatch / completeBatchWithStats /
  chatBatch): the re-throw of the captured first failure is intentional — narrow <Match> suppression
  with rationale in spotbugs-exclude.xml.
- OCP_OVERLY_CONCRETE_COLLECTION_PARAMETER (ChatMessage public constructor): the List<ToolCall> param is
  the deliberate public-API type; widening to Collection would be a binary-incompatible change — narrow
  <Match> suppression with rationale.

`mvn spotbugs:check` is green; 152 affected Java tests, Spotless, Javadoc clean. TODO.md updated
(the "flip the pom / ~65 findings" note was stale; notes that verify is not reached by the test jobs).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: vaijurao

TODO.md

@@ -435,7 +435,15 @@ Feel free to contribute fixes — PRs welcome.
 
 - **Null-safety refinement.** JSpecify + NullAway are now enforced at compile time in **strict JSpecify mode** with the extra options `CheckOptionalEmptiness`, `AcknowledgeRestrictiveAnnotations`, `AcknowledgeAndroidRecent`, `AssertsEnabled` (see `pom.xml`); `@NullMarked` on the three packages via `package-info.java`; JDK module exports in `.mvn/jvm.config`. The legacy `org.jetbrains.annotations` dep has been removed; all nullability annotations are JSpecify. Public-API methods that may legitimately have no value use `Optional<T>` rather than `@Nullable T` (`ChatResponse.getFirstMessage`, `ChatMessage.getParts`, `ChatRequest.buildToolsJson`). Open follow-up: review remaining unannotated public API surfaces for places where `@Nullable` would be more precise than the implicit non-null default.
 
-- **SpotBugs `effort=Max` + `threshold=Low`** — currently default effort/threshold. Raising both surfaces ~65 remaining findings (was 90; the cross-repo `OPM_OVERLY_PERMISSIVE_METHOD` suppression in `07109cc` silenced 25 of them pending the package refactor — see below). Top remaining patterns: `DRE_DECLARED_RUNTIME_EXCEPTION` 20, `WEM_WEAK_EXCEPTION_MESSAGING` 14. The BAF/sb/plugin playbook applies: flip pom, run `spotbugs:check`, fix at source where reasonable + narrow `<Match>` with rationale for structural false positives. Cross-cutting (tracked in [`../workspace/crossrepostatus.md`](../workspace/crossrepostatus.md)).
+- **SpotBugs `effort=Max` + `threshold=Low`** — **DONE (already enabled in `pom.xml`)**, with fb-contrib +
+  findsecbugs, bound to `verify`. The legacy "flip the pom / ~65 findings" note is stale: only a handful
+  of unexcluded findings remain at any time, and `spotbugs:check` is kept green. Most recent pass fixed the
+  6 introduced by the audit Tier-1–3 fixes — `withScalar` uses a single `instanceof Number` (no
+  `ITC_INHERITANCE_TYPE_CHECKING`); `ChatMessage.getToolCalls` returns a fresh unmodifiable view (no
+  `EI_EXPOSE_REP`); the `LlamaModel` batch methods' deliberate re-throw and the `ChatMessage` public
+  constructor's `List` param carry narrow `<Match>` rationale suppressions.
+  **Note:** `spotbugs:check` is bound to the `verify` phase, which the model-backed CI test jobs
+  (`mvn test` / `mvn package`) do not reach — run `mvn verify` (or a dedicated job) to gate it in CI.
 
 - **Drop the project-wide `OPM_OVERLY_PERMISSIVE_METHOD` suppression in
   `spotbugs-exclude.xml`** once the package-architecture refactor lands

spotbugs-exclude.xml

@@ -347,6 +347,36 @@ SPDX-License-Identifier: MIT
         <Method name="invoke"/>
     </Match>
 
+    <!--
+        LlamaModel.completeBatch / completeBatchWithStats / chatBatch dispatch N async requests, then
+        deliberately join EVERY future before re-throwing the first captured failure (a
+        CompletionException) — so a partial failure never abandons sibling requests running unobserved.
+        Re-throwing the captured RuntimeException is the intended behaviour; fb-contrib flags any
+        `throw <RuntimeException>` as THROWS_METHOD_THROWS_RUNTIMEEXCEPTION.
+    -->
+    <Match>
+        <Class name="net.ladenthin.llama.LlamaModel"/>
+        <Bug pattern="THROWS_METHOD_THROWS_RUNTIMEEXCEPTION"/>
+        <Or>
+            <Method name="completeBatch"/>
+            <Method name="completeBatchWithStats"/>
+            <Method name="chatBatch"/>
+        </Or>
+    </Match>
+
+    <!--
+        ChatMessage's public constructor intentionally accepts List<ToolCall> (the natural,
+        ergonomic public-API type, matching the parts constructor). fb-contrib notes the parameter
+        is only consumed as a Collection (defensively copied into an ArrayList) and suggests widening
+        it; doing so on a public constructor would be a binary-incompatible API change for no caller
+        benefit, so the concrete List type is kept deliberately.
+    -->
+    <Match>
+        <Class name="net.ladenthin.llama.value.ChatMessage"/>
+        <Bug pattern="OCP_OVERLY_CONCRETE_COLLECTION_PARAMETER"/>
+        <Method name="&lt;init&gt;"/>
+    </Match>
+
     <!--
         ChatMessage.requireNonNull is a precondition guard whose only
         meaningful state to report is the parameter name itself (the value

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

@@ -139,13 +139,11 @@ protected final <T extends JsonParameters> T withRaw(String key, String value) {
      */
     @SuppressWarnings("TypeParameterUnusedInFormals")
     protected final <T extends JsonParameters> T withScalar(String key, Object value) {
-        // String.valueOf(Float.NaN)/Infinity produce "NaN"/"Infinity", which are NOT valid JSON
-        // tokens and would make the whole request body unparseable by the native nlohmann parser.
-        // Reject at the source so the caller gets a clear error instead of an opaque downstream failure.
-        if (value instanceof Float && !Float.isFinite((Float) value)) {
-            throw new IllegalArgumentException(key + " must be a finite number but was " + value);
-        }
-        if (value instanceof Double && !Double.isFinite((Double) value)) {
+        // String.valueOf on a non-finite float/double yields "NaN"/"Infinity" — invalid JSON tokens
+        // the native nlohmann parser rejects. Reject at the source so the caller gets a clear error
+        // instead of an opaque downstream failure. Integer/Long are always finite (a no-op here);
+        // Boolean is not a Number and is skipped.
+        if (value instanceof Number && !Double.isFinite(((Number) value).doubleValue())) {
             throw new IllegalArgumentException(key + " must be a finite number but was " + value);
         }
         return withPut(key, String.valueOf(value));

src/main/java/net/ladenthin/llama/value/ChatMessage.java

@@ -186,7 +186,9 @@ public Optional<String> getToolCallId() {
      * @return the calls list, never {@code null}; empty when the message is not a tool-call turn
      */
     public List<ToolCall> getToolCalls() {
-        return toolCalls;
+        // Return a fresh unmodifiable view (mirrors getParts) so the stored list is never
+        // exposed directly — the caller cannot mutate this value type's internal state.
+        return Collections.unmodifiableList(toolCalls);
     }
 
     /**