fix(flags): honor versioned local property matching

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

@@ -0,0 +1,5 @@
+---
+hex/posthog: patch
+---
+
+Honor the definitions snapshot property_matching_version in local feature flag evaluation and shared definition caches. Missing or version 1 now intentionally matches released service legacy boolean truthiness rather than the former SDK behavior; version 2 uses explicit equality, with recursive truthiness for empty filters. Correct known-null complements and composite equality normalization.

lib/posthog/feature_flags/definition_loader.ex

@@ -166,6 +166,7 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do
           flags_by_key: %{String.t() => map()},
           group_type_mapping: map(),
           cohorts: map(),
+          property_matching_version: term(),
           minimal_flag_called_events: boolean()
         }
 
@@ -534,6 +535,7 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do
 
     if is_list(flags) and is_map(mapping) and is_map(cohorts) do
       minimal = value(body, "minimal_flag_called_events") == true
+      property_matching_version = value(body, "property_matching_version")
 
       snapshot = %{
         flags: flags,
@@ -543,13 +545,15 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do
           end),
         group_type_mapping: mapping,
         cohorts: cohorts,
+        property_matching_version: property_matching_version,
         minimal_flag_called_events: minimal
       }
 
       complete = %{
         "flags" => flags,
         "group_type_mapping" => mapping,
         "cohorts" => cohorts,
+        "property_matching_version" => property_matching_version,
         "minimal_flag_called_events" => minimal
       }
 
@@ -571,6 +575,9 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do
   defp value(map, "minimal_flag_called_events"),
     do: Map.get(map, "minimal_flag_called_events", Map.get(map, :minimal_flag_called_events))
 
+  defp value(map, "property_matching_version"),
+    do: Map.get(map, "property_matching_version", Map.get(map, :property_matching_version, 1))
+
   defp value(map, "key"), do: Map.get(map, "key", Map.get(map, :key))
 
   defp response_etag(%{headers: headers}), do: header_value(headers, "etag")

lib/posthog/feature_flags/flag_definition_cache_provider.ex

@@ -6,6 +6,9 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProvider do
   `:flag_definition_cache_provider`. The definition loader bounds and isolates
   every callback. Cached values must be maps containing `flags`,
   `group_type_mapping`, and `cohorts` (string or atom keys are accepted).
+  Preserve the complete envelope, including `property_matching_version`, with
+  the definitions. Only version 2 selects explicit property matching; older
+  cached envelopes omitting the version use released service legacy matching.
 
   A minimal provider can coordinate fetching and keep the complete envelope in
   an application-owned cache:

lib/posthog/feature_flags/local_evaluator.ex

@@ -434,9 +434,19 @@ defmodule PostHog.FeatureFlags.LocalEvaluator do
        when is_map(property) do
     {result, cache} =
       case value(property, "type") do
-        "cohort" -> {match_cohort(property, property_values, definitions, context, cache), cache}
-        "flag" -> match_dependency(property, definitions, context, cache)
-        _ -> {match_property(property, property_values, context.now), cache}
+        "cohort" ->
+          {match_cohort(property, property_values, definitions, context, cache), cache}
+
+        "flag" ->
+          match_dependency(property, definitions, context, cache)
+
+        _ ->
+          {match_property(
+             property,
+             property_values,
+             context.now,
+             Map.get(definitions, :property_matching_version, 1)
+           ), cache}
       end
 
     {apply_negation(result, value(property, "negation") == true), cache}
@@ -601,7 +611,7 @@ defmodule PostHog.FeatureFlags.LocalEvaluator do
     end
   end
 
-  defp match_property(property, property_values, now) do
+  defp match_property(property, property_values, now, property_matching_version) do
     key = value(property, "key")
     operator = value(property, "operator") || "exact"
     filter_value = value(property, "value")
@@ -610,32 +620,23 @@ defmodule PostHog.FeatureFlags.LocalEvaluator do
       :error ->
         :inconclusive
 
+      {:ok, property_value} when operator in ["exact", "is_not"] ->
+        match = exact_match?(property_value, filter_value, property_matching_version)
+        boolean_result(if(operator == "exact", do: match, else: not match))
+
       {:ok, property_value} ->
         apply_operator(operator, property_value, filter_value, now)
     end
+  rescue
+    # Unsupported comparisons must not prevent later conditions from matching.
+    _exception -> :inconclusive
   end
 
   defp apply_operator("is_set", _property, _filter, _now), do: :match
   defp apply_operator("is_not_set", _property, _filter, _now), do: :no_match
 
-  defp apply_operator("is_not", nil, filter, _now) do
-    match = if is_list(filter), do: nil in filter, else: is_nil(filter)
-    boolean_result(not match)
-  end
-
   defp apply_operator(_operator, nil, _filter, _now), do: :no_match
 
-  defp apply_operator(operator, property, filter, _now) when operator in ["exact", "is_not"] do
-    match =
-      if is_list(filter) do
-        Enum.any?(filter, &case_insensitive_equal?(property, &1))
-      else
-        case_insensitive_equal?(property, filter)
-      end
-
-    boolean_result(if(operator == "exact", do: match, else: not match))
-  end
-
   defp apply_operator(operator, property, filter, _now)
        when operator in [
               "icontains",
@@ -775,8 +776,108 @@ defmodule PostHog.FeatureFlags.LocalEvaluator do
   defp boolean_result(true), do: :match
   defp boolean_result(false), do: :no_match
 
-  defp case_insensitive_equal?(left, right),
-    do: String.downcase(to_string(left)) == String.downcase(to_string(right))
+  defp exact_match?(property, filter, property_matching_version) do
+    cond do
+      # The service treats an empty filter as recursive ALL truthiness in both modes.
+      filter == [] ->
+        truthy?(property)
+
+      property_matching_version != 2 and boolean_like?(filter) ->
+        truthy?(property) == truthy?(filter)
+
+      is_list(filter) ->
+        prepared_property = prepare_exact_comparison(property)
+        Enum.any?(filter, &case_insensitive_equal?(prepared_property, &1))
+
+      true ->
+        case_insensitive_equal?(prepare_exact_comparison(property), filter)
+    end
+  end
+
+  defp boolean_like?(value) when is_boolean(value), do: true
+  defp boolean_like?(value) when is_binary(value), do: String.downcase(value) in ["true", "false"]
+  defp boolean_like?(value) when is_list(value), do: Enum.all?(value, &boolean_like?/1)
+  defp boolean_like?(_value), do: false
+
+  defp truthy?(value) when is_boolean(value), do: value
+  defp truthy?(value) when is_binary(value), do: String.downcase(value) == "true"
+  defp truthy?(value) when is_list(value), do: Enum.all?(value, &truthy?/1)
+
+  # Jason sends non-boolean atoms as strings, but a struct's encoder is opaque.
+  defp truthy?(value) when is_atom(value) and not is_nil(value),
+    do: value |> Atom.to_string() |> truthy?()
+
+  defp truthy?(value) when is_struct(value) do
+    raise ArgumentError, "opaque JSON struct truthiness"
+  end
+
+  defp truthy?(_value), do: false
+
+  defp prepare_exact_comparison(value) do
+    string = exact_string(value)
+    {String.downcase(string), composite_json?(value), String.contains?(string, "Σ")}
+  end
+
+  defp case_insensitive_equal?({left_string, left_composite?, left_sigma?}, right) do
+    {right_string, right_composite?, right_sigma?} = prepare_exact_comparison(right)
+
+    # Rust lowercases sigma contextually; String.downcase/1 does not. Keep newly
+    # supported composite comparisons inconclusive if either side could differ.
+    if (left_composite? or right_composite?) and (left_sigma? or right_sigma?) do
+      raise ArgumentError, "ambiguous composite Unicode casing"
+    end
+
+    left_string == right_string
+  end
+
+  defp composite_json?(value), do: is_list(value) or (is_map(value) and not is_struct(value))
+
+  defp exact_string(value) when is_struct(value), do: to_string(value)
+
+  # Lists must remain whole JSON values, not Elixir charlists; known null is not missing.
+  defp exact_string(value) when is_nil(value) or is_list(value) or is_map(value),
+    do: value |> sort_json_objects() |> Jason.encode!()
+
+  defp exact_string(value), do: to_string(value)
+
+  # Nested encoders can hide unsorted keys and ambiguous numbers from normalization.
+  # Top-level scalar structs use exact_string/1 instead. OrderedObjects generated
+  # below contain already-normalized members and are never passed back through here.
+  defp sort_json_objects(value) when is_struct(value) do
+    raise ArgumentError, "opaque composite JSON struct"
+  end
+
+  # Map iteration order is not JSON key order, including for mixed atom/string keys.
+  defp sort_json_objects(value) when is_map(value) and not is_struct(value) do
+    members =
+      Enum.map(value, fn {key, member} ->
+        # Jason uses atom names for keys, including nil as "nil", not "".
+        key = if is_atom(key), do: Atom.to_string(key), else: to_string(key)
+        {key, sort_json_objects(member)}
+      end)
+
+    # JSON parsing on the service drops duplicate keys, unlike Jason.OrderedObject.
+    if length(Enum.uniq_by(members, &elem(&1, 0))) != map_size(value) do
+      raise ArgumentError, "ambiguous composite JSON keys"
+    end
+
+    members
+    |> Enum.sort_by(&elem(&1, 0))
+    |> Jason.OrderedObject.new()
+  end
+
+  defp sort_json_objects(value) when is_list(value), do: Enum.map(value, &sort_json_objects/1)
+
+  # Jason and serde_json differ for floats and integers outside the service's i64/u64 range.
+  # Leave these composite comparisons inconclusive through the existing evaluation boundary.
+  defp sort_json_objects(value)
+       when is_float(value) or
+              (is_integer(value) and
+                 (value < -9_223_372_036_854_775_808 or value > 18_446_744_073_709_551_615)) do
+    raise ArgumentError, "ambiguous composite JSON number"
+  end
+
+  defp sort_json_objects(value), do: value
 
   defp ascii_downcase(value) do
     for <<character <- value>>, into: "" do

test/posthog/feature_flags/flag_definition_cache_provider_test.exs

@@ -71,6 +71,113 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do
     start_supervised!({PostHog.Supervisor, cfg})
   end
 
+  test "matching version round trips through providers and version-only hydration replaces results" do
+    owner = self()
+    # No API request stub: only the owned definitions GET below is permitted.
+    stub(PostHog.API.Mock, :client, fn _key, _host ->
+      %PostHog.API.Client{client: :stub_client, module: PostHog.API.Mock}
+    end)
+
+    property = %{"key" => "prop", "value" => false}
+    cohort = %{"type" => "AND", "values" => [%{"type" => "OR", "values" => [property]}]}
+
+    flags =
+      for {key, properties, extra} <- [
+            {"person", [property], %{}},
+            {"group", [property], %{"aggregation_group_type_index" => 0}},
+            {"cohort", [%{"type" => "cohort", "value" => 1}], %{}}
+          ] do
+        %{
+          "key" => key,
+          "active" => true,
+          "filters" => Map.merge(%{"groups" => [%{"properties" => properties}]}, extra)
+        }
+      end
+
+    wire =
+      envelope("unused")
+      |> Map.put("flags", flags)
+      |> Map.put("cohorts", %{"1" => cohort})
+      |> Map.put("property_matching_version", 2)
+
+    expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts ->
+      {:ok, %{status: 200, body: wire, headers: %{}}}
+    end)
+
+    {:ok, provider} =
+      Agent.start_link(fn ->
+        %{owner: owner, decision: true, read: nil, store: :ok, shutdown: :ok}
+      end)
+
+    start_instance(__MODULE__.VersionOwner, provider)
+    assert DefinitionLoader.ready?(__MODULE__.VersionOwner)
+    assert_receive {:stored, stored}
+    assert stored == wire
+
+    Agent.update(provider, &%{&1 | decision: false, read: stored})
+    name = __MODULE__.VersionReader
+    start_instance(name, provider)
+
+    context = %{
+      distinct_id: "user",
+      person_properties: %{prop: "banana"},
+      groups: %{organization: "org"},
+      group_properties: %{organization: %{prop: "banana"}},
+      only_evaluate_locally: true
+    }
+
+    assert {:ok, frozen} = PostHog.FeatureFlags.evaluate_flags(name, context)
+
+    assert Enum.all?(frozen.flags, fn {_key, result} ->
+             not result.enabled and result.locally_evaluated
+           end)
+
+    initial = DefinitionLoader.definitions(name)
+    assert initial.property_matching_version == 2
+
+    for read <- [nil, {:raise, "unavailable"}, %{"flags" => []}] do
+      Agent.update(provider, &%{&1 | read: read})
+      capture_log(fn -> DefinitionLoader.refresh(name) end)
+      assert DefinitionLoader.definitions(name) == initial
+    end
+
+    for version <- [1, 2, 1, 2, :missing] do
+      # Atom-key cache documents are supported alongside JSON string keys.
+      cached = %{
+        flags: flags,
+        cohorts: %{"1" => cohort},
+        group_type_mapping: %{"0" => "organization"},
+        minimal_flag_called_events: true
+      }
+
+      cached =
+        if version == :missing,
+          do: cached,
+          else: Map.put(cached, :property_matching_version, version)
+
+      Agent.update(provider, &%{&1 | read: cached})
+      assert :ok = DefinitionLoader.refresh(name)
+      current = DefinitionLoader.definitions(name)
+      assert current.flags == initial.flags
+      assert current.cohorts == initial.cohorts
+      assert current.group_type_mapping == initial.group_type_mapping
+      assert current.property_matching_version == if(version == :missing, do: 1, else: version)
+      assert {:ok, result} = PostHog.FeatureFlags.evaluate_flags(name, context)
+      assert map_size(result.flags) == 3
+
+      for {_key, flag} <- result.flags do
+        assert flag.enabled == (version != 2)
+        assert flag.locally_evaluated
+      end
+
+      assert Enum.all?(frozen.flags, fn {_key, result} -> not result.enabled end)
+    end
+
+    assert DefinitionLoader.definitions(__MODULE__.VersionOwner).property_matching_version == 2
+    stop_supervised(name)
+    stop_supervised(__MODULE__.VersionOwner)
+  end
+
   test "negative decision reads complete cached definitions without an API request" do
     stub_with(PostHog.API.Mock, PostHog.API.Stub)