Skip to content

feat(flags): accept a caller default in FeatureFlagEvaluations#enabled? - #285

Merged
ioannisj merged 3 commits into
mainfrom
posthog/flag-evaluations-enabled-default-value
Oct 9, 2026
Merged

ioannisj merged 3 commits into
mainfrom
posthog/flag-evaluations-enabled-default-value

Conversation

@posthog

@posthog posthog Bot commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

The cross-SDK is-feature-enabled spec states a hard requirement with no server-SDK carve-out:

The SDK SHALL accept a caller-supplied boolean default (defaultValue; parameter placement per platform idiom) and SHALL return it whenever the flag has no value: flags not loaded yet, a failed flags request, or no flag with that key in the loaded flags. A flag that has a value — including false and variant strings — always wins over the caller-supplied default.

The compliance matrix records this as a ❌ Fail for posthog-ruby:

FeatureFlagEvaluations#enabled? has no default_value parameter: def enabled?(key) ... flag&.enabled ? true : false end — it collapses every miss to a hardcoded false rather than accepting a caller override.

Remediation: Add a default_value: keyword to FeatureFlagEvaluations#enabled?, distinguishing "no default supplied" (current false behavior) from an explicit caller-supplied default.

Today a caller cannot tell "the flag is off" apart from "we never got an answer", and cannot choose to fail open when flag data is unavailable (empty snapshot, failed /flags request, quota-limited response).

Explanation of the change

PostHog::FeatureFlagEvaluations#enabled? gains an optional default_value: keyword:

flags = posthog.evaluate_flags(distinct_id)
flags.enabled?('new-checkout', default_value: true)

The default is returned only when the key is absent from the snapshot. Any flag that has a value — true, false, or a variant string — still wins over it, and nil (the parameter's own default) preserves the historical false result for a missing flag.

$feature_flag_called reporting is deliberately untouched: the event still carries the real evaluated response (nil plus $feature_flag_error: flag_missing for a miss), not the caller's default, so exposure data keeps reflecting what the server actually said.

Scope is limited to this one contract. The deprecated Client#is_feature_enabled has the same gap but is not changed here — it already warns callers to move to evaluate_flags(...).enabled?, which is now the compliant surface. The matrix's other open gaps for this SDK are left alone.

Why this is backwards-compatible

Purely additive. default_value: defaults to nil, and the miss path is only diverted when a caller explicitly passes a non-nil value, so every existing call site behaves exactly as before. public_api_snapshot.txt is regenerated to record the new optional keyword.

💚 How did you test it?

Four new specs in spec/posthog/feature_flag_evaluations_spec.rb, mapping onto the spec's acceptance scenarios (acceptance/public/is-feature-enabled.feature):

  • a missing flag resolves to the caller default (true and false)
  • an existing value wins over the default (disabled flag, variant flag, boolean flag)
  • an empty snapshot uses the default, and still returns false when no default is given
  • $feature_flag_called still reports the evaluated response and flag_missing, not the default

Locally, on Ruby 3.2:

  • bundle exec rspec — 1252 examples, 0 failures (2 pending, the OTel-gemfile integration tests)
  • bundle exec rubocop — 129 files, no offenses
  • bundle exec rake public_api:check — clean after regenerating the snapshot

Follow-up work

  • The same default-value gap is recorded for posthog-node, posthog-php, posthog-go and posthog-dotnet; posthog-java is the existing reference implementation.
  • The spec's own "Surface variants" table shows a stale Ruby signature without a default parameter, and the docs for evaluate_flags could mention the new keyword.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm changeset to generate a changeset file

🤖 Agent context

Autonomy: Fully autonomous

Opened by the scheduled SDK-compliance agent, which reads the compliance matrices in PostHog/sdk-specs and implements one open, backwards-compatible gap per run. This gap was picked over the alternatives because it is a hard SHALL with a one-method, additive remediation on a non-deprecated surface, and because no existing PR or branch in this repo touched it.

Deliberate decisions: only the canonical evaluate_flags(...).enabled? path was changed (the legacy is_feature_enabled is deprecated, so extending it would add API surface we are steering callers away from); $feature_flag_called properties were left reporting the true evaluated response rather than the substituted default, so the default cannot distort exposure analytics.

No manual/end-to-end testing beyond the automated checks listed above.


Created with PostHog Desktop

🤖 Generated with Claude Code

The is-feature-enabled contract requires the SDK to accept a caller-supplied
boolean default and return it whenever the flag has no value. `enabled?`
collapsed every miss to a hardcoded `false`, leaving callers no way to make an
unknown flag resolve to `true`.

Add an optional `default_value:` keyword, returned only when the key is absent
from the snapshot. A flag that has a value, including `false` and variant
strings, still wins. Omitting the keyword keeps today's `false` result.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: 2fdafc3d-62a5-4644-b74a-650fbca5dc05
@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

posthog-ruby-sync Compliance Report

Date: 2026-10-08T17:35:44.229033+00:00
Duration: 93998ms

⚠️ Some Tests Failed

45/47 tests passed, 2 failed


Capture Tests

⚠️ 29/30 tests passed, 1 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 10ms
Format Validation.Event Has Uuid ✅ 7ms
Format Validation.Event Has Lib Properties ✅ 8ms
Format Validation.Distinct Id Is String ✅ 10ms
Format Validation.Token Is Present ✅ 9ms
Format Validation.Custom Properties Preserved ✅ 8ms
Format Validation.Event Has Timestamp ✅ 8ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 8ms
Retry Behavior.Retries On 503 ✅ 5248ms
Retry Behavior.Does Not Retry On 400 ✅ 2011ms
Retry Behavior.Does Not Retry On 401 ✅ 2011ms
Retry Behavior.Respects Retry After Header ✅ 8016ms
Retry Behavior.Implements Backoff ✅ 15371ms
Retry Behavior.Retries On 500 ✅ 5143ms
Retry Behavior.Retries On 502 ✅ 5114ms
Retry Behavior.Retries On 504 ✅ 5150ms
Retry Behavior.Max Retries Respected ✅ 15473ms
Deduplication.Generates Unique Uuids ✅ 21ms
Deduplication.Preserves Uuid On Retry ✅ 5145ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 10260ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 5121ms
Deduplication.No Duplicate Events In Batch ✅ 18ms
Deduplication.Different Events Have Different Uuids ✅ 9ms
Compression.Sends Gzip When Enabled ✅ 6ms
Batch Format.Uses Proper Batch Structure ✅ 6ms
Batch Format.Flush With No Events Sends Nothing ✅ 4ms
Batch Format.Multiple Events Batched Together ❌ 18ms
Error Handling.Does Not Retry On 403 ✅ 2006ms
Error Handling.Does Not Retry On 413 ✅ 2010ms
Error Handling.Retries On 408 ✅ 5115ms

Failures

batch_format.multiple_events_batched_together

Expected 1 requests, got 5

Feature_Flags Tests

⚠️ 16/17 tests passed, 1 failed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 8ms
Request Payload.Flags Request Uses V2 Query Param ✅ 7ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 7ms
Request Payload.Flags Request Omits Authorization Header ✅ 8ms
Request Payload.Token In Flags Body Matches Init ✅ 8ms
Request Payload.Groups Round Trip ✅ 8ms
Request Payload.Groups Default To Empty Object ✅ 8ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 6ms
Request Payload.Disable Geoip Omitted Defaults To False ❌ 7ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 6ms
Request Lifecycle.No Flags Request On Init Alone ✅ 2ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 6ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 10ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 7ms
Retry Behavior.Retries Flags On 502 ✅ 109ms
Retry Behavior.Retries Flags On 504 ✅ 111ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 9ms

Failures

request_payload.disable_geoip_omitted_defaults_to_false

Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['distinct_id', 'groups', 'person_properties', 'group_properties', 'flag_keys_to_evaluate', 'token']

@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

posthog-ruby-async Compliance Report

Date: 2026-10-08T17:35:47.782946+00:00
Duration: 97983ms

⚠️ Some Tests Failed

46/47 tests passed, 1 failed


Capture Tests

✅ 30/30 tests passed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 106ms
Format Validation.Event Has Uuid ✅ 104ms
Format Validation.Event Has Lib Properties ✅ 105ms
Format Validation.Distinct Id Is String ✅ 104ms
Format Validation.Token Is Present ✅ 103ms
Format Validation.Custom Properties Preserved ✅ 104ms
Format Validation.Event Has Timestamp ✅ 103ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 5ms
Retry Behavior.Retries On 503 ✅ 5308ms
Retry Behavior.Does Not Retry On 400 ✅ 2108ms
Retry Behavior.Does Not Retry On 401 ✅ 2107ms
Retry Behavior.Respects Retry After Header ✅ 8112ms
Retry Behavior.Implements Backoff ✅ 15520ms
Retry Behavior.Retries On 500 ✅ 5209ms
Retry Behavior.Retries On 502 ✅ 5210ms
Retry Behavior.Retries On 504 ✅ 5209ms
Retry Behavior.Max Retries Respected ✅ 15620ms
Deduplication.Generates Unique Uuids ✅ 109ms
Deduplication.Preserves Uuid On Retry ✅ 5209ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 10310ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 5211ms
Deduplication.No Duplicate Events In Batch ✅ 107ms
Deduplication.Different Events Have Different Uuids ✅ 104ms
Compression.Sends Gzip When Enabled ✅ 105ms
Batch Format.Uses Proper Batch Structure ✅ 103ms
Batch Format.Flush With No Events Sends Nothing ✅ 3ms
Batch Format.Multiple Events Batched Together ✅ 108ms
Error Handling.Does Not Retry On 403 ✅ 2106ms
Error Handling.Does Not Retry On 413 ✅ 2106ms
Error Handling.Retries On 408 ✅ 5209ms

Feature_Flags Tests

⚠️ 16/17 tests passed, 1 failed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 105ms
Request Payload.Flags Request Uses V2 Query Param ✅ 103ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 104ms
Request Payload.Flags Request Omits Authorization Header ✅ 104ms
Request Payload.Token In Flags Body Matches Init ✅ 104ms
Request Payload.Groups Round Trip ✅ 104ms
Request Payload.Groups Default To Empty Object ✅ 104ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 104ms
Request Payload.Disable Geoip Omitted Defaults To False ❌ 103ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 104ms
Request Lifecycle.No Flags Request On Init Alone ✅ 2ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 103ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 109ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 105ms
Retry Behavior.Retries Flags On 502 ✅ 204ms
Retry Behavior.Retries Flags On 504 ✅ 232ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 105ms

Failures

request_payload.disable_geoip_omitted_defaults_to_false

Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['distinct_id', 'groups', 'person_properties', 'group_properties', 'flag_keys_to_evaluate', 'token']

ioannisj
ioannisj previously approved these changes Oct 6, 2026
@ioannisj
ioannisj marked this pull request as ready for review October 6, 2026 23:57
@ioannisj
ioannisj requested a review from a team as a code owner October 6, 2026 23:57
@posthog-project-board-bot posthog-project-board-bot Bot moved this from Approved to In Review in Feature Flags Oct 6, 2026
@ioannisj
ioannisj dismissed their stale review October 8, 2026 13:03

🤖 An AI agent wrote this on behalf of @ioannisj.

Pulling my approval since this adds to the public API (default_value: on FeatureFlagEvaluations#enabled?), so I'd like the team to have a look too. Requested a review from team-client-libraries.

@dustinbyrne dustinbyrne left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a test

key = key.to_s
flag = @flags[key]
_record_access(key, flag)
return !!default_value if flag.nil? && !default_value.nil?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When /flags marks a flag failed: true (with errorsWhileComputingFlags), evaluate_flags still stores it as enabled: false. That means flag isn't nil here, so the default never applies. I confirmed this with a failing spec: { 'f' => { enabled: false, failed: true } } makes enabled?('f', default_value: true) return false. The doc comment and changeset both say the default covers "the evaluation failed", and that's the main fail-open case callers want this for. I think we need to treat failed records as having no value (skip them in evaluate_flags, or carry failed on the record and check it here). Otherwise we should drop that clause from the docs and changeset.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 An AI agent wrote this on behalf of @ioannisj.

Fixed the docs rather than the behavior in 25100ed. posthog-node's evaluateFlags stores failed flags the same way, and isEnabled only falls back to defaultValue when the key is absent, so skipping them here would put Ruby out of step with node.

The doc comment and changeset now say the default covers a failed /flags request (nothing in the snapshot at all), and that a flag the server marked failed resolves to false. Added specs for both, including your repro.

If we want fail-open on per-flag failures, I think that's a cross-SDK change and better raised in sdk-specs first.

@ioannisj
ioannisj merged commit bd2d976 into main Oct 9, 2026
23 checks passed
@ioannisj
ioannisj deleted the posthog/flag-evaluations-enabled-default-value branch October 9, 2026 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants