From fa57dc061f9b42319d9c7558e9e93a9cc31afbbe Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Mon, 12 Oct 2026 06:06:31 +0000 Subject: [PATCH] feat(flags): honor experiment holdouts in local flag evaluation A flag definition carrying `filters.holdout` was evaluated locally as though the holdout did not exist, so every identity inside the holdout got a variant that disagreed with the server. Check the holdout before release conditions, using the flag-level bucketing identity, and resolve a held-out identifier to the synthetic `holdout-` variant. Membership hashes `holdout-` rather than reusing the dot-separated flag hash, which selects a different population. Co-Authored-By: Claude Opus 5 Generated-By: PostHog Desktop Task-Id: 4c71ea2a-1fb6-45d3-ace5-a6acc28b9676 --- .changeset/local-eval-holdouts.md | 5 + lib/posthog/feature_flags.rb | 37 ++++++ spec/posthog/feature_flags_holdouts_spec.rb | 118 ++++++++++++++++++++ 3 files changed, 160 insertions(+) create mode 100644 .changeset/local-eval-holdouts.md create mode 100644 spec/posthog/feature_flags_holdouts_spec.rb diff --git a/.changeset/local-eval-holdouts.md b/.changeset/local-eval-holdouts.md new file mode 100644 index 00000000..b4831bdc --- /dev/null +++ b/.changeset/local-eval-holdouts.md @@ -0,0 +1,5 @@ +--- +"posthog-ruby": minor +--- + +Local flag evaluation honors experiment holdouts. A flag whose definition carries `filters.holdout` now resolves a held-out identifier to `holdout-` before its release conditions, matching the server instead of silently bucketing the identifier into a regular variant. diff --git a/lib/posthog/feature_flags.rb b/lib/posthog/feature_flags.rb index 97592cfb..814c9433 100644 --- a/lib/posthog/feature_flags.rb +++ b/lib/posthog/feature_flags.rb @@ -1110,6 +1110,13 @@ def match_feature_flag_properties(flag, distinct_id, properties, evaluation_cach groups: {}, group_properties: {}, snapshot: _evaluation_snapshot) flag_filters = flag[:filters] || {} + # Holdouts precede release conditions: a held-out identifier is excluded from + # the flag's targeting entirely rather than bucketed into one of its variants. + # The per-condition aggregation handled below never changes holdout membership, + # so this uses the flag-level bucketing identity the caller resolved. + holdout_variant = holdout_variant_for(flag_filters[:holdout], distinct_id) + return holdout_variant unless holdout_variant.nil? + flag_conditions = flag_filters[:groups] || [] flag_aggregation = flag_filters[:aggregation_group_type_index] early_exit = flag_filters[:early_exit] == true @@ -1233,6 +1240,36 @@ def _hash(key, distinct_id, salt = '') (Integer(hash_key[0..14], 16).to_f / 0xfffffffffffffff) end + # An experiment holdout excludes an identifier from the flag's release conditions + # entirely, resolving it to the synthetic variant "holdout-". That variant does + # not need to exist in `filters.multivariate.variants`. Returns nil when the flag has + # no usable holdout or the identifier falls outside it, leaving ordinary evaluation + # unchanged. + def holdout_variant_for(holdout, bucketing_value) + return nil unless holdout.is_a?(Hash) + + id = holdout[:id] + exclusion_percentage = holdout[:exclusion_percentage] + return nil if id.nil? || !exclusion_percentage.is_a?(Numeric) + + # The server clamps an out-of-range percentage rather than rejecting it, and treats + # 100 as everyone without hashing, so a full holdout can't miss on a hash boundary. + # Fractional percentages are kept as-is. + percentage = exclusion_percentage.to_f.clamp(0, 100) + return nil if percentage < 100 && _holdout_hash(bucketing_value) > (percentage / 100) + + "holdout-#{id}" + end + + # Holdout membership hashes `holdout-` with no separator, salt, flag + # key or holdout id taking part, so one identity lands in the same holdout population + # on every flag, the way the server buckets it. The dot-separated flag hash above + # selects a different population and must not be reused here. + def _holdout_hash(bucketing_value) + hash_key = Digest::SHA1.hexdigest "holdout-#{bucketing_value}" + (Integer(hash_key[0..14], 16).to_f / 0xfffffffffffffff) + end + def get_matching_variant(flag, distinct_id) hash_value = _hash(flag[:key], distinct_id, 'variant') matching_variant = variant_lookup_table(flag).find do |variant| diff --git a/spec/posthog/feature_flags_holdouts_spec.rb b/spec/posthog/feature_flags_holdouts_spec.rb new file mode 100644 index 00000000..aaaf2d75 --- /dev/null +++ b/spec/posthog/feature_flags_holdouts_spec.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe PostHog::FeatureFlagsPoller, 'experiment holdouts' do + let(:poller) { described_class.new(60, nil, API_KEY, 'https://us.i.posthog.com', 3) } + + after { poller.shutdown_poller } + + # An active multivariate flag whose only release condition matches everyone and + # forces "control", so any result other than "control" or false comes from the + # holdout rather than from ordinary assignment. + def flag(holdout, key: 'checkout', **overrides) + filters = { + groups: [{ properties: [], rollout_percentage: 100, variant: 'control' }], + multivariate: { variants: [{ key: 'control', rollout_percentage: 50 }, + { key: 'test', rollout_percentage: 50 }] } + } + filters[:holdout] = holdout unless holdout.nil? + { key: key, active: true, filters: filters }.merge(overrides) + end + + def evaluate(flags, distinct_id: 'user-1', key: 'checkout', groups: {}, group_type_mapping: {}) + poller._apply_flag_definitions({ flags: flags.is_a?(Array) ? flags : [flags], + group_type_mapping: group_type_mapping, cohorts: {} }) + poller.get_feature_flag(key, distinct_id, groups, {}, {}, true)[0] + end + + describe 'membership bucketing' do + # The server hashes `holdout-` with no flag key, holdout id or + # salt. The dot-separated flag hash selects a different population, so these + # vectors pin the input the hash is computed over. + { + 'user-1' => 0.17805599206573022, + 'user-5' => 0.6563813925994418 + }.each do |bucketing_value, expected| + it "hashes #{bucketing_value} the way the server does" do + expect(poller.send(:_holdout_hash, bucketing_value)).to be_within(1e-9).of(expected) + end + end + + it 'holds out an identifier inside the percentage and lets the others through' do + definition = flag({ id: 727, exclusion_percentage: 20 }) + expect(evaluate(definition, distinct_id: 'user-1')).to eq('holdout-727') + expect(evaluate(definition, distinct_id: 'user-5')).to eq('control') + end + + it 'compares inclusively and clamps the percentage without truncating fractions' do + allow(poller).to receive(:_holdout_hash).and_return(0.125) + + [[12.5, 'holdout-727'], [12.4, 'control'], [0, 'control'], [-10, 'control']].each do |percentage, expected| + expect(evaluate(flag({ id: 727, exclusion_percentage: percentage }))).to eq(expected) + end + end + + [100, 150].each do |percentage| + it "holds everyone out at #{percentage} without computing a hash" do + expect(poller).not_to receive(:_holdout_hash) + expect(evaluate(flag({ id: 727, exclusion_percentage: percentage }))).to eq('holdout-727') + end + end + + it 'uses the flag-level group key rather than the distinct id' do + definition = flag({ id: 727, exclusion_percentage: 20 }) + definition[:filters][:aggregation_group_type_index] = 0 + + result = evaluate(definition, distinct_id: 'user-5', groups: { company: 'user-1' }, + group_type_mapping: { '0' => 'company' }) + expect(result).to eq('holdout-727') + end + end + + describe 'precedence' do + it 'wins over release conditions and a condition variant override' do + definition = flag({ id: 727, exclusion_percentage: 100 }) + definition[:filters][:groups] = [{ + properties: [{ key: 'region', value: 'USA', operator: 'exact', type: 'person' }], + rollout_percentage: 0, + variant: 'test' + }] + + expect(evaluate(definition)).to eq('holdout-727') + end + + it 'does not enable an inactive flag' do + definition = flag({ id: 727, exclusion_percentage: 100 }, active: false) + expect(evaluate(definition)).to be false + end + + it 'leaves ordinary assignment alone when there is no holdout' do + expect(evaluate(flag(nil))).to eq('control') + end + + [{ id: nil, exclusion_percentage: 100 }, { exclusion_percentage: 100 }, + { id: 727, exclusion_percentage: nil }, { id: 727 }, {}].each do |holdout| + it "skips the incomplete holdout #{holdout.inspect} and assigns normally" do + expect(evaluate(flag(holdout))).to eq('control') + end + end + end + + describe 'bulk and dependency evaluation' do + it 'resolves the holdout once and compares dependencies against that value' do + dependent = flag(nil, key: 'dependent') + dependent[:filters][:groups] = [{ + properties: [{ key: 'checkout', value: 'holdout-727', operator: 'flag_evaluates_to', type: 'flag', + dependency_chain: ['checkout'] }], + rollout_percentage: 100, + variant: 'control' + }] + definitions = [flag({ id: 727, exclusion_percentage: 100 }), dependent] + poller._apply_flag_definitions({ flags: definitions, group_type_mapping: {}, cohorts: {} }) + + flags = poller.get_all_flags_and_payloads('user-1', {}, {}, {}, true)[:featureFlags] + expect(flags).to eq({ 'checkout' => 'holdout-727', 'dependent' => 'control' }) + end + end +end