From d48c61c2fd3b3d27a5e130e77879fa75411b2b79 Mon Sep 17 00:00:00 2001
From: Fabrizio Demaria <fdema@spotify.com>
Date: Fri, 29 May 2026 11:34:31 +0200
Subject: [PATCH] fix: fall back to call-site default for null variant
 properties
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A resolved flag property whose value is `null` was coerced to the type's
zero value (`false` / `0` / `0.0` / ""`) via `unwrap_or_default()` in
`into_value`, silently replacing the call-site default. A null property
means the variant defines no value, so resolution must fall back to the
call-site default — the same behaviour as a missing property, and what
every other Confidence SDK (and all local-resolve providers) does.

Skip null-valued fields during conversion so the downstream path lookup
misses and the default is served. Adds regression tests for a null
top-level property and a null nested-struct property.

Fixes #33

Co-authored-by: Cursor <cursoragent@cursor.com>
---
 confidence/src/models.rs | 101 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 99 insertions(+), 2 deletions(-)

diff --git a/confidence/src/models.rs b/confidence/src/models.rs
index 18c0f4d..462a6c5 100644
--- a/confidence/src/models.rs
+++ b/confidence/src/models.rs
@@ -81,7 +81,18 @@ impl FlagValueConversion<StructValue> for Option<Value> {
                     if let Value::Object(value_map) = value {
                         let new_map: HashMap<String, ConfidenceValue> = value_map
                             .into_iter()
-                            .map(|(key, value)| {
+                            .filter_map(|(key, value)| {
+                                // A null property carries no value: the variant doesn't define one,
+                                // so resolution must fall back to the call-site default (the same
+                                // behaviour as a missing property, and what every other Confidence
+                                // SDK does). Omitting the field here makes the downstream path
+                                // lookup miss, which surfaces the default. Without this, the
+                                // scalar arms below coerce null to the type's zero value via
+                                // `unwrap_or_default()` (false / 0 / 0.0 / ""), silently replacing
+                                // the call-site default.
+                                if value.is_null() {
+                                    return None;
+                                }
                                 let converted_value = match schema[&key].clone() {
                                     SchemaType::BoolType => ConfidenceValue::Bool(
                                         value.as_bool().unwrap_or_default(),
@@ -103,7 +114,7 @@ impl FlagValueConversion<StructValue> for Option<Value> {
                                         )))
                                     }
                                 };
-                                (key, converted_value)
+                                Some((key, converted_value))
                             })
                             .collect();
                         StructValue { fields: new_map }
@@ -194,3 +205,89 @@ impl From<reqwest::Error> for ResolveError {
         ResolveError::NetworkError(error)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn resolved_flags_with_nulls() -> ResolvedFlags {
+        let json = r#"
+        {
+          "resolvedFlags": [
+            {
+              "flag": "flags/test-flag",
+              "variant": "flags/test-flag/variants/treatment",
+              "value": {
+                "boolean-key": null,
+                "string-key": "served",
+                "struct-key": {
+                  "nested-null-key": null,
+                  "nested-boolean-key": true
+                }
+              },
+              "flagSchema": {
+                "schema": {
+                  "boolean-key": { "boolSchema": {} },
+                  "string-key": { "stringSchema": {} },
+                  "struct-key": {
+                    "structSchema": {
+                      "schema": {
+                        "nested-null-key": { "boolSchema": {} },
+                        "nested-boolean-key": { "boolSchema": {} }
+                      }
+                    }
+                  }
+                }
+              },
+              "reason": "RESOLVE_REASON_MATCH"
+            }
+          ],
+          "resolveToken": ""
+        }
+        "#;
+
+        let network: NetworkResolvedFlags = serde_json::from_str(json).unwrap();
+        network.into()
+    }
+
+    #[test]
+    fn null_valued_property_is_omitted_so_resolution_falls_back_to_default() {
+        let resolved = resolved_flags_with_nulls();
+        let fields = &resolved.flags[0].value.fields;
+
+        // A null property must be absent so the path lookup misses and the call-site
+        // default is served — not coerced to the type's zero value (false here).
+        assert!(
+            !fields.contains_key("boolean-key"),
+            "null property should be omitted, found {:?}",
+            fields.get("boolean-key")
+        );
+
+        // Non-null siblings are untouched.
+        assert_eq!(
+            fields.get("string-key"),
+            Some(&ConfidenceValue::String("served".to_string()))
+        );
+    }
+
+    #[test]
+    fn null_valued_nested_property_is_omitted() {
+        let resolved = resolved_flags_with_nulls();
+        let nested = resolved.flags[0]
+            .value
+            .fields
+            .get("struct-key")
+            .and_then(|v| v.as_struct())
+            .expect("struct-key should be present");
+
+        assert!(
+            !nested.fields.contains_key("nested-null-key"),
+            "null nested property should be omitted, found {:?}",
+            nested.fields.get("nested-null-key")
+        );
+        assert_eq!(
+            nested.fields.get("nested-boolean-key"),
+            Some(&ConfidenceValue::Bool(true))
+        );
+    }
+}