Skip to content

fix(flags): compare numeric string properties numerically #310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dustinbyrne merged 1 commit into from
Oct 20, 2026
Merged

fix(flags): compare numeric string properties numerically #310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-numeric-string-ordering.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"posthog-ruby": patch
---

Compare numeric strings numerically for locally evaluated feature flag `gt`, `gte`, `lt`, and `lte` conditions, including when the filter is a JSON number. This corrects flag membership for values such as `"10" > 2` and `"100" < 30`. Invalid or non-finite property values no longer match via string ordering; invalid numeric conditions use the existing remote-evaluation fallback.
36 changes: 22 additions & 14 deletions lib/posthog/feature_flags.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,21 @@ def self.compare(lhs, rhs, operator)
end
end

def self.parse_finite_number(value)
case value
when String
return unless /\A[+-]?([0-9]+(\.[0-9]*)?|\.[0-9]+)([eE][+-]?[0-9]+)?\z/.match?(value)
when Integer, Float
# JSON numbers remain supported alongside numeric strings.
else
return unless defined?(::BigDecimal) && value.is_a?(::BigDecimal)
end

number = value.to_f
number if number.finite?
end
private_class_method :parse_finite_number

def self.relative_date_parse_for_feature_flag_matching(value)
match = /^-?([0-9]+)([a-z])$/.match(value)
parsed_dt = DateTime.now.new_offset(0)
Expand Down Expand Up @@ -734,20 +749,13 @@ def self.match_property(property, property_values, cohort_properties = {}, prope
when 'not_regex'
PostHog::Utils.is_valid_regex(value.to_s) && Regexp.new(value.to_s).match(override_value.to_s).nil?
when 'gt', 'gte', 'lt', 'lte'
parsed_value = nil
begin
parsed_value = Float(value)
rescue StandardError # rubocop:disable Lint/SuppressedException
end
if !parsed_value.nil? && !override_value.nil?
if override_value.is_a?(String)
compare(override_value, value.to_s, operator)
else
compare(override_value, parsed_value, operator)
end
else
compare(override_value.to_s, value.to_s, operator)
end
parsed_value = parse_finite_number(value)
raise InconclusiveMatchError, 'Invalid numeric condition' if parsed_value.nil?

parsed_override = parse_finite_number(override_value)
return false if parsed_override.nil?

compare(parsed_override, parsed_value, operator)
when 'is_date_before', 'is_date_after'
override_date = PostHog::Utils.convert_to_datetime(override_value.to_s)
parsed_date = relative_date_parse_for_feature_flag_matching(value.to_s)
Expand Down
173 changes: 165 additions & 8 deletions spec/posthog/feature_flag_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require 'spec_helper'
require 'bigdecimal'
require 'timecop'

module PostHog
Expand All @@ -9,6 +10,68 @@ module PostHog
flags_endpoint = 'https://us.i.posthog.com/flags/?v=2'

describe 'local evaluation' do
describe 'numeric ordering' do
[
{ operator: 'gt', filter: 2, properties: { score: '10' }, expected: true },
{ operator: 'lt', filter: 30, properties: { score: '100' }, expected: false },
{ operator: 'gte', filter: '5', properties: { score: '10' }, expected: true },
{ operator: 'lte', filter: '5', properties: { score: '10' }, expected: false },
{ operator: 'gt', filter: '2', properties: { score: BigDecimal('10') }, expected: true },
{ operator: 'gte', filter: '10', properties: { score: BigDecimal('10') }, expected: true },
{ operator: 'lt', filter: '30', properties: { score: BigDecimal('10') }, expected: true },
{ operator: 'lte', filter: '10', properties: { score: BigDecimal('10') }, expected: true },
{
operator: 'lte', filter: '9007199254740992',
properties: { score: BigDecimal('9007199254740993') }, expected: true
},
{ operator: 'gte', filter: '0', properties: { score: BigDecimal('1e-400') }, expected: true },
{ operator: 'lt', filter: '5', properties: { score: BigDecimal('NaN') }, expected: false },
{ operator: 'gt', filter: '5', properties: { score: BigDecimal('Infinity') }, expected: false },
{ operator: 'lt', filter: '5', properties: { score: BigDecimal('-Infinity') }, expected: false },
{ operator: 'gt', filter: '5', properties: { score: BigDecimal('1e400') }, expected: false },
{ operator: 'gt', filter: '5', properties: { score: '10px' }, expected: false },
{ operator: 'lt', filter: 5, properties: { score: nil }, expected: false },
{ operator: 'gt', filter: '5px', properties: { score: '10' }, expected: 'server-fallback' },
{ operator: 'gt', filter: '5px', properties: { score: nil }, expected: 'server-fallback' },
{ operator: 'gt', filter: '5', properties: {}, expected: 'server-fallback' }
].each do |test_case|
it "evaluates #{test_case.inspect} with the expected fallback boundary" do
flag = {
key: 'numeric-flag', active: true,
filters: {
groups: [{
properties: [{
key: 'score', type: 'person', operator: test_case[:operator], value: test_case[:filter]
}],
rollout_percentage: 100
}]
}
}
stub_request(:get, 'https://us.i.posthog.com/flags/definitions?token=testsecret&send_cohorts=true')
.to_return(status: 200, body: { flags: [flag] }.to_json)
stub_request(:post, flags_endpoint)
.to_return(status: 200, body: { featureFlags: { 'numeric-flag' => 'server-fallback' } }.to_json)

client = Client.new(api_key: API_KEY, personal_api_key: API_KEY, test_mode: true)
options = { person_properties: test_case[:properties], flag_keys: ['numeric-flag'] }
local_expected = test_case[:expected] == 'server-fallback' ? nil : test_case[:expected]
local_flags = client.evaluate_flags('test-id', **options, only_evaluate_locally: true)
expect(local_flags.get_flag('numeric-flag')).to eq(local_expected)
assert_not_requested :post, flags_endpoint

flags = client.evaluate_flags('test-id', **options)
expect(flags.get_flag('numeric-flag')).to eq(test_case[:expected])
if test_case[:expected] == 'server-fallback'
assert_requested :post, flags_endpoint, times: 1
else
assert_not_requested :post, flags_endpoint
end
ensure
client&.shutdown
end
end
end

it 'evaluates person properties' do
api_feature_flag_res = {
'flags' => [
Expand Down Expand Up @@ -1701,16 +1764,108 @@ module PostHog
property_e = { 'key' => 'key', 'value' => '30', 'operator' => 'lt' }
expect(FeatureFlagsPoller.match_property(property_e, { 'key' => '29' })).to be true

# depending on the type of override, we adjust type comparison
expect(FeatureFlagsPoller.match_property(property_e, { 'key' => '100' })).to be true
expect(FeatureFlagsPoller.match_property(property_e, { 'key' => '100' })).to be false
expect(FeatureFlagsPoller.match_property(property_e, { 'key' => 100 })).to be false

property_f = { 'key' => 'key', 'value' => '123aloha', 'operator' => 'gt' }
expect(FeatureFlagsPoller.match_property(property_f, { 'key' => '123' })).to be false
expect(FeatureFlagsPoller.match_property(property_f, { 'key' => 122 })).to be false
['123', 122, 129].each do |actual|
expect do
FeatureFlagsPoller.match_property(property_f, { 'key' => actual })
end.to raise_error(InconclusiveMatchError)
end
end

# this turns into a string comparison
expect(FeatureFlagsPoller.match_property(property_f, { 'key' => 129 })).to be true
describe 'numeric ordering' do
[
['gt', 2, '10', true],
['lt', 30, '100', false],
['gte', '5', '10', true],
['lte', '5', '10', false]
].each do |operator, filter, actual, expected|
it "compares #{actual.inspect} #{operator} #{filter.inspect} numerically" do
property = { key: 'key', value: filter, operator: operator }
expect(FeatureFlagsPoller.match_property(property, { key: actual })).to be(expected)
end
end

{
'gt' => [false, false, true],
'gte' => [false, true, true],
'lt' => [true, false, false],
'lte' => [true, true, false]
}.each do |operator, expected_results|
[5, 5.0, '5'].each do |filter|
[2, 5, 10, 2.0, 5.0, 10.0, '2', '5', '10',
BigDecimal('2'), BigDecimal('5'), BigDecimal('10')].each_with_index do |actual, index|
it "compares #{actual.inspect} #{operator} #{filter.inspect} numerically" do
property = { key: 'key', value: filter, operator: operator }
expect(FeatureFlagsPoller.match_property(property, { key: actual })).to be(expected_results[index % 3])
end
end
end
end

[
['+10', 10], ['-10', -10], ['10.', 10], ['.5', 0.5], ['-.5', -0.5],
['+1.25e+2', 125], ['-1.25E-2', -0.0125], ['0010', 10], ['-0', 0],
['1e-400', 0], ['9007199254740993', 9_007_199_254_740_992],
[9_007_199_254_740_993, 9_007_199_254_740_992]
].each do |string, number|
it "converts #{string.inspect} on either side to finite binary64" do
%w[gt gte lt lte].each do |operator|
[[string, number], [number, string]].each do |actual, filter|
property = { key: 'key', value: filter, operator: operator }
expected = %w[gte lte].include?(operator)
expect(FeatureFlagsPoller.match_property(property, { key: actual })).to be(expected)
end
end
end
end

[
[BigDecimal('10.5'), 10.5], [BigDecimal('-0.5'), -0.5],
[BigDecimal('9007199254740993'), 9_007_199_254_740_992],
[BigDecimal('1e-400'), 0], [BigDecimal('-1e-400'), 0]
].each do |actual, filter|
it "converts BigDecimal #{actual.inspect} properties to finite binary64" do
%w[gt gte lt lte].each do |operator|
property = { key: 'key', value: filter, operator: operator }
expected = %w[gte lte].include?(operator)
expect(FeatureFlagsPoller.match_property(property, { key: actual })).to be(expected)
end
end
end

[
'', ' 10', '10 ', "10\n", "\n10", "10\r\n", '10px', '1_0', '0x10', '1,5',
'.', '+', '1e', 'NaN', 'Infinity', '-Infinity', '1e309', '锛戯紣',
nil, true, false, [], [10], {}, Float::NAN, Float::INFINITY, -Float::INFINITY, 10**400,
BigDecimal('NaN'), BigDecimal('Infinity'), BigDecimal('-Infinity'), BigDecimal('1e400')
].each do |invalid|
it "rejects #{invalid.inspect} as a property and treats it as inconclusive as a filter" do
%w[gt gte lt lte].each do |operator|
[5, '5'].each do |valid|
property = { key: 'key', value: valid, operator: operator }
expect(FeatureFlagsPoller.match_property(property, { key: invalid })).to be(false)

property = { key: 'key', value: invalid, operator: operator }
[valid, nil].each do |actual|
expect do
FeatureFlagsPoller.match_property(property, { key: actual })
end.to raise_error(InconclusiveMatchError)
end
end
end
end
end

it 'preserves inconclusive matching for missing properties' do
%w[gt gte lt lte].each do |operator|
expect do
FeatureFlagsPoller.match_property({ key: 'key', value: '5', operator: operator }, {})
end.to raise_error(InconclusiveMatchError)
end
end
end

it 'with date operators' do
Expand Down Expand Up @@ -1891,10 +2046,12 @@ module PostHog
expect(FeatureFlagsPoller.match_property(property_e, { 'key' => nil })).to be false

property_f = { 'key' => 'key', 'value' => 1, 'operator' => 'lt' }
expect(FeatureFlagsPoller.match_property(property_f, { 'key' => nil })).to be true
expect(FeatureFlagsPoller.match_property(property_f, { 'key' => nil })).to be false

property_g = { 'key' => 'key', 'value' => 'xyz', 'operator' => 'gte' }
expect(FeatureFlagsPoller.match_property(property_g, { 'key' => nil })).to be false
expect do
FeatureFlagsPoller.match_property(property_g, { 'key' => nil })
end.to raise_error(InconclusiveMatchError)

property_j = { 'key' => 'key', 'value' => '2022-05-01', 'operator' => 'is_date_after' }
expect { FeatureFlagsPoller.match_property(property_j, { 'key' => nil }) }.to raise_error(InconclusiveMatchError)
Expand Down
Loading