fix(types): restore LLMProvider source compatibility for the 7-arg primitive shape (#151)

saurabhjain1592 · web-flow · commit cbe13d23c8ac · 2026-04-28T18:49:58.000+02:00
Code-review finding on PR #148: the new 13-arg boxed constructor replaced the public 7-arg primitive shape, and getPriority() / getWeight() changed from `int` to `Integer`. That's a real breaking change in a PR framed as additive — pre-existing callers either fail to compile (constructor) or now see nullable return values where they expected primitives (accessors). This PR preserves source compatibility while keeping the boxed storage that the wire-shape contract needs: 1. Add a 7-arg primitive constructor that delegates to the new 13-arg one with nulls for the post-PR-#148 optional fields. Marked @deprecated to nudge new callers toward the boxed form. 2. getPriority() and getWeight() return primitive `int` again (null-safe-unbox to 0). Boxed access is via the new getPriorityBoxed() / getWeightBoxed() methods. 3. getEnabled() (which was BRAND NEW in PR #148, returning Boolean) is renamed to getEnabledBoxed() so the JavaBeans-style name doesn't lure callers into `boolean e = p.getEnabled()` and an NPE on null. Pre-PR-#148 had no getEnabled() so this rename has no consumers. 4. Same Boxed-suffix pattern applied to getHasApiKey → getHasApiKeyBoxed for symmetry; primitive `boolean hasApiKey()` was already there from before #148. Two new regression tests pin the source-compat shape: - llmProviderLegacyConstructorPreservesSourceCompat — exercises the 7-arg primitive constructor and confirms primitive-returning accessors round-trip correctly. - llmProviderPrimitiveAccessorsNullSafe — confirms primitive accessors null-safe-unbox to 0 / false when the boxed field is null (which is what Jackson produces when the JSON field is omitted). Existing test that asserted on `p.getEnabled()` updated to `p.getEnabledBoxed()` to match the new naming. Wire-shape baseline refreshed — the renamed getter is not part of the wire contract (Jackson reads via constructor), but the validator records SDK class shape and noticed the rename. No spec drift.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - **`pom.xml`** — `mvn verify -DskipUnitTests=true` now actually skips surefire (unit tests). Previously the property was unbound — `-DskipUnitTests` was a no-op flag and unit tests ran redundantly during integration-test invocations. The flag now binds to the surefire `<skipTests>` config; default remains `false`.
+- **`LLMProvider` source compatibility** — review-driven follow-up to PR #148. The original PR replaced the public 7-arg primitive constructor (`LLMProvider(name, type, enabled:bool, priority:int, weight:int, hasApiKey:bool, health)`) with a 13-arg boxed constructor and changed `getPriority()` / `getWeight()` from `int` to `Integer`. Pre-existing callers either failed to compile or started seeing nullable return values in what was framed as an additive change. **Restored:** the 7-arg primitive constructor (delegates to the 13-arg one with nulls for the post-PR-#148 optional fields, marked `@Deprecated` to point new callers at the boxed form), and primitive-returning `getPriority()` / `getWeight()` (null-safe-unboxes to 0). Boxed accessors remain available as `getPriorityBoxed()` / `getWeightBoxed()` / `getEnabledBoxed()` / `getHasApiKeyBoxed()` for callers that need to distinguish "explicitly 0" from "field not present". The boxed `getEnabled()` from PR #148 is renamed to `getEnabledBoxed()` (was a brand-new method in #148 with no consumers, safe to rename pre-tag).
 
 ## [6.1.0] - 2026-04-25 — Plugin Batch 1 explainability fields on MCP responses
 
diff --git a/src/main/java/com/getaxonflow/sdk/types/LLMProvider.java b/src/main/java/com/getaxonflow/sdk/types/LLMProvider.java
@@ -15,6 +15,7 @@
  */
 package com.getaxonflow.sdk.types;
 
+import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import java.util.Map;
@@ -26,10 +27,19 @@
  * populated when the provider config has them set; {@code settings} is a free-form
  * provider-specific map.
  *
- * <p>{@code enabled} and {@code hasApiKey} are typed as {@link Boolean} (boxed) so a
- * missing or {@code null} value in the JSON response is distinguishable from the
- * explicit boolean values — primitive {@code boolean} would silently default to
- * {@code false} and mask whether the field was actually emitted.
+ * <p><b>Source-compatibility note.</b> Pre-PR-#148 callers wrote {@code new LLMProvider(
+ * name, type, true, 0, 0, true, health)} (7 args, primitive booleans/ints) and called
+ * {@code int p = provider.getPriority()} / {@code int w = provider.getWeight()}. The
+ * 7-arg primitive constructor and the primitive-returning {@code getPriority()} /
+ * {@code getWeight()} accessors are preserved as a compatibility shim. The 13-arg
+ * boxed constructor is the Jackson entry point; new optional fields default to null
+ * via the legacy constructor.
+ *
+ * <p>Internal storage is boxed ({@link Boolean} / {@link Integer}) so the SDK can
+ * faithfully represent fields that were omitted by an older platform. New methods
+ * exposing the boxed values directly are suffixed with {@code Boxed} (e.g.
+ * {@link #getPriorityBoxed()}) for callers that need to distinguish "explicitly 0"
+ * from "field not present".
  */
 @JsonIgnoreProperties(ignoreUnknown = true)
 public final class LLMProvider {
@@ -48,6 +58,11 @@ public final class LLMProvider {
   private final Integer timeoutSeconds;
   private final Map<String, Object> settings;
 
+  /**
+   * Full constructor used by Jackson — accepts boxed types so a missing field in the
+   * JSON response stays null instead of silently becoming {@code false} / {@code 0}.
+   */
+  @JsonCreator
   public LLMProvider(
       @JsonProperty("name") String name,
       @JsonProperty("type") String type,
@@ -77,6 +92,36 @@ public LLMProvider(
     this.settings = settings;
   }
 
+  /**
+   * Pre-PR-#148 constructor signature — 7 args, primitive {@code boolean} /
+   * {@code int}. Preserved as a compatibility shim so callers that constructed
+   * {@code LLMProvider} directly continue to compile. Delegates to the full
+   * 13-arg constructor with null for the post-PR-#148 optional fields.
+   *
+   * @deprecated Prefer the 13-arg constructor when constructing programmatically;
+   *     this overload exists only to preserve compile-time source compatibility for
+   *     pre-existing call sites.
+   */
+  @Deprecated
+  public LLMProvider(
+      String name,
+      String type,
+      boolean enabled,
+      int priority,
+      int weight,
+      boolean hasApiKey,
+      LLMProviderHealth health) {
+    this(
+        name,
+        type,
+        Boolean.valueOf(enabled),
+        Integer.valueOf(priority),
+        Integer.valueOf(weight),
+        Boolean.valueOf(hasApiKey),
+        health,
+        null, null, null, null, null, null);
+  }
+
   public String getName() {
     return name;
   }
@@ -85,36 +130,75 @@ public String getType() {
     return type;
   }
 
-  /** May be null if the platform omitted the field. */
-  public Boolean getEnabled() {
+  /**
+   * Convenience: returns true if the {@code enabled} field was explicitly set to
+   * true; false otherwise (including when the field was omitted by the platform).
+   * Mirrors the pre-PR-#148 primitive-returning accessor.
+   */
+  public boolean isEnabled() {
+    return Boolean.TRUE.equals(enabled);
+  }
+
+  /**
+   * Returns the raw boxed {@code enabled} value. May be null if the platform
+   * omitted the field — use this when you need to distinguish "explicitly false"
+   * from "not set".
+   */
+  public Boolean getEnabledBoxed() {
     return enabled;
   }
 
-  /** Convenience: true if explicitly enabled, false otherwise (including null). */
-  public boolean isEnabled() {
-    return Boolean.TRUE.equals(enabled);
+  /**
+   * Convenience: returns the {@code priority} field as a primitive {@code int};
+   * returns 0 when the field was omitted. Mirrors the pre-PR-#148 primitive-
+   * returning accessor.
+   */
+  public int getPriority() {
+    return priority != null ? priority : 0;
   }
 
-  /** May be null if the platform omitted the field. */
-  public Integer getPriority() {
+  /**
+   * Returns the raw boxed {@code priority}; null when the platform omitted the
+   * field — use this when you need to distinguish "explicitly 0" from "not set".
+   */
+  public Integer getPriorityBoxed() {
     return priority;
   }
 
-  /** May be null if the platform omitted the field. */
-  public Integer getWeight() {
-    return weight;
+  /**
+   * Convenience: returns the {@code weight} field as a primitive {@code int};
+   * returns 0 when the field was omitted. Mirrors the pre-PR-#148 primitive-
+   * returning accessor.
+   */
+  public int getWeight() {
+    return weight != null ? weight : 0;
   }
 
-  /** May be null if the platform omitted the field. */
-  public Boolean getHasApiKey() {
-    return hasApiKey;
+  /**
+   * Returns the raw boxed {@code weight}; null when the platform omitted the
+   * field — use this when you need to distinguish "explicitly 0" from "not set".
+   */
+  public Integer getWeightBoxed() {
+    return weight;
   }
 
-  /** Convenience: true if has_api_key is explicitly true, false otherwise (including null). */
+  /**
+   * Convenience: returns true if {@code has_api_key} was explicitly set to true;
+   * false otherwise (including when the field was omitted). Mirrors the pre-
+   * PR-#148 primitive-returning accessor.
+   */
   public boolean hasApiKey() {
     return Boolean.TRUE.equals(hasApiKey);
   }
 
+  /**
+   * Returns the raw boxed {@code has_api_key}; null when the platform omitted the
+   * field — use this when you need to distinguish "explicitly false" from "not set".
+   */
+  public Boolean getHasApiKeyBoxed() {
+    return hasApiKey;
+  }
+
   /** Health snapshot; may be null if the platform did not return a health probe. */
   public LLMProviderHealth getHealth() {
     return health;
diff --git a/src/test/java/com/getaxonflow/sdk/AxonFlowTest.java b/src/test/java/com/getaxonflow/sdk/AxonFlowTest.java
@@ -801,10 +801,65 @@ void llmProviderEnabledIsBoxed() {
     List<LLMProvider> providers = axonflow.listLLMProviders();
     assertThat(providers).hasSize(1);
     LLMProvider p = providers.get(0);
-    assertThat(p.getEnabled()).isNull();
+    // The boxed accessor distinguishes "field not present" from "explicitly false";
+    // the convenience accessor returns false for both.
+    assertThat(p.getEnabledBoxed()).isNull();
     assertThat(p.isEnabled()).isFalse();
   }
 
+  @Test
+  @DisplayName("LLMProvider preserves pre-PR-#148 7-arg primitive constructor for source compat")
+  @SuppressWarnings("deprecation")
+  void llmProviderLegacyConstructorPreservesSourceCompat() {
+    // Pre-PR-#148 callers wrote `new LLMProvider(name, type, true, 1, 2, true, null)`
+    // with primitive booleans/ints. The 7-arg overload preserves that signature so
+    // those call sites continue to compile after the boxed-types change.
+    LLMProvider p = new LLMProvider("anthropic", "anthropic", true, 1, 2, true, null);
+
+    assertThat(p.getName()).isEqualTo("anthropic");
+    assertThat(p.getType()).isEqualTo("anthropic");
+    assertThat(p.isEnabled()).isTrue();
+    assertThat(p.hasApiKey()).isTrue();
+    // Primitive accessors return the unboxed value.
+    assertThat(p.getPriority()).isEqualTo(1);
+    assertThat(p.getWeight()).isEqualTo(2);
+    // Boxed accessors expose the same value, also non-null when set via the
+    // primitive constructor.
+    assertThat(p.getPriorityBoxed()).isEqualTo(1);
+    assertThat(p.getWeightBoxed()).isEqualTo(2);
+    assertThat(p.getEnabledBoxed()).isTrue();
+    assertThat(p.getHasApiKeyBoxed()).isTrue();
+    // Post-PR-#148 fields default to null when constructed via the legacy overload.
+    assertThat(p.getEndpoint()).isNull();
+    assertThat(p.getModel()).isNull();
+    assertThat(p.getRegion()).isNull();
+    assertThat(p.getRateLimit()).isNull();
+    assertThat(p.getTimeoutSeconds()).isNull();
+    assertThat(p.getSettings()).isNull();
+    assertThat(p.getHealth()).isNull();
+  }
+
+  @Test
+  @DisplayName("LLMProvider primitive accessors return 0/false when boxed field is null")
+  void llmProviderPrimitiveAccessorsNullSafe() {
+    // Construct via the boxed constructor with explicit nulls — Jackson's
+    // omit-field path produces this same state.
+    LLMProvider p = new LLMProvider(
+        "x", "openai", null, null, null, null, null,
+        null, null, null, null, null, null);
+
+    // Primitive accessors null-safe-unbox to 0 / false; boxed accessors expose
+    // the actual null so callers can distinguish "explicitly 0" from "not set".
+    assertThat(p.getPriority()).isEqualTo(0);
+    assertThat(p.getWeight()).isEqualTo(0);
+    assertThat(p.isEnabled()).isFalse();
+    assertThat(p.hasApiKey()).isFalse();
+    assertThat(p.getPriorityBoxed()).isNull();
+    assertThat(p.getWeightBoxed()).isNull();
+    assertThat(p.getEnabledBoxed()).isNull();
+    assertThat(p.getHasApiKeyBoxed()).isNull();
+  }
+
   // ========================================================================
   // MCP Connectors
   // ========================================================================
