feat: extend OpenFeature numeric, payload, and reason support

.sampo/changesets/roguish-knight-louhi.md

@@ -0,0 +1,5 @@
+---
+hex/posthog: patch
+---
+
+Support hexadecimal variants, JSON arrays, and reason metadata in the OpenFeature provider.

lib/posthog/open_feature.ex

@@ -44,15 +44,23 @@ if Code.ensure_loaded?(OpenFeature.Provider) do
 
     - Boolean flags resolve to whether the flag is enabled.
     - String flags resolve to the variant key.
-    - Number flags resolve to the variant key parsed as a number.
-    - Map flags resolve to the flag's JSON payload.
-
-    Reading a disabled flag returns the default value with reason `:default`,
-    unless the flag still carries a variant (or payload, for maps). That value
-    is then returned with reason `:default`, as in the Node and Python
-    providers. Reading an enabled flag whose value doesn't fit the requested
-    type returns the default value with `error_code: :type_mismatch`. Unknown
-    flags return `:flag_not_found`.
+    - Number flags resolve to the variant key parsed as a number, including
+      unsigned hexadecimal integers such as `0x10`.
+    - Map flags resolve to the flag's JSON object or array payload. The upstream
+      OpenFeature SDK requires a map default (`%{}`), even for array payloads;
+      `get_map_value/4` can return a list when the payload is an array.
+
+    For string, number, and map reads, a disabled flag returns the default value,
+    unless it still carries a variant (or object/array payload, for maps). That
+    value is then returned, as in the Node and Python providers. Boolean reads
+    return the flag's enabled state, including `false` regardless of the default.
+    Reading an enabled flag whose value doesn't fit the requested type returns
+    the default value with `error_code: :type_mismatch`. Unknown flags return
+    `:flag_not_found`.
+
+    Resolution details include `flag_metadata["posthog_reason"]` when PostHog
+    supplies an evaluation reason. Off results use `:disabled` when the server
+    explicitly reports the `flag_disabled` reason code, otherwise `:default`.
 
     The OpenFeature Elixir SDK has no error tuple for `:type_mismatch` or
     `:targeting_key_missing`, so these are returned as resolution details with
@@ -106,10 +114,10 @@ if Code.ensure_loaded?(OpenFeature.Provider) do
       with {:ok, %Result{} = result} <- evaluate(provider, key, default, context) do
         case result do
           %Result{variant: nil, enabled: false} ->
-            {:ok, default_details(default)}
+            {:ok, default_details(result, default)}
 
           %Result{variant: nil} ->
-            {:ok, type_mismatch(default, "Flag '#{key}' has no string variant.")}
+            {:ok, type_mismatch(result, default, "Flag '#{key}' has no string variant.")}
 
           %Result{variant: variant} ->
             {:ok, details(result, variant)}
@@ -122,29 +130,32 @@ if Code.ensure_loaded?(OpenFeature.Provider) do
       with {:ok, %Result{} = result} <- evaluate(provider, key, default, context) do
         case result do
           %Result{variant: nil, enabled: false} ->
-            {:ok, default_details(default)}
+            {:ok, default_details(result, default)}
 
           %Result{variant: nil} ->
-            {:ok, type_mismatch(default, "Flag '#{key}' has no numeric variant.")}
+            {:ok, type_mismatch(result, default, "Flag '#{key}' has no numeric variant.")}
 
           %Result{variant: variant} ->
             {:ok, number_details(result, key, variant, default)}
         end
       end
     end
 
+    # open-feature/elixir-sdk 0.1.3 guards get_map_value/get_map_details defaults
+    # with is_map/1 before calling the provider, but does not restrict returned
+    # values. Array payloads work with a map default; list defaults need an upstream fix.
     @impl true
     def resolve_map_value(provider, key, default, context) do
       with {:ok, %Result{} = result} <- evaluate(provider, key, default, context) do
         case result do
-          %Result{payload: payload} when is_map(payload) ->
+          %Result{payload: payload} when is_map(payload) or is_list(payload) ->
             {:ok, details(result, payload)}
 
           %Result{enabled: false} ->
-            {:ok, default_details(default)}
+            {:ok, default_details(result, default)}
 
           %Result{} ->
-            {:ok, type_mismatch(default, "Flag '#{key}' has no object/JSON payload.")}
+            {:ok, type_mismatch(result, default, "Flag '#{key}' has no object/JSON payload.")}
         end
       end
     end
@@ -210,21 +221,30 @@ if Code.ensure_loaded?(OpenFeature.Provider) do
 
     defp number_details(result, key, variant, default) do
       case parse_number(variant) do
-        {:ok, number} -> details(result, number)
-        :error -> type_mismatch(default, "Flag '#{key}' variant '#{variant}' is not a number.")
+        {:ok, number} ->
+          details(result, number)
+
+        :error ->
+          type_mismatch(result, default, "Flag '#{key}' variant '#{variant}' is not a number.")
       end
     end
 
     defp parse_number(variant) do
       trimmed = String.trim(variant)
 
-      if trimmed =~ ~r/\d/ do
-        case Integer.parse(trimmed) do
-          {integer, ""} -> {:ok, integer}
-          _ -> parse_float(trimmed)
-        end
-      else
-        :error
+      cond do
+        trimmed =~ ~r/\A0[xX][0-9a-fA-F]+\z/ ->
+          <<_prefix::binary-size(2), digits::binary>> = trimmed
+          {:ok, String.to_integer(digits, 16)}
+
+        trimmed =~ ~r/\d/ ->
+          case Integer.parse(trimmed) do
+            {integer, ""} -> {:ok, integer}
+            _ -> parse_float(trimmed)
+          end
+
+        true ->
+          :error
       end
     end
 
@@ -246,13 +266,32 @@ if Code.ensure_loaded?(OpenFeature.Provider) do
       %ResolutionDetails{
         value: value,
         variant: result.variant,
-        reason: if(result.enabled, do: :targeting_match, else: :default)
+        reason: resolution_reason(result),
+        flag_metadata: reason_metadata(result.reason)
       }
     end
 
-    defp default_details(default), do: %ResolutionDetails{value: default, reason: :default}
+    defp resolution_reason(%Result{enabled: true}), do: :targeting_match
+    defp resolution_reason(%Result{reason: %{"code" => "flag_disabled"}}), do: :disabled
+    defp resolution_reason(_result), do: :default
 
-    defp type_mismatch(default, message), do: error_details(default, :type_mismatch, message)
+    defp reason_metadata(%{"description" => description}) when is_binary(description),
+      do: %{"posthog_reason" => description}
+
+    defp reason_metadata(%{"code" => code}) when is_binary(code),
+      do: %{"posthog_reason" => code}
+
+    defp reason_metadata(reason) when is_binary(reason), do: %{"posthog_reason" => reason}
+    defp reason_metadata(_reason), do: %{}
+
+    defp default_details(result, default), do: %{details(result, default) | variant: nil}
+
+    defp type_mismatch(result, default, message) do
+      %{
+        error_details(default, :type_mismatch, message)
+        | flag_metadata: reason_metadata(result.reason)
+      }
+    end
 
     defp error_details(default, code, message) do
       %ResolutionDetails{value: default, reason: :error, error_code: code, error_message: message}

test/posthog/open_feature_test.exs

@@ -100,7 +100,11 @@ defmodule PostHog.OpenFeature.ProviderTest do
           {"-1", -1},
           {".5", 0.5},
           {"-.5", -0.5},
-          {"1.", 1.0}
+          {"1.", 1.0},
+          {"0x10", 16},
+          {"0Xff", 255},
+          {" 0xA0 ", 160},
+          {"0x0", 0}
         ] do
       test "parses variant #{inspect(variant)}" do
         expect_flag("flag", %{"variant" => unquote(variant)})
@@ -112,7 +116,20 @@ defmodule PostHog.OpenFeature.ProviderTest do
       end
     end
 
-    for variant <- ["abc", "", "  ", "12abc", ".", "-."] do
+    for variant <- [
+          "abc",
+          "",
+          "  ",
+          "12abc",
+          ".",
+          "-.",
+          "0x",
+          "0xgg",
+          "0x1.5",
+          "0x_10",
+          "-0x10",
+          "+0x10"
+        ] do
       test "returns type_mismatch for variant #{inspect(variant)}" do
         expect_flag("flag", %{"variant" => unquote(variant)})
 
@@ -172,14 +189,158 @@ defmodule PostHog.OpenFeature.ProviderTest do
                Provider.resolve_map_value(provider(), "flag", %{}, @context)
     end
 
-    test "returns type_mismatch for a non-object payload" do
-      expect_flag("flag", %{"metadata" => %{"payload" => "[1, 2]"}})
+    for payload <- [[], [1, 2], [%{"nested" => [true, nil]}]] do
+      test "resolves array payload #{inspect(payload)}" do
+        payload = unquote(Macro.escape(payload))
+        expect_flag("flag", %{"metadata" => %{"payload" => Jason.encode!(payload)}})
+
+        assert {:ok, %ResolutionDetails{value: ^payload, reason: :targeting_match}} =
+                 Provider.resolve_map_value(provider(), "flag", %{}, @context)
+      end
+    end
+
+    test "preserves array payloads on disabled flags" do
+      expect_flag("flag", %{"enabled" => false, "metadata" => %{"payload" => "[]"}})
+
+      assert {:ok, %ResolutionDetails{value: [], reason: :default}} =
+               Provider.resolve_map_value(provider(), "flag", %{}, @context)
+    end
+
+    test "returns type_mismatch for a scalar payload" do
+      expect_flag("flag", %{"metadata" => %{"payload" => "123"}})
 
       assert {:ok, %ResolutionDetails{value: %{}, error_code: :type_mismatch}} =
                Provider.resolve_map_value(provider(), "flag", %{}, @context)
     end
   end
 
+  describe "reason metadata" do
+    for {resolver, default, attrs} <- [
+          {:resolve_boolean_value, false, %{}},
+          {:resolve_string_value, "fallback", %{"variant" => "test"}},
+          {:resolve_number_value, 0, %{"variant" => "42"}},
+          {:resolve_map_value, %{}, %{"metadata" => %{"payload" => "{}"}}}
+        ] do
+      test "preserves PostHog reason for #{resolver}" do
+        expect_flag(
+          "flag",
+          Map.put(unquote(Macro.escape(attrs)), "reason", %{
+            "code" => "condition_match",
+            "description" => "Matched condition set 1"
+          })
+        )
+
+        assert {:ok,
+                %ResolutionDetails{
+                  reason: :targeting_match,
+                  flag_metadata: %{"posthog_reason" => "Matched condition set 1"}
+                }} =
+                 apply(Provider, unquote(resolver), [
+                   provider(),
+                   "flag",
+                   unquote(Macro.escape(default)),
+                   @context
+                 ])
+      end
+
+      test "reports explicit disabled reason for #{resolver}" do
+        expect_flag("flag", %{
+          "enabled" => false,
+          "reason" => %{
+            "code" => "flag_disabled",
+            "description" => "Flag switched off"
+          }
+        })
+
+        assert {:ok,
+                %ResolutionDetails{
+                  reason: :disabled,
+                  flag_metadata: %{"posthog_reason" => "Flag switched off"}
+                }} =
+                 apply(Provider, unquote(resolver), [
+                   provider(),
+                   "flag",
+                   unquote(Macro.escape(default)),
+                   @context
+                 ])
+      end
+    end
+
+    for {label, resolver, default, attrs} <- [
+          {"missing string variant", :resolve_string_value, "fallback", %{}},
+          {"missing numeric variant", :resolve_number_value, 0, %{}},
+          {"invalid numeric variant", :resolve_number_value, 0, %{"variant" => "abc"}},
+          {"missing payload", :resolve_map_value, %{}, %{}},
+          {"scalar payload", :resolve_map_value, %{}, %{"metadata" => %{"payload" => "123"}}}
+        ] do
+      test "preserves reason metadata on type mismatch for #{label}" do
+        expect_flag(
+          "flag",
+          Map.put(unquote(Macro.escape(attrs)), "reason", %{
+            "code" => "condition_match",
+            "description" => "Matched condition set 1"
+          })
+        )
+
+        assert {:ok,
+                %ResolutionDetails{
+                  value: unquote(Macro.escape(default)),
+                  reason: :error,
+                  error_code: :type_mismatch,
+                  flag_metadata: %{"posthog_reason" => "Matched condition set 1"}
+                }} =
+                 apply(Provider, unquote(resolver), [
+                   provider(),
+                   "flag",
+                   unquote(Macro.escape(default)),
+                   @context
+                 ])
+      end
+    end
+
+    test "does not infer disabled from the description" do
+      expect_flag("flag", %{
+        "enabled" => false,
+        "reason" => %{
+          "code" => "no_condition_match",
+          "description" => "User property disabled is true"
+        }
+      })
+
+      assert {:ok, %ResolutionDetails{reason: :default}} =
+               Provider.resolve_boolean_value(provider(), "flag", true, @context)
+    end
+
+    test "falls back to the reason code for metadata" do
+      expect_flag("flag", %{"enabled" => false, "reason" => %{"code" => "flag_disabled"}})
+
+      assert {:ok,
+              %ResolutionDetails{
+                reason: :disabled,
+                flag_metadata: %{"posthog_reason" => "flag_disabled"}
+              }} =
+               Provider.resolve_boolean_value(provider(), "flag", true, @context)
+    end
+
+    test "preserves string reasons used by local evaluation" do
+      expect_flag("flag", %{"reason" => "Evaluated locally"})
+
+      assert {:ok,
+              %ResolutionDetails{
+                reason: :targeting_match,
+                flag_metadata: %{"posthog_reason" => "Evaluated locally"}
+              }} =
+               Provider.resolve_boolean_value(provider(), "flag", false, @context)
+    end
+
+    test "omits metadata when no reason is available" do
+      expect_flag("flag", %{})
+
+      assert {:ok, %ResolutionDetails{flag_metadata: %{}}} =
+               Provider.resolve_boolean_value(provider(), "flag", false, @context)
+    end
+  end
+
   describe "errors" do
     test "returns flag_not_found when the flag is not in the response" do
       expect_flags(%{})
@@ -325,6 +486,18 @@ defmodule PostHog.OpenFeature.ProviderTest do
                "test"
     end
 
+    test "resolves array payloads with a map default", %{client: client} do
+      expect_flag("flag", %{"metadata" => %{"payload" => "[1, 2]"}})
+      assert OpenFeature.Client.get_map_value(client, "flag", %{}, context: @context) == [1, 2]
+    end
+
+    test "surfaces disabled reason and metadata", %{client: client} do
+      expect_flag("flag", %{"enabled" => false, "reason" => %{"code" => "flag_disabled"}})
+
+      assert %{reason: :disabled, flag_metadata: %{"posthog_reason" => "flag_disabled"}} =
+               OpenFeature.Client.get_boolean_details(client, "flag", true, context: @context)
+    end
+
     test "surfaces type_mismatch", %{client: client} do
       expect_flag("flag", %{"enabled" => true})